Hi Daniel and Andrew,
Sorry I found that I forgot a situation. Host kernel doesn't support
in-kernel AIA is not equal to host machine doesn't support AIA extension.
If user specifies aia=aplic-imsic when using KVM acceleration, we have 3
possibilities:
1. host doesn't support AIA extension -> report error since we can't handle
the stopei CSR.
2. host support AIA extension but doesn't have in-kernel AIA -> use usermode
IMSIC and handle the stopei CSR in QEMU
3. host support AIA extension and have in-kernel AIA -> use in-kernel AIA
and handle the stopei CSR in KVM
We need to update the kvm_riscv_handle_csr() for situation 2. And it's better
to determine the availability of AIA extension in riscv_imsic_realize().
Please ignore this patch, I will send another patchset to handle the
above situations.
Regards,
Yong-Xuan
On Mon, Nov 4, 2024 at 8:15 PM Daniel Henrique Barboza
<dbarb...@ventanamicro.com> wrote:
>
>
>
> 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
