Re: [PATCH 4/4] x86/sgx: Implement ENCLS[EUPDATESVN] and opportunistically call it during first EPC page alloc

[Jarkko Sakkinen <jarkko@kernel.org>]

On Thu, Mar 27, 2025 at 03:42:30PM +0000, Reshetova, Elena wrote:
> > > > > +	case SGX_NO_UPDATE:
> > > > > +		pr_debug("EUPDATESVN was successful, but CPUSVN was not
> > > > updated, "
> > > > > +			"because current SVN was not newer than
> > > > CPUSVN.\n");
> > > > > +		break;
> > > > > +	case SGX_EPC_NOT_READY:
> > > > > +		pr_debug("EPC is not ready for SVN update.");
> > > > > +		break;
> > > > > +	case SGX_INSUFFICIENT_ENTROPY:
> > > > > +		pr_debug("CPUSVN update is failed due to Insufficient
> > > > entropy in RNG, "
> > > > > +			"please try it later.\n");
> > > > > +		break;
> > > > > +	case SGX_EPC_PAGE_CONFLICT:
> > > > > +		pr_debug("CPUSVN update is failed due to concurrency
> > > > violation, please "
> > > > > +			"stop running any other ENCLS leaf and try it
> > > > later.\n");
> > > > > +		break;
> > > > > +	default:
> > > > > +		break;
> > > >
> > > > Remove pr_debug() statements.
> > >
> > > This I am not sure it is good idea. I think it would be useful for system
> > > admins to have a way to see that update either happened or not.
> > > It is true that you can find this out by requesting a new SGX attestation
> > > quote (and see if newer SVN is used), but it is not the faster way.
> > 
> > Maybe pr_debug() is them wrong level if they are meant for sysadmins?
> > 
> > I mean these should not happen in normal behavior like ever? As
> > pr_debug() I don't really grab this.
> 
> SGX_NO_UPDATE will absolutely happen normally all the time. 
> Since EUPDATESVN is executed every time EPC is empty, this is the
> most common code you will get back (because microcode updates are rare). 
> Others yes, that would indicate some error condition. 
> So, what is the pr_level that you would suggest? 

Right, got it. That changes my conclusions:

So I'd reformulate it like:

	switch (ret) {
	case 0:
		pr_info("EUPDATESVN: success\n);	
		break;
	case SGX_EPC_NOT_READY:
	case SGX_INSUFFICIENT_ENTROPY:
	case SGX_EPC_PAGE_CONFLICT:
		pr_err("EUPDATESVN: error %d\n", ret);
		/* TODO: block/teardown driver? */
		break;
	case SGX_NO_UPDATE:
		break;
	default:
		pr_err("EUPDATESVN: unknown error %d\n", ret);
		/* TODO: block/teardown driver? */
		break;
	}

Since when this is executed EPC usage is zero error cases should block
or teardown SGX driver, presuming that they are because of either
incorrect driver state or spurious error code.

If this happens, we definitely do not want service, right?

I'm not sure of all error codes how serious they are, or are all of them
consequence of incorrectly working driver.

BR, Jarkko