Re: [PATCH 1/2] target/arm: Add cpu property to control pauth

[Andrew Jones]

On Wed, Aug 12, 2020 at 08:10:47AM -0700, Richard Henderson wrote:
> On 8/12/20 4:00 AM, Andrew Jones wrote:
> > On Tue, Aug 11, 2020 at 11:53:38PM -0700, Richard Henderson wrote:
> >> The crypto overhead of emulating pauth can be significant for
> >> some workloads.  Add an enumeration property that allows the
> >> feature to be turned off, on with the architected algorithm,
> >> or on with an implementation defined algorithm.
> >>
> >> The architected algorithm is quite expensive to emulate;
> >> using another algorithm may allow hardware acceleration.
> >>
> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >>  target/arm/cpu64.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 64 insertions(+)
> >>
> >> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> >> index dd696183df..3181d0e2f8 100644
> >> --- a/target/arm/cpu64.c
> >> +++ b/target/arm/cpu64.c
> >> @@ -572,6 +572,69 @@ void aarch64_add_sve_properties(Object *obj)
> >>      }
> >>  }
> >>  
> >> +static const char * const pauth_names[] = {
> >> +    "off", "impdef", "arch"
> >> +};
> > 
> > Hi Richard,
> > 
> > Please add three boolean properties, rather than one enum:
> > 
> > pauth:            enable support of the pauth feature
> > pauth-fast:       enable QEMU's fast non-cryptographic hash for pauth
> >                   (pauth must be enabled)
> > pauth-arch:       enable the architected algorithm for pauth
> >                   (pauth must be enabled)
> > 
> > These booleans can then be added to the cpu feature probing list used by
> > qmp_query_cpu_model_expansion()
> 
> Why are 3 booleans better than one enum?
> 
> I'd forgotten about qmp_query_cpu_model_expansion(); can it not take anything
> but booleans?

Right. The probing works by getting a list of possible CPU features, which
are all boolean properties. That way the prober can try enabling/disabling
them without having to know about each property's set of valid values. We
could implement each as an enum and a second level of probing, but that
would complicate the probing, and I'm not sure enums gain us much over
multiple properties.

In this case, since pauth-fast and pauth-arch are mutually exclusive and
we want a pauth=on/off too, then we'll need a finalize function like SVE
has in order to support the following selections:

 # Default (pauth-arch), explicitly selected or not
 -cpu max[,pauth=on]
 -cpu max[,pauth=on][,pauth-fast=off],pauth-arch=on

 # Select pauth-fast
 -cpu max[,pauth=on][,pauth-arch=off],pauth-fast=on

 # Disable
 -cpu max,pauth=off
 -cpu max[,pauth=off],pauth-arch=off,pauth-fast=off

 # Mutual exclusion errors
 -cpu max,pauth=off,pauth-{arch,fast}=on
 -cpu max,pauth=on,pauth-arch=off,pauth-fast=off
 -cpu max[,pauth=on],pauth-arch=on,pauth-fast=on

 # Errors because we don't want to guess what the user means
 -cpu max[,pauth=on],pauth-arch=off
 -cpu max[,pauth=on],pauth-fast=off
 

Thanks,
drew