Re: [RFC 0/6] KVM: arm/arm64: gsi routing support

[Eric Auger <eric.auger@linaro.org>]

Andre,
On 06/23/2015 11:03 AM, Andre Przywara wrote:
> Hi Eric,
> 
> I went back reading the code and looked at how the x86 APIC works more
> closely to understand the GSI routing better.
> See below for more ...
> 
> On 22/06/15 10:21, Eric Auger wrote:
>> On 06/22/2015 10:40 AM, Andre Przywara wrote:
>>> Hi Eric,
>>>
>>> I briefly looked over the series, the patches itself look good overall.
>>> I have one or two comments on the actual code, but want to discuss the
>>> general approach first (more a dump of some first thoughts):
>>>
>>> On 18/06/15 18:40, Eric Auger wrote:
>>>> With the advent of GICv3 ITS in-kernel emulation, KVM GSI routing
>>>> appears to be requested. More specifically MSI routing is needed.
>>>> irqchip routing does not sound to be really useful on arm but usage of
>>>> MSI routing also mandates to integrate irqchip routing. The initial
>>>> implementation of irqfd on arm must be upgraded with the integration
>>>> of kvm irqchip.c code and the implementation of its standard hooks
>>>> in the architecture specific part.
>>>>
>>>> The series therefore allows and mandates the usage of KVM_SET_GSI_ROUTING
>>>> ioctl along with KVM_IRQFD. If the userspace does not define any routing
>>>> table, no irqfd injection can happen. The user-space can use
>>>> KVM_CAP_IRQ_ROUTING to detect whether a routing table is needed.
>>>>
>>>> for irqchip routing, the convention is, only SPI can be injected and the
>>>> SPI ID corresponds to irqchip.pin + 32. For MSI routing the interrupt ID
>>>> matches the MSI msg data. API evolve to support associating a device ID
>>>> to a routine entry.
>>>
>>> So if I get this right, in a guest ITS case we have now three different
>>> IRQ name spaces:
>>> a) the LPI number, which is guest internal. The ITS driver in the guest
>>> maintains it. We can track assignments and changes when handling the
>>> MAPVI command in the host kernel, but this would stay in the kernel, as
>>> I don't see an efficient way of propagating this to userland.
>>> b) the GSI number, which is used in communication between userland and
>>> the host kernel. The guest kernel does not know about this at all. Also
>>> the ioctl requires us to set the routing for _all_ GSIs, and I read it
>>> that it assumes starting at GSI 0.
>> all injected GSI must effectively have a routing entry in KVM. Starting
>> at 0 that's not requested. At qemu level there's just the constaint gsi
>> fits between [0, max route number].
> 
> Yeah, you are right, I somehow missed that each routing entry has a gsi
> field in it. So we have to allocate all of them at once with one ioctl,
> but they can be sparse.
> 
>>  So we cannot even pretend to have
>>> LPIs here, because we would need at least 8192 empty entries then, not
>>> to speak of the possibly sparse allocation above. So we have a
>>> completely distinct name space here.
>> What is done currently at qemu level for other archs -  if I understand
>> it correctly -  is there is static GSI routing for standard IRQ. For MSI
>> irqfd setup they use spare gsi number not yet used for GSI routing < max
>> route number. So this is sparse for MSI but not for standard IRQs.
>> Effectively we do not plan to have GSI routing for LPIs but only MSI
>> routing.
> 
> That seems to make sense to me. Since we already limit the number of
> SPIs to something sensible with our KVM_DEV_ARM_VGIC_GRP_NR_IRQS, we
> could infer an implicit direct routing for those SPIs. KVM could check
> the IRQ number against vgic.nr_irqs to see whether an IRQ is routed or not.
> Any GSI beyond that number would be an MSI with your enhanced DevID:EvID
> pair in it, which gets injected via the ITS emulation code (or the
> respective GICv2m code).
> 
> That would be the idea, but if it turns out that not routing SPIs but
> only MSIs requires too many changes to the (core) KVM code (haven't
> looked yet), we could require routing entries for SPIs as well.
Well you were right ;-) I forgot the fact KVM_SET_GSI_ROUTING overwrites
the existing routing table with new user-space provided entries and it's
not possible to keep any hidden irqchip routing entries in place without
the user-space being aware of them.

What I can simply do is preventing any non identity mapping for irqchip
routing entries, ie. gsi != irqchip.pin. This fixes the problem of
KVM_IRQ_LINE. This check can happen in arm kvm_set_routing_entry() and
that's it.

Best Regards

