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.au...@redhat.com>
>>>>
>>>> ---
>>>> 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"
Best Regards
Eric
>
> Will do. And since I have your attention (and this series is touching a
> few bits of the generic API), would you mind having a look at the first
> four patches and ack them if you think they are OK? They look good to
> me, but hey... ;-)
>
> Thanks!
>
> M.
>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
