Received-SPF: pass (google.com: domain of linux-kernel+bounces-130813-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <484636a4-d48f-4411-8db4-f0295a1560d6@intel.com>
Date: Thu, 4 Apr 2024 14:24:56 +1300
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v19 038/130] KVM: TDX: create/destroy VM structure
To: Chao Gao <chao.gao@intel.com>
CC: "Yamahata, Isaku" <isaku.yamahata@intel.com>, "kvm@vger.kernel.org"
	<kvm@vger.kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, "isaku.yamahata@gmail.com"
	<isaku.yamahata@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, "Aktas,
 Erdem" <erdemaktas@google.com>, Sean Christopherson <seanjc@google.com>,
	"Sagi Shahar" <sagis@google.com>, "Chen, Bo2" <chen.bo@intel.com>, "Yuan,
 Hang" <hang.yuan@intel.com>, "Zhang, Tina" <tina.zhang@intel.com>, "Sean
 Christopherson" <sean.j.christopherson@intel.com>,
	"isaku.yamahata@linux.intel.com" <isaku.yamahata@linux.intel.com>
References: <cover.1708933498.git.isaku.yamahata@intel.com>
 <7a508f88e8c8b5199da85b7a9959882ddf390796.1708933498.git.isaku.yamahata@intel.com>
 <ZfpwIespKy8qxWWE@chao-email>
 <20240321141709.GK1994522@ls.amr.corp.intel.com>
 <58e0cf59-1397-44a3-a6a0-e26b2e51ba7b@intel.com>
 <Zg38dcrwMg1a7iJT@chao-email>
