Re: [PATCH v8 009/103] KVM: TDX: Initialize the TDX module when loading the KVM intel kernel module

From: Isaku Yamahata
Date: Thu Aug 25 2022 - 16:26:28 EST


On Wed, Aug 10, 2022 at 04:18:01PM +0800,
Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx> wrote:

>
> On 2022/8/8 6:00, isaku.yamahata@xxxxxxxxx wrote:
> > From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> >
> > TDX requires several initialization steps for KVM to create guest TDs.
> > Detect CPU feature, enable VMX (TDX is based on VMX), detect the TDX module
> > availability, and initialize it. This patch implements those steps.
> >
> > There are several options on when to initialize the TDX module. A.) kernel
> > module loading time, B.) the first guest TD creation time. A.) was chosen.
> > With B.), a user may hit an error of the TDX initialization when trying to
> > create the first guest TD. The machine that fails to initialize the TDX
> > module can't boot any guest TD further. Such failure is undesirable and a
> > surprise because the user expects that the machine can accommodate guest
> > TD, but actually not. So A.) is better than B.).
> >
> > Introduce a module parameter, enable_tdx, to explicitly enable TDX KVM
> > support. It's off by default to keep same behavior for those who don't use
> > TDX. Implement hardware_setup method to detect TDX feature of CPU.
> > Because TDX requires all present CPUs to enable VMX (VMXON). The x86
> > specific kvm_arch_post_hardware_enable_setup overrides the existing weak
> > symbol of kvm_arch_post_hardware_enable_setup which is called at the KVM
> > module initialization.
> >
> > Suggested-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> > ---
> > arch/x86/include/asm/kvm_host.h | 1 +
> > arch/x86/kvm/Makefile | 1 +
> > arch/x86/kvm/vmx/main.c | 29 ++++++++++-
> > arch/x86/kvm/vmx/tdx.c | 89 +++++++++++++++++++++++++++++++++
> > arch/x86/kvm/vmx/tdx.h | 4 ++
> > arch/x86/kvm/vmx/x86_ops.h | 6 +++
> > arch/x86/kvm/x86.c | 8 +++
> > arch/x86/virt/vmx/tdx/tdx.c | 1 +
> > 8 files changed, 138 insertions(+), 1 deletion(-)
> > create mode 100644 arch/x86/kvm/vmx/tdx.c
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 3d000f060077..f432ad32515c 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1659,6 +1659,7 @@ struct kvm_x86_init_ops {
> > int (*cpu_has_kvm_support)(void);
> > int (*disabled_by_bios)(void);
> > int (*hardware_setup)(void);
> > + int (*post_hardware_enable_setup)(void);
> > unsigned int (*handle_intel_pt_intr)(void);
> > struct kvm_x86_ops *runtime_ops;
> > diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> > index ee4d0999f20f..e2c05195cb95 100644
> > --- a/arch/x86/kvm/Makefile
> > +++ b/arch/x86/kvm/Makefile
> > @@ -24,6 +24,7 @@ kvm-$(CONFIG_KVM_XEN) += xen.o
> > kvm-intel-y += vmx/vmx.o vmx/vmenter.o vmx/pmu_intel.o vmx/vmcs12.o \
> > vmx/evmcs.o vmx/nested.o vmx/posted_intr.o vmx/main.o
> > kvm-intel-$(CONFIG_X86_SGX_KVM) += vmx/sgx.o
> > +kvm-intel-$(CONFIG_INTEL_TDX_HOST) += vmx/tdx.o
> > kvm-amd-y += svm/svm.o svm/vmenter.o svm/pmu.o svm/nested.o svm/avic.o svm/sev.o
> > diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
> > index a0252cc0b48d..ac788af17d92 100644
> > --- a/arch/x86/kvm/vmx/main.c
> > +++ b/arch/x86/kvm/vmx/main.c
> > @@ -7,6 +7,32 @@
> > #include "pmu.h"
> > #include "tdx.h"
> > +static bool __read_mostly enable_tdx = IS_ENABLED(CONFIG_INTEL_TDX_HOST);
>
> So, if CONFIG_INTEL_TDX_HOST is opt-in in kconfig, the code will try to
> enable TDX.
>
> The behavior seems a bit different from what you mentioned in the commit
> message about the option to "explicitly enable TDX KVM
> support".

Oops. For least surpise, I'll fix it as follows.

diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
index ac788af17d92..973c437f1a4e 100644
--- a/arch/x86/kvm/vmx/main.c
+++ b/arch/x86/kvm/vmx/main.c
@@ -7,7 +7,7 @@
#include "pmu.h"
#include "tdx.h"

-static bool __read_mostly enable_tdx = IS_ENABLED(CONFIG_INTEL_TDX_HOST);
+static bool __read_mostly enable_tdx;
module_param_named(tdx, enable_tdx, bool, 0444);

static __init int vt_hardware_setup(void)

--
Isaku Yamahata <isaku.yamahata@xxxxxxxxx>