mbox series

[v2,0/3] coresight: enable debug module

Message ID 1488294420-14188-1-git-send-email-leo.yan@linaro.org
Headers show
Series coresight: enable debug module | expand

Message

Leo Yan Feb. 28, 2017, 3:06 p.m. UTC
ARMv8 architecture reference manual (ARM DDI 0487A.k) Chapter H7 "The
Sample-based Profiling Extension" has description for sampling
registers, we can utilize these registers to check program counter
value with combined CPU exception level, secure state, etc. So this is
helpful for CPU lockup bugs, e.g. if one CPU has run into infinite loop
with IRQ disabled; the 'hang' CPU cannot switch context and handle any
interrupt, so it cannot handle SMP call for stack dump, etc.

This patch series is to enable coresight debug module with sample-based
registers and register call back notifier for PCSR register dumping
when panic happens, so we can see below dumping info for panic; and
this patch series has considered the conditions for access permission
for debug registers self, so this can avoid access debug registers when
CPU power domain is off; the driver also try to figure out the CPU is
in secure or non-secure state.

ARM external debug module:
CPU[0]:
 EDPRSR:  0000000b (Power:On DLK:Unlock)
 EDPCSR:  [<ffff00000808eb54>] handle_IPI+0xe4/0x150
 EDCIDSR: 00000000
 EDVIDSR: 90000000 (State:Non-secure Mode:EL1/0 Width:64bits VMID:0)
CPU[1]:
 EDPRSR:  0000000b (Power:On DLK:Unlock)
 EDPCSR:  [<ffff0000087a64c0>] debug_notifier_call+0x108/0x288
 EDCIDSR: 00000000
 EDVIDSR: 90000000 (State:Non-secure Mode:EL1/0 Width:64bits VMID:0)

[...]

This patch series has been verified on 96boards Hikey.

Changes from v1:
* According to Mike Leach suggestion, remove the binding for debug
  module clocks which have been directly provided by CPU clocks.
* According to Mathieu Poirier suggestion, add function
  of_coresight_get_cpu() and some minor refactors for debug module
  driver.

Changes from RFC:
* According to Mike Leach suggestion, added check for EDPRSR to avoid
  lockup; added supporting EDVIDSR and EDCIDSR registers;
* According to Mark Rutland and Mathieu Poirier suggestion, rewrote
  the documentation for DT binding;
* According to Mark and Mathieu suggestion, refined debug driver;


Leo Yan (3):
  coresight: bindings for debug module
  coresight: refactor with function of_coresight_get_cpu
  coresight: add support for debug module

 .../devicetree/bindings/arm/coresight-debug.txt    |  40 +++
 drivers/hwtracing/coresight/Kconfig                |  10 +
 drivers/hwtracing/coresight/Makefile               |   1 +
 drivers/hwtracing/coresight/coresight-debug.c      | 378 +++++++++++++++++++++
 drivers/hwtracing/coresight/of_coresight.c         |  35 +-
 include/linux/coresight.h                          |   2 +
 6 files changed, 454 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/coresight-debug.txt
 create mode 100644 drivers/hwtracing/coresight/coresight-debug.c

-- 
2.7.4

Comments

Mathieu Poirier March 1, 2017, 3:45 p.m. UTC | #1
On Tue, Feb 28, 2017 at 11:06:58PM +0800, Leo Yan wrote:
> According to ARMv8 architecture reference manual (ARM DDI 0487A.k)

> Chapter 'Part H: External debug', the CPU can integrate debug module

> and it can support self-hosted debug and external debug. Especially

> for supporting self-hosted debug, this means the program can access

> the debug module from mmio region; and usually the mmio region is

> integrated with coresight.

> 

> So add document for binding debug component, includes binding to APB

> clock; and also need specify the CPU node which the debug module is

> dedicated to specific CPU.

> 

> Suggested-by: Mike Leach <mike.leach@linaro.org>

> Signed-off-by: Leo Yan <leo.yan@linaro.org>

> ---

