Re: [PATCH v2] x86/virt/tdx: Formalize SEAMCALL version encoding support

[Xu Yilun]

> The commit message covers the mechanics, but is silent on compatibility
> with older TDX modules. That context matters for the design choice, so
> regardless of where the implementation discussion lands, I think it
> needs to be spelled out here.
> 
> Who is responsible for picking a version the module supports?
> 
> TDH.VP.INIT can hardcode version 1 only because KVM already refuses to
> enable TDX on modules without TOPOLOGY_ENUM.
> 
> The TDH.SYS.UPDATE user from [1] is the opposite case: it has to pick
> version 0 or 1 at runtime depending on whether add-on features are
> configured, to keep working on modules that don't support them.
> 
> The second point is the actual argument for a version field in struct
> tdx_module_args rather than encoding the version in the leaf defines:
> the version is not always a compile-time property of the call site.

That's good point. I'll add the TDX module compatibility argument in
changelog:

  One concern is the compatibility with older TDX modules which don't
  recognize the new SEAMCALLs. The kernel should decide which SEAMCALL
  version to use at runtime, selecting the minimum version number for the
  required functionality. It can't overwrite the function number with a
  new value at compile time.

> 
> Without TDH.SYS.UPDATE context, the patch seems pointless.

mm.. It's true one target is to address the compatibility with old
module. But is the other concern valid?

  Another concern is the obscure usage of the 'fn' parameter for seamcall
  wrappers. An existing caller for TDH.VP.INIT packs the version into the
  'fn' to match the low-level TDX ABI RAX layout. The RAX layout
  interprets some bits differently, such as INTERRUPT_MODE, SEAMLDR flag,
  which are not a good fit for the function number definition.

Is this a blocker for you as an independent cleanup patch?

Thanks,
Yilun

> 
> > Link: https://lore.kernel.org/kvm/4f4b0f29-424b-45ed-8cfd-c77da2ea390f@xxxxxxxxx/ # [1]