diff mbox series

[v4,28/47] hw/arm/npcm7xx: use qemu_configure_nic_device, allow emc0/emc1 as aliases

Message ID 20240126173228.394202-29-dwmw2@infradead.org
State New
Headers show
Series Rework matching of network devices to -nic options | expand

Commit Message

David Woodhouse Jan. 26, 2024, 5:25 p.m. UTC 
From: David Woodhouse <dwmw@amazon.co.uk>

Also update the test to specify which device to attach the test socket
to, and remove the comment lamenting the fact that we can't do so.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 hw/arm/npcm7xx.c               | 16 +++++++++-------
 tests/qtest/npcm7xx_emc-test.c | 18 ++++--------------
 2 files changed, 13 insertions(+), 21 deletions(-)

Comments

Thomas Huth Jan. 31, 2024, 7:08 a.m. UTC | #1 
On 26/01/2024 18.25, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> Also update the test to specify which device to attach the test socket
> to, and remove the comment lamenting the fact that we can't do so.
> 
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>   hw/arm/npcm7xx.c               | 16 +++++++++-------
>   tests/qtest/npcm7xx_emc-test.c | 18 ++++--------------
>   2 files changed, 13 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
> index 15ff21d047..ee395864e4 100644
> --- a/hw/arm/npcm7xx.c
> +++ b/hw/arm/npcm7xx.c
> @@ -655,8 +655,9 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
>   
>       /*
>        * EMC Modules. Cannot fail.
> -     * The mapping of the device to its netdev backend works as follows:
> -     * emc[i] = nd_table[i]
> +     * Use the available NIC configurations in order, allowing 'emc0' and
> +     * 'emc1' to by used as aliases for the model= parameter to override.
> +     *
>        * This works around the inability to specify the netdev property for the
>        * emc device: it's not pluggable and thus the -device option can't be
>        * used.
> @@ -664,12 +665,13 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
>       QEMU_BUILD_BUG_ON(ARRAY_SIZE(npcm7xx_emc_addr) != ARRAY_SIZE(s->emc));
>       QEMU_BUILD_BUG_ON(ARRAY_SIZE(s->emc) != 2);
>       for (i = 0; i < ARRAY_SIZE(s->emc); i++) {
> -        s->emc[i].emc_num = i;
>           SysBusDevice *sbd = SYS_BUS_DEVICE(&s->emc[i]);
> -        if (nd_table[i].used) {
> -            qemu_check_nic_model(&nd_table[i], TYPE_NPCM7XX_EMC);
> -            qdev_set_nic_properties(DEVICE(sbd), &nd_table[i]);
> -        }
> +        char alias[6];
> +
> +        s->emc[i].emc_num = i;
> +        snprintf(alias, sizeof(alias), "emc%u", i);
> +        qemu_configure_nic_device(DEVICE(sbd), true, alias);
> +
>           /*
>            * The device exists regardless of whether it's connected to a QEMU
>            * netdev backend. So always instantiate it even if there is no
> diff --git a/tests/qtest/npcm7xx_emc-test.c b/tests/qtest/npcm7xx_emc-test.c
> index b046f1d76a..f7646fae2c 100644
> --- a/tests/qtest/npcm7xx_emc-test.c
> +++ b/tests/qtest/npcm7xx_emc-test.c
> @@ -225,21 +225,11 @@ static int *packet_test_init(int module_num, GString *cmd_line)
>       g_assert_cmpint(ret, != , -1);
>   
>       /*
> -     * KISS and use -nic. We specify two nics (both emc{0,1}) because there's
> -     * currently no way to specify only emc1: The driver implicitly relies on
> -     * emc[i] == nd_table[i].
> +     * KISS and use -nic. The driver accepts 'emc0' and 'emc1' as aliases
> +     * in the 'model' field to specify the device to match.
>        */
> -    if (module_num == 0) {
> -        g_string_append_printf(cmd_line,
> -                               " -nic socket,fd=%d,model=" TYPE_NPCM7XX_EMC " "
> -                               " -nic user,model=" TYPE_NPCM7XX_EMC " ",
> -                               test_sockets[1]);
> -    } else {
> -        g_string_append_printf(cmd_line,
> -                               " -nic user,model=" TYPE_NPCM7XX_EMC " "
> -                               " -nic socket,fd=%d,model=" TYPE_NPCM7XX_EMC " ",
> -                               test_sockets[1]);
> -    }
> +    g_string_append_printf(cmd_line, " -nic socket,fd=%d,model=emc%d ",
> +                           test_sockets[1], module_num);
>   
>       g_test_queue_destroy(packet_test_clear, test_sockets);
>       return test_sockets;

