Re: [PATCH] quorum: Remove unnecessary forward declaration

[Philippe Mathieu-Daudé via]

On 6/10/22 14:26, Kevin Wolf wrote:

Signed-off-by: Kevin Wolf 
---
  block/quorum.c | 2 --
  1 file changed, 2 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 04/13] hw/ppc/e500: Reduce usage of sysbus API

[Philippe Mathieu-Daudé via]

On 3/10/22 22:31, Bernhard Beschow wrote:

PlatformBusDevice has an mmio attribute which gets aliased to
SysBusDevice::mmio[0]. So PlatformbusDevice::mmio can be used directly,
avoiding the sysbus API.

Signed-off-by: Bernhard Beschow 
---
  hw/ppc/e500.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 00/13] ppc/e500: Add support for two types of flash, cleanup

[Philippe Mathieu-Daudé via]

Hi Daniel,

On 3/10/22 22:31, Bernhard Beschow wrote:

Cover letter:
~

This series adds support for -pflash and direct SD card access to the
PPC e500 boards. The idea is to increase compatibility with "real" firmware
images where only the bare minimum of drivers is compiled in.



Bernhard Beschow (13):
   hw/ppc/meson: Allow e500 boards to be enabled separately
   hw/gpio/meson: Introduce dedicated config switch for hw/gpio/mpc8xxx
   docs/system/ppc/ppce500: Add heading for networking chapter
   hw/ppc/e500: Reduce usage of sysbus API
   hw/ppc/mpc8544ds: Rename wrongly named method
   hw/ppc/mpc8544ds: Add platform bus
   hw/ppc/e500: Remove if statement which is now always true


This first part is mostly reviewed and can already go via your
ppc-next queue.


   hw/block/pflash_cfi01: Error out if device length isn't a power of two
   hw/ppc/e500: Implement pflash handling
   hw/sd/sdhci-internal: Unexport ESDHC defines
   hw/sd/sdhci: Rename ESDHC_* defines to USDHC_*
   hw/sd/sdhci: Implement Freescale eSDHC device model
   hw/ppc/e500: Add Freescale eSDHC to e500 boards


This second part still need work. I can take it via the sdmmc-next
queue.

Regards,

Phil.

Re: [PATCH v2 07/13] hw/ppc/e500: Remove if statement which is now always true

[Philippe Mathieu-Daudé via]

On 3/10/22 22:31, Bernhard Beschow wrote:

Now that the MPC8544DS board also has a platform bus, the if statement
is always true.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Bin Meng 
Reviewed-by: Philippe Mathieu-Daudé 
---
  hw/ppc/e500.c  | 30 ++
  hw/ppc/e500.h  |  1 -
  hw/ppc/e500plat.c  |  1 -
  hw/ppc/mpc8544ds.c |  1 -
  4 files changed, 14 insertions(+), 19 deletions(-)



  /* Platform Bus Device */
-if (pmc->has_platform_bus) {
-dev = qdev_new(TYPE_PLATFORM_BUS_DEVICE);
-dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE);
-qdev_prop_set_uint32(dev, "num_irqs", pmc->platform_bus_num_irqs);
-qdev_prop_set_uint32(dev, "mmio_size", pmc->platform_bus_size);
-sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-pms->pbus_dev = PLATFORM_BUS_DEVICE(dev);
-
-s = SYS_BUS_DEVICE(pms->pbus_dev);
-for (i = 0; i < pmc->platform_bus_num_irqs; i++) {
-int irqn = pmc->platform_bus_first_irq + i;
-sysbus_connect_irq(s, i, qdev_get_gpio_in(mpicdev, irqn));
-}
+dev = qdev_new(TYPE_PLATFORM_BUS_DEVICE);
+dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE);
+qdev_prop_set_uint32(dev, "num_irqs", pmc->platform_bus_num_irqs);
+qdev_prop_set_uint32(dev, "mmio_size", pmc->platform_bus_size);
+sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);


Should we abort(pmc->platform_bus_size > 0) now?


+pms->pbus_dev = PLATFORM_BUS_DEVICE(dev);
  
-memory_region_add_subregion(address_space_mem,

-pmc->platform_bus_base,
-&pms->pbus_dev->mmio);
+s = SYS_BUS_DEVICE(pms->pbus_dev);
+for (i = 0; i < pmc->platform_bus_num_irqs; i++) {
+int irqn = pmc->platform_bus_first_irq + i;
+sysbus_connect_irq(s, i, qdev_get_gpio_in(mpicdev, irqn));
  }
  
+memory_region_add_subregion(address_space_mem,

+pmc->platform_bus_base,
+&pms->pbus_dev->mmio);

Re: [PATCH v2 13/13] hw/ppc/e500: Add Freescale eSDHC to e500 boards

[Philippe Mathieu-Daudé via]

On 3/10/22 22:31, Bernhard Beschow wrote:

Adds missing functionality to emulated e500 SOCs which increases the
chance of given "real" firmware images to access SD cards.

Signed-off-by: Bernhard Beschow 
---
  docs/system/ppc/ppce500.rst | 13 +
  hw/ppc/Kconfig  |  1 +
  hw/ppc/e500.c   | 31 ++-
  3 files changed, 44 insertions(+), 1 deletion(-)



+static void dt_sdhc_create(void *fdt, const char *parent, const char *mpic)
+{
+hwaddr mmio = MPC85XX_ESDHC_REGS_OFFSET;
+hwaddr size = MPC85XX_ESDHC_REGS_SIZE;
+int irq = MPC85XX_ESDHC_IRQ;


Why not pass these 3 variable as argument?


+g_autofree char *name = NULL;
+
+name = g_strdup_printf("%s/sdhc@%" PRIx64, parent, mmio);
+qemu_fdt_add_subnode(fdt, name);
+qemu_fdt_setprop(fdt, name, "sdhci,auto-cmd12", NULL, 0);
+qemu_fdt_setprop_phandle(fdt, name, "interrupt-parent", mpic);
+qemu_fdt_setprop_cells(fdt, name, "bus-width", 4);
+qemu_fdt_setprop_cells(fdt, name, "interrupts", irq, 0x2);
+qemu_fdt_setprop_cells(fdt, name, "reg", mmio, size);
+qemu_fdt_setprop_string(fdt, name, "compatible", "fsl,esdhc");
+}
  
  typedef struct PlatformDevtreeData {

  void *fdt;
@@ -553,6 +573,8 @@ static int ppce500_load_device_tree(PPCE500MachineState 
*pms,
  
  dt_rtc_create(fdt, "i2c", "rtc");
  
+/* sdhc */

+dt_sdhc_create(fdt, soc, mpic);
  

Re: [PATCH v2 08/13] hw/block/pflash_cfi01: Error out if device length isn't a power of two

[Philippe Mathieu-Daudé via]

On 3/10/22 22:31, Bernhard Beschow wrote:

According to the JEDEC standard the device length is communicated to an
OS as an exponent (power of two).

Signed-off-by: Bernhard Beschow 
Reviewed-by: Bin Meng 
---
  hw/block/pflash_cfi01.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)


With CFI02 similarly fixed:
Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 09/13] hw/ppc/e500: Implement pflash handling

[Philippe Mathieu-Daudé via]

On 3/10/22 22:31, Bernhard Beschow wrote:

Allows e500 boards to have their root file system reside on flash using
only builtin devices located in the eLBC memory region.

Note that the flash memory area is only created when a -pflash argument is
given, and that the size is determined by the given file. The idea is to
put users into control.

Signed-off-by: Bernhard Beschow 
---
  docs/system/ppc/ppce500.rst | 12 ++
  hw/ppc/Kconfig  |  1 +
  hw/ppc/e500.c   | 76 +
  3 files changed, 89 insertions(+)



@@ -856,6 +892,7 @@ void ppce500_init(MachineState *machine)
  unsigned int pci_irq_nrs[PCI_NUM_PINS] = {1, 2, 3, 4};
  IrqLines *irqs;
  DeviceState *dev, *mpicdev;
+DriveInfo *dinfo;
  CPUPPCState *firstenv = NULL;
  MemoryRegion *ccsr_addr_space;
  SysBusDevice *s;
@@ -1024,6 +1061,45 @@ void ppce500_init(MachineState *machine)
  pmc->platform_bus_base,
  &pms->pbus_dev->mmio);
  
+dinfo = drive_get(IF_PFLASH, 0, 0);

+if (dinfo) {
+BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
+BlockDriverState *bs = blk_bs(blk);
+uint64_t size = bdrv_getlength(bs);
+uint64_t mmio_size = pms->pbus_dev->mmio.size;
+uint32_t sector_len = 64 * KiB;
+
+if (ctpop64(size) != 1) {
+error_report("Size of pflash file must be a power of two.");


This is a PFLASH restriction (which you already fixed in the previous
patch), not a board one.


+exit(1);
+}
+
+if (size > mmio_size) {
+error_report("Size of pflash file must not be bigger than %" PRIu64
+ " bytes.", mmio_size);


There is no hardware limitation here, you can wire flash bigger than the
memory aperture. What is above the aperture will simply be ignored.

Should we display a warning here instead of a fatal error?


+exit(1);
+}
+
+assert(QEMU_IS_ALIGNED(size, sector_len));


Similarly, this doesn't seem a problem the board code should worry
about: better to defer it to PFLASH realize().


+dev = qdev_new(TYPE_PFLASH_CFI01);
+qdev_prop_set_drive(dev, "drive", blk);
+qdev_prop_set_uint32(dev, "num-blocks", size / sector_len);
+qdev_prop_set_uint64(dev, "sector-length", sector_len);
+qdev_prop_set_uint8(dev, "width", 2);
+qdev_prop_set_bit(dev, "big-endian", true);
+qdev_prop_set_uint16(dev, "id0", 0x89);
+qdev_prop_set_uint16(dev, "id1", 0x18);
+qdev_prop_set_uint16(dev, "id2", 0x);
+qdev_prop_set_uint16(dev, "id3", 0x0);
+qdev_prop_set_string(dev, "name", "e500.flash");
+s = SYS_BUS_DEVICE(dev);
+sysbus_realize_and_unref(s, &error_fatal);
+
+memory_region_add_subregion(&pms->pbus_dev->mmio, 0,
+sysbus_mmio_get_region(s, 0));
+}
+
  /*
   * Smart firmware defaults ahead!
   *

Re: [PATCH v2 12/13] hw/sd/sdhci: Implement Freescale eSDHC device model

[Philippe Mathieu-Daudé via]

On 3/10/22 22:31, Bernhard Beschow wrote:

Will allow e500 boards to access SD cards using just their own devices.

Signed-off-by: Bernhard Beschow 
---
  hw/sd/sdhci.c | 147 +-
  include/hw/sd/sdhci.h |   3 +
  2 files changed, 149 insertions(+), 1 deletion(-)



+/* --- qdev Freescale eSDHC --- */
+
+/* Watermark Level Register */
+#define ESDHC_WML0x44
+
+/* Host Controller Capabilities Register 2 */
+#define ESDHC_CAPABILITIES_10x114


Not used?


+
+/* Control Register for DMA transfer */
+#define ESDHC_DMA_SYSCTL0x40c
+
+#define ESDHC_REGISTERS_MAP_SIZE0x410
+
+static uint64_t esdhci_read(void *opaque, hwaddr offset, unsigned size)
+{
+uint64_t ret;
+
+if (size != 4) {
+qemu_log_mask(LOG_GUEST_ERROR, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
+  " wrong size\n", size, offset);
+return 0;
+}
+
+if (offset & 0x3) {
+qemu_log_mask(LOG_GUEST_ERROR, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
+  " unaligned\n", size, offset);
+return 0;


Isn't it already enforced by esdhc_mmio_ops.valid.unaligned = false?


+}
+
+switch (offset) {
+case SDHC_SYSAD:
+case SDHC_BLKSIZE:
+case SDHC_ARGUMENT:
+case SDHC_TRNMOD:
+case SDHC_RSPREG0:
+case SDHC_RSPREG1:
+case SDHC_RSPREG2:
+case SDHC_RSPREG3:
+case SDHC_BDATA:
+case SDHC_PRNSTS:
+case SDHC_HOSTCTL:
+case SDHC_CLKCON:
+case SDHC_NORINTSTS:
+case SDHC_NORINTSTSEN:
+case SDHC_NORINTSIGEN:
+case SDHC_ACMD12ERRSTS:
+case SDHC_CAPAB:
+case SDHC_SLOT_INT_STATUS:
+ret = sdhci_read(opaque, offset, size);
+break;
+
+case ESDHC_WML:
+case ESDHC_DMA_SYSCTL:
+ret = 0;
+qemu_log_mask(LOG_UNIMP, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
+  " not implemented\n", size, offset);
+break;
+
+default:
+ret = 0;
+qemu_log_mask(LOG_GUEST_ERROR, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
+  " unknown offset\n", size, offset);
+break;
+}
+
+return ret;
+}
+
+static void esdhci_write(void *opaque, hwaddr offset, uint64_t val,
+ unsigned size)
+{
+if (size != 4) {
+qemu_log_mask(LOG_GUEST_ERROR, "ESDHC wr_%ub @0x%02" HWADDR_PRIx
+  " <- 0x%08lx wrong size\n", size, offset, val);
+return;
+}
+
+if (offset & 0x3) {
+qemu_log_mask(LOG_GUEST_ERROR, "ESDHC wr_%ub @0x%02" HWADDR_PRIx
+  " <- 0x%08lx unaligned\n", size, offset, val);
+return;
+}
+
+switch (offset) {
+case SDHC_SYSAD:
+case SDHC_BLKSIZE:
+case SDHC_ARGUMENT:
+case SDHC_TRNMOD:
+case SDHC_BDATA:
+case SDHC_HOSTCTL:
+case SDHC_CLKCON:
+case SDHC_NORINTSTS:
+case SDHC_NORINTSTSEN:
+case SDHC_NORINTSIGEN:
+case SDHC_FEAER:
+sdhci_write(opaque, offset, val, size);
+break;
+
+case ESDHC_WML:
+case ESDHC_DMA_SYSCTL:
+qemu_log_mask(LOG_UNIMP, "ESDHC wr_%ub @0x%02" HWADDR_PRIx " <- 0x%08lx 
"
+  "not implemented\n", size, offset, val);
+break;
+
+default:
+qemu_log_mask(LOG_GUEST_ERROR, "ESDHC wr_%ub @0x%02" HWADDR_PRIx
+  " <- 0x%08lx unknown offset\n", size, offset, val);
+break;
+}
+}
+
+static const MemoryRegionOps esdhc_mmio_ops = {
+.read = esdhci_read,
+.write = esdhci_write,
+.valid = {
+.min_access_size = 1,
+.max_access_size = 4,
+.unaligned = false
+},
+.endianness = DEVICE_BIG_ENDIAN,
+};

Re: [PATCH v2 05/13] hw/ppc/mpc8544ds: Rename wrongly named method

[Philippe Mathieu-Daudé via]

On 3/10/22 22:31, Bernhard Beschow wrote:

Signed-off-by: Bernhard Beschow 
---
  hw/ppc/mpc8544ds.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2] block: Refactor get_tmp_filename()

[Philippe Mathieu-Daudé via]

On 24/9/22 13:45, Bin Meng wrote:

From: Bin Meng 

At present there are two callers of get_tmp_filename() and they are
inconsistent.

One does:

 /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
 char *tmp_filename = g_malloc0(PATH_MAX + 1);
 ...
 ret = get_tmp_filename(tmp_filename, PATH_MAX + 1);

while the other does:

 s->qcow_filename = g_malloc(PATH_MAX);
 ret = get_tmp_filename(s->qcow_filename, PATH_MAX);

As we can see different 'size' arguments are passed. There are also
platform specific implementations inside the function, and this use
of snprintf is really undesirable.

Refactor this routine by changing its signature to:

 char *get_tmp_filename(void)

and use g_file_open_tmp() for a consistent implementation.

Signed-off-by: Bin Meng 
---

Changes in v2:
- Use g_autofree and g_steal_pointer

  include/block/block_int-common.h |  2 +-
  block.c  | 42 ++--
  block/vvfat.c|  8 +++---
  3 files changed, 18 insertions(+), 34 deletions(-)

diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 8947abab76..ea69a9349c 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -1230,7 +1230,7 @@ static inline BlockDriverState *child_bs(BdrvChild *child)
  }
  
  int bdrv_check_request(int64_t offset, int64_t bytes, Error **errp);

-int get_tmp_filename(char *filename, int size);
+char *get_tmp_filename(void);
  void bdrv_parse_filename_strip_prefix(const char *filename, const char 
*prefix,
QDict *options);
  
diff --git a/block.c b/block.c

index bc85f46eed..4e7a795566 100644
--- a/block.c
+++ b/block.c
@@ -860,38 +860,23 @@ int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry 
*geo)
  
  /*

   * Create a uniquely-named empty temporary file.
- * Return 0 upon success, otherwise a negative errno value.
+ * Return the actual name used upon success, otherwise NULL.
+ * The called function is responsible for freeing it.


s/called/caller/.


   */
-int get_tmp_filename(char *filename, int size)
+char *get_tmp_filename(void)
  {
-#ifdef _WIN32
-char temp_dir[MAX_PATH];
-/* GetTempFileName requires that its output buffer (4th param)
-   have length MAX_PATH or greater.  */
-assert(size >= MAX_PATH);
-return (GetTempPath(MAX_PATH, temp_dir)
-&& GetTempFileName(temp_dir, "qem", 0, filename)
-? 0 : -GetLastError());
-#else
+g_autofree char *filename = NULL;
  int fd;
-const char *tmpdir;
-tmpdir = getenv("TMPDIR");
-if (!tmpdir) {
-tmpdir = "/var/tmp";
-}
-if (snprintf(filename, size, "%s/vl.XX", tmpdir) >= size) {
-return -EOVERFLOW;
-}
-fd = mkstemp(filename);
+
+fd = g_file_open_tmp("vl.XX", &filename, NULL);
  if (fd < 0) {
-return -errno;
+return NULL;
  }
  if (close(fd) != 0) {
  unlink(filename);
-return -errno;
+return NULL;
  }
-return 0;
-#endif
+return g_steal_pointer(&filename);
  }



diff --git a/block/vvfat.c b/block/vvfat.c
index d6dd919683..135fafb166 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3146,10 +3146,10 @@ static int enable_write_target(BlockDriverState *bs, 
Error **errp)
  
  array_init(&(s->commits), sizeof(commit_t));
  
-s->qcow_filename = g_malloc(PATH_MAX);

