Re: [PATCH v2 5/5] KVM: arm64: Simplify PtrAuth alternative patching

[Andrew Scull <ascull@google.com>]

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.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm