Re: [PATCH v3 16/20] coresight: allow cti to be built as a module

[Mike Leach <mike.leach@linaro.org>]

Hi Tingwei,

On Mon, 20 Jul 2020 at 22:07, Mike Leach <mike.leach@linaro.org> wrote:
>
> Hi,
>
> I've further looked into this - there is a bug in the CTI driver
> cleanup that is unrelated to the module work. I will issue a patch to
> fix.
>
> Regards
>
> Mike
>

In addition to the patch I sent to the mailing list that fixes
existing code in the CTI driver, the following alteration is needed to
the module code to make it work:-
------------------------------
diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c
b/drivers/hwtracing/coresight/coresight-cti-core.c
index adc05e3f2e40..d1cbc35e2b68 100644
--- a/drivers/hwtracing/coresight/coresight-cti-core.c
+++ b/drivers/hwtracing/coresight/coresight-cti-core.c
@@ -809,7 +809,6 @@ static void cti_device_release(struct device *dev)
        struct cti_drvdata *ect_item, *ect_tmp;

        mutex_lock(&ect_mutex);
-       cti_remove_conn_xrefs(drvdata);
        cti_pm_release(drvdata);

        /* remove from the list */
@@ -828,6 +827,10 @@ static int __exit cti_remove(struct amba_device *adev)
 {
        struct cti_drvdata *drvdata = dev_get_drvdata(&adev->dev);

+       mutex_lock(&ect_mutex);
+       cti_remove_conn_xrefs(drvdata);
+       mutex_unlock(&ect_mutex);
+
        coresight_unregister(drvdata->csdev);

        return 0;
----------------------------

Additional testing showed that once this was added then the cti module
could be removed.

However, the following sequence failed.

1) Insert all modules needed for trace (testing cti can be removed:-

root@linaro-developer:/home/linaro/cs-mods# insmod coresight.ko
root@linaro-developer:/home/linaro/cs-mods# insmod coresight-etm4x.ko
root@linaro-developer:/home/linaro/cs-mods# insmod coresight-cti.ko
root@linaro-developer:/home/linaro/cs-mods# rmmod coresight-cti.ko
root@linaro-developer:/home/linaro/cs-mods# insmod coresight-cti.ko
root@linaro-developer:/home/linaro/cs-mods# insmod coresight-tmc.ko
root@linaro-developer:/home/linaro/cs-mods# insmod coresight-replicator.ko
root@linaro-developer:/home/linaro/cs-mods# insmod coresight-funnel.ko

2) run script to trace into etf

Tracing via sysfs
enabling etf
wait before enable etm
enabling etm 0.
enabling etm 1.
enabling etm 2.
enabling etm 3.
waiting for trace
disabling etm 0.
disabling etm 1.
disabling etm 2.
disabling etm 3.
disabling etf
16+0 records in
16+0 records out
8192 bytes (8.2 kB) copied, 0.00134308 s, 6.1 MB/s

3) remove CTI module - this should not make a difference to normal
tracing as the script used does not program the CTIs.

rmmod ../cs-mods/coresight-cti.ko

4) re-run the script to trace:

Tracing via sysfs
enabling etf
wait before enable etm
enabling etm 0.
enabling etm 1.
enabling etm 2.
enabling etm 3.
waiting for trace
disabling etm 0.
disabling etm 1.
disabling etm 2.
disabling etm 3.
disabling etf
Killed

Here I had to kill the command console as the system had crashed and
automatically restarted due to an internal error.

