[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-4.1] target/arm: Avoid bogus NSACR traps on
From: |
Damien Hedde |
Subject: |
Re: [Qemu-devel] [PATCH for-4.1] target/arm: Avoid bogus NSACR traps on M-profile without Security Extension |
Date: |
Thu, 1 Aug 2019 16:20:53 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.1 |
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
>