linux-kernel - Re: [PATCH 11/25] KVM: TDX: Report kvm_tdx_caps in KVM_TDX

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: <ZrrUanu3xYZyIshY@chao-email>
Date: Tue, 13 Aug 2024 11:35:06 +0800
From: Chao Gao <chao.gao@...el.com>
To: Rick Edgecombe <rick.p.edgecombe@...el.com>
CC: <seanjc@...gle.com>, <pbonzini@...hat.com>, <kvm@...r.kernel.org>,
	<kai.huang@...el.com>, <isaku.yamahata@...il.com>,
	<tony.lindgren@...ux.intel.com>, <xiaoyao.li@...el.com>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 11/25] KVM: TDX: Report kvm_tdx_caps in
 KVM_TDX_CAPABILITIES

On Mon, Aug 12, 2024 at 03:48:06PM -0700, Rick Edgecombe wrote:
>From: Xiaoyao Li <xiaoyao.li@...el.com>
>
>Report raw capabilities of TDX module to userspace isn't so useful
>and incorrect, because some of the capabilities might not be supported
>by KVM.
>
>Instead, report the KVM capp'ed capbilities to userspace.
>
>Removed the supported_gpaw field. Because CPUID.0x80000008.EAX[23:16] of
>KVM_SUPPORTED_CPUID enumerates the 5 level EPT support, i.e., if GPAW52
>is supported or not. Note, GPAW48 should be always supported. Thus no
>need for explicit enumeration.
>
>Signed-off-by: Xiaoyao Li <xiaoyao.li@...el.com>
>Signed-off-by: Rick Edgecombe <rick.p.edgecombe@...el.com>
>---
>uAPI breakout v1:
> - Code change due to previous patches changed to use exported 'struct
>   tdx_sysinfo' pointer.
>---
> arch/x86/include/uapi/asm/kvm.h | 14 +++----------
> arch/x86/kvm/vmx/tdx.c          | 36 ++++++++-------------------------
> 2 files changed, 11 insertions(+), 39 deletions(-)
>
>diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
>index c9eb2e2f5559..2e3caa5a58fd 100644
>--- a/arch/x86/include/uapi/asm/kvm.h
>+++ b/arch/x86/include/uapi/asm/kvm.h
>@@ -961,18 +961,10 @@ struct kvm_tdx_cpuid_config {
> 	__u32 edx;
> };
> 
>-/* supported_gpaw */
>-#define TDX_CAP_GPAW_48	(1 << 0)
>-#define TDX_CAP_GPAW_52	(1 << 1)
>-
> struct kvm_tdx_capabilities {
>-	__u64 attrs_fixed0;
>-	__u64 attrs_fixed1;
>-	__u64 xfam_fixed0;
>-	__u64 xfam_fixed1;
>-	__u32 supported_gpaw;
>-	__u32 padding;
>-	__u64 reserved[251];
>+	__u64 supported_attrs;
>+	__u64 supported_xfam;
>+	__u64 reserved[254];

I wonder why this patch and patch 9 weren't squashed together. Many changes
added by patch 9 are removed here.

> 
> 	__u32 nr_cpuid_configs;
> 	struct kvm_tdx_cpuid_config cpuid_configs[];
>diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>index d89973e554f6..f9faec217ea9 100644
>--- a/arch/x86/kvm/vmx/tdx.c
>+++ b/arch/x86/kvm/vmx/tdx.c
>@@ -49,7 +49,7 @@ static int tdx_get_capabilities(struct kvm_tdx_cmd *cmd)
> 	const struct tdx_sysinfo_td_conf *td_conf = &tdx_sysinfo->td_conf;
> 	struct kvm_tdx_capabilities __user *user_caps;
> 	struct kvm_tdx_capabilities *caps = NULL;
>-	int i, ret = 0;
>+	int ret = 0;
> 
> 	/* flags is reserved for future use */
> 	if (cmd->flags)
>@@ -70,39 +70,19 @@ static int tdx_get_capabilities(struct kvm_tdx_cmd *cmd)
> 		goto out;
> 	}
> 
>-	*caps = (struct kvm_tdx_capabilities) {
>-		.attrs_fixed0 = td_conf->attributes_fixed0,
>-		.attrs_fixed1 = td_conf->attributes_fixed1,
>-		.xfam_fixed0 = td_conf->xfam_fixed0,
>-		.xfam_fixed1 = td_conf->xfam_fixed1,
>-		.supported_gpaw = TDX_CAP_GPAW_48 |
>-		((kvm_host.maxphyaddr >= 52 &&
>-		  cpu_has_vmx_ept_5levels()) ? TDX_CAP_GPAW_52 : 0),
>-		.nr_cpuid_configs = td_conf->num_cpuid_config,
>-		.padding = 0,
>-	};
>+	caps->supported_attrs = kvm_tdx_caps->supported_attrs;
>+	caps->supported_xfam = kvm_tdx_caps->supported_xfam;
>+	caps->nr_cpuid_configs = kvm_tdx_caps->num_cpuid_config;
> 
> 	if (copy_to_user(user_caps, caps, sizeof(*caps))) {
> 		ret = -EFAULT;
> 		goto out;
> 	}
> 
>-	for (i = 0; i < td_conf->num_cpuid_config; i++) {
>-		struct kvm_tdx_cpuid_config cpuid_config = {
>-			.leaf = (u32)td_conf->cpuid_config_leaves[i],
>-			.sub_leaf = td_conf->cpuid_config_leaves[i] >> 32,
>-			.eax = (u32)td_conf->cpuid_config_values[i].eax_ebx,
>-			.ebx = td_conf->cpuid_config_values[i].eax_ebx >> 32,
>-			.ecx = (u32)td_conf->cpuid_config_values[i].ecx_edx,
>-			.edx = td_conf->cpuid_config_values[i].ecx_edx >> 32,
>-		};
>-
>-		if (copy_to_user(&(user_caps->cpuid_configs[i]), &cpuid_config,
>-					sizeof(struct kvm_tdx_cpuid_config))) {
>-			ret = -EFAULT;
>-			break;
>-		}
>-	}
>+	if (copy_to_user(user_caps->cpuid_configs, &kvm_tdx_caps->cpuid_configs,
>+			 kvm_tdx_caps->num_cpuid_config *
>+			 sizeof(kvm_tdx_caps->cpuid_configs[0])))
>+		ret = -EFAULT;
> 
> out:
> 	/* kfree() accepts NULL. */
>-- 
>2.34.1
>
>