1. Patchew
  2. QEMU
  3. View series

[PATCH] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.

TaiseiIto posted 1 patch 1 year, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/TY0PR0101MB42855925D8414E4773D6FA36A41D9@TY0PR0101MB4285.apcprd01.prod.exchangelabs.com
There is a newer version of this series
target/i386/gdbstub.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[PATCH] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.
Posted by TaiseiIto 1 year, 4 months ago 
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..6109ad166d 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 = (floatx80 *) &env->fpregs[r_index];
         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;
-- 
2.34.1
Re: [PATCH] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.
Posted by Richard Henderson 1 year, 4 months ago 
On 12/8/22 04:30, TaiseiIto wrote:
> 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..6109ad166d 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 = (floatx80 *) &env->fpregs[r_index];

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Note that the cast can be dropped by taking the address of the ".d" union member.


r~
RE: [PATCH] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.
Posted by 伊藤 太清 1 year, 4 months ago 
Supplementary explanation about the patch

1. Reproduction of the bug

The following 3 files are needed to reproduce the bug.

* test_os.s
* test_os.ld
* Makefile

And the following 2 tools, too.

* build-essential
* gdb

The contents of the above files are below.

---------- Begin of test_os.s ----------

            .code16
            .text
main:
            fninit  # Initialize the FPU
            fld1    # Push 1.0
            fldl2t  # Push log 2 10
            fldl2e # Push log 2 e
            fldpi   # Push pi
            fldlg2 # Push log 10 2
            fldln2 # Push log e 2
            fldz     # Push 0.0
loop:
            hlt
            jmp loop

---------- End of test_os.s ----------

---------- Begin of test_os.ld ----------

OUTPUT_FORMAT("binary");

BASE = 0x00007c00;

SECTIONS
{
            . = BASE;
            .text :
            {
                       test_os.o(.text)
            }
            . = BASE;
            . += 0x00000200;
            . -= 0x00000002;
            .boot_sector_sign :
            {
                       BYTE(0x55);
                       BYTE(0xaa);
            }
            /DISCARD/ :
            {
                       *(.eh_frame)
                       *(.note.gnu.property)
            }
}

---------- End of test_os.ld ----------

---------- Begin of Makefile ----------

TEST_OS_NAME = test_o
TEST_OS_NAME = test_os
TEST_OS_ASM = $(TEST_OS_NAME).s
TEST_OS_IMG = $(TEST_OS_NAME).img
TEST_OS_LNK = $(TEST_OS_NAME).ld
TEST_OS_MAP = $(TEST_OS_NAME).map
TEST_OS_OBJ = $(TEST_OS_NAME).o

all: $(TEST_OS_IMG)

test: $(TEST_OS_IMG)
            (qemu-system-i386 -boot order=a \
            -drive file=$<,format=raw,if=floppy \
            -S -gdb tcp::2159 -vnc localhost:0 &) && \
            gdb

$(TEST_OS_IMG): $(TEST_OS_OBJ) $(TEST_OS_LNK)
            ld $< -Map $(TEST_OS_MAP) -o $@ -T $(word 2, $^)

$(TEST_OS_OBJ): $(TEST_OS_ASM)
            gcc $^ -c -nostdlib -o $@ -Wall -Wextra

---------- End of Makefile ----------

Put these files on a same directory. "test_os.s" is source code of tiny OS
to run on QEMU. The OS consists only a boot sector. It initialize x87 FPU
and pushes some floating point values onto x87 FPU stack. "test_os.ld" is
its linker script. And you can make "test_os.img", a raw image of the OS.
Now, there are all things to reproduce the bug. You can "make test" to let
QEMU run the OS and wait for GDB, then GDB will start. Then you can execute
some GDB commands to reproduce the bug. Below is "result 1" reproducing the
bug.

---------- Begin of result 1 ----------

GNU gdb (GDB) 13.0.50.20221204-git
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word".
(gdb) target remote localhost:2159
Remote debugging using localhost:2159
warning: No executable has been specified and target does not support
determining executable automatically.  Try using the "file" command.
0x0000fff0 in ?? ()
(gdb) break *0x7c00
Breakpoint 1 at 0x7c00
(gdb) continue
Continuing.

