Re: [PATCH trivial] qemu-img: factor out parse_output_format() and use it in the code

[Michael Tokarev]

07.02.2024 20:52, Michael Tokarev :

Use common code and simplify error message


I've sent this as part of qemu-img --help/options refactoring
series, done in that context so this path does not make sense
anymore.

/mjt

RE: [RFC PATCH 5/5] cxl/core: add poison injection event handler

[Dan Williams]

Shiyang Ruan wrote:
> Currently driver only trace cxl events, poison injection on cxl memdev
> is silent.  OS needs to be notified then it could handle poison range
> in time.  Per CXL spec, the device error event could be signaled through
> FW-First and OS-First methods.
> 
> So, add poison event handler in OS-First method:
>   - qemu:
> - CXL device report POISON event to OS by MSI by sending GMER after
>   injecting a poison record

QEMU details do not belong in a kernel changelog. It is ok for an RFC,
but my hope is that this can be tested on hardware after being proven on
QEMU.

>   - CXL driver
> a. read the POISON event through GMER;   <-- this patch
> b. get POISON list;
> c. translate DPA to HPA;
> d. construct a mce instance, then call mce_log() to queue this mce
>instance;

It is not clear to me why the kernel should proactively fire machine
check notifications on injection? The changelog needs to make clear why
the kernel should do this, and the consequences of not going it.

For CPU consumed poison the machine check event will already fire. For
background discovery of poison, that should translate to a
memory_failure() notification with teh MF_ACTION_REQUIRED flag cleared.
Userspace, like rasdaemon, can then make a page offline decision.

RE: [RFC PATCH 4/5] cxl/core: add report option for cxl_mem_get_poison()

[Dan Williams]

Shiyang Ruan wrote:
> When a poison event is received, driver uses GET_POISON_LIST command
> to get the poison list.  Now driver has cxl_mem_get_poison(), so
> reuse it and add a parameter 'bool report', report poison record to MCE
> if set true.

If the memory error record has the poison event, why does the poison
list need to be retrieved by the kernel? I would expect it is sufficient
to just report the single poison event and leave it to userspace to
react to that event and retrieve more data if it wants.

RE: [RFC PATCH 3/5] cxl/core: introduce cxl_mem_report_poison()

[Dan Williams]

Shiyang Ruan wrote:
> If poison is detected(reported from cxl memdev), OS should be notified to
> handle it.  Introduce this function:
>   1. translate DPA to HPA;
>   2. construct a MCE instance; (TODO: more details need to be filled)
>   3. log it into MCE event queue;
> 
> After that, MCE mechanism can walk over its notifier chain to execute
> specific handlers.
> 
> Signed-off-by: Shiyang Ruan 
> ---
>  arch/x86/kernel/cpu/mce/core.c |  1 +
>  drivers/cxl/core/mbox.c| 33 +
>  2 files changed, 34 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
> index bc39252bc54f..a64c0aceb7e0 100644
> --- a/arch/x86/kernel/cpu/mce/core.c
> +++ b/arch/x86/kernel/cpu/mce/core.c
> @@ -131,6 +131,7 @@ void mce_setup(struct mce *m)
>   m->ppin = cpu_data(m->extcpu).ppin;
>   m->microcode = boot_cpu_data.microcode;
>  }
> +EXPORT_SYMBOL_GPL(mce_setup);

No, mce_setup() is x86 specific and the CXL subsystem is CPU
architecture independent. My expectation is that CXL should translate
errors for edac similar to how the ACPI GHES code does it. See usage of
edac_raw_mc_handle_error() and memory_failure_queue().

Otherwise an MCE is a CPU consumption of poison event, and CXL is
reporting device-side discovery of poison.

RE: [RFC PATCH 2/5] cxl/core: introduce cxl_memdev_dpa_to_hpa()

[Dan Williams]

Shiyang Ruan wrote:
> When a memdev is assigned to a region, its Device Physical Address will be
> mapped to Host Physical Address.  Introduce this helper function to
> translate HPA from a given memdev and its DPA.
> 
> Signed-off-by: Shiyang Ruan 
> ---
>  drivers/cxl/core/memdev.c | 12 
>  drivers/cxl/cxlmem.h  |  1 +
>  2 files changed, 13 insertions(+)
> 
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index dae8802ecdb0..c304e709ef0e 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -319,6 +319,18 @@ static int cxl_validate_poison_dpa(struct cxl_memdev 
> *cxlmd, u64 dpa)
>   return 0;
>  }
>  
> +phys_addr_t cxl_memdev_dpa_to_hpa(struct cxl_memdev *cxlmd, u64 dpa)
> +{
> + struct cxl_region *cxlr = cxl_dpa_to_region(cxlmd, dpa);
> +
> + if (cxlr)
> + return cxlr->params.res->start + dpa;
> + else {
> + dev_dbg(>dev, "device belongs to no region.\n");
> + return 0;
> + }
> +}

Hmm no, I would not say memdevs are assigned to regions, *endpoint
decoders* are assigned to regions. cxl_dpa_to_region() is only an
internal helper when the endpoint decoder is unknown. Otherwise endpoint
decoders have a direct-link to their region, if mapped. See usage of
"cxled->cxld.region".

RE: [RFC PATCH 1/5] cxl/core: correct length of DPA field masks

[Dan Williams]

Shiyang Ruan wrote:
> The length of Physical Address in General Media Event Record/DRAM Event
> Record is 64-bit, so the field mask should be defined as such length.

Can you include this user visible side-effect of this change. Looks like
this could cause usages of CXL_DPA_FLAGS_MASK to return an incorrect
result?

> 
> Signed-off-by: Shiyang Ruan 
> ---
>  drivers/cxl/core/trace.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
> index 89445435303a..388a87d972c2 100644
> --- a/drivers/cxl/core/trace.h
> +++ b/drivers/cxl/core/trace.h
> @@ -253,11 +253,11 @@ TRACE_EVENT(cxl_generic_event,
>   * DRAM Event Record
>   * CXL rev 3.0 section 8.2.9.2.1.2; Table 8-44
>   */
> -#define CXL_DPA_FLAGS_MASK   0x3F
> +#define CXL_DPA_FLAGS_MASK   0x3FULL
>  #define CXL_DPA_MASK (~CXL_DPA_FLAGS_MASK)
>  
> -#define CXL_DPA_VOLATILE BIT(0)
> -#define CXL_DPA_NOT_REPAIRABLE   BIT(1)
> +#define CXL_DPA_VOLATILE BIT_ULL(0)
> +#define CXL_DPA_NOT_REPAIRABLE   BIT_ULL(1)
>  #define show_dpa_flags(flags)__print_flags(flags, "|",   
>\
>   { CXL_DPA_VOLATILE, "VOLATILE"  }, \
>   { CXL_DPA_NOT_REPAIRABLE,   "NOT_REPAIRABLE"}  \
> -- 
> 2.34.1
> 

Re: [PATCH v3 08/11] hw/sparc/sun4m: Realize DMA controller before accessing it

[Mark Cave-Ayland]

On 09/02/2024 11:37, Peter Maydell wrote:


On Thu, 8 Feb 2024 at 18:14, Philippe Mathieu-Daudé  wrote:


We should not wire IRQs on unrealized device.

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

diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index e782c8ec7a..d52e6a7213 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -312,13 +312,11 @@ static void *sparc32_dma_init(hwaddr dma_base,
  dma = qdev_new(TYPE_SPARC32_DMA);
  espdma = SPARC32_ESPDMA_DEVICE(object_resolve_path_component(
 OBJECT(dma), "espdma"));
-sysbus_connect_irq(SYS_BUS_DEVICE(espdma), 0, espdma_irq);

  esp = SYSBUS_ESP(object_resolve_path_component(OBJECT(espdma), "esp"));

  ledma = SPARC32_LEDMA_DEVICE(object_resolve_path_component(
   OBJECT(dma), "ledma"));
-sysbus_connect_irq(SYS_BUS_DEVICE(ledma), 0, ledma_irq);

  lance = SYSBUS_PCNET(object_resolve_path_component(
   OBJECT(ledma), "lance"));
@@ -332,6 +330,11 @@ static void *sparc32_dma_init(hwaddr dma_base,
  }

  sysbus_realize_and_unref(SYS_BUS_DEVICE(dma), _fatal);
+
+sysbus_connect_irq(SYS_BUS_DEVICE(espdma), 0, espdma_irq);
+
+sysbus_connect_irq(SYS_BUS_DEVICE(ledma), 0, ledma_irq);
+


(This confused me briefly until I realised that this function
is reaching into the dma device to find child objects to connect
the IRQs to. Perhaps it would be nicer if the wrapping 'dma' object
passed through those IRQs to avoid that.)


FWIW I can't say I'm a fan of passing through IRQs for container devices that contain 
one or more child devices, since you end up with an extra layer of wiring complexity 
that can change depending upon the child device IRQ wiring. I find it much easier to 
access the child devices directly, since then the wiring is always consistent across 
all users.



ATB,

Mark.

Re: [PATCH v1 00/15] libvhost-user: support more memslots and cleanup memslot handling code

[David Hildenbrand]

On 07.02.24 12:40, Stefano Garzarella wrote:

On Fri, Feb 02, 2024 at 10:53:17PM +0100, David Hildenbrand wrote:

This series adds support for more memslots (509) to libvhost-user, to
make it fully compatible with virtio-mem that uses up to 256 memslots
accross all memory devices in "dynamic-memslot" mode (more details
in patch #3).

One simple fix upfront.

With that in place, this series optimizes and extens memory region
handling in libvhost-user:
* Heavily deduplicate and clean up the memory region handling code
* Speeds up GPA->VA translation with many memslots using binary search
* Optimize mmap_offset handling to use it as fd_offset for mmap()
* Avoid ring remapping when adding a single memory region
* Avoid dumping all guest memory, possibly allocating memory in sparse
  memory mappings when the process crashes

I'm being very careful to not break some weird corner case that modern
QEMU might no longer trigger, but older one could have triggered or some
other frontend might trigger.

The only thing where I am not careful is to forbid memory regions that
overlap in GPA space: it doesn't make any sense.

With this series, virtio-mem (with dynamic-memslots=on) +
qemu-storage-daemon works flawlessly and as expected in my tests.


I don't know much about this code, but I didn't find anything wrong with
it. Thank you also for the great cleanup!

Acked-by: Stefano Garzarella 


Thanks Stefano for the review, highly appreciated!

--
Cheers,

David / dhildenb

Re: [PATCH 00/23] qemu-img: refersh options and --help handling

[Michael Tokarev]

10.02.2024 00:22, Michael Tokarev wrote:

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

...

I forgot to run checkpatch.pl - minor edits, the result is at
https://gitlab.com/mjt0k/qemu/-/commits/qemu-img-options

/mjt

Re: [PATCH v3 11/11] hw/sparc64/cpu: Initialize GPIO before realizing CPU devices

[Mark Cave-Ayland]

On 09/02/2024 11:34, Peter Maydell wrote:


On Thu, 8 Feb 2024 at 18:14, Philippe Mathieu-Daudé  wrote:


Inline cpu_create() in order to call
qdev_init_gpio_in_named_with_opaque()
before the CPU is realized.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/sparc64/sparc64.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/sparc64/sparc64.c b/hw/sparc64/sparc64.c
index 72f0849f50..3091cde586 100644
--- a/hw/sparc64/sparc64.c
+++ b/hw/sparc64/sparc64.c
@@ -24,6 +24,7 @@


  #include "qemu/osdep.h"
+#include "qapi/error.h"
  #include "cpu.h"
  #include "hw/boards.h"
  #include "hw/sparc/sparc64.h"
@@ -271,9 +272,10 @@ SPARCCPU *sparc64_cpu_devinit(const char *cpu_type, 
uint64_t prom_addr)
  uint32_t  stick_frequency = 100 * 100;
  uint32_t hstick_frequency = 100 * 100;

-cpu = SPARC_CPU(cpu_create(cpu_type));
+cpu = SPARC_CPU(object_new(cpu_type));
  qdev_init_gpio_in_named(DEVICE(cpu), sparc64_cpu_set_ivec_irq,
  "ivec-irq", IVEC_MAX);
+qdev_realize(DEVICE(cpu), NULL, _fatal);
  env = >env;

  env->tick = cpu_timer_create("tick", cpu, tick_irq,
--
2.41.0


For the purposes of letting us enforce the "init GPIOs
before realize, not after" rule,
Reviewed-by: Peter Maydell 

but it looks like this code is adding a GPIO to a
device from code that's not actually part of the
implementation of the device. Ideally most of the code in
this file should be rolled into the CPU itself in target/sparc.


I suspect the reason the code is arranged like this is because IVECs aren't part of 
the core SPARC 64-bit architecture specification, although they happen to be 
implemented by the CPUs used by QEMU. Perhaps this would be better be handled on a 
CPU model basis, but I agree this shouldn't be a blocker for this patch.



ATB,

Mark.

[PATCH] i386: xen: fix compilation --without-default-devices

[Paolo Bonzini]

The xenpv machine type requires XEN_BUS, so select it.

Signed-off-by: Paolo Bonzini 
---
 accel/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/accel/Kconfig b/accel/Kconfig
index a30cf2eb483..794e0d18d21 100644
--- a/accel/Kconfig
+++ b/accel/Kconfig
@@ -16,3 +16,4 @@ config KVM
 config XEN
 bool
 select FSDEV_9P if VIRTFS
+select XEN_BUS
-- 
2.43.0

Re: [PATCH v3 11/11] hw/sparc64/cpu: Initialize GPIO before realizing CPU devices

[Mark Cave-Ayland]

On 08/02/2024 18:12, Philippe Mathieu-Daudé wrote:


Inline cpu_create() in order to call
qdev_init_gpio_in_named_with_opaque()
before the CPU is realized.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/sparc64/sparc64.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/sparc64/sparc64.c b/hw/sparc64/sparc64.c
index 72f0849f50..3091cde586 100644
--- a/hw/sparc64/sparc64.c
+++ b/hw/sparc64/sparc64.c
@@ -24,6 +24,7 @@
  
  
  #include "qemu/osdep.h"

+#include "qapi/error.h"
  #include "cpu.h"
  #include "hw/boards.h"
  #include "hw/sparc/sparc64.h"
@@ -271,9 +272,10 @@ SPARCCPU *sparc64_cpu_devinit(const char *cpu_type, 
uint64_t prom_addr)
  uint32_t  stick_frequency = 100 * 100;
  uint32_t hstick_frequency = 100 * 100;
  
-cpu = SPARC_CPU(cpu_create(cpu_type));

+cpu = SPARC_CPU(object_new(cpu_type));
  qdev_init_gpio_in_named(DEVICE(cpu), sparc64_cpu_set_ivec_irq,
  "ivec-irq", IVEC_MAX);
+qdev_realize(DEVICE(cpu), NULL, _fatal);
  env = >env;
  
  env->tick = cpu_timer_create("tick", cpu, tick_irq,


Reviewed-by: Mark Cave-Ayland 


ATB,

Mark.

Re: [PATCH v3 10/11] hw/sparc/leon3: Initialize GPIO before realizing CPU devices

[Mark Cave-Ayland]

On 08/02/2024 18:12, Philippe Mathieu-Daudé wrote:


Inline cpu_create() in order to call
qdev_init_gpio_in_named_with_opaque()
before the CPU is realized.

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

diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 0df5fc949d..0e1d749306 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -234,8 +234,11 @@ static void leon3_generic_hw_init(MachineState *machine)
  APBPnp *apb_pnp;
  
  /* Init CPU */

-cpu = SPARC_CPU(cpu_create(machine->cpu_type));
+cpu = SPARC_CPU(object_new(machine->cpu_type));
  env = >env;
+qdev_init_gpio_in_named_with_opaque(DEVICE(cpu), leon3_set_pil_in,
+env, "pil", 1);
+qdev_realize(DEVICE(cpu), NULL, _fatal);


I know it's not part of this patch, but I think that 
qdev_init_gpio_in_named_with_opaque() can be replaced with just 
qdev_init_gpio_in_named(), and leon3_set_pil_in() updated to take CPUState.



  cpu_sparc_set_id(env, 0);
  
@@ -261,8 +264,6 @@ static void leon3_generic_hw_init(MachineState *machine)
  
  /* Allocate IRQ manager */

  irqmpdev = qdev_new(TYPE_GRLIB_IRQMP);
-qdev_init_gpio_in_named_with_opaque(DEVICE(cpu), leon3_set_pil_in,
-env, "pil", 1);
  sysbus_realize_and_unref(SYS_BUS_DEVICE(irqmpdev), _fatal);
  sysbus_mmio_map(SYS_BUS_DEVICE(irqmpdev), 0, LEON3_IRQMP_OFFSET);
  qdev_connect_gpio_out_named(irqmpdev, "grlib-irq", 0,


Reviewed-by: Mark Cave-Ayland 


ATB,

Mark.

Re: [PATCH 11/23] qemu-img: allow specifying -f fmt for snapshot subcommand

[Michael Tokarev]

10.02.2024 00:22, Michael Tokarev wrote:

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


...

-c = getopt_long(argc, argv, ":la:c:d:hqU",
+c = getopt_long(argc, argv, ":la:c:d:fhqU",
  long_options, NULL);


Should be "f:" here, since -f expects an argument.  Fixed locally.

/mjt

Re: [PATCH v3 09/11] hw/sparc/leon3: Realize GRLIB IRQ controller before accessing it

[Mark Cave-Ayland]

On 08/02/2024 18:12, Philippe Mathieu-Daudé wrote:


We should not wire IRQs on unrealized device.

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

diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 2dfb742566..0df5fc949d 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -263,10 +263,10 @@ static void leon3_generic_hw_init(MachineState *machine)
  irqmpdev = qdev_new(TYPE_GRLIB_IRQMP);
  qdev_init_gpio_in_named_with_opaque(DEVICE(cpu), leon3_set_pil_in,
  env, "pil", 1);
-qdev_connect_gpio_out_named(irqmpdev, "grlib-irq", 0,
-qdev_get_gpio_in_named(DEVICE(cpu), "pil", 0));
  sysbus_realize_and_unref(SYS_BUS_DEVICE(irqmpdev), _fatal);
  sysbus_mmio_map(SYS_BUS_DEVICE(irqmpdev), 0, LEON3_IRQMP_OFFSET);
+qdev_connect_gpio_out_named(irqmpdev, "grlib-irq", 0,
+qdev_get_gpio_in_named(DEVICE(cpu), "pil", 0));
  env->irq_manager = irqmpdev;
  env->qemu_irq_ack = leon3_irq_manager;
  grlib_apb_pnp_add_entry(apb_pnp, LEON3_IRQMP_OFFSET, 0xFFF,


Reviewed-by: Mark Cave-Ayland 


ATB,

Mark.

Re: [PATCH v3 08/11] hw/sparc/sun4m: Realize DMA controller before accessing it

[Mark Cave-Ayland]

On 08/02/2024 18:12, Philippe Mathieu-Daudé wrote:


We should not wire IRQs on unrealized device.

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

diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index e782c8ec7a..d52e6a7213 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -312,13 +312,11 @@ static void *sparc32_dma_init(hwaddr dma_base,
  dma = qdev_new(TYPE_SPARC32_DMA);
  espdma = SPARC32_ESPDMA_DEVICE(object_resolve_path_component(
 OBJECT(dma), "espdma"));
-sysbus_connect_irq(SYS_BUS_DEVICE(espdma), 0, espdma_irq);
  
  esp = SYSBUS_ESP(object_resolve_path_component(OBJECT(espdma), "esp"));
  
  ledma = SPARC32_LEDMA_DEVICE(object_resolve_path_component(

   OBJECT(dma), "ledma"));
-sysbus_connect_irq(SYS_BUS_DEVICE(ledma), 0, ledma_irq);
  
  lance = SYSBUS_PCNET(object_resolve_path_component(

   OBJECT(ledma), "lance"));
@@ -332,6 +330,11 @@ static void *sparc32_dma_init(hwaddr dma_base,
  }
  
  sysbus_realize_and_unref(SYS_BUS_DEVICE(dma), _fatal);

+
+sysbus_connect_irq(SYS_BUS_DEVICE(espdma), 0, espdma_irq);
+
+sysbus_connect_irq(SYS_BUS_DEVICE(ledma), 0, ledma_irq);
+
  sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
  
  sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);


Reviewed-by: Mark Cave-Ayland 


ATB,

Mark.

Re: [PATCH 0/3] system/memory: Trivial fixes

[Michael Tokarev]

09.02.2024 18:00, Philippe Mathieu-Daudé :

- Include missing "exec/memory.h"
- Set machine as parent of io/mem regions

Philippe Mathieu-Daudé (3):
   cpu-target: Include missing 'exec/memory.h' header
   monitor/target: Include missing 'exec/memory.h' header
   system/physmem: Assign global system I/O Memory to machine


Picked up patches 1 & 2, but not 3 yet.

/mjt


  cpu-target.c  | 1 +
  monitor/hmp-cmds-target.c | 1 +
  system/physmem.c  | 7 ---
  3 files changed, 6 insertions(+), 3 deletions(-)

Re: [PATCH v3 06/11] hw/misc/macio: Realize IDE controller before accessing it

[Mark Cave-Ayland]

On 08/02/2024 18:12, Philippe Mathieu-Daudé wrote:


We should not wire IRQs on unrealized device.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/misc/macio/macio.c | 8 +---
  1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index c9f22f8515..db662a2065 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -122,15 +122,17 @@ static bool macio_realize_ide(MacIOState *s, 
MACIOIDEState *ide,
Error **errp)
  {
  SysBusDevice *sbd = SYS_BUS_DEVICE(ide);
+bool success;
  
-sysbus_connect_irq(sbd, 0, irq0);

-sysbus_connect_irq(sbd, 1, irq1);
  qdev_prop_set_uint32(DEVICE(ide), "channel", dmaid);
  object_property_set_link(OBJECT(ide), "dbdma", OBJECT(>dbdma),
   _abort);
  macio_ide_register_dma(ide);
+success = qdev_realize(DEVICE(ide), BUS(>macio_bus), errp);
+sysbus_connect_irq(sbd, 0, irq0);
+sysbus_connect_irq(sbd, 1, irq1);
  
-return qdev_realize(DEVICE(ide), BUS(>macio_bus), errp);

+return success;
  }
  
  static void macio_oldworld_realize(PCIDevice *d, Error **errp)


I see that Zoltan has already commented about checking the success of qdev_realise() 
before wiring the sysbus IRQs, so with that fixed:


Reviewed-by: Mark Cave-Ayland 


ATB,

Mark.

Re: [PATCH 09/88] esp: update TC check logic in do_dma_pdma_cb() to check for TC == 0

[Mark Cave-Ayland]

On 08/02/2024 18:11, Paolo Bonzini wrote:


On Thu, Feb 8, 2024 at 10:46 AM Mark Cave-Ayland
 wrote:


On 01/02/2024 11:36, Paolo Bonzini wrote:


Il gio 1 feb 2024, 12:25 Mark Cave-Ayland mailto:mark.cave-ayl...@ilande.co.uk>> ha scritto:

 On 01/02/2024 10:46, Paolo Bonzini wrote:

  > On Fri, Jan 12, 2024 at 1:55 PM Mark Cave-Ayland
  > mailto:mark.cave-ayl...@ilande.co.uk>> 
wrote:
  >>
  >> Invert the logic so that the end of DMA transfer check becomes one 
that checks
  >> for TC == 0 in the from device path in do_dma_pdma_cb().
  >>
  >> Signed-off-by: Mark Cave-Ayland mailto:mark.cave-ayl...@ilande.co.uk>>
  >> ---
  >>   hw/scsi/esp.c | 24 +++-
  >>   1 file changed, 11 insertions(+), 13 deletions(-)
  >>
  >> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
  >> index fecfef7c89..63c828c1b2 100644
  >> --- a/hw/scsi/esp.c
  >> +++ b/hw/scsi/esp.c
  >> @@ -602,21 +602,19 @@ static void do_dma_pdma_cb(ESPState *s)
  >>   return;
  >>   }
  >>
  >> -if (esp_get_tc(s) != 0) {
  >> -/* Copy device data to FIFO */
  >> -len = MIN(s->async_len, esp_get_tc(s));
  >> -len = MIN(len, fifo8_num_free(>fifo));
  >> -fifo8_push_all(>fifo, s->async_buf, len);
  >> -s->async_buf += len;
  >> -s->async_len -= len;
  >> -s->ti_size -= len;
  >> -esp_set_tc(s, esp_get_tc(s) - len);
  >> -return;
  >> +if (esp_get_tc(s) == 0) {
  >> +esp_lower_drq(s);
  >> +esp_dma_done(s);
  >>   }
  >
  > I'm only doing a cursory review, but shouldn't there be a return here?
  >
  > Paolo

 (goes and looks)

 Possibly there should, but my guess is that adding the return at that 
particular
 point in time at the series broke one of my reference images. In 
particular MacOS is
 notorious for requesting data transfers of X len, and setting the TC 
either too high
 or too low and let the in-built OS recovery logic handle these cases.


Absolutely, just noticing that there is a functional change but the commit 
message
showed it as a refactoring only.


Would you like me to come up with a reworded commit message here?


If you want to change it, it's okay because len is zero at this point
and the code after the "if" therefore does nothing. And the "if" will
become esp_dma_ti_check() so adding a return is kind of messy here.


Okay I'll leave it as it is - I'm not too worried about the detail here, since that 
particular section ends up being removed.



The fact that this is bisectable is quite insane, and I am not asking for any 
code
changes. TBH I wouldn't have blamed you if you just sent a patch to create 
esp2.c and
a patch to delete esp.c...


Heh... spending a chunk of time rewriting the emulation of a 30 year-old SCSI
controller is probably an odd choice, but the result is something that is
considerably more maintainable than the current implementation.


I absolutely agree, you went above and beyond and sorry if it wasn't
clear from the joke.


No worries, I wasn't taking it seriously either :)


Anything else that you'd like me to change before the series can be considered 
for merge?


No, go ahead!


Thanks! I'll take that as a nod to send a PR over the next few days.


ATB,

Mark.

Re: [PATCH v3 05/11] hw/ppc/prep: Realize ISA bridge before accessing it

[Mark Cave-Ayland]

On 08/02/2024 18:12, Philippe Mathieu-Daudé wrote:


We should not wire IRQs on unrealized device.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/ppc/prep.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 1a6cd05c61..4eb5477069 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -278,9 +278,9 @@ static void ibm_40p_init(MachineState *machine)
  
  /* PCI -> ISA bridge */

  i82378_dev = DEVICE(pci_new(PCI_DEVFN(11, 0), "i82378"));
+qdev_realize_and_unref(i82378_dev, BUS(pci_bus), _fatal);
  qdev_connect_gpio_out(i82378_dev, 0,
qdev_get_gpio_in(DEVICE(cpu), PPC6xx_INPUT_INT));
-qdev_realize_and_unref(i82378_dev, BUS(pci_bus), _fatal);
  
  sysbus_connect_irq(pcihost, 0, qdev_get_gpio_in(i82378_dev, 15));

  isa_bus = ISA_BUS(qdev_get_child_bus(i82378_dev, "isa.0"));


Reviewed-by: Mark Cave-Ayland 


ATB,

Mark.

[PATCH 08/23] qemu-img: refresh options/--help for "convert" command

[Michael Tokarev]

Add missing long options and --help output.

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

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 52 +++-
 1 file changed, 51 insertions(+), 1 deletion(-)

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

[PATCH 20/23] qemu-img: refresh options/--help for "dd" command

[Michael Tokarev]

Add missing long options and --help output.
---
 qemu-img.c | 29 ++---
 1 file changed, 22 insertions(+), 7 deletions(-)

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

[PATCH 06/23] qemu-img: refresh options/--help for "commit" command

[Michael Tokarev]

Add missing long options and --help output.

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

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

[PATCH 21/23] qemu-img: refresh options/--help for "measure" command

[Michael Tokarev]

Add missing long options and --help output.

Also add -s short option for --size (and remove OPTION_SIZE).
---
 qemu-img.c | 43 +++
 1 file changed, 31 insertions(+), 12 deletions(-)

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

[PATCH 13/23] qemu-img: refresh options/--help for "snapshot" command

[Michael Tokarev]

Add missing long options and --help output.
---
 qemu-img.c | 26 --
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index d9dfff2f07..67e6a7797d 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3553,9 +3553,15 @@ static int img_snapshot(const img_cmd_t *ccmd, int argc, 
char **argv)
 for(;;) {
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
+{"quiet", no_argument, 0, 'q'},
 {"object", required_argument, 0, OPTION_OBJECT},
+{"format", required_argument, 0, 'f'},
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 {"force-share", no_argument, 0, 'U'},
+{"list", no_argument, 0, 'l'},
+{"apply", no_argument, 0, 'a'},
+{"create", no_argument, 0, 'c'},
+{"delete", no_argument, 0, 'd'},
 {0, 0, 0, 0}
 };
 c = getopt_long(argc, argv, ":la:c:d:fhqU",
@@ -3571,8 +3577,24 @@ static int img_snapshot(const img_cmd_t *ccmd, int argc, 
char **argv)
 unrecognized_option(ccmd, argv[optind - 1]);
 break;
 case 'h':
-help();
-return 0;
+cmd_help(ccmd,
+"[-f FMT | --image-opts] [-l | -a|-c|-d SNAPSHOT]\n"
+"  [-U] [--object OBJDEF] FILENAME\n"
+,
+" -q|--quiet - quiet operations\n"
+" -f|--format FMT - specify FILENAME format explicitly\n"
+" --image-opts - indicates that FILENAME is a complete image specification\n"
+"  instead of a file name (incompatible with --format)\n"
+" -U|--force-share - open image in shared mode for concurrent access\n"
+" --object OBJDEF - QEMU user-creatable object (eg encryption key)\n"
+" Operation, one of:\n"
+"  -l|--list - list snapshots in FILENAME (the default)\n"
+"  -c|--create SNAPSHOT - create named snapshot\n"
+"  -a|--apply SNAPSHOT - apply named snapshot to the base\n"
+"  -d|--delete SNAPSHOT - delete named snapshot\n"
+" FILENAME - image file name (or specification with --image-opts)\n"
+);
+break;
 case 'f':
 fmt = optarg;
 break;
-- 
2.39.2

[PATCH 14/23] qemu-img: refresh options/--help for "rebase" command

[Michael Tokarev]

Add missing long options and --help output.

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

Probably should rename local variables.
---
 qemu-img.c | 30 +-
 1 file changed, 29 insertions(+), 1 deletion(-)

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

[PATCH 04/23] qemu-img: refresh options/--help for "check" command

[Michael Tokarev]

Add missing long options and --help output.

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

diff --git a/qemu-img.c b/qemu-img.c
index 4e962843da..3ae07bfae0 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -792,7 +792,9 @@ static int img_check(const img_cmd_t *ccmd, int argc, char 
**argv)
 int option_index = 0;
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
+{"quiet", no_argument, 0, 'q'},
 {"format", required_argument, 0, 'f'},
+{"cache", required_argument, 0, 'T'},
 {"repair", required_argument, 0, 'r'},
 {"output", required_argument, 0, OPTION_OUTPUT},
 {"object", required_argument, 0, OPTION_OBJECT},
@@ -813,7 +815,22 @@ static int img_check(const img_cmd_t *ccmd, int argc, char 
**argv)
 unrecognized_option(ccmd, argv[optind - 1]);
 break;
 case 'h':
-help();
+cmd_help(ccmd,
+"[-f FMT | --image-opts] [-T CACHE_MODE] [-r] [-u]\n"
+"  [--output human|json] [--object OBJDEF] FILENAME\n"
+,
+" -q|--quiet - quiet operations\n"
+" -f|--format FMT - specifies format of the image explicitly\n"
+" --image-opts - indicates that FILENAME is a complete image specification\n"
+"  instead of a file name (incompatible with --format)\n"
+" -T|--cache CACHE_MODE - cache mode when opening image (" BDRV_DEFAULT_CACHE 
")\n"
+" -U|--force-share - open image in shared mode for concurrent access\n"
+" --output human|json - output format\n"
+" -r|--repair leaks|all - repair particular aspect of the image\n"
+"  (image will be open in read-write mode, incompatible with --force-share)\n"
+" --object OBJDEF - QEMU user-creatable object (eg encryption key)\n"
+" FILENAME - the image file (or image specification) to operate on\n"
+);
 break;
 case 'f':
 fmt = optarg;
-- 
2.39.2

[PATCH 18/23] qemu-img: refresh options/--help for "bench" command

[Michael Tokarev]

Add missing long options and --help output.
---
 qemu-img.c | 36 ++--
 1 file changed, 34 insertions(+), 2 deletions(-)

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

[PATCH 01/23] qemu-img: pass current cmd info into command handlers

[Michael Tokarev]

In order to be able to give correct --help output, pass current cmd
information (img_cmd_t structure) to command handlers and to common
error reporting functions. After the change, in case of command-line
error, qemu-img will now print:

 Try 'qemu-img create --help' for more information.

Current cmd info will be useful in --help output as well.

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

diff --git a/qemu-img.c b/qemu-img.c
index 7668f86769..05f80b6e5b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -60,7 +60,7 @@
 
 typedef struct img_cmd_t {
 const char *name;
-int (*handler)(int argc, char **argv);
+int (*handler)(const struct img_cmd_t *ccmd, int argc, char **argv);
 } img_cmd_t;
 
 enum {
@@ -101,8 +101,8 @@ static void format_print(void *opaque, const char *name)
 printf(" %s", name);
 }
 
-static G_NORETURN G_GNUC_PRINTF(1, 2)
-void error_exit(const char *fmt, ...)
+static G_NORETURN G_GNUC_PRINTF(2, 3)
+void error_exit(const img_cmd_t *ccmd, const char *fmt, ...)
 {
 va_list ap;
 
@@ -110,20 +110,20 @@ void error_exit(const char *fmt, ...)
 error_vreport(fmt, ap);
 va_end(ap);
 
-error_printf("Try 'qemu-img --help' for more information\n");
+error_printf("Try 'qemu-img %s --help' for more information\n", ccmd ? 
ccmd->name : "");
 exit(EXIT_FAILURE);
 }
 
 static G_NORETURN
-void missing_argument(const char *option)
+void missing_argument(const img_cmd_t *ccmd, const char *option)
 {
-error_exit("missing argument for option '%s'", option);
+error_exit(ccmd, "missing argument for option '%s'", option);
 }
 
 static G_NORETURN
-void unrecognized_option(const char *option)
+void unrecognized_option(const img_cmd_t *ccmd, const char *option)
 {
-error_exit("unrecognized option '%s'", option);
+error_exit(ccmd, "unrecognized option '%s'", option);
 }
 
 /* Please keep in synch with docs/tools/qemu-img.rst */
@@ -508,7 +508,7 @@ static int64_t cvtnum(const char *name, const char *value)
 return cvtnum_full(name, value, 0, INT64_MAX);
 }
 
-static int img_create(int argc, char **argv)
+static int img_create(const img_cmd_t *ccmd, int argc, char **argv)
 {
 int c;
 uint64_t img_size = -1;
@@ -534,10 +534,10 @@ static int img_create(int argc, char **argv)
 }
 switch(c) {
 case ':':
-missing_argument(argv[optind - 1]);
+missing_argument(ccmd, argv[optind - 1]);
 break;
 case '?':
-unrecognized_option(argv[optind - 1]);
+unrecognized_option(ccmd, argv[optind - 1]);
 break;
 case 'h':
 help();
@@ -576,7 +576,7 @@ static int img_create(int argc, char **argv)
 }
 
 if (optind >= argc) {
-error_exit("Expecting image file name");
+error_exit(ccmd, "Expecting image file name");
 }
 optind++;
 
@@ -591,7 +591,7 @@ static int img_create(int argc, char **argv)
 img_size = (uint64_t)sval;
 }
 if (optind != argc) {
-error_exit("Unexpected argument: %s", argv[optind]);
+error_exit(ccmd, "Unexpected argument: %s", argv[optind]);
 }
 
 bdrv_img_create(filename, fmt, base_filename, base_fmt,
@@ -716,7 +716,7 @@ static int collect_image_check(BlockDriverState *bs,
  *  3 - Check completed, image has leaked clusters, but is good otherwise
  * 63 - Checks are not supported by the image format
  */
-static int img_check(int argc, char **argv)
+static int img_check(const img_cmd_t *ccmd, int argc, char **argv)
 {
 int c, ret;
 OutputFormat output_format = OFORMAT_HUMAN;
@@ -754,10 +754,10 @@ static int img_check(int argc, char **argv)
 }
 switch(c) {
 case ':':
-missing_argument(argv[optind - 1]);
+missing_argument(ccmd, argv[optind - 1]);
 break;
 case '?':
-unrecognized_option(argv[optind - 1]);
+unrecognized_option(ccmd, argv[optind - 1]);
 break;
 case 'h':
 help();
@@ -773,7 +773,7 @@ static int img_check(int argc, char **argv)
 } else if (!strcmp(optarg, "all")) {
 fix = BDRV_FIX_LEAKS | BDRV_FIX_ERRORS;
 } else {
-error_exit("Unknown option value for -r "
+error_exit(ccmd, "Unknown option value for -r "
"(expecting 'leaks' or 'all'): %s", optarg);
 }
 break;
@@ -798,7 +798,7 @@ static int img_check(int argc, char **argv)
 }
 }
 if (optind != argc - 1) {
-error_exit("Expecting one image file name");
+error_exit(ccmd, "Expecting one image file name");
 }
 filename = argv[optind++];
 
@@ -948,7 +948,7 @@ static void run_block_job(BlockJob *job, Error **errp)
 }
 }
 
-static int img_commit(int argc, char **argv)
+static int img_commit(const img_cmd_t *ccmd, 

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

[Michael Tokarev]

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

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

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

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

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

[Michael Tokarev]

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

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

diff --git a/qemu-img.c b/qemu-img.c
index 69d41e0a92..929a25a021 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4271,13 +4271,13 @@ static int img_resize(const img_cmd_t *ccmd, int argc, 
char **argv)
 
 /* Remove size from argv manually so that negative numbers are not treated
  * as options by getopt. */
-if (argc < 3) {
-error_exit(ccmd, "Not enough arguments");
-return 1;
+if (argc > 1 && argv[argc - 1][0] == '-'
+&& argv[argc-1][1] >= '0' && argv[argc-1][1] <= '9') {
+size = argv[--argc];
+} else {
+size = NULL;
 }
 
-size = argv[--argc];
-
 /* Parse getopt arguments */
 fmt = NULL;
 for(;;) {
@@ -4329,10 +4329,13 @@ static int img_resize(const img_cmd_t *ccmd, int argc, 
char **argv)
 break;
 }
 }
-if (optind != argc - 1) {
+if (optind + 1 + (size == NULL) != argc) {
 error_exit(ccmd, "Expecting image file name and size");
 }
 filename = argv[optind++];
+if (!size) {
+size = argv[optind++];
+}
 
 /* Choose grow, shrink, or absolute resize mode */
 switch (size[0]) {
-- 
2.39.2

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

[Michael Tokarev]

Use common code and simplify error message

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

diff --git a/qemu-img.c b/qemu-img.c
index 7edfc56572..4e962843da 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -145,6 +145,17 @@ void cmd_help(const img_cmd_t *ccmd,
 exit(EXIT_SUCCESS);
 }
 
+static OutputFormat parse_output_format(const img_cmd_t *ccmd, const char *arg)
+{
+if (!strcmp(arg, "json")) {
+return OFORMAT_JSON;
+} else if (!strcmp(arg, "human")) {
+return OFORMAT_HUMAN;
+} else {
+error_exit(ccmd, "--output expects 'human' or 'json' not '%s'", arg);
+}
+}
+
 /* Please keep in synch with docs/tools/qemu-img.rst */
 static G_NORETURN
 void help(void)
@@ -763,7 +774,7 @@ static int img_check(const img_cmd_t *ccmd, int argc, char 
**argv)
 {
 int c, ret;
 OutputFormat output_format = OFORMAT_HUMAN;
-const char *filename, *fmt, *output, *cache;
+const char *filename, *fmt, *cache;
 BlockBackend *blk;
 BlockDriverState *bs;
 int fix = 0;
@@ -775,7 +786,6 @@ static int img_check(const img_cmd_t *ccmd, int argc, char 
**argv)
 bool force_share = false;
 
 fmt = NULL;
-output = NULL;
 cache = BDRV_DEFAULT_CACHE;
 
 for(;;) {
@@ -821,7 +831,7 @@ static int img_check(const img_cmd_t *ccmd, int argc, char 
**argv)
 }
 break;
 case OPTION_OUTPUT:
-output = optarg;
+output_format = parse_output_format(ccmd, optarg);
 break;
 case 'T':
 cache = optarg;
@@ -845,15 +855,6 @@ static int img_check(const img_cmd_t *ccmd, int argc, char 
**argv)
 }
 filename = argv[optind++];
 
-if (output && !strcmp(output, "json")) {
-output_format = OFORMAT_JSON;
-} else if (output && !strcmp(output, "human")) {
-output_format = OFORMAT_HUMAN;
-} else if (output) {
-error_report("--output must be used with human or json as argument.");
-return 1;
-}
-
 ret = bdrv_parse_cache_mode(cache, , );
 if (ret < 0) {
 error_report("Invalid source cache option: %s", cache);
@@ -3047,13 +3048,12 @@ static int img_info(const img_cmd_t *ccmd, int argc, 
char **argv)
 int c;
 OutputFormat output_format = OFORMAT_HUMAN;
 bool chain = false;
-const char *filename, *fmt, *output;
+const char *filename, *fmt;
 BlockGraphInfoList *list;
 bool image_opts = false;
 bool force_share = false;
 
 fmt = NULL;
-output = NULL;
 for(;;) {
 int option_index = 0;
 static const struct option long_options[] = {
@@ -3088,7 +3088,7 @@ static int img_info(const img_cmd_t *ccmd, int argc, char 
**argv)
 force_share = true;
 break;
 case OPTION_OUTPUT:
-output = optarg;
+output_format = parse_output_format(ccmd, optarg);
 break;
 case OPTION_BACKING_CHAIN:
 chain = true;
@@ -3106,15 +3106,6 @@ static int img_info(const img_cmd_t *ccmd, int argc, 
char **argv)
 }
 filename = argv[optind++];
 
-if (output && !strcmp(output, "json")) {
-output_format = OFORMAT_JSON;
-} else if (output && !strcmp(output, "human")) {
-output_format = OFORMAT_HUMAN;
-} else if (output) {
-error_report("--output must be used with human or json as argument.");
-return 1;
-}
-
 list = collect_image_info_list(image_opts, filename, fmt, chain,
force_share);
 if (!list) {
@@ -3273,7 +3264,7 @@ static int img_map(const img_cmd_t *ccmd, int argc, char 
**argv)
 OutputFormat output_format = OFORMAT_HUMAN;
 BlockBackend *blk;
 BlockDriverState *bs;
-const char *filename, *fmt, *output;
+const char *filename, *fmt;
 int64_t length;
 MapEntry curr = { .length = 0 }, next;
 int ret = 0;
@@ -3283,7 +3274,6 @@ static int img_map(const img_cmd_t *ccmd, int argc, char 
**argv)
 int64_t max_length = -1;
 
 fmt = NULL;
-output = NULL;
 for (;;) {
 int option_index = 0;
 static const struct option long_options[] = {
@@ -3319,7 +3309,7 @@ static int img_map(const img_cmd_t *ccmd, int argc, char 
**argv)
 force_share = true;
 break;
 case OPTION_OUTPUT:
-output = optarg;
+output_format = parse_output_format(ccmd, optarg);
 break;
 case 's':
 start_offset = cvtnum("start offset", optarg);
@@ -3346,15 +3336,6 @@ static int img_map(const img_cmd_t *ccmd, int argc, char 
**argv)
 }
 filename = argv[optind];
 
-if (output && !strcmp(output, "json")) {
-output_format = OFORMAT_JSON;
-} else if (output && !strcmp(output, "human")) {
-output_format = OFORMAT_HUMAN;
-} else if (output) {
-error_report("--output must be used with human 

[PATCH 19/23] qemu-img: refresh options/--help for "bitmap" command

[Michael Tokarev]

Add missing long options and --help output.
---
 qemu-img.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 3be365cd07..9a0cd05d42 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -5103,7 +5103,24 @@ static int img_bitmap(const img_cmd_t *ccmd, int argc, 
char **argv)
 unrecognized_option(ccmd, argv[optind - 1]);
 break;
 case 'h':
-help();
+cmd_help(ccmd,
+"( --merge SOURCE | --add | --remove | --clear |\n"
+"  --enable | --disable ).. [-f FMT | --image-opts]\n"
+"  [ -b SRC_FILENAME [-F SOURCE_FMT]] [-g GRANULARITY] [--object OBJDEF]\n"
+"  FILENAME BITMAP\n"
+,
+" -f|--format FMT - specify FILENAME format explicitly\n"
+" --image-opts - indicates that FILENAME is a complete image specification\n"
+"  instead of a file name (incompatible with --format)\n"
+" --add - creates BITMAP, enables to record future edits\n"
+"  -g|--granularity GRANULARITY - sets non-default granularity for --add\n"
+" --remove - removes BITMAP\n"
+" --clear - clears BITMAP\n"
+" --enable, --disable - starts and stops recording future edits to BITMAP\n"
+" --merge SRC_FILENAME - merges contents of SRC_FILENAME bitmap into BITMAP\n"
+"  -b|--source-file SRC_FILENAME - select alternative source file for 
--merge\n"
+"  -F|--source-format SRC_FMT - specify format for SRC_FILENAME explicitly\n"
+);
 break;
 case 'b':
 src_filename = optarg;
-- 
2.39.2

[PATCH 17/23] qemu-img: refresh options/--help for "amend" command

[Michael Tokarev]

Add missing long options and --help output.
---
 qemu-img.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index e552401074..f598eba3a8 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4493,7 +4493,12 @@ static int img_amend(const img_cmd_t *ccmd, int argc, 
char **argv)
 for (;;) {
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
+{"quiet", no_argument, 0, 'q'},
+{"progress", no_argument, 0, 'p'},
 {"object", required_argument, 0, OPTION_OBJECT},
+{"format", required_argument, 0, 'f'},
+{"cache", required_argument, 0, 't'},
+{"options", required_argument, 0, 'o'},
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 {"force", no_argument, 0, OPTION_FORCE},
 {0, 0, 0, 0}
@@ -4512,7 +4517,18 @@ static int img_amend(const img_cmd_t *ccmd, int argc, 
char **argv)
 unrecognized_option(ccmd, argv[optind - 1]);
 break;
 case 'h':
-help();
+cmd_help(ccmd,
+"[-f FMT | --image-opts] [t CACHE] [--force] [-p] [-q]\n"
+"  [--object OBJDEF -o OPTIONS FILENAME\n"
+,
+" -q|--quiet - quiet operation\n"
+" -p|--progres - show progress\n"
+" -f|--format FMT - specify FILENAME format explicitly\n"
+" --image-opts - indicates that FILENAME is a complete image specification\n"
+"  instead of a file name (incompatible with --format)\n"
+" -t|--cache CACHE - cache mode for FILENAME (" BDRV_DEFAULT_CACHE ")\n"
+" --force - allow certain unsafe operations\n" 
+);
 break;
 case 'o':
 if (accumulate_options(, optarg) < 0) {
-- 
2.39.2

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

[Michael Tokarev]

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

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

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

[PATCH 05/23] qemu-img: simplify --repair error message

[Michael Tokarev]

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

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

[PATCH 11/23] qemu-img: allow specifying -f fmt for snapshot subcommand

[Michael Tokarev]

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

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

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

[PATCH 07/23] qemu-img: refresh options/--help for "compare" command

[Michael Tokarev]

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

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

[PATCH 16/23] qemu-img: refresh options/--help for "resize" command

[Michael Tokarev]

Add missing long options and --help output.
---
 qemu-img.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 929a25a021..e552401074 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4283,7 +4283,9 @@ static int img_resize(const img_cmd_t *ccmd, int argc, 
char **argv)
 for(;;) {
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
+{"quiet", no_argument, 0, 'q'},
 {"object", required_argument, 0, OPTION_OBJECT},
+{"format", required_argument, 0, 'f'},
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 {"preallocation", required_argument, 0, OPTION_PREALLOCATION},
 {"shrink", no_argument, 0, OPTION_SHRINK},
@@ -4302,8 +4304,21 @@ static int img_resize(const img_cmd_t *ccmd, int argc, 
char **argv)
 unrecognized_option(ccmd, argv[optind - 1]);
 break;
 case 'h':
-help();
-break;
+cmd_help(ccmd,
+"[-f FMT | --image-opts] [--preallocation PREALLOC] [--shrink]\n"
+"  [--object OBJECTDEF] [-q] FILENAME [+|-]SIZE\n"
+,
+" -q|--quiet - quiet operation\n"
+" -f|--format FMT - specify FILENAME format explicitly\n"
+" --image-opts - indicates that FILENAME is a complete image specification\n"
+"  instead of a file name (incompatible with --format)\n"
+" --shrink - allow operation when new size is smaller than original\n"
+" --preallocation PREALLOC - specify preallocation type for the new areas\n"
+" --object OBJDEF - QEMU user-creatable object (eg encryption key)\n"
+" FILENAME - image file (specification) to resize\n"
+" SIZE - new image size or amount by which to shrink/grow\n"
+);
+return 0;
 case 'f':
 fmt = optarg;
 break;
-- 
2.39.2

[PATCH 10/23] qemu-img: refresh options/--help for "map" command

[Michael Tokarev]

Add missing long options and --help output.
---
 qemu-img.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index a1a0ba99f0..5af0b8ec18 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3425,7 +3425,20 @@ static int img_map(const img_cmd_t *ccmd, int argc, char 
**argv)
 unrecognized_option(ccmd, argv[optind - 1]);
 break;
 case 'h':
-help();
+cmd_help(ccmd,
+"[-f FMT | --image-opts] [--object OBJDEF] [--output human|json]\n"
+"  [--start-offset OFFSET] [--max-length LENGTH] [-U] FILENAME\n"
+,
+" -f|--format FMT - specify FILENAME image format explicitly\n"
+" --image-opts - indicates that FILENAME is a complete image specification\n"
+"  instead of a file name (incompatible with --format)\n"
+" --start-offset OFFSET\n"
+" --max-length LENGTH\n"
+" --output human|json - specify output format name (default human)\n"
+" -U|--force-share - open image in shared mode for concurrent access\n"
+" --object OBJDEF - QEMU user-creatable object (eg encryption key)\n"
+" FILENAME - image file name (or specification with --image-opts)\n"
+);
 break;
 case 'f':
 fmt = optarg;
-- 
2.39.2

[PATCH 00/23] qemu-img: refersh options and --help handling

[Michael Tokarev]

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

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

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

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

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

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

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

Michael Tokarev (23):
  qemu-img: pass current cmd info into command handlers
  qemu-img: refresh options/--help for "create" subcommand
  qemu-img: factor out parse_output_format() and use it in the code
  qemu-img: refresh options/--help for "check" command
  qemu-img: simplify --repair error message
  qemu-img: refresh options/--help for "commit" command
  qemu-img: refresh options/--help for "compare" command
  qemu-img: refresh options/--help for "convert" command
  qemu-img: refresh options/--help for "info" command
  qemu-img: refresh options/--help for "map" command
  qemu-img: allow specifying -f fmt for snapshot subcommand
  qemu-img: make -l (list) the default for "snapshot" subcommand
  qemu-img: refresh options/--help for "snapshot" command
  qemu-img: refresh options/--help for "rebase" command
  qemu-img: resize: do not always eat last argument
  qemu-img: refresh options/--help for "resize" command
  qemu-img: refresh options/--help for "amend" command
  qemu-img: refresh options/--help for "bench" command
  qemu-img: refresh options/--help for "bitmap" command
  qemu-img: refresh options/--help for "dd" command
  qemu-img: refresh options/--help for "measure" command
  qemu-img: implement short --help, remove global help() function
  qemu-img: inline list of supported commands, remove qemu-img-cmds.h
include

 docs/tools/qemu-img.rst |   2 +-
 qemu-img-cmds.hx|   4 +-
 qemu-img.c  | 843 ++--
 3 files changed, 558 insertions(+), 291 deletions(-)

-- 
2.39.2

[PATCH 09/23] qemu-img: refresh options/--help for "info" command

[Michael Tokarev]

Add missing long options and --help output.

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

diff --git a/qemu-img.c b/qemu-img.c
index d4dafebff9..a1a0ba99f0 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -65,7 +65,6 @@ typedef struct img_cmd_t {
 
 enum {
 OPTION_OUTPUT = 256,
-OPTION_BACKING_CHAIN = 257,
 OPTION_OBJECT = 258,
 OPTION_IMAGE_OPTS = 259,
 OPTION_PATTERN = 260,
@@ -3173,13 +3172,13 @@ static int img_info(const img_cmd_t *ccmd, int argc, 
char **argv)
 {"help", no_argument, 0, 'h'},
 {"format", required_argument, 0, 'f'},
 {"output", required_argument, 0, OPTION_OUTPUT},
-{"backing-chain", no_argument, 0, OPTION_BACKING_CHAIN},
+{"backing-chain", no_argument, 0, 'b'},
 {"object", required_argument, 0, OPTION_OBJECT},
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 {"force-share", no_argument, 0, 'U'},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":f:hU",
+c = getopt_long(argc, argv, ":f:hbU",
 long_options, _index);
 if (c == -1) {
 break;
@@ -3192,7 +3191,20 @@ static int img_info(const img_cmd_t *ccmd, int argc, 
char **argv)
 unrecognized_option(ccmd, argv[optind - 1]);
 break;
 case 'h':
-help();
+cmd_help(ccmd,
+"[-f FMT | --image-opts] [-b] [-U] [--object OBJDEF]\n"
+"  [--output human|json] FILENAME\n"
+,
+" -f|--format FMT - specify FILENAME image format explicitly\n"
+" --image-opts - indicates that FILENAME is a complete image specification\n"
+"  instead of a file name (incompatible with --format)\n"
+" -b|--backing-chain - display information about backing chaing\n"
+" (in case the image is stacked\n"
+" -U|--force-share - open image in shared mode for concurrent access\n"
+" --object OBJDEF - QEMU user-creatable object (eg encryption key)\n"
+" --output human|json - specify output format name (default human)\n"
+" FILENAME - image file name (or specification with --image-opts)\n"
+);
 break;
 case 'f':
 fmt = optarg;
@@ -3203,7 +3215,7 @@ static int img_info(const img_cmd_t *ccmd, int argc, char 
**argv)
 case OPTION_OUTPUT:
 output_format = parse_output_format(ccmd, optarg);
 break;
-case OPTION_BACKING_CHAIN:
+case 'b':
 chain = true;
 break;
 case OPTION_OBJECT:
-- 
2.39.2

[PATCH 02/23] qemu-img: refresh options/--help for "create" subcommand

[Michael Tokarev]

Add missing long options (eg --format).

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

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

diff --git a/qemu-img.c b/qemu-img.c
index 05f80b6e5b..7edfc56572 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -126,6 +126,25 @@ void unrecognized_option(const img_cmd_t *ccmd, const char 
*option)
 error_exit(ccmd, "unrecognized option '%s'", option);
 }
 
+/*
+ * Print --help output for a command and exit.
+ * syntax and description are multi-line with trailing EOL
+ * (to allow easy extending of the text)
+ * syntax has each subsequent line starting with \t
+ * desrciption is indented by one char
+ */
+static G_NORETURN
+void cmd_help(const img_cmd_t *ccmd,
+  const char *syntax, const char *arguments)
+{
+printf("qemu-img %s %s"
+   "Arguments:\n"
+   " -h|--help - print this help and exit\n"
+   "%s",
+   ccmd->name, syntax, arguments);
+exit(EXIT_SUCCESS);
+}
+
 /* Please keep in synch with docs/tools/qemu-img.rst */
 static G_NORETURN
 void help(void)
@@ -524,7 +543,13 @@ static int img_create(const img_cmd_t *ccmd, int argc, 
char **argv)
 for(;;) {
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
+{"quiet", no_argument, 0, 'q'},
 {"object", required_argument, 0, OPTION_OBJECT},
+{"format", required_argument, 0, 'f'},
+{"backing", required_argument, 0, 'b'},
+{"backing-format", required_argument, 0, 'F'},
+{"backing-unsafe", no_argument, 0, 'u'},
+{"options", required_argument, 0, 'o'},
 {0, 0, 0, 0}
 };
 c = getopt_long(argc, argv, ":F:b:f:ho:qu",
@@ -540,7 +565,25 @@ static int img_create(const img_cmd_t *ccmd, int argc, 
char **argv)
 unrecognized_option(ccmd, argv[optind - 1]);
 break;
 case 'h':
-help();
+cmd_help(ccmd,
+"[-f FMT] [-o FMT_OPTS] [-b BACKING_FILENAME [-F BACKING_FMT]]\n"
+"  [--object OBJDEF] [-u] FILENAME [SIZE[bkKMGTPE]]\n"
+,
+" -q|--quiet - quiet operations\n"
+" -f|--format FMT - specifies format of the new image, default is raw\n"
+" -o|--options FMT_OPTS - format-specific options ('-o list' for list)\n"
+" -b|--backing BACKING_FILENAME - stack new image on top of BACKING_FILENAME\n"
+"  (for formats which support stacking)\n"
+" -F|--backing-format BACKING_FMT - specify format of BACKING_FILENAME\n"
+" -u|--backing-unsafe - do not fail if BACKING_FMT can not be read\n"
+" --object OBJDEF - QEMU user-creatable object (eg encryption key)\n"
+" FILENAME - image file to create.  It will be overriden if exists\n"
+" SIZE - image size with optional suffix: 'b' (byte, default), 'k' or\n"
+"  'K' (kilobyte, 1024b), 'M' (megabyte, 1024K), 'G' (gigabyte, 1024M),\n"
+"  'T' (terabyte, 1024G), 'P' (petabyte, 1024T), or 'E' (exabyte, 1024P)\n"
+"  SIZE is required unless BACKING_IMG is specified, in which case\n"
+"  it will be the same as size of BACKING_IMG\n"
+);
 break;
 case 'F':
 base_fmt = optarg;
-- 
2.39.2

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

[Michael Tokarev]

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

diff --git a/qemu-img.c b/qemu-img.c
index 1e09b78d00..d9dfff2f07 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3541,7 +3541,7 @@ static int img_snapshot(const img_cmd_t *ccmd, int argc, 
char **argv)
 BlockDriverState *bs;
 QEMUSnapshotInfo sn;
 char *filename, *fmt = NULL, *snapshot_name = NULL;
-int c, ret = 0, bdrv_oflags;
+int c, ret = 0;
 int action = 0;
 bool quiet = false;
 Error *err = NULL;
@@ -3549,7 +3549,6 @@ static int img_snapshot(const img_cmd_t *ccmd, int argc, 
char **argv)
 bool force_share = false;
 int64_t rt;
 
-bdrv_oflags = BDRV_O_RDWR;
 /* Parse commandline parameters */
 for(;;) {
 static const struct option long_options[] = {
@@ -3583,7 +3582,6 @@ static int img_snapshot(const img_cmd_t *ccmd, int argc, 
char **argv)
 return 0;
 }
 action = SNAPSHOT_LIST;
-bdrv_oflags &= ~BDRV_O_RDWR; /* no need for RW */
 break;
 case 'a':
 if (action) {
@@ -3629,9 +3627,14 @@ static int img_snapshot(const img_cmd_t *ccmd, int argc, 
char **argv)
 }
 filename = argv[optind++];
 
+if (!action) {
+action = SNAPSHOT_LIST;
+}
+
 /* Open the image */
-blk = img_open(image_opts, filename, fmt, bdrv_oflags, false, quiet,
-   force_share);
+blk = img_open(image_opts, filename, fmt,
+   action == SNAPSHOT_LIST ? 0 : BDRV_O_RDWR,
+   false, quiet, force_share);
 if (!blk) {
 return 1;
 }
-- 
2.39.2

Re: KVM/QEMU Community Call 6th Feb Agenda Items

[Philippe Mathieu-Daudé]

On 2/2/24 16:55, Alex Bennée wrote:

Hi,

The KVM/QEMU community call is at:

   https://meet.jit.si/kvmcallmeeting
   @
   6/2/2024 14:00 UTC


BTW there is a KVM-only community call every weeks
on Wednesday:
https://lore.kernel.org/lkml/20230522072508.ga326...@chaop.bj.intel.com/T/

Maybe time to name this one simply "QEMU"?

Re: KVM/QEMU Community Call 6th Feb Agenda Items (minutes)

[Philippe Mathieu-Daudé]

Hi,

On 2/2/24 16:55, Alex Bennée wrote:


The KVM/QEMU community call is at:

   https://meet.jit.si/kvmcallmeeting
   @
   6/2/2024 14:00 UTC

As I'll be away Philippe has volunteered to run the meeting.

So far we have one agenda item which is to discuss next steps from
Markus' post about dynamic modelling:

   Message-ID: <87o7d1i7ky@pond.sub.org>
   Date: Wed, 31 Jan 2024 21:14:21 +0100
   Subject: Dynamic & heterogeneous machines, initial configuration: problems


I took notes on a document but am unable to share it so far,
so I'm sharing the minutes replying here.

Some people complained the call wasn't announced like previous
ones with Juan calendar invite mail, so I built this Cc list from
the old calendar invite recipient list and mixed with the list from:
https://etherpad.opendev.org/p/qemu-emulation-bof%402022-11-29

Unfortunately exporting broke the document indenting, I tried to
adapt it my best.

---< minutes >---

6 Feb 2024 | QEMU community call

Attendees:
- Philippe Mathieu-Daudé
- Zhao Liu
- Markus Armbruster
- Mark Burton
- Anton Johansson
- Cédric Le Goater
- Bernhard Beschow
- Maciej S. Szmigiero
- Manos Pitsidianakis

Notes
- Discussion around https://lore.kernel.org/all/87o7d1i7ky@pond.sub.org/
- Future of NUMA topology (current options are too limited)
- should we add more CLI options
- Move to QOM containers (better for DynMach)
- Ambitious projects, how to share work?
- We can not go in all directions at once
- start a new binary to experiment (no backwards compatibility 
problems)

- Can we work in parallel? Are there serial steps?
- Can we avoid working on a fork?
- Should we avoid QMP as DSL to describe machine
- Convert QEMU code base to libraries?
- Is QDev DeviceRealize sufficient? SystemC steps:
- Construct
- Config start
- Config done
- Before end of elaboration
- Wire / Connect
- After end of elaboration
- Ready to start simulation
- Reset?
- Next steps?
- Add new experimental target: Phil
- Initial startup: Markus
- Currently QMP available too late for dynamic machine
- Define clean QEMU init state machine
- Shared TCG frontends: Anton
- Create QDev devices from QMP?
- Clarify QDev states
- Start QDev2 in-tree to avoid churn with the current QDev API?

Action items
[ ] Template for this document
[ ] Share this document out of Linaro
[ ] Setup a community calendar invite, Cc’ing interested people or 
letting them subscribe

[ ] Next meeting next week!

---< /minutes >---

In order to share tasks I put the "next steps" items on the wiki:
https://wiki.qemu.org/Dynamic_machine_and_heterogeneous_emulation_roadmap
Feel free to add any relevant steps and where you want to help.

Mark Burton asked if Peter Maydell and Paolo Bonzini could be in the
next week call (Tue 13) to discuss about next steps and whether a
in-tree prototype binary could be accepted in mainstream or not.

A such prototype binary has been proposed here:
https://lore.kernel.org/qemu-devel/20240209152945.25727-1-phi...@linaro.org/
Personally I think at this point there are no blocker so we can
discuss on the list as usual, there is no need for someone specific
presence to debate on this (yet) IMHO.

Still we'll have a call next week and discuss the roadmap. An
any other topics the community want to bring.

Regards,

Phil.

Re: [PATCH] hw/hppa/Kconfig: Fix building with "configure --without-default-devices"

[Philippe Mathieu-Daudé]

On 9/2/24 20:46, BALATON Zoltan wrote:

On Fri, 9 Feb 2024, Helge Deller wrote:

On 2/9/24 19:55, Thomas Huth wrote:

When running "configure" with "--without-default-devices", building
of qemu-system-hppa currently fails with:

  /usr/bin/ld: libqemu-hppa-softmmu.fa.p/hw_hppa_machine.c.o: in 
function `machine_HP_common_init_tail':

  hw/hppa/machine.c:399: undefined reference to `usb_bus_find'
  /usr/bin/ld: hw/hppa/machine.c:399: undefined reference to 
`usb_create_simple'
  /usr/bin/ld: hw/hppa/machine.c:400: undefined reference to 
`usb_bus_find'
  /usr/bin/ld: hw/hppa/machine.c:400: undefined reference to 
`usb_create_simple'

  collect2: error: ld returned 1 exit status
  ninja: build stopped: subcommand failed.
  make: *** [Makefile:162: run-ninja] Error 1

And after fixing this, the qemu-system-hppa binary refuses to run
due to the missing 'pci-ohci' and 'pci-serial' devices. Let's add
the right config switches to fix these problems.

Signed-off-by: Thomas Huth 
---
  hw/hppa/Kconfig | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/hppa/Kconfig b/hw/hppa/Kconfig
index ff8528aaa8..124d5e9e47 100644
--- a/hw/hppa/Kconfig
+++ b/hw/hppa/Kconfig
@@ -6,7 +6,7 @@ config HPPA_B160L
  select ASTRO
  select DINO
  select LASI
-    select SERIAL
+    select SERIAL_PCI


I think the "SERIAL" is needed too for the B160L machine.


SERIAL_PCI selects SERIAL so I think it should be pulled in without 
listing it separately but not sure what's the policy for these configs.


We prefer explicit dependencies.

SERIAL is for Lasi/Dino serial_mm_init().

Although pulling in SERIAL, SERIAL_PCI is for pci-serial*.


Regards,
BALATON Zoltan


Other than that,

Acked-by: Helge Deller 

Thank you!
Helge


  select ISA_BUS
  select I8259
  select IDE_CMD646
@@ -16,3 +16,4 @@ config HPPA_B160L
  select LASIPS2
  select PARALLEL
  select ARTIST
+    select USB_OHCI_PCI

[PULL 00/10] testing, doc and gdbstub updates

[Alex Bennée]

The following changes since commit 5d1fc614413b10dd94858b07a1b2e26b1aa0296c:

  Merge tag 'migration-staging-pull-request' of https://gitlab.com/peterx/qemu 
into staging (2024-02-09 11:22:20 +)

are available in the Git repository at:

  https://gitlab.com/stsquad/qemu.git tags/pull-maintainer-updates-090224-1

for you to fetch changes up to 86b75667e04b49a0b75f061f589b3fbec3fb78f1:

  tests/tcg: Add the syscall catchpoint gdbstub test (2024-02-09 17:52:40 +)


testing, doc and gdbstub updates:

  - add sqlite3 to openSUSE image
  - mark CRIS as deprecated
  - re-enable the TCG plugin tests
  - use select for semihosting
  - implement syscall catching in gdbstub


Alex Bennée (2):
  docs: mark CRIS support as deprecated
  Revert "hw/elf_ops: Ignore loadable segments with zero size"

Fabiano Rosas (1):
  tests/docker: Add sqlite3 module to openSUSE Leap container

Ilya Leoshkevich (5):
  gdbstub: Expose TARGET_SIGTRAP in a target-agnostic way
  gdbstub: Allow specifying a reason in stop packets
  gdbstub: Add syscall entry/return hooks
  gdbstub: Implement catching syscalls
  tests/tcg: Add the syscall catchpoint gdbstub test

Paolo Bonzini (2):
  configure: run plugin TCG tests again
  kconfig: use "select" to enable semihosting

 docs/about/deprecated.rst |   7 ++
 configure |   3 +
 configs/devices/m68k-softmmu/default.mak  |   2 -
 configs/devices/mips-softmmu/common.mak   |   3 -
 configs/devices/nios2-softmmu/default.mak |   2 -
 configs/devices/riscv32-softmmu/default.mak   |   2 -
 configs/devices/riscv64-softmmu/default.mak   |   2 -
 configs/devices/xtensa-softmmu/default.mak|   2 -
 gdbstub/internals.h   |   2 +
 include/gdbstub/user.h|  29 ++-
 include/hw/elf_ops.h  |  75 +--
 include/user/syscall-trace.h  |   7 +-
 gdbstub/gdbstub.c |   9 +++
 gdbstub/user-target.c |   5 ++
 gdbstub/user.c| 104 +-
 tests/tcg/multiarch/catch-syscalls.c  |  51 +
 target/m68k/Kconfig   |   1 +
 target/mips/Kconfig   |   1 +
 target/nios2/Kconfig  |   1 +
 target/riscv/Kconfig  |   2 +
 target/xtensa/Kconfig |   1 +
 tests/docker/dockerfiles/opensuse-leap.docker |   1 +
 tests/lcitool/mappings.yml|   4 +
 tests/lcitool/projects/qemu.yml   |   1 +
 tests/tcg/multiarch/Makefile.target   |  10 ++-
 tests/tcg/multiarch/gdbstub/catch-syscalls.py |  53 +
 26 files changed, 322 insertions(+), 58 deletions(-)
 create mode 100644 tests/tcg/multiarch/catch-syscalls.c
 create mode 100644 tests/tcg/multiarch/gdbstub/catch-syscalls.py

-- 
2.39.2

[PULL 07/10] gdbstub: Allow specifying a reason in stop packets

[Alex Bennée]

From: Ilya Leoshkevich 

The upcoming syscall catchpoint support needs to send stop packets with
an associated reason to GDB. Add an extra parameter to gdb_handlesig()
for that, and rename it to gdb_handlesig_reason(). Provide a
compatibility wrapper with an old name.

Signed-off-by: Ilya Leoshkevich 
Message-Id: <20240202152506.279476-3-...@linux.ibm.com>
Signed-off-by: Alex Bennée 
Message-Id: <20240207163812.3231697-12-alex.ben...@linaro.org>

diff --git a/include/gdbstub/user.h b/include/gdbstub/user.h
index d392e510c59..1fc43e04af5 100644
--- a/include/gdbstub/user.h
+++ b/include/gdbstub/user.h
@@ -10,9 +10,10 @@
 #define GDBSTUB_USER_H
 
 /**
- * gdb_handlesig() - yield control to gdb
+ * gdb_handlesig_reason() - yield control to gdb
  * @cpu: CPU
  * @sig: if non-zero, the signal number which caused us to stop
+ * @reason: stop reason for stop reply packet or NULL
  *
  * This function yields control to gdb, when a user-mode-only target
  * needs to stop execution. If @sig is non-zero, then we will send a
@@ -24,7 +25,18 @@
  * or 0 if no signal should be delivered, ie the signal that caused
  * us to stop should be ignored.
  */
-int gdb_handlesig(CPUState *, int);
+int gdb_handlesig_reason(CPUState *, int, const char *);
+
+/**
+ * gdb_handlesig() - yield control to gdb
+ * @cpu CPU
+ * @sig: if non-zero, the signal number which caused us to stop
+ * @see gdb_handlesig_reason()
+ */
+static inline int gdb_handlesig(CPUState *cpu, int sig)
+{
+return gdb_handlesig_reason(cpu, sig, NULL);
+}
 
 /**
  * gdb_signalled() - inform remote gdb of sig exit
diff --git a/gdbstub/user.c b/gdbstub/user.c
index dbe1d9b8875..63edca131ab 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -121,7 +121,7 @@ void gdb_qemu_exit(int code)
 exit(code);
 }
 
-int gdb_handlesig(CPUState *cpu, int sig)
+int gdb_handlesig_reason(CPUState *cpu, int sig, const char *reason)
 {
 char buf[256];
 int n;
@@ -141,6 +141,9 @@ int gdb_handlesig(CPUState *cpu, int sig)
 "T%02xthread:", gdb_target_signal_to_gdb(sig));
 gdb_append_thread_id(cpu, gdbserver_state.str_buf);
 g_string_append_c(gdbserver_state.str_buf, ';');
+if (reason) {
+g_string_append(gdbserver_state.str_buf, reason);
+}
 gdb_put_strbuf();
 gdbserver_state.allow_stop_reply = false;
 }
-- 
2.39.2

[PULL 01/10] tests/docker: Add sqlite3 module to openSUSE Leap container

[Alex Bennée]

From: Fabiano Rosas 

Avocado needs sqlite3:

  Failed to load plugin from module "avocado.plugins.journal":
  ImportError("Module 'sqlite3' is not installed.
  Use: sudo zypper install python311 to install it")

>From 'zypper info python311':
  "This package supplies rich command line features provided by
  readline, and sqlite3 support for the interpreter core, thus forming
  a so called "extended" runtime."

Include the appropriate package in the lcitool mappings which will
guarantee the dockerfile gets properly updated when lcitool is
run. Also include the updated dockerfile.

Signed-off-by: Fabiano Rosas 
Suggested-by: Andrea Bolognani 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20240117164227.32143-1-faro...@suse.de>
Signed-off-by: Alex Bennée 
Message-Id: <20240207163812.3231697-2-alex.ben...@linaro.org>

diff --git a/tests/docker/dockerfiles/opensuse-leap.docker 
b/tests/docker/dockerfiles/opensuse-leap.docker
index dc0e36ce488..cf753383a45 100644
--- a/tests/docker/dockerfiles/opensuse-leap.docker
+++ b/tests/docker/dockerfiles/opensuse-leap.docker
@@ -90,6 +90,7 @@ RUN zypper update -y && \
pcre-devel-static \
pipewire-devel \
pkgconfig \
+   python311 \
python311-base \
python311-pip \
python311-setuptools \
diff --git a/tests/lcitool/mappings.yml b/tests/lcitool/mappings.yml
index 0b908882f1d..407c03301bf 100644
--- a/tests/lcitool/mappings.yml
+++ b/tests/lcitool/mappings.yml
@@ -59,6 +59,10 @@ mappings:
 CentOSStream8:
 OpenSUSELeap15:
 
+  python3-sqlite3:
+CentOSStream8: python38
+OpenSUSELeap15: python311
+
   python3-tomli:
 # test using tomllib
 apk:
diff --git a/tests/lcitool/projects/qemu.yml b/tests/lcitool/projects/qemu.yml
index 82092c9f175..149b15de57b 100644
--- a/tests/lcitool/projects/qemu.yml
+++ b/tests/lcitool/projects/qemu.yml
@@ -97,6 +97,7 @@ packages:
  - python3-pip
  - python3-sphinx
  - python3-sphinx-rtd-theme
+ - python3-sqlite3
  - python3-tomli
  - python3-venv
  - rpm2cpio
-- 
2.39.2

[PULL 09/10] gdbstub: Implement catching syscalls

[Alex Bennée]

From: Ilya Leoshkevich 

GDB supports stopping on syscall entry and exit using the "catch
syscall" command. It relies on 3 packets, which are currently not
supported by QEMU:

* qSupported:QCatchSyscalls+ [1]
* QCatchSyscalls: [2]
* T05syscall_entry: and T05syscall_return: [3]

Implement generation and handling of these packets.

[1] 
https://sourceware.org/gdb/current/onlinedocs/gdb.html/General-Query-Packets.html#qSupported
[2] 
https://sourceware.org/gdb/current/onlinedocs/gdb.html/General-Query-Packets.html#QCatchSyscalls
[3] 
https://sourceware.org/gdb/current/onlinedocs/gdb.html/Stop-Reply-Packets.html

Signed-off-by: Ilya Leoshkevich 
Message-Id: <20240202152506.279476-5-...@linux.ibm.com>
[AJB: GString -> g_strdup_printf]
Signed-off-by: Alex Bennée 
Message-Id: <20240207163812.3231697-14-alex.ben...@linaro.org>

diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index aeb0d9b5377..56b7c13b750 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -195,6 +195,7 @@ void gdb_handle_v_file_close(GArray *params, void 
*user_ctx); /* user */
 void gdb_handle_v_file_pread(GArray *params, void *user_ctx); /* user */
 void gdb_handle_v_file_readlink(GArray *params, void *user_ctx); /* user */
 void gdb_handle_query_xfer_exec_file(GArray *params, void *user_ctx); /* user 
*/
+void gdb_handle_set_catch_syscalls(GArray *params, void *user_ctx); /* user */
 
 void gdb_handle_query_attached(GArray *params, void *user_ctx); /* both */
 
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 46d752bbc2c..7e73e916bdc 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -1617,6 +1617,7 @@ static void handle_query_supported(GArray *params, void 
*user_ctx)
 if (gdbserver_state.c_cpu->opaque) {
 g_string_append(gdbserver_state.str_buf, ";qXfer:auxv:read+");
 }
+g_string_append(gdbserver_state.str_buf, ";QCatchSyscalls+");
 #endif
 g_string_append(gdbserver_state.str_buf, ";qXfer:exec-file:read+");
 #endif
@@ -1810,6 +1811,14 @@ static const GdbCmdParseEntry gdb_gen_set_table[] = {
 .schema = "l0"
 },
 #endif
+#if defined(CONFIG_USER_ONLY)
+{
+.handler = gdb_handle_set_catch_syscalls,
+.cmd = "CatchSyscalls:",
+.cmd_startswith = 1,
+.schema = "s0",
+},
+#endif
 };
 
 static void handle_gen_query(GArray *params, void *user_ctx)
diff --git a/gdbstub/user.c b/gdbstub/user.c
index 2ba01c17faf..14918d1a217 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -10,6 +10,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/bitops.h"
 #include "qemu/cutils.h"
 #include "qemu/sockets.h"
 #include "exec/hwaddr.h"
@@ -21,11 +22,20 @@
 #include "trace.h"
 #include "internals.h"
 
+#define GDB_NR_SYSCALLS 1024
+typedef unsigned long GDBSyscallsMask[BITS_TO_LONGS(GDB_NR_SYSCALLS)];
+
 /* User-mode specific state */
 typedef struct {
 int fd;
 char *socket_path;
 int running_state;
+/*
+ * Store syscalls mask without memory allocation in order to avoid
+ * implementing synchronization.
+ */
+bool catch_all_syscalls;
+GDBSyscallsMask catch_syscalls_mask;
 } GDBUserState;
 
 static GDBUserState gdbserver_user_state;
@@ -503,10 +513,91 @@ void gdb_syscall_handling(const char *syscall_packet)
 gdb_handlesig(gdbserver_state.c_cpu, 0);
 }
 
+static bool should_catch_syscall(int num)
+{
+if (gdbserver_user_state.catch_all_syscalls) {
+return true;
+}
+if (num < 0 || num >= GDB_NR_SYSCALLS) {
+return false;
+}
+return test_bit(num, gdbserver_user_state.catch_syscalls_mask);
+}
+
 void gdb_syscall_entry(CPUState *cs, int num)
 {
+if (should_catch_syscall(num)) {
+g_autofree char *reason = g_strdup_printf("syscall_entry:%x;", num);
+gdb_handlesig_reason(cs, gdb_target_sigtrap(), reason);
+}
 }
 
 void gdb_syscall_return(CPUState *cs, int num)
 {
+if (should_catch_syscall(num)) {
+g_autofree char *reason = g_strdup_printf("syscall_return:%x;", num);
+gdb_handlesig_reason(cs, gdb_target_sigtrap(), reason);
+}
+}
+
+void gdb_handle_set_catch_syscalls(GArray *params, void *user_ctx)
+{
+const char *param = get_param(params, 0)->data;
+GDBSyscallsMask catch_syscalls_mask;
+bool catch_all_syscalls;
+unsigned int num;
+const char *p;
+
+/* "0" means not catching any syscalls. */
+if (strcmp(param, "0") == 0) {
+gdbserver_user_state.catch_all_syscalls = false;
+memset(gdbserver_user_state.catch_syscalls_mask, 0,
+   sizeof(gdbserver_user_state.catch_syscalls_mask));
+gdb_put_packet("OK");
+return;
+}
+
+/* "1" means catching all syscalls. */
+if (strcmp(param, "1") == 0) {
+gdbserver_user_state.catch_all_syscalls = true;
+gdb_put_packet("OK");
+return;
+}
+
+/*
+ * "1;..." means catching only the specified syscalls.
+ * The syscall list must not be empty.
+ */
+if (param[0] == '1' && param[1] == ';') {
+  

[PULL 08/10] gdbstub: Add syscall entry/return hooks

[Alex Bennée]

From: Ilya Leoshkevich 

The upcoming syscall catchpoint support needs to get control on syscall
entry and return. Provide the necessary hooks for that, which are
no-ops for now.

Signed-off-by: Ilya Leoshkevich 
Message-Id: <20240202152506.279476-4-...@linux.ibm.com>
Signed-off-by: Alex Bennée 
Message-Id: <20240207163812.3231697-13-alex.ben...@linaro.org>

diff --git a/include/gdbstub/user.h b/include/gdbstub/user.h
index 1fc43e04af5..68b6534130c 100644
--- a/include/gdbstub/user.h
+++ b/include/gdbstub/user.h
@@ -51,5 +51,18 @@ void gdb_signalled(CPUArchState *as, int sig);
  */
 void gdbserver_fork(CPUState *cs);
 
+/**
+ * gdb_syscall_entry() - inform gdb of syscall entry and yield control to it
+ * @cs: CPU
+ * @num: syscall number
+ */
+void gdb_syscall_entry(CPUState *cs, int num);
+
+/**
+ * gdb_syscall_entry() - inform gdb of syscall return and yield control to it
+ * @cs: CPU
+ * @num: syscall number
+ */
+void gdb_syscall_return(CPUState *cs, int num);
 
 #endif /* GDBSTUB_USER_H */
diff --git a/include/user/syscall-trace.h b/include/user/syscall-trace.h
index 557f881a79b..b48b2b2d0ae 100644
--- a/include/user/syscall-trace.h
+++ b/include/user/syscall-trace.h
@@ -11,6 +11,7 @@
 #define SYSCALL_TRACE_H
 
 #include "exec/user/abitypes.h"
+#include "gdbstub/user.h"
 #include "qemu/plugin.h"
 #include "trace/trace-root.h"
 
@@ -20,7 +21,7 @@
  * could potentially unify the -strace code here as well.
  */
 
-static inline void record_syscall_start(void *cpu, int num,
+static inline void record_syscall_start(CPUState *cpu, int num,
 abi_long arg1, abi_long arg2,
 abi_long arg3, abi_long arg4,
 abi_long arg5, abi_long arg6,
@@ -29,11 +30,13 @@ static inline void record_syscall_start(void *cpu, int num,
 qemu_plugin_vcpu_syscall(cpu, num,
  arg1, arg2, arg3, arg4,
  arg5, arg6, arg7, arg8);
+gdb_syscall_entry(cpu, num);
 }
 
-static inline void record_syscall_return(void *cpu, int num, abi_long ret)
+static inline void record_syscall_return(CPUState *cpu, int num, abi_long ret)
 {
 qemu_plugin_vcpu_syscall_ret(cpu, num, ret);
+gdb_syscall_return(cpu, num);
 }
 
 
diff --git a/gdbstub/user.c b/gdbstub/user.c
index 63edca131ab..2ba01c17faf 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -502,3 +502,11 @@ void gdb_syscall_handling(const char *syscall_packet)
 gdb_put_packet(syscall_packet);
 gdb_handlesig(gdbserver_state.c_cpu, 0);
 }
+
+void gdb_syscall_entry(CPUState *cs, int num)
+{
+}
+
+void gdb_syscall_return(CPUState *cs, int num)
+{
+}
-- 
2.39.2

[PULL 04/10] Revert "hw/elf_ops: Ignore loadable segments with zero size"

[Alex Bennée]

This regressed qemu-system-xtensa:

TESTtest_load_store on xtensa
  qemu-system-xtensa: Some ROM regions are overlapping
  These ROM regions might have been loaded by direct user request or by default.
  They could be BIOS/firmware images, a guest kernel, initrd or some other file 
loaded into guest memory.
  Check whether you intended to load all this guest code, and whether it has 
been built to load to the correct addresses.

  The following two regions overlap (in the memory address space):
test_load_store ELF program header segment 1 (addresses 0x1000 
- 0x1f26)
test_load_store ELF program header segment 2 (addresses 0x1ab8 
- 0x1ab8)
  make[1]: *** [Makefile:187: run-test_load_store] Error 1

This reverts commit 62570f1434160d356311e1c217537e24a4ac85cd.

Reviewed-by: Thomas Huth 
Signed-off-by: Alex Bennée 
Message-Id: <20240207163812.3231697-5-alex.ben...@linaro.org>

diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index 3e966ddd5a1..9c35d1b9da6 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -427,16 +427,6 @@ static ssize_t glue(load_elf, SZ)(const char *name, int fd,
 file_size = ph->p_filesz; /* Size of the allocated data */
 data_offset = ph->p_offset; /* Offset where the data is located */
 
-/*
- * Some ELF files really do have segments of zero size;
- * just ignore them rather than trying to set the wrong addr,
- * or create empty ROM blobs, because the zero-length blob can
- * falsely trigger the overlapping-ROM-blobs check.
- */
-if (mem_size == 0) {
-continue;
-}
-
 if (file_size > 0) {
 if (g_mapped_file_get_length(mapped_file) <
 file_size + data_offset) {
@@ -540,38 +530,45 @@ static ssize_t glue(load_elf, SZ)(const char *name, int 
fd,
 *pentry = ehdr.e_entry - ph->p_vaddr + ph->p_paddr;
 }
 
-if (load_rom) {
-g_autofree char *label =
-g_strdup_printf("%s ELF program header segment %d",
-name, i);
-
-/*
- * rom_add_elf_program() takes its own reference to
- * 'mapped_file'.
- */
-rom_add_elf_program(label, mapped_file, data, file_size,
-mem_size, addr, as);
-} else {
-MemTxResult res;
-
-res = address_space_write(as ? as : _space_memory,
-  addr, MEMTXATTRS_UNSPECIFIED,
-  data, file_size);
-if (res != MEMTX_OK) {
-goto fail;
-}
-/*
- * We need to zero'ify the space that is not copied
- * from file
- */
-if (file_size < mem_size) {
-res = address_space_set(as ? as : _space_memory,
-addr + file_size, 0,
-mem_size - file_size,
-MEMTXATTRS_UNSPECIFIED);
+/* Some ELF files really do have segments of zero size;
+ * just ignore them rather than trying to create empty
+ * ROM blobs, because the zero-length blob can falsely
+ * trigger the overlapping-ROM-blobs check.
+ */
+if (mem_size != 0) {
+if (load_rom) {
+g_autofree char *label =
+g_strdup_printf("%s ELF program header segment %d",
+name, i);
+
+/*
+ * rom_add_elf_program() takes its own reference to
+ * 'mapped_file'.
+ */
+rom_add_elf_program(label, mapped_file, data, file_size,
+mem_size, addr, as);
+} else {
+MemTxResult res;
+
+res = address_space_write(as ? as : _space_memory,
+  addr, MEMTXATTRS_UNSPECIFIED,
+  data, file_size);
 if (res != MEMTX_OK) {
 goto fail;
 }
+/*
+ * We need to zero'ify the space that is not copied
+ * from file
+ */
+if (file_size < mem_size) {
+res = address_space_set(as ? as : 
_space_memory,
+addr + file_size, 0,
+mem_size - file_size,
+

[PULL 10/10] tests/tcg: Add the syscall catchpoint gdbstub test

[Alex Bennée]

From: Ilya Leoshkevich 

Check that adding/removing syscall catchpoints works.

Reviewed-by: Alex Bennée 
Signed-off-by: Ilya Leoshkevich 
Message-Id: <20240202152506.279476-6-...@linux.ibm.com>
Signed-off-by: Alex Bennée 
Message-Id: <20240207163812.3231697-15-alex.ben...@linaro.org>

diff --git a/tests/tcg/multiarch/catch-syscalls.c 
b/tests/tcg/multiarch/catch-syscalls.c
new file mode 100644
index 000..d1ff1936a7a
--- /dev/null
+++ b/tests/tcg/multiarch/catch-syscalls.c
@@ -0,0 +1,51 @@
+/*
+ * Test GDB syscall catchpoints.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#define _GNU_SOURCE
+#include 
+#include 
+
+const char *catch_syscalls_state = "start";
+
+void end_of_main(void)
+{
+}
+
+int main(void)
+{
+int ret = EXIT_FAILURE;
+char c0 = 'A', c1;
+int fd[2];
+
+catch_syscalls_state = "pipe2";
+if (pipe2(fd, 0)) {
+goto out;
+}
+
+catch_syscalls_state = "write";
+if (write(fd[1], , sizeof(c0)) != sizeof(c0)) {
+goto out_close;
+}
+
+catch_syscalls_state = "read";
+if (read(fd[0], , sizeof(c1)) != sizeof(c1)) {
+goto out_close;
+}
+
+catch_syscalls_state = "check";
+if (c0 == c1) {
+ret = EXIT_SUCCESS;
+}
+
+out_close:
+catch_syscalls_state = "close";
+close(fd[0]);
+close(fd[1]);
+
+out:
+catch_syscalls_state = "end";
+end_of_main();
+return ret;
+}
diff --git a/tests/tcg/multiarch/Makefile.target 
b/tests/tcg/multiarch/Makefile.target
index 315a2e13588..e10951a8016 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -108,13 +108,21 @@ run-gdbstub-prot-none: prot-none
--bin $< --test $(MULTIARCH_SRC)/gdbstub/prot-none.py, \
accessing PROT_NONE memory)
 
+run-gdbstub-catch-syscalls: catch-syscalls
+   $(call run-test, $@, $(GDB_SCRIPT) \
+   --gdb $(GDB) \
+   --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+   --bin $< --test $(MULTIARCH_SRC)/gdbstub/catch-syscalls.py, \
+   hitting a syscall catchpoint)
+
 else
 run-gdbstub-%:
$(call skip-test, "gdbstub test $*", "need working gdb with $(patsubst 
-%,,$(TARGET_NAME)) support")
 endif
 EXTRA_RUNS += run-gdbstub-sha1 run-gdbstub-qxfer-auxv-read \
  run-gdbstub-proc-mappings run-gdbstub-thread-breakpoint \
- run-gdbstub-registers run-gdbstub-prot-none
+ run-gdbstub-registers run-gdbstub-prot-none \
+ run-gdbstub-catch-syscalls
 
 # ARM Compatible Semi Hosting Tests
 #
diff --git a/tests/tcg/multiarch/gdbstub/catch-syscalls.py 
b/tests/tcg/multiarch/gdbstub/catch-syscalls.py
new file mode 100644
index 000..ccce35902fb
--- /dev/null
+++ b/tests/tcg/multiarch/gdbstub/catch-syscalls.py
@@ -0,0 +1,53 @@
+"""Test GDB syscall catchpoints.
+
+SPDX-License-Identifier: GPL-2.0-or-later
+"""
+from test_gdbstub import main, report
+
+
+def check_state(expected):
+"""Check the catch_syscalls_state value"""
+actual = gdb.parse_and_eval("catch_syscalls_state").string()
+report(actual == expected, "{} == {}".format(actual, expected))
+
+
+def run_test():
+"""Run through the tests one by one"""
+gdb.Breakpoint("main")
+gdb.execute("continue")
+
+# Check that GDB stops for pipe2/read calls/returns, but not for write.
+gdb.execute("delete")
+try:
+gdb.execute("catch syscall pipe2 read")
+except gdb.error as exc:
+exc_str = str(exc)
+if "not supported on this architecture" in exc_str:
+print("SKIP: {}".format(exc_str))
+return
+raise
+for _ in range(2):
+gdb.execute("continue")
+check_state("pipe2")
+for _ in range(2):
+gdb.execute("continue")
+check_state("read")
+
+# Check that deletion works.
+gdb.execute("delete")
+gdb.Breakpoint("end_of_main")
+gdb.execute("continue")
+check_state("end")
+
+# Check that catch-all works (libc should at least call exit).
+gdb.execute("delete")
+gdb.execute("catch syscall")
+gdb.execute("continue")
+gdb.execute("delete")
+gdb.execute("continue")
+
+exitcode = int(gdb.parse_and_eval("$_exitcode"))
+report(exitcode == 0, "{} == 0".format(exitcode))
+
+
+main(run_test)
-- 
2.39.2

[PULL 06/10] gdbstub: Expose TARGET_SIGTRAP in a target-agnostic way

[Alex Bennée]

From: Ilya Leoshkevich 

The upcoming syscall catchpoint support needs to send SIGTRAP stop
packets to GDB. Being able to compile this support only once for all
targets is a good thing, and it requires hiding TARGET_SIGTRAP behind
a function call.

Signed-off-by: Ilya Leoshkevich 
Message-Id: <20240202152506.279476-2-...@linux.ibm.com>
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Alex Bennée 
Message-Id: <20240207163812.3231697-11-alex.ben...@linaro.org>

diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index 5c0c725e54c..aeb0d9b5377 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -136,6 +136,7 @@ void gdb_append_thread_id(CPUState *cpu, GString *buf);
 int gdb_get_cpu_index(CPUState *cpu);
 unsigned int gdb_get_max_cpus(void); /* both */
 bool gdb_can_reverse(void); /* softmmu, stub for user */
+int gdb_target_sigtrap(void); /* user */
 
 void gdb_create_default_process(GDBState *s);
 
diff --git a/gdbstub/user-target.c b/gdbstub/user-target.c
index c4bba4c72c7..b7d4c37cd81 100644
--- a/gdbstub/user-target.c
+++ b/gdbstub/user-target.c
@@ -418,3 +418,8 @@ void gdb_handle_query_xfer_exec_file(GArray *params, void 
*user_ctx)
 ts->bprm->filename + offset);
 gdb_put_strbuf();
 }
+
+int gdb_target_sigtrap(void)
+{
+return TARGET_SIGTRAP;
+}
-- 
2.39.2

[PULL 05/10] kconfig: use "select" to enable semihosting

[Alex Bennée]

From: Paolo Bonzini 

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

Signed-off-by: Paolo Bonzini 
Acked-by: Alistair Francis 
Reviewed-by: Thomas Huth 
Message-Id: <20240129115809.1039924-1-pbonz...@redhat.com>
Signed-off-by: Alex Bennée 
Message-Id: <20240207163812.3231697-10-alex.ben...@linaro.org>

diff --git a/configs/devices/m68k-softmmu/default.mak 
b/configs/devices/m68k-softmmu/default.mak
index 7f8619e4278..8dcaa28ed38 100644
--- a/configs/devices/m68k-softmmu/default.mak
+++ b/configs/devices/m68k-softmmu/default.mak
@@ -1,7 +1,5 @@
 # Default configuration for m68k-softmmu
 
-CONFIG_SEMIHOSTING=y
-
 # Boards:
 #
 CONFIG_AN5206=y
diff --git a/configs/devices/mips-softmmu/common.mak 
b/configs/devices/mips-softmmu/common.mak
index 7da99327a77..1a853841b27 100644
--- a/configs/devices/mips-softmmu/common.mak
+++ b/configs/devices/mips-softmmu/common.mak
@@ -1,8 +1,5 @@
 # Common mips*-softmmu CONFIG defines
 
-# CONFIG_SEMIHOSTING is always required on this architecture
-CONFIG_SEMIHOSTING=y
-
 CONFIG_ISA_BUS=y
 CONFIG_PCI=y
 CONFIG_PCI_DEVICES=y
diff --git a/configs/devices/nios2-softmmu/default.mak 
b/configs/devices/nios2-softmmu/default.mak
index 1bc4082ea99..e130d024e62 100644
--- a/configs/devices/nios2-softmmu/default.mak
+++ b/configs/devices/nios2-softmmu/default.mak
@@ -1,7 +1,5 @@
 # Default configuration for nios2-softmmu
 
-CONFIG_SEMIHOSTING=y
-
 # Boards:
 #
 CONFIG_NIOS2_10M50=y
diff --git a/configs/devices/riscv32-softmmu/default.mak 
b/configs/devices/riscv32-softmmu/default.mak
index d847bd5692e..94a236c9c25 100644
--- a/configs/devices/riscv32-softmmu/default.mak
+++ b/configs/devices/riscv32-softmmu/default.mak
@@ -3,8 +3,6 @@
 # Uncomment the following lines to disable these optional devices:
 #
 #CONFIG_PCI_DEVICES=n
-CONFIG_SEMIHOSTING=y
-CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
 
 # Boards:
 #
diff --git a/configs/devices/riscv64-softmmu/default.mak 
b/configs/devices/riscv64-softmmu/default.mak
index bc69301fa4a..3f680594484 100644
--- a/configs/devices/riscv64-softmmu/default.mak
+++ b/configs/devices/riscv64-softmmu/default.mak
@@ -3,8 +3,6 @@
 # Uncomment the following lines to disable these optional devices:
 #
 #CONFIG_PCI_DEVICES=n
-CONFIG_SEMIHOSTING=y
-CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
 
 # Boards:
 #
diff --git a/configs/devices/xtensa-softmmu/default.mak 
b/configs/devices/xtensa-softmmu/default.mak
index 4fe1bf00c94..49e4c9da88c 100644
--- a/configs/devices/xtensa-softmmu/default.mak
+++ b/configs/devices/xtensa-softmmu/default.mak
@@ -1,7 +1,5 @@
 # Default configuration for Xtensa
 
-CONFIG_SEMIHOSTING=y
-
 # Boards:
 #
 CONFIG_XTENSA_SIM=y
diff --git a/target/m68k/Kconfig b/target/m68k/Kconfig
index 23debad519a..9eae71486ff 100644
--- a/target/m68k/Kconfig
+++ b/target/m68k/Kconfig
@@ -1,2 +1,3 @@
 config M68K
 bool
+select SEMIHOSTING
diff --git a/target/mips/Kconfig b/target/mips/Kconfig
index 6adf1453548..eb19c94c7d4 100644
--- a/target/mips/Kconfig
+++ b/target/mips/Kconfig
@@ -1,5 +1,6 @@
 config MIPS
 bool
+select SEMIHOSTING
 
 config MIPS64
 bool
diff --git a/target/nios2/Kconfig b/target/nios2/Kconfig
index 1529ab8950d..c65550c861a 100644
--- a/target/nios2/Kconfig
+++ b/target/nios2/Kconfig
@@ -1,2 +1,3 @@
 config NIOS2
 bool
+select SEMIHOSTING
diff --git a/target/riscv/Kconfig b/target/riscv/Kconfig
index b9e5932f13f..adb7de3f37d 100644
--- a/target/riscv/Kconfig
+++ b/target/riscv/Kconfig
@@ -1,5 +1,7 @@
 config RISCV32
 bool
+select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
 
 config RISCV64
 bool
+select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
diff --git a/target/xtensa/Kconfig b/target/xtensa/Kconfig
index a3c8dc7f6d7..5e46049262d 100644
--- a/target/xtensa/Kconfig
+++ b/target/xtensa/Kconfig
@@ -1,2 +1,3 @@
 config XTENSA
 bool
+select SEMIHOSTING
-- 
2.39.2

[PULL 03/10] configure: run plugin TCG tests again

[Alex Bennée]

From: Paolo Bonzini 

Commit 39fb3cfc28b ("configure: clean up plugin option handling", 2023-10-18)
dropped the CONFIG_PLUGIN line from tests/tcg/config-host.mak, due to confusion
caused by the shadowing of $config_host_mak.  However, TCG tests were still
expecting it.  Oops.

Put it back, in the meanwhile the shadowing is gone so it's clear that it goes
in the tests/tcg configuration.

Cc:  
Fixes: 39fb3cfc28b ("configure: clean up plugin option handling", 2023-10-18)
Signed-off-by: Paolo Bonzini 
Message-Id: <20240124115332.612162-1-pbonz...@redhat.com>
Signed-off-by: Alex Bennée 
Message-Id: <20240207163812.3231697-4-alex.ben...@linaro.org>

diff --git a/configure b/configure
index 3d8e24ae011..ff058d6c486 100755
--- a/configure
+++ b/configure
@@ -1644,6 +1644,9 @@ fi
 mkdir -p tests/tcg
 echo "# Automatically generated by configure - do not modify" > 
tests/tcg/$config_host_mak
 echo "SRC_PATH=$source_path" >> tests/tcg/$config_host_mak
+if test "$plugins" = "yes" ; then
+echo "CONFIG_PLUGIN=y" >> tests/tcg/$config_host_mak
+fi
 
 tcg_tests_targets=
 for target in $target_list; do
-- 
2.39.2

Re: [PATCH] hw/hppa/Kconfig: Fix building with "configure --without-default-devices"

[BALATON Zoltan]

On Fri, 9 Feb 2024, Helge Deller wrote:

On 2/9/24 19:55, Thomas Huth wrote:

When running "configure" with "--without-default-devices", building
of qemu-system-hppa currently fails with:

  /usr/bin/ld: libqemu-hppa-softmmu.fa.p/hw_hppa_machine.c.o: in function 
`machine_HP_common_init_tail':

  hw/hppa/machine.c:399: undefined reference to `usb_bus_find'
  /usr/bin/ld: hw/hppa/machine.c:399: undefined reference to 
`usb_create_simple'

  /usr/bin/ld: hw/hppa/machine.c:400: undefined reference to `usb_bus_find'
  /usr/bin/ld: hw/hppa/machine.c:400: undefined reference to 
`usb_create_simple'

  collect2: error: ld returned 1 exit status
  ninja: build stopped: subcommand failed.
  make: *** [Makefile:162: run-ninja] Error 1

And after fixing this, the qemu-system-hppa binary refuses to run
due to the missing 'pci-ohci' and 'pci-serial' devices. Let's add
the right config switches to fix these problems.

Signed-off-by: Thomas Huth 
---
  hw/hppa/Kconfig | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/hppa/Kconfig b/hw/hppa/Kconfig
index ff8528aaa8..124d5e9e47 100644
--- a/hw/hppa/Kconfig
+++ b/hw/hppa/Kconfig
@@ -6,7 +6,7 @@ config HPPA_B160L
  select ASTRO
  select DINO
  select LASI
-select SERIAL
+select SERIAL_PCI


I think the "SERIAL" is needed too for the B160L machine.


SERIAL_PCI selects SERIAL so I think it should be pulled in without 
listing it separately but not sure what's the policy for these configs.


Regards,
BALATON Zoltan


Other than that,

Acked-by: Helge Deller 

Thank you!
Helge


  select ISA_BUS
  select I8259
  select IDE_CMD646
@@ -16,3 +16,4 @@ config HPPA_B160L
  select LASIPS2
  select PARALLEL
  select ARTIST
+select USB_OHCI_PCI

[PULL 02/10] docs: mark CRIS support as deprecated

[Alex Bennée]

This might be premature but while streamlining the avocado tests I
realised the only tests we have are "check-tcg" ones. The ageing
fedora-cris-cross image works well enough for developers but can't be
used in CI as we need supported build platforms to build QEMU.

Does this mean the writing is on the wall for this architecture?

Cc: Rabin Vincent 
Reviewed-by: Thomas Huth 
Acked-by: Edgar E. Iglesias 
Reviewed-by: Thomas Huth 
Signed-off-by: Alex Bennée 
Message-Id: <20240207163812.3231697-3-alex.ben...@linaro.org>

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index c7b95e6068e..7b0c59919e5 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -192,6 +192,13 @@ in the QEMU object model anymore. ``power5+``, 
``power5+_v2.1``,
 an alias, but for consistency these will get removed in a future
 release, too. Use ``power5p_v2.1`` and ``power7p_v2.1`` instead.
 
+CRIS CPU architecture (since 9.0)
+'
+
+The CRIS architecture was pulled from Linux in 4.17 and the compiler
+is no longer packaged in any distro making it harder to run the
+``check-tcg`` tests. Unless we can improve the testing situation there
+is a chance the code will bitrot without anyone noticing.
 
 System emulator machines
 
-- 
2.39.2

Re: [PATCH] hw/hppa/Kconfig: Fix building with "configure --without-default-devices"

[Helge Deller]

On 2/9/24 19:55, Thomas Huth wrote:

When running "configure" with "--without-default-devices", building
of qemu-system-hppa currently fails with:

  /usr/bin/ld: libqemu-hppa-softmmu.fa.p/hw_hppa_machine.c.o: in function 
`machine_HP_common_init_tail':
  hw/hppa/machine.c:399: undefined reference to `usb_bus_find'
  /usr/bin/ld: hw/hppa/machine.c:399: undefined reference to `usb_create_simple'
  /usr/bin/ld: hw/hppa/machine.c:400: undefined reference to `usb_bus_find'
  /usr/bin/ld: hw/hppa/machine.c:400: undefined reference to `usb_create_simple'
  collect2: error: ld returned 1 exit status
  ninja: build stopped: subcommand failed.
  make: *** [Makefile:162: run-ninja] Error 1

And after fixing this, the qemu-system-hppa binary refuses to run
due to the missing 'pci-ohci' and 'pci-serial' devices. Let's add
the right config switches to fix these problems.

Signed-off-by: Thomas Huth 
---
  hw/hppa/Kconfig | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/hppa/Kconfig b/hw/hppa/Kconfig
index ff8528aaa8..124d5e9e47 100644
--- a/hw/hppa/Kconfig
+++ b/hw/hppa/Kconfig
@@ -6,7 +6,7 @@ config HPPA_B160L
  select ASTRO
  select DINO
  select LASI
-select SERIAL
+select SERIAL_PCI


I think the "SERIAL" is needed too for the B160L machine.
Other than that,

Acked-by: Helge Deller 

Thank you!
Helge


  select ISA_BUS
  select I8259
  select IDE_CMD646
@@ -16,3 +16,4 @@ config HPPA_B160L
  select LASIPS2
  select PARALLEL
  select ARTIST
+select USB_OHCI_PCI

Re: [PATCH v3 9/9] hw/mem/cxl_type3: Add dpa range validation for accesses to dc regions

[fan]

On Wed, Jan 24, 2024 at 04:58:15PM +, Jonathan Cameron wrote:
> On Tue,  7 Nov 2023 10:07:13 -0800
> nifan@gmail.com wrote:
> 
> > From: Fan Ni 
> > 
> > Not all dpa range in the dc regions is valid to access until an extent
> DPA ... DC etc
> 
> > covering the range has been added. Add a bitmap for each region to
> > record whether a dc block in the region has been backed by dc extent.
> > For the bitmap, a bit in the bitmap represents a dc block. When a dc
> > extent is added, all the bits of the blocks in the extent will be set,
> > which will be cleared when the extent is released.
> > 
> > Signed-off-by: Fan Ni 
> 
> Hi Fan, one query inline and a few comments.
> 
> Jonathan
> 
> > 
> > --
> > JC changes:
> > - Rebase on what will be next gitlab.com/jic23/qemu CXL staging tree.
> > - Drop unnecessary handling of failed bitmap allocations. In common with
> >   most QEMU allocations they fail hard anyway.
> > - Use previously factored out cxl_find_region() helper
> > - Minor editorial stuff in comments such as spec version references
> >   according to the standard form I'm trying to push through the code.
> > Picked up Jørgen's fix:
> > https://lore.kernel.org/qemu-devel/d0d7ca1d-81bc-19b3-4904-d60046ded...@wdc.com/T/#u
> > ---
> >  hw/cxl/cxl-mailbox-utils.c  | 31 +--
> >  hw/mem/cxl_type3.c  | 78 +
> >  include/hw/cxl/cxl_device.h | 15 +--
> >  3 files changed, 109 insertions(+), 15 deletions(-)
> > 
> > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> > index 8e6a98753a..6be92fb5ba 100644
> > --- a/hw/cxl/cxl-mailbox-utils.c
> > +++ b/hw/cxl/cxl-mailbox-utils.c
> > @@ -1401,10 +1401,9 @@ CXLDCDRegion *cxl_find_dc_region(CXLType3Dev *ct3d, 
> > uint64_t dpa, uint64_t len)
> >  }
> >  
> >  void cxl_insert_extent_to_extent_list(CXLDCDExtentList *list,
> > - uint64_t dpa,
> > - uint64_t len,
> > - uint8_t *tag,
> > - uint16_t shared_seq)
> > +  uint64_t dpa, uint64_t len,
> > +  uint8_t *tag,
> > +  uint16_t shared_seq)
> 
> avoid noisy whitespace changes like this.
> 
> 
> > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> > index 43cea3d818..4ec65a751a 100644
> > --- a/hw/mem/cxl_type3.c
> > +++ b/hw/mem/cxl_type3.c
> 
> > +/*
> > + * Check whether a DPA range [dpa, dpa + len) has been backed with DC 
> > extents.
> > + * Used when validating read/write to dc regions
> > + */
> > +bool ct3_test_region_block_backed(CXLType3Dev *ct3d, uint64_t dpa,
> > +  uint64_t len)
> > +{
> > +CXLDCDRegion *region;
> > +uint64_t nbits;
> > +long nr;
> > +
> > +region = cxl_find_dc_region(ct3d, dpa, len);
> > +if (!region) {
> > +return false;
> > +}
> > +
> > +nr = (dpa - region->base) / region->block_size;
> > +nbits = DIV_ROUND_UP(len, region->block_size);
> > +return find_next_zero_bit(region->blk_bitmap, nr + nbits, nr) == nr + 
> > nbits;
> I'm not sure how this works... Is it taking a size or an end point?
> 
> Linux equivalent takes size, so I'd expect
> 
> return find_next_zero_bit(region->blk_bitmap, nbits, nr);
> Perhaps a comment would avoid any future confusion on this.
> 

My understanding is that the size is the size of the bitmap, which is
also end of the range to check, not the length of the range to check.

The function find_next_zero_bit(bitmap, size, offset) checks the bitmap range
[offset, size) to find the next unset bit, for the above test, we want to
check range [nr, nr + nbits), so the arguments passed to the function
should be right.

In the definition of the function, whenever offset >= size, it returns size
because size is the end of the range, So if we pass nbits and nr
to the function and nr >= nbits, which can be common, meaning (dpa-region_base)
\> len, the function will always return true; that is not what we want.

To sum up, the second parameter of the function should always be the end
of the range to check, for our case, it is nr + nbits.

Fan



> > +}
> > +
> > +/*
> > + * Mark the DPA range [dpa, dap + len) to be unbacked and inaccessible. 
> > This
> > + * happens when a dc extent is return by the host.
> > + */
> > +void ct3_clear_region_block_backed(CXLType3Dev *ct3d, uint64_t dpa,
> > +   uint64_t len)
> > +{
> > +CXLDCDRegion *region;
> > +uint64_t nbits;
> > +long nr;
> > +
> > +region = cxl_find_dc_region(ct3d, dpa, len);
> > +if (!region) {
> > +return;
> > +}
> > +
> > +nr = (dpa - region->base) / region->block_size;
> > +nbits = len / region->block_size;
> > +bitmap_clear(region->blk_bitmap, nr, nbits);
> > +}
> > +
> 

[PATCH] hw/i386/kvm/ioapic: Replace magic '24' value by proper definition

[Philippe Mathieu-Daudé]

Replace '24' -> KVM_IOAPIC_NUM_PINS.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/kvm/ioapic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index 409d0c8c76..b96fe84eed 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -35,7 +35,7 @@ void kvm_pc_setup_irq_routing(bool pci_enabled)
 kvm_irqchip_add_irq_route(s, i, KVM_IRQCHIP_PIC_SLAVE, i - 8);
 }
 if (pci_enabled) {
-for (i = 0; i < 24; ++i) {
+for (i = 0; i < KVM_IOAPIC_NUM_PINS; ++i) {
 if (i == 0) {
 kvm_irqchip_add_irq_route(s, i, KVM_IRQCHIP_IOAPIC, 2);
 } else if (i != 2) {
-- 
2.41.0

Re: [RFC v2 0/5] ARM Nested Virt Support

[Peter Maydell]

On Fri, 9 Feb 2024 at 16:00, Eric Auger  wrote:
>
> This series adds ARM Nested Virtualization support in KVM mode.
> This is a respin of previous contributions from Miguel [1] and Haibo [2].
>
> This was tested with Marc's v11 [3] on Ampere HW with fedora L1 guest and
> L2 guests booted without EDK2. However it does not work yet with
> EDK2 but it looks unrelated to this qemu integration (host hard lockups).
>
> The host needs to be booted with "kvm-arm.mode=nested" option and
> qemu needs to be invoked with :
>
> -machine virt,virtualization=on
>
> There is a known issue with hosts supporting SVE. Kernel does not support both
> SVE and NV2 and the current qemu integration has an issue with the
> scratch_host_vcpu startup because both are enabled if exposed by the kernel.
> This is independent on whether sve is disabled on the command line. 
> Unfortunately
> I lost access to the HW that expose that issue so I couldn't fix it in this
> version.

You can probably repro that by running the whole setup under
QEMU's FEAT_NV emulation, which will be able to give you a CPU
with both FEAT_NV and SVE.

Personally I think that this is a kernel missing-feature that
should really be fixed as part of getting the kernel patches
upstreamed. There's no cause to force every userspace VMM to
develop extra complications for this.

thanks
-- PMM

Re: [RFC v2 3/5] target/arm/kvm: Add helper to detect EL2 when using KVM

[Philippe Mathieu-Daudé]

On 9/2/24 16:59, Eric Auger wrote:

From: Haibo Xu 

Introduce query support for KVM_CAP_ARM_EL2.

Signed-off-by: Haibo Xu 
Signed-off-by: Miguel Luis 
Signed-off-by: Eric Auger 
Reviewed-by: Richard Henderson 
---
  target/arm/kvm.c |  5 +
  target/arm/kvm_arm.h | 12 
  2 files changed, 17 insertions(+)


Reviewed-by: Philippe Mathieu-Daudé 

[PATCH] hw/hppa/Kconfig: Fix building with "configure --without-default-devices"

[Thomas Huth]

When running "configure" with "--without-default-devices", building
of qemu-system-hppa currently fails with:

 /usr/bin/ld: libqemu-hppa-softmmu.fa.p/hw_hppa_machine.c.o: in function 
`machine_HP_common_init_tail':
 hw/hppa/machine.c:399: undefined reference to `usb_bus_find'
 /usr/bin/ld: hw/hppa/machine.c:399: undefined reference to `usb_create_simple'
 /usr/bin/ld: hw/hppa/machine.c:400: undefined reference to `usb_bus_find'
 /usr/bin/ld: hw/hppa/machine.c:400: undefined reference to `usb_create_simple'
 collect2: error: ld returned 1 exit status
 ninja: build stopped: subcommand failed.
 make: *** [Makefile:162: run-ninja] Error 1

And after fixing this, the qemu-system-hppa binary refuses to run
due to the missing 'pci-ohci' and 'pci-serial' devices. Let's add
the right config switches to fix these problems.

Signed-off-by: Thomas Huth 
---
 hw/hppa/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/hppa/Kconfig b/hw/hppa/Kconfig
index ff8528aaa8..124d5e9e47 100644
--- a/hw/hppa/Kconfig
+++ b/hw/hppa/Kconfig
@@ -6,7 +6,7 @@ config HPPA_B160L
 select ASTRO
 select DINO
 select LASI
-select SERIAL
+select SERIAL_PCI
 select ISA_BUS
 select I8259
 select IDE_CMD646
@@ -16,3 +16,4 @@ config HPPA_B160L
 select LASIPS2
 select PARALLEL
 select ARTIST
+select USB_OHCI_PCI
-- 
2.43.0

Re: [PATCH] kvm: emit GUEST_PANICKED event in case of abnormal KVM exit

[Alex Bennée]

Andrey Drobyshev  writes:

(Add kvm@vger to CC for wider review)

> Currently we emit GUEST_PANICKED event in case kvm_vcpu_ioctl() returns
> KVM_EXIT_SYSTEM_EVENT with the event type KVM_SYSTEM_EVENT_CRASH.  Let's
> extend this scenario and emit GUEST_PANICKED in case of an abnormal KVM
> exit.  That's a natural thing to do since in this case guest is no
> longer operational anyway.
>
> Signed-off-by: Andrey Drobyshev 
> Acked-by: Denis V. Lunev 
> ---
>  accel/kvm/kvm-all.c | 19 +++
>  1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index e39a810a4e..d74b3f0b0e 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -2816,6 +2816,14 @@ static void kvm_eat_signals(CPUState *cpu)
>  } while (sigismember(, SIG_IPI));
>  }
>  
> +static void kvm_emit_guest_crash(CPUState *cpu)
> +{
> +kvm_cpu_synchronize_state(cpu);
> +qemu_mutex_lock_iothread();
> +qemu_system_guest_panicked(cpu_get_crash_info(cpu));
> +qemu_mutex_unlock_iothread();
> +}
> +
>  int kvm_cpu_exec(CPUState *cpu)
>  {
>  struct kvm_run *run = cpu->kvm_run;
> @@ -2969,21 +2977,24 @@ int kvm_cpu_exec(CPUState *cpu)
>  ret = EXCP_INTERRUPT;
>  break;
>  case KVM_SYSTEM_EVENT_CRASH:
> -kvm_cpu_synchronize_state(cpu);
> -qemu_mutex_lock_iothread();
> -qemu_system_guest_panicked(cpu_get_crash_info(cpu));
> -qemu_mutex_unlock_iothread();
> +kvm_emit_guest_crash(cpu);
>  ret = 0;
>  break;
>  default:
>  DPRINTF("kvm_arch_handle_exit\n");
>  ret = kvm_arch_handle_exit(cpu, run);
> +if (ret < 0) {
> +kvm_emit_guest_crash(cpu);
> +}
>  break;
>  }
>  break;
>  default:
>  DPRINTF("kvm_arch_handle_exit\n");
>  ret = kvm_arch_handle_exit(cpu, run);
> +if (ret < 0) {
> +kvm_emit_guest_crash(cpu);
> +}
>  break;
>  }
>  } while (ret == 0);

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v6 0/5] Support message-based DMA in vfio-user server

[Jonathan Cameron via]

On Wed,  1 Nov 2023 06:16:06 -0700
Mattias Nissler  wrote:

> This series adds basic support for message-based DMA in qemu's vfio-user
> server. This is useful for cases where the client does not provide file
> descriptors for accessing system memory via memory mappings. My motivating use
> case is to hook up device models as PCIe endpoints to a hardware design. This
> works by bridging the PCIe transaction layer to vfio-user, and the endpoint
> does not access memory directly, but sends memory requests TLPs to the 
> hardware
> design in order to perform DMA.
> 
> Note that more work is needed to make message-based DMA work well: qemu
> currently breaks down DMA accesses into chunks of size 8 bytes at maximum, 
> each
> of which will be handled in a separate vfio-user DMA request message. This is
> quite terrible for large DMA accesses, such as when nvme reads and writes
> page-sized blocks for example. Thus, I would like to improve qemu to be able 
> to
> perform larger accesses, at least for indirect memory regions. I have 
> something
> working locally, but since this will likely result in more involved surgery 
> and
> discussion, I am leaving this to be addressed in a separate patch.
> 
Hi Mattias,

I was wondering what the status of this patch set is - seems no outstanding 
issues
have been raised?

I'd run into a similar problem with multiple DMA mappings using the bounce 
buffer
when using the emulated CXL memory with virtio-blk-pci accessing it.

In that particular case virtio-blk is using the "memory" address space, but
otherwise your first 2 patches work for me as well so I'd definitely like
to see those get merged!

Thanks,

Jonathan

> Changes from v1:
> 
> * Address Stefan's review comments. In particular, enforce an allocation limit
>   and don't drop the map client callbacks given that map requests can fail 
> when
>   hitting size limits.
> 
> * libvfio-user version bump now included in the series.
> 
> * Tested as well on big-endian s390x. This uncovered another byte order issue
>   in vfio-user server code that I've included a fix for.
> 
> Changes from v2:
> 
> * Add a preparatory patch to make bounce buffering an AddressSpace-specific
>   concept.
> 
> * The total buffer size limit parameter is now per AdressSpace and can be
>   configured for PCIDevice via a property.
> 
> * Store a magic value in first bytes of bounce buffer struct as a best effort
>   measure to detect invalid pointers in address_space_unmap.
> 
> Changes from v3:
> 
> * libvfio-user now supports twin-socket mode which uses separate sockets for
>   client->server and server->client commands, respectively. This addresses the
>   concurrent command bug triggered by server->client DMA access commands. See
>   https://github.com/nutanix/libvfio-user/issues/279 for details.
> 
> * Add missing teardown code in do_address_space_destroy.
> 
> * Fix bounce buffer size bookkeeping race condition.
> 
> * Generate unmap notification callbacks unconditionally.
> 
> * Some cosmetic fixes.
> 
> Changes from v4:
> 
> * Fix accidentally dropped memory_region_unref, control flow restored to match
>   previous code to simplify review.
> 
> * Some cosmetic fixes.
> 
> Changes from v5:
> 
> * Unregister indirect memory region in libvfio-user dma_unregister callback.
> 
> I believe all patches in the series have been reviewed appropriately, so my
> hope is that this will be the final iteration. Stefan, Peter, Jag, thanks for
> your feedback, let me know if there's anything else needed from my side before
> this can get merged.
> 
> Mattias Nissler (5):
>   softmmu: Per-AddressSpace bounce buffering
>   softmmu: Support concurrent bounce buffers
>   Update subprojects/libvfio-user
>   vfio-user: Message-based DMA support
>   vfio-user: Fix config space access byte order
> 
>  hw/pci/pci.c  |   8 ++
>  hw/remote/trace-events|   2 +
>  hw/remote/vfio-user-obj.c | 104 +
>  include/exec/cpu-common.h |   2 -
>  include/exec/memory.h |  41 +-
>  include/hw/pci/pci_device.h   |   3 +
>  subprojects/libvfio-user.wrap |   2 +-
>  system/dma-helpers.c  |   4 +-
>  system/memory.c   |   8 ++
>  system/physmem.c  | 141 ++
>  10 files changed, 226 insertions(+), 89 deletions(-)
> 

[PATCH] iotests: Make 144 deterministic again

[Kevin Wolf]

Since commit effd60c8 changed how QMP commands are processed, the order
of the block-commit return value and job events in iotests 144 wasn't
fixed and more and caused the test to fail intermittently.

Change the test to cache events first and then print them in a
predefined order.

Waiting three times for JOB_STATUS_CHANGE is a bit uglier than just
waiting for the JOB_STATUS_CHANGE that has "status": "ready", but the
tooling we have doesn't seem to allow the latter easily.

Fixes: effd60c878176bcaf97fa7ce2b12d04bb8ead6f7
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2126
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/144 | 12 +++-
 tests/qemu-iotests/144.out |  2 +-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/144 b/tests/qemu-iotests/144
index bdcc498fa2..d284a0e442 100755
--- a/tests/qemu-iotests/144
+++ b/tests/qemu-iotests/144
@@ -83,12 +83,22 @@ echo
 echo === Performing block-commit on active layer ===
 echo
 
+capture_events="BLOCK_JOB_READY JOB_STATUS_CHANGE"
+
 # Block commit on active layer, push the new overlay into base
 _send_qemu_cmd $h "{ 'execute': 'block-commit',
 'arguments': {
  'device': 'virtio0'
   }
-}" "READY"
+}" "return"
+
+_wait_event $h "JOB_STATUS_CHANGE"
+_wait_event $h "JOB_STATUS_CHANGE"
+_wait_event $h "JOB_STATUS_CHANGE"
+
+_wait_event $h "BLOCK_JOB_READY"
+
+capture_events=
 
 _send_qemu_cmd $h "{ 'execute': 'block-job-complete',
 'arguments': {
diff --git a/tests/qemu-iotests/144.out b/tests/qemu-iotests/144.out
index b3b4812015..2245ddfa10 100644
--- a/tests/qemu-iotests/144.out
+++ b/tests/qemu-iotests/144.out
@@ -25,9 +25,9 @@ Formatting 'TEST_DIR/tmp.qcow2', fmt=qcow2 cluster_size=65536 
extended_l2=off co
  'device': 'virtio0'
   }
 }
+{"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"JOB_STATUS_CHANGE", "data": {"status": "created", "id": "virtio0"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"JOB_STATUS_CHANGE", "data": {"status": "running", "id": "virtio0"}}
-{"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"JOB_STATUS_CHANGE", "data": {"status": "ready", "id": "virtio0"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_READY", "data": {"device": "virtio0", "len": 0, "offset": 0, 
"speed": 0, "type": "commit"}}
 { 'execute': 'block-job-complete',
-- 
2.43.0

Re: [PATCH v2 3/3] virtio-blk: Use ioeventfd_attach in start_ioeventfd

[Hanna Czenczek]

On 09.02.24 15:38, Michael Tokarev wrote:

02.02.2024 18:31, Hanna Czenczek :

Commit d3f6f294aeadd5f88caf0155e4360808c95b3146 ("virtio-blk: always set
ioeventfd during startup") has made virtio_blk_start_ioeventfd() always
kick the virtqueue (set the ioeventfd), regardless of whether the BB is
drained.  That is no longer necessary, because attaching the host
notifier will now set the ioeventfd, too; this happens either
immediately right here in virtio_blk_start_ioeventfd(), or later when
the drain ends, in virtio_blk_ioeventfd_attach().

With event_notifier_set() removed, the code becomes the same as the one
in virtio_blk_ioeventfd_attach(), so we can reuse that function.


The mentioned comit is v8.2.0-812-gd3f6f294ae, - ie, past 8.2.
Is this new change still relevant for stable?


Sorry again. :/  This patch is a clean-up patch that won’t apply to 
8.2.  Now, 8.2 does have basically the same logic as described in the 
patch message (d3f6f294aea restored it after it was broken), so a 
similar patch could be made for it (removing the event_notifier_set() 
from virtio_blk_data_plane_start()), but whether we kick the virtqueues 
once or twice on start-up probably won’t make a difference, certainly 
not in terms of correctness.


Hanna

Re: [PATCH 0/2] block: Allow concurrent BB context changes

[Hanna Czenczek]

On 09.02.24 15:08, Michael Tokarev wrote:

02.02.2024 17:47, Hanna Czenczek :

Hi,

Without the AioContext lock, a BB's context may kind of change at any
time (unless it has a root node, and I/O requests are pending). That
also means that its own context (BlockBackend.ctx) and that of its root
node can differ sometimes (while the context is being changed).


How relevant this is for -stable (8.2 at least) which does not have
"scsi: eliminate AioContext lock" patchset, and in particular,:
v8.2.0-124-geaad0fe260 "scsi: only access SCSIDevice->requests from
one thread"?

The issue first patch "block-backend: Allow concurrent context changes"
fixes (RHEL-19381) seems to be for 8.1.something, so it exists in 8.2
too, and this particular fix applies to 8.2.

But with other changes around all this, I'm a bit lost as of what should
be done on stable.  Not even thinking about 7.2 here :)


Ah, sorry, yes.  Since we do still have the AioContext lock, this series 
won’t be necessary in -stable.  Sorry for the noise!


Hanna

Re: [PATCH 3/3] system/physmem: Assign global system I/O Memory to machine

[Philippe Mathieu-Daudé]

(+Markus I forgot to Cc)

On 9/2/24 17:06, Peter Maydell wrote:

On Fri, 9 Feb 2024 at 15:01, Philippe Mathieu-Daudé  wrote:


So far there is only one system I/O and one system
memory per machine.

Signed-off-by: Philippe Mathieu-Daudé 
---
  system/physmem.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/system/physmem.c b/system/physmem.c
index 5e66d9ae36..50947a374e 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -2554,12 +2554,13 @@ static void memory_map_init(void)
  {
  system_memory = g_malloc(sizeof(*system_memory));

-memory_region_init(system_memory, NULL, "system", UINT64_MAX);
+memory_region_init(system_memory, OBJECT(current_machine),
+   "system", UINT64_MAX);
  address_space_init(_space_memory, system_memory, "memory");

  system_io = g_malloc(sizeof(*system_io));
-memory_region_init_io(system_io, NULL, _io_ops, NULL, "io",
-  65536);
+memory_region_init_io(system_io, OBJECT(current_machine),
+  _io_ops, NULL, "io", 65535);
  address_space_init(_space_io, system_io, "I/O");
  }


What's the intention in doing this? What does it change?


We want to remove access to pre-QOM and possibly hotplug QOM paths
from external API (CLI & QMP so far).

When the parent object is obvious and missing we simply have to
explicit it.


It seems to be OK to pass a non-Device owner in for
memory_region_init() (whereas it is *not* OK to do that
for memory_region_init_ram()), but this seems to be
getting a bit tricky.


Yes, memory_region_init_ram() is problematic; I'm hardly trying
to ignore it at this point.

Re: [PATCH 1/3] cpu-target: Include missing 'exec/memory.h' header

[Philippe Mathieu-Daudé]

On 9/2/24 17:01, Peter Maydell wrote:

On Fri, 9 Feb 2024 at 15:01, Philippe Mathieu-Daudé  wrote:


Include "exec/memory.h" in order to avoid:

   cpu-target.c:201:50: error: use of undeclared identifier 'TYPE_MEMORY_REGION'
   DEFINE_PROP_LINK("memory", CPUState, memory, TYPE_MEMORY_REGION,
^


Given that we don't actually see this error, presumably
we're implicitly dragging it in via some other include?


It is pulled in by the exec/cpu-all.h header which I'm trying to
sanitize (along with others). I'll add a note about this.


Anyway, better to be explicit than implicit, so

Reviewed-by: Peter Maydell 


Thanks!

Re: [PULL 00/34] Migration staging patches

[Peter Maydell]

On Thu, 8 Feb 2024 at 03:05,  wrote:
>
> From: Peter Xu 
>
> The following changes since commit 39a6e4f87e7b75a45b08d6dc8b8b7c2954c87440:
>
>   Merge tag 'pull-qapi-2024-02-03' of https://repo.or.cz/qemu/armbru into 
> staging (2024-02-03 13:31:58 +)
>
> are available in the Git repository at:
>
>   https://gitlab.com/peterx/qemu.git tags/migration-staging-pull-request
>
> for you to fetch changes up to 940bf8ff1ca82aa458c553d9aa9dd7671ed15a4d:
>
>   ci: Update comment for migration-compat-aarch64 (2024-02-07 10:51:27 +0800)
>
> 
> Migration pull
>
> - William's fix on hwpoison migration which used to crash QEMU
> - Peter's multifd cleanup + bugfix + optimizations
> - Avihai's fix on multifd crash over non-socket channels
> - Fabiano's multifd thread-race fix
> - Peter's CI fix series
>
> 


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/9.0
for any user-visible changes.

-- PMM

Re: [PATCH 3/3] system/physmem: Assign global system I/O Memory to machine

[Peter Maydell]

On Fri, 9 Feb 2024 at 15:01, Philippe Mathieu-Daudé  wrote:
>
> So far there is only one system I/O and one system
> memory per machine.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  system/physmem.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/system/physmem.c b/system/physmem.c
> index 5e66d9ae36..50947a374e 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -2554,12 +2554,13 @@ static void memory_map_init(void)
>  {
>  system_memory = g_malloc(sizeof(*system_memory));
>
> -memory_region_init(system_memory, NULL, "system", UINT64_MAX);
> +memory_region_init(system_memory, OBJECT(current_machine),
> +   "system", UINT64_MAX);
>  address_space_init(_space_memory, system_memory, "memory");
>
>  system_io = g_malloc(sizeof(*system_io));
> -memory_region_init_io(system_io, NULL, _io_ops, NULL, "io",
> -  65536);
> +memory_region_init_io(system_io, OBJECT(current_machine),
> +  _io_ops, NULL, "io", 65535);
>  address_space_init(_space_io, system_io, "I/O");
>  }

What's the intention in doing this? What does it change?

It seems to be OK to pass a non-Device owner in for
memory_region_init() (whereas it is *not* OK to do that
for memory_region_init_ram()), but this seems to be
getting a bit tricky.

thanks
-- PMM

[RFC v2 0/5] ARM Nested Virt Support

[Eric Auger]

This series adds ARM Nested Virtualization support in KVM mode.
This is a respin of previous contributions from Miguel [1] and Haibo [2].

This was tested with Marc's v11 [3] on Ampere HW with fedora L1 guest and
L2 guests booted without EDK2. However it does not work yet with
EDK2 but it looks unrelated to this qemu integration (host hard lockups).

The host needs to be booted with "kvm-arm.mode=nested" option and
qemu needs to be invoked with :

-machine virt,virtualization=on

There is a known issue with hosts supporting SVE. Kernel does not support both
SVE and NV2 and the current qemu integration has an issue with the
scratch_host_vcpu startup because both are enabled if exposed by the kernel.
This is independent on whether sve is disabled on the command line. 
Unfortunately
I lost access to the HW that expose that issue so I couldn't fix it in this
version.

This series can be found at:
https://github.com/eauger/qemu/tree/v8.2-nv-rfcv2

Previous version from Miguel:
[1] https://lore.kernel.org/all/20230227163718.62003-1-miguel.l...@oracle.com/
Previous version from Haibo:
[2] https://lore.kernel.org/qemu-devel/cover.1617281290.git.haibo...@linaro.org/
[3] Marc's kernel v11 series:
[PATCH v11 00/43] KVM: arm64: Nested Virtualization support (FEAT_NV2 only)

https://lore.kernel.org/linux-arm-kernel/20231120131027.854038-1-...@kernel.org/T/
available at: https://github.com/eauger/linux/tree/nv-6.8-nv2-v11

Haibo Xu (5):
  [Placeholder] headers: Partial headers update for NV2 enablement
  hw/arm: Allow setting KVM vGIC maintenance IRQ
  target/arm/kvm: Add helper to detect EL2 when using KVM
  target/arm: Enable feature ARM_FEATURE_EL2 if EL2 is supported
  hw/arm/virt: Allow virt extensions with KVM

 hw/arm/virt.c  |  6 +-
 hw/intc/arm_gicv3_common.c |  1 +
 hw/intc/arm_gicv3_kvm.c| 21 +
 include/hw/intc/arm_gicv3_common.h |  1 +
 linux-headers/asm-arm64/kvm.h  |  1 +
 linux-headers/linux/kvm.h  |  1 +
 target/arm/kvm.c   | 21 +
 target/arm/kvm_arm.h   | 12 
 8 files changed, 63 insertions(+), 1 deletion(-)

-- 
2.41.0

[RFC v2 4/5] target/arm: Enable feature ARM_FEATURE_EL2 if EL2 is supported

[Eric Auger]

From: Haibo Xu 

KVM_CAP_ARM_EL2 must be supported by the cpu to enable ARM_FEATURE_EL2.
In case the host does support NV, expose the feature.

Signed-off-by: Haibo Xu 
Signed-off-by: Miguel Luis 
Signed-off-by: Eric Auger 

---

v1 -> v2:
- remove isar_feature_aa64_aa32_el2 modif in target/arm/cpu.h
  [Richard] and use el2_supported in kvm_arch_init_vcpu
---
 target/arm/kvm.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 0996866afe..a08bc68a3f 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -238,6 +238,7 @@ static bool 
kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
  */
 int fdarray[3];
 bool sve_supported;
+bool el2_supported;
 bool pmu_supported = false;
 uint64_t features = 0;
 int err;
@@ -268,6 +269,14 @@ static bool 
kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
 init.features[0] |= 1 << KVM_ARM_VCPU_SVE;
 }
 
+/*
+ * Ask for EL2 if supported.
+ */
+el2_supported = kvm_arm_el2_supported();
+if (el2_supported) {
+init.features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
+}
+
 /*
  * Ask for Pointer Authentication if supported, so that we get
  * the unsanitized field values for AA64ISAR1_EL1.
@@ -449,6 +458,10 @@ static bool 
kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
 features |= 1ULL << ARM_FEATURE_PMU;
 features |= 1ULL << ARM_FEATURE_GENERIC_TIMER;
 
+if (el2_supported) {
+features |= 1ULL << ARM_FEATURE_EL2;
+}
+
 ahcf->features = features;
 
 return true;
@@ -1912,6 +1925,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
 cpu->kvm_init_features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS |
   1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);
 }
+if (kvm_arm_el2_supported()) {
+cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
+}
 
 /* Do KVM_ARM_VCPU_INIT ioctl */
 ret = kvm_arm_vcpu_init(cpu);
-- 
2.41.0

Re: [PATCH 2/3] monitor/target: Include missing 'exec/memory.h' header

[Peter Maydell]

On Fri, 9 Feb 2024 at 15:02, Philippe Mathieu-Daudé  wrote:
>
> Include "exec/memory.h" in order to avoid:
>
>   monitor/hmp-cmds-target.c:263:10: error: call to undeclared function 
> 'memory_region_is_ram';
>   ISO C99 and later do not support implicit function declarations 
> [-Wimplicit-function-declaration]
>   if (!memory_region_is_ram(mrs.mr) && !memory_region_is_romd(mrs.mr)) {
>^
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  monitor/hmp-cmds-target.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/monitor/hmp-cmds-target.c b/monitor/hmp-cmds-target.c
> index d9fbcac08d..9338ae8440 100644
> --- a/monitor/hmp-cmds-target.c
> +++ b/monitor/hmp-cmds-target.c
> @@ -25,6 +25,7 @@
>  #include "qemu/osdep.h"
>  #include "disas/disas.h"
>  #include "exec/address-spaces.h"
> +#include "exec/memory.h"
>  #include "monitor/hmp-target.h"
>  #include "monitor/monitor-internal.h"
>  #include "qapi/error.h"
> --
> 2.41.0

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH 1/3] cpu-target: Include missing 'exec/memory.h' header

[Peter Maydell]

On Fri, 9 Feb 2024 at 15:01, Philippe Mathieu-Daudé  wrote:
>
> Include "exec/memory.h" in order to avoid:
>
>   cpu-target.c:201:50: error: use of undeclared identifier 
> 'TYPE_MEMORY_REGION'
>   DEFINE_PROP_LINK("memory", CPUState, memory, TYPE_MEMORY_REGION,
>^

Given that we don't actually see this error, presumably
we're implicitly dragging it in via some other include?
Anyway, better to be explicit than implicit, so

Reviewed-by: Peter Maydell 

thanks
-- PMM

[RFC v2 3/5] target/arm/kvm: Add helper to detect EL2 when using KVM

[Eric Auger]

From: Haibo Xu 

Introduce query support for KVM_CAP_ARM_EL2.

Signed-off-by: Haibo Xu 
Signed-off-by: Miguel Luis 
Signed-off-by: Eric Auger 
Reviewed-by: Richard Henderson 
---
 target/arm/kvm.c |  5 +
 target/arm/kvm_arm.h | 12 
 2 files changed, 17 insertions(+)

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 81813030a5..0996866afe 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -1791,6 +1791,11 @@ bool kvm_arm_aarch32_supported(void)
 return kvm_check_extension(kvm_state, KVM_CAP_ARM_EL1_32BIT);
 }
 
+bool kvm_arm_el2_supported(void)
+{
+return kvm_check_extension(kvm_state, KVM_CAP_ARM_EL2);
+}
+
 bool kvm_arm_sve_supported(void)
 {
 return kvm_check_extension(kvm_state, KVM_CAP_ARM_SVE);
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index cfaa0d9bc7..36e4b37ec0 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -188,6 +188,13 @@ bool kvm_arm_pmu_supported(void);
  */
 bool kvm_arm_sve_supported(void);
 
+/**
+ * kvm_arm_el2_supported:
+ *
+ * Returns true if KVM can enable EL2 and false otherwise.
+ */
+bool kvm_arm_el2_supported(void);
+
 /**
  * kvm_arm_get_max_vm_ipa_size:
  * @ms: Machine state handle
@@ -235,6 +242,11 @@ static inline bool kvm_arm_sve_supported(void)
 return false;
 }
 
+static inline bool kvm_arm_el2_supported(void)
+{
+return false;
+}
+
 /*
  * These functions should never actually be called without KVM support.
  */
-- 
2.41.0

[RFC v2 5/5] hw/arm/virt: Allow virt extensions with KVM

[Eric Auger]

From: Haibo Xu 

Up to now virt support on guest has been only supported with TCG.
Now it becomes feasible to use it with KVM acceleration.

Signed-off-by: Haibo Xu 
Signed-off-by: Miguel Luis 
Signed-off-by: Eric Auger 

---

v1 -> v2:
- fixed test ordering: virt && ((kvm && !kvm_el2) || hvf) [Richard]
- tweeked the commit title & message
---
 hw/arm/virt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 5214aca898..ae7ac07301 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2109,7 +2109,8 @@ static void machvirt_init(MachineState *machine)
 exit(1);
 }
 
-if (vms->virt && (kvm_enabled() || hvf_enabled())) {
+if (vms->virt &&
+((kvm_enabled() && !kvm_arm_el2_supported()) || hvf_enabled())) {
 error_report("mach-virt: %s does not support providing "
  "Virtualization extensions to the guest CPU",
  current_accel_name());
-- 
2.41.0

[RFC v2 1/5] [Placeholder] headers: Partial headers update for NV2 enablement

[Eric Auger]

From: Haibo Xu 

For now let's only import the pieces needed to run NV on KVM.
Later on this will be replaced by a standard and comprehensive
linux header update using scripts/update-linux-headers.sh.

For now the changes are taken from
https://github.com/eauger/linux/tree/nv-6.8-nv2-v11

Signed-off-by: Eric Auger 
Signed-off-by: Haibo Xu 
Signed-off-by: Miguel Luis 
---
 linux-headers/asm-arm64/kvm.h | 1 +
 linux-headers/linux/kvm.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/linux-headers/asm-arm64/kvm.h b/linux-headers/asm-arm64/kvm.h
index c59ea55cd8..d46839f1d9 100644
--- a/linux-headers/asm-arm64/kvm.h
+++ b/linux-headers/asm-arm64/kvm.h
@@ -394,6 +394,7 @@ enum {
 #define KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS 6
 #define KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO  7
 #define KVM_DEV_ARM_VGIC_GRP_ITS_REGS 8
+#define KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ  9
 #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT 10
 #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK \
(0x3fULL << KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT)
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 549fea3a97..5af601d7b8 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -1197,6 +1197,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE 228
 #define KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES 229
 #define KVM_CAP_ARM_SUPPORTED_REG_MASK_RANGES 230
+#define KVM_CAP_ARM_EL2 231
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.41.0

[RFC v2 2/5] hw/arm: Allow setting KVM vGIC maintenance IRQ

[Eric Auger]

From: Haibo Xu 

Allow virt arm machine to set the intid for the KVM GIC maintenance
interrupt.

Signed-off-by: Haibo Xu 
Signed-off-by: Miguel Luis 
Signed-off-by: Eric Auger 

---
v1 -> v2:
- [Miguel] replaced the has_virt_extensions by the maintenance irq
  intid property. [Eric] restored kvm_device_check_attr and
  kvm_device_access standard usage and conditionally call those
  if the prop is set.
---
 hw/arm/virt.c  |  3 +++
 hw/intc/arm_gicv3_common.c |  1 +
 hw/intc/arm_gicv3_kvm.c| 21 +
 include/hw/intc/arm_gicv3_common.h |  1 +
 4 files changed, 26 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 368c2a415a..5214aca898 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -750,6 +750,9 @@ static void create_gic(VirtMachineState *vms, MemoryRegion 
*mem)
  OBJECT(mem), _fatal);
 qdev_prop_set_bit(vms->gic, "has-lpi", true);
 }
+} else {
+   qdev_prop_set_uint32(vms->gic, "maintenance-interrupt-id",
+ARCH_GIC_MAINT_IRQ);
 }
 } else {
 if (!kvm_irqchip_in_kernel()) {
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index cb55c72681..df056dc35c 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -564,6 +564,7 @@ static Property arm_gicv3_common_properties[] = {
 DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
 DEFINE_PROP_BOOL("has-lpi", GICv3State, lpi_enable, 0),
 DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn, 0),
+DEFINE_PROP_UINT32("maintenance-interrupt-id", GICv3State, maint_irq, 0),
 /*
  * Compatibility property: force 8 bits of physical priority, even
  * if the CPU being emulated should have fewer.
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 77eb37e131..23fad60515 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -22,6 +22,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "hw/intc/arm_gicv3_common.h"
+#include "hw/arm/virt.h"
 #include "qemu/error-report.h"
 #include "qemu/module.h"
 #include "sysemu/kvm.h"
@@ -820,6 +821,26 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
+if (s->maint_irq) {
+int ret;
+
+ret = kvm_device_check_attr(s->dev_fd,
+KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ, 0);
+if (!ret) {
+error_setg_errno(errp, errno,
+ "VGICv3 setting maintenance IRQ is not "
+ "supported by this host kernel");
+return;
+}
+
+ret = kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ, 0,
+>maint_irq, true, errp);
+if (ret) {
+error_setg_errno(errp, errno, "Failed to set VGIC maintenance 
IRQ");
+return;
+   }
+}
+
 multiple_redist_region_allowed =
 kvm_device_check_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
   KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION);
diff --git a/include/hw/intc/arm_gicv3_common.h 
b/include/hw/intc/arm_gicv3_common.h
index 4e2fb518e7..4ff421a165 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -246,6 +246,7 @@ struct GICv3State {
 uint32_t num_cpu;
 uint32_t num_irq;
 uint32_t revision;
+uint32_t maint_irq;
 bool lpi_enable;
 bool security_extn;
 bool force_8bit_prio;
-- 
2.41.0

Re: [RFC PATCH 0/7] hw/qdev: Split 'wiring' phase from 'realize'

[Bernhard Beschow]

Am 9. Februar 2024 12:32:18 UTC schrieb "Philippe Mathieu-Daudé" 
:
>Hi,
>
>Various issues related to implementing dynamic machines have
>been documented in [1].
>
>We are trying to understand what means "a qdev is realized".
>One explanation was "the device is guest visible"; however
>many devices are realized before being mapped, thus are not
>"guest visible". Some devices map / wire their IRQs before
>being realized (such ISA devices). There is a need for devices
>to be "automatically" mapped/wired (see [2]) such CLI-created
>devices, but this apply generically to dynamic machines.
>
>Currently the device creation steps are expected to roughly be:
>
>  (external use)(QDev core)   (Device Impl)
>     ~ ~~~
>
>   INIT enter
>   ->
> +--+
> |Allocate state|
> +--+
> ->
>+-+
>| INIT children   |
>| |
>| Alias children 
> properties
>| |
>| Expose properties   |
>INIT exit   +-+
>   <---
> ++
> | set properties |
> ||
> | set ClkIn  |
> ++  REALIZE enter
>   -->
>   +--+
>   | Use config properties|
>   |  |
>   | Realize children |
>   |  |
>   | Init GPIOs/IRQs  |
>   |  |
>   | Init MemoryRegions   |
>   +--+
>   REALIZE exit
>   <---
>   "realized" / "guest visible"
>+-+
>| Explicit wiring:|
>|   IRQs  |
>|   I/O / Mem |
>|   ClkOut|
>+-+ RESET enter
>->
>   +--+
>   | Reset default values |
>   +--+
>
>But as mentioned, various devices "wire" parts before they exit
>the "realize" step.
>In order to clarify, I'm trying to enforce what can be done
>*before* and *after* realization.
>
>*after* a device is expected to be stable (no more configurable)
>and fully usable.
>
>To be able to use internal/auto wiring (such ISA devices) and
>keep the current external/explicit wiring, I propose to add an
>extra "internal wiring" step, happening after the REALIZE step
>as:
>
>  (external use)(QDev core)   (Device Impl)
>     ~ ~~~
>
>   INIT enter
>   ->
> +--+
> |Allocate state|
> +--+
> ->
>+-+
>| INIT children   |
>| |
>| Alias children 
> properties
>| |
>| Expose properties   |
>INIT exit   +-+
>   <---
> ++
> | set properties |
> ||
> | set ClkIn  |
> ++  REALIZE enter
>   -->
>  

Re: [PATCH v2 0/1] target: New binary to prototype heterogeneous machines

[Philippe Mathieu-Daudé]

On 9/2/24 16:29, Philippe Mathieu-Daudé wrote:


This binary will be use to rework QEMU startup code,
paving the way toward dynamic machines. It might also
allow experimenting with multiple TCG target frontends
and possibly prototyping concurrent HW/SW accelerations.



v1: https://lore.kernel.org/qemu-devel/20220215002658.60678-1-f4...@amsat.org/


Supersedes: <20220215002658.60678-1-f4...@amsat.org>

[PATCH v2 1/1] target: Add system emulation aiming to target any architecture

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé 

Add the 'any'-architecture target.

- Only consider 64-bit targets
- Do not use any hardware accelerator (except qtest)
- For architecture constants, use:
  . max of supported targets phys/virt address space
  . max of supported targets MMU modes
  . min of supported targets variable page bits

Build as:

  $ ../configure --target-list=any-softmmu \
 --disable-hvf \
 --disable-kvm \
 --disable-nvmm \
 --disable-tcg \
 --disable-whpx \
 --disable-xen

Test as:

  $ qemu-system-any -M none,accel=qtest -monitor stdio
  QEMU 6.2.50 monitor - type 'help' for more information
  (qemu) info mtree
  address-space: I/O
- (prio 0, i/o): io

  address-space: memory
- (prio 0, i/o): system

  (qemu) info qom-tree
  /machine (none-machine)
/peripheral (container)
/peripheral-anon (container)
/unattached (container)
  /io[0] (memory-region)
  /sysbus (System)
  /system[0] (memory-region)
  (qemu) info qtree
  bus: main-system-bus
type System
  (qemu) info cpus
  (qemu)

Signed-off-by: Philippe Mathieu-Daudé 
---
 configs/devices/any-softmmu/default.mak |  9 +
 configs/targets/any-softmmu.mak |  3 +++
 meson.build |  6 --
 qapi/machine.json   |  2 +-
 include/sysemu/arch_init.h  |  1 +
 target/any/cpu-param.h  | 13 +
 target/any/cpu-qom.h| 12 
 target/any/cpu.h| 24 
 .gitlab-ci.d/buildtest.yml  | 20 
 hw/any/meson.build  |  5 +
 hw/meson.build  |  1 +
 target/Kconfig  |  1 +
 target/any/Kconfig  |  4 
 target/any/meson.build  |  7 +++
 target/meson.build  |  1 +
 15 files changed, 106 insertions(+), 3 deletions(-)
 create mode 100644 configs/devices/any-softmmu/default.mak
 create mode 100644 configs/targets/any-softmmu.mak
 create mode 100644 target/any/cpu-param.h
 create mode 100644 target/any/cpu-qom.h
 create mode 100644 target/any/cpu.h
 create mode 100644 hw/any/meson.build
 create mode 100644 target/any/Kconfig
 create mode 100644 target/any/meson.build

diff --git a/configs/devices/any-softmmu/default.mak 
b/configs/devices/any-softmmu/default.mak
new file mode 100644
index 00..dab0ce770e
--- /dev/null
+++ b/configs/devices/any-softmmu/default.mak
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+CONFIG_ISA_BUS=y
+CONFIG_PCI=y
+CONFIG_PCI_DEVICES=y
+CONFIG_I2C=y
+CONFIG_TPM=y
+CONFIG_NUBUS=y
+CONFIG_VIRTIO=y
diff --git a/configs/targets/any-softmmu.mak b/configs/targets/any-softmmu.mak
new file mode 100644
index 00..2c6cf1edd4
--- /dev/null
+++ b/configs/targets/any-softmmu.mak
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+TARGET_ARCH=any
diff --git a/meson.build b/meson.build
index e5d6f2d057..8a50193fa2 100644
--- a/meson.build
+++ b/meson.build
@@ -46,7 +46,7 @@ qapi_trace_events = []
 bsd_oses = ['gnu/kfreebsd', 'freebsd', 'netbsd', 'openbsd', 'dragonfly', 
'darwin']
 supported_oses = ['windows', 'freebsd', 'netbsd', 'openbsd', 'darwin', 
'sunos', 'linux']
 supported_cpus = ['ppc', 'ppc64', 's390x', 'riscv32', 'riscv64', 'x86', 
'x86_64',
-  'arm', 'aarch64', 'loongarch64', 'mips', 'mips64', 'sparc64']
+  'arm', 'aarch64', 'loongarch64', 'mips', 'mips64', 'sparc64', 'any']
 
 cpu = host_machine.cpu_family()
 
@@ -3016,7 +3016,9 @@ foreach target : target_dirs
 if default_targets
   continue
 endif
-error('No accelerator available for target @0@'.format(target))
+if 'any-softmmu' not in target_dirs
+  error('No accelerator available for target @0@'.format(target))
+endif
   endif
 
   actual_target_dirs += target
diff --git a/qapi/machine.json b/qapi/machine.json
index aa99fa333f..ac300669bb 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -36,7 +36,7 @@
  'mips64el', 'mipsel', 'nios2', 'or1k', 'ppc',
  'ppc64', 'riscv32', 'riscv64', 'rx', 's390x', 'sh4',
  'sh4eb', 'sparc', 'sparc64', 'tricore',
- 'x86_64', 'xtensa', 'xtensaeb' ] }
+ 'x86_64', 'xtensa', 'xtensaeb', 'any' ] }
 
 ##
 # @CpuS390State:
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 8850cb1a14..49bee75610 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -4,6 +4,7 @@
 
 enum {
 QEMU_ARCH_ALL = -1,
+QEMU_ARCH_ANY = -1,
 QEMU_ARCH_ALPHA = (1 << 0),
 QEMU_ARCH_ARM = (1 << 1),
 QEMU_ARCH_CRIS = (1 << 2),
diff --git a/target/any/cpu-param.h b/target/any/cpu-param.h
new file mode 100644
index 00..42e38ae991
--- /dev/null
+++ b/target/any/cpu-param.h
@@ -0,0 

[PATCH v2 0/1] target: New binary to prototype heterogeneous machines

[Philippe Mathieu-Daudé]

Almost 2 years later we got hundreds of cleanups patches
merged, so we can get this patch in. Building the 'any'
target has to be explictly requested in ./configure
target-list argument.

This binary will be use to rework QEMU startup code,
paving the way toward dynamic machines. It might also
allow experimenting with multiple TCG target frontends
and possibly prototyping concurrent HW/SW accelerations.

The corresponding CI jobs takes <5min:
https://gitlab.com/philmd/qemu/-/jobs/6138476547
Duration: 4 minutes 42 seconds

v1: https://lore.kernel.org/qemu-devel/20220215002658.60678-1-f4...@amsat.org/

Philippe Mathieu-Daudé (1):
  target: Add system emulation aiming to target any architecture

 configs/devices/any-softmmu/default.mak |  9 +
 configs/targets/any-softmmu.mak |  3 +++
 meson.build |  6 --
 qapi/machine.json   |  2 +-
 include/sysemu/arch_init.h  |  1 +
 target/any/cpu-param.h  | 13 +
 target/any/cpu-qom.h| 12 
 target/any/cpu.h| 24 
 .gitlab-ci.d/buildtest.yml  | 20 
 hw/any/meson.build  |  5 +
 hw/meson.build  |  1 +
 target/Kconfig  |  1 +
 target/any/Kconfig  |  4 
 target/any/meson.build  |  7 +++
 target/meson.build  |  1 +
 15 files changed, 106 insertions(+), 3 deletions(-)
 create mode 100644 configs/devices/any-softmmu/default.mak
 create mode 100644 configs/targets/any-softmmu.mak
 create mode 100644 target/any/cpu-param.h
 create mode 100644 target/any/cpu-qom.h
 create mode 100644 target/any/cpu.h
 create mode 100644 hw/any/meson.build
 create mode 100644 target/any/Kconfig
 create mode 100644 target/any/meson.build

-- 
2.41.0

[PATCH 3/3] system/physmem: Assign global system I/O Memory to machine

[Philippe Mathieu-Daudé]

So far there is only one system I/O and one system
memory per machine.

Signed-off-by: Philippe Mathieu-Daudé 
---
 system/physmem.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/system/physmem.c b/system/physmem.c
index 5e66d9ae36..50947a374e 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -2554,12 +2554,13 @@ static void memory_map_init(void)
 {
 system_memory = g_malloc(sizeof(*system_memory));
 
-memory_region_init(system_memory, NULL, "system", UINT64_MAX);
+memory_region_init(system_memory, OBJECT(current_machine),
+   "system", UINT64_MAX);
 address_space_init(_space_memory, system_memory, "memory");
 
 system_io = g_malloc(sizeof(*system_io));
-memory_region_init_io(system_io, NULL, _io_ops, NULL, "io",
-  65536);
+memory_region_init_io(system_io, OBJECT(current_machine),
+  _io_ops, NULL, "io", 65535);
 address_space_init(_space_io, system_io, "I/O");
 }
 
-- 
2.41.0

[PATCH 2/3] monitor/target: Include missing 'exec/memory.h' header

[Philippe Mathieu-Daudé]

Include "exec/memory.h" in order to avoid:

  monitor/hmp-cmds-target.c:263:10: error: call to undeclared function 
'memory_region_is_ram';
  ISO C99 and later do not support implicit function declarations 
[-Wimplicit-function-declaration]
  if (!memory_region_is_ram(mrs.mr) && !memory_region_is_romd(mrs.mr)) {
   ^

Signed-off-by: Philippe Mathieu-Daudé 
---
 monitor/hmp-cmds-target.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/monitor/hmp-cmds-target.c b/monitor/hmp-cmds-target.c
index d9fbcac08d..9338ae8440 100644
--- a/monitor/hmp-cmds-target.c
+++ b/monitor/hmp-cmds-target.c
@@ -25,6 +25,7 @@
 #include "qemu/osdep.h"
 #include "disas/disas.h"
 #include "exec/address-spaces.h"
+#include "exec/memory.h"
 #include "monitor/hmp-target.h"
 #include "monitor/monitor-internal.h"
 #include "qapi/error.h"
-- 
2.41.0

[PATCH 1/3] cpu-target: Include missing 'exec/memory.h' header

[Philippe Mathieu-Daudé]

Include "exec/memory.h" in order to avoid:

  cpu-target.c:201:50: error: use of undeclared identifier 'TYPE_MEMORY_REGION'
  DEFINE_PROP_LINK("memory", CPUState, memory, TYPE_MEMORY_REGION,
   ^

Signed-off-by: Philippe Mathieu-Daudé 
---
 cpu-target.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cpu-target.c b/cpu-target.c
index 958d63e882..86444cc2c6 100644
--- a/cpu-target.c
+++ b/cpu-target.c
@@ -31,6 +31,7 @@
 #else
 #include "hw/core/sysemu-cpu-ops.h"
 #include "exec/address-spaces.h"
+#include "exec/memory.h"
 #endif
 #include "sysemu/cpus.h"
 #include "sysemu/tcg.h"
-- 
2.41.0

[PATCH 0/3] system/memory: Trivial fixes

[Philippe Mathieu-Daudé]

- Include missing "exec/memory.h"
- Set machine as parent of io/mem regions

Philippe Mathieu-Daudé (3):
  cpu-target: Include missing 'exec/memory.h' header
  monitor/target: Include missing 'exec/memory.h' header
  system/physmem: Assign global system I/O Memory to machine

 cpu-target.c  | 1 +
 monitor/hmp-cmds-target.c | 1 +
 system/physmem.c  | 7 ---
 3 files changed, 6 insertions(+), 3 deletions(-)

-- 
2.41.0

Re: [PATCH v2 3/3] virtio-blk: Use ioeventfd_attach in start_ioeventfd

[Michael Tokarev]

02.02.2024 18:31, Hanna Czenczek :

Commit d3f6f294aeadd5f88caf0155e4360808c95b3146 ("virtio-blk: always set
ioeventfd during startup") has made virtio_blk_start_ioeventfd() always
kick the virtqueue (set the ioeventfd), regardless of whether the BB is
drained.  That is no longer necessary, because attaching the host
notifier will now set the ioeventfd, too; this happens either
immediately right here in virtio_blk_start_ioeventfd(), or later when
the drain ends, in virtio_blk_ioeventfd_attach().

With event_notifier_set() removed, the code becomes the same as the one
in virtio_blk_ioeventfd_attach(), so we can reuse that function.


The mentioned comit is v8.2.0-812-gd3f6f294ae, - ie, past 8.2.
Is this new change still relevant for stable?

Thanks,

/mjt

Re: [PATCH 0/2] block: Allow concurrent BB context changes

[Michael Tokarev]

02.02.2024 17:47, Hanna Czenczek :

Hi,

Without the AioContext lock, a BB's context may kind of change at any
time (unless it has a root node, and I/O requests are pending).  That
also means that its own context (BlockBackend.ctx) and that of its root
node can differ sometimes (while the context is being changed).


How relevant this is for -stable (8.2 at least) which does not have
"scsi: eliminate AioContext lock" patchset, and in particular,:
v8.2.0-124-geaad0fe260 "scsi: only access SCSIDevice->requests from
one thread"?

The issue first patch "block-backend: Allow concurrent context changes"
fixes (RHEL-19381) seems to be for 8.1.something, so it exists in 8.2
too, and this particular fix applies to 8.2.

But with other changes around all this, I'm a bit lost as of what should
be done on stable.  Not even thinking about 7.2 here :)

Thanks,

/mjt

[PULL 17/17] tests: Add case for LUKS volume with detached header

[Daniel P . Berrangé]

From: Hyman Huang 

Also, add a section to the MAINTAINERS file for detached
LUKS header, it only has a test case in it currently.

Signed-off-by: Hyman Huang 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Daniel P. Berrangé 
---
 MAINTAINERS   |   5 +
 tests/qemu-iotests/tests/luks-detached-header | 316 ++
 .../tests/luks-detached-header.out|   5 +
 3 files changed, 326 insertions(+)
 create mode 100755 tests/qemu-iotests/tests/luks-detached-header
 create mode 100644 tests/qemu-iotests/tests/luks-detached-header.out

diff --git a/MAINTAINERS b/MAINTAINERS
index 2f9741b898..f80db6a96a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3402,6 +3402,11 @@ F: migration/dirtyrate.c
 F: migration/dirtyrate.h
 F: include/sysemu/dirtyrate.h
 
+Detached LUKS header
+M: Hyman Huang 
+S: Maintained
+F: tests/qemu-iotests/tests/luks-detached-header
+
 D-Bus
 M: Marc-André Lureau 
 S: Maintained
diff --git a/tests/qemu-iotests/tests/luks-detached-header 
b/tests/qemu-iotests/tests/luks-detached-header
new file mode 100755
index 00..3455fd8de1
--- /dev/null
+++ b/tests/qemu-iotests/tests/luks-detached-header
@@ -0,0 +1,316 @@
+#!/usr/bin/env python3
+# group: rw auto
+#
+# Test LUKS volume with detached header
+#
+# Copyright (C) 2024 SmartX Inc.
+#
+# Authors:
+# Hyman Huang 
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import os
+import json
+import iotests
+from iotests import (
+imgfmt,
+qemu_img_create,
+qemu_img_info,
+QMPTestCase,
+)
+
+
+image_size = 128 * 1024 * 1024
+
+luks_img = os.path.join(iotests.test_dir, "luks.img")
+detached_header_img1 = os.path.join(iotests.test_dir, "detached_header.img1")
+detached_header_img2 = os.path.join(iotests.test_dir, "detached_header.img2")
+detached_payload_raw_img = os.path.join(
+iotests.test_dir, "detached_payload_raw.img"
+)
+detached_payload_qcow2_img = os.path.join(
+iotests.test_dir, "detached_payload_qcow2.img"
+)
+detached_header_raw_img = "json:" + json.dumps(
+{
+"driver": "luks",
+"file": {"filename": detached_payload_raw_img},
+"header": {
+"filename": detached_header_img1,
+},
+}
+)
+detached_header_qcow2_img = "json:" + json.dumps(
+{
+"driver": "luks",
+"file": {"filename": detached_payload_qcow2_img},
+"header": {"filename": detached_header_img2},
+}
+)
+
+secret_obj = "secret,id=sec0,data=foo"
+luks_opts = "key-secret=sec0"
+
+
+class TestDetachedLUKSHeader(QMPTestCase):
+def setUp(self) -> None:
+self.vm = iotests.VM()
+self.vm.add_object(secret_obj)
+self.vm.launch()
+
+# 1. Create the normal LUKS disk with 128M size
+self.vm.blockdev_create(
+{"driver": "file", "filename": luks_img, "size": 0}
+)
+self.vm.qmp_log(
+"blockdev-add",
+driver="file",
+filename=luks_img,
+node_name="luks-1-storage",
+)
+result = self.vm.blockdev_create(
+{
+"driver": imgfmt,
+"file": "luks-1-storage",
+"key-secret": "sec0",
+"size": image_size,
+"iter-time": 10,
+}
+)
+# None is expected
+self.assertEqual(result, None)
+
+# 2. Create the LUKS disk with detached header (raw)
+
+# Create detached LUKS header
+self.vm.blockdev_create(
+{"driver": "file", "filename": detached_header_img1, "size": 0}
+)
+self.vm.qmp_log(
+"blockdev-add",
+driver="file",
+filename=detached_header_img1,
+node_name="luks-2-header-storage",
+)
+
+# Create detached LUKS raw payload
+self.vm.blockdev_create(
+{"driver": "file", "filename": detached_payload_raw_img, "size": 0}
+)
+self.vm.qmp_log(
+"blockdev-add",
+driver="file",
+filename=detached_payload_raw_img,
+node_name="luks-2-payload-storage",
+)
+
+# Format LUKS disk with detached header
+result = self.vm.blockdev_create(
+{
+"driver": imgfmt,
+"header": "luks-2-header-storage",
+"file": "luks-2-payload-storage",
+

[PULL 02/17] crypto: Introduce SM4 symmetric cipher algorithm

[Daniel P . Berrangé]

From: Hyman Huang 

Introduce the SM4 cipher algorithms (OSCCA GB/T 32907-2016).

SM4 (GBT.32907-2016) is a cryptographic standard issued by the
Organization of State Commercial Administration of China (OSCCA)
as an authorized cryptographic algorithms for the use within China.

Detect the SM4 cipher algorithms and enable the feature silently
if it is available.

Signed-off-by: Hyman Huang 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Daniel P. Berrangé 
---
 crypto/block-luks.c | 11 
 crypto/cipher-gcrypt.c.inc  |  8 ++
 crypto/cipher-nettle.c.inc  | 49 +
 crypto/cipher.c |  6 
 meson.build | 26 +
 qapi/crypto.json|  5 +++-
 tests/unit/test-crypto-cipher.c | 13 +
 7 files changed, 117 insertions(+), 1 deletion(-)

diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index fb01ec38bb..f0813d69b4 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -95,12 +95,23 @@ qcrypto_block_luks_cipher_size_map_twofish[] = {
 { 0, 0 },
 };
 
+#ifdef CONFIG_CRYPTO_SM4
+static const QCryptoBlockLUKSCipherSizeMap
+qcrypto_block_luks_cipher_size_map_sm4[] = {
+{ 16, QCRYPTO_CIPHER_ALG_SM4},
+{ 0, 0 },
+};
+#endif
+
 static const QCryptoBlockLUKSCipherNameMap
 qcrypto_block_luks_cipher_name_map[] = {
 { "aes", qcrypto_block_luks_cipher_size_map_aes },
 { "cast5", qcrypto_block_luks_cipher_size_map_cast5 },
 { "serpent", qcrypto_block_luks_cipher_size_map_serpent },
 { "twofish", qcrypto_block_luks_cipher_size_map_twofish },
+#ifdef CONFIG_CRYPTO_SM4
+{ "sm4", qcrypto_block_luks_cipher_size_map_sm4},
+#endif
 };
 
 QEMU_BUILD_BUG_ON(sizeof(struct QCryptoBlockLUKSKeySlot) != 48);
diff --git a/crypto/cipher-gcrypt.c.inc b/crypto/cipher-gcrypt.c.inc
index a6a0117717..1377cbaf14 100644
--- a/crypto/cipher-gcrypt.c.inc
+++ b/crypto/cipher-gcrypt.c.inc
@@ -35,6 +35,9 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
 case QCRYPTO_CIPHER_ALG_SERPENT_256:
 case QCRYPTO_CIPHER_ALG_TWOFISH_128:
 case QCRYPTO_CIPHER_ALG_TWOFISH_256:
+#ifdef CONFIG_CRYPTO_SM4
+case QCRYPTO_CIPHER_ALG_SM4:
+#endif
 break;
 default:
 return false;
@@ -219,6 +222,11 @@ static QCryptoCipher 
*qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
 case QCRYPTO_CIPHER_ALG_TWOFISH_256:
 gcryalg = GCRY_CIPHER_TWOFISH;
 break;
+#ifdef CONFIG_CRYPTO_SM4
+case QCRYPTO_CIPHER_ALG_SM4:
+gcryalg = GCRY_CIPHER_SM4;
+break;
+#endif
 default:
 error_setg(errp, "Unsupported cipher algorithm %s",
QCryptoCipherAlgorithm_str(alg));
diff --git a/crypto/cipher-nettle.c.inc b/crypto/cipher-nettle.c.inc
index 24cc61f87b..42b39e18a2 100644
--- a/crypto/cipher-nettle.c.inc
+++ b/crypto/cipher-nettle.c.inc
@@ -33,6 +33,9 @@
 #ifndef CONFIG_QEMU_PRIVATE_XTS
 #include 
 #endif
+#ifdef CONFIG_CRYPTO_SM4
+#include 
+#endif
 
 static inline bool qcrypto_length_check(size_t len, size_t blocksize,
 Error **errp)
@@ -426,6 +429,30 @@ DEFINE_ECB_CBC_CTR_XTS(qcrypto_nettle_twofish,
QCryptoNettleTwofish, TWOFISH_BLOCK_SIZE,
twofish_encrypt_native, twofish_decrypt_native)
 
+#ifdef CONFIG_CRYPTO_SM4
+typedef struct QCryptoNettleSm4 {
+QCryptoCipher base;
+struct sm4_ctx key[2];
+} QCryptoNettleSm4;
+
+static void sm4_encrypt_native(void *ctx, size_t length,
+   uint8_t *dst, const uint8_t *src)
+{
+struct sm4_ctx *keys = ctx;
+sm4_crypt([0], length, dst, src);
+}
+
+static void sm4_decrypt_native(void *ctx, size_t length,
+   uint8_t *dst, const uint8_t *src)
+{
+struct sm4_ctx *keys = ctx;
+sm4_crypt([1], length, dst, src);
+}
+
+DEFINE_ECB(qcrypto_nettle_sm4,
+   QCryptoNettleSm4, SM4_BLOCK_SIZE,
+   sm4_encrypt_native, sm4_decrypt_native)
+#endif
 
 bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
  QCryptoCipherMode mode)
@@ -443,6 +470,9 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
 case QCRYPTO_CIPHER_ALG_TWOFISH_128:
 case QCRYPTO_CIPHER_ALG_TWOFISH_192:
 case QCRYPTO_CIPHER_ALG_TWOFISH_256:
+#ifdef CONFIG_CRYPTO_SM4
+case QCRYPTO_CIPHER_ALG_SM4:
+#endif
 break;
 default:
 return false;
@@ -701,6 +731,25 @@ static QCryptoCipher 
*qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
 
 return >base;
 }
+#ifdef CONFIG_CRYPTO_SM4
+case QCRYPTO_CIPHER_ALG_SM4:
+{
+QCryptoNettleSm4 *ctx = g_new0(QCryptoNettleSm4, 1);
+
+switch (mode) {
+case QCRYPTO_CIPHER_MODE_ECB:
+ctx->base.driver = _nettle_sm4_driver_ecb;
+break;
+default:
+goto bad_cipher_mode;
+}
+

[PULL 09/17] chardev: close QIOChannel before unref'ing

[Daniel P . Berrangé]

The chardev socket backend will unref the QIOChannel object while
it is still potentially open. When using TLS there could be a
pending TLS handshake taking place. If the channel is left open
then when the TLS handshake callback runs, it can end up accessing
free'd memory in the tcp_chr_tls_handshake method.

Closing the QIOChannel will unregister any pending handshake
source.

Reported-by: jiangyegen 
Reviewed-by: Marc-André Lureau 
Signed-off-by: Daniel P. Berrangé 
---
 chardev/char-socket.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 73947da188..7105753815 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -378,6 +378,10 @@ static void tcp_chr_free_connection(Chardev *chr)
  char_socket_yank_iochannel,
  QIO_CHANNEL(s->sioc));
 }
+
+if (s->ioc) {
+qio_channel_close(s->ioc, NULL);
+}
 object_unref(OBJECT(s->sioc));
 s->sioc = NULL;
 object_unref(OBJECT(s->ioc));
-- 
2.43.0

[PULL 03/17] qemu_init: increase NOFILE soft limit on POSIX

[Daniel P . Berrangé]

From: Fiona Ebner 

In many configurations, e.g. multiple vNICs with multiple queues or
with many Ceph OSDs, the default soft limit of 1024 is not enough.
QEMU is supposed to work fine with file descriptors >= 1024 and does
not use select() on POSIX. Bump the soft limit to the allowed hard
limit to avoid issues with the aforementioned configurations.

Of course the limit could be raised from the outside, but the man page
of systemd.exec states about 'LimitNOFILE=':

> Don't use.
> [...]
> Typically applications should increase their soft limit to the hard
> limit on their own, if they are OK with working with file
> descriptors above 1023,

If the soft limit is already the same as the hard limit, avoid the
superfluous setrlimit call. This can avoid a warning with a strict
seccomp filter blocking setrlimit if NOFILE was already raised before
executing QEMU.

Buglink: https://bugzilla.proxmox.com/show_bug.cgi?id=4507
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Fiona Ebner 
Signed-off-by: Daniel P. Berrangé 
---
 include/sysemu/os-posix.h |  1 +
 include/sysemu/os-win32.h |  5 +
 os-posix.c| 22 ++
 system/vl.c   |  2 ++
 4 files changed, 30 insertions(+)

diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index dff32ae185..b881ac6c6f 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -51,6 +51,7 @@ bool is_daemonized(void);
 void os_daemonize(void);
 bool os_set_runas(const char *user_id);
 void os_set_chroot(const char *path);
+void os_setup_limits(void);
 void os_setup_post(void);
 int os_mlock(void);
 
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 1047d260cb..b82a5d3ad9 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -128,6 +128,11 @@ static inline int os_mlock(void)
 return -ENOSYS;
 }
 
+static inline void os_setup_limits(void)
+{
+return;
+}
+
 #define fsync _commit
 
 #if !defined(lseek)
diff --git a/os-posix.c b/os-posix.c
index 52ef6990ff..a4284e2c07 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -24,6 +24,7 @@
  */
 
 #include "qemu/osdep.h"
+#include 
 #include 
 #include 
 #include 
@@ -256,6 +257,27 @@ void os_daemonize(void)
 }
 }
 
+void os_setup_limits(void)
+{
+struct rlimit nofile;
+
+if (getrlimit(RLIMIT_NOFILE, ) < 0) {
+warn_report("unable to query NOFILE limit: %s", strerror(errno));
+return;
+}
+
+if (nofile.rlim_cur == nofile.rlim_max) {
+return;
+}
+
+nofile.rlim_cur = nofile.rlim_max;
+
+if (setrlimit(RLIMIT_NOFILE, ) < 0) {
+warn_report("unable to set NOFILE limit: %s", strerror(errno));
+return;
+}
+}
+
 void os_setup_post(void)
 {
 int fd = 0;
diff --git a/system/vl.c b/system/vl.c
index 2a0bd08ff1..95cd2d91c4 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2778,6 +2778,8 @@ void qemu_init(int argc, char **argv)
 error_init(argv[0]);
 qemu_init_exec_dir(argv[0]);
 
+os_setup_limits();
+
 qemu_init_arch_modules();
 
 qemu_init_subsystems();
-- 
2.43.0

[PULL 15/17] block: Support detached LUKS header creation using qemu-img

[Daniel P . Berrangé]

From: Hyman Huang 

Even though a LUKS header might be created with cryptsetup,
qemu-img should be enhanced to accommodate it as well.

Add the 'detached-header' option to specify the creation of
a detached LUKS header. This is how it is used:
$ qemu-img create --object secret,id=sec0,data=abc123 -f luks
> -o cipher-alg=aes-256,cipher-mode=xts -o key-secret=sec0
> -o detached-header=true header.luks

Using qemu-img or cryptsetup tools to query information of
an LUKS header image as follows:

Assume a detached LUKS header image has been created by:
$ dd if=/dev/zero of=test-header.img bs=1M count=32
$ dd if=/dev/zero of=test-payload.img bs=1M count=1000
$ cryptsetup luksFormat --header test-header.img test-payload.img
> --force-password --type luks1

Header image information could be queried using cryptsetup:
$ cryptsetup luksDump test-header.img

or qemu-img:
$ qemu-img info 'json:{"driver":"luks","file":{"filename":
> "test-payload.img"},"header":{"filename":"test-header.img"}}'

When using qemu-img, keep in mind that the entire disk
information specified by the JSON-format string above must be
supplied on the commandline; if not, an overlay check will reveal
a problem with the LUKS volume check logic.

Signed-off-by: Hyman Huang 
Reviewed-by: Daniel P. Berrangé 
[changed to pass 'cflags' to block_crypto_co_create_generic]
Signed-off-by: Daniel P. Berrangé 
---
 block.c  |  5 -
 block/crypto.c   | 12 ++--
 block/crypto.h   |  8 
 qapi/crypto.json |  5 -
 4 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index 30afdcbba6..1ed9214f66 100644
--- a/block.c
+++ b/block.c
@@ -7357,7 +7357,10 @@ void bdrv_img_create(const char *filename, const char 
*fmt,
 goto out;
 }
 
-if (size == -1) {
+/* Parameter 'size' is not needed for detached LUKS header */
+if (size == -1 &&
+!(!strcmp(fmt, "luks") &&
+  qemu_opt_get_bool(opts, "detached-header", false))) {
 error_setg(errp, "Image creation needs a size parameter");
 goto out;
 }
diff --git a/block/crypto.c b/block/crypto.c
index 8e7ee5e9ac..21eed909c1 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -231,6 +231,7 @@ static QemuOptsList block_crypto_create_opts_luks = {
 BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_HASH_ALG(""),
 BLOCK_CRYPTO_OPT_DEF_LUKS_HASH_ALG(""),
 BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME(""),
+BLOCK_CRYPTO_OPT_DEF_LUKS_DETACHED_HEADER(""),
 { /* end of list */ }
 },
 };
@@ -405,7 +406,7 @@ block_crypto_co_create_generic(BlockDriverState *bs, 
int64_t size,
 
 data = (struct BlockCryptoCreateData) {
 .blk = blk,
-.size = size,
+.size = flags & QCRYPTO_BLOCK_CREATE_DETACHED ? 0 : size,
 .prealloc = prealloc,
 };
 
@@ -791,6 +792,9 @@ block_crypto_co_create_opts_luks(BlockDriver *drv, const 
char *filename,
 PreallocMode prealloc;
 char *buf = NULL;
 int64_t size;
+bool detached_hdr =
+qemu_opt_get_bool(opts, "detached-header", false);
+unsigned int cflags = 0;
 int ret;
 Error *local_err = NULL;
 
@@ -830,9 +834,13 @@ block_crypto_co_create_opts_luks(BlockDriver *drv, const 
char *filename,
 goto fail;
 }
 
+if (detached_hdr) {
+cflags |= QCRYPTO_BLOCK_CREATE_DETACHED;
+}
+
 /* Create format layer */
 ret = block_crypto_co_create_generic(bs, size, create_opts,
- prealloc, 0, errp);
+ prealloc, cflags, errp);
 if (ret < 0) {
 goto fail;
 }
diff --git a/block/crypto.h b/block/crypto.h
index 72e792c9af..dc3d2d5ed9 100644
--- a/block/crypto.h
+++ b/block/crypto.h
@@ -41,6 +41,7 @@
 #define BLOCK_CRYPTO_OPT_LUKS_IVGEN_HASH_ALG "ivgen-hash-alg"
 #define BLOCK_CRYPTO_OPT_LUKS_HASH_ALG "hash-alg"
 #define BLOCK_CRYPTO_OPT_LUKS_ITER_TIME "iter-time"
+#define BLOCK_CRYPTO_OPT_LUKS_DETACHED_HEADER "detached-header"
 #define BLOCK_CRYPTO_OPT_LUKS_KEYSLOT "keyslot"
 #define BLOCK_CRYPTO_OPT_LUKS_STATE "state"
 #define BLOCK_CRYPTO_OPT_LUKS_OLD_SECRET "old-secret"
@@ -100,6 +101,13 @@
 .help = "Select new state of affected keyslots (active/inactive)",\
 }
 
+#define BLOCK_CRYPTO_OPT_DEF_LUKS_DETACHED_HEADER(prefix) \
+{ \
+.name = prefix BLOCK_CRYPTO_OPT_LUKS_DETACHED_HEADER, \
+.type = QEMU_OPT_BOOL,\
+.help = "Create a detached LUKS header",  \
+}
+
 #define BLOCK_CRYPTO_OPT_DEF_LUKS_KEYSLOT(prefix)  \
 {  \
 .name = prefix BLOCK_CRYPTO_OPT_LUKS_KEYSLOT,  \
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 2f2aeff5fd..22c6cce3ae 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -226,6 +226,8 @@
 # @iter-time: number of milliseconds to spend in PBKDF passphrase
 

[PULL 07/17] docs: fix highlighting of CPU ABI header rows

[Daniel P . Berrangé]

The 'header-rows' directive indicates how many rows in the generated
table are to be highlighted as headers. We only have one such row in
the CSV file included. This removes the accident bold highlighting
of the 'i486' CPU model.

Signed-off-by: Daniel P. Berrangé 
---
 docs/system/cpu-models-x86.rst.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/system/cpu-models-x86.rst.inc 
b/docs/system/cpu-models-x86.rst.inc
index 7f6368f999..ba27b5683f 100644
--- a/docs/system/cpu-models-x86.rst.inc
+++ b/docs/system/cpu-models-x86.rst.inc
@@ -58,7 +58,7 @@ depending on the machine type is in use.
 .. csv-table:: x86-64 ABI compatibility levels
:file: cpu-models-x86-abi.csv
:widths: 40,15,15,15,15
-   :header-rows: 2
+   :header-rows: 1
 
 
 Preferred CPU models for Intel x86 hosts
-- 
2.43.0

[PULL 16/17] crypto: Introduce 'detached-header' field in QCryptoBlockInfoLUKS

[Daniel P . Berrangé]

From: Hyman Huang 

When querying the LUKS disk with the qemu-img tool or other APIs,
add information about whether the LUKS header is detached.

Additionally, update the test case with the appropriate
modification.

Signed-off-by: Hyman Huang 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Daniel P. Berrangé 
---
 crypto/block-luks.c| 2 ++
 qapi/crypto.json   | 3 +++
 tests/qemu-iotests/210.out | 4 
 3 files changed, 9 insertions(+)

diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index ab52c9dce1..3ee928fb5a 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -1271,6 +1271,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
 block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
 block->payload_offset = luks->header.payload_offset_sector *
 block->sector_size;
+block->detached_header = (block->payload_offset == 0) ? true : false;
 
 return 0;
 
@@ -1895,6 +1896,7 @@ static int qcrypto_block_luks_get_info(QCryptoBlock 
*block,
 info->u.luks.master_key_iters = luks->header.master_key_iterations;
 info->u.luks.uuid = g_strndup((const char *)luks->header.uuid,
   sizeof(luks->header.uuid));
+info->u.luks.detached_header = block->detached_header;
 
 for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
 slot = g_new0(QCryptoBlockInfoLUKSSlot, 1);
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 22c6cce3ae..ad8dd37175 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -317,6 +317,8 @@
 #
 # @hash-alg: the master key hash algorithm
 #
+# @detached-header: whether the LUKS header is detached (Since 9.0)
+#
 # @payload-offset: offset to the payload data in bytes
 #
 # @master-key-iters: number of PBKDF2 iterations for key material
@@ -333,6 +335,7 @@
'ivgen-alg': 'QCryptoIVGenAlgorithm',
'*ivgen-hash-alg': 'QCryptoHashAlgorithm',
'hash-alg': 'QCryptoHashAlgorithm',
+   'detached-header': 'bool',
'payload-offset': 'int',
'master-key-iters': 'int',
'uuid': 'str',
diff --git a/tests/qemu-iotests/210.out b/tests/qemu-iotests/210.out
index 96d9f749dd..94b29b2120 100644
--- a/tests/qemu-iotests/210.out
+++ b/tests/qemu-iotests/210.out
@@ -18,6 +18,7 @@ virtual size: 128 MiB (134217728 bytes)
 encrypted: yes
 Format specific information:
 ivgen alg: plain64
+detached header: false
 hash alg: sha256
 cipher alg: aes-256
 uuid: ----
@@ -70,6 +71,7 @@ virtual size: 64 MiB (67108864 bytes)
 encrypted: yes
 Format specific information:
 ivgen alg: plain64
+detached header: false
 hash alg: sha1
 cipher alg: aes-128
 uuid: ----
@@ -125,6 +127,7 @@ virtual size: 0 B (0 bytes)
 encrypted: yes
 Format specific information:
 ivgen alg: plain64
+detached header: false
 hash alg: sha256
 cipher alg: aes-256
 uuid: ----
@@ -195,6 +198,7 @@ virtual size: 0 B (0 bytes)
 encrypted: yes
 Format specific information:
 ivgen alg: plain64
+detached header: false
 hash alg: sha256
 cipher alg: aes-256
 uuid: ----
-- 
2.43.0

[PULL 13/17] crypto: Modify the qcrypto_block_create to support creation flags

[Daniel P . Berrangé]

From: Hyman Huang 

Expand the signature of qcrypto_block_create to enable the
formation of LUKS volumes with detachable headers. To accomplish
that, introduce QCryptoBlockCreateFlags to instruct the creation
process to set the payload_offset_sector to 0.

Signed-off-by: Hyman Huang 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Daniel P. Berrangé 
---
 block/crypto.c |  1 +
 block/qcow.c   |  2 +-
 block/qcow2.c  |  2 +-
 crypto/block-luks.c| 28 +---
 crypto/block.c |  4 +++-
 crypto/blockpriv.h |  2 ++
 include/crypto/block.h | 11 +++
 tests/unit/test-crypto-block.c |  2 ++
 8 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index e87dc84111..1b3f87922a 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -369,6 +369,7 @@ block_crypto_co_create_generic(BlockDriverState *bs, 
int64_t size,
   block_crypto_create_init_func,
   block_crypto_create_write_func,
   ,
+  0,
   errp);
 
 if (!crypto) {
diff --git a/block/qcow.c b/block/qcow.c
index c6d0e15f1e..ca8e1d5ec8 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -885,7 +885,7 @@ qcow_co_create(BlockdevCreateOptions *opts, Error **errp)
 header.crypt_method = cpu_to_be32(QCOW_CRYPT_AES);
 
 crypto = qcrypto_block_create(qcow_opts->encrypt, "encrypt.",
-  NULL, NULL, NULL, errp);
+  NULL, NULL, NULL, 0, errp);
 if (!crypto) {
 ret = -EINVAL;
 goto exit;
diff --git a/block/qcow2.c b/block/qcow2.c
index 9bee66fff5..204f5854cf 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3216,7 +3216,7 @@ qcow2_set_up_encryption(BlockDriverState *bs,
 crypto = qcrypto_block_create(cryptoopts, "encrypt.",
   qcow2_crypto_hdr_init_func,
   qcow2_crypto_hdr_write_func,
-  bs, errp);
+  bs, 0, errp);
 if (!crypto) {
 return -EINVAL;
 }
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 7e1235c213..ab52c9dce1 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -1315,6 +1315,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
 const char *hash_alg;
 g_autofree char *cipher_mode_spec = NULL;
 uint64_t iters;
+uint64_t detached_header_size;
 
 memcpy(_opts, >u.luks, sizeof(luks_opts));
 if (!luks_opts.has_iter_time) {
@@ -1543,19 +1544,32 @@ qcrypto_block_luks_create(QCryptoBlock *block,
 slot->stripes = QCRYPTO_BLOCK_LUKS_STRIPES;
 }
 
-/* The total size of the LUKS headers is the partition header + key
- * slot headers, rounded up to the nearest sector, combined with
- * the size of each master key material region, also rounded up
- * to the nearest sector */
-luks->header.payload_offset_sector = header_sectors +
-QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS * split_key_sectors;
+if (block->detached_header) {
+/*
+ * For a detached LUKS header image, set the payload_offset_sector
+ * to 0 to specify the starting point for read/write
+ */
+luks->header.payload_offset_sector = 0;
+} else {
+/*
+ * The total size of the LUKS headers is the partition header + key
+ * slot headers, rounded up to the nearest sector, combined with
+ * the size of each master key material region, also rounded up
+ * to the nearest sector
+ */
+luks->header.payload_offset_sector = header_sectors +
+QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS * split_key_sectors;
+}
 
 block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
 block->payload_offset = luks->header.payload_offset_sector *
 block->sector_size;
+detached_header_size =
+(header_sectors + QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS *
+ split_key_sectors) * block->sector_size;
 
 /* Reserve header space to match payload offset */
-initfunc(block, block->payload_offset, opaque, _err);
+initfunc(block, detached_header_size, opaque, _err);
 if (local_err) {
 error_propagate(errp, local_err);
 goto error;
diff --git a/crypto/block.c b/crypto/block.c
index 7bb4b74a37..506ea1d1a3 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -87,6 +87,7 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions 
*options,
QCryptoBlockInitFunc initfunc,
QCryptoBlockWriteFunc writefunc,
void *opaque,
+   unsigned int flags,
Error **errp)
 {