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
