[PATCH v2 0/5] KVM/arm64: Enable PtrAuth on non-VHE KVM

[PATCH v2 0/5] KVM/arm64: Enable PtrAuth on non-VHE KVM

[Marc Zyngier]

Not having PtrAuth on non-VHE KVM (for whatever reason VHE is not
enabled on a v8.3 system) has always looked like an oddity. This
trivial series remedies it, and allows a non-VHE KVM to offer PtrAuth
to its guests.

In the tradition of not having separate security between host-EL1 and
EL2, EL2 reuses the keys set up by host-EL1. It is likely that, should
we switch to a mode where EL2 is more distrusting of EL1, we'd have
private keys there.

The last two patches are respectively an optimization when
save/restoring the PtrAuth context, and a cleanup of the alternatives
used by that same save/restore code.

* From v1 [1]:
  - Move the hand-crafted literal load to using a mov_q macro (Andrew, Mark)
  - Added a cleanup of the alternatives on the save/restore path (Mark)

[1] https://lore.kernel.org/kvm/20200615081954.6233-1-maz@kernel.org/

Marc Zyngier (5):
  KVM: arm64: Enable Address Authentication at EL2 if available
  KVM: arm64: Allow ARM64_PTR_AUTH when ARM64_VHE=n
  KVM: arm64: Allow PtrAuth to be enabled from userspace on non-VHE
    systems
  KVM: arm64: Check HCR_EL2 instead of shadow copy to swap PtrAuth
    registers
  KVM: arm64: Simplify PtrAuth alternative patching

 arch/arm64/Kconfig                   |  4 +---
 arch/arm64/include/asm/kvm_ptrauth.h | 30 ++++++++++------------------
 arch/arm64/kvm/hyp-init.S            |  5 +++++
 arch/arm64/kvm/reset.c               | 21 ++++++++++---------
 4 files changed, 27 insertions(+), 33 deletions(-)

-- 
2.27.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v2 1/5] KVM: arm64: Enable Address Authentication at EL2 if available

[Marc Zyngier]

While initializing EL2, enable Address Authentication if detected
from EL1. We still use the EL1-provided keys though.

Acked-by: Andrew Scull <ascull@google.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/hyp-init.S | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/kvm/hyp-init.S b/arch/arm64/kvm/hyp-init.S
index 6e6ed5581eed..1587d146726a 100644
--- a/arch/arm64/kvm/hyp-init.S
+++ b/arch/arm64/kvm/hyp-init.S
@@ -104,6 +104,11 @@ alternative_else_nop_endif
 	 */
 	mov_q	x4, (SCTLR_EL2_RES1 | (SCTLR_ELx_FLAGS & ~SCTLR_ELx_A))
 CPU_BE(	orr	x4, x4, #SCTLR_ELx_EE)
+alternative_if ARM64_HAS_ADDRESS_AUTH
+	mov_q	x5, (SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | \
+		     SCTLR_ELx_ENDA | SCTLR_ELx_ENDB)
+	orr	x4, x4, x5
+alternative_else_nop_endif
 	msr	sctlr_el2, x4
 	isb
 
-- 
2.27.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 1/5] KVM: arm64: Enable Address Authentication at EL2 if available

[Mark Rutland]

On Mon, Jun 22, 2020 at 09:06:39AM +0100, Marc Zyngier wrote:
> While initializing EL2, enable Address Authentication if detected
> from EL1. We still use the EL1-provided keys though.
> 
> Acked-by: Andrew Scull <ascull@google.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> ---
>  arch/arm64/kvm/hyp-init.S | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm64/kvm/hyp-init.S b/arch/arm64/kvm/hyp-init.S
> index 6e6ed5581eed..1587d146726a 100644
> --- a/arch/arm64/kvm/hyp-init.S
> +++ b/arch/arm64/kvm/hyp-init.S
> @@ -104,6 +104,11 @@ alternative_else_nop_endif
>  	 */
>  	mov_q	x4, (SCTLR_EL2_RES1 | (SCTLR_ELx_FLAGS & ~SCTLR_ELx_A))
>  CPU_BE(	orr	x4, x4, #SCTLR_ELx_EE)
> +alternative_if ARM64_HAS_ADDRESS_AUTH
> +	mov_q	x5, (SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | \
> +		     SCTLR_ELx_ENDA | SCTLR_ELx_ENDB)
> +	orr	x4, x4, x5
> +alternative_else_nop_endif
>  	msr	sctlr_el2, x4
>  	isb
>  
> -- 
> 2.27.0
> 
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v2 2/5] KVM: arm64: Allow ARM64_PTR_AUTH when ARM64_VHE=n

[Marc Zyngier]

