Re: [Qemu-arm] [Qemu-devel] [PATCH for-4.1] target/arm: Avoid bogus NSACR traps on M-profile without Security Extension

[Damien Hedde]

On 8/1/19 12:57 PM, Peter Maydell wrote:
> In Arm v8.0 M-profile CPUs without the Security Extension and also in
> v7M CPUs, there is no NSACR register. However, the code we have to handle
> the FPU does not always check whether the ARM_FEATURE_M_SECURITY bit
> is set before testing whether env->v7m.nsacr permits access to the
> FPU. This means that for a CPU with an FPU but without the Security
> Extension we would always take a bogus fault when trying to stack
> the FPU registers on an exception entry.
> 
> We could fix this by adding extra feature bit checks for all uses,
> but it is simpler to just make the internal value of nsacr 0x3ff

s/0x3ff/0xcff/ I think, given you put 0xcff after and in the code

> ("all non-secure accesses allowed"), since this is not guest
> visible when the Security Extension is not present. This allows
> us to continue to follow the Arm ARM pseudocode which takes a
> similar approach. (In particular, in the v8.1 Arm ARM the register
> is documented as reading as 0xcff in this configuration.)
> 
> Fixes: https://bugs.launchpad.net/qemu/+bug/1838475
> Signed-off-by: Peter Maydell <address@hidden>
> ---
> I've marked this as for-4.1 because support for M-profile FPU
> is new in this release and this is a pretty bad bug for it.
> It probably doesn't qualify as so bad we need an rc4 but I
> think we need an rc4 anyway and it probably does merit putting
> in at that point.
> 
>  target/arm/cpu.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 9eb40ff755f..ec2ab95dbeb 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -266,6 +266,14 @@ static void arm_cpu_reset(CPUState *s)
>               * on ARM_FEATURE_V8 (we don't let the guest see the bit).
>               */
>              env->v7m.aircr = R_V7M_AIRCR_BFHFNMINS_MASK;
> +            /*
> +             * Set NSACR to indicate "NS access permitted to everything";
> +             * this avoids having to have all the tests of it being
> +             * conditional on ARM_FEATURE_M_SECURITY. Note also that from
> +             * v8.1M the guest-visible value of NSACR in a CPU without the
> +             * Security Extension is 0xcff.
> +             */
> +            env->v7m.nsacr = 0xcff;
>          }
>  
>          /* In v7M the reset value of this bit is IMPDEF, but ARM recommends
>