[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZivIF9vjKcuGie3s@google.com>
Date: Fri, 26 Apr 2024 08:28:23 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Rick P Edgecombe <rick.p.edgecombe@...el.com>
Cc: "tabba@...gle.com" <tabba@...gle.com>, Isaku Yamahata <isaku.yamahata@...el.com>,
Tina Zhang <tina.zhang@...el.com>, Kai Huang <kai.huang@...el.com>,
"binbin.wu@...ux.intel.com" <binbin.wu@...ux.intel.com>, Bo2 Chen <chen.bo@...el.com>,
"sagis@...gle.com" <sagis@...gle.com>,
"isaku.yamahata@...ux.intel.com" <isaku.yamahata@...ux.intel.com>,
"isaku.yamahata@...il.com" <isaku.yamahata@...il.com>, Erdem Aktas <erdemaktas@...gle.com>,
"kvm@...r.kernel.org" <kvm@...r.kernel.org>, "pbonzini@...hat.com" <pbonzini@...hat.com>,
Hang Yuan <hang.yuan@...el.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v19 011/130] KVM: Add new members to struct kvm_gfn_range
to operate on
On Fri, Apr 26, 2024, Rick P Edgecombe wrote:
> On Fri, 2024-04-26 at 08:39 +0100, Fuad Tabba wrote:
> > > I'm fine with those names. Anyway, I'm fine with wither way, two bools or
> > > enum.
> >
> > I don't have a strong opinion, but I'd brought it up in a previous
> > patch series. I think that having two bools to encode three states is
> > less intuitive and potentially more bug prone, more so than the naming
> > itself (i.e., _only):
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,
};
> > https://lore.kernel.org/all/ZUO1Giju0GkUdF0o@google.com/
>
> 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.
> 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.
Powered by blists - more mailing lists