[PATCH v2 0/7] KVM: arm/arm64: gsi routing support

[PATCH v2 0/7] KVM: arm/arm64: gsi routing support

[Eric Auger]

With the advent of GICv3 ITS in-kernel emulation, KVM GSI routing
appears to be requested. More specifically MSI routing is needed.
irqchip routing does not sound to be really useful on arm but usage of
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 without GICv3 ITS was tested using APM Xgene-I
  (qemu VIRTIO-PCI vhost net without gsi_direct_mapping).
- MSI routing with GICv3 ITS is *NOT* tested.

Code can be found at https://git.linaro.org/people/eric.auger/linux.git/shortlog/refs/heads/v4.2-rc1-gsi-routing-v2

It applies on Andre's [PATCH 00/13] arm64: KVM: GICv3 ITS emulation
(http://www.spinics.net/lists/kvm/msg117402.html)

History:

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: introduce KVM_IRQ_ROUTING_EXTENDED_MSI
  KVM: kvm_host: add devid in kvm_kernel_irq_routing_entry
  KVM: irqchip: convey devid to kvm_set_msi
  KVM: arm/arm64: enable irqchip routing
  KVM: arm/arm64: build a default routing table
  KVM: arm/arm64: enable MSI routing
  KVM: arm: implement kvm_set_msi by gsi direct mapping

 Documentation/virtual/kvm/api.txt |  32 ++++++++++--
 arch/arm/include/asm/kvm_host.h   |   2 +
 arch/arm/kvm/Kconfig              |   3 ++
 arch/arm/kvm/Makefile             |   2 +-
 arch/arm64/include/asm/kvm_host.h |   1 +
 arch/arm64/kvm/Kconfig            |   2 +
 arch/arm64/kvm/Makefile           |   2 +-
 include/kvm/arm_vgic.h            |   9 ----
 include/linux/kvm_host.h          |   7 ++-
 include/uapi/linux/kvm.h          |   6 ++-
 virt/kvm/arm/vgic-v2-emul.c       |  12 +++++
 virt/kvm/arm/vgic.c               | 101 +++++++++++++++++++++++++++++---------
 virt/kvm/eventfd.c                |   6 ++-
 virt/kvm/irqchip.c                |  12 ++++-
 14 files changed, 153 insertions(+), 44 deletions(-)

-- 
1.9.1

[PATCH v2 0/7] KVM: arm/arm64: gsi routing support

[Eric Auger]

With the advent of GICv3 ITS in-kernel emulation, KVM GSI routing
appears to be requested. More specifically MSI routing is needed.
irqchip routing does not sound to be really useful on arm but usage of
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 without GICv3 ITS was tested using APM Xgene-I
  (qemu VIRTIO-PCI vhost net without gsi_direct_mapping).
- MSI routing with GICv3 ITS is *NOT* tested.

Code can be found at https://git.linaro.org/people/eric.auger/linux.git/shortlog/refs/heads/v4.2-rc1-gsi-routing-v2

It applies on Andre's [PATCH 00/13] arm64: KVM: GICv3 ITS emulation
(http://www.spinics.net/lists/kvm/msg117402.html)

History:

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: introduce KVM_IRQ_ROUTING_EXTENDED_MSI
  KVM: kvm_host: add devid in kvm_kernel_irq_routing_entry
  KVM: irqchip: convey devid to kvm_set_msi
  KVM: arm/arm64: enable irqchip routing
  KVM: arm/arm64: build a default routing table
  KVM: arm/arm64: enable MSI routing
  KVM: arm: implement kvm_set_msi by gsi direct mapping

 Documentation/virtual/kvm/api.txt |  32 ++++++++++--
 arch/arm/include/asm/kvm_host.h   |   2 +
 arch/arm/kvm/Kconfig              |   3 ++
 arch/arm/kvm/Makefile             |   2 +-
 arch/arm64/include/asm/kvm_host.h |   1 +
 arch/arm64/kvm/Kconfig            |   2 +
 arch/arm64/kvm/Makefile           |   2 +-
 include/kvm/arm_vgic.h            |   9 ----
 include/linux/kvm_host.h          |   7 ++-
 include/uapi/linux/kvm.h          |   6 ++-
 virt/kvm/arm/vgic-v2-emul.c       |  12 +++++
 virt/kvm/arm/vgic.c               | 101 +++++++++++++++++++++++++++++---------
 virt/kvm/eventfd.c                |   6 ++-
 virt/kvm/irqchip.c                |  12 ++++-
 14 files changed, 153 insertions(+), 44 deletions(-)

-- 
1.9.1

[PATCH v2 1/7] KVM: api: introduce KVM_IRQ_ROUTING_EXTENDED_MSI

[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 a new routing entry type to
indicate the devid is populated.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

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 | 10 +++++++++-
 include/uapi/linux/kvm.h          |  6 +++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index bea8de7..3d920cd 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1453,6 +1453,7 @@ struct kvm_irq_routing_entry {
 #define KVM_IRQ_ROUTING_IRQCHIP 1
 #define KVM_IRQ_ROUTING_MSI 2
 #define KVM_IRQ_ROUTING_S390_ADAPTER 3
+#define KVM_IRQ_ROUTING_EXTENDED_MSI 4
 
 No flags are specified so far, the corresponding field must be set to zero.
 
@@ -1465,9 +1466,16 @@ struct kvm_irq_routing_msi {
 	__u32 address_lo;
 	__u32 address_hi;
 	__u32 data;
-	__u32 pad;
+	union {
+		__u32 pad;
+		__u32 devid;
+	};
 };
 
+for KVM_IRQ_ROUTING_EXTENDED_MSI routing entry type, the kvm_irq_routing_msi
+routing entry is used and devid is populated with the device ID that wrote
+the MSI message. For PCI, this is usually a BFD identifier in the lower 16 bits.
+
 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 9a261e5..39ec164 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -829,7 +829,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 {
@@ -844,6 +847,7 @@ struct kvm_irq_routing_s390_adapter {
 #define KVM_IRQ_ROUTING_IRQCHIP 1
 #define KVM_IRQ_ROUTING_MSI 2
 #define KVM_IRQ_ROUTING_S390_ADAPTER 3
+#define KVM_IRQ_ROUTING_EXTENDED_MSI 4
 
 struct kvm_irq_routing_entry {
 	__u32 gsi;
-- 
1.9.1

[PATCH v2 1/7] KVM: api: introduce KVM_IRQ_ROUTING_EXTENDED_MSI

[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 a new routing entry type to
indicate the devid is populated.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

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 | 10 +++++++++-
 include/uapi/linux/kvm.h          |  6 +++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index bea8de7..3d920cd 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1453,6 +1453,7 @@ struct kvm_irq_routing_entry {
 #define KVM_IRQ_ROUTING_IRQCHIP 1
 #define KVM_IRQ_ROUTING_MSI 2
 #define KVM_IRQ_ROUTING_S390_ADAPTER 3
+#define KVM_IRQ_ROUTING_EXTENDED_MSI 4
 
 No flags are specified so far, the corresponding field must be set to zero.
 
@@ -1465,9 +1466,16 @@ struct kvm_irq_routing_msi {
 	__u32 address_lo;
 	__u32 address_hi;
 	__u32 data;
-	__u32 pad;
+	union {
+		__u32 pad;
+		__u32 devid;
+	};
 };
 
+for KVM_IRQ_ROUTING_EXTENDED_MSI routing entry type, the kvm_irq_routing_msi
+routing entry is used and devid is populated with the device ID that wrote
+the MSI message. For PCI, this is usually a BFD identifier in the lower 16 bits.
+
 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 9a261e5..39ec164 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -829,7 +829,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 {
@@ -844,6 +847,7 @@ struct kvm_irq_routing_s390_adapter {
 #define KVM_IRQ_ROUTING_IRQCHIP 1
 #define KVM_IRQ_ROUTING_MSI 2
 #define KVM_IRQ_ROUTING_S390_ADAPTER 3
+#define KVM_IRQ_ROUTING_EXTENDED_MSI 4
 
 struct kvm_irq_routing_entry {
 	__u32 gsi;
-- 
1.9.1

[PATCH v2 2/7] KVM: kvm_host: add devid in kvm_kernel_irq_routing_entry

[Eric Auger]

Extend kvm_kernel_irq_routing_entry to transport devid. This is
needed for ARM. Its validity depends on the routing type entry.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

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 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 9564fd7..cac0fe4 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -321,7 +321,10 @@ struct kvm_kernel_irq_routing_entry {
 			unsigned irqchip;
 			unsigned pin;
 		} irqchip;
-		struct msi_msg msi;
+		struct {
+			struct msi_msg msi;
+			u32 devid;
+		};
 		struct kvm_s390_adapter_int adapter;
 	};
 	struct hlist_node link;
-- 
1.9.1

[PATCH v2 2/7] KVM: kvm_host: add devid in kvm_kernel_irq_routing_entry

[Eric Auger]

Extend kvm_kernel_irq_routing_entry to transport devid. This is
needed for ARM. Its validity depends on the routing type entry.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

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 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 9564fd7..cac0fe4 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -321,7 +321,10 @@ struct kvm_kernel_irq_routing_entry {
 			unsigned irqchip;
 			unsigned pin;
 		} irqchip;
-		struct msi_msg msi;
+		struct {
+			struct msi_msg msi;
+			u32 devid;
+		};
 		struct kvm_s390_adapter_int adapter;
 	};
 	struct hlist_node link;
-- 
1.9.1

[PATCH v2 3/7] KVM: irqchip: convey devid to kvm_set_msi

[Eric Auger]

on ARM, a devid field is populated in kvm_msi struct in case the
flag is set to KVM_MSI_VALID_DEVID. Let's populate the corresponding
kvm_kernel_irq_routing_entry devid field and set the msi type to
KVM_IRQ_ROUTING_EXTENDED_MSI.

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 virt/kvm/irqchip.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
index 21c1424..e678f8a 100644
--- a/virt/kvm/irqchip.c
+++ b/virt/kvm/irqchip.c
@@ -72,9 +72,17 @@ 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))
 		return -EINVAL;
 
+	if (msi->flags & KVM_MSI_VALID_DEVID) {
+		route.devid = msi->devid;
+		route.type = KVM_IRQ_ROUTING_EXTENDED_MSI;
+	} else if (!msi->flags)
+		return -EINVAL;
+
+	/* historically the route.type was not set */
+
 	route.msi.address_lo = msi->address_lo;
 	route.msi.address_hi = msi->address_hi;
 	route.msi.data = msi->data;
-- 
1.9.1

[PATCH v2 3/7] KVM: irqchip: convey devid to kvm_set_msi

[Eric Auger]

on ARM, a devid field is populated in kvm_msi struct in case the
flag is set to KVM_MSI_VALID_DEVID. Let's populate the corresponding
kvm_kernel_irq_routing_entry devid field and set the msi type to
KVM_IRQ_ROUTING_EXTENDED_MSI.

Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 virt/kvm/irqchip.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
index 21c1424..e678f8a 100644
--- a/virt/kvm/irqchip.c
+++ b/virt/kvm/irqchip.c
@@ -72,9 +72,17 @@ 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))
 		return -EINVAL;
 
+	if (msi->flags & KVM_MSI_VALID_DEVID) {
+		route.devid = msi->devid;
+		route.type = KVM_IRQ_ROUTING_EXTENDED_MSI;
+	} else if (!msi->flags)
+		return -EINVAL;
+
+	/* historically the route.type was not set */
+
 	route.msi.address_lo = msi->address_lo;
 	route.msi.address_hi = msi->address_hi;
 	route.msi.data = msi->data;
-- 
1.9.1

[PATCH v2 4/7] KVM: arm/arm64: enable irqchip routing

[Eric Auger]

This patch adds compilation and link against irqchip.

On ARM, irqchip routing is not really useful since there is
a single irqchip. However main motivation behind using irqchip
code is to enable MSI routing code. With the support of in-kernel
GICv3 ITS emulation, it now seems to be a MUST HAVE requirement.

Functions previously implemented in vgic.c and substitute
to more complex irqchip implementation are removed:

- kvm_send_userspace_msi
- kvm_irq_map_chip_pin
- kvm_set_irq
- kvm_irq_map_gsi.

They implemented a kernel default identity GSI routing. This is now
replaced by user-side provided routing.

Routing standard hooks are now implemented in vgic:
- kvm_set_routing_entry
- kvm_set_irq
- kvm_set_msi

Both HAVE_KVM_IRQCHIP and HAVE_KVM_IRQ_ROUTING are defined.
KVM_CAP_IRQ_ROUTING is advertised and KVM_SET_GSI_ROUTING is allowed.

MSI routing is not yet allowed.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---
v1 -> v2:
- fix bug reported by Andre related to msi.flags and msi.devid setting
  in kvm_send_userspace_msi
- avoid injecting reserved IRQ numbers in vgic_irqfd_set_irq

RFC -> PATCH
- reword api.txt:
  x move MSI routing comments in a subsequent patch,
  x clearly state GSI routing does not apply to KVM_IRQ_LINE
---
 Documentation/virtual/kvm/api.txt | 12 ++++---
 arch/arm/include/asm/kvm_host.h   |  2 ++
 arch/arm/kvm/Kconfig              |  2 ++
 arch/arm/kvm/Makefile             |  2 +-
 arch/arm64/include/asm/kvm_host.h |  1 +
 arch/arm64/kvm/Kconfig            |  2 ++
 arch/arm64/kvm/Makefile           |  2 +-
 include/kvm/arm_vgic.h            |  9 -----
 virt/kvm/arm/vgic.c               | 71 +++++++++++++++++++++++++--------------
 virt/kvm/irqchip.c                |  2 ++
 10 files changed, 65 insertions(+), 40 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 3d920cd..9276dac 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1421,13 +1421,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;
@@ -2335,9 +2338,10 @@ Note that closing the resamplefd is not sufficient to disable the
 irqfd.  The KVM_IRQFD_FLAG_RESAMPLE is only necessary on assignment
 and need not be specified with KVM_IRQFD_FLAG_DEASSIGN.
 
-On ARM/ARM64, the gsi field in the kvm_irqfd struct specifies the Shared
-Peripheral Interrupt (SPI) index, such that the GIC interrupt ID is
-given by gsi + 32.
+On arm/arm64, gsi routing being supported, the following can happen:
+- in case no routing entry is associated to this gsi, injection fails
+- in case the gsi is associated to an irqchip routing entry,
+  irqchip.pin + 32 corresponds to the injected SPI ID.
 
 4.76 KVM_PPC_ALLOCATE_HTAB
 
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index e896d2c..9871441 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -42,6 +42,8 @@
 
 #define KVM_VCPU_MAX_FEATURES 2
 
+#define KVM_IRQCHIP_NUM_PINS 988 /* 1020 -32 is the number of SPI */
+
 #include <kvm/arm_vgic.h>
 
 u32 *kvm_vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num, u32 mode);
diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
index bfb915d..151e710 100644
--- a/arch/arm/kvm/Kconfig
+++ b/arch/arm/kvm/Kconfig
@@ -31,6 +31,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 c5eef02c..1a8f48a 100644
--- a/arch/arm/kvm/Makefile
+++ b/arch/arm/kvm/Makefile
@@ -15,7 +15,7 @@ AFLAGS_init.o := -Wa,-march=armv7-a$(plus_virt)
 AFLAGS_interrupts.o := -Wa,-march=armv7-a$(plus_virt)
 
 KVM := ../../../virt/kvm
-kvm-arm-y = $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o $(KVM)/vfio.o
+kvm-arm-y = $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o $(KVM)/vfio.o $(KVM)/irqchip.o
 
 obj-y += kvm-arm.o init.o interrupts.o
 obj-y += arm.o handle_exit.o guest.o mmu.o emulate.o reset.o
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 2709db2..f6201eb 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -44,6 +44,7 @@
 #include <kvm/arm_arch_timer.h>
 
 #define KVM_VCPU_MAX_FEATURES 3
+#define KVM_IRQCHIP_NUM_PINS 988 /* 1020 -32 is the number of SPI */
 
 int __attribute_const__ kvm_target_cpu(void);
 int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index ff9722f..1a9900d 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -32,6 +32,8 @@ config KVM
 	select HAVE_KVM_EVENTFD
 	select HAVE_KVM_IRQFD
 	select HAVE_KVM_MSI
+	select HAVE_KVM_IRQCHIP
+	select HAVE_KVM_IRQ_ROUTING
 	---help---
 	  Support hosting virtualized guest machines.
 
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 9803307..90a08457 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -11,7 +11,7 @@ ARM=../../../arch/arm/kvm
 
 obj-$(CONFIG_KVM_ARM_HOST) += kvm.o
 
-kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o $(KVM)/vfio.o
+kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o $(KVM)/vfio.o $(KVM)/irqchip.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(ARM)/arm.o $(ARM)/mmu.o $(ARM)/mmio.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(ARM)/psci.o $(ARM)/perf.o
 
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 8f1be6a..10dc596 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -366,13 +366,4 @@ static inline int vgic_v3_probe(struct device_node *vgic_node,
 }
 #endif
 
-#ifdef CONFIG_HAVE_KVM_MSI
-int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi);
-#else
-static inline int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi)
-{
-	return -ENODEV;
-}
-#endif
-
 #endif
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 3630971..6c6c25e 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -2215,44 +2215,65 @@ out_free_irq:
 	return ret;
 }
 
-int kvm_irq_map_gsi(struct kvm *kvm,
-		    struct kvm_kernel_irq_routing_entry *entries,
-		    int gsi)
+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;
-}
-
-int kvm_irq_map_chip_pin(struct kvm *kvm, unsigned irqchip, unsigned pin)
-{
-	return pin;
-}
-
-int kvm_set_irq(struct kvm *kvm, int irq_source_id,
-		u32 irq, int level, bool line_status)
-{
-	unsigned int spi = irq + VGIC_NR_PRIVATE_IRQS;
+	unsigned int spi_id = e->irqchip.pin + VGIC_NR_PRIVATE_IRQS;
 
-	trace_kvm_set_irq(irq, level, irq_source_id);
+	trace_kvm_set_irq(spi_id, level, irq_source_id);
 
 	BUG_ON(!vgic_initialized(kvm));
 
-	return kvm_vgic_inject_irq(kvm, 0, spi, level);
+	if (spi_id > min(kvm->arch.vgic.nr_irqs, 1020))
+		return -EINVAL;
+	return kvm_vgic_inject_irq(kvm, 0, spi_id, level);
+}
+
+/**
+ * Populates a kvm routing entry from a user routing entry
+ * @e: kvm internal formatted entry
+ * @ue: user api formatted entry
+ * 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)
+{
+	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;
 }
 
-/* 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)
 {
-	return 0;
-}
+	struct kvm_msi msi;
+
+	msi.address_lo = e->msi.address_lo;
+	msi.address_hi = e->msi.address_hi;
+	msi.data = e->msi.data;
+	if (e->type == KVM_IRQ_ROUTING_EXTENDED_MSI) {
+		msi.devid = e->devid;
+		msi.flags = KVM_MSI_VALID_DEVID;
+	}
 
-#ifdef CONFIG_HAVE_KVM_MSI
-int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi)
-{
 	if (kvm->arch.vgic.vm_ops.inject_msi)
-		return kvm->arch.vgic.vm_ops.inject_msi(kvm, msi);
+		return kvm->arch.vgic.vm_ops.inject_msi(kvm, &msi);
 	else
 		return -ENODEV;
 }
-#endif
diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
index e678f8a..f26cadd 100644
--- a/virt/kvm/irqchip.c
+++ b/virt/kvm/irqchip.c
@@ -29,7 +29,9 @@
 #include <linux/srcu.h>
 #include <linux/export.h>
 #include <trace/events/kvm.h>
+#if !defined(CONFIG_ARM) && !defined(CONFIG_ARM64)
 #include "irq.h"
+#endif
 
 struct kvm_irq_routing_table {
 	int chip[KVM_NR_IRQCHIPS][KVM_IRQCHIP_NUM_PINS];
-- 
1.9.1

[PATCH v2 4/7] KVM: arm/arm64: enable irqchip routing

[Eric Auger]

This patch adds compilation and link against irqchip.

On ARM, irqchip routing is not really useful since there is
a single irqchip. However main motivation behind using irqchip
code is to enable MSI routing code. With the support of in-kernel
GICv3 ITS emulation, it now seems to be a MUST HAVE requirement.

Functions previously implemented in vgic.c and substitute
to more complex irqchip implementation are removed:

- kvm_send_userspace_msi
- kvm_irq_map_chip_pin
- kvm_set_irq
- kvm_irq_map_gsi.

They implemented a kernel default identity GSI routing. This is now
replaced by user-side provided routing.

Routing standard hooks are now implemented in vgic:
- kvm_set_routing_entry
- kvm_set_irq
- kvm_set_msi

Both HAVE_KVM_IRQCHIP and HAVE_KVM_IRQ_ROUTING are defined.
KVM_CAP_IRQ_ROUTING is advertised and KVM_SET_GSI_ROUTING is allowed.

MSI routing is not yet allowed.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---
v1 -> v2:
- fix bug reported by Andre related to msi.flags and msi.devid setting
  in kvm_send_userspace_msi
- avoid injecting reserved IRQ numbers in vgic_irqfd_set_irq

RFC -> PATCH
- reword api.txt:
  x move MSI routing comments in a subsequent patch,
  x clearly state GSI routing does not apply to KVM_IRQ_LINE
---
 Documentation/virtual/kvm/api.txt | 12 ++++---
 arch/arm/include/asm/kvm_host.h   |  2 ++
 arch/arm/kvm/Kconfig              |  2 ++
 arch/arm/kvm/Makefile             |  2 +-
 arch/arm64/include/asm/kvm_host.h |  1 +
 arch/arm64/kvm/Kconfig            |  2 ++
 arch/arm64/kvm/Makefile           |  2 +-
 include/kvm/arm_vgic.h            |  9 -----
 virt/kvm/arm/vgic.c               | 71 +++++++++++++++++++++++++--------------
 virt/kvm/irqchip.c                |  2 ++
 10 files changed, 65 insertions(+), 40 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 3d920cd..9276dac 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1421,13 +1421,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;
@@ -2335,9 +2338,10 @@ Note that closing the resamplefd is not sufficient to disable the
 irqfd.  The KVM_IRQFD_FLAG_RESAMPLE is only necessary on assignment
 and need not be specified with KVM_IRQFD_FLAG_DEASSIGN.
 
-On ARM/ARM64, the gsi field in the kvm_irqfd struct specifies the Shared
-Peripheral Interrupt (SPI) index, such that the GIC interrupt ID is
-given by gsi + 32.
+On arm/arm64, gsi routing being supported, the following can happen:
+- in case no routing entry is associated to this gsi, injection fails
+- in case the gsi is associated to an irqchip routing entry,
+  irqchip.pin + 32 corresponds to the injected SPI ID.
 
 4.76 KVM_PPC_ALLOCATE_HTAB
 
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index e896d2c..9871441 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -42,6 +42,8 @@
 
 #define KVM_VCPU_MAX_FEATURES 2
 
+#define KVM_IRQCHIP_NUM_PINS 988 /* 1020 -32 is the number of SPI */
+
 #include <kvm/arm_vgic.h>
 
 u32 *kvm_vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num, u32 mode);
diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
index bfb915d..151e710 100644
--- a/arch/arm/kvm/Kconfig
+++ b/arch/arm/kvm/Kconfig
@@ -31,6 +31,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 c5eef02c..1a8f48a 100644
--- a/arch/arm/kvm/Makefile
+++ b/arch/arm/kvm/Makefile
@@ -15,7 +15,7 @@ AFLAGS_init.o := -Wa,-march=armv7-a$(plus_virt)
 AFLAGS_interrupts.o := -Wa,-march=armv7-a$(plus_virt)
 
 KVM := ../../../virt/kvm
-kvm-arm-y = $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o $(KVM)/vfio.o
+kvm-arm-y = $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o $(KVM)/vfio.o $(KVM)/irqchip.o
 
 obj-y += kvm-arm.o init.o interrupts.o
 obj-y += arm.o handle_exit.o guest.o mmu.o emulate.o reset.o
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 2709db2..f6201eb 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -44,6 +44,7 @@
 #include <kvm/arm_arch_timer.h>
 
 #define KVM_VCPU_MAX_FEATURES 3
+#define KVM_IRQCHIP_NUM_PINS 988 /* 1020 -32 is the number of SPI */
 
 int __attribute_const__ kvm_target_cpu(void);
 int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index ff9722f..1a9900d 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -32,6 +32,8 @@ config KVM
 	select HAVE_KVM_EVENTFD
 	select HAVE_KVM_IRQFD
 	select HAVE_KVM_MSI
+	select HAVE_KVM_IRQCHIP
+	select HAVE_KVM_IRQ_ROUTING
 	---help---
 	  Support hosting virtualized guest machines.
 
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 9803307..90a08457 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -11,7 +11,7 @@ ARM=../../../arch/arm/kvm
 
 obj-$(CONFIG_KVM_ARM_HOST) += kvm.o
 
-kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o $(KVM)/vfio.o
+kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o $(KVM)/vfio.o $(KVM)/irqchip.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(ARM)/arm.o $(ARM)/mmu.o $(ARM)/mmio.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(ARM)/psci.o $(ARM)/perf.o
 
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 8f1be6a..10dc596 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -366,13 +366,4 @@ static inline int vgic_v3_probe(struct device_node *vgic_node,
 }
 #endif
 
-#ifdef CONFIG_HAVE_KVM_MSI
-int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi);
-#else
-static inline int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi)
-{
-	return -ENODEV;
-}
-#endif
-
 #endif
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 3630971..6c6c25e 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -2215,44 +2215,65 @@ out_free_irq:
 	return ret;
 }
 
-int kvm_irq_map_gsi(struct kvm *kvm,
-		    struct kvm_kernel_irq_routing_entry *entries,
-		    int gsi)
+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;
-}
-
-int kvm_irq_map_chip_pin(struct kvm *kvm, unsigned irqchip, unsigned pin)
-{
-	return pin;
-}
-
-int kvm_set_irq(struct kvm *kvm, int irq_source_id,
-		u32 irq, int level, bool line_status)
-{
-	unsigned int spi = irq + VGIC_NR_PRIVATE_IRQS;
+	unsigned int spi_id = e->irqchip.pin + VGIC_NR_PRIVATE_IRQS;
 
-	trace_kvm_set_irq(irq, level, irq_source_id);
+	trace_kvm_set_irq(spi_id, level, irq_source_id);
 
 	BUG_ON(!vgic_initialized(kvm));
 
-	return kvm_vgic_inject_irq(kvm, 0, spi, level);
+	if (spi_id > min(kvm->arch.vgic.nr_irqs, 1020))
+		return -EINVAL;
+	return kvm_vgic_inject_irq(kvm, 0, spi_id, level);
+}
+
+/**
+ * Populates a kvm routing entry from a user routing entry
+ * @e: kvm internal formatted entry
+ * @ue: user api formatted entry
+ * 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)
+{
+	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;
 }
 
-/* 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)
 {
-	return 0;
-}
+	struct kvm_msi msi;
+
+	msi.address_lo = e->msi.address_lo;
+	msi.address_hi = e->msi.address_hi;
+	msi.data = e->msi.data;
+	if (e->type == KVM_IRQ_ROUTING_EXTENDED_MSI) {
+		msi.devid = e->devid;
+		msi.flags = KVM_MSI_VALID_DEVID;
+	}
 
-#ifdef CONFIG_HAVE_KVM_MSI
-int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi)
-{
 	if (kvm->arch.vgic.vm_ops.inject_msi)
-		return kvm->arch.vgic.vm_ops.inject_msi(kvm, msi);
+		return kvm->arch.vgic.vm_ops.inject_msi(kvm, &msi);
 	else
 		return -ENODEV;
 }
-#endif
diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
index e678f8a..f26cadd 100644
--- a/virt/kvm/irqchip.c
+++ b/virt/kvm/irqchip.c
@@ -29,7 +29,9 @@
 #include <linux/srcu.h>
 #include <linux/export.h>
 #include <trace/events/kvm.h>
+#if !defined(CONFIG_ARM) && !defined(CONFIG_ARM64)
 #include "irq.h"
+#endif
 
 struct kvm_irq_routing_table {
 	int chip[KVM_NR_IRQCHIPS][KVM_IRQCHIP_NUM_PINS];
-- 
1.9.1

[PATCH v2 5/7] KVM: arm/arm64: build a default routing table

[Eric Auger]

Implement a default routing table made of flat irqchip routing
entries (gsi = irqchip.pin) covering the VGIC SPI indexes.
This routing table is overwritten by the first user-space call
to KVM_SET_GSI_ROUTING ioctl.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

PATCH: creation
---
 virt/kvm/arm/vgic.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 6c6c25e..63be67e 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1781,6 +1781,8 @@ int vgic_init(struct kvm *kvm)
 	ret |= vgic_init_bitmap(&dist->irq_cfg, nr_cpus, nr_irqs);
 	ret |= vgic_init_bytemap(&dist->irq_priority, nr_cpus, nr_irqs);
 
+	ret |= kvm_setup_default_irq_routing(kvm);
+
 	if (ret)
 		goto out;
 
@@ -2258,6 +2260,25 @@ out:
 	return r;
 }
 
+int kvm_setup_default_irq_routing(struct kvm *kvm)
+{
+	struct kvm_irq_routing_entry *entries;
+	u32 nr = kvm->arch.vgic.nr_irqs - VGIC_NR_PRIVATE_IRQS;
+	int i, ret;
+
+	entries = kcalloc(nr, sizeof(struct kvm_kernel_irq_routing_entry),
+			  GFP_KERNEL);
+	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;
+}
+
 int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
 		struct kvm *kvm, int irq_source_id,
 		int level, bool line_status)
-- 
1.9.1

[PATCH v2 5/7] KVM: arm/arm64: build a default routing table

[Eric Auger]

Implement a default routing table made of flat irqchip routing
entries (gsi = irqchip.pin) covering the VGIC SPI indexes.
This routing table is overwritten by the first user-space call
to KVM_SET_GSI_ROUTING ioctl.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

PATCH: creation
---
 virt/kvm/arm/vgic.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 6c6c25e..63be67e 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1781,6 +1781,8 @@ int vgic_init(struct kvm *kvm)
 	ret |= vgic_init_bitmap(&dist->irq_cfg, nr_cpus, nr_irqs);
 	ret |= vgic_init_bytemap(&dist->irq_priority, nr_cpus, nr_irqs);
 
+	ret |= kvm_setup_default_irq_routing(kvm);
+
 	if (ret)
 		goto out;
 
@@ -2258,6 +2260,25 @@ out:
 	return r;
 }
 
+int kvm_setup_default_irq_routing(struct kvm *kvm)
+{
+	struct kvm_irq_routing_entry *entries;
+	u32 nr = kvm->arch.vgic.nr_irqs - VGIC_NR_PRIVATE_IRQS;
+	int i, ret;
+
+	entries = kcalloc(nr, sizeof(struct kvm_kernel_irq_routing_entry),
+			  GFP_KERNEL);
+	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;
+}
+
 int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
 		struct kvm *kvm, int irq_source_id,
 		int level, bool line_status)
-- 
1.9.1

[PATCH v2 6/7] KVM: arm/arm64: enable MSI routing

[Eric Auger]

Up to now, only irqchip routing entries could be set. This patch
adds the capability to insert MSI routing entries, with or without
device id. Although standard MSI entries can be set, their
injection still is not supported. For ARM64, let's also increase
KVM_MAX_IRQ_ROUTES to 4096: include SPI irqchip flat routes plus
MSI routes. In the future this might be extended.

The new MSI routing entry type also must be managed similarly to
legacy KVM_IRQ_ROUTING_MSI in eventfd irqfd_wakeup and irqfd_update.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

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 | 10 ++++++++++
 include/linux/kvm_host.h          |  2 ++
 virt/kvm/arm/vgic.c               | 13 +++++++++++++
 virt/kvm/eventfd.c                |  6 ++++--
 4 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 9276dac..686b121 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1431,6 +1431,11 @@ 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.
 
+On arm/arm64, MSI routing through in-kernel GICv3 ITS must use
+KVM_IRQ_ROUTING_EXTENDED_MSI routing type and device ID must be set
+in msi struct. Otherwise, KVM_IRQ_ROUTING_MSI must be used without
+populating the msi devid field.
+
 struct kvm_irq_routing {
 	__u32 nr;
 	__u32 flags;
@@ -2342,6 +2347,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 cac0fe4..ea1a810 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -976,6 +976,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.c b/virt/kvm/arm/vgic.c
index 63be67e..c5546d8 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -2252,6 +2252,19 @@ 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;
+		break;
+	case KVM_IRQ_ROUTING_EXTENDED_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->devid = ue->u.msi.devid;
+		break;
 	default:
 		goto out;
 	}
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 9ff4193..d76d05d 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -238,7 +238,8 @@ irqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key)
 			irq = irqfd->irq_entry;
 		} while (read_seqcount_retry(&irqfd->irq_entry_sc, seq));
 		/* An event has been signaled, inject an interrupt */
-		if (irq.type == KVM_IRQ_ROUTING_MSI)
+		if (irq.type == KVM_IRQ_ROUTING_MSI ||
+		    irq.type == KVM_IRQ_ROUTING_EXTENDED_MSI)
 			kvm_set_msi(&irq, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1,
 					false);
 		else
@@ -294,7 +295,8 @@ static void irqfd_update(struct kvm *kvm, struct _irqfd *irqfd)
 	e = entries;
 	for (i = 0; i < n_entries; ++i, ++e) {
 		/* Only fast-path MSI. */
-		if (e->type == KVM_IRQ_ROUTING_MSI)
+		if (e->type == KVM_IRQ_ROUTING_MSI ||
+		    e->type == KVM_IRQ_ROUTING_EXTENDED_MSI)
 			irqfd->irq_entry = *e;
 	}
 
-- 
1.9.1

[PATCH v2 6/7] KVM: arm/arm64: enable MSI routing

[Eric Auger]

Up to now, only irqchip routing entries could be set. This patch
adds the capability to insert MSI routing entries, with or without
device id. Although standard MSI entries can be set, their
injection still is not supported. For ARM64, let's also increase
KVM_MAX_IRQ_ROUTES to 4096: include SPI irqchip flat routes plus
MSI routes. In the future this might be extended.

The new MSI routing entry type also must be managed similarly to
legacy KVM_IRQ_ROUTING_MSI in eventfd irqfd_wakeup and irqfd_update.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

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 | 10 ++++++++++
 include/linux/kvm_host.h          |  2 ++
 virt/kvm/arm/vgic.c               | 13 +++++++++++++
 virt/kvm/eventfd.c                |  6 ++++--
 4 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 9276dac..686b121 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1431,6 +1431,11 @@ 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.
 
+On arm/arm64, MSI routing through in-kernel GICv3 ITS must use
+KVM_IRQ_ROUTING_EXTENDED_MSI routing type and device ID must be set
+in msi struct. Otherwise, KVM_IRQ_ROUTING_MSI must be used without
+populating the msi devid field.
+
 struct kvm_irq_routing {
 	__u32 nr;
 	__u32 flags;
@@ -2342,6 +2347,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 cac0fe4..ea1a810 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -976,6 +976,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.c b/virt/kvm/arm/vgic.c
index 63be67e..c5546d8 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -2252,6 +2252,19 @@ 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;
+		break;
+	case KVM_IRQ_ROUTING_EXTENDED_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->devid = ue->u.msi.devid;
+		break;
 	default:
 		goto out;
 	}
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 9ff4193..d76d05d 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -238,7 +238,8 @@ irqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key)
 			irq = irqfd->irq_entry;
 		} while (read_seqcount_retry(&irqfd->irq_entry_sc, seq));
 		/* An event has been signaled, inject an interrupt */
-		if (irq.type == KVM_IRQ_ROUTING_MSI)
+		if (irq.type == KVM_IRQ_ROUTING_MSI ||
+		    irq.type == KVM_IRQ_ROUTING_EXTENDED_MSI)
 			kvm_set_msi(&irq, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1,
 					false);
 		else
@@ -294,7 +295,8 @@ static void irqfd_update(struct kvm *kvm, struct _irqfd *irqfd)
 	e = entries;
 	for (i = 0; i < n_entries; ++i, ++e) {
 		/* Only fast-path MSI. */
-		if (e->type == KVM_IRQ_ROUTING_MSI)
+		if (e->type == KVM_IRQ_ROUTING_MSI ||
+		    e->type == KVM_IRQ_ROUTING_EXTENDED_MSI)
 			irqfd->irq_entry = *e;
 	}
 
-- 
1.9.1

[PATCH v2 7/7] KVM: arm: implement kvm_set_msi by gsi direct mapping

[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 for arm too.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

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-v2-emul.c | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
index 151e710..0f58baf 100644
--- a/arch/arm/kvm/Kconfig
+++ b/arch/arm/kvm/Kconfig
@@ -31,6 +31,7 @@ config KVM
 	select KVM_VFIO
 	select HAVE_KVM_EVENTFD
 	select HAVE_KVM_IRQFD
+	select HAVE_KVM_MSI
 	select HAVE_KVM_IRQCHIP
 	select HAVE_KVM_IRQ_ROUTING
 	depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
diff --git a/virt/kvm/arm/vgic-v2-emul.c b/virt/kvm/arm/vgic-v2-emul.c
index 1390797..43013cc 100644
--- a/virt/kvm/arm/vgic-v2-emul.c
+++ b/virt/kvm/arm/vgic-v2-emul.c
@@ -478,6 +478,17 @@ static bool vgic_v2_queue_sgi(struct kvm_vcpu *vcpu, int irq)
 }
 
 /**
+ * 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)
+{
+	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);
+}
+
+/**
  * kvm_vgic_map_resources - Configure global VGIC state before running any VCPUs
  * @kvm: pointer to the kvm struct
  *
@@ -566,6 +577,7 @@ void vgic_v2_init_emulation(struct kvm *kvm)
 	dist->vm_ops.add_sgi_source = vgic_v2_add_sgi_source;
 	dist->vm_ops.init_model = vgic_v2_init_model;
 	dist->vm_ops.map_resources = vgic_v2_map_resources;
+	dist->vm_ops.inject_msi = vgic_v2m_inject_msi;
 
 	kvm->arch.max_vcpus = VGIC_V2_MAX_CPUS;
 }
-- 
1.9.1

[PATCH v2 7/7] KVM: arm: implement kvm_set_msi by gsi direct mapping

[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 for arm too.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

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-v2-emul.c | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
index 151e710..0f58baf 100644
--- a/arch/arm/kvm/Kconfig
+++ b/arch/arm/kvm/Kconfig
@@ -31,6 +31,7 @@ config KVM
 	select KVM_VFIO
 	select HAVE_KVM_EVENTFD
 	select HAVE_KVM_IRQFD
+	select HAVE_KVM_MSI
 	select HAVE_KVM_IRQCHIP
 	select HAVE_KVM_IRQ_ROUTING
 	depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
diff --git a/virt/kvm/arm/vgic-v2-emul.c b/virt/kvm/arm/vgic-v2-emul.c
index 1390797..43013cc 100644
--- a/virt/kvm/arm/vgic-v2-emul.c
+++ b/virt/kvm/arm/vgic-v2-emul.c
@@ -478,6 +478,17 @@ static bool vgic_v2_queue_sgi(struct kvm_vcpu *vcpu, int irq)
 }
 
 /**
+ * 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)
+{
+	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);
+}
+
+/**
  * kvm_vgic_map_resources - Configure global VGIC state before running any VCPUs
  * @kvm: pointer to the kvm struct
  *
@@ -566,6 +577,7 @@ void vgic_v2_init_emulation(struct kvm *kvm)
 	dist->vm_ops.add_sgi_source = vgic_v2_add_sgi_source;
 	dist->vm_ops.init_model = vgic_v2_init_model;
 	dist->vm_ops.map_resources = vgic_v2_map_resources;
+	dist->vm_ops.inject_msi = vgic_v2m_inject_msi;
 
 	kvm->arch.max_vcpus = VGIC_V2_MAX_CPUS;
 }
-- 
1.9.1

RE: [PATCH v2 0/7] KVM: arm/arm64: gsi routing support

[Pavel Fedin]

 Hello!

> 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

 Andre, you never replied to my last comment to the previous series. Are you going to do the same
change in your MSI API? Otherwise:
1. KVM_IRQ_LINE - we have completely own convention. Well, this was already done before us, we
cannot fix it.
2. KVM_SIGNAL_MSI - we use VALID_DEVID flag plus devid
3. KVM_SET_GSI_ROUTING - we use KVM_IRQ_ROUTING_EXTENDED_MSI plus devid

 Don't (2) and (3) together still look bad? Since we agreed on not using flags, i would suggest to
have KVM_SIGNAL_EXTENDED_MSI counterpart, which also doesn't use flags.
 I know, we were already talking about it, so, if this gets ignored for the second time, i assume
the Architects decided that fancy APIs are cool, and i promise to stop this.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

[PATCH v2 0/7] KVM: arm/arm64: gsi routing support

[Pavel Fedin]

 Hello!

> 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

 Andre, you never replied to my last comment to the previous series. Are you going to do the same
change in your MSI API? Otherwise:
1. KVM_IRQ_LINE - we have completely own convention. Well, this was already done before us, we
cannot fix it.
2. KVM_SIGNAL_MSI - we use VALID_DEVID flag plus devid
3. KVM_SET_GSI_ROUTING - we use KVM_IRQ_ROUTING_EXTENDED_MSI plus devid

 Don't (2) and (3) together still look bad? Since we agreed on not using flags, i would suggest to
have KVM_SIGNAL_EXTENDED_MSI counterpart, which also doesn't use flags.
 I know, we were already talking about it, so, if this gets ignored for the second time, i assume
the Architects decided that fancy APIs are cool, and i promise to stop this.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [PATCH v2 0/7] KVM: arm/arm64: gsi routing support

[Andre Przywara]

Hi Pavel,

On 09/07/15 15:37, Pavel Fedin wrote:
>  Hello!
> 
>> 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
> 
>  Andre, you never replied to my last comment to the previous series.

Oh dear, my draft folder again :-( Sorry for that!

> Are you going to do the same
> change in your MSI API? Otherwise:
> 1. KVM_IRQ_LINE - we have completely own convention. Well, this was already done before us, we
> cannot fix it.
> 2. KVM_SIGNAL_MSI - we use VALID_DEVID flag plus devid

Yes, because there is already a flag value and no other way to specify
this, in contrast to ...

> 3. KVM_SET_GSI_ROUTING - we use KVM_IRQ_ROUTING_EXTENDED_MSI plus devid

Here we already have a type field with some users, so lets piggy-back on
this.

Both ioctl extensions are coupled with a per-VM capability to let
userland know that it needs to provide a device ID.

>  Don't (2) and (3) together still look bad? Since we agreed on not using flags, i would suggest to
> have KVM_SIGNAL_EXTENDED_MSI counterpart, which also doesn't use flags.

Using flags on its own (without an explicit capability) is what I
opposed against, not flags in general. After all, that's what they are
meant for, right? In case of KVM_SET_GSI_ROUTING it just seems awkward
to me to use a flag when a different type would do as well.

But after all, I don't have a strong opinion on that matter, so if
others prefer using a flag I am also fine with that.

Poka,
Andre.

>  I know, we were already talking about it, so, if this gets ignored for the second time, i assume
> the Architects decided that fancy APIs are cool, and i promise to stop this.
> 
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
> 

[PATCH v2 0/7] KVM: arm/arm64: gsi routing support

[Andre Przywara]

Hi Pavel,

On 09/07/15 15:37, Pavel Fedin wrote:
>  Hello!
> 
>> 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
> 
>  Andre, you never replied to my last comment to the previous series.

Oh dear, my draft folder again :-( Sorry for that!

> Are you going to do the same
> change in your MSI API? Otherwise:
> 1. KVM_IRQ_LINE - we have completely own convention. Well, this was already done before us, we
> cannot fix it.
> 2. KVM_SIGNAL_MSI - we use VALID_DEVID flag plus devid

Yes, because there is already a flag value and no other way to specify
this, in contrast to ...

> 3. KVM_SET_GSI_ROUTING - we use KVM_IRQ_ROUTING_EXTENDED_MSI plus devid

Here we already have a type field with some users, so lets piggy-back on
this.

Both ioctl extensions are coupled with a per-VM capability to let
userland know that it needs to provide a device ID.

>  Don't (2) and (3) together still look bad? Since we agreed on not using flags, i would suggest to
> have KVM_SIGNAL_EXTENDED_MSI counterpart, which also doesn't use flags.

Using flags on its own (without an explicit capability) is what I
opposed against, not flags in general. After all, that's what they are
meant for, right? In case of KVM_SET_GSI_ROUTING it just seems awkward
to me to use a flag when a different type would do as well.

But after all, I don't have a strong opinion on that matter, so if
others prefer using a flag I am also fine with that.

Poka,
Andre.

>  I know, we were already talking about it, so, if this gets ignored for the second time, i assume
> the Architects decided that fancy APIs are cool, and i promise to stop this.
> 
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
> 

RE: [PATCH v2 0/7] KVM: arm/arm64: gsi routing support

[Pavel Fedin]

 Hi!

> > 3. KVM_SET_GSI_ROUTING - we use KVM_IRQ_ROUTING_EXTENDED_MSI plus devid
> 
> Here we already have a type field with some users, so lets piggy-back on
> this.

 We already have 'flags' there too.

> Both ioctl extensions are coupled with a per-VM capability to let
> userland know that it needs to provide a device ID.

> Using flags on its own (without an explicit capability) is what I
> opposed against, not flags in general.

 Ok, and in your next respin you'll add the capability, correct? So that we will finally have all
pieces in place.

> In case of KVM_SET_GSI_ROUTING it just seems awkward
> to me to use a flag when a different type would do as well.

 Well, MSI vs "Extended MSI" are even not different types really. It's just MSI which has devid. And
we *ALREADY* have VALID_DEVID flag.

> But after all, I don't have a strong opinion on that matter, so if
> others prefer using a flag I am also fine with that.

 So, ok, to be short... My vote is for flag, because it's already there and it keeps up with the
style we already have. Eric, this is my final statement about it. It's up to you to accept or
ignore. In qemu code flag is a little bit nicer because it's just stored in a variable and helps to
avoid several if-else's (however small ones). Compare:
--- cut ---
    kroute.gsi = virq;
    kroute.type = KVM_IRQ_ROUTING_MSI;
    kroute.u.msi.address_lo = (uint32_t)msg.address;
    kroute.u.msi.address_hi = msg.address >> 32;
    kroute.u.msi.data = le32_to_cpu(msg.data);
    kroute.flags = kvm_msi_flags;
    if (kroute.flags & KVM_MSI_VALID_DEVID) {
        kroute.u.msi.devid = (pci_bus_num(dev->bus) << 8) | dev->devfn;
    }
--- cut ---
and:
--- cut ---
    kroute.gsi = virq;
    if (use_extended_msi) {
        kroute.u.msi.devid = (pci_bus_num(dev->bus) << 8) | dev->devfn;
        kroute.type = KVM_IRQ_ROUTING_EXTENDED_MSI;
    } else {
        kroute.type = KVM_IRQ_ROUTING_MSI;
    }
    kroute.u.msi.address_lo = (uint32_t)msg.address;
    kroute.u.msi.address_hi = msg.address >> 32;
    kroute.u.msi.data = le32_to_cpu(msg.data);
    kroute.flags = kvm_msi_flags;
--- cut ---

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

[PATCH v2 0/7] KVM: arm/arm64: gsi routing support

[Pavel Fedin]

 Hi!

> > 3. KVM_SET_GSI_ROUTING - we use KVM_IRQ_ROUTING_EXTENDED_MSI plus devid
> 
> Here we already have a type field with some users, so lets piggy-back on
> this.

 We already have 'flags' there too.

> Both ioctl extensions are coupled with a per-VM capability to let
> userland know that it needs to provide a device ID.

> Using flags on its own (without an explicit capability) is what I
> opposed against, not flags in general.

 Ok, and in your next respin you'll add the capability, correct? So that we will finally have all
pieces in place.

> In case of KVM_SET_GSI_ROUTING it just seems awkward
> to me to use a flag when a different type would do as well.

 Well, MSI vs "Extended MSI" are even not different types really. It's just MSI which has devid. And
we *ALREADY* have VALID_DEVID flag.

> But after all, I don't have a strong opinion on that matter, so if
> others prefer using a flag I am also fine with that.

 So, ok, to be short... My vote is for flag, because it's already there and it keeps up with the
style we already have. Eric, this is my final statement about it. It's up to you to accept or
ignore. In qemu code flag is a little bit nicer because it's just stored in a variable and helps to
avoid several if-else's (however small ones). Compare:
--- cut ---
    kroute.gsi = virq;
    kroute.type = KVM_IRQ_ROUTING_MSI;
    kroute.u.msi.address_lo = (uint32_t)msg.address;
    kroute.u.msi.address_hi = msg.address >> 32;
    kroute.u.msi.data = le32_to_cpu(msg.data);
    kroute.flags = kvm_msi_flags;
    if (kroute.flags & KVM_MSI_VALID_DEVID) {
        kroute.u.msi.devid = (pci_bus_num(dev->bus) << 8) | dev->devfn;
    }
--- cut ---
and:
--- cut ---
    kroute.gsi = virq;
    if (use_extended_msi) {
        kroute.u.msi.devid = (pci_bus_num(dev->bus) << 8) | dev->devfn;
        kroute.type = KVM_IRQ_ROUTING_EXTENDED_MSI;
    } else {
        kroute.type = KVM_IRQ_ROUTING_MSI;
    }
    kroute.u.msi.address_lo = (uint32_t)msg.address;
    kroute.u.msi.address_hi = msg.address >> 32;
    kroute.u.msi.data = le32_to_cpu(msg.data);
    kroute.flags = kvm_msi_flags;
--- cut ---

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [PATCH v2 0/7] KVM: arm/arm64: gsi routing support

[Eric Auger]

Hi Pavel, Andre,
On 07/09/2015 05:52 PM, Pavel Fedin wrote:
>  Hi!
> 
>>> 3. KVM_SET_GSI_ROUTING - we use KVM_IRQ_ROUTING_EXTENDED_MSI plus devid
>>
>> Here we already have a type field with some users, so lets piggy-back on
>> this.
> 
>  We already have 'flags' there too.
> 
>> Both ioctl extensions are coupled with a per-VM capability to let
>> userland know that it needs to provide a device ID.
> 
>> Using flags on its own (without an explicit capability) is what I
>> opposed against, not flags in general.
> 
>  Ok, and in your next respin you'll add the capability, correct? So that we will finally have all
> pieces in place.
> 
>> In case of KVM_SET_GSI_ROUTING it just seems awkward
>> to me to use a flag when a different type would do as well.
> 
>  Well, MSI vs "Extended MSI" are even not different types really. It's just MSI which has devid. And
> we *ALREADY* have VALID_DEVID flag.
> 
>> But after all, I don't have a strong opinion on that matter, so if
>> others prefer using a flag I am also fine with that.
> 
>  So, ok, to be short... My vote is for flag, because it's already there and it keeps up with the
> style we already have. Eric, this is my final statement about it. It's up to you to accept or
> ignore. In qemu code flag is a little bit nicer because it's just stored in a variable and helps to
> avoid several if-else's (however small ones). Compare:
Well personally I prefer the type thing and I don't see much difference
at userspace level anyway. But I am not this kind of hyperspace
architect guy. So, since there is no consensus here, I would say let's
wait for formal reviews of our maintainers and I will align.

The v2 update is not the outcome of a consensus so I made arbitrary
decisions to progress & fix bugs and I hope this eventually works with
ITS ;-)

Best Regards

Eric
> --- cut ---
>     kroute.gsi = virq;
>     kroute.type = KVM_IRQ_ROUTING_MSI;
>     kroute.u.msi.address_lo = (uint32_t)msg.address;
>     kroute.u.msi.address_hi = msg.address >> 32;
>     kroute.u.msi.data = le32_to_cpu(msg.data);
>     kroute.flags = kvm_msi_flags;
>     if (kroute.flags & KVM_MSI_VALID_DEVID) {
>         kroute.u.msi.devid = (pci_bus_num(dev->bus) << 8) | dev->devfn;
>     }
> --- cut ---
> and:
> --- cut ---
>     kroute.gsi = virq;
>     if (use_extended_msi) {
>         kroute.u.msi.devid = (pci_bus_num(dev->bus) << 8) | dev->devfn;
>         kroute.type = KVM_IRQ_ROUTING_EXTENDED_MSI;
>     } else {
>         kroute.type = KVM_IRQ_ROUTING_MSI;
>     }
>     kroute.u.msi.address_lo = (uint32_t)msg.address;
>     kroute.u.msi.address_hi = msg.address >> 32;
>     kroute.u.msi.data = le32_to_cpu(msg.data);
>     kroute.flags = kvm_msi_flags;
> --- cut ---
> 
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
> 
> 

[PATCH v2 0/7] KVM: arm/arm64: gsi routing support

[Eric Auger]

Hi Pavel, Andre,
On 07/09/2015 05:52 PM, Pavel Fedin wrote:
>  Hi!
> 
>>> 3. KVM_SET_GSI_ROUTING - we use KVM_IRQ_ROUTING_EXTENDED_MSI plus devid
>>
>> Here we already have a type field with some users, so lets piggy-back on
>> this.
> 
>  We already have 'flags' there too.
> 
>> Both ioctl extensions are coupled with a per-VM capability to let
>> userland know that it needs to provide a device ID.
> 
>> Using flags on its own (without an explicit capability) is what I
>> opposed against, not flags in general.
> 
>  Ok, and in your next respin you'll add the capability, correct? So that we will finally have all
> pieces in place.
> 
>> In case of KVM_SET_GSI_ROUTING it just seems awkward
>> to me to use a flag when a different type would do as well.
> 
>  Well, MSI vs "Extended MSI" are even not different types really. It's just MSI which has devid. And
> we *ALREADY* have VALID_DEVID flag.
> 
>> But after all, I don't have a strong opinion on that matter, so if
>> others prefer using a flag I am also fine with that.
> 
>  So, ok, to be short... My vote is for flag, because it's already there and it keeps up with the
> style we already have. Eric, this is my final statement about it. It's up to you to accept or
> ignore. In qemu code flag is a little bit nicer because it's just stored in a variable and helps to
> avoid several if-else's (however small ones). Compare:
Well personally I prefer the type thing and I don't see much difference
at userspace level anyway. But I am not this kind of hyperspace
architect guy. So, since there is no consensus here, I would say let's
wait for formal reviews of our maintainers and I will align.

The v2 update is not the outcome of a consensus so I made arbitrary
decisions to progress & fix bugs and I hope this eventually works with
ITS ;-)

Best Regards

Eric
> --- cut ---
>     kroute.gsi = virq;
>     kroute.type = KVM_IRQ_ROUTING_MSI;
>     kroute.u.msi.address_lo = (uint32_t)msg.address;
>     kroute.u.msi.address_hi = msg.address >> 32;
>     kroute.u.msi.data = le32_to_cpu(msg.data);
>     kroute.flags = kvm_msi_flags;
>     if (kroute.flags & KVM_MSI_VALID_DEVID) {
>         kroute.u.msi.devid = (pci_bus_num(dev->bus) << 8) | dev->devfn;
>     }
> --- cut ---
> and:
> --- cut ---
>     kroute.gsi = virq;
>     if (use_extended_msi) {
>         kroute.u.msi.devid = (pci_bus_num(dev->bus) << 8) | dev->devfn;
>         kroute.type = KVM_IRQ_ROUTING_EXTENDED_MSI;
>     } else {
>         kroute.type = KVM_IRQ_ROUTING_MSI;
>     }
>     kroute.u.msi.address_lo = (uint32_t)msg.address;
>     kroute.u.msi.address_hi = msg.address >> 32;
>     kroute.u.msi.data = le32_to_cpu(msg.data);
>     kroute.flags = kvm_msi_flags;
> --- cut ---
> 
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
> 
> 

RE: [PATCH v2 0/7] KVM: arm/arm64: gsi routing support

[Pavel Fedin]

> Well personally I prefer the type thing and I don't see much difference
> at userspace level anyway. But I am not this kind of hyperspace
> architect guy. So, since there is no consensus here, I would say let's
> wait for formal reviews of our maintainers and I will align.

 Hah, okay... Don't take it for too much. Anyway your word weighs much more than my one...

> hope this eventually works with
> ITS ;-)

 You know... There are actually problems with irqfd. However i'm not sure whether they are result of
some bug or of poorly made vGIC CPU interface software emulation. I don't have hardware with working
vGICv3 here. With GICv2m everything is fine, but code path is quite different, and LPI != SPI.
 This is further overcomplicated by the fact that as soon as i start up the profiler, the problem
goes away. Looks like increased number of context switches have impact.
 Actually, the problem is very poor vhost-net throughput, which jumps up when i start the profiler
and goes back down when i stop it. Just to note...
 I'm off since now for a small vacation, will be back on monday or tuesday. See you guys!

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

[PATCH v2 0/7] KVM: arm/arm64: gsi routing support

[Pavel Fedin]

> Well personally I prefer the type thing and I don't see much difference
> at userspace level anyway. But I am not this kind of hyperspace
> architect guy. So, since there is no consensus here, I would say let's
> wait for formal reviews of our maintainers and I will align.

 Hah, okay... Don't take it for too much. Anyway your word weighs much more than my one...

> hope this eventually works with
> ITS ;-)

 You know... There are actually problems with irqfd. However i'm not sure whether they are result of
some bug or of poorly made vGIC CPU interface software emulation. I don't have hardware with working
vGICv3 here. With GICv2m everything is fine, but code path is quite different, and LPI != SPI.
 This is further overcomplicated by the fact that as soon as i start up the profiler, the problem
goes away. Looks like increased number of context switches have impact.
 Actually, the problem is very poor vhost-net throughput, which jumps up when i start the profiler
and goes back down when i stop it. Just to note...
 I'm off since now for a small vacation, will be back on monday or tuesday. See you guys!

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [PATCH v2 1/7] KVM: api: introduce KVM_IRQ_ROUTING_EXTENDED_MSI

[Andre Przywara]

On 09/07/15 09:22, Eric Auger wrote:
> 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 a new routing entry type to
> indicate the devid is populated.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Just some forward looking statement: If this gets based on top of my ITS
emulation v2 series, the documentation should state that the new
KVM_CAP_MSI_DEVID capability tells userland about the need to provide
the device ID.

Cheers,
Andre.

> 
> ---
> 
> 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 | 10 +++++++++-
>  include/uapi/linux/kvm.h          |  6 +++++-
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index bea8de7..3d920cd 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -1453,6 +1453,7 @@ struct kvm_irq_routing_entry {
>  #define KVM_IRQ_ROUTING_IRQCHIP 1
>  #define KVM_IRQ_ROUTING_MSI 2
>  #define KVM_IRQ_ROUTING_S390_ADAPTER 3
> +#define KVM_IRQ_ROUTING_EXTENDED_MSI 4
>  
>  No flags are specified so far, the corresponding field must be set to zero.
>  
> @@ -1465,9 +1466,16 @@ struct kvm_irq_routing_msi {
>  	__u32 address_lo;
>  	__u32 address_hi;
>  	__u32 data;
> -	__u32 pad;
> +	union {
> +		__u32 pad;
> +		__u32 devid;
> +	};
>  };
>  
> +for KVM_IRQ_ROUTING_EXTENDED_MSI routing entry type, the kvm_irq_routing_msi
> +routing entry is used and devid is populated with the device ID that wrote
> +the MSI message. For PCI, this is usually a BFD identifier in the lower 16 bits.
> +
>  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 9a261e5..39ec164 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -829,7 +829,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 {
> @@ -844,6 +847,7 @@ struct kvm_irq_routing_s390_adapter {
>  #define KVM_IRQ_ROUTING_IRQCHIP 1
>  #define KVM_IRQ_ROUTING_MSI 2
>  #define KVM_IRQ_ROUTING_S390_ADAPTER 3
> +#define KVM_IRQ_ROUTING_EXTENDED_MSI 4
>  
>  struct kvm_irq_routing_entry {
>  	__u32 gsi;
> 

[PATCH v2 1/7] KVM: api: introduce KVM_IRQ_ROUTING_EXTENDED_MSI

[Andre Przywara]

On 09/07/15 09:22, Eric Auger wrote:
> 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 a new routing entry type to
> indicate the devid is populated.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Just some forward looking statement: If this gets based on top of my ITS
emulation v2 series, the documentation should state that the new
KVM_CAP_MSI_DEVID capability tells userland about the need to provide
the device ID.

Cheers,
Andre.

> 
> ---
> 
> 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 | 10 +++++++++-
>  include/uapi/linux/kvm.h          |  6 +++++-
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index bea8de7..3d920cd 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -1453,6 +1453,7 @@ struct kvm_irq_routing_entry {
>  #define KVM_IRQ_ROUTING_IRQCHIP 1
>  #define KVM_IRQ_ROUTING_MSI 2
>  #define KVM_IRQ_ROUTING_S390_ADAPTER 3
> +#define KVM_IRQ_ROUTING_EXTENDED_MSI 4
>  
>  No flags are specified so far, the corresponding field must be set to zero.
>  
> @@ -1465,9 +1466,16 @@ struct kvm_irq_routing_msi {
>  	__u32 address_lo;
>  	__u32 address_hi;
>  	__u32 data;
> -	__u32 pad;
> +	union {
> +		__u32 pad;
> +		__u32 devid;
> +	};
>  };
>  
> +for KVM_IRQ_ROUTING_EXTENDED_MSI routing entry type, the kvm_irq_routing_msi
> +routing entry is used and devid is populated with the device ID that wrote
> +the MSI message. For PCI, this is usually a BFD identifier in the lower 16 bits.
> +
>  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 9a261e5..39ec164 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -829,7 +829,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 {
> @@ -844,6 +847,7 @@ struct kvm_irq_routing_s390_adapter {
>  #define KVM_IRQ_ROUTING_IRQCHIP 1
>  #define KVM_IRQ_ROUTING_MSI 2
>  #define KVM_IRQ_ROUTING_S390_ADAPTER 3
> +#define KVM_IRQ_ROUTING_EXTENDED_MSI 4
>  
>  struct kvm_irq_routing_entry {
>  	__u32 gsi;
> 

Re: [PATCH v2 3/7] KVM: irqchip: convey devid to kvm_set_msi

[Andre Przywara]

On 09/07/15 09:22, Eric Auger wrote:
> on ARM, a devid field is populated in kvm_msi struct in case the
> flag is set to KVM_MSI_VALID_DEVID. Let's populate the corresponding
> kvm_kernel_irq_routing_entry devid field and set the msi type to
> KVM_IRQ_ROUTING_EXTENDED_MSI.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> ---
>  virt/kvm/irqchip.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
> index 21c1424..e678f8a 100644
> --- a/virt/kvm/irqchip.c
> +++ b/virt/kvm/irqchip.c
> @@ -72,9 +72,17 @@ 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))
>  		return -EINVAL;
>  
> +	if (msi->flags & KVM_MSI_VALID_DEVID) {
> +		route.devid = msi->devid;
> +		route.type = KVM_IRQ_ROUTING_EXTENDED_MSI;
> +	} else if (!msi->flags)
> +		return -EINVAL;

I think we get away without using the extended type on the kernel side.
Within the kernel we don't have an ABI that we have to stick to forever,
so we can simplify things by re-using the existing type (no need to
check for both MSI types later).
So we always set the device ID, the only code that looks at it later is
the ITS emulation anyway, any other code path simply ignores that.

To be honest I am not 100% sure that is actually better, but I had
hacked it in a way where I didn't need it.
Also have a look at the other comments in the following patches for
clarification.

Cheers,
Andre.


> +
> +	/* historically the route.type was not set */
> +
>  	route.msi.address_lo = msi->address_lo;
>  	route.msi.address_hi = msi->address_hi;
>  	route.msi.data = msi->data;
> 

[PATCH v2 3/7] KVM: irqchip: convey devid to kvm_set_msi

[Andre Przywara]

