* [PATCH v10 1/6] x86/sgx: Convert sgx_(vepc_)open to __sgx_(vepc_)open
2025-08-01 11:25 [PATCH v10 0/6] Enable automatic SVN updates for SGX enclaves Elena Reshetova
@ 2025-08-01 11:25 ` Elena Reshetova
2025-08-01 16:42 ` Dave Hansen
2025-08-01 11:25 ` [PATCH v10 2/6] x86/sgx: Introduce functions to count the sgx_(vepc_)open() Elena Reshetova
` (4 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Elena Reshetova @ 2025-08-01 11:25 UTC (permalink / raw)
To: dave.hansen
Cc: jarkko, seanjc, kai.huang, mingo, linux-sgx, linux-kernel, x86,
asit.k.mallick, vincent.r.scarlata, chongc, erdemaktas,
vannapurve, bondarn, scott.raynor, Elena Reshetova
In order to introduce the counting of active sgx users on top
of clean functions that allocate vepc structures, covert existing
sgx_(vepc_)open to __sgx_(vepc_)open. Later patch will introduce the
top level wrappers that manage the usage count.
No functional change intended in this patch.
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
---
arch/x86/kernel/cpu/sgx/driver.c | 4 ++--
arch/x86/kernel/cpu/sgx/virt.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c
index 7f8d1e11dbee..9aa48f455c54 100644
--- a/arch/x86/kernel/cpu/sgx/driver.c
+++ b/arch/x86/kernel/cpu/sgx/driver.c
@@ -14,7 +14,7 @@ u64 sgx_attributes_reserved_mask;
u64 sgx_xfrm_reserved_mask = ~0x3;
u32 sgx_misc_reserved_mask;
-static int sgx_open(struct inode *inode, struct file *file)
+static int __sgx_open(struct inode *inode, struct file *file)
{
struct sgx_encl *encl;
int ret;
@@ -126,7 +126,7 @@ static long sgx_compat_ioctl(struct file *filep, unsigned int cmd,
static const struct file_operations sgx_encl_fops = {
.owner = THIS_MODULE,
- .open = sgx_open,
+ .open = __sgx_open,
.release = sgx_release,
.unlocked_ioctl = sgx_ioctl,
#ifdef CONFIG_COMPAT
diff --git a/arch/x86/kernel/cpu/sgx/virt.c b/arch/x86/kernel/cpu/sgx/virt.c
index 7aaa3652e31d..d8fdf7f39215 100644
--- a/arch/x86/kernel/cpu/sgx/virt.c
+++ b/arch/x86/kernel/cpu/sgx/virt.c
@@ -258,7 +258,7 @@ static int sgx_vepc_release(struct inode *inode, struct file *file)
return 0;
}
-static int sgx_vepc_open(struct inode *inode, struct file *file)
+static int __sgx_vepc_open(struct inode *inode, struct file *file)
{
struct sgx_vepc *vepc;
@@ -291,7 +291,7 @@ static long sgx_vepc_ioctl(struct file *file,
static const struct file_operations sgx_vepc_fops = {
.owner = THIS_MODULE,
- .open = sgx_vepc_open,
+ .open = __sgx_vepc_open,
.unlocked_ioctl = sgx_vepc_ioctl,
.compat_ioctl = sgx_vepc_ioctl,
.release = sgx_vepc_release,
--
2.45.2
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v10 1/6] x86/sgx: Convert sgx_(vepc_)open to __sgx_(vepc_)open
2025-08-01 11:25 ` [PATCH v10 1/6] x86/sgx: Convert sgx_(vepc_)open to __sgx_(vepc_)open Elena Reshetova
@ 2025-08-01 16:42 ` Dave Hansen
2025-08-04 7:16 ` Reshetova, Elena
0 siblings, 1 reply; 18+ messages in thread
From: Dave Hansen @ 2025-08-01 16:42 UTC (permalink / raw)
To: Elena Reshetova
Cc: jarkko, seanjc, kai.huang, mingo, linux-sgx, linux-kernel, x86,
asit.k.mallick, vincent.r.scarlata, chongc, erdemaktas,
vannapurve, bondarn, scott.raynor
On 8/1/25 04:25, Elena Reshetova wrote:
> In order to introduce the counting of active sgx users on top
> of clean functions that allocate vepc structures, covert existing
> sgx_(vepc_)open to __sgx_(vepc_)open. Later patch will introduce the
> top level wrappers that manage the usage count.
>
> No functional change intended in this patch.
I'm not following at all why this patch is needed. It looks like pure
unneeded churn to me.
^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH v10 1/6] x86/sgx: Convert sgx_(vepc_)open to __sgx_(vepc_)open
2025-08-01 16:42 ` Dave Hansen
@ 2025-08-04 7:16 ` Reshetova, Elena
0 siblings, 0 replies; 18+ messages in thread
From: Reshetova, Elena @ 2025-08-04 7:16 UTC (permalink / raw)
To: Hansen, Dave
Cc: jarkko, seanjc, Huang, Kai, mingo, linux-sgx, linux-kernel, x86,
Mallick, Asit K, Scarlata, Vincent R, Cai, Chong, Aktas, Erdem,
Annapurve, Vishal, Bondarevska, Nataliia, Raynor, Scott
> On 8/1/25 04:25, Elena Reshetova wrote:
> > In order to introduce the counting of active sgx users on top
> > of clean functions that allocate vepc structures, covert existing
> > sgx_(vepc_)open to __sgx_(vepc_)open. Later patch will introduce the
> > top level wrappers that manage the usage count.
> >
> > No functional change intended in this patch.
>
> I'm not following at all why this patch is needed. It looks like pure
> unneeded churn to me.
I am happy to merge this into patch 2 if there are no objections.
Jarkko originally wanted these two patches separated for clarity, so
I followed his suggestion.
Best Regards,
Elena.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v10 2/6] x86/sgx: Introduce functions to count the sgx_(vepc_)open()
2025-08-01 11:25 [PATCH v10 0/6] Enable automatic SVN updates for SGX enclaves Elena Reshetova
2025-08-01 11:25 ` [PATCH v10 1/6] x86/sgx: Convert sgx_(vepc_)open to __sgx_(vepc_)open Elena Reshetova
@ 2025-08-01 11:25 ` Elena Reshetova
2025-08-01 16:45 ` Dave Hansen
2025-08-01 11:25 ` [PATCH v10 3/6] x86/cpufeatures: Add X86_FEATURE_SGX_EUPDATESVN feature flag Elena Reshetova
` (3 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Elena Reshetova @ 2025-08-01 11:25 UTC (permalink / raw)
To: dave.hansen
Cc: jarkko, seanjc, kai.huang, mingo, linux-sgx, linux-kernel, x86,
asit.k.mallick, vincent.r.scarlata, chongc, erdemaktas,
vannapurve, bondarn, scott.raynor, Elena Reshetova
Currently SGX does not have a global counter to count the
active users from userspace or hypervisor. Define placeholder
functions sgx_inc/dec_usage_count that are used to increment
and decrement such a counter. Also, wire the call sites for
these functions.
The definition of the counter itself and the actual implementation
of these two functions comes next. The counter will be used by
the driver that would be attempting to call EUPDATESVN SGX instruction
only when incrementing from zero.
Note: the sgx_inc_usage_count prototype is defined to return
int for the cleanliness of the follow-up patches. When the
EUPDATESVN SGX instruction will be enabled in the follow-up patch,
the sgx_inc_usage_count will start to return int.
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
---
arch/x86/kernel/cpu/sgx/driver.c | 19 ++++++++++++++++++-
arch/x86/kernel/cpu/sgx/encl.c | 1 +
arch/x86/kernel/cpu/sgx/main.c | 10 ++++++++++
arch/x86/kernel/cpu/sgx/sgx.h | 3 +++
arch/x86/kernel/cpu/sgx/virt.c | 20 +++++++++++++++++++-
5 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c
index 9aa48f455c54..79d6020dfe9c 100644
--- a/arch/x86/kernel/cpu/sgx/driver.c
+++ b/arch/x86/kernel/cpu/sgx/driver.c
@@ -41,6 +41,23 @@ static int __sgx_open(struct inode *inode, struct file *file)
return 0;
}
+static int sgx_open(struct inode *inode, struct file *file)
+{
+ int ret;
+
+ ret = sgx_inc_usage_count();
+ if (ret)
+ return ret;
+
+ ret = __sgx_open(inode, file);
+ if (ret) {
+ sgx_dec_usage_count();
+ return ret;
+ }
+
+ return 0;
+}
+
static int sgx_release(struct inode *inode, struct file *file)
{
struct sgx_encl *encl = file->private_data;
@@ -126,7 +143,7 @@ static long sgx_compat_ioctl(struct file *filep, unsigned int cmd,
static const struct file_operations sgx_encl_fops = {
.owner = THIS_MODULE,
- .open = __sgx_open,
+ .open = sgx_open,
.release = sgx_release,
.unlocked_ioctl = sgx_ioctl,
#ifdef CONFIG_COMPAT
diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
index 279148e72459..3b54889ae4a4 100644
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -765,6 +765,7 @@ void sgx_encl_release(struct kref *ref)
WARN_ON_ONCE(encl->secs.epc_page);
kfree(encl);
+ sgx_dec_usage_count();
}
/*
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 2de01b379aa3..3a5cbd1c170e 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -917,6 +917,16 @@ int sgx_set_attribute(unsigned long *allowed_attributes,
}
EXPORT_SYMBOL_GPL(sgx_set_attribute);
+int sgx_inc_usage_count(void)
+{
+ return 0;
+}
+
+void sgx_dec_usage_count(void)
+{
+ return;
+}
+
static int __init sgx_init(void)
{
int ret;
diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h
index d2dad21259a8..f5940393d9bd 100644
--- a/arch/x86/kernel/cpu/sgx/sgx.h
+++ b/arch/x86/kernel/cpu/sgx/sgx.h
@@ -102,6 +102,9 @@ static inline int __init sgx_vepc_init(void)
}
#endif
+int sgx_inc_usage_count(void);
+void sgx_dec_usage_count(void);
+
void sgx_update_lepubkeyhash(u64 *lepubkeyhash);
#endif /* _X86_SGX_H */
diff --git a/arch/x86/kernel/cpu/sgx/virt.c b/arch/x86/kernel/cpu/sgx/virt.c
index d8fdf7f39215..b649c0610019 100644
--- a/arch/x86/kernel/cpu/sgx/virt.c
+++ b/arch/x86/kernel/cpu/sgx/virt.c
@@ -255,6 +255,7 @@ static int sgx_vepc_release(struct inode *inode, struct file *file)
xa_destroy(&vepc->page_array);
kfree(vepc);
+ sgx_dec_usage_count();
return 0;
}
@@ -273,6 +274,23 @@ static int __sgx_vepc_open(struct inode *inode, struct file *file)
return 0;
}
+static int sgx_vepc_open(struct inode *inode, struct file *file)
+{
+ int ret;
+
+ ret = sgx_inc_usage_count();
+ if (ret)
+ return ret;
+
+ ret = __sgx_vepc_open(inode, file);
+ if (ret) {
+ sgx_dec_usage_count();
+ return ret;
+ }
+
+ return 0;
+}
+
static long sgx_vepc_ioctl(struct file *file,
unsigned int cmd, unsigned long arg)
{
@@ -291,7 +309,7 @@ static long sgx_vepc_ioctl(struct file *file,
static const struct file_operations sgx_vepc_fops = {
.owner = THIS_MODULE,
- .open = __sgx_vepc_open,
+ .open = sgx_vepc_open,
.unlocked_ioctl = sgx_vepc_ioctl,
.compat_ioctl = sgx_vepc_ioctl,
.release = sgx_vepc_release,
--
2.45.2
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v10 2/6] x86/sgx: Introduce functions to count the sgx_(vepc_)open()
2025-08-01 11:25 ` [PATCH v10 2/6] x86/sgx: Introduce functions to count the sgx_(vepc_)open() Elena Reshetova
@ 2025-08-01 16:45 ` Dave Hansen
2025-08-04 7:18 ` Reshetova, Elena
0 siblings, 1 reply; 18+ messages in thread
From: Dave Hansen @ 2025-08-01 16:45 UTC (permalink / raw)
To: Elena Reshetova
Cc: jarkko, seanjc, kai.huang, mingo, linux-sgx, linux-kernel, x86,
asit.k.mallick, vincent.r.scarlata, chongc, erdemaktas,
vannapurve, bondarn, scott.raynor
On 8/1/25 04:25, Elena Reshetova wrote:
> Note: the sgx_inc_usage_count prototype is defined to return
> int for the cleanliness of the follow-up patches. When the
> EUPDATESVN SGX instruction will be enabled in the follow-up patch,
> the sgx_inc_usage_count will start to return int.
Please use parenthesis for function_names().
Second, sgx_inc_usage_count() _already_ returns 'int'. Now. In this patch.
So I'm confused what this is trying to say.
Is this trying to say that sgx_inc_usage_count() always returns success
(0) for now, but the future implementation can fail? *That's* why it
needs to have an 'int' return type.
^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH v10 2/6] x86/sgx: Introduce functions to count the sgx_(vepc_)open()
2025-08-01 16:45 ` Dave Hansen
@ 2025-08-04 7:18 ` Reshetova, Elena
0 siblings, 0 replies; 18+ messages in thread
From: Reshetova, Elena @ 2025-08-04 7:18 UTC (permalink / raw)
To: Hansen, Dave
Cc: jarkko, seanjc, Huang, Kai, mingo, linux-sgx, linux-kernel, x86,
Mallick, Asit K, Scarlata, Vincent R, Cai, Chong, Aktas, Erdem,
Annapurve, Vishal, Bondarevska, Nataliia, Raynor, Scott
> -----Original Message-----
> From: Hansen, Dave <dave.hansen@intel.com>
> Sent: Friday, August 1, 2025 7:45 PM
> To: Reshetova, Elena <elena.reshetova@intel.com>
> Cc: jarkko@kernel.org; seanjc@google.com; Huang, Kai
> <kai.huang@intel.com>; mingo@kernel.org; linux-sgx@vger.kernel.org; linux-
> kernel@vger.kernel.org; x86@kernel.org; Mallick, Asit K
> <asit.k.mallick@intel.com>; Scarlata, Vincent R <vincent.r.scarlata@intel.com>;
> Cai, Chong <chongc@google.com>; Aktas, Erdem <erdemaktas@google.com>;
> Annapurve, Vishal <vannapurve@google.com>; Bondarevska, Nataliia
> <bondarn@google.com>; Raynor, Scott <scott.raynor@intel.com>
> Subject: Re: [PATCH v10 2/6] x86/sgx: Introduce functions to count the
> sgx_(vepc_)open()
>
> On 8/1/25 04:25, Elena Reshetova wrote:
> > Note: the sgx_inc_usage_count prototype is defined to return
> > int for the cleanliness of the follow-up patches. When the
> > EUPDATESVN SGX instruction will be enabled in the follow-up patch,
> > the sgx_inc_usage_count will start to return int.
>
> Please use parenthesis for function_names().
Sure, will fix.
>
> Second, sgx_inc_usage_count() _already_ returns 'int'. Now. In this patch.
>
> So I'm confused what this is trying to say.
>
> Is this trying to say that sgx_inc_usage_count() always returns success
> (0) for now, but the future implementation can fail? *That's* why it
> needs to have an 'int' return type.
Yes, I will rephrase this better.
Best Regards,
Elena.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v10 3/6] x86/cpufeatures: Add X86_FEATURE_SGX_EUPDATESVN feature flag
2025-08-01 11:25 [PATCH v10 0/6] Enable automatic SVN updates for SGX enclaves Elena Reshetova
2025-08-01 11:25 ` [PATCH v10 1/6] x86/sgx: Convert sgx_(vepc_)open to __sgx_(vepc_)open Elena Reshetova
2025-08-01 11:25 ` [PATCH v10 2/6] x86/sgx: Introduce functions to count the sgx_(vepc_)open() Elena Reshetova
@ 2025-08-01 11:25 ` Elena Reshetova
2025-08-01 16:46 ` Dave Hansen
2025-08-01 11:25 ` [PATCH v10 4/6] x86/sgx: Define error codes for use by ENCLS[EUPDATESVN] Elena Reshetova
` (2 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Elena Reshetova @ 2025-08-01 11:25 UTC (permalink / raw)
To: dave.hansen
Cc: jarkko, seanjc, kai.huang, mingo, linux-sgx, linux-kernel, x86,
asit.k.mallick, vincent.r.scarlata, chongc, erdemaktas,
vannapurve, bondarn, scott.raynor, Elena Reshetova
Add a flag indicating whenever ENCLS[EUPDATESVN] SGX instruction
is supported. This will be used by SGX driver to perform CPU
SVN updates.
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
---
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/kernel/cpu/cpuid-deps.c | 1 +
arch/x86/kernel/cpu/scattered.c | 1 +
tools/arch/x86/include/asm/cpufeatures.h | 1 +
4 files changed, 4 insertions(+)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 602957dd2609..830d24ff1ada 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -494,6 +494,7 @@
#define X86_FEATURE_TSA_SQ_NO (21*32+11) /* AMD CPU not vulnerable to TSA-SQ */
#define X86_FEATURE_TSA_L1_NO (21*32+12) /* AMD CPU not vulnerable to TSA-L1 */
#define X86_FEATURE_CLEAR_CPU_BUF_VM (21*32+13) /* Clear CPU buffers using VERW before VMRUN */
+#define X86_FEATURE_SGX_EUPDATESVN (21*32+14) /* Support for ENCLS[EUPDATESVN] instruction */
/*
* BUG word(s)
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index 46efcbd6afa4..3d9f49ad0efd 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -79,6 +79,7 @@ static const struct cpuid_dep cpuid_deps[] = {
{ X86_FEATURE_SGX_LC, X86_FEATURE_SGX },
{ X86_FEATURE_SGX1, X86_FEATURE_SGX },
{ X86_FEATURE_SGX2, X86_FEATURE_SGX1 },
+ { X86_FEATURE_SGX_EUPDATESVN, X86_FEATURE_SGX1 },
{ X86_FEATURE_SGX_EDECCSSA, X86_FEATURE_SGX1 },
{ X86_FEATURE_XFD, X86_FEATURE_XSAVES },
{ X86_FEATURE_XFD, X86_FEATURE_XGETBV1 },
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index b4a1f6732a3a..d13444d11ba0 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -42,6 +42,7 @@ static const struct cpuid_bit cpuid_bits[] = {
{ X86_FEATURE_PER_THREAD_MBA, CPUID_ECX, 0, 0x00000010, 3 },
{ X86_FEATURE_SGX1, CPUID_EAX, 0, 0x00000012, 0 },
{ X86_FEATURE_SGX2, CPUID_EAX, 1, 0x00000012, 0 },
+ { X86_FEATURE_SGX_EUPDATESVN, CPUID_EAX, 10, 0x00000012, 0 },
{ X86_FEATURE_SGX_EDECCSSA, CPUID_EAX, 11, 0x00000012, 0 },
{ X86_FEATURE_HW_PSTATE, CPUID_EDX, 7, 0x80000007, 0 },
{ X86_FEATURE_CPB, CPUID_EDX, 9, 0x80000007, 0 },
diff --git a/tools/arch/x86/include/asm/cpufeatures.h b/tools/arch/x86/include/asm/cpufeatures.h
index ee176236c2be..78c3894c17c1 100644
--- a/tools/arch/x86/include/asm/cpufeatures.h
+++ b/tools/arch/x86/include/asm/cpufeatures.h
@@ -487,6 +487,7 @@
#define X86_FEATURE_PREFER_YMM (21*32+ 8) /* Avoid ZMM registers due to downclocking */
#define X86_FEATURE_APX (21*32+ 9) /* Advanced Performance Extensions */
#define X86_FEATURE_INDIRECT_THUNK_ITS (21*32+10) /* Use thunk for indirect branches in lower half of cacheline */
+#define X86_FEATURE_SGX_EUPDATESVN (21*32+14) /* Support for ENCLS[EUPDATESVN] instruction */
/*
* BUG word(s)
--
2.45.2
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v10 3/6] x86/cpufeatures: Add X86_FEATURE_SGX_EUPDATESVN feature flag
2025-08-01 11:25 ` [PATCH v10 3/6] x86/cpufeatures: Add X86_FEATURE_SGX_EUPDATESVN feature flag Elena Reshetova
@ 2025-08-01 16:46 ` Dave Hansen
0 siblings, 0 replies; 18+ messages in thread
From: Dave Hansen @ 2025-08-01 16:46 UTC (permalink / raw)
To: Elena Reshetova
Cc: jarkko, seanjc, kai.huang, mingo, linux-sgx, linux-kernel, x86,
asit.k.mallick, vincent.r.scarlata, chongc, erdemaktas,
vannapurve, bondarn, scott.raynor
On 8/1/25 04:25, Elena Reshetova wrote:
> Add a flag indicating whenever ENCLS[EUPDATESVN] SGX instruction
> is supported. This will be used by SGX driver to perform CPU
> SVN updates.
>
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v10 4/6] x86/sgx: Define error codes for use by ENCLS[EUPDATESVN]
2025-08-01 11:25 [PATCH v10 0/6] Enable automatic SVN updates for SGX enclaves Elena Reshetova
` (2 preceding siblings ...)
2025-08-01 11:25 ` [PATCH v10 3/6] x86/cpufeatures: Add X86_FEATURE_SGX_EUPDATESVN feature flag Elena Reshetova
@ 2025-08-01 11:25 ` Elena Reshetova
2025-08-01 16:57 ` Dave Hansen
2025-08-01 11:25 ` [PATCH v10 5/6] x86/sgx: Implement ENCLS[EUPDATESVN] Elena Reshetova
2025-08-01 11:25 ` [PATCH v10 6/6] x86/sgx: Enable automatic SVN updates for SGX enclaves Elena Reshetova
5 siblings, 1 reply; 18+ messages in thread
From: Elena Reshetova @ 2025-08-01 11:25 UTC (permalink / raw)
To: dave.hansen
Cc: jarkko, seanjc, kai.huang, mingo, linux-sgx, linux-kernel, x86,
asit.k.mallick, vincent.r.scarlata, chongc, erdemaktas,
vannapurve, bondarn, scott.raynor, Elena Reshetova
Add error codes for ENCLS[EUPDATESVN], then SGX CPUSVN update
process can know the execution state of EUPDATESVN and notify
userspace.
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
---
arch/x86/include/asm/sgx.h | 37 ++++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
index 6a0069761508..1abf1461fab6 100644
--- a/arch/x86/include/asm/sgx.h
+++ b/arch/x86/include/asm/sgx.h
@@ -28,21 +28,22 @@
#define SGX_CPUID_EPC_MASK GENMASK(3, 0)
enum sgx_encls_function {
- ECREATE = 0x00,
- EADD = 0x01,
- EINIT = 0x02,
- EREMOVE = 0x03,
- EDGBRD = 0x04,
- EDGBWR = 0x05,
- EEXTEND = 0x06,
- ELDU = 0x08,
- EBLOCK = 0x09,
- EPA = 0x0A,
- EWB = 0x0B,
- ETRACK = 0x0C,
- EAUG = 0x0D,
- EMODPR = 0x0E,
- EMODT = 0x0F,
+ ECREATE = 0x00,
+ EADD = 0x01,
+ EINIT = 0x02,
+ EREMOVE = 0x03,
+ EDGBRD = 0x04,
+ EDGBWR = 0x05,
+ EEXTEND = 0x06,
+ ELDU = 0x08,
+ EBLOCK = 0x09,
+ EPA = 0x0A,
+ EWB = 0x0B,
+ ETRACK = 0x0C,
+ EAUG = 0x0D,
+ EMODPR = 0x0E,
+ EMODT = 0x0F,
+ EUPDATESVN = 0x18,
};
/**
@@ -73,6 +74,10 @@ enum sgx_encls_function {
* public key does not match IA32_SGXLEPUBKEYHASH.
* %SGX_PAGE_NOT_MODIFIABLE: The EPC page cannot be modified because it
* is in the PENDING or MODIFIED state.
+ * %SGX_INSUFFICIENT_ENTROPY: Insufficient entropy in RNG.
+ * %SGX_NO_UPDATE: EUPDATESVN was successful, but CPUSVN was not
+ * updated because current SVN was not newer than
+ * CPUSVN.
* %SGX_UNMASKED_EVENT: An unmasked event, e.g. INTR, was received
*/
enum sgx_return_code {
@@ -81,6 +86,8 @@ enum sgx_return_code {
SGX_CHILD_PRESENT = 13,
SGX_INVALID_EINITTOKEN = 16,
SGX_PAGE_NOT_MODIFIABLE = 20,
+ SGX_INSUFFICIENT_ENTROPY = 29,
+ SGX_NO_UPDATE = 31,
SGX_UNMASKED_EVENT = 128,
};
--
2.45.2
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v10 4/6] x86/sgx: Define error codes for use by ENCLS[EUPDATESVN]
2025-08-01 11:25 ` [PATCH v10 4/6] x86/sgx: Define error codes for use by ENCLS[EUPDATESVN] Elena Reshetova
@ 2025-08-01 16:57 ` Dave Hansen
2025-08-04 7:21 ` Reshetova, Elena
0 siblings, 1 reply; 18+ messages in thread
From: Dave Hansen @ 2025-08-01 16:57 UTC (permalink / raw)
To: Elena Reshetova
Cc: jarkko, seanjc, kai.huang, mingo, linux-sgx, linux-kernel, x86,
asit.k.mallick, vincent.r.scarlata, chongc, erdemaktas,
vannapurve, bondarn, scott.raynor
On 8/1/25 04:25, Elena Reshetova wrote:
> Add error codes for ENCLS[EUPDATESVN], then SGX CPUSVN update
> process can know the execution state of EUPDATESVN and notify
> userspace.
>
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> ---
> arch/x86/include/asm/sgx.h | 37 ++++++++++++++++++++++---------------
> 1 file changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
> index 6a0069761508..1abf1461fab6 100644
> --- a/arch/x86/include/asm/sgx.h
> +++ b/arch/x86/include/asm/sgx.h
> @@ -28,21 +28,22 @@
> #define SGX_CPUID_EPC_MASK GENMASK(3, 0)
>
> enum sgx_encls_function {
> - ECREATE = 0x00,
> - EADD = 0x01,
> - EINIT = 0x02,
> - EREMOVE = 0x03,
> - EDGBRD = 0x04,
> - EDGBWR = 0x05,
> - EEXTEND = 0x06,
> - ELDU = 0x08,
> - EBLOCK = 0x09,
> - EPA = 0x0A,
> - EWB = 0x0B,
> - ETRACK = 0x0C,
> - EAUG = 0x0D,
> - EMODPR = 0x0E,
> - EMODT = 0x0F,
> + ECREATE = 0x00,
> + EADD = 0x01,
> + EINIT = 0x02,
> + EREMOVE = 0x03,
> + EDGBRD = 0x04,
> + EDGBWR = 0x05,
> + EEXTEND = 0x06,
> + ELDU = 0x08,
> + EBLOCK = 0x09,
> + EPA = 0x0A,
> + EWB = 0x0B,
> + ETRACK = 0x0C,
> + EAUG = 0x0D,
> + EMODPR = 0x0E,
> + EMODT = 0x0F,
> + EUPDATESVN = 0x18,
> };
This update is not consistent with the changelog nor the patch subject.
> /**
> @@ -73,6 +74,10 @@ enum sgx_encls_function {
> * public key does not match IA32_SGXLEPUBKEYHASH.
> * %SGX_PAGE_NOT_MODIFIABLE: The EPC page cannot be modified because it
> * is in the PENDING or MODIFIED state.
> + * %SGX_INSUFFICIENT_ENTROPY: Insufficient entropy in RNG.
> + * %SGX_NO_UPDATE: EUPDATESVN was successful, but CPUSVN was not
> + * updated because current SVN was not newer than
> + * CPUSVN.
This comment bothers me. This is an *ERROR* code. It means that
EUPDATESVN was *NOT* successful. It failed. It didn't do an update.
Now, it's not a _bad_ error code. It's kinda like read() returning 0.
It's a "no harm no foul" kind of thing. But it's *NOT* success.
Ideally, we find a way to relay this in a very succinct way.
^ permalink raw reply [flat|nested] 18+ messages in thread* RE: [PATCH v10 4/6] x86/sgx: Define error codes for use by ENCLS[EUPDATESVN]
2025-08-01 16:57 ` Dave Hansen
@ 2025-08-04 7:21 ` Reshetova, Elena
2025-08-04 14:19 ` Dave Hansen
0 siblings, 1 reply; 18+ messages in thread
From: Reshetova, Elena @ 2025-08-04 7:21 UTC (permalink / raw)
To: Hansen, Dave
Cc: jarkko, seanjc, Huang, Kai, mingo, linux-sgx, linux-kernel, x86,
Mallick, Asit K, Scarlata, Vincent R, Cai, Chong, Aktas, Erdem,
Annapurve, Vishal, Bondarevska, Nataliia, Raynor, Scott
> -----Original Message-----
> From: Hansen, Dave <dave.hansen@intel.com>
> Sent: Friday, August 1, 2025 7:57 PM
> To: Reshetova, Elena <elena.reshetova@intel.com>
> Cc: jarkko@kernel.org; seanjc@google.com; Huang, Kai
> <kai.huang@intel.com>; mingo@kernel.org; linux-sgx@vger.kernel.org; linux-
> kernel@vger.kernel.org; x86@kernel.org; Mallick, Asit K
> <asit.k.mallick@intel.com>; Scarlata, Vincent R <vincent.r.scarlata@intel.com>;
> Cai, Chong <chongc@google.com>; Aktas, Erdem <erdemaktas@google.com>;
> Annapurve, Vishal <vannapurve@google.com>; Bondarevska, Nataliia
> <bondarn@google.com>; Raynor, Scott <scott.raynor@intel.com>
> Subject: Re: [PATCH v10 4/6] x86/sgx: Define error codes for use by
> ENCLS[EUPDATESVN]
>
> On 8/1/25 04:25, Elena Reshetova wrote:
> > Add error codes for ENCLS[EUPDATESVN], then SGX CPUSVN update
> > process can know the execution state of EUPDATESVN and notify
> > userspace.
> >
> > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> > ---
> > arch/x86/include/asm/sgx.h | 37 ++++++++++++++++++++++---------------
> > 1 file changed, 22 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
> > index 6a0069761508..1abf1461fab6 100644
> > --- a/arch/x86/include/asm/sgx.h
> > +++ b/arch/x86/include/asm/sgx.h
> > @@ -28,21 +28,22 @@
> > #define SGX_CPUID_EPC_MASK GENMASK(3, 0)
> >
> > enum sgx_encls_function {
> > - ECREATE = 0x00,
> > - EADD = 0x01,
> > - EINIT = 0x02,
> > - EREMOVE = 0x03,
> > - EDGBRD = 0x04,
> > - EDGBWR = 0x05,
> > - EEXTEND = 0x06,
> > - ELDU = 0x08,
> > - EBLOCK = 0x09,
> > - EPA = 0x0A,
> > - EWB = 0x0B,
> > - ETRACK = 0x0C,
> > - EAUG = 0x0D,
> > - EMODPR = 0x0E,
> > - EMODT = 0x0F,
> > + ECREATE = 0x00,
> > + EADD = 0x01,
> > + EINIT = 0x02,
> > + EREMOVE = 0x03,
> > + EDGBRD = 0x04,
> > + EDGBWR = 0x05,
> > + EEXTEND = 0x06,
> > + ELDU = 0x08,
> > + EBLOCK = 0x09,
> > + EPA = 0x0A,
> > + EWB = 0x0B,
> > + ETRACK = 0x0C,
> > + EAUG = 0x0D,
> > + EMODPR = 0x0E,
> > + EMODT = 0x0F,
> > + EUPDATESVN = 0x18,
> > };
>
> This update is not consistent with the changelog nor the patch subject.
I can remove the alignment fix.
>
> > /**
> > @@ -73,6 +74,10 @@ enum sgx_encls_function {
> > * public key does not match
> IA32_SGXLEPUBKEYHASH.
> > * %SGX_PAGE_NOT_MODIFIABLE: The EPC page cannot be modified
> because it
> > * is in the PENDING or MODIFIED state.
> > + * %SGX_INSUFFICIENT_ENTROPY: Insufficient entropy in RNG.
> > + * %SGX_NO_UPDATE: EUPDATESVN was successful, but
> CPUSVN was not
> > + * updated because current SVN was not newer
> than
> > + * CPUSVN.
>
> This comment bothers me. This is an *ERROR* code. It means that
> EUPDATESVN was *NOT* successful. It failed. It didn't do an update.
>
> Now, it's not a _bad_ error code. It's kinda like read() returning 0.
> It's a "no harm no foul" kind of thing. But it's *NOT* success.
Yes, agree on both.
>
> Ideally, we find a way to relay this in a very succinct way.
Could you please elaborate what you mean by this?
Changing the description? The name or?
Best Regards,
Elena.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v10 4/6] x86/sgx: Define error codes for use by ENCLS[EUPDATESVN]
2025-08-04 7:21 ` Reshetova, Elena
@ 2025-08-04 14:19 ` Dave Hansen
2025-08-05 10:11 ` Reshetova, Elena
0 siblings, 1 reply; 18+ messages in thread
From: Dave Hansen @ 2025-08-04 14:19 UTC (permalink / raw)
To: Reshetova, Elena
Cc: jarkko, seanjc, Huang, Kai, mingo, linux-sgx, linux-kernel, x86,
Mallick, Asit K, Scarlata, Vincent R, Cai, Chong, Aktas, Erdem,
Annapurve, Vishal, Bondarevska, Nataliia, Raynor, Scott
On 8/4/25 00:21, Reshetova, Elena wrote:
...
>>> + EUPDATESVN = 0x18,
>>> };
>>
>> This update is not consistent with the changelog nor the patch subject.
>
> I can remove the alignment fix.
It's not the alignment.
It's the definition of EUPDATESVN that goes completely unmentioned.
...
>> Ideally, we find a way to relay this in a very succinct way.
>
> Could you please elaborate what you mean by this?
> Changing the description? The name or?
Relaying it in the comment next to the definition would be best.
^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH v10 4/6] x86/sgx: Define error codes for use by ENCLS[EUPDATESVN]
2025-08-04 14:19 ` Dave Hansen
@ 2025-08-05 10:11 ` Reshetova, Elena
0 siblings, 0 replies; 18+ messages in thread
From: Reshetova, Elena @ 2025-08-05 10:11 UTC (permalink / raw)
To: Hansen, Dave
Cc: jarkko, seanjc, Huang, Kai, mingo, linux-sgx, linux-kernel, x86,
Mallick, Asit K, Scarlata, Vincent R, Cai, Chong, Aktas, Erdem,
Annapurve, Vishal, Bondarevska, Nataliia, Raynor, Scott
> -----Original Message-----
> From: Hansen, Dave <dave.hansen@intel.com>
> Sent: Monday, August 4, 2025 5:20 PM
> To: Reshetova, Elena <elena.reshetova@intel.com>
> Cc: jarkko@kernel.org; seanjc@google.com; Huang, Kai
> <kai.huang@intel.com>; mingo@kernel.org; linux-sgx@vger.kernel.org; linux-
> kernel@vger.kernel.org; x86@kernel.org; Mallick, Asit K
> <asit.k.mallick@intel.com>; Scarlata, Vincent R <vincent.r.scarlata@intel.com>;
> Cai, Chong <chongc@google.com>; Aktas, Erdem <erdemaktas@google.com>;
> Annapurve, Vishal <vannapurve@google.com>; Bondarevska, Nataliia
> <bondarn@google.com>; Raynor, Scott <scott.raynor@intel.com>
> Subject: Re: [PATCH v10 4/6] x86/sgx: Define error codes for use by
> ENCLS[EUPDATESVN]
>
> On 8/4/25 00:21, Reshetova, Elena wrote:
> ...
> >>> + EUPDATESVN = 0x18,
> >>> };
> >>
> >> This update is not consistent with the changelog nor the patch subject.
> >
> > I can remove the alignment fix.
>
> It's not the alignment.
>
> It's the definition of EUPDATESVN that goes completely unmentioned.
Yes, good catch, this should be moved to the next patch indeed.
>
> ...
> >> Ideally, we find a way to relay this in a very succinct way.
> >
> > Could you please elaborate what you mean by this?
> > Changing the description? The name or?
>
> Relaying it in the comment next to the definition would be best.
Sure, will try to extend the current description to convey it better.
Best Regards,
Elena.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v10 5/6] x86/sgx: Implement ENCLS[EUPDATESVN]
2025-08-01 11:25 [PATCH v10 0/6] Enable automatic SVN updates for SGX enclaves Elena Reshetova
` (3 preceding siblings ...)
2025-08-01 11:25 ` [PATCH v10 4/6] x86/sgx: Define error codes for use by ENCLS[EUPDATESVN] Elena Reshetova
@ 2025-08-01 11:25 ` Elena Reshetova
2025-08-01 17:12 ` Dave Hansen
2025-08-01 11:25 ` [PATCH v10 6/6] x86/sgx: Enable automatic SVN updates for SGX enclaves Elena Reshetova
5 siblings, 1 reply; 18+ messages in thread
From: Elena Reshetova @ 2025-08-01 11:25 UTC (permalink / raw)
To: dave.hansen
Cc: jarkko, seanjc, kai.huang, mingo, linux-sgx, linux-kernel, x86,
asit.k.mallick, vincent.r.scarlata, chongc, erdemaktas,
vannapurve, bondarn, scott.raynor, Elena Reshetova
All running enclaves and cryptographic assets (such as internal SGX
encryption keys) are assumed to be compromised whenever an SGX-related
microcode update occurs. To mitigate this assumed compromise the new
supervisor SGX instruction ENCLS[EUPDATESVN] can generate fresh
cryptographic assets.
Before executing EUPDATESVN, all SGX memory must be marked as unused.
This requirement ensures that no potentially compromised enclave
survives the update and allows the system to safely regenerate
cryptographic assets.
Add the method to perform ENCLS[EUPDATESVN].
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
---
arch/x86/kernel/cpu/sgx/encls.h | 5 +++
arch/x86/kernel/cpu/sgx/main.c | 61 +++++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+)
diff --git a/arch/x86/kernel/cpu/sgx/encls.h b/arch/x86/kernel/cpu/sgx/encls.h
index 99004b02e2ed..d9160c89a93d 100644
--- a/arch/x86/kernel/cpu/sgx/encls.h
+++ b/arch/x86/kernel/cpu/sgx/encls.h
@@ -233,4 +233,9 @@ static inline int __eaug(struct sgx_pageinfo *pginfo, void *addr)
return __encls_2(EAUG, pginfo, addr);
}
+/* Attempt to update CPUSVN at runtime. */
+static inline int __eupdatesvn(void)
+{
+ return __encls_ret_1(EUPDATESVN, "");
+}
#endif /* _X86_ENCLS_H */
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 3a5cbd1c170e..5aae0c881963 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -16,6 +16,7 @@
#include <linux/vmalloc.h>
#include <asm/msr.h>
#include <asm/sgx.h>
+#include <asm/archrandom.h>
#include "driver.h"
#include "encl.h"
#include "encls.h"
@@ -917,6 +918,66 @@ int sgx_set_attribute(unsigned long *allowed_attributes,
}
EXPORT_SYMBOL_GPL(sgx_set_attribute);
+/**
+ * sgx_update_svn() - Attempt to call ENCLS[EUPDATESVN].
+ * This instruction attempts to update CPUSVN to the
+ * currently loaded microcode update SVN and generate new
+ * cryptographic assets. Must be called when EPC is empty.
+ * Most of the time, there will be no update and that's OK.
+ * If the failure is due to SGX_INSUFFICIENT_ENTROPY, the
+ * operation can be safely retried. In other failure cases,
+ * the retry should not be attempted.
+ *
+ * Return:
+ * 0: Success or not supported
+ * -EAGAIN: Can be safely retried, failure is due to lack of
+ * entropy in RNG.
+ * -EIO: Unexpected error, retries are not advisable.
+ */
+static int __maybe_unused sgx_update_svn(void)
+{
+ int ret;
+
+ /*
+ * If EUPDATESVN is not available, it is ok to
+ * silently skip it to comply with legacy behavior.
+ */
+ if (!cpu_feature_enabled(X86_FEATURE_SGX_EUPDATESVN))
+ return 0;
+
+ for (int i = 0; i < RDRAND_RETRY_LOOPS; i++) {
+ ret = __eupdatesvn();
+
+ /* Stop on success or unexpected errors: */
+ if (ret != SGX_INSUFFICIENT_ENTROPY)
+ break;
+ }
+
+ /*
+ * SVN successfully updated.
+ * Let users know when the update was successful.
+ */
+ if (!ret)
+ pr_info("SVN updated successfully\n");
+
+ if (!ret || ret == SGX_NO_UPDATE)
+ return 0;
+
+ /*
+ * SVN update failed due to lack of entropy in DRNG.
+ * Indicate to userspace that it should retry.
+ */
+ if (ret == SGX_INSUFFICIENT_ENTROPY)
+ return -EAGAIN;
+
+ /*
+ * EUPDATESVN was called when EPC is empty, all other error
+ * codes are unexpected.
+ */
+ ENCLS_WARN(ret, "EUPDATESVN");
+ return -EIO;
+}
+
int sgx_inc_usage_count(void)
{
return 0;
--
2.45.2
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v10 5/6] x86/sgx: Implement ENCLS[EUPDATESVN]
2025-08-01 11:25 ` [PATCH v10 5/6] x86/sgx: Implement ENCLS[EUPDATESVN] Elena Reshetova
@ 2025-08-01 17:12 ` Dave Hansen
2025-08-04 7:39 ` Reshetova, Elena
0 siblings, 1 reply; 18+ messages in thread
From: Dave Hansen @ 2025-08-01 17:12 UTC (permalink / raw)
To: Elena Reshetova
Cc: jarkko, seanjc, kai.huang, mingo, linux-sgx, linux-kernel, x86,
asit.k.mallick, vincent.r.scarlata, chongc, erdemaktas,
vannapurve, bondarn, scott.raynor
The changelog is missing a tidbit about the fact that this is still dead
code until sgx_inc_usage_count() gets a real implementation.
On 8/1/25 04:25, Elena Reshetova wrote:
...
> +/**
> + * sgx_update_svn() - Attempt to call ENCLS[EUPDATESVN].
> + * This instruction attempts to update CPUSVN to the
> + * currently loaded microcode update SVN and generate new
> + * cryptographic assets. Must be called when EPC is empty.
As a general rule, I'd much rather have the "Must be $FOO" rules written
in code than in a comment, or along with a comment:
/* EPC is guaranteed to be empty when there are no users: */
WARN(count, "Elevated usage count...");
> + * Most of the time, there will be no update and that's OK.
This should go with the check for SGX_NO_UPDATE, not here.
> + * If the failure is due to SGX_INSUFFICIENT_ENTROPY, the
> + * operation can be safely retried. In other failure cases,
> + * the retry should not be attempted.
Ditto. This is rewriting the code in comments.
> + * Return:
> + * 0: Success or not supported
> + * -EAGAIN: Can be safely retried, failure is due to lack of
> + * entropy in RNG.
> + * -EIO: Unexpected error, retries are not advisable.
> + */
> +static int __maybe_unused sgx_update_svn(void)
> +{
> + int ret;
> +
> + /*
> + * If EUPDATESVN is not available, it is ok to
> + * silently skip it to comply with legacy behavior.
> + */
> + if (!cpu_feature_enabled(X86_FEATURE_SGX_EUPDATESVN))
> + return 0;
> +
> + for (int i = 0; i < RDRAND_RETRY_LOOPS; i++) {
> + ret = __eupdatesvn();
> +
> + /* Stop on success or unexpected errors: */
> + if (ret != SGX_INSUFFICIENT_ENTROPY)
> + break;
> + }
> +
> + /*
> + * SVN successfully updated.
> + * Let users know when the update was successful.
> + */
> + if (!ret)
> + pr_info("SVN updated successfully\n");
> +
> + if (!ret || ret == SGX_NO_UPDATE)
> + return 0;
> +
> + /*
> + * SVN update failed due to lack of entropy in DRNG.
> + * Indicate to userspace that it should retry.
> + */
> + if (ret == SGX_INSUFFICIENT_ENTROPY)
> + return -EAGAIN;
There are four cases to handle. Doesn't it make sense to just write it
as four literal "case"s?
switch (ret) {
case 0:
pr_info("...");
return 0;
case SGX_NO_UPDATE:
return 0;
case SGX_INSUFFICIENT_ENTROPY:
return -EAGAIN;
default:
break;
}
> + ENCLS_WARN(ret, "EUPDATESVN");
> + return -EIO;
> +}
> +
> int sgx_inc_usage_count(void)
> {
> return 0;
^ permalink raw reply [flat|nested] 18+ messages in thread* RE: [PATCH v10 5/6] x86/sgx: Implement ENCLS[EUPDATESVN]
2025-08-01 17:12 ` Dave Hansen
@ 2025-08-04 7:39 ` Reshetova, Elena
0 siblings, 0 replies; 18+ messages in thread
From: Reshetova, Elena @ 2025-08-04 7:39 UTC (permalink / raw)
To: Hansen, Dave
Cc: jarkko, seanjc, Huang, Kai, mingo, linux-sgx, linux-kernel, x86,
Mallick, Asit K, Scarlata, Vincent R, Cai, Chong, Aktas, Erdem,
Annapurve, Vishal, Bondarevska, Nataliia, Raynor, Scott
> -----Original Message-----
> From: Hansen, Dave <dave.hansen@intel.com>
> Sent: Friday, August 1, 2025 8:13 PM
> To: Reshetova, Elena <elena.reshetova@intel.com>
> Cc: jarkko@kernel.org; seanjc@google.com; Huang, Kai
> <kai.huang@intel.com>; mingo@kernel.org; linux-sgx@vger.kernel.org; linux-
> kernel@vger.kernel.org; x86@kernel.org; Mallick, Asit K
> <asit.k.mallick@intel.com>; Scarlata, Vincent R <vincent.r.scarlata@intel.com>;
> Cai, Chong <chongc@google.com>; Aktas, Erdem <erdemaktas@google.com>;
> Annapurve, Vishal <vannapurve@google.com>; Bondarevska, Nataliia
> <bondarn@google.com>; Raynor, Scott <scott.raynor@intel.com>
> Subject: Re: [PATCH v10 5/6] x86/sgx: Implement ENCLS[EUPDATESVN]
>
> The changelog is missing a tidbit about the fact that this is still dead
> code until sgx_inc_usage_count() gets a real implementation.
Will add.
>
> On 8/1/25 04:25, Elena Reshetova wrote:
> ...
> > +/**
> > + * sgx_update_svn() - Attempt to call ENCLS[EUPDATESVN].
> > + * This instruction attempts to update CPUSVN to the
> > + * currently loaded microcode update SVN and generate new
> > + * cryptographic assets. Must be called when EPC is empty.
>
> As a general rule, I'd much rather have the "Must be $FOO" rules written
> in code than in a comment, or along with a comment:
>
> /* EPC is guaranteed to be empty when there are no users: */
> WARN(count, "Elevated usage count...");
I will change to do it this way.
>
> > + * Most of the time, there will be no update and that's OK.
>
> This should go with the check for SGX_NO_UPDATE, not here.
Ok, will fix.
>
> > + * If the failure is due to SGX_INSUFFICIENT_ENTROPY, the
> > + * operation can be safely retried. In other failure cases,
> > + * the retry should not be attempted.
>
> Ditto. This is rewriting the code in comments.
Ok, will drop.
>
> > + * Return:
> > + * 0: Success or not supported
> > + * -EAGAIN: Can be safely retried, failure is due to lack of
> > + * entropy in RNG.
> > + * -EIO: Unexpected error, retries are not advisable.
> > + */
> > +static int __maybe_unused sgx_update_svn(void)
> > +{
> > + int ret;
> > +
> > + /*
> > + * If EUPDATESVN is not available, it is ok to
> > + * silently skip it to comply with legacy behavior.
> > + */
> > + if (!cpu_feature_enabled(X86_FEATURE_SGX_EUPDATESVN))
> > + return 0;
> > +
> > + for (int i = 0; i < RDRAND_RETRY_LOOPS; i++) {
> > + ret = __eupdatesvn();
> > +
> > + /* Stop on success or unexpected errors: */
> > + if (ret != SGX_INSUFFICIENT_ENTROPY)
> > + break;
> > + }
> > +
> > + /*
> > + * SVN successfully updated.
> > + * Let users know when the update was successful.
> > + */
> > + if (!ret)
> > + pr_info("SVN updated successfully\n");
> > +
> > + if (!ret || ret == SGX_NO_UPDATE)
> > + return 0;
> > +
> > + /*
> > + * SVN update failed due to lack of entropy in DRNG.
> > + * Indicate to userspace that it should retry.
> > + */
> > + if (ret == SGX_INSUFFICIENT_ENTROPY)
> > + return -EAGAIN;
>
> There are four cases to handle. Doesn't it make sense to just write it
> as four literal "case"s?
>
> switch (ret) {
> case 0:
> pr_info("...");
> return 0;
> case SGX_NO_UPDATE:
> return 0;
> case SGX_INSUFFICIENT_ENTROPY:
> return -EAGAIN;
> default:
> break;
> }
>
>
Will re-write accordingly.
Thank you very much for your prompt review Dave!
Best Regards,
Elena.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v10 6/6] x86/sgx: Enable automatic SVN updates for SGX enclaves
2025-08-01 11:25 [PATCH v10 0/6] Enable automatic SVN updates for SGX enclaves Elena Reshetova
` (4 preceding siblings ...)
2025-08-01 11:25 ` [PATCH v10 5/6] x86/sgx: Implement ENCLS[EUPDATESVN] Elena Reshetova
@ 2025-08-01 11:25 ` Elena Reshetova
5 siblings, 0 replies; 18+ messages in thread
From: Elena Reshetova @ 2025-08-01 11:25 UTC (permalink / raw)
To: dave.hansen
Cc: jarkko, seanjc, kai.huang, mingo, linux-sgx, linux-kernel, x86,
asit.k.mallick, vincent.r.scarlata, chongc, erdemaktas,
vannapurve, bondarn, scott.raynor, Elena Reshetova
== Background ==
ENCLS[EUPDATESVN] is a new SGX instruction [1] which allows enclave
attestation to include information about updated microcode SVN without a
reboot. Before an EUPDATESVN operation can be successful, all SGX memory
(aka. EPC) must be marked as “unused” in the SGX hardware metadata
(aka.EPCM). This requirement ensures that no compromised enclave can
survive the EUPDATESVN procedure and provides an opportunity to generate
new cryptographic assets.
== Solution ==
Attempt to execute ENCLS[EUPDATESVN] every time the first file descriptor
is obtained via sgx_(vepc_)open(). In the most common case the microcode
SVN is already up-to-date, and the operation succeeds without updating SVN.
Note: while in such cases the underlying CR_BASE_KEY is regenrated, it does
not affect enclaves' visible keys obtained via EGETKEY instruction.
If it fails with any other error code than SGX_INSUFFICIENT_ENTROPY, this
is considered unexpected and the *open() returns an error. This should not
happen in practice.
On contrary, SGX_INSUFFICIENT_ENTROPY might happen due
to a pressure on the system's DRNG (RDSEED) and therefore the *open() can
be safely retried to allow normal enclave operation.
[1] Runtime Microcode Updates with Intel Software Guard Extensions,
https://cdrdv2.intel.com/v1/dl/getContent/648682
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
---
arch/x86/kernel/cpu/sgx/main.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 5aae0c881963..6a5d5cd726f6 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -934,7 +934,7 @@ EXPORT_SYMBOL_GPL(sgx_set_attribute);
* entropy in RNG.
* -EIO: Unexpected error, retries are not advisable.
*/
-static int __maybe_unused sgx_update_svn(void)
+static int sgx_update_svn(void)
{
int ret;
@@ -978,14 +978,25 @@ static int __maybe_unused sgx_update_svn(void)
return -EIO;
}
+/* Counter to count the active SGX users */
+static int sgx_usage_count;
+
+/* Mutex to ensure no concurrent EPC accesses during EUPDATESVN */
+static DEFINE_MUTEX(sgx_svn_lock);
+
int sgx_inc_usage_count(void)
{
+ guard(mutex)(&sgx_svn_lock);
+
+ if (sgx_usage_count++ == 0)
+ return sgx_update_svn();
+
return 0;
}
void sgx_dec_usage_count(void)
{
- return;
+ sgx_usage_count--;
}
static int __init sgx_init(void)
--
2.45.2
^ permalink raw reply [flat|nested] 18+ messages in thread