All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] target/riscv/cpu.c: duct tape fix for QMP + RV profiles introspection
@ 2026-10-21 17:31 Daniel Henrique Barboza
  2026-10-22  2:28 ` Alistair Francis
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Daniel Henrique Barboza @ 2026-10-21 17:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, chao.liu,
	armbru, philmd, Daniel Henrique Barboza, Palmer Dabbelt

Maruks reported in [1] a problem with QMP introspection with
qemu-system-riscv64 that happens after querying a profile CPU.  QEMU
crashes hitting an abort during satp_mode logic that, somehow, is being
triggered via profile code.

For every profile CPU that goes through init we're setting an 'enabled'
flag in the static RISCVProfile object.  This sounded like an okay idea
back when profiles were introduced, but now we've realized that QMP will
querying CPUs without exiting the QEMU process while also going multiple
times through cpu_init() and friends.  The result is that we're in a
situation where we have runtime information being stored in static arrays
that persists between CPU definitions being introspected.

A proper fix is to get rid of profile->enabled and turn profiles into
cpu->cfg flags like we do with every other extension.  Unfortunately
we've been using profile->enabled for lots of things and a redesign
would be in order, one which I'm not comfortable doing right now - we're
too close to the QEMU 11.2 code freeze for such a change.

For now let's silver tape a fix in cpu_init() to reset profile->enabled
for all available profiles.  We'll do a proper redesign in the next
release cycle.

[1] https://lore.kernel.org/qemu-devel/87ld7rmdi6.fsf@pond.sub.org/

Reported-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
---
 target/riscv/cpu.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 08a4d14ea1..46b74302db 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1550,6 +1550,25 @@ static void riscv_cpu_init(Object *obj)
     cpu->env.vext_ver = VEXT_VERSION_1_00_0;
     cpu->cfg.max_satp_mode = -1;
 
+    /*
+     * The way we're handling profiles does not cope well with QMP
+     * instropection.  We have a static array that stores RISCVCPUProfiles
+     * objs that hold runtime state for the lifetime of the QEMU process.
+     * Thus setting profile->enabled once, e.g. when instropecting a
+     * profile CPU, will leave the flag set for subsequent intropections.
+     *
+     * For now, clear all 'enabled' flags to avoid misleading QMP w.r.t
+     * profiles being set.
+     *
+     * FIXME: a better long term solution is to remove profile->enabled
+     * and just use regular cpu->cfg flags, avoiding static variables
+     * storing dynamic state.  This will help us not only with QMP but
+     * also single-binary support.
+     */
+    for (int i = 0; riscv_profiles[i] != NULL; i++) {
+        riscv_profiles[i]->enabled = false;
+    }
+
     if (mcc->def->profile) {
         mcc->def->profile->enabled = true;
     }
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] target/riscv/cpu.c: duct tape fix for QMP + RV profiles introspection
  2026-10-21 17:31 [PATCH] target/riscv/cpu.c: duct tape fix for QMP + RV profiles introspection Daniel Henrique Barboza
@ 2026-10-22  2:28 ` Alistair Francis
  2026-10-22  2:34 ` Alistair Francis
  2026-10-22  6:00 ` Markus Armbruster
  2 siblings, 0 replies; 6+ messages in thread
From: Alistair Francis @ 2026-10-22  2:28 UTC (permalink / raw)
  To: daniel.barboza@oss.qualcomm.com, qemu-devel@nongnu.org
  Cc: philmd@oss.qualcomm.com, qemu-riscv@nongnu.org,
	palmer@dabbelt.com, chao.liu@processmission.com,
	armbru@redhat.com, liwei1518@gmail.com,
	zhiwei_liu@linux.alibaba.com

