On 12/10/21 17:01, Sean Christopherson wrote:

quoted

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.