[libvirt] [PATCH 6/6] virpci: Allow greater PCI domain value in virPCIDeviceAddressIsValid

Michal Privoznik posted 6 patches 6 years, 6 months ago
[libvirt] [PATCH 6/6] virpci: Allow greater PCI domain value in virPCIDeviceAddressIsValid
Posted by Michal Privoznik 6 years, 6 months ago
There is no restriction on maximum value of PCI domain. In fact,
Linux kernel uses plain atomic inc when assigning PCI domains:

drivers/pci/pci.c:static int pci_get_new_domain_nr(void)
drivers/pci/pci.c-{
drivers/pci/pci.c-      return atomic_inc_return(&__domain_nr);
drivers/pci/pci.c-}

Of course, this function is called only if kernel was compiled
without PCI domain support or ACPI did not provide PCI domain.

However, QEMU still has the same restriction as us: in
set_pci_host_devaddr() QEMU checks if domain isn't greater than
0xffff. But one can argue that that's a QEMU limitation. We still
want to be able to cope with other hypervisors that don't have
this limitation (possibly).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 docs/schemas/basictypes.rng                   | 2 +-
 src/util/virpci.c                             | 2 +-
 tests/qemuxml2argvdata/pci-domain-invalid.xml | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 70d2101b78..81465273c8 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -324,7 +324,7 @@
   </define>
 
   <define name="pciDomain">
-    <ref name="uint16"/>
+    <ref name="uint32"/>
   </define>
   <define name="pciBus">
     <ref name="uint8"/>
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 36b7f8b424..bc7ff46194 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1668,7 +1668,7 @@ bool
 virPCIDeviceAddressIsValid(virPCIDeviceAddressPtr addr,
                            bool report)
 {
-    if (addr->domain > 0xFFFF) {
+    if (addr->domain > 0xFFFFFFFF) {
         if (report)
             virReportError(VIR_ERR_XML_ERROR,
                            _("Invalid PCI address domain='0x%x', "
diff --git a/tests/qemuxml2argvdata/pci-domain-invalid.xml b/tests/qemuxml2argvdata/pci-domain-invalid.xml
index 1ac56fc703..21f1dc98af 100644
--- a/tests/qemuxml2argvdata/pci-domain-invalid.xml
+++ b/tests/qemuxml2argvdata/pci-domain-invalid.xml
@@ -26,7 +26,7 @@
     <interface type='user'>
       <mac address='00:11:22:33:44:55'/>
       <model type='rtl8139'/>
-      <address type='pci' domain='0x10000' bus='0x00' slot='0x05' function='0x0'/>
+      <address type='pci' domain='0x0000' bus='0x100' slot='0x05' function='0x0'/>
     </interface>
     <memballoon model='none'/>
   </devices>
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] virpci: Allow greater PCI domain value in virPCIDeviceAddressIsValid
Posted by Ján Tomko 6 years, 6 months ago
On Wed, Jul 31, 2019 at 10:32:24AM +0200, Michal Privoznik wrote:
>There is no restriction on maximum value of PCI domain. In fact,
>Linux kernel uses plain atomic inc when assigning PCI domains:
>
>drivers/pci/pci.c:static int pci_get_new_domain_nr(void)
>drivers/pci/pci.c-{
>drivers/pci/pci.c-      return atomic_inc_return(&__domain_nr);
>drivers/pci/pci.c-}
>
>Of course, this function is called only if kernel was compiled
>without PCI domain support or ACPI did not provide PCI domain.
>
>However, QEMU still has the same restriction as us: in
>set_pci_host_devaddr() QEMU checks if domain isn't greater than
>0xffff. But one can argue that that's a QEMU limitation. We still
>want to be able to cope with other hypervisors that don't have
>this limitation (possibly).
>

I would argue that by lifting this check we fail to report the error
early for QEMU that (still) lacks the support for it.

But I doubt that the QEMU fix will be detectable in our capability
probing code, so:

>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> docs/schemas/basictypes.rng                   | 2 +-
> src/util/virpci.c                             | 2 +-
> tests/qemuxml2argvdata/pci-domain-invalid.xml | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list