Re: [PATCH v7 010/102] x86/virt/tdx: Add a helper function to return system wide info about TDX module

Yuan Yao
2022-07-07T02:46:07Z

On Mon, Jun 27, 2022 at 02:53:02PM -0700, isaku.yamahata@intel.com wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
>
> TDX KVM needs system-wide information about the TDX module, struct
> tdsysinfo_struct. Add a helper function tdx_get_sysinfo() to return it
> instead of KVM getting it with various error checks. Move out the struct
> definition about it to common place tdx_host.h.

Please correct the tdx_host.h to tdx.h or arch/x86/include/asm/tdx.h

>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
> arch/x86/include/asm/tdx.h | 55 +++++++++++++++++++++++++++++++++++++
> arch/x86/virt/vmx/tdx/tdx.c | 20 +++++++++++---
> arch/x86/virt/vmx/tdx/tdx.h | 52 -----------------------------------
> 3 files changed, 71 insertions(+), 56 deletions(-)
>
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 801f6e10b2db..dfea0dd71bc1 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -89,11 +89,66 @@ static inline long tdx_kvm_hypercall(unsigned int nr, unsigned long p1,
> #endif /* CONFIG_INTEL_TDX_GUEST && CONFIG_KVM_GUEST */
>
> #ifdef CONFIG_INTEL_TDX_HOST
> +struct tdx_cpuid_config {
> + u32 leaf;
> + u32 sub_leaf;
> + u32 eax;
> + u32 ebx;
> + u32 ecx;
> + u32 edx;
> +} __packed;
> +
> +#define TDSYSINFO_STRUCT_SIZE 1024
> +#define TDSYSINFO_STRUCT_ALIGNMENT 1024
> +
> +struct tdsysinfo_struct {
> + /* TDX-SEAM Module Info */
> + u32 attributes;
> + u32 vendor_id;
> + u32 build_date;
> + u16 build_num;
> + u16 minor_version;
> + u16 major_version;
> + u8 reserved0[14];
> + /* Memory Info */
> + u16 max_tdmrs;
> + u16 max_reserved_per_tdmr;
> + u16 pamt_entry_size;
> + u8 reserved1[10];
> + /* Control Struct Info */
> + u16 tdcs_base_size;
> + u8 reserved2[2];
> + u16 tdvps_base_size;
> + u8 tdvps_xfam_dependent_size;
> + u8 reserved3[9];
> + /* TD Capabilities */
> + u64 attributes_fixed0;
> + u64 attributes_fixed1;
> + u64 xfam_fixed0;
> + u64 xfam_fixed1;
> + u8 reserved4[32];
> + u32 num_cpuid_config;
> + /*
> + * The actual number of CPUID_CONFIG depends on above
> + * 'num_cpuid_config'. The size of 'struct tdsysinfo_struct'
> + * is 1024B defined by TDX architecture. Use a union with
> + * specific padding to make 'sizeof(struct tdsysinfo_struct)'
> + * equal to 1024.
> + */
> + union {
> + struct tdx_cpuid_config cpuid_configs[0];
> + u8 reserved5[892];
> + };
> +} __packed __aligned(TDSYSINFO_STRUCT_ALIGNMENT);
> +
> bool platform_tdx_enabled(void);
> int tdx_init(void);
> +const struct tdsysinfo_struct *tdx_get_sysinfo(void);
> #else /* !CONFIG_INTEL_TDX_HOST */
> static inline bool platform_tdx_enabled(void) { return false; }
> static inline int tdx_init(void) { return -ENODEV; }
> +struct tdsysinfo_struct;
> +static inline const struct tdsysinfo_struct *tdx_get_sysinfo(void) { return NULL; }
> #endif /* CONFIG_INTEL_TDX_HOST */
>
> #endif /* !__ASSEMBLY__ */
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index f9a6f8bdade8..14f53494156c 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -364,9 +364,9 @@ static int check_cmrs(struct cmr_info *cmr_array, int *actual_cmr_num)
> return 0;
> }
>
> -static int tdx_get_sysinfo(struct tdsysinfo_struct *tdsysinfo,
> - struct cmr_info *cmr_array,
> - int *actual_cmr_num)
> +static int __tdx_get_sysinfo(struct tdsysinfo_struct *tdsysinfo,
> + struct cmr_info *cmr_array,
> + int *actual_cmr_num)
> {
> struct tdx_module_output out;
> u64 ret;
> @@ -393,6 +393,18 @@ static int tdx_get_sysinfo(struct tdsysinfo_struct *tdsysinfo,
> return check_cmrs(cmr_array, actual_cmr_num);
> }
>
> +const struct tdsysinfo_struct *tdx_get_sysinfo(void)
> +{
> + const struct tdsysinfo_struct *r = NULL;
> +
> + mutex_lock(&tdx_module_lock);
> + if (tdx_module_status == TDX_MODULE_INITIALIZED)
> + r = &tdx_sysinfo;
> + mutex_unlock(&tdx_module_lock);
> + return r;
> +}
> +EXPORT_SYMBOL_GPL(tdx_get_sysinfo);
> +
> /*
> * Skip the memory region below 1MB. Return true if the entire
> * region is skipped. Otherwise, the updated range is returned.
> @@ -1116,7 +1128,7 @@ static int init_tdx_module(void)
> if (ret)
> goto out;
>
> - ret = tdx_get_sysinfo(&tdx_sysinfo, tdx_cmr_array, &tdx_cmr_num);
> + ret = __tdx_get_sysinfo(&tdx_sysinfo, tdx_cmr_array, &tdx_cmr_num);
> if (ret)
> goto out;
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index e0309558be13..c08e4ee2d0bf 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -65,58 +65,6 @@ struct cmr_info {
> #define MAX_CMRS 32
> #define CMR_INFO_ARRAY_ALIGNMENT 512
>
> -struct cpuid_config {
> - u32 leaf;
> - u32 sub_leaf;
> - u32 eax;
> - u32 ebx;
> - u32 ecx;
> - u32 edx;
> -} __packed;
> -
> -#define TDSYSINFO_STRUCT_SIZE 1024
> -#define TDSYSINFO_STRUCT_ALIGNMENT 1024
> -
> -struct tdsysinfo_struct {
> - /* TDX-SEAM Module Info */
> - u32 attributes;
> - u32 vendor_id;
> - u32 build_date;
> - u16 build_num;
> - u16 minor_version;
> - u16 major_version;
> - u8 reserved0[14];
> - /* Memory Info */
> - u16 max_tdmrs;
> - u16 max_reserved_per_tdmr;
> - u16 pamt_entry_size;
> - u8 reserved1[10];
> - /* Control Struct Info */
> - u16 tdcs_base_size;
> - u8 reserved2[2];
> - u16 tdvps_base_size;
> - u8 tdvps_xfam_dependent_size;
> - u8 reserved3[9];
> - /* TD Capabilities */
> - u64 attributes_fixed0;
> - u64 attributes_fixed1;
> - u64 xfam_fixed0;
> - u64 xfam_fixed1;
> - u8 reserved4[32];
> - u32 num_cpuid_config;
> - /*
> - * The actual number of CPUID_CONFIG depends on above
> - * 'num_cpuid_config'. The size of 'struct tdsysinfo_struct'
> - * is 1024B defined by TDX architecture. Use a union with
> - * specific padding to make 'sizeof(struct tdsysinfo_struct)'
> - * equal to 1024.
> - */
> - union {
> - struct cpuid_config cpuid_configs[0];
> - u8 reserved5[892];
> - };
> -} __packed __aligned(TDSYSINFO_STRUCT_ALIGNMENT);
> -
> struct tdmr_reserved_area {
> u64 offset;
> u64 size;
> --
> 2.25.1
>