[PATCH 0/4] Adjust TD settings on boot

[PATCH 0/4] Adjust TD settings on boot

[Kirill A. Shutemov]

Adjust TD setting on boot:

  - Disable EPT violation #VE on private memory if TD can
    control it;

  - Enable virtualization of topology-related CPUID leafs
    X2APIC_APICID MSR;

Kirill A. Shutemov (4):
  x86/tdx: Introduce tdg_vm_wr()
  x86/tdx: Rename tdx_parse_tdinfo() to tdx_setup()
  x86/tdx: Handle PENDING_EPT_VIOLATION_V2
  x86/tdx: Enable ENUM_TOPOLOGY

 arch/x86/coco/tdx/tdx.c           | 117 ++++++++++++++++++++++++++----
 arch/x86/include/asm/shared/tdx.h |  21 +++++-
 2 files changed, 123 insertions(+), 15 deletions(-)

-- 
2.43.0

[PATCH 1/4] x86/tdx: Introduce tdg_vm_wr()

[Kirill A. Shutemov]

Add a helper to write to a TD-scope metadata field and use it to set
NOTIFY_ENABLES.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
 arch/x86/coco/tdx/tdx.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index cc67f7380055..5ffe5ef99536 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -77,6 +77,20 @@ static inline void tdcall(u64 fn, struct tdx_module_args *args)
 		panic("TDCALL %lld failed (Buggy TDX module!)\n", fn);
 }
 
+static inline u64 tdg_vm_wr(u64 field, u64 value, u64 mask)
+{
+	struct tdx_module_args args = {
+		.rdx = field,
+		.r8 = value,
+		.r9 = mask,
+	};
+
+	tdcall(TDG_VM_WR, &args);
+
+	/* Old value */
+	return args.r8;
+}
+
 /**
  * tdx_mcall_get_report0() - Wrapper to get TDREPORT0 (a.k.a. TDREPORT
  *                           subtype 0) using TDG.MR.REPORT TDCALL.
@@ -902,10 +916,6 @@ static void tdx_kexec_unshare_mem(void)
 
 void __init tdx_early_init(void)
 {
-	struct tdx_module_args args = {
-		.rdx = TDCS_NOTIFY_ENABLES,
-		.r9 = -1ULL,
-	};
 	u64 cc_mask;
 	u32 eax, sig[3];
 
@@ -924,7 +934,7 @@ void __init tdx_early_init(void)
 	cc_set_mask(cc_mask);
 
 	/* Kernel does not use NOTIFY_ENABLES and does not need random #VEs */
-	tdcall(TDG_VM_WR, &args);
+	tdg_vm_wr(TDCS_NOTIFY_ENABLES, 0, -1ULL);
 
 	/*
 	 * All bits above GPA width are reserved and kernel treats shared bit
-- 
2.43.0

Re: [PATCH 1/4] x86/tdx: Introduce tdg_vm_wr()

[Kuppuswamy Sathyanarayanan]

On Sat, Mar 9, 2024 at 1:02 PM Kirill A. Shutemov
<kirill.shutemov@linux.intel.com> wrote:
>
> Add a helper to write to a TD-scope metadata field and use it to set
> NOTIFY_ENABLES.
>

Add a note about why you create this helper function. I think the
intention is to reuse it
to update other TD-scope fields.

> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> ---
>  arch/x86/coco/tdx/tdx.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index cc67f7380055..5ffe5ef99536 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -77,6 +77,20 @@ static inline void tdcall(u64 fn, struct tdx_module_args *args)
>                 panic("TDCALL %lld failed (Buggy TDX module!)\n", fn);
>  }
>
> +static inline u64 tdg_vm_wr(u64 field, u64 value, u64 mask)
> +{
> +       struct tdx_module_args args = {
> +               .rdx = field,
> +               .r8 = value,
> +               .r9 = mask,
> +       };
> +
> +       tdcall(TDG_VM_WR, &args);
> +
> +       /* Old value */
> +       return args.r8;

Since the update failure will panic, any use for returning the old value?

> +}
> +
>  /**
>   * tdx_mcall_get_report0() - Wrapper to get TDREPORT0 (a.k.a. TDREPORT
>   *                           subtype 0) using TDG.MR.REPORT TDCALL.
> @@ -902,10 +916,6 @@ static void tdx_kexec_unshare_mem(void)
>
>  void __init tdx_early_init(void)
>  {
> -       struct tdx_module_args args = {
> -               .rdx = TDCS_NOTIFY_ENABLES,
> -               .r9 = -1ULL,
> -       };
>         u64 cc_mask;
>         u32 eax, sig[3];
>
> @@ -924,7 +934,7 @@ void __init tdx_early_init(void)
>         cc_set_mask(cc_mask);
>
>         /* Kernel does not use NOTIFY_ENABLES and does not need random #VEs */
> -       tdcall(TDG_VM_WR, &args);
> +       tdg_vm_wr(TDCS_NOTIFY_ENABLES, 0, -1ULL);
>
>         /*
>          * All bits above GPA width are reserved and kernel treats shared bit
> --
> 2.43.0
>

Re: [PATCH 1/4] x86/tdx: Introduce tdg_vm_wr()

[Kirill A. Shutemov]

On Sat, Mar 09, 2024 at 04:02:58PM -0800, Kuppuswamy Sathyanarayanan wrote:
> > +       /* Old value */
> > +       return args.r8;
> 
> Since the update failure will panic, any use for returning the old value?

I don't have any immediate users for the old value, but I can imagine
situation when you need to do something else if your write result in
actual change of setting.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

[PATCH 2/4] x86/tdx: Rename tdx_parse_tdinfo() to tdx_setup()

[Kirill A. Shutemov]

Rename tdx_parse_tdinfo() to tdx_setup() and move setting NOTIFY_ENABLES
there.

The function will be extended to adjust TD configuration.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
 arch/x86/coco/tdx/tdx.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 5ffe5ef99536..afdaf46cabb9 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -181,7 +181,7 @@ static void __noreturn tdx_panic(const char *msg)
 		__tdx_hypercall(&args);
 }
 
