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

[Manos Pitsidianakis]

On Fri, 09 Feb 2024 23:59, Paolo Bonzini  wrote:

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




Reviewed-by: Manos Pitsidianakis 

Re: [PATCH v3 2/2] aspeed: fix hardcode boot address 0

[Cédric Le Goater]

On 2/15/24 08:59, Jamin Lin wrote:

In the previous design of ASPEED SOCs QEMU model, it set the boot
address at "0" which was the hardcode setting for ast10x0, ast2600,
ast2500 and ast2400.

According to the design of ast2700, it has a bootmcu(riscv-32) which
is used for executing SPL and initialize DRAM and copy u-boot image
from SPI/Flash to DRAM at address 0x4 at SPL boot stage.
Then, CPUs(cortex-a35) execute u-boot, kernel and rofs.

Currently, qemu not support emulate two CPU architectures
at the same machine. Therefore, qemu will only support
to emulate CPU(cortex-a35) side for ast2700 and the boot
address is "0x4 ".

Fixed hardcode boot address "0" for future models using
a different mapping address.

Signed-off-by: Troy Lee 
Signed-off-by: Jamin Lin 



Reviewed-by: Cédric Le Goater 

Thanks,

C.



---
  hw/arm/aspeed.c | 4 +++-
  hw/arm/aspeed_ast2400.c | 4 ++--
  hw/arm/aspeed_ast2600.c | 2 +-
  include/hw/arm/aspeed_soc.h | 2 --
  4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index aa165d583b..9fec245e4e 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -289,12 +289,14 @@ static void aspeed_install_boot_rom(AspeedMachineState 
*bmc, BlockBackend *blk,
  uint64_t rom_size)
  {
  AspeedSoCState *soc = bmc->soc;
+AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(soc);
  
  memory_region_init_rom(&bmc->boot_rom, NULL, "aspeed.boot_rom", rom_size,

 &error_abort);
  memory_region_add_subregion_overlap(&soc->spi_boot_container, 0,
  &bmc->boot_rom, 1);
-write_boot_rom(blk, ASPEED_SOC_SPI_BOOT_ADDR, rom_size, &error_abort);
+write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT],
+   rom_size, &error_abort);
  }
  
  void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,

