From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 4EA0621D0918C for ; Wed, 26 Jul 2017 09:21:26 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5FF714A707; Wed, 26 Jul 2017 16:23:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 5FF714A707 Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-75.phx2.redhat.com [10.3.116.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id 809886A759; Wed, 26 Jul 2017 16:23:27 +0000 (UTC) To: Jordan Justen , edk2-devel-01 Cc: "Dr. David Alan Gilbert" References: <20170711032231.29280-1-lersek@redhat.com> <20170711032231.29280-2-lersek@redhat.com> <150102801569.20332.10802753116663936503@jljusten-skl> From: Laszlo Ersek Message-ID: <227cab99-7986-8161-4d32-7ad0679bf301@redhat.com> Date: Wed, 26 Jul 2017 18:23:26 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <150102801569.20332.10802753116663936503@jljusten-skl> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Wed, 26 Jul 2017 16:23:28 +0000 (UTC) Subject: Re: [PATCH 1/1] OvmfPkg/PlatformPei: support >=1TB high RAM, and discontiguous high RAM 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: Wed, 26 Jul 2017 16:21:26 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 07/26/17 02:13, Jordan Justen wrote: > On 2017-07-10 20:22:31, Laszlo Ersek wrote: >> +STATIC >> +EFI_STATUS >> +E820HighRamIterate ( >> + IN E820_HIGH_RAM_ENTRY_CALLBACK Callback, >> + IN OUT VOID *Context >> + ) > > I think a simpler option would be: > > STATIC > EFI_STATUS > ScanOrAddE820HighRam ( > IN E820_HIGH_RAM_ENTRY_CALLBACK Callback, > OUT UINT64 *MaxAddress OPTIONAL > ) > > If MaxAddress != NULL, then scan for it, otherwise add HOBs. > > Do you anticipate future needs where the iterate callback could be > helpful? Not at the moment. Originally I started with two open-coded loops, but the code duplication in this case was really ugly. Using callbacks simplified the call sites very nicely. And, I was a bit concerned that you wouldn't like a solution that wasn't generic enough :) I can rework the function like suggested if you prefer that. > > You might also consider ScanOrAdd64BitE820Ram to somewhat clarify that > the 'HighRam' is addresses that don't fit in 32 bits. OK. > >> + QemuFwCfgSelectItem (FwCfgItem); >> + for (Processed = 0; Processed < FwCfgSize; Processed += sizeof E820Entry) { >> + QemuFwCfgReadBytes (sizeof E820Entry, &E820Entry); >> + DEBUG (( >> + DEBUG_VERBOSE, >> + "%a: Base=0x%Lx Length=0x%Lx Type=%u\n", >> + __FUNCTION__, >> + E820Entry.BaseAddr, >> + E820Entry.Length, >> + E820Entry.Type >> + )); >> + if (E820Entry.Type == EfiAcpiAddressRangeMemory && >> + E820Entry.BaseAddr >= BASE_4GB) { > > I guess at least for IA32/X64, today, and for the foreseeable future > the firmware device will cause a break at 4GB. Sorry, I don't understand. What do you mean by "firmware device" and "break at 4GB"? > It seems like we could just check for the end of the range to be above > 4GB to easily remove this assumption, right? I wanted to ensure that no RAM range would be included that (for any unexpected reason) straddled the 4GB mark. Do you mean that the pflash address range is guaranteed to end at 4GB (exclusive), so no RAM range can straddle the mark? Even so, what expression do you have in mind exactly? As far as I can imagine, checking the end vs. the base of the E820 entry would only complicate the above expression, not simplify it. Please elaborate :) Thanks Laszlo