Re: [RFA 3/5] New port: CR16: gdb port

[Pedro Alves <palves@redhat.com>]

On 06/26/2013 08:02 AM, Kaushik Phatak wrote:
> Hi Pedro,
> Thanks for another round of detailed review.
> 
>>> Hmm.  Just to be clear, isn't exposing r0_orig to gDB necessary for
>>> syscall restarting, like orig_eax/orig_rax on x86/x86_64, orig_r3 on ppc,
>>> orig_r2 on s390, etc.?  See e.g., i386_linux_write_pc, amd64_linux_write_pc,
>>> ppc_linux_write_pc, s390_write_pc.
> 
> The original PTRACE implementation disallowed write to orig_r0and1, however read
> was permitted. We can implement this as suggested above, so user may write a -1 to 
> this register to prevent a SIGSEGV or SIGILL similar to amd64_linux_write_pc.
> The signal handler checks for "regs->orig_r0and1 >= 0" before performing a -ERESTARTSYS
> I will add this register to linux-tdep in gdb as well as the gdbserver port, ok?

Sounds good.  You should put that register in a separate target description
feature.  See:

$ find features/ -name "*.xml" | xargs grep orig_
features/s390x-linux64v2.xml:    <reg name="orig_r2" bitsize="64" type="uint64" group="system"/>
features/rs6000/power-linux.xml:  <reg name="orig_r3" bitsize="32" regnum="71"/>
features/rs6000/power64-linux.xml:  <reg name="orig_r3" bitsize="64" regnum="71"/>
features/s390-linux64v2.xml:    <reg name="orig_r2" bitsize="32" type="uint32" group="system"/>
features/s390-linux64.xml:    <reg name="orig_r2" bitsize="32" type="uint32" group="system"/>
features/s390-linux64v1.xml:    <reg name="orig_r2" bitsize="32" type="uint32" group="system"/>
features/s390-linux32v2.xml:    <reg name="orig_r2" bitsize="32" type="uint32" group="system"/>
features/s390x-linux64v1.xml:    <reg name="orig_r2" bitsize="64" type="uint64" group="system"/>
features/i386/32bit-linux.xml:  <reg name="orig_eax" bitsize="32" type="int" regnum="41"/>
features/i386/64bit-linux.xml:  <reg name="orig_rax" bitsize="64" type="int" regnum="57"/>
features/s390-linux32.xml:    <reg name="orig_r2" bitsize="32" type="uint32" group="system"/>
features/s390-linux32v1.xml:    <reg name="orig_r2" bitsize="32" type="uint32" group="system"/>
features/s390x-linux64.xml:    <reg name="orig_r2" bitsize="64" type="uint64" group="system"/>

And:

$ grep tdesc_find_feature * | grep linux
amd64-linux-tdep.c:  feature = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.linux");
amd64-linux-tdep.c:  feature = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.linux");
i386-linux-tdep.c:  feature = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.linux");
i386-linux-tdep.c:  if (tdesc_find_feature (tdesc, "org.gnu.gdb.i386.avx"))
i386-linux-tdep.c:  else if (tdesc_find_feature (tdesc, "org.gnu.gdb.i386.sse"))
mips-linux-tdep.c:      feature = tdesc_find_feature (info.target_desc,
ppc-linux-tdep.c:      if (tdesc_find_feature (info.target_desc,
ppc-linux-tdep.c:      else if (tdesc_find_feature (info.target_desc,
ppc-linux-tdep.c:      if (tdesc_find_feature (info.target_desc,
ppc-linux-tdep.c:      else if (tdesc_find_feature (info.target_desc,
ppc-linux-tdep.c:      feature = tdesc_find_feature (info.target_desc,
s390-tdep.c:      feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.linux");

> 
>>> Are these always present in all versions of CR16 silicon?  IOW, are
>>> we safe with adding them to the core register set (*)?  
>>> (*) which registers are those btw?  I'm not that familiar with CR16.  :-)
> The following 5 registers have been added to this patch, which are debug registers,
> "dbs","dcrl","dsr","car0","car1"
> These registers are not part of every silicon and can be an optional feature.
> However, the current simulator port seems to support these by default.

Sure, but remote debugging such part with e.g., jtag should still be
supported.

> 
>>> you should really split them to a separate target description feature.
> Is there any other port i can refer for this?  

Look for tdesc_find_feature.  Maybe tic6x_gdbarch_init would be a simple
enough reference.

> I will run through my code again and work on the other formatting related comments
> provided.

Thanks.

-- 
Pedro Alves