Re: [edk2-devel] [edk2-platforms][PATCH v1 1/1] IntelSiliconPkg: Add BaseSmmAccessLibNull

["Michael Kubacki" <mikuback@linux.microsoft.com>]

Since you asked for an example that was just one that I provided. I 
don't think it detracts from the fact that a NULL instance makes sense 
if the SmmAccessLib library class exists. The fact that a NULL instance 
could not be allowed to exist is also confusing.

I don't want to get too distracted with the example given, but I 
completely agree that a different library instance should be used for 
pre-memory and post-memory. I think the library interface is too broad 
in scope and that contributes to causing this issue so I filed this BZ 
to request the BoardInitLib API be refactored:
https://bugzilla.tianocore.org/show_bug.cgi?id=3578

 From the other email thread about SmmAccessLib, I think we are on the 
same page that the library would be better as a PEIM. Is that something 
that could be done soon? Or could we have this until that is done?

Thanks,
Michael

On 8/20/2021 1:33 AM, Ni, Ray wrote:
> Null SmmAccessLib is confusing to me. Have you evaluated the option:
> Create two instances of BoardInitLib for pre-mem and post-mem. Pre-mem
> one doesn’t link to SmmAccessLib
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
>> Sent: Friday, August 20, 2021 12:53 AM
>> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>
>> Subject: Re: [edk2-devel] [edk2-platforms][PATCH v1 1/1] IntelSiliconPkg: Add BaseSmmAccessLibNull
>>
>> I don't understand your argument.
>>
>> The library class (SmmAccessLib) that already exists is the abstraction
>> layer. This is not introducing a new layer of abstraction. It is using
>> the current layer of abstraction.
>>
>> Thanks,
>> Michael
>>
>> On 8/19/2021 5:49 AM, Ni, Ray wrote:
>>> Michael,
>>>
>>> I don’t think scenario #1 is a good reason for NULL instance of
>>> SmmAccessLib. The root cause is BoardInitLib lib class supports pre-mem
>>> and post-mem board init.
>>>
>>> Below solution can avoid NULL SmmAccessLib:
>>>
>>> Create two instances of BoardInitLib for pre-mem and post-mem. Pre-mem
>>> one doesn’t link to SmmAccessLib.
>>>
>>> For scenario #2, if a particular platform doesn’t support S3, why does
>>> this platform include the PEIM?
>>>
>>> Please understand that I want to avoid introducing more abstraction layers.
>>>
>>> Thanks,
>>>
>>> Ray
>>>
>>> *From:* Michael Kubacki <mikuback@linux.microsoft.com>
>>> *Sent:* Friday, August 13, 2021 10:16 AM
>>> *To:* Ni; Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
>>> *Subject:* Re: [edk2-devel] [edk2-platforms][PATCH v1 1/1]
>>> IntelSiliconPkg: Add BaseSmmAccessLibNull
>>>
>>> Sure.
>>>
>>> Scenario #1:
>>>
>>> MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPreMem.inf and
>>> MinPlatormPkg/PlatformInit/PlatformInitPei/PlatformInitPostMem.inf both
>>> link against an instance of BoardInitLib.
>>>
>>> Many boards link against a single BoardInitLib instance. See example -
>>> https://github.com/tianocore/edk2-
>> platforms/blob/cd4e6b716c7d1bcde94035e7dce14b53a553e103/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenB
>> oardPkg.dsc#L203
>>> <https://github.com/tianocore/edk2-
>> platforms/blob/cd4e6b716c7d1bcde94035e7dce14b53a553e103/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenB
>> oardPkg.dsc#L203>
>>>
>>> That BoardInitLib instance may link against SmmAccessLib.
>>> PlatformInitPreMem may wish to library class override the SmmAccessLib
>>> to the NULL instance while keeping it to non-NULL instance in
>>> PlatformInitPostMem.
>>>
>>> Scenario #2:
>>>
>>> A PEIM is built that checks whether the boot mode is S3. If so, it calls
>>> PeiInstallSmmAccessPpi(). A particular platform does not support S3,
>>> therefore, it links BaseSmmAccessLibNull as its library instance for
>>> SmmAccessLib.
>>>
>>>
>>
>>
>> 
>>
>