[PATCHv5 4/4] x86/tdx: Enable CPU topology enumeration

Kirill A. Shutemov posted 4 patches 1 year, 5 months ago
There is a newer version of this series
[PATCHv5 4/4] x86/tdx: Enable CPU topology enumeration
Posted by Kirill A. Shutemov 1 year, 5 months ago
TDX 1.0 defines baseline behaviour of TDX guest platform. In 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
topology. 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. 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           | 27 +++++++++++++++++++++++++++
 arch/x86/include/asm/shared/tdx.h |  2 ++
 2 files changed, 29 insertions(+)

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index ba3103877b21..f6e48119d6fd 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -249,6 +249,32 @@ static void disable_sept_ve(u64 td_attr)
 	return;
 }
 
+/*
+ * 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.
+ * 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.
+ */
+static void enable_cpu_topology_enumeration(void)
+{
+	u64 configured;
+
+	/* Has the VMM provided a valid topology configuration? */
+	tdg_vm_rd(TDCS_TOPOLOGY_ENUM_CONFIGURED, &configured);
+	if (!configured) {
+		pr_err("VMM did not configure X2APIC_IDs properly\n");
+		return;
+	}
+
+	tdg_vm_wr(TDCS_TD_CTLS, TD_CTLS_ENUM_TOPOLOGY, TD_CTLS_ENUM_TOPOLOGY);
+}
+
 static void tdx_setup(u64 *cc_mask)
 {
 	struct tdx_module_args args = {};
@@ -280,6 +306,7 @@ static void tdx_setup(u64 *cc_mask)
 	tdg_vm_wr(TDCS_NOTIFY_ENABLES, 0, -1ULL);
 
 	disable_sept_ve(td_attr);
+	enable_cpu_topology_enumeration();
 }
 
 /*
diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
index fecb2a6e864b..89f7fcade8ae 100644
--- a/arch/x86/include/asm/shared/tdx.h
+++ b/arch/x86/include/asm/shared/tdx.h
@@ -23,12 +23,14 @@
 #define TDCS_CONFIG_FLAGS		0x1110000300000016
 #define TDCS_TD_CTLS			0x1110000300000017
 #define TDCS_NOTIFY_ENABLES		0x9100000000000010
+#define TDCS_TOPOLOGY_ENUM_CONFIGURED	0x9100000000000019
 
 /* 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: [PATCHv5 4/4] x86/tdx: Enable CPU topology enumeration
Posted by Xiaoyao Li 1 year, 5 months ago
On 6/24/2024 7:41 PM, Kirill A. Shutemov wrote:
> TDX 1.0 defines baseline behaviour of TDX guest platform. In 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
> topology. 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. 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           | 27 +++++++++++++++++++++++++++
>   arch/x86/include/asm/shared/tdx.h |  2 ++
>   2 files changed, 29 insertions(+)
> 
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index ba3103877b21..f6e48119d6fd 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -249,6 +249,32 @@ static void disable_sept_ve(u64 td_attr)
>   	return;
>   }
>   
> +/*
> + * 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.
> + * 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.
> + */
> +static void enable_cpu_topology_enumeration(void)
> +{
> +	u64 configured;
> +
> +	/* Has the VMM provided a valid topology configuration? */
> +	tdg_vm_rd(TDCS_TOPOLOGY_ENUM_CONFIGURED, &configured);
> +	if (!configured) {
> +		pr_err("VMM did not configure X2APIC_IDs properly\n");
> +		return;
> +	}
> +
> +	tdg_vm_wr(TDCS_TD_CTLS, TD_CTLS_ENUM_TOPOLOGY, TD_CTLS_ENUM_TOPOLOGY);
> +}
> +
>   static void tdx_setup(u64 *cc_mask)
>   {
>   	struct tdx_module_args args = {};
> @@ -280,6 +306,7 @@ static void tdx_setup(u64 *cc_mask)
>   	tdg_vm_wr(TDCS_NOTIFY_ENABLES, 0, -1ULL);
>   
>   	disable_sept_ve(td_attr);
> +	enable_cpu_topology_enumeration();
>   }
>   
>   /*
> diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
> index fecb2a6e864b..89f7fcade8ae 100644
> --- a/arch/x86/include/asm/shared/tdx.h
> +++ b/arch/x86/include/asm/shared/tdx.h
> @@ -23,12 +23,14 @@
>   #define TDCS_CONFIG_FLAGS		0x1110000300000016
>   #define TDCS_TD_CTLS			0x1110000300000017
>   #define TDCS_NOTIFY_ENABLES		0x9100000000000010

The ID for TDCS_NOTIFY_ENABLES I read from TDX spec is

	0x9110000300000010
	
> +#define TDCS_TOPOLOGY_ENUM_CONFIGURED	0x9100000000000019

and
	0x9110000000000019

for TOPOLOGY_ENUM_CONFIGURED.

Both version of them can work actually. But I'm curious where did you 
get them? just an old version of spec?
	
>   /* 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
Re: [PATCHv5 4/4] x86/tdx: Enable CPU topology enumeration
Posted by Kirill A. Shutemov 1 year, 5 months ago
On Wed, Jul 03, 2024 at 12:17:04AM +0800, Xiaoyao Li wrote:
> On 6/24/2024 7:41 PM, Kirill A. Shutemov wrote:
> > TDX 1.0 defines baseline behaviour of TDX guest platform. In 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
> > topology. 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. 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           | 27 +++++++++++++++++++++++++++
> >   arch/x86/include/asm/shared/tdx.h |  2 ++
> >   2 files changed, 29 insertions(+)
> > 
> > diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> > index ba3103877b21..f6e48119d6fd 100644
> > --- a/arch/x86/coco/tdx/tdx.c
> > +++ b/arch/x86/coco/tdx/tdx.c
> > @@ -249,6 +249,32 @@ static void disable_sept_ve(u64 td_attr)
> >   	return;
> >   }
> > +/*
> > + * 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.
> > + * 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.
> > + */
> > +static void enable_cpu_topology_enumeration(void)
> > +{
> > +	u64 configured;
> > +
> > +	/* Has the VMM provided a valid topology configuration? */
> > +	tdg_vm_rd(TDCS_TOPOLOGY_ENUM_CONFIGURED, &configured);
> > +	if (!configured) {
> > +		pr_err("VMM did not configure X2APIC_IDs properly\n");
> > +		return;
> > +	}
> > +
> > +	tdg_vm_wr(TDCS_TD_CTLS, TD_CTLS_ENUM_TOPOLOGY, TD_CTLS_ENUM_TOPOLOGY);
> > +}
> > +
> >   static void tdx_setup(u64 *cc_mask)
> >   {
> >   	struct tdx_module_args args = {};
> > @@ -280,6 +306,7 @@ static void tdx_setup(u64 *cc_mask)
> >   	tdg_vm_wr(TDCS_NOTIFY_ENABLES, 0, -1ULL);
> >   	disable_sept_ve(td_attr);
> > +	enable_cpu_topology_enumeration();
> >   }
> >   /*
> > diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
> > index fecb2a6e864b..89f7fcade8ae 100644
> > --- a/arch/x86/include/asm/shared/tdx.h
> > +++ b/arch/x86/include/asm/shared/tdx.h
> > @@ -23,12 +23,14 @@
> >   #define TDCS_CONFIG_FLAGS		0x1110000300000016
> >   #define TDCS_TD_CTLS			0x1110000300000017
> >   #define TDCS_NOTIFY_ENABLES		0x9100000000000010
> 
> The ID for TDCS_NOTIFY_ENABLES I read from TDX spec is
> 
> 	0x9110000300000010

> > +#define TDCS_TOPOLOGY_ENUM_CONFIGURED	0x9100000000000019
> 
> and
> 	0x9110000000000019
> 
> for TOPOLOGY_ENUM_CONFIGURED.
> 
> Both version of them can work actually. But I'm curious where did you get
> them? just an old version of spec?

TDX 1.0 spec had the old value. Newer spec defined meaning to previously
reserved bits. The changed bits define element size and context code.
These bits are ignored on read/write by TDX module.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov