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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2463ba67-aa8c-4f41-8f13-f1936a4f457a@intel.com>
Date: Fri, 6 Dec 2024 16:45:01 +0800
From: Xiaoyao Li <xiaoyao.li@...el.com>
To: Rick Edgecombe <rick.p.edgecombe@...el.com>, pbonzini@...hat.com,
 seanjc@...gle.com
Cc: yan.y.zhao@...el.com, isaku.yamahata@...il.com, kai.huang@...el.com,
 kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
 tony.lindgren@...ux.intel.com, reinette.chatre@...el.com,
 Isaku Yamahata <isaku.yamahata@...el.com>,
 Binbin Wu <binbin.wu@...ux.intel.com>
Subject: Re: [PATCH v2 16/25] KVM: TDX: Get system-wide info about TDX module
 on initialization

On 10/31/2024 3:00 AM, Rick Edgecombe wrote:
> From: Isaku Yamahata <isaku.yamahata@...el.com>
> 
> 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@...el.com>
> Co-developed-by: Xiaoyao Li <xiaoyao.li@...el.com>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@...el.com>
> Co-developed-by: Tony Lindgren <tony.lindgren@...ux.intel.com>
> Signed-off-by: Tony Lindgren <tony.lindgren@...ux.intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@...el.com>
> Reviewed-by: Binbin Wu <binbin.wu@...ux.intel.com>
> ---
> 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.
          */