I like the idea to use the alias to configure a certain on-board NIC :-)

Reviewed-by: Thomas Huth <thuth@redhat.com>
diff mbox series

Patch

diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
index 15ff21d047..ee395864e4 100644
--- a/hw/arm/npcm7xx.c
+++ b/hw/arm/npcm7xx.c
@@ -655,8 +655,9 @@  static void npcm7xx_realize(DeviceState *dev, Error **errp)
 
     /*
      * EMC Modules. Cannot fail.
-     * The mapping of the device to its netdev backend works as follows:
-     * emc[i] = nd_table[i]
+     * Use the available NIC configurations in order, allowing 'emc0' and
+     * 'emc1' to by used as aliases for the model= parameter to override.
+     *
      * This works around the inability to specify the netdev property for the
      * emc device: it's not pluggable and thus the -device option can't be
      * used.
@@ -664,12 +665,13 @@  static void npcm7xx_realize(DeviceState *dev, Error **errp)
     QEMU_BUILD_BUG_ON(ARRAY_SIZE(npcm7xx_emc_addr) != ARRAY_SIZE(s->emc));
     QEMU_BUILD_BUG_ON(ARRAY_SIZE(s->emc) != 2);
     for (i = 0; i < ARRAY_SIZE(s->emc); i++) {
-        s->emc[i].emc_num = i;
         SysBusDevice *sbd = SYS_BUS_DEVICE(&s->emc[i]);
-        if (nd_table[i].used) {
-            qemu_check_nic_model(&nd_table[i], TYPE_NPCM7XX_EMC);
-            qdev_set_nic_properties(DEVICE(sbd), &nd_table[i]);
-        }
+        char alias[6];
+
+        s->emc[i].emc_num = i;
+        snprintf(alias, sizeof(alias), "emc%u", i);
+        qemu_configure_nic_device(DEVICE(sbd), true, alias);
+
         /*
          * The device exists regardless of whether it's connected to a QEMU
          * netdev backend. So always instantiate it even if there is no
diff --git a/tests/qtest/npcm7xx_emc-test.c b/tests/qtest/npcm7xx_emc-test.c
index b046f1d76a..f7646fae2c 100644
--- a/tests/qtest/npcm7xx_emc-test.c
+++ b/tests/qtest/npcm7xx_emc-test.c
@@ -225,21 +225,11 @@  static int *packet_test_init(int module_num, GString *cmd_line)
     g_assert_cmpint(ret, != , -1);
 
     /*
-     * KISS and use -nic. We specify two nics (both emc{0,1}) because there's
-     * currently no way to specify only emc1: The driver implicitly relies on
-     * emc[i] == nd_table[i].
+     * KISS and use -nic. The driver accepts 'emc0' and 'emc1' as aliases
+     * in the 'model' field to specify the device to match.
      */
-    if (module_num == 0) {
-        g_string_append_printf(cmd_line,
-                               " -nic socket,fd=%d,model=" TYPE_NPCM7XX_EMC " "
-                               " -nic user,model=" TYPE_NPCM7XX_EMC " ",
-                               test_sockets[1]);
-    } else {
-        g_string_append_printf(cmd_line,
-                               " -nic user,model=" TYPE_NPCM7XX_EMC " "
-                               " -nic socket,fd=%d,model=" TYPE_NPCM7XX_EMC " ",
-                               test_sockets[1]);
-    }
+    g_string_append_printf(cmd_line, " -nic socket,fd=%d,model=emc%d ",
+                           test_sockets[1], module_num);
 
     g_test_queue_destroy(packet_test_clear, test_sockets);
     return test_sockets;