We currently prevent PtrAuth from even being built if KVM is selected,
but VHE isn't. It is a bit of a pointless restriction, since we also
check this at run time (rejecting the enabling of PtrAuth for the
vcpu if we're not running with VHE).

Just drop this apparently useless restriction.

Acked-by: Andrew Scull <ascull@google.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/Kconfig | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 31380da53689..d719ea9c596d 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1516,7 +1516,6 @@ menu "ARMv8.3 architectural features"
 config ARM64_PTR_AUTH
 	bool "Enable support for pointer authentication"
 	default y
-	depends on !KVM || ARM64_VHE
 	depends on (CC_HAS_SIGN_RETURN_ADDRESS || CC_HAS_BRANCH_PROT_PAC_RET) && AS_HAS_PAC
 	# GCC 9.1 and later inserts a .note.gnu.property section note for PAC
 	# which is only understood by binutils starting with version 2.33.1.
@@ -1543,8 +1542,7 @@ config ARM64_PTR_AUTH
 
 	  The feature is detected at runtime. If the feature is not present in
 	  hardware it will not be advertised to userspace/KVM guest nor will it
-	  be enabled. However, KVM guest also require VHE mode and hence
-	  CONFIG_ARM64_VHE=y option to use this feature.
+	  be enabled.
 
 	  If the feature is present on the boot CPU but not on a late CPU, then
 	  the late CPU will be parked. Also, if the boot CPU does not have
-- 
2.27.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v2 3/5] KVM: arm64: Allow PtrAuth to be enabled from userspace on non-VHE systems

[Marc Zyngier]

Now that the scene is set for enabling PtrAuth on non-VHE, drop
the restrictions preventing userspace from enabling it.

Acked-by: Andrew Scull <ascull@google.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/reset.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index d3b209023727..2a929789fe2e 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -42,6 +42,11 @@ static u32 kvm_ipa_limit;
 #define VCPU_RESET_PSTATE_SVC	(PSR_AA32_MODE_SVC | PSR_AA32_A_BIT | \
 				 PSR_AA32_I_BIT | PSR_AA32_F_BIT)
 
+static bool system_has_full_ptr_auth(void)
+{
+	return system_supports_address_auth() && system_supports_generic_auth();
+}
+
 /**
  * kvm_arch_vm_ioctl_check_extension
  *
@@ -80,8 +85,7 @@ int kvm_arch_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 		break;
 	case KVM_CAP_ARM_PTRAUTH_ADDRESS:
 	case KVM_CAP_ARM_PTRAUTH_GENERIC:
-		r = has_vhe() && system_supports_address_auth() &&
-				 system_supports_generic_auth();
+		r = system_has_full_ptr_auth();
 		break;
 	default:
 		r = 0;
@@ -205,19 +209,14 @@ static void kvm_vcpu_reset_sve(struct kvm_vcpu *vcpu)
 
 static int kvm_vcpu_enable_ptrauth(struct kvm_vcpu *vcpu)
 {
-	/* Support ptrauth only if the system supports these capabilities. */
-	if (!has_vhe())
-		return -EINVAL;
-
-	if (!system_supports_address_auth() ||
-	    !system_supports_generic_auth())
-		return -EINVAL;
 	/*
 	 * For now make sure that both address/generic pointer authentication
-	 * features are requested by the userspace together.
+	 * features are requested by the userspace together and the system
+	 * supports these capabilities.
 	 */
 	if (!test_bit(KVM_ARM_VCPU_PTRAUTH_ADDRESS, vcpu->arch.features) ||
-	    !test_bit(KVM_ARM_VCPU_PTRAUTH_GENERIC, vcpu->arch.features))
+	    !test_bit(KVM_ARM_VCPU_PTRAUTH_GENERIC, vcpu->arch.features) ||
+	    !system_has_full_ptr_auth())
 		return -EINVAL;
 
 	vcpu->arch.flags |= KVM_ARM64_GUEST_HAS_PTRAUTH;
-- 
2.27.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v2 4/5] KVM: arm64: Check HCR_EL2 instead of shadow copy to swap PtrAuth registers

[Marc Zyngier]

When save/restoring PtrAuth registers between host and guest, it is
pretty useless to fetch the in-memory state, while we have the right
state in the HCR_EL2 system register. Use that instead.

Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/include/asm/kvm_ptrauth.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_ptrauth.h b/arch/arm64/include/asm/kvm_ptrauth.h
index 6301813dcace..f1830173fa9e 100644
--- a/arch/arm64/include/asm/kvm_ptrauth.h
+++ b/arch/arm64/include/asm/kvm_ptrauth.h
@@ -74,7 +74,7 @@ alternative_if_not ARM64_HAS_ADDRESS_AUTH_IMP_DEF
 	b	1001f
 alternative_else_nop_endif
 1000:
-	ldr	\reg1, [\g_ctxt, #(VCPU_HCR_EL2 - VCPU_CONTEXT)]
+	mrs	\reg1, hcr_el2
 	and	\reg1, \reg1, #(HCR_API | HCR_APK)
 	cbz	\reg1, 1001f
 	add	\reg1, \g_ctxt, #CPU_APIAKEYLO_EL1
@@ -90,7 +90,7 @@ alternative_if_not ARM64_HAS_ADDRESS_AUTH_IMP_DEF
 	b	2001f
 alternative_else_nop_endif
 2000:
-	ldr	\reg1, [\g_ctxt, #(VCPU_HCR_EL2 - VCPU_CONTEXT)]
+	mrs	\reg1, hcr_el2
 	and	\reg1, \reg1, #(HCR_API | HCR_APK)
 	cbz	\reg1, 2001f
 	add	\reg1, \g_ctxt, #CPU_APIAKEYLO_EL1
-- 
2.27.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

[Marc Zyngier]

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@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 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
 
 .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

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

[Mark Rutland]

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@arm.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>

Looks good to me. One minor suggestion below, but either way:

Acked-by: Mark Rutland <mark.rutland@arm.com>

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

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

[Marc Zyngier]

Hi Mark,

On 2020-06-22 10:15, Mark Rutland wrote:
> 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@arm.com>
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
> 
> Looks good to me. One minor suggestion below, but either way:
> 
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> 
>> ---
>>  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.

I have folded in the following patch:

diff --git a/arch/arm64/include/asm/kvm_ptrauth.h 
b/arch/arm64/include/asm/kvm_ptrauth.h
index 7a72508a841b..0ddf98c3ba9f 100644
--- a/arch/arm64/include/asm/kvm_ptrauth.h
+++ b/arch/arm64/include/asm/kvm_ptrauth.h
@@ -68,29 +68,29 @@
   */
  .macro ptrauth_switch_to_guest g_ctxt, reg1, reg2, reg3
  alternative_if_not ARM64_HAS_ADDRESS_AUTH
-	b	1000f
+	b	.L__skip_switch\@
  alternative_else_nop_endif
  	mrs	\reg1, hcr_el2
  	and	\reg1, \reg1, #(HCR_API | HCR_APK)
-	cbz	\reg1, 1000f
+	cbz	\reg1, .L__skip_switch\@
  	add	\reg1, \g_ctxt, #CPU_APIAKEYLO_EL1
  	ptrauth_restore_state	\reg1, \reg2, \reg3
-1000:
+.L__skip_switch\@:
  .endm

  .macro ptrauth_switch_to_host g_ctxt, h_ctxt, reg1, reg2, reg3
  alternative_if_not ARM64_HAS_ADDRESS_AUTH
-	b	2000f
+	b	.L__skip_switch\@
  alternative_else_nop_endif
  	mrs	\reg1, hcr_el2
  	and	\reg1, \reg1, #(HCR_API | HCR_APK)
-	cbz	\reg1, 2000f
+	cbz	\reg1, .L__skip_switch\@
  	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
-2000:
+.L__skip_switch\@:
  .endm

  #else /* !CONFIG_ARM64_PTR_AUTH */


Thanks,

          M.
-- 
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

[Mark Rutland]

On Mon, Jun 22, 2020 at 11:25:41AM +0100, Marc Zyngier wrote:
> On 2020-06-22 10:15, Mark Rutland wrote:
> > On Mon, Jun 22, 2020 at 09:06:43AM +0100, Marc Zyngier wrote:
> I have folded in the following patch:
> 
> diff --git a/arch/arm64/include/asm/kvm_ptrauth.h
> b/arch/arm64/include/asm/kvm_ptrauth.h
> index 7a72508a841b..0ddf98c3ba9f 100644
> --- a/arch/arm64/include/asm/kvm_ptrauth.h
> +++ b/arch/arm64/include/asm/kvm_ptrauth.h
> @@ -68,29 +68,29 @@
>   */
>  .macro ptrauth_switch_to_guest g_ctxt, reg1, reg2, reg3
>  alternative_if_not ARM64_HAS_ADDRESS_AUTH
> -	b	1000f
> +	b	.L__skip_switch\@
>  alternative_else_nop_endif
>  	mrs	\reg1, hcr_el2
>  	and	\reg1, \reg1, #(HCR_API | HCR_APK)
> -	cbz	\reg1, 1000f
> +	cbz	\reg1, .L__skip_switch\@
>  	add	\reg1, \g_ctxt, #CPU_APIAKEYLO_EL1
>  	ptrauth_restore_state	\reg1, \reg2, \reg3
> -1000:
> +.L__skip_switch\@:
>  .endm
> 
>  .macro ptrauth_switch_to_host g_ctxt, h_ctxt, reg1, reg2, reg3
>  alternative_if_not ARM64_HAS_ADDRESS_AUTH
> -	b	2000f
> +	b	.L__skip_switch\@
>  alternative_else_nop_endif
>  	mrs	\reg1, hcr_el2
>  	and	\reg1, \reg1, #(HCR_API | HCR_APK)
> -	cbz	\reg1, 2000f
> +	cbz	\reg1, .L__skip_switch\@
>  	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
> -2000:
> +.L__skip_switch\@:
>  .endm

Looks good to me; thanks!

Mark.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

[Andrew Scull]

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

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

[Andrew Scull]

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