| Message ID | 1500236854-28271-2-git-send-email-mark.cave-ayland@ilande.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 16/07/2017 23:27, Mark Cave-Ayland wrote: > Also touch up the logic in do_pci_register_device() accordingly. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/pci/pci.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 0c6f74a..efc9c86 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -951,6 +951,11 @@ uint16_t pci_requester_id(PCIDevice *dev) > return pci_req_id_cache_extract(&dev->requester_id_cache); > } > > +static bool pci_bus_devfn_available(PCIBus *bus, int devfn) > +{ > + return !(bus->devices[devfn]); > +} > + > /* -1 for devfn means auto assign */ > static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, > const char *name, int devfn, > @@ -974,14 +979,15 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, > if (devfn < 0) { > for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices); > devfn += PCI_FUNC_MAX) { > - if (!bus->devices[devfn]) > + if (pci_bus_devfn_available(bus, devfn)) { > goto found; > + } > } > error_setg(errp, "PCI: no slot/function available for %s, all in use", > name); > return NULL; > found: ; > - } else if (bus->devices[devfn]) { > + } else if (!pci_bus_devfn_available(bus, devfn)) { > error_setg(errp, "PCI: slot %d function %d not available for %s," > " in use by %s", > PCI_SLOT(devfn), PCI_FUNC(devfn), name, > Reviewed-by: Marcel Apfelbaum <marcel@redhat.com> Thanks, Marcel
在 2017/7/17 上午4:27, Mark Cave-Ayland 写道: > Also touch up the logic in do_pci_register_device() accordingly. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/pci/pci.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 0c6f74a..efc9c86 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -951,6 +951,11 @@ uint16_t pci_requester_id(PCIDevice *dev) > return pci_req_id_cache_extract(&dev->requester_id_cache); > } > > +static bool pci_bus_devfn_available(PCIBus *bus, int devfn) > +{ > + return !(bus->devices[devfn]); Hi, I want to ask a question. According to the next patch, you check bus->devices[devfn] and reserved bit separately. Why not move the reserved bit check here? I think bus->devices[devfn] != NULL or revsered bit set means slot is unavailable. Regards, Yi Min > +} > + > /* -1 for devfn means auto assign */ > static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, > const char *name, int devfn, > @@ -974,14 +979,15 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, > if (devfn < 0) { > for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices); > devfn += PCI_FUNC_MAX) { > - if (!bus->devices[devfn]) > + if (pci_bus_devfn_available(bus, devfn)) { > goto found; > + } > } > error_setg(errp, "PCI: no slot/function available for %s, all in use", > name); > return NULL; > found: ; > - } else if (bus->devices[devfn]) { > + } else if (!pci_bus_devfn_available(bus, devfn)) { > error_setg(errp, "PCI: slot %d function %d not available for %s," > " in use by %s", > PCI_SLOT(devfn), PCI_FUNC(devfn), name,
On 04/09/17 11:01, Yi Min Zhao wrote: > I want to ask a question. According to the next patch, you check > bus->devices[devfn] > and reserved bit separately. Why not move the reserved bit check here? > I think bus->devices[devfn] != NULL or revsered bit set means slot is > unavailable. Primarily it was to allow callers to distinguish between the two different cases i.e. the error message can tell you that the reason the slot was unavailable was because it was reserved, i.e. you can *never* plug anything into it compared with unavailable where someone has plugged something into the slot with -device but it could be later removed, for example with hotplug. ATB, Mark.
diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 0c6f74a..efc9c86 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -951,6 +951,11 @@ uint16_t pci_requester_id(PCIDevice *dev) return pci_req_id_cache_extract(&dev->requester_id_cache); } +static bool pci_bus_devfn_available(PCIBus *bus, int devfn) +{ + return !(bus->devices[devfn]); +} + /* -1 for devfn means auto assign */ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, const char *name, int devfn, @@ -974,14 +979,15 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, if (devfn < 0) { for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices); devfn += PCI_FUNC_MAX) { - if (!bus->devices[devfn]) + if (pci_bus_devfn_available(bus, devfn)) { goto found; + } } error_setg(errp, "PCI: no slot/function available for %s, all in use", name); return NULL; found: ; - } else if (bus->devices[devfn]) { + } else if (!pci_bus_devfn_available(bus, devfn)) { error_setg(errp, "PCI: slot %d function %d not available for %s," " in use by %s", PCI_SLOT(devfn), PCI_FUNC(devfn), name,
Also touch up the logic in do_pci_register_device() accordingly. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- hw/pci/pci.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)