Re: [PATCH v17 018/116] KVM: TDX: x86: Add ioctl to get TDX systemwide parameters

[Binbin Wu <binbin.wu@linux.intel.com>]

On 11/7/2023 10:55 PM, isaku.yamahata@intel.com wrote:
> From: Sean Christopherson <sean.j.christopherson@intel.com>
>
> Implement an ioctl to get system-wide parameters for TDX.  Although the
> function is systemwide, vm scoped mem_enc ioctl works for userspace VMM
> like qemu and device scoped version is not define, re-use vm scoped
> mem_enc.
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
> v14 -> v15:
> - ABI change: added supported_gpaw and reserved area,
> ---
>   arch/x86/include/uapi/asm/kvm.h       | 24 ++++++++++
>   arch/x86/kvm/vmx/tdx.c                | 64 +++++++++++++++++++++++++++
>   tools/arch/x86/include/uapi/asm/kvm.h | 52 ++++++++++++++++++++++
>   3 files changed, 140 insertions(+)
>
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index 615fb60b3717..3fbd43d5177b 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -593,4 +593,28 @@ struct kvm_tdx_cmd {
>   	__u64 error;
>   };
>   
> +struct kvm_tdx_cpuid_config {
> +	__u32 leaf;
> +	__u32 sub_leaf;
> +	__u32 eax;
> +	__u32 ebx;
> +	__u32 ecx;
> +	__u32 edx;
> +};
> +
> +struct kvm_tdx_capabilities {
> +	__u64 attrs_fixed0;
> +	__u64 attrs_fixed1;
> +	__u64 xfam_fixed0;
> +	__u64 xfam_fixed1;
> +#define TDX_CAP_GPAW_48	(1 << 0)
> +#define TDX_CAP_GPAW_52	(1 << 1)
> +	__u32 supported_gpaw;
> +	__u32 padding;
> +	__u64 reserved[251];
> +
> +	__u32 nr_cpuid_configs;
> +	struct kvm_tdx_cpuid_config cpuid_configs[];
> +};
> +
>   #endif /* _ASM_X86_KVM_H */
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index ead229e34813..f9e80582865d 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -6,6 +6,7 @@
>   #include "capabilities.h"
>   #include "x86_ops.h"
>   #include "x86.h"
> +#include "mmu.h"
>   #include "tdx.h"
>   
>   #undef pr_fmt
> @@ -16,6 +17,66 @@
>   		offsetof(struct tdsysinfo_struct, cpuid_configs))	\
>   		/ sizeof(struct tdx_cpuid_config))
>   
> +static int tdx_get_capabilities(struct kvm_tdx_cmd *cmd)
> +{
> +	struct kvm_tdx_capabilities __user *user_caps;
> +	const struct tdsysinfo_struct *tdsysinfo;
> +	struct kvm_tdx_capabilities *caps = NULL;
> +	int ret = 0;
> +
> +	BUILD_BUG_ON(sizeof(struct kvm_tdx_cpuid_config) !=
> +		     sizeof(struct tdx_cpuid_config));
> +
> +	if (cmd->flags)
> +		return -EINVAL;
> +
> +	tdsysinfo = tdx_get_sysinfo();
> +	if (!tdsysinfo)
> +		return -EOPNOTSUPP;
> +
> +	caps = kmalloc(sizeof(*caps), GFP_KERNEL);
> +	if (!caps)
> +		return -ENOMEM;
> +
> +	user_caps = (void __user *)cmd->data;
> +	if (copy_from_user(caps, user_caps, sizeof(*caps))) {
> +		ret = -EFAULT;
> +		goto out;
> +	}
> +
> +	if (caps->nr_cpuid_configs < tdsysinfo->num_cpuid_config) {
> +		ret = -E2BIG;
> +		goto out;
> +	}
> +
> +	*caps = (struct kvm_tdx_capabilities) {
> +		.attrs_fixed0 = tdsysinfo->attributes_fixed0,
> +		.attrs_fixed1 = tdsysinfo->attributes_fixed1,
> +		.xfam_fixed0 = tdsysinfo->xfam_fixed0,
> +		.xfam_fixed1 = tdsysinfo->xfam_fixed1,
> +		.supported_gpaw = TDX_CAP_GPAW_48 |
> +		(kvm_get_shadow_phys_bits() >= 52 &&
> +		 cpu_has_vmx_ept_5levels()) ? TDX_CAP_GPAW_52 : 0,
> +		.nr_cpuid_configs = tdsysinfo->num_cpuid_config,
> +		.padding = 0,
> +	};
> +
> +	if (copy_to_user(user_caps, caps, sizeof(*caps))) {
> +		ret = -EFAULT;
> +		goto out;
> +	}
> +	if (copy_to_user(user_caps->cpuid_configs, &tdsysinfo->cpuid_configs,
> +			 tdsysinfo->num_cpuid_config *
> +			 sizeof(struct tdx_cpuid_config))) {
> +		ret = -EFAULT;
> +	}
> +
> +out:
> +	/* kfree() accepts NULL. */
> +	kfree(caps);
> +	return ret;
> +}
> +
>   int tdx_vm_ioctl(struct kvm *kvm, void __user *argp)
>   {
>   	struct kvm_tdx_cmd tdx_cmd;
> @@ -29,6 +90,9 @@ int tdx_vm_ioctl(struct kvm *kvm, void __user *argp)
>   	mutex_lock(&kvm->lock);
>   
>   	switch (tdx_cmd.id) {
> +	case KVM_TDX_CAPABILITIES:
> +		r = tdx_get_capabilities(&tdx_cmd);
> +		break;
>   	default:
>   		r = -EINVAL;
>   		goto out;
> diff --git a/tools/arch/x86/include/uapi/asm/kvm.h b/tools/arch/x86/include/uapi/asm/kvm.h
> index 1a6a1f987949..7a08723e99e2 100644
> --- a/tools/arch/x86/include/uapi/asm/kvm.h
> +++ b/tools/arch/x86/include/uapi/asm/kvm.h

According to the discussion, Sean's preference is "never update KVM's
uapi headers in tools/ in KVM's tree"
https://lore.kernel.org/all/Y8bZ%2FJ98V5i3wG%2Fv@google.com/

> @@ -562,4 +562,56 @@ struct kvm_pmu_event_filter {
>   /* x86-specific KVM_EXIT_HYPERCALL flags. */
>   #define KVM_EXIT_HYPERCALL_LONG_MODE	BIT(0)
>   
> +/* Trust Domain eXtension sub-ioctl() commands. */
> +enum kvm_tdx_cmd_id {
> +	KVM_TDX_CAPABILITIES = 0,
> +
> +	KVM_TDX_CMD_NR_MAX,
> +};
> +
> +struct kvm_tdx_cmd {
> +	/* enum kvm_tdx_cmd_id */
> +	__u32 id;
> +	/* flags for sub-commend. If sub-command doesn't use this, set zero. */
> +	__u32 flags;
> +	/*
> +	 * data for each sub-command. An immediate or a pointer to the actual
> +	 * data in process virtual address.  If sub-command doesn't use it,
> +	 * set zero.
> +	 */
> +	__u64 data;
> +	/*
> +	 * Auxiliary error code.  The sub-command may return TDX SEAMCALL
> +	 * status code in addition to -Exxx.
> +	 * Defined for consistency with struct kvm_sev_cmd.
> +	 */
> +	__u64 error;
> +	/* Reserved: Defined for consistency with struct kvm_sev_cmd. */
> +	__u64 unused;
> +};
> +
> +struct kvm_tdx_cpuid_config {
> +	__u32 leaf;
> +	__u32 sub_leaf;
> +	__u32 eax;
> +	__u32 ebx;
> +	__u32 ecx;
> +	__u32 edx;
> +};
> +
> +struct kvm_tdx_capabilities {
> +	__u64 attrs_fixed0;
> +	__u64 attrs_fixed1;
> +	__u64 xfam_fixed0;
> +	__u64 xfam_fixed1;
> +#define TDX_CAP_GPAW_48		(1 << 0)
> +#define TDX_CAP_GPAW_52		(1 << 1)
> +	__u32 supported_gpaw;
> +	__u32 padding;
> +	__u64 reserved[251];
> +
> +	__u32 nr_cpuid_configs;
> +	struct kvm_tdx_cpuid_config cpuid_configs[];
> +};
> +
>   #endif /* _ASM_X86_KVM_H */