Breakpoint 1, 0x00007c00 in ?? ()
(gdb) x/10i $eip
=> 0x7c00:      fninit
   0x7c02:      fld1
   0x7c04:      fldl2t
   0x7c06:      fldl2e
   0x7c08:      fldpi
   0x7c0a:      fldlg2
   0x7c0c:      fldln2
   0x7c0e:      fldz
   0x7c10:      hlt
   0x7c11:      jmp    0x7c10
(gdb) stepi
0x00007c02 in ?? ()
(gdb) info float
  R7: Valid   0x00000000000000000000 +0
  R6: Valid   0x00000000000000000000 +0
  R5: Valid   0x00000000000000000000 +0
  R4: Valid   0x00000000000000000000 +0
  R3: Valid   0x00000000000000000000 +0
  R2: Valid   0x00000000000000000000 +0
  R1: Valid   0x00000000000000000000 +0
=>R0: Valid   0x00000000000000000000 +0

Status Word:         0x0000
                       TOP: 0
Control Word:        0x037f   IM DM ZM OM UM PM
                       PC: Extended Precision (64-bits)
                       RC: Round to nearest
Tag Word:            0x0000
Instruction Pointer: 0x00:0x00000000
Operand Pointer:     0x00:0x00000000
Opcode:              0x0000
(gdb) stepi
0x00007c04 in ?? ()
(gdb) info float
=>R7: Valid   0x00000000000000000000 +0
  R6: Valid   0x3fff8000000000000000 +1
  R5: Valid   0x00000000000000000000 +0
  R4: Valid   0x00000000000000000000 +0
  R3: Valid   0x00000000000000000000 +0
  R2: Valid   0x00000000000000000000 +0
  R1: Valid   0x00000000000000000000 +0
  R0: Valid   0x00000000000000000000 +0

Status Word:         0x3800
                       TOP: 7
Control Word:        0x037f   IM DM ZM OM UM PM
                       PC: Extended Precision (64-bits)
                       RC: Round to nearest
Tag Word:            0x0000
Instruction Pointer: 0x00:0x00000000
Operand Pointer:     0x00:0x00000000
Opcode:              0x0000
(gdb) stepi
0x00007c06 in ?? ()
(gdb) info float
  R7: Valid   0x00000000000000000000 +0
=>R6: Valid   0x00000000000000000000 +0
  R5: Valid   0x3fff8000000000000000 +1
  R4: Valid   0x4000d49a784bcd1b8afe +3.321928094887362348
  R3: Valid   0x00000000000000000000 +0
  R2: Valid   0x00000000000000000000 +0
  R1: Valid   0x00000000000000000000 +0
  R0: Valid   0x00000000000000000000 +0

Status Word:         0x3000
                       TOP: 6
Control Word:        0x037f   IM DM ZM OM UM PM
                       PC: Extended Precision (64-bits)
                       RC: Round to nearest
Tag Word:            0x0000
Instruction Pointer: 0x00:0x00000000
Operand Pointer:     0x00:0x00000000
Opcode:              0x0000
(gdb) stepi
0x00007c08 in ?? ()
(gdb) info float
  R7: Valid   0x00000000000000000000 +0
  R6: Valid   0x00000000000000000000 +0
=>R5: Valid   0x00000000000000000000 +0
  R4: Valid   0x3fff8000000000000000 +1
  R3: Valid   0x4000d49a784bcd1b8afe +3.321928094887362348
  R2: Valid   0x3fffb8aa3b295c17f0bc +1.442695040888963407
  R1: Valid   0x00000000000000000000 +0
  R0: Valid   0x00000000000000000000 +0

Status Word:         0x2800
                       TOP: 5
Control Word:        0x037f   IM DM ZM OM UM PM
                       PC: Extended Precision (64-bits)
                       RC: Round to nearest
Tag Word:            0x0000
Instruction Pointer: 0x00:0x00000000
Operand Pointer:     0x00:0x00000000
Opcode:              0x0000
(gdb) stepi
0x00007c0a in ?? ()
(gdb) info float
  R7: Valid   0x00000000000000000000 +0
  R6: Valid   0x00000000000000000000 +0
  R5: Valid   0x00000000000000000000 +0
