[v1] target/riscv: Various fixes to gdbstub and CSR access

[PATCH 10/18] target/riscv: gdbstub: Turn on debugger mode before calling CSR predicate()

Posted by Bin Meng 2 years, 11 months ago

Since commit 94452ac4cf26 ("target/riscv: remove fflags, frm, and fcsr from riscv-*-fpu.xml")
the 3 FPU CSRs are removed from the XML target decription. The
original intent of that commit was based on the assumption that
the 3 FPU CSRs will show up in the riscv-csr.xml so the ones in
riscv-*-fpu.xml are redundant. But unforuantely that is not ture.
As the FPU CSR predicate() has a run-time check on MSTATUS.FS,
at the time when CSR XML is generated MSTATUS.FS is unset, hence
no FPU CSRs will be reported.

The FPU CSR predicate() already considered such a case of being
accessed by a debugger. All we need to do is to turn on debugger
mode before calling predicate().

Signed-off-by: Bin Meng <bmeng@tinylab.org>
---

 target/riscv/gdbstub.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 294f0ceb1c..ef52f41460 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -280,6 +280,10 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
     int bitsize = 16 << env->misa_mxl_max;
     int i;
 
+#if !defined(CONFIG_USER_ONLY)
+    env->debugger = true;
+#endif
+
     /* Until gdb knows about 128-bit registers */
     if (bitsize > 64) {
         bitsize = 64;
@@ -308,6 +312,11 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
     g_string_append_printf(s, "</feature>");
 
     cpu->dyn_csr_xml = g_string_free(s, false);
+
+#if !defined(CONFIG_USER_ONLY)
+    env->debugger = false;
+#endif
+
     return CSR_TABLE_SIZE;
 }
 
-- 
2.25.1

Re: [PATCH 10/18] target/riscv: gdbstub: Turn on debugger mode before calling CSR predicate()

Posted by LIU Zhiwei 2 years, 11 months ago

On 2023/2/14 2:02, Bin Meng wrote:
> Since commit 94452ac4cf26 ("target/riscv: remove fflags, frm, and fcsr from riscv-*-fpu.xml")
> the 3 FPU CSRs are removed from the XML target decription. The
> original intent of that commit was based on the assumption that
> the 3 FPU CSRs will show up in the riscv-csr.xml so the ones in
> riscv-*-fpu.xml are redundant. But unforuantely that is not ture.
> As the FPU CSR predicate() has a run-time check on MSTATUS.FS,
> at the time when CSR XML is generated MSTATUS.FS is unset, hence
> no FPU CSRs will be reported.
>
> The FPU CSR predicate() already considered such a case of being
> accessed by a debugger. All we need to do is to turn on debugger
> mode before calling predicate().
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
>   target/riscv/gdbstub.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 294f0ceb1c..ef52f41460 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -280,6 +280,10 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
>       int bitsize = 16 << env->misa_mxl_max;
>       int i;
>   
> +#if !defined(CONFIG_USER_ONLY)
> +    env->debugger = true;
> +#endif
> +
>       /* Until gdb knows about 128-bit registers */
>       if (bitsize > 64) {
>           bitsize = 64;
> @@ -308,6 +312,11 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
>       g_string_append_printf(s, "</feature>");
>   
>       cpu->dyn_csr_xml = g_string_free(s, false);
> +
> +#if !defined(CONFIG_USER_ONLY)
> +    env->debugger = false;
> +#endif
> +

Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>

Zhiwei

>       return CSR_TABLE_SIZE;
>   }
>

Re: [PATCH 10/18] target/riscv: gdbstub: Turn on debugger mode before calling CSR predicate()

Posted by weiwei 2 years, 11 months ago

On 2023/2/14 02:02, Bin Meng wrote:
> Since commit 94452ac4cf26 ("target/riscv: remove fflags, frm, and fcsr from riscv-*-fpu.xml")
> the 3 FPU CSRs are removed from the XML target decription. The
> original intent of that commit was based on the assumption that
> the 3 FPU CSRs will show up in the riscv-csr.xml so the ones in
> riscv-*-fpu.xml are redundant. But unforuantely that is not ture.

typo here -> true

otherwise, Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>

Regards,
Weiwei Li

> As the FPU CSR predicate() has a run-time check on MSTATUS.FS,
> at the time when CSR XML is generated MSTATUS.FS is unset, hence
> no FPU CSRs will be reported.
>
> The FPU CSR predicate() already considered such a case of being
> accessed by a debugger. All we need to do is to turn on debugger
> mode before calling predicate().
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
>   target/riscv/gdbstub.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 294f0ceb1c..ef52f41460 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -280,6 +280,10 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
>       int bitsize = 16 << env->misa_mxl_max;
>       int i;
>   
> +#if !defined(CONFIG_USER_ONLY)
> +    env->debugger = true;
> +#endif
> +
>       /* Until gdb knows about 128-bit registers */
>       if (bitsize > 64) {
>           bitsize = 64;
> @@ -308,6 +312,11 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
>       g_string_append_printf(s, "</feature>");
>   
>       cpu->dyn_csr_xml = g_string_free(s, false);
> +
> +#if !defined(CONFIG_USER_ONLY)
> +    env->debugger = false;
> +#endif
> +
>       return CSR_TABLE_SIZE;
>   }
>