Received-SPF: pass (google.com: domain of linux-kernel+bounces-141690-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <6cd2a9ce-f46a-44d0-9f76-8e493b940dc4@intel.com>
Date: Fri, 12 Apr 2024 10:58:58 +1200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v19 023/130] KVM: TDX: Initialize the TDX module when
 loading the KVM intel kernel module
To: Sean Christopherson <seanjc@google.com>
CC: "Yamahata, Isaku" <isaku.yamahata@intel.com>, "Zhang, Tina"
	<tina.zhang@intel.com>, "Yuan, Hang" <hang.yuan@intel.com>, "Chen, Bo2"
	<chen.bo@intel.com>, "sagis@google.com" <sagis@google.com>,
	"isaku.yamahata@gmail.com" <isaku.yamahata@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "Aktas, Erdem"
	<erdemaktas@google.com>, "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>, "isaku.yamahata@linux.intel.com"
	<isaku.yamahata@linux.intel.com>
References: <cover.1708933498.git.isaku.yamahata@intel.com>
 <f028d43abeadaa3134297d28fb99f283445c0333.1708933498.git.isaku.yamahata@intel.com>
 <d45bb93fb5fc18e7cda97d587dad4a1c987496a1.camel@intel.com>
 <20240322212321.GA1994522@ls.amr.corp.intel.com>
 <461b78c38ffb3e59229caa806b6ed22e2c847b77.camel@intel.com>
 <ZhawUG0BduPVvVhN@google.com>
 <8afbb648-b105-4e04-bf90-0572f589f58c@intel.com>
 <Zhftbqo-0lW-uGGg@google.com>
