Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 2/3] target/ppc: Add GDB callbacks for SPRs
On 01/02/2019 23:01, Fabiano Rosas wrote: > Alexey Kardashevskiy writes: > >> On 01/02/2019 08:57, Fabiano Rosas wrote: >>> Alexey Kardashevskiy writes: >>> On 31/01/2019 03:30, Fabiano Rosas wrote: > Alexey Kardashevskiy writes: > >> >> but this is a register which does not have endianness, the endianness >> appears here because the interface between gdb and qemu is >> uint8_t*==bytestream but this interface should have fixed endianness >> imho (now it is bigendian afaict). >> >> Something is not right here... > > Having a fixed endianness would not work because GDB have no way of > knowing how to represent what comes from the remote end. It definitely would. A register is stored as "unsigned long" in QEMU and all gdb has to do is printf("%lx") and that is it. >>> >>> OK, but something is not clear to me. Even if GDB just printf("%lx") the >>> value, we would still have to bswap when the host is LE, right? >> >> Not for %lx, this should just print a correct value. >> >>> QEMU BE: >>> (gdb) x/8xb >spr[287] >>> 0x11391760: 0x000x000x000x000x000x4e0x120x00 >>> >>> QEMU LE: >>> (gdb) x/8xb >spr[287] >>> 0x75bd98c0: 0x010x020x4b0x000x000x000x00 >>> 0x00 >>> The problem is that we want to pass it as a byte stream from the gdb_read_register() hook all the way to gdb and for some reason gdb does not define endianness of that stream but rather tries guessing the endianness which is broken. >>> >>> GDB does define the endianness of the stream (in a way): >>> >>> "Each byte of register data is described by two hex digits. The bytes >>> with the register are transmitted in target byte order." >>> >>> https://sourceware.org/gdb/current/onlinedocs/gdb/Packets.html#Packets >> >> >> Target byte order changes all the time so we need a endian-agnostic way >> of knowing the current endianness. >> >> >>> Today I was debugging rtas/clientinterface calls which a little endian kernel makes and these calls need to switch to the big endian first. And gdb goes nuts when this switch happens (note that I did not give an ELF to gdb this time so it picked LE itself). Even if it could fetch the endianness from QEMU, it would fail as it is an LE bit in MSR which is a register which is parsed according to the gdb's idea of endianness :) >>> >>> I think it would be possible to define another packet for the remote >>> protocol that informs the endianness explicitly at each time the guest >>> stops. If you provide more info on how to reproduce that issue I could >>> put in my list or go bug GDB folks about it. >>> > It will > always check the target endianness before printing a value, even if it > refers to a register: > > https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/findvar.c;hb=HEAD#l49 > > So in our case the contents of mem_buf need to match both the guest > endianness *and* what GDB has set for 'show endian' because it will > detect it automatically from the ELF. If it guesses incorrectly because > there is no ELF, we need to use the 'set endian' command. > > By the way, this is already the behavior for the registers that are > already implemented (e.g. $msr). Here's the commit that introduced > that: > > https://git.qemu.org/?p=qemu.git;a=commitdiff;h=8a286ce4502356ce0b97a2424a2cb7 > > Now, what might be a source of confusion here is the fact that we > *always* do a bswap when the host is LE because QEMU thinks that the ppc > guest is always BE. That requires the maybe_bswap function to make > things right in the end. > > What I could do is try to improve this by only swapping when the > guest's actual endianness (msr_le) is different from the host's. The bytestream for registers should have fixed endianness. But looking at the gdb code makes me think it is not going to happen :( >>> >>> Yes, I can't think of a way to fix that without changing the way GDB >>> exhibits the values or the remote protocol. >>> >>> May I proceed with this series as it is with the bswaps? >>> > That > is not entirely within the scope of this patch, though. True. But since you are touching this, may be you could fix gdb too :) Does gdb tell QEMU about what endianness it thinks that QEMU is using? Or can it read it from QEMU? I cannot easily spot this in QEMU... >>> >>> GDB currently does not tell and does not ask about endianness. So I >>> believe there is room for improvement here. I could not find it in the >>> documentation but I think that GDB supports file transfers and it asks >>> for the ELF in some scenarios. This approach could be one way of >>> informing it about the endianness, although it has its own shortcomings. >> >> >> There is no ELF in my scenario really. What I did was: >> >> 1. hack
Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 2/3] target/ppc: Add GDB callbacks for SPRs
Alexey Kardashevskiy writes: > On 01/02/2019 08:57, Fabiano Rosas wrote: >> Alexey Kardashevskiy writes: >> >>> On 31/01/2019 03:30, Fabiano Rosas wrote: Alexey Kardashevskiy writes: > > but this is a register which does not have endianness, the endianness > appears here because the interface between gdb and qemu is > uint8_t*==bytestream but this interface should have fixed endianness > imho (now it is bigendian afaict). > > Something is not right here... Having a fixed endianness would not work because GDB have no way of knowing how to represent what comes from the remote end. >>> >>> It definitely would. A register is stored as "unsigned long" in QEMU and >>> all gdb has to do is printf("%lx") and that is it. >> >> OK, but something is not clear to me. Even if GDB just printf("%lx") the >> value, we would still have to bswap when the host is LE, right? > > Not for %lx, this should just print a correct value. > >> QEMU BE: >> (gdb) x/8xb >spr[287] >> 0x11391760: 0x000x000x000x000x000x4e0x120x00 >> >> QEMU LE: >> (gdb) x/8xb >spr[287] >> 0x75bd98c0: 0x010x020x4b0x000x000x000x000x00 >> >>> The problem is that >>> we want to pass it as a byte stream from the gdb_read_register() hook >>> all the way to gdb and for some reason gdb does not define endianness of >>> that stream but rather tries guessing the endianness which is broken. >> >> GDB does define the endianness of the stream (in a way): >> >> "Each byte of register data is described by two hex digits. The bytes >> with the register are transmitted in target byte order." >> >> https://sourceware.org/gdb/current/onlinedocs/gdb/Packets.html#Packets > > > Target byte order changes all the time so we need a endian-agnostic way > of knowing the current endianness. > > >> >>> Today I was debugging rtas/clientinterface calls which a little endian >>> kernel makes and these calls need to switch to the big endian first. And >>> gdb goes nuts when this switch happens (note that I did not give an ELF >>> to gdb this time so it picked LE itself). Even if it could fetch the >>> endianness from QEMU, it would fail as it is an LE bit in MSR which is a >>> register which is parsed according to the gdb's idea of endianness :) >> >> I think it would be possible to define another packet for the remote >> protocol that informs the endianness explicitly at each time the guest >> stops. If you provide more info on how to reproduce that issue I could >> put in my list or go bug GDB folks about it. >> It will always check the target endianness before printing a value, even if it refers to a register: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/findvar.c;hb=HEAD#l49 So in our case the contents of mem_buf need to match both the guest endianness *and* what GDB has set for 'show endian' because it will detect it automatically from the ELF. If it guesses incorrectly because there is no ELF, we need to use the 'set endian' command. By the way, this is already the behavior for the registers that are already implemented (e.g. $msr). Here's the commit that introduced that: https://git.qemu.org/?p=qemu.git;a=commitdiff;h=8a286ce4502356ce0b97a2424a2cb7 Now, what might be a source of confusion here is the fact that we *always* do a bswap when the host is LE because QEMU thinks that the ppc guest is always BE. That requires the maybe_bswap function to make things right in the end. What I could do is try to improve this by only swapping when the guest's actual endianness (msr_le) is different from the host's. >>> >>> The bytestream for registers should have fixed endianness. But looking >>> at the gdb code makes me think it is not going to happen :( >> >> Yes, I can't think of a way to fix that without changing the way GDB >> exhibits the values or the remote protocol. >> >> May I proceed with this series as it is with the bswaps? >> That is not entirely within the scope of this patch, though. >>> >>> True. But since you are touching this, may be you could fix gdb too :) >>> >>> Does gdb tell QEMU about what endianness it thinks that QEMU is using? >>> Or can it read it from QEMU? I cannot easily spot this in QEMU... >> >> GDB currently does not tell and does not ask about endianness. So I >> believe there is room for improvement here. I could not find it in the >> documentation but I think that GDB supports file transfers and it asks >> for the ELF in some scenarios. This approach could be one way of >> informing it about the endianness, although it has its own shortcomings. > > > There is no ELF in my scenario really. What I did was: > > 1. hack qemu to not load slof.bin but load vmlinux instead and changed > starting pc to where I loaded the kernel (I did not have to but it is a > lot easier to
Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 2/3] target/ppc: Add GDB callbacks for SPRs
On 01/02/2019 08:57, Fabiano Rosas wrote: > Alexey Kardashevskiy writes: > >> On 31/01/2019 03:30, Fabiano Rosas wrote: >>> Alexey Kardashevskiy writes: >>> but this is a register which does not have endianness, the endianness appears here because the interface between gdb and qemu is uint8_t*==bytestream but this interface should have fixed endianness imho (now it is bigendian afaict). Something is not right here... >>> >>> Having a fixed endianness would not work because GDB have no way of >>> knowing how to represent what comes from the remote end. >> >> It definitely would. A register is stored as "unsigned long" in QEMU and >> all gdb has to do is printf("%lx") and that is it. > > OK, but something is not clear to me. Even if GDB just printf("%lx") the > value, we would still have to bswap when the host is LE, right? Not for %lx, this should just print a correct value. > QEMU BE: > (gdb) x/8xb >spr[287] > 0x11391760: 0x000x000x000x000x000x4e0x120x00 > > QEMU LE: > (gdb) x/8xb >spr[287] > 0x75bd98c0: 0x010x020x4b0x000x000x000x000x00 > >> The problem is that >> we want to pass it as a byte stream from the gdb_read_register() hook >> all the way to gdb and for some reason gdb does not define endianness of >> that stream but rather tries guessing the endianness which is broken. > > GDB does define the endianness of the stream (in a way): > > "Each byte of register data is described by two hex digits. The bytes > with the register are transmitted in target byte order." > > https://sourceware.org/gdb/current/onlinedocs/gdb/Packets.html#Packets Target byte order changes all the time so we need a endian-agnostic way of knowing the current endianness. > >> Today I was debugging rtas/clientinterface calls which a little endian >> kernel makes and these calls need to switch to the big endian first. And >> gdb goes nuts when this switch happens (note that I did not give an ELF >> to gdb this time so it picked LE itself). Even if it could fetch the >> endianness from QEMU, it would fail as it is an LE bit in MSR which is a >> register which is parsed according to the gdb's idea of endianness :) > > I think it would be possible to define another packet for the remote > protocol that informs the endianness explicitly at each time the guest > stops. If you provide more info on how to reproduce that issue I could > put in my list or go bug GDB folks about it. > >>> It will >>> always check the target endianness before printing a value, even if it >>> refers to a register: >>> >>> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/findvar.c;hb=HEAD#l49 >>> >>> So in our case the contents of mem_buf need to match both the guest >>> endianness *and* what GDB has set for 'show endian' because it will >>> detect it automatically from the ELF. If it guesses incorrectly because >>> there is no ELF, we need to use the 'set endian' command. >>> >>> By the way, this is already the behavior for the registers that are >>> already implemented (e.g. $msr). Here's the commit that introduced >>> that: >>> >>> https://git.qemu.org/?p=qemu.git;a=commitdiff;h=8a286ce4502356ce0b97a2424a2cb7 >>> >>> Now, what might be a source of confusion here is the fact that we >>> *always* do a bswap when the host is LE because QEMU thinks that the ppc >>> guest is always BE. That requires the maybe_bswap function to make >>> things right in the end. >>> >>> What I could do is try to improve this by only swapping when the >>> guest's actual endianness (msr_le) is different from the host's. >> >> The bytestream for registers should have fixed endianness. But looking >> at the gdb code makes me think it is not going to happen :( > > Yes, I can't think of a way to fix that without changing the way GDB > exhibits the values or the remote protocol. > > May I proceed with this series as it is with the bswaps? > >>> That >>> is not entirely within the scope of this patch, though. >> >> True. But since you are touching this, may be you could fix gdb too :) >> >> Does gdb tell QEMU about what endianness it thinks that QEMU is using? >> Or can it read it from QEMU? I cannot easily spot this in QEMU... > > GDB currently does not tell and does not ask about endianness. So I > believe there is room for improvement here. I could not find it in the > documentation but I think that GDB supports file transfers and it asks > for the ELF in some scenarios. This approach could be one way of > informing it about the endianness, although it has its own shortcomings. There is no ELF in my scenario really. What I did was: 1. hack qemu to not load slof.bin but load vmlinux instead and changed starting pc to where I loaded the kernel (I did not have to but it is a lot easier to ditch slof and set breakpoint in gdb before starting the guest). 2. start a guest, connect external gdb and observe garbage in $pc and disassembly until
Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 2/3] target/ppc: Add GDB callbacks for SPRs
Alexey Kardashevskiy writes: > On 31/01/2019 03:30, Fabiano Rosas wrote: >> Alexey Kardashevskiy writes: >> >>> >>> but this is a register which does not have endianness, the endianness >>> appears here because the interface between gdb and qemu is >>> uint8_t*==bytestream but this interface should have fixed endianness >>> imho (now it is bigendian afaict). >>> >>> Something is not right here... >> >> Having a fixed endianness would not work because GDB have no way of >> knowing how to represent what comes from the remote end. > > It definitely would. A register is stored as "unsigned long" in QEMU and > all gdb has to do is printf("%lx") and that is it. OK, but something is not clear to me. Even if GDB just printf("%lx") the value, we would still have to bswap when the host is LE, right? QEMU BE: (gdb) x/8xb >spr[287] 0x11391760: 0x000x000x000x000x000x4e0x120x00 QEMU LE: (gdb) x/8xb >spr[287] 0x75bd98c0: 0x010x020x4b0x000x000x000x000x00 > The problem is that > we want to pass it as a byte stream from the gdb_read_register() hook > all the way to gdb and for some reason gdb does not define endianness of > that stream but rather tries guessing the endianness which is broken. GDB does define the endianness of the stream (in a way): "Each byte of register data is described by two hex digits. The bytes with the register are transmitted in target byte order." https://sourceware.org/gdb/current/onlinedocs/gdb/Packets.html#Packets > Today I was debugging rtas/clientinterface calls which a little endian > kernel makes and these calls need to switch to the big endian first. And > gdb goes nuts when this switch happens (note that I did not give an ELF > to gdb this time so it picked LE itself). Even if it could fetch the > endianness from QEMU, it would fail as it is an LE bit in MSR which is a > register which is parsed according to the gdb's idea of endianness :) I think it would be possible to define another packet for the remote protocol that informs the endianness explicitly at each time the guest stops. If you provide more info on how to reproduce that issue I could put in my list or go bug GDB folks about it. >> It will >> always check the target endianness before printing a value, even if it >> refers to a register: >> >> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/findvar.c;hb=HEAD#l49 >> >> So in our case the contents of mem_buf need to match both the guest >> endianness *and* what GDB has set for 'show endian' because it will >> detect it automatically from the ELF. If it guesses incorrectly because >> there is no ELF, we need to use the 'set endian' command. >> >> By the way, this is already the behavior for the registers that are >> already implemented (e.g. $msr). Here's the commit that introduced >> that: >> >> https://git.qemu.org/?p=qemu.git;a=commitdiff;h=8a286ce4502356ce0b97a2424a2cb7 >> >> Now, what might be a source of confusion here is the fact that we >> *always* do a bswap when the host is LE because QEMU thinks that the ppc >> guest is always BE. That requires the maybe_bswap function to make >> things right in the end. >> >> What I could do is try to improve this by only swapping when the >> guest's actual endianness (msr_le) is different from the host's. > > The bytestream for registers should have fixed endianness. But looking > at the gdb code makes me think it is not going to happen :( Yes, I can't think of a way to fix that without changing the way GDB exhibits the values or the remote protocol. May I proceed with this series as it is with the bswaps? >> That >> is not entirely within the scope of this patch, though. > > True. But since you are touching this, may be you could fix gdb too :) > > Does gdb tell QEMU about what endianness it thinks that QEMU is using? > Or can it read it from QEMU? I cannot easily spot this in QEMU... GDB currently does not tell and does not ask about endianness. So I believe there is room for improvement here. I could not find it in the documentation but I think that GDB supports file transfers and it asks for the ELF in some scenarios. This approach could be one way of informing it about the endianness, although it has its own shortcomings.
Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 2/3] target/ppc: Add GDB callbacks for SPRs
On 31/01/2019 03:30, Fabiano Rosas wrote: > Alexey Kardashevskiy writes: > >> >> but this is a register which does not have endianness, the endianness >> appears here because the interface between gdb and qemu is >> uint8_t*==bytestream but this interface should have fixed endianness >> imho (now it is bigendian afaict). >> >> Something is not right here... > > Having a fixed endianness would not work because GDB have no way of > knowing how to represent what comes from the remote end. It definitely would. A register is stored as "unsigned long" in QEMU and all gdb has to do is printf("%lx") and that is it. The problem is that we want to pass it as a byte stream from the gdb_read_register() hook all the way to gdb and for some reason gdb does not define endianness of that stream but rather tries guessing the endianness which is broken. Today I was debugging rtas/clientinterface calls which a little endian kernel makes and these calls need to switch to the big endian first. And gdb goes nuts when this switch happens (note that I did not give an ELF to gdb this time so it picked LE itself). Even if it could fetch the endianness from QEMU, it would fail as it is an LE bit in MSR which is a register which is parsed according to the gdb's idea of endianness :) > It will > always check the target endianness before printing a value, even if it > refers to a register: > > https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/findvar.c;hb=HEAD#l49 > > So in our case the contents of mem_buf need to match both the guest > endianness *and* what GDB has set for 'show endian' because it will > detect it automatically from the ELF. If it guesses incorrectly because > there is no ELF, we need to use the 'set endian' command. > > By the way, this is already the behavior for the registers that are > already implemented (e.g. $msr). Here's the commit that introduced > that: > > https://git.qemu.org/?p=qemu.git;a=commitdiff;h=8a286ce4502356ce0b97a2424a2cb7 > > Now, what might be a source of confusion here is the fact that we > *always* do a bswap when the host is LE because QEMU thinks that the ppc > guest is always BE. That requires the maybe_bswap function to make > things right in the end. > > What I could do is try to improve this by only swapping when the > guest's actual endianness (msr_le) is different from the host's. The bytestream for registers should have fixed endianness. But looking at the gdb code makes me think it is not going to happen :( > That > is not entirely within the scope of this patch, though. True. But since you are touching this, may be you could fix gdb too :) Does gdb tell QEMU about what endianness it thinks that QEMU is using? Or can it read it from QEMU? I cannot easily spot this in QEMU... -- Alexey
Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 2/3] target/ppc: Add GDB callbacks for SPRs
Alexey Kardashevskiy writes: > > but this is a register which does not have endianness, the endianness > appears here because the interface between gdb and qemu is > uint8_t*==bytestream but this interface should have fixed endianness > imho (now it is bigendian afaict). > > Something is not right here... Having a fixed endianness would not work because GDB have no way of knowing how to represent what comes from the remote end. It will always check the target endianness before printing a value, even if it refers to a register: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/findvar.c;hb=HEAD#l49 So in our case the contents of mem_buf need to match both the guest endianness *and* what GDB has set for 'show endian' because it will detect it automatically from the ELF. If it guesses incorrectly because there is no ELF, we need to use the 'set endian' command. By the way, this is already the behavior for the registers that are already implemented (e.g. $msr). Here's the commit that introduced that: https://git.qemu.org/?p=qemu.git;a=commitdiff;h=8a286ce4502356ce0b97a2424a2cb7 Now, what might be a source of confusion here is the fact that we *always* do a bswap when the host is LE because QEMU thinks that the ppc guest is always BE. That requires the maybe_bswap function to make things right in the end. What I could do is try to improve this by only swapping when the guest's actual endianness (msr_le) is different from the host's. That is not entirely within the scope of this patch, though. Cheers
Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 2/3] target/ppc: Add GDB callbacks for SPRs
On 29/01/2019 07:00, Fabiano Rosas wrote: > David Gibson writes: > >> On Thu, Jan 24, 2019 at 06:20:02PM +1100, Alexey Kardashevskiy wrote: >>> >>> >>> On 23/01/2019 04:01, Fabiano Rosas wrote: These will be used to let GDB know about PPC's Special Purpose Registers (SPR). They take an index based on the order the registers appear in the XML file sent by QEMU to GDB. This index does not match the actual location of the registers in the env->spr array so the gdb_find_spr_idx function does that conversion. Signed-off-by: Fabiano Rosas --- target/ppc/translate_init.inc.c | 54 - 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c index 710064a25d..f29ac3558a 100644 --- a/target/ppc/translate_init.inc.c +++ b/target/ppc/translate_init.inc.c @@ -9487,6 +9487,55 @@ static bool avr_need_swap(CPUPPCState *env) #endif } +#if !defined(CONFIG_USER_ONLY) +static int gdb_find_spr_idx(CPUPPCState *env, int n) +{ +int i; + +for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) { +ppc_spr_t *spr = >spr_cb[i]; + +if (spr->name && spr->gdb_id == n) { +return i; +} +} +return -1; +} + +static int gdb_get_spr_reg(CPUPPCState *env, uint8_t *mem_buf, int n) +{ +int reg; +int len; + +reg = gdb_find_spr_idx(env, n); +if (reg < 0) { +return 0; +} + +len = TARGET_LONG_SIZE; +stn_p(mem_buf, len, env->spr[reg]); +ppc_maybe_bswap_register(env, mem_buf, len); >>> >>> >>> I am confused by this as it produces different results depending on the >>> guest mode: >> >> >> Hm, yeah, I thought the bswap here looked odd, but it wasn't obvious >> to me if it was bogus here, or just a bogus gdb interface we have to >> work around. >> >>> (gdb) p $pvr >>> $1 = 0x14c00 >>> (gdb) c >>> Continuing. >>> Program received signal SIGINT, Interrupt. >>> (gdb) p $pvr >>> $2 = 0x4c0100 >> >> But that behaviour definitely looks wrong. > > GDB detects the endianness by looking at the ELF headers: but this is a register which does not have endianness, the endianness appears here because the interface between gdb and qemu is uint8_t*==bytestream but this interface should have fixed endianness imho (now it is bigendian afaict). Something is not right here... > > (gdb) p/x $pvr > $1 = 0x1024b00 > (gdb) file ~/qemu/roms/SLOF/board-qemu/llfw/stage1.elf > Reading symbols from ~/qemu/roms/SLOF/board-qemu/llfw/stage1.elf...done. > (gdb) show endian > The target endianness is set automatically (currently big endian) > (gdb) p/x $pvr > $2 = 0x4b0201 > (gdb) c > Continuing. > > (gdb) ^C > Program received signal SIGINT, Interrupt. > 0x74a70cc0 in ?? () > (gdb) file vmlinux > Reading symbols from vmlinux...done. > (gdb) show endian > The target endianness is set automatically (currently little endian) > (gdb) p/x $pvr > $3 = 0x4b0201 > > The maybe_bswap_register is done due to QEMU having TARGET_WORDS_BIGENDIAN set > even after we have changed into LE mode. > >>> First print is when I stopped the guest in the SLOF firmware (so it is >>> big-endian) and then I continued and stopped gdb when the guest booted a >>> little-endian system; the KVM host is little endian, the machine running >>> gdb is LE too. >>> >>> QEMU monitor prints the same 0x4c0100 in both cases. >>> >>> I am adding the inventor of maybe_bswap_register() in cc: for >>> assistance. Swapping happens: >>> - once for BE: after stn_p() >>> *(unsigned long *)mem_buf is 0x14c00 >>> - twice for LE. >>> >>> >>> >>> >>> +return len; +} + +static int gdb_set_spr_reg(CPUPPCState *env, uint8_t *mem_buf, int n) +{ +int reg; +int len; + +reg = gdb_find_spr_idx(env, n); +if (reg < 0) { +return 0; +} + +len = TARGET_LONG_SIZE; +ppc_maybe_bswap_register(env, mem_buf, len); +env->spr[reg] = ldn_p(mem_buf, len); + +return len; +} +#endif + static int gdb_get_float_reg(CPUPPCState *env, uint8_t *mem_buf, int n) { if (n < 32) { @@ -9716,7 +9765,10 @@ static void ppc_cpu_realize(DeviceState *dev, Error **errp) gdb_register_coprocessor(cs, gdb_get_vsx_reg, gdb_set_vsx_reg, 32, "power-vsx.xml", 0); } - +#ifndef CONFIG_USER_ONLY +gdb_register_coprocessor(cs, gdb_get_spr_reg, gdb_set_spr_reg, + pcc->gdb_num_sprs, "power-spr.xml", 0); +#endif qemu_init_vcpu(cs); pcc->parent_realize(dev, errp); >>> > -- Alexey
Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 2/3] target/ppc: Add GDB callbacks for SPRs
David Gibson writes: > On Thu, Jan 24, 2019 at 06:20:02PM +1100, Alexey Kardashevskiy wrote: >> >> >> On 23/01/2019 04:01, Fabiano Rosas wrote: >> > These will be used to let GDB know about PPC's Special Purpose >> > Registers (SPR). >> > >> > They take an index based on the order the registers appear in the XML >> > file sent by QEMU to GDB. This index does not match the actual >> > location of the registers in the env->spr array so the >> > gdb_find_spr_idx function does that conversion. >> > >> > Signed-off-by: Fabiano Rosas >> > --- >> > target/ppc/translate_init.inc.c | 54 - >> > 1 file changed, 53 insertions(+), 1 deletion(-) >> > >> > diff --git a/target/ppc/translate_init.inc.c >> > b/target/ppc/translate_init.inc.c >> > index 710064a25d..f29ac3558a 100644 >> > --- a/target/ppc/translate_init.inc.c >> > +++ b/target/ppc/translate_init.inc.c >> > @@ -9487,6 +9487,55 @@ static bool avr_need_swap(CPUPPCState *env) >> > #endif >> > } >> > >> > +#if !defined(CONFIG_USER_ONLY) >> > +static int gdb_find_spr_idx(CPUPPCState *env, int n) >> > +{ >> > +int i; >> > + >> > +for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) { >> > +ppc_spr_t *spr = >spr_cb[i]; >> > + >> > +if (spr->name && spr->gdb_id == n) { >> > +return i; >> > +} >> > +} >> > +return -1; >> > +} >> > + >> > +static int gdb_get_spr_reg(CPUPPCState *env, uint8_t *mem_buf, int n) >> > +{ >> > +int reg; >> > +int len; >> > + >> > +reg = gdb_find_spr_idx(env, n); >> > +if (reg < 0) { >> > +return 0; >> > +} >> > + >> > +len = TARGET_LONG_SIZE; >> > +stn_p(mem_buf, len, env->spr[reg]); >> > +ppc_maybe_bswap_register(env, mem_buf, len); >> >> >> I am confused by this as it produces different results depending on the >> guest mode: > > > Hm, yeah, I thought the bswap here looked odd, but it wasn't obvious > to me if it was bogus here, or just a bogus gdb interface we have to > work around. > >> (gdb) p $pvr >> $1 = 0x14c00 >> (gdb) c >> Continuing. >> Program received signal SIGINT, Interrupt. >> (gdb) p $pvr >> $2 = 0x4c0100 > > But that behaviour definitely looks wrong. GDB detects the endianness by looking at the ELF headers: (gdb) p/x $pvr $1 = 0x1024b00 (gdb) file ~/qemu/roms/SLOF/board-qemu/llfw/stage1.elf Reading symbols from ~/qemu/roms/SLOF/board-qemu/llfw/stage1.elf...done. (gdb) show endian The target endianness is set automatically (currently big endian) (gdb) p/x $pvr $2 = 0x4b0201 (gdb) c Continuing. (gdb) ^C Program received signal SIGINT, Interrupt. 0x74a70cc0 in ?? () (gdb) file vmlinux Reading symbols from vmlinux...done. (gdb) show endian The target endianness is set automatically (currently little endian) (gdb) p/x $pvr $3 = 0x4b0201 The maybe_bswap_register is done due to QEMU having TARGET_WORDS_BIGENDIAN set even after we have changed into LE mode. >> First print is when I stopped the guest in the SLOF firmware (so it is >> big-endian) and then I continued and stopped gdb when the guest booted a >> little-endian system; the KVM host is little endian, the machine running >> gdb is LE too. >> >> QEMU monitor prints the same 0x4c0100 in both cases. >> >> I am adding the inventor of maybe_bswap_register() in cc: for >> assistance. Swapping happens: >> - once for BE: after stn_p() >> *(unsigned long *)mem_buf is 0x14c00 >> - twice for LE. >> >> >> >> >> >> > +return len; >> > +} >> > + >> > +static int gdb_set_spr_reg(CPUPPCState *env, uint8_t *mem_buf, int n) >> > +{ >> > +int reg; >> > +int len; >> > + >> > +reg = gdb_find_spr_idx(env, n); >> > +if (reg < 0) { >> > +return 0; >> > +} >> > + >> > +len = TARGET_LONG_SIZE; >> > +ppc_maybe_bswap_register(env, mem_buf, len); >> > +env->spr[reg] = ldn_p(mem_buf, len); >> > + >> > +return len; >> > +} >> > +#endif >> > + >> > static int gdb_get_float_reg(CPUPPCState *env, uint8_t *mem_buf, int n) >> > { >> > if (n < 32) { >> > @@ -9716,7 +9765,10 @@ static void ppc_cpu_realize(DeviceState *dev, Error >> > **errp) >> > gdb_register_coprocessor(cs, gdb_get_vsx_reg, gdb_set_vsx_reg, >> > 32, "power-vsx.xml", 0); >> > } >> > - >> > +#ifndef CONFIG_USER_ONLY >> > +gdb_register_coprocessor(cs, gdb_get_spr_reg, gdb_set_spr_reg, >> > + pcc->gdb_num_sprs, "power-spr.xml", 0); >> > +#endif >> > qemu_init_vcpu(cs); >> > >> > pcc->parent_realize(dev, errp); >> > >>
Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 2/3] target/ppc: Add GDB callbacks for SPRs
On Thu, Jan 24, 2019 at 06:20:02PM +1100, Alexey Kardashevskiy wrote: > > > On 23/01/2019 04:01, Fabiano Rosas wrote: > > These will be used to let GDB know about PPC's Special Purpose > > Registers (SPR). > > > > They take an index based on the order the registers appear in the XML > > file sent by QEMU to GDB. This index does not match the actual > > location of the registers in the env->spr array so the > > gdb_find_spr_idx function does that conversion. > > > > Signed-off-by: Fabiano Rosas > > --- > > target/ppc/translate_init.inc.c | 54 - > > 1 file changed, 53 insertions(+), 1 deletion(-) > > > > diff --git a/target/ppc/translate_init.inc.c > > b/target/ppc/translate_init.inc.c > > index 710064a25d..f29ac3558a 100644 > > --- a/target/ppc/translate_init.inc.c > > +++ b/target/ppc/translate_init.inc.c > > @@ -9487,6 +9487,55 @@ static bool avr_need_swap(CPUPPCState *env) > > #endif > > } > > > > +#if !defined(CONFIG_USER_ONLY) > > +static int gdb_find_spr_idx(CPUPPCState *env, int n) > > +{ > > +int i; > > + > > +for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) { > > +ppc_spr_t *spr = >spr_cb[i]; > > + > > +if (spr->name && spr->gdb_id == n) { > > +return i; > > +} > > +} > > +return -1; > > +} > > + > > +static int gdb_get_spr_reg(CPUPPCState *env, uint8_t *mem_buf, int n) > > +{ > > +int reg; > > +int len; > > + > > +reg = gdb_find_spr_idx(env, n); > > +if (reg < 0) { > > +return 0; > > +} > > + > > +len = TARGET_LONG_SIZE; > > +stn_p(mem_buf, len, env->spr[reg]); > > +ppc_maybe_bswap_register(env, mem_buf, len); > > > I am confused by this as it produces different results depending on the > guest mode: Hm, yeah, I thought the bswap here looked odd, but it wasn't obvious to me if it was bogus here, or just a bogus gdb interface we have to work around. > (gdb) p $pvr > $1 = 0x14c00 > (gdb) c > Continuing. > Program received signal SIGINT, Interrupt. > (gdb) p $pvr > $2 = 0x4c0100 But that behaviour definitely looks wrong. > First print is when I stopped the guest in the SLOF firmware (so it is > big-endian) and then I continued and stopped gdb when the guest booted a > little-endian system; the KVM host is little endian, the machine running > gdb is LE too. > > QEMU monitor prints the same 0x4c0100 in both cases. > > I am adding the inventor of maybe_bswap_register() in cc: for > assistance. Swapping happens: > - once for BE: after stn_p() > *(unsigned long *)mem_buf is 0x14c00 > - twice for LE. > > > > > > > +return len; > > +} > > + > > +static int gdb_set_spr_reg(CPUPPCState *env, uint8_t *mem_buf, int n) > > +{ > > +int reg; > > +int len; > > + > > +reg = gdb_find_spr_idx(env, n); > > +if (reg < 0) { > > +return 0; > > +} > > + > > +len = TARGET_LONG_SIZE; > > +ppc_maybe_bswap_register(env, mem_buf, len); > > +env->spr[reg] = ldn_p(mem_buf, len); > > + > > +return len; > > +} > > +#endif > > + > > static int gdb_get_float_reg(CPUPPCState *env, uint8_t *mem_buf, int n) > > { > > if (n < 32) { > > @@ -9716,7 +9765,10 @@ static void ppc_cpu_realize(DeviceState *dev, Error > > **errp) > > gdb_register_coprocessor(cs, gdb_get_vsx_reg, gdb_set_vsx_reg, > > 32, "power-vsx.xml", 0); > > } > > - > > +#ifndef CONFIG_USER_ONLY > > +gdb_register_coprocessor(cs, gdb_get_spr_reg, gdb_set_spr_reg, > > + pcc->gdb_num_sprs, "power-spr.xml", 0); > > +#endif > > qemu_init_vcpu(cs); > > > > pcc->parent_realize(dev, errp); > > > -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 2/3] target/ppc: Add GDB callbacks for SPRs
On 23/01/2019 04:01, Fabiano Rosas wrote: > These will be used to let GDB know about PPC's Special Purpose > Registers (SPR). > > They take an index based on the order the registers appear in the XML > file sent by QEMU to GDB. This index does not match the actual > location of the registers in the env->spr array so the > gdb_find_spr_idx function does that conversion. > > Signed-off-by: Fabiano Rosas > --- > target/ppc/translate_init.inc.c | 54 - > 1 file changed, 53 insertions(+), 1 deletion(-) > > diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c > index 710064a25d..f29ac3558a 100644 > --- a/target/ppc/translate_init.inc.c > +++ b/target/ppc/translate_init.inc.c > @@ -9487,6 +9487,55 @@ static bool avr_need_swap(CPUPPCState *env) > #endif > } > > +#if !defined(CONFIG_USER_ONLY) > +static int gdb_find_spr_idx(CPUPPCState *env, int n) > +{ > +int i; > + > +for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) { > +ppc_spr_t *spr = >spr_cb[i]; > + > +if (spr->name && spr->gdb_id == n) { > +return i; > +} > +} > +return -1; > +} > + > +static int gdb_get_spr_reg(CPUPPCState *env, uint8_t *mem_buf, int n) > +{ > +int reg; > +int len; > + > +reg = gdb_find_spr_idx(env, n); > +if (reg < 0) { > +return 0; > +} > + > +len = TARGET_LONG_SIZE; > +stn_p(mem_buf, len, env->spr[reg]); > +ppc_maybe_bswap_register(env, mem_buf, len); I am confused by this as it produces different results depending on the guest mode: (gdb) p $pvr $1 = 0x14c00 (gdb) c Continuing. Program received signal SIGINT, Interrupt. (gdb) p $pvr $2 = 0x4c0100 First print is when I stopped the guest in the SLOF firmware (so it is big-endian) and then I continued and stopped gdb when the guest booted a little-endian system; the KVM host is little endian, the machine running gdb is LE too. QEMU monitor prints the same 0x4c0100 in both cases. I am adding the inventor of maybe_bswap_register() in cc: for assistance. Swapping happens: - once for BE: after stn_p() *(unsigned long *)mem_buf is 0x14c00 - twice for LE. > +return len; > +} > + > +static int gdb_set_spr_reg(CPUPPCState *env, uint8_t *mem_buf, int n) > +{ > +int reg; > +int len; > + > +reg = gdb_find_spr_idx(env, n); > +if (reg < 0) { > +return 0; > +} > + > +len = TARGET_LONG_SIZE; > +ppc_maybe_bswap_register(env, mem_buf, len); > +env->spr[reg] = ldn_p(mem_buf, len); > + > +return len; > +} > +#endif > + > static int gdb_get_float_reg(CPUPPCState *env, uint8_t *mem_buf, int n) > { > if (n < 32) { > @@ -9716,7 +9765,10 @@ static void ppc_cpu_realize(DeviceState *dev, Error > **errp) > gdb_register_coprocessor(cs, gdb_get_vsx_reg, gdb_set_vsx_reg, > 32, "power-vsx.xml", 0); > } > - > +#ifndef CONFIG_USER_ONLY > +gdb_register_coprocessor(cs, gdb_get_spr_reg, gdb_set_spr_reg, > + pcc->gdb_num_sprs, "power-spr.xml", 0); > +#endif > qemu_init_vcpu(cs); > > pcc->parent_realize(dev, errp); > -- Alexey