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

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 20/08/2026 2:14 pm, Sergii Dmytruk wrote:
> On Tue, Aug 18, 2026 at 02:08:48PM +0200, Jan Beulich wrote:
>> On 02.08.2026 15:09, Sergii Dmytruk wrote:
>>> @@ -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) :);
>> Nit (style): Bad indentation, missing blanks, unnecessary \n, and stray colon.
>> Overall:
>>
>>     asm volatile ( "getsec"
>>                    : "=a" (getsec_caps)
>>                    : "a" (GETSEC_CAPABILITIES), "b" (0) );
>>
>> I further question the need for volatile here. (Like for we have for CPUID, we
>> anyway may want to gain a getsec() wrapper for GETSEC.)
> I think `volatile` was added just because it doesn't hurt, rather than
> because it's necessary, so it can be dropped.  Can add a wrapper, but
> there is only one use so far and a generic wrapper will have to use
> 64-bit parameters (`GETSEC[EXITAC]` sets RBX).

GETSEC is a horrible instruction.  The different functions (eax input)
produce and consume different registers,

This in turn requires different volatilities.  GETSEC[CAPABILITIES] and
GETSEC[PARAMETERS] should be non-volatile (they're read-only operation
without interesting side effects which the optimiser can safely discard)
whereas GETSEC[ENTERACCS] or GETSEC[SENTER] are really "jump into new
processor mode".  They're both longjmp-like so are considered volatile
by virtue of having no outputs.

I was going to request a getsec.h header to abstract these away.  Having
more than one location where we need to carefully check the asm
constraints against the SDM is too many.

~Andrew