diff mbox series

[v3,02/28] x86/sgx: Add EPC page flags to identify owner type

Message ID 20230712230202.47929-3-haitao.huang@linux.intel.com
State New
Headers
Series Add Cgroup support for SGX EPC memory |

Commit Message

Haitao Huang July 12, 2023, 11:01 p.m. UTC 
  From: Sean Christopherson <sean.j.christopherson@intel.com>

Two types of owners, 'sgx_encl' for VA pages and 'sgx_encl_page' for other,
can be stored in the union field in sgx_epc_page struct introduced in the
previous patch.

When cgroup OOM support is added in a later patch, the owning enclave of a
page will need to be identified. Retrieving the sgx_encl struct from a
sgx_epc_page will be different if the page is a VA page vs. other enclave
pages.

Add 2 flags which will identify the type of the owner and apply them
accordingly to newly allocated pages.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com>
Cc: Sean Christopherson <seanjc@google.com>

V3:
- Renamed the flags to clarify they are used to identify the type
of the owner.
---
 arch/x86/kernel/cpu/sgx/encl.c  | 4 ++++
 arch/x86/kernel/cpu/sgx/ioctl.c | 4 ++++
 arch/x86/kernel/cpu/sgx/sgx.h   | 6 ++++++
 3 files changed, 14 insertions(+)

Comments

Jarkko Sakkinen July 17, 2023, 12:41 p.m. UTC | #1 
On Wed Jul 12, 2023 at 11:01 PM UTC, Haitao Huang wrote:
> From: Sean Christopherson <sean.j.christopherson@intel.com>
>
> Two types of owners, 'sgx_encl' for VA pages and 'sgx_encl_page' for other,
> can be stored in the union field in sgx_epc_page struct introduced in the
> previous patch.

This would be easier to follow:

"Two types of owners of struct_epc_page, 'sgx_encl' for VA pages and
'sgx_encl_page' can be stored in the previously introduced union field."

> When cgroup OOM support is added in a later patch, the owning enclave of a
> page will need to be identified. Retrieving the sgx_encl struct from a
> sgx_epc_page will be different if the page is a VA page vs. other enclave
> pages.
>
> Add 2 flags which will identify the type of the owner and apply them
> accordingly to newly allocated pages.

This would be easier to follow:

"OOM support for cgroups requires that the owner needs to be identified
when selecting pages from the unreclaimable list. Address this by adding
flags for identifying the owner type."

It is better to carry the story a little bit forward than say that a
subsequent patch will require this :-) I.e. enough to get at least a
rough idea what is going on.

R, Jarkko
Jarkko Sakkinen July 17, 2023, 12:43 p.m. UTC | #2 
On Mon Jul 17, 2023 at 12:41 PM UTC, Jarkko Sakkinen wrote:
> On Wed Jul 12, 2023 at 11:01 PM UTC, Haitao Huang wrote:
> > From: Sean Christopherson <sean.j.christopherson@intel.com>
> >
> > Two types of owners, 'sgx_encl' for VA pages and 'sgx_encl_page' for other,
> > can be stored in the union field in sgx_epc_page struct introduced in the
> > previous patch.
>
> This would be easier to follow:
>
> "Two types of owners of struct_epc_page, 'sgx_encl' for VA pages and
> 'sgx_encl_page' can be stored in the previously introduced union field."
>
> > When cgroup OOM support is added in a later patch, the owning enclave of a
> > page will need to be identified. Retrieving the sgx_encl struct from a
> > sgx_epc_page will be different if the page is a VA page vs. other enclave
> > pages.
> >
> > Add 2 flags which will identify the type of the owner and apply them
> > accordingly to newly allocated pages.
>
> This would be easier to follow:
>
> "OOM support for cgroups requires that the owner needs to be identified
> when selecting pages from the unreclaimable list. Address this by adding
> flags for identifying the owner type."
>
> It is better to carry the story a little bit forward than say that a
> subsequent patch will require this :-) I.e. enough to get at least a
> rough idea what is going on.

