From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751823AbdBOVIM (ORCPT ); Wed, 15 Feb 2017 16:08:12 -0500 Received: from mail-it0-f45.google.com ([209.85.214.45]:37814 "EHLO mail-it0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751356AbdBOVIK (ORCPT ); Wed, 15 Feb 2017 16:08:10 -0500 Date: Wed, 15 Feb 2017 14:08:05 -0700 From: Mathieu Poirier To: Leo Yan Cc: Rob Herring , Mark Rutland , Wei Xu , Catalin Marinas , Will Deacon , linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Daniel Thompson Subject: Re: [PATCH RFC 2/3] coresight: add support for debug module Message-ID: <20170215210805.GB29730@linaro.org> References: <1486966298-16767-1-git-send-email-leo.yan@linaro.org> <1486966298-16767-3-git-send-email-leo.yan@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1486966298-16767-3-git-send-email-leo.yan@linaro.org> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.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 > --- > 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 > + * > + * 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 . > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include 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 >