On 09/07/15 09:22, Eric Auger wrote:
> on ARM, a devid field is populated in kvm_msi struct in case the
> flag is set to KVM_MSI_VALID_DEVID. Let's populate the corresponding
> kvm_kernel_irq_routing_entry devid field and set the msi type to
> KVM_IRQ_ROUTING_EXTENDED_MSI.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> ---
>  virt/kvm/irqchip.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
> index 21c1424..e678f8a 100644
> --- a/virt/kvm/irqchip.c
> +++ b/virt/kvm/irqchip.c
> @@ -72,9 +72,17 @@ 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))
>  		return -EINVAL;
>  
> +	if (msi->flags & KVM_MSI_VALID_DEVID) {
> +		route.devid = msi->devid;
> +		route.type = KVM_IRQ_ROUTING_EXTENDED_MSI;
> +	} else if (!msi->flags)
> +		return -EINVAL;

I think we get away without using the extended type on the kernel side.
Within the kernel we don't have an ABI that we have to stick to forever,
so we can simplify things by re-using the existing type (no need to
check for both MSI types later).
So we always set the device ID, the only code that looks at it later is
the ITS emulation anyway, any other code path simply ignores that.

To be honest I am not 100% sure that is actually better, but I had
hacked it in a way where I didn't need it.
Also have a look at the other comments in the following patches for
clarification.

Cheers,
Andre.


> +
> +	/* historically the route.type was not set */
> +
>  	route.msi.address_lo = msi->address_lo;
>  	route.msi.address_hi = msi->address_hi;
>  	route.msi.data = msi->data;
> 

Re: [PATCH v2 4/7] KVM: arm/arm64: enable irqchip routing

[Andre Przywara]

Hi Eric,

On 09/07/15 09:22, Eric Auger wrote:
> This patch adds compilation and link against irqchip.
> 
> On ARM, irqchip routing is not really useful since there is
> a single irqchip. However main motivation behind using irqchip
> code is to enable MSI routing code. With the support of in-kernel
> GICv3 ITS emulation, it now seems to be a MUST HAVE requirement.
> 

....

> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index 3630971..6c6c25e 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -2215,44 +2215,65 @@ out_free_irq:
>  	return ret;
>  }
>  
> -int kvm_irq_map_gsi(struct kvm *kvm,
> -		    struct kvm_kernel_irq_routing_entry *entries,
> -		    int gsi)
> +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;
> -}
> -
> -int kvm_irq_map_chip_pin(struct kvm *kvm, unsigned irqchip, unsigned pin)
> -{
> -	return pin;
> -}
> -
> -int kvm_set_irq(struct kvm *kvm, int irq_source_id,
> -		u32 irq, int level, bool line_status)
> -{
> -	unsigned int spi = irq + VGIC_NR_PRIVATE_IRQS;
> +	unsigned int spi_id = e->irqchip.pin + VGIC_NR_PRIVATE_IRQS;
>  
> -	trace_kvm_set_irq(irq, level, irq_source_id);
> +	trace_kvm_set_irq(spi_id, level, irq_source_id);
>  
>  	BUG_ON(!vgic_initialized(kvm));
>  
> -	return kvm_vgic_inject_irq(kvm, 0, spi, level);
> +	if (spi_id > min(kvm->arch.vgic.nr_irqs, 1020))
> +		return -EINVAL;
> +	return kvm_vgic_inject_irq(kvm, 0, spi_id, level);
> +}
> +
> +/**
> + * Populates a kvm routing entry from a user routing entry
> + * @e: kvm internal formatted entry
> + * @ue: user api formatted entry
> + * 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)
> +{
> +	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;
>  }
>  
> -/* 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)
>  {
> -	return 0;
> -}
> +	struct kvm_msi msi;
> +
> +	msi.address_lo = e->msi.address_lo;
> +	msi.address_hi = e->msi.address_hi;
> +	msi.data = e->msi.data;
> +	if (e->type == KVM_IRQ_ROUTING_EXTENDED_MSI) {
> +		msi.devid = e->devid;
> +		msi.flags = KVM_MSI_VALID_DEVID;
> +	}

Can't we make the assignment unconditional?
The GICv2m MSI code does not care about the devid and the ITS code
requires it.
This simplifies quite something in the following patches.
(This refers to the idea of not using the extended type in the kernel).

>  
> -#ifdef CONFIG_HAVE_KVM_MSI
> -int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi)
> -{
>  	if (kvm->arch.vgic.vm_ops.inject_msi)
> -		return kvm->arch.vgic.vm_ops.inject_msi(kvm, msi);
> +		return kvm->arch.vgic.vm_ops.inject_msi(kvm, &msi);
>  	else
>  		return -ENODEV;
>  }
> -#endif
> diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
> index e678f8a..f26cadd 100644
> --- a/virt/kvm/irqchip.c
> +++ b/virt/kvm/irqchip.c
> @@ -29,7 +29,9 @@
>  #include <linux/srcu.h>
>  #include <linux/export.h>
>  #include <trace/events/kvm.h>
> +#if !defined(CONFIG_ARM) && !defined(CONFIG_ARM64)
>  #include "irq.h"
> +#endif

To what irq.h is that referring to? And why is ARM not allowed to
include that?

Cheers,
Andre.

>  
>  struct kvm_irq_routing_table {
>  	int chip[KVM_NR_IRQCHIPS][KVM_IRQCHIP_NUM_PINS];
> 

[PATCH v2 4/7] KVM: arm/arm64: enable irqchip routing

[Andre Przywara]

Hi Eric,

On 09/07/15 09:22, Eric Auger wrote:
> This patch adds compilation and link against irqchip.
> 
> On ARM, irqchip routing is not really useful since there is
> a single irqchip. However main motivation behind using irqchip
> code is to enable MSI routing code. With the support of in-kernel
> GICv3 ITS emulation, it now seems to be a MUST HAVE requirement.
> 

....

> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index 3630971..6c6c25e 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -2215,44 +2215,65 @@ out_free_irq:
>  	return ret;
>  }
>  
> -int kvm_irq_map_gsi(struct kvm *kvm,
> -		    struct kvm_kernel_irq_routing_entry *entries,
> -		    int gsi)
> +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;
> -}
> -
> -int kvm_irq_map_chip_pin(struct kvm *kvm, unsigned irqchip, unsigned pin)
> -{
> -	return pin;
> -}
> -
> -int kvm_set_irq(struct kvm *kvm, int irq_source_id,
> -		u32 irq, int level, bool line_status)
> -{
> -	unsigned int spi = irq + VGIC_NR_PRIVATE_IRQS;
> +	unsigned int spi_id = e->irqchip.pin + VGIC_NR_PRIVATE_IRQS;
>  
> -	trace_kvm_set_irq(irq, level, irq_source_id);
> +	trace_kvm_set_irq(spi_id, level, irq_source_id);
>  
>  	BUG_ON(!vgic_initialized(kvm));
>  
> -	return kvm_vgic_inject_irq(kvm, 0, spi, level);
> +	if (spi_id > min(kvm->arch.vgic.nr_irqs, 1020))
> +		return -EINVAL;
> +	return kvm_vgic_inject_irq(kvm, 0, spi_id, level);
> +}
> +
> +/**
> + * Populates a kvm routing entry from a user routing entry
> + * @e: kvm internal formatted entry
> + * @ue: user api formatted entry
> + * 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)
> +{
> +	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;
>  }
>  
> -/* 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)
>  {
> -	return 0;
> -}
> +	struct kvm_msi msi;
> +
> +	msi.address_lo = e->msi.address_lo;
> +	msi.address_hi = e->msi.address_hi;
> +	msi.data = e->msi.data;
> +	if (e->type == KVM_IRQ_ROUTING_EXTENDED_MSI) {
> +		msi.devid = e->devid;
> +		msi.flags = KVM_MSI_VALID_DEVID;
> +	}

Can't we make the assignment unconditional?
The GICv2m MSI code does not care about the devid and the ITS code
requires it.
This simplifies quite something in the following patches.
(This refers to the idea of not using the extended type in the kernel).

>  
> -#ifdef CONFIG_HAVE_KVM_MSI
> -int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi)
> -{
>  	if (kvm->arch.vgic.vm_ops.inject_msi)
> -		return kvm->arch.vgic.vm_ops.inject_msi(kvm, msi);
> +		return kvm->arch.vgic.vm_ops.inject_msi(kvm, &msi);
>  	else
>  		return -ENODEV;
>  }
> -#endif
> diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
> index e678f8a..f26cadd 100644
> --- a/virt/kvm/irqchip.c
> +++ b/virt/kvm/irqchip.c
> @@ -29,7 +29,9 @@
>  #include <linux/srcu.h>
>  #include <linux/export.h>
>  #include <trace/events/kvm.h>
> +#if !defined(CONFIG_ARM) && !defined(CONFIG_ARM64)
>  #include "irq.h"
> +#endif

To what irq.h is that referring to? And why is ARM not allowed to
include that?

Cheers,
Andre.

>  
>  struct kvm_irq_routing_table {
>  	int chip[KVM_NR_IRQCHIPS][KVM_IRQCHIP_NUM_PINS];
> 

Re: [PATCH v2 6/7] KVM: arm/arm64: enable MSI routing

[Andre Przywara]

On 09/07/15 09:22, Eric Auger wrote:
> Up to now, only irqchip routing entries could be set. This patch
> adds the capability to insert MSI routing entries, with or without
> device id. Although standard MSI entries can be set, their
> injection still is not supported. For ARM64, let's also increase
> KVM_MAX_IRQ_ROUTES to 4096: include SPI irqchip flat routes plus
> MSI routes. In the future this might be extended.
> 
> The new MSI routing entry type also must be managed similarly to
> legacy KVM_IRQ_ROUTING_MSI in eventfd irqfd_wakeup and irqfd_update.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> 
> ---
> 
> 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 | 10 ++++++++++
>  include/linux/kvm_host.h          |  2 ++
>  virt/kvm/arm/vgic.c               | 13 +++++++++++++
>  virt/kvm/eventfd.c                |  6 ++++--
>  4 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 9276dac..686b121 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -1431,6 +1431,11 @@ 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.
>  
> +On arm/arm64, MSI routing through in-kernel GICv3 ITS must use
> +KVM_IRQ_ROUTING_EXTENDED_MSI routing type and device ID must be set
> +in msi struct. Otherwise, KVM_IRQ_ROUTING_MSI must be used without
> +populating the msi devid field.
> +
>  struct kvm_irq_routing {
>  	__u32 nr;
>  	__u32 flags;
> @@ -2342,6 +2347,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 cac0fe4..ea1a810 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -976,6 +976,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.c b/virt/kvm/arm/vgic.c
> index 63be67e..c5546d8 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -2252,6 +2252,19 @@ int kvm_set_routing_entry(struct kvm_kernel_irq_routing_entry *e,
>  		    (e->irqchip.irqchip >= KVM_NR_IRQCHIPS))
>  			goto out;
>  		break;

what about:

+	case KVM_IRQ_ROUTING_EXTENDED_MSI:
+		e->devid = ue->u.msi.devid;
+		/* fall through */
> +	case KVM_IRQ_ROUTING_MSI:
...

Also if we avoid KVM_IRQ_ROUTING_EXTENDED_MSI in the kernel, we could:
+		e->type = KVM_IRQ_ROUTING_MSI;
(before the fall through) and then get rid of the rest of this patch at all.

Cheers,
Andre.

> +		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;
> +		break;



> +	case KVM_IRQ_ROUTING_EXTENDED_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->devid = ue->u.msi.devid;
> +		break;
>  	default:
>  		goto out;
>  	}
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index 9ff4193..d76d05d 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -238,7 +238,8 @@ irqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key)
>  			irq = irqfd->irq_entry;
>  		} while (read_seqcount_retry(&irqfd->irq_entry_sc, seq));
>  		/* An event has been signaled, inject an interrupt */
> -		if (irq.type == KVM_IRQ_ROUTING_MSI)
> +		if (irq.type == KVM_IRQ_ROUTING_MSI ||
> +		    irq.type == KVM_IRQ_ROUTING_EXTENDED_MSI)
>  			kvm_set_msi(&irq, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1,
>  					false);
>  		else
> @@ -294,7 +295,8 @@ static void irqfd_update(struct kvm *kvm, struct _irqfd *irqfd)
>  	e = entries;
>  	for (i = 0; i < n_entries; ++i, ++e) {
>  		/* Only fast-path MSI. */
> -		if (e->type == KVM_IRQ_ROUTING_MSI)
> +		if (e->type == KVM_IRQ_ROUTING_MSI ||
> +		    e->type == KVM_IRQ_ROUTING_EXTENDED_MSI)
>  			irqfd->irq_entry = *e;
>  	}
>  
> 

[PATCH v2 6/7] KVM: arm/arm64: enable MSI routing

[Andre Przywara]

On 09/07/15 09:22, Eric Auger wrote:
> Up to now, only irqchip routing entries could be set. This patch
> adds the capability to insert MSI routing entries, with or without
> device id. Although standard MSI entries can be set, their
> injection still is not supported. For ARM64, let's also increase
> KVM_MAX_IRQ_ROUTES to 4096: include SPI irqchip flat routes plus
> MSI routes. In the future this might be extended.
> 
> The new MSI routing entry type also must be managed similarly to
> legacy KVM_IRQ_ROUTING_MSI in eventfd irqfd_wakeup and irqfd_update.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> 
> ---
> 
> 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 | 10 ++++++++++
>  include/linux/kvm_host.h          |  2 ++
>  virt/kvm/arm/vgic.c               | 13 +++++++++++++
>  virt/kvm/eventfd.c                |  6 ++++--
>  4 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 9276dac..686b121 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -1431,6 +1431,11 @@ 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.
>  
> +On arm/arm64, MSI routing through in-kernel GICv3 ITS must use
> +KVM_IRQ_ROUTING_EXTENDED_MSI routing type and device ID must be set
> +in msi struct. Otherwise, KVM_IRQ_ROUTING_MSI must be used without
> +populating the msi devid field.
> +
>  struct kvm_irq_routing {
>  	__u32 nr;
>  	__u32 flags;
> @@ -2342,6 +2347,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 cac0fe4..ea1a810 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -976,6 +976,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.c b/virt/kvm/arm/vgic.c
> index 63be67e..c5546d8 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -2252,6 +2252,19 @@ int kvm_set_routing_entry(struct kvm_kernel_irq_routing_entry *e,
>  		    (e->irqchip.irqchip >= KVM_NR_IRQCHIPS))
>  			goto out;
>  		break;

what about:

+	case KVM_IRQ_ROUTING_EXTENDED_MSI:
+		e->devid = ue->u.msi.devid;
+		/* fall through */
> +	case KVM_IRQ_ROUTING_MSI:
...

Also if we avoid KVM_IRQ_ROUTING_EXTENDED_MSI in the kernel, we could:
+		e->type = KVM_IRQ_ROUTING_MSI;
(before the fall through) and then get rid of the rest of this patch at all.

Cheers,
Andre.

> +		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;
> +		break;



> +	case KVM_IRQ_ROUTING_EXTENDED_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->devid = ue->u.msi.devid;
> +		break;
>  	default:
>  		goto out;
>  	}
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index 9ff4193..d76d05d 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -238,7 +238,8 @@ irqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key)
>  			irq = irqfd->irq_entry;
>  		} while (read_seqcount_retry(&irqfd->irq_entry_sc, seq));
>  		/* An event has been signaled, inject an interrupt */
> -		if (irq.type == KVM_IRQ_ROUTING_MSI)
> +		if (irq.type == KVM_IRQ_ROUTING_MSI ||
> +		    irq.type == KVM_IRQ_ROUTING_EXTENDED_MSI)
>  			kvm_set_msi(&irq, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1,
>  					false);
>  		else
> @@ -294,7 +295,8 @@ static void irqfd_update(struct kvm *kvm, struct _irqfd *irqfd)
>  	e = entries;
>  	for (i = 0; i < n_entries; ++i, ++e) {
>  		/* Only fast-path MSI. */
> -		if (e->type == KVM_IRQ_ROUTING_MSI)
> +		if (e->type == KVM_IRQ_ROUTING_MSI ||
> +		    e->type == KVM_IRQ_ROUTING_EXTENDED_MSI)
>  			irqfd->irq_entry = *e;
>  	}
>  
> 

Re: [PATCH v2 7/7] KVM: arm: implement kvm_set_msi by gsi direct mapping

[Andre Przywara]

