[v10,033/108] KVM: x86/mmu: Allow non-zero value for non-present SPTE and removed SPTE
Commit Message
Isaku Yamahata
Oct. 30, 2022, 6:22 a.m. UTC
From: Sean Christopherson <sean.j.christopherson@intel.com> For TD guest, the current way to emulate MMIO doesn't work any more, as KVM is not able to access the private memory of TD guest and do the emulation. Instead, TD guest expects to receive #VE when it accesses the MMIO and then it can explicitly makes hypercall to KVM to get the expected information. To achieve this, the TDX module always enables "EPT-violation #VE" in the VMCS control. And accordingly, KVM needs to configure the MMIO spte to trigger EPT violation (instead of misconfiguration) and at the same time, also clear the "suppress #VE" bit so the TD guest can get the #VE instead of causing actual EPT violation to KVM. In order for KVM to be able to have chance to set up the correct SPTE for MMIO for TD guest, the default non-present SPTE must have the "suppress guest accesses the MMIO. Also, when TD guest accesses the actual shared memory, it should continue to trigger EPT violation to the KVM instead of receiving the #VE (the TDX module guarantees KVM will receive EPT violation for private memory access). This means for the shared memory, the SPTE also must have the "suppress #VE" bit set for the non-present SPTE. Add "suppress VE" bit (bit 63) to SHADOW_NONPRESENT_VALUE and REMOVED_SPTE. Unconditionally set the "suppress #VE" bit (which is bit 63) for both AMD and Intel as: 1) AMD hardware doesn't use this bit when present bit is off; 2) for normal VMX guest, KVM never enables the "EPT-violation #VE" in VMCS control and "suppress #VE" bit is ignored by hardware. Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> --- arch/x86/include/asm/vmx.h | 1 + arch/x86/kvm/mmu/spte.c | 4 +++- arch/x86/kvm/mmu/spte.h | 22 +++++++++++++++++++++- arch/x86/kvm/mmu/tdp_mmu.c | 8 ++++++++ 4 files changed, 33 insertions(+), 2 deletions(-)
Comments
On Sat, 2022-10-29 at 23:22 -0700, isaku.yamahata@intel.com wrote: > From: Sean Christopherson <sean.j.christopherson@intel.com> > > For TD guest, the current way to emulate MMIO doesn't work any more, as KVM > is not able to access the private memory of TD guest and do the emulation. > Instead, TD guest expects to receive #VE when it accesses the MMIO and then > it can explicitly makes hypercall to KVM to get the expected information. ^ make > > To achieve this, the TDX module always enables "EPT-violation #VE" in the > VMCS control. And accordingly, KVM needs to configure the MMIO spte to > trigger EPT violation (instead of misconfiguration) and at the same time, > also clear the "suppress #VE" bit so the TD guest can get the #VE instead > of causing actual EPT violation to KVM. > > In order for KVM to be able to have chance to set up the correct SPTE for > MMIO for TD guest, the default non-present SPTE must have the "suppress > guest accesses the MMIO. > The above sentence is broken. > Also, when TD guest accesses the actual shared > memory, it should continue to trigger EPT violation to the KVM instead of > receiving the #VE (the TDX module guarantees KVM will receive EPT violation > for private memory access). This means for the shared memory, the SPTE > also must have the "suppress #VE" bit set for the non-present SPTE. > > Add "suppress VE" bit (bit 63) to SHADOW_NONPRESENT_VALUE and REMOVED_SPTE. > Unconditionally set the "suppress #VE" bit (which is bit 63) for both AMD > and Intel as: 1) AMD hardware doesn't use this bit when present bit is off; > 2) for normal VMX guest, KVM never enables the "EPT-violation #VE" in VMCS > control and "suppress #VE" bit is ignored by hardware. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> > --- > arch/x86/include/asm/vmx.h | 1 + > arch/x86/kvm/mmu/spte.c | 4 +++- > arch/x86/kvm/mmu/spte.h | 22 +++++++++++++++++++++- > arch/x86/kvm/mmu/tdp_mmu.c | 8 ++++++++ > 4 files changed, 33 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h > index 498dc600bd5c..cdbf12c1a83c 100644 > --- a/arch/x86/include/asm/vmx.h > +++ b/arch/x86/include/asm/vmx.h > @@ -511,6 +511,7 @@ enum vmcs_field { > #define VMX_EPT_IPAT_BIT (1ull << 6) > #define VMX_EPT_ACCESS_BIT (1ull << 8) > #define VMX_EPT_DIRTY_BIT (1ull << 9) > +#define VMX_EPT_SUPPRESS_VE_BIT (1ull << 63) > #define VMX_EPT_RWX_MASK (VMX_EPT_READABLE_MASK | \ > VMX_EPT_WRITABLE_MASK | \ > VMX_EPT_EXECUTABLE_MASK) > diff --git a/arch/x86/kvm/mmu/spte.c b/arch/x86/kvm/mmu/spte.c > index 2e08b2a45361..0b97a045c5f0 100644 > --- a/arch/x86/kvm/mmu/spte.c > +++ b/arch/x86/kvm/mmu/spte.c > @@ -419,7 +419,9 @@ void kvm_mmu_set_ept_masks(bool has_ad_bits, bool has_exec_only) > shadow_dirty_mask = has_ad_bits ? VMX_EPT_DIRTY_BIT : 0ull; > shadow_nx_mask = 0ull; > shadow_x_mask = VMX_EPT_EXECUTABLE_MASK; > - shadow_present_mask = has_exec_only ? 0ull : VMX_EPT_READABLE_MASK; > + /* VMX_EPT_SUPPRESS_VE_BIT is needed for W or X violation. */ > + shadow_present_mask = > + (has_exec_only ? 0ull : VMX_EPT_READABLE_MASK) | VMX_EPT_SUPPRESS_VE_BIT; I think this chunk can be in a separate patch since it doesn't handle fault from non-present to present, which is claimed by the patch title and changelog. Or I think you need to describe handling W or X fault part in the changelog. > /* > * EPT overrides the host MTRRs, and so KVM must program the desired > * memtype directly into the SPTEs. Note, this mask is just the mask > diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h > index 42ecaa75da15..7e0f79e8f45b 100644 > --- a/arch/x86/kvm/mmu/spte.h > +++ b/arch/x86/kvm/mmu/spte.h > @@ -148,7 +148,22 @@ static_assert(MMIO_SPTE_GEN_LOW_BITS == 8 && MMIO_SPTE_GEN_HIGH_BITS == 11); > > #define MMIO_SPTE_GEN_MASK GENMASK_ULL(MMIO_SPTE_GEN_LOW_BITS + MMIO_SPTE_GEN_HIGH_BITS - 1, 0) > > +/* > + * non-present SPTE value for both VMX and SVM for TDP MMU. ^ Non-present > + * For SVM NPT, for non-present spte (bit 0 = 0), other bits are ignored. > + * For VMX EPT, bit 63 is ignored if #VE is disabled. (EPT_VIOLATION_VE=0) > + * bit 63 is #VE suppress if #VE is enabled. (EPT_VIOLATION_VE=1) > + * For TDX: > + * Secure-EPT: TDX module sets EPT_VIOLATION_VE for Secure-EPT "EPT-violation #VE" is a VMCS control, so it applies to both Secure-EPT and shared-EPT. Looks "for Secure-EPT" is a little bit confusing. > + * private EPT: "suppress #VE" bit is ignored. CPU doesn't walk it. Looks "private EPT" just comes out of blue. I am not sure whether you need to mention it here. > + * conventional EPT: "suppress #VE" bit must be set to get EPT violation > + */ > +#ifdef CONFIG_X86_64 > +#define SHADOW_NONPRESENT_VALUE BIT_ULL(63) > +static_assert(!(SHADOW_NONPRESENT_VALUE & SPTE_MMU_PRESENT_MASK)); > +#else > #define SHADOW_NONPRESENT_VALUE 0ULL > +#endif > > extern u64 __read_mostly shadow_host_writable_mask; > extern u64 __read_mostly shadow_mmu_writable_mask; > @@ -189,13 +204,18 @@ extern u64 __read_mostly shadow_nonpresent_or_rsvd_mask; > * non-present intermediate value. Other threads which encounter this value > * should not modify the SPTE. > * > + * For X86_64 case, SHADOW_NONPRESENT_VALUE, "suppress #VE" bit, is set because > + * "EPT violation #VE" in the secondary VM execution control may be enabled. > + * Because TDX module sets "EPT violation #VE" for TD, "suppress #VE" bit for > + * the conventional EPT needs to be set. > + * This comment looks duplicated with the above one. Not sure whether needed. > * Use a semi-arbitrary value that doesn't set RWX bits, i.e. is not-present on > * bot AMD and Intel CPUs, and doesn't set PFN bits, i.e. doesn't create a L1TF > * vulnerability. Use only low bits to avoid 64-bit immediates. > * > * Only used by the TDP MMU. > */ > -#define REMOVED_SPTE 0x5a0ULL > +#define REMOVED_SPTE (SHADOW_NONPRESENT_VALUE | 0x5a0ULL) > > /* Removed SPTEs must not be misconstrued as shadow present PTEs. */ > static_assert(!(REMOVED_SPTE & SPTE_MMU_PRESENT_MASK)); > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c > index 38bc4c2f0f1f..1eee9c159958 100644 > --- a/arch/x86/kvm/mmu/tdp_mmu.c > +++ b/arch/x86/kvm/mmu/tdp_mmu.c > @@ -693,6 +693,14 @@ static inline int tdp_mmu_zap_spte_atomic(struct kvm *kvm, > * overwrite the special removed SPTE value. No bookkeeping is needed > * here since the SPTE is going from non-present to non-present. Use > * the raw write helper to avoid an unnecessary check on volatile bits. > + * > + * Set non-present value to SHADOW_NONPRESENT_VALUE, rather than 0. > + * It is because when TDX is enabled, TDX module always > + * enables "EPT-violation #VE", so KVM needs to set > + * "suppress #VE" bit in EPT table entries, in order to get > + * real EPT violation, rather than TDVMCALL. KVM sets > + * SHADOW_NONPRESENT_VALUE (which sets "suppress #VE" bit) so it > + * can be set when EPT table entries are zapped. > */ > __kvm_tdp_mmu_write_spte(iter->sptep, SHADOW_NONPRESENT_VALUE); > Ditto.
On Wed, Nov 09, 2022 at 11:24:12AM +0000, "Huang, Kai" <kai.huang@intel.com> wrote: > > diff --git a/arch/x86/kvm/mmu/spte.c b/arch/x86/kvm/mmu/spte.c > > index 2e08b2a45361..0b97a045c5f0 100644 > > --- a/arch/x86/kvm/mmu/spte.c > > +++ b/arch/x86/kvm/mmu/spte.c > > @@ -419,7 +419,9 @@ void kvm_mmu_set_ept_masks(bool has_ad_bits, bool has_exec_only) > > shadow_dirty_mask = has_ad_bits ? VMX_EPT_DIRTY_BIT : 0ull; > > shadow_nx_mask = 0ull; > > shadow_x_mask = VMX_EPT_EXECUTABLE_MASK; > > - shadow_present_mask = has_exec_only ? 0ull : VMX_EPT_READABLE_MASK; > > + /* VMX_EPT_SUPPRESS_VE_BIT is needed for W or X violation. */ > > + shadow_present_mask = > > + (has_exec_only ? 0ull : VMX_EPT_READABLE_MASK) | VMX_EPT_SUPPRESS_VE_BIT; > > I think this chunk can be in a separate patch since it doesn't handle fault from > non-present to present, which is claimed by the patch title and changelog. > > Or I think you need to describe handling W or X fault part in the changelog. Ok, let me try to split that hunk.
diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h index 498dc600bd5c..cdbf12c1a83c 100644 --- a/arch/x86/include/asm/vmx.h +++ b/arch/x86/include/asm/vmx.h @@ -511,6 +511,7 @@ enum vmcs_field { #define VMX_EPT_IPAT_BIT (1ull << 6) #define VMX_EPT_ACCESS_BIT (1ull << 8) #define VMX_EPT_DIRTY_BIT (1ull << 9) +#define VMX_EPT_SUPPRESS_VE_BIT (1ull << 63) #define VMX_EPT_RWX_MASK (VMX_EPT_READABLE_MASK | \ VMX_EPT_WRITABLE_MASK | \ VMX_EPT_EXECUTABLE_MASK) diff --git a/arch/x86/kvm/mmu/spte.c b/arch/x86/kvm/mmu/spte.c index 2e08b2a45361..0b97a045c5f0 100644 --- a/arch/x86/kvm/mmu/spte.c +++ b/arch/x86/kvm/mmu/spte.c @@ -419,7 +419,9 @@ void kvm_mmu_set_ept_masks(bool has_ad_bits, bool has_exec_only) shadow_dirty_mask = has_ad_bits ? VMX_EPT_DIRTY_BIT : 0ull; shadow_nx_mask = 0ull; shadow_x_mask = VMX_EPT_EXECUTABLE_MASK; - shadow_present_mask = has_exec_only ? 0ull : VMX_EPT_READABLE_MASK; + /* VMX_EPT_SUPPRESS_VE_BIT is needed for W or X violation. */ + shadow_present_mask = + (has_exec_only ? 0ull : VMX_EPT_READABLE_MASK) | VMX_EPT_SUPPRESS_VE_BIT; /* * EPT overrides the host MTRRs, and so KVM must program the desired * memtype directly into the SPTEs. Note, this mask is just the mask diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h index 42ecaa75da15..7e0f79e8f45b 100644 --- a/arch/x86/kvm/mmu/spte.h +++ b/arch/x86/kvm/mmu/spte.h @@ -148,7 +148,22 @@ static_assert(MMIO_SPTE_GEN_LOW_BITS == 8 && MMIO_SPTE_GEN_HIGH_BITS == 11); #define MMIO_SPTE_GEN_MASK GENMASK_ULL(MMIO_SPTE_GEN_LOW_BITS + MMIO_SPTE_GEN_HIGH_BITS - 1, 0) +/* + * non-present SPTE value for both VMX and SVM for TDP MMU. + * For SVM NPT, for non-present spte (bit 0 = 0), other bits are ignored. + * For VMX EPT, bit 63 is ignored if #VE is disabled. (EPT_VIOLATION_VE=0) + * bit 63 is #VE suppress if #VE is enabled. (EPT_VIOLATION_VE=1) + * For TDX: + * Secure-EPT: TDX module sets EPT_VIOLATION_VE for Secure-EPT + * private EPT: "suppress #VE" bit is ignored. CPU doesn't walk it. + * conventional EPT: "suppress #VE" bit must be set to get EPT violation + */ +#ifdef CONFIG_X86_64 +#define SHADOW_NONPRESENT_VALUE BIT_ULL(63) +static_assert(!(SHADOW_NONPRESENT_VALUE & SPTE_MMU_PRESENT_MASK)); +#else #define SHADOW_NONPRESENT_VALUE 0ULL +#endif extern u64 __read_mostly shadow_host_writable_mask; extern u64 __read_mostly shadow_mmu_writable_mask; @@ -189,13 +204,18 @@ extern u64 __read_mostly shadow_nonpresent_or_rsvd_mask; * non-present intermediate value. Other threads which encounter this value * should not modify the SPTE. * + * For X86_64 case, SHADOW_NONPRESENT_VALUE, "suppress #VE" bit, is set because + * "EPT violation #VE" in the secondary VM execution control may be enabled. + * Because TDX module sets "EPT violation #VE" for TD, "suppress #VE" bit for + * the conventional EPT needs to be set. + * * Use a semi-arbitrary value that doesn't set RWX bits, i.e. is not-present on * bot AMD and Intel CPUs, and doesn't set PFN bits, i.e. doesn't create a L1TF * vulnerability. Use only low bits to avoid 64-bit immediates. * * Only used by the TDP MMU. */ -#define REMOVED_SPTE 0x5a0ULL +#define REMOVED_SPTE (SHADOW_NONPRESENT_VALUE | 0x5a0ULL) /* Removed SPTEs must not be misconstrued as shadow present PTEs. */ static_assert(!(REMOVED_SPTE & SPTE_MMU_PRESENT_MASK)); diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c index 38bc4c2f0f1f..1eee9c159958 100644 --- a/arch/x86/kvm/mmu/tdp_mmu.c +++ b/arch/x86/kvm/mmu/tdp_mmu.c @@ -693,6 +693,14 @@ static inline int tdp_mmu_zap_spte_atomic(struct kvm *kvm, * overwrite the special removed SPTE value. No bookkeeping is needed * here since the SPTE is going from non-present to non-present. Use * the raw write helper to avoid an unnecessary check on volatile bits. + * + * Set non-present value to SHADOW_NONPRESENT_VALUE, rather than 0. + * It is because when TDX is enabled, TDX module always + * enables "EPT-violation #VE", so KVM needs to set + * "suppress #VE" bit in EPT table entries, in order to get + * real EPT violation, rather than TDVMCALL. KVM sets + * SHADOW_NONPRESENT_VALUE (which sets "suppress #VE" bit) so it + * can be set when EPT table entries are zapped. */ __kvm_tdp_mmu_write_spte(iter->sptep, SHADOW_NONPRESENT_VALUE);