From: tingwei@codeaurora•org
To: Mike Leach <mike.leach@linaro•org>
Cc: tsoni@codeaurora•org,
Sai Prakash Ranjan <saiprakash.ranjan@codeaurora•org>,
Kim Phillips <kim.phillips@arm•com>,
Mathieu Poirier <mathieu.poirier@linaro•org>,
Suzuki K Poulose <suzuki.poulose@arm•com>,
Alexander Shishkin <alexander.shishkin@linux•intel.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation•org>,
Coresight ML <coresight@lists•linaro.org>,
Mao Jinlong <jinlmao@codeaurora•org>,
Mian Yousaf Kaukab <ykaukab@suse•de>,
Russell King <linux@armlinux•org.uk>,
Randy Dunlap <rdunlap@infradead•org>,
Leo Yan <leo.yan@linaro•org>,
linux-arm-kernel <linux-arm-kernel-bounces@lists•infradead.org>,
linux-arm-kernel <linux-arm-kernel@lists•infradead.org>
Subject: Re: [PATCH v3 08/20] coresight: allow etm4x to be built as a module
Date: Sat, 18 Jul 2020 11:25:42 +0800 [thread overview]
Message-ID: <b619a6250e3642ace7c37bcd4965dedd@codeaurora.org> (raw)
In-Reply-To: <CAJ9a7VhJLYtK7BDbfKY0tYaNBYnSFGcv-sf5rge+3XxiCx-BLA@mail.gmail.com>
Hi Mike,
Thanks for reporting this. This is a good catch.
I was testing on db845c but I didn't encounter this issue with
multiple module load/unload test. I guess it's due to timing difference.
The issue is etm4_cpu_pm_notify() is removed from cpu pm notification
after last etm4 device is removed. There's racing condition when some
etm4 device say etm0 is removed but etm4_cpu_pm_notify() is still
called. I reproduced this issue by manually unbind etm4x device.
The fix is clean etmdrvdata[drvdata->cpu] in etm4_remove() so
etm4_cpu_pm_notify() is gracefully returned. I've verified this on
db845c with manually unbind etm4x device test. Do you mind test
on your db410 as well? I'll put it into v4.
@@ -1542,6 +1543,7 @@ static int __exit etm4_remove(struct amba_device
*adev)
etm_perf_symlink(drvdata->csdev, false);
+ etmdrvdata[drvdata->cpu] = NULL;
if (--etm4_count == 0) {
etm4_cpu_pm_unregister();
On 2020-07-18 01:05, Mike Leach wrote:
> Hi Tingwei,
>
> Couldn't see 00/20 cover note for this set, so reporting this here as
> it relates to etmv4 in part.
>
> The following sequence:-
> (There is a load dependency here of course, if I try to load ETMv4
> without the coresight core, then I get a lot of missing symbol error
> messages - I assume this is expected behaviour?)
Coresight_etm4x module depends on coresight module, so it's expected
behavior.
>
> root@linaro-developer:/home/linaro/cs-mods# insmod coresight.ko
> root@linaro-developer:/home/linaro/cs-mods# insmod coresight-etm4x.ko
>
> correctly loads the coresight core then ETMv4 module.
>
> [ 1208.214674] coresight etm0: CPU0: ETM v4.0 initialized
> [ 1208.215534] coresight etm1: CPU1: ETM v4.0 initialized
> [ 1208.221020] coresight etm2: CPU2: ETM v4.0 initialized
> [ 1208.224757] coresight etm3: CPU3: ETM v4.0 initialized
>
> However, if I then unload the ETMv4 module:-
>
> root@linaro-developer:/home/linaro/cs-mods# rmmod coresight-etm4x.ko
>
> I get a crash:-
>
> [ 1215.963689] ------------[ cut here ]------------
> [ 1215.963741] WARNING: CPU: 3 PID: 0 at
> drivers/hwtracing/coresight/coresight-etm4x-core.c:1364
> etm4_cpu_pm_notify+0xad8/0xb48 [coresight_etm4x]
> [ 1215.967373] Modules linked in: coresight_etm4x(-) coresight [last
> unloaded: coresight]
> [ 1215.979960] CPU: 3 PID: 0 Comm: swapper/3 Tainted: G W
> 5.8.0-rc5cs-modscs-mods-00020-gc03fe910680d #282
> [ 1215.987856] Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC
> (DT)
> [ 1215.998531] pstate: 80000085 (Nzcv daIf -PAN -UAO BTYPE=--)
> [ 1216.005396] pc : etm4_cpu_pm_notify+0xad8/0xb48 [coresight_etm4x]
> [ 1216.010687] lr : notifier_call_chain+0x5c/0xa0
> [ 1216.016926] sp : ffff000009fe7d50
> [ 1216.021265] x29: ffff000009fe7d50 x28: 0000000000000000
> [ 1216.024653] x27: 0000011b1d1464b5 x26: 0000000000000001
> [ 1216.030035] x25: ffff00003c79f888 x24: 0000000000000001
> [ 1216.035330] x23: 0000000000000000 x22: 0000000000000000
> [ 1216.040626] x21: 0000000000000000 x20: 0000000000000000
> [ 1216.045921] x19: ffff000038396480 x18: 0000000000000000
> [ 1216.051216] x17: 0000000000000000 x16: 0000000000000000
> [ 1216.056510] x15: 0000000000000000 x14: ffffffffffffffff
> [ 1216.061806] x13: ffffffffffffffff x12: 0000000000000000
> [ 1216.067101] x11: 0000000000000000 x10: 00000000000009b0
> [ 1216.072396] x9 : ffff000009fe7e70 x8 : ffff000009fd4010
> [ 1216.077690] x7 : 0000000000000000 x6 : 0000000023c2c562
> [ 1216.082986] x5 : 00ffffffffffffff x4 : 0000000000000003
> [ 1216.088281] x3 : ffff000038396880 x2 : ffff800008d8d280
> [ 1216.093576] x1 : ffff80002b1aa000 x0 : 000000005f454c42
> [ 1216.098873] Call trace:
> [ 1216.104178] etm4_cpu_pm_notify+0xad8/0xb48 [coresight_etm4x]
> [ 1216.106343] notifier_call_chain+0x5c/0xa0
> [ 1216.112242] __atomic_notifier_call_chain+0x48/0x60
> [ 1216.116236] cpu_pm_notify+0x44/0x70
> [ 1216.121008] cpu_pm_enter+0x3c/0x80
> [ 1216.124829] psci_enter_domain_idle_state+0x38/0xa8
> [ 1216.128040] cpuidle_enter_state+0x88/0x478
> [ 1216.132900] cpuidle_enter+0x44/0x58
> [ 1216.137069] call_cpuidle+0x40/0x70
> [ 1216.140888] do_idle+0x1e0/0x248
> [ 1216.144099] cpu_startup_entry+0x28/0x98
> [ 1216.147574] secondary_start_kernel+0x1a0/0x1f8
> [ 1216.151476] ---[ end trace 97bcd7b8bdd7f9a7 ]---
>
> This happens each time, though the CPU number can change.
> Test platform is Dragonboard DB410, linux 5.8-rc5 + this patch set.
>
> Regards
>
> Mike
>
>
> On Fri, 17 Jul 2020 at 06:48, Tingwei Zhang <tingwei@codeaurora•org>
> wrote:
>>
>> From: Kim Phillips <kim.phillips@arm•com>
>>
>> Allow to build coresight-etm4x as a module, for ease of development.
>>
>> - Kconfig becomes a tristate, to allow =m
>> - append -core to source file name to allow module to
>> be called coresight-etm4x by the Makefile
>> - add an etm4_remove function, for module unload
>> - add a MODULE_DEVICE_TABLE for autoloading on boot
>>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro•org>
>> Cc: Leo Yan <leo.yan@linaro•org>
>> Cc: Alexander Shishkin <alexander.shishkin@linux•intel.com>
>> Cc: Randy Dunlap <rdunlap@infradead•org>
>> Cc: Suzuki K Poulose <Suzuki.Poulose@arm•com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation•org>
>> Cc: Russell King <linux@armlinux•org.uk>
>> Signed-off-by: Kim Phillips <kim.phillips@arm•com>
>> Signed-off-by: Tingwei Zhang <tingwei@codeaurora•org>
>> ---
>> drivers/hwtracing/coresight/Kconfig | 5 ++-
>> drivers/hwtracing/coresight/Makefile | 4 +--
>> ...resight-etm4x.c => coresight-etm4x-core.c} | 31
>> ++++++++++++++++++-
>> 3 files changed, 36 insertions(+), 4 deletions(-)
>> rename drivers/hwtracing/coresight/{coresight-etm4x.c =>
> coresight-etm4x-core.c} (98%)
>>
>> diff --git a/drivers/hwtracing/coresight/Kconfig
> b/drivers/hwtracing/coresight/Kconfig
>> index 8fd9fd139cf3..d6e107bbd30b 100644
>> --- a/drivers/hwtracing/coresight/Kconfig
>> +++ b/drivers/hwtracing/coresight/Kconfig
>> @@ -78,7 +78,7 @@ config CORESIGHT_SOURCE_ETM3X
>> module will be called coresight-etm3x.
>>
>> config CORESIGHT_SOURCE_ETM4X
>> - bool "CoreSight Embedded Trace Macrocell 4.x driver"
>> + tristate "CoreSight Embedded Trace Macrocell 4.x driver"
>> depends on ARM64
>> select CORESIGHT_LINKS_AND_SINKS
>> select PID_IN_CONTEXTIDR
>> @@ -88,6 +88,9 @@ config CORESIGHT_SOURCE_ETM4X
>> for instruction level tracing. Depending on the implemented
> version
>> data tracing may also be available.
>>
>> + To compile this driver as a module, choose M here: the
>> + module will be called coresight-etm4x.
>> +
>> config CORESIGHT_STM
>> tristate "CoreSight System Trace Macrocell driver"
>> depends on (ARM && !(CPU_32v3 || CPU_32v4 || CPU_32v4T)) ||
> ARM64
>> diff --git a/drivers/hwtracing/coresight/Makefile
> b/drivers/hwtracing/coresight/Makefile
>> index d619cfd0abd8..271dc255454f 100644
>> --- a/drivers/hwtracing/coresight/Makefile
>> +++ b/drivers/hwtracing/coresight/Makefile
>> @@ -14,8 +14,8 @@ obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) +=
> coresight-funnel.o \
>> obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o
>> coresight-etm3x-y := coresight-etm3x-core.o coresight-etm-cp14.o \
>> coresight-etm3x-sysfs.o
>> -obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
>> - coresight-etm4x-sysfs.o
>> +obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o
>> +coresight-etm4x-y := coresight-etm4x-core.o coresight-etm4x-sysfs.o
>> obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
>> obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o
>> obj-$(CONFIG_CORESIGHT_CATU) += coresight-catu.o
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c
> b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> similarity index 98%
>> rename from drivers/hwtracing/coresight/coresight-etm4x.c
>> rename to drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 747afc875f91..b5945f62794c 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -1536,6 +1536,26 @@ static struct amba_cs_uci_id uci_id_etm4[] = {
>> }
>> };
>>
>> +static int __exit etm4_remove(struct amba_device *adev)
>> +{
>> + struct etmv4_drvdata *drvdata = dev_get_drvdata(&adev->dev);
>> +
>> + etm_perf_symlink(drvdata->csdev, false);
>> +
>> + if (--etm4_count == 0) {
>> + etm4_cpu_pm_unregister();
>> +
>> +
> cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
>> + if (hp_online)
>> + cpuhp_remove_state_nocalls(hp_online);
>> + }
>> +
>> + coresight_unregister(drvdata->csdev);
>> +
>> + return 0;
>> +}
>> +
>> +
>> static const struct amba_id etm4_ids[] = {
>> CS_AMBA_ID(0x000bb95d), /* Cortex-A53 */
>> CS_AMBA_ID(0x000bb95e), /* Cortex-A57 */
>> @@ -1553,12 +1573,21 @@ static const struct amba_id etm4_ids[] = {
>> {},
>> };
>>
>> +MODULE_DEVICE_TABLE(amba, etm4_ids);
>> +
>> static struct amba_driver etm4x_driver = {
>> .drv = {
>> .name = "coresight-etm4x",
>> + .owner = THIS_MODULE,
>> .suppress_bind_attrs = true,
>> },
>> .probe = etm4_probe,
>> + .remove = etm4_remove,
>> .id_table = etm4_ids,
>> };
>> -builtin_amba_driver(etm4x_driver);
>> +module_amba_driver(etm4x_driver);
>> +
>> +MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora•org>");
>> +MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro•org>");
>> +MODULE_DESCRIPTION("Arm CoreSight Program Flow Trace v4 driver");
>> +MODULE_LICENSE("GPL v2");
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> Forum,
>> a Linux Foundation Collaborative Project
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists•infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists•infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-07-18 3:27 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-17 5:45 [PATCH v3 00/20] coresight: allow to build coresight as modules Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 01/20] coresight: cpu_debug: add module name in Kconfig Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 02/20] coresight: cpu_debug: define MODULE_DEVICE_TABLE Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 03/20] coresight: use IS_ENABLED for CONFIGs that may be modules Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 04/20] coresight: add coresight prefix to barrier_pkt Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 05/20] coresight: export global symbols Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 06/20] Allow to build coresight-stm as a module, for ease of development Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 07/20] coresight: allow etm3x to be built as a module Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 08/20] coresight: allow etm4x " Tingwei Zhang
2020-07-17 17:05 ` Mike Leach
2020-07-18 3:25 ` tingwei [this message]
2020-07-18 17:38 ` Mike Leach
2020-07-21 14:55 ` Mathieu Poirier
2020-07-21 23:22 ` tingwei
2020-07-20 6:58 ` Sai Prakash Ranjan
2020-07-21 7:03 ` Sai Prakash Ranjan
2020-07-17 5:45 ` [PATCH v3 09/20] coresight: allow etb " Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 10/20] coresight: allow tpiu " Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 11/20] coresight: allow tmc " Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 12/20] coresight: remove multiple init calls from funnel driver Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 13/20] coresight: remove multiple init calls from replicator driver Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 14/20] coresight: allow funnel and replicator drivers to be built as modules Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 15/20] coresight: cti: add function to register cti associate ops Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 16/20] coresight: allow cti to be built as a module Tingwei Zhang
2020-07-20 17:00 ` Mike Leach
2020-07-20 21:07 ` Mike Leach
2020-07-21 16:35 ` Mike Leach
2020-07-21 23:27 ` tingwei
2020-07-17 5:45 ` [PATCH v3 17/20] coresight: tmc-etr: add function to register catu ops Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 18/20] coresight: allow catu drivers to be built as modules Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 19/20] coresight: add try_get_module() in coresight_grab_device() Tingwei Zhang
2020-07-22 10:49 ` Suzuki K Poulose
2020-07-22 10:48 ` Greg KH
2020-07-22 11:26 ` Suzuki K Poulose
2020-07-23 0:18 ` tingwei
2020-07-23 0:35 ` tingwei
2020-07-22 10:51 ` Suzuki K Poulose
2020-07-23 0:19 ` tingwei
2020-07-23 19:36 ` Mathieu Poirier
2020-07-24 1:17 ` Tingwei Zhang
2020-07-17 5:45 ` [PATCH v3 20/20] coresight: allow the coresight core driver to be built as a module Tingwei Zhang
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=b619a6250e3642ace7c37bcd4965dedd@codeaurora.org \
--to=tingwei@codeaurora$(echo .)org \
--cc=alexander.shishkin@linux$(echo .)intel.com \
--cc=coresight@lists$(echo .)linaro.org \
--cc=gregkh@linuxfoundation$(echo .)org \
--cc=jinlmao@codeaurora$(echo .)org \
--cc=kim.phillips@arm$(echo .)com \
--cc=leo.yan@linaro$(echo .)org \
--cc=linux-arm-kernel-bounces@lists$(echo .)infradead.org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux@armlinux$(echo .)org.uk \
--cc=mathieu.poirier@linaro$(echo .)org \
--cc=mike.leach@linaro$(echo .)org \
--cc=rdunlap@infradead$(echo .)org \
--cc=saiprakash.ranjan@codeaurora$(echo .)org \
--cc=suzuki.poulose@arm$(echo .)com \
--cc=tsoni@codeaurora$(echo .)org \
--cc=ykaukab@suse$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox