Certain SEAMCALL leaf functions may return error due to running out of
entropy, in which case the SEAMCALL should be retried as suggested by
the TDX spec.
Handle this case in SEAMCALL common function. Mimic the existing
rdrand_long() to retry RDRAND_RETRY_LOOPS times.
Signed-off-by: Kai Huang <[email protected]>
---
v10 -> v11:
- New patch
---
arch/x86/virt/vmx/tdx/tdx.c | 15 ++++++++++++++-
arch/x86/virt/vmx/tdx/tdx.h | 17 +++++++++++++++++
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index e82713dd5d54..e62e978eba1b 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"
@@ -33,12 +34,24 @@ static int __always_unused seamcall(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
struct tdx_module_output *out)
{
int cpu, ret = 0;
+ int retry;
u64 sret;
/* 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 the existing rdrand_long() to retry
+ * RDRAND_RETRY_LOOPS times.
+ */
+ retry = RDRAND_RETRY_LOOPS;
+ do {
+ sret = __seamcall(fn, rcx, rdx, r8, r9, out);
+ } while (sret == TDX_RND_NO_ENTROPY && --retry);
/* Save SEAMCALL return code if the caller wants it */
if (seamcall_ret)
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);
--
2.40.1
On Mon, Jun 05, 2023 at 02:27:19AM +1200,
Kai Huang <[email protected]> wrote:
> Certain SEAMCALL leaf functions may return error due to running out of
> entropy, in which case the SEAMCALL should be retried as suggested by
> the TDX spec.
>
> Handle this case in SEAMCALL common function. Mimic the existing
> rdrand_long() to retry RDRAND_RETRY_LOOPS times.
>
> Signed-off-by: Kai Huang <[email protected]>
> ---
>
> v10 -> v11:
> - New patch
>
> ---
> arch/x86/virt/vmx/tdx/tdx.c | 15 ++++++++++++++-
> arch/x86/virt/vmx/tdx/tdx.h | 17 +++++++++++++++++
> 2 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index e82713dd5d54..e62e978eba1b 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"
>
> @@ -33,12 +34,24 @@ static int __always_unused seamcall(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
> struct tdx_module_output *out)
> {
> int cpu, ret = 0;
> + int retry;
> u64 sret;
>
> /* 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 the existing rdrand_long() to retry
> + * RDRAND_RETRY_LOOPS times.
> + */
> + retry = RDRAND_RETRY_LOOPS;
> + do {
> + sret = __seamcall(fn, rcx, rdx, r8, r9, out);
> + } while (sret == TDX_RND_NO_ENTROPY && --retry);
>
> /* Save SEAMCALL return code if the caller wants it */
> if (seamcall_ret)
> 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);
> --
> 2.40.1
>
Reviewed-by: Isaku Yamahata <[email protected]>
--
Isaku Yamahata <[email protected]>
On 6/4/23 07:27, Kai Huang wrote:
> Certain SEAMCALL leaf functions may return error due to running out of
> entropy, in which case the SEAMCALL should be retried as suggested by
> the TDX spec.
>
> Handle this case in SEAMCALL common function. Mimic the existing
> rdrand_long() to retry RDRAND_RETRY_LOOPS times.
... because who are we kidding? When the TDX module says it doesn't
have enough entropy it means rdrand.
Reviewed-by: Dave Hansen <[email protected]>
On Wed, 2023-06-07 at 08:08 -0700, Hansen, Dave wrote:
> On 6/4/23 07:27, Kai Huang wrote:
> > Certain SEAMCALL leaf functions may return error due to running out of
> > entropy, in which case the SEAMCALL should be retried as suggested by
> > the TDX spec.
> >
> > Handle this case in SEAMCALL common function. Mimic the existing
> > rdrand_long() to retry RDRAND_RETRY_LOOPS times.
>
> ... because who are we kidding? When the TDX module says it doesn't
> have enough entropy it means rdrand.
The TDX spec says "e.g., RDRAND or RDSEED".
Do you prefer below?
Certain SEAMCALL leaf functions may return error due to running out of entropy
(e.g., RDRAND or RDSEED), in which case the SEAMCALL should be retried as
suggested by the TDX spec.
Handle this case in SEAMCALL common function. Based on the SDM there's no big
difference between RDRAND and RDSEED except the latter is "compliant to NIST
SP800-90B and NIST SP800-90C in the XOR construction mode". Just Mimic the
existing rdrand_long() to retry RDRAND_RETRY_LOOPS times.
>
> Reviewed-by: Dave Hansen <[email protected]>
>
>
Thanks!
On Mon, Jun 05, 2023 at 02:27:19AM +1200, Kai Huang wrote:
> Certain SEAMCALL leaf functions may return error due to running out of
> entropy, in which case the SEAMCALL should be retried as suggested by
> the TDX spec.
>
> Handle this case in SEAMCALL common function. Mimic the existing
> rdrand_long() to retry RDRAND_RETRY_LOOPS times.
>
> Signed-off-by: Kai Huang <[email protected]>
Reviewed-by: Kirill A. Shutemov <[email protected]>
--
Kiryl Shutsemau / Kirill A. Shutemov
On 6/7/23 16:36, Huang, Kai wrote:
> On Wed, 2023-06-07 at 08:08 -0700, Hansen, Dave wrote:
>> On 6/4/23 07:27, Kai Huang wrote:
>>> Certain SEAMCALL leaf functions may return error due to running out of
>>> entropy, in which case the SEAMCALL should be retried as suggested by
>>> the TDX spec.
>>>
>>> Handle this case in SEAMCALL common function. Mimic the existing
>>> rdrand_long() to retry RDRAND_RETRY_LOOPS times.
>>
>> ... because who are we kidding? When the TDX module says it doesn't
>> have enough entropy it means rdrand.
>
> The TDX spec says "e.g., RDRAND or RDSEED".
Let's just say something a bit more useful and ambiguous:
Some SEAMCALLs use the RDRAND hardware and can fail for the
same reasons as RDRAND. Use the kernel RDRAND retry logic for
them.
We don't need to say "RDRAND and RDSEED", just saying "RDRAND hardware"
is fine. Everybody knows what you mean.
On 4.06.23 г. 17:27 ч., Kai Huang wrote:
> Certain SEAMCALL leaf functions may return error due to running out of
> entropy, in which case the SEAMCALL should be retried as suggested by
> the TDX spec.
>
> Handle this case in SEAMCALL common function. Mimic the existing
> rdrand_long() to retry RDRAND_RETRY_LOOPS times.
>
> Signed-off-by: Kai Huang <[email protected]>
> ---
>
> v10 -> v11:
> - New patch
>
> ---
> arch/x86/virt/vmx/tdx/tdx.c | 15 ++++++++++++++-
> arch/x86/virt/vmx/tdx/tdx.h | 17 +++++++++++++++++
> 2 files changed, 31 insertions(+), 1 deletion(-)
>
<snip>
> 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
Where is this return value documented, in TDX module 1.0 spec there are
only: 8000020[123]00000000 specified and there's 80000800
(TDX_KEY_GENERATION_FAILED) and its description mentions the possible
failure due to lack of entropy?
<snip>
On Fri, 2023-06-09 at 17:42 +0300, Nikolay Borisov wrote:
>
> On 4.06.23 г. 17:27 ч., Kai Huang wrote:
> > Certain SEAMCALL leaf functions may return error due to running out of
> > entropy, in which case the SEAMCALL should be retried as suggested by
> > the TDX spec.
> >
> > Handle this case in SEAMCALL common function. Mimic the existing
> > rdrand_long() to retry RDRAND_RETRY_LOOPS times.
> >
> > Signed-off-by: Kai Huang <[email protected]>
> > ---
> >
> > v10 -> v11:
> > - New patch
> >
> > ---
> > arch/x86/virt/vmx/tdx/tdx.c | 15 ++++++++++++++-
> > arch/x86/virt/vmx/tdx/tdx.h | 17 +++++++++++++++++
> > 2 files changed, 31 insertions(+), 1 deletion(-)
> >
>
> <snip>
>
> > 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
>
> Where is this return value documented, in TDX module 1.0 spec there are
> only: 8000020[123]00000000 specified and there's 80000800
> (TDX_KEY_GENERATION_FAILED) and its description mentions the possible
> failure due to lack of entropy?
>
It's documented in TDX module V1.5 ABI Specification:
https://cdrdv2.intel.com/v1/dl/getContent/733579
The later versions of TDX module try to use TDX_RND_NO_ENTROPY to cover all
errors due to running out of entropy, but TDX module 1.0 for now doesn't.
This patch aims to resolve this error code in the common code.
On 04.06.23 16:27, Kai Huang wrote:
> Certain SEAMCALL leaf functions may return error due to running out of
> entropy, in which case the SEAMCALL should be retried as suggested by
> the TDX spec.
>
> Handle this case in SEAMCALL common function. Mimic the existing
> rdrand_long() to retry RDRAND_RETRY_LOOPS times.
>
> Signed-off-by: Kai Huang <[email protected]>
> ---
>
> v10 -> v11:
> - New patch
>
> ---
> arch/x86/virt/vmx/tdx/tdx.c | 15 ++++++++++++++-
> arch/x86/virt/vmx/tdx/tdx.h | 17 +++++++++++++++++
> 2 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index e82713dd5d54..e62e978eba1b 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"
>
> @@ -33,12 +34,24 @@ static int __always_unused seamcall(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
> struct tdx_module_output *out)
> {
> int cpu, ret = 0;
> + int retry;
> u64 sret;
>
> /* 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 the existing rdrand_long() to retry
> + * RDRAND_RETRY_LOOPS times.
> + */
> + retry = RDRAND_RETRY_LOOPS;
Nit: I'd just do a "int retry = RDRAND_RETRY_LOOPS" and simplify this
comment to "Mimic rdrand_long() retry behavior."
> + do {
> + sret = __seamcall(fn, rcx, rdx, r8, r9, out);
> + } while (sret == TDX_RND_NO_ENTROPY && --retry);
>
> /* Save SEAMCALL return code if the caller wants it */
> if (seamcall_ret)
> 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);
In general, LGTM
Reviewed-by: David Hildenbrand <[email protected]>
--
Cheers,
David / dhildenb
> > @@ -33,12 +34,24 @@ static int __always_unused seamcall(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
> > struct tdx_module_output *out)
> > {
> > int cpu, ret = 0;
> > + int retry;
> > u64 sret;
> >
> > /* 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 the existing rdrand_long() to retry
> > + * RDRAND_RETRY_LOOPS times.
> > + */
> > + retry = RDRAND_RETRY_LOOPS;
>
> Nit: I'd just do a "int retry = RDRAND_RETRY_LOOPS" and simplify this
> comment to "Mimic rdrand_long() retry behavior."
OK will do.
But I think you are talking about replacing the second paragraph but not the
entire comment?
>
> > + do {
> > + sret = __seamcall(fn, rcx, rdx, r8, r9, out);
> > + } while (sret == TDX_RND_NO_ENTROPY && --retry);
> >
> > /* Save SEAMCALL return code if the caller wants it */
> > if (seamcall_ret)
> > 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);
>
> In general, LGTM
>
> Reviewed-by: David Hildenbrand <[email protected]>
Thanks!
On 20.06.23 12:39, Huang, Kai wrote:
>
>>> @@ -33,12 +34,24 @@ static int __always_unused seamcall(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
>>> struct tdx_module_output *out)
>>> {
>>> int cpu, ret = 0;
>>> + int retry;
>>> u64 sret;
>>>
>>> /* 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 the existing rdrand_long() to retry
>>> + * RDRAND_RETRY_LOOPS times.
>>> + */
>>> + retry = RDRAND_RETRY_LOOPS;
>>
>> Nit: I'd just do a "int retry = RDRAND_RETRY_LOOPS" and simplify this
>> comment to "Mimic rdrand_long() retry behavior."
>
> OK will do.
>
> But I think you are talking about replacing the second paragraph but not the
> entire comment?
>
Yes.
--
Cheers,
David / dhildenb