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

[Anshuman Khandual]

On 1/17/22 6:55 PM, Mark Rutland wrote:
> On Mon, Jan 17, 2022 at 04:06:44PM +0530, Anshuman Khandual wrote:
>> On 1/13/22 3:54 PM, Mark Rutland wrote:
>>> 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?
>>
>> Please find the MDCR_EL3.SBRBE definition here.
>>
>> https://developer.arm.com/documentation/ddi0601/2021-12/AArch64-Registers/
>> MDCR-EL3--Monitor-Debug-Configuration-Register--EL3-?lang=en#fieldset_0-33_32-1
>>
>>>
>>>>
>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual at 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
>>
>> Right, that is some what counter-intuitive.
>>
>>> correct? What effect does the value 0x3 have?
>>
>> As per the definition above.
>>
>> 0b11	 This control does not cause any direct accesses to BRBE registers or
>>          instruction to be trapped, and does not cause any Exception levels to
>> 	 be a prohibited region.
>>
>> This basically allows BRBE usage in EL2/EL1.
> 
> Thanks for this!
> 
> Just to check, my understanding, there's nothing else that we need to
> initialize to ensure that BRBE won't start recording unexpectedly
> out-of-reset, becuase in BRBCR_EL* the E*BRE bits all warm-reset to 0
> and hence prevent recording in any of EL0/EL1/EL2 (and BRBE never
> records at EL3). Is that right?

Right, that is my understanding as well. Those required bits that start
BRBE recording, always warm-reset to 0.

> 
> Assuming so, I'll take this as-is.
> 
> Thanks,
> Mark.
>