From: Andre Przywara <andre.przywara@arm.com>
To: Jaxson Han <Jaxson.Han@arm.com>
Cc: Mark Rutland <Mark.Rutland@arm.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
Wei Chen <Wei.Chen@arm.com>
Subject: Re: [boot-wrapper PATCH 5/5] aarch64: Introduce EL2 boot code for Armv8-R AArch64
Date: Tue, 11 May 2021 08:59:57 +0100 [thread overview]
Message-ID: <20210511085957.3b0094c7@slackpad.fritz.box> (raw)
In-Reply-To: <AS8PR08MB61177711DFAE8E7B09A9661583539@AS8PR08MB6117.eurprd08.prod.outlook.com>
On Tue, 11 May 2021 02:03:32 +0000
Jaxson Han <Jaxson.Han@arm.com> wrote:
Hi,
> > -----Original Message-----
> > From: Andre Przywara <andre.przywara@arm.com>
> > Sent: Monday, May 10, 2021 4:55 PM
> > To: Jaxson Han <Jaxson.Han@arm.com>
> > Cc: Mark Rutland <Mark.Rutland@arm.com>; linux-arm-
> > kernel@lists.infradead.org; Wei Chen <Wei.Chen@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@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.
And out of curiosity: what did you expect from using that option?
> > 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?
Cheers,
Andre
> >
> > Cheers,
> > Andre
> >
> > >
> > > Cheers,
> > > Jaxson
> > >
> > > > -----Original Message-----
> > > > From: Jaxson Han
> > > > Sent: Wednesday, April 28, 2021 11:44 AM
> > > > To: Andre Przywara <andre.przywara@arm.com>
> > > > Cc: Mark Rutland <Mark.Rutland@arm.com>; linux-arm-
> > > > kernel@lists.infradead.org; Wei Chen <Wei.Chen@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@arm.com>
> > > > > Sent: Monday, April 26, 2021 8:36 PM
> > > > > To: Jaxson Han <Jaxson.Han@arm.com>
> > > > > Cc: Mark Rutland <Mark.Rutland@arm.com>; linux-arm-
> > > > > kernel@lists.infradead.org; Wei Chen <Wei.Chen@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@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@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
> > >
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-05-11 12:03 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-20 7:24 [boot-wrapper PATCH 0/5] Add Armv8-R AArch64 support Jaxson Han
2021-04-20 7:24 ` [boot-wrapper PATCH 1/5] Decouple V2M_SYS config by auto-detect dtb node Jaxson Han
2021-04-26 11:30 ` Andre Przywara
2021-04-28 3:23 ` Jaxson Han
2021-05-10 8:30 ` Andre Przywara
2021-05-10 8:45 ` Jaxson Han
2021-04-20 7:24 ` [boot-wrapper PATCH 2/5] aarch64: Rename labels and prepare for lower EL booting Jaxson Han
2021-04-26 11:40 ` Andre Przywara
2021-04-28 3:28 ` Jaxson Han
2021-04-20 7:24 ` [boot-wrapper PATCH 3/5] gic-v3: Prepare for gicv3 with EL2 Jaxson Han
2021-04-26 11:48 ` Andre Przywara
2021-04-28 3:30 ` Jaxson Han
2021-04-20 7:24 ` [boot-wrapper PATCH 4/5] aarch64: Prepare for booting " Jaxson Han
2021-04-26 11:51 ` Andre Przywara
2021-04-20 7:24 ` [boot-wrapper PATCH 5/5] aarch64: Introduce EL2 boot code for Armv8-R AArch64 Jaxson Han
2021-04-26 12:35 ` Andre Przywara
2021-04-28 3:44 ` Jaxson Han
2021-05-10 2:13 ` Jaxson Han
2021-05-10 8:54 ` Andre Przywara
2021-05-11 2:03 ` Jaxson Han
2021-05-11 7:59 ` Andre Przywara [this message]
2021-05-11 9:49 ` Jaxson Han
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210511085957.3b0094c7@slackpad.fritz.box \
--to=andre.przywara@arm.com \
--cc=Jaxson.Han@arm.com \
--cc=Mark.Rutland@arm.com \
--cc=Wei.Chen@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).