[RFC PATCH 03/20] x86/sgx: Track owning enclave in VA EPC pages

Kristen Carlson Accardi posted 20 patches 3 years, 6 months ago
There is a newer version of this series
[RFC PATCH 03/20] x86/sgx: Track owning enclave in VA EPC pages
Posted by Kristen Carlson Accardi 3 years, 6 months ago
From: Sean Christopherson <sean.j.christopherson@intel.com>

In order to fully account for an enclave's EPC page usage, store
the owning enclave of a VA EPC page.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
Cc: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kernel/cpu/sgx/encl.c  | 5 ++++-
 arch/x86/kernel/cpu/sgx/encl.h  | 2 +-
 arch/x86/kernel/cpu/sgx/ioctl.c | 2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
index f40d64206ded..a18f1311b57d 100644
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -1193,6 +1193,7 @@ void sgx_zap_enclave_ptes(struct sgx_encl *encl, unsigned long addr)
 
 /**
  * sgx_alloc_va_page() - Allocate a Version Array (VA) page
+ * @encl:    The enclave that this page is allocated to.
  * @reclaim: Reclaim EPC pages directly if none available. Enclave
  *           mutex should not be held if this is set.
  *
@@ -1202,7 +1203,7 @@ void sgx_zap_enclave_ptes(struct sgx_encl *encl, unsigned long addr)
  *   a VA page,
  *   -errno otherwise
  */
-struct sgx_epc_page *sgx_alloc_va_page(bool reclaim)
+struct sgx_epc_page *sgx_alloc_va_page(struct sgx_encl *encl, bool reclaim)
 {
 	struct sgx_epc_page *epc_page;
 	int ret;
@@ -1218,6 +1219,8 @@ struct sgx_epc_page *sgx_alloc_va_page(bool reclaim)
 		return ERR_PTR(-EFAULT);
 	}
 
+	epc_page->owner = encl;
+
 	return epc_page;
 }
 
diff --git a/arch/x86/kernel/cpu/sgx/encl.h b/arch/x86/kernel/cpu/sgx/encl.h
index f94ff14c9486..831d63f80f5a 100644
--- a/arch/x86/kernel/cpu/sgx/encl.h
+++ b/arch/x86/kernel/cpu/sgx/encl.h
@@ -116,7 +116,7 @@ struct sgx_encl_page *sgx_encl_page_alloc(struct sgx_encl *encl,
 					  unsigned long offset,
 					  u64 secinfo_flags);
 void sgx_zap_enclave_ptes(struct sgx_encl *encl, unsigned long addr);
-struct sgx_epc_page *sgx_alloc_va_page(bool reclaim);
+struct sgx_epc_page *sgx_alloc_va_page(struct sgx_encl *encl, bool reclaim);
 unsigned int sgx_alloc_va_slot(struct sgx_va_page *va_page);
 void sgx_free_va_slot(struct sgx_va_page *va_page, unsigned int offset);
 bool sgx_va_page_full(struct sgx_va_page *va_page);
diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index ebe79d60619f..9a1bb3c3211a 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -30,7 +30,7 @@ struct sgx_va_page *sgx_encl_grow(struct sgx_encl *encl, bool reclaim)
 		if (!va_page)
 			return ERR_PTR(-ENOMEM);
 
-		va_page->epc_page = sgx_alloc_va_page(reclaim);
+		va_page->epc_page = sgx_alloc_va_page(encl, reclaim);
 		if (IS_ERR(va_page->epc_page)) {
 			err = ERR_CAST(va_page->epc_page);
 			kfree(va_page);
-- 
2.37.3
Re: [RFC PATCH 03/20] x86/sgx: Track owning enclave in VA EPC pages
Posted by Jarkko Sakkinen 3 years, 6 months ago
On Thu, Sep 22, 2022 at 10:10:40AM -0700, Kristen Carlson Accardi wrote:
> From: Sean Christopherson <sean.j.christopherson@intel.com>
> 
> In order to fully account for an enclave's EPC page usage, store
> the owning enclave of a VA EPC page.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
> Cc: Sean Christopherson <seanjc@google.com>

Why this change fully accounts enclave's EPC page usage?

BR, Jarkko
Re: [RFC PATCH 03/20] x86/sgx: Track owning enclave in VA EPC pages
Posted by Dave Hansen 3 years, 6 months ago
On 9/22/22 10:10, Kristen Carlson Accardi wrote:
> -struct sgx_epc_page *sgx_alloc_va_page(bool reclaim)
> +struct sgx_epc_page *sgx_alloc_va_page(struct sgx_encl *encl, bool reclaim)
>  {
>  	struct sgx_epc_page *epc_page;
>  	int ret;
> @@ -1218,6 +1219,8 @@ struct sgx_epc_page *sgx_alloc_va_page(bool reclaim)
>  		return ERR_PTR(-EFAULT);
>  	}
>  
> +	epc_page->owner = encl;
> +
>  	return epc_page;
>  }

BTW, is there a flag or any other way to tell to what kind of object
->owner points?
Re: [RFC PATCH 03/20] x86/sgx: Track owning enclave in VA EPC pages
Posted by Kristen Carlson Accardi 3 years, 6 months ago
On Thu, 2022-09-22 at 11:55 -0700, Dave Hansen wrote:
> On 9/22/22 10:10, Kristen Carlson Accardi wrote:
> > -struct sgx_epc_page *sgx_alloc_va_page(bool reclaim)
> > +struct sgx_epc_page *sgx_alloc_va_page(struct sgx_encl *encl, bool
> > reclaim)
> >  {
> >         struct sgx_epc_page *epc_page;
> >         int ret;
> > @@ -1218,6 +1219,8 @@ struct sgx_epc_page *sgx_alloc_va_page(bool
> > reclaim)
> >                 return ERR_PTR(-EFAULT);
> >         }
> >  
> > +       epc_page->owner = encl;
> > +
> >         return epc_page;
> >  }
> 
> BTW, is there a flag or any other way to tell to what kind of object
> ->owner points?

The owner will only be an sgx_encl type if it is a va page, so to tell
what kind of object owner is, you look at the epc page flags - like
this:
        if (epc_page->flags & SGX_EPC_PAGE_ENCLAVE)
                encl = ((struct sgx_encl_page *)epc_page->owner)->encl;
        else if (epc_page->flags & SGX_EPC_PAGE_VERSION_ARRAY)
                encl = epc_page->owner;
...
Re: [RFC PATCH 03/20] x86/sgx: Track owning enclave in VA EPC pages
Posted by Dave Hansen 3 years, 6 months ago
On 9/22/22 13:04, Kristen Carlson Accardi wrote:
>> BTW, is there a flag or any other way to tell to what kind of object
>> ->owner points?
> The owner will only be an sgx_encl type if it is a va page, so to tell
> what kind of object owner is, you look at the epc page flags - like
> this:
>         if (epc_page->flags & SGX_EPC_PAGE_ENCLAVE)
>                 encl = ((struct sgx_encl_page *)epc_page->owner)->encl;
>         else if (epc_page->flags & SGX_EPC_PAGE_VERSION_ARRAY)
>                 encl = epc_page->owner;
> ...

I don't know how much refactoring it would take, but it would be nice if
that was a bit more obvious.  Basically, can we get the code that checks
for or sets SGX_EPC_PAGE_VERSION_ARRAY close to the code that assigns or
reads ->owner?