Re: [RFC v7 5/7] KVM: arm/arm64: enable irqchip routing

[Paolo Bonzini <pbonzini@redhat.com>]

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@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.

Paolo