=>R4: Valid   0x00000000000000000000 +0
  R3: Valid   0x3fff8000000000000000 +1
  R2: Valid   0x4000d49a784bcd1b8afe +3.321928094887362348
  R1: Valid   0x3fffb8aa3b295c17f0bc +1.442695040888963407
  R0: Valid   0x4000c90fdaa22168c235 +3.141592653589793239

Status Word:         0x2000
                       TOP: 4
Control Word:        0x037f   IM DM ZM OM UM PM
                       PC: Extended Precision (64-bits)
                       RC: Round to nearest
Tag Word:            0x0000
Instruction Pointer: 0x00:0x00000000
Operand Pointer:     0x00:0x00000000
Opcode:              0x0000
(gdb) stepi
0x00007c0c in ?? ()
(gdb) info float
  R7: Valid   0x4000c90fdaa22168c235 +3.141592653589793239
  R6: Valid   0x3ffd9a209a84fbcff799 +0.3010299956639811952
  R5: Valid   0x00000000000000000000 +0
  R4: Valid   0x00000000000000000000 +0
=>R3: Valid   0x00000000000000000000 +0
  R2: Valid   0x3fff8000000000000000 +1
  R1: Valid   0x4000d49a784bcd1b8afe +3.321928094887362348
  R0: Valid   0x3fffb8aa3b295c17f0bc +1.442695040888963407

Status Word:         0x1800
                       TOP: 3
Control Word:        0x037f   IM DM ZM OM UM PM
                       PC: Extended Precision (64-bits)
                       RC: Round to nearest
Tag Word:            0x0000
Instruction Pointer: 0x00:0x00000000
Operand Pointer:     0x00:0x00000000
Opcode:              0x0000
(gdb) stepi
0x00007c0e in ?? ()
(gdb) info float
  R7: Valid   0x3fffb8aa3b295c17f0bc +1.442695040888963407
  R6: Valid   0x4000c90fdaa22168c235 +3.141592653589793239
  R5: Valid   0x3ffd9a209a84fbcff799 +0.3010299956639811952
  R4: Valid   0x3ffeb17217f7d1cf79ac +0.6931471805599453094
  R3: Valid   0x00000000000000000000 +0
=>R2: Valid   0x00000000000000000000 +0
  R1: Valid   0x3fff8000000000000000 +1
  R0: Valid   0x4000d49a784bcd1b8afe +3.321928094887362348

Status Word:         0x1000
                       TOP: 2
Control Word:        0x037f   IM DM ZM OM UM PM
                       PC: Extended Precision (64-bits)
                       RC: Round to nearest
Tag Word:            0x0000
Instruction Pointer: 0x00:0x00000000
Operand Pointer:     0x00:0x00000000
Opcode:              0x0000
(gdb) stepi
0x00007c10 in ?? ()
(gdb) info float
  R7: Valid   0x4000d49a784bcd1b8afe +3.321928094887362348
  R6: Valid   0x3fffb8aa3b295c17f0bc +1.442695040888963407
  R5: Valid   0x4000c90fdaa22168c235 +3.141592653589793239
  R4: Valid   0x3ffd9a209a84fbcff799 +0.3010299956639811952
  R3: Valid   0x3ffeb17217f7d1cf79ac +0.6931471805599453094
  R2: Valid   0x00000000000000000000 +0
=>R1: Valid   0x00000000000000000000 +0
  R0: Valid   0x3fff8000000000000000 +1

Status Word:         0x0800
                       TOP: 1
Control Word:        0x037f   IM DM ZM OM UM PM
                       PC: Extended Precision (64-bits)
                       RC: Round to nearest
Tag Word:            0x0000
Instruction Pointer: 0x00:0x00000000
Operand Pointer:     0x00:0x00000000
Opcode:              0x0000

---------- End of result 1 ----------

As you can see, the FPU stack rotates every pushing.

2. Cause