Oops, sent by mistake. I was going to say that the flag would be better
named simply as SGX_EPC_OWNER_PAGE instead of SGX_EPC_OWNER_ENCL_PAGE.

BR, Jarkko
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
index 98e1086eab07..3bc2f95b1da2 100644
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -252,6 +252,7 @@  static struct sgx_encl_page *__sgx_encl_load_page(struct sgx_encl *encl,
 		epc_page = sgx_encl_eldu(&encl->secs, NULL);
 		if (IS_ERR(epc_page))
 			return ERR_CAST(epc_page);
+		epc_page->flags |= SGX_EPC_OWNER_ENCL_PAGE;
 	}
 
 	epc_page = sgx_encl_eldu(entry, encl->secs.epc_page);
@@ -260,6 +261,7 @@  static struct sgx_encl_page *__sgx_encl_load_page(struct sgx_encl *encl,
 
 	encl->secs_child_cnt++;
 	sgx_mark_page_reclaimable(entry->epc_page);
+	entry->epc_page->flags |= SGX_EPC_OWNER_ENCL_PAGE;
 
 	return entry;
 }
@@ -379,6 +381,7 @@  static vm_fault_t sgx_encl_eaug_page(struct vm_area_struct *vma,
 	encl->secs_child_cnt++;
 
 	sgx_mark_page_reclaimable(encl_page->epc_page);
+	encl_page->epc_page->flags |= SGX_EPC_OWNER_ENCL_PAGE;
 
 	phys_addr = sgx_get_epc_phys_addr(epc_page);
 	/*
@@ -1235,6 +1238,7 @@  struct sgx_epc_page *sgx_alloc_va_page(struct sgx_encl *encl, bool reclaim)
 		sgx_encl_free_epc_page(epc_page);
 		return ERR_PTR(-EFAULT);
 	}
+	epc_page->flags |= SGX_EPC_OWNER_ENCL;
 
 	return epc_page;
 }
diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index fa8c3f32ccf6..fe3e89cf013f 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -113,6 +113,8 @@  static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
 	encl->attributes = secs->attributes;
 	encl->attributes_mask = SGX_ATTR_UNPRIV_MASK;
 
+	encl->secs.epc_page->flags |= SGX_EPC_OWNER_ENCL_PAGE;
+
 	/* Set only after completion, as encl->lock has not been taken. */
 	set_bit(SGX_ENCL_CREATED, &encl->flags);
 
@@ -323,6 +325,7 @@  static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long src,
 	}
 
 	sgx_mark_page_reclaimable(encl_page->epc_page);
+	encl_page->epc_page->flags |= SGX_EPC_OWNER_ENCL_PAGE;
 	mutex_unlock(&encl->lock);
 	mmap_read_unlock(current->mm);
 	return ret;
@@ -977,6 +980,7 @@  static long sgx_enclave_modify_types(struct sgx_encl *encl,
 			mutex_lock(&encl->lock);
 
 			sgx_mark_page_reclaimable(entry->epc_page);
+			entry->epc_page->flags |= SGX_EPC_OWNER_ENCL_PAGE;
 		}
 
 		/* Change EPC type */
diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h
index dc1cbcfcf2d4..f6e3c5810eef 100644
--- a/arch/x86/kernel/cpu/sgx/sgx.h
+++ b/arch/x86/kernel/cpu/sgx/sgx.h
@@ -29,6 +29,12 @@ 
 /* Pages on free list */
 #define SGX_EPC_PAGE_IS_FREE		BIT(1)
 
+/* flag for pages owned by a sgx_encl_page */
+#define SGX_EPC_OWNER_ENCL_PAGE		BIT(3)
+
+/* flag for pages owned by a sgx_encl struct */
+#define SGX_EPC_OWNER_ENCL		BIT(4)
+
 struct sgx_epc_page {
 	unsigned int section;
 	u16 flags;