Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 1 Nov 2021 23:28:35 +0800
From:   Leo Yan <leo.yan@linaro.org>
To:     Kees Cook <keescook@chromium.org>
Cc:     Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Ard Biesheuvel <ardb@kernel.org>,
        Sami Tolvanen <samitolvanen@google.com>,
        Nicholas Piggin <npiggin@gmail.com>,
        James Morse <james.morse@arm.com>,
        Marc Zyngier <maz@kernel.org>, Joey Gouly <joey.gouly@arm.com>,
        Peter Collingbourne <pcc@google.com>,
        Vincenzo Frascino <vincenzo.frascino@arm.com>,
        "Peter Zijlstra (Intel)" <peterz@infradead.org>,
        Stephane Eranian <eranian@google.com>,
        James Clark <james.clark@arm.com>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [RFCv1 4/4] perf: arm_spe: Dynamically switch PID tracing to
 contextidr
Message-ID: <20211101152835.GB375622@leoy-ThinkPad-X240s>
References: <20211021134530.206216-1-leo.yan@linaro.org>
 <20211021134530.206216-5-leo.yan@linaro.org>
 <202110210848.35971643C6@keescook>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <202110210848.35971643C6@keescook>
Precedence: bulk

Hi Catalin, Will,

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?

- 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?


Thanks,
Leo