-ret = get_tmp_filename(s->qcow_filename, PATH_MAX);
-if (ret < 0) {
-error_setg_errno(errp, -ret, "can't create temporary file");
+s->qcow_filename = get_tmp_filename();
+if (!s->qcow_filename) {
+error_setg_errno(errp, errno, "can't create temporary file");
+ret = -errno;


I don't think we can reuse errno here. Wouldn't it be better to pass a 
Error* to get_tmp_filename, and use the error parameter to 
g_file_open_tmp()? That would match our Error* style use.



  goto err;
  }
  

Re: [PATCH v1 01/10] monitor: expose monitor_puts to rest of code

[Philippe Mathieu-Daudé via]

On 21/9/22 18:07, Alex Bennée wrote:

This helps us construct strings elsewhere before echoing to the
monitor. It avoids having to jump through hoops like:

   monitor_printf(mon, "%s", s->str);

It will be useful in following patches but for now convert all
existing plain "%s" printfs to use the _puts api.

Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 

---
v2
   - s/monitor_printf(mon, "%s"/monitor_puts(mon, /
---
  docs/devel/writing-monitor-commands.rst |  2 +-
  include/monitor/monitor.h   |  1 +
  monitor/monitor-internal.h  |  1 -
  block/monitor/block-hmp-cmds.c  | 10 +-
  hw/misc/mos6522.c   |  2 +-
  monitor/hmp-cmds.c  |  8 
  monitor/hmp.c   |  2 +-
  target/i386/helper.c|  2 +-
  8 files changed, 14 insertions(+), 14 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 5/9] exec/address-spaces: Wrap address space singletons into functions

[Philippe Mathieu-Daudé via]

On 20/9/22 01:17, Bernhard Beschow wrote:

In the next steps, these singletons will be resolved by turning them
into attributes of the system bus. The system bus is already accessible
via the global current_machine variable which will be made use of later
in the wrapper functions.

All changes have been performed with search-and-replace:
* s/&address_space_memory/get_address_space_memory()/
* s/&address_space_io/get_address_space_io()/
The only exceptions were exec/address-spaces.h and softmmu/physmem.c
which have been manually changed.

Signed-off-by: Bernhard Beschow 
---
  accel/hvf/hvf-accel-ops.c|  2 +-
  accel/kvm/kvm-all.c  | 12 ++--
  hw/alpha/dp264.c |  4 ++--
  hw/alpha/typhoon.c   |  4 ++--
  hw/arm/smmu-common.c |  4 ++--
  hw/arm/smmuv3.c  | 14 +++---
  hw/arm/virt.c|  2 +-
  hw/char/goldfish_tty.c   |  4 ++--
  hw/core/loader.c |  2 +-
  hw/dma/pl330.c   |  2 +-
  hw/dma/rc4030.c  |  2 +-
  hw/dma/xlnx-zynq-devcfg.c|  4 ++--
  hw/dma/xlnx_dpdma.c  |  8 
  hw/hppa/machine.c|  4 ++--
  hw/hyperv/hyperv.c   |  2 +-
  hw/hyperv/vmbus.c|  2 +-
  hw/i386/amd_iommu.c  | 18 +-
  hw/i386/fw_cfg.c |  2 +-
  hw/i386/intel_iommu.c| 24 
  hw/i386/microvm.c|  4 ++--
  hw/i386/pc.c |  2 +-
  hw/i386/xen/xen-hvm.c|  4 ++--
  hw/ide/ahci.c|  2 +-
  hw/ide/macio.c   | 10 +-
  hw/intc/apic.c   |  2 +-
  hw/intc/openpic_kvm.c|  2 +-
  hw/intc/pnv_xive.c   |  6 +++---
  hw/intc/pnv_xive2.c  |  6 +++---
  hw/intc/riscv_aplic.c|  2 +-
  hw/intc/spapr_xive.c |  2 +-
  hw/intc/xive.c   |  4 ++--
  hw/intc/xive2.c  |  4 ++--
  hw/mips/jazz.c   |  4 ++--
  hw/misc/lasi.c   |  2 +-
  hw/misc/macio/mac_dbdma.c|  8 
  hw/net/ftgmac100.c   | 16 
  hw/net/i82596.c  | 24 
  hw/net/imx_fec.c | 22 +++---
  hw/net/lasi_i82596.c |  2 +-
  hw/net/npcm7xx_emc.c | 14 +++---
  hw/openrisc/boot.c   |  2 +-
  hw/pci-host/dino.c   |  6 +++---
  hw/pci-host/pnv_phb3.c   |  6 +++---
  hw/pci-host/pnv_phb3_msi.c   |  6 +++---
  hw/pci-host/pnv_phb4.c   | 10 +-
  hw/pci/pci.c |  2 +-
  hw/ppc/pnv_psi.c |  2 +-
  hw/ppc/spapr.c   |  4 ++--
  hw/ppc/spapr_events.c|  2 +-
  hw/ppc/spapr_hcall.c |  4 ++--
  hw/ppc/spapr_iommu.c |  4 ++--
  hw/ppc/spapr_ovec.c  |  8 
  hw/ppc/spapr_rtas.c  |  2 +-
  hw/remote/iommu.c|  2 +-
  hw/remote/message.c  |  4 ++--
  hw/remote/proxy-memory-listener.c|  2 +-
  hw/riscv/boot.c  |  6 +++---
  hw/riscv/sifive_e.c  |  2 +-
  hw/riscv/sifive_u.c  |  2 +-
  hw/riscv/virt.c  |  2 +-
  hw/s390x/css.c   | 16 
  hw/s390x/ipl.h   |  2 +-
  hw/s390x/s390-pci-bus.c  |  4 ++--
  hw/s390x/s390-pci-inst.c | 10 +-
  hw/s390x/s390-skeys.c|  2 +-
  hw/s390x/virtio-ccw.c| 10 +-
  hw/sd/sdhci.c|  2 +-
  hw/sh4/r2d.c |  4 ++--
  hw/sparc/sun4m.c |  2 +-
  hw/sparc/sun4m_iommu.c   |  4 ++--
  hw/sparc64/sun4u_iommu.c |  4 ++--
  hw/timer/hpet.c  |  2 +-
  hw/usb/hcd-ehci-pci.c|  2 +-
  hw/usb/hcd-ehci-sysbus.c |  2 +-
  hw/usb/hcd-ohci.c|  2 +-
  hw/usb/hcd-xhci-sysbus.c |  2 +-
  hw/vfio/ap.c |  2 +-
  hw/vfio/ccw.c|  2 +-
  hw/vfio/common.c |  8 
  hw/vfio/platform.c   |  2 +-
  hw/virtio/vhost-vdpa.c   |  2 +-
  hw/virtio/vhost.c|  2 +-
  hw/virtio/virtio-bus.c   |  4 ++--
  hw/virtio/virtio-iommu.c |  6 +++---
  hw/virtio/virtio-pci.c   |  2 +-
  hw/xen/xen_pt.c  |  4 ++--
  include/exec/address-spaces.h|  4 ++--
  include/hw/elf_ops.h |  4 ++--
  include

Re: [PATCH 9/9] exec/address-spaces: Inline legacy functions

[Philippe Mathieu-Daudé via]

On 20/9/22 07:15, Philippe Mathieu-Daudé wrote:

On 20/9/22 01:17, Bernhard Beschow wrote:

The functions just access a global pointer and perform some pointer
arithmetic on top. Allow the compiler to see through this by inlining.


I thought about this while reviewing the previous patch, ...


Signed-off-by: Bernhard Beschow 
---
  include/exec/address-spaces.h | 30 ++
  softmmu/physmem.c | 28 
  2 files changed, 26 insertions(+), 32 deletions(-)

diff --git a/include/exec/address-spaces.h 
b/include/exec/address-spaces.h

index b31bd8dcf0..182af27cad 100644
--- a/include/exec/address-spaces.h
+++ b/include/exec/address-spaces.h
@@ -23,29 +23,51 @@
  #ifndef CONFIG_USER_ONLY
+#include "hw/boards.h"


... but I'm not a fan of including this header here. It is restricted to 
system emulation, but still... Let see what the others think.



  /**
   * Get the root memory region.  This is a legacy function, provided for
   * compatibility. Prefer using SysBusState::system_memory directly.
   */
-MemoryRegion *get_system_memory(void);
+inline MemoryRegion *get_system_memory(void)
+{
+    assert(current_machine);
+
+    return ¤t_machine->main_system_bus.system_memory;
+}


Maybe we can simply declare them with __attribute__ ((const)) in the 
previous patch?
See 
https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes

Re: [PATCH 9/9] exec/address-spaces: Inline legacy functions

[Philippe Mathieu-Daudé via]

On 20/9/22 01:17, Bernhard Beschow wrote:

The functions just access a global pointer and perform some pointer
arithmetic on top. Allow the compiler to see through this by inlining.


I thought about this while reviewing the previous patch, ...


Signed-off-by: Bernhard Beschow 
---
  include/exec/address-spaces.h | 30 ++
  softmmu/physmem.c | 28 
  2 files changed, 26 insertions(+), 32 deletions(-)

diff --git a/include/exec/address-spaces.h b/include/exec/address-spaces.h
index b31bd8dcf0..182af27cad 100644
--- a/include/exec/address-spaces.h
+++ b/include/exec/address-spaces.h
@@ -23,29 +23,51 @@
  
  #ifndef CONFIG_USER_ONLY
  
+#include "hw/boards.h"


... but I'm not a fan of including this header here. It is restricted to 
system emulation, but still... Let see what the others think.



  /**
   * Get the root memory region.  This is a legacy function, provided for
   * compatibility. Prefer using SysBusState::system_memory directly.
   */
-MemoryRegion *get_system_memory(void);
+inline MemoryRegion *get_system_memory(void)
+{
+assert(current_machine);
+
+return ¤t_machine->main_system_bus.system_memory;
+}
  
  /**

   * Get the root I/O port region.  This is a legacy function, provided for
   * compatibility. Prefer using SysBusState::system_io directly.
   */
-MemoryRegion *get_system_io(void);
+inline MemoryRegion *get_system_io(void)
+{
+assert(current_machine);
+
+return ¤t_machine->main_system_bus.system_io;
+}
  
  /**

   * Get the root memory address space.  This is a legacy function, provided for
   * compatibility. Prefer using SysBusState::address_space_memory directly.
   */
-AddressSpace *get_address_space_memory(void);
+inline AddressSpace *get_address_space_memory(void)
+{
+assert(current_machine);
+
+return ¤t_machine->main_system_bus.address_space_memory;
+}
  
  /**

   * Get the root I/O port address space.  This is a legacy function, provided
   * for compatibility. Prefer using SysBusState::address_space_io directly.
   */
-AddressSpace *get_address_space_io(void);
+inline AddressSpace *get_address_space_io(void)
+{
+assert(current_machine);
+
+return ¤t_machine->main_system_bus.address_space_io;
+}
  
  #endif
  
diff --git a/softmmu/physmem.c b/softmmu/physmem.c

index 07e9a9171c..dce088f55c 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2674,34 +2674,6 @@ static void memory_map_init(SysBusState *sysbus)
  address_space_init(&sysbus->address_space_io, system_io, "I/O");
  }
  
-MemoryRegion *get_system_memory(void)

-{
-assert(current_machine);
-
-return ¤t_machine->main_system_bus.system_memory;
-}
-
-MemoryRegion *get_system_io(void)
-{
-assert(current_machine);
-
-return ¤t_machine->main_system_bus.system_io;
-}
-
-AddressSpace *get_address_space_memory(void)
-{
-assert(current_machine);
-
-return ¤t_machine->main_system_bus.address_space_memory;
-}
-
-AddressSpace *get_address_space_io(void)
-{
-assert(current_machine);
-
-return ¤t_machine->main_system_bus.address_space_io;
-}
-
  static void invalidate_and_set_dirty(MemoryRegion *mr, hwaddr addr,
   hwaddr length)
  {

Re: [PATCH 8/9] softmmu/physmem: Let SysBusState absorb memory region and address space singletons

[Philippe Mathieu-Daudé via]

On 20/9/22 01:17, Bernhard Beschow wrote:

These singletons are actually properties of the system bus but so far it
hasn't been modelled that way. Fix this to make this relationship very
obvious.

The idea of the patch is to restrain futher proliferation of the use of
get_system_memory() and get_system_io() which are "temprary interfaces"


"further", "temporary"


"until a proper bus interface is available". This should now be the
case.

Note that the new attributes are values rather than a pointers. This
trades pointer dereferences for pointer arithmetic. The idea is to
reduce cache misses - a rule of thumb says that every pointer
dereference causes a cache miss while arithmetic is basically free.

Signed-off-by: Bernhard Beschow 
---
  include/exec/address-spaces.h | 19 ---
  include/hw/sysbus.h   |  6 +
  softmmu/physmem.c | 46 ++-
  3 files changed, 45 insertions(+), 26 deletions(-)

diff --git a/include/exec/address-spaces.h b/include/exec/address-spaces.h
index d5c8cbd718..b31bd8dcf0 100644
--- a/include/exec/address-spaces.h
+++ b/include/exec/address-spaces.h
@@ -23,17 +23,28 @@
  
  #ifndef CONFIG_USER_ONLY
  
-/* Get the root memory region.  This interface should only be used temporarily

- * until a proper bus interface is available.
+/**
+ * Get the root memory region.  This is a legacy function, provided for
+ * compatibility. Prefer using SysBusState::system_memory directly.
   */
  MemoryRegion *get_system_memory(void);



diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index 5bb3b88501..516e9091dc 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -17,6 +17,12 @@ struct SysBusState {
  /*< private >*/
  BusState parent_obj;
  /*< public >*/
+
+MemoryRegion system_memory;
+MemoryRegion system_io;
+
+AddressSpace address_space_io;
+AddressSpace address_space_memory;


Alternatively (renaming doc accordingly):

   struct {
   MemoryRegion mr;
   AddressSpace as;
   } io, memory;


  };
  
  #define TYPE_SYS_BUS_DEVICE "sys-bus-device"

diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 0ac920d446..07e9a9171c 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -86,12 +86,6 @@
   */
  RAMList ram_list = { .blocks = QLIST_HEAD_INITIALIZER(ram_list.blocks) };
  
-static MemoryRegion *system_memory;

-static MemoryRegion *system_io;
-
-static AddressSpace address_space_io;
-static AddressSpace address_space_memory;
-
  static MemoryRegion io_mem_unassigned;
  
  typedef struct PhysPageEntry PhysPageEntry;

@@ -146,7 +140,7 @@ typedef struct subpage_t {
  #define PHYS_SECTION_UNASSIGNED 0
  
  static void io_mem_init(void);

-static void memory_map_init(void);
+static void memory_map_init(SysBusState *sysbus);
  static void tcg_log_global_after_sync(MemoryListener *listener);
  static void tcg_commit(MemoryListener *listener);
  
@@ -2667,37 +2661,45 @@ static void tcg_commit(MemoryListener *listener)

  tlb_flush(cpuas->cpu);
  }
  
-static void memory_map_init(void)

+static void memory_map_init(SysBusState *sysbus)
  {


No need to pass a singleton by argument.

   assert(current_machine);

You can use get_system_memory() and get_system_io() in place :)

LGTM otherwise, great!


-system_memory = g_malloc(sizeof(*system_memory));
+MemoryRegion *system_memory = &sysbus->system_memory;
+MemoryRegion *system_io = &sysbus->system_io;
  
  memory_region_init(system_memory, NULL, "system", UINT64_MAX);

-address_space_init(&address_space_memory, system_memory, "memory");
+address_space_init(&sysbus->address_space_memory, system_memory, "memory");
  
-system_io = g_malloc(sizeof(*system_io));

  memory_region_init_io(system_io, NULL, &unassigned_io_ops, NULL, "io",
65536);
-address_space_init(&address_space_io, system_io, "I/O");
+address_space_init(&sysbus->address_space_io, system_io, "I/O");
  }
  
  MemoryRegion *get_system_memory(void)

  {
-return system_memory;
+assert(current_machine);
+
+return ¤t_machine->main_system_bus.system_memory;
  }
  
  MemoryRegion *get_system_io(void)

  {
-return system_io;
+assert(current_machine);
+
+return ¤t_machine->main_system_bus.system_io;
  }
  
  AddressSpace *get_address_space_memory(void)

  {
-return &address_space_memory;
+assert(current_machine);
+
+return ¤t_machine->main_system_bus.address_space_memory;
  }
  
  AddressSpace *get_address_space_io(void)

  {
-return &address_space_io;
+assert(current_machine);
+
+return ¤t_machine->main_system_bus.address_space_io;
  }

Re: [PATCH 6/9] target/loongarch/cpu: Remove unneeded include directive

[Philippe Mathieu-Daudé via]

On 20/9/22 01:17, Bernhard Beschow wrote:

The cpu is used in both user and system emulation context while sysbus.h
is system-only. Remove it since it's not needed anyway. Furthermore, it
would cause a compile error in the next commit.

Signed-off-by: Bernhard Beschow 
---
  target/loongarch/cpu.h | 1 -
  1 file changed, 1 deletion(-)

diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
index dce999aaac..c9ed2cb3e7 100644
--- a/target/loongarch/cpu.h
+++ b/target/loongarch/cpu.h
@@ -13,7 +13,6 @@
  #include "hw/registerfields.h"
  #include "qemu/timer.h"
  #include "exec/memory.h"
-#include "hw/sysbus.h"
  
  #define IOCSRF_TEMP 0

  #define IOCSRF_NODECNT  1


Renaming the subject as 'target: Remove unneeded "hw/sysbus.h" include 
directive' and fixing target/ppc/kvm.c:

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 3/9] hw/core/sysbus: Resolve main_system_bus singleton

[Philippe Mathieu-Daudé via]

On 20/9/22 01:17, Bernhard Beschow wrote:

In QEMU, a machine and the main_system_bus always go togehter. Usually
the bus is part of the machine which suggsts to host it there.


"together", "suggests"


Since tere is already a current_machine singleton, all code that
accesses the main_system_bus can be changed (behind the scenes) to go
through current_machine. This resolves a singleton. Futhermore, by


"Furthermore"


reifying it in code, the every-machine-has-exactly-one-main-system-bus
relationship becomes very obvious.

Note that the main_system_bus attribute is a value rather than a
pointer. This trades pointer dereferences for pointer arithmetic. The
idea is to reduce cache misses - a rule of thumb says that
every pointer dereference causes a cache miss while arithmetic is
basically free.

Signed-off-by: Bernhard Beschow 
---
  hw/core/bus.c   |  5 -
  hw/core/machine.c   |  3 +++
  hw/core/sysbus.c| 22 +-
  include/hw/boards.h |  1 +
  4 files changed, 13 insertions(+), 18 deletions(-)



diff --git a/include/hw/boards.h b/include/hw/boards.h
index 311ed17e18..7af940102d 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h


Likely missing the BusState declaration:

  #include "hw/qdev-core.h"


@@ -346,6 +346,7 @@ struct MachineState {
   */
  MemoryRegion *ram;
  DeviceMemoryState *device_memory;
+BusState main_system_bus;
  
  ram_addr_t ram_size;

  ram_addr_t maxram_size;

Re: [PATCH 2/9] exec/hwaddr.h: Add missing include

[Philippe Mathieu-Daudé via]

On 20/9/22 01:17, Bernhard Beschow wrote:

The next commit would not compile w/o the include directive.

Signed-off-by: Bernhard Beschow 
---
  include/exec/hwaddr.h | 1 +
  1 file changed, 1 insertion(+)

diff --git a/include/exec/hwaddr.h b/include/exec/hwaddr.h
index 8f16d179a8..616255317c 100644
--- a/include/exec/hwaddr.h
+++ b/include/exec/hwaddr.h
@@ -3,6 +3,7 @@
  #ifndef HWADDR_H
  #define HWADDR_H
  
+#include "qemu/osdep.h"


NAck: This is an anti-pattern. "qemu/osdep.h" must not be included
in .h, only in .c.

Isn't including "hw/qdev-core.h" in "include/hw/boards.h" enough in
the next patch?

Re: [PATCH 1/9] hw/riscv/sifive_e: Fix inheritance of SiFiveEState

[Philippe Mathieu-Daudé via]

On 20/9/22 01:17, Bernhard Beschow wrote:

SiFiveEState inherits from SysBusDevice while it's TypeInfo claims it to
inherit from TYPE_MACHINE. This is an inconsistency which can cause
undefined behavior such as memory corruption.

Change SiFiveEState to inherit from MachineState since it is registered
as a machine.

Signed-off-by: Bernhard Beschow 
---
  include/hw/riscv/sifive_e.h | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/riscv/sifive_e.h b/include/hw/riscv/sifive_e.h
index 83604da805..d738745925 100644
--- a/include/hw/riscv/sifive_e.h
+++ b/include/hw/riscv/sifive_e.h
@@ -22,6 +22,7 @@
  #include "hw/riscv/riscv_hart.h"
  #include "hw/riscv/sifive_cpu.h"
  #include "hw/gpio/sifive_gpio.h"
+#include "hw/boards.h"
  
  #define TYPE_RISCV_E_SOC "riscv.sifive.e.soc"

  #define RISCV_E_SOC(obj) \
@@ -41,7 +42,7 @@ typedef struct SiFiveESoCState {
  
  typedef struct SiFiveEState {

  /*< private >*/
-SysBusDevice parent_obj;
+MachineState parent_obj;


Ouch.

Fixes: 0869490b1c ("riscv: sifive_e: Manually define the machine")

Reviewed-by: Philippe Mathieu-Daudé 

[PATCH] block/qcow2-bitmap: Add missing cast to silent GCC error

[Philippe Mathieu-Daudé via]

Commit d1258dd0c8 ("qcow2: autoloading dirty bitmaps") added the
set_readonly_helper() GFunc handler, correctly casting the gpointer
user_data in both the g_slist_foreach() caller and the handler.
Few commits later (commit 1b6b0562db), the handler is reused in
qcow2_reopen_bitmaps_rw() but missing the gpointer cast, resulting
in the following error when using Homebrew GCC 12.2.0:

  [2/658] Compiling C object libblock.fa.p/block_qcow2-bitmap.c.o
  ../../block/qcow2-bitmap.c: In function 'qcow2_reopen_bitmaps_rw':
  ../../block/qcow2-bitmap.c:1211:60: error: incompatible type for argument 3 
of 'g_slist_foreach'
   1211 | g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, false);
|^
||
|_Bool
  In file included from 
/opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib/gmain.h:26,
   from 
/opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib/giochannel.h:33,
   from 
/opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib.h:54,
   from /Users/philmd/source/qemu/include/glib-compat.h:32,
   from /Users/philmd/source/qemu/include/qemu/osdep.h:144,
   from ../../block/qcow2-bitmap.c:28:
  /opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib/gslist.h:127:61: 
note: expected 'gpointer' {aka 'void *'} but argument is of type '_Bool'
127 |   gpointer  
user_data);
|   ~~^
  At top level:
  FAILED: libblock.fa.p/block_qcow2-bitmap.c.o

Fix by adding the missing gpointer cast.

Fixes: 1b6b0562db ("qcow2: support .bdrv_reopen_bitmaps_rw")
Signed-off-by: Philippe Mathieu-Daudé 
---
Cc: Vladimir Sementsov-Ogievskiy 
Cc: John Snow 
Cc: Max Reitz 
---
 block/qcow2-bitmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index ff3309846c..7197754843 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1208,7 +1208,7 @@ int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error 
**errp)
 }
 }
 
-g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, false);
+g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, (gpointer)false);
 ret = 0;
 
 out:
-- 
2.37.3

Re: [PATCH 00/11] ppc/e500: Add support for two types of flash, cleanup

[Philippe Mathieu-Daudé via]

On 15/9/22 17:25, Bernhard Beschow wrote:

This series adds support for -pflash and direct SD card access to the
PPC e500 boards. The idea is to increase compatibility with "real" firmware
images where only the bare minimum of drivers is compiled in.

The series is structured as follows:

Patches 1-3 perform some general cleanup which paves the way for the rest of
the series.

Patches 4-7 add -pflash handling where memory-mapped flash can be added on
user's behalf. That is, the flash memory region is only added if the -pflash
argument is supplied. Note that the cfi01 device model becomes stricter in
checking the size of the emulated flash space.

Patches 8-11 add a new device model - the Freescale eSDHC - to the e500
boards which was missing so far.

User documentation is also added as the new features become available.


Future possible cleanup: Looking at the e500 component, the MPC8544 GUTS
added in commit b0fb84236d ("PPC: E500: Implement reboot controller")
was used in hw/ppce500_mpc8544ds.c board, but in a later refactor
(commit 4a18e7c92a "PPC: e500: rename mpc8544ds into generic file")
this file got renamed as hw/ppc/e500.c; having a MPC8544 specific piece
in the common e500 seems odd.

Re: [PATCH 03/11] docs/system/ppc/ppce500: Add heading for networking chapter

[Philippe Mathieu-Daudé via]

On 15/9/22 17:25, Bernhard Beschow wrote:

The sudden change of topics is slightly confusing and makes the
networking information less visible. So separate the networking chapter
to improve comprehensibility.

Signed-off-by: Bernhard Beschow 
---
  docs/system/ppc/ppce500.rst | 3 +++
  1 file changed, 3 insertions(+)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 08/11] hw/sd/sdhci-internal: Unexport ESDHC defines

[Philippe Mathieu-Daudé via]

On 15/9/22 17:25, Bernhard Beschow wrote:

These defines aren't used outside of sdhci.c, so can be defined there.

Signed-off-by: Bernhard Beschow 
---
  hw/sd/sdhci-internal.h | 20 
  hw/sd/sdhci.c  | 19 +++
  2 files changed, 19 insertions(+), 20 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 02/11] hw/gpio/meson: Introduce dedicated config switch for hw/gpio/mpc8xxx

[Philippe Mathieu-Daudé via]

On 15/9/22 17:25, Bernhard Beschow wrote:

Having a dedicated config switch makes dependency handling cleaner.

Signed-off-by: Bernhard Beschow 
---
  hw/gpio/Kconfig | 3 +++
  hw/gpio/meson.build | 2 +-
  hw/ppc/Kconfig  | 1 +
  3 files changed, 5 insertions(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 05/11] hw/ppc/e500: Remove if statement which is now always true

[Philippe Mathieu-Daudé via]

On 15/9/22 17:25, Bernhard Beschow wrote:

Now that the MPC8544DS board also has a platform bus, the if statement
was always true.


s/was/is/.

Reviewed-by: Philippe Mathieu-Daudé 



Signed-off-by: Bernhard Beschow 
---
  hw/ppc/e500.c  | 30 ++
  hw/ppc/e500.h  |  1 -
  hw/ppc/e500plat.c  |  1 -
  hw/ppc/mpc8544ds.c |  1 -
  4 files changed, 14 insertions(+), 19 deletions(-)

Re: [PATCH 01/11] hw/ppc/meson: Allow e500 boards to be enabled separately

[Philippe Mathieu-Daudé via]

On 15/9/22 17:25, Bernhard Beschow wrote:

Gives users more fine-grained control over what should be compiled into
QEMU.

Signed-off-by: Bernhard Beschow 
---
  configs/devices/ppc-softmmu/default.mak | 3 ++-
  hw/ppc/Kconfig  | 8 
  hw/ppc/meson.build  | 6 ++
  3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/configs/devices/ppc-softmmu/default.mak 
b/configs/devices/ppc-softmmu/default.mak
index 658a454426..a887f5438b 100644
--- a/configs/devices/ppc-softmmu/default.mak
+++ b/configs/devices/ppc-softmmu/default.mak
@@ -1,7 +1,8 @@
  # Default configuration for ppc-softmmu
  
  # For embedded PPCs:

-CONFIG_E500=y
+CONFIG_E500PLAT=y
+CONFIG_MPC8544DS=y
  CONFIG_PPC405=y
  CONFIG_PPC440=y
  CONFIG_VIRTEX=y
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 3a4418a69e..22a64745d4 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -132,6 +132,14 @@ config E500
  select FDT_PPC
  select DS1338
  
+config E500PLAT

+bool
+select E500
+
+config MPC8544DS
+bool
+select E500
+
  config VIRTEX
  bool
  select PPC4XX
diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index 62801923f3..32babc9b48 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -71,12 +71,10 @@ ppc_ss.add(when: 'CONFIG_MAC_OLDWORLD', if_true: 
files('mac_oldworld.c'))
  # NewWorld PowerMac
  ppc_ss.add(when: 'CONFIG_MAC_NEWWORLD', if_true: files('mac_newworld.c'))
  # e500
+ppc_ss.add(when: 'CONFIG_E500PLAT', if_true: files('e500plat.c'))
+ppc_ss.add(when: 'CONFIG_MPC8544DS', if_true: files('mpc8544ds.c'))
  ppc_ss.add(when: 'CONFIG_E500', if_true: files(
'e500.c',
-  'mpc8544ds.c',
-  'e500plat.c'
-))
-ppc_ss.add(when: 'CONFIG_E500', if_true: files(
'mpc8544_guts.c',


Reviewed-by: Philippe Mathieu-Daudé 


'ppce500_spin.c'
  ))

Re: [PATCH 5/7] block/nfs: Fix 32-bit Windows build

[Philippe Mathieu-Daudé via]

On 8/9/22 15:28, Bin Meng wrote:

From: Bin Meng 

libnfs.h declares nfs_fstat() as the following for win32:

   int nfs_fstat(struct nfs_context *nfs, struct nfsfh *nfsfh,
 struct __stat64 *st);

The 'st' parameter should be of type 'struct __stat64'. The
codes happen to build successfully for 64-bit Windows, but it
does not build for 32-bit Windows.

