[PATCH] hw/arm/boot: set CPTR_EL3.ESM and SCR_EL3.EnTP2 when booting Linux with EL3

[PATCH] hw/arm/boot: set CPTR_EL3.ESM and SCR_EL3.EnTP2 when booting Linux with EL3

[Jerome Forissier]

According to the Linux kernel booting.rst [1], CPTR_EL3.ESM and
SCR_EL3.EnTP2 must be initialized to 1 when EL3 is present and FEAT_SME
is advertised. This has to be taken care of when QEMU boots directly
into the kernel (i.e., "-M virt,secure=on -cpu max -kernel Image").

Cc: qemu-stable@nongnu.org
Fixes: 78cb9776662a ("target/arm: Enable SME for -cpu max")
Link: [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/arm64/booting.rst?h=v6.0#n321
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 hw/arm/boot.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index ada2717f76..ee3858b673 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -763,6 +763,10 @@ static void do_cpu_reset(void *opaque)
                     if (cpu_isar_feature(aa64_sve, cpu)) {
                         env->cp15.cptr_el[3] |= R_CPTR_EL3_EZ_MASK;
                     }
+                    if (cpu_isar_feature(aa64_sme, cpu)) {
+                        env->cp15.cptr_el[3] |= R_CPTR_EL3_ESM_MASK;
+                        env->cp15.scr_el3 |= SCR_ENTP2;
+                    }
                     /* AArch64 kernels never boot in secure mode */
                     assert(!info->secure_boot);
                     /* This hook is only supported for AArch32 currently:
-- 
2.34.1

Re: [PATCH] hw/arm/boot: set CPTR_EL3.ESM and SCR_EL3.EnTP2 when booting Linux with EL3

[Peter Maydell]

On Mon, 3 Oct 2022 at 15:57, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> According to the Linux kernel booting.rst [1], CPTR_EL3.ESM and
> SCR_EL3.EnTP2 must be initialized to 1 when EL3 is present and FEAT_SME
> is advertised. This has to be taken care of when QEMU boots directly
> into the kernel (i.e., "-M virt,secure=on -cpu max -kernel Image").
>
> Cc: qemu-stable@nongnu.org
> Fixes: 78cb9776662a ("target/arm: Enable SME for -cpu max")
> Link: [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/arm64/booting.rst?h=v6.0#n321

Ooh, that detailed set of control bit requirements is new
since I last read that doc -- we should probably go through
and cross-check that we're setting them all correctly.

> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>  hw/arm/boot.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index ada2717f76..ee3858b673 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -763,6 +763,10 @@ static void do_cpu_reset(void *opaque)
>                      if (cpu_isar_feature(aa64_sve, cpu)) {
>                          env->cp15.cptr_el[3] |= R_CPTR_EL3_EZ_MASK;
>                      }
> +                    if (cpu_isar_feature(aa64_sme, cpu)) {
> +                        env->cp15.cptr_el[3] |= R_CPTR_EL3_ESM_MASK;
> +                        env->cp15.scr_el3 |= SCR_ENTP2;
> +                    }

The doc also says that we (as fake EL3) should be setting
SMCR_EL3.LEN to the same value for all CPUs. Currently we do
do that, but it's always the reset value of 0. Richard: does
that have any odd effects (I have a feeling it clamps the
VL to the minimum supported value)? Should we be setting
SMCR_EL3.LEN to the max supported value here ?

thanks
-- PMM

Re: [PATCH] hw/arm/boot: set CPTR_EL3.ESM and SCR_EL3.EnTP2 when booting Linux with EL3

[Peter Maydell]

On Mon, 10 Oct 2022 at 14:16, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 3 Oct 2022 at 15:57, Jerome Forissier
> <jerome.forissier@linaro.org> wrote:
> >
> > According to the Linux kernel booting.rst [1], CPTR_EL3.ESM and
> > SCR_EL3.EnTP2 must be initialized to 1 when EL3 is present and FEAT_SME
> > is advertised. This has to be taken care of when QEMU boots directly
> > into the kernel (i.e., "-M virt,secure=on -cpu max -kernel Image").
> >
> > Cc: qemu-stable@nongnu.org
> > Fixes: 78cb9776662a ("target/arm: Enable SME for -cpu max")
> > Link: [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/arm64/booting.rst?h=v6.0#n321
>
> Ooh, that detailed set of control bit requirements is new
> since I last read that doc -- we should probably go through
> and cross-check that we're setting them all correctly.
>
> > Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> > ---
> >  hw/arm/boot.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> > index ada2717f76..ee3858b673 100644
> > --- a/hw/arm/boot.c
> > +++ b/hw/arm/boot.c
> > @@ -763,6 +763,10 @@ static void do_cpu_reset(void *opaque)
> >                      if (cpu_isar_feature(aa64_sve, cpu)) {
> >                          env->cp15.cptr_el[3] |= R_CPTR_EL3_EZ_MASK;
> >                      }
> > +                    if (cpu_isar_feature(aa64_sme, cpu)) {
> > +                        env->cp15.cptr_el[3] |= R_CPTR_EL3_ESM_MASK;
> > +                        env->cp15.scr_el3 |= SCR_ENTP2;
> > +                    }
>
> The doc also says that we (as fake EL3) should be setting
> SMCR_EL3.LEN to the same value for all CPUs. Currently we do
> do that, but it's always the reset value of 0. Richard: does
> that have any odd effects (I have a feeling it clamps the
> VL to the minimum supported value)? Should we be setting
> SMCR_EL3.LEN to the max supported value here ?

In any case, this patch is right as far as it goes and
obviously improves the current situation, and it puts
SME into the same place as SVE, so I'll take this into
target-arm.next.

-- PMM

Re: [PATCH] hw/arm/boot: set CPTR_EL3.ESM and SCR_EL3.EnTP2 when booting Linux with EL3

[Richard Henderson]

On 10/10/22 06:16, Peter Maydell wrote:
> The doc also says that we (as fake EL3) should be setting
> SMCR_EL3.LEN to the same value for all CPUs. Currently we do
> do that, but it's always the reset value of 0. Richard: does
> that have any odd effects (I have a feeling it clamps the
> VL to the minimum supported value)?

Yes, it clamps to minimum.

> Should we be setting SMCR_EL3.LEN to the max supported value here?

We can set it to 15 universally without consequence.
It will be clamped to the max supported value elsewhere
when interpreting the combined EL[123] fields.


r~