| Message ID | 20200706125425.1671020-10-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Preliminary NV patches | expand |
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 --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);
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(-)