public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael Kubacki" <mikuback@linux.microsoft.com>
To: devel@edk2.groups.io, ray.ni@intel.com
Cc: "Chaganty, Rangasai V" <rangasai.v.chaganty@intel.com>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH v1 1/1] IntelSiliconPkg: Add BaseSmmAccessLibNull
Date: Tue, 10 Aug 2021 11:28:47 -0400	[thread overview]
Message-ID: <ad025e3e-d0bd-15fc-2e44-796843f900be@linux.microsoft.com> (raw)
In-Reply-To: <CO1PR11MB49301C906035D84DAA77B6268CF79@CO1PR11MB4930.namprd11.prod.outlook.com>

There is not a SmmAccess PEIM in IntelSiliconPkg.

There is a SmmAccessDxe driver:
https://github.com/tianocore/edk2-platforms/blob/master/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess.inf

And there is a PeiSmmAccessLib:
https://github.com/tianocore/edk2-platforms/tree/master/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess

As long as this is a library class, the NULL library instance is 
necessary for the same reason many NULL instances are necessary.

Scenario #1:

If a user has a library that depends on SmmAccessLib and that library is 
linked to two different modules, the user may wish to actually call the 
install function from one module and not link the functionality (or 
inherit the non-NULL SmmAccessLib dependencies) in the other module.

Scenario #2:

Someone is trying to build common code. That common code has a PEIM. 
That PEIM links SmmAccessLib. A downstream package leveraging that PEIM 
does not use the functionality in the PEIM that invokes SmmAccessLib. 
Therefore, the package would like to reduce the overall PEIM 
dependencies by linking a NULL library instance.

---

This is the result of an invoke-once interface being exposed as a 
generic library class.

---

This patch is not trying to modify interfaces such as converting the 
library class to a driver or anything more involved/impactful. It is 
simply making the design already in place have an alternative, optional 
instance available for platform integration.

Thanks
Michael

On 8/9/2021 10:30 PM, Ni, Ray wrote:
> Michael,
> If your platform doesn't need SmmAccessPPI, you don't put the SmmAccess PEIM in the FDF.
> Why do you need:
> 1. Put SmmAccess PEIM in FDF
> 2. Let SmmAccess PEIM link to a NULL dummy-do-nothing library
> 
> I feel the additional abstraction is not necessary.
> 
> Thanks,
> Ray
> 
>> -----Original Message-----
>> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
>> Sent: Monday, August 9, 2021 10:16 PM
>> To: devel@edk2.groups.io
>> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>
>> Subject: [edk2-platforms][PATCH v1 1/1] IntelSiliconPkg: Add BaseSmmAccessLibNull
>>
>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>
>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3540
>>
>> Adds a NULL instance of SmmAccessLib.
>>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>> ---
>>   Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAccessLibNull/BaseSmmAccessLibNull.c   | 33
>> ++++++++++++++++++++
>>   Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAccessLibNull/BaseSmmAccessLibNull.inf | 26
>> +++++++++++++++
>>   Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc                                                     |  1 +
>>   3 files changed, 60 insertions(+)
>>
>> diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAccessLibNull/BaseSmmAccessLibNull.c
>> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAccessLibNull/BaseSmmAccessLibNull.c
>> new file mode 100644
>> index 000000000000..f5ad306b380b
>> --- /dev/null
>> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAccessLibNull/BaseSmmAccessLibNull.c
>> @@ -0,0 +1,33 @@
>> +/** @file
>> +  A NULL library instance of SmmAccessLib.
>> +
>> +  Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>
>> +  Copyright (c) Microsoft Corporation.<BR>
>> +
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#include <Uefi.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/SmmAccessLib.h>
>> +
>> +/**
>> +  This function is to install an SMM Access PPI
>> +
>> +  @retval EFI_SUCCESS           - Ppi successfully started and installed.
>> +  @retval EFI_NOT_FOUND         - Ppi can't be found.
>> +  @retval EFI_OUT_OF_RESOURCES  - Ppi does not have enough resources to initialize the driver.
>> +  @retval EFI_UNSUPPORTED       - The PPI was not installed and installation is unsupported in
>> +                                  this instance of function implementation.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +PeiInstallSmmAccessPpi (
>> +  VOID
>> +  )
>> +{
>> +  ASSERT (FALSE);
>> +  return EFI_UNSUPPORTED;
>> +}
>> diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAccessLibNull/BaseSmmAccessLibNull.inf
>> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAccessLibNull/BaseSmmAccessLibNull.inf
>> new file mode 100644
>> index 000000000000..7fd3b0b89655
>> --- /dev/null
>> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAccessLibNull/BaseSmmAccessLibNull.inf
>> @@ -0,0 +1,26 @@
>> +## @file
>> +# A NULL library instance of SmmAccessLib.
>> +#
>> +# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
>> +# Copyright (c) Microsoft Corporation.<BR>
>> +# SPDX-License-Identifier: BSD-2-Clause-Patent
>> +#
>> +##
>> +
>> +[Defines]
>> +INF_VERSION     = 0x00010017
>> +BASE_NAME       = BaseSmmAccessLibNull
>> +FILE_GUID       = C1A14AB6-B757-4046-9B92-9DCE1A2154C6
>> +VERSION_STRING  = 1.0
>> +MODULE_TYPE     = BASE
>> +LIBRARY_CLASS   = SmmAccessLib
>> +
>> +[Packages]
>> +  MdePkg/MdePkg.dec
>> +  IntelSiliconPkg/IntelSiliconPkg.dec
>> +
>> +[LibraryClasses]
>> +  DebugLib
>> +
>> +[Sources]
>> +  BaseSmmAccessLibNull.c
>> diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
>> index 1092371d848e..dd0928ec58f3 100644
>> --- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
>> +++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
>> @@ -90,6 +90,7 @@ [Components]
>>     IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdateDxe.inf
>>     IntelSiliconPkg/Feature/Capsule/Library/MicrocodeFlashAccessLibNull/MicrocodeFlashAccessLibNull.inf
>>     IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePei.inf
>> +  IntelSiliconPkg/Feature/SmmAccess/Library/BaseSmmAccessLibNull/BaseSmmAccessLibNull.inf
>>     IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiFirmwareBootMediaLib.inf
>>     IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmmFirmwareBootMediaLib.inf
>>     IntelSiliconPkg/Library/DxeAslUpdateLib/DxeAslUpdateLib.inf
>> --
>> 2.28.0.windows.1
> 
> 
> 
> 
> 

  reply	other threads:[~2021-08-10 15:28 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-09 14:15 [edk2-platforms][PATCH v1 1/1] IntelSiliconPkg: Add BaseSmmAccessLibNull Michael Kubacki
2021-08-09 22:42 ` Chaganty, Rangasai V
2021-08-10  2:30 ` Ni, Ray
2021-08-10 15:28   ` Michael Kubacki [this message]
2021-08-12  5:32     ` [edk2-devel] " Ni, Ray
2021-08-13  2:16       ` Michael Kubacki
2021-08-18 18:45         ` Michael Kubacki
2021-08-19  9:49         ` Ni, Ray
2021-08-19 16:53           ` Michael Kubacki
2021-08-20  5:33             ` Ni, Ray
2021-08-20 19:34               ` Michael Kubacki
2021-09-09 14:12                 ` Michael Kubacki
2021-09-09 14:49                   ` Ni, Ray
2021-09-09 14:54                     ` Michael Kubacki
2021-09-09 14:58                       ` Ni, Ray
2021-09-09 15:23                         ` Michael Kubacki

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=ad025e3e-d0bd-15fc-2e44-796843f900be@linux.microsoft.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