Re: [PATCH] x86/kvm/tdx: Save %rbp in TDX_MODULE_CALL

From: Huang, Kai
Date: Thu May 23 2024 - 19:29:17 EST



On 24/05/2024 10:34 am, Huang, Kai wrote:


On 24/05/2024 12:43 am, Jürgen Groß wrote:
On 23.05.24 14:26, Huang, Kai wrote:
On Thu, 2024-05-23 at 10:30 +0000, Huang, Kai wrote:
On Thu, 2024-05-23 at 07:56 +0200, Jürgen Groß wrote:
On 20.05.24 13:54, Huang, Kai wrote:
On Fri, 2024-05-17 at 09:48 -0700, Dave Hansen wrote:
On 5/17/24 08:58, Juergen Gross wrote:
On 17.05.24 17:52, Dave Hansen wrote:
..
Once we have the specific TDX module version, we can go ask the folks
who write it if there were any RBP clobbering bugs.

Okay, how to get the TDX module version?

You need something like this:

https://lore.kernel.org/all/20231012134136.1310650-1-yi.sun@xxxxxxxxx/

This one prints TDX version info in the TDX guest, but not host.

The attached diff prints the TDX version (something like below) during
module initialization, and should meet Juergen's needs for temporary use:

[  113.543538] virt/tdx: module verson: major 1, minor 5, internal 0


.. and yeah, this needs to be upstream.


  From this thread I think it makes sense to add code to the TDX host code
to print the TDX version during module initialization.  I'll start to work
on this.

One thing is from the spec TDX has "4 versions": major, minor, update,
internal.  They are all 16-bit, and the overall version can be written in:

    <Major>.<Minor>.<Update>.<Internal>, e.g., 1.5.05.01

(see TDX module 1.5 API spec, section 3.3.2 "TDX Module Version".)

The attached diff only prints major, minor and internal, but leaves the
update out because I believe it is for module runtime update (yet to
confirm).

Given there are 4 versions, I think it makes sense to implement reading
them based on this patchset ...

https://lore.kernel.org/kvm/6940c326-bfca-4c67-badf-ab5c086bf492@xxxxxxxxx/T/

... which extends the global metadata reading code to support any
arbitrary struct and all element sizes (although all 4 versions are 16-
bit)?

With that I got:

[   29.328484] virt/tdx: module verson: major 1, minor 5, internal 0



Let me check TDX module guys on this and get back to you.

Hi Jurgen,

I was told the module starting with "1.5.06." has NO_RBP_MOD support.

And I think I was wrong about the <update> part of the version, and we
need that to determine the third part of the module version.

I was also told the 1.5.06 module hasn't been released to public yet, so I
guess your module doesn't support it.

I did another patch (attached) to check NO_RBP_MOD and reject module
initialization if it is not supported, and print out module version:

[  146.566641] virt/tdx: NO_RBP_MOD feature is not supported
[  146.572797] virt/tdx: module verson: 1.5.0.0
[  146.577731] virt/tdx: module initialization failed (-22)

You can have another try to verify at your side, if that helps.


[   29.362806] virt/tdx: 4071192 KB allocated for PAMT
[   29.362828] virt/tdx: module verson: 1.5.1.0
[   29.362830] virt/tdx: module initialized

Seems your module supports NO_RBP_MOD.

This feature is per-VM and also requires to be explicitly opt-in when creating the guest.  Could you check in your code whether the setup_tdparams() function has below code?

    td_params->exec_controls = TDX_CONTROL_FLAG_NO_RBP_MOD;


Oh from another thread I saw you mentioned you have the above code enabled. So from host's perspective the TD should have enabled this feature.

It's possible it is a TDX module bug if you are not able to see this flag in the guest using the way Kirill replied.