Fixes: 6542aa9c75bc ("block: add native support for NFS")
Fixes: 18a8056e0bc7 ("block/nfs: cache allocated filesize for read-only files")
Signed-off-by: Bin Meng 
---

  block/nfs.c | 8 
  1 file changed, 8 insertions(+)

diff --git a/block/nfs.c b/block/nfs.c
index 444c40b458..d5d67937dd 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -418,7 +418,11 @@ static int64_t nfs_client_open(NFSClient *client, 
BlockdevOptionsNfs *opts,
 int flags, int open_flags, Error **errp)
  {
  int64_t ret = -EINVAL;
+#ifdef _WIN32
+struct __stat64 st;
+#else
  struct stat st;
+#endif
  char *file = NULL, *strp = NULL;
  
  qemu_mutex_init(&client->mutex);

@@ -781,7 +785,11 @@ static int nfs_reopen_prepare(BDRVReopenState *state,
BlockReopenQueue *queue, Error **errp)
  {
  NFSClient *client = state->bs->opaque;
+#ifdef _WIN32
+struct __stat64 st;
+#else
  struct stat st;
+#endif
  int ret = 0;
  
  if (state->flags & BDRV_O_RDWR && bdrv_is_read_only(state->bs)) {


What about the field in struct NFSRPC?

Re: [PATCH v5 13/13] hw/isa/vt82c686: Create rtc-time alias in boards instead

[Philippe Mathieu-Daudé via]

On 1/9/22 13:41, Bernhard Beschow wrote:

According to good QOM practice, an object should only deal with objects
of its own sub tree. Having devices create an alias on the machine
object doesn't respect this good practice. To resolve this, create the
alias in the machine's code.

Signed-off-by: Bernhard Beschow 
---
  hw/isa/vt82c686.c   | 2 --
  hw/mips/fuloong2e.c | 4 
  hw/ppc/pegasos2.c   | 4 
  3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 48cd4d0036..3f9bd0c04d 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -632,8 +632,6 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
  if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
  return;
  }
-object_property_add_alias(qdev_get_machine(), "rtc-time", OBJECT(&s->rtc),
-  "date");
  isa_connect_gpio_out(ISA_DEVICE(&s->rtc), 0, s->rtc.isairq);
  
  for (i = 0; i < PCI_CONFIG_HEADER_SIZE; i++) {

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 3c46215616..b478483706 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -295,6 +295,10 @@ static void mips_fuloong2e_init(MachineState *machine)
  pci_dev = pci_create_simple_multifunction(pci_bus,
PCI_DEVFN(FULOONG2E_VIA_SLOT, 
0),
true, TYPE_VT82C686B_ISA);
+object_property_add_alias(OBJECT(machine), "rtc-time",
+  object_resolve_path_component(OBJECT(pci_dev),
+"rtc"),
+  "date");


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v5 00/13] Instantiate VT82xx functions in host device

[Philippe Mathieu-Daudé via]

On 12/9/22 21:50, Bernhard Beschow wrote:

Am 1. September 2022 11:41:14 UTC schrieb Bernhard Beschow :



Testing done:

* `qemu-system-ppc -machine pegasos2 -rtc base=localtime -device 
ati-vga,guest_hwcursor=true,romfile="" -cdrom morphos-3.17.iso -kernel 
morphos-3.17/boot.img`

  Boots successfully and it is possible to open games and tools.



* I was unable to test the fuloong2e board even before this series since it 
seems to be unfinished [1].

  A buildroot-baked kernel [2] booted but doesn't find its root partition, 
though the issues could be in the buildroot receipt I created.



[1] https://osdn.net/projects/qmiga/wiki/SubprojectPegasos2

[2] https://github.com/shentok/buildroot/commits/fuloong2e



Copying from v2 (just found it in my spam folder :/):
Series:
Reviewed-by: Philippe Mathieu-Daudé 

Review seems complete, thanks to all who participated! Now we just need someone 
to queue this series.

Best regards,
Bernhard


Excellent cleanup! Series queued to mips-next.

Re: [PATCH v5 11/13] hw/mips/fuloong2e: Inline vt82c686b_southbridge_init() and remove it

[Philippe Mathieu-Daudé via]

On 1/9/22 13:41, Bernhard Beschow wrote:

The previous patches moved most of this function into the via-isa device
model such that it has become fairly trivial. So inline it for
simplicity.

Suggested-by: BALATON Zoltan 
Signed-off-by: Bernhard Beschow 
---
  hw/mips/fuloong2e.c | 28 ++--
  1 file changed, 10 insertions(+), 18 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 2/2] vvfat: allow spaces in file names

[Philippe Mathieu-Daudé via]

On 3/9/22 18:23, Hervé Poussineau wrote:

In R/W mode, files with spaces were never created on host side.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1176
Fixes: c79e243ed67683d6d06692bd7040f7394da178b0
Signed-off-by: Hervé Poussineau 
---
  block/vvfat.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index 35057a51c67..9d877028573 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -499,7 +499,7 @@ static bool valid_filename(const unsigned char *name)
(c >= 'A' && c <= 'Z') ||
(c >= 'a' && c <= 'z') ||
c > 127 ||
-  strchr("$%'-_@~`!(){}^#&.+,;=[]", c) != NULL))
+  strchr(" $%'-_@~`!(){}^#&.+,;=[]", c) != NULL))
  {
  return false;
  }


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 2/8] qtest: Set "-net none" in qtest_init()

[Philippe Mathieu-Daudé via]

On 2/9/22 18:51, Juan Quintela wrote:

This way, we don't have networking by default.  If test needs it,
configure it.

Signed-off-by: Juan Quintela 
---
  tests/qtest/bios-tables-test.c | 2 +-
  tests/qtest/libqtest.c | 1 +
  2 files changed, 2 insertions(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 6/8] tests: Fix error strings

[Philippe Mathieu-Daudé via]

On 2/9/22 18:51, Juan Quintela wrote:

They were copy-pasted from e1000e and never changed.

Signed-off-by: Juan Quintela 
---
  tests/qtest/e1000-test.c  | 2 +-
  tests/qtest/es1370-test.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH] 9pfs: use GHashMap for fid table

[Philippe Mathieu-Daudé via]

Hi Linus,

On 3/9/22 17:03, Linus Heckemann wrote:

The previous implementation would iterate over the fid table for
lookup operations, resulting in an operation with O(n) complexity on
the number of open files and poor cache locality -- for nearly every
open, stat, read, write, etc operation.

This change uses a hashtable for this instead, significantly improving
the performance of the 9p filesystem. The runtime of NixOS's simple
installer test, which copies ~122k files totalling ~1.8GiB from 9p,
decreased by a factor of about 10.

Signed-off-by: Linus Heckemann 
---
  hw/9pfs/9p.c | 130 +++
  hw/9pfs/9p.h |   2 +-
  2 files changed, 69 insertions(+), 63 deletions(-)



@@ -424,12 +419,11 @@ static V9fsFidState *clunk_fid(V9fsState *s, int32_t fid)
  {
  V9fsFidState *fidp;
  
-QSIMPLEQ_FOREACH(fidp, &s->fid_list, next) {

-if (fidp->fid == fid) {
-QSIMPLEQ_REMOVE(&s->fid_list, fidp, V9fsFidState, next);
-fidp->clunked = true;
-return fidp;
-}
+fidp = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid));
+if (fidp) {
+g_hash_table_remove(s->fids, GINT_TO_POINTER(fid));
+fidp->clunked = true;
+return fidp;
  }
  return NULL;
  }
@@ -439,10 +433,15 @@ void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu)
  int reclaim_count = 0;
  V9fsState *s = pdu->s;
  V9fsFidState *f;
+


Style nitpicking: we don't restrict to C89 but still declare variables
at the beginning of functions, so please move this new line ...


+GHashTableIter iter;
+gpointer fid;


... here.


+g_hash_table_iter_init(&iter, s->fids);
+
  QSLIST_HEAD(, V9fsFidState) reclaim_list =
  QSLIST_HEAD_INITIALIZER(reclaim_list);
  
-QSIMPLEQ_FOREACH(f, &s->fid_list, next) {

+while (g_hash_table_iter_next(&iter, &fid, (void **) &f)) {


Please cast to (gpointer *) instead.


  /*
   * Unlink fids cannot be reclaimed. Check
   * for them and skip them. Also skip fids
@@ -518,12 +517,12 @@ static int coroutine_fn v9fs_mark_fids_unreclaim(V9fsPDU 
*pdu, V9fsPath *path)
  {
  int err;
  V9fsState *s = pdu->s;
-V9fsFidState *fidp, *fidp_next;
+V9fsFidState *fidp;
+gpointer fid;
+
+GHashTableIter iter;


Style.


+g_hash_table_iter_init(&iter, s->fids);
  
-fidp = QSIMPLEQ_FIRST(&s->fid_list);

-if (!fidp) {
-return 0;
-}
  
  /*

   * v9fs_reopen_fid() can yield : a reference on the fid must be held
@@ -534,7 +533,13 @@ static int coroutine_fn v9fs_mark_fids_unreclaim(V9fsPDU 
*pdu, V9fsPath *path)
   * iteration after we could get a reference on the next fid. Start with
   * the first one.
   */
-for (fidp->ref++; fidp; fidp = fidp_next) {
+while (g_hash_table_iter_next(&iter, &fid, (void **) &fidp)) {


gpointer *.


+/*
+ * Ensure the fid survives a potential clunk request during
+ * put_fid() below and v9fs_reopen_fid() in the next iteration.
+ */
+fidp->ref++;
+
  if (fidp->path.size == path->size &&
  !memcmp(fidp->path.data, path->data, path->size)) {
  /* Mark the fid non reclaimable. */
@@ -548,16 +553,6 @@ static int coroutine_fn v9fs_mark_fids_unreclaim(V9fsPDU 
*pdu, V9fsPath *path)
  }
  }
  
-fidp_next = QSIMPLEQ_NEXT(fidp, next);

-
-if (fidp_next) {
-/*
- * Ensure the next fid survives a potential clunk request during
- * put_fid() below and v9fs_reopen_fid() in the next iteration.
- */
-fidp_next->ref++;
-}
-
  /* We're done with this fid */
  put_fid(pdu, fidp);
  }
@@ -570,18 +565,20 @@ static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
  V9fsState *s = pdu->s;
  V9fsFidState *fidp;
  
+gpointer fid;

+GHashTableIter iter;


Style.


+g_hash_table_iter_init(&iter, s->fids);
+
  /* Free all fids */
-while (!QSIMPLEQ_EMPTY(&s->fid_list)) {
-/* Get fid */
-fidp = QSIMPLEQ_FIRST(&s->fid_list);
+while (g_hash_table_iter_next(&iter, &fid, (void **) &fidp)) {


gpointer *.


  fidp->ref++;
  
  /* Clunk fid */

-QSIMPLEQ_REMOVE(&s->fid_list, fidp, V9fsFidState, next);
  fidp->clunked = true;
  
  put_fid(pdu, fidp);

  }
+g_hash_table_remove_all(s->fids);
  }
  
  #define P9_QID_TYPE_DIR 0x80

@@ -3206,6 +3203,9 @@ static int coroutine_fn v9fs_complete_rename(V9fsPDU 
*pdu, V9fsFidState *fidp,
  V9fsState *s = pdu->s;
  V9fsFidState *dirfidp = NULL;
  


Please remove this extra new line.


+GHashTableIter iter;
+gpointer fid;
+
  v9fs_path_init(&new_path);
  if (newdirfid != -1) {
  dirfidp = get_fid(pdu, newdirfid);
@@ -3238,11 +3238,13 @@ static int coroutine_fn v9fs_complete_rename(V9fsPDU 
*pdu, V9fsFidState *fidp,
  if (err < 

Re: [PATCH v3 3/3] util/aio-win32: Correct the event array size in aio_poll()

[Philippe Mathieu-Daudé via]

On 24/8/22 10:52, Bin Meng wrote:

From: Bin Meng 

WaitForMultipleObjects() can only wait for MAXIMUM_WAIT_OBJECTS
object handles. Correct the event array size in aio_poll() and
add a assert() to ensure it does not cause out of bound access.

Signed-off-by: Bin Meng 
Reviewed-by: Stefan Weil 
Reviewed-by: Marc-André Lureau 
---

(no changes since v2)

Changes in v2:
- change 'count' to unsigned

  util/aio-win32.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: Error ret=-1 with EINTR in nbd_connect_systemd_socket_activation()

[Philippe Mathieu-Daudé via]

Cc'ing qemu-block@

On 11/7/22 08:38, Motohiro Kawahito wrote:
Hi, I’d like to connect to an encrypted QCOW2 file by 
nbd_connect_systemd_socket_activation(), but I got ret=-1 with EINTR.


The arg parameter I used is

qemu-nbd --object secret,id=sec0,data=abc123 --image-opts 
driver=qcow2,encrypt.format=luks,encrypt.key-secret=sec0,file.filename=/tmp/empty.qcow2


Can you find what a problem is? The version of qemu-nbd is

$ qemu-nbd -V

qemu-nbd 4.2.1 (Debian 1:4.2-3ubuntu6.23)

I created this encrypted QCOW2 image by the following command.

qemu-img create --object secret,id=sec0,data=abc123 -f qcow2 -o 
encrypt.format=luks,encrypt.key-secret=sec0 /tmp/empty.qcow2 8539292672


Note that I can connect to a normal QCOW2 file by this function without 
any error. (arg: qemu-nbd -f qcow2 /tmp/normal.qcow2)

Re: [PATCH] qsd: Do not use error_report() before monitor_init

[Philippe Mathieu-Daudé via]

On 9/6/22 14:28, Hanna Reitz wrote:

error_report() only works once monitor_init_globals_core() has been
called, which is not the case when parsing the --daemonize option.  Use
fprintf(stderr, ...) instead.

Fixes: 2525edd85fec53e23fda98974a15e3b3c8957596 ("qsd: Add --daemonize")
Signed-off-by: Hanna Reitz 
---
  storage-daemon/qemu-storage-daemon.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH] tests/qtest: Reduce npcm7xx_sdhci test image size

[Philippe Mathieu-Daudé via]

On 9/6/22 23:41, Hao Wu wrote:

Creating 1GB image for a simple qtest is unnecessary
and could lead to failures. We reduce the image size
to 1MB to reduce the test overhead.

Signed-off-by: Hao Wu 
---
  tests/qtest/npcm7xx_sdhci-test.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 1/1] block: use 'unsigned' for in_flight field on driver state

[Philippe Mathieu-Daudé via]

On 30/5/22 12:39, Denis V. Lunev wrote:

This patch makes in_flight field 'unsigned' for BDRVNBDState and
MirrorBlockJob. This matches the definition of this field on BDS
and is generically correct - we should never get negative value here.

Signed-off-by: Denis V. Lunev 
CC: John Snow 
CC: Vladimir Sementsov-Ogievskiy 
CC: Kevin Wolf 
CC: Hanna Reitz 
CC: Eric Blake 
---
  block/mirror.c | 2 +-
  block/nbd.c| 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)


FWIW there is another occurrence in qemu-img.c.

Re: [PATCH v2 00/25] hw/sd: Rework models for eMMC support

[Philippe Mathieu-Daudé via]

On 31/5/22 11:19, Cédric Le Goater wrote:

On 5/30/22 21:37, Philippe Mathieu-Daudé wrote:

From: Philippe Mathieu-Daudé 

Rebase/respin of Cédric RFC:
https://lore.kernel.org/qemu-devel/20220318132824.1134400-1-...@kaod.org/
(sorry it took me so long guys...)

Pushed at https://gitlab.com/philmd/qemu/-/commits/emmc-v2

I plan to queue patches 1-12 via sdmmc-next later this week.

Cédric, if you are happy with this series, it should be easy to rebase
your other patches on top and address the comments I left on the RFC :)


I pushed an update on :

   https://github.com/legoater/qemu/commits/aspeed-7.1

Here is an image :

   https://www.kaod.org/qemu/aspeed/mmc-p10bmc.qcow2

run with :

  qemu-system-arm -M rainier-bmc -net nic -net user -drive 
file=./mmc-p10bmc.qcow2,format=qcow2,if=sd,id=sd0,index=2 -nographic 
-nodefaults -snapshot -serial mon:stdio


Useful, thanks.

I see in hw/arm/aspeed_ast2600.c:

/* Init sd card slot class here so that they're under the correct 
parent */

for (i = 0; i < ASPEED_SDHCI_NUM_SLOTS; ++i) {
object_initialize_child(obj, "sd-controller.sdhci[*]",
&s->sdhci.slots[i], TYPE_SYSBUS_SDHCI);
}

object_initialize_child(obj, "emmc-controller.sdhci", 
&s->emmc.slots[0],

TYPE_SYSBUS_SDHCI);

/* eMMC Boot Controller stub */
create_unimplemented_device("aspeed.emmc-boot-controller",
sc->memmap[ASPEED_DEV_EMMC_BC],
0x1000);

/* eMMC */
if (!sysbus_realize(SYS_BUS_DEVICE(&s->emmc), errp)) {
return;
}
sysbus_mmio_map(SYS_BUS_DEVICE(&s->emmc), 0, 
sc->memmap[ASPEED_DEV_EMMC]);

sysbus_connect_irq(SYS_BUS_DEVICE(&s->emmc), 0,
   aspeed_soc_get_irq(s, ASPEED_DEV_EMMC));

Where is 'emmc-controller.sdhci' realized?

In aspeed_sdhci_realize() you set sd-spec-version" = 2, is that OK
with eMMC?

What expects the real hw?

Thanks,

Phil.

Re: [PATCH] hw/sd/sdhci: Block Size Register bits [14:12] is lost

[Philippe Mathieu-Daudé via]

On 21/3/22 06:56, Lu Gao wrote:

Block Size Register bits [14:12] is SDMA Buffer Boundary, it is missed
in register write, but it is needed in SDMA transfer. e.g. it will be
used in sdhci_sdma_transfer_multi_blocks to calculate boundary_ variables.

Missing this field will cause wrong operation for different SDMA Buffer
Boundary settings.


Fixes: d7dfca0807 ("hw/sdhci: introduce standard SD host controller")
Fixes: dfba99f17f ("hw/sdhci: Fix DMA Transfer Block Size field")

Reviewed-by: Philippe Mathieu-Daudé 


Signed-off-by: Lu Gao 
Signed-off-by: Jianxian Wen 
---
  hw/sd/sdhci.c | 15 +++
  1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index e0bbc90344..350ceb487d 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -321,6 +321,8 @@ static void sdhci_poweron_reset(DeviceState *dev)
  
  static void sdhci_data_transfer(void *opaque);
  
+#define BLOCK_SIZE_MASK (4 * KiB - 1)

+
  static void sdhci_send_command(SDHCIState *s)
  {
  SDRequest request;
@@ -371,7 +373,8 @@ static void sdhci_send_command(SDHCIState *s)
  
  sdhci_update_irq(s);
  
-if (!timeout && s->blksize && (s->cmdreg & SDHC_CMD_DATA_PRESENT)) {

+if (!timeout && (s->blksize & BLOCK_SIZE_MASK) &&
+(s->cmdreg & SDHC_CMD_DATA_PRESENT)) {
  s->data_count = 0;
  sdhci_data_transfer(s);
  }
@@ -406,7 +409,6 @@ static void sdhci_end_transfer(SDHCIState *s)
  /*
   * Programmed i/o data transfer
   */
-#define BLOCK_SIZE_MASK (4 * KiB - 1)
  
  /* Fill host controller's read buffer with BLKSIZE bytes of data from card */

  static void sdhci_read_block_from_card(SDHCIState *s)
@@ -1137,7 +1139,8 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, 
unsigned size)
  s->sdmasysad = (s->sdmasysad & mask) | value;
  MASKED_WRITE(s->sdmasysad, mask, value);
  /* Writing to last byte of sdmasysad might trigger transfer */
-if (!(mask & 0xFF00) && s->blkcnt && s->blksize &&
+if (!(mask & 0xFF00) && s->blkcnt &&
+(s->blksize & BLOCK_SIZE_MASK) &&
  SDHC_DMA_TYPE(s->hostctl1) == SDHC_CTRL_SDMA) {
  if (s->trnmod & SDHC_TRNS_MULTI) {
  sdhci_sdma_transfer_multi_blocks(s);
@@ -1151,7 +1154,11 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, 
unsigned size)
  if (!TRANSFERRING_DATA(s->prnsts)) {
  uint16_t blksize = s->blksize;
  
-MASKED_WRITE(s->blksize, mask, extract32(value, 0, 12));

+/*
+ * [14:12] SDMA Buffer Boundary
+ * [11:00] Transfer Block Size
+ */
+MASKED_WRITE(s->blksize, mask, extract32(value, 0, 15));
  MASKED_WRITE(s->blkcnt, mask >> 16, value >> 16);
  
  /* Limit block size to the maximum buffer size */

Re: -blockdev vs -snapshot bug

[Philippe Mathieu-Daudé via]

Cc'ing qemu-block@

On 13/5/22 20:54, Michael Tokarev wrote:

Hi!

Now here's something.. interesting.

I tested -blockdev here with a real image.  This way:

qemu-system-x86_64 ... -snapshot \
  -blockdev qcow2,node-name=q,file.driver=file,file.filename=w.qcow2 \
  -device ide-hd,drive=q

I always use -snapshot when testing something, not to damage the image.

And to my great surprise, the above command *did* damage my image.

It looks like -snapshot is entirely ignored by -blockdev.

This is quite a serious issue, to me anyway, - it is seriously
unexpected.

If it is intentional, I think an error should be thrown back
because the expectation is definitely different.

Or am I doing it wrong?

Thanks,

/mjt

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Philippe Mathieu-Daudé via]

On 31/5/22 07:58, Cédric Le Goater wrote:

On 5/30/22 19:40, Philippe Mathieu-Daudé wrote:

On 18/3/22 14:28, Cédric Le Goater wrote:

The initial eMMC support from Vincent Palatin was largely reworked to
match the current SD framework. The parameters mimick a real 4GB eMMC,
but it can be set to various sizes.

This adds a new QOM object class for EMMC devices.

Signed-off-by: Vincent Palatin 
Link: 
https://lore.kernel.org/r/1311635951-11047-5-git-send-email-vpala...@chromium.org 


[ jms: - Forward ported to QEMU 5.2 ]
Signed-off-by: Joel Stanley 
[ clg: - ported on aspeed-7.0 patchset
    - HPI activation ]
Signed-off-by: Cédric Le Goater 
---
  hw/sd/sdmmc-internal.h |  97 +++
  include/hw/sd/sd.h |   9 ++
  hw/sd/sd.c | 205 -
  hw/sd/sdmmc-internal.c |   2 +-
  4 files changed, 311 insertions(+), 2 deletions(-)




+static void emmc_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SDCardClass *sc = SD_CARD_CLASS(klass);
+
+    dc->desc = "eMMC";
+    sc->proto = &sd_proto_emmc;
+    sc->spec_version = SD_PHY_SPECv3_01_VERS; /* eMMC requirement */
+    sc->set_csd = sd_emmc_set_csd;
+}
+
+static const TypeInfo emmc_info = {
+    .name = TYPE_EMMC,
+    .parent = TYPE_SD_CARD,


Hmm this is odd to have the model inheriting features from SD_CARD but 
then behaving differently (one could enumerate QDEV objects implementing

TYPE_SD_CARD then use them expecting they match the SD card protocol).

Why do you need to have TYPE_SD_CARD as parent?


Simply for the initialization.

Could we simply duplicate sd_class_init() assignations instead? That
would likely make it easier to modify eMMC handlers.


May be we lack a base abstract class ?


I've been thinking about it but maybe not enough. I'll revisit.


It would clean up this section in the realize routine :

    sd->proto = sd->spi ? &sd_proto_spi : &sd_proto_sd;

     if (sc->proto) {
     sd->proto = sc->proto;
     }


In v2 I moved the 'proto' field from instance to class, so we don't need
this hack anymore.

Re: [PATCH v2 00/25] hw/sd: Rework models for eMMC support

[Philippe Mathieu-Daudé via]

On Tue, May 31, 2022 at 9:56 AM Philippe Mathieu-Daudé  wrote:
> On 31/5/22 08:31, Cédric Le Goater wrote:
> > On 5/30/22 21:37, Philippe Mathieu-Daudé wrote:

> >> I plan to queue patches 1-12 via sdmmc-next later this week.
> >>
> >> Cédric, if you are happy with this series, it should be easy to rebase
> >> your other patches on top and address the comments I left on the RFC :)
> >
> > Sure. I will for the first patches to be merged and I might introduce
> > a base class.
>
> Then consider patches 1-13 queued.

Oops too fast, new patches 3 & 13 are not reviewed.

Re: [PATCH v2 14/25] hw/sd: Basis for eMMC support

[Philippe Mathieu-Daudé via]

On 30/5/22 21:38, Philippe Mathieu-Daudé wrote:

From: Philippe Mathieu-Daudé 


I missed something during the cherry-pick, this should be:

From: Cédric Le Goater 


The initial eMMC support from Vincent Palatin was largely reworked to
match the current SD framework.

Signed-off-by: Cédric Le Goater 
Signed-off-by: Philippe Mathieu-Daudé 
---
TODO: Do not inherit TYPE_SD_CARD, duplicate sd_class_init()
---
  hw/sd/sd.c | 42 ++
  include/hw/sd/sd.h |  3 +++
  2 files changed, 45 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b2f16dbb73..8b178aa261 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2166,6 +2166,19 @@ static const SDProto sd_proto_sd = {
  },
  };
  
+static const SDProto sd_proto_emmc = {

+.name = "eMMC",
+.cmd = {
+[0] = sd_cmd_GO_IDLE_STATE,
+[5] = sd_cmd_illegal,
+[19]= sd_cmd_SEND_TUNING_BLOCK,
+[41]= sd_cmd_illegal,
+[52 ... 54] = sd_cmd_illegal,
+[58]= sd_cmd_illegal,
+[59]= sd_cmd_illegal,
+},
+};
+
  static void sd_instance_init(Object *obj)
  {
  SDState *sd = SD_CARD(obj);
@@ -2284,9 +2297,38 @@ static const TypeInfo sd_info = {
  .instance_finalize = sd_instance_finalize,
  };
  
+static void emmc_realize(DeviceState *dev, Error **errp)

+{
+SDState *sd = SD_CARD(dev);
+
+if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
+error_setg(errp, "Minimum spec for eMMC is v3.01");
+return;
+}
+
+sd_realize(dev, errp);
+}
+
+static void emmc_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+SDCardClass *sc = SD_CARD_CLASS(klass);
+
+dc->desc = "eMMC";
+dc->realize = emmc_realize;
+sc->proto = &sd_proto_emmc;
+}
+
+static const TypeInfo emmc_info = {
+.name = TYPE_EMMC,
+.parent = TYPE_SD_CARD,
+.class_init = emmc_class_init,
+ };
+
  static void sd_register_types(void)
  {
  type_register_static(&sd_info);
+type_register_static(&emmc_info);
  }
  
  type_init(sd_register_types)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 0d94e1f346..e52436b7a5 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -93,6 +93,9 @@ typedef struct {
  #define TYPE_SD_CARD "sd-card"
  OBJECT_DECLARE_TYPE(SDState, SDCardClass, SD_CARD)
  
+#define TYPE_EMMC "emmc"

+DECLARE_INSTANCE_CHECKER(SDState, EMMC, TYPE_EMMC)
+
  struct SDCardClass {
  /*< private >*/
  DeviceClass parent_class;

Re: [PATCH v2 00/25] hw/sd: Rework models for eMMC support

[Philippe Mathieu-Daudé via]

On 31/5/22 08:31, Cédric Le Goater wrote:

On 5/30/22 21:37, Philippe Mathieu-Daudé wrote:

From: Philippe Mathieu-Daudé 

Rebase/respin of Cédric RFC:
https://lore.kernel.org/qemu-devel/20220318132824.1134400-1-...@kaod.org/
(sorry it took me so long guys...)

Pushed at https://gitlab.com/philmd/qemu/-/commits/emmc-v2

I plan to queue patches 1-12 via sdmmc-next later this week.

Cédric, if you are happy with this series, it should be easy to rebase
your other patches on top and address the comments I left on the RFC :)


Sure. I will for the first patches to be merged and I might introduce
a base class.


Then consider patches 1-13 queued.


Thanks,

C.



Regards,

Phil.

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Philippe Mathieu-Daudé via]

On 18/3/22 14:28, Cédric Le Goater wrote:

The initial eMMC support from Vincent Palatin was largely reworked to
match the current SD framework. The parameters mimick a real 4GB eMMC,
but it can be set to various sizes.

This adds a new QOM object class for EMMC devices.

Signed-off-by: Vincent Palatin 
Link: 
https://lore.kernel.org/r/1311635951-11047-5-git-send-email-vpala...@chromium.org
[ jms: - Forward ported to QEMU 5.2 ]
Signed-off-by: Joel Stanley 
[ clg: - ported on aspeed-7.0 patchset
- HPI activation ]
Signed-off-by: Cédric Le Goater 
---
  hw/sd/sdmmc-internal.h |  97 +++
  include/hw/sd/sd.h |   9 ++
  hw/sd/sd.c | 205 -
  hw/sd/sdmmc-internal.c |   2 +-
  4 files changed, 311 insertions(+), 2 deletions(-)



  static void sd_instance_init(Object *obj)
  {
  SDState *sd = SD_CARD(obj);
@@ -2162,10 +2338,19 @@ static void sd_instance_finalize(Object *obj)
  static void sd_realize(DeviceState *dev, Error **errp)
  {
  SDState *sd = SD_CARD(dev);
+SDCardClass *sc = SD_CARD_GET_CLASS(sd);
  int ret;
  
  sd->proto = sd->spi ? &sd_proto_spi : &sd_proto_sd;
  
+if (sc->proto) {

+sd->proto = sc->proto;
+}
+
+if (sc->spec_version) {
+sd->spec_version = sc->spec_version;
+}
+
  switch (sd->spec_version) {
  case SD_PHY_SPECv1_10_VERS
   ... SD_PHY_SPECv3_01_VERS:



Instead I'd use:

-- >8 --
@@ -2301,14 +2297,26 @@ static const TypeInfo sd_info = {
 .instance_finalize = sd_instance_finalize,
 };

+static void emmc_realize(DeviceState *dev, Error **errp)
+{
+SDState *sd = SD_CARD(dev);
+
+if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
+error_setg(errp, "Minimum spec for eMMC is v3.01");
+return;
+}
+
+sd_realize(dev, errp);
+}
+
 static void emmc_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
 SDCardClass *sc = SD_CARD_CLASS(klass);

 dc->desc = "eMMC";
+dc->realize = emmc_realize;
 sc->proto = &sd_proto_emmc;
 }

---

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Philippe Mathieu-Daudé via]

On 18/3/22 14:28, Cédric Le Goater wrote:

The initial eMMC support from Vincent Palatin was largely reworked to
match the current SD framework. The parameters mimick a real 4GB eMMC,
but it can be set to various sizes.

This adds a new QOM object class for EMMC devices.

Signed-off-by: Vincent Palatin 
Link: 
https://lore.kernel.org/r/1311635951-11047-5-git-send-email-vpala...@chromium.org
[ jms: - Forward ported to QEMU 5.2 ]
Signed-off-by: Joel Stanley 
[ clg: - ported on aspeed-7.0 patchset
- HPI activation ]
Signed-off-by: Cédric Le Goater 
---
  hw/sd/sdmmc-internal.h |  97 +++
  include/hw/sd/sd.h |   9 ++
  hw/sd/sd.c | 205 -
  hw/sd/sdmmc-internal.c |   2 +-
  4 files changed, 311 insertions(+), 2 deletions(-)




+static void emmc_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+SDCardClass *sc = SD_CARD_CLASS(klass);
+
+dc->desc = "eMMC";
+sc->proto = &sd_proto_emmc;
+sc->spec_version = SD_PHY_SPECv3_01_VERS; /* eMMC requirement */
+sc->set_csd = sd_emmc_set_csd;
+}
+
+static const TypeInfo emmc_info = {
+.name = TYPE_EMMC,
+.parent = TYPE_SD_CARD,


Hmm this is odd to have the model inheriting features from SD_CARD but 
then behaving differently (one could enumerate QDEV objects implementing

TYPE_SD_CARD then use them expecting they match the SD card protocol).

Why do you need to have TYPE_SD_CARD as parent?

Could we simply duplicate sd_class_init() assignations instead? That
would likely make it easier to modify eMMC handlers.


+.class_init = emmc_class_init,
+ };

Re: [RFC PATCH 07/17] hw/sd: Add sd_cmd_SEND_OP_CMD() handler

[Philippe Mathieu-Daudé via]

On 10/5/22 08:57, Cédric Le Goater wrote:

On 5/9/22 23:12, Philippe Mathieu-Daudé wrote:

On 18/3/22 14:28, Cédric Le Goater wrote:

From: Philippe Mathieu-Daudé 

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20210624142209.1193073-9-f4...@amsat.org>
Signed-off-by: Cédric Le Goater 
---
  hw/sd/sd.c | 18 +-
  1 file changed, 9 insertions(+), 9 deletions(-)



@@ -2111,6 +2109,7 @@ static const SDProto sd_proto_spi = {
  .name = "SPI",
  .cmd = {
  [0] = sd_cmd_GO_IDLE_STATE,
+    [1] = sd_cmd_SEND_OP_CMD,
  [2 ... 4]   = sd_cmd_illegal,
  [5] = sd_cmd_illegal,
  [7] = sd_cmd_illegal,
@@ -2120,6 +2119,7 @@ static const SDProto sd_proto_spi = {
  },
  .cmd = {
  [6] = sd_cmd_unimplemented,
+    [41]    = sd_cmd_SEND_OP_CMD,
  },
  };


I missed adding the cmd_abbrev[1] entry.


Will you resend ?


Yes.

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Philippe Mathieu-Daudé via]

Hi Cédric,

On 18/3/22 14:28, Cédric Le Goater wrote:

The initial eMMC support from Vincent Palatin was largely reworked to
match the current SD framework. The parameters mimick a real 4GB eMMC,
but it can be set to various sizes.

This adds a new QOM object class for EMMC devices.

Signed-off-by: Vincent Palatin 
Link: 
https://lore.kernel.org/r/1311635951-11047-5-git-send-email-vpala...@chromium.org
[ jms: - Forward ported to QEMU 5.2 ]
Signed-off-by: Joel Stanley 
[ clg: - ported on aspeed-7.0 patchset
- HPI activation ]
Signed-off-by: Cédric Le Goater 
---
  hw/sd/sdmmc-internal.h |  97 +++
  include/hw/sd/sd.h |   9 ++
  hw/sd/sd.c | 205 -
  hw/sd/sdmmc-internal.c |   2 +-
  4 files changed, 311 insertions(+), 2 deletions(-)



+static const SDProto sd_proto_emmc = {


What about renaming as:

... emmc_proto = {


+.name = "eMMC",
+.cmd = {
+[0] = sd_cmd_GO_IDLE_STATE,
+[1] = sd_emmc_cmd_SEND_OP_CMD,


   = emmc_cmd_SEND_OP_CMD,


+[2] = sd_emmc_cmd_ALL_SEND_CID,


 ...

?


+[3] = sd_emmc_cmd_SEND_RELATIVE_ADDR,
+[5] = sd_cmd_illegal,
+[8] = sd_emmc_cmd_SEND_EXT_CSD,
+[19]= sd_cmd_SEND_TUNING_BLOCK,
+[21]= sd_emmc_cmd_SEND_TUNING_BLOCK,
+[41]= sd_cmd_illegal,
+[52 ... 54] = sd_cmd_illegal,
+[55]= sd_emmc_cmd_APP_CMD,
+[58]= sd_cmd_illegal,
+[59]= sd_cmd_illegal,
+},
+};

Re: [PATCH] hw/sd/sdcard: Return ILLEGAL for CMD19/CMD23 prior SD spec v3.01

[Philippe Mathieu-Daudé via]

On 9/5/22 16:29, Bin Meng wrote:

On Mon, May 9, 2022 at 10:13 PM Philippe Mathieu-Daudé
 wrote:


From: Philippe Mathieu-Daudé 

CMD19 (SEND_TUNING_BLOCK) and CMD23 (SET_BLOCK_COUNT) were
added in the Physical SD spec v3.01. When earlier spec version


nits: it should be spec v3.00, despite the fact that in QEMU we have
been using a name v3.01 to indicate v3.00.


Only the "Physical Layer Simplified Specification"s are public:
https://www.sdcard.org/downloads/pls/archives/

I'll rename to "Physical Layer Simplified Specification v3.01"
so it is clearer.




is requested, we should return ILLEGAL.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/sd/sd.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8e6fa09151..7e3bb12b1a 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1263,7 +1263,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, 
SDRequest req)

  case 19:/* CMD19: SEND_TUNING_BLOCK (SD) */
  if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
-break;
+goto bad_cmd;
  }
  if (sd->state == sd_transfer_state) {
  sd->state = sd_sendingdata_state;
@@ -1274,7 +1274,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, 
SDRequest req)

  case 23:/* CMD23: SET_BLOCK_COUNT */
  if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