The only message in the boot console, before the restart was:-
[ 2089.183265] Internal error: synchronous external abort: 96000010 [#1] SMP

Format: Log Type - Time(microsec) - Message - Optional Info
Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic
S - QC_IMAGE_VERSION_STRING=BOOT.BF.3.0-00261
S - IMAGE_VARIANT_STRING=HAAAANAAA
S - OEM_IMAGE_VERSION_STRING=C-BPATTH
S - Boot Config, 0x000002e1
S - Core 0 Frequency, 0 MHz
----- reboot continues ----

This could be an issue that has been fixed by patches on
coresight/next, or directly related to module load/unload.

As Mathieu has mentioned, the set needs to be rebased onto
coresight/next. You will then need to test that trace is successful
before and after removing the CTI module, before sending a new
version.

Regards

Mike


> On Mon, 20 Jul 2020 at 18:00, Mike Leach <mike.leach@linaro.org> wrote:
> >
> > Hi,
> >
> > Once again testing on DB410c.
> >
> > The following sequence:-
> >
> > root@linaro-developer:/home/linaro/cs-mods# insmod coresight.ko
> > root@linaro-developer:/home/linaro/cs-mods# insmod coresight-etm4x.ko
> > root@linaro-developer:/home/linaro/cs-mods# insmod coresight-cti.ko
> > root@linaro-developer:/home/linaro/cs-mods# rmmod coresight-cti.ko
> >
> > results in:-
> >
> > This in my command console:-
> >
> > Message from syslogd@linaro-developer at Jul 20 16:51:40 ...
> >  kernel:[  188.838141] Internal error: Oops: 96000004 [#1] SMP
> > Segmentation fault
> > root@linaro-developer:/home/linaro/cs-mods#
> > Message from syslogd@linaro-developer at Jul 20 16:51:40 ...
> >  kernel:[  189.063920] Code: aa0203f6 aa0103f7 aa1e03e0 d503201f (7940e260)
> >
> > AND this in the boot console:-
> >
> > root@linaro-developer:~# [  188.804340] Unable to handle kernel paging
> > request at virtual address 97e3327491382070
> > [  188.804370] Mem abort info:
> > [  188.811217]   ESR = 0x96000004
> > [  188.813832]   EC = 0x25: DABT (current EL), IL = 32 bits
> > [  188.817009]   SET = 0, FnV = 0
> > [  188.822448]   EA = 0, S1PTW = 0
> > [  188.825284] Data abort info:
> > [  188.828346]   ISV = 0, ISS = 0x00000004
> > [  188.831471]   CM = 0, WnR = 0
> > [  188.835029] [97e3327491382070] address between user and kernel address ranges
> > [  188.838141] Internal error: Oops: 96000004 [#1] SMP
> > [  188.845252] Modules linked in: coresight_cti(-) coresight_etm4x coresight
> > [  188.849947] CPU: 2 PID: 4233 Comm: rmmod Not tainted
> > 5.8.0-rc5cs-modscs-mods-00021-gcac254d1f7cc-dirty #284
> > [  188.856890] Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
> > [  188.866439] pstate: 60000005 (nZCv daif -PAN -UAO BTYPE=--)
> > [  188.873387] pc : kernfs_find_ns+0x28/0x120
> > [  188.878673] lr : kernfs_find_and_get_ns+0x44/0x68
> > [  188.882838] sp : ffff000034e77b00
> > [  188.887611] x29: ffff000034e77b00 x28: ffff000034d50d80
> > [  188.890912] x27: 0000000000000000 x26: 0000000000000000
> > [  188.896294] x25: 0000000000000000 x24: ffff0000382aa898
> > [  188.901589] x23: ffff800008d84938 x22: 0000000000000000
> > [  188.906883] x21: ffff800008d84938 x20: 97e3327491382000
> > [  188.912179] x19: 97e3327491382000 x18: ffffffffffffffff
> > [  188.917473] x17: 0000000000000000 x16: 0000000000000000
> > [  188.922770] x15: ffff800011609948 x14: 0000000000000040
> > [  188.928064] x13: 0000000000000228 x12: 0000000000000008
> > [  188.933360] x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
> > [  188.938654] x9 : 0000001d9cd9e3a6 x8 : 0000b790773d66d4
> > [  188.943950] x7 : 0000000000000002 x6 : ffff0000382a8ed4
> > [  188.949244] x5 : 0000000000000000 x4 : ffff0000382a8ed4
> > [  188.954540] x3 : 0000000000000000 x2 : 0000000000000000
> > [  188.959836] x1 : ffff800008d84938 x0 : ffff8000103609fc
> > [  188.965131] Call trace:
> > [  188.970428]  kernfs_find_ns+0x28/0x120
> > [  188.972596]  kernfs_find_and_get_ns+0x44/0x68
> > [  188.976419]  sysfs_remove_link_from_group+0x34/0x60
> > [  188.980857]  coresight_remove_sysfs_link+0x4c/0x88 [coresight]
> > [  188.985541]  cti_device_release+0x7c/0x170 [coresight_cti]
> > [  188.991435]  device_release+0x34/0x90
> > [  188.996905]  kobject_put+0x70/0x200
> > [  189.000634]  device_unregister+0x30/0x78
> > [  189.003942]  coresight_unregister+0x60/0x70 [coresight]
> > [  189.008107]  cti_remove+0x14/0x20 [coresight_cti]
> > [  189.013050]  amba_remove+0x34/0x1a0
> > [  189.017910]  device_release_driver_internal+0x100/0x1b8
> > [  189.021210]  driver_detach+0xa8/0x178
> > [  189.026417]  bus_remove_driver+0x64/0x100
> > [  189.030237]  driver_unregister+0x34/0x60
> > [  189.034228]  amba_driver_unregister+0x20/0x30
> > [  189.038228]  cti_exit+0x1c/0xd60 [coresight_cti]
> > [  189.042478]  __arm64_sys_delete_module+0x1c0/0x280
> > [  189.047167]  el0_svc_common.constprop.3+0xc8/0x170
> > [  189.051765]  do_el0_svc+0x34/0xc0
> > [  189.056538]  el0_sync_handler+0x144/0x1c0
> > [  189.059923]  el0_sync+0x140/0x180
> > [  189.063920] Code: aa0203f6 aa0103f7 aa1e03e0 d503201f (7940e260)
> > [  189.067218] ---[ end trace 05494e5d41cb7242 ]---
> >
> >  Message from syslogd@linaro-developer at Jul 20 16:51:40 ...
> >  kernel:[  188.838141] Internal error: Oops: 96000004 [#1] SMP
> >
> >  Message from syslogd@linaro-developer at Jul 20 16:51:40 ...
> >  kernel:[  189.063920] Code: aa0203f6 aa0103f7 aa1e03e0 d503201f (7940e260)
> >
> > Looks to be in the code that maintains the sysfs connections between
> > the devices. I haven't looked deeper than that.
> >
> > Regards
> >
> > Mike
> >
> > On Fri, 17 Jul 2020 at 06:49, Tingwei Zhang <tingwei@codeaurora.org> wrote:
> > >
> > > Allow to build coresight-cti 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-cti by the Makefile
> > > - add an cti_remove function, for module unload
> > > - add a MODULE_DEVICE_TABLE for autoloading on boot
> > >
> > > Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
> > > ---
> > >  drivers/hwtracing/coresight/Kconfig                |  5 ++++-
> > >  drivers/hwtracing/coresight/Makefile               |  4 ++--
> > >  .../{coresight-cti.c => coresight-cti-core.c}      | 14 ++++++++++++++
> > >  drivers/hwtracing/coresight/coresight-platform.c   |  1 +
> > >  drivers/hwtracing/coresight/coresight.c            |  1 +
> > >  5 files changed, 22 insertions(+), 3 deletions(-)
> > >  rename drivers/hwtracing/coresight/{coresight-cti.c => coresight-cti-core.c} (98%)
> > >
> > > diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> > > index f31778dd0b5d..b04aae2ceecc 100644
> > > --- a/drivers/hwtracing/coresight/Kconfig
> > > +++ b/drivers/hwtracing/coresight/Kconfig
> > > @@ -136,7 +136,7 @@ config CORESIGHT_CPU_DEBUG
> > >           module will be called coresight-cpu-debug.
> > >
> > >  config CORESIGHT_CTI
> > > -       bool "CoreSight Cross Trigger Interface (CTI) driver"
> > > +       tristate "CoreSight Cross Trigger Interface (CTI) driver"
> > >         depends on ARM || ARM64
> > >         help
> > >           This driver provides support for CoreSight CTI and CTM components.
> > > @@ -147,6 +147,9 @@ config CORESIGHT_CTI
> > >           halt compared to disabling sources and sinks normally in driver
> > >           software.
> > >
> > > +         To compile this driver as a module, choose M here: the
> > > +         module will be called coresight-cti.
> > > +
> > >  config CORESIGHT_CTI_INTEGRATION_REGS
> > >         bool "Access CTI CoreSight Integration Registers"
> > >         depends on CORESIGHT_CTI
> > > diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> > > index f2a568b969c4..0359d5a1588f 100644
> > > --- a/drivers/hwtracing/coresight/Makefile
> > > +++ b/drivers/hwtracing/coresight/Makefile
> > > @@ -19,6 +19,6 @@ 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
> > > -obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o \
> > > -                               coresight-cti-platform.o \
> > > +obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o
> > > +coresight-cti-y := coresight-cti-core.o        coresight-cti-platform.o \
> > >                                 coresight-cti-sysfs.o
> > > diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti-core.c
> > > similarity index 98%
> > > rename from drivers/hwtracing/coresight/coresight-cti.c
> > > rename to drivers/hwtracing/coresight/coresight-cti-core.c
> > > index 289b356a64a5..0f8fbe41363a 100644
> > > --- a/drivers/hwtracing/coresight/coresight-cti.c
> > > +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
> > > @@ -817,6 +817,14 @@ static void cti_device_release(struct device *dev)
> > >         if (drvdata->csdev_release)
> > >                 drvdata->csdev_release(dev);
> > >  }
> > > +static int __exit cti_remove(struct amba_device *adev)
> > > +{
> > > +       struct cti_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> > > +
> > > +       coresight_unregister(drvdata->csdev);
> > > +
> > > +       return 0;
> > > +}
> > >
> > >  static int cti_probe(struct amba_device *adev, const struct amba_id *id)
> > >  {
> > > @@ -957,6 +965,7 @@ static const struct amba_id cti_ids[] = {
> > >         CS_AMBA_UCI_ID(0x000bb9ed, uci_id_cti), /* Coresight CTI (SoC 600) */
> > >         { 0, 0},
> > >  };
> > > +MODULE_DEVICE_TABLE(amba, cti_ids);
> > >
> > >  static struct amba_driver cti_driver = {
> > >         .drv = {
> > > @@ -965,6 +974,7 @@ static struct amba_driver cti_driver = {
> > >                 .suppress_bind_attrs = true,
> > >         },
> > >         .probe          = cti_probe,
> > > +       .remove         = cti_remove,
> > >         .id_table       = cti_ids,
> > >  };
> > >
> > > @@ -987,3 +997,7 @@ static void __exit cti_exit(void)
> > >
> > >  module_init(cti_init);
> > >  module_exit(cti_exit);
> > > +
> > > +MODULE_AUTHOR("Mike Leach <mike.leach@linaro.org>");
> > > +MODULE_DESCRIPTION("Arm CoreSight CTI Driver");
> > > +MODULE_LICENSE("GPL v2");
> > > diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
> > > index e4912abda3aa..70477a256be3 100644
> > > --- a/drivers/hwtracing/coresight/coresight-platform.c
> > > +++ b/drivers/hwtracing/coresight/coresight-platform.c
> > > @@ -76,6 +76,7 @@ coresight_find_csdev_by_fwnode(struct fwnode_handle *r_fwnode)
> > >         }
> > >         return csdev;
> > >  }
> > > +EXPORT_SYMBOL_GPL(coresight_find_csdev_by_fwnode);
> > >
> > >  #ifdef CONFIG_OF
> > >  static inline bool of_coresight_legacy_ep_is_input(struct device_node *ep)
> > > diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
> > > index b814ca54acc9..55b699aca9ec 100644
> > > --- a/drivers/hwtracing/coresight/coresight.c
> > > +++ b/drivers/hwtracing/coresight/coresight.c
> > > @@ -273,6 +273,7 @@ void coresight_set_assoc_ectdev_mutex(struct coresight_device *csdev,
> > >         csdev->ect_dev = ect_csdev;
> > >         mutex_unlock(&coresight_mutex);
> > >  }
> > > +EXPORT_SYMBOL_GPL(coresight_set_assoc_ectdev_mutex);
> > >
> > >  static int coresight_enable_sink(struct coresight_device *csdev,
> > >                                  u32 mode, void *data)
> > > --
> > > 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
> >
> >
> >
> > --
> > Mike Leach
> > Principal Engineer, ARM Ltd.
> > Manchester Design Centre. UK
>
>
>
> --
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK



-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel