Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib: Reduce library API

2019-10-17 Thread Ni, Ray
nal Message- > From: Desimone, Nathaniel L > Sent: Thursday, October 17, 2019 3:27 PM > To: devel@edk2.groups.io; Ni, Ray ; Kubacki, Michael A > > Cc: Chaganty, Rangasai V > Subject: RE: [edk2-devel] [edk2-platforms][PATCH V1 1/1] > IntelSiliconPkg/BootMediaLib: Reduce

Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib: Reduce library API

2019-10-17 Thread Nate DeSimone
To: Kubacki, Michael A ; devel@edk2.groups.io Cc: Chaganty, Rangasai V Subject: Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib: Reduce library API > My concerns with that approach: > > 1. In general, I believe it is better if the library reads the valu

Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib: Reduce library API

2019-10-17 Thread Nate DeSimone
Reviewed-by: Nate DeSimone -Original Message- From: devel@edk2.groups.io On Behalf Of Kubacki, Michael A Sent: Monday, October 14, 2019 2:26 PM To: devel@edk2.groups.io Cc: Chaganty, Rangasai V ; Ni, Ray Subject: [edk2-devel] [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg

Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib: Reduce library API

2019-10-16 Thread Kubacki, Michael A
Thanks Ray, this was a good discussion. I will leave the library instances as-is for now. > -Original Message- > From: Ni, Ray > Sent: Tuesday, October 15, 2019 11:30 PM > To: Kubacki, Michael A ; > devel@edk2.groups.io > Cc: Chaganty, Rangasai V > Subject: RE: [edk2-platforms][PATCH

Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib: Reduce library API

2019-10-16 Thread Ni, Ray
> My concerns with that approach: > > 1. In general, I believe it is better if the library reads the value once and > caches it. The firmware boot media is fixed after power-on by nature and in > some platforms, boot media information may be provided to the IBB in a > temporary SRAM (or other

Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib: Reduce library API

2019-10-15 Thread Chaganty, Rangasai V
Thanks for reducing the redundant APIs. Reviewed-by: Sai Chaganty -Original Message- From: Kubacki, Michael A Sent: Monday, October 14, 2019 2:26 PM To: devel@edk2.groups.io Cc: Chaganty, Rangasai V ; Ni, Ray Subject: [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib:

Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib: Reduce library API

2019-10-15 Thread Kubacki, Michael A
My concerns with that approach: 1. In general, I believe it is better if the library reads the value once and caches it. The firmware boot media is fixed after power-on by nature and in some platforms, boot media information may be provided to the IBB in a temporary SRAM (or other volatile

Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib: Reduce library API

2019-10-14 Thread Ni, Ray
Mike, I don't think the library needs to cache the boot media type for RT and SMM. RT and SMM modules can have a module local variable to cache the boot media type. It simplifies the implementation of this library and also the platform DSC because there is no need to choose different library

Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib: Reduce library API

2019-10-14 Thread Kubacki, Michael A
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 /

Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib: Reduce library API

2019-10-14 Thread Ni, Ray
Reviewed-by: Ray Ni 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 > Sent: Tuesday, October 15, 2019 5:26 AM > To: devel@edk2.groups.io >

[edk2-devel] [edk2-platforms][PATCH V1 1/1] IntelSiliconPkg/BootMediaLib: Reduce library API

2019-10-14 Thread Kubacki, Michael A
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