== 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.

== Patch Contents ==

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.

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 | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 206bf41d8325..90b7416969e4 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -921,6 +921,9 @@ EXPORT_SYMBOL_GPL(sgx_set_attribute);
 /* Counter to count the active SGX users */
 static int __maybe_unused sgx_usage_count;
 
+/* Mutex to ensure no concurrent EPC accesses during EUPDATESVN */
+static DEFINE_MUTEX(sgx_svn_lock);
+
 /**
  * sgx_update_svn() - Attempt to call ENCLS[EUPDATESVN].
  * This instruction attempts to update CPUSVN to the
@@ -937,7 +940,7 @@ static int __maybe_unused sgx_usage_count;
  *  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;
 
@@ -983,12 +986,17 @@ static int __maybe_unused sgx_update_svn(void)
 
 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

On Thu, 2025-07-24 at 11:02 +0300, Reshetova, Elena wrote:
> == 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.
> 
> == Patch Contents ==

Nit: you can use "Solution" instead of "Patch Contents".

> 
> 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.

(Sorry I forgot to say this in the previous versions):

If I read the pseudo code correctly, when the SVN is already up-to-date,
the EUPDATESVN doesn't update SVN but it re-generates crypto assets
anyway.

This is no harm per the pseudo code, since the "crypto assets" is actually
the CR_BASE_KEY which is only used by EWB/ELDU flow per the SDM.

In other words, it doesn't impact other enclave visible keys (those from
EGETKEY) such as sealing key.

I think this is important.  Because if enclave visible keys such as
sealing key are lost on EUPDATESVN when SVN is already up-to-date (which
is the most common case), it will bring significant visible impact to
enclave.  E.g., one enclave could find its secret encrypted by sealing key
could never be retrieved after it restarts.

Assuming I didn't miss anything, can we also mention this in the
changelog?

> -----Original Message-----
> From: Huang, Kai <kai.huang@intel.com>
> Sent: Thursday, July 24, 2025 2:13 PM
> To: Reshetova, Elena <elena.reshetova@intel.com>; Hansen, Dave
> <dave.hansen@intel.com>
> Cc: seanjc@google.com; mingo@kernel.org; Scarlata, Vincent R
> <vincent.r.scarlata@intel.com>; x86@kernel.org; jarkko@kernel.org;
> Annapurve, Vishal <vannapurve@google.com>; linux-kernel@vger.kernel.org;
> Mallick, Asit K <asit.k.mallick@intel.com>; Aktas, Erdem
> <erdemaktas@google.com>; Cai, Chong <chongc@google.com>; Bondarevska,
> Nataliia <bondarn@google.com>; linux-sgx@vger.kernel.org; Raynor, Scott
> <scott.raynor@intel.com>
> Subject: Re: [PATCH v9 6/6] x86/sgx: Enable automatic SVN updates for SGX
> enclaves
> 
> On Thu, 2025-07-24 at 11:02 +0300, Reshetova, Elena wrote:
> > == 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.
> >
> > == Patch Contents ==
> 
> Nit: you can use "Solution" instead of "Patch Contents".

Sure.

> 
> >
> > 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.
> 
> (Sorry I forgot to say this in the previous versions):
> 
> If I read the pseudo code correctly, when the SVN is already up-to-date,
> the EUPDATESVN doesn't update SVN but it re-generates crypto assets
> anyway.
> 
> This is no harm per the pseudo code, since the "crypto assets" is actually
> the CR_BASE_KEY which is only used by EWB/ELDU flow per the SDM.
> 
> In other words, it doesn't impact other enclave visible keys (those from
> EGETKEY) such as sealing key.
> 
> I think this is important.  Because if enclave visible keys such as
> sealing key are lost on EUPDATESVN when SVN is already up-to-date (which
> is the most common case), it will bring significant visible impact to
> enclave.  E.g., one enclave could find its secret encrypted by sealing key
> could never be retrieved after it restarts.
> 
> Assuming I didn't miss anything, can we also mention this in the
> changelog?

Yes, you are right, anything like above behaviour would be a nightmare
in practice. So would smth like this work as an additional text:

"Note that in cases when SVN is already up-to-date and EUPDATESVN
is executed, it does not affect enclaves' visible keys obtained via EGETKEY
instruction."

?

Best Regards,
Elena.

> 
> > 
> > > 
> > > 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.
> > 
> > (Sorry I forgot to say this in the previous versions):
> > 
> > If I read the pseudo code correctly, when the SVN is already up-to-date,
> > the EUPDATESVN doesn't update SVN but it re-generates crypto assets
> > anyway.
> > 
> > This is no harm per the pseudo code, since the "crypto assets" is actually
> > the CR_BASE_KEY which is only used by EWB/ELDU flow per the SDM.
> > 
> > In other words, it doesn't impact other enclave visible keys (those from
> > EGETKEY) such as sealing key.
> > 
> > I think this is important.  Because if enclave visible keys such as
> > sealing key are lost on EUPDATESVN when SVN is already up-to-date (which
> > is the most common case), it will bring significant visible impact to
> > enclave.  E.g., one enclave could find its secret encrypted by sealing key
> > could never be retrieved after it restarts.
> > 
> > Assuming I didn't miss anything, can we also mention this in the
> > changelog?
> 
> Yes, you are right, anything like above behaviour would be a nightmare
> in practice. So would smth like this work as an additional text:
> 
> "Note that in cases when SVN is already up-to-date and EUPDATESVN
> is executed, it does not affect enclaves' visible keys obtained via EGETKEY
> instruction."
> 
> ?
> 

Yes works for me.  Thanks.

On Thu, 2025-07-24 at 21:14 +0000, Huang, Kai wrote:
> > 
> > > 
> > > > 
> > > > 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.
> > > 
> > > (Sorry I forgot to say this in the previous versions):
> > > 
> > > If I read the pseudo code correctly, when the SVN is already up-to-date,
> > > the EUPDATESVN doesn't update SVN but it re-generates crypto assets
> > > anyway.
> > > 
> > > This is no harm per the pseudo code, since the "crypto assets" is actually
> > > the CR_BASE_KEY which is only used by EWB/ELDU flow per the SDM.
> > > 
> > > In other words, it doesn't impact other enclave visible keys (those from
> > > EGETKEY) such as sealing key.
> > > 
> > > I think this is important.  Because if enclave visible keys such as
> > > sealing key are lost on EUPDATESVN when SVN is already up-to-date (which
> > > is the most common case), it will bring significant visible impact to
> > > enclave.  E.g., one enclave could find its secret encrypted by sealing key
> > > could never be retrieved after it restarts.
> > > 
> > > Assuming I didn't miss anything, can we also mention this in the
> > > changelog?
> > 
> > Yes, you are right, anything like above behaviour would be a nightmare
> > in practice. So would smth like this work as an additional text:
> > 
> > "Note that in cases when SVN is already up-to-date and EUPDATESVN
> > is executed, it does not affect enclaves' visible keys obtained via EGETKEY
> > instruction."
> > 
> > ?
> > 
> 
> Yes works for me.  Thanks.

Side topic, just out of curiosity, do you know why Intel decided to re-
generate CR_BASE_KEY even SVN is found to be up-to-date?

> -----Original Message-----
> From: Huang, Kai <kai.huang@intel.com>
> Sent: Friday, July 25, 2025 12:43 AM
> To: Reshetova, Elena <elena.reshetova@intel.com>; Hansen, Dave
> <dave.hansen@intel.com>
> Cc: seanjc@google.com; linux-sgx@vger.kernel.org; Scarlata, Vincent R
> <vincent.r.scarlata@intel.com>; x86@kernel.org; jarkko@kernel.org;
> Annapurve, Vishal <vannapurve@google.com>; linux-kernel@vger.kernel.org;
> Mallick, Asit K <asit.k.mallick@intel.com>; Aktas, Erdem
> <erdemaktas@google.com>; Cai, Chong <chongc@google.com>; Bondarevska,
> Nataliia <bondarn@google.com>; mingo@kernel.org; Raynor, Scott
> <scott.raynor@intel.com>
> Subject: Re: [PATCH v9 6/6] x86/sgx: Enable automatic SVN updates for SGX
> enclaves
> 
> On Thu, 2025-07-24 at 21:14 +0000, Huang, Kai wrote:
> > >
> > > >
> > > > >
> > > > > 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.
> > > >
> > > > (Sorry I forgot to say this in the previous versions):
> > > >
> > > > If I read the pseudo code correctly, when the SVN is already up-to-date,
> > > > the EUPDATESVN doesn't update SVN but it re-generates crypto assets
> > > > anyway.
> > > >
> > > > This is no harm per the pseudo code, since the "crypto assets" is actually
> > > > the CR_BASE_KEY which is only used by EWB/ELDU flow per the SDM.
> > > >
> > > > In other words, it doesn't impact other enclave visible keys (those from
> > > > EGETKEY) such as sealing key.
> > > >
> > > > I think this is important.  Because if enclave visible keys such as
> > > > sealing key are lost on EUPDATESVN when SVN is already up-to-date
> (which
> > > > is the most common case), it will bring significant visible impact to
> > > > enclave.  E.g., one enclave could find its secret encrypted by sealing key
> > > > could never be retrieved after it restarts.
> > > >
> > > > Assuming I didn't miss anything, can we also mention this in the
> > > > changelog?
> > >
> > > Yes, you are right, anything like above behaviour would be a nightmare
> > > in practice. So would smth like this work as an additional text:
> > >
> > > "Note that in cases when SVN is already up-to-date and EUPDATESVN
> > > is executed, it does not affect enclaves' visible keys obtained via EGETKEY
> > > instruction."
> > >
> > > ?
> > >
> >
> > Yes works for me.  Thanks.
> 
> Side topic, just out of curiosity, do you know why Intel decided to re-
> generate CR_BASE_KEY even SVN is found to be up-to-date?

This design pre-dates me, but as far as I understand it was due to some
internal design constraints present in Ice Lake Server, where this was
first introduced.

Best Regards,
Elena.

>  /* Counter to count the active SGX users */
>  static int __maybe_unused sgx_usage_count;

