date:20241014


On 10/14/24 11:02, Michael Tokarev wrote:

On 09.10.2024 03:04, Richard Henderson wrote:

Drop the 'else' so that ret is overridden with the
highest priority fault.

Fixes: d8bc1381250 ("target/hppa: Implement PSW_X")
Reviewed-by: Helge Deller 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 


Is this a qemu-stable material?  For now I assume yes, please
let me know if it is not.


The kernel fault thing is pretty nasty.
Fixing that probably requires all of patches 2-11.

I don't think the arm fix is serious enough to backport.


r~

Re: [PATCH] ui/console-vc: Silence warning about sprintf() on OpenBSD

2024-10-14 Thread Michael Tokarev


On 14.10.2024 18:15, Daniel P. Berrangé wrote:


These two lines are the only place in the code that uses the

char response[40];

so even better than switching to snprintf, how about just taking
buffer size out of the picture:

   g_autofree *response =
   g_strdup_printf("\033[%d;%dR",
   (s->y_base + s->y) % s->total_height + 1,
   s->x + 1);
   vc_respond_str(vc, response);


What's the reason to perform memory allocation in trivial places
like this?  If we're worrying about possible buffer size issue,
maybe asprintf() is a better alternative for such small things?
Fragmenting heap memory for no reason seems too much overkill.
But I'm old-scool, so.. :)

/mjt

[PATCH] plugins: fix qemu_plugin_reset

34e5e1 refactored the plugin context initialization. After this change,
tcg_ctx->plugin_insn is not reset inconditionnally anymore, but only if
one plugin at least is active.

When uninstalling the last plugin active, we stopped reinitializing
tcg_ctx->plugin_insn, which leads to memory callbacks being emitted.
This results in an error as they don't appear in a plugin op sequence as
expected.

The correct fix is to make sure we reset plugin translation variables
after current block translation ends. This way, we can catch any
potential misuse of those after a given block, in more than fixing the
current bug.

Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2570
Signed-off-by: Pierrick Bouvier 
---
 accel/tcg/plugin-gen.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 2ee4c22befd..2a8c8b2ad14 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -467,4 +467,9 @@ void plugin_gen_tb_end(CPUState *cpu, size_t num_insns)
 
 /* inject the instrumentation at the appropriate places */
 plugin_gen_inject(ptb);
+
+/* reset plugin translation state */
+tcg_ctx->plugin_db = NULL;
+tcg_ctx->plugin_insn = NULL;
+tcg_ctx->plugin_tb = NULL;
 }
-- 
2.39.5

Re: [RFC v3 2/2] target/riscv: rvv: improve performance of RISC-V vector loads and stores on large amounts of data.


On 10/14/24 15:01, Paolo Savini wrote:

This patch optimizes the emulation of unit-stride load/store RVV instructions
when the data being loaded/stored per iteration amounts to 64 bytes or more.
The optimization consists of calling __builtin_memcpy on chunks of data of 128
bytes between the memory address of the simulated vector register and the
destination memory address and vice versa.
This is done only if we have direct access to the RAM of the host machine,
if the host is little endiand and if it supports atomic 128 bit memory
operations.

Signed-off-by: Paolo Savini 
---
  target/riscv/vector_helper.c | 14 +-
  1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 75c24653f0..b3d0be8e39 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -488,7 +488,19 @@ vext_group_ldst_host(CPURISCVState *env, void *vd, 
uint32_t byte_end,
  }
  
  fn = fns[is_load][group_size];

-fn(vd, byte_offset, host + byte_offset);
+
+/* x86 and AMD processors provide strong guarantees of atomicity for
+ * 16-byte memory operations if the memory operands are 16-byte aligned */
+if (!HOST_BIG_ENDIAN && (byte_offset + 16 < byte_end) && ((byte_offset % 16) == 0) 
&&
+((cpuinfo & (CPUINFO_ATOMIC_VMOVDQA | CPUINFO_ATOMIC_VMOVDQU)) != 0)) {
+  group_size = MO_128;
+  if (is_load)
+__builtin_memcpy((uint8_t *)(vd + byte_offset), (uint8_t *)(host + 
byte_offset), 16);
+  else
+__builtin_memcpy((uint8_t *)(host + byte_offset), (uint8_t *)(vd + 
byte_offset), 16);
+} else {


This will not compile on anything other than x86.
Moreover, your comment about vmovdqa bears no relation to __builtin_memcpy.


r~

[PATCH] target/mips: Remove unused MEMOP_IDX() macro

MEMOP_IDX() is unused since commit 948f88661c6 ("target/mips:
Use cpu_*_data_ra for msa load/store"), remove it.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/mips/tcg/msa_helper.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/target/mips/tcg/msa_helper.c b/target/mips/tcg/msa_helper.c
index d2181763e72..1d40383ca4f 100644
--- a/target/mips/tcg/msa_helper.c
+++ b/target/mips/tcg/msa_helper.c
@@ -8211,14 +8211,6 @@ void helper_msa_ffint_u_df(CPUMIPSState *env, uint32_t 
df, uint32_t wd,
 /* Element-by-element access macros */
 #define DF_ELEMENTS(df) (MSA_WRLEN / DF_BITS(df))
 
-#if !defined(CONFIG_USER_ONLY)
-#define MEMOP_IDX(DF)   \
-MemOpIdx oi = make_memop_idx(MO_TE | DF | MO_UNALN, \
- mips_env_mmu_index(env));
-#else
-#define MEMOP_IDX(DF)
-#endif
-
 #if TARGET_BIG_ENDIAN
 static inline uint64_t bswap16x4(uint64_t x)
 {
-- 
2.45.2

Re: [PATCH v2 10/16] target/mips: Replace MO_TE by mo_endian()


On 10/14/24 15:18, Philippe Mathieu-Daudé wrote:

On 13/10/24 13:05, Richard Henderson wrote:

On 10/10/24 14:50, Philippe Mathieu-Daudé wrote:

+++ b/target/mips/tcg/msa_helper.c
@@ -8213,7 +8213,7 @@ void helper_msa_ffint_u_df(CPUMIPSState *env, uint32_t df, 
uint32_t wd,

  #if !defined(CONFIG_USER_ONLY)
  #define MEMOP_IDX(DF)   \
-    MemOpIdx oi = make_memop_idx(MO_TE | DF | MO_UNALN, \
+    MemOpIdx oi = make_memop_idx(mo_endian(dc) | DF | MO_UNALN,
 \
   mips_env_mmu_index(env));
  #else


This one is not within a translation context.
Surely this should be mo_endian_env().

I would have expected this not to compile?


Dead code since commit 948f88661c6 ("target/mips: Use cpu_*_data_ra
for msa load/store"):

$ git grep -w MEMOP_IDX
target/mips/tcg/msa_helper.c:8215:#define MEMOP_IDX(DF) 
 \
target/mips/tcg/msa_helper.c:8219:#define MEMOP_IDX(DF)

I'll send a cleanup patch removing the #define lines.


Ah, excellent.


Might I use your R-b tag on this patch, removing the tcg/msa_helper.c change?


Yes.

r~

Re: [PATCH] plugins: fix qemu_plugin_reset


On 10/14/24 15:33, Pierrick Bouvier wrote:

34e5e1 refactored the plugin context initialization. After this change,
tcg_ctx->plugin_insn is not reset inconditionnally anymore, but only if
one plugin at least is active.

When uninstalling the last plugin active, we stopped reinitializing
tcg_ctx->plugin_insn, which leads to memory callbacks being emitted.
This results in an error as they don't appear in a plugin op sequence as
expected.

The correct fix is to make sure we reset plugin translation variables
after current block translation ends. This way, we can catch any
potential misuse of those after a given block, in more than fixing the
current bug.

Fixes:https://gitlab.com/qemu-project/qemu/-/issues/2570
Signed-off-by: Pierrick Bouvier
---
  accel/tcg/plugin-gen.c | 5 +
  1 file changed, 5 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 1/3] include/exec: Improve probe_access_full{, _mmu} documentation


On 10/13/24 11:47, Richard Henderson wrote:

Suggested-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
  include/exec/exec-all.h | 29 ++---
  1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 72240ef426..2e4c4cc4b4 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -368,6 +368,13 @@ int probe_access_flags(CPUArchState *env, vaddr addr, int 
size,
   * The CPUTLBEntryFull structure returned via @pfull is transient
   * and must be consumed or copied immediately, before any further
   * access or changes to TLB @mmu_idx.
+ *
+ * This function will not fault if @nonfault is set, but will
+ * return TLB_INVALID_MASK if the page is not mapped, or is not
+ * accessible with @access_type.
+ *
+ * This function will return TLB_MMIO in order to force the access
+ * to be handled out-of-line if plugins wish to instrument the access.
   */
  int probe_access_full(CPUArchState *env, vaddr addr, int size,
MMUAccessType access_type, int mmu_idx,
@@ -375,22 +382,14 @@ int probe_access_full(CPUArchState *env, vaddr addr, int 
size,
CPUTLBEntryFull **pfull, uintptr_t retaddr);
  
  /**

- * probe_access_mmu() - Like probe_access_full except cannot fault and
- * doesn't trigger instrumentation.
+ * probe_access_full_mmu:
+ * Like probe_access_full, except:
   *
- * @env: CPUArchState
- * @vaddr: virtual address to probe
- * @size: size of the probe
- * @access_type: read, write or execute permission
- * @mmu_idx: softmmu index
- * @phost: ptr to return value host address or NULL
- * @pfull: ptr to return value CPUTLBEntryFull structure or NULL
- *
- * The CPUTLBEntryFull structure returned via @pfull is transient
- * and must be consumed or copied immediately, before any further
- * access or changes to TLB @mmu_idx.
- *
- * Returns: TLB flags as per probe_access_flags()
+ * This function is intended to be used for page table accesses by
+ * the target mmu itself.  Since such page walking happens while
+ * handling another potential mmu fault, this function never raises
+ * exceptions (akin to @nonfault true for probe_access_full).
+ * Likewise this function does not trigger plugin instrumentation.
   */
  int probe_access_full_mmu(CPUArchState *env, vaddr addr, int size,
MMUAccessType access_type, int mmu_idx,


Reviewed-by: Pierrick Bouvier

Re: [PATCH] plugins: fix qemu_plugin_reset


Sent a v2 to fix a leak issue with tcg_ctx->plugin_tb.

On 10/14/24 15:33, Pierrick Bouvier wrote:

34e5e1 refactored the plugin context initialization. After this change,
tcg_ctx->plugin_insn is not reset inconditionnally anymore, but only if
one plugin at least is active.

When uninstalling the last plugin active, we stopped reinitializing
tcg_ctx->plugin_insn, which leads to memory callbacks being emitted.
This results in an error as they don't appear in a plugin op sequence as
expected.

The correct fix is to make sure we reset plugin translation variables
after current block translation ends. This way, we can catch any
potential misuse of those after a given block, in more than fixing the
current bug.

Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2570
Signed-off-by: Pierrick Bouvier 
---
  accel/tcg/plugin-gen.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 2ee4c22befd..2a8c8b2ad14 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -467,4 +467,9 @@ void plugin_gen_tb_end(CPUState *cpu, size_t num_insns)
  
  /* inject the instrumentation at the appropriate places */

  plugin_gen_inject(ptb);
+
+/* reset plugin translation state */
+tcg_ctx->plugin_db = NULL;
+tcg_ctx->plugin_insn = NULL;
+tcg_ctx->plugin_tb = NULL;
  }

[PATCH v2] plugins: fix qemu_plugin_reset

34e5e1 refactored the plugin context initialization. After this change,
tcg_ctx->plugin_insn is not reset inconditionnally anymore, but only if
one plugin at least is active.

When uninstalling the last plugin active, we stopped reinitializing
tcg_ctx->plugin_insn, which leads to memory callbacks being emitted.
This results in an error as they don't appear in a plugin op sequence as
expected.

The correct fix is to make sure we reset plugin translation variables
after current block translation ends. This way, we can catch any
potential misuse of those after a given block, in more than fixing the
current bug.

v2: do not reset tcg_ctx->plugin_tb as it gets reused between
translations.

Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2570
Reviewed-by: Richard Henderson 
Signed-off-by: Pierrick Bouvier 
---
 accel/tcg/plugin-gen.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 2ee4c22befd..0f47bfbb489 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -467,4 +467,8 @@ void plugin_gen_tb_end(CPUState *cpu, size_t num_insns)
 
 /* inject the instrumentation at the appropriate places */
 plugin_gen_inject(ptb);
+
+/* reset plugin translation state (plugin_tb is reused between blocks) */
+tcg_ctx->plugin_db = NULL;
+tcg_ctx->plugin_insn = NULL;
 }
-- 
2.39.5

[PATCH] target/i386: Use only 16 and 32-bit operands for IN/OUT

The REX.W prefix is ignored for these instructions.
Mirror the solution already used for INS/OUTS: X86_SIZE_z.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2581
Signed-off-by: Richard Henderson 
---
 target/i386/tcg/decode-new.c.inc | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc
index 30be9237c3..429ed87bb6 100644
--- a/target/i386/tcg/decode-new.c.inc
+++ b/target/i386/tcg/decode-new.c.inc
@@ -1627,9 +1627,9 @@ static const X86OpEntry opcodes_root[256] = {
 [0xE2] = X86_OP_ENTRYr(LOOP,   J,b), /* implicit: CX with aflag size */
 [0xE3] = X86_OP_ENTRYr(JCXZ,   J,b), /* implicit: CX with aflag size */
 [0xE4] = X86_OP_ENTRYwr(IN,0,b, I_unsigned,b), /* AL */
-[0xE5] = X86_OP_ENTRYwr(IN,0,v, I_unsigned,b), /* AX/EAX */
+[0xE5] = X86_OP_ENTRYwr(IN,0,z, I_unsigned,b), /* AX/EAX */
 [0xE6] = X86_OP_ENTRYrr(OUT,   0,b, I_unsigned,b), /* AL */
-[0xE7] = X86_OP_ENTRYrr(OUT,   0,v, I_unsigned,b), /* AX/EAX */
+[0xE7] = X86_OP_ENTRYrr(OUT,   0,z, I_unsigned,b), /* AX/EAX */
 
 [0xF1] = X86_OP_ENTRY0(INT1,   svm(ICEBP)),
 [0xF4] = X86_OP_ENTRY0(HLT,chk(cpl0) svm(HLT)),
@@ -1761,9 +1761,9 @@ static const X86OpEntry opcodes_root[256] = {
 [0xEA] = X86_OP_ENTRYrr(JMPF,  I_unsigned,p, I_unsigned,w, chk(i64)),
 [0xEB] = X86_OP_ENTRYr(JMP,J,b),
 [0xEC] = X86_OP_ENTRYwr(IN,0,b, 2,w), /* AL, DX */
-[0xED] = X86_OP_ENTRYwr(IN,0,v, 2,w), /* AX/EAX, DX */
+[0xED] = X86_OP_ENTRYwr(IN,0,z, 2,w), /* AX/EAX, DX */
 [0xEE] = X86_OP_ENTRYrr(OUT,   0,b, 2,w), /* DX, AL */
-[0xEF] = X86_OP_ENTRYrr(OUT,   0,v, 2,w), /* DX, AX/EAX */
+[0xEF] = X86_OP_ENTRYrr(OUT,   0,z, 2,w), /* DX, AX/EAX */
 
 [0xF8] = X86_OP_ENTRY0(CLC),
 [0xF9] = X86_OP_ENTRY0(STC),
-- 
2.43.0

[PATCH] linux-user: Emulate /proc/self/maps under mmap_lock

2024-10-14 Thread Ilya Leoshkevich

If one thread modifies the mappings and another thread prints them,
a situation may occur that the printer thread sees a guest mapping
without a corresponding host mapping, leading to a crash in
open_self_maps_2().

Cc: qemu-sta...@nongnu.org
Fixes: 7b7a3366e142 ("linux-user: Use walk_memory_regions for open_self_maps")
Signed-off-by: Ilya Leoshkevich 
---
 linux-user/syscall.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 1354e756941..dd2ec0712b8 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8151,17 +8151,19 @@ static int open_self_maps_1(CPUArchState *env, int fd, 
bool smaps)
 {
 struct open_self_maps_data d = {
 .ts = get_task_state(env_cpu(env)),
-.host_maps = read_self_maps(),
 .fd = fd,
 .smaps = smaps
 };
 
+mmap_lock();
+d.host_maps = read_self_maps();
 if (d.host_maps) {
 walk_memory_regions(&d, open_self_maps_2);
 free_self_maps(d.host_maps);
 } else {
 walk_memory_regions(&d, open_self_maps_3);
 }
+mmap_unlock();
 return 0;
 }
 
-- 
2.47.0

Re: Rust BoF and maintainer minutes and planning the roadmap to Rust

2024-10-14 Thread Warner Losh

[[ sorry for the lag $LIFE has been over-full lately ]]

On Thu, Oct 3, 2024 at 3:56 AM Alex Bennée  wrote:

> Warner Losh  writes:
>
> > On Thu, Oct 3, 2024 at 2:53 AM Warner Losh  wrote:
> >
> >  On Thu, Sep 26, 2024 at 8:24 AM Alex Bennée 
> wrote:
> >
> >  One output from this discussion should be a clear statement that we are
> >  going forward with this work and the road map. A rough roadmap might
> >  look like:
> >
> >- 9.2   --enable-rust is available and developers can build with it.
> >rust devices have -x-device or -rust-device CLI flags for
> >runtime selection.
> >
> >- 10.x  rust devices feature complete and migration compatible,
> enabled
> >by default when rust compiler detected. No CLI selection
> >required as legacy portions won't be built. Any partial
> >conversions should be behind --enable-prototype-rust configure
> >flag.
> >
> >- 11.x  distros have enough infrastructure to build on supported
> >platforms. Rust becomes a mandatory dependency, old C versions
> >of converted code removed from build.
> >
> >- xx.y  QEMU becomes a pure native rust program and all C is expunged.
> >We may never get to this point.
> >
> >  We should publish the intention and the road map prominently although it
> >  was unclear if a blog post would be the best place vs expanding a
> >  section in the developers manual. Perhaps both make sense with a blog
> >  post for the statement of intent and rough timeline and the developer
> >  manual being expanded with any new rules and standards to follow?
> >
> >  FreeBSD is Tier 1 in rust only for amd64 (x86_64). It's Tier 2 for i386
> (which
> >  admittedly is going away) and Tier 3 for everything else.
> >
> > oops, I should have said it's Tier 2 with hosts for amd64, Tier 2 w/o
> hosts and
> > tier 3 for aarch64 (and everything else). In FreeBSD, amd64 and aarch64
> are
> > tier 1 supported platforms and I got those confused. It is an important
> difference
> > and later in my email I refer to it, so I thought a correction was in
> > order.
>
> Are there any other big projects coming down the line that have
> indicated a need for rust support?


There's a few things that may happen to help drive rust. People
have written a few things in rust that they hope to make default
once FreeBSD finishes its transition to pkgbase (though that's some
time in the future). There's also a desire to experiment with rust drivers
in the kernel for more fringe features to see if that helps us get done
faster.


> Obviously you don't have to worry
> about the Linux kernel but I wonder how much rust userspace you
> currently have packaged? Do you have the rust-vmm vhost-device binaries
> for example?
>

Yes. I believe we build those today.

I expect it to mostly work, most of the time, to be honest on FreeBSD. I
also
expect there to be more breakage than we see with llvm/clang...

So the bottom line should be that we'll be able to make it work, but there
is likely going to be more work since rust is less mature than C. However,
it's not clear if this is an occasional minor thing, or if it becomes major
and
frequent. And the only way to know that is to take the plunge, so don't
let this stop your plans.

Warner

Re: [PATCH v3 09/20] target/hppa: Fix priority of T, D, and B page faults

2024-10-14 Thread Michael Tokarev


On 14.10.2024 22:31, Richard Henderson wrote:

On 10/14/24 11:02, Michael Tokarev wrote:

On 09.10.2024 03:04, Richard Henderson wrote:

Drop the 'else' so that ret is overridden with the
highest priority fault.

Fixes: d8bc1381250 ("target/hppa: Implement PSW_X")
Reviewed-by: Helge Deller 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 


Is this a qemu-stable material?  For now I assume yes, please
let me know if it is not.


The kernel fault thing is pretty nasty.
Fixing that probably requires all of patches 2-11.

I don't think the arm fix is serious enough to backport.


Count me confused.

So, am I right you suggest picking up changes 2-11 for 9.1?
It looks maybe a bit too much, no?

Thanks,

/mjt

[PATCH V1 0/4] Arch agnostic ACPI changes to support vCPU Hotplug (on Archs like ARM)

Certain CPU architecture specifications [1][2][3] prohibit changes to the CPUs
*presence* after the kernel has booted. This is because many system
initializations depend on the exact CPU count at boot time and do not expect it
to change afterward. For example, components like interrupt controllers that are
closely coupled with CPUs, or various per-CPU features, may not support
configuration changes once the kernel has been initialized.

This requirement poses a challenge for virtualization features like vCPU
hotplug. To address this, changes to the ACPI AML are necessary to update the
`_STA.PRES` (presence) and `_STA.ENA` (enabled) bits accordingly during guest
initialization, as well as when vCPUs are hot-plugged or hot-unplugged. The
presence of unplugged vCPUs may need to be deliberately *simulated* at the ACPI
level to maintain a *persistent* view of vCPUs for the guest kernel.

This patch set introduces the following features:

1. ACPI Interface with Explicit PRESENT and ENABLED CPU States: It allows the
   guest kernel to evaluate these states using the `_STA` ACPI method.
   
2. Initialization of ACPI CPU States: These states are initialized during
   `machvirt_init` and when vCPUs are hot-(un)plugged. This enables hotpluggable
   vCPUs to be exposed to the guest kernel via ACPI.

3. Support for Migrating ACPI CPU States: The patch set ensures the migration of
   the newly introduced `is_{present,enabled}` ACPI CPU states to the
   destination VM.

The approach is flexible enough to accommodate ARM-like architectures that
intend to implement vCPU hotplug functionality. It is suitable for architectures
facing similar constraints to ARM or those that plan to implement vCPU
hotplugging independently of hardware support (if available).

This patch set is derived from the ARM-specific vCPU hotplug implementation [4]
and includes migration components adaptable to other architectures, following
suggestions [5] made by Igor Mammedov .

It can be applied independently, ensuring compatibility with existing hotplug
support in other architectures. I have tested this patch set in conjunction with
the ARM-specific vCPU hotplug changes (included in the upcoming RFC V5 [6]), and
everything worked as expected. I kindly request maintainers of other
architectures to provide a "Tested-by" after running their respective regression
tests.

Many thanks!


References:
[1] KVMForum 2023 Presentation: Challenges Revisited in Supporting Virt CPU 
Hotplug on
architectures that don’t Support CPU Hotplug (like ARM64)
a. Kernel Link: 
https://kvm-forum.qemu.org/2023/KVM-forum-cpu-hotplug_7OJ1YyJ.pdf
b. Qemu Link:  
https://kvm-forum.qemu.org/2023/Challenges_Revisited_in_Supporting_Virt_CPU_Hotplug_-__ii0iNb3.pdf
[2] KVMForum 2020 Presentation: Challenges in Supporting Virtual CPU Hotplug on
SoC Based Systems (like ARM64)
Link: https://kvmforum2020.sched.com/event/eE4m
[3] Check comment 5 in the bugzilla entry
Link: https://bugzilla.tianocore.org/show_bug.cgi?id=4481#c5
[4] [PATCH RFC V4 00/33] Support of Virtual CPU Hotplug for ARMv8 Arch
Link: 
https://lore.kernel.org/qemu-devel/20241009031815.250096-1-salil.me...@huawei.com/T/#mf32be203baa568a871dc625b732f666a4c4f1e68
[5] Architecture agnostic ACPI VMSD state migration (Discussion)
Link: 
https://lore.kernel.org/qemu-devel/20240715155436.577d3...@imammedo.users.ipa.redhat.com/
[6] Upcoming RFC V5, Support of Virtual CPU Hotplug for ARMv8 Arch
Link: https://github.com/salil-mehta/qemu/commits/virt-cpuhp-armv8/rfc-v5

Salil Mehta (4):
  hw/acpi: Initialize ACPI Hotplug CPU Status with Support for vCPU
`Persistence`
  hw/acpi: Update ACPI CPU Status `is_{present, enabled}` during vCPU
hot(un)plug
  hw/acpi: Reflect ACPI vCPU {present,enabled} states in ACPI
_STA.{PRES,ENA} Bits
  hw/acpi: Populate vCPU Hotplug VMSD to migrate `is_{present,enabled}`
states

 cpu-target.c patches.vcpuhp.rfc-v5.arch.agnostic.acpi  |  1 +
 hw/acpi/cpu.c  | 70 +++---
 hw/acpi/generic_event_device.c | 11 ++
 include/hw/acpi/cpu.h  | 21 ++
 include/hw/core/cpu.h  | 21 ++
 5 files changed, 119 insertions(+), 5 deletions(-)

-- 
2.34.1

[PATCH V1 1/4] hw/acpi: Initialize ACPI Hotplug CPU Status with Support for vCPU `Persistence`

Certain CPU architecture specifications [1][2][3] prohibit changes to CPU
presence after the kernel has booted. This limitation exists because many system
initializations rely on the exact CPU count at boot time and do not expect it to
change later. For example, components like interrupt controllers, which are
closely tied to CPUs, or various per-CPU features, may not support configuration
changes once the kernel has been initialized. This presents a challenge for
virtualization features such as vCPU hotplug.

To address this issue, introduce an `is_enabled` state in the `AcpiCpuStatus`,
which reflects whether a vCPU has been hot-plugged or hot-unplugged in QEMU,
marking it as (un)available in the Guest Kernel. The `is_present` state should
be set based on the `acpi_persistent` flag. In cases where unplugged vCPUs need
to be deliberately simulated in the ACPI to maintain a persistent view of vCPUs,
this flag ensures the guest kernel continues to see those vCPUs.

Additionally, introduce an `acpi_persistent` property that can be used to
initialize the ACPI vCPU presence state accordingly. Architectures requiring
ACPI to expose a persistent view of vCPUs can override its default value. Refer
to the patch-set implelenting vCPU hotplug support for ARM for more details on
its usage.

References:
[1] KVMForum 2023 Presentation: Challenges Revisited in Supporting Virt CPU 
Hotplug on
architectures that don’t Support CPU Hotplug (like ARM64)
a. Kernel Link: 
https://kvm-forum.qemu.org/2023/KVM-forum-cpu-hotplug_7OJ1YyJ.pdf
b. Qemu Link:  
https://kvm-forum.qemu.org/2023/Challenges_Revisited_in_Supporting_Virt_CPU_Hotplug_-__ii0iNb3.pdf
[2] KVMForum 2020 Presentation: Challenges in Supporting Virtual CPU Hotplug on
SoC Based Systems (like ARM64)
Link: https://kvmforum2020.sched.com/event/eE4m
[3] Check comment 5 in the bugzilla entry
Link: https://bugzilla.tianocore.org/show_bug.cgi?id=4481#c5

Signed-off-by: Salil Mehta 
---
 cpu-target.c  |  1 +
 hw/acpi/cpu.c | 35 ++-
 include/hw/acpi/cpu.h | 21 +
 include/hw/core/cpu.h | 21 +
 4 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/cpu-target.c b/cpu-target.c
index 499facf774..c8a29ab495 100644
--- a/cpu-target.c
+++ b/cpu-target.c
@@ -200,6 +200,7 @@ static Property cpu_common_props[] = {
  */
 DEFINE_PROP_LINK("memory", CPUState, memory, TYPE_MEMORY_REGION,
  MemoryRegion *),
+DEFINE_PROP_BOOL("acpi-persistent", CPUState, acpi_persistent, false),
 #endif
 DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 5cb60ca8bc..083c4010c2 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -225,7 +225,40 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
 state->dev_count = id_list->len;
 state->devs = g_new0(typeof(*state->devs), state->dev_count);
 for (i = 0; i < id_list->len; i++) {
-state->devs[i].cpu =  CPU(id_list->cpus[i].cpu);
+struct CPUState *cpu = CPU(id_list->cpus[i].cpu);
+/*
+ * In most architectures, CPUs that are marked as ACPI 'present' are
+ * also ACPI 'enabled' by default. These states remain consistent at
+ * both the QOM and ACPI levels.
+ */
+if (cpu) {
+state->devs[i].is_enabled = true;
+state->devs[i].is_present = true;
+state->devs[i].cpu = cpu;
+} else {
+state->devs[i].is_enabled = false;
+/*
+ * In some architectures, even 'unplugged' or 'disabled' QOM CPUs
+ * may be exposed as ACPI 'present.' This approach provides a
+ * persistent view of the vCPUs to the guest kernel. This could be
+ * due to an architectural constraint that requires every per-CPU
+ * component to be present at boot time, meaning the exact count of
+ * vCPUs must be known and cannot be altered after the kernel has
+ * booted. As a result, the vCPU states at the QOM and ACPI levels
+ * might become inconsistent. However, in such cases, the presence
+ * of vCPUs has been deliberately simulated at the ACPI level.
+ */
+if (acpi_persistent_cpu(first_cpu)) {
+state->devs[i].is_present = true;
+/*
+ * `CPUHotplugState::AcpiCpuStatus::cpu` becomes insignificant
+ * in this case
+ */
+} else {
+state->devs[i].is_present = false;
+state->devs[i].cpu = cpu;
+}
+}
 state->devs[i].arch_id = id_list->cpus[i].arch_id;
 }
 memory_region_init_io(&state->ctrl_reg, owner, &cpu_hotplug_ops, state,
diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
index 32654dc274..bd3f9973c9 100644
--- a/include/hw/acpi/cpu.h
+++ b/include/hw/acpi/cpu.h
@@ -26,6 +26,8 @@ typedef s

[PATCH V1 2/4] hw/acpi: Update ACPI CPU Status `is_{present, enabled}` during vCPU hot(un)plug

Update the `AcpiCpuStatus` for `is_enabled` and `is_present` accordingly when
vCPUs are hot-plugged or hot-unplugged, taking into account the *persistence*
of the vCPUs.

Signed-off-by: Salil Mehta 
---
 hw/acpi/cpu.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 083c4010c2..700aa855e9 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -291,6 +291,8 @@ void acpi_cpu_plug_cb(HotplugHandler *hotplug_dev,
 }
 
 cdev->cpu = CPU(dev);
+cdev->is_present = true;
+cdev->is_enabled = true;
 if (dev->hotplugged) {
 cdev->is_inserting = true;
 acpi_send_event(DEVICE(hotplug_dev), ACPI_CPU_HOTPLUG_STATUS);
@@ -322,6 +324,11 @@ void acpi_cpu_unplug_cb(CPUHotplugState *cpu_st,
 return;
 }
 
+cdev->is_enabled = false;
+if (!acpi_persistent_cpu(CPU(dev))) {
+cdev->is_present = false;
+}
+
 cdev->cpu = NULL;
 }
 
-- 
2.34.1

[PATCH V1 3/4] hw/acpi: Reflect ACPI vCPU {present, enabled} states in ACPI _STA.{PRES, ENA} Bits

Reflect the ACPI CPU hotplug `is_{present, enabled}` states in the `_STA.PRES`
(presence) and `_STA.ENA` (enabled) bits when the guest kernel evaluates the
ACPI `_STA` method during initialization, as well as when vCPUs are hot-plugged
or hot-unplugged. The presence of unplugged vCPUs may need to be deliberately
*simulated* at the ACPI level to maintain a *persistent* view of vCPUs for the
guest kernel.

Signed-off-by: Salil Mehta 
---
 hw/acpi/cpu.c | 26 ++
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 700aa855e9..23ea2b9c70 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -63,10 +63,11 @@ static uint64_t cpu_hotplug_rd(void *opaque, hwaddr addr, 
unsigned size)
 cdev = &cpu_st->devs[cpu_st->selector];
 switch (addr) {
 case ACPI_CPU_FLAGS_OFFSET_RW: /* pack and return is_* fields */
-val |= cdev->cpu ? 1 : 0;
+val |= cdev->is_enabled ? 1 : 0;
 val |= cdev->is_inserting ? 2 : 0;
 val |= cdev->is_removing  ? 4 : 0;
 val |= cdev->fw_remove  ? 16 : 0;
+val |= cdev->is_present ? 32 : 0;
 trace_cpuhp_acpi_read_flags(cpu_st->selector, val);
 break;
 case ACPI_CPU_CMD_DATA_OFFSET_RW:
@@ -376,6 +377,7 @@ const VMStateDescription vmstate_cpu_hotplug = {
 #define CPU_REMOVE_EVENT  "CRMV"
 #define CPU_EJECT_EVENT   "CEJ0"
 #define CPU_FW_EJECT_EVENT "CEJF"
+#define CPU_PRESENT   "CPRS"
 
 void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
 build_madt_cpu_fn build_madt_cpu, hwaddr base_addr,
@@ -436,7 +438,9 @@ void build_cpus_aml(Aml *table, MachineState *machine, 
CPUHotplugFeatures opts,
 aml_append(field, aml_named_field(CPU_EJECT_EVENT, 1));
 /* tell firmware to do device eject, write only */
 aml_append(field, aml_named_field(CPU_FW_EJECT_EVENT, 1));
-aml_append(field, aml_reserved_field(3));
+/* 1 if present, read only */
+aml_append(field, aml_named_field(CPU_PRESENT, 1));
+aml_append(field, aml_reserved_field(2));
 aml_append(field, aml_named_field(CPU_COMMAND, 8));
 aml_append(cpu_ctrl_dev, field);
 
@@ -466,6 +470,7 @@ void build_cpus_aml(Aml *table, MachineState *machine, 
CPUHotplugFeatures opts,
 Aml *ctrl_lock = aml_name("%s.%s", cphp_res_path, CPU_LOCK);
 Aml *cpu_selector = aml_name("%s.%s", cphp_res_path, CPU_SELECTOR);
 Aml *is_enabled = aml_name("%s.%s", cphp_res_path, CPU_ENABLED);
+Aml *is_present = aml_name("%s.%s", cphp_res_path, CPU_PRESENT);
 Aml *cpu_cmd = aml_name("%s.%s", cphp_res_path, CPU_COMMAND);
 Aml *cpu_data = aml_name("%s.%s", cphp_res_path, CPU_DATA);
 Aml *ins_evt = aml_name("%s.%s", cphp_res_path, CPU_INSERT_EVENT);
@@ -494,13 +499,26 @@ void build_cpus_aml(Aml *table, MachineState *machine, 
CPUHotplugFeatures opts,
 {
 Aml *idx = aml_arg(0);
 Aml *sta = aml_local(0);
+Aml *ifctx2;
+Aml *else_ctx;
 
 aml_append(method, aml_acquire(ctrl_lock, 0x));
 aml_append(method, aml_store(idx, cpu_selector));
 aml_append(method, aml_store(zero, sta));
-ifctx = aml_if(aml_equal(is_enabled, one));
+ifctx = aml_if(aml_equal(is_present, one));
 {
-aml_append(ifctx, aml_store(aml_int(0xF), sta));
+ifctx2 = aml_if(aml_equal(is_enabled, one));
+{
+/* cpu is present and enabled */
+aml_append(ifctx2, aml_store(aml_int(0xF), sta));
+}
+aml_append(ifctx, ifctx2);
+else_ctx = aml_else();
+{
+/* cpu is present but disabled */
+aml_append(else_ctx, aml_store(aml_int(0xD), sta));
+}
+aml_append(ifctx, else_ctx);
 }
 aml_append(method, ifctx);
 aml_append(method, aml_release(ctrl_lock));
-- 
2.34.1

[PATCH V1 4/4] hw/acpi: Populate vCPU Hotplug VMSD to migrate `is_{present, enabled}` states

The ACPI CPU hotplug states `is_{present, enabled}` must be migrated alongside
other vCPU hotplug states to the destination VM. Therefore, they should be
integrated into the existing CPU Hotplug VM State Description (VMSD) table.
Depending on the architecture and its implementation of CPU hotplug events
(such as ACPI GED, etc.), the CPU hotplug states should be populated
appropriately within their corresponding subsections of the VMSD table.

"acpi-ged (16)": {
"ged_state": {
"sel": "0x"
},
[...]
"acpi-ged/cpuhp": {
"cpuhp_state": {
"selector": "0x0005",
"command": "0x00",
"devs": [
{
"is_inserting": false,
"is_removing": false,
"is_present": true,
"is_enabled": true,
"ost_event": "0x",
"ost_status": "0x"
},
{
"is_inserting": false,
"is_removing": false,
"is_present": true,
"is_enabled": true,
"ost_event": "0x",
"ost_status": "0x"
},
{
"is_inserting": false,
"is_removing": false,
"is_present": true,
"is_enabled": true,
"ost_event": "0x",
"ost_status": "0x"
},
{
"is_inserting": false,
"is_removing": false,
"is_present": true,
"is_enabled": true,
"ost_event": "0x",
"ost_status": "0x"
},
{
"is_inserting": false,
"is_removing": false,
"is_present": true,
"is_enabled": false,
"ost_event": "0x",
"ost_status": "0x"
},
{
"is_inserting": false,
"is_removing": false,
"is_present": true,
"is_enabled": false,
"ost_event": "0x",
"ost_status": "0x"
}
]
}
}
},

Signed-off-by: Salil Mehta 
---
 hw/acpi/cpu.c  |  2 ++
 hw/acpi/generic_event_device.c | 11 +++
 2 files changed, 13 insertions(+)

diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 23ea2b9c70..d34c1e601e 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -340,6 +340,8 @@ static const VMStateDescription vmstate_cpuhp_sts = {
 .fields = (const VMStateField[]) {
 VMSTATE_BOOL(is_inserting, AcpiCpuStatus),
 VMSTATE_BOOL(is_removing, AcpiCpuStatus),
+VMSTATE_BOOL(is_present, AcpiCpuStatus),
+VMSTATE_BOOL(is_enabled, AcpiCpuStatus),
 VMSTATE_UINT32(ost_event, AcpiCpuStatus),
 VMSTATE_UINT32(ost_status, AcpiCpuStatus),
 VMSTATE_END_OF_LIST()
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index 15b4c3ebbf..a4d78a534c 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -331,6 +331,16 @@ static const VMStateDescription vmstate_memhp_state = {
 }
 };
 
+static const VMStateDescription vmstate_cpuhp_state = {
+.name = "acpi-ged/cpuhp",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields  = (VMStateField[]) {
+VMSTATE_CPU_HOTPLUG(cpuhp_state, AcpiGedState),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static const VMStateDescription vmstate_ged_state = {
 .name = "acpi-ged-state",
 .version_id = 1,
@@ -379,6 +389,7 @@ static const VMStateDescription vmstate_acpi_ged = {
 },
 .subsections = (const VMStateDescription * const []) {
 &vmstate_memhp_state,
+&vmstate_cpuhp_state,
 &vmstate_ghes_state,
 NULL
 }
-- 
2.34.1

Re: [PATCH v2 5/7] target/i386/cpu: Improve errors for out of bounds property values

2024-10-14 Thread Markus Armbruster

Philippe Mathieu-Daudé  writes:

> On 10/10/24 16:25, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé  writes:
>> 
>>> On 10/10/24 12:01, Markus Armbruster wrote:
 The error message for a "stepping" value that is out of bounds is a
 bit odd:
   $ qemu-system-x86_64 -cpu qemu64,stepping=16
   qemu-system-x86_64: can't apply global 
 qemu64-x86_64-cpu.stepping=16: Property .stepping doesn't take value 16 
 (minimum: 0, maximum: 15)
 The "can't apply global" part is an unfortunate artifact of -cpu's
 implementation.  Left for another day.
 The remainder feels overly verbose.  Change it to
   qemu64-x86_64-cpu: can't apply global qemu64-x86_64-cpu.stepping=16: 
 parameter 'stepping' can be at most 15
 Likewise for "family", "model", and "tsc-frequency".
 Signed-off-by: Markus Armbruster 

[...]

>>> Confusing:
>>>
>>>  qemu64-x86_64-cpu: can't apply global qemu64-x86_64-cpu.stepping=-1: 
>>> parameter 'stepping' can be at most 15
>>
>> For better or worse, visit_type_uint64() with the string input visitor
>> parses -1 modulo 2^64, i.e. as 2^64-1, just like strtoul() & friends.

I wish we had avoided that design mistake.  Likely too late to fix now.
The JSON parser gets it right.

> Would "parameter 'stepping' must be between 1 and 15" be clearer?

It might be clearer and would be wronger: zero is a valid value.

I could do "must be between 0 and 15".  But "stepping" is a *counter*.
A negative stepping makes no sense to me.

Same for model and family.

More so for tsc-frequency.

Thoughts?

Re: [PATCH] ui/console-vc: Silence warning about sprintf() on OpenBSD

2024-10-14 Thread Marc-André Lureau

On Mon, Oct 14, 2024 at 7:10 PM Thomas Huth  wrote:
>
> The linker on OpenBSD complains:
>
>  ld: warning: console-vc.c:824 (../src/ui/console-vc.c:824)([...]):
>  warning: sprintf() is often misused, please use snprintf()
>
> Using snprintf() is certainly better here, so let's switch to that
> function instead.
>
> Signed-off-by: Thomas Huth 

Reviewed-by: Marc-André Lureau 

> ---
>  ui/console-vc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/ui/console-vc.c b/ui/console-vc.c
> index 8393d532e7..336a1520eb 100644
> --- a/ui/console-vc.c
> +++ b/ui/console-vc.c
> @@ -821,9 +821,9 @@ static void vc_putchar(VCChardev *vc, int ch)
>  break;
>  case 6:
>  /* report cursor position */
> -sprintf(response, "\033[%d;%dR",
> -   (s->y_base + s->y) % s->total_height + 1,
> -s->x + 1);
> +snprintf(response, sizeof(response), "\033[%d;%dR",
> + (s->y_base + s->y) % s->total_height + 1,
> + s->x + 1);
>  vc_respond_str(vc, response);
>  break;
>  }
> --
> 2.46.1
>

RE: [PULL v2 40/61] hw/acpi: Update GED _EVT method AML with CPU scan

Hi Bibo,

>  From: maobibo 
>  Sent: Monday, October 14, 2024 9:53 AM
>  To: qemu-devel@nongnu.org; Salil Mehta 
>  Cc: Michael S. Tsirkin ; Peter Maydell
>  ; Salil Mehta ;
>  zhukeqian ; Jonathan Cameron
>  ; Gavin Shan ;
>  Vishnu Pajjuri ; Xianglai Li
>  ; Miguel Luis ; Shaoqin
>  Huang ; Zhao Liu ; Igor
>  Mammedov ; Ani Sinha 
>  Subject: Re: [PULL v2 40/61] hw/acpi: Update GED _EVT method AML with
>  CPU scan
>  
>  Hi Salil,
>  
>  When I debug cpu hotplug on LoongArch system, It reports error like this:
>   ACPI BIOS Error (bug): Could not resolve symbol [\_SB.GED.CSCN],
>  AE_NOT_FOUND
>   ACPI Error: Aborting method \_SB.GED._EVT due to previous error
>  (AE_NOT_FOUND)
>   acpi-ged ACPI0013:00: IRQ method execution failed
>  
>  
>  With this patch, GED CPU call method is "\\_SB.GED.CSCN", however in
>  function build_cpus_aml(), its method name is "\\_SB.CPUS.CSCN".
>   method = aml_method(event_handler_method, 0,
>  AML_NOTSERIALIZED);
>   aml_append(method, aml_call0("\\_SB.CPUS." CPU_SCAN_METHOD));
>   aml_append(table, method);
>  
>  It seems that CPU scanning method name is not consistent between
>  function build_cpus_aml() and build_ged_aml().


I believe your question stems from the following patch I've sent recently:

https://lore.kernel.org/qemu-devel/20241009031815.250096-16-salil.me...@huawei.com/

I’ve already proposed a fix for this issue. Does that not work for you?

Thanks
Salil.


>  
>  Regards
>  Bibo Mao
>  
>  On 2024/7/23 下午6:59, Michael S. Tsirkin wrote:
>  > From: Salil Mehta 
>  >
>  > OSPM evaluates _EVT method to map the event. The CPU hotplug event
>  > eventually results in start of the CPU scan. Scan figures out the CPU
>  > and the kind of
>  > event(plug/unplug) and notifies it back to the guest. Update the GED
>  > AML _EVT method with the call to method \\_SB.CPUS.CSCN (via
>  > \\_SB.GED.CSCN)
>  >
>  > Architecture specific code [1] might initialize its CPUs AML code by
>  > calling common function build_cpus_aml() like below for ARM:
>  >
>  > build_cpus_aml(scope, ms, opts, xx_madt_cpu_entry,
>  memmap[VIRT_CPUHP_ACPI].base,
>  > "\\_SB", "\\_SB.GED.CSCN", AML_SYSTEM_MEMORY);
>  >
>  > [1]
>  > https://lore.kernel.org/qemu-devel/20240613233639.202896-13-salil.meht
>  > a...@huawei.com/
>  >
>  > Co-developed-by: Keqian Zhu 
>  > Signed-off-by: Keqian Zhu 
>  > Signed-off-by: Salil Mehta 
>  > Reviewed-by: Jonathan Cameron 
>  > Reviewed-by: Gavin Shan 
>  > Tested-by: Vishnu Pajjuri 
>  > Tested-by: Xianglai Li 
>  > Tested-by: Miguel Luis 
>  > Reviewed-by: Shaoqin Huang 
>  > Tested-by: Zhao Liu 
>  > Reviewed-by: Igor Mammedov 
>  > Message-Id: <20240716111502.202344-5-salil.me...@huawei.com>
>  > Reviewed-by: Michael S. Tsirkin 
>  > Signed-off-by: Michael S. Tsirkin 
>  > ---
>  >   include/hw/acpi/generic_event_device.h | 1 +
>  >   hw/acpi/generic_event_device.c | 3 +++
>  >   2 files changed, 4 insertions(+)
>  >
>  > diff --git a/include/hw/acpi/generic_event_device.h
>  > b/include/hw/acpi/generic_event_device.h
>  > index e091ac2108..40af3550b5 100644
>  > --- a/include/hw/acpi/generic_event_device.h
>  > +++ b/include/hw/acpi/generic_event_device.h
>  > @@ -87,6 +87,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState,
>  ACPI_GED)
>  >   #define GED_DEVICE  "GED"
>  >   #define AML_GED_EVT_REG "EREG"
>  >   #define AML_GED_EVT_SEL "ESEL"
>  > +#define AML_GED_EVT_CPU_SCAN_METHOD "\\_SB.GED.CSCN"
>  >
>  >   /*
>  >* Platforms need to specify the GED event bitmap diff --git
>  > a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
>  > index 4641933a0f..15b4c3ebbf 100644
>  > --- a/hw/acpi/generic_event_device.c
>  > +++ b/hw/acpi/generic_event_device.c
>  > @@ -108,6 +108,9 @@ void build_ged_aml(Aml *table, const char *name,
>  HotplugHandler *hotplug_dev,
>  >   aml_append(if_ctx, aml_call0(MEMORY_DEVICES_CONTAINER
>  "."
>  >MEMORY_SLOT_SCAN_METHOD));
>  >   break;
>  > +case ACPI_GED_CPU_HOTPLUG_EVT:
>  > +aml_append(if_ctx,
>  aml_call0(AML_GED_EVT_CPU_SCAN_METHOD));
>  > +break;
>  >   case ACPI_GED_PWR_DOWN_EVT:
>  >   aml_append(if_ctx,
>  >
>  > aml_notify(aml_name(ACPI_POWER_BUTTON_DEVICE),
>  >

RE: [PULL v2 40/61] hw/acpi: Update GED _EVT method AML with CPU scan

Hi Igor,

>  From: qemu-devel-bounces+salil.mehta=huawei@nongnu.org   devel-bounces+salil.mehta=huawei@nongnu.org> On Behalf Of Igor
>  Mammedov
>  Sent: Monday, October 14, 2024 10:38 AM
>  
>  On Mon, 14 Oct 2024 16:52:55 +0800
>  maobibo  wrote:
>  
>  > Hi Salil,
>  >
>  > When I debug cpu hotplug on LoongArch system, It reports error like this:
>  >  ACPI BIOS Error (bug): Could not resolve symbol [\_SB.GED.CSCN],
>  > AE_NOT_FOUND
>  >  ACPI Error: Aborting method \_SB.GED._EVT due to previous error
>  > (AE_NOT_FOUND)
>  >  acpi-ged ACPI0013:00: IRQ method execution failed
>  >
>  >
>  > With this patch, GED CPU call method is "\\_SB.GED.CSCN", however in
>  > function build_cpus_aml(), its method name is "\\_SB.CPUS.CSCN".
>  >  method = aml_method(event_handler_method, 0,
>  AML_NOTSERIALIZED);
>  >  aml_append(method, aml_call0("\\_SB.CPUS." CPU_SCAN_METHOD));
>  >  aml_append(table, method);
>  >
>  > It seems that CPU scanning method name is not consistent between
>  > function build_cpus_aml() and build_ged_aml().
>  >
>  > Regards
>  > Bibo Mao
>  >
>  > On 2024/7/23 下午6:59, Michael S. Tsirkin wrote:
>  > > From: Salil Mehta 
>  > >
>  > > OSPM evaluates _EVT method to map the event. The CPU hotplug
>  event
>  > > eventually results in start of the CPU scan. Scan figures out the
>  > > CPU and the kind of
>  > > event(plug/unplug) and notifies it back to the guest. Update the GED
>  > > AML _EVT method with the call to method \\_SB.CPUS.CSCN (via
>  > > \\_SB.GED.CSCN)
>  > >
>  > > Architecture specific code [1] might initialize its CPUs AML code by
>  > > calling common function build_cpus_aml() like below for ARM:
>  > >
>  > > build_cpus_aml(scope, ms, opts, xx_madt_cpu_entry,
>  memmap[VIRT_CPUHP_ACPI].base,
>  > > "\\_SB", "\\_SB.GED.CSCN", AML_SYSTEM_MEMORY);
>  
>  it should be \\_SB.CPUS.CSCN

I guess we are getting back to where we started then?

https://lore.kernel.org/qemu-devel/20240706162845.3baf5...@imammedo.users.ipa.redhat.com/

Excerpt from above discussion and your suggestion:
[...]

I don't particularly like exposing cpu hotplug internals for outside code
and then making that code do plumbing hoping that nothing will explode
in the future.

build_cpus_aml() takes event_handler_method to create a method that
can be called by platform. What I suggest is to call that method here
instead of trying to expose CPU hotplug internals and manually building
call path here.
aka:
  build_cpus_aml(event_handler_method = PATH_TO_GED_DEVICE.CSCN)
and then call here 
  aml_append(if_ctx, aml_call0(CSCN));
which will call  CSCN in GED scope, that was be populated by
build_cpus_aml() to do cpu scan properly without need to expose
cpu hotplug internal names and then trying to fixup conflicts caused by that.

PS:
we should do the same for memory hotplug, we see in context above

[...]

Solution:
I've avoided above error in different way and keeping exactly what you
suggested \_SB.PATH_TO_GED_DEVICE.CSCN i.e. \_SB.GED.CSCN
Please have a look:

https://lore.kernel.org/qemu-devel/20241009031815.250096-16-salil.me...@huawei.com/

Many thanks!

Best regards
Salil.

[PATCH v2 1/6] ui/sdl2: Restore original context after new context creation

SDL API changes GL context to a newly created GL context, which differs
from other GL providers that don't switch context. Change SDL backend to
restore the original GL context. This allows Qemu's virtio-gpu to support
new virglrenderer async-fencing feature for Virgl contexts, otherwise
virglrenderer's vrend creates a fence-sync context on the Qemu's
main-loop thread that erroneously stays in-use by the main-loop after
creation, not allowing vrend's fence-sync thread switch to this new
context that belongs to it.

Signed-off-by: Dmitry Osipenko 
---
 ui/sdl2-gl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
index e01d9ab0c7bf..b1fe96d6af22 100644
--- a/ui/sdl2-gl.c
+++ b/ui/sdl2-gl.c
@@ -168,6 +168,9 @@ QEMUGLContext sdl2_gl_create_context(DisplayGLCtx *dgc,
 SDL_GL_CONTEXT_PROFILE_ES);
 ctx = SDL_GL_CreateContext(scon->real_window);
 }
+
+SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
+
 return (QEMUGLContext)ctx;
 }
 
-- 
2.47.0

[PATCH v2 0/6] Support virtio-gpu DRM native context

This patchset adds DRM native context support to VirtIO-GPU on Qemu.
It's based on the pending Venus v17 patches [1] that bring host blobs
support to virtio-gpu-gl device.

Based-on: 20240822185110.1757429-1-dmitry.osipe...@collabora.com

[1]
https://lore.kernel.org/qemu-devel/20240822185110.1757429-1-dmitry.osipe...@collabora.com/

Contarary to Virgl and Venus contexts which mediate high level GFX APIs,
DRM native context [2] mediates lower level kernel driver UAPI, which
reflects in a less CPU overhead and less/simpler code needed to support it.
DRM context consists of a host and guest parts that have to be implemented
for each GPU driver. On a guest side, DRM context presents a virtual GPU as
a real/native host GPU device for GL/VK applications.

[2] https://www.youtube.com/watch?v=9sFP_yddLLQ

Today there are four known DRM native context drivers existing in a wild:

- Freedreno (Qualcomm SoC GPUs), completely upstreamed
- AMDGPU, mostly merged into upstreams
- Intel (i915), merge requests are opened
- Asahi (Apple SoC GPUs), WIP status

# How to try out DRM context:

1. Like Venus and Virgl context, DRM context requires applying WIP
KVM patches [3] to host kernel, otherwise mapping of GPU memory blobs
will likely fail.

[3] https://lore.kernel.org/all/20240726235234.228822-1-sea...@google.com/

2. Use latest libvirglrenderer from upstream git/main for Freedreno
and AMDGPU native contexts. For Intel use patches [4].

[4] https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1384

3. On guest, use latest Mesa version for Freedreno. For AMDGPU use
Mesa patches [5], for Intel [6].

[5] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21658
[6] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/29870

4. On guest, use latest Linux kernel v6.6+.

Example Qemu cmdline that enables DRM context:

qemu-system-x86_64 -device virtio-vga-gl,hostmem=4G,blob=on,drm=on \
-machine q35,accel=kvm,memory-backend=mem1 \
-object memory-backend-memfd,id=mem1,size=8G -m 8G

# Note about known performance problem in Qemu:

DRM contexts are mapping host blobs extensively and these mapping
operations work slowly in Qemu. Exact reason is unknown. Mappings work
fast on Crosvm For DRM contexts this problem is more visible than for
Venus/Virgl.

Changelog:

v2: - Updated SDL2-dmabuf patch by making use of error_report() and
checking presense of X11+EGL in the system before making SDL2
to prefer EGL backend over GLX, suggested by Akihiko Odaki.

- Improved SDL2's dmabuf-presence check that wasn't done properly
in v1, where EGL was set up only after first console was fully
inited, and thus, SDL's display .has_dmabuf callback didn't work
for the first console. Now dmabuf support status is pre-checked
before console is registered.

- Updated commit description of the patch that fixes SDL2's context
switching logic with a more detailed explanation of the problem.
Suggested by Akihiko Odaki.

- Corrected rebase typo in the async-fencing patch and switched
async-fencing to use a sigle-linked list instead of the double,
as was suggested by Akihiko Odaki.

- Replaced "=true" with "=on" in the DRM native context documentation
example and made virtio_gpu_virgl_init() to fail with a error message
if DRM context can't be initialized instead of giving a warning
message, as was suggested by Akihiko Odaki.

- Added patchew's dependecy tag to the cover letter as was suggested by
Akihiko Odaki.

Dmitry Osipenko (5):
ui/sdl2: Restore original context after new context creation
linux-headers: Update to Linux v6.12-rc1
virtio-gpu: Handle virgl fence creation errors
virtio-gpu: Support asynchronous fencing
virtio-gpu: Support DRM native context

Pierre-Eric Pelloux-Prayer (1):
ui/sdl2: Implement dpy dmabuf functions

[PATCH v2 2/6] ui/sdl2: Implement dpy dmabuf functions

From: Pierre-Eric Pelloux-Prayer 

If EGL is used, we can rely on dmabuf to import textures without
doing copies.

To get this working on X11, we use the existing SDL hint:
SDL_HINT_VIDEO_X11_FORCE_EGL (because dmabuf can't be used with GLX).

Signed-off-by: Pierre-Eric Pelloux-Prayer 
Signed-off-by: Dmitry Osipenko 
---
 include/ui/sdl2.h |  7 ++
 ui/sdl2-gl.c  | 63 +++
 ui/sdl2.c | 31 +++
 3 files changed, 101 insertions(+)

diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index dbe6e3d9739b..9daf5ecffae7 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -45,6 +45,7 @@ struct sdl2_console {
 bool gui_keysym;
 SDL_GLContext winctx;
 QKbdState *kbd;
+bool has_dmabuf;
 #ifdef CONFIG_OPENGL
 QemuGLShader *gls;
 egl_fb guest_fb;
@@ -96,5 +97,11 @@ void sdl2_gl_scanout_texture(DisplayChangeListener *dcl,
  void *d3d_tex2d);
 void sdl2_gl_scanout_flush(DisplayChangeListener *dcl,
uint32_t x, uint32_t y, uint32_t w, uint32_t h);
+void sdl2_gl_scanout_dmabuf(DisplayChangeListener *dcl,
+QemuDmaBuf *dmabuf);
+void sdl2_gl_release_dmabuf(DisplayChangeListener *dcl,
+QemuDmaBuf *dmabuf);
+bool sdl2_gl_has_dmabuf(DisplayChangeListener *dcl);
+void sdl2_gl_console_init(struct sdl2_console *scon);
 
 #endif /* SDL2_H */
diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
index b1fe96d6af22..7612af18292c 100644
--- a/ui/sdl2-gl.c
+++ b/ui/sdl2-gl.c
@@ -26,6 +26,8 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/main-loop.h"
+#include "qemu/error-report.h"
 #include "ui/console.h"
 #include "ui/input.h"
 #include "ui/sdl2.h"
@@ -249,3 +251,64 @@ void sdl2_gl_scanout_flush(DisplayChangeListener *dcl,
 
 SDL_GL_SwapWindow(scon->real_window);
 }
+
+void sdl2_gl_scanout_dmabuf(DisplayChangeListener *dcl,
+QemuDmaBuf *dmabuf)
+{
+struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
+
+assert(scon->opengl);
+SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
+
+egl_dmabuf_import_texture(dmabuf);
+if (!qemu_dmabuf_get_texture(dmabuf)) {
+error_report("%s: failed fd=%d", __func__, qemu_dmabuf_get_fd(dmabuf));
+}
+
+sdl2_gl_scanout_texture(dcl, qemu_dmabuf_get_texture(dmabuf), false,
+qemu_dmabuf_get_width(dmabuf),
+qemu_dmabuf_get_height(dmabuf),
+0, 0,
+qemu_dmabuf_get_width(dmabuf),
+qemu_dmabuf_get_height(dmabuf),
+NULL);
+
+if (qemu_dmabuf_get_allow_fences(dmabuf)) {
+scon->guest_fb.dmabuf = dmabuf;
+}
+}
+
+void sdl2_gl_release_dmabuf(DisplayChangeListener *dcl,
+QemuDmaBuf *dmabuf)
+{
+egl_dmabuf_release_texture(dmabuf);
+}
+
+bool sdl2_gl_has_dmabuf(DisplayChangeListener *dcl)
+{
+struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
+
+return scon->has_dmabuf;
+}
+
+void sdl2_gl_console_init(struct sdl2_console *scon)
+{
+bool hidden = scon->hidden;
+
+scon->hidden = true;
+scon->surface = qemu_create_displaysurface(1, 1);
+sdl2_window_create(scon);
+
+/*
+ * QEMU checks whether console supports dma-buf before switching
+ * to the console.  To break this chicken-egg problem we pre-check
+ * dma-buf availability beforehand using a dummy SDL window.
+ */
+scon->has_dmabuf = qemu_egl_has_dmabuf();
+
+sdl2_window_destroy(scon);
+qemu_free_displaysurface(scon->surface);
+
+scon->surface = NULL;
+scon->hidden = hidden;
+}
diff --git a/ui/sdl2.c b/ui/sdl2.c
index bd4f5a9da14a..607181071b84 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -120,6 +120,9 @@ void sdl2_window_create(struct sdl2_console *scon)
 /* The SDL renderer is only used by sdl2-2D, when OpenGL is disabled */
 scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
 }
+
+qemu_egl_display = eglGetCurrentDisplay();
+
 sdl_update_caption(scon);
 }
 
@@ -820,6 +823,10 @@ static const DisplayChangeListenerOps dcl_gl_ops = {
 .dpy_gl_scanout_disable  = sdl2_gl_scanout_disable,
 .dpy_gl_scanout_texture  = sdl2_gl_scanout_texture,
 .dpy_gl_update   = sdl2_gl_scanout_flush,
+
+.dpy_gl_scanout_dmabuf   = sdl2_gl_scanout_dmabuf,
+.dpy_gl_release_dmabuf   = sdl2_gl_release_dmabuf,
+.dpy_has_dmabuf  = sdl2_gl_has_dmabuf,
 };
 
 static bool
@@ -847,6 +854,28 @@ static void sdl2_display_early_init(DisplayOptions *o)
 }
 }
 
+static void sdl2_set_hint_x11_force_egl(void)
+{
+#if defined(SDL_HINT_VIDEO_X11_FORCE_EGL) && defined(EGL_KHR_platform_x11)
+Display *x_disp = XOpenDisplay(NULL);
+EGLDisplay egl_display;
+
+if (!x_disp) {
+return;
+}
+
+/* Prefer EGL over GLX to get dma-bu

[PATCH v2 3/6] linux-headers: Update to Linux v6.12-rc1

Update kernel headers to bring new VirtIO-GPU DRM capset.

Signed-off-by: Dmitry Osipenko 
---
 include/standard-headers/drm/drm_fourcc.h |  43 
 include/standard-headers/linux/const.h|  17 ++
 include/standard-headers/linux/ethtool.h  | 226 ++
 include/standard-headers/linux/fuse.h |  22 +-
 .../linux/input-event-codes.h |   2 +
 include/standard-headers/linux/pci_regs.h |  41 +++-
 .../standard-headers/linux/virtio_balloon.h   |  16 +-
 include/standard-headers/linux/virtio_gpu.h   |   1 +
 linux-headers/asm-arm64/mman.h|   9 +
 linux-headers/asm-arm64/unistd.h  |  25 +-
 linux-headers/asm-generic/unistd.h|   6 +-
 linux-headers/asm-loongarch/kvm.h |  24 ++
 linux-headers/asm-loongarch/unistd.h  |   4 +-
 linux-headers/asm-riscv/kvm.h |   7 +
 linux-headers/asm-riscv/unistd.h  |  41 +---
 linux-headers/asm-x86/kvm.h   |   2 +
 linux-headers/asm-x86/unistd_64.h |   1 +
 linux-headers/asm-x86/unistd_x32.h|   1 +
 linux-headers/linux/bits.h|   3 +
 linux-headers/linux/const.h   |  17 ++
 linux-headers/linux/iommufd.h | 143 +--
 linux-headers/linux/kvm.h |  23 +-
 linux-headers/linux/mman.h|   1 +
 linux-headers/linux/psp-sev.h |  28 +++
 24 files changed, 610 insertions(+), 93 deletions(-)

diff --git a/include/standard-headers/drm/drm_fourcc.h 
b/include/standard-headers/drm/drm_fourcc.h
index b72917073d8d..d4a2231306e3 100644
--- a/include/standard-headers/drm/drm_fourcc.h
+++ b/include/standard-headers/drm/drm_fourcc.h
@@ -701,6 +701,31 @@ extern "C" {
  */
 #define I915_FORMAT_MOD_4_TILED_MTL_RC_CCS_CC fourcc_mod_code(INTEL, 15)
 
+/*
+ * Intel Color Control Surfaces (CCS) for graphics ver. 20 unified compression
+ * on integrated graphics
+ *
+ * The main surface is Tile 4 and at plane index 0. For semi-planar formats
+ * like NV12, the Y and UV planes are Tile 4 and are located at plane indices
+ * 0 and 1, respectively. The CCS for all planes are stored outside of the
+ * GEM object in a reserved memory area dedicated for the storage of the
+ * CCS data for all compressible GEM objects.
+ */
+#define I915_FORMAT_MOD_4_TILED_LNL_CCS fourcc_mod_code(INTEL, 16)
+
+/*
+ * Intel Color Control Surfaces (CCS) for graphics ver. 20 unified compression
+ * on discrete graphics
+ *
+ * The main surface is Tile 4 and at plane index 0. For semi-planar formats
+ * like NV12, the Y and UV planes are Tile 4 and are located at plane indices
+ * 0 and 1, respectively. The CCS for all planes are stored outside of the
+ * GEM object in a reserved memory area dedicated for the storage of the
+ * CCS data for all compressible GEM objects. The GEM object must be stored in
+ * contiguous memory with a size aligned to 64KB
+ */
+#define I915_FORMAT_MOD_4_TILED_BMG_CCS fourcc_mod_code(INTEL, 17)
+
 /*
  * Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks
  *
@@ -1475,6 +1500,7 @@ drm_fourcc_canonicalize_nvidia_format_mod(uint64_t 
modifier)
 #define AMD_FMT_MOD_TILE_VER_GFX10 2
 #define AMD_FMT_MOD_TILE_VER_GFX10_RBPLUS 3
 #define AMD_FMT_MOD_TILE_VER_GFX11 4
+#define AMD_FMT_MOD_TILE_VER_GFX12 5
 
 /*
  * 64K_S is the same for GFX9/GFX10/GFX10_RBPLUS and hence has GFX9 as 
canonical
@@ -1485,6 +1511,8 @@ drm_fourcc_canonicalize_nvidia_format_mod(uint64_t 
modifier)
 /*
  * 64K_D for non-32 bpp is the same for GFX9/GFX10/GFX10_RBPLUS and hence has
  * GFX9 as canonical version.
+ *
+ * 64K_D_2D on GFX12 is identical to 64K_D on GFX11.
  */
 #define AMD_FMT_MOD_TILE_GFX9_64K_D 10
 #define AMD_FMT_MOD_TILE_GFX9_64K_S_X 25
@@ -1492,6 +1520,21 @@ drm_fourcc_canonicalize_nvidia_format_mod(uint64_t 
modifier)
 #define AMD_FMT_MOD_TILE_GFX9_64K_R_X 27
 #define AMD_FMT_MOD_TILE_GFX11_256K_R_X 31
 
+/* Gfx12 swizzle modes:
+ *0 - LINEAR
+ *1 - 256B_2D  - 2D block dimensions
+ *2 - 4KB_2D
+ *3 - 64KB_2D
+ *4 - 256KB_2D
+ *5 - 4KB_3D   - 3D block dimensions
+ *6 - 64KB_3D
+ *7 - 256KB_3D
+ */
+#define AMD_FMT_MOD_TILE_GFX12_256B_2D 1
+#define AMD_FMT_MOD_TILE_GFX12_4K_2D 2
+#define AMD_FMT_MOD_TILE_GFX12_64K_2D 3
+#define AMD_FMT_MOD_TILE_GFX12_256K_2D 4
+
 #define AMD_FMT_MOD_DCC_BLOCK_64B 0
 #define AMD_FMT_MOD_DCC_BLOCK_128B 1
 #define AMD_FMT_MOD_DCC_BLOCK_256B 2
diff --git a/include/standard-headers/linux/const.h 
b/include/standard-headers/linux/const.h
index 1eb84b5087f8..2122610de7f9 100644
--- a/include/standard-headers/linux/const.h
+++ b/include/standard-headers/linux/const.h
@@ -28,6 +28,23 @@
 #define _BITUL(x)  (_UL(1) << (x))
 #define _BITULL(x) (_ULL(1) << (x))
 
+#if !defined(__ASSEMBLY__)
+/*
+ * Missing __asm__ support
+ *
+ * __BIT128() would not work in the __asm__ code, as it shifts an
+ * 'unsigned __init128' data type as direct representation of
+ * 12

[PATCH v2 5/6] virtio-gpu: Support asynchronous fencing

Support asynchronous fencing feature of virglrenderer. It allows Qemu to
handle fence as soon as it's signalled instead of periodically polling
the fence status. This feature is required for enabling DRM context
support in Qemu because legacy fencing mode isn't supported for DRM
contexts in virglrenderer.

Signed-off-by: Dmitry Osipenko 
---
 hw/display/virtio-gpu-gl.c |   3 +
 hw/display/virtio-gpu-virgl.c  | 134 -
 include/hw/virtio/virtio-gpu.h |  14 
 3 files changed, 133 insertions(+), 18 deletions(-)

diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 7c0e448b4661..53d938f23f20 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -170,6 +170,9 @@ static void virtio_gpu_gl_device_unrealize(DeviceState 
*qdev)
 if (gl->renderer_state >= RS_INITED) {
 #if VIRGL_VERSION_MAJOR >= 1
 qemu_bh_delete(gl->cmdq_resume_bh);
+
+virtio_gpu_virgl_reset_async_fences(g);
+qemu_bh_delete(gl->async_fence_bh);
 #endif
 if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
 timer_free(gl->print_stats);
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index b32ce44ba2b1..ad6512987079 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -891,6 +891,7 @@ static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
 void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
   struct virtio_gpu_ctrl_command *cmd)
 {
+VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
 bool cmd_suspended = false;
 int ret;
 
@@ -992,35 +993,117 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
 
 trace_virtio_gpu_fence_ctrl(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
 
-ret = virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, 0);
-if (ret)
-qemu_log_mask(LOG_GUEST_ERROR,
-  "%s: virgl_renderer_create_fence error: %s",
-  __func__, strerror(-ret));
+if (gl->context_fence_enabled &&
+(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_INFO_RING_IDX)) {
+uint32_t flags = 0;
+
+ret = virgl_renderer_context_create_fence(cmd->cmd_hdr.ctx_id, flags,
+  cmd->cmd_hdr.ring_idx,
+  cmd->cmd_hdr.fence_id);
+if (ret)
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: virgl_renderer_context_create_fence error: %s",
+  __func__, strerror(ret));
+} else {
+ret = virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, 0);
+if (ret)
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: virgl_renderer_create_fence error: %s",
+  __func__, strerror(-ret));
+}
 }
 
-static void virgl_write_fence(void *opaque, uint32_t fence)
+static void virtio_gpu_virgl_async_fence_bh(void *opaque)
 {
-VirtIOGPU *g = opaque;
+struct virtio_gpu_virgl_context_fence *f, *f_tmp;
 struct virtio_gpu_ctrl_command *cmd, *tmp;
+VirtIOGPU *g = opaque;
+VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
 
-QTAILQ_FOREACH_SAFE(cmd, &g->fenceq, next, tmp) {
-/*
- * the guest can end up emitting fences out of order
- * so we should check all fenced cmds not just the first one.
- */
-if (cmd->cmd_hdr.fence_id > fence) {
-continue;
+qemu_mutex_lock(&gl->async_fence_lock);
+
+QSLIST_FOREACH_SAFE(f, &gl->async_fenceq, next, f_tmp) {
+QTAILQ_FOREACH_SAFE(cmd, &g->fenceq, next, tmp) {
+/*
+ * the guest can end up emitting fences out of order
+ * so we should check all fenced cmds not just the first one.
+ */
+if (cmd->cmd_hdr.fence_id > f->fence_id) {
+continue;
+}
+if (cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_INFO_RING_IDX) {
+if (cmd->cmd_hdr.ring_idx != f->ring_idx) {
+continue;
+}
+if (cmd->cmd_hdr.ctx_id != f->ctx_id) {
+continue;
+}
+} else if (f->ring_idx >= 0) {
+/* ctx0 GL-query fences don't have ring info */
+continue;
+}
+virtio_gpu_ctrl_response_nodata(g, cmd, VIRTIO_GPU_RESP_OK_NODATA);
+QTAILQ_REMOVE(&g->fenceq, cmd, next);
+g_free(cmd);
 }
-trace_virtio_gpu_fence_resp(cmd->cmd_hdr.fence_id);
-virtio_gpu_ctrl_response_nodata(g, cmd, VIRTIO_GPU_RESP_OK_NODATA);
-QTAILQ_REMOVE(&g->fenceq, cmd, next);
-g_free(cmd);
+
+trace_virtio_gpu_fence_resp(f->fence_id);
+QSLIST_REMOVE(&gl->async_fenceq, f, virtio_gpu_virgl_context_fence,
+  next);
+g_free(f);
 g->inflight--;
 if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
 trace_virtio_

[PATCH v2 6/6] virtio-gpu: Support DRM native context

Add support for DRM native contexts to VirtIO-GPU. DRM context is enabled
using a new virtio-gpu-gl device option "drm=on".

Unlike Virgl and Venus contexts that operate on application API level,
DRM native contexts work on a kernel UAPI level. This lower level results
in a lightweight context implementations that yield better performance.

Signed-off-by: Dmitry Osipenko 
---
 docs/system/devices/virtio-gpu.rst | 11 +++
 hw/display/virtio-gpu-gl.c |  2 ++
 hw/display/virtio-gpu-virgl.c  | 22 ++
 hw/display/virtio-gpu.c| 15 +++
 include/hw/virtio/virtio-gpu.h |  3 +++
 5 files changed, 53 insertions(+)

diff --git a/docs/system/devices/virtio-gpu.rst 
b/docs/system/devices/virtio-gpu.rst
index b7eb0fc0e727..49a75138f7ef 100644
--- a/docs/system/devices/virtio-gpu.rst
+++ b/docs/system/devices/virtio-gpu.rst
@@ -82,6 +82,17 @@ of virtio-gpu host memory window. This is typically between 
256M and 8G.
 
 .. _venus: https://gitlab.freedesktop.org/virgl/venus-protocol/
 
+DRM native context is supported since release of `virglrenderer`_ v1.0.0
+using `drm`_ protocol. ``DRM`` virtio-gpu capability set ("capset") requires
+host blob support (``hostmem`` and ``blob`` fields) and should be enabled
+using ``drm`` field. The ``hostmem`` field specifies the size of virtio-gpu
+host memory window. This is typically between 256M and 8G.
+
+.. parsed-literal::
+-device virtio-gpu-gl,hostmem=8G,blob=on,drm=on
+
+.. _drm: https://gitlab.freedesktop.org/virgl/virglrenderer/-/tree/main/src/drm
+
 virtio-gpu rutabaga
 ---
 
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 53d938f23f20..bd0c0692a5c4 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -159,6 +159,8 @@ static Property virtio_gpu_gl_properties[] = {
 VIRTIO_GPU_FLAG_STATS_ENABLED, false),
 DEFINE_PROP_BIT("venus", VirtIOGPU, parent_obj.conf.flags,
 VIRTIO_GPU_FLAG_VENUS_ENABLED, false),
+DEFINE_PROP_BIT("drm", VirtIOGPU, parent_obj.conf.flags,
+VIRTIO_GPU_FLAG_DRM_ENABLED, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index ad6512987079..931805958ae8 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -1229,6 +1229,19 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
 if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
 flags |= VIRGL_RENDERER_VENUS | VIRGL_RENDERER_RENDER_SERVER;
 }
+if (virtio_gpu_drm_enabled(g->parent_obj.conf)) {
+flags |= VIRGL_RENDERER_DRM;
+
+if (!gl->context_fence_enabled) {
+/*
+ * Virglrenderer skips enabling DRM context support without
+ * enabled async-fence feature. VirtIO-GPU will initialize
+ * successfully, but DRM context won't be available in guest.
+ */
+error_report("DRM native context requires EGL display");
+return -EINVAL;
+}
+}
 #endif
 
 ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs);
@@ -1294,5 +1307,14 @@ GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g)
 }
 }
 
+if (virtio_gpu_drm_enabled(g->parent_obj.conf)) {
+virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_DRM,
+   &capset_max_ver,
+   &capset_max_size);
+if (capset_max_size) {
+virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_DRM);
+}
+}
+
 return capset_ids;
 }
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 0d1de7dc398c..cfd4ed8a104f 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1521,6 +1521,21 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error 
**errp)
 #endif
 }
 
+if (virtio_gpu_drm_enabled(g->parent_obj.conf)) {
+#ifdef VIRGL_VERSION_MAJOR
+#if VIRGL_VERSION_MAJOR >= 1
+if (!virtio_gpu_blob_enabled(g->parent_obj.conf) ||
+!virtio_gpu_hostmem_enabled(g->parent_obj.conf)) {
+error_setg(errp, "drm requires enabled blob and hostmem options");
+return;
+}
+#else
+error_setg(errp, "old virglrenderer, drm unsupported");
+return;
+#endif
+#endif
+}
+
 if (!virtio_gpu_base_device_realize(qdev,
 virtio_gpu_handle_ctrl_cb,
 virtio_gpu_handle_cursor_cb,
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 5673f0be85f4..fbb30d537af8 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -100,6 +100,7 @@ enum virtio_gpu_base_conf_flags {
 VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED,
 VIRTIO_GPU_FLAG_RUTABAGA_ENABLED,
 VIRTIO_GPU_FLAG_VENUS_ENABLED,
+VIRTIO_GPU_FLAG_DRM_ENABLED,
 };
 
 #define virtio_gpu_virgl_ena

[PATCH v2 4/6] virtio-gpu: Handle virgl fence creation errors

Print out error messages when virgl fence creation fails to aid debugging
of the fence-related bugs.

Signed-off-by: Dmitry Osipenko 
---
 hw/display/virtio-gpu-virgl.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index eedae7357f1a..b32ce44ba2b1 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -892,6 +892,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
   struct virtio_gpu_ctrl_command *cmd)
 {
 bool cmd_suspended = false;
+int ret;
 
 VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
 
@@ -990,7 +991,12 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
 }
 
 trace_virtio_gpu_fence_ctrl(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
-virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
+
+ret = virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, 0);
+if (ret)
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: virgl_renderer_create_fence error: %s",
+  __func__, strerror(-ret));
 }
 
 static void virgl_write_fence(void *opaque, uint32_t fence)
-- 
2.47.0

Re: [RFC QEMU PATCH v7 1/1] xen/pci: get gsi for passthrough devices

2024-10-14 Thread Chen, Jiqian

On 2024/10/15 03:34, Stewart Hildebrand wrote:
> +Edgar
> 
> On 5/16/24 06:13, Jiqian Chen wrote:
>> In PVH dom0, it uses the linux local interrupt mechanism,
>> when it allocs irq for a gsi, it is dynamic, and follow
>> the principle of applying first, distributing first. And
>> the irq number is alloced from small to large, but the
>> applying gsi number is not, may gsi 38 comes before gsi
>> 28, that causes the irq number is not equal with the gsi
>> number. And when passthrough a device, qemu wants to use
>> gsi to map pirq, xen_pt_realize->xc_physdev_map_pirq, but
>> the gsi number is got from file
>> /sys/bus/pci/devices//irq in current code, so it
>> will fail when mapping.
>>
>> Get gsi by using new function supported by Xen tools.
>>
>> Signed-off-by: Huang Rui 
>> Signed-off-by: Jiqian Chen 
> 
> I think you can safely remove the RFC tag since the Xen bits have been
> upstreamed.
Thank you! I will send a new version later this week and modify the patch 
according to your comments.

> 
>> ---
>>  hw/xen/xen-host-pci-device.c | 19 +++
>>  1 file changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
>> index 8c6e9a1716a2..2fe6a60434ba 100644
>> --- a/hw/xen/xen-host-pci-device.c
>> +++ b/hw/xen/xen-host-pci-device.c
>> @@ -10,6 +10,7 @@
>>  #include "qapi/error.h"
>>  #include "qemu/cutils.h"
>>  #include "xen-host-pci-device.h"
>> +#include "hw/xen/xen_native.h"
> 
> The inclusion order unfortunately seems to be delicate.
> "hw/xen/xen_native.h" should be before all the other xen
> includes, but after "qemu/osdep.h".
> 
>>  
>>  #define XEN_HOST_PCI_MAX_EXT_CAP \
>>  ((PCIE_CONFIG_SPACE_SIZE - PCI_CONFIG_SPACE_SIZE) / (PCI_CAP_SIZEOF + 
>> 4))
>> @@ -329,12 +330,17 @@ int xen_host_pci_find_ext_cap_offset(XenHostPCIDevice 
>> *d, uint32_t cap)
>>  return -1;
>>  }
>>  
>> +#define PCI_SBDF(seg, bus, dev, func) \
>> +uint32_t)(seg)) << 16) | \
>> +(PCI_BUILD_BDF(bus, PCI_DEVFN(dev, func
>> +
>>  void xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
>>   uint8_t bus, uint8_t dev, uint8_t func,
>>   Error **errp)
>>  {
>>  ERRP_GUARD();
>>  unsigned int v;
>> +uint32_t sdbf;
> 
> Typo: s/sdbf/sbdf/
> 
>>  
>>  d->config_fd = -1;
>>  d->domain = domain;
>> @@ -364,11 +370,16 @@ void xen_host_pci_device_get(XenHostPCIDevice *d, 
>> uint16_t domain,
>>  }
>>  d->device_id = v;
>>  
>> -xen_host_pci_get_dec_value(d, "irq", &v, errp);
>> -if (*errp) {
>> -goto error;
>> +sdbf = PCI_SBDF(domain, bus, dev, func);
>> +d->irq = xc_physdev_gsi_from_dev(xen_xc, sdbf);
> 
> This was renamed to xc_pcidev_get_gsi.
> 
> This also needs some sort of Xen interface version guard for backward
> compatibility since it's a new call introduced in Xen 4.20.
> 
>> +/* fail to get gsi, fallback to irq */
>> +if (d->irq == -1) {
>> +xen_host_pci_get_dec_value(d, "irq", &v, errp);
>> +if (*errp) {
>> +goto error;
>> +}
>> +d->irq = v;
>>  }
>> -d->irq = v;
>>  
>>  xen_host_pci_get_hex_value(d, "class", &v, errp);
>>  if (*errp) {
> 

-- 
Best regards,
Jiqian Chen.

[PATCH] Fix negative lost clock causing VM crash

2024-10-14 Thread shenjiatong

Under situation where virtual machine is running in a deployment where
the system time is unstable, there is a chance that legacy OpenStack
Windows machines without stimer enabled will crash if system time moves
backwards and diftfix=slew is enabled. This primarily caused by the fact
the system time moves faster than NTP server and after synchronization,
system time flows backwards.

Signed-off-by: shenjiatong 
---
 hw/rtc/mc146818rtc.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
index 8ccee9a385..fa5d7915b1 100644
--- a/hw/rtc/mc146818rtc.c
+++ b/hw/rtc/mc146818rtc.c
@@ -180,7 +180,6 @@ static void periodic_timer_update(MC146818RtcState *s, 
int64_t current_time,
 RTC_CLOCK_RATE, NANOSECONDS_PER_SECOND);
 last_periodic_clock = next_periodic_clock - old_period;
 lost_clock = cur_clock - last_periodic_clock;
-assert(lost_clock >= 0);
 }
 
 /*
@@ -199,10 +198,15 @@ static void periodic_timer_update(MC146818RtcState *s, 
int64_t current_time,
  */
 if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) {
 uint32_t old_irq_coalesced = s->irq_coalesced;
+if (lost_clock >= 0) {
+lost_clock += old_irq_coalesced * old_period;
+s->irq_coalesced = lost_clock / s->period;
+lost_clock %= s->period;
+} else {
+s->irq_coalesced = 0;
+lost_clock = 0;
+}
 
-lost_clock += old_irq_coalesced * old_period;
-s->irq_coalesced = lost_clock / s->period;
-lost_clock %= s->period;
 if (old_irq_coalesced != s->irq_coalesced ||
 old_period != s->period) {
 DPRINTF_C("cmos: coalesced irqs scaled from %d to %d, "
@@ -215,6 +219,7 @@ static void periodic_timer_update(MC146818RtcState *s, 
int64_t current_time,
  * no way to compensate the interrupt if LOST_TICK_POLICY_SLEW
  * is not used, we should make the time progress anyway.
  */
+lost_clock = MAX(0, lost_clock);
 lost_clock = MIN(lost_clock, period);
 }
 
-- 
2.43.0

Re: [RFC QEMU PATCH v7 1/1] xen/pci: get gsi for passthrough devices

2024-10-14 Thread Stewart Hildebrand

+Edgar

On 5/16/24 06:13, Jiqian Chen wrote:
> In PVH dom0, it uses the linux local interrupt mechanism,
> when it allocs irq for a gsi, it is dynamic, and follow
> the principle of applying first, distributing first. And
> the irq number is alloced from small to large, but the
> applying gsi number is not, may gsi 38 comes before gsi
> 28, that causes the irq number is not equal with the gsi
> number. And when passthrough a device, qemu wants to use
> gsi to map pirq, xen_pt_realize->xc_physdev_map_pirq, but
> the gsi number is got from file
> /sys/bus/pci/devices//irq in current code, so it
> will fail when mapping.
> 
> Get gsi by using new function supported by Xen tools.
> 
> Signed-off-by: Huang Rui 
> Signed-off-by: Jiqian Chen 

I think you can safely remove the RFC tag since the Xen bits have been
upstreamed.

> ---
>  hw/xen/xen-host-pci-device.c | 19 +++
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
> index 8c6e9a1716a2..2fe6a60434ba 100644
> --- a/hw/xen/xen-host-pci-device.c
> +++ b/hw/xen/xen-host-pci-device.c
> @@ -10,6 +10,7 @@
>  #include "qapi/error.h"
>  #include "qemu/cutils.h"
>  #include "xen-host-pci-device.h"
> +#include "hw/xen/xen_native.h"

The inclusion order unfortunately seems to be delicate.
"hw/xen/xen_native.h" should be before all the other xen
includes, but after "qemu/osdep.h".

>  
>  #define XEN_HOST_PCI_MAX_EXT_CAP \
>  ((PCIE_CONFIG_SPACE_SIZE - PCI_CONFIG_SPACE_SIZE) / (PCI_CAP_SIZEOF + 4))
> @@ -329,12 +330,17 @@ int xen_host_pci_find_ext_cap_offset(XenHostPCIDevice 
> *d, uint32_t cap)
>  return -1;
>  }
>  
> +#define PCI_SBDF(seg, bus, dev, func) \
> +uint32_t)(seg)) << 16) | \
> +(PCI_BUILD_BDF(bus, PCI_DEVFN(dev, func
> +
>  void xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
>   uint8_t bus, uint8_t dev, uint8_t func,
>   Error **errp)
>  {
>  ERRP_GUARD();
>  unsigned int v;
> +uint32_t sdbf;

Typo: s/sdbf/sbdf/

>  
>  d->config_fd = -1;
>  d->domain = domain;
> @@ -364,11 +370,16 @@ void xen_host_pci_device_get(XenHostPCIDevice *d, 
> uint16_t domain,
>  }
>  d->device_id = v;
>  
> -xen_host_pci_get_dec_value(d, "irq", &v, errp);
> -if (*errp) {
> -goto error;
> +sdbf = PCI_SBDF(domain, bus, dev, func);
> +d->irq = xc_physdev_gsi_from_dev(xen_xc, sdbf);

This was renamed to xc_pcidev_get_gsi.

This also needs some sort of Xen interface version guard for backward
compatibility since it's a new call introduced in Xen 4.20.

> +/* fail to get gsi, fallback to irq */
> +if (d->irq == -1) {
> +xen_host_pci_get_dec_value(d, "irq", &v, errp);
> +if (*errp) {
> +goto error;
> +}
> +d->irq = v;
>  }
> -d->irq = v;
>  
>  xen_host_pci_get_hex_value(d, "class", &v, errp);
>  if (*errp) {

Re: [PATCH v6] hw/misc/aspeed_hace: Fix SG Accumulative hashing

2024-10-14 Thread Andrew Jeffery

On Sat, 2024-10-12 at 08:20 +0200, Cédric Le Goater wrote:
> + Aspeed reviewers. Sorry about that.

All good. Seems sensible in concept and from a cursory glance, so if
you want to tack it on:

Acked-by: Andrew Jeffery

Re: [PULL v2 40/61] hw/acpi: Update GED _EVT method AML with CPU scan

2024-10-14 Thread maobibo


Hi Salil,

On 2024/10/15 上午3:59, Salil Mehta wrote:

Hi Bibo,


  From: maobibo 
  Sent: Monday, October 14, 2024 9:53 AM
  To: qemu-devel@nongnu.org; Salil Mehta 
  Cc: Michael S. Tsirkin ; Peter Maydell
  ; Salil Mehta ;
  zhukeqian ; Jonathan Cameron
  ; Gavin Shan ;
  Vishnu Pajjuri ; Xianglai Li
  ; Miguel Luis ; Shaoqin
  Huang ; Zhao Liu ; Igor
  Mammedov ; Ani Sinha 
  Subject: Re: [PULL v2 40/61] hw/acpi: Update GED _EVT method AML with
  CPU scan
  
  Hi Salil,
  
  When I debug cpu hotplug on LoongArch system, It reports error like this:

   ACPI BIOS Error (bug): Could not resolve symbol [\_SB.GED.CSCN],
  AE_NOT_FOUND
   ACPI Error: Aborting method \_SB.GED._EVT due to previous error
  (AE_NOT_FOUND)
   acpi-ged ACPI0013:00: IRQ method execution failed
  
  
  With this patch, GED CPU call method is "\\_SB.GED.CSCN", however in

  function build_cpus_aml(), its method name is "\\_SB.CPUS.CSCN".
   method = aml_method(event_handler_method, 0,
  AML_NOTSERIALIZED);
   aml_append(method, aml_call0("\\_SB.CPUS." CPU_SCAN_METHOD));
   aml_append(table, method);
  
  It seems that CPU scanning method name is not consistent between

  function build_cpus_aml() and build_ged_aml().



I believe your question stems from the following patch I've sent recently:

https://lore.kernel.org/qemu-devel/20241009031815.250096-16-salil.me...@huawei.com/

I’ve already proposed a fix for this issue. Does that not work for you?
yes, it works for me if AML_GED_EVT_CPU_SCAN_METHOD is used as parameter 
in function build_cpus_aml().


Sorry for the noise.

Regards
Bibo Mao


Thanks
Salil.


  
  Regards

  Bibo Mao
  
  On 2024/7/23 下午6:59, Michael S. Tsirkin wrote:

  > From: Salil Mehta 
  >
  > OSPM evaluates _EVT method to map the event. The CPU hotplug event
  > eventually results in start of the CPU scan. Scan figures out the CPU
  > and the kind of
  > event(plug/unplug) and notifies it back to the guest. Update the GED
  > AML _EVT method with the call to method \\_SB.CPUS.CSCN (via
  > \\_SB.GED.CSCN)
  >
  > Architecture specific code [1] might initialize its CPUs AML code by
  > calling common function build_cpus_aml() like below for ARM:
  >
  > build_cpus_aml(scope, ms, opts, xx_madt_cpu_entry,
  memmap[VIRT_CPUHP_ACPI].base,
  > "\\_SB", "\\_SB.GED.CSCN", AML_SYSTEM_MEMORY);
  >
  > [1]
  > https://lore.kernel.org/qemu-devel/20240613233639.202896-13-salil.meht
  > a...@huawei.com/
  >
  > Co-developed-by: Keqian Zhu 
  > Signed-off-by: Keqian Zhu 
  > Signed-off-by: Salil Mehta 
  > Reviewed-by: Jonathan Cameron 
  > Reviewed-by: Gavin Shan 
  > Tested-by: Vishnu Pajjuri 
  > Tested-by: Xianglai Li 
  > Tested-by: Miguel Luis 
  > Reviewed-by: Shaoqin Huang 
  > Tested-by: Zhao Liu 
  > Reviewed-by: Igor Mammedov 
  > Message-Id: <20240716111502.202344-5-salil.me...@huawei.com>
  > Reviewed-by: Michael S. Tsirkin 
  > Signed-off-by: Michael S. Tsirkin 
  > ---
  >   include/hw/acpi/generic_event_device.h | 1 +
  >   hw/acpi/generic_event_device.c | 3 +++
  >   2 files changed, 4 insertions(+)
  >
  > diff --git a/include/hw/acpi/generic_event_device.h
  > b/include/hw/acpi/generic_event_device.h
  > index e091ac2108..40af3550b5 100644
  > --- a/include/hw/acpi/generic_event_device.h
  > +++ b/include/hw/acpi/generic_event_device.h
  > @@ -87,6 +87,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState,
  ACPI_GED)
  >   #define GED_DEVICE  "GED"
  >   #define AML_GED_EVT_REG "EREG"
  >   #define AML_GED_EVT_SEL "ESEL"
  > +#define AML_GED_EVT_CPU_SCAN_METHOD "\\_SB.GED.CSCN"
  >
  >   /*
  >* Platforms need to specify the GED event bitmap diff --git
  > a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
  > index 4641933a0f..15b4c3ebbf 100644
  > --- a/hw/acpi/generic_event_device.c
  > +++ b/hw/acpi/generic_event_device.c
  > @@ -108,6 +108,9 @@ void build_ged_aml(Aml *table, const char *name,
  HotplugHandler *hotplug_dev,
  >   aml_append(if_ctx, aml_call0(MEMORY_DEVICES_CONTAINER
  "."
  >MEMORY_SLOT_SCAN_METHOD));
  >   break;
  > +case ACPI_GED_CPU_HOTPLUG_EVT:
  > +aml_append(if_ctx,
  aml_call0(AML_GED_EVT_CPU_SCAN_METHOD));
  > +break;
  >   case ACPI_GED_PWR_DOWN_EVT:
  >   aml_append(if_ctx,
  >
  > aml_notify(aml_name(ACPI_POWER_BUTTON_DEVICE),
  >

[RFC v3 0/2] target/riscv: add endianness checks and atomicity guarantees.

2024-10-14 Thread Paolo Savini

This version 3 of the patch adds endianness safety to both the optimizations
brought by the patch set.
It also adds some conditions that allow the __builtin_memcpy to be executed
on chunks of 16 bytes with guarantee of atomicity.

Changes from V2:
- patch 1:
  - add condition for the host not to be big endian.
- patch 2:
  - add condition for the host not to be big endian.
  - add condition for the host to support 16-byte atomic memory operations.
  - limit the large loads and stores to 16 byte chunks in order to guarantee
atomicity on a larger range of processors.

Cc: Richard Handerson 
Cc: Palmer Dabbelt 
Cc: Alistair Francis 
Cc: Bin Meng 
Cc: Weiwei Li 
Cc: Daniel Henrique Barboza 
Cc: Liu Zhiwei 
Cc: Helene Chelin 
Cc: Nathan Egge 
Cc: Max Chou 

Helene CHELIN (1):
  target/riscv: rvv: reduce the overhead for simple RISC-V vector
unit-stride loads and stores

Paolo Savini (1):
  target/riscv: rvv: improve performance of RISC-V vector loads and
stores on large amounts of data.

 target/riscv/vector_helper.c | 61 +++-
 1 file changed, 60 insertions(+), 1 deletion(-)

-- 
2.34.1

[RFC v3 1/2] target/riscv: rvv: reduce the overhead for simple RISC-V vector unit-stride loads and stores

2024-10-14 Thread Paolo Savini

From: Helene CHELIN 

This patch improves the performance of the emulation of the RVV unit-stride
loads and stores in the following cases:

- when the data being loaded/stored per iteration amounts to 8 bytes or less.
- when the vector length is 16 bytes (VLEN=128) and there's no grouping of the
  vector registers (LMUL=1).

The optimization consists of avoiding the overhead of probing the RAM of the
host machine and doing a loop load/store on the input data grouped in chunks
of as many bytes as possible (8,4,2,1 bytes).

Co-authored-by: Helene CHELIN 
Co-authored-by: Paolo Savini 

Signed-off-by: Helene CHELIN 
---
 target/riscv/vector_helper.c | 47 
 1 file changed, 47 insertions(+)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 4479726acf..75c24653f0 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -635,6 +635,53 @@ vext_ldst_us(void *vd, target_ulong base, CPURISCVState 
*env, uint32_t desc,
 
 VSTART_CHECK_EARLY_EXIT(env);
 
+#if defined(CONFIG_USER_ONLY) && !HOST_BIG_ENDIAN
+/* For data sizes <= 64 bits and for LMUL=1 with VLEN=128 bits we get a
+ * better performance by doing a simple simulation of the load/store
+ * without the overhead of prodding the host RAM */
+if ((nf == 1) && ((evl << log2_esz) <= 8 ||
+   ((vext_lmul(desc) == 0) && (simd_maxsz(desc) == 16 {
+
+   uint32_t evl_b = evl << log2_esz;
+
+for (uint32_t j = env->vstart; j < evl_b;) {
+   addr = base + j;
+if ((evl_b - j) >= 8) {
+if (is_load)
+lde_d_tlb(env, adjust_addr(env, addr), j, vd, ra);
+else
+ste_d_tlb(env, adjust_addr(env, addr), j, vd, ra);
+j += 8;
+}
+else if ((evl_b - j) >= 4) {
+if (is_load)
+lde_w_tlb(env, adjust_addr(env, addr), j, vd, ra);
+else
+ste_w_tlb(env, adjust_addr(env, addr), j, vd, ra);
+j += 4;
+}
+else if ((evl_b - j) >= 2) {
+if (is_load)
+lde_h_tlb(env, adjust_addr(env, addr), j, vd, ra);
+else
+ste_h_tlb(env, adjust_addr(env, addr), j, vd, ra);
+j += 2;
+}
+else {
+if (is_load)
+lde_b_tlb(env, adjust_addr(env, addr), j, vd, ra);
+else
+ste_b_tlb(env, adjust_addr(env, addr), j, vd, ra);
+j += 1;
+}
+}
+
+env->vstart = 0;
+vext_set_tail_elems_1s(evl, vd, desc, nf, esz, max_elems);
+return;
+}
+#endif
+
 vext_cont_ldst_elements(&info, base, env->vreg, env->vstart, evl, desc,
 log2_esz, false);
 /* Probe the page(s).  Exit with exception for any invalid page. */
-- 
2.34.1

[RFC v3 2/2] target/riscv: rvv: improve performance of RISC-V vector loads and stores on large amounts of data.

2024-10-14 Thread Paolo Savini

This patch optimizes the emulation of unit-stride load/store RVV instructions
when the data being loaded/stored per iteration amounts to 64 bytes or more.
The optimization consists of calling __builtin_memcpy on chunks of data of 128
bytes between the memory address of the simulated vector register and the
destination memory address and vice versa.
This is done only if we have direct access to the RAM of the host machine,
if the host is little endiand and if it supports atomic 128 bit memory
operations.

Signed-off-by: Paolo Savini 
---
 target/riscv/vector_helper.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 75c24653f0..b3d0be8e39 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -488,7 +488,19 @@ vext_group_ldst_host(CPURISCVState *env, void *vd, 
uint32_t byte_end,
 }
 
 fn = fns[is_load][group_size];
-fn(vd, byte_offset, host + byte_offset);
+
+/* x86 and AMD processors provide strong guarantees of atomicity for
+ * 16-byte memory operations if the memory operands are 16-byte aligned */
+if (!HOST_BIG_ENDIAN && (byte_offset + 16 < byte_end) && ((byte_offset % 
16) == 0) &&
+((cpuinfo & (CPUINFO_ATOMIC_VMOVDQA | CPUINFO_ATOMIC_VMOVDQU)) != 0)) {
+  group_size = MO_128;
+  if (is_load)
+__builtin_memcpy((uint8_t *)(vd + byte_offset), (uint8_t *)(host + 
byte_offset), 16);
+  else
+__builtin_memcpy((uint8_t *)(host + byte_offset), (uint8_t *)(vd + 
byte_offset), 16);
+} else {
+  fn(vd, byte_offset, host + byte_offset);
+}
 
 return 1 << group_size;
 }
-- 
2.34.1

Re: [PATCH] tcg: remove singlestep_enabled from DisasContextBase


On 10/10/24 05:36, Paolo Bonzini wrote:

It is used in a couple of places only, both within the same target.  Those can
use the cflags just as well, so remove the separate field.

Signed-off-by: Paolo Bonzini 
---
  include/exec/translator.h   | 2 --
  accel/tcg/translator.c  | 1 -
  target/mips/tcg/translate.c | 5 +++--
  3 files changed, 3 insertions(+), 5 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé

Re: [PATCH v2 3/3] target/i386: Remove ra parameter from ptw_translate


On 13/10/24 15:47, Richard Henderson wrote:

This argument is no longer used.

Suggested-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
  target/i386/tcg/sysemu/excp_helper.c | 18 +-
  1 file changed, 9 insertions(+), 9 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé

Re: [PATCH v2 10/16] target/mips: Replace MO_TE by mo_endian()


On 13/10/24 13:05, Richard Henderson wrote:

On 10/10/24 14:50, Philippe Mathieu-Daudé wrote:

+++ b/target/mips/tcg/msa_helper.c
@@ -8213,7 +8213,7 @@ void helper_msa_ffint_u_df(CPUMIPSState *env, 
uint32_t df, uint32_t wd,

  #if !defined(CONFIG_USER_ONLY)
  #define 
MEMOP_IDX(DF)   \
-    MemOpIdx oi = make_memop_idx(MO_TE | DF | 
MO_UNALN, \
+    MemOpIdx oi = make_memop_idx(mo_endian(dc) | DF | 
MO_UNALN, \

   mips_env_mmu_index(env));
  #else


This one is not within a translation context.
Surely this should be mo_endian_env().

I would have expected this not to compile?


Dead code since commit 948f88661c6 ("target/mips: Use cpu_*_data_ra
for msa load/store"):

$ git grep -w MEMOP_IDX
target/mips/tcg/msa_helper.c:8215:#define MEMOP_IDX(DF) 
 \

target/mips/tcg/msa_helper.c:8219:#define MEMOP_IDX(DF)

I'll send a cleanup patch removing the #define lines.



The rest of the changes appear correct, based on filenames.


Might I use your R-b tag on this patch, removing the tcg/msa_helper.c 
change?


Regards,

Phil.

Re: [PATCH] target/mips: Remove unused MEMOP_IDX() macro


On 10/14/24 16:22, Philippe Mathieu-Daudé wrote:

MEMOP_IDX() is unused since commit 948f88661c6 ("target/mips:
Use cpu_*_data_ra for msa load/store"), remove it.

Signed-off-by: Philippe Mathieu-Daudé
---
  target/mips/tcg/msa_helper.c | 8 
  1 file changed, 8 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [RFC PATCH 00/23] accel/tcg: Convert victim tlb to IntervalTree


On 10/9/24 17:50, Pierrick Bouvier wrote:

Eventually fixing the page size > TARGET_PAGE_SIZE performance issues.

E.g. with a 16k or 64k aarch64 guest kernel, we still have TARGET_PAGE_SIZE at 
4k, so all
guest pages are "large", and so run into our current behaviour of flushing the 
entire tlb
too often.

Even without that, I expect further cleanups to improve performance, we're just not 
there yet.



r~



Does merging pages over a given range be something we could benefit from too? In this 
case, entries in our tlbtree would have varying size, allowing us to cover more space with 
a single entry.


I don't know.  I kinda doubt it, because of tlb flushing mechanics.
But we shall see once everything up until that point is done.


r~

Re: [PATCH V1 0/4] Arch agnostic ACPI changes to support vCPU Hotplug (on Archs like ARM)

2024-10-14 Thread maobibo

With cpu-add/cpu-del command tested on LoongArch system, no migration 
tested. There is no negative influence with LoongArch cpu hotplug.


Regards
Bibo Mao

On 2024/10/15 上午3:22, Salil Mehta via wrote:

Certain CPU architecture specifications [1][2][3] prohibit changes to the CPUs
*presence* after the kernel has booted. This is because many system
initializations depend on the exact CPU count at boot time and do not expect it
to change afterward. For example, components like interrupt controllers that are
closely coupled with CPUs, or various per-CPU features, may not support
configuration changes once the kernel has been initialized.

This requirement poses a challenge for virtualization features like vCPU
hotplug. To address this, changes to the ACPI AML are necessary to update the
`_STA.PRES` (presence) and `_STA.ENA` (enabled) bits accordingly during guest
initialization, as well as when vCPUs are hot-plugged or hot-unplugged. The
presence of unplugged vCPUs may need to be deliberately *simulated* at the ACPI
level to maintain a *persistent* view of vCPUs for the guest kernel.

This patch set introduces the following features:

1. ACPI Interface with Explicit PRESENT and ENABLED CPU States: It allows the
guest kernel to evaluate these states using the `_STA` ACPI method.

2. Initialization of ACPI CPU States: These states are initialized during

`machvirt_init` and when vCPUs are hot-(un)plugged. This enables 
hotpluggable
vCPUs to be exposed to the guest kernel via ACPI.

3. Support for Migrating ACPI CPU States: The patch set ensures the migration of
the newly introduced `is_{present,enabled}` ACPI CPU states to the
destination VM.

The approach is flexible enough to accommodate ARM-like architectures that
intend to implement vCPU hotplug functionality. It is suitable for architectures
facing similar constraints to ARM or those that plan to implement vCPU
hotplugging independently of hardware support (if available).

This patch set is derived from the ARM-specific vCPU hotplug implementation [4]
and includes migration components adaptable to other architectures, following
suggestions [5] made by Igor Mammedov .

It can be applied independently, ensuring compatibility with existing hotplug
support in other architectures. I have tested this patch set in conjunction with
the ARM-specific vCPU hotplug changes (included in the upcoming RFC V5 [6]), and
everything worked as expected. I kindly request maintainers of other
architectures to provide a "Tested-by" after running their respective regression
tests.

Many thanks!


References:
[1] KVMForum 2023 Presentation: Challenges Revisited in Supporting Virt CPU 
Hotplug on
 architectures that don’t Support CPU Hotplug (like ARM64)
 a. Kernel Link: 
https://kvm-forum.qemu.org/2023/KVM-forum-cpu-hotplug_7OJ1YyJ.pdf
 b. Qemu Link:  
https://kvm-forum.qemu.org/2023/Challenges_Revisited_in_Supporting_Virt_CPU_Hotplug_-__ii0iNb3.pdf
[2] KVMForum 2020 Presentation: Challenges in Supporting Virtual CPU Hotplug on
 SoC Based Systems (like ARM64)
 Link: https://kvmforum2020.sched.com/event/eE4m
[3] Check comment 5 in the bugzilla entry
 Link: https://bugzilla.tianocore.org/show_bug.cgi?id=4481#c5
[4] [PATCH RFC V4 00/33] Support of Virtual CPU Hotplug for ARMv8 Arch
 Link: 
https://lore.kernel.org/qemu-devel/20241009031815.250096-1-salil.me...@huawei.com/T/#mf32be203baa568a871dc625b732f666a4c4f1e68
[5] Architecture agnostic ACPI VMSD state migration (Discussion)
 Link: 
https://lore.kernel.org/qemu-devel/20240715155436.577d3...@imammedo.users.ipa.redhat.com/
[6] Upcoming RFC V5, Support of Virtual CPU Hotplug for ARMv8 Arch
 Link: https://github.com/salil-mehta/qemu/commits/virt-cpuhp-armv8/rfc-v5

Salil Mehta (4):
   hw/acpi: Initialize ACPI Hotplug CPU Status with Support for vCPU
 `Persistence`
   hw/acpi: Update ACPI CPU Status `is_{present, enabled}` during vCPU
 hot(un)plug
   hw/acpi: Reflect ACPI vCPU {present,enabled} states in ACPI
 _STA.{PRES,ENA} Bits
   hw/acpi: Populate vCPU Hotplug VMSD to migrate `is_{present,enabled}`
 states

  cpu-target.c patches.vcpuhp.rfc-v5.arch.agnostic.acpi  |  1 +
  hw/acpi/cpu.c  | 70 +++---
  hw/acpi/generic_event_device.c | 11 ++
  include/hw/acpi/cpu.h  | 21 ++
  include/hw/core/cpu.h  | 21 ++
  5 files changed, 119 insertions(+), 5 deletions(-)

Re: [PATCH] migration: Remove interface query-migrationthreads

2024-10-14 Thread Daniel P . Berrangé

On Fri, Oct 11, 2024 at 11:34:17AM -0400, Peter Xu wrote:
> This reverts two commits:
> 
> 671326201dac8fe91222ba0045709f04a8ec3af4
> 1b1f4ab69c41279a45ccd0d3178e83471e6e4ec1
> 
> Meanwhile it adds an entry to removed-features.rst for the
> query-migrationthreads QMP command.
> 
> This patch originates from another patchset [1] that wanted to cleanup the
> interface and add corresponding HMP command, as lots of things are missing
> in the query report; so far it only reports the main thread and multifd
> sender threads; all the rest migration threads are not reported, including
> multifd recv threads.
> 
> As pointed out by Dan in the follow up discussions [1], the API is designed
> in an awkward way where CPU pinning may not cover the whole lifecycle of
> even the thread being reported.  When asked, we also didn't get chance to
> hear from the developer who introduced this feature to explain how this API
> can be properly used.
> 
> OTOH, this feature from debugging POV isn't very helpful either, as all
> these information can be easily obtained by GDB.  Esepcially, if with
> "-name $VM,debug-threads=on" we do already have names for each migration
> threads (which covers more than multifd sender threads).
> 
> So it looks like the API isn't helpful in any form as of now, besides it
> only adds maintenance burden to migration code, even if not much.
> 
> Considering that so far there's totally no justification on how to use this
> interface correctly, let's remove this interface instead of cleaning it up.
> 
> In this special case, we even go beyond normal deprecation procedure,
> because a deprecation process would only make sense when there are existing
> users. In this specific case, we expect zero serious users with this API.

We have no way of knowing whether there are existing users of this, or
any other feature in QEMU. This is why we have a formal deprecation
period, rather than immediately deleting existing features.

Yes, there are plenty of reasons why this feature is sub-optimal, but
it is not broken to the extent that it is *impossible* for people to
be using it.

IOW, I don't see that there's anything special here to justify bypassing
our deprecation process here.


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

[RFC PATCH 4/4] tests/tcg/aarch64: add system test for FEAT_XS

Add system test to make sure FEAT_XS is enabled for max cpu emulation
and that QEMU doesn't crash when encountering an NXS instruction
variant.

Signed-off-by: Manos Pitsidianakis 
---
 tests/tcg/aarch64/system/feat-xs.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/tests/tcg/aarch64/system/feat-xs.c 
b/tests/tcg/aarch64/system/feat-xs.c
new file mode 100644
index 
..52a481c577f9420fa2f6d6a794c1f26772cb4bff
--- /dev/null
+++ b/tests/tcg/aarch64/system/feat-xs.c
@@ -0,0 +1,27 @@
+/*
+ * FEAT_XS Test
+ *
+ * Copyright (c) 2024 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include 
+#include 
+
+int main(void)
+{
+uint64_t isar1;
+
+asm volatile ("mrs %0, id_aa64isar1_el1" : "=r"(isar1));
+if (((isar1 >> 56) & (0xff)) != 1) {
+ml_printf("FEAT_XS not supported by CPU");
+return 1;
+}
+/* VMALLE1NXS */
+asm volatile (".inst 0xd508971f");
+/* VMALLE1OSNXS */
+asm volatile (".inst 0xd508911f");
+
+return 0;
+}

-- 
2.45.2

[RFC PATCH 1/4] arm: Add FEAT_XS's TLBI NXS variants

Signed-off-by: Manos Pitsidianakis 
---
 target/arm/cpu-features.h |   5 +
 target/arm/helper.c   | 366 +++---
 2 files changed, 218 insertions(+), 153 deletions(-)

diff --git a/target/arm/cpu-features.h b/target/arm/cpu-features.h
index 
04ce2818263e2c3b99c59940001b65302e1d26d2..b4dcd429c3540e18c44d3c30f82f030be45719f2
 100644
--- a/target/arm/cpu-features.h
+++ b/target/arm/cpu-features.h
@@ -970,6 +970,11 @@ static inline bool isar_feature_aa64_sme_fa64(const 
ARMISARegisters *id)
 return FIELD_EX64(id->id_aa64smfr0, ID_AA64SMFR0, FA64);
 }
 
+static inline bool isar_feature_aa64_xs(const ARMISARegisters *id)
+{
+return FIELD_SEX64(id->id_aa64isar1, ID_AA64ISAR1, XS) >= 0;
+}
+
 /*
  * Feature tests for "does this exist in either 32-bit or 64-bit?"
  */
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 
3f77b40734f2db831254a0e4eb205751aec0d1e5..3104a2d1dab6e58bf454c75afd478ec6d5fe521f
 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5671,98 +5671,111 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
   .fgt = FGT_DCCISW,
   .access = PL1_W, .accessfn = access_tsw, .type = ARM_CP_NOP },
 /* TLBI operations */
-{ .name = "TLBI_VMALLE1IS", .state = ARM_CP_STATE_AA64,
+#define TLBI(name, opc0, opc1, crn, crm, opc2, access, accessfn, type, fgt, \
+ writefn)   \
+{ name, .state = ARM_CP_STATE_AA64, opc0, opc1, crn, crm, opc2, access, \
+  accessfn, type, fgt, writefn },   \
+{ name"NXS", .state = ARM_CP_STATE_AA64, opc0, opc1, crn + 1, crm, opc2,\
+   access, accessfn, type, fgt, writefn }
+ TLBI(.name = "TLBI_VMALLE1IS",
   .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 0,
   .access = PL1_W, .accessfn = access_ttlbis, .type = ARM_CP_NO_RAW,
   .fgt = FGT_TLBIVMALLE1IS,
-  .writefn = tlbi_aa64_vmalle1is_write },
-{ .name = "TLBI_VAE1IS", .state = ARM_CP_STATE_AA64,
+  .writefn = tlbi_aa64_vmalle1is_write),
+ TLBI(.name = "TLBI_VAE1IS",
   .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 1,
   .access = PL1_W, .accessfn = access_ttlbis, .type = ARM_CP_NO_RAW,
   .fgt = FGT_TLBIVAE1IS,
-  .writefn = tlbi_aa64_vae1is_write },
-{ .name = "TLBI_ASIDE1IS", .state = ARM_CP_STATE_AA64,
+  .writefn = tlbi_aa64_vae1is_write),
+ TLBI(.name = "TLBI_ASIDE1IS",
   .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 2,
   .access = PL1_W, .accessfn = access_ttlbis, .type = ARM_CP_NO_RAW,
   .fgt = FGT_TLBIASIDE1IS,
-  .writefn = tlbi_aa64_vmalle1is_write },
-{ .name = "TLBI_VAAE1IS", .state = ARM_CP_STATE_AA64,
+  .writefn = tlbi_aa64_vmalle1is_write),
+ TLBI(.name = "TLBI_VAAE1IS",
   .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 3,
   .access = PL1_W, .accessfn = access_ttlbis, .type = ARM_CP_NO_RAW,
   .fgt = FGT_TLBIVAAE1IS,
-  .writefn = tlbi_aa64_vae1is_write },
-{ .name = "TLBI_VALE1IS", .state = ARM_CP_STATE_AA64,
+  .writefn = tlbi_aa64_vae1is_write),
+ TLBI(.name = "TLBI_VALE1IS",
   .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 5,
   .access = PL1_W, .accessfn = access_ttlbis, .type = ARM_CP_NO_RAW,
   .fgt = FGT_TLBIVALE1IS,
-  .writefn = tlbi_aa64_vae1is_write },
-{ .name = "TLBI_VAALE1IS", .state = ARM_CP_STATE_AA64,
+  .writefn = tlbi_aa64_vae1is_write),
+ TLBI(.name = "TLBI_VAALE1IS",
   .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 7,
   .access = PL1_W, .accessfn = access_ttlbis, .type = ARM_CP_NO_RAW,
   .fgt = FGT_TLBIVAALE1IS,
-  .writefn = tlbi_aa64_vae1is_write },
-{ .name = "TLBI_VMALLE1", .state = ARM_CP_STATE_AA64,
+  .writefn = tlbi_aa64_vae1is_write),
+ TLBI(.name = "TLBI_VMALLE1",
   .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 7, .opc2 = 0,
   .access = PL1_W, .accessfn = access_ttlb, .type = ARM_CP_NO_RAW,
   .fgt = FGT_TLBIVMALLE1,
-  .writefn = tlbi_aa64_vmalle1_write },
-{ .name = "TLBI_VAE1", .state = ARM_CP_STATE_AA64,
+  .writefn = tlbi_aa64_vmalle1_write),
+ TLBI(.name = "TLBI_VAE1",
   .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 7, .opc2 = 1,
   .access = PL1_W, .accessfn = access_ttlb, .type = ARM_CP_NO_RAW,
   .fgt = FGT_TLBIVAE1,
-  .writefn = tlbi_aa64_vae1_write },
-{ .name = "TLBI_ASIDE1", .state = ARM_CP_STATE_AA64,
+  .writefn = tlbi_aa64_vae1_write),
+ TLBI(.name = "TLBI_ASIDE1",
   .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 7, .opc2 = 2,
   .access = PL1_W, .accessfn = access_ttlb, .type = ARM_CP_NO_RAW,
   .fgt = FGT_TLBIASIDE1,
-  .writefn = tlbi_aa64_vmalle1_write },
-{ .name = "TLBI_VAAE1", .state = ARM_CP_STATE_AA64,
+  .writefn = tlbi_aa64_vmalle1_write),
+ TLBI(.name = "TLBI_VAAE1",
   .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 7, .opc2 = 3,
   .access = PL1_W, .accessfn = access_ttlb, .type = ARM_CP_NO_RAW,
   .fgt = FGT_TLBIVAAE1,
-  .writef

[RFC PATCH 0/4] No-op support for Arm FEAT_XS, feedback needed

This series is an initial incomplete attempt at adding support for the
FEAT_XS feature in aarch64 TCG. This feature was introduced in ARMv8.7:
it adds a new memory attribute XS which indicates that a memory access
could take longer than usual to complete and also adds instruction
variants for TLBI maintenance and DSB.

These variants are implemented as no-ops, since QEMU TCG doesn't
implement caching.

This is my first foray into TCG and certain things weren't clear to me:

1. How to make sure the feature is implemented properly. Since we model
   cache maintenance as no-ops my understanding is the only
   functionality we need to provide is to expose the FEAT_XS feature bit
   and also make sure the nXS variants trap properly if configured with
   fine-grained traps.
2. Is there a point in adding a TCG test? If I read the manual
   correctly, the nXS variants should trap to the undefined instruction
   vector if unimplemented.

These patches lack support for FGT for now.

Signed-off-by: Manos Pitsidianakis 
---
Manos Pitsidianakis (4):
  arm: Add FEAT_XS's TLBI NXS variants
  arm/tcg: add decodetree entry for DSB nXS variant
  arm/tcg/cpu64: add FEAT_XS feat in max cpu
  tests/tcg/aarch64: add system test for FEAT_XS

 target/arm/cpu-features.h  |   5 +
 target/arm/helper.c| 366 +
 target/arm/tcg/a64.decode  |   3 +
 target/arm/tcg/cpu64.c |   1 +
 target/arm/tcg/translate-a64.c |   6 +
 tests/tcg/aarch64/system/feat-xs.c |  27 +++
 6 files changed, 255 insertions(+), 153 deletions(-)
---
base-commit: 7e3b6d8063f245d27eecce5aabe624b5785f2a77
change-id: 20240919-arm-feat-xs-73eedb23d937

--
γαῖα πυρί μιχθήτω

[RFC PATCH 3/4] arm/tcg/cpu64: add FEAT_XS feat in max cpu

Add FEAT_XS feature report value in max cpu's ID_AA64ISAR1 sys register.

Signed-off-by: Manos Pitsidianakis 
---
 target/arm/tcg/cpu64.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/tcg/cpu64.c b/target/arm/tcg/cpu64.c
index 
0168920828651492b1114d66ab0fc72c20dda2a8..8c8f88d84151952872f1b1987e98d789b501fb23
 100644
--- a/target/arm/tcg/cpu64.c
+++ b/target/arm/tcg/cpu64.c
@@ -1163,6 +1163,7 @@ void aarch64_max_tcg_initfn(Object *obj)
 t = FIELD_DP64(t, ID_AA64ISAR1, BF16, 2); /* FEAT_BF16, FEAT_EBF16 */
 t = FIELD_DP64(t, ID_AA64ISAR1, DGH, 1);  /* FEAT_DGH */
 t = FIELD_DP64(t, ID_AA64ISAR1, I8MM, 1); /* FEAT_I8MM */
+t = FIELD_DP64(t, ID_AA64ISAR1, XS, 1);   /* FEAT_XS */
 cpu->isar.id_aa64isar1 = t;
 
 t = cpu->isar.id_aa64isar2;

-- 
2.45.2

[RFC PATCH 2/4] arm/tcg: add decodetree entry for DSB nXS variant

The DSB nXS variant is always both a reads and writes request type.
Ignore the domain field like we do in plain DSB and perform a full
system barrier operation.

The DSB nXS variant is part of FEAT_XS made mandatory from Armv8.7.

Signed-off-by: Manos Pitsidianakis 
---
 target/arm/tcg/a64.decode  | 3 +++
 target/arm/tcg/translate-a64.c | 6 ++
 2 files changed, 9 insertions(+)

diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
index 
331a8e180c0b14e2abe3ec641a867235574316f7..c4f516abc18224932082cdf3e7530edc7a304bc1
 100644
--- a/target/arm/tcg/a64.decode
+++ b/target/arm/tcg/a64.decode
@@ -245,6 +245,9 @@ WFIT1101 0101  0011 0001  001 rd:5
 
 CLREX   1101 0101  0011 0011  010 1
 DSB_DMB 1101 0101  0011 0011 domain:2 types:2 10- 1
+# For the DSB nXS variant, types always equals MBReqTypes_All and we ignore the
+# domain bits.
+DSB_nXS 1101 0101  0011 0011 -- 10 001 1
 ISB 1101 0101  0011 0011  110 1
 SB  1101 0101  0011 0011  111 1
 
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 
071b6349fc38802a62f4b4056e369c4d8b1ecf94..85e71599203eee62b4d22a0b10ed676cc815dab6
 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -1959,6 +1959,12 @@ static bool trans_DSB_DMB(DisasContext *s, arg_DSB_DMB 
*a)
 return true;
 }
 
+static bool trans_DSB_nXS(DisasContext *ctx, arg_DSB_nXS *a)
+{
+tcg_gen_mb(TCG_BAR_SC | TCG_MO_ALL);
+return true;
+}
+
 static bool trans_ISB(DisasContext *s, arg_ISB *a)
 {
 /*

-- 
2.45.2

Re: [EXT] Re: [PATCH v1] hw/cxl: Fix background completion percentage calculation

2024-10-14 Thread ajay . opensrc

>On Mon, 14 Oct 2024 10:32 +0530 
> wrote:
>
>On Sat, 14 Sep 2024 16:50:21 +0530
> wrote:
>
>> From: Ajay Joshi 
>>
>> The current completion percentage calculation
>> does not account for the relative time since
>> the start of the background activity, this leads
>> to showing incorrect start percentage vs what has
>> actually been completed.
>>
>> This patch calculates the percentage based on the actual
>> elapsed time since the start of the operation.
>>
>> Fixes: 221d2cfbdb ("hw/cxl/mbox: Add support for background operations")
>>
>I'll include this is a fixes series I send to Michael + list later
>today.  However for future reference, no line break between tags in
>the tags block as it breaks some scripting.  I'll tidy that up.
>Note I think you missed Michael's point about this on the first version.
>+ as a second version, even without changes, this should have been v2.
>

Thanks Jonathan! Got it.
Sorry missed the versioning, will be more careful about it.

>
>Thanks
>
>Jonathan
>
>> Signed-off-by: Ajay Joshi 
>> Reviewed-by: Davidlohr Bueso 
>> Acked-by: Jonathan Cameron 
>> ---
>>  hw/cxl/cxl-mailbox-utils.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>> index c2ed251bb3..873d60c069 100644
>> --- a/hw/cxl/cxl-mailbox-utils.c
>> +++ b/hw/cxl/cxl-mailbox-utils.c
>> @@ -2708,7 +2708,8 @@ static void bg_timercb(void *opaque)
>>  }
>>  } else {
>>  /* estimate only */
>> -cci->bg.complete_pct = 100 * now / total_time;
>> +cci->bg.complete_pct =
>> +100 * (now - cci->bg.starttime) / cci->bg.runtime;
>>  timer_mod(cci->bg.timer, now + CXL_MBOX_BG_UPDATE_FREQ);
>>  }
>>

Re: possible bug in recent crypto patches in master branch

2024-10-14 Thread Daniel P . Berrangé

On Sun, Oct 13, 2024 at 10:32:36PM +0600, Dorjoy Chowdhury wrote:
> Hi,
> I think there maybe some bugs caused by the recent crypto patches that got
> merged to master. ref:
> https://lore.kernel.org/qemu-devel/cafeaca-e_1wflun2hpttt2bszxksmbnxkak_uzuhwrh_fb6...@mail.gmail.com/T/#t
> 
> I think before these patches the "qcrypto_hash_bytes" or
> "qcrypto_hash_bytesv" apis used to behave such that a user of the apis
> could pass his own allocated buffers. In that case, the passed buffers
> would be used to fill in the hash result instead of allocating a new
> buffer. But I think in the master branch now, these apis always allocate
> the result buffer regardless of it's users passing their own buffers or
> not. So this is problematic for wherever the users of the apis are passing
> their own allocated buffers. For example, I see target/i386/sev.c is one
> such api user. Am I missing something here or does this look like it's a
> clear bug?
> 
> If I am correct, I think it makes sense to keep the old behavior of the
> apis where new buffers are not allocated if the user supplies one (I think
> it probably also makes sense if we force the users to always provide the
> bufffer instead of the apis themselves allocating them). I noticed this bug
> in the nitro-enclave work I am doing where rebasing the branch builds but
> the actual behavior is buggy because of this new change of the api
> implementations.

Yes, you're right. The behaviour has changed.

Although the API docs suggested that the 'result' buffer was allocated
internally, they did not explicitly require that, and our impl did
lazy allocation.

I'll work on fixing this and adding test cases for it

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

Re: [PULL v3 00/18] Rust initial PoC + meson changes for 2024-10-07

On Mon, 14 Oct 2024 at 11:12, Peter Maydell  wrote:
>
> On Fri, 11 Oct 2024 at 18:13, Paolo Bonzini  wrote:
> > v2->v3: new patches
> > - scripts/archive-source: find directory name for subprojects
> > - docs: fix invalid footnote syntax
> > - docs: avoid footnotes consisting of just URLs
> > - docs: use consistent markup for footnotes
> >
> > 
> > * first commit for Rust support
> > * add CI job using Fedora + Rust nightly
> > * fix detection of ATOMIC128 on x86_64
> > * fix compilation with Sphinx 8.1.0

> Applied, thanks.

With this applied, I find that on one of my personal
local dev branches an incremental rebuild fails, because
meson complains about not finding a new enough bindgen,
even though I did not --enable-rust. Meson also complains
about a bogus coredata.dat and we end up running meson
three times before it eventually decides the error is fatal.

It looks like meson is incorrectly defaulting to "rust
enabled" rather than "rust disabled" here ?

e104462:jammy:qemu$ make -C build/x86 -j8
make: Entering directory '/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86'
config-host.mak is out-of-date, running configure
python determined to be '/usr/bin/python3'
python version: Python 3.10.12
mkvenv: Creating non-isolated virtual environment at 'pyvenv'
mkvenv: checking for meson>=1.5.0
mkvenv: checking for pycotap>=1.1.0
mkvenv: installing meson==1.5.0, pycotap==1.3.1
mkvenv: checking for sphinx>=3.4.3
mkvenv: checking for sphinx_rtd_theme>=0.5
[0/1] Regenerating build files.
WARNING: Regenerating configuration from scratch.
Reason: Coredata file
'/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86/meson-private/coredata.dat'
references functions or classes that don't exist. This probably means
that it was generated with an old version of meson.
The Meson build system
Version: 1.5.0
Source dir: /mnt/nvmedisk/linaro/qemu-from-laptop/qemu
Build dir: /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86
Build type: native build
Project name: qemu
Project version: 9.1.50
C compiler for the host machine: ccache gcc -m64 (gcc 11.4.0 "gcc
(Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0")
C linker for the host machine: gcc -m64 ld.bfd 2.38
Host machine cpu family: x86_64
Host machine cpu: x86_64
Program scripts/symlink-install-tree.py found: YES
(/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86/pyvenv/bin/python3
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/scripts/symlink-install-tree.py)
Program sh found: YES (/usr/bin/sh)
Program python3 found: YES
(/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86/pyvenv/bin/python3)
Rust compiler for the host machine: rustc -C linker=gcc -C
link-arg=-m64 (rustc 1.81.0)
Rust linker for the host machine: rustc -C linker=gcc -C link-arg=-m64
ld.bfd 2.38
Program iasl found: YES (/usr/bin/iasl)
Program bzip2 found: YES (/usr/bin/bzip2)
[trimmed a bunch of uninteresting meson output]
Executing subproject keycodemapdb

keycodemapdb| Project name: keycodemapdb
keycodemapdb| Project version: undefined
keycodemapdb| Program tools/keymap-gen found: YES
(/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/subprojects/keycodemapdb/tools/keymap-gen)
keycodemapdb| Build targets in project: 272
keycodemapdb| Subproject keycodemapdb finished.

Program scripts/decodetree.py found: YES
(/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86/pyvenv/bin/python3
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/scripts/decodetree.py)
Program ../scripts/modules/module_block.py found: YES
(/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86/pyvenv/bin/python3
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/block/../scripts/modules/module_block.py)
Program ../scripts/block-coroutine-wrapper.py found: YES
(/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86/pyvenv/bin/python3
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/block/../scripts/block-coroutine-wrapper.py)
Program scripts/modinfo-collect.py found: YES
(/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86/pyvenv/bin/python3
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/scripts/modinfo-collect.py)
Program scripts/modinfo-generate.py found: YES
(/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86/pyvenv/bin/python3
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/scripts/modinfo-generate.py)
Program nm found: YES
Program scripts/undefsym.py found: YES
(/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86/pyvenv/bin/python3
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/scripts/undefsym.py)
Program scripts/rust/rustc_args.py found: YES
(/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86/pyvenv/bin/python3
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/scripts/rust/rustc_args.py)
Program bindgen found: NO found 0.53.1 but need: '>=0.69.4'
(/home/petmay01/.cargo/bin/bindgen)

../../meson.build:3965:31: ERROR: Program 'bindgen' not found or not executable

A full log can be found at
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86/meson-logs/meson-log.txt
FAILED: build.ninja
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu

Re: [PULL v3 00/18] Rust initial PoC + meson changes for 2024-10-07

2024-10-14 Thread Paolo Bonzini

On Mon, Oct 14, 2024 at 12:40 PM Peter Maydell  wrote:
>
> On Mon, 14 Oct 2024 at 11:12, Peter Maydell  wrote:
> >
> > On Fri, 11 Oct 2024 at 18:13, Paolo Bonzini  wrote:
> > > v2->v3: new patches
> > > - scripts/archive-source: find directory name for subprojects
> > > - docs: fix invalid footnote syntax
> > > - docs: avoid footnotes consisting of just URLs
> > > - docs: use consistent markup for footnotes
> > >
> > > 
> > > * first commit for Rust support
> > > * add CI job using Fedora + Rust nightly
> > > * fix detection of ATOMIC128 on x86_64
> > > * fix compilation with Sphinx 8.1.0
>
> > Applied, thanks.
>
> With this applied, I find that on one of my personal
> local dev branches an incremental rebuild fails, because
> meson complains about not finding a new enough bindgen,
> even though I did not --enable-rust. Meson also complains
> about a bogus coredata.dat and we end up running meson
> three times before it eventually decides the error is fatal.

The report of coredata.dat is just a warning that it's not able to use
any cached data, which is expected when bumping the Meson version.

It's definitely going in the "if have_rust and have_system" path. If
you have the meson-logs/meson-log.txt and meson-private/cmd_line.txt
files, they can help debugging. I'd expect a "rust = disabled" line in
the latter... but yes I see what's happening. The

  test "$rust" != "auto" && meson_option_add "-Drust=$rust"

line is only executed when configure runs meson. Here it doesn't, and
Makefile just tells Meson to reconfigure itself. Meson then gets the
command line options from either coredata.dat (which has everything
cached in Python's pickle format) or cmd_line.txt (slow path when
Meson version is upgraded), but neither knows about the rust option;
and the meson_options.txt default is 'auto'.

To sum up:

1) this is specific to incremental builds

2) this is *not* specific to the Meson version change, the
coredata.dat warning is a red herring

3) this (mangled) patch would fix it:

diff --git a/configure b/configure
index 3e38a91616a..8a9a4153310 100755
--- a/configure
+++ b/configure
@@ -1987,7 +1987,7 @@ if test "$skip_meson" = no; then
   fi

   # QEMU options
-  test "$rust" != "auto" && meson_option_add "-Drust=$rust"
+  test "$rust" != "disabled" && meson_option_add "-Drust=$rust"
   test "$cfi" != false && meson_option_add "-Dcfi=$cfi" "-Db_lto=$cfi"
   test "$docs" != auto && meson_option_add "-Ddocs=$docs"
   test -n "${LIB_FUZZING_ENGINE+xxx}" && meson_option_add
"-Dfuzzing_engine=$LIB_FUZZING_ENGINE"
diff --git a/meson_options.txt b/meson_options.txt
index 2211f291b2d..fc6d5526d58 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -372,5 +372,5 @@ option('hexagon_idef_parser', type : 'boolean',
value : true,
 option('x86_version', type : 'combo', choices : ['0', '1', '2', '3',
'4'], value: '1',
description: 'tweak required x86_64 architecture version
beyond compiler default')

-option('rust', type: 'feature', value: 'auto',
+option('rust', type: 'feature', value: 'disabled',
description: 'Rust support')

and I can send it shortly; fortunately your tree never got a working
coredata.dat, so it hasn't stored anywhere the rust=auto value.

Paolo


Paolo

Re: [PULL v3 00/18] Rust initial PoC + meson changes for 2024-10-07

On Mon, 14 Oct 2024 13:40, Peter Maydell  wrote:
>On Mon, 14 Oct 2024 at 11:12, Peter Maydell  wrote:
>>
>> On Fri, 11 Oct 2024 at 18:13, Paolo Bonzini  wrote:
>> > v2->v3: new patches
>> > - scripts/archive-source: find directory name for subprojects
>> > - docs: fix invalid footnote syntax
>> > - docs: avoid footnotes consisting of just URLs
>> > - docs: use consistent markup for footnotes
>> >
>> > 
>> > * first commit for Rust support
>> > * add CI job using Fedora + Rust nightly
>> > * fix detection of ATOMIC128 on x86_64
>> > * fix compilation with Sphinx 8.1.0
>
>> Applied, thanks.
>
>With this applied, I find that on one of my personal
>local dev branches an incremental rebuild fails, because
>meson complains about not finding a new enough bindgen,
>even though I did not --enable-rust. Meson also complains
>about a bogus coredata.dat and we end up running meson
>three times before it eventually decides the error is fatal.
>
>It looks like meson is incorrectly defaulting to "rust
>enabled" rather than "rust disabled" here ?
>
>[trimmed]

In this pull request, meson_options.txt has:

 +option('rust', type: 'feature', value: 'auto',

So it's not disabled by default. It sounds like meson enables the Rust 
feature because it found the rustc binary.

[PATCH] configure, meson: synchronize defaults for configure and Meson Rust options

2024-10-14 Thread Paolo Bonzini

If the defaults for --enable-rust ($rust in configure) and Meson's rust
option are out of sync, incremental builds will pick Meson's default.

This happens because, on an incremental build, configure does not run
Meson, Make does instead.  Meson then gets the command line options
from either coredata.dat (which has everything cached in Python's pickle
format) or cmd_line.txt (slow path when Meson version is upgraded), but
neither knows about the rust option, and the meson_options.txt default
is used.

This will cause have_rust to be true if rustc is available; and the build
to fail because configure did not put a RUST_TARGET_TRIPLE in config-host.mak.

When in the Rust pull request I changed the $rust default from auto
to disabled, I should have made the same change to meson_options.txt;
do it now.

Cc: Manos Pitsidianakis 
Reported-by: Peter Maydell 
Reported-by: Daniel P. Berrangé 
Signed-off-by: Paolo Bonzini 
---
 configure | 2 +-
 meson_options.txt | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 3e38a91616a..8a9a4153310 100755
--- a/configure
+++ b/configure
@@ -1987,7 +1987,7 @@ if test "$skip_meson" = no; then
   fi
 
   # QEMU options
-  test "$rust" != "auto" && meson_option_add "-Drust=$rust"
+  test "$rust" != "disabled" && meson_option_add "-Drust=$rust"
   test "$cfi" != false && meson_option_add "-Dcfi=$cfi" "-Db_lto=$cfi"
   test "$docs" != auto && meson_option_add "-Ddocs=$docs"
   test -n "${LIB_FUZZING_ENGINE+xxx}" && meson_option_add 
"-Dfuzzing_engine=$LIB_FUZZING_ENGINE"
diff --git a/meson_options.txt b/meson_options.txt
index 2211f291b2d..fc6d5526d58 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -372,5 +372,5 @@ option('hexagon_idef_parser', type : 'boolean', value : 
true,
 option('x86_version', type : 'combo', choices : ['0', '1', '2', '3', '4'], 
value: '1',
description: 'tweak required x86_64 architecture version beyond 
compiler default')
 
-option('rust', type: 'feature', value: 'auto',
+option('rust', type: 'feature', value: 'disabled',
description: 'Rust support')
-- 
2.46.2

Re: [PULL v3 00/18] Rust initial PoC + meson changes for 2024-10-07

2024-10-14 Thread Daniel P . Berrangé

On Mon, Oct 14, 2024 at 11:40:11AM +0100, Peter Maydell wrote:
> On Mon, 14 Oct 2024 at 11:12, Peter Maydell  wrote:
> >
> > On Fri, 11 Oct 2024 at 18:13, Paolo Bonzini  wrote:
> > > v2->v3: new patches
> > > - scripts/archive-source: find directory name for subprojects
> > > - docs: fix invalid footnote syntax
> > > - docs: avoid footnotes consisting of just URLs
> > > - docs: use consistent markup for footnotes
> > >
> > > 
> > > * first commit for Rust support
> > > * add CI job using Fedora + Rust nightly
> > > * fix detection of ATOMIC128 on x86_64
> > > * fix compilation with Sphinx 8.1.0
> 
> > Applied, thanks.
> 
> With this applied, I find that on one of my personal
> local dev branches an incremental rebuild fails, because
> meson complains about not finding a new enough bindgen,
> even though I did not --enable-rust. Meson also complains
> about a bogus coredata.dat and we end up running meson
> three times before it eventually decides the error is fatal.
> 
> It looks like meson is incorrectly defaulting to "rust
> enabled" rather than "rust disabled" here ?

I've just hit a similar problem, except in my case I'm on Fedora 40
and have new enough rust + bindgen present. It downloaded a bunch of
rust stuff and then failed complaining about the target triple being
unknown:

$ make
make[1]: Entering directory '/var/home/berrange/src/virt/qemu/build'
config-host.mak is out-of-date, running configure
python determined to be '/usr/bin/python3'
python version: Python 3.12.5
mkvenv: Creating non-isolated virtual environment at 'pyvenv'
mkvenv: checking for meson>=1.5.0
mkvenv: checking for pycotap>=1.1.0
mkvenv: installing meson==1.5.0
mkvenv: checking for sphinx>=3.4.3
mkvenv: checking for sphinx_rtd_theme>=0.5
[0/1] Regenerating build files.
WARNING: Regenerating configuration from scratch.
Reason: Coredata file 
'/var/home/berrange/src/virt/qemu/build/meson-private/coredata.dat' references 
functions or classes that don't exist. This probably means that it was 
generated with an old version of meson.
The Meson build system
Version: 1.5.0
Source dir: /var/home/berrange/src/virt/qemu
Build dir: /var/home/berrange/src/virt/qemu/build
Build type: native build
Project name: qemu
Project version: 9.1.50
C compiler for the host machine: cc -m64 (gcc 14.2.1 "cc (GCC) 14.2.1 20240801 
(Red Hat 14.2.1-1)")
C linker for the host machine: cc -m64 ld.bfd 2.41-37
Host machine cpu family: x86_64
Host machine cpu: x86_64
Program scripts/symlink-install-tree.py found: YES 
(/var/home/berrange/src/virt/qemu/build/pyvenv/bin/python3 
/var/home/berrange/src/virt/qemu/scripts/symlink-install-tree.py)
Program sh found: YES (/usr/bin/sh)
Program python3 found: YES 
(/var/home/berrange/src/virt/qemu/build/pyvenv/bin/python3)
Rust compiler for the host machine: rustc -C linker=cc -C link-arg=-m64 (rustc 
1.81.0)
Rust linker for the host machine: rustc -C linker=cc -C link-arg=-m64 ld.bfd 
2.41-37
Program iasl found: NO
Program bzip2 found: YES (/usr/bin/bzip2)
Compiler for C supports link arguments -Wl,-z,relro: YES 
Compiler for C supports link arguments -Wl,-z,now: YES 
Checking if "-fzero-call-used-regs=used-gpr" compiles: YES 
Compiler for C supports arguments -ftrivial-auto-var-init=zero: YES 
Compiler for C supports arguments -fzero-call-used-regs=used-gpr: YES 
Compiler for C supports arguments -Wempty-body: YES 
Compiler for C supports arguments -Wendif-labels: YES 
Compiler for C supports arguments -Wexpansion-to-defined: YES 
Compiler for C supports arguments -Wformat-security: YES 
Compiler for C supports arguments -Wformat-y2k: YES 
Compiler for C supports arguments -Wignored-qualifiers: YES 
Compiler for C supports arguments -Wimplicit-fallthrough=2: YES 
Compiler for C supports arguments -Winit-self: YES 
Compiler for C supports arguments -Wmissing-format-attribute: YES 
Compiler for C supports arguments -Wmissing-prototypes: YES 
Compiler for C supports arguments -Wnested-externs: YES 
Compiler for C supports arguments -Wold-style-declaration: YES 
Compiler for C supports arguments -Wold-style-definition: YES 
Compiler for C supports arguments -Wredundant-decls: YES 
Compiler for C supports arguments -Wshadow=local: YES 
Compiler for C supports arguments -Wstrict-prototypes: YES 
Compiler for C supports arguments -Wtype-limits: YES 
Compiler for C supports arguments -Wundef: YES 
Compiler for C supports arguments -Wvla: YES 
Compiler for C supports arguments -Wwrite-strings: YES 
Compiler for C supports arguments -Wno-gnu-variable-sized-type-not-at-end: NO 
Compiler for C supports arguments -Wno-initializer-overrides: NO 
Compiler for C supports arguments -Wno-missing-include-dirs: YES 
Compiler for C supports arguments -Wno-psabi: YES 
Compiler for C supports arguments -Wno-shift-negative-value: YES 
Compiler for C supports arguments -Wno-string-plus-int: NO 
Compiler for C supports arguments -Wno-tautological-type-limit-compare: NO 
Compiler for

Re: [PATCH -qemu] hw/cxl: Support get/set mctp response payload size

On Thu, 10 Oct 2024 16:08:51 -0700
Fan Ni  wrote:

> On Wed, Oct 09, 2024 at 06:41:57PM -0700, Davidlohr Bueso wrote:
> > Add Get/Set Response Message Limit commands.
> > 
> > Signed-off-by: Davidlohr Bueso   
> 
> The commit log may include the cxl spec reference. Otherwise,
> 
> Reviewed-by: Fan Ni 
> 
> 
> > ---
> >  hw/cxl/cxl-mailbox-utils.c | 68 --
> >  1 file changed, 65 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> > index c2d776bc96eb..98416af794bb 100644
> > --- a/hw/cxl/cxl-mailbox-utils.c
> > +++ b/hw/cxl/cxl-mailbox-utils.c
> > @@ -7,6 +7,8 @@
> >   * COPYING file in the top-level directory.
> >   */
> >  
> > +#include 
> > +
> >  #include "qemu/osdep.h"
> >  #include "hw/pci/msi.h"
> >  #include "hw/pci/msix.h"
> > @@ -56,6 +58,8 @@ enum {
> >  INFOSTAT= 0x00,
> >  #define IS_IDENTIFY   0x1
> >  #define BACKGROUND_OPERATION_STATUS0x2
> > +#define GET_RESPONSE_MSG_LIMIT   0x3
> > +#define SET_RESPONSE_MSG_LIMIT   0x4
> >  EVENTS  = 0x01,
> >  #define GET_RECORDS   0x0
> >  #define CLEAR_RECORDS   0x1
> > @@ -393,7 +397,7 @@ static CXLRetCode cmd_infostat_identify(const struct 
> > cxl_cmd *cmd,
> >  uint16_t pcie_subsys_vid;
> >  uint16_t pcie_subsys_id;
> >  uint64_t sn;
> > -uint8_t max_message_size;
> > +uint8_t max_message_size;
Huh.  I wonder how that misalignment got in.

I'll spin a separate tidy up patch to deal with that and
include it in a trivial series I'm sending later today.

> >  uint8_t component_type;
> >  } QEMU_PACKED *is_identify;
> >  QEMU_BUILD_BUG_ON(sizeof(*is_identify) != 18);
> > @@ -422,12 +426,58 @@ static CXLRetCode cmd_infostat_identify(const struct 
> > cxl_cmd *cmd,
> >  is_identify->component_type = 0x3; /* Type 3 */
> >  }
> >  
> > -/* TODO: Allow this to vary across different CCIs */
> > -is_identify->max_message_size = 9; /* 512 bytes - 
> > MCTP_CXL_MAILBOX_BYTES */
> > +is_identify->max_message_size = (uint8_t)log2(cci->payload_max);
> >  *len_out = sizeof(*is_identify);
> >  return CXL_MBOX_SUCCESS;
> >  }
> >  
> > +/* CXL r3.1 section 8.2.9.1.3: Get Response Message Limit (Opcode 0003h) */
> > +static CXLRetCode cmd_get_response_msg_limit(const struct cxl_cmd *cmd,
> > + uint8_t *payload_in,
> > + size_t len_in,
> > + uint8_t *payload_out,
> > + size_t *len_out,
> > + CXLCCI *cci)
> > +{
> > +struct {
> > +uint8_t rsp_limit;
> > +} QEMU_PACKED *get_rsp_msg_limit = (void *)payload_out;
> > +QEMU_BUILD_BUG_ON(sizeof(*get_rsp_msg_limit) != 1);
> > +
> > +get_rsp_msg_limit->rsp_limit = (uint8_t)log2(cci->payload_max);
> > +
> > +*len_out = sizeof(*get_rsp_msg_limit);
> > +return CXL_MBOX_SUCCESS;
> > +}
> > +
> > +/* CXL r3.1 section 8.2.9.1.4: Set Response Message Limit (Opcode 0004h) */
> > +static CXLRetCode cmd_set_response_msg_limit(const struct cxl_cmd *cmd,
> > + uint8_t *payload_in,
> > + size_t len_in,
> > + uint8_t *payload_out,
> > + size_t *len_out,
> > + CXLCCI *cci)
> > +{
> > +struct {
> > +uint8_t rsp_limit;
> > +} QEMU_PACKED *in = (void *)payload_in;
> > +QEMU_BUILD_BUG_ON(sizeof(*in) != 1);
> > +struct {
> > +uint8_t rsp_limit;
> > +} QEMU_PACKED *out = (void *)payload_out;
> > +QEMU_BUILD_BUG_ON(sizeof(*out) != 1);
> > +
> > +if (in->rsp_limit < 8 || in->rsp_limit > 10) {

Good to document why these values - possibly using defines.
I think 8 is the minimum the spec allows, but is the 10 based on
anything specific?

I'll carry this on my gitlab staging branch but want to
tidy this up before suggesting Michael picks it up.

I'll end up splitting this up a little though as only one of
the MCTP mailboxes on that tree is not dependent on the i2c mctp
stuff that is queued up behind Klaus' series.  So I'll drag
the guts of this to near the top of my tree and include the extra
commands where that i2c_mctp is added.

hw/cxl/i2c_mctp_cxl: Initial device emulation

I'll push a new gitlab.com/jic23/qemu tree out later today with this
done.

Thanks,

Jonathan

> > +return CXL_MBOX_INVALID_INPUT;
> > +}
> > +
> > +cci->payload_max = 1 << in->rsp_limit;
> > +out->rsp_limit = in->rsp_limit;
> > +
> > +*len_out = sizeof(*out);
> > +return CXL_MBOX_SUCCESS;
> > +}
> > +
> >  static void cxl_set_dsp_active_bm(PCIBus *b, PCIDevice *d,
> >v

[PATCH v3 1/8] target/riscv: Add Ssdbltrp CSRs handling

Add ext_ssdbltrp in RISCVCPUConfig and implement MSTATUS.SDT,
{H|M}ENVCFG.DTE and modify the availability of MTVAL2 based on the
presence of the Ssdbltrp ISA extension.

Signed-off-by: Clément Léger 
Reviewed-by: Alistair Francis 
---
 target/riscv/cpu.h|  1 +
 target/riscv/cpu_bits.h   |  6 ++
 target/riscv/cpu_cfg.h|  1 +
 target/riscv/cpu_helper.c | 20 +
 target/riscv/csr.c| 45 ++-
 5 files changed, 63 insertions(+), 10 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index a84e719d3f..ee984bf270 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -553,6 +553,7 @@ void riscv_cpu_set_geilen(CPURISCVState *env, target_ulong 
geilen);
 bool riscv_cpu_vector_enabled(CPURISCVState *env);
 void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
 int riscv_env_mmu_index(CPURISCVState *env, bool ifetch);
+bool riscv_env_smode_dbltrp_enabled(CPURISCVState *env, bool virt);
 G_NORETURN void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
MMUAccessType access_type,
int mmu_idx, uintptr_t retaddr);
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index da1723496c..3a5588d4df 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -558,6 +558,7 @@
 #define MSTATUS_TVM 0x0010 /* since: priv-1.10 */
 #define MSTATUS_TW  0x0020 /* since: priv-1.10 */
 #define MSTATUS_TSR 0x0040 /* since: priv-1.10 */
+#define MSTATUS_SDT 0x0100
 #define MSTATUS_GVA 0x40ULL
 #define MSTATUS_MPV 0x80ULL
 
@@ -588,6 +589,7 @@ typedef enum {
 #define SSTATUS_XS  0x00018000
 #define SSTATUS_SUM 0x0004 /* since: priv-1.10 */
 #define SSTATUS_MXR 0x0008
+#define SSTATUS_SDT 0x0100
 
 #define SSTATUS64_UXL   0x0003ULL
 
@@ -777,11 +779,13 @@ typedef enum RISCVException {
 #define MENVCFG_CBIE   (3UL << 4)
 #define MENVCFG_CBCFE  BIT(6)
 #define MENVCFG_CBZE   BIT(7)
+#define MENVCFG_DTE(1ULL << 59)
 #define MENVCFG_ADUE   (1ULL << 61)
 #define MENVCFG_PBMTE  (1ULL << 62)
 #define MENVCFG_STCE   (1ULL << 63)
 
 /* For RV32 */
+#define MENVCFGH_DTE   BIT(27)
 #define MENVCFGH_ADUE  BIT(29)
 #define MENVCFGH_PBMTE BIT(30)
 #define MENVCFGH_STCE  BIT(31)
@@ -795,11 +799,13 @@ typedef enum RISCVException {
 #define HENVCFG_CBIE   MENVCFG_CBIE
 #define HENVCFG_CBCFE  MENVCFG_CBCFE
 #define HENVCFG_CBZE   MENVCFG_CBZE
+#define HENVCFG_DTEMENVCFG_DTE
 #define HENVCFG_ADUE   MENVCFG_ADUE
 #define HENVCFG_PBMTE  MENVCFG_PBMTE
 #define HENVCFG_STCE   MENVCFG_STCE
 
 /* For RV32 */
+#define HENVCFGH_DTEMENVCFGH_DTE
 #define HENVCFGH_ADUE   MENVCFGH_ADUE
 #define HENVCFGH_PBMTE  MENVCFGH_PBMTE
 #define HENVCFGH_STCE   MENVCFGH_STCE
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index ae2a945b5f..dd804f95d4 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -77,6 +77,7 @@ struct RISCVCPUConfig {
 bool ext_smstateen;
 bool ext_sstc;
 bool ext_smcntrpmf;
+bool ext_ssdbltrp;
 bool ext_svadu;
 bool ext_svinval;
 bool ext_svnapot;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 9d0400035f..77e7736d8a 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -63,6 +63,22 @@ int riscv_env_mmu_index(CPURISCVState *env, bool ifetch)
 #endif
 }
 
+bool riscv_env_smode_dbltrp_enabled(CPURISCVState *env, bool virt)
+{
+#ifdef CONFIG_USER_ONLY
+return false;
+#else
+if (!riscv_cpu_cfg(env)->ext_ssdbltrp) {
+return false;
+}
+if (virt) {
+return (env->henvcfg & HENVCFG_DTE) != 0;
+} else {
+return (env->menvcfg & MENVCFG_DTE) != 0;
+}
+#endif
+}
+
 void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
   uint64_t *cs_base, uint32_t *pflags)
 {
@@ -562,6 +578,10 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
 
 g_assert(riscv_has_ext(env, RVH));
 
+if (riscv_env_smode_dbltrp_enabled(env, current_virt)) {
+mstatus_mask |= MSTATUS_SDT;
+}
+
 if (current_virt) {
 /* Current V=1 and we are about to change to V=0 */
 env->vsstatus = env->mstatus & mstatus_mask;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index e5de72453c..d8280ec956 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -540,6 +540,15 @@ static RISCVExc

[PATCH v3 4/8] target/riscv: Add Ssdbltrp ISA extension enable switch

Add the switch to enable the Ssdbltrp ISA extension.

Signed-off-by: Clément Léger 
---
 target/riscv/cpu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 65347ccd5a..4a146bb637 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -190,6 +190,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
 ISA_EXT_DATA_ENTRY(ssccptr, PRIV_VERSION_1_11_0, has_priv_1_11),
 ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf),
 ISA_EXT_DATA_ENTRY(sscounterenw, PRIV_VERSION_1_12_0, has_priv_1_12),
+ISA_EXT_DATA_ENTRY(ssdbltrp, PRIV_VERSION_1_13_0, ext_ssdbltrp),
 ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc),
 ISA_EXT_DATA_ENTRY(sstvala, PRIV_VERSION_1_12_0, has_priv_1_12),
 ISA_EXT_DATA_ENTRY(sstvecd, PRIV_VERSION_1_12_0, has_priv_1_12),
@@ -1492,6 +1493,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
 MULTI_EXT_CFG_BOOL("smrnmi", ext_smrnmi, false),
 MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
 MULTI_EXT_CFG_BOOL("ssaia", ext_ssaia, false),
+MULTI_EXT_CFG_BOOL("ssdbltrp", ext_ssdbltrp, false),
 MULTI_EXT_CFG_BOOL("svade", ext_svade, false),
 MULTI_EXT_CFG_BOOL("svadu", ext_svadu, true),
 MULTI_EXT_CFG_BOOL("svinval", ext_svinval, false),
-- 
2.45.2

[PATCH v3 8/8] target/riscv: Add Smdbltrp ISA extension enable switch

Add the switch to enable the Smdbltrp ISA extension.

Signed-off-by: Clément Léger 
---
 target/riscv/cpu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 53e3bb6b37..6e22bfe37d 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -183,6 +183,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
 ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin),
 ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
 ISA_EXT_DATA_ENTRY(smcntrpmf, PRIV_VERSION_1_12_0, ext_smcntrpmf),
+ISA_EXT_DATA_ENTRY(smdbltrp, PRIV_VERSION_1_13_0, ext_smdbltrp),
 ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
 ISA_EXT_DATA_ENTRY(smrnmi, PRIV_VERSION_1_12_0, ext_smrnmi),
 ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
@@ -1492,6 +1493,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
 MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
 
 MULTI_EXT_CFG_BOOL("smaia", ext_smaia, false),
+MULTI_EXT_CFG_BOOL("smdbltrp", ext_smdbltrp, false),
 MULTI_EXT_CFG_BOOL("smepmp", ext_smepmp, false),
 MULTI_EXT_CFG_BOOL("smrnmi", ext_smrnmi, false),
 MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
-- 
2.45.2

[PATCH v3 2/8] target/riscv: Implement Ssdbltrp sret, mret and mnret behavior

When the Ssdbltrp extension is enabled, SSTATUS.SDT field is cleared
when executing sret. When executing mret/mnret, SSTATUS.SDT is cleared
when returning to U, VS or VU and VSSTATUS.SDT is cleared when returning
to VU from HS.

Signed-off-by: Clément Léger 
---
 target/riscv/op_helper.c | 35 ++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 6895c7596b..00b6f75102 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -287,6 +287,18 @@ target_ulong helper_sret(CPURISCVState *env)
 get_field(mstatus, MSTATUS_SPIE));
 mstatus = set_field(mstatus, MSTATUS_SPIE, 1);
 mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
+
+if (riscv_cpu_cfg(env)->ext_ssdbltrp) {
+if (riscv_has_ext(env, RVH)) {
+target_ulong prev_vu = get_field(env->hstatus, HSTATUS_SPV) &&
+   prev_priv == PRV_U;
+/* Returning to VU from HS, vsstatus.sdt = 0 */
+if (!env->virt_enabled && prev_vu) {
+env->vsstatus = set_field(env->vsstatus, MSTATUS_SDT, 0);
+}
+}
+mstatus = set_field(mstatus, MSTATUS_SDT, 0);
+}
 if (env->priv_ver >= PRIV_VERSION_1_12_0) {
 mstatus = set_field(mstatus, MSTATUS_MPRV, 0);
 }
@@ -297,7 +309,6 @@ target_ulong helper_sret(CPURISCVState *env)
 target_ulong hstatus = env->hstatus;
 
 prev_virt = get_field(hstatus, HSTATUS_SPV);
-
 hstatus = set_field(hstatus, HSTATUS_SPV, 0);
 
 env->hstatus = hstatus;
@@ -328,6 +339,22 @@ static void check_ret_from_m_mode(CPURISCVState *env, 
target_ulong retpc,
 riscv_raise_exception(env, RISCV_EXCP_INST_ACCESS_FAULT, GETPC());
 }
 }
+static target_ulong ssdbltrp_mxret(CPURISCVState *env, target_ulong mstatus,
+   target_ulong prev_priv,
+   target_ulong prev_virt)
+{
+/* If returning to U, VS or VU, sstatus.sdt = 0 */
+if (prev_priv == PRV_U || (prev_virt &&
+(prev_priv == PRV_S || prev_priv == PRV_U))) {
+mstatus = set_field(mstatus, MSTATUS_SDT, 0);
+/* If returning to VU, vsstatus.sdt = 0 */
+if (prev_virt && prev_priv == PRV_U) {
+env->vsstatus = set_field(env->vsstatus, MSTATUS_SDT, 0);
+}
+}
+
+return mstatus;
+}
 
 target_ulong helper_mret(CPURISCVState *env)
 {
@@ -345,6 +372,9 @@ target_ulong helper_mret(CPURISCVState *env)
 mstatus = set_field(mstatus, MSTATUS_MPP,
 riscv_has_ext(env, RVU) ? PRV_U : PRV_M);
 mstatus = set_field(mstatus, MSTATUS_MPV, 0);
+if (riscv_cpu_cfg(env)->ext_ssdbltrp) {
+mstatus = ssdbltrp_mxret(env, mstatus, prev_priv, prev_virt);
+}
 if ((env->priv_ver >= PRIV_VERSION_1_12_0) && (prev_priv != PRV_M)) {
 mstatus = set_field(mstatus, MSTATUS_MPRV, 0);
 }
@@ -382,6 +412,9 @@ target_ulong helper_mnret(CPURISCVState *env)
 if (prev_priv < PRV_M) {
 env->mstatus = set_field(env->mstatus, MSTATUS_MPRV, false);
 }
+if (riscv_cpu_cfg(env)->ext_ssdbltrp) {
+env->mstatus = ssdbltrp_mxret(env, env->mstatus, prev_priv, prev_virt);
+}
 
 if (riscv_has_ext(env, RVH) && prev_virt) {
 riscv_cpu_swap_hypervisor_regs(env);
-- 
2.45.2

[PATCH v3 5/8] target/riscv: Add Smdbltrp CSRs handling

Add `ext_smdbltrp`in RISCVCPUConfig and implement MSTATUS.MDT behavior.
Also set MDT to 1 at reset according to the specification.

Signed-off-by: Clément Léger 
---
 target/riscv/cpu.c  |  3 +++
 target/riscv/cpu_bits.h |  1 +
 target/riscv/cpu_cfg.h  |  1 +
 target/riscv/csr.c  | 15 +++
 4 files changed, 20 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 4a146bb637..53e3bb6b37 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -946,6 +946,9 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType 
type)
 env->mstatus_hs = set_field(env->mstatus_hs,
 MSTATUS64_UXL, env->misa_mxl);
 }
+if (riscv_cpu_cfg(env)->ext_smdbltrp) {
+env->mstatus = set_field(env->mstatus, MSTATUS_MDT, 1);
+}
 }
 env->mcause = 0;
 env->miclaim = MIP_SGEIP;
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 5557a86348..62bab1bf55 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -561,6 +561,7 @@
 #define MSTATUS_SDT 0x0100
 #define MSTATUS_GVA 0x40ULL
 #define MSTATUS_MPV 0x80ULL
+#define MSTATUS_MDT 0x400ULL /* Smdbltrp extension */
 
 #define MSTATUS64_UXL   0x0003ULL
 #define MSTATUS64_SXL   0x000CULL
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index dd804f95d4..4c4caa2b39 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -78,6 +78,7 @@ struct RISCVCPUConfig {
 bool ext_sstc;
 bool ext_smcntrpmf;
 bool ext_ssdbltrp;
+bool ext_smdbltrp;
 bool ext_svadu;
 bool ext_svinval;
 bool ext_svnapot;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d8280ec956..cc1940447a 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1617,6 +1617,14 @@ static RISCVException write_mstatus(CPURISCVState *env, 
int csrno,
 }
 }
 
+if (riscv_cpu_cfg(env)->ext_smdbltrp) {
+mask |= MSTATUS_MDT;
+if ((val & MSTATUS_MDT) != 0) {
+mstatus &= ~MSTATUS_MIE;
+val &= ~MSTATUS_MIE;
+}
+}
+
 if (xl != MXL_RV32 || env->debugger) {
 if (riscv_has_ext(env, RVH)) {
 mask |= MSTATUS_MPV | MSTATUS_GVA;
@@ -1655,6 +1663,13 @@ static RISCVException write_mstatush(CPURISCVState *env, 
int csrno,
 uint64_t valh = (uint64_t)val << 32;
 uint64_t mask = riscv_has_ext(env, RVH) ? MSTATUS_MPV | MSTATUS_GVA : 0;
 
+if (riscv_cpu_cfg(env)->ext_smdbltrp) {
+mask |= MSTATUS_MDT;
+if ((val & MSTATUS_MDT) != 0) {
+env->mstatus &= ~MSTATUS_MIE;
+val &= ~MSTATUS_MIE;
+}
+}
 env->mstatus = (env->mstatus & ~mask) | (valh & mask);
 
 return RISCV_EXCP_NONE;
-- 
2.45.2

[PATCH v3 0/8] target/riscv: Add support for Smdbltrp and Ssdbltrp extensions

A double trap typically arises during a sensitive phase in trap handling
operations — when an exception or interrupt occurs while the trap
handler (the component responsible for managing these events) is in a
non-reentrant state. This non-reentrancy usually occurs in the early
phase of trap handling, wherein the trap handler has not yet preserved
the necessary state to handle and resume from the trap. The occurrence
of such event is unlikely but can happen when dealing with hardware
errors.

This series adds support for Ssdbltrp and Smdbltrp ratified ISA
extensions [1]. It is based on the Smrnmi series [6].

Ssdbltrp can be tested using qemu[2], opensbi[3], linux[4] and
kvm-unit-tests[5]. Assuming you have a riscv environment available and
configured (CROSS_COMPILE), it can be built for riscv64 using the
following instructions:

Qemu:
  $ git clone https://github.com/rivosinc/qemu.git
  $ cd qemu
  $ git switch -C dbltrp_v3 dev/cleger/dbltrp_v3
  $ mkdir build && cd build
  $ ../configure --target-list=riscv64-softmmu
  $ make

OpenSBI:
  $ git clone https://github.com/rivosinc/opensbi.git
  $ cd opensbi
  $ git switch -C dbltrp_v3 dev/cleger/dbltrp_v3
  $ make O=build PLATFORM_RISCV_XLEN=64 PLATFORM=generic

Linux:
  $ git clone https://github.com/rivosinc/linux.git
  $ cd linux
  $ git switch -C dbltrp_v1 dev/cleger/dbltrp_v1
  $ export ARCH=riscv
  $ make O=build defconfig
  $ ./script/config --file build/.config --enable RISCV_DBLTRP
  $ make O=build

kvm-unit-tests:
  $ git clone https://github.com/clementleger/kvm-unit-tests.git
  $ cd kvm-unit-tests
  $ git switch -C dbltrp_v1 dev/cleger/dbltrp_v1
  $ ./configure --arch=riscv64 --cross-prefix=$CROSS_COMPILE
  $ make

You will also need kvmtool in your rootfs.

Run with kvm-unit-test test as kernel:
  $ qemu-system-riscv64 \
-M virt \
-cpu rv64,ssdbltrp=true,smdbltrp=true \
-nographic \
-serial mon:stdio \
-bios opensbi/build/platform/generic/firmware/fw_jump.bin \
-kernel kvm-unit-tests-dbltrp/riscv/sbi_dbltrp.flat
  ...
  [OpenSBI boot partially elided]
  Boot HART ISA Extensions  : 
sscofpmf,sstc,zicntr,zihpm,zicboz,zicbom,sdtrig,svadu,ssdbltrp
  ...
  ##
  #kvm-unit-tests
  ##

  PASS: sbi: fwft: FWFT extension probing no error
  PASS: sbi: fwft: FWFT extension is present
  PASS: sbi: fwft: dbltrp: Get double trap enable feature value
  PASS: sbi: fwft: dbltrp: Set double trap enable feature value == 0
  PASS: sbi: fwft: dbltrp: Get double trap enable feature value == 0
  PASS: sbi: fwft: dbltrp: Double trap disabled, trap first time ok
  PASS: sbi: fwft: dbltrp: Set double trap enable feature value == 1
  PASS: sbi: fwft: dbltrp: Get double trap enable feature value == 1
  PASS: sbi: fwft: dbltrp: Trapped twice allowed ok
  INFO: sbi: fwft: dbltrp: Should generate a double trap and crash !

  sbi_trap_error: hart0: trap0: double trap handler failed (error -10)

  sbi_trap_error: hart0: trap0: mcause=0x0010 
mtval=0x
  sbi_trap_error: hart0: trap0: mtval2=0x0003 
mtinst=0x
  sbi_trap_error: hart0: trap0: mepc=0x802000d8 
mstatus=0x800a01006900
  sbi_trap_error: hart0: trap0: ra=0x802001fc sp=0x80213e70
  sbi_trap_error: hart0: trap0: gp=0x tp=0x80088000
  sbi_trap_error: hart0: trap0: s0=0x80213e80 s1=0x0001
  sbi_trap_error: hart0: trap0: a0=0x80213e80 a1=0x80208193
  sbi_trap_error: hart0: trap0: a2=0x8020dc20 a3=0x000f
  sbi_trap_error: hart0: trap0: a4=0x80210cd8 a5=0x802110d0
  sbi_trap_error: hart0: trap0: a6=0x802136e4 a7=0x46574654
  sbi_trap_error: hart0: trap0: s2=0x80210cd9 s3=0x
  sbi_trap_error: hart0: trap0: s4=0x s5=0x
  sbi_trap_error: hart0: trap0: s6=0x s7=0x0001
  sbi_trap_error: hart0: trap0: s8=0x2000 s9=0x80083700
  sbi_trap_error: hart0: trap0: s10=0x s11=0x
  sbi_trap_error: hart0: trap0: t0=0x t1=0x80213ed8
  sbi_trap_error: hart0: trap0: t2=0x1000 t3=0x80213ee0
  sbi_trap_error: hart0: trap0: t4=0x t5=0x8020f8d0
  sbi_trap_error: hart0: trap0: t6=0x

Run with linux and kvm-unit-test test in kvm (testing VS-mode):
  $ qemu-system-riscv64 \
-M virt \
-cpu rv64,ssdbltrp=true,smdbltrp=true \
-nographic \
-serial mon:stdio \
-bios opensbi/build/platform/generic/firmware/fw_jump.bin \
-kernel linux/build/arch/riscv/boot/Image
  ...
  [Linux boot partially elided]
  [0.735079] riscv-dbltrp: Double trap handling registered
  ...

  $ lkvm run -k sbi_dbltrp.flat -m 128 -c 2
  ##

[PATCH v3 3/8] target/riscv: Implement Ssdbltrp exception handling

When the Ssdbltrp ISA extension is enabled, if a trap happens in S-mode
while SSTATUS.SDT isn't cleared, generate a double trap exception to
M-mode.

Signed-off-by: Clément Léger 
---
 target/riscv/cpu.c|  2 +-
 target/riscv/cpu_bits.h   |  1 +
 target/riscv/cpu_helper.c | 42 ++-
 3 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index cf06cd741a..65347ccd5a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -284,7 +284,7 @@ static const char * const riscv_excp_names[] = {
 "load_page_fault",
 "reserved",
 "store_page_fault",
-"reserved",
+"double_trap",
 "reserved",
 "reserved",
 "reserved",
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 3a5588d4df..5557a86348 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -699,6 +699,7 @@ typedef enum RISCVException {
 RISCV_EXCP_INST_PAGE_FAULT = 0xc, /* since: priv-1.10.0 */
 RISCV_EXCP_LOAD_PAGE_FAULT = 0xd, /* since: priv-1.10.0 */
 RISCV_EXCP_STORE_PAGE_FAULT = 0xf, /* since: priv-1.10.0 */
+RISCV_EXCP_DOUBLE_TRAP = 0x10,
 RISCV_EXCP_SW_CHECK = 0x12, /* since: priv-1.13.0 */
 RISCV_EXCP_HW_ERR = 0x13, /* since: priv-1.13.0 */
 RISCV_EXCP_INST_GUEST_PAGE_FAULT = 0x14,
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 77e7736d8a..5173155070 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1711,6 +1711,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 CPURISCVState *env = &cpu->env;
 bool virt = env->virt_enabled;
 bool write_gva = false;
+bool vsmode_exc;
 uint64_t s;
 int mode;
 
@@ -1725,6 +1726,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 !(env->mip & (1 << cause));
 bool vs_injected = env->hvip & (1 << cause) & env->hvien &&
 !(env->mip & (1 << cause));
+bool smode_double_trap = false;
+uint64_t hdeleg = async ? env->hideleg : env->hedeleg;
 target_ulong tval = 0;
 target_ulong tinst = 0;
 target_ulong htval = 0;
@@ -1841,13 +1844,34 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 !async &&
 mode == PRV_M;
 
+vsmode_exc = env->virt_enabled && (((hdeleg >> cause) & 1) || vs_injected);
+/*
+ * Check double trap condition only if already in S-mode and targeting
+ * S-mode
+ */
+if (cpu->cfg.ext_ssdbltrp && env->priv == PRV_S && mode == PRV_S) {
+bool dte = (env->menvcfg & MENVCFG_DTE) != 0;
+bool sdt = (env->mstatus & MSTATUS_SDT) != 0;
+/* In VS or HS */
+if (riscv_has_ext(env, RVH)) {
+if (vsmode_exc) {
+/* VS -> VS, use henvcfg instead of menvcfg*/
+dte = (env->henvcfg & HENVCFG_DTE) != 0;
+} else if (env->virt_enabled) {
+/* VS -> HS, mret/sret always reset dte to false */
+dte = false;
+}
+}
+smode_double_trap = dte && sdt;
+if (smode_double_trap) {
+mode = PRV_M;
+}
+}
+
 if (mode == PRV_S) {
 /* handle the trap in S-mode */
 if (riscv_has_ext(env, RVH)) {
-uint64_t hdeleg = async ? env->hideleg : env->hedeleg;
-
-if (env->virt_enabled &&
-(((hdeleg >> cause) & 1) || vs_injected)) {
+if (vsmode_exc) {
 /* Trap to VS mode */
 /*
  * See if we need to adjust cause. Yes if its VS mode interrupt
@@ -1880,6 +1904,9 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 s = set_field(s, MSTATUS_SPIE, get_field(s, MSTATUS_SIE));
 s = set_field(s, MSTATUS_SPP, env->priv);
 s = set_field(s, MSTATUS_SIE, 0);
+if (riscv_env_smode_dbltrp_enabled(env, virt)) {
+s = set_field(s, MSTATUS_SDT, 1);
+}
 env->mstatus = s;
 env->scause = cause | ((target_ulong)async << (TARGET_LONG_BITS - 1));
 env->sepc = env->pc;
@@ -1913,9 +1940,14 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 s = set_field(s, MSTATUS_MIE, 0);
 env->mstatus = s;
 env->mcause = cause | ~(((target_ulong)-1) >> async);
+if (smode_double_trap) {
+env->mtval2 = env->mcause;
+env->mcause = RISCV_EXCP_DOUBLE_TRAP;
+} else {
+env->mtval2 = mtval2;
+}
 env->mepc = env->pc;
 env->mtval = tval;
-env->mtval2 = mtval2;
 env->mtinst = tinst;
 
 if (cpu->cfg.ext_smrnmi && nmi_execp) {
-- 
2.45.2

[PATCH v3 7/8] target/riscv: Implement Smdbltrp behavior

When the Smsdbltrp ISA extension is enabled, if a trap happens while
MSTATUS.MDT is already set, it will trigger an abort or an NMI is the
Smrnmi extension is available.

Signed-off-by: Clément Léger 
---
 target/riscv/cpu_helper.c | 35 ++-
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 5173155070..8b44986cd6 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1699,6 +1699,17 @@ static target_ulong riscv_transformed_insn(CPURISCVState 
*env,
 return xinsn;
 }
 
+static void riscv_do_nmi(CPURISCVState *env, target_ulong cause, bool virt)
+{
+env->mnstatus = set_field(env->mnstatus, MNSTATUS_NMIE, false);
+env->mnstatus = set_field(env->mnstatus, MNSTATUS_MNPV, virt);
+env->mnstatus = set_field(env->mnstatus, MNSTATUS_MNPP, env->priv);
+env->mncause = cause;
+env->mnepc = env->pc;
+env->pc = env->rnmi_irqvec;
+riscv_cpu_set_mode(env, PRV_M, virt);
+}
+
 /*
  * Handle Traps
  *
@@ -1735,15 +1746,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 bool nmi_execp = false;
 
 if (cpu->cfg.ext_smrnmi && env->rnmip && async) {
-env->mnstatus = set_field(env->mnstatus, MNSTATUS_NMIE, false);
-env->mnstatus = set_field(env->mnstatus, MNSTATUS_MNPV,
-  env->virt_enabled);
-env->mnstatus = set_field(env->mnstatus, MNSTATUS_MNPP,
-  env->priv);
-env->mncause = cause | ((target_ulong)1U << (TARGET_LONG_BITS - 1));
-env->mnepc = env->pc;
-env->pc = env->rnmi_irqvec;
-riscv_cpu_set_mode(env, PRV_M, virt);
+riscv_do_nmi(env, cause | ((target_ulong)1U << (TARGET_LONG_BITS - 1)),
+ virt);
 return;
 }
 
@@ -1938,6 +1942,19 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 s = set_field(s, MSTATUS_MPIE, get_field(s, MSTATUS_MIE));
 s = set_field(s, MSTATUS_MPP, env->priv);
 s = set_field(s, MSTATUS_MIE, 0);
+if (cpu->cfg.ext_smdbltrp) {
+if (env->mstatus & MSTATUS_MDT) {
+assert(env->priv == PRV_M);
+if (!cpu->cfg.ext_smrnmi || nmi_execp) {
+cpu_abort(CPU(cpu), "M-mode double trap\n");
+} else {
+riscv_do_nmi(env, cause, false);
+return;
+}
+}
+
+s = set_field(s, MSTATUS_MDT, 1);
+}
 env->mstatus = s;
 env->mcause = cause | ~(((target_ulong)-1) >> async);
 if (smode_double_trap) {
-- 
2.45.2

[PATCH v3 6/8] target/riscv: Implement Smdbltrp sret, mret and mnret behavior

When the Ssdbltrp extension is enabled, SSTATUS.MDT field is cleared
when executing sret if executed in M-mode. When executing mret/mnret,
SSTATUS.MDT is cleared.

Signed-off-by: Clément Léger 
---
 target/riscv/op_helper.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 00b6f75102..9d0911f697 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -299,6 +299,9 @@ target_ulong helper_sret(CPURISCVState *env)
 }
 mstatus = set_field(mstatus, MSTATUS_SDT, 0);
 }
+if (riscv_cpu_cfg(env)->ext_smdbltrp && env->priv >= PRV_M) {
+mstatus = set_field(mstatus, MSTATUS_MDT, 0);
+}
 if (env->priv_ver >= PRIV_VERSION_1_12_0) {
 mstatus = set_field(mstatus, MSTATUS_MPRV, 0);
 }
@@ -375,6 +378,9 @@ target_ulong helper_mret(CPURISCVState *env)
 if (riscv_cpu_cfg(env)->ext_ssdbltrp) {
 mstatus = ssdbltrp_mxret(env, mstatus, prev_priv, prev_virt);
 }
+if (riscv_cpu_cfg(env)->ext_smdbltrp) {
+mstatus = set_field(mstatus, MSTATUS_MDT, 0);
+}
 if ((env->priv_ver >= PRIV_VERSION_1_12_0) && (prev_priv != PRV_M)) {
 mstatus = set_field(mstatus, MSTATUS_MPRV, 0);
 }
@@ -416,6 +422,12 @@ target_ulong helper_mnret(CPURISCVState *env)
 env->mstatus = ssdbltrp_mxret(env, env->mstatus, prev_priv, prev_virt);
 }
 
+if (riscv_cpu_cfg(env)->ext_smdbltrp) {
+if (prev_priv < PRV_M) {
+env->mstatus = set_field(env->mstatus, MSTATUS_MDT, false);
+}
+}
+
 if (riscv_has_ext(env, RVH) && prev_virt) {
 riscv_cpu_swap_hypervisor_regs(env);
 }
-- 
2.45.2

Re: [QEMU RFC] hw/mem/cxl_type3: add guard to avoid event log overflow during a DC extent add/release request

On Fri, 11 Oct 2024 13:24:50 -0700
nifan@gmail.com wrote:

> From: Fan Ni 
> 
> One DC extent add/release request can take multiple DC extents.
> For each extent in the request, one DC event record will be generated and
> isnerted into the event log. All the event records for the request will be
> grouped with the More flag (see CXL spec r3.1, Table 8-168 and 8-170).
> If an overflow happens during the process, the yet-to-insert records will
> get lost, leaving the device in a situation where it notifies the host
> only part of the extents involved, and the host never surfacing the
> extents received and waiting for the remaining extents.

Interesting corner.  For other 'events' an overflow is natural because
they can be out of the control of the device. This artificial limit
was to trigger the overflow handling in those cases. For this one I'd expect
the device to push back on the fabric management commands, or handle the
event log filling so overflow doesn't happen.

> 
> Add a check in qmp_cxl_process_dynamic_capacity_prescriptive and ensure
> the event log does not overflow during the process.
> 
> Currently we check the number of extents involved with the event
> overflow threshold, do we need to tight the check and compare with
> the remaining spot available in the event log?

Yes. I think we need to prevent other outstanding events causing us trouble.

Is it useful to support the case where we have more than one
group of extents outstanding?  If not we could simply fail the add whenever
that happens.  Maybe that is a reasonable stop gap until we have a reason
to care about that case. We probably care when we have FM-API hooked up
to this and want to test more advanced fabric management stuff, or poke
a corner of the kernel code perhaps?

I guess from a 'would it be right if a device did this' the answer may be
yes, but that doesn't mean Linux is going to support such a device
(at least not until we know they really exist).  Ira, what do you think
about this corner case?  Maybe detect and scream if we aren't already?

Jonathan

> 
> Signed-off-by: Fan Ni 
> ---
>  hw/cxl/cxl-events.c | 2 --
>  hw/mem/cxl_type3.c  | 7 +++
>  include/hw/cxl/cxl_events.h | 3 +++
>  3 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/cxl/cxl-events.c b/hw/cxl/cxl-events.c
> index 12dee2e467..05d8aae627 100644
> --- a/hw/cxl/cxl-events.c
> +++ b/hw/cxl/cxl-events.c
> @@ -16,8 +16,6 @@
>  #include "hw/cxl/cxl.h"
>  #include "hw/cxl/cxl_events.h"
>  
> -/* Artificial limit on the number of events a log can hold */
> -#define CXL_TEST_EVENT_OVERFLOW 8
>  
>  static void reset_overflow(CXLEventLog *log)
>  {
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 3d7289fa84..32668df365 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -2015,6 +2015,13 @@ static void 
> qmp_cxl_process_dynamic_capacity_prescriptive(const char *path,
>  num_extents++;
>  }
>  
> +if (num_extents > CXL_TEST_EVENT_OVERFLOW) {
> +error_setg(errp,
> +   "at most %d extents allowed in one add/release request",
> +   CXL_TEST_EVENT_OVERFLOW);
> +   return;
> +}
> +
>  /* Create extent list for event being passed to host */
>  i = 0;
>  list = records;
> diff --git a/include/hw/cxl/cxl_events.h b/include/hw/cxl/cxl_events.h
> index 38cadaa0f3..2a6b57e3e6 100644
> --- a/include/hw/cxl/cxl_events.h
> +++ b/include/hw/cxl/cxl_events.h
> @@ -12,6 +12,9 @@
>  
>  #include "qemu/uuid.h"
>  
> +/* Artificial limit on the number of events a log can hold */
> +#define CXL_TEST_EVENT_OVERFLOW 8
> +
>  /*
>   * CXL r3.1 section 8.2.9.2.2: Get Event Records (Opcode 0100h); Table 8-52
>   *

Re: [PATCH net-next v7] ptp: Add support for the AMZNC10C 'vmclock' device

2024-10-14 Thread David Woodhouse

On Wed, 2024-10-09 at 17:32 -0700, Jakub Kicinski wrote:
> On Sun, 06 Oct 2024 08:17:58 +0100 David Woodhouse wrote:
> > +config PTP_1588_CLOCK_VMCLOCK
> > +   tristate "Virtual machine PTP clock"
> > +   depends on X86_TSC || ARM_ARCH_TIMER
> > +   depends on PTP_1588_CLOCK && ACPI && ARCH_SUPPORTS_INT128
> > +   default y
> 
> Why default to enabled? Linus will not be happy..

Want an incremental patch to change that?


smime.p7s
Description: S/MIME cryptographic signature

Re: [PATCH v6 0/5] target/riscv: Add Smrnmi support.




On 11/10/2024 13:38, Daniel Henrique Barboza wrote:
> Hi Tommy,
> 
> 
> Do you plan to send a new version of this work soon? This series is a
> prerequisite
> of "target/riscv: Add support for Smdbltrp and Ssdbltrp extensions" and
> we need
> this series merged first. We have minor comments from Clément and

Hi Henrique,

If that's easier, I can still remove the dependency on Smrnmi and add
support for that later.

Clément

> Alistair so
> hopefully it shouldn't be too much work.
> 
> The code freeze for 9.2 will happen in the first/second week of
> November, so if you
> could send a new version to be merged in the next PR that would be great.
> 
> 
> Thanks,
> 
> Daniel
> 
> 
> 
> On 9/2/24 4:13 AM, Tommy Wu wrote:
>> This patchset added support for Smrnmi Extension in RISC-V.
>>
>> There are four new CSRs and one new instruction added to allow NMI to be
>> resumable in RISC-V, which are:
>>
>> =
>>    * mnscratch (0x740)
>>    * mnepc (0x741)
>>    * mncause   (0x742)
>>    * mnstatus  (0x744)
>> =
>>    * mnret: To return from RNMI interrupt/exception handler.
>> =
>>
>> RNMI also has higher priority than any other interrupts or exceptions
>> and cannot be disabled by software.
>>
>> RNMI may be used to route to other devices such as Bus Error Unit or
>> Watchdog Timer in the future.
>>
>> The interrupt/exception trap handler addresses of RNMI are
>> implementation defined.
>>
>> If anyone wants to test the patches, we can use the customized
>> OpenSBI[1],
>> and the customized QEMU[2].
>>
>> We implemented a PoC RNMI trap handler in the customized OpenSBI.
>> In the customized QEMU, we use the Smrnmi patches and the patch from
>> Damien Hedde[3]. The patch from Damien Hedde can be used to inject
>> the RNMI signal with the qmp command.
>>
>> [1] https://github.com/TommyWu-fdgkhdkgh/opensbi/tree/dev/twu/master
>> [2] https://github.com/TommyWu-fdgkhdkgh/qemu/tree/dev/twu/master
>> [3] https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg06232.html
>>
>> Test commands :
>> $ ./build/qemu-system-riscv64 -M virt -cpu rv64,smrnmi=true,
>> rnmi-interrupt-vector={Offset of the RNMI handler in the customized
>> OpenSBI.} -m 4G -smp 2 -serial mon:stdio -serial null -nographic
>> -bios fw_jump.elf -kernel Image -initrd rootfs.cpio
>> -qmp unix:/tmp/qmp-sock,server,wait=off
>>
>> Use qmp command to inject the RNMI interrupt.
>> $ ./scripts/qmp/qmp-shell /tmp/qmp-sock
>> (QEMU)  gpio-set path=/machine/soc0/harts[0] gpio=riscv.cpu.rnmi
>> number=0 value=true
>>
>> (QEMU)  gpio-set path=/machine/soc0/harts[0] gpio=riscv.cpu.rnmi
>> number=0 value=false
>>
>> Changelog:
>>
>> v6
>>    * Delete the redundant code in `riscv_cpu_do_interrupt`.
>>    ( Thank Alvin for the suggestion. )
>>    * Split the shared code in `helper_mret` and `helper_mnret` into a
>>  helper function `check_ret_from_m_mode`.
>>    ( Thank Alistair for the suggestion. )
>>
>> v5
>>    * Move the patch that adds the Smrnmi extension to the last patch.
>>    ( Thank Alistair for the suggestion. )
>>    * Implement an M-mode software PoC for this with implemented handlers.
>>    ( Thank Andrew Jones for the suggestion. )
>>    * Add a commit message to all patches of the series.
>>    ( Thank Andrew Jones for the suggestion. )
>>
>> v4
>>    * Fix some coding style issues.
>>    ( Thank Daniel for the suggestions. )
>>
>> v3
>>    * Update to the newest version of Smrnmi extension specification.
>>
>> v2
>>    * split up the series into more commits for convenience of review.
>>    * add missing rnmi_irqvec and rnmi_excpvec properties to riscv_harts.
>>
>> Tommy Wu (5):
>>    target/riscv: Add `ext_smrnmi` in the RISCVCPUConfig.
>>    target/riscv: Handle Smrnmi interrupt and exception.
>>    target/riscv: Add Smrnmi CSRs.
>>    target/riscv: Add Smrnmi mnret instruction.
>>    target/riscv: Add Smrnmi cpu extension.
>>
>>   hw/riscv/riscv_hart.c | 18 
>>   include/hw/riscv/riscv_hart.h |  4 +
>>   target/riscv/cpu.c    | 18 
>>   target/riscv/cpu.h    | 10 +++
>>   target/riscv/cpu_bits.h   | 23 ++
>>   target/riscv/cpu_cfg.h    |  1 +
>>   target/riscv/cpu_helper.c | 80 --
>>   target/riscv/csr.c    | 82 +++
>>   target/riscv/helper.h |  1 +
>>   target/riscv/insn32.decode    |  3 +
>>   .../riscv/insn_trans/trans_privileged.c.inc   | 12 +++
>>   target/riscv/op_helper.c  | 49 +--
>>   12 files changed, 291 insertions(+), 10 deletions(-)
>>

Re: [PATCH v2 3/8] target/riscv: Implement Ssdbltrp exception handling




On 11/10/2024 05:22, Alistair Francis wrote:
> On Wed, Sep 25, 2024 at 9:59 PM Clément Léger  wrote:
>>
>> When the Ssdbltrp ISA extension is enabled, if a trap happens in S-mode
>> while SSTATUS.SDT isn't cleared, generate a double trap exception to
>> M-mode.
>>
>> Signed-off-by: Clément Léger 
>> ---
>>  target/riscv/cpu.c|  2 +-
>>  target/riscv/cpu_bits.h   |  1 +
>>  target/riscv/cpu_helper.c | 47 ++-
>>  3 files changed, 43 insertions(+), 7 deletions(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index cf06cd741a..65347ccd5a 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -284,7 +284,7 @@ static const char * const riscv_excp_names[] = {
>>  "load_page_fault",
>>  "reserved",
>>  "store_page_fault",
>> -"reserved",
>> +"double_trap",
>>  "reserved",
>>  "reserved",
>>  "reserved",
>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>> index 3a5588d4df..5557a86348 100644
>> --- a/target/riscv/cpu_bits.h
>> +++ b/target/riscv/cpu_bits.h
>> @@ -699,6 +699,7 @@ typedef enum RISCVException {
>>  RISCV_EXCP_INST_PAGE_FAULT = 0xc, /* since: priv-1.10.0 */
>>  RISCV_EXCP_LOAD_PAGE_FAULT = 0xd, /* since: priv-1.10.0 */
>>  RISCV_EXCP_STORE_PAGE_FAULT = 0xf, /* since: priv-1.10.0 */
>> +RISCV_EXCP_DOUBLE_TRAP = 0x10,
>>  RISCV_EXCP_SW_CHECK = 0x12, /* since: priv-1.13.0 */
>>  RISCV_EXCP_HW_ERR = 0x13, /* since: priv-1.13.0 */
>>  RISCV_EXCP_INST_GUEST_PAGE_FAULT = 0x14,
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index 395d8235ce..69da3c3384 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -575,7 +575,9 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
>>  mstatus_mask |= MSTATUS_FS;
>>  }
>>  bool current_virt = env->virt_enabled;
>> -
>> +if (riscv_env_smode_dbltrp_enabled(env, current_virt)) {
>> +mstatus_mask |= MSTATUS_SDT;
>> +}
>>  g_assert(riscv_has_ext(env, RVH));
>>
>>  if (current_virt) {
>> @@ -1707,6 +1709,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>>  CPURISCVState *env = &cpu->env;
>>  bool virt = env->virt_enabled;
>>  bool write_gva = false;
>> +bool vsmode_exc;
>>  uint64_t s;
>>  int mode;
>>
>> @@ -1721,6 +1724,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>>  !(env->mip & (1 << cause));
>>  bool vs_injected = env->hvip & (1 << cause) & env->hvien &&
>>  !(env->mip & (1 << cause));
>> +bool smode_double_trap = false;
>> +uint64_t hdeleg = async ? env->hideleg : env->hedeleg;
>>  target_ulong tval = 0;
>>  target_ulong tinst = 0;
>>  target_ulong htval = 0;
>> @@ -1837,13 +1842,35 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>>  !async &&
>>  mode == PRV_M;
>>
>> +vsmode_exc = env->virt_enabled && (((hdeleg >> cause) & 1) || 
>> vs_injected);
>> +/*
>> + * Check double trap condition only if already in S-mode and targeting
>> + * S-mode
>> + */
>> +if (cpu->cfg.ext_ssdbltrp && env->priv == PRV_S && mode == PRV_S) {
>> +bool dte = (env->menvcfg & MENVCFG_DTE) != 0;
>> +bool sdt = (env->mstatus & MSTATUS_SDT) != 0;
>> +/* In VS or HS */
>> +if (riscv_has_ext(env, RVH)) {
>> +if (vsmode_exc) {
>> +/* VS -> VS */
>> +/* Stay in VS mode, use henvcfg instead of menvcfg*/
>> +dte = (env->henvcfg & HENVCFG_DTE) != 0;
>> +} else if (env->virt_enabled) {
>> +/* VS -> HS */
>> +dte = false;
> 
> I don't follow why this is false

Hi Alistair,

It's indeed probably lacking some comments here. The rationale is that
if you are trapping from VS to HS, then at some point, you returned to
VS using a sret/mret and thus cleared DTE, so rather than checking the
value of mstatus_hs, just assume it is false.

Thanks,

Clément

> 
> Alistair

Re: [PATCH v2 4/8] target/riscv: Add Ssdbltrp ISA extension enable switch




On 11/10/2024 05:24, Alistair Francis wrote:
> On Wed, Sep 25, 2024 at 9:59 PM Clément Léger  wrote:
>>
>> Add the switch to enable the Ssdbltrp ISA extension.
>>
>> Signed-off-by: Clément Léger 
>> ---
>>  target/riscv/cpu.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 65347ccd5a..4f52cf7ac0 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -190,6 +190,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
>>  ISA_EXT_DATA_ENTRY(ssccptr, PRIV_VERSION_1_11_0, has_priv_1_11),
>>  ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf),
>>  ISA_EXT_DATA_ENTRY(sscounterenw, PRIV_VERSION_1_12_0, has_priv_1_12),
>> +ISA_EXT_DATA_ENTRY(ssdbltrp, PRIV_VERSION_1_12_0, ext_ssdbltrp),
> 
> Shouldn't this be PRIV_VERSION_1_13_0?

Oups, yes indeed.

Thanks,

Clément

> 
> Alistair
> 
>>  ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc),
>>  ISA_EXT_DATA_ENTRY(sstvala, PRIV_VERSION_1_12_0, has_priv_1_12),
>>  ISA_EXT_DATA_ENTRY(sstvecd, PRIV_VERSION_1_12_0, has_priv_1_12),
>> @@ -1492,6 +1493,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
>>  MULTI_EXT_CFG_BOOL("smrnmi", ext_smrnmi, false),
>>  MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
>>  MULTI_EXT_CFG_BOOL("ssaia", ext_ssaia, false),
>> +MULTI_EXT_CFG_BOOL("ssdbltrp", ext_ssdbltrp, false),
>>  MULTI_EXT_CFG_BOOL("svade", ext_svade, false),
>>  MULTI_EXT_CFG_BOOL("svadu", ext_svadu, true),
>>  MULTI_EXT_CFG_BOOL("svinval", ext_svinval, false),
>> --
>> 2.45.2
>>
>>

Re: [PATCH v6 0/3] hw/{i2c,nvme}: mctp endpoint, nvme management interface model

On Wed, 20 Sep 2023 09:36:34 -0500
Corey Minyard  wrote:

> On Wed, Sep 20, 2023 at 06:31:25AM -0700, Klaus Jensen wrote:
> > On Sep 20 07:54, Corey Minyard wrote:  
> > > On Wed, Sep 20, 2023 at 12:48:03PM +0100, Jonathan Cameron via wrote:  
> > > > On Thu, 14 Sep 2023 11:53:40 +0200
> > > > Klaus Jensen  wrote:
> > > >   
> > > > > This adds a generic MCTP endpoint model that other devices may derive
> > > > > from.
> > > > > 
> > > > > Also included is a very basic implementation of an NVMe-MI device,
> > > > > supporting only a small subset of the required commands.
> > > > > 
> > > > > Since this all relies on i2c target mode, this can currently only be
> > > > > used with an SoC that includes the Aspeed I2C controller.
> > > > > 
> > > > > The easiest way to get up and running with this, is to grab my 
> > > > > buildroot
> > > > > overlay[1] (aspeed_ast2600evb_nmi_defconfig). It includes modified a
> > > > > modified dts as well as a couple of required packages.
> > > > > 
> > > > > QEMU can then be launched along these lines:
> > > > > 
> > > > >   qemu-system-arm \
> > > > > -nographic \
> > > > > -M ast2600-evb \
> > > > > -kernel output/images/zImage \
> > > > > -initrd output/images/rootfs.cpio \
> > > > > -dtb output/images/aspeed-ast2600-evb-nmi.dtb \
> > > > > -nic user,hostfwd=tcp::-:22 \
> > > > > -device nmi-i2c,address=0x3a \
> > > > > -serial mon:stdio
> > > > > 
> > > > > From within the booted system,
> > > > > 
> > > > >   mctp addr add 8 dev mctpi2c15
> > > > >   mctp link set mctpi2c15 up
> > > > >   mctp route add 9 via mctpi2c15
> > > > >   mctp neigh add 9 dev mctpi2c15 lladdr 0x3a
> > > > >   mi-mctp 1 9 info
> > > > > 
> > > > > Comments are very welcome!
> > > > > 
> > > > >   [1]: https://github.com/birkelund/hwtests/tree/main/br2-external
> > > > > 
> > > > > Signed-off-by: Klaus Jensen   
> > > > 
> > > > Hi Klaus,
> > > > 
> > > > Silly question, but who is likely to pick this up? + likely to be soon?
> > > > 
> > > > I'm going to post the CXL stuff that makes use of the core support 
> > > > shortly
> > > > and whilst I can point at this patch set on list, I'd keen to see it 
> > > > upstream
> > > > to reduce the dependencies (it's got 2 sets ahead of it of CXL stuff
> > > > anyway but that will all hopefully go through Michael Tsirkin's tree
> > > > for PCI stuff in one go).  
> > > 
> > > I can pick it up, but he can just request a merge, too.
> > > 
> > > I did have a question I asked earlier about tests.  It would be unusual
> > > at this point to add something like this without having some tests,
> > > especially injecting invalid data.
> > >   
> > 
> > Hi all,
> > 
> > Sorry for the late reply. I'm currently at SDC, but I will write up some
> > tests when I get back to in the office on Monday.
> > 
> > Corey, what kinds of tests would be best here? Avocado "acceptance"
> > tests or would you like to see something lower level?  
> 
> My main concern is testing what happens when bad data gets injected, to
> avoid people coming up with clever names for exploits in qemu.  It's not
> so much for this code, it's for the changes that comes in the future.
> 
> And, of course, normal functional tests to make sure it works.  What a
> friend of mine calls "dead chicken" tests.  You wave a dead chicken at
> it, and if the chicken is still dead everything is ok :).
> 
> I'm fine with either type of tests, but I'm not sure you can do this
> with avocado.  It's probably about the same amount of work either path
> you choose.
> 
> -corey

Hi Klaus, All,

I was looking at what dependencies I'm carrying on my CXL tree and this
series is one of the bigger bits :(

Any plans to take it forwards?
I have some other stuff to solve to have a fully upstream QEMU
solution for the CXL fm-api over mctp (direct from host anyway), but
if this is blocked indefinitely tackling how to get a controller onto
a typical server system isn't going to be productive :(

As Davidlohr called out at in the CXL LPC Uconf [1] this is really handy
for testing his work on libcxlmi. A number of people are looking
at more sophisticated CXL fabric emulation and that will also need
us to close this gap!

No promises but maybe we can find someone to help with adding tests
if that's the only remaining blocker.

https://lpc.events/event/18/contributions/1876/attachments/1441/3072/lpc24-dbueso-libcxlmi.pdf
 [1]

Jonathan

Re: [PULL v2 40/61] hw/acpi: Update GED _EVT method AML with CPU scan

2024-10-14 Thread Igor Mammedov

On Mon, 14 Oct 2024 16:52:55 +0800
maobibo  wrote:

> Hi Salil,
> 
> When I debug cpu hotplug on LoongArch system, It reports error like this:
>  ACPI BIOS Error (bug): Could not resolve symbol [\_SB.GED.CSCN], 
> AE_NOT_FOUND
>  ACPI Error: Aborting method \_SB.GED._EVT due to previous error 
> (AE_NOT_FOUND)
>  acpi-ged ACPI0013:00: IRQ method execution failed
> 
> 
> With this patch, GED CPU call method is "\\_SB.GED.CSCN", however in 
> function build_cpus_aml(), its method name is "\\_SB.CPUS.CSCN".
>  method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
>  aml_append(method, aml_call0("\\_SB.CPUS." CPU_SCAN_METHOD));
>  aml_append(table, method);
> 
> It seems that CPU scanning method name is not consistent between 
> function build_cpus_aml() and build_ged_aml().
> 
> Regards
> Bibo Mao
> 
> On 2024/7/23 下午6:59, Michael S. Tsirkin wrote:
> > From: Salil Mehta 
> > 
> > OSPM evaluates _EVT method to map the event. The CPU hotplug event 
> > eventually
> > results in start of the CPU scan. Scan figures out the CPU and the kind of
> > event(plug/unplug) and notifies it back to the guest. Update the GED AML 
> > _EVT
> > method with the call to method \\_SB.CPUS.CSCN (via \\_SB.GED.CSCN)
> > 
> > Architecture specific code [1] might initialize its CPUs AML code by calling
> > common function build_cpus_aml() like below for ARM:
> > 
> > build_cpus_aml(scope, ms, opts, xx_madt_cpu_entry, 
> > memmap[VIRT_CPUHP_ACPI].base,
> > "\\_SB", "\\_SB.GED.CSCN", AML_SYSTEM_MEMORY);

it should be \\_SB.CPUS.CSCN

> > 
> > [1] 
> > https://lore.kernel.org/qemu-devel/20240613233639.202896-13-salil.me...@huawei.com/
> > 
> > Co-developed-by: Keqian Zhu 
> > Signed-off-by: Keqian Zhu 
> > Signed-off-by: Salil Mehta 
> > Reviewed-by: Jonathan Cameron 
> > Reviewed-by: Gavin Shan 
> > Tested-by: Vishnu Pajjuri 
> > Tested-by: Xianglai Li 
> > Tested-by: Miguel Luis 
> > Reviewed-by: Shaoqin Huang 
> > Tested-by: Zhao Liu 
> > Reviewed-by: Igor Mammedov 
> > Message-Id: <20240716111502.202344-5-salil.me...@huawei.com>
> > Reviewed-by: Michael S. Tsirkin 
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >   include/hw/acpi/generic_event_device.h | 1 +
> >   hw/acpi/generic_event_device.c | 3 +++
> >   2 files changed, 4 insertions(+)
> > 
> > diff --git a/include/hw/acpi/generic_event_device.h 
> > b/include/hw/acpi/generic_event_device.h
> > index e091ac2108..40af3550b5 100644
> > --- a/include/hw/acpi/generic_event_device.h
> > +++ b/include/hw/acpi/generic_event_device.h
> > @@ -87,6 +87,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED)
> >   #define GED_DEVICE  "GED"
> >   #define AML_GED_EVT_REG "EREG"
> >   #define AML_GED_EVT_SEL "ESEL"
> > +#define AML_GED_EVT_CPU_SCAN_METHOD "\\_SB.GED.CSCN"
> >   
> >   /*
> >* Platforms need to specify the GED event bitmap
> > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> > index 4641933a0f..15b4c3ebbf 100644
> > --- a/hw/acpi/generic_event_device.c
> > +++ b/hw/acpi/generic_event_device.c
> > @@ -108,6 +108,9 @@ void build_ged_aml(Aml *table, const char *name, 
> > HotplugHandler *hotplug_dev,
> >   aml_append(if_ctx, aml_call0(MEMORY_DEVICES_CONTAINER "."
> >MEMORY_SLOT_SCAN_METHOD));
> >   break;
> > +case ACPI_GED_CPU_HOTPLUG_EVT:
> > +aml_append(if_ctx, aml_call0(AML_GED_EVT_CPU_SCAN_METHOD));
> > +break;
> >   case ACPI_GED_PWR_DOWN_EVT:
> >   aml_append(if_ctx,
> >  aml_notify(aml_name(ACPI_POWER_BUTTON_DEVICE),
> >   
>

Re: [PATCH v16 04/13] s390x/pci: Avoid creating zpci for VFs

2024-10-14 Thread Cédric Le Goater


Hello Akihiko,

On 10/12/24 13:05, Akihiko Odaki wrote:

On 2024/10/11 0:44, Cédric Le Goater wrote:

Hello Akihiko,

Sorry for the late reply.

On 9/18/24 17:32, Akihiko Odaki wrote:

On 2024/09/18 17:02, Cédric Le Goater wrote:

Hello,

On 9/13/24 05:44, Akihiko Odaki wrote:

VFs are automatically created by PF, and creating zpci for them will
result in unexpected usage of fids. Currently QEMU does not support
multifunction for s390x so we don't need zpci for VFs anyway.

Signed-off-by: Akihiko Odaki 
---
  hw/s390x/s390-pci-bus.c | 19 +--
  1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 3e57d5faca18..1a620f5b2a04 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -1080,6 +1080,16 @@ static void s390_pcihost_plug(HotplugHandler 
*hotplug_dev, DeviceState *dev,
  pbdev = s390_pci_find_dev_by_target(s, dev->id);
  if (!pbdev) {
+    /*
+ * VFs are automatically created by PF, and creating zpci for them
+ * will result in unexpected usage of fids. Currently QEMU does not
+ * support multifunction for s390x so we don't need zpci for VFs
+ * anyway.
+ */
+    if (pci_is_vf(pdev)) {
+    return;
+    }
+
  pbdev = s390_pci_device_new(s, dev->id, errp);
  if (!pbdev) {
  return;
@@ -1167,7 +1177,9 @@ static void s390_pcihost_unplug(HotplugHandler 
*hotplug_dev, DeviceState *dev,
  int32_t devfn;
  pbdev = s390_pci_find_dev_by_pci(s, PCI_DEVICE(dev));
-    g_assert(pbdev);
+    if (!pbdev) {
+    return;
+    }



I don't understand this change. Could you please explain ?


We need to tolerate that pbdev being NULL because VFs do no longer have zpci 
and pbdev will be NULL for them.


Then, I think we should extend the assert with a check on pci_is_vf(pdev)
to be symmetric with the plug handler and also, use the 'Error**' parameter
to report an error.


This should never happen unless there is a programming error so plain 
g_assert() without error reporting should be fine. We don't need to report an 
error when it is VF; we just don't have a work to do and nothing wrong happens 
here.


unplugging a VF is still an invalid thing to do, reporting an error is 
preferable IMO.

Thanks,

C.

Re: [PULL v2 40/61] hw/acpi: Update GED _EVT method AML with CPU scan

2024-10-14 Thread maobibo


Hi Salil,

When I debug cpu hotplug on LoongArch system, It reports error like this:
ACPI BIOS Error (bug): Could not resolve symbol [\_SB.GED.CSCN], 
AE_NOT_FOUND
ACPI Error: Aborting method \_SB.GED._EVT due to previous error 
(AE_NOT_FOUND)

acpi-ged ACPI0013:00: IRQ method execution failed


With this patch, GED CPU call method is "\\_SB.GED.CSCN", however in 
function build_cpus_aml(), its method name is "\\_SB.CPUS.CSCN".

method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
aml_append(method, aml_call0("\\_SB.CPUS." CPU_SCAN_METHOD));
aml_append(table, method);

It seems that CPU scanning method name is not consistent between 
function build_cpus_aml() and build_ged_aml().


Regards
Bibo Mao

On 2024/7/23 下午6:59, Michael S. Tsirkin wrote:

From: Salil Mehta 

OSPM evaluates _EVT method to map the event. The CPU hotplug event eventually
results in start of the CPU scan. Scan figures out the CPU and the kind of
event(plug/unplug) and notifies it back to the guest. Update the GED AML _EVT
method with the call to method \\_SB.CPUS.CSCN (via \\_SB.GED.CSCN)

Architecture specific code [1] might initialize its CPUs AML code by calling
common function build_cpus_aml() like below for ARM:

build_cpus_aml(scope, ms, opts, xx_madt_cpu_entry, memmap[VIRT_CPUHP_ACPI].base,
"\\_SB", "\\_SB.GED.CSCN", AML_SYSTEM_MEMORY);

[1] 
https://lore.kernel.org/qemu-devel/20240613233639.202896-13-salil.me...@huawei.com/

Co-developed-by: Keqian Zhu 
Signed-off-by: Keqian Zhu 
Signed-off-by: Salil Mehta 
Reviewed-by: Jonathan Cameron 
Reviewed-by: Gavin Shan 
Tested-by: Vishnu Pajjuri 
Tested-by: Xianglai Li 
Tested-by: Miguel Luis 
Reviewed-by: Shaoqin Huang 
Tested-by: Zhao Liu 
Reviewed-by: Igor Mammedov 
Message-Id: <20240716111502.202344-5-salil.me...@huawei.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
  include/hw/acpi/generic_event_device.h | 1 +
  hw/acpi/generic_event_device.c | 3 +++
  2 files changed, 4 insertions(+)

diff --git a/include/hw/acpi/generic_event_device.h 
b/include/hw/acpi/generic_event_device.h
index e091ac2108..40af3550b5 100644
--- a/include/hw/acpi/generic_event_device.h
+++ b/include/hw/acpi/generic_event_device.h
@@ -87,6 +87,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED)
  #define GED_DEVICE  "GED"
  #define AML_GED_EVT_REG "EREG"
  #define AML_GED_EVT_SEL "ESEL"
+#define AML_GED_EVT_CPU_SCAN_METHOD "\\_SB.GED.CSCN"
  
  /*

   * Platforms need to specify the GED event bitmap
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index 4641933a0f..15b4c3ebbf 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -108,6 +108,9 @@ void build_ged_aml(Aml *table, const char *name, 
HotplugHandler *hotplug_dev,
  aml_append(if_ctx, aml_call0(MEMORY_DEVICES_CONTAINER "."
   MEMORY_SLOT_SCAN_METHOD));
  break;
+case ACPI_GED_CPU_HOTPLUG_EVT:
+aml_append(if_ctx, aml_call0(AML_GED_EVT_CPU_SCAN_METHOD));
+break;
  case ACPI_GED_PWR_DOWN_EVT:
  aml_append(if_ctx,
 aml_notify(aml_name(ACPI_POWER_BUTTON_DEVICE),

Re: [PATCH RESEND v4 0/4] target/i386: Various Hyper-V related fixes

2024-10-14 Thread Vitaly Kuznetsov

Vitaly Kuznetsov  writes:

> Vitaly Kuznetsov  writes:
>
>> Changes since '[PATCH RESEND v3 0/3] i386: Fix Hyper-V Gen1 guests stuck 
>> on boot with 'hv-passthrough':
>>
>> - Added "target/i386: Make sure SynIC state is really updated before 
>> KVM_RUN" 
>>   to the set.
>>  
>> This is a long pending collection of fixes for various Hyper-V related 
>> issues, mostly detected by tests. On top of that, the patchset updates
>> Hyper-V related documentation adding recommendations.
>>
>> Vitaly Kuznetsov (4):
>>   target/i386: Fix conditional CONFIG_SYNDBG enablement
>>   target/i386: Exclude 'hv-syndbg' from 'hv-passthrough'
>>   target/i386: Make sure SynIC state is really updated before KVM_RUN
>>   docs/system: Add recommendations to Hyper-V enlightenments doc
>>
>>  docs/system/i386/hyperv.rst | 43 +
>>  target/i386/cpu.c   |  2 ++
>>  target/i386/kvm/hyperv.c|  1 +
>>  target/i386/kvm/kvm.c   | 18 ++--
>>  4 files changed, 54 insertions(+), 10 deletions(-)
>
> Ping) Some of these patches are really getting old :-)

Ping)

-- 
Vitaly

Re: [PATCH v6 0/5] target/riscv: Add Smrnmi support.

2024-10-14 Thread Frank Chang

Clément Léger  於 2024年10月14日 週一 下午3:36寫道：
>
>
>
> On 11/10/2024 13:38, Daniel Henrique Barboza wrote:
> > Hi Tommy,
> >
> >
> > Do you plan to send a new version of this work soon? This series is a
> > prerequisite
> > of "target/riscv: Add support for Smdbltrp and Ssdbltrp extensions" and
> > we need
> > this series merged first. We have minor comments from Clément and
>
> Hi Henrique,
>
> If that's easier, I can still remove the dependency on Smrnmi and add
> support for that later.
>
> Clément

Hi Clément,

Sorry for keeping you waiting. I've reviewed the comments from you and Alistair.
The comments should be straightforward to fix.
I will fix them and send out the patchset later today.
Hope that it makes things easier.


Regards,
Frank Chang

>
> > Alistair so
> > hopefully it shouldn't be too much work.
> >
> > The code freeze for 9.2 will happen in the first/second week of
> > November, so if you
> > could send a new version to be merged in the next PR that would be great.
> >
> >
> > Thanks,
> >
> > Daniel
> >
> >
> >
> > On 9/2/24 4:13 AM, Tommy Wu wrote:
> >> This patchset added support for Smrnmi Extension in RISC-V.
> >>
> >> There are four new CSRs and one new instruction added to allow NMI to be
> >> resumable in RISC-V, which are:
> >>
> >> =
> >>* mnscratch (0x740)
> >>* mnepc (0x741)
> >>* mncause   (0x742)
> >>* mnstatus  (0x744)
> >> =
> >>* mnret: To return from RNMI interrupt/exception handler.
> >> =
> >>
> >> RNMI also has higher priority than any other interrupts or exceptions
> >> and cannot be disabled by software.
> >>
> >> RNMI may be used to route to other devices such as Bus Error Unit or
> >> Watchdog Timer in the future.
> >>
> >> The interrupt/exception trap handler addresses of RNMI are
> >> implementation defined.
> >>
> >> If anyone wants to test the patches, we can use the customized
> >> OpenSBI[1],
> >> and the customized QEMU[2].
> >>
> >> We implemented a PoC RNMI trap handler in the customized OpenSBI.
> >> In the customized QEMU, we use the Smrnmi patches and the patch from
> >> Damien Hedde[3]. The patch from Damien Hedde can be used to inject
> >> the RNMI signal with the qmp command.
> >>
> >> [1] https://github.com/TommyWu-fdgkhdkgh/opensbi/tree/dev/twu/master
> >> [2] https://github.com/TommyWu-fdgkhdkgh/qemu/tree/dev/twu/master
> >> [3] https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg06232.html
> >>
> >> Test commands :
> >> $ ./build/qemu-system-riscv64 -M virt -cpu rv64,smrnmi=true,
> >> rnmi-interrupt-vector={Offset of the RNMI handler in the customized
> >> OpenSBI.} -m 4G -smp 2 -serial mon:stdio -serial null -nographic
> >> -bios fw_jump.elf -kernel Image -initrd rootfs.cpio
> >> -qmp unix:/tmp/qmp-sock,server,wait=off
> >>
> >> Use qmp command to inject the RNMI interrupt.
> >> $ ./scripts/qmp/qmp-shell /tmp/qmp-sock
> >> (QEMU)  gpio-set path=/machine/soc0/harts[0] gpio=riscv.cpu.rnmi
> >> number=0 value=true
> >>
> >> (QEMU)  gpio-set path=/machine/soc0/harts[0] gpio=riscv.cpu.rnmi
> >> number=0 value=false
> >>
> >> Changelog:
> >>
> >> v6
> >>* Delete the redundant code in `riscv_cpu_do_interrupt`.
> >>( Thank Alvin for the suggestion. )
> >>* Split the shared code in `helper_mret` and `helper_mnret` into a
> >>  helper function `check_ret_from_m_mode`.
> >>( Thank Alistair for the suggestion. )
> >>
> >> v5
> >>* Move the patch that adds the Smrnmi extension to the last patch.
> >>( Thank Alistair for the suggestion. )
> >>* Implement an M-mode software PoC for this with implemented handlers.
> >>( Thank Andrew Jones for the suggestion. )
> >>* Add a commit message to all patches of the series.
> >>( Thank Andrew Jones for the suggestion. )
> >>
> >> v4
> >>* Fix some coding style issues.
> >>( Thank Daniel for the suggestions. )
> >>
> >> v3
> >>* Update to the newest version of Smrnmi extension specification.
> >>
> >> v2
> >>* split up the series into more commits for convenience of review.
> >>* add missing rnmi_irqvec and rnmi_excpvec properties to riscv_harts.
> >>
> >> Tommy Wu (5):
> >>target/riscv: Add `ext_smrnmi` in the RISCVCPUConfig.
> >>target/riscv: Handle Smrnmi interrupt and exception.
> >>target/riscv: Add Smrnmi CSRs.
> >>target/riscv: Add Smrnmi mnret instruction.
> >>target/riscv: Add Smrnmi cpu extension.
> >>
> >>   hw/riscv/riscv_hart.c | 18 
> >>   include/hw/riscv/riscv_hart.h |  4 +
> >>   target/riscv/cpu.c| 18 
> >>   target/riscv/cpu.h| 10 +++
> >>   target/riscv/cpu_bits.h   | 23 ++
> >>   target/riscv/cpu_cfg.h|  1 +
> >>   target/riscv/cpu_helper.c | 80 +

Re: ALSA support in qemu-user?

On Mon, 14 Oct 2024 at 02:13, Andrew Randrianasulu
 wrote:
>
> some 8 years ago this patch was sent  to qemu-devel:
>
> https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg05333.html
> "[Qemu-devel] [PATCH 7/7] Add ALSA ioctls"
>
> I wonder why it was rejected, may be as part of series?

Hard to say from this distance, but looking at the patch
I think it probably was just that it was on the end of
a series that did a bunch of other things and the earlier
patches in the series had issues.

thanks
-- PMM

Re: [PATCH] ppc/pnv: Add support for TPM with SPI interface

2024-10-14 Thread Cédric Le Goater


Hello Dan,

On 10/14/24 02:08, dan tan wrote:

Hi Cédric,

Thank you for the review comments. Please see my response below.

thank you,
---
dan tan
power simulation
phone:+1.7373.099.138
email:dan...@linux.ibm.com


On 2024-09-12 12:20, Cédric Le Goater wrote:

Hello Dan,

On 9/12/24 18:09, dan tan wrote:

From: dan tan 

SPI interface to TPM TIS implementation via swtpm


I would split this patch in 3 :

1. device model
2. activation for the PowerNV machines
3. unit tests


Yes, I will break up the patch per your recommendation.



Each with a slightly more detailed commit log please. one line
is very minimal for a full device model :)


Understood. I will try to do a much better job in subsequent submissions.


A link to a datasheet is also appreciated.


Also, I wonder if this TPM device is designed by IBM or by some
other HW vendor. It would be good to know for a possible reuse.


This addition to the TPM device support is part of a much bigger project
to support an IBM hypervisor stack. In this system, IBM uses Nuvoton
Technology's TPM. Per Nuvoton's documentation, the NPCT7/6/5/4xx devices
implement all TCG commands and functionality, as defined in [TPM2.0]
Trusted Platform Module Library Specifications.

Although it is an IBM Power platform, the TPM does have a different
vendor ID and device ID than IBM's. I did not include Nuvoton's
vendor ID and this TPM model's device ID. Should I have added them?
There isn't anything I do specific to this device.


The OpenBMC device trees include devices such as :

  compatible = "nuvoton,npct75x", "tcg,tpm-tis-i2c";

So, if this is simply a Nuvoton NPCT75x TPM SPI device, please use
that name for the model also.




Some more comments below,



Signed-off-by: dan tan 
---
  include/sysemu/tpm.h   |   3 +
  hw/tpm/tpm_tis_spi.c   | 347 +
  tests/qtest/pnv-tpm-tis-spi-test.c | 223 ++
  hw/ppc/Kconfig |   1 +
  hw/tpm/Kconfig |   6 +
  hw/tpm/meson.build |   1 +
  tests/qtest/meson.build    |   2 +
  7 files changed, 583 insertions(+)
  create mode 100644 hw/tpm/tpm_tis_spi.c
  create mode 100644 tests/qtest/pnv-tpm-tis-spi-test.c

diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index 1ee568b3b6..22b05110e2 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -49,6 +49,7 @@ struct TPMIfClass {
  #define TYPE_TPM_CRB    "tpm-crb"
  #define TYPE_TPM_SPAPR  "tpm-spapr"
  #define TYPE_TPM_TIS_I2C    "tpm-tis-i2c"
+#define TYPE_TPM_TIS_SPI    "tpm-tis-spi"
    #define TPM_IS_TIS_ISA(chr) \
  object_dynamic_cast(OBJECT(chr), TYPE_TPM_TIS_ISA)
@@ -60,6 +61,8 @@ struct TPMIfClass {
  object_dynamic_cast(OBJECT(chr), TYPE_TPM_SPAPR)
  #define TPM_IS_TIS_I2C(chr)  \
  object_dynamic_cast(OBJECT(chr), TYPE_TPM_TIS_I2C)
+#define TPM_IS_TIS_SPI(chr)  \
+    object_dynamic_cast(OBJECT(chr), TYPE_TPM_TIS_SPI)
    /* returns NULL unless there is exactly one TPM device */
  static inline TPMIf *tpm_find(void)
diff --git a/hw/tpm/tpm_tis_spi.c b/hw/tpm/tpm_tis_spi.c
new file mode 100644
index 00..6e7f42b2db
--- /dev/null
+++ b/hw/tpm/tpm_tis_spi.c
@@ -0,0 +1,347 @@
+/*
+ * QEMU PowerPC SPI TPM 2.0 model
+ *
+ * Copyright (c) 2024, IBM Corporation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/sysbus.h"
+#include "hw/pci/pci_ids.h"
+#include "hw/acpi/tpm.h"
+#include "tpm_prop.h"
+#include "qemu/log.h"
+#include "tpm_tis.h"
+#include "hw/ssi/ssi.h"
+
+typedef struct TPMStateSPI {
+    /*< private >*/
+    SSIPeripheral parent_object;
+
+    uint8_t offset;   /* offset into data[] */
+    uint8_t spi_state;    /* READ / WRITE / IDLE */
+#define SPI_STATE_IDLE   0
+#define SPI_STATE_WRITE  1
+#define SPI_STATE_READ   2
+
+    bool    command;
+
+    uint8_t loc_sel;  /* Current locality */
+    uint32_t    tis_addr; /* tis address including locty */
+
+    /*< public >*/
+    TPMState    tpm_state; /* not a QOM object */
+
+} TPMStateSPI;
+
+typedef struct xfer_buffer xfer_buffer;
+
+#ifdef SPI_DEBUG_ENABLED
+#define SPI_DEBUG(x) (x)
+#else
+#define SPI_DEBUG(x)
+#endif


Please use trace events instead.



OK, I will change them


+DECLARE_INSTANCE_CHECKER(TPMStateSPI, TPM_TIS_SPI, TYPE_TPM_TIS_SPI)
+
+static inline void tpm_tis_spi_clear_data(TPMStateSPI *spist)
+{
+    spist->spi_state = 0;
+    spist->offset = 0;
+    spist->tis_addr = 0x;
+
+    return;
+}
+
+/* Callback from TPM to indicate that response is copied */
+static void tpm_tis_spi_request_completed(TPMIf *ti, int ret)
+{
+    TPMStateSPI *spist = TPM_TIS_SPI(ti);
+    TPMState *s = &spist->tpm_state;
+
+    /* Inform the common code. */
+    tpm_tis_request_completed(s, ret);
+}
+
+static enum TPMVersion tpm_tis_spi_get_tpm_version(TPMIf

Re: ALSA support in qemu-user?

2024-10-14 Thread Thomas Huth


On 14/10/2024 11.06, Peter Maydell wrote:

On Mon, 14 Oct 2024 at 02:13, Andrew Randrianasulu
 wrote:


some 8 years ago this patch was sent  to qemu-devel:

https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg05333.html
"[Qemu-devel] [PATCH 7/7] Add ALSA ioctls"

I wonder why it was rejected, may be as part of series?


Hard to say from this distance, but looking at the patch
I think it probably was just that it was on the end of
a series that did a bunch of other things and the earlier
patches in the series had issues.


Yes, looks like there were review comments on the series that were not 
addressed:


 https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg05557.html
 https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg05364.html

But mainly one of the problems were that the patches haven't been send in a 
proper threaded way, so it was hard to follow the series:


 https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg05546.html

Anyway, looking at the other patches, it seems most of them were not related 
to ALSA, so you might be fine in just picking that patch, get it to work 
with the latest version of QEMU again and send just that single updated 
patch to this mailing list again. YMMV of course.


 Thomas

Re: [PULL 08/20] virtio-net: Add only one queue pair when realizing

2024-10-14 Thread Laurent Vivier


Hi Akihiko,

On 04/06/2024 09:37, Jason Wang wrote:

From: Akihiko Odaki 

Multiqueue usage is not negotiated yet when realizing. If more than
one queue is added and the guest never requests to enable multiqueue,
the extra queues will not be deleted when unrealizing and leak.

Fixes: f9d6dbf0bf6e ("virtio-net: remove virtio queues if the guest doesn't support 
multiqueue")
Signed-off-by: Akihiko Odaki 
Signed-off-by: Jason Wang 
---
  hw/net/virtio-net.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 3cee2ef3ac..a8db8bfd9c 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3743,9 +3743,7 @@ static void virtio_net_device_realize(DeviceState *dev, 
Error **errp)
  n->net_conf.tx_queue_size = MIN(virtio_net_max_tx_queue_size(n),
  n->net_conf.tx_queue_size);
  
-for (i = 0; i < n->max_queue_pairs; i++) {

-virtio_net_add_queue(n, i);
-}
+virtio_net_add_queue(n, 0);
  
  n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);

  qemu_macaddr_default_if_unset(&n->nic_conf.macaddr);


This change breaks virtio net migration when multiqueue is enabled.

I think this is because virtqueues are half initialized after migration : they are 
initialized on guest side (kernel is using them) but not on QEMU side (realized has only 
initialized one). After migration, they are not initialized by the call to 
virtio_net_set_multiqueue() from virtio_net_set_features() because virtio_get_num_queues() 
reports already n->max_queue_pairs as this value is coming from the source guest memory.


I don't think we have a way to half-initialize a virtqueue (to initialize them only on 
QEMU side as they are already initialized on kernel side).


I think this change should be reverted to fix the migration issue.

How to reproduce the problem:

Source:

qemu-system-x86_64 -serial mon:stdio -accel kvm -cpu host -m 2G -display none -hda 
vm3.qcow2 -netdev tap,vhost=false,queues=2,id=hostnet0,script=/etc/qemu-ifup -device 
virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:49:47:db,mq=true


Destination:

qemu-system-x86_64 -serial mon:stdio -accel kvm -cpu host -m 2G -display none -hda 
vm3.qcow2 -netdev tap,vhost=false,queues=2,id=hostnet0,script=/etc/qemu-ifup -device 
virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:49:47:db,mq=true -incoming 
tcp:localhost:


In monitor:

migrate tcp:localhost:

Result on destination side:

(hangs and then: )
[   44.175916] watchdog: BUG: soft lockup - CPU#0 stuck for 26s! [kworker/0:0:8]
...
I think we have this error because the control virqueue is #3 for QEMU, whereas the kernel 
is using a control virqueue set by the multiqueue (max_queue_pairs * 2 + 1). There is a 
mismatch between queues...


Thanks,
Laurent

Re: [PATCH v4] scripts/qcow2-to-stdout.py: Add script to write qcow2 images to stdout

2024-10-14 Thread Alberto Garcia

ping

On Tue, Jul 30, 2024 at 04:15:52PM +0200, Alberto Garcia wrote:
> This tool converts a disk image to qcow2, writing the result directly
> to stdout. This can be used for example to send the generated file
> over the network.

[PATCH 6/8] chardev/mux: switch mux frontends management to bitset

Frontends can be attached and detached during run-time (although detach
is not implemented, but will follow). Counter variable of muxes is not
enough for proper attach/detach management, so this patch implements
bitset: if bit is set for the `mux_bitset` variable, then frontend
device can be found in the `backend` array (yes, huge confusion with
backend and frontends names).

Signed-off-by: Roman Penyaev 
Cc: "Marc-André Lureau" 
Cc: qemu-devel@nongnu.org
---
 chardev/char-mux.c | 41 +-
 chardev/char.c |  2 +-
 chardev/chardev-internal.h |  2 +-
 3 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 9294f955462e..9c3cacb2fecd 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -26,6 +26,7 @@
 #include "qapi/error.h"
 #include "qemu/module.h"
 #include "qemu/option.h"
+#include "qemu/bitops.h"
 #include "chardev/char.h"
 #include "sysemu/block-backend.h"
 #include "qapi/qapi-commands-control.h"
@@ -168,12 +169,19 @@ static int mux_proc_byte(Chardev *chr, MuxChardev *d, int 
ch)
 case 'b':
 qemu_chr_be_event(chr, CHR_EVENT_BREAK);
 break;
-case 'c':
-assert(d->mux_cnt > 0); /* handler registered with first fe */
+case 'c': {
+unsigned int bit;
+
+/* Handler registered with first fe */
+assert(d->mux_bitset != 0);
 /* Switch to the next registered device */
-mux_set_focus(chr, (d->focus + 1) % d->mux_cnt);
+bit = find_next_bit(&d->mux_bitset, MAX_MUX, d->focus + 1);
+if (bit >= MAX_MUX) {
+bit = find_next_bit(&d->mux_bitset, MAX_MUX, 0);
+}
+mux_set_focus(chr, bit);
 break;
-case 't':
+} case 't':
 d->timestamps = !d->timestamps;
 d->timestamps_start = -1;
 d->linestart = false;
@@ -243,15 +250,16 @@ static void mux_chr_read(void *opaque, const uint8_t 
*buf, int size)
 void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event)
 {
 MuxChardev *d = MUX_CHARDEV(chr);
-unsigned int i;
+int bit;
 
 if (!muxes_opened) {
 return;
 }
 
 /* Send the event to all registered listeners */
-for (i = 0; i < d->mux_cnt; i++) {
-mux_chr_send_event(d, i, event);
+bit = -1;
+while ((bit = find_next_bit(&d->mux_bitset, MAX_MUX, bit + 1)) < MAX_MUX) {
+mux_chr_send_event(d, bit, event);
 }
 }
 
@@ -276,10 +284,11 @@ static GSource *mux_chr_add_watch(Chardev *s, 
GIOCondition cond)
 static void char_mux_finalize(Object *obj)
 {
 MuxChardev *d = MUX_CHARDEV(obj);
-unsigned int i;
+int bit;
 
-for (i = 0; i < d->mux_cnt; i++) {
-CharBackend *be = d->backends[i];
+bit = -1;
+while ((bit = find_next_bit(&d->mux_bitset, MAX_MUX, bit + 1)) < MAX_MUX) {
+CharBackend *be = d->backends[bit];
 if (be) {
 be->chr = NULL;
 }
@@ -304,7 +313,10 @@ static void mux_chr_update_read_handlers(Chardev *chr)
 bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
  unsigned int *tag, Error **errp)
 {
-if (d->mux_cnt >= MAX_MUX) {
+unsigned int bit;
+
+bit = find_next_zero_bit(&d->mux_bitset, MAX_MUX, 0);
+if (bit >= MAX_MUX) {
 error_setg(errp,
"too many uses of multiplexed chardev '%s'"
" (maximum is " stringify(MAX_MUX) ")",
@@ -312,8 +324,9 @@ bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
 return false;
 }
 
-d->backends[d->mux_cnt] = b;
-*tag = d->mux_cnt++;
+d->mux_bitset |= (1 << bit);
+d->backends[bit] = b;
+*tag = bit;
 
 return true;
 }
@@ -322,7 +335,7 @@ void mux_set_focus(Chardev *chr, unsigned int focus)
 {
 MuxChardev *d = MUX_CHARDEV(chr);
 
-assert(focus < d->mux_cnt);
+assert(find_next_bit(&d->mux_bitset, MAX_MUX, focus) < MAX_MUX);
 
 if (d->focus != -1) {
 mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_OUT);
diff --git a/chardev/char.c b/chardev/char.c
index f54dc3a86286..a1722aa076d9 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -333,7 +333,7 @@ static bool qemu_chr_is_busy(Chardev *s)
 {
 if (CHARDEV_IS_MUX(s)) {
 MuxChardev *d = MUX_CHARDEV(s);
-return d->mux_cnt > 0;
+return d->mux_bitset != 0;
 } else {
 return s->be != NULL;
 }
diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
index 8126ce180690..b89aada5413b 100644
--- a/chardev/chardev-internal.h
+++ b/chardev/chardev-internal.h
@@ -37,8 +37,8 @@ struct MuxChardev {
 Chardev parent;
 CharBackend *backends[MAX_MUX];
 CharBackend chr;
+unsigned long mux_bitset;
 int focus;
-unsigned int mux_cnt;
 bool term_got_escape;
 /* Intermediate input buffer catches escape sequences even if the
currently active device

[PATCH 4/8] chardev/mux: convert size members to unsigned int

There is no sense to keep `focus`, `mux_cnt`, `prod`, `cons`
and `tag` variables as signed, those represent either size,
either position in array, which both are unsigned.

`focus` member of `MuxChardev` is kept signed, because initially
set to -1.

Signed-off-by: Roman Penyaev 
Cc: "Marc-André Lureau" 
Cc: qemu-devel@nongnu.org
---
 chardev/char-fe.c  |  2 +-
 chardev/char-mux.c | 10 +-
 chardev/chardev-internal.h |  8 
 include/chardev/char-fe.h  |  2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index b214ba3802b1..69b47d16bdfa 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -191,7 +191,7 @@ bool qemu_chr_fe_backend_open(CharBackend *be)
 
 bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp)
 {
-int tag = 0;
+unsigned int tag = 0;
 
 if (s) {
 if (CHARDEV_IS_MUX(s)) {
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 728596c6f346..b2d7abf2fc01 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -124,7 +124,8 @@ static void mux_print_help(Chardev *chr)
 }
 }
 
-static void mux_chr_send_event(MuxChardev *d, int mux_nr, QEMUChrEvent event)
+static void mux_chr_send_event(MuxChardev *d, unsigned int mux_nr,
+   QEMUChrEvent event)
 {
 CharBackend *be = d->backends[mux_nr];
 
@@ -242,7 +243,7 @@ static void mux_chr_read(void *opaque, const uint8_t *buf, 
int size)
 void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event)
 {
 MuxChardev *d = MUX_CHARDEV(chr);
-int i;
+unsigned int i;
 
 if (!muxes_opened) {
 return;
@@ -275,7 +276,7 @@ static GSource *mux_chr_add_watch(Chardev *s, GIOCondition 
cond)
 static void char_mux_finalize(Object *obj)
 {
 MuxChardev *d = MUX_CHARDEV(obj);
-int i;
+unsigned int i;
 
 for (i = 0; i < d->mux_cnt; i++) {
 CharBackend *be = d->backends[i];
@@ -300,11 +301,10 @@ static void mux_chr_update_read_handlers(Chardev *chr)
   chr->gcontext, true, false);
 }
 
-void mux_set_focus(Chardev *chr, int focus)
+void mux_set_focus(Chardev *chr, unsigned int focus)
 {
 MuxChardev *d = MUX_CHARDEV(chr);
 
-assert(focus >= 0);
 assert(focus < d->mux_cnt);
 
 if (d->focus != -1) {
diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
index 975c16de803e..ab93f6ea1720 100644
--- a/chardev/chardev-internal.h
+++ b/chardev/chardev-internal.h
@@ -38,14 +38,14 @@ struct MuxChardev {
 CharBackend *backends[MAX_MUX];
 CharBackend chr;
 int focus;
-int mux_cnt;
+unsigned int mux_cnt;
 bool term_got_escape;
 /* Intermediate input buffer catches escape sequences even if the
currently active device is not accepting any input - but only until it
is full as well. */
 unsigned char buffer[MAX_MUX][MUX_BUFFER_SIZE];
-int prod[MAX_MUX];
-int cons[MAX_MUX];
+unsigned int prod[MAX_MUX];
+unsigned int cons[MAX_MUX];
 int timestamps;
 
 /* Protected by the Chardev chr_write_lock.  */
@@ -59,7 +59,7 @@ DECLARE_INSTANCE_CHECKER(MuxChardev, MUX_CHARDEV,
 #define CHARDEV_IS_MUX(chr) \
 object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_MUX)
 
-void mux_set_focus(Chardev *chr, int focus);
+void mux_set_focus(Chardev *chr, unsigned int focus);
 void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event);
 
 Object *get_chardevs_root(void);
diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
index 3310449eaf03..8ef05b3dd095 100644
--- a/include/chardev/char-fe.h
+++ b/include/chardev/char-fe.h
@@ -20,7 +20,7 @@ struct CharBackend {
 IOReadHandler *chr_read;
 BackendChangeHandler *chr_be_change;
 void *opaque;
-int tag;
+unsigned int tag;
 bool fe_is_open;
 };
 
-- 
2.34.1

[PATCH 3/8] chardev/mux: use bool type for `linestart` and `term_got_escape`

Those are boolean variables, not signed integers.

Signed-off-by: Roman Penyaev 
Cc: "Marc-André Lureau" 
Cc: qemu-devel@nongnu.org
---
 chardev/char-mux.c | 10 +-
 chardev/chardev-internal.h |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index ee2d47b20d9b..728596c6f346 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -73,11 +73,11 @@ static int mux_chr_write(Chardev *chr, const uint8_t *buf, 
int len)
  * qemu_chr_fe_write and background I/O callbacks */
 qemu_chr_fe_write_all(&d->chr,
   (uint8_t *)buf1, strlen(buf1));
-d->linestart = 0;
+d->linestart = false;
 }
 ret += qemu_chr_fe_write(&d->chr, buf + i, 1);
 if (buf[i] == '\n') {
-d->linestart = 1;
+d->linestart = true;
 }
 }
 }
@@ -145,7 +145,7 @@ static void mux_chr_be_event(Chardev *chr, QEMUChrEvent 
event)
 static int mux_proc_byte(Chardev *chr, MuxChardev *d, int ch)
 {
 if (d->term_got_escape) {
-d->term_got_escape = 0;
+d->term_got_escape = false;
 if (ch == term_escape_char) {
 goto send_char;
 }
@@ -175,11 +175,11 @@ static int mux_proc_byte(Chardev *chr, MuxChardev *d, int 
ch)
 case 't':
 d->timestamps = !d->timestamps;
 d->timestamps_start = -1;
-d->linestart = 0;
+d->linestart = false;
 break;
 }
 } else if (ch == term_escape_char) {
-d->term_got_escape = 1;
+d->term_got_escape = true;
 } else {
 send_char:
 return 1;
diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
index c3024b51fdda..975c16de803e 100644
--- a/chardev/chardev-internal.h
+++ b/chardev/chardev-internal.h
@@ -39,7 +39,7 @@ struct MuxChardev {
 CharBackend chr;
 int focus;
 int mux_cnt;
-int term_got_escape;
+bool term_got_escape;
 /* Intermediate input buffer catches escape sequences even if the
currently active device is not accepting any input - but only until it
is full as well. */
@@ -49,7 +49,7 @@ struct MuxChardev {
 int timestamps;
 
 /* Protected by the Chardev chr_write_lock.  */
-int linestart;
+bool linestart;
 int64_t timestamps_start;
 };
 typedef struct MuxChardev MuxChardev;
-- 
2.34.1

Re: [PATCH] configure, meson: synchronize defaults for configure and Meson Rust options

On Mon, 14 Oct 2024 at 12:01, Paolo Bonzini  wrote:
>
> If the defaults for --enable-rust ($rust in configure) and Meson's rust
> option are out of sync, incremental builds will pick Meson's default.
>
> This happens because, on an incremental build, configure does not run
> Meson, Make does instead.  Meson then gets the command line options
> from either coredata.dat (which has everything cached in Python's pickle
> format) or cmd_line.txt (slow path when Meson version is upgraded), but
> neither knows about the rust option, and the meson_options.txt default
> is used.
>
> This will cause have_rust to be true if rustc is available; and the build
> to fail because configure did not put a RUST_TARGET_TRIPLE in config-host.mak.
>
> When in the Rust pull request I changed the $rust default from auto
> to disabled, I should have made the same change to meson_options.txt;
> do it now.
>
> Cc: Manos Pitsidianakis 
> Reported-by: Peter Maydell 
> Reported-by: Daniel P. Berrangé 
> Signed-off-by: Paolo Bonzini 

This fixes the issue I was seeing with my local
incremental rebuild.

Tested-by: Peter Maydell 

-- PMM

Re: [PATCH 1/1] chardev/char: fix qemu_chr_is_busy() check

Hi Marc-André,

On Thu, Oct 10, 2024 at 12:20 PM Marc-André Lureau
 wrote:
>
> Hi Roman
>
> On Thu, Oct 10, 2024 at 1:28 PM Roman Penyaev  wrote:
>>
>> `mux_cnt` struct member never goes negative or decrements,
>> so mux chardev can be !busy only when there are no
>> frontends attached. This patch fixes the always-true
>> check.
>>
>> Fixes: a4afa548fc6d ("char: move front end handlers in CharBackend")
>> Signed-off-by: Roman Penyaev 
>> Cc: "Marc-André Lureau" 
>> Cc: qemu-devel@nongnu.org
>
>
> Reviewed-by: Marc-André Lureau 
>
> That would be worth some new tests for chardev removal. It seems to be 
> lacking. And mux probably need extra fixing. I can take a look if you don't.

I've just sent an attempt to fix the removal of frontends, plus some test cases.
Please take a look.

Thanks.

--
Roman

[PATCH 0/8] chardev/mux: implement frontend detach

Frontend device can be detached in run-time, which can lead to a
"Chardev 'MUX' is busy" error (see the last patch with the test case
implementation). This series implements frontend detach for the
multiplexer based on bitset, which provides the ability to attach or
detach frontend devices in any order.

Also first patches do some refactoring the purpose of which is to make
integer unsigned where possible (such as sizes or lengths).

Roman Penyaev (8):
  chardev/char: fix qemu_chr_is_busy() check
  chardev/chardev-internal: remove unused `max_size` struct member
  chardev/mux: use bool type for `linestart` and `term_got_escape`
  chardev/mux: convert size members to unsigned int
  chardev/mux: introduce `mux_chr_attach_frontend() call
  chardev/mux: switch mux frontends management to bitset
  chardev/mux: implement detach of frontends from mux
  tests/unit/test-char: implement a few mux remove test cases

 chardev/char-fe.c  | 13 ++
 chardev/char-mux.c | 88 --
 chardev/char.c |  2 +-
 chardev/chardev-internal.h | 16 ---
 include/chardev/char-fe.h  |  2 +-
 tests/unit/test-char.c | 24 ++-
 6 files changed, 103 insertions(+), 42 deletions(-)

Signed-off-by: Roman Penyaev 
Cc: "Marc-André Lureau" 
Cc: qemu-devel@nongnu.org

-- 
2.34.1

[PATCH 1/8] chardev/char: fix qemu_chr_is_busy() check

`mux_cnt` struct member never goes negative or decrements,
so mux chardev can be !busy only when there are no
frontends attached. This patch fixes the always-true
check.

Fixes: a4afa548fc6d ("char: move front end handlers in CharBackend")
Signed-off-by: Roman Penyaev 
Cc: "Marc-André Lureau" 
Cc: qemu-devel@nongnu.org
---
 chardev/char.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/chardev/char.c b/chardev/char.c
index c0cc52824b48..f54dc3a86286 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -333,7 +333,7 @@ static bool qemu_chr_is_busy(Chardev *s)
 {
 if (CHARDEV_IS_MUX(s)) {
 MuxChardev *d = MUX_CHARDEV(s);
-return d->mux_cnt >= 0;
+return d->mux_cnt > 0;
 } else {
 return s->be != NULL;
 }
-- 
2.34.1

[PATCH 8/8] tests/unit/test-char: implement a few mux remove test cases

This patch tests:

1. feasibility of removing mux which does not have frontends attached
   or frontends were prior detached.
2. inability to remove mux which has frontends attached (mux is "busy")

Signed-off-by: Roman Penyaev 
Cc: "Marc-André Lureau" 
Cc: qemu-devel@nongnu.org
---
 tests/unit/test-char.c | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
index f273ce522612..2837dbb863a8 100644
--- a/tests/unit/test-char.c
+++ b/tests/unit/test-char.c
@@ -1,6 +1,7 @@
 #include "qemu/osdep.h"
 #include 
 
+#include "qapi/error.h"
 #include "qemu/config-file.h"
 #include "qemu/module.h"
 #include "qemu/option.h"
@@ -184,6 +185,21 @@ static void char_mux_test(void)
 char *data;
 FeHandler h1 = { 0, false, 0, false, }, h2 = { 0, false, 0, false, };
 CharBackend chr_be1, chr_be2;
+Error *error = NULL;
+
+/* Create mux and chardev to be immediately removed */
+opts = qemu_opts_create(qemu_find_opts("chardev"), "mux-label",
+1, &error_abort);
+qemu_opt_set(opts, "backend", "ringbuf", &error_abort);
+qemu_opt_set(opts, "size", "128", &error_abort);
+qemu_opt_set(opts, "mux", "on", &error_abort);
+chr = qemu_chr_new_from_opts(opts, NULL, &error_abort);
+g_assert_nonnull(chr);
+qemu_opts_del(opts);
+
+/* Remove just created mux and chardev */
+qmp_chardev_remove("mux-label", &error_abort);
+qmp_chardev_remove("mux-label-base", &error_abort);
 
 opts = qemu_opts_create(qemu_find_opts("chardev"), "mux-label",
 1, &error_abort);
@@ -334,7 +350,13 @@ static void char_mux_test(void)
 g_free(data);
 
 qemu_chr_fe_deinit(&chr_be1, false);
-qemu_chr_fe_deinit(&chr_be2, true);
+
+error = NULL;
+qmp_chardev_remove("mux-label", &error);
+g_assert_cmpstr(error_get_pretty(error), ==, "Chardev 'mux-label' is 
busy");
+
+qemu_chr_fe_deinit(&chr_be2, false);
+qmp_chardev_remove("mux-label", &error_abort);
 }
 
 
-- 
2.34.1

[PATCH 2/8] chardev/chardev-internal: remove unused `max_size` struct member

Clean up forgotten leftovers.

Signed-off-by: Roman Penyaev 
Cc: "Marc-André Lureau" 
Cc: qemu-devel@nongnu.org
---
 chardev/chardev-internal.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
index 4e03af31476c..c3024b51fdda 100644
--- a/chardev/chardev-internal.h
+++ b/chardev/chardev-internal.h
@@ -40,7 +40,6 @@ struct MuxChardev {
 int focus;
 int mux_cnt;
 int term_got_escape;
-int max_size;
 /* Intermediate input buffer catches escape sequences even if the
currently active device is not accepting any input - but only until it
is full as well. */
-- 
2.34.1

[PATCH 5/8] chardev/mux: introduce `mux_chr_attach_frontend() call

Move away logic which attaches frontend device to a mux
from `char-fe.c` to actual `char-mux.c` implementation
and make it a separate function.

No logic changes are made.

Signed-off-by: Roman Penyaev 
Cc: "Marc-André Lureau" 
Cc: qemu-devel@nongnu.org
---
 chardev/char-fe.c  |  9 +
 chardev/char-mux.c | 17 +
 chardev/chardev-internal.h |  2 ++
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 69b47d16bdfa..3b8771ca2ac4 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -197,16 +197,9 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error 
**errp)
 if (CHARDEV_IS_MUX(s)) {
 MuxChardev *d = MUX_CHARDEV(s);
 
-if (d->mux_cnt >= MAX_MUX) {
-error_setg(errp,
-   "too many uses of multiplexed chardev '%s'"
-   " (maximum is " stringify(MAX_MUX) ")",
-   s->label);
+if (!mux_chr_attach_frontend(d, b, &tag, errp)) {
 return false;
 }
-
-d->backends[d->mux_cnt] = b;
-tag = d->mux_cnt++;
 } else if (s->be) {
 error_setg(errp, "chardev '%s' is already in use", s->label);
 return false;
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index b2d7abf2fc01..9294f955462e 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -301,6 +301,23 @@ static void mux_chr_update_read_handlers(Chardev *chr)
   chr->gcontext, true, false);
 }
 
+bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
+ unsigned int *tag, Error **errp)
+{
+if (d->mux_cnt >= MAX_MUX) {
+error_setg(errp,
+   "too many uses of multiplexed chardev '%s'"
+   " (maximum is " stringify(MAX_MUX) ")",
+   d->parent.label);
+return false;
+}
+
+d->backends[d->mux_cnt] = b;
+*tag = d->mux_cnt++;
+
+return true;
+}
+
 void mux_set_focus(Chardev *chr, unsigned int focus)
 {
 MuxChardev *d = MUX_CHARDEV(chr);
diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
index ab93f6ea1720..8126ce180690 100644
--- a/chardev/chardev-internal.h
+++ b/chardev/chardev-internal.h
@@ -59,6 +59,8 @@ DECLARE_INSTANCE_CHECKER(MuxChardev, MUX_CHARDEV,
 #define CHARDEV_IS_MUX(chr) \
 object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_MUX)
 
+bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
+ unsigned int *tag, Error **errp);
 void mux_set_focus(Chardev *chr, unsigned int focus);
 void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event);
 
-- 
2.34.1

[PATCH 7/8] chardev/mux: implement detach of frontends from mux

With bitset management now it becomes feasible to implement
the logic of detaching frontends from multiplexer.

Signed-off-by: Roman Penyaev 
Cc: "Marc-André Lureau" 
Cc: qemu-devel@nongnu.org
---
 chardev/char-fe.c  |  2 +-
 chardev/char-mux.c | 20 +---
 chardev/chardev-internal.h |  1 +
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 3b8771ca2ac4..8ac6bebb6f74 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -225,7 +225,7 @@ void qemu_chr_fe_deinit(CharBackend *b, bool del)
 }
 if (CHARDEV_IS_MUX(b->chr)) {
 MuxChardev *d = MUX_CHARDEV(b->chr);
-d->backends[b->tag] = NULL;
+mux_chr_detach_frontend(d, b->tag);
 }
 if (del) {
 Object *obj = OBJECT(b->chr);
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 9c3cacb2fecd..649f8ff6ccbf 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -289,10 +289,10 @@ static void char_mux_finalize(Object *obj)
 bit = -1;
 while ((bit = find_next_bit(&d->mux_bitset, MAX_MUX, bit + 1)) < MAX_MUX) {
 CharBackend *be = d->backends[bit];
-if (be) {
-be->chr = NULL;
-}
+be->chr = NULL;
+d->backends[bit] = NULL;
 }
+d->mux_bitset = 0;
 qemu_chr_fe_deinit(&d->chr, false);
 }
 
@@ -331,6 +331,21 @@ bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
 return true;
 }
 
+bool mux_chr_detach_frontend(MuxChardev *d, unsigned int tag)
+{
+unsigned int bit;
+
+bit = find_next_bit(&d->mux_bitset, MAX_MUX, tag);
+if (bit >= MAX_MUX) {
+return false;
+}
+
+d->mux_bitset &= ~(1 << bit);
+d->backends[bit] = NULL;
+
+return true;
+}
+
 void mux_set_focus(Chardev *chr, unsigned int focus)
 {
 MuxChardev *d = MUX_CHARDEV(chr);
diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
index b89aada5413b..853807f3cb88 100644
--- a/chardev/chardev-internal.h
+++ b/chardev/chardev-internal.h
@@ -61,6 +61,7 @@ DECLARE_INSTANCE_CHECKER(MuxChardev, MUX_CHARDEV,
 
 bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
  unsigned int *tag, Error **errp);
+bool mux_chr_detach_frontend(MuxChardev *d, unsigned int tag);
 void mux_set_focus(Chardev *chr, unsigned int focus);
 void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event);
 
-- 
2.34.1

Re: [PATCH 1/1] hw/cxl/cxl-mailbox-utils: Fix for device DDR5 ECS control feature tables

On Mon, 14 Oct 2024 11:09:12 +0100
Jonathan Cameron via  wrote:

> On Fri, 27 Sep 2024 10:17:43 +0100
>  wrote:
> 
> > From: Shiju Jose 
> > 
> > CXL spec 3.1 section 8.2.9.9.11.2 describes the DDR5 Error Check Scrub (ECS)
> > control feature.
> > 
> > ECS log capabilities field in following ECS tables, which is common for all
> > memory media FRUs in a CXL device.
> > 
> > Fix struct CXLMemECSReadAttrs and struct CXLMemECSWriteAttrs to make
> > log entry type field common.
> > 
> > Fixes:2d41ce38fb9a (hw/cxl/cxl-mailbox-utils: Add device DDR5 ECS control 
> > feature)
Fixes tag format is wrong. I've fixed up as.
Fixes: 2d41ce38fb9a ("hw/cxl/cxl-mailbox-utils: Add device DDR5 ECS control 
feature")

> > Signed-off-by: Shiju Jose   
> Hi Shiju
> 
> Sorry for taking so long to get to this! 
> 
> Anyhow, one trivial comment inline that I'll fix up.
> I'm preparing a fixes series today so will include this patch in that.
> 
> Thanks,
> 
> Jonathan
> 
> > ---
> >  hw/cxl/cxl-mailbox-utils.c  | 27 ++-
> >  hw/mem/cxl_type3.c  |  9 -
> >  include/hw/cxl/cxl_device.h | 36 ++--
> >  3 files changed, 36 insertions(+), 36 deletions(-)
> > 
> > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> > index c2d776bc96..fa06cd7910 100644
> > --- a/hw/cxl/cxl-mailbox-utils.c
> > +++ b/hw/cxl/cxl-mailbox-utils.c
> > @@ -1152,10 +1152,8 @@ static CXLRetCode cmd_features_get_supported(const 
> > struct cxl_cmd *cmd,
> >   (struct CXLSupportedFeatureEntry) {
> >  .uuid = ecs_uuid,
> >  .feat_index = index,
> > -.get_feat_size = CXL_ECS_NUM_MEDIA_FRUS *
> > -sizeof(CXLMemECSReadAttrs),
> > -.set_feat_size = CXL_ECS_NUM_MEDIA_FRUS *
> > -sizeof(CXLMemECSWriteAttrs),
> > +.get_feat_size = sizeof(CXLMemECSReadAttrs),
> > +.set_feat_size = sizeof(CXLMemECSWriteAttrs),
> >  .attr_flags = CXL_FEAT_ENTRY_ATTR_FLAG_CHANGABLE,
> >  .get_feat_version = CXL_ECS_GET_FEATURE_VERSION,
> >  .set_feat_version = CXL_ECS_SET_FEATURE_VERSION,
> > @@ -1223,13 +1221,10 @@ static CXLRetCode cmd_features_get_feature(const 
> > struct cxl_cmd *cmd,
> > (uint8_t *)&ct3d->patrol_scrub_attrs + get_feature->offset,
> > bytes_to_copy);
> >  } else if (qemu_uuid_is_equal(&get_feature->uuid, &ecs_uuid)) {
> > -if (get_feature->offset >=  CXL_ECS_NUM_MEDIA_FRUS *
> > -sizeof(CXLMemECSReadAttrs)) {
> > +if (get_feature->offset >= sizeof(CXLMemECSReadAttrs)) {
> >  return CXL_MBOX_INVALID_INPUT;
> >  }
> > -bytes_to_copy = CXL_ECS_NUM_MEDIA_FRUS *
> > -sizeof(CXLMemECSReadAttrs) -
> > -get_feature->offset;
> > +bytes_to_copy = sizeof(CXLMemECSReadAttrs) - get_feature->offset;
> >  bytes_to_copy = MIN(bytes_to_copy, get_feature->count);
> >  memcpy(payload_out,
> > (uint8_t *)&ct3d->ecs_attrs + get_feature->offset,
> > @@ -1318,19 +1313,17 @@ static CXLRetCode cmd_features_set_feature(const 
> > struct cxl_cmd *cmd,
> >  
> >  ecs_set_feature = (void *)payload_in;
> >  ecs_write_attrs = ecs_set_feature->feat_data;
> > -memcpy((uint8_t *)ct3d->ecs_wr_attrs + hdr->offset,
> > +memcpy((uint8_t *)&ct3d->ecs_wr_attrs + hdr->offset,
> > ecs_write_attrs,
> > bytes_to_copy);
> >  set_feat_info->data_size += bytes_to_copy;
> >  
> >  if (data_transfer_flag == CXL_SET_FEATURE_FLAG_FULL_DATA_TRANSFER 
> > ||
> >  data_transfer_flag ==  
> > CXL_SET_FEATURE_FLAG_FINISH_DATA_TRANSFER) {
> > -for (count = 0; count < CXL_ECS_NUM_MEDIA_FRUS; count++) {
> > -ct3d->ecs_attrs[count].ecs_log_cap =
> > -  ct3d->ecs_wr_attrs[count].ecs_log_cap;
> > -ct3d->ecs_attrs[count].ecs_config =
> > -  ct3d->ecs_wr_attrs[count].ecs_config & 
> > 0x1F;
> > -}
> > +ct3d->ecs_attrs.ecs_log_cap = ct3d->ecs_wr_attrs.ecs_log_cap;
> > +for (count = 0; count < CXL_ECS_NUM_MEDIA_FRUS; count++)  
> Qemu style needs the brackets even for one line cases. I'll fix up whilst 
> applying
> Also side effect will be reducing the diff which is nice to have :)
> 
> > +ct3d->ecs_attrs.fru_attrs[count].ecs_config =
> > +ct3d->ecs_wr_attrs.fru_attrs[count].ecs_config & 
> > 0x1F;
> >  }
> >  } else {
> >  return CXL_MBOX_UNSUPPORTED;
> > @@ -1343,7 +1336,7 @@ static CXLRetCode cmd_features_set_feature(const 
> > struct cxl_cmd *cmd,
> >  if (qemu_uuid_is_equal(&hdr->uuid, &patrol_scrub_uuid)) {

Re: [PULL 00/27] tcg + linux patch queue

On Sun, 13 Oct 2024 at 23:20, Richard Henderson
 wrote:
>
> The following changes since commit 7e3b6d8063f245d27eecce5aabe624b5785f2a77:
>
>   Merge tag 'crypto-fixes-pull-request' of https://gitlab.com/berrange/qemu 
> into staging (2024-10-10 18:05:43 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20241013
>
> for you to fetch changes up to e530581ee06573fcf48c7f7a6c3f8ec6e5809243:
>
>   target/arm: Fix alignment fault priority in get_phys_addr_lpae (2024-10-13 
> 11:27:06 -0700)
>
> 
> linux-user/i386: Emulate orig_ax
> linux-user/vm86: Fix compilation with Clang
> tcg: remove singlestep_enabled from DisasContextBase
> accel/tcg: Add TCGCPUOps.tlb_fill_align
> target/hppa: Handle alignment faults in hppa_get_physical_address
> target/arm: Fix alignment fault priority in get_phys_addr_lpae
>


Applied, thanks.

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

-- PMM

[PATCH qemu 1/7] hw/cxl: Fix uint32 overflow cxl-mailbox-utils.c

From: Dmitry Frolov 

The sum offset + length may overflow uint32. Since this sum is
compared with uint64_t return value of get_lsa_size(), it makes
sense to choose uint64_t type for offset and length.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 3ebe676a3463 ("hw/cxl/device: Implement get/set Label Storage Area 
(LSA)")
Signed-off-by: Dmitry Frolov 
Link: https://lore.kernel.org/r/20240917080925.270597-2-fro...@swemel.ru
Signed-off-by: Jonathan Cameron 
---
 hw/cxl/cxl-mailbox-utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 9258e48f95..9f794e4655 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -1445,7 +1445,7 @@ static CXLRetCode cmd_ccls_get_lsa(const struct cxl_cmd 
*cmd,
 } QEMU_PACKED *get_lsa;
 CXLType3Dev *ct3d = CXL_TYPE3(cci->d);
 CXLType3Class *cvc = CXL_TYPE3_GET_CLASS(ct3d);
-uint32_t offset, length;
+uint64_t offset, length;
 
 get_lsa = (void *)payload_in;
 offset = get_lsa->offset;
-- 
2.43.0

[PATCH qemu 3/7] mem/cxl_type3: Fix overlapping region validation error

From: Yao Xingtao 

When injecting a new poisoned region through qmp_cxl_inject_poison(),
the newly injected region should not overlap with existing poisoned
regions.

The current validation method does not consider the following
overlapping region:
┌───┬───┬───┐
│a  │  b(a) │a  │
└───┴───┴───┘
(a is a newly added region, b is an existing region, and b is a
 subregion of a)

Fixes: 9547754f40ee ("hw/cxl: QMP based poison injection support")
Signed-off-by: Yao Xingtao 
Signed-off-by: Jonathan Cameron 
---
 hw/mem/cxl_type3.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index 44d491d8f6..16c60b9b0d 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -1381,9 +1381,7 @@ void qmp_cxl_inject_poison(const char *path, uint64_t 
start, uint64_t length,
 ct3d = CXL_TYPE3(obj);
 
 QLIST_FOREACH(p, &ct3d->poison_list, node) {
-if (((start >= p->start) && (start < p->start + p->length)) ||
-((start + length > p->start) &&
- (start + length <= p->start + p->length))) {
+if ((start < p->start + p->length) && (start + length > p->start)) {
 error_setg(errp,
"Overlap with existing poisoned region not supported");
 return;
-- 
2.43.0

[PATCH qemu 2/7] hw/cxl: Fix background completion percentage calculation