[intel-sgx-kernel-dev] intel_sgx: filter target CPUs via mm_cpumask when sending IPIs
diff mbox

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

Commit Message

Sean Christopherson March 17, 2017, 6:47 p.m. UTC
Use mm_cpumask to filter out CPUs that cannot possibly be running in the
enclave to avoid sending unnecessary IPIs when forcing CPUs to exit the
enclave.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 drivers/platform/x86/intel_sgx_page_cache.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

Comments

Jarkko Sakkinen March 17, 2017, 8:23 p.m. UTC | #1
On Fri, Mar 17, 2017 at 11:47:26AM -0700, Sean Christopherson wrote:
> Use mm_cpumask to filter out CPUs that cannot possibly be running in the
> enclave to avoid sending unnecessary IPIs when forcing CPUs to exit the
> enclave.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>

Does not look bad. I'll look into testing this at some point next
week.

/Jarkko

> ---
>  drivers/platform/x86/intel_sgx_page_cache.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c
> index e4f6b95..dfd0988 100644
> --- a/drivers/platform/x86/intel_sgx_page_cache.c
> +++ b/drivers/platform/x86/intel_sgx_page_cache.c
> @@ -219,6 +219,18 @@ static void sgx_ipi_cb(void *info)
>  {
>  }
>  
> +/**
> + * sgx_flush_cpus() - Force all CPUs to exit the enclave
> + * @encl:	enclave to flush
> + *
> + * Send IPIs to CPUs currently executing in the enclave's memory
> + * context to force them to exit the enclave.
> + */
> +static void sgx_flush_cpus(struct sgx_encl *encl)
> +{
> +	on_each_cpu_mask(mm_cpumask(encl->mm), sgx_ipi_cb, NULL, 1);
> +}
> +
>  static void sgx_eblock(struct sgx_encl *encl,
>  		       struct sgx_epc_page *epc_page)
>  {
> @@ -232,7 +244,7 @@ static void sgx_eblock(struct sgx_encl *encl,
>  	if (ret) {
>  		sgx_crit(encl, "EBLOCK returned %d\n", ret);
>  		sgx_invalidate(encl);
> -		smp_call_function(sgx_ipi_cb, NULL, 1);
> +		sgx_flush_cpus(encl);
>  	}
>  
>  }
> @@ -249,7 +261,7 @@ static void sgx_etrack(struct sgx_encl *encl)
>  	if (ret) {
>  		sgx_crit(encl, "ETRACK returned %d\n", ret);
>  		sgx_invalidate(encl);
> -		smp_call_function(sgx_ipi_cb, NULL, 1);
> +		sgx_flush_cpus(encl);
>  	}
>  }
>  
> @@ -310,14 +322,14 @@ static bool sgx_ewb(struct sgx_encl *encl,
>  
>  	if (ret == SGX_NOT_TRACKED) {
>  		/* slow path, IPI needed */
> -		smp_call_function(sgx_ipi_cb, NULL, 1);
> +		sgx_flush_cpus(encl);
>  		ret = __sgx_ewb(encl, entry);
>  	}
>  
>  	if (ret) {
>  		/* make enclave inaccessible */
>  		sgx_invalidate(encl);
> -		smp_call_function(sgx_ipi_cb, NULL, 1);
> +		sgx_flush_cpus(encl);
>  		if (ret > 0)
>  			sgx_err(encl, "EWB returned %d, enclave killed\n", ret);
>  		return false;
> -- 
> 2.7.4
> 
> _______________________________________________
> intel-sgx-kernel-dev mailing list
> intel-sgx-kernel-dev@lists.01.org
> https://lists.01.org/mailman/listinfo/intel-sgx-kernel-dev
Jarkko Sakkinen April 4, 2017, 5:09 p.m. UTC | #2
On Fri, Mar 17, 2017 at 11:47:26AM -0700, Sean Christopherson wrote:
> Use mm_cpumask to filter out CPUs that cannot possibly be running in the
> enclave to avoid sending unnecessary IPIs when forcing CPUs to exit the
> enclave.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>

I don't see why this couldn't merged for v1 driver.

> ---
>  drivers/platform/x86/intel_sgx_page_cache.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c
> index e4f6b95..dfd0988 100644
> --- a/drivers/platform/x86/intel_sgx_page_cache.c
> +++ b/drivers/platform/x86/intel_sgx_page_cache.c
> @@ -219,6 +219,18 @@ static void sgx_ipi_cb(void *info)
>  {
>  }
>  
> +/**
> + * sgx_flush_cpus() - Force all CPUs to exit the enclave
> + * @encl:	enclave to flush
> + *
> + * Send IPIs to CPUs currently executing in the enclave's memory
> + * context to force them to exit the enclave.
> + */

I might rip this comment away, though as it does not have much value.

> +static void sgx_flush_cpus(struct sgx_encl *encl)
> +{
> +	on_each_cpu_mask(mm_cpumask(encl->mm), sgx_ipi_cb, NULL, 1);
> +}
> +
>  static void sgx_eblock(struct sgx_encl *encl,
>  		       struct sgx_epc_page *epc_page)
>  {
> @@ -232,7 +244,7 @@ static void sgx_eblock(struct sgx_encl *encl,
>  	if (ret) {
>  		sgx_crit(encl, "EBLOCK returned %d\n", ret);
>  		sgx_invalidate(encl);
> -		smp_call_function(sgx_ipi_cb, NULL, 1);
> +		sgx_flush_cpus(encl);
>  	}
>  
>  }
> @@ -249,7 +261,7 @@ static void sgx_etrack(struct sgx_encl *encl)
>  	if (ret) {
>  		sgx_crit(encl, "ETRACK returned %d\n", ret);
>  		sgx_invalidate(encl);
> -		smp_call_function(sgx_ipi_cb, NULL, 1);
> +		sgx_flush_cpus(encl);
>  	}
>  }
>  
> @@ -310,14 +322,14 @@ static bool sgx_ewb(struct sgx_encl *encl,
>  
>  	if (ret == SGX_NOT_TRACKED) {
>  		/* slow path, IPI needed */
> -		smp_call_function(sgx_ipi_cb, NULL, 1);
> +		sgx_flush_cpus(encl);
>  		ret = __sgx_ewb(encl, entry);
>  	}
>  
>  	if (ret) {
>  		/* make enclave inaccessible */
>  		sgx_invalidate(encl);
> -		smp_call_function(sgx_ipi_cb, NULL, 1);
> +		sgx_flush_cpus(encl);
>  		if (ret > 0)
>  			sgx_err(encl, "EWB returned %d, enclave killed\n", ret);
>  		return false;
> -- 
> 2.7.4
> 
> _______________________________________________
> intel-sgx-kernel-dev mailing list
> intel-sgx-kernel-dev@lists.01.org
> https://lists.01.org/mailman/listinfo/intel-sgx-kernel-dev

I'll push this and test it and squash it if it works.

/Jarkko

Patch
diff mbox

diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c
index e4f6b95..dfd0988 100644
--- a/drivers/platform/x86/intel_sgx_page_cache.c
+++ b/drivers/platform/x86/intel_sgx_page_cache.c
@@ -219,6 +219,18 @@  static void sgx_ipi_cb(void *info)
 {
 }
 
+/**
+ * sgx_flush_cpus() - Force all CPUs to exit the enclave
+ * @encl:	enclave to flush
+ *
+ * Send IPIs to CPUs currently executing in the enclave's memory
+ * context to force them to exit the enclave.
+ */
+static void sgx_flush_cpus(struct sgx_encl *encl)
+{
+	on_each_cpu_mask(mm_cpumask(encl->mm), sgx_ipi_cb, NULL, 1);
+}
+
 static void sgx_eblock(struct sgx_encl *encl,
 		       struct sgx_epc_page *epc_page)
 {
@@ -232,7 +244,7 @@  static void sgx_eblock(struct sgx_encl *encl,
 	if (ret) {
 		sgx_crit(encl, "EBLOCK returned %d\n", ret);
 		sgx_invalidate(encl);
-		smp_call_function(sgx_ipi_cb, NULL, 1);
+		sgx_flush_cpus(encl);
 	}
 
 }
@@ -249,7 +261,7 @@  static void sgx_etrack(struct sgx_encl *encl)
 	if (ret) {
 		sgx_crit(encl, "ETRACK returned %d\n", ret);
 		sgx_invalidate(encl);
-		smp_call_function(sgx_ipi_cb, NULL, 1);
+		sgx_flush_cpus(encl);
 	}
 }
 
@@ -310,14 +322,14 @@  static bool sgx_ewb(struct sgx_encl *encl,
 
 	if (ret == SGX_NOT_TRACKED) {
 		/* slow path, IPI needed */
-		smp_call_function(sgx_ipi_cb, NULL, 1);
+		sgx_flush_cpus(encl);
 		ret = __sgx_ewb(encl, entry);
 	}
 
 	if (ret) {
 		/* make enclave inaccessible */
 		sgx_invalidate(encl);
-		smp_call_function(sgx_ipi_cb, NULL, 1);
+		sgx_flush_cpus(encl);
 		if (ret > 0)
 			sgx_err(encl, "EWB returned %d, enclave killed\n", ret);
 		return false;