RISC-V: Is it reasonable to extend current target_description for KGDB?

RISC-V: Is it reasonable to extend current target_description for KGDB?

[Vincent Chen]

Hi,
   I am a Linux developer and trying to add the KGDB support to the
RISC-V Linux. During the porting, I encountered a GDB issue with the
'g' packet. I hope the GDB experts can give me some advice.
The KGDB is a debugger provided by the kernel for users to debug
kernel space through GDB. To parse the GDB packets, the KGDB has a
simple gdb stub. However, this gdb stub does not support the
"qSupported" packet. In this case, GDB does not know the register
configuration on the remote target. From the gdb remote debugging
messages, I guess GDB will assume that the remote target only supports
x0~x31 plus pc registers. This will cause GDB not to query the remote
target about the content of the specified register by sending a "p"
packet If the specified register does not belong to the register list
of the "g" packet. In other words, GDB only can get the information of
x0~x31 plus pc registers from KGDB.
       One way to solve this problem may be to add 'qSupported' packet
support to the KGDB's gdb stub. However, I think this may be a bit
inappropriate because the KGDB in RISC-V only reports 36 registers,
namely 32 GPRs, $PC, $sstatus, $sbadaddr and $scause.  Therefore, from
the viewpoint of KGDB, if the register list of the 'p' packet can
include these 36 registers, the complex implementation for the
register query flow will be unnecessary. In fact, I found that other
architectures (such as ARM, ARM64, and PowerPC) make the register list
of the 'p' packet include all reported registers. However, I am not
familiar with GDB, and I don't know if the current GDB implementation
has some limitations or points to note about achieving this feature.
Therefore, I would be grateful if anyone could give me any advice or
would like to implement it.

Thanks

Best regards
Vincent Chen

Re: RISC-V: Is it reasonable to extend current target_description for KGDB?

[Jim Wilson]

On Tue, Oct 15, 2019 at 12:43 AM Vincent Chen <vincent.chen@sifive.com> wrote:
> namely 32 GPRs, $PC, $sstatus, $sbadaddr and $scause.  Therefore, from
> the viewpoint of KGDB, if the register list of the 'p' packet can
> include these 36 registers, the complex implementation for the
> register query flow will be unnecessary.

We don't have a dedicated RISC-V gdb developer.  If you need gdb
patches, you may need to write them yourself.

Excluding riscv linux native, the other important targets (openocd and
qemu) use xml, and would not be affected by changes to the default
register list.  I don't know how this would affect riscv linux native.
Someone would have to try it and see.  But since we use ptrace instead
of remote, I don't think that it would be affected.  We probably have
other unknown targets that might be affected but no way to identify
them.  There are a lot of people using RISC-V, but not very many
contributing back to the FSF sources, or even the RISC-V Foundation
sources.  I think Maciej may be working on the missing RISC-V
gdbserver port, so his work might be affected, but I would expect
gdbserver to use xml also and hence it should be OK.

One possible solution is to add a RISC-V specific command to choose
the register set used for the default p packet.  That way, if there
are problems with adding registers to it, people have the option to
switch back to the old way.  We already have the "set riscv
use-compressed-breakpoints [auto|on|off]" command.  So we could add a
similar command to choose the number of registers in the default p
packet, try changing the default to the full 36 register set, and wait
to see who complains.  People that complain can be told about the
command to switch back to the old 32 (33?) register set.

Jim

Re: RISC-V: Is it reasonable to extend current target_description for KGDB?

[Pedro Alves]

On 10/15/19 8:43 AM, Vincent Chen wrote:
> Hi,
>    I am a Linux developer and trying to add the KGDB support to the
> RISC-V Linux. During the porting, I encountered a GDB issue with the
> 'g' packet. I hope the GDB experts can give me some advice.
> The KGDB is a debugger provided by the kernel for users to debug
> kernel space through GDB. To parse the GDB packets, the KGDB has a
> simple gdb stub. However, this gdb stub does not support the
> "qSupported" packet. 

