Received-SPF: pass (google.com: domain of linux-kernel+bounces-109483-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <76e918cf-44ef-4e9b-9e56-84256b637398@intel.com>
Date: Thu, 21 Mar 2024 12:09:57 +1300
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v19 030/130] KVM: TDX: Add helper functions to print TDX
 SEAMCALL error
Content-Language: en-US
To: Isaku Yamahata <isaku.yamahata@intel.com>
CC: <kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<isaku.yamahata@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>,
	<erdemaktas@google.com>, Sean Christopherson <seanjc@google.com>, Sagi Shahar
	<sagis@google.com>, <chen.bo@intel.com>, <hang.yuan@intel.com>,
	<tina.zhang@intel.com>, Binbin Wu <binbin.wu@linux.intel.com>, Yuan Yao
	<yuan.yao@intel.com>, <isaku.yamahata@linux.intel.com>
References: <cover.1708933498.git.isaku.yamahata@intel.com>
 <1259755bde3a07ec4dc2c78626fa348cf7323b33.1708933498.git.isaku.yamahata@intel.com>
 <315bfb1b-7735-4e26-b881-fcaa25e0d3f3@intel.com>
 <20240320215013.GJ1994522@ls.amr.corp.intel.com>
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <20240320215013.GJ1994522@ls.amr.corp.intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?enZIZTFKdCt6cXBSZGhyMnpvcWZsN2cvVjRwcTJlY0ZxSkdVWjFBUTF2aHp5?=
 =?utf-8?B?NzlOYjgwWUh2bHdrMmYyeXVuN05UK2FzNDdlejhGVUJja3cxbFNwWkhwYXFj?=
 =?utf-8?B?RVhtU1pJVmUwMXhjRXNqUGw5bVRtYXhLaW9keWtUZ1g3Yk9pMVowMThSV3Nt?=
 =?utf-8?B?V1pmN1FuOFprRVZ0MjNwUzRrbXpJK2s0L2pVNHR2dmVQbnVMcjY2dW1sOGxy?=
 =?utf-8?B?MnVsLzk0ZVZ0WHhnS2xYQVpCeEp1ak5zcEdCQUozM2ppeVZob0VyU3hmckVR?=
 =?utf-8?B?VTZxb0lnOU9FS3dNV2hhR3l3RlVDbWVBWDhicFRmdGNVNFkvRDJPZlJzZnE0?=
 =?utf-8?B?MllPWFJyY29tZlJVcDNaYVk4Nk1VajJ3RkdvOG0zM24yeGZOWktyQWRFcEZF?=
 =?utf-8?B?ZFk5R1RzSndWREdLUjRpNi92UEk1QTU3MFM1YXNORXJBbXRJK3U3M1RlWUdN?=
 =?utf-8?B?YlZvbURhSDkvV2h5dTFKT0dXTEJtMHV5UlRHUzJZZGUzN3F1RWxDdTV2VVFz?=
 =?utf-8?B?OEs3eWRlaHczSVpQbXo1Q2dGbnBnbU1ZUVlGWlFkZ21nRks4OTFKcms3Mlpq?=
 =?utf-8?B?NEZyMjJSNm53TkNsR0d3WGJkV1d5RFRYcUFteThVdWZBSGlwQ0tCdDZXT0N6?=
 =?utf-8?B?SEJZWXFsR3JwS3Zld2pRUzVYWjc0cjVUUXZsR0pkSHlmU3NIR2l0am9MQjhv?=
 =?utf-8?B?ZncvaGJxbTYwUGRienVZcVgwTnFQUS9VeGFxUVhBa24vVXkyZnZEVUhraGtG?=
 =?utf-8?B?c0RxWFZEakhMd1FlWSs0Z1grYzVWQlJDM3ZaMGxMaTJHUitXdFQxNTNiWnFx?=
 =?utf-8?B?blhvT24zNVZ5REZwM1N2QlpNRmxMaUpZWElHRjU5VlBENldQMGRYRW5ZMUlv?=
 =?utf-8?B?NjYveWo4andWcVFVckd3dmVoQ3FUVkorbmVkcU9BSmJ5MWZ4UHhwdElwei9S?=
 =?utf-8?B?cm9oRmQvODVnVzR0ZjV3aFBOT3BXUzh6OXZGVlJhcEs5cCsvR0ZEVFVCc0l0?=
 =?utf-8?B?ZWlSMzJEOGQ0Smx6UHg0bXlsVjJFQ1JvYkpHTERvdk9BNXBLSTZSVjl3aTQ1?=
 =?utf-8?B?Ti94ZEJEOHdZcnFOMmlmUUVXb3g4RktmdEtxalVEb2thTzh6a2ZISUdweTNw?=
 =?utf-8?B?bnY5eC9wT3RCSmExNWNXMjh4d2M5cW9rMUQ0d3pPSzVONEh3VlNzV2dpWXF0?=
 =?utf-8?B?TlNGMUdLVlhGNm1Yb0JJcmlTN3BoSEhrejFiREtiM0VIbTBEb09KUzB0UURD?=
 =?utf-8?B?dEJhVmxpS0xsTnZSMHZnaHZYRndheGt3QXJHMzN5bVlkN1p5elJTZzA2TW5n?=
 =?utf-8?B?K1F5THhzbW4ySHNFcnZaSWMxanBJNW5zcHFzK1dXc2FVQmFpcm5JcThJNmI1?=
 =?utf-8?B?NzNQTEg5UXJxaW5pY1NDVVorYWMwVG5uekRDenRVeVVacWNjcjZtS0hnWkNQ?=
 =?utf-8?B?b3dHMVF3eGVTUXhWYWliUEJJNjhpeDdyaXVLQjFGeGM1TTVOMFRYVVZ5ZTB6?=
 =?utf-8?B?eERRNlZnVWZRbTU3clNpUmI5OFh4RTM5TDB1eHF1WExuUnA2c3lpbDVyd1V2?=
 =?utf-8?B?YzdMZFJjTll2R1ZsWkNHQ2tZcFVGdVFvR20rY3hpeFY0WDBqTVNOUnN1VWZn?=
 =?utf-8?B?T0kxekJBbHdOVEpVTFlLSHZ6M2NYbzNIYlJIdkRhOTVEc0tCU3BXd3pwdUpL?=
 =?utf-8?B?bjdQSEdXL005cXJoOG9pWVJLV3RmNEozUXArWStETDB6US9PTmtmVTcwcE5k?=
 =?utf-8?B?Mm4zZzI1TFE5eXRwZ2JvY1M1akxoMUYvTHcxUGwxVGd5S1VUaURrKzlid1Zw?=
 =?utf-8?B?OGluWGU3UFhIWUtPSWtvK3hheHFtMXl0OUpUMjRTRXMxMkdlUTY2RHhuMk1z?=
 =?utf-8?B?bHliaTNRS2RUcE5oMCt4dlEzOUgzV3BsQ2x3K2hhbjFmN0VkemhBRXgzbHUr?=
 =?utf-8?B?QUJCK0ZRT3ZndGVTYnlWSlRjak92eEd0REx0OWlrUGMweGFxa3B6a2QzYzdM?=
 =?utf-8?B?WnA3K093bE8wczVGL0plZDA4Q2VQRjNyV0Qrb0lNQzhGVzZkN3BzSmFUK0Z2?=
 =?utf-8?B?Y1ZSdmxkUmdEMW8zaXR2NndwMnozYVQ3VlJoblk2Mm12aDBseFZRK0E4cjFn?=
 =?utf-8?Q?t+WqGGxruq8cZG3qGriVkv1ZG?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 37c759bf-ef82-4434-345e-08dc4932e307
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Mar 2024 23:10:08.7282
 (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: /yPfPLC3kJi4+lb+mf+F7CYP4+uGJ9AH3Bcx8kMMXgRyTxGxugw9SsQetpONGnd1ZYAzSqsEo4GgzIzSXl1flw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB7569
X-OriginatorOrg: intel.com


> Does it make sense?
> 
> void pr_tdx_error(u64 op, u64 error_code)
> {
>          pr_err_ratelimited("SEAMCALL (0x%016llx) failed: 0x%016llx\n",
>                             op, error_code);
> }

Should we also have a _ret version?

void pr_seamcall_err(u64 op, u64 err)
{
	/* A comment to explain why using the _ratelimited() version? */
	pr_err_ratelimited(...);
}

void pr_seamcall_err_ret(u64 op, u64 err, struct tdx_module_args *arg)
{
	pr_err_seamcall(op, err);
	
	pr_err_ratelimited(...);
}

(Hmm... if you look at the tdx.c in TDX host, there's similar code 
there, and again, it was a little bit annoying when I did that..)

Again, if we just use seamcall_ret() for ALL SEAMCALLs except VP.ENTER, 
we can simply have one..

> 
> void pr_tdx_sept_error(u64 op, u64 error_code, const union tdx_sept_entry *entry,
> 		       const union tdx_sept_level_state *level_state)
> {
> #define MSG \
>          "SEAMCALL (0x%016llx) failed: 0x%016llx entry 0x%016llx level_state 0x%016llx\n"
>          pr_err_ratelimited(MSG, op, error_code, entry->raw, level_state->raw);
> }

A higher-level wrapper to print SEPT error is fine to me, but do it in a 
separate patch.