Messages in this thread Patch in this message | | | Date | Wed, 28 Jul 2021 23:32:42 +0000 | From | Sean Christopherson <> | Subject | Re: [PATCH v3 1/7] x86/sgx: Provide indication of life-cycle of EPC pages |
| |
On Wed, Jul 28, 2021, Dave Hansen wrote: > On 7/28/21 3:57 PM, Luck, Tony wrote: > >> Wouldn't it be safer to do something like: > >> > >> page->owner = owner ? owner : (void *)-1; > >> > >> -1 is non-NULL, but also invalid, which makes it harder for us to poke > >> ourselves in the eye. > > Does Linux have some #define INVALID_POINTER thing that > > provides a guaranteed bad (e.g. non-canonical) value? > > > > (void *)-1 seems hacky. > > ERR_PTR(-SOMETHING) wouldn't be too bad. I guess it could even be: > > page->owner = ERR_PTR(SGX_EPC_PAGE_VA); > > and then: > > #define SGX_EPC_PAGE_VA 0xffff...something...greppable > > I *thought* we had a file full of these magic values, but maybe I'm > misremembering the uapi magic header.
Rather than use a magic const, just pass in the actual va_page. The only reason NULL is passed is that prior to virtual EPC, there were only enclave pages and VA pages, and assiging a non-NULL pointer to sgx_epc_page.owner, which is a struct sgx_encl_page, was gross. Virtual EPC sets owner somewhat prematurely; it's needed iff an EPC cgroup is added, to support OOM EPC killing (and a pointer to va_page is also needed in this case).
sgx_epc_page.owner can even be converted to 'void *' without additional changes since all consumers capture it in a local sgx_encl_page variable.
diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c index 001808e3901c..f9da8fe4dd6b 100644 --- a/arch/x86/kernel/cpu/sgx/encl.c +++ b/arch/x86/kernel/cpu/sgx/encl.c @@ -674,12 +674,12 @@ int sgx_encl_test_and_clear_young(struct mm_struct *mm, * a VA page, * -errno otherwise */ -struct sgx_epc_page *sgx_alloc_va_page(void) +struct sgx_epc_page *sgx_alloc_va_page(struct sgx_va_page *va_page) { struct sgx_epc_page *epc_page; int ret;
- epc_page = sgx_alloc_epc_page(NULL, true); + epc_page = sgx_alloc_epc_page(va_page, true); if (IS_ERR(epc_page)) return ERR_CAST(epc_page);
diff --git a/arch/x86/kernel/cpu/sgx/encl.h b/arch/x86/kernel/cpu/sgx/encl.h index fec43ca65065..3d12dbeae14a 100644 --- a/arch/x86/kernel/cpu/sgx/encl.h +++ b/arch/x86/kernel/cpu/sgx/encl.h @@ -111,7 +111,7 @@ void sgx_encl_put_backing(struct sgx_backing *backing, bool do_write); int sgx_encl_test_and_clear_young(struct mm_struct *mm, struct sgx_encl_page *page);
-struct sgx_epc_page *sgx_alloc_va_page(void); +struct sgx_epc_page *sgx_alloc_va_page(struct sgx_va_page *va_page); unsigned int sgx_alloc_va_slot(struct sgx_va_page *va_page); void sgx_free_va_slot(struct sgx_va_page *va_page, unsigned int offset); bool sgx_va_page_full(struct sgx_va_page *va_page); diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 83df20e3e633..655ce0bb069d 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -30,7 +30,7 @@ static struct sgx_va_page *sgx_encl_grow(struct sgx_encl *encl) if (!va_page) return ERR_PTR(-ENOMEM);
- va_page->epc_page = sgx_alloc_va_page(); + va_page->epc_page = sgx_alloc_va_page(va_page); if (IS_ERR(va_page->epc_page)) { err = ERR_CAST(va_page->epc_page); kfree(va_page); diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h index 4628acec0009..4e1a410b8a62 100644 --- a/arch/x86/kernel/cpu/sgx/sgx.h +++ b/arch/x86/kernel/cpu/sgx/sgx.h @@ -29,7 +29,7 @@ struct sgx_epc_page { unsigned int section; unsigned int flags; - struct sgx_encl_page *owner; + void *owner; struct list_head list; };
| |