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

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 1908 bytes --]

On Fri, 2023-11-10 at 08:31 +0100, Philippe Mathieu-Daudé wrote:
> 
> > +    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.

Hm, not sure. Mostly, I'm trying not to change existing behaviour with
this series (except in carefully noted cases where the minutiæ of the
existing behaviour appear to be both unintended and unimportant, and it
would be unnecessarily complex to preserve the gratuitous differences
between the way that platforms have open-coded things).

I don't think it makes much sense *even* to use the new
qemu_configure_nic_device() with hotplug devices. The user might create
a *netdev* at startup, for later hotplug devices to use. But they
wouldn't use `-nic` for that, and any devices explicitly added through
hotplug will have an explicitly specified netdev, won't they?

I don't think we want to change that model and allow hotplug devices to
magically get config from -nic on the command line... do we?

We even have a warning for the case where NIC configurations are
provided with -nic but aren't consumed by the time the platform is
instantiated (although that doesn't *prevent* them from being used
later by hotplug).

But that's answering your question which was about "these functions".

For *this* particular function, pci_init_nic_in_slot(), it makes even
less sense to consider hotplug. This is for platforms to handle the
special cases, like "this board had an RTL8139 in slot 3", and make
that NIC appear in the appropriate slot. It's done as a special case
before processing the rest of the NICs which land in dynamically
assigned slots — which may even be on a *different* bus, in the case of
sun4u.


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]