Rather than having pointer authentication properties be specific to the
max CPU type, turn this into a generic feature that can be set for each
CPU model.  This means that for future CPU types the feature can be set
without having the ID_AA64ISAR1 bits clobbered in
arm_cpu_pauth_finalize.  This also makes it possible for real CPU models
to use the impdef algorithm for improved performance by setting
pauth-impdef=on on the command line.

Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jamie Iles <jamie@nuviainc.com>
---

Following Richard's suggestion to make impdef selectable for all CPUs 
where pointer auth is supported, I've moved this up to a full feature 
and then any future CPUs supporting pointer auth can just set 
ARM_FEATURE_PAUTH.

 target/arm/cpu.h   |  2 ++
 target/arm/cpu64.c | 13 +++++++++----
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 272fde83ca4e..f724744c4f2b 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -969,6 +969,7 @@ struct ARMCPU {
      */
     bool prop_pauth;
     bool prop_pauth_impdef;
+    bool has_pauth;
 
     /* DCZ blocksize, in log_2(words), ie low 4 bits of DCZID_EL0 */
     uint32_t dcz_blocksize;
@@ -2115,6 +2116,7 @@ enum arm_features {
     ARM_FEATURE_M_SECURITY, /* M profile Security Extension */
     ARM_FEATURE_M_MAIN, /* M profile Main Extension */
     ARM_FEATURE_V8_1M, /* M profile extras only in v8.1M and later */
+    ARM_FEATURE_PAUTH, /* has pointer authentication support */
 };
 
 static inline int arm_feature(CPUARMState *env, int feature)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index f42803ecaf1d..5a4386ce9218 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -760,10 +760,7 @@ static void aarch64_max_initfn(Object *obj)
         cpu->ctr = 0x80038003; /* 32 byte I and D cacheline size, VIPT icache */
         cpu->dcz_blocksize = 7; /*  512 bytes */
 #endif
-
-        /* Default to PAUTH on, with the architected algorithm. */
-        qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_property);
-        qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_impdef_property);
+        set_feature(&cpu->env, ARM_FEATURE_PAUTH);
     }
 
     aarch64_add_sve_properties(obj);
@@ -835,8 +832,16 @@ static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
 static void aarch64_cpu_instance_init(Object *obj)
 {
     ARMCPUClass *acc = ARM_CPU_GET_CLASS(obj);
+    ARMCPU *cpu = ARM_CPU(obj);
 
     acc->info->initfn(obj);
+
+    /* Default to PAUTH on, with the architected algorithm. */
+    if (arm_feature(&cpu->env, ARM_FEATURE_PAUTH)) {
+        qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_property);
+        qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_impdef_property);
+    }
+
     arm_cpu_post_init(obj);
 }
 
-- 
2.30.2

On 5/25/21 2:01 AM, Jamie Iles wrote:
> Rather than having pointer authentication properties be specific to the
> max CPU type, turn this into a generic feature that can be set for each
> CPU model.  This means that for future CPU types the feature can be set
> without having the ID_AA64ISAR1 bits clobbered in
> arm_cpu_pauth_finalize.  This also makes it possible for real CPU models
> to use the impdef algorithm for improved performance by setting
> pauth-impdef=on on the command line.
> 
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Jamie Iles <jamie@nuviainc.com>
> ---
> 
> Following Richard's suggestion to make impdef selectable for all CPUs
> where pointer auth is supported, I've moved this up to a full feature
> and then any future CPUs supporting pointer auth can just set
> ARM_FEATURE_PAUTH.

New patches should not be in-reply-to another thread.
They get lost that way.

>       bool prop_pauth;
>       bool prop_pauth_impdef;
> +    bool has_pauth;

What's this for?  It doesn't even seem to be used in this patch.

> @@ -2115,6 +2116,7 @@ enum arm_features {
>       ARM_FEATURE_M_SECURITY, /* M profile Security Extension */
>       ARM_FEATURE_M_MAIN, /* M profile Main Extension */
>       ARM_FEATURE_V8_1M, /* M profile extras only in v8.1M and later */
> +    ARM_FEATURE_PAUTH, /* has pointer authentication support */
>   };
>   
>   static inline int arm_feature(CPUARMState *env, int feature)
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index f42803ecaf1d..5a4386ce9218 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -760,10 +760,7 @@ static void aarch64_max_initfn(Object *obj)
>           cpu->ctr = 0x80038003; /* 32 byte I and D cacheline size, VIPT icache */
>           cpu->dcz_blocksize = 7; /*  512 bytes */
>   #endif
> -
> -        /* Default to PAUTH on, with the architected algorithm. */
> -        qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_property);
> -        qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_impdef_property);
> +        set_feature(&cpu->env, ARM_FEATURE_PAUTH);

I would rather you split out a function akin to aarch64_add_sve_properties, 
e.g. aarch64_add_pauth_properties.  We would call this function in any 
cpufoo_initfn that enables pauth.  It is hard to say more without the patch 
that adds the cpufoo_initfn which you are interested in.


> +    /* Default to PAUTH on, with the architected algorithm. */
> +    if (arm_feature(&cpu->env, ARM_FEATURE_PAUTH)) {

FWIW, this test is

     cpu_isar_feature(aa64_pauth, cpu)

without having to add ARM_FEATURE_PAUTH.


r~