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

Dave Hansen
2022-02-24T17:14:40Z

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?