There is a cause of the bug in a function "x86_cpu_gdb_read_register" in
"qemu/target/i386/gdbstub.c". GDB receives a command "info float" from stdin
and get values of the all registers containing FPU stack registers from QEMU
to print them. Then, QEMU picks registers in the function to form 'g' packet
to send to GDB. In line 124 of the c source file, absolute indexed FPU stack
registers, namely, R0, ... and R7 , are picked and inserted in 'g' packet.
However, GDB, the packet receiver, extracts FPU stack registers from the
packet and interpret these registers are stack top relative indexed, namely,
ST0, ... and ST7. As a result, GDB can't print FPU stack correctly.

3. Modification

In added lines of this patch, "n" is a register number of a register to
read. And "IDX_FP_REGS" is register number of the first FPU stack register
"R0". So, "r_index" is absolute index of FPU stack register to read. And
"env->fpstt" is a pointer to top of FPU stack. So, "st_index" is stack top
relative index of FPU stack register to read. By applying this modification,
QEMU inserts FPU stack registers ordered by stack top relative index in 'g'
packet.

4. After this patch

Below is "result 2" operating as same as "result 1" after applying this
patch.

---------- Begin of result 2 ----------

GNU gdb (GDB) 13.0.50.20221204-git
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word".
(gdb) target remote localhost:2159
Remote debugging using localhost:2159
warning: No executable has been specified and target does not support
determining executable automatically.  Try using the "file" command.
0x0000fff0 in ?? ()
(gdb) break *0x7c00
Breakpoint 1 at 0x7c00
(gdb) continue
Continuing.

Breakpoint 1, 0x00007c00 in ?? ()
(gdb) x/10i $eip
=> 0x7c00:      fninit
   0x7c02:      fld1
   0x7c04:      fldl2t
   0x7c06:      fldl2e
   0x7c08:      fldpi
   0x7c0a:      fldlg2
   0x7c0c:      fldln2
   0x7c0e:      fldz
   0x7c10:      hlt
   0x7c11:      jmp    0x7c10
(gdb) stepi
0x00007c02 in ?? ()
(gdb) info float
  R7: Valid   0x00000000000000000000 +0
  R6: Valid   0x00000000000000000000 +0
  R5: Valid   0x00000000000000000000 +0
  R4: Valid   0x00000000000000000000 +0
  R3: Valid   0x00000000000000000000 +0
  R2: Valid   0x00000000000000000000 +0
  R1: Valid   0x00000000000000000000 +0
=>R0: Valid   0x00000000000000000000 +0

Status Word:         0x0000
                       TOP: 0
Control Word:        0x037f   IM DM ZM OM UM PM
                       PC: Extended Precision (64-bits)
                       RC: Round to nearest
Tag Word:            0x0000
Instruction Pointer: 0x00:0x00000000
Operand Pointer:     0x00:0x00000000
Opcode:              0x0000
(gdb) stepi
0x00007c04 in ?? ()
(gdb) info float
=>R7: Valid   0x3fff8000000000000000 +1
  R6: Valid   0x00000000000000000000 +0
  R5: Valid   0x00000000000000000000 +0
  R4: Valid   0x00000000000000000000 +0
  R3: Valid   0x00000000000000000000 +0
  R2: Valid   0x00000000000000000000 +0
  R1: Valid   0x00000000000000000000 +0
  R0: Valid   0x00000000000000000000 +0

Status Word:         0x3800
                       TOP: 7
Control Word:        0x037f   IM DM ZM OM UM PM
                       PC: Extended Precision (64-bits)
                       RC: Round to nearest
Tag Word:            0x0000
Instruction Pointer: 0x00:0x00000000
Operand Pointer:     0x00:0x00000000
Opcode:              0x0000
(gdb) stepi
0x00007c06 in ?? ()
(gdb) info float
  R7: Valid   0x3fff8000000000000000 +1
=>R6: Valid   0x4000d49a784bcd1b8afe +3.321928094887362348
  R5: Valid   0x00000000000000000000 +0
  R4: Valid   0x00000000000000000000 +0
  R3: Valid   0x00000000000000000000 +0
  R2: Valid   0x00000000000000000000 +0
  R1: Valid   0x00000000000000000000 +0
  R0: Valid   0x00000000000000000000 +0

