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

[Richard Henderson]

On 5/24/21 8:29 PM, Richard Henderson wrote:
> 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.

Alternately, the bug is that the pauth properties have not been registered for 
the new cpu.  Given the performance overhead of the QARMA cipher under TCG, it 
will always make sense to be able to disable it.


r~