Skip to contentSkip to conversation

PCI: define PCIBUS_MAX_DEVICES and PCI_FUNCTIONS_PER_DEVICE in pci.h

PATCH In review Ken CC 2 participants 8 messages Aug 24, 2010 – Sep 7, 2010 KVM

lore
Share & tools
lore

Ken CC

author
#0 lore raw

PATCH 1/4 PCI: define PCIBUS_MAX_DEVICES and PCI_FUNCTIONS_PER_DEVICE in pci.h

And update the max function number used in struct PCIBus{} to PCIBUS_MAX_FUNCTIONS = PCI_FUNCTIONS_PER_DEVICE * PCIBUS_MAX_DEVICES

TODO:

    according to Avi Kivity, PCIBus.devices[] should be renamed to functions[]
Signed-off-by Ken CC <[email protected]>
---
 hw/pci.c |    2 +-
 hw/pci.h |    2 ++
 2 files changed, 3 insertions(+), 1 deletions(-)
hw/pci.c +1 -1
index 70dbace..9234fe3 100644
@@ -47,7 +47,7 @@ struct PCIBus {
     pci_hotplug_fn hotplug;
     DeviceState *hotplug_qdev;
     void *irq_opaque;
-    PCIDevice *devices[256];
+    PCIDevice *devices[PCIBUS_MAX_DEVICES * PCI_FUNCTIONS_PER_DEVICE];
     PCIDevice *parent_dev;
     target_phys_addr_t mem_base;
hw/pci.h +2 -0
index ccb99d0..eb97b76 100644
@@ -17,6 +17,8 @@ struct kvm_irq_routing_entry;
 #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
 #define PCI_FUNC(devfn)         ((devfn) & 0x07)
 #define PCI_FUNC_MAX            8
+#define PCI_FUNCTIONS_PER_DEVICE 8
+#define PCIBUS_MAX_DEVICES 32
 
 /* Class, Vendor and Device IDs from Linux's pci_ids.h */
 #include "pci_ids.h"
author
#1 lore raw

PATCH 2/4 pci init: fail qemu if devfn exceeding the max function number supported on bus

Check if devfn < PCIBUS_MAX_DEVICES * PCI_FUNCTIONS_PER_DEVICE

Signed-off-by Ken CC <[email protected]>
---
 hw/pci.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
hw/pci.c +1 -0
index 9234fe3..fc4becd 100644
@@ -747,6 +747,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
                                          PCIConfigWriteFunc *config_write,
                                          bool is_bridge)
 {
+    assert(devfn / PCI_FUNCTIONS_PER_DEVICE < PCIBUS_MAX_DEVICES);
     if (devfn < 0) {
         for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
             devfn += PCI_FUNC_MAX) {
author
#3 lore raw

PATCH 3/4 Check pci slot number against PCIBUS_MAX_DEVICES in parse_pci_devfn

In reply to Ken CC

If pci addr provided from command line is bigger than 32, PCIBUS_MAX_DEVICES, return error -EINVAL.

32 << 3 | 7 == 256 (PCIBUS_MAX_FUNCTIONS) PCIBUS_MAX_FUNCTIONS = PCIBUS_MAX_DEVICES * PCI_FUNCTIONS_PER_DEVICE

Signed-off-by Ken CC <[email protected]>
---
 hw/qdev-properties.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
hw/qdev-properties.c +3 -1
index 9219cd7..565fd08 100644
@@ -1,5 +1,5 @@
 #include "net.h"
-#include "qdev.h"
+#include "pci.h"
 #include "qerror.h"
 
 void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)

@@ -514,6 +514,8 @@ static int parse_pci_devfn(DeviceState *dev, Property *prop, const char *str)
             return -EINVAL;
         }
     }
+    if (slot >= PCIBUS_MAX_DEVICES)
+        return -EINVAL;
     if (str[n] != '\0')
         return -EINVAL;
     if (fn > 7)
author
#5 lore raw

PATCH 4/4 Rename PCI_FUNC_MAX to PCI_FUNCTIONS_PER_DEVICES in pci.[ch]

In reply to Ken CC