diff --git a/hw/arm/aspeed_ast2400.c b/hw/arm/aspeed_ast2400.c
index 95da85fee0..d125886207 100644
--- a/hw/arm/aspeed_ast2400.c
+++ b/hw/arm/aspeed_ast2400.c
@@ -26,7 +26,7 @@
  #define ASPEED_SOC_IOMEM_SIZE   0x0020
  
  static const hwaddr aspeed_soc_ast2400_memmap[] = {

-[ASPEED_DEV_SPI_BOOT]  =  ASPEED_SOC_SPI_BOOT_ADDR,
+[ASPEED_DEV_SPI_BOOT]  = 0x,
  [ASPEED_DEV_IOMEM]  = 0x1E60,
  [ASPEED_DEV_FMC]= 0x1E62,
  [ASPEED_DEV_SPI1]   = 0x1E63,
@@ -61,7 +61,7 @@ static const hwaddr aspeed_soc_ast2400_memmap[] = {
  };
  
  static const hwaddr aspeed_soc_ast2500_memmap[] = {

-[ASPEED_DEV_SPI_BOOT]  = ASPEED_SOC_SPI_BOOT_ADDR,
+[ASPEED_DEV_SPI_BOOT]  = 0x,
  [ASPEED_DEV_IOMEM]  = 0x1E60,
  [ASPEED_DEV_FMC]= 0x1E62,
  [ASPEED_DEV_SPI1]   = 0x1E63,
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index f74561ecdc..174be53770 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -22,7 +22,7 @@
  #define ASPEED_SOC_DPMCU_SIZE   0x0004
  
  static const hwaddr aspeed_soc_ast2600_memmap[] = {

-[ASPEED_DEV_SPI_BOOT]  = ASPEED_SOC_SPI_BOOT_ADDR,
+[ASPEED_DEV_SPI_BOOT]  = 0x,
  [ASPEED_DEV_SRAM]  = 0x1000,
  [ASPEED_DEV_DPMCU] = 0x1800,
  /* 0x1600 0x17FF : AHB BUS do LPC Bus bridge */
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index e1a023be53..c60fac900a 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -224,8 +224,6 @@ enum {
  ASPEED_DEV_FSI2,
  };
  
-#define ASPEED_SOC_SPI_BOOT_ADDR 0x0

-
  qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev);
  bool aspeed_soc_uart_realize(AspeedSoCState *s, Error **errp);
  void aspeed_soc_uart_set_chr(AspeedSoCState *s, int dev, Chardev *chr);

Re: [PATCH v3 1/2] aspeed: introduce a new UART0 device name

[Cédric Le Goater]

On 2/15/24 08:59, Jamin Lin wrote:

The Aspeed datasheet refers to the UART controllers
as UART1 - UART13 for the ast10x0, ast2600, ast2500
and ast2400 SoCs and the Aspeed ast2700 introduces an UART0
and the UART controllers as UART0 - UART12.

To keep the naming in the QEMU models
in sync with the datasheet, let's introduce a new  UART0 device name
and do the required adjustements.

Signed-off-by: Troy Lee 
Signed-off-by: Jamin Lin 


Reviewed-by: Cédric Le Goater 

One comment below,


---
  hw/arm/aspeed.c | 13 -
  hw/arm/aspeed_ast10x0.c |  1 +
  hw/arm/aspeed_ast2400.c |  2 ++
  hw/arm/aspeed_ast2600.c |  1 +
  hw/arm/aspeed_soc_common.c  | 10 ++
  include/hw/arm/aspeed_soc.h | 17 +
  6 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 09b1e823ba..aa165d583b 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -342,7 +342,7 @@ static void connect_serial_hds_to_uarts(AspeedMachineState 
*bmc)
  int uart_chosen = bmc->uart_chosen ? bmc->uart_chosen : amc->uart_default;
  
  aspeed_soc_uart_set_chr(s, uart_chosen, serial_hd(0));

-for (int i = 1, uart = ASPEED_DEV_UART1; i < sc->uarts_num; i++, uart++) {
+for (int i = 0, uart = sc->uarts_base; i < sc->uarts_num; i++, uart++) {
  if (uart == uart_chosen) {
  continue;
  }
@@ -1094,7 +1094,7 @@ static char *aspeed_get_bmc_console(Object *obj, Error 
**errp)
  AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(bmc);
  int uart_chosen = bmc->uart_chosen ? bmc->uart_chosen : amc->uart_default;
  
-return g_strdup_printf("uart%d", uart_chosen - ASPEED_DEV_UART1 + 1);

+return g_strdup_printf("uart%d", aspeed_uart_index(uart_chosen));
  }
  
  static void aspeed_set_bmc_console(Object *obj, const char *value, Error **errp)

@@ -1103,6 +1103,8 @@ static void aspeed_set_bmc_console(Object *obj, const 
char *value, Error **errp)
  AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(bmc);
  AspeedSoCClass *sc = 
ASPEED_SOC_CLASS(object_class_by_name(amc->soc_name));
  int val;
+int uart_first = aspeed_uart_first(sc);
+int uart_last = aspeed_uart_last(sc);
  
  if (sscanf(value, "uart%u", &val) != 1) {

  error_setg(errp, "Bad value for \"uart\" property");
@@ -1110,11 +1112,12 @@ static void aspeed_set_bmc_console(Object *obj, const 
char *value, Error **errp)
  }
  
  /* The number of UART depends on the SoC */

-if (val < 1 || val > sc->uarts_num) {
-error_setg(errp, "\"uart\" should be in range [1 - %d]", 
sc->uarts_num);
+if (val < uart_first || val > uart_last) {
+error_setg(errp, "\"uart\" should be in range [%d - %d]",
+   uart_first, uart_last);
  return;
  }
-bmc->uart_chosen = ASPEED_DEV_UART1 + val - 1;
+bmc->uart_chosen = val + ASPEED_DEV_UART0;
  }
  
  static void aspeed_machine_class_props_init(ObjectClass *oc)

diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index c3b5116a6a..2634e0f654 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -436,6 +436,7 @@ static void aspeed_soc_ast1030_class_init(ObjectClass 
*klass, void *data)
  sc->wdts_num = 4;
  sc->macs_num = 1;
  sc->uarts_num = 13;
+sc->uarts_base = ASPEED_DEV_UART1;
  sc->irqmap = aspeed_soc_ast1030_irqmap;
  sc->memmap = aspeed_soc_ast1030_memmap;
  sc->num_cpus = 1;
diff --git a/hw/arm/aspeed_ast2400.c b/hw/arm/aspeed_ast2400.c
index 8829561bb6..95da85fee0 100644
--- a/hw/arm/aspeed_ast2400.c
+++ b/hw/arm/aspeed_ast2400.c
@@ -523,6 +523,7 @@ static void aspeed_soc_ast2400_class_init(ObjectClass *oc, 
void *data)
  sc->wdts_num = 2;
  sc->macs_num = 2;
  sc->uarts_num= 5;
+sc->uarts_base   = ASPEED_DEV_UART1;
  sc->irqmap   = aspeed_soc_ast2400_irqmap;
  sc->memmap   = aspeed_soc_ast2400_memmap;
  sc->num_cpus = 1;
@@ -551,6 +552,7 @@ static void aspeed_soc_ast2500_class_init(ObjectClass *oc, 
void *data)
  sc->wdts_num = 3;
  sc->macs_num = 2;
  sc->uarts_num= 5;
+sc->uarts_base   = ASPEED_DEV_UART1;
  sc->irqmap   = aspeed_soc_ast2500_irqmap;
  sc->memmap   = aspeed_soc_ast2500_memmap;
  sc->num_cpus = 1;
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 4ee32ea99d..f74561ecdc 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -666,6 +666,7 @@ static void aspeed_soc_ast2600_class_init(ObjectClass *oc, 
void *data)
  sc->wdts_num = 4;
  sc->macs_num = 4;
  sc->uarts_num= 13;
+sc->uarts_base   = ASPEED_DEV_UART1;
  sc->irqmap   = aspeed_soc_ast2600_irqmap;
  sc->memmap   = aspeed_soc_ast2600_memmap;
  sc->num_cpus = 2;
diff --git a/hw/arm/aspeed_soc_common.c b/hw/arm/aspeed_soc_common.c
index 123a0c432c..95d0c0aba9 100644
--- a/hw/arm/aspeed_soc_common.c
+++ b/hw/arm/aspee

Re: [PATCH v4 00/10] Optimize buffer_is_zero

[Richard Henderson]

On 2/15/24 13:37, Alexander Monakov wrote:

Ah, I guess you might be running at low perf_event_paranoid setting that
allows unprivileged sampling of kernel events? In our submissions the
percentage was for perf_event_paranoid=2, i.e. relative to Qemu only,
excluding kernel time under syscalls.


Ok.  Eliminating kernel samples makes things easier to see.
But I still do not see a 40% reduction in runtime.

Just so we're on the same page:


Retrieve IE11.Win7.VirtualBox.zip from
https://archive.org/details/ie11.win7.virtualbox
and use

  unzip -p IE11.Win7.VirtualBox.zip | tar xv

to extract 'IE11 - Win7-disk001.vmdk'.

(Mikhail used a different image when preparing the patch)

On this image, I get 70% in buffer_zero_sse2 on a Sandy Bridge running

  qemu-img convert 'IE11 - Win7-disk001.vmdk' -O qcow2 /tmp/t.qcow2


With this, I see virtually all of the runtime in libz.so.
Therefore I converted this to raw first, to focus on the issue.

For avoidance of doubt:

$ ls -lsh test.raw && sha256sum test.raw
 12G -rw-r--r--  1 rth  rth   40G Feb 15 21:14 test.raw
3b056d839952538fed42fa898c6063646f4fda1bf7ea0180fbb5f29d21fe8e80  test.raw

Host: 11th Gen Intel(R) Core(TM) i7-1195G7 @ 2.90GHz
Compiler: gcc version 11.4.0 (Ubuntu 11.4.0-1ubuntu1~22.04)

master:
  57.48%  qemu-img-m  [.] buffer_zero_avx2
   3.60%  qemu-img-m  [.] is_allocated_sectors.part.0
   2.61%  qemu-img-m  [.] buffer_is_zero
  63.69%  -- total

v3:
  48.86%  qemu-img-v3  [.] is_allocated_sectors.part.0
   3.79%  qemu-img-v3  [.] buffer_zero_avx2
  52.65%  -- total
-17%  -- reduction from master

v4:
  54.60%  qemu-img-v4  [.] buffer_is_zero_ge256
   3.30%  qemu-img-v4  [.] buffer_zero_avx2
   3.17%  qemu-img-v4  [.] is_allocated_sectors.part.0
  61.07%  -- total
 -4%  -- reduction from master

v4+:
  46.65%  qemu-img  [.] is_allocated_sectors.part.0
   3.49%  qemu-img  [.] buffer_zero_avx2
   0.05%  qemu-img  [.] buffer_is_zero_ge256
  50.19%  -- total
-21%  -- reduction from master

The v4+ puts the 3 byte test back inline, like in your v3.

Importantly, it must be as 3 short-circuting tests, where my v4 "simplified" this to (s | 
m | e) != 0, on the assumption that the reduced number of branches would help.


Diving into perf, it becomes clear why:

 57.36 │   cmpb   $0x0,(%rbx)
  4.02 │ ↓ jne89
 21.84 │   cmpb   $0x0,0x1ff(%rbx)
  0.64 │ ↓ jne89
  8.45 │   cmpb   $0x0,0x100(%rbx)
  0.26 │ ↓ jne89
  0.06 │   mov$0x200,%esi
   │   mov%rbx,%rdi
  0.07 │ → call   buffer_is_zero_ge256

The three bytes are on 3 different cachelines.  Judging by the relative percentages, it 
would seem that the first byte alone eliminates slightly more than half of all blocks; the 
last byte eliminates more than half again; the middle byte eliminates a fair fraction of 
the rest.  With the short-circuit, the extra cachelines are not touched.


This is so important that it should be spelled out in a comment.

With that settled, I guess we need to talk about how much the out-of-line implementation 
matters at all.  I'm thinking about writing a test/bench/bufferiszero, with all-zero 
buffers of various sizes and alignments.  With that it would be easier to talk about 
whether any given implementation is is an improvement for that final 4% not eliminated by 
the three bytes.



(which does tell us that qemu-img is doing I/O inefficiently, it shouldn't
need two seconds to read a fully cached 5 Gigabyte file)


Indeed!


r~

Re: [PATCH 5/6] hw/vfio/common: Use RCU_READ macros

[Philippe Mathieu-Daudé]

On 24/1/24 15:09, Philippe Mathieu-Daudé wrote:

On 24/1/24 10:25, Manos Pitsidianakis wrote:
On Wed, 24 Jan 2024 09:42, Philippe Mathieu-Daudé  
wrote:

Replace the manual rcu_read_(un)lock calls by the
*RCU_READ_LOCK_GUARD macros (See commit ef46ae67ba
"docs/style: call out the use of GUARD macros").

Signed-off-by: Philippe Mathieu-Daudé 
---
hw/vfio/common.c | 34 --
1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 4aa86f563c..09878a3603 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -308,13 +308,13 @@ static void vfio_iommu_map_notify(IOMMUNotifier 
*n, IOMMUTLBEntry *iotlb)

    return;
    }

-    rcu_read_lock();
+    RCU_READ_LOCK_GUARD();

    if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
    bool read_only;

    if (!vfio_get_xlat_addr(iotlb, &vaddr, NULL, &read_only)) {
-    goto out;
+    return;


Since this is the only early return, we could alternatively do:

- if (!vfio_get_xlat_addr(iotlb, &vaddr, NULL, &read_only)) {
+ if (vfio_get_xlat_addr(iotlb, &vaddr, NULL, &read_only)) {

remove the goto/return, and wrap the rest of the codeflow in this if's 
brackets. And then we could use WITH_RCU_READ_LOCK_GUARD instead. 
That'd increase the code indentation however.


If the maintainer agrees with the style & code churn, I don't
mind respining.


Alex, Cédric, any preference?






    }
    /*
 * vaddr is only valid until rcu_read_unlock(). But after
@@ -343,8 +343,6 @@ static void vfio_iommu_map_notify(IOMMUNotifier 
*n, IOMMUTLBEntry *iotlb)

    vfio_set_migration_error(ret);
    }
    }
-out:
-    rcu_read_unlock();
}



Reviewed-by: Manos Pitsidianakis 


Thanks!

Re: [PATCH v4 0/3] Initial support for SPDM Responders

[Klaus Jensen]

On Feb 15 14:44, Jonathan Cameron wrote:
> On Tue, 13 Feb 2024 12:44:00 +1000
> Alistair Francis  wrote:
> 
> Hi All,
> 
> Just wanted to add that back in v2 Klaus Jensen stated:
> 
> "I have no problem with picking this up for nvme, but I'd rather not take
>  the full series through my tree without reviews/acks from the pci
>  maintainers."
> 
> So I'd like to add my request that Michael and/or Marcell takes a look
> when they have time.
> 
> I've been carrying more or less the first 2 patches in my CXL staging
> tree for a couple of years (the initial Linux Kernel support that Lukas
> Wunner is now handling was developed against this) and I would love
> to see this upstream. Along with PCI and CXL and NVME usecases this
> is a major part of the Confidential Compute device assignment story
> via PCI/TDISP and CXL equivalent.
> 
> It's not changed in significant ways since v2 back in October last year.
> 

Would someone be willing to sign up to maintain the spdm_socket backend?


signature.asc
Description: PGP signature

[PATCH 3/3] qtest: migration: Add negative validation test for 'uri' and 'channels' both set

[Het Gala]

Ideally QAPI 'migrate' and 'migrate-incoming' does not allow 'uri' and
'channels' both arguments to be present in the arguments list as they
are mutually exhaustive.

Add a negative test case to validate the same. Even before the migration
connection is established, qmp command will error out with
MIG_TEST_QMP_ERROR.

Signed-off-by: Het Gala 
---
 tests/qtest/migration-test.c | 83 
 1 file changed, 83 insertions(+)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 0bc69b1943..9b9395307f 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -26,6 +26,7 @@
 #include "qapi/qobject-output-visitor.h"
 #include "crypto/tlscredspsk.h"
 #include "qapi/qmp/qlist.h"
+#include "qemu/cutils.h"
 
 #include "migration-helpers.h"
 #include "tests/migration/migration-test.h"
@@ -2516,6 +2517,86 @@ static void test_validate_uuid_dst_not_set(void)
 do_test_validate_uuid(&args, false);
 }
 
+static MigrationChannelList *uri_to_channels(const char *uri)
+{
+MigrationChannelList *channels = g_new0(MigrationChannelList, 1);
+MigrationChannel *val = g_new0(MigrationChannel, 1);
+MigrationAddress *addr = g_new0(MigrationAddress, 1);
+char **saddr;
+
+addr->transport = MIGRATION_ADDRESS_TYPE_SOCKET;
+
+saddr = g_strsplit((char *)uri, ":", 3);
+if (!saddr[0] || saddr[0] != g_strdup("tcp")) {
+fprintf(stderr, "%s: Invalid URI: %s\n", __func__, uri);
+}
+addr->u.socket.type = SOCKET_ADDRESS_TYPE_INET;
+addr->u.socket.u.inet.host = g_strdup(saddr[1]);
+addr->u.socket.u.inet.port = g_strdup(saddr[2]);
+g_strfreev(saddr);
+
+val->channel_type = MIGRATION_CHANNEL_TYPE_MAIN;
+val->addr = addr;
+channels->value = val;
+
+return channels;
+}
+
+static void do_test_validate_uri_channel(MigrateCommon *args, bool should_fail)
+{
+QTestState *from, *to;
+
+if (test_migrate_start(&from, &to, args->listen_uri, &args->start)) {
+return;
+}
+
+/* Wait for the first serial output from the source */
+wait_for_serial("src_serial");
+
+/*
+ * 'uri' and 'channels' validation is checked even before the migration
+ * starts.
+ */
+if (args->result == MIG_TEST_QMP_ERROR) {
+migrate_qmp_fail(from, 
+args->connect_uri ? args->connect_uri : NULL,
+args->connect_channels ? args->connect_channels : NULL, "{}");
+goto finish;
+}
+
+migrate_qmp(from,
+args->connect_uri ? args->connect_uri : NULL,
+args->connect_channels ? args->connect_channels : NULL, "{}");
+
+if (should_fail) {
+qtest_set_expected_status(to, EXIT_FAILURE);
+wait_for_migration_fail(from, false);
+} else {
+wait_for_migration_complete(from);
+}
+
+finish:
+test_migrate_end(from, to, args->result == MIG_TEST_SUCCEED);
+}
+
+static void
+test_validate_uri_channels_both_set(void)
+{
+g_autofree char *uri = g_strdup("tcp:127.0.0.1:0");
+
+MigrateCommon args = {
+.start = {
+.hide_stderr = true,
+},
+.connect_uri = uri,
+.connect_channels = uri_to_channels(uri),
+.listen_uri = "defer",
+.result = MIG_TEST_QMP_ERROR,
+};
+
+do_test_validate_uri_channel(&args, true);
+}
+
 /*
  * The way auto_converge works, we need to do too many passes to
  * run this test.  Auto_converge logic is only run once every
@@ -3544,6 +3625,8 @@ int main(int argc, char **argv)
test_validate_uuid_src_not_set);
 migration_test_add("/migration/validate_uuid_dst_not_set",
test_validate_uuid_dst_not_set);
+migration_test_add("/migration/validate_uri/channels/both_set",
+   test_validate_uri_channels_both_set);
 /*
  * See explanation why this test is slow on function definition
  */
-- 
2.22.3

[PATCH 1/3] qtest: migration: Enhance qtest migration functions to support 'channels' argument

[Het Gala]

Introduce support for adding a 'channels' argument to migrate_qmp_fail
and migrate_qmp functions within the migration qtest framework, enabling
enhanced control over migration scenarios.

Signed-off-by: Het Gala 
---
 tests/qtest/dbus-vmstate-test.c |  2 +-
 tests/qtest/migration-helpers.c | 93 ++---
 tests/qtest/migration-helpers.h | 11 ++--
 tests/qtest/migration-test.c| 33 ++--
 4 files changed, 112 insertions(+), 27 deletions(-)

diff --git a/tests/qtest/dbus-vmstate-test.c b/tests/qtest/dbus-vmstate-test.c
index 6c990864e3..0ca572e29b 100644
--- a/tests/qtest/dbus-vmstate-test.c
+++ b/tests/qtest/dbus-vmstate-test.c
@@ -229,7 +229,7 @@ test_dbus_vmstate(Test *test)
 
 thread = g_thread_new("dbus-vmstate-thread", dbus_vmstate_thread, loop);
 
-migrate_qmp(src_qemu, uri, "{}");
+migrate_qmp(src_qemu, uri, NULL, "{}");
 test->src_qemu = src_qemu;
 if (test->migrate_fail) {
 wait_for_migration_fail(src_qemu, true);
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index e451dbdbed..d153677887 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include "qemu/ctype.h"
 #include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qlist.h"
 
 #include "migration-helpers.h"
 
@@ -43,7 +44,70 @@ bool migrate_watch_for_events(QTestState *who, const char 
*name,
 return false;
 }
 
-void migrate_qmp_fail(QTestState *who, const char *uri, const char *fmt, ...)
+static char *socketAddressType_to_str(SocketAddressType type)
+{
+switch (type) {
+case SOCKET_ADDRESS_TYPE_INET:
+return g_strdup("inet");
+case SOCKET_ADDRESS_TYPE_UNIX:
+return g_strdup("unix");
+case SOCKET_ADDRESS_TYPE_FD:
+return g_strdup("fd");
+case SOCKET_ADDRESS_TYPE_VSOCK:
+return g_strdup("vsock");
+default:
+return g_strdup("unknown address type");
+}
+}
+
+static QList *MigrationChannelList_to_QList(MigrationChannelList *channels)
+{
+MigrationChannel *channel = NULL;
+MigrationAddress *addr = NULL;
+SocketAddress *saddr = NULL;
+g_autofree const char *addr_type = NULL;
+QList *channelList = qlist_new();
+QDict *channelDict = qdict_new();
+QDict *addrDict = qdict_new();
+
+channel = channels->value;
+if (!channel || channel->channel_type == MIGRATION_CHANNEL_TYPE__MAX) {
+fprintf(stderr, "%s: Channel or its type is NULL\n",
+__func__);
+}
+g_assert(channel);
+if (channel->channel_type == MIGRATION_CHANNEL_TYPE_MAIN) {
+qdict_put_str(channelDict, "channel-type", g_strdup("main"));
+}
+
+addr = channel->addr;
+if (!addr || addr->transport == MIGRATION_ADDRESS_TYPE__MAX) {
+fprintf(stderr, "%s: addr or its transport is NULL\n",
+__func__);
+}
+g_assert(addr);
+if (addr->transport == MIGRATION_ADDRESS_TYPE_SOCKET) {
+qdict_put_str(addrDict, "transport", g_strdup("socket"));
+}
+
+saddr = &addr->u.socket;
+if (!saddr) {
+fprintf(stderr, "%s: saddr is NULL\n", __func__);
+}
+g_assert(saddr);
+addr_type = socketAddressType_to_str(saddr->type);
+qdict_put_str(addrDict, "type", addr_type);
+qdict_put_str(addrDict, "port", saddr->u.inet.port);
+qdict_put_str(addrDict, "host", saddr->u.inet.host);
+
+qdict_put_obj(channelDict, "addr", QOBJECT(addrDict));
+qlist_append_obj(channelList, QOBJECT(channelDict));
+
+return channelList;
+}
+
+void migrate_qmp_fail(QTestState *who, const char *uri,
+  MigrationChannelList *channels, const char *fmt, ...)
 {
 va_list ap;
 QDict *args, *err;
@@ -52,8 +116,16 @@ void migrate_qmp_fail(QTestState *who, const char *uri, 
const char *fmt, ...)
 args = qdict_from_vjsonf_nofail(fmt, ap);
 va_end(ap);
 
-g_assert(!qdict_haskey(args, "uri"));
-qdict_put_str(args, "uri", uri);
+if (uri) {
+g_assert(!qdict_haskey(args, "uri"));
+qdict_put_str(args, "uri", uri);
+}
+
+if (channels) {
+g_assert(!qdict_haskey(args, "channels"));
+QList *channelList = MigrationChannelList_to_QList(channels);
+qdict_put_obj(args, "channels", QOBJECT(channelList));
+}
 
 err = qtest_qmp_assert_failure_ref(
 who, "{ 'execute': 'migrate', 'arguments': %p}", args);
@@ -68,7 +140,8 @@ void migrate_qmp_fail(QTestState *who, const char *uri, 
const char *fmt, ...)
  * Arguments are built from @fmt... (formatted like
  * qobject_from_jsonf_nofail()) with "uri": @uri spliced in.
  */
-void migrate_qmp(QTestState *who, const char *uri, const char *fmt, ...)
+void migrate_qmp(QTestState *who, const char *uri,
+ MigrationChannelList *channels, const char *fmt, ...)
 {
 va_list ap;
 QDict *args;
@@ -77,8 +150,16 @@ void migrate_qmp(QTestState *who, const char *uri, const 
char *fmt, ...)
 args = qdict_from

[PATCH 0/3] qtest: migration: Add validation tests for 'channels' argument in migrate QAPIs

[Het Gala]

With recent migrate QAPI changes enabling direct use of the 'channels'
argument, avoiding redundant string parsing of the URI is achieved.

To ensure backward compatibility, both 'uri' and 'channels' arguments are
now optional in migration QMP commands. However, they are mutually exclusive,
requiring at least one for a successful migration connection.

Hence, validating 'uri' and 'channels' becomes crucial to prevent their
simultaneous presence in migrate QAPIs.

Patch Summary:
1. Introduce migrate_qmp() and migrate_qmp_fail() with 'channels' arguments
   and a conversion function from MigrationChannelList to QList.
2. Add a new field in the MigrateCommon struct to support the 'channels'
   argument during migration.
3. Include negative validation tests to disallow both arguments in migration
   QAPIs.

Het Gala (3):
  qtest: migration: Enhance qtest migration functions to support
'channels' argument
  qtest: migration: Introduce 'connect_channels' in MigrateCommon struct
  qtest: migration: Add negative validation test for 'uri' and
'channels' both set

 tests/qtest/dbus-vmstate-test.c |   2 +-
 tests/qtest/migration-helpers.c |  93 ++--
 tests/qtest/migration-helpers.h |  11 +--
 tests/qtest/migration-test.c| 123 +++-
 4 files changed, 202 insertions(+), 27 deletions(-)

-- 
2.22.3

[PATCH 2/3] qtest: migration: Introduce 'connect_channels' in MigrateCommon struct

[Het Gala]

migration QAPIs can now work with either 'channels' or 'uri' as their
argument.

Signed-off-by: Het Gala 
---
 tests/qtest/migration-test.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index e7f2719dcf..0bc69b1943 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -704,6 +704,13 @@ typedef struct {
  */
 const char *connect_uri;
 
+/*
+ * Optional: list of migration stream channels, each connected
+ * to a dst QEMU. It can be used instead of URI to carry out
+ * the same task as listen_uri or connect_uri.
+ */
+MigrationChannelList *connect_channels;
+
 /* Optional: callback to run at start to set migration parameters */
 TestMigrateStartHook start_hook;
 /* Optional: callback to run at finish to cleanup */
-- 
2.22.3

[PATCH v2] 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 
---
 v2: Keep "select SERIAL" instead of replacing it

 hw/hppa/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

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

Re: [PATCH 5/6] hw/vfio/common: Use RCU_READ macros

[Cédric Le Goater]

On 2/16/24 09:49, Philippe Mathieu-Daudé wrote:

On 24/1/24 15:09, Philippe Mathieu-Daudé wrote:

On 24/1/24 10:25, Manos Pitsidianakis wrote:

On Wed, 24 Jan 2024 09:42, Philippe Mathieu-Daudé  wrote:

Replace the manual rcu_read_(un)lock calls by the
*RCU_READ_LOCK_GUARD macros (See commit ef46ae67ba
"docs/style: call out the use of GUARD macros").

Signed-off-by: Philippe Mathieu-Daudé 
---
hw/vfio/common.c | 34 --
1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 4aa86f563c..09878a3603 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -308,13 +308,13 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, 
IOMMUTLBEntry *iotlb)
    return;
    }

-    rcu_read_lock();
+    RCU_READ_LOCK_GUARD();

    if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
    bool read_only;

    if (!vfio_get_xlat_addr(iotlb, &vaddr, NULL, &read_only)) {
-    goto out;
+    return;


Since this is the only early return, we could alternatively do:

- if (!vfio_get_xlat_addr(iotlb, &vaddr, NULL, &read_only)) {
+ if (vfio_get_xlat_addr(iotlb, &vaddr, NULL, &read_only)) {

remove the goto/return, and wrap the rest of the codeflow in this if's 
brackets. And then we could use WITH_RCU_READ_LOCK_GUARD instead. That'd 
increase the code indentation however.


If the maintainer agrees with the style & code churn, I don't
mind respining.


Alex, Cédric, any preference?


my choice would be to keep the 'goto' statement and protect
the vfio_get_xlat_addr() call with :

+WITH_RCU_READ_LOCK_GUARD() {
+if (vfio_get_xlat_addr(iotlb, NULL, &translated_addr, NULL)) {
+ret = vfio_get_dirty_bitmap(bcontainer, iova,
+iotlb->addr_mask + 1,
+translated_addr);
+if (ret) {
+error_report("vfio_iommu_map_dirty_notify(%p,"
+ " 0x%"HWADDR_PRIx
+ ", 0x%"HWADDR_PRIx") = %d (%s)",
+ bcontainer, iova, iotlb->addr_mask + 1, ret,
+ strerror(-ret));
+}
+}
 }



Thanks,

C.

RE: [PATCH v2 1/3] hw/cxl/cxl-mailbox-utils: Add support for feature commands (8.2.9.6)

[Shiju Jose via]

Hi Fan,

Thanks for the feedbacks.

>-Original Message-
>From: fan 
>Sent: 15 February 2024 18:06
>To: Shiju Jose 
>Cc: qemu-devel@nongnu.org; linux-...@vger.kernel.org; Jonathan Cameron
>; tanxiaofei ;
>Zengtao (B) ; Linuxarm 
>Subject: Re: [PATCH v2 1/3] hw/cxl/cxl-mailbox-utils: Add support for feature
>commands (8.2.9.6)
>
>On Fri, Nov 24, 2023 at 09:53:35PM +0800, shiju.j...@huawei.com wrote:
>> From: Shiju Jose 
>>
>> CXL spec 3.0 section 8.2.9.6 describes optional device specific features.
>> CXL devices supports features with changeable attributes.
>> Get Supported Features retrieves the list of supported device specific
>> features. The settings of a feature can be retrieved using Get Feature
>> and optionally modified using Set Feature.
>>
>> Reviewed-by: Davidlohr Bueso 
>> Signed-off-by: Shiju Jose 
>> ---
>
>Updated the references to align with cxl spec r3.1, other than that looks good 
>to
>me.

I had posted v3 of this series updated for spec r3.1. Please find here,
https://lore.kernel.org/qemu-devel/20240215110146.1444-1-shiju.j...@huawei.com/T/#t

>
>Fan
>
>>  hw/cxl/cxl-mailbox-utils.c | 167
>> +
>>  1 file changed, 167 insertions(+)
>>
>> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>> index 6184f44339..1bbc9a48a6 100644
>> --- a/hw/cxl/cxl-mailbox-utils.c
>> +++ b/hw/cxl/cxl-mailbox-utils.c
>> @@ -66,6 +66,10 @@ enum {
>>  LOGS= 0x04,
>>  #define GET_SUPPORTED 0x0
>>  #define GET_LOG   0x1
>> +FEATURES= 0x05,
>> +#define GET_SUPPORTED 0x0
>> +#define GET_FEATURE   0x1
>> +#define SET_FEATURE   0x2
>>  IDENTIFY= 0x40,
>>  #define MEMORY_DEVICE 0x0
>>  CCLS= 0x41,
>> @@ -785,6 +789,157 @@ static CXLRetCode cmd_logs_get_log(const struct
>cxl_cmd *cmd,
>>  return CXL_MBOX_SUCCESS;
>>  }
>>
>> +/* CXL r3.0 section 8.2.9.6: Features */ typedef struct
>> +CXLSupportedFeatureHeader {
>> +uint16_t entries;
>> +uint16_t nsuppfeats_dev;
>> +uint32_t reserved;
>> +} QEMU_PACKED CXLSupportedFeatureHeader;
>> +
>> +typedef struct CXLSupportedFeatureEntry {
>> +QemuUUID uuid;
>> +uint16_t feat_index;
>> +uint16_t get_feat_size;
>> +uint16_t set_feat_size;
>> +uint32_t attrb_flags;
>> +uint8_t get_feat_version;
>> +uint8_t set_feat_version;
>> +uint16_t set_feat_effects;
>> +uint8_t rsvd[18];
>> +} QEMU_PACKED CXLSupportedFeatureEntry;
>> +
>> +enum CXL_SUPPORTED_FEATURES_LIST {
>> +CXL_FEATURE_MAX
>> +};
>> +
>> +/* Get Feature CXL 3.0 Spec 8.2.9.6.2 */
>> +/*
>> + * Get Feature input payload
>> + * CXL rev 3.0 section 8.2.9.6.2; Table 8-79  */
>> +/* Get Feature : Payload in selection */ enum
>> +CXL_GET_FEATURE_SELECTION {
>> +CXL_GET_FEATURE_SEL_CURRENT_VALUE = 0x0,
>> +CXL_GET_FEATURE_SEL_DEFAULT_VALUE = 0x1,
>> +CXL_GET_FEATURE_SEL_SAVED_VALUE = 0x2,
>> +CXL_GET_FEATURE_SEL_MAX
>> +};
>> +
>> +/* Set Feature CXL 3.0 Spec 8.2.9.6.3 */
>> +/*
>> + * Set Feature input payload
>> + * CXL rev 3.0 section 8.2.9.6.3; Table 8-81  */ typedef struct
>> +CXLSetFeatureInHeader {
>> +QemuUUID uuid;
>> +uint32_t flags;
>> +uint16_t offset;
>> +uint8_t version;
>> +uint8_t rsvd[9];
>> +} QEMU_PACKED QEMU_ALIGNED(16) CXLSetFeatureInHeader;
>> +
>> +/* Set Feature : Payload in flags */
>> +#define CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MASK   0x7
>> +enum CXL_SET_FEATURE_FLAG_DATA_TRANSFER {
>> +CXL_SET_FEATURE_FLAG_FULL_DATA_TRANSFER = 0x0,
>> +CXL_SET_FEATURE_FLAG_INITIATE_DATA_TRANSFER = 0x1,
>> +CXL_SET_FEATURE_FLAG_CONTINUE_DATA_TRANSFER = 0x2,
>> +CXL_SET_FEATURE_FLAG_FINISH_DATA_TRANSFER = 0x3,
>> +CXL_SET_FEATURE_FLAG_ABORT_DATA_TRANSFER = 0x4,
>> +CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MAX
>> +};
>> +
>> +/* CXL r3.0 section 8.2.9.6.1: Get Supported Features (Opcode 0500h)
>> +*/ static CXLRetCode cmd_features_get_supported(const struct cxl_cmd
>*cmd,
>> + uint8_t *payload_in,
>> + size_t len_in,
>> + uint8_t *payload_out,
>> + size_t *len_out,
>> + CXLCCI *cci) {
>> +struct {
>> +uint32_t count;
>> +uint16_t start_index;
>> +uint16_t reserved;
>> +} QEMU_PACKED QEMU_ALIGNED(16) * get_feats_in = (void
>> +*)payload_in;
>> +
>> +struct {
>> +CXLSupportedFeatureHeader hdr;
>> +CXLSupportedFeatureEntry feat_entries[];
>> +} QEMU_PACKED QEMU_ALIGNED(16) * get_feats_out = (void
>*)payload_out;
>> +uint16_t index;
>> +uint16_t entry, req_entries;
>> +uint16_t feat_entries = 0;
>> +
>> +if (get_feats_in->count < sizeof(CXLSupportedFeatureHeader) ||
>> +get_feats_in->start_index > CXL_FEATURE_MAX) {
>> +return CXL_MBOX_INVALID_I

RE: [PATCH v2 0/3] hw/cxl/cxl-mailbox-utils: Add feature commands, device patrol scrub control and DDR5 ECS control features

[Shiju Jose via]

Hi Fan,

>-Original Message-
>From: fan 
>Sent: 15 February 2024 18:09
>To: Shiju Jose 
>Cc: qemu-devel@nongnu.org; linux-...@vger.kernel.org; Jonathan Cameron
>; tanxiaofei ;
>Zengtao (B) ; Linuxarm 
>Subject: Re: [PATCH v2 0/3] hw/cxl/cxl-mailbox-utils: Add feature commands,
>device patrol scrub control and DDR5 ECS control features
>
>On Fri, Nov 24, 2023 at 09:53:34PM +0800, shiju.j...@huawei.com wrote:
>> From: Shiju Jose 
>>
>> Add support for the feature commands, device patrol scrub control and
>> DDR5 ECS control features.
>>
>> CXL spec 3.0 section 8.2.9.6 describes optional device specific features.
>> CXL spec 3.1 section 8.2.9.9.11.1 describes the device patrol scrub
>> control feature.
>> CXL spec 3.1 section 8.2.9.9.11.2 describes the DDR5 Error Check Scrub
>> (ECS) control feature.
>>
>> The patches are available here,
>> https://gitlab.com/shiju.jose/qemu/-/tree/cxl-scrub-2023-11-14
>> and is based on Jonathan's branch
>> https://gitlab.com/jic23/qemu/-/tree/cxl-2023-10-16
>>
>> Changes
>> v1 -> v2
>> 1. Changes for Davidlohr comments. Thanks.
>>  - Changed CXL SET feature data transfer flags as enum.
>>  - Modified pointer supported_feats to get_feats_out.
>>  - Removed an unnecessary branch.
>>  - Use MIN().
>>  - Move setting of hdr.nsuppfeats further down.
>>  - Return CXL_MBOX_UNSUPPORTED if non-zero selection flag is passed.
>>  - Add more IMMEDIATE_*.* flags set_feature.
>>  - Corrected a spelling error.
>>
>> Shiju Jose (3):
>>   hw/cxl/cxl-mailbox-utils: Add support for feature commands (8.2.9.6)
>>   hw/cxl/cxl-mailbox-utils: Add device patrol scrub control feature
>>   hw/cxl/cxl-mailbox-utils: Add device DDR5 ECS control feature
>>
>>  hw/cxl/cxl-mailbox-utils.c | 360
>> +
>>  1 file changed, 360 insertions(+)
>>
>> --
>> 2.34.1
>>
>
>Recently, Jonathan has updated all the specification references to align
>with cxl spec r3.1, so for the next version, we may want to also do
>that.

I had posted recently v3 of this series updated for spec r3.1. Please find,
https://lore.kernel.org/qemu-devel/20240215110146.1444-1-shiju.j...@huawei.com/T/#t

>
>Fan

Thanks,
Shiju

Re: [RFC PATCH v4 10/10] util/bufferiszero: Add sve acceleration for aarch64

[Alex Bennée]

Richard Henderson  writes:

> Signed-off-by: Richard Henderson 
> ---
>
> RFC because I've not benchmarked this on real hw, only run it
> through qemu for validation.

I think we have an a64fx is the TCWG lab you could probably run the
tests on if you want. Otherwise I might be able to spin up a Graviton
on AWS to run a measurement. Do we have a benchmark test to run?

>
> ---
>  host/include/aarch64/host/cpuinfo.h |  1 +
>  util/bufferiszero.c | 49 +
>  util/cpuinfo-aarch64.c  |  1 +
>  meson.build | 13 
>  4 files changed, 64 insertions(+)
>
> diff --git a/host/include/aarch64/host/cpuinfo.h 
> b/host/include/aarch64/host/cpuinfo.h
> index fe671534e4..b4b816cd07 100644
> --- a/host/include/aarch64/host/cpuinfo.h
> +++ b/host/include/aarch64/host/cpuinfo.h
> @@ -12,6 +12,7 @@
>  #define CPUINFO_AES (1u << 3)
>  #define CPUINFO_PMULL   (1u << 4)
>  #define CPUINFO_BTI (1u << 5)
> +#define CPUINFO_SVE (1u << 6)
>  
>  /* Initialized with a constructor. */
>  extern unsigned cpuinfo;
> diff --git a/util/bufferiszero.c b/util/bufferiszero.c
> index 2809b09225..af64c9c224 100644
> --- a/util/bufferiszero.c
> +++ b/util/bufferiszero.c
> @@ -270,13 +270,62 @@ static bool buffer_is_zero_simd(const void *buf, size_t 
> len)
>  return vaddvq_u32(vceqzq_u32(t0)) == -4;
>  }
>  
> +#ifdef CONFIG_SVE_OPT
> +#include 
> +
> +#ifndef __ARM_FEATURE_SVE
> +__attribute__((target("+sve")))
> +#endif
> +static bool buffer_is_zero_sve(const void *buf, size_t len)
> +{
> +svbool_t p, t = svptrue_b8();
> +size_t i, n;
> +
> +/*
> + * For the first vector, align to 16 -- reading 1 to 256 bytes.
> + * Note this routine is only called with len >= 256, which is the
> + * architectural maximum vector length: the first vector always fits.
> + */
> +i = 0;
> +n = QEMU_ALIGN_PTR_DOWN(buf + svcntb(), 16) - buf;
> +p = svwhilelt_b8(i, n);
> +
> +do {
> +svuint8_t d = svld1_u8(p, buf + i);
> +
> +p = svcmpne_n_u8(t, d, 0);
> +if (unlikely(svptest_any(t, p))) {
> +return false;
> +}
> +i += n;
> +n = svcntb();
> +p = svwhilelt_b8(i, len);
> +} while (svptest_any(t, p));
> +
> +return true;
> +}
> +#endif /* CONFIG_SVE_OPT */
> +
>  static biz_accel_fn const accel_table[] = {
>  buffer_is_zero_int_ge256,
>  buffer_is_zero_simd,
> +#ifdef CONFIG_SVE_OPT
> +buffer_is_zero_sve,
> +#endif
>  };
>  
> +#ifdef CONFIG_SVE_OPT
> +static unsigned accel_index;
> +static void __attribute__((constructor)) init_accel(void)
> +{
> +accel_index = (cpuinfo & CPUINFO_SVE ? 2 : 1);
> +buffer_is_zero_accel = accel_table[accel_index];
> +}
> +#define INIT_ACCEL NULL
> +#else
>  static unsigned accel_index = 1;
>  #define INIT_ACCEL buffer_is_zero_simd
> +#endif /* CONFIG_SVE_OPT */
>  
>  bool test_buffer_is_zero_next_accel(void)
>  {
> diff --git a/util/cpuinfo-aarch64.c b/util/cpuinfo-aarch64.c
> index 4c8a005715..a1e22ea66e 100644
> --- a/util/cpuinfo-aarch64.c
> +++ b/util/cpuinfo-aarch64.c
> @@ -61,6 +61,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
>  info |= (hwcap & HWCAP_USCAT ? CPUINFO_LSE2 : 0);
>  info |= (hwcap & HWCAP_AES ? CPUINFO_AES : 0);
>  info |= (hwcap & HWCAP_PMULL ? CPUINFO_PMULL : 0);
> +info |= (hwcap & HWCAP_SVE ? CPUINFO_SVE : 0);
>  
>  unsigned long hwcap2 = qemu_getauxval(AT_HWCAP2);
>  info |= (hwcap2 & HWCAP2_BTI ? CPUINFO_BTI : 0);
> diff --git a/meson.build b/meson.build
> index c1dc83e4c0..89a8241bc0 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2822,6 +2822,18 @@ config_host_data.set('CONFIG_ARM_AES_BUILTIN', 
> cc.compiles('''
>  void foo(uint8x16_t *p) { *p = vaesmcq_u8(*p); }
>'''))
>  
> +config_host_data.set('CONFIG_SVE_OPT', cc.compiles('''
> +#include 
> +#ifndef __ARM_FEATURE_SVE
> +__attribute__((target("+sve")))
> +#endif
> +void foo(void *p) {
> +svbool_t t = svptrue_b8();
> +svuint8_t d = svld1_u8(t, p);
> +svptest_any(t, svcmpne_n_u8(t, d, 0));
> +}
> +  '''))
> +
>  have_pvrdma = get_option('pvrdma') \
>.require(rdma.found(), error_message: 'PVRDMA requires OpenFabrics 
> libraries') \
>.require(cc.compiles(gnu_source_prefix + '''
> @@ -4232,6 +4244,7 @@ summary_info += {'memory allocator':  
> get_option('malloc')}
>  summary_info += {'avx2 optimization': 
> config_host_data.get('CONFIG_AVX2_OPT')}
>  summary_info += {'avx512bw optimization': 
> config_host_data.get('CONFIG_AVX512BW_OPT')}
>  summary_info += {'avx512f optimization': 
> config_host_data.get('CONFIG_AVX512F_OPT')}
> +summary_info += {'sve optimization': config_host_data.get('CONFIG_SVE_OPT')}
>  summary_info += {'gcov':  get_option('b_coverage')}
>  summary_info += {'thread sanitizer':  get_option('tsan')}
>  summary_info += {'CFI support':   get_opt

[PATCH] iotests: adapt to output change for recently introduced 'detached header' field

[Fiona Ebner]

Failure was noticed when running the tests for the qcow2 image format.

Fixes: 0bd779e27e ("crypto: Introduce 'detached-header' field in 
QCryptoBlockInfoLUKS")
Signed-off-by: Fiona Ebner 
---
 tests/qemu-iotests/198.out | 2 ++
 tests/qemu-iotests/206.out | 1 +
 2 files changed, 3 insertions(+)

diff --git a/tests/qemu-iotests/198.out b/tests/qemu-iotests/198.out
index 805494916f..62fb73fa3e 100644
--- a/tests/qemu-iotests/198.out
+++ b/tests/qemu-iotests/198.out
@@ -39,6 +39,7 @@ Format specific information:
 compression type: COMPRESSION_TYPE
 encrypt:
 ivgen alg: plain64
+detached header: false
 hash alg: sha256
 cipher alg: aes-256
 uuid: ----
@@ -84,6 +85,7 @@ Format specific information:
 compression type: COMPRESSION_TYPE
 encrypt:
 ivgen alg: plain64
+detached header: false
 hash alg: sha256
 cipher alg: aes-256
 uuid: ----
diff --git a/tests/qemu-iotests/206.out b/tests/qemu-iotests/206.out
index 7e95694777..979f00f9bf 100644
--- a/tests/qemu-iotests/206.out
+++ b/tests/qemu-iotests/206.out
@@ -114,6 +114,7 @@ Format specific information:
 refcount bits: 16
 encrypt:
 ivgen alg: plain64
+detached header: false
 hash alg: sha1
 cipher alg: aes-128
 uuid: ----
-- 
2.39.2

Re: [PATCH] docs/system/ppc: Document running Linux on AmigaNG machines

[Thomas Huth]

On 16/02/2024 01.10, BALATON Zoltan wrote:

Documentation on how to run Linux on the amigaone, pegasos2 and
sam460ex machines is currently burried in the depths of the qemu-devel


s/burried/buried/


mailing list and in the source code. Let's collect the information in
the QEMU handbook for a one stop solution.

Co-authored-by: Bernhard Beschow 
Signed-off-by: BALATON Zoltan 
---
Supersedes: <20231216123013.67978-1-shen...@gmail.com>

  MAINTAINERS |   1 +
  docs/system/ppc/amigang.rst | 160 
  docs/system/target-ppc.rst  |   1 +
  3 files changed, 162 insertions(+)
  create mode 100644 docs/system/ppc/amigang.rst

diff --git a/MAINTAINERS b/MAINTAINERS
index a24c2b51b6..8e5b47e7b4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1560,6 +1560,7 @@ F: hw/rtc/m41t80.c
  F: pc-bios/canyonlands.dt[sb]
  F: pc-bios/u-boot-sam460ex-20100605.bin
  F: roms/u-boot-sam460ex
+F: docs/system/ppc/amigang.rst
  
  pegasos2

  M: BALATON Zoltan 
diff --git a/docs/system/ppc/amigang.rst b/docs/system/ppc/amigang.rst
new file mode 100644
index 00..c03a7e0d66
--- /dev/null
+++ b/docs/system/ppc/amigang.rst
@@ -0,0 +1,160 @@
+AmigaNG boards (``amigaone``, ``pegasos2``, ``sam460ex``)
+=
+
+These PowerPC machines emulate boards that are primarily used for
+running Amiga like OSes (AmigaOS 4, MorphOS and AROS) but these can
+also run Linux which is what this section documents.
+
+Eyetech AmigaOne/Mai Logic Teron (``amigaone``)
+===


Wouldn't it be better to use a subsection here (i.e. "" instead of 
"") ? And also adjust the subsections below to subsubsections?



+
+The ``amigaone`` machine emulates an AmigaOne XE mainboard by Eyetech
+which is a rebranded Mai Logic Teron board with modified U-Boot
+firmware to support AmigaOS 4.
+
+Emulated devices
+
+
+ * PowerPC 7457 CPU (can also use``-cpu g3, 750cxe, 750fx`` or ``750gx``)
+ * Articia S north bridge
+ * VIA VT82C686B south bridge
+ * PCI VGA compatible card (guests may need other card instead)
+ * PS/2 keyboard and mouse
+
+Firmware
+
+
+A firmware binary is necessary for the boot process. It is a modified
+U-Boot under GPL but its source is lost so it cannot be included in
+QEMU. A binary is available at
+https://www.hyperion-entertainment.com/index.php/downloads?view=files&parent=28.
+The ROM image is in the last 512kB which can be extracted with the
+following command:
+
+.. code-block:: bash
+
+  $ tail -c 524288 updater.image > u-boot-amigaone.bin
+
+The BIOS emulator in the firmware is unable to run QEMU‘s standard
+vgabios so ``VGABIOS-lgpl-latest.bin`` is needed instead which can be
+downloaded from http://www.nongnu.org/vgabios.
+
+Running Linux
+-
+
+There are some Linux images under the following link that work on the
+``amigaone`` machine:
+https://sourceforge.net/projects/amigaone-linux/files/debian-installer/.
+To boot the system run:
+
+.. code-block:: bash
+
+  $ qemu-system-ppc -machine amigaone -bios u-boot-amigaone.bin \
+-cdrom "A1 Linux Net Installer.iso" \
+-device ati-vga,model=rv100,romfile=VGABIOS-lgpl-latest.bin
+
+From the firmware menu that appears select ``Boot sequence`` →
+``Amiga Multiboot Options`` and set ``Boot device 1`` to
+``Onboard VIA IDE CDROM``. Then hit escape until the main screen appears again,
+hit escape once more and from the exit menu that appears select either
+``Save settings and exit`` or ``Use settings for this session only``. It may
+take a long time loading the kernel into memory but eventually it boots and the
+installer becomes visible. The ``ati-vga`` RV100 emulation is not
+complete yet so only frame buffer works, DRM and 3D is not available.

...

 Thomas

RE: [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub control feature

[Shiju Jose via]

Hi Fan,

>-Original Message-
>From: fan 
>Sent: 15 February 2024 18:47
>To: Shiju Jose 
>Cc: qemu-devel@nongnu.org; linux-...@vger.kernel.org; Jonathan Cameron
>; tanxiaofei ;
>Zengtao (B) ; Linuxarm ;
>fan...@samsung.com
>Subject: Re: [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub
>control feature
>
>On Fri, Nov 24, 2023 at 09:53:36PM +0800, shiju.j...@huawei.com wrote:
>> From: Shiju Jose 
>>
>> CXL spec 3.1 section 8.2.9.9.11.1 describes the device patrol scrub
>> control feature. The device patrol scrub proactively locates and makes
>> corrections to errors in regular cycle. The patrol scrub control
>> allows the request to configure patrol scrub input configurations.
>>
>> The patrol scrub control allows the requester to specify the number of
>> hours for which the patrol scrub cycles must be completed, provided
>> that the requested number is not less than the minimum number of hours
>> for the patrol scrub cycle that the device is capable of. In addition,
>> the patrol scrub controls allow the host to disable and enable the
>> feature in case disabling of the feature is needed for other purposes
>> such as performance-aware operations which require the background
>> operations to be turned off.
>>
>> Reviewed-by: Davidlohr Bueso 
>> Signed-off-by: Shiju Jose 
>> ---
>
>LGTM except for some minor comments inlined.
>
>
>>  hw/cxl/cxl-mailbox-utils.c | 97
>> +-
>>  1 file changed, 96 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>> index 1bbc9a48a6..5a6f4e4029 100644
>> --- a/hw/cxl/cxl-mailbox-utils.c
>> +++ b/hw/cxl/cxl-mailbox-utils.c
>> @@ -809,6 +809,7 @@ typedef struct CXLSupportedFeatureEntry {  }
>> QEMU_PACKED CXLSupportedFeatureEntry;
>>
>>  enum CXL_SUPPORTED_FEATURES_LIST {
>> +CXL_FEATURE_PATROL_SCRUB = 0,
>>  CXL_FEATURE_MAX
>>  };
>>
>> @@ -849,6 +850,37 @@ enum CXL_SET_FEATURE_FLAG_DATA_TRANSFER {
>>  CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MAX
>>  };
>>
>> +/* CXL r3.1 section 8.2.9.9.11.1: Device Patrol Scrub Control Feature
>> +*/ static const QemuUUID patrol_scrub_uuid = {
>> +.data = UUID(0x96dad7d6, 0xfde8, 0x482b, 0xa7, 0x33,
>> + 0x75, 0x77, 0x4e, 0x06, 0xdb, 0x8a) };
>> +
>> +#define CXL_MEMDEV_PS_GET_FEATURE_VERSION0x01
>> +#define CXL_MEMDEV_PS_SET_FEATURE_VERSION0x01
>> +#define CXL_MEMDEV_PS_SCRUB_CYCLE_CHANGE_CAP_DEFAULTBIT(0)
>> +#define CXL_MEMDEV_PS_SCRUB_REALTIME_REPORT_CAP_DEFAULT
>BIT(1)
>> +#define CXL_MEMDEV_PS_CUR_SCRUB_CYCLE_DEFAULT12
>> +#define CXL_MEMDEV_PS_MIN_SCRUB_CYCLE_DEFAULT1
>> +#define CXL_MEMDEV_PS_ENABLE_DEFAULT0
>> +
>> +/* CXL memdev patrol scrub control attributes */ struct
>> +CXLMemPatrolScrubReadAttrbs {
>> +uint8_t scrub_cycle_cap;
>> +uint16_t scrub_cycle;
>> +uint8_t scrub_flags;
>> +} QEMU_PACKED cxl_memdev_ps_feat_read_attrbs;
>> +
>> +typedef struct CXLMemPatrolScrubWriteAttrbs {
>> +uint8_t scrub_cycle_hr;
>> +uint8_t scrub_flags;
>> +} QEMU_PACKED CXLMemPatrolScrubWriteAttrbs;
>> +
>> +typedef struct CXLMemPatrolScrubSetFeature {
>> +CXLSetFeatureInHeader hdr;
>> +CXLMemPatrolScrubWriteAttrbs feat_data; } QEMU_PACKED
>> +QEMU_ALIGNED(16) CXLMemPatrolScrubSetFeature;
>> +
>>  /* CXL r3.0 section 8.2.9.6.1: Get Supported Features (Opcode 0500h)
>> */  static CXLRetCode cmd_features_get_supported(const struct cxl_cmd
>*cmd,
>>   uint8_t *payload_in, @@
>> -872,7 +904,7 @@ static CXLRetCode cmd_features_get_supported(const
>struct cxl_cmd *cmd,
>>  uint16_t feat_entries = 0;
>>
>>  if (get_feats_in->count < sizeof(CXLSupportedFeatureHeader) ||
>> -get_feats_in->start_index > CXL_FEATURE_MAX) {
>> +get_feats_in->start_index >= CXL_FEATURE_MAX) {
>
>Not totally sure about this, the spec says "...Greater than..." although I 
>also think
>it should be >=. Similar things for the offset usage below.

Spec r3.1 described in Table 8-95. Get Supported Features Input Payload  as , 
"Starting Feature Index: Index of the first requested Supported Feature Entry.
Feature index is a zero-based value."
Thus I believe get_feats_in->start_index >= CXL_FEATURE_MAX  is correct  because
the feature index is zero-based value.

Regarding the offset usage mentioned, can you point which code?
Is it get_feature->offset? 

>
>Fan
>
>>  return CXL_MBOX_INVALID_INPUT;
>>  }
>>  req_entries = (get_feats_in->count - @@ -884,6 +916,31 @@ static
>> CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
>>  entry = 0;
>>  while (entry < req_entries) {
>>  switch (index) {
>> +case  CXL_FEATURE_PATROL_SCRUB:
>> +/* Fill supported feature entry for device patrol scrub control 
>> */
>> +get_feats_out->feat_entries[entry] =
>> +   (struct CXLSupportedFeatureEntry) {
>> +.uuid = patro

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

[Philippe Mathieu-Daudé]

On 16/2/24 10:16, 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 
---
  v2: Keep "select SERIAL" instead of replacing it


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 1/6] libqos/virtio.c: init all elems in qvring_indirect_desc_setup()

[Thomas Huth]

On 13/02/2024 20.17, Daniel Henrique Barboza wrote:

The loop isn't setting the values for the last element. Every other
element is being initialized with addr = 0, flags = VRING_DESC_F_NEXT
and next = i + 1. The last elem is never touched.

This became a problem when enabling a RISC-V 'virt' libqos machine in
the 'indirect' test of virti-blk-test.c. The 'flags' for the last
element will end up being an odd number (since we didn't touch it).
Being an odd number it will be mistaken by VRING_DESC_F_NEXT, which
happens to be 1.

Deep into hw/virt/virtio.c, in virtqueue_split_pop(), into
virtqueue_split_read_next_desc(), a check for VRING_DESC_F_NEXT will be
made to see if we're supposed to chain. The code will keep up chaining
in the last element because the unintialized value happens to be odd.


s/unintialized/uninitialized/


We'll error out right after that because desc->next (which is also
uninitialized) will be >= max. A VIRTQUEUE_READ_DESC_ERROR will be
returned, with an error message like this in the stderr:

qemu-system-riscv64: Desc next is 49391

Since we never returned, w'll end up timing out at qvirtio_wait_used_elem():


s/w'll/we'll/


ERROR:../tests/qtest/libqos/virtio.c:236:qvirtio_wait_used_elem:
 assertion failed: (g_get_monotonic_time() - start_time <= timeout_us)

The root cause is using unintialized values from guest_alloc() in


s/unintialized/uninitialized/

With the typos fixed:
Reviewed-by: Thomas Huth 

Re: [PATCH v4 0/3] Initial support for SPDM Responders

[Alistair Francis]

On Fri, Feb 16, 2024 at 6:52 PM Klaus Jensen  wrote:
>
> On Feb 15 14:44, Jonathan Cameron wrote:
> > On Tue, 13 Feb 2024 12:44:00 +1000
> > Alistair Francis  wrote:
> >
> > Hi All,
> >
> > Just wanted to add that back in v2 Klaus Jensen stated:
> >
> > "I have no problem with picking this up for nvme, but I'd rather not take
> >  the full series through my tree without reviews/acks from the pci
> >  maintainers."
> >
> > So I'd like to add my request that Michael and/or Marcell takes a look
> > when they have time.
> >
> > I've been carrying more or less the first 2 patches in my CXL staging
> > tree for a couple of years (the initial Linux Kernel support that Lukas
> > Wunner is now handling was developed against this) and I would love
> > to see this upstream. Along with PCI and CXL and NVME usecases this
> > is a major part of the Confidential Compute device assignment story
> > via PCI/TDISP and CXL equivalent.
> >
> > It's not changed in significant ways since v2 back in October last year.
> >
>
> Would someone be willing to sign up to maintain the spdm_socket backend?

I'm happy to and should have already edited MAINTAINERS to indicate
that. I'll fix that up in the next version

Alistair

Re: [PATCH 2/6] libqos/virtio.c: fix 'avail_event' offset in qvring_init()

[Thomas Huth]

On 13/02/2024 20.17, Daniel Henrique Barboza wrote:

In qvring_init() we're writing vq->used->avail_event at "vq->used + 2 +
array_size".  The struct pointed by vq->used is, from virtio_ring.h
Linux header):

  * // A ring of used descriptor heads with free-running index.
  * __virtio16 used_flags;
  * __virtio16 used_idx;
  * struct vring_used_elem used[num];
  * __virtio16 avail_event_idx;

So 'flags' is the word right at vq->used. 'idx' is vq->used + 2. We need
to skip 'used_idx' by adding + 2 bytes, and then sum the vector size, to
reach avail_event_idx. An example on how to properly access this field
can be found in qvirtqueue_kick():

avail_event = qvirtio_readw(d, qts, vq->used + 4 +
 sizeof(struct vring_used_elem) * vq->size);

This error was detected when enabling the RISC-V 'virt' libqos machine.
The 'idx' test from vhost-user-blk-test.c errors out with a timeout in
qvirtio_wait_used_elem(). The timeout happens because when processing
the first element, 'avail_event' is read in qvirtqueue_kick() as non-zero
because we didn't initialize it properly (and the memory at that point
happened to be non-zero). 'idx' is 0.

All of this makes this condition fail because "idx - avail_event" will
overflow and be non-zero:

/* < 1 because we add elements to avail queue one by one */
if ((flags & VRING_USED_F_NO_NOTIFY) == 0 &&
 (!vq->event || (uint16_t)(idx-avail_event) < 1)) {
 d->bus->virtqueue_kick(d, vq);
}

As a result the virtqueue is never kicked and we'll timeout waiting for it.

Fixes: 1053587c3f ("libqos: Added EVENT_IDX support")
Signed-off-by: Daniel Henrique Barboza 
---
  tests/qtest/libqos/virtio.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/libqos/virtio.c b/tests/qtest/libqos/virtio.c
index 4f39124eba..82a6e122bf 100644
--- a/tests/qtest/libqos/virtio.c
+++ b/tests/qtest/libqos/virtio.c
@@ -265,7 +265,7 @@ void qvring_init(QTestState *qts, const QGuestAllocator 
*alloc, QVirtQueue *vq,
  /* vq->used->idx */
  qvirtio_writew(vq->vdev, qts, vq->used + 2, 0);
  /* vq->used->avail_event */
-qvirtio_writew(vq->vdev, qts, vq->used + 2 +
+qvirtio_writew(vq->vdev, qts, vq->used + 4 +
 sizeof(struct vring_used_elem) * vq->size, 0);
  }
  


Reviewed-by: Thomas Huth 

[RFC 3/4] mirror: move some checks to qmp

[Fiona Ebner]

From: Fabian Grünbichler 

and assert the passing conditions in block/mirror.c. while incremental
mode was never available for drive-mirror, it makes the interface more
uniform w.r.t. backup block jobs.

Signed-off-by: Fabian Grünbichler 
Signed-off-by: Thomas Lamprecht 
[FE: rebase for 9.0]
Signed-off-by: Fiona Ebner 
---
 block/mirror.c | 28 +---
 blockdev.c | 29 +
 2 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 84155b1f78..15d1c060eb 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1755,31 +1755,13 @@ static BlockJob *mirror_start_job(
 
 GLOBAL_STATE_CODE();
 
-if (sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
-error_setg(errp, "Sync mode '%s' not supported",
-   MirrorSyncMode_str(sync_mode));
-return NULL;
-} else if (sync_mode == MIRROR_SYNC_MODE_BITMAP) {
-if (!bitmap) {
-error_setg(errp, "Must provide a valid bitmap name for '%s'"
-   " sync mode",
-   MirrorSyncMode_str(sync_mode));
-return NULL;
-}
-} else if (bitmap) {
-error_setg(errp,
-   "sync mode '%s' is not compatible with bitmaps",
-   MirrorSyncMode_str(sync_mode));
-return NULL;
-}
+/* QMP interface protects us from these cases */
+assert(sync_mode != MIRROR_SYNC_MODE_INCREMENTAL);
+assert((bitmap && sync_mode == MIRROR_SYNC_MODE_BITMAP) ||
+   (!bitmap && sync_mode != MIRROR_SYNC_MODE_BITMAP));
+assert(!(bitmap && granularity));
 
 if (bitmap) {
-if (granularity) {
-error_setg(errp, "granularity (%d)"
-   "cannot be specified when a bitmap is provided",
-   granularity);
-return NULL;
-}
 granularity = bdrv_dirty_bitmap_granularity(bitmap);
 
 if (bitmap_mode != BITMAP_SYNC_MODE_NEVER) {
diff --git a/blockdev.c b/blockdev.c
index aeb9fde9f3..519f408359 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2852,7 +2852,36 @@ static void blockdev_mirror_common(const char *job_id, 
BlockDriverState *bs,
 sync = MIRROR_SYNC_MODE_FULL;
 }
 
+if ((sync == MIRROR_SYNC_MODE_BITMAP) ||
+(sync == MIRROR_SYNC_MODE_INCREMENTAL)) {
+/* done before desugaring 'incremental' to print the right message */
+if (!bitmap_name) {
+error_setg(errp, "Must provide a valid bitmap name for "
+   "'%s' sync mode", MirrorSyncMode_str(sync));
+return;
+}
+}
+
+if (sync == MIRROR_SYNC_MODE_INCREMENTAL) {
+if (has_bitmap_mode &&
+bitmap_mode != BITMAP_SYNC_MODE_ON_SUCCESS) {
+error_setg(errp, "Bitmap sync mode must be '%s' "
+   "when using sync mode '%s'",
+   BitmapSyncMode_str(BITMAP_SYNC_MODE_ON_SUCCESS),
+   MirrorSyncMode_str(sync));
+return;
+}
+has_bitmap_mode = true;
+sync = MIRROR_SYNC_MODE_BITMAP;
+bitmap_mode = BITMAP_SYNC_MODE_ON_SUCCESS;
+}
+
 if (bitmap_name) {
+if (sync != MIRROR_SYNC_MODE_BITMAP) {
+error_setg(errp, "Sync mode '%s' not supported with bitmap.",
+   MirrorSyncMode_str(sync));
+return;
+}
 if (granularity) {
 error_setg(errp, "Granularity and bitmap cannot both be set");
 return;
-- 
2.39.2

[RFC 4/4] iotests: add test for bitmap mirror

[Fiona Ebner]

From: Fabian Grünbichler 

heavily based on/practically forked off iotest 257 for bitmap backups,
but:

- no writes to filter node 'mirror-top' between completion and
finalization, as those seem to deadlock?
- no inclusion of not-yet-available full/top sync modes in combination
with bitmaps
- extra set of reference/test mirrors to verify that writes in parallel
with active mirror work

intentionally keeping copyright and ownership of original test case to
honor provenance.

Signed-off-by: Fabian Grünbichler 
Signed-off-by: Thomas Lamprecht 
[FE: rebase for 9.0, i.e. adapt to renames like vm.command -> vm.cmd,
 specifying explicit image format for rebase,
 adapt to new behavior of qemu_img(),
 dropping of 'status' field in output, etc.
 rename test from '384' to 'bitmap-sync-mirror']
Signed-off-by: Fiona Ebner 
---
 tests/qemu-iotests/tests/bitmap-sync-mirror   |  550 
 .../qemu-iotests/tests/bitmap-sync-mirror.out | 2810 +
 2 files changed, 3360 insertions(+)
 create mode 100755 tests/qemu-iotests/tests/bitmap-sync-mirror
 create mode 100644 tests/qemu-iotests/tests/bitmap-sync-mirror.out

diff --git a/tests/qemu-iotests/tests/bitmap-sync-mirror 
b/tests/qemu-iotests/tests/bitmap-sync-mirror
new file mode 100755
index 00..6cd9b74dac
--- /dev/null
+++ b/tests/qemu-iotests/tests/bitmap-sync-mirror
@@ -0,0 +1,550 @@
+#!/usr/bin/env python3
+# group: rw
+#
+# Test bitmap-sync mirrors (incremental, differential, and partials)
+#
+# Copyright (c) 2019 John Snow for Red Hat, Inc.
+#
+# 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 .
+#
+# owner=js...@redhat.com
+
+import math
+import os
+
+import iotests
+from iotests import log, qemu_img
+
+SIZE = 64 * 1024 * 1024
+GRANULARITY = 64 * 1024
+
+
+class Pattern:
+def __init__(self, byte, offset, size=GRANULARITY):
+self.byte = byte
+self.offset = offset
+self.size = size
+
+def bits(self, granularity):
+lower = self.offset // granularity
+upper = (self.offset + self.size - 1) // granularity
+return set(range(lower, upper + 1))
+
+
+class PatternGroup:
+"""Grouping of Pattern objects. Initialize with an iterable of Patterns."""
+def __init__(self, patterns):
+self.patterns = patterns
+
+def bits(self, granularity):
+"""Calculate the unique bits dirtied by this pattern grouping"""
+res = set()
+for pattern in self.patterns:
+res |= pattern.bits(granularity)
+return res
+
+
+GROUPS = [
+PatternGroup([
+# Batch 0: 4 clusters
+Pattern('0x49', 0x000),
+Pattern('0x6c', 0x010),   # 1M
+Pattern('0x6f', 0x200),   # 32M
+Pattern('0x76', 0x3ff)]), # 64M - 64K
+PatternGroup([
+# Batch 1: 6 clusters (3 new)
+Pattern('0x65', 0x000),   # Full overwrite
+Pattern('0x77', 0x00f8000),   # Partial-left (1M-32K)
+Pattern('0x72', 0x2008000),   # Partial-right (32M+32K)
+Pattern('0x69', 0x3fe)]), # Adjacent-left (64M - 128K)
+PatternGroup([
+# Batch 2: 7 clusters (3 new)
+Pattern('0x74', 0x001),   # Adjacent-right
+Pattern('0x69', 0x00e8000),   # Partial-left  (1M-96K)
+Pattern('0x6e', 0x2018000),   # Partial-right (32M+96K)
+Pattern('0x67', 0x3fe,
+2*GRANULARITY)]), # Overwrite [(64M-128K)-64M)
+PatternGroup([
+# Batch 3: 8 clusters (5 new)
+# Carefully chosen such that nothing re-dirties the one cluster
+# that copies out successfully before failure in Group #1.
+Pattern('0xaa', 0x001,
+3*GRANULARITY),   # Overwrite and 2x Adjacent-right
+Pattern('0xbb', 0x00d8000),   # Partial-left (1M-160K)
+Pattern('0xcc', 0x2028000),   # Partial-right (32M+160K)
+Pattern('0xdd', 0x3fc)]), # New; leaving a gap to the right
+]
+
+
+class EmulatedBitmap:
+def __init__(self, granularity=GRANULARITY):
+self._bits = set()
+self.granularity = granularity
+
+def dirty_bits(self, bits):
+self._bits |= set(bits)
+
+def dirty_group(self, n):
+self.dirty_bits(GROUPS[n].bits(self.granularity))
+
+def clear(self):
+self._bits = set()
+
+def clear_bits(self, bits):
+self._bits -= set(bits)
+
+def clear_bit(self, bit):
+self.c

[RFC 2/4] drive-mirror: add support for conditional and always bitmap sync modes

[Fiona Ebner]

From: John Snow 

Teach mirror two new tricks for using bitmaps:

Always: no matter what, we synchronize the copy_bitmap back to the
sync_bitmap. In effect, this allows us resume a failed mirror at a later
date.

Conditional: On success only, we sync the bitmap. This is akin to
incremental backup modes; we can use this bitmap to later refresh a
successfully created mirror.

Originally-by: John Snow 
[FG: add check for bitmap-mode without bitmap
 switch to bdrv_dirty_bitmap_merge_internal]
Signed-off-by: Fabian Grünbichler 
Signed-off-by: Thomas Lamprecht 
[FE: rebase for 9.0]
Signed-off-by: Fiona Ebner 
---

The original patch this was based on came from a WIP git branch and
thus has no Signed-off-by trailer from John, see [0]. I added an
Originally-by trailer for now. Let me know if I should drop that and
wait for John's Signed-off-by instead.

[0] https://lore.kernel.org/qemu-devel/1599140071.n44h532eeu.astr...@nora.none/

 block/mirror.c | 24 ++--
 blockdev.c |  3 +++
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 315dff11e2..84155b1f78 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -689,8 +689,6 @@ static int mirror_exit_common(Job *job)
 bdrv_unfreeze_backing_chain(mirror_top_bs, target_bs);
 }
 
-bdrv_release_dirty_bitmap(s->dirty_bitmap);
-
 /* Make sure that the source BDS doesn't go away during bdrv_replace_node,
  * before we can call bdrv_drained_end */
 bdrv_ref(src);
@@ -796,6 +794,18 @@ static int mirror_exit_common(Job *job)
 bdrv_drained_end(target_bs);
 bdrv_unref(target_bs);
 
+if (s->sync_bitmap) {
+if (s->bitmap_mode == BITMAP_SYNC_MODE_ALWAYS ||
+(s->bitmap_mode == BITMAP_SYNC_MODE_ON_SUCCESS &&
+ job->ret == 0 && ret == 0)) {
+/* Success; synchronize copy back to sync. */
+bdrv_clear_dirty_bitmap(s->sync_bitmap, NULL);
+bdrv_dirty_bitmap_merge_internal(s->sync_bitmap, s->dirty_bitmap,
+ NULL, true);
+}
+}
+bdrv_release_dirty_bitmap(s->dirty_bitmap);
+
 bs_opaque->job = NULL;
 
 bdrv_drained_end(src);
@@ -1755,10 +1765,6 @@ static BlockJob *mirror_start_job(
" sync mode",
MirrorSyncMode_str(sync_mode));
 return NULL;
-} else if (bitmap_mode != BITMAP_SYNC_MODE_NEVER) {
-error_setg(errp,
-   "Bitmap Sync Mode '%s' is not supported by Mirror",
-   BitmapSyncMode_str(bitmap_mode));
 }
 } else if (bitmap) {
 error_setg(errp,
@@ -1775,6 +1781,12 @@ static BlockJob *mirror_start_job(
 return NULL;
 }
 granularity = bdrv_dirty_bitmap_granularity(bitmap);
+
+if (bitmap_mode != BITMAP_SYNC_MODE_NEVER) {
+if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT, errp)) {
+return NULL;
+}
+}
 } else if (granularity == 0) {
 granularity = bdrv_get_default_bitmap_granularity(target);
 }
diff --git a/blockdev.c b/blockdev.c
index c65d9ded70..aeb9fde9f3 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2873,6 +2873,9 @@ static void blockdev_mirror_common(const char *job_id, 
BlockDriverState *bs,
 if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_ALLOW_RO, errp)) {
 return;
 }
+} else if (has_bitmap_mode) {
+error_setg(errp, "Cannot specify bitmap sync mode without a bitmap");
+return;
 }
 
 if (!replaces) {
-- 
2.39.2

[RFC 0/4] mirror: implement incremental and bitmap modes

[Fiona Ebner]

Previous discussion from when this was sent upstream [0] (it's been a
while). I rebased the patches and re-ordered and squashed like
suggested back then [1].

This implements two new mirror modes:

- bitmap mirror mode with always/on-success/never bitmap sync mode
- incremental mirror mode as sugar for bitmap + on-success

Use cases:
* Possibility to resume a failed mirror later.
* Possibility to only mirror deltas to a previously mirrored volume.
* Possibility to (efficiently) mirror an drive that was previously
  mirrored via some external mechanism (e.g. ZFS replication).

We are using the last one in production without any issues since about
4 years now. In particular, like mentioned in [2]:

> - create bitmap(s)
> - (incrementally) replicate storage volume(s) out of band (using ZFS)
> - incrementally drive mirror as part of a live migration of VM
> - drop bitmap(s)


Now, the IO test added in patch 4/4 actually contains yet another use
case, namely doing incremental mirrors to stand-alone qcow2 "diff"
images, that only contain the delta and can be rebased later. I had to
adapt the IO test, because its output expected the mirror bitmap to
still be dirty, but nowadays the mirror is apparently already done
when the bitmaps are queried. So I thought, I'll just use
'write-blocking' mode to avoid any potential timing issues.

But this exposed an issue with the diff image approach. If a write is
not aligned to the granularity of the mirror target, then rebasing the
diff image onto a backing image will not yield the desired result,
because the full cluster is considered to be allocated and will "hide"
some part of the base/backing image. The failure can be seen by either
using 'write-blocking' mode in the IO test or setting the (bitmap)
granularity to 32 KiB rather than the current 64 KiB.

The question is how to deal with these edge cases? Some possibilities
that would make sense to me:

For 'background' mode:
* prohibit if target's cluster size is larger than the bitmap
  granularity
* document the limitation

For 'write-blocking' mode:
* disallow in combination with bitmap mode (would not be happy about
  it, because I'd like to use this without diff images)
* for writes that are not aligned to the target's cluster size, read
  the relevant/missing parts from the source image to be able to write
  whole target clusters (seems rather complex)
* document the limitation


[0]: 
https://lore.kernel.org/qemu-devel/20200218100740.2228521-1-f.gruenbich...@proxmox.com/
[1]: 
https://lore.kernel.org/qemu-devel/d35a76de-78d5-af56-0b34-f7bd2bbd3...@redhat.com/
[2]: https://lore.kernel.org/qemu-devel/1599127031.9uxdp5h9o2.astr...@nora.none/


Fabian Grünbichler (2):
  mirror: move some checks to qmp
  iotests: add test for bitmap mirror

John Snow (2):
  drive-mirror: add support for sync=bitmap mode=never
  drive-mirror: add support for conditional and always bitmap sync modes

 block/mirror.c|   94 +-
 blockdev.c|   70 +-
 include/block/block_int-global-state.h|4 +-
 qapi/block-core.json  |   25 +-
 tests/qemu-iotests/tests/bitmap-sync-mirror   |  550 
 .../qemu-iotests/tests/bitmap-sync-mirror.out | 2810 +
 tests/unit/test-block-iothread.c  |4 +-
 7 files changed, 3527 insertions(+), 30 deletions(-)
 create mode 100755 tests/qemu-iotests/tests/bitmap-sync-mirror
 create mode 100644 tests/qemu-iotests/tests/bitmap-sync-mirror.out

-- 
2.39.2

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

[Fiona Ebner]

From: John Snow 

This patch adds support for the "BITMAP" sync mode to drive-mirror and
blockdev-mirror. It adds support only for the BitmapSyncMode "never,"
because it's the simplest mode.

This mode simply uses a user-provided bitmap as an initial copy
manifest, and then does not clear any bits in the bitmap at the
conclusion of the operation.

Any new writes dirtied during the operation are copied out, in contrast
to backup. Note that whether these writes are reflected in the bitmap
at the conclusion of the operation depends on whether that bitmap is
actually recording!

This patch was originally based on one by Ma Haocong, but it has since
been modified pretty heavily.

Suggested-by: Ma Haocong 
Signed-off-by: Ma Haocong 
Signed-off-by: John Snow 
[FG: switch to bdrv_dirty_bitmap_merge_internal]
Signed-off-by: Fabian Grünbichler 
Signed-off-by: Thomas Lamprecht 
[FE: rebase for 9.0
 update version and formatting in QAPI]
Signed-off-by: Fiona Ebner 
---
 block/mirror.c | 96 --
 blockdev.c | 38 +-
 include/block/block_int-global-state.h |  4 +-
 qapi/block-core.json   | 25 ++-
 tests/unit/test-block-iothread.c   |  4 +-
 5 files changed, 139 insertions(+), 28 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 5145eb53e1..315dff11e2 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -51,7 +51,7 @@ typedef struct MirrorBlockJob {
 BlockDriverState *to_replace;
 /* Used to block operations on the drive-mirror-replace target */
 Error *replace_blocker;
-bool is_none_mode;
+MirrorSyncMode sync_mode;
 BlockMirrorBackingMode backing_mode;
 /* Whether the target image requires explicit zero-initialization */
 bool zero_target;
@@ -73,6 +73,8 @@ typedef struct MirrorBlockJob {
 size_t buf_size;
 int64_t bdev_length;
 unsigned long *cow_bitmap;
+BdrvDirtyBitmap *sync_bitmap;
+BitmapSyncMode bitmap_mode;
 BdrvDirtyBitmap *dirty_bitmap;
 BdrvDirtyBitmapIter *dbi;
 uint8_t *buf;
@@ -718,7 +720,8 @@ static int mirror_exit_common(Job *job)
  &error_abort);
 
 if (!abort && s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) {
-BlockDriverState *backing = s->is_none_mode ? src : s->base;
+BlockDriverState *backing;
+backing = s->sync_mode == MIRROR_SYNC_MODE_NONE ? src : s->base;
 BlockDriverState *unfiltered_target = bdrv_skip_filters(target_bs);
 
 if (bdrv_cow_bs(unfiltered_target) != backing) {
@@ -815,6 +818,16 @@ static void mirror_abort(Job *job)
 assert(ret == 0);
 }
 
+/* Always called after commit/abort. */
+static void mirror_clean(Job *job)
+{
+MirrorBlockJob *s = container_of(job, MirrorBlockJob, common.job);
+
+if (s->sync_bitmap) {
+bdrv_dirty_bitmap_set_busy(s->sync_bitmap, false);
+}
+}
+
 static void coroutine_fn mirror_throttle(MirrorBlockJob *s)
 {
 int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
@@ -1011,7 +1024,8 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
 mirror_free_init(s);
 
 s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
-if (!s->is_none_mode) {
+if ((s->sync_mode == MIRROR_SYNC_MODE_TOP) ||
+(s->sync_mode == MIRROR_SYNC_MODE_FULL)) {
 ret = mirror_dirty_init(s);
 if (ret < 0 || job_is_cancelled(&s->common.job)) {
 goto immediate_exit;
@@ -1302,6 +1316,7 @@ static const BlockJobDriver mirror_job_driver = {
 .run= mirror_run,
 .prepare= mirror_prepare,
 .abort  = mirror_abort,
+.clean  = mirror_clean,
 .pause  = mirror_pause,
 .complete   = mirror_complete,
 .cancel = mirror_cancel,
@@ -1320,6 +1335,7 @@ static const BlockJobDriver commit_active_job_driver = {
 .run= mirror_run,
 .prepare= mirror_prepare,
 .abort  = mirror_abort,
+.clean  = mirror_clean,
 .pause  = mirror_pause,
 .complete   = mirror_complete,
 .cancel = commit_active_cancel,
@@ -1712,7 +1728,10 @@ static BlockJob *mirror_start_job(
  BlockCompletionFunc *cb,
  void *opaque,
  const BlockJobDriver *driver,
- bool is_none_mode, BlockDriverState *base,
+ MirrorSyncMode sync_mode,
+ BdrvDirtyBitmap *bitmap,
+ BitmapSyncMode bitmap_mode,
+ BlockDriverState *base,
  bool auto_complete, const char *filter_node_name,
  bool is_mirror, MirrorCopyMode copy_mode,

Re: [PATCH 6/6] tests/libqos: add riscv/virt machine nodes

[Thomas Huth]

On 13/02/2024 20.17, Daniel Henrique Barboza wrote:

Add a RISC-V 'virt' machine to the graph. This implementation is a
modified copy of the existing arm machine in arm-virt-machine.c

It contains a virtio-mmio and a generic-pcihost controller. The
generic-pcihost controller hardcodes assumptions from the ARM 'virt'
machine, like ecam and pio_base addresses, so we'll add an extra step to
set its parameters after creating it.

Our command line is incremented with 'aclint' parameters to allow the
machine to run MSI tests.

Signed-off-by: Daniel Henrique Barboza 
---
  tests/qtest/libqos/meson.build  |   1 +
  tests/qtest/libqos/riscv-virt-machine.c | 137 
  2 files changed, 138 insertions(+)
  create mode 100644 tests/qtest/libqos/riscv-virt-machine.c


Acked-by: Thomas Huth 

Re: [PULL 00/35] target-arm queue

[Peter Maydell]

On Thu, 15 Feb 2024 at 17:35, Peter Maydell  wrote:
>
> The following changes since commit 5767815218efd3cbfd409505ed824d5f356044ae:
>
>   Merge tag 'for_upstream' of 
> https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging (2024-02-14 
> 15:45:52 +)
>
> are available in the Git repository at:
>
>   https://git.linaro.org/people/pmaydell/qemu-arm.git 
> tags/pull-target-arm-20240215
>
> for you to fetch changes up to f780e63fe731b058fe52d43653600d8729a1b5f2:
>
>   docs: Add documentation for the mps3-an536 board (2024-02-15 14:32:39 +)
>
> 
> target-arm queue:
>  * hw/arm/xilinx_zynq: Wire FIQ between CPU <> GIC
>  * linux-user/aarch64: Choose SYNC as the preferred MTE mode
>  * Fix some errors in SVE/SME handling of MTE tags
>  * hw/pci-host/raven.c: Mark raven_io_ops as implementing unaligned accesses
>  * hw/block/tc58128: Don't emit deprecation warning under qtest
>  * tests/qtest: Fix handling of npcm7xx and GMAC tests
>  * hw/arm/virt: Wire up non-secure EL2 virtual timer IRQ
>  * tests/qtest/npcm7xx_emc-test: Connect all NICs to a backend
>  * Don't assert on vmload/vmsave of M-profile CPUs
>  * hw/arm/smmuv3: add support for stage 1 access fault
>  * hw/arm/stellaris: QOM cleanups
>  * Use new CBAR encoding for all v8 CPUs, not all aarch64 CPUs
>  * Improve Cortex_R52 IMPDEF sysreg modelling
>  * Allow access to SPSR_hyp from hyp mode
>  * New board model mps3-an536 (Cortex-R52)
>


Applied, thanks.

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

-- PMM

[PATCH 01/21] hw/i386/pc: Do not use C99 mixed-declarations style

[Philippe Mathieu-Daudé]

QEMU's coding style generally forbids C99 mixed declarations.

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

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 196827531a..3c00a87317 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1227,6 +1227,7 @@ void pc_basic_device_init(struct PCMachineState *pcms,
  */
 if (pcms->hpet_enabled) {
 qemu_irq rtc_irq;
+uint8_t compat;
 
 hpet = qdev_try_new(TYPE_HPET);
 if (!hpet) {
@@ -1238,8 +1239,7 @@ void pc_basic_device_init(struct PCMachineState *pcms,
  * use IRQ16~23, IRQ8 and IRQ2.  If the user has already set
  * the property, use whatever mask they specified.
  */
-uint8_t compat = object_property_get_uint(OBJECT(hpet),
-HPET_INTCAP, NULL);
+compat = object_property_get_uint(OBJECT(hpet), HPET_INTCAP, NULL);
 if (!compat) {
 qdev_prop_set_uint32(hpet, HPET_INTCAP, hpet_irqs);
 }
-- 
2.41.0

[PATCH 12/21] hw/pci-host/q35: Update q35_host_props[] comment

[Philippe Mathieu-Daudé]

Commit aff39be0ed ("hw/pci-host: Use object_initialize_child for
correct reference counting") replaced object_initialize() by
object_initialize_child(), update the comment.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/pci-host/q35.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 0d7d4e3f08..0eb1c7034d 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -165,10 +165,10 @@ static void q35_host_get_pci_hole64_end(Object *obj, 
Visitor *v,
 /*
  * NOTE: setting defaults for the mch.* fields in this table
  * doesn't work, because mch is a separate QOM object that is
- * zeroed by the object_initialize(&s->mch, ...) call inside
+ * zeroed by the object_initialize_child(..., &s->mch, ...) call inside
  * q35_host_initfn().  The default values for those
  * properties need to be initialized manually by
- * q35_host_initfn() after the object_initialize() call.
+ * q35_host_initfn() after the object_initialize_child() call.
  */
 static Property q35_host_props[] = {
 DEFINE_PROP_UINT64(PCIE_HOST_MCFG_BASE, Q35PCIHost, parent_obj.base_addr,
@@ -211,7 +211,7 @@ static void q35_host_initfn(Object *obj)
 object_initialize_child(OBJECT(s), "mch", &s->mch, TYPE_MCH_PCI_DEVICE);
 qdev_prop_set_int32(DEVICE(&s->mch), "addr", PCI_DEVFN(0, 0));
 qdev_prop_set_bit(DEVICE(&s->mch), "multifunction", false);
-/* mch's object_initialize resets the default value, set it again */
+/* mch's object_initialize_child resets the default value, set it again */
 qdev_prop_set_uint64(DEVICE(s), PCI_HOST_PROP_PCI_HOLE64_SIZE,
  Q35_PCI_HOST_HOLE64_SIZE_DEFAULT);
 object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_START, "uint32",
-- 
2.41.0

[PATCH 02/21] hw/i386/pc_sysfw: Use qdev_is_realized() instead of QOM API

[Philippe Mathieu-Daudé]

Prefer QDev API for QDev objects, avoid the underlying QOM layer.

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

diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index c8d9e71b88..3efabbbab2 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -107,17 +107,15 @@ void pc_system_flash_cleanup_unused(PCMachineState *pcms)
 {
 char *prop_name;
 int i;
-Object *dev_obj;
 
 assert(PC_MACHINE_GET_CLASS(pcms)->pci_enabled);
 
 for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {
-dev_obj = OBJECT(pcms->flash[i]);
-if (!object_property_get_bool(dev_obj, "realized", &error_abort)) {
+if (!qdev_is_realized(DEVICE(pcms->flash[i]))) {
 prop_name = g_strdup_printf("pflash%d", i);
 object_property_del(OBJECT(pcms), prop_name);
 g_free(prop_name);
-object_unparent(dev_obj);
+object_unparent(OBJECT(pcms->flash[i]));
 pcms->flash[i] = NULL;
 }
 }
-- 
2.41.0

[PATCH 05/21] hw/ppc/pnv_bmc: Use qdev_new() instead of QOM API

[Philippe Mathieu-Daudé]

Prefer QDev API for QDev objects, avoid the underlying QOM layer.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ppc/pnv_bmc.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c
index 99f1e8d7f9..0c1274df21 100644
--- a/hw/ppc/pnv_bmc.c
+++ b/hw/ppc/pnv_bmc.c
@@ -269,13 +269,13 @@ void pnv_bmc_set_pnor(IPMIBmc *bmc, PnvPnor *pnor)
  */
 IPMIBmc *pnv_bmc_create(PnvPnor *pnor)
 {
-Object *obj;
+DeviceState *dev;
 
-obj = object_new(TYPE_IPMI_BMC_SIMULATOR);
-qdev_realize(DEVICE(obj), NULL, &error_fatal);
-pnv_bmc_set_pnor(IPMI_BMC(obj), pnor);
+dev = qdev_new(TYPE_IPMI_BMC_SIMULATOR);
+qdev_realize(dev, NULL, &error_fatal);
+pnv_bmc_set_pnor(IPMI_BMC(dev), pnor);
 
-return IPMI_BMC(obj);
+return IPMI_BMC(dev);
 }
 
 typedef struct ForeachArgs {
-- 
2.41.0

[PATCH 00/21] hw: More QDev cleanups

[Philippe Mathieu-Daudé]

Various QDev cleanups extracted to my "enforce QDev API" branch.
- When available, instead of plain QOM, use QDev API equivalent
- Add missing QOM parentship for some obj created with qdev_*new()
- Prefer object_initialize_child() over object_initialize()

Philippe Mathieu-Daudé (21):
  hw/i386/pc: Do not use C99 mixed-declarations style
  hw/i386/pc_sysfw: Use qdev_is_realized() instead of QOM API
  hw/ppc/spapr_cpu: Use qdev_is_realized() instead of QOM API
  hw/tricore/testboard: Use qdev_new() instead of QOM basic API
  hw/ppc/pnv_bmc: Use qdev_new() instead of QOM API
  hw: Replace DEVICE(object_new) -> qdev_new()
  target: Replace DEVICE(object_new) -> qdev_new()
  hw/isa: Inline isa_try_new()
  hw/usb: Inline usb_try_new()
  hw/usb: Inline usb_new()
  hw/usb: Add QOM parentship relation with hub devices
  hw/pci-host/q35: Update q35_host_props[] comment
  hw/pci-host/raven: Embedded OrIRQ in PRePPCIState
  hw/pci-host/raven: Prefer object_initialize_child over
object_initialize
  hw/core/register: Prefer object_initialize_child over
object_initialize
  hw/net/can/versal: Prefer object_initialize_child over
object_initialize
  hw/i386/iommu: Prefer object_initialize_child over object_initialize
  hw/pci-host/versatile: Replace object_initialize() -> _child()
  hw/s390x/zpci-bus: Add QOM parentship relation with zPCI devices
  hw/arm/mps2: Add QOM parentship relation with OR IRQ gates
  hw: Add QOM parentship relation with CPUs

 include/hw/isa/isa.h |  1 -
 include/hw/net/ne2000-isa.h  |  2 +-
 include/hw/tricore/tricore_testdevice.h  |  3 ---
 include/hw/usb.h |  1 -
 hw/arm/mps2.c|  5 +
 hw/arm/musicpal.c|  2 +-
 hw/core/qdev.c   |  2 +-
 hw/core/register.c   |  2 +-
 hw/i386/amd_iommu.c  |  6 +++---
 hw/i386/pc.c |  6 +++---
 hw/i386/pc_sysfw.c   |  6 ++
 hw/i386/x86.c|  1 +
 hw/isa/isa-bus.c |  5 -
 hw/microblaze/petalogix_ml605_mmu.c  |  1 +
 hw/microblaze/petalogix_s3adsp1800_mmu.c |  1 +
 hw/mips/cps.c|  1 +
 hw/net/can/xlnx-versal-canfd.c   |  2 +-
 hw/nios2/10m50_devboard.c|  1 +
 hw/pci-host/q35.c|  6 +++---
 hw/pci-host/raven.c  | 18 --
 hw/pci-host/versatile.c  |  3 ++-
 hw/ppc/e500.c|  1 +
 hw/ppc/pnv_bmc.c | 10 +-
 hw/ppc/spapr.c   |  1 +
 hw/ppc/spapr_cpu_core.c  |  3 +--
 hw/s390x/s390-pci-bus.c  |  1 +
 hw/sparc/sun4m.c |  4 ++--
 hw/tricore/tricore_testboard.c   |  4 +---
 hw/usb/bus.c | 17 -
 hw/usb/dev-serial.c  |  2 +-
 target/mips/cpu.c|  2 +-
 target/xtensa/cpu.c  |  2 +-
 32 files changed, 55 insertions(+), 67 deletions(-)

-- 
2.41.0

[PATCH 13/21] hw/pci-host/raven: Embedded OrIRQ in PRePPCIState

[Philippe Mathieu-Daudé]

Since we know the size of the OrIRQ object, we can initialize
it directly in place with object_initialize_child(). Doing so
we also set the QOM parent <-> child relationship.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/pci-host/raven.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
index c7a0a2878a..9e47caebc5 100644
--- a/hw/pci-host/raven.c
+++ b/hw/pci-host/raven.c
@@ -60,7 +60,7 @@ DECLARE_INSTANCE_CHECKER(PREPPCIState, RAVEN_PCI_HOST_BRIDGE,
 struct PRePPCIState {
 PCIHostState parent_obj;
 
-OrIRQState *or_irq;
+OrIRQState or_irq;
 qemu_irq pci_irqs[PCI_NUM_PINS];
 PCIBus pci_bus;
 AddressSpace pci_io_as;
@@ -249,14 +249,14 @@ static void raven_pcihost_realizefn(DeviceState *d, Error 
**errp)
 } else {
 /* According to PReP specification section 6.1.6 "System Interrupt
  * Assignments", all PCI interrupts are routed via IRQ 15 */
-s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
-object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
+object_initialize_child(OBJECT(dev), "or-irq", &s->or_irq, 
TYPE_OR_IRQ);
+object_property_set_int(OBJECT(&s->or_irq), "num-lines", PCI_NUM_PINS,
 &error_fatal);
-qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
-sysbus_init_irq(dev, &s->or_irq->out_irq);
+qdev_realize(DEVICE(&s->or_irq), NULL, &error_fatal);
+sysbus_init_irq(dev, &s->or_irq.out_irq);
 
 for (i = 0; i < PCI_NUM_PINS; i++) {
-s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(s->or_irq), i);
+s->pci_irqs[i] = qdev_get_gpio_in(DEVICE(&s->or_irq), i);
 }
 }
 
-- 
2.41.0

[PATCH 11/21] hw/usb: Add QOM parentship relation with hub devices

[Philippe Mathieu-Daudé]

QDev objects created with qdev_*new() need to manually add
their parent relationship with object_property_add_child().

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

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index a599e2552b..baad04f466 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -439,6 +439,7 @@ void usb_claim_port(USBDevice *dev, Error **errp)
 /* Create a new hub and chain it on */
 hub = USB_DEVICE(qdev_try_new("usb-hub"));
 if (hub) {
+object_property_add_child(OBJECT(dev), "hub", OBJECT(hub));
 usb_realize_and_unref(hub, bus, NULL);
 }
 }
-- 
2.41.0

[PATCH 14/21] hw/pci-host/raven: Prefer object_initialize_child over object_initialize

[Philippe Mathieu-Daudé]

When the QOM parent is available, prefer object_initialize_child()
over object_initialize(), since it create the parent relationship.

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

diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
index 9e47caebc5..5ef25edba6 100644
--- a/hw/pci-host/raven.c
+++ b/hw/pci-host/raven.c
@@ -290,7 +290,6 @@ static void raven_pcihost_initfn(Object *obj)
 PCIHostState *h = PCI_HOST_BRIDGE(obj);
 PREPPCIState *s = RAVEN_PCI_HOST_BRIDGE(obj);
 MemoryRegion *address_space_mem = get_system_memory();
-DeviceState *pci_dev;
 
 memory_region_init(&s->pci_io, obj, "pci-io", 0x3f80);
 memory_region_init_io(&s->pci_io_non_contiguous, obj, &raven_io_ops, s,
@@ -328,11 +327,10 @@ static void raven_pcihost_initfn(Object *obj)
 
 h->bus = &s->pci_bus;
 
-object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_RAVEN_PCI_DEVICE);
-pci_dev = DEVICE(&s->pci_dev);
+object_initialize_child(obj, "bridge", &s->pci_dev, TYPE_RAVEN_PCI_DEVICE);
 object_property_set_int(OBJECT(&s->pci_dev), "addr", PCI_DEVFN(0, 0),
 NULL);
-qdev_prop_set_bit(pci_dev, "multifunction", false);
+qdev_prop_set_bit(DEVICE(&s->pci_dev), "multifunction", false);
 }
 
 static void raven_realize(PCIDevice *d, Error **errp)
-- 
2.41.0

[PATCH 18/21] hw/pci-host/versatile: Replace object_initialize() -> _child()

[Philippe Mathieu-Daudé]

When the QOM parent is available, prefer object_initialize_child()
over object_initialize(), since it create the parent relationship.

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

diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
index 0e65deb3f9..5d1f9cc96d 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
@@ -410,7 +410,8 @@ static void pci_vpb_realize(DeviceState *dev, Error **errp)
   PCI_DEVFN(11, 0), TYPE_PCI_BUS);
 h->bus = &s->pci_bus;
 
-object_initialize(&s->pci_dev, sizeof(s->pci_dev), 
TYPE_VERSATILE_PCI_HOST);
+object_initialize_child(OBJECT(dev), "pci-func0",
+&s->pci_dev, TYPE_VERSATILE_PCI_HOST);
 
 for (i = 0; i < 4; i++) {
 sysbus_init_irq(sbd, &s->irq[i]);
-- 
2.41.0

[PATCH 06/21] hw: Replace DEVICE(object_new) -> qdev_new()

[Philippe Mathieu-Daudé]

Prefer QDev API for QDev objects, avoid the underlying QOM layer.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/arm/musicpal.c | 2 +-
 hw/core/qdev.c| 2 +-
 hw/sparc/sun4m.c  | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index 2020f73a57..74e4d24aab 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -1238,7 +1238,7 @@ static void musicpal_init(MachineState *machine)
   qdev_get_gpio_in(pic, MP_TIMER4_IRQ), NULL);
 
 /* Logically OR both UART IRQs together */
-uart_orgate = DEVICE(object_new(TYPE_OR_IRQ));
+uart_orgate = qdev_new(TYPE_OR_IRQ);
 object_property_set_int(OBJECT(uart_orgate), "num-lines", 2, &error_fatal);
 qdev_realize_and_unref(uart_orgate, NULL, &error_fatal);
 qdev_connect_gpio_out(uart_orgate, 0,
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index c68d0f7c51..a271380d20 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -171,7 +171,7 @@ DeviceState *qdev_try_new(const char *name)
 if (!module_object_class_by_name(name)) {
 return NULL;
 }
-return DEVICE(object_new(name));
+return qdev_new(name);
 }
 
 static QTAILQ_HEAD(, DeviceListener) device_listeners
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index d52e6a7213..fedc4b8b3f 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -979,7 +979,7 @@ static void sun4m_hw_init(MachineState *machine)
 sysbus_mmio_map(s, 0, hwdef->ms_kb_base);
 
 /* Logically OR both its IRQs together */
-ms_kb_orgate = DEVICE(object_new(TYPE_OR_IRQ));
+ms_kb_orgate = qdev_new(TYPE_OR_IRQ);
 object_property_set_int(OBJECT(ms_kb_orgate), "num-lines", 2, 
&error_fatal);
 qdev_realize_and_unref(ms_kb_orgate, NULL, &error_fatal);
 sysbus_connect_irq(s, 0, qdev_get_gpio_in(ms_kb_orgate, 0));
@@ -1000,7 +1000,7 @@ static void sun4m_hw_init(MachineState *machine)
 sysbus_mmio_map(s, 0, hwdef->serial_base);
 
 /* Logically OR both its IRQs together */
-serial_orgate = DEVICE(object_new(TYPE_OR_IRQ));
+serial_orgate = qdev_new(TYPE_OR_IRQ);
 object_property_set_int(OBJECT(serial_orgate), "num-lines", 2,
 &error_fatal);
 qdev_realize_and_unref(serial_orgate, NULL, &error_fatal);
-- 
2.41.0

[PATCH 07/21] target: Replace DEVICE(object_new) -> qdev_new()

[Philippe Mathieu-Daudé]

Prefer QDev API for QDev objects, avoid the underlying QOM layer.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/mips/cpu.c   | 2 +-
 target/xtensa/cpu.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index d644adbc77..6b3909ee08 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -649,7 +649,7 @@ MIPSCPU *mips_cpu_create_with_clock(const char *cpu_type, 
Clock *cpu_refclk)
 {
 DeviceState *cpu;
 
-cpu = DEVICE(object_new(cpu_type));
+cpu = qdev_new(cpu_type);
 qdev_connect_clock_in(cpu, "clk-in", cpu_refclk);
 qdev_realize(cpu, NULL, &error_abort);
 
diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index 79f91819df..4f9408e1a0 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -205,7 +205,7 @@ XtensaCPU *xtensa_cpu_create_with_clock(const char 
*cpu_type, Clock *cpu_refclk)
 {
 DeviceState *cpu;
 
-cpu = DEVICE(object_new(cpu_type));
+cpu = qdev_new(cpu_type);
 qdev_connect_clock_in(cpu, "clk-in", cpu_refclk);
 qdev_realize(cpu, NULL, &error_abort);
 
-- 
2.41.0

[PATCH 20/21] hw/arm/mps2: Add QOM parentship relation with OR IRQ gates

[Philippe Mathieu-Daudé]

QDev objects created with object_new() need to manually add
their parent relationship with object_property_add_child().

Signed-off-by: Philippe Mathieu-Daudé 
---
Better would be to embedded an call object_initialize_child()...
---
 hw/arm/mps2.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
index 50919ee46d..780f2adf0f 100644
--- a/hw/arm/mps2.c
+++ b/hw/arm/mps2.c
@@ -274,6 +274,7 @@ static void mps2_common_init(MachineState *machine)
 
 orgate = object_new(TYPE_OR_IRQ);
 object_property_set_int(orgate, "num-lines", 6, &error_fatal);
+object_property_add_child(OBJECT(machine), "orgate12", orgate);
 qdev_realize(DEVICE(orgate), NULL, &error_fatal);
 orgate_dev = DEVICE(orgate);
 qdev_connect_gpio_out(orgate_dev, 0, qdev_get_gpio_in(armv7m, 12));
@@ -317,6 +318,7 @@ static void mps2_common_init(MachineState *machine)
 
 orgate = object_new(TYPE_OR_IRQ);
 object_property_set_int(orgate, "num-lines", 10, &error_fatal);
+object_property_add_child(OBJECT(machine), "orgate-12", orgate);
 qdev_realize(DEVICE(orgate), NULL, &error_fatal);
 orgate_dev = DEVICE(orgate);
 qdev_connect_gpio_out(orgate_dev, 0, qdev_get_gpio_in(armv7m, 12));
@@ -333,6 +335,8 @@ static void mps2_common_init(MachineState *machine)
 
 txrx_orgate = object_new(TYPE_OR_IRQ);
 object_property_set_int(txrx_orgate, "num-lines", 2, &error_fatal);
+object_property_add_child(OBJECT(machine),
+  "orgate-uart[*]", txrx_orgate);
 qdev_realize(DEVICE(txrx_orgate), NULL, &error_fatal);
 txrx_orgate_dev = DEVICE(txrx_orgate);
 qdev_connect_gpio_out(txrx_orgate_dev, 0,
@@ -425,6 +429,7 @@ static void mps2_common_init(MachineState *machine)
 
 orgate = object_new(TYPE_OR_IRQ);
 object_property_set_int(orgate, "num-lines", 2, &error_fatal);
+object_property_add_child(OBJECT(machine), "orgate-ssi[*]", orgate);
 orgate_dev = DEVICE(orgate);
 qdev_realize(orgate_dev, NULL, &error_fatal);
 qdev_connect_gpio_out(orgate_dev, 0,
-- 
2.41.0

[PATCH 09/21] hw/usb: Inline usb_try_new()

[Philippe Mathieu-Daudé]

Inline the single use of usb_try_new().

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/usb/bus.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 59c39945dd..148224f06a 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -334,11 +334,6 @@ USBDevice *usb_new(const char *name)
 return USB_DEVICE(qdev_new(name));
 }
 
-static USBDevice *usb_try_new(const char *name)
-{
-return USB_DEVICE(qdev_try_new(name));
-}
-
 bool usb_realize_and_unref(USBDevice *dev, USBBus *bus, Error **errp)
 {
 return qdev_realize_and_unref(&dev->qdev, &bus->qbus, errp);
@@ -447,7 +442,7 @@ void usb_claim_port(USBDevice *dev, Error **errp)
 } else {
 if (bus->nfree == 1 && strcmp(object_get_typename(OBJECT(dev)), 
"usb-hub") != 0) {
 /* Create a new hub and chain it on */
-hub = usb_try_new("usb-hub");
+hub = USB_DEVICE(qdev_try_new("usb-hub"));
 if (hub) {
 usb_realize_and_unref(hub, bus, NULL);
 }
-- 
2.41.0

[PATCH 17/21] hw/i386/iommu: Prefer object_initialize_child over object_initialize

[Philippe Mathieu-Daudé]

When the QOM parent is available, prefer object_initialize_child()
over object_initialize(), since it create the parent relationship.

Rename the 'klass' variable as 'obj' since the argument holds a
reference to an instance object and not a class one.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/amd_iommu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 7329553ad3..c3afbc4130 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1616,11 +1616,11 @@ static const VMStateDescription vmstate_amdvi_sysbus = {
 .unmigratable = 1
 };
 
-static void amdvi_sysbus_instance_init(Object *klass)
+static void amdvi_sysbus_instance_init(Object *obj)
 {
-AMDVIState *s = AMD_IOMMU_DEVICE(klass);
+AMDVIState *s = AMD_IOMMU_DEVICE(obj);
 
-object_initialize(&s->pci, sizeof(s->pci), TYPE_AMD_IOMMU_PCI);
+object_initialize_child(obj, "iommu", &s->pci, TYPE_AMD_IOMMU_PCI);
 }
 
 static void amdvi_sysbus_class_init(ObjectClass *klass, void *data)
-- 
2.41.0

[PATCH 21/21] hw: Add QOM parentship relation with CPUs

[Philippe Mathieu-Daudé]

QDev objects created with object_new() need to manually add
their parent relationship with object_property_add_child().

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/i386/x86.c| 1 +
 hw/microblaze/petalogix_ml605_mmu.c  | 1 +
 hw/microblaze/petalogix_s3adsp1800_mmu.c | 1 +
 hw/mips/cps.c| 1 +
 hw/nios2/10m50_devboard.c| 1 +
 hw/ppc/e500.c| 1 +
 hw/ppc/spapr.c   | 1 +
 7 files changed, 7 insertions(+)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 684dce90e9..7021419d91 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -102,6 +102,7 @@ void x86_cpu_new(X86MachineState *x86ms, int64_t apic_id, 
Error **errp)
 if (!object_property_set_uint(cpu, "apic-id", apic_id, errp)) {
 goto out;
 }
+object_property_add_child(OBJECT(x86ms), "cpu[*]", OBJECT(cpu));
 qdev_realize(DEVICE(cpu), NULL, errp);
 
 out:
diff --git a/hw/microblaze/petalogix_ml605_mmu.c 
b/hw/microblaze/petalogix_ml605_mmu.c
index 0f5fabc32e..dfd881322d 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -83,6 +83,7 @@ petalogix_ml605_init(MachineState *machine)
 
 /* init CPUs */
 cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU));
+object_property_add_child(OBJECT(machine), "cpu", OBJECT(cpu));
 object_property_set_str(OBJECT(cpu), "version", "8.10.a", &error_abort);
 /* Use FPU but don't use floating point conversion and square
  * root instructions
diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c 
b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index dad46bd7f9..255d8d4d47 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -70,6 +70,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
 MemoryRegion *sysmem = get_system_memory();
 
 cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU));
+object_property_add_child(OBJECT(machine), "cpu", OBJECT(cpu));
 object_property_set_str(OBJECT(cpu), "version", "7.10.d", &error_abort);
 qdev_realize(DEVICE(cpu), NULL, &error_abort);
 
diff --git a/hw/mips/cps.c b/hw/mips/cps.c
index 07b73b0a1f..6b4e918807 100644
--- a/hw/mips/cps.c
+++ b/hw/mips/cps.c
@@ -84,6 +84,7 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
 /* All cores use the same clock tree */
 qdev_connect_clock_in(DEVICE(cpu), "clk-in", s->clock);
 
+object_property_add_child(OBJECT(dev), "cpu[*]", OBJECT(cpu));
 if (!qdev_realize_and_unref(DEVICE(cpu), NULL, errp)) {
 return;
 }
diff --git a/hw/nios2/10m50_devboard.c b/hw/nios2/10m50_devboard.c
index 6cb32f777b..f6a691d340 100644
--- a/hw/nios2/10m50_devboard.c
+++ b/hw/nios2/10m50_devboard.c
@@ -95,6 +95,7 @@ static void nios2_10m50_ghrd_init(MachineState *machine)
 cpu->exception_addr = 0xc8000120;
 cpu->fast_tlb_miss_addr = 0xc100;
 
+object_property_add_child(OBJECT(machine), "cpu", OBJECT(cpu));
 qdev_realize_and_unref(DEVICE(cpu), NULL, &error_fatal);
 
 if (nms->vic) {
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 3bd12b54ab..77b7d2858c 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -956,6 +956,7 @@ void ppce500_init(MachineState *machine)
  */
 object_property_set_bool(OBJECT(cs), "start-powered-off", i != 0,
  &error_abort);
+object_property_add_child(OBJECT(machine), "cpu[*]", OBJECT(cpu));
 qdev_realize_and_unref(DEVICE(cs), NULL, &error_fatal);
 
 if (!firstenv) {
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0d72d286d8..b6e5caa0d2 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2715,6 +2715,7 @@ static void spapr_init_cpus(SpaprMachineState *spapr)
 &error_fatal);
 object_property_set_int(core, CPU_CORE_PROP_CORE_ID, core_id,
 &error_fatal);
+object_property_add_child(OBJECT(spapr), "cpu[*]", OBJECT(core));
 qdev_realize(DEVICE(core), NULL, &error_fatal);
 
 object_unref(core);
-- 
2.41.0

[PATCH 16/21] hw/net/can/versal: Prefer object_initialize_child over object_initialize

[Philippe Mathieu-Daudé]

When the QOM parent is available, prefer object_initialize_child()
over object_initialize(), since it create the parent relationship.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/net/can/xlnx-versal-canfd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/can/xlnx-versal-canfd.c b/hw/net/can/xlnx-versal-canfd.c
index 47a14cfe63..f8e4bd75e4 100644
--- a/hw/net/can/xlnx-versal-canfd.c
+++ b/hw/net/can/xlnx-versal-canfd.c
@@ -1900,7 +1900,7 @@ static int canfd_populate_regarray(XlnxVersalCANFDState 
*s,
 int index = rae[i].addr / 4;
 RegisterInfo *r = &s->reg_info[index];
 
-object_initialize(r, sizeof(*r), TYPE_REGISTER);
+object_initialize_child(OBJECT(s), "reg[*]", r, TYPE_REGISTER);
 
 *r = (RegisterInfo) {
 .data = &s->regs[index],
-- 
2.41.0

Re: [RFC PATCH v4 10/10] util/bufferiszero: Add sve acceleration for aarch64

[Alex Bennée]

Richard Henderson  writes:

> Signed-off-by: Richard Henderson 
> ---
>
> RFC because I've not benchmarked this on real hw, only run it
> through qemu for validation.
>

>  
> +#ifdef CONFIG_SVE_OPT
> +static unsigned accel_index;
> +static void __attribute__((constructor)) init_accel(void)
> +{
> +accel_index = (cpuinfo & CPUINFO_SVE ? 2 : 1);
> +buffer_is_zero_accel = accel_table[accel_index];
> +}

This really needs to be:

  -accel_index = (cpuinfo & CPUINFO_SVE ? 2 : 1);
  +unsigned info = cpuinfo_init();
  +accel_index = (info & CPUINFO_SVE ? 2 : 1);

because otherwise you are relying on constructor initialisation order
and on the Graviton 3 I built on it wasn't detecting the SVE. With that I
get this from "perf record ./tests/unit/test-bufferiszero -m thorough"

  51.17%  test-bufferisze  test-bufferiszero  [.] buffer_is_zero_sve
  18.92%  test-bufferisze  test-bufferiszero  [.] buffer_is_zero_simd
  18.02%  test-bufferisze  test-bufferiszero  [.] buffer_is_zero_int_ge256
   7.67%  test-bufferisze  test-bufferiszero  [.] buffer_is_zero_ool
   4.09%  test-bufferisze  test-bufferiszero  [.] test_1

but as I mentioned before it would be nice to have a proper benchmark
for the buffer utils as I'm sure the unit test would be prone to noise.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH 10/21] hw/usb: Inline usb_new()

[Philippe Mathieu-Daudé]

Inline the 2 uses of usb_new().

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/usb.h| 1 -
 hw/usb/bus.c| 9 ++---
 hw/usb/dev-serial.c | 2 +-
 3 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/include/hw/usb.h b/include/hw/usb.h
index 32c23a5ca2..2d820685cc 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -500,7 +500,6 @@ void usb_bus_release(USBBus *bus);
 USBBus *usb_bus_find(int busnr);
 void usb_legacy_register(const char *typename, const char *usbdevice_name,
  USBDevice *(*usbdevice_init)(void));
-USBDevice *usb_new(const char *name);
 bool usb_realize_and_unref(USBDevice *dev, USBBus *bus, Error **errp);
 USBDevice *usb_create_simple(USBBus *bus, const char *name);
 USBDevice *usbdevice_create(const char *cmdline);
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 148224f06a..a599e2552b 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -329,11 +329,6 @@ void usb_legacy_register(const char *typename, const char 
*usbdevice_name,
 }
 }
 
-USBDevice *usb_new(const char *name)
-{
-return USB_DEVICE(qdev_new(name));
-}
-
 bool usb_realize_and_unref(USBDevice *dev, USBBus *bus, Error **errp)
 {
 return qdev_realize_and_unref(&dev->qdev, &bus->qbus, errp);
@@ -341,7 +336,7 @@ bool usb_realize_and_unref(USBDevice *dev, USBBus *bus, 
Error **errp)
 
 USBDevice *usb_create_simple(USBBus *bus, const char *name)
 {
-USBDevice *dev = usb_new(name);
+USBDevice *dev = USB_DEVICE(qdev_new(name));
 
 usb_realize_and_unref(dev, bus, &error_abort);
 return dev;
@@ -693,7 +688,7 @@ USBDevice *usbdevice_create(const char *driver)
 return NULL;
 }
 
-dev = f->usbdevice_init ? f->usbdevice_init() : usb_new(f->name);
+dev = f->usbdevice_init ? f->usbdevice_init() : 
USB_DEVICE(qdev_new(f->name));
 if (!dev) {
 error_report("Failed to create USB device '%s'", f->name);
 return NULL;
diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 63047d79cf..6e79c46d53 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -624,7 +624,7 @@ static USBDevice *usb_braille_init(void)
 return NULL;
 }
 
-dev = usb_new("usb-braille");
+dev = USB_DEVICE(qdev_new("usb-braille"));
 qdev_prop_set_chr(&dev->qdev, "chardev", cdrv);
 return dev;
 }
-- 
2.41.0

[PATCH 03/21] hw/ppc/spapr_cpu: Use qdev_is_realized() instead of QOM API

[Philippe Mathieu-Daudé]

Prefer QDev API for QDev objects, avoid the underlying QOM layer.

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

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 0c0fb3f1b0..40b7c52f7f 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -245,8 +245,7 @@ static void spapr_cpu_core_unrealize(DeviceState *dev)
  * spapr_cpu_core_realize(), make sure we only unrealize
  * vCPUs that have already been realized.
  */
-if (object_property_get_bool(OBJECT(sc->threads[i]), "realized",
- &error_abort)) {
+if (qdev_is_realized(DEVICE(sc->threads[i]))) {
 spapr_unrealize_vcpu(sc->threads[i], sc);
 }
 spapr_delete_vcpu(sc->threads[i]);
-- 
2.41.0

[PATCH 08/21] hw/isa: Inline isa_try_new()

[Philippe Mathieu-Daudé]

Inline the 2 single uses of isa_try_new().

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/isa/isa.h| 1 -
 include/hw/net/ne2000-isa.h | 2 +-
 hw/i386/pc.c| 2 +-
 hw/isa/isa-bus.c| 5 -
 4 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 40d6224a4e..8475120849 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -81,7 +81,6 @@ IsaDma *isa_bus_get_dma(ISABus *bus, int nchan);
  */
 qemu_irq isa_bus_get_irq(ISABus *bus, unsigned irqnum);
 ISADevice *isa_new(const char *name);
-ISADevice *isa_try_new(const char *name);
 bool isa_realize_and_unref(ISADevice *dev, ISABus *bus, Error **errp);
 ISADevice *isa_create_simple(ISABus *bus, const char *name);
 
diff --git a/include/hw/net/ne2000-isa.h b/include/hw/net/ne2000-isa.h
index 73bae10ad1..2440ac8621 100644
--- a/include/hw/net/ne2000-isa.h
+++ b/include/hw/net/ne2000-isa.h
@@ -22,7 +22,7 @@ static inline ISADevice *isa_ne2000_init(ISABus *bus, int 
base, int irq,
 {
 ISADevice *d;
 
-d = isa_try_new(TYPE_ISA_NE2000);
+d = ISA_DEVICE(qdev_try_new(TYPE_ISA_NE2000));
 if (d) {
 DeviceState *dev = DEVICE(d);
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 3c00a87317..e8130774ad 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1183,7 +1183,7 @@ static void pc_superio_init(ISABus *isa_bus, bool 
create_fdctrl,
 i8042 = isa_create_simple(isa_bus, TYPE_I8042);
 if (!no_vmport) {
 isa_create_simple(isa_bus, TYPE_VMPORT);
-vmmouse = isa_try_new("vmmouse");
+vmmouse = ISA_DEVICE(qdev_try_new("vmmouse"));
 } else {
 vmmouse = NULL;
 }
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index f1e0f14007..8aaf44a3ef 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -158,11 +158,6 @@ ISADevice *isa_new(const char *name)
 return ISA_DEVICE(qdev_new(name));
 }
 
-ISADevice *isa_try_new(const char *name)
-{
-return ISA_DEVICE(qdev_try_new(name));
-}
-
 ISADevice *isa_create_simple(ISABus *bus, const char *name)
 {
 ISADevice *dev;
-- 
2.41.0

Re: [PATCH 05/21] hw/ppc/pnv_bmc: Use qdev_new() instead of QOM API

[Cédric Le Goater]

On 2/16/24 12:02, Philippe Mathieu-Daudé wrote:

Prefer QDev API for QDev objects, avoid the underlying QOM layer.

Signed-off-by: Philippe Mathieu-Daudé 



Reviewed-by: Cédric Le Goater 

Thanks,

C.



---
  hw/ppc/pnv_bmc.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c
index 99f1e8d7f9..0c1274df21 100644
--- a/hw/ppc/pnv_bmc.c
+++ b/hw/ppc/pnv_bmc.c
@@ -269,13 +269,13 @@ void pnv_bmc_set_pnor(IPMIBmc *bmc, PnvPnor *pnor)
   */
  IPMIBmc *pnv_bmc_create(PnvPnor *pnor)
  {
-Object *obj;
+DeviceState *dev;
  
-obj = object_new(TYPE_IPMI_BMC_SIMULATOR);

-qdev_realize(DEVICE(obj), NULL, &error_fatal);
-pnv_bmc_set_pnor(IPMI_BMC(obj), pnor);
+dev = qdev_new(TYPE_IPMI_BMC_SIMULATOR);
+qdev_realize(dev, NULL, &error_fatal);
+pnv_bmc_set_pnor(IPMI_BMC(dev), pnor);
  
-return IPMI_BMC(obj);

+return IPMI_BMC(dev);
  }
  
  typedef struct ForeachArgs {

[PATCH 04/21] hw/tricore/testboard: Use qdev_new() instead of QOM basic API

[Philippe Mathieu-Daudé]

Prefer QDev API for QDev objects, avoid the underlying QOM layer.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/tricore/tricore_testdevice.h | 3 ---
 hw/tricore/tricore_testboard.c  | 4 +---
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/include/hw/tricore/tricore_testdevice.h 
b/include/hw/tricore/tricore_testdevice.h
index 8b4fe15f24..2c57b62f22 100644
--- a/include/hw/tricore/tricore_testdevice.h
+++ b/include/hw/tricore/tricore_testdevice.h
@@ -25,12 +25,9 @@
 OBJECT_CHECK(TriCoreTestDeviceState, (obj), TYPE_TRICORE_TESTDEVICE)
 
 typedef struct {
-/*  */
 SysBusDevice parent_obj;
 
-/*  */
 MemoryRegion iomem;
-
 } TriCoreTestDeviceState;
 
 #endif
diff --git a/hw/tricore/tricore_testboard.c b/hw/tricore/tricore_testboard.c
index b6810e3be0..c29db8b451 100644
--- a/hw/tricore/tricore_testboard.c
+++ b/hw/tricore/tricore_testboard.c
@@ -89,9 +89,7 @@ static void tricore_testboard_init(MachineState *machine, int 
board_id)
 memory_region_add_subregion(sysmem, 0xf005, pcp_data);
 memory_region_add_subregion(sysmem, 0xf006, pcp_text);
 
-test_dev = g_new(TriCoreTestDeviceState, 1);
-object_initialize(test_dev, sizeof(TriCoreTestDeviceState),
-  TYPE_TRICORE_TESTDEVICE);
+test_dev = TRICORE_TESTDEVICE(qdev_new(TYPE_TRICORE_TESTDEVICE));
 memory_region_add_subregion(sysmem, 0xf000, &test_dev->iomem);
 
 
-- 
2.41.0

[PATCH 19/21] hw/s390x/zpci-bus: Add QOM parentship relation with zPCI devices

[Philippe Mathieu-Daudé]

QDev objects created with qdev_*new() need to manually add
their parent relationship with object_property_add_child().

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/s390x/s390-pci-bus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 3e57d5faca..6d07a7b530 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -934,6 +934,7 @@ static S390PCIBusDevice *s390_pci_device_new(S390pciState 
*s,
 "zPCI device could not be created: ");
 return NULL;
 }
+object_property_add_child(OBJECT(s), "zpci[*]", OBJECT(dev));
 if (!qdev_realize_and_unref(dev, BUS(s->bus), &local_err)) {
 object_unparent(OBJECT(dev));
 error_propagate_prepend(errp, local_err,
-- 
2.41.0

[PATCH 15/21] hw/core/register: Prefer object_initialize_child over object_initialize

[Philippe Mathieu-Daudé]

When the QOM parent is available, prefer object_initialize_child()
over object_initialize(), since it create the parent relationship.

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

diff --git a/hw/core/register.c b/hw/core/register.c
index 95b0150c0a..b6beca0e02 100644
--- a/hw/core/register.c
+++ b/hw/core/register.c
@@ -259,7 +259,7 @@ static RegisterInfoArray *register_init_block(DeviceState 
*owner,
 RegisterInfo *r = &ri[index];
 
 /* Init the register, this will zero it. */
-object_initialize((void *)r, sizeof(*r), TYPE_REGISTER);
+object_initialize_child(OBJECT(owner), "reg[*]", r, TYPE_REGISTER);
 
 /* Set the properties of the register */
 r->data = data + data_size * index;
-- 
2.41.0

Re: [PATCH v3 1/9] usb: inline device creation functions

[Philippe Mathieu-Daudé]

On 13/2/24 16:49, Paolo Bonzini wrote:

Allow boards to use the device creation functions even if USB itself
is not available; of course the functions will fail inexorably, but
this can be okay if the calls are conditional on the existence of
some USB host controller device.  This is for example the case for
hw/mips/loongson3_virt.c.

Acked-by: Richard Henderson 
Signed-off-by: Paolo Bonzini 
---
  include/hw/usb.h | 27 ---
  hw/usb/bus.c | 23 ---
  2 files changed, 24 insertions(+), 26 deletions(-)


See alternatives:
https://lore.kernel.org/qemu-devel/20240216110313.17039-10-phi...@linaro.org/
https://lore.kernel.org/qemu-devel/20240216110313.17039-11-phi...@linaro.org/

[PATCH 1/3] trans_rvv.c.inc: write CSRs must call mark_vs_dirty() too

[Daniel Henrique Barboza]

In the Vector spec section 3.2 [1]:

"When mstatus.VS is set to Initial or Clean, executing any instruction
 that changes vector state, including the vector CSRs, will change
 mstatus.VS to Dirty."

ldst_us_trans(), ldst_stride_trans(), ldst_index_trans() and
ldst_whole_trans() will change vector state regardless of being a store
op or not. Stores will set env->vstart to zero after execution (see
vext_ldst_us() in vector_helper.c), and this is vector CSR state change.

[1] 
https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#vector-start-index-csr-vstart

Fixes: 8e1ee1fb57 ("target/riscv: rvv-1.0: add translation-time vector context 
status")
Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/insn_trans/trans_rvv.c.inc | 19 ---
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index 9e101ab434..044c9c903e 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -638,10 +638,7 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1, 
uint32_t data,
 
 fn(dest, mask, base, tcg_env, desc);
 
-if (!is_store) {
-mark_vs_dirty(s);
-}
-
+mark_vs_dirty(s);
 gen_set_label(over);
 return true;
 }
@@ -799,10 +796,7 @@ static bool ldst_stride_trans(uint32_t vd, uint32_t rs1, 
uint32_t rs2,
 
 fn(dest, mask, base, stride, tcg_env, desc);
 
-if (!is_store) {
-mark_vs_dirty(s);
-}
-
+mark_vs_dirty(s);
 gen_set_label(over);
 return true;
 }
@@ -906,10 +900,7 @@ static bool ldst_index_trans(uint32_t vd, uint32_t rs1, 
uint32_t vs2,
 
 fn(dest, mask, base, index, tcg_env, desc);
 
-if (!is_store) {
-mark_vs_dirty(s);
-}
-
+mark_vs_dirty(s);
 gen_set_label(over);
 return true;
 }
@@ -1104,9 +1095,7 @@ static bool ldst_whole_trans(uint32_t vd, uint32_t rs1, 
uint32_t nf,
 
 fn(dest, base, tcg_env, desc);
 
-if (!is_store) {
-mark_vs_dirty(s);
-}
+mark_vs_dirty(s);
 gen_set_label(over);
 
 return true;
-- 
2.43.0

[PATCH 0/3] riscv: set vstart_eq_zero on mark_vs_dirty

[Daniel Henrique Barboza]

Hi,

This is my shot to fix https://gitlab.com/qemu-project/qemu/-/issues/1976.

First patch ensures that every vector instruction that changes the
vector state will call mark_vs_dirty(). Second patch is a trivial
simplification.

Third patch is where the bug is solved: check if 'vstart' is zeroed and
set vstart_eq_zero accordingly. 

Patches based on alistair/riscv-to-apply.next. It can also be fetched
here:

https://gitlab.com/danielhb/qemu/-/tree/vstart_bug1976_v1


Daniel Henrique Barboza (3):
  trans_rvv.c.inc: write CSRs must call mark_vs_dirty() too
  trans_rvv.c.inc: remove redundant mark_vs_dirty() calls
  target/riscv/translate.c: set vstart_eq_zero in mark_vs_dirty()

 target/riscv/insn_trans/trans_rvv.c.inc | 28 +++--
 target/riscv/translate.c| 22 +++
 2 files changed, 29 insertions(+), 21 deletions(-)

-- 
2.43.0

[PATCH 3/3] target/riscv/translate.c: set vstart_eq_zero in mark_vs_dirty()

[Daniel Henrique Barboza]

The 'vstart_eq_zero' flag which is used to determine if some insns, like
vector reductor operations, should SIGILL. At this moment the flag is
being updated only during cpu_get_tb_cpu_state(), at the start of each
translation block.

This cadence isn't enough and we're facing situations where a vector
instruction successfully updated 'vstart' to zero, but the flag was
still marked as 'false', resulting in a SIGILL because instructions are
checking the flag.

mark_vs_dirty() is called after any instruction changes Vector CSR
state, making it a good place to update 'vstart_eq_zero'.

Fixes: 8e1ee1fb57 ("target/riscv: rvv-1.0: add translation-time vector context 
status")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1976
Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/translate.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 177418b2b9..9943538894 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -652,9 +652,13 @@ static inline void mark_fs_dirty(DisasContext *ctx) { }
  */
 static void mark_vs_dirty(DisasContext *ctx)
 {
+TCGLabel *vstart_zero, *done;
 TCGv tmp;
 
 if (ctx->mstatus_vs != EXT_STATUS_DIRTY) {
+vstart_zero = gen_new_label();
+done = gen_new_label();
+
 /* Remember the state change for the rest of the TB.  */
 ctx->mstatus_vs = EXT_STATUS_DIRTY;
 
@@ -668,6 +672,24 @@ static void mark_vs_dirty(DisasContext *ctx)
 tcg_gen_ori_tl(tmp, tmp, MSTATUS_VS);
 tcg_gen_st_tl(tmp, tcg_env, offsetof(CPURISCVState, mstatus_hs));
 }
+
+/*
+ * We can safely make 'vl_eq_vlmax = false' if we marked
+ * VS as dirty with non-zero 'vstart', i.e. there's a fault
+ * to be handled. If 'vstart' is zero then we should retain
+ * the existing 'vl_eq_vlmax' - it'll be recalculated on the
+ * start of the next TB or during vset{i}vl{i} (that forces a
+ * TB end).
+ */
+tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vstart, 0, vstart_zero);
+ctx->vstart_eq_zero = false;
+ctx->vl_eq_vlmax = false;
+tcg_gen_br(done);
+
+gen_set_label(vstart_zero);
+ctx->vstart_eq_zero = true;
+
+gen_set_label(done);
 }
 }
 #else
-- 
2.43.0

[PATCH 2/3] trans_rvv.c.inc: remove redundant mark_vs_dirty() calls

[Daniel Henrique Barboza]

trans_vmv_v_i , trans_vfmv_v_f and the trans_##NAME macro from
GEN_VMV_WHOLE_TRANS() are calling mark_vs_dirty() in both branches of
their 'ifs'. conditionals.

Call it just once in the end like other functions are doing.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/insn_trans/trans_rvv.c.inc | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index 044c9c903e..778e2723dd 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -2094,7 +2094,6 @@ static bool trans_vmv_v_i(DisasContext *s, arg_vmv_v_i *a)
 if (s->vl_eq_vlmax && !(s->vta && s->lmul < 0)) {
 tcg_gen_gvec_dup_imm(s->sew, vreg_ofs(s, a->rd),
  MAXSZ(s), MAXSZ(s), simm);
-mark_vs_dirty(s);
 } else {
 TCGv_i32 desc;
 TCGv_i64 s1;
@@ -2115,9 +2114,9 @@ static bool trans_vmv_v_i(DisasContext *s, arg_vmv_v_i *a)
 tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, a->rd));
 fns[s->sew](dest, s1, tcg_env, desc);
 
-mark_vs_dirty(s);
 gen_set_label(over);
 }
+mark_vs_dirty(s);
 return true;
 }
 return false;
@@ -2660,7 +2659,6 @@ static bool trans_vfmv_v_f(DisasContext *s, arg_vfmv_v_f 
*a)
 
 tcg_gen_gvec_dup_i64(s->sew, vreg_ofs(s, a->rd),
  MAXSZ(s), MAXSZ(s), t1);
-mark_vs_dirty(s);
 } else {
 TCGv_ptr dest;
 TCGv_i32 desc;
@@ -2686,9 +2684,9 @@ static bool trans_vfmv_v_f(DisasContext *s, arg_vfmv_v_f 
*a)
 
 fns[s->sew - 1](dest, t1, tcg_env, desc);
 
-mark_vs_dirty(s);
 gen_set_label(over);
 }
+mark_vs_dirty(s);
 return true;
 }
 return false;
@@ -3632,15 +3630,14 @@ static bool trans_##NAME(DisasContext *s, arg_##NAME * 
a)   \
 if (s->vstart_eq_zero) {\
 tcg_gen_gvec_mov(s->sew, vreg_ofs(s, a->rd),\
  vreg_ofs(s, a->rs2), maxsz, maxsz);\
-mark_vs_dirty(s);   \
 } else {\
 TCGLabel *over = gen_new_label();   \
 tcg_gen_brcondi_tl(TCG_COND_GEU, cpu_vstart, maxsz, over);  \
 tcg_gen_gvec_2_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2), \
tcg_env, maxsz, maxsz, 0, gen_helper_vmvr_v); \
-mark_vs_dirty(s);   \
 gen_set_label(over);\
 }   \
+mark_vs_dirty(s);   \
 return true;\
 }   \
 return false;   \
-- 
2.43.0

Re: [PATCH] .gitlab-ci/windows.yml: Don't install libusb or spice packages on 32-bit

[Alex Bennée]

Peter Maydell  writes:

> When msys2 updated their libusb packages to libusb 1.0.27, they
> dropped support for building them for mingw32, leaving only mingw64
> packages.  This broke our CI job, as the 'pacman' package install now
> fails with:
>
> error: target not found: mingw-w64-i686-libusb
> error: target not found: mingw-w64-i686-usbredir
>
> (both these binary packages are from the libusb source package).
>
> Similarly, spice is now 64-bit only:
> error: target not found: mingw-w64-i686-spice
>
> Fix this by dropping these packages from the list we install for our
> msys2-32bit build.  We do this with a simple mechanism for the
> msys2-64bit and msys2-32bit jobs to specify a list of extra packages
> to install on top of the common ones we install for both jobs.
>
> Cc: qemu-sta...@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2160
> Signed-off-by: Peter Maydell 

Queued to testing/next, thanks.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v4 02/36] linux-user: Adjust SVr4 NULL page mapping

[Alex Bennée]

Richard Henderson  writes:

> Use TARGET_PAGE_SIZE and MAP_FIXED_NOREPLACE.
>
> We really should be attending to this earlier during
> probe_guest_base, as well as better detection and
> emulation of various Linux personalities.

Do we know all our supported systems support this flag now? 

>
> Signed-off-by: Richard Henderson 
> Reviewed-by: Ilya Leoshkevich 
> Reviewed-by: Pierrick Bouvier 
> Acked-by: Helge Deller 
> Message-Id: <20240102015808.132373-3-richard.hender...@linaro.org>
> ---
>  linux-user/elfload.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index b8eef893d0..e918a13748 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -3912,8 +3912,9 @@ int load_elf_binary(struct linux_binprm *bprm, struct 
> image_info *info)
> and some applications "depend" upon this behavior.  Since
> we do not have the power to recompile these, we emulate
> the SVr4 behavior.  Sigh.  */
> -target_mmap(0, qemu_host_page_size, PROT_READ | PROT_EXEC,
> -MAP_FIXED | MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +target_mmap(0, TARGET_PAGE_SIZE, PROT_READ | PROT_EXEC,
> +MAP_FIXED_NOREPLACE | MAP_PRIVATE | MAP_ANONYMOUS,
> +-1, 0);
>  }
>  #ifdef TARGET_MIPS
>  info->interp_fp_abi = interp_info.fp_abi;

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: QEMU 8.2.0 aarch64 sve ldff1b returning 1 byte when 16 are expected

[Alex Bennée]

Mark Charney OS  writes:

> Using QEMU v8.2.0 (and also the HEAD of the git master branch), I
> encountered an unexpected situation: an ldff1b is returning 1 byte
> when I run with the QEMU user level plugin (and setting FFR as if
> there was a fault).
>
> However the ldff1b actually loads 16 bytes when: (a) I run this same
> test natively on a system with SVE support (no QEMU involved) or (b)
> when I run this test interactively (logged in to a console) in GDB
> running on the QEMU (with no plugin involved).
>
> I was wondering if this one-byte-per-ldff1b was a known/expected
> behavior with plugins?  I guess it is legal to only return one byte,
> but I was wondering why QEMU did this and if there was some way to get
> QEMU to return 16 bytes in the absence of faults (or as many bytes as
> it can up until the fault).

Could it be a change with the location w.r.t a page boundary between the
two cases?

>
> There is *no* page boundary being crossed in the examples of interest,
> and no MMIO, so a partial data return is not expected. The page
> referenced is mapped and previously referenced.
>
> Talking to Alex Bennee, he pointed out:
>
>> I'm wondering if this is a result of the fix in 6d03226b422
>> (plugins: force slow path when plugins instrument memory ops). This
>> will always force the slow path which is where we instrument the
>> operation.
>
> I attempted to revert this commit locally and no longer got memop
> callbacks for any SVE load operations, first fault, nonfault or not
> "normal" predicated SVE operations. But I believe ldff1b are returning
> 16 bytes (judging by the control flow).
>
> Our goal is to use QEMU for tracing with a home-grown plugin.  For our
> purposes, we were expecting to observe control flow like what we see
> on SVE-enabled hardware where ldff1b returns 16 bytes in the absence
> of faults.
>
> If necessary, I can provide a reproducer, that includes:
>   - a sve strcpy loop from one of Alex's talks.

Yeah lets add the test case.

>   - a simple user level plugin

It should show up with execlog as well right?

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v5 0/3] linux-user: Allow gdbstub to ignore page protection

[Alex Bennée]

Ilya Leoshkevich  writes:

> v4: https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg05857.html
> v4 -> v5: Fix the probe_proc_self_mem vs probe_proc_self_mem() typo.
>   Add Alex's R-b and A-b.

I was going to pull this and realised it already went in via Richard's
tcg-next. Did this fix get merged?

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

RE: [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub control feature

[Shiju Jose via]

Hi Davidlohr,

Thanks for the feedback.
Please find reply inline.

>-Original Message-
>From: Davidlohr Bueso 
>Sent: 15 February 2024 20:56
>To: Shiju Jose 
>Cc: qemu-devel@nongnu.org; linux-...@vger.kernel.org; Jonathan Cameron
>; tanxiaofei ;
>Zengtao (B) ; Linuxarm ;
>fan...@samsung.com
>Subject: Re: [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub
>control feature
>
>On Fri, 24 Nov 2023, shiju.j...@huawei.com wrote:
>
>>From: Shiju Jose 
>>
>>CXL spec 3.1 section 8.2.9.9.11.1 describes the device patrol scrub
>>control feature. The device patrol scrub proactively locates and makes
>>corrections to errors in regular cycle. The patrol scrub control allows
>>the request to configure patrol scrub input configurations.
>>
>>The patrol scrub control allows the requester to specify the number of
>>hours for which the patrol scrub cycles must be completed, provided
>>that the requested number is not less than the minimum number of hours
>>for the patrol scrub cycle that the device is capable of. In addition,
>>the patrol scrub controls allow the host to disable and enable the
>>feature in case disabling of the feature is needed for other purposes
>>such as performance-aware operations which require the background
>>operations to be turned off.
>>
>>Reviewed-by: Davidlohr Bueso 
>>Signed-off-by: Shiju Jose 
>>---
>> hw/cxl/cxl-mailbox-utils.c | 97 +-
>> 1 file changed, 96 insertions(+), 1 deletion(-)
>>
>>diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>>index 1bbc9a48a6..5a6f4e4029 100644
>>--- a/hw/cxl/cxl-mailbox-utils.c
>>+++ b/hw/cxl/cxl-mailbox-utils.c
>>@@ -809,6 +809,7 @@ typedef struct CXLSupportedFeatureEntry {  }
>>QEMU_PACKED CXLSupportedFeatureEntry;
>>
>> enum CXL_SUPPORTED_FEATURES_LIST {
>>+CXL_FEATURE_PATROL_SCRUB = 0,
>> CXL_FEATURE_MAX
>> };
>>
>>@@ -849,6 +850,37 @@ enum CXL_SET_FEATURE_FLAG_DATA_TRANSFER {
>> CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MAX
>> };
>>
>>+/* CXL r3.1 section 8.2.9.9.11.1: Device Patrol Scrub Control Feature
>>+*/ static const QemuUUID patrol_scrub_uuid = {
>>+.data = UUID(0x96dad7d6, 0xfde8, 0x482b, 0xa7, 0x33,
>>+ 0x75, 0x77, 0x4e, 0x06, 0xdb, 0x8a) };
>>+
>>+#define CXL_MEMDEV_PS_GET_FEATURE_VERSION0x01
>>+#define CXL_MEMDEV_PS_SET_FEATURE_VERSION0x01
>>+#define CXL_MEMDEV_PS_SCRUB_CYCLE_CHANGE_CAP_DEFAULTBIT(0)
>>+#define CXL_MEMDEV_PS_SCRUB_REALTIME_REPORT_CAP_DEFAULT
>BIT(1)
>>+#define CXL_MEMDEV_PS_CUR_SCRUB_CYCLE_DEFAULT12
>>+#define CXL_MEMDEV_PS_MIN_SCRUB_CYCLE_DEFAULT1
>>+#define CXL_MEMDEV_PS_ENABLE_DEFAULT0
>>+
>>+/* CXL memdev patrol scrub control attributes */ struct
>>+CXLMemPatrolScrubReadAttrbs {
>>+uint8_t scrub_cycle_cap;
>>+uint16_t scrub_cycle;
>>+uint8_t scrub_flags;
>>+} QEMU_PACKED cxl_memdev_ps_feat_read_attrbs;
>>+
>>+typedef struct CXLMemPatrolScrubWriteAttrbs {
>>+uint8_t scrub_cycle_hr;
>>+uint8_t scrub_flags;
>>+} QEMU_PACKED CXLMemPatrolScrubWriteAttrbs;
>
>fyi there is an ask, which I certainly agree with, to make these static here
>instead of at runtime.
I will make cxl_memdev_ps_feat_read_attrbs static, however can't make const 
because 
cxl_memdev_ps_feat_read_attrbs use to store the attributes  in the set_feature.
May be rename cxl_memdev_ps_feat_read_attrbs to cxl_memdev_ps_feat_ attrbs to 
avoid confusion?
  
>
>https://lore.kernel.org/linux-cxl/20240119175006.7...@huawei.com/
>
>Also, this series probably needs rebasing per Jonathan's latest branch with 
>lots of
>updates.

The v3 posted recently was rebased to 
Jonathan's recent branch
https://gitlab.com/jic23/qemu/-/tree/cxl-2024-02-05-draft

Looks like latest branch is cxl-2024-02-14. I will rebase.
 
https://lore.kernel.org/qemu-devel/20240215110146.1444-1-shiju.j...@huawei.com/T/#t
 
>
>>+
>>+typedef struct CXLMemPatrolScrubSetFeature {
>>+CXLSetFeatureInHeader hdr;
>>+CXLMemPatrolScrubWriteAttrbs feat_data; } QEMU_PACKED
>>+QEMU_ALIGNED(16) CXLMemPatrolScrubSetFeature;
>>+
>> /* CXL r3.0 section 8.2.9.6.1: Get Supported Features (Opcode 0500h)
>>*/  static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
>>  uint8_t *payload_in, @@
>>-872,7 +904,7 @@ static CXLRetCode cmd_features_get_supported(const
>struct cxl_cmd *cmd,
>> uint16_t feat_entries = 0;
>>
>> if (get_feats_in->count < sizeof(CXLSupportedFeatureHeader) ||
>>-get_feats_in->start_index > CXL_FEATURE_MAX) {
>>+get_feats_in->start_index >= CXL_FEATURE_MAX) {
>> return CXL_MBOX_INVALID_INPUT;
>> }
>> req_entries = (get_feats_in->count - @@ -884,6 +916,31 @@ static
>>CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
>> entry = 0;
>> while (entry < req_entries) {
>> switch (index) {
>>+case  CXL_FEATURE_PATROL_SCRUB:
>>+/* Fill supported feature entry for device patrol scrub control 

Re: [RFC PATCH] include/qom/object.h: New OBJECT_DEFINE_SIMPLE_TYPE{,_WITH_INTEFACES} macros

[Daniel P . Berrangé]

On Mon, Feb 12, 2024 at 05:49:25PM +, Peter Maydell wrote:
> We have an OBJECT_DEFINE_TYPE_EXTENDED macro, plus several variations
> on it, which emits the boilerplate for the TypeInfo and ensures it is
> registered with the type system.  However, all the existing macros
> insist that the type being defined has its own FooClass struct, so
> they aren't useful for the common case of a simple leaf class which
> doesn't have any new methods or any other need for its own class
> struct (that is, for the kind of type that OBJECT_DECLARE_SIMPLE_TYPE
> declares).
> 
> Pull the actual implementation of OBJECT_DEFINE_TYPE_EXTENDED out
> into a new DO_OBJECT_DEFINE_TYPE_EXTENDED which parameterizes the
> value we use for the class_size field.  This lets us add a new
> OBJECT_DEFINE_SIMPLE_TYPE which does the same job as the various
> existing OBJECT_DEFINE_*_TYPE_* family macros for this kind of simple
> type, and the variant OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES for
> when the type will implement some interfaces.
> 
> Signed-off-by: Peter Maydell 
> ---
> I've marked this RFC largely because this patch doesn't include
> any uses of the new macros. I wanted them for a series I'm currently
> working on, but I wanted to send this out early so it could have an
> extended review period and a bit more visibility than if I stuck
> it inside an 8 patch series about some other topic. (In fact, this
> is the second time I looked at the OBJECT_DEFINE_* macros and found
> they weren't suitable for the common case kind of type. The first
> time around I went back to writing the type out the old fashioned
> way, but this time I figured I'd try improving the macros.)

The macros were intended to simplify QOM boilerplate. So given that
they're not currently addressing a common use case of no-Class objects,
it makes sense to extend the macros as you suggest here.

>  docs/devel/qom.rst   |  34 +++--
>  include/qom/object.h | 114 +--
>  2 files changed, 117 insertions(+), 31 deletions(-)

Reviewed-by: Daniel P. Berrangé 

> 
> diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst
> index 9918fac7f21..0889ca949c1 100644
> --- a/docs/devel/qom.rst
> +++ b/docs/devel/qom.rst
> @@ -348,12 +348,14 @@ used. This does the same as 
> OBJECT_DECLARE_SIMPLE_TYPE(), but without
>  the 'struct MyDeviceClass' definition.
>  
>  To implement the type, the OBJECT_DEFINE macro family is available.
> -In the simple case the OBJECT_DEFINE_TYPE macro is suitable:
> +For the simplest case of a leaf class which doesn't need any of its
> +own virtual functions (i.e. which was declared with 
> OBJECT_DECLARE_SIMPLE_TYPE)
> +the OBJECT_DEFINE_SIMPLE_TYPE macro is suitable:
>  
>  .. code-block:: c
> :caption: Defining a simple type
>  
> -   OBJECT_DEFINE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)
> +   OBJECT_DEFINE_SIMPLE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)
>  
>  This is equivalent to the following:
>  
> @@ -370,7 +372,6 @@ This is equivalent to the following:
> .instance_size = sizeof(MyDevice),
> .instance_init = my_device_init,
> .instance_finalize = my_device_finalize,
> -   .class_size = sizeof(MyDeviceClass),
> .class_init = my_device_class_init,
> };
>  
> @@ -385,13 +386,36 @@ This is sufficient to get the type registered with the 
> type
>  system, and the three standard methods now need to be implemented
>  along with any other logic required for the type.
>  
> +If the class needs its own virtual methods, or has some other
> +per-class state it needs to store in its own class struct,
> +then you can use the OBJECT_DEFINE_TYPE macro. This does the
> +same thing as OBJECT_DEFINE_SIMPLE_TYPE, but it also sets the
> +class_size of the type to the size of the class struct.
> +
> +.. code-block:: c
> +   :caption: Defining a type which needs a class struct
> +
> +   OBJECT_DEFINE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)
> +
>  If the type needs to implement one or more interfaces, then the
> -OBJECT_DEFINE_TYPE_WITH_INTERFACES() macro can be used instead.
> -This accepts an array of interface type names.
> +OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES() and
> +OBJECT_DEFINE_TYPE_WITH_INTERFACES() macros can be used instead.
> +These accept an array of interface type names. The difference between
> +them is that the former is for simple leaf classes that don't need
> +a class struct, and the latter is for when you will be defining
> +a class struct.
>  
>  .. code-block:: c
> :caption: Defining a simple type implementing interfaces
>  
> +   OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(MyDevice, my_device,
> + MY_DEVICE, DEVICE,
> + { TYPE_USER_CREATABLE },
> + { NULL })
> +
> +.. code-block:: c
> +   :caption: Defining a type implementing interfaces
> +
> OBJECT_DEFINE_TYPE_WITH_INTERFACES(MyDevice, my_d

Re: [PATCH v5 0/3] linux-user: Allow gdbstub to ignore page protection

[Ilya Leoshkevich]

On Fri, 2024-02-16 at 11:44 +, Alex Bennée wrote:
> Ilya Leoshkevich  writes:
> 
> > v4:
> > https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg05857.html
> > v4 -> v5: Fix the probe_proc_self_mem vs probe_proc_self_mem()
> > typo.
> >   Add Alex's R-b and A-b.
> 
> I was going to pull this and realised it already went in via
> Richard's
> tcg-next. Did this fix get merged?
> 

Hi, yes, I submitted it separately and it was merged as:

commit da4038d2da6d3a3d5f86665bd51b2ba49df5d652
Author: Ilya Leoshkevich 
Date:   Wed Jan 31 23:02:18 2024 +0100

tests/tcg: Fix the /proc/self/mem probing in the PROT_NONE gdbstub
test

Re: [PATCH 1/4] target/riscv: Add CSR tcontrol of debug trigger module

[Daniel Henrique Barboza]

On 2/16/24 03:13, Alvin Chang wrote:

The RISC-V debug specification defines an optional CSR "tcontrol" within
the trigger module. This commit adds its read/write operations and
related bit-field definitions.

Signed-off-by: Alvin Chang 
---
  target/riscv/cpu.h  |  1 +
  target/riscv/cpu_bits.h |  3 +++
  target/riscv/csr.c  | 15 +++
  3 files changed, 19 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f52dce78ba..f9ae3e3025 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -364,6 +364,7 @@ struct CPUArchState {
  target_ulong tdata1[RV_MAX_TRIGGERS];
  target_ulong tdata2[RV_MAX_TRIGGERS];
  target_ulong tdata3[RV_MAX_TRIGGERS];
+target_ulong tcontrol;
  target_ulong mcontext;
  struct CPUBreakpoint *cpu_breakpoint[RV_MAX_TRIGGERS];
  struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS];
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index fc2068ee4d..3b3a7a0fa4 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -353,6 +353,7 @@
  #define CSR_TDATA2  0x7a2
  #define CSR_TDATA3  0x7a3
  #define CSR_TINFO   0x7a4
+#define CSR_TCONTROL0x7a5
  #define CSR_MCONTEXT0x7a8
  
  /* Debug Mode Registers */

@@ -900,6 +901,8 @@ typedef enum RISCVException {
  #define JVT_BASE   (~0x3F)
  
  /* Debug Sdtrig CSR masks */

+#define TCONTROL_MTE   BIT(3)
+#define TCONTROL_MPTE  BIT(7)
  #define MCONTEXT32 0x003F
  #define MCONTEXT64 0x1FFFULL
  #define MCONTEXT32_HCONTEXT0x007F
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d4e8ac13b9..816c530481 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3937,6 +3937,20 @@ static RISCVException read_tinfo(CPURISCVState *env, int 
csrno,
  return RISCV_EXCP_NONE;
  }
  
+static RISCVException read_tcontrol(CPURISCVState *env, int csrno,

+target_ulong *val)
+{
+*val = env->tcontrol;
+return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_tcontrol(CPURISCVState *env, int csrno,
+ target_ulong val)
+{
+env->tcontrol = val & (TCONTROL_MPTE | TCONTROL_MTE);
+return RISCV_EXCP_NONE;
+}
+
  static RISCVException read_mcontext(CPURISCVState *env, int csrno,
  target_ulong *val)
  {
@@ -4861,6 +4875,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
  [CSR_TDATA2]=  { "tdata2",   debug, read_tdata,write_tdata},
  [CSR_TDATA3]=  { "tdata3",   debug, read_tdata,write_tdata},
  [CSR_TINFO] =  { "tinfo",debug, read_tinfo,write_ignore   },
+[CSR_TCONTROL]  =  { "tcontrol", debug, read_tcontrol, write_tcontrol },


The spec reads:

"This optional register is only accessible in M-mode and Debug Mode and provides
 various control bits related to triggers."

"debug()" is checking only if we have the 'debug' cpu option enabled:


static RISCVException debug(CPURISCVState *env, int csrno)
{
if (riscv_cpu_cfg(env)->debug) {
return RISCV_EXCP_NONE;
}

return RISCV_EXCP_ILLEGAL_INST;
}


It looks like we don't have a "Debug Mode" model.

Section 4.1 of the spec mentions the following about "Debug Mode":

"1. All implemented instructions operate just as they do in M-mode, unless an
 exception is mentioned in this list.
 2. All operations are executed with machine mode privilege, except that 
additional
 Debug Mode CSRs are accessible and MPRV in mstatus may be ignored according to
 mprven. Full permission checks, or a relaxed set of permission checks, will 
apply
 according to relaxedpriv (...)"


So, if the operations are "executed with machine mode privilege" then can we 
expect
env->priv == PRV_M ? As it is now tcontrol will execute in any mode, so checking
for PRV_M seems reasonable.


Thanks,


Daniel






  [CSR_MCONTEXT]  =  { "mcontext", debug, read_mcontext, write_mcontext },
  
  /* User Pointer Masking */

Re: [PATCH] docs/system/ppc: Document running Linux on AmigaNG machines

[BALATON Zoltan]

On Fri, 16 Feb 2024, Thomas Huth wrote:

On 16/02/2024 01.10, BALATON Zoltan wrote:

Documentation on how to run Linux on the amigaone, pegasos2 and
sam460ex machines is currently burried in the depths of the qemu-devel


s/burried/buried/


mailing list and in the source code. Let's collect the information in
the QEMU handbook for a one stop solution.

Co-authored-by: Bernhard Beschow 
Signed-off-by: BALATON Zoltan 
---
Supersedes: <20231216123013.67978-1-shen...@gmail.com>

  MAINTAINERS |   1 +
  docs/system/ppc/amigang.rst | 160 
  docs/system/target-ppc.rst  |   1 +
  3 files changed, 162 insertions(+)
  create mode 100644 docs/system/ppc/amigang.rst

diff --git a/MAINTAINERS b/MAINTAINERS
index a24c2b51b6..8e5b47e7b4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1560,6 +1560,7 @@ F: hw/rtc/m41t80.c
  F: pc-bios/canyonlands.dt[sb]
  F: pc-bios/u-boot-sam460ex-20100605.bin
  F: roms/u-boot-sam460ex
+F: docs/system/ppc/amigang.rst
pegasos2
  M: BALATON Zoltan 
diff --git a/docs/system/ppc/amigang.rst b/docs/system/ppc/amigang.rst
new file mode 100644
index 00..c03a7e0d66
--- /dev/null
+++ b/docs/system/ppc/amigang.rst
@@ -0,0 +1,160 @@
+AmigaNG boards (``amigaone``, ``pegasos2``, ``sam460ex``)
+=
+
+These PowerPC machines emulate boards that are primarily used for
+running Amiga like OSes (AmigaOS 4, MorphOS and AROS) but these can
+also run Linux which is what this section documents.
+
+Eyetech AmigaOne/Mai Logic Teron (``amigaone``)
+===


Wouldn't it be better to use a subsection here (i.e. "" instead of 
"") ? And also adjust the subsections below to subsubsections?


This used to be the top but I've added another paragraph and forgot to 
change the headings. I'll check how other docs are organised and send 
another version after waiting for some time to see if anobody else spots 
some typos.


Thanks,
BALATON Zoltan


+
+The ``amigaone`` machine emulates an AmigaOne XE mainboard by Eyetech
+which is a rebranded Mai Logic Teron board with modified U-Boot
+firmware to support AmigaOS 4.
+
+Emulated devices
+
+
+ * PowerPC 7457 CPU (can also use``-cpu g3, 750cxe, 750fx`` or ``750gx``)
+ * Articia S north bridge
+ * VIA VT82C686B south bridge
+ * PCI VGA compatible card (guests may need other card instead)
+ * PS/2 keyboard and mouse
+
+Firmware
+
+
+A firmware binary is necessary for the boot process. It is a modified
+U-Boot under GPL but its source is lost so it cannot be included in
+QEMU. A binary is available at
+https://www.hyperion-entertainment.com/index.php/downloads?view=files&parent=28.
+The ROM image is in the last 512kB which can be extracted with the
+following command:
+
+.. code-block:: bash
+
+  $ tail -c 524288 updater.image > u-boot-amigaone.bin
+
+The BIOS emulator in the firmware is unable to run QEMU‘s standard
+vgabios so ``VGABIOS-lgpl-latest.bin`` is needed instead which can be
+downloaded from http://www.nongnu.org/vgabios.
+
+Running Linux
+-
+
+There are some Linux images under the following link that work on the
+``amigaone`` machine:
+https://sourceforge.net/projects/amigaone-linux/files/debian-installer/.
+To boot the system run:
+
+.. code-block:: bash
+
+  $ qemu-system-ppc -machine amigaone -bios u-boot-amigaone.bin \
+-cdrom "A1 Linux Net Installer.iso" \
+-device 
ati-vga,model=rv100,romfile=VGABIOS-lgpl-latest.bin

+
+From the firmware menu that appears select ``Boot sequence`` →
+``Amiga Multiboot Options`` and set ``Boot device 1`` to
+``Onboard VIA IDE CDROM``. Then hit escape until the main screen appears 
again,

+hit escape once more and from the exit menu that appears select either
+``Save settings and exit`` or ``Use settings for this session only``. It 
may
+take a long time loading the kernel into memory but eventually it boots 
and the

+installer becomes visible. The ``ati-vga`` RV100 emulation is not
+complete yet so only frame buffer works, DRM and 3D is not available.

...

Thomas

[PATCH v3 02/11] {linux,bsd}-user: Update ts_tid after fork()

[Ilya Leoshkevich]

Currently ts_tid contains the parent tid after fork(), which is not
correct. So far it has not affected anything, but the upcoming
follow-fork-mode child support relies on the correct value, so fix it.

Signed-off-by: Ilya Leoshkevich 
---
 bsd-user/main.c   | 1 +
 linux-user/main.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index e5efb7b8458..4140edc8311 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -127,6 +127,7 @@ void fork_end(int child)
  * state, so we don't need to end_exclusive() here.
  */
 qemu_init_cpu_list();
+((TaskState *)thread_cpu->opaque)->ts_tid = qemu_get_thread_id();
 gdbserver_fork(thread_cpu);
 } else {
 mmap_fork_end(child);
diff --git a/linux-user/main.c b/linux-user/main.c
index 74b2fbb3938..e6427d72332 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -160,6 +160,7 @@ void fork_end(int child)
 }
 }
 qemu_init_cpu_list();
+((TaskState *)thread_cpu->opaque)->ts_tid = qemu_get_thread_id();
 gdbserver_fork(thread_cpu);
 } else {
 cpu_list_unlock();
-- 
2.43.0

[PATCH v3 07/11] gdbstub: Introduce gdb_handle_query_supported_user()

[Ilya Leoshkevich]

The upcoming follow-fork-mode child support requires advertising the
fork-events feature, which is user-specific. Introduce a user-specific
hook for this.

Signed-off-by: Ilya Leoshkevich 
---
 gdbstub/gdbstub.c   | 12 +---
 gdbstub/internals.h |  1 +
 gdbstub/user.c  |  4 
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 7e73e916bdc..43d79dfdd59 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -1599,6 +1599,7 @@ static void handle_query_thread_extra(GArray *params, 
void *user_ctx)
 
 static void handle_query_supported(GArray *params, void *user_ctx)
 {
+const char *gdb_supported;
 CPUClass *cc;
 
 g_string_printf(gdbserver_state.str_buf, "PacketSize=%x", 
MAX_PACKET_LENGTH);
@@ -1622,9 +1623,14 @@ static void handle_query_supported(GArray *params, void 
*user_ctx)
 g_string_append(gdbserver_state.str_buf, ";qXfer:exec-file:read+");
 #endif
 
-if (params->len &&
-strstr(get_param(params, 0)->data, "multiprocess+")) {
-gdbserver_state.multiprocess = true;
+if (params->len) {
+gdb_supported = get_param(params, 0)->data;
+if (strstr(gdb_supported, "multiprocess+")) {
+gdbserver_state.multiprocess = true;
+}
+#if defined(CONFIG_USER_ONLY)
+gdb_handle_query_supported_user(gdb_supported);
+#endif
 }
 
 g_string_append(gdbserver_state.str_buf, ";vContSupported+;multiprocess+");
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index 56b7c13b750..e6063835b1f 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -196,6 +196,7 @@ 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_supported_user(const char *gdb_supported); /* user */
 
 void gdb_handle_query_attached(GArray *params, void *user_ctx); /* both */
 
diff --git a/gdbstub/user.c b/gdbstub/user.c
index 6ac9b684427..d8db5bd3949 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -382,6 +382,10 @@ void gdbserver_fork_end(pid_t pid)
 disable_gdbstub();
 }
 
+void gdb_handle_query_supported_user(const char *gdb_supported)
+{
+}
+
 /*
  * Execution state helpers
  */
-- 
2.43.0

[PATCH v3 01/11] gdbstub: Support disablement in a multi-threaded process

[Ilya Leoshkevich]

The upcoming follow-fork-mode child support will require disabling
gdbstub in the parent process, which may have multiple threads (which
are represented as CPUs).

Loop over all CPUs in order to remove breakpoints and disable
single-step. Move the respective code into a separate function.

Signed-off-by: Ilya Leoshkevich 
---
 gdbstub/user.c | 19 +++
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/gdbstub/user.c b/gdbstub/user.c
index 14918d1a217..e17f7ece908 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -356,16 +356,27 @@ int gdbserver_start(const char *port_or_path)
 return -1;
 }
 
+static void disable_gdbstub(void)
+{
+CPUState *cpu;
+
+close(gdbserver_user_state.fd);
+gdbserver_user_state.fd = -1;
+CPU_FOREACH(cpu) {
+cpu_breakpoint_remove_all(cpu, BP_GDB);
+/* no cpu_watchpoint_remove_all for user-mode */
+cpu_single_step(cpu, 0);
+tb_flush(cpu);
+}
+}
+
 /* Disable gdb stub for child processes.  */
 void gdbserver_fork(CPUState *cpu)
 {
 if (!gdbserver_state.init || gdbserver_user_state.fd < 0) {
 return;
 }
-close(gdbserver_user_state.fd);
-gdbserver_user_state.fd = -1;
-cpu_breakpoint_remove_all(cpu, BP_GDB);
-/* no cpu_watchpoint_remove_all for user-mode */
+disable_gdbstub();
 }
 
 /*
-- 
2.43.0

[PATCH v3 03/11] gdbstub: Introduce gdbserver_fork_start()

[Ilya Leoshkevich]

The upcoming follow-fork-mode child support requires knowing when
fork() is about to happen in order to initialize its state. Add a hook
for that.

Signed-off-by: Ilya Leoshkevich 
---
 bsd-user/main.c| 1 +
 gdbstub/user.c | 4 
 include/gdbstub/user.h | 5 +
 linux-user/main.c  | 1 +
 4 files changed, 11 insertions(+)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index 4140edc8311..bfe6888ea89 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -106,6 +106,7 @@ void fork_start(void)
 start_exclusive();
 cpu_list_lock();
 mmap_fork_start();
+gdbserver_fork_start();
 }
 
 void fork_end(int child)
diff --git a/gdbstub/user.c b/gdbstub/user.c
index e17f7ece908..5024c670f85 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -356,6 +356,10 @@ int gdbserver_start(const char *port_or_path)
 return -1;
 }
 
+void gdbserver_fork_start(void)
+{
+}
+
 static void disable_gdbstub(void)
 {
 CPUState *cpu;
diff --git a/include/gdbstub/user.h b/include/gdbstub/user.h
index 68b6534130c..e33f8d9a9a6 100644
--- a/include/gdbstub/user.h
+++ b/include/gdbstub/user.h
@@ -45,6 +45,11 @@ static inline int gdb_handlesig(CPUState *cpu, int sig)
  */
 void gdb_signalled(CPUArchState *as, int sig);
 
+/**
+ * gdbserver_fork_start() - inform gdb of the upcoming fork()
+ */
+void gdbserver_fork_start(void);
+
 /**
  * gdbserver_fork() - disable gdb stub for child processes.
  * @cs: CPU
diff --git a/linux-user/main.c b/linux-user/main.c
index e6427d72332..8c7bea1c631 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -144,6 +144,7 @@ void fork_start(void)
 mmap_fork_start();
 cpu_list_lock();
 qemu_plugin_user_prefork_lock();
+gdbserver_fork_start();
 }
 
 void fork_end(int child)
-- 
2.43.0

[PATCH v3 05/11] {linux,bsd}-user: Pass pid to gdbserver_fork()

[Ilya Leoshkevich]

The upcoming follow-fork-mode child support requires knowing the child
pid. Pass it down.

Signed-off-by: Ilya Leoshkevich 
---
 bsd-user/main.c| 2 +-
 gdbstub/user.c | 2 +-
 include/gdbstub/user.h | 2 +-
 linux-user/main.c  | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index bc233a85cef..e8c658eda5d 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -131,7 +131,7 @@ void fork_end(abi_long pid)
  */
 qemu_init_cpu_list();
 ((TaskState *)thread_cpu->opaque)->ts_tid = qemu_get_thread_id();
-gdbserver_fork(thread_cpu);
+gdbserver_fork(pid);
 } else {
 mmap_fork_end(child);
 cpu_list_unlock();
diff --git a/gdbstub/user.c b/gdbstub/user.c
index 5024c670f85..69b9857e5b6 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -375,7 +375,7 @@ static void disable_gdbstub(void)
 }
 
 /* Disable gdb stub for child processes.  */
-void gdbserver_fork(CPUState *cpu)
+void gdbserver_fork(pid_t pid)
 {
 if (!gdbserver_state.init || gdbserver_user_state.fd < 0) {
 return;
diff --git a/include/gdbstub/user.h b/include/gdbstub/user.h
index e33f8d9a9a6..66dd0d319cf 100644
--- a/include/gdbstub/user.h
+++ b/include/gdbstub/user.h
@@ -54,7 +54,7 @@ void gdbserver_fork_start(void);
  * gdbserver_fork() - disable gdb stub for child processes.
  * @cs: CPU
  */
-void gdbserver_fork(CPUState *cs);
+void gdbserver_fork(pid_t pid);
 
 /**
  * gdb_syscall_entry() - inform gdb of syscall entry and yield control to it
diff --git a/linux-user/main.c b/linux-user/main.c
index f1a0267816b..ad1c6394520 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -164,7 +164,7 @@ void fork_end(abi_long pid)
 }
 qemu_init_cpu_list();
 ((TaskState *)thread_cpu->opaque)->ts_tid = qemu_get_thread_id();
-gdbserver_fork(thread_cpu);
+gdbserver_fork(pid);
 } else {
 cpu_list_unlock();
 }
-- 
2.43.0

[PATCH v3 00/11] gdbstub: Implement follow-fork-mode child

[Ilya Leoshkevich]

v2: https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg00810.html
v2 -> v3: Rebase on top of master.
  Fix a typo in the 01/11 commit message.

v1: https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06646.html
v1 -> v2: Factor out a number of prep patches;
  Add a state transition diagram comment (Alex).
  Improve a few comments;
  Extend the ts_tid fix to bsd.

Hi,

I needed to debug a linux-user crash between fork() and exec() [1] and
realized that gdbstub does not allow this. This series lifts this
restriction (one still cannot debug past exec() though). Patches 1-9
are preliminary refactorings, patch 10 is the implementation, and patch
11 is the test.

[1] https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg06424.html

Best regards,
Ilya

Ilya Leoshkevich (11):
  gdbstub: Support disablement in a multi-threaded process
  {linux,bsd}-user: Update ts_tid after fork()
  gdbstub: Introduce gdbserver_fork_start()
  {linux,bsd}-user: Pass pid to fork_end()
  {linux,bsd}-user: Pass pid to gdbserver_fork()
  gdbstub: Call gdbserver_fork() both in parent and in child
  gdbstub: Introduce gdb_handle_query_supported_user()
  gdbstub: Introduce gdb_handle_set_thread_user()
  gdbstub: Introduce gdb_handle_detach_user()
  gdbstub: Implement follow-fork-mode child
  tests/tcg: Add two follow-fork-mode tests

 bsd-user/freebsd/os-proc.h|   6 +-
 bsd-user/main.c   |   9 +-
 bsd-user/qemu.h   |   2 +-
 gdbstub/gdbstub.c |  29 ++-
 gdbstub/internals.h   |   3 +
 gdbstub/user.c| 244 +-
 include/gdbstub/user.h|  11 +-
 linux-user/main.c |   8 +-
 linux-user/syscall.c  |   6 +-
 linux-user/user-internals.h   |   2 +-
 tests/tcg/multiarch/Makefile.target   |  17 +-
 tests/tcg/multiarch/follow-fork-mode.c|  56 
 .../gdbstub/follow-fork-mode-child.py |  40 +++
 .../gdbstub/follow-fork-mode-parent.py|  16 ++
 14 files changed, 424 insertions(+), 25 deletions(-)
 create mode 100644 tests/tcg/multiarch/follow-fork-mode.c
 create mode 100644 tests/tcg/multiarch/gdbstub/follow-fork-mode-child.py
 create mode 100644 tests/tcg/multiarch/gdbstub/follow-fork-mode-parent.py

-- 
2.43.0

[PATCH v3 04/11] {linux,bsd}-user: Pass pid to fork_end()

[Ilya Leoshkevich]

The upcoming follow-fork-mode child support requires knowing the child
pid. Pass it down.

Signed-off-by: Ilya Leoshkevich 
---
 bsd-user/freebsd/os-proc.h  | 6 +++---
 bsd-user/main.c | 4 +++-
 bsd-user/qemu.h | 2 +-
 linux-user/main.c   | 4 +++-
 linux-user/syscall.c| 6 +++---
 linux-user/user-internals.h | 2 +-
 6 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/bsd-user/freebsd/os-proc.h b/bsd-user/freebsd/os-proc.h
index d6418780344..3003c8cb637 100644
--- a/bsd-user/freebsd/os-proc.h
+++ b/bsd-user/freebsd/os-proc.h
@@ -208,7 +208,7 @@ static inline abi_long do_freebsd_fork(void *cpu_env)
  */
 set_second_rval(cpu_env, child_flag);
 
-fork_end(child_flag);
+fork_end(ret);
 
 return ret;
 }
@@ -252,7 +252,7 @@ static inline abi_long do_freebsd_rfork(void *cpu_env, 
abi_long flags)
  * value: 0 for parent process, 1 for child process.
  */
 set_second_rval(cpu_env, child_flag);
-fork_end(child_flag);
+fork_end(ret);
 
 return ret;
 
@@ -285,7 +285,7 @@ static inline abi_long do_freebsd_pdfork(void *cpu_env, 
abi_ulong target_fdp,
  * value: 0 for parent process, 1 for child process.
  */
 set_second_rval(cpu_env, child_flag);
-fork_end(child_flag);
+fork_end(ret);
 
 return ret;
 }
diff --git a/bsd-user/main.c b/bsd-user/main.c
index bfe6888ea89..bc233a85cef 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -109,8 +109,10 @@ void fork_start(void)
 gdbserver_fork_start();
 }
 
-void fork_end(int child)
+void fork_end(abi_long pid)
 {
+int child = pid == 0;
+
 if (child) {
 CPUState *cpu, *next_cpu;
 /*
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index dc842fffa7d..2414a87559b 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -180,7 +180,7 @@ void cpu_loop(CPUArchState *env);
 char *target_strerror(int err);
 int get_osversion(void);
 void fork_start(void);
-void fork_end(int child);
+void fork_end(abi_long pid);
 
 #include "qemu/log.h"
 
diff --git a/linux-user/main.c b/linux-user/main.c
index 8c7bea1c631..f1a0267816b 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -147,8 +147,10 @@ void fork_start(void)
 gdbserver_fork_start();
 }
 
-void fork_end(int child)
+void fork_end(abi_long pid)
 {
+int child = pid == 0;
+
 qemu_plugin_user_postfork(child);
 mmap_fork_end(child);
 if (child) {
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index e384e142489..572d8e0ed1c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6669,7 +6669,7 @@ static int do_fork(CPUArchState *env, unsigned int flags, 
abi_ulong newsp,
 if (ret == 0) {
 /* Child Process.  */
 cpu_clone_regs_child(env, newsp, flags);
-fork_end(1);
+fork_end(ret);
 /* There is a race condition here.  The parent process could
theoretically read the TID in the child process before the child
tid is set.  This would require using either ptrace
@@ -6700,8 +6700,8 @@ static int do_fork(CPUArchState *env, unsigned int flags, 
abi_ulong newsp,
 }
 #endif
 put_user_u32(pid_fd, parent_tidptr);
-}
-fork_end(0);
+}
+fork_end(ret);
 }
 g_assert(!cpu_in_exclusive_context(cpu));
 }
diff --git a/linux-user/user-internals.h b/linux-user/user-internals.h
index c63ef45fc78..9014014d920 100644
--- a/linux-user/user-internals.h
+++ b/linux-user/user-internals.h
@@ -71,7 +71,7 @@ const char *target_strerror(int err);
 int get_osversion(void);
 void init_qemu_uname_release(void);
 void fork_start(void);
-void fork_end(int child);
+void fork_end(abi_long pid);
 
 /**
  * probe_guest_base:
-- 
2.43.0

[PATCH v3 09/11] gdbstub: Introduce gdb_handle_detach_user()

[Ilya Leoshkevich]

The upcoming follow-fork-mode child support needs to perform certain
actions when GDB detaches from the stopped parent or the stopped child.
Introduce a user-specific hook for this.

Signed-off-by: Ilya Leoshkevich 
---
 gdbstub/gdbstub.c   | 6 ++
 gdbstub/internals.h | 1 +
 gdbstub/user.c  | 5 +
 3 files changed, 12 insertions(+)

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index adcd977cd57..46f5dd47e9e 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -991,6 +991,12 @@ static void handle_detach(GArray *params, void *user_ctx)
 pid = get_param(params, 0)->val_ul;
 }
 
+#ifdef CONFIG_USER_ONLY
+if (gdb_handle_detach_user(pid)) {
+return;
+}
+#endif
+
 process = gdb_get_process(pid);
 gdb_process_breakpoint_remove_all(process);
 process->attached = false;
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index b4905c7181a..b4724598384 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -198,6 +198,7 @@ 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_supported_user(const char *gdb_supported); /* user */
 bool gdb_handle_set_thread_user(uint32_t pid, uint32_t tid); /* user */
+bool gdb_handle_detach_user(uint32_t pid); /* user */
 
 void gdb_handle_query_attached(GArray *params, void *user_ctx); /* both */
 
diff --git a/gdbstub/user.c b/gdbstub/user.c
index 60f3e83849e..32518f275f5 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -391,6 +391,11 @@ bool gdb_handle_set_thread_user(uint32_t pid, uint32_t tid)
 return false;
 }
 
+bool gdb_handle_detach_user(uint32_t pid)
+{
+return false;
+}
+
 /*
  * Execution state helpers
  */
-- 
2.43.0

[PATCH v3 08/11] gdbstub: Introduce gdb_handle_set_thread_user()

[Ilya Leoshkevich]

The upcoming follow-fork-mode child support needs to perform certain
actions when GDB switches between the stopped parent and the stopped
child. Introduce a user-specific hook for this.

Signed-off-by: Ilya Leoshkevich 
---
 gdbstub/gdbstub.c   | 11 +--
 gdbstub/internals.h |  1 +
 gdbstub/user.c  |  5 +
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 43d79dfdd59..adcd977cd57 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -1066,6 +1066,7 @@ static void handle_cont_with_sig(GArray *params, void 
*user_ctx)
 
 static void handle_set_thread(GArray *params, void *user_ctx)
 {
+uint32_t pid, tid;
 CPUState *cpu;
 
 if (params->len != 2) {
@@ -1083,8 +1084,14 @@ static void handle_set_thread(GArray *params, void 
*user_ctx)
 return;
 }
 
-cpu = gdb_get_cpu(get_param(params, 1)->thread_id.pid,
-  get_param(params, 1)->thread_id.tid);
+pid = get_param(params, 1)->thread_id.pid;
+tid = get_param(params, 1)->thread_id.tid;
+#ifdef CONFIG_USER_ONLY
+if (gdb_handle_set_thread_user(pid, tid)) {
+return;
+}
+#endif
+cpu = gdb_get_cpu(pid, tid);
 if (!cpu) {
 gdb_put_packet("E22");
 return;
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index e6063835b1f..b4905c7181a 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -197,6 +197,7 @@ 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_supported_user(const char *gdb_supported); /* user */
+bool gdb_handle_set_thread_user(uint32_t pid, uint32_t tid); /* user */
 
 void gdb_handle_query_attached(GArray *params, void *user_ctx); /* both */
 
diff --git a/gdbstub/user.c b/gdbstub/user.c
index d8db5bd3949..60f3e83849e 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -386,6 +386,11 @@ void gdb_handle_query_supported_user(const char 
*gdb_supported)
 {
 }
 
+bool gdb_handle_set_thread_user(uint32_t pid, uint32_t tid)
+{
+return false;
+}
+
 /*
  * Execution state helpers
  */
-- 
2.43.0

Re: [PATCH 0/2] trace: fix ability to use systemtap with qemu tools

[Daniel P . Berrangé]

Ping: Stefan, are you OK with picking up these trace patches
for merge ?

On Mon, Jan 08, 2024 at 05:13:54PM +, Daniel P. Berrangé wrote:
> Currently we're only generating .stp definitions for the system and
> user emulators forgetting all about the tools which support tracing
> too.
> 
> Daniel P. Berrangé (2):
>   tracetool: remove redundant --target-type / --target-name args
>   meson: generate .stp files for tools too
> 
>  docs/devel/tracing.rst |  3 +-
>  meson.build| 63 +++---
>  scripts/tracetool.py   | 24 
>  3 files changed, 46 insertions(+), 44 deletions(-)


> 
> -- 
> 2.43.0
> 

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

[PATCH v3 06/11] gdbstub: Call gdbserver_fork() both in parent and in child

[Ilya Leoshkevich]

The upcoming follow-fork-mode child support requires post-fork message
exchange between the parent and the child. Prepare gdbserver_fork() for
this purpose. Rename it to gdbserver_fork_end() to better reflect its
purpose.

Signed-off-by: Ilya Leoshkevich 
---
 bsd-user/main.c| 3 ++-
 gdbstub/user.c | 5 ++---
 include/gdbstub/user.h | 6 +++---
 linux-user/main.c  | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index e8c658eda5d..1890d6365f7 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -131,10 +131,11 @@ void fork_end(abi_long pid)
  */
 qemu_init_cpu_list();
 ((TaskState *)thread_cpu->opaque)->ts_tid = qemu_get_thread_id();
-gdbserver_fork(pid);
+gdbserver_fork_end(pid);
 } else {
 mmap_fork_end(child);
 cpu_list_unlock();
+gdbserver_fork_end(pid);
 end_exclusive();
 }
 }
diff --git a/gdbstub/user.c b/gdbstub/user.c
index 69b9857e5b6..6ac9b684427 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -374,10 +374,9 @@ static void disable_gdbstub(void)
 }
 }
 
-/* Disable gdb stub for child processes.  */
-void gdbserver_fork(pid_t pid)
+void gdbserver_fork_end(pid_t pid)
 {
-if (!gdbserver_state.init || gdbserver_user_state.fd < 0) {
+if (pid != 0 || !gdbserver_state.init || gdbserver_user_state.fd < 0) {
 return;
 }
 disable_gdbstub();
diff --git a/include/gdbstub/user.h b/include/gdbstub/user.h
index 66dd0d319cf..dd03dbdd6df 100644
--- a/include/gdbstub/user.h
+++ b/include/gdbstub/user.h
@@ -51,10 +51,10 @@ void gdb_signalled(CPUArchState *as, int sig);
 void gdbserver_fork_start(void);
 
 /**
- * gdbserver_fork() - disable gdb stub for child processes.
- * @cs: CPU
+ * gdbserver_fork_end() - inform gdb of the completed fork()
+ * @pid: 0 if in child process, -1 if fork failed, child process pid otherwise
  */
-void gdbserver_fork(pid_t pid);
+void gdbserver_fork_end(pid_t pid);
 
 /**
  * gdb_syscall_entry() - inform gdb of syscall entry and yield control to it
diff --git a/linux-user/main.c b/linux-user/main.c
index ad1c6394520..dde5081e2f4 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -164,10 +164,10 @@ void fork_end(abi_long pid)
 }
 qemu_init_cpu_list();
 ((TaskState *)thread_cpu->opaque)->ts_tid = qemu_get_thread_id();
-gdbserver_fork(pid);
 } else {
 cpu_list_unlock();
 }
+gdbserver_fork_end(pid);
 /*
  * qemu_init_cpu_list() reinitialized the child exclusive state, but we
  * also need to keep current_cpu consistent, so call end_exclusive() for
-- 
2.43.0

[PATCH v3 11/11] tests/tcg: Add two follow-fork-mode tests

[Ilya Leoshkevich]

Add follow-fork-mode child and and follow-fork-mode parent tests.
Check for the obvious pitfalls, such as lingering breakpoints,
catchpoints, and single-step mode.

Signed-off-by: Ilya Leoshkevich 
---
 tests/tcg/multiarch/Makefile.target   | 17 +-
 tests/tcg/multiarch/follow-fork-mode.c| 56 +++
 .../gdbstub/follow-fork-mode-child.py | 40 +
 .../gdbstub/follow-fork-mode-parent.py| 16 ++
 4 files changed, 128 insertions(+), 1 deletion(-)
 create mode 100644 tests/tcg/multiarch/follow-fork-mode.c
 create mode 100644 tests/tcg/multiarch/gdbstub/follow-fork-mode-child.py
 create mode 100644 tests/tcg/multiarch/gdbstub/follow-fork-mode-parent.py

diff --git a/tests/tcg/multiarch/Makefile.target 
b/tests/tcg/multiarch/Makefile.target
index e10951a8016..b8b70c81860 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -115,6 +115,20 @@ run-gdbstub-catch-syscalls: catch-syscalls
--bin $< --test $(MULTIARCH_SRC)/gdbstub/catch-syscalls.py, \
hitting a syscall catchpoint)
 
+run-gdbstub-follow-fork-mode-child: follow-fork-mode
+   $(call run-test, $@, $(GDB_SCRIPT) \
+   --gdb $(GDB) \
+   --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+   --bin $< --test 
$(MULTIARCH_SRC)/gdbstub/follow-fork-mode-child.py, \
+   following children on fork)
+
+run-gdbstub-follow-fork-mode-parent: follow-fork-mode
+   $(call run-test, $@, $(GDB_SCRIPT) \
+   --gdb $(GDB) \
+   --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+   --bin $< --test 
$(MULTIARCH_SRC)/gdbstub/follow-fork-mode-parent.py, \
+   following parents on fork)
+
 else
 run-gdbstub-%:
$(call skip-test, "gdbstub test $*", "need working gdb with $(patsubst 
-%,,$(TARGET_NAME)) support")
@@ -122,7 +136,8 @@ 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-catch-syscalls
+ run-gdbstub-catch-syscalls run-gdbstub-follow-fork-mode-child \
+ run-gdbstub-follow-fork-mode-parent
 
 # ARM Compatible Semi Hosting Tests
 #
diff --git a/tests/tcg/multiarch/follow-fork-mode.c 
b/tests/tcg/multiarch/follow-fork-mode.c
new file mode 100644
index 000..cb6b032b388
--- /dev/null
+++ b/tests/tcg/multiarch/follow-fork-mode.c
@@ -0,0 +1,56 @@
+/*
+ * Test GDB's follow-fork-mode.
+ *
+ * fork() a chain of processes.
+ * Parents sends one byte to their children, and children return their
+ * position in the chain, in order to prove that they survived GDB's fork()
+ * handling.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include 
+#include 
+#include 
+#include 
+
+void break_after_fork(void)
+{
+}
+
+int main(void)
+{
+int depth = 42, err, i, fd[2], status;
+pid_t child, pid;
+ssize_t n;
+char b;
+
+for (i = 0; i < depth; i++) {
+err = pipe(fd);
+assert(err == 0);
+child = fork();
+break_after_fork();
+assert(child != -1);
+if (child == 0) {
+close(fd[1]);
+
+n = read(fd[0], &b, 1);
+close(fd[0]);
+assert(n == 1);
+assert(b == (char)i);
+} else {
+close(fd[0]);
+
+b = (char)i;
+n = write(fd[1], &b, 1);
+close(fd[1]);
+assert(n == 1);
+
+pid = waitpid(child, &status, 0);
+assert(pid == child);
+assert(WIFEXITED(status));
+return WEXITSTATUS(status) - 1;
+}
+}
+
+return depth;
+}
diff --git a/tests/tcg/multiarch/gdbstub/follow-fork-mode-child.py 
b/tests/tcg/multiarch/gdbstub/follow-fork-mode-child.py
new file mode 100644
index 000..72a6e440c08
--- /dev/null
+++ b/tests/tcg/multiarch/gdbstub/follow-fork-mode-child.py
@@ -0,0 +1,40 @@
+"""Test GDB's follow-fork-mode child.
+
+SPDX-License-Identifier: GPL-2.0-or-later
+"""
+from test_gdbstub import main, report
+
+
+def run_test():
+"""Run through the tests one by one"""
+gdb.execute("set follow-fork-mode child")
+# Check that the parent breakpoints are unset.
+gdb.execute("break break_after_fork")
+# Check that the parent syscall catchpoints are unset.
+# Skip this check on the architectures that don't have them.
+have_fork_syscall = False
+for fork_syscall in ("fork", "clone", "clone2", "clone3"):
+try:
+gdb.execute("catch syscall {}".format(fork_syscall))
+except gdb.error:
+pass
+else:
+have_fork_syscall = True
+gdb.execute("continue")
+for i in range(42):
+if have_fork_syscall:
+# syscall entry.
+if i % 2 == 0:
+# Check that the parent single-stepping is turned off.
+gdb.execute("si")
+

[PATCH v3 10/11] gdbstub: Implement follow-fork-mode child

[Ilya Leoshkevich]

Currently it's not possible to use gdbstub for debugging linux-user
code that runs in a forked child, which is normally done using the `set
follow-fork-mode child` GDB command. Purely on the protocol level, the
missing piece is the fork-events feature.

However, a deeper problem is supporting $Hg switching between different
processes - right now it can do only threads. Implementing this for the
general case would be quite complicated, but, fortunately, for the
follow-fork-mode case there are a few factors that greatly simplify
things: fork() happens in the exclusive section, there are only two
processes involved, and before one of them is resumed, the second one
is detached.

This makes it possible to implement a simplified scheme: the parent and
the child share the gdbserver socket, it's used only by one of them at
any given time, which is coordinated through a separate socketpair. The
processes can read from the gdbserver socket only one byte at a time,
which is not great for performance, but, fortunately, the
follow-fork-mode handling involves only a few messages.

Advertise the fork-events support, and remember whether GDB has it
as well. Implement the state machine that is initialized on fork(),
decides the current owner of the gdbserver socket, and is terminated
when one of the two processes is detached. The logic for the parent and
the child is the same, only the initial state is different.

Signed-off-by: Ilya Leoshkevich 
---
 gdbstub/user.c | 212 -
 1 file changed, 210 insertions(+), 2 deletions(-)

diff --git a/gdbstub/user.c b/gdbstub/user.c
index 32518f275f5..7dcc1159cc0 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -25,6 +25,61 @@
 #define GDB_NR_SYSCALLS 1024
 typedef unsigned long GDBSyscallsMask[BITS_TO_LONGS(GDB_NR_SYSCALLS)];
 
+/*
+ * Forked child talks to its parent in order to let GDB enforce the
+ * follow-fork-mode. This happens inside a start_exclusive() section, so that
+ * the other threads, which may be forking too, do not interfere. The
+ * implementation relies on GDB not sending $vCont until it has detached
+ * either from the parent (follow-fork-mode child) or from the child
+ * (follow-fork-mode parent).
+ *
+ * The parent and the child share the GDB socket; at any given time only one
+ * of them is allowed to use it, as is reflected in the respective fork_state.
+ * This is negotiated via the fork_sockets pair as a reaction to $Hg.
+ *
+ * Below is a short summary of the possible state transitions:
+ *
+ * ENABLED : Terminal state.
+ * DISABLED: Terminal state.
+ * ACTIVE  : Parent initial state.
+ * INACTIVE: Child initial state.
+ * ACTIVE   -> DEACTIVATING: On $Hg.
+ * ACTIVE   -> ENABLING: On $D.
+ * ACTIVE   -> DISABLING   : On $D.
+ * ACTIVE   -> DISABLED: On communication error.
+ * DEACTIVATING -> INACTIVE: On gdb_read_byte() return.
+ * DEACTIVATING -> DISABLED: On communication error.
+ * INACTIVE -> ACTIVE  : On $Hg in the peer.
+ * INACTIVE -> ENABLE  : On $D in the peer.
+ * INACTIVE -> DISABLE : On $D in the peer.
+ * INACTIVE -> DISABLED: On communication error.
+ * ENABLING -> ENABLED : On gdb_read_byte() return.
+ * ENABLING -> DISABLED: On communication error.
+ * DISABLING-> DISABLED: On gdb_read_byte() return.
+ */
+enum GDBForkState {
+/* Fully owning the GDB socket. */
+GDB_FORK_ENABLED,
+/* Working with the GDB socket; the peer is inactive. */
+GDB_FORK_ACTIVE,
+/* Handing off the GDB socket to the peer. */
+GDB_FORK_DEACTIVATING,
+/* The peer is working with the GDB socket. */
+GDB_FORK_INACTIVE,
+/* Asking the peer to close its GDB socket fd. */
+GDB_FORK_ENABLING,
+/* Asking the peer to take over, closing our GDB socket fd. */
+GDB_FORK_DISABLING,
+/* The peer has taken over, our GDB socket fd is closed. */
+GDB_FORK_DISABLED,
+};
+
+enum GDBForkMessage {
+GDB_FORK_ACTIVATE = 'a',
+GDB_FORK_ENABLE = 'e',
+GDB_FORK_DISABLE = 'd',
+};
+
 /* User-mode specific state */
 typedef struct {
 int fd;
@@ -36,6 +91,10 @@ typedef struct {
  */
 bool catch_all_syscalls;
 GDBSyscallsMask catch_syscalls_mask;
+bool fork_events;
+enum GDBForkState fork_state;
+int fork_sockets[2];
+pid_t fork_peer_pid, fork_peer_tid;
 } GDBUserState;
 
 static GDBUserState gdbserver_user_state;
@@ -358,6 +417,18 @@ int gdbserver_start(const char *port_or_path)
 
 void gdbserver_fork_start(void)
 {
+if (!gdbserver_state.init || gdbserver_user_state.fd < 0) {
+return;
+}
+if (!gdbserver_user_state.fork_events ||
+qemu_socketpair(AF_UNIX, SOCK_STREAM, 0,
+gdbserver_user_state.fork_sockets) < 0) {
+gdbserver_user_state.fork_state = G

Re: [PATCH 11/14] vfio: Extend vfio_set_migration_error() with Error* argument

[Cédric Le Goater]

Hello Avihai,

On 2/12/24 10:35, Avihai Horon wrote:

Hi Cedric,

On 07/02/2024 15:33, Cédric Le Goater wrote:

External email: Use caution opening links or attachments


vfio_set_migration_error() sets the 'return' error on the migration
stream if a migration is in progress. To improve error reporting, add
a new Error* argument to also set the Error object on the migration
stream.

Signed-off-by: Cédric Le Goater 
---
  hw/vfio/common.c | 50 +---
  1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 
82173b039c47150f5edd05d329192c5b9c8a9a0f..afe8b6bd294fd5904f394a5db48aae3fd718b14e
 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -148,16 +148,18 @@ bool vfio_viommu_preset(VFIODevice *vbasedev)
  return vbasedev->bcontainer->space->as != &address_space_memory;
  }

-static void vfio_set_migration_error(int err)
+static void vfio_set_migration_error(int ret, Error *err)
  {
  MigrationState *ms = migrate_get_current();

  if (migration_is_setup_or_active(ms->state)) {
  WITH_QEMU_LOCK_GUARD(&ms->qemu_file_lock) {
  if (ms->to_dst_file) {
-    qemu_file_set_error(ms->to_dst_file, err);
+    qemu_file_set_error_obj(ms->to_dst_file, ret, err);
  }
  }
+    } else {
+    error_report_err(err);
  }
  }

@@ -296,15 +298,17 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, 
IOMMUTLBEntry *iotlb)
  VFIOContainerBase *bcontainer = giommu->bcontainer;
  hwaddr iova = iotlb->iova + giommu->iommu_offset;
  void *vaddr;
+    Error *local_err = NULL;
  int ret;

  trace_vfio_iommu_map_notify(iotlb->perm == IOMMU_NONE ? "UNMAP" : "MAP",
  iova, iova + iotlb->addr_mask);

  if (iotlb->target_as != &address_space_memory) {
-    error_report("Wrong target AS \"%s\", only system memory is allowed",
- iotlb->target_as->name ? iotlb->target_as->name : "none");
-    vfio_set_migration_error(-EINVAL);
+    error_setg(&local_err,
+   "Wrong target AS \"%s\", only system memory is allowed",
+   iotlb->target_as->name ? iotlb->target_as->name : "none");
+    vfio_set_migration_error(-EINVAL, local_err);
  return;
  }

@@ -336,11 +340,12 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, 
IOMMUTLBEntry *iotlb)
  ret = vfio_container_dma_unmap(bcontainer, iova,
 iotlb->addr_mask + 1, iotlb);
  if (ret) {
-    error_report("vfio_container_dma_unmap(%p, 0x%"HWADDR_PRIx", "
- "0x%"HWADDR_PRIx") = %d (%s)",
- bcontainer, iova,
- iotlb->addr_mask + 1, ret, strerror(-ret));
-    vfio_set_migration_error(ret);
+    error_setg(&local_err,
+   "vfio_container_dma_unmap(%p, 0x%"HWADDR_PRIx", "
+   "0x%"HWADDR_PRIx") = %d (%s)",
+   bcontainer, iova,
+   iotlb->addr_mask + 1, ret, strerror(-ret));
+    vfio_set_migration_error(ret, local_err);
  }
  }
  out:
@@ -1224,13 +1229,15 @@ static void vfio_iommu_map_dirty_notify(IOMMUNotifier 
*n, IOMMUTLBEntry *iotlb)
  VFIOContainerBase *bcontainer = giommu->bcontainer;
  hwaddr iova = iotlb->iova + giommu->iommu_offset;
  ram_addr_t translated_addr;
+    Error *local_err = NULL;
  int ret = -EINVAL;

  trace_vfio_iommu_map_dirty_notify(iova, iova + iotlb->addr_mask);

  if (iotlb->target_as != &address_space_memory) {
-    error_report("Wrong target AS \"%s\", only system memory is allowed",
- iotlb->target_as->name ? iotlb->target_as->name : "none");
+    error_setg(&local_err,
+   "Wrong target AS \"%s\", only system memory is allowed",
+   iotlb->target_as->name ? iotlb->target_as->name : "none");
  goto out;
  }

@@ -1239,17 +1246,18 @@ static void vfio_iommu_map_dirty_notify(IOMMUNotifier 
*n, IOMMUTLBEntry *iotlb)
  ret = vfio_get_dirty_bitmap(bcontainer, iova, iotlb->addr_mask + 1,
  translated_addr);


If vfio_get_xlat_addr() above (it's not shown here) returns false, we will pass 
a NULL local_err to vfio_set_migration_error() and it may de-reference NULL ptr 
in error_report_err().


Ah yes. Thanks for spotting this.



Should we refactor vfio_get_xlat_addr() to get errp, 


I think we should add an Error** parameter to vfio_get_xlat_addr() and
memory_get_xlat_addr(). It shouldn't be too much change and would be
cleaner.

Thanks,

C.



or add an else branch below, setting -EINVAL (and removing the default -EINVAL 
from the top of the function)?

Thanks.


  if (ret) {
-    error_report("vfio_iommu_map_dirty_notify(%p, 0x%"HWADDR_PRIx", "
- 

Re: [PATCH 1/4] Add ivshmem-flat device

[Philippe Mathieu-Daudé]

On 27/11/23 06:20, Gustavo Romero wrote:

Add a new device, ivshmem-flat, which is similar to the ivshmem PCI but
does not require a PCI bus. It's meant to be used on machines like those
with Cortex-M MCUs, which usually lack a PCI/PCIe bus, e.g. lm3s6965evb
and mps2-an385.

The device currently only supports the sysbus bus.

The following is an example on how to create the ivshmem-flat device on
a Stellaris machine:

$ qemu-system-arm -cpu cortex-m3 -machine lm3s6965evb -nographic
   -net none -chardev stdio,id=con,mux=on
   -serial chardev:con -mon chardev=con,mode=readline
   -chardev socket,path=/tmp/ivshmem_socket,id=ivf
   -device 
ivshmem-flat,x-irq-qompath=/machine/unattached/device[1]/nvic/unnamed-gpio-in[0],x-bus-qompath="/sysbus",chardev=ivf
   -kernel zephyr_qemu.elf

The new device, just like the ivshmem PCI device, supports both peer
notification via hardware interrupts and shared memory.

The IRQ QOM path for the target machine can be determined by creating
the VM without the ivshmem-flat device, going to the QEMU console and
listing the QOM nodes with 'info qom-tree'. In the Stellaris example
above the input IRQ is in the NVIC IC.

The MMRs for status and control (notification) are mapped to the MMIO
region at 0x400FF000 (default), whilst the shared memory region start
is mapped at addr. 0x4010 (default), but both addresses can be set
when creating the device by using 'x-bus-address-{mmr,shmem}' options,
respectively.

The device shared memory size can be set using the 'shmem-size' option
and it defaults to 4 MiB, which is the default size of shmem allocated
by the ivshmem server.

Signed-off-by: Gustavo Romero 
---
  docs/system/devices/ivshmem-flat.rst |  89 +
  hw/arm/mps2.c|   2 +
  hw/arm/stellaris.c   |   5 +-
  hw/arm/virt.c|   2 +
  hw/core/sysbus-fdt.c |   1 +
  hw/misc/Kconfig  |   5 +
  hw/misc/ivshmem-flat.c   | 477 +++
  hw/misc/meson.build  |   2 +
  hw/misc/trace-events |  18 +
  include/hw/misc/ivshmem-flat.h   |  72 
  10 files changed, 672 insertions(+), 1 deletion(-)
  create mode 100644 docs/system/devices/ivshmem-flat.rst
  create mode 100644 hw/misc/ivshmem-flat.c
  create mode 100644 include/hw/misc/ivshmem-flat.h




diff --git a/include/hw/misc/ivshmem-flat.h b/include/hw/misc/ivshmem-flat.h
new file mode 100644
index 00..2f6f7462f6
--- /dev/null
+++ b/include/hw/misc/ivshmem-flat.h
@@ -0,0 +1,72 @@
+/*
+ * Inter-VM Shared Memory Flat Device
+ *
+ * SPDX-FileCopyrightText: 2023 Linaro Ltd.
+ * SPDX-FileContributor: Gustavo Romero 
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ */
+
+#ifndef IVSHMEM_FLAT_H
+#define IVSHMEM_FLAT_H
+


I had to include the following headers:

 #include "qemu/queue.h"
 #include "qemu/event_notifier.h"
 #include "chardev/char-fe.h"
 #include "exec/memory.h"
 #include "qom/object.h"
 #include "hw/sysbus.h"

in order to fix:

include/hw/misc/ivshmem-flat.h:50:19: error: field has incomplete type 
'EventNotifier' (aka 'struct EventNotifier')

EventNotifier event_notifier;
  ^
include/hw/misc/ivshmem-flat.h:55:5: error: type name requires a 
specifier or qualifier

QTAILQ_ENTRY(IvshmemPeer) next;
^
include/hw/misc/ivshmem-flat.h:62:5: error: unknown type name 'SysBusDevice'
SysBusDevice parent_obj;
^
include/hw/misc/ivshmem-flat.h:70:5: error: unknown type name 'CharBackend'
CharBackend server_chr;
^
include/hw/misc/ivshmem-flat.h:76:18: error: field has incomplete type 
'MemoryRegion' (aka 'struct MemoryRegion')

MemoryRegion iomem;
 ^


+#define IVSHMEM_MAX_VECTOR_NUM 64
+
+#define TYPE_IVSHMEM_FLAT "ivshmem-flat"
+typedef struct IvshmemFTState IvshmemFTState;
+
+DECLARE_INSTANCE_CHECKER(IvshmemFTState, IVSHMEM_FLAT, TYPE_IVSHMEM_FLAT)
+
+/* Ivshmem registers. See ./docs/specs/ivshmem-spec.txt for details. */
+enum ivshmem_registers {
+INTMASK = 0,
+INTSTATUS = 4,
+IVPOSITION = 8,
+DOORBELL = 12,
+};
+
+typedef struct VectorInfo {
+EventNotifier event_notifier;
+uint16_t id;
+} VectorInfo;
+
+typedef struct IvshmemPeer {
+QTAILQ_ENTRY(IvshmemPeer) next;
+VectorInfo vector[IVSHMEM_MAX_VECTOR_NUM];
+int vector_counter;
+uint16_t id;
+} IvshmemPeer;
+
+struct IvshmemFTState {
+SysBusDevice parent_obj;
+
+uint64_t msg_buf;
+int msg_buffered_bytes;
+
+QTAILQ_HEAD(, IvshmemPeer) peer;
+IvshmemPeer own;
+
+CharBackend server_chr;
+
+char *bus_qompath;
+
+/* IRQ */
+qemu_irq irq;
+char *irq_qompath;
+
+/* MMRs */
+MemoryRegion iomem;
+uint64_t bus_address_mmr;
+uint32_t intmask;
+uint32_t intstatus;
+uint32_t ivposition;
+uint32_t doorbell;
+
+/* Shared memory */
+MemoryRegion shmem;
+int shmem

Re: [PATCH] target: hppa: Fix unaligned double word accesses for hppa64

[Charlie Jenkins]

On Thu, Feb 15, 2024 at 09:34:15PM -0800, Guenter Roeck wrote:
> Unaligned 64-bit accesses were found in Linux to clobber carry bits,
> resulting in bad results if an arithmetic operation involving a
> carry bit was executed after an unaligned 64-bit operation.
> 
> hppa 2.0 defines additional carry bits in PSW register bits 32..39.
> When restoring PSW after executing an unaligned instruction trap,
> those bits were not cleared and ended up to be active all the time.
> Clearing bit 32..39 in psw prior to restoring it solves the problem.
> 
> Fixes: 931adff31478 ("target/hppa: Update cpu_hppa_get/put_psw for hppa64")
> Cc: Richard Henderson 
> Cc: Charlie Jenkins 
> Cc: Helge Deller 
> Signed-off-by: Guenter Roeck 
> ---
>  target/hppa/helper.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/target/hppa/helper.c b/target/hppa/helper.c
> index 859644c47a..7b798d1227 100644
> --- a/target/hppa/helper.c
> +++ b/target/hppa/helper.c
> @@ -76,7 +76,12 @@ void cpu_hppa_put_psw(CPUHPPAState *env, target_ulong psw)
>  }
>  psw &= ~reserved;
>  
> -env->psw = psw & ~(PSW_N | PSW_V | PSW_CB);
> +if (hppa_is_pa20(env)) {
> +env->psw = psw & ~(PSW_N | PSW_V | PSW_CB | 0xffull);

I thought there was something fishy in this function but was slow on the
uptake...

How about defining a new macro (PSW_CB_HIGH) to hold this value?

- Charlie

> +} else {
> +env->psw = psw & ~(PSW_N | PSW_V | PSW_CB);
> +}
> +
>  env->psw_n = (psw / PSW_N) & 1;
>  env->psw_v = -((psw / PSW_V) & 1);
>  
> -- 
> 2.39.2
> 

Re: [PATCH] .gitlab-ci/windows.yml: Don't install libusb or spice packages on 32-bit

[Peter Maydell]

On Fri, 16 Feb 2024 at 11:29, Alex Bennée  wrote:
>
> Peter Maydell  writes:
>
> > When msys2 updated their libusb packages to libusb 1.0.27, they
> > dropped support for building them for mingw32, leaving only mingw64
> > packages.  This broke our CI job, as the 'pacman' package install now
> > fails with:
> >
> > error: target not found: mingw-w64-i686-libusb
> > error: target not found: mingw-w64-i686-usbredir
> >
> > (both these binary packages are from the libusb source package).
> >
> > Similarly, spice is now 64-bit only:
> > error: target not found: mingw-w64-i686-spice
> >
> > Fix this by dropping these packages from the list we install for our
> > msys2-32bit build.  We do this with a simple mechanism for the
> > msys2-64bit and msys2-32bit jobs to specify a list of extra packages
> > to install on top of the common ones we install for both jobs.
> >
> > Cc: qemu-sta...@nongnu.org
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2160
> > Signed-off-by: Peter Maydell 
>
> Queued to testing/next, thanks.

I'm just testing a merge where I've applied this directly to
get our CI back to green, so you don't need to take it via
your tree.

thanks
-- PMM

Re: [PATCH 3/4] target/riscv: Set the value of CSR tcontrol when trapping to M-mode

[Daniel Henrique Barboza]

On 2/16/24 03:13, Alvin Chang wrote:

 From the RISC-V debug specification, it defines the following operations
for CSR tcontrol when any trap into M-mode is taken:
1. tcontrol.MPTE is set to the value of tcontrol.MTE
2. tcontrol.MTE is set to 0

This commit implements the above operations into
riscv_cpu_do_interrupt().

Signed-off-by: Alvin Chang 
---


Reviewed-by: Daniel Henrique Barboza 



  target/riscv/cpu_helper.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index d462d95ee1..037ae21062 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1806,6 +1806,12 @@ void riscv_cpu_do_interrupt(CPUState *cs)
  riscv_cpu_set_virt_enabled(env, 0);
  }
  
+/* Trapping to M-mode. Set tcontrol CSR in debug Sdtrig extension. */

+s = env->tcontrol;
+s = set_field(s, TCONTROL_MPTE, get_field(s, TCONTROL_MTE));
+s = set_field(s, TCONTROL_MTE, 0);
+env->tcontrol = s;
+
  s = env->mstatus;
  s = set_field(s, MSTATUS_MPIE, get_field(s, MSTATUS_MIE));
  s = set_field(s, MSTATUS_MPP, env->priv);

Re: [PATCH 2/4] target/riscv: Reset CSR tcontrol when the trigger module resets

[Daniel Henrique Barboza]

On 2/16/24 03:13, Alvin Chang wrote:

When the trigger module resets, reset the value of CSR tcontrol as zero.

Signed-off-by: Alvin Chang 
---


Reviewed-by: Daniel Henrique Barboza 


  target/riscv/debug.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index e30d99cc2f..e3832a643e 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -941,5 +941,6 @@ void riscv_trigger_reset_hold(CPURISCVState *env)
  timer_del(env->itrigger_timer[i]);
  }
  
+env->tcontrol = 0;

  env->mcontext = 0;
  }

Re: [PATCH 4/4] target/riscv: Set the value of CSR tcontrol when mret is executed

[Daniel Henrique Barboza]

On 2/16/24 03:13, Alvin Chang wrote:

The RISC-V debug specification defines the following operation for CSR
tcontrol when "mret" is executed:
- tcontrol.MTE is set to the value of tcontrol.MPTE

This commit implements the above operation into helper_mret().

Note that from tech-debug mailing list:
https://lists.riscv.org/g/tech-debug/topic/102702615#1461
The debug specification does not mention the operation to tcontrol.MPTE
when "mret" is executed. Therefore, we just keep its current value.

Signed-off-by: Alvin Chang 
---


Reviewed-by: Daniel Henrique Barboza 


  target/riscv/op_helper.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index f414aaebdb..12822b3afa 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -347,6 +347,12 @@ target_ulong helper_mret(CPURISCVState *env)
  mstatus = set_field(mstatus, MSTATUS_MPRV, 0);
  }
  env->mstatus = mstatus;
+
+uint64_t tcontrol = env->tcontrol;
+tcontrol = set_field(tcontrol, TCONTROL_MTE,
+ get_field(tcontrol, TCONTROL_MPTE));
+env->tcontrol = tcontrol;
+
  riscv_cpu_set_mode(env, prev_priv);
  
  if (riscv_has_ext(env, RVH)) {

Re: [PATCH v5 0/9] Misc clean ups to target/ppc exception handling

[BALATON Zoltan]

On Tue, 30 Jan 2024, BALATON Zoltan wrote:

On Thu, 18 Jan 2024, BALATON Zoltan wrote:

These are some small clean ups for target/ppc/excp_helper.c trying to
make this code a bit simpler. No functional change is intended. This
series was submitted before but only partially merged due to freeze
and conflicting series os thia was postponed then to avoid conflicts.


Ping?


Ping^2?

Regards,
BALATON Zoltan


v5:
- rebase on master
- keep logging nip pointing to the sc instruction
- add another patch

v4: Rebased on master dropping what was merged

BALATON Zoltan (9):
 target/ppc: Use env_cpu for cpu_abort in excp_helper
 target/ppc: Readability improvements in exception handlers
 target/ppc: Fix gen_sc to use correct nip
 target/ppc: Move patching nip from exception handler to helper_scv
 target/ppc: Simplify syscall exception handlers
 target/ppc: Clean up ifdefs in excp_helper.c, part 1
 target/ppc: Clean up ifdefs in excp_helper.c, part 2
 target/ppc: Clean up ifdefs in excp_helper.c, part 3
 target/ppc: Remove interrupt handler wrapper functions

target/ppc/cpu.h |   1 +
target/ppc/excp_helper.c | 490 +--
target/ppc/translate.c   |  16 +-
3 files changed, 170 insertions(+), 337 deletions(-)

Re: [PATCH 1/6] target/ppc: Fix 440 tlbwe TLB invalidation gaps

[BALATON Zoltan]

On Thu, 18 Jan 2024, Nicholas Piggin wrote:

The 440 software TLB write entry misses several cases that must flush
the TCG TLB:
- If the new size is smaller than the existing size, the EA no longer
 covered should be flushed. This looks like an inverted inequality test.
- If the TLB PID changes.
- If the TLB attr bit 0 (translation address space) changes.
- If low prot (access control) bits change.

Fix this by removing tricks to avoid TLB flushes, and just invalidate
the TLB if any valid entry is being changed, similarly to 4xx.

Signed-off-by: Nicholas Piggin 


This series was missing a cover letter so patchew did not pick it up 
correctly. However this improves the sam460ex performance a lot so I'd 
like this to be included in 9.0 release. Nick, maybe it's time to start 
merging patches and send a pull request to avoid getting conflicts in last 
minute that could cause series to miss release. So an early pull request 
would help to get everybody on the same page.


Regards,
BALATON Zoltan

[PULL 0/7] Ui patches

[marcandre . lureau]

From: Marc-André Lureau 

The following changes since commit 3ff11e4dcabe2b5b4c26e49d741018ec326f127f:

  Merge tag 'pull-target-arm-20240215' of 
https://git.linaro.org/people/pmaydell/qemu-arm into staging (2024-02-15 
17:36:30 +)

are available in the Git repository at:

  https://gitlab.com/marcandre.lureau/qemu.git tags/ui-pull-request

for you to fetch changes up to 186acfbaf7f325833702f50f75ef5116dc29e233:

  tests/qtest: Depend on dbus_display1_dep (2024-02-16 17:27:22 +0400)


UI-related fixes



Akihiko Odaki (3):
  audio: Depend on dbus_display1_dep
  meson: Explicitly specify dbus-display1.h dependency
  tests/qtest: Depend on dbus_display1_dep

Daniel P. Berrangé (1):
  ui: reject extended clipboard message if not activated

Fiona Ebner (2):
  ui/clipboard: mark type as not available when there is no data
  ui/clipboard: add asserts for update and request

Tianlan Zhou (1):
  ui/console: Fix console resize with placeholder surface

 ui/clipboard.c  | 26 +++---
 ui/console.c|  2 +-
 ui/vnc.c|  5 +
 audio/meson.build   |  3 ++-
 tests/qtest/meson.build |  2 +-
 ui/meson.build  |  2 +-
 6 files changed, 33 insertions(+), 7 deletions(-)

-- 
2.43.1

[PULL 6/7] meson: Explicitly specify dbus-display1.h dependency

[marcandre . lureau]

From: Akihiko Odaki 

Explicitly specify dbus-display1.h as a dependency so that files
depending on it will not get compiled too early.

Fixes: 1222070e7728 ("meson: ensure dbus-display generated code is built before 
other units")
Signed-off-by: Akihiko Odaki 
Reviewed-by: Marc-André Lureau 
Message-Id: <20240214-dbus-v7-2-7eff29f04...@daynix.com>
---
 ui/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/meson.build b/ui/meson.build
index 376e0d771b..0b7e2b6f6b 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -91,7 +91,7 @@ if dbus_display
   '--c-namespace', 'QemuDBus',
   '--generate-c-code', '@BASENAME@'])
   dbus_display1_lib = static_library('dbus-display1', dbus_display1, 
dependencies: gio)
-  dbus_display1_dep = declare_dependency(link_with: dbus_display1_lib, 
include_directories: include_directories('.'))
+  dbus_display1_dep = declare_dependency(link_with: dbus_display1_lib, 
sources: dbus_display1[0])
   dbus_ss.add(when: [gio, dbus_display1_dep],
   if_true: [files(
 'dbus-chardev.c',
-- 
2.43.1

[PULL 1/7] ui: reject extended clipboard message if not activated

[marcandre . lureau]

From: Daniel P. Berrangé 

The extended clipboard message protocol requires that the client
activate the extension by requesting a psuedo encoding. If this
is not done, then any extended clipboard messages from the client
should be considered invalid and the client dropped.

Signed-off-by: Daniel P. Berrangé 
Reviewed-by: Marc-André Lureau 
Message-Id: <20240115095119.654271-1-berra...@redhat.com>
---
 ui/vnc.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/ui/vnc.c b/ui/vnc.c
index 3db87fd89c..af20d24534 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2445,6 +2445,11 @@ static int protocol_client_msg(VncState *vs, uint8_t 
*data, size_t len)
 }
 
 if (read_s32(data, 4) < 0) {
+if (!vnc_has_feature(vs, VNC_FEATURE_CLIPBOARD_EXT)) {
+error_report("vnc: extended clipboard message while disabled");
+vnc_client_error(vs);
+break;
+}
 if (dlen < 4) {
 error_report("vnc: malformed payload (header less than 4 
bytes)"
  " in extended clipboard pseudo-encoding.");
-- 
2.43.1

[PULL 5/7] audio: Depend on dbus_display1_dep

[marcandre . lureau]

From: Akihiko Odaki 

dbusaudio needs dbus_display1_dep.

Fixes: 739362d4205c ("audio: add "dbus" audio backend")
Signed-off-by: Akihiko Odaki 
Reviewed-by: Marc-André Lureau 
Message-Id: <20240214-dbus-v7-1-7eff29f04...@daynix.com>
---
 audio/meson.build | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/audio/meson.build b/audio/meson.build
index c8f658611f..608f35e6af 100644
--- a/audio/meson.build
+++ b/audio/meson.build
@@ -30,7 +30,8 @@ endforeach
 
 if dbus_display
 module_ss = ss.source_set()
-module_ss.add(when: [gio, pixman], if_true: files('dbusaudio.c'))
+module_ss.add(when: [gio, dbus_display1_dep, pixman],
+  if_true: files('dbusaudio.c'))
 audio_modules += {'dbus': module_ss}
 endif
 
-- 
2.43.1

[PULL 7/7] tests/qtest: Depend on dbus_display1_dep

[marcandre . lureau]

From: Akihiko Odaki 

It ensures dbus-display1.c will not be recompiled.

Signed-off-by: Akihiko Odaki 
Reviewed-by: Marc-André Lureau 
Message-Id: <20240214-dbus-v7-3-7eff29f04...@daynix.com>
---
 tests/qtest/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 2b89e8634b..430d49b409 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -344,7 +344,7 @@ if vnc.found()
 endif
 
 if dbus_display
-  qtests += {'dbus-display-test': [dbus_display1, gio]}
+  qtests += {'dbus-display-test': [dbus_display1_dep, gio]}
 endif
 
 qtest_executables = {}
-- 
2.43.1

Re: [PULL 00/56] Misc HW patches for 2024-02-15

[Peter Maydell]

On Thu, 15 Feb 2024 at 17:58, Philippe Mathieu-Daudé  wrote:
>
> The following changes since commit 5767815218efd3cbfd409505ed824d5f356044ae:
>
>   Merge tag 'for_upstream' of 
> https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging (2024-02-14 
> 15:45:52 +)
>
> are available in the Git repository at:
>
>   https://github.com/philmd/qemu.git tags/hw-misc-20240215
>
> for you to fetch changes up to 9a4b35f57eefbfc6977ed47d1f19d839e9e4784d:
>
>   hw/ide/ich9: Use AHCIPCIState typedef (2024-02-15 16:58:47 +0100)
>
> 
> Misc HW patch queue
>
> - Remove unused MIPS SAAR* registers (Phil)
> - Remove warning when testing the TC58128 NAND EEPROM (Peter)
> - KConfig cleanups around ISA SuperI/O and MIPS (Paolo)
> - QDev API uses sanitization (Philippe)
> - Split AHCI model as PCI / SysBus (Philippe)
> - Add SMP support to SPARC Leon3 board (Clément)
>


Applied, thanks.

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

-- PMM

[PULL 4/7] ui/console: Fix console resize with placeholder surface

[marcandre . lureau]

From: Tianlan Zhou 

In `qemu_console_resize()`, the old surface of the console is keeped if the new
console size is the same as the old one. If the old surface is a placeholder,
and the new size of console is the same as the placeholder surface (640*480),
the surface won't be replace.
In this situation, the surface's `QEMU_PLACEHOLDER_FLAG` flag is still set, so
the console won't be displayed in SDL display mode.
This patch fixes this problem by forcing a new surface if the old one is a
placeholder.

Signed-off-by: Tianlan Zhou 
Reviewed-by: Marc-André Lureau 
Message-ID: <20240207172024.8-1-bobby...@126.com>
---
 ui/console.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/console.c b/ui/console.c
index 7db921e3b7..832055675c 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1577,7 +1577,7 @@ void qemu_console_resize(QemuConsole *s, int width, int 
height)
 assert(QEMU_IS_GRAPHIC_CONSOLE(s));
 
 if ((s->scanout.kind != SCANOUT_SURFACE ||
- (surface && surface->flags & QEMU_ALLOCATED_FLAG)) &&
+ (surface && !is_buffer_shared(surface) && !is_placeholder(surface))) 
&&
 qemu_console_get_width(s, -1) == width &&
 qemu_console_get_height(s, -1) == height) {
 return;
-- 
2.43.1

[PULL 2/7] ui/clipboard: mark type as not available when there is no data

[marcandre . lureau]

From: Fiona Ebner 

With VNC, a client can send a non-extended VNC_MSG_CLIENT_CUT_TEXT
message with len=0. In qemu_clipboard_set_data(), the clipboard info
will be updated setting data to NULL (because g_memdup(data, size)
returns NULL when size is 0). If the client does not set the
VNC_ENCODING_CLIPBOARD_EXT feature when setting up the encodings, then
the 'request' callback for the clipboard peer is not initialized.
Later, because data is NULL, qemu_clipboard_request() can be reached
via vdagent_chr_write() and vdagent_clipboard_recv_request() and
there, the clipboard owner's 'request' callback will be attempted to
be called, but that is a NULL pointer.

In particular, this can happen when using the KRDC (22.12.3) VNC
client.

Another scenario leading to the same issue is with two clients (say
noVNC and KRDC):

The noVNC client sets the extension VNC_FEATURE_CLIPBOARD_EXT and
initializes its cbpeer.

The KRDC client does not, but triggers a vnc_client_cut_text() (note
it's not the _ext variant)). There, a new clipboard info with it as
the 'owner' is created and via qemu_clipboard_set_data() is called,
which in turn calls qemu_clipboard_update() with that info.

In qemu_clipboard_update(), the notifier for the noVNC client will be
called, i.e. vnc_clipboard_notify() and also set vs->cbinfo for the
noVNC client. The 'owner' in that clipboard info is the clipboard peer
for the KRDC client, which did not initialize the 'request' function.
That sounds correct to me, it is the owner of that clipboard info.

Then when noVNC sends a VNC_MSG_CLIENT_CUT_TEXT message (it did set
the VNC_FEATURE_CLIPBOARD_EXT feature correctly, so a check for it
passes), that clipboard info is passed to qemu_clipboard_request() and
the original segfault still happens.

Fix the issue by handling updates with size 0 differently. In
particular, mark in the clipboard info that the type is not available.

While at it, switch to g_memdup2(), because g_memdup() is deprecated.

Cc: qemu-sta...@nongnu.org
Fixes: CVE-2023-6683
Reported-by: Markus Frank 
Suggested-by: Marc-André Lureau 
Signed-off-by: Fiona Ebner 
Reviewed-by: Marc-André Lureau 
Tested-by: Markus Frank 
Message-ID: <20240124105749.204610-1-f.eb...@proxmox.com>
---
 ui/clipboard.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/ui/clipboard.c b/ui/clipboard.c
index 3d14bffaf8..b3f6fa3c9e 100644
--- a/ui/clipboard.c
+++ b/ui/clipboard.c
@@ -163,9 +163,15 @@ void qemu_clipboard_set_data(QemuClipboardPeer *peer,
 }
 
 g_free(info->types[type].data);
-info->types[type].data = g_memdup(data, size);
-info->types[type].size = size;
-info->types[type].available = true;
+if (size) {
+info->types[type].data = g_memdup2(data, size);
+info->types[type].size = size;
+info->types[type].available = true;
+} else {
+info->types[type].data = NULL;
+info->types[type].size = 0;
+info->types[type].available = false;
+}
 
 if (update) {
 qemu_clipboard_update(info);
-- 
2.43.1

[PULL 3/7] ui/clipboard: add asserts for update and request

[marcandre . lureau]

From: Fiona Ebner 

Should an issue like CVE-2023-6683 ever appear again in the future,
it will be more obvious which assumption was violated.

Suggested-by: Marc-André Lureau 
Signed-off-by: Fiona Ebner 
Reviewed-by: Marc-André Lureau 
Message-ID: <20240124105749.204610-2-f.eb...@proxmox.com>
---
 ui/clipboard.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/ui/clipboard.c b/ui/clipboard.c
index b3f6fa3c9e..4264884a6c 100644
--- a/ui/clipboard.c
+++ b/ui/clipboard.c
@@ -65,12 +65,24 @@ bool qemu_clipboard_check_serial(QemuClipboardInfo *info, 
bool client)
 
 void qemu_clipboard_update(QemuClipboardInfo *info)
 {
+uint32_t type;
 QemuClipboardNotify notify = {
 .type = QEMU_CLIPBOARD_UPDATE_INFO,
 .info = info,
 };
 assert(info->selection < QEMU_CLIPBOARD_SELECTION__COUNT);
 
+for (type = 0; type < QEMU_CLIPBOARD_TYPE__COUNT; type++) {
+/*
+ * If data is missing, the clipboard owner's 'request' callback needs 
to
+ * be set. Otherwise, there is no way to get the clipboard data and
+ * qemu_clipboard_request() cannot be called.
+ */
+if (info->types[type].available && !info->types[type].data) {
+assert(info->owner && info->owner->request);
+}
+}
+
 notifier_list_notify(&clipboard_notifiers, ¬ify);
 
 if (cbinfo[info->selection] != info) {
@@ -132,6 +144,8 @@ void qemu_clipboard_request(QemuClipboardInfo *info,
 !info->owner)
 return;
 
+assert(info->owner->request);
+
 info->types[type].requested = true;
 info->owner->request(info, type);
 }
-- 
2.43.1

RE: [PATCH 1/4] target/riscv: Add CSR tcontrol of debug trigger module

[張哲嘉]

Hi Daniel,

> -Original Message-
> From: Daniel Henrique Barboza 
> Sent: Friday, February 16, 2024 8:51 PM
> To: Alvin Che-Chia Chang(張哲嘉) ;
> qemu-ri...@nongnu.org; qemu-devel@nongnu.org
> Cc: alistair.fran...@wdc.com; bin.m...@windriver.com;
> liwei1...@gmail.com; zhiwei_...@linux.alibaba.com
> Subject: Re: [PATCH 1/4] target/riscv: Add CSR tcontrol of debug trigger 
> module
> 
> 
> 
> On 2/16/24 03:13, Alvin Chang wrote:
> > The RISC-V debug specification defines an optional CSR "tcontrol"
> > within the trigger module. This commit adds its read/write operations
> > and related bit-field definitions.
> >
> > Signed-off-by: Alvin Chang 
> > ---
> >   target/riscv/cpu.h  |  1 +
> >   target/riscv/cpu_bits.h |  3 +++
> >   target/riscv/csr.c  | 15 +++
> >   3 files changed, 19 insertions(+)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index
> > f52dce78ba..f9ae3e3025 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -364,6 +364,7 @@ struct CPUArchState {
> >   target_ulong tdata1[RV_MAX_TRIGGERS];
> >   target_ulong tdata2[RV_MAX_TRIGGERS];
> >   target_ulong tdata3[RV_MAX_TRIGGERS];
> > +target_ulong tcontrol;
> >   target_ulong mcontext;
> >   struct CPUBreakpoint *cpu_breakpoint[RV_MAX_TRIGGERS];
> >   struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS]; diff
> > --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h index
> > fc2068ee4d..3b3a7a0fa4 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -353,6 +353,7 @@
> >   #define CSR_TDATA2  0x7a2
> >   #define CSR_TDATA3  0x7a3
> >   #define CSR_TINFO   0x7a4
> > +#define CSR_TCONTROL0x7a5
> >   #define CSR_MCONTEXT0x7a8
> >
> >   /* Debug Mode Registers */
> > @@ -900,6 +901,8 @@ typedef enum RISCVException {
> >   #define JVT_BASE   (~0x3F)
> >
> >   /* Debug Sdtrig CSR masks */
> > +#define TCONTROL_MTE   BIT(3)
> > +#define TCONTROL_MPTE  BIT(7)
> >   #define MCONTEXT32 0x003F
> >   #define MCONTEXT64
> 0x1FFFULL
> >   #define MCONTEXT32_HCONTEXT0x007F
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c index
> > d4e8ac13b9..816c530481 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -3937,6 +3937,20 @@ static RISCVException read_tinfo(CPURISCVState
> *env, int csrno,
> >   return RISCV_EXCP_NONE;
> >   }
> >
> > +static RISCVException read_tcontrol(CPURISCVState *env, int csrno,
> > +target_ulong *val) {
> > +*val = env->tcontrol;
> > +return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_tcontrol(CPURISCVState *env, int csrno,
> > + target_ulong val) {
> > +env->tcontrol = val & (TCONTROL_MPTE | TCONTROL_MTE);
> > +return RISCV_EXCP_NONE;
> > +}
> > +
> >   static RISCVException read_mcontext(CPURISCVState *env, int csrno,
> >   target_ulong *val)
> >   {
> > @@ -4861,6 +4875,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> >   [CSR_TDATA2]=  { "tdata2",   debug, read_tdata,
> write_tdata},
> >   [CSR_TDATA3]=  { "tdata3",   debug, read_tdata,
> write_tdata},
> >   [CSR_TINFO] =  { "tinfo",debug, read_tinfo,
> write_ignore   },
> > +[CSR_TCONTROL]  =  { "tcontrol", debug, read_tcontrol,
> > + write_tcontrol },
> 
> The spec reads:
> 
> "This optional register is only accessible in M-mode and Debug Mode and
> provides
>   various control bits related to triggers."
> 
> "debug()" is checking only if we have the 'debug' cpu option enabled:
> 
> 
> static RISCVException debug(CPURISCVState *env, int csrno) {
>  if (riscv_cpu_cfg(env)->debug) {
>  return RISCV_EXCP_NONE;
>  }
> 
>  return RISCV_EXCP_ILLEGAL_INST;
> }
> 
> 
> It looks like we don't have a "Debug Mode" model.

Yes, currently RISC-V QEMU doesn't have "Debug Mode", so I just ignore it and 
only consider M-mode here.

> 
> Section 4.1 of the spec mentions the following about "Debug Mode":
> 
> "1. All implemented instructions operate just as they do in M-mode, unless an
>   exception is mentioned in this list.
>   2. All operations are executed with machine mode privilege, except that
> additional
>   Debug Mode CSRs are accessible and MPRV in mstatus may be ignored
> according to
>   mprven. Full permission checks, or a relaxed set of permission checks, will
> apply
>   according to relaxedpriv (...)"
> 
> 
> So, if the operations are "executed with machine mode privilege" then can we
> expect
> env->priv == PRV_M ? As it is now tcontrol will execute in any mode, so
> env->checking
> for PRV_M seems reasonable.

The riscv_csrrw_check() function has checked the privilege level by the 
accessed CSR address.
Please see https://github.com/qemu/qemu/blob/master/target/

Re: QEMU 8.2.0 aarch64 sve ldff1b returning 1 byte when 16 are expected

[Peter Maydell]

On Fri, 16 Feb 2024 at 11:40, Alex Bennée  wrote:
>
> Mark Charney OS  writes:
> > Talking to Alex Bennee, he pointed out:
> >
> >> I'm wondering if this is a result of the fix in 6d03226b422
> >> (plugins: force slow path when plugins instrument memory ops). This
> >> will always force the slow path which is where we instrument the
> >> operation.

Yes, this sounds like it. The implementation of the SVE first-fault
instructions is that we probe the TLB to see if the address for bytes
beyond the first load is backed by host memory. If it's actually backed
by MMIO then we will not load from that. A comment in sve_ldnfff1_r()
explains why:

 * Per the MemSingleNF pseudocode, a no-fault load from Device memory
 * must not actually hit the bus -- it returns (UNKNOWN, FAULT) instead.
 *
 * Unfortuately we do not have access to the memory attributes from the
 * PTE to tell Device memory from Normal memory.  So we make a mostly
 * correct check, and indicate (UNKNOWN, FAULT) for any MMIO.
 * This gives the right answer for the common cases of "Normal memory,
 * backed by host RAM" and "Device memory, backed by MMIO".
 * The architecture allows us to suppress an NF load and return
 * (UNKNOWN, FAULT) for any reason, so our behaviour for the corner
 * case of "Normal memory, backed by MMIO" is permitted.  The case we
 * get wrong is "Device memory, backed by host RAM", for which we
 * should return (UNKNOWN, FAULT) for but do not.

Unfortunately because the plugin memory-access handling forces the
slow-path it is TLB_MMIO here, and so it ends up in the "normal memory,
backed by MMIO" corner case, where we do the architecturally-permitted
but perhaps unexpected thing of only loading one byte.

thanks
-- PMM