Re: [PATCH v5 06/18] x86/sgx: Introduce EPC page states

From: Haitao Huang
Date: Wed Oct 04 2023 - 11:25:00 EST


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