PCI_FUNC_MAX is introduced by 6eab3de16d36c48a983366b09d0a0029a5260bc3
and
6fa84913eccec4266a27c81ae88465f6790742b9
which should be safe to rename to PCI_FUNCTIONS_PER_DEVICES.

Signed-off-by Ken CC <[email protected]>
---
 hw/pci.c |    4 ++--
 hw/pci.h |    1 -
 2 files changed, 2 insertions(+), 3 deletions(-)
hw/pci.c +2 -2
index fc4becd..3901455 100644
@@ -711,7 +711,7 @@ static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
         return 0;
     }
     /* function 0 indicates single function, so function > 0 must be NULL */
-    for (func = 1; func < PCI_FUNC_MAX; ++func) {
+    for (func = 1; func < PCI_FUNCTIONS_PER_DEVICE; ++func) {
         if (bus->devices[PCI_DEVFN(slot, func)]) {
             error_report("PCI: %x.0 indicates single function, "
                          "but %x.%x is already populated.",

@@ -750,7 +750,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
     assert(devfn / PCI_FUNCTIONS_PER_DEVICE < PCIBUS_MAX_DEVICES);
     if (devfn < 0) {
         for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
-            devfn += PCI_FUNC_MAX) {
+            devfn += PCI_FUNCTIONS_PER_DEVICE) {
             if (!bus->devices[devfn])
                 goto found;
         }
hw/pci.h +0 -1
index eb97b76..f6fb6d8 100644
@@ -16,7 +16,6 @@ struct kvm_irq_routing_entry;
 #define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) & 0x07))
 #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
 #define PCI_FUNC(devfn)         ((devfn) & 0x07)
-#define PCI_FUNC_MAX            8
 #define PCI_FUNCTIONS_PER_DEVICE 8
 #define PCIBUS_MAX_DEVICES 32
#7 lore raw
In reply to Ken CC
Quoted 30 lines · Ken CC

And update the max function number used in struct PCIBus{} to PCIBUS_MAX_FUNCTIONS = PCI_FUNCTIONS_PER_DEVICE * PCIBUS_MAX_DEVICES

TODO:

    according to Avi Kivity, PCIBus.devices[] should be renamed to functions[]
Signed-off-by Ken CC <[email protected]>
---
 hw/pci.c |    2 +-
 hw/pci.h |    2 ++
 2 files changed, 3 insertions(+), 1 deletions(-)
hw/pci.c +1 -1
index 70dbace..9234fe3 100644
@@ -47,7 +47,7 @@ struct PCIBus {
     pci_hotplug_fn hotplug;
     DeviceState *hotplug_qdev;
     void *irq_opaque;
-    PCIDevice *devices[256];
+    PCIDevice *devices[PCIBUS_MAX_DEVICES * PCI_FUNCTIONS_PER_DEVICE];
     PCIDevice *parent_dev;
     target_phys_addr_t mem_base;
hw/pci.h +0 -0
index ccb99d0..eb97b76 100644

hw/pci.h struct kvm_irq_routing_entry; · line 21

Quoted 5 lines
hw/pci.h +2 -0
@@ -17,6 +17,8 @@ struct kvm_irq_routing_entry;
 #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
 #define PCI_FUNC(devfn)         ((devfn) & 0x07)
 #define PCI_FUNC_MAX            8
+#define PCI_FUNCTIONS_PER_DEVICE 8
+#define PCIBUS_MAX_DEVICES 32

PCI_FUNCTIONS_PER_DEVICE is same as PCI_FUNC_MAX. Why do we need another macro?

Quoted 2 lines
hw/pci.h +0 -0
 /* Class, Vendor and Device IDs from Linux's pci_ids.h */
 #include "pci_ids.h"
#2 lore raw

Re: [PATCH 2/4] pci init: fail qemu if devfn exceeding the max function number supported on bus

In reply to Ken CC
Quoted 12 lines · Ken CC

Check if devfn < PCIBUS_MAX_DEVICES * PCI_FUNCTIONS_PER_DEVICE

Signed-off-by Ken CC <[email protected]>
---
 hw/pci.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
hw/pci.c +0 -0
index 9234fe3..fc4becd 100644

hw/pci.c static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, · line 750

Quoted 4 lines
hw/pci.c +1 -0
@@ -747,6 +747,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
                                          PCIConfigWriteFunc *config_write,
                                          bool is_bridge)
 {
+    assert(devfn / PCI_FUNCTIONS_PER_DEVICE < PCIBUS_MAX_DEVICES);

I guess it'll happen to work even for < 0, but it might be obvious to only do this in >= 0 case.
Just remove the 'else' and stick the assert there.

Quoted 3 lines
hw/pci.c +0 -0
     if (devfn < 0) {
         for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
             devfn += PCI_FUNC_MAX) {
#4 lore raw

Re: [PATCH 3/4] Check pci slot number against PCIBUS_MAX_DEVICES in parse_pci_devfn

In reply to Ken CC
Quoted 5 lines · Ken CC

If pci addr provided from command line is bigger than 32, PCIBUS_MAX_DEVICES, return error -EINVAL.

32 << 3 | 7 == 256 (PCIBUS_MAX_FUNCTIONS) PCIBUS_MAX_FUNCTIONS = PCIBUS_MAX_DEVICES * PCI_FUNCTIONS_PER_DEVICE

Not really sure what these equations mean, the patch makes sense though, we should validate input.

Quoted 25 lines
Signed-off-by Ken CC <[email protected]>
---
 hw/qdev-properties.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
hw/qdev-properties.c +3 -1
index 9219cd7..565fd08 100644
@@ -1,5 +1,5 @@
 #include "net.h"
-#include "qdev.h"
+#include "pci.h"
 #include "qerror.h"
 
 void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)

@@ -514,6 +514,8 @@ static int parse_pci_devfn(DeviceState *dev, Property *prop, const char *str)
             return -EINVAL;
         }
     }
+    if (slot >= PCIBUS_MAX_DEVICES)
+        return -EINVAL;
     if (str[n] != '\0')
         return -EINVAL;
     if (fn > 7)
#6 lore raw

Re: [PATCH 4/4] Rename PCI_FUNC_MAX to PCI_FUNCTIONS_PER_DEVICES in pci.[ch]

In reply to Ken CC
Quoted 7 lines · Ken CC

PCI_FUNC_MAX is introduced by 6eab3de16d36c48a983366b09d0a0029a5260bc3
and
6fa84913eccec4266a27c81ae88465f6790742b9
which should be safe to rename to PCI_FUNCTIONS_PER_DEVICES.

Signed-off-by Ken CC <[email protected]>

I liked the shorter name better ...

Quoted 39 lines
---
 hw/pci.c |    4 ++--
 hw/pci.h |    1 -
 2 files changed, 2 insertions(+), 3 deletions(-)
hw/pci.c +2 -2
index fc4becd..3901455 100644
@@ -711,7 +711,7 @@ static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev)
         return 0;
     }
     /* function 0 indicates single function, so function > 0 must be NULL */
-    for (func = 1; func < PCI_FUNC_MAX; ++func) {
+    for (func = 1; func < PCI_FUNCTIONS_PER_DEVICE; ++func) {
         if (bus->devices[PCI_DEVFN(slot, func)]) {
             error_report("PCI: %x.0 indicates single function, "
                          "but %x.%x is already populated.",

@@ -750,7 +750,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
     assert(devfn / PCI_FUNCTIONS_PER_DEVICE < PCIBUS_MAX_DEVICES);
     if (devfn < 0) {
         for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
-            devfn += PCI_FUNC_MAX) {
+            devfn += PCI_FUNCTIONS_PER_DEVICE) {
             if (!bus->devices[devfn])
                 goto found;
         }
hw/pci.h +0 -1
index eb97b76..f6fb6d8 100644
@@ -16,7 +16,6 @@ struct kvm_irq_routing_entry;
 #define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) & 0x07))
 #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
 #define PCI_FUNC(devfn)         ((devfn) & 0x07)
-#define PCI_FUNC_MAX            8
 #define PCI_FUNCTIONS_PER_DEVICE 8
 #define PCIBUS_MAX_DEVICES 32