All of lore.kernel.org
 help / color / mirror / Atom feed
From: mathieu.poirier@linaro.org (Mathieu Poirier)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH RFC 2/3] coresight: add support for debug module
Date: Wed, 15 Feb 2017 14:08:05 -0700	[thread overview]
Message-ID: <20170215210805.GB29730@linaro.org> (raw)
In-Reply-To: <1486966298-16767-3-git-send-email-leo.yan@linaro.org>

On Mon, Feb 13, 2017 at 02:11:37PM +0800, Leo Yan wrote:
> Coresight includes debug module and usually the module connects with CPU
> debug logic. ARMv8 architecture reference manual (ARMv8-ARM) has defined
> the debug registers in the chapter "H9: External Debug Register
> Descriptions".
> 
> After enable the debug module we can check CPU state and PC value, etc.
> So this is helpful for some CPU lockup bugs, e.g. if one CPU has run
> into infinite loop with IRQ disabled. So the CPU cannot switch context
> and handle any interrupt, so it cannot handle SMP call for stack dump,
> etc. Furthermore, now ARMv8 introduces some other runtime firmwares like
> ARM trusted firmware BL31, so sometime CPU hard lock may happen in the
> firmware and cannot return back to kernel.
> 
> This patch is to enable coresight debug module and register callback
> notifier for panic; so when system detect the CPU lockup we can utilize
> debug module registers to get to know PC value for all CPUs; so we can
> quickly know the hang address for CPUs.
> 
> This is initial driver for coresight debug module and could enhance it
> later according to debugging requirement.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  drivers/hwtracing/coresight/Kconfig           |   8 ++
>  drivers/hwtracing/coresight/Makefile          |   1 +
>  drivers/hwtracing/coresight/coresight-debug.c | 169 ++++++++++++++++++++++++++
>  3 files changed, 178 insertions(+)
>  create mode 100644 drivers/hwtracing/coresight/coresight-debug.c
> 
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index 130cb21..dcf59cc 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -89,4 +89,12 @@ config CORESIGHT_STM
>  	  logging useful software events or data coming from various entities
>  	  in the system, possibly running different OSs
>  
> +config CORESIGHT_DEBUG
> +	bool "CoreSight debug driver"
> +	depends on ARM || ARM64
> +	help
> +	  This driver provides support for coresight debugging module. This
> +	  is primarily used for printing out debug registers for panic and
> +	  soft and hard lockup.
> +
>  endif
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index af480d9..d540d45 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
>  					coresight-etm4x-sysfs.o
>  obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
>  obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
> +obj-$(CONFIG_CORESIGHT_DEBUG) += coresight-debug.o
> diff --git a/drivers/hwtracing/coresight/coresight-debug.c b/drivers/hwtracing/coresight/coresight-debug.c
> new file mode 100644
> index 0000000..28206a83
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-debug.c
> @@ -0,0 +1,169 @@
> +/*
> + * Copyright(C) 2017 Linaro Limited. All rights reserved.
> + * Author: Leo Yan <leo.yan@linaro.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/moduleparam.h>
> +#include <linux/init.h>
> +#include <linux/types.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <linux/fs.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/smp.h>
> +#include <linux/sysfs.h>
> +#include <linux/stat.h>
> +#include <linux/clk.h>
> +#include <linux/cpu.h>
> +#include <linux/coresight.h>
> +#include <linux/amba/bus.h>
> +#include <linux/uaccess.h>

When possible, please try to order the header files alphabetically.

> +
> +#include "coresight-priv.h"
> +
> +#define EDPCSR_LO	0x0A0
> +#define EDPCSR_HI	0x0AC
> +#define EDOSLAR		0x300
> +#define EDOSLSR		0x304
> +#define EDPDCR		0x310
> +#define EDPDSR		0x314

EDOSLSR, DEPDCR and EDPDSR aren't used in the code presented below.

> +
> +struct debug_drvdata {
> +	void __iomem	*base;
> +	struct device	*dev;
> +	int		cpu;
> +};
> +
> +static struct debug_drvdata *debug_drvdata[NR_CPUS];
> +
> +static void debug_os_unlock(struct debug_drvdata *drvdata)
> +{
> +	/* Unlocks the debug registers */
> +	writel_relaxed(0x0, drvdata->base + EDOSLAR);
> +	isb();
> +}
> +
> +static void debug_read_pcsr(struct debug_drvdata *drvdata)
> +{
> +	u32 pcsr_hi, pcsr_lo;
> +
> +	CS_UNLOCK(drvdata->base);
> +
> +	debug_os_unlock(drvdata);
> +
> +#ifdef CONFIG_64BIT
> +	pcsr_lo = readl_relaxed(drvdata->base + EDPCSR_LO);
> +	pcsr_hi = readl_relaxed(drvdata->base + EDPCSR_HI);
> +
> +	pr_emerg("CPU[%d]: PSCR=0x%lx\n", drvdata->cpu,
> +		 ((unsigned long)pcsr_hi << 32 | (unsigned long)pcsr_lo));
> +#else
> +	pcsr_lo = readl_relaxed(drvdata->base + EDPCSR_LO);
> +
> +	pr_emerg("CPU[%d]: PSCR=0x%lx\n", drvdata->cpu,	pcsr_lo);
> +#endif
> +
> +	CS_LOCK(drvdata->base);
> +}
> +
> +/*
> + * Dump out memory limit information on panic.
> + */
> +static int dump_debug(struct notifier_block *self, unsigned long v, void *p)
> +{
> +	int i;
> +
> +	pr_emerg("Coresight debug module:\n");
> +
> +	for_each_possible_cpu(i) {
> +
> +		if (!debug_drvdata[i])
> +			continue;
> +
> +		debug_read_pcsr(debug_drvdata[i]);

Mark and Mike have raised a number of problems with this.  If I get things
correctly the pseudocode (CreatePCSample() and EDPCSRlo[]), found in the
reference manual, should be dealing with most of them. 

One thing that is subtle in the peudocode is the memory-mapped access to these
registers and something else that should be checked here before proceeding. 

> +	}
> +
> +	return 0;
> +}
> +
> +static struct notifier_block debug_notifier = {
> +	.notifier_call = dump_debug,
> +};
> +
> +static int __init register_coresight_debug_dumper(void)
> +{
> +	atomic_notifier_chain_register(&panic_notifier_list,
> +				       &debug_notifier);
> +	return 0;
> +}
> +__initcall(register_coresight_debug_dumper);
> +
> +static int debug_probe(struct amba_device *adev, const struct amba_id *id)
> +{
> +	void __iomem *base;
> +	struct device *dev = &adev->dev;
> +	struct coresight_platform_data *pdata = NULL;
> +	struct debug_drvdata *drvdata;
> +	struct resource *res = &adev->res;
> +	struct device_node *np = adev->dev.of_node;
> +
> +	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM;
> +
> +	if (np) {
> +		pdata = of_get_coresight_platform_data(dev, np);
> +		if (IS_ERR(pdata))
> +			return PTR_ERR(pdata);
> +		adev->dev.platform_data = pdata;
> +	}
> +
> +	drvdata->dev = &adev->dev;
> +	dev_set_drvdata(dev, drvdata);
> +
> +	/* Validity for the resource is already checked by the AMBA core */
> +	base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	drvdata->base = base;
> +	drvdata->cpu = pdata ? pdata->cpu : 0;
> +	debug_drvdata[drvdata->cpu] = drvdata;

Add atomic_notifier_chain_register() here rather than letting the infrastructure
deal with it in an initcall.  That way we don't clutter the panic_notifier_list
needlessly if the IP is not present.

Thanks,
Mathieu

> +
> +	dev_info(dev, "%s initialized\n", (char *)id->data);
> +	return 0;
> +}
> +
> +static struct amba_id debug_ids[] = {
> +	{       /* Debug for Cortex-A53 */
> +		.id	= 0x000bbd03,
> +		.mask	= 0x000fffff,
> +		.data	= "debug",
> +	},
> +	{ 0, 0},
> +};
> +
> +static struct amba_driver debug_driver = {
> +	.drv = {
> +		.name   = "coresight-debug",
> +		.suppress_bind_attrs = true,
> +	},
> +	.probe		= debug_probe,
> +	.id_table	= debug_ids,
> +};
> +builtin_amba_driver(debug_driver);
> -- 
> 2.7.4
> 

