I think it would be nice to have both. That was just an example of where
SmmAccessLib could be linked that would cause two instances of the
library API to be invoked by common code that is linked to two different
modules.
Some platforms might not use BoardInitLib that could benefit from the
PEIM in IntelSiliconPkg.
On 9/9/2021 10:58 AM, Ni, Ray wrote:
I think refactoring it to a PEIM is better.
But I am not sure if having below Bugzilla completed can meet your needs
without refactoring the lib to PEIM.
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
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
Sent: Thursday, September 9, 2021 10:54 PM
To: devel@edk2.groups.io; Ni, Ray <ray...@intel.com>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH v1 1/1] IntelSiliconPkg: Add
BaseSmmAccessLibNull
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 <mikub...@linux.microsoft.com>
Sent: Thursday, September 9, 2021 10:12 PM
To: Ni, Ray <ray...@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...@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 <mikub...@linux.microsoft.com>
*Sent:* Friday, August 13, 2021 10:16 AM
*To:* Ni; Ni, Ray <ray...@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.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80445): https://edk2.groups.io/g/devel/message/80445
Mute This Topic: https://groups.io/mt/84769134/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
