Re: [RFC v6 4/6] KVM: arm/arm64: enable irqchip routing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Eric,

On 06/07/16 09:47, 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>
> 
> ---
> 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/include/asm/kvm_host.h   |  2 +
>  arch/arm/kvm/Kconfig              |  2 +
>  arch/arm/kvm/Makefile             |  1 +
>  arch/arm64/include/asm/kvm_host.h |  1 +
>  arch/arm64/kvm/Kconfig            |  2 +
>  arch/arm64/kvm/Makefile           |  1 +
>  virt/kvm/arm/vgic/vgic-init.c     | 27 +++++++++++++
>  virt/kvm/arm/vgic/vgic-irqfd.c    | 82 ++++++++++++++++++++++++++++++---------
>  virt/kvm/arm/vgic/vgic.c          |  7 ----
>  virt/kvm/irqchip.c                |  2 +
>  11 files changed, 109 insertions(+), 30 deletions(-)
> 
> 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.

Do you have an idea whether this is a limitation? Or something that
requires arch specific code in userland?
Or is there another issue with extending this to KVM_IRQ_LINE (ignoring
the fact that we don't need it)?

> +
>  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/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 3c40facd..161997e 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -37,6 +37,8 @@
>  
>  #define KVM_VCPU_MAX_FEATURES 2
>  
> +#define KVM_IRQCHIP_NUM_PINS 988 /* 1020 - 32 is the number of SPIs */
> +
>  #include <kvm/arm_vgic.h>
>  
>  #define KVM_MAX_VCPUS VGIC_V2_MAX_CPUS
> 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..025d812 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/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index ebe8904..58f4a60 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -34,6 +34,7 @@
>  #define KVM_PRIVATE_MEM_SLOTS 4
>  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
>  #define KVM_HALT_POLL_NS_DEFAULT 500000
> +#define KVM_IRQCHIP_NUM_PINS 988 /* 1020 - 32 is the number of SPIs */
>  
>  #include <kvm/arm_vgic.h>
>  #include <kvm/arm_arch_timer.h>
> 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/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
> index 01a60dc..591571e 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_setup_default_irq_routing(kvm);
> +	if (ret)
> +		goto out;
> +
>  	dist->initialized = true;
>  out:
>  	return ret;
> @@ -457,3 +461,26 @@ out_free_irq:
>  			kvm_get_running_vcpus());
>  	return ret;
>  }
> +
> +int kvm_setup_default_irq_routing(struct kvm *kvm)

Can you make this static (and rename it to avoid the clash)? I see that
x86 also has it public, but there is no architecture agnostic code using
that, so I consider this just a leftover on their side.

Cheers,
Andre.

> +{
> +	struct kvm_irq_routing_entry *entries;
> +	struct vgic_dist *dist = &kvm->arch.vgic;
> +	u32 nr = dist->nr_spis;
> +	int i, ret;
> +
> +	entries = kcalloc(nr, sizeof(struct kvm_kernel_irq_routing_entry),
> +			  GFP_KERNEL);
> +	if (!entries)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < nr; i++) {
> +		entries[i].gsi = i;
> +		entries[i].type = KVM_IRQ_ROUTING_IRQCHIP;
> +		entries[i].u.irqchip.irqchip = 0;
> +		entries[i].u.irqchip.pin = i;
> +	}
> +	ret = kvm_set_irq_routing(kvm, entries, nr, 0);
> +	kfree(entries);
> +	return ret;
> +}
> diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c
> index c675513..b03ab4e 100644
> --- a/virt/kvm/arm/vgic/vgic-irqfd.c
> +++ b/virt/kvm/arm/vgic/vgic-irqfd.c
> @@ -17,36 +17,80 @@
>  #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;
>  
> -int kvm_irq_map_chip_pin(struct kvm *kvm, unsigned int irqchip,
> -			 unsigned int pin)
> -{
> -	return pin;
> +	BUG_ON(!vgic_initialized(kvm));
> +
> +	if (spi_id > min(dist->nr_spis, 1020))
> +		return -EINVAL;
> +	return kvm_vgic_inject_irq(kvm, 0, spi_id, level);
>  }
>  
> -int kvm_set_irq(struct kvm *kvm, int irq_source_id,
> -		u32 irq, int level, bool line_status)
> +/**
> + * 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)
>  {
> -	unsigned int spi = irq + VGIC_NR_PRIVATE_IRQS;
> +	int r = -EINVAL;
>  
> -	trace_kvm_set_irq(irq, level, irq_source_id);
> -
> -	BUG_ON(!vgic_initialized(kvm));
> -
> -	return kvm_vgic_inject_irq(kvm, 0, spi, level);
> +	switch (ue->type) {
> +	case KVM_IRQ_ROUTING_IRQCHIP:
> +		e->set = vgic_irqfd_set_irq;
> +		e->irqchip.irqchip = ue->u.irqchip.irqchip;
> +		e->irqchip.pin = ue->u.irqchip.pin;
> +		if ((e->irqchip.pin >= KVM_IRQCHIP_NUM_PINS) ||
> +		    (e->irqchip.irqchip >= KVM_NR_IRQCHIPS))
> +			goto out;
> +		break;
> +	default:
> +		goto out;
> +	}
> +	r = 0;
> +out:
> +	return r;
>  }
>  
> -/* MSI not implemented yet */
> +/**
> + * kvm_set_msi: inject the MSI corresponding to the
> + * MSI routing entry
> + *
> + * This is the entry point for irqfd MSI injection
> + * and userspace MSI injection.
> + */
>  int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
>  		struct kvm *kvm, int irq_source_id,
>  		int level, bool line_status)
>  {
> -	return 0;
> +	struct kvm_msi msi;
> +
> +	msi.address_lo = e->msi.address_lo;
> +	msi.address_hi = e->msi.address_hi;
> +	msi.data = e->msi.data;
> +	msi.flags = e->flags;
> +	msi.devid = e->devid;
> +
> +	if (!vgic_has_its(kvm))
> +		return -ENODEV;
> +
> +	return vgic_its_inject_msi(kvm, &msi);
>  }
> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> index c4f3aba..b254833 100644
> --- a/virt/kvm/arm/vgic/vgic.c
> +++ b/virt/kvm/arm/vgic/vgic.c
> @@ -684,10 +684,3 @@ bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, unsigned int virt_irq)
>  	return map_is_active;
>  }
>  
> -int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi)
> -{
> -	if (vgic_has_its(kvm))
> -		return vgic_its_inject_msi(kvm, msi);
> -	else
> -		return -ENODEV;
> -}
> diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
> index 32e5646..03632e3 100644
> --- a/virt/kvm/irqchip.c
> +++ b/virt/kvm/irqchip.c
> @@ -29,7 +29,9 @@
>  #include <linux/srcu.h>
>  #include <linux/export.h>
>  #include <trace/events/kvm.h>
> +#if !defined(CONFIG_ARM) && !defined(CONFIG_ARM64)
>  #include "irq.h"
> +#endif
>  
>  int kvm_irq_map_gsi(struct kvm *kvm,
>  		    struct kvm_kernel_irq_routing_entry *entries, int gsi)
> 
--
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



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux