Signed-off-by: Aaron Lindsay <aaron@os.amperecomputing.com>
---
target/arm/cpu.h | 5 +++
target/arm/cpu64.c | 81 ++++++++++++++++++++++++++++++++++++++--------
2 files changed, 72 insertions(+), 14 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 9c3cbc9a29..40b4631f11 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1039,6 +1039,11 @@ struct ArchCPU {
*/
bool prop_pauth;
bool prop_pauth_impdef;
+ bool prop_pauth_qarma3;
+ bool prop_pauth_epac;
+ bool prop_pauth2; // also known as EnhancedPAC2/EPAC2
+ bool prop_pauth_fpac;
+ bool prop_pauth_fpac_combine;
bool prop_lpa2;
/* DCZ blocksize, in log_2(words), ie low 4 bits of DCZID_EL0 */
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 0e021960fb..315acabbe2 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -590,8 +590,7 @@ static void aarch64_add_sme_properties(Object *obj)
void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp)
{
- int arch_val = 0, impdef_val = 0;
- uint64_t t;
+ int address_auth = 0, generic_auth = 0;
/* Exit early if PAuth is enabled, and fall through to disable it */
if ((kvm_enabled() || hvf_enabled()) && cpu->prop_pauth) {
@@ -603,30 +602,79 @@ void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp)
return;
}
- /* TODO: Handle HaveEnhancedPAC, HaveEnhancedPAC2, HaveFPAC. */
+ if (cpu->prop_pauth_epac &&
+ (cpu->prop_pauth2 ||
+ cpu->prop_pauth_fpac ||
+ cpu->prop_pauth_fpac_combine)) {
+ error_setg(errp, "'pauth-epac' feature not compatible with any of "
+ "'pauth-2', 'pauth-fpac', or 'pauth-fpac-combine'");
+ return;
+ }
+
+ /* Determine the PAC features independently of the algorithm */
+ if (cpu->prop_pauth_fpac_combine) {
+ address_auth = 0b0101;
+ } else if (cpu->prop_pauth_fpac) {
+ address_auth = 0b0100;
+ } else if (cpu->prop_pauth2) {
+ address_auth = 0b0011;
+ } else if (cpu->prop_pauth_epac) {
+ address_auth = 0b0010;
+ }
+
+ /* Write the features into the correct field for the algorithm in use */
if (cpu->prop_pauth) {
+ uint64_t t;
+
+ if (cpu->prop_pauth_impdef && cpu->prop_pauth_qarma3) {
+ error_setg(errp, "Cannot set both qarma3 ('pauth-qarma3') and "
+ "impdef ('pauth-impdef') pointer authentication ciphers");
+ return;
+ }
+
+ if (address_auth == 0)
+ address_auth = 0b0001;
+ generic_auth = 1;
+
if (cpu->prop_pauth_impdef) {
- impdef_val = 1;
+ t = cpu->isar.id_aa64isar1;
+ t = FIELD_DP64(t, ID_AA64ISAR1, API, address_auth);
+ t = FIELD_DP64(t, ID_AA64ISAR1, GPI, generic_auth);
+ cpu->isar.id_aa64isar1 = t;
+ } else if (cpu->prop_pauth_qarma3) {
+ t = cpu->isar.id_aa64isar2;
+ t = FIELD_DP64(t, ID_AA64ISAR2, APA3, address_auth);
+ t = FIELD_DP64(t, ID_AA64ISAR2, GPA3, generic_auth);
+ cpu->isar.id_aa64isar2 = t;
} else {
- arch_val = 1;
+ t = cpu->isar.id_aa64isar1;
+ t = FIELD_DP64(t, ID_AA64ISAR1, APA, address_auth);
+ t = FIELD_DP64(t, ID_AA64ISAR1, GPA, generic_auth);
+ cpu->isar.id_aa64isar1 = t;
}
- } else if (cpu->prop_pauth_impdef) {
- error_setg(errp, "cannot enable pauth-impdef without pauth");
+ } else if (cpu->prop_pauth_impdef || cpu->prop_pauth_qarma3) {
+ error_setg(errp, "cannot enable pauth-impdef or pauth-qarma3 without pauth");
+ error_append_hint(errp, "Add pauth=on to the CPU property list.\n");
+ } else if (address_auth != 0) {
+ error_setg(errp, "cannot enable any pauth* features without pauth");
error_append_hint(errp, "Add pauth=on to the CPU property list.\n");
}
-
- t = cpu->isar.id_aa64isar1;
- t = FIELD_DP64(t, ID_AA64ISAR1, APA, arch_val);
- t = FIELD_DP64(t, ID_AA64ISAR1, GPA, arch_val);
- t = FIELD_DP64(t, ID_AA64ISAR1, API, impdef_val);
- t = FIELD_DP64(t, ID_AA64ISAR1, GPI, impdef_val);
- cpu->isar.id_aa64isar1 = t;
}
static Property arm_cpu_pauth_property =
DEFINE_PROP_BOOL("pauth", ARMCPU, prop_pauth, true);
static Property arm_cpu_pauth_impdef_property =
DEFINE_PROP_BOOL("pauth-impdef", ARMCPU, prop_pauth_impdef, false);
+static Property arm_cpu_pauth_qarma3_property =
+ DEFINE_PROP_BOOL("pauth-qarma3", ARMCPU, prop_pauth_qarma3, false);
+static Property arm_cpu_pauth_epac_property =
+ DEFINE_PROP_BOOL("pauth-epac", ARMCPU, prop_pauth_epac, false);
+static Property arm_cpu_pauth2_property =
+ DEFINE_PROP_BOOL("pauth2", ARMCPU, prop_pauth2, false);
+static Property arm_cpu_pauth_fpac_property =
+ DEFINE_PROP_BOOL("pauth-fpac", ARMCPU, prop_pauth_fpac, false);
+static Property arm_cpu_pauth_fpac_combine_property =
+ DEFINE_PROP_BOOL("pauth-fpac-combine", ARMCPU, prop_pauth_fpac_combine, false);
static void aarch64_add_pauth_properties(Object *obj)
{
@@ -646,6 +694,11 @@ static void aarch64_add_pauth_properties(Object *obj)
cpu->prop_pauth = cpu_isar_feature(aa64_pauth, cpu);
} else {
qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_impdef_property);
+ qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_qarma3_property);
+ qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_epac_property);
+ qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth2_property);
+ qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_fpac_property);
+ qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_fpac_combine_property);
}
}
--
2.25.1On 2/22/23 09:35, Aaron Lindsay wrote:
> Signed-off-by: Aaron Lindsay <aaron@os.amperecomputing.com>
> ---
> target/arm/cpu.h | 5 +++
> target/arm/cpu64.c | 81 ++++++++++++++++++++++++++++++++++++++--------
> 2 files changed, 72 insertions(+), 14 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 9c3cbc9a29..40b4631f11 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1039,6 +1039,11 @@ struct ArchCPU {
> */
> bool prop_pauth;
> bool prop_pauth_impdef;
> + bool prop_pauth_qarma3;
> + bool prop_pauth_epac;
> + bool prop_pauth2; // also known as EnhancedPAC2/EPAC2
No c++ comments.
> + if (cpu->prop_pauth_epac &&
> + (cpu->prop_pauth2 ||
> + cpu->prop_pauth_fpac ||
> + cpu->prop_pauth_fpac_combine)) {
Indentation.
> + if (address_auth == 0)
> + address_auth = 0b0001;
Missing braces.
> +static Property arm_cpu_pauth2_property =
> + DEFINE_PROP_BOOL("pauth2", ARMCPU, prop_pauth2, false);
> +static Property arm_cpu_pauth_fpac_property =
> + DEFINE_PROP_BOOL("pauth-fpac", ARMCPU, prop_pauth_fpac, false);
> +static Property arm_cpu_pauth_fpac_combine_property =
> + DEFINE_PROP_BOOL("pauth-fpac-combine", ARMCPU, prop_pauth_fpac_combine, false);
For -cpu max, I would expect these to default on.
Or perhaps not expose these or epac as properties at all.
> @@ -646,6 +694,11 @@ static void aarch64_add_pauth_properties(Object *obj)
> cpu->prop_pauth = cpu_isar_feature(aa64_pauth, cpu);
> } else {
> qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_impdef_property);
> + qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_qarma3_property);
> + qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_epac_property);
> + qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth2_property);
> + qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_fpac_property);
> + qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_fpac_combine_property);
I think the *only* property that makes sense for KVM is pauth=on/off, which controls if
KVM exposes the key registers at all (and if off, APA/GPA/etc all get zeroed). There is
certainly no way to adjust the algorithm exposed by the hardware.
The primary reason we have a property for pauth at all is speed of emulation. When we
first enabled qarma5, we saw a major slowdown, with pauth_computepac consuming nearly 50%
of the entire runtime. Later we added impdef, as a way of doing *some* testing of pauth
without the extreme overhead of qarma5.
I see that qarma3 does about half the work of qarma5, so it would be interesting to
measure the relative speed of the 3 implementations on a boot of kernel + selftests.
You may want to look a the code generated and play with flatten and noinline attributes
around pauth_computepac and subroutines. E.g.
static uint64_t __attribute__((flatten, noinline))
pauth_computepac_qarma5(uint64_t data, uint64_t modifier, ARMPACKey key)
{
return pauth_computepac_architected(data, modifier, key, false);
}
static uint64_t __attribute__((flatten, noinline))
pauth_computepac_qarma3(uint64_t data, uint64_t modifier, ARMPACKey key)
{
return pauth_computepac_architected(data, modifier, key, true);
}
static uint64_t __attribute__((flatten, noinline))
pauth_computepac_impdef(uint64_t data, uint64_t modifier, ARMPACKey key)
{
return qemu_xxhash64_4(data, modifier, key.lo, key.hi);
}
static uint64_t pauth_computepac(CPUARMState *env, uint64_t data,
uint64_t modifier, ARMPACKey key)
{
if (cpu_isar_feature(aa64_pauth_arch_qarma5, env_archcpu(env))) {
return pauth_computepac_qarma5(data, modifier, key);
} else if (cpu_isar_feature(aa64_pauth_arch_qarma3, env_archcpu(env))) {
return pauth_computepac_qarma3(data, modifier, key);
} else {
return pauth_computepac_impdef(data, modifier, key);
}
}
r~
On Feb 22 12:14, Richard Henderson wrote:
> On 2/22/23 09:35, Aaron Lindsay wrote:
> > +static Property arm_cpu_pauth2_property =
> > + DEFINE_PROP_BOOL("pauth2", ARMCPU, prop_pauth2, false);
> > +static Property arm_cpu_pauth_fpac_property =
> > + DEFINE_PROP_BOOL("pauth-fpac", ARMCPU, prop_pauth_fpac, false);
> > +static Property arm_cpu_pauth_fpac_combine_property =
> > + DEFINE_PROP_BOOL("pauth-fpac-combine", ARMCPU, prop_pauth_fpac_combine, false);
>
> For -cpu max, I would expect these to default on.
> Or perhaps not expose these or epac as properties at all.
I've removed these properties, and epac's as well. It now defaults to
the equivalent of prop_pauth_fpac_combine==true in my previous patch.
> I see that qarma3 does about half the work of qarma5, so it would be
> interesting to measure the relative speed of the 3 implementations on a boot
> of kernel + selftests.
>
> You may want to look a the code generated and play with flatten and noinline
> attributes around pauth_computepac and subroutines. E.g.
>
> static uint64_t __attribute__((flatten, noinline))
> pauth_computepac_qarma5(uint64_t data, uint64_t modifier, ARMPACKey key)
> {
> return pauth_computepac_architected(data, modifier, key, false);
> }
>
> static uint64_t __attribute__((flatten, noinline))
> pauth_computepac_qarma3(uint64_t data, uint64_t modifier, ARMPACKey key)
> {
> return pauth_computepac_architected(data, modifier, key, true);
> }
>
> static uint64_t __attribute__((flatten, noinline))
> pauth_computepac_impdef(uint64_t data, uint64_t modifier, ARMPACKey key)
> {
> return qemu_xxhash64_4(data, modifier, key.lo, key.hi);
> }
>
> static uint64_t pauth_computepac(CPUARMState *env, uint64_t data,
> uint64_t modifier, ARMPACKey key)
> {
> if (cpu_isar_feature(aa64_pauth_arch_qarma5, env_archcpu(env))) {
> return pauth_computepac_qarma5(data, modifier, key);
> } else if (cpu_isar_feature(aa64_pauth_arch_qarma3, env_archcpu(env))) {
> return pauth_computepac_qarma3(data, modifier, key);
> } else {
> return pauth_computepac_impdef(data, modifier, key);
> }
> }
I have not played around with this further. Do you feel this is
important to look into prior to merging this patchset (since QARMA3
isn't the default)?
-Aaron
On 3/22/23 13:36, Aaron Lindsay wrote: > I have not played around with this further. Do you feel this is > important to look into prior to merging this patchset (since QARMA3 > isn't the default)? No, a mere curiosity. r~
On Wed, 22 Feb 2023 at 22:14, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 2/22/23 09:35, Aaron Lindsay wrote:
> > Signed-off-by: Aaron Lindsay <aaron@os.amperecomputing.com>
> > ---
> > +static Property arm_cpu_pauth2_property =
> > + DEFINE_PROP_BOOL("pauth2", ARMCPU, prop_pauth2, false);
> > +static Property arm_cpu_pauth_fpac_property =
> > + DEFINE_PROP_BOOL("pauth-fpac", ARMCPU, prop_pauth_fpac, false);
> > +static Property arm_cpu_pauth_fpac_combine_property =
> > + DEFINE_PROP_BOOL("pauth-fpac-combine", ARMCPU, prop_pauth_fpac_combine, false);
>
> For -cpu max, I would expect these to default on.
> Or perhaps not expose these or epac as properties at all.
Yes; unless there's a reason why we want the properties, we
shouldn't bother defining them. As Richard says, having
a 'pauth' property is a special case where we needed to be
able to avoid the massive extra emulation overhead of doing
it per the architected algorithm.
thanks
-- PMM
© 2016 - 2026 Red Hat, Inc.