Re: [PATCH for-8.3 v2 05/46] hw/i386/pc: use qemu_get_nic_info() and pci_init_nic_devices()

[Philippe Mathieu-Daudé]

Hi David,

+Markus/Bernhard

On 6/11/23 20:49, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> Eliminate direct access to nd_table[] and nb_nics by processing the the
> Xen and ISA NICs first and then calling pci_init_nic_devices() for the
> rest.
>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> Reviewed-by: Paul Durrant <paul@xen.org>
> ---
>   hw/i386/pc.c                | 26 ++++++++++++++++----------
>   include/hw/net/ne2000-isa.h |  2 --
>   2 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index c2bc3fa52d..4078d2d231 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -652,8 +652,11 @@ static void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd)
>   {
>       static int nb_ne2k = 0;
>   
> -    if (nb_ne2k == NE2000_NB_MAX)
> +    if (nb_ne2k == NE2000_NB_MAX) {
> +        error_setg(&error_fatal,

In the context of dynamically created machines I'd rather have
this function,

> +                   "maximum number of ISA NE2000 devices exceeded");
>           return;
> +    }
>       isa_ne2000_init(bus, ne2000_io[nb_ne2k],
>                       ne2000_irq[nb_ne2k], nd);
>       nb_ne2k++;
> @@ -1291,23 +1294,26 @@ void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, 
> PCIBus *pci_bus,
>                    BusState *xen_bus)
>   {
>       MachineClass *mc = MACHINE_CLASS(pcmc);
> -    int i;
> +    bool default_is_ne2k = g_str_equal(mc->default_nic, TYPE_ISA_NE2000);
> +    NICInfo *nd;
>   
>       rom_set_order_override(FW_CFG_ORDER_OVERRIDE_NIC);
> -    for (i = 0; i < nb_nics; i++) {
> -        NICInfo *nd = &nd_table[i];
> -        const char *model = nd->model ? nd->model : mc->default_nic;
>   
> -        if (xen_bus && (!nd->model || g_str_equal(model, "xen-net-device"))) {
> +    if (xen_bus) {
> +        while (nc = qemu_find_nic_info("xen-net-device", true, NULL)) {
>               DeviceState *dev = qdev_new("xen-net-device");
>               qdev_set_nic_properties(dev, nd);
>               qdev_realize_and_unref(dev, xen_bus, &error_fatal);

and this one non-fatal (primarily for API example). But this is pending
on a discussion on another thread, see:
c1322f3b-2ae2-4ca7-9a76-a2a434dc8315@linaro.org/">https://lore.kernel.org/qemu-devel/c1322f3b-2ae2-4ca7-9a76-a2a434dc8315@linaro.org/
so no changed requested so far.

> -        } else if (g_str_equal(model, "ne2k_isa")) {
> -            pc_init_ne2k_isa(isa_bus, nd);
> -        } else {
> -            pci_nic_init_nofail(nd, pci_bus, model, NULL);
>           }
>       }

Regards,

Phil.