-break;
+goto bad_cmd;
  }
  switch (sd->state) {
  case sd_transfer_state:
--


Reviewed-by: Bin Meng 


Thanks! Queued.

Re: [PATCH] hw/nvme: deprecate the use-intel-id compatibility parameter

[Philippe Mathieu-Daudé via]

On 29/4/22 07:41, Klaus Jensen wrote:

From: Klaus Jensen 

Since version 5.2 commit 6eb7a071292a ("hw/block/nvme: change controller
pci id"), the emulated NVMe controller has defaulted to a non-Intel PCI
identifier.

Deprecate the compatibility parameter so we can get rid of it once and
for all.

Signed-off-by: Klaus Jensen 
---
  docs/about/deprecated.rst | 8 
  1 file changed, 8 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 896e5a97abbd..450f945ac25f 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -356,6 +356,14 @@ contains native support for this feature and thus use of 
the option
  ROM approach is obsolete. The native SeaBIOS support can be activated
  by using ``-machine graphics=off``.
  
+``-device nvme,use-intel-id=on|off`` (since 7.1)

+
+
+The ``nvme`` device originally used a PCI Vendor/Device Identifier combination
+from Intel that was not properly allocated. Since version 5.2, the controller
+has used a properly allocated identifier. Deprecate the ``use-intel-id``
+machine compatibility parameter.
+
  
  Block device options

  


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 1/6] scsi-disk: add FORMAT UNIT command

[Philippe Mathieu-Daudé via]

On 21/4/22 08:51, Mark Cave-Ayland wrote:

When initialising a drive ready to install MacOS, Apple HD SC Setup first 
attempts
to format the drive. Add a simple FORMAT UNIT command which simply returns 
success
to allow the format to succeed.

Signed-off-by: Mark Cave-Ayland 
---
  hw/scsi/scsi-disk.c  | 4 
  hw/scsi/trace-events | 1 +
  2 files changed, 5 insertions(+)



diff --git a/hw/scsi/trace-events b/hw/scsi/trace-events
index 20fb0dc162..e91b55a961 100644
--- a/hw/scsi/trace-events
+++ b/hw/scsi/trace-events
@@ -334,6 +334,7 @@ scsi_disk_emulate_command_UNMAP(size_t xfer) "Unmap (len 
%zd)"
  scsi_disk_emulate_command_VERIFY(int bytchk) "Verify (bytchk %d)"
  scsi_disk_emulate_command_WRITE_SAME(int cmd, size_t xfer) "WRITE SAME %d (len 
%zd)"
  scsi_disk_emulate_command_UNKNOWN(int cmd, const char *name) "Unknown SCSI command 
(0x%2.2x=%s)"
+scsi_disk_emulate_command_FORMAT_UNIT(size_t xfer) "Format Unit (len %zd)"


%zu (%zd is for ssize_t), otherwise:

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2] hw/block/fdc-sysbus: Always mark sysbus floppy controllers as not having DMA

[Philippe Mathieu-Daudé via]

On 5/5/22 12:18, Peter Maydell wrote:

The sysbus floppy controllers (devices sysbus-fdc and sun-fdtwo)
don't support DMA.  The core floppy controller code expects this to
be indicated by setting FDCtrl::dma_chann to -1.  This used to be
done in the device instance_init functions sysbus_fdc_initfn() and
sun4m_fdc_initfn(), but in commit 1430759ec3e we refactored this code
and accidentally lost the setting of dma_chann.

For sysbus-fdc this has no ill effects because we were redundantly
also setting dma_chann in fdctrl_init_sysbus(), but for sun-fdtwo
this means that guests which try to enable DMA on the floppy
controller will cause QEMU to crash because FDCtrl::dma is NULL.

Set dma_chann to -1 in the common instance init, and remove the
redundant code in fdctrl_init_sysbus() that is also setting it.

There is a six-year-old FIXME comment in the jazz board code to the
effect that in theory it should support doing DMA via a custom DMA
controller.  If anybody ever chooses to fix that they can do it by
adding support for setting both FDCtrl::dma_chann and FDCtrl::dma.
(A QOM link property 'dma-controller' on the sysbus device which can
be set to an instance of IsaDmaClass is probably the way to go.)

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/958
Signed-off-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Mark Cave-Ayland 
---
Changes v1->v2: remove now-unused 'fdctrl' local variable
  from fdctrl_init_sysbus()
---
  include/hw/block/fdc.h |  3 +--
  hw/block/fdc-sysbus.c  | 16 +++-
  hw/mips/jazz.c |  2 +-
  3 files changed, 13 insertions(+), 8 deletions(-)


Queued to mips-next, thanks.

Re: [PATCH 1/4] hw/ide/atapi.c: Correct typos (CD-CDROM -> CD-ROM)

[Philippe Mathieu-Daudé via]

On 28/5/22 22:46, Lev Kujawski wrote:

Signed-off-by: Lev Kujawski 
---
  hw/ide/atapi.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 4/4] hw/ide/piix: Ignore writes of hardwired PCI command register bits

[Philippe Mathieu-Daudé via]

On 28/5/22 22:47, Lev Kujawski wrote:

One method to enable PCI bus mastering for IDE controllers, often used
by x86 firmware, is to write 0x7 to the PCI command register.  Neither
the PIIX3 specification nor actual hardware (a Tyan S1686D system)
permit modification of the Memory Space Enable (MSE) bit, 1, and thus
the command register would be left in an unspecified state without
this patch.

Signed-off-by: Lev Kujawski 
---
  hw/ide/piix.c | 25 +
  1 file changed, 25 insertions(+)

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 76ea8fd9f6..f1d1168ecd 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -25,6 +25,8 @@
   * References:
   *  [1] 82371FB (PIIX) AND 82371SB (PIIX3) PCI ISA IDE XCELERATOR,
   *  290550-002, Intel Corporation, April 1997.
+ *  [2] 82371AB PCI-TO-ISA / IDE XCELERATOR (PIIX4), 290562-001,
+ *  Intel Corporation, April 1997.
   */
  
  #include "qemu/osdep.h"

@@ -32,6 +34,7 @@
  #include "migration/vmstate.h"
  #include "qapi/error.h"
  #include "qemu/module.h"
+#include "qemu/range.h"
  #include "sysemu/block-backend.h"
  #include "sysemu/blockdev.h"
  #include "sysemu/dma.h"
@@ -220,6 +223,26 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
  }
  }
  
+static void piix_pci_config_write(PCIDevice *d, uint32_t addr,

+  uint32_t val, int l)
+{
+/*
+ * Mask all IDE PCI command register bits except for Bus Master
+ * Function Enable (bit 2) and I/O Space Enable (bit 1), as the
+ * remainder are hardwired to 0 [1, p.48] [2, p.89-90].
+ *
+ * NOTE: According to the PIIX3 datasheet [1], the Memory Space
+ * Enable (MSE bit) is hardwired to 1, but this is contradicted by
+ * actual PIIX3 hardware, the datasheet itself (viz., Default
+ * Value: h), and the PIIX4 datasheet [2].
+ */
+if (range_covers_byte(addr, l, PCI_COMMAND)) {
+val &= ~(0xfffa << ((PCI_COMMAND - addr) << 3));


Watch out, len can be 1/2/4.


+}
+
+pci_default_write_config(d, addr, val, l);
+}
+
  /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
  static void piix3_ide_class_init(ObjectClass *klass, void *data)
  {
@@ -232,6 +255,7 @@ static void piix3_ide_class_init(ObjectClass *klass, void 
*data)
  k->vendor_id = PCI_VENDOR_ID_INTEL;
  k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1;
  k->class_id = PCI_CLASS_STORAGE_IDE;
+k->config_write = piix_pci_config_write;
  set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
  dc->hotpluggable = false;
  }
@@ -260,6 +284,7 @@ static void piix4_ide_class_init(ObjectClass *klass, void 
*data)
  k->vendor_id = PCI_VENDOR_ID_INTEL;
  k->device_id = PCI_DEVICE_ID_INTEL_82371AB;
  k->class_id = PCI_CLASS_STORAGE_IDE;
+k->config_write = piix_pci_config_write;
  set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
  dc->hotpluggable = false;
  }

Re: [PATCH 3/4] piix_ide_reset: Use pci_set_* functions instead of direct access

[Philippe Mathieu-Daudé via]

On 28/5/22 22:47, Lev Kujawski wrote:

Eliminates the remaining TODOs in hw/ide/piix.c by:
- Using pci_set_{size} functions to write the PIIX PCI configuration
   space instead of manipulating it directly as an array; and
- Documenting default register values by reference to the controlling
   specification.

Signed-off-by: Lev Kujawski 
---
  hw/ide/piix.c | 17 +
  1 file changed, 9 insertions(+), 8 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 3/3] include/hw/ide: Unexport pci_piix3_xen_ide_unplug()

[Philippe Mathieu-Daudé via]

On 13/5/22 20:09, Bernhard Beschow wrote:

This function was declared in a generic and public header, implemented
in a device-specific source file but only used in xen_platform. Given its
'aux' parameter, this function is more xen-specific than piix-specific.
Also, the hardcoded magic constants seem to be generic and related to
PCIIDEState and IDEBus rather than piix.

Therefore, move this function to xen_platform, unexport it, and drop the
"piix3" in the function name as well.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Paul Durrant 
---
  hw/i386/xen/xen_platform.c | 48 +-
  hw/ide/piix.c  | 46 
  include/hw/ide.h   |  3 ---
  3 files changed, 47 insertions(+), 50 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PULL 0/9] Block patches

[Philippe Mathieu-Daudé via]

On Thu, May 12, 2022 at 12:29 AM Philippe Mathieu-Daudé  wrote:
>
> Hi Stefan, Nicolas,
>
> > Nicolas Saenz Julienne (3):
> >   Introduce event-loop-base abstract class
> >   util/main-loop: Introduce the main loop into QOM
> >   util/event-loop-base: Introduce options to set the thread pool size
>
> I'm getting this warning on Darwin:
>
> ...
> [379/1097] Linking static target libevent-loop-base.a
> warning: 
> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib:
> archive library: libevent-loop-base.a the table of contents is empty
> (no object file members in the library define global symbols)
> ...

Opened https://gitlab.com/qemu-project/qemu/-/issues/1044 to track this.

Re: [PATCH 2/2] coroutine: Revert to constant batch size

[Philippe Mathieu-Daudé via]

Hi Hiroki,

On Thu, May 12, 2022 at 8:57 AM 成川 弘樹  wrote:
>
> Thank you for your fix.
>
> I confirmed that after applying this patch, my intended performance
> improvement by 4c41c69e is still kept in our environment.

Is that equivalent to a formal
Tested-by: Hiroki Narukawa 
tag?

> On 2022/05/11 0:10, Kevin Wolf wrote:
> > Commit 4c41c69e changed the way the coroutine pool is sized because for
> > virtio-blk devices with a large queue size and heavy I/O, it was just
> > too small and caused coroutines to be deleted and reallocated soon
> > afterwards. The change made the size dynamic based on the number of
> > queues and the queue size of virtio-blk devices.
> >
> > There are two important numbers here: Slightly simplified, when a
> > coroutine terminates, it is generally stored in the global release pool
> > up to a certain pool size, and if the pool is full, it is freed.
> > Conversely, when allocating a new coroutine, the coroutines in the
> > release pool are reused if the pool already has reached a certain
> > minimum size (the batch size), otherwise we allocate new coroutines.
> >
> > The problem after commit 4c41c69e is that it not only increases the
> > maximum pool size (which is the intended effect), but also the batch
> > size for reusing coroutines (which is a bug). It means that in cases
> > with many devices and/or a large queue size (which defaults to the
> > number of vcpus for virtio-blk-pci), many thousand coroutines could be
> > sitting in the release pool without being reused.
> >
> > This is not only a waste of memory and allocations, but it actually
> > makes the QEMU process likely to hit the vm.max_map_count limit on Linux
> > because each coroutine requires two mappings (its stack and the guard
> > page for the stack), causing it to abort() in qemu_alloc_stack() because
> > when the limit is hit, mprotect() starts to fail with ENOMEM.
> >
> > In order to fix the problem, change the batch size back to 64 to avoid
> > uselessly accumulating coroutines in the release pool, but keep the
> > dynamic maximum pool size so that coroutines aren't freed too early
> > in heavy I/O scenarios.
> >
> > Note that this fix doesn't strictly make it impossible to hit the limit,
> > but this would only happen if most of the coroutines are actually in use
> > at the same time, not just sitting in a pool. This is the same behaviour
> > as we already had before commit 4c41c69e. Fully preventing this would
> > require allowing qemu_coroutine_create() to return an error, but it
> > doesn't seem to be a scenario that people hit in practice.
> >
> > Cc: qemu-sta...@nongnu.org
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2079938
> > Fixes: 4c41c69e05fe28c0f95f8abd2ebf407e95a4f04b
> > Signed-off-by: Kevin Wolf 
> > ---

Re: [PULL 0/9] Block patches

[Philippe Mathieu-Daudé via]

Hi Stefan, Nicolas,

On Mon, May 9, 2022 at 3:14 PM Stefan Hajnoczi  wrote:
>
> The following changes since commit 554623226f800acf48a2ed568900c1c968ec9a8b:
>
>   Merge tag 'qemu-sparc-20220508' of https://github.com/mcayland/qemu into 
> staging (2022-05-08 17:03:26 -0500)
>
> are available in the Git repository at:
>
>   https://gitlab.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to 3dc584abeef0e1277c2de8c1c1974cb49444eb0a:
>
>   virtio-scsi: move request-related items from .h to .c (2022-05-09 10:45:04 
> +0100)
>
> 
> Pull request
>
> - Add new thread-pool-min/thread-pool-max parameters to control the thread 
> pool
>   used for async I/O.
>
> - Fix virtio-scsi IOThread 100% CPU consumption QEMU 7.0 regression.
>
> 
>
> Nicolas Saenz Julienne (3):
>   Introduce event-loop-base abstract class
>   util/main-loop: Introduce the main loop into QOM
>   util/event-loop-base: Introduce options to set the thread pool size

I'm getting this warning on Darwin:

...
[379/1097] Linking static target libevent-loop-base.a
warning: 
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib:
archive library: libevent-loop-base.a the table of contents is empty
(no object file members in the library define global symbols)
...

Having:

$ nm libevent-loop-base.a

libevent-loop-base.a(event-loop-base.c.o):
05d0 d _.compoundliteral
 U ___stack_chk_fail
 U ___stack_chk_guard
05e0 d _aio_max_batch_info
 t _do_qemu_init_register_types
 U _error_setg_internal
01d4 t _event_loop_base_can_be_deleted
0054 t _event_loop_base_class_init
0138 t _event_loop_base_complete
0260 t _event_loop_base_get_param
0400 s _event_loop_base_info
001c t _event_loop_base_instance_init
02c4 t _event_loop_base_set_param
 U _object_class_dynamic_cast_assert
 U _object_class_property_add
 U _object_dynamic_cast_assert
 U _object_get_class
 U _register_module_init
0010 t _register_types
0600 d _thread_pool_max_info
05f0 d _thread_pool_min_info
 U _type_register_static
 U _visit_type_int64
0468 s l_.str
0478 s l_.str.1
05a6 s l_.str.10
047f s l_.str.2
048e s l_.str.3
04d9 s l_.str.4
04e7 s l_.str.5
04eb s l_.str.6
04fb s l_.str.7
050b s l_.str.8
0574 s l_.str.9
04c9 s l___func__.EVENT_LOOP_BASE
055a s l___func__.EVENT_LOOP_BASE_GET_CLASS
0545 s l___func__.USER_CREATABLE_CLASS
058c s l___func__.event_loop_base_set_param
 t ltmp0
0400 s ltmp1
0468 s ltmp2
05d0 d ltmp3
0610 s ltmp4
2b78 s ltmp5

Maybe smth missing on the meson side? (+Paolo)

$ git-diff 554623226..178bacb66 meson.build
...
+event_loop_base = files('event-loop-base.c')
+event_loop_base = static_library('event-loop-base', sources:
event_loop_base + genh,
+ build_by_default: true)
+event_loop_base = declare_dependency(link_whole: event_loop_base,
+ dependencies: [qom])
...

Any clue?

Re: [RFC PATCH 17/17] hw/sd: Subtract bootarea size from blk

[Philippe Mathieu-Daudé via]

Hi Joel,

On 18/3/22 14:28, Cédric Le Goater wrote:

From: Joel Stanley 

The userdata size is derived from the file the user passes on the
command line, but we must take into account the boot areas.

Signed-off-by: Joel Stanley 
Signed-off-by: Cédric Le Goater 
---
  hw/sd/sd.c | 6 ++
  1 file changed, 6 insertions(+)



@@ -655,6 +656,11 @@ static void sd_reset(DeviceState *dev)
  }
  size = sect << 9;
  
+if (sc->bootpart_offset) {

+unsigned int boot_capacity = sd->ext_csd[EXT_CSD_BOOT_MULT] << 17;


What about adding a tiny helper?

static unsigned sd_boot_capacity_bytes(SDState *sd)
{
return sd->ext_csd[EXT_CSD_BOOT_MULT] << 18;
}


+size -= boot_capacity * 2;
+}
+
  sect = sd_addr_to_wpnum(size) + 1;
  
  sd->state = sd_idle_state;

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Philippe Mathieu-Daudé via]

Hi Cédric,

On 18/3/22 14:28, Cédric Le Goater wrote:

The initial eMMC support from Vincent Palatin was largely reworked to
match the current SD framework. The parameters mimick a real 4GB eMMC,
but it can be set to various sizes.

This adds a new QOM object class for EMMC devices.

Signed-off-by: Vincent Palatin 
Link: 
https://lore.kernel.org/r/1311635951-11047-5-git-send-email-vpala...@chromium.org
[ jms: - Forward ported to QEMU 5.2 ]
Signed-off-by: Joel Stanley 
[ clg: - ported on aspeed-7.0 patchset
- HPI activation ]
Signed-off-by: Cédric Le Goater 
---
  hw/sd/sdmmc-internal.h |  97 +++
  include/hw/sd/sd.h |   9 ++
  hw/sd/sd.c | 205 -
  hw/sd/sdmmc-internal.c |   2 +-
  4 files changed, 311 insertions(+), 2 deletions(-)


Do you mind splitting as:

- Add TYPE_EMMC, emmc_class_init and sd_proto_emmc[] with
  already existing handlers (1 patch)

- Add new handlers, from smaller to sd_emmc_set_csd(),
  and finally mmc_set_ext_csd() with the EXT_CSD definitions
  (various patches).

Otherwise LGTM!

What is your test suite?

Thanks,

Phil.

Re: [RFC PATCH 07/17] hw/sd: Add sd_cmd_SEND_OP_CMD() handler

[Philippe Mathieu-Daudé via]

On 18/3/22 14:28, Cédric Le Goater wrote:

From: Philippe Mathieu-Daudé 

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20210624142209.1193073-9-f4...@amsat.org>
Signed-off-by: Cédric Le Goater 
---
  hw/sd/sd.c | 18 +-
  1 file changed, 9 insertions(+), 9 deletions(-)



@@ -2111,6 +2109,7 @@ static const SDProto sd_proto_spi = {
  .name = "SPI",
  .cmd = {
  [0] = sd_cmd_GO_IDLE_STATE,
+[1] = sd_cmd_SEND_OP_CMD,
  [2 ... 4]   = sd_cmd_illegal,
  [5] = sd_cmd_illegal,
  [7] = sd_cmd_illegal,
@@ -2120,6 +2119,7 @@ static const SDProto sd_proto_spi = {
  },
  .cmd = {
  [6] = sd_cmd_unimplemented,
+[41]= sd_cmd_SEND_OP_CMD,
  },
  };
  


I missed adding the cmd_abbrev[1] entry.

Re: [RFC PATCH 10/17] hw/sd: Add sd_cmd_SEND_TUNING_BLOCK() handler

[Philippe Mathieu-Daudé via]

On 18/3/22 14:28, Cédric Le Goater wrote:

From: Joel Stanley 

Signed-off-by: Joel Stanley 
Signed-off-by: Cédric Le Goater 
---
  hw/sd/sd.c | 28 +---
  1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 25e86c893bba..602ed6eb0701 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1042,6 +1042,22 @@ static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState 
*sd, SDRequest req)
  }
  }
  
+static sd_rsp_type_t sd_cmd_SEND_TUNING_BLOCK(SDState *sd, SDRequest req)

+{
+if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
+return sd_invalid_state_for_cmd(sd, req);


There was a bug here, this should be:

   return sd_illegal;

(see 
https://lore.kernel.org/qemu-devel/20220509141320.98374-1-philippe.mathieu.da...@gmail.com/. 
I can fix upon applying).


Reviewed-by: Philippe Mathieu-Daudé 


+}
+
+if (sd->state != sd_transfer_state) {
+return sd_invalid_state_for_cmd(sd, req);
+}
+
+sd->state = sd_sendingdata_state;
+sd->data_offset = 0;
+
+return sd_r1;
+}
+
  static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
  {
  uint32_t rca = 0x;
@@ -1285,17 +1301,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, 
SDRequest req)
  }
  break;
  
-case 19:/* CMD19: SEND_TUNING_BLOCK (SD) */

-if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
-break;
-}
-if (sd->state == sd_transfer_state) {
-sd->state = sd_sendingdata_state;
-sd->data_offset = 0;
-return sd_r1;
-}
-break;
-
  case 23:/* CMD23: SET_BLOCK_COUNT */
  if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
  break;
@@ -2132,6 +2137,7 @@ static const SDProto sd_proto_sd = {
  [2] = sd_cmd_ALL_SEND_CID,
  [3] = sd_cmd_SEND_RELATIVE_ADDR,
  [5] = sd_cmd_illegal,
+[19]= sd_cmd_SEND_TUNING_BLOCK,
  [52 ... 54] = sd_cmd_illegal,
  [58]= sd_cmd_illegal,
  [59]= sd_cmd_illegal,

Re: [PATCH 2/2] .gitlab-ci.d: export meson testlog.txt as an artifact

[Philippe Mathieu-Daudé via]

On 9/5/22 14:41, Daniel P. Berrangé wrote:

When running 'make check' we only get a summary of progress on the
console. Fortunately meson/ninja have saved the raw test output to a
logfile. Exposing this log will make it easier to debug failures that
happen in CI.

Signed-off-by: Daniel P. Berrangé 
---
  .gitlab-ci.d/buildtest-template.yml | 12 ++--
  1 file changed, 10 insertions(+), 2 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v7 00/22] host: Support macOS 12

[Philippe Mathieu-Daudé via]

On 3/5/22 11:40, Claudio Fontana wrote:

On 3/7/22 12:17 AM, Philippe Mathieu-Daudé wrote:

From: Philippe Mathieu-Daudé 

Few patches to be able to build QEMU on macOS 12 (Monterey).

Missing review:
  0006-hvf-Fix-OOB-write-in-RDTSCP-instruction-decode.patch
  0013-osdep-Avoid-using-Clang-specific-__builtin_available.patch
  0014-meson-Resolve-the-entitlement.sh-script-once-for-goo.patch
  0015-meson-Log-QEMU_CXXFLAGS-content-in-summary.patch
  0016-configure-Pass-filtered-QEMU_OBJCFLAGS-to-meson.patch
  0017-ui-cocoa-Constify-qkeycode-translation-arrays.patch
  0020-ui-cocoa-capture-all-keys-and-combos-when-mouse-is-g.patch
  0021-ui-cocoa-add-option-to-swap-Option-and-Command.patch
  0022-gitlab-ci-Support-macOS-12-via-cirrus-run.patch

Since v6:
- Dropped merged patches
- Addressed Akihiko Odaki comments (squashed 2 patches, added R/T-b)
- Dropped 'configure: Disable out-of-line atomic operations on Aarch64'
- Add few macos patches on the list pending for 7.0 so tested by CI



Hi Philippe, I did not find v6 somehow,

and I was looking for patch:

"[PATCH v5 06/16] hvf: Enable RDTSCP support"

was it dropped / merged with something else? I do not see it in latest git, nor 
in this respin,
maybe it is in your tree somewhere?


The patch stayed unreviewed during 2 months, so I dropped it.

Now it got at least a Tested-by tag from Silvio, I'll include it in the
next PR.

Regards,

Phil.

Re: [PATCH-for-7.0 v2] block-qdict: Fix -Werror=maybe-uninitialized build failure

[Philippe Mathieu-Daudé via]