On 09/07/15 09:22, Eric Auger wrote:
> 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 for arm too.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> 
> ---
> 
> 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-v2-emul.c | 12 ++++++++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
> index 151e710..0f58baf 100644
> --- a/arch/arm/kvm/Kconfig
> +++ b/arch/arm/kvm/Kconfig
> @@ -31,6 +31,7 @@ config KVM
>  	select KVM_VFIO
>  	select HAVE_KVM_EVENTFD
>  	select HAVE_KVM_IRQFD
> +	select HAVE_KVM_MSI

I wonder if this requires some more code to only advertise
KVM_CAP_SIGNAL_MSI if userland actually sets up a GICv2M?
Otherwise userland could get the idea of being able to inectj MSIs
without the guest actually being prepared for that (because the GICv2M
driver did not initialize).

Cheers,
Andre.

>  	select HAVE_KVM_IRQCHIP
>  	select HAVE_KVM_IRQ_ROUTING
>  	depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
> diff --git a/virt/kvm/arm/vgic-v2-emul.c b/virt/kvm/arm/vgic-v2-emul.c
> index 1390797..43013cc 100644
> --- a/virt/kvm/arm/vgic-v2-emul.c
> +++ b/virt/kvm/arm/vgic-v2-emul.c
> @@ -478,6 +478,17 @@ static bool vgic_v2_queue_sgi(struct kvm_vcpu *vcpu, int irq)
>  }
>  
>  /**
> + * 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)
> +{
> +	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);
> +}
> +
> +/**
>   * kvm_vgic_map_resources - Configure global VGIC state before running any VCPUs
>   * @kvm: pointer to the kvm struct
>   *
> @@ -566,6 +577,7 @@ void vgic_v2_init_emulation(struct kvm *kvm)
>  	dist->vm_ops.add_sgi_source = vgic_v2_add_sgi_source;
>  	dist->vm_ops.init_model = vgic_v2_init_model;
>  	dist->vm_ops.map_resources = vgic_v2_map_resources;
> +	dist->vm_ops.inject_msi = vgic_v2m_inject_msi;
>  
>  	kvm->arch.max_vcpus = VGIC_V2_MAX_CPUS;
>  }
> 

[PATCH v2 7/7] KVM: arm: implement kvm_set_msi by gsi direct mapping

[Andre Przywara]

On 09/07/15 09:22, Eric Auger wrote:
> 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 for arm too.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> 
> ---
> 
> 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-v2-emul.c | 12 ++++++++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
> index 151e710..0f58baf 100644
> --- a/arch/arm/kvm/Kconfig
> +++ b/arch/arm/kvm/Kconfig
> @@ -31,6 +31,7 @@ config KVM
>  	select KVM_VFIO
>  	select HAVE_KVM_EVENTFD
>  	select HAVE_KVM_IRQFD
> +	select HAVE_KVM_MSI

I wonder if this requires some more code to only advertise
KVM_CAP_SIGNAL_MSI if userland actually sets up a GICv2M?
Otherwise userland could get the idea of being able to inectj MSIs
without the guest actually being prepared for that (because the GICv2M
driver did not initialize).

Cheers,
Andre.

>  	select HAVE_KVM_IRQCHIP
>  	select HAVE_KVM_IRQ_ROUTING
>  	depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
> diff --git a/virt/kvm/arm/vgic-v2-emul.c b/virt/kvm/arm/vgic-v2-emul.c
> index 1390797..43013cc 100644
> --- a/virt/kvm/arm/vgic-v2-emul.c
> +++ b/virt/kvm/arm/vgic-v2-emul.c
> @@ -478,6 +478,17 @@ static bool vgic_v2_queue_sgi(struct kvm_vcpu *vcpu, int irq)
>  }
>  
>  /**
> + * 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)
> +{
> +	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);
> +}
> +
> +/**
>   * kvm_vgic_map_resources - Configure global VGIC state before running any VCPUs
>   * @kvm: pointer to the kvm struct
>   *
> @@ -566,6 +577,7 @@ void vgic_v2_init_emulation(struct kvm *kvm)
>  	dist->vm_ops.add_sgi_source = vgic_v2_add_sgi_source;
>  	dist->vm_ops.init_model = vgic_v2_init_model;
>  	dist->vm_ops.map_resources = vgic_v2_map_resources;
> +	dist->vm_ops.inject_msi = vgic_v2m_inject_msi;
>  
>  	kvm->arch.max_vcpus = VGIC_V2_MAX_CPUS;
>  }
> 

Re: [PATCH v2 1/7] KVM: api: introduce KVM_IRQ_ROUTING_EXTENDED_MSI

[Eric Auger]

Hi Andre,
On 07/11/2015 12:42 AM, Andre Przywara wrote:
> On 09/07/15 09:22, Eric Auger wrote:
>> 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 a new routing entry type to
>> indicate the devid is populated.
>>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> 
> Just some forward looking statement: If this gets based on top of my ITS
> emulation v2 series,
yes this is my intent
 the documentation should state that the new
> KVM_CAP_MSI_DEVID capability tells userland about the need to provide
> the device ID.
I will add this note

Thanks!
Eric
> 
> Cheers,
> Andre.
> 
>>
>> ---
>>
>> 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 | 10 +++++++++-
>>  include/uapi/linux/kvm.h          |  6 +++++-
>>  2 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index bea8de7..3d920cd 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -1453,6 +1453,7 @@ struct kvm_irq_routing_entry {
>>  #define KVM_IRQ_ROUTING_IRQCHIP 1
>>  #define KVM_IRQ_ROUTING_MSI 2
>>  #define KVM_IRQ_ROUTING_S390_ADAPTER 3
>> +#define KVM_IRQ_ROUTING_EXTENDED_MSI 4
>>  
>>  No flags are specified so far, the corresponding field must be set to zero.
>>  
>> @@ -1465,9 +1466,16 @@ struct kvm_irq_routing_msi {
>>  	__u32 address_lo;
>>  	__u32 address_hi;
>>  	__u32 data;
>> -	__u32 pad;
>> +	union {
>> +		__u32 pad;
>> +		__u32 devid;
>> +	};
>>  };
>>  
>> +for KVM_IRQ_ROUTING_EXTENDED_MSI routing entry type, the kvm_irq_routing_msi
>> +routing entry is used and devid is populated with the device ID that wrote
>> +the MSI message. For PCI, this is usually a BFD identifier in the lower 16 bits.
>> +
>>  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 9a261e5..39ec164 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -829,7 +829,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 {
>> @@ -844,6 +847,7 @@ struct kvm_irq_routing_s390_adapter {
>>  #define KVM_IRQ_ROUTING_IRQCHIP 1
>>  #define KVM_IRQ_ROUTING_MSI 2
>>  #define KVM_IRQ_ROUTING_S390_ADAPTER 3
>> +#define KVM_IRQ_ROUTING_EXTENDED_MSI 4
>>  
>>  struct kvm_irq_routing_entry {
>>  	__u32 gsi;
>>
> 

[PATCH v2 1/7] KVM: api: introduce KVM_IRQ_ROUTING_EXTENDED_MSI

[Eric Auger]

Hi Andre,
On 07/11/2015 12:42 AM, Andre Przywara wrote:
> On 09/07/15 09:22, Eric Auger wrote:
>> 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 a new routing entry type to
>> indicate the devid is populated.
>>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> 
> Just some forward looking statement: If this gets based on top of my ITS
> emulation v2 series,
yes this is my intent
 the documentation should state that the new
> KVM_CAP_MSI_DEVID capability tells userland about the need to provide
> the device ID.
I will add this note

Thanks!
Eric
> 
> Cheers,
> Andre.
> 
>>
>> ---
>>
>> 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 | 10 +++++++++-
>>  include/uapi/linux/kvm.h          |  6 +++++-
>>  2 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index bea8de7..3d920cd 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -1453,6 +1453,7 @@ struct kvm_irq_routing_entry {
>>  #define KVM_IRQ_ROUTING_IRQCHIP 1
>>  #define KVM_IRQ_ROUTING_MSI 2
>>  #define KVM_IRQ_ROUTING_S390_ADAPTER 3
>> +#define KVM_IRQ_ROUTING_EXTENDED_MSI 4
>>  
>>  No flags are specified so far, the corresponding field must be set to zero.
>>  
>> @@ -1465,9 +1466,16 @@ struct kvm_irq_routing_msi {
>>  	__u32 address_lo;
>>  	__u32 address_hi;
>>  	__u32 data;
>> -	__u32 pad;
>> +	union {
>> +		__u32 pad;
>> +		__u32 devid;
>> +	};
>>  };
>>  
>> +for KVM_IRQ_ROUTING_EXTENDED_MSI routing entry type, the kvm_irq_routing_msi
>> +routing entry is used and devid is populated with the device ID that wrote
>> +the MSI message. For PCI, this is usually a BFD identifier in the lower 16 bits.
>> +
>>  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 9a261e5..39ec164 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -829,7 +829,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 {
>> @@ -844,6 +847,7 @@ struct kvm_irq_routing_s390_adapter {
>>  #define KVM_IRQ_ROUTING_IRQCHIP 1
>>  #define KVM_IRQ_ROUTING_MSI 2
>>  #define KVM_IRQ_ROUTING_S390_ADAPTER 3
>> +#define KVM_IRQ_ROUTING_EXTENDED_MSI 4
>>  
>>  struct kvm_irq_routing_entry {
>>  	__u32 gsi;
>>
> 

Re: [PATCH v2 4/7] KVM: arm/arm64: enable irqchip routing

[Eric Auger]

Hi Andre,
On 07/11/2015 01:15 AM, Andre Przywara wrote:
> Hi Eric,
> 
> On 09/07/15 09:22, Eric Auger wrote:
>> This patch adds compilation and link against irqchip.
>>
>> On ARM, irqchip routing is not really useful since there is
>> a single irqchip. However main motivation behind using irqchip
>> code is to enable MSI routing code. With the support of in-kernel
>> GICv3 ITS emulation, it now seems to be a MUST HAVE requirement.
>>
> 
> ....
> 
>> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
>> index 3630971..6c6c25e 100644
>> --- a/virt/kvm/arm/vgic.c
>> +++ b/virt/kvm/arm/vgic.c
>> @@ -2215,44 +2215,65 @@ out_free_irq:
>>  	return ret;
>>  }
>>  
>> -int kvm_irq_map_gsi(struct kvm *kvm,
>> -		    struct kvm_kernel_irq_routing_entry *entries,
>> -		    int gsi)
>> +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;
>> -}
>> -
>> -int kvm_irq_map_chip_pin(struct kvm *kvm, unsigned irqchip, unsigned pin)
>> -{
>> -	return pin;
>> -}
>> -
>> -int kvm_set_irq(struct kvm *kvm, int irq_source_id,
>> -		u32 irq, int level, bool line_status)
>> -{
>> -	unsigned int spi = irq + VGIC_NR_PRIVATE_IRQS;
>> +	unsigned int spi_id = e->irqchip.pin + VGIC_NR_PRIVATE_IRQS;
>>  
>> -	trace_kvm_set_irq(irq, level, irq_source_id);
>> +	trace_kvm_set_irq(spi_id, level, irq_source_id);
>>  
>>  	BUG_ON(!vgic_initialized(kvm));
>>  
>> -	return kvm_vgic_inject_irq(kvm, 0, spi, level);
>> +	if (spi_id > min(kvm->arch.vgic.nr_irqs, 1020))
>> +		return -EINVAL;
>> +	return kvm_vgic_inject_irq(kvm, 0, spi_id, level);
>> +}
>> +
>> +/**
>> + * Populates a kvm routing entry from a user routing entry
>> + * @e: kvm internal formatted entry
>> + * @ue: user api formatted entry
>> + * 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)
>> +{
>> +	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;
>>  }
>>  
>> -/* 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)
>>  {
>> -	return 0;
>> -}
>> +	struct kvm_msi msi;
>> +
>> +	msi.address_lo = e->msi.address_lo;
>> +	msi.address_hi = e->msi.address_hi;
>> +	msi.data = e->msi.data;
>> +	if (e->type == KVM_IRQ_ROUTING_EXTENDED_MSI) {
>> +		msi.devid = e->devid;
>> +		msi.flags = KVM_MSI_VALID_DEVID;
>> +	}
> 
> Can't we make the assignment unconditional?
> The GICv2m MSI code does not care about the devid and the ITS code
> requires it.
> This simplifies quite something in the following patches.
> (This refers to the idea of not using the extended type in the kernel).

How are we going to make sure the userspace provided a valid devid then?
- we have this info at user struct level: kvm_irq_routing_msi
- we wouldn't propagate the info at kernel struct level:
kvm_kernel_irq_routing_entry
- the only place where we could check the devid availability against the
need is at kvm_set_routing_entry I think (routing adaptation on ARM).

What is going to happen if devid == 0 since unset?

> 
>>  
>> -#ifdef CONFIG_HAVE_KVM_MSI
>> -int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi)
>> -{
>>  	if (kvm->arch.vgic.vm_ops.inject_msi)
>> -		return kvm->arch.vgic.vm_ops.inject_msi(kvm, msi);
>> +		return kvm->arch.vgic.vm_ops.inject_msi(kvm, &msi);
>>  	else
>>  		return -ENODEV;
>>  }
>> -#endif
>> diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
>> index e678f8a..f26cadd 100644
>> --- a/virt/kvm/irqchip.c
>> +++ b/virt/kvm/irqchip.c
>> @@ -29,7 +29,9 @@
>>  #include <linux/srcu.h>
>>  #include <linux/export.h>
>>  #include <trace/events/kvm.h>
>> +#if !defined(CONFIG_ARM) && !defined(CONFIG_ARM64)
>>  #include "irq.h"
>> +#endif
> 
> To what irq.h is that referring to? And why is ARM not allowed to
> include that?
this refers to arch/x86/kvm/irq.h, arch/powerpc/kvm/irq.h, ...

This typically declares things we have in include/kvm/arm_vgic.h
like irqchip_in_kernel

So currently I don't see things we should put in that header.

Cheers

Eric




> Cheers,
> Andre.
> 
>>  
>>  struct kvm_irq_routing_table {
>>  	int chip[KVM_NR_IRQCHIPS][KVM_IRQCHIP_NUM_PINS];
>>

[PATCH v2 4/7] KVM: arm/arm64: enable irqchip routing

[Eric Auger]

Hi Andre,
On 07/11/2015 01:15 AM, Andre Przywara wrote:
> Hi Eric,
> 
> On 09/07/15 09:22, Eric Auger wrote:
>> This patch adds compilation and link against irqchip.
>>
>> On ARM, irqchip routing is not really useful since there is
>> a single irqchip. However main motivation behind using irqchip
>> code is to enable MSI routing code. With the support of in-kernel
>> GICv3 ITS emulation, it now seems to be a MUST HAVE requirement.
>>
> 
> ....
> 
>> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
>> index 3630971..6c6c25e 100644
>> --- a/virt/kvm/arm/vgic.c
>> +++ b/virt/kvm/arm/vgic.c
>> @@ -2215,44 +2215,65 @@ out_free_irq:
>>  	return ret;
>>  }
>>  
>> -int kvm_irq_map_gsi(struct kvm *kvm,
>> -		    struct kvm_kernel_irq_routing_entry *entries,
>> -		    int gsi)
>> +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;
>> -}
>> -
>> -int kvm_irq_map_chip_pin(struct kvm *kvm, unsigned irqchip, unsigned pin)
>> -{
>> -	return pin;
>> -}
>> -
>> -int kvm_set_irq(struct kvm *kvm, int irq_source_id,
>> -		u32 irq, int level, bool line_status)
>> -{
>> -	unsigned int spi = irq + VGIC_NR_PRIVATE_IRQS;
>> +	unsigned int spi_id = e->irqchip.pin + VGIC_NR_PRIVATE_IRQS;
>>  
>> -	trace_kvm_set_irq(irq, level, irq_source_id);
>> +	trace_kvm_set_irq(spi_id, level, irq_source_id);
>>  
>>  	BUG_ON(!vgic_initialized(kvm));
>>  
>> -	return kvm_vgic_inject_irq(kvm, 0, spi, level);
>> +	if (spi_id > min(kvm->arch.vgic.nr_irqs, 1020))
>> +		return -EINVAL;
>> +	return kvm_vgic_inject_irq(kvm, 0, spi_id, level);
>> +}
>> +
>> +/**
>> + * Populates a kvm routing entry from a user routing entry
>> + * @e: kvm internal formatted entry
>> + * @ue: user api formatted entry
>> + * 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)
>> +{
>> +	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;
>>  }
>>  
>> -/* 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)
>>  {
>> -	return 0;
>> -}
>> +	struct kvm_msi msi;
>> +
>> +	msi.address_lo = e->msi.address_lo;
>> +	msi.address_hi = e->msi.address_hi;
>> +	msi.data = e->msi.data;
>> +	if (e->type == KVM_IRQ_ROUTING_EXTENDED_MSI) {
>> +		msi.devid = e->devid;
>> +		msi.flags = KVM_MSI_VALID_DEVID;
>> +	}
> 
> Can't we make the assignment unconditional?
> The GICv2m MSI code does not care about the devid and the ITS code
> requires it.
> This simplifies quite something in the following patches.
> (This refers to the idea of not using the extended type in the kernel).

How are we going to make sure the userspace provided a valid devid then?
- we have this info at user struct level: kvm_irq_routing_msi
- we wouldn't propagate the info at kernel struct level:
kvm_kernel_irq_routing_entry
- the only place where we could check the devid availability against the
need is at kvm_set_routing_entry I think (routing adaptation on ARM).

What is going to happen if devid == 0 since unset?

> 
>>  
>> -#ifdef CONFIG_HAVE_KVM_MSI
>> -int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi)
>> -{
>>  	if (kvm->arch.vgic.vm_ops.inject_msi)
>> -		return kvm->arch.vgic.vm_ops.inject_msi(kvm, msi);
>> +		return kvm->arch.vgic.vm_ops.inject_msi(kvm, &msi);
>>  	else
>>  		return -ENODEV;
>>  }
>> -#endif
>> diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
>> index e678f8a..f26cadd 100644
>> --- a/virt/kvm/irqchip.c
>> +++ b/virt/kvm/irqchip.c
>> @@ -29,7 +29,9 @@
>>  #include <linux/srcu.h>
>>  #include <linux/export.h>
>>  #include <trace/events/kvm.h>
>> +#if !defined(CONFIG_ARM) && !defined(CONFIG_ARM64)
>>  #include "irq.h"
>> +#endif
> 
> To what irq.h is that referring to? And why is ARM not allowed to
> include that?
this refers to arch/x86/kvm/irq.h, arch/powerpc/kvm/irq.h, ...

This typically declares things we have in include/kvm/arm_vgic.h
like irqchip_in_kernel

So currently I don't see things we should put in that header.

Cheers

Eric




> Cheers,
> Andre.
> 
>>  
>>  struct kvm_irq_routing_table {
>>  	int chip[KVM_NR_IRQCHIPS][KVM_IRQCHIP_NUM_PINS];
>>

RE: [PATCH v2 4/7] KVM: arm/arm64: enable irqchip routing

[Pavel Fedin]

 Hello!

> >> +	struct kvm_msi msi;
> >> +
> >> +	msi.address_lo = e->msi.address_lo;
> >> +	msi.address_hi = e->msi.address_hi;
> >> +	msi.data = e->msi.data;
> >> +	if (e->type == KVM_IRQ_ROUTING_EXTENDED_MSI) {
> >> +		msi.devid = e->devid;
> >> +		msi.flags = KVM_MSI_VALID_DEVID;
> >> +	}
> >
> > Can't we make the assignment unconditional?
> > The GICv2m MSI code does not care about the devid and the ITS code
> > requires it.
> > This simplifies quite something in the following patches.
> > (This refers to the idea of not using the extended type in the kernel).
> 
> How are we going to make sure the userspace provided a valid devid then?
> - we have this info at user struct level: kvm_irq_routing_msi
> - we wouldn't propagate the info at kernel struct level:
> kvm_kernel_irq_routing_entry
> - the only place where we could check the devid availability against the
> need is at kvm_set_routing_entry I think (routing adaptation on ARM).
> 
> What is going to happen if devid == 0 since unset?

> > +	if (msi->flags & KVM_MSI_VALID_DEVID) {
> > +		route.devid = msi->devid;
> > +		route.type = KVM_IRQ_ROUTING_EXTENDED_MSI;
> > +	} else if (!msi->flags)
> > +		return -EINVAL;
> 
> I think we get away without using the extended type on the kernel side.
> Within the kernel we don't have an ABI that we have to stick to forever,
> so we can simplify things by re-using the existing type (no need to
> check for both MSI types later).
> So we always set the device ID, the only code that looks at it later is
> the ITS emulation anyway, any other code path simply ignores that.

 Sorry for delayed reply, i'm a bit busy so cannot check all the emails in time...
 This is one more reason for using KVM_MSI_VALID_DEVID flag with KVM_IRQ_ROUTING_MSI. In this case
you don't have to bother about those conditions and just copy devid + flags pair between route and
MSI structures.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

[PATCH v2 4/7] KVM: arm/arm64: enable irqchip routing

[Pavel Fedin]

 Hello!

> >> +	struct kvm_msi msi;
> >> +
> >> +	msi.address_lo = e->msi.address_lo;
> >> +	msi.address_hi = e->msi.address_hi;
> >> +	msi.data = e->msi.data;
> >> +	if (e->type == KVM_IRQ_ROUTING_EXTENDED_MSI) {
> >> +		msi.devid = e->devid;
> >> +		msi.flags = KVM_MSI_VALID_DEVID;
> >> +	}
> >
> > Can't we make the assignment unconditional?
> > The GICv2m MSI code does not care about the devid and the ITS code
> > requires it.
> > This simplifies quite something in the following patches.
> > (This refers to the idea of not using the extended type in the kernel).
> 
> How are we going to make sure the userspace provided a valid devid then?
> - we have this info at user struct level: kvm_irq_routing_msi
> - we wouldn't propagate the info at kernel struct level:
> kvm_kernel_irq_routing_entry
> - the only place where we could check the devid availability against the
> need is at kvm_set_routing_entry I think (routing adaptation on ARM).
> 
> What is going to happen if devid == 0 since unset?

> > +	if (msi->flags & KVM_MSI_VALID_DEVID) {
> > +		route.devid = msi->devid;
> > +		route.type = KVM_IRQ_ROUTING_EXTENDED_MSI;
> > +	} else if (!msi->flags)
> > +		return -EINVAL;
> 
> I think we get away without using the extended type on the kernel side.
> Within the kernel we don't have an ABI that we have to stick to forever,
> so we can simplify things by re-using the existing type (no need to
> check for both MSI types later).
> So we always set the device ID, the only code that looks at it later is
> the ITS emulation anyway, any other code path simply ignores that.

 Sorry for delayed reply, i'm a bit busy so cannot check all the emails in time...
 This is one more reason for using KVM_MSI_VALID_DEVID flag with KVM_IRQ_ROUTING_MSI. In this case
you don't have to bother about those conditions and just copy devid + flags pair between route and
MSI structures.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

RE: [PATCH v2 3/7] KVM: irqchip: convey devid to kvm_set_msi

[Pavel Fedin]

> -----Original Message-----
> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On Behalf Of Eric Auger
> Sent: Thursday, July 09, 2015 11:22 AM
> To: eric.auger@st.com; eric.auger@linaro.org; linux-arm-kernel@lists.infradead.org;
> marc.zyngier@arm.com; christoffer.dall@linaro.org; kvmarm@lists.cs.columbia.edu;
> kvm@vger.kernel.org
> Cc: patches@linaro.org; andre.przywara@arm.com; p.fedin@samsung.com; pbonzini@redhat.com
> Subject: [PATCH v2 3/7] KVM: irqchip: convey devid to kvm_set_msi
> 
> on ARM, a devid field is populated in kvm_msi struct in case the
> flag is set to KVM_MSI_VALID_DEVID. Let's populate the corresponding
> kvm_kernel_irq_routing_entry devid field and set the msi type to
> KVM_IRQ_ROUTING_EXTENDED_MSI.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> ---
>  virt/kvm/irqchip.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
> index 21c1424..e678f8a 100644
> --- a/virt/kvm/irqchip.c
> +++ b/virt/kvm/irqchip.c
> @@ -72,9 +72,17 @@ 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))
>  		return -EINVAL;
> 
> +	if (msi->flags & KVM_MSI_VALID_DEVID) {
> +		route.devid = msi->devid;
> +		route.type = KVM_IRQ_ROUTING_EXTENDED_MSI;
> +	} else if (!msi->flags)
> +		return -EINVAL;

 Did you mean "if (msg->flags)" here (inverted condition)?

> +
> +	/* historically the route.type was not set */
> +
>  	route.msi.address_lo = msi->address_lo;
>  	route.msi.address_hi = msi->address_hi;
>  	route.msi.data = msi->data;
> --
> 1.9.1

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

