* [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
* 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@vger.kernel.org, avi@redhat.com, yongjie.ren@intel.com 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
* [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 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@vger.kernel.org, avi@redhat.com, yongjie.ren@intel.com 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@vger.kernel.org, yongjie.ren@intel.com 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox