linux-kernel - Re: [PATCH v19 036/130] KVM: TDX: x86: Add ioctl to get TDX systemwide parameters

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: <20240323002847.GB2357401@ls.amr.corp.intel.com>
Date: Fri, 22 Mar 2024 17:28:47 -0700
From: Isaku Yamahata <isaku.yamahata@...el.com>
To: "Huang, Kai" <kai.huang@...el.com>
Cc: "Yamahata, Isaku" <isaku.yamahata@...el.com>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"isaku.yamahata@...il.com" <isaku.yamahata@...il.com>,
	Paolo Bonzini <pbonzini@...hat.com>,
	"Aktas, Erdem" <erdemaktas@...gle.com>,
	Sean Christopherson <seanjc@...gle.com>,
	Sagi Shahar <sagis@...gle.com>, "Chen, Bo2" <chen.bo@...el.com>,
	"Yuan, Hang" <hang.yuan@...el.com>,
	"Zhang, Tina" <tina.zhang@...el.com>,
	Sean Christopherson <sean.j.christopherson@...el.com>,
	isaku.yamahata@...ux.intel.com
Subject: Re: [PATCH v19 036/130] KVM: TDX: x86: Add ioctl to get TDX
 systemwide parameters

On Fri, Mar 22, 2024 at 11:26:17AM +1300,
"Huang, Kai" <kai.huang@...el.com> wrote:

> > diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> > index 9ea46d143bef..e28189c81691 100644
> > --- a/arch/x86/include/uapi/asm/kvm.h
> > +++ b/arch/x86/include/uapi/asm/kvm.h
> > @@ -604,4 +604,21 @@ 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];
> > +
> > +	__u32 nr_cpuid_configs;
> > +	struct kvm_tdx_cpuid_config cpuid_configs[];
> > +};
> > +
> 
> I think you should use __DECLARE_FLEX_ARRAY().
> 
> It's already used in existing KVM UAPI header:
> 
> struct kvm_nested_state {
> 	...
>         union {
>                 __DECLARE_FLEX_ARRAY(struct kvm_vmx_nested_state_data,
> 					 vmx);
>                 __DECLARE_FLEX_ARRAY(struct kvm_svm_nested_state_data,
> 					 svm);
>         } data;
> }

Yes, will use it.


> > +	if (copy_to_user(user_caps->cpuid_configs, &tdx_info->cpuid_configs,
> > +			 tdx_info->num_cpuid_config *
> > +			 sizeof(tdx_info->cpuid_configs[0]))) {
> > +		ret = -EFAULT;
> > +	}
> 
> I think the '{ }' is needed here.

Unnecessary? Will remove braces.


> > +
> > +out:
> > +	/* kfree() accepts NULL. */
> > +	kfree(caps);
> > +	return ret;
> > +}
> > +
> >   int tdx_vm_ioctl(struct kvm *kvm, void __user *argp)
> >   {
> >   	struct kvm_tdx_cmd tdx_cmd;
> > @@ -68,6 +121,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/arch/x86/kvm/vmx/tdx.h b/arch/x86/kvm/vmx/tdx.h
> > index 473013265bd8..22c0b57f69ca 100644
> > --- a/arch/x86/kvm/vmx/tdx.h
> > +++ b/arch/x86/kvm/vmx/tdx.h
> > @@ -3,6 +3,9 @@
> >   #define __KVM_X86_TDX_H
> >   #ifdef CONFIG_INTEL_TDX_HOST
> > +
> > +#include "tdx_ops.h"
> > +
> 
> It appears "tdx_ops.h" is used for making SEAMCALLs.
> 
> I don't see this patch uses any SEAMCALL so I am wondering whether this
> chunk is needed here?

Will remove it to move it to an appropriate patch
-- 
Isaku Yamahata <isaku.yamahata@...el.com>