Re: [BOOT-WRAPPER PATCH 3/3] aarch64: Enable use of GCS for EL2 and below

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2244 bytes --]

On Wed, Nov 27, 2024 at 10:25:54AM +0000, Mark Rutland wrote:
> On Tue, Nov 26, 2024 at 06:53:03PM +0000, Mark Brown wrote:

> > The values on reset follow the same pattern in all the GCS control
> > registers down to GCSCRE0_EL1 so the initialisations of the other
> > control registers are equally redundant.  We should be consistent here,
> > either initialising all the GCS control registers or relying on the
> > architecture defaults for all of them, and the note in the changelog
> > about them needs an update if the initialisation is there.

> Hmm... that's somewhat unusual, usually the architecture has a "minimal
> reset policy", where _ELx register fields are only reset when ELx is the
> highest EL after the reset, leaving it to SW to initialise lower ELs.
> It's not clear to me if not following that here was deliberate or an
> oversight.

Yeah, I remember being a bit surprised by the choice.

> > > That does raise the question of what specifically happens for a return
> > > at EL3 when GCSCR_EL3.{PCRSEL,RVCHKEN} == {1,0}. Can you enlighten me?

> > RET will attempt to load and use a GCS record, the pseudocode is in
> > LoadCheckGCSRecord() which isn't EL dependent other than the selection
> > of which GCSPR and GCSCR to use and setting the access as privileged if
> > we're not at EL0.

> Sorry, I got the bits backwards, and had meant the case where:

> 	GCSCR_EL3.{PCRSEL,RVCHKEN} == {0,1}

> ... where I assume the RVCHKEN bit has no effect given we don't have a
> GCS return value to compare against, but wanted to confirm that there
> wasn't an architectural edge-case that we might need to feed back to
> architects.

Oh, good - I was wondering why you weren't asking that rather than what
you asked.

> It *seems* like RVCHKEN has no effect for RET when PCRSEL is clear.
> In ARM DDI 0487K.a, C6.2.291, the pseudocode for RET starts with:

...

> AFAICT GCSReturnValueCheckEnabled() is only used by
> LoadCheckGCSRecord(), and that's only used by the pseudocode for
> RET/RETAA/RETAB, so it looks like we're good.

Yes, we only do a return value check if PCRSEL is active - it is only
meaningful as part of a GCS load since it's a comparison of the value we
got from the GCS with the value passed into the ret.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]