On Mon, 2023-06-05 at 12:24 +0200, ~lexbaileylowrisc wrote:
> From: Emmanuel Blot <[email protected]>
>
> Signed-off-by: Lex Bailey <[email protected]>
> ---
> MAINTAINERS | 14 +-
> hmp-commands-info.hx | 12 ++
> hw/riscv/Kconfig | 3 +
> hw/riscv/ibex_common.c | 316
> ++++++++++++++++++++++++++++++++
> hw/riscv/meson.build | 1 +
> include/hw/riscv/ibex_common.h | 322
> +++++++++++++++++++++++++++++++++
> 6 files changed, 666 insertions(+), 2 deletions(-)
> create mode 100644 hw/riscv/ibex_common.c
> create mode 100644 include/hw/riscv/ibex_common.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e0c481e212..8b8bb7b554 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1716,9 +1716,13 @@ M: Alistair Francis <[email protected]>
> L: [email protected]
> S: Supported
> F: hw/riscv/opentitan.c
> -F: hw/*/ibex_*.c
> +F: hw/ssi/ibex_spi_host.c
> +F: hw/timer/ibex_timer.c
> +F: hw/char/ibex_uart.c
> F: include/hw/riscv/opentitan.h
> -F: include/hw/*/ibex_*.h
> +F: include/hw/ssi/ibex_spi_host.h
> +F: include/hw/timer/ibex_timer.h
> +F: include/hw/char/ibex_uart.h
> Microchip PolarFire SoC Icicle Kit
> L: [email protected]
> @@ -4450,6 +4454,12 @@ F: docs/devel/ebpf_rss.rst
> F: ebpf/*
> F: tools/ebpf/*
>
> +Ibex
> +M: lowRISC <[email protected]>
> +S: Supported
> +F: hw/riscv/ibex_common.c
> +F: include/hw/riscv/ibex_common.h
> +F: include/hw/riscv/ibex_irq.h
> Build and test automation
A change like this should be it's own patch.
But we should leave as is. It seems a bit strange to change it to a
email that has never reviewed or even submitted a patch.
If you guys want to help maintain things that's great! But let's start
off with some reviews first before taking over a whole range of files.
> -------------------------
> Build and test automation, general continuous integration
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 74c741f80e..90f05f7599 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -1010,3 +1010,15 @@ SRST
> ``info firmware-log``
> Show the firmware (ovmf) debug log.
> ERST
> +
> + {
> + .name = "ibex",
> + .args_type = "",
> + .params = "",
> + .help = "Show Ibex vCPU info",
> + },
> +
> +SRST
> + ``info ibex``
> + Show Ibex vCPU information.
> +ERST
We don't want to custom Ibex HMP command. This should be dropped
> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> index e6aa32ee8f..7877f0615c 100644
> --- a/hw/riscv/Kconfig
> +++ b/hw/riscv/Kconfig
> @@ -7,6 +7,9 @@ config RISCV_NUMA
> config IBEX
> bool
>
> +config IBEX_COMMON
> + bool
> +
> # RISC-V machines in alphabetical order
>
> config MICROCHIP_PFSOC
> diff --git a/hw/riscv/ibex_common.c b/hw/riscv/ibex_common.c
> new file mode 100644
> index 0000000000..c6056767c7
> --- /dev/null
> +++ b/hw/riscv/ibex_common.c
> @@ -0,0 +1,316 @@
> +/*
> + * QEMU RISC-V Helpers for LowRISC Ibex Demo System & OpenTitan
> EarlGrey
What is the justification for this?
> + *
> + * Copyright (c) 2022-2023 Rivos, Inc.
> + *
> + * Author(s):
> + * Emmanuel Blot <[email protected]>
> + * Loïc Lefort <[email protected]>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +#include "qemu/log.h"
> +#include "cpu.h"
> +#include "disas/disas.h"
> +#include "elf.h"
> +#include "exec/hwaddr.h"
> +#include "hw/boards.h"
> +#include "hw/loader.h"
> +#include "hw/misc/unimp.h"
> +#include "hw/qdev-core.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/riscv/ibex_common.h"
> +#include "hw/sysbus.h"
> +#include "monitor/monitor.h"
> +
> +
> +static void ibex_mmio_map_device(SysBusDevice *dev, MemoryRegion
> *mr,
> + unsigned nr, hwaddr addr)
> +{
> + assert(nr < dev->num_mmio);
> + assert(dev->mmio[nr].addr == (hwaddr)-1);
> + dev->mmio[nr].addr = addr;
> + memory_region_add_subregion(mr, addr, dev->mmio[nr].memory);
> +}
> +
> +DeviceState **ibex_create_devices(const IbexDeviceDef *defs,
> unsigned count,
> + DeviceState *parent)
> +{
> + DeviceState **devices = g_new0(DeviceState *, count);
> + unsigned unimp_count = 0;
> + for (unsigned idx = 0; idx < count; idx++) {
> + const IbexDeviceDef *def = &defs[idx];
> + assert(def->type);
> + devices[idx] = qdev_new(def->type);
> +
> + char *name;
> + if (!strcmp(def->type, TYPE_UNIMPLEMENTED_DEVICE)) {
> + if (def->name) {
> + name = g_strdup_printf("%s[%u]", def->name, def-
> >instance);
> + } else {
> + name = g_strdup_printf(TYPE_UNIMPLEMENTED_DEVICE
> "[%u]",
> + unimp_count);
> + }
> + unimp_count += 1u;
> + } else {
> + name = g_strdup_printf("%s[%u]", def->type, def-
> >instance);
> + }
> + object_property_add_child(OBJECT(parent), name,
> OBJECT(devices[idx]));
> + g_free(name);
> + }
> + return devices;
> +}
> +
> +void ibex_link_devices(DeviceState **devices, const IbexDeviceDef
> *defs,
> + unsigned count)
> +{
> + /* Link devices */
> + for (unsigned idx = 0; idx < count; idx++) {
> + DeviceState *dev = devices[idx];
> + const IbexDeviceLinkDef *link = defs[idx].link;
> + if (link) {
> + while (link->propname) {
> + DeviceState *target = devices[link->index];
> + (void)object_property_set_link(OBJECT(dev), link-
> >propname,
> + OBJECT(target),
> &error_fatal);
> + link++;
> + }
> + }
> + }
> +}
> +
> +void ibex_define_device_props(DeviceState **devices, const
> IbexDeviceDef *defs,
> + unsigned count)
> +{
> + for (unsigned idx = 0; idx < count; idx++) {
> + DeviceState *dev = devices[idx];
> + const IbexDevicePropDef *prop = defs[idx].prop;
> + if (prop) {
> + while (prop->propname) {
> + switch (prop->type) {
> + case IBEX_PROP_TYPE_BOOL:
> + object_property_set_bool(OBJECT(dev), prop-
> >propname,
> + prop->b, &error_fatal);
> + break;
> + case IBEX_PROP_TYPE_INT:
> + object_property_set_int(OBJECT(dev), prop-
> >propname,
> + prop->i, &error_fatal);
> + break;
> + case IBEX_PROP_TYPE_UINT:
> + object_property_set_int(OBJECT(dev), prop-
> >propname,
> + prop->u, &error_fatal);
> + break;
> + case IBEX_PROP_TYPE_STR:
> + object_property_set_str(OBJECT(dev), prop-
> >propname,
> + prop->s, &error_fatal);
> + break;
> + default:
> + g_assert_not_reached();
> + break;
> + }
> + prop++;
> + }
> + }
> + }
> +}
> +
> +void ibex_realize_system_devices(DeviceState **devices,
> + const IbexDeviceDef *defs, unsigned
> count)
> +{
> + BusState *bus = sysbus_get_default();
> +
> + ibex_realize_devices(devices, bus, defs, count);
> +
> + MemoryRegion *mrs[] = { get_system_memory(), NULL, NULL, NULL };
> +
> + ibex_map_devices(devices, mrs, defs, count);
> +}
> +
> +void ibex_realize_devices(DeviceState **devices, BusState *bus,
> + const IbexDeviceDef *defs, unsigned count)
> +{
> + /* Realize devices */
> + for (unsigned idx = 0; idx < count; idx++) {
> + DeviceState *dev = devices[idx];
> + const IbexDeviceDef *def = &defs[idx];
> +
> + if (def->cfg) {
> + def->cfg(dev, def, DEVICE(OBJECT(dev)->parent));
> + }
> +
> + if (def->memmap) {
> + SysBusDevice *busdev =
> + (SysBusDevice *)object_dynamic_cast(OBJECT(dev),
> +
> TYPE_SYS_BUS_DEVICE);
> + if (!busdev) {
> + /* non-sysbus devices are not supported for now */
> + g_assert_not_reached();
> + }
> +
> + qdev_realize_and_unref(DEVICE(busdev), bus,
> &error_fatal);
> + } else {
> + /* device is not connected to a bus */
> + qdev_realize_and_unref(dev, NULL, &error_fatal);
> + }
> + }
> +}
> +
> +void ibex_map_devices(DeviceState **devices, MemoryRegion **mrs,
> + const IbexDeviceDef *defs, unsigned count)
> +{
> + for (unsigned idx = 0; idx < count; idx++) {
> + DeviceState *dev = devices[idx];
> + const IbexDeviceDef *def = &defs[idx];
> +
> + if (def->memmap) {
> + SysBusDevice *busdev =
> + (SysBusDevice *)object_dynamic_cast(OBJECT(dev),
> +
> TYPE_SYS_BUS_DEVICE);
> + if (busdev) {
> + const MemMapEntry *memmap = def->memmap;
> + unsigned mem = 0;
> + while (memmap->size) {
> + unsigned region = IBEX_MEMMAP_GET_REGIDX(memmap-
> >base);
> + MemoryRegion *mr = mrs[region];
> + if (mr) {
> + ibex_mmio_map_device(busdev, mr, mem,
> +
> IBEX_MEMMAP_GET_ADDRESS(
> + memmap->base));
> + }
> + mem++;
> + memmap++;
> + }
> + }
> + }
> + }
> +}
> +
> +void ibex_connect_devices(DeviceState **devices, const IbexDeviceDef
> *defs,
> + unsigned count)
> +{
> + /* Connect GPIOs (in particular, IRQs) */
> + for (unsigned idx = 0; idx < count; idx++) {
> + DeviceState *dev = devices[idx];
> + const IbexDeviceDef *def = &defs[idx];
> +
> + if (def->gpio) {
> + const IbexGpioConnDef *conn = def->gpio;
> + while (conn->out.num >= 0 && conn->in.num >= 0) {
> + qemu_irq in_gpio =
> + qdev_get_gpio_in_named(devices[conn->in.index],
> + conn->in.name, conn-
> >in.num);
> +
> + qdev_connect_gpio_out_named(dev, conn->out.name,
> conn->out.num,
> + in_gpio);
> + conn++;
> + }
> + }
> + }
> +}
> +
> +void ibex_configure_devices(DeviceState **devices, BusState *bus,
> + const IbexDeviceDef *defs, unsigned
> count)
> +{
> + ibex_link_devices(devices, defs, count);
> + ibex_define_device_props(devices, defs, count);
> + ibex_realize_devices(devices, bus, defs, count);
> + ibex_connect_devices(devices, defs, count);
> +}
> +
> +void ibex_unimp_configure(DeviceState *dev, const IbexDeviceDef
> *def,
> + DeviceState *parent)
> +{
> + if (def->name) {
> + qdev_prop_set_string(dev, "name", def->name);
> + }
> + assert(def->memmap != NULL);
> + assert(def->memmap->size != 0);
> + qdev_prop_set_uint64(dev, "size", def->memmap->size);
> +}
+
> +void ibex_load_kernel(AddressSpace *as)
> +{
> + MachineState *ms = MACHINE(qdev_get_machine());
> +
> + /* load kernel if provided */
> + if (ms->kernel_filename) {
> + uint64_t kernel_entry;
> + if (load_elf_ram_sym(ms->kernel_filename, NULL, NULL, NULL,
> + &kernel_entry, NULL, NULL, NULL, 0,
> EM_RISCV, 1, 0,
> + as, true, NULL) <= 0) {
> + error_report("Cannot load ELF kernel %s", ms-
> >kernel_filename);
> + exit(EXIT_FAILURE);
> + }
> +
> + CPUState *cpu;
> + CPU_FOREACH(cpu) {
> + if (!as || cpu->as == as) {
> + CPURISCVState *env = &RISCV_CPU(cpu)->env;
> + env->resetvec = (target_ulong)kernel_entry;
> + }
> + }
> + }
> +}
I don't think we need public Ibex specific functions for anything above
> +
> +uint64_t ibex_get_current_pc(void)
> +{
> + CPUState *cs = current_cpu;
> +
> + return cs && cs->cc->get_pc ? cs->cc->get_pc(cs) : 0u;
> +}
> +
> +/* x0 is replaced with PC */
> +static const char ibex_ireg_names[32u][4u] = {
> + "pc", "ra", "sp", "gp", "tp", "t0", "t1", "t2", "s0", "s1",
> "a0",
> + "a1", "a2", "a3", "a4", "a5", "a6", "a7", "s2", "s3", "s4",
> "s5",
> + "s6", "s7", "s8", "s9", "s10", "s11", "t3", "t4", "t5", "t6",
> +};
What? Why do you need your own register names?
> +
> +void ibex_log_vcpu_registers(uint64_t regbm)
> +{
> + CPURISCVState *env = &RISCV_CPU(current_cpu)->env;
> + qemu_log_mask(CPU_LOG_TB_IN_ASM, "\n....\n");
> + if (regbm & 0x1u) {
> + qemu_log_mask(CPU_LOG_TB_IN_ASM, "%4s: 0x" TARGET_FMT_lx
> "\n",
> + ibex_ireg_names[0], env->pc);
> + }
> + for (unsigned gix = 1u; gix < 32u; gix++) {
> + uint64_t mask = 1u << gix;
> + if (regbm & mask) {
> + qemu_log_mask(CPU_LOG_TB_IN_ASM, "%4s: 0x" TARGET_FMT_lx
> "\n",
> + ibex_ireg_names[gix], env->gpr[gix]);
> + }
> + }
> +}
Or a custom logger?
> +
> +/*
> + * Note: this is not specific to Ibex, and might apply to any vCPU.
> + */
> +static void hmp_info_ibex(Monitor *mon, const QDict *qdict)
> +{
> + CPUState *cpu;
> +
> + CPU_FOREACH(cpu) {
> + vaddr pc;
> + const char *symbol;
> + if (cpu->cc->get_pc) {
> + pc = cpu->cc->get_pc(cpu);
> + symbol = lookup_symbol(pc);
> + } else {
> + pc = -1;
> + symbol = "?";
> + }
> + monitor_printf(mon, "* CPU #%d: 0x%" PRIx64 " in '%s'\n",
> + cpu->cpu_index, (uint64_t)pc, symbol);
> + }
> +}
> +
> +static void ibex_register_types(void)
> +{
> + monitor_register_hmp("ibex", true, &hmp_info_ibex);
> +}
> +
> +type_init(ibex_register_types)
> diff --git a/hw/riscv/meson.build b/hw/riscv/meson.build
> index 533472e22a..70d63f56b5 100644
> --- a/hw/riscv/meson.build
> +++ b/hw/riscv/meson.build
> @@ -3,6 +3,7 @@ riscv_ss.add(files('boot.c'))
> riscv_ss.add(when: 'CONFIG_RISCV_NUMA', if_true: files('numa.c'))
> riscv_ss.add(files('riscv_hart.c'))
> riscv_ss.add(when: 'CONFIG_OPENTITAN', if_true:
> files('opentitan.c'))
> +riscv_ss.add(when: 'CONFIG_IBEX_COMMON', if_true:
> files('ibex_common.c'))
> riscv_ss.add(when: 'CONFIG_RISCV_VIRT', if_true: files('virt.c'))
> riscv_ss.add(when: 'CONFIG_SHAKTI_C', if_true: files('shakti_c.c'))
> riscv_ss.add(when: 'CONFIG_SIFIVE_E', if_true: files('sifive_e.c'))
> diff --git a/include/hw/riscv/ibex_common.h
> b/include/hw/riscv/ibex_common.h
> new file mode 100644
> index 0000000000..6c7dae5cbe
> --- /dev/null
> +++ b/include/hw/riscv/ibex_common.h
> @@ -0,0 +1,322 @@
> +/*
> + * QEMU RISC-V Helpers for LowRISC OpenTitan EarlGrey and related
> systems
> + *
> + * Copyright (c) 2022-2023 Rivos, Inc.
> + *
> + * Author(s):
> + * Emmanuel Blot <[email protected]>
> + * Loïc Lefort <[email protected]>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef HW_RISCV_IBEX_COMMON_H
> +#define HW_RISCV_IBEX_COMMON_H
> +
> +#include "qemu/osdep.h"
> +#include "qom/object.h"
> +#include "exec/hwaddr.h"
> +#include "hw/qdev-core.h"
> +
> +
> +/* -----------------------------------------------------------------
> ------- */
> +/* Devices & GPIOs */
> +/* -----------------------------------------------------------------
> ------- */
> +
> +#define IBEX_MAX_MMIO_ENTRIES 4u
> +#define IBEX_MAX_GPIO_ENTRIES 16u
> +
> +typedef struct IbexDeviceDef IbexDeviceDef;
> +
> +typedef void (*ibex_dev_cfg_fn)(DeviceState *dev, const
> IbexDeviceDef *def,
> + DeviceState *parent);
> +
> +/**
> + * Structure defining a GPIO connection (in particular, IRQs) from
> the current
> + * device to a target device
> + */
> +typedef struct {
> + /** Source GPIO */
> + struct {
> + /** Name of source GPIO array or NULL for unnamed */
> + const char *name;
> + /** Index of source output GPIO */
> + int num;
> + } out;
> +
> + /** Target GPIO */
> + struct {
> + /** Target device index */
> + int index;
> + /** Name of target input GPIO array or NULL for unnamed */
> + const char *name;
> + /** Index of target input GPIO */
> + int num;
> + } in;
> +} IbexGpioConnDef;
> +
> +typedef struct {
> + /** Name of the property to assign the linked device to */
> + const char *propname;
> + /** Linked device index */
> + int index;
> +} IbexDeviceLinkDef;
> +
> +/** Type of device property */
> +typedef enum {
> + IBEX_PROP_TYPE_BOOL,
> + IBEX_PROP_TYPE_INT,
> + IBEX_PROP_TYPE_UINT,
> + IBEX_PROP_TYPE_STR,
> +} IbexPropertyType;
Why do you need custom properties?
> +
> +typedef struct {
> + /** Name of the property */
> + const char *propname;
> + /** Type of property */
> + IbexPropertyType type;
> + /** Value */
> + union {
> + bool b;
> + int64_t i;
> + uint64_t u;
> + const char *s;
> + };
> +} IbexDevicePropDef;
> +
> +struct IbexDeviceDef {
> + /** Registered type of the device */
> + const char *type;
> + /** Optional name, may be NULL */
> + const char *name;
> + /** Instance number, default to 0 */
> + int instance;
> + /** Optional configuration function */
> + ibex_dev_cfg_fn cfg;
> + /** Array of memory map */
> + const MemMapEntry *memmap;
> + /** Array of GPIO connections */
> + const IbexGpioConnDef *gpio;
> + /** Array of linked devices */
> + const IbexDeviceLinkDef *link;
> + /** Array of properties */
> + const IbexDevicePropDef *prop;
> +};
> +
> +/*
> + * Special memory address marked to flag a special MemMapEntry.
> + * Flagged MemMapEntry are used to select a memory region while mem
> mapping
> + * devices. There could be up to 4 different regions.
> + */
> +#define IBEX_MEMMAP_REGIDX_COUNT 4u
> +#define IBEX_MEMMAP_REGIDX_MASK \
> + ((IBEX_MEMMAP_REGIDX_COUNT) - 1u) /* address are always word-
> aligned */
> +#define IBEX_MEMMAP_MAKE_REG(_addr_, _flag_) \
> + ((_addr_) | ((_flag_) & IBEX_MEMMAP_REGIDX_MASK))
> +#define IBEX_MEMMAP_GET_REGIDX(_addr_) ((_addr_) &
> IBEX_MEMMAP_REGIDX_MASK)
> +#define IBEX_MEMMAP_GET_ADDRESS(_addr_) ((_addr_) &
> ~IBEX_MEMMAP_REGIDX_MASK)
> +
> +/**
> + * Create memory map entries, each arg is MemMapEntry definition
> + */
> +#define MEMMAPENTRIES(...) \
> + (const MemMapEntry[]) \
> + { \
> + __VA_ARGS__, \
> + { \
> + .size = 0u \
> + } \
> + }
> +
> +/**
> + * Create GPIO connection entries, each arg is IbexGpioConnDef
> definition
> + */
> +#define IBEXGPIOCONNDEFS(...) \
> + (const IbexGpioConnDef[]) \
> + { \
> + __VA_ARGS__, \
> + { \
> + .out = {.num = -1 } \
> + } \
> + }
> +
> +/**
> + * Create device link entries, each arg is IbexDeviceLinkDef
> definition
> + */
> +#define IBEXDEVICELINKDEFS(...) \
> + (const IbexDeviceLinkDef[]) \
> + { \
> + __VA_ARGS__, \
> + { \
> + .propname = NULL \
> + } \
> + }
> +
> +/**
> + * Create device property entries, each arg is IbexDevicePropDef
> definition
> + */
> +#define IBEXDEVICEPROPDEFS(...) \
> + (const IbexDevicePropDef[]) \
> + { \
> + __VA_ARGS__, \
> + { \
> + .propname = NULL \
> + } \
> + }
> +
> +/**
> + * Create a IbexGpioConnDef to connect two unnamed GPIOs
> + */
> +#define IBEX_GPIO(_irq_, _in_idx_, _num_) \
> + { \
> + .out = { \
> + .num = (_irq_), \
> + }, \
> + .in = { \
> + .index = (_in_idx_), \
> + .num = (_num_), \
> + } \
> + }
> +
> +/**
> + * Create a IbexGpioConnDef to connect a SysBus IRQ to an unnamed
> GPIO
> + */
> +#define IBEX_GPIO_SYSBUS_IRQ(_irq_, _in_idx_, _num_) \
> + { \
> + .out = { \
> + .name = SYSBUS_DEVICE_GPIO_IRQ, \
> + .num = (_irq_), \
> + }, \
> + .in = { \
> + .index = (_in_idx_), \
> + .num = (_num_), \
> + } \
> + }
> +
> +/**
> + * Create a IbexLinkDeviceDef to link one device to another
> + */
> +#define IBEX_DEVLINK(_pname_, _idx_) \
> + { \
> + .propname = (_pname_), .index = (_idx_), \
> + }
> +
> +/**
> + * Create a boolean device property
> + */
> +#define IBEX_DEV_BOOL_PROP(_pname_, _b_) \
> + { \
> + .propname = (_pname_), .type = IBEX_PROP_TYPE_BOOL, .b =
> (_b_), \
> + }
> +
> +/**
> + * Create a signed integer device property
> + */
> +#define IBEX_DEV_INT_PROP(_pname_, _i_) \
> + { \
> + .propname = (_pname_), .type = IBEX_PROP_TYPE_INT, .i =
> (_i_), \
> + }
> +
> +/**
> + * Create an unsigned integer device property
> + */
> +#define IBEX_DEV_UINT_PROP(_pname_, _u_) \
> + { \
> + .propname = (_pname_), .type = IBEX_PROP_TYPE_UINT, .u =
> (_u_), \
> + }
> +
> +/**
> + * Create a string device property
> + */
> +#define IBEX_DEV_STRING_PROP(_pname_, _s_) \
> + { \
> + .propname = (_pname_), .type = IBEX_PROP_TYPE_STR, .s =
> (_s_), \
> + }
> +
> +DeviceState **ibex_create_devices(const IbexDeviceDef *defs,
> unsigned count,
> + DeviceState *parent);
> +void ibex_link_devices(DeviceState **devices, const IbexDeviceDef
> *defs,
> + unsigned count);
> +void ibex_define_device_props(DeviceState **devices, const
> IbexDeviceDef *defs,
> + unsigned count);
> +void ibex_realize_system_devices(DeviceState **devices,
> + const IbexDeviceDef *defs, unsigned
> count);
> +void ibex_realize_devices(DeviceState **devices, BusState *bus,
> + const IbexDeviceDef *defs, unsigned
> count);
> +void ibex_connect_devices(DeviceState **devices, const IbexDeviceDef
> *defs,
> + unsigned count);
> +void ibex_map_devices(DeviceState **devices, MemoryRegion **mrs,
> + const IbexDeviceDef *defs, unsigned count);
> +void ibex_configure_devices(DeviceState **devices, BusState *bus,
> + const IbexDeviceDef *defs, unsigned
> count);
> +
> +/**
> + * Utility function to configure unimplemented device.
> + * The Ibex device definition should have one defined memory entry,
> and an
> + * optional name.
> + */
> +void ibex_unimp_configure(DeviceState *dev, const IbexDeviceDef
> *def,
> + DeviceState *parent);
> +
> +/* -----------------------------------------------------------------
> ------- */
> +/* CPU */
> +/* -----------------------------------------------------------------
> ------- */
> +
> +/**
> + * Load an ELF application into a CPU address space.
> + * @as the address space to load the application into, maybe NULL to
> use the
> + * default address space
> + */
> +void ibex_load_kernel(AddressSpace *as);
> +
> +/**
> + * Helper for device debugging: report the current guest PC, if any.
> + *
> + * If a HW access is performed from another device but the CPU,
> reported PC
> + * is 0.
> + */
> +uint64_t ibex_get_current_pc(void);
> +
> +enum {
> + RV_GPR_PC = (1u << 0u),
> + RV_GPR_RA = (1u << 1u),
> + RV_GPR_SP = (1u << 2u),
> + RV_GPR_GP = (1u << 3u),
> + RV_GPR_TP = (1u << 4u),
> + RV_GPR_T0 = (1u << 5u),
> + RV_GPR_T1 = (1u << 6u),
> + RV_GPR_T2 = (1u << 7u),
> + RV_GPR_S0 = (1u << 8u),
> + RV_GPR_S1 = (1u << 9u),
> + RV_GPR_A0 = (1u << 10u),
> + RV_GPR_A1 = (1u << 11u),
> + RV_GPR_A2 = (1u << 12u),
> + RV_GPR_A3 = (1u << 13u),
> + RV_GPR_A4 = (1u << 14u),
> + RV_GPR_A5 = (1u << 15u),
> + RV_GPR_A6 = (1u << 16u),
> + RV_GPR_A7 = (1u << 17u),
> + RV_GPR_S2 = (1u << 18u),
> + RV_GPR_S3 = (1u << 19u),
> + RV_GPR_S4 = (1u << 20u),
> + RV_GPR_S5 = (1u << 21u),
> + RV_GPR_S6 = (1u << 22u),
> + RV_GPR_S7 = (1u << 23u),
> + RV_GPR_S8 = (1u << 24u),
> + RV_GPR_S9 = (1u << 25u),
> + RV_GPR_S10 = (1u << 26u),
> + RV_GPR_S11 = (1u << 27u),
> + RV_GPR_T3 = (1u << 28u),
> + RV_GPR_T4 = (1u << 29u),
> + RV_GPR_T5 = (1u << 30u),
> + RV_GPR_T6 = (1u << 31u),
> +};
> +
> +/**
> + * Log current vCPU registers.
> + *
> + * @regbm is a bitmap of registers to be dumped [x1..t6], pc replace
> x0
> + */
> +void ibex_log_vcpu_registers(uint64_t regbm);
> +
> +#endif /* HW_RISCV_IBEX_COMMON_H */
I think this entire patch should be dropped and just use the existing
QEMU infrastructure. If there is a **good** reason for adding something
that needs to be well documented in the commit message and each part
added individually
Alistair