Re: [RFC PATCH v6 037/104] KVM: x86/mmu: Allow non-zero value for non-present SPTE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Aug 05, 2022, David Matlack wrote:
> On Thu, Aug 4, 2022 at 5:04 PM Huang, Kai <kai.huang@xxxxxxxxx> wrote:
> >
> > > > In addition to the suggestions above, I'd suggest breaking this patch
> > > > up, since it is doing multiple things:
> > > >
> > > > 1. Patch initialize shadow page tables to EMPTY_SPTE (0) and
> > > >    replace TDP MMU hard-coded 0 with EMPTY_SPTE.
> > > > 2. Patch to change FNAME(sync_page) to not assume EMPTY_SPTE is 0.
> > > > 3. Patch to set bit 63 in EMPTY_SPTE.
> > > > 4. Patch to set bit 63 in REMOVED_SPTE.
> >
> > I think 1/2 can be separate patches, but 3/4 should be done together.
> >
> > Patch 3 alone is broken as when TDP MMU zaps SPTE and replaces it with
> > REMOVED_SPTE, it loses bit 63.  This is not what we want.  We always want
> > bit 63 set if it is supposed to be  set to a non-present SPTE.
> 
> How is patch 3 alone be broken? The TDX support that depends on bit 63
> does not exist at this point in the series, i.e. setting bit 63 is
> entirely optional and only done in preparation for future patches.

Hmm, I agree with Kai on this specific point.  Will it cause functional problems?
No.  Is KVM technically broken?  IMO, yes, because the intent of the code is to
ensure bit 63 is set for all SPTEs that are not-present (and not misconfigured)
in hardware.

I 100% agree on patches doing too much, but in this particular case it's easy to
capture the above semantics in a shortlog:

  KVM: x86/mmu: Set bit 63 (EPT's SUPPRESS_VE) in all not-present SPTEs



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux