Re: [PATCH v6 13/15] hw/pci: Determine if rombar is explicitly enabled

[Akihiko Odaki]

On 2024/02/21 17:15, Markus Armbruster wrote:

Akihiko Odaki  writes:


vfio determines if rombar is explicitly enabled by inspecting QDict.
Inspecting QDict is not nice because QDict is untyped and depends on the
details on the external interface. Add an infrastructure to determine if
rombar is explicitly enabled to hw/pci.

Signed-off-by: Akihiko Odaki 
---
  include/hw/pci/pci_device.h | 5 +
  1 file changed, 5 insertions(+)

diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index ca151325085d..c4fdc96ef50d 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -205,6 +205,11 @@ static inline uint16_t pci_get_bdf(PCIDevice *dev)
  return PCI_BUILD_BDF(pci_bus_num(pci_get_bus(dev)), dev->devfn);
  }
  
+static inline bool pci_rom_bar_explicitly_enabled(PCIDevice *dev)

+{
+return dev->rom_bar && dev->rom_bar != -1;


Compares signed with unsigned: rom_bar is uint32_t, -1 is int.

If int is at most 32 bits, the comparison converts -1 to
(uint32_t)0x.

If int is wider, it converts dev->rom_bar to int without changing its
value.  In particular, it converts the default value 0x (written
as -1 in the previous patch) to (int)0x.  Oops.

Best not to mix signed and unsigned in comparisons.  Easy enough to
avoid here.

Still, we don't actually test "rom_bar has not been set".  We test
"rom_bar has the default value".  Nothing stops a user from passing
rombar=0x to -device.  See my review of the next patch.


I followed addr and romsize properties that already use -1 as the 
default value. addr is int32_t, but romsize is uint32_t. I'll change 
romsize and rom_bar to use UINT32_MAX as the default value.


This changes the behavior of 0x, but that should be fine. Most 
people should only set 0 or 1. Maybe someone types wrongly and sets a 
number like 2 or 12, but nobody types 0x by mistake.

Re: [PATCH v6 12/15] hw/pci: Use -1 as a default value for rombar

[Akihiko Odaki]

On 2024/02/21 16:59, Markus Armbruster wrote:

Akihiko Odaki  writes:


Currently there is no way to distinguish the case that rombar is
explicitly specified as 1 and the case that rombar is not specified.

Set rombar -1 by default to distinguish these cases just as it is done
for addr and romsize. It was confirmed that changing the default value
to -1 will not change the behavior by looking at occurences of rom_bar.

$ git grep -w rom_bar
hw/display/qxl.c:328:QXLRom *rom = memory_region_get_ram_ptr(&d->rom_bar);
hw/display/qxl.c:431:qxl_set_dirty(&qxl->rom_bar, 0, qxl->rom_size);
hw/display/qxl.c:1048:QXLRom *rom = 
memory_region_get_ram_ptr(&qxl->rom_bar);
hw/display/qxl.c:2131:memory_region_init_rom(&qxl->rom_bar, OBJECT(qxl), 
"qxl.vrom",
hw/display/qxl.c:2154: PCI_BASE_ADDRESS_SPACE_MEMORY, &qxl->rom_bar);
hw/display/qxl.h:101:MemoryRegion   rom_bar;
hw/pci/pci.c:74:DEFINE_PROP_UINT32("rombar",  PCIDevice, rom_bar, 1),
hw/pci/pci.c:2329:if (!pdev->rom_bar) {
hw/vfio/pci.c:1019:if (vdev->pdev.romfile || !vdev->pdev.rom_bar) {
hw/xen/xen_pt_load_rom.c:29:if (dev->romfile || !dev->rom_bar) {
include/hw/pci/pci_device.h:150:uint32_t rom_bar;

rom_bar refers to a different variable in qxl. It is only tested if
the value is 0 or not in the other places.


Makes me wonder why it's uint32_t.  Not this patch's problem.


Indeed. It should have been OnOffAuto. I'm not changing it now though 
because I'm too worried if it is too disruptive.





Signed-off-by: Akihiko Odaki 
---
  hw/pci/pci.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 54b375da2d26..909c5b3ee4ee 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -71,7 +71,7 @@ static Property pci_props[] = {
  DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
  DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
  DEFINE_PROP_UINT32("romsize", PCIDevice, romsize, -1),
-DEFINE_PROP_UINT32("rombar",  PCIDevice, rom_bar, 1),
+DEFINE_PROP_UINT32("rombar",  PCIDevice, rom_bar, -1),
  DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
  QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false),
  DEFINE_PROP_BIT("x-pcie-lnksta-dllla", PCIDevice, cap_present,

[PATCH 15/28] qemu-img: snapshot: allow specifying -f fmt

[Michael Tokarev]

For consistency with other commands, and since it already
accepts --image-opts, allow specifying -f fmt too.

Signed-off-by: Michael Tokarev 
Reviewed-by: Daniel P. Berrangé 
---
 docs/tools/qemu-img.rst | 2 +-
 qemu-img-cmds.hx| 4 ++--
 qemu-img.c  | 9 ++---
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index 3653adb963..9b628c4da5 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -663,7 +663,7 @@ Command description:
   bitmap support, or 0 if bitmaps are supported but there is nothing
   to copy.
 
-.. option:: snapshot [--object OBJECTDEF] [--image-opts] [-U] [-q] [-l | -a 
SNAPSHOT | -c SNAPSHOT | -d SNAPSHOT] FILENAME
+.. option:: snapshot [--object OBJECTDEF] [-f FMT | --image-opts] [-U] [-q] 
[-l | -a SNAPSHOT | -c SNAPSHOT | -d SNAPSHOT] FILENAME
 
   List, apply, create or delete snapshots in image *FILENAME*.
 
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index c9dd70a892..2c5a8a28f9 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -84,9 +84,9 @@ SRST
 ERST
 
 DEF("snapshot", img_snapshot,
-"snapshot [--object objectdef] [--image-opts] [-U] [-q] [-l | -a snapshot 
| -c snapshot | -d snapshot] filename")
+"snapshot [--object objectdef] [-f fmt | --image-opts] [-U] [-q] [-l | -a 
snapshot | -c snapshot | -d snapshot] filename")
 SRST
-.. option:: snapshot [--object OBJECTDEF] [--image-opts] [-U] [-q] [-l | -a 
SNAPSHOT | -c SNAPSHOT | -d SNAPSHOT] FILENAME
+.. option:: snapshot [--object OBJECTDEF] [-f FMT | --image-opts] [-U] [-q] 
[-l | -a SNAPSHOT | -c SNAPSHOT | -d SNAPSHOT] FILENAME
 ERST
 
 DEF("rebase", img_rebase,
diff --git a/qemu-img.c b/qemu-img.c
index 3f719bbecf..85c37d491d 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3594,7 +3594,7 @@ static int img_snapshot(const img_cmd_t *ccmd, int argc, 
char **argv)
 BlockBackend *blk;
 BlockDriverState *bs;
 QEMUSnapshotInfo sn;
-char *filename, *snapshot_name = NULL;
+char *filename, *fmt = NULL, *snapshot_name = NULL;
 int c, ret = 0, bdrv_oflags;
 int action = 0;
 bool quiet = false;
@@ -3613,7 +3613,7 @@ static int img_snapshot(const img_cmd_t *ccmd, int argc, 
char **argv)
 {"force-share", no_argument, 0, 'U'},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":la:c:d:hqU",
+c = getopt_long(argc, argv, ":la:c:d:f:hqU",
 long_options, NULL);
 if (c == -1) {
 break;
@@ -3628,6 +3628,9 @@ static int img_snapshot(const img_cmd_t *ccmd, int argc, 
char **argv)
 case 'h':
 help();
 return 0;
+case 'f':
+fmt = optarg;
+break;
 case 'l':
 if (action) {
 error_exit(argv[0], "Cannot mix '-l', '-a', '-c', '-d'");
@@ -3681,7 +3684,7 @@ static int img_snapshot(const img_cmd_t *ccmd, int argc, 
char **argv)
 filename = argv[optind++];
 
 /* Open the image */
-blk = img_open(image_opts, filename, NULL, bdrv_oflags, false, quiet,
+blk = img_open(image_opts, filename, fmt, bdrv_oflags, false, quiet,
force_share);
 if (!blk) {
 return 1;
-- 
2.39.2

[PATCH 03/28] qemu-img: create: convert img_size to signed, simplify handling

[Michael Tokarev]

Initializing an unsigned as -1, or using temporary
sval for conversion is awkward.  Since we don't allow
other "negative" values anyway, use signed value and
pass it to bdrv_img_create() (where it is properly
converted to unsigned), simplifying code.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index ae14ed833d..df425b2517 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -511,7 +511,7 @@ static int64_t cvtnum(const char *name, const char *value)
 static int img_create(int argc, char **argv)
 {
 int c;
-uint64_t img_size = -1;
+int64_t img_size = -1;
 const char *fmt = "raw";
 const char *base_fmt = NULL;
 const char *filename;
@@ -582,13 +582,10 @@ static int img_create(int argc, char **argv)
 
 /* Get image size, if specified */
 if (optind < argc) {
-int64_t sval;
-
-sval = cvtnum("image size", argv[optind++]);
-if (sval < 0) {
+img_size = cvtnum("image size", argv[optind++]);
+if (img_size < 0) {
 goto fail;
 }
-img_size = (uint64_t)sval;
 }
 if (optind != argc) {
 error_exit("Unexpected argument: %s", argv[optind]);
-- 
2.39.2

[PULL 18/25] hw/ide: Add the possibility to disable the CompactFlash device in the build

[Philippe Mathieu-Daudé]

From: Thomas Huth 

For distros like downstream RHEL, it would be helpful to allow to disable
the CompactFlash device. For making this possible, we need a separate
Kconfig switch for this device, and the code should reside in a separate
file. Let's also introduce a new header ide-dev.h which can be used to
collect definitions related to IDE devices.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
Acked-by: Mark Cave-Ayland 
Message-ID: <20240220085505.30255-2-th...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/ide/ide-dev.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 include/hw/ide/ide-dev.h
 create mode 100644 hw/ide/cf.c

diff --git a/include/hw/ide/ide-dev.h b/include/hw/ide/ide-dev.h
new file mode 100644
index 00..7e9663cda9
--- /dev/null
+++ b/include/hw/ide/ide-dev.h
@@ -0,0 +1,41 @@
+/*
+ * ide device definitions
+ *
+ * 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 .
+ */
+
+#ifndef IDE_DEV_H
+#define IDE_DEV_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..2a425cb0f2
--- /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 .
+ */
+
+#include "qemu/osdep.h"
+#include "hw/ide/ide-dev.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(&ide_cf_info);
+}
+
+type_init(ide_cf_register_type)
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 1b3b4da01d..4189313d30 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/ide-dev.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
-#include "hw/b

[PULL 03/25] hw/ppc/ppc4xx_pci: Remove unused "hw/ppc/ppc.h" header

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: BALATON Zoltan 
Reviewed-by: Thomas Huth 
Message-Id: <20240215105017.57748-2-phi...@linaro.org>
---
 hw/ppc/ppc440_pcix.c | 1 -
 hw/ppc/ppc4xx_pci.c  | 1 -
 2 files changed, 2 deletions(-)

diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index df4ee374d0..d84418cb7b 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -25,7 +25,6 @@
 #include "qemu/module.h"
 #include "qemu/units.h"
 #include "hw/irq.h"
-#include "hw/ppc/ppc.h"
 #include "hw/ppc/ppc4xx.h"
 #include "hw/pci/pci_device.h"
 #include "hw/pci/pci_host.h"
diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index 0a07aab5d1..e4101398c9 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -24,7 +24,6 @@
 #include "qemu/osdep.h"
 #include "qemu/log.h"
 #include "hw/irq.h"
-#include "hw/ppc/ppc.h"
 #include "hw/ppc/ppc4xx.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
-- 
2.41.0

[PULL 11/25] hw/i386/x86: Turn apic_xrupt_override into class attribute

[Philippe Mathieu-Daudé]

From: Bernhard Beschow 

The attribute isn't user-changeable and only true for pc-based machines. Turn it
into a class attribute which allows for inlining pc_guest_info_init() into
pc_machine_initfn().

Signed-off-by: Bernhard Beschow 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240208220349.4948-4-shen...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/i386/x86.h | 3 ++-
 hw/i386/acpi-common.c | 3 ++-
 hw/i386/pc.c  | 5 ++---
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index da19ae1546..8e306db7bb 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -34,6 +34,8 @@ struct X86MachineClass {
 bool save_tsc_khz;
 /* use DMA capable linuxboot option rom */
 bool fwcfg_dma_enabled;
+/* CPU and apic information: */
+bool apic_xrupt_override;
 };
 
 struct X86MachineState {
@@ -57,7 +59,6 @@ struct X86MachineState {
 uint64_t above_4g_mem_start;
 
 /* CPU and apic information: */
-bool apic_xrupt_override;
 unsigned pci_irq_mask;
 unsigned apic_id_limit;
 uint16_t boot_cpus;
diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
index 43dc23f7e0..cea4b3d71c 100644
--- a/hw/i386/acpi-common.c
+++ b/hw/i386/acpi-common.c
@@ -100,6 +100,7 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
 int i;
 bool x2apic_mode = false;
 MachineClass *mc = MACHINE_GET_CLASS(x86ms);
+X86MachineClass *x86mc = X86_MACHINE_GET_CLASS(x86ms);
 const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
 AcpiTable table = { .sig = "APIC", .rev = 3, .oem_id = oem_id,
 .oem_table_id = oem_table_id };
@@ -122,7 +123,7 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
  IO_APIC_SECONDARY_ADDRESS, IO_APIC_SECONDARY_IRQBASE);
 }
 
-if (x86ms->apic_xrupt_override) {
+if (x86mc->apic_xrupt_override) {
 build_xrupt_override(table_data, 0, 2,
 0 /* Flags: Conforms to the specifications of the bus */);
 }
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 1733dffc00..d7183780bd 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -700,9 +700,6 @@ void pc_machine_done(Notifier *notifier, void *data)
 
 void pc_guest_info_init(PCMachineState *pcms)
 {
-X86MachineState *x86ms = X86_MACHINE(pcms);
-
-x86ms->apic_xrupt_override = true;
 pcms->machine_done.notify = pc_machine_done;
 qemu_add_machine_init_done_notifier(&pcms->machine_done);
 }
@@ -1795,6 +1792,7 @@ static bool pc_hotplug_allowed(MachineState *ms, 
DeviceState *dev, Error **errp)
 static void pc_machine_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
+X86MachineClass *x86mc = X86_MACHINE_CLASS(oc);
 PCMachineClass *pcmc = PC_MACHINE_CLASS(oc);
 HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
 
@@ -1814,6 +1812,7 @@ static void pc_machine_class_init(ObjectClass *oc, void 
*data)
 pcmc->pvh_enabled = true;
 pcmc->kvmclock_create_always = true;
 pcmc->resizable_acpi_blob = true;
+x86mc->apic_xrupt_override = true;
 assert(!mc->get_hotplug_handler);
 mc->get_hotplug_handler = pc_get_hotplug_handler;
 mc->hotplug_allowed = pc_hotplug_allowed;
-- 
2.41.0

[PULL 20/25] hw/ide: Move IDE DMA related definitions to a separate header ide-dma.h

[Philippe Mathieu-Daudé]

From: Thomas Huth 

These definitions are required outside of the hw/ide/ code, too,
so lets's move them from internal.h to a new header called ide-dma.h.

Signed-off-by: Thomas Huth 
Acked-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240220085505.30255-4-th...@redhat.com>
[PMD: Use IDEDMAOps typedef in struct IDEDMA]
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/ide/ide-dma.h  | 37 +
 include/hw/ide/internal.h | 29 +
 2 files changed, 38 insertions(+), 28 deletions(-)
 create mode 100644 include/hw/ide/ide-dma.h

diff --git a/include/hw/ide/ide-dma.h b/include/hw/ide/ide-dma.h
new file mode 100644
index 00..d0b19ac9c5
--- /dev/null
+++ b/include/hw/ide/ide-dma.h
@@ -0,0 +1,37 @@
+#ifndef HW_IDE_DMA_H
+#define HW_IDE_DMA_H
+
+#include "block/aio.h"
+#include "qemu/iov.h"
+
+typedef struct IDEState IDEState;
+typedef struct IDEDMAOps IDEDMAOps;
+typedef struct IDEDMA IDEDMA;
+
+typedef void DMAStartFunc(const IDEDMA *, IDEState *, BlockCompletionFunc *);
+typedef void DMAVoidFunc(const IDEDMA *);
+typedef int DMAIntFunc(const IDEDMA *, bool);
+typedef int32_t DMAInt32Func(const IDEDMA *, int32_t len);
+typedef void DMAu32Func(const IDEDMA *, uint32_t);
+typedef void DMAStopFunc(const IDEDMA *, bool);
+
+struct IDEDMAOps {
+DMAStartFunc *start_dma;
+DMAVoidFunc *pio_transfer;
+DMAInt32Func *prepare_buf;
+DMAu32Func *commit_buf;
+DMAIntFunc *rw_buf;
+DMAVoidFunc *restart;
+DMAVoidFunc *restart_dma;
+DMAStopFunc *set_inactive;
+DMAVoidFunc *cmd_done;
+DMAVoidFunc *reset;
+};
+
+struct IDEDMA {
+const IDEDMAOps *ops;
+QEMUIOVector qiov;
+BlockAIOCB *aiocb;
+};
+
+#endif
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 3bdcc75597..a3a6702eec 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -11,6 +11,7 @@
 #include "sysemu/dma.h"
 #include "hw/block/block.h"
 #include "exec/ioport.h"
+#include "hw/ide/ide-dma.h"
 
 /* debug IDE devices */
 #define USE_DMA_CDROM
@@ -18,8 +19,6 @@
 
 typedef struct IDEDevice IDEDevice;
 typedef struct IDEState IDEState;
-typedef struct IDEDMA IDEDMA;
-typedef struct IDEDMAOps IDEDMAOps;
 
 #define TYPE_IDE_BUS "IDE"
 OBJECT_DECLARE_SIMPLE_TYPE(IDEBus, IDE_BUS)
@@ -332,13 +331,6 @@ typedef enum { IDE_HD, IDE_CD, IDE_CFATA } IDEDriveKind;
 
 typedef void EndTransferFunc(IDEState *);
 
-typedef void DMAStartFunc(const IDEDMA *, IDEState *, BlockCompletionFunc *);
-typedef void DMAVoidFunc(const IDEDMA *);
-typedef int DMAIntFunc(const IDEDMA *, bool);
-typedef int32_t DMAInt32Func(const IDEDMA *, int32_t len);
-typedef void DMAu32Func(const IDEDMA *, uint32_t);
-typedef void DMAStopFunc(const IDEDMA *, bool);
-
 struct unreported_events {
 bool eject_request;
 bool new_media;
@@ -460,25 +452,6 @@ struct IDEState {
 int ncq_queues;
 };
 
-struct IDEDMAOps {
-DMAStartFunc *start_dma;
-DMAVoidFunc *pio_transfer;
-DMAInt32Func *prepare_buf;
-DMAu32Func *commit_buf;
-DMAIntFunc *rw_buf;
-DMAVoidFunc *restart;
-DMAVoidFunc *restart_dma;
-DMAStopFunc *set_inactive;
-DMAVoidFunc *cmd_done;
-DMAVoidFunc *reset;
-};
-
-struct IDEDMA {
-const struct IDEDMAOps *ops;
-QEMUIOVector qiov;
-BlockAIOCB *aiocb;
-};
-
 struct IDEBus {
 BusState qbus;
 IDEDevice *master;
-- 
2.41.0

[PULL 01/25] hw/input/pckbd: Open-code i8042_setup_a20_line() wrapper

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé 

Since the named GPIO lines are a "public" interface to the device,
we can directly call qdev_connect_gpio_out_named(), making it
consistent with how the other A20 input source (port92) is wired.

Suggested-by: Peter Maydell 
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Yanan Wang 
Message-Id: <20211218130437.1516929-6-f4...@amsat.org>
---
 include/hw/input/i8042.h | 1 -
 hw/i386/pc.c | 3 ++-
 hw/input/pckbd.c | 5 -
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/include/hw/input/i8042.h b/include/hw/input/i8042.h
index 9fb3f8d787..e90f008b66 100644
--- a/include/hw/input/i8042.h
+++ b/include/hw/input/i8042.h
@@ -89,7 +89,6 @@ struct MMIOKBDState {
 
 
 void i8042_isa_mouse_fake_event(ISAKBDState *isa);
-void i8042_setup_a20_line(ISADevice *dev, qemu_irq a20_out);
 
 static inline bool i8042_present(void)
 {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 28194014f8..9cbc59665f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1195,7 +1195,8 @@ static void pc_superio_init(ISABus *isa_bus, bool 
create_fdctrl,
 port92 = isa_create_simple(isa_bus, TYPE_PORT92);
 
 a20_line = qemu_allocate_irqs(handle_a20_line_change, first_cpu, 2);
-i8042_setup_a20_line(i8042, a20_line[0]);
+qdev_connect_gpio_out_named(DEVICE(i8042),
+I8042_A20_LINE, 0, a20_line[0]);
 qdev_connect_gpio_out_named(DEVICE(port92),
 PORT92_A20_LINE, 0, a20_line[1]);
 g_free(a20_line);
diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index 90a4d9eb40..74f10b640f 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -777,11 +777,6 @@ void i8042_isa_mouse_fake_event(ISAKBDState *isa)
 ps2_mouse_fake_event(&s->ps2mouse);
 }
 
-void i8042_setup_a20_line(ISADevice *dev, qemu_irq a20_out)
-{
-qdev_connect_gpio_out_named(DEVICE(dev), I8042_A20_LINE, 0, a20_out);
-}
-
 static const VMStateDescription vmstate_kbd_isa = {
 .name = "pckbd",
 .version_id = 3,
-- 
2.41.0

[PULL 17/25] hw/isa/meson.build: Sort alphabetically

[Philippe Mathieu-Daudé]

From: Bernhard Beschow 

Fixes: fbd758008f0f "hw/isa: extract FDC37M81X to a separate file"

Signed-off-by: Bernhard Beschow 
Reviewed-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240218131701.91132-2-shen...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/isa/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/isa/meson.build b/hw/isa/meson.build
index f650b39507..3219282217 100644
--- a/hw/isa/meson.build
+++ b/hw/isa/meson.build
@@ -1,10 +1,10 @@
 system_ss.add(when: 'CONFIG_APM', if_true: files('apm.c'))
+system_ss.add(when: 'CONFIG_FDC37M81X', if_true: files('fdc37m81x-superio.c'))
 system_ss.add(when: 'CONFIG_I82378', if_true: files('i82378.c'))
 system_ss.add(when: 'CONFIG_ISA_BUS', if_true: files('isa-bus.c'))
 system_ss.add(when: 'CONFIG_ISA_SUPERIO', if_true: files('isa-superio.c'))
 system_ss.add(when: 'CONFIG_PC87312', if_true: files('pc87312.c'))
 system_ss.add(when: 'CONFIG_PIIX', if_true: files('piix.c'))
-system_ss.add(when: 'CONFIG_FDC37M81X', if_true: files('fdc37m81x-superio.c'))
 system_ss.add(when: 'CONFIG_SMC37C669', if_true: files('smc37c669-superio.c'))
 system_ss.add(when: 'CONFIG_VT82C686', if_true: files('vt82c686.c'))
 
-- 
2.41.0

[PULL 06/25] hw/ppc/ppc440_pcix: Move ppc440_pcix.c to hw/pci-host/

[Philippe Mathieu-Daudé]

ppc440_pcix.c is moved from the target specific ppc_ss[] meson
source set to pci_ss[] which is common to all targets: the
object is built once.

Reviewed-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20240215105017.57748-5-phi...@linaro.org>
---
 MAINTAINERS| 2 +-
 hw/{ppc => pci-host}/ppc440_pcix.c | 0
 hw/pci-host/Kconfig| 4 
 hw/pci-host/meson.build| 1 +
 hw/pci-host/trace-events   | 8 
 hw/ppc/Kconfig | 1 +
 hw/ppc/meson.build | 2 +-
 hw/ppc/trace-events| 8 
 8 files changed, 16 insertions(+), 10 deletions(-)
 rename hw/{ppc => pci-host}/ppc440_pcix.c (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8d9ccd5073..5535df4487 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1556,7 +1556,7 @@ L: qemu-...@nongnu.org
 S: Maintained
 F: hw/ppc/sam460ex.c
 F: hw/ppc/ppc440_uc.c
-F: hw/ppc/ppc440_pcix.c
+F: hw/pci-host/ppc440_pcix.c
 F: hw/display/sm501*
 F: hw/ide/sii3112.c
 F: hw/rtc/m41t80.c
diff --git a/hw/ppc/ppc440_pcix.c b/hw/pci-host/ppc440_pcix.c
similarity index 100%
rename from hw/ppc/ppc440_pcix.c
rename to hw/pci-host/ppc440_pcix.c
diff --git a/hw/pci-host/Kconfig b/hw/pci-host/Kconfig
index 0a221e719e..c91880b237 100644
--- a/hw/pci-host/Kconfig
+++ b/hw/pci-host/Kconfig
@@ -10,6 +10,10 @@ config PPC4XX_PCI
 bool
 select PCI
 
+config PPC440_PCIX
+bool
+select PCI
+
 config RAVEN_PCI
 bool
 select PCI
diff --git a/hw/pci-host/meson.build b/hw/pci-host/meson.build
index eb6dc71c88..3001e93a43 100644
--- a/hw/pci-host/meson.build
+++ b/hw/pci-host/meson.build
@@ -15,6 +15,7 @@ pci_ss.add(when: 'CONFIG_SH_PCI', if_true: files('sh_pci.c'))
 
 # PPC devices
 pci_ss.add(when: 'CONFIG_PPC4XX_PCI', if_true: files('ppc4xx_pci.c'))
+pci_ss.add(when: 'CONFIG_PPC440_PCIX', if_true: files('ppc440_pcix.c'))
 pci_ss.add(when: 'CONFIG_RAVEN_PCI', if_true: files('raven.c'))
 pci_ss.add(when: 'CONFIG_GRACKLE_PCI', if_true: files('grackle.c'))
 # NewWorld PowerMac
diff --git a/hw/pci-host/trace-events b/hw/pci-host/trace-events
index 90a37ebff0..0a816b9aa1 100644
--- a/hw/pci-host/trace-events
+++ b/hw/pci-host/trace-events
@@ -41,6 +41,14 @@ unin_read(uint64_t addr, uint64_t value) "addr=0x%" PRIx64 " 
val=0x%"PRIx64
 ppc4xx_pci_map_irq(int32_t devfn, int irq_num, int slot) "devfn 0x%x irq %d -> 
%d"
 ppc4xx_pci_set_irq(int irq_num) "PCI irq %d"
 
+# ppc440_pcix.c
+ppc440_pcix_map_irq(int32_t devfn, int irq_num, int slot) "devfn 0x%x irq %d 
-> %d"
+ppc440_pcix_set_irq(int irq_num) "PCI irq %d"
+ppc440_pcix_update_pim(int idx, uint64_t size, uint64_t la) "Added window %d 
of size=0x%" PRIx64 " to CPU=0x%" PRIx64
+ppc440_pcix_update_pom(int idx, uint32_t size, uint64_t la, uint64_t pcia) 
"Added window %d of size=0x%x from CPU=0x%" PRIx64 " to PCI=0x%" PRIx64
+ppc440_pcix_reg_read(uint64_t addr, uint32_t val) "addr 0x%" PRIx64 " = 0x%" 
PRIx32
+ppc440_pcix_reg_write(uint64_t addr, uint32_t val, uint32_t size) "addr 0x%" 
PRIx64 " = 0x%" PRIx32 " size 0x%" PRIx32
+
 # pnv_phb4.c
 pnv_phb4_xive_notify(uint64_t notif_port, uint64_t data) "notif=@0x%"PRIx64" 
data=0x%"PRIx64
 pnv_phb4_xive_notify_ic(uint64_t addr, uint64_t data) "addr=@0x%"PRIx64" 
data=0x%"PRIx64
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 82e847d22c..99d571fa20 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -46,6 +46,7 @@ config PPC440
 imply TEST_DEVICES
 imply E1000_PCI
 select PCI_EXPRESS
+select PPC440_PCIX
 select PPC4XX
 select SERIAL
 select FDT_PPC
diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index d0efc0aba5..da14fccce5 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -60,7 +60,7 @@ ppc_ss.add(when: 'CONFIG_PPC405', if_true: files(
   'ppc405_uc.c'))
 ppc_ss.add(when: 'CONFIG_PPC440', if_true: files(
   'ppc440_bamboo.c',
-  'ppc440_pcix.c', 'ppc440_uc.c'))
+  'ppc440_uc.c'))
 ppc_ss.add(when: 'CONFIG_PPC4XX', if_true: files(
   'ppc4xx_devs.c',
   'ppc4xx_sdram.c'))
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index b59fbf340f..157ea756e9 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -146,14 +146,6 @@ rs6000mc_size_read(uint32_t addr, uint32_t val) "read 
addr=0x%x val=0x%x"
 rs6000mc_size_write(uint32_t addr, uint32_t val) "write addr=0x%x val=0x%x"
 rs6000mc_parity_read(uint32_t addr, uint32_t val) "read addr=0x%x val=0x%x"
 
-# ppc440_pcix.c
-ppc440_pcix_map_irq(int32_t devfn, int irq_num, int slot) "devfn 0x%x irq %d 
-> %d"
-ppc440_pcix_set_irq(int irq_num) "PCI irq %d"
-ppc440_pcix_update_pim(int idx, uint64_t size, uint64_t la) "Added window %d 
of size=0x%" PRIx64 " to CPU=0x%" PRIx64
-ppc440_pcix_update_pom(int idx, uint32_t size, uint64_t la, uint64_t pcia) 
"Added window %d of size=0x%x from CPU=0x%" PRIx64 " to PCI=0x%" PRIx64
-ppc440_pcix_reg_read(uint64_t addr, uint32_t val) "addr 0x%" PRIx64 " = 0x%" 
PRIx32
-ppc440_pcix_reg_write(uint64_t addr, uint32_t val, uint32_

[PULL 22/25] hw/ide: Move IDE bus related definitions to a new header ide-bus.h

[Philippe Mathieu-Daudé]

From: Thomas Huth 

Let's consolidate the public IDE bus related functions in a separate
header.

Signed-off-by: Thomas Huth 
Acked-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240220085505.30255-6-th...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/ide/ide-bus.h  | 42 +++
 include/hw/ide/internal.h | 40 +
 2 files changed, 43 insertions(+), 39 deletions(-)
 create mode 100644 include/hw/ide/ide-bus.h

diff --git a/include/hw/ide/ide-bus.h b/include/hw/ide/ide-bus.h
new file mode 100644
index 00..4841a7dcd6
--- /dev/null
+++ b/include/hw/ide/ide-bus.h
@@ -0,0 +1,42 @@
+#ifndef HW_IDE_BUS_H
+#define HW_IDE_BUS_H
+
+#include "exec/ioport.h"
+#include "hw/ide/ide-dev.h"
+#include "hw/ide/ide-dma.h"
+
+struct IDEBus {
+BusState qbus;
+IDEDevice *master;
+IDEDevice *slave;
+IDEState ifs[2];
+QEMUBH *bh;
+
+int bus_id;
+int max_units;
+IDEDMA *dma;
+uint8_t unit;
+uint8_t cmd;
+qemu_irq irq; /* bus output */
+
+int error_status;
+uint8_t retry_unit;
+int64_t retry_sector_num;
+uint32_t retry_nsector;
+PortioList portio_list;
+PortioList portio2_list;
+VMChangeStateEntry *vmstate;
+};
+
+#define TYPE_IDE_BUS "IDE"
+OBJECT_DECLARE_SIMPLE_TYPE(IDEBus, IDE_BUS)
+
+void ide_bus_init(IDEBus *idebus, size_t idebus_size, DeviceState *dev,
+  int bus_id, int max_units);
+IDEDevice *ide_bus_create_drive(IDEBus *bus, int unit, DriveInfo *drive);
+
+int ide_get_geometry(BusState *bus, int unit,
+ int16_t *cyls, int8_t *heads, int8_t *secs);
+int ide_get_bios_chs_trans(BusState *bus, int unit);
+
+#endif
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 1aab89b27b..d3ec16a945 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -8,16 +8,10 @@
  */
 
 #include "hw/ide.h"
-#include "exec/ioport.h"
-#include "hw/ide/ide-dma.h"
-#include "hw/ide/ide-dev.h"
+#include "hw/ide/ide-bus.h"
 
 /* debug IDE devices */
 #define USE_DMA_CDROM
-#include "qom/object.h"
-
-#define TYPE_IDE_BUS "IDE"
-OBJECT_DECLARE_SIMPLE_TYPE(IDEBus, IDE_BUS)
 
 /* Device/Head ("select") Register */
 #define ATA_DEV_SELECT  0x10
@@ -338,29 +332,6 @@ typedef struct IDEBufferedRequest {
 bool orphaned;
 } IDEBufferedRequest;
 
-struct IDEBus {
-BusState qbus;
-IDEDevice *master;
-IDEDevice *slave;
-IDEState ifs[2];
-QEMUBH *bh;
-
-int bus_id;
-int max_units;
-IDEDMA *dma;
-uint8_t unit;
-uint8_t cmd;
-qemu_irq irq; /* bus output */
-
-int error_status;
-uint8_t retry_unit;
-int64_t retry_sector_num;
-uint32_t retry_nsector;
-PortioList portio_list;
-PortioList portio2_list;
-VMChangeStateEntry *vmstate;
-};
-
 /* These are used for the error_status field of IDEBus */
 #define IDE_RETRY_MASK 0xf8
 #define IDE_RETRY_DMA  0x08
@@ -477,15 +448,6 @@ void ide_cancel_dma_sync(IDEState *s);
 void ide_atapi_cmd(IDEState *s);
 void ide_atapi_cmd_reply_end(IDEState *s);
 
-/* hw/ide/qdev.c */
-void ide_bus_init(IDEBus *idebus, size_t idebus_size, DeviceState *dev,
-  int bus_id, int max_units);
-IDEDevice *ide_bus_create_drive(IDEBus *bus, int unit, DriveInfo *drive);
-
-int ide_get_geometry(BusState *bus, int unit,
- int16_t *cyls, int8_t *heads, int8_t *secs);
-int ide_get_bios_chs_trans(BusState *bus, int unit);
-
 int ide_handle_rw_error(IDEState *s, int error, int op);
 
 #endif /* HW_IDE_INTERNAL_H */
-- 
2.41.0

[PULL 24/25] hw/ide: Stop exposing internal.h to non-IDE files

[Philippe Mathieu-Daudé]

From: Thomas Huth 

include/hw/ide/internal.h is currently included by include/hw/ide/pci.h
and thus exposed to a lot of files that are not part of the IDE subsystem.
Stop including internal.h there and use the appropriate new headers
ide-bus.h and ide-dma.h instead.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
Acked-by: Mark Cave-Ayland 
Message-ID: <20240220085505.30255-8-th...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/ide/pci.h | 2 +-
 hw/i386/pc.c | 2 +-
 hw/ide/cmd646.c  | 1 +
 hw/ide/pci.c | 1 +
 hw/ide/piix.c| 1 +
 hw/ide/sii3112.c | 1 +
 hw/ide/via.c | 1 +
 7 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index a814a0a7c3..ef03764caa 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -1,7 +1,7 @@
 #ifndef HW_IDE_PCI_H
 #define HW_IDE_PCI_H
 
-#include "hw/ide/internal.h"
+#include "hw/ide/ide-bus.h"
 #include "hw/pci/pci_device.h"
 #include "qom/object.h"
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 1ee41a5e56..f8eb684a49 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -31,7 +31,7 @@
 #include "hw/i386/fw_cfg.h"
 #include "hw/i386/vmport.h"
 #include "sysemu/cpus.h"
-#include "hw/ide/internal.h"
+#include "hw/ide/ide-bus.h"
 #include "hw/timer/hpet.h"
 #include "hw/loader.h"
 #include "hw/rtc/mc146818rtc.h"
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index c0bcfa4414..23d213ff01 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -33,6 +33,7 @@
 #include "sysemu/reset.h"
 
 #include "hw/ide/pci.h"
+#include "hw/ide/internal.h"
 #include "trace.h"
 
 /* CMD646 specific */
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index ca85d8474c..73efeec7f4 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -30,6 +30,7 @@
 #include "sysemu/dma.h"
 #include "qemu/error-report.h"
 #include "qemu/module.h"
+#include "hw/ide/internal.h"
 #include "hw/ide/pci.h"
 #include "trace.h"
 
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 4e5e12935f..1773a068c3 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -30,6 +30,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "hw/pci/pci.h"
+#include "hw/ide/internal.h"
 #include "hw/ide/piix.h"
 #include "hw/ide/pci.h"
 #include "trace.h"
diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
index 63dc4a0494..321b9e46a1 100644
--- a/hw/ide/sii3112.c
+++ b/hw/ide/sii3112.c
@@ -13,6 +13,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/ide/internal.h"
 #include "hw/ide/pci.h"
 #include "qemu/module.h"
 #include "trace.h"
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 3f3c484253..cf151e70ec 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -25,6 +25,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/ide/internal.h"
 #include "hw/pci/pci.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
-- 
2.41.0

[PULL 21/25] hw/ide: Move IDE device related definitions to ide-dev.h

[Philippe Mathieu-Daudé]

From: Thomas Huth 

Untangle internal.h by moving public IDE device related
definitions to ide-dev.h.

Signed-off-by: Thomas Huth 
Acked-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240220085505.30255-5-th...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/ide/ide-dev.h  | 143 +-
 include/hw/ide/internal.h | 143 +-
 hw/ide/ide-dev.c  |   1 +
 3 files changed, 144 insertions(+), 143 deletions(-)

diff --git a/include/hw/ide/ide-dev.h b/include/hw/ide/ide-dev.h
index 7e9663cda9..1f62e58ebc 100644
--- a/include/hw/ide/ide-dev.h
+++ b/include/hw/ide/ide-dev.h
@@ -20,9 +20,150 @@
 #ifndef IDE_DEV_H
 #define IDE_DEV_H
 
+#include "sysemu/dma.h"
 #include "hw/qdev-properties.h"
 #include "hw/block/block.h"
-#include "hw/ide/internal.h"
+
+typedef struct IDEDevice IDEDevice;
+typedef struct IDEState IDEState;
+typedef struct IDEBus IDEBus;
+
+typedef void EndTransferFunc(IDEState *);
+
+#define MAX_IDE_DEVS 2
+
+#define TYPE_IDE_DEVICE "ide-device"
+OBJECT_DECLARE_TYPE(IDEDevice, IDEDeviceClass, IDE_DEVICE)
+
+typedef enum { IDE_HD, IDE_CD, IDE_CFATA } IDEDriveKind;
+
+struct unreported_events {
+bool eject_request;
+bool new_media;
+};
+
+enum ide_dma_cmd {
+IDE_DMA_READ = 0,
+IDE_DMA_WRITE,
+IDE_DMA_TRIM,
+IDE_DMA_ATAPI,
+IDE_DMA__COUNT
+};
+
+/* NOTE: IDEState represents in fact one drive */
+struct IDEState {
+IDEBus *bus;
+uint8_t unit;
+/* ide config */
+IDEDriveKind drive_kind;
+int drive_heads, drive_sectors;
+int cylinders, heads, sectors, chs_trans;
+int64_t nb_sectors;
+int mult_sectors;
+int identify_set;
+uint8_t identify_data[512];
+int drive_serial;
+char drive_serial_str[21];
+char drive_model_str[41];
+uint64_t wwn;
+/* ide regs */
+uint8_t feature;
+uint8_t error;
+uint32_t nsector;
+uint8_t sector;
+uint8_t lcyl;
+uint8_t hcyl;
+/* other part of tf for lba48 support */
+uint8_t hob_feature;
+uint8_t hob_nsector;
+uint8_t hob_sector;
+uint8_t hob_lcyl;
+uint8_t hob_hcyl;
+
+uint8_t select;
+uint8_t status;
+
+bool io8;
+bool reset_reverts;
+
+/* set for lba48 access */
+uint8_t lba48;
+BlockBackend *blk;
+char version[9];
+/* ATAPI specific */
+struct unreported_events events;
+uint8_t sense_key;
+uint8_t asc;
+bool tray_open;
+bool tray_locked;
+uint8_t cdrom_changed;
+int packet_transfer_size;
+int elementary_transfer_size;
+int32_t io_buffer_index;
+int lba;
+int cd_sector_size;
+int atapi_dma; /* true if dma is requested for the packet cmd */
+BlockAcctCookie acct;
+BlockAIOCB *pio_aiocb;
+QEMUIOVector qiov;
+QLIST_HEAD(, IDEBufferedRequest) buffered_requests;
+/* ATA DMA state */
+uint64_t io_buffer_offset;
+int32_t io_buffer_size;
+QEMUSGList sg;
+/* PIO transfer handling */
+int req_nb_sectors; /* number of sectors per interrupt */
+EndTransferFunc *end_transfer_func;
+uint8_t *data_ptr;
+uint8_t *data_end;
+uint8_t *io_buffer;
+/* PIO save/restore */
+int32_t io_buffer_total_len;
+int32_t cur_io_buffer_offset;
+int32_t cur_io_buffer_len;
+uint8_t end_transfer_fn_idx;
+QEMUTimer *sector_write_timer; /* only used for win2k install hack */
+uint32_t irq_count; /* counts IRQs when using win2k install hack */
+/* CF-ATA extended error */
+uint8_t ext_error;
+/* CF-ATA metadata storage */
+uint32_t mdata_size;
+uint8_t *mdata_storage;
+int media_changed;
+enum ide_dma_cmd dma_cmd;
+/* SMART */
+uint8_t smart_enabled;
+uint8_t smart_autosave;
+int smart_errors;
+uint8_t smart_selftest_count;
+uint8_t *smart_selftest_data;
+/* AHCI */
+int ncq_queues;
+};
+
+struct IDEDeviceClass {
+DeviceClass parent_class;
+void (*realize)(IDEDevice *dev, Error **errp);
+};
+
+struct IDEDevice {
+DeviceState qdev;
+uint32_t unit;
+BlockConf conf;
+int chs_trans;
+char *version;
+char *serial;
+char *model;
+uint64_t wwn;
+/*
+ * 0x- rotation rate not reported
+ * 0x0001- non-rotating medium (SSD)
+ * 0x0002-0x0400 - reserved
+ * 0x0401-0xffe  - rotations per minute
+ * 0x- reserved
+ */
+uint16_t rotation_rate;
+};
 
 typedef struct IDEDrive {
 IDEDevice dev;
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index a3a6702eec..1aab89b27b 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -8,23 +8,17 @@
  */
 
 #include "hw/ide.h"
-#include "sysemu/dma.h"
-#include "hw/block/block.h"
 #include "exec/ioport.h"
 #include "hw/ide/ide-dma.h"
+#include "hw/ide/ide-dev.h"
 
 /* debug IDE devices */
 #define USE_DMA_CDROM
 #include "qom/object.h"
 
-typedef struct IDEDevice IDEDevice;
-typede

[PATCH 19/28] qemu-img: resize: do not always eat last argument

[Michael Tokarev]

'qemu-img resize --help' does not work, since it wants more
arguments.  Also it -size is only recognized as a very last
argument, but it is common for tools to handle other options
after positional arguments too.

Tell getopt_long() to return non-options together with options,
and process filename and size in the loop, and check if there's
an argument right after filename which looks like -N (number),
and treat it as size (decrement).  This way we can handle --help,
and we can also have options after filename and size, and `--'
will be handled fine too.

The only case which is not handled right is when there's an option
between filename and size, and size is given as decrement, - in
this case -size will be treated as option, not as size.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 41 +++--
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 2a4bff2872..c8b0b68d67 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4296,7 +4296,7 @@ static int img_resize(const img_cmd_t *ccmd, int argc, 
char **argv)
 {
 Error *err = NULL;
 int c, ret, relative;
-const char *filename, *fmt, *size;
+const char *filename = NULL, *fmt = NULL, *size = NULL;
 int64_t n, total_size, current_size;
 bool quiet = false;
 BlockBackend *blk = NULL;
@@ -4319,17 +4319,7 @@ static int img_resize(const img_cmd_t *ccmd, int argc, 
char **argv)
 bool image_opts = false;
 bool shrink = false;
 
-/* Remove size from argv manually so that negative numbers are not treated
- * as options by getopt. */
-if (argc < 3) {
-error_exit(argv[0], "Not enough arguments");
-return 1;
-}
-
-size = argv[--argc];
-
 /* Parse getopt arguments */
-fmt = NULL;
 for(;;) {
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
@@ -4339,7 +4329,7 @@ static int img_resize(const img_cmd_t *ccmd, int argc, 
char **argv)
 {"shrink", no_argument, 0, OPTION_SHRINK},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":f:hq",
+c = getopt_long(argc, argv, "-:f:hq",
 long_options, NULL);
 if (c == -1) {
 break;
@@ -4377,12 +4367,35 @@ static int img_resize(const img_cmd_t *ccmd, int argc, 
char **argv)
 case OPTION_SHRINK:
 shrink = true;
 break;
+case 1: /* a non-optional argument */
+if (!filename) {
+filename = optarg;
+/* see if we have -size (number) next to filename */
+if (optind < argc) {
+size = argv[optind];
+if (size[0] == '-' && size[1] >= '0' && size[1] <= '9') {
+++optind;
+} else {
+size = NULL;
+}
+}
+} else if (!size) {
+size = optarg;
+} else {
+error_exit(argv[0], "Extra argument(s) in command line");
+}
+break;
 }
 }
-if (optind != argc - 1) {
+if (!filename && optind < argc) {
+filename = argv[optind++];
+}
+if (!size && optind < argc) {
+size = argv[optind++];
+}
+if (!filename || !size || optind < argc) {
 error_exit(argv[0], "Expecting image file name and size");
 }
-filename = argv[optind++];
 
 /* Choose grow, shrink, or absolute resize mode */
 switch (size[0]) {
-- 
2.39.2

[PULL 23/25] hw/ide: Remove the include/hw/ide.h legacy file

[Philippe Mathieu-Daudé]

From: Thomas Huth 

There was only one prototype left in this legacy file. Move it to
ide-dev.h to finally get rid of it.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
Acked-by: Mark Cave-Ayland 
Message-ID: <20240220085505.30255-7-th...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS   | 1 -
 include/hw/ide.h  | 9 -
 include/hw/ide/ide-dev.h  | 2 ++
 include/hw/ide/internal.h | 3 +--
 4 files changed, 3 insertions(+), 12 deletions(-)
 delete mode 100644 include/hw/ide.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 5535df4487..9dd98a923f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1937,7 +1937,6 @@ IDE
 M: John Snow 
 L: qemu-block@nongnu.org
 S: Odd Fixes
-F: include/hw/ide.h
 F: include/hw/ide/
 F: hw/ide/
 F: hw/block/block.c
diff --git a/include/hw/ide.h b/include/hw/ide.h
deleted file mode 100644
index db963bdb77..00
--- a/include/hw/ide.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef HW_IDE_H
-#define HW_IDE_H
-
-#include "exec/memory.h"
-
-/* ide/core.c */
-void ide_drive_get(DriveInfo **hd, int max_bus);
-
-#endif /* HW_IDE_H */
diff --git a/include/hw/ide/ide-dev.h b/include/hw/ide/ide-dev.h
index 1f62e58ebc..708cc0fda3 100644
--- a/include/hw/ide/ide-dev.h
+++ b/include/hw/ide/ide-dev.h
@@ -179,4 +179,6 @@ typedef struct IDEDrive {
 
 void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp);
 
+void ide_drive_get(DriveInfo **hd, int max_bus);
+
 #endif
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index d3ec16a945..20dde37f45 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -4,10 +4,9 @@
 /*
  * QEMU IDE Emulation -- internal header file
  * only files in hw/ide/ are supposed to include this file.
- * non-internal declarations are in hw/ide.h
+ * non-internal declarations are in hw/include/ide-*.h
  */
 
-#include "hw/ide.h"
 #include "hw/ide/ide-bus.h"
 
 /* debug IDE devices */
-- 
2.41.0

[PULL 10/25] hw/i386/pc: Do pc_cmos_init_late() from pc_machine_done()

[Philippe Mathieu-Daudé]

From: Peter Maydell 

In the i386 PC machine, we want to run the pc_cmos_init_late()
function only once the IDE and floppy drive devices have been set up.
We currently do this using qemu_register_reset(), and then have the
function call qemu_unregister_reset() on itself, so it runs exactly
once.

This was an expedient way to do it back in 2010 when we first added
this (in commit c0897e0cb94e8), but now we have a more obvious point
to do "machine initialization that has to happen after generic device
init": the machine-init-done hook.

Do the pc_cmos_init_late() work from our existing PC machine init
done hook function, so we can drop the use of qemu_register_reset()
and qemu_unregister_reset().

Because the pointers to the devices we need (the IDE buses and the
RTC) are now all in the machine state, we don't need the
pc_cmos_init_late_arg struct and can just pass the PCMachineState
pointer.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240220160622.114437-3-peter.mayd...@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc.c | 39 ---
 1 file changed, 16 insertions(+), 23 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 3e9ca6295f..1733dffc00 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -465,11 +465,6 @@ static void pc_cmos_init_floppy(MC146818RtcState 
*rtc_state, ISADevice *floppy)
 mc146818rtc_set_cmos_data(rtc_state, REG_EQUIPMENT_BYTE, val);
 }
 
-typedef struct pc_cmos_init_late_arg {
-MC146818RtcState *rtc_state;
-BusState *idebus[2];
-} pc_cmos_init_late_arg;
-
 typedef struct check_fdc_state {
 ISADevice *floppy;
 bool multiple;
@@ -530,23 +525,25 @@ static ISADevice *pc_find_fdc0(void)
 return state.floppy;
 }
 
-static void pc_cmos_init_late(void *opaque)
+static void pc_cmos_init_late(PCMachineState *pcms)
 {
-pc_cmos_init_late_arg *arg = opaque;
-MC146818RtcState *s = arg->rtc_state;
+X86MachineState *x86ms = X86_MACHINE(pcms);
+MC146818RtcState *s = MC146818_RTC(x86ms->rtc);
 int16_t cylinders;
 int8_t heads, sectors;
 int val;
 int i, trans;
 
 val = 0;
-if (arg->idebus[0] && ide_get_geometry(arg->idebus[0], 0,
-   &cylinders, &heads, §ors) >= 0) 
{
+if (pcms->idebus[0] &&
+ide_get_geometry(pcms->idebus[0], 0,
+ &cylinders, &heads, §ors) >= 0) {
 cmos_init_hd(s, 0x19, 0x1b, cylinders, heads, sectors);
 val |= 0xf0;
 }
-if (arg->idebus[0] && ide_get_geometry(arg->idebus[0], 1,
-   &cylinders, &heads, §ors) >= 0) 
{
+if (pcms->idebus[0] &&
+ide_get_geometry(pcms->idebus[0], 1,
+ &cylinders, &heads, §ors) >= 0) {
 cmos_init_hd(s, 0x1a, 0x24, cylinders, heads, sectors);
 val |= 0x0f;
 }
@@ -558,10 +555,11 @@ static void pc_cmos_init_late(void *opaque)
geometry.  It is always such that: 1 <= sects <= 63, 1
<= heads <= 16, 1 <= cylinders <= 16383. The BIOS
geometry can be different if a translation is done. */
-if (arg->idebus[i / 2] &&
-ide_get_geometry(arg->idebus[i / 2], i % 2,
+BusState *idebus = pcms->idebus[i / 2];
+if (idebus &&
+ide_get_geometry(idebus, i % 2,
  &cylinders, &heads, §ors) >= 0) {
-trans = ide_get_bios_chs_trans(arg->idebus[i / 2], i % 2) - 1;
+trans = ide_get_bios_chs_trans(idebus, i % 2) - 1;
 assert((trans & ~3) == 0);
 val |= trans << (i * 2);
 }
@@ -569,15 +567,12 @@ static void pc_cmos_init_late(void *opaque)
 mc146818rtc_set_cmos_data(s, 0x39, val);
 
 pc_cmos_init_floppy(s, pc_find_fdc0());
-
-qemu_unregister_reset(pc_cmos_init_late, opaque);
 }
 
 void pc_cmos_init(PCMachineState *pcms,
   ISADevice *rtc)
 {
 int val;
-static pc_cmos_init_late_arg arg;
 X86MachineState *x86ms = X86_MACHINE(pcms);
 MC146818RtcState *s = MC146818_RTC(rtc);
 
@@ -631,11 +626,7 @@ void pc_cmos_init(PCMachineState *pcms,
 val |= 0x04; /* PS/2 mouse installed */
 mc146818rtc_set_cmos_data(s, REG_EQUIPMENT_BYTE, val);
 
-/* hard drives and FDC */
-arg.rtc_state = s;
-arg.idebus[0] = pcms->idebus[0];
-arg.idebus[1] = pcms->idebus[1];
-qemu_register_reset(pc_cmos_init_late, &arg);
+/* hard drives and FDC are handled by pc_cmos_init_late() */
 }
 
 static void handle_a20_line_change(void *opaque, int irq, int level)
@@ -703,6 +694,8 @@ void pc_machine_done(Notifier *notifier, void *data)
 /* update FW_CFG_NB_CPUS to account for -device added CPUs */
 fw_cfg_modify_i16(x86ms->fw_cfg, FW_CFG_NB_CPUS, x86ms->boot_cpus);
 }
+
+pc_cmos_init_late(pcms);
 }
 
 void pc_guest_info_init(PCMachineState *pcms)
-- 
2.41.0

[PATCH 27/28] qemu-img: inline list of supported commands, remove qemu-img-cmds.h include

[Michael Tokarev]

also add short description to each command and use it in --help

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 41 ++---
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index ea284dca2d..299e34e470 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -61,6 +61,7 @@
 typedef struct img_cmd_t {
 const char *name;
 int (*handler)(const struct img_cmd_t *ccmd, int argc, char **argv);
+const char *description;
 } img_cmd_t;
 
 enum {
@@ -126,14 +127,14 @@ void cmd_help(const img_cmd_t *ccmd,
   const char *syntax, const char *arguments)
 {
 printf(
-"Usage:\n"
+"%s.  Usage:\n"
 "  %s %s %s"
 "\n"
 "Arguments:\n"
 "  -h, --help\n"
 " print this help and exit\n"
 "%s\n",
-   "qemu-img", ccmd->name,
+   ccmd->description, "qemu-img", ccmd->name,
syntax, arguments);
 exit(EXIT_SUCCESS);
 }
@@ -5824,10 +5825,36 @@ out:
 }
 
 static const img_cmd_t img_cmds[] = {
-#define DEF(option, callback, arg_string)\
-{ option, callback },
-#include "qemu-img-cmds.h"
-#undef DEF
+{ "amend", img_amend,
+  "Update format-specific options of the image" },
+{ "bench", img_bench,
+  "Run simple image benchmark" },
+{ "bitmap", img_bitmap,
+  "Perform modifications of the persistent bitmap in the image" },
+{ "check", img_check,
+  "Check basic image integrity" },
+{ "commit", img_commit,
+  "Commit image to its backing file" },
+{ "compare", img_compare,
+  "Check if two images have the same contents" },
+{ "convert", img_convert,
+  "Copy one image to another with optional format conversion" },
+{ "create", img_create,
+  "Create and format new image file" },
+{ "dd", img_dd,
+  "Copy input to output with optional format conversion" },
+{ "info", img_info,
+  "Display information about image" },
+{ "map", img_map,
+  "Dump image metadata" },
+{ "measure", img_measure,
+  "Calculate file size requred for a new image" },
+{ "rebase", img_rebase,
+  "Change backing file of the image" },
+{ "resize", img_resize,
+  "Resize the image to the new size" },
+{ "snapshot", img_snapshot,
+  "List or manipulate snapshots within image" },
 { NULL, NULL, },
 };
 
@@ -5892,7 +5919,7 @@ QEMU_IMG_VERSION
 "\n"
 "Recognized commands (run qemu-img COMMAND --help for command-specific 
help):\n\n");
 for (cmd = img_cmds; cmd->name != NULL; cmd++) {
-printf("  %s\n", cmd->name);
+printf("  %s - %s\n", cmd->name, cmd->description);
 }
 printf("\nSupported image formats:\n");
 c = 99; /* force a newline */
-- 
2.39.2

[PATCH 02/28] qemu-img: measure: convert img_size to signed, simplify handling

[Michael Tokarev]

qemu_opt_set_number() expects signed int64_t.

Use int64_t instead of uint64_t for img_size, use -1 as "unset"
value instead of UINT64_MAX, and do not require temporary sval
for conversion from string.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 5a756be600..ae14ed833d 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -5362,7 +5362,7 @@ static int img_measure(int argc, char **argv)
 QemuOpts *sn_opts = NULL;
 QemuOptsList *create_opts = NULL;
 bool image_opts = false;
-uint64_t img_size = UINT64_MAX;
+int64_t img_size = -1;
 BlockMeasureInfo *info = NULL;
 Error *local_err = NULL;
 int ret = 1;
@@ -5420,16 +5420,11 @@ static int img_measure(int argc, char **argv)
 }
 break;
 case OPTION_SIZE:
-{
-int64_t sval;
-
-sval = cvtnum("image size", optarg);
-if (sval < 0) {
+img_size = cvtnum("image size", optarg);
+if (img_size < 0) {
 goto out;
 }
-img_size = (uint64_t)sval;
-}
-break;
+break;
 }
 }
 
@@ -5444,11 +5439,11 @@ static int img_measure(int argc, char **argv)
 error_report("--image-opts, -f, and -l require a filename argument.");
 goto out;
 }
-if (filename && img_size != UINT64_MAX) {
+if (filename && img_size != -1) {
 error_report("--size N cannot be used together with a filename.");
 goto out;
 }
-if (!filename && img_size == UINT64_MAX) {
+if (!filename && img_size == -1) {
 error_report("Either --size N or one filename must be specified.");
 goto out;
 }
@@ -5496,7 +5491,7 @@ static int img_measure(int argc, char **argv)
 goto out;
 }
 }
-if (img_size != UINT64_MAX) {
+if (img_size != -1) {
 qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size, &error_abort);
 }
 
-- 
2.39.2

[PULL 07/25] hw/i2c/smbus_slave: Add object path on error prints

[Philippe Mathieu-Daudé]

From: Joe Komlodi 

The current logging doesn't tell us which specific smbus device is an
error state.

Signed-off-by: Joe Komlodi 
Reviewed-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240202204847.2062798-3-koml...@google.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i2c/smbus_slave.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/i2c/smbus_slave.c b/hw/i2c/smbus_slave.c
index 1300c9ec72..9f9afc25a4 100644
--- a/hw/i2c/smbus_slave.c
+++ b/hw/i2c/smbus_slave.c
@@ -25,11 +25,15 @@
 #define DPRINTF(fmt, ...) \
 do { printf("smbus(%02x): " fmt , dev->i2c.address, ## __VA_ARGS__); } while 
(0)
 #define BADF(fmt, ...) \
-do { fprintf(stderr, "smbus: error: " fmt , ## __VA_ARGS__); exit(1);} while 
(0)
+do { g_autofree char *qom_path = object_get_canonical_path(OBJECT(dev));  \
+fprintf(stderr, "%s: smbus: error: " fmt , qom_path, ## __VA_ARGS__); \
+exit(1); } while (0)
 #else
 #define DPRINTF(fmt, ...) do {} while(0)
 #define BADF(fmt, ...) \
-do { fprintf(stderr, "smbus: error: " fmt , ## __VA_ARGS__);} while (0)
+do { g_autofree char *qom_path = object_get_canonical_path(OBJECT(dev));  \
+fprintf(stderr, "%s: smbus: error: " fmt , qom_path, ## __VA_ARGS__); \
+ } while (0)
 #endif
 
 enum {
-- 
2.41.0

[PATCH 16/28] qemu-img: snapshot: make -l (list) the default, simplify option handling

[Michael Tokarev]

When no -l/-a/-c/-d specified, assume -l (list).

Use the same values for SNAPSHOT_LIST/etc constants as the
option chars (lacd), this makes it possible to simplify
option handling a lot, combining cases for 4 options into
one.

Also remove bdrv_oflags handling (only list can use RO mode).

Signed-off-by: Michael Tokarev 
---
 docs/tools/qemu-img.rst |  2 +-
 qemu-img.c  | 52 ++---
 2 files changed, 19 insertions(+), 35 deletions(-)

diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index 9b628c4da5..df184d15b9 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -256,7 +256,7 @@ Parameters to snapshot subcommand:
 
 .. option:: -l
 
-  Lists all snapshots in the given image
+  Lists all snapshots in the given image (default action)
 
 Command description:
 
diff --git a/qemu-img.c b/qemu-img.c
index 85c37d491d..ee35768af8 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3584,10 +3584,11 @@ out:
 return ret < 0;
 }
 
-#define SNAPSHOT_LIST   1
-#define SNAPSHOT_CREATE 2
-#define SNAPSHOT_APPLY  3
-#define SNAPSHOT_DELETE 4
+/* the same as options */
+#define SNAPSHOT_LIST   'l'
+#define SNAPSHOT_CREATE 'c'
+#define SNAPSHOT_APPLY  'a'
+#define SNAPSHOT_DELETE 'd'
 
 static int img_snapshot(const img_cmd_t *ccmd, int argc, char **argv)
 {
@@ -3595,7 +3596,7 @@ static int img_snapshot(const img_cmd_t *ccmd, int argc, 
char **argv)
 BlockDriverState *bs;
 QEMUSnapshotInfo sn;
 char *filename, *fmt = NULL, *snapshot_name = NULL;
-int c, ret = 0, bdrv_oflags;
+int c, ret = 0;
 int action = 0;
 bool quiet = false;
 Error *err = NULL;
@@ -3603,7 +3604,6 @@ static int img_snapshot(const img_cmd_t *ccmd, int argc, 
char **argv)
 bool force_share = false;
 int64_t rt;
 
-bdrv_oflags = BDRV_O_RDWR;
 /* Parse commandline parameters */
 for(;;) {
 static const struct option long_options[] = {
@@ -3631,36 +3631,15 @@ static int img_snapshot(const img_cmd_t *ccmd, int 
argc, char **argv)
 case 'f':
 fmt = optarg;
 break;
-case 'l':
-if (action) {
-error_exit(argv[0], "Cannot mix '-l', '-a', '-c', '-d'");
-return 0;
-}
-action = SNAPSHOT_LIST;
-bdrv_oflags &= ~BDRV_O_RDWR; /* no need for RW */
-break;
-case 'a':
+case SNAPSHOT_LIST:
+case SNAPSHOT_APPLY:
+case SNAPSHOT_CREATE:
+case SNAPSHOT_DELETE:
 if (action) {
 error_exit(argv[0], "Cannot mix '-l', '-a', '-c', '-d'");
 return 0;
 }
-action = SNAPSHOT_APPLY;
-snapshot_name = optarg;
-break;
-case 'c':
-if (action) {
-error_exit(argv[0], "Cannot mix '-l', '-a', '-c', '-d'");
-return 0;
-}
-action = SNAPSHOT_CREATE;
-snapshot_name = optarg;
-break;
-case 'd':
-if (action) {
-error_exit(argv[0], "Cannot mix '-l', '-a', '-c', '-d'");
-return 0;
-}
-action = SNAPSHOT_DELETE;
+action = c;
 snapshot_name = optarg;
 break;
 case 'q':
@@ -3683,9 +3662,14 @@ static int img_snapshot(const img_cmd_t *ccmd, int argc, 
char **argv)
 }
 filename = argv[optind++];
 
+if (!action) {
+action = SNAPSHOT_LIST;
+}
+
 /* Open the image */
-blk = img_open(image_opts, filename, fmt, bdrv_oflags, false, quiet,
-   force_share);
+blk = img_open(image_opts, filename, fmt,
+   action == SNAPSHOT_LIST ? 0 : BDRV_O_RDWR,
+   false, quiet, force_share);
 if (!blk) {
 return 1;
 }
-- 
2.39.2

[PATCH 13/28] qemu-img: info: refresh options/--help

[Michael Tokarev]

Add missing long options and --help output.
Also add -b short option for --backing-chain, and remove
now-unused OPTION_BACKING_CHAIN.

While at it, remove unused option_index variable.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 40 +++-
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 911cdc159c..cc51da31cf 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -65,7 +65,6 @@ typedef struct img_cmd_t {
 
 enum {
 OPTION_OUTPUT = 256,
-OPTION_BACKING_CHAIN = 257,
 OPTION_OBJECT = 258,
 OPTION_IMAGE_OPTS = 259,
 OPTION_PATTERN = 260,
@@ -3219,31 +3218,44 @@ static int img_info(const img_cmd_t *ccmd, int argc, 
char **argv)
 
 fmt = NULL;
 for(;;) {
-int option_index = 0;
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
 {"format", required_argument, 0, 'f'},
 {"output", required_argument, 0, OPTION_OUTPUT},
-{"backing-chain", no_argument, 0, OPTION_BACKING_CHAIN},
+{"backing-chain", no_argument, 0, 'b'},
 {"object", required_argument, 0, OPTION_OBJECT},
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 {"force-share", no_argument, 0, 'U'},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":f:hU",
-long_options, &option_index);
+c = getopt_long(argc, argv, "f:hbU",
+long_options, NULL);
 if (c == -1) {
 break;
 }
 switch(c) {
-case ':':
-missing_argument(argv[optind - 1]);
-break;
-case '?':
-unrecognized_option(argv[optind - 1]);
-break;
 case 'h':
-help();
+cmd_help(ccmd,
+"[-f FMT | --image-opts] [-b] [-U] [--object OBJDEF]\n"
+"[--output human|json] FILENAME\n"
+,
+"  -f, --format FMT\n"
+" specify FILENAME image format explicitly\n"
+"  --image-opts\n"
+" indicates that FILENAME is a complete image specification\n"
+" instead of a file name (incompatible with --format)\n"
+"  -b, --backing-chain\n"
+" display information about backing chaing\n"
+"  (in case the image is stacked\n"
+"  -U, --force-share\n"
+" open image in shared mode for concurrent access\n"
+"  --object OBJDEF\n"
+" QEMU user-creatable object (eg encryption key)\n"
+"  --output human|json\n"
+" specify output format name (default human)\n"
+"  FILENAME\n"
+" image file name (or specification with --image-opts)\n"
+);
 break;
 case 'f':
 fmt = optarg;
@@ -3254,7 +3266,7 @@ static int img_info(const img_cmd_t *ccmd, int argc, char 
**argv)
 case OPTION_OUTPUT:
 output_format = parse_output_format(argv[0], optarg);
 break;
-case OPTION_BACKING_CHAIN:
+case 'b':
 chain = true;
 break;
 case OPTION_OBJECT:
@@ -3263,6 +3275,8 @@ static int img_info(const img_cmd_t *ccmd, int argc, char 
**argv)
 case OPTION_IMAGE_OPTS:
 image_opts = true;
 break;
+default:
+tryhelp(argv[0]);
 }
 }
 if (optind != argc - 1) {
-- 
2.39.2

[PATCH 28/28] qemu-img: extend cvtnum() and use it in more places

[Michael Tokarev]

cvtnum() expects input string to specify some sort of size
(optionally with KMG... suffix).  However, there are a lot
of other number conversions in there (using qemu_strtol &Co),
also, not all conversions which use cvtnum, actually expects
size, - like dd count=nn.

Add bool issize argument to cvtnum() to specify if it should
treat the argument as a size or something else, - this changes
conversion routine in use and error text.

Use the new cvtnum() in more places (like where strtol were used),
since it never return negative number in successful conversion.
When it makes sense, also specify upper or lower bounds at the
same time.  This simplifies option processing in multiple places,
removing the need of local temporary variables and longer error
reporting code.

While at it, fix errors, like depth in measure must be >= 1,
while the previous code allowed it to be 0.

In a few places, change unsigned variables (like of type size_t)
to be signed instead, - to avoid the need of temporary conversion
variable.  All these variables are okay to be signed, we never
assign <0 value to them except of the cases of conversion error,
where we return immediately.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 118 -
 1 file changed, 44 insertions(+), 74 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 299e34e470..a066c4cfc4 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -397,18 +397,23 @@ static int add_old_style_options(const char *fmt, 
QemuOpts *opts,
 return 0;
 }
 
-static int64_t cvtnum_full(const char *name, const char *value, int64_t min,
-   int64_t max)
+static int64_t cvtnum_full(const char *name, const char *value,
+   bool issize, int64_t min, int64_t max)
 {
 int err;
 uint64_t res;
 
-err = qemu_strtosz(value, NULL, &res);
+err = issize ? qemu_strtosz(value, NULL, &res) :
+   qemu_strtou64(value, NULL, 0, &res);
 if (err < 0 && err != -ERANGE) {
-error_report("Invalid %s specified. You may use "
- "k, M, G, T, P or E suffixes for", name);
-error_report("kilobytes, megabytes, gigabytes, terabytes, "
- "petabytes and exabytes.");
+if (issize) {
+error_report("Invalid %s specified. You may use "
+ "k, M, G, T, P or E suffixes for", name);
+error_report("kilobytes, megabytes, gigabytes, terabytes, "
+ "petabytes and exabytes.");
+} else {
+error_report("Invalid %s specified.", name);
+}
 return err;
 }
 if (err == -ERANGE || res > max || res < min) {
@@ -419,9 +424,9 @@ static int64_t cvtnum_full(const char *name, const char 
*value, int64_t min,
 return res;
 }
 
-static int64_t cvtnum(const char *name, const char *value)
+static int64_t cvtnum(const char *name, const char *value, bool issize)
 {
-return cvtnum_full(name, value, 0, INT64_MAX);
+return cvtnum_full(name, value, issize, 0, INT64_MAX);
 }
 
 static int img_create(const img_cmd_t *ccmd, int argc, char **argv)
@@ -525,7 +530,7 @@ static int img_create(const img_cmd_t *ccmd, int argc, char 
**argv)
 
 /* Get image size, if specified */
 if (optind < argc) {
-img_size = cvtnum("image size", argv[optind++]);
+img_size = cvtnum("image size", argv[optind++], true);
 if (img_size < 0) {
 goto fail;
 }
@@ -987,7 +992,7 @@ static int img_commit(const img_cmd_t *ccmd, int argc, char 
**argv)
 quiet = true;
 break;
 case 'r':
-rate_limit = cvtnum("rate limit", optarg);
+rate_limit = cvtnum("rate limit", optarg, true);
 if (rate_limit < 0) {
 return 1;
 }
@@ -2412,7 +2417,7 @@ static int img_convert(const img_cmd_t *ccmd, int argc, 
char **argv)
 {
 int64_t sval;
 
-sval = cvtnum("buffer size for sparse output", optarg);
+sval = cvtnum("buffer size for sparse output", optarg, true);
 if (sval < 0) {
 goto fail_getopt;
 } else if (!QEMU_IS_ALIGNED(sval, BDRV_SECTOR_SIZE) ||
@@ -2444,10 +2449,9 @@ static int img_convert(const img_cmd_t *ccmd, int argc, 
char **argv)
 skip_create = true;
 break;
 case 'm':
-if (qemu_strtol(optarg, NULL, 0, &s.num_coroutines) ||
-s.num_coroutines < 1 || s.num_coroutines > MAX_COROUTINES) {
-error_report("Invalid number of coroutines. Allowed number of"
- " coroutines is between 1 and %d", 
MAX_COROUTINES);
+s.num_coroutines = cvtnum_full("number of coroutines", optarg,
+   false, 1, MAX_COROUTINES);
+if (s.num_coroutines < 0) {
 goto fail_getopt;
 }
 break;
@@ -

[PATCH 20/28] qemu-img: resize: refresh options/--help

[Michael Tokarev]

Add missing long options and --help output.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 37 -
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index c8b0b68d67..45fbef5d37 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4323,27 +4323,44 @@ static int img_resize(const img_cmd_t *ccmd, int argc, 
char **argv)
 for(;;) {
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
+{"quiet", no_argument, 0, 'q'},
 {"object", required_argument, 0, OPTION_OBJECT},
+{"format", required_argument, 0, 'f'},
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 {"preallocation", required_argument, 0, OPTION_PREALLOCATION},
 {"shrink", no_argument, 0, OPTION_SHRINK},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, "-:f:hq",
+c = getopt_long(argc, argv, "-f:hq",
 long_options, NULL);
 if (c == -1) {
 break;
 }
 switch(c) {
-case ':':
-missing_argument(argv[optind - 1]);
-break;
-case '?':
-unrecognized_option(argv[optind - 1]);
-break;
 case 'h':
-help();
-break;
+cmd_help(ccmd,
+"[-f FMT | --image-opts] [--preallocation PREALLOC] [--shrink]\n"
+"[--object OBJECTDEF] [-q] FILENAME [+|-]SIZE\n"
+,
+"  -q, --quiet\n"
+" quiet operation\n"
+"  -f, --format FMT\n"
+" specify FILENAME format explicitly\n"
+"  --image-opts\n"
+" indicates that FILENAME is a complete image specification\n"
+"   instead of a file name (incompatible with --format)\n"
+"  --shrink\n"
+" allow operation when new size is smaller than original\n"
+"  --preallocation PREALLOC\n"
+" specify preallocation type for the new areas\n"
+"  --object OBJDEF\n"
+" QEMU user-creatable object (eg encryption key)\n"
+"  FILENAME\n"
+" image file (specification) to resize\n"
+"  SIZE\n"
+" new image size or amount by which to shrink/grow\n"
+);
+return 0;
 case 'f':
 fmt = optarg;
 break;
@@ -4385,6 +4402,8 @@ static int img_resize(const img_cmd_t *ccmd, int argc, 
char **argv)
 error_exit(argv[0], "Extra argument(s) in command line");
 }
 break;
+default:
+tryhelp(argv[0]);
 }
 }
 if (!filename && optind < argc) {
-- 
2.39.2

[PATCH v2 00/28] qemu-img: refersh options and --help handling, cleanups

[Michael Tokarev]

Quite big patchset trying to implement normal, readable qemu-img --help
(and qemu-img COMMAND --help) output with readable descriptions, and
adding many long options in the process.

In the end I stopped using qemu-img-opts.hx in qemu-img.c, perhaps
this can be avoided, with only list of commands and their desrciptions
kept there, but I don't see big advantage here.  The same list should
be included in docs/tools/qemu-img.rst, - this is not done now.

Also each command syntax isn't reflected in the doc for now, because
I want to give good names for options first, - and there, we've quite
some inconsistences and questions.  For example, measure --output=OFMT
-O OFMT, - this is priceless :)  I've no idea why we have this ugly
--output=json thing, why not have --json? ;)  I gave the desired
format long name --target-format to avoid clash with --output.

For rebase, src vs tgt probably should be renamed in local variables
too, and I'm not even sure I've got the caches right. For caches,
the thing is inconsistent across commands.

For compare, I used --a-format/--b-format (for -f/-F), - this can
be made --souce-format and --target-format, to compare source (file1)
with target (file2).

For bitmap, things are scary, I'm not sure what -b SRC_FILENAME
really means, - for now I gave it --source option, but this does
not make it more clear, suggestions welcome.

There are many other inconsistencies, I can't fix them all in one
go.. :)

Changes since v1:

 - reformatted help text to be less condensed
 - added cleanups (first 3 patches and last patch)
 - change argv[0] handling and getopt error reporting to
   fix inherent bug (see patch 4 "global option processing"
   for details)
 - removed missing_argument() & unrecognized_option()
   and handling of '?' and ':' getopt return values
 - more robust handling of resize filename -size vs options
   ("resize: do not always eat last argument")
 - larger cleanup in snapshot mode handling
   ("snapshot: make -l (list) the default...")
 - cvtnum and number conversion and bugfixes
   ("extend cvtnum() and use it in more places")
 - removed unused option_index variable in two places
 - added a few fixmes
 - various other minor changes

I kept Dan's R-b for a few patches he reviewed
despite the changed, - hopefully it's okay, since
the new changes are not related to the initial ones.
Keeping him in Cc for that.

Michael Tokarev (28):
  qemu-img: stop printing error twice in a few places
  qemu-img: measure: convert img_size to signed, simplify handling
  qemu-img: create: convert img_size to signed, simplify handling
  qemu-img: global option processing and error printing
  qemu-img: pass current cmd info into command handlers
  qemu-img: create: refresh options/--help
  qemu-img: factor out parse_output_format() and use it in the code
  qemu-img: check: refresh options/--help
  qemu-img: simplify --repair error message
  qemu-img: commit: refresh options/--help
  qemu-img: compare: refresh options/--help
  qemu-img: convert: refresh options/--help
  qemu-img: info: refresh options/--help
  qemu-img: map: refresh options/--help
  qemu-img: snapshot: allow specifying -f fmt
  qemu-img: snapshot: make -l (list) the default
  qemu-img: snapshot: refresh options/--help
  qemu-img: rebase: refresh options/--help
  qemu-img: resize: do not always eat last argument
  qemu-img: resize: refresh options/--help
  qemu-img: amend: refresh options/--help
  qemu-img: bench: refresh options/--help
  qemu-img: bitmap: refresh options/--help
  qemu-img: dd: refresh options/--help
  qemu-img: measure: refresh options/--help
  qemu-img: implement short --help, remove global help() function
  qemu-img: inline list of supported commands, remove qemu-img-cmds.h
include
  qemu-img: extend cvtnum() and use it in more places

 docs/tools/qemu-img.rst |4 +-
 qemu-img-cmds.hx|4 +-
 qemu-img.c  | 1143 +++
 3 files changed, 670 insertions(+), 481 deletions(-)

-- 
2.39.2

[PULL 15/25] hw/i386/pc_sysfw: Inline pc_system_flash_create() and remove it

[Philippe Mathieu-Daudé]

From: Bernhard Beschow 

pc_system_flash_create() checked for pcmc->pci_enabled which is redundant since
its caller already checked it. The method can be turned into just two lines, so
inline and remove it.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240208220349.4948-8-shen...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc_sysfw.c | 15 ++-
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index b4c3833352..2dcaa116ad 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -91,18 +91,6 @@ static PFlashCFI01 *pc_pflash_create(PCMachineState *pcms,
 return PFLASH_CFI01(dev);
 }
 
-static void pc_system_flash_create(PCMachineState *pcms)
-{
-PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
-
-if (pcmc->pci_enabled) {
-pcms->flash[0] = pc_pflash_create(pcms, "system.flash0",
-  "pflash0");
-pcms->flash[1] = pc_pflash_create(pcms, "system.flash1",
-  "pflash1");
-}
-}
-
 static void pc_system_flash_cleanup_unused(PCMachineState *pcms)
 {
 char *prop_name;
@@ -212,7 +200,8 @@ void pc_system_firmware_init(PCMachineState *pcms,
 return;
 }
 
-pc_system_flash_create(pcms);
+pcms->flash[0] = pc_pflash_create(pcms, "system.flash0", "pflash0");
+pcms->flash[1] = pc_pflash_create(pcms, "system.flash1", "pflash1");
 
 /* Map legacy -drive if=pflash to machine properties */
 for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {
-- 
2.41.0

[PATCH 12/28] qemu-img: convert: refresh options/--help

[Michael Tokarev]

Add missing long options and --help output.

convert uses -B for --backing, - why not -b?

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 90 --
 1 file changed, 81 insertions(+), 9 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index fd61b25ea7..911cdc159c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2403,30 +2403,100 @@ static int img_convert(const img_cmd_t *ccmd, int 
argc, char **argv)
 for(;;) {
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
+{"quiet", no_argument, 0, 'q'},
 {"object", required_argument, 0, OPTION_OBJECT},
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+{"source-image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+{"source-format", required_argument, 0, 'f'},
+{"source-cache", required_argument, 0, 'T'},
+{"snapshot", required_argument, 0, 'l'},
+{"sparse-size", required_argument, 0, 'S'},
+{"output-format", required_argument, 0, 'O'},
+{"options", required_argument, 0, 'o'},
+{"output-cache", required_argument, 0, 't'},
+{"backing", required_argument, 0, 'B'},
+{"backing-format", required_argument, 0, 'F'},
 {"force-share", no_argument, 0, 'U'},
 {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
 {"salvage", no_argument, 0, OPTION_SALVAGE},
 {"target-is-zero", no_argument, 0, OPTION_TARGET_IS_ZERO},
 {"bitmaps", no_argument, 0, OPTION_BITMAPS},
 {"skip-broken-bitmaps", no_argument, 0, OPTION_SKIP_BROKEN},
+{"rate", required_argument, 0, 'r'},
+{"parallel", required_argument, 0, 'm'},
+{"oob-writes", no_argument, 0, 'W'},
+{"copy-range-offloading", no_argument, 0, 'C'},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":hf:O:B:CcF:o:l:S:pt:T:qnm:WUr:",
+c = getopt_long(argc, argv, "hf:O:B:CcF:o:l:S:pt:T:qnm:WUr:",
 long_options, NULL);
 if (c == -1) {
 break;
 }
-switch(c) {
-case ':':
-missing_argument(argv[optind - 1]);
-break;
-case '?':
-unrecognized_option(argv[optind - 1]);
-break;
+switch (c) {
 case 'h':
-help();
+cmd_help(ccmd,
+"[-f SRC_FMT|--image-opts] [-T SRC_CACHE] [--bitmaps 
[--skip-broken-bitmaps]]\n"
+"[-o TGT_OPTS|--target-image-opts] [-t TGT_CACHE] [-n]\n"
+"[-B BACKING_FILENAME [-F BACKING_FMT]]\n"
+"SRC_FILENAME [SRC_FILENAME2 [...]] TGT_FILENAME\n"
+,
+"  -q, --quiet\n"
+" quiet operations\n"
+"  -p, --progress\n"
+" show operation progress\n"
+"  -f, --source-format SRC_FMT\n"
+" specify SRC_FILENAME source image format explicitly\n"
+"  --source-image-opts\n"
+" indicates that SRC_FILENAME is a complete image specification\n"
+" instead of a file name (incompatible with --source-format)\n"
+"  -l, --source-snapshot SNAPSHOT_PARAMS\n"
+" specify source snapshot parameters\n"
+"  -T, --source-cache SRC_CACHE\n"
+" source image(s) cache mode (" BDRV_DEFAULT_CACHE ")\n"
+"  -O, --target-format TGT_FMT\n"
+" specify TGT_FILENAME image format (default is raw)\n"
+"  --target-image-opts\n"
+" indicates that TGT_FILENAME is a complete image specification\n"
+" instead of a file name (incompatible with --output-format)\n"
+"  -o, --target-options TGT_OPTS\n"
+" TARGET_FMT-specific options\n"
+"  -c, --compress\n"
+" create compressed output image (qcow and qcow2 format only)\n"
+"  -t, --target-cache TGT_CACHE\n"
+" cache mode when opening output image (unsafe)\n"
+"  -B, --backing BACKING_FILENAME\n"
+" create output to be a CoW on top of BACKING_FILENAME\n"
+"  -F, --backing-format BACKING_FMT\n"
+" specify BACKING_FILENAME image format explicitly\n"
+"  -n, --no-create\n"
+" omit target volume creation (eg on rbd)\n"
+"  --target-is-zero\n"
+"  -S, --sparse-size SPARSE_SIZE\n"
+" XXX todo\n"
+"  --bitmaps\n"
+" also copy any persistent bitmaps present in source\n"
+"  --skip-broken-bitmaps\n"
+" skip (do not error out) any broken bitmaps\n"
+"  -U, --force-share\n"
+" open images in shared mode for concurrent access\n"
+"  -r, --rate RATE\n"
+" I/O rate limit\n"
+"  -m, --parallel NUM_COROUTINES\n"
+" specify parallelism (default 8)\n"
+"  -C, --copy-range-offloading\n"
+" use copy_range offloading\n"
+"  --salvage\n"
+" XXX todo\n"
+"  -W, --oob-writes\n"
+" enable out-of-order writes to improve performance\n"
+"  --object OBJDEF\n"
+" QEMU user-creatable object (eg encryption key)\n"
+"  SRC_FILENAME\n"
+" source image file name (or specification with --image-opts)\n"
+"  TGT_FILENAME\n"
+" target (output) image file name\n"
+);
   

[PATCH 14/28] qemu-img: map: refresh options/--help

[Michael Tokarev]

Add missing long options and --help output.

While at it, remove unused option_index variable.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 34 --
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index cc51da31cf..3f719bbecf 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3453,7 +3453,6 @@ static int img_map(const img_cmd_t *ccmd, int argc, char 
**argv)
 
 fmt = NULL;
 for (;;) {
-int option_index = 0;
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
 {"format", required_argument, 0, 'f'},
@@ -3465,20 +3464,33 @@ static int img_map(const img_cmd_t *ccmd, int argc, 
char **argv)
 {"max-length", required_argument, 0, 'l'},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":f:s:l:hU",
-long_options, &option_index);
+c = getopt_long(argc, argv, "f:s:l:hU",
+long_options, NULL);
 if (c == -1) {
 break;
 }
 switch (c) {
-case ':':
-missing_argument(argv[optind - 1]);
-break;
-case '?':
-unrecognized_option(argv[optind - 1]);
-break;
 case 'h':
-help();
+cmd_help(ccmd,
+"[-f FMT | --image-opts] [--object OBJDEF] [--output human|json]\n"
+"[--start-offset OFFSET] [--max-length LENGTH] [-U] FILENAME\n"
+,
+"  -f, --format FMT\n"
+" specify FILENAME image format explicitly\n"
+"  --image-opts\n"
+" indicates that FILENAME is a complete image specification\n"
+" instead of a file name (incompatible with --format)\n"
+"  --start-offset OFFSET\n"
+"  --max-length LENGTH\n"
+"  --output human|json\n"
+" specify output format name (default human)\n"
+"  -U, --force-share\n"
+" open image in shared mode for concurrent access\n"
+"  --object OBJDEF\n"
+" QEMU user-creatable object (eg encryption key)\n"
+"  FILENAME\n"
+" image file name (or specification with --image-opts)\n"
+);
 break;
 case 'f':
 fmt = optarg;
@@ -3507,6 +3519,8 @@ static int img_map(const img_cmd_t *ccmd, int argc, char 
**argv)
 case OPTION_IMAGE_OPTS:
 image_opts = true;
 break;
+default:
+tryhelp(argv[0]);
 }
 }
 if (optind != argc - 1) {
-- 
2.39.2

[PULL 14/25] hw/i386/pc: Confine system flash handling to pc_sysfw

[Philippe Mathieu-Daudé]

From: Bernhard Beschow 

Rather than distributing PC system flash handling across three files, let's
confine it to one. Now, pc_system_firmware_init() creates, configures and cleans
up the system flash which makes the code easier to understand. It also avoids
the extra call to pc_system_flash_cleanup_unused() in the Xen case.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240208220349.4948-7-shen...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/i386/pc.h | 2 --
 hw/i386/pc.c | 1 -
 hw/i386/pc_piix.c| 1 -
 hw/i386/pc_sysfw.c   | 6 --
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 0a8a96600c..e8f4af5d5c 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -193,8 +193,6 @@ void pc_i8259_create(ISABus *isa_bus, qemu_irq *i8259_irqs);
 #define TYPE_PORT92 "port92"
 
 /* pc_sysfw.c */
-void pc_system_flash_create(PCMachineState *pcms);
-void pc_system_flash_cleanup_unused(PCMachineState *pcms);
 void pc_system_firmware_init(PCMachineState *pcms, MemoryRegion *rom_memory);
 bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
int *data_len);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e526498164..1ee41a5e56 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1733,7 +1733,6 @@ static void pc_machine_initfn(Object *obj)
 #endif
 pcms->default_bus_bypass_iommu = false;
 
-pc_system_flash_create(pcms);
 pcms->pcspk = isa_new(TYPE_PC_SPEAKER);
 object_property_add_alias(OBJECT(pcms), "pcspk-audiodev",
   OBJECT(pcms->pcspk), "audiodev");
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 34203927e1..ec7c07b362 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -231,7 +231,6 @@ static void pc_init1(MachineState *machine,
 assert(machine->ram_size == x86ms->below_4g_mem_size +
 x86ms->above_4g_mem_size);
 
-pc_system_flash_cleanup_unused(pcms);
 if (machine->kernel_filename != NULL) {
 /* For xen HVM direct kernel boot, load linux here */
 xen_load_linux(pcms);
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index c8d9e71b88..b4c3833352 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -91,7 +91,7 @@ static PFlashCFI01 *pc_pflash_create(PCMachineState *pcms,
 return PFLASH_CFI01(dev);
 }
 
-void pc_system_flash_create(PCMachineState *pcms)
+static void pc_system_flash_create(PCMachineState *pcms)
 {
 PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
 
@@ -103,7 +103,7 @@ void pc_system_flash_create(PCMachineState *pcms)
 }
 }
 
-void pc_system_flash_cleanup_unused(PCMachineState *pcms)
+static void pc_system_flash_cleanup_unused(PCMachineState *pcms)
 {
 char *prop_name;
 int i;
@@ -212,6 +212,8 @@ void pc_system_firmware_init(PCMachineState *pcms,
 return;
 }
 
+pc_system_flash_create(pcms);
+
 /* Map legacy -drive if=pflash to machine properties */
 for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {
 pflash_cfi01_legacy_drive(pcms->flash[i],
-- 
2.41.0

[PULL 13/25] hw/i386/pc: Defer smbios_set_defaults() to machine_done

[Philippe Mathieu-Daudé]

From: Bernhard Beschow 

Handling most of smbios data generation in the machine_done notifier is similar
to how the ARM virt machine handles it which also calls smbios_set_defaults()
there. The result is that all pc machines are freed from explicitly worrying
about smbios setup.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240208220349.4948-6-shen...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/fw_cfg.h |  3 ++-
 include/hw/i386/pc.h |  1 -
 hw/i386/fw_cfg.c | 12 +++-
 hw/i386/pc.c |  2 +-
 hw/i386/pc_piix.c| 10 --
 hw/i386/pc_q35.c |  9 -
 6 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/hw/i386/fw_cfg.h b/hw/i386/fw_cfg.h
index 86ca7c1c0c..1e1de6b4a3 100644
--- a/hw/i386/fw_cfg.h
+++ b/hw/i386/fw_cfg.h
@@ -10,6 +10,7 @@
 #define HW_I386_FW_CFG_H
 
 #include "hw/boards.h"
+#include "hw/i386/pc.h"
 #include "hw/nvram/fw_cfg.h"
 
 #define FW_CFG_IO_BASE 0x510
@@ -22,7 +23,7 @@
 FWCfgState *fw_cfg_arch_create(MachineState *ms,
uint16_t boot_cpus,
uint16_t apic_id_limit);
-void fw_cfg_build_smbios(MachineState *ms, FWCfgState *fw_cfg);
+void fw_cfg_build_smbios(PCMachineState *ms, FWCfgState *fw_cfg);
 void fw_cfg_build_feature_control(MachineState *ms, FWCfgState *fw_cfg);
 void fw_cfg_add_acpi_dsdt(Aml *scope, FWCfgState *fw_cfg);
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 39cdb9b933..0a8a96600c 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -12,7 +12,6 @@
 #include "hw/hotplug.h"
 #include "qom/object.h"
 #include "hw/i386/sgx-epc.h"
-#include "hw/firmware/smbios.h"
 #include "hw/cxl/cxl.h"
 
 #define HPET_INTCAP "hpet-intcap"
diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index 7362daa45a..98a478c276 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -48,15 +48,25 @@ const char *fw_cfg_arch_key_name(uint16_t key)
 return NULL;
 }
 
-void fw_cfg_build_smbios(MachineState *ms, FWCfgState *fw_cfg)
+void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg)
 {
 #ifdef CONFIG_SMBIOS
 uint8_t *smbios_tables, *smbios_anchor;
 size_t smbios_tables_len, smbios_anchor_len;
 struct smbios_phys_mem_area *mem_array;
 unsigned i, array_count;
+MachineState *ms = MACHINE(pcms);
+PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+MachineClass *mc = MACHINE_GET_CLASS(pcms);
 X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
 
+if (pcmc->smbios_defaults) {
+/* These values are guest ABI, do not change */
+smbios_set_defaults("QEMU", mc->desc, mc->name,
+pcmc->smbios_legacy_mode, 
pcmc->smbios_uuid_encoded,
+pcms->smbios_entry_point_type);
+}
+
 /* tell smbios about cpuid version and features */
 smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 694de8e130..e526498164 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -689,7 +689,7 @@ void pc_machine_done(Notifier *notifier, void *data)
 
 acpi_setup();
 if (x86ms->fw_cfg) {
-fw_cfg_build_smbios(MACHINE(pcms), x86ms->fw_cfg);
+fw_cfg_build_smbios(pcms, x86ms->fw_cfg);
 fw_cfg_build_feature_control(MACHINE(pcms), x86ms->fw_cfg);
 /* update FW_CFG_NB_CPUS to account for -device added CPUs */
 fw_cfg_modify_i16(x86ms->fw_cfg, FW_CFG_NB_CPUS, x86ms->boot_cpus);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index eeca725504..34203927e1 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -36,7 +36,6 @@
 #include "hw/rtc/mc146818rtc.h"
 #include "hw/southbridge/piix.h"
 #include "hw/display/ramfb.h"
-#include "hw/firmware/smbios.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_ids.h"
 #include "hw/usb.h"
@@ -225,15 +224,6 @@ static void pc_init1(MachineState *machine,
&error_abort);
 }
 
-if (pcmc->smbios_defaults) {
-MachineClass *mc = MACHINE_GET_CLASS(machine);
-/* These values are guest ABI, do not change */
-smbios_set_defaults("QEMU", mc->desc,
-mc->name, pcmc->smbios_legacy_mode,
-pcmc->smbios_uuid_encoded,
-pcms->smbios_entry_point_type);
-}
-
 /* allocate ram and load rom/bios */
 if (!xen_enabled()) {
 pc_memory_init(pcms, system_memory, rom_memory, hole64_size);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 8053d8cccb..ab7750c346 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -45,7 +45,6 @@
 #include "hw/i386/amd_iommu.h"
 #include "hw/i386/intel_iommu.h"
 #include "hw/display/ramfb.h"
-#include "hw/firmware/smbios.h"
 #include "hw/ide/pci.h"
 #include "hw/ide/ahci-pci.h"
 #include "hw/intc/ioapic.h"
@@ -188,14 +187,6 @@ static void pc_q35_init(MachineState *machine)
 k

[PATCH 26/28] qemu-img: implement short --help, remove global help() function

[Michael Tokarev]

now once all individual subcommands has --help support, remove
the large unreadable help() thing and replace it with small
global --help, which refers to individual command --help for
more info.

While at it, also line-wrap list of formats after 75 chars.

Since missing_argument() and unrecognized_option() are now unused,
remove them.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 172 -
 1 file changed, 39 insertions(+), 133 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index d72e1f565b..ea284dca2d 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -94,11 +94,6 @@ typedef enum OutputFormat {
 /* Default to cache=writeback as data integrity is not important for qemu-img 
*/
 #define BDRV_DEFAULT_CACHE "writeback"
 
-static void format_print(void *opaque, const char *name)
-{
-printf(" %s", name);
-}
-
 static G_NORETURN
 void tryhelp(const char *argv0)
 {
@@ -118,18 +113,6 @@ void error_exit(const char *argv0, const char *fmt, ...)
 tryhelp(argv0);
 }
 
-static G_NORETURN
-void missing_argument(const char *option)
-{
-error_exit("qemu-img", "missing argument for option '%s'", option);
-}
-
-static G_NORETURN
-void unrecognized_option(const char *option)
-{
-error_exit("qemu-img", "unrecognized option '%s'", option);
-}
-
 /*
  * Print --help output for a command and exit.
  * syntax and description are multi-line with trailing EOL
@@ -166,114 +149,6 @@ static OutputFormat parse_output_format(const char 
*argv0, const char *arg)
 }
 }
 
-/* Please keep in synch with docs/tools/qemu-img.rst */
-static G_NORETURN
-void help(void)
-{
-const char *help_msg =
-   QEMU_IMG_VERSION
-   "usage: qemu-img [standard options] command [command options]\n"
-   "QEMU disk image utility\n"
-   "\n"
-   "'-h', '--help'   display this help and exit\n"
-   "'-V', '--version'output version information and exit\n"
-   "'-T', '--trace'  
[[enable=]][,events=][,file=]\n"
-   " specify tracing options\n"
-   "\n"
-   "Command syntax:\n"
-#define DEF(option, callback, arg_string)\
-   "  " arg_string "\n"
-#include "qemu-img-cmds.h"
-#undef DEF
-   "\n"
-   "Command parameters:\n"
-   "  'filename' is a disk image filename\n"
-   "  'objectdef' is a QEMU user creatable object definition. See the 
qemu(1)\n"
-   "manual page for a description of the object properties. The 
most common\n"
-   "object type is a 'secret', which is used to supply passwords 
and/or\n"
-   "encryption keys.\n"
-   "  'fmt' is the disk image format. It is guessed automatically in 
most cases\n"
-   "  'cache' is the cache mode used to write the output disk image, 
the valid\n"
-   "options are: 'none', 'writeback' (default, except for 
convert), 'writethrough',\n"
-   "'directsync' and 'unsafe' (default for convert)\n"
-   "  'src_cache' is the cache mode used to read input disk images, 
the valid\n"
-   "options are the same as for the 'cache' option\n"
-   "  'size' is the disk image size in bytes. Optional suffixes\n"
-   "'k' or 'K' (kilobyte, 1024), 'M' (megabyte, 1024k), 'G' 
(gigabyte, 1024M),\n"
-   "'T' (terabyte, 1024G), 'P' (petabyte, 1024T) and 'E' (exabyte, 
1024P)  are\n"
-   "supported. 'b' is ignored.\n"
-   "  'output_filename' is the destination disk image filename\n"
-   "  'output_fmt' is the destination format\n"
-   "  'options' is a comma separated list of format specific options 
in a\n"
-   "name=value format. Use -o help for an overview of the options 
supported by\n"
-   "the used format\n"
-   "  'snapshot_param' is param used for internal snapshot, format\n"
-   "is 'snapshot.id=[ID],snapshot.name=[NAME]', or\n"
-   "'[ID_OR_NAME]'\n"
-   "  '-c' indicates that target image must be compressed (qcow format 
only)\n"
-   "  '-u' allows unsafe backing chains. For rebasing, it is assumed 
that old and\n"
-   "   new backing file match exactly. The image doesn't need a 
working\n"
-   "   backing file before rebasing in this case (useful for 
renaming the\n"
-   "   backing file). For image creation, allow creating without 
attempting\n"
-   "   to open the backing file.\n"
-   "  '-h' with or without a command shows this help and lists the 
supported formats\n"
-   "  '-p' show progress of command (only certain commands)\n"
-   "  '-q' use Quiet mode - do not print any output (except errors)\n"
-   "  '-S' indicates the consecutive number of bytes (defaults to 4k) 
that must\n"
-   "   contain only zeros for qemu-img to create a sparse image 
during\n"
-

[PATCH 18/28] qemu-img: rebase: refresh options/--help

[Michael Tokarev]

Add missing long options and --help output.

Options added:
 --format, --cache - for the image in question
 --backing, --backing-format, --backing-cache, --backing-unsafe -
   for the new backing file
(was eg CACHE vs SRC_CACHE, which is unclear).

Probably should rename local variables.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 55 +-
 1 file changed, 46 insertions(+), 9 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index ce939708d4..2a4bff2872 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3792,26 +3792,61 @@ static int img_rebase(const img_cmd_t *ccmd, int argc, 
char **argv)
 for(;;) {
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
+{"quiet", no_argument, 0, 'q'},
+{"progress", no_argument, 0, 'p'},
 {"object", required_argument, 0, OPTION_OBJECT},
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 {"force-share", no_argument, 0, 'U'},
+{"format", required_argument, 0, 'f'},
+{"cache", required_argument, 0, 't'},
 {"compress", no_argument, 0, 'c'},
+{"backing", required_argument, 0, 'b'},
+{"backing-format", required_argument, 0, 'F'},
+{"backing-cache", required_argument, 0, 'T'},
+{"backing-unsafe", no_argument, 0, 'u'},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":hf:F:b:upt:T:qUc",
+c = getopt_long(argc, argv, "hf:F:b:upt:T:qUc",
 long_options, NULL);
 if (c == -1) {
 break;
 }
-switch(c) {
-case ':':
-missing_argument(argv[optind - 1]);
-break;
-case '?':
-unrecognized_option(argv[optind - 1]);
-break;
+switch (c) {
 case 'h':
-help();
+cmd_help(ccmd,
+"[-f FMT | --image-opts] [-t CACHE] [-q] [-U] [-p]\n"
+"[-b BACKING_FILENAME [-F BACKING_FMT] [-T BACKING_CACHE]] [-u]\n"
+"[--object OBJDEF] [-c] FILENAME\n"
+"Rebases FILENAME on top of BACKING_FILENAME or no backing file\n"
+,
+"  -q, --quiet\n"
+" quiet operation\n"
+"  -p, --progress\n"
+" show progress indicator\n"
+"  -f, --format FMT\n"
+" specify FILENAME format explicitly\n"
+"  --image-opts\n"
+" indicates that FILENAME is a complete image specification\n"
+" instead of a file name (incompatible with --format)\n"
+"  -t, --cache CACHE\n"
+" cache mode for FILENAME (" BDRV_DEFAULT_CACHE ")\n"
+"  -b, --backing BACKING_FILENAME|\"\"\n"
+" rebase onto this file (or no backing file)\n"
+"  -F, --backing-format BACKING_FMT\n"
+" specify format for BACKING_FILENAME\n"
+"  -T, --backing-cache CACHE\n"
+" BACKING_FILENAME cache mode (" BDRV_DEFAULT_CACHE ")\n"
+"  -u, --backing-unsafe\n"
+" do not fail if BACKING_FILENAME can not be read\n"
+"  -c, --compress\n"
+" compress image (when image supports this)\n"
+"  -U, --force-share\n"
+" open image in shared mode for concurrent access\n"
+"  --object OBJDEF\n"
+" QEMU user-creatable object (eg encryption key)\n"
+"  FILENAME\n"
+" image file name (or specification with --image-opts)\n"
+);
 return 0;
 case 'f':
 fmt = optarg;
@@ -3849,6 +3884,8 @@ static int img_rebase(const img_cmd_t *ccmd, int argc, 
char **argv)
 case 'c':
 compress = true;
 break;
+default:
+tryhelp(argv[0]);
 }
 }
 
-- 
2.39.2

[PATCH 24/28] qemu-img: dd: refresh options/--help

[Michael Tokarev]

Add missing long options and --help output.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 39 +--
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index e4027ece20..af7841573c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -5502,31 +5502,48 @@ static int img_dd(const img_cmd_t *ccmd, int argc, char 
**argv)
 const struct option long_options[] = {
 { "help", no_argument, 0, 'h'},
 { "object", required_argument, 0, OPTION_OBJECT},
+{ "format", required_argument, 0, 'f'},
+{ "output-format", required_argument, 0, 'O'},
 { "image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 { "force-share", no_argument, 0, 'U'},
 { 0, 0, 0, 0 }
 };
 
-while ((c = getopt_long(argc, argv, ":hf:O:U", long_options, NULL))) {
+while ((c = getopt_long(argc, argv, "hf:O:U", long_options, NULL))) {
 if (c == EOF) {
 break;
 }
 switch (c) {
+case 'h':
+cmd_help(ccmd,
+"[-f FMT|--image-opts] [-O OUTPUT_FMT] [-U]\n"
+"[bs=BLOCK_SIZE] [count=BLOCKS] if=INPUT of=OUTPUT\n"
+,
+"  -f, --format FMT\n"
+" specify format for INPUT explicitly\n"
+"  --image-opts\n"
+" indicates that INPUT is a complete image specification\n"
+" instead of a file name (incompatible with --format)\n"
+"  -O, --output-format OUTPUT_FMT\n"
+" format of the OUTPUT (default raw)\n"
+"  -U, --force-share\n"
+" open images in shared mode for concurrent access\n"
+"  bs=BLOCK_SIZE[kKMGTP]\n"
+" size of I/O block (default 512)\n"
+"  count=COUNT\n"
+" number of blocks to convert (default whole INPUT)\n"
+"  if=INPUT\n"
+" input file name (or image specification with --image-opts)\n"
+"  of=OUTPUT\n"
+" output file name to create\n"
+);
+break;
 case 'O':
 out_fmt = optarg;
 break;
 case 'f':
 fmt = optarg;
 break;
-case ':':
-missing_argument(argv[optind - 1]);
-break;
-case '?':
-unrecognized_option(argv[optind - 1]);
-break;
-case 'h':
-help();
-break;
 case 'U':
 force_share = true;
 break;
@@ -5536,6 +5553,8 @@ static int img_dd(const img_cmd_t *ccmd, int argc, char 
**argv)
 case OPTION_IMAGE_OPTS:
 image_opts = true;
 break;
+default:
+tryhelp(argv[0]);
 }
 }
 
-- 
2.39.2

[PULL 25/25] hw/sparc/leon3: Fix wrong usage of DO_UPCAST macro

[Philippe Mathieu-Daudé]

From: Thomas Huth 

leon3.c currently fails to compile with some compilers when the -Wvla
option has been enabled:

 ../hw/sparc/leon3.c: In function ‘leon3_cpu_reset’:
 ../hw/sparc/leon3.c:153:5: error: ISO C90 forbids variable length array
  ‘offset_must_be_zero’ [-Werror=vla]
   153 | ResetData *s = (ResetData *)DO_UPCAST(ResetData, info[id], info);
   | ^
 cc1: all warnings being treated as errors

Looking at this code, the DO_UPCAST macro is indeed used in a wrong way
here: DO_UPCAST is supposed to check that the second parameter is the
first entry of the struct that the first parameter indicates, but since
we use and index into the info[] array, this of course cannot work.

The intention here was likely rather to use the container_of() macro
instead, so switch the code accordingly.

Fixes: d65aba8286 ("hw/sparc/leon3: implement multiprocessor")
Signed-off-by: Thomas Huth 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240221180751.190489-1-th...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/sparc/leon3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 4873b59b6c..6aaa04cb19 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -150,7 +150,7 @@ static void leon3_cpu_reset(void *opaque)
 {
 struct CPUResetData *info = (struct CPUResetData *) opaque;
 int id = info->id;
-ResetData *s = (ResetData *)DO_UPCAST(ResetData, info[id], info);
+ResetData *s = container_of(info, ResetData, info[id]);
 CPUState *cpu = CPU(s->info[id].cpu);
 CPUSPARCState *env = cpu_env(cpu);
 
-- 
2.41.0

[PATCH 11/28] qemu-img: compare: refresh options/--help

[Michael Tokarev]

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 45 +
 1 file changed, 37 insertions(+), 8 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 1271217272..fd61b25ea7 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1487,25 +1487,52 @@ static int img_compare(const img_cmd_t *ccmd, int argc, 
char **argv)
 for (;;) {
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
+{"quiet", no_argument, 0, 'q'},
 {"object", required_argument, 0, OPTION_OBJECT},
+{"cache", required_argument, 0, 'T'},
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+{"a-format", required_argument, 0, 'f'},
+{"left-format", required_argument, 0, 'f'},
+{"b-format", required_argument, 0, 'F'},
+{"right-format", required_argument, 0, 'F'},
 {"force-share", no_argument, 0, 'U'},
+{"strict", no_argument, 0, 's'},
+{"progress", no_argument, 0, 'p'},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":hf:F:T:pqsU",
+c = getopt_long(argc, argv, "hf:F:T:pqsU",
 long_options, NULL);
 if (c == -1) {
 break;
 }
 switch (c) {
-case ':':
-missing_argument(argv[optind - 1]);
-break;
-case '?':
-unrecognized_option(argv[optind - 1]);
-break;
 case 'h':
-help();
+cmd_help(ccmd,
+"[--image-opts | [-f FMT] [-F FMT]] [-s]\n"
+"[-T CACHE] [-U] [--object OBJDEF] FILENAME1 FILENAME2\n"
+,
+"  -q, --quiet\n"
+" quiet operation\n"
+"  -p, --progress\n"
+" show operation progress\n"
+"  -f, --a-format FMT\n"
+" specify FILENAME1 image format explicitly\n"
+"  -F, --b-format FMT\n"
+" specify FILENAME2 image format explicitly\n"
+"  --image-opts\n"
+" indicates that FILENAMEs are complete image specifications\n"
+" instead of file names (incompatible with --a-format and --b-format)\n"
+"  -s, --strict\n"
+" strict mode, also check if sizes are equal\n"
+"  -T, --cache CACHE_MODE\n"
+" images caching mode (" BDRV_DEFAULT_CACHE ")\n"
+"  -U, --force-share\n"
+" open images in shared mode for concurrent access\n"
+"  --object OBJDEF\n"
+" QEMU user-creatable object (eg encryption key)\n"
+"  FILENAME1, FILENAME2\n"
+" image files (or specifications) to compare\n"
+);
 break;
 case 'f':
 fmt1 = optarg;
@@ -1546,6 +1573,8 @@ static int img_compare(const img_cmd_t *ccmd, int argc, 
char **argv)
 case OPTION_IMAGE_OPTS:
 image_opts = true;
 break;
+default:
+tryhelp(argv[0]);
 }
 }
 
-- 
2.39.2

[PULL 19/25] hw/ide: Split qdev.c into ide-bus.c and ide-dev.c

[Philippe Mathieu-Daudé]

From: Thomas Huth 

qdev.c is a mixture between IDE bus specific functions and IDE device
functions. Let's split it up to make it more obvious which part is
related to bus handling and which part is related to device handling.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
Acked-by: Mark Cave-Ayland 
Message-ID: <20240220085505.30255-3-th...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ide/ide-bus.c | 111 +++
 hw/ide/{qdev.c => ide-dev.c} |  87 +--
 hw/arm/Kconfig   |   2 +
 hw/ide/Kconfig   |  30 ++
 hw/ide/meson.build   |   3 +-
 5 files changed, 134 insertions(+), 99 deletions(-)
 create mode 100644 hw/ide/ide-bus.c
 rename hw/ide/{qdev.c => ide-dev.c} (78%)

diff --git a/hw/ide/ide-bus.c b/hw/ide/ide-bus.c
new file mode 100644
index 00..57fe67b29c
--- /dev/null
+++ b/hw/ide/ide-bus.c
@@ -0,0 +1,111 @@
+/*
+ * ide bus support for qdev.
+ *
+ * Copyright (c) 2009 Gerd Hoffmann 
+ *
+ * This library 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 .
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "qemu/module.h"
+#include "hw/ide/internal.h"
+#include "sysemu/block-backend.h"
+#include "sysemu/blockdev.h"
+#include "sysemu/runstate.h"
+
+static char *idebus_get_fw_dev_path(DeviceState *dev);
+static void idebus_unrealize(BusState *qdev);
+
+static void ide_bus_class_init(ObjectClass *klass, void *data)
+{
+BusClass *k = BUS_CLASS(klass);
+
+k->get_fw_dev_path = idebus_get_fw_dev_path;
+k->unrealize = idebus_unrealize;
+}
+
+static void idebus_unrealize(BusState *bus)
+{
+IDEBus *ibus = IDE_BUS(bus);
+
+if (ibus->vmstate) {
+qemu_del_vm_change_state_handler(ibus->vmstate);
+}
+}
+
+static const TypeInfo ide_bus_info = {
+.name = TYPE_IDE_BUS,
+.parent = TYPE_BUS,
+.instance_size = sizeof(IDEBus),
+.class_init = ide_bus_class_init,
+};
+
+void ide_bus_init(IDEBus *idebus, size_t idebus_size, DeviceState *dev,
+ int bus_id, int max_units)
+{
+qbus_init(idebus, idebus_size, TYPE_IDE_BUS, dev, NULL);
+idebus->bus_id = bus_id;
+idebus->max_units = max_units;
+}
+
+static char *idebus_get_fw_dev_path(DeviceState *dev)
+{
+char path[30];
+
+snprintf(path, sizeof(path), "%s@%x", qdev_fw_name(dev),
+ ((IDEBus *)dev->parent_bus)->bus_id);
+
+return g_strdup(path);
+}
+
+IDEDevice *ide_bus_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
+{
+DeviceState *dev;
+
+dev = qdev_new(drive->media_cd ? "ide-cd" : "ide-hd");
+qdev_prop_set_uint32(dev, "unit", unit);
+qdev_prop_set_drive_err(dev, "drive", blk_by_legacy_dinfo(drive),
+&error_fatal);
+qdev_realize_and_unref(dev, &bus->qbus, &error_fatal);
+return DO_UPCAST(IDEDevice, qdev, dev);
+}
+
+int ide_get_geometry(BusState *bus, int unit,
+ int16_t *cyls, int8_t *heads, int8_t *secs)
+{
+IDEState *s = &DO_UPCAST(IDEBus, qbus, bus)->ifs[unit];
+
+if (s->drive_kind != IDE_HD || !s->blk) {
+return -1;
+}
+
+*cyls = s->cylinders;
+*heads = s->heads;
+*secs = s->sectors;
+return 0;
+}
+
+int ide_get_bios_chs_trans(BusState *bus, int unit)
+{
+return DO_UPCAST(IDEBus, qbus, bus)->ifs[unit].chs_trans;
+}
+
+static void ide_bus_register_type(void)
+{
+type_register_static(&ide_bus_info);
+}
+
+type_init(ide_bus_register_type)
diff --git a/hw/ide/qdev.c b/hw/ide/ide-dev.c
similarity index 78%
rename from hw/ide/qdev.c
rename to hw/ide/ide-dev.c
index 4189313d30..15d088fd06 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/ide-dev.c
@@ -1,5 +1,5 @@
 /*
- * ide bus support for qdev.
+ * IDE device functions
  *
  * Copyright (c) 2009 Gerd Hoffmann 
  *
@@ -18,71 +18,21 @@
  */
 
 #include "qemu/osdep.h"
-#include "sysemu/dma.h"
 #include "qapi/error.h"
 #include "qapi/qapi-types-block.h"
 #include "qemu/error-report.h"
-#include "qemu/main-loop.h"
 #include "qemu/module.h"
 #include "hw/ide/ide-dev.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
 #include "sysemu/sysemu.h"
-#include "sysemu/runstate.h"
 #include "qapi/visitor.h"
 
-/* - */
-
-static char *idebus_get_fw_dev_path(DeviceState *dev);
-static void idebus_unrealize(B

[PATCH 23/28] qemu-img: bitmap: refresh options/--help

[Michael Tokarev]

Add missing long options and --help output.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 40 
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 8455832d34..e4027ece20 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -5168,20 +5168,42 @@ static int img_bitmap(const img_cmd_t *ccmd, int argc, 
char **argv)
 {"source-format", required_argument, 0, 'F'},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":b:f:F:g:h", long_options, NULL);
+c = getopt_long(argc, argv, "b:f:F:g:h",
+long_options, NULL);
 if (c == -1) {
 break;
 }
 
 switch (c) {
-case ':':
-missing_argument(argv[optind - 1]);
-break;
-case '?':
-unrecognized_option(argv[optind - 1]);
-break;
 case 'h':
-help();
+cmd_help(ccmd,
+"( --merge SOURCE | --add | --remove | --clear |\n"
+"--enable | --disable ).. [-f FMT | --image-opts]\n"
+"[ -b SRC_FILENAME [-F SOURCE_FMT]] [-g GRANULARITY] [--object 
OBJDEF]\n"
+"FILENAME BITMAP\n"
+,
+"  -f, --format FMT\n"
+" specify FILENAME format explicitly\n"
+"  --image-opts\n"
+" indicates that FILENAME is a complete image specification\n"
+" instead of a file name (incompatible with --format)\n"
+"  --add\n"
+" creates BITMAP, enables to record future edits\n"
+"   -g, --granularity GRANULARITY\n"
+" sets non-default granularity for --add\n"
+"  --remove\n"
+" removes BITMAP\n"
+"  --clear\n"
+" clears BITMAP\n"
+"  --enable, --disable\n"
+" starts and stops recording future edits to BITMAP\n"
+"  --merge SRC_FILENAME\n"
+" merges contents of SRC_FILENAME bitmap into BITMAP\n"
+"   -b, --source-file SRC_FILENAME\n"
+" select alternative source file for --merge\n"
+"   -F, --source-format SRC_FMT\n"
+" specify format for SRC_FILENAME explicitly\n"
+);
 break;
 case 'b':
 src_filename = optarg;
@@ -5237,6 +5259,8 @@ static int img_bitmap(const img_cmd_t *ccmd, int argc, 
char **argv)
 case OPTION_IMAGE_OPTS:
 image_opts = true;
 break;
+default:
+tryhelp(argv[0]);
 }
 }
 
-- 
2.39.2

[PULL 04/25] hw/ppc/ppc4xx_pci: Extract PCI host definitions to hw/pci-host/ppc4xx.h

[Philippe Mathieu-Daudé]

Reviewed-by: BALATON Zoltan 
Reviewed-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20240215105017.57748-3-phi...@linaro.org>
---
 MAINTAINERS  |  1 +
 include/hw/pci-host/ppc4xx.h | 17 +
 include/hw/ppc/ppc4xx.h  |  5 -
 hw/ppc/ppc440_bamboo.c   |  1 +
 hw/ppc/ppc440_pcix.c |  2 +-
 hw/ppc/ppc440_uc.c   |  1 +
 hw/ppc/ppc4xx_pci.c  |  2 +-
 hw/ppc/sam460ex.c|  1 +
 8 files changed, 23 insertions(+), 7 deletions(-)
 create mode 100644 include/hw/pci-host/ppc4xx.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 7d61fb9319..9b6ce9a934 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2070,6 +2070,7 @@ F: hw/ppc/ppc4xx*.c
 F: hw/ppc/ppc440_uc.c
 F: hw/ppc/ppc440.h
 F: hw/i2c/ppc4xx_i2c.c
+F: include/hw/pci-host/ppc4xx.h
 F: include/hw/ppc/ppc4xx.h
 F: include/hw/i2c/ppc4xx_i2c.h
 F: hw/intc/ppc-uic.c
diff --git a/include/hw/pci-host/ppc4xx.h b/include/hw/pci-host/ppc4xx.h
new file mode 100644
index 00..32396417fc
--- /dev/null
+++ b/include/hw/pci-host/ppc4xx.h
@@ -0,0 +1,17 @@
+/*
+ * QEMU PowerPC 4xx PCI-host definitions
+ *
+ * Copyright (c) 2018-2023 BALATON Zoltan
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_PCIHOST_PPC4XX_H
+#define HW_PCIHOST_PPC4XX_H
+
+#define TYPE_PPC4xx_HOST_BRIDGE "ppc4xx-host-bridge"
+#define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
+#define TYPE_PPC440_PCIX_HOST "ppc440-pcix-host"
+#define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
+
+#endif
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index ea7740239b..1bd9b8821b 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -29,11 +29,6 @@
 #include "exec/memory.h"
 #include "hw/sysbus.h"
 
-#define TYPE_PPC4xx_HOST_BRIDGE "ppc4xx-host-bridge"
-#define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
-#define TYPE_PPC440_PCIX_HOST "ppc440-pcix-host"
-#define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
-
 /*
  * Generic DCR device
  */
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index c75c3083e6..e18f57efce 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -24,6 +24,7 @@
 #include "elf.h"
 #include "hw/char/serial.h"
 #include "hw/ppc/ppc.h"
+#include "hw/pci-host/ppc4xx.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/reset.h"
 #include "hw/sysbus.h"
diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index d84418cb7b..1926ae2a27 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -25,7 +25,7 @@
 #include "qemu/module.h"
 #include "qemu/units.h"
 #include "hw/irq.h"
-#include "hw/ppc/ppc4xx.h"
+#include "hw/pci-host/ppc4xx.h"
 #include "hw/pci/pci_device.h"
 #include "hw/pci/pci_host.h"
 #include "trace.h"
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index 7d6ca70387..1312aa2080 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -14,6 +14,7 @@
 #include "qemu/log.h"
 #include "hw/irq.h"
 #include "hw/ppc/ppc4xx.h"
+#include "hw/pci-host/ppc4xx.h"
 #include "hw/qdev-properties.h"
 #include "hw/pci/pci.h"
 #include "sysemu/reset.h"
diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index e4101398c9..b6c6c8993c 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -24,7 +24,7 @@
 #include "qemu/osdep.h"
 #include "qemu/log.h"
 #include "hw/irq.h"
-#include "hw/ppc/ppc4xx.h"
+#include "hw/pci-host/ppc4xx.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
 #include "sysemu/reset.h"
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 1e615b8d35..a28498f39c 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -25,6 +25,7 @@
 #include "elf.h"
 #include "exec/memory.h"
 #include "ppc440.h"
+#include "hw/pci-host/ppc4xx.h"
 #include "hw/block/flash.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/reset.h"
-- 
2.41.0

[PATCH 08/28] qemu-img: check: refresh options/--help

[Michael Tokarev]

Add missing long options and --help output.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 38 ++
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 01894c097b..69fa9701e9 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -804,7 +804,9 @@ static int img_check(const img_cmd_t *ccmd, int argc, char 
**argv)
 int option_index = 0;
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
+{"quiet", no_argument, 0, 'q'},
 {"format", required_argument, 0, 'f'},
+{"cache", required_argument, 0, 'T'},
 {"repair", required_argument, 0, 'r'},
 {"output", required_argument, 0, OPTION_OUTPUT},
 {"object", required_argument, 0, OPTION_OBJECT},
@@ -812,20 +814,38 @@ static int img_check(const img_cmd_t *ccmd, int argc, 
char **argv)
 {"force-share", no_argument, 0, 'U'},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":hf:r:T:qU",
+c = getopt_long(argc, argv, "hf:r:T:qU",
 long_options, &option_index);
 if (c == -1) {
 break;
 }
 switch(c) {
-case ':':
-missing_argument(argv[optind - 1]);
-break;
-case '?':
-unrecognized_option(argv[optind - 1]);
-break;
 case 'h':
-help();
+cmd_help(ccmd,
+"[-f FMT | --image-opts] [-T CACHE_MODE] [-r] [-u]\n"
+"[--output human|json] [--object OBJDEF] FILENAME\n"
+,
+"  -q, --quiet\n"
+" quiet operations\n"
+"  -f, --format FMT\n"
+" specifies format of the image explicitly\n"
+"  --image-opts\n"
+" indicates that FILENAME is a complete image specification\n"
+" instead of a file name (incompatible with --format)\n"
+"  -T, --cache CACHE_MODE\n"
+" image cache mode (" BDRV_DEFAULT_CACHE ")\n"
+"  -U, --force-share\n"
+" open image in shared mode for concurrent access\n"
+"  --output human|json\n"
+" output format\n"
+"  -r, --repair leaks|all\n"
+" repair particular aspect of the image\n"
+" (image will be open in read-write mode, incompatible with 
--force-share)\n"
+"  --object OBJDEF\n"
+" QEMU user-creatable object (eg encryption key)\n"
+"  FILENAME\n"
+" the image file (or image specification) to operate on\n"
+);
 break;
 case 'f':
 fmt = optarg;
@@ -860,6 +880,8 @@ static int img_check(const img_cmd_t *ccmd, int argc, char 
**argv)
 case OPTION_IMAGE_OPTS:
 image_opts = true;
 break;
+default:
+tryhelp(argv[0]);
 }
 }
 if (optind != argc - 1) {
-- 
2.39.2

[PATCH 25/28] qemu-img: measure: refresh options/--help

[Michael Tokarev]

Add missing long options and --help output.

Also add -s short option for --size (and remove OPTION_SIZE).

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 53 -
 1 file changed, 40 insertions(+), 13 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index af7841573c..d72e1f565b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -71,7 +71,6 @@ enum {
 OPTION_FLUSH_INTERVAL = 261,
 OPTION_NO_DRAIN = 262,
 OPTION_TARGET_IMAGE_OPTS = 263,
-OPTION_SIZE = 264,
 OPTION_PREALLOCATION = 265,
 OPTION_SHRINK = 266,
 OPTION_SALVAGE = 267,
@@ -5744,15 +5743,6 @@ static void 
dump_json_block_measure_info(BlockMeasureInfo *info)
 
 static int img_measure(const img_cmd_t *ccmd, int argc, char **argv)
 {
-static const struct option long_options[] = {
-{"help", no_argument, 0, 'h'},
-{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
-{"object", required_argument, 0, OPTION_OBJECT},
-{"output", required_argument, 0, OPTION_OUTPUT},
-{"size", required_argument, 0, OPTION_SIZE},
-{"force-share", no_argument, 0, 'U'},
-{0, 0, 0, 0}
-};
 OutputFormat output_format = OFORMAT_HUMAN;
 BlockBackend *in_blk = NULL;
 BlockDriver *drv;
@@ -5773,12 +5763,47 @@ static int img_measure(const img_cmd_t *ccmd, int argc, 
char **argv)
 int ret = 1;
 int c;
 
+static const struct option long_options[] = {
+{"help", no_argument, 0, 'h'},
+{"target-format", required_argument, 0, 'O'},
+{"format", required_argument, 0, 'f'},
+{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+{"options", required_argument, 0, 'o'},
+{"snapshot", required_argument, 0, 'l'},
+{"object", required_argument, 0, OPTION_OBJECT},
+{"output", required_argument, 0, OPTION_OUTPUT},
+{"size", required_argument, 0, 's'},
+{"force-share", no_argument, 0, 'U'},
+{0, 0, 0, 0}
+};
+
 while ((c = getopt_long(argc, argv, "hf:O:o:l:U",
 long_options, NULL)) != -1) {
 switch (c) {
-case '?':
 case 'h':
-help();
+cmd_help(ccmd,
+"[-f FMT|--image-opts] [-o OPTIONS] [-O OUTPUT_FMT]\n"
+"   [--output OFMT] [--object OBJDEF] [-l SNAPSHOT_PARAM]\n"
+"   (--size SIZE | FILENAME)\n"
+,
+"  -O, --target-format FMT\n"
+" desired target/output image format (default raw)\n"
+"  -s, --size SIZE\n"
+" measure file size for given image size\n"
+"  FILENAME\n"
+" measure file size required to convert from FILENAME\n"
+"  -f, --format\n"
+" specify format of FILENAME explicitly\n"
+"  --image-opts\n"
+" indicates that FILENAME is a complete image specification\n"
+" instead of a file name (incompatible with --format)\n"
+"  -l, --snapshot SNAPSHOT\n"
+" use this snapshot in FILENAME as source\n"
+"  --output human|json\n"
+" output format\n"
+"  -U, --force-share\n"
+" open images in shared mode for concurrent access\n"
+);
 break;
 case 'f':
 fmt = optarg;
@@ -5816,12 +5841,14 @@ static int img_measure(const img_cmd_t *ccmd, int argc, 
char **argv)
 case OPTION_OUTPUT:
 output_format = parse_output_format(argv[0], optarg);
 break;
-case OPTION_SIZE:
+case 's':
 img_size = cvtnum("image size", optarg);
 if (img_size < 0) {
 goto out;
 }
 break;
+default:
+tryhelp(argv[0]);
 }
 }
 
-- 
2.39.2

[PATCH 07/28] qemu-img: factor out parse_output_format() and use it in the code

[Michael Tokarev]

Use common code and simplify error message

Signed-off-by: Michael Tokarev 
Reviewed-by: Daniel P. Berrangé 
---
 qemu-img.c | 63 --
 1 file changed, 18 insertions(+), 45 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 7e4c993b9c..01894c097b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -157,6 +157,17 @@ void cmd_help(const img_cmd_t *ccmd,
 exit(EXIT_SUCCESS);
 }
 
+static OutputFormat parse_output_format(const char *argv0, const char *arg)
+{
+if (!strcmp(arg, "json")) {
+return OFORMAT_JSON;
+} else if (!strcmp(arg, "human")) {
+return OFORMAT_HUMAN;
+} else {
+error_exit(argv0, "--output expects 'human' or 'json' not '%s'", arg);
+}
+}
+
 /* Please keep in synch with docs/tools/qemu-img.rst */
 static G_NORETURN
 void help(void)
@@ -775,7 +786,7 @@ static int img_check(const img_cmd_t *ccmd, int argc, char 
**argv)
 {
 int c, ret;
 OutputFormat output_format = OFORMAT_HUMAN;
-const char *filename, *fmt, *output, *cache;
+const char *filename, *fmt, *cache;
 BlockBackend *blk;
 BlockDriverState *bs;
 int fix = 0;
@@ -787,7 +798,6 @@ static int img_check(const img_cmd_t *ccmd, int argc, char 
**argv)
 bool force_share = false;
 
 fmt = NULL;
-output = NULL;
 cache = BDRV_DEFAULT_CACHE;
 
 for(;;) {
@@ -833,7 +843,7 @@ static int img_check(const img_cmd_t *ccmd, int argc, char 
**argv)
 }
 break;
 case OPTION_OUTPUT:
-output = optarg;
+output_format = parse_output_format(argv[0], optarg);
 break;
 case 'T':
 cache = optarg;
@@ -857,15 +867,6 @@ static int img_check(const img_cmd_t *ccmd, int argc, char 
**argv)
 }
 filename = argv[optind++];
 
-if (output && !strcmp(output, "json")) {
-output_format = OFORMAT_JSON;
-} else if (output && !strcmp(output, "human")) {
-output_format = OFORMAT_HUMAN;
-} else if (output) {
-error_report("--output must be used with human or json as argument.");
-return 1;
-}
-
 ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
 if (ret < 0) {
 error_report("Invalid source cache option: %s", cache);
@@ -3059,13 +3060,12 @@ static int img_info(const img_cmd_t *ccmd, int argc, 
char **argv)
 int c;
 OutputFormat output_format = OFORMAT_HUMAN;
 bool chain = false;
-const char *filename, *fmt, *output;
+const char *filename, *fmt;
 BlockGraphInfoList *list;
 bool image_opts = false;
 bool force_share = false;
 
 fmt = NULL;
-output = NULL;
 for(;;) {
 int option_index = 0;
 static const struct option long_options[] = {
@@ -3100,7 +3100,7 @@ static int img_info(const img_cmd_t *ccmd, int argc, char 
**argv)
 force_share = true;
 break;
 case OPTION_OUTPUT:
-output = optarg;
+output_format = parse_output_format(argv[0], optarg);
 break;
 case OPTION_BACKING_CHAIN:
 chain = true;
@@ -3118,15 +3118,6 @@ static int img_info(const img_cmd_t *ccmd, int argc, 
char **argv)
 }
 filename = argv[optind++];
 
-if (output && !strcmp(output, "json")) {
-output_format = OFORMAT_JSON;
-} else if (output && !strcmp(output, "human")) {
-output_format = OFORMAT_HUMAN;
-} else if (output) {
-error_report("--output must be used with human or json as argument.");
-return 1;
-}
-
 list = collect_image_info_list(image_opts, filename, fmt, chain,
force_share);
 if (!list) {
@@ -3285,7 +3276,7 @@ static int img_map(const img_cmd_t *ccmd, int argc, char 
**argv)
 OutputFormat output_format = OFORMAT_HUMAN;
 BlockBackend *blk;
 BlockDriverState *bs;
-const char *filename, *fmt, *output;
+const char *filename, *fmt;
 int64_t length;
 MapEntry curr = { .length = 0 }, next;
 int ret = 0;
@@ -3295,7 +3286,6 @@ static int img_map(const img_cmd_t *ccmd, int argc, char 
**argv)
 int64_t max_length = -1;
 
 fmt = NULL;
-output = NULL;
 for (;;) {
 int option_index = 0;
 static const struct option long_options[] = {
@@ -3331,7 +3321,7 @@ static int img_map(const img_cmd_t *ccmd, int argc, char 
**argv)
 force_share = true;
 break;
 case OPTION_OUTPUT:
-output = optarg;
+output_format = parse_output_format(argv[0], optarg);
 break;
 case 's':
 start_offset = cvtnum("start offset", optarg);
@@ -3358,15 +3348,6 @@ static int img_map(const img_cmd_t *ccmd, int argc, char 
**argv)
 }
 filename = argv[optind];
 
-if (output && !strcmp(output, "json")) {
-output_format = OFORMAT_JSON;
-} else if (output && !strcmp(output, "human")) {
-output_format = OFORMAT_HUMAN;
-} else if (output) 

[PATCH 21/28] qemu-img: amend: refresh options/--help

[Michael Tokarev]

Add missing long options and --help output.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 34 ++
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 45fbef5d37..0d17738fb6 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4557,26 +4557,42 @@ static int img_amend(const img_cmd_t *ccmd, int argc, 
char **argv)
 for (;;) {
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
+{"quiet", no_argument, 0, 'q'},
+{"progress", no_argument, 0, 'p'},
 {"object", required_argument, 0, OPTION_OBJECT},
+{"format", required_argument, 0, 'f'},
+{"cache", required_argument, 0, 't'},
+{"options", required_argument, 0, 'o'},
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 {"force", no_argument, 0, OPTION_FORCE},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":ho:f:t:pq",
+c = getopt_long(argc, argv, "ho:f:t:pq",
 long_options, NULL);
 if (c == -1) {
 break;
 }
 
 switch (c) {
-case ':':
-missing_argument(argv[optind - 1]);
-break;
-case '?':
-unrecognized_option(argv[optind - 1]);
-break;
 case 'h':
-help();
+cmd_help(ccmd,
+"[-f FMT | --image-opts] [t CACHE] [--force] [-p] [-q]\n"
+"[--object OBJDEF -o OPTIONS FILENAME\n"
+,
+"  -q, --quiet\n"
+" quiet operation\n"
+"  -p, --progres\n"
+" show progress\n"
+"  -f, --format FMT\n"
+" specify FILENAME format explicitly\n"
+"  --image-opts\n"
+" indicates that FILENAME is a complete image specification\n"
+"   instead of a file name (incompatible with --format)\n"
+"  -t, --cache CACHE\n"
+" cache mode for FILENAME (" BDRV_DEFAULT_CACHE ")\n"
+"  --force\n"
+" allow certain unsafe operations\n"
+);
 break;
 case 'o':
 if (accumulate_options(&options, optarg) < 0) {
@@ -4605,6 +4621,8 @@ static int img_amend(const img_cmd_t *ccmd, int argc, 
char **argv)
 case OPTION_FORCE:
 force = true;
 break;
+default:
+tryhelp(argv[0]);
 }
 }
 
-- 
2.39.2

[PULL 08/25] hw/i386/pc_piix: Share pc_cmos_init() invocation between pc and isapc machines

[Philippe Mathieu-Daudé]

From: Bernhard Beschow 

Both invocations are the same and either one is always executed. Avoid this
redundancy.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240208220349.4948-3-shen...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc_piix.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 999b7b806c..9064511507 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -341,11 +341,8 @@ static void pc_init1(MachineState *machine,
 
 pc_nic_init(pcmc, isa_bus, pci_bus);
 
-if (pcmc->pci_enabled) {
-pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state);
-}
 #ifdef CONFIG_IDE_ISA
-else {
+if (!pcmc->pci_enabled) {
 DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
 int i;
 
@@ -363,10 +360,11 @@ static void pc_init1(MachineState *machine,
 busname[4] = '0' + i;
 idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
 }
-pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state);
 }
 #endif
 
+pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state);
+
 if (piix4_pm) {
 smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
 
-- 
2.41.0

[PULL 16/25] hw/i386/pc_q35: Populate interrupt handlers before realizing LPC PCI function

[Philippe Mathieu-Daudé]

From: Bernhard Beschow 

The interrupt handlers need to be populated before the device is realized since
internal devices such as the RTC are wired during realize(). If the interrupt
handlers aren't populated, devices such as the RTC will be wired with a NULL
interrupt handler, i.e. MC146818RtcState::irq is NULL.

Fixes: fc11ca08bc29 "hw/i386/q35: Realize LPC PCI function before accessing it"

Cc: Philippe Mathieu-Daudé 
Signed-off-by: Bernhard Beschow 
Message-ID: <20240217104644.19755-1-shen...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc_q35.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index ab7750c346..53fb3db26d 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -228,10 +228,10 @@ static void pc_q35_init(MachineState *machine)
 lpc_dev = DEVICE(lpc);
 qdev_prop_set_bit(lpc_dev, "smm-enabled",
   x86_machine_is_smm_enabled(x86ms));
-pci_realize_and_unref(lpc, host_bus, &error_fatal);
 for (i = 0; i < IOAPIC_NUM_PINS; i++) {
 qdev_connect_gpio_out_named(lpc_dev, ICH9_GPIO_GSI, i, x86ms->gsi[i]);
 }
+pci_realize_and_unref(lpc, host_bus, &error_fatal);
 
 rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(lpc), "rtc"));
 
-- 
2.41.0

[PULL 12/25] hw/i386/pc: Merge pc_guest_info_init() into pc_machine_initfn()

[Philippe Mathieu-Daudé]

From: Bernhard Beschow 

Resolves redundant code in the piix and q35 machines.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240208220349.4948-5-shen...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/i386/pc.h | 2 --
 hw/i386/pc.c | 9 +++--
 hw/i386/pc_piix.c| 2 --
 hw/i386/pc_q35.c | 2 --
 4 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index cf2fa60868..39cdb9b933 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -154,8 +154,6 @@ extern int fd_bootchk;
 
 void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
 
-void pc_guest_info_init(PCMachineState *pcms);
-
 #define PCI_HOST_PROP_RAM_MEM  "ram-mem"
 #define PCI_HOST_PROP_PCI_MEM  "pci-mem"
 #define PCI_HOST_PROP_SYSTEM_MEM   "system-mem"
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d7183780bd..694de8e130 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -698,12 +698,6 @@ void pc_machine_done(Notifier *notifier, void *data)
 pc_cmos_init_late(pcms);
 }
 
-void pc_guest_info_init(PCMachineState *pcms)
-{
-pcms->machine_done.notify = pc_machine_done;
-qemu_add_machine_init_done_notifier(&pcms->machine_done);
-}
-
 /* setup pci memory address space mapping into system address space */
 void pc_pci_as_mapping_init(MemoryRegion *system_memory,
 MemoryRegion *pci_address_space)
@@ -1744,6 +1738,9 @@ static void pc_machine_initfn(Object *obj)
 object_property_add_alias(OBJECT(pcms), "pcspk-audiodev",
   OBJECT(pcms->pcspk), "audiodev");
 cxl_machine_init(obj, &pcms->cxl_devices_state);
+
+pcms->machine_done.notify = pc_machine_done;
+qemu_add_machine_init_done_notifier(&pcms->machine_done);
 }
 
 static void pc_machine_reset(MachineState *machine, ShutdownCause reason)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index bea096f569..eeca725504 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -225,8 +225,6 @@ static void pc_init1(MachineState *machine,
&error_abort);
 }
 
-pc_guest_info_init(pcms);
-
 if (pcmc->smbios_defaults) {
 MachineClass *mc = MACHINE_GET_CLASS(machine);
 /* These values are guest ABI, do not change */
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 0e9bd27a6e..8053d8cccb 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -188,8 +188,6 @@ static void pc_q35_init(MachineState *machine)
 kvmclock_create(pcmc->kvmclock_create_always);
 }
 
-pc_guest_info_init(pcms);
-
 if (pcmc->smbios_defaults) {
 /* These values are guest ABI, do not change */
 smbios_set_defaults("QEMU", mc->desc,
-- 
2.41.0

[PATCH 04/28] qemu-img: global option processing and error printing

[Michael Tokarev]

In order to correctly print executable name in various
error messages, pass argv[0] to error_exit() function.
This way, error messages will refer to actual executable
name, which may be different from 'qemu-img'.

For subcommands, pass whole argv[] array, so argv[0] is
the executable name, not subcommand name.  In order to
do that, avoid resetting optind but continue with the
next option.  Also don't require at least 3 options on
the command line: it makes no sense with options before
subcommand.

Before invoking a subcommand, replace argv[0] to include
the subcommand name.

Introduce tryhelp() function which just prints

 try 'command-name --help' for more info

and exits.  When tryhelp() is called from within a subcommand
handler, the message will look like:

 try 'command-name subcommand --help' for more info

qemu-img uses getopt_long() with ':' as the first char in
optstring parameter, which means it doesn't print error
messages but return ':' or '?' instead, and qemu-img uses
unrecognized_option() or missing_argument() function to
print error messages.  But it doesn't quite work:

 $ ./qemu-img -xx
 qemu-img: unrecognized option './qemu-img'

so the aim is to let getopt_long() to print regular error
messages instead (removing ':' prefix from optstring) and
remove handling of '?' and ':' "options" entirely.  With
concatenated argv[0] and the subcommand, it all finally
does the right thing in all cases.  This will be done in
subsequent changes command by command, with main() done
last.

unrecognized_option() and missing_argument() functions
prototypes aren't changed by this patch, since they're
called from many places and will be removed a few patches
later.  Only artifical "qemu-img" argv0 is provided in
there for now.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 75 +++---
 1 file changed, 38 insertions(+), 37 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index df425b2517..44dbf5be4f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -101,8 +101,15 @@ static void format_print(void *opaque, const char *name)
 printf(" %s", name);
 }
 
-static G_NORETURN G_GNUC_PRINTF(1, 2)
-void error_exit(const char *fmt, ...)
+static G_NORETURN
+void tryhelp(const char *argv0)
+{
+error_printf("Try '%s --help' for more info\n", argv0);
+exit(EXIT_FAILURE);
+}
+
+static G_NORETURN G_GNUC_PRINTF(2, 3)
+void error_exit(const char *argv0, const char *fmt, ...)
 {
 va_list ap;
 
@@ -110,20 +117,19 @@ void error_exit(const char *fmt, ...)
 error_vreport(fmt, ap);
 va_end(ap);
 
-error_printf("Try 'qemu-img --help' for more information\n");
-exit(EXIT_FAILURE);
+tryhelp(argv0);
 }
 
 static G_NORETURN
 void missing_argument(const char *option)
 {
-error_exit("missing argument for option '%s'", option);
+error_exit("qemu-img", "missing argument for option '%s'", option);
 }
 
 static G_NORETURN
 void unrecognized_option(const char *option)
 {
-error_exit("unrecognized option '%s'", option);
+error_exit("qemu-img", "unrecognized option '%s'", option);
 }
 
 /* Please keep in synch with docs/tools/qemu-img.rst */
@@ -576,7 +582,7 @@ static int img_create(int argc, char **argv)
 }
 
 if (optind >= argc) {
-error_exit("Expecting image file name");
+error_exit(argv[0], "Expecting image file name");
 }
 optind++;
 
@@ -588,7 +594,7 @@ static int img_create(int argc, char **argv)
 }
 }
 if (optind != argc) {
-error_exit("Unexpected argument: %s", argv[optind]);
+error_exit(argv[0], "Unexpected argument: %s", argv[optind]);
 }
 
 bdrv_img_create(filename, fmt, base_filename, base_fmt,
@@ -770,7 +776,7 @@ static int img_check(int argc, char **argv)
 } else if (!strcmp(optarg, "all")) {
 fix = BDRV_FIX_LEAKS | BDRV_FIX_ERRORS;
 } else {
-error_exit("Unknown option value for -r "
+error_exit(argv[0], "Unknown option value for -r "
"(expecting 'leaks' or 'all'): %s", optarg);
 }
 break;
@@ -795,7 +801,7 @@ static int img_check(int argc, char **argv)
 }
 }
 if (optind != argc - 1) {
-error_exit("Expecting one image file name");
+error_exit(argv[0], "Expecting one image file name");
 }
 filename = argv[optind++];
 
@@ -1025,7 +1031,7 @@ static int img_commit(int argc, char **argv)
 }
 
 if (optind != argc - 1) {
-error_exit("Expecting one image file name");
+error_exit(argv[0], "Expecting one image file name");
 }
 filename = argv[optind++];
 
@@ -1446,7 +1452,7 @@ static int img_compare(int argc, char **argv)
 
 
 if (optind != argc - 2) {
-error_exit("Expecting two image file names");
+error_exit(argv[0], "Expecting two image file names");
 }
 filename1 = argv[optind++];
 filename2 = argv[optind++];
@@ -3056,7 +3062,7 @@ static int img_info

[PULL 09/25] hw/i386/pc: Store pointers to IDE buses in PCMachineState

[Philippe Mathieu-Daudé]

From: Peter Maydell 

Add the two IDE bus BusState pointers to the set we keep in PCMachineState.
This allows us to avoid passing them to pc_cmos_init(), and also will
allow a refactoring of how we call pc_cmos_init_late().

Signed-off-by: Peter Maydell 
Acked-by: Richard Henderson 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Philippe Mathieu-Daudé 
[PMD: Do not zero-init pcms->idebus[] again]
Message-ID: <20240220160622.114437-2-peter.mayd...@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/i386/pc.h |  4 +++-
 hw/i386/pc.c |  5 ++---
 hw/i386/pc_piix.c| 12 
 hw/i386/pc_q35.c |  9 +++--
 4 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 02a0deedd3..cf2fa60868 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -17,6 +17,8 @@
 
 #define HPET_INTCAP "hpet-intcap"
 
+#define MAX_IDE_BUS 2
+
 /**
  * PCMachineState:
  * @acpi_dev: link to ACPI PM device that performs ACPI hotplug handling
@@ -37,6 +39,7 @@ typedef struct PCMachineState {
 PFlashCFI01 *flash[2];
 ISADevice *pcspk;
 DeviceState *iommu;
+BusState *idebus[MAX_IDE_BUS];
 
 /* Configuration options: */
 uint64_t max_ram_below_4g;
@@ -182,7 +185,6 @@ void pc_basic_device_init(struct PCMachineState *pcms,
   bool create_fdctrl,
   uint32_t hpet_irqs);
 void pc_cmos_init(PCMachineState *pcms,
-  BusState *ide0, BusState *ide1,
   ISADevice *s);
 void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus);
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 9cbc59665f..3e9ca6295f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -574,7 +574,6 @@ static void pc_cmos_init_late(void *opaque)
 }
 
 void pc_cmos_init(PCMachineState *pcms,
-  BusState *idebus0, BusState *idebus1,
   ISADevice *rtc)
 {
 int val;
@@ -634,8 +633,8 @@ void pc_cmos_init(PCMachineState *pcms,
 
 /* hard drives and FDC */
 arg.rtc_state = s;
-arg.idebus[0] = idebus0;
-arg.idebus[1] = idebus1;
+arg.idebus[0] = pcms->idebus[0];
+arg.idebus[1] = pcms->idebus[1];
 qemu_register_reset(pc_cmos_init_late, &arg);
 }
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9064511507..bea096f569 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -68,7 +68,6 @@
 #include "kvm/kvm-cpu.h"
 #include "target/i386/cpu.h"
 
-#define MAX_IDE_BUS 2
 #define XEN_IOAPIC_NUM_PIRQS 128ULL
 
 #ifdef CONFIG_IDE_ISA
@@ -114,7 +113,6 @@ static void pc_init1(MachineState *machine,
 Object *piix4_pm = NULL;
 qemu_irq smi_irq;
 GSIState *gsi_state;
-BusState *idebus[MAX_IDE_BUS];
 ISADevice *rtc_state;
 MemoryRegion *ram_memory;
 MemoryRegion *pci_memory = NULL;
@@ -299,8 +297,8 @@ static void pc_init1(MachineState *machine,
 piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm");
 dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ide"));
 pci_ide_create_devs(PCI_DEVICE(dev));
-idebus[0] = qdev_get_child_bus(dev, "ide.0");
-idebus[1] = qdev_get_child_bus(dev, "ide.1");
+pcms->idebus[0] = qdev_get_child_bus(dev, "ide.0");
+pcms->idebus[1] = qdev_get_child_bus(dev, "ide.1");
 } else {
 isa_bus = isa_bus_new(NULL, system_memory, system_io,
   &error_abort);
@@ -312,8 +310,6 @@ static void pc_init1(MachineState *machine,
 
 i8257_dma_init(OBJECT(machine), isa_bus, 0);
 pcms->hpet_enabled = false;
-idebus[0] = NULL;
-idebus[1] = NULL;
 }
 
 if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
@@ -358,12 +354,12 @@ static void pc_init1(MachineState *machine,
  * second one.
  */
 busname[4] = '0' + i;
-idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
+pcms->idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
 }
 }
 #endif
 
-pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state);
+pc_cmos_init(pcms, rtc_state);
 
 if (piix4_pm) {
 smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index d346fa3b1d..0e9bd27a6e 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -126,7 +126,6 @@ static void pc_q35_init(MachineState *machine)
 PCIBus *host_bus;
 PCIDevice *lpc;
 DeviceState *lpc_dev;
-BusState *idebus[MAX_SATA_PORTS];
 ISADevice *rtc_state;
 MemoryRegion *system_memory = get_system_memory();
 MemoryRegion *system_io = get_system_io();
@@ -300,13 +299,11 @@ static void pc_q35_init(MachineState *machine)
  ICH9_SATA1_FUNC),
"ich9-ahci");
 ich9 = ICH9_AHCI(pdev);
-idebus[0] = qdev_get_child_bus(DEVICE(pdev), "i

[PATCH 17/28] qemu-img: snapshot: refresh options/--help

[Michael Tokarev]

Add missing long options and --help output.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 45 -
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index ee35768af8..ce939708d4 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3608,26 +3608,51 @@ static int img_snapshot(const img_cmd_t *ccmd, int 
argc, char **argv)
 for(;;) {
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
+{"quiet", no_argument, 0, 'q'},
 {"object", required_argument, 0, OPTION_OBJECT},
+{"format", required_argument, 0, 'f'},
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 {"force-share", no_argument, 0, 'U'},
+{"list", no_argument, 0, SNAPSHOT_LIST},
+{"apply", no_argument, 0, SNAPSHOT_APPLY},
+{"create", no_argument, 0, SNAPSHOT_CREATE},
+{"delete", no_argument, 0, SNAPSHOT_DELETE},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":la:c:d:f:hqU",
+c = getopt_long(argc, argv, "la:c:d:f:hqU",
 long_options, NULL);
 if (c == -1) {
 break;
 }
 switch(c) {
-case ':':
-missing_argument(argv[optind - 1]);
-break;
-case '?':
-unrecognized_option(argv[optind - 1]);
-break;
 case 'h':
-help();
-return 0;
+cmd_help(ccmd,
+"[-f FMT | --image-opts] [-l | -a|-c|-d SNAPSHOT]\n"
+"[-U] [--object OBJDEF] FILENAME\n"
+,
+"  -q, --quiet\n"
+"  quiet operations\n"
+"  -f, --format FMT\n"
+"  specify FILENAME format explicitly\n"
+"  --image-opts\n"
+"  indicates that FILENAME is a complete image specification\n"
+"   instead of a file name (incompatible with --format)\n"
+"  -U, --force-share\n"
+"  open image in shared mode for concurrent access\n"
+"  --object OBJDEF\n"
+"  QEMU user-creatable object (eg encryption key)\n"
+"  Operation, one of:\n"
+"-l, --list\n"
+"   list snapshots in FILENAME (the default)\n"
+"-c, --create SNAPSHOT\n"
+"   create named snapshot\n"
+"-a, --apply SNAPSHOT\n"
+"   apply named snapshot to the base\n"
+"-d, --delete SNAPSHOT\n"
+"   delete named snapshot\n"
+"  FILENAME - image file name (or specification with --image-opts)\n"
+);
+break;
 case 'f':
 fmt = optarg;
 break;
@@ -3654,6 +3679,8 @@ static int img_snapshot(const img_cmd_t *ccmd, int argc, 
char **argv)
 case OPTION_IMAGE_OPTS:
 image_opts = true;
 break;
+default:
+tryhelp(argv[0]);
 }
 }
 
-- 
2.39.2

[PATCH 22/28] qemu-img: bench: refresh options/--help

[Michael Tokarev]

Add missing long options and --help output.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 64 +-
 1 file changed, 54 insertions(+), 10 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 0d17738fb6..8455832d34 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4847,28 +4847,70 @@ static int img_bench(const img_cmd_t *ccmd, int argc, 
char **argv)
 for (;;) {
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
-{"flush-interval", required_argument, 0, OPTION_FLUSH_INTERVAL},
+{"format", required_argument, 0, 'f'},
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+{"cache", required_argument, 0, 't'},
+{"count", required_argument, 0, 'c'},
+{"depth", required_argument, 0, 'd'},
+{"offset", required_argument, 0, 'o'},
+{"buffer-size", required_argument, 0, 's'},
+{"step-size", required_argument, 0, 'S'},
+{"aio", required_argument, 0, 'i'},
+{"native", no_argument, 0, 'n'},
+{"write", no_argument, 0, 'w'},
 {"pattern", required_argument, 0, OPTION_PATTERN},
+{"flush-interval", required_argument, 0, OPTION_FLUSH_INTERVAL},
 {"no-drain", no_argument, 0, OPTION_NO_DRAIN},
 {"force-share", no_argument, 0, 'U'},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":hc:d:f:ni:o:qs:S:t:wU", long_options,
-NULL);
+c = getopt_long(argc, argv, "hc:d:f:ni:o:qs:S:t:wU",
+long_options, NULL);
 if (c == -1) {
 break;
 }
 
 switch (c) {
-case ':':
-missing_argument(argv[optind - 1]);
-break;
-case '?':
-unrecognized_option(argv[optind - 1]);
-break;
 case 'h':
-help();
+cmd_help(ccmd,
+"[-f FMT | --image-opts] [-t CACHE] [-c COUNT] [-d DEPTH]\n"
+"[-o OFFSET] [-s BUFFER_SIZE] [-S STEP_SIZE] [-i AIO] [-n]\n"
+"[-w [--pattern PATTERN] [--flush-interval INTERVAL [--no-drain]]]\n"
+,
+"  -q, --quiet\n"
+" quiet operations\n"
+"  -f, --format FMT\n"
+" specify FILENAME format explicitly\n"
+"  --image-opts\n"
+" indicates that FILENAME is a complete image specification\n"
+" instead of a file name (incompatible with --format)\n"
+"  -t, --cache CACHE\n"
+" cache mode for FILENAME (" BDRV_DEFAULT_CACHE ")\n"
+"  -c, --count COUNT\n"
+" number of I/O requests to perform\n"
+"  -s, --buffer-size BUFFER_SIZE\n"
+" size of each I/O request\n"
+"  -d, --depth DEPTH\n"
+" number of requests to perform in parallel\n"
+"  -o, --offset OFFSET\n"
+" start first request at this OFFSET\n"
+"  -S, --step-size STEP_SIZE\n"
+" each next request offset increment\n"
+"  -i, --aio AIO\n"
+" async-io backend (threads, native, io_uring)\n"
+"  -n, --native\n"
+" use native AIO backend if possible\n"
+"  -w, --write\n"
+" perform write test (default is read)\n"
+"  --pattern PATTERN\n"
+" write this pattern byte instead of zero\n"
+"  --flush-interval FLUSH_INTERVAL\n"
+" issue flush after this number of requests\n"
+"  --no-drain\n"
+" do not wait when flushing pending requests\n"
+"  -U, --force-share\n"
+" open images in shared mode for concurrent access\n"
+);
 break;
 case 'c':
 {
@@ -4985,6 +5027,8 @@ static int img_bench(const img_cmd_t *ccmd, int argc, 
char **argv)
 case OPTION_IMAGE_OPTS:
 image_opts = true;
 break;
+default:
+tryhelp(argv[0]);
 }
 }
 
-- 
2.39.2

[PULL 02/25] hw/sysbus: Inline and remove sysbus_add_io()

[Philippe Mathieu-Daudé]

sysbus_add_io(...) is a simple wrapper to
memory_region_add_subregion(get_system_io(), ...).
It is used in 3 places; inline it directly.

Rationale: we want to move to an explicit I/O bus,
rather that an implicit one. Besides in heterogeneous
setup we can have more than one I/O bus.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Peter Maydell 
Message-Id: <20240216150441.45681-1-phi...@linaro.org>
---
 include/hw/sysbus.h | 2 --
 hw/core/sysbus.c| 6 --
 hw/i386/kvmvapic.c  | 2 +-
 hw/mips/mipssim.c   | 2 +-
 hw/nvram/fw_cfg.c   | 5 +++--
 5 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index 3564b7b6a2..14dbc22d0c 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -83,8 +83,6 @@ void sysbus_mmio_map(SysBusDevice *dev, int n, hwaddr addr);
 void sysbus_mmio_map_overlap(SysBusDevice *dev, int n, hwaddr addr,
  int priority);
 void sysbus_mmio_unmap(SysBusDevice *dev, int n);
-void sysbus_add_io(SysBusDevice *dev, hwaddr addr,
-   MemoryRegion *mem);
 MemoryRegion *sysbus_address_space(SysBusDevice *dev);
 
 bool sysbus_realize(SysBusDevice *dev, Error **errp);
diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 35f902b582..9f1d5b2d6d 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -298,12 +298,6 @@ static char *sysbus_get_fw_dev_path(DeviceState *dev)
 return g_strdup(qdev_fw_name(dev));
 }
 
-void sysbus_add_io(SysBusDevice *dev, hwaddr addr,
-   MemoryRegion *mem)
-{
-memory_region_add_subregion(get_system_io(), addr, mem);
-}
-
 MemoryRegion *sysbus_address_space(SysBusDevice *dev)
 {
 return get_system_memory();
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index f2b0aff479..3be64fba3b 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -727,7 +727,7 @@ static void vapic_realize(DeviceState *dev, Error **errp)
 VAPICROMState *s = VAPIC(dev);
 
 memory_region_init_io(&s->io, OBJECT(s), &vapic_ops, s, "kvmvapic", 2);
-sysbus_add_io(sbd, VAPIC_IO_PORT, &s->io);
+memory_region_add_subregion(get_system_io(), VAPIC_IO_PORT, &s->io);
 sysbus_init_ioports(sbd, VAPIC_IO_PORT, 2);
 
 option_rom[nb_option_roms].name = "kvmvapic.bin";
diff --git a/hw/mips/mipssim.c b/hw/mips/mipssim.c
index a12427b6c8..57c8c33e2b 100644
--- a/hw/mips/mipssim.c
+++ b/hw/mips/mipssim.c
@@ -226,7 +226,7 @@ mips_mipssim_init(MachineState *machine)
 qdev_prop_set_uint8(dev, "endianness", DEVICE_LITTLE_ENDIAN);
 sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
 sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, env->irq[4]);
-sysbus_add_io(SYS_BUS_DEVICE(dev), 0x3f8,
+memory_region_add_subregion(get_system_io(), 0x3f8,
   sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
 }
 
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index e85493d513..6d6b17462d 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -1142,6 +1142,7 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t 
dma_iobase,
 SysBusDevice *sbd;
 FWCfgIoState *ios;
 FWCfgState *s;
+MemoryRegion *iomem = get_system_io();
 bool dma_requested = dma_iobase && dma_as;
 
 dev = qdev_new(TYPE_FW_CFG_IO);
@@ -1155,7 +1156,7 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t 
dma_iobase,
 sbd = SYS_BUS_DEVICE(dev);
 sysbus_realize_and_unref(sbd, &error_fatal);
 ios = FW_CFG_IO(dev);
-sysbus_add_io(sbd, iobase, &ios->comb_iomem);
+memory_region_add_subregion(iomem, iobase, &ios->comb_iomem);
 
 s = FW_CFG(dev);
 
@@ -1163,7 +1164,7 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t 
dma_iobase,
 /* 64 bits for the address field */
 s->dma_as = dma_as;
 s->dma_addr = 0;
-sysbus_add_io(sbd, dma_iobase, &s->dma_iomem);
+memory_region_add_subregion(iomem, dma_iobase, &s->dma_iomem);
 }
 
 return s;
-- 
2.41.0

[PATCH 10/28] qemu-img: commit: refresh options/--help

[Michael Tokarev]

Add missing long options and --help output.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 44 
 1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index eba13724b0..1271217272 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1047,24 +1047,50 @@ static int img_commit(const img_cmd_t *ccmd, int argc, 
char **argv)
 for(;;) {
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
+{"quiet", no_argument, 0, 'q'},
 {"object", required_argument, 0, OPTION_OBJECT},
+{"format", required_argument, 0, 'f'},
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+{"cache", required_argument, 0, 't'},
+{"drop", no_argument, 0, 'd'},
+{"base", required_argument, 0, 'b'},
+{"progress", no_argument, 0, 'p'},
+{"rate", required_argument, 0, 'r'},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":f:ht:b:dpqr:",
+c = getopt_long(argc, argv, "f:ht:b:dpqr:",
 long_options, NULL);
 if (c == -1) {
 break;
 }
 switch(c) {
-case ':':
-missing_argument(argv[optind - 1]);
-break;
-case '?':
-unrecognized_option(argv[optind - 1]);
-break;
 case 'h':
-help();
+cmd_help(ccmd,
+"[-f FMT | --image-opts] [-t CACHE_MODE] [-b BASE_IMG] [-d]\n"
+"[-r RATE] [--object OBJDEF] FILENAME\n"
+,
+"  -q, --quiet\n"
+" quiet operations\n"
+"  -p, --progress\n"
+" show operation progress\n"
+"  -f, --format FMT\n"
+" specify FILENAME image format explicitly\n"
+"  --image-opts\n"
+" indicates that FILENAME is a complete image specification\n"
+" instead of a file name (incompatible with --format)\n"
+"  -t, --cache CACHE_MODE image cache mode (" BDRV_DEFAULT_CACHE ")\n"
+"  -d, --drop\n"
+" skip emptying FILENAME on completion\n"
+"  -b, --base BASE_IMG\n"
+" image in the backing chain to which to commit changes\n"
+" instead of the previous one (implies --drop)\n"
+"  -r, --rate RATE\n"
+" I/O rate limit\n"
+"  --object OBJDEF\n"
+" QEMU user-creatable object (eg encryption key)\n"
+"  FILENAME\n"
+" name of the image file to operate on\n"
+);
 break;
 case 'f':
 fmt = optarg;
@@ -1098,6 +1124,8 @@ static int img_commit(const img_cmd_t *ccmd, int argc, 
char **argv)
 case OPTION_IMAGE_OPTS:
 image_opts = true;
 break;
+default:
+tryhelp(argv[0]);
 }
 }
 
-- 
2.39.2

[PULL 05/25] hw/ppc/ppc4xx_pci: Move ppc4xx_pci.c to hw/pci-host/

[Philippe Mathieu-Daudé]

ppc4xx_pci.c is moved from the target specific ppc_ss[] meson
source set to pci_ss[] which is common to all targets: the
object is built once.

Declare PPC4XX_PCI selector in pci-host/Kconfig.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Thomas Huth 
Message-Id: <20240215105017.57748-4-phi...@linaro.org>
---
 MAINTAINERS   | 1 +
 hw/{ppc => pci-host}/ppc4xx_pci.c | 0
 hw/pci-host/ppce500.c | 2 +-
 hw/pci-host/Kconfig   | 4 
 hw/pci-host/meson.build   | 1 +
 hw/pci-host/trace-events  | 4 
 hw/ppc/Kconfig| 2 +-
 hw/ppc/meson.build| 1 -
 hw/ppc/trace-events   | 4 
 9 files changed, 12 insertions(+), 7 deletions(-)
 rename hw/{ppc => pci-host}/ppc4xx_pci.c (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9b6ce9a934..8d9ccd5073 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1423,6 +1423,7 @@ Bamboo
 L: qemu-...@nongnu.org
 S: Orphan
 F: hw/ppc/ppc440_bamboo.c
+F: hw/pci-host/ppc4xx_pci.c
 F: tests/avocado/ppc_bamboo.py
 
 e500
diff --git a/hw/ppc/ppc4xx_pci.c b/hw/pci-host/ppc4xx_pci.c
similarity index 100%
rename from hw/ppc/ppc4xx_pci.c
rename to hw/pci-host/ppc4xx_pci.c
diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
index fa0d67b342..95b983b2b3 100644
--- a/hw/pci-host/ppce500.c
+++ b/hw/pci-host/ppce500.c
@@ -5,7 +5,7 @@
  *
  * Author: Yu Liu, 
  *
- * This file is derived from hw/ppc4xx_pci.c,
+ * This file is derived from ppc4xx_pci.c,
  * the copyright for that material belongs to the original owners.
  *
  * This is free software; you can redistribute it and/or modify
diff --git a/hw/pci-host/Kconfig b/hw/pci-host/Kconfig
index f046d76a68..0a221e719e 100644
--- a/hw/pci-host/Kconfig
+++ b/hw/pci-host/Kconfig
@@ -6,6 +6,10 @@ config XEN_IGD_PASSTHROUGH
 default y
 depends on XEN && PCI_I440FX
 
+config PPC4XX_PCI
+bool
+select PCI
+
 config RAVEN_PCI
 bool
 select PCI
diff --git a/hw/pci-host/meson.build b/hw/pci-host/meson.build
index 36d5ab756f..eb6dc71c88 100644
--- a/hw/pci-host/meson.build
+++ b/hw/pci-host/meson.build
@@ -14,6 +14,7 @@ pci_ss.add(when: 'CONFIG_REMOTE_PCIHOST', if_true: 
files('remote.c'))
 pci_ss.add(when: 'CONFIG_SH_PCI', if_true: files('sh_pci.c'))
 
 # PPC devices
+pci_ss.add(when: 'CONFIG_PPC4XX_PCI', if_true: files('ppc4xx_pci.c'))
 pci_ss.add(when: 'CONFIG_RAVEN_PCI', if_true: files('raven.c'))
 pci_ss.add(when: 'CONFIG_GRACKLE_PCI', if_true: files('grackle.c'))
 # NewWorld PowerMac
diff --git a/hw/pci-host/trace-events b/hw/pci-host/trace-events
index b2f47e6335..90a37ebff0 100644
--- a/hw/pci-host/trace-events
+++ b/hw/pci-host/trace-events
@@ -37,6 +37,10 @@ unin_data_read(uint64_t addr, unsigned len, uint64_t val) 
"read addr 0x%"PRIx64
 unin_write(uint64_t addr, uint64_t value) "addr=0x%" PRIx64 " val=0x%"PRIx64
 unin_read(uint64_t addr, uint64_t value) "addr=0x%" PRIx64 " val=0x%"PRIx64
 
+# ppc4xx_pci.c
+ppc4xx_pci_map_irq(int32_t devfn, int irq_num, int slot) "devfn 0x%x irq %d -> 
%d"
+ppc4xx_pci_set_irq(int irq_num) "PCI irq %d"
+
 # pnv_phb4.c
 pnv_phb4_xive_notify(uint64_t notif_port, uint64_t data) "notif=@0x%"PRIx64" 
data=0x%"PRIx64
 pnv_phb4_xive_notify_ic(uint64_t addr, uint64_t data) "addr=@0x%"PRIx64" 
data=0x%"PRIx64
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 44263a58c4..82e847d22c 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -53,7 +53,7 @@ config PPC440
 config PPC4XX
 bool
 select BITBANG_I2C
-select PCI
+select PPC4XX_PCI
 select PPC_UIC
 
 config SAM460EX
diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index eba3406e7f..d0efc0aba5 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -63,7 +63,6 @@ ppc_ss.add(when: 'CONFIG_PPC440', if_true: files(
   'ppc440_pcix.c', 'ppc440_uc.c'))
 ppc_ss.add(when: 'CONFIG_PPC4XX', if_true: files(
   'ppc4xx_devs.c',
-  'ppc4xx_pci.c',
   'ppc4xx_sdram.c'))
 ppc_ss.add(when: 'CONFIG_SAM460EX', if_true: files('sam460ex.c'))
 # PReP
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index f670e8906c..b59fbf340f 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -146,10 +146,6 @@ rs6000mc_size_read(uint32_t addr, uint32_t val) "read 
addr=0x%x val=0x%x"
 rs6000mc_size_write(uint32_t addr, uint32_t val) "write addr=0x%x val=0x%x"
 rs6000mc_parity_read(uint32_t addr, uint32_t val) "read addr=0x%x val=0x%x"
 
-# ppc4xx_pci.c
-ppc4xx_pci_map_irq(int32_t devfn, int irq_num, int slot) "devfn 0x%x irq %d -> 
%d"
-ppc4xx_pci_set_irq(int irq_num) "PCI irq %d"
-
 # ppc440_pcix.c
 ppc440_pcix_map_irq(int32_t devfn, int irq_num, int slot) "devfn 0x%x irq %d 
-> %d"
 ppc440_pcix_set_irq(int irq_num) "PCI irq %d"
-- 
2.41.0

[PULL 00/25] Misc HW patches for 2024-02-21

[Philippe Mathieu-Daudé]

The following changes since commit 760b4dcdddba4a40b9fa0eb78fdfc7eda7cb83d0:

  Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging 
(2024-02-20 10:11:08 +)

are available in the Git repository at:

  https://github.com/philmd/qemu.git tags/hw-misc-20240221

for you to fetch changes up to df07f6bb563f25f39f4f5887adab557e42bdee59:

  hw/sparc/leon3: Fix wrong usage of DO_UPCAST macro (2024-02-21 22:13:03 +0100)

Following checkpatch.pl error ignored:

  ERROR: Macros with complex values should be enclosed in parenthesis
  #62: FILE: include/hw/ide/ide-dev.h:31:
  +#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)


Misc HW patch queue

- Remove sysbus_add_io (Phil)
- Build PPC 4xx PCI host bridges once (Phil)
- Display QOM path while debugging SMBus targets (Joe)
- Simplify x86 PC code (Bernhard)
- Remove qemu_[un]register_reset() calls in x86 PC CMOS (Peter)
- Fix wiring of ICH9 LPC interrupts (Bernhard)
- Split core IDE as device / bus / dma (Thomas)
- Fix invalid use of DO_UPCAST() in Leon3 (Thomas)



Bernhard Beschow (8):
  hw/i386/pc_piix: Share pc_cmos_init() invocation between pc and isapc
machines
  hw/i386/x86: Turn apic_xrupt_override into class attribute
  hw/i386/pc: Merge pc_guest_info_init() into pc_machine_initfn()
  hw/i386/pc: Defer smbios_set_defaults() to machine_done
  hw/i386/pc: Confine system flash handling to pc_sysfw
  hw/i386/pc_sysfw: Inline pc_system_flash_create() and remove it
  hw/i386/pc_q35: Populate interrupt handlers before realizing LPC PCI
function
  hw/isa/meson.build: Sort alphabetically

Joe Komlodi (1):
  hw/i2c/smbus_slave: Add object path on error prints

Peter Maydell (2):
  hw/i386/pc: Store pointers to IDE buses in PCMachineState
  hw/i386/pc: Do pc_cmos_init_late() from pc_machine_done()

Philippe Mathieu-Daudé (6):
  hw/input/pckbd: Open-code i8042_setup_a20_line() wrapper
  hw/sysbus: Inline and remove sysbus_add_io()
  hw/ppc/ppc4xx_pci: Remove unused "hw/ppc/ppc.h" header
  hw/ppc/ppc4xx_pci: Extract PCI host definitions to
hw/pci-host/ppc4xx.h
  hw/ppc/ppc4xx_pci: Move ppc4xx_pci.c to hw/pci-host/
  hw/ppc/ppc440_pcix: Move ppc440_pcix.c to hw/pci-host/

Thomas Huth (8):
  hw/ide: Add the possibility to disable the CompactFlash device in the
build
  hw/ide: Split qdev.c into ide-bus.c and ide-dev.c
  hw/ide: Move IDE DMA related definitions to a separate header
ide-dma.h
  hw/ide: Move IDE device related definitions to ide-dev.h
  hw/ide: Move IDE bus related definitions to a new header ide-bus.h
  hw/ide: Remove the include/hw/ide.h legacy file
  hw/ide: Stop exposing internal.h to non-IDE files
  hw/sparc/leon3: Fix wrong usage of DO_UPCAST macro

 MAINTAINERS|   5 +-
 hw/i386/fw_cfg.h   |   3 +-
 include/hw/i386/pc.h   |   9 +-
 include/hw/i386/x86.h  |   3 +-
 include/hw/ide.h   |   9 --
 include/hw/ide/ide-bus.h   |  42 ++
 include/hw/ide/ide-dev.h   | 184 +
 include/hw/ide/ide-dma.h   |  37 +
 include/hw/ide/internal.h  | 211 +
 include/hw/ide/pci.h   |   2 +-
 include/hw/input/i8042.h   |   1 -
 include/hw/pci-host/ppc4xx.h   |  17 +++
 include/hw/ppc/ppc4xx.h|   5 -
 include/hw/sysbus.h|   2 -
 hw/core/sysbus.c   |   6 -
 hw/i2c/smbus_slave.c   |   8 +-
 hw/i386/acpi-common.c  |   3 +-
 hw/i386/fw_cfg.c   |  12 +-
 hw/i386/kvmvapic.c |   2 +-
 hw/i386/pc.c   |  60 
 hw/i386/pc_piix.c  |  31 +
 hw/i386/pc_q35.c   |  22 +--
 hw/i386/pc_sysfw.c |  17 +--
 hw/ide/cf.c|  58 
 hw/ide/cmd646.c|   1 +
 hw/ide/ide-bus.c   | 111 +++
 hw/ide/{qdev.c => ide-dev.c}   | 137 +--
 hw/ide/pci.c   |   1 +
 hw/ide/piix.c  |   1 +
 hw/ide/sii3112.c   |   1 +
 hw/ide/via.c   |   1 +
 hw/input/pckbd.c   |   5 -
 hw/mips/mipssim.c  |   2 +-
 hw/nvram/fw_cfg.c  |   5 +-
 hw/{ppc => pci-host}/ppc440_pcix.c |   3 +-
 hw/{ppc => pci-host}/ppc4xx_pci.c  |   3 +-
 hw/pci-host/ppce500.c 

[PATCH 05/28] qemu-img: pass current cmd info into command handlers

[Michael Tokarev]

This info will be used to generate --help output.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 44dbf5be4f..38ac0f1845 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -60,7 +60,7 @@
 
 typedef struct img_cmd_t {
 const char *name;
-int (*handler)(int argc, char **argv);
+int (*handler)(const struct img_cmd_t *ccmd, int argc, char **argv);
 } img_cmd_t;
 
 enum {
@@ -514,7 +514,7 @@ static int64_t cvtnum(const char *name, const char *value)
 return cvtnum_full(name, value, 0, INT64_MAX);
 }
 
-static int img_create(int argc, char **argv)
+static int img_create(const img_cmd_t *ccmd, int argc, char **argv)
 {
 int c;
 int64_t img_size = -1;
@@ -719,7 +719,7 @@ static int collect_image_check(BlockDriverState *bs,
  *  3 - Check completed, image has leaked clusters, but is good otherwise
  * 63 - Checks are not supported by the image format
  */
-static int img_check(int argc, char **argv)
+static int img_check(const img_cmd_t *ccmd, int argc, char **argv)
 {
 int c, ret;
 OutputFormat output_format = OFORMAT_HUMAN;
@@ -951,7 +951,7 @@ static void run_block_job(BlockJob *job, Error **errp)
 }
 }
 
-static int img_commit(int argc, char **argv)
+static int img_commit(const img_cmd_t *ccmd, int argc, char **argv)
 {
 int c, ret, flags;
 const char *filename, *fmt, *cache, *base;
@@ -1358,7 +1358,7 @@ static int check_empty_sectors(BlockBackend *blk, int64_t 
offset,
  * 1 - Images differ
  * >1 - Error occurred
  */
-static int img_compare(int argc, char **argv)
+static int img_compare(const img_cmd_t *ccmd, int argc, char **argv)
 {
 const char *fmt1 = NULL, *fmt2 = NULL, *cache, *filename1, *filename2;
 BlockBackend *blk1, *blk2;
@@ -2234,7 +2234,7 @@ static void set_rate_limit(BlockBackend *blk, int64_t 
rate_limit)
 blk_set_io_limits(blk, &cfg);
 }
 
-static int img_convert(int argc, char **argv)
+static int img_convert(const img_cmd_t *ccmd, int argc, char **argv)
 {
 int c, bs_i, flags, src_flags = BDRV_O_NO_SHARE;
 const char *fmt = NULL, *out_fmt = NULL, *cache = "unsafe",
@@ -3002,7 +3002,7 @@ err:
 return NULL;
 }
 
-static int img_info(int argc, char **argv)
+static int img_info(const img_cmd_t *ccmd, int argc, char **argv)
 {
 int c;
 OutputFormat output_format = OFORMAT_HUMAN;
@@ -3227,7 +3227,7 @@ static inline bool entry_mergeable(const MapEntry *curr, 
const MapEntry *next)
 return true;
 }
 
-static int img_map(int argc, char **argv)
+static int img_map(const img_cmd_t *ccmd, int argc, char **argv)
 {
 int c;
 OutputFormat output_format = OFORMAT_HUMAN;
@@ -3376,7 +3376,7 @@ out:
 #define SNAPSHOT_APPLY  3
 #define SNAPSHOT_DELETE 4
 
-static int img_snapshot(int argc, char **argv)
+static int img_snapshot(const img_cmd_t *ccmd, int argc, char **argv)
 {
 BlockBackend *blk;
 BlockDriverState *bs;
@@ -3534,7 +3534,7 @@ static int img_snapshot(int argc, char **argv)
 return 0;
 }
 
-static int img_rebase(int argc, char **argv)
+static int img_rebase(const img_cmd_t *ccmd, int argc, char **argv)
 {
 BlockBackend *blk = NULL, *blk_old_backing = NULL, *blk_new_backing = NULL;
 uint8_t *buf_old = NULL;
@@ -4028,7 +4028,7 @@ out:
 return 0;
 }
 
-static int img_resize(int argc, char **argv)
+static int img_resize(const img_cmd_t *ccmd, int argc, char **argv)
 {
 Error *err = NULL;
 int c, ret, relative;
@@ -4241,7 +4241,7 @@ static int print_amend_option_help(const char *format)
 return 0;
 }
 
-static int img_amend(int argc, char **argv)
+static int img_amend(const img_cmd_t *ccmd, int argc, char **argv)
 {
 Error *err = NULL;
 int c, ret = 0;
@@ -4505,7 +4505,7 @@ static void bench_cb(void *opaque, int ret)
 }
 }
 
-static int img_bench(int argc, char **argv)
+static int img_bench(const img_cmd_t *ccmd, int argc, char **argv)
 {
 int c, ret = 0;
 const char *fmt = NULL, *filename;
@@ -4775,7 +4775,7 @@ typedef struct ImgBitmapAction {
 QSIMPLEQ_ENTRY(ImgBitmapAction) next;
 } ImgBitmapAction;
 
-static int img_bitmap(int argc, char **argv)
+static int img_bitmap(const img_cmd_t *ccmd, int argc, char **argv)
 {
 Error *err = NULL;
 int c, ret = 1;
@@ -5075,7 +5075,7 @@ static int img_dd_skip(const char *arg,
 return 0;
 }
 
-static int img_dd(int argc, char **argv)
+static int img_dd(const img_cmd_t *ccmd, int argc, char **argv)
 {
 int ret = 0;
 char *arg = NULL;
@@ -5341,7 +5341,7 @@ static void dump_json_block_measure_info(BlockMeasureInfo 
*info)
 g_string_free(str, true);
 }
 
-static int img_measure(int argc, char **argv)
+static int img_measure(const img_cmd_t *ccmd, int argc, char **argv)
 {
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
@@ -5603,7 +5603,7 @@ int main(int argc, char **argv)
 for (cmd = img_cmds; cmd->name != NULL; cmd++) {
   

[PATCH 06/28] qemu-img: create: refresh options/--help

[Michael Tokarev]

Create helper function cmd_help() to display command-specific
help text, and use it to print --help for 'create' subcommand.

Add missing long options (eg --format) in img_create().

Remove usage of missing_argument()/unrecognized_option() in
img_create().

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 68 +++---
 1 file changed, 60 insertions(+), 8 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 38ac0f1845..7e4c993b9c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -132,6 +132,31 @@ void unrecognized_option(const char *option)
 error_exit("qemu-img", "unrecognized option '%s'", option);
 }
 
+/*
+ * Print --help output for a command and exit.
+ * syntax and description are multi-line with trailing EOL
+ * (to allow easy extending of the text)
+ * syntax has each subsequent line indented by 8 chars.
+ * desrciption is indented by 2 chars for argument on each own line,
+ * and with 5 chars for argument description (like -h arg below).
+ */
+static G_NORETURN
+void cmd_help(const img_cmd_t *ccmd,
+  const char *syntax, const char *arguments)
+{
+printf(
+"Usage:\n"
+"  %s %s %s"
+"\n"
+"Arguments:\n"
+"  -h, --help\n"
+" print this help and exit\n"
+"%s\n",
+   "qemu-img", ccmd->name,
+   syntax, arguments);
+exit(EXIT_SUCCESS);
+}
+
 /* Please keep in synch with docs/tools/qemu-img.rst */
 static G_NORETURN
 void help(void)
@@ -530,23 +555,48 @@ static int img_create(const img_cmd_t *ccmd, int argc, 
char **argv)
 for(;;) {
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
+{"quiet", no_argument, 0, 'q'},
 {"object", required_argument, 0, OPTION_OBJECT},
+{"format", required_argument, 0, 'f'},
+{"backing", required_argument, 0, 'b'},
+{"backing-format", required_argument, 0, 'F'},
+{"backing-unsafe", no_argument, 0, 'u'},
+{"options", required_argument, 0, 'o'},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":F:b:f:ho:qu",
+c = getopt_long(argc, argv, "F:b:f:ho:qu",
 long_options, NULL);
 if (c == -1) {
 break;
 }
 switch(c) {
-case ':':
-missing_argument(argv[optind - 1]);
-break;
-case '?':
-unrecognized_option(argv[optind - 1]);
-break;
 case 'h':
-help();
+cmd_help(ccmd,
+"[-f FMT] [-o FMT_OPTS] [-b BACKING_FILENAME [-F BACKING_FMT]]\n"
+"[--object OBJDEF] [-u] FILENAME [SIZE[bkKMGTPE]]\n"
+,
+"  -q, --quiet\n"
+" quiet operations\n"
+"  -f, --format FMT\n"
+" specifies format of the new image, default is raw\n"
+"  -o, --options FMT_OPTS\n"
+" format-specific options ('-o list' for list)\n"
+"  -b, --backing BACKING_FILENAME\n"
+" stack new image on top of BACKING_FILENAME\n"
+" (for formats which support stacking)\n"
+"  -F, --backing-format BACKING_FMT\n"
+" specify format of BACKING_FILENAME\n"
+"  -u, --backing-unsafe\n"
+" do not fail if BACKING_FMT can not be read\n"
+"  --object OBJDEF\n"
+" QEMU user-creatable object (eg encryption key)\n"
+"  FILENAME\n"
+" image file to create.  It will be overridden if exists\n"
+"  SIZE\n"
+" image size with optional suffix (multiplies in 1024)\n"
+" SIZE is required unless BACKING_IMG is specified,\n"
+" in which case it will be the same as size of BACKING_IMG\n"
+);
 break;
 case 'F':
 base_fmt = optarg;
@@ -571,6 +621,8 @@ static int img_create(const img_cmd_t *ccmd, int argc, char 
**argv)
 case OPTION_OBJECT:
 user_creatable_process_cmdline(optarg);
 break;
+default:
+tryhelp(argv[0]);
 }
 }
 
-- 
2.39.2

[PATCH 09/28] qemu-img: simplify --repair error message

[Michael Tokarev]

Signed-off-by: Michael Tokarev 
Reviewed-by: Daniel P. Berrangé 
---
 qemu-img.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 69fa9701e9..eba13724b0 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -858,8 +858,9 @@ static int img_check(const img_cmd_t *ccmd, int argc, char 
**argv)
 } else if (!strcmp(optarg, "all")) {
 fix = BDRV_FIX_LEAKS | BDRV_FIX_ERRORS;
 } else {
-error_exit(argv[0], "Unknown option value for -r "
-   "(expecting 'leaks' or 'all'): %s", optarg);
+error_exit(argv[0],
+   "--repair (-r) expects 'leaks' or 'all' not '%s'",
+   optarg);
 }
 break;
 case OPTION_OUTPUT:
-- 
2.39.2

[PATCH 01/28] qemu-img: stop printing error twice in a few places

[Michael Tokarev]

Currently we have:

  ./qemu-img resize none +10
  qemu-img: Could not open 'none': Could not open 'none': No such file or 
directory

stop printing the message twice, - local_err already has
all the info, no need to prepend additional text there.

There are a few other places like this, but I'm unsure
about these.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 7668f86769..5a756be600 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -404,7 +404,7 @@ static BlockBackend *img_open_file(const char *filename,
 }
 blk = blk_new_open(filename, NULL, options, flags, &local_err);
 if (!blk) {
-error_reportf_err(local_err, "Could not open '%s': ", filename);
+error_report_err(local_err);
 return NULL;
 }
 blk_set_enable_write_cache(blk, !writethrough);
@@ -597,7 +597,7 @@ static int img_create(int argc, char **argv)
 bdrv_img_create(filename, fmt, base_filename, base_fmt,
 options, img_size, flags, quiet, &local_err);
 if (local_err) {
-error_reportf_err(local_err, "%s: ", filename);
+error_report_err(local_err);
 goto fail;
 }
 
@@ -5253,9 +5253,7 @@ static int img_dd(int argc, char **argv)
 
 ret = bdrv_create(drv, out.filename, opts, &local_err);
 if (ret < 0) {
-error_reportf_err(local_err,
-  "%s: error while creating output image: ",
-  out.filename);
+error_report_err(local_err);
 ret = -1;
 goto out;
 }
-- 
2.39.2

Re: [PATCH v2 4/7] hw/ide: Move IDE device related definitions to ide-dev.h

[Philippe Mathieu-Daudé]

On 21/2/24 20:13, Thomas Huth wrote:

On 21/02/2024 19.43, Philippe Mathieu-Daudé wrote:

On 20/2/24 09:55, Thomas Huth wrote:

Unentangle internal.h by moving public IDE device related


Disentangle ... from? Untangle? TIL Unentangle.


You're right, "disentangle" seems to be the more appropriate word.
I'll fix it up.


No need, series queued and fixed!


Anyway I'm not a native English speaker, so:
Reviewed-by: Philippe Mathieu-Daudé 


Thanks!

  Thomas

Re: [PATCH v2 4/7] hw/ide: Move IDE device related definitions to ide-dev.h

[Thomas Huth]

On 21/02/2024 19.43, Philippe Mathieu-Daudé wrote:

On 20/2/24 09:55, Thomas Huth wrote:

Unentangle internal.h by moving public IDE device related


Disentangle ... from? Untangle? TIL Unentangle.


You're right, "disentangle" seems to be the more appropriate word.
I'll fix it up.


Anyway I'm not a native English speaker, so:
Reviewed-by: Philippe Mathieu-Daudé 


Thanks!

 Thomas

Re: [PATCH v2 5/7] hw/ide: Move IDE bus related definitions to a new header ide-bus.h

[Philippe Mathieu-Daudé]

On 20/2/24 09:55, Thomas Huth wrote:

Let's consolidate the public IDE bus related functions in a separate
header.

Signed-off-by: Thomas Huth 
---
  include/hw/ide/ide-bus.h  | 42 +++
  include/hw/ide/internal.h | 40 +
  2 files changed, 43 insertions(+), 39 deletions(-)
  create mode 100644 include/hw/ide/ide-bus.h


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 4/7] hw/ide: Move IDE device related definitions to ide-dev.h

[Philippe Mathieu-Daudé]

On 20/2/24 09:55, Thomas Huth wrote:

Unentangle internal.h by moving public IDE device related


Disentangle ... from? Untangle? TIL Unentangle.

Anyway I'm not a native English speaker, so:
Reviewed-by: Philippe Mathieu-Daudé 


definitions to ide-dev.h.

Signed-off-by: Thomas Huth 
---
  include/hw/ide/ide-dev.h  | 143 +-
  include/hw/ide/internal.h | 143 +-
  hw/ide/ide-dev.c  |   1 +
  3 files changed, 144 insertions(+), 143 deletions(-)

Re: [PATCH v2 3/7] hw/ide: Move IDE DMA related definitions to a separate header ide-dma.h

[Philippe Mathieu-Daudé]

On 20/2/24 09:55, Thomas Huth wrote:

These definitions are required outside of the hw/ide/ code, too,
so lets's move them from internal.h to a new header called ide-dma.h.

Signed-off-by: Thomas Huth 
---
  include/hw/ide/ide-dma.h  | 37 +
  include/hw/ide/internal.h | 29 +
  2 files changed, 38 insertions(+), 28 deletions(-)
  create mode 100644 include/hw/ide/ide-dma.h

diff --git a/include/hw/ide/ide-dma.h b/include/hw/ide/ide-dma.h
new file mode 100644
index 00..3ac4c3c9fa
--- /dev/null
+++ b/include/hw/ide/ide-dma.h
@@ -0,0 +1,37 @@
+#ifndef HW_IDE_DMA_H
+#define HW_IDE_DMA_H
+
+#include "block/aio.h"
+#include "qemu/iov.h"
+
+typedef struct IDEState IDEState;
+typedef struct IDEDMAOps IDEDMAOps;
+typedef struct IDEDMA IDEDMA;
+
+typedef void DMAStartFunc(const IDEDMA *, IDEState *, BlockCompletionFunc *);
+typedef void DMAVoidFunc(const IDEDMA *);
+typedef int DMAIntFunc(const IDEDMA *, bool);
+typedef int32_t DMAInt32Func(const IDEDMA *, int32_t len);
+typedef void DMAu32Func(const IDEDMA *, uint32_t);
+typedef void DMAStopFunc(const IDEDMA *, bool);
+
+struct IDEDMAOps {
+DMAStartFunc *start_dma;
+DMAVoidFunc *pio_transfer;
+DMAInt32Func *prepare_buf;
+DMAu32Func *commit_buf;
+DMAIntFunc *rw_buf;
+DMAVoidFunc *restart;
+DMAVoidFunc *restart_dma;
+DMAStopFunc *set_inactive;
+DMAVoidFunc *cmd_done;
+DMAVoidFunc *reset;
+};
+
+struct IDEDMA {
+const struct IDEDMAOps *ops;


While here, let's use the typedef, otherwise:

Reviewed-by: Philippe Mathieu-Daudé 


+QEMUIOVector qiov;
+BlockAIOCB *aiocb;
+};

[PATCH 2/2] Implement SMBIOS type 9 v2.6

[Nabih Estefan]

From: Felix Wu 

Signed-off-by: Felix Wu 
Signed-off-by: Nabih Estefan 
---
 hw/smbios/smbios.c   | 49 +---
 include/hw/firmware/smbios.h |  4 +++
 qemu-options.hx  |  2 +-
 3 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 38b3ea172c..e3d5d8f2e2 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -123,7 +123,7 @@ static QTAILQ_HEAD(, type8_instance) type8 = 
QTAILQ_HEAD_INITIALIZER(type8);
 
 /* type 9 instance for parsing */
 struct type9_instance {
-const char *slot_designation;
+const char *slot_designation, *pcidev;
 uint8_t slot_type, slot_data_bus_width, current_usage, slot_length,
 slot_characteristics1, slot_characteristics2;
 uint16_t slot_id;
@@ -436,6 +436,11 @@ static const QemuOptDesc qemu_smbios_type9_opts[] = {
 .type = QEMU_OPT_NUMBER,
 .help = "slot characteristics2, see the spec",
 },
+{
+.name = "pci_device",
+.type = QEMU_OPT_STRING,
+.help = "PCI device, if provided."
+}
 };
 
 static const QemuOptDesc qemu_smbios_type11_opts[] = {
@@ -866,7 +871,7 @@ static void smbios_build_type_8_table(void)
 }
 }
 
-static void smbios_build_type_9_table(void)
+static void smbios_build_type_9_table(Error **errp)
 {
 unsigned instance = 0;
 struct type9_instance *t9;
@@ -883,6 +888,43 @@ static void smbios_build_type_9_table(void)
 t->slot_characteristics1 = t9->slot_characteristics1;
 t->slot_characteristics2 = t9->slot_characteristics2;
 
+if (t9->pcidev) {
+PCIDevice *pdev = NULL;
+int rc = pci_qdev_find_device(t9->pcidev, &pdev);
+if (rc != 0) {
+error_setg(errp,
+   "No PCI device %s for SMBIOS type 9 entry %s",
+   t9->pcidev, t9->slot_designation);
+return;
+}
+/*
+ * We only handle the case were the device is attached to
+ * the PCI root bus. The general case is more complex as
+ * bridges are enumerated later and the table would need
+ * to be updated at this moment.
+ */
+if (!pci_bus_is_root(pci_get_bus(pdev))) {
+error_setg(errp,
+   "Cannot create type 9 entry for PCI device %s: "
+   "not attached to the root bus",
+   t9->pcidev);
+return;
+}
+t->segment_group_number = cpu_to_le16(0);
+t->bus_number = pci_dev_bus_num(pdev);
+t->device_number = pdev->devfn;
+} else {
+/*
+ * Per SMBIOS spec, For slots that are not of the PCI, AGP, PCI-X,
+ * or PCI-Express type that do not have bus/device/function
+ * information, 0FFh should be populated in the fields of Segment
+ * Group Number, Bus Number, Device/Function Number.
+ */
+t->segment_group_number = 0xff;
+t->bus_number = 0xff;
+t->device_number = 0xff;
+}
+
 SMBIOS_BUILD_TABLE_POST;
 instance++;
 }
@@ -1207,7 +1249,7 @@ void smbios_get_tables(MachineState *ms,
 }
 
 smbios_build_type_8_table();
-smbios_build_type_9_table();
+smbios_build_type_9_table(errp);
 smbios_build_type_11_table();
 
 #define MAX_DIMM_SZ (16 * GiB)
@@ -1556,6 +1598,7 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
 t->slot_id = qemu_opt_get_number(opts, "slot_id", 0);
 t->slot_characteristics1 = qemu_opt_get_number(opts, 
"slot_characteristics1", 0);
 t->slot_characteristics2 = qemu_opt_get_number(opts, 
"slot_characteristics2", 0);
+save_opt(&t->pcidev, opts, "pcidev");
 QTAILQ_INSERT_TAIL(&type9, t, next);
 return;
 }
diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
index 9ab114aea2..c21b8d3285 100644
--- a/include/hw/firmware/smbios.h
+++ b/include/hw/firmware/smbios.h
@@ -222,6 +222,10 @@ struct smbios_type_9 {
 uint16_t slot_id;
 uint8_t slot_characteristics1;
 uint8_t slot_characteristics2;
+/* SMBIOS spec v2.6+ */
+uint16_t segment_group_number;
+uint8_t bus_number;
+uint8_t device_number;
 } QEMU_PACKED;
 
 /* SMBIOS type 11 - OEM strings */
diff --git a/qemu-options.hx b/qemu-options.hx
index 9ddb1b1fb3..6a16b808cf 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2717,7 +2717,7 @@ SRST
 ``-smbios 
type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str][,processor-family=%d][,processor-id=%d]``
 Specify SMBIOS type 4 fields
 
-``-smbios 
type=9[,slot_designation=str][,slot_type=%d][,slot_data_bus_width=%d][,current_usage=%d][,slot_length=%d][,slot_id=%d][,slot_characteristics1=%d][,slot_characteristics12=%d

[PATCH] hw/nvme/ns: Add NVMe NGUID property

[Nabih Estefan]

From: Roque Arcudia Hernandez 

This patch adds a way to specify an NGUID for a given NVMe Namespace using a
string of hexadecimal digits with an optional '-' separator to group bytes. For
instance:

-device nvme-ns,nguid="e9accd3b83904e13167cf0593437f57d"

If provided, the NGUID will be part of the Namespace Identification Descriptor
list and the Identify Namespace data.

Signed-off-by: Roque Arcudia Hernandez 
Signed-off-by: Nabih Estefan 
---
 docs/system/devices/nvme.rst |   7 ++
 hw/nvme/ctrl.c   |  12 +++
 hw/nvme/meson.build  |   2 +-
 hw/nvme/nguid.c  | 192 +++
 hw/nvme/ns.c |   2 +
 hw/nvme/nvme.h   |  70 -
 6 files changed, 256 insertions(+), 29 deletions(-)
 create mode 100644 hw/nvme/nguid.c

diff --git a/docs/system/devices/nvme.rst b/docs/system/devices/nvme.rst
index 4ea957baed..d2b1ca9645 100644
--- a/docs/system/devices/nvme.rst
+++ b/docs/system/devices/nvme.rst
@@ -81,6 +81,13 @@ There are a number of parameters available:
   Set the UUID of the namespace. This will be reported as a "Namespace UUID"
   descriptor in the Namespace Identification Descriptor List.
 
+``nguid``
+  Set the NGUID of the namespace. This will be reported as a "Namespace 
Globally
+  Unique Identifier" descriptor in the Namespace Identification Descriptor 
List.
+  It is specified as a string of hexadecimal digits containing exactly 16 bytes
+  or "auto" for a random value. An optional '-' separator could be used to 
group
+  bytes. If not specified the NGUID will remain all zeros.
+
 ``eui64``
   Set the EUI-64 of the namespace. This will be reported as a "IEEE Extended
   Unique Identifier" descriptor in the Namespace Identification Descriptor 
List.
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index f026245d1e..a2dc990dc1 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -5640,6 +5640,10 @@ static uint16_t nvme_identify_ns_descr_list(NvmeCtrl *n, 
NvmeRequest *req)
 NvmeIdNsDescr hdr;
 uint8_t v[NVME_NIDL_UUID];
 } QEMU_PACKED uuid = {};
+struct {
+NvmeIdNsDescr hdr;
+uint8_t v[NVME_NIDL_NGUID];
+} QEMU_PACKED nguid = {};
 struct {
 NvmeIdNsDescr hdr;
 uint64_t v;
@@ -5668,6 +5672,14 @@ static uint16_t nvme_identify_ns_descr_list(NvmeCtrl *n, 
NvmeRequest *req)
 pos += sizeof(uuid);
 }
 
+if (!nvme_nguid_is_null(&ns->params.nguid)) {
+nguid.hdr.nidt = NVME_NIDT_NGUID;
+nguid.hdr.nidl = NVME_NIDL_NGUID;
+memcpy(nguid.v, ns->params.nguid.data, NVME_NIDL_NGUID);
+memcpy(pos, &nguid, sizeof(nguid));
+pos += sizeof(nguid);
+}
+
 if (ns->params.eui64) {
 eui64.hdr.nidt = NVME_NIDT_EUI64;
 eui64.hdr.nidl = NVME_NIDL_EUI64;
diff --git a/hw/nvme/meson.build b/hw/nvme/meson.build
index 1a6a2ca2f3..7d5caa53c2 100644
--- a/hw/nvme/meson.build
+++ b/hw/nvme/meson.build
@@ -1 +1 @@
-system_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('ctrl.c', 'dif.c', 
'ns.c', 'subsys.c'))
+system_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('ctrl.c', 'dif.c', 
'ns.c', 'subsys.c', 'nguid.c'))
\ No newline at end of file
diff --git a/hw/nvme/nguid.c b/hw/nvme/nguid.c
new file mode 100644
index 00..3e3e0567c5
--- /dev/null
+++ b/hw/nvme/nguid.c
@@ -0,0 +1,192 @@
+/*
+ *  QEMU NVMe NGUID functions
+ *
+ * Copyright 2024 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program 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 General Public License
+ * for more details.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/visitor.h"
+#include "qemu/ctype.h"
+#include "nvme.h"
+
+#define NGUID_SEPARATOR '-'
+
+#define NGUID_VALUE_AUTO "auto"
+
+#define NGUID_FMT  \
+"%02hhx%02hhx%02hhx%02hhx" \
+"%02hhx%02hhx%02hhx%02hhx" \
+"%02hhx%02hhx%02hhx%02hhx" \
+"%02hhx%02hhx%02hhx%02hhx"
+
+#define NGUID_STR_LEN (2 * NGUID_LEN + 1)
+
+bool nvme_nguid_is_null(const NvmeNGUID *nguid)
+{
+int i;
+for (i = 0; i < NGUID_LEN; i++) {
+if (nguid->data[i] != 0) {
+return false;
+}
+}
+return true;
+}
+
+static void nvme_nguid_generate(NvmeNGUID *out)
+{
+int i;
+uint32_t x;
+
+QEMU_BUILD_BUG_ON((NGUID_LEN % sizeof(x)) != 0);
+
+for (i = 0; i < NGUID_LEN; i += sizeof(x)) {
+x = g_random_int();
+memcpy(&out->data[i], &x, sizeof(x));
+}
+}
+
+/*
+ * The Linux Kernel typically prints the NGUID of an NVMe namespace using the
+ * same format as the UUID. For instance:
+ *
+ * $ cat /sys/class/block/nvme0n1/nguid
+ * e9accd3b-8390-4e13-167c-f05

[PATCH 0/2] ARM GICv3 ITS DeviceID modification implementation

[Nabih Estefan]

This patch series modifies the ARM GICv3 ITS to use the already existing
send_msi virtual function when writing the GITS_TRANSLATER in order to be able
to modify the final DeviceID to an implementation specific version that requires
extra information besides the BDF that comes in the requester_id. This is
achieved by using inheritance and redefinition of the send_msi while the
parent's send_msi could still be used to inject the modified DeviceID.

Roque Arcudia Hernandez (2):
  hw/intc/arm_gicv3_its_common: Increase DeviceID to 32 bits
  hw/intc/arm_gicv3_its: Use send_msi in the GITS_TRANSLATER write

 hw/intc/arm_gicv3_its.c| 20 +---
 hw/intc/arm_gicv3_its_kvm.c|  2 +-
 include/hw/intc/arm_gicv3_its_common.h | 15 ---
 3 files changed, 22 insertions(+), 15 deletions(-)

-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH 2/2] hw/arm/smmu-common: Create virtual function for implementation defined StreamID

[Nabih Estefan]

From: Roque Arcudia Hernandez 

According to the SMMU specification the StreamID construction and size is
IMPLEMENTATION DEFINED, the size being between 0 and 32 bits.

This patch creates virtual functions get_sid and get_iommu_mr to allow different
implementations of how the StreamID is constructed via inheritance. The default
implementation of these functions will match the current ones where the BDF is
used directly as StreamID.

Signed-off-by: Roque Arcudia Hernandez 
Signed-off-by: Nabih Estefan 
---
 hw/arm/smmu-common.c | 12 
 include/hw/arm/smmu-common.h | 16 +++-
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 4caedb4998..14b3240a88 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -621,6 +621,11 @@ static const PCIIOMMUOps smmu_ops = {
 };
 
 IOMMUMemoryRegion *smmu_iommu_mr(SMMUState *s, uint32_t sid)
+{
+return ARM_SMMU_GET_CLASS(s)->get_iommu_mr(s, sid);
+}
+
+static IOMMUMemoryRegion *smmu_base_iommu_mr(SMMUState *s, uint32_t sid)
 {
 uint8_t bus_n, devfn;
 SMMUPciBus *smmu_bus;
@@ -659,6 +664,11 @@ void smmu_inv_notifiers_all(SMMUState *s)
 }
 }
 
+static uint32_t smmu_base_get_sid(SMMUDevice *sdev)
+{
+return PCI_BUILD_BDF(pci_bus_num(sdev->bus), sdev->devfn);
+}
+
 static void smmu_base_realize(DeviceState *dev, Error **errp)
 {
 SMMUState *s = ARM_SMMU(dev);
@@ -709,6 +719,8 @@ static void smmu_base_class_init(ObjectClass *klass, void 
*data)
 device_class_set_parent_realize(dc, smmu_base_realize,
 &sbc->parent_realize);
 rc->phases.hold = smmu_base_reset_hold;
+sbc->get_sid = smmu_base_get_sid;
+sbc->get_iommu_mr = smmu_base_iommu_mr;
 }
 
 static const TypeInfo smmu_base_info = {
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index 5ec2e6c1a4..d53121fe37 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -131,6 +131,9 @@ typedef struct SMMUIOTLBKey {
 uint8_t level;
 } SMMUIOTLBKey;
 
+#define TYPE_ARM_SMMU "arm-smmu"
+OBJECT_DECLARE_TYPE(SMMUState, SMMUBaseClass, ARM_SMMU)
+
 struct SMMUState {
 /*  */
 SysBusDevice  dev;
@@ -147,6 +150,9 @@ struct SMMUState {
 PCIBus *primary_bus;
 };
 
+typedef uint32_t GetSidFunc(SMMUDevice *obj);
+typedef IOMMUMemoryRegion *GetIommuMr(SMMUState *s, uint32_t sid);
+
 struct SMMUBaseClass {
 /*  */
 SysBusDeviceClass parent_class;
@@ -154,19 +160,19 @@ struct SMMUBaseClass {
 /*< public >*/
 
 DeviceRealize parent_realize;
+GetSidFunc *get_sid;
+GetIommuMr *get_iommu_mr;
 
 };
 
-#define TYPE_ARM_SMMU "arm-smmu"
-OBJECT_DECLARE_TYPE(SMMUState, SMMUBaseClass, ARM_SMMU)
-
 /* Return the SMMUPciBus handle associated to a PCI bus number */
 SMMUPciBus *smmu_find_smmu_pcibus(SMMUState *s, uint8_t bus_num);
 
 /* Return the stream ID of an SMMU device */
-static inline uint16_t smmu_get_sid(SMMUDevice *sdev)
+static inline uint32_t smmu_get_sid(SMMUDevice *sdev)
 {
-return PCI_BUILD_BDF(pci_bus_num(sdev->bus), sdev->devfn);
+SMMUState *s = sdev->smmu;
+return ARM_SMMU_GET_CLASS(s)->get_sid(sdev);
 }
 
 /**
-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH 0/2] ARM SMMUv3 StreamID Implementation

[Nabih Estefan]

This patch series modifies the ARM SMMUv3 to be able to work with an
implementation specific StreamID that does not match exactly the PCIe BDF.
The way to achieve this is by converting the smmu_get_sid and smmu_iommu_mr
functions to virtual functions that can be overridden by inheritance, making
sure the StreamID is consistently 32 bits and removing the hardcoding of the
SMMU_IDR1.SIDSIZE to 16 bits.

Roque Arcudia Hernandez (2):
  hw/arm/smmuv3: Check StreamIDs against SMMU_IDR1.SIDSIZE value
  hw/arm/smmu-common: Create virtual function for implementation defined
StreamID

 hw/arm/smmu-common.c | 12 
 hw/arm/smmuv3.c  |  4 +++-
 include/hw/arm/smmu-common.h | 16 +++-
 3 files changed, 26 insertions(+), 6 deletions(-)

-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH 1/2] hw/arm/smmuv3: Check StreamIDs against SMMU_IDR1.SIDSIZE value

[Nabih Estefan]

From: Roque Arcudia Hernandez 

Current implementation checks the StreamIDs against STRTAB_BASE_CFG.LOG2SIZE
register field value and a constant SMMU_IDR1_SIDSIZE which is also used as
initial value for field SMMU_IDR1.SIDSIZE.

This limits the possibility of extending the SMMUv3 by inheritance and
redefining the value of SMMU_IDR1.SIDSIZE because the check is hardcoded to the
constant SMMU_IDR1_SIDSIZE rather than the register value.

Signed-off-by: Roque Arcudia Hernandez 
Signed-off-by: Nabih Estefan 
---
 hw/arm/smmuv3.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 9eb56a70f3..a01031821a 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -580,15 +580,17 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, 
STE *ste,
 {
 dma_addr_t addr, strtab_base;
 uint32_t log2size;
+uint32_t idr1_sidsize;
 int strtab_size_shift;
 int ret;
 
 trace_smmuv3_find_ste(sid, s->features, s->sid_split);
 log2size = FIELD_EX32(s->strtab_base_cfg, STRTAB_BASE_CFG, LOG2SIZE);
+idr1_sidsize = FIELD_EX32(s->idr[1], IDR1, SIDSIZE);
 /*
  * Check SID range against both guest-configured and implementation limits
  */
-if (sid >= (1 << MIN(log2size, SMMU_IDR1_SIDSIZE))) {
+if (sid >= (1 << MIN(log2size, idr1_sidsize))) {
 event->type = SMMU_EVT_C_BAD_STREAMID;
 return -EINVAL;
 }
-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH 1/2] hw/intc/arm_gicv3_its_common: Increase DeviceID to 32 bits

[Nabih Estefan]

From: Roque Arcudia Hernandez 

According to the “GICv3 and GICv4 Software Overview” the DeviceID is
IMPLEMENTATION DEFINED. This patch increases the DeviceID in send_msi virtual
function to 32 bits to allow the possibility of a redefinition of it with a
bigger DeviceID.

Signed-off-by: Roque Arcudia Hernandez 
Signed-off-by: Nabih Estefan 
---
 hw/intc/arm_gicv3_its_kvm.c| 2 +-
 include/hw/intc/arm_gicv3_its_common.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
index 3befc960db..4eaf1cfcad 100644
--- a/hw/intc/arm_gicv3_its_kvm.c
+++ b/hw/intc/arm_gicv3_its_kvm.c
@@ -42,7 +42,7 @@ struct KVMARMITSClass {
 };
 
 
-static int kvm_its_send_msi(GICv3ITSState *s, uint32_t value, uint16_t devid)
+static int kvm_its_send_msi(GICv3ITSState *s, uint32_t value, uint32_t devid)
 {
 struct kvm_msi msi;
 
diff --git a/include/hw/intc/arm_gicv3_its_common.h 
b/include/hw/intc/arm_gicv3_its_common.h
index 7dc712b38d..e072c36cca 100644
--- a/include/hw/intc/arm_gicv3_its_common.h
+++ b/include/hw/intc/arm_gicv3_its_common.h
@@ -117,7 +117,7 @@ struct GICv3ITSCommonClass {
 SysBusDeviceClass parent_class;
 /*< public >*/
 
-int (*send_msi)(GICv3ITSState *s, uint32_t data, uint16_t devid);
+int (*send_msi)(GICv3ITSState *s, uint32_t data, uint32_t devid);
 void (*pre_save)(GICv3ITSState *s);
 void (*post_load)(GICv3ITSState *s);
 };
-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH 2/2] hw/intc/arm_gicv3_its: Use send_msi in the GITS_TRANSLATER write

[Nabih Estefan]

From: Roque Arcudia Hernandez 

This is trying to achieve 2 things: To be able to redefine the send_msi in a
derived class of arm_gicv3_its and/or to expose a method call interface to
inject interrupts from another device.

Signed-off-by: Roque Arcudia Hernandez 
Signed-off-by: Nabih Estefan 
---
 hw/intc/arm_gicv3_its.c| 20 +---
 include/hw/intc/arm_gicv3_its_common.h | 13 +++--
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 52e9aca9c6..9342e96be3 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -20,16 +20,6 @@
 #include "qom/object.h"
 #include "qapi/error.h"
 
-typedef struct GICv3ITSClass GICv3ITSClass;
-/* This is reusing the GICv3ITSState typedef from ARM_GICV3_ITS_COMMON */
-DECLARE_OBJ_CHECKERS(GICv3ITSState, GICv3ITSClass,
- ARM_GICV3_ITS, TYPE_ARM_GICV3_ITS)
-
-struct GICv3ITSClass {
-GICv3ITSCommonClass parent_class;
-ResettablePhases parent_phases;
-};
-
 /*
  * This is an internal enum used to distinguish between LPI triggered
  * via command queue and LPI triggered via gits_translater write.
@@ -1561,7 +1551,8 @@ static MemTxResult gicv3_its_translation_write(void 
*opaque, hwaddr offset,
 switch (offset) {
 case GITS_TRANSLATER:
 if (s->ctlr & R_GITS_CTLR_ENABLED_MASK) {
-result = do_process_its_cmd(s, attrs.requester_id, data, NONE);
+GICv3ITSCommonClass *c = ARM_GICV3_ITS_COMMON_GET_CLASS(s);
+result = c->send_msi(s, data, attrs.requester_id);
 }
 break;
 default:
@@ -1994,6 +1985,12 @@ static void gicv3_its_reset_hold(Object *obj)
 }
 }
 
+static int gicv3_its_send_msi(GICv3ITSState *s, uint32_t eventid,
+  uint32_t devid)
+{
+return do_process_its_cmd(s, devid, eventid, NONE);
+}
+
 static void gicv3_its_post_load(GICv3ITSState *s)
 {
 if (s->ctlr & R_GITS_CTLR_ENABLED_MASK) {
@@ -2020,6 +2017,7 @@ static void gicv3_its_class_init(ObjectClass *klass, void 
*data)
 resettable_class_set_parent_phases(rc, NULL, gicv3_its_reset_hold, NULL,
&ic->parent_phases);
 icc->post_load = gicv3_its_post_load;
+icc->send_msi = gicv3_its_send_msi;
 }
 
 static const TypeInfo gicv3_its_info = {
diff --git a/include/hw/intc/arm_gicv3_its_common.h 
b/include/hw/intc/arm_gicv3_its_common.h
index e072c36cca..c81bd0b26e 100644
--- a/include/hw/intc/arm_gicv3_its_common.h
+++ b/include/hw/intc/arm_gicv3_its_common.h
@@ -25,8 +25,6 @@
 #include "hw/intc/arm_gicv3_common.h"
 #include "qom/object.h"
 
-#define TYPE_ARM_GICV3_ITS "arm-gicv3-its"
-
 #define ITS_CONTROL_SIZE 0x1
 #define ITS_TRANS_SIZE   0x1
 #define ITS_SIZE (ITS_CONTROL_SIZE + ITS_TRANS_SIZE)
@@ -132,4 +130,15 @@ struct GICv3ITSCommonClass {
  */
 const char *its_class_name(void);
 
+#define TYPE_ARM_GICV3_ITS "arm-gicv3-its"
+typedef struct GICv3ITSClass GICv3ITSClass;
+/* This is reusing the GICv3ITSState typedef from ARM_GICV3_ITS_COMMON */
+DECLARE_OBJ_CHECKERS(GICv3ITSState, GICv3ITSClass,
+ ARM_GICV3_ITS, TYPE_ARM_GICV3_ITS)
+
+struct GICv3ITSClass {
+GICv3ITSCommonClass parent_class;
+ResettablePhases parent_phases;
+};
+
 #endif
-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH 0/2] SMBIOS type 9 descriptor implementation

[Nabih Estefan]

This patch series implements SMBIOS type 9 descriptor, system slots.
For each system slot, we can assign one descriptor for it if needed.
In versions later than 2.6, a new PCI device field was added to make sure the
descriptor is associated with a certain device, if provided.
For ease of usage, qemu-options.hx was updated.

Felix Wu (2):
  Implement base of SMBIOS type 9 descriptor.
  Implement SMBIOS type 9 v2.6

 hw/smbios/smbios.c   | 142 +++
 include/hw/firmware/smbios.h |  17 +
 qemu-options.hx  |   3 +
 3 files changed, 162 insertions(+)

-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH 1/2] Implement base of SMBIOS type 9 descriptor.

[Nabih Estefan]

From: Felix Wu 

Version 2.1+.

Signed-off-by: Felix Wu 
Signed-off-by: Nabih Estefan 
---
 hw/smbios/smbios.c   | 99 
 include/hw/firmware/smbios.h | 13 +
 qemu-options.hx  |  3 ++
 3 files changed, 115 insertions(+)

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index a3c4e52ce9..38b3ea172c 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -121,6 +121,16 @@ struct type8_instance {
 };
 static QTAILQ_HEAD(, type8_instance) type8 = QTAILQ_HEAD_INITIALIZER(type8);
 
+/* type 9 instance for parsing */
+struct type9_instance {
+const char *slot_designation;
+uint8_t slot_type, slot_data_bus_width, current_usage, slot_length,
+slot_characteristics1, slot_characteristics2;
+uint16_t slot_id;
+QTAILQ_ENTRY(type9_instance) next;
+};
+static QTAILQ_HEAD(, type9_instance) type9 = QTAILQ_HEAD_INITIALIZER(type9);
+
 static struct {
 size_t nvalues;
 char **values;
@@ -380,6 +390,54 @@ static const QemuOptDesc qemu_smbios_type8_opts[] = {
 { /* end of list */ }
 };
 
+static const QemuOptDesc qemu_smbios_type9_opts[] = {
+{
+.name = "type",
+.type = QEMU_OPT_NUMBER,
+.help = "SMBIOS element type",
+},
+{
+.name = "slot_designation",
+.type = QEMU_OPT_STRING,
+.help = "string number for reference designation",
+},
+{
+.name = "slot_type",
+.type = QEMU_OPT_NUMBER,
+.help = "connector type",
+},
+{
+.name = "slot_data_bus_width",
+.type = QEMU_OPT_NUMBER,
+.help = "port type",
+},
+{
+.name = "current_usage",
+.type = QEMU_OPT_NUMBER,
+.help = "current usage",
+},
+{
+.name = "slot_length",
+.type = QEMU_OPT_NUMBER,
+.help = "system slot length",
+},
+{
+.name = "slot_id",
+.type = QEMU_OPT_NUMBER,
+.help = "system slot id",
+},
+{
+.name = "slot_characteristics1",
+.type = QEMU_OPT_NUMBER,
+.help = "slot characteristics1, see the spec",
+},
+{
+.name = "slot_characteristics2",
+.type = QEMU_OPT_NUMBER,
+.help = "slot characteristics2, see the spec",
+},
+};
+
 static const QemuOptDesc qemu_smbios_type11_opts[] = {
 {
 .name = "type",
@@ -609,6 +667,7 @@ bool smbios_skip_table(uint8_t type, bool required_table)
 #define T2_BASE 0x200
 #define T3_BASE 0x300
 #define T4_BASE 0x400
+#define T9_BASE 0x900
 #define T11_BASE 0xe00
 
 #define T16_BASE 0x1000
@@ -807,6 +866,28 @@ static void smbios_build_type_8_table(void)
 }
 }
 
+static void smbios_build_type_9_table(void)
+{
+unsigned instance = 0;
+struct type9_instance *t9;
+
+QTAILQ_FOREACH(t9, &type9, next) {
+SMBIOS_BUILD_TABLE_PRE(9, T9_BASE + instance, true);
+
+SMBIOS_TABLE_SET_STR(9, slot_designation, t9->slot_designation);
+t->slot_type = t9->slot_type;
+t->slot_data_bus_width = t9->slot_data_bus_width;
+t->current_usage = t9->current_usage;
+t->slot_length = t9->slot_length;
+t->slot_id = t9->slot_id;
+t->slot_characteristics1 = t9->slot_characteristics1;
+t->slot_characteristics2 = t9->slot_characteristics2;
+
+SMBIOS_BUILD_TABLE_POST;
+instance++;
+}
+}
+
 static void smbios_build_type_11_table(void)
 {
 char count_str[128];
@@ -1126,6 +1207,7 @@ void smbios_get_tables(MachineState *ms,
 }
 
 smbios_build_type_8_table();
+smbios_build_type_9_table();
 smbios_build_type_11_table();
 
 #define MAX_DIMM_SZ (16 * GiB)
@@ -1460,6 +1542,23 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
 t8_i->port_type = qemu_opt_get_number(opts, "port_type", 0);
 QTAILQ_INSERT_TAIL(&type8, t8_i, next);
 return;
+case 9: {
+if (!qemu_opts_validate(opts, qemu_smbios_type9_opts, errp)) {
+return;
+}
+struct type9_instance *t;
+t = g_new0(struct type9_instance, 1);
+save_opt(&t->slot_designation, opts, "slot_designation");
+t->slot_type = qemu_opt_get_number(opts, "slot_type", 0);
+t->slot_data_bus_width = qemu_opt_get_number(opts, 
"slot_data_bus_width", 0);
+t->current_usage = qemu_opt_get_number(opts, "current_usage", 0);
+t->slot_length = qemu_opt_get_number(opts, "slot_length", 0);
+t->slot_id = qemu_opt_get_number(opts, "slot_id", 0);
+t->slot_characteristics1 = qemu_opt_get_number(opts, 
"slot_characteristics1", 0);
+t->slot_characteristics2 = qemu_opt_get_number(opts, 
"slot_characteristics2", 0);
+QTAILQ_INSERT_TAIL(&type9, t, next);
+return;
+}
 case 11:
 if (!qemu_opts_validate(opts, qemu_smbios_type11_opts, errp)) {
 return;
diff --git a/include/hw/fi

Re: [PATCH 23/23] qemu-img: inline list of supported commands, remove qemu-img-cmds.h include

[Daniel P . Berrangé]

On Wed, Feb 21, 2024 at 07:31:42PM +0300, Michael Tokarev wrote:
> 20.02.2024 21:48, Daniel P. Berrangé:
> 
> > This ends up looking a bit muddled together. I don't think we
> > need repeat 'qemu-img ' twice, and could add a little
> > more whitespace
> > 
> > eg instead of:
> > 
> > $ ./build/qemu-img check --help
> > qemu-img check: Check basic image integrity.  Usage:
> > qemu-img check [-f FMT | --image-opts] [-T CACHE_MODE] [-r] [-u]
> >  [--output human|json] [--object OBJDEF] FILENAME
> > Arguments:
> > ...snip...
> > 
> > have it look like
> > 
> > $ ./build/qemu-img check --help
> > Check basic image integrity.
> > 
> > Usage:
> > 
> >qemu-img check [-f FMT | --image-opts] [-T CACHE_MODE] [-r] [-u]
> >  [--output human|json] [--object OBJDEF] FILENAME
> > 
> > Arguments:
> > ...snip...
> 
> Here's the current way how `create' help text looks like:
> 
> $ ./qemu-img create --help
> Create and format qemu image file.  Usage:
>   qemu-img create [-f FMT] [-o FMT_OPTS] [-b BACKING_FILENAME [-F 
> BACKING_FMT]]
> [--object OBJDEF] [-u] FILENAME [SIZE[bkKMGTPE]]
> Arguments:
>   -h, --help
>  print this help and exit
>   -q, --quiet
>  quiet operations
>   -f, --format FMT
>  specifies format of the new image, default is raw
>   -o, --options FMT_OPTS
>  format-specific options ('-o list' for list)
>   -b, --backing BACKING_FILENAME
>  stack new image on top of BACKING_FILENAME
>  (for formats which support stacking)
>   -F, --backing-format BACKING_FMT
>  specify format of BACKING_FILENAME
>   -u, --backing-unsafe
>  do not fail if BACKING_FMT can not be read
>   --object OBJDEF
>  QEMU user-creatable object (eg encryption key)
>   FILENAME
>  image file to create.  It will be overridden if exists
>   SIZE
>  image size with optional suffix (multiplies in 1024)
>  SIZE is required unless BACKING_IMG is specified,
>  in which case it will be the same as size of BACKING_IMG
> 
> Maybe it's a good idea to add newlines around the "syntax" part,
> ie, after "Usage:" and before "Arguments:".  I don't think it needs
> extra newlines between each argument description though, - this way
> it becomes just too long.
> 
> What do you think?

I still prefer to have more vertical whitespace, as I find it harder
to read through without it. I was using the typical man page option
/ usage formatting as a guide in my feedback.

Still, it would be useful to see what other maintainers think, as I'm
just one data point.

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH 23/23] qemu-img: inline list of supported commands, remove qemu-img-cmds.h include

[Michael Tokarev]

20.02.2024 21:48, Daniel P. Berrangé:


This ends up looking a bit muddled together. I don't think we
need repeat 'qemu-img ' twice, and could add a little
more whitespace

eg instead of:

$ ./build/qemu-img check --help
qemu-img check: Check basic image integrity.  Usage:
qemu-img check [-f FMT | --image-opts] [-T CACHE_MODE] [-r] [-u]
 [--output human|json] [--object OBJDEF] FILENAME
Arguments:
...snip...

have it look like

$ ./build/qemu-img check --help
Check basic image integrity.

Usage:

   qemu-img check [-f FMT | --image-opts] [-T CACHE_MODE] [-r] [-u]
 [--output human|json] [--object OBJDEF] FILENAME

Arguments:
...snip...


Here's the current way how `create' help text looks like:

$ ./qemu-img create --help
Create and format qemu image file.  Usage:
  qemu-img create [-f FMT] [-o FMT_OPTS] [-b BACKING_FILENAME [-F BACKING_FMT]]
[--object OBJDEF] [-u] FILENAME [SIZE[bkKMGTPE]]
Arguments:
  -h, --help
 print this help and exit
  -q, --quiet
 quiet operations
  -f, --format FMT
 specifies format of the new image, default is raw
  -o, --options FMT_OPTS
 format-specific options ('-o list' for list)
  -b, --backing BACKING_FILENAME
 stack new image on top of BACKING_FILENAME
 (for formats which support stacking)
  -F, --backing-format BACKING_FMT
 specify format of BACKING_FILENAME
  -u, --backing-unsafe
 do not fail if BACKING_FMT can not be read
  --object OBJDEF
 QEMU user-creatable object (eg encryption key)
  FILENAME
 image file to create.  It will be overridden if exists
  SIZE
 image size with optional suffix (multiplies in 1024)
 SIZE is required unless BACKING_IMG is specified,
 in which case it will be the same as size of BACKING_IMG

Maybe it's a good idea to add newlines around the "syntax" part,
ie, after "Usage:" and before "Arguments:".  I don't think it needs
extra newlines between each argument description though, - this way
it becomes just too long.

What do you think?

Thanks,

/mjt

Re: [PATCH 12/23] qemu-img: make -l (list) the default for "snapshot" subcommand

[Michael Tokarev]

20.02.2024 21:51, Michael Tokarev wrote:

20.02.2024 20:45, Daniel P. Berrangé wrote:

On Sat, Feb 10, 2024 at 12:22:33AM +0300, Michael Tokarev wrote:

also remove bdrv_oflags handling (only list can use RO mode)
---
  qemu-img.c | 13 -
  1 file changed, 8 insertions(+), 5 deletions(-)


I'd suggest docs/tools/qemu-img.rst should also be updated to say


qemu-img.rst needs to be updated, significantly more than this.


Actually, - yes, you're right in this case.  Added.  A good suggestion.

Thanks,

/mjt

Re: [RFC 1/4] drive-mirror: add support for sync=bitmap mode=never

[Fiona Ebner]

Am 21.02.24 um 07:55 schrieb Markus Armbruster:
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index ab5a93a966..ac05483958 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -2181,6 +2181,15 @@
>>  # destination (all the disk, only the sectors allocated in the
>>  # topmost image, or only new I/O).
>>  #
>> +# @bitmap: The name of a bitmap to use for sync=bitmap mode.  This
>> +# argument must be present for bitmap mode and absent otherwise.
>> +# The bitmap's granularity is used instead of @granularity.
>> +# (Since 9.0).
> 
> What happens when the user specifies @granularity anyway?  Error or
> silently ignored?
> 

It's an error:

>> +if (bitmap) {
>> +if (granularity) {
>> +error_setg(errp, "granularity (%d)"
>> +   "cannot be specified when a bitmap is provided",
>> +   granularity);
>> +return NULL;
>> +}

>> +#
>> +# @bitmap-mode: Specifies the type of data the bitmap should contain
>> +# after the operation concludes.  Must be present if sync is
>> +# "bitmap".  Must NOT be present otherwise.  (Since 9.0)
> 
> Members that must be present when and only when some enum member has a
> certain value should perhaps be in a union branch.  Perhaps the block
> maintainers have an opinion here.
> 

Sounds sensible to me. Considering also the next patches, in the end it
could be a union discriminated by the @sync which contains @bitmap and
@bitmap-mode when it's the 'bitmap' sync mode, @bitmap when it's the
'incremental' sync mode (@bitmap-sync mode needs to be 'on-success'
then, so there is no choice for the user) and which contains
@granularity for the other sync modes.

Best Regards,
Fiona

Re: [PATCH v2 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h

[Mark Cave-Ayland]

On 20/02/2024 08:54, Thomas Huth wrote:


While trying to make it possible to compile-out the CompactFlash IDE device
in downstream distributions (first patch), we noticed that there are more
things in the IDE code that could use a proper clean up:

First, hw/ide/qdev.c is quite a mix between IDE BUS specific functions
and (disk) device specific functions. Thus the second patch splits qdev.c
into two new separate files to make it more obvious which part belongs
to which kind of devices.

The remaining patches unentangle include/hw/ide/internal.h, which is meant
as a header that should only be used internally to the IDE subsystem, but
which is currently exposed to the world since include/hw/ide/pci.h includes
this header, too. Thus we move the definitions that are also required for
non-IDE code to other new header files, so we can finally change pci.h to
stop including internal.h. After these changes, internal.h is only included
by files in hw/ide/ as it should be.

v2:
- Change the order of the DMA patch and move typedef struct IDEDMAOps
   and typedef struct IDEDMA IDEDMA into ide-dma.h, too
- Make sure that the headers are self-contained (i.e. #include the
   right other headers)
- Remove some more remnants of include/hw/ide.h

Thomas Huth (7):
   hw/ide: Add the possibility to disable the CompactFlash device in the
 build
   hw/ide: Split qdev.c into ide-bus.c and ide-dev.c
   hw/ide: Move IDE DMA related definitions to a separate header
 ide-dma.h
   hw/ide: Move IDE device related definitions to ide-dev.h
   hw/ide: Move IDE bus related definitions to a new header ide-bus.h
   hw/ide: Remove the include/hw/ide.h legacy file
   hw/ide: Stop exposing internal.h to non-IDE files

  MAINTAINERS  |   1 -
  include/hw/ide.h |   9 --
  include/hw/ide/ide-bus.h |  42 +++
  include/hw/ide/ide-dev.h | 184 ++
  include/hw/ide/ide-dma.h |  37 ++
  include/hw/ide/internal.h| 211 +--
  include/hw/ide/pci.h |   2 +-
  hw/i386/pc.c |   2 +-
  hw/ide/cf.c  |  58 ++
  hw/ide/cmd646.c  |   1 +
  hw/ide/ide-bus.c | 111 ++
  hw/ide/{qdev.c => ide-dev.c} | 137 +--
  hw/ide/pci.c |   1 +
  hw/ide/piix.c|   1 +
  hw/ide/sii3112.c |   1 +
  hw/ide/via.c |   1 +
  hw/arm/Kconfig   |   2 +
  hw/ide/Kconfig   |  32 --
  hw/ide/meson.build   |   4 +-
  19 files changed, 470 insertions(+), 367 deletions(-)
  delete mode 100644 include/hw/ide.h
  create mode 100644 include/hw/ide/ide-bus.h
  create mode 100644 include/hw/ide/ide-dev.h
  create mode 100644 include/hw/ide/ide-dma.h
  create mode 100644 hw/ide/cf.c
  create mode 100644 hw/ide/ide-bus.c
  rename hw/ide/{qdev.c => ide-dev.c} (67%)


I've had a quick skim of this series, and it looks like a good tidy-up to me so:

Acked-by: Mark Cave-Ayland 


ATB,

Mark.

Re: [PATCH 15/23] qemu-img: resize: do not always eat last argument

[Michael Tokarev]

20.02.2024 20:57, Daniel P. Berrangé пишет:

On Sat, Feb 10, 2024 at 12:22:36AM +0300, Michael Tokarev wrote:

'qemu-img resize --help' does not work, since it wants more arguments.
Only eat last option at the beginning if it starts like -N.., and allow
getopt() to do its work, and eat it up at the end if not already eaten.
This will not allow to mix options and size anyway, but it is better
than now.

Signed-off-by: Michael Tokarev 
---
  qemu-img.c | 15 +--
  1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 69d41e0a92..929a25a021 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4271,13 +4271,13 @@ static int img_resize(const img_cmd_t *ccmd, int argc, 
char **argv)
  
  /* Remove size from argv manually so that negative numbers are not treated

   * as options by getopt. */
-if (argc < 3) {
-error_exit(ccmd, "Not enough arguments");
-return 1;
+if (argc > 1 && argv[argc - 1][0] == '-'
+&& argv[argc-1][1] >= '0' && argv[argc-1][1] <= '9') {
+size = argv[--argc];
+} else {
+size = NULL;
  }


We already have a variable 'int relative' that is set to '-1'
or '+1' depending on whether we have a -ve or +ve size.

I think it is clearer to follow if we just set 'relative' much
earlier before parsing by moving this chunk of code to before
the getopt:


Well, we'll also have to repeat the same switch after getopt, since
there, I only test for -[0-9], not +[0-9] or [0-9].   But yes, it
can be done too.

But this is more interesting, - I think we should switch getopt to
use 'return in order' option, and process options together with
getopt, looking at the next option at each iteration, - if it looks
like [+-]size if we already got the filename part.  Lemme try to
do that..

/mjt

Re: [PATCH v6 15/15] hw/qdev: Remove opts member

[Markus Armbruster]

Akihiko Odaki  writes:

> It is no longer used.
>
> Signed-off-by: Akihiko Odaki 
> Reviewed-by: Philippe Mathieu-Daudé 
> ---
>  include/hw/qdev-core.h |  4 
>  hw/core/qdev.c |  1 -
>  system/qdev-monitor.c  | 12 +++-
>  3 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 9228e96c87e9..5954404dcbfe 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -237,10 +237,6 @@ struct DeviceState {
>   * @pending_deleted_expires_ms: optional timeout for deletion events
>   */
>  int64_t pending_deleted_expires_ms;
> -/**
> - * @opts: QDict of options for the device
> - */
> -QDict *opts;
>  /**
>   * @hotplugged: was device added after PHASE_MACHINE_READY?
>   */
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index c68d0f7c512f..7349c9a86be8 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -706,7 +706,6 @@ static void device_finalize(Object *obj)
>  dev->canonical_path = NULL;
>  }
>  
> -qobject_unref(dev->opts);
>  g_free(dev->id);
>  }
>  
> diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
> index a13db763e5dd..71c00f62ee38 100644
> --- a/system/qdev-monitor.c
> +++ b/system/qdev-monitor.c
> @@ -625,6 +625,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
>  char *id;
>  DeviceState *dev = NULL;
>  BusState *bus = NULL;
> +QDict *properties;
>  
>  driver = qdict_get_try_str(opts, "driver");
>  if (!driver) {
> @@ -705,13 +706,14 @@ DeviceState *qdev_device_add_from_qdict(const QDict 
> *opts,
>  }
>  
>  /* set properties */
> -dev->opts = qdict_clone_shallow(opts);
> -qdict_del(dev->opts, "driver");
> -qdict_del(dev->opts, "bus");
> -qdict_del(dev->opts, "id");
> +properties = qdict_clone_shallow(opts);
> +qdict_del(properties, "driver");
> +qdict_del(properties, "bus");
> +qdict_del(properties, "id");
>  
> -object_set_properties_from_keyval(&dev->parent_obj, dev->opts, from_json,
> +object_set_properties_from_keyval(&dev->parent_obj, properties, 
> from_json,
>errp);
> +qobject_unref(properties);
>  if (*errp) {
>  goto err_del_dev;
>  }

Reviewed-by: Markus Armbruster 

Depends on the previous few patches, of course.

Re: [PATCH v6 14/15] vfio: Avoid inspecting option QDict for rombar

[Markus Armbruster]

Akihiko Odaki  writes:

> Use pci_rom_bar_explicitly_enabled() to determine if rombar is explicitly
> enabled.
>
> Signed-off-by: Akihiko Odaki 
> ---
>  hw/vfio/pci.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 4fa387f0430d..647f15b2a060 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1012,7 +1012,6 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev)
>  {
>  uint32_t orig, size = cpu_to_le32((uint32_t)PCI_ROM_ADDRESS_MASK);
>  off_t offset = vdev->config_offset + PCI_ROM_ADDRESS;
> -DeviceState *dev = DEVICE(vdev);
>  char *name;
>  int fd = vdev->vbasedev.fd;
>  
> @@ -1046,7 +1045,7 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev)
>  }
>  
>  if (vfio_opt_rom_in_denylist(vdev)) {
> -if (dev->opts && qdict_haskey(dev->opts, "rombar")) {
> +if (pci_rom_bar_explicitly_enabled(&vdev->pdev)) {
>  warn_report("Device at %s is known to cause system instability"
>  " issues during option rom execution",
>  vdev->vbasedev.name);

Consider -device ...,rombar=0x.

Before the patch, the condition is true.

Afterwards, it's false.

Do we care?

Re: [PATCH v6 13/15] hw/pci: Determine if rombar is explicitly enabled

[Markus Armbruster]

Akihiko Odaki  writes:

> vfio determines if rombar is explicitly enabled by inspecting QDict.
> Inspecting QDict is not nice because QDict is untyped and depends on the
> details on the external interface. Add an infrastructure to determine if
> rombar is explicitly enabled to hw/pci.
>
> Signed-off-by: Akihiko Odaki 
> ---
>  include/hw/pci/pci_device.h | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
> index ca151325085d..c4fdc96ef50d 100644
> --- a/include/hw/pci/pci_device.h
> +++ b/include/hw/pci/pci_device.h
> @@ -205,6 +205,11 @@ static inline uint16_t pci_get_bdf(PCIDevice *dev)
>  return PCI_BUILD_BDF(pci_bus_num(pci_get_bus(dev)), dev->devfn);
>  }
>  
> +static inline bool pci_rom_bar_explicitly_enabled(PCIDevice *dev)
> +{
> +return dev->rom_bar && dev->rom_bar != -1;

Compares signed with unsigned: rom_bar is uint32_t, -1 is int.

If int is at most 32 bits, the comparison converts -1 to
(uint32_t)0x.

If int is wider, it converts dev->rom_bar to int without changing its
value.  In particular, it converts the default value 0x (written
as -1 in the previous patch) to (int)0x.  Oops.

Best not to mix signed and unsigned in comparisons.  Easy enough to
avoid here.

Still, we don't actually test "rom_bar has not been set".  We test
"rom_bar has the default value".  Nothing stops a user from passing
rombar=0x to -device.  See my review of the next patch.

> +}
> +
>  static inline void pci_set_power(PCIDevice *pci_dev, bool state)
>  {
>  /*

Re: [PATCH v6 12/15] hw/pci: Use -1 as a default value for rombar

[Markus Armbruster]

Akihiko Odaki  writes:

> Currently there is no way to distinguish the case that rombar is
> explicitly specified as 1 and the case that rombar is not specified.
>
> Set rombar -1 by default to distinguish these cases just as it is done
> for addr and romsize. It was confirmed that changing the default value
> to -1 will not change the behavior by looking at occurences of rom_bar.
>
> $ git grep -w rom_bar
> hw/display/qxl.c:328:QXLRom *rom = memory_region_get_ram_ptr(&d->rom_bar);
> hw/display/qxl.c:431:qxl_set_dirty(&qxl->rom_bar, 0, qxl->rom_size);
> hw/display/qxl.c:1048:QXLRom *rom = 
> memory_region_get_ram_ptr(&qxl->rom_bar);
> hw/display/qxl.c:2131:memory_region_init_rom(&qxl->rom_bar, OBJECT(qxl), 
> "qxl.vrom",
> hw/display/qxl.c:2154: PCI_BASE_ADDRESS_SPACE_MEMORY, &qxl->rom_bar);
> hw/display/qxl.h:101:MemoryRegion   rom_bar;
> hw/pci/pci.c:74:DEFINE_PROP_UINT32("rombar",  PCIDevice, rom_bar, 1),
> hw/pci/pci.c:2329:if (!pdev->rom_bar) {
> hw/vfio/pci.c:1019:if (vdev->pdev.romfile || !vdev->pdev.rom_bar) {
> hw/xen/xen_pt_load_rom.c:29:if (dev->romfile || !dev->rom_bar) {
> include/hw/pci/pci_device.h:150:uint32_t rom_bar;
>
> rom_bar refers to a different variable in qxl. It is only tested if
> the value is 0 or not in the other places.

Makes me wonder why it's uint32_t.  Not this patch's problem.

> Signed-off-by: Akihiko Odaki 
> ---
>  hw/pci/pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 54b375da2d26..909c5b3ee4ee 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -71,7 +71,7 @@ static Property pci_props[] = {
>  DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
>  DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
>  DEFINE_PROP_UINT32("romsize", PCIDevice, romsize, -1),
> -DEFINE_PROP_UINT32("rombar",  PCIDevice, rom_bar, 1),
> +DEFINE_PROP_UINT32("rombar",  PCIDevice, rom_bar, -1),
>  DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
>  QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false),
>  DEFINE_PROP_BIT("x-pcie-lnksta-dllla", PCIDevice, cap_present,