[PATCH] hw/arm/boot: Set NSACR.{CP11, CP10} in dummy SMC setup routine

Clement Deschamps posted 1 patch 4 years, 5 months ago
Test asan passed
Test checkpatch passed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191104151137.81931-1-clement.deschamps@greensocs.com
Maintainers: Peter Maydell <peter.maydell@linaro.org>
hw/arm/boot.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH] hw/arm/boot: Set NSACR.{CP11, CP10} in dummy SMC setup routine
Posted by Clement Deschamps 4 years, 5 months ago
Set the NSACR CP11 and CP10 bits, to allow FPU access in Non-Secure state
when using dummy SMC setup routine. Otherwise an AArch32 kernel will UNDEF as
soon as it tries to use the FPU.

This fixes kernel panic when booting raspbian on raspi2.

Successfully tested with:
  2017-01-11-raspbian-jessie-lite.img
  2018-11-13-raspbian-stretch-lite.img
  2019-07-10-raspbian-buster-lite.img

See also commit ece628fcf6 that fixes the issue when *not* using the
dummy SMC setup routine.

Fixes: fc1120a7f5
Signed-off-by: Clement Deschamps <clement.deschamps@greensocs.com>
---
 hw/arm/boot.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index ef6724960c..8fb4a63606 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -240,6 +240,9 @@ void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
     };
     uint32_t board_setup_blob[] = {
         /* board setup addr */
+        0xee110f51, /* mrc     p15, 0, r0, c1, c1, 2  ;read NSACR */
+        0xe3800b03, /* orr     r0, #0xc00             ;set CP11, CP10 */
+        0xee010f51, /* mcr     p15, 0, r0, c1, c1, 2  ;write NSACR */
         0xe3a00e00 + (mvbar_addr >> 4), /* mov r0, #mvbar_addr */
         0xee0c0f30, /* mcr     p15, 0, r0, c12, c0, 1 ;set MVBAR */
         0xee110f11, /* mrc     p15, 0, r0, c1 , c1, 0 ;read SCR */
-- 
2.23.0


Re: [PATCH] hw/arm/boot: Set NSACR.{CP11, CP10} in dummy SMC setup routine
Posted by Bonnans, Laurent 4 years, 5 months ago
On 11/4/19 4:11 PM, Clement Deschamps wrote:

> Set the NSACR CP11 and CP10 bits, to allow FPU access in Non-Secure state
> when using dummy SMC setup routine. Otherwise an AArch32 kernel will UNDEF as
> soon as it tries to use the FPU.
>
> This fixes kernel panic when booting raspbian on raspi2.
I can confirm that it solves the kernel panics we've had in our tests 
when running our yocto-built images.

Tested-by: Laurent Bonnans <laurent.bonnans@here.com>

Laurent
>
> Successfully tested with:
>    2017-01-11-raspbian-jessie-lite.img
>    2018-11-13-raspbian-stretch-lite.img
>    2019-07-10-raspbian-buster-lite.img
>
> See also commit ece628fcf6 that fixes the issue when *not* using the
> dummy SMC setup routine.
>
> Fixes: fc1120a7f5
> Signed-off-by: Clement Deschamps <clement.deschamps@greensocs.com>
> ---
>   hw/arm/boot.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index ef6724960c..8fb4a63606 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -240,6 +240,9 @@ void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
>       };
>       uint32_t board_setup_blob[] = {
>           /* board setup addr */
> +        0xee110f51, /* mrc     p15, 0, r0, c1, c1, 2  ;read NSACR */
> +        0xe3800b03, /* orr     r0, #0xc00             ;set CP11, CP10 */
> +        0xee010f51, /* mcr     p15, 0, r0, c1, c1, 2  ;write NSACR */
>           0xe3a00e00 + (mvbar_addr >> 4), /* mov r0, #mvbar_addr */
>           0xee0c0f30, /* mcr     p15, 0, r0, c12, c0, 1 ;set MVBAR */
>           0xee110f11, /* mrc     p15, 0, r0, c1 , c1, 0 ;read SCR */
Re: [PATCH] hw/arm/boot: Set NSACR.{CP11, CP10} in dummy SMC setup routine
Posted by Peter Maydell 4 years, 5 months ago
On Mon, 4 Nov 2019 at 15:12, Clement Deschamps
<clement.deschamps@greensocs.com> wrote:
>
> Set the NSACR CP11 and CP10 bits, to allow FPU access in Non-Secure state
> when using dummy SMC setup routine. Otherwise an AArch32 kernel will UNDEF as
> soon as it tries to use the FPU.
>
> This fixes kernel panic when booting raspbian on raspi2.
>
> Successfully tested with:
>   2017-01-11-raspbian-jessie-lite.img
>   2018-11-13-raspbian-stretch-lite.img
>   2019-07-10-raspbian-buster-lite.img
>
> See also commit ece628fcf6 that fixes the issue when *not* using the
> dummy SMC setup routine.
>
> Fixes: fc1120a7f5
> Signed-off-by: Clement Deschamps <clement.deschamps@greensocs.com>
> ---
>  hw/arm/boot.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index ef6724960c..8fb4a63606 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -240,6 +240,9 @@ void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
>      };
>      uint32_t board_setup_blob[] = {
>          /* board setup addr */
> +        0xee110f51, /* mrc     p15, 0, r0, c1, c1, 2  ;read NSACR */
> +        0xe3800b03, /* orr     r0, #0xc00             ;set CP11, CP10 */
> +        0xee010f51, /* mcr     p15, 0, r0, c1, c1, 2  ;write NSACR */
>          0xe3a00e00 + (mvbar_addr >> 4), /* mov r0, #mvbar_addr */
>          0xee0c0f30, /* mcr     p15, 0, r0, c12, c0, 1 ;set MVBAR */
>          0xee110f11, /* mrc     p15, 0, r0, c1 , c1, 0 ;read SCR */
> --
> 2.23.0

Yep. The boot.c logic sets NSACR.{CP11,CP10} for booting a
Linux kernel unless (a) the board says we should boot the
kernel in Secure mode or (b) this is the primary CPU and
the board requires the board_setup code to run in Secure mode.
In that case the board_setup code must set NSACR in the same
way that it is responsible for dropping down to NS.

This affects highbank and raspi, which are the only boards
that set info->secure_board_setup; both of those use the
arm_write_secure_board_setup_dummy_smc(), so this fix is
all we should need.

I plan to fold in this comment update:

diff --git a/include/hw/arm/boot.h b/include/hw/arm/boot.h
index 7f4d0ca7cdf..ce2b48b88bc 100644
--- a/include/hw/arm/boot.h
+++ b/include/hw/arm/boot.h
@@ -107,9 +107,12 @@ struct arm_boot_info {
     void (*write_board_setup)(ARMCPU *cpu,
                               const struct arm_boot_info *info);

-    /* If set, the board specific loader/setup blob will be run from secure
+    /*
+     * If set, the board specific loader/setup blob will be run from secure
      * mode, regardless of secure_boot. The blob becomes responsible for
-     * changing to non-secure state if implementing a non-secure boot
+     * changing to non-secure state if implementing a non-secure boot,
+     * including setting up EL3/Secure registers such as the NSACR as
+     * required by the Linux booting ABI before the switch to non-secure.
      */
     bool secure_board_setup;

and tweak the commit message to mention highbank, when I
apply it to target-arm.next.

thanks
-- PMM