Re: [PATCH v3 02/10] hw/misc: add RISC-V RPMI transport and Base support

Daniel,

On Fri, Jul 17, 2026 at 6:24 PM Daniel Henrique Barboza
<[email protected]> wrote:
>
> Hi Subbu,
>
> On 7/14/2026 4:56 AM, Subrahmanya Lingappa wrote:
> > RPMI (RISC-V Platform Management Interface) is the RISC-V non-ISA
> > platform management specification. It defines a shared-memory transport,
> > message protocol, and service groups for platform management.
> >
> > Add the QEMU transport backend and Base service plumbing for virt using
> > librpmi.
> >
> > This implementation targets the RISC-V Platform Management Interface
> > (RPMI) Specification v1.0 Ratified:
> >
> > https://github.com/riscv-non-isa/riscv-rpmi/releases/download/v1.0/riscv-rpmi.pdf
> >
> > The librpmi backend is maintained at:
> >
> > https://github.com/riscv-software-src/librpmi
> >
> > Signed-off-by: Subrahmanya Lingappa <[email protected]>
> > ---
> >   MAINTAINERS                   |   2 +
> >   hw/misc/Kconfig               |   4 +
> >   hw/misc/meson.build           |   3 +
> >   hw/misc/riscv_rpmi.c          | 476 ++++++++++++++++++++++++++++++++++
> >   hw/misc/riscv_rpmi_internal.h |  24 ++
> >   include/hw/misc/riscv_rpmi.h  |  90 +++++++
> >   6 files changed, 599 insertions(+)
> >   create mode 100644 hw/misc/riscv_rpmi.c
> >   create mode 100644 hw/misc/riscv_rpmi_internal.h
> >   create mode 100644 include/hw/misc/riscv_rpmi.h
> >
>
> [ ... ]
>
> > +
> > +static void riscv_rpmi_class_init(ObjectClass *klass, const void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > +    dc->realize = riscv_rpmi_realize;
> > +    dc->unrealize = riscv_rpmi_unrealize;
> > +    dc->vmsd = &riscv_rpmi_vmstate;
> > +    device_class_set_legacy_reset(dc, riscv_rpmi_reset);
>
>
> This is something that I noticed yesterday with another work and I didn't point to
> you earlier - we shouldn't use this legacy API.  In include/hw/core/qdev.h:
>
>
>   * device_class_set_legacy_reset(): set the DeviceClass::reset method
>   * @dc: The device class
>   * @dev_reset: the reset function
>   *
>   * This function sets the DeviceClass::reset method. This is widely
>   * used in existing code, but new code should prefer to use the
>   * Resettable API as documented in docs/devel/reset.rst.
>
>
> Unless this device has an exclusive reset procedure you can do like riscv-iommu-sys
> does:
>
>
>      ResettableClass *rc = RESETTABLE_CLASS(klass);
>
>      rc->phases.hold = riscv_iommu_sys_reset_hold;
>
>
> In this case you would do
>
>
>      rc->phases.hold =   riscv_rpmi_reset
>
>
> You can ignore the extra ResetType argument in case you don't need it.
>
>
> The rest of the code LGTM.  Thanks,
> Daniel
>
  Fixed in v4. I switched the RPMI device reset handling to the Resettable
  API instead of device_class_set_legacy_reset().

Thanks,
subbu

>
>
>
>
>
> > +    device_class_set_props(dc, riscv_rpmi_properties);
> > +}
> > +
> > +static const TypeInfo riscv_rpmi_info = {
> > +    .name          = TYPE_RISCV_RPMI,
> > +    .parent        = TYPE_SYS_BUS_DEVICE,
> > +    .instance_size = sizeof(RiscvRpmiState),
> > +    .instance_init = riscv_rpmi_init,
> > +    .instance_finalize = riscv_rpmi_finalize,
> > +    .class_init    = riscv_rpmi_class_init,
> > +};
> > +
> > +static void riscv_rpmi_register_types(void)
> > +{
> > +    type_register_static(&riscv_rpmi_info);
> > +}
> > +
> > +type_init(riscv_rpmi_register_types)
> > +
> > +DeviceState *riscv_rpmi_create(const RiscvRpmiConfig *cfg, Error **errp)
> > +{
> > +    DeviceState *dev;
> > +    RiscvRpmiState *s;
> > +
> > +    if (!cfg) {
> > +        error_setg(errp, "missing RPMI configuration");
> > +        return NULL;
> > +    }
> > +
> > +    dev = qdev_new(TYPE_RISCV_RPMI);
> > +    qdev_prop_set_uint64(dev, "shmem-base", cfg->shmem_base);
> > +    qdev_prop_set_uint64(dev, "shmem-size", cfg->shmem_size);
> > +    qdev_prop_set_uint32(dev, "a2p-req-size", cfg->a2p_req_size);
> > +    qdev_prop_set_uint32(dev, "p2a-req-size", cfg->p2a_req_size);
> > +
> > +    s = RISCV_RPMI(dev);
> > +    riscv_rpmi_configure_base(s, cfg);
> > +
> > +    if (!sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), errp)) {
> > +        return NULL;
> > +    }
> > +
> > +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, cfg->doorbell_base);
> > +    return dev;
> > +}
> > diff --git a/hw/misc/riscv_rpmi_internal.h b/hw/misc/riscv_rpmi_internal.h
> > new file mode 100644
> > index 0000000000..447c0acb55
> > --- /dev/null
> > +++ b/hw/misc/riscv_rpmi_internal.h
> > @@ -0,0 +1,24 @@
> > +/*
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + *
> > + * RISC-V RPMI internal service helpers.
> > + *
> > + * Copyright (c) 2026 Qualcomm Technologies, Inc.
> > + * Author:
> > + *  Subrahmanya Lingappa <[email protected]>
> > + */
> > +
> > +#ifndef HW_MISC_RISCV_RPMI_INTERNAL_H
> > +#define HW_MISC_RISCV_RPMI_INTERNAL_H
> > +
> > +#include "qapi/error.h"
> > +#include "hw/misc/riscv_rpmi.h"
> > +#include "librpmi.h"
> > +
> > +#define RPMI_PLAT_INFO "QEMU RISC-V RPMI"
> > +
> > +extern const struct rpmi_shmem_platform_ops rpmi_shmem_qemu_ops;
> > +bool riscv_rpmi_service_enabled(RiscvRpmiState *s,
> > +                                RiscvRpmiServiceKind kind);
> > +
> > +#endif
> > diff --git a/include/hw/misc/riscv_rpmi.h b/include/hw/misc/riscv_rpmi.h
> > new file mode 100644
> > index 0000000000..cb2658e57e
> > --- /dev/null
> > +++ b/include/hw/misc/riscv_rpmi.h
> > @@ -0,0 +1,90 @@
> > +/*
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + *
> > + * RISC-V RPMI definitions shared by RPMI transport and FDT helpers.
> > + *
> > + * Copyright (c) 2026 Qualcomm Technologies, Inc.
> > + * Author:
> > + *  Subrahmanya Lingappa <[email protected]>
> > + */
> > +
> > +#ifndef HW_MISC_RISCV_RPMI_H
> > +#define HW_MISC_RISCV_RPMI_H
> > +
> > +#include "exec/hwaddr.h"
> > +#include "hw/core/sysbus.h"
> > +#include "qom/object.h"
> > +#include "qemu/notify.h"
> > +
> > +#define RPMI_QUEUE_SLOT_SIZE 64
> > +#define RPMI_DBREG_SIZE      0x1000
> > +
> > +#define RPMI_ALL_NUM_QUEUES 4
> > +#define RPMI_A2P_NUM_QUEUES 2
> > +#define RPMI_ALL_NUM_REGS   (RPMI_ALL_NUM_QUEUES + 1)
> > +#define RPMI_A2P_NUM_REGS   (RPMI_A2P_NUM_QUEUES + 1)
> > +
> > +#define VIRT_RPMI_A2P_REQ_SIZE (16 * RPMI_QUEUE_SLOT_SIZE)
> > +#define VIRT_RPMI_P2A_REQ_SIZE 0
> > +
> > +
> > +#define TYPE_RISCV_RPMI "riscv-rpmi"
> > +OBJECT_DECLARE_SIMPLE_TYPE(RiscvRpmiState, RISCV_RPMI)
> > +
> > +struct rpmi_context;
> > +struct rpmi_service_group;
> > +struct rpmi_shmem;
> > +struct rpmi_transport;
> > +
> > +typedef enum RiscvRpmiServiceKind {
> > +    RISCV_RPMI_SERVICE_INVALID = 0,
> > +} RiscvRpmiServiceKind;
> > +
> > +typedef struct RiscvRpmiServiceConfig {
> > +    RiscvRpmiServiceKind kind;
> > +    const char *node_name;
> > +    const char *compatible;
> > +    uint32_t service_group;
> > +    bool has_mpxy_channel;
> > +    uint32_t mpxy_channel;
> > +} RiscvRpmiServiceConfig;
> > +
> > +typedef struct RiscvRpmiConfig {
> > +    hwaddr doorbell_base;
> > +    hwaddr shmem_base;
> > +    hwaddr shmem_size;
> > +    uint32_t a2p_req_size;
> > +    uint32_t p2a_req_size;
> > +    const char *platform_info;
> > +
> > +    const uint32_t *hart_ids;
> > +    uint32_t hart_count;
> > +    const RiscvRpmiServiceConfig *services;
> > +    uint32_t service_count;
> > +} RiscvRpmiConfig;
> > +
> > +struct RiscvRpmiState {
> > +    SysBusDevice parent_obj;
> > +
> > +    MemoryRegion mmio;
> > +    MemoryRegion shmem;
> > +    uint64_t shmem_base;
> > +    uint64_t shmem_size;
> > +    uint32_t a2p_req_size;
> > +    uint32_t p2a_req_size;
> > +    char *platform_info;
> > +
> > +    uint32_t *hart_ids;
> > +    uint32_t hart_count;
> > +    const RiscvRpmiServiceConfig *services;
> > +    uint32_t service_count;
> > +    uint32_t doorbell;
> > +    struct rpmi_shmem *rpmi_shmem;
> > +    struct rpmi_transport *transport;
> > +    struct rpmi_context *context;
> > +    bool has_shmem;
> > +};
> > +
> > +DeviceState *riscv_rpmi_create(const RiscvRpmiConfig *cfg, Error **errp);
> > +
> > +#endif
>