Eric
> After all that's what for instance kvmtool sets up for x86, creating
> default 1:1 mappings for ISA and low PIC IRQs and allocating MSIs on
> demand after that.
> 
>>> c) The DevID:EvID pair, which actually identifies an IRQ in all the
>>> three regimes and is the only authoritative ID.
>>>
>>> So that means we need to maintain the connection between all the three,
>>> somehow duplicating the whole ITS mapping again to map GSIs to DevID:EvID.
>> Currently the KVM routing table indeed stores GSI/DevID:EvID mapping.
>>>
>>> So I wonder if we could use DevID:EvID directly.
>>> The KVM_IRQFD ioctl struct has some space, so we could put the DevID
>>> into the pad area.
>>> Also (more forward-looking) KVM_CAP_ASSIGN_DEV_IRQ identifies guest IRQs
>>> by an u32, but again there is quite some padding area available.
> 
>> ASSIGN_DEV_IRQ is a deprecated feature. We should not use that API I think.
> 
> OK, so do we have other users of the GSI routing beside IRQFD then?
> 
> I will go ahead and try to implement some code matching Eric's patches
> in kvmtool to test the GSI routing.
> 
> Eric, how did you test the irqchip routing on the Midway?
> 
> Cheers,
> Andre.
> 
>> Eric
>>>
>>> In general I am a bit reluctant to introduce just another level of
>>> complexity to the already quite convoluted way of doing IRQs and MSIs on
>>> ARM(64), that's why I will investigate if we can use DevID:EvID to refer
>>> to an interrupt.
>>>
>>> So far,
>>> Andre.
>>>
>>>>
>>>> Known Issues of this RFC:
>>>>
>>>> - One of the biggest is the API inconsistencies on ARM. Blame me.
>>>>   Routing should apply to KVM_IRQ_LINE ioctl which is not the case yet
>>>>   in this series. It only applies to irqfd.
>>>>   on x86 typically this KVM_IRQ_LINE is plugged onto irqchip.c kvm_set_irq
>>>>   whereas on ARM we inject directly through kvm_vgic_inject_irq
>>>>   x on arm/arm64 gsi has a specific structure:
>>>>     bits:  | 31 ... 24 | 23  ... 16 | 15    ...    0 |
>>>>     field: | irq_type  | vcpu_index |     irq_id     |
>>>>     where irq_id matches the Interrupt ID
>>>> - for KVM_IRQFD without routing (current implementation) the gsi field
>>>>   corresponds to an SPI index = irq_id (above) -32.
>>>> - as far as understand qemu integration, gsi is supposed to be within
>>>>   [0, KVM_MAX_IRQ_ROUTES]. Difficult to use KVM_IRQ_LINE gsi.
>>>> - to be defined what we choose as a convention with irqchip routing is
>>>>   applied: gsi -> irqchip input pin.
>>>> - Or shouldn't we simply rule out any userspace irqchip routing and stick
>>>>   to MSI routing? we could define a fixed identity in-kernel irqchip mapping
>>>>   and only offer MSI routing.
>>>> - static allocation of chip[KVM_NR_IRQCHIPS][KVM_IRQCHIP_NUM_PINS];
>>>>   arbitrary put KVM_IRQCHIP_NUM_PINS = 1020 - 32 (SPI count). On s390
>>>>   this is even bigger.
>>>>
>>>> Currently tested on irqchip routing only (Calxeda midway only),
>>>> ie NOT TESTED on MSI routing yet.
>>>>
>>>> This is a very preliminary RFC to ease the discussion.
>>>>
>>>> Code can be found at https://git.linaro.org/people/eric.auger/linux.git/shortlog/refs/heads/v4.1-rc8-gsi-routing-rfc
>>>>
>>>> It applies on Andre's [PATCH 00/13] arm64: KVM: GICv3 ITS emulation
>>>> (http://www.spinics.net/lists/kvm/msg117402.html)
>>>>
>>>> Eric Auger (6):
>>>>   KVM: api: add kvm_irq_routing_extended_msi
>>>>   KVM: kvm_host: add kvm_extended_msi
>>>>   KVM: irqchip: convey devid to kvm_set_msi
>>>>   KVM: arm/arm64: enable irqchip routing
>>>>   KVM: arm/arm64: enable MSI routing
>>>>   KVM: arm: implement kvm_set_msi by gsi direct mapping
>>>>
>>>>  Documentation/virtual/kvm/api.txt | 20 ++++++--
>>>>  arch/arm/include/asm/kvm_host.h   |  2 +
>>>>  arch/arm/kvm/Kconfig              |  3 ++
>>>>  arch/arm/kvm/Makefile             |  2 +-
>>>>  arch/arm64/include/asm/kvm_host.h |  1 +
>>>>  arch/arm64/kvm/Kconfig            |  2 +
>>>>  arch/arm64/kvm/Makefile           |  2 +-
>>>>  include/kvm/arm_vgic.h            |  9 ----
>>>>  include/linux/kvm_host.h          | 10 ++++
>>>>  include/uapi/linux/kvm.h          |  9 ++++
>>>>  virt/kvm/arm/vgic.c               | 96 +++++++++++++++++++++++++++------------
>>>>  virt/kvm/irqchip.c                | 20 ++++++--
>>>>  12 files changed, 128 insertions(+), 48 deletions(-)
>>>>
>>