| Message ID | TY0PR0101MB4285AD60FE3976F1AD5C6D02A4F89@TY0PR0101MB4285.apcprd01.prod.exchangelabs.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,PING] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets. | expand |
TaiseiIto <taisei1212@outlook.jp> writes: > This is a ping to the patch below. > > https://patchew.org/QEMU/TY0PR0101MB42855925D8414E4773D6FA36A41D9@TY0PR0101MB4285.apcprd01.prod.exchangelabs.com/ > > Before this commit, when GDB attached an OS working on QEMU, order of FPU > stack registers printed by GDB command 'info float' was wrong. There was a > bug causing the problem in 'g' packets sent by QEMU to GDB. The packets have > values of registers of machine emulated by QEMU containing FPU stack > registers. There are 2 ways to specify a x87 FPU stack register. The first > is specifying by absolute indexed register names (R0, ..., R7). The second > is specifying by stack top relative indexed register names (ST0, ..., ST7). > Values of the FPU stack registers should be located in 'g' packet and be > ordered by the relative index. But QEMU had located these registers ordered > by the absolute index. After this commit, when QEMU reads registers to make > a 'g' packet, QEMU specifies FPU stack registers by the relative index. > Then, the registers are ordered correctly in the packet. As a result, GDB, > the packet receiver, can print FPU stack registers in the correct order. > > Signed-off-by: TaiseiIto <taisei1212@outlook.jp> > --- > target/i386/gdbstub.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c > index c3a2cf6f28..786971284a 100644 > --- a/target/i386/gdbstub.c > +++ b/target/i386/gdbstub.c > @@ -121,7 +121,9 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n) > return gdb_get_reg32(mem_buf, env->regs[gpr_map32[n]]); > } > } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) { > - floatx80 *fp = (floatx80 *) &env->fpregs[n - IDX_FP_REGS]; > + int st_index = n - IDX_FP_REGS; > + int r_index = (st_index + env->fpstt) % 8; > + floatx80 *fp = &env->fpregs[r_index].d; > int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low)); > len += gdb_get_reg16(mem_buf, cpu_to_le16(fp->high)); > return len; I'm sorry I though Paolo had already grabbed this, or is this a second fix to the FP handling?
Thank you for your reply. My first patch is already merged as a commit 75ac231c67cdb13f0609943fab5499963858b587 by Paolo. But it seems my second patch isn't merged yet. If Paolo or someone else plans to merge it, it's no problem. This is just a ping to the second patch. Not a new fix. ----- List of my patches. ----- The below is my first patch already merged as a commit 75ac231c67cdb13f0609943fab5499963858b587 by Paolo. https://patchew.org/QEMU/TY0PR0101MB4285F637209075C9F65FCDA6A4479@TY0PR0101MB4285.apcprd01.prod.exchangelabs.com/ The below is my second patch. https://patchew.org/QEMU/TY0PR0101MB42855925D8414E4773D6FA36A41D9@TY0PR0101MB4285.apcprd01.prod.exchangelabs.com/ The below is my second patch fixed according to Richard's review. https://patchew.org/QEMU/TY0PR0101MB4285923FBE9AD97CE832D95BA4E59@TY0PR0101MB4285.apcprd01.prod.exchangelabs.com/ The below is ping to fixed second patch. This is just a ping. Not a new fix. https://patchew.org/QEMU/TY0PR0101MB4285AD60FE3976F1AD5C6D02A4F89@TY0PR0101MB4285.apcprd01.prod.exchangelabs.com/ ------------------------------- Thanks. Taisei Sent from Mail<https://go.microsoft.com/fwlink/?LinkId=550986> for Windows From: Alex Bennée<mailto:alex.bennee@linaro.org> Sent: Saturday, January 7, 2023 7:16 PM To: TaiseiIto<mailto:taisei1212@outlook.jp> Cc: qemu-devel@nongnu.org<mailto:qemu-devel@nongnu.org>; richard.henderson@linaro.org<mailto:richard.henderson@linaro.org>; Paolo Bonzini<mailto:pbonzini@redhat.com> Subject: Re: [PATCH v2] [PING] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets. TaiseiIto <taisei1212@outlook.jp> writes: > This is a ping to the patch below. > > https://patchew.org/QEMU/TY0PR0101MB42855925D8414E4773D6FA36A41D9@TY0PR0101MB4285.apcprd01.prod.exchangelabs.com/ > > Before this commit, when GDB attached an OS working on QEMU, order of FPU > stack registers printed by GDB command 'info float' was wrong. There was a > bug causing the problem in 'g' packets sent by QEMU to GDB. The packets have > values of registers of machine emulated by QEMU containing FPU stack > registers. There are 2 ways to specify a x87 FPU stack register. The first > is specifying by absolute indexed register names (R0, ..., R7). The second > is specifying by stack top relative indexed register names (ST0, ..., ST7). > Values of the FPU stack registers should be located in 'g' packet and be > ordered by the relative index. But QEMU had located these registers ordered > by the absolute index. After this commit, when QEMU reads registers to make > a 'g' packet, QEMU specifies FPU stack registers by the relative index. > Then, the registers are ordered correctly in the packet. As a result, GDB, > the packet receiver, can print FPU stack registers in the correct order. > > Signed-off-by: TaiseiIto <taisei1212@outlook.jp> > --- > target/i386/gdbstub.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c > index c3a2cf6f28..786971284a 100644 > --- a/target/i386/gdbstub.c > +++ b/target/i386/gdbstub.c > @@ -121,7 +121,9 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n) > return gdb_get_reg32(mem_buf, env->regs[gpr_map32[n]]); > } > } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) { > - floatx80 *fp = (floatx80 *) &env->fpregs[n - IDX_FP_REGS]; > + int st_index = n - IDX_FP_REGS; > + int r_index = (st_index + env->fpstt) % 8; > + floatx80 *fp = &env->fpregs[r_index].d; > int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low)); > len += gdb_get_reg16(mem_buf, cpu_to_le16(fp->high)); > return len; I'm sorry I though Paolo had already grabbed this, or is this a second fix to the FP handling? -- Alex Bennée Virtualisation Tech Lead @ Linaro
> Year 2020 I made 2 attempts to contribute this patch. Unfortunately "git > format-patch" produced crippled patches which were not possible to > apply. Some @@-lines got extra code that didn't belong in those lines. > Now I am instead trying to send my patch using sourcehut. Unfortunately, > it seems as if the patch created by sourcehut is still crippled, Much to my surprise, it seems as if the patch created and sent by sourcehut applies cleanly. I falsely thought that it was the source text after the @@ lines that caused the problems, but it turned out that when I sent my first attempts by mail lines got wrapped by my email client and those wrapped lines caused the problem. The patch v2 which 2020 I created manually with git and sent by email got a nice"singed-off-by" line, the patch v3 created by sourcehut misses that line. Is the missing signed-off-by line a show stopper? If so, is sourcehut somehow usable to post patches? If sourcehut is unusable for this purpose I might have to send the patch as email again, but to avoid lines getting wrapped I will then post them as attachements instead of the preferred way as inline in the email text. regards Henrik
diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c index c3a2cf6f28..786971284a 100644 --- a/target/i386/gdbstub.c +++ b/target/i386/gdbstub.c @@ -121,7 +121,9 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n) return gdb_get_reg32(mem_buf, env->regs[gpr_map32[n]]); } } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) { - floatx80 *fp = (floatx80 *) &env->fpregs[n - IDX_FP_REGS]; + int st_index = n - IDX_FP_REGS; + int r_index = (st_index + env->fpstt) % 8; + floatx80 *fp = &env->fpregs[r_index].d; int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low)); len += gdb_get_reg16(mem_buf, cpu_to_le16(fp->high)); return len;
This is a ping to the patch below. https://patchew.org/QEMU/TY0PR0101MB42855925D8414E4773D6FA36A41D9@TY0PR0101MB4285.apcprd01.prod.exchangelabs.com/ Before this commit, when GDB attached an OS working on QEMU, order of FPU stack registers printed by GDB command 'info float' was wrong. There was a bug causing the problem in 'g' packets sent by QEMU to GDB. The packets have values of registers of machine emulated by QEMU containing FPU stack registers. There are 2 ways to specify a x87 FPU stack register. The first is specifying by absolute indexed register names (R0, ..., R7). The second is specifying by stack top relative indexed register names (ST0, ..., ST7). Values of the FPU stack registers should be located in 'g' packet and be ordered by the relative index. But QEMU had located these registers ordered by the absolute index. After this commit, when QEMU reads registers to make a 'g' packet, QEMU specifies FPU stack registers by the relative index. Then, the registers are ordered correctly in the packet. As a result, GDB, the packet receiver, can print FPU stack registers in the correct order. Signed-off-by: TaiseiIto <taisei1212@outlook.jp> --- target/i386/gdbstub.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)