Re: [EXTERNAL] [PATCH v2] KVM: Don't actually set a request when evicting vCPUs for GFN cache invd

David Woodhouse
2022-02-25T20:16:00Z

[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]

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 #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]