Re: [PATCH 5/8] hw/riscv/virt.c, riscv_aplic.c: add 'emulated_aplic' helpers

[Alistair Francis]

On Fri, Oct 11, 2024 at 5:05 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> The current logic to determine if we don't need an emulated APLIC
> controller, i.e. KVM will provide for us, is to determine if we're
> running KVM, with in-kernel irqchip support, and running
> aia=aplic-imsic. This is modelled by riscv_is_kvm_aia_aplic_imsic() and
> virt_use_kvm_aia_aplic_imsic().
>
> This won't suffice to support irqchip_split() mode: it will match
> exactly the same conditions as the one above, but setting the irqchip to
> 'split' mode will now require us to emulate an APLIC s-mode controller,
> like we're doing with 'aia=aplic'.
>
> Create a new riscv_use_emulated_aplic() helper that will encapsulate
> this logic. Replace the uses of "riscv_is_kvm_aia_aplic_imsic()" with
> this helper every time we're taking a decision on emulate an APLIC
> controller or not. Do the same in virt.c with virt_use_emulated_aplic().
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

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

Alistair

> ---
>  hw/intc/riscv_aplic.c         | 24 +++++++++++++++++++++---
>  hw/riscv/virt.c               | 14 ++++++++++++--
>  include/hw/intc/riscv_aplic.h |  1 +
>  3 files changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
> index 20de8c63a2..0696e20ddf 100644
> --- a/hw/intc/riscv_aplic.c
> +++ b/hw/intc/riscv_aplic.c
> @@ -32,6 +32,7 @@
>  #include "target/riscv/cpu.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/kvm.h"
> +#include "sysemu/tcg.h"
>  #include "kvm/kvm_riscv.h"
>  #include "migration/vmstate.h"
>
> @@ -159,6 +160,23 @@ bool riscv_is_kvm_aia_aplic_imsic(bool msimode)
>      return kvm_irqchip_in_kernel() && msimode;
>  }
>
> +bool riscv_use_emulated_aplic(bool msimode)
> +{
> +#ifdef CONFIG_KVM
> +    if (tcg_enabled()) {
> +        return true;
> +    }
> +
> +    if (!riscv_is_kvm_aia_aplic_imsic(msimode)) {
> +        return true;
> +    }
> +
> +    return kvm_kernel_irqchip_split();
> +#else
> +    return true;
> +#endif
> +}
> +
>  static bool riscv_aplic_irq_rectified_val(RISCVAPLICState *aplic,
>                                            uint32_t irq)
>  {
> @@ -853,7 +871,7 @@ static void riscv_aplic_realize(DeviceState *dev, Error 
> **errp)
>      uint32_t i;
>      RISCVAPLICState *aplic = RISCV_APLIC(dev);
>
> -    if (!riscv_is_kvm_aia_aplic_imsic(aplic->msimode)) {
> +    if (riscv_use_emulated_aplic(aplic->msimode)) {
>          aplic->bitfield_words = (aplic->num_irqs + 31) >> 5;
>          aplic->sourcecfg = g_new0(uint32_t, aplic->num_irqs);
>          aplic->state = g_new0(uint32_t, aplic->num_irqs);
> @@ -877,7 +895,7 @@ static void riscv_aplic_realize(DeviceState *dev, Error 
> **errp)
>       * have IRQ lines delegated by their parent APLIC.
>       */
>      if (!aplic->parent) {
> -        if (kvm_enabled() && riscv_is_kvm_aia_aplic_imsic(aplic->msimode)) {
> +        if (kvm_enabled() && !riscv_use_emulated_aplic(aplic->msimode)) {
>              qdev_init_gpio_in(dev, riscv_kvm_aplic_request, aplic->num_irqs);
>          } else {
>              qdev_init_gpio_in(dev, riscv_aplic_request, aplic->num_irqs);
> @@ -1021,7 +1039,7 @@ DeviceState *riscv_aplic_create(hwaddr addr, hwaddr 
> size,
>
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>
> -    if (!riscv_is_kvm_aia_aplic_imsic(msimode)) {
> +    if (riscv_use_emulated_aplic(msimode)) {
>          sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
>      }
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index f1bdc1c535..39fd9b7c3e 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -64,6 +64,13 @@ static bool virt_use_kvm_aia_aplic_imsic(RISCVVirtAIAType 
> aia_type)
>      return riscv_is_kvm_aia_aplic_imsic(msimode);
>  }
>
> +static bool virt_use_emulated_aplic(RISCVVirtAIAType aia_type)
> +{
> +    bool msimode = aia_type == VIRT_AIA_TYPE_APLIC_IMSIC;
> +
> +    return riscv_use_emulated_aplic(msimode);
> +}
> +
>  static bool virt_aclint_allowed(void)
>  {
>      return tcg_enabled() || qtest_enabled();
> @@ -776,8 +783,11 @@ static void create_fdt_sockets(RISCVVirtState *s, const 
> MemMapEntry *memmap,
>          *msi_pcie_phandle = msi_s_phandle;
>      }
>
> -    /* KVM AIA aplic-imsic only has one APLIC instance */
> -    if (kvm_enabled() && virt_use_kvm_aia_aplic_imsic(s->aia_type)) {
> +    /*
> +     * With KVM AIA aplic-imsic, using an irqchip without split
> +     * mode, we'll use only one APLIC instance.
> +     */
> +    if (!virt_use_emulated_aplic(s->aia_type)) {
>          create_fdt_socket_aplic(s, memmap, 0,
>                                  msi_m_phandle, msi_s_phandle, phandle,
>                                  &intc_phandles[0], xplic_phandles,
> diff --git a/include/hw/intc/riscv_aplic.h b/include/hw/intc/riscv_aplic.h
> index fd0e6427d9..74ae5d87b5 100644
> --- a/include/hw/intc/riscv_aplic.h
> +++ b/include/hw/intc/riscv_aplic.h
> @@ -72,6 +72,7 @@ struct RISCVAPLICState {
>
>  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);
>
>  DeviceState *riscv_aplic_create(hwaddr addr, hwaddr size,
>      uint32_t hartid_base, uint32_t num_harts, uint32_t num_sources,
> --
> 2.45.2
>
>