All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/4] x86/sgx: add struct sgx_vepc_page to manage EPC pages for vepc
@ 2022-05-10  3:17 Zhiquan Li
  2022-05-11 10:32 ` Jarkko Sakkinen
  0 siblings, 1 reply; 3+ messages in thread
From: Zhiquan Li @ 2022-05-10  3:17 UTC (permalink / raw)
  To: linux-sgx, tony.luck; +Cc: jarkko, dave.hansen, seanjc, fan.du, zhiquan1.li

Current SGX data structures are insufficient to track the EPC pages for
vepc. For example, if we want to retrieve the virtual address of an EPC
page allocated to an enclave on host, we can find this info from its
owner, the ‘desc’ field of struct sgx_encl_page. However, if the EPC
page is allocated to a KVM guest, this is not available, as their owner
is a shared vepc.

So, we introduce struct sgx_vepc_page which can be the owner of EPC
pages for vepc and saves the useful info of EPC pages for vepc, like
struct sgx_encl_page.

Canonical memory failure collects victim tasks by iterating all the
tasks one by one and use reverse mapping to get victim tasks’ virtual
address.

This is not necessary for SGX - as one EPC page can be mapped to ONE
enclave only. So, this 1:1 mapping enforcement allows us to find task
virtual address with physical address directly.

Signed-off-by: Zhiquan Li <zhiquan1.li@intel.com>
---
 arch/x86/kernel/cpu/sgx/sgx.h  |  7 +++++++
 arch/x86/kernel/cpu/sgx/virt.c | 24 +++++++++++++++++++-----
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h
index 83ff8c3e81cf..cc01d992453a 100644
--- a/arch/x86/kernel/cpu/sgx/sgx.h
+++ b/arch/x86/kernel/cpu/sgx/sgx.h
@@ -28,6 +28,8 @@
 
 /* Pages on free list */
 #define SGX_EPC_PAGE_IS_FREE		BIT(1)
+/* Pages is used by VM guest */
+#define SGX_EPC_PAGE_IS_VEPC		BIT(2)
 
 struct sgx_epc_page {
 	unsigned int section;
@@ -106,4 +108,9 @@ struct sgx_vepc {
 	struct mutex lock;
 };
 
+struct sgx_vepc_page {
+	unsigned long vaddr;
+	struct sgx_vepc *vepc;
+};
+
 #endif /* _X86_SGX_H */
diff --git a/arch/x86/kernel/cpu/sgx/virt.c b/arch/x86/kernel/cpu/sgx/virt.c
index c9c8638b5dc4..d7945a47ced8 100644
--- a/arch/x86/kernel/cpu/sgx/virt.c
+++ b/arch/x86/kernel/cpu/sgx/virt.c
@@ -29,6 +29,7 @@ static int __sgx_vepc_fault(struct sgx_vepc *vepc,
 			    struct vm_area_struct *vma, unsigned long addr)
 {
 	struct sgx_epc_page *epc_page;
+	struct sgx_vepc_page *owner;
 	unsigned long index, pfn;
 	int ret;
 
@@ -41,13 +42,22 @@ static int __sgx_vepc_fault(struct sgx_vepc *vepc,
 	if (epc_page)
 		return 0;
 
-	epc_page = sgx_alloc_epc_page(vepc, false);
-	if (IS_ERR(epc_page))
-		return PTR_ERR(epc_page);
+	owner = kzalloc(sizeof(*owner), GFP_KERNEL);
+	if (!owner)
+		return -ENOMEM;
+	owner->vepc = vepc;
+	owner->vaddr = addr & PAGE_MASK;
+
+	epc_page = sgx_alloc_epc_page(owner, false);
+	if (IS_ERR(epc_page)) {
+		ret = PTR_ERR(epc_page);
+		goto err_free_owner;
+	}
+	epc_page->flags = SGX_EPC_PAGE_IS_VEPC;
 
 	ret = xa_err(xa_store(&vepc->page_array, index, epc_page, GFP_KERNEL));
 	if (ret)
-		goto err_free;
+		goto err_free_page;
 
 	pfn = PFN_DOWN(sgx_get_epc_phys_addr(epc_page));
 
@@ -61,8 +71,10 @@ static int __sgx_vepc_fault(struct sgx_vepc *vepc,
 
 err_delete:
 	xa_erase(&vepc->page_array, index);
-err_free:
+err_free_page:
 	sgx_free_epc_page(epc_page);
+err_free_owner:
+	kfree(owner);
 	return ret;
 }
 
@@ -122,6 +134,7 @@ static int sgx_vepc_remove_page(struct sgx_epc_page *epc_page)
 
 static int sgx_vepc_free_page(struct sgx_epc_page *epc_page)
 {
+	struct sgx_vepc_page *owner = (struct sgx_vepc_page *)epc_page->owner;
 	int ret = sgx_vepc_remove_page(epc_page);
 	if (ret) {
 		/*
@@ -141,6 +154,7 @@ static int sgx_vepc_free_page(struct sgx_epc_page *epc_page)
 		return ret;
 	}
 
+	kfree(owner);
 	sgx_free_epc_page(epc_page);
 	return 0;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 2/4] x86/sgx: add struct sgx_vepc_page to manage EPC pages for vepc
  2022-05-10  3:17 [PATCH 2/4] x86/sgx: add struct sgx_vepc_page to manage EPC pages for vepc Zhiquan Li
@ 2022-05-11 10:32 ` Jarkko Sakkinen
  2022-05-12 12:16   ` Zhiquan Li
  0 siblings, 1 reply; 3+ messages in thread
From: Jarkko Sakkinen @ 2022-05-11 10:32 UTC (permalink / raw)
  To: Zhiquan Li; +Cc: linux-sgx, tony.luck, dave.hansen, seanjc, fan.du

On Tue, May 10, 2022 at 11:17:37AM +0800, Zhiquan Li wrote:
> Current SGX data structures are insufficient to track the EPC pages for
> vepc. For example, if we want to retrieve the virtual address of an EPC
> page allocated to an enclave on host, we can find this info from its
> owner, the ‘desc’ field of struct sgx_encl_page. However, if the EPC
> page is allocated to a KVM guest, this is not available, as their owner
> is a shared vepc.
> 
> So, we introduce struct sgx_vepc_page which can be the owner of EPC
> pages for vepc and saves the useful info of EPC pages for vepc, like
> struct sgx_encl_page.
> 
> Canonical memory failure collects victim tasks by iterating all the
> tasks one by one and use reverse mapping to get victim tasks’ virtual
> address.
> 
> This is not necessary for SGX - as one EPC page can be mapped to ONE
> enclave only. So, this 1:1 mapping enforcement allows us to find task
> virtual address with physical address directly.
> 
> Signed-off-by: Zhiquan Li <zhiquan1.li@intel.com>
> ---
>  arch/x86/kernel/cpu/sgx/sgx.h  |  7 +++++++
>  arch/x86/kernel/cpu/sgx/virt.c | 24 +++++++++++++++++++-----
>  2 files changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h
> index 83ff8c3e81cf..cc01d992453a 100644
> --- a/arch/x86/kernel/cpu/sgx/sgx.h
> +++ b/arch/x86/kernel/cpu/sgx/sgx.h
> @@ -28,6 +28,8 @@
>  
>  /* Pages on free list */
>  #define SGX_EPC_PAGE_IS_FREE		BIT(1)
> +/* Pages is used by VM guest */
> +#define SGX_EPC_PAGE_IS_VEPC		BIT(2)
>  
>  struct sgx_epc_page {
>  	unsigned int section;
> @@ -106,4 +108,9 @@ struct sgx_vepc {
>  	struct mutex lock;
>  };
>  
> +struct sgx_vepc_page {
> +	unsigned long vaddr;
> +	struct sgx_vepc *vepc;
> +};

Same comment as for struct sgx_vepc.

> +
>  #endif /* _X86_SGX_H */
> diff --git a/arch/x86/kernel/cpu/sgx/virt.c b/arch/x86/kernel/cpu/sgx/virt.c
> index c9c8638b5dc4..d7945a47ced8 100644
> --- a/arch/x86/kernel/cpu/sgx/virt.c
> +++ b/arch/x86/kernel/cpu/sgx/virt.c
> @@ -29,6 +29,7 @@ static int __sgx_vepc_fault(struct sgx_vepc *vepc,
>  			    struct vm_area_struct *vma, unsigned long addr)
>  {
>  	struct sgx_epc_page *epc_page;
> +	struct sgx_vepc_page *owner;
>  	unsigned long index, pfn;
>  	int ret;
>  
> @@ -41,13 +42,22 @@ static int __sgx_vepc_fault(struct sgx_vepc *vepc,
>  	if (epc_page)
>  		return 0;
>  
> -	epc_page = sgx_alloc_epc_page(vepc, false);
> -	if (IS_ERR(epc_page))
> -		return PTR_ERR(epc_page);
> +	owner = kzalloc(sizeof(*owner), GFP_KERNEL);
> +	if (!owner)
> +		return -ENOMEM;
> +	owner->vepc = vepc;
> +	owner->vaddr = addr & PAGE_MASK;
> +
> +	epc_page = sgx_alloc_epc_page(owner, false);
> +	if (IS_ERR(epc_page)) {
> +		ret = PTR_ERR(epc_page);
> +		goto err_free_owner;
> +	}
> +	epc_page->flags = SGX_EPC_PAGE_IS_VEPC;
>  
>  	ret = xa_err(xa_store(&vepc->page_array, index, epc_page, GFP_KERNEL));
>  	if (ret)
> -		goto err_free;
> +		goto err_free_page;
>  
>  	pfn = PFN_DOWN(sgx_get_epc_phys_addr(epc_page));
>  
> @@ -61,8 +71,10 @@ static int __sgx_vepc_fault(struct sgx_vepc *vepc,
>  
>  err_delete:
>  	xa_erase(&vepc->page_array, index);
> -err_free:
> +err_free_page:
>  	sgx_free_epc_page(epc_page);
> +err_free_owner:
> +	kfree(owner);
>  	return ret;
>  }
>  
> @@ -122,6 +134,7 @@ static int sgx_vepc_remove_page(struct sgx_epc_page *epc_page)
>  
>  static int sgx_vepc_free_page(struct sgx_epc_page *epc_page)
>  {
> +	struct sgx_vepc_page *owner = (struct sgx_vepc_page *)epc_page->owner;
>  	int ret = sgx_vepc_remove_page(epc_page);
>  	if (ret) {
>  		/*
> @@ -141,6 +154,7 @@ static int sgx_vepc_free_page(struct sgx_epc_page *epc_page)
>  		return ret;
>  	}
>  
> +	kfree(owner);
>  	sgx_free_epc_page(epc_page);
>  	return 0;
>  }
> -- 
> 2.25.1
> 

BR, Jarkko

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 2/4] x86/sgx: add struct sgx_vepc_page to manage EPC pages for vepc
  2022-05-11 10:32 ` Jarkko Sakkinen
