From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (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 9A5C32112121A for ; Thu, 6 Sep 2018 02:56:41 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E08247DAC2; Thu, 6 Sep 2018 09:56:40 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-62.rdu2.redhat.com [10.10.120.62]) by smtp.corp.redhat.com (Postfix) with ESMTP id 369162166B41; Thu, 6 Sep 2018 09:56:38 +0000 (UTC) To: "Ni, Ruiyu" , Andrew Fish Cc: Leif Lindholm , edk2-devel@lists.01.org, Jaben Carsey , Alexander Graf , Heinrich Schuchardt , AKASHI Takahiro , Mike Kinney References: <20180905172546.hxc2vqn6pgmr2zqs@bivouac.eciton.net> <8f700131-c4da-59bf-e1d3-0f4000e65215@redhat.com> <4C4EEEA0-8656-49D5-9DEE-8A810F6D8FC2@apple.com> <14abfd24-94eb-53f0-8ed6-721cbbc55137@Intel.com> From: Laszlo Ersek Message-ID: Date: Thu, 6 Sep 2018 11:56:38 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <14abfd24-94eb-53f0-8ed6-721cbbc55137@Intel.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Thu, 06 Sep 2018 09:56:40 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Thu, 06 Sep 2018 09:56:40 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: portability of ShellPkg X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 06 Sep 2018 09:56:41 -0000 Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit On 09/06/18 04:34, Ni, Ruiyu wrote: > On 9/6/2018 3:47 AM, Andrew Fish wrote: >> >> Laszlo, >> >> gEfiMemoryTypeInformationGuid is an edk2/MdeModulePkg concept used to >> give the DXE Core hints on how to reduce fragmentation in the memory >> map. Typically there is code in PEI that creates a HOB and may consume >> a variable written by the BDS. This library seems to be the generic >> way to do it on an edk2 platform. >> >> Thus this library is not just PI but edk2 MdeModulePkg specific in >> some of its assumptions. In general this UefiBootManagerLib seems >> focused on construction and edk2 BDS. It would probably make more >> sense to break out the UEFI Spec related bits so they could be used in >> generic UEFI Applications. >> >> Thanks, >> >> Andrew Fish >> >> > > Andrew, > I agree refactoring UefiBootManagerLib to separate the pure UEFI, pure > PI and EDKII specific looks much more cleaner. > So far the PI related bits in UefiBootManagerLib may include: > 1. Hob access for S4 support (memory type information HOB). > 2. Boot from Firmware Volume support. > > But that requires introducing two or more library classes so affecting > all existing platforms. EfiCreateEventLegacyBootEx() in UefiLib also > touches PI event gEfiEventLegacyBootGuid. > > And I think the value of refactor might be small. > > I think root cause of this problem is not UefiBootManagerLib includes > PI, it's the assertion in DxeHobLib. > So I am thinking maybe a very light fix is to remove the constructor of > DxeHobLib. > > I talked with Liming about this and he suggested that instead of > removing the constructor, it's safer to just remove the assertion in the > constructor. Because removing the constructor of HobLib may cause > AutoGen process generates a different order of library constructors > calling sequence, which may break the platform. > > So I propose to just remove the assertion in DxeHobLib constructor. > Thoughts? I think keeping the constructor itself is important and a good idea. I also think that we should *perhaps* keep the assertion *somewhere*, just not in the constructor. Because, at least some of the HobLib APIs cannot return errors (and their callers expect them to succeed at all times). This suggests we should still trip assertions when a HobLib API is called *in practice* on a non-PI UEFI platform. Thanks Laszlo