| Message ID | 20210719160346.609914-7-tabba@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Fixed features for protected VMs | expand |
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 <tabba@google.com> > --- > 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 >
Hi Drew, On Tue, Jul 20, 2021 at 3:53 PM Andrew Jones <drjones@redhat.com> wrote: > > 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? You're right. I thought that these were actually being initialized to the same values, but having a closer look at the code the mdcr values are not the same as pre-patch. I will fix this. Thanks! /fuad > Thanks, > drew > > > > > Signed-off-by: Fuad Tabba <tabba@google.com> > > --- > > 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 > > >
On Wed, Jul 21, 2021 at 08:37:21AM +0100, Fuad Tabba wrote: > On Tue, Jul 20, 2021 at 3:53 PM Andrew Jones <drjones@redhat.com> wrote: > > > > 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? > > You're right. I thought that these were actually being initialized to > the same values, but having a closer look at the code the mdcr values > are not the same as pre-patch. I will fix this. Can you elaborate on the issue here, please? I was just looking at this but aren't you now relying on __init_el2_debug to configure this, which should be fine? Will
Hi Will, On Thu, Aug 12, 2021 at 10:46 AM Will Deacon <will@kernel.org> wrote: > > On Wed, Jul 21, 2021 at 08:37:21AM +0100, Fuad Tabba wrote: > > On Tue, Jul 20, 2021 at 3:53 PM Andrew Jones <drjones@redhat.com> wrote: > > > > > > 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? > > > > You're right. I thought that these were actually being initialized to > > the same values, but having a closer look at the code the mdcr values > > are not the same as pre-patch. I will fix this. > > Can you elaborate on the issue here, please? I was just looking at this > but aren't you now relying on __init_el2_debug to configure this, which > should be fine? I *think* that it should be fine, but as Drew pointed out, the host does not end up with the same mdcr_el2 value after the deactivation in this patch as it did after deactivation before this patch. In my v4 (not sent out yet), I have fixed it to ensure that the host does end up with the same value as the one before this patch. That should make it easier to check that there's no functional change. I'll look into it further, and if I can convince myself that there aren't any issues and that this patch makes the code cleaner, I will add it as a separate patch instead to make reviewing easier. Thanks, /fuad > Will
Hey Fuad, On Thu, Aug 12, 2021 at 11:28:50AM +0200, Fuad Tabba wrote: > On Thu, Aug 12, 2021 at 10:46 AM Will Deacon <will@kernel.org> wrote: > > > > On Wed, Jul 21, 2021 at 08:37:21AM +0100, Fuad Tabba wrote: > > > On Tue, Jul 20, 2021 at 3:53 PM Andrew Jones <drjones@redhat.com> wrote: > > > > > > > > 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? > > > > > > You're right. I thought that these were actually being initialized to > > > the same values, but having a closer look at the code the mdcr values > > > are not the same as pre-patch. I will fix this. > > > > Can you elaborate on the issue here, please? I was just looking at this > > but aren't you now relying on __init_el2_debug to configure this, which > > should be fine? > > I *think* that it should be fine, but as Drew pointed out, the host > does not end up with the same mdcr_el2 value after the deactivation in > this patch as it did after deactivation before this patch. In my v4 > (not sent out yet), I have fixed it to ensure that the host does end > up with the same value as the one before this patch. That should make > it easier to check that there's no functional change. > > I'll look into it further, and if I can convince myself that there > aren't any issues and that this patch makes the code cleaner, I will > add it as a separate patch instead to make reviewing easier. Cheers. I think the new code might actually be better, as things like MDCR_EL2.E2PB are RES0 if SPE is not implemented. The init code takes care to set those only if if probes SPE first, whereas the code you're removing doesn't seem to check that. Will
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);
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. Signed-off-by: Fuad Tabba <tabba@google.com> --- 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(-)