Re: [PATCH] target/riscv: correct "code should not be reached" for x-rv128
On Mon, Jan 24, 2022 at 5:54 PM Frédéric Pétrot
wrote:
>
> The addition of uxl support in gdbstub adds a few checks on the maximum
> register length, but omitted MXL_RV128, leading to the occurence of
> "code should not be reached" in a few places.
> This patch makes rv128 react as rv64 for gdb, as previously.
>
> Signed-off-by: Frédéric Pétrot
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> target/riscv/gdbstub.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index f531a74c2f..9ed049c29e 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -64,6 +64,7 @@ int riscv_cpu_gdb_read_register(CPUState *cs, GByteArray
> *mem_buf, int n)
> case MXL_RV32:
> return gdb_get_reg32(mem_buf, tmp);
> case MXL_RV64:
> +case MXL_RV128:
> return gdb_get_reg64(mem_buf, tmp);
> default:
> g_assert_not_reached();
> @@ -84,6 +85,7 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t
> *mem_buf, int n)
> length = 4;
> break;
> case MXL_RV64:
> +case MXL_RV128:
> if (env->xl < MXL_RV64) {
> tmp = (int32_t)ldq_p(mem_buf);
> } else {
> @@ -420,6 +422,7 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState
> *cs)
> 1, "riscv-32bit-virtual.xml", 0);
> break;
> case MXL_RV64:
> +case MXL_RV128:
> gdb_register_coprocessor(cs, riscv_gdb_get_virtual,
> riscv_gdb_set_virtual,
> 1, "riscv-64bit-virtual.xml", 0);
> --
> 2.34.1
>
>
Re: [PATCH] target/riscv: correct "code should not be reached" for x-rv128
Le 24/01/2022 à 09:47, LIU Zhiwei a écrit :
On 2022/1/24 下午3:49, Frédéric Pétrot wrote:
The addition of uxl support in gdbstub adds a few checks on the maximum
register length, but omitted MXL_RV128, leading to the occurence of
"code should not be reached" in a few places.
This patch makes rv128 react as rv64 for gdb, as previously.
If that is case for rv128, you should also add
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1cb0436187..5ada71e5bf 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -528,9 +528,8 @@ static void riscv_cpu_realize(DeviceState *dev, Error
**errp)
switch (env->misa_mxl_max) {
#ifdef TARGET_RISCV64
case MXL_RV64:
- cc->gdb_core_xml_file = "riscv-64bit-cpu.xml";
- break;
case MXL_RV128:
+ cc->gdb_core_xml_file = "riscv-64bit-cpu.xml";
break;
Thanks Zhiwei for pointing that out, I resend a patch with that too.
Strangely enough, I didn't bump into that case.
Still I don't know why we should make rv128 react as rv64 for gdb?
Well, I should surely do what is necessary to have a working 128-bit
connection to gdb, but it has a bit of influence on other stuff than qemu,
e.g. when I configure the gdb-xml file with bitsize 128, gdb tests the
xlens and returns "bfd requires xlen 8, but target has xlen 16", and
after that no register can be queried.
Gdb checks what is called "arch features" and knows only about ELFCLASS32
and ELFCLASS64, so we are stuck for now.
Thanks,
Frédéric
Thanks,
Zhiwei
Signed-off-by: Frédéric Pétrot
---
target/riscv/gdbstub.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index f531a74c2f..9ed049c29e 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -64,6 +64,7 @@ int riscv_cpu_gdb_read_register(CPUState *cs, GByteArray
*mem_buf, int n)
case MXL_RV32:
return gdb_get_reg32(mem_buf, tmp);
case MXL_RV64:
+ case MXL_RV128:
return gdb_get_reg64(mem_buf, tmp);
default:
g_assert_not_reached();
@@ -84,6 +85,7 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t
*mem_buf, int n)
length = 4;
break;
case MXL_RV64:
+ case MXL_RV128:
if (env->xl < MXL_RV64) {
tmp = (int32_t)ldq_p(mem_buf);
} else {
@@ -420,6 +422,7 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
1, "riscv-32bit-virtual.xml", 0);
break;
case MXL_RV64:
+ case MXL_RV128:
gdb_register_coprocessor(cs, riscv_gdb_get_virtual,
riscv_gdb_set_virtual,
1, "riscv-64bit-virtual.xml", 0);
--
+---+
| Frédéric Pétrot, Pr. Grenoble INP-Ensimag/TIMA, Ensimag deputy director |
| Mob/Pho: +33 6 74 57 99 65/+33 4 76 57 48 70 Ad augusta per angusta |
| http://tima.univ-grenoble-alpes.fr frederic.pet...@univ-grenoble-alpes.fr |
+---+
Re: [PATCH] target/riscv: correct "code should not be reached" for x-rv128
On 2022/1/24 下午3:49, Frédéric Pétrot wrote:
The addition of uxl support in gdbstub adds a few checks on the maximum
register length, but omitted MXL_RV128, leading to the occurence of
"code should not be reached" in a few places.
This patch makes rv128 react as rv64 for gdb, as previously.
If that is case for rv128, you should also add
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1cb0436187..5ada71e5bf 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -528,9 +528,8 @@ static void riscv_cpu_realize(DeviceState *dev,
Error **errp)
switch (env->misa_mxl_max) {
#ifdef TARGET_RISCV64
case MXL_RV64:
- cc->gdb_core_xml_file = "riscv-64bit-cpu.xml";
- break;
case MXL_RV128:
+ cc->gdb_core_xml_file = "riscv-64bit-cpu.xml";
break;
Still I don't know why we should make rv128 react as rv64 for gdb?
Thanks,
Zhiwei
Signed-off-by: Frédéric Pétrot
---
target/riscv/gdbstub.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index f531a74c2f..9ed049c29e 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -64,6 +64,7 @@ int riscv_cpu_gdb_read_register(CPUState *cs, GByteArray
*mem_buf, int n)
case MXL_RV32:
return gdb_get_reg32(mem_buf, tmp);
case MXL_RV64:
+case MXL_RV128:
return gdb_get_reg64(mem_buf, tmp);
default:
g_assert_not_reached();
@@ -84,6 +85,7 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t
*mem_buf, int n)
length = 4;
break;
case MXL_RV64:
+case MXL_RV128:
if (env->xl < MXL_RV64) {
tmp = (int32_t)ldq_p(mem_buf);
} else {
@@ -420,6 +422,7 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
1, "riscv-32bit-virtual.xml", 0);
break;
case MXL_RV64:
+case MXL_RV128:
gdb_register_coprocessor(cs, riscv_gdb_get_virtual,
riscv_gdb_set_virtual,
1, "riscv-64bit-virtual.xml", 0);
[PATCH] target/riscv: correct "code should not be reached" for x-rv128
The addition of uxl support in gdbstub adds a few checks on the maximum
register length, but omitted MXL_RV128, leading to the occurence of
"code should not be reached" in a few places.
This patch makes rv128 react as rv64 for gdb, as previously.
Signed-off-by: Frédéric Pétrot
---
target/riscv/gdbstub.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index f531a74c2f..9ed049c29e 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -64,6 +64,7 @@ int riscv_cpu_gdb_read_register(CPUState *cs, GByteArray
*mem_buf, int n)
case MXL_RV32:
return gdb_get_reg32(mem_buf, tmp);
case MXL_RV64:
+case MXL_RV128:
return gdb_get_reg64(mem_buf, tmp);
default:
g_assert_not_reached();
@@ -84,6 +85,7 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t
*mem_buf, int n)
length = 4;
break;
case MXL_RV64:
+case MXL_RV128:
if (env->xl < MXL_RV64) {
tmp = (int32_t)ldq_p(mem_buf);
} else {
@@ -420,6 +422,7 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
1, "riscv-32bit-virtual.xml", 0);
break;
case MXL_RV64:
+case MXL_RV128:
gdb_register_coprocessor(cs, riscv_gdb_get_virtual,
riscv_gdb_set_virtual,
1, "riscv-64bit-virtual.xml", 0);
--
2.34.1
