Re: [PATCH 5/5] kconfig: use "select" to enable semihosting

2024-02-01 Thread Thomas Huth

On 01/02/2024 13.28, Alex Bennée wrote:

From: Paolo Bonzini 

Just like all other dependencies, these can be expressed in Kconfig
files rather than in the default configurations.

Signed-off-by: Paolo Bonzini 
Acked-by: Alistair Francis 
Message-Id: <20240129115809.1039924-1-pbonz...@redhat.com>
Signed-off-by: Alex Bennée 
---
  configs/devices/m68k-softmmu/default.mak| 2 --
  configs/devices/mips-softmmu/common.mak | 3 ---
  configs/devices/nios2-softmmu/default.mak   | 2 --
  configs/devices/riscv32-softmmu/default.mak | 2 --
  configs/devices/riscv64-softmmu/default.mak | 2 --
  configs/devices/xtensa-softmmu/default.mak  | 2 --
  target/m68k/Kconfig | 1 +
  target/mips/Kconfig | 1 +
  target/nios2/Kconfig| 1 +
  target/riscv/Kconfig| 2 ++
  target/xtensa/Kconfig   | 1 +
  11 files changed, 6 insertions(+), 13 deletions(-)


Reviewed-by: Thomas Huth 


___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


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

2024-02-01 Thread Thomas Huth

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", 

dp8393x,

    TYPE_DP8393X);
    dev = DEVICE(>dp8393x);
-    qdev_set_nic_properties(dev, _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.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 




Re: [PATCH v4 37/47] hw/net/lasi_i82596: Re-enable build

2024-02-01 Thread Thomas Huth

On 26/01/2024 18.25, David Woodhouse wrote:

From: David Woodhouse 

When converting to the shiny build-system-du-jour, a typo prevented the
last_i82596 driver from being built. Correct the config option name to
re-enable the build. And include "sysemu/sysemu.h" so it actually builds.

Fixes: b1419fa66558 ("meson: convert hw/net")
Signed-off-by: David Woodhouse 
---
  hw/net/lasi_i82596.c | 1 +
  hw/net/meson.build   | 2 +-
  2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/net/lasi_i82596.c b/hw/net/lasi_i82596.c
index 6a3147fe2d..09e830ba5f 100644
--- a/hw/net/lasi_i82596.c
+++ b/hw/net/lasi_i82596.c
@@ -14,6 +14,7 @@
  #include "qapi/error.h"
  #include "qemu/timer.h"
  #include "hw/sysbus.h"
+#include "sysemu/sysemu.h"
  #include "net/eth.h"
  #include "hw/net/lasi_82596.h"
  #include "hw/net/i82596.h"
diff --git a/hw/net/meson.build b/hw/net/meson.build
index 9afceb0619..2b426d3d5a 100644
--- a/hw/net/meson.build
+++ b/hw/net/meson.build
@@ -33,7 +33,7 @@ system_ss.add(when: 'CONFIG_MARVELL_88W8618', if_true: 
files('mv88w8618_eth.c'))
  system_ss.add(when: 'CONFIG_CADENCE', if_true: files('cadence_gem.c'))
  system_ss.add(when: 'CONFIG_STELLARIS_ENET', if_true: 
files('stellaris_enet.c'))
  system_ss.add(when: 'CONFIG_LANCE', if_true: files('lance.c'))
-system_ss.add(when: 'CONFIG_LASI_I82596', if_true: files('lasi_i82596.c'))
+system_ss.add(when: 'CONFIG_LASI_82596', if_true: files('lasi_i82596.c'))
  system_ss.add(when: 'CONFIG_I82596_COMMON', if_true: files('i82596.c'))
  system_ss.add(when: 'CONFIG_SUNHME', if_true: files('sunhme.c'))
  system_ss.add(when: 'CONFIG_FTGMAC100', if_true: files('ftgmac100.c'))


Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2144




Re: [PATCH v4 37/47] hw/net/lasi_i82596: Re-enable build

2024-02-01 Thread Thomas Huth

On 26/01/2024 18.25, David Woodhouse wrote:

From: David Woodhouse 

When converting to the shiny build-system-du-jour, a typo prevented the
last_i82596 driver from being built. Correct the config option name to
re-enable the build. And include "sysemu/sysemu.h" so it actually builds.

Fixes: b1419fa66558 ("meson: convert hw/net")
Signed-off-by: David Woodhouse 
---
  hw/net/lasi_i82596.c | 1 +
  hw/net/meson.build   | 2 +-
  2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/net/lasi_i82596.c b/hw/net/lasi_i82596.c
index 6a3147fe2d..09e830ba5f 100644
--- a/hw/net/lasi_i82596.c
+++ b/hw/net/lasi_i82596.c
@@ -14,6 +14,7 @@
  #include "qapi/error.h"
  #include "qemu/timer.h"
  #include "hw/sysbus.h"
+#include "sysemu/sysemu.h"
  #include "net/eth.h"
  #include "hw/net/lasi_82596.h"
  #include "hw/net/i82596.h"
diff --git a/hw/net/meson.build b/hw/net/meson.build
index 9afceb0619..2b426d3d5a 100644
--- a/hw/net/meson.build
+++ b/hw/net/meson.build
@@ -33,7 +33,7 @@ system_ss.add(when: 'CONFIG_MARVELL_88W8618', if_true: 
files('mv88w8618_eth.c'))
  system_ss.add(when: 'CONFIG_CADENCE', if_true: files('cadence_gem.c'))
  system_ss.add(when: 'CONFIG_STELLARIS_ENET', if_true: 
files('stellaris_enet.c'))
  system_ss.add(when: 'CONFIG_LANCE', if_true: files('lance.c'))
-system_ss.add(when: 'CONFIG_LASI_I82596', if_true: files('lasi_i82596.c'))
+system_ss.add(when: 'CONFIG_LASI_82596', if_true: files('lasi_i82596.c'))
  system_ss.add(when: 'CONFIG_I82596_COMMON', if_true: files('i82596.c'))
  system_ss.add(when: 'CONFIG_SUNHME', if_true: files('sunhme.c'))
  system_ss.add(when: 'CONFIG_FTGMAC100', if_true: files('ftgmac100.c'))


Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2144




Re: [PATCH 1/4] hw/isa/vt82c686: Consolidate the use of device_class_set_parent_realize()

2024-02-01 Thread Thomas Huth

On 01/02/2024 09.40, Zhao Liu wrote:

From: Zhao Liu 

Use device_class_set_parent_realize() to set parent realize() directly.

Signed-off-by: Zhao Liu 
---
  hw/isa/vt82c686.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index d3e0f6d01fb6..a99eae4f6333 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -328,8 +328,8 @@ static void via_superio_class_init(ObjectClass *klass, void 
*data)
  DeviceClass *dc = DEVICE_CLASS(klass);
  ISASuperIOClass *sc = ISA_SUPERIO_CLASS(klass);
  
-sc->parent_realize = dc->realize;

-dc->realize = via_superio_realize;
+device_class_set_parent_realize(dc, via_superio_realize,
+>parent_realize);
  }
  
  static const TypeInfo via_superio_info = {


Reviewed-by: Thomas Huth 




Re: [PATCH 2/4] hw/isa/pc87312: Consolidate the use of device_class_set_parent_realize()

2024-02-01 Thread Thomas Huth

On 01/02/2024 09.40, Zhao Liu wrote:

From: Zhao Liu 

Use device_class_set_parent_realize() to set parent realize() directly.

Signed-off-by: Zhao Liu 
---
  hw/isa/pc87312.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/isa/pc87312.c b/hw/isa/pc87312.c
index ee23f3e164df..64dd17b537f2 100644
--- a/hw/isa/pc87312.c
+++ b/hw/isa/pc87312.c
@@ -338,10 +338,10 @@ static void pc87312_class_init(ObjectClass *klass, void 
*data)
  DeviceClass *dc = DEVICE_CLASS(klass);
  ISASuperIOClass *sc = ISA_SUPERIO_CLASS(klass);
  
-sc->parent_realize = dc->realize;

-dc->realize = pc87312_realize;
  dc->reset = pc87312_reset;
  dc->vmsd = _pc87312;
+device_class_set_parent_realize(dc, pc87312_realize,
+>parent_realize);
  device_class_set_props(dc, pc87312_properties);
  
  sc->parallel = (ISASuperIOFuncs){


Reviewed-by: Thomas Huth 




Re: [PATCH 3/4] hw/intc/s390_flic: Consolidate the use of device_class_set_parent_realize()

2024-02-01 Thread Thomas Huth

On 01/02/2024 09.40, Zhao Liu wrote:

From: Zhao Liu 

Use device_class_set_parent_realize() to set parent realize() directly.

Signed-off-by: Zhao Liu 
---
  hw/intc/s390_flic_kvm.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index 4d5cbb2a2fb6..baaa30dcb734 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -646,9 +646,10 @@ static void kvm_s390_flic_class_init(ObjectClass *oc, void 
*data)
  {
  DeviceClass *dc = DEVICE_CLASS(oc);
  S390FLICStateClass *fsc = S390_FLIC_COMMON_CLASS(oc);
+KVMS390FLICStateClass *kfsc = KVM_S390_FLIC_CLASS(oc);
  
-KVM_S390_FLIC_CLASS(oc)->parent_realize = dc->realize;

-dc->realize = kvm_s390_flic_realize;
+device_class_set_parent_realize(dc, kvm_s390_flic_realize,
+>parent_realize);
  dc->vmsd = _s390_flic_vmstate;
  dc->reset = kvm_s390_flic_reset;
  fsc->register_io_adapter = kvm_s390_register_io_adapter;


Reviewed-by: Thomas Huth 




Re: [PATCH 4/4] hw/arm/smmuv3: Consolidate the use of device_class_set_parent_realize()

2024-02-01 Thread Thomas Huth

On 01/02/2024 09.40, Zhao Liu wrote:

From: Zhao Liu 

Use device_class_set_parent_realize() to set parent realize() directly.

Signed-off-by: Zhao Liu 
---
  hw/arm/smmuv3.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 68eeef3e1d4c..b3d8642a4990 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1857,8 +1857,8 @@ static void smmuv3_class_init(ObjectClass *klass, void 
*data)
  dc->vmsd = _smmuv3;
  resettable_class_set_parent_phases(rc, NULL, smmu_reset_hold, NULL,
 >parent_phases);
-c->parent_realize = dc->realize;
-dc->realize = smmu_realize;
+device_class_set_parent_realize(dc, smmu_realize,
+>parent_realize);
  device_class_set_props(dc, smmuv3_properties);
  }
  


Reviewed-by: Thomas Huth 




[PATCH] hw/ide: Add the possibility to disable the CompactFlash device in the build

2024-02-01 Thread Thomas Huth
For distros like downstream RHEL, it would be helpful to allow to disable
the CompactFlash device. For making this possible, we need a separate
Kconfig switch for this device, and the code should reside in a separate
file.

Signed-off-by: Thomas Huth 
---
 hw/ide/qdev-ide.h  | 41 
 hw/ide/cf.c| 58 ++
 hw/ide/qdev.c  | 51 ++--
 hw/ide/Kconfig |  4 
 hw/ide/meson.build |  1 +
 5 files changed, 106 insertions(+), 49 deletions(-)
 create mode 100644 hw/ide/qdev-ide.h
 create mode 100644 hw/ide/cf.c

diff --git a/hw/ide/qdev-ide.h b/hw/ide/qdev-ide.h
new file mode 100644
index 00..3dd977466c
--- /dev/null
+++ b/hw/ide/qdev-ide.h
@@ -0,0 +1,41 @@
+/*
+ * ide bus support for qdev.
+ *
+ * Copyright (c) 2009 Gerd Hoffmann 
+ *
+ * This code is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef QDEV_IDE_H
+#define QDEV_IDE_H
+
+#include "hw/qdev-properties.h"
+#include "hw/block/block.h"
+#include "hw/ide/internal.h"
+
+typedef struct IDEDrive {
+IDEDevice dev;
+} IDEDrive;
+
+#define DEFINE_IDE_DEV_PROPERTIES() \
+DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),\
+DEFINE_BLOCK_ERROR_PROPERTIES(IDEDrive, dev.conf),  \
+DEFINE_PROP_STRING("ver",  IDEDrive, dev.version),  \
+DEFINE_PROP_UINT64("wwn",  IDEDrive, dev.wwn, 0),   \
+DEFINE_PROP_STRING("serial",  IDEDrive, dev.serial),\
+DEFINE_PROP_STRING("model", IDEDrive, dev.model)
+
+void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp);
+
+#endif
diff --git a/hw/ide/cf.c b/hw/ide/cf.c
new file mode 100644
index 00..0b4bb57591
--- /dev/null
+++ b/hw/ide/cf.c
@@ -0,0 +1,58 @@
+/*
+ * ide CompactFlash support
+ *
+ * This code is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/ide/qdev-ide.h"
+#include "qapi/qapi-types-block.h"
+
+static void ide_cf_realize(IDEDevice *dev, Error **errp)
+{
+ide_dev_initfn(dev, IDE_CFATA, errp);
+}
+
+static Property ide_cf_properties[] = {
+DEFINE_IDE_DEV_PROPERTIES(),
+DEFINE_BLOCK_CHS_PROPERTIES(IDEDrive, dev.conf),
+DEFINE_PROP_BIOS_CHS_TRANS("bios-chs-trans",
+IDEDrive, dev.chs_trans, BIOS_ATA_TRANSLATION_AUTO),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void ide_cf_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+IDEDeviceClass *k = IDE_DEVICE_CLASS(klass);
+
+k->realize  = ide_cf_realize;
+dc->fw_name = "drive";
+dc->desc= "virtual CompactFlash card";
+device_class_set_props(dc, ide_cf_properties);
+}
+
+static const TypeInfo ide_cf_info = {
+.name  = "ide-cf",
+.parent= TYPE_IDE_DEVICE,
+.instance_size = sizeof(IDEDrive),
+.class_init= ide_cf_class_init,
+};
+
+static void ide_cf_register_type(void)
+{
+type_register_static(_cf_info);
+}
+
+type_init(ide_cf_register_type)
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 1b3b4da01d..a2f2d0ea08 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -24,12 +24,9 @@
 #include "qemu/error-report.h"
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
-#include "hw/ide/internal.h"
-#include "hw/qdev-properties.h"
-#include "hw/qdev-properties-system.h"
+#include "hw/ide/qdev-ide.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
-#include "hw/block/block.h"
 #include "sysemu/sysemu.h"
 #includ

[PATCH] hw/ide: Add the possibility to disable the CompactFlash device in the build

2024-02-01 Thread Thomas Huth
For distros like downstream RHEL, it would be helpful to allow to disable
the CompactFlash device. For making this possible, we need a separate
Kconfig switch for this device, and the code should reside in a separate
file.

Signed-off-by: Thomas Huth 
---
 hw/ide/qdev-ide.h  | 41 
 hw/ide/cf.c| 58 ++
 hw/ide/qdev.c  | 51 ++--
 hw/ide/Kconfig |  4 
 hw/ide/meson.build |  1 +
 5 files changed, 106 insertions(+), 49 deletions(-)
 create mode 100644 hw/ide/qdev-ide.h
 create mode 100644 hw/ide/cf.c

diff --git a/hw/ide/qdev-ide.h b/hw/ide/qdev-ide.h
new file mode 100644
index 00..3dd977466c
--- /dev/null
+++ b/hw/ide/qdev-ide.h
@@ -0,0 +1,41 @@
+/*
+ * ide bus support for qdev.
+ *
+ * Copyright (c) 2009 Gerd Hoffmann 
+ *
+ * This code is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef QDEV_IDE_H
+#define QDEV_IDE_H
+
+#include "hw/qdev-properties.h"
+#include "hw/block/block.h"
+#include "hw/ide/internal.h"
+
+typedef struct IDEDrive {
+IDEDevice dev;
+} IDEDrive;
+
+#define DEFINE_IDE_DEV_PROPERTIES() \
+DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),\
+DEFINE_BLOCK_ERROR_PROPERTIES(IDEDrive, dev.conf),  \
+DEFINE_PROP_STRING("ver",  IDEDrive, dev.version),  \
+DEFINE_PROP_UINT64("wwn",  IDEDrive, dev.wwn, 0),   \
+DEFINE_PROP_STRING("serial",  IDEDrive, dev.serial),\
+DEFINE_PROP_STRING("model", IDEDrive, dev.model)
+
+void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp);
+
+#endif
diff --git a/hw/ide/cf.c b/hw/ide/cf.c
new file mode 100644
index 00..0b4bb57591
--- /dev/null
+++ b/hw/ide/cf.c
@@ -0,0 +1,58 @@
+/*
+ * ide CompactFlash support
+ *
+ * This code is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/ide/qdev-ide.h"
+#include "qapi/qapi-types-block.h"
+
+static void ide_cf_realize(IDEDevice *dev, Error **errp)
+{
+ide_dev_initfn(dev, IDE_CFATA, errp);
+}
+
+static Property ide_cf_properties[] = {
+DEFINE_IDE_DEV_PROPERTIES(),
+DEFINE_BLOCK_CHS_PROPERTIES(IDEDrive, dev.conf),
+DEFINE_PROP_BIOS_CHS_TRANS("bios-chs-trans",
+IDEDrive, dev.chs_trans, BIOS_ATA_TRANSLATION_AUTO),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void ide_cf_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+IDEDeviceClass *k = IDE_DEVICE_CLASS(klass);
+
+k->realize  = ide_cf_realize;
+dc->fw_name = "drive";
+dc->desc= "virtual CompactFlash card";
+device_class_set_props(dc, ide_cf_properties);
+}
+
+static const TypeInfo ide_cf_info = {
+.name  = "ide-cf",
+.parent= TYPE_IDE_DEVICE,
+.instance_size = sizeof(IDEDrive),
+.class_init= ide_cf_class_init,
+};
+
+static void ide_cf_register_type(void)
+{
+type_register_static(_cf_info);
+}
+
+type_init(ide_cf_register_type)
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 1b3b4da01d..a2f2d0ea08 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -24,12 +24,9 @@
 #include "qemu/error-report.h"
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
-#include "hw/ide/internal.h"
-#include "hw/qdev-properties.h"
-#include "hw/qdev-properties-system.h"
+#include "hw/ide/qdev-ide.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
-#include "hw/block/block.h"
 #include "sysemu/sysemu.h"
 #includ

Re: [PATCH v3 4/4] tests/tcg/s390x: Test CONVERT TO BINARY

2024-01-31 Thread Thomas Huth

On 01/02/2024 00.07, Ilya Leoshkevich wrote:

Check the CVB's and CVBG's corner cases.

Co-developed-by: Pavel Zbitskiy 
Signed-off-by: Ilya Leoshkevich 
---
  tests/tcg/s390x/Makefile.target |  1 +
  tests/tcg/s390x/cvb.c   | 47 +
  2 files changed, 48 insertions(+)
  create mode 100644 tests/tcg/s390x/cvb.c

diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 04e4bddd83d..e2aba2ec274 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -46,6 +46,7 @@ TESTS+=laalg
  TESTS+=add-logical-with-carry
  TESTS+=lae
  TESTS+=cvd
+TESTS+=cvb
  
  cdsg: CFLAGS+=-pthread

  cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/cvb.c b/tests/tcg/s390x/cvb.c
new file mode 100644
index 000..47b7a7965f4
--- /dev/null
+++ b/tests/tcg/s390x/cvb.c
@@ -0,0 +1,47 @@
+/*
+ * Test the CONVERT TO DECIMAL instruction.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include 
+#include 
+#include 
+
+static int32_t cvb(uint64_t x)
+{
+uint32_t ret;
+
+asm("cvb %[ret],%[x]" : [ret] "=r" (ret) : [x] "R" (x));
+
+return ret;
+}
+
+static int64_t cvbg(__uint128_t x)
+{
+int64_t ret;
+
+asm("cvbg %[ret],%[x]" : [ret] "=r" (ret) : [x] "T" (x));
+
+return ret;
+}


Just to be on the safe side, could you please add a check for CVBY, too?

 Thanks,
  Thomas





Re: [PATCH v3 3/4] target/s390x: implement CVB, CVBY and CVBG

2024-01-31 Thread Thomas Huth

On 01/02/2024 00.07, Ilya Leoshkevich wrote:

From: Pavel Zbitskiy 

Convert to Binary - counterparts of the already implemented Convert
to Decimal (CVD*) instructions.
Example from the Principles of Operation: 25594C becomes 63FA.

[iii: Use separate functions for CVB and CVBG for simplicity].

Signed-off-by: Pavel Zbitskiy 
---
  target/s390x/helper.h|  1 +
  target/s390x/tcg/insn-data.h.inc |  4 
  target/s390x/tcg/int_helper.c| 40 
  target/s390x/tcg/translate.c | 12 ++
  4 files changed, 57 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 332a9a9c632..3c607f4e437 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -88,6 +88,7 @@ DEF_HELPER_FLAGS_3(tcxb, TCG_CALL_NO_RWG_SE, i32, env, i128, 
i64)
  DEF_HELPER_FLAGS_2(sqeb, TCG_CALL_NO_WG, i64, env, i64)
  DEF_HELPER_FLAGS_2(sqdb, TCG_CALL_NO_WG, i64, env, i64)
  DEF_HELPER_FLAGS_2(sqxb, TCG_CALL_NO_WG, i128, env, i128)
+DEF_HELPER_FLAGS_3(cvb, TCG_CALL_NO_WG, i64, env, i64, i32)
  DEF_HELPER_FLAGS_1(cvd, TCG_CALL_NO_RWG_SE, i64, s32)
  DEF_HELPER_FLAGS_1(cvdg, TCG_CALL_NO_RWG_SE, i128, s64)
  DEF_HELPER_FLAGS_4(pack, TCG_CALL_NO_WG, void, env, i32, i64, i64)
diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc
index 388dcb8dbbc..9eb998d4c25 100644
--- a/target/s390x/tcg/insn-data.h.inc
+++ b/target/s390x/tcg/insn-data.h.inc
@@ -293,6 +293,10 @@
  D(0xec73, CLFIT,   RIE_a, GIE, r1_32u, i2_16u, 0, 0, ct, 0, 1)
  D(0xec71, CLGIT,   RIE_a, GIE, r1_o, i2_16u, 0, 0, ct, 0, 1)
  
+/* CONVERT TO BINARY */

+C(0x4f00, CVB, RX_a,  Z,   la2, 0, new, r1_32, cvb, 0)
+C(0xe306, CVBY,RXY_a, LD,  la2, 0, new, r1_32, cvb, 0)
+C(0xe30e, CVBG,RXY_a, Z,   la2, 0, r1, 0, cvbg, 0)
  /* CONVERT TO DECIMAL */
  C(0x4e00, CVD, RX_a,  Z,   r1_o, a2, 0, 0, cvd, 0)
  C(0xe326, CVDY,RXY_a, LD,  r1_o, a2, 0, 0, cvd, 0)
diff --git a/target/s390x/tcg/int_helper.c b/target/s390x/tcg/int_helper.c
index 121e3006a65..002d4b52dda 100644
--- a/target/s390x/tcg/int_helper.c
+++ b/target/s390x/tcg/int_helper.c
@@ -25,6 +25,7 @@
  #include "exec/exec-all.h"
  #include "qemu/host-utils.h"
  #include "exec/helper-proto.h"
+#include "exec/cpu_ldst.h"
  
  /* #define DEBUG_HELPER */

  #ifdef DEBUG_HELPER
@@ -98,6 +99,45 @@ Int128 HELPER(divu64)(CPUS390XState *env, uint64_t ah, 
uint64_t al, uint64_t b)
  tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
  }
  
+uint64_t HELPER(cvb)(CPUS390XState *env, uint64_t src, uint32_t n)

+{
+int64_t dec, sign = 0, digit, val = 0, pow10 = 0;
+const uintptr_t ra = GETPC();
+uint64_t tmpsrc;
+int i, j;
+
+for (i = 0; i < n; i++) {
+tmpsrc = wrap_address(env, src + (n - i - 1) * 8);
+dec = cpu_ldq_data_ra(env, tmpsrc, ra);
+for (j = 0; j < 16; j++, dec >>= 4) {
+if (i == 0 && j == 0) {
+sign = dec & 0xf;
+if (sign < 0xa) {
+tcg_s390_data_exception(env, 0, ra);
+}
+continue;
+}
+digit = dec & 0xf;
+if (digit > 0x9) {
+tcg_s390_data_exception(env, 0, ra);
+}
+if (i == 0 && j == 1) {
+if (sign == 0xb || sign == 0xd) {
+val = -digit;
+pow10 = -10;
+} else {
+val = digit;
+pow10 = 10;
+}
+} else {
+val += digit * pow10;
+pow10 *= 10;
+}
+}
+}
+return val;
+}


I just noticed that there was even a v5 of Pavel's patch where David noted 
that the fixed-point-divide exception checks are still missing, see:


https://patchwork.kernel.org/project/qemu-devel/patch/20180902003322.3428-4-pavel.zbits...@gmail.com/ 



Could you add those, too, please?

 Thanks,
  Thomas




Re: [PATCH v3 4/4] tests/tcg/s390x: Test CONVERT TO BINARY

2024-01-31 Thread Thomas Huth

On 01/02/2024 00.07, Ilya Leoshkevich wrote:

Check the CVB's and CVBG's corner cases.

Co-developed-by: Pavel Zbitskiy 
Signed-off-by: Ilya Leoshkevich 
---
  tests/tcg/s390x/Makefile.target |  1 +
  tests/tcg/s390x/cvb.c   | 47 +
  2 files changed, 48 insertions(+)
  create mode 100644 tests/tcg/s390x/cvb.c

diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 04e4bddd83d..e2aba2ec274 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -46,6 +46,7 @@ TESTS+=laalg
  TESTS+=add-logical-with-carry
  TESTS+=lae
  TESTS+=cvd
+TESTS+=cvb
  
  cdsg: CFLAGS+=-pthread

  cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/cvb.c b/tests/tcg/s390x/cvb.c
new file mode 100644
index 000..47b7a7965f4
--- /dev/null
+++ b/tests/tcg/s390x/cvb.c
@@ -0,0 +1,47 @@
+/*
+ * Test the CONVERT TO DECIMAL instruction.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include 
+#include 
+#include 
+
+static int32_t cvb(uint64_t x)
+{
+uint32_t ret;
+
+asm("cvb %[ret],%[x]" : [ret] "=r" (ret) : [x] "R" (x));
+
+return ret;
+}
+
+static int64_t cvbg(__uint128_t x)
+{
+int64_t ret;
+
+asm("cvbg %[ret],%[x]" : [ret] "=r" (ret) : [x] "T" (x));
+
+return ret;
+}
+
+int main(void)
+{
+__uint128_t m = (((__uint128_t)0x9223372036854775) << 16) | 0x8070;
+
+assert(cvb(0xc) == 0);
+assert(cvb(0x1c) == 1);
+assert(cvb(0x25594c) == 25594);
+assert(cvb(0x1d) == -1);
+assert(cvb(0x2147483647c) == 0x7fff);
+assert(cvb(0x2147483647d) == -0x7fff);
+
+assert(cvbg(0xc) == 0);
+assert(cvbg(0x1c) == 1);
+assert(cvbg(0x25594c) == 25594);
+assert(cvbg(0x1d) == -1);
+assert(cvbg(m | 0xc) == 0x7fff);
+assert(cvbg(m | 0xd) == -0x7fff);
+
+return EXIT_SUCCESS;
+}


Reviewed-by: Thomas Huth 




Re: [PATCH v3 2/4] tests/tcg/s390x: Test CONVERT TO DECIMAL

2024-01-31 Thread Thomas Huth

On 01/02/2024 00.07, Ilya Leoshkevich wrote:

Check the CVD's and CVDG's corner cases.

Signed-off-by: Ilya Leoshkevich 
---
  tests/tcg/s390x/Makefile.target |  1 +
  tests/tcg/s390x/cvd.c   | 45 +
  2 files changed, 46 insertions(+)
  create mode 100644 tests/tcg/s390x/cvd.c

diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 30994dcf9c2..04e4bddd83d 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -45,6 +45,7 @@ TESTS+=clc
  TESTS+=laalg
  TESTS+=add-logical-with-carry
  TESTS+=lae
+TESTS+=cvd
  
  cdsg: CFLAGS+=-pthread

  cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/cvd.c b/tests/tcg/s390x/cvd.c
new file mode 100644
index 000..c1fb63ca9a6
--- /dev/null
+++ b/tests/tcg/s390x/cvd.c
@@ -0,0 +1,45 @@
+/*
+ * Test the CONVERT TO DECIMAL instruction.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include 
+#include 
+#include 
+
+static uint64_t cvd(int32_t x)
+{
+uint64_t ret;
+
+asm("cvd %[x],%[ret]" : [ret] "=R" (ret) : [x] "r" (x));
+
+return ret;
+}
+
+static __uint128_t cvdg(int64_t x)
+{
+__uint128_t ret;
+
+asm("cvdg %[x],%[ret]" : [ret] "=T" (ret) : [x] "r" (x));
+
+return ret;
+}
+
+int main(void)
+{
+__uint128_t m = (((__uint128_t)0x9223372036854775) << 16) | 0x8070;
+
+assert(cvd(0) == 0xc);
+assert(cvd(1) == 0x1c);
+assert(cvd(-1) == 0x1d);
+assert(cvd(0x7fff) == 0x2147483647c);
+assert(cvd(-0x7fff) == 0x2147483647d);
+
+assert(cvdg(0) == 0xc);
+assert(cvdg(1) == 0x1c);
+assert(cvdg(-1) == 0x1d);
+assert(cvdg(0x7fff) == (m | 0xc));
+assert(cvdg(-0x7fff) == (m | 0xd));
+
+return EXIT_SUCCESS;
+}


Reviewed-by: Thomas Huth 




Re: [PATCH 0/2] Enable -Wvla, forbidding use of variable length arrays

2024-01-31 Thread Thomas Huth

On 25/01/2024 18.32, Peter Maydell wrote:

For a while now I've had an on-and-off-again campaign to get rid of
the handful of uses of C variable-length-array syntax in our
codebase.  The rationale for this is that if the array size can be
controlled by the guest and we don't get the size limit checking
right, this is an easy to exploit security issue.  (An example
problem of this kind from the past is CVE-2021-3527).  Forbidding
them entirely is a defensive measure against further bugs of this
kind.

I submitted a bunch of patches to this effect last year, and
the result is we're now down to just a single use of VLAs, in
a test program. This patchset removes that last VLA usage,
and enables -Wvla in our warning options, so that we will catch
any future attempts to use this C feature.

thanks
-- PMM

Peter Maydell (2):
   tests/qtest/xlnx-versal-trng-test.c: Drop use of variable length array
   meson: Enable -Wvla

  meson.build |  1 +
  tests/qtest/xlnx-versal-trng-test.c | 19 +++
  2 files changed, 12 insertions(+), 8 deletions(-)


There's still a vla left in the ppc kvm code:

 https://gitlab.com/thuth/qemu/-/jobs/6063230079#L2005

../target/ppc/kvm.c: In function ‘kvmppc_save_htab’:
../target/ppc/kvm.c:2691:5: error: ISO C90 forbids variable length array 
‘buf’ [-Werror=vla]

 2691 | uint8_t buf[bufsize];
  | ^~~
../target/ppc/kvm.c: In function ‘kvmppc_read_hptes’:
../target/ppc/kvm.c:2773:9: error: ISO C90 forbids variable length array 
‘buf’ [-Werror=vla]

 2773 | char buf[sizeof(*hdr) + m * HASH_PTE_SIZE_64];
  | ^~~~
cc1: all warnings being treated as errors

 Thomas




Re: [PATCH 0/3] make vm-build-freebsd fixes

2024-01-31 Thread Thomas Huth

On 25/01/2024 20.48, Ilya Leoshkevich wrote:

Hi,

I needed to verify that my qemu-user changes didn't break BSD, and
Daniel Berrange suggested vm-build-freebsd on IRC. I had several
problems with it, which this series resolves.

Best regards,
Ilya

Ilya Leoshkevich (3):
   tests/vm: Set UseDNS=no in the sshd configuration
   tests/vm/freebsd: Reload the sshd configuration
   meson: Disable CONFIG_NOTIFY1 on FreeBSD

  meson.build| 1 +
  tests/vm/basevm.py | 2 ++
  tests/vm/freebsd   | 2 ++
  3 files changed, 5 insertions(+)


Tested-by: Thomas Huth 

I can take the patches through my tree (and fix the second patch to use 
console_wait_send() if you don't mind).


 Thomas




Re: [PATCH 1/3] tests/vm: Set UseDNS=no in the sshd configuration

2024-01-31 Thread Thomas Huth

On 25/01/2024 20.48, Ilya Leoshkevich wrote:

make vm-build-freebsd sometimes fails with "Connection timed out during
banner exchange". The client strace shows:

 13:59:30 write(3, "SSH-2.0-OpenSSH_9.3\r\n", 21) = 21
 13:59:30 getpid()   = 252655
 13:59:30 poll([{fd=3, events=POLLIN}], 1, 5000) = 1 ([{fd=3, 
revents=POLLIN}])
 13:59:32 read(3, "S", 1)= 1
 13:59:32 poll([{fd=3, events=POLLIN}], 1, 3625) = 1 ([{fd=3, 
revents=POLLIN}])
 13:59:32 read(3, "S", 1)= 1
 13:59:32 poll([{fd=3, events=POLLIN}], 1, 3625) = 1 ([{fd=3, 
revents=POLLIN}])
 13:59:32 read(3, "H", 1)= 1

There is a 2s delay during connection, and ConnectTimeout is set to 1.
Raising it makes the issue go away, but we can do better. The server
truss shows:

 888: 27.811414714 socket(PF_INET,SOCK_DGRAM|SOCK_CLOEXEC,0) = 5 (0x5)
 888: 27.811765030 connect(5,{ AF_INET 10.0.2.3:53 },16) = 0 (0x0)
 888: 27.812166941 
sendto(5,"\^Z/\^A\0\0\^A\0\0\0\0\0\0\^A2"...,39,0,NULL,0) = 39 (0x27)
 888: 29.363970743 poll({ 5/POLLRDNORM },1,5000) = 1 (0x1)

So the delay is due to a DNS query. Disable DNS queries in the server
config.

Signed-off-by: Ilya Leoshkevich 
---
  tests/vm/basevm.py | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 61725b83254..c0d62c08031 100644
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -423,6 +423,8 @@ def console_ssh_init(self, prompt, user, pw):
  def console_sshd_config(self, prompt):
  self.console_wait(prompt)
  self.console_send("echo 'PermitRootLogin yes' >> 
/etc/ssh/sshd_config\n")
+self.console_wait(prompt)
+self.console_send("echo 'UseDNS no' >> /etc/ssh/sshd_config\n")
  for var in self.envvars:
  self.console_wait(prompt)
      self.console_send("echo 'AcceptEnv %s' >> /etc/ssh/sshd_config\n" 
% var)


Reviewed-by: Thomas Huth 




Re: [PATCH 3/3] meson: Disable CONFIG_NOTIFY1 on FreeBSD

2024-01-31 Thread Thomas Huth

On 25/01/2024 20.48, Ilya Leoshkevich wrote:

make vm-build-freebsd fails with:

 ld: error: undefined symbol: inotify_init1
 >>> referenced by filemonitor-inotify.c:183 
(../src/util/filemonitor-inotify.c:183)
 >>>   util_filemonitor-inotify.c.o:(qemu_file_monitor_new) in 
archive libqemuutil.a

On FreeBSD inotify functions are defined in libinotify.so, so it might
be tempting to add it to the dependencies. Doing so, however, reveals
that this library handles rename events differently from Linux:

 $ FILEMONITOR_DEBUG=1 build/tests/unit/test-util-filemonitor
 Rename /tmp/test-util-filemonitor-K13LI2/fish/one.txt -> 
/tmp/test-util-filemonitor-K13LI2/two.txt
 Event id=2 event=2 file=one.txt
 Queue event id 2 event 2 file one.txt
 Queue event id 1 event 2 file two.txt
 Queue event id 10002 event 2 file two.txt
 Queue event id 1 event 0 file two.txt
 Queue event id 10002 event 0 file two.txt
 Event id=1 event=0 file=two.txt
 Expected event 0 but got 2

FreeBSD itself disables this functionality in the respective port [1].
So do it upstream too.

[1] 
https://cgit.freebsd.org/ports/tree/emulators/qemu-devel/files/patch-util_meson.build?id=984366c18f1bc54e38751afc29be08c596b83696

Signed-off-by: Ilya Leoshkevich 
---
  meson.build | 1 +
  1 file changed, 1 insertion(+)

diff --git a/meson.build b/meson.build
index d0329966f1b..3d67d78b522 100644
--- a/meson.build
+++ b/meson.build
@@ -2403,6 +2403,7 @@ config_host_data.set('CONFIG_GETRANDOM',
  config_host_data.set('CONFIG_INOTIFY',
   cc.has_header_symbol('sys/inotify.h', 'inotify_init'))
  config_host_data.set('CONFIG_INOTIFY1',
+ host_os != 'freebsd' and
   cc.has_header_symbol('sys/inotify.h', 'inotify_init1'))
  config_host_data.set('CONFIG_PRCTL_PR_SET_TIMERSLACK',
   cc.has_header_symbol('sys/prctl.h', 'PR_SET_TIMERSLACK'))


Reviewed-by: Thomas Huth 




Re: [PATCH 2/3] tests/vm/freebsd: Reload the sshd configuration

2024-01-31 Thread Thomas Huth

On 25/01/2024 20.48, Ilya Leoshkevich wrote:

After console_sshd_config(), the SSH server needs to be nudged to pick
up the new configs. The scripts for the other BSD flavors already do
this with a reboot, but a simple reload is sufficient.

Signed-off-by: Ilya Leoshkevich 
---
  tests/vm/freebsd | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/tests/vm/freebsd b/tests/vm/freebsd
index b581bd17fb7..9971bc1f2b2 100755
--- a/tests/vm/freebsd
+++ b/tests/vm/freebsd
@@ -108,6 +108,8 @@ class FreeBSDVM(basevm.BaseVM):
  prompt = "root@freebsd:~ #"
  self.console_ssh_init(prompt, "root", self._config["root_pass"])
  self.console_sshd_config(prompt)
+self.console_wait(prompt)
+self.console_send("service sshd reload\n")


Could be simplified to:

self.console_wait_send(prompt, "service sshd reload\n")

 Thomas




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

2024-01-31 Thread Thomas Huth

On 26/01/2024 18.25, David Woodhouse wrote:

From: David Woodhouse 

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 
---
  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(_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", >dp8393x,
  TYPE_DP8393X);
  dev = DEVICE(>dp8393x);
-qdev_set_nic_properties(dev, _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.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(>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 29/47] hw/arm/stellaris: use qemu_find_nic_info()

2024-01-31 Thread Thomas Huth

On 26/01/2024 18.25, David Woodhouse wrote:

From: David Woodhouse 

Rather than just using qemu_configure_nic_device(), populate the MAC
address in the system-registers device by peeking at the NICInfo before
it's assigned to the device.

Generate the MAC address early, if there is no matching -nic option.
Otherwise the MAC address wouldn't be generated until net_client_init1()
runs.

Signed-off-by: David Woodhouse 
---
  hw/arm/stellaris.c | 30 ++
  1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index d18b1144af..34c5a86ac2 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -1028,7 +1028,8 @@ static void stellaris_init(MachineState *ms, 
stellaris_board_info *board)
  DeviceState *ssys_dev;
  int i;
  int j;
-const uint8_t *macaddr;
+NICInfo *nd;
+MACAddr mac;
  
  MemoryRegion *sram = g_new(MemoryRegion, 1);

  MemoryRegion *flash = g_new(MemoryRegion, 1);
@@ -1051,12 +1052,22 @@ static void stellaris_init(MachineState *ms, 
stellaris_board_info *board)
   * need its sysclk output.
   */
  ssys_dev = qdev_new(TYPE_STELLARIS_SYS);
-/* Most devices come preprogrammed with a MAC address in the user data. */
-macaddr = nd_table[0].macaddr.a;
+
+/*
+ * Most devices come preprogrammed with a MAC address in the user data.
+ * Generate a MAC address now, if there isn't a matching -nic for it.
+ */
+nd = qemu_find_nic_info("stellaris_enet", true, "stellaris");
+if (nd) {
+memcpy(mac.a, nd->macaddr.a, sizeof(mac.a));
+} else {
+qemu_macaddr_default_if_unset();
+}
+
  qdev_prop_set_uint32(ssys_dev, "user0",
- macaddr[0] | (macaddr[1] << 8) | (macaddr[2] << 16));
+ mac.a[0] | (mac.a[1] << 8) | (mac.a[2] << 16));
  qdev_prop_set_uint32(ssys_dev, "user1",
- macaddr[3] | (macaddr[4] << 8) | (macaddr[5] << 16));
+ mac.a[3] | (mac.a[4] << 8) | (mac.a[5] << 16));


Out of scope of your patch, but I wonder why we didn't use 
qdev_prop_set_macaddr() with an according MAC address property for this 
device...?



  qdev_prop_set_uint32(ssys_dev, "did0", board->did0);
  qdev_prop_set_uint32(ssys_dev, "did1", board->did1);
  qdev_prop_set_uint32(ssys_dev, "dc0", board->dc0);
@@ -1269,10 +1280,13 @@ static void stellaris_init(MachineState *ms, 
stellaris_board_info *board)
  if (board->dc4 & (1 << 28)) {
  DeviceState *enet;
  
-qemu_check_nic_model(_table[0], "stellaris");

-
  enet = qdev_new("stellaris_enet");
-qdev_set_nic_properties(enet, _table[0]);
+if (nd) {
+qdev_set_nic_properties(enet, nd);
+} else {
+qdev_prop_set_macaddr(enet, "mac", mac.a);
+}
+
  sysbus_realize_and_unref(SYS_BUS_DEVICE(enet), _fatal);
  sysbus_mmio_map(SYS_BUS_DEVICE(enet), 0, 0x40048000);
  sysbus_connect_irq(SYS_BUS_DEVICE(enet), 0, qdev_get_gpio_in(nvic, 
42));


Reviewed-by: Thomas Huth 




Re: [PATCH v4 46/47] net: remove qemu_show_nic_models(), qemu_find_nic_model()

2024-01-31 Thread Thomas Huth

On 26/01/2024 18.25, David Woodhouse wrote:

From: David Woodhouse 

These old functions can be removed now too. Let net_param_nic() print
the full set of network devices directly, and also make it note that a
list more specific to this platform/config will be available by using
'-nic model=help' instead.

Signed-off-by: David Woodhouse 
---
  include/net/net.h |  3 ---
  net/net.c | 39 ++-
  2 files changed, 6 insertions(+), 36 deletions(-)


Reviewed-by: Thomas Huth 





Re: [PATCH v4 42/47] hw/sparc/sun4m: use qemu_find_nic_info()

2024-01-31 Thread Thomas Huth

On 26/01/2024 18.25, David Woodhouse wrote:

From: David Woodhouse 

Obtain the MAC address from the NIC configuration if there is one, or
generate one explicitly so that it can be placed in the PROM.

Signed-off-by: David Woodhouse 
---
  hw/sparc/sun4m.c | 20 ++--
  1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 550af01690..e782c8ec7a 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -299,13 +299,15 @@ static void *iommu_init(hwaddr addr, uint32_t version, 
qemu_irq irq)
  
  static void *sparc32_dma_init(hwaddr dma_base,

hwaddr esp_base, qemu_irq espdma_irq,
-  hwaddr le_base, qemu_irq ledma_irq, NICInfo *nd)
+  hwaddr le_base, qemu_irq ledma_irq,
+  MACAddr *mac)
  {
  DeviceState *dma;
  ESPDMADeviceState *espdma;
  LEDMADeviceState *ledma;
  SysBusESPState *esp;
  SysBusPCNetState *lance;
+NICInfo *nd = qemu_find_nic_info("lance", true, NULL);
  
  dma = qdev_new(TYPE_SPARC32_DMA);

  espdma = SPARC32_ESPDMA_DEVICE(object_resolve_path_component(
@@ -320,7 +322,14 @@ static void *sparc32_dma_init(hwaddr dma_base,
  
  lance = SYSBUS_PCNET(object_resolve_path_component(

   OBJECT(ledma), "lance"));
-qdev_set_nic_properties(DEVICE(lance), nd);
+
+if (nd) {
+qdev_set_nic_properties(DEVICE(lance), nd);
+memcpy(mac->a, nd->macaddr.a, sizeof(mac->a));
+} else {
+qemu_macaddr_default_if_unset(mac);
+qdev_prop_set_macaddr(DEVICE(lance), "mac", mac->a);
+}
  
  sysbus_realize_and_unref(SYS_BUS_DEVICE(dma), _fatal);

  sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
@@ -823,7 +832,7 @@ static void sun4m_hw_init(MachineState *machine)
  unsigned int smp_cpus = machine->smp.cpus;
  unsigned int max_cpus = machine->smp.max_cpus;
  HostMemoryBackend *ram_memdev = machine->memdev;
-NICInfo *nd = _table[0];
+MACAddr hostid;
  
  if (machine->ram_size > hwdef->max_mem) {

  error_report("Too much memory for this machine: %" PRId64 ","
@@ -884,10 +893,9 @@ static void sun4m_hw_init(MachineState *machine)
  hwdef->iommu_pad_base, hwdef->iommu_pad_len);
  }
  
-qemu_check_nic_model(nd, TYPE_LANCE);

  sparc32_dma_init(hwdef->dma_base,
   hwdef->esp_base, slavio_irq[18],
- hwdef->le_base, slavio_irq[16], nd);
+ hwdef->le_base, slavio_irq[16], );
  
  if (graphic_depth != 8 && graphic_depth != 24) {

  error_report("Unsupported depth: %d", graphic_depth);
@@ -1039,7 +1047,7 @@ static void sun4m_hw_init(MachineState *machine)
  machine->initrd_filename,
  machine->ram_size, _size);
  
-nvram_init(nvram, (uint8_t *)>macaddr, machine->kernel_cmdline,

+nvram_init(nvram, hostid.a, machine->kernel_cmdline,
 machine->boot_config.order, machine->ram_size, kernel_size,
 graphic_width, graphic_height, graphic_depth,
 hwdef->nvram_machine_id, "Sun4m");


Reviewed-by: Thomas Huth 




Re: [PATCH v4 39/47] hw/openrisc/openrisc_sim: use qemu_create_nic_device()

2024-01-31 Thread Thomas Huth

On 26/01/2024 18.25, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/openrisc/openrisc_sim.c | 18 +-
  1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index 35da123aef..bffd6f721f 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -170,7 +170,7 @@ static void openrisc_create_fdt(Or1ksimState *state,
  
  static void openrisc_sim_net_init(Or1ksimState *state, hwaddr base, hwaddr size,

int num_cpus, OpenRISCCPU *cpus[],
-  int irq_pin, NICInfo *nd)
+  int irq_pin)
  {
  void *fdt = state->fdt;
  DeviceState *dev;
@@ -178,8 +178,10 @@ static void openrisc_sim_net_init(Or1ksimState *state, 
hwaddr base, hwaddr size,
  char *nodename;
  int i;
  
-dev = qdev_new("open_eth");

-qdev_set_nic_properties(dev, nd);
+dev = qemu_create_nic_device("open_eth", true, NULL);
+if (!dev) {
+return;
+}
  
  s = SYS_BUS_DEVICE(dev);

  sysbus_realize_and_unref(s, _fatal);
@@ -313,12 +315,10 @@ static void openrisc_sim_init(MachineState *machine)
  openrisc_create_fdt(state, or1ksim_memmap, smp_cpus, machine->ram_size,
  machine->kernel_cmdline);
  
-if (nd_table[0].used) {

-openrisc_sim_net_init(state, or1ksim_memmap[OR1KSIM_ETHOC].base,
-  or1ksim_memmap[OR1KSIM_ETHOC].size,
-  smp_cpus, cpus,
-  OR1KSIM_ETHOC_IRQ, nd_table);
-}
+openrisc_sim_net_init(state, or1ksim_memmap[OR1KSIM_ETHOC].base,
+  or1ksim_memmap[OR1KSIM_ETHOC].size,
+  smp_cpus, cpus,
+  OR1KSIM_ETHOC_IRQ);
  
  if (smp_cpus > 1) {

  openrisc_sim_ompic_init(state, or1ksim_memmap[OR1KSIM_OMPIC].base,


Reviewed-by: Thomas Huth 




Re: [PATCH v4 37/47] hw/net/lasi_i82596: Re-enable build

2024-01-31 Thread Thomas Huth

On 26/01/2024 18.25, David Woodhouse wrote:

From: David Woodhouse 

When converting to the shiny build-system-du-jour, a typo prevented the
last_i82596 driver from being built. Correct the config option name to
re-enable the build. And include "sysemu/sysemu.h" so it actually builds.

Fixes: b1419fa66558 ("meson: convert hw/net")
Signed-off-by: David Woodhouse 
---
  hw/net/lasi_i82596.c | 1 +
  hw/net/meson.build   | 2 +-
  2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/net/lasi_i82596.c b/hw/net/lasi_i82596.c
index 6a3147fe2d..09e830ba5f 100644
--- a/hw/net/lasi_i82596.c
+++ b/hw/net/lasi_i82596.c
@@ -14,6 +14,7 @@
  #include "qapi/error.h"
  #include "qemu/timer.h"
  #include "hw/sysbus.h"
+#include "sysemu/sysemu.h"
  #include "net/eth.h"
  #include "hw/net/lasi_82596.h"
  #include "hw/net/i82596.h"
diff --git a/hw/net/meson.build b/hw/net/meson.build
index 9afceb0619..2b426d3d5a 100644
--- a/hw/net/meson.build
+++ b/hw/net/meson.build
@@ -33,7 +33,7 @@ system_ss.add(when: 'CONFIG_MARVELL_88W8618', if_true: 
files('mv88w8618_eth.c'))
  system_ss.add(when: 'CONFIG_CADENCE', if_true: files('cadence_gem.c'))
  system_ss.add(when: 'CONFIG_STELLARIS_ENET', if_true: 
files('stellaris_enet.c'))
  system_ss.add(when: 'CONFIG_LANCE', if_true: files('lance.c'))
-system_ss.add(when: 'CONFIG_LASI_I82596', if_true: files('lasi_i82596.c'))
+system_ss.add(when: 'CONFIG_LASI_82596', if_true: files('lasi_i82596.c'))
  system_ss.add(when: 'CONFIG_I82596_COMMON', if_true: files('i82596.c'))
  system_ss.add(when: 'CONFIG_SUNHME', if_true: files('sunhme.c'))
  system_ss.add(when: 'CONFIG_FTGMAC100', if_true: files('ftgmac100.c'))


I'm surprised that it even did not bitrot much further!

Reviewed-by: Thomas Huth 




Re: [PATCH v4 36/47] hw/mips/jazz: use qemu_find_nic_info()

2024-01-31 Thread Thomas Huth

On 26/01/2024 18.25, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/mips/jazz.c | 15 +++
  1 file changed, 7 insertions(+), 8 deletions(-)


Reviewed-by: Thomas Huth 




Re: [PATCH v4 35/47] hw/mips/mipssim: use qemu_create_nic_device()

2024-01-31 Thread Thomas Huth

On 26/01/2024 18.25, David Woodhouse wrote:

From: David Woodhouse 

The MIPS SIM platform instantiates its NIC only if a corresponding
configuration exists for it. Use qemu_create_nic_device() function for
that.

Signed-off-by: David Woodhouse 
---
  hw/mips/mipssim.c | 13 +++--
  1 file changed, 7 insertions(+), 6 deletions(-)


Reviewed-by: Thomas Huth 




Re: [PATCH v4 30/47] hw/arm: use qemu_configure_nic_device()

2024-01-31 Thread Thomas Huth

On 26/01/2024 18.25, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/arm/mps2-tz.c |  8 ++--
  hw/arm/msf2-soc.c|  6 +-
  hw/arm/musicpal.c|  3 +--
  hw/arm/xilinx_zynq.c | 11 ---
  hw/arm/xlnx-versal.c |  7 +--
  hw/arm/xlnx-zynqmp.c |  8 +---
  6 files changed, 10 insertions(+), 33 deletions(-)


Reviewed-by: Thomas Huth 




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

2024-01-30 Thread Thomas Huth

On 26/01/2024 18.25, David Woodhouse wrote:

From: David Woodhouse 

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 
---
  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(>emc[i]);
-if (nd_table[i].used) {
-qemu_check_nic_model(_table[i], TYPE_NPCM7XX_EMC);
-qdev_set_nic_properties(DEVICE(sbd), _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 




Re: [PATCH v4 26/47] hw/net/lan9118: use qemu_configure_nic_device()

2024-01-30 Thread Thomas Huth

On 26/01/2024 18.25, David Woodhouse wrote:

From: David Woodhouse 

Some callers instantiate the device unconditionally, others will do so only
if there is a NICInfo to go with it. This appears to be fairly random, but
preseve the existing behaviour for now.

Signed-off-by: David Woodhouse 
---
  hw/arm/kzm.c | 4 ++--
  hw/arm/mps2.c| 2 +-
  hw/arm/realview.c| 6 ++
  hw/arm/vexpress.c| 4 ++--
  hw/net/lan9118.c | 5 ++---
  include/hw/net/lan9118.h | 2 +-
  6 files changed, 10 insertions(+), 13 deletions(-)


Reviewed-by: Thomas Huth 





Re: [PATCH v3 02/29] hw/core: Declare CPUArchId::cpu as CPUState instead of Object

2024-01-30 Thread Thomas Huth

On 29/01/2024 17.44, Philippe Mathieu-Daudé wrote:

Do not accept any Object for CPUArchId::cpu field,
restrict it to CPUState type.

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/hw/boards.h| 2 +-
  hw/core/machine.c  | 4 ++--
  hw/i386/x86.c  | 2 +-
  hw/loongarch/virt.c| 2 +-
  hw/ppc/spapr.c | 5 ++---
  hw/s390x/s390-virtio-ccw.c | 2 +-
  6 files changed, 8 insertions(+), 9 deletions(-)


Reviewed-by: Thomas Huth 




Re: [PATCH v4 8/8] Add tests for the STM32L4x5_RCC

2024-01-30 Thread Thomas Huth

On 30/01/2024 17.13, Arnaud Minier wrote:

Tests:
- the ability to change the sysclk of the device
- the ability to enable/disable/configure the PLLs
- if the clock multiplexers work
- the register flags and the generation of irqs

Signed-off-by: Arnaud Minier 
Signed-off-by: Inès Varhol 
---
  tests/qtest/meson.build  |   3 +-
  tests/qtest/stm32l4x5_rcc-test.c | 207 +++
  2 files changed, 209 insertions(+), 1 deletion(-)
  create mode 100644 tests/qtest/stm32l4x5_rcc-test.c


Acked-by: Thomas Huth 




Re: [PATCH v5 6/6] tests/qtest/pvpanic: add tests for pvshutdown event

2024-01-29 Thread Thomas Huth

On 29/01/2024 20.28, Thomas Weißschuh wrote:

Validate that a shutdown via the pvpanic device emits the correct
QMP events.

Signed-off-by: Thomas Weißschuh 
---
  tests/qtest/pvpanic-pci-test.c | 39 +++
  tests/qtest/pvpanic-test.c | 29 +
  2 files changed, 68 insertions(+)

diff --git a/tests/qtest/pvpanic-pci-test.c b/tests/qtest/pvpanic-pci-test.c
index b372caf41dc0..e1c05d383219 100644
--- a/tests/qtest/pvpanic-pci-test.c
+++ b/tests/qtest/pvpanic-pci-test.c
@@ -85,11 +85,50 @@ static void test_panic(void)
  qtest_quit(qts);
  }
  
+static void test_pvshutdown(void)

+{
+uint8_t val;
+QDict *response, *data;
+QTestState *qts;
+QPCIBus *pcibus;
+QPCIDevice *dev;
+QPCIBar bar;
+
+qts = qtest_init("-device pvpanic-pci,addr=04.0");
+pcibus = qpci_new_pc(qts, NULL);
+dev = qpci_device_find(pcibus, QPCI_DEVFN(0x4, 0x0));
+qpci_device_enable(dev);
+bar = qpci_iomap(dev, 0, NULL);
+
+qpci_memread(dev, bar, 0, , sizeof(val));
+g_assert_cmpuint(val, ==, PVPANIC_EVENTS);
+
+val = 4;


Could you use PVPANIC_SHUTDOWN here instead of the magic value?


+qpci_memwrite(dev, bar, 0, , sizeof(val));
+
+response = qtest_qmp_eventwait_ref(qts, "GUEST_PVSHUTDOWN");
+qobject_unref(response);
+
+response = qtest_qmp_eventwait_ref(qts, "SHUTDOWN");
+g_assert(qdict_haskey(response, "data"));
+data = qdict_get_qdict(response, "data");
+g_assert(qdict_haskey(data, "guest"));
+g_assert(qdict_get_bool(data, "guest"));
+g_assert(qdict_haskey(data, "reason"));
+g_assert_cmpstr(qdict_get_str(data, "reason"), ==, "guest-shutdown");
+qobject_unref(response);
+
+g_free(dev);
+qpci_free_pc(pcibus);
+qtest_quit(qts);
+}
+
  int main(int argc, char **argv)
  {
  g_test_init(, , NULL);
  qtest_add_func("/pvpanic-pci/panic", test_panic);
  qtest_add_func("/pvpanic-pci/panic-nopause", test_panic_nopause);
+qtest_add_func("/pvpanic-pci/pvshutdown", test_pvshutdown);
  
  return g_test_run();

  }
diff --git a/tests/qtest/pvpanic-test.c b/tests/qtest/pvpanic-test.c
index ccc603472f5d..ff1f25f46586 100644
--- a/tests/qtest/pvpanic-test.c
+++ b/tests/qtest/pvpanic-test.c
@@ -58,11 +58,40 @@ static void test_panic(void)
  qtest_quit(qts);
  }
  
+static void test_pvshutdown(void)

+{
+uint8_t val;
+QDict *response, *data;
+QTestState *qts;
+
+qts = qtest_init("-device pvpanic");
+
+val = qtest_inb(qts, 0x505);
+g_assert_cmpuint(val, ==, PVPANIC_EVENTS);
+
+qtest_outb(qts, 0x505, 0x4);


dito, use PVPANIC_SHUTDOWN instead of 4 ?

(as a separate clean-up, we should maybe also introduce a #define for 0x505 
one day, but that's something for another patch)



+response = qtest_qmp_eventwait_ref(qts, "GUEST_PVSHUTDOWN");
+qobject_unref(response);
+
+response = qtest_qmp_eventwait_ref(qts, "SHUTDOWN");
+g_assert(qdict_haskey(response, "data"));
+data = qdict_get_qdict(response, "data");
+g_assert(qdict_haskey(data, "guest"));
+g_assert(qdict_get_bool(data, "guest"));
+g_assert(qdict_haskey(data, "reason"));
+g_assert_cmpstr(qdict_get_str(data, "reason"), ==, "guest-shutdown");
+qobject_unref(response);
+
+qtest_quit(qts);
+}
+
  int main(int argc, char **argv)
  {
  g_test_init(, , NULL);
  qtest_add_func("/pvpanic/panic", test_panic);
  qtest_add_func("/pvpanic/panic-nopause", test_panic_nopause);
+qtest_add_func("/pvpanic/pvshutdown", test_pvshutdown);
  
  return g_test_run();

  }



With PVPANIC_SHUTDOWN instead of 4:
Reviewed-by: Thomas Huth 




Re: [PATCH] hw/hyperv: Include missing headers

2024-01-29 Thread Thomas Huth

On 29/01/2024 18.00, Philippe Mathieu-Daudé wrote:

Include missing headers in order to avoid when refactoring
unrelated headers:

   hw/hyperv/hyperv.c:33:18: error: field ‘msg_page_mr’ has incomplete type
 33 | MemoryRegion msg_page_mr;
|  ^~~
   hw/hyperv/hyperv.c: In function ‘synic_update’:
   hw/hyperv/hyperv.c:64:13: error: implicit declaration of function 
‘memory_region_del_subregion’ [-Werror=implicit-function-declaration]
 64 | memory_region_del_subregion(get_system_memory(),
| ^~~
   hw/hyperv/hyperv.c: In function ‘hyperv_hcall_signal_event’:
   hw/hyperv/hyperv.c:683:17: error: implicit declaration of function 
‘ldq_phys’; did you mean ‘ldub_phys’? [-Werror=implicit-function-declaration]
683 | param = ldq_phys(_space_memory, addr);
| ^~~~
| ldub_phys
   hw/hyperv/hyperv.c:683:17: error: nested extern declaration of ‘ldq_phys’ 
[-Werror=nested-externs]
   hw/hyperv/hyperv.c: In function ‘hyperv_hcall_retreive_dbg_data’:
   hw/hyperv/hyperv.c:792:24: error: ‘TARGET_PAGE_SIZE’ undeclared (first use 
in this function); did you mean ‘TARGET_PAGE_BITS’?
792 | msg.u.recv.count = TARGET_PAGE_SIZE - sizeof(*debug_data_out);
|^~~~
|TARGET_PAGE_BITS
   hw/hyperv/hyperv.c: In function ‘hyperv_syndbg_send’:
   hw/hyperv/hyperv.c:885:16: error: ‘HV_SYNDBG_STATUS_INVALID’ undeclared 
(first use in this function)
885 | return HV_SYNDBG_STATUS_INVALID;
|^~~~

Signed-off-by: Philippe Mathieu-Daudé 
---
BTW who maintains this code?

$ ./scripts/get_maintainer.pl -f hw/hyperv/hyperv.c
get_maintainer.pl: No maintainers found, printing recent contributors.
get_maintainer.pl: Do not blindly cc: them on patches!  Use common sense.
---
  hw/hyperv/hyperv.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
index 57b402b956..6c4a18dd0e 100644
--- a/hw/hyperv/hyperv.c
+++ b/hw/hyperv/hyperv.c
@@ -12,6 +12,7 @@
  #include "qemu/module.h"
  #include "qapi/error.h"
  #include "exec/address-spaces.h"
+#include "exec/memory.h"
  #include "sysemu/kvm.h"
  #include "qemu/bitops.h"
  #include "qemu/error-report.h"
@@ -21,6 +22,9 @@
  #include "qemu/rcu_queue.h"
  #include "hw/hyperv/hyperv.h"
  #include "qom/object.h"
+#include "target/i386/kvm/hyperv-proto.h"
+#include "target/i386/cpu.h"
+#include "exec/cpu-all.h"
  
  struct SynICState {

  DeviceState parent_obj;


Reviewed-by: Thomas Huth 




Re: [PATCH] hw/hyperv: Include missing headers

2024-01-29 Thread Thomas Huth

On 29/01/2024 18.00, Philippe Mathieu-Daudé wrote:

Include missing headers in order to avoid when refactoring
unrelated headers:

   hw/hyperv/hyperv.c:33:18: error: field ‘msg_page_mr’ has incomplete type
 33 | MemoryRegion msg_page_mr;
|  ^~~
   hw/hyperv/hyperv.c: In function ‘synic_update’:
   hw/hyperv/hyperv.c:64:13: error: implicit declaration of function 
‘memory_region_del_subregion’ [-Werror=implicit-function-declaration]
 64 | memory_region_del_subregion(get_system_memory(),
| ^~~
   hw/hyperv/hyperv.c: In function ‘hyperv_hcall_signal_event’:
   hw/hyperv/hyperv.c:683:17: error: implicit declaration of function 
‘ldq_phys’; did you mean ‘ldub_phys’? [-Werror=implicit-function-declaration]
683 | param = ldq_phys(_space_memory, addr);
| ^~~~
| ldub_phys
   hw/hyperv/hyperv.c:683:17: error: nested extern declaration of ‘ldq_phys’ 
[-Werror=nested-externs]
   hw/hyperv/hyperv.c: In function ‘hyperv_hcall_retreive_dbg_data’:
   hw/hyperv/hyperv.c:792:24: error: ‘TARGET_PAGE_SIZE’ undeclared (first use 
in this function); did you mean ‘TARGET_PAGE_BITS’?
792 | msg.u.recv.count = TARGET_PAGE_SIZE - sizeof(*debug_data_out);
|^~~~
|TARGET_PAGE_BITS
   hw/hyperv/hyperv.c: In function ‘hyperv_syndbg_send’:
   hw/hyperv/hyperv.c:885:16: error: ‘HV_SYNDBG_STATUS_INVALID’ undeclared 
(first use in this function)
885 | return HV_SYNDBG_STATUS_INVALID;
|^~~~

Signed-off-by: Philippe Mathieu-Daudé 
---
BTW who maintains this code?

$ ./scripts/get_maintainer.pl -f hw/hyperv/hyperv.c
get_maintainer.pl: No maintainers found, printing recent contributors.
get_maintainer.pl: Do not blindly cc: them on patches!  Use common sense.
---
  hw/hyperv/hyperv.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
index 57b402b956..6c4a18dd0e 100644
--- a/hw/hyperv/hyperv.c
+++ b/hw/hyperv/hyperv.c
@@ -12,6 +12,7 @@
  #include "qemu/module.h"
  #include "qapi/error.h"
  #include "exec/address-spaces.h"
+#include "exec/memory.h"
  #include "sysemu/kvm.h"
  #include "qemu/bitops.h"
  #include "qemu/error-report.h"
@@ -21,6 +22,9 @@
  #include "qemu/rcu_queue.h"
  #include "hw/hyperv/hyperv.h"
  #include "qom/object.h"
+#include "target/i386/kvm/hyperv-proto.h"
+#include "target/i386/cpu.h"
+#include "exec/cpu-all.h"
  
  struct SynICState {

  DeviceState parent_obj;


Reviewed-by: Thomas Huth 




Re: [PATCH] hw/intc/xics: Include missing 'cpu.h' header

2024-01-29 Thread Thomas Huth

On 29/01/2024 18.05, Philippe Mathieu-Daudé wrote:

Include missing headers in order to avoid when refactoring
unrelated headers:

   hw/intc/xics.c: In function 'icp_realize':
   hw/intc/xics.c:304:5: error: unknown type name 'PowerPCCPU'
 304 | PowerPCCPU *cpu;
 | ^~

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/intc/xics.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 8b25787227..700abfa7a6 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -40,6 +40,7 @@
  #include "hw/irq.h"
  #include "sysemu/kvm.h"
  #include "sysemu/reset.h"
+#include "target/ppc/cpu.h"
  
  void icp_pic_print_info(ICPState *icp, Monitor *mon)

  {


Reviewed-by: Thomas Huth 




Re: [PATCH] configure: put all symlink creation together

2024-01-29 Thread Thomas Huth

On 29/01/2024 14.48, Paolo Bonzini wrote:

Based-on: <20240129133651.1106552-1-pbonz...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
  configure | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index 9cdb5a6818b..3cd736b139f 100755
--- a/configure
+++ b/configure
@@ -1538,6 +1538,11 @@ for f in $LINKS ; do
  fi
  done
  
+# use included Linux headers for KVM architectures

+if test "$host_os" = "linux" && test -n "$linux_arch"; then
+  symlink "$source_path/linux-headers/asm-$linux_arch" linux-headers/asm
+fi
+
  echo "# Automatically generated by configure - do not modify" > 
Makefile.prereqs
  
  # Mac OS X ships with a broken assembler

@@ -1605,11 +1610,6 @@ echo "GENISOIMAGE=$genisoimage" >> $config_host_mak
  echo "MESON=$meson" >> $config_host_mak
  echo "NINJA=$ninja" >> $config_host_mak
  echo "EXESUF=$EXESUF" >> $config_host_mak
-# use included Linux headers for KVM architectures
-if test "$host_os" = "linux" && test -n "$linux_arch"; then
-  symlink "$source_path/linux-headers/asm-$linux_arch" linux-headers/asm
-fi
-
  if test "$default_targets" = "yes"; then
echo "CONFIG_DEFAULT_TARGETS=y" >> $config_host_mak
  fi


Reviewed-by: Thomas Huth 




Re: [PATCH] configure: do not create legacy symlinks

2024-01-29 Thread Thomas Huth

On 29/01/2024 14.36, Paolo Bonzini wrote:

With more than three years since Meson was introduced in the build system, 
people
have had quite some time to move away from the foo-softmmu/qemu-system-* and
foo-linux-user/qemu-* symbolic links.  Remove them, and with them another
instance of the "softmmu" name for system emulators.

Signed-off-by: Paolo Bonzini 
---
  configure | 10 --
  1 file changed, 10 deletions(-)

diff --git a/configure b/configure
index ff058d6c486..9cdb5a6818b 100755
--- a/configure
+++ b/configure
@@ -1605,21 +1605,11 @@ echo "GENISOIMAGE=$genisoimage" >> $config_host_mak
  echo "MESON=$meson" >> $config_host_mak
  echo "NINJA=$ninja" >> $config_host_mak
  echo "EXESUF=$EXESUF" >> $config_host_mak
-


Maybe keep the empty line here?


  # use included Linux headers for KVM architectures
  if test "$host_os" = "linux" && test -n "$linux_arch"; then
symlink "$source_path/linux-headers/asm-$linux_arch" linux-headers/asm
  fi
  
-for target in $target_list; do

-target_dir="$target"
-target_name=$(echo $target | cut -d '-' -f 1)$EXESUF
-case $target in
-*-user) symlink "../qemu-$target_name" "$target_dir/qemu-$target_name" 
;;
-*) symlink "../qemu-system-$target_name" 
"$target_dir/qemu-system-$target_name" ;;
-esac
-done
-


Anyway,
Reviewed-by: Thomas Huth 





[PATCH v2 2/3] target/arm/tcg/m_helper.c: Include the full helpers only with CONFIG_ARM_V7M

2024-01-29 Thread Thomas Huth
If CONFIG_ARM_V7M is not set, we don't want to include the full-fledged
helper functions that require additional functions for linking. The
reduced set of the linux-user functions works fine as stubs in this
case, so change the #ifdef statement accordingly.

Signed-off-by: Thomas Huth 
---
 target/arm/tcg/m_helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/arm/tcg/m_helper.c b/target/arm/tcg/m_helper.c
index d1f1e02acc..a5a6e96fc3 100644
--- a/target/arm/tcg/m_helper.c
+++ b/target/arm/tcg/m_helper.c
@@ -22,6 +22,7 @@
 #endif
 #if !defined(CONFIG_USER_ONLY)
 #include "hw/intc/armv7m_nvic.h"
+#include CONFIG_DEVICES
 #endif
 
 static void v7m_msr_xpsr(CPUARMState *env, uint32_t mask,
@@ -69,7 +70,7 @@ uint32_t arm_v7m_mrs_control(CPUARMState *env, uint32_t 
secure)
 return value;
 }
 
-#ifdef CONFIG_USER_ONLY
+#if defined(CONFIG_USER_ONLY) || !defined(CONFIG_ARM_V7M)
 
 void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val)
 {
-- 
2.43.0




[PATCH v2 1/3] target/arm: Move v7m-related code from cpu32.c into a separate file

2024-01-29 Thread Thomas Huth
Move the code to a separate file so that we do not have to compile
it anymore if CONFIG_ARM_V7M is not set.

Signed-off-by: Thomas Huth 
---
 target/arm/tcg/cpu-v7m.c   | 290 +
 target/arm/tcg/cpu32.c | 261 -
 target/arm/meson.build |   3 +
 target/arm/tcg/meson.build |   3 +
 4 files changed, 296 insertions(+), 261 deletions(-)
 create mode 100644 target/arm/tcg/cpu-v7m.c

diff --git a/target/arm/tcg/cpu-v7m.c b/target/arm/tcg/cpu-v7m.c
new file mode 100644
index 00..89a25444a2
--- /dev/null
+++ b/target/arm/tcg/cpu-v7m.c
@@ -0,0 +1,290 @@
+/*
+ * QEMU ARMv7-M TCG-only CPUs.
+ *
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * This code is licensed under the GNU GPL v2 or later.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "hw/core/tcg-cpu-ops.h"
+#include "internals.h"
+
+#if !defined(CONFIG_USER_ONLY)
+
+#include "hw/intc/armv7m_nvic.h"
+
+static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
+{
+CPUClass *cc = CPU_GET_CLASS(cs);
+ARMCPU *cpu = ARM_CPU(cs);
+CPUARMState *env = >env;
+bool ret = false;
+
+/*
+ * ARMv7-M interrupt masking works differently than -A or -R.
+ * There is no FIQ/IRQ distinction. Instead of I and F bits
+ * masking FIQ and IRQ interrupts, an exception is taken only
+ * if it is higher priority than the current execution priority
+ * (which depends on state like BASEPRI, FAULTMASK and the
+ * currently active exception).
+ */
+if (interrupt_request & CPU_INTERRUPT_HARD
+&& (armv7m_nvic_can_take_pending_exception(env->nvic))) {
+cs->exception_index = EXCP_IRQ;
+cc->tcg_ops->do_interrupt(cs);
+ret = true;
+}
+return ret;
+}
+
+#endif /* !CONFIG_USER_ONLY */
+
+static void cortex_m0_initfn(Object *obj)
+{
+ARMCPU *cpu = ARM_CPU(obj);
+set_feature(>env, ARM_FEATURE_V6);
+set_feature(>env, ARM_FEATURE_M);
+
+cpu->midr = 0x410cc200;
+
+/*
+ * These ID register values are not guest visible, because
+ * we do not implement the Main Extension. They must be set
+ * to values corresponding to the Cortex-M0's implemented
+ * features, because QEMU generally controls its emulation
+ * by looking at ID register fields. We use the same values as
+ * for the M3.
+ */
+cpu->isar.id_pfr0 = 0x0030;
+cpu->isar.id_pfr1 = 0x0200;
+cpu->isar.id_dfr0 = 0x0010;
+cpu->id_afr0 = 0x;
+cpu->isar.id_mmfr0 = 0x0030;
+cpu->isar.id_mmfr1 = 0x;
+cpu->isar.id_mmfr2 = 0x;
+cpu->isar.id_mmfr3 = 0x;
+cpu->isar.id_isar0 = 0x01141110;
+cpu->isar.id_isar1 = 0x02111000;
+cpu->isar.id_isar2 = 0x21112231;
+cpu->isar.id_isar3 = 0x0110;
+cpu->isar.id_isar4 = 0x01310102;
+cpu->isar.id_isar5 = 0x;
+cpu->isar.id_isar6 = 0x;
+}
+
+static void cortex_m3_initfn(Object *obj)
+{
+ARMCPU *cpu = ARM_CPU(obj);
+set_feature(>env, ARM_FEATURE_V7);
+set_feature(>env, ARM_FEATURE_M);
+set_feature(>env, ARM_FEATURE_M_MAIN);
+cpu->midr = 0x410fc231;
+cpu->pmsav7_dregion = 8;
+cpu->isar.id_pfr0 = 0x0030;
+cpu->isar.id_pfr1 = 0x0200;
+cpu->isar.id_dfr0 = 0x0010;
+cpu->id_afr0 = 0x;
+cpu->isar.id_mmfr0 = 0x0030;
+cpu->isar.id_mmfr1 = 0x;
+cpu->isar.id_mmfr2 = 0x;
+cpu->isar.id_mmfr3 = 0x;
+cpu->isar.id_isar0 = 0x01141110;
+cpu->isar.id_isar1 = 0x02111000;
+cpu->isar.id_isar2 = 0x21112231;
+cpu->isar.id_isar3 = 0x0110;
+cpu->isar.id_isar4 = 0x01310102;
+cpu->isar.id_isar5 = 0x;
+cpu->isar.id_isar6 = 0x;
+}
+
+static void cortex_m4_initfn(Object *obj)
+{
+ARMCPU *cpu = ARM_CPU(obj);
+
+set_feature(>env, ARM_FEATURE_V7);
+set_feature(>env, ARM_FEATURE_M);
+set_feature(>env, ARM_FEATURE_M_MAIN);
+set_feature(>env, ARM_FEATURE_THUMB_DSP);
+cpu->midr = 0x410fc240; /* r0p0 */
+cpu->pmsav7_dregion = 8;
+cpu->isar.mvfr0 = 0x10110021;
+cpu->isar.mvfr1 = 0x1111;
+cpu->isar.mvfr2 = 0x;
+cpu->isar.id_pfr0 = 0x0030;
+cpu->isar.id_pfr1 = 0x0200;
+cpu->isar.id_dfr0 = 0x0010;
+cpu->id_afr0 = 0x;
+cpu->isar.id_mmfr0 = 0x0030;
+cpu->isar.id_mmfr1 = 0x;
+cpu->isar.id_mmfr2 = 0x;
+cpu->isar.id_mmfr3 = 0x;
+cpu->isar.id_isar0 = 0x01141110;
+cpu->isar.id_isar1 = 0x02111000;
+cpu->isar.id_isar2 = 0x21112231;
+cpu->isar.id_isar3 = 0x0110;
+cpu->isar.id_isar4 = 0x01310102;

[PATCH v2 0/3] target/arm: Allow compilation without CONFIG_ARM_V7M

2024-01-29 Thread Thomas Huth
We've got a switch to disable v7m code since a long time - but it
currently cannot be disabled since linking then fails due to missing
functions. But thanks to the clean-ups that have been done during the
past years, it's not that difficult anymore to finally make it possible
to disable CONFIG_ARM_V7M: We just have to move some v7m-related code
out of cpu32.c to a separate file (that we only compile if the switch
CONFIG_ARM_V7M is enabled) and make sure to use the stub functions in
m_helper.c if it is disabled. Then we can finally remove the hard-coded
"select ARM_V7M" from the Kconfig file.

v2:
- Updated a comment
- Avoid #ifdef in cpu-v7m.c, handle it via meson.build instead

Thomas Huth (3):
  target/arm: Move v7m-related code from cpu32.c into a separate file
  target/arm/tcg/m_helper.c: Include the full helpers only with
CONFIG_ARM_V7M
  target/arm/Kconfig: Stop requiring CONFIG_ARM_V7M

 target/arm/tcg/cpu-v7m.c   | 290 +
 target/arm/tcg/cpu32.c | 261 -
 target/arm/tcg/m_helper.c  |   3 +-
 target/arm/Kconfig |   4 -
 target/arm/meson.build |   3 +
 target/arm/tcg/meson.build |   3 +
 6 files changed, 298 insertions(+), 266 deletions(-)
 create mode 100644 target/arm/tcg/cpu-v7m.c

-- 
2.43.0




[PATCH v2 3/3] target/arm/Kconfig: Stop requiring CONFIG_ARM_V7M

2024-01-29 Thread Thomas Huth
Now that we made sure that ARM_V7M code only gets compiled if really
needed, we can drop the hard requirement for CONFIG_ARM_V7M in the
Kconfig file.

Tested-by: Fabiano Rosas 
Signed-off-by: Thomas Huth 
---
 target/arm/Kconfig | 4 
 1 file changed, 4 deletions(-)

diff --git a/target/arm/Kconfig b/target/arm/Kconfig
index bf57d739cd..3fffdcb61b 100644
--- a/target/arm/Kconfig
+++ b/target/arm/Kconfig
@@ -2,10 +2,6 @@ config ARM
 bool
 select ARM_COMPATIBLE_SEMIHOSTING if TCG
 
-# We need to select this until we move m_helper.c and the
-# translate.c v7m helpers under ARM_V7M.
-select ARM_V7M if TCG
-
 config AARCH64
 bool
 select ARM
-- 
2.43.0




Re: [PATCH v2 19/23] target/s390x: Prefer fast cpu_env() over slower CPU QOM cast macro

2024-01-28 Thread Thomas Huth

On 26/01/2024 23.04, Philippe Mathieu-Daudé wrote:

Mechanical patch produced running the command documented
in scripts/coccinelle/cpu_env.cocci_template header.

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/s390x/cpu-dump.c|  3 +--
  target/s390x/gdbstub.c |  6 ++
  target/s390x/helper.c  |  3 +--
  target/s390x/kvm/kvm.c |  6 ++
  target/s390x/tcg/excp_helper.c | 11 +++
  target/s390x/tcg/translate.c   |  3 +--
  6 files changed, 10 insertions(+), 22 deletions(-)


Reviewed-by: Thomas Huth 




Re: [PATCH v2 11/23] target/m68k: Prefer fast cpu_env() over slower CPU QOM cast macro

2024-01-28 Thread Thomas Huth

On 26/01/2024 23.03, Philippe Mathieu-Daudé wrote:

Mechanical patch produced running the command documented
in scripts/coccinelle/cpu_env.cocci_template header.

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/m68k/cpu.c   | 30 ++
  target/m68k/gdbstub.c   |  6 ++
  target/m68k/helper.c|  3 +--
  target/m68k/m68k-semi.c |  6 ++
  target/m68k/op_helper.c | 11 +++
  target/m68k/translate.c |  3 +--
  6 files changed, 19 insertions(+), 40 deletions(-)


Reviewed-by: Thomas Huth 




Re: [PATCH] hw/scsi/lsi53c895a: add missing decrement of reentrancy counter

2024-01-28 Thread Thomas Huth

On 28/01/2024 21.22, Sven Schnelle wrote:

When the maximum count of SCRIPTS instructions is reached, the code
stops execution and returns, but fails to decrement the reentrancy
counter. This effectively renders the SCSI controller unusable
because on next entry the reentrancy counter is still above the limit.

This bug was seen on HP-UX 10.20 which seems to trigger SCRIPTS
loops.


Out of curiosity: What happened there before we introduced the 
reentrancy_level fix? Did it end up in an endless loop, or was it finishing 
at one point? In the latter case, we might need to adjust the 
"reentrancy_level > 8" to allow deeper nesting.



Fixes: b987718bbb ("hw/scsi/lsi53c895a: Fix reentrancy issues in the LSI controller 
(CVE-2023-0330)")
Signed-off-by: Sven Schnelle 
---
  hw/scsi/lsi53c895a.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 34e3b89287..d607a5f9fb 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -1159,6 +1159,7 @@ again:
  lsi_script_scsi_interrupt(s, LSI_SIST0_UDC, 0);
  lsi_disconnect(s);
  trace_lsi_execute_script_stop();
+reentrancy_level--;
  return;
  }
  insn = read_dword(s, s->dsp);


Reviewed-by: Thomas Huth 




Re: [PATCH 2/2] bulk: Prefer fast cpu_env() over slower CPU QOM cast macro

2024-01-26 Thread Thomas Huth

On 25/01/2024 17.56, Philippe Mathieu-Daudé wrote:

Mechanical patch produced running the command documented
in scripts/coccinelle/cpu_env.cocci_template header.

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/hvf/vmx.h   |  9 +++
  hw/i386/vmmouse.c   |  6 ++---
  hw/i386/xen/xen-hvm.c   |  3 +--
  hw/intc/arm_gicv3_cpuif_common.c|  3 +--
  hw/ppc/mpc8544_guts.c   |  3 +--
  hw/ppc/pnv.c|  3 +--
  hw/ppc/pnv_xscom.c  |  3 +--
  hw/ppc/ppce500_spin.c   |  3 +--
  hw/ppc/spapr.c  |  3 +--
  hw/ppc/spapr_caps.c |  6 ++---
  target/alpha/cpu.c  | 21 +--
  target/alpha/gdbstub.c  |  6 ++---
  target/alpha/helper.c   | 12 +++--
  target/alpha/mem_helper.c   |  9 +++
  target/arm/cpu.c| 15 ---
  target/arm/debug_helper.c   |  6 ++---
  target/arm/gdbstub.c|  6 ++---
  target/arm/gdbstub64.c  |  6 ++---
  target/arm/helper.c |  9 +++
  target/arm/hvf/hvf.c| 12 +++--
  target/arm/kvm.c|  3 +--
  target/arm/ptw.c|  3 +--
  target/arm/tcg/cpu32.c  |  3 +--
  target/avr/cpu.c| 21 +--
  target/avr/gdbstub.c|  6 ++---
  target/avr/helper.c |  9 +++
  target/cris/cpu.c   |  3 +--
  target/cris/gdbstub.c   |  9 +++
  target/cris/helper.c| 12 +++--
  target/cris/translate.c |  3 +--
  target/hppa/cpu.c   |  6 ++---
  target/hppa/int_helper.c|  6 ++---
  target/hppa/mem_helper.c|  3 +--
  target/i386/arch_memory_mapping.c   |  3 +--
  target/i386/cpu-dump.c  |  3 +--
  target/i386/cpu.c   | 36 +
  target/i386/helper.c| 30 +++--
  target/i386/hvf/hvf.c   |  6 ++---
  target/i386/hvf/x86.c   |  3 +--
  target/i386/hvf/x86_emu.c   |  6 ++---
  target/i386/hvf/x86_task.c  | 10 +++
  target/i386/hvf/x86hvf.c|  6 ++---
  target/i386/kvm/kvm.c   |  6 ++---
  target/i386/kvm/xen-emu.c   | 30 +++--
  target/i386/tcg/sysemu/bpt_helper.c |  3 +--
  target/i386/tcg/tcg-cpu.c   | 12 +++--
  target/i386/tcg/user/excp_helper.c  |  3 +--
  target/i386/tcg/user/seg_helper.c   |  3 +--
  target/m68k/cpu.c   | 30 +++--
  target/m68k/gdbstub.c   |  6 ++---
  target/m68k/helper.c|  3 +--
  target/m68k/m68k-semi.c |  6 ++---
  target/m68k/op_helper.c |  9 +++
  target/m68k/translate.c |  3 +--
  target/microblaze/helper.c  |  3 +--
  target/microblaze/translate.c   |  3 +--
  target/mips/cpu.c   |  9 +++
  target/mips/gdbstub.c   |  6 ++---
  target/mips/kvm.c   | 27 +++
  target/mips/sysemu/physaddr.c   |  3 +--
  target/mips/tcg/exception.c |  3 +--
  target/mips/tcg/op_helper.c |  3 +--
  target/mips/tcg/sysemu/special_helper.c |  3 +--
  target/mips/tcg/sysemu/tlb_helper.c |  6 ++---
  target/mips/tcg/translate.c |  3 +--
  target/nios2/cpu.c  |  9 +++
  target/nios2/helper.c   |  3 +--
  target/nios2/nios2-semi.c   |  6 ++---
  target/openrisc/gdbstub.c   |  3 +--
  target/openrisc/interrupt.c |  6 ++---
  target/openrisc/translate.c |  3 +--
  target/ppc/cpu_init.c   |  9 +++
  target/ppc/excp_helper.c|  3 +--
  target/ppc/gdbstub.c| 12 +++--
  target/ppc/kvm.c|  6 ++---
  target/ppc/ppc-qmp-cmds.c   |  3 +--
  target/ppc/user_only_helper.c   |  3 +--
  target/riscv/arch_dump.c|  6 ++---
  target/riscv/cpu.c  | 15 ---
  target/riscv/cpu_helper.c   | 13 +++--
  target/riscv/debug.c|  9 +++
  target/riscv/gdbstub.c  |  6 ++---
  target/riscv/kvm/kvm-cpu.c  |  6 ++---
  target/riscv/tcg/tcg-cpu.c  |  9 +++
  target/riscv/translate.c|  3 +--
  target/rx/gdbstub.c |  6 ++---
  target/rx/helper.c  |  6 ++---
  target/rx/translate.c   |  3 +--
  target/s390x/cpu-dump.c 

Re: [PATCH 2/2] bulk: Prefer fast cpu_env() over slower CPU QOM cast macro

2024-01-26 Thread Thomas Huth

On 25/01/2024 17.56, Philippe Mathieu-Daudé wrote:

Mechanical patch produced running the command documented
in scripts/coccinelle/cpu_env.cocci_template header.

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/hvf/vmx.h   |  9 +++
  hw/i386/vmmouse.c   |  6 ++---
  hw/i386/xen/xen-hvm.c   |  3 +--
  hw/intc/arm_gicv3_cpuif_common.c|  3 +--
  hw/ppc/mpc8544_guts.c   |  3 +--
  hw/ppc/pnv.c|  3 +--
  hw/ppc/pnv_xscom.c  |  3 +--
  hw/ppc/ppce500_spin.c   |  3 +--
  hw/ppc/spapr.c  |  3 +--
  hw/ppc/spapr_caps.c |  6 ++---
  target/alpha/cpu.c  | 21 +--
  target/alpha/gdbstub.c  |  6 ++---
  target/alpha/helper.c   | 12 +++--
  target/alpha/mem_helper.c   |  9 +++
  target/arm/cpu.c| 15 ---
  target/arm/debug_helper.c   |  6 ++---
  target/arm/gdbstub.c|  6 ++---
  target/arm/gdbstub64.c  |  6 ++---
  target/arm/helper.c |  9 +++
  target/arm/hvf/hvf.c| 12 +++--
  target/arm/kvm.c|  3 +--
  target/arm/ptw.c|  3 +--
  target/arm/tcg/cpu32.c  |  3 +--
  target/avr/cpu.c| 21 +--
  target/avr/gdbstub.c|  6 ++---
  target/avr/helper.c |  9 +++
  target/cris/cpu.c   |  3 +--
  target/cris/gdbstub.c   |  9 +++
  target/cris/helper.c| 12 +++--
  target/cris/translate.c |  3 +--
  target/hppa/cpu.c   |  6 ++---
  target/hppa/int_helper.c|  6 ++---
  target/hppa/mem_helper.c|  3 +--
  target/i386/arch_memory_mapping.c   |  3 +--
  target/i386/cpu-dump.c  |  3 +--
  target/i386/cpu.c   | 36 +
  target/i386/helper.c| 30 +++--
  target/i386/hvf/hvf.c   |  6 ++---
  target/i386/hvf/x86.c   |  3 +--
  target/i386/hvf/x86_emu.c   |  6 ++---
  target/i386/hvf/x86_task.c  | 10 +++
  target/i386/hvf/x86hvf.c|  6 ++---
  target/i386/kvm/kvm.c   |  6 ++---
  target/i386/kvm/xen-emu.c   | 30 +++--
  target/i386/tcg/sysemu/bpt_helper.c |  3 +--
  target/i386/tcg/tcg-cpu.c   | 12 +++--
  target/i386/tcg/user/excp_helper.c  |  3 +--
  target/i386/tcg/user/seg_helper.c   |  3 +--
  target/m68k/cpu.c   | 30 +++--
  target/m68k/gdbstub.c   |  6 ++---
  target/m68k/helper.c|  3 +--
  target/m68k/m68k-semi.c |  6 ++---
  target/m68k/op_helper.c |  9 +++
  target/m68k/translate.c |  3 +--
  target/microblaze/helper.c  |  3 +--
  target/microblaze/translate.c   |  3 +--
  target/mips/cpu.c   |  9 +++
  target/mips/gdbstub.c   |  6 ++---
  target/mips/kvm.c   | 27 +++
  target/mips/sysemu/physaddr.c   |  3 +--
  target/mips/tcg/exception.c |  3 +--
  target/mips/tcg/op_helper.c |  3 +--
  target/mips/tcg/sysemu/special_helper.c |  3 +--
  target/mips/tcg/sysemu/tlb_helper.c |  6 ++---
  target/mips/tcg/translate.c |  3 +--
  target/nios2/cpu.c  |  9 +++
  target/nios2/helper.c   |  3 +--
  target/nios2/nios2-semi.c   |  6 ++---
  target/openrisc/gdbstub.c   |  3 +--
  target/openrisc/interrupt.c |  6 ++---
  target/openrisc/translate.c |  3 +--
  target/ppc/cpu_init.c   |  9 +++
  target/ppc/excp_helper.c|  3 +--
  target/ppc/gdbstub.c| 12 +++--
  target/ppc/kvm.c|  6 ++---
  target/ppc/ppc-qmp-cmds.c   |  3 +--
  target/ppc/user_only_helper.c   |  3 +--
  target/riscv/arch_dump.c|  6 ++---
  target/riscv/cpu.c  | 15 ---
  target/riscv/cpu_helper.c   | 13 +++--
  target/riscv/debug.c|  9 +++
  target/riscv/gdbstub.c  |  6 ++---
  target/riscv/kvm/kvm-cpu.c  |  6 ++---
  target/riscv/tcg/tcg-cpu.c  |  9 +++
  target/riscv/translate.c|  3 +--
  target/rx/gdbstub.c |  6 ++---
  target/rx/helper.c  |  6 ++---
  target/rx/translate.c   |  3 +--
  target/s390x/cpu-dump.c 

Re: [PATCH 1/3] target/arm: Move v7m-related code from cpu32.c into a separate file

2024-01-26 Thread Thomas Huth

On 26/01/2024 11.44, Philippe Mathieu-Daudé wrote:

Hi Thomas,

On 26/1/24 09:39, Thomas Huth wrote:

Move the code to a separate file so that we do not have to compile
it anymore if CONFIG_ARM_V7M is not set.

Signed-off-by: Thomas Huth 
---
  target/arm/tcg/cpu-v7m.c   | 292 +
  target/arm/tcg/cpu32.c | 261 -
  target/arm/tcg/meson.build |   4 +
  3 files changed, 296 insertions(+), 261 deletions(-)
  create mode 100644 target/arm/tcg/cpu-v7m.c

diff --git a/target/arm/tcg/cpu-v7m.c b/target/arm/tcg/cpu-v7m.c
new file mode 100644
index 00..d61873ab6d
--- /dev/null
+++ b/target/arm/tcg/cpu-v7m.c
@@ -0,0 +1,292 @@
+/*
+ * QEMU ARM V7 TCG-only CPUs.


s/V7/v7M/


ok


+ *
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * This code is licensed under the GNU GPL v2 or later.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "hw/core/tcg-cpu-ops.h"
+#include "internals.h"
+#include "hw/intc/armv7m_nvic.h"
+
+/* CPU models. These are not needed for the AArch64 linux-user build. */
+#if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)


Could we do that in meson instead?


I don't think so... (see below)


diff --git a/target/arm/tcg/meson.build b/target/arm/tcg/meson.build
index 6fca38f2cc..8c7f6b43f3 100644
--- a/target/arm/tcg/meson.build
+++ b/target/arm/tcg/meson.build
@@ -52,6 +52,10 @@ arm_ss.add(when: 'TARGET_AARCH64', if_true: files(
    'sve_helper.c',
  ))
+arm_ss.add(when: 'CONFIG_ARM_V7M', if_true: files(


-> arm_system_ss


Don't we still need the code for the 32-bit qemu-arm binary? See:

$ ./qemu-arm -cpu help | grep cortex-m
  cortex-m0
  cortex-m3
  cortex-m33
  cortex-m4
  cortex-m55
  cortex-m7

So I don't think we should add this to arm_system_ss only, should we?

 Thomas



+  'cpu-v7m.c'
+))







Re: [PATCH v3 00/46] Rework matching of network devices to -nic options

2024-01-26 Thread Thomas Huth

On 25/01/2024 01.38, Jason Wang wrote:

On Wed, Jan 24, 2024 at 9:14 PM David Woodhouse  wrote:


Hi Jason,

I think this series probably lives or dies with you. I think it's a
worthwhile cleanup, but I no longer have an immediate need for it; I
shipped a slightly ugly workaround in QEMU 8.2.

Please could you let me know if it's worth persisting with it?


Yes it is.


Agreed! It would be great of getting rid of the ugly global nd_table[] finally!


Thomas, I remember you've done tweaks for -nic in the past. would you
like to review this series?


I tried to skim through the patches today, some nits here and there, but 
nothing sever, so IMHO it should be fine once they are fixed.


 HTH,
  Thomas




Re: [PATCH v3 45/46] net: remove qemu_show_nic_models(), qemu_find_nic_model()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

These old functions can be removed now too. Let net_param_nic() print
the full set of network devices directly, and also make it note that a
list more specific to this platform/config will be available by using
'-nic model=help' instead.

Signed-off-by: David Woodhouse 
---
  include/net/net.h |  3 ---
  net/net.c | 39 ++-
  2 files changed, 6 insertions(+), 36 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 1be8b40074..19fb82833c 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -203,9 +203,6 @@ void qemu_set_vnet_hdr_len(NetClientState *nc, int len);
  int qemu_set_vnet_le(NetClientState *nc, bool is_le);
  int qemu_set_vnet_be(NetClientState *nc, bool is_be);
  void qemu_macaddr_default_if_unset(MACAddr *macaddr);
-int qemu_show_nic_models(const char *arg, const char *const *models);
-int qemu_find_nic_model(NICInfo *nd, const char * const *models,
-const char *default_model);
  NICInfo *qemu_find_nic_info(const char *typename, bool match_default,
  const char *alias);
  bool qemu_configure_nic_device(DeviceState *dev, bool match_default,
diff --git a/net/net.c b/net/net.c
index ffd4b42d5a..09ab0889f5 100644
--- a/net/net.c
+++ b/net/net.c
@@ -977,38 +977,6 @@ GPtrArray *qemu_get_nic_models(const char *device_type)
  return nic_models;
  }
  
-int qemu_show_nic_models(const char *arg, const char *const *models)

-{
-int i;
-
-if (!arg || !is_help_option(arg)) {
-return 0;
-}
-
-printf("Available NIC models:\n");
-for (i = 0 ; models[i]; i++) {
-printf("%s\n", models[i]);
-}
-return 1;
-}
-
-int qemu_find_nic_model(NICInfo *nd, const char * const *models,
-const char *default_model)
-{
-int i;
-
-if (!nd->model)
-nd->model = g_strdup(default_model);
-
-for (i = 0 ; models[i]; i++) {
-if (strcmp(nd->model, models[i]) == 0)
-return i;
-}
-
-error_report("Unsupported NIC model: %s", nd->model);
-return -1;
-}
-
  static int net_init_nic(const Netdev *netdev, const char *name,
  NetClientState *peer, Error **errp)
  {
@@ -1791,9 +1759,14 @@ static int net_param_nic(void *dummy, QemuOpts *opts, 
Error **errp)
  }
  if (is_help_option(type)) {
  GPtrArray *nic_models = qemu_get_nic_models(TYPE_DEVICE);
+int i;
  show_netdevs();
  printf("\n");
-qemu_show_nic_models(type, (const char **)nic_models->pdata);
+printf("Supported NIC models "


Can we please keep "Available" instead of "Supported" ? ... since not each 
NIC is supported on each machine type...


 Thomas



+   "(use -nic model=help for a filtered list):\n");
+for (i = 0 ; nic_models->pdata[i]; i++) {
+printf("%s\n", (char *)nic_models->pdata[i]);
+}
  g_ptr_array_free(nic_models, true);
  exit(0);
  }





Re: [PATCH v3 45/46] net: remove qemu_show_nic_models(), qemu_find_nic_model()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

These old functions can be removed now too. Let net_param_nic() print
the full set of network devices directly, and also make it note that a
list more specific to this platform/config will be available by using
'-nic model=help' instead.

Signed-off-by: David Woodhouse 
---
  include/net/net.h |  3 ---
  net/net.c | 39 ++-
  2 files changed, 6 insertions(+), 36 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 1be8b40074..19fb82833c 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -203,9 +203,6 @@ void qemu_set_vnet_hdr_len(NetClientState *nc, int len);
  int qemu_set_vnet_le(NetClientState *nc, bool is_le);
  int qemu_set_vnet_be(NetClientState *nc, bool is_be);
  void qemu_macaddr_default_if_unset(MACAddr *macaddr);
-int qemu_show_nic_models(const char *arg, const char *const *models);
-int qemu_find_nic_model(NICInfo *nd, const char * const *models,
-const char *default_model);
  NICInfo *qemu_find_nic_info(const char *typename, bool match_default,
  const char *alias);
  bool qemu_configure_nic_device(DeviceState *dev, bool match_default,
diff --git a/net/net.c b/net/net.c
index ffd4b42d5a..09ab0889f5 100644
--- a/net/net.c
+++ b/net/net.c
@@ -977,38 +977,6 @@ GPtrArray *qemu_get_nic_models(const char *device_type)
  return nic_models;
  }
  
-int qemu_show_nic_models(const char *arg, const char *const *models)

-{
-int i;
-
-if (!arg || !is_help_option(arg)) {
-return 0;
-}
-
-printf("Available NIC models:\n");
-for (i = 0 ; models[i]; i++) {
-printf("%s\n", models[i]);
-}
-return 1;
-}
-
-int qemu_find_nic_model(NICInfo *nd, const char * const *models,
-const char *default_model)
-{
-int i;
-
-if (!nd->model)
-nd->model = g_strdup(default_model);
-
-for (i = 0 ; models[i]; i++) {
-if (strcmp(nd->model, models[i]) == 0)
-return i;
-}
-
-error_report("Unsupported NIC model: %s", nd->model);
-return -1;
-}
-
  static int net_init_nic(const Netdev *netdev, const char *name,
  NetClientState *peer, Error **errp)
  {
@@ -1791,9 +1759,14 @@ static int net_param_nic(void *dummy, QemuOpts *opts, 
Error **errp)
  }
  if (is_help_option(type)) {
  GPtrArray *nic_models = qemu_get_nic_models(TYPE_DEVICE);
+int i;
  show_netdevs();
  printf("\n");
-qemu_show_nic_models(type, (const char **)nic_models->pdata);
+printf("Supported NIC models "


Can we please keep "Available" instead of "Supported" ? ... since not each 
NIC is supported on each machine type...


 Thomas



+   "(use -nic model=help for a filtered list):\n");
+for (i = 0 ; nic_models->pdata[i]; i++) {
+printf("%s\n", (char *)nic_models->pdata[i]);
+}
  g_ptr_array_free(nic_models, true);
  exit(0);
  }





Re: [PATCH v3 42/46] hw/xtensa/xtfpga: use qemu_create_nic_device()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/xtensa/xtfpga.c | 13 ++---
  1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index fbad1c83a3..f49e6591dc 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -141,14 +141,16 @@ static void xtfpga_net_init(MemoryRegion *address_space,
  hwaddr base,
  hwaddr descriptors,
  hwaddr buffers,
-qemu_irq irq, NICInfo *nd)
+qemu_irq irq)
  {
  DeviceState *dev;
  SysBusDevice *s;
  MemoryRegion *ram;
  
-dev = qdev_new("open_eth");

-qdev_set_nic_properties(dev, nd);
+dev = qemu_create_nic_device("open_eth", true, NULL);
+if (!dev) {
+return;
+}
  
  s = SYS_BUS_DEVICE(dev);

  sysbus_realize_and_unref(s, _fatal);
@@ -301,10 +303,7 @@ static void xtfpga_init(const XtfpgaBoardDesc *board, 
MachineState *machine)
  memory_region_add_subregion(system_memory, board->io[1], io);
  }
  xtfpga_fpga_init(system_io, 0x0d02, freq);
-if (nd_table[0].used) {
-xtfpga_net_init(system_io, 0x0d03, 0x0d030400, 0x0d80,
-extints[1], nd_table);
-}
+xtfpga_net_init(system_io, 0x0d03, 0x0d030400, 0x0d80, extints[1]);
  
  serial_mm_init(system_io, 0x0d050020, 2, extints[0],

 115200, serial_hd(0), DEVICE_NATIVE_ENDIAN);


Reviewed-by: Thomas Huth 




Re: [PATCH v3 39/46] hw/riscv: use qemu_configure_nic_device()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/riscv/microchip_pfsoc.c | 14 ++
  hw/riscv/sifive_u.c|  7 +--
  2 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index b775aa8946..7725dfbde5 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -202,7 +202,6 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, 
Error **errp)
  MemoryRegion *envm_data = g_new(MemoryRegion, 1);
  MemoryRegion *qspi_xip_mem = g_new(MemoryRegion, 1);
  char *plic_hart_config;
-NICInfo *nd;
  int i;
  
  sysbus_realize(SYS_BUS_DEVICE(>e_cpus), _abort);

@@ -411,17 +410,8 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, 
Error **errp)
  memmap[MICROCHIP_PFSOC_USB].size);
  
  /* GEMs */

-
-nd = _table[0];
-if (nd->used) {
-qemu_check_nic_model(nd, TYPE_CADENCE_GEM);
-qdev_set_nic_properties(DEVICE(>gem0), nd);
-}
-nd = _table[1];
-if (nd->used) {
-qemu_check_nic_model(nd, TYPE_CADENCE_GEM);
-qdev_set_nic_properties(DEVICE(>gem1), nd);
-}
+qemu_configure_nic_device(DEVICE(>gem0), true, NULL);
+qemu_configure_nic_device(DEVICE(>gem1), true, NULL);
  
  object_property_set_int(OBJECT(>gem0), "revision", GEM_REVISION, errp);

  object_property_set_int(OBJECT(>gem0), "phy-addr", 8, errp);
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index ec76dce6c9..5207ec1fa5 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -789,7 +789,6 @@ static void sifive_u_soc_realize(DeviceState *dev, Error 
**errp)
  MemoryRegion *l2lim_mem = g_new(MemoryRegion, 1);
  char *plic_hart_config;
  int i, j;
-NICInfo *nd = _table[0];
  
  qdev_prop_set_uint32(DEVICE(>u_cpus), "num-harts", ms->smp.cpus - 1);

  qdev_prop_set_uint32(DEVICE(>u_cpus), "hartid-base", 1);
@@ -893,11 +892,7 @@ static void sifive_u_soc_realize(DeviceState *dev, Error 
**errp)
  }
  sysbus_mmio_map(SYS_BUS_DEVICE(>otp), 0, 
memmap[SIFIVE_U_DEV_OTP].base);
  
-/* FIXME use qdev NIC properties instead of nd_table[] */

-if (nd->used) {
-qemu_check_nic_model(nd, TYPE_CADENCE_GEM);
-qdev_set_nic_properties(DEVICE(>gem), nd);
-}
+qemu_configure_nic_device(DEVICE(>gem), true, NULL);
  object_property_set_int(OBJECT(>gem), "revision", GEM_REVISION,
      _abort);
  if (!sysbus_realize(SYS_BUS_DEVICE(>gem), errp)) {


Reviewed-by: Thomas Huth 




Re: [PATCH v3 38/46] hw/openrisc/openrisc_sim: use qemu_create_nic_device()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/openrisc/openrisc_sim.c | 18 +-
  1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index 35da123aef..bffd6f721f 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -170,7 +170,7 @@ static void openrisc_create_fdt(Or1ksimState *state,
  
  static void openrisc_sim_net_init(Or1ksimState *state, hwaddr base, hwaddr size,

int num_cpus, OpenRISCCPU *cpus[],
-  int irq_pin, NICInfo *nd)
+  int irq_pin)
  {
  void *fdt = state->fdt;
  DeviceState *dev;
@@ -178,8 +178,10 @@ static void openrisc_sim_net_init(Or1ksimState *state, 
hwaddr base, hwaddr size,
  char *nodename;
  int i;
  
-dev = qdev_new("open_eth");

-qdev_set_nic_properties(dev, nd);
+dev = qemu_create_nic_device("open_eth", true, NULL);
+if (!dev) {
+return;
+}
  
  s = SYS_BUS_DEVICE(dev);

  sysbus_realize_and_unref(s, _fatal);
@@ -313,12 +315,10 @@ static void openrisc_sim_init(MachineState *machine)
  openrisc_create_fdt(state, or1ksim_memmap, smp_cpus, machine->ram_size,
  machine->kernel_cmdline);
  
-if (nd_table[0].used) {

-openrisc_sim_net_init(state, or1ksim_memmap[OR1KSIM_ETHOC].base,
-  or1ksim_memmap[OR1KSIM_ETHOC].size,
-  smp_cpus, cpus,
-  OR1KSIM_ETHOC_IRQ, nd_table);
-}
+openrisc_sim_net_init(state, or1ksim_memmap[OR1KSIM_ETHOC].base,
+  or1ksim_memmap[OR1KSIM_ETHOC].size,
+  smp_cpus, cpus,
+  OR1KSIM_ETHOC_IRQ);
  
  if (smp_cpus > 1) {

  openrisc_sim_ompic_init(state, or1ksim_memmap[OR1KSIM_OMPIC].base,


Reviewed-by: Thomas Huth 




Re: [PATCH v3 37/46] hw/net/lasi_i82596: use qemu_configure_nic_device()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/net/lasi_i82596.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/net/lasi_i82596.c b/hw/net/lasi_i82596.c
index 6a3147fe2d..2bb4f2c4ca 100644
--- a/hw/net/lasi_i82596.c
+++ b/hw/net/lasi_i82596.c
@@ -125,11 +125,10 @@ SysBusI82596State *lasi_82596_init(MemoryRegion 
*addr_space,
  static const MACAddr HP_MAC = {
  .a = { 0x08, 0x00, 0x09, 0xef, 0x34, 0xf6 } };
  
-qemu_check_nic_model(_table[0], TYPE_LASI_82596);

  dev = qdev_new(TYPE_LASI_82596);
  s = SYSBUS_I82596(dev);
  s->state.irq = lan_irq;
-qdev_set_nic_properties(dev, _table[0]);
+qemu_configure_nic_device(dev, true, NULL);
  sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
  s->state.conf.macaddr = HP_MAC; /* set HP MAC prefix */


Reviewed-by: Thomas Huth 




Re: [PATCH v3 36/46] hw/mips/jazz: use qemu_find_nic_info()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

Extract the MAC address from the NICInfo, or generate one explicitly if
there was no corresponding NIC configuration, to put it in the PROM.


Uh, I don't see any MAC handling in the patch below? Is this the right 
comment for this patch?


 Thomas



Signed-off-by: David Woodhouse 
---
  hw/mips/jazz.c | 15 +++
  1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
index 0d2348aa5a..5bf3e328db 100644
--- a/hw/mips/jazz.c
+++ b/hw/mips/jazz.c
@@ -119,15 +119,19 @@ static const MemoryRegionOps dma_dummy_ops = {
  .endianness = DEVICE_NATIVE_ENDIAN,
  };
  
-static void mips_jazz_init_net(NICInfo *nd, IOMMUMemoryRegion *rc4030_dma_mr,

+static void mips_jazz_init_net(IOMMUMemoryRegion *rc4030_dma_mr,
 DeviceState *rc4030, MemoryRegion 
*dp8393x_prom)
  {
  DeviceState *dev;
  SysBusDevice *sysbus;
  int checksum, i;
  uint8_t *prom;
+NICInfo *nd;
  
-qemu_check_nic_model(nd, "dp83932");

+nd = qemu_find_nic_info("dp8393x", true, "dp82932");
+if (!nd) {
+return;
+}
  
  dev = qdev_new("dp8393x");

  qdev_set_nic_properties(dev, nd);
@@ -324,12 +328,7 @@ static void mips_jazz_init(MachineState *machine,
  }
  
  /* Network controller */

-if (nb_nics == 1) {
-mips_jazz_init_net(_table[0], rc4030_dma_mr, rc4030, dp8393x_prom);
-} else if (nb_nics > 1) {
-error_report("This machine only supports one NIC");
-exit(1);
-}
+mips_jazz_init_net(rc4030_dma_mr, rc4030, dp8393x_prom);
  
  /* SCSI adapter */

  dev = qdev_new(TYPE_SYSBUS_ESP);





[Bug 1225187] Re: qemu hangs in windows 7 host with -serial pipe:windbg

2024-01-26 Thread Thomas Huth
** Changed in: qemu
   Status: New => Invalid

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1225187

Title:
  qemu hangs in windows 7 host with -serial pipe:windbg

Status in QEMU:
  Invalid

Bug description:
  Execution line:
  qemu-system-i386.exe -m 512 c:\Disks\Qemu_XP_en.vhd  -serial pipe:windbg

  It waits for the pipe.
  Execute windbg
  c:\WINDDK\7600.16385.1\Debuggers\windbg.exe -k 
com:pipe,port=\\.\pipe\windbg,resets=0,reconnect

  GUI black screen shown. QEMU hangs.

  qemu v1.5.3 (c0b1a7e207094dba0b37a892b41fe4cab3195e44). MinGW built.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1225187/+subscriptions




Re: [PATCH v3 31/46] hw/net/etraxfs-eth: use qemu_configure_nic_device()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/cris/axis_dev88.c  | 9 -
  hw/net/etraxfs_eth.c  | 5 ++---
  include/hw/cris/etraxfs.h | 2 +-
  3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/hw/cris/axis_dev88.c b/hw/cris/axis_dev88.c
index d82050d927..5556634921 100644
--- a/hw/cris/axis_dev88.c
+++ b/hw/cris/axis_dev88.c
@@ -308,15 +308,14 @@ void axisdev88_init(MachineState *machine)
  
  /* Add the two ethernet blocks.  */

  dma_eth = g_malloc0(sizeof dma_eth[0] * 4); /* Allocate 4 channels.  */
-etraxfs_eth_init(_table[0], 0x30034000, 1, _eth[0], _eth[1]);
-if (nb_nics > 1) {
-etraxfs_eth_init(_table[1], 0x30036000, 2, _eth[2], 
_eth[3]);
-}
  
+etraxfs_eth_init(0x30034000, 1, _eth[0], _eth[1]);

  /* The DMA Connector block is missing, hardwire things for now.  */
  etraxfs_dmac_connect_client(etraxfs_dmac, 0, _eth[0]);
  etraxfs_dmac_connect_client(etraxfs_dmac, 1, _eth[1]);
-if (nb_nics > 1) {
+
+if (qemu_find_nic_info("etraxfs-eth", true, "fseth")) {
+etraxfs_eth_init(0x30036000, 2, _eth[2], _eth[3]);
  etraxfs_dmac_connect_client(etraxfs_dmac, 6, _eth[2]);
  etraxfs_dmac_connect_client(etraxfs_dmac, 7, _eth[3]);
  }
diff --git a/hw/net/etraxfs_eth.c b/hw/net/etraxfs_eth.c
index ba57a978d1..5faf20c782 100644
--- a/hw/net/etraxfs_eth.c
+++ b/hw/net/etraxfs_eth.c
@@ -647,15 +647,14 @@ static void etraxfs_eth_class_init(ObjectClass *klass, 
void *data)
  
  /* Instantiate an ETRAXFS Ethernet MAC.  */

  DeviceState *
-etraxfs_eth_init(NICInfo *nd, hwaddr base, int phyaddr,
+etraxfs_eth_init(hwaddr base, int phyaddr,
   struct etraxfs_dma_client *dma_out,
   struct etraxfs_dma_client *dma_in)
  {
  DeviceState *dev;
-qemu_check_nic_model(nd, "fseth");
  
  dev = qdev_new("etraxfs-eth");

-qdev_set_nic_properties(dev, nd);
+qemu_configure_nic_device(dev, true, "fseth");
  qdev_prop_set_uint32(dev, "phyaddr", phyaddr);
  
  /*

diff --git a/include/hw/cris/etraxfs.h b/include/hw/cris/etraxfs.h
index 467b529dc0..012c4e9974 100644
--- a/include/hw/cris/etraxfs.h
+++ b/include/hw/cris/etraxfs.h
@@ -31,7 +31,7 @@
  #include "hw/sysbus.h"
  #include "qapi/error.h"
  
-DeviceState *etraxfs_eth_init(NICInfo *nd, hwaddr base, int phyaddr,

+DeviceState *etraxfs_eth_init(hwaddr base, int phyaddr,
struct etraxfs_dma_client *dma_out,
    struct etraxfs_dma_client *dma_in);
  


Reviewed-by: Thomas Huth 




Re: [PATCH v3 35/46] hw/mips/mipssim: use qemu_create_nic_device()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

The MIPS SIM platform instantiates its NIC only if a corresponding
configuration exists for it. Use qemu_create_nic_device() function for
that.

Signed-off-by: David Woodhouse 
---
  hw/mips/mipssim.c | 13 +++--
  1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/mips/mipssim.c b/hw/mips/mipssim.c
index 01e323904d..16af31648e 100644
--- a/hw/mips/mipssim.c
+++ b/hw/mips/mipssim.c
@@ -118,13 +118,15 @@ static void main_cpu_reset(void *opaque)
  }
  }
  
-static void mipsnet_init(int base, qemu_irq irq, NICInfo *nd)

+static void mipsnet_init(int base, qemu_irq irq)
  {
  DeviceState *dev;
  SysBusDevice *s;
  
-dev = qdev_new("mipsnet");

-qdev_set_nic_properties(dev, nd);
+dev = qemu_create_nic_device("mipsnet", true, NULL);
+if (!dev) {
+return;
+}
  
  s = SYS_BUS_DEVICE(dev);

  sysbus_realize_and_unref(s, _fatal);
@@ -225,9 +227,8 @@ mips_mipssim_init(MachineState *machine)
sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
  }
  
-if (nd_table[0].used)

-/* MIPSnet uses the MIPS CPU INT0, which is interrupt 2. */
-mipsnet_init(0x4200, env->irq[2], _table[0]);
+/* MIPSnet uses the MIPS CPU INT0, which is interrupt 2. */
+mipsnet_init(0x4200, env->irq[2]);
  }
  
  static void mips_mipssim_machine_init(MachineClass *mc)


Reviewed-by: Thomas Huth 




Re: [PATCH v3 35/46] hw/mips/mipssim: use qemu_create_nic_device()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

The MIPS SIM platform instantiates its NIC only if a corresponding
configuration exists for it. Use qemu_create_nic_device() function for
that.

Signed-off-by: David Woodhouse 
---
  hw/mips/mipssim.c | 13 +++--
  1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/mips/mipssim.c b/hw/mips/mipssim.c
index 01e323904d..16af31648e 100644
--- a/hw/mips/mipssim.c
+++ b/hw/mips/mipssim.c
@@ -118,13 +118,15 @@ static void main_cpu_reset(void *opaque)
  }
  }
  
-static void mipsnet_init(int base, qemu_irq irq, NICInfo *nd)

+static void mipsnet_init(int base, qemu_irq irq)
  {
  DeviceState *dev;
  SysBusDevice *s;
  
-dev = qdev_new("mipsnet");

-qdev_set_nic_properties(dev, nd);
+dev = qemu_create_nic_device("mipsnet", true, NULL);
+if (!dev) {
+return;
+}
  
  s = SYS_BUS_DEVICE(dev);

  sysbus_realize_and_unref(s, _fatal);
@@ -225,9 +227,8 @@ mips_mipssim_init(MachineState *machine)
sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
  }
  
-if (nd_table[0].used)

-/* MIPSnet uses the MIPS CPU INT0, which is interrupt 2. */
-mipsnet_init(0x4200, env->irq[2], _table[0]);
+/* MIPSnet uses the MIPS CPU INT0, which is interrupt 2. */
+mipsnet_init(0x4200, env->irq[2]);
  }
  
  static void mips_mipssim_machine_init(MachineClass *mc)


Reviewed-by: Thomas Huth 




Re: [PATCH v3 46/46] net: make nb_nics and nd_table[] static in net/net.c

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 


Please add: "Also remove the leftover definition of host_net_devices which 
has been forgotten to be removed in commit 7cc28cb061040cb089." (or so)


With that:
Reviewed-by: Thomas Huth 




Signed-off-by: David Woodhouse 
---
  include/net/net.h | 4 
  net/net.c | 3 +++
  system/globals.c  | 2 --
  3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 19fb82833c..766201c62c 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -247,10 +247,6 @@ struct NICInfo {
  int nvectors;
  };
  
-extern int nb_nics;

-extern NICInfo nd_table[MAX_NICS];
-extern const char *host_net_devices[];
-
  /* from net.c */
  extern NetClientStateList net_clients;
  bool netdev_is_modern(const char *optstr);
diff --git a/net/net.c b/net/net.c
index 09ab0889f5..71cccb19da 100644
--- a/net/net.c
+++ b/net/net.c
@@ -77,6 +77,9 @@ static NetdevQueue nd_queue = 
QSIMPLEQ_HEAD_INITIALIZER(nd_queue);
  
  static GHashTable *nic_model_help;
  
+static int nb_nics;

+static NICInfo nd_table[MAX_NICS];
+
  /***/
  /* network device redirectors */
  
diff --git a/system/globals.c b/system/globals.c

index e83b5428d1..b6d4e72530 100644
--- a/system/globals.c
+++ b/system/globals.c
@@ -36,8 +36,6 @@ int display_opengl;
  const char* keyboard_layout;
  bool enable_mlock;
  bool enable_cpu_pm;
-int nb_nics;
-NICInfo nd_table[MAX_NICS];
  int autostart = 1;
  int vga_interface_type = VGA_NONE;
  bool vga_interface_created;





Re: [PATCH v3 46/46] net: make nb_nics and nd_table[] static in net/net.c

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 


Please add: "Also remove the leftover definition of host_net_devices which 
has been forgotten to be removed in commit 7cc28cb061040cb089." (or so)


With that:
Reviewed-by: Thomas Huth 




Signed-off-by: David Woodhouse 
---
  include/net/net.h | 4 
  net/net.c | 3 +++
  system/globals.c  | 2 --
  3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 19fb82833c..766201c62c 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -247,10 +247,6 @@ struct NICInfo {
  int nvectors;
  };
  
-extern int nb_nics;

-extern NICInfo nd_table[MAX_NICS];
-extern const char *host_net_devices[];
-
  /* from net.c */
  extern NetClientStateList net_clients;
  bool netdev_is_modern(const char *optstr);
diff --git a/net/net.c b/net/net.c
index 09ab0889f5..71cccb19da 100644
--- a/net/net.c
+++ b/net/net.c
@@ -77,6 +77,9 @@ static NetdevQueue nd_queue = 
QSIMPLEQ_HEAD_INITIALIZER(nd_queue);
  
  static GHashTable *nic_model_help;
  
+static int nb_nics;

+static NICInfo nd_table[MAX_NICS];
+
  /***/
  /* network device redirectors */
  
diff --git a/system/globals.c b/system/globals.c

index e83b5428d1..b6d4e72530 100644
--- a/system/globals.c
+++ b/system/globals.c
@@ -36,8 +36,6 @@ int display_opengl;
  const char* keyboard_layout;
  bool enable_mlock;
  bool enable_cpu_pm;
-int nb_nics;
-NICInfo nd_table[MAX_NICS];
  int autostart = 1;
  int vga_interface_type = VGA_NONE;
  bool vga_interface_created;





Re: [PATCH v3 44/46] hw/pci: remove pci_nic_init_nofail()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

This function is no longer used.

Signed-off-by: David Woodhouse 
---
  hw/pci/pci.c | 72 
  include/hw/pci/pci.h |  3 --
  2 files changed, 75 deletions(-)


Reviewed-by: Thomas Huth 





Re: [PATCH v3 44/46] hw/pci: remove pci_nic_init_nofail()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

This function is no longer used.

Signed-off-by: David Woodhouse 
---
  hw/pci/pci.c | 72 
  include/hw/pci/pci.h |  3 --
  2 files changed, 75 deletions(-)


Reviewed-by: Thomas Huth 





Re: [PATCH v3 43/46] net: remove qemu_check_nic_model()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 


Please add a short patch description à la "All callers have been converted 
in the previous patches, so this is not required anymore".


With that:
Reviewed-by: Thomas Huth 



Signed-off-by: David Woodhouse 
---
  include/net/net.h |  1 -
  net/net.c | 13 -
  2 files changed, 14 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 31e63d1f0d..1be8b40074 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -204,7 +204,6 @@ int qemu_set_vnet_le(NetClientState *nc, bool is_le);
  int qemu_set_vnet_be(NetClientState *nc, bool is_be);
  void qemu_macaddr_default_if_unset(MACAddr *macaddr);
  int qemu_show_nic_models(const char *arg, const char *const *models);
-void qemu_check_nic_model(NICInfo *nd, const char *model);
  int qemu_find_nic_model(NICInfo *nd, const char * const *models,
  const char *default_model);
  NICInfo *qemu_find_nic_info(const char *typename, bool match_default,
diff --git a/net/net.c b/net/net.c
index 4651b3f443..ffd4b42d5a 100644
--- a/net/net.c
+++ b/net/net.c
@@ -992,19 +992,6 @@ int qemu_show_nic_models(const char *arg, const char 
*const *models)
  return 1;
  }
  
-void qemu_check_nic_model(NICInfo *nd, const char *model)

-{
-const char *models[2];
-
-models[0] = model;
-models[1] = NULL;
-
-if (qemu_show_nic_models(nd->model, models))
-exit(0);
-if (qemu_find_nic_model(nd, models, model) < 0)
-exit(1);
-}
-
  int qemu_find_nic_model(NICInfo *nd, const char * const *models,
  const char *default_model)
  {





Re: [PATCH v3 43/46] net: remove qemu_check_nic_model()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 


Please add a short patch description à la "All callers have been converted 
in the previous patches, so this is not required anymore".


With that:
Reviewed-by: Thomas Huth 



Signed-off-by: David Woodhouse 
---
  include/net/net.h |  1 -
  net/net.c | 13 -
  2 files changed, 14 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 31e63d1f0d..1be8b40074 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -204,7 +204,6 @@ int qemu_set_vnet_le(NetClientState *nc, bool is_le);
  int qemu_set_vnet_be(NetClientState *nc, bool is_be);
  void qemu_macaddr_default_if_unset(MACAddr *macaddr);
  int qemu_show_nic_models(const char *arg, const char *const *models);
-void qemu_check_nic_model(NICInfo *nd, const char *model);
  int qemu_find_nic_model(NICInfo *nd, const char * const *models,
  const char *default_model);
  NICInfo *qemu_find_nic_info(const char *typename, bool match_default,
diff --git a/net/net.c b/net/net.c
index 4651b3f443..ffd4b42d5a 100644
--- a/net/net.c
+++ b/net/net.c
@@ -992,19 +992,6 @@ int qemu_show_nic_models(const char *arg, const char 
*const *models)
  return 1;
  }
  
-void qemu_check_nic_model(NICInfo *nd, const char *model)

-{
-const char *models[2];
-
-models[0] = model;
-models[1] = NULL;
-
-if (qemu_show_nic_models(nd->model, models))
-exit(0);
-if (qemu_find_nic_model(nd, models, model) < 0)
-exit(1);
-}
-
  int qemu_find_nic_model(NICInfo *nd, const char * const *models,
  const char *default_model)
  {





Re: [PATCH v3 34/46] hw/microblaze: use qemu_configure_nic_device()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/microblaze/petalogix_ml605_mmu.c  | 3 +--
  hw/microblaze/petalogix_s3adsp1800_mmu.c | 3 +--
  2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/microblaze/petalogix_ml605_mmu.c 
b/hw/microblaze/petalogix_ml605_mmu.c
index fb7889cf67..0f5fabc32e 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -133,7 +133,6 @@ petalogix_ml605_init(MachineState *machine)
  sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[TIMER_IRQ]);
  
  /* axi ethernet and dma initialization. */

-qemu_check_nic_model(_table[0], "xlnx.axi-ethernet");
  eth0 = qdev_new("xlnx.axi-ethernet");
  dma = qdev_new("xlnx.axi-dma");
  
@@ -145,7 +144,7 @@ petalogix_ml605_init(MachineState *machine)

"axistream-connected-target", NULL);
  cs = object_property_get_link(OBJECT(dma),
"axistream-control-connected-target", NULL);
-qdev_set_nic_properties(eth0, _table[0]);
+qemu_configure_nic_device(eth0, true, NULL);
  qdev_prop_set_uint32(eth0, "rxmem", 0x1000);
  qdev_prop_set_uint32(eth0, "txmem", 0x1000);
  object_property_set_link(OBJECT(eth0), "axistream-connected", ds,
diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c 
b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 505639c298..dad46bd7f9 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -114,9 +114,8 @@ petalogix_s3adsp1800_init(MachineState *machine)
  sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, TIMER_BASEADDR);
  sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[TIMER_IRQ]);
  
-qemu_check_nic_model(_table[0], "xlnx.xps-ethernetlite");

  dev = qdev_new("xlnx.xps-ethernetlite");
-qdev_set_nic_properties(dev, _table[0]);
+qemu_configure_nic_device(dev, true, NULL);
  qdev_prop_set_uint32(dev, "tx-ping-pong", 0);
  qdev_prop_set_uint32(dev, "rx-ping-pong", 0);
  sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);


Reviewed-by: Thomas Huth 




Re: [PATCH v3 34/46] hw/microblaze: use qemu_configure_nic_device()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/microblaze/petalogix_ml605_mmu.c  | 3 +--
  hw/microblaze/petalogix_s3adsp1800_mmu.c | 3 +--
  2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/microblaze/petalogix_ml605_mmu.c 
b/hw/microblaze/petalogix_ml605_mmu.c
index fb7889cf67..0f5fabc32e 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -133,7 +133,6 @@ petalogix_ml605_init(MachineState *machine)
  sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[TIMER_IRQ]);
  
  /* axi ethernet and dma initialization. */

-qemu_check_nic_model(_table[0], "xlnx.axi-ethernet");
  eth0 = qdev_new("xlnx.axi-ethernet");
  dma = qdev_new("xlnx.axi-dma");
  
@@ -145,7 +144,7 @@ petalogix_ml605_init(MachineState *machine)

"axistream-connected-target", NULL);
  cs = object_property_get_link(OBJECT(dma),
"axistream-control-connected-target", NULL);
-qdev_set_nic_properties(eth0, _table[0]);
+qemu_configure_nic_device(eth0, true, NULL);
  qdev_prop_set_uint32(eth0, "rxmem", 0x1000);
  qdev_prop_set_uint32(eth0, "txmem", 0x1000);
  object_property_set_link(OBJECT(eth0), "axistream-connected", ds,
diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c 
b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 505639c298..dad46bd7f9 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -114,9 +114,8 @@ petalogix_s3adsp1800_init(MachineState *machine)
  sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, TIMER_BASEADDR);
  sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[TIMER_IRQ]);
  
-qemu_check_nic_model(_table[0], "xlnx.xps-ethernetlite");

  dev = qdev_new("xlnx.xps-ethernetlite");
-qdev_set_nic_properties(dev, _table[0]);
+qemu_configure_nic_device(dev, true, NULL);
  qdev_prop_set_uint32(dev, "tx-ping-pong", 0);
  qdev_prop_set_uint32(dev, "rx-ping-pong", 0);
  sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);


Reviewed-by: Thomas Huth 




Re: [PATCH v3 32/46] hw/m68k/mcf5208: use qemu_create_nic_device()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/m68k/mcf5208.c | 19 ++-
  1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index d22d8536db..0cfb806c20 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -206,16 +206,16 @@ static void mcf5208_sys_init(MemoryRegion *address_space, 
qemu_irq *pic)
  }
  }
  
-static void mcf_fec_init(MemoryRegion *sysmem, NICInfo *nd, hwaddr base,

- qemu_irq *irqs)
+static void mcf_fec_init(MemoryRegion *sysmem, hwaddr base, qemu_irq *irqs)
  {
  DeviceState *dev;
  SysBusDevice *s;
  int i;
  
-qemu_check_nic_model(nd, TYPE_MCF_FEC_NET);

-dev = qdev_new(TYPE_MCF_FEC_NET);
-qdev_set_nic_properties(dev, nd);
+dev = qemu_create_nic_device(TYPE_MCF_FEC_NET, true, NULL);
+if (!dev) {
+return;
+}
  
  s = SYS_BUS_DEVICE(dev);

  sysbus_realize_and_unref(s, _fatal);
@@ -267,14 +267,7 @@ static void mcf5208evb_init(MachineState *machine)
  
  mcf5208_sys_init(address_space_mem, pic);
  
-if (nb_nics > 1) {

-error_report("Too many NICs");
-exit(1);
-}


I wonder whether we'd need a different mechanism to specify the maximum 
amount of on-board NICs now... Anyway, we can also think of that later, so:


Reviewed-by: Thomas Huth 





Re: [PATCH v3 32/46] hw/m68k/mcf5208: use qemu_create_nic_device()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.27, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/m68k/mcf5208.c | 19 ++-
  1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index d22d8536db..0cfb806c20 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -206,16 +206,16 @@ static void mcf5208_sys_init(MemoryRegion *address_space, 
qemu_irq *pic)
  }
  }
  
-static void mcf_fec_init(MemoryRegion *sysmem, NICInfo *nd, hwaddr base,

- qemu_irq *irqs)
+static void mcf_fec_init(MemoryRegion *sysmem, hwaddr base, qemu_irq *irqs)
  {
  DeviceState *dev;
  SysBusDevice *s;
  int i;
  
-qemu_check_nic_model(nd, TYPE_MCF_FEC_NET);

-dev = qdev_new(TYPE_MCF_FEC_NET);
-qdev_set_nic_properties(dev, nd);
+dev = qemu_create_nic_device(TYPE_MCF_FEC_NET, true, NULL);
+if (!dev) {
+return;
+}
  
  s = SYS_BUS_DEVICE(dev);

  sysbus_realize_and_unref(s, _fatal);
@@ -267,14 +267,7 @@ static void mcf5208evb_init(MachineState *machine)
  
  mcf5208_sys_init(address_space_mem, pic);
  
-if (nb_nics > 1) {

-error_report("Too many NICs");
-exit(1);
-}


I wonder whether we'd need a different mechanism to specify the maximum 
amount of on-board NICs now... Anyway, we can also think of that later, so:


Reviewed-by: Thomas Huth 





Re: [PATCH v3 27/46] hw/arm/highbank: use qemu_create_nic_device()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/arm/highbank.c | 12 +---
  1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index c21e18d08f..6a0e20e58d 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -296,19 +296,17 @@ static void calxeda_init(MachineState *machine, enum 
cxmachines machine_id)
  
  sysbus_create_simple(TYPE_SYSBUS_AHCI, 0xffe08000, pic[83]);
  
-if (nd_table[0].used) {

-qemu_check_nic_model(_table[0], "xgmac");
-dev = qdev_new("xgmac");
-qdev_set_nic_properties(dev, _table[0]);
+dev = qemu_create_nic_device("xgmac", true, NULL);
+if (dev) {
  sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
  sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xfff5);
  sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[77]);
  sysbus_connect_irq(SYS_BUS_DEVICE(dev), 1, pic[78]);
  sysbus_connect_irq(SYS_BUS_DEVICE(dev), 2, pic[79]);
+}
  
-qemu_check_nic_model(_table[1], "xgmac");

-dev = qdev_new("xgmac");
-qdev_set_nic_properties(dev, _table[1]);
+dev = qemu_create_nic_device("xgmac", true, NULL);
+if (dev) {
  sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
  sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xfff51000);
  sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[80]);


Reviewed-by: Thomas Huth 




Re: [PATCH 06/10] m68k: Clean up includes

2024-01-26 Thread Thomas Huth

On 25/01/2024 17.34, Peter Maydell wrote:

This commit was created with scripts/clean-includes:
./scripts/clean-includes --git m68k include/hw/audio/asc.h include/hw/m68k/*.h

All .c should include qemu/osdep.h first.  The script performs three
related cleanups:

* Ensure .c files include qemu/osdep.h first.
* Including it in a .h is redundant, since the .c  already includes
   it.  Drop such inclusions.
* Likewise, including headers qemu/osdep.h includes is redundant.
   Drop these, too.

Signed-off-by: Peter Maydell 
---
  include/hw/audio/asc.h  | 1 -
  include/hw/m68k/q800-glue.h | 1 -
  2 files changed, 2 deletions(-)

diff --git a/include/hw/audio/asc.h b/include/hw/audio/asc.h
index 4741f92c461..04fac270b6a 100644
--- a/include/hw/audio/asc.h
+++ b/include/hw/audio/asc.h
@@ -13,7 +13,6 @@
  #ifndef HW_AUDIO_ASC_H
  #define HW_AUDIO_ASC_H
  
-#include "qemu/osdep.h"

  #include "hw/sysbus.h"
  #include "audio/audio.h"
  
diff --git a/include/hw/m68k/q800-glue.h b/include/hw/m68k/q800-glue.h

index ceb916d16c1..04fac25f6c2 100644
--- a/include/hw/m68k/q800-glue.h
+++ b/include/hw/m68k/q800-glue.h
@@ -23,7 +23,6 @@
  #ifndef HW_Q800_GLUE_H
  #define HW_Q800_GLUE_H
  
-#include "qemu/osdep.h"

  #include "hw/sysbus.h"
  
  #define TYPE_GLUE "q800-glue"


Reviewed-by: Thomas Huth 




Re: [PATCH v3 10/46] hw/hppa: use pci_init_nic_devices()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/hppa/machine.c | 7 ++-
  1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index c8da7c18d5..19d477105e 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -338,7 +338,6 @@ static void machine_HP_common_init_tail(MachineState 
*machine, PCIBus *pci_bus,
  uint64_t kernel_entry = 0, kernel_low, kernel_high;
  MemoryRegion *addr_space = get_system_memory();
  MemoryRegion *rom_region;
-long i;
  unsigned int smp_cpus = machine->smp.cpus;
  SysBusDevice *s;
  
@@ -362,10 +361,8 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,

  qdev_get_gpio_in(lasi_dev, LASI_IRQ_LAN_HPA));
  }
  
-for (i = 0; i < nb_nics; i++) {

-if (!enable_lasi_lan()) {
-pci_nic_init_nofail(_table[i], pci_bus, mc->default_nic, NULL);
-}
+if (!enable_lasi_lan()) {
+pci_init_nic_devices(pci_bus, mc->default_nic);
  }
  
  /* BMC board: HP Powerbar SP2 Diva (with console only) */


Reviewed-by: Thomas Huth 




Re: [PATCH v3 09/46] hw/arm/virt: use pci_init_nic_devices()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/arm/virt.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 2793121cb4..8cf9237001 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1454,9 +1454,7 @@ static void create_pcie(VirtMachineState *vms)
  pci->bypass_iommu = vms->default_bus_bypass_iommu;
  vms->bus = pci->bus;
  if (vms->bus) {
-for (i = 0; i < nb_nics; i++) {
-pci_nic_init_nofail(_table[i], pci->bus, mc->default_nic, NULL);
-}
+pci_init_nic_devices(pci->bus, mc->default_nic);
  }
  
  nodename = vms->pciehb_nodename = g_strdup_printf("/pcie@%" PRIx64, base);


Reviewed-by: Thomas Huth 




Re: [PATCH v3 07/46] hw/alpha/dp264: use pci_init_nic_devices()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/alpha/dp264.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 03495e1e60..52a1fa310b 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -124,9 +124,7 @@ static void clipper_init(MachineState *machine)
  pci_vga_init(pci_bus);
  
  /* Network setup.  e1000 is good enough, failing Tulip support.  */

-for (i = 0; i < nb_nics; i++) {
-pci_nic_init_nofail(_table[i], pci_bus, mc->default_nic, NULL);
-}
+pci_init_nic_devices(pci_bus, mc->default_nic);
  
  /* Super I/O */

  isa_create_simple(isa_bus, TYPE_SMC37C669_SUPERIO);


Reviewed-by: Thomas Huth 




Re: [PATCH v3 25/46] hw/net/smc91c111: use qemu_configure_nic_device()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Some callers instantiate the device unconditionally, others will do so only
if there is a NICInfo to go with it. This appears to be fairly random, but
preserve the existing behaviour for now.

Signed-off-by: David Woodhouse 
---

...

diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c
index 49b7c26102..702d0e8e83 100644
--- a/hw/net/smc91c111.c
+++ b/hw/net/smc91c111.c
@@ -818,14 +818,13 @@ static void smc91c111_register_types(void)
  
  /* Legacy helper function.  Should go away when machine config files are

 implemented.  */
-void smc91c111_init(NICInfo *nd, uint32_t base, qemu_irq irq)
+void smc91c111_init(uint32_t base, qemu_irq irq)
  {
  DeviceState *dev;
  SysBusDevice *s;
  
-qemu_check_nic_model(nd, "smc91c111");

  dev = qdev_new(TYPE_SMC91C111);
-qdev_set_nic_properties(dev, nd);
+qemu_configure_nic_device(dev, true, NULL);


Wouldn't it be possible to use qemu_create_nic_device() here, too?

Anyway:
Reviewed-by: Thomas Huth 



  s = SYS_BUS_DEVICE(dev);
  sysbus_realize_and_unref(s, _fatal);
  sysbus_mmio_map(s, 0, base);





Re: [PATCH v3 24/46] hw/arm/fsl: use qemu_configure_nic_device()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/arm/fsl-imx25.c  | 2 +-
  hw/arm/fsl-imx6.c   | 2 +-
  hw/arm/fsl-imx6ul.c | 2 +-
  hw/arm/fsl-imx7.c   | 2 +-
  4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
index 9d2fb75a68..a24fa7b443 100644
--- a/hw/arm/fsl-imx25.c
+++ b/hw/arm/fsl-imx25.c
@@ -170,7 +170,7 @@ static void fsl_imx25_realize(DeviceState *dev, Error 
**errp)
  
  object_property_set_uint(OBJECT(>fec), "phy-num", s->phy_num,

   _abort);
-qdev_set_nic_properties(DEVICE(>fec), _table[0]);
+qemu_configure_nic_device(DEVICE(>fec), true, NULL);
  
  if (!sysbus_realize(SYS_BUS_DEVICE(>fec), errp)) {

  return;
diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
index b2153022c0..02f3024090 100644
--- a/hw/arm/fsl-imx6.c
+++ b/hw/arm/fsl-imx6.c
@@ -380,7 +380,7 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp)
  
  object_property_set_uint(OBJECT(>eth), "phy-num", s->phy_num,

   _abort);
-qdev_set_nic_properties(DEVICE(>eth), _table[0]);
+qemu_configure_nic_device(DEVICE(>eth), true, NULL);
  if (!sysbus_realize(SYS_BUS_DEVICE(>eth), errp)) {
  return;
  }
diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
index e37b69a5e1..ca3dd439ec 100644
--- a/hw/arm/fsl-imx6ul.c
+++ b/hw/arm/fsl-imx6ul.c
@@ -442,7 +442,7 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error 
**errp)
   s->phy_num[i], _abort);
  object_property_set_uint(OBJECT(>eth[i]), "tx-ring-num",
   FSL_IMX6UL_ETH_NUM_TX_RINGS, _abort);
-qdev_set_nic_properties(DEVICE(>eth[i]), _table[i]);
+qemu_configure_nic_device(DEVICE(>eth[i]), true, NULL);
  sysbus_realize(SYS_BUS_DEVICE(>eth[i]), _abort);
  
  sysbus_mmio_map(SYS_BUS_DEVICE(>eth[i]), 0,

diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
index 474cfdc87c..1acbe065db 100644
--- a/hw/arm/fsl-imx7.c
+++ b/hw/arm/fsl-imx7.c
@@ -446,7 +446,7 @@ static void fsl_imx7_realize(DeviceState *dev, Error **errp)
   s->phy_num[i], _abort);
  object_property_set_uint(OBJECT(>eth[i]), "tx-ring-num",
   FSL_IMX7_ETH_NUM_TX_RINGS, _abort);
-qdev_set_nic_properties(DEVICE(>eth[i]), _table[i]);
+qemu_configure_nic_device(DEVICE(>eth[i]), true, NULL);
  sysbus_realize(SYS_BUS_DEVICE(>eth[i]), _abort);
  
  sysbus_mmio_map(SYS_BUS_DEVICE(>eth[i]), 0, FSL_IMX7_ENETn_ADDR[i]);


Reviewed-by: Thomas Huth 




Re: [PATCH v3 23/46] hw/arm/exynos4: use qemu_create_nic_device()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/arm/exynos4_boards.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
index b0e13eb4f0..003992189b 100644
--- a/hw/arm/exynos4_boards.c
+++ b/hw/arm/exynos4_boards.c
@@ -76,10 +76,8 @@ static void lan9215_init(uint32_t base, qemu_irq irq)
  SysBusDevice *s;
  
  /* This should be a 9215 but the 9118 is close enough */

-if (nd_table[0].used) {
-qemu_check_nic_model(_table[0], "lan9118");
-dev = qdev_new(TYPE_LAN9118);
-qdev_set_nic_properties(dev, _table[0]);
+dev = qemu_create_nic_device(TYPE_LAN9118, true, NULL);
+if (dev) {
  qdev_prop_set_uint32(dev, "mode_16bit", 1);
  s = SYS_BUS_DEVICE(dev);
  sysbus_realize_and_unref(s, _fatal);


Reviewed-by: Thomas Huth 




Re: [PATCH v3 21/46] hw/arm/allwinner: use qemu_configure_nic_device()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/arm/allwinner-a10.c |  6 +-
  hw/arm/allwinner-h3.c  |  6 +-
  hw/arm/allwinner-r40.c | 27 ++-
  3 files changed, 4 insertions(+), 35 deletions(-)

diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
index b0ea3f7f66..57f52871ec 100644
--- a/hw/arm/allwinner-a10.c
+++ b/hw/arm/allwinner-a10.c
@@ -142,11 +142,7 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
  sysbus_realize(SYS_BUS_DEVICE(>dramc), _fatal);
  sysbus_mmio_map(SYS_BUS_DEVICE(>dramc), 0, AW_A10_DRAMC_BASE);
  
-/* FIXME use qdev NIC properties instead of nd_table[] */

-if (nd_table[0].used) {
-qemu_check_nic_model(_table[0], TYPE_AW_EMAC);
-qdev_set_nic_properties(DEVICE(>emac), _table[0]);
-}
+qemu_configure_nic_device(DEVICE(>emac), true, NULL);
  if (!sysbus_realize(SYS_BUS_DEVICE(>emac), errp)) {
  return;
  }
diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
index f05afddf7e..4f102ad082 100644
--- a/hw/arm/allwinner-h3.c
+++ b/hw/arm/allwinner-h3.c
@@ -369,11 +369,7 @@ static void allwinner_h3_realize(DeviceState *dev, Error 
**errp)
"sd-bus");
  
  /* EMAC */

-/* FIXME use qdev NIC properties instead of nd_table[] */
-if (nd_table[0].used) {
-qemu_check_nic_model(_table[0], TYPE_AW_SUN8I_EMAC);
-qdev_set_nic_properties(DEVICE(>emac), _table[0]);
-}
+qemu_configure_nic_device(DEVICE(>emac), true, NULL);
  object_property_set_link(OBJECT(>emac), "dma-memory",
   OBJECT(get_system_memory()), _fatal);
  sysbus_realize(SYS_BUS_DEVICE(>emac), _fatal);
diff --git a/hw/arm/allwinner-r40.c b/hw/arm/allwinner-r40.c
index a0d367c60d..4d5661b014 100644
--- a/hw/arm/allwinner-r40.c
+++ b/hw/arm/allwinner-r40.c
@@ -294,7 +294,6 @@ static void allwinner_r40_init(Object *obj)
  
  static void allwinner_r40_realize(DeviceState *dev, Error **errp)

  {
-const char *r40_nic_models[] = { "gmac", "emac", NULL };
  AwR40State *s = AW_R40(dev);
  
  /* CPUs */

@@ -454,31 +453,8 @@ static void allwinner_r40_realize(DeviceState *dev, Error 
**errp)
  sysbus_mmio_map(SYS_BUS_DEVICE(>dramc), 2,
  s->memmap[AW_R40_DEV_DRAMPHY]);
  
-/* nic support gmac and emac */

-for (int i = 0; i < ARRAY_SIZE(r40_nic_models) - 1; i++) {
-NICInfo *nic = _table[i];
-
-if (!nic->used) {
-continue;
-}
-if (qemu_show_nic_models(nic->model, r40_nic_models)) {
-exit(0);
-}
-
-switch (qemu_find_nic_model(nic, r40_nic_models, r40_nic_models[0])) {
-case 0: /* gmac */
-qdev_set_nic_properties(DEVICE(>gmac), nic);
-break;
-case 1: /* emac */
-qdev_set_nic_properties(DEVICE(>emac), nic);
-break;
-default:
-exit(1);
-break;
-}
-}
-
  /* GMAC */
+qemu_configure_nic_device(DEVICE(>gmac), true, "gmac");
  object_property_set_link(OBJECT(>gmac), "dma-memory",
   OBJECT(get_system_memory()), 
_fatal);
  sysbus_realize(SYS_BUS_DEVICE(>gmac), _fatal);
@@ -487,6 +463,7 @@ static void allwinner_r40_realize(DeviceState *dev, Error 
**errp)
 qdev_get_gpio_in(DEVICE(>gic), 
AW_R40_GIC_SPI_GMAC));
  
  /* EMAC */

+qemu_configure_nic_device(DEVICE(>emac), true, "emac");
  sysbus_realize(SYS_BUS_DEVICE(>emac), _fatal);
  sysbus_mmio_map(SYS_BUS_DEVICE(>emac), 0, s->memmap[AW_R40_DEV_EMAC]);
  sysbus_connect_irq(SYS_BUS_DEVICE(>emac), 0,


Reviewed-by: Thomas Huth 




Re: [PATCH v3 20/46] hw/xtensa/virt: use pci_init_nic_devices()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/xtensa/virt.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/xtensa/virt.c b/hw/xtensa/virt.c
index a6cf646e99..5310a88861 100644
--- a/hw/xtensa/virt.c
+++ b/hw/xtensa/virt.c
@@ -102,9 +102,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++) {
-pci_nic_init_nofail(_table[i], pci->bus, mc->default_nic, NULL);
-}
+pci_init_nic_devices(pci->bus, mc->default_nic);
  }
  }


Reviewed-by: Thomas Huth 





Re: [PATCH v3 03/46] net: add qemu_create_nic_bus_devices()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

This will instantiate any NICs which live on a given bus type. Each bus
is allowed *one* substitution (for PCI it's virtio → virtio-net-pci, for
Xen it's xen → xen-net-device; no point in overengineering it unless we
actually want more).

Signed-off-by: David Woodhouse 
Reviewed-by: Paul Durrant 
---
  include/net/net.h |  3 +++
  net/net.c | 53 +++
  2 files changed, 56 insertions(+)


Reviewed-by: Thomas Huth 





Re: [PATCH v3 03/46] net: add qemu_create_nic_bus_devices()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

This will instantiate any NICs which live on a given bus type. Each bus
is allowed *one* substitution (for PCI it's virtio → virtio-net-pci, for
Xen it's xen → xen-net-device; no point in overengineering it unless we
actually want more).

Signed-off-by: David Woodhouse 
Reviewed-by: Paul Durrant 
---
  include/net/net.h |  3 +++
  net/net.c | 53 +++
  2 files changed, 56 insertions(+)


Reviewed-by: Thomas Huth 





Re: [PATCH v3 01/46] net: add qemu_{configure, create}_nic_device(), qemu_find_nic_info()

2024-01-26 Thread Thomas Huth

On 26/01/2024 15.34, David Woodhouse wrote:

On Fri, 2024-01-26 at 15:24 +0100, Thomas Huth wrote:

On 26/01/2024 15.16, David Woodhouse wrote:

On Fri, 2024-01-26 at 12:10 +0100, Thomas Huth wrote:



+/* "Please create a device, if you have a configuration for it" */
+DeviceState *qemu_create_nic_device(const char *typename, bool match_default,
+    const char *alias)
+{
+    NICInfo *nd = qemu_find_nic_info(typename, match_default, alias);
+    DeviceState *dev;
+
+    if (!nd) {
+    return NULL;
+    }


The qemu_check_nic_model() function that was used in some code that you
turned into qemu_create_nic_device() used to set:

   if (!nd->model)
   nd->model = g_strdup(default_model);

(in the qemu_find_nic_model() function that has been called by
qemu_check_nic_model())

Should we do that also here to make sure that nd->model is not NULL afterwards?


Good question, but I don't think we care. The qdev_set_nic_properties()
function certainly doesn't propagate nd->model to anywhere.

I renamed nd->model to nd->modelname in a patch shown below, just to be
100% sure I'm not missing any other code paths which might consume it.


Ok, thanks for checking! Maybe mention it in the patch description in v4, so
that we've got it recorded somewhere that nd->model might be left at NULL
afterwards, but that there are no further consumers, so it should be fine.


Makes sense.

https://git.infradead.org/?p=users/dwmw2/qemu.git;a=commitdiff;h=694f82bc09080
now says:


net: add qemu_{configure,create}_nic_device(), qemu_find_nic_info()

Most code which directly accesses nd_table[] and nb_nics uses them for
one of two things. Either "I have created a NIC device and I'd like a
configuration for it", or "I will create a NIC device *if* there is a
configuration for it".  With some variants on the theme around whether
they actually *check* if the model specified in the configuration is
the right one.

Provide functions which perform both of those, allowing platforms to
be a little more consistent and as a step towards making nd_table[]
and nb_nics private to the net code.

One might argue that platforms ought to be consistent about whether
they create the unconfigured devices or not, but making significant
user-visible changes is explicitly *not* the intent right now.

The new functions leave the 'model' field of the NICInfo as NULL after
using it for the default NIC model, unlike the qemu_check_nic_model()
function which does set nd->model to match default_model explicitly.
This is acceptable because there is no code which consumes nd->model
except this NIC-matching code in net/net.c, and no reasonable excuse
for any code wanting to use nd->model in future.


With that feel free to add:
Reviewed-by: Thomas Huth 





Re: [PATCH v3 01/46] net: add qemu_{configure, create}_nic_device(), qemu_find_nic_info()

2024-01-26 Thread Thomas Huth

On 26/01/2024 15.34, David Woodhouse wrote:

On Fri, 2024-01-26 at 15:24 +0100, Thomas Huth wrote:

On 26/01/2024 15.16, David Woodhouse wrote:

On Fri, 2024-01-26 at 12:10 +0100, Thomas Huth wrote:



+/* "Please create a device, if you have a configuration for it" */
+DeviceState *qemu_create_nic_device(const char *typename, bool match_default,
+    const char *alias)
+{
+    NICInfo *nd = qemu_find_nic_info(typename, match_default, alias);
+    DeviceState *dev;
+
+    if (!nd) {
+    return NULL;
+    }


The qemu_check_nic_model() function that was used in some code that you
turned into qemu_create_nic_device() used to set:

   if (!nd->model)
   nd->model = g_strdup(default_model);

(in the qemu_find_nic_model() function that has been called by
qemu_check_nic_model())

Should we do that also here to make sure that nd->model is not NULL afterwards?


Good question, but I don't think we care. The qdev_set_nic_properties()
function certainly doesn't propagate nd->model to anywhere.

I renamed nd->model to nd->modelname in a patch shown below, just to be
100% sure I'm not missing any other code paths which might consume it.


Ok, thanks for checking! Maybe mention it in the patch description in v4, so
that we've got it recorded somewhere that nd->model might be left at NULL
afterwards, but that there are no further consumers, so it should be fine.


Makes sense.

https://git.infradead.org/?p=users/dwmw2/qemu.git;a=commitdiff;h=694f82bc09080
now says:


net: add qemu_{configure,create}_nic_device(), qemu_find_nic_info()

Most code which directly accesses nd_table[] and nb_nics uses them for
one of two things. Either "I have created a NIC device and I'd like a
configuration for it", or "I will create a NIC device *if* there is a
configuration for it".  With some variants on the theme around whether
they actually *check* if the model specified in the configuration is
the right one.

Provide functions which perform both of those, allowing platforms to
be a little more consistent and as a step towards making nd_table[]
and nb_nics private to the net code.

One might argue that platforms ought to be consistent about whether
they create the unconfigured devices or not, but making significant
user-visible changes is explicitly *not* the intent right now.

The new functions leave the 'model' field of the NICInfo as NULL after
using it for the default NIC model, unlike the qemu_check_nic_model()
function which does set nd->model to match default_model explicitly.
This is acceptable because there is no code which consumes nd->model
except this NIC-matching code in net/net.c, and no reasonable excuse
for any code wanting to use nd->model in future.


With that feel free to add:
Reviewed-by: Thomas Huth 





Re: [PATCH v3 02/46] net: report list of available models according to platform

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

By noting the models for which a configuration was requested, we can give
the user an accurate list of which NIC models were actually available on
the platform/configuration that was otherwise chosen.

Signed-off-by: David Woodhouse 
Reviewed-by: Paul Durrant 
---
  net/net.c | 94 +++
  1 file changed, 94 insertions(+)

diff --git a/net/net.c b/net/net.c
index aeb7f573fc..962904eaef 100644
--- a/net/net.c
+++ b/net/net.c
@@ -75,6 +75,8 @@ typedef QSIMPLEQ_HEAD(, NetdevQueueEntry) NetdevQueue;
  
  static NetdevQueue nd_queue = QSIMPLEQ_HEAD_INITIALIZER(nd_queue);
  
+static GHashTable *nic_model_help;

+
  /***/
  /* network device redirectors */
  
@@ -1087,12 +1089,94 @@ static int net_init_nic(const Netdev *netdev, const char *name,

  return idx;
  }
  
+static gboolean add_nic_result(gpointer key, gpointer value, gpointer user_data)

+{
+GPtrArray *results = user_data;
+GPtrArray *alias_list = value;
+const char *model = key;
+char *result;
+
+if (!alias_list) {
+result = g_strdup(model);
+} else {
+GString *result_str = g_string_new(model);
+int i;
+
+g_string_append(result_str, " (aka ");


It's an abbreviation, so I'd rather use "a.k.a." instead of "aka".

Apart from that, the patch looks reasonable to me.

 Thomas


+for (i = 0; i < alias_list->len; i++) {
+if (i) {
+g_string_append(result_str, ", ");
+}
+g_string_append(result_str, alias_list->pdata[i]);
+}
+g_string_append(result_str, ")");
+result = result_str->str;
+g_string_free(result_str, false);
+g_ptr_array_unref(alias_list);
+}
+g_ptr_array_add(results, result);
+return true;
+}

...




Re: [PATCH v3 02/46] net: report list of available models according to platform

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

By noting the models for which a configuration was requested, we can give
the user an accurate list of which NIC models were actually available on
the platform/configuration that was otherwise chosen.

Signed-off-by: David Woodhouse 
Reviewed-by: Paul Durrant 
---
  net/net.c | 94 +++
  1 file changed, 94 insertions(+)

diff --git a/net/net.c b/net/net.c
index aeb7f573fc..962904eaef 100644
--- a/net/net.c
+++ b/net/net.c
@@ -75,6 +75,8 @@ typedef QSIMPLEQ_HEAD(, NetdevQueueEntry) NetdevQueue;
  
  static NetdevQueue nd_queue = QSIMPLEQ_HEAD_INITIALIZER(nd_queue);
  
+static GHashTable *nic_model_help;

+
  /***/
  /* network device redirectors */
  
@@ -1087,12 +1089,94 @@ static int net_init_nic(const Netdev *netdev, const char *name,

  return idx;
  }
  
+static gboolean add_nic_result(gpointer key, gpointer value, gpointer user_data)

+{
+GPtrArray *results = user_data;
+GPtrArray *alias_list = value;
+const char *model = key;
+char *result;
+
+if (!alias_list) {
+result = g_strdup(model);
+} else {
+GString *result_str = g_string_new(model);
+int i;
+
+g_string_append(result_str, " (aka ");


It's an abbreviation, so I'd rather use "a.k.a." instead of "aka".

Apart from that, the patch looks reasonable to me.

 Thomas


+for (i = 0; i < alias_list->len; i++) {
+if (i) {
+g_string_append(result_str, ", ");
+}
+g_string_append(result_str, alias_list->pdata[i]);
+}
+g_string_append(result_str, ")");
+result = result_str->str;
+g_string_free(result_str, false);
+g_ptr_array_unref(alias_list);
+}
+g_ptr_array_add(results, result);
+return true;
+}

...




Re: [PATCH v3 01/46] net: add qemu_{configure, create}_nic_device(), qemu_find_nic_info()

2024-01-26 Thread Thomas Huth

On 26/01/2024 15.16, David Woodhouse wrote:

On Fri, 2024-01-26 at 12:10 +0100, Thomas Huth wrote:



+/* "Please create a device, if you have a configuration for it" */
+DeviceState *qemu_create_nic_device(const char *typename, bool match_default,
+    const char *alias)
+{
+    NICInfo *nd = qemu_find_nic_info(typename, match_default, alias);
+    DeviceState *dev;
+
+    if (!nd) {
+    return NULL;
+    }


The qemu_check_nic_model() function that was used in some code that you
turned into qemu_create_nic_device() used to set:

  if (!nd->model)
  nd->model = g_strdup(default_model);

(in the qemu_find_nic_model() function that has been called by
qemu_check_nic_model())

Should we do that also here to make sure that nd->model is not NULL afterwards?


Good question, but I don't think we care. The qdev_set_nic_properties()
function certainly doesn't propagate nd->model to anywhere.

I renamed nd->model to nd->modelname in a patch shown below, just to be
100% sure I'm not missing any other code paths which might consume it.


Ok, thanks for checking! Maybe mention it in the patch description in v4, so 
that we've got it recorded somewhere that nd->model might be left at NULL 
afterwards, but that there are no further consumers, so it should be fine?


 Thomas





Re: [PATCH v3 01/46] net: add qemu_{configure, create}_nic_device(), qemu_find_nic_info()

2024-01-26 Thread Thomas Huth

On 26/01/2024 15.16, David Woodhouse wrote:

On Fri, 2024-01-26 at 12:10 +0100, Thomas Huth wrote:



+/* "Please create a device, if you have a configuration for it" */
+DeviceState *qemu_create_nic_device(const char *typename, bool match_default,
+    const char *alias)
+{
+    NICInfo *nd = qemu_find_nic_info(typename, match_default, alias);
+    DeviceState *dev;
+
+    if (!nd) {
+    return NULL;
+    }


The qemu_check_nic_model() function that was used in some code that you
turned into qemu_create_nic_device() used to set:

  if (!nd->model)
  nd->model = g_strdup(default_model);

(in the qemu_find_nic_model() function that has been called by
qemu_check_nic_model())

Should we do that also here to make sure that nd->model is not NULL afterwards?


Good question, but I don't think we care. The qdev_set_nic_properties()
function certainly doesn't propagate nd->model to anywhere.

I renamed nd->model to nd->modelname in a patch shown below, just to be
100% sure I'm not missing any other code paths which might consume it.


Ok, thanks for checking! Maybe mention it in the patch description in v4, so 
that we've got it recorded somewhere that nd->model might be left at NULL 
afterwards, but that there are no further consumers, so it should be fine?


 Thomas





Re: [PATCH v3 19/46] hw/sparc64/sun4u: use pci_init_nic_devices()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

The first sunhme NIC gets placed a function 1 on slot 1 of PCI bus A,
and the rest are dynamically assigned on PCI bus B.

Previously, any PCI NIC would get the special treatment purely by
virtue of being first in the list.

Signed-off-by: David Woodhouse 
---
  hw/sparc64/sun4u.c | 27 ---
  1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 24d53bf5fd..eda9b58a21 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -639,29 +639,18 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
  
  memset(, 0, sizeof(MACAddr));

  onboard_nic = false;
-for (i = 0; i < nb_nics; i++) {
-PCIBus *bus;
-nd = _table[i];
-
-if (!nd->model || strcmp(nd->model, mc->default_nic) == 0) {
-if (!onboard_nic) {
-pci_dev = pci_new_multifunction(PCI_DEVFN(1, 1), 
mc->default_nic);
-bus = pci_busA;
-memcpy(, >macaddr.a, sizeof(MACAddr));
-onboard_nic = true;
-} else {
-pci_dev = pci_new(-1, mc->default_nic);
-bus = pci_busB;
-}
-} else {
-pci_dev = pci_new(-1, nd->model);
-bus = pci_busB;
-}
  
+nd = qemu_find_nic_info(mc->default_nic, true, NULL);

+if (nd) {
+pci_dev = pci_new_multifunction(PCI_DEVFN(1, 1), mc->default_nic);
  dev = _dev->qdev;
  qdev_set_nic_properties(dev, nd);
-pci_realize_and_unref(pci_dev, bus, _fatal);
+pci_realize_and_unref(pci_dev, pci_busA, _fatal);
+
+memcpy(, >macaddr.a, sizeof(MACAddr));
+onboard_nic = true;
  }
+pci_init_nic_devices(pci_busB, mc->default_nic);
  
  /* If we don't have an onboard NIC, grab a default MAC address so that

   * we have a valid machine id */


Reviewed-by: Thomas Huth 




Re: [PATCH v3 17/46] hw/ppc: use pci_init_nic_devices()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/ppc/e500.c  |  4 +---
  hw/ppc/mac_newworld.c  |  4 +---
  hw/ppc/mac_oldworld.c  |  4 +---
  hw/ppc/ppc440_bamboo.c | 14 +-
  4 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 566f1200dd..3bd12b54ab 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -1079,9 +1079,7 @@ void ppce500_init(MachineState *machine)
  
  if (pci_bus) {

  /* Register network interfaces. */
-for (i = 0; i < nb_nics; i++) {
-pci_nic_init_nofail(_table[i], pci_bus, mc->default_nic, NULL);
-}
+pci_init_nic_devices(pci_bus, mc->default_nic);
  }
  
  /* Register spinning region */

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 535710314a..b36dbaf2b6 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -444,9 +444,7 @@ static void ppc_core99_init(MachineState *machine)
  graphic_depth = 15;
  }
  
-for (i = 0; i < nb_nics; i++) {

-pci_nic_init_nofail(_table[i], pci_bus, mc->default_nic, NULL);
-}
+pci_init_nic_devices(pci_bus, mc->default_nic);
  
  /* The NewWorld NVRAM is not located in the MacIO device */

  if (kvm_enabled() && qemu_real_host_page_size() > 4096) {
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 9acc7adfc9..1981d3d8f6 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -277,9 +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(_table[i], pci_bus, mc->default_nic, NULL);
-}
+pci_init_nic_devices(pci_bus, mc->default_nic);
  
  /* MacIO IDE */

  ide_drive_get(hd, ARRAY_SIZE(hd));
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index a189942de4..c75c3083e6 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -161,7 +161,6 @@ static void bamboo_init(MachineState *machine)
  DeviceState *uicdev;
  SysBusDevice *uicsbd;
  int success;
-int i;
  
  if (kvm_enabled()) {

  error_report("machine %s does not support the KVM accelerator",
@@ -234,14 +233,11 @@ static void bamboo_init(MachineState *machine)
  }
  
  if (pcibus) {

-/* Register network interfaces. */
-for (i = 0; i < nb_nics; i++) {
-/*
- * 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(_table[i], pcibus, mc->default_nic, NULL);
-}
+/*
+ * There are no PCI NICs on the Bamboo board, but there are
+ * PCI slots, so we can pick whatever default model we want.
+ */
+pci_init_nic_devices(pcibus, mc->default_nic);
  }
  
  /* Load kernel. */


Reviewed-by: Thomas Huth 




Re: [PATCH v3 16/46] hw/ppc/spapr: use qemu_get_nic_info() and pci_init_nic_devices()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Avoid directly referencing nd_table[] by first instantiating any
spapr-vlan devices using a qemu_get_nic_info() loop, then calling
pci_init_nic_devices() to do the rest.

No functional change intended.

Signed-off-by: David Woodhouse 
---
  hw/ppc/spapr.c | 18 +-
  1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4997aa4f1d..37604e7caf 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2796,6 +2796,7 @@ static void spapr_machine_init(MachineState *machine)
  MemoryRegion *sysmem = get_system_memory();
  long load_limit, fw_size;
  Error *resize_hpt_err = NULL;
+NICInfo *nd;
  
  if (!filename) {

  error_report("Could not find LPAR firmware '%s'", bios_name);
@@ -2996,21 +2997,12 @@ static void spapr_machine_init(MachineState *machine)
  
  phb = spapr_create_default_phb();
  
-for (i = 0; i < nb_nics; i++) {

-NICInfo *nd = _table[i];
-
-if (!nd->model) {
-nd->model = g_strdup("spapr-vlan");
-}
-
-if (g_str_equal(nd->model, "spapr-vlan") ||
-g_str_equal(nd->model, "ibmveth")) {
-spapr_vlan_create(spapr->vio_bus, nd);
-} else {
-pci_nic_init_nofail(_table[i], phb->bus, nd->model, NULL);
-}
+while ((nd = qemu_find_nic_info("spapr-vlan", true, "ibmveth"))) {
+spapr_vlan_create(spapr->vio_bus, nd);
  }
  
+pci_init_nic_devices(phb->bus, NULL);

+
  for (i = 0; i <= drive_get_max_bus(IF_SCSI); i++) {
  spapr_vscsi_create(spapr->vio_bus);
  }


Reviewed-by: Thomas Huth 




Re: [PATCH v3 15/46] hw/ppc/prep: use pci_init_nic_devices()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Previously, the first PCI NIC would be placed in PCI slot 3 and the rest
would be dynamically assigned. Even if the user overrode the default NIC
type and made it something other than PCNet.

Now, the first PCNet NIC (that is, anything not explicitly specified
to be anything different) will go to slot 3 even if it isn't the first
NIC specified on the commnd line. And anything else will be dynamically
assigned.

Signed-off-by: David Woodhouse 
---
  hw/ppc/prep.c | 8 +++-
  1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 137276bcb9..1a6cd05c61 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -241,7 +241,6 @@ static void ibm_40p_init(MachineState *machine)
  ISADevice *isa_dev;
  ISABus *isa_bus;
  void *fw_cfg;
-int i;
  uint32_t kernel_base = 0, initrd_base = 0;
  long kernel_size = 0, initrd_size = 0;
  char boot_device;
@@ -336,10 +335,9 @@ static void ibm_40p_init(MachineState *machine)
  /* XXX: s3-trio at PCI_DEVFN(2, 0) */
  pci_vga_init(pci_bus);
  
-for (i = 0; i < nb_nics; i++) {

-pci_nic_init_nofail(_table[i], pci_bus, mc->default_nic,
-i == 0 ? "3" : NULL);
-}
+/* First PCNET device at PCI_DEVFN(3, 0) */
+pci_init_nic_in_slot(pci_bus, mc->default_nic, NULL, "3");
+pci_init_nic_devices(pci_bus, mc->default_nic);
  }
  
  /* Prepare firmware configuration for OpenBIOS */


Fine for me ... Hervé, could you maybe comment from a 40p point of view, too?

Reviewed-by: Thomas Huth 




Re: [PATCH v3 14/46] hw/mips/loongson3_virt: use pci_init_nic_devices()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/mips/loongson3_virt.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
index 33eae01eca..caedde2df0 100644
--- a/hw/mips/loongson3_virt.c
+++ b/hw/mips/loongson3_virt.c
@@ -451,9 +451,7 @@ static inline void loongson3_virt_devices_init(MachineState 
*machine,
  usb_create_simple(usb_bus_find(-1), "usb-tablet");
  }
  
-for (i = 0; i < nb_nics; i++) {

-pci_nic_init_nofail(_table[i], pci_bus, mc->default_nic, NULL);
-}
+pci_init_nic_devices(pci_bus, mc->default_nic);
  }
  
  static void mips_loongson3_virt_init(MachineState *machine)


Reviewed-by: Thomas Huth 




Re: [PATCH v3 14/46] hw/mips/loongson3_virt: use pci_init_nic_devices()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
  hw/mips/loongson3_virt.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
index 33eae01eca..caedde2df0 100644
--- a/hw/mips/loongson3_virt.c
+++ b/hw/mips/loongson3_virt.c
@@ -451,9 +451,7 @@ static inline void loongson3_virt_devices_init(MachineState 
*machine,
  usb_create_simple(usb_bus_find(-1), "usb-tablet");
  }
  
-for (i = 0; i < nb_nics; i++) {

-pci_nic_init_nofail(_table[i], pci_bus, mc->default_nic, NULL);
-}
+pci_init_nic_devices(pci_bus, mc->default_nic);
  }
  
  static void mips_loongson3_virt_init(MachineState *machine)


Reviewed-by: Thomas Huth 




Re: [PATCH v3 13/46] hw/mips/malta: use pci_init_nic_devices()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

The Malta board setup code would previously place the first NIC into PCI
slot 11 if was a PCNet card, and the rest (including the first if it was
anything other than a PCNet card) would be dynamically assigned.

Now it will place any PCNet NIC into slot 11, and then anything else will
be dynamically assigned.

Signed-off-by: David Woodhouse 
---
  hw/mips/malta.c | 15 +++
  1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index d22bb1edef..af74008c82 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -612,18 +612,9 @@ static MaltaFPGAState *malta_fpga_init(MemoryRegion 
*address_space,
  /* Network support */
  static void network_init(PCIBus *pci_bus)
  {
-int i;
-
-for (i = 0; i < nb_nics; i++) {
-NICInfo *nd = _table[i];
-const char *default_devaddr = NULL;
-
-if (i == 0 && (!nd->model || strcmp(nd->model, "pcnet") == 0))
-/* The malta board has a PCNet card using PCI SLOT 11 */
-default_devaddr = "0b";
-
-pci_nic_init_nofail(nd, pci_bus, "pcnet", default_devaddr);
-}
+/* The malta board has a PCNet card using PCI SLOT 11 */
+pci_init_nic_in_slot(pci_bus, "pcnet", NULL, "0b");
+    pci_init_nic_devices(pci_bus, "pcnet");
  }


Reviewed-by: Thomas Huth 

Philippe, could you maybe have a look at this, too?





Re: [PATCH v3 13/46] hw/mips/malta: use pci_init_nic_devices()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

The Malta board setup code would previously place the first NIC into PCI
slot 11 if was a PCNet card, and the rest (including the first if it was
anything other than a PCNet card) would be dynamically assigned.

Now it will place any PCNet NIC into slot 11, and then anything else will
be dynamically assigned.

Signed-off-by: David Woodhouse 
---
  hw/mips/malta.c | 15 +++
  1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index d22bb1edef..af74008c82 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -612,18 +612,9 @@ static MaltaFPGAState *malta_fpga_init(MemoryRegion 
*address_space,
  /* Network support */
  static void network_init(PCIBus *pci_bus)
  {
-int i;
-
-for (i = 0; i < nb_nics; i++) {
-NICInfo *nd = _table[i];
-const char *default_devaddr = NULL;
-
-if (i == 0 && (!nd->model || strcmp(nd->model, "pcnet") == 0))
-/* The malta board has a PCNet card using PCI SLOT 11 */
-default_devaddr = "0b";
-
-pci_nic_init_nofail(nd, pci_bus, "pcnet", default_devaddr);
-}
+/* The malta board has a PCNet card using PCI SLOT 11 */
+pci_init_nic_in_slot(pci_bus, "pcnet", NULL, "0b");
+    pci_init_nic_devices(pci_bus, "pcnet");
  }


Reviewed-by: Thomas Huth 

Philippe, could you maybe have a look at this, too?





Re: [PATCH v3 12/46] hw/mips/fuloong2e: use pci_init_nic_devices()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

The previous behaviour was: *if* the first NIC specified on the command
line was an RTL8139 (or unspecified model) then it gets assigned to PCI
slot 7, which is where the Fuloong board had an RTL8139. All other
devices (including the first, if it was specified a anything other then
an rtl8319) get dynamically assigned on the bus.

The new behaviour is subtly different: If the first NIC was given a
specific model *other* than rtl8139, and a subsequent NIC was not,
then the rtl8139 (or unspecified) NIC will go to slot 7 and the rest
will be dynamically assigned.


Sounds fine for me ... Philippe, what do you think?

Reviewed-by: Thomas Huth 




Signed-off-by: David Woodhouse 
---
  hw/mips/fuloong2e.c | 16 +++-
  1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 97b2c8ed8e..a45aac368c 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -201,19 +201,9 @@ static void main_cpu_reset(void *opaque)
  /* Network support */
  static void network_init(PCIBus *pci_bus)
  {
-int i;
-
-for (i = 0; i < nb_nics; i++) {
-NICInfo *nd = _table[i];
-const char *default_devaddr = NULL;
-
-if (i == 0 && (!nd->model || strcmp(nd->model, "rtl8139") == 0)) {
-/* The Fuloong board has a RTL8139 card using PCI SLOT 7 */
-default_devaddr = "07";
-}
-
-pci_nic_init_nofail(nd, pci_bus, "rtl8139", default_devaddr);
-}
+/* The Fuloong board has a RTL8139 card using PCI SLOT 7 */
+pci_init_nic_in_slot(pci_bus, "rtl8139", NULL, "07");
+pci_init_nic_devices(pci_bus, "rtl8139");
  }
  
  static void mips_fuloong2e_init(MachineState *machine)





Re: [PATCH v3 12/46] hw/mips/fuloong2e: use pci_init_nic_devices()

2024-01-26 Thread Thomas Huth

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

The previous behaviour was: *if* the first NIC specified on the command
line was an RTL8139 (or unspecified model) then it gets assigned to PCI
slot 7, which is where the Fuloong board had an RTL8139. All other
devices (including the first, if it was specified a anything other then
an rtl8319) get dynamically assigned on the bus.

The new behaviour is subtly different: If the first NIC was given a
specific model *other* than rtl8139, and a subsequent NIC was not,
then the rtl8139 (or unspecified) NIC will go to slot 7 and the rest
will be dynamically assigned.


Sounds fine for me ... Philippe, what do you think?

Reviewed-by: Thomas Huth 




Signed-off-by: David Woodhouse 
---
  hw/mips/fuloong2e.c | 16 +++-
  1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 97b2c8ed8e..a45aac368c 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -201,19 +201,9 @@ static void main_cpu_reset(void *opaque)
  /* Network support */
  static void network_init(PCIBus *pci_bus)
  {
-int i;
-
-for (i = 0; i < nb_nics; i++) {
-NICInfo *nd = _table[i];
-const char *default_devaddr = NULL;
-
-if (i == 0 && (!nd->model || strcmp(nd->model, "rtl8139") == 0)) {
-/* The Fuloong board has a RTL8139 card using PCI SLOT 7 */
-default_devaddr = "07";
-}
-
-pci_nic_init_nofail(nd, pci_bus, "rtl8139", default_devaddr);
-}
+/* The Fuloong board has a RTL8139 card using PCI SLOT 7 */
+pci_init_nic_in_slot(pci_bus, "rtl8139", NULL, "07");
+pci_init_nic_devices(pci_bus, "rtl8139");
  }
  
  static void mips_fuloong2e_init(MachineState *machine)





Re: [PATCH v3 05/46] hw/i386/pc: use qemu_get_nic_info() and pci_init_nic_devices()

2024-01-26 Thread Thomas Huth

On 26/01/2024 12.25, David Woodhouse wrote:

On Fri, 2024-01-26 at 12:20 +0100, Thomas Huth wrote:

On 26/01/2024 12.13, David Woodhouse wrote:

On Fri, 2024-01-26 at 11:43 +0100, Thomas Huth wrote:

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

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 
Reviewed-by: Paul Durrant 
---
    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 496498df3a..d80c536d88 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -658,8 +658,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(_fatal,
+   "maximum number of ISA NE2000 devices exceeded");
    return;
+    }


error_setg(_fatal, ...) quits QEMU, so the "return;" does not make
much sense anymore.
Now, according to include/qapi/error.h :

    * Please don't error_setg(_fatal, ...), use error_report() and
    * exit(), because that's more obvious.

So I'd suggest to do that instead.


It's going slightly in the opposite direction to what's requested in
https://lore.kernel.org/qemu-devel/34e2c0c6-4e04-486a-8e1f-4afdc461a...@linaro.org/

I was thinking that a future patch would let the _fatal be an
Error** passed in by the caller, and not actually hard-coded to be
fatal at all.

But sure, unless Philippe objects I'm happy to do it as you show above.


Now that you mention it, I'd also prefer having an Error** parameter to the
function instead, that's certainly cleaner. So if you don't mind, please
follow Philippe's suggestion instead!


Right. There's a whole bunch of functions to untangle, that take an
Error** but don't return success/failure independently as they should.
Or don't even take the Error**.

Rather than trying to fix that as part of this series, this was my
compromise — making it easy to switch that explicit _fatal out
for a function parameter, but not trying to shave that part of the yak
myself just yet.


I think the nicest compromise is to add the "Error **errp" to the 
pc_init_ne2k_isa() and change the caller to pass in _fatal there ... 
further clean-up (passing the error even up further in the stack) is out of 
scope of this series, indeed.


 Thomas




Re: [PATCH v3 05/46] hw/i386/pc: use qemu_get_nic_info() and pci_init_nic_devices()

2024-01-26 Thread Thomas Huth

On 26/01/2024 12.25, David Woodhouse wrote:

On Fri, 2024-01-26 at 12:20 +0100, Thomas Huth wrote:

On 26/01/2024 12.13, David Woodhouse wrote:

On Fri, 2024-01-26 at 11:43 +0100, Thomas Huth wrote:

On 08/01/2024 21.26, David Woodhouse wrote:

From: David Woodhouse 

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 
Reviewed-by: Paul Durrant 
---
    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 496498df3a..d80c536d88 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -658,8 +658,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(_fatal,
+   "maximum number of ISA NE2000 devices exceeded");
    return;
+    }


error_setg(_fatal, ...) quits QEMU, so the "return;" does not make
much sense anymore.
Now, according to include/qapi/error.h :

    * Please don't error_setg(_fatal, ...), use error_report() and
    * exit(), because that's more obvious.

So I'd suggest to do that instead.


It's going slightly in the opposite direction to what's requested in
https://lore.kernel.org/qemu-devel/34e2c0c6-4e04-486a-8e1f-4afdc461a...@linaro.org/

I was thinking that a future patch would let the _fatal be an
Error** passed in by the caller, and not actually hard-coded to be
fatal at all.

But sure, unless Philippe objects I'm happy to do it as you show above.


Now that you mention it, I'd also prefer having an Error** parameter to the
function instead, that's certainly cleaner. So if you don't mind, please
follow Philippe's suggestion instead!


Right. There's a whole bunch of functions to untangle, that take an
Error** but don't return success/failure independently as they should.
Or don't even take the Error**.

Rather than trying to fix that as part of this series, this was my
compromise — making it easy to switch that explicit _fatal out
for a function parameter, but not trying to shave that part of the yak
myself just yet.


I think the nicest compromise is to add the "Error **errp" to the 
pc_init_ne2k_isa() and change the caller to pass in _fatal there ... 
further clean-up (passing the error even up further in the stack) is out of 
scope of this series, indeed.


 Thomas




<    5   6   7   8   9   10   11   12   13   14   >