On Wed, 2026-10-21 at 14:31 -0300, Daniel Henrique Barboza wrote:
> Maruks reported in [1] a problem with QMP introspection with
> qemu-system-riscv64 that happens after querying a profile CPU.  QEMU
> crashes hitting an abort during satp_mode logic that, somehow, is
> being
> triggered via profile code.
> 
> For every profile CPU that goes through init we're setting an
> 'enabled'
> flag in the static RISCVProfile object.  This sounded like an okay
> idea
> back when profiles were introduced, but now we've realized that QMP
> will
> querying CPUs without exiting the QEMU process while also going
> multiple
> times through cpu_init() and friends.  The result is that we're in a
> situation where we have runtime information being stored in static
> arrays
> that persists between CPU definitions being introspected.
> 
> A proper fix is to get rid of profile->enabled and turn profiles into
> cpu->cfg flags like we do with every other extension.  Unfortunately
> we've been using profile->enabled for lots of things and a redesign
> would be in order, one which I'm not comfortable doing right now -
> we're
> too close to the QEMU 11.2 code freeze for such a change.
> 
> For now let's silver tape a fix in cpu_init() to reset profile-
> >enabled
> for all available profiles.  We'll do a proper redesign in the next
> release cycle.
> 
> [1] https://lore.kernel.org/qemu-devel/87ld7rmdi6.fsf@pond.sub.org/
> 
> Reported-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Daniel Henrique Barboza
> <daniel.barboza@oss.qualcomm.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 08a4d14ea1..46b74302db 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1550,6 +1550,25 @@ static void riscv_cpu_init(Object *obj)
>      cpu->env.vext_ver = VEXT_VERSION_1_00_0;
>      cpu->cfg.max_satp_mode = -1;
>  
> +    /*
> +     * The way we're handling profiles does not cope well with QMP
> +     * instropection.  We have a static array that stores
> RISCVCPUProfiles
> +     * objs that hold runtime state for the lifetime of the QEMU
> process.
> +     * Thus setting profile->enabled once, e.g. when instropecting a
> +     * profile CPU, will leave the flag set for subsequent
> intropections.
> +     *
> +     * For now, clear all 'enabled' flags to avoid misleading QMP
> w.r.t
> +     * profiles being set.
> +     *
> +     * FIXME: a better long term solution is to remove profile-
> >enabled
> +     * and just use regular cpu->cfg flags, avoiding static
> variables
> +     * storing dynamic state.  This will help us not only with QMP
> but
> +     * also single-binary support.
> +     */
> +    for (int i = 0; riscv_profiles[i] != NULL; i++) {
> +        riscv_profiles[i]->enabled = false;
> +    }
> +
>      if (mcc->def->profile) {
>          mcc->def->profile->enabled = true;
>      }

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] target/riscv/cpu.c: duct tape fix for QMP + RV profiles introspection
  2026-10-21 17:31 [PATCH] target/riscv/cpu.c: duct tape fix for QMP + RV profiles introspection Daniel Henrique Barboza
  2026-10-22  2:28 ` Alistair Francis
@ 2026-10-22  2:34 ` Alistair Francis
  2026-10-22  6:00 ` Markus Armbruster
  2 siblings, 0 replies; 6+ messages in thread
From: Alistair Francis @ 2026-10-22  2:34 UTC (permalink / raw)
  To: daniel.barboza@oss.qualcomm.com, qemu-devel@nongnu.org
  Cc: philmd@oss.qualcomm.com, qemu-riscv@nongnu.org,
	palmer@dabbelt.com, chao.liu@processmission.com,
	armbru@redhat.com, liwei1518@gmail.com,
	zhiwei_liu@linux.alibaba.com

On Wed, 2026-10-21 at 14:31 -0300, Daniel Henrique Barboza wrote:
> Maruks reported in [1] a problem with QMP introspection with
> qemu-system-riscv64 that happens after querying a profile CPU.  QEMU
> crashes hitting an abort during satp_mode logic that, somehow, is
> being
> triggered via profile code.
> 
> For every profile CPU that goes through init we're setting an
> 'enabled'
> flag in the static RISCVProfile object.  This sounded like an okay
> idea
> back when profiles were introduced, but now we've realized that QMP
> will
> querying CPUs without exiting the QEMU process while also going
> multiple
> times through cpu_init() and friends.  The result is that we're in a
> situation where we have runtime information being stored in static
> arrays
> that persists between CPU definitions being introspected.
> 
> A proper fix is to get rid of profile->enabled and turn profiles into
> cpu->cfg flags like we do with every other extension.  Unfortunately
> we've been using profile->enabled for lots of things and a redesign
> would be in order, one which I'm not comfortable doing right now -
> we're
> too close to the QEMU 11.2 code freeze for such a change.
> 
> For now let's silver tape a fix in cpu_init() to reset profile-
> >enabled
> for all available profiles.  We'll do a proper redesign in the next
> release cycle.
> 
> [1] https://lore.kernel.org/qemu-devel/87ld7rmdi6.fsf@pond.sub.org/
> 
> Reported-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Daniel Henrique Barboza
> <daniel.barboza@oss.qualcomm.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/cpu.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 08a4d14ea1..46b74302db 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1550,6 +1550,25 @@ static void riscv_cpu_init(Object *obj)
>      cpu->env.vext_ver = VEXT_VERSION_1_00_0;
>      cpu->cfg.max_satp_mode = -1;
>  
> +    /*
> +     * The way we're handling profiles does not cope well with QMP
> +     * instropection.  We have a static array that stores
> RISCVCPUProfiles
> +     * objs that hold runtime state for the lifetime of the QEMU
> process.
> +     * Thus setting profile->enabled once, e.g. when instropecting a
> +     * profile CPU, will leave the flag set for subsequent
> intropections.
> +     *
> +     * For now, clear all 'enabled' flags to avoid misleading QMP
> w.r.t
> +     * profiles being set.
> +     *
> +     * FIXME: a better long term solution is to remove profile-
> >enabled
> +     * and just use regular cpu->cfg flags, avoiding static
> variables
> +     * storing dynamic state.  This will help us not only with QMP
> but
> +     * also single-binary support.
> +     */
> +    for (int i = 0; riscv_profiles[i] != NULL; i++) {
> +        riscv_profiles[i]->enabled = false;
> +    }
> +
>      if (mcc->def->profile) {
>          mcc->def->profile->enabled = true;
>      }

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] target/riscv/cpu.c: duct tape fix for QMP + RV profiles introspection
  2026-10-21 17:31 [PATCH] target/riscv/cpu.c: duct tape fix for QMP + RV profiles introspection Daniel Henrique Barboza
  2026-10-22  2:28 ` Alistair Francis
  2026-10-22  2:34 ` Alistair Francis
@ 2026-10-22  6:00 ` Markus Armbruster
  2026-10-22 13:21   ` Daniel Henrique Barboza
  2 siblings, 1 reply; 6+ messages in thread
From: Markus Armbruster @ 2026-10-22  6:00 UTC (permalink / raw)
  To: Daniel Henrique Barboza
  Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
	chao.liu, philmd, Palmer Dabbelt

Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com> writes:

> Maruks reported in [1] a problem with QMP introspection with
> qemu-system-riscv64 that happens after querying a profile CPU.  QEMU
> crashes hitting an abort during satp_mode logic that, somehow, is being
> triggered via profile code.
>
> For every profile CPU that goes through init we're setting an 'enabled'
> flag in the static RISCVProfile object.  This sounded like an okay idea
> back when profiles were introduced, but now we've realized that QMP will
> querying CPUs without exiting the QEMU process while also going multiple
> times through cpu_init() and friends.  The result is that we're in a
> situation where we have runtime information being stored in static arrays
> that persists between CPU definitions being introspected.
>
> A proper fix is to get rid of profile->enabled and turn profiles into
> cpu->cfg flags like we do with every other extension.  Unfortunately
> we've been using profile->enabled for lots of things and a redesign
> would be in order, one which I'm not comfortable doing right now - we're
> too close to the QEMU 11.2 code freeze for such a change.
>
> For now let's silver tape a fix in cpu_init() to reset profile->enabled
> for all available profiles.  We'll do a proper redesign in the next
> release cycle.
>
> [1] https://lore.kernel.org/qemu-devel/87ld7rmdi6.fsf@pond.sub.org/
>
> Reported-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

That was quick, thank you!

Tested-by: Markus Armbruster <armbru@redhat.com>



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] target/riscv/cpu.c: duct tape fix for QMP + RV profiles introspection
  2026-10-22  6:00 ` Markus Armbruster
@ 2026-10-22 13:21   ` Daniel Henrique Barboza
  2026-10-23  3:11     ` Alistair Francis
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Henrique Barboza @ 2026-10-22 13:21 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
	chao.liu, philmd, Palmer Dabbelt



On 10/22/2026 3:00 AM, Markus Armbruster wrote:
> Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com> writes:
> 
>> Maruks reported in [1] a problem with QMP introspection with

Just now noticed the typo :D


Alistair, can you fix it in-tree or should I re-send?  Thanks,

Daniel

>> qemu-system-riscv64 that happens after querying a profile CPU.  QEMU
>> crashes hitting an abort during satp_mode logic that, somehow, is being
>> triggered via profile code.
>>
>> For every profile CPU that goes through init we're setting an 'enabled'
>> flag in the static RISCVProfile object.  This sounded like an okay idea
>> back when profiles were introduced, but now we've realized that QMP will
>> querying CPUs without exiting the QEMU process while also going multiple
>> times through cpu_init() and friends.  The result is that we're in a
>> situation where we have runtime information being stored in static arrays
>> that persists between CPU definitions being introspected.
>>
>> A proper fix is to get rid of profile->enabled and turn profiles into
>> cpu->cfg flags like we do with every other extension.  Unfortunately
>> we've been using profile->enabled for lots of things and a redesign
>> would be in order, one which I'm not comfortable doing right now - we're
>> too close to the QEMU 11.2 code freeze for such a change.
>>
>> For now let's silver tape a fix in cpu_init() to reset profile->enabled
>> for all available profiles.  We'll do a proper redesign in the next
>> release cycle.
>>
>> [1] https://lore.kernel.org/qemu-devel/87ld7rmdi6.fsf@pond.sub.org/
>>
>> Reported-by: Markus Armbruster <armbru@redhat.com>
>> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
> 
> That was quick, thank you!
> 
> Tested-by: Markus Armbruster <armbru@redhat.com>
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] target/riscv/cpu.c: duct tape fix for QMP + RV profiles introspection
  2026-10-22 13:21   ` Daniel Henrique Barboza
@ 2026-10-23  3:11     ` Alistair Francis
  0 siblings, 0 replies; 6+ messages in thread