>  .../devicetree/bindings/arm/coresight-debug.txt    | 40 ++++++++++++++++++++++

>  1 file changed, 40 insertions(+)

>  create mode 100644 Documentation/devicetree/bindings/arm/coresight-debug.txt

> 

> diff --git a/Documentation/devicetree/bindings/arm/coresight-debug.txt b/Documentation/devicetree/bindings/arm/coresight-debug.txt

> new file mode 100644

> index 0000000..89820d5

> --- /dev/null

> +++ b/Documentation/devicetree/bindings/arm/coresight-debug.txt

> @@ -0,0 +1,40 @@

> +* CoreSight Debug Component:

> +

> +CoreSight debug component are compliant with the ARMv8 architecture reference

> +manual (ARM DDI 0487A.k) Chapter 'Part H: External debug'. The external debug

> +module is mainly used for two modes: self-hosted debug and external debug, and

> +it can be accessed from mmio region from Coresight and eventually the debug

> +module connects with CPU for debugging. And the debug module provides

> +sample-based profiling extension, which can be used to sample CPU program

> +counter, secure state and exception level, etc; usually every CPU has one

> +dedicated debug module to be connected.

> +

> +Required properties:

> +

> +- compatible : should be

> +	     * "arm,coresight-debug", "arm,primecell"; supplemented with

> +	       "arm,primecell" as driver is using the AMBA bus interface.


             * "arm,coresight-debug"; supplemented with "arm,primecell" since
               this driver is using the AMBA bus interface.

> +

> +- reg : physical base address and length of the register set.

> +

> +- clocks : the clock associated to this component.

> +

> +- clock-names : the name of the clock referenced by the code. Since we are

> +                using the AMBA framework, the name of the clock providing

> +		the interconnect should be "apb_pclk" and the clock is

> +		mandatory. The interface between the debug logic and the

> +		processor core is clocked by the internal CPU clock, so it

> +		is enabled with CPU clock by default.

> +

> +- cpu : the cpu phandle the debug module is affined to. When omitted

> +	the source is considered to belong to CPU0.


s/source/module

With those change:

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>


> +

> +Example:

> +

> +	debug@f6590000 {

> +		compatible = "arm,coresight-debug","arm,primecell";

> +		reg = <0 0xf6590000 0 0x1000>;

> +		clocks = <&sys_ctrl HI6220_DAPB_CLK>;

> +		clock-names = "apb_pclk";

> +		cpu = <&cpu0>;

> +	};

> -- 

> 2.7.4

>
Leo Yan March 2, 2017, 12:39 a.m. UTC | #2
On Wed, Mar 01, 2017 at 09:02:33AM -0700, Mathieu Poirier wrote:
> On Tue, Feb 28, 2017 at 11:06:59PM +0800, Leo Yan wrote:

> > This is refactor to add function of_coresight_get_cpu(), so it's used to

> > retrieve CPU id for coresight component. Finally can use it as a common

> > function for multiple places.

> > 

> > Suggested-by: Mathieu Poirier <mathieu.poirier@linaro.org>

> > Signed-off-by: Leo Yan <leo.yan@linaro.org>

> > ---

> >  drivers/hwtracing/coresight/of_coresight.c | 35 ++++++++++++++++++++----------

> >  include/linux/coresight.h                  |  2 ++

> >  2 files changed, 25 insertions(+), 12 deletions(-)

> > 

> > diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c

> > index 629e031..2e7bbe2 100644

> > --- a/drivers/hwtracing/coresight/of_coresight.c

> > +++ b/drivers/hwtracing/coresight/of_coresight.c

> > @@ -101,14 +101,34 @@ static int of_coresight_alloc_memory(struct device *dev,

> >  	return 0;

> >  }

> >  

> > +int of_coresight_get_cpu(struct device_node *node)

