diff mbox series

[v12,06/22] x86/virt/tdx: Handle SEAMCALL running out of entropy error

Message ID b5bf58246659572bd68d46b14e90e9e5b37f7f93.1687784645.git.kai.huang@intel.com (mailing list archive)
State New, archived
Headers show
Series TDX host kernel support | expand

Commit Message

Huang, Kai June 26, 2023, 2:12 p.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.

Signed-off-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---

v11 -> v12:
 - Added tags from Dave/Kirill/David.
 - Improved changelog (Dave).
 - Slight code improvement (David)
   - Initialize retry directly when declaring it.
   - Simplify comment around mimic rdrand_long().

v10 -> v11:
 - New patch

---
 arch/x86/virt/vmx/tdx/tdx.c | 16 ++++++++++++++--
 arch/x86/virt/vmx/tdx/tdx.h | 17 +++++++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)

Comments

Peter Zijlstra June 28, 2023, 1:02 p.m. UTC | #1
On Tue, Jun 27, 2023 at 02:12:36AM +1200, Kai Huang wrote:

>  	cpu = get_cpu();
> -	sret = __seamcall(fn, rcx, rdx, r8, r9, out);
> +
> +	/*
> +	 * Certain SEAMCALL leaf functions may return error due to
> +	 * running out of entropy, in which case the SEAMCALL should
> +	 * be retried.  Handle this in SEAMCALL common function.
> +	 *
> +	 * Mimic rdrand_long() retry behavior.

Yeah, except that doesn't have preemption disabled.. you do.

> +	 */
> +	do {
> +		sret = __seamcall(fn, rcx, rdx, r8, r9, out);
> +	} while (sret == TDX_RND_NO_ENTROPY && --retry);
> +
>  	put_cpu();
Huang, Kai June 28, 2023, 11:30 p.m. UTC | #2
On Wed, 2023-06-28 at 15:02 +0200, Peter Zijlstra wrote:
> On Tue, Jun 27, 2023 at 02:12:36AM +1200, Kai Huang wrote:
> 
> >  	cpu = get_cpu();
> > -	sret = __seamcall(fn, rcx, rdx, r8, r9, out);
> > +
> > +	/*
> > +	 * Certain SEAMCALL leaf functions may return error due to
> > +	 * running out of entropy, in which case the SEAMCALL should
> > +	 * be retried.  Handle this in SEAMCALL common function.
> > +	 *
> > +	 * Mimic rdrand_long() retry behavior.
> 
> Yeah, except that doesn't have preemption disabled.. you do.
> 

Agreed.  I'll change to only disable preemption around one SEAMCALL (for error
printing CPU id).

But doing this, it makes more sense to split this wrapper function out as a
separate patch and put it after the skeleton patch since this way we require the
caller to guarantee all online cpus must have been in VMX operation (SEAMCALL
requires CPU must be in VMX operation), which is the assumption that
tdx_enable() has anyway.

Hi Kirill/Dave/David,

Please let me know if you have comments?

> > +	 */
> > +	do {
> > +		sret = __seamcall(fn, rcx, rdx, r8, r9, out);
> > +	} while (sret == TDX_RND_NO_ENTROPY && --retry);
> > +
> >  	put_cpu();
diff mbox series

Patch

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index f8233cba5931..141d12376c4d 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -15,6 +15,7 @@ 
 #include <linux/smp.h>
 #include <asm/msr-index.h>
 #include <asm/msr.h>
+#include <asm/archrandom.h>
 #include <asm/tdx.h>
 #include "tdx.h"
 
@@ -32,12 +33,23 @@  static int __always_unused seamcall(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
 				    u64 *seamcall_ret,
 				    struct tdx_module_output *out)
 {
+	int cpu, retry = RDRAND_RETRY_LOOPS;
 	u64 sret;
-	int cpu;
 
 	/* Need a stable CPU id for printing error message */
 	cpu = get_cpu();
-	sret = __seamcall(fn, rcx, rdx, r8, r9, out);
+
+	/*
+	 * Certain SEAMCALL leaf functions may return error due to
+	 * running out of entropy, in which case the SEAMCALL should
+	 * be retried.  Handle this in SEAMCALL common function.
+	 *
+	 * Mimic rdrand_long() retry behavior.
+	 */
+	do {
+		sret = __seamcall(fn, rcx, rdx, r8, r9, out);
+	} while (sret == TDX_RND_NO_ENTROPY && --retry);
+
 	put_cpu();
 
 	/* Save SEAMCALL return code if the caller wants it */
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 48ad1a1ba737..55dbb1b8c971 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -4,6 +4,23 @@ 
 
 #include <linux/types.h>
 
+/*
+ * This file contains both macros and data structures defined by the TDX
+ * architecture and Linux defined software data structures and functions.
+ * The two should not be mixed together for better readability.  The
+ * architectural definitions come first.
+ */
+
+/*
+ * TDX SEAMCALL error codes
+ */
+#define TDX_RND_NO_ENTROPY	0x8000020300000000ULL
+
+/*
+ * Do not put any hardware-defined TDX structure representations below
+ * this comment!
+ */
+
 struct tdx_module_output;
 u64 __seamcall(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
 	       struct tdx_module_output *out);