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 C839621A04823 for ; Mon, 1 May 2017 11:41:00 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 38091558B3; Mon, 1 May 2017 18:41:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 38091558B3 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 38091558B3 Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-22.phx2.redhat.com [10.3.117.22]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2759B5C551; Mon, 1 May 2017 18:40:58 +0000 (UTC) To: Jordan Justen , edk2-devel-01 References: <20170429201500.18496-1-lersek@redhat.com> <20170429201500.18496-3-lersek@redhat.com> <149351328512.20670.1563878734495138189@jljusten-skl> <030f8312-35ce-5c86-205c-2ee6c0b5ab8b@redhat.com> <149358697668.23065.6363402854761002239@jljusten-skl> <149365940885.25909.1007719045522991203@jljusten-skl> Cc: Gary Ching-Pang Lin From: Laszlo Ersek Message-ID: <88d156c9-c18e-c4e8-b9a3-641a1b6b4102@redhat.com> Date: Mon, 1 May 2017 20:40:58 +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: <149365940885.25909.1007719045522991203@jljusten-skl> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Mon, 01 May 2017 18:41:00 +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: Mon, 01 May 2017 18:41:01 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 05/01/17 19:23, Jordan Justen wrote: > On 2017-05-01 03:51:42, Laszlo Ersek wrote: >> On 04/30/17 23:16, Jordan Justen wrote: >>> On 2017-04-30 07:42:36, Laszlo Ersek wrote: >>> >>>> $ build \ >>>> -b DEBUG \ >>>> -a IA32 -a X64 \ >>>> -p OvmfPkg/OvmfPkgIa32X64.dsc \ >>>> -t GCC48 \ >>>> -D SMM_REQUIRE \ >>>> -D SECURE_BOOT_ENABLE \ >>>> -D HTTP_BOOT_ENABLE \ >>>> -D NETWORK_IP6_ENABLE \ >>>> -D TLS_ENABLE >>> >>> Do you enable the last 3 in your production builds? I didn't think it >>> was the case, but it would change things... >> >> That's a very good question, and I expected it. >> >> Any sane person being responsible for supporting a package will strive >> very hard to minimize the features enabled in the package, in order to >> minimize the problem surface / support burden. I tend to consider myself >> a sane person, so no, HTTP_BOOT_ENABLE, NETWORK_IP6_ENABLE, and >> TLS_ENABLE are not turned on. >> >> (TLS_ENABLE carries even more weight, because it increases the security >> attack surface, so turning *that* off is very desirable.) >> >> *But*, I certainly want to keep the *ability* to turn these features on >> (and maybe later features, in 2-3 years' time) if a customer or a >> partner requests it. > > It sounds like you don't expect to 'support' this. At least not to the > same level as the rest of the firmware. I hope never to have to support these, but at some point into the future of RHEL7, I might have no choice. > I think it is fine to say, if you want to enable these, you may have > to disable debug on some other features, As I explained earlier, universal DEBUG coverage is a requirement for supportability. ( Example: in parallel to this discussion, our virt QE reported an independent issue, namely an out-of-SMRAM condition with 240+ VCPUs. https://bugzilla.redhat.com/show_bug.cgi?id=1447027 The out-of-SMRAM condition is caught by an ASSERT() only. It is in "UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c", function SetStaticPageTable(): 212 PageDirectoryEntry = AllocatePageTableMemory (1); 213 ASSERT(PageDirectoryEntry != NULL); 214 ZeroMem (PageDirectoryEntry, EFI_PAGES_TO_SIZE(1)); I find it plausible that if this memory allocation fails, then the firmware has really no way to continue -- that's okay. But, the ASSERT() disappears in a RELEASE build -- not okay. Memory allocation failures should never be handled with *just* an ASSERT(). I hope you appreciate my insistence on DEBUG a bit more, through this real-life example. ) > or remove some other features. Removing features on purpose can be called "offensive regression" in an enterprise distro, and it is the antithesis of why people decide to pay for enterprise support. > In other words, at this point I don't think the size of these should > be added into the equation for how 'full' the 2MB image is. I think at this point it is clear that these patches are not appropriate for upstream OvmfPkg. That's an acceptable closure for me -- while I would have preferred to get the patches in, the real moral imperative for me is to *try* the upstreaming at least, whenever I think the patches are applicable to upstream. I think we've had an honest and thorough discussion on this -- thank you very much for taking the time! (Especially over the weekend!) The current upstream 2MB build should continue working for everyday purposes. Cheers Laszlo