WARNING: multiple messages have this Message-ID (diff)
From: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
To: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	Wei Xu <xuwei5-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>,
	Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>,
	Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Daniel Thompson
	<daniel.thompson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Subject: Re: [PATCH RFC 2/3] coresight: add support for debug module
Date: Wed, 15 Feb 2017 14:08:05 -0700	[thread overview]
Message-ID: <20170215210805.GB29730@linaro.org> (raw)
In-Reply-To: <1486966298-16767-3-git-send-email-leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On Mon, Feb 13, 2017 at 02:11:37PM +0800, Leo Yan wrote:
> Coresight includes debug module and usually the module connects with CPU
> debug logic. ARMv8 architecture reference manual (ARMv8-ARM) has defined
> the debug registers in the chapter "H9: External Debug Register
> Descriptions".
> 
> After enable the debug module we can check CPU state and PC value, etc.
> So this is helpful for some CPU lockup bugs, e.g. if one CPU has run
> into infinite loop with IRQ disabled. So the CPU cannot switch context
> and handle any interrupt, so it cannot handle SMP call for stack dump,
> etc. Furthermore, now ARMv8 introduces some other runtime firmwares like
> ARM trusted firmware BL31, so sometime CPU hard lock may happen in the
> firmware and cannot return back to kernel.
> 
> This patch is to enable coresight debug module and register callback
> notifier for panic; so when system detect the CPU lockup we can utilize
> debug module registers to get to know PC value for all CPUs; so we can
> quickly know the hang address for CPUs.
> 
> This is initial driver for coresight debug module and could enhance it
> later according to debugging requirement.
> 
> Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/hwtracing/coresight/Kconfig           |   8 ++
>  drivers/hwtracing/coresight/Makefile          |   1 +
>  drivers/hwtracing/coresight/coresight-debug.c | 169 ++++++++++++++++++++++++++
>  3 files changed, 178 insertions(+)
>  create mode 100644 drivers/hwtracing/coresight/coresight-debug.c
> 
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index 130cb21..dcf59cc 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -89,4 +89,12 @@ config CORESIGHT_STM
>  	  logging useful software events or data coming from various entities
>  	  in the system, possibly running different OSs
>  
> +config CORESIGHT_DEBUG
> +	bool "CoreSight debug driver"
> +	depends on ARM || ARM64
> +	help
> +	  This driver provides support for coresight debugging module. This
> +	  is primarily used for printing out debug registers for panic and
> +	  soft and hard lockup.
> +
>  endif
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index af480d9..d540d45 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
>  					coresight-etm4x-sysfs.o
>  obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
>  obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
> +obj-$(CONFIG_CORESIGHT_DEBUG) += coresight-debug.o
> diff --git a/drivers/hwtracing/coresight/coresight-debug.c b/drivers/hwtracing/coresight/coresight-debug.c
> new file mode 100644
> index 0000000..28206a83
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-debug.c
> @@ -0,0 +1,169 @@
> +/*
> + * Copyright(C) 2017 Linaro Limited. All rights reserved.
> + * Author: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/moduleparam.h>
> +#include <linux/init.h>
> +#include <linux/types.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <linux/fs.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/smp.h>
> +#include <linux/sysfs.h>
> +#include <linux/stat.h>
> +#include <linux/clk.h>
> +#include <linux/cpu.h>
> +#include <linux/coresight.h>
> +#include <linux/amba/bus.h>
> +#include <linux/uaccess.h>

When possible, please try to order the header files alphabetically.

> +
> +#include "coresight-priv.h"
> +
> +#define EDPCSR_LO	0x0A0
> +#define EDPCSR_HI	0x0AC
> +#define EDOSLAR		0x300
> +#define EDOSLSR		0x304
> +#define EDPDCR		0x310
> +#define EDPDSR		0x314

EDOSLSR, DEPDCR and EDPDSR aren't used in the code presented below.

> +
> +struct debug_drvdata {
> +	void __iomem	*base;
> +	struct device	*dev;
> +	int		cpu;
> +};
> +
> +static struct debug_drvdata *debug_drvdata[NR_CPUS];
> +
> +static void debug_os_unlock(struct debug_drvdata *drvdata)
> +{
> +	/* Unlocks the debug registers */
> +	writel_relaxed(0x0, drvdata->base + EDOSLAR);
> +	isb();
> +}
> +
> +static void debug_read_pcsr(struct debug_drvdata *drvdata)
> +{
> +	u32 pcsr_hi, pcsr_lo;
> +
> +	CS_UNLOCK(drvdata->base);
> +
> +	debug_os_unlock(drvdata);
> +
> +#ifdef CONFIG_64BIT
> +	pcsr_lo = readl_relaxed(drvdata->base + EDPCSR_LO);
> +	pcsr_hi = readl_relaxed(drvdata->base + EDPCSR_HI);
> +
> +	pr_emerg("CPU[%d]: PSCR=0x%lx\n", drvdata->cpu,
> +		 ((unsigned long)pcsr_hi << 32 | (unsigned long)pcsr_lo));
> +#else
> +	pcsr_lo = readl_relaxed(drvdata->base + EDPCSR_LO);
> +
> +	pr_emerg("CPU[%d]: PSCR=0x%lx\n", drvdata->cpu,	pcsr_lo);
> +#endif
> +
> +	CS_LOCK(drvdata->base);
> +}
> +
> +/*
> + * Dump out memory limit information on panic.
> + */
> +static int dump_debug(struct notifier_block *self, unsigned long v, void *p)
> +{
> +	int i;
> +
> +	pr_emerg("Coresight debug module:\n");
> +
> +	for_each_possible_cpu(i) {
> +
> +		if (!debug_drvdata[i])
> +			continue;
> +
> +		debug_read_pcsr(debug_drvdata[i]);

Mark and Mike have raised a number of problems with this.  If I get things
correctly the pseudocode (CreatePCSample() and EDPCSRlo[]), found in the
reference manual, should be dealing with most of them. 

One thing that is subtle in the peudocode is the memory-mapped access to these
registers and something else that should be checked here before proceeding. 

> +	}
> +
> +	return 0;
> +}
> +
> +static struct notifier_block debug_notifier = {
> +	.notifier_call = dump_debug,
> +};
> +
> +static int __init register_coresight_debug_dumper(void)
> +{
> +	atomic_notifier_chain_register(&panic_notifier_list,
> +				       &debug_notifier);
> +	return 0;
> +}
> +__initcall(register_coresight_debug_dumper);
> +
> +static int debug_probe(struct amba_device *adev, const struct amba_id *id)
> +{
> +	void __iomem *base;
> +	struct device *dev = &adev->dev;
> +	struct coresight_platform_data *pdata = NULL;
> +	struct debug_drvdata *drvdata;
> +	struct resource *res = &adev->res;
> +	struct device_node *np = adev->dev.of_node;
> +
> +	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM;
> +
> +	if (np) {
> +		pdata = of_get_coresight_platform_data(dev, np);
> +		if (IS_ERR(pdata))
> +			return PTR_ERR(pdata);
> +		adev->dev.platform_data = pdata;
> +	}
> +
> +	drvdata->dev = &adev->dev;
> +	dev_set_drvdata(dev, drvdata);
> +
> +	/* Validity for the resource is already checked by the AMBA core */
> +	base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	drvdata->base = base;
> +	drvdata->cpu = pdata ? pdata->cpu : 0;
> +	debug_drvdata[drvdata->cpu] = drvdata;

Add atomic_notifier_chain_register() here rather than letting the infrastructure
deal with it in an initcall.  That way we don't clutter the panic_notifier_list
needlessly if the IP is not present.

Thanks,
Mathieu

> +
> +	dev_info(dev, "%s initialized\n", (char *)id->data);
> +	return 0;
> +}
> +
> +static struct amba_id debug_ids[] = {
> +	{       /* Debug for Cortex-A53 */
> +		.id	= 0x000bbd03,
> +		.mask	= 0x000fffff,
> +		.data	= "debug",
> +	},
> +	{ 0, 0},
> +};
> +
> +static struct amba_driver debug_driver = {
> +	.drv = {
> +		.name   = "coresight-debug",
> +		.suppress_bind_attrs = true,
> +	},
> +	.probe		= debug_probe,
> +	.id_table	= debug_ids,
> +};
> +builtin_amba_driver(debug_driver);
> -- 
> 2.7.4
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: Leo Yan <leo.yan@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Wei Xu <xuwei5@hisilicon.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Daniel Thompson <daniel.thompson@linaro.org>
Subject: Re: [PATCH RFC 2/3] coresight: add support for debug module
Date: Wed, 15 Feb 2017 14:08:05 -0700	[thread overview]
Message-ID: <20170215210805.GB29730@linaro.org> (raw)
In-Reply-To: <1486966298-16767-3-git-send-email-leo.yan@linaro.org>

