| Message ID | 020963ffeebb642cd47ec5827bce95e9e14863d7.1418264106.git.hutao@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Dec 11, 2014 at 10:20:28AM +0800, Hu Tao wrote: > Introduce PCI_DEVFN_AUTO rather than using -1 in code. > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > --- > hw/core/qdev-properties.c | 1 + > hw/pci/pci.c | 5 ++--- > include/hw/pci/pci.h | 2 ++ > 3 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c > index 2e47f70..df4ad14 100644 > --- a/hw/core/qdev-properties.c > +++ b/hw/core/qdev-properties.c > @@ -6,6 +6,7 @@ > #include "net/hub.h" > #include "qapi/visitor.h" > #include "sysemu/char.h" > +#include "hw/pci/pci.h" Oops, this one shouldn't be here, will resend. Regards, Hu
On Thu, Dec 11, 2014 at 10:20:28AM +0800, Hu Tao wrote: > Introduce PCI_DEVFN_AUTO rather than using -1 in code. > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > --- > hw/core/qdev-properties.c | 1 + > hw/pci/pci.c | 5 ++--- > include/hw/pci/pci.h | 2 ++ > 3 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c > index 2e47f70..df4ad14 100644 > --- a/hw/core/qdev-properties.c > +++ b/hw/core/qdev-properties.c > @@ -6,6 +6,7 @@ > #include "net/hub.h" > #include "qapi/visitor.h" > #include "sysemu/char.h" > +#include "hw/pci/pci.h" > > void qdev_prop_set_after_realize(DeviceState *dev, const char *name, > Error **errp) > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 371699c..73c7dec 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -50,7 +50,7 @@ static char *pcibus_get_fw_dev_path(DeviceState *dev); > static void pcibus_reset(BusState *qbus); > > static Property pci_props[] = { > - DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1), > + DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, PCI_DEVFN_AUTO), > DEFINE_PROP_STRING("romfile", PCIDevice, romfile), > DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1), > DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present, > @@ -801,7 +801,6 @@ static void do_pci_unregister_device(PCIDevice *pci_dev) > address_space_destroy(&pci_dev->bus_master_as); > } > > -/* -1 for devfn means auto assign */ > static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, > const char *name, int devfn) > { > @@ -810,7 +809,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, > PCIConfigWriteFunc *config_write = pc->config_write; > AddressSpace *dma_as; > > - if (devfn < 0) { > + if (devfn == PCI_DEVFN_AUTO) { How do we know we found all places that do this? How do we know parameter was already validate for other values < 0? Can you pls mention in the commit log why you think it's safe? > for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices); > devfn += PCI_FUNC_MAX) { > if (!bus->devices[devfn]) > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > index b18759a..9206b12 100644 > --- a/include/hw/pci/pci.h > +++ b/include/hw/pci/pci.h > @@ -164,6 +164,8 @@ enum { > QEMU_PCIE_SLTCAP_PCP = (1 << QEMU_PCIE_SLTCAP_PCP_BITNR), > }; > > +#define PCI_DEVFN_AUTO -1 > + > #define TYPE_PCI_DEVICE "pci-device" > #define PCI_DEVICE(obj) \ > OBJECT_CHECK(PCIDevice, (obj), TYPE_PCI_DEVICE) > -- > 1.9.3
On Wed, Jan 21, 2015 at 02:00:02PM +0200, Michael S. Tsirkin wrote: > On Thu, Dec 11, 2014 at 10:20:28AM +0800, Hu Tao wrote: > > Introduce PCI_DEVFN_AUTO rather than using -1 in code. > > > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > > --- > > hw/core/qdev-properties.c | 1 + > > hw/pci/pci.c | 5 ++--- > > include/hw/pci/pci.h | 2 ++ > > 3 files changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c > > index 2e47f70..df4ad14 100644 > > --- a/hw/core/qdev-properties.c > > +++ b/hw/core/qdev-properties.c > > @@ -6,6 +6,7 @@ > > #include "net/hub.h" > > #include "qapi/visitor.h" > > #include "sysemu/char.h" > > +#include "hw/pci/pci.h" > > > > void qdev_prop_set_after_realize(DeviceState *dev, const char *name, > > Error **errp) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > > index 371699c..73c7dec 100644 > > --- a/hw/pci/pci.c > > +++ b/hw/pci/pci.c > > @@ -50,7 +50,7 @@ static char *pcibus_get_fw_dev_path(DeviceState *dev); > > static void pcibus_reset(BusState *qbus); > > > > static Property pci_props[] = { > > - DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1), > > + DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, PCI_DEVFN_AUTO), > > DEFINE_PROP_STRING("romfile", PCIDevice, romfile), > > DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1), > > DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present, > > @@ -801,7 +801,6 @@ static void do_pci_unregister_device(PCIDevice *pci_dev) > > address_space_destroy(&pci_dev->bus_master_as); > > } > > > > -/* -1 for devfn means auto assign */ > > static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, > > const char *name, int devfn) > > { > > @@ -810,7 +809,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, > > PCIConfigWriteFunc *config_write = pc->config_write; > > AddressSpace *dma_as; > > > > - if (devfn < 0) { > > + if (devfn == PCI_DEVFN_AUTO) { > > How do we know we found all places that do this? Good question! Actually I found other places creating devfn=-1 pci devices, by calling pci_create_simple_multifunction() and pci_create_simple(). But I don't have a solid way to discover all these places. So we should keep the original code here. Please drop this patch. Thanks, Hu > How do we know parameter was already validate for > other values < 0? > Can you pls mention in the commit log why you think > it's safe? > > > > for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices); > > devfn += PCI_FUNC_MAX) { > > if (!bus->devices[devfn]) > > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > > index b18759a..9206b12 100644 > > --- a/include/hw/pci/pci.h > > +++ b/include/hw/pci/pci.h > > @@ -164,6 +164,8 @@ enum { > > QEMU_PCIE_SLTCAP_PCP = (1 << QEMU_PCIE_SLTCAP_PCP_BITNR), > > }; > > > > +#define PCI_DEVFN_AUTO -1 > > + > > #define TYPE_PCI_DEVICE "pci-device" > > #define PCI_DEVICE(obj) \ > > OBJECT_CHECK(PCIDevice, (obj), TYPE_PCI_DEVICE) > > -- > > 1.9.3
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index 2e47f70..df4ad14 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -6,6 +6,7 @@ #include "net/hub.h" #include "qapi/visitor.h" #include "sysemu/char.h" +#include "hw/pci/pci.h" void qdev_prop_set_after_realize(DeviceState *dev, const char *name, Error **errp) diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 371699c..73c7dec 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -50,7 +50,7 @@ static char *pcibus_get_fw_dev_path(DeviceState *dev); static void pcibus_reset(BusState *qbus); static Property pci_props[] = { - DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1), + DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, PCI_DEVFN_AUTO), DEFINE_PROP_STRING("romfile", PCIDevice, romfile), DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1), DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present, @@ -801,7 +801,6 @@ static void do_pci_unregister_device(PCIDevice *pci_dev) address_space_destroy(&pci_dev->bus_master_as); } -/* -1 for devfn means auto assign */ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, const char *name, int devfn) { @@ -810,7 +809,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, PCIConfigWriteFunc *config_write = pc->config_write; AddressSpace *dma_as; - if (devfn < 0) { + if (devfn == PCI_DEVFN_AUTO) { for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices); devfn += PCI_FUNC_MAX) { if (!bus->devices[devfn]) diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index b18759a..9206b12 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -164,6 +164,8 @@ enum { QEMU_PCIE_SLTCAP_PCP = (1 << QEMU_PCIE_SLTCAP_PCP_BITNR), }; +#define PCI_DEVFN_AUTO -1 + #define TYPE_PCI_DEVICE "pci-device" #define PCI_DEVICE(obj) \ OBJECT_CHECK(PCIDevice, (obj), TYPE_PCI_DEVICE)
Introduce PCI_DEVFN_AUTO rather than using -1 in code. Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- hw/core/qdev-properties.c | 1 + hw/pci/pci.c | 5 ++--- include/hw/pci/pci.h | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-)