| Message ID | a5be8d9fd5753e17ad2ae4b5fc360501bdf2e84a.1667110240.git.isaku.yamahata@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM TDX basic feature support | expand |
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);