Received-SPF: pass (google.com: domain of linux-kernel+bounces-188120-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <62231ebd-9ed9-464c-a4db-f8c79fd3ce5e@intel.com>
Date: Fri, 24 May 2024 11:28:46 +1200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] x86/kvm/tdx: Save %rbp in TDX_MODULE_CALL
From: "Huang, Kai" <kai.huang@intel.com>
To: =?UTF-8?B?SsO8cmdlbiBHcm/Dnw==?= <jgross@suse.com>,
	"kirill.shutemov@linux.intel.com" <kirill.shutemov@linux.intel.com>, "Hansen,
 Dave" <dave.hansen@intel.com>
CC: "bp@alien8.de" <bp@alien8.de>, "dave.hansen@linux.intel.com"
	<dave.hansen@linux.intel.com>, "hpa@zytor.com" <hpa@zytor.com>,
	"mingo@redhat.com" <mingo@redhat.com>, "tglx@linutronix.de"
	<tglx@linutronix.de>, "x86@kernel.org" <x86@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-coco@lists.linux.dev" <linux-coco@lists.linux.dev>
References: <20240517121450.20420-1-jgross@suse.com>
 <ohvjbokpaxagc26kxmlrujab7cw3bekgi5ln7dt46cbsaxcqqh@crvqeohfazmf>
 <f63e1217-3dbe-458d-8c14-7880811d30ba@suse.com>
 <2a2guben2ysyeb43rzg6zelzpa57o24ufai3mi6ocewwvgu63l@c7dle47q7hzw>
 <03d27b6a-be96-44d7-b4ea-aa00ccab4cc5@suse.com>
 <fc0e8ab7-86d4-4428-be31-82e1ece6dd21@intel.com>
 <c0067319-2653-4cbd-8fee-1ccf21b1e646@suse.com>
 <6df4fb48-9947-46ec-af5a-66fa06d6a83b@intel.com>
 <86ca805d-7ed7-47dd-8228-5e19fbade53f@suse.com>
 <f7edef9c-5eb5-4664-a193-3bb063674742@intel.com>
 <f02d9ebb-a2b3-4cb3-871b-34324d374d01@suse.com>
 <4b3adb59-50ea-419e-ad02-e19e8ca20dee@intel.com>
 <c7319c0614c9a644fa1f9b349bf654834b615543.camel@intel.com>
 <e2d844ad-182a-4fc0-a06a-d609c9cbef74@suse.com>
 <2e07ebc774bf176ebfab40e6ba8bc14ae6a94e0c.camel@intel.com>
 <a9bbb31e6660343e95d3351febc6e3b9661a7944.camel@intel.com>
 <c188878d-ff67-45da-a963-e643bfb927a8@suse.com>
 <cc57bb62-be80-4c97-a8f8-5cd0548c3c11@intel.com>
