Re: [PATCH v1 5/5] coresight: etm4x: save/restore state across CPU low power states

[Mathieu Poirier <mathieu.poirier@linaro.org>]

On Wed, 19 Jun 2019 at 05:07, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Wed, Jun 19, 2019 at 11:38:12AM +0100, Suzuki K Poulose wrote:
> > Cc: Al Grant, Mike Leach
> >
> > Hi Sudeep,
> >
> > On 18/06/2019 14:21, Sudeep Holla wrote:
> > > On Tue, Jun 18, 2019 at 01:54:33PM +0100, Andrew Murray wrote:
> > > > Some hardware will ignore bit TRCPDCR.PU which is used to signal
> > > > to hardware that power should not be removed from the trace unit.
> > >
> > > So, how or can we identify or discover such system ? DT/ACPI ?
> > >
> >
> > I don't think there is a mechanism at the moment to identify such
> > systems. But if we really need to know this information, we could
> > always think about it.
> >
>
> I prefer that as we shouldn't systems that are not broken.
>
> > > > Let's mitigate against this by saving and restoring the trace
> > > > unit state when the CPU enters low power states.
> > > >
> > >
> > > I prefer to do this conditionally. It's unnecessary on systems which
> > > don't ignore the TRCPDCR.PU and I really don't like them to be penalised
> > > while we want to add this support for *broken* systems.
> >
> > It is conditional. i.e, you may disable the operation using a kernel/module
> > parameter, which I think should be mentioned in the description here.
> >
>
> Why should the user of coresight need to know if the corresponding
> hardware module is broken or not. I prefer the firmware tell OS.

I think using ACPI/DT is the best and simplest solution.

>
> > >
> > > This is generally most useful to debug CPU suspend/resume exercising
> > > the code path completely with emulated CPU power on/off as most of the
> > > systems have the trace unit and the CPUs in the same power domain.
> >
> > I understand, which is specifically why this comes with an option to handle
> > such cases.
> >
>
> OK
>
> > >
> > > Just curious if this reported on any platforms ?
> > >
> >
> > I have heard people complaining about this, but not sure about the exact
> > platform(s) affected.

Are you referring to platforms that ignore the TRCPDCR.PU bit?  If so
Juno is the only one that does _not_ ignore it, hence the need to find
another solution.

> >
>
> One we add mechanism in place, platform need to advertise that it's
> broken in firmware(DT/ACPI). Or just have a blacklist if we don't
> want to add anything extra to the firmware(DT/ACPI) ?
>
> > > I wounder if we can use TRCPDSR(Power Down Status Register) to check the
> > > status. I know on Juno, it doesn't loose context rather the power down
> > > is emulated and saving/restoring may not be needed at all. Have you
> > > tested on Juno with and without these patches and seen any difference ?
> >
> > The problem is trace unit looses power the moment CPU goes to low power mode
> > and if we try to read this register, it could cause unexpected side-effects.
> >
>
> No I meant before CPU loose power i.e. in CPU_ENTER case. However I do
> remember you/Andrew mentioning that even that may be bogus on broken
> systems, so firmware is only way to avoid penalising all platforms IMO.

I wouldn't assume that anything is working properly.

>
> Or other option is to stop the coresight tracing session like we do
> for PMUs or not entering idle when there's any active coresight session
> in progress on such platforms.
>
> --
> Regards,
> Sudeep

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