Re: [PATCH v2 16/25] KVM: TDX: Get system-wide info about TDX module on initialization

[Xiaoyao Li]

On 10/31/2024 3:00 AM, Rick Edgecombe wrote:
> From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
>
> TDX KVM needs system-wide information about the TDX module. Generate the
> data based on tdx_sysinfo td_conf CPUID data.
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> Co-developed-by: Xiaoyao Li <xiaoyao.li@xxxxxxxxx>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@xxxxxxxxx>
> Co-developed-by: Tony Lindgren <tony.lindgren@xxxxxxxxxxxxxxx>
> Signed-off-by: Tony Lindgren <tony.lindgren@xxxxxxxxxxxxxxx>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
> Reviewed-by: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>
> ---
> uAPI breakout v2:
>   - Update stale patch description (Binbin)
>   - Add KVM_TDX_CAPABILITIES where it's first used (Binbin)
>   - Drop Drop unused KVM_TDX_CPUID_NO_SUBLEAF (Chao)
>   - Drop mmu.h, it's only needed in later patches (Binbin)
>   - Fold in Xiaoyao's capabilities changes (Tony)
>   - Generate data without struct kvm_tdx_caps (Tony)
>   - Use struct kvm_cpuid_entry2 as suggested (Binbin)
>   - Use helpers for phys_addr_bits (Paolo)
>   - Check TDX and KVM capabilities on _tdx_bringup() (Xiaoyao)
>   - Change code around cpuid_config_value since
>     struct tdx_cpuid_config_value {} is removed (Kai)
>
> uAPI breakout v1:
>   - Mention about hardware_unsetup(). (Binbin)
>   - Added Reviewed-by. (Binbin)
>   - Eliminated tdx_md_read(). (Kai)
>   - Include "x86_ops.h" to tdx.c as the patch to initialize TDX module
>     doesn't include it anymore.
>   - Introduce tdx_vm_ioctl() as the first tdx func in x86_ops.h
>
> v19:
>   - Added features0
>   - Use tdx_sys_metadata_read()
>   - Fix error recovery path by Yuan
>
> Change v18:
>   - Newly Added
> ---
>   arch/x86/include/uapi/asm/kvm.h |   9 +++
>   arch/x86/kvm/vmx/tdx.c          | 137 ++++++++++++++++++++++++++++++++
>   2 files changed, 146 insertions(+)
>
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index b6cb87f2b477..0630530af334 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -928,6 +928,8 @@ struct kvm_hyperv_eventfd {
>   
>   /* Trust Domain eXtension sub-ioctl() commands. */
>   enum kvm_tdx_cmd_id {
> +	KVM_TDX_CAPABILITIES = 0,
> +
>   	KVM_TDX_CMD_NR_MAX,
>   };
>   
> @@ -950,4 +952,11 @@ struct kvm_tdx_cmd {
>   	__u64 hw_error;
>   };
>   
> +struct kvm_tdx_capabilities {
> +	__u64 supported_attrs;
> +	__u64 supported_xfam;
> +	__u64 reserved[254];
> +	struct kvm_cpuid2 cpuid;

Could we rename it to "configurable_cpuid" to call out that it only 
reports the bits that are allowable for userspace to configure at 0 or 1 
at will.

If could we can even add a comment of

	/*
         * Bit of value 1 means the bit is free to be configured as
         * 0/1 by userspace VMM.
         * The value 0 only means the bit is not configurable, while
         * the actual value of it depends on TDX module/KVM
         * implementation.
         */