Message ID | 9565b2ccc347752607039e036fd8d19d78401b53.1699527082.git.kai.huang@intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b129:0:b0:403:3b70:6f57 with SMTP id q9csp379394vqs; Thu, 9 Nov 2023 03:57:10 -0800 (PST) X-Google-Smtp-Source: AGHT+IG7y40KLXZuuSbwBerk0MqzbEYcSqz0ha9cg+SmLlEU7aG+q/08/MVLKo7GkjgLYwz7wDjr X-Received: by 2002:a05:6300:8003:b0:16b:e2e5:fe55 with SMTP id an3-20020a056300800300b0016be2e5fe55mr5231840pzc.51.1699531030410; Thu, 09 Nov 2023 03:57:10 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1699531030; cv=none; d=google.com; s=arc-20160816; b=lryNQ++1fo0FOGI7UMAF57oAVLCYQKwgDfGydZLHdRAjxPiD67vqBHBqyNgHhKIhSy ujxetpPYU7UYE8XUX+Aumh3r7G0jEfA/trmGVDaFsvDoEiYMzqTSk+NBh5+Sq3Qc7kLn BXdHyZTbeHuHrDPc9UiOxWohSMFXWlCjB/J/h44yM+8hpXEuPd2GmugTKP9zKzi2PdkR 28YXzq7UqJv76g8B4c3p1csxtojn2Qqmh7GWqKXWrSV+DA6lV9ne52eBBJeHDlYC/iuk uCV04ykF3LPicpmFcuA3ZVjSwiInoHr5uQBI1aS4jjShEwLTfvfsgLjqGsCWUpEO7vMF UGIg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=PT39/zHrtpH2paRcRURY3Qcbw7iuMnIESeSBk1RYBkA=; fh=WBgbLtMencYhgeHuu2sUs5b9THiYLgy17d2w1N+xuf4=; b=v+KXW+zQPHmFbrP1cCcxpnFpAu7cppnq+flJLxDwPFoQOCd3pFlY6wL4b1CHNddeyw ukEDHtTovjkU/LH0DBg+pRkkjATYffQhSwgjrPLD47QAtFkVcqCjT/RFYFPK6PpRPIyr lAUfQgbUjh/fWDfKQlv0CRWbiAt1UsOK/XHYpQwBVh4jlQRO11BIhZzV+vxIewBGYpw1 7jqozuiXElDLXaLhOUIfKPRToI/b6XrJ5SV55zVbJkVRa0bWDaj4hQTVI+X8G0DtCXqH Y1XJbLdCsecnvetri+YBstnJwb426wh933Wj1rgu7VxUolKnDDDgP55vl2lCbhU3STEK bdKg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=XobDRleS; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id lr13-20020a17090b4b8d00b002808736e2b3si1665845pjb.142.2023.11.09.03.57.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Nov 2023 03:57:10 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=XobDRleS; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id DBC038360169; Thu, 9 Nov 2023 03:57:06 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234218AbjKIL47 (ORCPT <rfc822;jaysivo@gmail.com> + 32 others); Thu, 9 Nov 2023 06:56:59 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47868 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234424AbjKIL4u (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 9 Nov 2023 06:56:50 -0500 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.8]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 44DA1258A; Thu, 9 Nov 2023 03:56:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1699531007; x=1731067007; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=E2LIAFh/IrogizgMIxQnR6vkAoXLaTONKab0ZSp7Cg8=; b=XobDRleSa6BBWU7ZjVqJWZkDm+xz8gkpNDrRPNpUD0VWkdG748ImGvwl 5T2Jbhte55iLEUNMwEmWn64LugRrtf7F2ZUpbNgqjNjzmyCt3PtLIVpct PGYLmkHZ9ODDFFqyNJyyRFLyk9K+rMwCo+OZLnHNaCM6TXtcl7Nnrw1m1 M9g7dIPUxf072drQn0bw16r+2anrwmXRsDgcNpIuXNxJUl14LooXzJNPm GntOF6KUQRurUkhmNk50NLni539wSh7y8xy3DrvCuB2G29tZluVelUecB jYVcZu3EQ1MEwreENC+sPF4AxF3UA5thBmU7gpoBj7oHNjfLNS0gbKRnl g==; X-IronPort-AV: E=McAfee;i="6600,9927,10888"; a="2936374" X-IronPort-AV: E=Sophos;i="6.03,289,1694761200"; d="scan'208";a="2936374" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmvoesa102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Nov 2023 03:56:47 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10888"; a="766976648" X-IronPort-AV: E=Sophos;i="6.03,289,1694761200"; d="scan'208";a="766976648" Received: from shadphix-mobl.amr.corp.intel.com (HELO khuang2-desk.gar.corp.intel.com) ([10.209.83.35]) by fmsmga007-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Nov 2023 03:56:40 -0800 From: Kai Huang <kai.huang@intel.com> To: linux-kernel@vger.kernel.org, kvm@vger.kernel.org Cc: x86@kernel.org, dave.hansen@intel.com, kirill.shutemov@linux.intel.com, peterz@infradead.org, tony.luck@intel.com, tglx@linutronix.de, bp@alien8.de, mingo@redhat.com, hpa@zytor.com, seanjc@google.com, pbonzini@redhat.com, rafael@kernel.org, david@redhat.com, dan.j.williams@intel.com, len.brown@intel.com, ak@linux.intel.com, isaku.yamahata@intel.com, ying.huang@intel.com, chao.gao@intel.com, sathyanarayanan.kuppuswamy@linux.intel.com, nik.borisov@suse.com, bagasdotme@gmail.com, sagis@google.com, imammedo@redhat.com, kai.huang@intel.com Subject: [PATCH v15 05/23] x86/virt/tdx: Handle SEAMCALL no entropy error in common code Date: Fri, 10 Nov 2023 00:55:42 +1300 Message-ID: <9565b2ccc347752607039e036fd8d19d78401b53.1699527082.git.kai.huang@intel.com> X-Mailer: git-send-email 2.41.0 In-Reply-To: <cover.1699527082.git.kai.huang@intel.com> References: <cover.1699527082.git.kai.huang@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Thu, 09 Nov 2023 03:57:06 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1782087449637707169 X-GMAIL-MSGID: 1782087449637707169 |
Series |
TDX host kernel support
|
|
Commit Message
Kai Huang
Nov. 9, 2023, 11:55 a.m. UTC
Some SEAMCALLs use the RDRAND hardware and can fail for the same reasons as RDRAND. Use the kernel RDRAND retry logic for them. There are three __seamcall*() variants. Do the SEAMCALL retry in common code and add a wrapper for each of them. Signed-off-by: Kai Huang <kai.huang@intel.com> Reviewed-by: Kirill A. Shutemov <kirll.shutemov@linux.intel.com> Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> --- v14 -> v15: - Added Sathy's tag. v13 -> v14: - Use real function sc_retry() instead of using macros. (Dave) - Added Kirill's tag. v12 -> v13: - New implementation due to TDCALL assembly series. --- arch/x86/include/asm/tdx.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
Comments
On 11/9/23 03:55, Kai Huang wrote: > Some SEAMCALLs use the RDRAND hardware and can fail for the same reasons > as RDRAND. Use the kernel RDRAND retry logic for them. > > There are three __seamcall*() variants. Do the SEAMCALL retry in common > code and add a wrapper for each of them. The new common wrapper looks great, thanks: Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
On Fri, Nov 10, 2023 at 12:55:42AM +1300, Kai Huang <kai.huang@intel.com> wrote: > Some SEAMCALLs use the RDRAND hardware and can fail for the same reasons > as RDRAND. Use the kernel RDRAND retry logic for them. > > There are three __seamcall*() variants. Do the SEAMCALL retry in common > code and add a wrapper for each of them. > > Signed-off-by: Kai Huang <kai.huang@intel.com> > Reviewed-by: Kirill A. Shutemov <kirll.shutemov@linux.intel.com> > Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > --- > > v14 -> v15: > - Added Sathy's tag. > > v13 -> v14: > - Use real function sc_retry() instead of using macros. (Dave) > - Added Kirill's tag. > > v12 -> v13: > - New implementation due to TDCALL assembly series. > > --- > arch/x86/include/asm/tdx.h | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h > index ea9a0320b1f8..f1c0c15469f8 100644 > --- a/arch/x86/include/asm/tdx.h > +++ b/arch/x86/include/asm/tdx.h > @@ -24,6 +24,11 @@ > #define TDX_SEAMCALL_GP (TDX_SW_ERROR | X86_TRAP_GP) > #define TDX_SEAMCALL_UD (TDX_SW_ERROR | X86_TRAP_UD) > > +/* > + * TDX module SEAMCALL leaf function error codes > + */ > +#define TDX_RND_NO_ENTROPY 0x8000020300000000ULL > + > #ifndef __ASSEMBLY__ > > /* > @@ -84,6 +89,27 @@ u64 __seamcall(u64 fn, struct tdx_module_args *args); > u64 __seamcall_ret(u64 fn, struct tdx_module_args *args); > u64 __seamcall_saved_ret(u64 fn, struct tdx_module_args *args); > > +#include <asm/archrandom.h> > + > +typedef u64 (*sc_func_t)(u64 fn, struct tdx_module_args *args); > + > +static inline u64 sc_retry(sc_func_t func, u64 fn, > + struct tdx_module_args *args) > +{ > + int retry = RDRAND_RETRY_LOOPS; > + u64 ret; > + > + do { > + ret = func(fn, args); > + } while (ret == TDX_RND_NO_ENTROPY && --retry); This loop assumes that args isn't touched when TDX_RND_NO_ENTRYPOY is returned. It's not true. TDH.SYS.INIT() and TDH.SYS.LP.INIT() clear RCX, RDX, etc on error including TDX_RND_NO_ENTRY. Because TDH.SYS.INIT() takes RCX as input, this wrapper doesn't work. TDH.SYS.LP.INIT() doesn't use RCX, RDX ... as input. So it doesn't matter. Other SEAMCALLs doesn't touch registers on the no entropy error. TDH.EXPORTS.STATE.IMMUTABLE(), TDH.IMPORTS.STATE.IMMUTABLE(), TDH.MNG.ADDCX(), and TDX.MNG.CREATE(). TDH.SYS.INIT() is an exception.
> > +#include <asm/archrandom.h> > > + > > +typedef u64 (*sc_func_t)(u64 fn, struct tdx_module_args *args); > > + > > +static inline u64 sc_retry(sc_func_t func, u64 fn, > > + struct tdx_module_args *args) > > +{ > > + int retry = RDRAND_RETRY_LOOPS; > > + u64 ret; > > + > > + do { > > + ret = func(fn, args); > > + } while (ret == TDX_RND_NO_ENTROPY && --retry); > > This loop assumes that args isn't touched when TDX_RND_NO_ENTRYPOY is returned. > It's not true. TDH.SYS.INIT() and TDH.SYS.LP.INIT() clear RCX, RDX, etc on > error including TDX_RND_NO_ENTRY. Because TDH.SYS.INIT() takes RCX as input, > this wrapper doesn't work. TDH.SYS.LP.INIT() doesn't use RCX, RDX ... as > input. So it doesn't matter. > > Other SEAMCALLs doesn't touch registers on the no entropy error. > TDH.EXPORTS.STATE.IMMUTABLE(), TDH.IMPORTS.STATE.IMMUTABLE(), TDH.MNG.ADDCX(), > and TDX.MNG.CREATE(). TDH.SYS.INIT() is an exception. If I am reading the spec (TDX module 1.5 ABI) correctly the TDH.SYS.INIT doesn't return TDX_RND_NO_ENTROPY. TDH.SYS.LP.INIT indeed can return NO_ENTROPY but as you said it doesn't take any register as input. So technically the code works fine. (Even the TDH.SYS.INIT can return NO_ENTROPY the code still works fine because the RCX must be 0 for TDH.SYS.INIT.) Also, I can hardly think out of any reason why TDX module needs to clobber input registers in case of NO_ENTROPY for *ANY* SEAMCALL. But despite that, I am not opposing the idea that it *MIGHT* be better to "not assume" NO_ENTROPY will never clobber registers either, e.g., for the sake of future extendibility. In this case, the below diff should address: diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h index a621721f63dd..962a7a6be721 100644 --- a/arch/x86/include/asm/tdx.h +++ b/arch/x86/include/asm/tdx.h @@ -97,12 +97,23 @@ typedef u64 (*sc_func_t)(u64 fn, struct tdx_module_args *args); static inline u64 sc_retry(sc_func_t func, u64 fn, struct tdx_module_args *args) { + struct tdx_module_args _args = *args; int retry = RDRAND_RETRY_LOOPS; u64 ret; - do { - ret = func(fn, args); - } while (ret == TDX_RND_NO_ENTROPY && --retry); +again: + ret = func(fn, args); + if (ret == TDX_RND_NO_ENTROPY && --retry) { + /* + * Do not assume TDX module will never clobber the input + * registers when any SEAMCALL fails with out of entropy. + * In this case the original input registers in @args + * are clobbered. Always restore the input registers + * before retrying the SEAMCALL. + */ + *args = _args; + goto again; + } return ret; } The downside is we will have an additional memory copy of 'struct tdx_module_args' for each SEAMCALL, but I don't believe this will have any difference in practice. Or, we can go and ask TDX module guys to promise no input registers will be clobbered in case of NO_ENTROPY. Hi Dave, Do you have any opinion?
On Wed, Nov 15, 2023 at 10:41:46AM +0000, "Huang, Kai" <kai.huang@intel.com> wrote: > > > > +#include <asm/archrandom.h> > > > + > > > +typedef u64 (*sc_func_t)(u64 fn, struct tdx_module_args *args); > > > + > > > +static inline u64 sc_retry(sc_func_t func, u64 fn, > > > + struct tdx_module_args *args) > > > +{ > > > + int retry = RDRAND_RETRY_LOOPS; > > > + u64 ret; > > > + > > > + do { > > > + ret = func(fn, args); > > > + } while (ret == TDX_RND_NO_ENTROPY && --retry); > > > > This loop assumes that args isn't touched when TDX_RND_NO_ENTRYPOY is returned. > > It's not true. TDH.SYS.INIT() and TDH.SYS.LP.INIT() clear RCX, RDX, etc on > > error including TDX_RND_NO_ENTRY. Because TDH.SYS.INIT() takes RCX as input, > > this wrapper doesn't work. TDH.SYS.LP.INIT() doesn't use RCX, RDX ... as > > input. So it doesn't matter. > > > > Other SEAMCALLs doesn't touch registers on the no entropy error. > > TDH.EXPORTS.STATE.IMMUTABLE(), TDH.IMPORTS.STATE.IMMUTABLE(), TDH.MNG.ADDCX(), > > and TDX.MNG.CREATE(). TDH.SYS.INIT() is an exception. > > If I am reading the spec (TDX module 1.5 ABI) correctly the TDH.SYS.INIT doesn't > return TDX_RND_NO_ENTROPY. The next updated spec would fix it. > TDH.SYS.LP.INIT indeed can return NO_ENTROPY but as > you said it doesn't take any register as input. So technically the code works > fine. (Even the TDH.SYS.INIT can return NO_ENTROPY the code still works fine > because the RCX must be 0 for TDH.SYS.INIT.) Ah yes, I agree with you. So it doesn't matter. > Also, I can hardly think out of any reason why TDX module needs to clobber input > registers in case of NO_ENTROPY for *ANY* SEAMCALL. But despite that, I am not > opposing the idea that it *MIGHT* be better to "not assume" NO_ENTROPY will > never clobber registers either, e.g., for the sake of future extendibility. In > this case, the below diff should address: Now we agreed that TDH.SYS.INIT() and TDH.SYS.LP.INIT() doesn't matter, I'm fine with this patch. (TDX KVM handles other SEAMCALLS itself.) Reviewed-by: Isaku Yamahata <isaku.yamahata@intel.com>
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h index ea9a0320b1f8..f1c0c15469f8 100644 --- a/arch/x86/include/asm/tdx.h +++ b/arch/x86/include/asm/tdx.h @@ -24,6 +24,11 @@ #define TDX_SEAMCALL_GP (TDX_SW_ERROR | X86_TRAP_GP) #define TDX_SEAMCALL_UD (TDX_SW_ERROR | X86_TRAP_UD) +/* + * TDX module SEAMCALL leaf function error codes + */ +#define TDX_RND_NO_ENTROPY 0x8000020300000000ULL + #ifndef __ASSEMBLY__ /* @@ -84,6 +89,27 @@ u64 __seamcall(u64 fn, struct tdx_module_args *args); u64 __seamcall_ret(u64 fn, struct tdx_module_args *args); u64 __seamcall_saved_ret(u64 fn, struct tdx_module_args *args); +#include <asm/archrandom.h> + +typedef u64 (*sc_func_t)(u64 fn, struct tdx_module_args *args); + +static inline u64 sc_retry(sc_func_t func, u64 fn, + struct tdx_module_args *args) +{ + int retry = RDRAND_RETRY_LOOPS; + u64 ret; + + do { + ret = func(fn, args); + } while (ret == TDX_RND_NO_ENTROPY && --retry); + + return ret; +} + +#define seamcall(_fn, _args) sc_retry(__seamcall, (_fn), (_args)) +#define seamcall_ret(_fn, _args) sc_retry(__seamcall_ret, (_fn), (_args)) +#define seamcall_saved_ret(_fn, _args) sc_retry(__seamcall_saved_ret, (_fn), (_args)) + bool platform_tdx_enabled(void); #else static inline bool platform_tdx_enabled(void) { return false; }