linux-kernel - Re: [PATCH v19 09/11] arm64: Handle BRBE booting requirements

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL_JsqK7eBmRKL0AC99hj5AipL4jki5rrhSvbZCbUxa0vnEKMg@mail.gmail.com>
Date: Wed, 12 Feb 2025 15:21:46 -0600
From: Rob Herring <robh@...nel.org>
To: Leo Yan <leo.yan@....com>
Cc: Will Deacon <will@...nel.org>, Mark Rutland <mark.rutland@....com>, 
	Catalin Marinas <catalin.marinas@....com>, Jonathan Corbet <corbet@....net>, Marc Zyngier <maz@...nel.org>, 
	Oliver Upton <oliver.upton@...ux.dev>, Joey Gouly <joey.gouly@....com>, 
	Suzuki K Poulose <suzuki.poulose@....com>, Zenghui Yu <yuzenghui@...wei.com>, 
	James Clark <james.clark@...aro.org>, Anshuman Khandual <anshuman.khandual@....com>, 
	linux-arm-kernel@...ts.infradead.org, linux-perf-users@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org, 
	kvmarm@...ts.linux.dev
Subject: Re: [PATCH v19 09/11] arm64: Handle BRBE booting requirements

On Wed, Feb 12, 2025 at 6:10 AM Leo Yan <leo.yan@....com> wrote:
>
> On Sun, Feb 02, 2025 at 06:43:03PM -0600, Rob Herring (Arm) wrote:
> >
> > From: Anshuman Khandual <anshuman.khandual@....com>
> >
> > To use the Branch Record Buffer Extension (BRBE), some configuration is
> > necessary at EL3 and EL2. This patch documents the requirements and adds
> > the initial EL2 setup code, which largely consists of configuring the
> > fine-grained traps and initializing a couple of BRBE control registers.
> >
> > Before this patch, __init_el2_fgt() would initialize HDFGRTR_EL2 and
> > HDFGWTR_EL2 with the same value, relying on the read/write trap controls
> > for a register occupying the same bit position in either register. The
> > 'nBRBIDR' trap control only exists in bit 59 of HDFGRTR_EL2, while bit
> > 59 of HDFGRTR_EL2 is RES0, and so this assumption no longer holds.
>
> s/HDFGRTR_EL2/HDFGWTR_EL2
>
> > To handle HDFGRTR_EL2 and HDFGWTR_EL2 having (slightly) different bit
> > layouts, __init_el2_fgt() is changed to accumulate the HDFGRTR_EL2 and
> > HDFGWTR_EL2 control bits separately. While making this change the
> > open-coded value (1 << 62) is replaced with
> > HDFG{R,W}TR_EL2_nPMSNEVFR_EL1_MASK.
> >
> > The BRBCR_EL1 and BRBCR_EL2 registers are unusual and require special
> > initialisation: even though they are subject to E2H renaming, both have
> > an effect regardless of HCR_EL2.TGE, even when running at EL2, and
> > consequently both need to be initialised. This is handled in
> > __init_el2_brbe() with a comment to explain the situation.
> >
> > Cc: Marc Zyngier <maz@...nel.org>
> > Cc: Oliver Upton <oliver.upton@...ux.dev>
> > Signed-off-by: Anshuman Khandual <anshuman.khandual@....com>
> > [Mark: rewrite commit message, fix typo in comment]
> > Signed-off-by: Mark Rutland <mark.rutland@....com>
> > Signed-off-by: Rob Herring (Arm) <robh@...nel.org>
> > ---
> >  Documentation/arch/arm64/booting.rst | 21 +++++++++
> >  arch/arm64/include/asm/el2_setup.h   | 86 ++++++++++++++++++++++++++++++++++--
> >  2 files changed, 104 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/arch/arm64/booting.rst b/Documentation/arch/arm64/booting.rst
> > index cad6fdc96b98..0a421757cacf 100644
> > --- a/Documentation/arch/arm64/booting.rst
> > +++ b/Documentation/arch/arm64/booting.rst
> > @@ -352,6 +352,27 @@ Before jumping into the kernel, the following conditions must be met:
> >
> >      - HWFGWTR_EL2.nSMPRI_EL1 (bit 54) must be initialised to 0b01.
> >
> > +  For CPUs with feature Branch Record Buffer Extension (FEAT_BRBE):
> > +
> > +  - If EL3 is present:
> > +
> > +    - MDCR_EL3.SBRBE (bits 33:32) must be initialised to 0b11.
>
> Can MDCR_EL3.SBRBE be 0b01 ?

Yes, in fact I think that should be required instead. If it is 0b11,
then recording of secure EL0, EL1, and EL2 would be allowed and
accessible to non-secure world. Though I suppose EL3 could explicitly
pause BRBE instead.

> > +
> > +  - If the kernel is entered at EL1 and EL2 is present:
> > +
> > +    - BRBCR_EL2.CC (bit 3) must be initialised to 0b1.
> > +    - BRBCR_EL2.MPRED (bit 4) must be initialised to 0b1.
>
> Should clarify BRBCR_EL2.TS to be initialised to 0b00 ?  Arm ARM
> claims the reset behaviour of the TS field is unknown value.  The
> assembly code below actually has initializes the TS field as zero.

Humm, we don't currently care what it is initialized to because the
timestamp is never used. We would care in the future if we use
timestamps. Will 0b00 be the only correct value? I'm not sure.

> Except the above minor comments, I read the assembly code and it looks
> good to me:
>
> Reviewed-by: Leo Yan <leo.yan@....com>

Thank you.

Rob