Seems you forgot to remove this __maybe_unused.

I think you can just introduce 'sgx_usage_count' in this patch.

>  
> +/* Mutex to ensure no concurrent EPC accesses during EUPDATESVN */
> +static DEFINE_MUTEX(sgx_svn_lock);
> +
>  /**
>   * sgx_update_svn() - Attempt to call ENCLS[EUPDATESVN].
>   * This instruction attempts to update CPUSVN to the
> @@ -937,7 +940,7 @@ static int __maybe_unused sgx_usage_count;
>   *  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;
>  
> @@ -983,12 +986,17 @@ static int __maybe_unused sgx_update_svn(void)
>  
>  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

> -----Original Message-----
> From: Huang, Kai <kai.huang@intel.com>
> Sent: Thursday, July 24, 2025 1:18 PM
> To: Reshetova, Elena <elena.reshetova@intel.com>; Hansen, Dave
> <dave.hansen@intel.com>
> Cc: seanjc@google.com; mingo@kernel.org; Scarlata, Vincent R
> <vincent.r.scarlata@intel.com>; x86@kernel.org; jarkko@kernel.org;
> Annapurve, Vishal <vannapurve@google.com>; linux-kernel@vger.kernel.org;
> Mallick, Asit K <asit.k.mallick@intel.com>; Aktas, Erdem
> <erdemaktas@google.com>; Cai, Chong <chongc@google.com>; Bondarevska,
> Nataliia <bondarn@google.com>; linux-sgx@vger.kernel.org; Raynor, Scott
> <scott.raynor@intel.com>
> Subject: Re: [PATCH v9 6/6] x86/sgx: Enable automatic SVN updates for SGX
> enclaves
> 
> 
> >  /* Counter to count the active SGX users */
> >  static int __maybe_unused sgx_usage_count;
> 
> Seems you forgot to remove this __maybe_unused.

Yes, too much patch gymnastics ((

> 
> I think you can just introduce 'sgx_usage_count' in this patch.

I agree, would be easier to review.

Best Regards,
Elena.