Messages in this thread | | | From | David Matlack <> | Date | Tue, 7 Feb 2023 09:47:09 -0800 | Subject | Re: [Patch v2 2/5] KVM: x86/mmu: Optimize SPTE change flow for clear-dirty-log |
| |
On Tue, Feb 7, 2023 at 9:37 AM Vipin Sharma <vipinsh@google.com> wrote: > > On Mon, Feb 6, 2023 at 3:41 PM David Matlack <dmatlack@google.com> wrote: > > > > On Fri, Feb 03, 2023 at 11:28:19AM -0800, Vipin Sharma wrote: > > > > if (wrprot || spte_ad_need_write_protect(iter.old_spte)) > > clear_bits = PT_WRITABLE_MASK; > > else > > clear_bits = shadow_dirty_mask; > > > > if (!(iter->old_spte & clear_bits)) > > continue; > > > > iter.old_spte = kvm_tdp_mmu_clear_spte_bit(&iter, clear_bits); > > > > Yeah, this is better. Even better if I just initialize like: > > u64 clear_bits = shadow_dirty_mask; > > This will also get rid of the else part.
On that topic... Do we need to recalculate clear_bits for every spte? wrprot is passed as a parameter so that will not change. And spte_ad_need_write_protect() should either return true or false for all SPTEs in the TDP MMU. Specifically, make_spte() has this code:
if (sp->role.ad_disabled) spte |= SPTE_TDP_AD_DISABLED_MASK; else if (kvm_mmu_page_ad_need_write_protect(sp)) spte |= SPTE_TDP_AD_WRPROT_ONLY_MASK;
sp->role.ad_disabled is never modified in TDP MMU pages. So it should be constant for all pages. And kvm_mmu_page_ad_need_write_protect() will always return false for TDP MMU pages since sp->role.guest_mode is always false.
So this can just be:
u64 clear_bit = (wrprot || !kvm_ad_enabled()) ? PT_WRITABLE_MASK : shadow_dirty_mask;
| |