All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC v7 0/7] KVM: arm/arm64: gsi routing support
@ 2016-07-18 13:25 Eric Auger
  2016-07-18 13:25 ` [RFC v7 1/7] KVM: api: pass the devid in the msi routing entry Eric Auger
                   ` (6 more replies)
  0 siblings, 7 replies; 30+ messages in thread
From: Eric Auger @ 2016-07-18 13:25 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, marc.zyngier, christoffer.dall,
	andre.przywara
  Cc: drjones, kvmarm, kvm, pbonzini

With the advent of GICv3 ITS in-kernel emulation, KVM MSI routing
becomes mandated for proper VIRTIO-PCI vhost integration.

In QEMU, when the VIRTIO-PCI device is programmed with the MSI message,
we previously used direct_mapping trick: this consists in extracting the
IRQ ID found in the MSI message and associating an irqfd to that IRQ ID.
When vhost worker thread gets a new buffer it signals the irqfd and kvm
then injects this IRQ ID on guest. That way although the guest uses MSIs,
no MSI emulation is used.

This worked fine with GICv2m but does not work anymore with GICV3 ITS.
Indeed this latter implements IRQ translation: what is found in the MSI
message no more is the target IRQ ID but is an intermediate event ID used
in the translation process.

Hence true MSI routing is needed so that the vhost back channel irqfd is
associated to a dummy gsi ID, routed towards the programmed MSI. When KVM
injects the MSI through the in-kernel ITS emulation, the MSI is properly
translated and eventually the LPI ID associated to the event ID is injected
on guest.

MSI routing also mandates to integrate irqchip routing. The initial
implementation of irqfd on arm must be upgraded with the integration
of kvm irqchip.c code and the implementation of its standard hooks
in the architecture specific part.

In case KVM_SET_GSI_ROUTING ioctl is not called, a default routing
table with flat irqchip routing entries is built enabling to inject gsi
corresponding to the SPI indexes seen by the guest.

As soon as KVM_SET_GSI_ROUTING is called, user-space overwrites this
default routing table and is responsible for building the whole routing
table.

for arm/arm64 KVM_SET_GSI_ROUTING has a limited support:
- only applies to KVM_IRQFD and not to KVM_IRQ_LINE

- irqchip routing was tested on Calxeda midway (VFIO with irqfd)
  with and without explicit routing
- MSI routing was tested on AMD Overdrive and Cavium ThunderX

Code + dependencies can be found at:
https://github.com/eauger/linux/tree/v4.7-rc7-its-emul-v10-gsi-routing-v7

The series depends on

[1]: [PATCH v10 00/17] KVM: arm64: GICv3 ITS emulation
     http://www.spinics.net/lists/kvm/msg135687.html
   + [PATCH] KVM: arm/arm64: fix vGICv2 KVM_DEV_ARM_VGIC_GRP_CPU/DIST_REGS
[2]: [PATCH] KVM: arm/arm64: The GIC is dead, long live the GIC

hence the RFC.

GSI flat routing setup on QEMU can be found at:
https://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg06262.html

History:
v6 -> v7:
- take into account Drew's and Andre's comments
- new patch moving declarations of kvm_setup_default_irq_routing and
  kvm_setup_empty_irq_routing outside of kvm_host.h
- vgic_v2m_inject_msi moved in vgic-irqfd
- re-introduce irq.h
- add msi_ prefix to flags/devid kvm_kernel_irq_routing_entry fields
- move kvm_vgic_setup_default_irq_routing declaration in arm_vgic.h
  and definition in vgic-irqfd.c
