diff mbox

[5/7] qemu-kvm: Move gsi bits from kvm_msix_vector_add to kvm_msi_add_message

Message ID cbb603304713eeb0ad413a98ab7705cb28dcc428.1303554218.git.jan.kiszka@web.de (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Kiszka April 23, 2011, 10:23 a.m. UTC
From: Jan Kiszka <jan.kiszka@siemens.com>

Testing support and allocating a GSI for an MSI message is required both
for MSI and MSI-X. At this chance, drop the aging version warning.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/msix.c  |   13 -------------
 qemu-kvm.c |   11 +++++++++++
 2 files changed, 11 insertions(+), 13 deletions(-)

Comments

Michael S. Tsirkin April 27, 2011, 12:54 p.m. UTC | #1
On Sat, Apr 23, 2011 at 12:23:38PM +0200, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Testing support and allocating a GSI for an MSI message is required both
> for MSI and MSI-X. At this chance, drop the aging version warning.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

No objection, but I do note that this means running on an old
kernel will lead to a silent failure.
stderr output is not in fact much better: I think we should
check the capability in msix_init. Care coding this up?

> ---
>  hw/msix.c  |   13 -------------
>  qemu-kvm.c |   11 +++++++++++
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/msix.c b/hw/msix.c
> index 1bdffb6..8c8bc18 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -113,19 +113,6 @@ static int kvm_msix_vector_add(PCIDevice *dev, unsigned vector)
>      KVMMsiMessage *kmm = dev->msix_irq_entries + vector;
>      int r;
>  
> -    if (!kvm_has_gsi_routing()) {
> -        fprintf(stderr, "Warning: no MSI-X support found. "
> -                "At least kernel 2.6.30 is required for MSI-X support.\n"
> -               );
> -        return -EOPNOTSUPP;
> -    }
> -
> -    r = kvm_get_irq_route_gsi();
> -    if (r < 0) {
> -        fprintf(stderr, "%s: kvm_get_irq_route_gsi failed: %s\n", __func__, strerror(-r));
> -        return r;
> -    }
> -    kmm->gsi = r;
>      kvm_msix_message_from_vector(dev, vector, kmm);
>      r = kvm_msi_message_add(kmm);
>      if (r < 0) {
> diff --git a/qemu-kvm.c b/qemu-kvm.c
> index 9cbc109..7317f87 100644
> --- a/qemu-kvm.c
> +++ b/qemu-kvm.c
> @@ -984,6 +984,17 @@ static void kvm_msi_routing_entry(struct kvm_irq_routing_entry *e,
>  int kvm_msi_message_add(KVMMsiMessage *msg)
>  {
>      struct kvm_irq_routing_entry e;
> +    int ret;
> +
> +    if (!kvm_has_gsi_routing()) {
> +        return -EOPNOTSUPP;
> +    }
> +
> +    ret = kvm_get_irq_route_gsi();
> +    if (ret < 0) {
> +        return ret;
> +    }
> +    msg->gsi = ret;
>  
>      kvm_msi_routing_entry(&e, msg);
>      return kvm_add_routing_entry(&e);
> -- 
> 1.7.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
Jan Kiszka April 27, 2011, 1:29 p.m. UTC | #2
On 2011-04-27 14:54, Michael S. Tsirkin wrote:
> On Sat, Apr 23, 2011 at 12:23:38PM +0200, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Testing support and allocating a GSI for an MSI message is required both
>> for MSI and MSI-X. At this chance, drop the aging version warning.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> No objection, but I do note that this means running on an old
> kernel will lead to a silent failure.

Shouldn't be silent: The caller of msi_vector_use should check and
process the error (virtio should even forward it to guest IIUC).
> stderr output is not in fact much better: I think we should
> check the capability in msix_init. Care coding this up?

I think the motivation to check on vector activation is that devices and
guests without a need for MSI should not cause a failure if MSI is
unsupported by KVM.

Jan
Michael S. Tsirkin April 27, 2011, 1:36 p.m. UTC | #3
On Wed, Apr 27, 2011 at 03:29:15PM +0200, Jan Kiszka wrote:
> On 2011-04-27 14:54, Michael S. Tsirkin wrote:
> > On Sat, Apr 23, 2011 at 12:23:38PM +0200, Jan Kiszka wrote:
> >> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >> Testing support and allocating a GSI for an MSI message is required both
> >> for MSI and MSI-X. At this chance, drop the aging version warning.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > No objection, but I do note that this means running on an old
> > kernel will lead to a silent failure.
> 
> Shouldn't be silent: The caller of msi_vector_use should check and
> process the error (virtio should even forward it to guest IIUC).

It does, but the msi/msix spec does not allow this so not
all guest OSes can use this extended reporting.

> > stderr output is not in fact much better: I think we should
> > check the capability in msix_init. Care coding this up?
> 
> I think the motivation to check on vector activation is that devices and
> guests without a need for MSI should not cause a failure if MSI is
> unsupported by KVM.
> 
> Jan

IMO it's better not to report MSI capability in this setup.


> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux
--
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
diff mbox

Patch

diff --git a/hw/msix.c b/hw/msix.c
index 1bdffb6..8c8bc18 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -113,19 +113,6 @@  static int kvm_msix_vector_add(PCIDevice *dev, unsigned vector)
     KVMMsiMessage *kmm = dev->msix_irq_entries + vector;
     int r;
 
-    if (!kvm_has_gsi_routing()) {
-        fprintf(stderr, "Warning: no MSI-X support found. "
-                "At least kernel 2.6.30 is required for MSI-X support.\n"
-               );
-        return -EOPNOTSUPP;
-    }
-
-    r = kvm_get_irq_route_gsi();
-    if (r < 0) {
-        fprintf(stderr, "%s: kvm_get_irq_route_gsi failed: %s\n", __func__, strerror(-r));
-        return r;
-    }
-    kmm->gsi = r;
     kvm_msix_message_from_vector(dev, vector, kmm);
     r = kvm_msi_message_add(kmm);
     if (r < 0) {
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 9cbc109..7317f87 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -984,6 +984,17 @@  static void kvm_msi_routing_entry(struct kvm_irq_routing_entry *e,
 int kvm_msi_message_add(KVMMsiMessage *msg)
 {
     struct kvm_irq_routing_entry e;
+    int ret;
+
+    if (!kvm_has_gsi_routing()) {
+        return -EOPNOTSUPP;
+    }
+
+    ret = kvm_get_irq_route_gsi();
+    if (ret < 0) {
+        return ret;
+    }
+    msg->gsi = ret;
 
     kvm_msi_routing_entry(&e, msg);
     return kvm_add_routing_entry(&e);