Re: [PATCH] target/arm: don't clobber ID_AA64ISAR1 pointer auth

[Richard Henderson]

On 5/24/21 1:43 AM, Jamie Iles wrote:
> The pointer auth properties are added to the max CPU type but the
> finalization happens for all CPUs.  It makes sense to be able to disable
> pointer authentication for the max CPU type, but for future CPUs that
> implement pointer authentication and have bits set in ID_AA64ISAR1,
> don't clobber them unless there is a property registered that can
> disable them.
>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Jamie Iles <jamie@nuviainc.com>
> ---
>   target/arm/cpu64.c | 33 +++++++++++++++++++++------------
>   1 file changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index f0a9e968c9c1..81c9e494acb6 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -575,26 +575,31 @@ void aarch64_add_sve_properties(Object *obj)
>   
>   void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp)
>   {
> -    int arch_val = 0, impdef_val = 0;
> +    int apa = FIELD_EX64(cpu->isar.id_aa64isar1, ID_AA64ISAR1, APA);
> +    int gpa = FIELD_EX64(cpu->isar.id_aa64isar1, ID_AA64ISAR1, GPA);
> +    int api = FIELD_EX64(cpu->isar.id_aa64isar1, ID_AA64ISAR1, API);
> +    int gpi = FIELD_EX64(cpu->isar.id_aa64isar1, ID_AA64ISAR1, GPI);
>       uint64_t t;
>   
> +    if (object_property_find(OBJECT(cpu), "pauth-impdef")) {
> +        api = gpi = cpu->prop_pauth_impdef;
> +    }
> +
> +    if (object_property_find(OBJECT(cpu), "pauth")) {
> +        apa = gpa = cpu->prop_pauth;
> +    }

This seems overly complex.  If the pauth property doesn't exist, can you just 
early exit from the function?  And surely the pauth-impdef properly would exist 
if and only if pauth does.


r~