public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ni, Ruiyu" <ruiyu.ni@Intel.com>
To: Andrew Fish <afish@apple.com>, Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>,
	edk2-devel@lists.01.org, Jaben Carsey <jaben.carsey@intel.com>,
	Alexander Graf <agraf@suse.de>,
	Heinrich Schuchardt <xypron.glpk@gmx.de>,
	AKASHI Takahiro <takahiro.akashi@linaro.org>,
	Mike Kinney <michael.d.kinney@intel.com>
Subject: Re: portability of ShellPkg
Date: Thu, 6 Sep 2018 10:34:14 +0800	[thread overview]
Message-ID: <14abfd24-94eb-53f0-8ed6-721cbbc55137@Intel.com> (raw)
In-Reply-To: <4C4EEEA0-8656-49D5-9DEE-8A810F6D8FC2@apple.com>

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?



-- 
Thanks,
Ray


  reply	other threads:[~2018-09-06  2:33 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-05 17:25 portability of ShellPkg Leif Lindholm
2018-09-05 17:30 ` Carsey, Jaben
2018-09-05 17:41   ` Leif Lindholm
2018-09-05 18:03   ` Andrew Fish
2018-09-05 18:05     ` Carsey, Jaben
2018-09-05 18:20       ` Andrew Fish
2018-09-05 18:23         ` Carsey, Jaben
2018-09-05 18:33           ` Andrew Fish
2018-09-05 18:53             ` Carsey, Jaben
2018-09-05 18:43 ` Laszlo Ersek
2018-09-05 19:47   ` Andrew Fish
2018-09-06  2:34     ` Ni, Ruiyu [this message]
2018-09-06  9:56       ` Laszlo Ersek
2018-09-06 17:17         ` Kinney, Michael D
2018-09-06 22:31           ` Andrew Fish

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=14abfd24-94eb-53f0-8ed6-721cbbc55137@Intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox