Received-SPF: pass (google.com: domain of linux-kernel+bounces-146051-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <34d64c12-9ed5-4c63-8465-29f7fdce20dc@intel.com>
Date: Tue, 16 Apr 2024 12:55:33 +1200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v19 027/130] KVM: TDX: Define TDX architectural
 definitions
To: Isaku Yamahata <isaku.yamahata@linux.intel.com>, Yan Zhao
	<yan.y.zhao@intel.com>
CC: <isaku.yamahata@intel.com>, <kvm@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <isaku.yamahata@gmail.com>, Paolo Bonzini
	<pbonzini@redhat.com>, <erdemaktas@google.com>, Sean Christopherson
	<seanjc@google.com>, Sagi Shahar <sagis@google.com>, <chen.bo@intel.com>,
	<hang.yuan@intel.com>, <tina.zhang@intel.com>, Sean Christopherson
	<sean.j.christopherson@intel.com>, Xiaoyao Li <xiaoyao.li@intel.com>
References: <cover.1708933498.git.isaku.yamahata@intel.com>
 <522cbfe6e5a351f88480790fe3c3be36c82ca4b1.1708933498.git.isaku.yamahata@intel.com>
 <ZeGC64sAzg4EN3G5@yzhao56-desk.sh.intel.com>
 <20240305082138.GD10568@ls.amr.corp.intel.com>
Content-Language: en-US
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <20240305082138.GD10568@ls.amr.corp.intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?QUtJcFNET2FnWkVBUllyYkpBYnhMUGtBQWZ2c0lvazVXdHZpVVpMRXZRMFM0?=
 =?utf-8?B?NktDZUVMM0RtaVlSOTgrOE9CZGtSSkNXcitLVVFwUHhvSHVyQmltR00vNUFH?=
 =?utf-8?B?elRING1UY0NhenBWQ1ZvSlpKMkNxWjhiRTFRbTNEemc1N3IzeTFZSkM2Vjlh?=
 =?utf-8?B?Ync0NFRaNFZRZjVNYWF4WjQxZEYwQ0dKcGhkMnJLOGRDOUowSnZNMnZvOFBO?=
 =?utf-8?B?cXpnRXQybk5CMm1xcDAxQUdDZUdlWUJkYzhLdlZob0hITlo1ZUVMaWtMOEo4?=
 =?utf-8?B?b3lWYnFsaGJFQXlYWTBJREZlTUREbWpuY2IwT1AwczdLL1A0RCtFT080TE9I?=
 =?utf-8?B?SGh4UUJWSmNJL2dSemwrU2E3R1hqV1lQTERlbzM3NlREQ3Z6VExzamFScGpn?=
 =?utf-8?B?eWJJTkNqV05yNm11VDR3ZFhURURNN2s3VnhoY2svUUtBS0ZRdmt4VUNLUjdx?=
 =?utf-8?B?Mno1cDBMcUFuai8rSVVNSFRocHpld01uTFV1N2JkN3oySGNxcVQwSXk5RHFm?=
 =?utf-8?B?RW1Ybm5ncml0eFpNM3JVNmY3Ym9HUWVyVGVpRnEwTkJiZFl5M0hweTFuaVFw?=
 =?utf-8?B?RktneU5pcFF5cUVWa0ZsY0hVSXQ2WXV2eVg0aU9wS3ZUNmNySUJtMjkyeE1T?=
 =?utf-8?B?UVZDMmNwaEFtR2tqVUtEUkRRVmxFZVdUOUhWekZpVEhGVUlONmRlTlVCT25Z?=
 =?utf-8?B?Y2FBS29jUlBoVHRYNEhhYWlBY1c4V21WWlFWZWZoSnM1LzZSSlVhUGFJNkcz?=
 =?utf-8?B?SjlJY0lIcXpXb2lIV1hjS05qdlZtOVd6Vmx5d1JsUW5yMEJtd25pQ2xaYmVh?=
 =?utf-8?B?ejJvMGY4aUpNV0taUHVyRVZvY1Vrc2NVc1hVV2swRER6eUVhMlRnaWNaVGFO?=
 =?utf-8?B?YllncitNendHcjFiak5uRXphYzBnRmxSNU9HbEVVMjlXNldvd3R3MDIyaG9W?=
 =?utf-8?B?WnJqeFFlVUpOdXN2T3NrRTBLU3JJZStXTVRpWkhtbDFrZmhWaThsc3dsWUQ1?=
 =?utf-8?B?MHhMV29VZ2s0eGtwV1hrejNjMjVYemgraExHL1B6VFZYQis2aWlVT0VqSXhO?=
 =?utf-8?B?WWxyeWlvQmtGcmp0RmZuaG9LMFl4ekdRZy81dEVVRUd5YWxNSHRuZVNBNDhl?=
 =?utf-8?B?dHVXeVFpMmxucEtRQ3EwWi9TY2VzVTFzc1VMRHFkMFRzOTIvWmVHM2FWVVQ1?=
 =?utf-8?B?b21odUhJYmZrWi9tU2FlTGwxajV1bzlZM3VlcjNFaEhCTWtLQXJMdE1Va0tm?=
 =?utf-8?B?MjM4WDZHdnR1OHF1S0NWTkNvOHJwdFZXRWhtMVFPMDRXQmpSYzdsbXd6aFUr?=
 =?utf-8?B?amgvR0FuUGY0dmZtSWswRHM3SUg1VkNFL0d1RXNlY3JtVThIVXNIREd5SEYz?=
 =?utf-8?B?WmliOGRhcjh5bTFCVFN6UUZCRFhCMFZGeUxsc0xtb3V5bmhtK0l6Z3ozbkpF?=
 =?utf-8?B?NWFhMWs3YUpOL2M2M1VibkM3bnZ1MVpXMGxmSnNpcVZJc0xScGQ2N0ZNMktY?=
 =?utf-8?B?dDhsWS9yVXJ3ZERqSmY1ayttTnNsbnZUWW5pUnQ3dmhhUVdtL0xLQ0FpK0Nv?=
 =?utf-8?B?K25RUUQwUE9TSGk5Y0ZrTkt2VkM0aEJ0VzVvdmhOZVZHcXJlMnM4V3owdUM5?=
 =?utf-8?B?S3Z1UGVXckxwOFVhN0EvNG84K3Njelk2ek5TWTZlekFmZTlYTjQrSEF0UjB3?=
 =?utf-8?B?K1BFSXlKeW9jUWx2QVpLWE1GRWdaVGR1RHBiaUVSbWpwZGFERmoweE1LaC9J?=
 =?utf-8?B?bmpkd2RpYkdEZnhoOEExQTYxcEVHZVNCTWRKSTY3RndYRVNObjkzZjJHN3BW?=
 =?utf-8?B?SHRJQ3luTHBjSzZFcXYzNzRrQ2Y3dURVM1FRKzlmL0c5V0VsdVVNTXRTaXVD?=
 =?utf-8?B?M011dDJKTzJMMk53L1lsajVOWGZmdndjcW15VWh4aDUxdkVFTUo2SWRJbmxJ?=
 =?utf-8?B?eVV3RVQ1NThFbXY2cmZoZTVNL1pDODZFRzloMWVzVGtXNHpXaHBSOXg1RlN1?=
 =?utf-8?B?Rk41ZEVIT0hIYjBBZWorQmdsSHREYjRtN01XSVlTZ3gyS3d0dlNRdWR6Y2Y2?=
 =?utf-8?B?RjFRdER5Z3Fia1BaQjFVMklob0VsZTkydkJXaDBzWFZXNVROc2RYMlpEQVUv?=
 =?utf-8?Q?zgWsrJUykaHVUofI2ZYCIa7BS?=
