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()?