Re: [PATCH 2/6] pnv/xive: Fix possible undefined shift error in group size calculation

[Cédric Le Goater]

On 3/17/25 06:23, Nicholas Piggin wrote:

Coverity discovered a potential shift overflow in group size calculation
in the case of a guest error. Add checks and logs to ensure a issues are
caught.

Make the group and crowd error checking code more similar to one another
while here.

Resolves: Coverity CID 1593724
Fixes: 9cb7f6ebed60 ("ppc/xive2: Support group-matching when looking for 
target")
Cc: Cédric Le Goater 
Signed-off-by: Nicholas Piggin 



Reviewed-by: Cédric Le Goater 

Thanks,

C.



---
  hw/intc/xive.c  | 27 ---
  hw/intc/xive2.c | 19 ++-
  2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index e86f2749328..3eb28c2265d 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1662,12 +1662,20 @@ uint32_t xive_get_vpgroup_size(uint32_t nvp_index)
   * (starting with the least significant bits) in the NVP index
   * gives the size of the group.
   */
-return 1 << (ctz32(~nvp_index) + 1);
+int first_zero = cto32(nvp_index);
+if (first_zero >= 31) {
+qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid group index 0x%08x",
+   nvp_index);
+return 0;
+}
+
+return 1U << (first_zero + 1);
  }
  
  static uint8_t xive_get_group_level(bool crowd, bool ignore,

  uint32_t nvp_blk, uint32_t nvp_index)
  {
+int first_zero;
  uint8_t level;
  
  if (!ignore) {

@@ -1675,12 +1683,25 @@ static uint8_t xive_get_group_level(bool crowd, bool 
ignore,
  return 0;
  }
  
-level = (ctz32(~nvp_index) + 1) & 0b;

+first_zero = cto32(nvp_index);
+if (first_zero >= 31) {
+qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid group index 0x%08x",
+   nvp_index);
+return 0;
+}
+
+level = (first_zero + 1) & 0b;
  if (crowd) {
  uint32_t blk;
  
  /* crowd level is bit position of first 0 from the right in nvp_blk */

-blk = ctz32(~nvp_blk) + 1;
+first_zero = cto32(nvp_blk);
+if (first_zero >= 31) {
+qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid crowd block 0x%08x",
+   nvp_blk);
+return 0;
+}
+blk = first_zero + 1;
  
  /*

   * Supported crowd sizes are 2^1, 2^2, and 2^4. 2^3 is not supported.
diff --git a/hw/intc/xive2.c b/hw/intc/xive2.c
index f8ef6154878..311b42e15d3 100644
--- a/hw/intc/xive2.c
+++ b/hw/intc/xive2.c
@@ -1153,13 +1153,15 @@ static bool xive2_vp_match_mask(uint32_t cam1, uint32_t 
cam2,
  
  static uint8_t xive2_get_vp_block_mask(uint32_t nvt_blk, bool crowd)

  {
-uint8_t size, block_mask = 0b;
+uint8_t block_mask = 0b;
  
  /* 3 supported crowd sizes: 2, 4, 16 */

  if (crowd) {
-size = xive_get_vpgroup_size(nvt_blk);
-if (size == 8) {
-qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid crowd size of 8n");
+uint32_t size = xive_get_vpgroup_size(nvt_blk);
+
+if (size != 2 && size != 4 && size != 16) {
+qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid crowd size of %d",
+   size);
  return block_mask;
  }
  block_mask &= ~(size - 1);
@@ -1172,7 +1174,14 @@ static uint32_t xive2_get_vp_index_mask(uint32_t 
nvt_index, bool cam_ignore)
  uint32_t index_mask = 0xFF; /* 24 bits */
  
  if (cam_ignore) {

-index_mask &= ~(xive_get_vpgroup_size(nvt_index) - 1);
+uint32_t size = xive_get_vpgroup_size(nvt_index);
+
+if (size < 2) {
+qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid group size of %d",
+   size);
+return index_mask;
+}
+index_mask &= ~(size - 1);
  }
  return index_mask;
  }

Re: [PATCH 3/6] ppc/xive2: Fix logical / bitwise comparison typo

[Cédric Le Goater]

On 3/17/25 06:23, Nicholas Piggin wrote:

The comparison as written is always false (perhaps confusingly, because
the functions/macros are not really booleans but return 0 or the tested
bit value). Change to use logical-and.

Resolves: Coverity CID 1593721
Cc: Cédric Le Goater 
Signed-off-by: Nicholas Piggin 



Reviewed-by: Cédric Le Goater 

Thanks,

C.



---
  hw/intc/xive2.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/intc/xive2.c b/hw/intc/xive2.c
index 311b42e15d3..7d584dfafaf 100644
--- a/hw/intc/xive2.c
+++ b/hw/intc/xive2.c
@@ -1344,7 +1344,7 @@ static void xive2_router_end_notify(Xive2Router *xrtr, 
uint8_t end_blk,
  return;
  }
  
-if (xive2_end_is_crowd(&end) & !xive2_end_is_ignore(&end)) {

+if (xive2_end_is_crowd(&end) && !xive2_end_is_ignore(&end)) {
  qemu_log_mask(LOG_GUEST_ERROR,
"XIVE: invalid END, 'crowd' bit requires 'ignore' 
bit\n");
  return;

Re: Broken NetBSD Orange Pi image URL in QEMU tests

[Thomas Huth]

On 15/03/2025 22.01, Niek Linnenbank wrote:

Hello Stefan,

As of today, it seems the URL is working properly again. I've done a few 
downloads without any error.
What I did notice is that NetBSD provides a 'cdn.netbsd.org cdn.netbsd.org>' also, but I can't see any noticable difference yet.


Oh, weird, in commit 380f7268b7ba4a6 I had to switch the URL from the cdn to 
the archive one since the file was not available on the cdn server 
anymore... but now it seems to be back! Certainly something to keep in mind 
in case we face problems with this asset again.


 Thomas

Re: [PATCH v3 6/7] memory: Attach MemoryAttributeManager to guest_memfd-backed RAMBlocks

[Chenyi Qiang]

On 3/17/2025 2:18 PM, Tony Lindgren wrote:
> Hi,
> 
> On Mon, Mar 10, 2025 at 04:18:34PM +0800, Chenyi Qiang wrote:
>> --- a/system/physmem.c
>> +++ b/system/physmem.c
>> @@ -1885,6 +1886,16 @@ static void ram_block_add(RAMBlock *new_block, Error 
>> **errp)
>>  qemu_mutex_unlock_ramlist();
>>  goto out_free;
>>  }
>> +
>> +new_block->memory_attribute_manager = 
>> MEMORY_ATTRIBUTE_MANAGER(object_new(TYPE_MEMORY_ATTRIBUTE_MANAGER));
>> +if 
>> (memory_attribute_manager_realize(new_block->memory_attribute_manager, 
>> new_block->mr)) {
>> +error_setg(errp, "Failed to realize memory attribute manager");
>> +object_unref(OBJECT(new_block->memory_attribute_manager));
>> +close(new_block->guest_memfd);
>> +ram_block_discard_require(false);
>> +qemu_mutex_unlock_ramlist();
>> +goto out_free;
>> +}
>>  }
>>  
>>  ram_size = (new_block->offset + new_block->max_length) >> 
>> TARGET_PAGE_BITS;
> 
> Might as well put the above into a separate memory manager init function
> to start with. It keeps the goto out_free error path unified, and makes
> things more future proof if the rest of ram_block_add() ever develops a
> need to check for errors.

Which part to be defined in a separate function? The init function of
object_new() + realize(), or the error handling operation
(object_unref() + close() + ram_block_discard_require(false))?

If need to check for errors in the rest of ram_block_add() in future,
how about adding a new label before out_free and move the error handling
there?

> 
> Regards,
> 
> Tony

[PATCH 0/4] hw/intc/loongarch_pch: Cleanup with register name

[Bibo Mao]

Here is cleanup with register name, the width of some registers are 64 bit.
To emulate 32 bit memory access, it is split into two registers with suffix
name low and high. Here register name and (register name + 4) is used
rather than splitting into two registers.

There is no function change in this patch set.

Bibo Mao (4):
  hw/intc/loongarch_pch: Use default path when access some registers
  hw/intc/loongarch_pch: Rename register name
  hw/intc/loongarch_pch: Rename macro PCH_PIC_xxx_OFFSET with
PCH_PIC_xxx
  hw/intc/loongarch_pch: Remove some duplicate macro

 hw/intc/loongarch_pch_pic.c| 82 +++---
 hw/loongarch/virt.c|  4 +-
 include/hw/intc/loongarch_pic_common.h | 36 ---
 3 files changed, 49 insertions(+), 73 deletions(-)


base-commit: aa90f1161bb17a4863e16ec2f75104cff0752d4e
-- 
2.39.3

Re: [PATCH v3 6/7] memory: Attach MemoryAttributeManager to guest_memfd-backed RAMBlocks

[Tony Lindgren]

On Mon, Mar 17, 2025 at 03:32:16PM +0800, Chenyi Qiang wrote:
> 
> 
> On 3/17/2025 2:18 PM, Tony Lindgren wrote:
> > Hi,
> > 
> > On Mon, Mar 10, 2025 at 04:18:34PM +0800, Chenyi Qiang wrote:
> >> --- a/system/physmem.c
> >> +++ b/system/physmem.c
> >> @@ -1885,6 +1886,16 @@ static void ram_block_add(RAMBlock *new_block, 
> >> Error **errp)
> >>  qemu_mutex_unlock_ramlist();
> >>  goto out_free;
> >>  }
> >> +
> >> +new_block->memory_attribute_manager = 
> >> MEMORY_ATTRIBUTE_MANAGER(object_new(TYPE_MEMORY_ATTRIBUTE_MANAGER));
> >> +if 
> >> (memory_attribute_manager_realize(new_block->memory_attribute_manager, 
> >> new_block->mr)) {
> >> +error_setg(errp, "Failed to realize memory attribute 
> >> manager");
> >> +object_unref(OBJECT(new_block->memory_attribute_manager));
> >> +close(new_block->guest_memfd);
> >> +ram_block_discard_require(false);
> >> +qemu_mutex_unlock_ramlist();
> >> +goto out_free;
> >> +}
> >>  }
> >>  
> >>  ram_size = (new_block->offset + new_block->max_length) >> 
> >> TARGET_PAGE_BITS;
> > 
> > Might as well put the above into a separate memory manager init function
> > to start with. It keeps the goto out_free error path unified, and makes
> > things more future proof if the rest of ram_block_add() ever develops a
> > need to check for errors.
> 
> Which part to be defined in a separate function? The init function of
> object_new() + realize(), or the error handling operation
> (object_unref() + close() + ram_block_discard_require(false))?

I was thinking the whole thing, including freeing :) But maybe there's
something more to consider to keep calls paired.

> If need to check for errors in the rest of ram_block_add() in future,
> how about adding a new label before out_free and move the error handling
> there?

Yeah that would work too.

Regards,

Tony

[PATCH 2/4] hw/intc/loongarch_pch: Rename register name

[Bibo Mao]

For some registers with width 8 bytes, its name is something like
PCH_PIC_INT_ID_LO and PCH_PIC_INT_ID_HI. From hardware manual,
register name is PCH_PIC_INT_ID instead. Here name PCH_PIC_INT_ID
is used, and PCH_PIC_INT_ID + 4 is used for PCH_PIC_INT_ID_HI.

Signed-off-by: Bibo Mao 
---
 hw/intc/loongarch_pch_pic.c| 32 +-
 hw/loongarch/virt.c|  2 +-
 include/hw/intc/loongarch_pic_common.h | 27 --
 3 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/hw/intc/loongarch_pch_pic.c b/hw/intc/loongarch_pch_pic.c
