Re: [PATCH v5 1/3] x86/sgx: Split SGX_ENCL_PAGE_BEING_RECLAIMED into two flags

On Wed Aug 21, 2024 at 1:02 PM EEST, Dmitrii Kuvaiskii wrote:

The page reclaimer thread sets SGX_ENC_PAGE_BEING_RECLAIMED flag when the enclave page is being reclaimed (moved to the backing store). This flag however has two logical meanings:

1. Don't attempt to load the enclave page (the page is busy), see __sgx_encl_load_page().
2. Don't attempt to remove the PCMD page corresponding to this enclave page (the PCMD page is busy), see reclaimer_writing_to_pcmd().

To reflect these two meanings, split SGX_ENCL_PAGE_BEING_RECLAIMED into two flags: SGX_ENCL_PAGE_BUSY and SGX_ENCL_PAGE_PCMD_BUSY. Currently, both flags are set only when the enclave page is being reclaimed (by the page reclaimer thread). A future commit will introduce new cases when the enclave page is being operated on; these new cases will set only the SGX_ENCL_PAGE_BUSY flag.

Cc: stable@vger.kernel.org Signed-off-by: Dmitrii Kuvaiskii dmitrii.kuvaiskii@intel.com Reviewed-by: Haitao Huang haitao.huang@linux.intel.com Acked-by: Kai Huang kai.huang@intel.com

---

arch/x86/kernel/cpu/sgx/encl.c | 16 +++++++--------- arch/x86/kernel/cpu/sgx/encl.h | 10 ++++++++-- arch/x86/kernel/cpu/sgx/main.c | 4 ++-- 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c index 279148e72459..c0a3c00284c8 100644 --- a/arch/x86/kernel/cpu/sgx/encl.c +++ b/arch/x86/kernel/cpu/sgx/encl.c @@ -46,10 +46,10 @@ static int sgx_encl_lookup_backing(struct sgx_encl *encl, unsigned long page_ind

• a check if an enclave page sharing the PCMD page is in the process of being
• reclaimed.

• • The reclaimer sets the SGX_ENCL_PAGE_BEING_RECLAIMED flag when it
• • intends to reclaim that enclave page - it means that the PCMD page
• • associated with that enclave page is about to get some data and thus
• • even if the PCMD page is empty, it should not be truncated.

• • The reclaimer sets the SGX_ENCL_PAGE_PCMD_BUSY flag when it intends to
• • reclaim that enclave page - it means that the PCMD page associated with that
• • enclave page is about to get some data and thus even if the PCMD page is
• • empty, it should not be truncated.
  • Context: Enclave mutex (&sgx_encl->lock) must be held.
  • Return: 1 if the reclaimer is about to write to the PCMD page

@@ -77,8 +77,7 @@ static int reclaimer_writing_to_pcmd(struct sgx_encl *encl, * Stop when reaching the SECS page - it does not * have a page_array entry and its reclaim is * started and completed with enclave mutex held so

•  * it does not use the SGX_ENCL_PAGE_BEING_RECLAIMED
  

•  * flag.
  

•  * it does not use the SGX_ENCL_PAGE_PCMD_BUSY flag.
  

  */ if (addr == encl->base + encl->size) break;

@@ -91,8 +90,7 @@ static int reclaimer_writing_to_pcmd(struct sgx_encl *encl, * VA page slot ID uses same bit as the flag so it is important * to ensure that the page is not already in backing store. */

• if (entry->epc_page &&
  

•     (entry->desc & SGX_ENCL_PAGE_BEING_RECLAIMED)) {
  

• if (entry->epc_page && (entry->desc & SGX_ENCL_PAGE_PCMD_BUSY)) {
  reclaimed = 1;
  break;
  

  }

@@ -257,7 +255,7 @@ static struct sgx_encl_page *__sgx_encl_load_page(struct sgx_encl *encl, /* Entry successfully located. */ if (entry->epc_page) {

• if (entry->desc & SGX_ENCL_PAGE_BEING_RECLAIMED)
  

• if (entry->desc & SGX_ENCL_PAGE_BUSY)
  return ERR_PTR(-EBUSY);
  

return entry; diff --git a/arch/x86/kernel/cpu/sgx/encl.h b/arch/x86/kernel/cpu/sgx/encl.h index f94ff14c9486..b566b8ad5f33 100644 --- a/arch/x86/kernel/cpu/sgx/encl.h +++ b/arch/x86/kernel/cpu/sgx/encl.h @@ -22,8 +22,14 @@ /* 'desc' bits holding the offset in the VA (version array) page. */ #define SGX_ENCL_PAGE_VA_OFFSET_MASK GENMASK_ULL(11, 3) -/* 'desc' bit marking that the page is being reclaimed. */ -#define SGX_ENCL_PAGE_BEING_RECLAIMED BIT(3) +/* 'desc' bit indicating that the page is busy (being reclaimed). */ +#define SGX_ENCL_PAGE_BUSY BIT(2)

+/*

• • 'desc' bit indicating that PCMD page associated with the enclave page is
• • busy (because the enclave page is being reclaimed).
• */

+#define SGX_ENCL_PAGE_PCMD_BUSY BIT(3) struct sgx_encl_page { unsigned long desc; diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index 166692f2d501..e94b09c43673 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -204,7 +204,7 @@ static void sgx_encl_ewb(struct sgx_epc_page *epc_page, void *va_slot; int ret;

• encl_page->desc &= ~SGX_ENCL_PAGE_BEING_RECLAIMED;

• encl_page->desc &= ~(SGX_ENCL_PAGE_BUSY | SGX_ENCL_PAGE_PCMD_BUSY);

va_page = list_first_entry(&encl->va_pages, struct sgx_va_page, list); @@ -340,7 +340,7 @@ static void sgx_reclaim_pages(void) goto skip; }

• encl_page->desc |= SGX_ENCL_PAGE_BEING_RECLAIMED;
  

• encl_page->desc |= SGX_ENCL_PAGE_BUSY | SGX_ENCL_PAGE_PCMD_BUSY;
  

  mutex_unlock(&encl_page->encl->lock); continue;

Reviewed-by: Jarkko Sakkinen jarkko@kernel.org

BR, Jarkko