Re: [boot-wrapper] [PATCH] aarch64: Enable TRBE for the non-secure world

From: Mark Rutland <mark.rutland@arm.com>
To: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: Andre.Przywara@arm.com, Suzuki.Poulose@arm.com,
	james.morse@arm.com, linux-arm-kernel@lists.infradead.org,
	Anshuman Khandual <anshuman.khandual@arm.com>
Subject: Re: [boot-wrapper] [PATCH] aarch64: Enable TRBE for the non-secure world
Date: Thu, 25 Feb 2021 18:33:04 +0000	[thread overview]
Message-ID: <20210225183304.GB41738@C02TD0UTHF1T.local> (raw)
In-Reply-To: <76566432-2f01-a0a6-26c6-1c2351a4c942@arm.com>

On Fri, Feb 12, 2021 at 05:33:08PM +0000, Alexandru Elisei wrote:
> Hello,
> 
> On 2/11/21 11:26 AM, Anshuman Khandual wrote:
> > MDCR_EL3.NSTB resets to an UNKNOWN value. Configure it to allow the trace
> > buffer to use non-secure memory and to permit direct register accesses from
> > the non-secure world. Before that, just check AA64DFR0_EL1.TraceBuffer and
> > make sure TRBE is implemented. We still continue to reset MDCR_EL3 register
> > to zero with the exception of MDCR_EL3.NSPB and MDCR_EL3.NSTB.
> >
> > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> > ---
> >  arch/aarch64/boot.S | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
> > index 37f4b98..e47cf59 100644
> > --- a/arch/aarch64/boot.S
> > +++ b/arch/aarch64/boot.S
> > @@ -71,6 +71,14 @@ _start:
> >  	ldr	x1, =(0x3 << 12)
> >  	orr	x0, x0, x1
> >  
> > +1:	mrs	x1, id_aa64dfr0_el1
> 
> Nitpick: we could change the first read of ID_AA64DFR0_EL1 to use x2 as the
> destination register, to avoid this second read.

I've kept this as-is for now; my general preference is to keep things
such that each of the feature enables can be read indepdendently, even
if that requires some redundant work.

> > +	ubfx	x1, x1, #44, #4
> > +	cbz	x1, 1f
> > +
> > +	// Enable TRBE for the non-secure world.
> > +	ldr	x1, =(0x3 << 24)
> > +	orr	x0, x0, x1
> > +
> >  1:	msr	mdcr_el3, x0			// Disable traps to EL3
> 
> Looked at [1] for the field definitions, and the patch indeed does what it says.
> 0b11 for MDCR_EL3.NSTB means that the buffer owning regime is the non-secure
> state, and accesses to the buffer control registers from the *secure state* are
> trapped to EL3, which is what we want.
> 
> With or without the destination register change:
> 
> Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks for the review -- I folded your tag in.

Mark.

> 
> [1] https://developer.arm.com/documentation/ddi0601/2020-12/AArch64-Registers/
> 
> Thanks,
> 
> Alex
> 
> >  
> >  	mrs	x0, id_aa64pfr0_el1

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