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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b1d7199262b9da496a04a36447541e9166f35f19.camel@intel.com>
Date:   Wed, 27 Sep 2023 10:28:36 +0000
From:   "Huang, Kai" <kai.huang@...el.com>
To:     "hpa@...or.com" <hpa@...or.com>,
        "linux-sgx@...r.kernel.org" <linux-sgx@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "cgroups@...r.kernel.org" <cgroups@...r.kernel.org>,
        "bp@...en8.de" <bp@...en8.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "jarkko@...nel.org" <jarkko@...nel.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "haitao.huang@...ux.intel.com" <haitao.huang@...ux.intel.com>,
        "Mehta, Sohil" <sohil.mehta@...el.com>,
        "tj@...nel.org" <tj@...nel.org>,
        "mingo@...hat.com" <mingo@...hat.com>
CC:     "kristen@...ux.intel.com" <kristen@...ux.intel.com>,
        "yangjie@...rosoft.com" <yangjie@...rosoft.com>,
        "Li, Zhiquan1" <zhiquan1.li@...el.com>,
        "Christopherson,, Sean" <seanjc@...gle.com>,
        "mikko.ylinen@...ux.intel.com" <mikko.ylinen@...ux.intel.com>,
        "Zhang, Bo" <zhanb@...rosoft.com>,
        "anakrish@...rosoft.com" <anakrish@...rosoft.com>
Subject: Re: [PATCH v5 06/18] x86/sgx: Introduce EPC page states

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.