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: <ZiqGRErxDJ1FE8iA@google.com>
Date: Thu, 25 Apr 2024 09:35:16 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Kai Huang <kai.huang@...el.com>
Cc: Tina Zhang <tina.zhang@...el.com>, Hang Yuan <hang.yuan@...el.com>, 
	Bo2 Chen <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>, Erdem Aktas <erdemaktas@...gle.com>, 
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>, "pbonzini@...hat.com" <pbonzini@...hat.com>, 
	Isaku Yamahata <isaku.yamahata@...el.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 Tue, Apr 23, 2024, Kai Huang wrote:
> On Tue, 2024-04-23 at 08:15 -0700, Sean Christopherson wrote:
> > Presumably that approach relies on something blocking onlining CPUs when TDX is
> > active.  And if that's not the case, the proposed patches are buggy.
> 
> The current patch ([PATCH 023/130] KVM: TDX: Initialize the TDX module
> when loading the KVM intel kernel module) indeed is buggy, but I don't
> quite follow why we need to block onlining CPU  when TDX is active?

I was saying that based on my reading of the code, either (a) the code is buggy
or (b) something blocks onlining CPUs when TDX is active.  Sounds like the answer
is (a).

> There's no hard things that prevent us to do so.  KVM just need to do
> VMXON + tdx_cpu_enable() inside kvm_online_cpu().
> 
> > 
> > > Btw, the ideal (or probably the final) plan is to handle tdx_cpu_enable()
> > > in TDX's own CPU hotplug callback in the core-kernel and hide it from all
> > > other in-kernel TDX users.  
> > > 
> > > Specifically:
> > > 
> > > 1) that callback, e.g., tdx_online_cpu() will be placed _before_ any in-
> > > kernel TDX users like KVM's callback.
> > > 2) In tdx_online_cpu(), we do VMXON + tdx_cpu_enable() + VMXOFF, and
> > > return error in case of any error to prevent that cpu from going online.
> > > 
> > > That makes sure that, if TDX is supported by the platform, we basically
> > > guarantees all online CPUs are ready to issue SEAMCALL (of course, the in-
> > > kernel TDX user still needs to do VMXON for it, but that's TDX user's
> > > responsibility).
> > > 
> > > But that obviously needs to move VMXON to the core-kernel.
> > 
> > It doesn't strictly have to be core kernel per se, just in code that sits below
> > KVM, e.g. in a seperate module called VAC[*] ;-)
> > 
> > [*] https://lore.kernel.org/all/ZW6FRBnOwYV-UCkY@google.com
> 
> Could you elaborate why vac.ko is necessary?
> 
> Being a module natually we will need to handle module init and exit.  But
> TDX cannot be disabled and re-enabled after initialization, so in general
> the vac.ko doesn't quite fit for TDX.
> 
> And I am not sure what's the fundamental difference between managing TDX
> module in a module vs in the core-kernel from KVM's perspective.

VAC isn't strictly necessary.  What I was saying is that it's not strictly
necessary for the core kernel to handle VMXON either.  I.e. it could be done in
something like VAC, or it could be done in the core kernel.

The important thing is that they're handled by _one_ entity.  What we have today
is probably the worst setup; VMXON is handled by KVM, but TDX.SYS.LP.INIT is
handled by core kernel (sort of).