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

[Dan Williams]

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.