Re: [PATCH 5/5] kconfig: use "select" to enable semihosting
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()
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
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
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()
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()
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()
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()
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
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
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
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
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
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
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
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
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
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
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
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()
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()
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()
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()
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()
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
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()
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()
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()
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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()
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()
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()
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()
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()
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()
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
** 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()
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()
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()
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
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
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()
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()
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()
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()
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()
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()
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()
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()
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()
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
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()
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()
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()
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()
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()
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()
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()
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()
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()
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()
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()
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()
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
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
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()
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()
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()
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()
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()
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()
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()
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()
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()
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()
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()
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()
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()
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()
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