Re: [RFCv1 4/4] perf: arm_spe: Dynamically switch PID tracing to contextidr

[Catalin Marinas]

On Mon, Nov 01, 2021 at 11:28:35PM +0800, Leo Yan wrote:
> On Thu, Oct 21, 2021 at 08:49:46AM -0700, Kees Cook wrote:
> > On Thu, Oct 21, 2021 at 09:45:30PM +0800, Leo Yan wrote:
> > > Now Arm64 provides API for enabling and disable PID tracing, Arm SPE
> > > driver invokes these functions to dynamically enable it during
> > > profiling when the program runs in root PID name space, and disable PID
> > > tracing when the perf event is stopped.
> > > 
> > > Device drivers should not depend on CONFIG_PID_IN_CONTEXTIDR for PID
> > > tracing, so this patch uses the consistent condition for setting bit
> > > EL1_CX for PMSCR.
> > 
> > My own preference here would be to not bother with the new
> > enable/disable helpers, but just open code it right here. (Save a patch
> > and is the only user.) But I defer to the taste of arm64 maintainers. :)
> 
> Before I send out a new version for this patch set (for support
> dynamic PID tracing on Arm64), I'd like to get your opinions for two
> things:
> 
> - Firstly, as Kees suggested to directly use variable
>   'contextidr_in_use' in drivers, which is exported as GPL symbol,
>   it's not necessarily to add two helpers contextidr_{enable|disable}().
>   What's your preference for this?

My preference would be to keep the helpers.

> - Secondly, now this patch set only support dynamic PID tracing for
>   Arm64; and there would be two customers to use dynamic PID tracing:
>   Arm SPE and Coresight ETMv4.x.  So this patch set doesn't support
>   dynamic PID tracing for Arm32 (under arch/arm).
> 
>   Do you accept this patch set for enabling PID tracing on Arm64 and we
>   can defer to support Arm32 when really need PID tracing on Arm32?
>   Or we should enable PID dynamic tracing for Arm64 and Arm32 in one
>   go?

If it doesn't break arm32, it's fine by me.

What's the cost of always enabling CONFIG_PID_IN_CONTEXTIDR? If it's
negligible, I'd not bother at all with any of the enabling/disabling.

Another question: can you run multiple instances of SPE for different
threads on different CPUs? What happens to the global contextidr_in_use
key when one of them stops?

-- 
Catalin