1. Patchew
  2. QEMU
  3. View series

[PATCH v4] riscv/gdbstub: add V bit to priv reg

Yanfeng Liu posted 1 patch 1 year, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/tencent._5F1993B55C24DE7979BF34B200F78287002907@qq.com
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
target/riscv/gdbstub.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
[PATCH v4] riscv/gdbstub: add V bit to priv reg
Posted by Yanfeng Liu 1 year, 1 month ago 
This adds virtualization mode (V bit) as bit(2) of register `priv`
per RiscV debug spec v1.0.0-rc4. Checked with gdb-multiarch v12.1.

Note that GDB may display `INVALID` tag for `priv` reg when V bit
is set, this doesn't affect actual access to the bit though.

Signed-off-by: Yanfeng Liu <yfliu2008@qq.com>
---
 target/riscv/gdbstub.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index c07df972f1..18e88f416a 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -213,7 +213,10 @@ static int riscv_gdb_get_virtual(CPUState *cs, GByteArray *buf, int n)
         RISCVCPU *cpu = RISCV_CPU(cs);
         CPURISCVState *env = &cpu->env;
 
-        return gdb_get_regl(buf, env->priv);
+        /* Per RiscV debug spec v1.0.0 rc4 */
+        target_ulong vbit = (env->virt_enabled) ? BIT(2) : 0;
+
+        return gdb_get_regl(buf, env->priv | vbit);
 #endif
     }
     return 0;
@@ -226,10 +229,22 @@ static int riscv_gdb_set_virtual(CPUState *cs, uint8_t *mem_buf, int n)
         RISCVCPU *cpu = RISCV_CPU(cs);
         CPURISCVState *env = &cpu->env;
 
-        env->priv = ldtul_p(mem_buf) & 0x3;
-        if (env->priv == PRV_RESERVED) {
-            env->priv = PRV_S;
+        target_ulong new_priv = ldtul_p(mem_buf) & 0x3;
+        bool new_virt = 0;
+
+        if (new_priv == PRV_RESERVED) {
+            new_priv = PRV_S;
+        }
+
+        if (new_priv != PRV_M) {
+            new_virt = (ldtul_p(mem_buf) & BIT(2)) >> 2;
         }
+
+        if (riscv_has_ext(env, RVH) && new_virt != env->virt_enabled) {
+            riscv_cpu_swap_hypervisor_regs(env);
+        }
+
+        riscv_cpu_set_mode(env, new_priv, new_virt);
 #endif
         return sizeof(target_ulong);
     }
-- 
2.34.1
Re: [PATCH v4] riscv/gdbstub: add V bit to priv reg
Posted by Alistair Francis 1 year, 1 month ago 
On Mon, Dec 16, 2024 at 7:38 AM Yanfeng Liu <yfliu2008@qq.com> wrote:
>
> This adds virtualization mode (V bit) as bit(2) of register `priv`
> per RiscV debug spec v1.0.0-rc4. Checked with gdb-multiarch v12.1.
>
> Note that GDB may display `INVALID` tag for `priv` reg when V bit
> is set, this doesn't affect actual access to the bit though.
>
> Signed-off-by: Yanfeng Liu <yfliu2008@qq.com>

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

Alistair

> ---
>  target/riscv/gdbstub.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index c07df972f1..18e88f416a 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -213,7 +213,10 @@ static int riscv_gdb_get_virtual(CPUState *cs, GByteArray *buf, int n)
>          RISCVCPU *cpu = RISCV_CPU(cs);
>          CPURISCVState *env = &cpu->env;
>
> -        return gdb_get_regl(buf, env->priv);
> +        /* Per RiscV debug spec v1.0.0 rc4 */
> +        target_ulong vbit = (env->virt_enabled) ? BIT(2) : 0;
> +
> +        return gdb_get_regl(buf, env->priv | vbit);
>  #endif
>      }
>      return 0;
> @@ -226,10 +229,22 @@ static int riscv_gdb_set_virtual(CPUState *cs, uint8_t *mem_buf, int n)
>          RISCVCPU *cpu = RISCV_CPU(cs);
>          CPURISCVState *env = &cpu->env;
>
> -        env->priv = ldtul_p(mem_buf) & 0x3;
> -        if (env->priv == PRV_RESERVED) {
> -            env->priv = PRV_S;
> +        target_ulong new_priv = ldtul_p(mem_buf) & 0x3;
> +        bool new_virt = 0;
> +
> +        if (new_priv == PRV_RESERVED) {
> +            new_priv = PRV_S;
> +        }
> +
> +        if (new_priv != PRV_M) {
> +            new_virt = (ldtul_p(mem_buf) & BIT(2)) >> 2;
>          }
> +
> +        if (riscv_has_ext(env, RVH) && new_virt != env->virt_enabled) {
> +            riscv_cpu_swap_hypervisor_regs(env);
> +        }
> +
> +        riscv_cpu_set_mode(env, new_priv, new_virt);
>  #endif
>          return sizeof(target_ulong);
>      }
> --
> 2.34.1
>
>
Re: [PATCH v4] riscv/gdbstub: add V bit to priv reg
Posted by Alistair Francis 1 year, 1 month ago 
On Mon, Dec 16, 2024 at 7:38 AM Yanfeng Liu <yfliu2008@qq.com> wrote:
>
> This adds virtualization mode (V bit) as bit(2) of register `priv`
> per RiscV debug spec v1.0.0-rc4. Checked with gdb-multiarch v12.1.
>
> Note that GDB may display `INVALID` tag for `priv` reg when V bit
> is set, this doesn't affect actual access to the bit though.
>
> Signed-off-by: Yanfeng Liu <yfliu2008@qq.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/gdbstub.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index c07df972f1..18e88f416a 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -213,7 +213,10 @@ static int riscv_gdb_get_virtual(CPUState *cs, GByteArray *buf, int n)
>          RISCVCPU *cpu = RISCV_CPU(cs);
>          CPURISCVState *env = &cpu->env;
>
> -        return gdb_get_regl(buf, env->priv);
> +        /* Per RiscV debug spec v1.0.0 rc4 */
> +        target_ulong vbit = (env->virt_enabled) ? BIT(2) : 0;
> +
> +        return gdb_get_regl(buf, env->priv | vbit);
>  #endif
>      }
>      return 0;
> @@ -226,10 +229,22 @@ static int riscv_gdb_set_virtual(CPUState *cs, uint8_t *mem_buf, int n)
>          RISCVCPU *cpu = RISCV_CPU(cs);
>          CPURISCVState *env = &cpu->env;
>
> -        env->priv = ldtul_p(mem_buf) & 0x3;
> -        if (env->priv == PRV_RESERVED) {
> -            env->priv = PRV_S;
> +        target_ulong new_priv = ldtul_p(mem_buf) & 0x3;
> +        bool new_virt = 0;
> +
> +        if (new_priv == PRV_RESERVED) {
> +            new_priv = PRV_S;
> +        }
> +
> +        if (new_priv != PRV_M) {
> +            new_virt = (ldtul_p(mem_buf) & BIT(2)) >> 2;
>          }
> +
> +        if (riscv_has_ext(env, RVH) && new_virt != env->virt_enabled) {
> +            riscv_cpu_swap_hypervisor_regs(env);
> +        }
> +
> +        riscv_cpu_set_mode(env, new_priv, new_virt);
>  #endif
>          return sizeof(target_ulong);
>      }
> --
> 2.34.1
>
>
Re: [PATCH v4] riscv/gdbstub: add V bit to priv reg
Posted by Mario Fleischmann 1 year, 1 month ago 

On 15.12.2024 22:37, Yanfeng Liu wrote:
> This adds virtualization mode (V bit) as bit(2) of register `priv`
> per RiscV debug spec v1.0.0-rc4. Checked with gdb-multiarch v12.1.
> 
> Note that GDB may display `INVALID` tag for `priv` reg when V bit
> is set, this doesn't affect actual access to the bit though.
> 
> Signed-off-by: Yanfeng Liu <yfliu2008@qq.com>
> ---
>   target/riscv/gdbstub.c | 23 +++++++++++++++++++----
>   1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index c07df972f1..18e88f416a 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -213,7 +213,10 @@ static int riscv_gdb_get_virtual(CPUState *cs, GByteArray *buf, int n)
>           RISCVCPU *cpu = RISCV_CPU(cs);
>           CPURISCVState *env = &cpu->env;
>   
> -        return gdb_get_regl(buf, env->priv);
> +        /* Per RiscV debug spec v1.0.0 rc4 */
> +        target_ulong vbit = (env->virt_enabled) ? BIT(2) : 0;
> +
> +        return gdb_get_regl(buf, env->priv | vbit);
>   #endif
>       }
>       return 0;
> @@ -226,10 +229,22 @@ static int riscv_gdb_set_virtual(CPUState *cs, uint8_t *mem_buf, int n)
>           RISCVCPU *cpu = RISCV_CPU(cs);
>           CPURISCVState *env = &cpu->env;
>   
> -        env->priv = ldtul_p(mem_buf) & 0x3;
> -        if (env->priv == PRV_RESERVED) {
> -            env->priv = PRV_S;
> +        target_ulong new_priv = ldtul_p(mem_buf) & 0x3;
> +        bool new_virt = 0;
> +
> +        if (new_priv == PRV_RESERVED) {
> +            new_priv = PRV_S;
> +        }
> +
> +        if (new_priv != PRV_M) {
> +            new_virt = (ldtul_p(mem_buf) & BIT(2)) >> 2;
>           }
> +
> +        if (riscv_has_ext(env, RVH) && new_virt != env->virt_enabled) {
> +            riscv_cpu_swap_hypervisor_regs(env);
> +        }
> +
> +        riscv_cpu_set_mode(env, new_priv, new_virt);
>   #endif
>           return sizeof(target_ulong);
>       }

Thanks for the submission, it works as expected:

(gdb) info register $priv
priv           0x4      prv:4 [INVALID]
(gdb) info register $sstatus
sstatus        0x200004022      8589951010
(gdb) set $priv = 0x0
(gdb) info register $priv
priv           0x0      prv:0 [User/Application]
(gdb) info register $sstatus
sstatus        0x8000000200006022       -9223372028264816606
(gdb) set $priv = 0x4
(gdb) info register $priv
priv           0x4      prv:4 [INVALID]
(gdb) info register $sstatus
sstatus        0x200004022      8589951010

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.