diff mbox

[intel-sgx-kernel-dev] intel_sgx: clean up EPC bank code

Message ID 20170614162811.28716-1-jarkko.sakkinen@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jarkko Sakkinen June 14, 2017, 4:28 p.m. UTC 
Removed redundant probing code and cleaned up style issues in code for
multiple EPC banks.

Reported-by: Dmitrii Kuvaiskii <Dmitrii.Kuvaiskii@tu-dresden.de>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/platform/x86/intel_sgx/sgx.h            |   6 +-
 drivers/platform/x86/intel_sgx/sgx_main.c       | 112 ++++++------------------
 drivers/platform/x86/intel_sgx/sgx_page_cache.c |  13 +--
 3 files changed, 37 insertions(+), 94 deletions(-)

Comments

Jarkko Sakkinen June 15, 2017, 9:01 p.m. UTC | #1 
squashed

/Jarkko

On Wed, Jun 14, 2017 at 06:28:11PM +0200, Jarkko Sakkinen wrote:
> Removed redundant probing code and cleaned up style issues in code for
> multiple EPC banks.
> 
> Reported-by: Dmitrii Kuvaiskii <Dmitrii.Kuvaiskii@tu-dresden.de>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  drivers/platform/x86/intel_sgx/sgx.h            |   6 +-
>  drivers/platform/x86/intel_sgx/sgx_main.c       | 112 ++++++------------------
>  drivers/platform/x86/intel_sgx/sgx_page_cache.c |  13 +--
>  3 files changed, 37 insertions(+), 94 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel_sgx/sgx.h b/drivers/platform/x86/intel_sgx/sgx.h
> index d5cdf962219d..3ccbc0ffbabc 100644
> --- a/drivers/platform/x86/intel_sgx/sgx.h
> +++ b/drivers/platform/x86/intel_sgx/sgx.h
> @@ -149,11 +149,11 @@ struct sgx_encl {
>  };
>  
>  struct sgx_epc_bank {
> +	unsigned long pa;
>  #ifdef CONFIG_X86_64
> -	void *mem;
> +	unsigned long va;
>  #endif
> -	unsigned long start;
> -	unsigned long end;
> +	unsigned long size;
>  };
>  
>  extern struct workqueue_struct *sgx_add_page_wq;
> diff --git a/drivers/platform/x86/intel_sgx/sgx_main.c b/drivers/platform/x86/intel_sgx/sgx_main.c
> index 98ee05de8db8..cf1e6ec2cd7e 100644
> --- a/drivers/platform/x86/intel_sgx/sgx_main.c
> +++ b/drivers/platform/x86/intel_sgx/sgx_main.c
> @@ -161,78 +161,6 @@ static struct miscdevice sgx_dev = {
>  	.mode   = 0666,
>  };
>  
> -static int sgx_init_platform(void)
> -{
> -	unsigned int eax, ebx, ecx, edx;
> -	unsigned long size;
> -	int i;
> -
> -	cpuid(0, &eax, &ebx, &ecx, &edx);
> -	if (eax < SGX_CPUID) {
> -		pr_err("intel_sgx: CPUID is missing the SGX leaf instruction\n");
> -		return -ENODEV;
> -	}
> -
> -	if (!boot_cpu_has(X86_FEATURE_SGX)) {
> -		pr_err("intel_sgx: CPU is missing the SGX feature\n");
> -		return -ENODEV;
> -	}
> -
> -	cpuid_count(SGX_CPUID, 0x0, &eax, &ebx, &ecx, &edx);
> -	if (!(eax & 1)) {
> -		pr_err("intel_sgx: CPU does not support the SGX 1.0 instruction set\n");
> -		return -ENODEV;
> -	}
> -
> -	if (boot_cpu_has(X86_FEATURE_OSXSAVE)) {
> -		cpuid_count(SGX_CPUID, 0x1, &eax, &ebx, &ecx, &edx);
> -		sgx_xfrm_mask = (((u64)edx) << 32) + (u64)ecx;
> -		for (i = 2; i < 64; i++) {
> -			cpuid_count(0x0D, i, &eax, &ebx, &ecx, &edx);
> -			if ((1 << i) & sgx_xfrm_mask)
> -				sgx_ssaframesize_tbl[i] =
> -					(168 + eax + ebx + PAGE_SIZE - 1) /
> -					PAGE_SIZE;
> -		}
> -	}
> -
> -	cpuid_count(SGX_CPUID, 0x0, &eax, &ebx, &ecx, &edx);
> -	if (edx & 0xFFFF) {
> -#ifdef CONFIG_X86_64
> -		sgx_encl_size_max_64 = 1ULL << (edx & 0xFF);
> -#endif
> -		sgx_encl_size_max_32 = 1ULL << ((edx >> 8) & 0xFF);
> -	}
> -
> -	sgx_nr_epc_banks = 0;
> -	do {
> -		cpuid_count(SGX_CPUID, sgx_nr_epc_banks + 2,
> -				&eax, &ebx, &ecx, &edx);
> -		if (eax & 0xf) {
> -			sgx_epc_banks[sgx_nr_epc_banks].start =
> -				(((u64) (ebx & 0xfffff)) << 32) +
> -				(u64) (eax & 0xfffff000);
> -			size = (((u64) (edx & 0xfffff)) << 32) +
> -				(u64) (ecx & 0xfffff000);
> -			sgx_epc_banks[sgx_nr_epc_banks].end =
> -				sgx_epc_banks[sgx_nr_epc_banks].start + size;
> -			if (!sgx_epc_banks[sgx_nr_epc_banks].start)
> -				return -ENODEV;
> -			sgx_nr_epc_banks++;
> -		} else {
> -			break;
> -		}
> -	} while (sgx_nr_epc_banks < SGX_MAX_EPC_BANKS);
> -
> -	/* There should be at least one EPC area or something is wrong. */
> -	if (!sgx_nr_epc_banks) {
> -		WARN_ON(1);
> -		return 1;
> -	}
> -
> -	return 0;
> -}
> -
>  static int sgx_pm_suspend(struct device *dev)
>  {
>  	struct sgx_tgid_ctx *ctx;
> @@ -262,6 +190,9 @@ static SIMPLE_DEV_PM_OPS(sgx_drv_pm, sgx_pm_suspend, sgx_pm_resume);
>  
>  static int sgx_dev_init(struct device *dev)
>  {
> +	unsigned int eax, ebx, ecx, edx;
> +	unsigned long pa;
> +	unsigned long size;
>  	unsigned int wq_flags;
>  	int ret;
>  	int i;
> @@ -271,28 +202,37 @@ static int sgx_dev_init(struct device *dev)
>  	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
>  		return -ENODEV;
>  
> -	ret = sgx_init_platform();
> -	if (ret)
> -		return ret;
> +	for (i = 0; i < SGX_MAX_EPC_BANKS; i++) {
> +		cpuid_count(SGX_CPUID, i + 2, &eax, &ebx, &ecx, &edx);
> +		if (!(eax & 0xf))
> +			break;
> +
> +		pa = ((u64)(ebx & 0xfffff) << 32) + (u64)(eax & 0xfffff000);
> +		size = ((u64)(edx & 0xfffff) << 32) + (u64)(ecx & 0xfffff000);
> +
> +		dev_info(dev, "EPC bank 0x%lx-0x%lx\n", pa, pa + size);
> +
> +		sgx_epc_banks[i].pa = pa;
> +		sgx_epc_banks[i].size = size;
> +	}
>  
> -	pr_info("intel_sgx: Number of EPCs %d\n", sgx_nr_epc_banks);
> +	sgx_nr_epc_banks = i;
>  
>  	for (i = 0; i < sgx_nr_epc_banks; i++) {
> -		pr_info("intel_sgx: EPC memory range 0x%lx-0x%lx\n",
> -			sgx_epc_banks[i].start, sgx_epc_banks[i].end);
>  #ifdef CONFIG_X86_64
> -		sgx_epc_banks[i].mem = ioremap_cache(sgx_epc_banks[i].start,
> -			sgx_epc_banks[i].end - sgx_epc_banks[i].start);
> -		if (!sgx_epc_banks[i].mem) {
> +		sgx_epc_banks[i].va = (unsigned long)
> +			ioremap_cache(sgx_epc_banks[i].pa,
> +				      sgx_epc_banks[i].size);
> +		if (!sgx_epc_banks[i].va) {
>  			sgx_nr_epc_banks = i;
>  			ret = -ENOMEM;
>  			goto out_iounmap;
>  		}
>  #endif
> -		ret = sgx_add_epc_bank(sgx_epc_banks[i].start,
> -			sgx_epc_banks[i].end - sgx_epc_banks[i].start);
> +		ret = sgx_add_epc_bank(sgx_epc_banks[i].pa,
> +				       sgx_epc_banks[i].size);
>  		if (ret) {
> -			sgx_nr_epc_banks = i+1;
> +			sgx_nr_epc_banks = i + 1;
>  			goto out_iounmap;
>  		}
>  	}
> @@ -325,7 +265,7 @@ static int sgx_dev_init(struct device *dev)
>  out_iounmap:
>  #ifdef CONFIG_X86_64
>  	for (i = 0; i < sgx_nr_epc_banks; i++)
> -		iounmap(sgx_epc_banks[i].mem);
> +		iounmap((void *)sgx_epc_banks[i].va);
>  #endif
>  	return ret;
>  }
> @@ -388,7 +328,7 @@ static int sgx_drv_remove(struct platform_device *pdev)
>  	destroy_workqueue(sgx_add_page_wq);
>  #ifdef CONFIG_X86_64
>  	for (i = 0; i < sgx_nr_epc_banks; i++)
> -		iounmap(sgx_epc_banks[i].mem);
> +		iounmap((void *)sgx_epc_banks[i].va);
>  #endif
>  	sgx_page_cache_teardown();
>  
> diff --git a/drivers/platform/x86/intel_sgx/sgx_page_cache.c b/drivers/platform/x86/intel_sgx/sgx_page_cache.c
> index 8e01427eea39..3ace84724273 100644
> --- a/drivers/platform/x86/intel_sgx/sgx_page_cache.c
> +++ b/drivers/platform/x86/intel_sgx/sgx_page_cache.c
> @@ -546,14 +546,17 @@ void *sgx_get_page(struct sgx_epc_page *entry)
>  #ifdef CONFIG_X86_32
>  	return kmap_atomic_pfn(PFN_DOWN(entry->pa));
>  #else
> +	unsigned long offset;
>  	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);
> -		}
> +		if (entry->pa < sgx_epc_banks[i].pa)
> +			continue;
> +
> +		offset = entry->pa - sgx_epc_banks[i].pa;
> +
> +		if (offset < sgx_epc_banks[i].size)
> +			return (void *)(sgx_epc_banks[i].va + offset);
>  	}
>  
>  	return NULL;
> -- 
> 2.11.0
>
diff mbox

Patch

diff --git a/drivers/platform/x86/intel_sgx/sgx.h b/drivers/platform/x86/intel_sgx/sgx.h
index d5cdf962219d..3ccbc0ffbabc 100644
--- a/drivers/platform/x86/intel_sgx/sgx.h
+++ b/drivers/platform/x86/intel_sgx/sgx.h
@@ -149,11 +149,11 @@  struct sgx_encl {
 };
 
 struct sgx_epc_bank {
+	unsigned long pa;
 #ifdef CONFIG_X86_64
-	void *mem;
+	unsigned long va;
 #endif
-	unsigned long start;
-	unsigned long end;
+	unsigned long size;
 };
 
 extern struct workqueue_struct *sgx_add_page_wq;
diff --git a/drivers/platform/x86/intel_sgx/sgx_main.c b/drivers/platform/x86/intel_sgx/sgx_main.c
index 98ee05de8db8..cf1e6ec2cd7e 100644
--- a/drivers/platform/x86/intel_sgx/sgx_main.c
+++ b/drivers/platform/x86/intel_sgx/sgx_main.c
@@ -161,78 +161,6 @@  static struct miscdevice sgx_dev = {
 	.mode   = 0666,
 };
 
-static int sgx_init_platform(void)
-{
-	unsigned int eax, ebx, ecx, edx;
-	unsigned long size;
-	int i;
-
-	cpuid(0, &eax, &ebx, &ecx, &edx);
-	if (eax < SGX_CPUID) {
-		pr_err("intel_sgx: CPUID is missing the SGX leaf instruction\n");
-		return -ENODEV;
-	}
-
-	if (!boot_cpu_has(X86_FEATURE_SGX)) {
-		pr_err("intel_sgx: CPU is missing the SGX feature\n");
-		return -ENODEV;
-	}
-
-	cpuid_count(SGX_CPUID, 0x0, &eax, &ebx, &ecx, &edx);
-	if (!(eax & 1)) {
-		pr_err("intel_sgx: CPU does not support the SGX 1.0 instruction set\n");
-		return -ENODEV;
-	}
-
-	if (boot_cpu_has(X86_FEATURE_OSXSAVE)) {
-		cpuid_count(SGX_CPUID, 0x1, &eax, &ebx, &ecx, &edx);
-		sgx_xfrm_mask = (((u64)edx) << 32) + (u64)ecx;
-		for (i = 2; i < 64; i++) {
-			cpuid_count(0x0D, i, &eax, &ebx, &ecx, &edx);
-			if ((1 << i) & sgx_xfrm_mask)
-				sgx_ssaframesize_tbl[i] =
-					(168 + eax + ebx + PAGE_SIZE - 1) /
-					PAGE_SIZE;
-		}
-	}
-
-	cpuid_count(SGX_CPUID, 0x0, &eax, &ebx, &ecx, &edx);
-	if (edx & 0xFFFF) {
-#ifdef CONFIG_X86_64
-		sgx_encl_size_max_64 = 1ULL << (edx & 0xFF);
-#endif
-		sgx_encl_size_max_32 = 1ULL << ((edx >> 8) & 0xFF);
-	}
-
-	sgx_nr_epc_banks = 0;
-	do {
-		cpuid_count(SGX_CPUID, sgx_nr_epc_banks + 2,
-				&eax, &ebx, &ecx, &edx);
-		if (eax & 0xf) {
-			sgx_epc_banks[sgx_nr_epc_banks].start =
-				(((u64) (ebx & 0xfffff)) << 32) +
-				(u64) (eax & 0xfffff000);
-			size = (((u64) (edx & 0xfffff)) << 32) +
-				(u64) (ecx & 0xfffff000);
-			sgx_epc_banks[sgx_nr_epc_banks].end =
-				sgx_epc_banks[sgx_nr_epc_banks].start + size;
-			if (!sgx_epc_banks[sgx_nr_epc_banks].start)
-				return -ENODEV;
-			sgx_nr_epc_banks++;
-		} else {
-			break;
-		}
-	} while (sgx_nr_epc_banks < SGX_MAX_EPC_BANKS);
-
-	/* There should be at least one EPC area or something is wrong. */
-	if (!sgx_nr_epc_banks) {
-		WARN_ON(1);
-		return 1;
-	}
-
-	return 0;
-}
-
 static int sgx_pm_suspend(struct device *dev)
 {
 	struct sgx_tgid_ctx *ctx;
@@ -262,6 +190,9 @@  static SIMPLE_DEV_PM_OPS(sgx_drv_pm, sgx_pm_suspend, sgx_pm_resume);
 
 static int sgx_dev_init(struct device *dev)
 {
+	unsigned int eax, ebx, ecx, edx;
+	unsigned long pa;
+	unsigned long size;
 	unsigned int wq_flags;
 	int ret;
 	int i;
@@ -271,28 +202,37 @@  static int sgx_dev_init(struct device *dev)
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
 		return -ENODEV;
 
-	ret = sgx_init_platform();
-	if (ret)
-		return ret;
+	for (i = 0; i < SGX_MAX_EPC_BANKS; i++) {
+		cpuid_count(SGX_CPUID, i + 2, &eax, &ebx, &ecx, &edx);
+		if (!(eax & 0xf))
+			break;
+
+		pa = ((u64)(ebx & 0xfffff) << 32) + (u64)(eax & 0xfffff000);
+		size = ((u64)(edx & 0xfffff) << 32) + (u64)(ecx & 0xfffff000);
+
+		dev_info(dev, "EPC bank 0x%lx-0x%lx\n", pa, pa + size);
+
+		sgx_epc_banks[i].pa = pa;
+		sgx_epc_banks[i].size = size;
+	}
 
-	pr_info("intel_sgx: Number of EPCs %d\n", sgx_nr_epc_banks);
+	sgx_nr_epc_banks = i;
 
 	for (i = 0; i < sgx_nr_epc_banks; i++) {
-		pr_info("intel_sgx: EPC memory range 0x%lx-0x%lx\n",
-			sgx_epc_banks[i].start, sgx_epc_banks[i].end);
 #ifdef CONFIG_X86_64
-		sgx_epc_banks[i].mem = ioremap_cache(sgx_epc_banks[i].start,
-			sgx_epc_banks[i].end - sgx_epc_banks[i].start);
-		if (!sgx_epc_banks[i].mem) {
+		sgx_epc_banks[i].va = (unsigned long)
+			ioremap_cache(sgx_epc_banks[i].pa,
+				      sgx_epc_banks[i].size);
+		if (!sgx_epc_banks[i].va) {
 			sgx_nr_epc_banks = i;
 			ret = -ENOMEM;
 			goto out_iounmap;
 		}
 #endif
-		ret = sgx_add_epc_bank(sgx_epc_banks[i].start,
-			sgx_epc_banks[i].end - sgx_epc_banks[i].start);
+		ret = sgx_add_epc_bank(sgx_epc_banks[i].pa,
+				       sgx_epc_banks[i].size);
 		if (ret) {
-			sgx_nr_epc_banks = i+1;
+			sgx_nr_epc_banks = i + 1;
 			goto out_iounmap;
 		}
 	}
@@ -325,7 +265,7 @@  static int sgx_dev_init(struct device *dev)
 out_iounmap:
 #ifdef CONFIG_X86_64
 	for (i = 0; i < sgx_nr_epc_banks; i++)
-		iounmap(sgx_epc_banks[i].mem);
+		iounmap((void *)sgx_epc_banks[i].va);
 #endif
 	return ret;
 }
@@ -388,7 +328,7 @@  static int sgx_drv_remove(struct platform_device *pdev)
 	destroy_workqueue(sgx_add_page_wq);
 #ifdef CONFIG_X86_64
 	for (i = 0; i < sgx_nr_epc_banks; i++)
-		iounmap(sgx_epc_banks[i].mem);
+		iounmap((void *)sgx_epc_banks[i].va);
 #endif
 	sgx_page_cache_teardown();
 
diff --git a/drivers/platform/x86/intel_sgx/sgx_page_cache.c b/drivers/platform/x86/intel_sgx/sgx_page_cache.c
index 8e01427eea39..3ace84724273 100644
--- a/drivers/platform/x86/intel_sgx/sgx_page_cache.c
+++ b/drivers/platform/x86/intel_sgx/sgx_page_cache.c
@@ -546,14 +546,17 @@  void *sgx_get_page(struct sgx_epc_page *entry)
 #ifdef CONFIG_X86_32
 	return kmap_atomic_pfn(PFN_DOWN(entry->pa));
 #else
+	unsigned long offset;
 	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);
-		}
+		if (entry->pa < sgx_epc_banks[i].pa)
+			continue;
+
+		offset = entry->pa - sgx_epc_banks[i].pa;
+
+		if (offset < sgx_epc_banks[i].size)
+			return (void *)(sgx_epc_banks[i].va + offset);
 	}
 
 	return NULL;