[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <CU4EJXH6STEZ.20R6EOX7V3XGD@seitikki>
Date: Mon, 17 Jul 2023 11:14:56 +0000
From: "Jarkko Sakkinen" <jarkko@...nel.org>
To: "Haitao Huang" <haitao.huang@...ux.intel.com>,
<dave.hansen@...ux.intel.com>, <tj@...nel.org>,
<linux-kernel@...r.kernel.org>, <linux-sgx@...r.kernel.org>,
<cgroups@...r.kernel.org>, "Thomas Gleixner" <tglx@...utronix.de>,
"Ingo Molnar" <mingo@...hat.com>, "Borislav Petkov" <bp@...en8.de>,
<x86@...nel.org>, "H. Peter Anvin" <hpa@...or.com>
Cc: <kai.huang@...el.com>, <reinette.chatre@...el.com>,
<zhiquan1.li@...el.com>, <kristen@...ux.intel.com>,
<seanjc@...gle.com>
Subject: Re: [PATCH v3 01/28] x86/sgx: Store struct sgx_encl when allocating
new VA pages
On Wed Jul 12, 2023 at 11:01 PM UTC, Haitao Huang wrote:
> In a later patch, when a cgroup has exceeded the max capacity for EPC pages
> and there are no more Enclave EPC pages associated with the cgroup that can
> be reclaimed, the only pages still associated with an enclave will be the
> unreclaimable Version Array (VA) pages or SECS pages, and the entire
> enclave will need to be killed to free up those pages.
>
> Currently, given an enclave pointer it is easy to find the associated VA
> pages and free them, however, OOM killing an enclave based on cgroup limits
> will require examining a cgroup's unreclaimable page list, and finding an
> enclave given a SECS page or a VA page. This will require a backpointer
> from a page to an enclave, including for VA pages.
>
> When allocating new Version Array (VA) pages, pass the struct sgx_encl of
> the enclave that is allocating the page. sgx_alloc_epc_page() will store
> this value in the owner field of the struct sgx_epc_page. In a later
> patch, VA pages will be placed in an unreclaimable queue, and then when the
> cgroup max limit is reached and there are no more reclaimable pages and the
> enclave must be OOM killed, all the VA pages associated with that enclave
> can be uncharged and freed.
>
> To avoid casting needed to access the two types of owners: sgx_encl for VA
> pages, sgx_encl_page for other pages, replace 'owner' field in sgx_epc_page
> with a union of the two types.
I think the action taken is correct but the reasoning is a bit
convoluted.
Why not instead put something like:
"Because struct sgx_epc_page instances of VA pages are not owned by an
sgx_encl_page instance in the first place, mark their owner as sgx_encl,
in order to make it reachable from the unreclaimable list."
The code change itself, and rest of the paragraphs do look reasonable.
BR, Jarkko
Powered by blists - more mailing lists