Re: [RFC PATCH 03/23] x86/sgx: Introduce virtual EPC for use by KVM guests

Kai Huang
2021-01-07T01:39:58Z

On Wed, 6 Jan 2021 16:52:49 -0800 Dave Hansen wrote:
> On 1/6/21 4:47 PM, Kai Huang wrote:
> >>>> + ret = __eremove(sgx_get_epc_virt_addr(epc_page));
> >>>> + if (ret) {
> >>>> + /*
> >>>> + * Only SGX_CHILD_PRESENT is expected, which is because of
> >>>> + * EREMOVE-ing an SECS still with child, in which case it can
> >>>> + * be handled by EREMOVE-ing the SECS again after all pages in
> >>>> + * virtual EPC have been EREMOVE-ed. See comments in below in
> >>>> + * sgx_virt_epc_release().
> >>>> + */
> >>>> + WARN_ON_ONCE(ret != SGX_CHILD_PRESENT);
> >>>> + return ret;
> >>>> + }
> >>> I find myself wondering what errors could cause the WARN_ON_ONCE() to be
> >>> hit. The SDM indicates that it's only:
> >>>
> >>> SGX_ENCLAVE_ACT If there are still logical processors executing
> >>> inside the enclave.
> >>>
> >>> Should that be mentioned in the comment?
> >> And faults, which are also spliced into the return value by the ENCLS macros.
> >> I do remember hitting this WARN when I broke things, though I can't remember
> >> whether it was a fault or the SGX_ENCLAVE_ACT scenario. Probably the latter?
> > I'll add a comment saying that there should be no active logical processor
> > still running inside guest's enclave. We cannot handle SGX_ENCLAVE_ACT here
> > anyway.
>
> One more thing...
>
> Could we dump out the *actual* error code with a WARN(), please? If we
> see a warning, I'd rather not have to disassemble the instructions and
> check against register values to see whether the error code was sane.

Sure. But WARN_ONCE() should be used, right, instead of WARN()?