[PATCH v2 3/7] KVM: irqchip: convey devid to kvm_set_msi

[Pavel Fedin]

> -----Original Message-----
> From: kvm-owner at vger.kernel.org [mailto:kvm-owner at vger.kernel.org] On Behalf Of Eric Auger
> Sent: Thursday, July 09, 2015 11:22 AM
> To: eric.auger at st.com; eric.auger at linaro.org; linux-arm-kernel at lists.infradead.org;
> marc.zyngier at arm.com; christoffer.dall at linaro.org; kvmarm at lists.cs.columbia.edu;
> kvm at vger.kernel.org
> Cc: patches at linaro.org; andre.przywara at arm.com; p.fedin at samsung.com; pbonzini at redhat.com
> Subject: [PATCH v2 3/7] KVM: irqchip: convey devid to kvm_set_msi
> 
> on ARM, a devid field is populated in kvm_msi struct in case the
> flag is set to KVM_MSI_VALID_DEVID. Let's populate the corresponding
> kvm_kernel_irq_routing_entry devid field and set the msi type to
> KVM_IRQ_ROUTING_EXTENDED_MSI.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> ---
>  virt/kvm/irqchip.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
> index 21c1424..e678f8a 100644
> --- a/virt/kvm/irqchip.c
> +++ b/virt/kvm/irqchip.c
> @@ -72,9 +72,17 @@ 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))
>  		return -EINVAL;
> 
> +	if (msi->flags & KVM_MSI_VALID_DEVID) {
> +		route.devid = msi->devid;
> +		route.type = KVM_IRQ_ROUTING_EXTENDED_MSI;
> +	} else if (!msi->flags)
> +		return -EINVAL;

 Did you mean "if (msg->flags)" here (inverted condition)?

> +
> +	/* historically the route.type was not set */
> +
>  	route.msi.address_lo = msi->address_lo;
>  	route.msi.address_hi = msi->address_hi;
>  	route.msi.data = msi->data;
> --
> 1.9.1

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [PATCH v2 3/7] KVM: irqchip: convey devid to kvm_set_msi

[Paolo Bonzini]

On 17/07/2015 09:27, Pavel Fedin wrote:
>> > +	} else if (!msi->flags)
>> > +		return -EINVAL;
>  Did you mean "if (msg->flags)" here (inverted condition)?

Indeed, and also you need { } around the return for the QEMU coding
standard.

Paolo

>> > +
>> > +	/* historically the route.type was not set */
>> > +

[PATCH v2 3/7] KVM: irqchip: convey devid to kvm_set_msi

[Paolo Bonzini]

On 17/07/2015 09:27, Pavel Fedin wrote:
>> > +	} else if (!msi->flags)
>> > +		return -EINVAL;
>  Did you mean "if (msg->flags)" here (inverted condition)?

Indeed, and also you need { } around the return for the QEMU coding
standard.

Paolo

>> > +
>> > +	/* historically the route.type was not set */
>> > +

RE: [PATCH v2 3/7] KVM: irqchip: convey devid to kvm_set_msi

[Pavel Fedin]

 Hello!

> >> > +	} else if (!msi->flags)
> >> > +		return -EINVAL;
> >  Did you mean "if (msg->flags)" here (inverted condition)?
> 
> Indeed, and also you need { } around the return for the QEMU coding
> standard.

 It's kernel and not qemu :)

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

[PATCH v2 3/7] KVM: irqchip: convey devid to kvm_set_msi

[Pavel Fedin]

 Hello!

> >> > +	} else if (!msi->flags)
> >> > +		return -EINVAL;
> >  Did you mean "if (msg->flags)" here (inverted condition)?
> 
> Indeed, and also you need { } around the return for the QEMU coding
> standard.

 It's kernel and not qemu :)

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [PATCH v2 3/7] KVM: irqchip: convey devid to kvm_set_msi

[Eric Auger]

On 07/17/2015 12:21 PM, Pavel Fedin wrote:
>  Hello!
> 
>>>>> +	} else if (!msi->flags)
>>>>> +		return -EINVAL;
>>>  Did you mean "if (msg->flags)" here (inverted condition)?
Yes thanks for spotting this.

Thanks

Eric
>>
>> Indeed, and also you need { } around the return for the QEMU coding
>> standard.
> 
>  It's kernel and not qemu :)
> 
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
> 

[PATCH v2 3/7] KVM: irqchip: convey devid to kvm_set_msi

[Eric Auger]

On 07/17/2015 12:21 PM, Pavel Fedin wrote:
>  Hello!
> 
>>>>> +	} else if (!msi->flags)
>>>>> +		return -EINVAL;
>>>  Did you mean "if (msg->flags)" here (inverted condition)?
Yes thanks for spotting this.

Thanks

Eric
>>
>> Indeed, and also you need { } around the return for the QEMU coding
>> standard.
> 
>  It's kernel and not qemu :)
> 
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
> 

Re: [PATCH v2 7/7] KVM: arm: implement kvm_set_msi by gsi direct mapping

[Eric Auger]

Hi Andre,
On 07/11/2015 01:17 AM, Andre Przywara wrote:
> On 09/07/15 09:22, Eric Auger wrote:
>> 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 for arm too.
>>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>
>> ---
>>
>> 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-v2-emul.c | 12 ++++++++++++
>>  2 files changed, 13 insertions(+)
>>
>> diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
>> index 151e710..0f58baf 100644
>> --- a/arch/arm/kvm/Kconfig
>> +++ b/arch/arm/kvm/Kconfig
>> @@ -31,6 +31,7 @@ config KVM
>>  	select KVM_VFIO
>>  	select HAVE_KVM_EVENTFD
>>  	select HAVE_KVM_IRQFD
>> +	select HAVE_KVM_MSI
> 
> I wonder if this requires some more code to only advertise
> KVM_CAP_SIGNAL_MSI if userland actually sets up a GICv2M?
> Otherwise userland could get the idea of being able to inectj MSIs
> without the guest actually being prepared for that (because the GICv2M
> driver did not initialize).
Not sure I get what you mean here. By directly transforming the user
provided MSI msg into an SPI ID, do we really have to care about GICv2M?

Best Regards

