On 2/8/21 2:54 AM, Kai Huang wrote: > From: Jarkko Sakkinen <jarkko@xxxxxxxxxx> > > Encapsulate the snippet in sgx_free_epc_page() concerning EREMOVE to > sgx_reset_epc_page(), which is a static helper function for > sgx_encl_release(). It's the only function existing, which deals with > initialized pages. I didn't really like the changelog the last time around, so I wrote you a new one: > https://lore.kernel.org/kvm/8250aedb-a623-646d-071a-75ece2c41c09@xxxxxxxxx/ The "v4" changelog is pretty hard for me to read. It doesn't tell me why we can "wipe out EREMOVE" or how it is going to get used going forward. > + > +/* > + * Place the page in uninitialized state. Called by in sgx_encl_release() > + * before sgx_free_epc_page(), which requires EPC page is already in clean > + * slate. > + */ I really don't like comments like that that refer to callers. They're basically guaranteed to become obsolete. /* * Place the page in uninitialized state. Only usable by callers that * know the page is in a clean state in which EREMOVE will succeed. */ > +static void sgx_reset_epc_page(struct sgx_epc_page *epc_page) > +{ > + int ret; > + > + WARN_ON_ONCE(epc_page->flags & SGX_EPC_PAGE_RECLAIMER_TRACKED); > + > + ret = __eremove(sgx_get_epc_virt_addr(epc_page)); > + if (WARN_ONCE(ret, "EREMOVE returned %d (0x%x)", ret, ret)) > + return; > +} The uncommented warnings aren't very nice, but I guess they're in the existing code.