- remove BUG_ON(!vgic_initialized(kvm) in vgic_irqfd_set_irq
- move KVM_IRQCHIP_NUM_PINS in arm_vgic.h and use VGIC_MAX_SPI instead
  of 1020

v5 -> v6:
- rebase on Andre's v8 + removal of old vgic
- tested on Cavium ThunderX

V4 -> v5:
- rebase on Andre's v7 + final new vgic code
- check msi->data is within SPI range in vgic_v2m_inject_msi
- squashed enable irq routing and default irqchip table patches
- handle default irqchip table allocation failure
- some rephrasing in doc & comment according to Christoffer's feedbacks
- lock issue reported by Pavel seems to have disappear after 4.2 (MSI injection
  fast path)

v3 -> v4:
- rebase on top of NEW-VGIC RFC and ITS emulation series v4. This is not
  a stable foundation yet. Hence the revert to RFC. This v4 mostly is a
  reflesh/reminder.
- rewrite the cover letter

v2 -> v3:
- don't use KVM_IRQ_ROUTING_EXTENDED_MSI type at uapi and kernel level anymore;
  use KVM_MSI_VALID_DEVID flag instead
- propagate user flags downto the kernel to make sure the userspace
  correctly set devid in GICv3 ITS case (still under discussion)

v1 -> v2:
- user API changed:
  x devid id passed in kvm_irq_routing_msi
  x kept the new routing entry type: KVM_IRQ_ROUTING_EXTENDED_MSI
- kvm_host.h: adopt Andre's proposal to replace the msi_msg by a struct
  composed of the msi_msg and devid in kvm_kernel_irq_routing_entry
- Fix bug reported by Pavel: Added KVM_IRQ_ROUTING_EXTENDED_MSI handling
  in eventfd.c
- added vgic_v2m_inject_msi in vgic-v2-emul.c as suggested by Andre
- fix bug reported by Andre: bad setting of msi.flags and msi.devid
  in kvm_send_userspace_msi
- avoid injecting reserved IRQ numbers in vgic_irqfd_set_irq

RFC -> PATCH:
- clearly state limited support on arm/arm64:
  KVM_IRQ_LINE not impacted by GSI routing
- add default routing table feature (new patch file)
- changed uapi to use padding field area
- reword api.txt


Eric Auger (7):
  KVM: api: pass the devid in the msi routing entry
  KVM: kvm_host: add devid in kvm_kernel_irq_routing_entry
  KVM: irqchip: convey devid to kvm_set_msi
  KVM: move kvm_setup_default/empty_irq_routing declaration in arch
    specific header
  KVM: arm/arm64: enable irqchip routing
  KVM: arm/arm64: enable MSI routing
  KVM: arm: enable KVM_SIGNAL_MSI and MSI routing

 Documentation/virtual/kvm/api.txt |  36 +++++++++--
 arch/arm/kvm/Kconfig              |   3 +
 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 ++++++
 arch/x86/kvm/irq.h                |   3 +
 include/kvm/arm_vgic.h            |   7 +++
 include/linux/kvm_host.h          |  10 ++-
 include/uapi/linux/kvm.h          |   5 +-
 virt/kvm/arm/vgic/vgic-init.c     |   4 ++
 virt/kvm/arm/vgic/vgic-irqfd.c    | 124 ++++++++++++++++++++++++++++++++------
 virt/kvm/arm/vgic/vgic.c          |   7 ---
 virt/kvm/irqchip.c                |   6 +-
 15 files changed, 211 insertions(+), 36 deletions(-)
 create mode 100644 arch/arm/kvm/irq.h
 create mode 100644 arch/arm64/kvm/irq.h

-- 
1.9.1


^ permalink raw reply	[flat|nested] 30+ messages in thread
* [RFC v7 1/7] KVM: api: pass the devid in the msi routing entry
  2016-07-18 13:25 [RFC v7 0/7] KVM: arm/arm64: gsi routing support Eric Auger
@ 2016-07-18 13:25 ` Eric Auger
  2016-07-21 16:01   ` Radim Krčmář
  2016-07-18 13:25 ` [RFC v7 2/7] KVM: kvm_host: add devid in kvm_kernel_irq_routing_entry Eric Auger
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 30+ messages in thread
From: Eric Auger @ 2016-07-18 13:25 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, marc.zyngier, christoffer.dall,
	andre.przywara
  Cc: drjones, kvmarm, kvm, pbonzini

On ARM, the MSI msg (address and data) comes along with
out-of-band device ID information. The device ID encodes the
device that writes the MSI msg. Let's convey the device id in
kvm_irq_routing_msi and use KVM_MSI_VALID_DEVID flag value in
kvm_irq_routing_entry to indicate the msi devid is populated.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>

---

v6 -> v7:
- Added Andre's R-b

v4 -> v5:
- some rephrasing in api.txt according to Christoffer's comments
v2 -> v3:
- replace usage of KVM_IRQ_ROUTING_EXTENDED_MSI type by
  usage of KVM_MSI_VALID_DEVID flag
- add note about KVM_CAP_MSI_DEVID capability

v1 -> v2:
- devid id passed in kvm_irq_routing_msi instead of in
  kvm_irq_routing_entry

RFC -> PATCH
- remove kvm_irq_routing_extended_msi and use union instead
---
 Documentation/virtual/kvm/api.txt | 19 +++++++++++++++++--
 include/uapi/linux/kvm.h          |  5 ++++-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index f60b137..0065c8e 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1468,7 +1468,11 @@ struct kvm_irq_routing_entry {
 #define KVM_IRQ_ROUTING_S390_ADAPTER 3
 #define KVM_IRQ_ROUTING_HV_SINT 4
 
-No flags are specified so far, the corresponding field must be set to zero.
+flags:
+- KVM_MSI_VALID_DEVID: used along with KVM_IRQ_ROUTING_MSI
+  routing entry type, specifies that the devid field contains
+  a valid value.
+- zero otherwise
 
 struct kvm_irq_routing_irqchip {
 	__u32 irqchip;
@@ -1479,9 +1483,20 @@ struct kvm_irq_routing_msi {
 	__u32 address_lo;
 	__u32 address_hi;
 	__u32 data;
-	__u32 pad;
+	union {
+		__u32 pad;
+		__u32 devid;
+	};
 };
 
+devid: If KVM_MSI_VALID_DEVID is set, contains a unique device identifier
+       for the device that wrote the MSI message.
+       For PCI, this is usually a BFD identifier in the lower 16 bits.
+
+The per-VM KVM_CAP_MSI_DEVID capability advertises the requirement to
+provide the device ID. If this capability is not set, userland cannot
+rely on the kernel to allow the KVM_MSI_VALID_DEVID flag being set.
+
 struct kvm_irq_routing_s390_adapter {
 	__u64 ind_addr;
 	__u64 summary_addr;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index d8c4c32..eb22208 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -879,7 +879,10 @@ struct kvm_irq_routing_msi {
 	__u32 address_lo;
 	__u32 address_hi;
 	__u32 data;
-	__u32 pad;
+	union {
+		__u32 pad;
+		__u32 devid;
+	};
 };
 
 struct kvm_irq_routing_s390_adapter {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [RFC v7 2/7] KVM: kvm_host: add devid in kvm_kernel_irq_routing_entry
  2016-07-18 13:25 [RFC v7 0/7] KVM: arm/arm64: gsi routing support Eric Auger
  2016-07-18 13:25 ` [RFC v7 1/7] KVM: api: pass the devid in the msi routing entry Eric Auger
@ 2016-07-18 13:25 ` Eric Auger
  2016-07-21 16:13   ` Radim Krčmář
  2016-07-18 13:25 ` [RFC v7 3/7] KVM: irqchip: convey devid to kvm_set_msi Eric Auger
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 30+ messages in thread
From: Eric Auger @ 2016-07-18 13:25 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, marc.zyngier, christoffer.dall,
	andre.przywara
  Cc: drjones, kvmarm, kvm, pbonzini

Extend kvm_kernel_irq_routing_entry to transport the device id
field, devid. A new flags field makes possible to indicate the
devid is valid. Those additions are used for ARM GICv3 ITS MSI
injection.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

---
v6 -> v7:
- added msi_ prefix to flags and dev_id fields

v4 -> v5:
- add Christoffer's R-b

v2 -> v3:
- add flags

v1 -> v2:
- replace msi_msg field by a struct composed of msi_msg and devid

RFC -> PATCH:
- reword the commit message after change in first patch (uapi)
---
 include/linux/kvm_host.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index c87fe6f..3d2cbb4 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -317,7 +317,11 @@ struct kvm_kernel_irq_routing_entry {
 			unsigned irqchip;
 			unsigned pin;
 		} irqchip;
-		struct msi_msg msi;
+		struct {
+			struct msi_msg msi;
+			u32 msi_flags;
+			u32 msi_devid;
+		};
 		struct kvm_s390_adapter_int adapter;
 		struct kvm_hv_sint hv_sint;
 	};
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [RFC v7 3/7] KVM: irqchip: convey devid to kvm_set_msi
  2016-07-18 13:25 [RFC v7 0/7] KVM: arm/arm64: gsi routing support Eric Auger
  2016-07-18 13:25 ` [RFC v7 1/7] KVM: api: pass the devid in the msi routing entry Eric Auger
  2016-07-18 13:25 ` [RFC v7 2/7] KVM: kvm_host: add devid in kvm_kernel_irq_routing_entry Eric Auger
@ 2016-07-18 13:25 ` Eric Auger
  2016-07-18 13:25 ` [RFC v7 4/7] KVM: move kvm_setup_default/empty_irq_routing declaration in arch specific header Eric Auger
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 30+ messages in thread
From: Eric Auger @ 2016-07-18 13:25 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, marc.zyngier, christoffer.dall,
	andre.przywara
  Cc: drjones, kvmarm, kvm, pbonzini

on ARM, a devid field is populated in kvm_msi struct in case the
flag is set to KVM_MSI_VALID_DEVID. Let's propagate both flags and
devid field in kvm_kernel_irq_routing_entry.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

---
v6 -> v7:
- use renamed msi_flags and msi_devid
- added Andre's R-b

v4 -> v5:
- Add Christoffer's R-b

v2 -> v3:
- do not set the type to KVM_IRQ_ROUTING_EXTENDED_MSI anymore as
  suggested by Andre
- correct msi->flags check
- propagate the flags
---
 virt/kvm/irqchip.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
index 8db197b..5511824 100644
--- a/virt/kvm/irqchip.c
+++ b/virt/kvm/irqchip.c
@@ -62,12 +62,14 @@ int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi)
 {
 	struct kvm_kernel_irq_routing_entry route;
 
-	if (!irqchip_in_kernel(kvm) || msi->flags != 0)
+	if (!irqchip_in_kernel(kvm) || (msi->flags & ~KVM_MSI_VALID_DEVID))
 		return -EINVAL;
 
 	route.msi.address_lo = msi->address_lo;
 	route.msi.address_hi = msi->address_hi;
 	route.msi.data = msi->data;
+	route.msi_flags = msi->flags;
+	route.msi_devid = msi->devid;
 
 	return kvm_set_msi(&route, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1, false);
 }
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [RFC v7 4/7] KVM: move kvm_setup_default/empty_irq_routing declaration in arch specific header
  2016-07-18 13:25 [RFC v7 0/7] KVM: arm/arm64: gsi routing support Eric Auger
                   ` (2 preceding siblings ...)
  2016-07-18 13:25 ` [RFC v7 3/7] KVM: irqchip: convey devid to kvm_set_msi Eric Auger
@ 2016-07-18 13:25 ` Eric Auger
  2016-07-18 13:25 ` [RFC v7 5/7] KVM: arm/arm64: enable irqchip routing Eric Auger
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 30+ messages in thread
From: Eric Auger @ 2016-07-18 13:25 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, marc.zyngier, christoffer.dall,
	andre.przywara
  Cc: pbonzini, kvmarm, kvm

kvm_setup_default_irq_routing and kvm_setup_empty_irq_routing are
not used by generic code. So let's move the declarations in x86 irq.h
header instead of kvm_host.h.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Suggested-by: Andre Przywara <andre.przywara@arm.com>

---

- new patch in v7
---
 arch/x86/kvm/irq.h       | 3 +++
 include/linux/kvm_host.h | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index 61ebdc1..035731e 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -120,4 +120,7 @@ void __kvm_migrate_timers(struct kvm_vcpu *vcpu);
 
 int apic_has_pending_timer(struct kvm_vcpu *vcpu);
 
+int kvm_setup_default_irq_routing(struct kvm *kvm);
+int kvm_setup_empty_irq_routing(struct kvm *kvm);
+
 #endif
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 3d2cbb4..dcdff5c2 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1042,8 +1042,6 @@ static inline int mmu_notifier_retry(struct kvm *kvm, unsigned long mmu_seq)
 #define KVM_MAX_IRQ_ROUTES 1024
 #endif
 
-int kvm_setup_default_irq_routing(struct kvm *kvm);
-int kvm_setup_empty_irq_routing(struct kvm *kvm);
 int kvm_set_irq_routing(struct kvm *kvm,
 			const struct kvm_irq_routing_entry *entries,
 			unsigned nr,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [RFC v7 5/7] KVM: arm/arm64: enable irqchip routing
  2016-07-18 13:25 [RFC v7 0/7] KVM: arm/arm64: gsi routing support Eric Auger
                   ` (3 preceding siblings ...)
  2016-07-18 13:25 ` [RFC v7 4/7] KVM: move kvm_setup_default/empty_irq_routing declaration in arch specific header Eric Auger
@ 2016-07-18 13:25 ` Eric Auger
  2016-07-19 14:56   ` Marc Zyngier
  2016-07-18 13:25 ` [RFC v7 6/7] KVM: arm/arm64: enable MSI routing Eric Auger
  2016-07-18 13:25 ` [RFC v7 7/7] KVM: arm: enable KVM_SIGNAL_MSI and " Eric Auger
  6 siblings, 1 reply; 30+ messages in thread
From: Eric Auger @ 2016-07-18 13:25 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, marc.zyngier, christoffer.dall,
	andre.przywara
  Cc: pbonzini, kvmarm, kvm

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)
 {
-	return pin;
+	int r = -EINVAL;
+
+	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;
 }
 
-int kvm_set_irq(struct kvm *kvm, int irq_source_id,
-		u32 irq, int level, bool line_status)
+/**
+ * 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)
 {
-	unsigned int spi = irq + VGIC_NR_PRIVATE_IRQS;
+	struct kvm_msi msi;
 
-	trace_kvm_set_irq(irq, level, irq_source_id);
+	msi.address_lo = e->msi.address_lo;
+	msi.address_hi = e->msi.address_hi;
+	msi.data = e->msi.data;
+	msi.flags = e->msi_flags;
+	msi.devid = e->msi_devid;
 
-	BUG_ON(!vgic_initialized(kvm));
+	if (!vgic_has_its(kvm))
+		return -ENODEV;
 
-	return kvm_vgic_inject_irq(kvm, 0, spi, level);
+	return vgic_its_inject_msi(kvm, &msi);
 }
 
-/* MSI not implemented yet */
-int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
-		struct kvm *kvm, int irq_source_id,
-		int level, bool line_status)
+int kvm_vgic_setup_default_irq_routing(struct kvm *kvm)
 {
-	return 0;
+	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.c b/virt/kvm/arm/vgic/vgic.c
index 424cb9c..1b3e2cd 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -719,10 +719,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;
-}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [RFC v7 6/7] KVM: arm/arm64: enable MSI routing
  2016-07-18 13:25 [RFC v7 0/7] KVM: arm/arm64: gsi routing support Eric Auger
                   ` (4 preceding siblings ...)
  2016-07-18 13:25 ` [RFC v7 5/7] KVM: arm/arm64: enable irqchip routing Eric Auger
@ 2016-07-18 13:25 ` Eric Auger
  2016-07-21 16:21   ` Radim Krčmář
  2016-07-18 13:25 ` [RFC v7 7/7] KVM: arm: enable KVM_SIGNAL_MSI and " Eric Auger
  6 siblings, 1 reply; 30+ messages in thread
From: Eric Auger @ 2016-07-18 13:25 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, marc.zyngier, christoffer.dall,
	andre.przywara
  Cc: pbonzini, kvmarm, kvm

Up to now, only irqchip routing entries could be set. This patch
adds the capability to insert MSI routing entries.

For ARM64, let's also increase KVM_MAX_IRQ_ROUTES to 4096: this
include SPI irqchip routes plus MSI routes. In the future this
might be extended.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>

---
v6 -> v7:
- added Andre's R-b

v2 -> v3:
- remove any reference to KVM_IRQ_ROUTING_EXTENDED_MSI type
- unconditionnaly uapi flags and devid downto the kernel
  routing entry struct
- handle KVM_MSI_VALID_DEVID flag in kvm_set_irq_routing
- note about KVM_CAP_MSI_DEVID moved in the first patch file
  of the series

v1 -> v2:
- adapt to new routing entry types

RFC -> PATCH:
- move api MSI routing updates into that patch file
- use new devid field of user api struct
---
 Documentation/virtual/kvm/api.txt | 5 +++++
 include/linux/kvm_host.h          | 2 ++
 virt/kvm/arm/vgic/vgic-irqfd.c    | 8 ++++++++
 virt/kvm/irqchip.c                | 2 +-
 4 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 3bb60d3..60d4999 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2375,6 +2375,11 @@ 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.
+- in case the gsi is associated to an MSI routing entry,
+  * without GICv3 ITS in-kernel emulation, MSI data matches the SPI ID
+    of the injected SPI
+  * with GICv3 ITS in-kernel emulation, the MSI message and device ID
+    are translated into an LPI.
 
 4.76 KVM_PPC_ALLOCATE_HTAB
 
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index dcdff5c2..7800ff7 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1038,6 +1038,8 @@ static inline int mmu_notifier_retry(struct kvm *kvm, unsigned long mmu_seq)
 
 #ifdef CONFIG_S390
 #define KVM_MAX_IRQ_ROUTES 4096 //FIXME: we can have more than that...
+#elif defined(CONFIG_ARM64)
+#define KVM_MAX_IRQ_ROUTES 4096
 #else
 #define KVM_MAX_IRQ_ROUTES 1024
 #endif
diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c
index c4750b7..4f0d7eb 100644
--- a/virt/kvm/arm/vgic/vgic-irqfd.c
+++ b/virt/kvm/arm/vgic/vgic-irqfd.c
@@ -60,6 +60,14 @@ int kvm_set_routing_entry(struct kvm_kernel_irq_routing_entry *e,
 		    (e->irqchip.irqchip >= KVM_NR_IRQCHIPS))
 			goto out;
 		break;
+	case KVM_IRQ_ROUTING_MSI:
+		e->set = kvm_set_msi;
+		e->msi.address_lo = ue->u.msi.address_lo;
+		e->msi.address_hi = ue->u.msi.address_hi;
+		e->msi.data = ue->u.msi.data;
+		e->msi_flags = ue->flags;
+		e->msi_devid = ue->u.msi.devid;
+		break;
 	default:
 		goto out;
 	}
diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
index 5511824..81d95d2 100644
--- a/virt/kvm/irqchip.c
+++ b/virt/kvm/irqchip.c
@@ -209,7 +209,7 @@ int kvm_set_irq_routing(struct kvm *kvm,
 			goto out;
 
 		r = -EINVAL;
-		if (ue->flags) {
+		if (ue->flags & ~KVM_MSI_VALID_DEVID) {
 			kfree(e);
 			goto out;
 		}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [RFC v7 7/7] KVM: arm: enable KVM_SIGNAL_MSI and MSI routing
  2016-07-18 13:25 [RFC v7 0/7] KVM: arm/arm64: gsi routing support Eric Auger
                   ` (5 preceding siblings ...)
  2016-07-18 13:25 ` [RFC v7 6/7] KVM: arm/arm64: enable MSI routing Eric Auger
@ 2016-07-18 13:25 ` Eric Auger
  2016-07-21 16:33   ` Radim Krčmář
  6 siblings, 1 reply; 30+ messages in thread
From: Eric Auger @ 2016-07-18 13:25 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, marc.zyngier, christoffer.dall,
	andre.przywara
  Cc: drjones, kvmarm, kvm, pbonzini

If the ITS modality is not available, let's simply support MSI
injection by transforming the MSI.data into an SPI ID.

This becomes possible to use KVM_SIGNAL_MSI ioctl and MSI
routing for arm too.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

v6 -> v7:
- move vgic_v2m_inject_msi into vgic-irqfd

v4 -> v5:
- on vgic_v2m_inject_msi check the msi->data is within the SPI range
- move KVM_HAVE_MSI in the KVM section (to be symetrical with ARM64)

v2 -> v3:
- reword the commit message
- add sanity check about devid provision

v1 -> v2:
- introduce vgic_v2m_inject_msi in vgic-v2-emul.c following Andre's
  advice
---
 arch/arm/kvm/Kconfig           |  1 +
 virt/kvm/arm/vgic/vgic-irqfd.c | 19 ++++++++++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
index 3e1cd04..90d0176 100644
--- a/arch/arm/kvm/Kconfig
+++ b/arch/arm/kvm/Kconfig
@@ -34,6 +34,7 @@ config KVM
 	select HAVE_KVM_IRQFD
 	select HAVE_KVM_IRQCHIP
 	select HAVE_KVM_IRQ_ROUTING
+	select HAVE_KVM_MSI
 	depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
 	---help---
 	  Support hosting virtualized guest machines.
diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c
index 4f0d7eb..28c96ad 100644
--- a/virt/kvm/arm/vgic/vgic-irqfd.c
+++ b/virt/kvm/arm/vgic/vgic-irqfd.c
@@ -77,6 +77,23 @@ out:
 }
 
 /**
+ * vgic_v2m_inject_msi: emulates GICv2M MSI injection by injecting
+ * the SPI ID matching the msi data
+ *
+ * @kvm: pointer to the kvm struct
+ * @msi: the msi struct handle
+ */
+static int vgic_v2m_inject_msi(struct kvm *kvm, struct kvm_msi *msi)
+{
+	if (msi->flags & KVM_MSI_VALID_DEVID)
+		return -EINVAL;
+	if (!vgic_valid_spi(kvm, msi->data))
+		return -EINVAL;
+
+	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);
+}
+
+/**
  * kvm_set_msi: inject the MSI corresponding to the
  * MSI routing entry
  *
@@ -96,7 +113,7 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
 	msi.devid = e->msi_devid;
 
 	if (!vgic_has_its(kvm))
-		return -ENODEV;
+		return vgic_v2m_inject_msi(kvm, &msi);
 
 	return vgic_its_inject_msi(kvm, &msi);
 }
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* Re: [RFC v7 5/7] KVM: arm/arm64: enable irqchip routing
  2016-07-18 13:25 ` [RFC v7 5/7] KVM: arm/arm64: enable irqchip routing Eric Auger
@ 2016-07-19 14:56   ` Marc Zyngier
  2016-07-19 15:46     ` Paolo Bonzini
  0 siblings, 1 reply; 30+ messages in thread
From: Marc Zyngier @ 2016-07-19 14:56 UTC (permalink / raw)
  To: Eric Auger, eric.auger.pro, christoffer.dall, andre.przywara
  Cc: drjones, kvmarm, kvm, pbonzini

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,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* Re: [RFC v7 5/7] KVM: arm/arm64: enable irqchip routing
  2016-07-19 14:56   ` Marc Zyngier
@ 2016-07-19 15:46     ` Paolo Bonzini
  2016-07-19 16:16       ` Marc Zyngier
  0 siblings, 1 reply; 30+ messages in thread
From: Paolo Bonzini @ 2016-07-19 15:46 UTC (permalink / raw)
  To: Marc Zyngier, Eric Auger, eric.auger.pro, christoffer.dall,
	andre.przywara
  Cc: kvmarm, kvm



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

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 5/7] KVM: arm/arm64: enable irqchip routing
  2016-07-19 15:46     ` Paolo Bonzini
@ 2016-07-19 16:16       ` Marc Zyngier
  2016-07-20  7:31         ` Auger Eric
  0 siblings, 1 reply; 30+ messages in thread
From: Marc Zyngier @ 2016-07-19 16:16 UTC (permalink / raw)
  To: Paolo Bonzini, Eric Auger, eric.auger.pro, christoffer.dall,
	andre.przywara
  Cc: drjones, kvmarm, kvm

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

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

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 5/7] KVM: arm/arm64: enable irqchip routing
  2016-07-19 16:16       ` Marc Zyngier
@ 2016-07-20  7:31         ` Auger Eric
  2016-07-21 15:33           ` Marc Zyngier
  0 siblings, 1 reply; 30+ messages in thread
From: Auger Eric @ 2016-07-20  7:31 UTC (permalink / raw)
  To: Marc Zyngier, Paolo Bonzini, eric.auger.pro, christoffer.dall,
	andre.przywara
  Cc: drjones, kvmarm, kvm

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

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 5/7] KVM: arm/arm64: enable irqchip routing
  2016-07-20  7:31         ` Auger Eric
@ 2016-07-21 15:33           ` Marc Zyngier
  0 siblings, 0 replies; 30+ messages in thread
From: Marc Zyngier @ 2016-07-21 15:33 UTC (permalink / raw)
  To: Auger Eric, Paolo Bonzini, eric.auger.pro, christoffer.dall,
	andre.przywara, Radim
  Cc: drjones, kvmarm, kvm

On 20/07/16 08:31, Auger Eric wrote:
> 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@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"

As I still need for this to compile on 4.7, but don't want to make
-next explode, I'm about to add the following hack on top:

diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c
index 28c96ad..b6e93d0 100644
--- a/virt/kvm/arm/vgic/vgic-irqfd.c
+++ b/virt/kvm/arm/vgic/vgic-irqfd.c
@@ -46,8 +46,15 @@ static int vgic_irqfd_set_irq(struct kvm_kernel_irq_routing_entry *e,
  * @ue: user api routing entry handle
  * return 0 on success, -EINVAL on errors.
  */
+#ifdef KVM_CAP_X2APIC_API
+int kvm_set_routing_entry(struct kvm *kvm,
+			  struct kvm_kernel_irq_routing_entry *e,
+			  const struct kvm_irq_routing_entry *ue)
+#else
+/* Remove this version and the ifdefery once merged into 4.8 */
 int kvm_set_routing_entry(struct kvm_kernel_irq_routing_entry *e,
 			  const struct kvm_irq_routing_entry *ue)
