| From | Paolo Bonzini <> | Subject | Re: [PATCH 09/54] KVM: x86/mmu: Unconditionally zap unsync SPs when creating >4k SP at GFN | Date | Wed, 23 Jun 2021 16:36:50 +0200 |
| |
On 22/06/21 19:56, Sean Christopherson wrote: > When creating a new upper-level shadow page, zap unsync shadow pages at > the same target gfn instead of attempting to sync the pages. This fixes > a bug where an unsync shadow page could be sync'd with an incompatible > context, e.g. wrong smm, is_guest, etc... flags. In practice, the bug is > relatively benign as sync_page() is all but guaranteed to fail its check > that the guest's desired gfn (for the to-be-sync'd page) matches the > current gfn associated with the shadow page. I.e. kvm_sync_page() would > end up zapping the page anyways. > > Alternatively, __kvm_sync_page() could be modified to explicitly verify > the mmu_role of the unsync shadow page is compatible with the current MMU > context. But, except for this specific case, __kvm_sync_page() is called > iff the page is compatible, e.g. the transient sync in kvm_mmu_get_page() > requires an exact role match, and the call from kvm_sync_mmu_roots() is > only synchronizing shadow pages from the current MMU (which better be > compatible or KVM has problems). And as described above, attempting to > sync shadow pages when creating an upper-level shadow page is unlikely > to succeed, e.g. zero successful syncs were observed when running Linux > guests despite over a million attempts.
One issue, this WARN_ON may now trigger:
WARN_ON(!list_empty(&invalid_list));
due to a kvm_mmu_prepare_zap_page that could have happened on an earlier iteration of the for_each_valid_sp. Before your change, __kvm_sync_page would be called always before kvm_sync_pages could add anything to invalid_list.
Paolo
|