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

[Teddy Astie <teddy.astie@xxxxxxxxxx>]

Le 02/08/2026 à 15:14, Sergii Dmytruk a écrit :
> From: Michał Żygowski <michal.zygowski@xxxxxxxxx>
>
> Report TXT capabilities so that dom0 can query the Intel TXT or AMD
> SKINIT support information using xl dmesg.
>
> Signed-off-by: Michał Żygowski <michal.zygowski@xxxxxxxxx>
> Signed-off-by: Sergii Dmytruk <sergii.dmytruk@xxxxxxxxx>
> ---
>
> 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

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature