From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7B2ECC00140 for ; Fri, 12 Aug 2022 05:47:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234638AbiHLFrL (ORCPT ); Fri, 12 Aug 2022 01:47:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52030 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229524AbiHLFrL (ORCPT ); Fri, 12 Aug 2022 01:47:11 -0400 Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C57F09DFBC for ; Thu, 11 Aug 2022 22:47:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1660283229; x=1691819229; h=to:cc:subject:references:date:mime-version: content-transfer-encoding:from:message-id:in-reply-to; bh=3h1DAyXXL3WnxVuUd27sSoL4lJqhk//8ll5HbdOHyl4=; b=Y1gJd8QUtMBvL0BwdPRKh/XBrCE13361/sB/O/QgsWeYlXxsjljXXjXE MRhgemMUH7gBszGZUna9paCDNuNb9MwFLjDOf9oz2cU2ogtwb0lvSvDUg pPqh9h0Puup3X2odXIoDdDQXE1FGfjWGsUzoIeBtlnRJB4xmSFrKOwzSu FpPbe1/XVxrDd0nGaznOKlnhpCecXcvLpPOcQvAhhljskl2rX7merxAeU eKaW2MOCd+gA7FwFMSZBDgdpHTOj26HpeM2j1GTC5yO4G1MhJv3SPi6aB TYWHAq18IPxVX3C27AGjSIfTh2/xrcw4It3rw7u5MM8G5o0H94LlfQzdZ g==; X-IronPort-AV: E=McAfee;i="6400,9594,10436"; a="353270244" X-IronPort-AV: E=Sophos;i="5.93,231,1654585200"; d="scan'208";a="353270244" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Aug 2022 22:47:09 -0700 X-IronPort-AV: E=Sophos;i="5.93,231,1654585200"; d="scan'208";a="581967376" Received: from arpitami-mobl1.amr.corp.intel.com (HELO hhuan26-mobl1.mshome.net) ([10.209.156.107]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 11 Aug 2022 22:47:08 -0700 Content-Type: text/plain; charset=iso-8859-15; format=flowed; delsp=yes To: "Dhanraj, Vijay" , "Jarkko Sakkinen" , "Haitao Huang" Cc: "linux-sgx@vger.kernel.org" , "Chatre, Reinette" , "dave.hansen@linux.intel.com" , "Huang, Haitao" Subject: Re: [PATCH] Add SGX selftest `augment_via_eaccept_long` References: Date: Fri, 12 Aug 2022 00:47:07 -0500 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: "Haitao Huang" Organization: Intel Corp Message-ID: In-Reply-To: User-Agent: Opera Mail/1.0 (Win32) Precedence: bulk List-ID: X-Mailing-List: linux-sgx@vger.kernel.org On Thu, 11 Aug 2022 21:29:42 -0500, Haitao Huang wrote: > Hi Jarkko > > On Wed, 10 Aug 2022 20:50:54 -0500, Jarkko Sakkinen > wrote: > >> On Thu, Aug 11, 2022 at 04:36:57AM +0300, Jarkko Sakkinen wrote: >>> On Thu, Aug 11, 2022 at 04:01:15AM +0300, Jarkko Sakkinen wrote: >>> > On Wed, Aug 10, 2022 at 12:09:56AM +0000, Dhanraj, Vijay wrote: >>> > > >>> > > >>> > > > -----Original Message----- >>> > > > From: Jarkko Sakkinen >>> > > > Sent: Tuesday, August 9, 2022 11:53 AM >>> > > > To: Dhanraj, Vijay >>> > > > Cc: linux-sgx@vger.kernel.org; Chatre, Reinette >>> > > > ; dave.hansen@linux.intel.com; Huang, >>> Haitao >>> > > > >>> > > > Subject: Re: [PATCH] Add SGX selftest `augment_via_eaccept_long` >>> > > > >>> > > > On Tue, Aug 09, 2022 at 05:08:21PM +0000, Dhanraj, Vijay wrote: >>> > > > > >>> > > > > >>> > > > > > -----Original Message----- >>> > > > > > From: Jarkko Sakkinen >>> > > > > > Sent: Tuesday, August 9, 2022 9:10 AM >>> > > > > > To: Dhanraj, Vijay >>> > > > > > Cc: linux-sgx@vger.kernel.org; Chatre, Reinette >>> > > > > > ; dave.hansen@linux.intel.com; >>> Huang, >>> > > > > > Haitao >>> > > > > > Subject: Re: [PATCH] Add SGX selftest >>> `augment_via_eaccept_long` >>> > > > > > >>> > > > > > On Tue, Aug 09, 2022 at 01:45:35PM +0300, Jarkko Sakkinen >>> wrote: >>> > > > > > > On Mon, Aug 08, 2022 at 06:29:13PM +0300, Jarkko Sakkinen >>> wrote: >>> > > > > > > > On Mon, Aug 08, 2022 at 01:00:54PM +0000, Dhanraj, Vijay >>> wrote: >>> > > > > > > > > >>> > > > > > > > > > -----Original Message----- >>> > > > > > > > > > From: Jarkko Sakkinen >>> > > > > > > > > > Sent: Monday, August 8, 2022 5:18 AM >>> > > > > > > > > > To: Dhanraj, Vijay >>> > > > > > > > > > Cc: linux-sgx@vger.kernel.org; Chatre, Reinette >>> > > > > > > > > > ; >>> dave.hansen@linux.intel.com; >>> > > > > > > > > > Huang, Haitao >>> > > > > > > > > > Subject: Re: [PATCH] Add SGX selftest >>> > > > > > > > > > `augment_via_eaccept_long` >>> > > > > > > > > > >>> > > > > > > > > > On Thu, Aug 04, 2022 at 01:14:56PM -0700, >>> > > > > > > > > > vijay.dhanraj@intel.com >>> > > > > > wrote: >>> > > > > > > > > > > From: Vijay Dhanraj >>> > > > > > > > > > > >>> > > > > > > > > > > This commit adds a new test case which is same as >>> > > > > > > > > > > `augment_via_eaccept` but adds more number of EPC >>> pages to >>> > > > > > > > > > > stress test >>> > > > > > > > > > `EAUG` via `EACCEPT`. >>> > > > > > > > > > > >>> > > > > > > > > > > Signed-off-by: Vijay Dhanraj >>> >>> > > > > > > > > > > Signed-off-by: Haitao Huang >>> >>> > > > > > > > > > >>> > > > > > > > > > Hey, to reproduce the original issue: does it >>> reproduce on >>> > > > > > > > > > VM or should I run baremetal kernel? >>> > > > > > > > > > >>> > > > > > > > > > BR, Jarkko >>> > > > > > > > > >>> > > > > > > > > Hi Jarkko, The issue should be reproducible on >>> baremetal kernel. >>> > > > > > > > >>> > > > > > > > Thanks. >>> > > > > > > >>> > > > > > > I need comment out other tests in order to make sane out >>> of this >>> > > > > > > :-) >>> > > > > > > >>> > > > > > > Mentioning this because came into realization that stress >>> tests >>> > > > > > > should be IMHO moved each to a separate binary (so that >>> they can >>> > > > > > > be run separately). Just a note (TODO) to myself. >>> > > > > > > >>> > > > > > > I'll work on this today again and *possibly* split your >>> test to >>> > > > > > > its own application to get a starting point for >>> forementioned. >>> > > > > > >>> > > > > > I got >>> > > > > > >>> > > > > > # RUN enclave.augment_via_eaccept_long ... >>> > > > > > # main.c:1241:augment_via_eaccept_long:test enclave: >>> total_size = >>> > > > > > 8192, >>> > > > > > seg->size = 8192 # main.c:1241:augment_via_eaccept_long:test >>> enclave: >>> > > > > > total_size = 12288, seg->size = 4096 # >>> > > > > > main.c:1241:augment_via_eaccept_long:test enclave: >>> total_size = >>> > > > > > 36864, >>> > > > > > seg->size = 24576 # >>> main.c:1241:augment_via_eaccept_long:test enclave: >>> > > > > > total_size = 40960, seg->size = 4096 # >>> > > > > > main.c:1259:augment_via_eaccept_long:mmaping pages at end of >>> > > > enclave... >>> > > > > > # main.c:1273:augment_via_eaccept_long:Entering enclave to >>> run >>> > > > > > EACCEPT for each page of 8589934592 bytes may take a while >>> ... >>> > > > > > # OK enclave.augment_via_eaccept_long >>> > > > > > >>> > > > > > The CPU used for testing was according to /proc/cpuinfo: >>> > > > > > >>> > > > > > model name : Intel(R) Xeon(R) Gold 6338 CPU @ 2.00GHz >>> > > > > > >>> > > > > > I have couple of queries: >>> > > > > > >>> > > > > > 1. Is it possible to get dmesg output? >>> > > > > I did check the dmesg output but couldn't find anything >>> related to the >>> > > > failure. Just the general log messages. >>> > > > > >>> > > > > > 2. Do I have to repeat the test multiple times, or does it >>> > > > > > occur unconditionaly? >>> > > > > > >>> > > > > I was able to repro every time but it was a bit sporadic for >>> Haitao. >>> > > > > >>> > > > > > BR, Jarkko >>> > > > > >>> > > > > Also, did you set the PRMRR size to 2GB per socket in the >>> BIOS? The >>> > > > > issue is only reproduced for oversubscribed scenario. When I >>> set my >>> > > > > PRMRR to 64GB per socket, I wasn't able to repro the issue. >>> > > > >>> > > > I need to revisit this. >>> > > > >>> > > > Can you simply run test_sgx with gdb and see where it hits? >>> > > > HOST_CFLAGS has apparently "-g" already. >>> > > > >>> > > > > Regards, Vijay >>> > > > >>> > > > BR, Jarkko >>> > > >>> > > I am able to repro the issue when I reduce the PRMRR to 2B/socket >>> but not but not able to break on the assertion failure with gdb. I >>> also enabled debug attribute in the secs but still no avail. Anything >>> I am missing here? >>> > > >>> > > diff --git a/tools/testing/selftests/sgx/load.c >>> b/tools/testing/selftests/sgx/load.c >>> > > index 7de1b15c90b1..c4bccd3f5f17 100644 >>> > > --- a/tools/testing/selftests/sgx/load.c >>> > > +++ b/tools/testing/selftests/sgx/load.c >>> > > @@ -87,7 +87,7 @@ static bool encl_ioc_create(struct encl *encl) >>> > > >>> > > memset(secs, 0, sizeof(*secs)); >>> > > secs->ssa_frame_size = 1; >>> > > - secs->attributes = SGX_ATTR_MODE64BIT; >>> > > + secs->attributes = SGX_ATTR_MODE64BIT | SGX_ATTR_DEBUG; >>> > > secs->xfrm = 3; >>> > > secs->base = encl->encl_base; >>> > > secs->size = encl->encl_size; >>> > > >>> > > Regards, Vijay >>> > >>> > I get also full pass with 2GB configuration (and also observed that >>> > kselftest runs much faster with this configuration). >>> > >>> > But I looked at sgx_alloc_epc_page() and saw this: >>> > >>> > if (list_empty(&sgx_active_page_list)) >>> > return ERR_PTR(-ENOMEM); >>> > >>> > if (!reclaim) { >>> > page = ERR_PTR(-EBUSY); >>> > break; >>> > } >>> > >>> > In sgx_vma_fault(), when running completely out of reclaimable >>> pages, this >>> > causes VM_FAULT_SIGBUS returned instead of VM_FAULT_NOPAGE: >>> > >>> > entry = sgx_encl_load_page(encl, addr, vma->vm_flags); >>> > if (IS_ERR(entry)) { >>> > mutex_unlock(&encl->lock); >>> > >>> > if (PTR_ERR(entry) == -EBUSY) >>> > return VM_FAULT_NOPAGE; >>> > >>> > return VM_FAULT_SIGBUS; >>> > } >>> > >>> > Not sure if those should be re-ordered that would keep the process >>> stuck up >>> > until there is something to reclaim. Now we use NOPAGE to address >>> situation >>> > when there is actually something to reclaim but because of locking >>> side of >>> > things we pass reclaim=false to sgx_alloc_epc_page(). >>> > >>> > So this is kind of OOM behaviour how it works now instead of stalling >>> > processes. >>> >>> Right, I looked at the original email at was really a page fault >>> that was catched. The above theory cannot possibly hold, as the >>> process does not exit with a bus error. >>> >>> I looked next to sgx_encl_eaug_page(), and found this: >>> >>> encl_page = sgx_encl_page_alloc(encl, addr - encl->base, >>> secinfo_flags); >>> if (IS_ERR(encl_page)) >>> return VM_FAULT_OOM; >>> >>> This is AFAIK the only code path in sgx_vma_fault() flow that >>> allocates non-EPC memory, and the code paths where EPC allocation >>> fails the result would be SIGBUS. >>> >>> So perhaps allocation is failing here. You could pretty easily >>> trace allocations with bpftrace and kretprobe to see if this is >>> what is happening, e.g. in one terminal: >>> >>> sudo bpftrace -e 'kr:sgx_encl_page_alloc /retval != 0/ { >>> printf("%d\n", retval); }' >> >> Should be >> >> sudo bpftrace -e 'kr:sgx_encl_page_alloc /(long)retval < 0/ { >> printf("%d\n", retval); }' >> >> BR, Jarkko > > I tried these probs and got following results when failure happens (not > always happen on my device). > > sudo bpftrace -e 'kr:sgx_encl_page_alloc /(int64)retval <0 / { > printf("%X\n", retval); }' > > --> lots of negative values, I believe they are valid addresses in > unsigned long type. So I looked up IS_ERR_VALUE macro and translated it > in following probes. > > sudo bpftrace -e 'kr:sgx_encl_page_alloc /(uint64)retval >= > (uint64)(-4095)/ { printf("%X\n", retval); }' > > --> none triggered > > sudo bpftrace -e 'kr:sgx_alloc_epc_page /(uint64)retval >= > (uint64)(-4095)/ { printf("%X\n", retval); }' > > --> FFFFFFF0 printed, which I believe is -EBUSY. > Using this probe: sudo bpftrace -e 'kr:sgx_encl_grow /(uint64)retval >= (uint64)(-4095)/ { printf("%lX\n", retval); }' I found sgx_encl_grow could also return -EBUSY when allocating the VA page for the page to be EAUG'd. But we did not handle the case in sgx_encl_eaug_page, and follow changes seem to fix the problem with limited tests: diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c index 24c1bb8eb196..527de07c0e0b 100644 --- a/arch/x86/kernel/cpu/sgx/encl.c +++ b/arch/x86/kernel/cpu/sgx/encl.c @@ -345,7 +345,11 @@ static vm_fault_t sgx_encl_eaug_page(struct vm_area_struct *vma, va_page = sgx_encl_grow(encl, false); if (IS_ERR(va_page)) + { + if (PTR_ERR(va_page) == -EBUSY) + vmret = VM_FAULT_NOPAGE; goto err_out_epc; + } if (va_page) list_add(&va_page->list, &encl->va_pages); Will do more testing to verify. Let me know if the changes make sense. Thanks Haitao