Re: [Qemu-devel] [PATCHv3 1/2] pci: move check for existing devfn into new pci_bus_devfn_available() helper

[Yi Min Zhao <zyimin@linux.vnet.ibm.com>]

在 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,