> On Thu, Apr 24, 2025, Elena Reshetova wrote:
> > > On Thu, Apr 24, 2025, Elena Reshetova wrote:
> > > +void sgx_dec_usage_count(void)
> > > +{
> > > + if (atomic_dec_return(&sgx_usage_count))
> > > + return;
> > > +
> > > + guard(mutex)(&sgx_svn_lock);
> > > +
> > > + if (atomic_read(&sgx_usage_count))
> > > + return;
> > > +
> > > + sgx_update_svn();
> >
> > Why do we want to try to execute this on release also? I would think that
> > doing this in sgx_inc_usage_count() is enough.
>
> I assume an actual SVN update takes some amount of time?
Yes, correct, it is not a fast action and can be even slower if we are
running out of entropy and have to retry.
>
> If that's correct, then doing the work upon destroying the last enclave is
> desirable,
> as it's less likely to introduce delay that negatively affects userspace.
> Userspace
> generally won't care about a 10us delay when destroying a process, but a
> 10us delay
> to launch an enclave could be quite problematic, e.g. in the TDX use case
> where
> enclaves may be launched on-demand in response to a guest attestation
> request.
Ok, but in this case, you are hooking both: create and release.
I guess your line of thinking is that in most of the cases it will be handled by
a release flow when destroying enclaves, but in cases we happen to need
to update a machine which doesn’t have a single enclave yet, the create flow
will be used. The problem is that if you look at the instruction definition,
we won't save too much when executing this in release flow (Kai I think already
pointed
this out):
IF (Other instruction is accessing EPC) THEN
RFLAGS.ZF := 1
RAX := SGX_LOCKFAIL;
GOTO ERROR_EXIT;
FI
(* Verify EPC is ready *)
IF (the EPC contains any valid pages) THEN
RFLAGS.ZF := 1;
RAX := SGX_EPC_NOT_READY;
GOTO ERROR_EXIT;
FI
(* Refresh paging key *)
IF (NOT RDSEED(&TMP_KEY, 16)) THEN
RFLAGS.ZF := 1;
RAX := SGX_INSUFFICIENT_ENTROPY;
GOTO ERROR_EXIT;
FI
(* Commit *)
CR_BASE_KEY := TMP_KEY;
TMP_CPUSVN := CR_CPUSVN;
(* Update CPUSVN to current minimum patch even if locked *)
(* Determine if info status is needed *)
----------------------------
All above would be done anyhow on create even if we successfully
executed it on release previously (( And then finally we go into:
IF (TMP_CPUSVN = CR_CPUSVN) THEN
RFLAGS.CF := 1;
RAX := SGX_NO_UPDATE;
FI
ERROR_EXIT:
>
> If the update time is tiny, then I agree that hooking release would probably
> do
> more harm than good.
So, it is not that the time is tiny, it is like we will do it twice, unnecessary
and potentially quite long in both cases (taking RDSEED step into account).
The reason why the instruction is defined this way is that it was not intended
originally
to be inserted into some existing SGX flows, but was envisioned to be
standalone cmd
for the host orchestrator to execute once it thinks system is ready.
Best Regards,
Elena.