> > +{

> > +	int cpu;

> > +	struct device_node *dn;

> > +

> > +	dn = of_parse_phandle(node, "cpu", 0);

> > +

> > +	/* Affinity defaults to CPU0 */

> > +	if (!dn)

> > +		return 0;

> > +

> > +	for_each_possible_cpu(cpu) {

> > +		if (dn == of_get_cpu_node(cpu, NULL))

> > +			break;

> > +	}

> > +	of_node_put(dn);

> 

> In the case where no CPU nodes are found this function will return the number

> of the last CPU that was probed instead of '0'.


Sure, will fix this.

> > +

> > +	return cpu;

> > +}

> > +EXPORT_SYMBOL_GPL(of_coresight_get_cpu);

> 

> Any reason you exported this - the fact that is it declared in corsight.h should

> be enough.


Will remove it.

Thanks,
Leo Yan

> > +

> >  struct coresight_platform_data *of_get_coresight_platform_data(

> >  				struct device *dev, struct device_node *node)

> >  {

> > -	int i = 0, ret = 0, cpu;

> > +	int i = 0, ret = 0;

> >  	struct coresight_platform_data *pdata;

> >  	struct of_endpoint endpoint, rendpoint;

> >  	struct device *rdev;

> > -	struct device_node *dn;

> >  	struct device_node *ep = NULL;

> >  	struct device_node *rparent = NULL;

> >  	struct device_node *rport = NULL;

> > @@ -175,16 +195,7 @@ struct coresight_platform_data *of_get_coresight_platform_data(

> >  		} while (ep);

> >  	}

> >  

> > -	/* Affinity defaults to CPU0 */

> > -	pdata->cpu = 0;

> > -	dn = of_parse_phandle(node, "cpu", 0);

> > -	for (cpu = 0; dn && cpu < nr_cpu_ids; cpu++) {

> > -		if (dn == of_get_cpu_node(cpu, NULL)) {

> > -			pdata->cpu = cpu;

> > -			break;

> > -		}

> > -	}

> > -	of_node_put(dn);

> > +	pdata->cpu = of_coresight_get_cpu(node);

> >  

> >  	return pdata;

> >  }

> > diff --git a/include/linux/coresight.h b/include/linux/coresight.h

> > index 2a5982c..6709561 100644

> > --- a/include/linux/coresight.h

> > +++ b/include/linux/coresight.h

> > @@ -263,11 +263,13 @@ static inline int coresight_timeout(void __iomem *addr, u32 offset,

> >  #endif

> >  

> >  #ifdef CONFIG_OF

> > +extern int of_coresight_get_cpu(struct device_node *node);

> >  extern struct coresight_platform_data *of_get_coresight_platform_data(

> >  				struct device *dev, struct device_node *node);

> >  #else

> >  static inline struct coresight_platform_data *of_get_coresight_platform_data(

> >  	struct device *dev, struct device_node *node) { return NULL; }

> > +static int of_coresight_get_cpu(struct device_node *node) { return 0; }

> >  #endif

> >  

> >  #ifdef CONFIG_PID_NS

> > -- 

> > 2.7.4

> >
Rob Herring (Arm) March 3, 2017, 6:21 a.m. UTC | #3
On Tue, Feb 28, 2017 at 11:06:58PM +0800, Leo Yan wrote:
> According to ARMv8 architecture reference manual (ARM DDI 0487A.k)

> Chapter 'Part H: External debug', the CPU can integrate debug module

> and it can support self-hosted debug and external debug. Especially

> for supporting self-hosted debug, this means the program can access

> the debug module from mmio region; and usually the mmio region is

> integrated with coresight.

> 

> So add document for binding debug component, includes binding to APB

> clock; and also need specify the CPU node which the debug module is

> dedicated to specific CPU.

> 

> Suggested-by: Mike Leach <mike.leach@linaro.org>

> Signed-off-by: Leo Yan <leo.yan@linaro.org>

> ---

>  .../devicetree/bindings/arm/coresight-debug.txt    | 40 ++++++++++++++++++++++

>  1 file changed, 40 insertions(+)

>  create mode 100644 Documentation/devicetree/bindings/arm/coresight-debug.txt


Acked-by: Rob Herring <robh@kernel.org>