Re: [PATCH 6/8] hw/intc/riscv_aplic: add kvm_msicfgaddr for split mode aplic-imsic

[Alistair Francis]

On Fri, Oct 11, 2024 at 5:07 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> The last step to enable KVM AIA aplic-imsic with irqchip in split mode
> is to deal with how MSIs are going to be sent. In our current design we
> don't allow an APLIC controller to send MSIs unless it's on m-mode. And
> we also do not allow Supervisor MSI address configuration via the
> 'smsiaddrcfg' and 'smsiaddrcfgh' registers unless it's also a m-mode
> APLIC controller.
>
> Add a new RISCVACPLICState attribute called 'kvm_msicfgaddr'. This
> attribute represents the base configuration address for MSIs, in our
> case the base addr of the IMSIC controller. This attribute is being set
> only when running irqchip_split() mode with aia=aplic-imsic.
>
> During riscv_aplic_msi_send() we'll check if the attribute was set to
> skip the check for a m-mode APLIC controller and to change the resulting
> MSI addr by adding kvm_msicfgaddr right before address_space_stl_le().
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/intc/riscv_aplic.c         | 42 +++++++++++++++++++++++++++--------
>  hw/riscv/virt.c               |  6 ++++-
>  include/hw/intc/riscv_aplic.h |  6 +++++
>  3 files changed, 44 insertions(+), 10 deletions(-)
>
> diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
> index 0696e20ddf..4de458e395 100644
> --- a/hw/intc/riscv_aplic.c
> +++ b/hw/intc/riscv_aplic.c
> @@ -177,6 +177,16 @@ bool riscv_use_emulated_aplic(bool msimode)
>  #endif
>  }
>
> +void riscv_aplic_set_kvm_msicfgaddr(RISCVAPLICState *aplic, hwaddr addr)
> +{
> +#ifdef CONFIG_KVM
> +    if (riscv_use_emulated_aplic(aplic->msimode)) {
> +        aplic->kvm_msicfgaddr = extract64(addr, 0, 32);
> +        aplic->kvm_msicfgaddrH = extract64(addr, 32, 32);
> +    }
> +#endif
> +}
> +
>  static bool riscv_aplic_irq_rectified_val(RISCVAPLICState *aplic,
>                                            uint32_t irq)
>  {
> @@ -377,13 +387,16 @@ static void riscv_aplic_msi_send(RISCVAPLICState *aplic,
>      uint32_t lhxs, lhxw, hhxs, hhxw, group_idx, msicfgaddr, msicfgaddrH;
>
>      aplic_m = aplic;
> -    while (aplic_m && !aplic_m->mmode) {
> -        aplic_m = aplic_m->parent;
> -    }
> -    if (!aplic_m) {
> -        qemu_log_mask(LOG_GUEST_ERROR, "%s: m-level APLIC not found\n",
> -                      __func__);
> -        return;
> +
> +    if (!aplic->kvm_splitmode) {
> +        while (aplic_m && !aplic_m->mmode) {
> +            aplic_m = aplic_m->parent;
> +        }
> +        if (!aplic_m) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: m-level APLIC not found\n",
> +                          __func__);
> +            return;
> +        }
>      }
>
>      if (aplic->mmode) {
> @@ -415,6 +428,11 @@ static void riscv_aplic_msi_send(RISCVAPLICState *aplic,
>      addr |= (uint64_t)(guest_idx & APLIC_xMSICFGADDR_PPN_HART(lhxs));
>      addr <<= APLIC_xMSICFGADDR_PPN_SHIFT;
>
> +    if (aplic->kvm_splitmode) {
> +        addr |= aplic->kvm_msicfgaddr;
> +        addr |= ((uint64_t)aplic->kvm_msicfgaddrH << 32);
> +    }
> +
>      address_space_stl_le(&address_space_memory, addr,
>                           eiid, MEMTXATTRS_UNSPECIFIED, &result);
>      if (result != MEMTX_OK) {
> @@ -888,6 +906,10 @@ static void riscv_aplic_realize(DeviceState *dev, Error 
> **errp)
>          memory_region_init_io(&aplic->mmio, OBJECT(dev), &riscv_aplic_ops,
>                                aplic, TYPE_RISCV_APLIC, aplic->aperture_size);
>          sysbus_init_mmio(SYS_BUS_DEVICE(dev), &aplic->mmio);
> +
> +        if (kvm_enabled()) {
> +            aplic->kvm_splitmode = true;
> +        }
>      }
>
>      /*
> @@ -935,8 +957,8 @@ static Property riscv_aplic_properties[] = {
>
>  static const VMStateDescription vmstate_riscv_aplic = {
>      .name = "riscv_aplic",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>      .fields = (const VMStateField[]) {
>              VMSTATE_UINT32(domaincfg, RISCVAPLICState),
>              VMSTATE_UINT32(mmsicfgaddr, RISCVAPLICState),
> @@ -944,6 +966,8 @@ static const VMStateDescription vmstate_riscv_aplic = {
>              VMSTATE_UINT32(smsicfgaddr, RISCVAPLICState),
>              VMSTATE_UINT32(smsicfgaddrH, RISCVAPLICState),
>              VMSTATE_UINT32(genmsi, RISCVAPLICState),
> +            VMSTATE_UINT32(kvm_msicfgaddr, RISCVAPLICState),
> +            VMSTATE_UINT32(kvm_msicfgaddrH, RISCVAPLICState),
>              VMSTATE_VARRAY_UINT32(sourcecfg, RISCVAPLICState,
>                                    num_irqs, 0,
>                                    vmstate_info_uint32, uint32_t),
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 39fd9b7c3e..e5202bad10 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1221,7 +1221,7 @@ static DeviceState *virt_create_aia(RISCVVirtAIAType 
> aia_type, int aia_guests,
>                                      int base_hartid, int hart_count)
>  {
>      int i;
> -    hwaddr addr;
> +    hwaddr addr = 0;
>      uint32_t guest_bits;
>      DeviceState *aplic_s = NULL;
>      DeviceState *aplic_m = NULL;
> @@ -1271,6 +1271,10 @@ static DeviceState *virt_create_aia(RISCVVirtAIAType 
> aia_type, int aia_guests,
>                                   VIRT_IRQCHIP_NUM_PRIO_BITS,
>                                   msimode, false, aplic_m);
>
> +    if (kvm_enabled() && msimode) {
> +        riscv_aplic_set_kvm_msicfgaddr(RISCV_APLIC(aplic_s), addr);
> +    }
> +
>      return kvm_enabled() ? aplic_s : aplic_m;
>  }
>
> diff --git a/include/hw/intc/riscv_aplic.h b/include/hw/intc/riscv_aplic.h
> index 74ae5d87b5..489b9133c2 100644
> --- a/include/hw/intc/riscv_aplic.h
> +++ b/include/hw/intc/riscv_aplic.h
> @@ -68,11 +68,17 @@ struct RISCVAPLICState {
>      uint32_t num_irqs;
>      bool msimode;
>      bool mmode;
> +
> +    /* To support KVM aia=aplic-imsic with irqchip split mode */
> +    bool kvm_splitmode;
> +    uint32_t kvm_msicfgaddr;
> +    uint32_t kvm_msicfgaddrH;
>  };
>
>  void riscv_aplic_add_child(DeviceState *parent, DeviceState *child);
>  bool riscv_is_kvm_aia_aplic_imsic(bool msimode);
>  bool riscv_use_emulated_aplic(bool msimode);
> +void riscv_aplic_set_kvm_msicfgaddr(RISCVAPLICState *aplic, hwaddr addr);
>
>  DeviceState *riscv_aplic_create(hwaddr addr, hwaddr size,
>      uint32_t hartid_base, uint32_t num_harts, uint32_t num_sources,
> --
> 2.45.2
>
>