Re: [PATCH v4 02/23] x86/cpu: report SMX, TXT and SKINIT capabilities

Teddy Astie <[email protected]>
Newsgroups org.xenproject.lists.xen-devel
Message-ID <1787230800.8631fc262581453bbf619ec5b2062170.1a01f41ca51000c4f3@vates.tech>
Le 02/08/2026 à 15:14, Sergii Dmytruk a écrit :
> From: Michał Żygowski <[email protected]>
> 
> Report TXT capabilities so that dom0 can query the Intel TXT or AMD
> SKINIT support information using xl dmesg.
> 
> Signed-off-by: Michał Żygowski <[email protected]>
> Signed-off-by: Sergii Dmytruk <[email protected]>
> ---
> 
> Notes:
>      v4: fixed conditions for not reporting capabilities (match correct comments)
>      v4: define GETSEC_* macros used only by xen/arch/x86/cpu/intel.c in the file itself (to not depend on Slaunch)
>      v4: don't postpone restoring state of X86_CR4_SMXE, do it before printing test results
> 
>   xen/arch/x86/cpu/amd.c   | 16 +++++++++++++
>   xen/arch/x86/cpu/cpu.h   |  1 +
>   xen/arch/x86/cpu/hygon.c |  1 +
>   xen/arch/x86/cpu/intel.c | 50 ++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 68 insertions(+)
> 
> diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
> index 70783c9a0a..5ea16ad8a8 100644
> --- a/xen/arch/x86/cpu/amd.c
> +++ b/xen/arch/x86/cpu/amd.c
> @@ -617,6 +617,21 @@ void amd_process_freq(const struct cpuinfo_x86 *c,
>   		*low_mhz = amd_parse_freq(c->family, lo);
>   }
>   
> +void amd_log_skinit(const struct cpuinfo_x86 *c)
> +{
> +    /*
> +     * Run only on BSP and not during resume to report the capability only once.
> +     */
> +    if ( system_state == SYS_STATE_resume || smp_processor_id() )
> +        return;
> +
> +    printk("CPU: SKINIT capability ");
> +    if ( !test_bit(X86_FEATURE_SKINIT, &boot_cpu_data.x86_capability) )
> +        printk("not supported\n");
> +    else
> +        printk("supported\n");
> +}
> +
>   void cf_check early_init_amd(struct cpuinfo_x86 *c)
>   {
>   	if (c == &boot_cpu_data)
> @@ -1325,6 +1340,7 @@ static void cf_check init_amd(struct cpuinfo_x86 *c)
>   		setup_force_cpu_cap(X86_FEATURE_XEN_REP_MOVSB);
>   
>   	amd_log_freq(c);
> +	amd_log_skinit(c);
>   }

On which Xen branch this patch is based ?

early_init_amd() doesn't seem to have amd_log_freq (which is in init_amd 
instead).

>   
>   const struct cpu_dev __initconst_cf_clobber amd_cpu_dev = {
> diff --git a/xen/arch/x86/cpu/cpu.h b/xen/arch/x86/cpu/cpu.h
> index bbede57ab0..17935190b7 100644
> --- a/xen/arch/x86/cpu/cpu.h
> +++ b/xen/arch/x86/cpu/cpu.h
> @@ -21,6 +21,7 @@ extern bool detect_extended_topology(struct cpuinfo_x86 *c);
>   
>   void cf_check early_init_amd(struct cpuinfo_x86 *c);
>   void amd_log_freq(const struct cpuinfo_x86 *c);
> +void amd_log_skinit(const struct cpuinfo_x86 *c);
>   void amd_init_de_cfg(const struct cpuinfo_x86 *c);
>   void amd_init_lfence_dispatch(void);
>   void amd_init_ssbd(const struct cpuinfo_x86 *c);
> diff --git a/xen/arch/x86/cpu/hygon.c b/xen/arch/x86/cpu/hygon.c
> index 7a9fc25d31..608a7c4319 100644
> --- a/xen/arch/x86/cpu/hygon.c
> +++ b/xen/arch/x86/cpu/hygon.c
> @@ -90,6 +90,7 @@ static void cf_check init_hygon(struct cpuinfo_x86 *c)
>   	}
>   
>   	amd_log_freq(c);
> +	amd_log_skinit(c);
>   }
>   
>   const struct cpu_dev __initconst_cf_clobber hygon_cpu_dev = {
> diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
> index 90c9d36186..ddb34c0c02 100644
> --- a/xen/arch/x86/cpu/intel.c
> +++ b/xen/arch/x86/cpu/intel.c
> @@ -14,6 +14,11 @@
>   
>   #include "cpu.h"
>   
> +/* EAX value for GETSEC leaf functions. Intel SDM: GETSEC[CAPABILITIES] */
> +#define GETSEC_CAPABILITIES             0
> +/* Intel SDM: GETSEC Capability Result Encoding */
> +#define GETSEC_CAP_TXT_CHIPSET          1
> +
>   /*
>    * MSR_MCU_OPT_CTRL is a collection of unrelated functionality, with separate
>    * enablement requirements, but which want to be consistent across the system.
> @@ -620,6 +625,49 @@ static void init_intel_perf(struct cpuinfo_x86 *c)
>       }
>   }
>   
> +/*
> + * Print out the SMX and TXT capabilties, so that dom0 can determine if the
> + * system is DRTM-capable.
> + */
> +static void intel_log_smx_txt(void)
> +{
> +    unsigned long cr4_val, getsec_caps;
> +
> +    /*
> +     * Run only on BSP and not during resume to report the capability only once.
> +     */
> +    if ( system_state == SYS_STATE_resume || smp_processor_id() )
> +        return;
> +
> +    printk("CPU: SMX capability ");
> +    if ( !test_bit(X86_FEATURE_SMX, &boot_cpu_data.x86_capability) )
> +    {
> +        printk("not supported\n");
> +        return;
> +    }
> +    printk("supported\n");
> +
> +    /* Can't run GETSEC without VMX and SMX */
> +    if ( !test_bit(X86_FEATURE_VMX, &boot_cpu_data.x86_capability) )
> +        return;
> +
> +    cr4_val = read_cr4();
> +    if ( !(cr4_val & X86_CR4_SMXE) )
> +        write_cr4(cr4_val | X86_CR4_SMXE);
> +
> +    asm volatile ("getsec\n"
> +        : "=a" (getsec_caps)
> +        : "a" (GETSEC_CAPABILITIES), "b" (0) :);
> +
> +    if ( !(cr4_val & X86_CR4_SMXE) )
> +        write_cr4(cr4_val & ~X86_CR4_SMXE);
> +

That looks wrong, the logic is written as : if SMXE is cleared, you 
clear SMXE again.
Regardless, if we're looking to use TXT later on, wouldn't it be 
preferable to keep SMXE bit on ? That means we would not only do 
reporting but "basic" initialization.

> +    if ( getsec_caps & GETSEC_CAP_TXT_CHIPSET )
> +        printk("Chipset supports TXT\n");
> +    else
> +        printk("Chipset does not support TXT\n");
> +}
> +
>   static void cf_check init_intel(struct cpuinfo_x86 *c)
>   {
>   	/* Detect the extended topology information if available */
> @@ -634,6 +682,8 @@ static void cf_check init_intel(struct cpuinfo_x86 *c)
>   		detect_ht(c);
>   	}
>   
> +	intel_log_smx_txt();
> +
>   	/* Work around errata */
>   	Intel_errata_workarounds(c);
>   

Teddy
OpenPGP_signature.asc (application/pgp-signature, 665 B)
-----BEGIN PGP SIGNATURE-----

wsD5BAABCAAjFiEEGAIew9LzHY3pdrqtZg+p0QLLz9AFAmqG+k8FAwAAAAAACgkQZg+p0QLLz9A0
Jwv9EZF7DPtiJhv4rCFPnm1OI81u/Sb39PR+e0j0TkNiDDTOX94A+WFezBBH/KQD9T4BdWpEA5Xq
YHhkpojpl7jVQ25wa4MXgboKE/h+u5164WgqNTKwQHKmESQ6APlLrusQFPbqPNKeJe1ej5n1KMsl
+hCZ5Zd0sFcJ9bNRKhHoJFCONi5GurAcMtgF3YQIH+8JfxxX0Rqhi/8tcHYT8OAT0A0T0Pn1Bx3F
csa+Xu3g5vFt5YSLMrtowZz8rGWHhDSZQAU1a6+XrfQk1g6mPE0SoSBB0K3y8rUvLWKCZ4JHrJdv
XCHYAisufJVD2x8nuhE9S5ohluNWjqBPCv67coWUgSQJqud8ifj1D7umpBphx1f2g5NSpqr7OKAe
x2IqURKX5MTQ4mc1rgxAbcxMkvgsGs/ZYtbzOtoPjGOpbdcqrnvhADBmmnI73TmZoMn/pvCce7hD
DS7jiAGPuwKcg4pKAWuX9Qu+8Jb4s3fYfCbPP4BpBN5hPBJuJpgVboXMkaj7
=X/A8
-----END PGP SIGNATURE-----
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.