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

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

So you would rather leave it as a library class instead of refactoring 
it to a PEIM?

Again, the problem is it is a library class. So I am asking whether you 
want to treat it as a library class or you are going to refactor it to a 
PEIM.

On 9/9/2021 10:49 AM, Ni, Ray wrote:
> No, I don't.
> I still don't think having a NULL SmmAccessLib is a good idea.
> 
>> -----Original Message-----
>> From: Michael Kubacki <mikuback@linux.microsoft.com>
>> Sent: Thursday, September 9, 2021 10:12 PM
>> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
>> Subject: Re: [edk2-devel] [edk2-platforms][PATCH v1 1/1] IntelSiliconPkg: Add BaseSmmAccessLibNull
>>
>> Ray,
>>
>> Do you have plans to do something here? Whether take this patch or
>> refactor SmmAccessLib to a PEIM?
>>
>> Thanks,
>> Michael
>>
>> On 8/20/2021 3:34 PM, Michael Kubacki wrote:
>>> 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.
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
> 
> 
> 
> 
>