-static void tdx_parse_tdinfo(u64 *cc_mask)
+static void tdx_setup(u64 *cc_mask)
 {
 	struct tdx_module_args args = {};
 	unsigned int gpa_width;
@@ -206,6 +206,9 @@ static void tdx_parse_tdinfo(u64 *cc_mask)
 	gpa_width = args.rcx & GENMASK(5, 0);
 	*cc_mask = BIT_ULL(gpa_width - 1);
 
+	/* Kernel does not use NOTIFY_ENABLES and does not need random #VEs */
+	tdg_vm_wr(TDCS_NOTIFY_ENABLES, 0, -1ULL);
+
 	/*
 	 * The kernel can not handle #VE's when accessing normal kernel
 	 * memory.  Ensure that no #VE will be delivered for accesses to
@@ -930,11 +933,11 @@ void __init tdx_early_init(void)
 	setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
 
 	cc_vendor = CC_VENDOR_INTEL;
-	tdx_parse_tdinfo(&cc_mask);
-	cc_set_mask(cc_mask);
 
-	/* Kernel does not use NOTIFY_ENABLES and does not need random #VEs */
-	tdg_vm_wr(TDCS_NOTIFY_ENABLES, 0, -1ULL);
+	/* Configure the TD */
+	tdx_setup(&cc_mask);
+
+	cc_set_mask(cc_mask);
 
 	/*
 	 * All bits above GPA width are reserved and kernel treats shared bit
-- 
2.43.0

Re: [PATCH 2/4] x86/tdx: Rename tdx_parse_tdinfo() to tdx_setup()

[Kuppuswamy Sathyanarayanan]

On 3/9/24 1:02 PM, Kirill A. Shutemov wrote:
> Rename tdx_parse_tdinfo() to tdx_setup() and move setting NOTIFY_ENABLES
> there.
>
> The function will be extended to adjust TD configuration.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> ---

Looks good to me.

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

>  arch/x86/coco/tdx/tdx.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index 5ffe5ef99536..afdaf46cabb9 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -181,7 +181,7 @@ static void __noreturn tdx_panic(const char *msg)
>  		__tdx_hypercall(&args);
>  }
>  
> -static void tdx_parse_tdinfo(u64 *cc_mask)
> +static void tdx_setup(u64 *cc_mask)
>  {
>  	struct tdx_module_args args = {};
>  	unsigned int gpa_width;
> @@ -206,6 +206,9 @@ static void tdx_parse_tdinfo(u64 *cc_mask)
>  	gpa_width = args.rcx & GENMASK(5, 0);
>  	*cc_mask = BIT_ULL(gpa_width - 1);
>  
> +	/* Kernel does not use NOTIFY_ENABLES and does not need random #VEs */
> +	tdg_vm_wr(TDCS_NOTIFY_ENABLES, 0, -1ULL);
> +
>  	/*
>  	 * The kernel can not handle #VE's when accessing normal kernel
>  	 * memory.  Ensure that no #VE will be delivered for accesses to
> @@ -930,11 +933,11 @@ void __init tdx_early_init(void)
>  	setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
>  
>  	cc_vendor = CC_VENDOR_INTEL;
> -	tdx_parse_tdinfo(&cc_mask);
> -	cc_set_mask(cc_mask);
>  
> -	/* Kernel does not use NOTIFY_ENABLES and does not need random #VEs */
> -	tdg_vm_wr(TDCS_NOTIFY_ENABLES, 0, -1ULL);
> +	/* Configure the TD */
> +	tdx_setup(&cc_mask);
> +
> +	cc_set_mask(cc_mask);
>  
>  	/*
>  	 * All bits above GPA width are reserved and kernel treats shared bit

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

[PATCH 3/4] x86/tdx: Handle PENDING_EPT_VIOLATION_V2

[Kirill A. Shutemov]

PENDING_EPT_VIOLATION_V2 allows TD to control whether access to
a pending page triggers #VE.

Kernel doesn't want to see any #VEs on accesses to private memory:
disable such #VEs.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
 arch/x86/coco/tdx/tdx.c           | 66 ++++++++++++++++++++++++++++---
 arch/x86/include/asm/shared/tdx.h | 18 ++++++++-
 2 files changed, 78 insertions(+), 6 deletions(-)

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index afdaf46cabb9..d9ea82f8772d 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -77,6 +77,17 @@ static inline void tdcall(u64 fn, struct tdx_module_args *args)
 		panic("TDCALL %lld failed (Buggy TDX module!)\n", fn);
 }
 
+static inline u64 tdg_vm_rd(u64 field)
+{
+	struct tdx_module_args args = {
+		.rdx = field,
+	};
+
+	tdcall(TDG_VM_RD, &args);
+
+	return args.r8;
+}
+
 static inline u64 tdg_vm_wr(u64 field, u64 value, u64 mask)
 {
 	struct tdx_module_args args = {
@@ -91,6 +102,17 @@ static inline u64 tdg_vm_wr(u64 field, u64 value, u64 mask)
 	return args.r8;
 }
 
+static inline u64 tdg_sys_rd(u64 field)
+{
+	struct tdx_module_args args = {
+		.rdx = field,
+	};
+
+	tdcall(TDG_SYS_RD, &args);
+
+	return args.r8;
+}
+
 /**
  * tdx_mcall_get_report0() - Wrapper to get TDREPORT0 (a.k.a. TDREPORT
  *                           subtype 0) using TDG.MR.REPORT TDCALL.
@@ -185,7 +207,8 @@ static void tdx_setup(u64 *cc_mask)
 {
 	struct tdx_module_args args = {};
 	unsigned int gpa_width;
-	u64 td_attr;
+	u64 td_attr, features;
+	bool sept_ve_disabled;
 
 	/*
 	 * TDINFO TDX module call is used to get the TD execution environment
@@ -206,19 +229,52 @@ static void tdx_setup(u64 *cc_mask)
 	gpa_width = args.rcx & GENMASK(5, 0);
 	*cc_mask = BIT_ULL(gpa_width - 1);
 
+	td_attr = args.rdx;
+
 	/* Kernel does not use NOTIFY_ENABLES and does not need random #VEs */
 	tdg_vm_wr(TDCS_NOTIFY_ENABLES, 0, -1ULL);
 
+	features = tdg_sys_rd(TDCS_TDX_FEATURES0);
+
 	/*
 	 * The kernel can not handle #VE's when accessing normal kernel
 	 * memory.  Ensure that no #VE will be delivered for accesses to
 	 * TD-private memory.  Only VMM-shared memory (MMIO) will #VE.
+	 *
+	 * Check if the TD is created with SEPT #VE disabled.
 	 */
-	td_attr = args.rdx;
-	if (!(td_attr & ATTR_SEPT_VE_DISABLE)) {
-		const char *msg = "TD misconfiguration: SEPT_VE_DISABLE attribute must be set.";
+	sept_ve_disabled = td_attr & ATTR_SEPT_VE_DISABLE;
 
-		/* Relax SEPT_VE_DISABLE check for debug TD. */
+	/*
+	 * Check if flexible control of SEPT #VE is supported.
+	 *
+	 * The check consists of verifying if the feature is supported by the
+	 * TDX module (the TDX_FEATURES0 check) and if the feature is enabled
+	 * for this TD (CONFIG_FLAGS check).
+	 *
+	 * If flexible control is supported, disable SEPT #VE.
+	 *
+	 * Disable SEPT #VE regardless of ATTR_SEPT_VE_DISABLE status as
+	 * flexible control allows software running before the kernel to
+	 * enable it.
+	 *
+	 * Skip SEPT disabling for debug TD. SEPT #VE is unsafe but can be
+	 * useful for debugging to produce a stack trace. Known to be useful
+	 * for debugging unaccepted memory problems.
+	 */
+	if (features & TDX_FEATURES0_PENDING_EPT_VIOLATION_V2 &&
+	    (tdg_vm_rd(TDCS_CONFIG_FLAGS) & TDCS_CONFIG_FLEXIBLE_PENDING_VE) &&
+	    !(td_attr & ATTR_DEBUG)) {
+		tdg_vm_wr(TDCS_TD_CTLS,
+			  TD_CTLS_PENDING_VE_DISABLE,
+			  TD_CTLS_PENDING_VE_DISABLE);
+		sept_ve_disabled = true;
+	}
+
+	if (!sept_ve_disabled) {
+		const char *msg = "TD misconfiguration: SEPT #VE has to be disabled";
+
+		/* Relax SEPT #VE disable check for debug TD. */
 		if (td_attr & ATTR_DEBUG)
 			pr_warn("%s\n", msg);
 		else
diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
index fdfd41511b02..29a61c72e4dd 100644
--- a/arch/x86/include/asm/shared/tdx.h
+++ b/arch/x86/include/asm/shared/tdx.h
@@ -16,11 +16,27 @@
 #define TDG_VP_VEINFO_GET		3
 #define TDG_MR_REPORT			4
 #define TDG_MEM_PAGE_ACCEPT		6
+#define TDG_VM_RD			7
 #define TDG_VM_WR			8
+#define TDG_SYS_RD			11
 
-/* TDCS fields. To be used by TDG.VM.WR and TDG.VM.RD module calls */
+/* TDX Global Metadata. To be used by TDG.SYS.RD */
+#define TDCS_TDX_FEATURES0		0x0A00000300000008
+
+/* TDX TD-Scope Metadata. To be used by TDG.VM.WR and TDG.VM.RD */
+#define TDCS_CONFIG_FLAGS		0x1110000300000016
+#define TDCS_TD_CTLS			0x1110000300000017
 #define TDCS_NOTIFY_ENABLES		0x9100000000000010
 
+/* TDCS_TDX_FEATURES0 bits */
+#define TDX_FEATURES0_PENDING_EPT_VIOLATION_V2	BIT_ULL(16)
+
+/* TDCS_CONFIG_FLAGS bits */
+#define TDCS_CONFIG_FLEXIBLE_PENDING_VE	BIT_ULL(1)
+
+/* TDCS_TD_CTLS bits */
+#define TD_CTLS_PENDING_VE_DISABLE	BIT_ULL(0)
+
 /* TDX hypercall Leaf IDs */
 #define TDVMCALL_MAP_GPA		0x10001
 #define TDVMCALL_GET_QUOTE		0x10002
-- 
2.43.0

[PATCH 4/4] x86/tdx: Enable ENUM_TOPOLOGY

[Kirill A. Shutemov]

TDX 1.0 generates a #VE when accessing topology-related CPUID leafs
(0xB and 0x1F) and the X2APIC_APICID MSR. The kernel returns all
zeros on CPUID #VEs. In practice, this means that the kernel can only
boot with a plain topology. Any complications will cause problems.

The ENUM_TOPOLOGY feature allows the VMM to provide topology
information to the guest in a safe manner. Enabling the feature
eliminates topology-related #VEs: the TDX module virtualizes
accesses to the CPUID leafs and the MSR.

Enable ENUM_TOPOLOGY if it is available.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
 arch/x86/coco/tdx/tdx.c           | 20 ++++++++++++++++++++
 arch/x86/include/asm/shared/tdx.h |  3 +++
 2 files changed, 23 insertions(+)

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index d9ea82f8772d..291e45db8d54 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -280,6 +280,26 @@ static void tdx_setup(u64 *cc_mask)
 		else
 			tdx_panic(msg);
 	}
+
+	/*
+	 * TDX 1.0 generates a #VE when accessing topology-related CPUID leafs
+	 * (0xB and 0x1F) and the X2APIC_APICID MSR. The kernel returns all
+	 * zeros on CPUID #VEs. In practice, this means that the kernel can only
+	 * boot with a plain topology. Any complications will cause problems.
+	 *
+	 * The ENUM_TOPOLOGY feature allows the VMM to provide topology
+	 * information to the guest in a safe manner. Enabling the feature
+	 * eliminates topology-related #VEs: the TDX module virtualizes
+	 * accesses to the CPUID leafs and the MSR.
+	 *
+	 * Enable ENUM_TOPOLOGY if it is available.
+	 */
+	if ((features & TDX_FEATURES0_ENUM_TOPOLOGY) &&
+	    tdg_vm_rd(TDCS_TOPOLOGY_ENUM_CONFIGURED)) {
+		tdg_vm_wr(TDCS_TD_CTLS,
+			  TD_CTLS_ENUM_TOPOLOGY,
+			  TD_CTLS_ENUM_TOPOLOGY);
+	}
 }
 
 /*
diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
index 29a61c72e4dd..2964c506b241 100644
--- a/arch/x86/include/asm/shared/tdx.h
+++ b/arch/x86/include/asm/shared/tdx.h
@@ -27,15 +27,18 @@
 #define TDCS_CONFIG_FLAGS		0x1110000300000016
 #define TDCS_TD_CTLS			0x1110000300000017
 #define TDCS_NOTIFY_ENABLES		0x9100000000000010
+#define TDCS_TOPOLOGY_ENUM_CONFIGURED	0x9100000000000019
 
 /* TDCS_TDX_FEATURES0 bits */
 #define TDX_FEATURES0_PENDING_EPT_VIOLATION_V2	BIT_ULL(16)
+#define TDX_FEATURES0_ENUM_TOPOLOGY		BIT_ULL(20)
 
 /* TDCS_CONFIG_FLAGS bits */
 #define TDCS_CONFIG_FLEXIBLE_PENDING_VE	BIT_ULL(1)
 
 /* TDCS_TD_CTLS bits */
 #define TD_CTLS_PENDING_VE_DISABLE	BIT_ULL(0)
+#define TD_CTLS_ENUM_TOPOLOGY		BIT_ULL(1)
 
 /* TDX hypercall Leaf IDs */
 #define TDVMCALL_MAP_GPA		0x10001
-- 
2.43.0

Re: [PATCH 4/4] x86/tdx: Enable ENUM_TOPOLOGY

[Kuppuswamy Sathyanarayanan]

On 3/9/24 1:02 PM, Kirill A. Shutemov wrote:
> TDX 1.0 generates a #VE when accessing topology-related CPUID leafs
> (0xB and 0x1F) and the X2APIC_APICID MSR. The kernel returns all
> zeros on CPUID #VEs. In practice, this means that the kernel can only
> boot with a plain topology. Any complications will cause problems.

Is this issue only for TDX 1.0? What about TDX > 1.0?

>
> The ENUM_TOPOLOGY feature allows the VMM to provide topology
> information to the guest in a safe manner. Enabling the feature
> eliminates topology-related #VEs: the TDX module virtualizes
> accesses to the CPUID leafs and the MSR.
>
> Enable ENUM_TOPOLOGY if it is available.

I cant find the ENUM_TOPOLOGY in ABI spec (https://cdrdv2.intel.com/v1/dl/getContent/795381).

Can you point me to the correct document?

>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> ---
>  arch/x86/coco/tdx/tdx.c           | 20 ++++++++++++++++++++
>  arch/x86/include/asm/shared/tdx.h |  3 +++
>  2 files changed, 23 insertions(+)
>
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index d9ea82f8772d..291e45db8d54 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -280,6 +280,26 @@ static void tdx_setup(u64 *cc_mask)
>  		else
>  			tdx_panic(msg);
>  	}
> +
> +	/*
> +	 * TDX 1.0 generates a #VE when accessing topology-related CPUID leafs
> +	 * (0xB and 0x1F) and the X2APIC_APICID MSR. The kernel returns all
> +	 * zeros on CPUID #VEs. In practice, this means that the kernel can only
> +	 * boot with a plain topology. Any complications will cause problems.
> +	 *
> +	 * The ENUM_TOPOLOGY feature allows the VMM to provide topology
> +	 * information to the guest in a safe manner. Enabling the feature
> +	 * eliminates topology-related #VEs: the TDX module virtualizes
> +	 * accesses to the CPUID leafs and the MSR.
> +	 *
> +	 * Enable ENUM_TOPOLOGY if it is available.
> +	 */

Why are we overriding it in guest? Why can't this by done by TDX Module
or QEMU during initialization?

> +	if ((features & TDX_FEATURES0_ENUM_TOPOLOGY) &&
> +	    tdg_vm_rd(TDCS_TOPOLOGY_ENUM_CONFIGURED)) {
> +		tdg_vm_wr(TDCS_TD_CTLS,
> +			  TD_CTLS_ENUM_TOPOLOGY,
> +			  TD_CTLS_ENUM_TOPOLOGY);
> +	}
>  }
>  
>  /*
> diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
> index 29a61c72e4dd..2964c506b241 100644
> --- a/arch/x86/include/asm/shared/tdx.h
> +++ b/arch/x86/include/asm/shared/tdx.h
> @@ -27,15 +27,18 @@
>  #define TDCS_CONFIG_FLAGS		0x1110000300000016
>  #define TDCS_TD_CTLS			0x1110000300000017
>  #define TDCS_NOTIFY_ENABLES		0x9100000000000010
> +#define TDCS_TOPOLOGY_ENUM_CONFIGURED	0x9100000000000019
>  
>  /* TDCS_TDX_FEATURES0 bits */
>  #define TDX_FEATURES0_PENDING_EPT_VIOLATION_V2	BIT_ULL(16)
> +#define TDX_FEATURES0_ENUM_TOPOLOGY		BIT_ULL(20)
>  
>  /* TDCS_CONFIG_FLAGS bits */
>  #define TDCS_CONFIG_FLEXIBLE_PENDING_VE	BIT_ULL(1)
>  
>  /* TDCS_TD_CTLS bits */
>  #define TD_CTLS_PENDING_VE_DISABLE	BIT_ULL(0)
> +#define TD_CTLS_ENUM_TOPOLOGY		BIT_ULL(1)
>  
>  /* TDX hypercall Leaf IDs */
>  #define TDVMCALL_MAP_GPA		0x10001

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Re: [PATCH 4/4] x86/tdx: Enable ENUM_TOPOLOGY

