| Message ID | 20231012205422.3924618-3-oliver.upton@linux.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Load stage-2 in vcpu_load() on VHE | expand |
On 2023/10/13 4:54, Oliver Upton wrote: > From: Marc Zyngier <maz@kernel.org> > > An MMU notifier could cause us to clobber the stage-2 context loaded on > a CPU when we switch to another VM's context to invalidate. This isn't > an issue right now as the stage-2 context gets reloaded on every guest > entry, but is disastrous when moving __load_stage2() into the > vcpu_load() path. > > Restore the previous stage-2 context on the way out of a TLB > invalidation if we installed something else. Deliberately do this after > TGE=1 is synchronized to keep things safe in light of the speculative AT > errata. > > Signed-off-by: Marc Zyngier <maz@kernel.org> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev> > --- > arch/arm64/kvm/hyp/vhe/tlb.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/kvm/hyp/vhe/tlb.c b/arch/arm64/kvm/hyp/vhe/tlb.c > index f3f2e142e4f4..ef21153ce5fa 100644 > --- a/arch/arm64/kvm/hyp/vhe/tlb.c > +++ b/arch/arm64/kvm/hyp/vhe/tlb.c > @@ -11,18 +11,25 @@ > #include <asm/tlbflush.h> > > struct tlb_inv_context { > - unsigned long flags; > - u64 tcr; > - u64 sctlr; > + struct kvm_s2_mmu *mmu; > + unsigned long flags; > + u64 tcr; > + u64 sctlr; > }; > > static void __tlb_switch_to_guest(struct kvm_s2_mmu *mmu, > struct tlb_inv_context *cxt) > { > + struct kvm_vcpu *vcpu = kvm_get_running_vcpu(); > u64 val; > > local_irq_save(cxt->flags); > > + if (vcpu && mmu != vcpu->arch.hw_mmu) > + cxt->mmu = mmu; Shouldn't this be cxt->mm = vcpu->arch.hw_mmu (the "previous" S2 context)? Thanks, Zenghui
On Wed, Oct 18, 2023 at 03:00:42PM +0800, Zenghui Yu wrote: > On 2023/10/13 4:54, Oliver Upton wrote: > > From: Marc Zyngier <maz@kernel.org> > > > > An MMU notifier could cause us to clobber the stage-2 context loaded on > > a CPU when we switch to another VM's context to invalidate. This isn't > > an issue right now as the stage-2 context gets reloaded on every guest > > entry, but is disastrous when moving __load_stage2() into the > > vcpu_load() path. > > > > Restore the previous stage-2 context on the way out of a TLB > > invalidation if we installed something else. Deliberately do this after > > TGE=1 is synchronized to keep things safe in light of the speculative AT > > errata. > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > Signed-off-by: Oliver Upton <oliver.upton@linux.dev> > > --- > > arch/arm64/kvm/hyp/vhe/tlb.c | 17 ++++++++++++++--- > > 1 file changed, 14 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm64/kvm/hyp/vhe/tlb.c b/arch/arm64/kvm/hyp/vhe/tlb.c > > index f3f2e142e4f4..ef21153ce5fa 100644 > > --- a/arch/arm64/kvm/hyp/vhe/tlb.c > > +++ b/arch/arm64/kvm/hyp/vhe/tlb.c > > @@ -11,18 +11,25 @@ > > #include <asm/tlbflush.h> > > struct tlb_inv_context { > > - unsigned long flags; > > - u64 tcr; > > - u64 sctlr; > > + struct kvm_s2_mmu *mmu; > > + unsigned long flags; > > + u64 tcr; > > + u64 sctlr; > > }; > > static void __tlb_switch_to_guest(struct kvm_s2_mmu *mmu, > > struct tlb_inv_context *cxt) > > { > > + struct kvm_vcpu *vcpu = kvm_get_running_vcpu(); > > u64 val; > > local_irq_save(cxt->flags); > > + if (vcpu && mmu != vcpu->arch.hw_mmu) > > + cxt->mmu = mmu; > > Shouldn't this be > > cxt->mm = vcpu->arch.hw_mmu (the "previous" S2 context)? It absolutely should, and Marc had it right the first time.
diff --git a/arch/arm64/kvm/hyp/vhe/tlb.c b/arch/arm64/kvm/hyp/vhe/tlb.c index f3f2e142e4f4..ef21153ce5fa 100644 --- a/arch/arm64/kvm/hyp/vhe/tlb.c +++ b/arch/arm64/kvm/hyp/vhe/tlb.c @@ -11,18 +11,25 @@ #include <asm/tlbflush.h> struct tlb_inv_context { - unsigned long flags; - u64 tcr; - u64 sctlr; + struct kvm_s2_mmu *mmu; + unsigned long flags; + u64 tcr; + u64 sctlr; }; static void __tlb_switch_to_guest(struct kvm_s2_mmu *mmu, struct tlb_inv_context *cxt) { + struct kvm_vcpu *vcpu = kvm_get_running_vcpu(); u64 val; local_irq_save(cxt->flags); + if (vcpu && mmu != vcpu->arch.hw_mmu) + cxt->mmu = mmu; + else + cxt->mmu = NULL; + if (cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT)) { /* * For CPUs that are affected by ARM errata 1165522 or 1530923, @@ -69,6 +76,10 @@ static void __tlb_switch_to_host(struct tlb_inv_context *cxt) write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2); isb(); + /* ... and the stage-2 MMU context that we switched away from */ + if (cxt->mmu) + __load_stage2(cxt->mmu, cxt->mmu->arch); + if (cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT)) { /* Restore the registers to what they were */ write_sysreg_el1(cxt->tcr, SYS_TCR);