Re: [PATCH 11/25] KVM: TDX: Report kvm_tdx_caps in KVM_TDX_CAPABILITIES

Chao Gao
2024-08-13T03:35:19Z

On Mon, Aug 12, 2024 at 03:48:06PM -0700, Rick Edgecombe wrote:
>From: Xiaoyao Li <xiaoyao.li@intel.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@intel.com>
>Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.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
>
>