Re: [bootwrapper PATCH] aarch64: Enable BRBE for the non-secure world

From: Mark Rutland <mark.rutland@arm.com>
To: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, andre.przywara@arm.com,
	robin.murphy@arm.com
Subject: Re: [bootwrapper PATCH] aarch64: Enable BRBE for the non-secure world
Date: Thu, 13 Jan 2022 10:24:07 +0000	[thread overview]
Message-ID: <Yd/9x5t/4BMjZX3t@FVFF77S0Q05N> (raw)
In-Reply-To: <1642066868-23151-1-git-send-email-anshuman.khandual@arm.com>

Hi Anshuman,

This looks fine structurally, I'm just not sure of a couple of details because
I can't find the relevant documentation -- more on that below.

On Thu, Jan 13, 2022 at 03:11:08PM +0530, Anshuman Khandual wrote:
> MDCR_EL3.SBRBE resets to an UNKNOWN value. Configure it to allow the BRBE
> buffer usage and direct register access in the non-secure world. But just
> before that, check AA64DFR0_EL1.BRBE and make sure BRBE is implemented. We
> still continue to reset MDCR_EL3 register to zero with the exception of
> MDCR_EL3.NSPB, MDCR_EL3.NSTB and MDCR_EL3.SBRBE.

I'm struggling to find where the BRBE system register fields are documented.

I looked at the latest ARM ARM (DDI 0487G.b):

  https://developer.arm.com/documentation/ddi0487/gb/

... and the Armv9 supplement (DDI 0608A.a):

  https://developer.arm.com/documentation/ddi0608/aa/

... but AFAICT, neither of those describe the bit-positions of the relevant
fields, so I can't check that those are correct. The other extensions (at leat
TME) describe that in the supplement, so this looks like a bug/oversight. 

Am I looking at the right documents? If this is meant to be in the supplement,
could you please raise a bug report to get that fixed?

> 
> 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 bfbb6ec..ca1b292 100644
> --- a/arch/aarch64/boot.S
> +++ b/arch/aarch64/boot.S
> @@ -103,6 +103,14 @@ ASM_FUNC(_start)
>  	ldr	x1, =(0x3 << 24)
>  	orr	x0, x0, x1
>  
> +1:	mrs     x1, id_aa64dfr0_el1
> +	ubfx    x1, x1, #52, #4
> +	cbz     x1, 1f
> +
> +	// Enable BRBE for the non-secure world.
> +	ldr     x1, =(0x3 << 32)
> +	orr     x0, x0, x1
> +

I assume this is the `SBRBE` field, which naming-wise sounds like it controls
Secure rather than Non-Secure (e,g. by way of comparison to `NSPB`). Is that
correct? What effect does the value 0x3 have?

Thanks,
Mark.

>  1:	msr	mdcr_el3, x0			// Disable traps to EL3
>  
>  	mrs	x0, id_aa64pfr0_el1
> -- 
> 2.7.4
> 

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