Re: [PATCH v7 17/20] x86/virt/tdx: Configure global KeyID on all packages

[Dave Hansen]

On 11/20/22 16:26, Kai Huang wrote:
> After the array of TDMRs and the global KeyID are configured to the TDX
> module, use TDH.SYS.KEY.CONFIG to configure the key of the global KeyID
> on all packages.

I want to circle back to this because it potentially has the same class
of issue that TDH.SYS.LP.INIT had.  So, here's some more background
followed by the key question: is TDH.SYS.KEY.CONFIG too strict?  Should
we explore relaxing it?

Here's the very long-winded way of asking the same thing:

This key is used to protect TDX module memory which is too large to fit
into the limited range-register-protected (SMRR) areas that most of the
module uses.  Right now, that metadata includes the Physical Address
Metadata Tables (PAMT) and "TD Root" (TDR) pages.  Using this "global
KeyID" provides stronger isolation and integrity protection for these
structures than is provided by KeyID-0.

The "global KeyID" only strictly needs to be programmed into a memory
controllers if a PAMT or TDR page is allocated in memory attached to
that controller.  However, the TDX module currently requires that
TDH.SYS.KEY.CONFIG be executed on one processor in each package.  This
is true even if there is no TDX Memory Region (TDMR) attached to that
package.

This was likely done for simplicity in the TDX module.  It currently has
no NUMA awareness (or even trusted NUMA metadata) and no ability to
correlate processor packages with the memory attached to their memory
controllers.

The TDH.SYS.KEY.CONFIG design is actually pretty similar to Kirill's
MKTME implementation[1].  Basically blast the KeyID configuration out to
one processor in each package, regardless of whether the KeyID will ever
get used on that package.

While this requirement from the TDX module is _slightly_ too strict, I'm
not quite as worried about it as I was about the *super* strict
TDH.SYS.LP.INIT requirements.  It's a lot harder and more rare to have
an entire package of CPUs unavailable versus a single logical CPU.
There is, for instance, no side-channel mitigation that disables an
entire package worth of CPUs.  I'm not even sure if we allow an entire
package worth of NOHZ_FULL-indisposed processors.

I'm happy to go run the same drill for TDH.SYS.KEY.CONFIG that we did
for TDH.SYS.LP.INIT.  Basically, can we relax the too-strict
restrictions?  But, I'm not sure anyone will ever reap a practical
benefit from it.  I'm tempted to just leave it as-is.

Does anyone feel differently?

1.
https://lore.kernel.org/lkml/20190508144422.13171-1-kirill.shutemov@xxxxxxxxxxxxxxx/T/#m936f260a345284687f8e929675f68f3d514725f5