Eric
> 
> Cheers,
> Andre.
> 
>>  	select HAVE_KVM_IRQCHIP
>>  	select HAVE_KVM_IRQ_ROUTING
>>  	depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
>> diff --git a/virt/kvm/arm/vgic-v2-emul.c b/virt/kvm/arm/vgic-v2-emul.c
>> index 1390797..43013cc 100644
>> --- a/virt/kvm/arm/vgic-v2-emul.c
>> +++ b/virt/kvm/arm/vgic-v2-emul.c
>> @@ -478,6 +478,17 @@ static bool vgic_v2_queue_sgi(struct kvm_vcpu *vcpu, int irq)
>>  }
>>  
>>  /**
>> + * 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)
>> +{
>> +	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);
>> +}
>> +
>> +/**
>>   * kvm_vgic_map_resources - Configure global VGIC state before running any VCPUs
>>   * @kvm: pointer to the kvm struct
>>   *
>> @@ -566,6 +577,7 @@ void vgic_v2_init_emulation(struct kvm *kvm)
>>  	dist->vm_ops.add_sgi_source = vgic_v2_add_sgi_source;
>>  	dist->vm_ops.init_model = vgic_v2_init_model;
>>  	dist->vm_ops.map_resources = vgic_v2_map_resources;
>> +	dist->vm_ops.inject_msi = vgic_v2m_inject_msi;
>>  
>>  	kvm->arch.max_vcpus = VGIC_V2_MAX_CPUS;
>>  }
>>
> 

[PATCH v2 7/7] KVM: arm: implement kvm_set_msi by gsi direct mapping

[Eric Auger]

Hi Andre,
On 07/11/2015 01:17 AM, Andre Przywara wrote:
> On 09/07/15 09:22, Eric Auger wrote:
>> 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 for arm too.
>>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>
>> ---
>>
>> 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-v2-emul.c | 12 ++++++++++++
>>  2 files changed, 13 insertions(+)
>>
>> diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
>> index 151e710..0f58baf 100644
>> --- a/arch/arm/kvm/Kconfig
>> +++ b/arch/arm/kvm/Kconfig
>> @@ -31,6 +31,7 @@ config KVM
>>  	select KVM_VFIO
>>  	select HAVE_KVM_EVENTFD
>>  	select HAVE_KVM_IRQFD
>> +	select HAVE_KVM_MSI
> 
> I wonder if this requires some more code to only advertise
> KVM_CAP_SIGNAL_MSI if userland actually sets up a GICv2M?
> Otherwise userland could get the idea of being able to inectj MSIs
> without the guest actually being prepared for that (because the GICv2M
> driver did not initialize).
Not sure I get what you mean here. By directly transforming the user
provided MSI msg into an SPI ID, do we really have to care about GICv2M?

Best Regards

Eric
> 
> Cheers,
> Andre.
> 
>>  	select HAVE_KVM_IRQCHIP
>>  	select HAVE_KVM_IRQ_ROUTING
>>  	depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
>> diff --git a/virt/kvm/arm/vgic-v2-emul.c b/virt/kvm/arm/vgic-v2-emul.c
>> index 1390797..43013cc 100644
>> --- a/virt/kvm/arm/vgic-v2-emul.c
>> +++ b/virt/kvm/arm/vgic-v2-emul.c
>> @@ -478,6 +478,17 @@ static bool vgic_v2_queue_sgi(struct kvm_vcpu *vcpu, int irq)
>>  }
>>  
>>  /**
>> + * 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)
>> +{
>> +	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);
>> +}
>> +
>> +/**
>>   * kvm_vgic_map_resources - Configure global VGIC state before running any VCPUs
>>   * @kvm: pointer to the kvm struct
>>   *
>> @@ -566,6 +577,7 @@ void vgic_v2_init_emulation(struct kvm *kvm)
>>  	dist->vm_ops.add_sgi_source = vgic_v2_add_sgi_source;
>>  	dist->vm_ops.init_model = vgic_v2_init_model;
>>  	dist->vm_ops.map_resources = vgic_v2_map_resources;
>> +	dist->vm_ops.inject_msi = vgic_v2m_inject_msi;
>>  
>>  	kvm->arch.max_vcpus = VGIC_V2_MAX_CPUS;
>>  }
>>
> 

Re: [PATCH v2 7/7] KVM: arm: implement kvm_set_msi by gsi direct mapping

[Andre Przywara]

On 31/07/15 13:59, Eric Auger wrote:
> Hi Andre,
> On 07/11/2015 01:17 AM, Andre Przywara wrote:
>> On 09/07/15 09:22, Eric Auger wrote:
>>> 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 for arm too.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>>
>>> ---
>>>
>>> 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-v2-emul.c | 12 ++++++++++++
>>>  2 files changed, 13 insertions(+)
>>>
>>> diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
>>> index 151e710..0f58baf 100644
>>> --- a/arch/arm/kvm/Kconfig
>>> +++ b/arch/arm/kvm/Kconfig
>>> @@ -31,6 +31,7 @@ config KVM
>>>  	select KVM_VFIO
>>>  	select HAVE_KVM_EVENTFD
>>>  	select HAVE_KVM_IRQFD
>>> +	select HAVE_KVM_MSI
>>
>> I wonder if this requires some more code to only advertise
>> KVM_CAP_SIGNAL_MSI if userland actually sets up a GICv2M?
>> Otherwise userland could get the idea of being able to inectj MSIs
>> without the guest actually being prepared for that (because the GICv2M
>> driver did not initialize).
> Not sure I get what you mean here. By directly transforming the user
> provided MSI msg into an SPI ID, do we really have to care about GICv2M?

By user provided message you mean from user space? So this is an
emulated device, which the guest programs with an MSI payload and a
doorbell address at least? So how would a guest know these things
without having a MSI capable interrupt controller?

Or are we talking about different things here?

Cheers,
Andre.

> 
> Best Regards
> 
> Eric
>>
>> Cheers,
>> Andre.
>>
>>>  	select HAVE_KVM_IRQCHIP
>>>  	select HAVE_KVM_IRQ_ROUTING
>>>  	depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
>>> diff --git a/virt/kvm/arm/vgic-v2-emul.c b/virt/kvm/arm/vgic-v2-emul.c
>>> index 1390797..43013cc 100644
>>> --- a/virt/kvm/arm/vgic-v2-emul.c
>>> +++ b/virt/kvm/arm/vgic-v2-emul.c
>>> @@ -478,6 +478,17 @@ static bool vgic_v2_queue_sgi(struct kvm_vcpu *vcpu, int irq)
>>>  }
>>>  
>>>  /**
>>> + * 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)
>>> +{
>>> +	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);
>>> +}
>>> +
>>> +/**
>>>   * kvm_vgic_map_resources - Configure global VGIC state before running any VCPUs
>>>   * @kvm: pointer to the kvm struct
>>>   *
>>> @@ -566,6 +577,7 @@ void vgic_v2_init_emulation(struct kvm *kvm)
>>>  	dist->vm_ops.add_sgi_source = vgic_v2_add_sgi_source;
>>>  	dist->vm_ops.init_model = vgic_v2_init_model;
>>>  	dist->vm_ops.map_resources = vgic_v2_map_resources;
>>> +	dist->vm_ops.inject_msi = vgic_v2m_inject_msi;
>>>  
>>>  	kvm->arch.max_vcpus = VGIC_V2_MAX_CPUS;
>>>  }
>>>
>>
> 

[PATCH v2 7/7] KVM: arm: implement kvm_set_msi by gsi direct mapping

[Andre Przywara]

On 31/07/15 13:59, Eric Auger wrote:
> Hi Andre,
> On 07/11/2015 01:17 AM, Andre Przywara wrote:
>> On 09/07/15 09:22, Eric Auger wrote:
>>> 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 for arm too.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>>
>>> ---
>>>
>>> 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-v2-emul.c | 12 ++++++++++++
>>>  2 files changed, 13 insertions(+)
>>>
>>> diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
>>> index 151e710..0f58baf 100644
>>> --- a/arch/arm/kvm/Kconfig
>>> +++ b/arch/arm/kvm/Kconfig
>>> @@ -31,6 +31,7 @@ config KVM
>>>  	select KVM_VFIO
>>>  	select HAVE_KVM_EVENTFD
>>>  	select HAVE_KVM_IRQFD
>>> +	select HAVE_KVM_MSI
>>
>> I wonder if this requires some more code to only advertise
>> KVM_CAP_SIGNAL_MSI if userland actually sets up a GICv2M?
>> Otherwise userland could get the idea of being able to inectj MSIs
>> without the guest actually being prepared for that (because the GICv2M
>> driver did not initialize).
> Not sure I get what you mean here. By directly transforming the user
> provided MSI msg into an SPI ID, do we really have to care about GICv2M?

By user provided message you mean from user space? So this is an
emulated device, which the guest programs with an MSI payload and a
doorbell address at least? So how would a guest know these things
without having a MSI capable interrupt controller?

Or are we talking about different things here?

Cheers,
Andre.

> 
> Best Regards
> 
> Eric
>>
>> Cheers,
>> Andre.
>>
>>>  	select HAVE_KVM_IRQCHIP
>>>  	select HAVE_KVM_IRQ_ROUTING
>>>  	depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
>>> diff --git a/virt/kvm/arm/vgic-v2-emul.c b/virt/kvm/arm/vgic-v2-emul.c
>>> index 1390797..43013cc 100644
>>> --- a/virt/kvm/arm/vgic-v2-emul.c
>>> +++ b/virt/kvm/arm/vgic-v2-emul.c
>>> @@ -478,6 +478,17 @@ static bool vgic_v2_queue_sgi(struct kvm_vcpu *vcpu, int irq)
>>>  }
>>>  
>>>  /**
>>> + * 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)
>>> +{
>>> +	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);
>>> +}
>>> +
>>> +/**
>>>   * kvm_vgic_map_resources - Configure global VGIC state before running any VCPUs
>>>   * @kvm: pointer to the kvm struct
>>>   *
>>> @@ -566,6 +577,7 @@ void vgic_v2_init_emulation(struct kvm *kvm)
>>>  	dist->vm_ops.add_sgi_source = vgic_v2_add_sgi_source;
>>>  	dist->vm_ops.init_model = vgic_v2_init_model;
>>>  	dist->vm_ops.map_resources = vgic_v2_map_resources;
>>> +	dist->vm_ops.inject_msi = vgic_v2m_inject_msi;
>>>  
>>>  	kvm->arch.max_vcpus = VGIC_V2_MAX_CPUS;
>>>  }
>>>
>>
> 

Re: [PATCH v2 7/7] KVM: arm: implement kvm_set_msi by gsi direct mapping

[Eric Auger]

On 08/02/2015 10:23 PM, Andre Przywara wrote:
> On 31/07/15 13:59, Eric Auger wrote:
>> Hi Andre,
>> On 07/11/2015 01:17 AM, Andre Przywara wrote:
>>> On 09/07/15 09:22, Eric Auger wrote:
>>>> 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 for arm too.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>>>
>>>> ---
>>>>
>>>> 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-v2-emul.c | 12 ++++++++++++
>>>>  2 files changed, 13 insertions(+)
>>>>
>>>> diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
>>>> index 151e710..0f58baf 100644
>>>> --- a/arch/arm/kvm/Kconfig
>>>> +++ b/arch/arm/kvm/Kconfig
>>>> @@ -31,6 +31,7 @@ config KVM
>>>>  	select KVM_VFIO
>>>>  	select HAVE_KVM_EVENTFD
>>>>  	select HAVE_KVM_IRQFD
>>>> +	select HAVE_KVM_MSI
>>>
>>> I wonder if this requires some more code to only advertise
>>> KVM_CAP_SIGNAL_MSI if userland actually sets up a GICv2M?
>>> Otherwise userland could get the idea of being able to inectj MSIs
>>> without the guest actually being prepared for that (because the GICv2M
>>> driver did not initialize).
>> Not sure I get what you mean here. By directly transforming the user
>> provided MSI msg into an SPI ID, do we really have to care about GICv2M?
> 
> By user provided message you mean from user space? So this is an
> emulated device, which the guest programs with an MSI payload and a
> doorbell address at least? So how would a guest know these things
> without having a MSI capable interrupt controller?
> 
> Or are we talking about different things here?

I meant GICv2m is emulated on user-side today so I don't see how the
kernel could check anything. For the MSI injection, in that case, we
only use irqfd standard IRQ injection.

Cheers

Eric
> 
> Cheers,
> Andre.
> 
>>
>> Best Regards
>>
>> Eric
>>>
>>> Cheers,
>>> Andre.
>>>
>>>>  	select HAVE_KVM_IRQCHIP
>>>>  	select HAVE_KVM_IRQ_ROUTING
>>>>  	depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
>>>> diff --git a/virt/kvm/arm/vgic-v2-emul.c b/virt/kvm/arm/vgic-v2-emul.c
>>>> index 1390797..43013cc 100644
>>>> --- a/virt/kvm/arm/vgic-v2-emul.c
>>>> +++ b/virt/kvm/arm/vgic-v2-emul.c
>>>> @@ -478,6 +478,17 @@ static bool vgic_v2_queue_sgi(struct kvm_vcpu *vcpu, int irq)
>>>>  }
>>>>  
>>>>  /**
>>>> + * 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)
>>>> +{
>>>> +	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);
>>>> +}
>>>> +
>>>> +/**
>>>>   * kvm_vgic_map_resources - Configure global VGIC state before running any VCPUs
>>>>   * @kvm: pointer to the kvm struct
>>>>   *
>>>> @@ -566,6 +577,7 @@ void vgic_v2_init_emulation(struct kvm *kvm)
>>>>  	dist->vm_ops.add_sgi_source = vgic_v2_add_sgi_source;
>>>>  	dist->vm_ops.init_model = vgic_v2_init_model;
>>>>  	dist->vm_ops.map_resources = vgic_v2_map_resources;
>>>> +	dist->vm_ops.inject_msi = vgic_v2m_inject_msi;
>>>>  
>>>>  	kvm->arch.max_vcpus = VGIC_V2_MAX_CPUS;
>>>>  }
>>>>
>>>
>>
> 

[PATCH v2 7/7] KVM: arm: implement kvm_set_msi by gsi direct mapping

[Eric Auger]

On 08/02/2015 10:23 PM, Andre Przywara wrote:
> On 31/07/15 13:59, Eric Auger wrote:
>> Hi Andre,
>> On 07/11/2015 01:17 AM, Andre Przywara wrote:
>>> On 09/07/15 09:22, Eric Auger wrote:
>>>> 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 for arm too.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>>>
>>>> ---
>>>>
>>>> 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-v2-emul.c | 12 ++++++++++++
>>>>  2 files changed, 13 insertions(+)
>>>>
>>>> diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
>>>> index 151e710..0f58baf 100644
>>>> --- a/arch/arm/kvm/Kconfig
>>>> +++ b/arch/arm/kvm/Kconfig
>>>> @@ -31,6 +31,7 @@ config KVM
>>>>  	select KVM_VFIO
>>>>  	select HAVE_KVM_EVENTFD
>>>>  	select HAVE_KVM_IRQFD
>>>> +	select HAVE_KVM_MSI
>>>
>>> I wonder if this requires some more code to only advertise
>>> KVM_CAP_SIGNAL_MSI if userland actually sets up a GICv2M?
>>> Otherwise userland could get the idea of being able to inectj MSIs
>>> without the guest actually being prepared for that (because the GICv2M
>>> driver did not initialize).
>> Not sure I get what you mean here. By directly transforming the user
>> provided MSI msg into an SPI ID, do we really have to care about GICv2M?
> 
> By user provided message you mean from user space? So this is an
> emulated device, which the guest programs with an MSI payload and a
> doorbell address at least? So how would a guest know these things
> without having a MSI capable interrupt controller?
> 
> Or are we talking about different things here?

I meant GICv2m is emulated on user-side today so I don't see how the
kernel could check anything. For the MSI injection, in that case, we
only use irqfd standard IRQ injection.

Cheers

Eric
> 
> Cheers,
> Andre.
> 
>>
>> Best Regards
>>
>> Eric
>>>
>>> Cheers,
>>> Andre.
>>>
>>>>  	select HAVE_KVM_IRQCHIP
>>>>  	select HAVE_KVM_IRQ_ROUTING
>>>>  	depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
>>>> diff --git a/virt/kvm/arm/vgic-v2-emul.c b/virt/kvm/arm/vgic-v2-emul.c
>>>> index 1390797..43013cc 100644
>>>> --- a/virt/kvm/arm/vgic-v2-emul.c
>>>> +++ b/virt/kvm/arm/vgic-v2-emul.c
>>>> @@ -478,6 +478,17 @@ static bool vgic_v2_queue_sgi(struct kvm_vcpu *vcpu, int irq)
>>>>  }
>>>>  
>>>>  /**
>>>> + * 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)
>>>> +{
>>>> +	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);
>>>> +}
>>>> +
>>>> +/**
>>>>   * kvm_vgic_map_resources - Configure global VGIC state before running any VCPUs
>>>>   * @kvm: pointer to the kvm struct
>>>>   *
>>>> @@ -566,6 +577,7 @@ void vgic_v2_init_emulation(struct kvm *kvm)
>>>>  	dist->vm_ops.add_sgi_source = vgic_v2_add_sgi_source;
>>>>  	dist->vm_ops.init_model = vgic_v2_init_model;
>>>>  	dist->vm_ops.map_resources = vgic_v2_map_resources;
>>>> +	dist->vm_ops.inject_msi = vgic_v2m_inject_msi;
>>>>  
>>>>  	kvm->arch.max_vcpus = VGIC_V2_MAX_CPUS;
>>>>  }
>>>>
>>>
>>
>