Re: [PATCH v2 01/17] target/arm: Add isar_feature_aa32_vfp_simd

[Peter Maydell]

On Mon, 24 Feb 2020 at 22:22, Richard Henderson
<address@hidden> wrote:
>
> Use this in the places that were checking ARM_FEATURE_VFP, and
> are obviously testing for the existance of the register set
> as opposed to testing for some particular instruction extension.
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/arm/cpu.h        |  9 +++++++++
>  hw/intc/armv7m_nvic.c   | 20 ++++++++++----------
>  linux-user/arm/signal.c |  4 ++--
>  target/arm/arch_dump.c  | 11 ++++++-----
>  target/arm/cpu.c        |  4 ++--
>  target/arm/helper.c     |  4 ++--
>  target/arm/m_helper.c   | 11 ++++++-----
>  7 files changed, 37 insertions(+), 26 deletions(-)
>

> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 79db169e04..8841cc7fde 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -894,7 +894,7 @@ static void cpacr_write(CPUARMState *env, const 
> ARMCPRegInfo *ri,
>           * ASEDIS [31] and D32DIS [30] are both UNK/SBZP without VFP.
>           * TRCDIS [28] is RAZ/WI since we do not implement a trace macrocell.
>           */
> -        if (arm_feature(env, ARM_FEATURE_VFP)) {
> +        if (cpu_isar_feature(aa32_vfp_simd, env_archcpu(env))) {
>              /* VFP coprocessor: cp10 & cp11 [23:20] */
>              mask |= (1 << 31) | (1 << 30) | (0xf << 20);

This use in cpacr_write() is ok but it prompted me to have
a look at the whole function, which is used also for AArch64
CPACR_EL1. Currently we have an odd setup where for a pre-v8
core we carefully check and enforce the RAZ/WI bits, but
for v8 we allow any random bits to be set. Anyway, that's
I guess architecturally valid and not an issue related to
this patchset, so:

Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM