On Thu 11 Feb 2010 15:06:34 Ulrich Weigand wrote:
> Joel Brobecker wrote:
> > Still looks good to me :). Just a couple of nits, but otherwise approved.
> > It would still be nice if Ulrich had a chance to take a quick look ;-).
> 
> Looks good to me as well.  The only nit I've found is this:
> 
> @@ -1595,6 +2012,20 @@ ppc_linux_read_description (struct target_ops *ops)
>  	perror_with_name (_("Unable to fetch AltiVec registers"));
>      }
> 
> +  /* Check for kernel support for new BOOKE debugging registers.  */
> +  if (ptrace (PPC_PTRACE_GETHWDBGINFO, tid, 0, &booke_debug_info) >= 0)
> +    {
> +      have_ptrace_new_debug_booke = 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 new BOOKE registers support.  */
> +      have_ptrace_new_debug_booke = 0;
> +      memset (&booke_debug_info, 0, sizeof (struct ppc_debug_info));
> +    }
> +
>    /* Power ISA 2.05 (implemented by Power 6 and newer processors)
>  increases the FPSCR from 32 bits to 64 bits. Even though Power 7 supports
>  this ISA version, it doesn't have PPC_FEATURE_ARCH_2_05 set, only
> 
> 
> Setting global variables like have_ptrace_new_debug_booke as an implicit
> side effect of ppc_linux_read_description isn't really nice; it makes
> assumptions about the sequence in which these routines are called that
> may not be guaranteed after potential future changes ...
> 
> Why not use a routine to retrieve the info that is called whereever
> necessary (the routine can internally cache the data and the fact
> that it useless to try as kernel support is missing)?

Good point. This version adds a have_ptrace_booke_interface function that
is called whenever we need to check for support of the booke ptrace
interface.

This patch was approved before the 7.1 branch was created, but I didn't
commit it then because the kernel support wasn't committed upstream yet.
It is in the linux-next tree as of last friday, and will appear in 2.6.34.
Can I commit this patch both for the 7.1 branch and HEAD?

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


2010-02-22  Sergio Durigan Junior  <sergiodj@linux.vnet.ibm.com>
	    Thiago Jung Bauermann  <bauerman@br.ibm.com>

	* ppc-linux-nat.c (PTRACE_GET_DEBUGREG): Update comment.
	(PPC_PTRACE_GETWDBGINFO, PPC_PTRACE_SETHWDEBUG, PPC_PTRACE_DELHWDEBUG,
	ppc_debug_info, PPC_DEBUG_FEATURE_INSN_BP_RANGE,
	PPC_DEBUG_FEATURE_INSN_BP_MASK, PPC_DEBUG_FEATURE_DATA_BP_RANGE,
	PPC_DEBUG_FEATURE_DATA_BP_MASK, ppc_hw_breakpoint,
	PPC_BREAKPOINT_TRIGGER_EXECUTE, PPC_BREAKPOINT_TRIGGER READ,
	PPC_BREAKPOINT_TRIGGER_WRITE, PPC_BREAKPOINT_TRIGGER_RW,
	PPC_BREAKPOINT_MODE_EXACT PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE,
	PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE, PPC_BREAKPOINT_MODE_MASK,
	PPC_BREAKPOINT_CONDITION_NONE, PPC_BREAKPOINT_CONDITION_AND,
	PPC_BREAKPOINT_CONDITION_EXACT, PPC_BREAKPOINT_CONDITION_OR,
	PPC_BREAKPOINT_CONDITION_AND_OR, PPC_BREAKPOINT_CONDITION_BE_ALL,
	PPC_BREAKPOINT_CONDITION_BE_SHIFT, PPC_BREAKPOINT_CONDITION_BE):
	Define, in case <ptrace.h> doesn't provide it.
	(have_ptrace_booke_interface): New function.
	(ppc_linux_check_watch_resources): Renamed to ...
	(ppc_linux_can_use_hw_breakpoint): ... this.  Handle BookE processors.
	(booke_debug_info): New variable.
	(max_slots_number): Ditto.
	(hw_break_tuple): New struct.
	(thread_points): Ditto.
	(ppc_threads): New variable.
	(PPC_DEBUG_CURRENT_VERSION): New define.
	(ppc_linux_region_ok_for_hw_watchpoint): Handle BookE processors.
	(booke_cmp_hw_point): New function.
	(booke_find_thread_points_by_tid): Ditto.
	(booke_insert_point): Ditto.
	(booke_remove_point): Ditto.
	(ppc_linux_insert_hw_breakpoint): Ditto.
	(ppc_linux_remove_hw_breakpoint): Ditto.
	(ppc_linux_insert_watchpoint): Handle BookE processors.
	(ppc_linux_remove_watchpoint): Ditto.
	(ppc_linux_new_thread): Ditto.
	(ppc_linux_stopped_data_address): Ditto.
	(ppc_linux_watchpoint_addr_within_range): Ditto.
	(ppc_linux_read_description): Query the target to know if it
	supports the new kernel BookE interface.
	(_initialize_ppc_linux_nat): Initialize to_insert_hw_breakpoint and
	to_remove_hw_breakpoint fields of the target operations struct.