On Wed, Mar 16, 2022 at 3:52 PM Markus Armbruster  wrote:
>
> Murilo Opsfelder Araújo  writes:
>
> > Hi, Philippe.
> >
> > On Monday, March 14, 2022 10:47:11 AM -03 Philippe Mathieu-Daudé wrote:
> >> On 11/3/22 23:16, Murilo Opsfelder Araujo wrote:
> >> > Building QEMU on Fedora 37 (Rawhide Prerelease) ppc64le failed with the
> >> > following error:
> >> >
> >> >  $ ../configure --prefix=/usr/local/qemu-disabletcg 
> >> > --target-list=ppc-softmmu,ppc64-softmmu --disable-tcg 
> >> > --disable-linux-user
> >> >  ...
> >> >  $ make -j$(nproc)
> >> >  ...
> >> >  FAILED: libqemuutil.a.p/qobject_block-qdict.c.o
> >>
> >> This part >>>
> >>
> >> >  cc -m64 -mlittle-endian -Ilibqemuutil.a.p -I. -I.. 
> >> > -Isubprojects/libvhost-user -I../subprojects/libvhost-user -Iqapi 
> >> > -Itrace -Iui -Iui/shader -I/usr/include/glib-2.0 
> >> > -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 -I/usr/include/lib
> >> >  mount -I/usr/include/blkid -I/usr/include/gio-unix-2.0 
> >> > -I/usr/include/p11-kit-1 -I/usr/include/pixman-1 
> >> > -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g 
> >> > -isystem /root/qemu/linux-headers -isystem linux-headers -iquote
> >> >   . -iquote /root/qemu -iquote /root/qemu/include -iquote 
> >> > /root/qemu/disas/libvixl -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 
> >> > -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE 
> >> > -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite
> >> >  -strings -Wmissing-prototypes -fno-strict-aliasing -fno-common 
> >> > -fwrapv -Wold-style-declaration -Wold-style-definition -Wtype-limits 
> >> > -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers 
> >> > -Wempty-body -Wnested-externs -Wendif-label
> >> >  s -Wexpansion-to-defined -Wimplicit-fallthrough=2 
> >> > -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi 
> >> > -fstack-protector-strong -fPIE -MD -MQ 
> >> > libqemuutil.a.p/qobject_block-qdict.c.o -MF 
> >> > libqemuutil.a.p/qobject_block-qdict.c.o.d -
> >> >  o libqemuutil.a.p/qobject_block-qdict.c.o -c 
> >> > ../qobject/block-qdict.c
> >>
> >> <<< is noise (doesn't provide any value) and could be stripped.
> >
> > Is this something the committer/maintainer could edit when applying the 
> > commit
> > or do you need I resend the v3?
> >
> > Cheers!
>
> I'll take care of it unless Kevin or Hanna beat me to the punch.

Thanks!

Same error on latest Debian:

include/qapi/qmp/qobject.h: In function 'qdict_array_split':
include/qapi/qmp/qobject.h:49:17: error: 'subqdict' may be used
uninitialized [-Werror=maybe-uninitialized]
   49 | typeof(obj) _obj = (obj);   \
  | ^~~~
../qobject/block-qdict.c:227:16: note: 'subqdict' declared here
  227 | QDict *subqdict;
  |^~~~
cc1: all warnings being treated as errors
FAILED: libqemuutil.a.p/qobject_block-qdict.c.o

Tested-by: Philippe Mathieu-Daudé 

Re: [PATCH v2] tests/qemu-iotests: Rework the checks and spots using GNU sed

[Philippe Mathieu-Daudé via]

On 16/2/22 13:54, Thomas Huth wrote:

Instead of failing the iotests if GNU sed is not available (or skipping
them completely in the check-block.sh script), it would be better to
simply skip the bash-based tests that rely on GNU sed, so that the other
tests could still be run. Thus we now explicitely use "gsed" (either as
direct program or as a wrapper around "sed" if it's the GNU version)
in the spots that rely on the GNU sed behavior. Statements that use the
"-r" parameter of sed have been switched to use "-E" instead, since this
switch is supported by all sed versions on our supported build hosts
(most also support "-r", but macOS' sed only supports "-E"). With all
these changes in place, we then can also remove the sed checks from the
check-block.sh script, so that "make check-block" can now be run on
systems without GNU sed, too.

Signed-off-by: Thomas Huth 
---
  I've checked that this still works fine with "make vm-build-freebsd",
  "make vm-build-netbsd" and "make vm-build-openbsd" and the Cirrus-CI
  macOS tasks.

  tests/check-block.sh | 12 --
  tests/qemu-iotests/271   |  2 +-
  tests/qemu-iotests/common.filter | 65 
  tests/qemu-iotests/common.rc | 45 +++---
  4 files changed, 57 insertions(+), 67 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 1/4] python/utils: add enboxify() text decoration utility

[Philippe Mathieu-Daudé via]

On 16/2/22 00:53, John Snow wrote:

On Tue, Feb 15, 2022 at 5:55 PM Eric Blake  wrote:


On Tue, Feb 15, 2022 at 05:08:50PM -0500, John Snow wrote:

print(enboxify(msg, width=72, name="commit message"))

┏━ commit message ━┓
┃ enboxify() takes a chunk of text and wraps it in a text art box that ┃
┃  adheres to a specified width. An optional title label may be given, ┃
┃  and any of the individual glyphs used to draw the box may be┃


Why do these two lines have a leading space,


┃ replaced or specified as well.   ┃


but this one doesn't?  It must be an off-by-one corner case when your
choice of space to wrap on is exactly at the wrap column.



Right, you're probably witnessing the right-pad *and* the actual space.


┗━━┛

Signed-off-by: John Snow 
---
  python/qemu/utils/__init__.py | 58 +++
  1 file changed, 58 insertions(+)



+def _wrap(line: str) -> str:
+return os.linesep.join([
+wrapped_line.ljust(lwidth) + suffix
+for wrapped_line in textwrap.wrap(
+line, width=lwidth, initial_indent=prefix,
+subsequent_indent=prefix, replace_whitespace=False,
+drop_whitespace=False, break_on_hyphens=False)


Always nice when someone else has written the cool library function to
do all the hard work for you ;)  But this is probably where you have the 
off-by-one I called out above.



Yeah, I just didn't want it to eat multiple spaces if they were
present -- I wanted it to reproduce them faithfully. The tradeoff is
some silliness near the margins.

Realistically, if I want something any better than what I've done
here, I should find a library to do it for me instead -- but for the
sake of highlighting some important information, this may be
just-enough-juice.


's/^┃  /┃ /' on top ;D

Re: [PATCH v5 10/16] audio/coreaudio: Remove a deprecation warning on macOS 12

[Philippe Mathieu-Daudé via]

On 15/2/22 07:53, Akihiko Odaki wrote:

On Tue, Feb 15, 2022 at 3:57 AM Philippe Mathieu-Daudé  wrote:


When building on macOS 12 we get:

   audio/coreaudio.c:50:5: error: 'kAudioObjectPropertyElementMaster' is 
deprecated: first deprecated in macOS 12.0 [-Werror,-Wdeprecated-declarations]
   kAudioObjectPropertyElementMaster
   ^
   kAudioObjectPropertyElementMain
   
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreAudio.framework/Headers/AudioHardwareBase.h:208:5:
 note: 'kAudioObjectPropertyElementMaster' has been explicitly marked 
deprecated here
   kAudioObjectPropertyElementMaster 
API_DEPRECATED_WITH_REPLACEMENT("kAudioObjectPropertyElementMain", macos(10.0, 
12.0), ios(2.0, 15.0), watchos(1.0, 8.0), tvos(9.0, 15.0)) = 
kAudioObjectPropertyElementMain
   ^

Replace by kAudioObjectPropertyElementMain, redefining it to
kAudioObjectPropertyElementMaster if not available.

Suggested-by: Akihiko Odaki 
Suggested-by: Christian Schoenebeck 
Suggested-by: Roman Bolshakov 
Reviewed-by: Christian Schoenebeck 
Signed-off-by: Philippe Mathieu-Daudé 
---
  audio/coreaudio.c | 17 +++--
  1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index d8a21d3e50..5b3aeaced0 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -44,10 +44,15 @@ typedef struct coreaudioVoiceOut {
  bool enabled;
  } coreaudioVoiceOut;

+#if !defined(MAC_OS_VERSION_12_0) \
+|| (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0)
+#define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster
+#endif
+


This still uses MAC_OS_X_VERSION_MAX_ALLOWED. Apparently the change to
use MAC_OS_X_VERSION_MIN_REQUIRED went to "[PATCH v5 16/16] gitlab-ci:
Support macOS 12 via cirrus-run".


Yet another failed rebase... Thanks for noticing it!

[PATCH v5 13/16] ui/cocoa: Add Services menu

[Philippe Mathieu-Daudé via]

From: Akihiko Odaki 

Services menu functionality of Cocoa is described at:
https://developer.apple.com/design/human-interface-guidelines/macos/extensions/services/

Signed-off-by: Akihiko Odaki 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Message-Id: <20220214091320.51750-1-akihiko.od...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 ui/cocoa.m | 4 
 1 file changed, 4 insertions(+)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 7a1ddd4075..becca58cb7 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1591,11 +1591,15 @@ static void create_initial_menus(void)
 NSMenuItem  *menuItem;
 
 [NSApp setMainMenu:[[NSMenu alloc] init]];
+[NSApp setServicesMenu:[[NSMenu alloc] initWithTitle:@"Services"]];
 
 // Application menu
 menu = [[NSMenu alloc] initWithTitle:@""];
 [menu addItemWithTitle:@"About QEMU" action:@selector(do_about_menu_item:) 
keyEquivalent:@""]; // About QEMU
 [menu addItem:[NSMenuItem separatorItem]]; //Separator
+menuItem = [menu addItemWithTitle:@"Services" action:nil 
keyEquivalent:@""];
+[menuItem setSubmenu:[NSApp servicesMenu]];
+[menu addItem:[NSMenuItem separatorItem]];
 [menu addItemWithTitle:@"Hide QEMU" action:@selector(hide:) 
keyEquivalent:@"h"]; //Hide QEMU
 menuItem = (NSMenuItem *)[menu addItemWithTitle:@"Hide Others" 
action:@selector(hideOtherApplications:) keyEquivalent:@"h"]; // Hide Others
 [menuItem 
setKeyEquivalentModifierMask:(NSEventModifierFlagOption|NSEventModifierFlagCommand)];
-- 
2.34.1

[PATCH v5 11/16] audio/dbus: Fix building with modules on macOS

[Philippe Mathieu-Daudé via]

When configuring QEMU with --enable-modules we get on macOS:

  --- stderr ---
  Dependency ui-dbus cannot be satisfied

ui-dbus depends on pixman and opengl, so add these dependencies
to audio-dbus.

Fixes: 739362d420 ("audio: add "dbus" audio backend")
Reviewed-by: Li Zhang 
Signed-off-by: Philippe Mathieu-Daudé 
---
 audio/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/audio/meson.build b/audio/meson.build
index 0ac3791d0b..d9b295514f 100644
--- a/audio/meson.build
+++ b/audio/meson.build
@@ -28,7 +28,7 @@ endforeach
 
 if dbus_display
 module_ss = ss.source_set()
-module_ss.add(when: gio, if_true: files('dbusaudio.c'))
+module_ss.add(when: [gio, pixman, opengl, 'CONFIG_GIO'], if_true: 
files('dbusaudio.c'))
 audio_modules += {'dbus': module_ss}
 endif
 
-- 
2.34.1

[PATCH v5 05/16] hvf: Fix OOB write in RDTSCP instruction decode

[Philippe Mathieu-Daudé via]

From: Cameron Esfahani 

A guest could craft a specific stream of instructions that will have QEMU
write 0xF9 to inappropriate locations in memory.  Add additional asserts
to check for this.  Generate a #UD if there are more than 14 prefix bytes.

Found by Julian Stecklina 

Signed-off-by: Cameron Esfahani 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/i386/hvf/x86_decode.c | 11 +--
 target/i386/hvf/x86hvf.c |  8 
 target/i386/hvf/x86hvf.h |  1 +
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/target/i386/hvf/x86_decode.c b/target/i386/hvf/x86_decode.c
index 062713b1a4..fbaf1813e8 100644
--- a/target/i386/hvf/x86_decode.c
+++ b/target/i386/hvf/x86_decode.c
@@ -24,6 +24,7 @@
 #include "vmx.h"
 #include "x86_mmu.h"
 #include "x86_descr.h"
+#include "x86hvf.h"
 
 #define OPCODE_ESCAPE   0xf
 
@@ -541,7 +542,8 @@ static void decode_lidtgroup(CPUX86State *env, struct 
x86_decode *decode)
 };
 decode->cmd = group[decode->modrm.reg];
 if (0xf9 == decode->modrm.modrm) {
-decode->opcode[decode->len++] = decode->modrm.modrm;
+VM_PANIC_ON(decode->opcode_len >= sizeof(decode->opcode));
+decode->opcode[decode->opcode_len++] = decode->modrm.modrm;
 decode->cmd = X86_DECODE_CMD_RDTSCP;
 }
 }
@@ -1847,7 +1849,8 @@ void calc_modrm_operand(CPUX86State *env, struct 
x86_decode *decode,
 
 static void decode_prefix(CPUX86State *env, struct x86_decode *decode)
 {
-while (1) {
+/* At most 14 prefix bytes. */
+for (int i = 0; i < 14; i++) {
 /*
  * REX prefix must come after legacy prefixes.
  * REX before legacy is ignored.
@@ -1892,6 +1895,8 @@ static void decode_prefix(CPUX86State *env, struct 
x86_decode *decode)
 return;
 }
 }
+/* Too many prefixes!  Generate #UD. */
+hvf_inject_ud(env);
 }
 
 void set_addressing_size(CPUX86State *env, struct x86_decode *decode)
@@ -2090,11 +2095,13 @@ static void decode_opcodes(CPUX86State *env, struct 
x86_decode *decode)
 uint8_t opcode;
 
 opcode = decode_byte(env, decode);
+VM_PANIC_ON(decode->opcode_len >= sizeof(decode->opcode));
 decode->opcode[decode->opcode_len++] = opcode;
 if (opcode != OPCODE_ESCAPE) {
 decode_opcode_1(env, decode, opcode);
 } else {
 opcode = decode_byte(env, decode);
+VM_PANIC_ON(decode->opcode_len >= sizeof(decode->opcode));
 decode->opcode[decode->opcode_len++] = opcode;
 decode_opcode_2(env, decode, opcode);
 }
diff --git a/target/i386/hvf/x86hvf.c b/target/i386/hvf/x86hvf.c
index 05ec1bddc4..a805c125d9 100644
--- a/target/i386/hvf/x86hvf.c
+++ b/target/i386/hvf/x86hvf.c
@@ -425,6 +425,14 @@ bool hvf_inject_interrupts(CPUState *cpu_state)
 & (CPU_INTERRUPT_INIT | CPU_INTERRUPT_TPR));
 }
 
+void hvf_inject_ud(CPUX86State *env)
+{
+env->exception_nr = EXCP06_ILLOP;
+env->exception_injected = 1;
+env->has_error_code = false;
+env->error_code = 0;
+}
+
 int hvf_process_events(CPUState *cpu_state)
 {
 X86CPU *cpu = X86_CPU(cpu_state);
diff --git a/target/i386/hvf/x86hvf.h b/target/i386/hvf/x86hvf.h
index 99ed8d608d..ef472a32f9 100644
--- a/target/i386/hvf/x86hvf.h
+++ b/target/i386/hvf/x86hvf.h
@@ -22,6 +22,7 @@
 
 int hvf_process_events(CPUState *);
 bool hvf_inject_interrupts(CPUState *);
+void hvf_inject_ud(CPUX86State *);
 void hvf_set_segment(struct CPUState *cpu, struct vmx_segment *vmx_seg,
  SegmentCache *qseg, bool is_tr);
 void hvf_get_segment(SegmentCache *qseg, struct vmx_segment *vmx_seg);
-- 
2.34.1

[PATCH v5 04/16] hvf: Use standard CR0 and CR4 register definitions

[Philippe Mathieu-Daudé via]

From: Cameron Esfahani 

No need to have our own definitions of these registers.

Signed-off-by: Cameron Esfahani 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/i386/hvf/vmx.h  | 17 +
 target/i386/hvf/x86.c  |  6 +++---
 target/i386/hvf/x86.h  | 34 --
 target/i386/hvf/x86_mmu.c  |  2 +-
 target/i386/hvf/x86_task.c |  3 ++-
 5 files changed, 15 insertions(+), 47 deletions(-)

diff --git a/target/i386/hvf/vmx.h b/target/i386/hvf/vmx.h
index 6df87116f6..29b7deed3c 100644
--- a/target/i386/hvf/vmx.h
+++ b/target/i386/hvf/vmx.h
@@ -124,10 +124,11 @@ static inline void macvm_set_cr0(hv_vcpuid_t vcpu, 
uint64_t cr0)
 uint64_t efer = rvmcs(vcpu, VMCS_GUEST_IA32_EFER);
 uint64_t old_cr0 = rvmcs(vcpu, VMCS_GUEST_CR0);
 uint64_t changed_cr0 = old_cr0 ^ cr0;
-uint64_t mask = CR0_PG | CR0_CD | CR0_NW | CR0_NE | CR0_ET;
+uint64_t mask = CR0_PG_MASK | CR0_CD_MASK | CR0_NW_MASK |
+CR0_NE_MASK | CR0_ET_MASK;
 uint64_t entry_ctls;
 
-if ((cr0 & CR0_PG) && (rvmcs(vcpu, VMCS_GUEST_CR4) & CR4_PAE) &&
+if ((cr0 & CR0_PG_MASK) && (rvmcs(vcpu, VMCS_GUEST_CR4) & CR4_PAE_MASK) &&
 !(efer & MSR_EFER_LME)) {
 address_space_read(&address_space_memory,
rvmcs(vcpu, VMCS_GUEST_CR3) & ~0x1f,
@@ -142,8 +143,8 @@ static inline void macvm_set_cr0(hv_vcpuid_t vcpu, uint64_t 
cr0)
 wvmcs(vcpu, VMCS_CR0_SHADOW, cr0);
 
 if (efer & MSR_EFER_LME) {
-if (changed_cr0 & CR0_PG) {
-if (cr0 & CR0_PG) {
+if (changed_cr0 & CR0_PG_MASK) {
+if (cr0 & CR0_PG_MASK) {
 enter_long_mode(vcpu, cr0, efer);
 } else {
 exit_long_mode(vcpu, cr0, efer);
@@ -155,8 +156,8 @@ static inline void macvm_set_cr0(hv_vcpuid_t vcpu, uint64_t 
cr0)
 }
 
 /* Filter new CR0 after we are finished examining it above. */
-cr0 = (cr0 & ~(mask & ~CR0_PG));
-wvmcs(vcpu, VMCS_GUEST_CR0, cr0 | CR0_NE | CR0_ET);
+cr0 = (cr0 & ~(mask & ~CR0_PG_MASK));
+wvmcs(vcpu, VMCS_GUEST_CR0, cr0 | CR0_NE_MASK | CR0_ET_MASK);
 
 hv_vcpu_invalidate_tlb(vcpu);
 hv_vcpu_flush(vcpu);
@@ -164,11 +165,11 @@ static inline void macvm_set_cr0(hv_vcpuid_t vcpu, 
uint64_t cr0)
 
 static inline void macvm_set_cr4(hv_vcpuid_t vcpu, uint64_t cr4)
 {
-uint64_t guest_cr4 = cr4 | CR4_VMXE;
+uint64_t guest_cr4 = cr4 | CR4_VMXE_MASK;
 
 wvmcs(vcpu, VMCS_GUEST_CR4, guest_cr4);
 wvmcs(vcpu, VMCS_CR4_SHADOW, cr4);
-wvmcs(vcpu, VMCS_CR4_MASK, CR4_VMXE);
+wvmcs(vcpu, VMCS_CR4_MASK, CR4_VMXE_MASK);
 
 hv_vcpu_invalidate_tlb(vcpu);
 hv_vcpu_flush(vcpu);
diff --git a/target/i386/hvf/x86.c b/target/i386/hvf/x86.c
index 2898bb70a8..91a3fe002c 100644
--- a/target/i386/hvf/x86.c
+++ b/target/i386/hvf/x86.c
@@ -119,7 +119,7 @@ bool x86_read_call_gate(struct CPUState *cpu, struct 
x86_call_gate *idt_desc,
 bool x86_is_protected(struct CPUState *cpu)
 {
 uint64_t cr0 = rvmcs(cpu->hvf->fd, VMCS_GUEST_CR0);
-return cr0 & CR0_PE;
+return cr0 & CR0_PE_MASK;
 }
 
 bool x86_is_real(struct CPUState *cpu)
@@ -150,13 +150,13 @@ bool x86_is_long64_mode(struct CPUState *cpu)
 bool x86_is_paging_mode(struct CPUState *cpu)
 {
 uint64_t cr0 = rvmcs(cpu->hvf->fd, VMCS_GUEST_CR0);
-return cr0 & CR0_PG;
+return cr0 & CR0_PG_MASK;
 }
 
 bool x86_is_pae_enabled(struct CPUState *cpu)
 {
 uint64_t cr4 = rvmcs(cpu->hvf->fd, VMCS_GUEST_CR4);
-return cr4 & CR4_PAE;
+return cr4 & CR4_PAE_MASK;
 }
 
 target_ulong linear_addr(struct CPUState *cpu, target_ulong addr, X86Seg seg)
diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
index 782664c2ea..947b98da41 100644
--- a/target/i386/hvf/x86.h
+++ b/target/i386/hvf/x86.h
@@ -42,40 +42,6 @@ typedef struct x86_register {
 };
 } __attribute__ ((__packed__)) x86_register;
 
-typedef enum x86_reg_cr0 {
-CR0_PE =(1L << 0),
-CR0_MP =(1L << 1),
-CR0_EM =(1L << 2),
-CR0_TS =(1L << 3),
-CR0_ET =(1L << 4),
-CR0_NE =(1L << 5),
-CR0_WP =(1L << 16),
-CR0_AM =(1L << 18),
-CR0_NW =(1L << 29),
-CR0_CD =(1L << 30),
-CR0_PG =(1L << 31),
-} x86_reg_cr0;
-
-typedef enum x86_reg_cr4 {
-CR4_VME =(1L << 0),
-CR4_PVI =(1L << 1),
-CR4_TSD =(1L << 2),
-CR4_DE  =(1L << 3),
-CR4_PSE =(1L << 4),
-CR4_PAE =(1L << 5),
-CR4_MSE =(1L << 6),
-CR4_PGE =(1L << 7),
-CR4_PCE =(1L << 8),
-CR4_OSFXSR = (1L << 9),
-CR4_OSXMMEXCPT = (1L << 10),
-CR4_VMXE =   (1L << 13),
-CR4_SMXE =   (1L << 14),
-CR4_FSGSBASE =   (1L << 16),
-CR4_PCIDE =  (1L << 17),
-CR4_OSXSAVE =(1L << 18

[PATCH v5 15/16] lcitool: refresh

[Philippe Mathieu-Daudé via]

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/docker/dockerfiles/ubuntu1804.docker | 2 --
 tests/docker/dockerfiles/ubuntu2004.docker | 2 --
 2 files changed, 4 deletions(-)

diff --git a/tests/docker/dockerfiles/ubuntu1804.docker 
b/tests/docker/dockerfiles/ubuntu1804.docker
index 699f2dfc6a..040938277a 100644
--- a/tests/docker/dockerfiles/ubuntu1804.docker
+++ b/tests/docker/dockerfiles/ubuntu1804.docker
@@ -65,7 +65,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
 libpam0g-dev \
 libpcre2-dev \
 libpixman-1-dev \
-libpmem-dev \
 libpng-dev \
 libpulse-dev \
 librbd-dev \
@@ -89,7 +88,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
 libvdeplug-dev \
 libvirglrenderer-dev \
 libvte-2.91-dev \
-libxen-dev \
 libzstd-dev \
 llvm \
 locales \
diff --git a/tests/docker/dockerfiles/ubuntu2004.docker 
b/tests/docker/dockerfiles/ubuntu2004.docker
index 87513125b8..159e7f60c9 100644
--- a/tests/docker/dockerfiles/ubuntu2004.docker
+++ b/tests/docker/dockerfiles/ubuntu2004.docker
@@ -66,7 +66,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
 libpam0g-dev \
 libpcre2-dev \
 libpixman-1-dev \
-libpmem-dev \
 libpng-dev \
 libpulse-dev \
 librbd-dev \
@@ -91,7 +90,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
 libvdeplug-dev \
 libvirglrenderer-dev \
 libvte-2.91-dev \
-libxen-dev \
 libzstd-dev \
 llvm \
 locales \
-- 
2.34.1

[PATCH v5 16/16] gitlab-ci: Support macOS 12 via cirrus-run

[Philippe Mathieu-Daudé via]

Add support for macOS 12 build on Cirrus-CI, similarly to commit
0e103a65ba1 ("gitlab: support for ... macOS 11 via cirrus-run"),
but with the following differences:
 - Enable modules (configure --enable-modules)
 - Do not run softfloat3 tests (make check-softfloat)
 - Run Aarch64 qtests instead of x86_64 ones

Generate the vars file by calling 'make lcitool-refresh'.

Signed-off-by: Philippe Mathieu-Daudé 
---
 .gitlab-ci.d/cirrus.yml   | 16 
 .gitlab-ci.d/cirrus/macos-12.vars | 16 
 audio/coreaudio.c |  2 +-
 tests/lcitool/refresh |  1 +
 4 files changed, 34 insertions(+), 1 deletion(-)
 create mode 100644 .gitlab-ci.d/cirrus/macos-12.vars

diff --git a/.gitlab-ci.d/cirrus.yml b/.gitlab-ci.d/cirrus.yml
index b96b22e269..be1dce5d4e 100644
--- a/.gitlab-ci.d/cirrus.yml
+++ b/.gitlab-ci.d/cirrus.yml
@@ -87,6 +87,22 @@ x64-macos-11-base-build:
 PKG_CONFIG_PATH: 
/usr/local/opt/curl/lib/pkgconfig:/usr/local/opt/ncurses/lib/pkgconfig:/usr/local/opt/readline/lib/pkgconfig
 TEST_TARGETS: check-unit check-block check-qapi-schema check-softfloat 
check-qtest-x86_64
 
+x64-macos-12-base-build:
+  extends: .cirrus_build_job
+  variables:
+NAME: macos-12
+CIRRUS_VM_INSTANCE_TYPE: osx_instance
+CIRRUS_VM_IMAGE_SELECTOR: image
+CIRRUS_VM_IMAGE_NAME: monterey-base
+CIRRUS_VM_CPUS: 12
+CIRRUS_VM_RAM: 24G
+UPDATE_COMMAND: brew update
+INSTALL_COMMAND: brew install
+PATH_EXTRA: /usr/local/opt/ccache/libexec:/usr/local/opt/gettext/bin
+PKG_CONFIG_PATH: 
/usr/local/opt/curl/lib/pkgconfig:/usr/local/opt/ncurses/lib/pkgconfig:/usr/local/opt/readline/lib/pkgconfig
+CONFIGURE_ARGS: --enable-modules
+TEST_TARGETS: check-unit check-block check-qapi-schema check-qtest-aarch64
+
 
 # The following jobs run VM-based tests via KVM on a Linux-based Cirrus-CI job
 .cirrus_kvm_job:
diff --git a/.gitlab-ci.d/cirrus/macos-12.vars 
b/.gitlab-ci.d/cirrus/macos-12.vars
new file mode 100644
index 00..a793258c64
--- /dev/null
+++ b/.gitlab-ci.d/cirrus/macos-12.vars
@@ -0,0 +1,16 @@
+# THIS FILE WAS AUTO-GENERATED
+#
+#  $ lcitool variables macos-12 qemu
+#
+# https://gitlab.com/libvirt/libvirt-ci
+
+CCACHE='/usr/local/bin/ccache'
+CPAN_PKGS='Test::Harness'
+CROSS_PKGS=''
+MAKE='/usr/local/bin/gmake'
+NINJA='/usr/local/bin/ninja'
+PACKAGING_COMMAND='brew'
+PIP3='/usr/local/bin/pip3'
+PKGS='bash bc bzip2 capstone ccache cpanminus ctags curl dbus diffutils dtc 
gcovr gettext git glib gnu-sed gnutls gtk+3 jemalloc jpeg-turbo libepoxy libffi 
libgcrypt libiscsi libnfs libpng libslirp libssh libtasn1 libusb llvm lzo make 
meson ncurses nettle ninja perl pixman pkg-config python3 rpm2cpio sdl2 
sdl2_image snappy sparse spice-protocol tesseract texinfo usbredir vde vte3 
zlib zstd'
+PYPI_PKGS='PyYAML numpy pillow sphinx sphinx-rtd-theme virtualenv'
+PYTHON='/usr/local/bin/python3'
diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index 5b3aeaced0..1faef7fa7a 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -45,7 +45,7 @@ typedef struct coreaudioVoiceOut {
 } coreaudioVoiceOut;
 
 #if !defined(MAC_OS_VERSION_12_0) \
-|| (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0)
+|| (MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_VERSION_12_0)
 #define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster
 #endif
 
diff --git a/tests/lcitool/refresh b/tests/lcitool/refresh
index 4ab90a310a..a714e2851d 100755
--- a/tests/lcitool/refresh
+++ b/tests/lcitool/refresh
@@ -89,6 +89,7 @@ try:
generate_cirrus("freebsd-12")
generate_cirrus("freebsd-13")
generate_cirrus("macos-11")
+   generate_cirrus("macos-12")
 
sys.exit(0)
 except Exception as ex:
-- 
2.34.1

[PATCH v5 14/16] ui/cocoa: Do not alert even without block devices

[Philippe Mathieu-Daudé via]

From: Akihiko Odaki 

Signed-off-by: Akihiko Odaki 
Message-Id: <20220213021418.2155-1-akihiko.od...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 ui/cocoa.m | 5 -
 1 file changed, 5 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index becca58cb7..6cadd43309 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1713,11 +1713,6 @@ static void addRemovableDevicesMenuItems(void)
 
 currentDevice = qmp_query_block(NULL);
 pointerToFree = currentDevice;
-if(currentDevice == NULL) {
-NSBeep();
-QEMU_Alert(@"Failed to query for block devices!");
-return;
-}
 
 menu = [[[NSApp mainMenu] itemWithTitle:@"Machine"] submenu];
 
-- 
2.34.1

[PATCH v5 08/16] hvf: Remove deprecated hv_vcpu_flush() calls

[Philippe Mathieu-Daudé via]

When building on macOS 11 [*], we get:

  In file included from ../target/i386/hvf/hvf.c:59:
  ../target/i386/hvf/vmx.h:174:5: error: 'hv_vcpu_flush' is deprecated: first 
deprecated in macOS 11.0 - This API has no effect and always returns 
HV_UNSUPPORTED [-Werror,-Wdeprecated-declarations]
  hv_vcpu_flush(vcpu);
  ^
  
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/Hypervisor.framework/Headers/hv.h:364:20:
 note: 'hv_vcpu_flush' has been explicitly marked deprecated here
  extern hv_return_t hv_vcpu_flush(hv_vcpuid_t vcpu)
 ^

Since this call "has no effect", simply remove it ¯\_(ツ)_/¯

Not very useful deprecation doc:
https://developer.apple.com/documentation/hypervisor/1441386-hv_vcpu_flush

[*] Also 10.15 (Catalina):
https://lore.kernel.org/qemu-devel/yd3dmsqz1sijw...@roolebo.dev/

Reviewed-by: Roman Bolshakov 
Tested-by: Roman Bolshakov 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/i386/hvf/vmx.h  | 2 --
 target/i386/hvf/x86_task.c | 1 -
 target/i386/hvf/x86hvf.c   | 2 --
 3 files changed, 5 deletions(-)

diff --git a/target/i386/hvf/vmx.h b/target/i386/hvf/vmx.h
index 29b7deed3c..573ddc33c0 100644
--- a/target/i386/hvf/vmx.h
+++ b/target/i386/hvf/vmx.h
@@ -160,7 +160,6 @@ static inline void macvm_set_cr0(hv_vcpuid_t vcpu, uint64_t 
cr0)
 wvmcs(vcpu, VMCS_GUEST_CR0, cr0 | CR0_NE_MASK | CR0_ET_MASK);
 
 hv_vcpu_invalidate_tlb(vcpu);
-hv_vcpu_flush(vcpu);
 }
 
 static inline void macvm_set_cr4(hv_vcpuid_t vcpu, uint64_t cr4)
@@ -172,7 +171,6 @@ static inline void macvm_set_cr4(hv_vcpuid_t vcpu, uint64_t 
cr4)
 wvmcs(vcpu, VMCS_CR4_MASK, CR4_VMXE_MASK);
 
 hv_vcpu_invalidate_tlb(vcpu);
-hv_vcpu_flush(vcpu);
 }
 
 static inline void macvm_set_rip(CPUState *cpu, uint64_t rip)
diff --git a/target/i386/hvf/x86_task.c b/target/i386/hvf/x86_task.c
index e1301599e9..d24daf6a41 100644
--- a/target/i386/hvf/x86_task.c
+++ b/target/i386/hvf/x86_task.c
@@ -182,5 +182,4 @@ void vmx_handle_task_switch(CPUState *cpu, 
x68_segment_selector tss_sel, int rea
 store_regs(cpu);
 
 hv_vcpu_invalidate_tlb(cpu->hvf->fd);
-hv_vcpu_flush(cpu->hvf->fd);
 }
diff --git a/target/i386/hvf/x86hvf.c b/target/i386/hvf/x86hvf.c
index cff59f3144..a338c207b7 100644
--- a/target/i386/hvf/x86hvf.c
+++ b/target/i386/hvf/x86hvf.c
@@ -125,8 +125,6 @@ static void hvf_put_segments(CPUState *cpu_state)
 
 hvf_set_segment(cpu_state, &seg, &env->ldt, false);
 vmx_write_segment_descriptor(cpu_state, &seg, R_LDTR);
-
-hv_vcpu_flush(cpu_state->hvf->fd);
 }
 
 void hvf_put_msrs(CPUState *cpu_state)
-- 
2.34.1

[PATCH v5 10/16] audio/coreaudio: Remove a deprecation warning on macOS 12

[Philippe Mathieu-Daudé via]

When building on macOS 12 we get:

  audio/coreaudio.c:50:5: error: 'kAudioObjectPropertyElementMaster' is 
deprecated: first deprecated in macOS 12.0 [-Werror,-Wdeprecated-declarations]
  kAudioObjectPropertyElementMaster
  ^
  kAudioObjectPropertyElementMain
  
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreAudio.framework/Headers/AudioHardwareBase.h:208:5:
 note: 'kAudioObjectPropertyElementMaster' has been explicitly marked 
deprecated here
  kAudioObjectPropertyElementMaster 
API_DEPRECATED_WITH_REPLACEMENT("kAudioObjectPropertyElementMain", macos(10.0, 
12.0), ios(2.0, 15.0), watchos(1.0, 8.0), tvos(9.0, 15.0)) = 
kAudioObjectPropertyElementMain
  ^

Replace by kAudioObjectPropertyElementMain, redefining it to
kAudioObjectPropertyElementMaster if not available.

Suggested-by: Akihiko Odaki 
Suggested-by: Christian Schoenebeck 
Suggested-by: Roman Bolshakov 
Reviewed-by: Christian Schoenebeck 
Signed-off-by: Philippe Mathieu-Daudé 
---
 audio/coreaudio.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index d8a21d3e50..5b3aeaced0 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -44,10 +44,15 @@ typedef struct coreaudioVoiceOut {
 bool enabled;
 } coreaudioVoiceOut;
 
+#if !defined(MAC_OS_VERSION_12_0) \
+|| (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0)
+#define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster
+#endif
+
 static const AudioObjectPropertyAddress voice_addr = {
 kAudioHardwarePropertyDefaultOutputDevice,
 kAudioObjectPropertyScopeGlobal,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
 };
 
 static OSStatus coreaudio_get_voice(AudioDeviceID *id)
@@ -69,7 +74,7 @@ static OSStatus coreaudio_get_framesizerange(AudioDeviceID id,
 AudioObjectPropertyAddress addr = {
 kAudioDevicePropertyBufferFrameSizeRange,
 kAudioDevicePropertyScopeOutput,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
 };
 
 return AudioObjectGetPropertyData(id,
@@ -86,7 +91,7 @@ static OSStatus coreaudio_get_framesize(AudioDeviceID id, 
UInt32 *framesize)
 AudioObjectPropertyAddress addr = {
 kAudioDevicePropertyBufferFrameSize,
 kAudioDevicePropertyScopeOutput,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
 };
 
 return AudioObjectGetPropertyData(id,
@@ -103,7 +108,7 @@ static OSStatus coreaudio_set_framesize(AudioDeviceID id, 
UInt32 *framesize)
 AudioObjectPropertyAddress addr = {
 kAudioDevicePropertyBufferFrameSize,
 kAudioDevicePropertyScopeOutput,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
 };
 
 return AudioObjectSetPropertyData(id,
@@ -121,7 +126,7 @@ static OSStatus coreaudio_set_streamformat(AudioDeviceID id,
 AudioObjectPropertyAddress addr = {
 kAudioDevicePropertyStreamFormat,
 kAudioDevicePropertyScopeOutput,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
 };
 
 return AudioObjectSetPropertyData(id,
@@ -138,7 +143,7 @@ static OSStatus coreaudio_get_isrunning(AudioDeviceID id, 
UInt32 *result)
 AudioObjectPropertyAddress addr = {
 kAudioDevicePropertyDeviceIsRunning,
 kAudioDevicePropertyScopeOutput,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
 };
 
 return AudioObjectGetPropertyData(id,
-- 
2.34.1

[PATCH v5 09/16] block/file-posix: Remove a deprecation warning on macOS 12

[Philippe Mathieu-Daudé via]

When building on macOS 12 we get:

  block/file-posix.c:3335:18: warning: 'IOMasterPort' is deprecated: first 
deprecated in macOS 12.0 [-Wdeprecated-declarations]
  kernResult = IOMasterPort( MACH_PORT_NULL, &masterPort );
   ^~~~
   IOMainPort

Replace by IOMainPort, redefining it to IOMasterPort if not available.

Suggested-by: Akihiko Odaki 
Reviewed-by: Christian Schoenebeck 
Reviewed by: Cameron Esfahani 
Signed-off-by: Philippe Mathieu-Daudé 
---
 block/file-posix.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 1f1756e192..13393ad296 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -3319,17 +3319,23 @@ BlockDriver bdrv_file = {
 #if defined(__APPLE__) && defined(__MACH__)
 static kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath,
 CFIndex maxPathSize, int flags);
+
+#if !defined(MAC_OS_VERSION_12_0) \
+|| (MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_VERSION_12_0)
+#define IOMainPort IOMasterPort
+#endif
+
 static char *FindEjectableOpticalMedia(io_iterator_t *mediaIterator)
 {
 kern_return_t kernResult = KERN_FAILURE;
-mach_port_t masterPort;
+mach_port_t mainPort;
 CFMutableDictionaryRef  classesToMatch;
 const char *matching_array[] = {kIODVDMediaClass, kIOCDMediaClass};
 char *mediaType = NULL;
 
-kernResult = IOMasterPort( MACH_PORT_NULL, &masterPort );
+kernResult = IOMainPort(MACH_PORT_NULL, &mainPort);
 if ( KERN_SUCCESS != kernResult ) {
-printf( "IOMasterPort returned %d\n", kernResult );
+printf("IOMainPort returned %d\n", kernResult);
 }
 
 int index;
@@ -3342,7 +3348,7 @@ static char *FindEjectableOpticalMedia(io_iterator_t 
*mediaIterator)
 }
 CFDictionarySetValue(classesToMatch, CFSTR(kIOMediaEjectableKey),
  kCFBooleanTrue);
-kernResult = IOServiceGetMatchingServices(masterPort, classesToMatch,
+kernResult = IOServiceGetMatchingServices(mainPort, classesToMatch,
   mediaIterator);
 if (kernResult != KERN_SUCCESS) {
 error_report("Note: IOServiceGetMatchingServices returned %d",
-- 
2.34.1

[PATCH v5 12/16] ui/cocoa: Remove allowedFileTypes restriction in SavePanel

[Philippe Mathieu-Daudé via]

setAllowedFileTypes is deprecated in macOS 12.

Per Akihiko Odaki [*]:

  An image file, which is being chosen by the panel, can be a
  raw file and have a variety of file extensions and many are not
  covered by the provided list (e.g. "udf"). Other platforms like
  GTK can provide an option to open a file with an extension not
  listed, but Cocoa can't. It forces the user to rename the file
  to give an extension in the list. Moreover, Cocoa does not tell
  which extensions are in the list so the user needs to read the
  source code, which is pretty bad.

Since this code is harming the usability rather than improving it,
simply remove the [NSSavePanel allowedFileTypes:] call, fixing:

  [2789/6622] Compiling Objective-C object libcommon.fa.p/ui_cocoa.m.o
  ui/cocoa.m:1411:16: error: 'setAllowedFileTypes:' is deprecated: first 
deprecated in macOS 12.0 - Use -allowedContentTypes instead 
[-Werror,-Wdeprecated-declarations]
  [openPanel setAllowedFileTypes: supportedImageFileTypes];
 ^
  
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSSavePanel.h:215:49:
 note: property 'allowedFileTypes' is declared deprecated here
  @property (nullable, copy) NSArray *allowedFileTypes 
API_DEPRECATED("Use -allowedContentTypes instead", macos(10.3,12.0));
  ^
  
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSSavePanel.h:215:49:
 note: 'setAllowedFileTypes:' has been explicitly marked deprecated here
  FAILED: libcommon.fa.p/ui_cocoa.m.o

[*] 
https://lore.kernel.org/qemu-devel/4dde2e66-63cb-4390-9538-c032310db...@gmail.com/

Suggested-by: Akihiko Odaki 
Reviewed-by: Roman Bolshakov 
Tested-by: Roman Bolshakov 
Reviewed-by: Christian Schoenebeck 
Reviewed by: Cameron Esfahani 
Signed-off-by: Philippe Mathieu-Daudé 
---
 ui/cocoa.m | 6 --
 1 file changed, 6 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index ac18e14ce0..7a1ddd4075 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -100,7 +100,6 @@ static int gArgc;
 static char **gArgv;
 static bool stretch_video;
 static NSTextField *pauseLabel;
-static NSArray * supportedImageFileTypes;
 
 static QemuSemaphore display_init_sem;
 static QemuSemaphore app_started_sem;
@@ -1168,10 +1167,6 @@ QemuCocoaView *cocoaView;
 [pauseLabel setTextColor: [NSColor blackColor]];
 [pauseLabel sizeToFit];
 
-// set the supported image file types that can be opened
-supportedImageFileTypes = [NSArray arrayWithObjects: @"img", @"iso", 
@"dmg",
- @"qcow", @"qcow2", @"cloop", @"vmdk", @"cdr",
-  @"toast", nil];
 [self make_about_window];
 }
 return self;
@@ -1414,7 +1409,6 @@ QemuCocoaView *cocoaView;
 openPanel = [NSOpenPanel openPanel];
 [openPanel setCanChooseFiles: YES];
 [openPanel setAllowsMultipleSelection: NO];
-[openPanel setAllowedFileTypes: supportedImageFileTypes];
 if([openPanel runModal] == NSModalResponseOK) {
 NSString * file = [[[openPanel URLs] objectAtIndex: 0] path];
 if(file == nil) {
-- 
2.34.1

[PATCH v5 07/16] hvf: Make hvf_get_segments() / hvf_put_segments() local

[Philippe Mathieu-Daudé via]

Both hvf_get_segments/hvf_put_segments() functions are only
used within x86hvf.c: do not declare them as public API.

Reviewed-by: Roman Bolshakov 
Tested-by: Roman Bolshakov 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/i386/hvf/x86hvf.c | 4 ++--
 target/i386/hvf/x86hvf.h | 2 --
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/target/i386/hvf/x86hvf.c b/target/i386/hvf/x86hvf.c
index a805c125d9..cff59f3144 100644
--- a/target/i386/hvf/x86hvf.c
+++ b/target/i386/hvf/x86hvf.c
@@ -83,7 +83,7 @@ void hvf_put_xsave(CPUState *cpu_state)
 }
 }
 
-void hvf_put_segments(CPUState *cpu_state)
+static void hvf_put_segments(CPUState *cpu_state)
 {
 CPUX86State *env = &X86_CPU(cpu_state)->env;
 struct vmx_segment seg;
@@ -166,7 +166,7 @@ void hvf_get_xsave(CPUState *cpu_state)
 x86_cpu_xrstor_all_areas(X86_CPU(cpu_state), xsave, xsave_len);
 }
 
-void hvf_get_segments(CPUState *cpu_state)
+static void hvf_get_segments(CPUState *cpu_state)
 {
 CPUX86State *env = &X86_CPU(cpu_state)->env;
 
diff --git a/target/i386/hvf/x86hvf.h b/target/i386/hvf/x86hvf.h
index ef472a32f9..427cdc1c13 100644
--- a/target/i386/hvf/x86hvf.h
+++ b/target/i386/hvf/x86hvf.h
@@ -27,11 +27,9 @@ void hvf_set_segment(struct CPUState *cpu, struct 
vmx_segment *vmx_seg,
  SegmentCache *qseg, bool is_tr);
 void hvf_get_segment(SegmentCache *qseg, struct vmx_segment *vmx_seg);
 void hvf_put_xsave(CPUState *cpu_state);
-void hvf_put_segments(CPUState *cpu_state);
 void hvf_put_msrs(CPUState *cpu_state);
 void hvf_get_xsave(CPUState *cpu_state);
 void hvf_get_msrs(CPUState *cpu_state);
 void vmx_clear_int_window_exiting(CPUState *cpu);
-void hvf_get_segments(CPUState *cpu_state);
 void vmx_update_tpr(CPUState *cpu);
 #endif
-- 
2.34.1

[PATCH v5 06/16] hvf: Enable RDTSCP support

[Philippe Mathieu-Daudé via]

From: Cameron Esfahani 

Pass through RDPID and RDTSCP support in CPUID if host supports it.
Correctly detect if CPU_BASED_TSC_OFFSET and CPU_BASED2_RDTSCP would
be supported in primary and secondary processor-based VM-execution
controls.  Enable RDTSCP in secondary processor controls if RDTSCP
support is indicated in CPUID.

Signed-off-by: Cameron Esfahani 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/i386/hvf/hvf.c   | 26 +-
 target/i386/hvf/vmcs.h  |  3 ++-
 target/i386/hvf/x86_cpuid.c |  7 ---
 3 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 4ba6e82fab..4712fe66d4 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -221,6 +221,7 @@ int hvf_arch_init_vcpu(CPUState *cpu)
 {
 X86CPU *x86cpu = X86_CPU(cpu);
 CPUX86State *env = &x86cpu->env;
+uint64_t reqCap;
 
 init_emu();
 init_decoder();
@@ -257,19 +258,26 @@ int hvf_arch_init_vcpu(CPUState *cpu)
 /* set VMCS control fields */
 wvmcs(cpu->hvf->fd, VMCS_PIN_BASED_CTLS,
   cap2ctrl(hvf_state->hvf_caps->vmx_cap_pinbased,
-  VMCS_PIN_BASED_CTLS_EXTINT |
-  VMCS_PIN_BASED_CTLS_NMI |
-  VMCS_PIN_BASED_CTLS_VNMI));
+   VMCS_PIN_BASED_CTLS_EXTINT |
+   VMCS_PIN_BASED_CTLS_NMI |
+   VMCS_PIN_BASED_CTLS_VNMI));
 wvmcs(cpu->hvf->fd, VMCS_PRI_PROC_BASED_CTLS,
   cap2ctrl(hvf_state->hvf_caps->vmx_cap_procbased,
-  VMCS_PRI_PROC_BASED_CTLS_HLT |
-  VMCS_PRI_PROC_BASED_CTLS_MWAIT |
-  VMCS_PRI_PROC_BASED_CTLS_TSC_OFFSET |
-  VMCS_PRI_PROC_BASED_CTLS_TPR_SHADOW) |
+   VMCS_PRI_PROC_BASED_CTLS_HLT |
+   VMCS_PRI_PROC_BASED_CTLS_MWAIT |
+   VMCS_PRI_PROC_BASED_CTLS_TSC_OFFSET |
+   VMCS_PRI_PROC_BASED_CTLS_TPR_SHADOW) |
   VMCS_PRI_PROC_BASED_CTLS_SEC_CONTROL);
+
+reqCap = VMCS_PRI_PROC_BASED2_CTLS_APIC_ACCESSES;
+
+/* Is RDTSCP support in CPUID?  If so, enable it in the VMCS. */
+if (hvf_get_supported_cpuid(0x8001, 0, R_EDX) & CPUID_EXT2_RDTSCP) {
+reqCap |= VMCS_PRI_PROC_BASED2_CTLS_RDTSCP;
+}
+
 wvmcs(cpu->hvf->fd, VMCS_SEC_PROC_BASED_CTLS,
-  cap2ctrl(hvf_state->hvf_caps->vmx_cap_procbased2,
-   VMCS_PRI_PROC_BASED2_CTLS_APIC_ACCESSES));
+  cap2ctrl(hvf_state->hvf_caps->vmx_cap_procbased2, reqCap));
 
 wvmcs(cpu->hvf->fd, VMCS_ENTRY_CTLS, 
cap2ctrl(hvf_state->hvf_caps->vmx_cap_entry,
   0));
diff --git a/target/i386/hvf/vmcs.h b/target/i386/hvf/vmcs.h
index 42de7ebc3a..bb4c764557 100644
--- a/target/i386/hvf/vmcs.h
+++ b/target/i386/hvf/vmcs.h
@@ -354,7 +354,7 @@
 #define VMCS_PRI_PROC_BASED_CTLS_TSC_OFFSET (1 << 3)
 #define VMCS_PRI_PROC_BASED_CTLS_HLT (1 << 7)
 #define VMCS_PRI_PROC_BASED_CTLS_MWAIT (1 << 10)
-#define VMCS_PRI_PROC_BASED_CTLS_TSC   (1 << 12)
+#define VMCS_PRI_PROC_BASED_CTLS_RDTSC (1 << 12)
 #define VMCS_PRI_PROC_BASED_CTLS_CR8_LOAD  (1 << 19)
 #define VMCS_PRI_PROC_BASED_CTLS_CR8_STORE (1 << 20)
 #define VMCS_PRI_PROC_BASED_CTLS_TPR_SHADOW(1 << 21)
@@ -362,6 +362,7 @@
 #define VMCS_PRI_PROC_BASED_CTLS_SEC_CONTROL   (1 << 31)
 
 #define VMCS_PRI_PROC_BASED2_CTLS_APIC_ACCESSES (1 << 0)
+#define VMCS_PRI_PROC_BASED2_CTLS_RDTSCP(1 << 3)
 #define VMCS_PRI_PROC_BASED2_CTLS_X2APIC(1 << 4)
 
 enum task_switch_reason {
diff --git a/target/i386/hvf/x86_cpuid.c b/target/i386/hvf/x86_cpuid.c
index 32b0d131df..b11ddaa349 100644
--- a/target/i386/hvf/x86_cpuid.c
+++ b/target/i386/hvf/x86_cpuid.c
@@ -96,7 +96,8 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx,
 ebx &= ~CPUID_7_0_EBX_INVPCID;
 }
 
-ecx &= CPUID_7_0_ECX_AVX512_VBMI | CPUID_7_0_ECX_AVX512_VPOPCNTDQ;
+ecx &= CPUID_7_0_ECX_AVX512_VBMI | CPUID_7_0_ECX_AVX512_VPOPCNTDQ |
+   CPUID_7_0_ECX_RDPID;
 edx &= CPUID_7_0_EDX_AVX512_4VNNIW | CPUID_7_0_EDX_AVX512_4FMAPS;
 } else {
 ebx = 0;
@@ -133,11 +134,11 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t 
idx,
 CPUID_FXSR | CPUID_EXT2_FXSR | CPUID_EXT2_PDPE1GB | 
CPUID_EXT2_3DNOWEXT |
 CPUID_EXT2_3DNOW | CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | 
CPUID_EXT2_NX;
 hv_vmx_read_capability(HV_VMX_CAP_PROCBASED2, &cap);
-if (!(cap & CPU_BASED2_RDTSCP)) {
+if (!(cap2ctrl(cap, CPU_BASED2_RDTSCP) & CPU_BASED2_RDTSCP)) {
 edx &= ~CPUID_EXT2_RDTSCP;
 }
 hv_vmx_read_capability(HV_VMX_CAP_PROCBASED, &cap);
-if (!(cap & CPU_BASED_TSC_OFFSET)) {
+if (!(cap2ctrl(cap, CPU_BASED_TSC_OFFSET) & CPU_BASED_TSC_OFFSET)) {
 edx &= ~CPUID_EXT2_RDTSCP;
 }
 ecx &= CPUID_EXT3_LAHF_LM | CPUID_EXT3_CMP_LEG | CPUID_EXT3_CR8LEG |
-- 
2.34.1

[PATCH v5 03/16] tests/fp/berkeley-testfloat-3: Ignore ignored #pragma directives

[Philippe Mathieu-Daudé via]

Since we already use -Wno-unknown-pragmas, we can also use
-Wno-ignored-pragmas. This silences hundred of warnings using
clang 13 on macOS Monterey:

  [409/771] Compiling C object 
tests/fp/libtestfloat.a.p/berkeley-testfloat-3_source_test_az_f128_rx.c.o
  ../tests/fp/berkeley-testfloat-3/source/test_az_f128_rx.c:49:14: warning: 
'#pragma FENV_ACCESS' is not supported on this target - ignored 
[-Wignored-pragmas]
  #pragma STDC FENV_ACCESS ON
   ^
  1 warning generated.

Having:

  $ cc -v
  Apple clang version 13.0.0 (clang-1300.0.29.30)

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/fp/meson.build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/fp/meson.build b/tests/fp/meson.build
index 59776a00a7..5192264a08 100644
--- a/tests/fp/meson.build
+++ b/tests/fp/meson.build
@@ -30,6 +30,7 @@ tfcflags = [
   '-Wno-implicit-fallthrough',
   '-Wno-strict-prototypes',
   '-Wno-unknown-pragmas',
+  '-Wno-ignored-pragmas',
   '-Wno-uninitialized',
   '-Wno-missing-prototypes',
   '-Wno-return-type',
-- 
2.34.1

[PATCH v5 01/16] MAINTAINERS: Add Akihiko Odaki to macOS-relateds

[Philippe Mathieu-Daudé via]

From: Akihiko Odaki 

Signed-off-by: Akihiko Odaki 
Reviewed-by: Christian Schoenebeck 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20220213021215.1974-1-akihiko.od...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b0b845f445..ce6f4f9228 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2406,6 +2406,7 @@ F: audio/alsaaudio.c
 Core Audio framework backend
 M: Gerd Hoffmann 
 R: Christian Schoenebeck 
+R: Akihiko Odaki 
 S: Odd Fixes
 F: audio/coreaudio.c
 
@@ -2658,6 +2659,7 @@ F: util/drm.c
 
 Cocoa graphics
 M: Peter Maydell 
+R: Akihiko Odaki 
 S: Odd Fixes
 F: ui/cocoa.m
 
-- 
2.34.1

[PATCH v5 02/16] configure: Allow passing extra Objective C compiler flags

[Philippe Mathieu-Daudé via]

We can pass C/CPP/LD flags via CFLAGS/CXXFLAGS/LDFLAGS environment
variables, or via configure --extra-cflags / --extra-cxxflags /
--extra-ldflags options. Provide similar behavior for Objective C:
use existing flags from $OBJCFLAGS, or passed via --extra-objcflags.

Signed-off-by: Philippe Mathieu-Daudé 
---
 configure   | 8 
 meson.build | 5 +
 2 files changed, 13 insertions(+)

diff --git a/configure b/configure
index 3a29eff5cc..06c03cebd3 100755
--- a/configure
+++ b/configure
@@ -287,6 +287,7 @@ done
 
 EXTRA_CFLAGS=""
 EXTRA_CXXFLAGS=""
+EXTRA_OBJCFLAGS=""
 EXTRA_LDFLAGS=""
 
 xen_ctrl_version="$default_feature"
@@ -391,9 +392,12 @@ for opt do
   --extra-cflags=*)
 EXTRA_CFLAGS="$EXTRA_CFLAGS $optarg"
 EXTRA_CXXFLAGS="$EXTRA_CXXFLAGS $optarg"
+EXTRA_OBJCFLAGS="$EXTRA_OBJCFLAGS $optarg"
 ;;
   --extra-cxxflags=*) EXTRA_CXXFLAGS="$EXTRA_CXXFLAGS $optarg"
   ;;
+  --extra-objcflags=*) EXTRA_OBJCFLAGS="$EXTRA_OBJCFLAGS $optarg"
+  ;;
   --extra-ldflags=*) EXTRA_LDFLAGS="$EXTRA_LDFLAGS $optarg"
   ;;
   --enable-debug-info) debug_info="yes"
@@ -774,6 +778,8 @@ for opt do
   ;;
   --extra-cxxflags=*)
   ;;
+  --extra-objcflags=*)
+  ;;
   --extra-ldflags=*)
   ;;
   --enable-debug-info)
@@ -1312,6 +1318,7 @@ Advanced options (experts only):
   --objcc=OBJCCuse Objective-C compiler OBJCC [$objcc]
   --extra-cflags=CFLAGSappend extra C compiler flags CFLAGS
   --extra-cxxflags=CXXFLAGS append extra C++ compiler flags CXXFLAGS
+  --extra-objcflags=OBJCFLAGS append extra Objective C compiler flags OBJCFLAGS
   --extra-ldflags=LDFLAGS  append extra linker flags LDFLAGS
   --cross-cc-ARCH=CC   use compiler when building ARCH guest test cases
   --cross-cc-cflags-ARCH=  use compiler flags when building ARCH guest tests
@@ -3724,6 +3731,7 @@ if test "$skip_meson" = no; then
   echo "[built-in options]" >> $cross
   echo "c_args = [$(meson_quote $CFLAGS $EXTRA_CFLAGS)]" >> $cross
   echo "cpp_args = [$(meson_quote $CXXFLAGS $EXTRA_CXXFLAGS)]" >> $cross
+  test -n "$objcc" && echo "objc_args = [$(meson_quote $OBJCFLAGS 
$EXTRA_OBJCFLAGS)]" >> $cross
   echo "c_link_args = [$(meson_quote $CFLAGS $LDFLAGS $EXTRA_CFLAGS 
$EXTRA_LDFLAGS)]" >> $cross
   echo "cpp_link_args = [$(meson_quote $CXXFLAGS $LDFLAGS $EXTRA_CXXFLAGS 
$EXTRA_LDFLAGS)]" >> $cross
   echo "[binaries]" >> $cross
diff --git a/meson.build b/meson.build
index ae5f7eec6e..df25e7a5e7 100644
--- a/meson.build
+++ b/meson.build
@@ -3292,6 +3292,11 @@ if link_language == 'cpp'
+ ['-O' + 
get_option('optimization')]
+ (get_option('debug') ? ['-g'] 
: []))}
 endif
+if targetos == 'darwin'
+  summary_info += {'OBJCFLAGS':   ' '.join(get_option('objc_args')
+   + ['-O' + 
get_option('optimization')]
+   + (get_option('debug') ? ['-g'] 
: []))}
+endif
 link_args = get_option(link_language + '_link_args')
 if link_args.length() > 0
   summary_info += {'LDFLAGS': ' '.join(link_args)}
-- 
2.34.1

[PATCH v5 00/16] host: Support macOS 12

[Philippe Mathieu-Daudé via]

Few patches to be able to build QEMU on macOS 12 (Monterey).

This basically consists of adapting deprecated APIs.

CI job added to avoid bitrotting.

Since v4:
- Use MAC_OS_X_VERSION_MIN_REQUIRED definition (Akihiko)
- Include patches from Akihiko

Since v3:
- Fix --enable-modules
- Ignore #pragma on softfloat3 tests
- Addressed Akihiko Odaki comments
- Include Cameron Esfahani patches

Since v2:
- Addressed Akihiko Odaki comments:
  . use __is_identifier(),
  . remove cocoa setAllowedFileTypes()
- Addressed Daniel Berrangé comment:
  . rebased on testing/next, update libvirt-ci/lcitool

Based on Alex's testing/next

Akihiko Odaki (3):
  MAINTAINERS: Add Akihiko Odaki to macOS-relateds
  ui/cocoa: Add Services menu
  ui/cocoa: Do not alert even without block devices

Cameron Esfahani (3):
  hvf: Use standard CR0 and CR4 register definitions
  hvf: Fix OOB write in RDTSCP instruction decode
  hvf: Enable RDTSCP support

Philippe Mathieu-Daudé (10):
  configure: Allow passing extra Objective C compiler flags
  tests/fp/berkeley-testfloat-3: Ignore ignored #pragma directives
  hvf: Make hvf_get_segments() / hvf_put_segments() local
  hvf: Remove deprecated hv_vcpu_flush() calls
  block/file-posix: Remove a deprecation warning on macOS 12
  audio/coreaudio: Remove a deprecation warning on macOS 12
  audio/dbus: Fix building with modules on macOS
  ui/cocoa: Remove allowedFileTypes restriction in SavePanel
  lcitool: refresh
  gitlab-ci: Support macOS 12 via cirrus-run

 .gitlab-ci.d/cirrus.yml| 16 ++
 .gitlab-ci.d/cirrus/macos-12.vars  | 16 ++
 MAINTAINERS|  2 ++
 audio/coreaudio.c  | 17 +++
 audio/meson.build  |  2 +-
 block/file-posix.c | 14 ++---
 configure  |  8 +
 meson.build|  5 
 target/i386/hvf/hvf.c  | 26 +++--
 target/i386/hvf/vmcs.h |  3 +-
 target/i386/hvf/vmx.h  | 19 ++--
 target/i386/hvf/x86.c  |  6 ++--
 target/i386/hvf/x86.h  | 34 --
 target/i386/hvf/x86_cpuid.c|  7 +++--
 target/i386/hvf/x86_decode.c   | 11 +--
 target/i386/hvf/x86_mmu.c  |  2 +-
 target/i386/hvf/x86_task.c |  4 +--
 target/i386/hvf/x86hvf.c   | 14 ++---
 target/i386/hvf/x86hvf.h   |  3 +-
 tests/docker/dockerfiles/ubuntu1804.docker |  2 --
 tests/docker/dockerfiles/ubuntu2004.docker |  2 --
 tests/fp/meson.build   |  1 +
 tests/lcitool/refresh  |  1 +
 ui/cocoa.m | 15 +++---
 24 files changed, 133 insertions(+), 97 deletions(-)
 create mode 100644 .gitlab-ci.d/cirrus/macos-12.vars

-- 
2.34.1

Re: [PATCH] ui/cocoa: Do not alert even without block devices

[Philippe Mathieu-Daudé via]

On 13/2/22 03:14, Akihiko Odaki wrote:

Signed-off-by: Akihiko Odaki 
---
  ui/cocoa.m | 5 -
  1 file changed, 5 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index ac18e14ce01..271a2676026 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1715,11 +1715,6 @@ static void addRemovableDevicesMenuItems(void)
  
  currentDevice = qmp_query_block(NULL);

  pointerToFree = currentDevice;
-if(currentDevice == NULL) {
-NSBeep();
-QEMU_Alert(@"Failed to query for block devices!");
-return;
-}
  
  menu = [[[NSApp mainMenu] itemWithTitle:@"Machine"] submenu];
  


Cc'ing qemu-block@ and Markus (QMP).

I always wondered the point of this annoying warning but never
found out.

Is this menu updated when removable blkdev are hot-plugged from
the monitor or QMP?

Tested-by: Philippe Mathieu-Daudé 

Re: bdrv_is_allocated

[Philippe Mathieu-Daudé via]

On 13/2/22 15:24, 沈梦姣 wrote:

Hi,
I’m trying to understand this function, but seems no note in the header file, 
could anyone help explain this function? It will be great if there is an 
example. Thanks in advance!

thanks


Cc'ing qemu-block@ list.

Re: [PATCH] hw/block/fdc-isa: Respect QOM properties when building AML

[Philippe Mathieu-Daudé via]

On 9/2/22 20:15, Bernhard Beschow wrote:

Other ISA devices such as serial-isa use the properties in their
build_aml functions. fdc-isa not using them is probably an oversight.

Signed-off-by: Bernhard Beschow 
---
  hw/block/fdc-isa.c | 11 +++
  1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/block/fdc-isa.c b/hw/block/fdc-isa.c
index 3bf64e0665..ab663dce93 100644
--- a/hw/block/fdc-isa.c
+++ b/hw/block/fdc-isa.c
@@ -216,6 +216,7 @@ int cmos_get_fd_drive_type(FloppyDriveType fd0)
  
  static void fdc_isa_build_aml(ISADevice *isadev, Aml *scope)

  {
+FDCtrlISABus *isa = ISA_FDC(isadev);
  Aml *dev;
  Aml *crs;
  int i;
@@ -227,11 +228,13 @@ static void fdc_isa_build_aml(ISADevice *isadev, Aml 
*scope)
  };
  
  crs = aml_resource_template();

-aml_append(crs, aml_io(AML_DECODE16, 0x03F2, 0x03F2, 0x00, 0x04));
-aml_append(crs, aml_io(AML_DECODE16, 0x03F7, 0x03F7, 0x00, 0x01));
-aml_append(crs, aml_irq_no_flags(6));
  aml_append(crs,
-aml_dma(AML_COMPATIBILITY, AML_NOTBUSMASTER, AML_TRANSFER8, 2));
+aml_io(AML_DECODE16, isa->iobase + 2, isa->iobase + 2, 0x00, 0x04));
+aml_append(crs,
+aml_io(AML_DECODE16, isa->iobase + 7, isa->iobase + 7, 0x00, 0x01));
+aml_append(crs, aml_irq_no_flags(isa->irq));
+aml_append(crs,
+aml_dma(AML_COMPATIBILITY, AML_NOTBUSMASTER, AML_TRANSFER8, isa->dma));
  
  dev = aml_device("FDC0");

  aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0700")));


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 1/6] tests/qemu-iotests: Improve the check for GNU sed

[Philippe Mathieu-Daudé via]

On 8/2/22 13:38, Thomas Huth wrote:

On 08/02/2022 13.28, Hanna Reitz wrote:

On 08.02.22 13:13, Thomas Huth wrote:

On 08/02/2022 12.46, Hanna Reitz wrote:

On 08.02.22 11:13, Thomas Huth wrote:
Instead of failing the iotests if GNU sed is not available (or 
skipping

them completely in the check-block.sh script), it would be better to
simply skip the bash-based tests, so that the python-based tests could
still be run. Thus add the check for BusyBox sed to common.rc and mark
the tests as "not run" if GNU sed is not available. Then we can also
remove the sed checks from the check-block.sh script.

Signed-off-by: Thomas Huth 
---
  tests/check-block.sh | 12 
  tests/qemu-iotests/common.rc | 26 +-
  2 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/tests/check-block.sh b/tests/check-block.sh
index 720a46bc36..af0c574812 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -52,18 +52,6 @@ if LANG=C bash --version | grep -q 'GNU bash, 
version [123]' ; then

  skip "bash version too old ==> Not running the qemu-iotests."
  fi
-if ! (sed --version | grep 'GNU sed') > /dev/null 2>&1 ; then


This specifically tests for `sed`, whereas...


There was a check for "gsed" one line later:

 if ! command -v gsed >/dev/null 2>&1; then

... so the check-block.sh script ran the iotests also if "sed" was 
not GNU, but gsed was available.


Oh, right.


[...]

diff --git a/tests/qemu-iotests/common.rc 
b/tests/qemu-iotests/common.rc

index 9885030b43..9ea504810c 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -17,17 +17,27 @@
  # along with this program.  If not, see 
.

  #
+# bail out, setting up .notrun file
+_notrun()
+{
+    echo "$*" >"$OUTPUT_DIR/$seq.notrun"
+    echo "$seq not run: $*"
+    status=0
+    exit
+}
+
+# We need GNU sed for the iotests. Make sure to not use BusyBox sed
+# which says that "This is not GNU sed version 4.0"
  SED=
  for sed in sed gsed; do
-    ($sed --version | grep 'GNU sed') > /dev/null 2>&1
+    ($sed --version | grep -v "not GNU sed" | grep 'GNU sed') > 
/dev/null 2>&1


...this will accept `gsed`, too.  The problem is that many bash 
iotests just use `sed` instead of `$SED`, so I think if we let this 
do the gatekeeping, then we should change this to just check for `sed`.


I think we should be fine - at least for the tests in the "auto" 
group. Otherwise we would have seen test failures on non-Linux 
systems like *BSD earlier already.


Makes sense, but I’m quite uncomfortable with this.


The current code with $SED has been introduced almost three years ago 
already...



  Can’t we just do `alias sed=gsed`?


Maybe ... but let's ask Philippe and Kevin first, who Signed-off commit 
bde36af1ab4f476 that introduced the current way with $SED: What's your 
opinion about this?


This commit was to have check-block working on the OpenBSD VM image.

Re: [PATCH 2/6] tests/qemu-iotests/meson.build: Improve the indentation

[Philippe Mathieu-Daudé via]

On 8/2/22 11:13, Thomas Huth wrote:

By using subdir_done(), we can get rid of one level of indentation
in this file. This will make it easier to add more conditions to
skip the iotests in future patches.

Signed-off-by: Thomas Huth 
---
  tests/qemu-iotests/meson.build | 61 ++
  1 file changed, 32 insertions(+), 29 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH] block/vvfat: Fix memleaks in vvfat_close()

