On Mon, Jun 22, 2020 at 09:06:43AM +0100, Marc Zyngier wrote: > We currently decide to execute the PtrAuth save/restore code based > on a set of branches that evaluate as (ARM64_HAS_ADDRESS_AUTH_ARCH || > ARM64_HAS_ADDRESS_AUTH_IMP_DEF). This can be easily replaced by > a much simpler test as the ARM64_HAS_ADDRESS_AUTH capability is > exactly this expression. > > Suggested-by: Mark Rutland <mark.rutland@xxxxxxx> > Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx> Looks good to me. One minor suggestion below, but either way: Acked-by: Mark Rutland <mark.rutland@xxxxxxx> > --- > arch/arm64/include/asm/kvm_ptrauth.h | 26 +++++++++----------------- > 1 file changed, 9 insertions(+), 17 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_ptrauth.h b/arch/arm64/include/asm/kvm_ptrauth.h > index f1830173fa9e..7a72508a841b 100644 > --- a/arch/arm64/include/asm/kvm_ptrauth.h > +++ b/arch/arm64/include/asm/kvm_ptrauth.h > @@ -61,44 +61,36 @@ > > /* > * Both ptrauth_switch_to_guest and ptrauth_switch_to_host macros will > - * check for the presence of one of the cpufeature flag > - * ARM64_HAS_ADDRESS_AUTH_ARCH or ARM64_HAS_ADDRESS_AUTH_IMP_DEF and > + * check for the presence ARM64_HAS_ADDRESS_AUTH, which is defined as > + * (ARM64_HAS_ADDRESS_AUTH_ARCH || ARM64_HAS_ADDRESS_AUTH_IMP_DEF) and > * then proceed ahead with the save/restore of Pointer Authentication > - * key registers. > + * key registers if enabled for the guest. > */ > .macro ptrauth_switch_to_guest g_ctxt, reg1, reg2, reg3 > -alternative_if ARM64_HAS_ADDRESS_AUTH_ARCH > +alternative_if_not ARM64_HAS_ADDRESS_AUTH > b 1000f > alternative_else_nop_endif > -alternative_if_not ARM64_HAS_ADDRESS_AUTH_IMP_DEF > - b 1001f > -alternative_else_nop_endif > -1000: > mrs \reg1, hcr_el2 > and \reg1, \reg1, #(HCR_API | HCR_APK) > - cbz \reg1, 1001f > + cbz \reg1, 1000f > add \reg1, \g_ctxt, #CPU_APIAKEYLO_EL1 > ptrauth_restore_state \reg1, \reg2, \reg3 > -1001: > +1000: > .endm Since these are in macros, we could use \@ to generate a macro-specific lavel rather than a magic number, which would be less likely to conflict with the surrounding environment and would be more descriptive. We do that in a few places already, and here it could look something like: | alternative_if_not ARM64_HAS_ADDRESS_AUTH | b .L__skip_pauth_switch\@ | alternative_else_nop_endif | | ... | | .L__skip_pauth_switch\@: Per the gas documentation | \@ | | as maintains a counter of how many macros it has executed in this | pseudo-variable; you can copy that number to your output with ‘\@’, | but only within a macro definition. No worries if you don't want to change that now; the Acked-by stands either way. Mark. > > .macro ptrauth_switch_to_host g_ctxt, h_ctxt, reg1, reg2, reg3 > -alternative_if ARM64_HAS_ADDRESS_AUTH_ARCH > +alternative_if_not ARM64_HAS_ADDRESS_AUTH > b 2000f > alternative_else_nop_endif > -alternative_if_not ARM64_HAS_ADDRESS_AUTH_IMP_DEF > - b 2001f > -alternative_else_nop_endif > -2000: > mrs \reg1, hcr_el2 > and \reg1, \reg1, #(HCR_API | HCR_APK) > - cbz \reg1, 2001f > + cbz \reg1, 2000f > add \reg1, \g_ctxt, #CPU_APIAKEYLO_EL1 > ptrauth_save_state \reg1, \reg2, \reg3 > add \reg1, \h_ctxt, #CPU_APIAKEYLO_EL1 > ptrauth_restore_state \reg1, \reg2, \reg3 > isb > -2001: > +2000: > .endm > > #else /* !CONFIG_ARM64_PTR_AUTH */ > -- > 2.27.0 >
