On Fri, 2023-09-22 at 20:06 -0700, Haitao Huang wrote: > Use the lower 3 bits in the flags field of sgx_epc_page struct to > track EPC states in its life cycle and define an enum for possible > states. More state(s) will be added later. This patch does more than what the changelog claims to do. AFAICT it does below: 1) Use the lower 3 bits to track EPC page status 2) Rename SGX_EPC_PAGE_RECLAIMER_TRACKED to SGX_EPC_PAGE_RERCLAIMABLE 3) Introduce a new state SGX_EPC_PAGE_UNRECLAIMABLE 4) Track SECS and VA pages as SGX_EPC_PAGE_UNRECLAIMABLE The changelog only says 1) IIUC. If we really want to do all these in one patch, then the changelog should at least mention the justification of all of them. But I don't see why 3) and 4) need to be done here. Instead, IMHO they should be done in a separate patch, and do it after the unreclaimable list is introduced (or you need to bring that patch forward). For instance, ... [snip] > + > + /* Page is in use but tracked in an unreclaimable LRU list. These are > + * only reclaimable when the whole enclave is OOM killed or the enclave > + * is released, e.g., VA, SECS pages > + * Becomes NOT_TRACKED after sgx_drop_epc() > + */ > + SGX_EPC_PAGE_UNRECLAIMABLE = 3, ... We even don't have the unreclaimable LRU list yet. It's odd to have this comment here.
