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

["Huang, Kai" <kai.huang@intel.com>]

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@intel.com/
>>>>>
>>>>> 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@intel.com/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;