Content-Language: en-US
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <Zg38dcrwMg1a7iJT@chao-email>
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?NWZTeVAxS0Y1RC9hTVp6RHVZSzhlQ0I0TTg0V3U3clFTaWUyYTM3Q2VnVnFP?=
 =?utf-8?B?T0NOQzJtKzNhYURyS3lZRGppVVVOSklCL3YwUkVhTWVXRkZBb1VmVWd4TExW?=
 =?utf-8?B?dW5WVjgrRWhEbEhsRjFzSTR4Z1d6UjZlWGM4R1IzMTJTZXVZMDMrMzdpRUg4?=
 =?utf-8?B?Vm9GcVJGSW1kTWh5eDlheC9XM1l4TEdGTGJXR1YyTi9Bam9jcGNTczlEUUZ6?=
 =?utf-8?B?ZHF6RDk0cEhVUmtlSzhjUFB2ODMvRnBxcytaZTlDN0FqN2JUYmgzVDE4cktK?=
 =?utf-8?B?a3Z6UW5mL0I5NllJQ1U2TXRtbitzNmgyZUJnL1E1TVBUR2JsZ3BYcmtsaU54?=
 =?utf-8?B?SFJVYmExUEJ4QjNoNE44VzB5ZlFIaDZKaEZrYitVM0c3R2kvMWxBdDMveElP?=
 =?utf-8?B?TFhkdDZhZ1pDZ2FTQlhzVFJHSkFXU2lJck5kVFhBRytIWnRSMGJzTHdLd0pp?=
 =?utf-8?B?MThWbkV4ZURpam1zb2NsUkNLTVdGMUR6NlkyMWNYUmszaHdyaEJZeEg0WDkr?=
 =?utf-8?B?NmZtY1RmcnFjUEFUNUhOV2pNUzBmMVRlaVQvZTFrYUZrTHVuR3BjU2hEOERG?=
 =?utf-8?B?YWNncHRtaUJ4YjhkR3Z2TWl4UzNtSXFKYzRLcCtYNmNEZTcxSXF6bW5Wckpl?=
 =?utf-8?B?M1hOV2dHSlNvYzRRejlRMDlCSEpnMHd3UWV4MU80aFdxL20yemVna3ZXSnFK?=
 =?utf-8?B?cHZDY0xvRStEU0x5YUNXbWEyMTkyd1dnUGYzek1IR0kxYi90SWxwVDV2QW5x?=
 =?utf-8?B?NFpSS0hWZDVKZUhiVk5MbmIrSzlBeDgwbnpieEVKYlMyNmNxWXBtTnpzOXRz?=
 =?utf-8?B?T2RpYzd3dWZtYWZqSCt3VWFKVjlEK3ZQbVVRZjZ0OVgwcXFtSzRka0ZHQW40?=
 =?utf-8?B?aG5BNDUrSm5yR2lWVkpiZEM0V1E4MldScERCNlAvWkdFYVJiQ2hnSytOMU5n?=
 =?utf-8?B?MnVha1ZYMjhiOFJnUUIzRnFpVzEwZFBUQkliRnlGZXNNcENMTUN4SkRTNHJs?=
 =?utf-8?B?T2M4eVl6MFplUWNxSUFRaTBQdmdLQjcvcENGdmEzUmcrY2w5UDhkZjVyQVRO?=
 =?utf-8?B?YUIvWTI1eHcvcitOQithU21PbHlKbHR3V1RZWjIrby9qbkZhczBxMnlXQmNS?=
 =?utf-8?B?WDYrMnpFR1lQZ05qRkRlcGNvVDVrQ1hOciszeWJHMjZuUnZPTmQ4Z0VRTHFk?=
 =?utf-8?B?aFBXand6Y2xRZHZKL2NkMU5Jd0UwM2IrM0dWeERqWEcwOEhCZVJFd2JxNDFE?=
 =?utf-8?B?S25sZ2VnUjdCazRaL0NCakpHSjdPZFh3YWpEa1phWWtyY3Y4SEk2TU5IcllH?=
 =?utf-8?B?cWxyZVl2QXQ3c21yKzNpU1RwZ0xhQzNNbEIwQWJXcDlvdkZzclN2SjZDNFc5?=
 =?utf-8?B?U2lMSEMwdkV4bU1HM0pyNVBLdnFRTDhJMnhoVnZuTVJNQy9XVDRCdFk2Z3hs?=
 =?utf-8?B?UHNHMlh3MXBUbWtBWGFnUElFbUdjTjlmQ1RacU1YeHpBMk44UWQ3MklaNlFv?=
 =?utf-8?B?SjAyY1kwSUg3b0FvWE9nOWRDQVRTSndab2lXL2wyRExSbGtheE1HNjRjVlVl?=
 =?utf-8?B?VjAzUmNaaDd4UXA4d2NIRTZSUGxiLzU3ZXR0M0dUNCs5SUl2azFjQ3FNaHlM?=
 =?utf-8?B?MHlwKzBKN1JHNjZ5YVVCSENGNWs1UC9sd3RZVW1oclNhY3BRa3RRRkNHaUkw?=
 =?utf-8?B?dnpRbHBEUEtlM0U3ZXVBNWJTU2JqbkxzUGxYR2gwaERmMC91L01reGVKQ0JP?=
 =?utf-8?B?N2xuMzNuTUxiamx5Z1NGV0k3OXhFcElaTWpFYnFEUExNbXJpM0tQUFhnOFB6?=
 =?utf-8?B?NXpiY3NZOUVqVHFNdkp4aW1TZysvOUhRZ2FaNmtsN1VKMmpTL3o3KzNERk5K?=
 =?utf-8?B?OGhjR0NvcDZQZEJNNm1WWSt6UTFFSGtmbFMvaktuRnYwMzRiL1ByTXcvRFBy?=
 =?utf-8?B?Q1lBeENuZDU2QTlKd1cvUC92N0psTHoyNk0zRGovaGlqWUpCSEtONEhQbXNv?=
 =?utf-8?B?WlBzSEF4d1h4MVBtSFhXdmNCWE15UW1xa2gzTFIyOXp4Y1gvb1k0WlpQRjJm?=
 =?utf-8?B?Qm03UG9QTnRnZjdOK2ZES2dOc0F1YjdHZTZ5Ukk0NmRXQ0h5RU1yYXlBRmRh?=
 =?utf-8?Q?k2fhwck9ONLLqya7mZ302u1bQ?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 7993ad33-2b48-4851-f29f-08dc544610d9
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Apr 2024 01:25:08.8878
 (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: 9t9fIfPqwvND7uTHgOKAtlc5EVVTznEE8rKqDAAhiCm/SA+hoJKA5gTgTcqrexS3Nx9xVynZH+VsMxQzoue6SQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5230
X-OriginatorOrg: intel.com


On 4/04/2024 2:03 pm, Chao Gao wrote:
> On Thu, Apr 04, 2024 at 11:13:49AM +1300, Huang, Kai wrote:
>>
>>
>> On 22/03/2024 3:17 am, Yamahata, Isaku wrote:
>>>>> +
>>>>> +	for_each_online_cpu(i) {
>>>>> +		int pkg = topology_physical_package_id(i);
>>>>> +
>>>>> +		if (cpumask_test_and_set_cpu(pkg, packages))
>>>>> +			continue;
>>>>> +
>>>>> +		/*
>>>>> +		 * Program the memory controller in the package with an
>>>>> +		 * encryption key associated to a TDX private host key id
>>>>> +		 * assigned to this TDR.  Concurrent operations on same memory
>>>>> +		 * controller results in TDX_OPERAND_BUSY.  Avoid this race by
>>>>> +		 * mutex.
>>>>> +		 */
>>>>> +		mutex_lock(&tdx_mng_key_config_lock[pkg]);
>>>> the lock is superfluous to me. with cpu lock held, even if multiple CPUs try to
>>>> create TDs, the same set of CPUs (the first online CPU of each package) will be
>>>> selected to configure the key because of the cpumask_test_and_set_cpu() above.
>>>> it means, we never have two CPUs in the same socket trying to program the key,
>>>> i.e., no concurrent calls.
>>> Makes sense. Will drop the lock.
>>
>> Hmm.. Skipping in cpumask_test_and_set_cpu() would result in the second
>> TDH.MNG.KEY.CONFIG not being done for the second VM.  No?
> 
> No. Because @packages isn't shared between VMs.

I see.  Thanks.