On Mon, Jul 19, 2021 at 05:03:37PM +0100, Fuad Tabba wrote: > On deactivating traps, restore the value of mdcr_el2 from the > newly created and preserved host value vcpu context, rather than > directly reading the hardware register. > > Up until and including this patch the two values are the same, > i.e., the hardware register and the vcpu one. A future patch will > be changing the value of mdcr_el2 on activating traps, and this > ensures that its value will be restored. > > No functional change intended.
I'm probably missing something, but I can't convince myself that the host will end up with the same mdcr_el2 value after deactivating traps after this patch as before. We clearly now restore whatever we had when activating traps (presumably whatever we configured at init_el2_state time), but is that equivalent to what we had before with the masking and ORing that this patch drops? Thanks, drew > > Signed-off-by: Fuad Tabba <[email protected]> > --- > arch/arm64/include/asm/kvm_host.h | 5 ++++- > arch/arm64/include/asm/kvm_hyp.h | 2 +- > arch/arm64/kvm/hyp/include/hyp/switch.h | 6 +++++- > arch/arm64/kvm/hyp/nvhe/switch.c | 11 ++--------- > arch/arm64/kvm/hyp/vhe/switch.c | 12 ++---------- > arch/arm64/kvm/hyp/vhe/sysreg-sr.c | 2 +- > 6 files changed, 15 insertions(+), 23 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_host.h > b/arch/arm64/include/asm/kvm_host.h > index 4d2d974c1522..76462c6a91ee 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -287,10 +287,13 @@ struct kvm_vcpu_arch { > /* Stage 2 paging state used by the hardware on next switch */ > struct kvm_s2_mmu *hw_mmu; > > - /* HYP configuration */ > + /* Values of trap registers for the guest. */ > u64 hcr_el2; > u64 mdcr_el2; > > + /* Values of trap registers for the host before guest entry. */ > + u64 mdcr_el2_host; > + > /* Exception Information */ > struct kvm_vcpu_fault_info fault; > > diff --git a/arch/arm64/include/asm/kvm_hyp.h > b/arch/arm64/include/asm/kvm_hyp.h > index 9d60b3006efc..657d0c94cf82 100644 > --- a/arch/arm64/include/asm/kvm_hyp.h > +++ b/arch/arm64/include/asm/kvm_hyp.h > @@ -95,7 +95,7 @@ void __sve_restore_state(void *sve_pffr, u32 *fpsr); > > #ifndef __KVM_NVHE_HYPERVISOR__ > void activate_traps_vhe_load(struct kvm_vcpu *vcpu); > -void deactivate_traps_vhe_put(void); > +void deactivate_traps_vhe_put(struct kvm_vcpu *vcpu); > #endif > > u64 __guest_enter(struct kvm_vcpu *vcpu); > diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h > b/arch/arm64/kvm/hyp/include/hyp/switch.h > index e4a2f295a394..a0e78a6027be 100644 > --- a/arch/arm64/kvm/hyp/include/hyp/switch.h > +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h > @@ -92,11 +92,15 @@ static inline void __activate_traps_common(struct > kvm_vcpu *vcpu) > write_sysreg(0, pmselr_el0); > write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0); > } > + > + vcpu->arch.mdcr_el2_host = read_sysreg(mdcr_el2); > write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2); > } > > -static inline void __deactivate_traps_common(void) > +static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu) > { > + write_sysreg(vcpu->arch.mdcr_el2_host, mdcr_el2); > + > write_sysreg(0, hstr_el2); > if (kvm_arm_support_pmu_v3()) > write_sysreg(0, pmuserenr_el0); > diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c > b/arch/arm64/kvm/hyp/nvhe/switch.c > index f7af9688c1f7..1778593a08a9 100644 > --- a/arch/arm64/kvm/hyp/nvhe/switch.c > +++ b/arch/arm64/kvm/hyp/nvhe/switch.c > @@ -69,12 +69,10 @@ static void __activate_traps(struct kvm_vcpu *vcpu) > static void __deactivate_traps(struct kvm_vcpu *vcpu) > { > extern char __kvm_hyp_host_vector[]; > - u64 mdcr_el2, cptr; > + u64 cptr; > > ___deactivate_traps(vcpu); > > - mdcr_el2 = read_sysreg(mdcr_el2); > - > if (cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT)) { > u64 val; > > @@ -92,13 +90,8 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu) > isb(); > } > > - __deactivate_traps_common(); > - > - mdcr_el2 &= MDCR_EL2_HPMN_MASK; > - mdcr_el2 |= MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT; > - mdcr_el2 |= MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT; > + __deactivate_traps_common(vcpu); > > - write_sysreg(mdcr_el2, mdcr_el2); > write_sysreg(this_cpu_ptr(&kvm_init_params)->hcr_el2, hcr_el2); > > cptr = CPTR_EL2_DEFAULT; > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c > index b3229924d243..0d0c9550fb08 100644 > --- a/arch/arm64/kvm/hyp/vhe/switch.c > +++ b/arch/arm64/kvm/hyp/vhe/switch.c > @@ -91,17 +91,9 @@ void activate_traps_vhe_load(struct kvm_vcpu *vcpu) > __activate_traps_common(vcpu); > } > > -void deactivate_traps_vhe_put(void) > +void deactivate_traps_vhe_put(struct kvm_vcpu *vcpu) > { > - u64 mdcr_el2 = read_sysreg(mdcr_el2); > - > - mdcr_el2 &= MDCR_EL2_HPMN_MASK | > - MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT | > - MDCR_EL2_TPMS; > - > - write_sysreg(mdcr_el2, mdcr_el2); > - > - __deactivate_traps_common(); > + __deactivate_traps_common(vcpu); > } > > /* Switch to the guest for VHE systems running in EL2 */ > diff --git a/arch/arm64/kvm/hyp/vhe/sysreg-sr.c > b/arch/arm64/kvm/hyp/vhe/sysreg-sr.c > index 2a0b8c88d74f..007a12dd4351 100644 > --- a/arch/arm64/kvm/hyp/vhe/sysreg-sr.c > +++ b/arch/arm64/kvm/hyp/vhe/sysreg-sr.c > @@ -101,7 +101,7 @@ void kvm_vcpu_put_sysregs_vhe(struct kvm_vcpu *vcpu) > struct kvm_cpu_context *host_ctxt; > > host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt; > - deactivate_traps_vhe_put(); > + deactivate_traps_vhe_put(vcpu); > > __sysreg_save_el1_state(guest_ctxt); > __sysreg_save_user_state(guest_ctxt); > -- > 2.32.0.402.g57bb445576-goog > _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
