On Mon, Jan 30, 2023 at 10:09 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > On Sat, Jan 28, 2023, Vipin Sharma wrote: > > On Wed, Jan 25, 2023 at 5:49 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > -static void handle_changed_spte(struct kvm *kvm, int as_id, gfn_t gfn, > > > - u64 old_spte, u64 new_spte, int level, > > > - bool shared) > > > -{ > > > - __handle_changed_spte(kvm, as_id, gfn, old_spte, new_spte, level, > > > - shared); > > > handle_changed_spte_acc_track(old_spte, new_spte, level); > > > - handle_changed_spte_dirty_log(kvm, as_id, gfn, old_spte, > > > - new_spte, level); > > > + > > > + /* COMMENT GOES HERE. */ > > > > Current "shared" callers are not making a page dirty. If a new > > "shared" caller makes a page dirty then make sure > > handle_changed_spte_dirty_log is called. > > > > How is this? > > I was hoping for a more definitive "rule" than "KVM doesn't currently do XYZ". > > > > + if (!shared) > > > + handle_changed_spte_dirty_log(kvm, as_id, gfn, old_spte, > > > + new_spte, level); > > > } > > > What if implementation is changed a little more? I can think of two options: Option 1: Remove handle_changed_spte_dirty_log() and let the callers handle mark_page_dirty_in_slot(). Similar to how fast page faults do this. This will get rid of the "shared" variable and defining its rules for the shared and nonshared flow. Option 2: Changing meaning of this variable from "shared" to something like "handle_dirty_log" Callers will know if they want dirty log to be handled or not. I am preferring option 1.