On Fri, 2024-04-26 at 08:28 -0700, Sean Christopherson wrote:
> Hmm, yeah, I buy that argument. We could even harded further by poisoning '0'
> to force KVM to explicitly. Aha! And maybe use a bitmap?
>
> enum {
> BUGGY_KVM_INVALIDATION = 0,
> PROCESS_SHARED = BIT(0),
> PROCESS_PRIVATE = BIT(1),
> PROCESS_PRIVATE_AND_SHARED = PROCESS_SHARED |
> PROCESS_PRIVATE,
> };
Seems like it would work for all who have been concerned. The previous objection
to the enum (can't find the mail) was for requiring logic like:
if (zap == PROCESS_PRIVATE_AND_SHARED || zap == PROCESS_PRIVATE)
do_private_zap_stuff();
We are trying to tie things up internally so we can jointly have something to
stare at again, as the patches are diverging. But will make this adjustment.
>
> > > https://lore.kernel.org/all/ZUO1Giju0GkUdF0o@xxxxxxxxxx/
> >
> > Currently in our internal branch we switched to:
> > exclude_private
> > exclude_shared
> >
> > It came together bettter in the code that uses it.
>
> If the choice is between an enum and exclude_*, I would strongly prefer the
> enum.
> Using exclude_* results in inverted polarity for the code that triggers
> invalidations.
Right, the awkwardness lands in that code.
The processing code looks nice though:
https://lore.kernel.org/kvm/5210e6e6e2eb73b04cb7039084015612479ae2fe.camel@xxxxxxxxx/
>
> > But I started to wonder if we actually really need exclude_shared. For TDX
> > zapping private memory has to be done with more care, because it cannot be
> > re-
> > populated without guest coordination. But for shared memory if we are
> > zapping a
> > range that includes both private and shared memory, I don't think it should
> > hurt
> > to zap the shared memory.
>
> Hell no, I am not risking taking on more baggage in KVM where userspace or
> some
> other subsystem comes to rely on KVM spuriously zapping SPTEs in response to
> an
> unrelated userspace action.
Hmm, I see the point. Thanks. This was just being left for later discussion
anyway.
