On 2/14/22 20:24, Sean Christopherson wrote:
On Mon, Feb 14, 2022, Paolo Bonzini wrote:
On 2/11/22 19:48, Sean Christopherson wrote:
On Wed, Feb 09, 2022, Paolo Bonzini wrote:
- kvm_mmu_unload(vcpu);
kvm_init_mmu(vcpu);
+ kvm_mmu_new_pgd(vcpu, vcpu->arch.cr3);
This is too risky IMO, there are far more flows than just MOV CR0/CR4 that are
affected, e.g. SMM transitions, KVM_SET_SREG, etc...
I'm not concerned about the TLB flush aspects so much as the addition of
kvm_mmu_new_pgd() in new paths.
Okay, yeah those are more complex and the existing ones are broken too.
- if ((cr0 ^ old_cr0) & KVM_MMU_CR0_ROLE_BITS)
+ if ((cr0 ^ old_cr0) & KVM_MMU_CR0_ROLE_BITS) {
+ /* Flush the TLB if CR0 is changed 1 -> 0. */
+ if ((old_cr0 & X86_CR0_PG) && !(cr0 & X86_CR0_PG))
+ kvm_mmu_unload(vcpu);
Calling kvm_mmu_unload() instead of requesting a flush isn't coherent with respect
to the comment, or with SMEP handling. And the SMEP handling isn't coherent with
respect to the changelog. Please elaborate :-)
Yep, will do (the CR0.PG=0 case is similar to the CR0.PCIDE=0 case below).
Oh, you're freeing all roots to ensure a future MOV CR3 with NO_FLUSH and PCIDE=1
can't reuse a stale root. That's necessary if and only if the MMU is shadowing
the guest, non-nested TDP MMUs just need to flush the guest's TLB. The same is
true for the PCIDE case, i.e. we could optimize that too, though the main motivation
would be to clarify why all roots are unloaded.
Yes. Clarifying all this should be done before the big change to
kvm_mmu_reset_context().
Using kvm_mmu_unload() avoids loading a cached root just to throw it away
immediately after,
The shadow paging case will throw it away, but not the non-nested TDP MMU case?
Yes, the TDP case is okay since the role is the same. kvm_init_mmu() is
enough.
but I can change this to a new KVM_REQ_MMU_UPDATE_ROOT flag that does
kvm_mmu_new_pgd(vcpu, vcpu->arch.cr3);
I don't think that's necessary, I was just confused by the discrepancy.
It may not be necessary but it is clearer IMO. Let me post a new patch.
Paolo
