Re: [PATCH v4] x86/sgx: Free backing memory after faulting the enclave page

[Dave Hansen]

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@xxxxxxxxx/

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?