Messages in this thread | | | Date | Fri, 10 Dec 2021 17:13:29 +0100 | Subject | Re: [PATCH 1/7] KVM: x86: Retry page fault if MMU reload is pending and root has no sp | From | Paolo Bonzini <> |
| |
On 12/10/21 17:01, Sean Christopherson wrote: >> KVM_REQ_MMU_RELOAD is raised after kvm->arch.mmu_valid_gen is fixed (of >> course, otherwise the other CPU might just not see any obsoleted page >> from the legacy MMU), therefore any check on KVM_REQ_MMU_RELOAD is just >> advisory. > > I disagree. IMO, KVM should not be installing SPTEs into obsolete shadow pages, > which is what continuing on allows. I don't _think_ it's problematic, but I do > think it's wrong. > > [...] Eh, for all intents and purposes, KVM_REQ_MMU_RELOAD very much says > special roots are obsolete. The root will be unloaded, i.e. will no > longer be used, i.e. is obsolete.
I understand that---but it takes some unspoken details to understand that. In particular that both kvm_reload_remote_mmus and is_page_fault_stale are called under mmu_lock write-lock, and that there's no unlock between updating mmu_valid_gen and calling kvm_reload_remote_mmus.
(This also suggests, for the other six patches, keeping kvm_reload_remote_mmus and just moving it to arch/x86/kvm/mmu/mmu.c, with an assertion that the MMU lock is held for write).
But since we have a way forward for having no special roots to worry about, it seems an unnecessary overload for 1) a patch that will last one or two releasees at most 2) a case that has been handled in the inefficient way forever.
Paolo
> The other way to check for an invalid special root would be to treat > it as obsolete if any of its children in entries 0-3 are present and > obsolete. That would be more precise, but it provides no benefit > given KVM's current implementation. > > I'm not completely opposed to doing nothing, but I do think it's > silly to continue on knowing that the work done by the page fault is > all but gauranteed to be useless. >
| |