On 20/07/16 08:31, Auger Eric wrote:
> Hi Marc,
>
> On 19/07/2016 18:16, Marc Zyngier wrote:
>> On 19/07/16 16:46, Paolo Bonzini wrote:
>>>
>>>
>>> On 19/07/2016 16:56, Marc Zyngier wrote:
>>>> On 18/07/16 14:25, Eric Auger wrote:
>>>>> This patch adds compilation and link against irqchip.
>>>>>
>>>>> Main motivation behind using irqchip code is to enable MSI
>>>>> routing code. In the future irqchip routing may also be useful
>>>>> when targeting multiple irqchips.
>>>>>
>>>>> Routing standard callbacks now are implemented in vgic-irqfd:
>>>>> - kvm_set_routing_entry
>>>>> - kvm_set_irq
>>>>> - kvm_set_msi
>>>>>
>>>>> They only are supported with new_vgic code.
>>>>>
>>>>> Both HAVE_KVM_IRQCHIP and HAVE_KVM_IRQ_ROUTING are defined.
>>>>> KVM_CAP_IRQ_ROUTING is advertised and KVM_SET_GSI_ROUTING is allowed.
>>>>>
>>>>> So from now on IRQCHIP routing is enabled and a routing table entry
>>>>> must exist for irqfd injection to succeed for a given SPI. This patch
>>>>> builds a default flat irqchip routing table (gsi=irqchip.pin) covering
>>>>> all the VGIC SPI indexes. This routing table is overwritten by the
>>>>> first first user-space call to KVM_SET_GSI_ROUTING ioctl.
>>>>>
>>>>> MSI routing setup is not yet allowed.
>>>>>
>>>>> Signed-off-by: Eric Auger <eric.auger@xxxxxxxxxx>
>>>>>
>>>>> ---
>>>>> v6 -> v7:
>>>>> - re-introduce irq.h
>>>>> - use kvm_kernel_irq_routing_entry renamed fields: msi_flags, msi_devid
>>>>> - moved kvm_vgic_setup_default_irq_routing declaration in arm_vgic.h and
>>>>> definition in vgic-irqfd.c
>>>>> - correct double / in Makefile
>>>>> - remove BUG_ON(!vgic_initialized(kvm) in vgic_irqfd_set_irq since
>>>>> in any case we have a lazy_init in update_irq_pending
>>>>> - move KVM_IRQCHIP_NUM_PINS in arm_vgic.h
>>>>> - use VGIC_MAX_SPI
>>>>>
>>>>> v5 -> v6:
>>>>> - rebase on top of Andre's v8 + removal of old vgic
>>>>>
>>>>> v4 -> v5:
>>>>> - vgic_irqfd.c was renamed into vgic-irqfd.c by Andre
>>>>> - minor comment changes
>>>>> - remove trace_kvm_set_irq since it is called in irqchip
>>>>> - remove CONFIG_HAVE_KVM_MSI setting (done in KVM section)
>>>>> - despite Christoffer's question, in kvm_set_msi, I kept the copy from
>>>>> the input "struct kvm_kernel_irq_routing_entry *e" imposed by the
>>>>> irqchip callback API into the struct kvm_msi * passed to
>>>>> vits_inject_msi. Since vits_inject_msi is directly called by
>>>>> kvm_send_userspace_msi which takes a struct kvm_msi*, makes sense
>>>>> to me to keep the copy.
>>>>> - squash former [PATCH v4 5/7] KVM: arm/arm64: build a default routing
>>>>> table into that patch
>>>>> - handle default routing table alloc failure
>>>>>
>>>>> v3 -> v4:
>>>>> - provide support only for new-vgic
>>>>> - code previously in vgic.c now in vgic_irqfd.c
>>>>>
>>>>> v2 -> v3:
>>>>> - unconditionally set devid and KVM_MSI_VALID_DEVID flag as suggested
>>>>> by Andre (KVM_IRQ_ROUTING_EXTENDED_MSI type not used anymore)
>>>>> - vgic_irqfd_set_irq now is static
>>>>> - propagate flags
>>>>> - add comments
>>>>>
>>>>> v1 -> v2:
>>>>> - fix bug reported by Andre related to msi.flags and msi.devid setting
>>>>> in kvm_send_userspace_msi
>>>>> - avoid injecting reserved IRQ numbers in vgic_irqfd_set_irq
>>>>>
>>>>> RFC -> PATCH
>>>>> - reword api.txt:
>>>>> x move MSI routing comments in a subsequent patch,
>>>>> x clearly state GSI routing does not apply to KVM_IRQ_LINE
>>>>> ---
>>>>> Documentation/virtual/kvm/api.txt | 12 +++--
>>>>> arch/arm/kvm/Kconfig | 2 +
>>>>> arch/arm/kvm/Makefile | 1 +
>>>>> arch/arm/kvm/irq.h | 19 +++++++
>>>>> arch/arm64/kvm/Kconfig | 2 +
>>>>> arch/arm64/kvm/Makefile | 1 +
>>>>> arch/arm64/kvm/irq.h | 19 +++++++
>>>>> include/kvm/arm_vgic.h | 7 +++
>>>>> virt/kvm/arm/vgic/vgic-init.c | 4 ++
>>>>> virt/kvm/arm/vgic/vgic-irqfd.c | 101 +++++++++++++++++++++++++++++++-------
>>>>> virt/kvm/arm/vgic/vgic.c | 7 ---
>>>>> 11 files changed, 146 insertions(+), 29 deletions(-)
>>>>> create mode 100644 arch/arm/kvm/irq.h
>>>>> create mode 100644 arch/arm64/kvm/irq.h
>>>>>
>>>>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>>>>> index 0065c8e..3bb60d3 100644
>>>>> --- a/Documentation/virtual/kvm/api.txt
>>>>> +++ b/Documentation/virtual/kvm/api.txt
>>>>> @@ -1433,13 +1433,16 @@ KVM_ASSIGN_DEV_IRQ. Partial deassignment of host or guest IRQ is allowed.
>>>>> 4.52 KVM_SET_GSI_ROUTING
>>>>>
>>>>> Capability: KVM_CAP_IRQ_ROUTING
>>>>> -Architectures: x86 s390
>>>>> +Architectures: x86 s390 arm arm64
>>>>> Type: vm ioctl
>>>>> Parameters: struct kvm_irq_routing (in)
>>>>> Returns: 0 on success, -1 on error
>>>>>
>>>>> Sets the GSI routing table entries, overwriting any previously set entries.
>>>>>
>>>>> +On arm/arm64, GSI routing has the following limitation:
>>>>> +- GSI routing does not apply to KVM_IRQ_LINE but only to KVM_IRQFD.
>>>>> +
>>>>> struct kvm_irq_routing {
>>>>> __u32 nr;
>>>>> __u32 flags;
>>>>> @@ -2368,9 +2371,10 @@ Note that closing the resamplefd is not sufficient to disable the
>>>>> irqfd. The KVM_IRQFD_FLAG_RESAMPLE is only necessary on assignment
>>>>> and need not be specified with KVM_IRQFD_FLAG_DEASSIGN.
>>>>>
>>>>> -On ARM/ARM64, the gsi field in the kvm_irqfd struct specifies the Shared
>>>>> -Peripheral Interrupt (SPI) index, such that the GIC interrupt ID is
>>>>> -given by gsi + 32.
>>>>> +On arm/arm64, gsi routing being supported, the following can happen:
>>>>> +- in case no routing entry is associated to this gsi, injection fails
>>>>> +- in case the gsi is associated to an irqchip routing entry,
>>>>> + irqchip.pin + 32 corresponds to the injected SPI ID.
>>>>>
>>>>> 4.76 KVM_PPC_ALLOCATE_HTAB
>>>>>
>>>>> diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
>>>>> index 95a0005..3e1cd04 100644
>>>>> --- a/arch/arm/kvm/Kconfig
>>>>> +++ b/arch/arm/kvm/Kconfig
>>>>> @@ -32,6 +32,8 @@ config KVM
>>>>> select KVM_VFIO
>>>>> select HAVE_KVM_EVENTFD
>>>>> select HAVE_KVM_IRQFD
>>>>> + select HAVE_KVM_IRQCHIP
>>>>> + select HAVE_KVM_IRQ_ROUTING
>>>>> depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
>>>>> ---help---
>>>>> Support hosting virtualized guest machines.
>>>>> diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile
>>>>> index 5e28df8..10d77a6 100644
>>>>> --- a/arch/arm/kvm/Makefile
>>>>> +++ b/arch/arm/kvm/Makefile
>>>>> @@ -29,4 +29,5 @@ obj-y += $(KVM)/arm/vgic/vgic-v2.o
>>>>> obj-y += $(KVM)/arm/vgic/vgic-mmio.o
>>>>> obj-y += $(KVM)/arm/vgic/vgic-mmio-v2.o
>>>>> obj-y += $(KVM)/arm/vgic/vgic-kvm-device.o
>>>>> +obj-y += $(KVM)/irqchip.o
>>>>> obj-y += $(KVM)/arm/arch_timer.o
>>>>> diff --git a/arch/arm/kvm/irq.h b/arch/arm/kvm/irq.h
>>>>> new file mode 100644
>>>>> index 0000000..b74099b
>>>>> --- /dev/null
>>>>> +++ b/arch/arm/kvm/irq.h
>>>>> @@ -0,0 +1,19 @@
>>>>> +/*
>>>>> + * irq.h: in kernel interrupt controller related definitions
>>>>> + * Copyright (c) 2016 Red Hat, Inc.
>>>>> + *
>>>>> + * This program is free software; you can redistribute it and/or modify it
>>>>> + * under the terms and conditions of the GNU General Public License,
>>>>> + * version 2, as published by the Free Software Foundation.
>>>>> + *
>>>>> + * This header is included by irqchip.c. However, on ARM, interrupt
>>>>> + * controller declarations are located in include/kvm/arm_vgic.h since
>>>>> + * they are mostly shared between arm and arm64.
>>>>> + */
>>>>> +
>>>>> +#ifndef __IRQ_H
>>>>> +#define __IRQ_H
>>>>> +
>>>>> +#include <kvm/arm_vgic.h>
>>>>> +
>>>>> +#endif
>>>>> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
>>>>> index 9d2eff0..9c9edc9 100644
>>>>> --- a/arch/arm64/kvm/Kconfig
>>>>> +++ b/arch/arm64/kvm/Kconfig
>>>>> @@ -37,6 +37,8 @@ config KVM
>>>>> select KVM_ARM_VGIC_V3
>>>>> select KVM_ARM_PMU if HW_PERF_EVENTS
>>>>> select HAVE_KVM_MSI
>>>>> + select HAVE_KVM_IRQCHIP
>>>>> + select HAVE_KVM_IRQ_ROUTING
>>>>> ---help---
>>>>> Support hosting virtualized guest machines.
>>>>> We don't support KVM with 16K page tables yet, due to the multiple
>>>>> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
>>>>> index a5b9664..695eb3c 100644
>>>>> --- a/arch/arm64/kvm/Makefile
>>>>> +++ b/arch/arm64/kvm/Makefile
>>>>> @@ -30,5 +30,6 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v2.o
>>>>> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v3.o
>>>>> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-kvm-device.o
>>>>> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-its.o
>>>>> +kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o
>>>>> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o
>>>>> kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o
>>>>> diff --git a/arch/arm64/kvm/irq.h b/arch/arm64/kvm/irq.h
>>>>> new file mode 100644
>>>>> index 0000000..b74099b
>>>>> --- /dev/null
>>>>> +++ b/arch/arm64/kvm/irq.h
>>>>> @@ -0,0 +1,19 @@
>>>>> +/*
>>>>> + * irq.h: in kernel interrupt controller related definitions
>>>>> + * Copyright (c) 2016 Red Hat, Inc.
>>>>> + *
>>>>> + * This program is free software; you can redistribute it and/or modify it
>>>>> + * under the terms and conditions of the GNU General Public License,
>>>>> + * version 2, as published by the Free Software Foundation.
>>>>> + *
>>>>> + * This header is included by irqchip.c. However, on ARM, interrupt
>>>>> + * controller declarations are located in include/kvm/arm_vgic.h since
>>>>> + * they are mostly shared between arm and arm64.
>>>>> + */
>>>>> +
>>>>> +#ifndef __IRQ_H
>>>>> +#define __IRQ_H
>>>>> +
>>>>> +#include <kvm/arm_vgic.h>
>>>>> +
>>>>> +#endif
>>>>> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
>>>>> index 4e63a07..260c8e9 100644
>>>>> --- a/include/kvm/arm_vgic.h
>>>>> +++ b/include/kvm/arm_vgic.h
>>>>> @@ -34,6 +34,7 @@
>>>>> #define VGIC_MAX_SPI 1019
>>>>> #define VGIC_MAX_RESERVED 1023
>>>>> #define VGIC_MIN_LPI 8192
>>>>> +#define KVM_IRQCHIP_NUM_PINS (1020 - 32)
>>>>>
>>>>> enum vgic_type {
>>>>> VGIC_V2, /* Good ol' GICv2 */
>>>>> @@ -313,4 +314,10 @@ static inline int kvm_vgic_get_max_vcpus(void)
>>>>>
>>>>> int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi);
>>>>>
>>>>> +/**
>>>>> + * kvm_vgic_setup_default_irq_routing:
>>>>> + * Setup a default flat gsi routing table mapping all SPIs
>>>>> + */
>>>>> +int kvm_vgic_setup_default_irq_routing(struct kvm *kvm);
>>>>> +
>>>>> #endif /* __KVM_ARM_VGIC_H */
>>>>> diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
>>>>> index 01a60dc..1aba785 100644
>>>>> --- a/virt/kvm/arm/vgic/vgic-init.c
>>>>> +++ b/virt/kvm/arm/vgic/vgic-init.c
>>>>> @@ -264,6 +264,10 @@ int vgic_init(struct kvm *kvm)
>>>>> kvm_for_each_vcpu(i, vcpu, kvm)
>>>>> kvm_vgic_vcpu_init(vcpu);
>>>>>
>>>>> + ret = kvm_vgic_setup_default_irq_routing(kvm);
>>>>> + if (ret)
>>>>> + goto out;
>>>>> +
>>>>> dist->initialized = true;
>>>>> out:
>>>>> return ret;
>>>>> diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c
>>>>> index c675513..c4750b7 100644
>>>>> --- a/virt/kvm/arm/vgic/vgic-irqfd.c
>>>>> +++ b/virt/kvm/arm/vgic/vgic-irqfd.c
>>>>> @@ -17,36 +17,101 @@
>>>>> #include <linux/kvm.h>
>>>>> #include <linux/kvm_host.h>
>>>>> #include <trace/events/kvm.h>
>>>>> +#include <kvm/arm_vgic.h>
>>>>> +#include "vgic.h"
>>>>>
>>>>> -int kvm_irq_map_gsi(struct kvm *kvm,
>>>>> - struct kvm_kernel_irq_routing_entry *entries,
>>>>> - int gsi)
>>>>> +/**
>>>>> + * vgic_irqfd_set_irq: inject the IRQ corresponding to the
>>>>> + * irqchip routing entry
>>>>> + *
>>>>> + * This is the entry point for irqfd IRQ injection
>>>>> + */
>>>>> +static int vgic_irqfd_set_irq(struct kvm_kernel_irq_routing_entry *e,
>>>>> + struct kvm *kvm, int irq_source_id,
>>>>> + int level, bool line_status)
>>>>> {
>>>>> - return 0;
>>>>> + unsigned int spi_id = e->irqchip.pin + VGIC_NR_PRIVATE_IRQS;
>>>>> + struct vgic_dist *dist = &kvm->arch.vgic;
>>>>> +
>>>>> + if (spi_id > min(dist->nr_spis, VGIC_MAX_SPI))
>>>>> + return -EINVAL;
>>>>> + return kvm_vgic_inject_irq(kvm, 0, spi_id, level);
>>>>> }
>>>>>
>>>>> -int kvm_irq_map_chip_pin(struct kvm *kvm, unsigned int irqchip,
>>>>> - unsigned int pin)
>>>>> +/**
>>>>> + * kvm_set_routing_entry: populate a kvm routing entry
>>>>> + * from a user routing entry
>>>>> + *
>>>>> + * @e: kvm kernel routing entry handle
>>>>> + * @ue: user api routing entry handle
>>>>> + * return 0 on success, -EINVAL on errors.
>>>>> + */
>>>>> +int kvm_set_routing_entry(struct kvm_kernel_irq_routing_entry *e,
>>>>> + const struct kvm_irq_routing_entry *ue)
>>>>
>>>> For the record, this fails to build with next, and I'm carrying the
>>>> following fix:
>>>>
>>>> diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c
>>>> index 28c96ad..1cacdcf 100644
>>>> --- a/virt/kvm/arm/vgic/vgic-irqfd.c
>>>> +++ b/virt/kvm/arm/vgic/vgic-irqfd.c
>>>> @@ -42,11 +42,13 @@ static int vgic_irqfd_set_irq(struct kvm_kernel_irq_routing_entry *e,
>>>> * kvm_set_routing_entry: populate a kvm routing entry
>>>> * from a user routing entry
>>>> *
>>>> + * @kvm: the associated VM struct
>>>> * @e: kvm kernel routing entry handle
>>>> * @ue: user api routing entry handle
>>>> * return 0 on success, -EINVAL on errors.
>>>> */
>>>> -int kvm_set_routing_entry(struct kvm_kernel_irq_routing_entry *e,
>>>> +int kvm_set_routing_entry(struct kvm *kvm,
>>>> + struct kvm_kernel_irq_routing_entry *e,
>>>> const struct kvm_irq_routing_entry *ue)
>>>> {
>>>> int r = -EINVAL;
>>>
>>> Thanks, please remind me when sending a pull request.
>
> Thanks for pointing this. the conflict is with
>
> "KVM: pass struct kvm to kvm_set_routing_entry"
As I still need for this to compile on 4.7, but don't want to make
-next explode, I'm about to add the following hack on top:
diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c
index 28c96ad..b6e93d0 100644
--- a/virt/kvm/arm/vgic/vgic-irqfd.c
+++ b/virt/kvm/arm/vgic/vgic-irqfd.c
@@ -46,8 +46,15 @@ static int vgic_irqfd_set_irq(struct kvm_kernel_irq_routing_entry *e,
* @ue: user api routing entry handle
* return 0 on success, -EINVAL on errors.
*/
+#ifdef KVM_CAP_X2APIC_API
+int kvm_set_routing_entry(struct kvm *kvm,
+ struct kvm_kernel_irq_routing_entry *e,
+ const struct kvm_irq_routing_entry *ue)
+#else
+/* Remove this version and the ifdefery once merged into 4.8 */
int kvm_set_routing_entry(struct kvm_kernel_irq_routing_entry *e,
const struct kvm_irq_routing_entry *ue)
+#endif
{
int r = -EINVAL;
which I find really horrible. The alternative would be to merge
c63cf538eb4b ("KVM: pass struct kvm to kvm_set_routing_entry")
in the kvmarm tree.
What do you people think?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
