Re: [PATCH v8 55/55] docs: Add TDX documentation

[Xiaoyao Li <xiaoyao.li@intel.com>]

On 4/2/2025 6:50 PM, Daniel P. Berrangé wrote:
> CC libvirt / Jiri, for confirmation about whether the CPUID restrictions
> listed below will have any possible impact on libvirt CPUID handling...
> 
> On Tue, Apr 01, 2025 at 09:02:05AM -0400, Xiaoyao Li wrote:
>> Add docs/system/i386/tdx.rst for TDX support, and add tdx in
>> confidential-guest-support.rst
>>
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> ---
>> Changes in v6:
>>   - Add more information of "Feature configuration"
>>   - Mark TD Attestation as future work because KVM now drops the support
>>     of it.
>>
>> Changes in v5:
>>   - Add TD attestation section and update the QEMU parameter;
>>
>> Changes since v1:
>>   - Add prerequisite of private gmem;
>>   - update example command to launch TD;
>>
>> Changes since RFC v4:
>>   - add the restriction that kernel-irqchip must be split
>> ---
>>   docs/system/confidential-guest-support.rst |   1 +
>>   docs/system/i386/tdx.rst                   | 156 +++++++++++++++++++++
>>   docs/system/target-i386.rst                |   1 +
>>   3 files changed, 158 insertions(+)
>>   create mode 100644 docs/system/i386/tdx.rst
> 
> 
>> +Feature Configuration
>> +---------------------
>> +
>> +Unlike non-TDX VM, the CPU features (enumerated by CPU or MSR) of a TD are not
>> +under full control of VMM. VMM can only configure part of features of a TD on
>> +``KVM_TDX_INIT_VM`` command of VM scope ``MEMORY_ENCRYPT_OP`` ioctl.
>> +
>> +The configurable features have three types:
>> +
>> +- Attributes:
>> +  - PKS (bit 30) controls whether Supervisor Protection Keys is exposed to TD,
>> +  which determines related CPUID bit and CR4 bit;
>> +  - PERFMON (bit 63) controls whether PMU is exposed to TD.
>> +
>> +- XSAVE related features (XFAM):
>> +  XFAM is a 64b mask, which has the same format as XCR0 or IA32_XSS MSR. It
>> +  determines the set of extended features available for use by the guest TD.
>> +
>> +- CPUID features:
>> +  Only some bits of some CPUID leaves are directly configurable by VMM.
>> +
>> +What features can be configured is reported via TDX capabilities.
>> +
>> +TDX capabilities
>> +~~~~~~~~~~~~~~~~
>> +
>> +The VM scope ``MEMORY_ENCRYPT_OP`` ioctl provides command ``KVM_TDX_CAPABILITIES``
>> +to get the TDX capabilities from KVM. It returns a data structure of
>> +``struct kvm_tdx_capabilities``, which tells the supported configuration of
>> +attributes, XFAM and CPUIDs.
>> +
>> +TD attributes
>> +~~~~~~~~~~~~~
>> +
>> +QEMU supports configuring raw 64-bit TD attributes directly via "attributes"
>> +property of "tdx-guest" object. Note, it's users' responsibility to provide a
>> +valid value because some bits may not supported by current QEMU or KVM yet.
>> +
>> +QEMU also supports the configuration of individual attribute bits that are
>> +supported by it, via properties of "tdx-guest" object.
>> +E.g., "sept-ve-disable" (bit 28).
>> +
>> +MSR based features
>> +~~~~~~~~~~~~~~~~~~
>> +
>> +Current KVM doesn't support MSR based feature (e.g., MSR_IA32_ARCH_CAPABILITIES)
>> +configuration for TDX, and it's a future work to enable it in QEMU when KVM adds
>> +support of it.
>> +
>> +Feature check
>> +~~~~~~~~~~~~~
>> +
>> +QEMU checks if the final (CPU) features, determined by given cpu model and
>> +explicit feature adjustment of "+featureA/-featureB", can be supported or not.
>> +It can produce feature not supported warning like
>> +
>> +  "warning: host doesn't support requested feature: CPUID.07H:EBX.intel-pt [bit 25]"
>> +
>> +It can also produce warning like
>> +
>> +  "warning: TDX forcibly sets the feature: CPUID.80000007H:EDX.invtsc [bit 8]"
>> +
>> +if the fixed-1 feature is requested to be disabled explicitly. This is newly
>> +added to QEMU for TDX because TDX has fixed-1 features that are forcibly enabled
>> +by TDX module and VMM cannot disable them.
> 
> This is where I'm wondering if libvirt has anything to be concerned
> about. Possibly when libvirt queries the actual CPUID after launching
> the guest it will just "do the right thing" ? Wondering if there's any
> need for libvirt to be aware of CPUID restrictions before that point
> though ?
> 
> 
>> +
>> +Launching a TD (TDX VM)
>> +-----------------------
>> +
>> +To launch a TD, the necessary command line options are tdx-guest object and
>> +split kernel-irqchip, as below:
>> +
>> +.. parsed-literal::
>> +
>> +    |qemu_system_x86| \\
>> +        -object tdx-guest,id=tdx0 \\
>> +        -machine ...,kernel-irqchip=split,confidential-guest-support=tdx0 \\
>> +        -bios OVMF.fd \\
> 
> I don't think we need to show 'kernel-irqchip=split' now that we "do the
> right thing" by default
> 
> 
> This surely also ought to include '-accel kvm', as IIUC there's no
> TCG support for TDX.
> 
> And presumably '-cpu host', since QEMU's default 'qemu64' CPU model
> is likely a terrible match for what TDX will force set.
> 
>> +
>> +Restrictions
>> +------------
>> +
>> + - kernel-irqchip must be split;
> 
> Can append
> 
>     "This is set by default for TDX guests if kernel-irqchip is left on
>      its default 'auto' setting."

This and above feedback are all good. Accept them all and will update in 
the next version.

Thanks!

>> +
>> + - No readonly support for private memory;
>> +
>> + - No SMM support: SMM support requires manipulating the guest register states
>> +   which is not allowed;
>> +
> 
> 
> With regards,
> Daniel