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. After all that's what for instance kvmtool sets up for x86, creating default 1:1 mappings for ISA and low APIC 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(-) >>> > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html