1. Patchew
  2. linux
  3. [PATCH v7 000/102] KVM TDX basic feature support
  4. View patch

[PATCH v7 043/102] KVM: x86/mmu: Focibly use TDP MMU for TDX

isaku.yamahata@intel.com posted 102 patches 3 years, 5 months ago
There is a newer version of this series
[PATCH v7 043/102] KVM: x86/mmu: Focibly use TDP MMU for TDX
Posted by isaku.yamahata@intel.com 3 years, 5 months ago 
From: Isaku Yamahata <isaku.yamahata@intel.com>

In this patch series, TDX supports only TDP MMU and doesn't support legacy
MMU.  Forcibly use TDP MMU for TDX irrelevant of kernel parameter to
disable TDP MMU.

Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
 arch/x86/kvm/mmu/tdp_mmu.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 82f1bfac7ee6..7eb41b176d1e 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -18,8 +18,13 @@ int kvm_mmu_init_tdp_mmu(struct kvm *kvm)
 {
 	struct workqueue_struct *wq;
 
-	if (!tdp_enabled || !READ_ONCE(tdp_mmu_enabled))
-		return 0;
+	/*
+	 *  Because TDX supports only TDP MMU, forcibly use TDP MMU in the case
+	 *  of TDX.
+	 */
+	if (kvm->arch.vm_type != KVM_X86_TDX_VM &&
+		(!tdp_enabled || !READ_ONCE(tdp_mmu_enabled)))
+		return false;
 
 	wq = alloc_workqueue("kvm", WQ_UNBOUND|WQ_MEM_RECLAIM|WQ_CPU_INTENSIVE, 0);
 	if (!wq)
-- 
2.25.1
Re: [PATCH v7 043/102] KVM: x86/mmu: Focibly use TDP MMU for TDX
Posted by Sean Christopherson 3 years, 5 months ago 
s/Focibly/Forcibly, but that's a moot point because KVM shouldn't override the
the module param.  KVM should instead _require_ the TDP MMU to be enabled.  E.g.
if userspace disables the TDP MMU to workaround a fatal bug, then forcing the TDP
MMU may silently expose KVM to said bug.

And overriding tdp_enabled is just mind-boggling broken, all of the SPTE masks
will be wrong.

On Mon, Jun 27, 2022, isaku.yamahata@intel.com wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
> 
> In this patch series, TDX supports only TDP MMU and doesn't support legacy
> MMU.  Forcibly use TDP MMU for TDX irrelevant of kernel parameter to
> disable TDP MMU.

Do not refer to the "patch series", instead phrase the statement with respect to
what KVM support.

  Require the TDP MMU for TDX guests, the so called "shadow" MMU does not
  support mapping guest private memory, i.e. does not support Secure-EPT.

> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
>  arch/x86/kvm/mmu/tdp_mmu.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 82f1bfac7ee6..7eb41b176d1e 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -18,8 +18,13 @@ int kvm_mmu_init_tdp_mmu(struct kvm *kvm)
>  {
>  	struct workqueue_struct *wq;
>  
> -	if (!tdp_enabled || !READ_ONCE(tdp_mmu_enabled))
> -		return 0;
> +	/*
> +	 *  Because TDX supports only TDP MMU, forcibly use TDP MMU in the case
> +	 *  of TDX.
> +	 */
> +	if (kvm->arch.vm_type != KVM_X86_TDX_VM &&
> +		(!tdp_enabled || !READ_ONCE(tdp_mmu_enabled)))
> +		return false;

Yeah, no.

	if (!tdp_enabled || !READ_ONCE(tdp_mmu_enabled))
		return kvm->arch.vm_type == KVM_X86_TDX_VM ? -EINVAL : 0;

>  
>  	wq = alloc_workqueue("kvm", WQ_UNBOUND|WQ_MEM_RECLAIM|WQ_CPU_INTENSIVE, 0);
>  	if (!wq)
> -- 
> 2.25.1
>
Re: [PATCH v7 043/102] KVM: x86/mmu: Focibly use TDP MMU for TDX
Posted by Isaku Yamahata 3 years, 5 months ago 
On Mon, Jul 11, 2022 at 02:56:29PM +0000,
Sean Christopherson <seanjc@google.com> wrote:

> s/Focibly/Forcibly, but that's a moot point because KVM shouldn't override the
> the module param.  KVM should instead _require_ the TDP MMU to be enabled.  E.g.
> if userspace disables the TDP MMU to workaround a fatal bug, then forcing the TDP
> MMU may silently expose KVM to said bug.
> 
> And overriding tdp_enabled is just mind-boggling broken, all of the SPTE masks
> will be wrong.
> 
> On Mon, Jun 27, 2022, isaku.yamahata@intel.com wrote:
> > From: Isaku Yamahata <isaku.yamahata@intel.com>
> > 
> > In this patch series, TDX supports only TDP MMU and doesn't support legacy
> > MMU.  Forcibly use TDP MMU for TDX irrelevant of kernel parameter to
> > disable TDP MMU.
> 
> Do not refer to the "patch series", instead phrase the statement with respect to
> what KVM support.
> 
>   Require the TDP MMU for TDX guests, the so called "shadow" MMU does not
>   support mapping guest private memory, i.e. does not support Secure-EPT.

Thanks for rewrite of the commit message.  Now the TDP MMU is default, I'll change

> > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> > ---
> >  arch/x86/kvm/mmu/tdp_mmu.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> > index 82f1bfac7ee6..7eb41b176d1e 100644
> > --- a/arch/x86/kvm/mmu/tdp_mmu.c
> > +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> > @@ -18,8 +18,13 @@ int kvm_mmu_init_tdp_mmu(struct kvm *kvm)
> >  {
> >  	struct workqueue_struct *wq;
> >  
> > -	if (!tdp_enabled || !READ_ONCE(tdp_mmu_enabled))
> > -		return 0;
> > +	/*
> > +	 *  Because TDX supports only TDP MMU, forcibly use TDP MMU in the case
> > +	 *  of TDX.
> > +	 */
> > +	if (kvm->arch.vm_type != KVM_X86_TDX_VM &&
> > +		(!tdp_enabled || !READ_ONCE(tdp_mmu_enabled)))
> > +		return false;
> 
> Yeah, no.
> 
> 	if (!tdp_enabled || !READ_ONCE(tdp_mmu_enabled))
> 		return kvm->arch.vm_type == KVM_X86_TDX_VM ? -EINVAL : 0;

I'll use -EOPNOTSUPP instead of -EINVAL.
-- 
Isaku Yamahata <isaku.yamahata@gmail.com>
Re: [PATCH v7 043/102] KVM: x86/mmu: Focibly use TDP MMU for TDX
Posted by Yuan Yao 3 years, 5 months ago 
On Mon, Jun 27, 2022 at 02:53:35PM -0700, isaku.yamahata@intel.com wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
>
> In this patch series, TDX supports only TDP MMU and doesn't support legacy
> MMU.  Forcibly use TDP MMU for TDX irrelevant of kernel parameter to
> disable TDP MMU.
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
>  arch/x86/kvm/mmu/tdp_mmu.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 82f1bfac7ee6..7eb41b176d1e 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -18,8 +18,13 @@ int kvm_mmu_init_tdp_mmu(struct kvm *kvm)
>  {
>  	struct workqueue_struct *wq;
>
> -	if (!tdp_enabled || !READ_ONCE(tdp_mmu_enabled))
> -		return 0;
> +	/*
> +	 *  Because TDX supports only TDP MMU, forcibly use TDP MMU in the case
> +	 *  of TDX.
> +	 */
> +	if (kvm->arch.vm_type != KVM_X86_TDX_VM &&
> +		(!tdp_enabled || !READ_ONCE(tdp_mmu_enabled)))
> +		return false;

Please return 0 here for int return value type.

>
>  	wq = alloc_workqueue("kvm", WQ_UNBOUND|WQ_MEM_RECLAIM|WQ_CPU_INTENSIVE, 0);
>  	if (!wq)
> --
> 2.25.1
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.