Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1173890 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7] KVM: arm/arm64: gsi routing support Eric Auger <eric.auger@linaro.org> - 2015-06-29 17:40 +0200
    [PATCH 7/7] KVM: arm: implement kvm_set_msi by gsi direct mapping Eric Auger <eric.auger@linaro.org> - 2015-06-29 17:40 +0200
    [PATCH 2/7] KVM: kvm_host: add kvm_extended_msi Eric Auger <eric.auger@linaro.org> - 2015-06-29 17:40 +0200
    [PATCH 5/7] KVM: arm/arm64: build a default routing table Eric Auger <eric.auger@linaro.org> - 2015-06-29 17:40 +0200
    [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi Eric Auger <eric.auger@linaro.org> - 2015-06-29 17:40 +0200
      RE: [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi Pavel Fedin <p.fedin@samsung.com> - 2015-07-02 10:50 +0200
      RE: [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi Pavel Fedin <p.fedin@samsung.com> - 2015-07-03 17:30 +0200
        Re: [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi Eric Auger <eric.auger@linaro.org> - 2015-07-03 17:50 +0200
      Re: [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi Andre Przywara <andre.przywara@arm.com> - 2015-07-03 18:00 +0200
      Re: [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi Andre Przywara <andre.przywara@arm.com> - 2015-07-07 12:10 +0200
        RE: [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi Pavel Fedin <p.fedin@samsung.com> - 2015-07-07 13:00 +0200
    [PATCH 6/7] KVM: arm/arm64: enable MSI routing Eric Auger <eric.auger@linaro.org> - 2015-06-29 17:40 +0200
    [PATCH 3/7] KVM: irqchip: convey devid to kvm_set_msi Eric Auger <eric.auger@linaro.org> - 2015-06-29 17:50 +0200

#1173890 — [PATCH 0/7] KVM: arm/arm64: gsi routing support

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)
- MSI routing without GICv3 ITS was tested using APM Xgene-I
  (qemu VIRTIO-PCI vhost net without gsi_direct_mapping).
- MSI routing with GICv2 ITS is NOT yet tested.

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

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

History:

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: add kvm_irq_routing_extended_msi
  KVM: kvm_host: add kvm_extended_msi
  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 |  30 ++++++++--
 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          |  10 ++++
 include/uapi/linux/kvm.h          |   6 +-
 virt/kvm/arm/vgic.c               | 117 ++++++++++++++++++++++++++++----------
 virt/kvm/irqchip.c                |  20 +++++--
 12 files changed, 154 insertions(+), 50 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1173895 — [PATCH 7/7] KVM: arm: implement kvm_set_msi by gsi direct mapping

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>
---
 arch/arm/kvm/Kconfig | 1 +
 virt/kvm/arm/vgic.c  | 5 +++++
 2 files changed, 6 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.c b/virt/kvm/arm/vgic.c
index 0b4c48c..b3c10dc 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -2314,6 +2314,11 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
 			return kvm->arch.vgic.vm_ops.inject_msi(kvm, &msi);
 		else
 			return -ENODEV;
+	case KVM_IRQ_ROUTING_MSI:
+		if (kvm->arch.vgic.vm_ops.inject_msi)
+			return -EINVAL;
+		else
+			return kvm_vgic_inject_irq(kvm, 0, e->msi.data, level);
 	default:
 		return -EINVAL;
 	}
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1173896 — [PATCH 2/7] KVM: kvm_host: add kvm_extended_msi

Add a new kvm_extended_msi struct to store the additional device ID
specific to ARM. kvm_kernel_irq_routing_entry union now encompasses
this new struct.

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

---

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

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index ad45054..e1c1c0d 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -304,6 +304,13 @@ struct kvm_s390_adapter_int {
 	u32 adapter_id;
 };
 
+struct kvm_extended_msi {
+	u32     address_lo;     /* low 32 bits of msi message address */
+	u32     address_hi;     /* high 32 bits of msi message address */
+	u32     data;           /* 16 bits of msi message data */
+	u32	devid;		/* out-of-band device ID */
+};
+
 struct kvm_kernel_irq_routing_entry {
 	u32 gsi;
 	u32 type;
@@ -317,6 +324,7 @@ struct kvm_kernel_irq_routing_entry {
 		} irqchip;
 		struct msi_msg msi;
 		struct kvm_s390_adapter_int adapter;
+		struct kvm_extended_msi ext_msi;
 	};
 	struct hlist_node link;
 };
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1173898 — [PATCH 5/7] KVM: arm/arm64: build a default routing table

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 212a5ff..410fee1 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1783,6 +1783,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;
 
@@ -2264,6 +2266,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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1173901 — [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi

On ARM, the MSI msg (address and data) comes along with
out-of-band device ID information. The device ID encodes the device
that composes the MSI msg. Let's create a new routing entry type,
dubbed KVM_IRQ_ROUTING_EXTENDED_MSI and use the __u32 pad space
to convey the device ID.

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

---

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

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index d20fd94..6426ae9 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1414,7 +1414,10 @@ struct kvm_irq_routing_entry {
 	__u32 gsi;
 	__u32 type;
 	__u32 flags;
-	__u32 pad;
+	union {
+		__u32 pad;
+		__u32 devid;
+	};
 	union {
 		struct kvm_irq_routing_irqchip irqchip;
 		struct kvm_irq_routing_msi msi;
@@ -1427,6 +1430,10 @@ 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
+
+In case of KVM_IRQ_ROUTING_EXTENDED_MSI routing type, devid is used to convey
+the device ID.
 
 No flags are specified so far, the corresponding field must be set to zero.
 
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 2a23705..8484681 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -841,12 +841,16 @@ 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;
 	__u32 type;
 	__u32 flags;
-	__u32 pad;
+	union {
+		__u32 pad;
+		__u32 devid;
+	};
 	union {
 		struct kvm_irq_routing_irqchip irqchip;
 		struct kvm_irq_routing_msi msi;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1175828 — RE: [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi

 Hello!

> What if we use KVM_MSI_VALID_DEVID flag instead of new KVM_IRQ_ROUTING_EXTENDED_MSI
> definition? I
> believe this would make an API more consistent and introduce less new definitions.

 I have just found one more flaw in your implementation. If you take a look at irqfd_wakeup()...
--- cut ---
		/* An event has been signaled, inject an interrupt */
		if (irq.type == KVM_IRQ_ROUTING_MSI)
			kvm_set_msi(&irq, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1,
					false);
		else
			schedule_work(&irqfd->inject);
--- cut ---
 You apparently missed KVM_IRQ_ROUTING_EXTENDED_MSI here, as well as in irqfd_update(). But, if you
accept my API proposal, this becomes irrelevant.

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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1176665 — RE: [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi

 Hi!

> OK so both of you say the same thing. Will respin accordingly

 You may also want to add this:
Tested-by: Pavel Fedin <p.fedin@samsung.com>

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1176681 — Re: [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi

On 07/03/2015 05:29 PM, Pavel Fedin wrote:
>  Hi!
> 
>> OK so both of you say the same thing. Will respin accordingly
> 
>  You may also want to add this:
> Tested-by: Pavel Fedin <p.fedin@samsung.com>

Thanks Pavel for the intent. However since I am going to change the uapi
and correct the bug you spotted out, this will need to be tested again.
T-b is applied when the code is stable and bug-free I think ;-)

Best Regards

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1176686 — Re: [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi

Hi,

On 03/07/15 10:05, Andre Przywara wrote:
> Hi Pavel,
> 
> On 02/07/15 08:26, Pavel Fedin wrote:
>>  Hello!
>>
>>> -----Original Message-----
>>> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On Behalf Of Eric Auger
>>> Sent: Monday, June 29, 2015 6:37 PM
>>> To: eric.auger@st.com; eric.auger@linaro.org; linux-arm-kernel@lists.infradead.org;
>>> marc.zyngier@arm.com; christoffer.dall@linaro.org; andre.przywara@arm.com;
>>> kvmarm@lists.cs.columbia.edu; kvm@vger.kernel.org
>>> Cc: linux-kernel@vger.kernel.org; patches@linaro.org; p.fedin@samsung.com; pbonzini@redhat.com
>>> Subject: [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi
>>>
>>> On ARM, the MSI msg (address and data) comes along with
>>> out-of-band device ID information. The device ID encodes the device
>>> that composes the MSI msg. Let's create a new routing entry type,
>>> dubbed KVM_IRQ_ROUTING_EXTENDED_MSI and use the __u32 pad space
>>> to convey the device ID.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>>
>>> ---
>>>
>>> RFC -> PATCH
>>> - remove kvm_irq_routing_extended_msi and use union instead
>>> ---
>>>  Documentation/virtual/kvm/api.txt | 9 ++++++++-
>>>  include/uapi/linux/kvm.h          | 6 +++++-
>>>  2 files changed, 13 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>>> index d20fd94..6426ae9 100644
>>> --- a/Documentation/virtual/kvm/api.txt
>>> +++ b/Documentation/virtual/kvm/api.txt
>>> @@ -1414,7 +1414,10 @@ struct kvm_irq_routing_entry {
>>>  	__u32 gsi;
>>>  	__u32 type;
>>>  	__u32 flags;
>>> -	__u32 pad;
>>> +	union {
>>> +		__u32 pad;
>>> +		__u32 devid;
>>> +	};
>>>  	union {
>>>  		struct kvm_irq_routing_irqchip irqchip;
>>>  		struct kvm_irq_routing_msi msi;
>>
>>  devid is actually a part of MSI bunch. Shouldn't it be a part of struct kvm_irq_routing_msi then?
>> It also has reserved pad.
>>
>>> @@ -1427,6 +1430,10 @@ 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
>>> +
>>> +In case of KVM_IRQ_ROUTING_EXTENDED_MSI routing type, devid is used to convey
>>> +the device ID.
>>>
>>>  No flags are specified so far, the corresponding field must be set to zero.
>>
>> What if we use KVM_MSI_VALID_DEVID flag instead of new KVM_IRQ_ROUTING_EXTENDED_MSI definition? I
>> believe this would make an API more consistent and introduce less new definitions.
> 
> I like this approach, but it runs into problems:
> As you read above the current documentation says that the flags field
> must be zero and the current KVM_SET_GSI_ROUTING handler bails out if it
> isn't. So userland would need to know whether it's safe to set that
> field. Introducing a new KVM_CAP_... value seems overkill if we could
> just have a new routing entry type. So we could still reuse the existing
> struct kvm_irq_routing_msi (and extend that with the devid field), but
> we would have to add a new routing type number.

> Maybe we could collapse this into the existing MSI type + flag when
> handing it further down the kernel?

FWIW, I gave this a try, this doesn't look to bad. I carried the new
type down till virt/kvm/arm/vgic.c:kvm_set_routing_entry(), where the
EXTENDED type got turned back into the normal MSI type while setting the
flag in the internal struct kvm_kernel_irq_routing_entry. This keeps the
new type only to the userland facing side, with the kernel code staying
mostly the same.
Together with a new KVM_CAP_MSIS_REQUIRE_DEVID capability I can now
drive both GICv2M and ITS emulation from the same userland base in a
sane manner.
If someone wants to have a look now, tell me, otherwise I will wait for
Eric's upcoming code drop and comment on that then.

Cheers,
Andre.

> 
> Cheers,
> Andre.
> 
>>
>>>
>>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>>> index 2a23705..8484681 100644
>>> --- a/include/uapi/linux/kvm.h
>>> +++ b/include/uapi/linux/kvm.h
>>> @@ -841,12 +841,16 @@ 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;
>>>  	__u32 type;
>>>  	__u32 flags;
>>> -	__u32 pad;
>>> +	union {
>>> +		__u32 pad;
>>> +		__u32 devid;
>>> +	};
>>>  	union {
>>>  		struct kvm_irq_routing_irqchip irqchip;
>>>  		struct kvm_irq_routing_msi msi;
>>> --
>>> 1.9.1
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>> Kind regards,
>> Pavel Fedin
>> Expert Engineer
>> Samsung Electronics Research center Russia
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1178299 — Re: [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi

Good morning Pavel,

On 07/07/15 08:16, Pavel Fedin wrote:
>  Hello!
> 
>> Wouldn't:
>>     if (kvm_vm_check_extension(s, KVM_CAP_MSI_DEVID)) {
>>         kroute.flags = KVM_MSI_VALID_DEVID;
>>         kroute.u.msi.devid = (pci_bus_num(dev->bus) << 8) | dev->devfn;
>>     }
>>
>> be saner (without a global variable)?
> 
>  No it would not, because:
> a) kvm_vm_check_extension() calls ioctl every time, therefore it's slow. But, well, doesn't really
> matter because it's possible to check for the capability once in generic code, and cache it.

Indeed, as mentioned before I have it in a wrapper function with a
static variable.

> b) Capability is a global thing as far as i understand. The kernel either has it, or doesn't have.

There are two flavours of capabilities: global and per-VM ones,
depending on which fd you are issuing the ioctl. The per-VM ones are
just not very widely used yet (I only found PowerPC doing so).

> However, whether we want this flag or not, depends also on what GIC model we use. GICv2(m) doesn't
> want it, GICv3 does. qemu actually has two sets of flags: one set actually specifies capabilities,
> another set enables use of these capabilities.

That's why I do a per-VM capability check and I do it as late as
possible to let the GIC initialize first (hence the wrapper function).

>  But, well, you can make GICv2 kernel code simply ignoring it instead of bailing out if flags != 0.
> And add the capability for ARM64 architecture (ARM32 can't use GICv3, can it?). And this will work
> and it'll be OK. So, i'm not against it, and if you want it, you can do it. I just want to point
> that it is not strictly necessary to add new APIs, because existing ones are pretty much enough.

As said before I don't like the idea of inferring the validity of a flag
by some hard-coded dependencies like "GICv3 on ARM64". I guess ARM(32)
will get GICv3 support sooner or later (I think I saw patches to do so
already). So as soon as a kernel supports it, we automatically get the
support from userland without changing a single line there. Also what
tells you that no other architecture or IRQ controller will ever need a
device ID? I just don't want to end up with something like:
(GICV3 && ARM64) || (GICV3 && ARM && KERNEL>4.4) || (SuperIRQC && i986)
or
(ARM || ARM64) && HAS_IRQ_ROUTING

Instead: If the kernel needs it, it tells you. Full stop.

> But, you are the architects here, so you of course can do it if you want.
> It's just me being not a
> big fan of adding APIs without which it's completely possible to live.
> 
>  Below i'm answering to Eric's comment, because my reply is tightly coupled with this one.
> 
>> So not sure whether we eventually concluded;-)
>> - introduce a KVM_CAP_MSI_DEVID capability? All OK except Pavel not
>>  convinced?
> 
>  See above. I'm not against it, i just don't think it's necessary. You can do it if you want, it
> actually won't change things much.
> 
>> - userspaces puts the devid in struct kvm_irq_routing_msi pad field:
>> consensus (we do not intrduce a new kvm_irq_routing_ext_msi)
> 
>  Yes.
> 
>> - userspace tells it conveyed a devid by setting
>> A) the kvm_irq_routing_entry's field?
>> B) the kvm_irq_routing_entry's type
>> no consensus. If there is a cap, does it really matter?
> 
>  It has absolutely nothing to do with the cap. My argument here is the same as above again - why
> adding new API's / definitions? We already have KVM_MSI_VALID_DEVID and we already have 'flags'
> field. Using them would just make the API more consistent because KVM_SIGNAL_MSI already uses them
> in absolutely the same manner. That's my point and nothing more.

To be honest it's me to blame here to not having introduced the
capability earlier. At the moment ARM has a different code path for
KVM_SIGNAL_MSI, which does not bail out if the flag field is set. With
Eric's patches this changes and we use the irqchip.c generic code, which
returns -EINVAL atm. So I plan to introduce this capability already with
the ITS emulation series, so we can just pick it up in the IRQ routing
series.
So we now have already two users of this, if that makes more sense.

Cheers,
Andre.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1178337 — RE: [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi

> I just don't want to end up with something like:
> (GICV3 && ARM64) || (GICV3 && ARM && KERNEL>4.4) || (SuperIRQC && i986)
> or
> (ARM || ARM64) && HAS_IRQ_ROUTING
> 
> Instead: If the kernel needs it, it tells you. Full stop.

 Agree.

> To be honest it's me to blame here to not having introduced the
> capability earlier. At the moment ARM has a different code path for
> KVM_SIGNAL_MSI, which does not bail out if the flag field is set. With
> Eric's patches this changes and we use the irqchip.c generic code, which
> returns -EINVAL atm. So I plan to introduce this capability already with
> the ITS emulation series, so we can just pick it up in the IRQ routing
> series.

 Then may be you follow https://lkml.org/lkml/2015/7/7/115 and replace flag with something like
KVM_SIGNAL_EXT_MSI ioctl ? After all you were one of people who voted against using flags.

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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1173903 — [PATCH 6/7] KVM: arm/arm64: enable MSI routing

Up to now, only irqchip routing entries could be set. This patch
adds the capability to insert MSI routing entries, extended or
standard ones. 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.

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

---

RFC -> PATCH:
- move api MSI routing updates into that patch file
- use new devid field of user api struct
---
 Documentation/virtual/kvm/api.txt |  9 +++++++++
 include/linux/kvm_host.h          |  2 ++
 virt/kvm/arm/vgic.c               | 13 +++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 1e0d5f5..b411232 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1405,6 +1405,10 @@ 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 to specify additionnal device ID.
+Otherwise, KVM_IRQ_ROUTING_MSI must be used.
+
 struct kvm_irq_routing {
 	__u32 nr;
 	__u32 flags;
@@ -2315,6 +2319,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 patches 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 e1c1c0d..4ca8f8e 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -927,6 +927,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 410fee1..0b4c48c 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -2258,6 +2258,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->ext_msi.address_lo = ue->u.msi.address_lo;
+		e->ext_msi.address_hi = ue->u.msi.address_hi;
+		e->ext_msi.data = ue->u.msi.data;
+		e->ext_msi.devid = ue->devid;
+		break;
 	default:
 		goto out;
 	}
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1173908 — [PATCH 3/7] KVM: irqchip: convey devid to kvm_set_msi

on ARM, a devid field is conveyed in kvm_msi struct. Let's choose the
routing type and struct according to its availability and fill the
corresponding struct. Also remove the flag check now this latter can
be non null.

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

diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
index 1d56a90..e76c7d2 100644
--- a/virt/kvm/irqchip.c
+++ b/virt/kvm/irqchip.c
@@ -73,12 +73,22 @@ 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;
 
-	route.msi.address_lo = msi->address_lo;
-	route.msi.address_hi = msi->address_hi;
-	route.msi.data = msi->data;
+	if (msi->flags & KVM_MSI_VALID_DEVID) {
+		route.type = KVM_IRQ_ROUTING_EXTENDED_MSI;
+		route.ext_msi.address_lo = msi->address_lo;
+		route.ext_msi.address_hi = msi->address_hi;
+		route.ext_msi.data = msi->data;
+		route.ext_msi.devid= msi->devid;
+	}
+	else {
+		route.type = KVM_IRQ_ROUTING_MSI;
+		route.msi.address_lo = msi->address_lo;
+		route.msi.address_hi = msi->address_hi;
+		route.msi.data = msi->data;
+	}
 
 	return kvm_set_msi(&route, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1, false);
 }
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web