[<prev] [next>] [day] [month] [year] [list]
Message-ID: <0a96ecb7-50cc-634d-2f94-09e547be7b44@intel.com>
Date: Mon, 16 Nov 2020 10:33:24 -0800
From: Dave Hansen <dave.hansen@...el.com>
To: Hillf Danton <hdanton@...a.com>,
Jarkko Sakkinen <jarkko@...nel.org>
Cc: x86@...nel.org, linux-sgx@...r.kernel.org,
linux-kernel@...r.kernel.org,
Sean Christopherson <sean.j.christopherson@...el.com>,
Jethro Beekman <jethro@...tanix.com>,
Serge Ayoun <serge.ayoun@...el.com>, akpm@...ux-foundation.org
Subject: Re: [PATCH v41 05/24] x86/sgx: Initialize metadata for Enclave Page
Cache (EPC) sections
On 11/14/20 12:42 AM, Hillf Danton wrote:
> On Fri, 13 Nov 2020 00:01:16 +0200 Jarkko Sakkinen wrote:
>> + */
>> +static void sgx_sanitize_section(struct sgx_epc_section *section)
>> +{
>> + struct sgx_epc_page *page;
>> + LIST_HEAD(dirty);
>> + int ret;
>> +
>> + while (!list_empty(§ion->laundry_list)) {
>> + if (kthread_should_stop())
>> + return;
>> +
>> + spin_lock(§ion->lock);
>> +
>> + page = list_first_entry(§ion->laundry_list,
>> + struct sgx_epc_page, list);
>> +
>> + ret = __eremove(sgx_get_epc_virt_addr(page));
>> + if (!ret)
>> + list_move(&page->list, §ion->page_list);
>> + else
>> + list_move_tail(&page->list, &dirty);
>> +
>> + spin_unlock(§ion->lock);
>> +
>> + cond_resched();
>> + }
>> +
>> + list_splice(&dirty, §ion->laundry_list);
>
> Move list splice into the section under section->lock.
The naming, commenting and changelogs could be better here. But, I
think the code is correct.
section->lock actually only protects ->page_list.
->laundry_list is initialized in core code, but after that is only used
by ksgxd and is effectively a thread-local structure.
I can think of a few other ways of doing this so that, for instance,
laundry_list was a thread-local structure in ksgxd that is freed after
initialization and sanitizing. But, this is pretty simple, although
under-documented, and wastes a list_head worth of space per section at
runtime.
>> +}
> [...]
>
>> +struct sgx_epc_page {
>> + unsigned int section;
>
> To make the sgx page naturally packed, add a small pad to match both
> sizeof(struct list_head) and X86_64. Feel free to turn back on the
> pad OTOH to save memory.
You make a good point: it's pretty obvious that the current code is
space-optimized. That doesn't seem like a bad thing to me, though.
Powered by blists - more mailing lists