+#endif
 {
 	int r = -EINVAL;
 
which I find really horrible. The alternative would be to merge 
c63cf538eb4b ("KVM: pass struct kvm to kvm_set_routing_entry")
in the kvmarm tree.

What do you people think?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* Re: [RFC v7 1/7] KVM: api: pass the devid in the msi routing entry
  2016-07-18 13:25 ` [RFC v7 1/7] KVM: api: pass the devid in the msi routing entry Eric Auger
@ 2016-07-21 16:01   ` Radim Krčmář
  2016-07-21 16:43     ` Andre Przywara
  0 siblings, 1 reply; 30+ messages in thread
From: Radim Krčmář @ 2016-07-21 16:01 UTC (permalink / raw)
  To: Eric Auger
  Cc: eric.auger.pro, marc.zyngier, christoffer.dall, andre.przywara,
	drjones, kvmarm, kvm, pbonzini

2016-07-18 13:25+0000, Eric Auger:
> On ARM, the MSI msg (address and data) comes along with
> out-of-band device ID information. The device ID encodes the
> device that writes the MSI msg. Let's convey the device id in
> kvm_irq_routing_msi and use KVM_MSI_VALID_DEVID flag value in
> kvm_irq_routing_entry to indicate the msi devid is populated.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> 
> ---
> 
> v6 -> v7:
> - Added Andre's R-b
> 
> v4 -> v5:
> - some rephrasing in api.txt according to Christoffer's comments
> v2 -> v3:
> - replace usage of KVM_IRQ_ROUTING_EXTENDED_MSI type by
>   usage of KVM_MSI_VALID_DEVID flag
> - add note about KVM_CAP_MSI_DEVID capability
> 
> v1 -> v2:
> - devid id passed in kvm_irq_routing_msi instead of in
>   kvm_irq_routing_entry
> 
> RFC -> PATCH
> - remove kvm_irq_routing_extended_msi and use union instead
> ---
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> @@ -1479,9 +1483,20 @@ struct kvm_irq_routing_msi {
>  	__u32 address_lo;
>  	__u32 address_hi;
>  	__u32 data;
> -	__u32 pad;
> +	union {
> +		__u32 pad;
> +		__u32 devid;
> +	};
>  };
>  
> +devid: If KVM_MSI_VALID_DEVID is set, contains a unique device identifier
> +       for the device that wrote the MSI message.
> +       For PCI, this is usually a BFD identifier in the lower 16 bits.
> +
> +The per-VM KVM_CAP_MSI_DEVID capability advertises the requirement to
> +provide the device ID. If this capability is not set, userland cannot
> +rely on the kernel to allow the KVM_MSI_VALID_DEVID flag being set.

It would be better to enforce this mentioned dependency on set
KVM_CAP_MSI_DEVID, but is the dependency even required?
It seems we were checking flags for zero, so KVM_MSI_VALID_DEVID
couldn't have been set by old userspaces, therefor it is ok to only make
it depend only on the presence of KVM_CAP_MSI_DEVID, like the patch does
now.  (I assume KVM_MSI_VALID_DEVID and KVM_CAP_MSI_DEVID are being
merged at the same time.)

Then there would be little point in having KVM_CAP_MSI_DEVID enableable,
so does enabling KVM_CAP_MSI_DEVID mean that every MSI must have a valid
devid?

Thanks.

---
I'm confused about the purpose behind two dynamic flags that seem to do
that same thing, but those are just nitpicks, the API looks good in
general.

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 2/7] KVM: kvm_host: add devid in kvm_kernel_irq_routing_entry
  2016-07-18 13:25 ` [RFC v7 2/7] KVM: kvm_host: add devid in kvm_kernel_irq_routing_entry Eric Auger
@ 2016-07-21 16:13   ` Radim Krčmář
  2016-07-21 16:54     ` Marc Zyngier
  0 siblings, 1 reply; 30+ messages in thread
From: Radim Krčmář @ 2016-07-21 16:13 UTC (permalink / raw)
  To: Eric Auger
  Cc: eric.auger.pro, marc.zyngier, christoffer.dall, andre.przywara,
	drjones, kvmarm, kvm, pbonzini

2016-07-18 13:25+0000, Eric Auger:
> Extend kvm_kernel_irq_routing_entry to transport the device id
> field, devid. A new flags field makes possible to indicate the
> devid is valid. Those additions are used for ARM GICv3 ITS MSI
> injection.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
> 
> ---
> v6 -> v7:
> - added msi_ prefix to flags and dev_id fields
> 
> v4 -> v5:
> - add Christoffer's R-b
> 
> v2 -> v3:
> - add flags
> 
> v1 -> v2:
> - replace msi_msg field by a struct composed of msi_msg and devid
> 
> RFC -> PATCH:
> - reword the commit message after change in first patch (uapi)
> ---
>  include/linux/kvm_host.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index c87fe6f..3d2cbb4 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -317,7 +317,11 @@ struct kvm_kernel_irq_routing_entry {
>  			unsigned irqchip;
>  			unsigned pin;
>  		} irqchip;
> -		struct msi_msg msi;
> +		struct {
> +			struct msi_msg msi;
> +			u32 msi_flags;
> +			u32 msi_devid;

I'd much rather see them as msi.flags and msi.devid.
I didn't notice a code that passes struct msi_msg anywhere, so using an
ad-hoc struct like irqchip or defining a new one would work fine.

Thanks.

> +		};
>  		struct kvm_s390_adapter_int adapter;
>  		struct kvm_hv_sint hv_sint;
>  	};
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 6/7] KVM: arm/arm64: enable MSI routing
  2016-07-18 13:25 ` [RFC v7 6/7] KVM: arm/arm64: enable MSI routing Eric Auger
@ 2016-07-21 16:21   ` Radim Krčmář
  2016-07-21 20:50     ` Auger Eric
  0 siblings, 1 reply; 30+ messages in thread
From: Radim Krčmář @ 2016-07-21 16:21 UTC (permalink / raw)
  To: Eric Auger
  Cc: eric.auger.pro, marc.zyngier, christoffer.dall, andre.przywara,
	drjones, kvmarm, kvm, pbonzini

2016-07-18 13:25+0000, Eric Auger:
> Up to now, only irqchip routing entries could be set. This patch
> adds the capability to insert MSI routing entries.
> 
> For ARM64, let's also increase KVM_MAX_IRQ_ROUTES to 4096: this
> include SPI irqchip routes plus MSI routes. In the future this
> might be extended.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> 
> ---
> v6 -> v7:
> - added Andre's R-b
> 
> v2 -> v3:
> - remove any reference to KVM_IRQ_ROUTING_EXTENDED_MSI type
> - unconditionnaly uapi flags and devid downto the kernel
>   routing entry struct
> - handle KVM_MSI_VALID_DEVID flag in kvm_set_irq_routing
> - note about KVM_CAP_MSI_DEVID moved in the first patch file
>   of the series
> 
> v1 -> v2:
> - adapt to new routing entry types
> 
> RFC -> PATCH:
> - move api MSI routing updates into that patch file
> - use new devid field of user api struct
> ---
> diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
> @@ -209,7 +209,7 @@ int kvm_set_irq_routing(struct kvm *kvm,
>  			goto out;
>  
>  		r = -EINVAL;
> -		if (ue->flags) {
> +		if (ue->flags & ~KVM_MSI_VALID_DEVID) {

This allows KVM_MSI_VALID_DEVID flag for all route types, which is not
what we want.
To avoid checking separately when setting MSI route in every arch,
I think we could have a switch (ue->type) here as well.

>  			kfree(e);
>  			goto out;
>  		}

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 7/7] KVM: arm: enable KVM_SIGNAL_MSI and MSI routing
  2016-07-18 13:25 ` [RFC v7 7/7] KVM: arm: enable KVM_SIGNAL_MSI and " Eric Auger
@ 2016-07-21 16:33   ` Radim Krčmář
  2016-07-21 21:10     ` Auger Eric
  0 siblings, 1 reply; 30+ messages in thread
From: Radim Krčmář @ 2016-07-21 16:33 UTC (permalink / raw)
  To: Eric Auger
  Cc: kvm, marc.zyngier, andre.przywara, pbonzini, kvmarm, eric.auger.pro

2016-07-18 13:25+0000, Eric Auger:
> If the ITS modality is not available, let's simply support MSI
> injection by transforming the MSI.data into an SPI ID.
> 
> This becomes possible to use KVM_SIGNAL_MSI ioctl and MSI
> routing for arm too.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> 
> v6 -> v7:
> - move vgic_v2m_inject_msi into vgic-irqfd
> 
> v4 -> v5:
> - on vgic_v2m_inject_msi check the msi->data is within the SPI range
> - move KVM_HAVE_MSI in the KVM section (to be symetrical with ARM64)
> 
> v2 -> v3:
> - reword the commit message
> - add sanity check about devid provision
> 
> v1 -> v2:
> - introduce vgic_v2m_inject_msi in vgic-v2-emul.c following Andre's
>   advice
> ---
> diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c
> +static int vgic_v2m_inject_msi(struct kvm *kvm, struct kvm_msi *msi)
> +{
> +	if (msi->flags & KVM_MSI_VALID_DEVID)
> +		return -EINVAL;
> +	if (!vgic_valid_spi(kvm, msi->data))
> +		return -EINVAL;
> +
> +	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);

Hm, this isn't very MSI related ...

arm already has KVM_IRQ_LINE/kvm_vm_ioctl_irq_line with
KVM_ARM_IRQ_TYPE_SPI that does
  kvm_vgic_inject_irq(kvm, 0, irq_num, level)

Is that interface lacking?

Thanks.