Status Word:         0x3000
                       TOP: 6
Control Word:        0x037f   IM DM ZM OM UM PM
                       PC: Extended Precision (64-bits)
                       RC: Round to nearest
Tag Word:            0x0000
Instruction Pointer: 0x00:0x00000000
Operand Pointer:     0x00:0x00000000
Opcode:              0x0000
(gdb) stepi
0x00007c08 in ?? ()
(gdb) info float
  R7: Valid   0x3fff8000000000000000 +1
  R6: Valid   0x4000d49a784bcd1b8afe +3.321928094887362348
=>R5: Valid   0x3fffb8aa3b295c17f0bc +1.442695040888963407
  R4: Valid   0x00000000000000000000 +0
  R3: Valid   0x00000000000000000000 +0
  R2: Valid   0x00000000000000000000 +0
  R1: Valid   0x00000000000000000000 +0
  R0: Valid   0x00000000000000000000 +0

Status Word:         0x2800
                       TOP: 5
Control Word:        0x037f   IM DM ZM OM UM PM
                       PC: Extended Precision (64-bits)
                       RC: Round to nearest
Tag Word:            0x0000
Instruction Pointer: 0x00:0x00000000
Operand Pointer:     0x00:0x00000000
Opcode:              0x0000
(gdb) stepi
0x00007c0a in ?? ()
(gdb) info float
  R7: Valid   0x3fff8000000000000000 +1
  R6: Valid   0x4000d49a784bcd1b8afe +3.321928094887362348
  R5: Valid   0x3fffb8aa3b295c17f0bc +1.442695040888963407
=>R4: Valid   0x4000c90fdaa22168c235 +3.141592653589793239
  R3: Valid   0x00000000000000000000 +0
  R2: Valid   0x00000000000000000000 +0
  R1: Valid   0x00000000000000000000 +0
  R0: Valid   0x00000000000000000000 +0

Status Word:         0x2000
                       TOP: 4
Control Word:        0x037f   IM DM ZM OM UM PM
                       PC: Extended Precision (64-bits)
                       RC: Round to nearest
Tag Word:            0x0000
Instruction Pointer: 0x00:0x00000000
Operand Pointer:     0x00:0x00000000
Opcode:              0x0000
(gdb) stepi
0x00007c0c in ?? ()
(gdb) info float
  R7: Valid   0x3fff8000000000000000 +1
  R6: Valid   0x4000d49a784bcd1b8afe +3.321928094887362348
  R5: Valid   0x3fffb8aa3b295c17f0bc +1.442695040888963407
  R4: Valid   0x4000c90fdaa22168c235 +3.141592653589793239
=>R3: Valid   0x3ffd9a209a84fbcff799 +0.3010299956639811952
  R2: Valid   0x00000000000000000000 +0
  R1: Valid   0x00000000000000000000 +0
  R0: Valid   0x00000000000000000000 +0

Status Word:         0x1800
                       TOP: 3
Control Word:        0x037f   IM DM ZM OM UM PM
                       PC: Extended Precision (64-bits)
                       RC: Round to nearest
Tag Word:            0x0000
Instruction Pointer: 0x00:0x00000000
Operand Pointer:     0x00:0x00000000
Opcode:              0x0000
(gdb) stepi
0x00007c0e in ?? ()
(gdb) info float
  R7: Valid   0x3fff8000000000000000 +1
  R6: Valid   0x4000d49a784bcd1b8afe +3.321928094887362348
  R5: Valid   0x3fffb8aa3b295c17f0bc +1.442695040888963407
  R4: Valid   0x4000c90fdaa22168c235 +3.141592653589793239
  R3: Valid   0x3ffd9a209a84fbcff799 +0.3010299956639811952
=>R2: Valid   0x3ffeb17217f7d1cf79ac +0.6931471805599453094
  R1: Valid   0x00000000000000000000 +0
  R0: Valid   0x00000000000000000000 +0

Status Word:         0x1000
                       TOP: 2
Control Word:        0x037f   IM DM ZM OM UM PM
                       PC: Extended Precision (64-bits)
                       RC: Round to nearest
