From 9ad14eca3ebd96fed6ccfab859b10fe5565880eb Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Fri, 1 Dec 2023 10:26:38 +0100 Subject: [PATCH 01/11] [ot] hw/opentitan: ot_ibex_wrapper: use Ibex private XBAR for remapper Signed-off-by: Emmanuel Blot --- hw/opentitan/ot_ibex_wrapper_darjeeling.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/hw/opentitan/ot_ibex_wrapper_darjeeling.c b/hw/opentitan/ot_ibex_wrapper_darjeeling.c index 1eb95c4e4cfa..0e83b2fbc543 100644 --- a/hw/opentitan/ot_ibex_wrapper_darjeeling.c +++ b/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,8 +723,7 @@ 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(); } } @@ -738,7 +738,6 @@ static void ot_ibex_wrapper_dj_remapper_create( int priority = (int)(PARAM_NUM_REGIONS - slot); - MemoryRegion *sys_mem = get_system_memory(); MemoryRegion *mr_dst; char *name = @@ -750,13 +749,13 @@ 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; + mr_dst = (mrs.mr && mrs_lsize >= size) ? mrs.mr : s->sys_mem; hwaddr offset = dst - mr_dst->addr; 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 +1346,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 +1366,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 +1392,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); } From 2aefebf1670e7d3dd77ca59070d21d49ff83a744 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Fri, 1 Dec 2023 15:23:12 +0100 Subject: [PATCH 02/11] [ot] hw/opentitan: ot_ibex_wrapper: manage aliased memory region targets Signed-off-by: Emmanuel Blot --- hw/opentitan/ot_ibex_wrapper_darjeeling.c | 60 ++++++++++++++++++++++- hw/opentitan/ot_ibex_wrapper_earlgrey.c | 3 +- hw/opentitan/trace-events | 2 +- 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/hw/opentitan/ot_ibex_wrapper_darjeeling.c b/hw/opentitan/ot_ibex_wrapper_darjeeling.c index 0e83b2fbc543..a6c231729c60 100644 --- a/hw/opentitan/ot_ibex_wrapper_darjeeling.c +++ b/hw/opentitan/ot_ibex_wrapper_darjeeling.c @@ -728,12 +728,56 @@ ot_ibex_wrapper_dj_remapper_destroy(OtIbexWrapperDjState *s, unsigned slot) } } +/* 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); @@ -752,7 +796,19 @@ static void ot_ibex_wrapper_dj_remapper_create( 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 : s->sys_mem; - hwaddr offset = dst - mr_dst->addr; + + /* + * 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(s->sys_mem, src, mr, priority); diff --git a/hw/opentitan/ot_ibex_wrapper_earlgrey.c b/hw/opentitan/ot_ibex_wrapper_earlgrey.c index d19c28788ffd..84bb408d99ab 100644 --- a/hw/opentitan/ot_ibex_wrapper_earlgrey.c +++ b/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); diff --git a/hw/opentitan/trace-events b/hw/opentitan/trace-events index e89a2c09abc4..953ab77c4003 100644 --- a/hw/opentitan/trace-events +++ b/hw/opentitan/trace-events @@ -169,7 +169,7 @@ ot_hmac_debug(const char *msg) "%s" ot_ibex_wrapper_io_read_out(unsigned int addr, const char * regname, uint32_t val, uint64_t pc) "addr=0x%02x (%s), val=0x%08x, pc=0x%" PRIx64 ot_ibex_wrapper_io_write(unsigned int addr, const char * regname, uint32_t val, uint64_t pc) "addr=0x%02x (%s), val=0x%08x, pc=0x%" PRIx64 ot_ibex_wrapper_info(const char *func, int line, const char *msg) "%s:%d %s" -ot_ibex_wrapper_map(unsigned slot, uint32_t src, uint32_t dst, uint32_t size) "region %u from 0x%08x to 0x%08x on 0x%x bytes" +ot_ibex_wrapper_map(unsigned slot, uint32_t src, uint32_t dst, uint32_t size, const char *name, uint32_t offset) "region %u from 0x%08x to 0x%08x on 0x%x bytes (%s), off 0x%x" ot_ibex_wrapper_fill_entropy(uint32_t bits, bool fips) "0x%08" PRIx32 " fips:%u" ot_ibex_wrapper_request_entropy(bool again) "%u" ot_ibex_wrapper_unmap(unsigned slot) "region %u" From ffff962405dc97dcedf2e4662d351c8fe05c1d10 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Fri, 1 Dec 2023 16:15:24 +0100 Subject: [PATCH 03/11] [ot] hw/opentitan: ot_common: add an string utility function (strrcmp) Signed-off-by: Emmanuel Blot --- hw/opentitan/ot_common.c | 9 +++++++++ include/hw/opentitan/ot_common.h | 6 ++++++ 2 files changed, 15 insertions(+) diff --git a/hw/opentitan/ot_common.c b/hw/opentitan/ot_common.c index e86a66cb9599..f0d19fb0a511 100644 --- a/hw/opentitan/ot_common.c +++ b/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)); +} diff --git a/include/hw/opentitan/ot_common.h b/include/hw/opentitan/ot_common.h index 20af1fbab831..572a71e47761 100644 --- a/include/hw/opentitan/ot_common.h +++ b/include/hw/opentitan/ot_common.h @@ -157,4 +157,10 @@ void ot_common_ignore_chr_status_lines(CharBackend *chr); */ Chardev *ot_common_get_chardev_by_id(const char *chrid); +/* ------------------------------------------------------------------------ */ +/* String utilities */ +/* ------------------------------------------------------------------------ */ + +int ot_common_string_ends_with(const char *str, const char *suffix); + #endif /* HW_OPENTITAN_OT_COMMON_H */ From 772e51cf047514fa7bb92f697fed24ad7b7d52b6 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Fri, 1 Dec 2023 15:19:51 +0100 Subject: [PATCH 04/11] [ot] hw/opentitan: spi_host: ensure qemu_log use __func__ and EOL char Signed-off-by: Emmanuel Blot --- hw/opentitan/ot_spi_host.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/hw/opentitan/ot_spi_host.c b/hw/opentitan/ot_spi_host.c index 100876192596..b763539a27b5 100644 --- a/hw/opentitan/ot_spi_host.c +++ b/hw/opentitan/ot_spi_host.c @@ -767,8 +767,8 @@ static void ot_spi_host_step_fsm(OtSPIHostState *s, const char *cause) if (!(read || write)) { /* dummy mode uses clock cycle count rather than byte count */ if (length % 8u) { - qemu_log_mask(LOG_UNIMP, "Unsupported clock cycle count: %u\n", - length); + qemu_log_mask(LOG_UNIMP, "%s: unsupported clock cycle count: %u\n", + __func__, length); } length = DIV_ROUND_UP(length, 8u); } @@ -922,7 +922,7 @@ static uint64_t ot_spi_host_read(void *opaque, hwaddr addr, unsigned int size) uint32_t val32; if (s->on_reset) { - qemu_log_mask(LOG_GUEST_ERROR, "device in reset"); + qemu_log_mask(LOG_GUEST_ERROR, "%s: device in reset\n", __func__); return 0u; } @@ -934,8 +934,8 @@ static uint64_t ot_spi_host_read(void *opaque, hwaddr addr, unsigned int size) case R_COMMAND: case R_TXDATA: qemu_log_mask(LOG_GUEST_ERROR, - "W/O register 0x%02" HWADDR_PRIx " (%s)\n", addr, - REG_NAME(reg)); + "%s: W/O register 0x%02" HWADDR_PRIx " (%s)\n", __func__, + addr, REG_NAME(reg)); val32 = 0u; break; case R_INTR_STATE: @@ -986,7 +986,8 @@ static uint64_t ot_spi_host_read(void *opaque, hwaddr addr, unsigned int size) } default: val32 = 0u; - qemu_log_mask(LOG_GUEST_ERROR, "Bad offset 0x%" HWADDR_PRIx "\n", addr); + qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n", + __func__, addr); } uint64_t pc = ibex_get_current_pc(); @@ -1029,7 +1030,7 @@ static void ot_spi_host_write(void *opaque, hwaddr addr, uint64_t val64, trace_ot_spi_host_write(addr, REG_NAME(reg), val64, pc); if (s->on_reset) { - qemu_log_mask(LOG_GUEST_ERROR, "device in reset"); + qemu_log_mask(LOG_GUEST_ERROR, "%s: device in reset\n", __func__); return; } @@ -1144,8 +1145,8 @@ static void ot_spi_host_write(void *opaque, hwaddr addr, uint64_t val64, } case R_RXDATA: qemu_log_mask(LOG_GUEST_ERROR, - "R/O register 0x%02" HWADDR_PRIx " (%s)\n", addr, - REG_NAME(reg)); + "%s: R/O register 0x%02" HWADDR_PRIx " (%s)\n", __func__, + addr, REG_NAME(reg)); break; case R_TXDATA: { /* @@ -1198,7 +1199,8 @@ static void ot_spi_host_write(void *opaque, hwaddr addr, uint64_t val64, ot_spi_host_update_event(s); break; default: - qemu_log_mask(LOG_GUEST_ERROR, "Bad offset 0x%" HWADDR_PRIx "\n", addr); + qemu_log_mask(LOG_GUEST_ERROR, "%s: bad offset 0x%" HWADDR_PRIx "\n", + __func__, addr); break; } } From 80d283c645f656c2258d6b18cc22d410ff535df8 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Mon, 4 Dec 2023 11:08:33 +0100 Subject: [PATCH 05/11] [ot] hw/opentitan: spi_host: fix warning for dual/quad clock cycle count Signed-off-by: Emmanuel Blot --- hw/opentitan/ot_spi_host.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/hw/opentitan/ot_spi_host.c b/hw/opentitan/ot_spi_host.c index b763539a27b5..d278d4811f3a 100644 --- a/hw/opentitan/ot_spi_host.c +++ b/hw/opentitan/ot_spi_host.c @@ -762,13 +762,15 @@ static void ot_spi_host_step_fsm(OtSPIHostState *s, const char *cause) uint32_t command = (uint32_t)headcmd->command; bool read = ot_spi_host_is_rx(command); bool write = ot_spi_host_is_tx(command); - bool multi = FIELD_EX32(command, COMMAND, SPEED) != 0; + unsigned speed = FIELD_EX32(command, COMMAND, SPEED); + bool multi = speed != 0; uint32_t length = FIELD_EX32(command, COMMAND, LEN) + 1u; if (!(read || write)) { /* dummy mode uses clock cycle count rather than byte count */ - if (length % 8u) { - qemu_log_mask(LOG_UNIMP, "%s: unsupported clock cycle count: %u\n", - __func__, length); + if (length % (1u << (3u - speed))) { + qemu_log_mask(LOG_UNIMP, + "%s: unsupported clk cycle count: %u for speed %u\n", + __func__, length, speed); } length = DIV_ROUND_UP(length, 8u); } From b7433aea44d73bb90ba98ec0f9cfd2d008d6e9a8 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Mon, 4 Dec 2023 16:42:45 +0100 Subject: [PATCH 06/11] [ot] hw/riscv: ot_darjeeling: align IRQ map with current RTL Signed-off-by: Emmanuel Blot --- hw/riscv/ot_darjeeling.c | 47 +++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/hw/riscv/ot_darjeeling.c b/hw/riscv/ot_darjeeling.c index e72588e7f6ef..30ad039df923 100644 --- a/hw/riscv/ot_darjeeling.c +++ b/hw/riscv/ot_darjeeling.c @@ -41,7 +41,6 @@ #include "hw/opentitan/ot_csrng.h" #include "hw/opentitan/ot_edn.h" #include "hw/opentitan/ot_entropy_src.h" -#include "hw/opentitan/ot_flash.h" #include "hw/opentitan/ot_gpio.h" #include "hw/opentitan/ot_hmac.h" #include "hw/opentitan/ot_ibex_wrapper_darjeeling.h" @@ -231,9 +230,9 @@ static const IbexDeviceDef ot_darjeeling_soc_devices[] = { { 0x21110000u, 0x1000u } ), .gpio = IBEXGPIOCONNDEFS( - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 114), - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(1, PLIC, 115), - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(2, PLIC, 116), + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 115), + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(1, PLIC, 116), + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(2, PLIC, 117), OT_DARJEELING_SOC_CLKMGR_HINT(OT_CLKMGR_HINT_HMAC) ), }, @@ -243,9 +242,9 @@ static const IbexDeviceDef ot_darjeeling_soc_devices[] = { { 0x21120000u, 0x1000u } ), .gpio = IBEXGPIOCONNDEFS( - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 117), - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(1, PLIC, 118), - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(2, PLIC, 119) + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 118), + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(1, PLIC, 119), + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(2, PLIC, 120) ), .link = IBEXDEVICELINKDEFS( OT_DARJEELING_SOC_DEVLINK("edn", EDN0) @@ -261,7 +260,7 @@ static const IbexDeviceDef ot_darjeeling_soc_devices[] = { { 0x21130000u, 0x10000u } ), .gpio = IBEXGPIOCONNDEFS( - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 120), + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 121), OT_DARJEELING_SOC_CLKMGR_HINT(OT_CLKMGR_HINT_OTBN) ), .link = IBEXDEVICELINKDEFS( @@ -287,10 +286,10 @@ static const IbexDeviceDef ot_darjeeling_soc_devices[] = { { 0x21150000u, 0x1000u } ), .gpio = IBEXGPIOCONNDEFS( - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 122), - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(1, PLIC, 123), - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(2, PLIC, 124), - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(3, PLIC, 125) + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 123), + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(1, PLIC, 124), + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(2, PLIC, 125), + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(3, PLIC, 126) ), .link = IBEXDEVICELINKDEFS( OT_DARJEELING_SOC_DEVLINK("random_src", AST), @@ -304,8 +303,8 @@ static const IbexDeviceDef ot_darjeeling_soc_devices[] = { { 0x21170000u, 0x1000u } ), .gpio = IBEXGPIOCONNDEFS( - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 126), - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(1, PLIC, 127) + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 127), + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(1, PLIC, 128) ), .link = IBEXDEVICELINKDEFS( OT_DARJEELING_SOC_DEVLINK("csrng", CSRNG) @@ -321,8 +320,8 @@ static const IbexDeviceDef ot_darjeeling_soc_devices[] = { { 0x21180000u, 0x1000u } ), .gpio = IBEXGPIOCONNDEFS( - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 128), - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(1, PLIC, 129) + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 129), + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(1, PLIC, 130) ), .link = IBEXDEVICELINKDEFS( OT_DARJEELING_SOC_DEVLINK("csrng", CSRNG) @@ -543,7 +542,7 @@ static const IbexDeviceDef ot_darjeeling_soc_devices[] = { IBEX_DEV_STRING_PROP("hart-config", "M"), IBEX_DEV_UINT_PROP("hartid-base", 0u), /* note: should always be max_irq + 1 */ - IBEX_DEV_UINT_PROP("num-sources", 153u), + IBEX_DEV_UINT_PROP("num-sources", 164u), IBEX_DEV_UINT_PROP("num-priorities", 3u), IBEX_DEV_UINT_PROP("priority-base", 0x0u), IBEX_DEV_UINT_PROP("pending-base", 0x1000u), @@ -621,6 +620,10 @@ static const IbexDeviceDef ot_darjeeling_soc_devices[] = { .memmap = MEMMAPENTRIES( { 0x30020000u, 0x40u } ), + .gpio = IBEXGPIOCONNDEFS( + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 80), + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(1, PLIC, 81) + ) }, [OT_DARJEELING_SOC_DEV_I2C0] = { .type = TYPE_UNIMPLEMENTED_DEVICE, @@ -696,8 +699,8 @@ static const IbexDeviceDef ot_darjeeling_soc_devices[] = { { 0x30300000u, 0x1000u } ), .gpio = IBEXGPIOCONNDEFS( - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 75), - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(1, PLIC, 76) + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 76), + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(1, PLIC, 77) ), .prop = IBEXDEVICEPROPDEFS( IBEX_DEV_UINT_PROP("bus-num", 0) @@ -729,7 +732,7 @@ static const IbexDeviceDef ot_darjeeling_soc_devices[] = { { 0x30400000u, 0x1000u } ), .gpio = IBEXGPIOCONNDEFS( - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 77) + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 78) ), .link = IBEXDEVICELINKDEFS( OT_DARJEELING_SOC_DEVLINK("rstmgr", RSTMGR) @@ -766,8 +769,8 @@ static const IbexDeviceDef ot_darjeeling_soc_devices[] = { { 0x30470000u, 0x1000u } ), .gpio = IBEXGPIOCONNDEFS( - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 78), - OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(1, PLIC, 79), + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(0, PLIC, 79), + OT_DARJEELING_SOC_GPIO_SYSBUS_IRQ(1, PLIC, 80), OT_DARJEELING_SOC_SIGNAL(OPENTITAN_AON_TIMER_WKUP, 0, PWRMGR, \ OPENTITAN_PWRMGR_WKUP_REQ, \ OT_PWRMGR_WAKEUP_AON_TIMER), From 07b1fa259ec81467109bc974052aeac71e43e069 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Wed, 29 Nov 2023 17:10:17 +0100 Subject: [PATCH 07/11] [ot] hw/opentitan: ot_mbx fix a bug with SYS_INTR status System interrupt bit was not reflecting the actual interrupt status. Signed-off-by: Emmanuel Blot --- hw/opentitan/ot_mbx.c | 44 +++++++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/hw/opentitan/ot_mbx.c b/hw/opentitan/ot_mbx.c index 4d2e12603eab..72b65ce521ad 100644 --- a/hw/opentitan/ot_mbx.c +++ b/hw/opentitan/ot_mbx.c @@ -240,6 +240,25 @@ 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; + // placeholder: should trigger system interrupt from here + } +} + static void ot_mbx_clear_busy(OtMbxState *s) { uint32_t *hregs = s->host.regs; @@ -345,7 +364,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 +430,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 +494,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 +520,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 +647,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 +658,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; From 33466263f7a1ce261f413a2778af4b3e779b45af Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Mon, 4 Dec 2023 16:36:01 +0100 Subject: [PATCH 08/11] [ot] hw/opentitan: ot_mbx: add new mailbox interrupt for errors Signed-off-by: Emmanuel Blot --- hw/opentitan/ot_mbx.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/hw/opentitan/ot_mbx.c b/hw/opentitan/ot_mbx.c index 72b65ce521ad..6f795596ccfd 100644 --- a/hw/opentitan/ot_mbx.c +++ b/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_) \ @@ -250,13 +253,24 @@ static void ot_mbx_set_error(OtMbxState *s) { uint32_t *hregs = s->host.regs; - // should busy be set? + /* 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 } + + /* + * 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) From d5db22c350f21076ca187d69e204b8aa3fede415 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Mon, 4 Dec 2023 16:45:01 +0100 Subject: [PATCH 09/11] [ot] script/opentitan: devproxy: add mailbox interrupt for errors Signed-off-by: Emmanuel Blot --- scripts/opentitan/devproxy.py | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/scripts/opentitan/devproxy.py b/scripts/opentitan/devproxy.py index da0045573c62..d0aac99df077 100644 --- a/scripts/opentitan/devproxy.py +++ b/scripts/opentitan/devproxy.py @@ -182,7 +182,7 @@ class DeviceProxy: :paran regcount: count of 32-bit registers in the remote device """ - #pylint: disable=too-many-instance-attributes + # pylint: disable=too-many-instance-attributes MB4_TRUE = 0x6 """Multibit bool true value.""" @@ -195,7 +195,7 @@ class DeviceProxy: def __init__(self, proxy: 'Proxy', name: str, devid: int, addr: int, count: int, offset: int): - #pylint: disable=too-many-arguments + # pylint: disable=too-many-arguments logname = self.__class__.__name__.lower().replace('proxy', '') self._log = getLogger(f'proxy.{logname}') if offset > 0xffff: @@ -366,7 +366,7 @@ class MbxHostProxy(DeviceProxy): :param role: optional access role """ - #pylint: disable=too-many-public-methods + # pylint: disable=too-many-public-methods DEVICE_ID = 'mbh' @@ -394,6 +394,7 @@ class MbxHostProxy(DeviceProxy): IRQS = { 'MSGREADY': 0, 'ABORT': 1, + 'ERROR': 3, } """Supported IRQ channels.""" @@ -504,7 +505,7 @@ def is_object_ready(self) -> bool: :return: true if OBJECT READY bit is set """ res = (bool)(self.read_word(self._role, self.REGS['STATUS']) & - (1 << 31)) + (1 << 31)) self._log.debug('%d', res) return res @@ -684,7 +685,7 @@ def abort(self) -> None: def go(self) -> None: """Tell the mailbox responder that a request is reading to be processed. """ - #pylint: disable=invalid-name + # pylint: disable=invalid-name self._log.debug('') self.write_word(self._role, self.REGS['CONTROL'], 1 << 31, 1 << 31) @@ -945,7 +946,7 @@ class ProxyEngine: """Tool to access and remotely drive devices and memories. """ - #pylint: disable=too-many-instance-attributes + # pylint: disable=too-many-instance-attributes VERSION = (0, 12) """Protocol version.""" @@ -1035,7 +1036,7 @@ def resume(self) -> None: def discover_devices(self) -> None: """Enumerate and store supported remote devices. """ - #pylint: disable=too-many-locals + # pylint: disable=too-many-locals try: devices = self.exchange('ED') except ProxyCommandError as exc: @@ -1140,7 +1141,7 @@ def intercept_mmio_access(self, root: Optional[MemoryRoot], address: int, specified count of access. Use 0 to disable auto-discard :param prority: priority of the intercepter """ - #pylint: disable=too-many-arguments + # pylint: disable=too-many-arguments if not read and not write: raise ValueError('Read or/and Write should be specified') if not isinstance(priority, int) or not priority or priority > 0x3f: @@ -1244,7 +1245,7 @@ def _send(self, command: str, data: bytes): uid = (0 << 31) | (self._tx_uid & ~(1 << 31)) self._log.debug('TX cmd:%s, len:%d, uid:%d', command, len(data), uid) request = spack(self.HEADER, bytes(reversed(command.encode())), - len(data), uid) + len(data), uid) request = b''.join((request, data)) try: if self._port: @@ -1258,8 +1259,8 @@ def _send(self, command: str, data: bytes): def _receive(self): """Worker thread that handle data reception. """ - #pylint: disable=too-many-branches - #pylint: disable=too-many-statements + # pylint: disable=too-many-branches + # pylint: disable=too-many-statements buffer = bytearray() cmd = '' length = 0 @@ -1290,7 +1291,7 @@ def _receive(self): uid = uid & ~(1 << 31) cmd = bytes(reversed(bcmd)).decode() self._log.debug('RX cmd:%s, len:%d, uid:%d, resp:%d', - cmd, length, uid, resp) + cmd, length, uid, resp) buffer = buffer[self.HEADER_SIZE:] if len(buffer) < length: continue @@ -1311,7 +1312,7 @@ def _receive(self): length = 0 uid = 0 resp = False - #pylint: disable=broad-except + # pylint: disable=broad-except except Exception as exc: self._resume = False self._log.fatal('Exception: %s', exc) @@ -1340,7 +1341,7 @@ def _notify(self) -> None: continue self._request_handler(rcmd, payload, *self._request_args) - #pylint: disable=broad-except + # pylint: disable=broad-except except Exception as exc: self._log.fatal('Exception: %s', exc) break @@ -1428,7 +1429,7 @@ def _main(): proxy.connect('localhost', 8003) proxy.discover_devices() proxy.discover_memory_spaces() - #pylint: disable-msg=broad-except + # pylint: disable-msg=broad-except except Exception as exc: print(f'\nError: {exc}', file=stderr) if debug: @@ -1439,7 +1440,7 @@ def _main(): if __name__ == '__main__': - #pylint: disable=ungrouped-imports + # pylint: disable=ungrouped-imports from argparse import ArgumentParser from sys import exit as sysexit, stderr from traceback import format_exc From 95e3e36912cc0d6ece57f841576b42153a199abd Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Mon, 4 Dec 2023 18:23:40 +0100 Subject: [PATCH 10/11] [ot] hw/opentitan: ot_dma: add support for transaction roles Only available when MemTxAttrs is tainted with a role attribute. Signed-off-by: Emmanuel Blot --- hw/opentitan/ot_dma.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/hw/opentitan/ot_dma.c b/hw/opentitan/ot_dma.c index 94ac237e97d7..425d25a5aec0 100644 --- a/hw/opentitan/ot_dma.c +++ b/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(), }; From 7f5d74b73585d285b080a528a21c14731b3e02c6 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Mon, 4 Dec 2023 18:52:56 +0100 Subject: [PATCH 11/11] [ot] hw/opentitan: ot_dev_proxy.c: fix `MEMTXATTRS_WITH_ROLE` syntax Signed-off-by: Emmanuel Blot --- hw/opentitan/ot_dev_proxy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/opentitan/ot_dev_proxy.c b/hw/opentitan/ot_dev_proxy.c index a69bc218bb61..b142619fe06d 100644 --- a/hw/opentitan/ot_dev_proxy.c +++ b/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