On Mon, Feb 13, 2017 at 02:11:37PM +0800, Leo Yan wrote:
> Coresight includes debug module and usually the module connects with CPU
> debug logic. ARMv8 architecture reference manual (ARMv8-ARM) has defined
> the debug registers in the chapter "H9: External Debug Register
> Descriptions".
> 
> After enable the debug module we can check CPU state and PC value, etc.
> So this is helpful for some CPU lockup bugs, e.g. if one CPU has run
> into infinite loop with IRQ disabled. So the CPU cannot switch context
> and handle any interrupt, so it cannot handle SMP call for stack dump,
> etc. Furthermore, now ARMv8 introduces some other runtime firmwares like
> ARM trusted firmware BL31, so sometime CPU hard lock may happen in the
> firmware and cannot return back to kernel.
> 
> This patch is to enable coresight debug module and register callback
> notifier for panic; so when system detect the CPU lockup we can utilize
> debug module registers to get to know PC value for all CPUs; so we can
> quickly know the hang address for CPUs.
> 
> This is initial driver for coresight debug module and could enhance it
> later according to debugging requirement.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  drivers/hwtracing/coresight/Kconfig           |   8 ++
>  drivers/hwtracing/coresight/Makefile          |   1 +
>  drivers/hwtracing/coresight/coresight-debug.c | 169 ++++++++++++++++++++++++++
>  3 files changed, 178 insertions(+)
>  create mode 100644 drivers/hwtracing/coresight/coresight-debug.c
> 
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index 130cb21..dcf59cc 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -89,4 +89,12 @@ config CORESIGHT_STM
>  	  logging useful software events or data coming from various entities
>  	  in the system, possibly running different OSs
>  
> +config CORESIGHT_DEBUG
> +	bool "CoreSight debug driver"
> +	depends on ARM || ARM64
> +	help
> +	  This driver provides support for coresight debugging module. This
> +	  is primarily used for printing out debug registers for panic and
> +	  soft and hard lockup.
> +
>  endif
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index af480d9..d540d45 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
>  					coresight-etm4x-sysfs.o
>  obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
>  obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
> +obj-$(CONFIG_CORESIGHT_DEBUG) += coresight-debug.o
> diff --git a/drivers/hwtracing/coresight/coresight-debug.c b/drivers/hwtracing/coresight/coresight-debug.c
> new file mode 100644
> index 0000000..28206a83
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-debug.c
> @@ -0,0 +1,169 @@
> +/*
> + * Copyright(C) 2017 Linaro Limited. All rights reserved.
> + * Author: Leo Yan <leo.yan@linaro.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/moduleparam.h>
> +#include <linux/init.h>
> +#include <linux/types.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <linux/fs.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/smp.h>
> +#include <linux/sysfs.h>
> +#include <linux/stat.h>
> +#include <linux/clk.h>
> +#include <linux/cpu.h>
> +#include <linux/coresight.h>
> +#include <linux/amba/bus.h>
> +#include <linux/uaccess.h>

When possible, please try to order the header files alphabetically.

> +
> +#include "coresight-priv.h"
> +
> +#define EDPCSR_LO	0x0A0
> +#define EDPCSR_HI	0x0AC
> +#define EDOSLAR		0x300
> +#define EDOSLSR		0x304
> +#define EDPDCR		0x310
> +#define EDPDSR		0x314

EDOSLSR, DEPDCR and EDPDSR aren't used in the code presented below.

> +
> +struct debug_drvdata {
> +	void __iomem	*base;
> +	struct device	*dev;
> +	int		cpu;
> +};
> +
> +static struct debug_drvdata *debug_drvdata[NR_CPUS];
> +
> +static void debug_os_unlock(struct debug_drvdata *drvdata)
> +{
> +	/* Unlocks the debug registers */
> +	writel_relaxed(0x0, drvdata->base + EDOSLAR);
> +	isb();
> +}
> +
> +static void debug_read_pcsr(struct debug_drvdata *drvdata)
> +{
> +	u32 pcsr_hi, pcsr_lo;
> +
> +	CS_UNLOCK(drvdata->base);
> +
> +	debug_os_unlock(drvdata);
> +
> +#ifdef CONFIG_64BIT
> +	pcsr_lo = readl_relaxed(drvdata->base + EDPCSR_LO);
> +	pcsr_hi = readl_relaxed(drvdata->base + EDPCSR_HI);
> +
> +	pr_emerg("CPU[%d]: PSCR=0x%lx\n", drvdata->cpu,
> +		 ((unsigned long)pcsr_hi << 32 | (unsigned long)pcsr_lo));
> +#else
> +	pcsr_lo = readl_relaxed(drvdata->base + EDPCSR_LO);
> +
> +	pr_emerg("CPU[%d]: PSCR=0x%lx\n", drvdata->cpu,	pcsr_lo);
> +#endif
> +
> +	CS_LOCK(drvdata->base);
> +}
> +
> +/*
> + * Dump out memory limit information on panic.
> + */
> +static int dump_debug(struct notifier_block *self, unsigned long v, void *p)
> +{
> +	int i;
> +
> +	pr_emerg("Coresight debug module:\n");
> +
> +	for_each_possible_cpu(i) {
> +
> +		if (!debug_drvdata[i])
> +			continue;
> +
> +		debug_read_pcsr(debug_drvdata[i]);

Mark and Mike have raised a number of problems with this.  If I get things
correctly the pseudocode (CreatePCSample() and EDPCSRlo[]), found in the
reference manual, should be dealing with most of them. 

One thing that is subtle in the peudocode is the memory-mapped access to these
registers and something else that should be checked here before proceeding. 

> +	}
> +
> +	return 0;
> +}
> +
> +static struct notifier_block debug_notifier = {
> +	.notifier_call = dump_debug,
> +};
> +
> +static int __init register_coresight_debug_dumper(void)
> +{
> +	atomic_notifier_chain_register(&panic_notifier_list,
> +				       &debug_notifier);
> +	return 0;
> +}
> +__initcall(register_coresight_debug_dumper);
> +
> +static int debug_probe(struct amba_device *adev, const struct amba_id *id)
> +{
> +	void __iomem *base;
> +	struct device *dev = &adev->dev;
> +	struct coresight_platform_data *pdata = NULL;
> +	struct debug_drvdata *drvdata;
> +	struct resource *res = &adev->res;
> +	struct device_node *np = adev->dev.of_node;
> +
> +	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM;
> +
> +	if (np) {
> +		pdata = of_get_coresight_platform_data(dev, np);
> +		if (IS_ERR(pdata))
> +			return PTR_ERR(pdata);
> +		adev->dev.platform_data = pdata;
> +	}
> +
> +	drvdata->dev = &adev->dev;
> +	dev_set_drvdata(dev, drvdata);
> +
> +	/* Validity for the resource is already checked by the AMBA core */
> +	base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	drvdata->base = base;
> +	drvdata->cpu = pdata ? pdata->cpu : 0;
> +	debug_drvdata[drvdata->cpu] = drvdata;

Add atomic_notifier_chain_register() here rather than letting the infrastructure
deal with it in an initcall.  That way we don't clutter the panic_notifier_list
needlessly if the IP is not present.

Thanks,
Mathieu

> +
> +	dev_info(dev, "%s initialized\n", (char *)id->data);
> +	return 0;
> +}
> +
> +static struct amba_id debug_ids[] = {
> +	{       /* Debug for Cortex-A53 */
> +		.id	= 0x000bbd03,
> +		.mask	= 0x000fffff,
> +		.data	= "debug",
> +	},
> +	{ 0, 0},
> +};
> +
> +static struct amba_driver debug_driver = {
> +	.drv = {
> +		.name   = "coresight-debug",
> +		.suppress_bind_attrs = true,
> +	},
> +	.probe		= debug_probe,
> +	.id_table	= debug_ids,
> +};
> +builtin_amba_driver(debug_driver);
> -- 
> 2.7.4
> 

  parent reply	other threads:[~2017-02-15 21:08 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-13  6:11 [PATCH RFC 0/3] coresight: enable debug module Leo Yan
2017-02-13  6:11 ` Leo Yan
2017-02-13  6:11 ` Leo Yan
2017-02-13  6:11 ` [PATCH RFC 1/3] coresight: binding for coresight debug driver Leo Yan
2017-02-13  6:11   ` Leo Yan
2017-02-15 11:13   ` Mark Rutland
2017-02-15 11:13     ` Mark Rutland
2017-02-15 11:13     ` Mark Rutland
2017-02-15 20:08   ` Mathieu Poirier
2017-02-15 20:08     ` Mathieu Poirier
2017-02-15 20:08     ` Mathieu Poirier
2017-02-16 13:55     ` Leo Yan
2017-02-16 13:55       ` Leo Yan
2017-02-13  6:11 ` [PATCH RFC 2/3] coresight: add support for debug module Leo Yan
2017-02-13  6:11   ` Leo Yan
2017-02-15 11:43   ` Mark Rutland
2017-02-15 11:43     ` Mark Rutland
2017-02-15 11:43     ` Mark Rutland
2017-02-16 18:14     ` Mathieu Poirier
2017-02-16 18:14       ` Mathieu Poirier
2017-02-16 18:14       ` Mathieu Poirier
2017-02-15 11:44   ` Mark Rutland
2017-02-15 11:44     ` Mark Rutland
2017-02-15 11:44     ` Mark Rutland
2017-02-16 15:21     ` Leo Yan
2017-02-16 15:21       ` Leo Yan
2017-02-16 15:21       ` Leo Yan
2017-02-15 21:08   ` Mathieu Poirier [this message]
2017-02-15 21:08     ` Mathieu Poirier
2017-02-15 21:08     ` Mathieu Poirier
2017-02-16 15:26     ` Leo Yan
2017-02-16 15:26       ` Leo Yan
2017-02-16 15:26       ` Leo Yan
2017-02-13  6:11 ` [PATCH RFC 3/3] arm64: dts: register Hi6220's coresight " Leo Yan
2017-02-13  6:11   ` Leo Yan
2017-02-15 21:19   ` Mathieu Poirier
2017-02-15 21:19     ` Mathieu Poirier
2017-02-15 21:19     ` Mathieu Poirier
2017-02-13 15:58 ` [PATCH RFC 0/3] coresight: enable " Mike Leach
2017-02-13 23:37   ` Leo Yan
2017-02-13 23:37     ` Leo Yan
2017-02-13 23:37     ` Leo Yan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170215210805.GB29730@linaro.org \
    --to=mathieu.poirier@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.