index 3fc4227159..428809b927 100644
--- a/hw/intc/loongarch_pch_pic.c
+++ b/hw/intc/loongarch_pch_pic.c
@@ -79,10 +79,10 @@ static uint64_t loongarch_pch_pic_low_readw(void *opaque, 
hwaddr addr,
 uint32_t offset = addr & 0xfff;
 
 switch (offset) {
-case PCH_PIC_INT_ID_LO:
+case PCH_PIC_INT_ID:
 val = PCH_PIC_INT_ID_VAL;
 break;
-case PCH_PIC_INT_ID_HI:
+case PCH_PIC_INT_ID + 4:
 /*
  * With 7A1000 manual
  *   bit  0-15 pch irqchip version
@@ -90,22 +90,22 @@ static uint64_t loongarch_pch_pic_low_readw(void *opaque, 
hwaddr addr,
  */
 val = deposit32(PCH_PIC_INT_ID_VER, 16, 16, s->irq_num - 1);
 break;
-case PCH_PIC_INT_MASK_LO:
+case PCH_PIC_INT_MASK:
 val = (uint32_t)s->int_mask;
 break;
-case PCH_PIC_INT_MASK_HI:
+case PCH_PIC_INT_MASK + 4:
 val = s->int_mask >> 32;
 break;
-case PCH_PIC_INT_EDGE_LO:
+case PCH_PIC_INT_EDGE:
 val = (uint32_t)s->intedge;
 break;
-case PCH_PIC_INT_EDGE_HI:
+case PCH_PIC_INT_EDGE + 4:
 val = s->intedge >> 32;
 break;
-case PCH_PIC_HTMSI_EN_LO:
+case PCH_PIC_HTMSI_EN:
 val = (uint32_t)s->htmsi_en;
 break;
-case PCH_PIC_HTMSI_EN_HI:
+case PCH_PIC_HTMSI_EN + 4:
 val = s->htmsi_en >> 32;
 break;
 default:
@@ -135,7 +135,7 @@ static void loongarch_pch_pic_low_writew(void *opaque, 
hwaddr addr,
 trace_loongarch_pch_pic_low_writew(size, addr, data);
 
 switch (offset) {
-case PCH_PIC_INT_MASK_LO:
+case PCH_PIC_INT_MASK:
 old = s->int_mask;
 s->int_mask = get_writew_val(old, data, 0);
 old_valid = (uint32_t)old;
@@ -146,7 +146,7 @@ static void loongarch_pch_pic_low_writew(void *opaque, 
hwaddr addr,
 pch_pic_update_irq(s, (~old_valid & data), 0);
 }
 break;
-case PCH_PIC_INT_MASK_HI:
+case PCH_PIC_INT_MASK + 4:
 old = s->int_mask;
 s->int_mask = get_writew_val(old, data, 1);
 old_valid = (uint32_t)(old >> 32);
@@ -159,20 +159,20 @@ static void loongarch_pch_pic_low_writew(void *opaque, 
hwaddr addr,
 pch_pic_update_irq(s, int_mask << 32, 0);
 }
 break;
-case PCH_PIC_INT_EDGE_LO:
+case PCH_PIC_INT_EDGE:
 s->intedge = get_writew_val(s->intedge, data, 0);
 break;
-case PCH_PIC_INT_EDGE_HI:
+case PCH_PIC_INT_EDGE + 4:
 s->intedge = get_writew_val(s->intedge, data, 1);
 break;
-case PCH_PIC_INT_CLEAR_LO:
+case PCH_PIC_INT_CLEAR:
 if (s->intedge & data) {
 s->intirr &= (~data);
 pch_pic_update_irq(s, data, 0);
 s->intisr &= (~data);
 }
 break;
-case PCH_PIC_INT_CLEAR_HI:
+case PCH_PIC_INT_CLEAR + 4:
 value <<= 32;
 if (s->intedge & value) {
 s->intirr &= (~value);
@@ -180,10 +180,10 @@ static void loongarch_pch_pic_low_writew(void *opaque, 
hwaddr addr,
 s->intisr &= (~value);
 }
 break;
-case PCH_PIC_HTMSI_EN_LO:
+case PCH_PIC_HTMSI_EN:
 s->htmsi_en = get_writew_val(s->htmsi_en, data, 0);
 break;
-case PCH_PIC_HTMSI_EN_HI:
+case PCH_PIC_HTMSI_EN + 4:
 s->htmsi_en = get_writew_val(s->htmsi_en, data, 1);
 break;
 default:
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index a5840ff968..f886b08c6d 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -430,7 +430,7 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
 VIRT_IOAPIC_REG_BASE + PCH_PIC_ROUTE_ENTRY_OFFSET,
 sysbus_mmio_get_region(d, 1));
 memory_region_add_subregion(get_system_memory(),
-VIRT_IOAPIC_REG_BASE + PCH_PIC_INT_STATUS_LO,
+VIRT_IOAPIC_REG_BASE + PCH_PIC_INT_STATUS,
 sysbus_mmio_get_region(d, 2));
 
 /* Connect pch_pic irqs to extioi */
diff --git a/include/hw/intc/loongarch_pic_common.h 
b/include/hw/intc/loongarch_pic_common.h
index 43cce48978..c04471b08d 100644
--- a/include/hw/intc/loongarch_pic_common.h
+++ b/include/hw/intc/loongarch_pic_common.h
@@ -12,28 +12,19 @@
 
 #define PCH_PIC_INT_ID_VAL  0x700UL
 #define PCH_PIC_INT_ID_VER  0x1UL
-

[PATCH 1/4] hw/intc/loongarch_pch: Use default path when access some registers

[Bibo Mao]

For some registers such as PCH_PIC_AUTO_CTRL0_LO etc which are not
emulated, emulation driver does nothing. It is the same with default
handling, here remove these registers and use the default path for
simplification.

Signed-off-by: Bibo Mao 
---
 hw/intc/loongarch_pch_pic.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/hw/intc/loongarch_pch_pic.c b/hw/intc/loongarch_pch_pic.c
index acd75ccb0c..3fc4227159 100644
--- a/hw/intc/loongarch_pch_pic.c
+++ b/hw/intc/loongarch_pch_pic.c
@@ -108,11 +108,6 @@ static uint64_t loongarch_pch_pic_low_readw(void *opaque, 
hwaddr addr,
 case PCH_PIC_HTMSI_EN_HI:
 val = s->htmsi_en >> 32;
 break;
-case PCH_PIC_AUTO_CTRL0_LO:
-case PCH_PIC_AUTO_CTRL0_HI:
-case PCH_PIC_AUTO_CTRL1_LO:
-case PCH_PIC_AUTO_CTRL1_HI:
-break;
 default:
 break;
 }
@@ -191,11 +186,6 @@ static void loongarch_pch_pic_low_writew(void *opaque, 
hwaddr addr,
 case PCH_PIC_HTMSI_EN_HI:
 s->htmsi_en = get_writew_val(s->htmsi_en, data, 1);
 break;
-case PCH_PIC_AUTO_CTRL0_LO:
-case PCH_PIC_AUTO_CTRL0_HI:
-case PCH_PIC_AUTO_CTRL1_LO:
-case PCH_PIC_AUTO_CTRL1_HI:
-break;
 default:
 break;
 }
-- 
2.39.3

Re: [PATCH RFC v4 00/11] virtio-net: Offload hashing without eBPF

[Lei Yang]

QE tested this RFC series of patches with virtio-net regression tests,
everything works fine.

Tested-by: Lei Yang 

On Thu, Mar 13, 2025 at 2:56 PM Akihiko Odaki  wrote:
>
> I'm proposing to add a feature to offload virtio-net RSS/hash report to
> Linux. This series contain patches to utilize the proposed Linux feature.
> The patches for Linux are available at:
> https://lore.kernel.org/r/20250307-rss-v9-0-df7662402...@daynix.com/
>
> This work was presented at LPC 2024:
> https://lpc.events/event/18/contributions/1963/
>
> Patch "docs/devel/ebpf_rss.rst: Update for peer RSS" provides comparion
> of existing RSS mechanism and the new one (called "peer RSS") and
> explains how QEMU selects one.
>
> ---
> Changes in v4:
> - Rebased.
> - Added a reference to the documentation to the cover letter.
> - Link to v3: 
> https://lore.kernel.org/r/20240915-hash-v3-0-79cb08d28...@daynix.com
>
> ---
> Akihiko Odaki (11):
>   qdev-properties: Add DEFINE_PROP_ON_OFF_AUTO_BIT64()
>   net/vhost-vdpa: Report hashing capability
>   virtio-net: Move virtio_net_get_features() down
>   virtio-net: Retrieve peer hashing capability
>   net/vhost-vdpa: Remove dummy SetSteeringEBPF
>   virtio-net: Add hash type options
>   net: Allow configuring virtio hashing
>   virtio-net: Use qemu_set_vnet_hash()
>   virtio-net: Offload hashing without vhost
>   tap: Report virtio-net hashing support on Linux
>   docs/devel/ebpf_rss.rst: Update for peer RSS
>
>  docs/devel/ebpf_rss.rst|  23 ++-
>  include/hw/qdev-properties.h   |  18 +++
>  include/hw/virtio/virtio-net.h |   6 +-
>  include/net/net.h  |  20 +++
>  net/tap-linux.h|   2 +
>  net/tap_int.h  |   3 +
>  hw/core/qdev-properties.c  |  67 -
>  hw/net/virtio-net.c| 331 
> +
>  net/net.c  |  14 ++
>  net/tap-bsd.c  |  10 ++
>  net/tap-linux.c|  18 +++
>  net/tap-solaris.c  |  10 ++
>  net/tap-stub.c |  10 ++
>  net/tap.c  |  15 ++
>  net/vhost-vdpa.c   |  41 -
>  15 files changed, 478 insertions(+), 110 deletions(-)
> ---
> base-commit: 825b96dbcee23d134b691fc75618b59c5f53da32
> change-id: 20240828-hash-628329a45d4d
>
> Best regards,
> --
> Akihiko Odaki 
>
>

[PATCH 3/4] hw/intc/loongarch_pch: Rename macro PCH_PIC_xxx_OFFSET with PCH_PIC_xxx

[Bibo Mao]

Macro PCH_PIC_HTMSI_VEC_OFFSET and PCH_PIC_ROUTE_ENTRY_OFFSET is renamed
as PCH_PIC_HTMSI_VEC and PCH_PIC_ROUTE_ENTRY separately, it is easier to
understand.

Signed-off-by: Bibo Mao 
---
 hw/intc/loongarch_pch_pic.c| 20 ++--
 hw/loongarch/virt.c|  2 +-
 include/hw/intc/loongarch_pic_common.h |  4 ++--
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/hw/intc/loongarch_pch_pic.c b/hw/intc/loongarch_pch_pic.c
index 428809b927..087ab80ff8 100644
--- a/hw/intc/loongarch_pch_pic.c
+++ b/hw/intc/loongarch_pch_pic.c
@@ -251,18 +251,18 @@ static uint64_t loongarch_pch_pic_readb(void *opaque, 
hwaddr addr,
 {
 LoongArchPICCommonState *s = LOONGARCH_PIC_COMMON(opaque);
 uint64_t val = 0;
-uint32_t offset = (addr & 0xfff) + PCH_PIC_ROUTE_ENTRY_OFFSET;
+uint32_t offset = (addr & 0xfff) + PCH_PIC_ROUTE_ENTRY;
 int64_t offset_tmp;
 
 switch (offset) {
-case PCH_PIC_HTMSI_VEC_OFFSET ... PCH_PIC_HTMSI_VEC_END:
-offset_tmp = offset - PCH_PIC_HTMSI_VEC_OFFSET;
+case PCH_PIC_HTMSI_VEC ... PCH_PIC_HTMSI_VEC_END:
+offset_tmp = offset - PCH_PIC_HTMSI_VEC;
 if (offset_tmp >= 0 && offset_tmp < 64) {
 val = s->htmsi_vector[offset_tmp];
 }
 break;
-case PCH_PIC_ROUTE_ENTRY_OFFSET ... PCH_PIC_ROUTE_ENTRY_END:
-offset_tmp = offset - PCH_PIC_ROUTE_ENTRY_OFFSET;
+case PCH_PIC_ROUTE_ENTRY ... PCH_PIC_ROUTE_ENTRY_END:
+offset_tmp = offset - PCH_PIC_ROUTE_ENTRY;
 if (offset_tmp >= 0 && offset_tmp < 64) {
 val = s->route_entry[offset_tmp];
 }
@@ -280,19 +280,19 @@ static void loongarch_pch_pic_writeb(void *opaque, hwaddr 
addr,
 {
 LoongArchPICCommonState *s = LOONGARCH_PIC_COMMON(opaque);
 int32_t offset_tmp;
-uint32_t offset = (addr & 0xfff) + PCH_PIC_ROUTE_ENTRY_OFFSET;
+uint32_t offset = (addr & 0xfff) + PCH_PIC_ROUTE_ENTRY;
 
 trace_loongarch_pch_pic_writeb(size, addr, data);
 
 switch (offset) {
-case PCH_PIC_HTMSI_VEC_OFFSET ... PCH_PIC_HTMSI_VEC_END:
-offset_tmp = offset - PCH_PIC_HTMSI_VEC_OFFSET;
+case PCH_PIC_HTMSI_VEC ... PCH_PIC_HTMSI_VEC_END:
+offset_tmp = offset - PCH_PIC_HTMSI_VEC;
 if (offset_tmp >= 0 && offset_tmp < 64) {
 s->htmsi_vector[offset_tmp] = (uint8_t)(data & 0xff);
 }
 break;
-case PCH_PIC_ROUTE_ENTRY_OFFSET ... PCH_PIC_ROUTE_ENTRY_END:
-offset_tmp = offset - PCH_PIC_ROUTE_ENTRY_OFFSET;
+case PCH_PIC_ROUTE_ENTRY ... PCH_PIC_ROUTE_ENTRY_END:
+offset_tmp = offset - PCH_PIC_ROUTE_ENTRY;
 if (offset_tmp >= 0 && offset_tmp < 64) {
 s->route_entry[offset_tmp] = (uint8_t)(data & 0xff);
 }
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index f886b08c6d..a18455bc89 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -427,7 +427,7 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
 memory_region_add_subregion(get_system_memory(), VIRT_IOAPIC_REG_BASE,
 sysbus_mmio_get_region(d, 0));
 memory_region_add_subregion(get_system_memory(),
-VIRT_IOAPIC_REG_BASE + PCH_PIC_ROUTE_ENTRY_OFFSET,
+VIRT_IOAPIC_REG_BASE + PCH_PIC_ROUTE_ENTRY,
 sysbus_mmio_get_region(d, 1));
 memory_region_add_subregion(get_system_memory(),
 VIRT_IOAPIC_REG_BASE + PCH_PIC_INT_STATUS,
diff --git a/include/hw/intc/loongarch_pic_common.h 
b/include/hw/intc/loongarch_pic_common.h
index c04471b08d..b33bebb129 100644
--- a/include/hw/intc/loongarch_pic_common.h
+++ b/include/hw/intc/loongarch_pic_common.h
@@ -19,9 +19,9 @@
 #define PCH_PIC_INT_CLEAR   0x80
 #define PCH_PIC_AUTO_CTRL0  0xc0
 #define PCH_PIC_AUTO_CTRL1  0xe0
-#define PCH_PIC_ROUTE_ENTRY_OFFSET  0x100
+#define PCH_PIC_ROUTE_ENTRY 0x100
 #define PCH_PIC_ROUTE_ENTRY_END 0x13f
-#define PCH_PIC_HTMSI_VEC_OFFSET0x200
+#define PCH_PIC_HTMSI_VEC   0x200
 #define PCH_PIC_HTMSI_VEC_END   0x23f
 #define PCH_PIC_INT_STATUS  0x3a0
 #define PCH_PIC_INT_POL 0x3e0
-- 
2.39.3

[PATCH 4/4] hw/intc/loongarch_pch: Remove some duplicate macro

[Bibo Mao]

The meaning of macro definition STATUS_LO_START is simliar with
PCH_PIC_INT_STATUS, only that offset is different, the same for
macro POL_LO_START. Now remove these duplicated macro definitions.

Signed-off-by: Bibo Mao 
---
 hw/intc/loongarch_pch_pic.c| 20 ++--
 include/hw/intc/loongarch_pic_common.h |  5 -
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/hw/intc/loongarch_pch_pic.c b/hw/intc/loongarch_pch_pic.c
index 087ab80ff8..ce5ef5b926 100644
--- a/hw/intc/loongarch_pch_pic.c
+++ b/hw/intc/loongarch_pch_pic.c
@@ -196,19 +196,19 @@ static uint64_t loongarch_pch_pic_high_readw(void 
*opaque, hwaddr addr,
 {
 LoongArchPICCommonState *s = LOONGARCH_PIC_COMMON(opaque);
 uint64_t val = 0;
-uint32_t offset = addr & 0xfff;
+uint32_t offset = addr + PCH_PIC_INT_STATUS;
 
 switch (offset) {
-case STATUS_LO_START:
+case PCH_PIC_INT_STATUS:
 val = (uint32_t)(s->intisr & (~s->int_mask));
 break;
-case STATUS_HI_START:
+case PCH_PIC_INT_STATUS + 4:
 val = (s->intisr & (~s->int_mask)) >> 32;
 break;
-case POL_LO_START:
+case PCH_PIC_INT_POL:
 val = (uint32_t)s->int_polarity;
 break;
-case POL_HI_START:
+case PCH_PIC_INT_POL + 4:
 val = s->int_polarity >> 32;
 break;
 default:
@@ -224,21 +224,21 @@ static void loongarch_pch_pic_high_writew(void *opaque, 
hwaddr addr,
 {
 LoongArchPICCommonState *s = LOONGARCH_PIC_COMMON(opaque);
 uint32_t offset, data = (uint32_t)value;
-offset = addr & 0xfff;
+offset = addr + PCH_PIC_INT_STATUS;
 
 trace_loongarch_pch_pic_high_writew(size, addr, data);
 
 switch (offset) {
-case STATUS_LO_START:
+case PCH_PIC_INT_STATUS:
 s->intisr = get_writew_val(s->intisr, data, 0);
 break;
-case STATUS_HI_START:
+case PCH_PIC_INT_STATUS + 4:
 s->intisr = get_writew_val(s->intisr, data, 1);
 break;
-case POL_LO_START:
+case PCH_PIC_INT_POL:
 s->int_polarity = get_writew_val(s->int_polarity, data, 0);
 break;
-case POL_HI_START:
+case PCH_PIC_INT_POL + 4:
 s->int_polarity = get_writew_val(s->int_polarity, data, 1);
 break;
 default:
diff --git a/include/hw/intc/loongarch_pic_common.h 
b/include/hw/intc/loongarch_pic_common.h
index b33bebb129..ef6edc15bf 100644
--- a/include/hw/intc/loongarch_pic_common.h
+++ b/include/hw/intc/loongarch_pic_common.h
@@ -26,11 +26,6 @@
 #define PCH_PIC_INT_STATUS  0x3a0
 #define PCH_PIC_INT_POL 0x3e0
 
-#define STATUS_LO_START 0
-#define STATUS_HI_START 0x4
-#define POL_LO_START0x40
-#define POL_HI_START0x44
-
 #define TYPE_LOONGARCH_PIC_COMMON "loongarch_pic_common"
 OBJECT_DECLARE_TYPE(LoongArchPICCommonState,
 LoongArchPICCommonClass, LOONGARCH_PIC_COMMON)
-- 
2.39.3

Re: [PATCH v6] hw/misc/vmfwupdate: Introduce hypervisor fw-cfg interface support

[Gerd Hoffman]

On Fri, Mar 14, 2025 at 03:50:19PM +0100, Alexander Graf wrote:
> 
> On 14.03.25 15:08, Gerd Hoffman wrote:
> >Hi,
> > 
> > > > Ok, assuming we allow the guest submit a IGVM image (which makes sense
> > > > indeed, otherwise we'll probably end up re-inventing IGVM).  How will
> > > > the kernel hashes be handled then?  I assume they will not be part of
> > > > the igvm image, but they must be part of the launch measurement ...
> > > The kernel hashes must be embedded in the IGVM image by the time you 
> > > invoke
> > > vmfwupdate. That means when you generate the FUKI, you take 4 inputs:
> > > Generic firmware image, kernel, initramfs, cmdline. Out of those, you
> > > generate and embed an IGVM image that consists of the firmware image as 
> > > well
> > > as the kernel hash page.
> > If your input firmware image already is an IGVM (say coconut), what is
> > supposed to happen?
> 
> I'll leave the details to Jörg on how he envisions it, but IIUC the flow for
> a "readily assembled IGVM" is different. In case of a COCONUT-SVSM IGVM, you
> expect chaining of trust. So the SVSM implements a TPM which then the OS
> would use with measured boot etc etc.

Well, I don't consider igvm being useful for svsm only.  Shipping
standard edk2 as igvm looks useful to me.  Main benefit: pre-calculate
launch measurements without having to know qemu internals.

> It's a fundamentally different concept from FUKI.

Hmm?  IGVM is just a different way to ship the firmware (and optionally
more).

> But it could share the same vmfwupdate mechanism to load.

Yep.  But we have to sort the details.

 (1) How we are going to load kernel + initrd in case the firmware is
 igvm?  Just update the igvm to also include linux kernel and
 initrd (see parallel reply from Jörg)?  If so, how does the
 launched firmware find the kernel + initrd?
 While digging around in the igvm spec I've seen there is the
 concept of 'parameters'.  Can this be used to pass on the memory
 location of kernel + initrd + cmdline?  Maybe the kernel hashes too?

 (2) Will the igvm be generated on the fly from FUKI data?  Or should
 the distros ship igvm images with firmware + kernel + initrd?

 (3) How we are going to handle uki add-ons?

 (4) Do we want keep the region list?  Or declare that igvm should be
 used in case a single region is not enough?  Or even go all-in and
 use IGVM exclusively?

take care,
  Gerd

Re: [PATCH v3 4/7] memory-attribute-manager: Introduce MemoryAttributeManager to manage RAMBLock with guest_memfd

[David Hildenbrand]

On 17.03.25 03:54, Chenyi Qiang wrote:



On 3/14/2025 8:11 PM, Gupta, Pankaj wrote:

On 3/10/2025 9:18 AM, Chenyi Qiang wrote:

As the commit 852f0048f3 ("RAMBlock: make guest_memfd require
uncoordinated discard") highlighted, some subsystems like VFIO may
disable ram block discard. However, guest_memfd relies on the discard
operation to perform page conversion between private and shared memory.
This can lead to stale IOMMU mapping issue when assigning a hardware
device to a confidential VM via shared memory. To address this, it is
crucial to ensure systems like VFIO refresh its IOMMU mappings.

RamDiscardManager is an existing concept (used by virtio-mem) to adjust
VFIO mappings in relation to VM page assignment. Effectively page
conversion is similar to hot-removing a page in one mode and adding it
back in the other. Therefore, similar actions are required for page
conversion events. Introduce the RamDiscardManager to guest_memfd to
facilitate this process.

Since guest_memfd is not an object, it cannot directly implement the
RamDiscardManager interface. One potential attempt is to implement it in
HostMemoryBackend. This is not appropriate because guest_memfd is per
RAMBlock. Some RAMBlocks have a memory backend but others do not. In
particular, the ones like virtual BIOS calling
memory_region_init_ram_guest_memfd() do not.

To manage the RAMBlocks with guest_memfd, define a new object named
MemoryAttributeManager to implement the RamDiscardManager interface. The


Isn't this should be the other way around. 'MemoryAttributeManager'
should be an interface and RamDiscardManager a type of it, an
implementation?


We want to use 'MemoryAttributeManager' to represent RAMBlock to
implement the RamDiscardManager interface callbacks because RAMBlock is
not an object. It includes some metadata of guest_memfd like
shared_bitmap at the same time.

I can't get it that make 'MemoryAttributeManager' an interface and
RamDiscardManager a type of it. Can you elaborate it a little bit? I
think at least we need someone to implement the RamDiscardManager interface.


shared <-> private is translated (abstracted) to "populated <-> 
discarded", which makes sense. The other way around would be wrong.


It's going to be interesting once we have more logical states, for 
example supporting virtio-mem for confidential VMs.


Then we'd have "shared+populated, private+populated, shared+discard, 
private+discarded". Not sure if this could simply be achieved by 
allowing multiple RamDiscardManager that are effectively chained, or if 
we'd want a different interface.


--
Cheers,

David / dhildenb

Re: [PATCH 2/2] memory: suppress INVALID_MEM logs caused by debug access

[Nicholas Piggin]

On Mon Mar 17, 2025 at 7:03 PM AEST, Philippe Mathieu-Daudé wrote:
> On 14/3/25 08:41, Nicholas Piggin wrote:
>> Debugger-driven invalid memory accesses are not guest errors, so should
>> not cause these error logs.
>> 
>> Debuggers can access memory wildly, including access to addresses not
>> specified by the user (e.g., gdb it might try to walk the stack or load
>> target addresses to display disassembly). Failure is reported
>> synchronously by the GDB protcol so the user can be notified via the
>> debugger client.
>> 
>> Signed-off-by: Nicholas Piggin 
>> ---
>>   system/memory.c | 37 ++---
>>   1 file changed, 22 insertions(+), 15 deletions(-)
>> 
>> diff --git a/system/memory.c b/system/memory.c
>> index 4c829793a0a..960f66e8d7e 100644
>> --- a/system/memory.c
>> +++ b/system/memory.c
>> @@ -1412,18 +1412,23 @@ bool memory_region_access_valid(MemoryRegion *mr,
>>   {
>
> Alternatively:
>
>  int invalid_mem_mask = attrs.debug ? LOG_INVALID_MEM : 0;

Oh that's a thing? Would save a level of indent and might look
nicer. (I guess you have x : y expressions reversed)

Thanks,
Nick

>
>>   if (mr->ops->valid.accepts
>>   && !mr->ops->valid.accepts(mr->opaque, addr, size, is_write, 
>> attrs)) {
>> -qemu_log_mask(LOG_INVALID_MEM, "Invalid %s at addr 0x%" HWADDR_PRIX
>> -  ", size %u, region '%s', reason: rejected\n",
>> -  is_write ? "write" : "read",
>> -  addr, size, memory_region_name(mr));
>> +if (attrs.debug) {
>> +/* Don't log memory errors due to debugger accesses */
>> +qemu_log_mask(LOG_INVALID_MEM, "Invalid %s at addr 0x%" 
>> HWADDR_PRIX
>> +  ", size %u, region '%s', reason: rejected\n",
>> +  is_write ? "write" : "read",
>> +  addr, size, memory_region_name(mr));
>> +}
>>   return false;
>>   }

Re: [PATCH 2/6] amd_iommu: Fix Device ID decoding for INVALIDATE_IOTLB_PAGES command

[Vasant Hegde]

Alejandro,

On 3/11/2025 8:54 PM, Alejandro Jimenez wrote:
> The DeviceID bits are extracted using an incorrect offset in the call to
> amdvi_iotlb_remove_page(). This field is read (correctly) earlier, so use
> the value already retrieved for devid.
> 
> Cc: qemu-sta...@nongnu.org
> Fixes: d29a09ca6842 ("hw/i386: Introduce AMD IOMMU")
> Signed-off-by: Alejandro Jimenez 

Fix looks good.

Reviewed-by: Vasant Hegde 

FYI. I think we need to reconsider IOTLB invalidation stuff. Its suppose to
invalidate the device side TLB, not IOMMU one. Before that we need to fix the
way we generate hash it. Ideally we should switch to domain ID and fix the size
as well. We are looking into this.

-Vasant


> ---
>  hw/i386/amd_iommu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 5b21cf134a..068eeb0cae 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -508,7 +508,7 @@ static void amdvi_inval_inttable(AMDVIState *s, uint64_t 
> *cmd)
>  static void iommu_inval_iotlb(AMDVIState *s, uint64_t *cmd)
>  {
>  
> -uint16_t devid = extract64(cmd[0], 0, 16);
> +uint16_t devid = cpu_to_le16(extract64(cmd[0], 0, 16));
>  if (extract64(cmd[1], 1, 1) || extract64(cmd[1], 3, 1) ||
>  extract64(cmd[1], 6, 6)) {
>  amdvi_log_illegalcom_error(s, extract64(cmd[0], 60, 4),
> @@ -521,7 +521,7 @@ static void iommu_inval_iotlb(AMDVIState *s, uint64_t 
> *cmd)
>  &devid);
>  } else {
>  amdvi_iotlb_remove_page(s, cpu_to_le64(extract64(cmd[1], 12, 52)) << 
> 12,
> -cpu_to_le16(extract64(cmd[1], 0, 16)));
> +devid);
>  }
>  trace_amdvi_iotlb_inval();
>  }

[PATCH 17/17] rust/vmstate: Add unit test for vmstate_validate

[Zhao Liu]

Add a unit test for vmstate_validate, which corresponds to the C version
macro: VMSTATE_VALIDATE.

Signed-off-by: Zhao Liu 
---
 rust/qemu-api/tests/vmstate_tests.rs | 91 +++-
 1 file changed, 89 insertions(+), 2 deletions(-)

diff --git a/rust/qemu-api/tests/vmstate_tests.rs 
b/rust/qemu-api/tests/vmstate_tests.rs
index a8bc00a56494..bb615fd7bdee 100644
--- a/rust/qemu-api/tests/vmstate_tests.rs
+++ b/rust/qemu-api/tests/vmstate_tests.rs
@@ -2,7 +2,7 @@
 // Author(s): Zhao Liu 
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use std::{ffi::CStr, mem::size_of, ptr::NonNull, slice};
+use std::{ffi::CStr, mem::size_of, os::raw::c_void, ptr::NonNull, slice};
 
 use qemu_api::{
 bindings::{
@@ -13,7 +13,7 @@
 cell::{BqlCell, Opaque},
 impl_vmstate_forward,
 vmstate::{VMStateDescription, VMStateField, VMStateFlags},
-vmstate_fields, vmstate_of, vmstate_struct, vmstate_unused,
+vmstate_fields, vmstate_of, vmstate_struct, vmstate_unused, 
vmstate_validate,
 zeroable::Zeroable,
 };
 
@@ -378,3 +378,90 @@ fn test_vmstate_macro_fooc() {
 // The last VMStateField in VMSTATE_FOOC.
 assert_eq!(foo_fields[4].flags, VMStateFlags::VMS_END);
 }
+
+// === Test VMSTATE_FOOD ===
+// Test the use cases of the vmstate macro, corresponding to the following C
+// macro variants:
+//   * VMSTATE_FOOD:
+// - VMSTATE_VALIDATE
+
+// Add more member fields when vmstate_of/vmstate_struct support "test"
+// parameter.
+struct FooD;
+
+impl FooD {
+fn validate_food_0(&self, _version_id: u8) -> bool {
+true
+}
+
+fn validate_food_1(_state: &FooD, _version_id: u8) -> bool {
+false
+}
+}
+
+fn validate_food_2(_state: &FooD, _version_id: u8) -> bool {
+true
+}
+
+static VMSTATE_FOOD: VMStateDescription = VMStateDescription {
+name: c_str!("foo_d").as_ptr(),
+version_id: 3,
+minimum_version_id: 1,
+fields: vmstate_fields! {
+vmstate_validate!(FooD, c_str!("foo_d_0"), FooD::validate_food_0),
+vmstate_validate!(FooD, c_str!("foo_d_1"), FooD::validate_food_1),
+vmstate_validate!(FooD, c_str!("foo_d_2"), validate_food_2),
+},
+..Zeroable::ZERO
+};
+
+#[test]
+fn test_vmstate_macro_food() {
+let foo_fields: &[VMStateField] = unsafe { 
slice::from_raw_parts(VMSTATE_FOOD.fields, 4) };
+let mut foo_d = FooD;
+let foo_d_p = &mut foo_d as *mut _ as *mut c_void;
+
+// 1st VMStateField in VMSTATE_FOOD
+assert_eq!(
+unsafe { CStr::from_ptr(foo_fields[0].name) }.to_bytes_with_nul(),
+b"foo_d_0\0"
+);
+assert_eq!(foo_fields[0].offset, 0);
+assert_eq!(foo_fields[0].num_offset, 0);
+assert_eq!(foo_fields[0].info.is_null(), true);
+assert_eq!(foo_fields[0].version_id, 0);
+assert_eq!(foo_fields[0].size, 0);
+assert_eq!(foo_fields[0].num, 0);
+assert_eq!(
+foo_fields[0].flags.0,
+VMStateFlags::VMS_ARRAY.0 | VMStateFlags::VMS_MUST_EXIST.0
+);
+assert_eq!(foo_fields[0].vmsd.is_null(), true);
+assert_eq!(
+unsafe { foo_fields[0].field_exists.unwrap()(foo_d_p, 0) },
+true
+);
+
+// 2nd VMStateField in VMSTATE_FOOD
+assert_eq!(
+unsafe { CStr::from_ptr(foo_fields[1].name) }.to_bytes_with_nul(),
+b"foo_d_1\0"
+);
+assert_eq!(
+unsafe { foo_fields[1].field_exists.unwrap()(foo_d_p, 1) },
+false
+);
+
+// 3rd VMStateField in VMSTATE_FOOD
+assert_eq!(
+unsafe { CStr::from_ptr(foo_fields[2].name) }.to_bytes_with_nul(),
+b"foo_d_2\0"
+);
+assert_eq!(
+unsafe { foo_fields[2].field_exists.unwrap()(foo_d_p, 2) },
+true
+);
+
+// The last VMStateField in VMSTATE_FOOD.
+assert_eq!(foo_fields[3].flags, VMStateFlags::VMS_END);
+}
-- 
2.34.1

[PATCH 15/17] rust/vmstate: Add unit test for vmstate_{of|struct} macro

[Zhao Liu]

Add a unit test to cover some patterns accepted by vmstate_of and
vmstate_struct macros, which correspond to the following C version
macros:

 * VMSTATE_BOOL_V
 * VMSTATE_U64
 * VMSTATE_STRUCT_VARRAY_UINT8
 * (no C version) MULTIPLY variant of VMSTATE_STRUCT_VARRAY_UINT32
 * VMSTATE_ARRAY

Signed-off-by: Zhao Liu 
---
 rust/qemu-api/tests/vmstate_tests.rs | 144 ++-
 1 file changed, 142 insertions(+), 2 deletions(-)

diff --git a/rust/qemu-api/tests/vmstate_tests.rs 
b/rust/qemu-api/tests/vmstate_tests.rs
index 25b28b298066..27df66b5766e 100644
--- a/rust/qemu-api/tests/vmstate_tests.rs
+++ b/rust/qemu-api/tests/vmstate_tests.rs
@@ -5,10 +5,14 @@
 use std::{ffi::CStr, mem::size_of, slice};
 
 use qemu_api::{
-bindings::{vmstate_info_int8, vmstate_info_uint8, 
vmstate_info_unused_buffer},
+bindings::{
+vmstate_info_bool, vmstate_info_int64, vmstate_info_int8, 
vmstate_info_uint64,
+vmstate_info_uint8, vmstate_info_unused_buffer,
+},
 c_str,
+cell::BqlCell,
 vmstate::{VMStateDescription, VMStateField, VMStateFlags},
-vmstate_fields, vmstate_of, vmstate_unused,
+vmstate_fields, vmstate_of, vmstate_struct, vmstate_unused,
 zeroable::Zeroable,
 };
 
@@ -125,3 +129,139 @@ fn test_vmstate_macro_fooa() {
 // The last VMStateField in VMSTATE_FOOA.
 assert_eq!(foo_fields[4].flags, VMStateFlags::VMS_END);
 }
+
+// === Test VMSTATE_FOOB ===
+// Test the use cases of the vmstate macro, corresponding to the following C
+// macro variants:
+//   * VMSTATE_FOOB:
+// - VMSTATE_BOOL_V
+// - VMSTATE_U64
+// - VMSTATE_STRUCT_VARRAY_UINT8
+// - (no C version) MULTIPLY variant of VMSTATE_STRUCT_VARRAY_UINT32
+// - VMSTATE_ARRAY
+#[repr(C)]
+#[derive(qemu_api_macros::offsets)]
+struct FooB {
+arr_a: [FooA; FOO_ARRAY_MAX],
+num_a: u8,
+arr_a_mul: [FooA; FOO_ARRAY_MAX],
+num_a_mul: u32,
+wrap: BqlCell,
+val: bool,
+// FIXME: Use Timer array. Now we can't since it's hard to link savevm.c 
to test.
+arr_i64: [i64; FOO_ARRAY_MAX],
+}
+
+static VMSTATE_FOOB: VMStateDescription = VMStateDescription {
+name: c_str!("foo_b").as_ptr(),
+version_id: 2,
+minimum_version_id: 1,
+fields: vmstate_fields! {
+vmstate_of!(FooB, val, version = 2),
+vmstate_of!(FooB, wrap),
+vmstate_struct!(FooB, arr_a, [0 .. num_a], VMSTATE_FOOA, FooA, version 
= 1),
+vmstate_struct!(FooB, arr_a_mul, [0 .. num_a_mul * 32], VMSTATE_FOOA, 
FooA, version = 2),
+vmstate_of!(FooB, arr_i64),
+},
+..Zeroable::ZERO
+};
+
+#[test]
+fn test_vmstate_macro_foob() {
+let foo_fields: &[VMStateField] = unsafe { 
slice::from_raw_parts(VMSTATE_FOOB.fields, 6) };
+
+// 1st VMStateField ("val") in VMSTATE_FOOB (corresponding to 
VMSTATE_BOOL_V)
+assert_eq!(
+unsafe { CStr::from_ptr(foo_fields[0].name) }.to_bytes_with_nul(),
+b"val\0"
+);
+assert_eq!(foo_fields[0].offset, 136);
+assert_eq!(foo_fields[0].num_offset, 0);
+assert_eq!(foo_fields[0].info, unsafe {
+::core::ptr::addr_of!(vmstate_info_bool)
+});
+assert_eq!(foo_fields[0].version_id, 2);
+assert_eq!(foo_fields[0].size, 1);
+assert_eq!(foo_fields[0].num, 0);
+assert_eq!(foo_fields[0].flags, VMStateFlags::VMS_SINGLE);
+assert_eq!(foo_fields[0].vmsd.is_null(), true);
+assert_eq!(foo_fields[0].field_exists.is_none(), true);
+
+// 2nd VMStateField ("wrap") in VMSTATE_FOOB (corresponding to VMSTATE_U64)
+assert_eq!(
+unsafe { CStr::from_ptr(foo_fields[1].name) }.to_bytes_with_nul(),
+b"wrap\0"
+);
+assert_eq!(foo_fields[1].offset, 128);
+assert_eq!(foo_fields[1].num_offset, 0);
+assert_eq!(foo_fields[1].info, unsafe {
+::core::ptr::addr_of!(vmstate_info_uint64)
+});
+assert_eq!(foo_fields[1].version_id, 0);
+assert_eq!(foo_fields[1].size, 8);
+assert_eq!(foo_fields[1].num, 0);
+assert_eq!(foo_fields[1].flags, VMStateFlags::VMS_SINGLE);
+assert_eq!(foo_fields[1].vmsd.is_null(), true);
+assert_eq!(foo_fields[1].field_exists.is_none(), true);
+
+// 3rd VMStateField ("arr_a") in VMSTATE_FOOB (corresponding to
+// VMSTATE_STRUCT_VARRAY_UINT8)
+assert_eq!(
+unsafe { CStr::from_ptr(foo_fields[2].name) }.to_bytes_with_nul(),
+b"arr_a\0"
+);
+assert_eq!(foo_fields[2].offset, 0);
+assert_eq!(foo_fields[2].num_offset, 60);
+assert_eq!(foo_fields[2].info.is_null(), true); // 
VMSTATE_STRUCT_VARRAY_UINT8 doesn't set info field.
+assert_eq!(foo_fields[2].version_id, 1);
+assert_eq!(foo_fields[2].size, 20);
+assert_eq!(foo_fields[2].num, 0);
+assert_eq!(
+foo_fields[2].flags.0,
+VMStateFlags::VMS_STRUCT.0 | VMStateFlags::VMS_VARRAY_UINT8.0
+);
+assert_eq!(foo_fields[2].vmsd, &VMSTATE_FOOA);
+assert_eq!(foo_fields[2].field_exists.is_none(), true);
+
+// 4th VMS

[PATCH 14/17] rust/vmstate: Add unit test for vmstate_of macro

[Zhao Liu]

The vmstate has too many combinations of VMStateFlags and VMStateField.
Currently, the best way to test is to ensure that the Rust vmstate
definition is consistent with the (possibly corresponding) C version.

Add a unit test to cover some patterns accepted by vmstate_of macro,
which correspond to the following C version macros:
 * VMSTATE_U16
 * VMSTATE_UNUSED
 * VMSTATE_VARRAY_UINT16_UNSAFE
 * VMSTATE_VARRAY_MULTIPLY

Note: Because vmstate_info_* are defined in vmstate-types.c, it's
necessary to link libmigration to rust unit tests. In the future,
maybe it's possible to spilt libmigration from rust_qemu_api_objs.

Signed-off-by: Zhao Liu 
---
 rust/qemu-api/meson.build|   5 +-
 rust/qemu-api/src/vmstate.rs |   6 +-
 rust/qemu-api/tests/tests.rs |   2 +
 rust/qemu-api/tests/vmstate_tests.rs | 127 +++
 4 files changed, 134 insertions(+), 6 deletions(-)
 create mode 100644 rust/qemu-api/tests/vmstate_tests.rs

diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index a3f226ccc2a5..858685ddd4a4 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -58,7 +58,8 @@ rust_qemu_api_objs = static_library(
   libchardev.extract_all_objects(recursive: false),
   libcrypto.extract_all_objects(recursive: false),
   libauthz.extract_all_objects(recursive: false),
-  libio.extract_all_objects(recursive: false)])
+  libio.extract_all_objects(recursive: false),
+  libmigration.extract_all_objects(recursive: false)])
 rust_qemu_api_deps = declare_dependency(
 dependencies: [
   qom_ss.dependencies(),
@@ -71,7 +72,7 @@ rust_qemu_api_deps = declare_dependency(
 test('rust-qemu-api-integration',
 executable(
 'rust-qemu-api-integration',
-'tests/tests.rs',
+files('tests/tests.rs', 'tests/vmstate_tests.rs'),
 override_options: ['rust_std=2021', 'build.rust_std=2021'],
 rust_args: ['--test'],
 install: false,
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 4eefd54ca120..3fdd5948f6d7 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -27,10 +27,8 @@
 use core::{marker::PhantomData, mem, ptr::NonNull};
 use std::os::raw::{c_int, c_void};
 
-pub use crate::bindings::{VMStateDescription, VMStateField};
-use crate::{
-bindings::VMStateFlags, callbacks::FnCall, prelude::*, qom::Owned, 
zeroable::Zeroable,
-};
+pub use crate::bindings::{VMStateDescription, VMStateField, VMStateFlags};
+use crate::{callbacks::FnCall, prelude::*, qom::Owned, zeroable::Zeroable};
 
 /// This macro is used to call a function with a generic argument bound
 /// to the type of a field.  The function must take a
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index 269122e7ec19..99a7aab6fed9 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -17,6 +17,8 @@
 zeroable::Zeroable,
 };
 
+mod vmstate_tests;
+
 // Test that macros can compile.
 pub static VMSTATE: VMStateDescription = VMStateDescription {
 name: c_str!("name").as_ptr(),
diff --git a/rust/qemu-api/tests/vmstate_tests.rs 
b/rust/qemu-api/tests/vmstate_tests.rs
new file mode 100644
index ..25b28b298066
--- /dev/null
+++ b/rust/qemu-api/tests/vmstate_tests.rs
@@ -0,0 +1,127 @@
+// Copyright (C) 2025 Intel Corporation.
+// Author(s): Zhao Liu 
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+use std::{ffi::CStr, mem::size_of, slice};
+
+use qemu_api::{
+bindings::{vmstate_info_int8, vmstate_info_uint8, 
vmstate_info_unused_buffer},
+c_str,
+vmstate::{VMStateDescription, VMStateField, VMStateFlags},
+vmstate_fields, vmstate_of, vmstate_unused,
+zeroable::Zeroable,
+};
+
+const FOO_ARRAY_MAX: usize = 3;
+
+// === Test VMSTATE_FOOA ===
+// Test the use cases of the vmstate macro, corresponding to the following C
+// macro variants:
+//   * VMSTATE_FOOA:
+// - VMSTATE_U16
+// - VMSTATE_UNUSED
+// - VMSTATE_VARRAY_UINT16_UNSAFE
+// - VMSTATE_VARRAY_MULTIPLY
+#[repr(C)]
+#[derive(qemu_api_macros::offsets)]
+struct FooA {
+arr: [u8; FOO_ARRAY_MAX],
+num: u16,
+arr_mul: [i8; FOO_ARRAY_MAX],
+num_mul: u32,
+elem: i8,
+}
+
+static VMSTATE_FOOA: VMStateDescription = VMStateDescription {
+name: c_str!("foo_a").as_ptr(),
+version_id: 1,
+minimum_version_id: 1,
+fields: vmstate_fields! {
+vmstate_of!(FooA, elem),
+vmstate_unused!(size_of::()),
+vmstate_of!(FooA, arr, [0 .. num], version = 0),
+vmstate_of!(FooA, arr_mul, [0 .. num_mul * 16]),
+},
+..Zeroable::ZERO
+};
+
+#[test]
+fn test_vmstate_macro_fooa() {
+let foo_fields: &[VMStateField] = unsafe { 
slice::from_raw_parts(VMSTATE_FOOA.fields, 5) };
+
+// 1st VMStateField ("elem") in VMSTATE_FOOA (corresponding to VMSTATE_U16)
+assert_eq!(
+  

[PATCH 04/17] rust/vmstate: Use ident instead of expr to parse vmsd in vmstate_struct macro

[Zhao Liu]

When specify an array field in vmstate_struct macro, there will be an
error:

> local ambiguity when calling macro `vmstate_struct`: multiple parsing
> options: built-in NTs expr ('vmsd') or 1 other option.

This is because "expr" can't recognize the "vmsd" field correctly, so
that it gets confused with the previous array field.

To fix the above issue, use "ident" for "vmsd" field, and explicitly
refer to it in the macro.

Signed-off-by: Zhao Liu 
---
 rust/hw/char/pl011/src/device_class.rs | 2 +-
 rust/qemu-api/src/vmstate.rs   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/rust/hw/char/pl011/src/device_class.rs 
b/rust/hw/char/pl011/src/device_class.rs
index 0b2076ddaa0f..e43a5d6cd063 100644
--- a/rust/hw/char/pl011/src/device_class.rs
+++ b/rust/hw/char/pl011/src/device_class.rs
@@ -72,7 +72,7 @@ extern "C" fn pl011_post_load(opaque: *mut c_void, 
version_id: c_int) -> c_int {
 post_load: Some(pl011_post_load),
 fields: vmstate_fields! {
 vmstate_unused!(core::mem::size_of::()),
-vmstate_struct!(PL011State, regs, &VMSTATE_PL011_REGS, 
BqlRefCell),
+vmstate_struct!(PL011State, regs, VMSTATE_PL011_REGS, 
BqlRefCell),
 },
 subsections: vmstate_subsections! {
 VMSTATE_PL011_CLOCK
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 94efbd8bb735..3f95d4825149 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -435,7 +435,7 @@ macro_rules! vmstate_unused {
 #[doc(alias = "VMSTATE_STRUCT")]
 #[macro_export]
 macro_rules! vmstate_struct {
-($struct_name:ty, $field_name:ident $(, [0 .. $num:ident $(* 
$factor:expr)?])?, $vmsd:expr, $type:ty $(,)?) => {
+($struct_name:ty, $field_name:ident $(, [0 .. $num:ident $(* 
$factor:expr)?])?, $vmsd:ident, $type:ty $(,)?) => {
 $crate::bindings::VMStateField {
 name: ::core::concat!(::core::stringify!($field_name), "\0")
 .as_bytes()
@@ -447,7 +447,7 @@ macro_rules! vmstate_struct {
 },
 size: ::core::mem::size_of::<$type>(),
 flags: $crate::bindings::VMStateFlags::VMS_STRUCT,
-vmsd: $vmsd,
+vmsd: &$vmsd,
 ..$crate::zeroable::Zeroable::ZERO $(
 .with_varray_flag($crate::call_func_with_field!(
 $crate::vmstate::vmstate_varray_flag,
-- 
2.34.1

Re: [PATCH 1/2] io/block: Refactoring the bdrv_drained_begin() function and implement a timeout mechanism.

[Stefan Hajnoczi]

On Mon, Mar 17, 2025 at 08:18:28PM +0800, zoudongjie wrote:
> On Thu, 13 Mar, 2025 at 12:09:45 +0800, Stefan Hajnoczi wrote:
> > On Sat, Mar 08, 2025 at 06:16:17PM +0800, zoudongjie wrote:
> > > @@ -342,16 +350,25 @@ static void coroutine_fn 
> > > bdrv_co_yield_to_drain(BlockDriverState *bs,
> > >  /* If we are resumed from some other event (such as an aio 
> > > completion or a
> > >   * timer callback), it is a bug in the caller that should be fixed. 
> > > */
> > >  assert(data.done);
> > > +return data.ret;
> > >  }
> > >  
> > > -static void bdrv_do_drained_begin(BlockDriverState *bs, BdrvChild 
> > > *parent,
> > > -  bool poll)
> > > +static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs,
> > > +bool begin,
> > > +BdrvChild *parent,
> > > +bool poll)
> > > +{
> > > +bdrv_co_yield_to_drain_timeout(bs, begin, parent, poll, -1);
> > 
> > Is this safe on 32-bit platforms?
> 
> I'm sorry, can it be more specific here, I didn't get it.

I was thinking about -1 vs -1ull integer literals, but it's not a
problem for int64_t so everything is fine here.

Stefan


signature.asc
Description: PGP signature

[PATCH 16/17] rust/vmstate: Add unit test for pointer case

[Zhao Liu]

Add a unit test to cover some patterns accepted by vmstate_of macro,
which correspond to the following C version macros:
 * VMSTATE_POINTER
 * VMSTATE_ARRAY_OF_POINTER

Note: Currently, vmstate_struct can't handle the pointer to structure
case. Leave this case as a FIXME and use vmstate_unused as a place
holder.

Signed-off-by: Zhao Liu 
---
 rust/qemu-api/tests/vmstate_tests.rs | 121 ++-
 1 file changed, 117 insertions(+), 4 deletions(-)

diff --git a/rust/qemu-api/tests/vmstate_tests.rs 
b/rust/qemu-api/tests/vmstate_tests.rs
index 27df66b5766e..a8bc00a56494 100644
--- a/rust/qemu-api/tests/vmstate_tests.rs
+++ b/rust/qemu-api/tests/vmstate_tests.rs
@@ -2,15 +2,16 @@
 // Author(s): Zhao Liu 
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use std::{ffi::CStr, mem::size_of, slice};
+use std::{ffi::CStr, mem::size_of, ptr::NonNull, slice};
 
 use qemu_api::{
 bindings::{
-vmstate_info_bool, vmstate_info_int64, vmstate_info_int8, 
vmstate_info_uint64,
-vmstate_info_uint8, vmstate_info_unused_buffer,
+vmstate_info_bool, vmstate_info_int32, vmstate_info_int64, 
vmstate_info_int8,
+vmstate_info_uint64, vmstate_info_uint8, vmstate_info_unused_buffer,
 },
 c_str,
-cell::BqlCell,
+cell::{BqlCell, Opaque},
+impl_vmstate_forward,
 vmstate::{VMStateDescription, VMStateField, VMStateFlags},
 vmstate_fields, vmstate_of, vmstate_struct, vmstate_unused,
 zeroable::Zeroable,
@@ -265,3 +266,115 @@ fn test_vmstate_macro_foob() {
 // The last VMStateField in VMSTATE_FOOB.
 assert_eq!(foo_fields[5].flags, VMStateFlags::VMS_END);
 }
+
+// === Test VMSTATE_FOOC ===
+// Test the use cases of the vmstate macro, corresponding to the following C
+// macro variants:
+//   * VMSTATE_FOOC:
+// - VMSTATE_POINTER
+// - VMSTATE_ARRAY_OF_POINTER
+struct FooCWrapper([Opaque<*mut u8>; FOO_ARRAY_MAX]); // Though Opaque<> array 
is almost impossible.
+
+impl_vmstate_forward!(FooCWrapper);
+
+#[repr(C)]
+#[derive(qemu_api_macros::offsets)]
+struct FooC {
+ptr: *const i32,
+ptr_a: NonNull,
+arr_ptr: [Box; FOO_ARRAY_MAX],
+arr_ptr_wrap: FooCWrapper,
+}
+
+static VMSTATE_FOOC: VMStateDescription = VMStateDescription {
+name: c_str!("foo_c").as_ptr(),
+version_id: 3,
+minimum_version_id: 1,
+fields: vmstate_fields! {
+vmstate_of!(FooC, ptr, version = 2),
+// FIXME: Currently vmstate_struct doesn't support the pointer to 
structure.
+// VMSTATE_STRUCT_POINTER: vmstate_struct!(FooC, ptr_a, VMSTATE_FOOA, 
NonNull)
+vmstate_unused!(size_of::>()),
+vmstate_of!(FooC, arr_ptr),
+vmstate_of!(FooC, arr_ptr_wrap),
+},
+..Zeroable::ZERO
+};
+
+#[test]
+fn test_vmstate_macro_fooc() {
+let foo_fields: &[VMStateField] = unsafe { 
slice::from_raw_parts(VMSTATE_FOOC.fields, 6) };
+
+// 1st VMStateField ("ptr") in VMSTATE_FOOC (corresponding to 
VMSTATE_POINTER)
+assert_eq!(
+unsafe { CStr::from_ptr(foo_fields[0].name) }.to_bytes_with_nul(),
+b"ptr\0"
+);
+assert_eq!(foo_fields[0].offset, 0);
+assert_eq!(foo_fields[0].num_offset, 0);
+assert_eq!(foo_fields[0].info, unsafe {
+::core::ptr::addr_of!(vmstate_info_int32)
+});
+assert_eq!(foo_fields[0].version_id, 2);
+assert_eq!(foo_fields[0].size, 4);
+assert_eq!(foo_fields[0].num, 0);
+assert_eq!(
+foo_fields[0].flags.0,
+VMStateFlags::VMS_SINGLE.0 | VMStateFlags::VMS_POINTER.0
+);
+assert_eq!(foo_fields[0].vmsd.is_null(), true);
+assert_eq!(foo_fields[0].field_exists.is_none(), true);
+
+// 2nd VMStateField ("unused") in VMSTATE_FOOC
+let mut offset = size_of::<*const i32>();
+let size_ptr_a = size_of::>();
+
+// 3rd VMStateField ("arr_ptr") in VMSTATE_FOOC (corresponding to
+// VMSTATE_ARRAY_OF_POINTER)
+offset = offset + size_ptr_a;
+let size_arr_ptr = size_of::>();
+assert_eq!(
+unsafe { CStr::from_ptr(foo_fields[2].name) }.to_bytes_with_nul(),
+b"arr_ptr\0"
+);
+assert_eq!(foo_fields[2].offset, offset);
+assert_eq!(foo_fields[2].num_offset, 0);
+assert_eq!(foo_fields[2].info, unsafe {
+::core::ptr::addr_of!(vmstate_info_uint8)
+});
+assert_eq!(foo_fields[2].version_id, 0);
+assert_eq!(foo_fields[2].size, size_arr_ptr);
+assert_eq!(foo_fields[2].num, FOO_ARRAY_MAX as i32);
+assert_eq!(
+foo_fields[2].flags.0,
+VMStateFlags::VMS_ARRAY.0 | VMStateFlags::VMS_ARRAY_OF_POINTER.0
+);
+assert_eq!(foo_fields[2].vmsd.is_null(), true);
+assert_eq!(foo_fields[2].field_exists.is_none(), true);
+
+// 4th VMStateField ("arr_ptr_wrap") in VMSTATE_FOOC (corresponding to
+// VMSTATE_ARRAY_OF_POINTER)
+offset = offset + size_of::>() * FOO_ARRAY_MAX;
+let size_arr_ptr_wrap = size_of::>();
+assert_eq!(
+unsafe { CStr::from_ptr(foo_fields[3].name) }

Re: [PATCH for-10.1 1/9] core/cpu.h: gdb_arch_name string should not be freed

[Alex Bennée]

Peter Maydell  writes:

> The documentation for the CPUClass::gdb_arch_name method claims that
> the returned string should be freed with g_free().  This is not
> correct: in commit a650683871ba728 we changed this method to
> instead return a simple constant string, but forgot to update
> the documentation.
>
> Make the documentation match the new semantics.
>
> Fixes: a650683871ba728 ("hw/core/cpu: Return static value with 
> gdb_arch_name()")
> Signed-off-by: Peter Maydell 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH for-10.1 2/9] gdbstub: Allow gdb_core_xml_file to be set at runtime

[Alex Bennée]

Peter Maydell  writes:

> Currently the CPUClass:gdb_core_xml_file setting is a simple 'const
> char *' which the CPU class must set to a fixed string.  Allow the
> CPU class to instead set a new method gdb_get_core_xml_file() which
> returns this string.
>
> This will allow Arm CPUs to use different XML files for AArch32 vs
> AArch64 without having to have an extra AArch64-specific class type
> purely to give somewhere to set cc->gdb_core_xml_file differently.
>
> Signed-off-by: Peter Maydell 

Acked-by: Alex Bennée 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH for-10.1 9/9] target/arm: Remove TYPE_AARCH64_CPU

[Peter Maydell]

The TYPE_AARCH64_CPU class is an abstract type that is the parent of
all the AArch64 CPUs.  It now has no special behaviour of its own, so
we can eliminate it and make the AArch64 CPUs directly inherit from
TYPE_ARM_CPU.

Signed-off-by: Peter Maydell 
---
 target/arm/cpu-qom.h   |  5 -
 target/arm/cpu.h   |  4 
 target/arm/internals.h |  1 -
 target/arm/cpu64.c | 49 +-
 target/arm/tcg/cpu64.c |  2 +-
 5 files changed, 2 insertions(+), 59 deletions(-)

diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
index b497667d61e..2fcb0e12525 100644
--- a/target/arm/cpu-qom.h
+++ b/target/arm/cpu-qom.h
@@ -28,11 +28,6 @@ OBJECT_DECLARE_CPU_TYPE(ARMCPU, ARMCPUClass, ARM_CPU)
 
 #define TYPE_ARM_MAX_CPU "max-" TYPE_ARM_CPU
 
-#define TYPE_AARCH64_CPU "aarch64-cpu"
-typedef struct AArch64CPUClass AArch64CPUClass;
-DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU,
-   TYPE_AARCH64_CPU)
-
 #define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU
 #define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX)
 
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 8f52380c88c..c9c53a6294b 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1161,10 +1161,6 @@ struct ARMCPUClass {
 ResettablePhases parent_phases;
 };
 
-struct AArch64CPUClass {
-ARMCPUClass parent_class;
-};
-
 /* Callback functions for the generic timer's timers. */
 void arm_gt_ptimer_cb(void *opaque);
 void arm_gt_vtimer_cb(void *opaque);
diff --git a/target/arm/internals.h b/target/arm/internals.h
index a18d87fa28b..0c24208f183 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -350,7 +350,6 @@ static inline int r14_bank_number(int mode)
 }
 
 void arm_cpu_register(const ARMCPUInfo *info);
-void aarch64_cpu_register(const ARMCPUInfo *info);
 
 void register_cp_regs_for_features(ARMCPU *cpu);
 void init_cpreg_list(ARMCPU *cpu);
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 49cf06a7bdc..200da1c489b 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -781,59 +781,12 @@ static const ARMCPUInfo aarch64_cpus[] = {
 #endif
 };
 
-static void aarch64_cpu_finalizefn(Object *obj)
-{
-}
-
-static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
-{
-}
-
-static void aarch64_cpu_instance_init(Object *obj)
-{
-ARMCPUClass *acc = ARM_CPU_GET_CLASS(obj);
-
-acc->info->initfn(obj);
-arm_cpu_post_init(obj);
-}
-
-static void cpu_register_class_init(ObjectClass *oc, void *data)
-{
-ARMCPUClass *acc = ARM_CPU_CLASS(oc);
-
-acc->info = data;
-}
-
-void aarch64_cpu_register(const ARMCPUInfo *info)
-{
-TypeInfo type_info = {
-.parent = TYPE_AARCH64_CPU,
-.instance_init = aarch64_cpu_instance_init,
-.class_init = info->class_init ?: cpu_register_class_init,
-.class_data = (void *)info,
-};
-
-type_info.name = g_strdup_printf("%s-" TYPE_ARM_CPU, info->name);
-type_register_static(&type_info);
-g_free((void *)type_info.name);
-}
-
-static const TypeInfo aarch64_cpu_type_info = {
-.name = TYPE_AARCH64_CPU,
-.parent = TYPE_ARM_CPU,
-.instance_finalize = aarch64_cpu_finalizefn,
-.abstract = true,
-.class_init = aarch64_cpu_class_init,
-};
-
 static void aarch64_cpu_register_types(void)
 {
 size_t i;
 
-type_register_static(&aarch64_cpu_type_info);
-
 for (i = 0; i < ARRAY_SIZE(aarch64_cpus); ++i) {
-aarch64_cpu_register(&aarch64_cpus[i]);
+arm_cpu_register(&aarch64_cpus[i]);
 }
 }
 
diff --git a/target/arm/tcg/cpu64.c b/target/arm/tcg/cpu64.c
index 29ab0ac79da..5d8ed2794d3 100644
--- a/target/arm/tcg/cpu64.c
+++ b/target/arm/tcg/cpu64.c
@@ -1316,7 +1316,7 @@ static void aarch64_cpu_register_types(void)
 size_t i;
 
 for (i = 0; i < ARRAY_SIZE(aarch64_cpus); ++i) {
-aarch64_cpu_register(&aarch64_cpus[i]);
+arm_cpu_register(&aarch64_cpus[i]);
 }
 }
 
-- 
2.43.0

[PATCH for-10.1 8/9] target/arm/kvm: don't check TYPE_AARCH64_CPU

[Peter Maydell]

We want to merge TYPE_AARCH64_CPU with TYPE_ARM_CPU, so enforcing in
kvm_arch_init_vcpu() that the CPU class is a subclass of
TYPE_AARCH64_CPU will no longer be possible.

It's safe to just remove this test, because any purely-AArch32 CPU
will fail the "kvm_target isn't set" check, because we no longer
support the old AArch32-host KVM setup and so CPUs like the Cortex-A7
no longer set cpu->kvm_target. Only the 'host', 'max', and the
odd special cases 'cortex-a53' and 'cortex-a57' set kvm_target.

Signed-off-by: Peter Maydell 
---
 target/arm/kvm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index da30bdbb234..7418eb57537 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -1882,8 +1882,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
 CPUARMState *env = &cpu->env;
 uint64_t psciver;
 
-if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE ||
-!object_dynamic_cast(OBJECT(cpu), TYPE_AARCH64_CPU)) {
+if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE) {
 error_report("KVM is not supported for this guest CPU type");
 return -EINVAL;
 }
-- 
2.43.0

[PATCH for-10.1 0/9] target/arm: Remove TYPE_AARCH64_CPU class

[Peter Maydell]

Currently we have a class hierarchy for Arm CPUs where
all the 32-bit CPUs (including M-profile) inherit directly
from TYPE_ARM_CPU, but the 64-bit CPUs inherit from
TYPE_AARCH64_CPU, which is a subclass of TYPE_ARM_CPU.
This subclass does essentially two things:
 * it sets up fields and methods for the gdbstub so that
   the gdbstub presents an AArch64 CPU to gdb rather than
   an AArch32 one
 * it defines the 'aarch64' CPU property which you can use
   with KVM to disable AArch64 and create a 32-bit VM
   (with "-cpu host,aarch64=off")

This is a bit weird, because the 32-bit CPU you create with
KVM and aarch64=off is still a subclass of TYPE_AARCH64_CPU.
It also still presents gdb with an AArch64 CPU, so you
effectively can't use the gdbstub with this kind of VM.

This patchseries removes TYPE_AARCH64_CPU so that all CPUs,
both AArch32 and AArch64, directly inherit from TYPE_ARM_CPU.
This lets us fix the bug with gdbstub and "aarch64=off".

Most of the gdbstub related CPUClass fields are already methods,
so we can make the existing TYPE_ARM_CPU ones redirect to
the AArch64 functions if ARM_FEATURE_AARCH64 is set. The
odd-one-out is gdb_core_xml_file, so we have to add a new
optional method gdb_get_core_xml_file to allow us to select
the right XML file at runtime. (We could, like gdb_arch_name,
simply replace the existing static string field with the
method for all targets, but at least for this patchset I
didn't want to get into that complexity.)

We make the 'aarch64' property be an object property defined
if the ARM_FEATURE_AARCH64 is set rather than a class property;
this brings it into line with our other CPU properties.

Once we've done that and removed a check on TYPE_AARCH64_CPU
in the KVM code that hasn't been needed since we removed
32-bit Arm host KVM support, we can remove TYPE_AARCH64_CPU
entirely.

(The rationale here is that I think we should be able to
enable 'aarch64=off' for TCG CPUs too, so this will become
less of an odd KVM-specific corner case, and this seemed
worth cleaning up.)

thanks
-- PMM

Peter Maydell (9):
  core/cpu.h: gdb_arch_name string should not be freed
  gdbstub: Allow gdb_core_xml_file to be set at runtime
  target/arm: Handle AArch64 in TYPE_ARM_CPU gdb_arch_name
  target/arm: Handle gdb_core_xml_file in TYPE_ARM_CPU
  target/arm: Handle AArch64 gdb read/write regs in TYPE_ARM_CPU
  target/arm: Present AArch64 gdbstub based on ARM_FEATURE_AARCH64
  target/arm: Move aarch64 CPU property code to TYPE_ARM_CPU
  target/arm/kvm: don't check TYPE_AARCH64_CPU
  target/arm: Remove TYPE_AARCH64_CPU

 include/hw/core/cpu.h|  8 +++-
 target/arm/cpu-qom.h |  5 ---
 target/arm/cpu.h |  4 --
 target/arm/internals.h   |  7 ++-
 gdbstub/gdbstub.c| 23 --
 target/arm/cpu.c | 55 ++-
 target/arm/cpu64.c   | 94 +---
 target/arm/gdbstub.c | 12 +
 target/arm/kvm.c |  3 +-
 target/arm/tcg/cpu-v7m.c |  1 -
 target/arm/tcg/cpu64.c   |  2 +-
 11 files changed, 101 insertions(+), 113 deletions(-)

-- 
2.43.0

[PATCH for-10.1 5/9] target/arm: Handle AArch64 gdb read/write regs in TYPE_ARM_CPU

[Peter Maydell]

Instead of having the TYPE_AARCH64_CPU subclass set
CPUClass::gdb_read_register and ::gdb_write_register to different
methods from those of the TYPE_ARM_CPU parent class, have the
TYPE_ARM_CPU methods handle either AArch32 or AArch64 at runtime.

Signed-off-by: Peter Maydell 
---
 target/arm/cpu64.c   |  5 -
 target/arm/gdbstub.c | 12 
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 3094df366ec..2f87df082cd 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -815,11 +815,6 @@ static void aarch64_cpu_finalizefn(Object *obj)
 
 static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
 {
-CPUClass *cc = CPU_CLASS(oc);
-
-cc->gdb_read_register = aarch64_cpu_gdb_read_register;
-cc->gdb_write_register = aarch64_cpu_gdb_write_register;
-
 object_class_property_add_bool(oc, "aarch64", aarch64_cpu_get_aarch64,
aarch64_cpu_set_aarch64);
 object_class_property_set_description(oc, "aarch64",
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 30068c22627..ce4497ad7c3 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -44,6 +44,12 @@ int arm_cpu_gdb_read_register(CPUState *cs, GByteArray 
*mem_buf, int n)
 ARMCPU *cpu = ARM_CPU(cs);
 CPUARMState *env = &cpu->env;
 
+#ifdef TARGET_AARCH64
+if (arm_gdbstub_is_aarch64(cpu)) {
+return aarch64_cpu_gdb_read_register(cs, mem_buf, n);
+}
+#endif
+
 if (n < 16) {
 /* Core integer register.  */
 return gdb_get_reg32(mem_buf, env->regs[n]);
@@ -66,6 +72,12 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 CPUARMState *env = &cpu->env;
 uint32_t tmp;
 
+#ifdef TARGET_AARCH64
+if (arm_gdbstub_is_aarch64(cpu)) {
+return aarch64_cpu_gdb_write_register(cs, mem_buf, n);
+}
+#endif
+
 tmp = ldl_p(mem_buf);
 
 /*
-- 
2.43.0

[PATCH 13/17] rust/vmstate: Support vmstate_validate

[Zhao Liu]

In C version, VMSTATE_VALIDATE accepts the function pointer, which is
used to check if some conditions of structure could meet, although the
C version macro doesn't accept any structure as the opaque type.

But it's hard to integrate VMSTATE_VALIDAE into vmstate_struct, a new
macro has to be introduced to specifically handle the case corresponding
to VMSTATE_VALIDATE.

One of the difficulties is inferring the type of a callback by its name
`test_fn`. We can't directly use `test_fn` as a parameter of
test_cb_builder__() to get its type "F", because in this way, Rust
compiler will be too conservative on drop check and complain "the
destructor for this type cannot be evaluated in constant functions".

Fortunately, PhantomData could help in this case, because it is
considered to never have a destructor, no matter its field type [*].

The `phantom__()` in the `call_func_with_field` macro provides a good
example of using PhantomData to infer type. So copy this idea and apply
it to the `vmstate_validate` macro.

[*]: https://doc.rust-lang.org/std/ops/trait.Drop.html#drop-check

Signed-off-by: Zhao Liu 
---
 rust/qemu-api/src/vmstate.rs | 57 +++-
 1 file changed, 56 insertions(+), 1 deletion(-)

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index bb41bfd291c0..4eefd54ca120 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -25,9 +25,12 @@
 //!   functionality that is missing from `vmstate_of!`.
 
 use core::{marker::PhantomData, mem, ptr::NonNull};
+use std::os::raw::{c_int, c_void};
 
 pub use crate::bindings::{VMStateDescription, VMStateField};
-use crate::{bindings::VMStateFlags, prelude::*, qom::Owned, 
zeroable::Zeroable};
+use crate::{
+bindings::VMStateFlags, callbacks::FnCall, prelude::*, qom::Owned, 
zeroable::Zeroable,
+};
 
 /// This macro is used to call a function with a generic argument bound
 /// to the type of a field.  The function must take a
@@ -292,6 +295,14 @@ pub const fn with_varray_multiply(mut self, num: u32) -> 
VMStateField {
 self.num = num as i32;
 self
 }
+
+#[must_use]
+pub const fn with_exist_check(mut self) -> Self {
+assert!(self.flags.0 == 0);
+self.flags = VMStateFlags(VMStateFlags::VMS_MUST_EXIST.0 | 
VMStateFlags::VMS_ARRAY.0);
+self.num = 0; // 0 elements: no data, only run test_fn callback
+self
+}
 }
 
 /// This macro can be used (by just passing it a type) to forward the `VMState`
@@ -510,6 +521,50 @@ macro_rules! vmstate_fields {
 }}
 }
 
+pub extern "C" fn rust_vms_test_field_exists FnCall<(&'a T, u8), 
bool>>(
+opaque: *mut c_void,
+version_id: c_int,
+) -> bool {
+let owner: &T = unsafe { &*(opaque.cast::()) };
+let version: u8 = version_id.try_into().unwrap();
+// SAFETY: the opaque was passed as a reference to `T`.
+F::call((owner, version))
+}
+
+pub type VMSFieldExistCb = unsafe extern "C" fn(
+opaque: *mut std::os::raw::c_void,
+version_id: std::os::raw::c_int,
+) -> bool;
+
+#[doc(alias = "VMSTATE_VALIDATE")]
+#[macro_export]
+macro_rules! vmstate_validate {
+($struct_name:ty, $test_name:expr, $test_fn:expr $(,)?) => {
+$crate::bindings::VMStateField {
+name: ::std::ffi::CStr::as_ptr($test_name),
+// TODO: Use safe callback.
+field_exists: {
+const fn test_cb_builder__<
+T,
+F: for<'a> $crate::callbacks::FnCall<(&'a T, u8), bool>,
+>(
+_phantom: ::core::marker::PhantomData,
+) -> $crate::vmstate::VMSFieldExistCb {
+let _: () = F::ASSERT_IS_SOME;
+$crate::vmstate::rust_vms_test_field_exists::
+}
+
+const fn phantom__(_: &T) -> ::core::marker::PhantomData 
{
+::core::marker::PhantomData
+}
+Some(test_cb_builder__::<$struct_name, 
_>(phantom__(&$test_fn)))
+},
+..$crate::zeroable::Zeroable::ZERO
+}
+.with_exist_check()
+};
+}
+
 /// A transparent wrapper type for the `subsections` field of
 /// [`VMStateDescription`].
 ///
-- 
2.34.1

[PATCH 11/17] rust/vmstate: Re-implement VMState trait for timer binding

[Zhao Liu]

At present, Rust side has a timer binding "timer::Timer", so the vmstate
for timer should base on that binding instead of the raw
"binding::QEMUTimer".

It's possible to apply impl_vmstate_transparent for cell::Opaque and
then impl_vmstate_forward for timer::Timer. But binding::QEMUTimer
shouldn't be used directly, so that vmstate for such raw timer type is
useless.

Thus, apply impl_vmstate_scalar for timer::Timer. And since Opaque<> is
useful, apply impl_vmstate_transparent for cell::Opaque as well.

Signed-off-by: Zhao Liu 
---
 rust/qemu-api/src/vmstate.rs | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 806d531b0c37..3d4c50ca86f9 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -27,12 +27,7 @@
 use core::{marker::PhantomData, mem, ptr::NonNull};
 
 pub use crate::bindings::{VMStateDescription, VMStateField};
-use crate::{
-bindings::{self, VMStateFlags},
-prelude::*,
-qom::Owned,
-zeroable::Zeroable,
-};
+use crate::{bindings::VMStateFlags, prelude::*, qom::Owned, 
zeroable::Zeroable};
 
 /// This macro is used to call a function with a generic argument bound
 /// to the type of a field.  The function must take a
@@ -344,6 +339,7 @@ unsafe impl<$base> VMState for $type where $base: VMState 
$($where)* {
 impl_vmstate_transparent!(std::pin::Pin where T: VMState);
 impl_vmstate_transparent!(crate::cell::BqlCell where T: VMState);
 impl_vmstate_transparent!(crate::cell::BqlRefCell where T: VMState);
+impl_vmstate_transparent!(crate::cell::Opaque where T: VMState);
 
 #[macro_export]
 macro_rules! impl_vmstate_bitsized {
@@ -390,7 +386,7 @@ unsafe impl VMState for $type {
 impl_vmstate_scalar!(vmstate_info_uint16, u16, VMS_VARRAY_UINT16);
 impl_vmstate_scalar!(vmstate_info_uint32, u32, VMS_VARRAY_UINT32);
 impl_vmstate_scalar!(vmstate_info_uint64, u64);
-impl_vmstate_scalar!(vmstate_info_timer, bindings::QEMUTimer);
+impl_vmstate_scalar!(vmstate_info_timer, crate::timer::Timer);
 
 // Pointer types using the underlying type's VMState plus VMS_POINTER
 // Note that references are not supported, though references to cells
-- 
2.34.1

[PATCH 09/17] rust/vmstate: Fix unnecessary VMState bound of with_varray_flag()

[Zhao Liu]

The VMState type bound is not used in with_varray_flag().

And for vmstate_struct, Rust cannot infer the type of `num` from the
call_func_with_field(), so this causes the compiling error because it
complains "cannot satisfy `_: VMState`" in with_varray_flag().

Note Rust can infer the type in vmstate_of macro so that
with_varray_flag() can work at there. It is possible that the different
initialization ways in the two macros cause differences in Rust's
type inference.

But in fact, the VMState type bound is not used in with_varray_flag()
and vmstate_varray_flag() has already checked the VMState type, it's
safe to drop VMState bound of with_varray_flag(), which can fix the
above compiling error.

Signed-off-by: Zhao Liu 
---
 rust/qemu-api/src/vmstate.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index a9d97d9a856e..2749e8af2b5a 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -275,7 +275,7 @@ pub const fn with_pointer_flag(mut self) -> Self {
 }
 
 #[must_use]
-pub const fn with_varray_flag(mut self, flag: VMStateFlags) -> 
VMStateField {
+pub const fn with_varray_flag(mut self, flag: VMStateFlags) -> 
VMStateField {
 assert!((self.flags.0 & VMStateFlags::VMS_ARRAY.0) != 0);
 self.flags = VMStateFlags(self.flags.0 & !VMStateFlags::VMS_ARRAY.0);
 self.flags = VMStateFlags(self.flags.0 | flag.0);
-- 
2.34.1

[PATCH 1/2] accel/tcg: Remove unnecesary inclusion of memory-internal.h in cputlb.c

[Philippe Mathieu-Daudé]

At some point cputlb.c stopped depending on the
"exec/memory-internal.h" header. Clean that now.

Signed-off-by: Philippe Mathieu-Daudé 
---
 accel/tcg/cputlb.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index fb22048876e..5007bdbcd75 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -26,7 +26,6 @@
 #include "exec/cpu_ldst.h"
 #include "exec/cputlb.h"
 #include "exec/tb-flush.h"
-#include "exec/memory-internal.h"
 #include "exec/ram_addr.h"
 #include "exec/mmu-access-type.h"
 #include "exec/tlb-common.h"
-- 
2.47.1

Re: [PATCH v5 05/17] exec/memory.h: make devend_memop "target defines" agnostic

[Pierrick Bouvier]

On 3/17/25 08:48, Philippe Mathieu-Daudé wrote:

On 14/3/25 18:31, Pierrick Bouvier wrote:

Will allow to make system/memory.c common later.

Reviewed-by: Richard Henderson 
Signed-off-by: Pierrick Bouvier 
---
   include/exec/memory.h | 16 
   1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index da21e9150b5..069021ac3ff 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -3138,25 +3138,17 @@ address_space_write_cached(MemoryRegionCache *cache, 
hwaddr addr,
   MemTxResult address_space_set(AddressSpace *as, hwaddr addr,
 uint8_t c, hwaddr len, MemTxAttrs attrs);
   
-#ifdef COMPILING_PER_TARGET

   /* enum device_endian to MemOp.  */
   static inline MemOp devend_memop(enum device_endian end)
   {
   QEMU_BUILD_BUG_ON(DEVICE_HOST_ENDIAN != DEVICE_LITTLE_ENDIAN &&
 DEVICE_HOST_ENDIAN != DEVICE_BIG_ENDIAN);
   
-#if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN

-/* Swap if non-host endianness or native (target) endianness */
-return (end == DEVICE_HOST_ENDIAN) ? 0 : MO_BSWAP;
-#else
-const int non_host_endianness =
-DEVICE_LITTLE_ENDIAN ^ DEVICE_BIG_ENDIAN ^ DEVICE_HOST_ENDIAN;
-
-/* In this case, native (target) endianness needs no swap.  */
-return (end == non_host_endianness) ? MO_BSWAP : 0;
-#endif
+bool big_endian = (end == DEVICE_NATIVE_ENDIAN
+   ? target_words_bigendian()
+   : end == DEVICE_BIG_ENDIAN);


Unnecessary parenthesis?



Not strictly needed indeed.
Code is refactored in patch 14 anyways.


+return big_endian ? MO_BE : MO_LE;
   }
-#endif /* COMPILING_PER_TARGET */
   
   /*

* Inhibit technologies that require discarding of pages in RAM blocks, e.g.,

RE: [PATCH 02/39] target/hexagon: Implement {c,}swi helpers

[ltaylorsimpson]

> -Original Message-
> From: Brian Cain 
> Sent: Friday, February 28, 2025 11:28 PM
> To: qemu-devel@nongnu.org
> Cc: brian.c...@oss.qualcomm.com; richard.hender...@linaro.org;
> phi...@linaro.org; quic_mathb...@quicinc.com; a...@rev.ng; a...@rev.ng;
> quic_mlie...@quicinc.com; ltaylorsimp...@gmail.com;
> alex.ben...@linaro.org; quic_mbur...@quicinc.com;
> sidn...@quicinc.com; Brian Cain 
> Subject: [PATCH 02/39] target/hexagon: Implement {c,}swi helpers
> 
> From: Brian Cain 
> 
> {c,}swi are the "software interrupt"/"Cancel pending interrupts" instructions.
> 
> Signed-off-by: Brian Cain 

Reviewed-by: Taylor Simpson 

Re: [PATCH for-10.1 8/9] target/arm/kvm: don't check TYPE_AARCH64_CPU

[Philippe Mathieu-Daudé]

On 17/3/25 15:28, Peter Maydell wrote:

We want to merge TYPE_AARCH64_CPU with TYPE_ARM_CPU, so enforcing in
kvm_arch_init_vcpu() that the CPU class is a subclass of
TYPE_AARCH64_CPU will no longer be possible.

It's safe to just remove this test, because any purely-AArch32 CPU
will fail the "kvm_target isn't set" check, because we no longer
support the old AArch32-host KVM setup and so CPUs like the Cortex-A7
no longer set cpu->kvm_target. Only the 'host', 'max', and the
odd special cases 'cortex-a53' and 'cortex-a57' set kvm_target.

Signed-off-by: Peter Maydell 
---
  target/arm/kvm.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 0/2] exec: Restrict memory-internal.h to system/

[David Hildenbrand]

On 17.03.25 17:13, Philippe Mathieu-Daudé wrote:

Only file units within the system/ directory need access to
"memory-internal.h". Move it to system/ to restrict its scope.

Based-on: <20250314173139.2122904-1-pierrick.bouv...@linaro.org>


Reviewed-by: David Hildenbrand 

--
Cheers,

David / dhildenb

Re: [PATCH 0/2] exec: Restrict memory-internal.h to system/

[Richard Henderson]

On 3/17/25 09:13, Philippe Mathieu-Daudé wrote:

Only file units within the system/ directory need access to
"memory-internal.h". Move it to system/ to restrict its scope.

Based-on: <20250314173139.2122904-1-pierrick.bouv...@linaro.org>

Philippe Mathieu-Daudé (2):
   accel/tcg: Remove unnecesary inclusion of memory-internal.h in
 cputlb.c
   exec: Restrict memory-internal.h to system/

  MAINTAINERS| 2 +-
  {include/exec => system}/memory-internal.h | 6 --
  accel/tcg/cputlb.c | 1 -
  system/memory.c| 3 ++-
  system/physmem.c   | 3 ++-
  5 files changed, 5 insertions(+), 10 deletions(-)
  rename {include/exec => system}/memory-internal.h (88%)



Reviewed-by: Richard Henderson 


r~

Re: [RFC PATCH v2 03/20] hw/arm/smmuv3-accel: Add initial infrastructure for smmuv3-accel device

[Eric Auger]

On 3/17/25 6:54 PM, Nicolin Chen wrote:
> On Wed, Mar 12, 2025 at 04:15:10PM +0100, Eric Auger wrote:
>> On 3/11/25 3:10 PM, Shameer Kolothum wrote:
>>> Based on SMMUv3 as a parent device, add a user-creatable smmuv3-accel
>>> device. In order to support vfio-pci dev assignment with a Guest
>> guest
>>> SMMUv3, the physical SMMUv3 has to be configured in nested(S1+s2)
>> nested (s1+s2)
>>> mode, with Guest owning the S1 page tables. Subsequent patches will
>> the guest
>>> add support for smmuv3-accel to provide this.
>> Can't this -accel smmu also works with emulated devices? Do we want an
>> exclusive usage?
> Is there any benefit from emulated devices working in the HW-
> accelerated nested translation mode?

Not really but do we have any justification for using different device
name in accel mode? I am not even sure that accel option is really
needed. Ideally the qemu device should be able to detect it is
protecting a VFIO device, in which case it shall check whether nested is
supported by host SMMU and then automatically turn accel mode?

I gave the example of the vfio device which has different class
implementration depending on the iommufd option being set or not.

Thanks

Eric

>
> Thanks
> Nicolin
>

Re: [RFC PATCH v4 5/5] s390: implementing CHSC SEI for AP config change

[Thomas Huth]

On 11/03/2025 16.16, Rorie Reyes wrote:

Handle interception of the CHSC SEI instruction for requests
indicating the guest's AP configuration has changed.

Signed-off-by: Rorie Reyes 
Reviewed-by: Anthony Krowiak 
Tested-by: Anthony Krowiak 
---
  target/s390x/ioinst.c | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
index a944f16c25..f061c6db14 100644
--- a/target/s390x/ioinst.c
+++ b/target/s390x/ioinst.c
@@ -17,6 +17,7 @@
  #include "trace.h"
  #include "hw/s390x/s390-pci-bus.h"
  #include "target/s390x/kvm/pv.h"
+#include "hw/s390x/ap-bridge.h"
  
  /* All I/O instructions but chsc use the s format */

  static uint64_t get_address_from_regs(CPUS390XState *env, uint32_t ipb,
@@ -573,13 +574,19 @@ out:
  
  static int chsc_sei_nt0_get_event(void *res)

  {
-/* no events yet */
+if (s390_has_feat(S390_FEAT_AP)) {
+return ap_chsc_sei_nt0_get_event(res);
+}
+
  return 1;
  }
  
  static int chsc_sei_nt0_have_event(void)

  {
-/* no events yet */
+if (s390_has_feat(S390_FEAT_AP)) {
+return ap_chsc_sei_nt0_have_event();
+}
+
  return 0;
  }


 Hi!

This unfortunately fails to link when configuring QEMU with the 
"--without-default-devices" configure switch:


/usr/bin/ld: libqemu-s390x-softmmu.a.p/target_s390x_ioinst.c.o: in function 
`ioinst_handle_chsc':
/tmp/qemu-mini/target/s390x/ioinst.c:587:(.text+0x1ce1): undefined reference 
to `ap_chsc_sei_nt0_have_event'
/usr/bin/ld: /tmp/qemu-mini/target/s390x/ioinst.c:578:(.text+0x1d1c): 
undefined reference to `ap_chsc_sei_nt0_get_event'

collect2: error: ld returned 1 exit status

I guess you have to rather use some callback mechanism, stubs or #ifdefs 
here instead.


 Thomas

Re: [PATCH v3 3/4] hw/s390x/ccw: Have CCW machine implement a qmp_dump_skeys() callback

[Thomas Huth]

On 10/03/2025 16.14, Philippe Mathieu-Daudé wrote:

In preparation to make @dump-skeys command generic,
extract s390_qmp_dump_skeys() out of qmp_dump_skeys().
Register it as CCW qmp_dump_skeys() callback.

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/hw/s390x/storage-keys.h | 1 +
  hw/s390x/s390-skeys.c   | 7 ++-
  hw/s390x/s390-virtio-ccw.c  | 3 +++
  3 files changed, 10 insertions(+), 1 deletion(-)


Reviewed-by: Thomas Huth 

Re: [PATCH v3 4/4] qapi/machine: Make @dump-skeys command generic

[Thomas Huth]

On 10/03/2025 16.14, Philippe Mathieu-Daudé wrote:

Reduce misc-target.json by one target specific command.

Error message is returned for machines not implementing
TYPE_DUMP_SKEYS_INTERFACE:

   $ qemu-system-aarch64 -M virt -S -qmp stdio
   {"QMP": {"version": {"qemu": {"micro": 50, "major": 9}}, "capabilities": 
["oob"]}}
   { "execute": "qmp_capabilities" }
   {"return": {}}
   { "execute": "dump-skeys", "arguments": { "filename": "/tmp/foo" }  }
   {"error": {"class": "GenericError", "desc": "Storage keys information not 
available for this architecture"}}

Signed-off-by: Philippe Mathieu-Daudé 
---


Reviewed-by: Thomas Huth 

[PULL 2/2] Revert "meson.build: default to -gsplit-dwarf for debug info"

[Paolo Bonzini]

This reverts commit 563b1a35ed1f1151505d4fe5f723827d1b3fd4bc.

Split debug info support is broken when cross compiling
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99973).  People
that would like to use it can add it via --extra-cflags.

Reported-by: Konstantin Kostiuk 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Paolo Bonzini 
---
 meson.build   | 6 --
 meson_options.txt | 2 --
 scripts/meson-buildoptions.sh | 2 --
 3 files changed, 10 deletions(-)

diff --git a/meson.build b/meson.build
index 7f75256acf9..41f68d38069 100644
--- a/meson.build
+++ b/meson.build
@@ -604,10 +604,6 @@ if get_option('tsan')
   qemu_ldflags = ['-fsanitize=thread'] + qemu_ldflags
 endif
 
-if get_option('debug') and get_option('split_debug')
-  qemu_cflags += '-gsplit-dwarf'
-endif
-
 # Detect support for PT_GNU_RELRO + DT_BIND_NOW.
 # The combination is known as "full relro", because .got.plt is read-only too.
 qemu_ldflags += cc.get_supported_link_arguments('-Wl,-z,relro', '-Wl,-z,now')
@@ -4599,8 +4595,6 @@ if have_rust
   summary_info += {'bindgen': bindgen.full_path()}
   summary_info += {'bindgen version': bindgen.version()}
 endif
-# option_cflags is purely for the summary display, meson will pass
-# -g/-O options directly
 option_cflags = (get_option('debug') ? ['-g'] : [])
 if get_option('optimization') != 'plain'
   option_cflags += ['-O' + get_option('optimization')]
diff --git a/meson_options.txt b/meson_options.txt
index 3432123fee2..59d973bca00 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -362,8 +362,6 @@ option('debug_mutex', type: 'boolean', value: false,
description: 'mutex debugging support')
 option('debug_stack_usage', type: 'boolean', value: false,
description: 'measure coroutine stack usage')
-option('split_debug', type: 'boolean', value: true,
-   description: 'split debug info from object files')
 option('qom_cast_debug', type: 'boolean', value: true,
description: 'cast debugging support')
 option('slirp_smbd', type : 'feature', value : 'auto',
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index aca6e688302..3e8e00852b2 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -504,8 +504,6 @@ _meson_option_parse() {
 --disable-strict-rust-lints) printf "%s" -Dstrict_rust_lints=false ;;
 --enable-strip) printf "%s" -Dstrip=true ;;
 --disable-strip) printf "%s" -Dstrip=false ;;
---enable-split-debug) printf "%s" -Dsplit_debug=true ;;
---disable-split-debug) printf "%s" -Dsplit_debug=false ;;
 --sysconfdir=*) quote_sh "-Dsysconfdir=$2" ;;
 --enable-tcg) printf "%s" -Dtcg=enabled ;;
 --disable-tcg) printf "%s" -Dtcg=disabled ;;
-- 
2.48.1

Re: [PATCH v5 00/17] make system memory API available for common code

[Markus Armbruster]

Philippe Mathieu-Daudé  writes:

> Hi,
>
> On 14/3/25 19:39, Pierrick Bouvier wrote:
>> On 3/14/25 11:34, Anthony PERARD wrote:
>>> On Fri, Mar 14, 2025 at 10:33:08AM -0700, Pierrick Bouvier wrote:
 Hi,

 one patch is missing review:
 [PATCH v5 12/17] hw/xen: add stubs for various functions.
>>>
>>> My "Acked-by" wasn't enough? Feel free try change it to "Reviewed-by"
>>> instead.
>>>
>> Those are differents. From what I understand, Reviewed implies Acked, but 
>> the opposite is not true. If it was, they would be equivalent.
>> Thanks.
>
> IIUC on QEMU Acked-by means "as a maintainer of files modified by
> this patch, I don't have objection on my area, as long as someone
> else takes the patch". It doesn't mean the patch has been reviewed.
>
> Please correct me if I'm wrong.

Documentation/process/submitting-patches.rst has some advice, but not
much.  Kernel docs are more thorough, and I believe the information
there applies to QEMU reasonably well:

https://docs.kernel.org/process/5.Posting.html#patch-formatting-and-changelogs

Re: [PATCH for-10.0 1/9] core/cpu.h: gdb_arch_name string should not be freed

[Philippe Mathieu-Daudé]

On 17/3/25 15:28, Peter Maydell wrote:

The documentation for the CPUClass::gdb_arch_name method claims that
the returned string should be freed with g_free().  This is not
correct: in commit a650683871ba728 we changed this method to
instead return a simple constant string, but forgot to update
the documentation.

Make the documentation match the new semantics.

Fixes: a650683871ba728 ("hw/core/cpu: Return static value with gdb_arch_name()")
Signed-off-by: Peter Maydell 
---
  include/hw/core/cpu.h | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 

[PULL 0/2] Fixes for QEMU 10.0 hard freeze

[Paolo Bonzini]

The following changes since commit aa90f1161bb17a4863e16ec2f75104cff0752d4e:

  Merge tag 'migration-20250314-pull-request' of 
https://gitlab.com/farosas/qemu into staging (2025-03-16 02:45:22 -0400)

are available in the Git repository at:

  https://gitlab.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to f35432a4f699d8e450f65e44ddcd5911f2d8c146:

  Revert "meson.build: default to -gsplit-dwarf for debug info" (2025-03-17 
14:22:07 +0100)


fixes


Paolo Bonzini (1):
  Revert "meson.build: default to -gsplit-dwarf for debug info"

Tigran Sogomonian (1):
  hw/misc: use extract64 instead of 1 << i

 meson.build   | 6 --
 hw/misc/mps2-fpgaio.c | 2 +-
 meson_options.txt | 2 --
 scripts/meson-buildoptions.sh | 2 --
 4 files changed, 1 insertion(+), 11 deletions(-)
-- 
2.48.1

Re: [PATCH for-10.1 9/9] target/arm: Remove TYPE_AARCH64_CPU

[Philippe Mathieu-Daudé]

On 17/3/25 15:28, Peter Maydell wrote:

The TYPE_AARCH64_CPU class is an abstract type that is the parent of
all the AArch64 CPUs.  It now has no special behaviour of its own, so
we can eliminate it and make the AArch64 CPUs directly inherit from
TYPE_ARM_CPU.

Signed-off-by: Peter Maydell 
---
  target/arm/cpu-qom.h   |  5 -
  target/arm/cpu.h   |  4 
  target/arm/internals.h |  1 -
  target/arm/cpu64.c | 49 +-
  target/arm/tcg/cpu64.c |  2 +-
  5 files changed, 2 insertions(+), 59 deletions(-)


Thanks!

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH for-10.1 5/9] target/arm: Handle AArch64 gdb read/write regs in TYPE_ARM_CPU

[Philippe Mathieu-Daudé]

On 17/3/25 15:28, Peter Maydell wrote:

Instead of having the TYPE_AARCH64_CPU subclass set
CPUClass::gdb_read_register and ::gdb_write_register to different
methods from those of the TYPE_ARM_CPU parent class, have the
TYPE_ARM_CPU methods handle either AArch32 or AArch64 at runtime.

Signed-off-by: Peter Maydell 
---
  target/arm/cpu64.c   |  5 -
  target/arm/gdbstub.c | 12 
  2 files changed, 12 insertions(+), 5 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH for-10.1 6/9] target/arm: Present AArch64 gdbstub based on ARM_FEATURE_AARCH64

[Philippe Mathieu-Daudé]

On 17/3/25 15:28, Peter Maydell wrote:

Currently we provide an AArch64 gdbstub for CPUs which are
TYPE_AARCH64_CPU, and an AArch32 gdbstub for those which are only
TYPE_ARM_CPU.  This mostly does the right thing, except in the
corner case of KVM with -cpu host,aarch64=off.  That produces a CPU
which is TYPE_AARCH64_CPU but which has ARM_FEATURE_AARCH64 removed
and which to the guest is in AArch32 mode.

Now we have moved all the handling of AArch64-vs-AArch32 gdbstub
behaviour into TYPE_ARM_CPU we can change the condition we use for
whether to select the AArch64 gdbstub to look at ARM_FEATURE_AARCH64.
This will mean that we now correctly provide an AArch32 gdbstub for
aarch64=off CPUs.

Signed-off-by: Peter Maydell 
---
  target/arm/internals.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index a14c269fa5a..a18d87fa28b 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1694,7 +1694,7 @@ void aarch64_add_sme_properties(Object *obj);
  /* Return true if the gdbstub is presenting an AArch64 CPU */
  static inline bool arm_gdbstub_is_aarch64(ARMCPU *cpu)


4 uses, maybe worth removing (directly inlining) as a cleanup
patch on top of this conversion series.


  {
-return object_dynamic_cast(OBJECT(cpu), TYPE_AARCH64_CPU);
+return arm_feature(&cpu->env, ARM_FEATURE_AARCH64);
  }
  
  /* Read the CONTROL register as the MRS instruction would. */

Re: [RFC PATCH v2 07/20] hw/arm/smmu-common: Introduce callbacks for PCIIOMMUOps

[Eric Auger]

Hi Shameer,

On 3/13/25 9:09 AM, Shameerali Kolothum Thodi wrote:
> Hi Eric,
>
>> -Original Message-
>> From: Eric Auger 
>> Sent: Wednesday, March 12, 2025 4:24 PM
>> To: Shameerali Kolothum Thodi
>> ; qemu-...@nongnu.org;
>> qemu-devel@nongnu.org
>> Cc: peter.mayd...@linaro.org; j...@nvidia.com; nicol...@nvidia.com;
>> ddut...@redhat.com; berra...@redhat.com; nath...@nvidia.com;
>> mo...@nvidia.com; smost...@google.com; Linuxarm
>> ; Wangzhou (B) ;
>> jiangkunkun ; Jonathan Cameron
>> ; zhangfei@linaro.org
>> Subject: Re: [RFC PATCH v2 07/20] hw/arm/smmu-common: Introduce
>> callbacks for PCIIOMMUOps
>>
>>
>>
>>
>> On 3/11/25 3:10 PM, Shameer Kolothum wrote:
>>> Subsequently smmuv3-accel will provide these callbacks
>>>
>>> Signed-off-by: Shameer Kolothum
>> 
>>> ---
>>>  hw/arm/smmu-common.c | 27 +++
>>>  include/hw/arm/smmu-common.h |  5 +
>>>  2 files changed, 32 insertions(+)
>>>
>>> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c index
>>> 83c0693f5a..9fd455baa0 100644
>>> --- a/hw/arm/smmu-common.c
>>> +++ b/hw/arm/smmu-common.c
>>> @@ -865,6 +865,10 @@ static AddressSpace *smmu_find_add_as(PCIBus
>> *bus, void *opaque, int devfn)
>>>  SMMUState *s = opaque;
>>>  SMMUPciBus *sbus = smmu_get_sbus(s, bus);
>>>
>>> +if (s->accel && s->get_address_space) {
>>> +return s->get_address_space(bus, opaque, devfn);
>>> +}
>>> +
>> why do we require that new call site? This needs to be documented in the
>> commit msg esp. because we don't know what this cb will do.
> Currently, this is where the first time SMMUDevice sdev is allocated. And for 
> smmuv3-accel
> cases we are introducing a new SMMUv3AccelDevice accel_dev for holding 
> additional
> accel specific information. In order to do that the above cb is used. Same 
> for other callbacks
> as well.
>
> Another way of avoiding the callbacks would be to  move the 
> pci_setup_iommu(bus, ops) 
> call from the smmu-common.c to smmuv3/smmuv3-accel and handle it directly 
> there.
>
> Or is there a better idea?
>
>>>  sdev = sbus->pbdev[devfn];
>>>  if (!sdev) {
>>>  sdev = sbus->pbdev[devfn] = g_new0(SMMUDevice, 1); @@ -874,8
>>> +878,31 @@ static AddressSpace *smmu_find_add_as(PCIBus *bus, void
>> *opaque, int devfn)
>>>  return &sdev->as;
>>>  }
>>>
>>> +static bool smmu_dev_set_iommu_device(PCIBus *bus, void *opaque,
>> int devfn,
>>> +  HostIOMMUDevice *hiod, Error
>>> +**errp) {
>>> +SMMUState *s = opaque;
>>> +
>>> +if (s->accel && s->set_iommu_device) {
>>> +return s->set_iommu_device(bus, opaque, devfn, hiod, errp);
>>> +}
>>> +
>>> +return false;
>>> +}
>>> +
>>> +static void smmu_dev_unset_iommu_device(PCIBus *bus, void *opaque,
>>> +int devfn) {
>>> +SMMUState *s = opaque;
>>> +
>>> +if (s->accel && s->unset_iommu_device) {
>>> +s->unset_iommu_device(bus, opaque, devfn);
>>> +}
>>> +}
>>> +
>>>  static const PCIIOMMUOps smmu_ops = {
>>>  .get_address_space = smmu_find_add_as,
>>> +.set_iommu_device = smmu_dev_set_iommu_device,
>>> +.unset_iommu_device = smmu_dev_unset_iommu_device,
>>>  };
>>>
>>>  SMMUDevice *smmu_find_sdev(SMMUState *s, uint32_t sid) diff --git
>>> a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
>> index
>>> 80ff2ef6aa..7b05640167 100644
>>> --- a/include/hw/arm/smmu-common.h
>>> +++ b/include/hw/arm/smmu-common.h
>>> @@ -160,6 +160,11 @@ struct SMMUState {
>>>
>>>  /* For smmuv3-accel */
>>>  bool accel;
>>> +
>>> +AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, int
>> devfn);
>>> +bool (*set_iommu_device)(PCIBus *bus, void *opaque, int devfn,
>>> + HostIOMMUDevice *dev, Error **errp);
>>> +void (*unset_iommu_device)(PCIBus *bus, void *opaque, int devfn);
>> I think this should be exposed by a class and only implemented in the
>> smmuv3 accel device. Adding those cbs directly in the State looks not the
>> std way.
> Ok. You mean we can directly place  PCIIOMMUOps *ops here then? 
When I first skimmed through the series I assumed you would use 2
seperate devices, in which case that would use 2 different
implementations of the same class. You may have a look at
docs/devel/qom.rst and Methods and class there.

Now as I commented earlier I think the end user shall instantiate the
same device for non accel and accel. I would advocate for passing an
option telling whether we want accel modality. Then it rather looks like
what was done for vfio device with either legacy or iommufd backend.

depending on whether the iommufd option is passed you select the right
class implementation:
see hw/vfio/common.c and vfio_attach_device


    const VFIOIOMMUClass *ops =
    VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY));

    if (vbasedev->iommufd) {
    ops =
VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
    }

I would doing something si

Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a pxb-pcie bus

[Eric Auger]

On 3/13/25 9:22 AM, Shameerali Kolothum Thodi wrote:
> Hi Eric,
>
>> -Original Message-
>> From: Eric Auger 
>> Sent: Wednesday, March 12, 2025 4:42 PM
>> To: Shameerali Kolothum Thodi
>> ; qemu-...@nongnu.org;
>> qemu-devel@nongnu.org
>> Cc: peter.mayd...@linaro.org; j...@nvidia.com; nicol...@nvidia.com;
>> ddut...@redhat.com; berra...@redhat.com; nath...@nvidia.com;
>> mo...@nvidia.com; smost...@google.com; Linuxarm
>> ; Wangzhou (B) ;
>> jiangkunkun ; Jonathan Cameron
>> ; zhangfei@linaro.org
>> Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a pxb-
>> pcie bus
>>
>>
>>
>>
>> On 3/12/25 5:34 PM, Shameerali Kolothum Thodi wrote:
>>> Hi Eric,
>>>
 -Original Message-
 From: Eric Auger 
 Sent: Wednesday, March 12, 2025 4:08 PM
 To: Shameerali Kolothum Thodi
 ; qemu-...@nongnu.org;
 qemu-devel@nongnu.org
 Cc: peter.mayd...@linaro.org; j...@nvidia.com; nicol...@nvidia.com;
 ddut...@redhat.com; berra...@redhat.com; nath...@nvidia.com;
 mo...@nvidia.com; smost...@google.com; Linuxarm
 ; Wangzhou (B) ;
 jiangkunkun ; Jonathan Cameron
 ; zhangfei@linaro.org
 Subject: Re: [RFC PATCH v2 05/20] hw/arm/smmuv3-accel: Associate a
>> pxb-
 pcie bus

 Hi Shameer,


 On 3/11/25 3:10 PM, Shameer Kolothum wrote:
> User must associate a pxb-pcie root bus to smmuv3-accel
> and that is set as the primary-bus for the smmu dev.
 why do we require a pxb-pcie root bus? why can't pci.0 root bus be used
 for simpler use cases (ie. I just want to passthough a NIC in
 accelerated mode). Or may pci.0 is also called a pax-pcie root bus?
>>> The idea was since pcie.0 is the default RC with virt, leave that to cases
>> where
>>> we want to attach any emulated devices and use pxb-pcie based RCs for
>> vfio-pci.
>> yes but for simpler use case you may not want the extra pain to
>> instantiate a pxb-pcie device. Actually libvirt does not instantiate it
>> by default.
 Besides, why do we put the constraint to plug on a root bus. I know that
 at this point we always plug to pci.0 but with the new -device option it
 would be possible to plug it anywhere in the pcie hierarchy. At SOC
 level can't an SMMU be plugged anywhere protecting just a few RIDs?
>>> In my understanding normally(or atleast in the most common cases) it is
>> attached
>>> to root complexes. Also IORT mappings are at the root complex level,
>> right?
>> Yes I do agree the IORT describes ID mappings between RC and SMMU but
>> the actual ID mappings allow you to be much more precise and state that
>> a given SMMU only translates few RIDs within that RID space. If you
>> force the device bus to be a root bus you can't model that anymore.
>>
> Do we really need to support that? What if the user then have another 
> smmuv3-accel
> in the associated upstream buses/RC as well? Not sure how to handle that.
Well I agree we would need to reject such kind of config. Maybe we can
relax this requirement and connect the smmu to a root bus (including
pci.0 though). Then this SMMU would translate all the input RIDs. This
is not as flexible as what the IORT allows but maybe that's enough for
our use cases.

Thanks

Eric
>  
> Thanks,
> Shameer

Re: [PATCH v2 1/2] gdbstub: Improve physical memory access handling

[Richard Henderson]

On 3/16/25 22:16, Nicholas Piggin wrote:

Bring gdb's physical memory access handling up to speed with the CPU
memory access, by setting MemTxAttribute.debug=1, and by checking for
memory transaction errors.

GDB with PhyMemMode will now report failure for memory access outside
valid system memory addresses, and it is also able to write to ROMs as
it can with virtual memory access.

Reviewed-by: David Hildenbrand
Signed-off-by: Nicholas Piggin
---
  gdbstub/system.c | 27 +--
  1 file changed, 21 insertions(+), 6 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 2/2] memory: suppress INVALID_MEM logs caused by debug access

[Richard Henderson]

On 3/16/25 22:16, Nicholas Piggin wrote:

Debugger-driven invalid memory accesses are not guest errors, so should
not cause these error logs.

Debuggers can access memory wildly, including access to addresses not
specified by the user (e.g., gdb it might try to walk the stack or load
target addresses to display disassembly). Failure is reported
synchronously by the GDB protcol so the user can be notified via the
debugger client.

Reviewed-by: David Hildenbrand 
Signed-off-by: Nicholas Piggin 
---
  system/memory.c | 37 ++---
  1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index 4c829793a0a..960f66e8d7e 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1412,18 +1412,23 @@ bool memory_region_access_valid(MemoryRegion *mr,
  {
  if (mr->ops->valid.accepts
  && !mr->ops->valid.accepts(mr->opaque, addr, size, is_write, attrs)) {
-qemu_log_mask(LOG_INVALID_MEM, "Invalid %s at addr 0x%" HWADDR_PRIX
-  ", size %u, region '%s', reason: rejected\n",
-  is_write ? "write" : "read",
-  addr, size, memory_region_name(mr));
+if (attrs.debug) {


!attrs.debug.


r~

Re: [PATCH v5 11/17] exec/ram_addr: call xen_hvm_modified_memory only if xen is enabled

[Pierrick Bouvier]

On 3/17/25 09:23, Philippe Mathieu-Daudé wrote:

On 17/3/25 17:22, Philippe Mathieu-Daudé wrote:

On 17/3/25 17:07, Pierrick Bouvier wrote:

On 3/17/25 08:50, Philippe Mathieu-Daudé wrote:

On 14/3/25 18:31, Pierrick Bouvier wrote:

Reviewed-by: Richard Henderson 
Signed-off-by: Pierrick Bouvier 
---
    include/exec/ram_addr.h | 8 ++--
    1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index f5d574261a3..92e8708af76 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -339,7 +339,9 @@ static inline void
cpu_physical_memory_set_dirty_range(ram_addr_t start,
    }
    }
-    xen_hvm_modified_memory(start, length);
+    if (xen_enabled()) {
+    xen_hvm_modified_memory(start, length);


Please remove the stub altogether.



We can eventually ifdef this code under CONFIG_XEN, but it may still
be available or not. The matching stub for xen_hvm_modified_memory()
will assert in case it is reached.

Which change would you expect precisely?


-- >8 --
diff --git a/include/system/xen-mapcache.h b/include/system/xen-mapcache.h
index b68f196ddd5..bb454a7c96c 100644
--- a/include/system/xen-mapcache.h
+++ b/include/system/xen-mapcache.h
@@ -14,8 +14,6 @@

   typedef hwaddr (*phys_offset_to_gaddr_t)(hwaddr phys_offset,
    ram_addr_t size);
-#ifdef CONFIG_XEN_IS_POSSIBLE
-
   void xen_map_cache_init(phys_offset_to_gaddr_t f,
   void *opaque);
   uint8_t *xen_map_cache(MemoryRegion *mr, hwaddr phys_addr, hwaddr size,
@@ -28,44 +26,5 @@ void xen_invalidate_map_cache(void);
   uint8_t *xen_replace_cache_entry(hwaddr old_phys_addr,
    hwaddr new_phys_addr,
    hwaddr size);
-#else
-
-static inline void xen_map_cache_init(phys_offset_to_gaddr_t f,
-  void *opaque)
-{
-}
-
-static inline uint8_t *xen_map_cache(MemoryRegion *mr,
- hwaddr phys_addr,
- hwaddr size,
- ram_addr_t ram_addr_offset,
- uint8_t lock,
- bool dma,
- bool is_write)
-{
-    abort();
-}
-
-static inline ram_addr_t xen_ram_addr_from_mapcache(void *ptr)
-{
-    abort();
-}
-
-static inline void xen_invalidate_map_cache_entry(uint8_t *buffer)
-{
-}
-
-static inline void xen_invalidate_map_cache(void)
-{
-}
-
-static inline uint8_t *xen_replace_cache_entry(hwaddr old_phys_addr,
-   hwaddr new_phys_addr,
-   hwaddr size)
-{
-    abort();
-}
-
-#endif

   #endif /* XEN_MAPCACHE_H */


(sorry, the include/system/xen-mapcache.h change is for the next patch)


diff --git a/include/system/xen.h b/include/system/xen.h
index 990c19a8ef0..04fe30cca50 100644
--- a/include/system/xen.h
+++ b/include/system/xen.h
@@ -30,25 +30,16 @@ extern bool xen_allowed;

   #define xen_enabled()   (xen_allowed)

-void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length);
-void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
-   struct MemoryRegion *mr, Error **errp);
-
   #else /* !CONFIG_XEN_IS_POSSIBLE */

   #define xen_enabled() 0
-static inline void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t
length)
-{
-    /* nothing */
-}
-static inline void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
- MemoryRegion *mr, Error **errp)
-{
-    g_assert_not_reached();
-}

   #endif /* CONFIG_XEN_IS_POSSIBLE */

+void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length);
+void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
+   MemoryRegion *mr, Error **errp);
+
   bool xen_mr_is_memory(MemoryRegion *mr);
   bool xen_mr_is_grants(MemoryRegion *mr);
   #endif
---




Sounds good!
I will include it in next version.

Re: [PATCH RFC v2] Integration coroutines into fuse export

[Stefan Hajnoczi]

On Sun, Mar 16, 2025 at 01:30:20AM +0800, saz97 wrote:
> Signed-off-by: Changzhi Xie 
> 
> This commit refactors the FUSE export to process read and write operations
> using coroutines, improving concurrency and avoiding blocking the main loop.
> 
> The main changes include:
> 1.  Introduce FuseIORequest structure to encapsulate I/O parameters and state
> 2.  Move read/write processing into coroutine fuse_read_coroutine and 
> fuse_write_coroutine
> 3.  Use blk_co_pread/pwrite for async block layer access
> ---
>  block/export/fuse.c | 189 +++-
>  1 file changed, 132 insertions(+), 57 deletions(-)
> 
> diff --git a/block/export/fuse.c b/block/export/fuse.c
> index 465cc9891d..3314f64706 100644
> --- a/block/export/fuse.c
> +++ b/block/export/fuse.c
> @@ -39,6 +39,7 @@
>  
>  #ifdef __linux__
>  #include 
> +#include 
>  #endif
>  
>  /* Prevent overly long bounce buffer allocations */
> @@ -49,7 +50,6 @@ typedef struct FuseExport {
>  BlockExport common;
>  
>  struct fuse_session *fuse_session;
> -struct fuse_buf fuse_buf;
>  unsigned int in_flight; /* atomic */
>  bool mounted, fd_handler_set_up;
>  
> @@ -64,6 +64,14 @@ typedef struct FuseExport {
>  gid_t st_gid;
>  } FuseExport;
>  
> +typedef struct FuseIORequest {
> +fuse_req_t req;
> +size_t size;
> +off_t offset;
> +FuseExport *exp;
> +char *write_buf;
> +} FuseIORequest;
> +
>  static GHashTable *exports;
>  static const struct fuse_lowlevel_ops fuse_ops;
>  
> @@ -288,6 +296,7 @@ fail:
>  static void read_from_fuse_export(void *opaque)
>  {
>  FuseExport *exp = opaque;
> +struct fuse_buf buf = {};
>  int ret;
>  
>  blk_exp_ref(&exp->common);
> @@ -295,20 +304,30 @@ static void read_from_fuse_export(void *opaque)
>  qatomic_inc(&exp->in_flight);
>  
>  do {
> -ret = fuse_session_receive_buf(exp->fuse_session, &exp->fuse_buf);
> +ret = fuse_session_receive_buf(exp->fuse_session, &buf);
>  } while (ret == -EINTR);
>  if (ret < 0) {
>  goto out;
>  }
>  
> -fuse_session_process_buf(exp->fuse_session, &exp->fuse_buf);
> +fuse_session_process_buf(exp->fuse_session, &buf);
>  
>  out:
> +struct fuse_in_header *in = (struct fuse_in_header *)buf.mem;
> +
> +if (in->opcode == FUSE_WRITE || in->opcode == FUSE_READ) {
> +g_free(buf.mem);
> +return;

Returning here is not safe because &buf was passed to
fuse_session_process_buf() and is located on read_from_fuse_export()'s
stack. The coroutine must not access buf after this function returns.

I suggest moving most of this function into a coroutine so that struct
fuse_buf can be on the coroutine's stack. That way it outlives
fuse_session_process_buf().

Doing this also avoids duplicating the code below and eliminates the
need for FuseIORequest.

> +}
> +
>  if (qatomic_fetch_dec(&exp->in_flight) == 1) {
>  aio_wait_kick(); /* wake AIO_WAIT_WHILE() */
>  }
>  
>  blk_exp_unref(&exp->common);
> +
> +g_free(buf.mem);

Please make this free(buf.mem) since libfuse uses malloc(3).

> +
>  }
>  
>  static void fuse_export_shutdown(BlockExport *blk_exp)
> @@ -347,7 +366,6 @@ static void fuse_export_delete(BlockExport *blk_exp)
>  fuse_session_destroy(exp->fuse_session);
>  }
>  
> -free(exp->fuse_buf.mem);
>  g_free(exp->mountpoint);
>  }
>  
> @@ -570,102 +588,159 @@ static void fuse_open(fuse_req_t req, fuse_ino_t 
> inode,
>  fuse_reply_open(req, fi);
>  }
>  
> -/**
> - * Handle client reads from the exported image.
> - */
> -static void fuse_read(fuse_req_t req, fuse_ino_t inode,
> -  size_t size, off_t offset, struct fuse_file_info *fi)
> +static void coroutine_fn fuse_read_coroutine(void *opaque)
>  {
> -FuseExport *exp = fuse_req_userdata(req);
> +FuseIORequest *io_req = opaque;
> +FuseExport *exp = io_req->exp;
>  int64_t length;
> -void *buf;
> +void *buffer;
>  int ret;
>  
> -/* Limited by max_read, should not happen */
> -if (size > FUSE_MAX_BOUNCE_BYTES) {
> -fuse_reply_err(req, EINVAL);
> -return;
> +if (io_req->size > FUSE_MAX_BOUNCE_BYTES) {
> +fuse_reply_err(io_req->req, EINVAL);
> +goto cleanup;
>  }
>  
> -/**
> - * Clients will expect short reads at EOF, so we have to limit
> - * offset+size to the image length.
> - */
>  length = blk_getlength(exp->common.blk);
>  if (length < 0) {
> -fuse_reply_err(req, -length);
> -return;
> +fuse_reply_err(io_req->req, -length);
> +goto cleanup;
>  }
>  
> -if (offset + size > length) {
> -size = length - offset;
> +if (io_req->offset + io_req->size > length) {
> +io_req->size = length - io_req->offset;
>  }
>  
> -buf = qemu_try_blockalign(blk_bs(exp->common.blk), size);
> -if (!buf) {
> -fuse_reply_err(req, ENOMEM);
> -return;
>

Re: [PATCH] docs/system: Fix the information on how to run certain functional tests

[Niek Linnenbank]

Hi Thomas,

On Mon, Mar 17, 2025 at 7:50 AM Thomas Huth  wrote:

>   Hi!
>
> On 16/03/2025 21.21, Niek Linnenbank wrote:
> > Hi Thomas,
> >
> > On Tue, Mar 11, 2025 at 5:08 PM Thomas Huth  > > wrote:
> >
> > The tests have been converted to the functional framework, so
> > we should not talk about Avocado here anymore.
> ...
> > diff --git a/docs/system/arm/orangepi.rst
> b/docs/system/arm/orangepi.rst
> > index db87e81fec4..8b9448ca7b0 100644
> > --- a/docs/system/arm/orangepi.rst
> > +++ b/docs/system/arm/orangepi.rst
> > @@ -257,9 +257,9 @@ Orange Pi PC integration tests
> >
> > Perhaps for consistency, we can also rename to 'functional tests' here.
>
> Agreed, we don't use the term "integration tests" for this anymore.
>
> >
> >   The Orange Pi PC machine has several integration tests included.
> >
> > And same on this line.
> >
> >   To run the whole set of tests, build QEMU from source and simply
> > -provide the following command:
> > +provide the following command from the build directory:
> >
> >   .. code-block:: bash
> >
> > -  $ AVOCADO_ALLOW_LARGE_STORAGE=yes avocado --show=app,console run \
> > - -t machine:orangepi-pc tests/avocado/boot_linux_console.py
> > +  $ QEMU_TEST_ALLOW_LARGE_STORAGE=1 \
> > +meson test --suite thorough func-arm-arm_orangepi
> >
> > I've tried to run on my Ubuntu 24.04.1 LTS based system using this exact
> > same command, but got this error:
> >
> > $ QEMU_TEST_ALLOW_LARGE_STORAGE=1 meson test --suite thorough func-arm-
> > arm_orangepi
> >
> > ERROR: Build data file '/home/user/qemu/build/meson-private/build.dat'
> > references functions or classes that don't exist. This probably means
> that
> > it was generated with an old version of meson. Consider reconfiguring
> the
> > directory with "meson setup --reconfigure".
> >
> > The meson version I have installed via apt-get is 1.3.2-1ubuntu1. Only
> when
> > running using the 'meson' command from the pyvenv, it runs OK:
> >
> > $ QEMU_TEST_ALLOW_LARGE_STORAGE=1 ./pyvenv/bin/meson test --suite
> thorough
> > func-arm-arm_orangepi
>
> Oh, you're right! Thanks for catching it!
>
> I guess I still had a "export PYTHONPATH=$HOME/qemu/python" in my
> environment, so I did not notice. Would you like to send a patch, or want
> me
> to do it?
>

Yes please feel free to go ahead with the patch. With the above minor
remarks resolved, it looks fine to me:

Reviewed-by: Niek Linnenbank 

Regards,
Niek


>
>   Thomas
>
>

-- 
Niek Linnenbank

Re: [PATCH 0/1 RFC] FUSE Export Coroutine Integration Cover Letter

[Stefan Hajnoczi]

On Sun, Mar 16, 2025 at 01:30:06AM +0800, saz97 wrote:
> Signed-off-by: Changzhi Xie 
> 
> FUSE Export Coroutine Integration Cover Letter
> 
> This patch series refactors QEMU's FUSE export module to leverage coroutines 
> for read/write operations, 
> addressing concurrency limitations and aligning with QEMU's asynchronous I/O 
> model. The changes 
> demonstrate measurable performance improvements while simplifying resource 
> management.
> 
> 1. Technical Implementation
> Key modifications address prior review feedback (Stefan Hajnoczi) and 
> optimize execution flow:
> 
> ​1.1 Coroutine Integration
> Convert fuse_read()/fuse_write() to launch coroutines (fuse_*_coroutine)
> Utilize non-blocking blk_co_pread()/blk_co_pwrite() for block layer access
> Eliminate main loop blocking during heavy I/O workloads
> 
> 1.2 ​Buffer Management
> Removed explicit buffer pre-allocation in read_from_fuse_export()
> Replaced fuse_buf_free() with g_free() due to libfuse3 API constraints
> 
> ​1.3 Resource Lifecycle
> Moved in_flight decrement and blk_exp_unref() into coroutines
> Added FUSE opcode checks (FUSE_READ/FUSE_WRITE) to prevent premature cleanup
> 
> 1.4 ​Structural Improvements
> Simplified FuseIORequest structure:
> Removed redundant fuse_ino_t and fuse_file_info fields
> Retained minimal parameter passing requirements
> 
> 2. Performance Validation
> Tested using fio with 4K random RW pattern, and the result is the average of 
> 5 runs:
> fio --ioengine=io_uring --numjobs=1 --runtime=30 --ramp_time=5 --rw=randrw 
> --bs=4k --time_based=1
> 
> Key Results
> 
> Metric   iodepth=1   iodepth=64
> ​Read Latency   ▼ 2.7% (3.8k→3kns)  ▼ 1.3% (4.7M→4.6M ns)
> ​Write Latency▼ 3.6% (112k→108kns)▼ 2.8% (5.2M→5.0M ns)
> ​Read IOPS4740 → 4729 (±0.2%)   ▲ 2.1% (6391→6529)
> ​Write IOPS   4738 → 4727 (±0.2%)   ▲ 2.2% (6390→6529)
> ​Throughput   ~18.9 GB/s (stable)   ▲ 2.1% (25.6→26.1 GB/s)

Are you sure throughput is GB/s instead of MB/s?

iodepth=1 read 4729 IOPS * bs=4k = 18 MB/s

Also, fio was configured with --rw=randrw, so the total throughput
should be read throughput + write throughput. Based on the read and
write IOPS numbers, the total throughput should be ~36 MB/s. Which
throughput number are you showing?

> 
> Analysis
> 
> ​High Concurrency (iodepth=64):
> Sustained throughput gains (+2.1-2.2%) demonstrate improved scalability
> Latency reductions confirm reduced contention in concurrent operations

This is surprising. Before this patch series the FUSE export code only
submits 1 request at a time, so the iodepth=64 results should be only a
little better than the iodepth=1 results. After this patch series the
FUSE export code should be submitting all 64 requests concurrently and
improving performance by more than 2%.

Why was the improvement only 2%?

> 
> saz97 (1):
>   Integration coroutines into fuse export
> 
>  block/export/fuse.c | 189 +++-
>  1 file changed, 132 insertions(+), 57 deletions(-)
> 
> -- 
> 2.34.1
> 


signature.asc
Description: PGP signature

[BUG][powerpc] KVM Guest Boot Failure – Hangs at "Booting Linux via __start()”

[misanjum]

Bug Description:
Encountering a boot failure when launching a KVM guest with 
qemu-system-ppc64. The guest hangs at boot, and the QEMU monitor 
crashes.



Reproduction Steps:
# qemu-system-ppc64 --version
QEMU emulator version 9.2.50 (v9.2.0-2799-g0462a32b4f)
Copyright (c) 2003-2025 Fabrice Bellard and the QEMU Project developers

# /usr/bin/qemu-system-ppc64 -name avocado-vt-vm1 -machine 
pseries,accel=kvm \

  -m 32768 -smp 32,sockets=1,cores=32,threads=1 -nographic \
  -device virtio-scsi-pci,id=scsi \
  -drive 
file=/home/kvmci/tests/data/avocado-vt/images/rhel8.0devel-ppc64le.qcow2,if=none,id=drive0,format=qcow2 
\

  -device scsi-hd,drive=drive0,bus=scsi.0 \
  -netdev bridge,id=net0,br=virbr0 \
  -device virtio-net-pci,netdev=net0 \
  -serial pty \
  -device virtio-balloon-pci \
  -cpu host
QEMU 9.2.50 monitor - type 'help' for more information
char device redirected to /dev/pts/2 (label serial0)
(qemu)
(qemu) qemu-system-ppc64: warning: kernel_irqchip allowed but 
unavailable: IRQ_XIVE capability must be present for KVM

Falling back to kernel-irqchip=off
** Qemu Hang

(In another ssh session)
# screen /dev/pts/2
Preparing to boot Linux version 6.10.4-200.fc40.ppc64le 
(mockbuild@c23cc4e677614c34bb22d54eeea4dc1f) (gcc (GCC) 14.2.1 20240801 
(Red Hat 14.2.1-1), GNU ld version 2.41-37.fc40) #1 SMP Sun Aug 11 
15:20:17 UTC 2024

Detected machine type: 0101
command line: 
BOOT_IMAGE=(ieee1275/disk,msdos2)/vmlinuz-6.10.4-200.fc40.ppc64le 
root=/dev/mapper/fedora-root ro rd.lvm.lv=fedora/root crashkernel=1024M

Max number of cores passed to firmware: 2048 (NR_CPUS = 2048)
Calling ibm,client-architecture-support... done
memory layout at init:
  memory_limit :  (16 MB aligned)
  alloc_bottom : 0820
  alloc_top: 3000
  alloc_top_hi : 0008
  rmo_top  : 3000
  ram_top  : 0008
instantiating rtas at 0x2fff... done
prom_hold_cpus: skipped
copying OF device tree...
Building dt strings...
Building dt structure...
Device tree strings 0x0821 -> 0x08210bd0
Device tree struct  0x0822 -> 0x0823
Quiescing Open Firmware ...
Booting Linux via __start() @ 0x0044 ...
** Guest Console Hang


Git Bisect:
Performing git bisect points to the following patch:
# git bisect bad
e8291ec16da80566c121c68d9112be458954d90b is the first bad commit
commit e8291ec16da80566c121c68d9112be458954d90b (HEAD)
Author: Nicholas Piggin 
Date:   Thu Dec 19 13:40:31 2024 +1000

target/ppc: fix timebase register reset state

(H)DEC and PURR get reset before icount does, which causes them to 
be

skewed and not match the init state. This can cause replay to not
match the recorded trace exactly. For DEC and HDEC this is usually 
not

noticable since they tend to get programmed before affecting the
target machine. PURR has been observed to cause replay bugs when
running Linux.

Fix this by resetting using a time of 0.

Message-ID: <20241219034035.1826173-2-npig...@gmail.com>
Signed-off-by: Nicholas Piggin 

 hw/ppc/ppc.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)


Reverting the patch helps boot the guest.
Thanks,
Misbah Anjum N

Re: [PATCH v3 0/7] Move memory listener register to vhost_vdpa_init

[Lei Yang]

Hi Jonah

I tested this series with the vhost_vdpa device based on mellanox
ConnectX-6 DX nic and hit the host kernel crash. This problem can be
easier to reproduce under the hotplug/unplug device scenario.
For the core dump messages please review the attachment.
FW version:
#  flint -d :0d:00.0 q |grep Version
FW Version:22.44.1036
Product Version:   22.44.1036

Best Regards
Lei

On Fri, Mar 14, 2025 at 9:04 PM Jonah Palmer  wrote:
>
> Current memory operations like pinning may take a lot of time at the
> destination.  Currently they are done after the source of the migration is
> stopped, and before the workload is resumed at the destination.  This is a
> period where neigher traffic can flow, nor the VM workload can continue
> (downtime).
>
> We can do better as we know the memory layout of the guest RAM at the
> destination from the moment that all devices are initializaed.  So
> moving that operation allows QEMU to communicate the kernel the maps
> while the workload is still running in the source, so Linux can start
> mapping them.
>
> As a small drawback, there is a time in the initialization where QEMU
> cannot respond to QMP etc.  By some testing, this time is about
> 0.2seconds.  This may be further reduced (or increased) depending on the
> vdpa driver and the platform hardware, and it is dominated by the cost
> of memory pinning.
>
> This matches the time that we move out of the called downtime window.
> The downtime is measured as checking the trace timestamp from the moment
> the source suspend the device to the moment the destination starts the
> eight and last virtqueue pair.  For a 39G guest, it goes from ~2.2526
> secs to 2.0949.
>
> Future directions on top of this series may include to move more things ahead
> of the migration time, like set DRIVER_OK or perform actual iterative 
> migration
> of virtio-net devices.
>
> Comments are welcome.
>
> This series is a different approach of series [1]. As the title does not
> reflect the changes anymore, please refer to the previous one to know the
> series history.
>
> This series is based on [2], it must be applied after it.
>
> [Jonah Palmer]
> This series was rebased after [3] was pulled in, as [3] was a prerequisite
> fix for this series.
>
> v3:
> ---
> * Rebase
>
> v2:
> ---
> * Move the memory listener registration to vhost_vdpa_set_owner function.
> * Move the iova_tree allocation to net_vhost_vdpa_init.
>
> v1 at https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg02136.html.
>
> [1] 
> https://patchwork.kernel.org/project/qemu-devel/cover/20231215172830.2540987-1-epere...@redhat.com/
> [2] https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg05910.html
> [3] 
> https://lore.kernel.org/qemu-devel/20250217144936.3589907-1-jonah.pal...@oracle.com/
>
> Eugenio Pérez (7):
>   vdpa: check for iova tree initialized at net_client_start
>   vdpa: reorder vhost_vdpa_set_backend_cap
>   vdpa: set backend capabilities at vhost_vdpa_init
>   vdpa: add listener_registered
>   vdpa: reorder listener assignment
>   vdpa: move iova_tree allocation to net_vhost_vdpa_init
>   vdpa: move memory listener register to vhost_vdpa_init
>
>  hw/virtio/vhost-vdpa.c | 98 ++
>  include/hw/virtio/vhost-vdpa.h | 22 +++-
>  net/vhost-vdpa.c   | 34 ++--
>  3 files changed, 88 insertions(+), 66 deletions(-)
>
> --
> 2.43.5
>
>
[  257.598060] openvswitch: Open vSwitch switching datapath
[  257.826760] mlx5_core :0d:00.0: E-Switch: Enable: mode(LEGACY), nvfs(4), 
necvfs(0), active vports(5)
[  257.928288] pci :0d:00.2: [15b3:101e] type 00 class 0x02 PCIe 
Endpoint
[  257.928363] pci :0d:00.2: enabling Extended Tags
[  257.931674] mlx5_core :0d:00.2: enabling device ( -> 0002)
[  257.931747] mlx5_core :0d:00.2: PTM is not supported by PCIe
[  257.931766] mlx5_core :0d:00.2: firmware version: 22.44.1036
[  258.127431] mlx5_core :0d:00.2: Rate limit: 127 rates are supported, 
range: 0Mbps to 97656Mbps
[  258.139089] mlx5_core :0d:00.2: Assigned random MAC address 
76:34:f6:43:68:d7
[  258.282267] mlx5_core :0d:00.2: MLX5E: StrdRq(1) RqSz(8) StrdSz(2048) 
RxCqeCmprss(0 enhanced)
[  258.286453] mlx5_core :0d:00.2 ens2f0v0: renamed from eth0
[  258.317591] pci :0d:00.3: [15b3:101e] type 00 class 0x02 PCIe 
Endpoint
[  258.317669] pci :0d:00.3: enabling Extended Tags
[  258.321033] mlx5_core :0d:00.3: enabling device ( -> 0002)
[  258.321110] mlx5_core :0d:00.3: PTM is not supported by PCIe
[  258.321128] mlx5_core :0d:00.3: firmware version: 22.44.1036
[  258.442412] mlx5_core :0d:00.2 ens2f0v0: Link up
[  258.521068] mlx5_core :0d:00.3: Rate limit: 127 rates are supported, 
range: 0Mbps to 97656Mbps
[  258.532963] mlx5_core :0d:00.3: Assigned random MAC address 
0a:b5:ca:d1:b3:e8
[  258.674658] mlx5_core :0d:00.3: MLX5E: StrdRq(1) RqSz(8) StrdSz(2048) 
RxCqeCmprss(0 enhanced)
[  258.678230] m

Re: [PATCH v3 4/7] memory-attribute-manager: Introduce MemoryAttributeManager to manage RAMBLock with guest_memfd

[Chenyi Qiang]

On 3/18/2025 1:01 AM, Gupta, Pankaj wrote:
> On 3/17/2025 11:36 AM, David Hildenbrand wrote:
>> On 17.03.25 03:54, Chenyi Qiang wrote:
>>>
>>>
>>> On 3/14/2025 8:11 PM, Gupta, Pankaj wrote:
 On 3/10/2025 9:18 AM, Chenyi Qiang wrote:
> As the commit 852f0048f3 ("RAMBlock: make guest_memfd require
> uncoordinated discard") highlighted, some subsystems like VFIO may
> disable ram block discard. However, guest_memfd relies on the discard
> operation to perform page conversion between private and shared
> memory.
> This can lead to stale IOMMU mapping issue when assigning a hardware
> device to a confidential VM via shared memory. To address this, it is
> crucial to ensure systems like VFIO refresh its IOMMU mappings.
>
> RamDiscardManager is an existing concept (used by virtio-mem) to
> adjust
> VFIO mappings in relation to VM page assignment. Effectively page
> conversion is similar to hot-removing a page in one mode and adding it
> back in the other. Therefore, similar actions are required for page
> conversion events. Introduce the RamDiscardManager to guest_memfd to
> facilitate this process.
>
> Since guest_memfd is not an object, it cannot directly implement the
> RamDiscardManager interface. One potential attempt is to implement
> it in
> HostMemoryBackend. This is not appropriate because guest_memfd is per
> RAMBlock. Some RAMBlocks have a memory backend but others do not. In
> particular, the ones like virtual BIOS calling
> memory_region_init_ram_guest_memfd() do not.
>
> To manage the RAMBlocks with guest_memfd, define a new object named
> MemoryAttributeManager to implement the RamDiscardManager
> interface. The

 Isn't this should be the other way around. 'MemoryAttributeManager'
 should be an interface and RamDiscardManager a type of it, an
 implementation?
>>>
>>> We want to use 'MemoryAttributeManager' to represent RAMBlock to
>>> implement the RamDiscardManager interface callbacks because RAMBlock is
>>> not an object. It includes some metadata of guest_memfd like
>>> shared_bitmap at the same time.
>>>
>>> I can't get it that make 'MemoryAttributeManager' an interface and
>>> RamDiscardManager a type of it. Can you elaborate it a little bit? I
>>> think at least we need someone to implement the RamDiscardManager
>>> interface.
>>
>> shared <-> private is translated (abstracted) to "populated <->
>> discarded", which makes sense. The other way around would be wrong.
>>
>> It's going to be interesting once we have more logical states, for
>> example supporting virtio-mem for confidential VMs.
>>
>> Then we'd have "shared+populated, private+populated, shared+discard,
>> private+discarded". Not sure if this could simply be achieved by
>> allowing multiple RamDiscardManager that are effectively chained, or
>> if we'd want a different interface.
> 
> Exactly! In any case generic manager (parent class) would make more
> sense that can work on different operations/states implemented in child
> classes (can be chained as well).

Ah, we are talking about the generic state management. Sorry for my slow
reaction.

So we need to
1. Define a generic manager Interface, e.g.
MemoryStateManager/GenericStateManager.
2. Make RamDiscardManager the child of MemoryStateManager which manages
the state of populated and discarded.
3. Define a new child manager Interface PrivateSharedManager which
manages the state of private and shared.
4. Define a new object ConfidentialMemoryAttribute to implement the
PrivateSharedManager interface.
(Welcome to rename the above Interface/Object)

Is my understanding correct?

> 
> Best regards,
> Pankaj
>>
> 

Re: [PATCH 12/17] rust/vmstate: Support version field in vmstate macros

[Zhao Liu]

On Mon, Mar 17, 2025 at 05:38:10PM +0100, Paolo Bonzini wrote:
> Date: Mon, 17 Mar 2025 17:38:10 +0100
> From: Paolo Bonzini 
> Subject: Re: [PATCH 12/17] rust/vmstate: Support version field in vmstate
>  macros
> 
> On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu  wrote:
> > Add "version = *" in vmstate macros to help set version_id in
> > VMStateField.
> 
> Could it use a ".with_min_version(2)" annotation (or something similar) 
> instead?
> 

Ah, thanks! I didn't realize there was already a with_version_id() and
didn't understand your design intent, so

vmstate_of!(FooC, ptr).with_version_id(2),

would have been good enough! There is no need to add this `version` field.

Regards,
Zhao

Re: [PATCH v3 0/7] Move memory listener register to vhost_vdpa_init

[Jason Wang]

On Tue, Mar 18, 2025 at 9:55 AM Lei Yang  wrote:
>
> Hi Jonah
>
> I tested this series with the vhost_vdpa device based on mellanox
> ConnectX-6 DX nic and hit the host kernel crash. This problem can be
> easier to reproduce under the hotplug/unplug device scenario.
> For the core dump messages please review the attachment.
> FW version:
> #  flint -d :0d:00.0 q |grep Version
> FW Version:22.44.1036
> Product Version:   22.44.1036

The trace looks more like a mlx5e driver bug other than vDPA?

[ 3256.256707] Call Trace:
[ 3256.256708]  
[ 3256.256709]  ? show_trace_log_lvl+0x1c4/0x2df
[ 3256.256714]  ? show_trace_log_lvl+0x1c4/0x2df
[ 3256.256715]  ? __build_skb+0x4a/0x60
[ 3256.256719]  ? __die_body.cold+0x8/0xd
[ 3256.256720]  ? die_addr+0x39/0x60
[ 3256.256725]  ? exc_general_protection+0x1ec/0x420
[ 3256.256729]  ? asm_exc_general_protection+0x22/0x30
[ 3256.256736]  ? __build_skb_around+0x8c/0xf0
[ 3256.256738]  __build_skb+0x4a/0x60
[ 3256.256740]  build_skb+0x11/0xa0
[ 3256.256743]  mlx5e_skb_from_cqe_mpwrq_linear+0x156/0x280 [mlx5_core]
[ 3256.256872]  mlx5e_handle_rx_cqe_mpwrq_rep+0xcb/0x1e0 [mlx5_core]
[ 3256.256964]  mlx5e_rx_cq_process_basic_cqe_comp+0x39f/0x3c0 [mlx5_core]
[ 3256.257053]  mlx5e_poll_rx_cq+0x3a/0xc0 [mlx5_core]
[ 3256.257139]  mlx5e_napi_poll+0xe2/0x710 [mlx5_core]
[ 3256.257226]  __napi_poll+0x29/0x170
[ 3256.257229]  net_rx_action+0x29c/0x370
[ 3256.257231]  handle_softirqs+0xce/0x270
[ 3256.257236]  __irq_exit_rcu+0xa3/0xc0
[ 3256.257238]  common_interrupt+0x80/0xa0

Which kernel tree did you use? Can you please try net.git?

Thanks

>
> Best Regards
> Lei
>
> On Fri, Mar 14, 2025 at 9:04 PM Jonah Palmer  wrote:
> >
> > Current memory operations like pinning may take a lot of time at the
> > destination.  Currently they are done after the source of the migration is
> > stopped, and before the workload is resumed at the destination.  This is a
> > period where neigher traffic can flow, nor the VM workload can continue
> > (downtime).
> >
> > We can do better as we know the memory layout of the guest RAM at the
> > destination from the moment that all devices are initializaed.  So
> > moving that operation allows QEMU to communicate the kernel the maps
> > while the workload is still running in the source, so Linux can start
> > mapping them.
> >
> > As a small drawback, there is a time in the initialization where QEMU
> > cannot respond to QMP etc.  By some testing, this time is about
> > 0.2seconds.  This may be further reduced (or increased) depending on the
> > vdpa driver and the platform hardware, and it is dominated by the cost
> > of memory pinning.
> >
> > This matches the time that we move out of the called downtime window.
> > The downtime is measured as checking the trace timestamp from the moment
> > the source suspend the device to the moment the destination starts the
> > eight and last virtqueue pair.  For a 39G guest, it goes from ~2.2526
> > secs to 2.0949.
> >
> > Future directions on top of this series may include to move more things 
> > ahead
> > of the migration time, like set DRIVER_OK or perform actual iterative 
> > migration
> > of virtio-net devices.
> >
> > Comments are welcome.
> >
> > This series is a different approach of series [1]. As the title does not
> > reflect the changes anymore, please refer to the previous one to know the
> > series history.
> >
> > This series is based on [2], it must be applied after it.
> >
> > [Jonah Palmer]
> > This series was rebased after [3] was pulled in, as [3] was a prerequisite
> > fix for this series.
> >
> > v3:
> > ---
> > * Rebase
> >
> > v2:
> > ---
> > * Move the memory listener registration to vhost_vdpa_set_owner function.
> > * Move the iova_tree allocation to net_vhost_vdpa_init.
> >
> > v1 at https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg02136.html.
> >
> > [1] 
> > https://patchwork.kernel.org/project/qemu-devel/cover/20231215172830.2540987-1-epere...@redhat.com/
> > [2] https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg05910.html
> > [3] 
> > https://lore.kernel.org/qemu-devel/20250217144936.3589907-1-jonah.pal...@oracle.com/
> >
> > Eugenio Pérez (7):
> >   vdpa: check for iova tree initialized at net_client_start
> >   vdpa: reorder vhost_vdpa_set_backend_cap
> >   vdpa: set backend capabilities at vhost_vdpa_init
> >   vdpa: add listener_registered
> >   vdpa: reorder listener assignment
> >   vdpa: move iova_tree allocation to net_vhost_vdpa_init
> >   vdpa: move memory listener register to vhost_vdpa_init
> >
> >  hw/virtio/vhost-vdpa.c | 98 ++
> >  include/hw/virtio/vhost-vdpa.h | 22 +++-
> >  net/vhost-vdpa.c   | 34 ++--
> >  3 files changed, 88 insertions(+), 66 deletions(-)
> >
> > --
> > 2.43.5
> >
> >

[PATCH v2] tests/functional/test_arm_orangepi: rename test class to 'OrangePiMachine'

[Niek Linnenbank]

The test class in this file contains all functional test cases
for testing the Orange Pi PC board. It should be given a name
matching the Qemu machine it covers.

This commit sets the test class name to 'OrangePiMachine'.

Fixes: 380f7268b7b ("tests/functional: Convert the OrangePi tests to the 
functional framework")
Reviewed-by: Thomas Huth 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Niek Linnenbank 
---
 tests/functional/test_arm_orangepi.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/functional/test_arm_orangepi.py 
b/tests/functional/test_arm_orangepi.py
index 18ee50216b..28919391e5 100755
--- a/tests/functional/test_arm_orangepi.py
+++ b/tests/functional/test_arm_orangepi.py
@@ -14,7 +14,7 @@
 from qemu_test.utils import image_pow2ceil_expand
 
 
-class BananaPiMachine(LinuxKernelTest):
+class OrangePiMachine(LinuxKernelTest):
 
 ASSET_DEB = Asset(
 ('https://apt.armbian.com/pool/main/l/linux-6.6.16/'
-- 
2.43.0

RE: [PATCH 16/39] target/hexagon: Implement hex_tlb_lookup_by_asid()

[ltaylorsimpson]

> -Original Message-
> From: Brian Cain 
> Sent: Friday, February 28, 2025 11:28 PM
> To: qemu-devel@nongnu.org
> Cc: brian.c...@oss.qualcomm.com; richard.hender...@linaro.org;
> phi...@linaro.org; quic_mathb...@quicinc.com; a...@rev.ng; a...@rev.ng;
> quic_mlie...@quicinc.com; ltaylorsimp...@gmail.com;
> alex.ben...@linaro.org; quic_mbur...@quicinc.com;
> sidn...@quicinc.com; Brian Cain 
> Subject: [PATCH 16/39] target/hexagon: Implement
> hex_tlb_lookup_by_asid()
> 
> From: Brian Cain 
> 
> Signed-off-by: Brian Cain 

Reviewed-by: Taylor Simpson 

Re: [PATCH 14/17] rust/vmstate: Add unit test for vmstate_of macro

[Zhao Liu]

On Mon, Mar 17, 2025 at 06:11:35PM +0100, Paolo Bonzini wrote:
> Date: Mon, 17 Mar 2025 18:11:35 +0100
> From: Paolo Bonzini 
> Subject: Re: [PATCH 14/17] rust/vmstate: Add unit test for vmstate_of macro
> 
> Thanks very much for the tests!
> 
> On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu  wrote:
> > -pub use crate::bindings::{VMStateDescription, VMStateField};
> > -use crate::{
> > -bindings::VMStateFlags, callbacks::FnCall, prelude::*, qom::Owned, 
> > zeroable::Zeroable,
> > -};
> > +pub use crate::bindings::{VMStateDescription, VMStateField, VMStateFlags};
> 
> Does VMStateFlags have to be part of the public API?

I can do `use qemu_api::bindings::VMStateFlags` in vmstate_test.rs
directly, which is better since it's the raw value of VMStateField that
I need to check!

> > +assert_eq!(foo_fields[0].info, unsafe {
> > +::core::ptr::addr_of!(vmstate_info_int8)
> > +});
> 
> You can use & instead of addr_of here.

Thanks! Will fix.

Regards,
Zhao

Re: [PATCH 00/17] rust/vmstate: Clean up, fix, enhance & test

[Zhao Liu]

On Mon, Mar 17, 2025 at 06:20:15PM +0100, Paolo Bonzini wrote:
> Date: Mon, 17 Mar 2025 18:20:15 +0100
> From: Paolo Bonzini 
> Subject: Re: [PATCH 00/17] rust/vmstate: Clean up, fix, enhance & test
> 
> On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu  wrote:
> > Hi,
> >
> > This series is in preparation for HPET migration support (in particular,
> > to support varray and vmstate_validate), and it also cleans up and fixes
> > the current vmstate! However, there is still the gap from being a ‘stable’
> > vmstate.
> 
> Already a great improvement!
> 
> I quickly went through things that I was planning to do in a slightly
> different way, but overall this is great work.  Give me a day or two
> to finish reviewing and testing, most of it can already be applied to
> qemu-rust or even to 10.0.

Thanks for your quick feedback! I think the main open now is about the
best way to implement `field_exists`... pls see my patch 13 reply.

I can quickly refresh v2 to help you apply!

Regards,
Zhao

Re: Building QEMU as a Shared Library

[Saanjh Sengupta]

Hi Alex,

You mentioned about a patch series; do you have it handy with you?

If so, could you please direct me to the same ?

On 14 Mar 2025, at 12:11 AM, Alex Bennée  wrote:

Saanjh Sengupta mailto:saanjhsengu...@outlook.com>> 
writes:

Hi,

What we are trying to achieve is that the QEMU should run for a particular 
number of instructions, let's say for example
1 instructions and then pause it's emulation. After a resume trigger is 
received to the QEMU it must resume it's
emulation and start the instruction count from 10001 (which basically
means that the context should be saved).

I think you want to run under icount and you will need to modify the
trigger plugin. Under icount we run each vCPU in turn, so if the plugin
pauses the vCPU will de-facto be paused.

You would have to implement some sort of control interface in the
plugin. Or you could add an API to trigger the gdbstub. I think I had
that on a patch series at one point.


In the previous mail when you mentioned g_usleep, I believe this shall not work 
(as per our use-case) since it will reset the
instruction count to 0 (as per what you mentioned).

To achieve the use-case, do you have any leads/suggestions ?

Regards
Saanjh Sengupta

-
From: Pierrick Bouvier 
Sent: Wednesday, March 12, 2025 11:50:23 am
To: Saanjh Sengupta ; Philippe Mathieu-Daudé 
; Paolo Bonzini
; Marc-André Lureau 
Cc: amir.gon...@neuroblade.ai ; 
qemu-devel@nongnu.org ; Alex
Bennée 
Subject: Re: Building QEMU as a Shared Library

On 3/11/25 21:31, Saanjh Sengupta wrote:


Hi,

Thank you for the clarification. Regarding the last time
/"Stoptrigger might be a better fit for what you want to do, and instead
of exiting, you want to resume emulation after N insn. The function
qemu_clock_advance_virtual_time() can only be used to move the time
forward, and you can not stop the "virtual time" by design."/
/
/
I did not quite understand this. Even if I have to modify the
stopTrigger plugin, I would want it to pause rather than exiting.
For example: It gets 1 instructions executed after that it should
pause and after some time it should then resume again execute till 2
instructions (because previously it executed till 1 and then it must
execute till 2). How do I do this? How do I state the code to pause
the qemu's emulation after 1 instructions?


By using g_usleep to pause the current cpu.
As well, it's needed to reset insn_count to 0 to count instructions again.

With this command line:
./build/qemu-system-x86_64 -plugin
./build/contrib/plugins/libstoptrigger.so,icount=1000 -d plugin

And with those changes to stoptrigger:

diff --git a/contrib/plugins/stoptrigger.c b/contrib/plugins/stoptrigger.c
index b3a6ed66a7b..77fd413cef1 100644
--- a/contrib/plugins/stoptrigger.c
+++ b/contrib/plugins/stoptrigger.c
@@ -41,11 +41,12 @@ typedef struct {
 int exit_code;
 } ExitInfo;

-static void exit_emulation(int return_code, char *message)
+static void pause_emulation(int return_code, char *message)
 {
 qemu_plugin_outs(message);
 g_free(message);
-exit(return_code);
+/* exit(return_code); */
+g_usleep(1 * G_USEC_PER_SEC);
 }

 static void exit_icount_reached(unsigned int cpu_index, void *udata)
@@ -53,7 +54,9 @@ static void exit_icount_reached(unsigned int
cpu_index, void *udata)
 uint64_t insn_vaddr = qemu_plugin_u64_get(current_pc, cpu_index);
 char *msg = g_strdup_printf("icount reached at 0x%" PRIx64 ",
exiting\n",
 insn_vaddr);
-exit_emulation(icount_exit_code, msg);
+pause_emulation(icount_exit_code, msg);
+/* reset instruction counter */
+qemu_plugin_u64_set(insn_count, cpu_index, 0);
 }

 static void exit_address_reached(unsigned int cpu_index, void *udata)
@@ -61,7 +64,7 @@ static void exit_address_reached(unsigned int
cpu_index, void *udata)
 ExitInfo *ei = udata;
 g_assert(ei);
 char *msg = g_strdup_printf("0x%" PRIx64 " reached, exiting\n",
ei->exit_addr);
-exit_emulation(ei->exit_code, msg);
+pause_emulation(ei->exit_code, msg);
 }

Moreover, I tried an activity where I was utilising the QMP protocol to
control the virtual time (with respect to the IPS plugin). In that
context when the QMP stop is triggered, my virtual time does got freezed
until the resume is triggered. Does this mean I am able to manipulate
the virtual time of the QEMU?


I am not sure of how it works, but the plugin interface only allows to
move time forward.



Regards
Saanjh Sengupta

*From:* Pierrick Bouvier 
*Sent:* Wednesday, March 12, 2025 2:14:47 AM
*To:* Saanjh Sengupta ; Philippe Mathieu-
Daudé ; Paolo Bonzini ; Marc-
André Lureau 
*Cc:* amir.gon...@neuroblade.ai ; qemu-
de...@nongnu.org ; Alex Bennée

*Subject:* Re: Building QEMU as a Shared Library
On 3/11/25 02:50, Saanjh Sengupta wro

[PATCH] docs/system: Use the meson binary from the pyvenv

[Thomas Huth]

From: Thomas Huth 

To avoid problems with the meson installation from the host
system, we should always use the meson from our venv instead.
Thus use this in the documentation, too.

While we're at it, also mention that it has to be run from
the build folder (in the igb.rst file; the other two files
were already fine).

Suggested-by: Niek Linnenbank 
Signed-off-by: Thomas Huth 
---
 docs/system/arm/bananapi_m2u.rst | 2 +-
 docs/system/arm/orangepi.rst | 2 +-
 docs/system/devices/igb.rst  | 5 +++--
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/docs/system/arm/bananapi_m2u.rst b/docs/system/arm/bananapi_m2u.rst
index d30db8d04c3..6efa222c16f 100644
--- a/docs/system/arm/bananapi_m2u.rst
+++ b/docs/system/arm/bananapi_m2u.rst
@@ -136,4 +136,4 @@ provide the following command:
 
   $ cd qemu-build-dir
   $ QEMU_TEST_ALLOW_LARGE_STORAGE=1 \
-meson test --suite thorough func-arm-arm_bpim2u
+pyvenv/bin/meson test --suite thorough func-arm-arm_bpim2u
diff --git a/docs/system/arm/orangepi.rst b/docs/system/arm/orangepi.rst
index 8b9448ca7b0..716062fca9c 100644
--- a/docs/system/arm/orangepi.rst
+++ b/docs/system/arm/orangepi.rst
@@ -262,4 +262,4 @@ provide the following command from the build directory:
 .. code-block:: bash
 
   $ QEMU_TEST_ALLOW_LARGE_STORAGE=1 \
-meson test --suite thorough func-arm-arm_orangepi
+pyvenv/bin/meson test --suite thorough func-arm-arm_orangepi
diff --git a/docs/system/devices/igb.rst b/docs/system/devices/igb.rst
index 9145af5c757..71f31cb1160 100644
--- a/docs/system/devices/igb.rst
+++ b/docs/system/devices/igb.rst
@@ -57,11 +57,12 @@ directory:
   meson test qtest-x86_64/qos-test
 
 ethtool can test register accesses, interrupts, etc. It is automated as an
-functional test and can be ran with the following command:
+functional test and can be run from the build directory with the following
+command:
 
 .. code:: shell
 
-  meson test --suite thorough func-x86_64-netdev_ethtool
+  pyvenv/bin/meson test --suite thorough func-x86_64-netdev_ethtool
 
 References
 ==
-- 
2.48.1

Re: [PATCH v2] microvm: do not use the lastest cpu version

[Ani Sinha]

On Wed, Mar 5, 2025 at 7:56 PM Ani Sinha  wrote:
>
> On Wed, Mar 5, 2025 at 7:12 PM Stefan Hajnoczi  wrote:
> >
> > On Wed, Mar 05, 2025 at 01:24:25PM +0530, Ani Sinha wrote:
> > > On Sat, Mar 1, 2025 at 9:04 PM Ani Sinha  wrote:
> > > >
> > > > On Thu, Feb 20, 2025 at 12:36 PM Zhao Liu  wrote:
> > > > >
> > > > > On Thu, Feb 20, 2025 at 12:23:26PM +0530, Ani Sinha wrote:
> > > > > > Date: Thu, 20 Feb 2025 12:23:26 +0530
> > > > > > From: Ani Sinha 
> > > > > > Subject: [PATCH v2] microvm: do not use the lastest cpu version
> > > > > > X-Mailer: git-send-email 2.45.2
> > > > > >
> > > > > > commit 0788a56bd1ae3 ("i386: Make unversioned CPU models be 
> > > > > > aliases")
> > > > > > introduced 'default_cpu_version' for PCMachineClass. This created 
> > > > > > three
> > > > > > categories of CPU models:
> > > > > >  - Most unversioned CPU models would use version 1 by default.
> > > > > >  - For machines 4.0.1 and older that do not support cpu model 
> > > > > > aliases, a
> > > > > >special default_cpu_version value of CPU_VERSION_LEGACY is used.
> > > > > >  - It was thought that future machines would use the latest value 
> > > > > > of cpu
> > > > > >versions corresponding to default_cpu_version value of
> > > > > >CPU_VERSION_LATEST [1].
> > > > > >
> > > > > > All pc machines still use the default cpu version of 1 for
> > > > > > unversioned cpu models. CPU_VERSION_LATEST is a moving target and
> > > > > > changes with time. Therefore, if machines use CPU_VERSION_LATEST, 
> > > > > > it would
> > > > > > mean that over a period of time, for the same versioned machine 
> > > > > > type,
> > > > > > the cpu version would be different depending on what the latest was 
> > > > > > at that
> > > > > > time. This would break guests even when they use a constant specific
> > > > > > versioned machine type.
> > > > > > Additionally, microvm machines are not versioned anyway and 
> > > > > > therefore
> > > > > > there is no requirement to use the latest cpu model by default.
> > > > > > Let microvms use the non-versioned cpu model and remove all 
> > > > > > references
> > > > > > to CPU_VERSION_LATEST as there are no other users (nor we anticipate
> > > > > > future consumers of CPU_VERSION_LATEST).
> > > > > >
> > > > > > Those users who need spefific cpu versions can use explicit version 
> > > > > > in
> > > > > > the QEMU command line to select the specific cpu version desired.
> > > > > >
> > > > > > CI pipline does not break with this change.
> > > > > >
> > > > > > 1) See commit dcafd1ef0af227 ("i386: Register versioned CPU models")
> > > > > >
> > > > > > CC: imamm...@redhat.com
> > > > > > CC: zhao1@intel.com
> > > > > > Reviewed-by: Igor Mammedov 
> > > > > > Reviewed-by: Sergio Lopez 
> > > > > > Signed-off-by: Ani Sinha 
> > > > > > ---
> > > > > >  hw/i386/microvm.c |  2 +-
> > > > > >  target/i386/cpu.c | 15 ---
> > > > > >  target/i386/cpu.h |  4 
> > > > > >  3 files changed, 1 insertion(+), 20 deletions(-)
> > > > > >
> > > > > > changelog:
> > > > > > v2: tags added, more explanation in the commit log.
> > > > >
> > > > > Reviewed-by: Zhao Liu 
> > > > >
> > > >
> > > > Who is picking this up?
> > >
> > > I sent a pull request for this and a couple other reviewed patches
> > > myself. Two reasons:
> > > - wanted to see this in the upstream sooner as some other bits of the
> > > work is pending on it.
> > > - I never sent a pull request before and wanted to go through the
> > > process to learn how to do it in case I needed it in the future.
> > >
> > > i hope the PR is ok. If not, I can resend after corrections. I used
> > > Peter's script 
> > > https://git.linaro.org/people/peter.maydell/misc-scripts.git/plain/make-pullreq
> >
> > This should go via Paolo's tree. I have pinged him to remind him of your
> > patches.
> >
> > Please only send pull requests for subsystems where you are listed as
> > the maintainer in the MAINTAINERS file.
> >
> > It doesn't scale when people send me PRs directly because I need to do a
> > bunch of extra sanity checking and helping people get their one-off PRs
> > properly signed and formatted. I also don't like to bypass
> > sub-maintainers because I'm less qualified to do the final review than
> > the sub-maintainers themselves.
>
> Fair enough! I hope the three patches get merged soon.

This has still not merged! Unfortunate.

[PATCH] docs/system/arm: Use "functional tests" instead of "integration tests"

[Thomas Huth]

From: Thomas Huth 

We don't use the term "integration tests" for these kind of tests
anymore, it's "functional tests" nowadays.

Suggested-by: Niek Linnenbank 
Signed-off-by: Thomas Huth 
---
 docs/system/arm/bananapi_m2u.rst | 6 +++---
 docs/system/arm/orangepi.rst | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/docs/system/arm/bananapi_m2u.rst b/docs/system/arm/bananapi_m2u.rst
index 6efa222c16f..03cc5618c38 100644
--- a/docs/system/arm/bananapi_m2u.rst
+++ b/docs/system/arm/bananapi_m2u.rst
@@ -125,10 +125,10 @@ And then boot it.
 
   $ qemu-system-arm -M bpim2u -nographic -sd sd.img
 
-Banana Pi M2U integration tests
-"""
+Banana Pi M2U functional tests
+""
 
-The Banana Pi M2U machine has several integration tests included.
+The Banana Pi M2U machine has several functional tests included.
 To run the whole set of tests, build QEMU from source and simply
 provide the following command:
 
diff --git a/docs/system/arm/orangepi.rst b/docs/system/arm/orangepi.rst
index 716062fca9c..d81f6c3bfd2 100644
--- a/docs/system/arm/orangepi.rst
+++ b/docs/system/arm/orangepi.rst
@@ -252,10 +252,10 @@ and set the following environment variables before 
booting:
 Optionally you may save the environment variables to SD card with 'saveenv'.
 To continue booting simply give the 'boot' command and NetBSD boots.
 
-Orange Pi PC integration tests
-""
+Orange Pi PC functional tests
+"
 
-The Orange Pi PC machine has several integration tests included.
+The Orange Pi PC machine has several functional tests included.
 To run the whole set of tests, build QEMU from source and simply
 provide the following command from the build directory:
 
-- 
2.48.1

[BUG][powerpc] KVM Guest Boot Failure and Hang at "Booting Linux via __start()"

[misanjum]

Bug Description:
Encountering a boot failure when launching a KVM guest with 
'qemu-system-ppc64'. The guest hangs at boot, and the QEMU monitor 
crashes.



Reproduction Steps:
# qemu-system-ppc64 --version
QEMU emulator version 9.2.50 (v9.2.0-2799-g0462a32b4f)
Copyright (c) 2003-2025 Fabrice Bellard and the QEMU Project developers

# /usr/bin/qemu-system-ppc64 -name avocado-vt-vm1 -machine 
pseries,accel=kvm \

  -m 32768 -smp 32,sockets=1,cores=32,threads=1 -nographic \
  -device virtio-scsi-pci,id=scsi \
  -drive 
file=/home/kvmci/tests/data/avocado-vt/images/rhel8.0devel-ppc64le.qcow2,if=none,id=drive0,format=qcow2 
\

  -device scsi-hd,drive=drive0,bus=scsi.0 \
  -netdev bridge,id=net0,br=virbr0 \
  -device virtio-net-pci,netdev=net0 \
  -serial pty \
  -device virtio-balloon-pci \
  -cpu host
QEMU 9.2.50 monitor - type 'help' for more information
char device redirected to /dev/pts/2 (label serial0)
(qemu)
(qemu) qemu-system-ppc64: warning: kernel_irqchip allowed but 
unavailable: IRQ_XIVE capability must be present for KVM

Falling back to kernel-irqchip=off
** Qemu Hang

(In another ssh session)
# screen /dev/pts/2
Preparing to boot Linux version 6.10.4-200.fc40.ppc64le 
(mockbuild@c23cc4e677614c34bb22d54eeea4dc1f) (gcc (GCC) 14.2.1 20240801 
(Red Hat 14.2.1-1), GNU ld version 2.41-37.fc40) #1 SMP Sun Aug 11 
15:20:17 UTC 2024

Detected machine type: 0101
command line: 
BOOT_IMAGE=(ieee1275/disk,msdos2)/vmlinuz-6.10.4-200.fc40.ppc64le 
root=/dev/mapper/fedora-root ro rd.lvm.lv=fedora/root crashkernel=1024M

Max number of cores passed to firmware: 2048 (NR_CPUS = 2048)
Calling ibm,client-architecture-support... done
memory layout at init:
  memory_limit :  (16 MB aligned)
  alloc_bottom : 0820
  alloc_top: 3000
  alloc_top_hi : 0008
  rmo_top  : 3000
  ram_top  : 0008
instantiating rtas at 0x2fff... done
prom_hold_cpus: skipped
copying OF device tree...
Building dt strings...
Building dt structure...
Device tree strings 0x0821 -> 0x08210bd0
Device tree struct  0x0822 -> 0x0823
Quiescing Open Firmware ...
Booting Linux via __start() @ 0x0044 ...
** Guest Console Hang


Git Bisect:
Performing git bisect points to the following patch:
# git bisect bad
e8291ec16da80566c121c68d9112be458954d90b is the first bad commit
commit e8291ec16da80566c121c68d9112be458954d90b (HEAD)
Author: Nicholas Piggin 
Date:   Thu Dec 19 13:40:31 2024 +1000

target/ppc: fix timebase register reset state

(H)DEC and PURR get reset before icount does, which causes them to 
be

skewed and not match the init state. This can cause replay to not
match the recorded trace exactly. For DEC and HDEC this is usually 
not

noticable since they tend to get programmed before affecting the
target machine. PURR has been observed to cause replay bugs when
running Linux.

Fix this by resetting using a time of 0.

Message-ID: <20241219034035.1826173-2-npig...@gmail.com>
Signed-off-by: Nicholas Piggin 

 hw/ppc/ppc.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)


Reverting the patch helps boot the guest.
Thanks,
Misbah Anjum N

Re: [BUG][powerpc] KVM Guest Boot Failure – Hangs at "Booting Linux via __start()”

[Nicholas Piggin]

Thanks for the report.

Tricky problem. A secondary CPU is hanging before it is started by the
primary via rtas call.

That secondary keeps calling kvm_cpu_exec(), which keeps exiting out
early with EXCP_HLT because kvm_arch_process_async_events() returns
true because that cpu has ->halted=1. That just goes around he run
loop because there is an interrupt pending (DEC).

So it never runs. It also never releases the BQL, and another CPU,
the primary which is actually supposed to be running, is stuck in
spapr_set_all_lpcrs() in run_on_cpu() waiting for the BQL.

This patch just exposes the bug I think, by causing the interrupt.
although I'm not quite sure why it's okay previously (-ve decrementer
values should be causing a timer exception too). The timer exception
should not be taken as an interrupt by those secondary CPUs, and it
doesn't because it is masked, until set_all_lpcrs sets an LPCR value
that enables powersave wakeup on decrementer interrupt.

The start_powered_off sate just sets ->halted, which makes it look
like a powersaving state. Logically I think it's not the same thing
as far as spapr goes. I don't know why start_powered_off only sets
->halted, and not ->stop/stopped as well.

Not sure how best to solve it cleanly. I'll send a revert if I can't
get something working soon.

Thanks,
Nick

On Tue Mar 18, 2025 at 7:09 AM AEST, misanjum wrote:
> Bug Description:
> Encountering a boot failure when launching a KVM guest with 
> qemu-system-ppc64. The guest hangs at boot, and the QEMU monitor 
> crashes.
>
>
> Reproduction Steps:
> # qemu-system-ppc64 --version
> QEMU emulator version 9.2.50 (v9.2.0-2799-g0462a32b4f)
> Copyright (c) 2003-2025 Fabrice Bellard and the QEMU Project developers
>
> # /usr/bin/qemu-system-ppc64 -name avocado-vt-vm1 -machine 
> pseries,accel=kvm \
>-m 32768 -smp 32,sockets=1,cores=32,threads=1 -nographic \
>-device virtio-scsi-pci,id=scsi \
>-drive 
> file=/home/kvmci/tests/data/avocado-vt/images/rhel8.0devel-ppc64le.qcow2,if=none,id=drive0,format=qcow2
>  
> \
>-device scsi-hd,drive=drive0,bus=scsi.0 \
>-netdev bridge,id=net0,br=virbr0 \
>-device virtio-net-pci,netdev=net0 \
>-serial pty \
>-device virtio-balloon-pci \
>-cpu host
> QEMU 9.2.50 monitor - type 'help' for more information
> char device redirected to /dev/pts/2 (label serial0)
> (qemu)
> (qemu) qemu-system-ppc64: warning: kernel_irqchip allowed but 
> unavailable: IRQ_XIVE capability must be present for KVM
> Falling back to kernel-irqchip=off
> ** Qemu Hang
>
> (In another ssh session)
> # screen /dev/pts/2
> Preparing to boot Linux version 6.10.4-200.fc40.ppc64le 
> (mockbuild@c23cc4e677614c34bb22d54eeea4dc1f) (gcc (GCC) 14.2.1 20240801 
> (Red Hat 14.2.1-1), GNU ld version 2.41-37.fc40) #1 SMP Sun Aug 11 
> 15:20:17 UTC 2024
> Detected machine type: 0101
> command line: 
> BOOT_IMAGE=(ieee1275/disk,msdos2)/vmlinuz-6.10.4-200.fc40.ppc64le 
> root=/dev/mapper/fedora-root ro rd.lvm.lv=fedora/root crashkernel=1024M
> Max number of cores passed to firmware: 2048 (NR_CPUS = 2048)
> Calling ibm,client-architecture-support... done
> memory layout at init:
>memory_limit :  (16 MB aligned)
>alloc_bottom : 0820
>alloc_top: 3000
>alloc_top_hi : 0008
>rmo_top  : 3000
>ram_top  : 0008
> instantiating rtas at 0x2fff... done
> prom_hold_cpus: skipped
> copying OF device tree...
> Building dt strings...
> Building dt structure...
> Device tree strings 0x0821 -> 0x08210bd0
> Device tree struct  0x0822 -> 0x0823
> Quiescing Open Firmware ...
> Booting Linux via __start() @ 0x0044 ...
> ** Guest Console Hang
>
>
> Git Bisect:
> Performing git bisect points to the following patch:
> # git bisect bad
> e8291ec16da80566c121c68d9112be458954d90b is the first bad commit
> commit e8291ec16da80566c121c68d9112be458954d90b (HEAD)
> Author: Nicholas Piggin 
> Date:   Thu Dec 19 13:40:31 2024 +1000
>
>  target/ppc: fix timebase register reset state
>
>  (H)DEC and PURR get reset before icount does, which causes them to 
> be
>  skewed and not match the init state. This can cause replay to not
>  match the recorded trace exactly. For DEC and HDEC this is usually 
> not
>  noticable since they tend to get programmed before affecting the
>  target machine. PURR has been observed to cause replay bugs when
>  running Linux.
>
>  Fix this by resetting using a time of 0.
>
>  Message-ID: <20241219034035.1826173-2-npig...@gmail.com>
>  Signed-off-by: Nicholas Piggin 
>
>   hw/ppc/ppc.c | 11 ---
>   1 file changed, 8 insertions(+), 3 deletions(-)
>
>
> Reverting the patch helps boot the guest.
> Thanks,
> Misbah Anjum N

[PATCH v6 03/18] exec/memory_ldst: extract memory_ldst declarations from cpu-all.h

[Pierrick Bouvier]

They are now accessible through exec/memory.h instead, and we make sure
all variants are available for common or target dependent code.

Reviewed-by: Richard Henderson 
Signed-off-by: Pierrick Bouvier 
---
 include/exec/cpu-all.h | 12 
 include/exec/memory_ldst.h.inc |  4 
 2 files changed, 16 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index e56c064d46f..0e8205818a4 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -44,18 +44,6 @@
 
 #include "exec/hwaddr.h"
 
-#define SUFFIX
-#define ARG1 as
-#define ARG1_DECLAddressSpace *as
-#define TARGET_ENDIANNESS
-#include "exec/memory_ldst.h.inc"
-
-#define SUFFIX   _cached_slow
-#define ARG1 cache
-#define ARG1_DECLMemoryRegionCache *cache
-#define TARGET_ENDIANNESS
-#include "exec/memory_ldst.h.inc"
-
 static inline void stl_phys_notdirty(AddressSpace *as, hwaddr addr, uint32_t 
val)
 {
 address_space_stl_notdirty(as, addr, val,
diff --git a/include/exec/memory_ldst.h.inc b/include/exec/memory_ldst.h.inc
index 92ad74e9560..7270235c600 100644
--- a/include/exec/memory_ldst.h.inc
+++ b/include/exec/memory_ldst.h.inc
@@ -19,7 +19,6 @@
  * License along with this library; if not, see .
  */
 
-#ifdef TARGET_ENDIANNESS
 uint16_t glue(address_space_lduw, SUFFIX)(ARG1_DECL,
 hwaddr addr, MemTxAttrs attrs, MemTxResult *result);
 uint32_t glue(address_space_ldl, SUFFIX)(ARG1_DECL,
@@ -34,7 +33,6 @@ void glue(address_space_stl, SUFFIX)(ARG1_DECL,
 hwaddr addr, uint32_t val, MemTxAttrs attrs, MemTxResult *result);
 void glue(address_space_stq, SUFFIX)(ARG1_DECL,
 hwaddr addr, uint64_t val, MemTxAttrs attrs, MemTxResult *result);
-#else
 uint8_t glue(address_space_ldub, SUFFIX)(ARG1_DECL,
 hwaddr addr, MemTxAttrs attrs, MemTxResult *result);
 uint16_t glue(address_space_lduw_le, SUFFIX)(ARG1_DECL,
@@ -63,9 +61,7 @@ void glue(address_space_stq_le, SUFFIX)(ARG1_DECL,
 hwaddr addr, uint64_t val, MemTxAttrs attrs, MemTxResult *result);
 void glue(address_space_stq_be, SUFFIX)(ARG1_DECL,
 hwaddr addr, uint64_t val, MemTxAttrs attrs, MemTxResult *result);
-#endif
 
 #undef ARG1_DECL
 #undef ARG1
 #undef SUFFIX
-#undef TARGET_ENDIANNESS
-- 
2.39.5

[PATCH 08/13] target/arm/cpu: flags2 is always uint64_t

[Pierrick Bouvier]

Do not rely on target dependent type, but use a fixed type instead.
Since the original type is unsigned, it should be safe to extend its
size without any side effect.

Signed-off-by: Pierrick Bouvier 
---
 target/arm/cpu.h| 2 +-
 target/arm/tcg/hflags.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 96f7801a239..27a0d4550f2 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -194,7 +194,7 @@ typedef struct ARMPACKey {
 /* See the commentary above the TBFLAG field definitions.  */
 typedef struct CPUARMTBFlags {
 uint32_t flags;
-target_ulong flags2;
+uint64_t flags2;
 } CPUARMTBFlags;
 
 typedef struct ARMMMUFaultInfo ARMMMUFaultInfo;
diff --git a/target/arm/tcg/hflags.c b/target/arm/tcg/hflags.c
index 8d79b8b7ae1..e51d9f7b159 100644
--- a/target/arm/tcg/hflags.c
+++ b/target/arm/tcg/hflags.c
@@ -506,8 +506,8 @@ void assert_hflags_rebuild_correctly(CPUARMState *env)
 
 if (unlikely(c.flags != r.flags || c.flags2 != r.flags2)) {
 fprintf(stderr, "TCG hflags mismatch "
-"(current:(0x%08x,0x" TARGET_FMT_lx ")"
-" rebuilt:(0x%08x,0x" TARGET_FMT_lx ")\n",
+"(current:(0x%08x,0x%016" PRIx64 ")"
+" rebuilt:(0x%08x,0x%016" PRIx64 ")\n",
 c.flags, c.flags2, r.flags, r.flags2);
 abort();
 }
-- 
2.39.5

[PATCH 09/13] target/arm/cpu: define ARM_MAX_VQ once for aarch32 and aarch64

[Pierrick Bouvier]

This will affect zregs field for aarch32.
This field is used for MVE and SVE implementations. MVE implementation
is clipping index value to 0 or 1 for zregs[*].d[],
so we should not touch the rest of data in this case anyway.

Signed-off-by: Pierrick Bouvier 
---
 target/arm/cpu.h | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 27a0d4550f2..00f78d64bd8 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -169,11 +169,7 @@ typedef struct ARMGenericTimer {
  * Align the data for use with TCG host vector operations.
  */
 
-#ifdef TARGET_AARCH64
-# define ARM_MAX_VQ16
-#else
-# define ARM_MAX_VQ1
-#endif
+#define ARM_MAX_VQ16
 
 typedef struct ARMVectorReg {
 uint64_t d[2 * ARM_MAX_VQ] QEMU_ALIGNED(16);
-- 
2.39.5

[PATCH 07/13] target/arm/cpu: always define kvm related registers

[Pierrick Bouvier]

This does not hurt, even if they are not used.

Signed-off-by: Pierrick Bouvier 
---
 target/arm/cpu.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 23c2293f7d1..96f7801a239 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -971,7 +971,6 @@ struct ArchCPU {
  */
 uint32_t kvm_target;
 
-#ifdef CONFIG_KVM
 /* KVM init features for this CPU */
 uint32_t kvm_init_features[7];
 
@@ -984,7 +983,6 @@ struct ArchCPU {
 
 /* KVM steal time */
 OnOffAuto kvm_steal_time;
-#endif /* CONFIG_KVM */
 
 /* Uniprocessor system with MP extensions */
 bool mp_is_up;
-- 
2.39.5

[PATCH 00/13] single-binary: start make hw/arm/ common (boot.c)

[Pierrick Bouvier]

This series focuses on removing compilation units duplication in hw/arm. We
start with this architecture because it should not be too hard to transform it,
and should give us some good hints on the difficulties we'll meet later.

We first start by making changes in global headers to be able to not rely on
specific target defines. We then focus on removing those defines from
target/arm/cpu.h.

>From there, we modify build system to create a new hw common library (per base
architecture, "arm" in this case), instead of compiling the same files for every
target.

Finally, we can declare hw/arm/boot.c common as a first step for this subsystem.

This series needs to be applied on top of
https://lore.kernel.org/qemu-devel/20250317183417.285700-19-pierrick.bouv...@linaro.org/
to compile.

Pierrick Bouvier (13):
  exec/cpu-all: restrict BSWAP_NEEDED to target specific code
  exec/cpu-all: restrict compile time assert to target specific code
  exec/target_page: runtime defintion for TARGET_PAGE_BITS_MIN
  exec/cpu-all: allow to include specific cpu
  target/arm/cpu: move KVM_HAVE_MCE_INJECTION to kvm-all.c file directly
  exec/poison: KVM_HAVE_MCE_INJECTION can now be poisoned
  target/arm/cpu: always define kvm related registers
  target/arm/cpu: flags2 is always uint64_t
  target/arm/cpu: define ARM_MAX_VQ once for aarch32 and aarch64
  target/arm/cpu: define same set of registers for aarch32 and aarch64
  target/arm/cpu: remove inline stubs for aarch32 emulation
  meson: add common hw files
  hw/arm/boot: make compilation unit hw common

 meson.build| 36 +++-
 include/exec/cpu-all.h | 12 ++--
 include/exec/poison.h  |  2 ++
 include/exec/target_page.h |  3 +++
 include/system/kvm.h   |  2 --
 target/arm/cpu.h   | 28 +++-
 accel/kvm/kvm-all.c|  4 
 hw/arm/boot.c  |  1 +
 target/arm/helper.c|  6 ++
 target/arm/tcg/hflags.c|  4 ++--
 hw/arm/meson.build |  5 -
 11 files changed, 70 insertions(+), 33 deletions(-)

-- 
2.39.5

[PATCH 05/13] target/arm/cpu: move KVM_HAVE_MCE_INJECTION to kvm-all.c file directly

[Pierrick Bouvier]

This define is used only in accel/kvm/kvm-all.c, so we push directly the
definition there. Add more visibility to kvm_arch_on_sigbus_vcpu() to
allow removing this define from any header.

The only other architecture defining KVM_HAVE_MCE_INJECTION is i386,
which we can cleanup later.

Signed-off-by: Pierrick Bouvier 
---
 include/system/kvm.h | 2 --
 target/arm/cpu.h | 4 
 accel/kvm/kvm-all.c  | 4 
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/system/kvm.h b/include/system/kvm.h
index 716c7dcdf6b..b690dda1370 100644
--- a/include/system/kvm.h
+++ b/include/system/kvm.h
@@ -392,9 +392,7 @@ bool kvm_vcpu_id_is_valid(int vcpu_id);
 /* Returns VCPU ID to be used on KVM_CREATE_VCPU ioctl() */
 unsigned long kvm_arch_vcpu_id(CPUState *cpu);
 
-#ifdef KVM_HAVE_MCE_INJECTION
 void kvm_arch_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
-#endif
 
 void kvm_arch_init_irq_routing(KVMState *s);
 
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 7aeb012428c..23c2293f7d1 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -33,10 +33,6 @@
 
 #define CPU_INCLUDE "target/arm/cpu.h"
 
-#ifdef TARGET_AARCH64
-#define KVM_HAVE_MCE_INJECTION 1
-#endif
-
 #define EXCP_UDEF1   /* undefined instruction */
 #define EXCP_SWI 2   /* software interrupt */
 #define EXCP_PREFETCH_ABORT  3
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index f89568bfa39..28de3990699 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -13,6 +13,10 @@
  *
  */
 
+#ifdef TARGET_AARCH64
+#define KVM_HAVE_MCE_INJECTION 1
+#endif
+
 #include "qemu/osdep.h"
 #include 
 #include 
-- 
2.39.5

[PATCH 12/13] meson: add common hw files

[Pierrick Bouvier]

Those files will be compiled once per base architecture ("arm" in this
case), instead of being compiled for every variant/bitness of
architecture.

We make sure to not include target cpu definitions (exec/cpu-defs.h) by
defining header guard directly. This way, a given compilation unit can
access a specific cpu definition, but not access to compile time defines
associated.

Previous commits took care to clean up some headers to not rely on
cpu-defs.h content.

Signed-off-by: Pierrick Bouvier 
---
 meson.build | 36 +++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 672a0f79d11..0dec7d9750e 100644
--- a/meson.build
+++ b/meson.build
@@ -3689,6 +3689,7 @@ hw_arch = {}
 target_arch = {}
 target_system_arch = {}
 target_user_arch = {}
+hw_common_arch = {}
 
 # NOTE: the trace/ subdirectory needs the qapi_trace_events variable
 # that is filled in by qapi/.
@@ -4065,6 +4066,33 @@ common_all = static_library('common',
 implicit_include_directories: false,
 dependencies: common_ss.all_dependencies())
 
+# construct common libraries per base architecture
+hw_common_arch_libs = {}
+foreach target : target_dirs
+  config_target = config_target_mak[target]
+  target_base_arch = config_target['TARGET_BASE_ARCH']
+
+  # check if already generated
+  if target_base_arch in hw_common_arch_libs
+continue
+  endif
+
+  if target_base_arch in hw_common_arch
+src = hw_common_arch[target_base_arch]
+lib = static_library(
+  'hw_' + target_base_arch,
+  build_by_default: false,
+  sources: src.all_sources() + genh,
+  include_directories: common_user_inc,
+  implicit_include_directories: false,
+  # prevent common code to access cpu compile time
+  # definition, but still allow access to cpu.h
+  c_args: ['-DCPU_DEFS_H', '-DCONFIG_SOFTMMU'],
+  dependencies: src.all_dependencies())
+hw_common_arch_libs += {target_base_arch: lib}
+  endif
+endforeach
+
 if have_rust
   # We would like to use --generate-cstr, but it is only available
   # starting with bindgen 0.66.0.  The oldest supported versions
@@ -4230,8 +4258,14 @@ foreach target : target_dirs
   arch_deps += t.dependencies()
 
   target_common = common_ss.apply(config_target, strict: false)
-  objects = common_all.extract_objects(target_common.sources())
+  objects = [common_all.extract_objects(target_common.sources())]
   arch_deps += target_common.dependencies()
+  if target_type == 'system' and target_base_arch in hw_common_arch_libs
+src = hw_common_arch[target_base_arch].apply(config_target, strict: false)
+lib = hw_common_arch_libs[target_base_arch]
+objects += lib.extract_objects(src.sources())
+arch_deps += src.dependencies()
+  endif
 
   target_specific = specific_ss.apply(config_target, strict: false)
   arch_srcs += target_specific.sources()
-- 
2.39.5

[PATCH 13/13] hw/arm/boot: make compilation unit hw common

[Pierrick Bouvier]

Now we eliminated poisoned identifiers from headers, this file can now
be compiled once for all arm targets.

Signed-off-by: Pierrick Bouvier 
---
 hw/arm/boot.c  | 1 +
 hw/arm/meson.build | 5 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index e296b62fa12..639f737aefe 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -23,6 +23,7 @@
 #include "hw/loader.h"
 #include "elf.h"
 #include "system/device_tree.h"
+#include "target/arm/cpu.h"
 #include "qemu/config-file.h"
 #include "qemu/option.h"
 #include "qemu/units.h"
diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index ac473ce7cda..9e8c96059eb 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -1,5 +1,5 @@
 arm_ss = ss.source_set()
-arm_ss.add(files('boot.c'))
+arm_common_ss = ss.source_set()
 arm_ss.add(when: 'CONFIG_ARM_VIRT', if_true: files('virt.c'))
 arm_ss.add(when: 'CONFIG_ACPI', if_true: files('virt-acpi-build.c'))
 arm_ss.add(when: 'CONFIG_DIGIC', if_true: files('digic_boards.c'))
@@ -75,4 +75,7 @@ system_ss.add(when: 'CONFIG_SX1', if_true: 
files('omap_sx1.c'))
 system_ss.add(when: 'CONFIG_VERSATILE', if_true: files('versatilepb.c'))
 system_ss.add(when: 'CONFIG_VEXPRESS', if_true: files('vexpress.c'))
 
+arm_common_ss.add(fdt, files('boot.c'))
+
 hw_arch += {'arm': arm_ss}
+hw_common_arch += {'arm': arm_common_ss}
-- 
2.39.5

[PATCH 02/13] exec/cpu-all: restrict compile time assert to target specific code

[Pierrick Bouvier]

TLB_FLAGS defines are based on TARGET_PAGE_BITS_MIN, which is defined
for every target.

In the next commit, we'll introduce a non-static define for
TARGET_PAGE_BITS_MIN in common code, thus, we can't check this at
compile time, except in target specific code.

Signed-off-by: Pierrick Bouvier 
---
 include/exec/cpu-all.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 6dd71eb0de9..7c6c47c43ed 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -112,8 +112,10 @@ static inline int cpu_mmu_index(CPUState *cs, bool ifetch)
 
 #define TLB_SLOW_FLAGS_MASK  (TLB_BSWAP | TLB_WATCHPOINT | TLB_CHECK_ALIGNED)
 
+#ifdef COMPILING_PER_TARGET
 /* The two sets of flags must not overlap. */
 QEMU_BUILD_BUG_ON(TLB_FLAGS_MASK & TLB_SLOW_FLAGS_MASK);
+#endif
 
 #endif /* !CONFIG_USER_ONLY */
 
-- 
2.39.5

[PATCH 10/13] target/arm/cpu: define same set of registers for aarch32 and aarch64

[Pierrick Bouvier]

To eliminate TARGET_AARCH64, we need to make various definitions common
between 32 and 64 bit Arm targets.
Added registers are used only by aarch64 code, and the only impact is on
the size of CPUARMState, and added zarray
(ARMVectorReg zarray[ARM_MAX_VQ * 16]) member (+64KB)

It could be eventually possible to allocate this array only for aarch64
emulation, but I'm not sure it's worth the hassle to save a few KB per
vcpu. Running qemu-system takes already several hundreds of MB of
(resident) memory, and qemu-user takes dozens of MB of (resident) memory
anyway.

Signed-off-by: Pierrick Bouvier 
---
 target/arm/cpu.h | 6 --
 1 file changed, 6 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 00f78d64bd8..51b6428cfec 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -175,7 +175,6 @@ typedef struct ARMVectorReg {
 uint64_t d[2 * ARM_MAX_VQ] QEMU_ALIGNED(16);
 } ARMVectorReg;
 
-#ifdef TARGET_AARCH64
 /* In AArch32 mode, predicate registers do not exist at all.  */
 typedef struct ARMPredicateReg {
 uint64_t p[DIV_ROUND_UP(2 * ARM_MAX_VQ, 8)] QEMU_ALIGNED(16);
@@ -185,7 +184,6 @@ typedef struct ARMPredicateReg {
 typedef struct ARMPACKey {
 uint64_t lo, hi;
 } ARMPACKey;
-#endif
 
 /* See the commentary above the TBFLAG field definitions.  */
 typedef struct CPUARMTBFlags {
@@ -656,13 +654,11 @@ typedef struct CPUArchState {
 struct {
 ARMVectorReg zregs[32];
 
-#ifdef TARGET_AARCH64
 /* Store FFR as pregs[16] to make it easier to treat as any other.  */
 #define FFR_PRED_NUM 16
 ARMPredicateReg pregs[17];
 /* Scratch space for aa64 sve predicate temporary.  */
 ARMPredicateReg preg_tmp;
-#endif
 
 /* We store these fpcsr fields separately for convenience.  */
 uint32_t qc[4] QEMU_ALIGNED(16);
@@ -707,7 +703,6 @@ typedef struct CPUArchState {
 uint32_t cregs[16];
 } iwmmxt;
 
-#ifdef TARGET_AARCH64
 struct {
 ARMPACKey apia;
 ARMPACKey apib;
@@ -739,7 +734,6 @@ typedef struct CPUArchState {
  * to keep the offsets into the rest of the structure smaller.
  */
 ARMVectorReg zarray[ARM_MAX_VQ * 16];
-#endif
 
 struct CPUBreakpoint *cpu_breakpoint[16];
 struct CPUWatchpoint *cpu_watchpoint[16];
-- 
2.39.5

[PATCH 06/13] exec/poison: KVM_HAVE_MCE_INJECTION can now be poisoned

[Pierrick Bouvier]

We prevent common code to use this define by mistake.

Signed-off-by: Pierrick Bouvier 
---
 include/exec/poison.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/exec/poison.h b/include/exec/poison.h
index 8ed04b31083..816f6f99d16 100644
--- a/include/exec/poison.h
+++ b/include/exec/poison.h
@@ -67,4 +67,6 @@
 #pragma GCC poison CONFIG_WHPX
 #pragma GCC poison CONFIG_XEN
 
+#pragma GCC poison KVM_HAVE_MCE_INJECTION
+
 #endif
-- 
2.39.5

[PATCH 03/13] exec/target_page: runtime defintion for TARGET_PAGE_BITS_MIN

[Pierrick Bouvier]

We introduce later a mechanism to skip cpu definitions inclusion, so we
can detect it here, and call the correct runtime function instead.

Signed-off-by: Pierrick Bouvier 
---
 include/exec/target_page.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/exec/target_page.h b/include/exec/target_page.h
index 8e89e5cbe6f..aeddb25c743 100644
--- a/include/exec/target_page.h
+++ b/include/exec/target_page.h
@@ -40,6 +40,9 @@ extern const TargetPageBits target_page;
 #  define TARGET_PAGE_MASK   ((TARGET_PAGE_TYPE)target_page.mask)
 # endif
 # define TARGET_PAGE_SIZE(-(int)TARGET_PAGE_MASK)
+# ifndef TARGET_PAGE_BITS_MIN
+#  define TARGET_PAGE_BITS_MIN qemu_target_page_bits_min()
+# endif
 #else
 # define TARGET_PAGE_BITS_MIN TARGET_PAGE_BITS
 # define TARGET_PAGE_SIZE(1 << TARGET_PAGE_BITS)
-- 
2.39.5

[PATCH 04/13] exec/cpu-all: allow to include specific cpu

[Pierrick Bouvier]

Including "cpu.h" from code that is not compiled per target is ambiguous
by definition. Thus we introduce a conditional include, to allow every
architecture to set this, to point to the correct definition.

hw/X or target/X will now include directly "target/X/cpu.h", and
"target/X/cpu.h" will define CPU_INCLUDE to itself.
We already do this change for arm cpu as part of this commit.

Signed-off-by: Pierrick Bouvier 
---
 include/exec/cpu-all.h | 4 
 target/arm/cpu.h   | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 7c6c47c43ed..1a756c0cfb3 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -46,7 +46,11 @@
 
 CPUArchState *cpu_copy(CPUArchState *env);
 
+#ifdef CPU_INCLUDE
+#include CPU_INCLUDE
+#else
 #include "cpu.h"
+#endif
 
 #ifdef CONFIG_USER_ONLY
 
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index a8177c6c2e8..7aeb012428c 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -31,6 +31,8 @@
 #include "target/arm/multiprocessing.h"
 #include "target/arm/gtimer.h"
 
+#define CPU_INCLUDE "target/arm/cpu.h"
+
 #ifdef TARGET_AARCH64
 #define KVM_HAVE_MCE_INJECTION 1
 #endif
-- 
2.39.5

Re: [PATCH 04/17] rust/vmstate: Use ident instead of expr to parse vmsd in vmstate_struct macro

[Zhao Liu]

On Tue, Mar 18, 2025 at 10:46:10AM +0800, Zhao Liu wrote:
> Date: Tue, 18 Mar 2025 10:46:10 +0800
> From: Zhao Liu 
> Subject: Re: [PATCH 04/17] rust/vmstate: Use ident instead of expr to parse
>  vmsd in vmstate_struct macro
> 
> On Mon, Mar 17, 2025 at 06:17:07PM +0100, Paolo Bonzini wrote:
> > Date: Mon, 17 Mar 2025 18:17:07 +0100
> > From: Paolo Bonzini 
> > Subject: Re: [PATCH 04/17] rust/vmstate: Use ident instead of expr to parse
> >  vmsd in vmstate_struct macro
> > 
> > On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu  wrote:
> > >
> > > When specify an array field in vmstate_struct macro, there will be an
> > > error:
> > >
> > > > local ambiguity when calling macro `vmstate_struct`: multiple parsing
> > > > options: built-in NTs expr ('vmsd') or 1 other option.
> > >
> > > This is because "expr" can't recognize the "vmsd" field correctly, so
> > > that it gets confused with the previous array field.
> > >
> > > To fix the above issue, use "ident" for "vmsd" field, and explicitly
> > > refer to it in the macro.
> > 
> > I think this is not needed if the varray case is left as is, and other
> > cases use .with_...() instead of arguments?
> > 
> 
> Yes! With a[0..num], the `vmsd` could be parsed correctly. I'll drop this
> patch as well and refresh unit tests.
> 

Additionally, at present, IMO it is not suitable to replace the vmsd argument
with .with_vmsd() method because VMS_STRUCT requires a vmsd field, and
.with_vmsd() is optional.

There is no way to ensure that vmsd is not omitted... unless VMS_STRUCT is
also set within .with_vmsd(). This would be akin to merging vmstate_struct
and vmstate_of, but I suppose that would be a long way as for now.

So I prefer vmsd argument at the moment :-)

Thanks,
Zhao

Re: [PATCH 13/17] rust/vmstate: Support vmstate_validate

[Zhao Liu]

> > +#[doc(alias = "VMSTATE_VALIDATE")]
> > +#[macro_export]
> > +macro_rules! vmstate_validate {
> > +($struct_name:ty, $test_name:expr, $test_fn:expr $(,)?) => {
> > +$crate::bindings::VMStateField {
> > +name: ::std::ffi::CStr::as_ptr($test_name),
> > +// TODO: Use safe callback.
> 
> Why is the TODO still there?

I forgot to delete this comment...

> > +field_exists: {
> > +const fn test_cb_builder__<
> > +T,
> > +F: for<'a> $crate::callbacks::FnCall<(&'a T, u8), 
> > bool>,
> > +>(
> > +_phantom: ::core::marker::PhantomData,
> > +) -> $crate::vmstate::VMSFieldExistCb {
> > +let _: () = F::ASSERT_IS_SOME;
> > +$crate::vmstate::rust_vms_test_field_exists::
> > +}
> > +
> > +const fn phantom__(_: &T) -> 
> > ::core::marker::PhantomData {
> > +::core::marker::PhantomData
> > +}
> > +Some(test_cb_builder__::<$struct_name, 
> > _>(phantom__(&$test_fn)))
> > +},
> > +..$crate::zeroable::Zeroable::ZERO
> > +}
> > +.with_exist_check()
> > +};
> 
> Would it be possible, or make sense, to move most of the code for
> field_exists inside .with_exist_check()?
> 

If so, the method would be like:

pub fn with_exist_check(
 mut self,
 _cb: F
 ) -> Self
 where
 F: for<'a> FnCall<(&'a T, u8), bool>,

Then the use case could be like:

vmstate_struct!(HPETState, timers[0 .. num_timers], &VMSTATE_HPET_TIMER,
BqlRefCell).with_exist_check(foo_field_check),

Here we need to specify the structure type in with_exist_check, though it's
already specified in vmstate_struct as the first field.

In this way, I understand with_exist_check() doesn't need phantom__()
trick.

Instead, (after I dropped the few patches you mentioned,) now vmstate_of
& vmstate_struct could accept the optional "test_fn" field (luckily, at
least test_fn can still be parsed!), then the example would be:

vmstate_struct!(HPETState, timers[0 .. num_timers], &VMSTATE_HPET_TIMER,
BqlRefCell, foo_field_check)

And in this way, phantom__() is necessary.

So I think the main issue is the format, which do you prefer?

Thanks,
Zhao

Re: [PATCH 03/17] rust/vmstate: Add a prefix separator ", " for the array field in vmstate macros

[Zhao Liu]

On Mon, Mar 17, 2025 at 05:37:06PM +0100, Paolo Bonzini wrote:
> Date: Mon, 17 Mar 2025 17:37:06 +0100
> From: Paolo Bonzini 
> Subject: Re: [PATCH 03/17] rust/vmstate: Add a prefix separator ", " for
>  the array field in vmstate macros
> 
> On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu  wrote:
> >
> > The fields are separated by ",", so it's necessary to add ", " in array
> > field to avoid matching failure.
> 
> This is not a field though, the only (intended) fields are name and
> field. It's meant to mimic the slice since &a[0..n].
> 

Oops, I misunderstood the format...yes, something like

vmstate_struct!(HPETState, timers[0 .. num_timers], VMSTATE_HPET_TIMER, 
BqlRefCell)

can work!

I'll drop this patch and refresh the unit test in v2.

Thanks,
Zhao

Re: [PATCH 04/17] rust/vmstate: Use ident instead of expr to parse vmsd in vmstate_struct macro

[Zhao Liu]

On Mon, Mar 17, 2025 at 06:17:07PM +0100, Paolo Bonzini wrote:
> Date: Mon, 17 Mar 2025 18:17:07 +0100
> From: Paolo Bonzini 
> Subject: Re: [PATCH 04/17] rust/vmstate: Use ident instead of expr to parse
>  vmsd in vmstate_struct macro
> 
> On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu  wrote:
> >
> > When specify an array field in vmstate_struct macro, there will be an
> > error:
> >
> > > local ambiguity when calling macro `vmstate_struct`: multiple parsing
> > > options: built-in NTs expr ('vmsd') or 1 other option.
> >
> > This is because "expr" can't recognize the "vmsd" field correctly, so
> > that it gets confused with the previous array field.
> >
> > To fix the above issue, use "ident" for "vmsd" field, and explicitly
> > refer to it in the macro.
> 
> I think this is not needed if the varray case is left as is, and other
> cases use .with_...() instead of arguments?
> 

Yes! With a[0..num], the `vmsd` could be parsed correctly. I'll drop this
patch as well and refresh unit tests.

Thanks,
Zhao

Re: [PULL v2 1/6] target/loongarch: Fix vldi inst

[gaosong]

Cc: qemu-sta...@nongnu.org

Fix : https://gitlab.com/qemu-project/qemu/-/issues/2865

在 2024/12/27 下午2:22, Bibo Mao 写道:

From: Guo Hongyu 

Refer to the link below for a description of the vldi instructions:
https://jia.je/unofficial-loongarch-intrinsics-guide/lsx/misc/#synopsis_88
Fixed errors in vldi instruction implementation.

Signed-off-by: Guo Hongyu 
Tested-by: Xianglai Li 
Signed-off-by: Xianglai Li 
Reviewed-by: Bibo Mao 
Signed-off-by: Bibo Mao 
---
  target/loongarch/tcg/insn_trans/trans_vec.c.inc | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/loongarch/tcg/insn_trans/trans_vec.c.inc 
b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
index 92b1d22e28..d317dfcc1c 100644
--- a/target/loongarch/tcg/insn_trans/trans_vec.c.inc
+++ b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
@@ -3480,7 +3480,7 @@ static uint64_t vldi_get_value(DisasContext *ctx, 
uint32_t imm)
  break;
  case 1:
  /* data: {2{16'0, imm[7:0], 8'0}} */
-data = (t << 24) | (t << 8);
+data = (t << 40) | (t << 8);
  break;
  case 2:
  /* data: {2{8'0, imm[7:0], 16'0}} */

Re: [PATCH 2/2] memory: suppress INVALID_MEM logs caused by debug access

[Philippe Mathieu-Daudé]

On 14/3/25 08:41, Nicholas Piggin wrote:

Debugger-driven invalid memory accesses are not guest errors, so should
not cause these error logs.

Debuggers can access memory wildly, including access to addresses not
specified by the user (e.g., gdb it might try to walk the stack or load
target addresses to display disassembly). Failure is reported
synchronously by the GDB protcol so the user can be notified via the
debugger client.

Signed-off-by: Nicholas Piggin 
---
  system/memory.c | 37 ++---
  1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index 4c829793a0a..960f66e8d7e 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1412,18 +1412,23 @@ bool memory_region_access_valid(MemoryRegion *mr,
  {


Alternatively:

int invalid_mem_mask = attrs.debug ? LOG_INVALID_MEM : 0;


  if (mr->ops->valid.accepts
  && !mr->ops->valid.accepts(mr->opaque, addr, size, is_write, attrs)) {
-qemu_log_mask(LOG_INVALID_MEM, "Invalid %s at addr 0x%" HWADDR_PRIX
-  ", size %u, region '%s', reason: rejected\n",
-  is_write ? "write" : "read",
-  addr, size, memory_region_name(mr));
+if (attrs.debug) {
+/* Don't log memory errors due to debugger accesses */
+qemu_log_mask(LOG_INVALID_MEM, "Invalid %s at addr 0x%" HWADDR_PRIX
+  ", size %u, region '%s', reason: rejected\n",
+  is_write ? "write" : "read",
+  addr, size, memory_region_name(mr));
+}
  return false;
  }

Re: [PATCH v7 5/5] migration: add MULTIFD_RECV_SYNC migration command

[Prasad Pandit]

Hi,

On Fri, 14 Mar 2025 at 01:40, Peter Xu  wrote:
>+save_section_header(f, se, QEMU_VM_SECTION_PART);
> +ram_save_zero_page(f, se->opaque);
>I'll stop requesting a why here...

* Earlier in this thread you mentioned 'We need a header'. I took it
as a 'RAM page' header, not save_section_header(). Section type
(QEMU_VM_COMMAND) was sent by qemu_savevm_command_send() as well.

> but I think this is another example that even if all the tests pass it may 
> not be correct.

* This is also an example of - communication is hard.

> From f9343dfc777ef04168443e86a1fa3922296ea563 Mon Sep 17 00:00:00 2001
> From: Peter Xu 
> Date: Thu, 13 Mar 2025 15:34:10 -0400
> Subject: [PATCH 1/2] migration: Add save_postcopy_prepare() savevm handler
>
> Add a savevm handler for a module to opt-in sending extra sections right
> before postcopy starts, and before VM is stopped.
>
> RAM will start to use this new savevm handler in the next patch to do flush
> and sync for multifd pages.
>
> Note that we choose to do it before VM stopped because the current only
> potential user is not sensitive to VM status, so doing it before VM is
> stopped is preferred to enlarge any postcopy downtime.
>
> It is still a bit unfortunate that we need to introduce such a new savevm
> handler just for the only use case, however it's so far the cleanest.
>
> Signed-off-by: Peter Xu 
> ---
>  include/migration/register.h | 15 +++
>  migration/savevm.h   |  1 +
>  migration/migration.c|  4 
>  migration/savevm.c   | 33 +
>  4 files changed, 53 insertions(+)
>
> diff --git a/include/migration/register.h b/include/migration/register.h
> index c041ce32f2..b79dc81b8d 100644
> --- a/include/migration/register.h
> +++ b/include/migration/register.h
> @@ -189,6 +189,21 @@ typedef struct SaveVMHandlers {
>
>  /* This runs outside the BQL!  */
>
> +/**
> + * @save_postcopy_prepare
> + *
> + * This hook will be invoked on the source side right before switching
> + * to postcopy (before VM stopped).
> + *
> + * @f:  QEMUFile where to send the data
> + * @opaque: Data pointer passed to register_savevm_live()
> + * @errp:   Error** used to report error message
> + *
> + * Returns: true if succeeded, false if error occured.  When false is
> + * returned, @errp must be set.
> + */
> +bool (*save_postcopy_prepare)(QEMUFile *f, void *opaque, Error **errp);
> +
>  /**
>   * @state_pending_estimate
>   *
> diff --git a/migration/savevm.h b/migration/savevm.h
> index 138c39a7f9..2d5e9c7166 100644
> --- a/migration/savevm.h
> +++ b/migration/savevm.h
> @@ -45,6 +45,7 @@ void qemu_savevm_state_pending_exact(uint64_t *must_precopy,
>  void qemu_savevm_state_pending_estimate(uint64_t *must_precopy,
>  uint64_t *can_postcopy);
>  int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool 
> in_postcopy);
> +bool qemu_savevm_state_postcopy_prepare(QEMUFile *f, Error **errp);
>  void qemu_savevm_send_ping(QEMUFile *f, uint32_t value);
>  void qemu_savevm_send_open_return_path(QEMUFile *f);
>  int qemu_savevm_send_packaged(QEMUFile *f, const uint8_t *buf, size_t len);
> diff --git a/migration/migration.c b/migration/migration.c
> index d46e776e24..212f6b4145 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2707,6 +2707,10 @@ static int postcopy_start(MigrationState *ms, Error 
> **errp)
>  }
>  }
>
> +if (!qemu_savevm_state_postcopy_prepare(ms->to_dst_file, errp)) {
> +return -1;
> +}
> +
>  trace_postcopy_start();
>  bql_lock();
>  trace_postcopy_start_set_run();
> diff --git a/migration/savevm.c b/migration/savevm.c
> index ce158c3512..23ef4c7dc9 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1523,6 +1523,39 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f)
>  qemu_fflush(f);
>  }
>
> +bool qemu_savevm_state_postcopy_prepare(QEMUFile *f, Error **errp)
> +{
> +SaveStateEntry *se;
> +bool ret;
> +
> +QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> +if (!se->ops || !se->ops->save_postcopy_prepare) {
> +continue;
> +}
> +
> +if (se->ops->is_active) {
> +if (!se->ops->is_active(se->opaque)) {
> +continue;
> +}
> +}
> +
> +trace_savevm_section_start(se->idstr, se->section_id);
> +
> +save_section_header(f, se, QEMU_VM_SECTION_PART);
> +ret = se->ops->save_postcopy_prepare(f, se->opaque, errp);
> +save_section_footer(f, se);
> +
> +trace_savevm_section_end(se->idstr, se->section_id, ret);
> +
> +if (!ret) {
> +assert(*errp);
> +return false;
> +}
> +}
> +
> +return true;
> +}
> +
>  int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool 
> in_postcopy)
>  {
>  int64_t start_ts_each

Re: [PATCH 3/6] amd_iommu: Update bitmasks representing DTE reserved fields

[Vasant Hegde]

Hi ,


On 3/13/2025 7:53 PM, Alejandro Jimenez wrote:
> 
> 
> On 3/12/25 12:12 AM, Arun Kodilkar, Sairaj wrote:
>> Hi Alejandro,
>>
>> On 3/11/2025 8:54 PM, Alejandro Jimenez wrote:
> 
> [...]
> 
>>> --- a/hw/i386/amd_iommu.h
>>> +++ b/hw/i386/amd_iommu.h
>>> @@ -25,6 +25,8 @@
>>>   #include "hw/i386/x86-iommu.h"
>>>   #include "qom/object.h"
>>> +#define GENMASK64(h, l)  (((~0ULL) >> (63 - (h) + (l))) << (l))
>>> +
>>
>> qemu provides the similar macro 'MAKE_64BIT_MASK' in file
>> 'include/qemu/bitops.h', you can use this existing macro
>> instead of redefining.
> 
> Hi Sairaj,
> 
> I became aware of MAKE_64BIT_MASK() because you used it in your recent patch,
> but as you mentioned they are similar but not the same. I personally find that
> using bit indexes is less prone to errors since that is the same format the 
> spec
> uses to define the fields.
> So translating a RSVD[6:2] field from the spec becomes:
> 
> GENMASK64(6, 2);
> vs
> MAKE_64BIT_MASK(6, 5);
> 
> The latter is more prone to off-by-one errors in my opinion, specially when 
> you
> are defining lots of masks. Perhaps more importantly, I'd like to 
> progressively
> convert the amd_iommu definitions to use GENMASK() and the code that retrieves
> bit fields to use FIELD_GET().
> 
> I am planning on later porting the generic GENMASK* definitions (from the 
> kernel
> into "qemu/bitops.h", since the RISC-V IOMMU is also a consumer of GENMASK, 
> but
> I am trying to keep the focus on the AMD vIOMMU for this series.

I like GENMASK. Its easy to read.

RISCV has GENMASK_ULL. As you said, we can consider consolidating them later.

-Vasant

Re: [PATCH 1/6] amd_iommu: Fix Miscellanous Information Register 0 offsets

[Vasant Hegde]

On 3/11/2025 8:54 PM, Alejandro Jimenez wrote:
> The definitions encoding the maximum Virtual, Physical, and Guest Virtual
> Address sizes supported by the IOMMU are using incorrect offsets i.e. the
> VASize and GVASize offsets are switched.
> 
> Cc: qemu-sta...@nongnu.org
> Fixes: d29a09ca6842 ("hw/i386: Introduce AMD IOMMU")
> Signed-off-by: Alejandro Jimenez 

Good catch. I had read this code but missed to catch this!

Reviewed-by: Vasant Hegde 

-Vasant

> ---
>  hw/i386/amd_iommu.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
> index 28125130c6..4c708f8d74 100644
> --- a/hw/i386/amd_iommu.h
> +++ b/hw/i386/amd_iommu.h
> @@ -196,9 +196,9 @@
>  #define AMDVI_PAGE_SHIFT_4K 12
>  #define AMDVI_PAGE_MASK_4K  (~((1ULL << AMDVI_PAGE_SHIFT_4K) - 1))
>  
> -#define AMDVI_MAX_VA_ADDR  (48UL << 5)
> -#define AMDVI_MAX_PH_ADDR  (40UL << 8)
> -#define AMDVI_MAX_GVA_ADDR (48UL << 15)
> +#define AMDVI_MAX_GVA_ADDR  (48UL << 5)
> +#define AMDVI_MAX_PH_ADDR   (40UL << 8)
> +#define AMDVI_MAX_VA_ADDR   (48UL << 15)
>  
>  /* Completion Wait data size */
>  #define AMDVI_COMPLETION_DATA_SIZE8

Re: [PATCH 3/6] amd_iommu: Update bitmasks representing DTE reserved fields

[Vasant Hegde]

Hi Alejandro,


On 3/11/2025 8:54 PM, Alejandro Jimenez wrote:
> The DTE validation method verifies that all bits in reserved DTE fields are
> unset. Update them according to the latest definition available in AMD I/O
> Virtualization Technology (IOMMU) Specification - Section 2.2.2.1 Device
> Table Entry Format. Remove the magic numbers and use a macro helper to
> generate bitmasks covering the specified ranges for better legibility.
> 
> Note that some reserved fields specify that events are generated when they
> contain non-zero bits, or checks are skipped under certain configurations.
> This change only updates the reserved masks, checks for special conditions
> are not yet implemented.

Thanks! Eventually we should add some check in amdvi_get_dte(). We can do it 
later.


> 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Alejandro Jimenez 
> ---
>  hw/i386/amd_iommu.c | 7 ---
>  hw/i386/amd_iommu.h | 9 ++---
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 068eeb0cae..8b97abe28c 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -848,9 +848,10 @@ static inline uint64_t amdvi_get_perms(uint64_t entry)
>  static bool amdvi_validate_dte(AMDVIState *s, uint16_t devid,
> uint64_t *dte)
>  {
> -if ((dte[0] & AMDVI_DTE_LOWER_QUAD_RESERVED)
> -|| (dte[1] & AMDVI_DTE_MIDDLE_QUAD_RESERVED)
> -|| (dte[2] & AMDVI_DTE_UPPER_QUAD_RESERVED) || dte[3]) {
> +if ((dte[0] & AMDVI_DTE_QUAD0_RESERVED) ||
> +(dte[1] & AMDVI_DTE_QUAD1_RESERVED) ||
> +(dte[2] & AMDVI_DTE_QUAD2_RESERVED) ||
> +(dte[3] & AMDVI_DTE_QUAD3_RESERVED)) {
>  amdvi_log_illegaldevtab_error(s, devid,
>s->devtab +
>devid * AMDVI_DEVTAB_ENTRY_SIZE, 0);
> diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
> index 4c708f8d74..8d5d882a06 100644
> --- a/hw/i386/amd_iommu.h
> +++ b/hw/i386/amd_iommu.h
> @@ -25,6 +25,8 @@
>  #include "hw/i386/x86-iommu.h"
>  #include "qom/object.h"
>  
> +#define GENMASK64(h, l)  (((~0ULL) >> (63 - (h) + (l))) << (l))
> +
>  /* Capability registers */
>  #define AMDVI_CAPAB_BAR_LOW   0x04
>  #define AMDVI_CAPAB_BAR_HIGH  0x08
> @@ -162,9 +164,10 @@
>  #define AMDVI_FEATURE_PC  (1ULL << 9) /* Perf counters   
> */
>  
>  /* reserved DTE bits */
> -#define AMDVI_DTE_LOWER_QUAD_RESERVED  0x803000fc
> -#define AMDVI_DTE_MIDDLE_QUAD_RESERVED 0x0100
> -#define AMDVI_DTE_UPPER_QUAD_RESERVED  0x08f0
> +#define AMDVI_DTE_QUAD0_RESERVED(GENMASK64(6, 2) | GENMASK64(63, 63))
> +#define AMDVI_DTE_QUAD1_RESERVED0
> +#define AMDVI_DTE_QUAD2_RESERVEDGENMASK64(53, 52)
> +#define AMDVI_DTE_QUAD3_RESERVED(GENMASK64(14, 0) | GENMASK64(53, 
> 48))


In vIOMMU case guest is not expected to set any value in DTE[3]. So I am
wondering whether to match the spec -OR- what is expected in vIOMMU context. If
we want to go with later then it complicates as there are many other fields like
GV, etc is not expected to be used.

So since this matches the spec I think we are good for now.

Reviewed-by: Vasant Hegde 

-Vasant

[PATCH v3] tests/functional: remove all class level fields

[Daniel P . Berrangé]

A number of fields are set at the class level on QemuBaseTest, even
though the exact same named field is then set at the object level
later in most cases.

The 'self.logger' initialization in ACPI bits test needs to be removed
since 'self.log' won't exist at that point in the flow. It already
initialized 'self.logger' later in the setUp() method, so the __init__
method was redundant.

Signed-off-by: Daniel P. Berrangé 
---
 tests/functional/qemu_test/testcase.py | 6 --
 tests/functional/test_acpi_bits.py | 1 -
 2 files changed, 7 deletions(-)

diff --git a/tests/functional/qemu_test/testcase.py 
b/tests/functional/qemu_test/testcase.py
index 50d232a7c6..50c401b8c3 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -33,12 +33,6 @@
 
 class QemuBaseTest(unittest.TestCase):
 
-arch = None
-
-workdir = None
-log = None
-logdir = None
-
 '''
 @params compressed: filename, Asset, or file-like object to uncompress
 @params format: optional compression format (gzip, lzma)
diff --git a/tests/functional/test_acpi_bits.py 
b/tests/functional/test_acpi_bits.py
index 20da435687..8e0563a97b 100755
--- a/tests/functional/test_acpi_bits.py
+++ b/tests/functional/test_acpi_bits.py
@@ -119,7 +119,6 @@ def __init__(self, *args, **kwargs):
 
 self._debugcon_addr = '0x403'
 self._debugcon_log = 'debugcon-log.txt'
-self.logger = self.log
 
 def _print_log(self, log):
 self.logger.info('\nlogs from biosbits follows:')
-- 
2.48.1

Re: [PATCH v3 6/7] memory: Attach MemoryAttributeManager to guest_memfd-backed RAMBlocks

[Tony Lindgren]

On Mon, Mar 17, 2025 at 06:21:13PM +0800, Chenyi Qiang wrote:
> 
> 
> On 3/17/2025 5:45 PM, Tony Lindgren wrote:
> > On Mon, Mar 17, 2025 at 03:32:16PM +0800, Chenyi Qiang wrote:
> >>
> >>
> >> On 3/17/2025 2:18 PM, Tony Lindgren wrote:
> >>> Hi,
> >>>
> >>> On Mon, Mar 10, 2025 at 04:18:34PM +0800, Chenyi Qiang wrote:
>  --- a/system/physmem.c
>  +++ b/system/physmem.c
>  @@ -1885,6 +1886,16 @@ static void ram_block_add(RAMBlock *new_block, 
>  Error **errp)
>   qemu_mutex_unlock_ramlist();
>   goto out_free;
>   }
>  +
>  +new_block->memory_attribute_manager = 
>  MEMORY_ATTRIBUTE_MANAGER(object_new(TYPE_MEMORY_ATTRIBUTE_MANAGER));
>  +if 
>  (memory_attribute_manager_realize(new_block->memory_attribute_manager, 
>  new_block->mr)) {
>  +error_setg(errp, "Failed to realize memory attribute 
>  manager");
>  +object_unref(OBJECT(new_block->memory_attribute_manager));
>  +close(new_block->guest_memfd);
>  +ram_block_discard_require(false);
>  +qemu_mutex_unlock_ramlist();
>  +goto out_free;
>  +}
>   }
>   
>   ram_size = (new_block->offset + new_block->max_length) >> 
>  TARGET_PAGE_BITS;
> >>>
> >>> Might as well put the above into a separate memory manager init function
> >>> to start with. It keeps the goto out_free error path unified, and makes
> >>> things more future proof if the rest of ram_block_add() ever develops a
> >>> need to check for errors.
> >>
> >> Which part to be defined in a separate function? The init function of
> >> object_new() + realize(), or the error handling operation
> >> (object_unref() + close() + ram_block_discard_require(false))?
> > 
> > I was thinking the whole thing, including freeing :) But maybe there's
> > something more to consider to keep calls paired.
> 
> If putting the whole thing separately, I think the rest part to do error
> handling still needs to add the same operation. Or I misunderstand
> something?

So maybe you suggestion of just a separate clean-up function would work:

new_block->memory_attribute_manager =
MEMORY_ATTRIBUTE_MANAGER(object_new(TYPE_MEMORY_ATTRIBUTE_MANAGER));
if (memory_attribute_manager_realize(new_block->memory_attribute_manager,
new_block->mr)) {
memory_attribute_manager_cleanup(...);
goto out_free;
}

> >> If need to check for errors in the rest of ram_block_add() in future,
> >> how about adding a new label before out_free and move the error handling
> >> there?
> > 
> > Yeah that would work too.
> 
> I'm not sure if we should add such change directly, or we can wait for
> the real error check introduced in future.

Right, not sure either.

Regards,

Tony

[PATCH 1/4] target/arm: Explicitly set ARM_CP_NO_GDB

[Akihiko Odaki]

Explicitly set ARM_CP_NO_GDB when ARM_CP_NO_RAW is set.

Signed-off-by: Akihiko Odaki 
---
 hw/intc/arm_gicv3_cpuif.c  | 100 +-
 hw/intc/arm_gicv3_kvm.c|   2 +-
 target/arm/debug_helper.c  |   2 +-
 target/arm/helper.c| 219 +-
 target/arm/tcg/tlb-insns.c | 256 +
 5 files changed, 321 insertions(+), 258 deletions(-)

diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index 7f1d071c198b..e02854204d25 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -2432,7 +2432,7 @@ static void icc_reset(CPUARMState *env, const 
ARMCPRegInfo *ri)
 static const ARMCPRegInfo gicv3_cpuif_reginfo[] = {
 { .name = "ICC_PMR_EL1", .state = ARM_CP_STATE_BOTH,
   .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 6, .opc2 = 0,
-  .type = ARM_CP_IO | ARM_CP_NO_RAW,
+  .type = ARM_CP_IO | ARM_CP_NO_RAW | ARM_CP_NO_GDB,
   .access = PL1_RW, .accessfn = gicv3_irqfiq_access,
   .readfn = icc_pmr_read,
   .writefn = icc_pmr_write,
@@ -2444,32 +2444,32 @@ static const ARMCPRegInfo gicv3_cpuif_reginfo[] = {
 },
 { .name = "ICC_IAR0_EL1", .state = ARM_CP_STATE_BOTH,
   .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 8, .opc2 = 0,
-  .type = ARM_CP_IO | ARM_CP_NO_RAW,
+  .type = ARM_CP_IO | ARM_CP_NO_RAW | ARM_CP_NO_GDB,
   .access = PL1_R, .accessfn = gicv3_fiq_access,
   .readfn = icc_iar0_read,
 },
 { .name = "ICC_EOIR0_EL1", .state = ARM_CP_STATE_BOTH,
   .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 8, .opc2 = 1,
-  .type = ARM_CP_IO | ARM_CP_NO_RAW,
+  .type = ARM_CP_IO | ARM_CP_NO_RAW | ARM_CP_NO_GDB,
   .access = PL1_W, .accessfn = gicv3_fiq_access,
   .writefn = icc_eoir_write,
 },
 { .name = "ICC_HPPIR0_EL1", .state = ARM_CP_STATE_BOTH,
   .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 8, .opc2 = 2,
-  .type = ARM_CP_IO | ARM_CP_NO_RAW,
+  .type = ARM_CP_IO | ARM_CP_NO_RAW | ARM_CP_NO_GDB,
   .access = PL1_R, .accessfn = gicv3_fiq_access,
   .readfn = icc_hppir0_read,
 },
 { .name = "ICC_BPR0_EL1", .state = ARM_CP_STATE_BOTH,
   .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 8, .opc2 = 3,
-  .type = ARM_CP_IO | ARM_CP_NO_RAW,
+  .type = ARM_CP_IO | ARM_CP_NO_RAW | ARM_CP_NO_GDB,
   .access = PL1_RW, .accessfn = gicv3_fiq_access,
   .readfn = icc_bpr_read,
   .writefn = icc_bpr_write,
 },
 { .name = "ICC_AP0R0_EL1", .state = ARM_CP_STATE_BOTH,
   .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 8, .opc2 = 4,
-  .type = ARM_CP_IO | ARM_CP_NO_RAW,
+  .type = ARM_CP_IO | ARM_CP_NO_RAW | ARM_CP_NO_GDB,
   .access = PL1_RW, .accessfn = gicv3_fiq_access,
   .readfn = icc_ap_read,
   .writefn = icc_ap_write,
@@ -2477,81 +2477,81 @@ static const ARMCPRegInfo gicv3_cpuif_reginfo[] = {
 /* All the ICC_AP1R*_EL1 registers are banked */
 { .name = "ICC_AP1R0_EL1", .state = ARM_CP_STATE_BOTH,
   .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 9, .opc2 = 0,
-  .type = ARM_CP_IO | ARM_CP_NO_RAW,
+  .type = ARM_CP_IO | ARM_CP_NO_RAW | ARM_CP_NO_GDB,
   .access = PL1_RW, .accessfn = gicv3_irq_access,
   .readfn = icc_ap_read,
   .writefn = icc_ap_write,
 },
 { .name = "ICC_DIR_EL1", .state = ARM_CP_STATE_BOTH,
   .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 11, .opc2 = 1,
-  .type = ARM_CP_IO | ARM_CP_NO_RAW,
+  .type = ARM_CP_IO | ARM_CP_NO_RAW | ARM_CP_NO_GDB,
   .access = PL1_W, .accessfn = gicv3_dir_access,
   .writefn = icc_dir_write,
 },
 { .name = "ICC_RPR_EL1", .state = ARM_CP_STATE_BOTH,
   .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 11, .opc2 = 3,
-  .type = ARM_CP_IO | ARM_CP_NO_RAW,
+  .type = ARM_CP_IO | ARM_CP_NO_RAW | ARM_CP_NO_GDB,
   .access = PL1_R, .accessfn = gicv3_irqfiq_access,
   .readfn = icc_rpr_read,
 },
 { .name = "ICC_SGI1R_EL1", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 11, .opc2 = 5,
-  .type = ARM_CP_IO | ARM_CP_NO_RAW,
+  .type = ARM_CP_IO | ARM_CP_NO_RAW | ARM_CP_NO_GDB,
   .access = PL1_W, .accessfn = gicv3_sgi_access,
   .writefn = icc_sgi1r_write,
 },
 { .name = "ICC_SGI1R",
   .cp = 15, .opc1 = 0, .crm = 12,
-  .type = ARM_CP_64BIT | ARM_CP_IO | ARM_CP_NO_RAW,
+  .type = ARM_CP_64BIT | ARM_CP_IO | ARM_CP_NO_RAW | ARM_CP_NO_GDB,
   .access = PL1_W, .accessfn = gicv3_sgi_access,
   .writefn = icc_sgi1r_write,
 },
 { .name = "ICC_ASGI1R_EL1", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 11, .opc2 = 6,
-  .type = ARM_CP_IO | ARM_CP_NO_RAW,
+  .type = ARM_CP_IO | ARM_CP_NO_RAW | ARM_CP_NO_GDB,
   .access = PL1_W, .accessfn = gicv3_sgi_access,
   .writefn = icc_asgi1r_write,
 },
 { .name = "ICC_ASGI1R",
   .cp = 15, .opc1 = 1, .crm = 12,
-  .type = ARM_CP_64BIT | ARM_CP_IO | ARM_CP_NO_RAW,
+  .type = ARM_CP_64BIT | ARM_CP_I

[PATCH 2/4] target/arm: Do not imply ARM_CP_NO_GDB

[Akihiko Odaki]

Do not imply ARM_CP_NO_GDB when ARM_CP_NO_RAW.

A register without raw access support may still expose some state to GDB
that is managed by something else.

A register may its state with another register but may not be used for
either migration or KVM state synchronization. For example, a
multiplexing register cannot support raw access. KVM may also have
a problem when synchronizing a register.

Such a register can be flagged with ARM_CP_ALIAS | ARM_CP_NO_RAW, but
its value can be still exposed to GDB as it's usually the case for
registers flagged with ARM_CP_ALIAS. ARM_CP_NO_GDB should not be implied
in this case.

Signed-off-by: Akihiko Odaki 
---
 target/arm/cpregs.h  | 8 
 target/arm/gdbstub.c | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/target/arm/cpregs.h b/target/arm/cpregs.h
index 52377c6eb50f..99e2afc84250 100644
--- a/target/arm/cpregs.h
+++ b/target/arm/cpregs.h
@@ -75,10 +75,10 @@ enum {
  */
 ARM_CP_IO= 1 << 9,
 /*
- * Flag: Register has no underlying state and does not support raw access
- * for state saving/loading; it will not be used for either migration or
- * KVM state synchronization. Typically this is for "registers" which are
- * actually used as instructions for cache maintenance and so on.
+ * Flag: Register does not support raw access for state saving/loading; it
+ * will not be used for either migration or KVM state synchronization.
+ * Typically this is for "registers" which are actually used as 
instructions
+ * for cache maintenance and so on.
  */
 ARM_CP_NO_RAW= 1 << 10,
 /*
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 30068c226273..4459e90811b8 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -270,7 +270,7 @@ static void arm_register_sysreg_for_feature(gpointer key, 
gpointer value,
 CPUARMState *env = &cpu->env;
 DynamicGDBFeatureInfo *dyn_feature = &cpu->dyn_sysreg_feature;
 
-if (!(ri->type & (ARM_CP_NO_RAW | ARM_CP_NO_GDB))) {
+if (!(ri->type & ARM_CP_NO_GDB)) {
 if (arm_feature(env, ARM_FEATURE_AARCH64)) {
 if (ri->state == ARM_CP_STATE_AA64) {
 arm_gen_one_feature_sysreg(¶m->builder, dyn_feature,

-- 
2.48.1

[PATCH 0/4] target/arm: Flag PMCNTENCLR with ARM_CP_NO_RAW

[Akihiko Odaki]

Supersedes: <20250314-clr-v2-1-7c7220c17...@daynix.com>
("[PATCH v2] target/arm: Define raw write for PMU CLR registers")

A normal write to PMCNTENCLR clears written bits so it is not
appropriate for writing a raw value. This kind of situation is usually
handled by setting a raw write function, but flag the register with
ARM_CP_NO_RAW instead to workaround a problem with KVM.

KVM also has the same problem with PMINTENSET so add a comment to
note that. This register is already flagged with ARM_CP_NO_RAW.

Signed-off-by: Akihiko Odaki 
---
Akihiko Odaki (4):
  target/arm: Explicitly set ARM_CP_NO_GDB
  target/arm: Do not imply ARM_CP_NO_GDB
  target/arm: Expose PMINTENCLR to GDB
  target/arm: Flag PMCNTENCLR with ARM_CP_NO_RAW

 target/arm/cpregs.h|   8 +-
 hw/intc/arm_gicv3_cpuif.c  | 100 +-
 hw/intc/arm_gicv3_kvm.c|   2 +-
 target/arm/debug_helper.c  |   2 +-
 target/arm/gdbstub.c   |   2 +-
 target/arm/helper.c| 229 +++-
 target/arm/tcg/tlb-insns.c | 256 +
 7 files changed, 336 insertions(+), 263 deletions(-)
---
base-commit: 825b96dbcee23d134b691fc75618b59c5f53da32
change-id: 20250314-raw-198885f67b73

Best regards,
-- 
Akihiko Odaki 

[PATCH 3/4] target/arm: Expose PMINTENCLR to GDB

[Akihiko Odaki]

PMINTENCLR and PMINTENCLR_EL1 are aliases of PMINTENSET_EL1. Expose them
as we do for other alias registers.

Signed-off-by: Akihiko Odaki 
---
 target/arm/helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 5ce468ac2693..2dd8f4d56a1e 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2025,14 +2025,14 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
 { .name = "PMINTENCLR", .cp = 15, .crn = 9, .crm = 14, .opc1 = 0, .opc2 = 
2,
   .access = PL1_RW, .accessfn = access_tpm,
   .fgt = FGT_PMINTEN,
-  .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_NO_RAW | ARM_CP_NO_GDB,
+  .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_NO_RAW,
   .fieldoffset = offsetof(CPUARMState, cp15.c9_pminten),
   .writefn = pmintenclr_write, },
 { .name = "PMINTENCLR_EL1", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 0, .crn = 9, .crm = 14, .opc2 = 2,
   .access = PL1_RW, .accessfn = access_tpm,
   .fgt = FGT_PMINTEN,
-  .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_NO_RAW | ARM_CP_NO_GDB,
+  .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_NO_RAW,
   .fieldoffset = offsetof(CPUARMState, cp15.c9_pminten),
   .writefn = pmintenclr_write },
 { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH,

-- 
2.48.1

[PATCH 4/4] target/arm: Flag PMCNTENCLR with ARM_CP_NO_RAW

[Akihiko Odaki]

A normal write to PMCNTENCLR clears written bits so it is not
appropriate for writing a raw value. This kind of situation is usually
handled by setting a raw write function, but flag the register with
ARM_CP_NO_RAW instead to workaround a problem with KVM.

KVM also has the same problem with PMINTENSET so add a comment to
note that. This register is already flagged with ARM_CP_NO_RAW.

Signed-off-by: Akihiko Odaki 
---
 target/arm/helper.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 2dd8f4d56a1e..ee15132aadea 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1901,12 +1901,21 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
   .accessfn = pmreg_access,
   .fgt = FGT_PMCNTEN,
   .writefn = pmcntenclr_write,
-  .type = ARM_CP_ALIAS | ARM_CP_IO },
+  /*
+   * Flag PMCNTENCLR with ARM_CP_NO_RAW instead of implementing raw
+   * access to workaround a problem with KVM; KVM applies bit operations
+   * that prevents writing back raw values since Linux 6.7:
+   * 
https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a45f41d754e0b37de4b7dc1fb3c6b7a1285882fc
+   *
+   * It is fine to lack raw access with PMCNTENCLR because KVM still
+   * exposes PMCNTENSET, which shares the underlying state.
+   */
+  .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_NO_RAW },
 { .name = "PMCNTENCLR_EL0", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 2,
   .access = PL0_RW, .accessfn = pmreg_access,
   .fgt = FGT_PMCNTEN,
-  .type = ARM_CP_ALIAS | ARM_CP_IO,
+  .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_NO_RAW,
   .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcnten),
   .writefn = pmcntenclr_write },
 { .name = "PMOVSR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 3,
@@ -2025,6 +2034,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
 { .name = "PMINTENCLR", .cp = 15, .crn = 9, .crm = 14, .opc1 = 0, .opc2 = 
2,
   .access = PL1_RW, .accessfn = access_tpm,
   .fgt = FGT_PMINTEN,
+  /* Flag PMINTENCLR with ARM_CP_NO_RAW, as with PMCNTENCLR. */
   .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_NO_RAW,
   .fieldoffset = offsetof(CPUARMState, cp15.c9_pminten),
   .writefn = pmintenclr_write, },

-- 
2.48.1