1. Patchew
  2. QEMU
  3. [PATCH v4 00/47] Rework matching of network devices to -nic options
  4. View patch

[PATCH v4 33/47] hw/m68k/q800: use qemu_find_nic_info()

David Woodhouse posted 47 patches 10 months ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Beniamino Galvani <b.galvani@gmail.com>, Peter Maydell <peter.maydell@linaro.org>, Strahinja Jankovic <strahinja.p.jankovic@gmail.com>, Niek Linnenbank <nieklinnenbank@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel Stanley <joel@jms.id.au>, Igor Mitsyanko <i.mitsyanko@gmail.com>, Jean-Christophe Dubois <jcd@tribudubois.net>, Andrey Smirnov <andrew.smirnov@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Rob Herring <robh@kernel.org>, Subbaraya Sundeep <sundeep.lkml@gmail.com>, Jan Kiszka <jan.kiszka@web.de>, Tyrone Ting <kfting@nuvoton.com>, Hao Wu <wuhaotsh@google.com>, Radoslaw Biernacki <rad@semihalf.com>, Leif Lindholm <quic_llindhol@quicinc.com>, Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Alistair Francis <alistair@alistair23.me>, Helge Deller <deller@gmx.de>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Song Gao <gaosong@loongson.cn>, Thomas Huth <huth@tuxfamily.org>, Laurent Vivier <laurent@vivier.eu>, Huacai Chen <chenhuacai@kernel.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, "Hervé Poussineau" <hpoussin@reactos.org>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Aurelien Jarno <aurelien@aurel32.net>, Jason Wang <jasowang@redhat.com>, Jia Liu <proljc@gmail.com>, Stafford Horne <shorne@gmail.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Bin Meng <bin.meng@windriver.com>, Palmer Dabbelt <palmer@dabbelt.com>, Weiwei Li <liwei1518@gmail.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Yoshinori Sato <ysato@users.sourceforge.jp>, Magnus Damm <magnus.damm@gmail.com>, Artyom Tarasenko <atar4qemu@gmail.com>, Stefano Stabellini <sstabellini@kernel.org>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul@xen.org>, Max Filippov <jcmvbkbc@gmail.com>
[PATCH v4 33/47] hw/m68k/q800: use qemu_find_nic_info()
Posted by David Woodhouse 10 months ago 
From: David Woodhouse <dwmw@amazon.co.uk>

If a corresponding NIC configuration was found, it will have a MAC address
already assigned, so use that. Else, generate and assign a default one.

Using qemu_find_nic_info() is simpler than the alternative of using
qemu_configure_nic_device() and then having to fetch the "mac" property
as a string and convert it.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 hw/m68k/q800.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index b80a3b6d5f..fa7683bf76 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -48,6 +48,7 @@
 #include "hw/display/macfb.h"
 #include "hw/block/swim.h"
 #include "net/net.h"
+#include "net/util.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "sysemu/qtest.h"
@@ -270,6 +271,8 @@ static void q800_machine_init(MachineState *machine)
     BusState *adb_bus;
     NubusBus *nubus;
     DriveInfo *dinfo;
+    NICInfo *nd;
+    MACAddr mac;
     uint8_t rng_seed[32];
 
     linux_boot = (kernel_filename != NULL);
@@ -370,13 +373,6 @@ static void q800_machine_init(MachineState *machine)
 
     /* MACSONIC */
 
-    if (nb_nics > 1) {
-        error_report("q800 can only have one ethernet interface");
-        exit(1);
-    }
-
-    qemu_check_nic_model(&nd_table[0], "dp83932");
-
     /*
      * MacSonic driver needs an Apple MAC address
      * Valid prefix are:
@@ -386,14 +382,21 @@ static void q800_machine_init(MachineState *machine)
      * 08:00:07 Apple
      * (Q800 use the last one)
      */