@ 2022-05-12 12:16   ` Zhiquan Li
  0 siblings, 0 replies; 3+ messages in thread
From: Zhiquan Li @ 2022-05-12 12:16 UTC (permalink / raw)
  To: Jarkko Sakkinen; +Cc: linux-sgx, tony.luck, dave.hansen, seanjc, fan.du



On 2022/5/11 18:32, Jarkko Sakkinen wrote:
> On Tue, May 10, 2022 at 11:17:37AM +0800, Zhiquan Li wrote:
>> Current SGX data structures are insufficient to track the EPC pages for
>> vepc. For example, if we want to retrieve the virtual address of an EPC
>> page allocated to an enclave on host, we can find this info from its
>> owner, the ‘desc’ field of struct sgx_encl_page. However, if the EPC
>> page is allocated to a KVM guest, this is not available, as their owner
>> is a shared vepc.
>>
>> So, we introduce struct sgx_vepc_page which can be the owner of EPC
>> pages for vepc and saves the useful info of EPC pages for vepc, like
>> struct sgx_encl_page.
>>
>> Canonical memory failure collects victim tasks by iterating all the
>> tasks one by one and use reverse mapping to get victim tasks’ virtual
>> address.
>>
>> This is not necessary for SGX - as one EPC page can be mapped to ONE
>> enclave only. So, this 1:1 mapping enforcement allows us to find task
>> virtual address with physical address directly.
>>
>> Signed-off-by: Zhiquan Li <zhiquan1.li@intel.com>
>> ---
>>  arch/x86/kernel/cpu/sgx/sgx.h  |  7 +++++++
>>  arch/x86/kernel/cpu/sgx/virt.c | 24 +++++++++++++++++++-----
>>  2 files changed, 26 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h
>> index 83ff8c3e81cf..cc01d992453a 100644
>> --- a/arch/x86/kernel/cpu/sgx/sgx.h
>> +++ b/arch/x86/kernel/cpu/sgx/sgx.h
>> @@ -28,6 +28,8 @@
>>  
>>  /* Pages on free list */
>>  #define SGX_EPC_PAGE_IS_FREE		BIT(1)
>> +/* Pages is used by VM guest */
>> +#define SGX_EPC_PAGE_IS_VEPC		BIT(2)
>>  
>>  struct sgx_epc_page {
>>  	unsigned int section;
>> @@ -106,4 +108,9 @@ struct sgx_vepc {
>>  	struct mutex lock;
>>  };
>>  
>> +struct sgx_vepc_page {
>> +	unsigned long vaddr;
>> +	struct sgx_vepc *vepc;
>> +};
> 
> Same comment as for struct sgx_vepc.

No problem, I will add comments in v2 patch.

Best Regards,
Zhiquan

> 
>> +
>>  #endif /* _X86_SGX_H */
>> diff --git a/arch/x86/kernel/cpu/sgx/virt.c b/arch/x86/kernel/cpu/sgx/virt.c
>> index c9c8638b5dc4..d7945a47ced8 100644
>> --- a/arch/x86/kernel/cpu/sgx/virt.c
>> +++ b/arch/x86/kernel/cpu/sgx/virt.c
>> @@ -29,6 +29,7 @@ static int __sgx_vepc_fault(struct sgx_vepc *vepc,
>>  			    struct vm_area_struct *vma, unsigned long addr)
>>  {
>>  	struct sgx_epc_page *epc_page;
>> +	struct sgx_vepc_page *owner;
>>  	unsigned long index, pfn;
>>  	int ret;
>>  
>> @@ -41,13 +42,22 @@ static int __sgx_vepc_fault(struct sgx_vepc *vepc,
>>  	if (epc_page)
>>  		return 0;
>>  
>> -	epc_page = sgx_alloc_epc_page(vepc, false);
>> -	if (IS_ERR(epc_page))
>> -		return PTR_ERR(epc_page);
>> +	owner = kzalloc(sizeof(*owner), GFP_KERNEL);
>> +	if (!owner)
>> +		return -ENOMEM;
>> +	owner->vepc = vepc;
>> +	owner->vaddr = addr & PAGE_MASK;
>> +
>> +	epc_page = sgx_alloc_epc_page(owner, false);
>> +	if (IS_ERR(epc_page)) {
>> +		ret = PTR_ERR(epc_page);
>> +		goto err_free_owner;
>> +	}
>> +	epc_page->flags = SGX_EPC_PAGE_IS_VEPC;
>>  
>>  	ret = xa_err(xa_store(&vepc->page_array, index, epc_page, GFP_KERNEL));
>>  	if (ret)
>> -		goto err_free;
>> +		goto err_free_page;
>>  
>>  	pfn = PFN_DOWN(sgx_get_epc_phys_addr(epc_page));
>>  
>> @@ -61,8 +71,10 @@ static int __sgx_vepc_fault(struct sgx_vepc *vepc,
>>  
>>  err_delete:
>>  	xa_erase(&vepc->page_array, index);
>> -err_free:
>> +err_free_page:
>>  	sgx_free_epc_page(epc_page);
>> +err_free_owner:
>> +	kfree(owner);
>>  	return ret;
>>  }
>>  
>> @@ -122,6 +134,7 @@ static int sgx_vepc_remove_page(struct sgx_epc_page *epc_page)
>>  
>>  static int sgx_vepc_free_page(struct sgx_epc_page *epc_page)
>>  {
>> +	struct sgx_vepc_page *owner = (struct sgx_vepc_page *)epc_page->owner;
>>  	int ret = sgx_vepc_remove_page(epc_page);
>>  	if (ret) {
>>  		/*
>> @@ -141,6 +154,7 @@ static int sgx_vepc_free_page(struct sgx_epc_page *epc_page)
>>  		return ret;
>>  	}
>>  
>> +	kfree(owner);
>>  	sgx_free_epc_page(epc_page);
>>  	return 0;
>>  }
>> -- 
>> 2.25.1
>>
> 
> BR, Jarkko

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-05-12 12:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-10  3:17 [PATCH 2/4] x86/sgx: add struct sgx_vepc_page to manage EPC pages for vepc Zhiquan Li
2022-05-11 10:32 ` Jarkko Sakkinen
2022-05-12 12:16   ` Zhiquan Li

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.