From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 0A4012034D816 for ; Thu, 9 Nov 2017 05:11:38 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3BA83369BD; Thu, 9 Nov 2017 13:15:40 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-19.rdu2.redhat.com [10.10.120.19]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4F9BD6F132; Thu, 9 Nov 2017 13:15:38 +0000 (UTC) To: "Ni, Ruiyu" , "Justen, Jordan L" , Jeff Fan Cc: "Kinney, Michael D" , "edk2-devel@lists.01.org" , "Yao, Jiewen" , "Dong, Eric" , Ard Biesheuvel References: <20171012084810.148196-1-ruiyu.ni@intel.com> <20171012084810.148196-4-ruiyu.ni@intel.com> <03e369bb-77c4-0134-258f-bdae62cbc8c5@redhat.com> <151019243761.10467.634318081879242382@jljusten-skl> <734D49CCEBEEF84792F5B80ED585239D5BAB7B62@SHSMSX104.ccr.corp.intel.com> <151021054953.14125.10727630216824816281@jljusten-skl> <734D49CCEBEEF84792F5B80ED585239D5BAB804D@SHSMSX104.ccr.corp.intel.com> From: Laszlo Ersek Message-ID: <9474d983-b1c1-b83b-34ef-10bb84586ef6@redhat.com> Date: Thu, 9 Nov 2017 14:15:37 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5BAB804D@SHSMSX104.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 09 Nov 2017 13:15:40 +0000 (UTC) Subject: Re: [PATCH 3/4] UefiCpuPkg/MtrrLib: Update algorithm to calculate optimal settings X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Nov 2017 13:11:39 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 11/09/17 08:11, Ni, Ruiyu wrote: > The old version is not just slow. > It cannot work in certain cases. That's why the new version was developed. > > The new version adds a new API which allows caller to pass in the scratch > buffer instead of using the stack. If a platform has limited stack, it can use > that API. (1) OK, so let me summarize: (1a) Ray and Jeff confirmed that the AP stacks should be affected in *neither* PiSmmCpuDxeSmm *nor* the MpInitLib client modules (CpuMpPei, CpuDxe). (1b) There is a new MtrrLib class API that allows the client module to pass in a preallocated scratch buffer. (1a) sounds great. (1b) is an option we may or may not want to exercise in OVMF. I have the patches ready for enlarging the temp SEC/PEI RAM (and as part of that, the temp stack), which is one alternative. However, because OVMF's PEI phase runs from RAM (and not flash), the other alternative is just to add a sufficiently large static UINT8 array to PlatformPei, and pass that as scratch space to MtrrLib. (2) However: I don't know *how* the new API -- MtrrSetMemoryAttributesInMtrrSettings() -- is supposed to be used. In OvmfPkg/PlatformPei, we have the following MtrrLib calls: * OvmfPkg/PlatformPei/Xen.c: - MtrrSetMemoryAttribute() (in a loop, basically) * OvmfPkg/PlatformPei/MemDetect.c: - IsMtrrSupported() - MtrrGetAllMtrrs() - MtrrSetAllMtrrs() - MtrrSetMemoryAttribute() Is my understanding correct that MtrrSetMemoryAttribute() is the only function that is affected? (3) Is my understanding correct that MtrrSetMemoryAttributesInMtrrSettings() should be used like this: (3a) start with MtrrGetAllMtrrs() (3b) collect all *foreseeable* MtrrSetMemoryAttribute() calls into an array of MTRR_MEMORY_RANGE elements (3c) Perform a batch update on the output of (3a) by calling MtrrSetMemoryAttributesInMtrrSettings(). For this, the array from (3b), plus a caller-allocated scratch space, have to be passed in,. (3d) Finally, call MtrrSetAllMtrrs(). Is this correct? I think we could use this. Jordan, which alternative do you prefer; larger stack and unchanged code in PlatformPei, or same stack and updated code in PlatformPei? (4) Ray: would it be possible to expose SCRATCH_BUFFER_SIZE (with a new name MTRR_SCRATCH_BUFFER_SIZE) in the library class header? I see the new RETURN_BUFFER_TOO_SMALL status codes, and I don't really want to deal with them. The library class header should provide clients with a size macro that *guarantees* that RETURN_BUFFER_TOO_SMALL will not occur. Practically speaking, I would use MTRR_SCRATCH_BUFFER_SIZE in the definition of the static UINT8 array in PlatformPei. (If Jordan prefers this alternative to the larger temp stack.) Thanks! Laszlo >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of >> Jordan Justen >> Sent: Thursday, November 9, 2017 2:56 PM >> To: Ni, Ruiyu ; Laszlo Ersek >> Cc: Kinney, Michael D ; edk2- >> devel@lists.01.org; Yao, Jiewen ; Dong, Eric >> ; Ard Biesheuvel >> Subject: Re: [edk2] [PATCH 3/4] UefiCpuPkg/MtrrLib: Update algorithm to >> calculate optimal settings >> >> On 2017-11-08 19:04:35, Ni, Ruiyu wrote: >>> Jordan, Laszlo, >>> >>> I didn't realize that a platform may have less than 4-page stack >>> before memory is ready. If I was aware of that, I would change the >>> default scratch buffer size to 2 page, which should be enough too. >> >> This does not sound much better. I'm saying that the BASE library should only >> use at most a few hundred bytes of stack. >> >> Apparently the old algorithm did not use much memory, but perhaps was >> slow? Can we put it back in place for the BASE version of the library? >> Then, we can add a DXE specific version that uses a large buffer which it can >> allocate, and potentially free. >> >> -Jordan >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel