[intel-sgx-kernel-dev] intel_sgx: use offset bits of EPC page to store its bank
diff mbox

Message ID 1492018856-5706-1-git-send-email-sean.j.christopherson@intel.com
State New
Headers show

Commit Message

Sean Christopherson April 12, 2017, 5:40 p.m. UTC
Store the bank index of an EPC page in the unused page offset of the
EPC address.  This allows sgx_get_epc_page to calculate the virtual
address of an EPC page in constant time (for x64-86) and without any
branches.  Rename sgx_epc_page's pa to addr to reflect that it tracks
more than just the PA and add macros to get the PA and BANK.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/include/asm/sgx.h                      |  7 +++++--
 drivers/platform/x86/intel_sgx/sgx.h            |  2 +-
 drivers/platform/x86/intel_sgx/sgx_ioctl.c      |  2 +-
 drivers/platform/x86/intel_sgx/sgx_main.c       |  2 +-
 drivers/platform/x86/intel_sgx/sgx_page_cache.c | 19 +++++--------------
 drivers/platform/x86/intel_sgx/sgx_util.c       |  2 +-
 6 files changed, 14 insertions(+), 20 deletions(-)

Comments

Jarkko Sakkinen April 13, 2017, 12:43 p.m. UTC | #1
On Wed, Apr 12, 2017 at 10:40:56AM -0700, Sean Christopherson wrote:
> Store the bank index of an EPC page in the unused page offset of the
> EPC address.  This allows sgx_get_epc_page to calculate the virtual
> address of an EPC page in constant time (for x64-86) and without any
> branches.  Rename sgx_epc_page's pa to addr to reflect that it tracks
> more than just the PA and add macros to get the PA and BANK.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>

Thanks. This is obviously better way to do this.

> ---
>  arch/x86/include/asm/sgx.h                      |  7 +++++--
>  drivers/platform/x86/intel_sgx/sgx.h            |  2 +-
>  drivers/platform/x86/intel_sgx/sgx_ioctl.c      |  2 +-
>  drivers/platform/x86/intel_sgx/sgx_main.c       |  2 +-
>  drivers/platform/x86/intel_sgx/sgx_page_cache.c | 19 +++++--------------
>  drivers/platform/x86/intel_sgx/sgx_util.c       |  2 +-
>  6 files changed, 14 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
> index 233d3a6..d8706f6 100644
> --- a/arch/x86/include/asm/sgx.h
> +++ b/arch/x86/include/asm/sgx.h
> @@ -342,10 +342,13 @@ static inline int __emodt(struct sgx_secinfo *secinfo, void *epc)
>  struct sgx_encl;
>  
>  struct sgx_epc_page {
> -	resource_size_t	pa;
> -	struct list_head free_list;
> +	resource_size_t     addr;
> +	struct list_head    free_list;
>  };
>  
> +#define EPC_PA(entry) ((entry)->addr & PAGE_MASK)
> +#define EPC_BANK(entry) ((entry)->addr & ~PAGE_MASK)

- Do not align struct fields. It's a pain to maintain.
- Do not rename the field.
- Remove these macros.

/Jarkko

Patch
diff mbox

diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
index 233d3a6..d8706f6 100644
--- a/arch/x86/include/asm/sgx.h
+++ b/arch/x86/include/asm/sgx.h
@@ -342,10 +342,13 @@  static inline int __emodt(struct sgx_secinfo *secinfo, void *epc)
 struct sgx_encl;
 
 struct sgx_epc_page {
-	resource_size_t	pa;
-	struct list_head free_list;
+	resource_size_t     addr;
+	struct list_head    free_list;
 };
 
+#define EPC_PA(entry) ((entry)->addr & PAGE_MASK)
+#define EPC_BANK(entry) ((entry)->addr & ~PAGE_MASK)
+
 extern struct sgx_epc_page *sgx_alloc_page(unsigned int flags);
 extern int sgx_free_page(struct sgx_epc_page *entry, struct sgx_encl *encl);
 extern void *sgx_get_page(struct sgx_epc_page *entry);
diff --git a/drivers/platform/x86/intel_sgx/sgx.h b/drivers/platform/x86/intel_sgx/sgx.h
index 82355bb..8c0e19a 100644
--- a/drivers/platform/x86/intel_sgx/sgx.h
+++ b/drivers/platform/x86/intel_sgx/sgx.h
@@ -222,7 +222,7 @@  enum sgx_alloc_flags {
 };
 
 int ksgxswapd(void *p);
-int sgx_page_cache_init(resource_size_t start, unsigned long size);
+int sgx_page_cache_init(resource_size_t start, unsigned long size, int epc_bank);
 void sgx_page_cache_teardown(void);
 struct sgx_epc_page *sgx_alloc_page(unsigned int flags);
 int sgx_free_page(struct sgx_epc_page *entry, struct sgx_encl *encl);
diff --git a/drivers/platform/x86/intel_sgx/sgx_ioctl.c b/drivers/platform/x86/intel_sgx/sgx_ioctl.c
index ba7c0d2..cb5cfe1 100644
--- a/drivers/platform/x86/intel_sgx/sgx_ioctl.c
+++ b/drivers/platform/x86/intel_sgx/sgx_ioctl.c
@@ -244,7 +244,7 @@  static bool sgx_process_add_page_req(struct sgx_add_page_req *req)
 		goto out;
 	}
 
-	ret = vm_insert_pfn(vma, encl_page->addr, PFN_DOWN(epc_page->pa));
+	ret = vm_insert_pfn(vma, encl_page->addr, PFN_DOWN(EPC_PA(epc_page)));
 	if (ret)
 		goto out;
 
diff --git a/drivers/platform/x86/intel_sgx/sgx_main.c b/drivers/platform/x86/intel_sgx/sgx_main.c
index b2a1bc4..84db81d 100644
--- a/drivers/platform/x86/intel_sgx/sgx_main.c
+++ b/drivers/platform/x86/intel_sgx/sgx_main.c
@@ -290,7 +290,7 @@  static int sgx_dev_init(struct device *dev)
 		}
 #endif
 		ret = sgx_page_cache_init(sgx_epc_banks[i].start,
-			sgx_epc_banks[i].end - sgx_epc_banks[i].start);
+			sgx_epc_banks[i].end - sgx_epc_banks[i].start, i);
 		if (ret) {
 			sgx_nr_epc_banks = i+1;
 			goto out_iounmap;
diff --git a/drivers/platform/x86/intel_sgx/sgx_page_cache.c b/drivers/platform/x86/intel_sgx/sgx_page_cache.c
index 4ffde27..44a39ae 100644
--- a/drivers/platform/x86/intel_sgx/sgx_page_cache.c
+++ b/drivers/platform/x86/intel_sgx/sgx_page_cache.c
@@ -428,7 +428,7 @@  int ksgxswapd(void *p)
 	return 0;
 }
 
-int sgx_page_cache_init(resource_size_t start, unsigned long size)
+int sgx_page_cache_init(resource_size_t start, unsigned long size, int epc_bank)
 {
 	unsigned long i;
 	struct sgx_epc_page *new_epc_page, *entry;
@@ -438,7 +438,7 @@  int sgx_page_cache_init(resource_size_t start, unsigned long size)
 		new_epc_page = kzalloc(sizeof(*new_epc_page), GFP_KERNEL);
 		if (!new_epc_page)
 			goto err_freelist;
-		new_epc_page->pa = start + i;
+		new_epc_page->addr = (start + i) | epc_bank;
 
 		spin_lock(&sgx_free_list_lock);
 		list_add_tail(&new_epc_page->free_list, &sgx_free_list);
@@ -584,19 +584,10 @@  EXPORT_SYMBOL(sgx_free_page);
 void *sgx_get_page(struct sgx_epc_page *entry)
 {
 #ifdef CONFIG_X86_32
-	return kmap_atomic_pfn(PFN_DOWN(entry->pa));
+	return kmap_atomic_pfn(PFN_DOWN(EPC_PA(entry)));
 #else
-	int i;
-
-	for (i = 0; i < sgx_nr_epc_banks; i++) {
-		if (entry->pa < sgx_epc_banks[i].end &&
-		    entry->pa >= sgx_epc_banks[i].start) {
-			return sgx_epc_banks[i].mem +
-				(entry->pa - sgx_epc_banks[i].start);
-		}
-	}
-
-	return NULL;
+	int i = EPC_BANK(entry);
+	return sgx_epc_banks[i].mem + (EPC_PA(entry) - sgx_epc_banks[i].start);
 #endif
 }
 EXPORT_SYMBOL(sgx_get_page);
diff --git a/drivers/platform/x86/intel_sgx/sgx_util.c b/drivers/platform/x86/intel_sgx/sgx_util.c
index 25e949d..315f4ea 100644
--- a/drivers/platform/x86/intel_sgx/sgx_util.c
+++ b/drivers/platform/x86/intel_sgx/sgx_util.c
@@ -315,7 +315,7 @@  static struct sgx_encl_page *sgx_do_fault(struct vm_area_struct *vma,
 	if (rc)
 		goto out;
 
-	rc = vm_insert_pfn(vma, entry->addr, PFN_DOWN(epc_page->pa));
+	rc = vm_insert_pfn(vma, entry->addr, PFN_DOWN(EPC_PA(epc_page)));
 	if (rc)
 		goto out;