On Mon, Jun 22, 2020 at 11:39:32AM +0100, Andrew Scull wrote: > On Mon, Jun 22, 2020 at 10:15:08AM +0100, Mark Rutland wrote: > > On Mon, Jun 22, 2020 at 09:06:43AM +0100, Marc Zyngier wrote: > > > > > --- 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. > > Is this relibale for this sort of application? The description just > sounds like a counter of macros rather than specifically a unique label > generator. It may work most of the time but also seems that it has the > potential to be more fragile given that it would change based on the > rest of the code in the file to potentially conflict with something it > didn't previously conflict with. Ah, you invoke a macro in order for the label to be generated so it will increment and the label is namespaced by the prefix. I see.
