nanohub: fix event handling for CHRE
Test: scenario is below
cherry-pick ag/1511251
build and update the nanohub OS;
rebuild all the GTS nanoapps and then rebuild GTS itself
gts-tradefed run everything --skip-system-status-check
com.android.compatibility.common.tradefed.targetprep.NetworkConnectivityChecker
--module GtsLocationTestCases --test
com.google.android.location.gts.ContextHubSendEventNanoAppTest#runTest
--primary-abi-only
Test should pass
Bug: 31975247
Change-Id: I4b1dfa5ee42e8418356bfffe6199dae0bdb84d64
Signed-off-by: Alexey Polyudov <apolyudov@google.com>
diff --git a/firmware/app/chre/chre_test0.app/main.c b/firmware/app/chre/chre_test0.app/main.c
index 46700ff..acdba52 100644
--- a/firmware/app/chre/chre_test0.app/main.c
+++ b/firmware/app/chre/chre_test0.app/main.c
@@ -18,6 +18,8 @@
#include <inttypes.h>
#include <chre.h>
+#define CHRE_APP_TAG "CHRE App 0: "
+
/* chre.h does not define printf format attribute for chreLog() */
void chreLog(enum chreLogLevel level, const char *str, ...) __attribute__ ((__format__ (__printf__, 2, 3)));
@@ -40,9 +42,17 @@
static int cnt;
static struct MyTimer mTimer;
+static void nanoappFreeEvent(uint16_t eventType, void *data)
+{
+ chreLog(CHRE_LOG_INFO, CHRE_APP_TAG "event callback invoked: eventType=%04" PRIX16
+ "; data=%p\n", eventType, data);
+}
+
// Default implementation for message free
static void nanoappFreeMessage(void *msg, size_t size)
{
+ chreLog(CHRE_LOG_INFO, CHRE_APP_TAG "message callback invoked: msg=%p; size=%08zu\n",
+ msg, size);
chreHeapFree(msg);
}
@@ -51,14 +61,13 @@
mMyAppId = chreGetAppId();
mMyTid = chreGetInstanceId();
cnt = 3;
- chreSendEvent(EVT_LOCAL_SETUP, NULL, NULL, mMyTid);
- chreLog(CHRE_LOG_INFO, "CHRE App 0: init");
+ chreSendEvent(EVT_LOCAL_SETUP, (void*)0x87654321, nanoappFreeEvent, mMyTid);
+ chreLog(CHRE_LOG_INFO, CHRE_APP_TAG "init\n");
return true;
}
void nanoappEnd(void)
{
- chreLog(CHRE_LOG_INFO, "CHRE App 0: terminating");
}
void nanoappHandleEvent(uint32_t srcTid, uint16_t evtType, const void* evtData)
@@ -66,7 +75,7 @@
switch (evtType) {
case EVT_LOCAL_SETUP:
mTimer.timerId = chreTimerSet(kOneSecond, &mTimer, false);
- chreLog(CHRE_LOG_INFO, "CHRE App 0: started with tid %04" PRIX32
+ chreLog(CHRE_LOG_INFO, CHRE_APP_TAG "started with tid %04" PRIX32
" timerid %" PRIu32
"\n", mMyTid, mTimer.timerId);
break;
@@ -75,7 +84,7 @@
const struct MyTimer *t = (const struct MyTimer *)evtData;
struct ExtMsg *extMsg = chreHeapAlloc(sizeof(*extMsg));
- chreLog(CHRE_LOG_INFO, "CHRE App 0: received timer %" PRIu32
+ chreLog(CHRE_LOG_INFO, CHRE_APP_TAG "received timer %" PRIu32
" (TIME: %" PRIu64
") cnt: %d\n", t->timerId, chreGetTime(), cnt);
extMsg->msg = 0x01;
@@ -90,7 +99,7 @@
const struct chreMessageFromHostData *msg = (const struct chreMessageFromHostData *)evtData;
const uint8_t *data = (const uint8_t *)msg->message;
const size_t size = msg->messageSize;
- chreLog(CHRE_LOG_INFO, "CHRE App 0: message=%p; code=%d; size=%zu",
+ chreLog(CHRE_LOG_INFO, CHRE_APP_TAG "message=%p; code=%d; size=%zu\n",
data, (data && size) ? data[0] : 0, size);
break;
}
diff --git a/firmware/app/chre/common/chre_app.c b/firmware/app/chre/common/chre_app.c
index f95402c..d12941a 100644
--- a/firmware/app/chre/common/chre_app.c
+++ b/firmware/app/chre/common/chre_app.c
@@ -70,6 +70,10 @@
u.msg.messageSize = hdr->size;
break;
}
+ default:
+ // ignore any other system events; OS may send them to any app
+ if (evt < EVT_NO_FIRST_USER_EVENT)
+ return;
}
nanoappHandleEvent(srcTid, evt, data);
}
diff --git a/firmware/os/core/nanohub_chre.c b/firmware/os/core/nanohub_chre.c
index 6a9a4e2..7bb42c8 100644
--- a/firmware/os/core/nanohub_chre.c
+++ b/firmware/os/core/nanohub_chre.c
@@ -45,17 +45,6 @@
* None of the methods returning uint32_t are cast to uintptr_t
* This is done in order to let compiler warn us if our assumption is not safe for some reason
*/
-void osChreTaskHandle(struct Task *task, uint32_t evtType, const void *evtData)
-{
- uint16_t evt = evtType;
- if (evt < EVT_FIRST_CHRE_USER_EVENT && evt >= EVT_FIRST_CHRE_SYS_EVENT) {
- evt -= EVT_FIRST_CHRE_SYS_EVENT;
- } else if (evt >= EVT_FIRST_CHRE_USER_EVENT) {
- evt = evt - EVT_FIRST_CHRE_USER_EVENT + CHRE_EVENT_FIRST_USER_VALUE;
- }
- evtType = EVENT_WITH_ORIGIN(evt, EVENT_GET_ORIGIN(evtType));
- cpuAppHandle(task->app, &task->platInfo, evtType, evtData);
-}
static inline uint64_t osChreGetAppId(void)
{
@@ -148,24 +137,33 @@
heapFree(ptr);
}
+/*
+ * we have no way to verify if this is a CHRE event; just trust the caller to do the right thing
+ */
+void osChreFreeEvent(uint32_t tid, chreEventCompleteFunction *cbFreeEvt, uint32_t evtType, void * evtData)
+{
+ struct Task *chreTask = osTaskFindByTid(tid);
+ struct Task *preempted = osSetCurrentTask(chreTask);
+ if (chreTask && osTaskIsChre(chreTask))
+ osTaskInvokeEventFreeCallback(chreTask, cbFreeEvt, evtType, evtData);
+ osSetCurrentTask(preempted);
+}
+
static bool osChreSendEvent(uint16_t evtType, void *evtData,
chreEventCompleteFunction *evtFreeCallback,
uint32_t toTid)
{
- uint32_t evt;
- if (evtType >= CHRE_EVENT_FIRST_USER_VALUE) {
- evt = evtType - CHRE_EVENT_FIRST_USER_VALUE + EVT_FIRST_CHRE_USER_EVENT;
- if (evt >= EVT_DEBUG_LOG) {
- osLog(LOG_INFO, "%s: CHRE User Event %04" PRIX16 " is not compatible with nanohub", __func__, evtType);
- return false;
- }
- } else if (evtType < MAX_CHRE_SYS_EVENTS) {
- evt = evtType + EVT_FIRST_CHRE_SYS_EVENT;
- } else {
- osLog(LOG_INFO, "%s: CHRE System Event %04" PRIX16 " is not compatible with nanohub", __func__, evtType);
+ /*
+ * this primitive may only be used for USER CHRE events;
+ * system events come from the OS itself through different path,
+ * and are interpreted by the CHRE app compatibility library.
+ * therefore, we have to enforce the evtType >= CHRE_EVENT_FIRST_USER_VALUE.
+ */
+ if (evtType < CHRE_EVENT_FIRST_USER_VALUE) {
+ osChreFreeEvent(osGetCurrentTid(), evtFreeCallback, evtType, evtData);
return false;
}
- return osEnqueuePrivateEvtNew(evt, evtData, evtFreeCallback, toTid);
+ return osEnqueuePrivateEvtNew(evtType, evtData, evtFreeCallback, toTid);
}
static void osChreApiSendEvent(uintptr_t *retValP, va_list args)
@@ -199,7 +197,7 @@
result = osEnqueueEvtOrFree(EVT_APP_TO_HOST, hostMsg, heapFree);
out:
- if (freeCallback && message && messageSize)
+ if (freeCallback)
osTaskInvokeMessageFreeCallback(osGetCurrentTask(), freeCallback, message, messageSize);
return result;
}
diff --git a/firmware/os/core/seos.c b/firmware/os/core/seos.c
index 603a55d..450fb66 100644
--- a/firmware/os/core/seos.c
+++ b/firmware/os/core/seos.c
@@ -71,7 +71,7 @@
return mCurrentTask;
}
-static struct Task *osSetCurrentTask(struct Task *task)
+struct Task *osSetCurrentTask(struct Task *task)
{
struct Task *old = mCurrentTask;
while (true) {
@@ -90,11 +90,6 @@
return (atomicReadByte(&task->flags) & mask) != 0;
}
-static bool osTaskIsChre(const struct Task *task)
-{
- return (task->app->hdr.fwFlags & FL_APP_HDR_CHRE) != 0;
-}
-
bool osAppIsChre(uint16_t tid)
{
struct Task *task = osTaskFindByTid(tid);
@@ -321,11 +316,8 @@
static inline void osTaskHandle(struct Task *task, uint32_t evtType, const void* evtData)
{
struct Task *preempted = osSetCurrentTask(task);
- uint16_t evt = EVENT_GET_EVENT(evtType);
- if (osTaskIsChre(task))
- osChreTaskHandle(task, evtType, evtData);
- else
- cpuAppHandle(task->app, &task->platInfo, evt, evtData);
+ cpuAppHandle(task->app, &task->platInfo,
+ osTaskIsChre(task) ? evtType : EVENT_GET_EVENT(evtType), evtData);
osSetCurrentTask(preempted);
}
@@ -340,7 +332,9 @@
{
if (!task || !freeCallback)
return;
- cpuAppInvoke(task->app, &task->platInfo, (void (*)(uintptr_t,uintptr_t))freeCallback, (uintptr_t)event, (uintptr_t)data);
+ cpuAppInvoke(task->app, &task->platInfo,
+ (void (*)(uintptr_t,uintptr_t))freeCallback,
+ (uintptr_t)event, (uintptr_t)data);
}
void osEventHeapFree(uint16_t event, void *data)
@@ -350,33 +344,30 @@
static void handleEventFreeing(uint32_t evtType, void *evtData, TaggedPtr evtFreeData) // watch out, this is synchronous
{
- uint16_t evt;
- uint16_t tid;
- struct Task *srcTask;
+ struct Task *srcTask = osTaskFindByTid(EVENT_GET_ORIGIN(evtType));
if (!taggedPtrToUint(evtFreeData) || !evtData)
return;
- evt = EVENT_GET_EVENT(evtType);
- tid = EVENT_GET_ORIGIN(evtType);
- srcTask = osTaskFindByTid(tid);
+ if (!srcTask) {
+ osLog(LOG_ERROR, "ERROR: Failed to find task to free event: evtType=%08" PRIX32 "\n", evtType);
+ return;
+ }
+
+ // release non-CHRE event; we can't determine if this is CHRE or non-CHRE event, but
+ // this method is only called to release non-CHRE events, so we make use of that fact
if (taggedPtrIsPtr(evtFreeData)) {
- struct Task *task = osGetCurrentTask();
- if ((task->app->hdr.fwFlags & FL_APP_HDR_CHRE)) {
- osTaskInvokeEventFreeCallback(task, (void (*)(uint16_t, void *))taggedPtrToPtr(evtFreeData),
- evt, evtData);
- } else {
- ((EventFreeF)taggedPtrToPtr(evtFreeData))(evtData);
- }
+ // this is for internal non-CHRE tasks, and CHRE tasks
+ // System may schedule non-CHRE events on behalf of CHRE app;
+ // this is the place we release them
+ struct Task *preempted = osSetCurrentTask(srcTask);
+ ((EventFreeF)taggedPtrToPtr(evtFreeData))(evtData);
+ osSetCurrentTask(preempted);
} else {
+ // this is for external non-CHRE tasks
struct AppEventFreeData fd = {.evtType = evtType, .evtData = evtData};
- struct Task* task = osTaskFindByTid(taggedPtrToUint(evtFreeData));
-
- if (!task)
- osLog(LOG_ERROR, "EINCEPTION: Failed to find app to call app to free event sent to app(s).\n");
- else
- osTaskHandle(task, EVT_APP_FREE_EVT_DATA, &fd);
+ osTaskHandle(srcTask, EVT_APP_FREE_EVT_DATA, &fd);
}
osTaskAddIoCount(srcTask, -1);
}
@@ -716,7 +707,7 @@
osTaskHandle(task, EVT_APP_STOP, NULL);
osEnqueueEvtOrFree(EVT_APP_END, task, NULL);
} else {
- osTaskEnd(task);
+ osTaskEnd(task); // calls app END() and Release()
osUnloadApp(task);
}
@@ -726,9 +717,13 @@
void osTaskAbort(struct Task *task)
{
- osRemoveTask(task);
- osTaskRelease(task);
- osUnloadApp(task);
+ if (!task)
+ return;
+
+ osRemoveTask(task); // remove from active task list
+ // do not call app END()
+ osTaskRelease(task); // release all system resources
+ osUnloadApp(task); // destroy platform app object in RAM
}
static bool osExtAppFind(struct SegmentIterator *it, uint64_t appId)
@@ -990,7 +985,8 @@
case EVT_PRIVATE_EVT:
task = osTaskFindByTid(da->privateEvt.toTid);
- evtType = EVENT_WITH_ORIGIN(da->privateEvt.evtType, da->privateEvt.fromTid);
+ evtType = EVENT_WITH_ORIGIN(da->privateEvt.evtType & EVT_MASK, da->privateEvt.fromTid);
+ evtData = da->privateEvt.evtData;
if (task) {
//private events cannot be retained
TaggedPtr *tmp = mCurEvtEventFreeingInfo;
@@ -998,8 +994,13 @@
osTaskHandle(task, evtType, da->privateEvt.evtData);
mCurEvtEventFreeingInfo = tmp;
}
-
- handleEventFreeing(evtType, da->privateEvt.evtData, da->privateEvt.evtFreeInfo);
+ if ((da->privateEvt.evtType >> 16) == EVT_PRIVATE_CLASS_CHRE) {
+ osChreFreeEvent(da->privateEvt.fromTid,
+ (void (*)(uint16_t, void *))taggedPtrToPtr(da->privateEvt.evtFreeInfo),
+ evtType, evtData);
+ } else {
+ handleEventFreeing(evtType, evtData, da->privateEvt.evtFreeInfo);
+ }
break;
}
osSetCurrentTask(preempted);
@@ -1206,7 +1207,7 @@
if (taggedPtrToUint(evtFreeInfo) && evtData)
osTaskAddIoCount(task, 1);
- act->privateEvt.evtType = evtType & EVT_MASK;
+ act->privateEvt.evtType = evtType;
act->privateEvt.evtData = evtData;
act->privateEvt.evtFreeInfo = evtFreeInfo;
act->privateEvt.fromTid = task->tid;
@@ -1218,13 +1219,14 @@
return result;
}
+// only called to send events for CHRE apps
bool osEnqueuePrivateEvtNew(uint16_t evtType, void *evtData,
void (*evtFreeCallback)(uint16_t evtType, void *evtData),
uint32_t toTid)
{
- if (!osEnqueuePrivateEvtEx(evtType, evtData, taggedPtrMakeFromPtr(evtFreeCallback), toTid)) {
- handleEventFreeing(EVENT_WITH_ORIGIN(evtType, osGetCurrentTid()),
- evtData, taggedPtrMakeFromPtr(evtFreeCallback));
+ if (!osEnqueuePrivateEvtEx(evtType | (EVT_PRIVATE_CLASS_CHRE << 16), evtData,
+ taggedPtrMakeFromPtr(evtFreeCallback), toTid)) {
+ osChreFreeEvent(osGetCurrentTid(), evtFreeCallback, evtType, evtData);
return false;
}
return true;
@@ -1232,13 +1234,13 @@
bool osEnqueuePrivateEvt(uint32_t evtType, void *evtData, EventFreeF evtFreeF, uint32_t toTid)
{
- return osEnqueuePrivateEvtEx(evtType, evtData, taggedPtrMakeFromPtr(evtFreeF), toTid);
+ return osEnqueuePrivateEvtEx(evtType & EVT_MASK, evtData, taggedPtrMakeFromPtr(evtFreeF), toTid);
}
bool osEnqueuePrivateEvtAsApp(uint32_t evtType, void *evtData, uint32_t fromAppTid, uint32_t toTid)
{
(void)fromAppTid;
- return osEnqueuePrivateEvtEx(evtType, evtData, taggedPtrMakeFromUint(osGetCurrentTid()), toTid);
+ return osEnqueuePrivateEvtEx(evtType & EVT_MASK, evtData, taggedPtrMakeFromUint(osGetCurrentTid()), toTid);
}
bool osTidById(uint64_t *appId, uint32_t *tid)
diff --git a/firmware/os/cpu/cortexm4/appSupport.c b/firmware/os/cpu/cortexm4/appSupport.c
index 5c6605c..41bf8a3 100644
--- a/firmware/os/cpu/cortexm4/appSupport.c
+++ b/firmware/os/cpu/cortexm4/appSupport.c
@@ -202,6 +202,7 @@
(void)callWithR9((const void*)APP_FLASH_RELOC_BASE(app), app->vec.end, platInfo->data, 0, 0);
else
APP_VEC(app)->end();
+ osLog(LOG_INFO, "App ID %016" PRIX64 "; TID=%04" PRIX32 " terminated\n", app->hdr.appId, osGetCurrentTid());
}
void cpuAppHandle(const struct AppHdr *app, struct PlatAppInfo *platInfo, uint32_t evtType, const void* evtData)
diff --git a/firmware/os/inc/chreApi.h b/firmware/os/inc/chreApi.h
index 8f1ddc6..ef43429 100644
--- a/firmware/os/inc/chreApi.h
+++ b/firmware/os/inc/chreApi.h
@@ -60,5 +60,7 @@
//called by os entry point to export the api
void osChreApiExport(void);
+// release CHRE event and optionally call completion callback
+void osChreFreeEvent(uint32_t tid, void (*free_info)(uint16_t, void *), uint32_t evtType, void * evtData);
#endif
diff --git a/firmware/os/inc/eventnums.h b/firmware/os/inc/eventnums.h
index 1bb2c6c..e22fa04 100644
--- a/firmware/os/inc/eventnums.h
+++ b/firmware/os/inc/eventnums.h
@@ -20,8 +20,6 @@
#include <stdint.h>
#include "toolchain.h"
-#define MAX_CHRE_SYS_EVENTS 0x100
-
/* These define ranges of reserved events */
// local events are 16-bit always
#define EVT_NO_FIRST_USER_EVENT 0x00000100 //all events lower than this are reserved for the OS. all of them are nondiscardable necessarily!
@@ -31,8 +29,6 @@
#define EVT_APP_TO_HOST 0x00000401 //app data to host. Type is struct HostHubRawPacket
#define EVT_MARSHALLED_SENSOR_DATA 0x00000402 //marshalled event data. Type is MarshalledUserEventData
#define EVT_RESET_REASON 0x00000403 //reset reason to host.
-#define EVT_FIRST_CHRE_SYS_EVENT 0x00000500
-#define EVT_FIRST_CHRE_USER_EVENT (EVT_FIRST_CHRE_SYS_EVENT + MAX_CHRE_SYS_EVENTS)
#define EVT_DEBUG_LOG 0x00007F01 // send message payload to Linux kernel log
#define EVT_MASK 0x0000FFFF
diff --git a/firmware/os/inc/seos_priv.h b/firmware/os/inc/seos_priv.h
index 214d65a..b38cf4b 100644
--- a/firmware/os/inc/seos_priv.h
+++ b/firmware/os/inc/seos_priv.h
@@ -20,7 +20,6 @@
#include <inttypes.h>
#include <seos.h>
-#define EVT_PRIVATE_EVT 0x00000003
#define NO_NODE (TaskIndex)(-1)
#define for_each_task(listHead, task) for (task = osTaskByIdx((listHead)->next); task; task = osTaskByIdx(task->list.next))
@@ -31,6 +30,9 @@
#define EVT_SUBSCRIBE_TO_EVT 0x00000000
#define EVT_UNSUBSCRIBE_TO_EVT 0x00000001
#define EVT_DEFERRED_CALLBACK 0x00000002
+#define EVT_PRIVATE_EVT 0x00000003
+
+#define EVT_PRIVATE_CLASS_CHRE 0x00000001
#define EVENT_WITH_ORIGIN(evt, origin) (((evt) & EVT_MASK) | ((origin) << (32 - TASK_TID_BITS)))
#define EVENT_GET_ORIGIN(evt) ((evt) >> (32 - TASK_TID_BITS))
@@ -106,12 +108,18 @@
uint8_t osTaskIndex(struct Task *task);
struct Task *osGetCurrentTask();
+struct Task *osSetCurrentTask(struct Task *task);
struct Task *osTaskFindByTid(uint32_t tid);
void osTaskAbort(struct Task *task);
void osTaskInvokeMessageFreeCallback(struct Task *task, void (*freeCallback)(void *, size_t), void *message, uint32_t messageSize);
void osTaskInvokeEventFreeCallback(struct Task *task, void (*freeCallback)(uint16_t, void *), uint16_t event, void *data);
void osChreTaskHandle(struct Task *task, uint32_t evtType, const void *evtData);
+static inline bool osTaskIsChre(const struct Task *task)
+{
+ return (task->app->hdr.fwFlags & FL_APP_HDR_CHRE) != 0;
+}
+
static inline void osTaskMakeNewTid(struct Task *task)
{
task->tid = ((task->tid + TASK_TID_INCREMENT) & TASK_TID_COUNTER_MASK) |
