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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
> 



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux