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 D986321A04804 for ; Fri, 5 May 2017 09:46:08 -0700 (PDT) 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 2821A61D38; Fri, 5 May 2017 16:46:08 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 2821A61D38 Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 2821A61D38 Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-120.phx2.redhat.com [10.3.116.120]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2A0D817970; Fri, 5 May 2017 16:46:06 +0000 (UTC) To: Jordan Justen References: <20170503213947.32290-1-lersek@redhat.com> <20170503213947.32290-4-lersek@redhat.com> <149397463765.13335.18098739662202283758@jljusten-skl> <9ff79644-8767-bf10-7d0a-2f5ab6555e23@redhat.com> <149399903262.14207.13532012674948464509@jljusten-skl> Cc: edk2-devel-01 , Gary Ching-Pang Lin From: Laszlo Ersek Message-ID: Date: Fri, 5 May 2017 18:46:06 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <149399903262.14207.13532012674948464509@jljusten-skl> 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.39]); Fri, 05 May 2017 16:46:08 +0000 (UTC) Subject: Re: [PATCH v2 3/5] OvmfPkg: introduce 4MB flash image (mainly) for Windows HCK 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: Fri, 05 May 2017 16:46:09 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 05/05/17 17:43, Jordan Justen wrote: > On 2017-05-05 05:07:25, Laszlo Ersek wrote: >> >> Which one do you prefer: >> (1) I can take your patch, stick in the UINTN cast, and expand the >> commit message a bit (similarly to what's on my patch), >> (2) we can go with my patch as well. > > I prefer your patch. > > Re: "0001-OvmfPkg-PlatformPei-handle-non-power-of-two-spare-si.patch" > >> + Alignment = GetPowerOfTwo32 (Alignment) << 1; > > Do you think this might end up needing a UINT32 cast with 64-bit PEI? No, the prototype is UINT32 EFIAPI GetPowerOfTwo32 ( IN UINT32 Operand ) and we can safely shift the returned UINT32 in both 32-bit PEI and 64-bit PEI with the << operator. The result of the shift will also have type UINT32, which we can safely re-assign to the UINT32 Alignment variable. Shifting the result of GetPowerOfTwo64() with << would be problematic in 32-bit PEI. > > Reviewed-by: Jordan Justen Thanks! I verified this in the (now again default 2MB build) with the -bios command line like this: (1) boot to the UEFI shell normally. The log says: > Reserved variable store memory: 0x7F50000; size: 128kb, > alignment: 0x10000 > ... > EMU Variable FVB Started > EMU Variable FVB: Using pre-reserved block at 7F50000 > EMU Variable FVB: Basic FV headers were invalid > Installing FVB for EMU Variable support > ... > Ftw: FtwWorkSpaceLba - 0x0, WorkBlockSize - 0x10000, > FtwWorkSpaceBase - 0xE000 > Ftw: FtwSpareLba - 0x1, SpareBlockSize - 0x10000 > Ftw: NumberOfWorkBlock - 0x1, FtwWorkBlockLba - 0x0 > Ftw: WorkSpaceLbaInSpare - 0x0, WorkSpaceBaseInSpare - 0xE000 > Ftw: Remaining work space size - FE0 > Ftw: Work block header check mismatch > Ftw: Work block header check mismatch > Ftw: Both working and spare blocks are invalid, init workspace > Ftw: start to reclaim work space > Ftw: reclaim work space successfully (2) run the following shell commands: Shell> setvar TestVar0000 -guid ebe29c42-f3d1-4f96-a7c2-5585ce88f056 -bs -nv ="0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" Shell> reset (3) after reboot, the log says: > Reserved variable store memory: 0x7F50000; size: 128kb, > alignment: 0x10000 > ... > EMU Variable FVB Started > EMU Variable FVB: Using pre-reserved block at 7F50000 > EMU Variable FVB: Found valid pre-existing FV > ... > Ftw: FtwWorkSpaceLba - 0x0, WorkBlockSize - 0x10000, > FtwWorkSpaceBase - 0xE000 > Ftw: FtwSpareLba - 0x1, SpareBlockSize - 0x10000 > Ftw: NumberOfWorkBlock - 0x1, FtwWorkBlockLba - 0x0 > Ftw: WorkSpaceLbaInSpare - 0x0, WorkSpaceBaseInSpare - 0xE000 > Ftw: Remaining work space size - FE0 (4) run the following shell command: Shell> setvar TestVar0000 -guid ebe29c42-f3d1-4f96-a7c2-5585ce88f056 EBE29C42-F3D1-4F96-A7C2-5585CE88F056 - TestVar0000 - 0040 Bytes 01 23 45 67 89 AB CD EF 01 23 45 67 89 AB CD EF 01 23 45 67 89 AB CD EF 01 23 45 67 89 AB CD EF 01 23 45 67 89 AB CD EF 01 23 45 67 89 AB CD EF 01 23 45 67 89 AB CD EF 01 23 45 67 89 AB CD EF Also tested this with the 4MB build, using pflash. The log says > Reserved variable store memory: 0xBFE80000; size: 528kb, > alignment: 0x80000 The alignment is 512KB, which is the next power of two after 264KB. > >> Regarding the non-flash path, I have the attached work-in-progress patches: >> >> - "OvmfPkg: sync PcdVariableStoreSize with PcdFlashNvStorageVariableSize", >> - and "wip". > > I looked over the non-wip patch, and it seems good, but maybe we > should just revert the default size for now. Works for me. > For a revert of bba8dfbec3bbc4fba7fa6398ba3cf76593e0725e: > > Reviewed-by: Jordan Justen > Thanks! Commits: 1 6e49d01cfb43 Revert "OvmfPkg: make the 4MB flash size the default" 2 0c79471d6a98 OvmfPkg/PlatformPei: handle non-power-of-two spare size for emu variables I've also filed the following TianoCore Feature Request: https://bugzilla.tianocore.org/show_bug.cgi?id=527 If you or Gary feel inclined to work on this, that would be awesome. I have no idea when I'll get to it; my TODO list hasn't been this long in ages. Of course I cannot really *request* that you guys please pick up my slack, just sayin' that you please feel free to... :) Thanks! Laszlo