Tag Word:            0x0000
Instruction Pointer: 0x00:0x00000000
Operand Pointer:     0x00:0x00000000
Opcode:              0x0000
(gdb) stepi
0x00007c10 in ?? ()
(gdb) info float
  R7: Valid   0x3fff8000000000000000 +1
  R6: Valid   0x4000d49a784bcd1b8afe +3.321928094887362348
  R5: Valid   0x3fffb8aa3b295c17f0bc +1.442695040888963407
  R4: Valid   0x4000c90fdaa22168c235 +3.141592653589793239
  R3: Valid   0x3ffd9a209a84fbcff799 +0.3010299956639811952
  R2: Valid   0x3ffeb17217f7d1cf79ac +0.6931471805599453094
=>R1: Valid   0x00000000000000000000 +0
  R0: Valid   0x00000000000000000000 +0

Status Word:         0x0800
                       TOP: 1
Control Word:        0x037f   IM DM ZM OM UM PM
                       PC: Extended Precision (64-bits)
                       RC: Round to nearest
Tag Word:            0x0000
Instruction Pointer: 0x00:0x00000000
Operand Pointer:     0x00:0x00000000
Opcode:              0x0000

---------- End of result 2 ----------
[PATCH] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.
Posted by TaiseiIto 1 year, 4 months ago 
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..6109ad166d 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;
-- 
2.34.1
Re: [PATCH] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.
Posted by Paolo Bonzini 1 year, 2 months ago 
Queued, thanks.

Paolo
Re: [PATCH] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.
Posted by Richard Henderson 1 year, 4 months ago 
On 12/18/22 20:04, TaiseiIto wrote:
> 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(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

> 
> diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
> index c3a2cf6f28..6109ad166d 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;
Re: [PATCH] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.
Posted by Alex Bennée 1 year, 4 months ago 
TaiseiIto <taisei1212@outlook.jp> writes:

> 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..6109ad166d 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;

Shouldn't this have Richard's reviewed by tag? It's also useful if you
add a revision number to the subject (e.g. git send-email -v2) as well
as noting the differences under a --- marker so reviewers can see what
changed.

  https://qemu.readthedocs.io/en/latest/devel/submitting-a-patch.html#when-resending-patches-add-a-version-tag

Thanks,

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
[PATCH] [PING] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.
Posted by TaiseiIto 1 year, 2 months ago 
This is a ping to the patch below.

https://patchew.org/QEMU/TY0PR0101MB4285923FBE9AD97CE832D95BA4E59@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;
-- 
2.34.1
[PATCH v2] [PING^3] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.
Posted by TaiseiIto 1 year, 2 months ago 
This is a ping to the patch below.

https://patchew.org/QEMU/TY0PR0101MB4285923FBE9AD97CE832D95BA4E59@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;
-- 
2.34.1
Re: [PATCH v2] [PING^3] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.
Posted by Alex Bennée 1 year, 2 months ago 
TaiseiIto <taisei1212@outlook.jp> writes:

> This is a ping to the patch below.
>
> https://patchew.org/QEMU/TY0PR0101MB4285923FBE9AD97CE832D95BA4E59@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>

I'm confused what changed between v1 and v2? Why isn't Richard's tag applied?

> ---
>  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;


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
[PATCH v2] [PING] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.
Posted by TaiseiIto 1 year, 3 months ago 
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;
-- 
2.34.1
Re: [PATCH v2] [PING] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.
Posted by Alex Bennée 1 year, 3 months ago 
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
RE: [PATCH v2] [PING] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.
Posted by 伊藤 太清 1 year, 3 months ago 
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
Re: [PATCH qemu v3 0/1] Emulating sun keyboard language layout dip switches
Posted by Henrik Carlqvist 1 year, 3 months ago 
> 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
[PATCH v2] [PING^2] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.
Posted by TaiseiIto 1 year, 2 months ago 
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;
-- 
2.34.1
[PATCH] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.
Posted by TaiseiIto 1 year, 4 months ago 
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..6109ad166d 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;
-- 
2.34.1

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.