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

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

 



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"

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