[boot-wrapper PATCH 5/5] aarch64: Introduce EL2 boot code for Armv8-R AArch64
Jaxson Han
Jaxson.Han at arm.com
Tue May 11 02:49:03 PDT 2021
Hi Andre,
> -----Original Message-----
> From: Andre Przywara <andre.przywara at arm.com>
> Sent: Tuesday, May 11, 2021 4:00 PM
> To: Jaxson Han <Jaxson.Han at arm.com>
> Cc: Mark Rutland <Mark.Rutland at arm.com>; linux-arm-
> kernel at lists.infradead.org; Wei Chen <Wei.Chen at arm.com>
> Subject: Re: [boot-wrapper PATCH 5/5] aarch64: Introduce EL2 boot code for
> Armv8-R AArch64
>
> On Tue, 11 May 2021 02:03:32 +0000
> Jaxson Han <Jaxson.Han at arm.com> wrote:
>
> Hi,
>
> > > -----Original Message-----
> > > From: Andre Przywara <andre.przywara at arm.com>
> > > Sent: Monday, May 10, 2021 4:55 PM
> > > To: Jaxson Han <Jaxson.Han at arm.com>
> > > Cc: Mark Rutland <Mark.Rutland at arm.com>; linux-arm-
> > > kernel at lists.infradead.org; Wei Chen <Wei.Chen at arm.com>
> > > Subject: Re: [boot-wrapper PATCH 5/5] aarch64: Introduce EL2 boot
> > > code for Armv8-R AArch64
> > >
> > > On Mon, 10 May 2021 02:13:45 +0000
> > > Jaxson Han <Jaxson.Han at arm.com> wrote:
> > >
> > > > Hi Andre,
> > > >
> > > > Since GCC 11 has been released and GCC 11 supports the '
> > > > -march=armv8-r', we got a problem when compile the boot-wrapper
> with '
> > > -march=armv8-r':
> > > > | ../git/arch/aarch64/boot.S:71: Error: selected processor does
> > > > | not support
> > > system register name 'scr_el3'
> > > > | ../git/arch/aarch64/boot.S:73: Error: selected processor does
> > > > | not support
> > > system register name 'cptr_el3'
> > > > | ../git/arch/aarch64/boot.S:84: Error: selected processor does
> > > > | not support
> > > system register name 'mdcr_el3'
> > > > | ../git/arch/aarch64/boot.S:90: Error: selected processor does
> > > > | not support
> > > system register name 'cptr_el3'
> > > > | ../git/arch/aarch64/boot.S:92: Error: selected processor does
> > > > | not support
> > > system register name 'cptr_el3'
> > > > | ../git/arch/aarch64/boot.S:194: Error: selected processor does
> > > > | not
> > > support system register name 'elr_el3'
> > > > | ../git/arch/aarch64/boot.S:195: Error: selected processor does
> > > > | not
> > > support system register name 'spsr_el3'
> > > >
> > > > It seems we may need some #if macro to disable all _el3 registers,
> > > > but it will break our auto-detection (users should add more
> > > > compile/build
> > > parameter).
> > > > So, may I ask your suggestions? :)
> > >
> > > Why do you need that in the first place? I think your version worked
> > > without it? At least Ubuntu's 9.3.0 compiled it just fine.
> > > Or does GCC 11 complain about some v8-r specific registers and you
> > > need to add this armv8-r to let them pass, sacrificing all EL3 registers on
> the way?
> >
> > The problem comes from AIS yocto-bsp team. The reason they need this,
> > I think, may be that they want to test this new option for v8-r since
> > GCC 11 supports v8-r. Anyway, the current version works well. And I
> > agree it's neither necessary nor first priority to solve this problem
> > for boot-wrapper. But I think it's worth to discuss with you and see
> > your suggestions :)
>
> Well, but each software package sets the stage for the compiler options it can
> or cannot accept. And since the boot-wrapper is still foremost a v8-A
> software, just compiling it with v8-r (or any other random switch) won't work.
> So it's just not valid to use that switch there - it's the task of the Makefile to
> set compiler options, any user choices have no guarantee of working anyway.
I got it. And we decided remove this option, because it's almost no gain.
>
> And out of curiosity: what did you expect from using that option?
At the very beginning, maybe, expecting reorganization of v8-r registers,
or some optimizations?
But, since boot-wrapper does nothing but some inits, this option seems
useless totally.
So, we remove it.
>
> > > One solution could be to move all accesses to v8-r registers into a
> > > separate file, and only assemble/compile this with the v8-r switch.
> > > But this sounds like some serious plumbing in the code base.
> > >
> > > What you could try as well is to use this "s3_0_c12_c12_5" like
> > > system register encoding style (this example is for ICC_SRE_EL1).
> > > The kernel uses this trick to avoid dependencies on gas knowing
> > > about all (new) system register names. Not sure if that is enough to trick
> gas into accepting it?
> > >
> > > Hope that helps.
> >
> > Yes, it helps! Based on this, we could easily evaluate the efforts:)
>
> Did you change the EL3 registers this way? Was there any effect on the code?
No, I didn't. There's no need to try, I think, with so many efforts and no gain. :)
Thanks,
Andre
>
> Cheers,
> Andre
>
> > >
> > > Cheers,
> > > Andre
> > >
> > > >
> > > > Cheers,
> > > > Jaxson
> > > >
> > > > > -----Original Message-----
> > > > > From: Jaxson Han
> > > > > Sent: Wednesday, April 28, 2021 11:44 AM
> > > > > To: Andre Przywara <andre.przywara at arm.com>
> > > > > Cc: Mark Rutland <Mark.Rutland at arm.com>; linux-arm-
> > > > > kernel at lists.infradead.org; Wei Chen <Wei.Chen at arm.com>
> > > > > Subject: RE: [boot-wrapper PATCH 5/5] aarch64: Introduce EL2
> > > > > boot code for Armv8-R AArch64
> > > > >
> > > > > Hi Andre,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Andre Przywara <andre.przywara at arm.com>
> > > > > > Sent: Monday, April 26, 2021 8:36 PM
> > > > > > To: Jaxson Han <Jaxson.Han at arm.com>
> > > > > > Cc: Mark Rutland <Mark.Rutland at arm.com>; linux-arm-
> > > > > > kernel at lists.infradead.org; Wei Chen <Wei.Chen at arm.com>
> > > > > > Subject: Re: [boot-wrapper PATCH 5/5] aarch64: Introduce EL2
> > > > > > boot code for Armv8-R AArch64
> > > > > >
> > > > > > On Tue, 20 Apr 2021 15:24:38 +0800 Jaxson Han
> > > > > > <jaxson.han at arm.com>
> > > > > > wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > > The Armv8-R AArch64 profile does not support the EL3 exception
> level.
> > > > > > > The Armv8-R AArch64 profile allows for an (optional)
> > > > > > > VMSAv8-64 MMU at EL1, which allows to run off-the-shelf
> > > > > > > Linux. However EL2 only supports a PMSA, which is not
> > > > > > > supported by Linux, so we need to drop into EL1 before entering
> the kernel.
> > > > > > >
> > > > > > > The boot sequence is:
> > > > > > > If CurrentEL == EL3, then goto EL3 initialisation and drop to lower
> EL
> > > > > > > before entering the kernel.
> > > > > > > If CurrentEL == EL2 && id_aa64mmfr0_el1.MSA == 0xf (Armv8-R
> > > aarch64),
> > > > > > > then goto Armv8-R AArch64 initialisation and drop to EL1 before
> > > > > > > entering the kernel.
> > > > > > > Else, no initialisation and keep the current EL before entering the
> > > > > > > kernel.
> > > > > > >
> > > > > > > Signed-off-by: Jaxson Han <jaxson.han at arm.com>
> > > > > > > ---
> > > > > > > arch/aarch64/boot.S | 51
> > > > > > > +++++++++++++++++++++++++++++++++++++++++++++
> > > > > > > 1 file changed, 51 insertions(+)
> > > > > > >
> > > > > > > diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S index
> > > > > > > f7dbf3f..6961a2a 100644
> > > > > > > --- a/arch/aarch64/boot.S
> > > > > > > +++ b/arch/aarch64/boot.S
> > > > > > > @@ -25,16 +25,22 @@ _start:
> > > > > > > * Boot sequence
> > > > > > > * If CurrentEL == EL3, then goto EL3 initialisation and drop to
> > > > > > > * lower EL before entering the kernel.
> > > > > > > + * If CurrentEL == EL2 && id_aa64mmfr0_el1.MSA == 0xf,
> > > then goto
> > > > > > > + * Armv8-R AArch64 initialisation and drop to EL1 before
> > > > > > > + * entering the kernel.
> > > > > > > * Else, no initialisation and keep the current EL before
> > > > > > > * entering the kernel.
> > > > > > > */
> > > > > > > mrs x0, CurrentEL
> > > > > > > cmp x0, #CURRENTEL_EL3
> > > > > > > beq el3_init
> > > > > > > + cmp x0, #CURRENTEL_EL2
> > > > > > > + beq el2_init
> > > > > >
> > > > > > nitpick: I tend to compare against EL2, then use b.gt for EL3,
> > > > > > b.lt for
> > > > > > EL1 and b.eq for EL2 code. Saves you an extra cmp here.
> > > > >
> > > > > Exactly, I will. Thanks!
> > > > >
> > > > > >
> > > > > > > /*
> > > > > > > * We stay in the current EL for entering the kernel
> > > > > > > */
> > > > > > > +keep_el:
> > > > > > > mov w0, #1
> > > > > > > ldr x1, =flag_keep_el
> > > > > > > str w0, [x1]
> > > > > > > @@ -112,6 +118,43 @@ el3_init:
> > > > > > > str w0, [x1]
> > > > > > > b el_max_init
> > > > > > >
> > > > > > > + /*
> > > > > > > + * EL2 Armv8-R AArch64 initialisation
> > > > > > > + */
> > > > > > > +el2_init:
> > > > > > > + /* Detect Armv8-R AArch64 */
> > > > > > > + mrs x1, id_aa64mmfr0_el1
> > > > > > > + ubfx x1, x1, #48, #4 // MSA
> > > > > > > + /* 0xf means Armv8-R AArch64 */
> > > > > > > + cmp x1, 0xf
> > > > > > > + bne keep_el
> > > > > >
> > > > > > Don't we need to also check bits[55:52], to have at least 0b0010?
> > > > > > IIUC the support for VMSA in EL1&0 is optional, and should be
> > > > > > checked before we proceed? VTCR_EL2[31] can only be set in the
> > > 0b0010 case.
> > > > >
> > > > > Yes, it should be checked, I will add it.
> > > > >
> > > > > >
> > > > > > > +
> > > > > > > + mrs x0, midr_el1
> > > > > > > + msr vpidr_el2, x0
> > > > > > > +
> > > > > > > + mrs x0, mpidr_el1
> > > > > > > + msr vmpidr_el2, x0
> > > > > > > +
> > > > > > > + mov x0, #(1 << 31) // VTCR_MSA:
> > > VMSAv8-64
> > > > > > support
> > > > > > > + msr vtcr_el2, x0
> > > > > > > +
> > > > > > > + /* Enable pointer authentication if present */
> > > > > > > + mrs x1, id_aa64isar1_el1
> > > > > > > + ldr x2, =(((0xff) << 24) | (0xff << 4))
> > > > > >
> > > > > > Each feature only holds four bits, so the mask you shift should be
> 0xf.
> > > > >
> > > > > Yes, I will fix.
> > > > >
> > > > > >
> > > > > > > + and x1, x1, x2
> > > > > > > + cbz x1, 1f
> > > > > > > +
> > > > > > > + mrs x0, hcr_el2
> > > > > >
> > > > > > Shouldn't we force HCR_EL2, instead of modifying it? Just to
> > > > > > make sure nothing unexpected traps into EL2, which we don't
> > > > > > handle very
> > > well?
> > > > > > So basically just set bit 31 (RES1), plus those two bits on
> > > > > > top, if needed. But I also wonder about FIEN[47] and EnSCXT[53] ...
> > > > >
> > > > > Right, we should force to set HCR_EL2. The API and APK is needed.
> > > > > And I will also check if we need the FIEN[47] and EnSCXT[53].
> > > > >
> > > > > Thanks,
> > > > > Jaxson
> > > > >
> > > > > >
> > > > > >
> > > > > > Rest looks alright.
> > > > > >
> > > > > > Cheers,
> > > > > > Andre
> > > > > >
> > > > > > > + orr x0, x0, #(1 << 40) // AP key enable
> > > > > > > + orr x0, x0, #(1 << 41) // AP insn enable
> > > > > > > + msr hcr_el2, x0
> > > > > > > +
> > > > > > > +1: isb
> > > > > > > + mov w0, #SPSR_KERNEL_EL1
> > > > > > > + ldr x1, =spsr_to_elx
> > > > > > > + str w0, [x1]
> > > > > > > + b el_max_init
> > > > > > > +
> > > > > > > el_max_init:
> > > > > > > ldr x0, =CNTFRQ
> > > > > > > msr cntfrq_el0, x0
> > > > > > > @@ -169,10 +212,18 @@ jump_kernel:
> > > > > > > */
> > > > > > > bfi x4, x19, #5, #1
> > > > > > >
> > > > > > > + mrs x5, CurrentEL
> > > > > > > + cmp x5, #CURRENTEL_EL2
> > > > > > > + b.eq 1f
> > > > > > > +
> > > > > > > msr elr_el3, x19
> > > > > > > msr spsr_el3, x4
> > > > > > > eret
> > > > > > >
> > > > > > > +1: msr elr_el2, x19
> > > > > > > + msr spsr_el2, x4
> > > > > > > + eret
> > > > > > > +
> > > > > > > .ltorg
> > > > > > >
> > > > > > > .data
> > > >
> >
More information about the linux-arm-kernel
mailing list