On Mon, Apr 13, 2020 at 01:55:30PM +0530, Sai Prakash Ranjan wrote:
> Hi Suzuki,
>
> On 2020-04-13 04:47, Suzuki K Poulose wrote:
> > Hi Sai,
> >
> > On 04/09/2020 12:35 PM, Sai Prakash Ranjan wrote:
> > > Reading TMC mode register in tmc_read_prepare_etb without
> > > enabling the TMC hardware leads to async exceptions like
> > > the one in the call trace below. This can happen if the
> > > user tries to read the TMC etf data via device node without
> > > setting up source and the sink first which enables the TMC
> > > hardware in the path. So make sure that the TMC is enabled
> > > before we try to read TMC data.
> >
> > So, one can trigger the same SError by simply :
> >
> > $ cat /sys/bus/coresight/device/tmc_etb0/mgmt/mode
> >
>
> I do not see any SError when I run the above command.
>
> localhost ~ # cat /sys/bus/coresight/devices/tmc_etf0/mgmt/mode
> 0x0
>
> And this is most likely due to
>
> commit cd9e3474bb793dc ("coresight: add PM runtime calls to
> coresight_simple_func()")
Ok, so this is related to power management (you can ignore my question in the
previous email).
Regarding function tmc_read_prepare_etb(), the best way to deal with this is
probably make sure drvdata->mode != CS_MODE_DISABLED before reading TMC_MODE.
If there is a buffer to read it will have been copied when the ETB was disabled
and there won't be a need to access the HW.
Mathieu
>
> > And also :
> >
> > >
> > > Kernel panic - not syncing: Asynchronous SError Interrupt
> > > CPU: 7 PID: 2605 Comm: hexdump Tainted: G S 5.4.30
> > > #122
> > > Call trace:
> > > dump_backtrace+0x0/0x188
> > > show_stack+0x20/0x2c
> > > dump_stack+0xdc/0x144
> > > panic+0x168/0x36c
> > > panic+0x0/0x36c
> > > arm64_serror_panic+0x78/0x84
> > > do_serror+0x130/0x138
> > > el1_error+0x84/0xf8
> > > tmc_read_prepare_etb+0x88/0xb8
> > > tmc_open+0x40/0xd8
> > > misc_open+0x120/0x158
> > > chrdev_open+0xb8/0x1a4
> > > do_dentry_open+0x268/0x3a0
> > > vfs_open+0x34/0x40
> > > path_openat+0x39c/0xdf4
> > > do_filp_open+0x90/0x10c
> > > do_sys_open+0x150/0x3e8
> > > __arm64_compat_sys_openat+0x28/0x34
> > > el0_svc_common+0xa8/0x160
> > > el0_svc_compat_handler+0x2c/0x38
> > > el0_svc_compat+0x8/0x10
> > >
> > > Fixes: 4525412a5046 ("coresight: tmc: making prepare/unprepare
> > > functions generic")
> > > Reported-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> > > Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@xxxxxxxxxxxxxx>
> > > ---
> > > drivers/hwtracing/coresight/coresight-tmc.c | 5 +++++
> > > drivers/hwtracing/coresight/coresight-tmc.h | 1 +
> > > 2 files changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/hwtracing/coresight/coresight-tmc.c
> > > b/drivers/hwtracing/coresight/coresight-tmc.c
> > > index 1cf82fa58289..7bae69748ab7 100644
> > > --- a/drivers/hwtracing/coresight/coresight-tmc.c
> > > +++ b/drivers/hwtracing/coresight/coresight-tmc.c
> > > @@ -62,11 +62,13 @@ void tmc_flush_and_stop(struct tmc_drvdata
> > > *drvdata)
> > > void tmc_enable_hw(struct tmc_drvdata *drvdata)
> > > {
> > > + drvdata->enable = true;
> > > writel_relaxed(TMC_CTL_CAPT_EN, drvdata->base + TMC_CTL);
> > > }
> > > void tmc_disable_hw(struct tmc_drvdata *drvdata)
> > > {
> > > + drvdata->enable = false;
> > > writel_relaxed(0x0, drvdata->base + TMC_CTL);
> > > }
> > > @@ -102,6 +104,9 @@ static int tmc_read_prepare(struct tmc_drvdata
> > > *drvdata)
> > > {
> > > int ret = 0;
> > > + if (!drvdata->enable)
> > > + return -EINVAL;
> > > +
> >
> > Does this check always guarantee that the TMC is enabled when
> > we actually get to reading the MODE ? This needs to be done
> > under the spinlock.
> >
>
> Ok I will make this change.
>
> Thanks,
> Sai
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
