From: Sean Christopherson <sean.j.christopherson@intel.com> To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Cc: linux-kernel@vger.kernel.org, x86@kernel.org, linux-sgx@vger.kernel.org, akpm@linux-foundation.org, dave.hansen@intel.com, nhorman@redhat.com, npmccallum@redhat.com, haitao.huang@intel.com, andriy.shevchenko@linux.intel.com, tglx@linutronix.de, kai.svahn@intel.com, bp@alien8.de, josh@joshtriplett.org, luto@kernel.org, kai.huang@intel.com, rientjes@google.com, cedric.xing@intel.com, puiterwijk@redhat.com, linux-mm@kvack.org, Ismo Puustinen <ismo.puustinen@intel.com>, Mark Shanahan <mark.shanahan@intel.com>, Mikko Ylinen <mikko.ylinen@intel.com>, Derek Bombien <derek.bombien@intel.com> Subject: Re: [PATCH v28 16/22] x86/sgx: Add a page reclaimer Date: Thu, 5 Mar 2020 11:03:54 -0800 Message-ID: <20200305190354.GK11500@linux.intel.com> (raw) In-Reply-To: <20200303233609.713348-17-jarkko.sakkinen@linux.intel.com> +cc some more Intel people On Wed, Mar 04, 2020 at 01:36:03AM +0200, Jarkko Sakkinen wrote: > +static struct page *sgx_encl_get_backing_page(struct sgx_encl *encl, > + pgoff_t index) > +{ > + struct inode *inode = encl->backing->f_path.dentry->d_inode; > + struct address_space *mapping = inode->i_mapping; > + gfp_t gfpmask = mapping_gfp_mask(mapping); Question for folks that care about cgroup accounting: should the shmem page allocation be accounted, even though it'll charge the "wrong" task? E.g. gfp_t gfpmask = mapping_gfp_mask(mapping) | __GFP_ACCOUNT; Background details... To reclaim an EPC page, the encrypted data must be written to addressable memory when the page is evicted from the EPC. I.e. RAM or persistent memory acts as swap space for the EPC. In the SGX code this is referred to as the "backing store" or just "backing". As implemented, the backing for EPC reclaim is not pre-allocated. I don't think anyone would want to change this behavior as it would eat regular memory even if EPC isn't oversubscribed. But, as implemented it's not accounted. The reason we haven't explicitly forced accounting is because allocations for the backing would be charged to the task doing the evicting, as opposed to the task that originally allocated the EPC page. This means that task 'A' can do a DOS of sorts on task 'B' by allocating a large amount of EPC with the goal of causing 'B' to trigger (normal memory) OOM and get killed. AFAICT, that's still preferable to not accounting at all, e.g. at least 'B' would need to be allocating a decent amount of EPC as well to trigger OOM, as opposed to today where the allocations are completely unaccounted and so cause a more global OOM. We've also discussed taking a file descriptor to hold the backing, but unless I'm misreading the pagecache code, that doesn't solve the incorrect accounting problem because the current task, i.e. evicting task, would be charged. In other words, whether the backing is kernel or user controlled is purely an ABI question. In theory, SGX could do some form of manual memcg accounting in sgx_encl_get_backing_page(), but that gets ridiculously ugly, if it's even possible, e.g. determining whether or not the page was already allocated would be a nightmare. Long term, I think this can be solved, or at least mitigated, by adding a "backing" limit to the EPC cgroup (which obviously doesn't exist in yet) to allow limiting the amount of memory that can be indirectly consumed by creating a large enclave. E.g. account each page during ENCLAVE_ADD_PAGES, with some logic to transfer the charges when a mm_struct is disassociated from an enclave (which will probably be needed for the base EPC cgroup anyways). > + > + return shmem_read_mapping_page_gfp(mapping, index, gfpmask); > +} > + > +/** > + * sgx_encl_get_backing() - Pin the backing storage > + * @encl: an enclave > + * @page_index: enclave page index > + * @backing: data for accessing backing storage for the page > + * > + * Pin the backing storage pages for storing the encrypted contents and Paging > + * Crypto MetaData (PCMD) of an enclave page. > + * > + * Return: > + * 0 on success, > + * -errno otherwise. > + */ > +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); > + struct page *contents; > + struct page *pcmd; > + > + contents = sgx_encl_get_backing_page(encl, page_index); > + if (IS_ERR(contents)) > + return PTR_ERR(contents); > + > + pcmd = sgx_encl_get_backing_page(encl, pcmd_index); > + if (IS_ERR(pcmd)) { > + put_page(contents); > + return PTR_ERR(pcmd); > + } > + > + backing->page_index = page_index; > + backing->contents = contents; > + backing->pcmd = pcmd; > + backing->pcmd_offset = > + (page_index & (PAGE_SIZE / sizeof(struct sgx_pcmd) - 1)) * > + sizeof(struct sgx_pcmd); > + > + return 0; > +} > + > +/** > + * sgx_encl_put_backing() - Unpin the backing storage > + * @backing: data for accessing backing storage for the page > + * @do_write: mark pages dirty > + */ > +void sgx_encl_put_backing(struct sgx_backing *backing, bool do_write) > +{ > + if (do_write) { > + set_page_dirty(backing->pcmd); > + set_page_dirty(backing->contents); > + } > + > + put_page(backing->pcmd); > + put_page(backing->contents); > +}
next prev parent reply index Thread overview: 109+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-03-03 23:35 [PATCH v28 00/22] Intel SGX foundations Jarkko Sakkinen 2020-03-03 23:35 ` [PATCH v28 01/22] x86/sgx: Update MAINTAINERS Jarkko Sakkinen 2020-03-03 23:35 ` [PATCH v28 02/22] x86/cpufeatures: x86/msr: Add Intel SGX hardware bits Jarkko Sakkinen 2020-03-03 23:35 ` [PATCH v28 03/22] x86/cpufeatures: x86/msr: Intel SGX Launch Control " Jarkko Sakkinen 2020-03-03 23:35 ` [PATCH v28 04/22] x86/mm: x86/sgx: Signal SIGSEGV with PF_SGX Jarkko Sakkinen 2020-03-03 23:35 ` [PATCH v28 05/22] x86/sgx: Add SGX microarchitectural data structures Jarkko Sakkinen 2020-03-03 23:35 ` [PATCH v28 06/22] x86/sgx: Add wrappers for ENCLS leaf functions Jarkko Sakkinen 2020-03-09 21:14 ` Sean Christopherson 2020-03-03 23:35 ` [PATCH v28 07/22] x86/cpu/intel: Detect SGX supprt Jarkko Sakkinen 2020-03-09 21:56 ` Sean Christopherson 2020-03-11 17:03 ` Jarkko Sakkinen 2020-03-03 23:35 ` [PATCH v28 08/22] x86/sgx: Enumerate and track EPC sections Jarkko Sakkinen 2020-03-03 23:35 ` [PATCH v28 09/22] x86/sgx: Add functions to allocate and free EPC pages Jarkko Sakkinen 2020-03-03 23:35 ` [PATCH v28 10/22] mm: Introduce vm_ops->may_mprotect() Jarkko Sakkinen 2020-03-03 23:35 ` [PATCH v28 11/22] x86/sgx: Linux Enclave Driver Jarkko Sakkinen 2020-03-05 17:40 ` Sean Christopherson 2020-03-05 18:24 ` Jethro Beekman 2020-03-05 19:04 ` Sean Christopherson 2020-03-06 19:00 ` Jarkko Sakkinen 2020-03-19 18:22 ` Dr. Greg 2020-03-06 18:58 ` Jarkko Sakkinen 2020-03-03 23:35 ` [PATCH v28 12/22] docs: x86/sgx: Document SGX micro architecture and kernel internals Jarkko Sakkinen 2020-03-03 23:36 ` [PATCH v28 13/22] selftests/x86: Recurse into subdirectories Jarkko Sakkinen 2020-03-03 23:36 ` [PATCH v28 14/22] selftests/x86: Add a selftest for SGX Jarkko Sakkinen 2020-03-04 19:27 ` Nathaniel McCallum 2020-03-05 11:33 ` Jarkko Sakkinen 2020-03-06 15:42 ` Dr. Greg 2020-03-06 19:07 ` Jarkko Sakkinen 2020-03-07 17:42 ` Dr. Greg 2020-03-10 13:08 ` Jarkko Sakkinen 2020-03-11 13:28 ` Jarkko Sakkinen [not found] ` <20200311164047.GG21852@linux.intel.com> 2020-03-13 19:24 ` Jarkko Sakkinen 2020-03-04 19:44 ` Nathaniel McCallum 2020-03-04 19:51 ` Nathaniel McCallum 2020-03-06 5:32 ` Dr. Greg 2020-03-06 19:04 ` Jarkko Sakkinen 2020-03-10 19:29 ` Haitao Huang 2020-03-11 9:13 ` Dr. Greg 2020-03-11 17:15 ` Haitao Huang 2020-03-17 1:07 ` Dr. Greg 2020-03-03 23:36 ` [PATCH v28 15/22] x86/sgx: Add provisioning Jarkko Sakkinen 2020-03-03 23:36 ` [PATCH v28 16/22] x86/sgx: Add a page reclaimer Jarkko Sakkinen 2020-03-05 19:03 ` Sean Christopherson [this message] 2020-03-06 18:47 ` Jarkko Sakkinen 2020-03-12 18:38 ` Sean Christopherson 2020-03-15 0:27 ` Jarkko Sakkinen 2020-03-15 1:17 ` Jarkko Sakkinen 2020-03-09 21:16 ` Sean Christopherson 2020-03-03 23:36 ` [PATCH v28 17/22] x86/sgx: ptrace() support for the SGX driver Jarkko Sakkinen 2020-03-03 23:36 ` [PATCH v28 18/22] x86/vdso: Add support for exception fixup in vDSO functions Jarkko Sakkinen 2020-03-03 23:36 ` [PATCH v28 19/22] x86/fault: Add helper function to sanitize error code Jarkko Sakkinen 2020-03-03 23:36 ` [PATCH v28 20/22] x86/traps: Attempt to fixup exceptions in vDSO before signaling Jarkko Sakkinen 2020-03-03 23:36 ` [PATCH v28 21/22] x86/vdso: Implement a vDSO for Intel SGX enclave call Jarkko Sakkinen 2020-03-11 17:30 ` Nathaniel McCallum 2020-03-11 17:38 ` Jethro Beekman 2020-03-11 19:15 ` Nathaniel McCallum 2020-03-13 15:48 ` Nathaniel McCallum 2020-03-13 16:46 ` Sean Christopherson 2020-03-13 18:32 ` Nathaniel McCallum 2020-03-13 18:44 ` Sean Christopherson 2020-03-13 20:14 ` Nathaniel McCallum 2020-03-13 22:08 ` Sean Christopherson 2020-03-14 14:10 ` Nathaniel McCallum 2020-03-18 23:40 ` Sean Christopherson 2020-03-19 0:38 ` Xing, Cedric 2020-03-19 1:03 ` Sean Christopherson 2020-03-20 13:55 ` Nathaniel McCallum 2020-03-15 1:25 ` Jarkko Sakkinen 2020-03-15 17:53 ` Nathaniel McCallum 2020-03-16 13:31 ` Jethro Beekman 2020-03-16 13:57 ` Nathaniel McCallum 2020-03-16 13:59 ` Jethro Beekman 2020-03-16 14:03 ` Nathaniel McCallum 2020-03-16 17:17 ` Sean Christopherson 2020-03-16 21:27 ` Jarkko Sakkinen 2020-03-16 21:29 ` Jarkko Sakkinen 2020-03-16 22:55 ` Sean Christopherson 2020-03-16 23:56 ` Xing, Cedric 2020-03-18 22:01 ` Jarkko Sakkinen 2020-03-18 22:18 ` Jarkko Sakkinen 2020-03-16 13:56 ` Jarkko Sakkinen 2020-03-16 14:01 ` Nathaniel McCallum 2020-03-16 21:38 ` Jarkko Sakkinen 2020-03-16 22:53 ` Sean Christopherson 2020-03-16 23:50 ` Xing, Cedric 2020-03-16 23:59 ` Sean Christopherson 2020-03-17 0:18 ` Xing, Cedric 2020-03-17 0:27 ` Sean Christopherson 2020-03-17 16:37 ` Nathaniel McCallum 2020-03-17 16:50 ` Nathaniel McCallum 2020-03-17 21:40 ` Xing, Cedric 2020-03-17 22:09 ` Sean Christopherson 2020-03-17 22:36 ` Xing, Cedric 2020-03-17 23:57 ` Sean Christopherson 2020-03-17 22:23 ` Xing, Cedric 2020-03-18 13:01 ` Nathaniel McCallum 2020-03-20 15:53 ` Nathaniel McCallum 2020-03-17 16:28 ` Nathaniel McCallum 2020-03-18 22:58 ` Jarkko Sakkinen 2020-03-18 22:39 ` Jarkko Sakkinen 2020-03-11 19:30 ` Nathaniel McCallum 2020-03-13 0:52 ` Sean Christopherson 2020-03-13 16:07 ` Nathaniel McCallum 2020-03-13 16:33 ` Sean Christopherson 2020-03-03 23:36 ` [PATCH v28 22/22] selftests/x86: Add vDSO selftest for SGX Jarkko Sakkinen 2020-03-04 19:24 ` [PATCH v28 00/22] Intel SGX foundations Nathaniel McCallum 2020-03-17 16:00 ` Jordan Hand 2020-03-18 21:56 ` Jarkko Sakkinen 2020-03-19 17:16 ` Dr. Greg
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200305190354.GK11500@linux.intel.com \ --to=sean.j.christopherson@intel.com \ --cc=akpm@linux-foundation.org \ --cc=andriy.shevchenko@linux.intel.com \ --cc=bp@alien8.de \ --cc=cedric.xing@intel.com \ --cc=dave.hansen@intel.com \ --cc=derek.bombien@intel.com \ --cc=haitao.huang@intel.com \ --cc=ismo.puustinen@intel.com \ --cc=jarkko.sakkinen@linux.intel.com \ --cc=josh@joshtriplett.org \ --cc=kai.huang@intel.com \ --cc=kai.svahn@intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=linux-sgx@vger.kernel.org \ --cc=luto@kernel.org \ --cc=mark.shanahan@intel.com \ --cc=mikko.ylinen@intel.com \ --cc=nhorman@redhat.com \ --cc=npmccallum@redhat.com \ --cc=puiterwijk@redhat.com \ --cc=rientjes@google.com \ --cc=tglx@linutronix.de \ --cc=x86@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Linux-Sgx Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/linux-sgx/0 linux-sgx/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 linux-sgx linux-sgx/ https://lore.kernel.org/linux-sgx \ linux-sgx@vger.kernel.org public-inbox-index linux-sgx Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-sgx AGPL code for this site: git clone https://public-inbox.org/public-inbox.git