public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Kubacki, Michael A" <michael.a.kubacki@intel.com>
To: "Chaganty, Rangasai V" <rangasai.v.chaganty@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Dong, Eric" <eric.dong@intel.com>,
	"Gao, Liming" <liming.gao@intel.com>,
	"Ni, Ray" <ray.ni@intel.com>
Subject: Re: [edk2-platforms][PATCH V1 0/3] Add FW Boot Media Device Indicator
Date: Thu, 3 Oct 2019 01:02:28 +0000	[thread overview]
Message-ID: <DM6PR11MB3834A9C873F5758D2791B1B6B59F0@DM6PR11MB3834.namprd11.prod.outlook.com> (raw)
In-Reply-To: <BCAAFC0A0683754C9A88D2C4E3F3A9C7F2F49C6E@FMSMSX108.amr.corp.intel.com>

In platforms built for boot media other than SPI flash there has been a compelling
need for silicon and platform code to be aware of the firmware boot media but
apart from the UEFI variable driver (which is a special case being addressed
here - https://github.com/makubacki/edk2/tree/storage_agnostic_uefi_variables),
this has not been the case for edk2 repository packages. In some cases, code in SEC
has made assumptions about the reset vector or FIT pointer that do not necessarily
translate to storage media that does not support MMIO. These cases have been
handled more gracefully than checking the firmware boot media technology. This is
just an observation, not necessarily a case for it to stay in IntelSiliconPkg (which
does make it accessible to Intel silicon and platform code).

I suppose the firmware boot media properties could be treated in a similar manner
to Boot Mode as defined in the Platform Initialization spec. If this was done, it may
make more sense to abstract the technology impact onto firmware, for example,
whether it supports MMIO or not (NOR vs NAND flash) instead of what is defined
here with specific technologies such as eMMC and UFS. Otherwise, the specification
describing this would be subject to constant expansion over time keeping pace with
new technologies and cross-generation code (not silicon or platform specific drivers)
may base conditionals on something like eMMC when its algorithm really applies to
all NAND media (which, again, has been the proven observation thus far outside
silicon and platform code).

With that said, I see the firmware boot technology details being more pertinent to
silicon and platform code so that's why this change is made now. It can immediately
address existing needs for these details in silicon and platform code.  Some form of
the firmware boot media details in a more generic package could be useful but it
will likely not align closely with the scope of information needed at this level and is
an undertaking, that in my opinion, is separate but compatible with the work done here.

> -----Original Message-----
> From: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>
> Sent: Wednesday, October 2, 2019 4:03 PM
> To: Kubacki, Michael A <michael.a.kubacki@intel.com>;
> devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Gao, Liming <liming.gao@intel.com>;
> Ni, Ray <ray.ni@intel.com>
> Subject: RE: [edk2-platforms][PATCH V1 0/3] Add FW Boot Media Device
> Indicator
> 
> I am not sure if there is a silicon scope around the FirmwareBootMediaLib.
> Have we considered adding this interface to MdePkg, instead?
> 
> -----Original Message-----
> From: Kubacki, Michael A
> Sent: Monday, September 30, 2019 6:16 PM
> To: devel@edk2.groups.io
> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Gao, Liming <liming.gao@intel.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: [edk2-platforms][PATCH V1 0/3] Add FW Boot Media Device
> Indicator
> 
> This patch series introduces a mechanism for determining the firmware boot
> media device. This allows the firmware boot media to be discovered through
> a standardized API.
> 
> Traditionally, most systems have only supported firmware storage on SPI
> flash. Presently, several other storage technologies are being used to store
> boot system firmware such as eMMC, UFS, and NVMe.
> 
> The API for all board, platform, and silicon code to consume the firmware
> boot media device is provided by the FirmwareBootMediaLib in
> IntelSiliconPkg.
> 
> A driver (FirmwareBootMediaInfoPei) is added to BoardModulePkg to serve
> as a consistent location for reporting the firmware boot device information.
> In order to abstract the potentially hardware-specific details to determine
> the boot media (for platforms that support multiple firmware boot media
> devices), the driver retrieves the boot media information using a new library
> class introduced called FirmwareBootMediaInfoLib. A default instance of this
> library class is provided in BoardModulePkg that always returns SPI flash. This
> is intended to serve as a default implementation of the library for the most
> common scenario and to easily allow a board package to substitute the logic
> required to determine the boot media in more complex scenarios.
> Ultimately, FirmwareBootMediaInfoPei produces a HOB containing the
> firmware boot media device information so it can be used in the HOB
> consumer phase.
> 
> Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
> 
> Michael Kubacki (3):
>   IntelSiliconPkg/FirmwareBootMediaLib: Add library
>   BoardModulePkg/FirmwareBootMediaInfoLib: Add library
>   BoardModulePkg/FirmwareBootMediaInfoPei: Add module
> 
>  Platform/Intel/BoardModulePkg/BoardModulePkg.dec
> |   3 +
>  Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec                                                 |   4
> +-
>  Platform/Intel/BoardModulePkg/BoardModulePkg.dsc
> |   5 +
>  Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc                                                 |   4
> +-
> 
> Platform/Intel/BoardModulePkg/FirmwareBootMediaInfo/FirmwareBootMe
> diaInfoPei.inf                  |  46 +++++++++
> 
> Platform/Intel/BoardModulePkg/Library/PeiFirmwareBootMediaInfoLib/Pei
> FirmwareBootMediaInfoLib.inf |  35 +++++++
> 
> Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmmFirm
> wareBootMediaLib.inf        |  43 ++++++++
> 
> Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiFirmwareB
> ootMediaLib.inf           |  38 +++++++
> 
> Platform/Intel/BoardModulePkg/Include/Library/FirmwareBootMediaInfoLi
> b.h                          |  26 +++++
>  Silicon/Intel/IntelSiliconPkg/Include/Library/FirmwareBootMediaLib.h
> | 106 +++++++++++++++++++
> 
> Platform/Intel/BoardModulePkg/FirmwareBootMediaInfo/FirmwareBootMe
> diaInfoPei.c                    |  76 ++++++++++++++
> 
> Platform/Intel/BoardModulePkg/Library/PeiFirmwareBootMediaInfoLib/Pei
> FirmwareBootMediaInfoLib.c   |  24 +++++
> 
> Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmmFirm
> wareBootMediaLib.c          | 107 +++++++++++++++++++
> 
> Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/FirmwareBoo
> tMediaLib.c                | 109 ++++++++++++++++++++
> 
> Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiFirmwareB
> ootMediaLib.c             |  82 +++++++++++++++
>  15 files changed, 706 insertions(+), 2 deletions(-)  create mode 100644
> Platform/Intel/BoardModulePkg/FirmwareBootMediaInfo/FirmwareBootMe
> diaInfoPei.inf
>  create mode 100644
> Platform/Intel/BoardModulePkg/Library/PeiFirmwareBootMediaInfoLib/Pei
> FirmwareBootMediaInfoLib.inf
>  create mode 100644
> Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmmFirm
> wareBootMediaLib.inf
>  create mode 100644
> Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiFirmwareB
> ootMediaLib.inf
>  create mode 100644
> Platform/Intel/BoardModulePkg/Include/Library/FirmwareBootMediaInfoLi
> b.h
>  create mode 100644
> Silicon/Intel/IntelSiliconPkg/Include/Library/FirmwareBootMediaLib.h
>  create mode 100644
> Platform/Intel/BoardModulePkg/FirmwareBootMediaInfo/FirmwareBootMe
> diaInfoPei.c
>  create mode 100644
> Platform/Intel/BoardModulePkg/Library/PeiFirmwareBootMediaInfoLib/Pei
> FirmwareBootMediaInfoLib.c
>  create mode 100644
> Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmmFirm
> wareBootMediaLib.c
>  create mode 100644
> Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/FirmwareBoo
> tMediaLib.c
>  create mode 100644
> Silicon/Intel/IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiFirmwareB
> ootMediaLib.c
> 
> --
> 2.16.2.windows.1
> 


  reply	other threads:[~2019-10-03  1:02 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-01  1:15 [edk2-platforms][PATCH V1 0/3] Add FW Boot Media Device Indicator Kubacki, Michael A
2019-10-01  1:15 ` [edk2-platforms][PATCH V1 1/3] IntelSiliconPkg/FirmwareBootMediaLib: Add library Kubacki, Michael A
2019-10-01  1:15 ` [edk2-platforms][PATCH V1 2/3] BoardModulePkg/FirmwareBootMediaInfoLib: " Kubacki, Michael A
2019-10-08  7:01   ` Dong, Eric
2019-10-01  1:15 ` [edk2-platforms][PATCH V1 3/3] BoardModulePkg/FirmwareBootMediaInfoPei: Add module Kubacki, Michael A
2019-10-02 23:03 ` [edk2-platforms][PATCH V1 0/3] Add FW Boot Media Device Indicator Chaganty, Rangasai V
2019-10-03  1:02   ` Kubacki, Michael A [this message]
2019-10-03  4:20     ` [edk2-devel] " Andrew Fish
2019-10-03  7:54       ` Kubacki, Michael A
2019-10-07  1:34         ` Chaganty, Rangasai V

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=DM6PR11MB3834A9C873F5758D2791B1B6B59F0@DM6PR11MB3834.namprd11.prod.outlook.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