diff mbox series

[v3,09/17] KVM: arm64: pauth: Use ctxt_sys_reg() instead of raw sys_regs access

Message ID 20200706125425.1671020-10-maz@kernel.org (mailing list archive)
State Mainlined
Commit 2c3db77c62b681af60757378219edcc93f658ba1
Headers show
Series KVM: arm64: Preliminary NV patches | expand

Commit Message

Marc Zyngier July 6, 2020, 12:54 p.m. UTC
Now that we have a wrapper for the sysreg accesses, let's use that
consistently.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/hyp/include/hyp/switch.h | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

Comments

Alexandru Elisei July 7, 2020, 1:05 p.m. UTC | #1
Hi,

On 7/6/20 1:54 PM, Marc Zyngier wrote:
> Now that we have a wrapper for the sysreg accesses, let's use that
> consistently.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/hyp/include/hyp/switch.h | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> index 7cf14e4f9f77..70367699d69a 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> @@ -364,11 +364,14 @@ static inline bool esr_is_ptrauth_trap(u32 esr)
>  	return false;
>  }
>  
> -#define __ptrauth_save_key(regs, key)						\
> -({										\
> -	regs[key ## KEYLO_EL1] = read_sysreg_s(SYS_ ## key ## KEYLO_EL1);	\
> -	regs[key ## KEYHI_EL1] = read_sysreg_s(SYS_ ## key ## KEYHI_EL1);	\
> -})
> +#define __ptrauth_save_key(ctxt, key)					\
> +	do {								\

Nitpick: the indentation for the do instruction doesn't match the indentation for
while(0).

> +	u64 __val;                                                      \
> +	__val = read_sysreg_s(SYS_ ## key ## KEYLO_EL1);                \
> +	ctxt_sys_reg(ctxt, key ## KEYLO_EL1) = __val;                   \
> +	__val = read_sysreg_s(SYS_ ## key ## KEYHI_EL1);                \
> +	ctxt_sys_reg(ctxt, key ## KEYHI_EL1) = __val;                   \
> +} while(0)
>  
>  static inline bool __hyp_handle_ptrauth(struct kvm_vcpu *vcpu)
>  {
> @@ -380,11 +383,11 @@ static inline bool __hyp_handle_ptrauth(struct kvm_vcpu *vcpu)
>  		return false;
>  
>  	ctxt = &__hyp_this_cpu_ptr(kvm_host_data)->host_ctxt;
> -	__ptrauth_save_key(ctxt->sys_regs, APIA);
> -	__ptrauth_save_key(ctxt->sys_regs, APIB);
> -	__ptrauth_save_key(ctxt->sys_regs, APDA);
> -	__ptrauth_save_key(ctxt->sys_regs, APDB);
> -	__ptrauth_save_key(ctxt->sys_regs, APGA);
> +	__ptrauth_save_key(ctxt, APIA);
> +	__ptrauth_save_key(ctxt, APIB);
> +	__ptrauth_save_key(ctxt, APDA);
> +	__ptrauth_save_key(ctxt, APDB);
> +	__ptrauth_save_key(ctxt, APGA);
>  
>  	vcpu_ptrauth_enable(vcpu);
>  

Looks good to me. I also grep'ed for sys_regs at the top level of the sources and
I didn't find any instances that we could replace with the accessors:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,
Alex
diff mbox series

Patch

diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 7cf14e4f9f77..70367699d69a 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -364,11 +364,14 @@  static inline bool esr_is_ptrauth_trap(u32 esr)
 	return false;
 }
 
-#define __ptrauth_save_key(regs, key)						\
-({										\
-	regs[key ## KEYLO_EL1] = read_sysreg_s(SYS_ ## key ## KEYLO_EL1);	\
-	regs[key ## KEYHI_EL1] = read_sysreg_s(SYS_ ## key ## KEYHI_EL1);	\
-})
+#define __ptrauth_save_key(ctxt, key)					\
+	do {								\
+	u64 __val;                                                      \
+	__val = read_sysreg_s(SYS_ ## key ## KEYLO_EL1);                \
+	ctxt_sys_reg(ctxt, key ## KEYLO_EL1) = __val;                   \
+	__val = read_sysreg_s(SYS_ ## key ## KEYHI_EL1);                \
+	ctxt_sys_reg(ctxt, key ## KEYHI_EL1) = __val;                   \
+} while(0)
 
 static inline bool __hyp_handle_ptrauth(struct kvm_vcpu *vcpu)
 {
@@ -380,11 +383,11 @@  static inline bool __hyp_handle_ptrauth(struct kvm_vcpu *vcpu)
 		return false;
 
 	ctxt = &__hyp_this_cpu_ptr(kvm_host_data)->host_ctxt;
-	__ptrauth_save_key(ctxt->sys_regs, APIA);
-	__ptrauth_save_key(ctxt->sys_regs, APIB);
-	__ptrauth_save_key(ctxt->sys_regs, APDA);
-	__ptrauth_save_key(ctxt->sys_regs, APDB);
-	__ptrauth_save_key(ctxt->sys_regs, APGA);
+	__ptrauth_save_key(ctxt, APIA);
+	__ptrauth_save_key(ctxt, APIB);
+	__ptrauth_save_key(ctxt, APDA);
+	__ptrauth_save_key(ctxt, APDB);
+	__ptrauth_save_key(ctxt, APGA);
 
 	vcpu_ptrauth_enable(vcpu);