From: Alistair Francis @ 2026-10-23  3:11 UTC (permalink / raw)
  To: daniel.barboza@oss.qualcomm.com, armbru@redhat.com
  Cc: philmd@oss.qualcomm.com, qemu-riscv@nongnu.org,
	palmer@dabbelt.com, chao.liu@processmission.com,
	qemu-devel@nongnu.org, liwei1518@gmail.com,
	zhiwei_liu@linux.alibaba.com

On Thu, 2026-10-22 at 10:21 -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 10/22/2026 3:00 AM, Markus Armbruster wrote:
> > Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com> writes:
> > 
> > > Maruks reported in [1] a problem with QMP introspection with
> 
> Just now noticed the typo :D
> 
> 
> Alistair, can you fix it in-tree or should I re-send?  Thanks,

Fixed it on my side

Alistair

> 
> Daniel
> 
> > > qemu-system-riscv64 that happens after querying a profile CPU. 
> > > QEMU
> > > crashes hitting an abort during satp_mode logic that, somehow, is
> > > being
> > > triggered via profile code.
> > > 
> > > For every profile CPU that goes through init we're setting an
> > > 'enabled'
> > > flag in the static RISCVProfile object.  This sounded like an
> > > okay idea
> > > back when profiles were introduced, but now we've realized that
> > > QMP will
> > > querying CPUs without exiting the QEMU process while also going
> > > multiple
> > > times through cpu_init() and friends.  The result is that we're
> > > in a
> > > situation where we have runtime information being stored in
> > > static arrays
> > > that persists between CPU definitions being introspected.
> > > 
> > > A proper fix is to get rid of profile->enabled and turn profiles
> > > into
> > > cpu->cfg flags like we do with every other extension. 
> > > Unfortunately
> > > we've been using profile->enabled for lots of things and a
> > > redesign
> > > would be in order, one which I'm not comfortable doing right now
> > > - we're
> > > too close to the QEMU 11.2 code freeze for such a change.
> > > 
> > > For now let's silver tape a fix in cpu_init() to reset profile-
> > > >enabled
> > > for all available profiles.  We'll do a proper redesign in the
> > > next
> > > release cycle.
> > > 
> > > [1]
> > > https://lore.kernel.org/qemu-devel/87ld7rmdi6.fsf@pond.sub.org/
> > > 
> > > Reported-by: Markus Armbruster <armbru@redhat.com>
> > > Signed-off-by: Daniel Henrique Barboza
> > > <daniel.barboza@oss.qualcomm.com>
> > 
> > That was quick, thank you!
> > 
> > Tested-by: Markus Armbruster <armbru@redhat.com>
> > 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-23  3:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-21 17:31 [PATCH] target/riscv/cpu.c: duct tape fix for QMP + RV profiles introspection Daniel Henrique Barboza
2026-10-22  2:28 ` Alistair Francis
2026-10-22  2:34 ` Alistair Francis
2026-10-22  6:00 ` Markus Armbruster
2026-10-22 13:21   ` Daniel Henrique Barboza
2026-10-23  3:11     ` Alistair Francis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.