Re: [PATCH] disable ptrace BookE interface for PowerPC server processors

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Friday, May 25 2012, Edjunior Barbosa Machado wrote:

> On 05/22/2012 02:34 PM, Mark Kettenis wrote:
>
> Thanks for the feedback and sorry for the late reply. I was discussing
> with the kernel developer responsible for the next version of the ptrace
> booke interface and he proposed that the 'features' field from the
> struct ppc_debug_info returned by ptrace PPC_PTRACE_GETHWDBGINFO call
> could be used to check if its availability. According to him, this field
> is currently 0 for servers, but once the new ptrace interface becomes
> functional, it will return the flag PPC_DEBUG_FEATURE_DATA_BP_RANGE
> enabled. Moreover, this change will not affect the behavior on embedded
> processors (which already has PPC_DEBUG_FEATURE_DATA_BP_RANGE and
> PPC_DEBUG_FEATURE_DATA_BP_MASK enabled).

Thanks for the patch.

> Please consider the new version of the patch below.

> gdb/
> 2012-25-05  Edjunior Machado  <emachado@linux.vnet.ibm.com>
>
> 	* ppc-linux-nat.c (have_ptrace_booke_interface): disable ptrace booke
> 	interface for powerpc server processors if not available in
> 	kernel.

I guess you could rewrite this to:

        * ppc-linux-nat.c (have_ptrace_booke_interface): Disable ptrace
          BookE interface for PowerPC server processors if not available
          in the Linux kernel.

> diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
> index 34c40b4..9ffcf88 100644
> --- a/gdb/ppc-linux-nat.c
> +++ b/gdb/ppc-linux-nat.c
> @@ -1421,17 +1421,18 @@ have_ptrace_booke_interface (void)
>        /* Check for kernel support for BOOKE debug registers.  */
>        if (ptrace (PPC_PTRACE_GETHWDBGINFO, tid, 0, &booke_debug_info) >= 0)
>  	{
> -	  have_ptrace_booke_interface = 1;
> -	  max_slots_number = booke_debug_info.num_instruction_bps
> -	    + booke_debug_info.num_data_bps
> -	    + booke_debug_info.num_condition_regs;
> -	}
> -      else
> -	{
> -	  /* Old school interface and no BOOKE debug registers support.  */
> -	  have_ptrace_booke_interface = 0;
> -	  memset (&booke_debug_info, 0, sizeof (struct ppc_debug_info));


> +	  if (booke_debug_info.features)

I'd rather use an explicit check like this:

          if (booke_debug_info.features != 0)

Also, I guess you could put a comment above this check and insert a
brief explanation like you did in the top of this message, WDYT?

From what I remember when I hacked this interface, the patch is good
(and somewhat trivial to me) :-).

Thanks,

-- 
Sergio