Content-Language: en-US
In-Reply-To: <cc57bb62-be80-4c97-a8f8-5cd0548c3c11@intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dFpsQ29IVkp5OGE0YmJsNmd2TlE1bjY2b0F5c1ZPR2Iwb2xnUjg5a3FlZjBH?=
 =?utf-8?B?TnhHcjBIODVUQThlTkorZC9YNFJkbGlITktMTnJ5ZS9iMWJkQ1V0V1VkVnRM?=
 =?utf-8?B?ZGJNTFhmNVpSQ21YZkM2VUVXVllNSzdqTVl0bnR2QzN6NVcvZHhxUzVLM2dE?=
 =?utf-8?B?VUREMGptSEhnblpwMVBUcEF6SzJSRXVxZWEwcFRFL0FxY1dBODdNQ2VXbkdh?=
 =?utf-8?B?aDJLT0NIY2MrTTVoVklRWjVZVk9nRnhmamJ4bm1BWmp1bVNwVHpVNktPQ0k4?=
 =?utf-8?B?dnllVldQL1pvc0JGL1pPNGU5b2tCaXlKWmVuNytXUmRrWlp0MDZFWDg2YnFl?=
 =?utf-8?B?YWd3TitJVk9IYWlKMXR2b2tlOHpJa2dwSUhZRkdhWkpYZ2RzdEVmek0xaEtk?=
 =?utf-8?B?RHpRVzRsZFZrVzR4YVMyQVhoNlEzcCt3dUc1NE5pLytRSjVYWkhERzY1b0Zt?=
 =?utf-8?B?WkhqNFZmbkNjRXdHUC9qK1ZySDByN2ZUTEw3RTRyYTNFZHNSR0lKb3FhV3Rz?=
 =?utf-8?B?SmQybVBiaDQ3ejhPZ2tWd1cwZGtzbUtuRjY0N2RVc1JJNTNLbTZUcHlNdzhL?=
 =?utf-8?B?R0ZpY0dlNStkai9QeWFVN0VVSmZ5bVlBS3Q0bFpaSTJZeHFUcDNGYlg3RCsy?=
 =?utf-8?B?Y1E1NDRnNkZ4K0hCbmc3dnhYSTZWQVJZK3FZd2xrUksyZWQ1MldFVDV3ZEJr?=
 =?utf-8?B?a3R4VWsydzh2STcweGZkYVZEc015Y1owV1BjTC8yQm5PNXI4MXBhWHJJd2Fw?=
 =?utf-8?B?MXZleTZiaVM5dU11U0ZWUEhMc1duTGx2ekx2K2xNRWlNUUwxMSt2OGlKaXkz?=
 =?utf-8?B?ck9Mb2l4cVNVKzBVSzIyU1pWMzBCbldXWkRYcHh1Kytrd1pVWE1kLzFaenlp?=
 =?utf-8?B?VUlEYnFFaFh0NXZCTVpOQVF5TmxoZjNPRVY1bVRmNzZ4b0dRWDUzREtFYldy?=
 =?utf-8?B?SjB1N0lFNDRxcWZJemswam9UMzJGTldtWWNiYXpQWlg1ZmpLV200MUR3Lzg4?=
 =?utf-8?B?RzkxOUpqVG9UZTlpU0Nwb1luc2UrcUw0b2RtVXJEVEpvSVpjNGtYQ01Xayt2?=
 =?utf-8?B?MHZ0K0s4Q1hMMFdzcHBNdFNHRWNCMHluaEVrdTB3RnVTV1FrQWZiYnk2Wjhq?=
 =?utf-8?B?ajNabDBzVUxLc2UrdVBrQ2t5TldsUnI0dWhlZUU1bEZBVFdHZER2d1FkZUtG?=
 =?utf-8?B?YXBKRndib1FIc3FnSE9HUnAyb3BJN2h5TGVncnZhODNGTU91OHJuTkN5ekxJ?=
 =?utf-8?B?UXBOZmtseEV2dmZlZmFOOXpwT2kwcXVJaU5kVm1RRXRqL2xTazFyVTZaU2F5?=
 =?utf-8?B?eEE2NWY5R3IvTnlmb2xPRXhZeVd5dGJlK3kzNDR5NEptc0JBb21jNWZrTUdo?=
 =?utf-8?B?THVkbmZXTStVcmlnaVNkZFFYYVI1RDA3WE9IUGYrV3pVN3Z0eWZvNXVtVTJ1?=
 =?utf-8?B?VWlNbDFUSjJyOEY3VEdDMkQ1TFJyVUwrNC9iam9wWEs2NTBVRUJxMUtwdnd4?=
 =?utf-8?B?VGxreTJ0dXRLTkhnaTg4SllnaTl5a1A3V05HUTFQalErcFVMNlo4OGgxWEh4?=
 =?utf-8?B?NzVVRkRGUkcxOVBDbmlwZzBnbWRLWTdEOVNzaVhFZHdGek9Sd1lRR0EzTnRi?=
 =?utf-8?B?WjM2ZVJHVXMzUU9nSjB2NS8zanhHcHU3S2ZGa0tRSHhwOFFCSmo1bDl2cVhU?=
 =?utf-8?B?MUMyaEtHb0U5NjkxZGhJSFpjMlU4bDU5L3p5WndsVkkwL0RYSURBNVQ3WXEr?=
 =?utf-8?B?S0NSRXd4SlZ4UStuVDRlVWxHRGxiR0E0c0NOKzczaTU0dHR6Z2ZsbU5JNXpy?=
 =?utf-8?B?d0IvVDhHekJia2IyQUJzSXJJWXF5ZzNmd3J6MGoycStyRUhCWnMvNmROT3hv?=
 =?utf-8?B?aFFxbmN3dVdMZ1F2OS81eENMeWFsUzlQVEZrZ2NqdkU5NVh3dHc2MzJCVHMy?=
 =?utf-8?B?RnhBbUJjMHZjS3ZRN2hNbE1TS01IL0JMV1NRam9ESHpZYVVJbTBhVk8yTEh0?=
 =?utf-8?B?UEVnNVkxamZJWDdzRWRacHZqS21TUExBVFRJbjQxVmUyeEtoYkl0OVZ4d2V4?=
 =?utf-8?B?UlBFZmRpTlNBL1dxdWpMcTh4UE5aQXR1dVVmczFIMTlKS1F0QjlMaGZpckRV?=
 =?utf-8?Q?8BXJmPDMhnkct4L5y8NUaEYua?=
X-MS-Exchange-CrossTenant-Network-Message-Id: ef1de8aa-9e9b-49c9-20ad-08dc7b801d75
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 May 2024 23:28:56.2017
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Zu8wMYN+wC0rTgqEsWtfDtKc6/UDYLLphcGmWZyPx+74UvNtaAzyzf6NOx0uAFCJZVr0VDRuGj9J1zT3htSDww==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB8159
X-OriginatorOrg: intel.com


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@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;
> 

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.