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

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

 



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.

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



[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