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 230C221A134BB for ; Wed, 3 May 2017 06:46:43 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5B8D180472; Wed, 3 May 2017 13:46:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 5B8D180472 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 5B8D180472 Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-118.phx2.redhat.com [10.3.116.118]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4F92078C1F; Wed, 3 May 2017 13:46:40 +0000 (UTC) From: Laszlo Ersek To: Jordan Justen , edk2-devel-01 References: <20170429201500.18496-1-lersek@redhat.com> <149365940885.25909.1007719045522991203@jljusten-skl> <88d156c9-c18e-c4e8-b9a3-641a1b6b4102@redhat.com> <149366640991.26266.1222435765632598609@jljusten-skl> <62f44903-c06a-fb0f-0761-17cf9107620e@redhat.com> <149368192252.29568.13017173745830665833@jljusten-skl> <149374934642.31820.11722993986360080415@jljusten-skl> <60f5cceb-a49a-f0ee-389a-d603d2c62c06@redhat.com> <149376153469.32333.11251854925847683511@jljusten-skl> Cc: Gary Ching-Pang Lin Message-ID: <53c41649-06a4-a1e0-d294-acf129da556c@redhat.com> Date: Wed, 3 May 2017 15:46:39 +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: <149376153469.32333.11251854925847683511@jljusten-skl> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Wed, 03 May 2017 13:46:42 +0000 (UTC) Subject: Re: [PATCH 2/3] OvmfPkg: introduce FD_SIZE_4MB (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: Wed, 03 May 2017 13:46:43 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 05/02/17 23:45, Jordan Justen wrote: > On 2017-05-02 12:31:39, Laszlo Ersek wrote: >> On 05/02/17 20:22, Jordan Justen wrote: >>> On 2017-05-02 07:39:04, Laszlo Ersek wrote: >>>> >>>> I wouldn't mind if we made more room for the varstore in the 2MB build, >>>> even at the expense of FVMAIN_COMPACT, if we also kept the current 2MB >>>> build the default, so that the "new" (incompatible) 2MB build doesn't >>>> come as a surprise to unsuspecting downstreams. >>>> >>>> Regarding the 4MB build: >>>> - we can discuss that on top of the above "new" 2MB build, >>>> - we can discuss it instead of the above "new" 2MB build, >>>> - we can postpone it for now, for upstream. >>> >>> I was hoping there was a way to avoid the need to add 4MB, but you >>> needing to support the layout until 2024 really adds risk to the 2MB >>> image. I think there is a decent chance 2MB would work until then, but >>> I can also see how it adds significant risk. >>> >>> If we are adding the 4MB layout, then we may as well make it the >>> default for debug builds. >> >> OK, I think that's technically doable. Based on your commit e3dca1859b24 >> ("OvmfPkg: Increase default RELEASE build image size to 2MB", >> 2016-01-29), the $(TARGET) macro can be used in FDF files. >> >>> I'm not sure what to do about 2MB then. In >>> the short term, I say we do nothing. >> >> Do you mean "do nothing about 2MB", or "do nothing at all in the fdf.inc"? >> >> (You have to be really specific with me these days, sorry...) >> >> If I understand correctly, we'd have to reinstate FD_SIZE_2MB then, so >> that people that want to stick with the 2MB build for DEBUG (and NOOPT) >> can select it. Given that 4MB would become the new default for those >> targets. > > Ah. I guess I dropped FD_SIZE_2MB in e3dca1859b24, which I don't think > I should have done. Going forward, I think we should allow > FD_SIZE_1/2/4MB. Agreed. > > Regarding RELEASE builds, I'm not sure what we should do. Should we > just change it to 4MB as well? In the past, I preferred to allow > release builds to use the smaller size, since it fit. But, in this > case we also know that leaving 2MB size will mean a known test will > fail. The test failing doesn't mean a real user is likely to be > impacted, but I guess Microsoft feels the larger size may be required > in some scenarios. > > What do you think? (Maybe not a fair question since you don't use the > release build.) I guess the safe option is to just bump the default > for both the debug and release builds to the ridiculously large (er, I > mean luxuriously spacious :) 4MB image. Will do that in v2. (Maybe we should call the 4MB build FD_SIZE_LUXURIOUS_FRIDGE. :) ) > >>> I feel fairly confident of the 4MB image supporting your code size >>> needs until 2024. What seems less certain in future varstore >>> requirements. Right now the requirement is 120~128k. I think rather >>> than 248k in the 4MB layout, we should make it 256k. (Since these >>> kinds of things often progress in powers-of-two.) It will make for a >>> couple unfriendly offsets, but it seems worth it to avoid being 8k shy >>> of the next power-of-two size. >>> >>> In my other email, I mentioned the event-log. I did ask around a bit >>> about this, but I didn't find anyone willing to fight for more space. >>> Therefore, I think we should just keep it at 4k. >> >> That means 256K for the varstore, 4K for the event log, 4K for the FTW >> working block. >> >> How much for the spare area? Currently the spare area equals the sum of >> the former three. The spare area is used both while reclaiming the >> varstore, and while reclaiming the FTW working block. (Not sure about >> the event log.) So I'd say we should stick with our tradition, and make >> the spare area 256K + 4K + 4K = 264K in size. That would result in a >> 528K NVRAM. (Which is 16K larger than in the current patch.) >> >> In turn, I wouldn't increase FVMAIN_COMPACT by 1664K, to 3376K, but by >> 16K less (1648K) to 3360K. The full FD size would remain 4M. >> >> Does this sound okay? > > Yes. Yay! \o/ > This will leave the split rom sizes being a multiple of 16k rather > than 512k. Today they are a multiple of 128k. I don't expect this > would be an issue for qemu/kvm. Do you agree? I agree. In "hw/i386/pc_sysfw.c", QEMU expects the pflash chip sizes to be multiples of 4KB (1 << 12). I'll attempt to write, test and post v2 today. Thank you, Laszlo