[PATCH v2] OvmfPkg: Add QemuFwCfgLibNull

[PATCH v2] OvmfPkg: Add QemuFwCfgLibNull

[Rebecca Cran]

Add a null implementation library for QemuFwCfgLib, in order to
support building PciHostBridgeLib for bhyve.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
 .../Library/QemuFwCfgLib/QemuFwCfgLibNull.inf |  37 ++++
 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c  | 209 ++++++++++++++++++
 2 files changed, 246 insertions(+)
 create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibNull.inf
 create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c

diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibNull.inf b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibNull.inf
new file mode 100644
index 0000000000..09f86c2b02
--- /dev/null
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibNull.inf
@@ -0,0 +1,37 @@
+## @file
+#
+#  Stateful, implicitly initialized fw_cfg library.
+#
+#  Copyright (C) 2013, Red Hat, Inc.
+#  Copyright (c) 2008 - 2012, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = QemuFwCfgLibNull
+  FILE_GUID                      = B9D1A1F2-01E2-4732-982D-C7F9ED51AC6B
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = QemuFwCfgLib
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64
+#
+
+[Sources]
+  QemuFwCfgLibInternal.h
+  QemuFwCfgNull.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+  DebugLib
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c
new file mode 100644
index 0000000000..fc1d940721
--- /dev/null
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c
@@ -0,0 +1,209 @@
+/** @file
+
+  Stateful and implicitly initialized fw_cfg library implementation.
+
+  Copyright (C) 2013, Red Hat, Inc.
+  Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017, Advanced Micro Devices. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Uefi.h>
+#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
+  )
+{
+  return FALSE;
+}
+
+
+/**
+  Selects a firmware configuration item for reading.
+
+  Following this call, any data read from this item will start from
+  the beginning of the configuration item's data.
+
+  @param[in] QemuFwCfgItem - Firmware Configuration item to read
+
+**/
+VOID
+EFIAPI
+QemuFwCfgSelectItem (
+  IN FIRMWARE_CONFIG_ITEM   QemuFwCfgItem
+  )
+{
+  ASSERT (FALSE);
+}
+
+
+/**
+  Reads firmware configuration bytes into a buffer
+
+  If called multiple times, then the data read will
+  continue at the offset of the firmware configuration
+  item where the previous read ended.
+
+  @param[in] Size - Size in bytes to read
+  @param[in] Buffer - Buffer to store data into
+
+**/
+VOID
+EFIAPI
+QemuFwCfgReadBytes (
+  IN UINTN                  Size,
+  IN VOID                   *Buffer  OPTIONAL
+  )
+{
+  ASSERT (FALSE);
+}
+
+
+/**
+  Writes firmware configuration bytes from a buffer
+
+  If called multiple times, then the data written will
+  continue at the offset of the firmware configuration
+  item where the previous write ended.
+
+  @param[in] Size - Size in bytes to write
+  @param[in] Buffer - Buffer to read data from
+
+**/
+VOID
+EFIAPI
+QemuFwCfgWriteBytes (
+  IN UINTN                  Size,
+  IN VOID                   *Buffer
+  )
+{
+  ASSERT (FALSE);
+}
+
+
+/**
+  Skip bytes in the firmware configuration item.
+
+  Increase the offset of the firmware configuration item without transferring
+  bytes between the item and a caller-provided buffer. Subsequent read, write
+  or skip operations will commence at the increased offset.
+
+  @param[in] Size  Number of bytes to skip.
+**/
+VOID
+EFIAPI
+QemuFwCfgSkipBytes (
+  IN UINTN                  Size
+  )
+{
+  ASSERT (FALSE);
+}
+
+
+/**
+  Reads a UINT8 firmware configuration value
+
+  @return    Value of Firmware Configuration item read
+
+**/
+UINT8
+EFIAPI
+QemuFwCfgRead8 (
+  VOID
+  )
+{
+  ASSERT (FALSE);
+  return 0;
+}
+
+
+/**
+  Reads a UINT16 firmware configuration value
+
+  @return    Value of Firmware Configuration item read
+
+**/
+UINT16
+EFIAPI
+QemuFwCfgRead16 (
+  VOID
+  )
+{
+  ASSERT (FALSE);
+  return 0;
+}
+
+
+/**
+  Reads a UINT32 firmware configuration value
+
+  @return    Value of Firmware Configuration item read
+
+**/
+UINT32
+EFIAPI
+QemuFwCfgRead32 (
+  VOID
+  )
+{
+  ASSERT (FALSE);
+  return 0;
+}
+
+
+/**
+  Reads a UINT64 firmware configuration value
+
+  @return    Value of Firmware Configuration item read
+
+**/
+UINT64
+EFIAPI
+QemuFwCfgRead64 (
+  VOID
+  )
+{
+  ASSERT (FALSE);
+  return 0;
+}
+
+
+/**
+  Find the configuration item corresponding to the firmware configuration file.
+
+  @param[in]  Name - Name of file to look up.
+  @param[out] Item - Configuration item corresponding to the file, to be passed
+                     to QemuFwCfgSelectItem ().
+  @param[out] Size - Number of bytes in the file.
+
+  @return    RETURN_SUCCESS       If file is found.
+             RETURN_NOT_FOUND     If file is not found.
+             RETURN_UNSUPPORTED   If firmware configuration is unavailable.
+
+**/
+RETURN_STATUS
+EFIAPI
+QemuFwCfgFindFile (
+  IN   CONST CHAR8           *Name,
+  OUT  FIRMWARE_CONFIG_ITEM  *Item,
+  OUT  UINTN                 *Size
+  )
+{
+  return RETURN_UNSUPPORTED;
+}
+
-- 
2.25.1

