On Tue, Dec 24, 2013 at 8:50 AM, Laszlo Ersek <[email protected]> wrote:
> The QemuFwCfgSecLib library instance
> - is stateless,
> - has no library constructor,
> - is available to SEC client code,
> - must be queried with QemuFwCfgIsAvailable() before use,
> - is restricted to SEC in order to limit the explicit querying
>   requirement. (There is no current user.)
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <[email protected]>
> ---
>  OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf | 54 +++++++++++++++++
>  OvmfPkg/Library/QemuFwCfgLib/StatelessImpl.c     | 76 
> ++++++++++++++++++++++++
>  OvmfPkg/OvmfPkgIa32.dsc                          |  1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc                       |  1 +
>  OvmfPkg/OvmfPkgX64.dsc                           |  1 +
>  5 files changed, 133 insertions(+)
>  create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
>  create mode 100644 OvmfPkg/Library/QemuFwCfgLib/StatelessImpl.c
>
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf 
> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> new file mode 100644
> index 0000000..a9eba83
> --- /dev/null
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> @@ -0,0 +1,54 @@
> +## @file
> +#
> +#  Stateless fw_cfg library that must be queried before use.
> +#
> +#  Copyright (C) 2013, Red Hat, Inc.
> +#  Copyright (c) 2008 - 2012, Intel Corporation. All rights reserved.<BR>
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD 
> License
> +#  which accompanies this distribution. The full text of the license may be 
> found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = QemuFwCfgSecLib
> +  FILE_GUID                      = 60a910e5-7443-413d-9a30-97e57497cd1b
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = QemuFwCfgLib|SEC
> +
> +#
> +# The following information is for reference only and not required by the 
> build tools.
> +#
> +#  VALID_ARCHITECTURES           = IA32 X64
> +#
> +
> +[Sources]
> +  QemuFwCfgLib.c
> +  StatelessImpl.c

Can you call this QemuFwCfgLibSec.c instead, and use
QemuFwCfgLibPeiDxe.c in the previous patch? I don't mind your names,
but my suggestions would better follow the convention used in EDK II.

> +
> +[Sources.IA32]
> +  Ia32/IoLibExAsm.asm
> +  Ia32/IoLibExAsm.S
> +
> +[Sources.X64]
> +  X64/IoLibExAsm.asm
> +  X64/IoLibExAsm.S
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
> +  DebugLib
> +  IoLib
> +  MemoryAllocationLib
> +
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/StatelessImpl.c 
> b/OvmfPkg/Library/QemuFwCfgLib/StatelessImpl.c
> new file mode 100644
> index 0000000..6e33ee2
> --- /dev/null
> +++ b/OvmfPkg/Library/QemuFwCfgLib/StatelessImpl.c
> @@ -0,0 +1,76 @@
> +/** @file
> +
> +  Stateless fw_cfg library implementation.
> +
> +  Clients must call QemuFwCfgIsAvailable() first.
> +
> +  Copyright (C) 2013, Red Hat, Inc.
> +  Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, 
> WITHOUT
> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +**/
> +
> +#include <Library/DebugLib.h>
> +#include <Library/QemuFwCfgLib.h>
> +
> +
> +/**
> +  Returns a boolean indicating if the firmware configuration interface
> +  is available or not.
> +
> +  This function may change fw_cfg state.
> +
> +  @retval    TRUE   The interface is available
> +  @retval    FALSE  The interface is not available
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +QemuFwCfgIsAvailable (
> +  VOID
> +  )
> +{
> +  UINT32 Signature;
> +  UINT32 Revision;
> +
> +  QemuFwCfgSelectItem (QemuFwCfgItemSignature);
> +  Signature = QemuFwCfgRead32 ();
> +  DEBUG ((EFI_D_INFO, "FW CFG Signature: 0x%x\n", Signature));
> +  QemuFwCfgSelectItem (QemuFwCfgItemInterfaceVersion);
> +  Revision = QemuFwCfgRead32 ();
> +  DEBUG ((EFI_D_INFO, "FW CFG Revision: 0x%x\n", Revision));
> +  if ((Signature != SIGNATURE_32 ('Q', 'E', 'M', 'U')) ||
> +      (Revision < 1)
> +     ) {
> +    DEBUG ((EFI_D_INFO, "QemuFwCfg interface not supported.\n"));
> +    return FALSE;
> +  }
> +
> +  DEBUG ((EFI_D_INFO, "QemuFwCfg interface is supported.\n"));
> +  return TRUE;
> +}
> +
> +
> +/**
> +  Returns a boolean indicating if the firmware configuration interface is
> +  available for library-internal purposes.
> +
> +  This function never changes fw_cfg state.
> +
> +  @retval    TRUE   The interface is available internally.
> +  @retval    FALSE  The interface is not available internally.
> +**/
> +BOOLEAN
> +EFIAPI
> +InternalQemuFwCfgIsAvailable (
> +  VOID
> +  )
> +{
> +  return TRUE;

How about adding a comment:
"We always return TRUE, because the consumer of this library ought to
have called QemuFwCfgIsAvailable before making other calls which would
hit this path."

Or, just have this call QemuFwCfgIsAvailable and re-detect each time.

With the recommended 2 changes, patch 26-28 are
Reviewed-by: Jordan Justen <[email protected]>

> +}
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index fc60c3c..86431bc 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -130,6 +130,7 @@
>  !endif
>
>  [LibraryClasses.common.SEC]
> +  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
>  !ifdef $(DEBUG_ON_SERIAL_PORT)
>    DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>  !else
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 3b01638..4e87a76 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -135,6 +135,7 @@
>  !endif
>
>  [LibraryClasses.common.SEC]
> +  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
>  !ifdef $(DEBUG_ON_SERIAL_PORT)
>    DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>  !else
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 283fcf5..519186f 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -135,6 +135,7 @@
>  !endif
>
>  [LibraryClasses.common.SEC]
> +  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
>  !ifdef $(DEBUG_ON_SERIAL_PORT)
>    DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>  !else
> --
> 1.8.3.1
>
>
>
> ------------------------------------------------------------------------------
> Rapidly troubleshoot problems before they affect your business. Most IT
> organizations don't have a clear picture of how application performance
> affects their revenue. With AppDynamics, you get 100% visibility into your
> Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
> http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to