linux-kernel - Re: [PATCH v19 023/130] KVM: TDX: Initialize the TDX module when loading the KVM intel kernel module

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: <8afbb648-b105-4e04-bf90-0572f589f58c@intel.com>
Date: Thu, 11 Apr 2024 11:15:43 +1200
From: "Huang, Kai" <kai.huang@...el.com>
To: Sean Christopherson <seanjc@...gle.com>
CC: "Yamahata, Isaku" <isaku.yamahata@...el.com>, "Zhang, Tina"
	<tina.zhang@...el.com>, "Yuan, Hang" <hang.yuan@...el.com>, "Chen, Bo2"
	<chen.bo@...el.com>, "sagis@...gle.com" <sagis@...gle.com>,
	"isaku.yamahata@...il.com" <isaku.yamahata@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Aktas, Erdem"
	<erdemaktas@...gle.com>, "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"pbonzini@...hat.com" <pbonzini@...hat.com>, "isaku.yamahata@...ux.intel.com"
	<isaku.yamahata@...ux.intel.com>
Subject: Re: [PATCH v19 023/130] KVM: TDX: Initialize the TDX module when
 loading the KVM intel kernel module



On 11/04/2024 3:29 am, Sean Christopherson wrote:
> On Wed, Apr 10, 2024, Kai Huang wrote:
>> On Fri, 2024-03-22 at 14:23 -0700, Isaku Yamahata wrote:
>>>>> +	r = atomic_read(&enable.err);
>>>>> +	if (!r)
>>>>> +		r = tdx_module_setup();
>>>>> +	else
>>>>> +		r = -EIO;
>>>>> +	on_each_cpu(vmx_off, &enable.enabled, true);
>>>>> +	cpus_read_unlock();
>>>>> +	free_cpumask_var(enable.enabled);
>>>>> +
>>>>> +out:
>>>>> +	return r;
>>>>> +}
>>>>
>>>> At last, I think there's one problem here:
>>>>
>>>> KVM actually only registers CPU hotplug callback in kvm_init(), which happens
>>>> way after tdx_hardware_setup().
>>>>
>>>> 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. :-)

> 
> 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?