Re: [PATCH v5 07/22] x86/virt/tdx: Implement SEAMCALL function

[Kai Huang <kai.huang@intel.com>]

On Tue, 2022-07-19 at 12:39 -0700, Dan Williams wrote:
> Kai Huang wrote:
> > On Mon, 2022-06-27 at 13:58 -0700, Dave Hansen wrote:
> > > On 6/26/22 22:23, Kai Huang wrote:
> > > > On Fri, 2022-06-24 at 11:38 -0700, Dave Hansen wrote:
> > > > > On 6/22/22 04:16, Kai Huang wrote:
> > > > > > SEAMCALL instruction causes #GP when SEAMRR isn't enabled, and #UD when
> > > > > > CPU is not in VMX operation.  The TDX_MODULE_CALL macro doesn't handle
> > > > > > SEAMCALL exceptions.  Leave to the caller to guarantee those conditions
> > > > > > before calling __seamcall().
> > > > > 
> > > > > I was trying to make the argument earlier that you don't need *ANY*
> > > > > detection for TDX, other than the ability to make a SEAMCALL.
> > > > > Basically, patch 01/22 could go away.
> > > ...
> > > > > So what does patch 01/22 buy us?  One EXTABLE entry?
> > > > 
> > > > There are below pros if we can detect whether TDX is enabled by BIOS during boot
> > > > before initializing the TDX Module:
> > > > 
> > > > 1) There are requirements from customers to report whether platform supports TDX
> > > > and the TDX keyID numbers before initializing the TDX module so the userspace
> > > > cloud software can use this information to do something.  Sorry I cannot find
> > > > the lore link now.
> > > 
> > > <sigh>
> > > 
> > > Never listen to customers literally.  It'll just lead you down the wrong
> > > path.  They told you, "we need $FOO in dmesg" and you ran with it
> > > without understanding why.  The fact that you even *need* to find the
> > > lore link is because you didn't bother to realize what they really needed.
> > > 
> > > dmesg is not ABI.  It's for humans.  If you need data out of the kernel,
> > > do it with a *REAL* ABI.  Not dmesg.
> > 
> > Showing in the dmesg is the first step, but later we have plan to expose keyID
> > info via /sysfs.  Of course, it's always arguable customer's such requirement is
> > absolutely needed, but to me it's still a good thing to have code to detect TDX
> > during boot.  The code isn't complicated as you can see.
> > 
> > > 
> > > > 2) As you can see, it can be used to handle ACPI CPU/memory hotplug and driver
> > > > managed memory hotplug.  Kexec() support patch also can use it.
> > > > 
> > > > Particularly, in concept, ACPI CPU/memory hotplug is only related to whether TDX
> > > > is enabled by BIOS, but not whether TDX module is loaded, or the result of
> > > > initializing the TDX module.  So I think we should have some code to detect TDX
> > > > during boot.
> > > 
> > > This is *EXACTLY* why our colleagues at Intel needs to tell us about
> > > what the OS and firmware should do when TDX is in varying states of decay.
> > 
> > Yes I am working on it to make it public.
> > 
> > > 
> > > Does the mere presence of the TDX module prevent hotplug?  
> > > 
> > 
> > For ACPI CPU hotplug, yes.  The TDX module even doesn't need to be loaded. 
> > Whether SEAMRR is enabled determines.
> > 
> > For ACPI memory hotplug, in practice yes.  For architectural behaviour, I'll
> > work with others internally to get some public statement.
> > 
> > > Or, if a
> > > system has the TDX module loaded but no intent to ever use TDX, why
> > > can't it just use hotplug like a normal system which is not addled with
> > > the TDX albatross around its neck?
> > 
> > I think if a machine has enabled TDX in the BIOS, the user of the machine very
> > likely has intention to actually use TDX.
> > 
> > Yes for driver-managed memory hotplug, it makes sense if user doesn't want to
> > use TDX, it's better to not disable it.  But to me it's also not a disaster if
> > we just disable driver-managed memory hotplug if TDX is enabled by BIOS.
> 
> No, driver-managed memory hotplug is how Linux handles "dedicated
> memory" management. The architecture needs to comprehend that end users
> may want to move address ranges into and out of Linux core-mm management
> independently of whether those address ranges are also covered by a SEAM
> range.

But to avoid GFP_TDX (and ZONE_TDX) staff, we need to guarantee all memory pages
in page allocator are TDX pages.  To me it's at least quite fair that user needs
to *choose* to use driver-managed memory hotplug or TDX.

If automatically disable driver-managed memory hotplug on a TDX BIOS enabled
platform isn't desired, how about we introduce a kernel command line (i.e.
use_tdx={on|off}) to let user to choose?

If user specifies use_tdx=on, then user cannot use driver-managed memory
hotplug.  if use_tdx=off, then user cannot use TDX even it is enabled by BIOS.