All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] pci-assign: Fix MSI-X support
@ 2011-09-22  3:12 Alex Williamson
  2011-09-22  3:12 ` [PATCH 1/2] pci-assign: Re-order initfn for memory API Alex Williamson
  2011-09-22  3:12 ` [PATCH 2/2] pci-assign: Fix MSI-X registration Alex Williamson
  0 siblings, 2 replies; 6+ messages in thread
From: Alex Williamson @ 2011-09-22  3:12 UTC (permalink / raw)
  To: kvm; +Cc: jan.kiszka, avi, yongjie.ren, alex.williamson

Assigned device MSI-X support hasn't been working, this fixes
it.  I believe this should also fix:

https://bugs.launchpad.net/qemu/+bug/830558

Thanks,

Alex

---

Alex Williamson (2):
      pci-assign: Fix MSI-X registration
      pci-assign: Re-order initfn for memory API


 hw/device-assignment.c |   29 +++++++++++++++++------------
 1 files changed, 17 insertions(+), 12 deletions(-)

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/2] pci-assign: Re-order initfn for memory API
  2011-09-22  3:12 [PATCH 0/2] pci-assign: Fix MSI-X support Alex Williamson
@ 2011-09-22  3:12 ` Alex Williamson
  2011-09-22  9:03   ` Jan Kiszka
  2011-09-22  3:12 ` [PATCH 2/2] pci-assign: Fix MSI-X registration Alex Williamson
  1 sibling, 1 reply; 6+ messages in thread
From: Alex Williamson @ 2011-09-22  3:12 UTC (permalink / raw)
  To: kvm; +Cc: jan.kiszka, avi, yongjie.ren, alex.williamson

We now need to scan PCI capabilities and setup an MSI-X page
before we walk the device resources since the overlay is now
setup during init instead of at the first mapping by the guest.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 hw/device-assignment.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 288f80c..93913b3 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -1603,6 +1603,14 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
         goto out;
     }
 
+    if (assigned_device_pci_cap_init(pci_dev) < 0)
+        goto out;
+
+    /* intercept MSI-X entry page in the MMIO */
+    if (dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX)
+        if (assigned_dev_register_msix_mmio(dev))
+            goto out;
+
     /* handle real device's MMIO/PIO BARs */
     if (assigned_dev_register_regions(dev->real_device.regions,
                                       dev->real_device.region_number,
@@ -1618,9 +1626,6 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
     dev->h_busnr = dev->host.bus;
     dev->h_devfn = PCI_DEVFN(dev->host.dev, dev->host.func);
 
-    if (assigned_device_pci_cap_init(pci_dev) < 0)
-        goto out;
-
     /* assign device to guest */
     r = assign_device(dev);
     if (r < 0)
@@ -1631,11 +1636,6 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
     if (r < 0)
         goto assigned_out;
 
-    /* intercept MSI-X entry page in the MMIO */
-    if (dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX)
-        if (assigned_dev_register_msix_mmio(dev))
-            goto assigned_out;
-
     assigned_dev_load_option_rom(dev);
     QLIST_INSERT_HEAD(&devs, dev, next);
 


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/2] pci-assign: Fix MSI-X registration
  2011-09-22  3:12 [PATCH 0/2] pci-assign: Fix MSI-X support Alex Williamson
  2011-09-22  3:12 ` [PATCH 1/2] pci-assign: Re-order initfn for memory API Alex Williamson
@ 2011-09-22  3:12 ` Alex Williamson
  2011-09-22  9:04   ` Jan Kiszka
  1 sibling, 1 reply; 6+ messages in thread
From: Alex Williamson @ 2011-09-22  3:12 UTC (permalink / raw)
  To: kvm; +Cc: jan.kiszka, avi, yongjie.ren, alex.williamson

Commit c4525754 added a capability check for KVM_CAP_DEVICE_MSIX,
which is unfortunately not exposed, resulting in MSIX never
being listed as a capability.  This breaks anything depending on
MSIX, such as igbvf.  Since we can't specifically check for MSIX
support and KVM_CAP_ASSIGN_DEV_IRQ indicates more than just MSI,
let's just revert c4525754 and replace it with a sanity check that
we need KVM_CAP_ASSIGN_DEV_IRQ if the device supports any kind of
interrupt (which is still mostly paranoia).

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 hw/device-assignment.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 93913b3..b5bde68 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -1189,8 +1189,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
 
     /* Expose MSI capability
      * MSI capability is the 1st capability in capability config */
-    pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0);
-    if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ)) {
+    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0))) {
         dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI;
         /* Only 32-bit/no-mask currently supported */
         if ((ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSI, pos, 10)) < 0) {
@@ -1211,8 +1210,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
         pci_set_word(pci_dev->wmask + pos + PCI_MSI_DATA_32, 0xffff);
     }
     /* Expose MSI-X capability */
-    pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSIX, 0);
-    if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_DEVICE_MSIX)) {
+    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSIX, 0))) {
         int bar_nr;
         uint32_t msix_table_entry;
 
@@ -1606,6 +1604,13 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
     if (assigned_device_pci_cap_init(pci_dev) < 0)
         goto out;
 
+    if (!kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ) &&
+        (dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX ||
+         dev->cap.available & ASSIGNED_DEVICE_CAP_MSI ||
+         assigned_dev_pci_read_byte(pci_dev, PCI_INTERRUPT_PIN) != 0)) {
+        goto out;
+    }
+
     /* intercept MSI-X entry page in the MMIO */
     if (dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX)
         if (assigned_dev_register_msix_mmio(dev))


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] pci-assign: Re-order initfn for memory API
  2011-09-22  3:12 ` [PATCH 1/2] pci-assign: Re-order initfn for memory API Alex Williamson
@ 2011-09-22  9:03   ` Jan Kiszka
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Kiszka @ 2011-09-22  9:03 UTC (permalink / raw)
  To: Alex Williamson; +Cc: kvm, avi, yongjie.ren

On 2011-09-22 05:12, Alex Williamson wrote:
> We now need to scan PCI capabilities and setup an MSI-X page
> before we walk the device resources since the overlay is now
> setup during init instead of at the first mapping by the guest.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
> 
>  hw/device-assignment.c |   16 ++++++++--------
>  1 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index 288f80c..93913b3 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -1603,6 +1603,14 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
>          goto out;
>      }
>  
> +    if (assigned_device_pci_cap_init(pci_dev) < 0)
> +        goto out;
> +
> +    /* intercept MSI-X entry page in the MMIO */
> +    if (dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX)
> +        if (assigned_dev_register_msix_mmio(dev))
> +            goto out;
> +

Please adjust the coding style at this chance.

Looks correct otherwise.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] pci-assign: Fix MSI-X registration
  2011-09-22  3:12 ` [PATCH 2/2] pci-assign: Fix MSI-X registration Alex Williamson
@ 2011-09-22  9:04   ` Jan Kiszka
  2011-10-10 13:55     ` Avi Kivity
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2011-09-22  9:04 UTC (permalink / raw)
  To: Alex Williamson; +Cc: kvm, avi, yongjie.ren

On 2011-09-22 05:12, Alex Williamson wrote:
> Commit c4525754 added a capability check for KVM_CAP_DEVICE_MSIX,
> which is unfortunately not exposed, resulting in MSIX never
> being listed as a capability. 

Oops. Should we fix this nevertheless in the kernel?

> This breaks anything depending on
> MSIX, such as igbvf.  Since we can't specifically check for MSIX
> support and KVM_CAP_ASSIGN_DEV_IRQ indicates more than just MSI,
> let's just revert c4525754 and replace it with a sanity check that
> we need KVM_CAP_ASSIGN_DEV_IRQ if the device supports any kind of
> interrupt (which is still mostly paranoia).
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
> 
>  hw/device-assignment.c |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index 93913b3..b5bde68 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -1189,8 +1189,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
>  
>      /* Expose MSI capability
>       * MSI capability is the 1st capability in capability config */
> -    pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0);
> -    if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ)) {
> +    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0))) {
>          dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI;
>          /* Only 32-bit/no-mask currently supported */
>          if ((ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSI, pos, 10)) < 0) {
> @@ -1211,8 +1210,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
>          pci_set_word(pci_dev->wmask + pos + PCI_MSI_DATA_32, 0xffff);
>      }
>      /* Expose MSI-X capability */
> -    pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSIX, 0);
> -    if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_DEVICE_MSIX)) {
> +    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSIX, 0))) {
>          int bar_nr;
>          uint32_t msix_table_entry;
>  
> @@ -1606,6 +1604,13 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
>      if (assigned_device_pci_cap_init(pci_dev) < 0)
>          goto out;
>  
> +    if (!kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ) &&
> +        (dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX ||
> +         dev->cap.available & ASSIGNED_DEVICE_CAP_MSI ||
> +         assigned_dev_pci_read_byte(pci_dev, PCI_INTERRUPT_PIN) != 0)) {
> +        goto out;
> +    }
> +

That's not equivalent as it needlessly prevents IRQ support in the
absence of KVM_CAP_ASSIGN_DEV_IRQ.

Let's just fix the core issue and replace the test for
KVM_CAP_DEVICE_MSIX with a test call of KVM_ASSIGN_SET_MSIX_NR, passing
in a NULL struct. If it returns -EFAULT, the IOCTL is known and MSIX is
supported.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] pci-assign: Fix MSI-X registration
  2011-09-22  9:04   ` Jan Kiszka
@ 2011-10-10 13:55     ` Avi Kivity
  0 siblings, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2011-10-10 13:55 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Alex Williamson, kvm, yongjie.ren

On 09/22/2011 12:04 PM, Jan Kiszka wrote:
> >           goto out;
> >
> >  +    if (!kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ)&&
> >  +        (dev->cap.available&  ASSIGNED_DEVICE_CAP_MSIX ||
> >  +         dev->cap.available&  ASSIGNED_DEVICE_CAP_MSI ||
> >  +         assigned_dev_pci_read_byte(pci_dev, PCI_INTERRUPT_PIN) != 0)) {
> >  +        goto out;
> >  +    }
> >  +
>
> That's not equivalent as it needlessly prevents IRQ support in the
> absence of KVM_CAP_ASSIGN_DEV_IRQ.
>
> Let's just fix the core issue and replace the test for
> KVM_CAP_DEVICE_MSIX with a test call of KVM_ASSIGN_SET_MSIX_NR, passing
> in a NULL struct. If it returns -EFAULT, the IOCTL is known and MSIX is
> supported.
>

Or just add KVM_CAP_DEVICE_MSIX to the kernel and backport it where needed?

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-10-10 13:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-22  3:12 [PATCH 0/2] pci-assign: Fix MSI-X support Alex Williamson
2011-09-22  3:12 ` [PATCH 1/2] pci-assign: Re-order initfn for memory API Alex Williamson
2011-09-22  9:03   ` Jan Kiszka
2011-09-22  3:12 ` [PATCH 2/2] pci-assign: Fix MSI-X registration Alex Williamson
2011-09-22  9:04   ` Jan Kiszka
2011-10-10 13:55     ` Avi Kivity

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