On 11/4/24 8:14 AM, Yong-Xuan Wang wrote:
Hi Daniel and Andrew,

When handling an external interrupt via IMSIC, we need to use the stopei CSR
to claim the top interrupt. Even though the QEMU can emulate the IMSIC devices
without the in-kernel irqchip, we still need to trap and emulate the stopei
CSR. But since the host machine doesn't support the AIA extension, the guest OS
will hit the illegal instruction exception instead of the virutal instruction
exception when it accesses the stopei CSR. We can't have a chance to redirect
this instruction to QEMU. So I think we can directly report errors when the
user wants to use KVM AIA(MSI) without in-kernel AIA irqchip support.

Can you please add this info in the commit message? This makes it clearer
that there's not much we can do in QEMU aside from erroring out.

Also, please add a:

Fixes: 48c2c33c52 ("target/riscv: select KVM AIA in riscv virt machine")


Thanks,

Daniel


Regards,
Yong-Xuan

On Fri, Nov 1, 2024 at 11:09 PM Andrew Jones <ajo...@ventanamicro.com> wrote:

On Fri, Nov 01, 2024 at 08:45:13AM -0300, Daniel Henrique Barboza wrote:


On 11/1/24 5:36 AM, Yong-Xuan Wang wrote:
Currently QEMU will continue to emulate the AIA MSI devices and enable the
AIA extension for guest OS when the host kernel doesn't support the
in-kernel AIA irqchip. This will cause an illegal instruction exception
when the guest OS uses the IMSIC devices. Add additional checks to ensure
the guest OS only uses the AIA MSI device when the host kernel supports
the in-kernel AIA chip.

Signed-off-by: Yong-Xuan Wang <yongxuan.w...@sifive.com>
Reviewed-by: Jim Shu <jim....@sifive.com>
---
   hw/riscv/virt.c | 19 +++++++++++++------
   1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 45a8c4f8190d..0d8e047844a6 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1567,12 +1567,19 @@ static void virt_machine_init(MachineState *machine)
           }
       }
-    if (kvm_enabled() && virt_use_kvm_aia(s)) {
-        kvm_riscv_aia_create(machine, IMSIC_MMIO_GROUP_MIN_SHIFT,
-                             VIRT_IRQCHIP_NUM_SOURCES, VIRT_IRQCHIP_NUM_MSIS,
-                             memmap[VIRT_APLIC_S].base,
-                             memmap[VIRT_IMSIC_S].base,
-                             s->aia_guests);
+    if (kvm_enabled() && s->aia_type == VIRT_AIA_TYPE_APLIC_IMSIC) {
+        if (virt_use_kvm_aia(s)) {
+            kvm_riscv_aia_create(machine, IMSIC_MMIO_GROUP_MIN_SHIFT,
+                                 VIRT_IRQCHIP_NUM_SOURCES,
+                                 VIRT_IRQCHIP_NUM_MSIS,
+                                 memmap[VIRT_APLIC_S].base,
+                                 memmap[VIRT_IMSIC_S].base,
+                                 s->aia_guests);
+        } else {
+            error_report("Host machine doesn't support in-kernel APLIC MSI, "
+                         "please use aia=none or aia=aplic");
+            exit(1);
+        }

As you said in the commit msg it looks like we have a bug in this particular 
path: kvm accel,
aia=aplic-imsic, no irqchip present. Erroring out is one possible solution but 
I wonder why we
couldn't just emulate the APLIC and IMSIC controllers in this case. We have 
code that does
that in TCG, so it would be a matter of adding the needed plumbing to treat KVM 
AIA without
irqchip == TCG AIA.

Drew, care to weight in? Thanks,


If I understand the motivation for this patch correctly, then we'll always
need something like it anyway. With the proposal of supporting KVM with
usermode-imsic, then KVM would ultimately have three possible states:
inkernel-irqchip, usermode-imsic, nothing. usermode-imsic will need KVM
support for forwarding imsic accesses to QEMU, but when that support isn't
present (and the inkernel-irqchip isn't selected either), then we should
still want to error out before allowing the guest to try accesses that
can't work.

Thanks,
drew

Reply via email to