On Fri, 2022-02-25 at 18:52 +0000, Sean Christopherson wrote: > On Fri, Feb 25, 2022, David Woodhouse wrote: > > > > I think I'm going to rip out the 'dirty' flag from the gfn_to_pfn_cache > > > > completely and add a function (to be called with an active vCPU > > > > context) which marks the page dirty *now*. > > > > > > Hrm, something like? > > > > > > 1. Drop @dirty from kvm_gfn_to_pfn_cache_init() > > > 2. Rename @dirty => @old_dirty in kvm_gfn_to_pfn_cache_refresh() > > > 3. Add an API to mark the associated slot dirty without unmapping > > > > > > I think that makes sense. > > > > Except I'll drop 'dirty' from kvm_gfn_to_pfn_cache_refresh() too. > > There's no scope for a deferred "oh, I meant to tell you that was > > dirty" even in that case, is there? Use the API we add in your #3. > > But won't we end up with a bunch of call sites that attempt to determine whether > not the dirty status needs to be flushed? I'm specifically thinking of scenarios > where the status needs to be conditionally flushed, e.g. if the backing pfn doesn't > change, then it's ok to not mark the page dirty. Not handling that in the refresh > helper will either lead to unnecessary dirtying or duplicate code/work. I don't know that it's ever OK to leave the dirty status cached in the GPC to be flushed at some unspecified "later" time. Once the vCPU exits back to userspace, that might be the *last* time it's ever run, and we have the same ABI issue as (snipped) above, that userspace might have done its final read of the dirty log, and might not expect anything else to be marked dirty. We might be able to defer it until we return to userspace, but no further. So I don't see flushing it in *refresh* as being useful? And we *can't* flush it in invalidate anyway, as discussed.
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