> +}
> +
> +/**
>   * kvm_set_msi: inject the MSI corresponding to the
>   * MSI routing entry
>   *
> @@ -96,7 +113,7 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
>  	msi.devid = e->msi_devid;
>  
>  	if (!vgic_has_its(kvm))
> -		return -ENODEV;
> +		return vgic_v2m_inject_msi(kvm, &msi);
>  
>  	return vgic_its_inject_msi(kvm, &msi);
>  }
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 1/7] KVM: api: pass the devid in the msi routing entry
  2016-07-21 16:01   ` Radim Krčmář
@ 2016-07-21 16:43     ` Andre Przywara
  2016-07-21 17:15       ` Radim Krčmář
  0 siblings, 1 reply; 30+ messages in thread
From: Andre Przywara @ 2016-07-21 16:43 UTC (permalink / raw)
  To: Radim Krčmář, Eric Auger
  Cc: eric.auger.pro, marc.zyngier, christoffer.dall, drjones, kvmarm,
	kvm, pbonzini

Hi Radim,

On 21/07/16 17:01, Radim Krčmář wrote:
> 2016-07-18 13:25+0000, Eric Auger:
>> On ARM, the MSI msg (address and data) comes along with
>> out-of-band device ID information. The device ID encodes the
>> device that writes the MSI msg. Let's convey the device id in
>> kvm_irq_routing_msi and use KVM_MSI_VALID_DEVID flag value in
>> kvm_irq_routing_entry to indicate the msi devid is populated.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
>>
>> ---
>>
>> v6 -> v7:
>> - Added Andre's R-b
>>
>> v4 -> v5:
>> - some rephrasing in api.txt according to Christoffer's comments
>> v2 -> v3:
>> - replace usage of KVM_IRQ_ROUTING_EXTENDED_MSI type by
>>   usage of KVM_MSI_VALID_DEVID flag
>> - add note about KVM_CAP_MSI_DEVID capability
>>
>> v1 -> v2:
>> - devid id passed in kvm_irq_routing_msi instead of in
>>   kvm_irq_routing_entry
>>
>> RFC -> PATCH
>> - remove kvm_irq_routing_extended_msi and use union instead
>> ---
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> @@ -1479,9 +1483,20 @@ struct kvm_irq_routing_msi {
>>  	__u32 address_lo;
>>  	__u32 address_hi;
>>  	__u32 data;
>> -	__u32 pad;
>> +	union {
>> +		__u32 pad;
>> +		__u32 devid;
>> +	};
>>  };
>>  
>> +devid: If KVM_MSI_VALID_DEVID is set, contains a unique device identifier
>> +       for the device that wrote the MSI message.
>> +       For PCI, this is usually a BFD identifier in the lower 16 bits.
>> +
>> +The per-VM KVM_CAP_MSI_DEVID capability advertises the requirement to
>> +provide the device ID. If this capability is not set, userland cannot
>> +rely on the kernel to allow the KVM_MSI_VALID_DEVID flag being set.
> 
> It would be better to enforce this mentioned dependency on set
> KVM_CAP_MSI_DEVID, but is the dependency even required?
> It seems we were checking flags for zero, so KVM_MSI_VALID_DEVID
> couldn't have been set by old userspaces, therefor it is ok to only make
> it depend only on the presence of KVM_CAP_MSI_DEVID, like the patch does
> now.  (I assume KVM_MSI_VALID_DEVID and KVM_CAP_MSI_DEVID are being
> merged at the same time.)
> 
> Then there would be little point in having KVM_CAP_MSI_DEVID enableable,
> so does enabling KVM_CAP_MSI_DEVID mean that every MSI must have a valid
> devid?

KVM_CAP_MSI_DEVID tells userland that it's fine to set the
KVM_MSI_VALID_DEVID flag (because the kernel would bark otherwise).

KVM_MSI_VALID_DEVID tells the kernel that there is some meaningful
device ID data in the field formerly known as "pad".

IIRC we started with the VALID_DEVID flag, then found that we need the
CAP because we repurposed the pad field.

Does that make sense? Admittedly this _is_ confusing ;-)

Cheers,
Andre.


> 
> Thanks.
> 
> ---
> I'm confused about the purpose behind two dynamic flags that seem to do
> that same thing, but those are just nitpicks, the API looks good in
> general.
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 2/7] KVM: kvm_host: add devid in kvm_kernel_irq_routing_entry
  2016-07-21 16:13   ` Radim Krčmář
@ 2016-07-21 16:54     ` Marc Zyngier
  2016-07-21 17:22       ` Radim Krčmář
  0 siblings, 1 reply; 30+ messages in thread
From: Marc Zyngier @ 2016-07-21 16:54 UTC (permalink / raw)
  To: Radim Krčmář, Eric Auger
  Cc: kvm, andre.przywara, pbonzini, kvmarm, eric.auger.pro

On 21/07/16 17:13, Radim Krčmář wrote:
> 2016-07-18 13:25+0000, Eric Auger:
>> Extend kvm_kernel_irq_routing_entry to transport the device id
>> field, devid. A new flags field makes possible to indicate the
>> devid is valid. Those additions are used for ARM GICv3 ITS MSI
>> injection.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
>>
>> ---
>> v6 -> v7:
>> - added msi_ prefix to flags and dev_id fields
>>
>> v4 -> v5:
>> - add Christoffer's R-b
>>
>> v2 -> v3:
>> - add flags
>>
>> v1 -> v2:
>> - replace msi_msg field by a struct composed of msi_msg and devid
>>
>> RFC -> PATCH:
>> - reword the commit message after change in first patch (uapi)
>> ---
>>  include/linux/kvm_host.h | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index c87fe6f..3d2cbb4 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -317,7 +317,11 @@ struct kvm_kernel_irq_routing_entry {
>>  			unsigned irqchip;
>>  			unsigned pin;
>>  		} irqchip;
>> -		struct msi_msg msi;
>> +		struct {
>> +			struct msi_msg msi;
>> +			u32 msi_flags;
>> +			u32 msi_devid;
> 
> I'd much rather see them as msi.flags and msi.devid.

That's not really possible, as msi_msg is the core data structure for
MSIs, and nobody really expects this tructure to change.

> I didn't notice a code that passes struct msi_msg anywhere, so using an
> ad-hoc struct like irqchip or defining a new one would work fine.

I guess we could have an arm-specific one:

		struct arm_msi {
			struct msi_msg msg;
			u32 flags;
			u32 devid;
		};

and use that. Would that be OK with you?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 1/7] KVM: api: pass the devid in the msi routing entry
  2016-07-21 16:43     ` Andre Przywara
@ 2016-07-21 17:15       ` Radim Krčmář
  2016-07-21 20:48         ` Auger Eric
  0 siblings, 1 reply; 30+ messages in thread
From: Radim Krčmář @ 2016-07-21 17:15 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Eric Auger, eric.auger.pro, marc.zyngier, christoffer.dall,
	drjones, kvmarm, kvm, pbonzini

2016-07-21 17:43+0100, Andre Przywara:
> Hi Radim,
> 
> On 21/07/16 17:01, Radim Krčmář wrote:
>> 2016-07-18 13:25+0000, Eric Auger:
>>> On ARM, the MSI msg (address and data) comes along with
>>> out-of-band device ID information. The device ID encodes the
>>> device that writes the MSI msg. Let's convey the device id in
>>> kvm_irq_routing_msi and use KVM_MSI_VALID_DEVID flag value in
>>> kvm_irq_routing_entry to indicate the msi devid is populated.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
>>>
>>> ---
>>>  
>>> +devid: If KVM_MSI_VALID_DEVID is set, contains a unique device identifier
>>> +       for the device that wrote the MSI message.
>>> +       For PCI, this is usually a BFD identifier in the lower 16 bits.
>>> +
>>> +The per-VM KVM_CAP_MSI_DEVID capability advertises the requirement to
>>> +provide the device ID. If this capability is not set, userland cannot
>>> +rely on the kernel to allow the KVM_MSI_VALID_DEVID flag being set.
>> 
>> It would be better to enforce this mentioned dependency on set
>> KVM_CAP_MSI_DEVID, but is the dependency even required?
>> It seems we were checking flags for zero, so KVM_MSI_VALID_DEVID
>> couldn't have been set by old userspaces, therefor it is ok to only make
>> it depend only on the presence of KVM_CAP_MSI_DEVID, like the patch does
>> now.  (I assume KVM_MSI_VALID_DEVID and KVM_CAP_MSI_DEVID are being
>> merged at the same time.)
>> 
>> Then there would be little point in having KVM_CAP_MSI_DEVID enableable,
>> so does enabling KVM_CAP_MSI_DEVID mean that every MSI must have a valid
>> devid?
> 
> KVM_CAP_MSI_DEVID tells userland that it's fine to set the
> KVM_MSI_VALID_DEVID flag (because the kernel would bark otherwise).
> 
> KVM_MSI_VALID_DEVID tells the kernel that there is some meaningful
> device ID data in the field formerly known as "pad".
> 
> IIRC we started with the VALID_DEVID flag, then found that we need the
> CAP because we repurposed the pad field.
> 
> Does that make sense? Admittedly this _is_ confusing ;-)

It does, thanks.
Some capability is need and I thought that KVM_CAP_MSI_DEVID has to be
enabled by userspace before KVM_MSI_VALID_DEVID can be used, which isn't
the case.  It is enabled conditionally based on vgic ITS ... my bad.

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 2/7] KVM: kvm_host: add devid in kvm_kernel_irq_routing_entry
  2016-07-21 16:54     ` Marc Zyngier
@ 2016-07-21 17:22       ` Radim Krčmář
  2016-07-21 17:25         ` Marc Zyngier
  0 siblings, 1 reply; 30+ messages in thread
From: Radim Krčmář @ 2016-07-21 17:22 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Eric Auger, eric.auger.pro, christoffer.dall, andre.przywara,
	drjones, kvmarm, kvm, pbonzini

2016-07-21 17:54+0100, Marc Zyngier:
> On 21/07/16 17:13, Radim Krčmář wrote:
>> 2016-07-18 13:25+0000, Eric Auger:
>>> Extend kvm_kernel_irq_routing_entry to transport the device id
>>> field, devid. A new flags field makes possible to indicate the
>>> devid is valid. Those additions are used for ARM GICv3 ITS MSI
>>> injection.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
>>>
>>> ---
>>> v6 -> v7:
>>> - added msi_ prefix to flags and dev_id fields
>>>
>>> v4 -> v5:
>>> - add Christoffer's R-b
>>>
>>> v2 -> v3:
>>> - add flags
>>>
>>> v1 -> v2:
>>> - replace msi_msg field by a struct composed of msi_msg and devid
>>>
>>> RFC -> PATCH:
>>> - reword the commit message after change in first patch (uapi)
>>> ---
>>>  include/linux/kvm_host.h | 6 +++++-
>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>>> index c87fe6f..3d2cbb4 100644
>>> --- a/include/linux/kvm_host.h
>>> +++ b/include/linux/kvm_host.h
>>> @@ -317,7 +317,11 @@ struct kvm_kernel_irq_routing_entry {
>>>  			unsigned irqchip;
>>>  			unsigned pin;
>>>  		} irqchip;
>>> -		struct msi_msg msi;
>>> +		struct {
>>> +			struct msi_msg msi;
>>> +			u32 msi_flags;
>>> +			u32 msi_devid;
>> 
>> I'd much rather see them as msi.flags and msi.devid.
> 
> That's not really possible, as msi_msg is the core data structure for
> MSIs, and nobody really expects this tructure to change.
> 
>> I didn't notice a code that passes struct msi_msg anywhere, so using an
>> ad-hoc struct like irqchip or defining a new one would work fine.
> 
> I guess we could have an arm-specific one:
> 
> 		struct arm_msi {
> 			struct msi_msg msg;
> 			u32 flags;
> 			u32 devid;
> 		};
> 
> and use that. Would that be OK with you?

The feature wants to be arch-neutral, so I would rather ignore struct
msi_msg.  kvm_kernel_irq_routing_entry practically mirrors routing
entries from KVM API and there we have:

  struct kvm_msi {
  	__u32 address_lo;
  	__u32 address_hi;
  	__u32 data;
  	__u32 flags;
  	__u32 devid;
  	__u8  pad[12];
  };

I think that something like

  struct {
  	u32 address_lo;
  	u32 address_hi;
  	u32 data;
  	u32 flags;
  	u32 devid;
  } msi;

would get us consistency, minimal changes to existing code, no namespace
hierarchy, and no "." vs "_" mistakes at a good price of some code
duplication and concept separation.

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 2/7] KVM: kvm_host: add devid in kvm_kernel_irq_routing_entry
  2016-07-21 17:22       ` Radim Krčmář
@ 2016-07-21 17:25         ` Marc Zyngier
  2016-07-21 20:47           ` Auger Eric
  0 siblings, 1 reply; 30+ messages in thread
From: Marc Zyngier @ 2016-07-21 17:25 UTC (permalink / raw)
  To: Radim Krčmář
  Cc: Eric Auger, eric.auger.pro, christoffer.dall, andre.przywara,
	drjones, kvmarm, kvm, pbonzini

On 21/07/16 18:22, Radim Krčmář wrote:
> 2016-07-21 17:54+0100, Marc Zyngier:
>> On 21/07/16 17:13, Radim Krčmář wrote:
>>> 2016-07-18 13:25+0000, Eric Auger:
>>>> Extend kvm_kernel_irq_routing_entry to transport the device id
>>>> field, devid. A new flags field makes possible to indicate the
>>>> devid is valid. Those additions are used for ARM GICv3 ITS MSI
>>>> injection.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
>>>>
>>>> ---
>>>> v6 -> v7:
>>>> - added msi_ prefix to flags and dev_id fields
>>>>
>>>> v4 -> v5:
>>>> - add Christoffer's R-b
>>>>
>>>> v2 -> v3:
>>>> - add flags
>>>>
>>>> v1 -> v2:
>>>> - replace msi_msg field by a struct composed of msi_msg and devid
>>>>
>>>> RFC -> PATCH:
>>>> - reword the commit message after change in first patch (uapi)
>>>> ---
>>>>  include/linux/kvm_host.h | 6 +++++-
>>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>>>> index c87fe6f..3d2cbb4 100644
>>>> --- a/include/linux/kvm_host.h
>>>> +++ b/include/linux/kvm_host.h
>>>> @@ -317,7 +317,11 @@ struct kvm_kernel_irq_routing_entry {
>>>>  			unsigned irqchip;
>>>>  			unsigned pin;
>>>>  		} irqchip;
>>>> -		struct msi_msg msi;
>>>> +		struct {
>>>> +			struct msi_msg msi;
>>>> +			u32 msi_flags;
>>>> +			u32 msi_devid;
>>>
>>> I'd much rather see them as msi.flags and msi.devid.
>>
>> That's not really possible, as msi_msg is the core data structure for
>> MSIs, and nobody really expects this tructure to change.
>>
>>> I didn't notice a code that passes struct msi_msg anywhere, so using an
>>> ad-hoc struct like irqchip or defining a new one would work fine.
>>
>> I guess we could have an arm-specific one:
>>
>> 		struct arm_msi {
>> 			struct msi_msg msg;
>> 			u32 flags;
>> 			u32 devid;
>> 		};
>>
>> and use that. Would that be OK with you?
> 
> The feature wants to be arch-neutral, so I would rather ignore struct
> msi_msg.  kvm_kernel_irq_routing_entry practically mirrors routing
> entries from KVM API and there we have:
> 
>   struct kvm_msi {
>   	__u32 address_lo;
>   	__u32 address_hi;
>   	__u32 data;
>   	__u32 flags;
>   	__u32 devid;
>   	__u8  pad[12];
>   };
> 
> I think that something like
> 
>   struct {
>   	u32 address_lo;
>   	u32 address_hi;
>   	u32 data;
>   	u32 flags;
>   	u32 devid;
>   } msi;
> 
> would get us consistency, minimal changes to existing code, no namespace
> hierarchy, and no "." vs "_" mistakes at a good price of some code
> duplication and concept separation.

Fair enough. Eric, can you give this a try?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 2/7] KVM: kvm_host: add devid in kvm_kernel_irq_routing_entry
  2016-07-21 17:25         ` Marc Zyngier
@ 2016-07-21 20:47           ` Auger Eric
  0 siblings, 0 replies; 30+ messages in thread
From: Auger Eric @ 2016-07-21 20:47 UTC (permalink / raw)
  To: Marc Zyngier, Radim Krčmář
  Cc: kvm, andre.przywara, pbonzini, kvmarm, eric.auger.pro

Hi,

On 21/07/2016 19:25, Marc Zyngier wrote:
> On 21/07/16 18:22, Radim Krčmář wrote:
>> 2016-07-21 17:54+0100, Marc Zyngier:
>>> On 21/07/16 17:13, Radim Krčmář wrote:
>>>> 2016-07-18 13:25+0000, Eric Auger:
>>>>> Extend kvm_kernel_irq_routing_entry to transport the device id
>>>>> field, devid. A new flags field makes possible to indicate the
>>>>> devid is valid. Those additions are used for ARM GICv3 ITS MSI
>>>>> injection.
>>>>>
>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
>>>>>
>>>>> ---
>>>>> v6 -> v7:
>>>>> - added msi_ prefix to flags and dev_id fields
>>>>>
>>>>> v4 -> v5:
>>>>> - add Christoffer's R-b
>>>>>
>>>>> v2 -> v3:
>>>>> - add flags
>>>>>
>>>>> v1 -> v2:
>>>>> - replace msi_msg field by a struct composed of msi_msg and devid
>>>>>
>>>>> RFC -> PATCH:
>>>>> - reword the commit message after change in first patch (uapi)
>>>>> ---
>>>>>  include/linux/kvm_host.h | 6 +++++-
>>>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>>>>> index c87fe6f..3d2cbb4 100644
>>>>> --- a/include/linux/kvm_host.h
>>>>> +++ b/include/linux/kvm_host.h
>>>>> @@ -317,7 +317,11 @@ struct kvm_kernel_irq_routing_entry {
>>>>>  			unsigned irqchip;
>>>>>  			unsigned pin;
>>>>>  		} irqchip;
>>>>> -		struct msi_msg msi;
>>>>> +		struct {
>>>>> +			struct msi_msg msi;
>>>>> +			u32 msi_flags;
>>>>> +			u32 msi_devid;
>>>>
>>>> I'd much rather see them as msi.flags and msi.devid.
>>>
>>> That's not really possible, as msi_msg is the core data structure for
>>> MSIs, and nobody really expects this tructure to change.
>>>
>>>> I didn't notice a code that passes struct msi_msg anywhere, so using an
>>>> ad-hoc struct like irqchip or defining a new one would work fine.
>>>
>>> I guess we could have an arm-specific one:
>>>
>>> 		struct arm_msi {
>>> 			struct msi_msg msg;
>>> 			u32 flags;
>>> 			u32 devid;
>>> 		};
>>>
>>> and use that. Would that be OK with you?
>>
>> The feature wants to be arch-neutral, so I would rather ignore struct
>> msi_msg.  kvm_kernel_irq_routing_entry practically mirrors routing
>> entries from KVM API and there we have:
>>
>>   struct kvm_msi {
>>   	__u32 address_lo;
>>   	__u32 address_hi;
>>   	__u32 data;
>>   	__u32 flags;
>>   	__u32 devid;
>>   	__u8  pad[12];
>>   };
>>
>> I think that something like
>>
>>   struct {
>>   	u32 address_lo;
>>   	u32 address_hi;
>>   	u32 data;
>>   	u32 flags;
>>   	u32 devid;
>>   } msi;
>>
>> would get us consistency, minimal changes to existing code, no namespace
>> hierarchy, and no "." vs "_" mistakes at a good price of some code
>> duplication and concept separation.
> 
> Fair enough. Eric, can you give this a try?
Yes I will respin quickly replacing the struct msi_msg msi by the struct
proposed by Radim.

Thanks

Eric
> 
> Thanks,
> 
> 	M.
> 
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 1/7] KVM: api: pass the devid in the msi routing entry
  2016-07-21 17:15       ` Radim Krčmář
@ 2016-07-21 20:48         ` Auger Eric
  0 siblings, 0 replies; 30+ messages in thread
From: Auger Eric @ 2016-07-21 20:48 UTC (permalink / raw)
  To: Radim Krčmář, Andre Przywara
  Cc: eric.auger.pro, marc.zyngier, christoffer.dall, drjones, kvmarm,
	kvm, pbonzini

Hi,

On 21/07/2016 19:15, Radim Krčmář wrote:
> 2016-07-21 17:43+0100, Andre Przywara:
>> Hi Radim,
>>
>> On 21/07/16 17:01, Radim Krčmář wrote:
>>> 2016-07-18 13:25+0000, Eric Auger:
>>>> On ARM, the MSI msg (address and data) comes along with
>>>> out-of-band device ID information. The device ID encodes the
>>>> device that writes the MSI msg. Let's convey the device id in
>>>> kvm_irq_routing_msi and use KVM_MSI_VALID_DEVID flag value in
>>>> kvm_irq_routing_entry to indicate the msi devid is populated.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
>>>>
>>>> ---
>>>>  
>>>> +devid: If KVM_MSI_VALID_DEVID is set, contains a unique device identifier
>>>> +       for the device that wrote the MSI message.
>>>> +       For PCI, this is usually a BFD identifier in the lower 16 bits.
>>>> +
>>>> +The per-VM KVM_CAP_MSI_DEVID capability advertises the requirement to
>>>> +provide the device ID. If this capability is not set, userland cannot
>>>> +rely on the kernel to allow the KVM_MSI_VALID_DEVID flag being set.
>>>
>>> It would be better to enforce this mentioned dependency on set
>>> KVM_CAP_MSI_DEVID, but is the dependency even required?
>>> It seems we were checking flags for zero, so KVM_MSI_VALID_DEVID
>>> couldn't have been set by old userspaces, therefor it is ok to only make
>>> it depend only on the presence of KVM_CAP_MSI_DEVID, like the patch does
>>> now.  (I assume KVM_MSI_VALID_DEVID and KVM_CAP_MSI_DEVID are being
>>> merged at the same time.)
>>>
>>> Then there would be little point in having KVM_CAP_MSI_DEVID enableable,
>>> so does enabling KVM_CAP_MSI_DEVID mean that every MSI must have a valid
>>> devid?
>>
>> KVM_CAP_MSI_DEVID tells userland that it's fine to set the
>> KVM_MSI_VALID_DEVID flag (because the kernel would bark otherwise).
>>
>> KVM_MSI_VALID_DEVID tells the kernel that there is some meaningful
>> device ID data in the field formerly known as "pad".
>>
>> IIRC we started with the VALID_DEVID flag, then found that we need the
>> CAP because we repurposed the pad field.
>>
>> Does that make sense? Admittedly this _is_ confusing ;-)
> 
> It does, thanks.
> Some capability is need and I thought that KVM_CAP_MSI_DEVID has to be
> enabled by userspace before KVM_MSI_VALID_DEVID can be used, which isn't
> the case.  It is enabled conditionally based on vgic ITS ... my bad.
> 

Great

Thanks Andre for the clarification

Eric

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 6/7] KVM: arm/arm64: enable MSI routing
  2016-07-21 16:21   ` Radim Krčmář
@ 2016-07-21 20:50     ` Auger Eric
  0 siblings, 0 replies; 30+ messages in thread
From: Auger Eric @ 2016-07-21 20:50 UTC (permalink / raw)
  To: Radim Krčmář
  Cc: eric.auger.pro, marc.zyngier, christoffer.dall, andre.przywara,
	drjones, kvmarm, kvm, pbonzini

Hi Radim,
On 21/07/2016 18:21, Radim Krčmář wrote:
> 2016-07-18 13:25+0000, Eric Auger:
>> Up to now, only irqchip routing entries could be set. This patch
>> adds the capability to insert MSI routing entries.
>>
>> For ARM64, let's also increase KVM_MAX_IRQ_ROUTES to 4096: this
>> include SPI irqchip routes plus MSI routes. In the future this
>> might be extended.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
>>
>> ---
>> v6 -> v7:
>> - added Andre's R-b
>>
>> v2 -> v3:
>> - remove any reference to KVM_IRQ_ROUTING_EXTENDED_MSI type
>> - unconditionnaly uapi flags and devid downto the kernel
>>   routing entry struct
>> - handle KVM_MSI_VALID_DEVID flag in kvm_set_irq_routing
>> - note about KVM_CAP_MSI_DEVID moved in the first patch file
>>   of the series
>>
>> v1 -> v2:
>> - adapt to new routing entry types
>>
>> RFC -> PATCH:
>> - move api MSI routing updates into that patch file
>> - use new devid field of user api struct
>> ---
>> diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
>> @@ -209,7 +209,7 @@ int kvm_set_irq_routing(struct kvm *kvm,
>>  			goto out;
>>  
>>  		r = -EINVAL;
>> -		if (ue->flags) {
>> +		if (ue->flags & ~KVM_MSI_VALID_DEVID) {
> 
> This allows KVM_MSI_VALID_DEVID flag for all route types, which is not
> what we want.
> To avoid checking separately when setting MSI route in every arch,
> I think we could have a switch (ue->type) here as well.
Yes correct, I will add this type check.

Thanks

Eric
> 
>>  			kfree(e);
>>  			goto out;
>>  		}

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 7/7] KVM: arm: enable KVM_SIGNAL_MSI and MSI routing
  2016-07-21 16:33   ` Radim Krčmář
@ 2016-07-21 21:10     ` Auger Eric
  2016-07-22 13:39       ` Radim Krčmář
  0 siblings, 1 reply; 30+ messages in thread
From: Auger Eric @ 2016-07-21 21:10 UTC (permalink / raw)
  To: Radim Krčmář
  Cc: eric.auger.pro, marc.zyngier, christoffer.dall, andre.przywara,
	drjones, kvmarm, kvm, pbonzini

Hi Radim,

On 21/07/2016 18:33, Radim Krčmář wrote:
> 2016-07-18 13:25+0000, Eric Auger:
>> If the ITS modality is not available, let's simply support MSI
>> injection by transforming the MSI.data into an SPI ID.
>>
>> This becomes possible to use KVM_SIGNAL_MSI ioctl and MSI
>> routing for arm too.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> v6 -> v7:
>> - move vgic_v2m_inject_msi into vgic-irqfd
>>
>> v4 -> v5:
>> - on vgic_v2m_inject_msi check the msi->data is within the SPI range
>> - move KVM_HAVE_MSI in the KVM section (to be symetrical with ARM64)
>>
>> v2 -> v3:
>> - reword the commit message
>> - add sanity check about devid provision
>>
>> v1 -> v2:
>> - introduce vgic_v2m_inject_msi in vgic-v2-emul.c following Andre's
>>   advice
>> ---
>> diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c
>> +static int vgic_v2m_inject_msi(struct kvm *kvm, struct kvm_msi *msi)
>> +{
>> +	if (msi->flags & KVM_MSI_VALID_DEVID)
>> +		return -EINVAL;
>> +	if (!vgic_valid_spi(kvm, msi->data))
>> +		return -EINVAL;
>> +
>> +	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);
> 
> Hm, this isn't very MSI related ...
> 
> arm already has KVM_IRQ_LINE/kvm_vm_ioctl_irq_line with
> KVM_ARM_IRQ_TYPE_SPI that does
>   kvm_vgic_inject_irq(kvm, 0, irq_num, level)
> 
> Is that interface lacking?

You mean KVM_SIGNAL_MSI? Well at QEMU level, for ARM/ARM64 is doesn't.
For kvm-tools I guess, Andre manages without.

My first feeling was it is part of the KVM API and we can implement it
easily for GICv2M, as we do for GICv3 ITS . This can avoid a user app to
do what QEMU implements as "kvm_gsi_direct_mapping" and manage the
translation into the semantic of the ARM GSI.

But Well, if you prefer we do not implement it for GICv2M, since
considered as far fetched I can remove this patch.

Thank you for your time

Best Regards

Eric
> 
> Thanks.
> 
>> +}
>> +
>> +/**
>>   * kvm_set_msi: inject the MSI corresponding to the
>>   * MSI routing entry
>>   *
>> @@ -96,7 +113,7 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
>>  	msi.devid = e->msi_devid;
>>  
>>  	if (!vgic_has_its(kvm))
>> -		return -ENODEV;
>> +		return vgic_v2m_inject_msi(kvm, &msi);
>>  
>>  	return vgic_its_inject_msi(kvm, &msi);
>>  }
>> -- 
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 7/7] KVM: arm: enable KVM_SIGNAL_MSI and MSI routing
  2016-07-21 21:10     ` Auger Eric
@ 2016-07-22 13:39       ` Radim Krčmář
  2016-07-22 13:52         ` Auger Eric
  0 siblings, 1 reply; 30+ messages in thread
From: Radim Krčmář @ 2016-07-22 13:39 UTC (permalink / raw)
  To: Auger Eric
  Cc: kvm, marc.zyngier, andre.przywara, pbonzini, kvmarm, eric.auger.pro

2016-07-21 23:10+0200, Auger Eric:
> On 21/07/2016 18:33, Radim Krčmář wrote:
>> 2016-07-18 13:25+0000, Eric Auger:
>>> If the ITS modality is not available, let's simply support MSI
>>> injection by transforming the MSI.data into an SPI ID.
>>>
>>> This becomes possible to use KVM_SIGNAL_MSI ioctl and MSI
>>> routing for arm too.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>
>>> ---
>>> diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c
>>> +static int vgic_v2m_inject_msi(struct kvm *kvm, struct kvm_msi *msi)
>>> +{
>>> +	if (msi->flags & KVM_MSI_VALID_DEVID)
>>> +		return -EINVAL;
>>> +	if (!vgic_valid_spi(kvm, msi->data))
>>> +		return -EINVAL;
>>> +
>>> +	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);
>> 
>> Hm, this isn't very MSI related ...
>> 
>> arm already has KVM_IRQ_LINE/kvm_vm_ioctl_irq_line with
>> KVM_ARM_IRQ_TYPE_SPI that does
>>   kvm_vgic_inject_irq(kvm, 0, irq_num, level)
>> 
>> Is that interface lacking?
> 
> You mean KVM_SIGNAL_MSI? Well at QEMU level, for ARM/ARM64 is doesn't.

No, I meant KVM_IRQ_LINE, the one that is used to deliver SPI today.
Or isn't it?

> For kvm-tools I guess, Andre manages without.
> 
> My first feeling was it is part of the KVM API and we can implement it
> easily for GICv2M, as we do for GICv3 ITS . This can avoid a user app to
> do what QEMU implements as "kvm_gsi_direct_mapping" and manage the
> translation into the semantic of the ARM GSI.

I think that reusing KVM_SIGNAL_MSI and KVM_IRQ_ROUTING_MSI for SPI is
unfortunate.

SPI only uses msi.data, which makes remaining fields in the msi struct
arbitrary and [5/7] defined KVM_IRQ_ROUTING_IRQCHIP for SPI, so two
route types now do the same, but only sometimes (without ITS), which
makes the situation even less understandable ...

Delivering SPI as KVM_IRQ_ROUTING_IRQCHIP seems more sensible and if we
wanted ad-hoc delivery of KVM_IRQ_ROUTING_IRQCHIP, then I would prefer a
new interface to two different meanings for KVM_SIGNAL_MSI:
KVM_SIGNAL_MSI was created because we didn't have anything that could
inject an interrupt without setting up a route with KVM_SET_GSI_ROUTING
and we are still missing a generic interface to do that.

> But Well, if you prefer we do not implement it for GICv2M, since
> considered as far fetched I can remove this patch.

I do, thanks.  Documentation in [6/7] was ahead and needs changing then.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 7/7] KVM: arm: enable KVM_SIGNAL_MSI and MSI routing
  2016-07-22 13:39       ` Radim Krčmář
@ 2016-07-22 13:52         ` Auger Eric
  2016-07-22 13:56           ` Radim Krčmář
  0 siblings, 1 reply; 30+ messages in thread
From: Auger Eric @ 2016-07-22 13:52 UTC (permalink / raw)
  To: Radim Krčmář
  Cc: eric.auger.pro, marc.zyngier, christoffer.dall, andre.przywara,
	drjones, kvmarm, kvm, pbonzini

Hi Radim,

On 22/07/2016 15:39, Radim Krčmář wrote:
> 2016-07-21 23:10+0200, Auger Eric:
>> On 21/07/2016 18:33, Radim Krčmář wrote:
>>> 2016-07-18 13:25+0000, Eric Auger:
>>>> If the ITS modality is not available, let's simply support MSI
>>>> injection by transforming the MSI.data into an SPI ID.
>>>>
>>>> This becomes possible to use KVM_SIGNAL_MSI ioctl and MSI
>>>> routing for arm too.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>
>>>> ---
>>>> diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c
>>>> +static int vgic_v2m_inject_msi(struct kvm *kvm, struct kvm_msi *msi)
>>>> +{
>>>> +	if (msi->flags & KVM_MSI_VALID_DEVID)
>>>> +		return -EINVAL;
>>>> +	if (!vgic_valid_spi(kvm, msi->data))
>>>> +		return -EINVAL;
>>>> +
>>>> +	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);
>>>
>>> Hm, this isn't very MSI related ...
>>>
>>> arm already has KVM_IRQ_LINE/kvm_vm_ioctl_irq_line with
>>> KVM_ARM_IRQ_TYPE_SPI that does
>>>   kvm_vgic_inject_irq(kvm, 0, irq_num, level)
>>>
>>> Is that interface lacking?
>>
>> You mean KVM_SIGNAL_MSI? Well at QEMU level, for ARM/ARM64 is doesn't.
> 
> No, I meant KVM_IRQ_LINE, the one that is used to deliver SPI today.
> Or isn't it?
> 
>> For kvm-tools I guess, Andre manages without.
>>
>> My first feeling was it is part of the KVM API and we can implement it
>> easily for GICv2M, as we do for GICv3 ITS . This can avoid a user app to
>> do what QEMU implements as "kvm_gsi_direct_mapping" and manage the
>> translation into the semantic of the ARM GSI.
> 
> I think that reusing KVM_SIGNAL_MSI and KVM_IRQ_ROUTING_MSI for SPI is
> unfortunate.
> 
> SPI only uses msi.data, which makes remaining fields in the msi struct
> arbitrary and [5/7] defined KVM_IRQ_ROUTING_IRQCHIP for SPI, so two
> route types now do the same, but only sometimes (without ITS), which
> makes the situation even less understandable ...
> 
> Delivering SPI as KVM_IRQ_ROUTING_IRQCHIP seems more sensible and if we
> wanted ad-hoc delivery of KVM_IRQ_ROUTING_IRQCHIP, then I would prefer a
> new interface to two different meanings for KVM_SIGNAL_MSI:
> KVM_SIGNAL_MSI was created because we didn't have anything that could
> inject an interrupt without setting up a route with KVM_SET_GSI_ROUTING
> and we are still missing a generic interface to do that.
> 
>> But Well, if you prefer we do not implement it for GICv2M, since
>> considered as far fetched I can remove this patch.
> 
> I do, thanks.  Documentation in [6/7] was ahead and needs changing then.

Argh just saw your reply after sending v8. Will respin immediatly.

Sorry for the confusion

Thanks

Eric
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 7/7] KVM: arm: enable KVM_SIGNAL_MSI and MSI routing
  2016-07-22 13:52         ` Auger Eric
@ 2016-07-22 13:56           ` Radim Krčmář
  2016-07-22 13:59             ` Auger Eric
  0 siblings, 1 reply; 30+ messages in thread
From: Radim Krčmář @ 2016-07-22 13:56 UTC (permalink / raw)
  To: Auger Eric
  Cc: eric.auger.pro, marc.zyngier, christoffer.dall, andre.przywara,
	drjones, kvmarm, kvm, pbonzini

2016-07-22 15:52+0200, Auger Eric:
> On 22/07/2016 15:39, Radim Krčmář wrote:
>> 2016-07-21 23:10+0200, Auger Eric:
>>> On 21/07/2016 18:33, Radim Krčmář wrote:
>>>> 2016-07-18 13:25+0000, Eric Auger:
>>>>> If the ITS modality is not available, let's simply support MSI
>>>>> injection by transforming the MSI.data into an SPI ID.
>>>>>
>>>>> This becomes possible to use KVM_SIGNAL_MSI ioctl and MSI
>>>>> routing for arm too.
>>>>>
>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>>
>>>>> ---
>>>>> diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c
>>>>> +static int vgic_v2m_inject_msi(struct kvm *kvm, struct kvm_msi *msi)
>>>>> +{
>>>>> +	if (msi->flags & KVM_MSI_VALID_DEVID)
>>>>> +		return -EINVAL;
>>>>> +	if (!vgic_valid_spi(kvm, msi->data))
>>>>> +		return -EINVAL;
>>>>> +
>>>>> +	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);
>>>>
>>>> Hm, this isn't very MSI related ...
>>>>
>>>> arm already has KVM_IRQ_LINE/kvm_vm_ioctl_irq_line with
>>>> KVM_ARM_IRQ_TYPE_SPI that does
>>>>   kvm_vgic_inject_irq(kvm, 0, irq_num, level)
>>>>
>>>> Is that interface lacking?
>>>
>>> You mean KVM_SIGNAL_MSI? Well at QEMU level, for ARM/ARM64 is doesn't.
>> 
>> No, I meant KVM_IRQ_LINE, the one that is used to deliver SPI today.
>> Or isn't it?
>> 
>>> For kvm-tools I guess, Andre manages without.
>>>
>>> My first feeling was it is part of the KVM API and we can implement it
>>> easily for GICv2M, as we do for GICv3 ITS . This can avoid a user app to
>>> do what QEMU implements as "kvm_gsi_direct_mapping" and manage the
>>> translation into the semantic of the ARM GSI.
>> 
>> I think that reusing KVM_SIGNAL_MSI and KVM_IRQ_ROUTING_MSI for SPI is
>> unfortunate.
>> 
>> SPI only uses msi.data, which makes remaining fields in the msi struct
>> arbitrary and [5/7] defined KVM_IRQ_ROUTING_IRQCHIP for SPI, so two
>> route types now do the same, but only sometimes (without ITS), which
>> makes the situation even less understandable ...
>> 
>> Delivering SPI as KVM_IRQ_ROUTING_IRQCHIP seems more sensible and if we
>> wanted ad-hoc delivery of KVM_IRQ_ROUTING_IRQCHIP, then I would prefer a
>> new interface to two different meanings for KVM_SIGNAL_MSI:
>> KVM_SIGNAL_MSI was created because we didn't have anything that could
>> inject an interrupt without setting up a route with KVM_SET_GSI_ROUTING
>> and we are still missing a generic interface to do that.
>> 
>>> But Well, if you prefer we do not implement it for GICv2M, since
>>> considered as far fetched I can remove this patch.
>> 
>> I do, thanks.  Documentation in [6/7] was ahead and needs changing then.
> 
> Argh just saw your reply after sending v8. Will respin immediatly.
> 
> Sorry for the confusion

No problem.  Give me half an hour for a review, please. :)

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [RFC v7 7/7] KVM: arm: enable KVM_SIGNAL_MSI and MSI routing
  2016-07-22 13:56           ` Radim Krčmář
@ 2016-07-22 13:59             ` Auger Eric
  0 siblings, 0 replies; 30+ messages in thread
From: Auger Eric @ 2016-07-22 13:59 UTC (permalink / raw)
  To: Radim Krčmář
  Cc: eric.auger.pro, marc.zyngier, christoffer.dall, andre.przywara,
	drjones, kvmarm, kvm, pbonzini



On 22/07/2016 15:56, Radim Krčmář wrote:
> 2016-07-22 15:52+0200, Auger Eric:
>> On 22/07/2016 15:39, Radim Krčmář wrote:
>>> 2016-07-21 23:10+0200, Auger Eric:
>>>> On 21/07/2016 18:33, Radim Krčmář wrote:
>>>>> 2016-07-18 13:25+0000, Eric Auger:
>>>>>> If the ITS modality is not available, let's simply support MSI
>>>>>> injection by transforming the MSI.data into an SPI ID.
>>>>>>
>>>>>> This becomes possible to use KVM_SIGNAL_MSI ioctl and MSI
>>>>>> routing for arm too.
>>>>>>
>>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>>>
>>>>>> ---
>>>>>> diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c
>>>>>> +static int vgic_v2m_inject_msi(struct kvm *kvm, struct kvm_msi *msi)
>>>>>> +{
>>>>>> +	if (msi->flags & KVM_MSI_VALID_DEVID)
>>>>>> +		return -EINVAL;
>>>>>> +	if (!vgic_valid_spi(kvm, msi->data))
>>>>>> +		return -EINVAL;
>>>>>> +
>>>>>> +	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);
>>>>>
>>>>> Hm, this isn't very MSI related ...
>>>>>
>>>>> arm already has KVM_IRQ_LINE/kvm_vm_ioctl_irq_line with
>>>>> KVM_ARM_IRQ_TYPE_SPI that does
>>>>>   kvm_vgic_inject_irq(kvm, 0, irq_num, level)
>>>>>
>>>>> Is that interface lacking?
>>>>
>>>> You mean KVM_SIGNAL_MSI? Well at QEMU level, for ARM/ARM64 is doesn't.
>>>
>>> No, I meant KVM_IRQ_LINE, the one that is used to deliver SPI today.
>>> Or isn't it?
>>>
>>>> For kvm-tools I guess, Andre manages without.
>>>>
>>>> My first feeling was it is part of the KVM API and we can implement it
>>>> easily for GICv2M, as we do for GICv3 ITS . This can avoid a user app to
>>>> do what QEMU implements as "kvm_gsi_direct_mapping" and manage the
>>>> translation into the semantic of the ARM GSI.
>>>
>>> I think that reusing KVM_SIGNAL_MSI and KVM_IRQ_ROUTING_MSI for SPI is
>>> unfortunate.
>>>
>>> SPI only uses msi.data, which makes remaining fields in the msi struct
>>> arbitrary and [5/7] defined KVM_IRQ_ROUTING_IRQCHIP for SPI, so two
>>> route types now do the same, but only sometimes (without ITS), which
>>> makes the situation even less understandable ...
>>>
>>> Delivering SPI as KVM_IRQ_ROUTING_IRQCHIP seems more sensible and if we
>>> wanted ad-hoc delivery of KVM_IRQ_ROUTING_IRQCHIP, then I would prefer a
>>> new interface to two different meanings for KVM_SIGNAL_MSI:
>>> KVM_SIGNAL_MSI was created because we didn't have anything that could
>>> inject an interrupt without setting up a route with KVM_SET_GSI_ROUTING
>>> and we are still missing a generic interface to do that.
>>>
>>>> But Well, if you prefer we do not implement it for GICv2M, since
>>>> considered as far fetched I can remove this patch.
>>>
>>> I do, thanks.  Documentation in [6/7] was ahead and needs changing then.
>>
>> Argh just saw your reply after sending v8. Will respin immediatly.
>>
>> Sorry for the confusion
> 
> No problem.  Give me half an hour for a review, please. :)
Sure

Eric
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread
end of thread, other threads:[~2016-07-22 13:59 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-18 13:25 [RFC v7 0/7] KVM: arm/arm64: gsi routing support Eric Auger
2016-07-18 13:25 ` [RFC v7 1/7] KVM: api: pass the devid in the msi routing entry Eric Auger
2016-07-21 16:01   ` Radim Krčmář
2016-07-21 16:43     ` Andre Przywara
2016-07-21 17:15       ` Radim Krčmář
2016-07-21 20:48         ` Auger Eric
2016-07-18 13:25 ` [RFC v7 2/7] KVM: kvm_host: add devid in kvm_kernel_irq_routing_entry Eric Auger
2016-07-21 16:13   ` Radim Krčmář
2016-07-21 16:54     ` Marc Zyngier
2016-07-21 17:22       ` Radim Krčmář
2016-07-21 17:25         ` Marc Zyngier
2016-07-21 20:47           ` Auger Eric
2016-07-18 13:25 ` [RFC v7 3/7] KVM: irqchip: convey devid to kvm_set_msi Eric Auger
2016-07-18 13:25 ` [RFC v7 4/7] KVM: move kvm_setup_default/empty_irq_routing declaration in arch specific header Eric Auger
2016-07-18 13:25 ` [RFC v7 5/7] KVM: arm/arm64: enable irqchip routing Eric Auger
2016-07-19 14:56   ` Marc Zyngier
2016-07-19 15:46     ` Paolo Bonzini
2016-07-19 16:16       ` Marc Zyngier
2016-07-20  7:31         ` Auger Eric
2016-07-21 15:33           ` Marc Zyngier
2016-07-18 13:25 ` [RFC v7 6/7] KVM: arm/arm64: enable MSI routing Eric Auger
2016-07-21 16:21   ` Radim Krčmář
2016-07-21 20:50     ` Auger Eric
2016-07-18 13:25 ` [RFC v7 7/7] KVM: arm: enable KVM_SIGNAL_MSI and " Eric Auger
2016-07-21 16:33   ` Radim Krčmář
2016-07-21 21:10     ` Auger Eric
2016-07-22 13:39       ` Radim Krčmář
2016-07-22 13:52         ` Auger Eric
2016-07-22 13:56           ` Radim Krčmář
2016-07-22 13:59             ` Auger Eric

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.