-    nd_table[0].macaddr.a[0] = 0x08;
-    nd_table[0].macaddr.a[1] = 0x00;
-    nd_table[0].macaddr.a[2] = 0x07;
-
     object_initialize_child(OBJECT(machine), "dp8393x", &m->dp8393x,
                             TYPE_DP8393X);
     dev = DEVICE(&m->dp8393x);
-    qdev_set_nic_properties(dev, &nd_table[0]);
+    nd = qemu_find_nic_info(TYPE_DP8393X, true, "dp83932");
+    if (nd) {
+        qdev_set_nic_properties(dev, nd);
+        memcpy(mac.a, nd->macaddr.a, sizeof(mac.a));
+    } else {
+        qemu_macaddr_default_if_unset(&mac);
+    }
+    mac.a[0] = 0x08;
+    mac.a[1] = 0x00;
+    mac.a[2] = 0x07;
+    qdev_prop_set_macaddr(dev, "mac", mac.a);
+
     qdev_prop_set_uint8(dev, "it_shift", 2);
     qdev_prop_set_bit(dev, "big_endian", true);
     object_property_set_link(OBJECT(dev), "dma_mr",
@@ -414,7 +417,7 @@ static void q800_machine_init(MachineState *machine)
     prom = memory_region_get_ram_ptr(&m->dp8393x_prom);
     checksum = 0;
     for (i = 0; i < 6; i++) {
-        prom[i] = revbit8(nd_table[0].macaddr.a[i]);
+        prom[i] = revbit8(mac.a[i]);
         checksum ^= prom[i];
     }
     prom[7] = 0xff - checksum;
-- 
2.43.0
Re: [PATCH v4 33/47] hw/m68k/q800: use qemu_find_nic_info()
Posted by Thomas Huth 10 months ago 
On 26/01/2024 18.25, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> If a corresponding NIC configuration was found, it will have a MAC address
> already assigned, so use that. Else, generate and assign a default one.
> 
> Using qemu_find_nic_info() is simpler than the alternative of using
> qemu_configure_nic_device() and then having to fetch the "mac" property
> as a string and convert it.
> 
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>   hw/m68k/q800.c | 29 ++++++++++++++++-------------
>   1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index b80a3b6d5f..fa7683bf76 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -48,6 +48,7 @@
>   #include "hw/display/macfb.h"
>   #include "hw/block/swim.h"
>   #include "net/net.h"
> +#include "net/util.h"
>   #include "qapi/error.h"
>   #include "qemu/error-report.h"
>   #include "sysemu/qtest.h"
> @@ -270,6 +271,8 @@ static void q800_machine_init(MachineState *machine)
>       BusState *adb_bus;
>       NubusBus *nubus;
>       DriveInfo *dinfo;
> +    NICInfo *nd;
> +    MACAddr mac;
>       uint8_t rng_seed[32];
>   
>       linux_boot = (kernel_filename != NULL);
> @@ -370,13 +373,6 @@ static void q800_machine_init(MachineState *machine)
>   
>       /* MACSONIC */
>   
> -    if (nb_nics > 1) {
> -        error_report("q800 can only have one ethernet interface");
> -        exit(1);
> -    }
> -
> -    qemu_check_nic_model(&nd_table[0], "dp83932");
> -
>       /*
>        * MacSonic driver needs an Apple MAC address
>        * Valid prefix are:
> @@ -386,14 +382,21 @@ static void q800_machine_init(MachineState *machine)
>        * 08:00:07 Apple
>        * (Q800 use the last one)
>        */
> -    nd_table[0].macaddr.a[0] = 0x08;
> -    nd_table[0].macaddr.a[1] = 0x00;
> -    nd_table[0].macaddr.a[2] = 0x07;
> -
>       object_initialize_child(OBJECT(machine), "dp8393x", &m->dp8393x,
>                               TYPE_DP8393X);
>       dev = DEVICE(&m->dp8393x);
> -    qdev_set_nic_properties(dev, &nd_table[0]);
> +    nd = qemu_find_nic_info(TYPE_DP8393X, true, "dp83932");
> +    if (nd) {
> +        qdev_set_nic_properties(dev, nd);
> +        memcpy(mac.a, nd->macaddr.a, sizeof(mac.a));
> +    } else {
> +        qemu_macaddr_default_if_unset(&mac);
> +    }
> +    mac.a[0] = 0x08;
> +    mac.a[1] = 0x00;
> +    mac.a[2] = 0x07;

Don't we have to change nd->macaddr.a[0 to 2] with this hard-coded 
MAC-prefix, too?

  Thomas

> +    qdev_prop_set_macaddr(dev, "mac", mac.a);
> +
>       qdev_prop_set_uint8(dev, "it_shift", 2);
>       qdev_prop_set_bit(dev, "big_endian", true);
>       object_property_set_link(OBJECT(dev), "dma_mr",
> @@ -414,7 +417,7 @@ static void q800_machine_init(MachineState *machine)
>       prom = memory_region_get_ram_ptr(&m->dp8393x_prom);
>       checksum = 0;
>       for (i = 0; i < 6; i++) {
> -        prom[i] = revbit8(nd_table[0].macaddr.a[i]);
> +        prom[i] = revbit8(mac.a[i]);
>           checksum ^= prom[i];
>       }
>       prom[7] = 0xff - checksum;
Re: [PATCH v4 33/47] hw/m68k/q800: use qemu_find_nic_info()
Posted by David Woodhouse 10 months ago 
On Wed, 2024-01-31 at 13:18 +0100, Thomas Huth wrote:
> 
> > @@ -386,14 +382,21 @@ static void q800_machine_init(MachineState
> > *machine)
> >         * 08:00:07 Apple
> >         * (Q800 use the last one)
> >         */
> > -    nd_table[0].macaddr.a[0] = 0x08;
> > -    nd_table[0].macaddr.a[1] = 0x00;
> > -    nd_table[0].macaddr.a[2] = 0x07;
> > -
> >        object_initialize_child(OBJECT(machine), "dp8393x", &m-
> > >dp8393x,
> >                                TYPE_DP8393X);
> >        dev = DEVICE(&m->dp8393x);
> > -    qdev_set_nic_properties(dev, &nd_table[0]);
> > +    nd = qemu_find_nic_info(TYPE_DP8393X, true, "dp83932");
> > +    if (nd) {
> > +        qdev_set_nic_properties(dev, nd);
> > +        memcpy(mac.a, nd->macaddr.a, sizeof(mac.a));
> > +    } else {
> > +        qemu_macaddr_default_if_unset(&mac);
> > +    }
> > +    mac.a[0] = 0x08;
> > +    mac.a[1] = 0x00;
> > +    mac.a[2] = 0x07;
> 
> Don't we have to change nd->macaddr.a[0 to 2] with this hard-coded 
> MAC-prefix, too?

I don't think so.

We either get the MAC address from 'nd' if that exists, or generate a
new MAC address with qemu_macaddr_default_if_unset().

Then we override the OUI in the actual device. We don't care about 'nd'
any more at that point.
Re: [PATCH v4 33/47] hw/m68k/q800: use qemu_find_nic_info()
Posted by Thomas Huth 9 months, 4 weeks ago 
On 31/01/2024 15.18, David Woodhouse wrote:
> On Wed, 2024-01-31 at 13:18 +0100, Thomas Huth wrote:
>>
>>> @@ -386,14 +382,21 @@ static void q800_machine_init(MachineState
>>> *machine)
>>>          * 08:00:07 Apple
>>>          * (Q800 use the last one)
>>>          */
>>> -    nd_table[0].macaddr.a[0] = 0x08;
>>> -    nd_table[0].macaddr.a[1] = 0x00;
>>> -    nd_table[0].macaddr.a[2] = 0x07;
>>> -
>>>         object_initialize_child(OBJECT(machine), "dp8393x", &m-
>>>> dp8393x,
>>>                                 TYPE_DP8393X);
>>>         dev = DEVICE(&m->dp8393x);
>>> -    qdev_set_nic_properties(dev, &nd_table[0]);
>>> +    nd = qemu_find_nic_info(TYPE_DP8393X, true, "dp83932");
>>> +    if (nd) {
>>> +        qdev_set_nic_properties(dev, nd);
>>> +        memcpy(mac.a, nd->macaddr.a, sizeof(mac.a));
>>> +    } else {
>>> +        qemu_macaddr_default_if_unset(&mac);
>>> +    }
>>> +    mac.a[0] = 0x08;
>>> +    mac.a[1] = 0x00;
>>> +    mac.a[2] = 0x07;
>>
>> Don't we have to change nd->macaddr.a[0 to 2] with this hard-coded
>> MAC-prefix, too?
> 
> I don't think so.
> 
> We either get the MAC address from 'nd' if that exists, or generate a
> new MAC address with qemu_macaddr_default_if_unset().
> 
> Then we override the OUI in the actual device. We don't care about 'nd'
> any more at that point.

I just double-checked, and yes, you're right, so:

Reviewed-by: Thomas Huth <thuth@redhat.com>
Re: [PATCH v4 33/47] hw/m68k/q800: use qemu_find_nic_info()
Posted by David Woodhouse 9 months, 4 weeks ago 
On Thu, 2024-02-01 at 11:30 +0100, Thomas Huth wrote:
> On 31/01/2024 15.18, David Woodhouse wrote:
> > On Wed, 2024-01-31 at 13:18 +0100, Thomas Huth wrote:
> > > Don't we have to change nd->macaddr.a[0 to 2] with this hard-coded
> > > MAC-prefix, too?
> > 
> > I don't think so.
> > 
> > We either get the MAC address from 'nd' if that exists, or generate a
> > new MAC address with qemu_macaddr_default_if_unset().
> > 
> > Then we override the OUI in the actual device. We don't care about 'nd'
> > any more at that point.
> 
> I just double-checked, and yes, you're right, so:
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Thank you, just just about completes my set. Every patch now has a
Reviewed-by: except two of them (hw/s390x/s390-virtio-ccw and
hw/arm/aspeed) which only have an Acked-by: 

I think according to The Rules I just need a maintainer to *tell* me to
send a pull request and then I'm allowed to do so?
Re: [PATCH v4 33/47] hw/m68k/q800: use qemu_find_nic_info()
Posted by Peter Maydell 9 months, 4 weeks ago 
On Thu, 1 Feb 2024 at 16:07, David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Thu, 2024-02-01 at 11:30 +0100, Thomas Huth wrote:
> > On 31/01/2024 15.18, David Woodhouse wrote:
> > > On Wed, 2024-01-31 at 13:18 +0100, Thomas Huth wrote:
> > > > Don't we have to change nd->macaddr.a[0 to 2] with this hard-coded
> > > > MAC-prefix, too?
> > >
> > > I don't think so.
> > >
> > > We either get the MAC address from 'nd' if that exists, or generate a
> > > new MAC address with qemu_macaddr_default_if_unset().
> > >
> > > Then we override the OUI in the actual device. We don't care about 'nd'
> > > any more at that point.
> >
> > I just double-checked, and yes, you're right, so:
> >
> > Reviewed-by: Thomas Huth <thuth@redhat.com>
>
> Thank you, just just about completes my set. Every patch now has a
> Reviewed-by: except two of them (hw/s390x/s390-virtio-ccw and
> hw/arm/aspeed) which only have an Acked-by:
>
> I think according to The Rules I just need a maintainer to *tell* me to
> send a pull request and then I'm allowed to do so?

Yeah, go ahead and send a pullreq for this.

(I have a new board model brewing so it would be handy for me
to have the new functions upstream so I can create the ethernet
device in the right way :-))

thanks
-- PMM

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.