Fixes for Ibex Wrapper, DMA, SPI and mailboxes

hw/opentitan/ot_common.c

@@ -142,3 +142,12 @@ Chardev *ot_common_get_chardev_by_id(const char *chrid)
 
     return match.chr;
 }
+
+int ot_common_string_ends_with(const char *str, const char *suffix)
+{
+    size_t str_len = strlen(str);
+    size_t suffix_len = strlen(suffix);
+
+    return (str_len >= suffix_len) &&
+           (!memcmp(str + str_len - suffix_len, suffix, suffix_len));
+}

hw/opentitan/ot_dev_proxy.c

@@ -187,7 +187,7 @@ enum OtDevProxyErr {
 #define MEMTXATTRS_WITH_ROLE(_r_) \
     (MemTxAttrs) \
     { \
-        .role = _r_ \
+        .role = (_r_) \
     }
 #define MEMTXATTRS_GET_ROLE(_a_) ((_a_).unspecified ? 0xfu : (_a_).role);
 #else

hw/opentitan/ot_dma.c

@@ -163,6 +163,12 @@ REG32(INT_SRC_VAL_8, 0xecu)
 REG32(INT_SRC_VAL_9, 0xf0u)
 REG32(INT_SRC_VAL_10, 0xf4u)
 
+#if defined(MEMTXATTRS_HAS_ROLE) && (MEMTXATTRS_HAS_ROLE != 0)
+#define OT_DMA_HAS_ROLE
+#else
+#undef OT_DMA_HAS_ROLE
+#endif
+
 typedef enum {
     TRANSACTION_WIDTH_BYTE = 0x0,
     TRANSACTION_WIDTH_HALF = 0x1,
@@ -244,6 +250,9 @@ struct OtDMAState {
     char *ctn_as_name; /* externel port AS unique name */
     char *sys_as_name; /* external system AS unique name */
     char *dma_id;
+#ifdef OT_DMA_HAS_ROLE
+    uint8_t role;
+#endif
 };
 
 #define R32_OFF(_r_) ((_r_) / sizeof(uint32_t))
@@ -633,6 +642,9 @@ static bool ot_dma_go(OtDMAState *s)
     OtDMAOp *op = &s->op;
 
     op->attrs.unspecified = false;
+#ifdef OT_DMA_HAS_ROLE
+    op->attrs.role = (unsigned)s->role;
+#endif
     op->size = s->regs[R_TOTAL_DATA_SIZE];
 
     /*
@@ -1054,6 +1066,9 @@ static Property ot_dma_properties[] = {
     DEFINE_PROP_STRING("ot_as_name", OtDMAState, ot_as_name),
     DEFINE_PROP_STRING("ctn_as_name", OtDMAState, ctn_as_name),
     DEFINE_PROP_STRING("sys_as_name", OtDMAState, sys_as_name),
+#ifdef OT_DMA_HAS_ROLE
+    DEFINE_PROP_UINT8("role", OtDMAState, role, UINT8_MAX),
+#endif
     DEFINE_PROP_STRING("id", OtDMAState, dma_id),
     DEFINE_PROP_END_OF_LIST(),
 };

hw/opentitan/ot_ibex_wrapper_darjeeling.c

@@ -696,6 +696,7 @@ struct OtIbexWrapperDarjeelingState {
 
     MemoryRegion mmio;
     MemoryRegion remappers[PARAM_NUM_REGIONS];
+    MemoryRegion *sys_mem;
 
     uint32_t *regs;
     OtIbexTestLogEngine *log_engine;
@@ -722,23 +723,65 @@ ot_ibex_wrapper_dj_remapper_destroy(OtIbexWrapperDjState *s, unsigned slot)
         memory_region_transaction_begin();
         memory_region_set_enabled(mr, false);
         /* QEMU memory model enables unparenting alias regions */
-        MemoryRegion *sys_mem = get_system_memory();
-        memory_region_del_subregion(sys_mem, mr);
+        memory_region_del_subregion(s->sys_mem, mr);
         memory_region_transaction_commit();
     }
 }
 