X-MS-Exchange-CrossTenant-Network-Message-Id: fab8cc67-7c29-4f74-4412-08dc5daff1bf
X-MS-Exchange-CrossTenant-AuthSource: PH7PR11MB5983.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Apr 2024 00:55:43.7771
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 9gbOo1x2BX7HfaxXyrHJeRYNEzTBYAuc008cX8enrt5LBQTfuzOJqFBWLgkOakGFllUSTnUfw9VsOESZdHFDIQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB5801
X-OriginatorOrg: intel.com


On 5/03/2024 9:21 pm, Isaku Yamahata wrote:
> On Fri, Mar 01, 2024 at 03:25:31PM +0800,
> Yan Zhao <yan.y.zhao@intel.com> wrote:
> 
>>> + * TD_PARAMS is provided as an input to TDH_MNG_INIT, the size of which is 1024B.
>>> + */
>>> +#define TDX_MAX_VCPUS	(~(u16)0)
>> This value will be treated as -1 in tdx_vm_init(),
>> 	"kvm->max_vcpus = min(kvm->max_vcpus, TDX_MAX_VCPUS);"
>>
>> This will lead to kvm->max_vcpus being -1 by default.
>> Is this by design or just an error?
>> If it's by design, why not set kvm->max_vcpus = -1 in tdx_vm_init() directly.
>> If an unexpected error, may below is better?
>>
>> #define TDX_MAX_VCPUS   (int)((u16)(~0UL))
>> or
>> #define TDX_MAX_VCPUS 65536
> 
> You're right. I'll use ((int)U16_MAX).
> As TDX 1.5 introduced metadata MAX_VCPUS_PER_TD, I'll update to get the value
> and trim it further. Something following.
> 

[...]

>   
> +	u16 max_vcpus_per_td;
> +

[...]

> -	kvm->max_vcpus = min(kvm->max_vcpus, TDX_MAX_VCPUS);
> +	kvm->max_vcpus = min3(kvm->max_vcpus, tdx_info->max_vcpus_per_td,
> +			     TDX_MAX_VCPUS);
>   

[...]

> -#define TDX_MAX_VCPUS	(~(u16)0)
> +#define TDX_MAX_VCPUS	((int)U16_MAX)

Why do you even need TDX_MAX_VCPUS, given it cannot exceed U16_MAX and 
you will have the 'u16 max_vcpus_per_td' anyway?

IIUC, in KVM_ENABLE_CAP(KVM_CAP_MAX_VCPUS), we can overwrite the 
kvm->max_vcpus to the 'max_vcpus' provided by the userspace, and make 
sure it doesn't exceed tdx_info->max_vcpus_per_td.

Anything I am missing?