Message ID | 20230629125449.234945-1-thuth@redhat.com |
---|---|
State | New |
Headers | show |
Series | hw: Simplify calls to pci_nic_init_nofail() | expand |
Hi Thomas, On 29/6/23 14:54, Thomas Huth wrote: > pci_nic_init_nofail() calls qemu_find_nic_model(), and this function > sets nd->model = g_strdup(default_model) if it has not been initialized > yet. So we don't have to set nd->model to the default_nic in the > calling sites. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > hw/arm/sbsa-ref.c | 8 +------- > hw/arm/virt.c | 8 +------- > hw/loongarch/virt.c | 8 +------- > hw/mips/loongson3_virt.c | 8 +------- > hw/xtensa/virt.c | 8 +------- > 5 files changed, 5 insertions(+), 35 deletions(-) > > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c > index b774d80291..d8e13ddbfe 100644 > --- a/hw/arm/sbsa-ref.c > +++ b/hw/arm/sbsa-ref.c > @@ -683,13 +683,7 @@ static void create_pcie(SBSAMachineState *sms) > pci = PCI_HOST_BRIDGE(dev); > if (pci->bus) { > for (i = 0; i < nb_nics; i++) { > - NICInfo *nd = &nd_table[i]; > - > - if (!nd->model) { > - nd->model = g_strdup(mc->default_nic); > - } > - > - pci_nic_init_nofail(nd, pci->bus, nd->model, NULL); > + pci_nic_init_nofail(&nd_table[i], pci->bus, mc->default_nic, NULL); > } > } > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 3937e30477..b660119bce 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -1477,13 +1477,7 @@ static void create_pcie(VirtMachineState *vms) > vms->bus = pci->bus; > if (vms->bus) { > for (i = 0; i < nb_nics; i++) { > - NICInfo *nd = &nd_table[i]; > - > - if (!nd->model) { > - nd->model = g_strdup(mc->default_nic); > - } > - > - pci_nic_init_nofail(nd, pci->bus, nd->model, NULL); > + pci_nic_init_nofail(&nd_table[i], pci->bus, mc->default_nic, NULL); > } > } > > diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c > index ca8824b6ef..51a453fa9a 100644 > --- a/hw/loongarch/virt.c > +++ b/hw/loongarch/virt.c > @@ -547,13 +547,7 @@ static void loongarch_devices_init(DeviceState *pch_pic, LoongArchMachineState * > > /* Network init */ > for (i = 0; i < nb_nics; i++) { > - NICInfo *nd = &nd_table[i]; > - > - if (!nd->model) { > - nd->model = g_strdup(mc->default_nic); > - } > - > - pci_nic_init_nofail(nd, pci_bus, nd->model, NULL); > + pci_nic_init_nofail(&nd_table[i], pci_bus, mc->default_nic, NULL); > } > > /* > diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c > index 216812f660..3dd91da7a6 100644 > --- a/hw/mips/loongson3_virt.c > +++ b/hw/mips/loongson3_virt.c > @@ -454,13 +454,7 @@ static inline void loongson3_virt_devices_init(MachineState *machine, > } > > for (i = 0; i < nb_nics; i++) { > - NICInfo *nd = &nd_table[i]; > - > - if (!nd->model) { > - nd->model = g_strdup(mc->default_nic); > - } > - > - pci_nic_init_nofail(nd, pci_bus, nd->model, NULL); > + pci_nic_init_nofail(&nd_table[i], pci_bus, mc->default_nic, NULL); > } > } > > diff --git a/hw/xtensa/virt.c b/hw/xtensa/virt.c > index b87f842e74..a6cf646e99 100644 > --- a/hw/xtensa/virt.c > +++ b/hw/xtensa/virt.c > @@ -103,13 +103,7 @@ static void create_pcie(MachineState *ms, CPUXtensaState *env, int irq_base, > pci = PCI_HOST_BRIDGE(dev); > if (pci->bus) { > for (i = 0; i < nb_nics; i++) { > - NICInfo *nd = &nd_table[i]; > - > - if (!nd->model) { > - nd->model = g_strdup(mc->default_nic); > - } > - > - pci_nic_init_nofail(nd, pci->bus, nd->model, NULL); > + pci_nic_init_nofail(&nd_table[i], pci->bus, mc->default_nic, NULL); > } > } > } This remind me of a branch from end of April with this unfinished patch, did we already discuss this together? -- >8 -- diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index e6d0574a29..6bb02dc64f 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -317,6 +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); +PCIDevice *pci_nic_init_default(NICInfo *nd, PCIBus *rootbus, + const char *default_model, + const char *default_devaddr); PCIDevice *pci_vga_init(PCIBus *bus); diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c index 03495e1e60..23e5c307a4 100644 --- a/hw/alpha/dp264.c +++ b/hw/alpha/dp264.c @@ -125,7 +125,7 @@ static void clipper_init(MachineState *machine) /* Network setup. e1000 is good enough, failing Tulip support. */ for (i = 0; i < nb_nics; i++) { - pci_nic_init_nofail(&nd_table[i], pci_bus, mc->default_nic, NULL); + pci_nic_init_default(&nd_table[i], pci_bus, mc->default_nic, NULL); } /* Super I/O */ diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c index 792371fdce..a59e74a81d 100644 --- a/hw/arm/sbsa-ref.c +++ b/hw/arm/sbsa-ref.c @@ -638,13 +638,7 @@ static void create_pcie(SBSAMachineState *sms) pci = PCI_HOST_BRIDGE(dev); if (pci->bus) { for (i = 0; i < nb_nics; i++) { - NICInfo *nd = &nd_table[i]; - - if (!nd->model) { - nd->model = g_strdup("e1000e"); - } - - pci_nic_init_nofail(nd, pci->bus, nd->model, NULL); + pci_nic_init_default(nd, pci->bus, "e1000e", NULL); } } diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 9b9f7d9c68..6418bd2fa9 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1477,13 +1477,7 @@ static void create_pcie(VirtMachineState *vms) vms->bus = pci->bus; if (vms->bus) { for (i = 0; i < nb_nics; i++) { - NICInfo *nd = &nd_table[i]; - - if (!nd->model) { - nd->model = g_strdup(mc->default_nic); - } - - pci_nic_init_nofail(nd, pci->bus, nd->model, NULL); + pci_nic_init_default(&nd_table[i], pci->bus, mc->default_nic, NULL); } } diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c index b00a91ecfe..e6316d76d1 100644 --- a/hw/hppa/machine.c +++ b/hw/hppa/machine.c @@ -273,7 +273,7 @@ static void machine_hppa_init(MachineState *machine) for (i = 0; i < nb_nics; i++) { if (!enable_lasi_lan()) { - pci_nic_init_nofail(&nd_table[i], pci_bus, mc->default_nic, NULL); + pci_nic_init_default(&nd_table[i], pci_bus, mc->default_nic, NULL); } } diff --git a/hw/i386/pc.c b/hw/i386/pc.c index bb62c994fa..59ad1ef9f9 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1370,12 +1370,10 @@ void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus) 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 (g_str_equal(model, "ne2k_isa")) { + if (nd->model && g_str_equal(nd->model, "ne2k_isa")) { pc_init_ne2k_isa(isa_bus, nd); } else { - pci_nic_init_nofail(nd, pci_bus, model, NULL); + pci_nic_init_default(nd, pci_bus, mc->default_nic, NULL); } } rom_reset_order_override(); diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c index ceddec1b23..1c6824921b 100644 --- a/hw/loongarch/virt.c +++ b/hw/loongarch/virt.c @@ -526,13 +526,7 @@ static void loongarch_devices_init(DeviceState *pch_pic, LoongArchMachineState * /* Network init */ for (i = 0; i < nb_nics; i++) { - NICInfo *nd = &nd_table[i]; - - if (!nd->model) { - nd->model = g_strdup(mc->default_nic); - } - - pci_nic_init_nofail(nd, pci_bus, nd->model, NULL); + pci_nic_init_default(&nd_table[i], pci_bus, mc->default_nic, NULL); } /* diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c index 216812f660..8fef3fc49c 100644 --- a/hw/mips/loongson3_virt.c +++ b/hw/mips/loongson3_virt.c @@ -454,13 +454,7 @@ static inline void loongson3_virt_devices_init(MachineState *machine, } for (i = 0; i < nb_nics; i++) { - NICInfo *nd = &nd_table[i]; - - if (!nd->model) { - nd->model = g_strdup(mc->default_nic); - } - - pci_nic_init_nofail(nd, pci_bus, nd->model, NULL); + pci_nic_init_default(&nd_table[i], pci_bus, mc->default_nic, NULL); } } diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 1cc7c89036..e5a2aaee87 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -1867,6 +1867,7 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus, unsigned slot; if (nd->model && !strcmp(nd->model, "virtio")) { + // DEPRECATE g_free(nd->model); nd->model = g_strdup("virtio-net-pci"); } @@ -1923,6 +1924,19 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus, return pci_dev; } +PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus, + const char *default_model, + const char *default_devaddr); +PCIDevice *pci_nic_init_default(NICInfo *nd, PCIBus *rootbus, + const char *default_model, + const char *default_devaddr) +{ + if (!nd->model) { + nd->model = g_strdup(default_model); + } + return pci_nic_init_nofail(nd, rootbus, nd->model, default_devaddr); +} + PCIDevice *pci_vga_init(PCIBus *bus) { vga_interface_created = true; diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index b6eb599751..a53420506b 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -1074,7 +1074,7 @@ void ppce500_init(MachineState *machine) if (pci_bus) { /* Register network interfaces. */ for (i = 0; i < nb_nics; i++) { - pci_nic_init_nofail(&nd_table[i], pci_bus, mc->default_nic, NULL); + pci_nic_init_default(&nd_table[i], pci_bus, mc->default_nic, NULL); } } diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index 535710314a..7c86e51997 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -445,7 +445,7 @@ static void ppc_core99_init(MachineState *machine) } for (i = 0; i < nb_nics; i++) { - pci_nic_init_nofail(&nd_table[i], pci_bus, mc->default_nic, NULL); + pci_nic_init_default(&nd_table[i], pci_bus, mc->default_nic, NULL); } /* The NewWorld NVRAM is not located in the MacIO device */ diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c index 510ff0eaaf..8d42e14909 100644 --- a/hw/ppc/mac_oldworld.c +++ b/hw/ppc/mac_oldworld.c @@ -277,7 +277,7 @@ static void ppc_heathrow_init(MachineState *machine) pci_vga_init(pci_bus); for (i = 0; i < nb_nics; i++) { - pci_nic_init_nofail(&nd_table[i], pci_bus, mc->default_nic, NULL); + pci_nic_init_default(&nd_table[i], pci_bus, mc->default_nic, NULL); } /* MacIO IDE */ diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c index f969fa3c29..d58a8952f6 100644 --- a/hw/ppc/ppc440_bamboo.c +++ b/hw/ppc/ppc440_bamboo.c @@ -247,7 +247,7 @@ static void bamboo_init(MachineState *machine) * There are no PCI NICs on the Bamboo board, but there are * PCI slots, so we can pick whatever default model we want. */ - pci_nic_init_nofail(&nd_table[i], pcibus, mc->default_nic, NULL); + pci_nic_init_default(&nd_table[i], pcibus, mc->default_nic, NULL); } } diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c index 4610abddbd..e50b3b6230 100644 --- a/hw/ppc/prep.c +++ b/hw/ppc/prep.c @@ -324,8 +324,8 @@ static void ibm_40p_init(MachineState *machine) pci_vga_init(pci_bus); for (i = 0; i < nb_nics; i++) { - pci_nic_init_nofail(&nd_table[i], pci_bus, mc->default_nic, - i == 0 ? "3" : NULL); + pci_nic_init_default(&nd_table[i], pci_bus, mc->default_nic, + i == 0 ? "3" : NULL); } } diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c index 4944994e9c..0b82dc0032 100644 --- a/hw/sh4/r2d.c +++ b/hw/sh4/r2d.c @@ -309,9 +309,10 @@ static void r2d_init(MachineState *machine) 0x555, 0x2aa, 0); /* NIC: rtl8139 on-board, and 2 slots. */ - for (i = 0; i < nb_nics; i++) - pci_nic_init_nofail(&nd_table[i], pci_bus, - mc->default_nic, i == 0 ? "2" : NULL); + for (i = 0; i < nb_nics; i++) { + pci_nic_init_default(&nd_table[i], pci_bus, + mc->default_nic, i == 0 ? "2" : NULL); + } /* USB keyboard */ usb_create_simple(usb_bus_find(-1), "usb-kbd"); diff --git a/hw/xtensa/virt.c b/hw/xtensa/virt.c index b87f842e74..39e63b57cf 100644 --- a/hw/xtensa/virt.c +++ b/hw/xtensa/virt.c @@ -103,13 +103,7 @@ static void create_pcie(MachineState *ms, CPUXtensaState *env, int irq_base, pci = PCI_HOST_BRIDGE(dev); if (pci->bus) { for (i = 0; i < nb_nics; i++) { - NICInfo *nd = &nd_table[i]; - - if (!nd->model) { - nd->model = g_strdup(mc->default_nic); - } - - pci_nic_init_nofail(nd, pci->bus, nd->model, NULL); + pci_nic_init_default(&nd_table[i], pci->bus, mc->default_nic, NULL); } } } ---
On 29/06/2023 15.47, Philippe Mathieu-Daudé wrote: > Hi Thomas, > > On 29/6/23 14:54, Thomas Huth wrote: >> pci_nic_init_nofail() calls qemu_find_nic_model(), and this function >> sets nd->model = g_strdup(default_model) if it has not been initialized >> yet. So we don't have to set nd->model to the default_nic in the >> calling sites. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> hw/arm/sbsa-ref.c | 8 +------- >> hw/arm/virt.c | 8 +------- >> hw/loongarch/virt.c | 8 +------- >> hw/mips/loongson3_virt.c | 8 +------- >> hw/xtensa/virt.c | 8 +------- >> 5 files changed, 5 insertions(+), 35 deletions(-) ... > This remind me of a branch from end of April with this > unfinished patch, did we already discuss this together? No, I haven't seen your patch yet, neither we talked about it. I came up with the idea for my patch on my own after looking at certain spots in the code. But I guess you could easily rebase your patch on top of mine in case you want to finish it ;-) Thomas
On 29/6/23 16:58, Thomas Huth wrote: > On 29/06/2023 15.47, Philippe Mathieu-Daudé wrote: >> Hi Thomas, >> >> On 29/6/23 14:54, Thomas Huth wrote: >>> pci_nic_init_nofail() calls qemu_find_nic_model(), and this function >>> sets nd->model = g_strdup(default_model) if it has not been initialized >>> yet. So we don't have to set nd->model to the default_nic in the >>> calling sites. >>> >>> Signed-off-by: Thomas Huth <thuth@redhat.com> >>> --- >>> hw/arm/sbsa-ref.c | 8 +------- >>> hw/arm/virt.c | 8 +------- >>> hw/loongarch/virt.c | 8 +------- >>> hw/mips/loongson3_virt.c | 8 +------- >>> hw/xtensa/virt.c | 8 +------- >>> 5 files changed, 5 insertions(+), 35 deletions(-) > ... >> This remind me of a branch from end of April with this >> unfinished patch, did we already discuss this together? > > No, I haven't seen your patch yet, neither we talked about it. I came up > with the idea for my patch on my own after looking at certain spots in > the code. But I guess you could easily rebase your patch on top of mine > in case you want to finish it ;-) Yeah sure, I was just wondering :) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
29.06.2023 15:54, Thomas Huth пишет: > pci_nic_init_nofail() calls qemu_find_nic_model(), and this function > sets nd->model = g_strdup(default_model) if it has not been initialized > yet. So we don't have to set nd->model to the default_nic in the > calling sites. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > hw/arm/sbsa-ref.c | 8 +------- > hw/arm/virt.c | 8 +------- > hw/loongarch/virt.c | 8 +------- > hw/mips/loongson3_virt.c | 8 +------- > hw/xtensa/virt.c | 8 +------- > 5 files changed, 5 insertions(+), 35 deletions(-) > > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c > index b774d80291..d8e13ddbfe 100644 > --- a/hw/arm/sbsa-ref.c > +++ b/hw/arm/sbsa-ref.c > @@ -683,13 +683,7 @@ static void create_pcie(SBSAMachineState *sms) > pci = PCI_HOST_BRIDGE(dev); > if (pci->bus) { > for (i = 0; i < nb_nics; i++) { > - NICInfo *nd = &nd_table[i]; > - > - if (!nd->model) { > - nd->model = g_strdup(mc->default_nic); > - } > - > - pci_nic_init_nofail(nd, pci->bus, nd->model, NULL); > + pci_nic_init_nofail(&nd_table[i], pci->bus, mc->default_nic, NULL); > } > } What a wonderful copy-spagetti it was :) Applied to my trivial-patches branch, thank you! /mjt
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c index b774d80291..d8e13ddbfe 100644 --- a/hw/arm/sbsa-ref.c +++ b/hw/arm/sbsa-ref.c @@ -683,13 +683,7 @@ static void create_pcie(SBSAMachineState *sms) pci = PCI_HOST_BRIDGE(dev); if (pci->bus) { for (i = 0; i < nb_nics; i++) { - NICInfo *nd = &nd_table[i]; - - if (!nd->model) { - nd->model = g_strdup(mc->default_nic); - } - - pci_nic_init_nofail(nd, pci->bus, nd->model, NULL); + pci_nic_init_nofail(&nd_table[i], pci->bus, mc->default_nic, NULL); } } diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 3937e30477..b660119bce 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1477,13 +1477,7 @@ static void create_pcie(VirtMachineState *vms) vms->bus = pci->bus; if (vms->bus) { for (i = 0; i < nb_nics; i++) { - NICInfo *nd = &nd_table[i]; - - if (!nd->model) { - nd->model = g_strdup(mc->default_nic); - } - - pci_nic_init_nofail(nd, pci->bus, nd->model, NULL); + pci_nic_init_nofail(&nd_table[i], pci->bus, mc->default_nic, NULL); } } diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c index ca8824b6ef..51a453fa9a 100644 --- a/hw/loongarch/virt.c +++ b/hw/loongarch/virt.c @@ -547,13 +547,7 @@ static void loongarch_devices_init(DeviceState *pch_pic, LoongArchMachineState * /* Network init */ for (i = 0; i < nb_nics; i++) { - NICInfo *nd = &nd_table[i]; - - if (!nd->model) { - nd->model = g_strdup(mc->default_nic); - } - - pci_nic_init_nofail(nd, pci_bus, nd->model, NULL); + pci_nic_init_nofail(&nd_table[i], pci_bus, mc->default_nic, NULL); } /* diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c index 216812f660..3dd91da7a6 100644 --- a/hw/mips/loongson3_virt.c +++ b/hw/mips/loongson3_virt.c @@ -454,13 +454,7 @@ static inline void loongson3_virt_devices_init(MachineState *machine, } for (i = 0; i < nb_nics; i++) { - NICInfo *nd = &nd_table[i]; - - if (!nd->model) { - nd->model = g_strdup(mc->default_nic); - } - - pci_nic_init_nofail(nd, pci_bus, nd->model, NULL); + pci_nic_init_nofail(&nd_table[i], pci_bus, mc->default_nic, NULL); } } diff --git a/hw/xtensa/virt.c b/hw/xtensa/virt.c index b87f842e74..a6cf646e99 100644 --- a/hw/xtensa/virt.c +++ b/hw/xtensa/virt.c @@ -103,13 +103,7 @@ static void create_pcie(MachineState *ms, CPUXtensaState *env, int irq_base, pci = PCI_HOST_BRIDGE(dev); if (pci->bus) { for (i = 0; i < nb_nics; i++) { - NICInfo *nd = &nd_table[i]; - - if (!nd->model) { - nd->model = g_strdup(mc->default_nic); - } - - pci_nic_init_nofail(nd, pci->bus, nd->model, NULL); + pci_nic_init_nofail(&nd_table[i], pci->bus, mc->default_nic, NULL); } } }
pci_nic_init_nofail() calls qemu_find_nic_model(), and this function sets nd->model = g_strdup(default_model) if it has not been initialized yet. So we don't have to set nd->model to the default_nic in the calling sites. Signed-off-by: Thomas Huth <thuth@redhat.com> --- hw/arm/sbsa-ref.c | 8 +------- hw/arm/virt.c | 8 +------- hw/loongarch/virt.c | 8 +------- hw/mips/loongson3_virt.c | 8 +------- hw/xtensa/virt.c | 8 +------- 5 files changed, 5 insertions(+), 35 deletions(-)