[edk2-devel] [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib: Reduce library API
Kubacki, Michael A
michael.a.kubacki at intel.com
Tue Oct 15 02:21:35 UTC 2019
The two library instances work within the constraints of PEI, DXE, Runtime DXE, and SMM.
I cannot find how a single instance can support all these environments (in as clean a manner) as
is done with the two instances.
Relevant constraints:
* PEI: Cannot write to global variable
* Runtime DXE / SMM: Boot Services are not available outside module entry point
Solution:
* PEI instance: Always retrieve the value from a HOB (use heap for R/W).
* DXE, Runtime DXE, SMM instance: Retrieve the value from the HOB in the entry point
(constructor) when the driver is dispatched and Boot Services are available and store
the value in a global variable so it can be accessed in Runtime DXE and SMM.
Two separate instances resolve these requirements quite naturally.
How would you propose meeting the same level of support with one instance?
Thanks,
Michael
> -----Original Message-----
> From: Ni, Ray <ray.ni at intel.com>
> Sent: Monday, October 14, 2019 6:30 PM
> To: Kubacki, Michael A <michael.a.kubacki at intel.com>;
> devel at edk2.groups.io
> Cc: Chaganty, Rangasai V <rangasai.v.chaganty at intel.com>
> Subject: RE: [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib:
> Reduce library API
>
> Reviewed-by: Ray Ni <ray.ni at intel.com>
>
> Mike,
> Thanks for reducing the API.
>
> For the other comments I raised (single library instance usable for PEI and
> DXE), do you have any comments?
>
>
> > -----Original Message-----
> > From: Kubacki, Michael A <michael.a.kubacki at intel.com>
> > Sent: Tuesday, October 15, 2019 5:26 AM
> > To: devel at edk2.groups.io
> > Cc: Chaganty, Rangasai V <rangasai.v.chaganty at intel.com>; Ni, Ray
> > <ray.ni at intel.com>
> > Subject: [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib:
> > Reduce library API
> >
> > Removes the following functions from FirmwareBootMediaLib.h:
> > * FirmwareBootMediaIsSpi ()
> > * FirmwareBootMediaIsUfs ()
> > * FirmwareBootMediaIsEmmc ()
> > * FirmwareBootMediaIsNvme ()
> >
> > It is preferred to have a single method to retrieve the firmware boot
> media.
> > To reduce overall maintenance effort over time, the
> > FirmwareBootMediaIsXxx () pattern is removed in favor of returning the
> > firmware boot media type via GetFirmwareBootMediaType ().
> >
> > Cc: Sai Chaganty <rangasai.v.chaganty at intel.com>
> > Cc: Ray Ni <ray.ni at intel.com>
> > Signed-off-by: Michael Kubacki <michael.a.kubacki at intel.com>
> > ---
> >
> >
> Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmmFirm
> > wareBootMediaLib.inf | 1 -
> >
> >
> Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiFirmwareB
> > ootMediaLib.inf | 1 -
> > Silicon/Intel/IntelSiliconPkg/Include/Library/FirmwareBootMediaLib.h
> > | 48 ---------
> >
> >
> Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/FirmwareBoo
> > tMediaLib.c | 109 --------------------
> > 4 files changed, 159 deletions(-)
> >
> > diff --git
> >
> a/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmmFi
> > r
> > mwareBootMediaLib.inf
> >
> b/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmmFi
> > r
> > mwareBootMediaLib.inf
> > index 83ed5f04af..7e10b5f7a7 100644
> > ---
> >
> a/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmmFi
> > r
> > mwareBootMediaLib.inf
> > +++
> > b/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmm
> > +++ FirmwareBootMediaLib.inf
> > @@ -27,7 +27,6 @@
> > #
> >
> > [Sources]
> > - FirmwareBootMediaLib.c
> > DxeSmmFirmwareBootMediaLib.c
> >
> > [Packages]
> > diff --git
> > a/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiFirmw
> > ar
> > eBootMediaLib.inf
> > b/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiFirmw
> > ar
> > eBootMediaLib.inf
> > index 063c4027d3..ff1da31387 100644
> > ---
> > a/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiFirmw
> > ar
> > eBootMediaLib.inf
> > +++ b/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiF
> > +++ ir
> > +++ mwareBootMediaLib.inf
> > @@ -22,7 +22,6 @@
> > LIBRARY_CLASS = FirmwareBootMediaLib
> >
> > [Sources]
> > - FirmwareBootMediaLib.c
> > PeiFirmwareBootMediaLib.c
> >
> > [Packages]
> > diff --git
> > a/Silicon/Intel/IntelSiliconPkg/Include/Library/FirmwareBootMediaLib.h
> > b/Silicon/Intel/IntelSiliconPkg/Include/Library/FirmwareBootMediaLib.h
> > index aca9593a84..b36ebacf30 100644
> > ---
> > a/Silicon/Intel/IntelSiliconPkg/Include/Library/FirmwareBootMediaLib.h
> > +++ b/Silicon/Intel/IntelSiliconPkg/Include/Library/FirmwareBootMediaL
> > +++ ib
> > +++ .h
> > @@ -55,52 +55,4 @@ FirmwareBootMediaIsKnown (
> > VOID
> > );
> >
> > -/**
> > - Determines if the platform firmware is booting from SPI.
> > -
> > - @retval TRUE Platform firmware is booting from SPI
> > - @retval FALSE Platform firmware is booting from a non-SPI device or
> the
> > boot media is unknown
> > -**/
> > -BOOLEAN
> > -EFIAPI
> > -FirmwareBootMediaIsSpi (
> > - VOID
> > - );
> > -
> > -/**
> > - Determines if the platform firmware is booting from UFS.
> > -
> > - @retval TRUE Platform firmware is booting from UFS
> > - @retval FALSE Platform firmware is booting from a non-UFS device or
> > the boot media is unknown
> > -**/
> > -BOOLEAN
> > -EFIAPI
> > -FirmwareBootMediaIsUfs (
> > - VOID
> > - );
> > -
> > -/**
> > - Determines if the platform firmware is booting from eMMC.
> > -
> > - @retval TRUE Platform firmware is booting from eMMC
> > - @retval FALSE Platform firmware is booting from a non-eMMC device
> or
> > the boot media is unknown
> > -**/
> > -BOOLEAN
> > -EFIAPI
> > -FirmwareBootMediaIsEmmc (
> > - VOID
> > - );
> > -
> > -/**
> > - Determines if the platform firmware is booting from NVMe.
> > -
> > - @retval TRUE Platform firmware is booting from NVMe.
> > - @retval FALSE Platform firmware is booting from a non-NVMe device
> or
> > the boot media is unknown
> > -**/
> > -BOOLEAN
> > -EFIAPI
> > -FirmwareBootMediaIsNvme (
> > - VOID
> > - );
> > -
> > #endif
> > diff --git
> > a/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/Firmware
> > B
> > ootMediaLib.c
> > b/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/Firmware
> > B
> > ootMediaLib.c
> > deleted file mode 100644
> > index 11a14d172d..0000000000
> > ---
> > a/Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/Firmware
> > B
> > ootMediaLib.c
> > +++ /dev/null
> > @@ -1,109 +0,0 @@
> > -/** @file
> > - This library identifies the firmware boot media device.
> > -
> > - The firmware boot media device is used to make system
> > initialization decisions in the boot flow dependent
> > - upon firmware boot media. Note that the firmware boot media is the
> > storage media that the boot firmware is stored on.
> > - It is not the OS storage media which may be stored upon a different
> > non- volatile storage device.
> > -
> > - This file contains library implementation common to all boot phases.
> > -
> > -Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > -SPDX-License-Identifier: BSD-2-Clause-Patent
> > -
> > -**/
> > -
> > -#include <Library/BaseLib.h>
> > -#include <Library/DebugLib.h>
> > -#include <Library/FirmwareBootMediaLib.h>
> > -
> > -/**
> > - Determines if the platform firmware is booting from SPI.
> > -
> > - @retval TRUE Platform firmware is booting from SPI
> > - @retval FALSE Platform firmware is booting from a non-SPI device or
> the
> > boot media is unknown
> > -**/
> > -BOOLEAN
> > -EFIAPI
> > -FirmwareBootMediaIsSpi (
> > - VOID
> > - )
> > -{
> > - EFI_STATUS Status;
> > - FW_BOOT_MEDIA_TYPE BootMedia;
> > -
> > - Status = GetFirmwareBootMediaType (&BootMedia);
> > - if (EFI_ERROR (Status) || BootMedia != FwBootMediaSpi) {
> > - return FALSE;
> > - } else {
> > - return TRUE;
> > - }
> > -}
> > -
> > -/**
> > - Determines if the platform firmware is booting from UFS.
> > -
> > - @retval TRUE Platform firmware is booting from UFS
> > - @retval FALSE Platform firmware is booting from a non-UFS device or
> > the boot media is unknown
> > -**/
> > -BOOLEAN
> > -EFIAPI
> > -FirmwareBootMediaIsUfs (
> > - VOID
> > - )
> > -{
> > - EFI_STATUS Status;
> > - FW_BOOT_MEDIA_TYPE BootMedia;
> > -
> > - Status = GetFirmwareBootMediaType (&BootMedia);
> > - if (EFI_ERROR (Status) || BootMedia != FwBootMediaUfs) {
> > - return FALSE;
> > - } else {
> > - return TRUE;
> > - }
> > -}
> > -
> > -/**
> > - Determines if the platform firmware is booting from eMMC.
> > -
> > - @retval TRUE Platform firmware is booting from eMMC
> > - @retval FALSE Platform firmware is booting from a non-eMMC device
> or
> > the boot media is unknown
> > -**/
> > -BOOLEAN
> > -EFIAPI
> > -FirmwareBootMediaIsEmmc (
> > - VOID
> > - )
> > -{
> > - EFI_STATUS Status;
> > - FW_BOOT_MEDIA_TYPE BootMedia;
> > -
> > - Status = GetFirmwareBootMediaType (&BootMedia);
> > - if (EFI_ERROR (Status) || BootMedia != FwBootMediaEmmc) {
> > - return FALSE;
> > - } else {
> > - return TRUE;
> > - }
> > -}
> > -
> > -/**
> > - Determines if the platform firmware is booting from NVMe.
> > -
> > - @retval TRUE Platform firmware is booting from NVMe.
> > - @retval FALSE Platform firmware is booting from a non-NVMe device
> or
> > the boot media is unknown
> > -**/
> > -BOOLEAN
> > -EFIAPI
> > -FirmwareBootMediaIsNvme (
> > - VOID
> > - )
> > -{
> > - EFI_STATUS Status;
> > - FW_BOOT_MEDIA_TYPE BootMedia;
> > -
> > - Status = GetFirmwareBootMediaType (&BootMedia);
> > - if (EFI_ERROR (Status) || BootMedia != FwBootMediaNvme) {
> > - return FALSE;
> > - } else {
> > - return TRUE;
> > - }
> > -}
> > --
> > 2.16.2.windows.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#48969): https://edk2.groups.io/g/devel/message/48969
Mute This Topic: https://groups.io/mt/34538841/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list