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

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

[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 <ken.ccao@gmail.com>
---
 hw/pci.c |    2 +-
 hw/pci.h |    2 ++
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 70dbace..9234fe3 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -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;
 
diff --git a/hw/pci.h b/hw/pci.h
index ccb99d0..eb97b76 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -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"

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

[Ken CC]

Check if devfn < PCIBUS_MAX_DEVICES * PCI_FUNCTIONS_PER_DEVICE

Signed-off-by: Ken CC <ken.ccao@gmail.com>
---
 hw/pci.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 9234fe3..fc4becd 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -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) {

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

[Michael S. Tsirkin]

On Tue, Aug 24, 2010 at 09:27:10PM +0800, Ken CC wrote:
> Check if devfn < PCIBUS_MAX_DEVICES * PCI_FUNCTIONS_PER_DEVICE
> 
> Signed-off-by: Ken CC <ken.ccao@gmail.com>
> ---
>  hw/pci.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 9234fe3..fc4becd 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -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.


>      if (devfn < 0) {
>          for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
>              devfn += PCI_FUNC_MAX) {

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

[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 <ken.ccao@gmail.com>
---
 hw/qdev-properties.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 9219cd7..565fd08 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -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)

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

[Michael S. Tsirkin]

On Tue, Aug 24, 2010 at 09:27:41PM +0800, Ken CC wrote:
> 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.

> Signed-off-by: Ken CC <ken.ccao@gmail.com>
> ---
>  hw/qdev-properties.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 9219cd7..565fd08 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -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)

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

[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 <ken.ccao@gmail.com>
---
 hw/pci.c |    4 ++--
 hw/pci.h |    1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index fc4becd..3901455 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -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;
         }
diff --git a/hw/pci.h b/hw/pci.h
index eb97b76..f6fb6d8 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -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
 

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

[Michael S. Tsirkin]

On Tue, Aug 24, 2010 at 09:28:11PM +0800, Ken CC wrote:
> 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 <ken.ccao@gmail.com>

I liked the shorter name better ...

> ---
>  hw/pci.c |    4 ++--
>  hw/pci.h |    1 -
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index fc4becd..3901455 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -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;
>          }
> diff --git a/hw/pci.h b/hw/pci.h
> index eb97b76..f6fb6d8 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -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
>  

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

[Michael S. Tsirkin]

On Tue, Aug 24, 2010 at 09:25:26PM +0800, Ken CC wrote:
> 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 <ken.ccao@gmail.com>
> ---
>  hw/pci.c |    2 +-
>  hw/pci.h |    2 ++
>  2 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 70dbace..9234fe3 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -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;
>  
> diff --git a/hw/pci.h b/hw/pci.h
> index ccb99d0..eb97b76 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -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?

>  /* Class, Vendor and Device IDs from Linux's pci_ids.h */
>  #include "pci_ids.h"