Re: [edk2-devel] [PATCH v2] OvmfPkg: Add QemuFwCfgLibNull

[Laszlo Ersek]

On 04/30/20 15:22, Rebecca Cran wrote:
> Add a null implementation library for QemuFwCfgLib, in order to
> support building PciHostBridgeLib for bhyve.
>
> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>  .../Library/QemuFwCfgLib/QemuFwCfgLibNull.inf |  37 ++++
>  OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c  | 209 ++++++++++++++++++
>  2 files changed, 246 insertions(+)
>  create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibNull.inf
>  create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c

(a) For the record, the "bhyve v3" instance of this patch [0] and the
"bhyve v4" instance of this patch [1] are identical.

(b) This stand-alone patch differs from both the "bhyve v3" instance [0]
and the (identical) "bhyve v4" instance [1] as follows:

> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c
> index af29471c1f27..2a8f1316ba6c 100644
> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c
> @@ -11,7 +11,7 @@
>
>  #include <Uefi.h>
>  #include <Library/DebugLib.h>
> -#include <IndustryStandard/QemuFwCfg.h>
> +#include <Library/QemuFwCfgLib.h>
>
>  /**
>    Returns a boolean indicating if the firmware configuration interface

which is what Phil asked for in [2]. So that's good.

(c) However, you didn't address my "bhyve v3" comments at all, which I
posted at [3]. (The #include directive that you updated at Phil's
request only happens to solve part of my point (8).)

Please pay more attention to review feedback. If you post a new version
without even attempting to address review comments, that's a waste of
time for both you and your reviewers.

(d) Please get your git setup in functional order *consistently*. This
patch only adds new files (doesn't modify existing files), so there is
no conflict due to <CR><CR><LF> on context lines or removed lines. But
both new files have <CR><CR><LF> after the patch is applied with git-am.

Thanks
Laszlo

[0] http://mid.mail-archive.com/20200421030955.114850-3-rebecca@bsdio.com
    https://edk2.groups.io/g/devel/message/57692

[1] http://mid.mail-archive.com/20200424182914.138915-3-rebecca@bsdio.com
    https://edk2.groups.io/g/devel/message/58098

[2] http://mid.mail-archive.com/5934622c-7235-d073-cdf4-957d6b684c67@redhat.com
    https://edk2.groups.io/g/devel/message/58230

[3] http://mid.mail-archive.com/b3bd046e-1762-243f-e1c6-01660ad6a8d4@redhat.com
    https://edk2.groups.io/g/devel/message/57913

Re: [edk2-devel] [PATCH v2] OvmfPkg: Add QemuFwCfgLibNull

[Rebecca Cran]

On 4/30/20 10:46 AM, Laszlo Ersek wrote:
>
> Please pay more attention to review feedback. If you post a new version
> without even attempting to address review comments, that's a waste of
> time for both you and your reviewers.

It's something I really do need to work on. I need to take a step back 
and read up on effective ways of working with an email-based review 
system, because at the moment I'm a bit overwhelmed with sorting through 
the feedback, applying it (or not) etc. It's a new way of working, both 
over email and with multi-patch series.


> (d) Please get your git setup in functional order *consistently*. This
> patch only adds new files (doesn't modify existing files), so there is
> no conflict due to <CR><CR><LF> on context lines or removed lines. But
> both new files have <CR><CR><LF> after the patch is applied with git-am.

I thought the SetupGit.py script should have done that, but reading 
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers 
I see there's more I should configure.


-- 
Rebecca Cran