What about: "x86/sgx: Add encl_page->vm_run_prot_bits for dynamic permission changes" On Wed, Dec 01, 2021 at 11:23:03AM -0800, Reinette Chatre wrote: > Enclave creators declare their paging permission intent at the time > the pages are added to the enclave. These paging permissions are > vetted when pages are added to the enclave and stashed off > (in sgx_encl_page->vm_max_prot_bits) for later comparison with > enclave PTEs. > > Current permission support assume that enclave page permissions > remain static for the lifetime of the enclave. This is about to change > with the addition of support for SGX2 where the permissions of enclave > pages belonging to an initialized enclave may be changed during the > enclave's lifetime. > > Introduce runtime protection bits in preparation for support of By writing "Introduce runtime protection bits", instead of simply "Add encl_page->vm_run_prot_bits", the only thing you are adding is obfuscation. Try to refer to the "exact thing", instead of English rephrasing whenever possible. > enclave page permission changes. These bits reflect the active > permissions of an enclave page and are not to exceed the maximum > protection bits that passed scrutiny during enclave creation. > > Associate runtime protection bits with each enclave page. Initialize > the runtime protection bits to the vetted maximum protection bits > on page creation. Use the runtime protection bits for any access > checks. I guess the first sentence in this paragraph is completely redundant as the first sentence of the previous paragraph tells the exact same story. > struct sgx_encl_page hosting this information is maintained for each > enclave page so the space consumed by the struct is important. > The existing vm_max_prot_bits is already unsigned long while only using > three bits. Transition to a bitfield for the two members containing > protection bits. > > Signed-off-by: Reinette Chatre <reinette.chatre@xxxxxxxxx> So this commit message left the most important thing unanswered, or I missed it (which happens quite often): why two fields instead of one? Why vm_max_port_bits needs to stay constant? It's something that should be clearly documented. /Jarkko
