linux-kernel - Re: [PATCHv5 4/4] x86/tdx: Enable CPU topology enumeration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f6b4wziuumuorfp3dbkrqc5kzuv3linrbreptpiw6iwnf3u2rb@ttq3rt6lxntv>
Date: Wed, 3 Jul 2024 15:33:09 +0300
From: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
To: Xiaoyao Li <xiaoyao.li@...el.com>
Cc: Dave Hansen <dave.hansen@...ux.intel.com>, 
	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, 
	x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>, linux-coco@...ts.linux.dev, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCHv5 4/4] x86/tdx: Enable CPU topology enumeration

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@...ux.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