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

[Marcel Apfelbaum <marcel@redhat.com>]

On 16/07/2017 23:27, Mark Cave-Ayland wrote:
> Add a new slot_reserved_mask bitmask to PCIBus indicating whether or not each
> PCI slot on the bus is reserved. Ensure that it is initialised to zero to
> maintain the existing behaviour that all slots are available by default, and
> add the additional check with appropriate error reporting to
> do_pci_register_device().
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/pci/pci.c             |   18 +++++++++++++++---
>   include/hw/pci/pci_bus.h |    1 +
>   2 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index efc9c86..41591dc 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -371,6 +371,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
>   {
>       assert(PCI_FUNC(devfn_min) == 0);
>       bus->devfn_min = devfn_min;
> +    bus->slot_reserved_mask = 0x0;
>       bus->address_space_mem = address_space_mem;
>       bus->address_space_io = address_space_io;
>   
> @@ -956,6 +957,11 @@ static bool pci_bus_devfn_available(PCIBus *bus, int devfn)
>       return !(bus->devices[devfn]);
>   }
>   
> +static bool pci_bus_devfn_reserved(PCIBus *bus, int devfn)
> +{
> +    return bus->slot_reserved_mask & (1UL << PCI_SLOT(devfn));
> +}
> +
>   /* -1 for devfn means auto assign */
>   static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>                                            const char *name, int devfn,
> @@ -979,14 +985,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);
>           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",
> diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
> index 5484a9b..bc34fd0 100644
> --- a/include/hw/pci/pci_bus.h
> +++ b/include/hw/pci/pci_bus.h
> @@ -23,6 +23,7 @@ struct PCIBus {
>       PCIIOMMUFunc iommu_fn;
>       void *iommu_opaque;
>       uint8_t devfn_min;
> +    uint32_t slot_reserved_mask;
>       pci_set_irq_fn set_irq;
>       pci_map_irq_fn map_irq;
>       pci_route_irq_fn route_intx_to_irq;
> 

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>

Thanks,
Marcel