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

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

Acked-by: Mark Rutland <mark.rutl...@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

Reply via email to