| Message ID | 20200422120050.3693593-21-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Preliminary NV patches | expand |
Hi Marc, On 22/04/2020 13:00, Marc Zyngier wrote: > As ELR-EL1 is a VNCR-capable register with ARMv8.4-NV, let's move it to > the sys_regs array and repaint the accessors. While we're at it, let's > kill the now useless accessors used only on the fault injection path. Reviewed-by: James Morse <james.morse@arm.com> A curiosity: > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 95977b80265ce..46949fce3e813 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -184,6 +184,8 @@ enum vcpu_sysreg { Comment above the enum has some claims about the order, but its already out of order with __vcpu_read_sys_reg_from_cpu()... (PAR_EL1 being the culprit) (I think it only matters for searching by encoding, which is checked at boot.) > APGAKEYLO_EL1, > APGAKEYHI_EL1, > > + ELR_EL1, > + > /* 32bit specific registers. Keep them at the end of the range */ > DACR32_EL2, /* Domain Access Control Register */ > IFSR32_EL2, /* Instruction Fault Status Register */ > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 5bda4af36a0e7..7c2fffb20c217 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -95,6 +95,7 @@ static bool __vcpu_read_sys_reg_from_cpu(int reg, u64 *val) > case TPIDR_EL1: *val = read_sysreg_s(SYS_TPIDR_EL1); break; > case AMAIR_EL1: *val = read_sysreg_s(SYS_AMAIR_EL12); break; > case CNTKCTL_EL1: *val = read_sysreg_s(SYS_CNTKCTL_EL12); break; > + case ELR_EL1: *val = read_sysreg_s(SYS_ELR_EL12); break; > case PAR_EL1: *val = read_sysreg_s(SYS_PAR_EL1); break; > case DACR32_EL2: *val = read_sysreg_s(SYS_DACR32_EL2); break; > case IFSR32_EL2: *val = read_sysreg_s(SYS_IFSR32_EL2); break; Thanks, James
On 2020-05-26 17:29, James Morse wrote: > Hi Marc, > > On 22/04/2020 13:00, Marc Zyngier wrote: >> As ELR-EL1 is a VNCR-capable register with ARMv8.4-NV, let's move it >> to >> the sys_regs array and repaint the accessors. While we're at it, let's >> kill the now useless accessors used only on the fault injection path. > > Reviewed-by: James Morse <james.morse@arm.com> > > > A curiosity: > >> diff --git a/arch/arm64/include/asm/kvm_host.h >> b/arch/arm64/include/asm/kvm_host.h >> index 95977b80265ce..46949fce3e813 100644 >> --- a/arch/arm64/include/asm/kvm_host.h >> +++ b/arch/arm64/include/asm/kvm_host.h >> @@ -184,6 +184,8 @@ enum vcpu_sysreg { > > Comment above the enum has some claims about the order, but its > already out of order with > __vcpu_read_sys_reg_from_cpu()... (PAR_EL1 being the culprit) This comment dates back from the original assembly implementation, where I was paranoid about accessing the sys_regs array in strict order to maximize the chances of the prefetcher doing the right thing. As always with premature optimization, it was worthless, and moving things to C forced us to do things differently anyway (not to mention VHE...). I'll delete the comment in a separate patch. Thanks, M.
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h index c0d9363f328dc..1b8a396528851 100644 --- a/arch/arm64/include/asm/kvm_emulate.h +++ b/arch/arm64/include/asm/kvm_emulate.h @@ -133,27 +133,6 @@ static __always_inline unsigned long *vcpu_pc(const struct kvm_vcpu *vcpu) return (unsigned long *)&vcpu_gp_regs(vcpu)->pc; } -static inline unsigned long *__vcpu_elr_el1(const struct kvm_vcpu *vcpu) -{ - return (unsigned long *)&vcpu->arch.ctxt.elr_el1; -} - -static inline unsigned long vcpu_read_elr_el1(const struct kvm_vcpu *vcpu) -{ - if (vcpu->arch.sysregs_loaded_on_cpu) - return read_sysreg_el1(SYS_ELR); - else - return *__vcpu_elr_el1(vcpu); -} - -static inline void vcpu_write_elr_el1(const struct kvm_vcpu *vcpu, unsigned long v) -{ - if (vcpu->arch.sysregs_loaded_on_cpu) - write_sysreg_el1(v, SYS_ELR); - else - *__vcpu_elr_el1(vcpu) = v; -} - static __always_inline unsigned long *vcpu_cpsr(const struct kvm_vcpu *vcpu) { return (unsigned long *)&vcpu_gp_regs(vcpu)->pstate; diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 95977b80265ce..46949fce3e813 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -184,6 +184,8 @@ enum vcpu_sysreg { APGAKEYLO_EL1, APGAKEYHI_EL1, + ELR_EL1, + /* 32bit specific registers. Keep them at the end of the range */ DACR32_EL2, /* Domain Access Control Register */ IFSR32_EL2, /* Instruction Fault Status Register */ @@ -238,7 +240,6 @@ struct kvm_cpu_context { struct user_pt_regs regs; /* sp = sp_el0 */ u64 sp_el1; - u64 elr_el1; u64 spsr[KVM_NR_SPSR]; diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index 9fec9231b63e2..ab99fe91c16d0 100644 --- a/arch/arm64/kvm/guest.c +++ b/arch/arm64/kvm/guest.c @@ -126,7 +126,7 @@ static void *core_reg_addr(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) return &vcpu->arch.ctxt.sp_el1; case KVM_REG_ARM_CORE_REG(elr_el1): - return &vcpu->arch.ctxt.elr_el1; + return __ctxt_sys_reg(&vcpu->arch.ctxt, ELR_EL1); case KVM_REG_ARM_CORE_REG(spsr[0]) ... KVM_REG_ARM_CORE_REG(spsr[KVM_NR_SPSR - 1]): diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c index d9dd19f39a537..1d862b81eb70a 100644 --- a/arch/arm64/kvm/hyp/sysreg-sr.c +++ b/arch/arm64/kvm/hyp/sysreg-sr.c @@ -62,7 +62,7 @@ static void __hyp_text __sysreg_save_el1_state(struct kvm_cpu_context *ctxt) ctxt_sys_reg(ctxt, TPIDR_EL1) = read_sysreg(tpidr_el1); ctxt->sp_el1 = read_sysreg(sp_el1); - ctxt->elr_el1 = read_sysreg_el1(SYS_ELR); + ctxt_sys_reg(ctxt, ELR_EL1) = read_sysreg_el1(SYS_ELR); ctxt->spsr[KVM_SPSR_EL1] = read_sysreg_el1(SYS_SPSR); } @@ -167,7 +167,7 @@ static void __hyp_text __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt) } write_sysreg(ctxt->sp_el1, sp_el1); - write_sysreg_el1(ctxt->elr_el1, SYS_ELR); + write_sysreg_el1(ctxt_sys_reg(ctxt, ELR_EL1), SYS_ELR); write_sysreg_el1(ctxt->spsr[KVM_SPSR_EL1], SYS_SPSR); } diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c index 6aafc2825c1cb..d3ebf8bca4b89 100644 --- a/arch/arm64/kvm/inject_fault.c +++ b/arch/arm64/kvm/inject_fault.c @@ -116,7 +116,7 @@ static void inject_abt64(struct kvm_vcpu *vcpu, bool is_iabt, unsigned long addr bool is_aarch32 = vcpu_mode_is_32bit(vcpu); u32 esr = 0; - vcpu_write_elr_el1(vcpu, *vcpu_pc(vcpu)); + vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL1); *vcpu_pc(vcpu) = get_except_vector(vcpu, except_type_sync); *vcpu_cpsr(vcpu) = get_except64_pstate(vcpu); @@ -151,7 +151,7 @@ static void inject_undef64(struct kvm_vcpu *vcpu) unsigned long cpsr = *vcpu_cpsr(vcpu); u32 esr = (ESR_ELx_EC_UNKNOWN << ESR_ELx_EC_SHIFT); - vcpu_write_elr_el1(vcpu, *vcpu_pc(vcpu)); + vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL1); *vcpu_pc(vcpu) = get_except_vector(vcpu, except_type_sync); *vcpu_cpsr(vcpu) = get_except64_pstate(vcpu); diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 5bda4af36a0e7..7c2fffb20c217 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -95,6 +95,7 @@ static bool __vcpu_read_sys_reg_from_cpu(int reg, u64 *val) case TPIDR_EL1: *val = read_sysreg_s(SYS_TPIDR_EL1); break; case AMAIR_EL1: *val = read_sysreg_s(SYS_AMAIR_EL12); break; case CNTKCTL_EL1: *val = read_sysreg_s(SYS_CNTKCTL_EL12); break; + case ELR_EL1: *val = read_sysreg_s(SYS_ELR_EL12); break; case PAR_EL1: *val = read_sysreg_s(SYS_PAR_EL1); break; case DACR32_EL2: *val = read_sysreg_s(SYS_DACR32_EL2); break; case IFSR32_EL2: *val = read_sysreg_s(SYS_IFSR32_EL2); break; @@ -135,6 +136,7 @@ static bool __vcpu_write_sys_reg_to_cpu(u64 val, int reg) case TPIDR_EL1: write_sysreg_s(val, SYS_TPIDR_EL1); break; case AMAIR_EL1: write_sysreg_s(val, SYS_AMAIR_EL12); break; case CNTKCTL_EL1: write_sysreg_s(val, SYS_CNTKCTL_EL12); break; + case ELR_EL1: write_sysreg_s(val, SYS_ELR_EL12); break; case PAR_EL1: write_sysreg_s(val, SYS_PAR_EL1); break; case DACR32_EL2: write_sysreg_s(val, SYS_DACR32_EL2); break; case IFSR32_EL2: write_sysreg_s(val, SYS_IFSR32_EL2); break;
As ELR-EL1 is a VNCR-capable register with ARMv8.4-NV, let's move it to the sys_regs array and repaint the accessors. While we're at it, let's kill the now useless accessors used only on the fault injection path. Signed-off-by: Marc Zyngier <maz@kernel.org> --- arch/arm64/include/asm/kvm_emulate.h | 21 --------------------- arch/arm64/include/asm/kvm_host.h | 3 ++- arch/arm64/kvm/guest.c | 2 +- arch/arm64/kvm/hyp/sysreg-sr.c | 4 ++-- arch/arm64/kvm/inject_fault.c | 4 ++-- arch/arm64/kvm/sys_regs.c | 2 ++ 6 files changed, 9 insertions(+), 27 deletions(-)