* [PATCH] target/loongarch: Split fcc register to fcc0-7 in gdbstub @ 2023-08-08 5:42 Jiajie Chen 2023-08-08 6:10 ` bibo mao 0 siblings, 1 reply; 8+ messages in thread From: Jiajie Chen @ 2023-08-08 5:42 UTC (permalink / raw) To: qemu-devel Cc: Jiajie Chen, Xiaojuan Yang, Song Gao, Alex Bennée, Philippe Mathieu-Daudé Since GDB 13.1(GDB commit ea3352172), GDB LoongArch changed to use fcc0-7 instead of fcc register. This commit partially reverts commit 2f149c759 (`target/loongarch: Update gdb_set_fpu() and gdb_get_fpu()`) to match the behavior of GDB. Note that it is a breaking change for GDB 13.0 or earlier, but it is also required for GDB 13.1 or later to work. Signed-off-by: Jiajie Chen <c@jia.je> --- gdb-xml/loongarch-fpu.xml | 9 ++++++++- target/loongarch/gdbstub.c | 16 +++++++--------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/gdb-xml/loongarch-fpu.xml b/gdb-xml/loongarch-fpu.xml index 78e42cf5dd..e81e3382e7 100644 --- a/gdb-xml/loongarch-fpu.xml +++ b/gdb-xml/loongarch-fpu.xml @@ -45,6 +45,13 @@ <reg name="f29" bitsize="64" type="fputype" group="float"/> <reg name="f30" bitsize="64" type="fputype" group="float"/> <reg name="f31" bitsize="64" type="fputype" group="float"/> - <reg name="fcc" bitsize="64" type="uint64" group="float"/> + <reg name="fcc0" bitsize="8" type="uint8" group="float"/> + <reg name="fcc1" bitsize="8" type="uint8" group="float"/> + <reg name="fcc2" bitsize="8" type="uint8" group="float"/> + <reg name="fcc3" bitsize="8" type="uint8" group="float"/> + <reg name="fcc4" bitsize="8" type="uint8" group="float"/> + <reg name="fcc5" bitsize="8" type="uint8" group="float"/> + <reg name="fcc6" bitsize="8" type="uint8" group="float"/> + <reg name="fcc7" bitsize="8" type="uint8" group="float"/> <reg name="fcsr" bitsize="32" type="uint32" group="float"/> </feature> diff --git a/target/loongarch/gdbstub.c b/target/loongarch/gdbstub.c index 0752fff924..15ad6778f1 100644 --- a/target/loongarch/gdbstub.c +++ b/target/loongarch/gdbstub.c @@ -70,10 +70,9 @@ static int loongarch_gdb_get_fpu(CPULoongArchState *env, { if (0 <= n && n < 32) { return gdb_get_reg64(mem_buf, env->fpr[n].vreg.D(0)); - } else if (n == 32) { - uint64_t val = read_fcc(env); - return gdb_get_reg64(mem_buf, val); - } else if (n == 33) { + } else if (32 <= n && n < 40) { + return gdb_get_reg8(mem_buf, env->cf[n - 32]); + } else if (n == 40) { return gdb_get_reg32(mem_buf, env->fcsr0); } return 0; @@ -87,11 +86,10 @@ static int loongarch_gdb_set_fpu(CPULoongArchState *env, if (0 <= n && n < 32) { env->fpr[n].vreg.D(0) = ldq_p(mem_buf); length = 8; - } else if (n == 32) { - uint64_t val = ldq_p(mem_buf); - write_fcc(env, val); - length = 8; - } else if (n == 33) { + } else if (32 <= n && n < 40) { + env->cf[n - 32] = ldub_p(mem_buf); + length = 1; + } else if (n == 40) { env->fcsr0 = ldl_p(mem_buf); length = 4; } -- 2.41.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] target/loongarch: Split fcc register to fcc0-7 in gdbstub 2023-08-08 5:42 [PATCH] target/loongarch: Split fcc register to fcc0-7 in gdbstub Jiajie Chen @ 2023-08-08 6:10 ` bibo mao 2023-08-08 6:28 ` bibo mao 2023-08-08 9:55 ` Jiajie Chen 0 siblings, 2 replies; 8+ messages in thread From: bibo mao @ 2023-08-08 6:10 UTC (permalink / raw) To: Jiajie Chen, qemu-devel Cc: Song Gao, Alex Bennée, Philippe Mathieu-Daudé I am not familiar with gdb, is there abi breakage? I do not know how gdb client works with gdb server with different versions. Regards Bibo Mao 在 2023/8/8 13:42, Jiajie Chen 写道: > Since GDB 13.1(GDB commit ea3352172), GDB LoongArch changed to use > fcc0-7 instead of fcc register. This commit partially reverts commit > 2f149c759 (`target/loongarch: Update gdb_set_fpu() and gdb_get_fpu()`) > to match the behavior of GDB. > > Note that it is a breaking change for GDB 13.0 or earlier, but it is > also required for GDB 13.1 or later to work. > > Signed-off-by: Jiajie Chen <c@jia.je> > --- > gdb-xml/loongarch-fpu.xml | 9 ++++++++- > target/loongarch/gdbstub.c | 16 +++++++--------- > 2 files changed, 15 insertions(+), 10 deletions(-) > > diff --git a/gdb-xml/loongarch-fpu.xml b/gdb-xml/loongarch-fpu.xml > index 78e42cf5dd..e81e3382e7 100644 > --- a/gdb-xml/loongarch-fpu.xml > +++ b/gdb-xml/loongarch-fpu.xml > @@ -45,6 +45,13 @@ > <reg name="f29" bitsize="64" type="fputype" group="float"/> > <reg name="f30" bitsize="64" type="fputype" group="float"/> > <reg name="f31" bitsize="64" type="fputype" group="float"/> > - <reg name="fcc" bitsize="64" type="uint64" group="float"/> > + <reg name="fcc0" bitsize="8" type="uint8" group="float"/> > + <reg name="fcc1" bitsize="8" type="uint8" group="float"/> > + <reg name="fcc2" bitsize="8" type="uint8" group="float"/> > + <reg name="fcc3" bitsize="8" type="uint8" group="float"/> > + <reg name="fcc4" bitsize="8" type="uint8" group="float"/> > + <reg name="fcc5" bitsize="8" type="uint8" group="float"/> > + <reg name="fcc6" bitsize="8" type="uint8" group="float"/> > + <reg name="fcc7" bitsize="8" type="uint8" group="float"/> > <reg name="fcsr" bitsize="32" type="uint32" group="float"/> > </feature> > diff --git a/target/loongarch/gdbstub.c b/target/loongarch/gdbstub.c > index 0752fff924..15ad6778f1 100644 > --- a/target/loongarch/gdbstub.c > +++ b/target/loongarch/gdbstub.c > @@ -70,10 +70,9 @@ static int loongarch_gdb_get_fpu(CPULoongArchState *env, > { > if (0 <= n && n < 32) { > return gdb_get_reg64(mem_buf, env->fpr[n].vreg.D(0)); > - } else if (n == 32) { > - uint64_t val = read_fcc(env); > - return gdb_get_reg64(mem_buf, val); > - } else if (n == 33) { > + } else if (32 <= n && n < 40) { > + return gdb_get_reg8(mem_buf, env->cf[n - 32]); > + } else if (n == 40) { > return gdb_get_reg32(mem_buf, env->fcsr0); > } > return 0; > @@ -87,11 +86,10 @@ static int loongarch_gdb_set_fpu(CPULoongArchState *env, > if (0 <= n && n < 32) { > env->fpr[n].vreg.D(0) = ldq_p(mem_buf); > length = 8; > - } else if (n == 32) { > - uint64_t val = ldq_p(mem_buf); > - write_fcc(env, val); > - length = 8; > - } else if (n == 33) { > + } else if (32 <= n && n < 40) { > + env->cf[n - 32] = ldub_p(mem_buf); > + length = 1; > + } else if (n == 40) { > env->fcsr0 = ldl_p(mem_buf); > length = 4; > } ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] target/loongarch: Split fcc register to fcc0-7 in gdbstub 2023-08-08 6:10 ` bibo mao @ 2023-08-08 6:28 ` bibo mao 2023-08-08 9:55 ` Jiajie Chen 1 sibling, 0 replies; 8+ messages in thread From: bibo mao @ 2023-08-08 6:28 UTC (permalink / raw) To: Jiajie Chen, Tiezhu Yang Cc: Song Gao, Alex Bennée, Philippe Mathieu-Daudé, qemu-devel add loongarch gdb maintainer. 在 2023/8/8 14:10, bibo mao 写道: > > I am not familiar with gdb, is there abi breakage? > I do not know how gdb client works with gdb server with different versions. > > Regards > Bibo Mao > > > 在 2023/8/8 13:42, Jiajie Chen 写道: >> Since GDB 13.1(GDB commit ea3352172), GDB LoongArch changed to use >> fcc0-7 instead of fcc register. This commit partially reverts commit >> 2f149c759 (`target/loongarch: Update gdb_set_fpu() and gdb_get_fpu()`) >> to match the behavior of GDB. >> >> Note that it is a breaking change for GDB 13.0 or earlier, but it is >> also required for GDB 13.1 or later to work. >> >> Signed-off-by: Jiajie Chen <c@jia.je> >> --- >> gdb-xml/loongarch-fpu.xml | 9 ++++++++- >> target/loongarch/gdbstub.c | 16 +++++++--------- >> 2 files changed, 15 insertions(+), 10 deletions(-) >> >> diff --git a/gdb-xml/loongarch-fpu.xml b/gdb-xml/loongarch-fpu.xml >> index 78e42cf5dd..e81e3382e7 100644 >> --- a/gdb-xml/loongarch-fpu.xml >> +++ b/gdb-xml/loongarch-fpu.xml >> @@ -45,6 +45,13 @@ >> <reg name="f29" bitsize="64" type="fputype" group="float"/> >> <reg name="f30" bitsize="64" type="fputype" group="float"/> >> <reg name="f31" bitsize="64" type="fputype" group="float"/> >> - <reg name="fcc" bitsize="64" type="uint64" group="float"/> >> + <reg name="fcc0" bitsize="8" type="uint8" group="float"/> >> + <reg name="fcc1" bitsize="8" type="uint8" group="float"/> >> + <reg name="fcc2" bitsize="8" type="uint8" group="float"/> >> + <reg name="fcc3" bitsize="8" type="uint8" group="float"/> >> + <reg name="fcc4" bitsize="8" type="uint8" group="float"/> >> + <reg name="fcc5" bitsize="8" type="uint8" group="float"/> >> + <reg name="fcc6" bitsize="8" type="uint8" group="float"/> >> + <reg name="fcc7" bitsize="8" type="uint8" group="float"/> >> <reg name="fcsr" bitsize="32" type="uint32" group="float"/> >> </feature> >> diff --git a/target/loongarch/gdbstub.c b/target/loongarch/gdbstub.c >> index 0752fff924..15ad6778f1 100644 >> --- a/target/loongarch/gdbstub.c >> +++ b/target/loongarch/gdbstub.c >> @@ -70,10 +70,9 @@ static int loongarch_gdb_get_fpu(CPULoongArchState *env, >> { >> if (0 <= n && n < 32) { >> return gdb_get_reg64(mem_buf, env->fpr[n].vreg.D(0)); >> - } else if (n == 32) { >> - uint64_t val = read_fcc(env); >> - return gdb_get_reg64(mem_buf, val); >> - } else if (n == 33) { >> + } else if (32 <= n && n < 40) { >> + return gdb_get_reg8(mem_buf, env->cf[n - 32]); >> + } else if (n == 40) { >> return gdb_get_reg32(mem_buf, env->fcsr0); >> } >> return 0; >> @@ -87,11 +86,10 @@ static int loongarch_gdb_set_fpu(CPULoongArchState *env, >> if (0 <= n && n < 32) { >> env->fpr[n].vreg.D(0) = ldq_p(mem_buf); >> length = 8; >> - } else if (n == 32) { >> - uint64_t val = ldq_p(mem_buf); >> - write_fcc(env, val); >> - length = 8; >> - } else if (n == 33) { >> + } else if (32 <= n && n < 40) { >> + env->cf[n - 32] = ldub_p(mem_buf); >> + length = 1; >> + } else if (n == 40) { >> env->fcsr0 = ldl_p(mem_buf); >> length = 4; >> } > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] target/loongarch: Split fcc register to fcc0-7 in gdbstub 2023-08-08 6:10 ` bibo mao 2023-08-08 6:28 ` bibo mao @ 2023-08-08 9:55 ` Jiajie Chen 2023-08-08 10:03 ` Jiajie Chen 1 sibling, 1 reply; 8+ messages in thread From: Jiajie Chen @ 2023-08-08 9:55 UTC (permalink / raw) To: bibo mao, qemu-devel Cc: Song Gao, Alex Bennée, Philippe Mathieu-Daudé On 2023/8/8 14:10, bibo mao wrote: > I am not familiar with gdb, is there abi breakage? > I do not know how gdb client works with gdb server with different versions. Not abi breakage, but gdb will complain: warning: while parsing target description (at line 1): Target description specified unknown architecture "loongarch64" warning: Could not load XML target description; ignoring warning: No executable has been specified and target does not support determining executable automatically. Try using the "file" command. Truncated register 38 in remote 'g' packet And gdb can no longer debug kernel running in qemu. You can reproduce this error using latest qemu(without this patch) and gdb(13.1 or later). > > Regards > Bibo Mao > > > 在 2023/8/8 13:42, Jiajie Chen 写道: >> Since GDB 13.1(GDB commit ea3352172), GDB LoongArch changed to use >> fcc0-7 instead of fcc register. This commit partially reverts commit >> 2f149c759 (`target/loongarch: Update gdb_set_fpu() and gdb_get_fpu()`) >> to match the behavior of GDB. >> >> Note that it is a breaking change for GDB 13.0 or earlier, but it is >> also required for GDB 13.1 or later to work. >> >> Signed-off-by: Jiajie Chen <c@jia.je> >> --- >> gdb-xml/loongarch-fpu.xml | 9 ++++++++- >> target/loongarch/gdbstub.c | 16 +++++++--------- >> 2 files changed, 15 insertions(+), 10 deletions(-) >> >> diff --git a/gdb-xml/loongarch-fpu.xml b/gdb-xml/loongarch-fpu.xml >> index 78e42cf5dd..e81e3382e7 100644 >> --- a/gdb-xml/loongarch-fpu.xml >> +++ b/gdb-xml/loongarch-fpu.xml >> @@ -45,6 +45,13 @@ >> <reg name="f29" bitsize="64" type="fputype" group="float"/> >> <reg name="f30" bitsize="64" type="fputype" group="float"/> >> <reg name="f31" bitsize="64" type="fputype" group="float"/> >> - <reg name="fcc" bitsize="64" type="uint64" group="float"/> >> + <reg name="fcc0" bitsize="8" type="uint8" group="float"/> >> + <reg name="fcc1" bitsize="8" type="uint8" group="float"/> >> + <reg name="fcc2" bitsize="8" type="uint8" group="float"/> >> + <reg name="fcc3" bitsize="8" type="uint8" group="float"/> >> + <reg name="fcc4" bitsize="8" type="uint8" group="float"/> >> + <reg name="fcc5" bitsize="8" type="uint8" group="float"/> >> + <reg name="fcc6" bitsize="8" type="uint8" group="float"/> >> + <reg name="fcc7" bitsize="8" type="uint8" group="float"/> >> <reg name="fcsr" bitsize="32" type="uint32" group="float"/> >> </feature> >> diff --git a/target/loongarch/gdbstub.c b/target/loongarch/gdbstub.c >> index 0752fff924..15ad6778f1 100644 >> --- a/target/loongarch/gdbstub.c >> +++ b/target/loongarch/gdbstub.c >> @@ -70,10 +70,9 @@ static int loongarch_gdb_get_fpu(CPULoongArchState *env, >> { >> if (0 <= n && n < 32) { >> return gdb_get_reg64(mem_buf, env->fpr[n].vreg.D(0)); >> - } else if (n == 32) { >> - uint64_t val = read_fcc(env); >> - return gdb_get_reg64(mem_buf, val); >> - } else if (n == 33) { >> + } else if (32 <= n && n < 40) { >> + return gdb_get_reg8(mem_buf, env->cf[n - 32]); >> + } else if (n == 40) { >> return gdb_get_reg32(mem_buf, env->fcsr0); >> } >> return 0; >> @@ -87,11 +86,10 @@ static int loongarch_gdb_set_fpu(CPULoongArchState *env, >> if (0 <= n && n < 32) { >> env->fpr[n].vreg.D(0) = ldq_p(mem_buf); >> length = 8; >> - } else if (n == 32) { >> - uint64_t val = ldq_p(mem_buf); >> - write_fcc(env, val); >> - length = 8; >> - } else if (n == 33) { >> + } else if (32 <= n && n < 40) { >> + env->cf[n - 32] = ldub_p(mem_buf); >> + length = 1; >> + } else if (n == 40) { >> env->fcsr0 = ldl_p(mem_buf); >> length = 4; >> } ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] target/loongarch: Split fcc register to fcc0-7 in gdbstub 2023-08-08 9:55 ` Jiajie Chen @ 2023-08-08 10:03 ` Jiajie Chen 2023-08-08 11:42 ` bibo mao 0 siblings, 1 reply; 8+ messages in thread From: Jiajie Chen @ 2023-08-08 10:03 UTC (permalink / raw) To: bibo mao, qemu-devel Cc: Song Gao, Alex Bennée, Philippe Mathieu-Daudé On 2023/8/8 17:55, Jiajie Chen wrote: > > On 2023/8/8 14:10, bibo mao wrote: >> I am not familiar with gdb, is there abi breakage? >> I do not know how gdb client works with gdb server with different >> versions. There seemed no versioning in the process, but rather in-code xml validation. In gdb, the code only allows new xml (fcc0-7) and rejects old one (fcc), so gdb breaks qemu first and do not consider backward compatibility with qemu. > > Not abi breakage, but gdb will complain: > > warning: while parsing target description (at line 1): Target > description specified unknown architecture "loongarch64" > warning: Could not load XML target description; ignoring > warning: No executable has been specified and target does not support > determining executable automatically. Try using the "file" command. > Truncated register 38 in remote 'g' packet Sorry, to be clear, the actual error message is: (gdb) target extended-remote localhost:1234 Remote debugging using localhost:1234 warning: Architecture rejected target-supplied description warning: No executable has been specified and target does not support It rejects the target description xml given by qemu, thus using the builtin one. However, there is a mismatch in fcc registers, so it will not work if we list floating point registers. At the same time, if we are using loongarch32 target(I recently posted patches to support this), it will reject the target description and fallback to loongarch64, making gcc not usable. > > And gdb can no longer debug kernel running in qemu. You can reproduce > this error using latest qemu(without this patch) and gdb(13.1 or later). > >> >> Regards >> Bibo Mao >> >> >> 在 2023/8/8 13:42, Jiajie Chen 写道: >>> Since GDB 13.1(GDB commit ea3352172), GDB LoongArch changed to use >>> fcc0-7 instead of fcc register. This commit partially reverts commit >>> 2f149c759 (`target/loongarch: Update gdb_set_fpu() and gdb_get_fpu()`) >>> to match the behavior of GDB. >>> >>> Note that it is a breaking change for GDB 13.0 or earlier, but it is >>> also required for GDB 13.1 or later to work. >>> >>> Signed-off-by: Jiajie Chen <c@jia.je> >>> --- >>> gdb-xml/loongarch-fpu.xml | 9 ++++++++- >>> target/loongarch/gdbstub.c | 16 +++++++--------- >>> 2 files changed, 15 insertions(+), 10 deletions(-) >>> >>> diff --git a/gdb-xml/loongarch-fpu.xml b/gdb-xml/loongarch-fpu.xml >>> index 78e42cf5dd..e81e3382e7 100644 >>> --- a/gdb-xml/loongarch-fpu.xml >>> +++ b/gdb-xml/loongarch-fpu.xml >>> @@ -45,6 +45,13 @@ >>> <reg name="f29" bitsize="64" type="fputype" group="float"/> >>> <reg name="f30" bitsize="64" type="fputype" group="float"/> >>> <reg name="f31" bitsize="64" type="fputype" group="float"/> >>> - <reg name="fcc" bitsize="64" type="uint64" group="float"/> >>> + <reg name="fcc0" bitsize="8" type="uint8" group="float"/> >>> + <reg name="fcc1" bitsize="8" type="uint8" group="float"/> >>> + <reg name="fcc2" bitsize="8" type="uint8" group="float"/> >>> + <reg name="fcc3" bitsize="8" type="uint8" group="float"/> >>> + <reg name="fcc4" bitsize="8" type="uint8" group="float"/> >>> + <reg name="fcc5" bitsize="8" type="uint8" group="float"/> >>> + <reg name="fcc6" bitsize="8" type="uint8" group="float"/> >>> + <reg name="fcc7" bitsize="8" type="uint8" group="float"/> >>> <reg name="fcsr" bitsize="32" type="uint32" group="float"/> >>> </feature> >>> diff --git a/target/loongarch/gdbstub.c b/target/loongarch/gdbstub.c >>> index 0752fff924..15ad6778f1 100644 >>> --- a/target/loongarch/gdbstub.c >>> +++ b/target/loongarch/gdbstub.c >>> @@ -70,10 +70,9 @@ static int >>> loongarch_gdb_get_fpu(CPULoongArchState *env, >>> { >>> if (0 <= n && n < 32) { >>> return gdb_get_reg64(mem_buf, env->fpr[n].vreg.D(0)); >>> - } else if (n == 32) { >>> - uint64_t val = read_fcc(env); >>> - return gdb_get_reg64(mem_buf, val); >>> - } else if (n == 33) { >>> + } else if (32 <= n && n < 40) { >>> + return gdb_get_reg8(mem_buf, env->cf[n - 32]); >>> + } else if (n == 40) { >>> return gdb_get_reg32(mem_buf, env->fcsr0); >>> } >>> return 0; >>> @@ -87,11 +86,10 @@ static int >>> loongarch_gdb_set_fpu(CPULoongArchState *env, >>> if (0 <= n && n < 32) { >>> env->fpr[n].vreg.D(0) = ldq_p(mem_buf); >>> length = 8; >>> - } else if (n == 32) { >>> - uint64_t val = ldq_p(mem_buf); >>> - write_fcc(env, val); >>> - length = 8; >>> - } else if (n == 33) { >>> + } else if (32 <= n && n < 40) { >>> + env->cf[n - 32] = ldub_p(mem_buf); >>> + length = 1; >>> + } else if (n == 40) { >>> env->fcsr0 = ldl_p(mem_buf); >>> length = 4; >>> } ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] target/loongarch: Split fcc register to fcc0-7 in gdbstub 2023-08-08 10:03 ` Jiajie Chen @ 2023-08-08 11:42 ` bibo mao 2023-08-08 18:40 ` Alex Bennée 0 siblings, 1 reply; 8+ messages in thread From: bibo mao @ 2023-08-08 11:42 UTC (permalink / raw) To: Jiajie Chen, Tiezhu Yang Cc: Song Gao, Alex Bennée, Philippe Mathieu-Daudé, qemu-devel I think that it is problem of loongarch gdb, rather qemu. If so, everytime when gdb changes register layout, qemu need modify. There should be compatible requirements between gdb client and gdb server. Tiezhu, what is your opition? Regards Bibo Mao 在 2023/8/8 18:03, Jiajie Chen 写道: > > On 2023/8/8 17:55, Jiajie Chen wrote: >> >> On 2023/8/8 14:10, bibo mao wrote: >>> I am not familiar with gdb, is there abi breakage? >>> I do not know how gdb client works with gdb server with different versions. > There seemed no versioning in the process, but rather in-code xml validation. In gdb, the code only allows new xml (fcc0-7) and rejects old one (fcc), so gdb breaks qemu first and do not consider backward compatibility with qemu. >> >> Not abi breakage, but gdb will complain: >> >> warning: while parsing target description (at line 1): Target description specified unknown architecture "loongarch64" >> warning: Could not load XML target description; ignoring >> warning: No executable has been specified and target does not support >> determining executable automatically. Try using the "file" command. >> Truncated register 38 in remote 'g' packet > > Sorry, to be clear, the actual error message is: > > (gdb) target extended-remote localhost:1234 > Remote debugging using localhost:1234 > warning: Architecture rejected target-supplied description > warning: No executable has been specified and target does not support > > It rejects the target description xml given by qemu, thus using the builtin one. However, there is a mismatch in fcc registers, so it will not work if we list floating point registers. > > At the same time, if we are using loongarch32 target(I recently posted patches to support this), it will reject the target description and fallback to loongarch64, making gcc not usable. > >> >> And gdb can no longer debug kernel running in qemu. You can reproduce this error using latest qemu(without this patch) and gdb(13.1 or later). >> >>> >>> Regards >>> Bibo Mao >>> >>> >>> 在 2023/8/8 13:42, Jiajie Chen 写道: >>>> Since GDB 13.1(GDB commit ea3352172), GDB LoongArch changed to use >>>> fcc0-7 instead of fcc register. This commit partially reverts commit >>>> 2f149c759 (`target/loongarch: Update gdb_set_fpu() and gdb_get_fpu()`) >>>> to match the behavior of GDB. >>>> >>>> Note that it is a breaking change for GDB 13.0 or earlier, but it is >>>> also required for GDB 13.1 or later to work. >>>> >>>> Signed-off-by: Jiajie Chen <c@jia.je> >>>> --- >>>> gdb-xml/loongarch-fpu.xml | 9 ++++++++- >>>> target/loongarch/gdbstub.c | 16 +++++++--------- >>>> 2 files changed, 15 insertions(+), 10 deletions(-) >>>> >>>> diff --git a/gdb-xml/loongarch-fpu.xml b/gdb-xml/loongarch-fpu.xml >>>> index 78e42cf5dd..e81e3382e7 100644 >>>> --- a/gdb-xml/loongarch-fpu.xml >>>> +++ b/gdb-xml/loongarch-fpu.xml >>>> @@ -45,6 +45,13 @@ >>>> <reg name="f29" bitsize="64" type="fputype" group="float"/> >>>> <reg name="f30" bitsize="64" type="fputype" group="float"/> >>>> <reg name="f31" bitsize="64" type="fputype" group="float"/> >>>> - <reg name="fcc" bitsize="64" type="uint64" group="float"/> >>>> + <reg name="fcc0" bitsize="8" type="uint8" group="float"/> >>>> + <reg name="fcc1" bitsize="8" type="uint8" group="float"/> >>>> + <reg name="fcc2" bitsize="8" type="uint8" group="float"/> >>>> + <reg name="fcc3" bitsize="8" type="uint8" group="float"/> >>>> + <reg name="fcc4" bitsize="8" type="uint8" group="float"/> >>>> + <reg name="fcc5" bitsize="8" type="uint8" group="float"/> >>>> + <reg name="fcc6" bitsize="8" type="uint8" group="float"/> >>>> + <reg name="fcc7" bitsize="8" type="uint8" group="float"/> >>>> <reg name="fcsr" bitsize="32" type="uint32" group="float"/> >>>> </feature> >>>> diff --git a/target/loongarch/gdbstub.c b/target/loongarch/gdbstub.c >>>> index 0752fff924..15ad6778f1 100644 >>>> --- a/target/loongarch/gdbstub.c >>>> +++ b/target/loongarch/gdbstub.c >>>> @@ -70,10 +70,9 @@ static int loongarch_gdb_get_fpu(CPULoongArchState *env, >>>> { >>>> if (0 <= n && n < 32) { >>>> return gdb_get_reg64(mem_buf, env->fpr[n].vreg.D(0)); >>>> - } else if (n == 32) { >>>> - uint64_t val = read_fcc(env); >>>> - return gdb_get_reg64(mem_buf, val); >>>> - } else if (n == 33) { >>>> + } else if (32 <= n && n < 40) { >>>> + return gdb_get_reg8(mem_buf, env->cf[n - 32]); >>>> + } else if (n == 40) { >>>> return gdb_get_reg32(mem_buf, env->fcsr0); >>>> } >>>> return 0; >>>> @@ -87,11 +86,10 @@ static int loongarch_gdb_set_fpu(CPULoongArchState *env, >>>> if (0 <= n && n < 32) { >>>> env->fpr[n].vreg.D(0) = ldq_p(mem_buf); >>>> length = 8; >>>> - } else if (n == 32) { >>>> - uint64_t val = ldq_p(mem_buf); >>>> - write_fcc(env, val); >>>> - length = 8; >>>> - } else if (n == 33) { >>>> + } else if (32 <= n && n < 40) { >>>> + env->cf[n - 32] = ldub_p(mem_buf); >>>> + length = 1; >>>> + } else if (n == 40) { >>>> env->fcsr0 = ldl_p(mem_buf); >>>> length = 4; >>>> } ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] target/loongarch: Split fcc register to fcc0-7 in gdbstub 2023-08-08 11:42 ` bibo mao @ 2023-08-08 18:40 ` Alex Bennée 2023-08-19 6:44 ` gaosong 0 siblings, 1 reply; 8+ messages in thread From: Alex Bennée @ 2023-08-08 18:40 UTC (permalink / raw) To: bibo mao Cc: Jiajie Chen, Tiezhu Yang, Song Gao, Philippe Mathieu-Daudé, qemu-devel bibo mao <maobibo@loongson.cn> writes: > I think that it is problem of loongarch gdb, rather qemu. > If so, everytime when gdb changes register layout, qemu need modify. > There should be compatible requirements between gdb client and gdb server. > > Tiezhu, > > what is your opition? You can always register additional custom regsets which is what we do for the extended Aarch64 regs. See ->gdb_get_dynamic_xml > > Regards > Bibo Mao > > 在 2023/8/8 18:03, Jiajie Chen 写道: >> >> On 2023/8/8 17:55, Jiajie Chen wrote: >>> >>> On 2023/8/8 14:10, bibo mao wrote: >>>> I am not familiar with gdb, is there abi breakage? >>>> I do not know how gdb client works with gdb server with different versions. >> There seemed no versioning in the process, but rather in-code xml >> validation. In gdb, the code only allows new xml (fcc0-7) and >> rejects old one (fcc), so gdb breaks qemu first and do not consider >> backward compatibility with qemu. >>> >>> Not abi breakage, but gdb will complain: >>> >>> warning: while parsing target description (at line 1): Target >>> description specified unknown architecture "loongarch64" >>> warning: Could not load XML target description; ignoring >>> warning: No executable has been specified and target does not support >>> determining executable automatically. Try using the "file" command. >>> Truncated register 38 in remote 'g' packet >> >> Sorry, to be clear, the actual error message is: >> >> (gdb) target extended-remote localhost:1234 >> Remote debugging using localhost:1234 >> warning: Architecture rejected target-supplied description >> warning: No executable has been specified and target does not support >> >> It rejects the target description xml given by qemu, thus using the >> builtin one. However, there is a mismatch in fcc registers, so it >> will not work if we list floating point registers. >> >> At the same time, if we are using loongarch32 target(I recently >> posted patches to support this), it will reject the target >> description and fallback to loongarch64, making gcc not usable. >> >>> >>> And gdb can no longer debug kernel running in qemu. You can >>> reproduce this error using latest qemu(without this patch) and >>> gdb(13.1 or later). >>> >>>> >>>> Regards >>>> Bibo Mao >>>> >>>> >>>> 在 2023/8/8 13:42, Jiajie Chen 写道: >>>>> Since GDB 13.1(GDB commit ea3352172), GDB LoongArch changed to use >>>>> fcc0-7 instead of fcc register. This commit partially reverts commit >>>>> 2f149c759 (`target/loongarch: Update gdb_set_fpu() and gdb_get_fpu()`) >>>>> to match the behavior of GDB. >>>>> >>>>> Note that it is a breaking change for GDB 13.0 or earlier, but it is >>>>> also required for GDB 13.1 or later to work. >>>>> >>>>> Signed-off-by: Jiajie Chen <c@jia.je> >>>>> --- >>>>> gdb-xml/loongarch-fpu.xml | 9 ++++++++- >>>>> target/loongarch/gdbstub.c | 16 +++++++--------- >>>>> 2 files changed, 15 insertions(+), 10 deletions(-) >>>>> >>>>> diff --git a/gdb-xml/loongarch-fpu.xml b/gdb-xml/loongarch-fpu.xml >>>>> index 78e42cf5dd..e81e3382e7 100644 >>>>> --- a/gdb-xml/loongarch-fpu.xml >>>>> +++ b/gdb-xml/loongarch-fpu.xml >>>>> @@ -45,6 +45,13 @@ >>>>> <reg name="f29" bitsize="64" type="fputype" group="float"/> >>>>> <reg name="f30" bitsize="64" type="fputype" group="float"/> >>>>> <reg name="f31" bitsize="64" type="fputype" group="float"/> >>>>> - <reg name="fcc" bitsize="64" type="uint64" group="float"/> >>>>> + <reg name="fcc0" bitsize="8" type="uint8" group="float"/> >>>>> + <reg name="fcc1" bitsize="8" type="uint8" group="float"/> >>>>> + <reg name="fcc2" bitsize="8" type="uint8" group="float"/> >>>>> + <reg name="fcc3" bitsize="8" type="uint8" group="float"/> >>>>> + <reg name="fcc4" bitsize="8" type="uint8" group="float"/> >>>>> + <reg name="fcc5" bitsize="8" type="uint8" group="float"/> >>>>> + <reg name="fcc6" bitsize="8" type="uint8" group="float"/> >>>>> + <reg name="fcc7" bitsize="8" type="uint8" group="float"/> >>>>> <reg name="fcsr" bitsize="32" type="uint32" group="float"/> >>>>> </feature> >>>>> diff --git a/target/loongarch/gdbstub.c b/target/loongarch/gdbstub.c >>>>> index 0752fff924..15ad6778f1 100644 >>>>> --- a/target/loongarch/gdbstub.c >>>>> +++ b/target/loongarch/gdbstub.c >>>>> @@ -70,10 +70,9 @@ static int loongarch_gdb_get_fpu(CPULoongArchState *env, >>>>> { >>>>> if (0 <= n && n < 32) { >>>>> return gdb_get_reg64(mem_buf, env->fpr[n].vreg.D(0)); >>>>> - } else if (n == 32) { >>>>> - uint64_t val = read_fcc(env); >>>>> - return gdb_get_reg64(mem_buf, val); >>>>> - } else if (n == 33) { >>>>> + } else if (32 <= n && n < 40) { >>>>> + return gdb_get_reg8(mem_buf, env->cf[n - 32]); >>>>> + } else if (n == 40) { >>>>> return gdb_get_reg32(mem_buf, env->fcsr0); >>>>> } >>>>> return 0; >>>>> @@ -87,11 +86,10 @@ static int loongarch_gdb_set_fpu(CPULoongArchState *env, >>>>> if (0 <= n && n < 32) { >>>>> env->fpr[n].vreg.D(0) = ldq_p(mem_buf); >>>>> length = 8; >>>>> - } else if (n == 32) { >>>>> - uint64_t val = ldq_p(mem_buf); >>>>> - write_fcc(env, val); >>>>> - length = 8; >>>>> - } else if (n == 33) { >>>>> + } else if (32 <= n && n < 40) { >>>>> + env->cf[n - 32] = ldub_p(mem_buf); >>>>> + length = 1; >>>>> + } else if (n == 40) { >>>>> env->fcsr0 = ldl_p(mem_buf); >>>>> length = 4; >>>>> } -- Alex Bennée Virtualisation Tech Lead @ Linaro ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] target/loongarch: Split fcc register to fcc0-7 in gdbstub 2023-08-08 18:40 ` Alex Bennée @ 2023-08-19 6:44 ` gaosong 0 siblings, 0 replies; 8+ messages in thread From: gaosong @ 2023-08-19 6:44 UTC (permalink / raw) To: Alex Bennée, bibo mao Cc: Jiajie Chen, Tiezhu Yang, Philippe Mathieu-Daudé, qemu-devel Hi, Alex 在 2023/8/9 上午2:40, Alex Bennée 写道: > > bibo mao <maobibo@loongson.cn> writes: > >> I think that it is problem of loongarch gdb, rather qemu. >> If so, everytime when gdb changes register layout, qemu need modify. >> There should be compatible requirements between gdb client and gdb server. >> >> Tiezhu, >> >> what is your opition? > > You can always register additional custom regsets which is what we do > for the extended Aarch64 regs. See ->gdb_get_dynamic_xml > Thanks for you suggestions. we will use this method for vector extented. For this patch: Acked-by: Song Gao <gaosong@loongson.cn> Thanks. Song Gao >> >> Regards >> Bibo Mao >> >> 在 2023/8/8 18:03, Jiajie Chen 写道: >>> >>> On 2023/8/8 17:55, Jiajie Chen wrote: >>>> >>>> On 2023/8/8 14:10, bibo mao wrote: >>>>> I am not familiar with gdb, is there abi breakage? >>>>> I do not know how gdb client works with gdb server with different versions. >>> There seemed no versioning in the process, but rather in-code xml >>> validation. In gdb, the code only allows new xml (fcc0-7) and >>> rejects old one (fcc), so gdb breaks qemu first and do not consider >>> backward compatibility with qemu. >>>> >>>> Not abi breakage, but gdb will complain: >>>> >>>> warning: while parsing target description (at line 1): Target >>>> description specified unknown architecture "loongarch64" >>>> warning: Could not load XML target description; ignoring >>>> warning: No executable has been specified and target does not support >>>> determining executable automatically. Try using the "file" command. >>>> Truncated register 38 in remote 'g' packet >>> >>> Sorry, to be clear, the actual error message is: >>> >>> (gdb) target extended-remote localhost:1234 >>> Remote debugging using localhost:1234 >>> warning: Architecture rejected target-supplied description >>> warning: No executable has been specified and target does not support >>> >>> It rejects the target description xml given by qemu, thus using the >>> builtin one. However, there is a mismatch in fcc registers, so it >>> will not work if we list floating point registers. >>> >>> At the same time, if we are using loongarch32 target(I recently >>> posted patches to support this), it will reject the target >>> description and fallback to loongarch64, making gcc not usable. >>> >>>> >>>> And gdb can no longer debug kernel running in qemu. You can >>>> reproduce this error using latest qemu(without this patch) and >>>> gdb(13.1 or later). >>>> >>>>> >>>>> Regards >>>>> Bibo Mao >>>>> >>>>> >>>>> 在 2023/8/8 13:42, Jiajie Chen 写道: >>>>>> Since GDB 13.1(GDB commit ea3352172), GDB LoongArch changed to use >>>>>> fcc0-7 instead of fcc register. This commit partially reverts commit >>>>>> 2f149c759 (`target/loongarch: Update gdb_set_fpu() and gdb_get_fpu()`) >>>>>> to match the behavior of GDB. >>>>>> >>>>>> Note that it is a breaking change for GDB 13.0 or earlier, but it is >>>>>> also required for GDB 13.1 or later to work. >>>>>> >>>>>> Signed-off-by: Jiajie Chen <c@jia.je> >>>>>> --- >>>>>> gdb-xml/loongarch-fpu.xml | 9 ++++++++- >>>>>> target/loongarch/gdbstub.c | 16 +++++++--------- >>>>>> 2 files changed, 15 insertions(+), 10 deletions(-) >>>>>> >>>>>> diff --git a/gdb-xml/loongarch-fpu.xml b/gdb-xml/loongarch-fpu.xml >>>>>> index 78e42cf5dd..e81e3382e7 100644 >>>>>> --- a/gdb-xml/loongarch-fpu.xml >>>>>> +++ b/gdb-xml/loongarch-fpu.xml >>>>>> @@ -45,6 +45,13 @@ >>>>>> <reg name="f29" bitsize="64" type="fputype" group="float"/> >>>>>> <reg name="f30" bitsize="64" type="fputype" group="float"/> >>>>>> <reg name="f31" bitsize="64" type="fputype" group="float"/> >>>>>> - <reg name="fcc" bitsize="64" type="uint64" group="float"/> >>>>>> + <reg name="fcc0" bitsize="8" type="uint8" group="float"/> >>>>>> + <reg name="fcc1" bitsize="8" type="uint8" group="float"/> >>>>>> + <reg name="fcc2" bitsize="8" type="uint8" group="float"/> >>>>>> + <reg name="fcc3" bitsize="8" type="uint8" group="float"/> >>>>>> + <reg name="fcc4" bitsize="8" type="uint8" group="float"/> >>>>>> + <reg name="fcc5" bitsize="8" type="uint8" group="float"/> >>>>>> + <reg name="fcc6" bitsize="8" type="uint8" group="float"/> >>>>>> + <reg name="fcc7" bitsize="8" type="uint8" group="float"/> >>>>>> <reg name="fcsr" bitsize="32" type="uint32" group="float"/> >>>>>> </feature> >>>>>> diff --git a/target/loongarch/gdbstub.c b/target/loongarch/gdbstub.c >>>>>> index 0752fff924..15ad6778f1 100644 >>>>>> --- a/target/loongarch/gdbstub.c >>>>>> +++ b/target/loongarch/gdbstub.c >>>>>> @@ -70,10 +70,9 @@ static int loongarch_gdb_get_fpu(CPULoongArchState *env, >>>>>> { >>>>>> if (0 <= n && n < 32) { >>>>>> return gdb_get_reg64(mem_buf, env->fpr[n].vreg.D(0)); >>>>>> - } else if (n == 32) { >>>>>> - uint64_t val = read_fcc(env); >>>>>> - return gdb_get_reg64(mem_buf, val); >>>>>> - } else if (n == 33) { >>>>>> + } else if (32 <= n && n < 40) { >>>>>> + return gdb_get_reg8(mem_buf, env->cf[n - 32]); >>>>>> + } else if (n == 40) { >>>>>> return gdb_get_reg32(mem_buf, env->fcsr0); >>>>>> } >>>>>> return 0; >>>>>> @@ -87,11 +86,10 @@ static int loongarch_gdb_set_fpu(CPULoongArchState *env, >>>>>> if (0 <= n && n < 32) { >>>>>> env->fpr[n].vreg.D(0) = ldq_p(mem_buf); >>>>>> length = 8; >>>>>> - } else if (n == 32) { >>>>>> - uint64_t val = ldq_p(mem_buf); >>>>>> - write_fcc(env, val); >>>>>> - length = 8; >>>>>> - } else if (n == 33) { >>>>>> + } else if (32 <= n && n < 40) { >>>>>> + env->cf[n - 32] = ldub_p(mem_buf); >>>>>> + length = 1; >>>>>> + } else if (n == 40) { >>>>>> env->fcsr0 = ldl_p(mem_buf); >>>>>> length = 4; >>>>>> } > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-08-19 6:46 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-08-08 5:42 [PATCH] target/loongarch: Split fcc register to fcc0-7 in gdbstub Jiajie Chen 2023-08-08 6:10 ` bibo mao 2023-08-08 6:28 ` bibo mao 2023-08-08 9:55 ` Jiajie Chen 2023-08-08 10:03 ` Jiajie Chen 2023-08-08 11:42 ` bibo mao 2023-08-08 18:40 ` Alex Bennée 2023-08-19 6:44 ` gaosong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).