[Kirill A. Shutemov]

On Sat, Mar 09, 2024 at 07:56:11PM -0800, Kuppuswamy Sathyanarayanan wrote:
> 
> On 3/9/24 1:02 PM, Kirill A. Shutemov wrote:
> > TDX 1.0 generates a #VE when accessing topology-related CPUID leafs
> > (0xB and 0x1F) and the X2APIC_APICID MSR. The kernel returns all
> > zeros on CPUID #VEs. In practice, this means that the kernel can only
> > boot with a plain topology. Any complications will cause problems.
> 
> Is this issue only for TDX 1.0? What about TDX > 1.0?

TDX 1.0 defines the baseline for TDX. Any change in behaviour will be
gated by explicit opt-in. In this case, it is ENUM_TOPOLOGY.

> > The ENUM_TOPOLOGY feature allows the VMM to provide topology
> > information to the guest in a safe manner. Enabling the feature
> > eliminates topology-related #VEs: the TDX module virtualizes
> > accesses to the CPUID leafs and the MSR.
> >
> > Enable ENUM_TOPOLOGY if it is available.
> 
> I cant find the ENUM_TOPOLOGY in ABI spec (https://cdrdv2.intel.com/v1/dl/getContent/795381).
> 
> Can you point me to the correct document?

I guess JSON dump is out of sync with the PDF doc. See this:

https://cdrdv2.intel.com/v1/dl/getContent/733579

-- 
  Kiryl Shutsemau / Kirill A. Shutemov