I don't understand the resistance to just adding the support.
It is not complicated, and would surely end up being used to 
negotiate support for other optional features.

Thanks,
Pedro Alves

Re: RISC-V: Is it reasonable to extend current target_description for KGDB?

[Maciej W. Rozycki]

On Wed, 16 Oct 2019, Pedro Alves wrote:

> >    I am a Linux developer and trying to add the KGDB support to the
> > RISC-V Linux. During the porting, I encountered a GDB issue with the
> > 'g' packet. I hope the GDB experts can give me some advice.
> > The KGDB is a debugger provided by the kernel for users to debug
> > kernel space through GDB. To parse the GDB packets, the KGDB has a
> > simple gdb stub. However, this gdb stub does not support the
> > "qSupported" packet. 
> 
> I don't understand the resistance to just adding the support.
> It is not complicated, and would surely end up being used to 
> negotiate support for other optional features.

 Seconded!  A while ago I actually had a conversation with Jim and other 
RISC-V people as to XML description support in the RISC-V backend and my 
understanding has been we had a consensus to make it mandatory for debug 
stubs and get that enforced on the GDB side, getting rid of any heuristics 
now present for debug stubs with no description support.

 For legacy debug stubs (odd to speak of any in the context of RISC-V, an 
architecture introduced ~10 years after XML description support has been 
added to GDB!) there's always the:

(gdb) set tdesc filename ...

command available to supply XML description information manually.

  Maciej

Re: RISC-V: Is it reasonable to extend current target_description for KGDB?

[Maciej W. Rozycki]

On Tue, 15 Oct 2019, Jim Wilson wrote:

> One possible solution is to add a RISC-V specific command to choose
> the register set used for the default p packet.  That way, if there
> are problems with adding registers to it, people have the option to
> switch back to the old way.  We already have the "set riscv
> use-compressed-breakpoints [auto|on|off]" command.  So we could add a
> similar command to choose the number of registers in the default p
> packet, try changing the default to the full 36 register set, and wait
> to see who complains.  People that complain can be told about the
> command to switch back to the old 32 (33?) register set.

 There's such a generic command already available:

(gdb) set tdesc filename ...

 NB, this has nothing to do with the use of `g'/`G' vs `p'/`P' packets, 
both use the same register indices; for the overlapping part, that is of 
course.  Then no heuristics can cover registers outside the range of the 
`g'/`G' packets of course, an XML description does have to be provided for 
those registers to be recognised.

  Maciej

Re: RISC-V: Is it reasonable to extend current target_description for KGDB?

[Vincent Chen]

On Wed, Oct 16, 2019 at 9:21 PM Maciej W. Rozycki <macro@wdc.com> wrote:
>
> On Tue, 15 Oct 2019, Jim Wilson wrote:
>
> > One possible solution is to add a RISC-V specific command to choose
> > the register set used for the default p packet.  That way, if there
> > are problems with adding registers to it, people have the option to
> > switch back to the old way.  We already have the "set riscv
> > use-compressed-breakpoints [auto|on|off]" command.  So we could add a
> > similar command to choose the number of registers in the default p
> > packet, try changing the default to the full 36 register set, and wait
> > to see who complains.  People that complain can be told about the
> > command to switch back to the old 32 (33?) register set.
>
>  There's such a generic command already available:
>
> (gdb) set tdesc filename ...
>
>  NB, this has nothing to do with the use of `g'/`G' vs `p'/`P' packets,
> both use the same register indices; for the overlapping part, that is of
> course.  Then no heuristics can cover registers outside the range of the
> `g'/`G' packets of course, an XML description does have to be provided for
> those registers to be recognised.
>

I understood. From the perspective of flexibility, compatibility and
user-friendly, I agree that adding the negotiation support to the
kernel is a better way.

Thanks to Jim, Pedro, and Macie for your comments.

Best regards,
Vincent Chen