On Wed, 2022-08-03 at 21:33 +0000, Sean Christopherson wrote: > Add compile-time and init-time sanity checks to ensure that the MMIO SPTE > mask doesn't overlap the MMIO SPTE generation. The generation currently > avoids using bit 63, but that's as much coincidence as it is strictly > necessarly. That will change in the future, as TDX support will require > setting bit 63 (SUPPRESS_VE) in the mask. Explicitly carve out the bits > that are allowed in the mask so that any future shuffling of SPTE MMIO > bits doesn't silently break MMIO caching. Reviwed-by: Kai Huang <kai.huang@xxxxxxxxx> Btw, should you also check SPTE_MMU_PRESENT_MASK (or in another patch)? > > Suggested-by: Kai Huang <kai.huang@xxxxxxxxx> Thanks for giving me the credit as I don't feel I deserve it. > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx> > --- > arch/x86/kvm/mmu/spte.c | 8 ++++++++ > arch/x86/kvm/mmu/spte.h | 9 +++++++++ > 2 files changed, 17 insertions(+) > > diff --git a/arch/x86/kvm/mmu/spte.c b/arch/x86/kvm/mmu/spte.c > index 7314d27d57a4..08e8c46f3037 100644 > --- a/arch/x86/kvm/mmu/spte.c > +++ b/arch/x86/kvm/mmu/spte.c > @@ -343,6 +343,14 @@ void kvm_mmu_set_mmio_spte_mask(u64 mmio_value, u64 mmio_mask, u64 access_mask) > if (!enable_mmio_caching) > mmio_value = 0; > > + /* > + * The mask must contain only bits that are carved out specifically for > + * the MMIO SPTE mask, e.g. to ensure there's no overlap with the MMIO > + * generation. > + */ > + if (WARN_ON(mmio_mask & ~SPTE_MMIO_ALLOWED_MASK)) > + mmio_value = 0; > + > /* > * Disable MMIO caching if the MMIO value collides with the bits that > * are used to hold the relocated GFN when the L1TF mitigation is > diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h > index cabe3fbb4f39..6cd3936cbe1f 100644 > --- a/arch/x86/kvm/mmu/spte.h > +++ b/arch/x86/kvm/mmu/spte.h > @@ -125,6 +125,15 @@ static_assert(!(EPT_SPTE_MMU_WRITABLE & SHADOW_ACC_TRACK_SAVED_MASK)); > static_assert(!(SPTE_MMU_PRESENT_MASK & > (MMIO_SPTE_GEN_LOW_MASK | MMIO_SPTE_GEN_HIGH_MASK))); > > +/* > + * The SPTE MMIO mask is allowed to use "present" bits (i.e. all EPT RWX bits), > + * all physical address bits (additional checks ensure the mask doesn't overlap > + * legal PA bits), and bit 63 (carved out for future usage, e.g. SUPPRESS_VE). > + */ > +#define SPTE_MMIO_ALLOWED_MASK (BIT_ULL(63) | GENMASK_ULL(51, 12) | GENMASK_ULL(2, 0)) > +static_assert(!(SPTE_MMIO_ALLOWED_MASK & > + (MMIO_SPTE_GEN_LOW_MASK | MMIO_SPTE_GEN_HIGH_MASK))); > + > #define MMIO_SPTE_GEN_LOW_BITS (MMIO_SPTE_GEN_LOW_END - MMIO_SPTE_GEN_LOW_START + 1) > #define MMIO_SPTE_GEN_HIGH_BITS (MMIO_SPTE_GEN_HIGH_END - MMIO_SPTE_GEN_HIGH_START + 1) > > > base-commit: 93472b79715378a2386598d6632c654a2223267b
