Re: [PATCH v3 04/10] hw/riscv/virt: add opt-in RPMI base support

On 7/14/2026 4:56 AM, Subrahmanya Lingappa wrote:
> Add the virt machine rpmi=on/off option and wire the initial RPMI Base
> transport into machine realization.
> 
> The option remains disabled by default and requires TCG plus librpmi.
> Invalid accelerator/build combinations fail during machine
> initialization. When enabled, virt allocates the RPMI shared-memory
> transport, creates the RPMI device, configures Base service metadata,
> and emits the corresponding device-tree nodes for firmware discovery.
> 
> Signed-off-by: Subrahmanya Lingappa <[email protected]>
> ---
>   hw/riscv/Kconfig             |  1 +
>   hw/riscv/virt.c              | 96 ++++++++++++++++++++++++++++++++++++
>   include/hw/misc/riscv_rpmi.h | 10 ++++
>   include/hw/riscv/virt.h      |  3 ++
>   4 files changed, 110 insertions(+)
> 
> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> index 54e41a6afc..4e25be113a 100644
> --- a/hw/riscv/Kconfig
> +++ b/hw/riscv/Kconfig
> @@ -49,6 +49,7 @@ config RISCV_VIRT
>       imply VIRTIO_VGA
>       imply TEST_DEVICES
>       imply TPM_TIS_SYSBUS
> +    imply RISCV_RPMI
>       select DEVICE_TREE
>       select RISCV_NUMA
>       select GOLDFISH_RTC
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 33775a61fd..e52ccfae1f 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -45,6 +45,8 @@
>   #include "hw/intc/riscv_aplic.h"
>   #include "hw/intc/sifive_plic.h"
>   #include "hw/misc/sifive_test.h"
> +#include "hw/misc/riscv_rpmi.h"
> +#include "hw/riscv/rpmi-fdt.h"
>   #include "hw/core/platform-bus.h"
>   #include "chardev/char.h"
>   #include "system/device_tree.h"
> @@ -97,6 +99,8 @@ static const MemMapEntry virt_memmap[] = {
>       [VIRT_UART0] =        { 0x10000000,         0x100 },
>       [VIRT_VIRTIO] =       { 0x10001000,        0x1000 },
>       [VIRT_FW_CFG] =       { 0x10100000,          0x18 },
> +    [VIRT_RPMI_SHMEM] =   { 0x10200000,       0x20000 },
> +    [VIRT_RPMI_DOORBELL] = { 0x10230000,        0x1000 },
>       [VIRT_FLASH] =        { 0x20000000,     0x4000000 },
>       [VIRT_IMSIC_M] =      { 0x24000000, VIRT_IMSIC_MAX_SIZE },
>       [VIRT_IMSIC_S] =      { 0x28000000, VIRT_IMSIC_MAX_SIZE },
> @@ -1002,6 +1006,40 @@ static void create_fdt_iommu(RISCVVirtState *s, uint16_t bdf)
>       s->pci_iommu_bdf = bdf;
>   }
>   
> +
> +static RiscvRpmiConfig virt_rpmi_config(RISCVVirtState *s,
> +                                        const uint32_t *hart_ids,
> +                                        uint32_t hart_count)
> +{
> +    return (RiscvRpmiConfig) {
> +        .doorbell_base = s->memmap[VIRT_RPMI_DOORBELL].base,
> +        .shmem_base = s->memmap[VIRT_RPMI_SHMEM].base,
> +        .shmem_size = s->memmap[VIRT_RPMI_SHMEM].size,
> +        .a2p_req_size = VIRT_RPMI_A2P_REQ_SIZE,
> +        .p2a_req_size = VIRT_RPMI_P2A_REQ_SIZE,
> +        .platform_info = "QEMU RISC-V virt RPMI",
> +        .hart_ids = hart_ids,
> +        .hart_count = hart_count,
> +    };
> +}
> +
> +static void create_fdt_rpmi(RISCVVirtState *s, uint32_t *phandle,
> +                            uint32_t msi_phandle)
> +{
> +    RiscvRpmiConfig rpmi_cfg = virt_rpmi_config(s, NULL, 0);
> +    uint32_t rpmi_mbox_handle;
> +    RiscvRpmiFdtMboxConfig cfg = {
> +        .shmem_base = rpmi_cfg.shmem_base,
> +        .doorbell_base = rpmi_cfg.doorbell_base,
> +        .a2p_req_size = rpmi_cfg.a2p_req_size,
> +        .p2a_req_size = rpmi_cfg.p2a_req_size,
> +        .doorbell_size = s->memmap[VIRT_RPMI_DOORBELL].size,
> +    };
> +
> +    riscv_rpmi_fdt_add_mbox(MACHINE(s)->fdt, &cfg, phandle,
> +                            &rpmi_mbox_handle);
> +}
> +
>   static void finalize_fdt(RISCVVirtState *s)
>   {
>       uint32_t phandle = 1, irq_mmio_phandle = 1, msi_pcie_phandle = 1;
> @@ -1021,6 +1059,10 @@ static void finalize_fdt(RISCVVirtState *s)
>       create_fdt_pcie(s, irq_pcie_phandle, msi_pcie_phandle,
>                       iommu_sys_phandle);
>   
> +    if (s->have_rpmi) {
> +        create_fdt_rpmi(s, &phandle, msi_pcie_phandle);
> +    }
> +
>       create_fdt_reset(s, &phandle);
>   
>       create_fdt_uart(s, irq_mmio_phandle);
> @@ -1410,6 +1452,18 @@ static void virt_machine_init(MachineState *machine)
>           exit(1);
>       }
>   
> +
> +    if (s->have_rpmi) {
> +#ifndef CONFIG_LIBRPMI
> +        error_report("RISC-V RPMI support is not compiled in");
> +        exit(1);
> +#else
> +        if (kvm_enabled()) {
> +            error_report("RISC-V RPMI support is not available with KVM");
> +            exit(1);
> +        }
> +#endif
> +    }
>       /* Initialize sockets */
>       mmio_irqchip = virtio_irqchip = pcie_irqchip = NULL;
>       for (i = 0; i < socket_count; i++) {
> @@ -1552,6 +1606,25 @@ static void virt_machine_init(MachineState *machine)
>       /* SiFive Test MMIO device */
>       sifive_test_create(s->memmap[VIRT_TEST].base);
>   
> +    if (s->have_rpmi) {
> +        MachineClass *mc = MACHINE_GET_CLASS(machine);
> +        const CPUArchIdList *possible_cpus = mc->possible_cpu_arch_ids(machine);
> +        g_autofree uint32_t *rpmi_hart_ids =
> +            g_new0(uint32_t, machine->smp.cpus);
> +        RiscvRpmiConfig rpmi_cfg;
> +        Error *local_err = NULL;
> +
> +        for (i = 0; i < machine->smp.cpus; i++) {
> +            rpmi_hart_ids[i] = possible_cpus->cpus[i].arch_id;
> +        }
> +
> +        rpmi_cfg = virt_rpmi_config(s, rpmi_hart_ids, machine->smp.cpus);
> +        if (!riscv_rpmi_create(&rpmi_cfg, &local_err)) {
> +            error_report_err(local_err);
> +            exit(1);
> +        }
> +    }
> +
>       /* VirtIO MMIO devices */
>       for (i = 0; i < VIRTIO_COUNT; i++) {
>           sysbus_create_simple("virtio-mmio",
> @@ -1636,6 +1709,7 @@ static void virt_machine_instance_init(Object *obj)
>   
>       s->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
>       s->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
> +    s->have_rpmi = false;
>       s->acpi = ON_OFF_AUTO_AUTO;
>       s->iommu_sys = ON_OFF_AUTO_AUTO;
>   }
> @@ -1710,6 +1784,21 @@ static void virt_set_aclint(Object *obj, bool value, Error **errp)
>       s->have_aclint = value;
>   }
>   
> +
> +static bool virt_get_rpmi(Object *obj, Error **errp)
> +{
> +    RISCVVirtState *s = RISCV_VIRT_MACHINE(obj);
> +
> +    return s->have_rpmi;
> +}
> +
> +static void virt_set_rpmi(Object *obj, bool value, Error **errp)
> +{
> +    RISCVVirtState *s = RISCV_VIRT_MACHINE(obj);
> +
> +    s->have_rpmi = value;
> +}
> +
>   bool virt_is_iommu_sys_enabled(RISCVVirtState *s)
>   {
>       return s->iommu_sys == ON_OFF_AUTO_ON;
> @@ -1831,6 +1920,13 @@ static void virt_machine_class_init(ObjectClass *oc, const void *data)
>                                             "enable/disable emulating "
>                                             "ACLINT devices");
>   
> +
> +    object_class_property_add_bool(oc, "rpmi", virt_get_rpmi,
> +                                   virt_set_rpmi);
> +    object_class_property_set_description(oc, "rpmi",
> +                                          "Set on/off to enable/disable "
> +                                          "RISC-V RPMI devices");
> +
>       object_class_property_add_str(oc, "aia", virt_get_aia,
>                                     virt_set_aia);
>       object_class_property_set_description(oc, "aia",
> diff --git a/include/hw/misc/riscv_rpmi.h b/include/hw/misc/riscv_rpmi.h
> index cb2658e57e..b5d8a32f9b 100644
> --- a/include/hw/misc/riscv_rpmi.h
> +++ b/include/hw/misc/riscv_rpmi.h
> @@ -85,6 +85,16 @@ struct RiscvRpmiState {
>       bool has_shmem;
>   };
>   
> +#ifdef CONFIG_LIBRPMI
>   DeviceState *riscv_rpmi_create(const RiscvRpmiConfig *cfg, Error **errp);
> +#else
> +static inline DeviceState *riscv_rpmi_create(const RiscvRpmiConfig *cfg,
> +                                             Error **errp)
> +{
> +    (void)cfg;
> +    (void)errp;
> +    return NULL;
> +}

I think you can do:

static inline DeviceState *riscv_rpmi_create(const RiscvRpmiConfig *cfg G_GNUC_UNUSED,
                                              Error **errp G_GNUC_UNUSED)
{
     return NULL;
}

Instead of using the args as void to not get warnings.


Everything else LGTM.  Thanks,
Daniel


> +#endif
>   
>   #endif
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index 18a2a323a3..f7c48613bf 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -55,6 +55,7 @@ struct RISCVVirtState {
>   
>       int fdt_size;
>       bool have_aclint;
> +    bool have_rpmi;
>       RISCVVirtAIAType aia_type;
>       int aia_guests;
>       char *oem_id;
> @@ -79,6 +80,8 @@ enum {
>       VIRT_UART0,
>       VIRT_VIRTIO,
>       VIRT_FW_CFG,
> +    VIRT_RPMI_SHMEM,
> +    VIRT_RPMI_DOORBELL,
>       VIRT_IMSIC_M,
>       VIRT_IMSIC_S,
>       VIRT_FLASH,