[Philippe Mathieu-Daudé via]

On 7/2/22 12:37, Hanna Reitz wrote:

qcow_filename and used_clusters are allocated in enable_write_target(),
but freed only in the error path of vvfat_open().  Free them in
vvfat_close(), too.

Signed-off-by: Hanna Reitz 
---
  block/vvfat.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/block/vvfat.c b/block/vvfat.c
index b2b58d93b8..811ba76e30 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3213,6 +3213,8 @@ static void vvfat_close(BlockDriverState *bs)
  array_free(&(s->directory));
  array_free(&(s->mapping));
  g_free(s->cluster_buffer);
+g_free(s->qcow_filename);
+g_free(s->used_clusters);


Previously posted / reviewed :)

https://lore.kernel.org/qemu-devel/20210430162519.271607-2-phi...@redhat.com/
https://lore.kernel.org/qemu-devel/20210430162519.271607-3-phi...@redhat.com/

[PATCH v3 2/2] block/export/fuse: Fix build failure on FreeBSD

[Philippe Mathieu-Daudé via]

When building on FreeBSD we get:

  [816/6851] Compiling C object libblockdev.fa.p/block_export_fuse.c.o
  ../block/export/fuse.c:628:16: error: use of undeclared identifier 
'FALLOC_FL_KEEP_SIZE'
  if (mode & FALLOC_FL_KEEP_SIZE) {
 ^
  ../block/export/fuse.c:651:16: error: use of undeclared identifier 
'FALLOC_FL_PUNCH_HOLE'
  if (mode & FALLOC_FL_PUNCH_HOLE) {
 ^
  ../block/export/fuse.c:652:22: error: use of undeclared identifier 
'FALLOC_FL_KEEP_SIZE'
  if (!(mode & FALLOC_FL_KEEP_SIZE)) {
   ^
  3 errors generated.
  FAILED: libblockdev.fa.p/block_export_fuse.c.o

Meson indeed reported FALLOC_FL_PUNCH_HOLE is not available:

  C compiler for the host machine: cc (clang 10.0.1 "FreeBSD clang version 
10.0.1")
  Checking for function "fallocate" : NO
  Checking for function "posix_fallocate" : YES
  Header  has symbol "FALLOC_FL_PUNCH_HOLE" : NO
  Header  has symbol "FALLOC_FL_ZERO_RANGE" : NO
  ...

Similarly to commit 304332039 ("block/export/fuse.c: fix musl build"),
guard the code requiring FALLOC_FL_KEEP_SIZE / FALLOC_FL_PUNCH_HOLE
definitions under CONFIG_FALLOCATE_PUNCH_HOLE #ifdef'ry.

Signed-off-by: Philippe Mathieu-Daudé 
---
 block/export/fuse.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/block/export/fuse.c b/block/export/fuse.c
index d25e478c0a2..fdda8e3c818 100644
--- a/block/export/fuse.c
+++ b/block/export/fuse.c
@@ -625,9 +625,11 @@ static void fuse_fallocate(fuse_req_t req, fuse_ino_t 
inode, int mode,
 return;
 }
 
+#ifdef CONFIG_FALLOCATE_PUNCH_HOLE
 if (mode & FALLOC_FL_KEEP_SIZE) {
 length = MIN(length, blk_len - offset);
 }
+#endif /* CONFIG_FALLOCATE_PUNCH_HOLE */
 
 if (!mode) {
 /* We can only fallocate at the EOF with a truncate */
@@ -648,6 +650,7 @@ static void fuse_fallocate(fuse_req_t req, fuse_ino_t 
inode, int mode,
 ret = fuse_do_truncate(exp, offset + length, true,
PREALLOC_MODE_FALLOC);
 }
+#ifdef CONFIG_FALLOCATE_PUNCH_HOLE
 else if (mode & FALLOC_FL_PUNCH_HOLE) {
 if (!(mode & FALLOC_FL_KEEP_SIZE)) {
 fuse_reply_err(req, EINVAL);
@@ -662,6 +665,7 @@ static void fuse_fallocate(fuse_req_t req, fuse_ino_t 
inode, int mode,
 length -= size;
 } while (ret == 0 && length > 0);
 }
+#endif /* CONFIG_FALLOCATE_PUNCH_HOLE */
 #ifdef CONFIG_FALLOCATE_ZERO_RANGE
 else if (mode & FALLOC_FL_ZERO_RANGE) {
 if (!(mode & FALLOC_FL_KEEP_SIZE) && offset + length > blk_len) {
-- 
2.34.1

[PATCH v3 1/2] block/export/fuse: Rearrange if-else-if ladder in fuse_fallocate()

[Philippe Mathieu-Daudé via]

In order to safely maintain a mixture of #ifdef'ry with if-else-if
ladder, rearrange the last statement (!mode) first. Since it is
mutually exclusive with the other conditions, checking it first
doesn't make any logical difference, but allows to add #ifdef'ry
around in a more cleanly way.

Suggested-by: Kevin Wolf 
Signed-off-by: Philippe Mathieu-Daudé 
---

One minor checkpatch error:

 ERROR: else should follow close brace '}'
 #29: FILE: block/export/fuse.c:651:
 +}
 +else if (mode & FALLOC_FL_PUNCH_HOLE) {
---
 block/export/fuse.c | 41 +
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/block/export/fuse.c b/block/export/fuse.c
index 6710d8aed86..d25e478c0a2 100644
--- a/block/export/fuse.c
+++ b/block/export/fuse.c
@@ -629,7 +629,26 @@ static void fuse_fallocate(fuse_req_t req, fuse_ino_t 
inode, int mode,
 length = MIN(length, blk_len - offset);
 }
 
-if (mode & FALLOC_FL_PUNCH_HOLE) {
+if (!mode) {
+/* We can only fallocate at the EOF with a truncate */
+if (offset < blk_len) {
+fuse_reply_err(req, EOPNOTSUPP);
+return;
+}
+
+if (offset > blk_len) {
+/* No preallocation needed here */
+ret = fuse_do_truncate(exp, offset, true, PREALLOC_MODE_OFF);
+if (ret < 0) {
+fuse_reply_err(req, -ret);
+return;
+}
+}
+
+ret = fuse_do_truncate(exp, offset + length, true,
+   PREALLOC_MODE_FALLOC);
+}
+else if (mode & FALLOC_FL_PUNCH_HOLE) {
 if (!(mode & FALLOC_FL_KEEP_SIZE)) {
 fuse_reply_err(req, EINVAL);
 return;
@@ -665,25 +684,7 @@ static void fuse_fallocate(fuse_req_t req, fuse_ino_t 
inode, int mode,
 } while (ret == 0 && length > 0);
 }
 #endif /* CONFIG_FALLOCATE_ZERO_RANGE */
-else if (!mode) {
-/* We can only fallocate at the EOF with a truncate */
-if (offset < blk_len) {
-fuse_reply_err(req, EOPNOTSUPP);
-return;
-}
-
-if (offset > blk_len) {
-/* No preallocation needed here */
-ret = fuse_do_truncate(exp, offset, true, PREALLOC_MODE_OFF);
-if (ret < 0) {
-fuse_reply_err(req, -ret);
-return;
-}
-}
-
-ret = fuse_do_truncate(exp, offset + length, true,
-   PREALLOC_MODE_FALLOC);
-} else {
+else {
 ret = -EOPNOTSUPP;
 }
 
-- 
2.34.1

[PATCH v3 0/2] block/export/fuse: Fix build failure on FreeBSD

[Philippe Mathieu-Daudé via]

Since v2:
- Rearrange if-else-if ladder first (Kevin)

Philippe Mathieu-Daudé (2):
  block/export/fuse: Rearrange if-else-if ladder in fuse_fallocate()
  block/export/fuse: Fix build failure on FreeBSD

 block/export/fuse.c | 45 +
 1 file changed, 25 insertions(+), 20 deletions(-)

-- 
2.34.1

Re: [RFC PATCH] block/export/fuse: Fix build failure on FreeBSD

[Philippe Mathieu-Daudé via]

On 1/27/22 17:15, Kevin Wolf wrote:
> Am 22.01.2022 um 14:49 hat Philippe Mathieu-Daudé geschrieben:
>> When building on FreeBSD we get:
>>
>>   [816/6851] Compiling C object libblockdev.fa.p/block_export_fuse.c.o
>>   ../block/export/fuse.c:628:16: error: use of undeclared identifier 
>> 'FALLOC_FL_KEEP_SIZE'
>>   if (mode & FALLOC_FL_KEEP_SIZE) {
>>  ^
>>   ../block/export/fuse.c:632:16: error: use of undeclared identifier 
>> 'FALLOC_FL_PUNCH_HOLE'
>>   if (mode & FALLOC_FL_PUNCH_HOLE) {
>>  ^
>>   ../block/export/fuse.c:633:22: error: use of undeclared identifier 
>> 'FALLOC_FL_KEEP_SIZE'
>>   if (!(mode & FALLOC_FL_KEEP_SIZE)) {
>>^
>>   3 errors generated.
>>   FAILED: libblockdev.fa.p/block_export_fuse.c.o
>>
>> Meson indeed reported FALLOC_FL_PUNCH_HOLE is not available:
>>
>>   C compiler for the host machine: cc (clang 10.0.1 "FreeBSD clang version 
>> 10.0.1")
>>   Checking for function "fallocate" : NO
>>   Checking for function "posix_fallocate" : YES
>>   Header  has symbol "FALLOC_FL_PUNCH_HOLE" : NO
>>   Header  has symbol "FALLOC_FL_ZERO_RANGE" : NO
>>   ...
>>
>> Similarly to commit 304332039 ("block/export/fuse.c: fix musl build"),
>> guard the code requiring FALLOC_FL_KEEP_SIZE / FALLOC_FL_PUNCH_HOLE
>> definitions under CONFIG_FALLOCATE_PUNCH_HOLE #ifdef'ry.
>>
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>> Fragile #ifdef'ry... Any clever idea?
> 
> I guess we could reorder things. The last case (!mode) is mutually
> exclusive with the other conditions, so checking it first doesn't make a
> difference, and then you can #ifdef things out more cleanly.

This is indeed clever, thanks! v3 soon.

Re: [PATCH] block.h: remove outdated comment

[Philippe Mathieu-Daudé via]

On 31/1/22 13:56, Emanuele Giuseppe Esposito wrote:

The comment "disk I/O throttling" doesn't make any sense at all
any more. It was added in commit 0563e191516 to describe
bdrv_io_limits_enable()/disable(), which were removed in commit
97148076, so the comment is just a forgotten leftover.

Suggested-by: Kevin Wolf 
Signed-off-by: Emanuele Giuseppe Esposito 
---
  include/block/block.h | 1 -
  1 file changed, 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH] qapi/block: Cosmetic change in BlockExportType schema

[Philippe Mathieu-Daudé via]

On 31/1/22 22:37, Eric Blake wrote:

On Sun, Jan 30, 2022 at 07:50:41PM +0100, Philippe Mathieu-Daudé wrote:

On 28/1/22 21:54, Eric Blake wrote:

On Wed, Jan 19, 2022 at 01:14:39PM +0100, Philippe Mathieu-Daudé wrote:

From: Philippe Mathieu-Daude 


'git am' used this line to insert the authorship...



From: Philippe Mathieu-Daudé 


...then left this line in the commit body, which I manually deleted,
without spotting the difference between the two.



The doubled From: looks odd here.  I'll double-check that git doesn't
mess up the actual commit once I apply the patch.


I played with the git --from option to not appear in the list as
'"Philippe Mathieu-Daudé via" ':
https://lore.kernel.org/qemu-devel/efc5f304-f3d2-ff7b-99a6-673595ff0...@amsat.org/
by using a different sendemail.from (removing the acute in my
lastname) to force a correct author.from.
git-am should have picked the 2nd form, but I see the 1st in commit
3a8fa0edd1. Just curious, did you had to modify it manually?


Alas, since I managed to overlook the change in the acute (I suppose
I'm cursed with having a boring name, so unlike many list participants
who are overjoyed by the power of UTF-8 to make self-expression more
accurate, I have not had as much experience with thinking about it),
my manual edits explain why the merged commit ended up with a less
desirable spelling.  I apologize for the mishap.  Do we need/want a
.mailmap entry to aid git at listing your preferred spelling?


A missing acute is not a big deal, compared to other alphabets where
people try to approximate their name pronunciation to Latin symbols,
and is still better than UTF-8 mojibake :)