On Tue, 03 Oct 2023 15:03:48 -0500, Huang, Kai <kai.huang@xxxxxxxxx> wrote:
On Mon, 2023-10-02 at 23:49 -0500, Haitao Huang wrote:
On Wed, 27 Sep 2023 05:28:36 -0500, Huang, Kai <kai.huang@xxxxxxxxx>
wrote:
> 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.
>
I don't quite get why you would view 3) as a separate item from 1).
1) is about using some method to track EPC page status, 3) is adding a
new
state.
Why cannot they be separated?
In my view, 4) is not done as long as there is not separate list to
track
it.
You are literally doing below:
@@ -113,6 +113,9 @@ static int sgx_encl_create(struct sgx_encl *encl,
struct
sgx_secs *secs)
encl->attributes = secs->attributes;
encl->attributes_mask = SGX_ATTR_UNPRIV_MASK;
+ sgx_record_epc_page(encl->secs.epc_page,
+ SGX_EPC_PAGE_UNRECLAIMABLE);
+
Which obviously is tracking SECS as unreclaimable page here.
The only thing you are not doing now is to put to the actual list, which
you
introduced in a later patch.
But why not just doing them together?
I see where the problem is now. Initially these states are bit masks so
UNTRACKED and UNRECLAIMABLE are all not masked (set zero). I'll change
these "record" calls with UNTRACKED instead, and later replace with
UNRECLAIMABLE when they are actually added to the list. So UNRECLAIMABLE
state can also be delayed until that patch with the list added.
Thanks.
Haitao
