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

[Peter Maydell]

On 21 July 2012 11:22, Jan Kiszka <address@hidden> wrote:
> On 2012-07-21 11:56, Peter Maydell wrote:
>> Or are you trying to talk about defining interrupt routes when the
>> source and destination are both kernel code but the route needs to
>> be set by userspace (ie is machine specific not cpu specific)?
>
> It describes this requirement primarily.
>
>> Whether that's possible sounds to me like it would depend on all
>> the board model code between the source and destination rather
>> than being a single global boolean check.
>
> It depends on the feature set of the in-kernel irqchips and if this can
> possibly vary on real hw.

If the interrupt route is on-CPU then its routing is fixed (for
that CPU), and you don't need to care about irqfds because the
kernel knows what CPU it's providing to the guest, has both ends
of the connection and can just do the right thing however is most
convenient for the internal implementation. If the interrupt route
is off-CPU then all bets are off because the routing is machine
specific and could go through any kind of logic between the peripheral
and the CPU's irqchip.

I don't see how you can do this with QEMU's current IRQ infrastructure,
which basically just hardwires everything with qemu_irq lines and
doesn't provide any way to query the routing and logic from an
irq source to its destination.

>>>>>> But you can perfectly well have an in-kernel-irqchip that doesn't
>>>>>> support irqfd
>>>>>
>>>>> You could, thought this doesn't make much sense.
>>>>
>>>> Why doesn't it make sense? On ARM, in-kernel-irqchip means you can take
>>>> advantage of the hardware support for a virtual GIC, and you can use
>>>> the virtual timer support too. These are both big performance advantages
>>>> even if QEMU never does anything with irqfds. (In fact the current
>>>> ARM KVM VGIC code doesn't support irqfds as far as I can see from
>>>> a quick scan of the kernel code.)
>>>
>>> It doesn't make sense as it means your in-kernel irqchip model is
>>> semi-finished. If you didn't consider how to support direct in-kernel
>>> IRQ injections, you risk designing something that requires userspace
>>> quirk handling later on when extending it to full-featured in-kernel
>>> irqchip support.
>>
>> Well, the in-kernel virtual timer already does direct in-kernel IRQ
>> injection to the VGIC: it calls a function to say "inject IRQ X"...
>> (this works because the interrupt line used is fixed by the CPU,
>> it's not a board model property so there is no need for the routing
>> to be defined by userspace. (ie both ends of this irq injection are
>> in the CPU proper.))
>
> Could you inject IRQs from an in-kernel helper that (partially or fully)
> emulates some device sitting on peripheral buses as well (like PCI)? If
> not, you aren't done with the in-kernel irqchip model yet.

This is still sounding like "there is an extra feature which you should
probably implement at some point and should certainly design with the
intention of supporting", not "you cannot have an irqchip without irqfds".

Therefore QEMU code which cares about irqfds should be doing
checks for irqfd functionality, not "is there an in kernel
irqchip".

>> As far as I can tell you seem to be saying "irqfds are an extra
>> feature you might want later", which is exactly "you can have
>> an irqchip without them".
>
> Once the prerequisites for peripheral interrupt injections are there,
> enabling irqfd for your arch should be straightforward. I'm picking on
> those prerequisites here, not irqfd.
>
>>
>> (Is there some summary of the design of the irqfds stuff somewhere I
>> can go and read up?)
>
> linux/Documentation/virtual/kvm/api.txt is a good start, though not
> really a high-level summary.

I looked for 'irqfd' in that and found a straightforward ioctl for
"wire this eventfd up to this irqchip input". That doesn't say anything
about remapping of IRQs, and it's not clear to me either why a
straightforward "use an ioctl to deliver incoming interrupts" design
would be broken by adding that later: it's just a different source
for the interrupt...

-- PMM