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

[Huang, Kai]

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;