lkml.org 
[lkml]   [2022]   [Feb]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4] x86/sgx: Free backing memory after faulting the enclave page
On 2/22/22 04:03, Jarkko Sakkinen wrote:
> + if (pcmd_page_empty) {
> + pgoff_t pcmd_off = encl->size + PAGE_SIZE /* SECS */ +
> + page_index * sizeof(struct sgx_pcmd);
> +
> + sgx_encl_truncate_backing_page(encl, PFN_DOWN(pcmd_off));
> + }
> +
> return ret;
> }
>
> @@ -583,7 +613,7 @@ static struct page *sgx_encl_get_backing_page(struct sgx_encl *encl,
> static int sgx_encl_get_backing(struct sgx_encl *encl, unsigned long page_index,
> struct sgx_backing *backing)
> {
> - pgoff_t pcmd_index = PFN_DOWN(encl->size) + 1 + (page_index >> 5);
> + pgoff_t pcmd_off = encl->size + PAGE_SIZE /* SECS */ + page_index * sizeof(struct sgx_pcmd);

Jarkko, I really don't like how this looks. The '/* SECS */' thing is
pretty ugly and the comment in the middle of an arithmetic operation is
just really hard to read.

Then, there's the fact that this gem is copied-and-pasted. Oh, and it
looks a wee bit over 80 columns.

I went to the trouble of writing a nice, fully-fleshed-out helper
function for this with a comment included:

> https://lore.kernel.org/all/8afec431-4dfc-d8df-152b-76cca0e17ccb@intel.com/

Was there a problem using that? The change from the last version is:

* Sanitized the offset calculations.

Given that there have been multiple different calculations over the four
versions so far, which version was right? v3 or v4?

\
 
 \ /
  Last update: 2022-02-24 18:15    [W:0.126 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site