Re: [Qemu-devel] [PATCH 3/3] pci: add reserved slot check to do_pci_register_device()

[Marcel Apfelbaum]

On 07/07/2017 10:44, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <address@hidden>
> ---
>   hw/pci/pci.c |   21 ++++++++++++++++++---
>   1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 239161e..9dece2a 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -961,6 +961,15 @@ static bool pci_bus_devfn_available(PCIBus *bus, int devfn)
>       }
>   }
>  

Hi Mark,


> +static bool pci_bus_devfn_reserved(PCIBus *bus, int devfn)
> +{
> +    if (bus->dev_reserved_mask & (1UL << PCI_SLOT(devfn))) {

Looking at this code maybe we should rename the field to
"slot_mask"? Only a suggestion.

> +        return true;
> +    } else {
> +        return false;
> +    }
> +}
> + >   /* -1 for devfn means auto assign */
>   static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>                                            const char *name, int devfn,
> @@ -984,14 +993,20 @@ 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 (pci_bus_devfn_available(bus, devfn)) {
> +            if (pci_bus_devfn_available(bus, devfn) &&
> +                   !pci_bus_devfn_reserved(bus, devfn)) {
>                   goto found;
>               }
>           }
> -        error_setg(errp, "PCI: no slot/function available for %s, all in use",
> -                   name);
> +        error_setg(errp, "PCI: no slot/function available for %s, all in use "
> +                   "or reserved", name);

I wouldn't combine slot available/reserved, maybe check for reserved
before "available"?

>           return NULL;
>       found: ;
> +    } else if (pci_bus_devfn_reserved(bus, devfn)) {
> +        error_setg(errp, "PCI: slot %d function %d not available for %s,"
> +                   " reserved",
> +                   PCI_SLOT(devfn), PCI_FUNC(devfn), name);
> +        return NULL;
>       } else if (!pci_bus_devfn_available(bus, devfn)) {
>           error_setg(errp, "PCI: slot %d function %d not available for %s,"
>                      " in use by %s",

Thanks for the patches!
Marcel