+/* NOLINTNEXTLINE */
+static bool ot_ibex_wrapper_dj_mr_map_offset(
+    hwaddr *offset, const MemoryRegion *root, hwaddr dst, size_t size,
+    const MemoryRegion *tmr)
+{
+    if (root == tmr) {
+        return true;
+    }
+
+    const MemoryRegion *mr;
+
+    QTAILQ_FOREACH(mr, &root->subregions, subregions_link) {
+        if (dst < mr->addr ||
+            (dst + size) > (mr->addr + int128_getlo(mr->size))) {
+            continue;
+        }
+
+        bool ret;
+
+        if (mr->alias) {
+            hwaddr alias_offset = mr->addr - mr->alias_offset;
+            dst -= alias_offset;
+
+            ret = ot_ibex_wrapper_dj_mr_map_offset(offset, mr->alias, dst, size,
+                                                   tmr);
+            if (ret) {
+                /*
+                 * the selected MR tree leads to the target region, so update
+                 * the alias offset with the local offset
+                 */
+                *offset += alias_offset;
+            }
+        } else {
+            ret = ot_ibex_wrapper_dj_mr_map_offset(offset, mr, dst, size, tmr);
+            if (ret) {
+                *offset += mr->addr;
+            }
+        }
+
+        return ret;
+    }
+
+    return false;
+}
+
 static void ot_ibex_wrapper_dj_remapper_create(
     OtIbexWrapperDjState *s, unsigned slot, hwaddr dst, hwaddr src, size_t size)
 {
     g_assert(slot < PARAM_NUM_REGIONS);
     MemoryRegion *mr = &s->remappers[slot];
-    trace_ot_ibex_wrapper_map(slot, src, dst, size);
     g_assert(!memory_region_is_mapped(mr));
 
     int priority = (int)(PARAM_NUM_REGIONS - slot);
 
-    MemoryRegion *sys_mem = get_system_memory();
     MemoryRegion *mr_dst;
 
     char *name =
@@ -750,13 +793,25 @@ static void ot_ibex_wrapper_dj_remapper_create(
      * map on the whole address space.
      */
     MemoryRegionSection mrs;
-    mrs = memory_region_find(sys_mem, dst, (uint64_t)size);
+    mrs = memory_region_find(s->sys_mem, dst, (uint64_t)size);
     size_t mrs_lsize = int128_getlo(mrs.size);
-    mr_dst = (mrs.mr && mrs_lsize >= size) ? mrs.mr : sys_mem;
-    hwaddr offset = dst - mr_dst->addr;
+    mr_dst = (mrs.mr && mrs_lsize >= size) ? mrs.mr : s->sys_mem;
+
+    /*
+     * adjust the offset if the memory region target for the mapping
+     * is itself mapped through memory region(s)
+     */
+    hwaddr offset = 0;
+    if (ot_ibex_wrapper_dj_mr_map_offset(&offset, s->sys_mem, dst, size,
+                                         mr_dst)) {
+        offset = dst - offset;
+    }
+
+    trace_ot_ibex_wrapper_map(slot, src, dst, size, mr_dst->name,
+                              (uint32_t)offset);
     memory_region_init_alias(mr, OBJECT(s), name, mr_dst, offset,
                              (uint64_t)size);
-    memory_region_add_subregion_overlap(sys_mem, src, mr, priority);
+    memory_region_add_subregion_overlap(s->sys_mem, src, mr, priority);
     memory_region_set_enabled(mr, true);
     memory_region_transaction_commit();
     g_free(name);
@@ -1347,6 +1402,8 @@ static void ot_ibex_wrapper_dj_reset(DeviceState *dev)
 {
     OtIbexWrapperDjState *s = OT_IBEX_WRAPPER_DARJEELING(dev);
 
+    g_assert(s->sys_mem);
+
     for (unsigned slot = 0; slot < PARAM_NUM_REGIONS; slot++) {
         ot_ibex_wrapper_dj_remapper_destroy(s, slot);
     }
@@ -1365,6 +1422,14 @@ static void ot_ibex_wrapper_dj_reset(DeviceState *dev)
     s->log_engine->as = ot_common_get_local_address_space(dev);
 }
 
+static void ot_ibex_wrapper_dj_realize(DeviceState *dev, Error **errp)
+{
+    OtIbexWrapperDjState *s = OT_IBEX_WRAPPER_DARJEELING(dev);
+    (void)errp;
+
+    s->sys_mem = ot_common_get_local_address_space(dev)->root;
+}
+
 static void ot_ibex_wrapper_dj_init(Object *obj)
 {
     OtIbexWrapperDjState *s = OT_IBEX_WRAPPER_DARJEELING(obj);
@@ -1383,6 +1448,7 @@ static void ot_ibex_wrapper_dj_class_init(ObjectClass *klass, void *data)
     (void)data;
 
     dc->reset = &ot_ibex_wrapper_dj_reset;
+    dc->realize = &ot_ibex_wrapper_dj_realize;
     device_class_set_props(dc, ot_ibex_wrapper_dj_properties);
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
 }

hw/opentitan/ot_ibex_wrapper_earlgrey.c

@@ -252,7 +252,6 @@ static void ot_ibex_wrapper_eg_remapper_create(
 {
     g_assert(slot < PARAM_NUM_REGIONS);
     MemoryRegion *mr = &s->remappers[slot];
-    trace_ot_ibex_wrapper_map(slot, src, dst, size);
     g_assert(!memory_region_is_mapped(mr));
 
     int priority = (int)(PARAM_NUM_REGIONS - slot);
@@ -273,6 +272,8 @@ static void ot_ibex_wrapper_eg_remapper_create(
     size_t mrs_lsize = int128_getlo(mrs.size);
     mr_dst = (mrs.mr && mrs_lsize >= size) ? mrs.mr : sys_mem;
     hwaddr offset = dst - mr_dst->addr;
+    trace_ot_ibex_wrapper_map(slot, src, dst, size, mr_dst->name,
+                              (uint32_t)offset);
     memory_region_init_alias(mr, OBJECT(s), name, mr_dst, offset,
                              (uint64_t)size);
     memory_region_add_subregion_overlap(sys_mem, src, mr, priority);

hw/opentitan/ot_mbx.c

@@ -60,6 +60,7 @@
 REG32(HOST_INTR_STATE, 0x00u)
     SHARED_FIELD(INTR_MBX_READY, 0u, 1u)
     SHARED_FIELD(INTR_MBX_ABORT, 1u, 1u)
+    SHARED_FIELD(INTR_MBX_ERROR, 2u, 1u)
 REG32(HOST_INTR_ENABLE, 0x04u)
 REG32(HOST_INTR_TEST, 0x08u)
 REG32(HOST_ALERT_TEST, 0x0cu)
@@ -123,13 +124,15 @@ REG32(SYS_READ_DATA, 0x14u)
 #define REGS_SYSLOCAL_COUNT (R_SYSLOCAL_LAST_REG + 1u)
 #define REGS_SYSLOCAL_SIZE  (REGS_SYSLOCAL_COUNT * sizeof(uint32_t))
 
-#define HOST_INTR_MASK  (INTR_MBX_READY_MASK | INTR_MBX_ABORT_MASK)
+#define HOST_INTR_MASK \
+    (INTR_MBX_READY_MASK | INTR_MBX_ABORT_MASK | INTR_MBX_ERROR_MASK)
 #define HOST_INTR_COUNT (HOST_INTR_MASK - 1u)
 #define HOST_ALERT_TEST_MASK \
     (R_HOST_ALERT_TEST_FATAL_FAULT_MASK | R_HOST_ALERT_TEST_RECOV_FAULT_MASK)
 #define HOST_CONTROL_MASK \
     (R_HOST_CONTROL_ABORT_MASK | R_HOST_CONTROL_ABORT_MASK)
 
+static_assert(OT_MBX_HOST_REGS_COUNT == REGS_HOST_COUNT, "Invalid HOST regs");
 static_assert(OT_MBX_SYS_REGS_COUNT == REGS_SYS_COUNT, "Invalid SYS regs");
 
 #define REG_NAME(_kind_, _reg_) \
@@ -240,6 +243,36 @@ static bool ot_mbx_is_on_abort(const OtMbxState *s)
     return (bool)(s->host.regs[R_HOST_CONTROL] & R_HOST_CONTROL_ABORT_MASK);
 }
 
+static bool ot_mbx_is_sys_interrupt(const OtMbxState *s)
+{
+    return (bool)(s->host.regs[R_HOST_STATUS] &
+                  R_HOST_STATUS_SYS_INTR_STATE_MASK);
+}
+
+static void ot_mbx_set_error(OtMbxState *s)
+{
+    uint32_t *hregs = s->host.regs;
+
+    /* should busy be set? */
+    hregs[R_HOST_CONTROL] |= R_HOST_CONTROL_ERROR_MASK;
+
+    if (hregs[R_HOST_STATUS] & R_HOST_STATUS_SYS_INTR_ENABLE_MASK) {
+        hregs[R_HOST_STATUS] |= R_HOST_STATUS_SYS_INTR_STATE_MASK;
+    }
+
+    /*
+     * Note: you should not use this interrupt, as it might create
+     * hard-to-manage signalling since IRQ might be raise at unexpected time
+     * in mailbox management. You've been warned.
+     *
+     * On error, wait for GO bit to be set, then handle any HW error at this
+     * point. If the SYS side detects the error bit before it sets the GO flag
+     * it can immediately trigger an abort.
+     */
+    hregs[R_HOST_INTR_STATE] |= INTR_MBX_ERROR_MASK;
+    ot_mbx_host_update_irqs(s);
+}
+
 static void ot_mbx_clear_busy(OtMbxState *s)
 {
     uint32_t *hregs = s->host.regs;
@@ -345,7 +378,9 @@ static void ot_mbx_host_regs_write(void *opaque, hwaddr addr, uint64_t val64,
             ot_mbx_clear_busy(s);
             hregs[reg] &= ~R_HOST_CONTROL_ABORT_MASK; /* RW1C */
         }
-        hregs[reg] |= val32 & R_HOST_CONTROL_ERROR_MASK; /* RW1S */
+        if (val32 & R_HOST_CONTROL_ERROR_MASK) { /* RW1S */
+            ot_mbx_set_error(s);
+        };
         xtrace_ot_mbx_status(s);
         break;
     case R_HOST_STATUS:
@@ -409,7 +444,6 @@ static void ot_mbx_host_regs_write(void *opaque, hwaddr addr, uint64_t val64,
 
             if (hregs[R_HOST_STATUS] & R_HOST_STATUS_SYS_INTR_ENABLE_MASK) {
                 hregs[R_HOST_STATUS] |= R_HOST_STATUS_SYS_INTR_STATE_MASK;
-                // placeholder: should trigger system interrupt from here
             }
         }
         xtrace_ot_mbx_status(s);
@@ -474,19 +508,6 @@ static void ot_mbx_sys_go(OtMbxState *s)
     }
 }
 
-static void ot_mbx_sys_set_error(OtMbxState *s)
-{
-    uint32_t *hregs = s->host.regs;
-
-    // should busy be set?
-    hregs[R_HOST_CONTROL] |= R_HOST_CONTROL_ERROR_MASK;
-
-    if (hregs[R_HOST_STATUS] & R_HOST_STATUS_SYS_INTR_ENABLE_MASK) {
-        hregs[R_HOST_STATUS] |= R_HOST_STATUS_SYS_INTR_STATE_MASK;
-        // placeholder: should trigger system interrupt from here
-    }
-}
-
 static MemTxResult ot_mbx_sys_regs_read_with_attrs(
     void *opaque, hwaddr addr, uint64_t *val64, unsigned size, MemTxAttrs attrs)
 {
@@ -513,7 +534,8 @@ static MemTxResult ot_mbx_sys_regs_read_with_attrs(
         break;
     case R_SYS_STATUS:
         val32 = FIELD_DP32(0, SYS_STATUS, BUSY, (uint32_t)ot_mbx_is_busy(s));
-        val32 = FIELD_DP32(val32, SYS_STATUS, INT, (uint32_t)ot_mbx_is_busy(s));
+        val32 = FIELD_DP32(val32, SYS_STATUS, INT,
+                           (uint32_t)ot_mbx_is_sys_interrupt(s));
         val32 = FIELD_DP32(val32, SYS_STATUS, ERROR,
                            (uint32_t)ot_mbx_is_on_error(s));
         val32 = FIELD_DP32(val32, SYS_STATUS, READY,
@@ -639,7 +661,7 @@ static MemTxResult ot_mbx_sys_regs_write_with_attrs(
         if (hregs[R_HOST_IN_WRITE_PTR] >= hregs[R_HOST_IN_LIMIT_ADDR]) {
             qemu_log_mask(LOG_GUEST_ERROR, "%s: %s write overflow\n", __func__,
                           s->mbx_id);
-            ot_mbx_sys_set_error(s);
+            ot_mbx_set_error(s);
             xtrace_ot_mbx_status(s);
         }
         hwaddr waddr = (hwaddr)hregs[R_HOST_IN_WRITE_PTR];
@@ -650,7 +672,7 @@ static MemTxResult ot_mbx_sys_regs_write_with_attrs(
             qemu_log_mask(LOG_GUEST_ERROR,
                           "%s: %s Cannot write @ 0x%" HWADDR_PRIx ": %u\n",
                           __func__, s->mbx_id, waddr, mres);
-            ot_mbx_sys_set_error(s);
+            ot_mbx_set_error(s);
             xtrace_ot_mbx_status(s);
             ibex_irq_set(&s->host.alerts[ALERT_RECOVERABLE], 1);
             break;