| Message ID | 20220124074940.363064-1-frederic.petrot@univ-grenoble-alpes.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/riscv: correct "code should not be reached" for x-rv128 | expand |
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 <frederic.petrot@univ-grenoble-alpes.fr> > --- > 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);
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 <frederic.petrot@univ-grenoble-alpes.fr> >> --- >> 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);
On Mon, Jan 24, 2022 at 5:54 PM Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr> 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 <frederic.petrot@univ-grenoble-alpes.fr> 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 > >
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);
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 <frederic.petrot@univ-grenoble-alpes.fr> --- target/riscv/gdbstub.c | 3 +++ 1 file changed, 3 insertions(+)