On 11/26/25 11:20 AM, Alvin Che-Chia Chang(張哲嘉) wrote:
Hi Daniel,

-----Original Message-----
From: Daniel Henrique Barboza <[email protected]>
Sent: Wednesday, November 26, 2025 7:38 PM
To: Alvin Che-Chia Chang(張哲嘉) <[email protected]>;
[email protected]; [email protected]
Cc: [email protected]; [email protected];
[email protected]; [email protected]; Yuming Yu-Ming
Chang(張育銘) <[email protected]>
Subject: Re: [PATCH 1/2] target/riscv: Add 'debug_ver' to set version of debug
specification

[EXTERNAL MAIL]

Hi,


There are 2 points I would like to bring here. First is the addition of a new
string property.

Although it's not forbidden, and in fact we have string properties in RISC-V CPU
code (which I'll be dealing with in the next QEMU cycle), we would like to
avoid adding new str props. The reason is that str props are harder to work with
in management layers (e.g. libvirt) and to expose in QMP.

I suggest using booleans instead. They are way easier to work with in the upper
layers and we don't have to maintain string parsing code as well. Instead of
adding a 'debug_ver' string prop and parse values, add two bool properties:
debug-0.13 and debug-1.0.

Using two bool properties instead of string properties is okay to us.
Thank you for guiding us this concern.


Second, we have a 'debug' flag that is set to default 'true'. From what I'm
reading in this patch the existing flag would be equivalent to 'debug-0.13'
I proposed above. In this case we can keep the existing flag and add a single
'debug-1.0' flag to indicate that the user wants to use v1.0 instead of v0.13.

I would prefer renaming existing 'debug' flag to 'debug-v0.13' and adding new 
flag 'debug-v1.0' in order not to confuse users.
How should we handle the situation if user provides both 
debug-v0.13=true,debug-v1.0=true ?
Should we throw an error or treat this situation as if only debug-v1.0=true ?

What you did in v2 works for me. The handling of both 'debug' and 'debug-1.0' is
mostly a backend decision that we're free to make, as long as we're having
common sense in how the flag disable/enable features.

As for renaming/removing existing flags and properties, we can't rename/delete
existing stuff unless we go through a deprecation cycle (add the flags in
deprecated.rst, flag is removed one year later). We do this because there
are existing users and apps that can be compromised when we delete/change
options from the command line.


Thanks,

Daniel



Sincerely,
Alvin Chang



Thanks,

Daniel



On 11/26/25 4:12 AM, Alvin Chang wrote:
The similar control did in 'priv_spec' and 'vext_spec' now is
available for version of debug specification. Currently we accept
"v0.13" and "v1.0" versions. Users can provide 'debug_spec' into CPU
option to set intended version of the debug specification.

For examples:
1. -cpu max,debug_spec=v0.13
2. -cpu max,debug_spec=v1.0

Signed-off-by: Alvin Chang <[email protected]>
Reviewed-by: Yu-Ming Chang <[email protected]>
---
   target/riscv/cpu.c         | 69
++++++++++++++++++++++++++++++++++++++
   target/riscv/cpu.h         | 13 +++++++
   target/riscv/machine.c     |  5 +--
   target/riscv/tcg/tcg-cpu.c |  3 ++
   4 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index
73d4280..dbcdfcd 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1123,6 +1123,7 @@ static void riscv_cpu_init(Object *obj)
       cpu->cfg.pmp_regions = 16;
       cpu->cfg.pmp_granularity = MIN_RISCV_PMP_GRANULARITY;
       cpu->env.vext_ver = VEXT_VERSION_1_00_0;
+    cpu->env.debug_ver = DEBUG_VERSION_0_13_0;
       cpu->cfg.max_satp_mode = -1;

       if (mcc->def->profile) {
@@ -1138,6 +1139,9 @@ static void riscv_cpu_init(Object *obj)
       if (mcc->def->vext_spec != RISCV_PROFILE_ATTR_UNUSED) {
           cpu->env.vext_ver = mcc->def->vext_spec;
       }
+    if (mcc->def->debug_spec != RISCV_PROFILE_ATTR_UNUSED) {
+        cpu->env.debug_ver = mcc->def->debug_spec;
+    }
   #ifndef CONFIG_USER_ONLY
       if (mcc->def->custom_csrs) {
           riscv_register_custom_csrs(cpu, mcc->def->custom_csrs); @@
-1720,6 +1724,66 @@ static const PropertyInfo prop_priv_spec = {
       .set = prop_priv_spec_set,
   };

+static int debug_spec_from_str(const char *debug_spec_str) {
+    int debug_version = -1;
+
+    if (!g_strcmp0(debug_spec_str, DEBUG_VER_0_13_0_STR)) {
+        debug_version = DEBUG_VERSION_0_13_0;
+    } else if (!g_strcmp0(debug_spec_str, DEBUG_VER_1_00_0_STR)) {
+        debug_version = DEBUG_VERSION_1_00_0;
+    }
+
+    return debug_version;
+}
+
+static const char *debug_spec_to_str(int debug_version) {
+    switch (debug_version) {
+    case DEBUG_VERSION_0_13_0:
+        return DEBUG_VER_0_13_0_STR;
+    case DEBUG_VERSION_1_00_0:
+        return DEBUG_VER_1_00_0_STR;
+    default:
+        return NULL;
+    }
+}
+
+static void prop_debug_spec_set(Object *obj, Visitor *v, const char *name,
+                                void *opaque, Error **errp) {
+    RISCVCPU *cpu = RISCV_CPU(obj);
+    g_autofree char *value = NULL;
+    int debug_version = -1;
+
+    visit_type_str(v, name, &value, errp);
+
+    debug_version = debug_spec_from_str(value);
+    if (debug_version < 0) {
+        error_setg(errp, "Unsupported debug spec version '%s'", value);
+        return;
+    }
+
+    cpu_option_add_user_setting(name, debug_version);
+    cpu->env.debug_ver = debug_version; }
+
+static void prop_debug_spec_get(Object *obj, Visitor *v, const char *name,
+                                void *opaque, Error **errp) {
+    RISCVCPU *cpu = RISCV_CPU(obj);
+    const char *value = debug_spec_to_str(cpu->env.debug_ver);
+
+    visit_type_str(v, name, (char **)&value, errp); }
+
+static const PropertyInfo prop_debug_spec = {
+    .type = "str",
+    .description = "debug_spec",
+    .get = prop_debug_spec_get,
+    .set = prop_debug_spec_set,
+};
+
   static void prop_vext_spec_set(Object *obj, Visitor *v, const char *name,
                                  void *opaque, Error **errp)
   {
@@ -2648,6 +2712,7 @@ static const Property riscv_cpu_properties[] = {

       {.name = "priv_spec", .info = &prop_priv_spec},
       {.name = "vext_spec", .info = &prop_vext_spec},
+    {.name = "debug_spec", .info = &prop_debug_spec},

       {.name = "vlen", .info = &prop_vlen},
       {.name = "elen", .info = &prop_elen}, @@ -2818,6 +2883,10 @@
static void riscv_cpu_class_base_init(ObjectClass *c, const void *data)
               assert(def->vext_spec != 0);
               mcc->def->vext_spec = def->vext_spec;
           }
+        if (def->debug_spec != RISCV_PROFILE_ATTR_UNUSED) {
+            assert(def->debug_spec <= DEBUG_VERSION_LATEST);
+            mcc->def->debug_spec = def->debug_spec;
+        }
           mcc->def->misa_ext |= def->misa_ext;

           riscv_cpu_cfg_merge(&mcc->def->cfg, &def->cfg); diff --git
a/target/riscv/cpu.h b/target/riscv/cpu.h index 36e7f10..fc1ae7c
100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -100,6 +100,7 @@ typedef struct riscv_cpu_profile {
       bool present;
       bool user_set;
       int priv_spec;
+    int debug_spec;
       int satp_mode;
       const int32_t ext_offsets[];
   } RISCVCPUProfile;
@@ -123,6 +124,16 @@ enum {
       PRIV_VERSION_LATEST = PRIV_VERSION_1_13_0,
   };

+/* Debug specification version */
+#define DEBUG_VER_0_13_0_STR "v0.13"
+#define DEBUG_VER_1_00_0_STR "v1.0"
+enum {
+    DEBUG_VERSION_0_13_0 = 0,
+    DEBUG_VERSION_1_00_0,
+
+    DEBUG_VERSION_LATEST = DEBUG_VERSION_1_00_0, };
+
   #define VEXT_VERSION_1_00_0 0x00010000
   #define VEXT_VER_1_00_0_STR "v1.0"

@@ -245,6 +256,7 @@ struct CPUArchState {

       target_ulong priv_ver;
       target_ulong vext_ver;
+    target_ulong debug_ver;

       /* RISCVMXL, but uint32_t for vmstate migration */
       uint32_t misa_mxl;      /* current mxl */
@@ -563,6 +575,7 @@ typedef struct RISCVCPUDef {
       uint32_t misa_ext;
       int priv_spec;
       int32_t vext_spec;
+    int debug_spec;
       RISCVCPUConfig cfg;
       bool bare;
       const RISCVCSR *custom_csrs;
diff --git a/target/riscv/machine.c b/target/riscv/machine.c index
18d790a..8658f55 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -427,8 +427,8 @@ static const VMStateDescription vmstate_sstc = {

   const VMStateDescription vmstate_riscv_cpu = {
       .name = "cpu",
-    .version_id = 10,
-    .minimum_version_id = 10,
+    .version_id = 11,
+    .minimum_version_id = 11,
       .post_load = riscv_cpu_post_load,
       .fields = (const VMStateField[]) {
           VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32), @@ -443,6
+443,7 @@ const VMStateDescription vmstate_riscv_cpu = {
           VMSTATE_UINTTL(env.guest_phys_fault_addr, RISCVCPU),
           VMSTATE_UINTTL(env.priv_ver, RISCVCPU),
           VMSTATE_UINTTL(env.vext_ver, RISCVCPU),
+        VMSTATE_UINTTL(env.debug_ver, RISCVCPU),
           VMSTATE_UINT32(env.misa_mxl, RISCVCPU),
           VMSTATE_UINT32(env.misa_ext, RISCVCPU),
           VMSTATE_UNUSED(4),
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index d396825..160fcf1 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -1598,6 +1598,9 @@ static void riscv_init_max_cpu_extensions(Object
*obj)
       /* set vector version */
       env->vext_ver = VEXT_VERSION_1_00_0;

+    /* Set debug version */
+    env->debug_ver = DEBUG_VERSION_0_13_0;
+
       /* Zfinx is not compatible with F. Disable it */
       isa_ext_update_enabled(cpu, CPU_CFG_OFFSET(ext_zfinx), false);
       isa_ext_update_enabled(cpu, CPU_CFG_OFFSET(ext_zdinx), false);

CONFIDENTIALITY NOTICE:

This e-mail (and its attachments) may contain confidential and legally 
privileged information or information protected from disclosure. If you are not 
the intended recipient, you are hereby notified that any disclosure, copying, 
distribution, or use of the information contained herein is strictly 
prohibited. In this case, please immediately notify the sender by return 
e-mail, delete the message (and any accompanying documents) and destroy all 
printed hard copies. Thank you for your cooperation.

Copyright ANDES TECHNOLOGY CORPORATION - All Rights Reserved.


Reply via email to