Re: [PATCH v8 05/10] riscv: add RPMI system reset service

On Fri, Aug 14, 2026 at 10:24 AM Subrahmanya Lingappa
<[email protected]> wrote:
>
> Add a QEMU implementation of the RPMI System Reset service group.
>
> The service advertises shutdown and cold reboot reset types, validates
> guest requests, and dispatches accepted requests through the machine
> reset and shutdown callbacks. This lets firmware use RPMI reset commands
> while keeping the board-specific reset policy in the RISC-V virt machine.
>
> Signed-off-by: Subrahmanya Lingappa <[email protected]>
> Reviewed-by: Daniel Henrique Barboza <[email protected]>
> ---
>  hw/misc/meson.build           |   1 +
>  hw/misc/riscv_rpmi.c          |  70 +++++++++++-
>  hw/misc/riscv_rpmi_internal.h |  12 +-
>  hw/misc/riscv_rpmi_sysreset.c | 206 ++++++++++++++++++++++++++++++++++
>  hw/riscv/virt.c               |  51 ++++++++-
>  include/hw/misc/riscv_rpmi.h  |  15 +--
>  include/hw/riscv/rpmi-fdt.h   |  11 ++
>  7 files changed, 349 insertions(+), 17 deletions(-)
>  create mode 100644 hw/misc/riscv_rpmi_sysreset.c
>
> diff --git a/hw/misc/meson.build b/hw/misc/meson.build
> index 92c9cd6064..482f56504e 100644
> --- a/hw/misc/meson.build
> +++ b/hw/misc/meson.build
> @@ -170,4 +170,5 @@ system_ss.add(when: 'CONFIG_SBSA_REF', if_true: files('sbsa_ec.c'))
>  system_ss.add(when: 'CONFIG_LASI', if_true: files('lasi.c'))
>  system_ss.add(when: 'CONFIG_RISCV_RPMI', if_true: [files(
>    'riscv_rpmi.c',
> +  'riscv_rpmi_sysreset.c',
>  ), librpmi], if_false: files('riscv_rpmi-stub.c'))
> diff --git a/hw/misc/riscv_rpmi.c b/hw/misc/riscv_rpmi.c
> index 54a790fc7c..9575cd3a8d 100644
> --- a/hw/misc/riscv_rpmi.c
> +++ b/hw/misc/riscv_rpmi.c
> @@ -171,10 +171,37 @@ static bool riscv_rpmi_transport_indices_valid(RiscvRpmiState *s)
>                                            s->a2p_req_size);
>  }
>
> +typedef struct RiscvRpmiServiceOps {
> +    uint32_t service_group;
> +    bool (*add)(RiscvRpmiState *s, Error **errp);
> +    void (*remove)(RiscvRpmiState *s);
> +} RiscvRpmiServiceOps;
> +
> +static const RiscvRpmiServiceOps riscv_rpmi_service_ops[] = {
> +    {
> +        .service_group = RISCV_RPMI_SRVGRP_SYSTEM_RESET,
> +        .add = riscv_rpmi_sysreset_add,
> +        .remove = riscv_rpmi_sysreset_remove,
> +    },
> +};
> +
> +static const RiscvRpmiServiceOps *riscv_rpmi_service_ops_by_group(
> +    uint32_t service_group)
> +{
> +    for (uint32_t i = 0; i < ARRAY_SIZE(riscv_rpmi_service_ops); i++) {
> +        if (riscv_rpmi_service_ops[i].service_group == service_group) {
> +            return &riscv_rpmi_service_ops[i];
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
>  static void riscv_rpmi_configure_base(RiscvRpmiState *s,
>                                        const RiscvRpmiConfig *cfg)
>  {
>      s->platform_info = g_strdup(cfg->platform_info);
> +    s->machine_ops = cfg->machine_ops;
>      s->services = cfg->services;
>      s->service_count = cfg->service_count;
>
> @@ -211,7 +238,9 @@ static void riscv_rpmi_reset_hold(Object *obj, ResetType type)
>
>  static void riscv_rpmi_cleanup(RiscvRpmiState *s)
>  {
> -
> +    for (uint32_t i = ARRAY_SIZE(riscv_rpmi_service_ops); i > 0; i--) {
> +        riscv_rpmi_service_ops[i - 1].remove(s);
> +    }
>
>      if (s->context) {
>          rpmi_context_destroy(s->context);
> @@ -234,12 +263,12 @@ static void riscv_rpmi_cleanup(RiscvRpmiState *s)
>      }
>  }
>
> -bool riscv_rpmi_service_enabled(RiscvRpmiState *s, RiscvRpmiServiceKind kind)
> +bool riscv_rpmi_service_enabled(RiscvRpmiState *s, uint32_t service_group)
>
Could you squash this change with the original commit introducing
this? Also, I think it is better to have the framework changes in a
separate commit and then machine specific code to enable the service
in another.

Thanks,
Sunil