I will send a v3 patch to fix it, thanks! - Jim
On Sun, Aug 10, 2025 at 12:39 AM Daniel Henrique Barboza <[email protected]> wrote: > > > > On 4/17/25 7:52 AM, Jim Shu wrote: > > Implement the RISC-V WorldGuard Checker, which sits in front of RAM or > > device MMIO and allow software to configure it to either pass through or > > reject transactions. > > > > We implement the wgChecker as a QEMU IOMMU, which will direct transactions > > either through to the devices and memory behind it or to a special > > "never works" AddressSpace if they are blocked. > > > > This initial commit implements the skeleton of the device: > > * it always permits accesses > > * it doesn't implement wgChecker's slot registers > > * it doesn't implement the interrupt or other behaviour > > for blocked transactions > > > > Signed-off-by: Jim Shu <[email protected]> > > --- > > hw/misc/meson.build | 2 +- > > hw/misc/riscv_wgchecker.c | 603 +++++++++++++++++++++++++++++ > > hw/misc/trace-events | 8 + > > include/hw/misc/riscv_worldguard.h | 63 +++ > > 4 files changed, 675 insertions(+), 1 deletion(-) > > create mode 100644 hw/misc/riscv_wgchecker.c > > > > diff --git a/hw/misc/meson.build b/hw/misc/meson.build > > index 3d2f4bb6a3..73c11bc7c9 100644 > > --- a/hw/misc/meson.build > > +++ b/hw/misc/meson.build > > @@ -34,7 +34,7 @@ system_ss.add(when: 'CONFIG_SIFIVE_E_PRCI', if_true: > > files('sifive_e_prci.c')) > > system_ss.add(when: 'CONFIG_SIFIVE_E_AON', if_true: > > files('sifive_e_aon.c')) > > system_ss.add(when: 'CONFIG_SIFIVE_U_OTP', if_true: > > files('sifive_u_otp.c')) > > system_ss.add(when: 'CONFIG_SIFIVE_U_PRCI', if_true: > > files('sifive_u_prci.c')) > > -specific_ss.add(when: 'CONFIG_RISCV_WORLDGUARD', if_true: > > files('riscv_worldguard.c')) > > +specific_ss.add(when: 'CONFIG_RISCV_WORLDGUARD', if_true: > > files('riscv_worldguard.c', 'riscv_wgchecker.c')) > > > > subdir('macio') > > > > diff --git a/hw/misc/riscv_wgchecker.c b/hw/misc/riscv_wgchecker.c > > new file mode 100644 > > index 0000000000..ea50f4f53a > > --- /dev/null > > +++ b/hw/misc/riscv_wgchecker.c > > @@ -0,0 +1,603 @@ > > (...) > > > + > > +static void riscv_wgchecker_realize(DeviceState *dev, Error **errp) > > +{ > > + Object *obj = OBJECT(dev); > > + RISCVWgCheckerState *s = RISCV_WGCHECKER(dev); > > + uint64_t size; > > + > > + if (worldguard_config == NULL) { > > + error_setg(errp, "Couldn't find global WorldGuard configs. " > > + "Please realize %s device at first.", > > Extra "at": "Please realize %s device first." > > > Everything else LGTM: > > > Reviewed-by: Daniel Henrique Barboza <[email protected]> > > > + TYPE_RISCV_WORLDGUARD); > > + return; > > + } > > + > > + if (s->slot_count == 0) { > > + error_setg(errp, "wgChecker slot-count couldn't be zero."); > > + return; > > + } > > + > > + memory_region_init_io(&s->mmio, OBJECT(dev), &riscv_wgchecker_ops, s, > > + TYPE_RISCV_WGCHECKER, s->mmio_size); > > + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio); > > + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq); > > + > > + /* Address range should be NAPOT alignment */ > > + address_range_align_napot(s); > > + > > + for (int i=0; i<WGC_NUM_REGIONS; i++) { > > + WgCheckerRegion *region = &s->mem_regions[i]; > > + > > + if (!region->downstream) { > > + continue; > > + } > > + region->wgchecker = s; > > + > > + const char *upstream_name = g_strdup_printf( > > + "wgchecker-upstream-%"HWADDR_PRIx, region->region_offset); > > + const char *downstream_name = g_strdup_printf( > > + "wgchecker-downstream-%"HWADDR_PRIx, region->region_offset); > > + > > + size = memory_region_size(region->downstream); > > + memory_region_init_iommu(®ion->upstream, > > sizeof(region->upstream), > > + TYPE_RISCV_WGC_IOMMU_MEMORY_REGION, > > + obj, upstream_name, size); > > + > > + /* upstream MRs are 2nd ~ (n+1)th MemoryRegion. */ > > + sysbus_init_mmio(SYS_BUS_DEVICE(dev), > > MEMORY_REGION(®ion->upstream)); > > + > > + /* > > + * This memory region is not exposed to users of this device as a > > + * sysbus MMIO region, but is instead used internally as something > > + * that our IOMMU translate function might direct accesses to. > > + */ > > + memory_region_init_io(®ion->blocked_io, obj, > > &riscv_wgc_mem_blocked_ops, > > + region, "wgchecker-blocked-io", size); > > + > > + address_space_init(®ion->downstream_as, region->downstream, > > + downstream_name); > > + address_space_init(®ion->blocked_io_as, ®ion->blocked_io, > > + "wgchecker-blocked-io"); > > + } > > +} > > + > > +static void riscv_wgchecker_unrealize(DeviceState *dev) > > +{ > > + RISCVWgCheckerState *s = RISCV_WGCHECKER(dev); > > + > > + g_free(s->slots); > > + if (s->num_default_slots && s->default_slots) { > > + g_free(s->default_slots); > > + } > > +} > > + > > +static void riscv_wgchecker_reset_enter(Object *obj, ResetType type) > > +{ > > + RISCVWgCheckerState *s = RISCV_WGCHECKER(obj); > > + uint64_t start = s->addr_range_start; > > + uint64_t end = s->addr_range_start + s->addr_range_size; > > + int nslots = s->slot_count; > > + > > + s->errcause = 0; > > + s->erraddr = 0; > > +} > > + > > +static void riscv_wgchecker_class_init(ObjectClass *klass, void *data) > > +{ > > + DeviceClass *dc = DEVICE_CLASS(klass); > > + > > + device_class_set_props(dc, riscv_wgchecker_properties); > > + dc->user_creatable = true; > > + dc->realize = riscv_wgchecker_realize; > > + dc->unrealize = riscv_wgchecker_unrealize; > > + ResettableClass *rc = RESETTABLE_CLASS(klass); > > + rc->phases.enter = riscv_wgchecker_reset_enter; > > +} > > + > > +static void riscv_wgchecker_instance_init(Object *obj) > > +{ > > + RISCVWgCheckerState *s = RISCV_WGCHECKER(obj); > > + > > + s->num_default_slots = 0; > > +} > > + > > +static const TypeInfo riscv_wgchecker_info = { > > + .name = TYPE_RISCV_WGCHECKER, > > + .parent = TYPE_SYS_BUS_DEVICE, > > + .instance_init = riscv_wgchecker_instance_init, > > + .instance_size = sizeof(RISCVWgCheckerState), > > + .class_init = riscv_wgchecker_class_init, > > +}; > > + > > +static void riscv_wgchecker_register_types(void) > > +{ > > + type_register_static(&riscv_wgchecker_info); > > + type_register_static(&riscv_wgc_iommu_memory_region_info); > > +} > > + > > +type_init(riscv_wgchecker_register_types) > > + > > +/* > > + * Create WgChecker device > > + */ > > +DeviceState *riscv_wgchecker_create(hwaddr addr, uint32_t size, > > + qemu_irq irq, uint32_t slot_count, > > + uint64_t addr_range_start, > > + uint64_t addr_range_size, > > + uint32_t num_of_region, > > + MemoryRegion **downstream, > > + uint64_t *region_offset, > > + uint32_t num_default_slots, > > + WgCheckerSlot *default_slots) > > +{ > > + DeviceState *dev = qdev_new(TYPE_RISCV_WGCHECKER); > > + RISCVWgCheckerState *s = RISCV_WGCHECKER(dev); > > + char name_mr[32]; > > + char name_offset[32]; > > + int i; > > + > > + qdev_prop_set_uint32(dev, "slot-count", slot_count); > > + qdev_prop_set_uint32(dev, "mmio-size", size); > > + qdev_prop_set_uint64(dev, "addr-range-start", addr_range_start); > > + if (addr_range_size) { > > + qdev_prop_set_uint64(dev, "addr-range-size", addr_range_size); > > + } > > + > > + g_assert(num_of_region <= WGC_NUM_REGIONS); > > + for (i=0; i<num_of_region; i++) { > > + snprintf(name_mr, 32, "downstream-mr[%d]", i); > > + snprintf(name_offset, 32, "region-offset[%d]", i); > > + > > + object_property_set_link(OBJECT(dev), name_mr, > > + OBJECT(downstream[i]), &error_fatal); > > + qdev_prop_set_uint64(dev, name_offset, region_offset[i]); > > + } > > + > > + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); > > + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr); > > + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq); > > + return dev; > > +} > > diff --git a/hw/misc/trace-events b/hw/misc/trace-events > > index 4383808d7a..b1d8538220 100644 > > --- a/hw/misc/trace-events > > +++ b/hw/misc/trace-events > > @@ -395,3 +395,11 @@ ivshmem_flat_interrupt_peer(uint16_t peer_id, uint16_t > > vector_id) "Interrupting > > i2c_echo_event(const char *id, const char *event) "%s: %s" > > i2c_echo_recv(const char *id, uint8_t data) "%s: recv 0x%02" PRIx8 > > i2c_echo_send(const char *id, uint8_t data) "%s: send 0x%02" PRIx8 > > + > > +# riscv_worldguard.c > > +riscv_wgchecker_mmio_read(uint64_t base, uint64_t offset, unsigned int > > size) "base = 0x%"PRIx64", offset = 0x%"PRIx64", size = 0x%x" > > +riscv_wgchecker_mmio_write(uint64_t base, uint64_t offset, unsigned int > > size, uint64_t val) "base = 0x%"PRIx64", offset = 0x%"PRIx64", size = 0x%x, > > val = 0x%"PRIx64 > > + > > +riscv_wgc_mem_blocked_read(uint64_t addr, unsigned size, uint32_t wid) > > "wgChecker blocked read: offset 0x%" PRIx64 " size %u wid %" PRIu32 > > +riscv_wgc_mem_blocked_write(uint64_t addr, uint64_t data, unsigned size, > > uint32_t wid) "wgChecker blocked write: offset 0x%" PRIx64 " data 0x%" > > PRIx64 " size %u wid %" PRIu32 > > +riscv_wgc_translate(uint64_t addr, int flags, int wid, const char *res) > > "wgChecker translate: addr 0x%016" PRIx64 " flags 0x%x wid %d: %s" > > diff --git a/include/hw/misc/riscv_worldguard.h > > b/include/hw/misc/riscv_worldguard.h > > index 211a72e438..7b5aae866a 100644 > > --- a/include/hw/misc/riscv_worldguard.h > > +++ b/include/hw/misc/riscv_worldguard.h > > @@ -53,4 +53,67 @@ void riscv_worldguard_apply_cpu(uint32_t hartid); > > uint32_t mem_attrs_to_wid(MemTxAttrs attrs); > > bool could_access_wgblocks(MemTxAttrs attrs, const char *wgblock); > > > > +#define TYPE_RISCV_WGCHECKER "riscv.wgchecker" > > + > > +typedef struct RISCVWgCheckerState RISCVWgCheckerState; > > +DECLARE_INSTANCE_CHECKER(RISCVWgCheckerState, RISCV_WGCHECKER, > > + TYPE_RISCV_WGCHECKER) > > + > > +#define TYPE_RISCV_WGC_IOMMU_MEMORY_REGION > > "riscv-wgc-iommu-memory-region" > > + > > +typedef struct WgCheckerSlot WgCheckerSlot; > > +struct WgCheckerSlot { > > + uint64_t addr; > > + uint64_t perm; > > + uint32_t cfg; > > +}; > > + > > +typedef struct WgCheckerRegion WgCheckerRegion; > > +struct WgCheckerRegion { > > + MemoryRegion *downstream; > > + uint64_t region_offset; > > + > > + IOMMUMemoryRegion upstream; > > + MemoryRegion blocked_io; > > + AddressSpace downstream_as; > > + AddressSpace blocked_io_as; > > + > > + RISCVWgCheckerState *wgchecker; > > +}; > > + > > +#define WGC_NUM_REGIONS 16 > > + > > +struct RISCVWgCheckerState { > > + /*< private >*/ > > + SysBusDevice parent_obj; > > + > > + /*< public >*/ > > + MemoryRegion mmio; > > + qemu_irq irq; > > + > > + /* error reg */ > > + uint64_t errcause; > > + uint64_t erraddr; > > + > > + /* Memory regions protected by wgChecker */ > > + WgCheckerRegion mem_regions[WGC_NUM_REGIONS]; > > + > > + /* Property */ > > + uint32_t slot_count; /* nslots */ > > + uint32_t mmio_size; > > + uint64_t addr_range_start; > > + uint64_t addr_range_size; > > + bool hw_bypass; > > +}; > > + > > +DeviceState *riscv_wgchecker_create(hwaddr addr, uint32_t size, > > + qemu_irq irq, uint32_t slot_count, > > + uint64_t addr_range_start, > > + uint64_t addr_range_size, > > + uint32_t num_of_region, > > + MemoryRegion **downstream, > > + uint64_t *region_offset, > > + uint32_t num_default_slots, > > + WgCheckerSlot *default_slots); > > + > > #endif >