Content-Language: en-US
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <Zhftbqo-0lW-uGGg@google.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?SWlFUVI4WmRUYWIzY0R1T0kzOVJzcElJQUVNTjgxRCtwSytKTDBqQlpwd05m?=
 =?utf-8?B?RUhUekljY2ZubTJiMGdzMWpDaGNJUC9paEhVZmZRQnp0Q0pZRDNOMG5PTWw1?=
 =?utf-8?B?V1ZUdWo0L1V3Qzh0UDBFOXN4QXpRQ1JTTi84M1NyRklUUEFOQTRkR1ZOOTYr?=
 =?utf-8?B?d2xUU3VmUURtSUtkZHhkSkptZzBXbGlQcG1SYU1qK3VMSWsrd2VoSUs2YzVm?=
 =?utf-8?B?Um1yL01ZTWQxQlNFdnU5S0hvUUFad1BHdlVocERFZCtyRWVhaG9vTnRzdnZS?=
 =?utf-8?B?ODJoUEU4Sjd2UGNnaXczZjdKbmRReVJveWx5U3Q4UGg0Qm8rZ2VPS2x5MTNh?=
 =?utf-8?B?ZGcxOVBHSTl2cDhodWlaTndzZHk3bGEvQjJHNnJ6K3pucWRkU0hYQm96WjNa?=
 =?utf-8?B?RUVzWjFzMTZ0ckJDTkp2Kyt5NUdWalVNM0E5QllCZUQ4c1F0aE03UWYwV012?=
 =?utf-8?B?Q0lialJPd3cwUmV4MkFha3RkR3dDOFFnVFpkKzArU3ZjNkdzbWNlSUxRQ3Jw?=
 =?utf-8?B?RFFtbnpZRmtSNmJFWEc4YnZRVlVSR3ZnVWFkZGxTejJHM0xsNHlURTNVNXND?=
 =?utf-8?B?NHk2VlpSQjVhYmJERlRiWndKZ1NSekM5WmNzc3ZRSlpjWDRaWFV1NnQwYXZE?=
 =?utf-8?B?cDIrTjVDb05zNVh4OXEwRTJ6YjFEMERsc0pXeVJGREhwYTNGVVAvaU9HWkxC?=
 =?utf-8?B?WVBhMkQxRnV6ZHg0LzFhTFhzSjdXVXVZK0dGTngrRFNtMkw0aVUzejVhSmV5?=
 =?utf-8?B?NyttQ2xueDBNOHRJK1ZYZWlVWUV4dXVZSXZQWXF5OHczczVSTzl3NU9hUytI?=
 =?utf-8?B?U3I2U0t2NlFsTDBvajZ0ZkpkbUxQMXQrdnJsTnpKNzMzYy9GZlF6SDNjOHhS?=
 =?utf-8?B?RERGb2NnWEFpRHgzTmxnaGVudTBUN1Y5Z3dhb2JhQ0tUNnB6UXBIQm5lUms1?=
 =?utf-8?B?dXVwVE5NdU15VHRxM3dRTG01NlBKL09QbWpnQ2hUaUlHbzNtUFFqUGdKbjdm?=
 =?utf-8?B?V0pjLzV5a21GMFdWYTQyZGxSeEUxclJhRkcrTklXbkJhTzF6N1dheEt4SVA1?=
 =?utf-8?B?aFdtOWtTYnNQeHhaOHJ5bDZjWEZubHozWTNjcjhLa0greE9VMDFybW5OUE9n?=
 =?utf-8?B?WEJDVFVlSkZUenFFQVk0TERtMGdvbkpzRU1VcVNrVUorejRYTDNPcnZOd1JG?=
 =?utf-8?B?NlltRThYeFZqUTJFN01aQUpveDF1S3VJMStHblltQnZTNDNNSlNpaUlwZlU3?=
 =?utf-8?B?c1RGYUtJYTYyMzVxR3BZMk9aUEVhanRoUUp6ejJEM29QMFpiamFwSUgwUmxx?=
 =?utf-8?B?cnAyUkNuOVVBcE5qajlsRUNrVCs2Z2VlMEQ3ZjRCV1pndC83cCtXdEZ2QUVT?=
 =?utf-8?B?b0ZlUnZkNE9xWS9HZVljcWc3ZXBTYTNlTUlCWndRNDBZci8vM2FxSWoyakFR?=
 =?utf-8?B?Y2FJU2thaUd6ZUxoSEp2Q2F6N0pSdzdpUVpUR2ZETFUra2c1VlZKRVlNK29l?=
 =?utf-8?B?TmlUQTAxM1pSOVdBT0VyUUR5bjlzYVVQUStWOFJOd0J5L3VobDN5SHlLSW5H?=
 =?utf-8?B?MUg0VDl5d1o1cE9Tc2pndzB1Z0VnYUFkRi9BMGpuNGtSZ2ZySEo4WjFTdE9L?=
 =?utf-8?B?TEpYbmxrT3ZTU0NpVm40SEZJRGR2M2JmVmNRYzdLZmZLK0ZWeFZnMXBpZmlh?=
 =?utf-8?B?SW5TZjdCdDRWY1lkckx2UjAyVFU1ZFNhRUxJMUVoMlVGVG1tK0xLU1QrNndy?=
 =?utf-8?B?OGxIZlNaL1VubjFTT0tqQldSSnNPVzRraFZDYUdMOFB1Wi8wVTZjdGNkNWty?=
 =?utf-8?B?dVNqZW9lMTlJdVFhaGs2WjlFMU1uN0tDbXptMFpjV2FkckFybUhEbGV3bFpS?=
 =?utf-8?B?QUFJWVc3SndCUHp1NGdrN0hreVNCb0o3akhXZDQ2VkRWdnpZSU9iODEyYk5m?=
 =?utf-8?B?cXRaWExEWDJ0dGVUZTZGNnRXb3VPSHYvUDJuZ1h6elg3SkZOcXk3ejdYd0ph?=
 =?utf-8?B?dEhMT0JpVHM1cG5jL2g5T1JzVjVOeUp5QStvNHc2VlVUNzlrams2bXR0RnJO?=
 =?utf-8?B?c1hwYzB6V2NUbE9DajUzRWtObkJzcXBxTTcxckYyVDVYMnc3cFRTZVI2OW5W?=
 =?utf-8?Q?cR0pQlwA0SEHOn+/ovdmknJoo?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 9dc1051b-35a9-4a59-71e0-08dc5a7aff34
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Apr 2024 22:59:09.4747
 (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: 8fpS71L3eaCqoNGDl5q3t86+dVFXZbA3Mf5VP1dlNCvf4TP3CuwcL+O3PWLHsF7d1iCAuuoYTM8LPdpRm2U3tQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR11MB5268
X-OriginatorOrg: intel.com


On 12/04/2024 2:03 am, Sean Christopherson wrote:
> On Thu, Apr 11, 2024, Kai Huang wrote:
>> On 11/04/2024 3:29 am, Sean Christopherson wrote:
>>> On Wed, Apr 10, 2024, Kai Huang wrote:
>>>>>> What happens if any CPU goes online *BETWEEN* tdx_hardware_setup() and
>>>>>> kvm_init()?
>>>>>>
>>>>>> Looks we have two options:
>>>>>>
>>>>>> 1) move registering CPU hotplug callback before tdx_hardware_setup(), or
>>>>>> 2) we need to disable CPU hotplug until callbacks have been registered.
>>>
>>> This is all so dumb (not TDX, the current state of KVM).  All of the hardware
>>> enabling crud is pointless complex inherited from misguided, decade old paranoia
>>> that led to the decision to enable VMX if and only if VMs are running.  Enabling
>>> VMX doesn't make the system less secure, and the insane dances we are doing to
>>> do VMXON on-demand makes everything *more* fragile.
>>>
>>> And all of this complexity really was driven by VMX, enabling virtualization for
>>> every other vendor, including AMD/SVM, is completely uninteresting.  Forcing other
>>> architectures/vendors to take on yet more complexity doesn't make any sense.
>>
>> Ah, I actually preferred this solution, but I was trying to follow your
>> suggestion here:
>>
>> https://lore.kernel.org/lkml/ZW6FRBnOwYV-UCkY@google.com/
>>
>> form which I interpreted you didn't like always having VMX enabled when KVM
>> is present. :-)
> 
> I had a feeling I said something along those lines in the past.
> 
>>> Barely tested, and other architectures would need to be converted, but I don't
>>> see any obvious reasons why we can't simply enable virtualization when the module
>>> is loaded.
>>>
>>> The diffstat pretty much says it all.
>>
>> Thanks a lot for the code!
>>
>> I can certainly follow up with this and generate a reviewable patchset if I
>> can confirm with you that this is what you want?
> 
> Yes, I think it's the right direction.  I still have minor concerns about VMX
> being enabled while kvm.ko is loaded, which means that VMXON will _always_ be
> enabled if KVM is built-in.  But after seeing the complexity that is needed to
> safely initialize TDX, and after seeing just how much complexity KVM already
> has because it enables VMX on-demand (I hadn't actually tried removing that code
> before), I think the cost of that complexity far outweighs the risk of "always"
> being post-VMXON.

Does always leaving VMXON have any actual damage, given we have 
emergency virtualization shutdown?

> 
> Within reason, I recommend getting feedback from others before you spend _too_
> much time on this.  It's entirely possible I'm missing/forgetting some other angle.

Sure.  Could you suggest who should we try to get feedback from?

Perhaps you can just help to Cc them?

Thanks for your time.