Re: [PATCH for-8.3 v2 04/46] hw/pci: add pci_init_nic_devices(), pci_init_nic_in_slot()

[Philippe Mathieu-Daudé]

Hi David,

On 6/11/23 20:49, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> The loop over nd_table[] to add PCI NICs is repeated in quite a few
> places. Add a helper function to do it.
>
> Some platforms also try to instantiate a specific model in a specific
> slot, to match the real hardware. Add pci_init_nic_in_slot() for that
> purpose.
>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> Reviewed-by: Paul Durrant <paul@xen.org>
> ---
>   hw/pci/pci.c         | 45 ++++++++++++++++++++++++++++++++++++++++++++
>   include/hw/pci/pci.h |  4 +++-
>   2 files changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 885c04b6f5..5703266c0b 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1925,6 +1925,51 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus 
> *rootbus,
>       return pci_dev;
>   }
>   
> +void pci_init_nic_devices(PCIBus *bus, const char *default_model)
> +{
> +    qemu_create_nic_bus_devices(&bus->qbus, TYPE_PCI_DEVICE, default_model,
> +                                "virtio", "virtio-net-pci");
> +}
> +
> +bool pci_init_nic_in_slot(PCIBus *rootbus, const char *model,
> +                          const char *alias, const char *devaddr)
> +{
> +    NICInfo *nd = qemu_find_nic_info(model, true, alias);
> +    int dom, busnr, devfn;
> +    PCIDevice *pci_dev;
> +    unsigned slot;
> +    PCIBus *bus;
> +
> +    if (!nd) {
> +        return false;
> +    }
> +
> +    if (!devaddr || pci_parse_devaddr(devaddr, &dom, &busnr, &slot, NULL) < 0) 
> {
> +        error_report("Invalid PCI device address %s for device %s",
> +                     devaddr, model);
> +        exit(1);
> +    }
> +
> +    if (dom != 0) {
> +        error_report("No support for non-zero PCI domains");
> +        exit(1);
> +    }
> +
> +    devfn = PCI_DEVFN(slot, 0);
> +
> +    bus = pci_find_bus_nr(rootbus, busnr);
> +    if (!bus) {
> +        error_report("Invalid PCI device address %s for device %s",
> +                     devaddr, model);
> +        exit(1);
> +    }
> +
> +    pci_dev = pci_new(devfn, model);
> +    qdev_set_nic_properties(&pci_dev->qdev, nd);
> +    pci_realize_and_unref(pci_dev, bus, &error_fatal);

Could these functions be used with hotplug devices?

If so we should propagate the error, not make it fatal.

> +    return true;
> +}
> +
>   PCIDevice *pci_vga_init(PCIBus *bus)
>   {
>       vga_interface_created = true;
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index ea5aff118b..684d49bdcd 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -317,7 +317,9 @@ void pci_device_reset(PCIDevice *dev);
>   PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
>                                  const char *default_model,
>                                  const char *default_devaddr);
> -
> +void pci_init_nic_devices(PCIBus *bus, const char *default_model);
> +bool pci_init_nic_in_slot(PCIBus *rootbus, const char *default_model,
> +                          const char *alias, const char *devaddr);
>   PCIDevice *pci_vga_init(PCIBus *bus);
>   
>   static inline PCIBus *pci_get_bus(const PCIDevice *dev)