Re: [Qemu-devel] [PATCH] kvm: Move kvm_allows_irq0_override() to target-i386

[Peter Maydell]

On 23 July 2012 17:55, Jan Kiszka <address@hidden> wrote:
> On 2012-07-23 17:19, Peter Maydell wrote:
>> OK, let's see if we can get some agreement about naming here.
>>
>> First, some test-functions I think we definitely need:
>>
>>  kvm_interrupts_are_async()
>>    -- true if interrupt delivery is asynchronous
>>       default false in kvm_init, set true in kvm_irqchip_create,
>>       architectures may set it true in kvm_arch_init [ARM will
>>       do so; PPC might want to do so]
>>
>>  kvm_irqchip_in_kernel()
>>    -- the user-settable option, actual behaviour is arch specific
>>       on x86, true means (as it does now) LAPIC,IOAPIC,PIT in kernel
>>       on ARM, we ignore this setting and just DTRT
>
> You should reject kernel_irqchip=off as long as you only support an
> in-kernel GIC model.

Agreed. (At the moment we still have code in QEMU for out-of-kernel
GIC models, which is legacy from before the VGIC implementation;
depending on whether that actually vanishes from the kernel ABI
or not I may include the QEMU code which makes arm_pic.c handle
injecting interrupts to KVM via ioctl. But in that case we would
reject =off for A15 and =on for non-A15 (slightly pointlessly
since non-A15 will fail the "must be an A15" check.))

>>       on PPC, used as a convenience setting for whether to use
>>       an in-kernel model of the interrupt controller
>>       Shouldn't be used in non-target-specific code
>>
>> and two I'm not quite so sure about:
>>
>>  kvm_has_msi_routing()
>>    -- true if we can do routing of MSIs
>
> GSI, not MSI.
>
>>       set true only if x86 and kvm_irqchip_in_kernel
>
> It means that the target architecture supports routing of various
> interrupt sources (userspace, irqfds, in-kernel device models) to
> different in-kernel IRQ sinks (CPU cores, irqchip models, whatever).
> Interrupt messages via (binary-state) irqfd depend on it.

So I still don't actually understand what this interrupt routing
is (as far as the kernel is concerned). Surely the way this should
work is that you use KVM_IRQFD to say "this fd goes to this
interrupt sink" (with some sensible scheme for encoding what the
interrupt destination actually is, like an (irqchip,pin) tuple
or something). Why does the kernel need to care about irq
routing?

>> kvm-all.c:kvm_irqchip_set_irq():
>>  -- (just an assert) should be kvm_interrupts_are_async()
>
> The name kvm_irqchip_set_irq implies so far that it injects into an
> in-kernel irqchip model. Either different functions for archs that don't
> follow this concept need to be provided, or this function requires
> renaming (kvm_set_irq_async or so).

Yes, renaming the function would probably be reasonable.
(Currently my QEMU code for using the VGIC just does a direct
ioctl itself though, because kvm_irqchip_set_irq() doesn't actually
do very much and it's a bit awkward that it insists on fishing
the ioctl number out from the KVMState.)

>> kvm-all.c:kvm_irqchip_assign_irqfd():
>>  -- should be true if kvm_has_irqfds()
>
> The same issue. Plus there is that naming conflict again if we should
> ever see irqfd without some in-kernel irqchip. But even s390 would have
> a logical "irqchip" for me at the point it may route interrupt messages
> from devices directly to the CPUs.

I don't think you can have irqfds without an irqchip because where
would you be sending the interrupts?

-- PMM