From 7c24c770be3b3e25822cf7c45619ee20ed61c172 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonny=20Sv=C3=A4rd?= Date: Thu, 14 Jan 2021 19:53:17 +0100 Subject: Improve mailbox message handling Introduce a 32b magic for each message. Verify the magic for all incoming messages. Add reset function - in case of protocol error, effectively reset/empty the incoming queue. Add an error message type and message Add version request/response Verify payload length of responses (when applicable) Change-Id: I8aadd4012024492533d52e2cdb38630fce5c36e2 --- kernel/ethosu_core_interface.h | 41 ++++++++++++++++- kernel/ethosu_device.c | 69 ++++++++++++++++++++++++++--- kernel/ethosu_mailbox.c | 62 ++++++++++++++++++++++---- kernel/ethosu_mailbox.h | 20 +++++++++ kernel/uapi/ethosu.h | 1 + utils/inference_runner/inference_runner.cpp | 4 +- 6 files changed, 180 insertions(+), 17 deletions(-) diff --git a/kernel/ethosu_core_interface.h b/kernel/ethosu_core_interface.h index 86e10ac..a3a21e0 100644 --- a/kernel/ethosu_core_interface.h +++ b/kernel/ethosu_core_interface.h @@ -33,16 +33,24 @@ /** Maximum number of PMU counters to be returned for inference */ #define ETHOSU_CORE_PMU_MAX 4 +#define ETHOSU_CORE_MSG_MAGIC 0x41457631 +#define ETHOSU_CORE_MSG_VERSION_MAJOR 0 +#define ETHOSU_CORE_MSG_VERSION_MINOR 2 +#define ETHOSU_CORE_MSG_VERSION_PATCH 0 + /** * enum ethosu_core_msg_type - Message types * * Types for the messages sent between the host and the core subsystem. */ enum ethosu_core_msg_type { - ETHOSU_CORE_MSG_PING = 1, + ETHOSU_CORE_MSG_ERR = 1, + ETHOSU_CORE_MSG_PING, ETHOSU_CORE_MSG_PONG, ETHOSU_CORE_MSG_INFERENCE_REQ, ETHOSU_CORE_MSG_INFERENCE_RSP, + ETHOSU_CORE_MSG_VERSION_REQ, + ETHOSU_CORE_MSG_VERSION_RSP, ETHOSU_CORE_MSG_MAX }; @@ -50,6 +58,7 @@ enum ethosu_core_msg_type { * struct ethosu_core_msg - Message header */ struct ethosu_core_msg { + uint32_t magic; uint32_t type; uint32_t length; }; @@ -105,4 +114,34 @@ struct ethosu_core_inference_rsp { uint64_t pmu_cycle_counter_count; }; +/** + * struct ethosu_core_msg_verson - Message protocol version + */ +struct ethosu_core_msg_version { + uint8_t major; + uint8_t minor; + uint8_t patch; + uint8_t _reserved; +}; + +/** + * enum ethosu_core_msg_err_type - Error types + */ +enum ethosu_core_msg_err_type { + ETHOSU_CORE_MSG_ERR_GENERIC = 0, + ETHOSU_CORE_MSG_ERR_UNSUPPORTED_TYPE, + ETHOSU_CORE_MSG_ERR_INVALID_PAYLOAD, + ETHOSU_CORE_MSG_ERR_INVALID_SIZE, + ETHOSU_CORE_MSG_ERR_INVALID_MAGIC, + ETHOSU_CORE_MSG_ERR_MAX +}; + +/** + * struct ethosu_core_msg_err - Error message struct + */ +struct ethosu_core_msg_err { + uint32_t type; /* optional use of extra error code */ + char msg[128]; +}; + #endif diff --git a/kernel/ethosu_device.c b/kernel/ethosu_device.c index 5301f86..e82b4c0 100644 --- a/kernel/ethosu_device.c +++ b/kernel/ethosu_device.c @@ -55,14 +55,17 @@ * Functions ****************************************************************************/ +/* Incoming messages */ static int ethosu_handle_msg(struct ethosu_device *edev) { + int ret; struct ethosu_core_msg header; union { + struct ethosu_core_msg_err error; struct ethosu_core_inference_rsp inf; + struct ethosu_core_msg_version version; } data; - int ret; /* Read message */ ret = ethosu_mailbox_read(&edev->mailbox, &header, &data, sizeof(data)); @@ -70,24 +73,71 @@ static int ethosu_handle_msg(struct ethosu_device *edev) return ret; switch (header.type) { + case ETHOSU_CORE_MSG_ERR: + if (header.length != sizeof(data.error)) { + dev_warn(edev->dev, + "Msg: Error message of incorrect size. size=%u, expected=%zu\n", header.length, + sizeof(data.error)); + ret = -EBADMSG; + break; + } + + data.error.msg[sizeof(data.error.msg) - 1] = '\0'; + dev_warn(edev->dev, "Msg: Error. type=%u, msg=\"%s\"\n", + data.error.type, data.error.msg); + ret = -EBADMSG; + break; case ETHOSU_CORE_MSG_PING: dev_info(edev->dev, "Msg: Ping\n"); - ret = ethosu_mailbox_ping(&edev->mailbox); + ret = ethosu_mailbox_pong(&edev->mailbox); break; case ETHOSU_CORE_MSG_PONG: dev_info(edev->dev, "Msg: Pong\n"); break; case ETHOSU_CORE_MSG_INFERENCE_RSP: + if (header.length != sizeof(data.inf)) { + dev_warn(edev->dev, + "Msg: Inference response of incorrect size. size=%u, expected=%zu\n", header.length, + sizeof(data.inf)); + ret = -EBADMSG; + break; + } + dev_info(edev->dev, "Msg: Inference response. user_arg=0x%llx, ofm_count=%u, status=%u\n", data.inf.user_arg, data.inf.ofm_count, data.inf.status); ethosu_inference_rsp(edev, &data.inf); break; + case ETHOSU_CORE_MSG_VERSION_RSP: + if (header.length != sizeof(data.version)) { + dev_warn(edev->dev, + "Msg: Version response of incorrect size. size=%u, expected=%zu\n", header.length, + sizeof(data.version)); + ret = -EBADMSG; + break; + } + + dev_info(edev->dev, "Msg: Version response v%u.%u.%u\n", + data.version.major, data.version.minor, + data.version.patch); + + /* Check major and minor version match, else return error */ + if (data.version.major != ETHOSU_CORE_MSG_VERSION_MAJOR || + data.version.minor != ETHOSU_CORE_MSG_VERSION_MINOR) { + dev_warn(edev->dev, "Msg: Version mismatch detected! "); + dev_warn(edev->dev, "Local version: v%u.%u.%u\n", + ETHOSU_CORE_MSG_VERSION_MAJOR, + ETHOSU_CORE_MSG_VERSION_MINOR, + ETHOSU_CORE_MSG_VERSION_PATCH); + } + + break; + default: - dev_warn(edev->dev, - "Msg: Unsupported msg type. type=%u, length=%u", - header.type, header.length); + /* This should not happen due to version checks */ + dev_warn(edev->dev, "Msg: Protocol error\n"); + ret = -EPROTO; break; } @@ -122,6 +172,10 @@ static long ethosu_ioctl(struct file *file, dev_info(edev->dev, "Ioctl. cmd=%u, arg=%lu\n", cmd, arg); switch (cmd) { + case ETHOSU_IOCTL_VERSION_REQ: + dev_info(edev->dev, "Ioctl: Send version request\n"); + ret = ethosu_mailbox_version_request(&edev->mailbox); + break; case ETHOSU_IOCTL_PING: { dev_info(edev->dev, "Ioctl: Send ping\n"); ret = ethosu_mailbox_ping(&edev->mailbox); @@ -173,6 +227,11 @@ static void ethosu_mbox_rx(void *user_arg) do { ret = ethosu_handle_msg(edev); + if (ret && ret != -ENOMSG) + /* Need to start over in case of error, empty the queue + * by fast-forwarding read position to write position. + * */ + ethosu_mailbox_reset(&edev->mailbox); } while (ret == 0); mutex_unlock(&edev->mutex); diff --git a/kernel/ethosu_mailbox.c b/kernel/ethosu_mailbox.c index 77b9614..f61c181 100644 --- a/kernel/ethosu_mailbox.c +++ b/kernel/ethosu_mailbox.c @@ -105,7 +105,10 @@ static int ethosu_queue_write_msg(struct ethosu_mailbox *mbox, void *data, size_t length) { - struct ethosu_core_msg msg = { .type = type, .length = length }; + struct ethosu_core_msg msg = { + .magic = ETHOSU_CORE_MSG_MAGIC, + .type = type, .length= length + }; const struct kvec vec[2] = { { &msg, sizeof(msg) }, { data, length } @@ -123,9 +126,12 @@ static int ethosu_queue_read(struct ethosu_mailbox *mbox, uint8_t *dst = (uint8_t *)data; const uint8_t *end = dst + length; uint32_t rpos = queue->header.read; + size_t queue_avail = ethosu_queue_available(queue); - if (length > ethosu_queue_available(queue)) + if (queue_avail == 0) return -ENOMSG; + else if (length > queue_avail) + return -EBADMSG; while (dst < end) { *dst++ = src[rpos]; @@ -137,6 +143,11 @@ static int ethosu_queue_read(struct ethosu_mailbox *mbox, return 0; } +void ethosu_mailbox_reset(struct ethosu_mailbox *mbox) +{ + mbox->out_queue->header.read = mbox->out_queue->header.write; +} + int ethosu_mailbox_read(struct ethosu_mailbox *mbox, struct ethosu_core_msg *header, void *data, @@ -144,22 +155,44 @@ int ethosu_mailbox_read(struct ethosu_mailbox *mbox, { int ret; - /* Read message header */ + /* Read message header magic */ ret = ethosu_queue_read(mbox, header, sizeof(*header)); - if (ret) + if (ret) { + if (ret != -ENOMSG) + dev_warn(mbox->dev, + "Msg: Failed to read message header\n"); + return ret; + } + + if (header->magic != ETHOSU_CORE_MSG_MAGIC) { + dev_warn(mbox->dev, + "Msg: Invalid magic. Got: %08X but expected %08X\n", + header->magic, ETHOSU_CORE_MSG_MAGIC); + + return -EINVAL; + } - dev_info(mbox->dev, "mbox: Read msg header. type=%u, length=%u", - header->type, header->length); + dev_info(mbox->dev, + "mbox: Read msg header. magic=%08X, type=%u, length=%u", + header->magic, header->type, header->length); /* Check that payload is not larger than allocated buffer */ - if (header->length > length) + if (header->length > length) { + dev_warn(mbox->dev, + "Msg: Buffer size (%zu) too small for message (%u)\n", + sizeof(data), header->length); + return -ENOMEM; + } - /* Ready payload data */ + /* Read payload data */ ret = ethosu_queue_read(mbox, data, header->length); - if (ret) + if (ret) { + dev_warn(mbox->dev, "Msg: Failed to read payload data\n"); + return -EBADMSG; + } return 0; } @@ -169,6 +202,17 @@ int ethosu_mailbox_ping(struct ethosu_mailbox *mbox) return ethosu_queue_write_msg(mbox, ETHOSU_CORE_MSG_PING, NULL, 0); } +int ethosu_mailbox_pong(struct ethosu_mailbox *mbox) +{ + return ethosu_queue_write_msg(mbox, ETHOSU_CORE_MSG_PONG, NULL, 0); +} + +int ethosu_mailbox_version_request(struct ethosu_mailbox *mbox) +{ + return ethosu_queue_write_msg(mbox, ETHOSU_CORE_MSG_VERSION_REQ, NULL, + 0); +} + int ethosu_mailbox_inference(struct ethosu_mailbox *mbox, void *user_arg, uint32_t ifm_count, diff --git a/kernel/ethosu_mailbox.h b/kernel/ethosu_mailbox.h index 8f539ee..0bc5ffb 100644 --- a/kernel/ethosu_mailbox.h +++ b/kernel/ethosu_mailbox.h @@ -24,6 +24,7 @@ /**************************************************************************** * Includes ****************************************************************************/ +#include "ethosu_core_interface.h" #include #include @@ -86,6 +87,11 @@ int ethosu_mailbox_read(struct ethosu_mailbox *mbox, void *data, size_t length); +/** + * ethosu_mailbox_reset() - Reset to end of queue + */ +void ethosu_mailbox_reset(struct ethosu_mailbox *mbox); + /** * ethosu_mailbox_ping() - Send ping message * @@ -93,6 +99,20 @@ int ethosu_mailbox_read(struct ethosu_mailbox *mbox, */ int ethosu_mailbox_ping(struct ethosu_mailbox *mbox); +/** + * ethosu_mailbox_pong() - Send pong response + * + * Return: 0 on success, else error code. + */ +int ethosu_mailbox_pong(struct ethosu_mailbox *mbox); + +/** + * ethosu_mailbox_version_response - Send version request + * + * Return: 0 on succes, else error code + */ +int ethosu_mailbox_version_request(struct ethosu_mailbox *mbox); + /** * ethosu_mailbox_inference() - Send inference * diff --git a/kernel/uapi/ethosu.h b/kernel/uapi/ethosu.h index 8f870c9..0127033 100644 --- a/kernel/uapi/ethosu.h +++ b/kernel/uapi/ethosu.h @@ -39,6 +39,7 @@ #define ETHOSU_IOWR(nr, type) _IOWR(ETHOSU_IOCTL_BASE, nr, type) #define ETHOSU_IOCTL_PING ETHOSU_IO(0x00) +#define ETHOSU_IOCTL_VERSION_REQ ETHOSU_IO(0x01) #define ETHOSU_IOCTL_BUFFER_CREATE ETHOSU_IOR(0x10, \ struct ethosu_uapi_buffer_create) #define ETHOSU_IOCTL_BUFFER_SET ETHOSU_IOR(0x11, \ diff --git a/utils/inference_runner/inference_runner.cpp b/utils/inference_runner/inference_runner.cpp index b9707a9..e8e6ec8 100644 --- a/utils/inference_runner/inference_runner.cpp +++ b/utils/inference_runner/inference_runner.cpp @@ -206,8 +206,8 @@ int main(int argc, char *argv[]) { try { Device device; - cout << "Send ping" << endl; - device.ioctl(ETHOSU_IOCTL_PING); + cout << "Send version request" << endl; + device.ioctl(ETHOSU_IOCTL_VERSION_REQ); /* Create network */ cout << "Create network" << endl; -- cgit v1.2.1