* [PATCH 1/7] OvmfPkg: introduce QemuFwCfgSimpleParserLib
2020-04-24 7:53 [PATCH 0/7] OvmfPkg, ArmVirtPkg: control PXE v4/v6 boot support from the QEMU cmdline Laszlo Ersek
@ 2020-04-24 7:53 ` Laszlo Ersek
2020-04-24 9:13 ` Philippe Mathieu-Daudé
2020-04-24 7:53 ` [PATCH 2/7] OvmfPkg/PlatformPei: parse "X-PciMmio64Mb" with QemuFwCfgSimpleParserLib Laszlo Ersek
` (7 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Laszlo Ersek @ 2020-04-24 7:53 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Jordan Justen, Per Sundstrom,
Philippe Mathieu-Daudé
We already parse some boolean and integer values from named fw_cfg files
(usually into PCDs), and we're going to cover more. Add a dedicated
library for centralizing the parsing logic.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Per Sundstrom <per_sundstrom@yahoo.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/OvmfPkg.dec | 4 +
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h | 128 +++++++
OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf | 27 ++
OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParser.c | 398 ++++++++++++++++++++
7 files changed, 560 insertions(+)
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 28030391cff2..8a46fe73344e 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -60,6 +60,10 @@ [LibraryClasses]
#
QemuFwCfgS3Lib|Include/Library/QemuFwCfgS3Lib.h
+ ## @libraryclass Parse the contents of named fw_cfg files as simple
+ # (scalar) data types.
+ QemuFwCfgSimpleParserLib|Include/Library/QemuFwCfgSimpleParserLib.h
+
## @libraryclass Rewrite the BootOrder NvVar based on QEMU's "bootorder"
# fw_cfg file.
#
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index d5e90c001370..5e2972063110 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -160,6 +160,7 @@ [LibraryClasses]
UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
SerializeVariablesLib|OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf
QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
+ QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 066f49aeaee0..18e6909a33fa 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -164,6 +164,7 @@ [LibraryClasses]
UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
SerializeVariablesLib|OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf
QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
+ QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index ac510522a9ff..3d24cc4c1cfb 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -164,6 +164,7 @@ [LibraryClasses]
UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
SerializeVariablesLib|OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf
QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
+ QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
diff --git a/OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h b/OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h
new file mode 100644
index 000000000000..c6062bae8770
--- /dev/null
+++ b/OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h
@@ -0,0 +1,128 @@
+/** @file
+ Parse the contents of named fw_cfg files as simple (scalar) data types.
+
+ Copyright (C) 2020, Red Hat, Inc.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef QEMU_FW_CFG_SIMPLE_PARSER_LIB_H_
+#define QEMU_FW_CFG_SIMPLE_PARSER_LIB_H_
+
+#include <Base.h>
+
+/**
+ Look up FileName with QemuFwCfgFindFile() from QemuFwCfgLib. Read the fw_cfg
+ file into a small array with automatic storage duration. Parse the array as
+ the textual representation of a BOOLEAN.
+
+ @param[in] FileName The name of the fw_cfg file to look up and parse.
+
+ @param[out] Value On success, Value is TRUE if the contents of the fw_cfg
+ file case-insensitively match "true", "yes", "y",
+ "enable", "enabled", "1".
+
+ On success, Value is FALSE if the contents of the fw_cfg
+ file case-insensitively match "false", "no", "n",
+ "disable", "disabled", "0".
+
+ On failure, Value is not changed.
+
+ @retval RETURN_SUCCESS Parsing successful. Value has been set.
+
+ @retval RETURN_UNSUPPORTED Firmware configuration is unavailable.
+
+ @retval RETURN_PROTOCOL_ERROR Parsing failed. Value has not been changed.
+
+ @return Error codes propagated from
+ QemuFwCfgFindFile(). Value has not been
+ changed.
+**/
+RETURN_STATUS
+EFIAPI
+QemuFwCfgParseBool (
+ IN CONST CHAR8 *FileName,
+ OUT BOOLEAN *Value
+ );
+
+/**
+ Look up FileName with QemuFwCfgFindFile() from QemuFwCfgLib. Read the fw_cfg
+ file into a small array with automatic storage duration. Parse the array as
+ the textual representation of a UINT8.
+
+ @param[in] FileName The name of the fw_cfg file to look up and parse.
+
+ @param[in] ParseAsHex If TRUE, call BaseLib's AsciiStrHexToUint64S() for
+ parsing the fw_cfg file.
+
+ If FALSE, call BaseLib's AsciiStrDecimalToUint64S()
+ for parsing the fw_cfg file.
+
+ @param[out] Value On success, Value has been parsed with the BaseLib
+ function determined by ParseAsHex, and also
+ range-checked for [0, MAX_UINT8].
+
+ On failure, Value is not changed.
+
+ @retval RETURN_SUCCESS Parsing successful. Value has been set.
+
+ @retval RETURN_UNSUPPORTED Firmware configuration is unavailable.
+
+ @retval RETURN_PROTOCOL_ERROR Parsing failed. Value has not been changed.
+
+ @retval RETURN_PROTOCOL_ERROR Parsing succeeded, but the result does not fit
+ in the [0, MAX_UINT8] range. Value has not
+ been changed.
+
+ @return Error codes propagated from
+ QemuFwCfgFindFile() and from the BaseLib
+ function selected by ParseAsHex. Value has not
+ been changed.
+**/
+RETURN_STATUS
+EFIAPI
+QemuFwCfgParseUint8 (
+ IN CONST CHAR8 *FileName,
+ IN BOOLEAN ParseAsHex,
+ OUT UINT8 *Value
+ );
+
+//
+// The following functions behave identically to QemuFwCfgParseUint8(),
+// only their range checks use MAX_UINT16, MAX_UINT32, MAX_UINT64, MAX_UINTN,
+// respectively.
+//
+
+RETURN_STATUS
+EFIAPI
+QemuFwCfgParseUint16 (
+ IN CONST CHAR8 *FileName,
+ IN BOOLEAN ParseAsHex,
+ OUT UINT16 *Value
+ );
+
+RETURN_STATUS
+EFIAPI
+QemuFwCfgParseUint32 (
+ IN CONST CHAR8 *FileName,
+ IN BOOLEAN ParseAsHex,
+ OUT UINT32 *Value
+ );
+
+RETURN_STATUS
+EFIAPI
+QemuFwCfgParseUint64 (
+ IN CONST CHAR8 *FileName,
+ IN BOOLEAN ParseAsHex,
+ OUT UINT64 *Value
+ );
+
+RETURN_STATUS
+EFIAPI
+QemuFwCfgParseUintn (
+ IN CONST CHAR8 *FileName,
+ IN BOOLEAN ParseAsHex,
+ OUT UINTN *Value
+ );
+
+#endif // QEMU_FW_CFG_SIMPLE_PARSER_LIB_H_
diff --git a/OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf b/OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
new file mode 100644
index 000000000000..c0e6ab3a1303
--- /dev/null
+++ b/OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
@@ -0,0 +1,27 @@
+## @file
+# Parse the contents of named fw_cfg files as simple (scalar) data types.
+#
+# Copyright (C) 2020, Red Hat, Inc.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+ INF_VERSION = 1.29
+ BASE_NAME = QemuFwCfgSimpleParserLib
+ FILE_GUID = a9a1211d-061e-4b64-af30-5dd0cac9dc99
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = QemuFwCfgSimpleParserLib
+ CONSTRUCTOR = QemuFwCfgSimpleParserInit
+
+[Sources]
+ QemuFwCfgSimpleParser.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ QemuFwCfgLib
diff --git a/OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParser.c b/OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParser.c
new file mode 100644
index 000000000000..5ca22fcf0dee
--- /dev/null
+++ b/OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParser.c
@@ -0,0 +1,398 @@
+/** @file
+ Parse the contents of named fw_cfg files as simple (scalar) data types.
+
+ Copyright (C) 2020, Red Hat, Inc.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/QemuFwCfgLib.h>
+#include <Library/QemuFwCfgSimpleParserLib.h>
+
+//
+// Size of the longest valid UINT64 string, including the terminating NUL.
+//
+#define UINT64_STRING_MAX_SIZE \
+ MAX (sizeof "18446744073709551615", sizeof "0xFFFFFFFFFFFFFFFF")
+
+//
+// Size of the longest valid BOOL string (see the "mTrueString" and
+// "mFalseString" arrays below), including the terminating NUL.
+//
+#define BOOL_STRING_MAX_SIZE (sizeof "disabled")
+
+//
+// Length of "\r\n", not including the terminating NUL.
+//
+#define CRLF_LENGTH (sizeof "\r\n" - 1)
+
+//
+// Words recognized as representing TRUE or FALSE.
+//
+STATIC CONST CHAR8 * CONST mTrueString[] = {
+ "true", "yes", "y", "enable", "enabled", "1"
+};
+STATIC CONST CHAR8 * CONST mFalseString[] = {
+ "false", "no", "n", "disable", "disabled", "0"
+};
+
+//
+// Helper functions.
+//
+
+/**
+ Look up FileName with QemuFwCfgFindFile() from QemuFwCfgLib. Read the fw_cfg
+ file into the caller-provided CHAR8 array. NUL-terminate the array.
+
+ @param[in] FileName The name of the fw_cfg file to look up and read.
+
+ @param[in,out] BufferSize On input, number of bytes available in Buffer.
+
+ On output, the number of bytes that have been
+ stored to Buffer.
+
+ On error, BufferSize is indeterminate.
+
+ @param[out] Buffer The buffer to read the fw_cfg file into. If the
+ fw_cfg file contents are not NUL-terminated, then
+ a NUL character is placed into Buffer after the
+ fw_cfg file contents.
+
+ On error, Buffer is indeterminate.
+
+ @retval RETURN_SUCCESS Buffer has been populated with the fw_cfg file
+ contents. Buffer is NUL-terminated regardless
+ of whether the fw_cfg file itself was
+ NUL-terminated.
+
+ @retval RETURN_UNSUPPORTED Firmware configuration is unavailable.
+
+ @retval RETURN_PROTOCOL_ERROR The fw_cfg file does not fit into Buffer.
+ (This is considered a QEMU configuration
+ error; BufferSize is considered authoritative
+ for the contents of the fw_cfg file identified
+ by FileName.)
+
+ @retval RETURN_PROTOCOL_ERROR The fw_cfg file contents are not themselves
+ NUL-terminated, and an extra NUL byte does not
+ fit into Buffer. (Again a QEMU configuration
+ error.)
+
+ @return Error codes propagated from
+ QemuFwCfgFindFile().
+**/
+STATIC
+RETURN_STATUS
+QemuFwCfgGetAsString (
+ IN CONST CHAR8 *FileName,
+ IN OUT UINTN *BufferSize,
+ OUT CHAR8 *Buffer
+ )
+{
+ RETURN_STATUS Status;
+ FIRMWARE_CONFIG_ITEM FwCfgItem;
+ UINTN FwCfgSize;
+
+ if (!QemuFwCfgIsAvailable ()) {
+ return RETURN_UNSUPPORTED;
+ }
+
+ Status = QemuFwCfgFindFile (FileName, &FwCfgItem, &FwCfgSize);
+ if (RETURN_ERROR (Status)) {
+ return Status;
+ }
+ if (FwCfgSize > *BufferSize) {
+ return RETURN_PROTOCOL_ERROR;
+ }
+
+ QemuFwCfgSelectItem (FwCfgItem);
+ QemuFwCfgReadBytes (FwCfgSize, Buffer);
+
+ //
+ // If Buffer is already NUL-terminated due to fw_cfg contents, we're done.
+ //
+ if (FwCfgSize > 0 && Buffer[FwCfgSize - 1] == '\0') {
+ *BufferSize = FwCfgSize;
+ return RETURN_SUCCESS;
+ }
+ //
+ // Otherwise, append a NUL byte to Buffer (if we have room for it).
+ //
+ if (FwCfgSize == *BufferSize) {
+ return RETURN_PROTOCOL_ERROR;
+ }
+ Buffer[FwCfgSize] = '\0';
+ *BufferSize = FwCfgSize + 1;
+ return RETURN_SUCCESS;
+}
+
+/**
+ Remove a trailing \r\n or \n sequence from a string.
+
+ @param[in,out] BufferSize On input, the number of bytes in Buffer, including
+ the terminating NUL.
+
+ On output, the adjusted string size (including the
+ terminating NUL), after stripping the \r\n or \n
+ suffix.
+
+ @param[in,out] Buffer The NUL-terminated string to trim.
+**/
+STATIC
+VOID
+StripNewline (
+ IN OUT UINTN *BufferSize,
+ IN OUT CHAR8 *Buffer
+ )
+{
+ UINTN InSize, OutSize;
+
+ InSize = *BufferSize;
+ OutSize = InSize;
+
+ if (InSize >= 3 &&
+ Buffer[InSize - 3] == '\r' && Buffer[InSize - 2] == '\n') {
+ OutSize = InSize - 2;
+ } else if (InSize >= 2 && Buffer[InSize - 2] == '\n') {
+ OutSize = InSize - 1;
+ }
+
+ if (OutSize < InSize) {
+ Buffer[OutSize - 1] = '\0';
+ *BufferSize = OutSize;
+ }
+}
+
+/**
+ Read the fw_cfg file identified by FileName as a string into a small array
+ with automatic storage duration, using QemuFwCfgGetAsString(). Parse the
+ string as a UINT64. Perform a range-check on the parsed value.
+
+ @param[in] FileName The name of the fw_cfg file to look up and parse.
+
+ @param[in] ParseAsHex If TRUE, call BaseLib's AsciiStrHexToUint64S() for
+ parsing the fw_cfg file.
+
+ If FALSE, call BaseLib's AsciiStrDecimalToUint64S()
+ for parsing the fw_cfg file.
+
+ @param[in] Limit The inclusive upper bound on the parsed UINT64 value.
+
+ @param[out] Value On success, Value has been parsed with the BaseLib
+ function determined by ParseAsHex, and has been
+ range-checked against [0, Limit].
+
+ On failure, Value is not changed.
+
+ @retval RETURN_SUCCESS Parsing successful. Value has been set.
+
+ @retval RETURN_UNSUPPORTED Firmware configuration is unavailable.
+
+ @retval RETURN_PROTOCOL_ERROR Parsing failed. Value has not been changed.
+
+ @retval RETURN_PROTOCOL_ERROR Parsing succeeded, but the result does not fit
+ in the [0, Limit] range. Value has not been
+ changed.
+
+ @return Error codes propagated from
+ QemuFwCfgFindFile() and from the BaseLib
+ function selected by ParseAsHex. Value has not
+ been changed.
+**/
+STATIC
+RETURN_STATUS
+QemuFwCfgParseUint64WithLimit (
+ IN CONST CHAR8 *FileName,
+ IN BOOLEAN ParseAsHex,
+ IN UINT64 Limit,
+ OUT UINT64 *Value
+ )
+{
+ UINTN Uint64StringSize;
+ CHAR8 Uint64String[UINT64_STRING_MAX_SIZE + CRLF_LENGTH];
+ RETURN_STATUS Status;
+ CHAR8 *EndPointer;
+ UINT64 Uint64;
+
+ Uint64StringSize = sizeof Uint64String;
+ Status = QemuFwCfgGetAsString (FileName, &Uint64StringSize, Uint64String);
+ if (RETURN_ERROR (Status)) {
+ return Status;
+ }
+
+ StripNewline (&Uint64StringSize, Uint64String);
+
+ if (ParseAsHex) {
+ Status = AsciiStrHexToUint64S (Uint64String, &EndPointer, &Uint64);
+ } else {
+ Status = AsciiStrDecimalToUint64S (Uint64String, &EndPointer, &Uint64);
+ }
+ if (RETURN_ERROR (Status)) {
+ return Status;
+ }
+
+ //
+ // Report a wire protocol error if the subject sequence is empty, or trailing
+ // garbage is present, or Limit is not honored.
+ //
+ if (EndPointer == Uint64String || *EndPointer != '\0' || Uint64 > Limit) {
+ return RETURN_PROTOCOL_ERROR;
+ }
+
+ *Value = Uint64;
+ return RETURN_SUCCESS;
+}
+
+//
+// Public functions.
+//
+
+RETURN_STATUS
+EFIAPI
+QemuFwCfgSimpleParserInit (
+ VOID
+ )
+{
+ //
+ // Do nothing, just participate in constructor dependency ordering.
+ //
+ return RETURN_SUCCESS;
+}
+
+RETURN_STATUS
+EFIAPI
+QemuFwCfgParseBool (
+ IN CONST CHAR8 *FileName,
+ OUT BOOLEAN *Value
+ )
+{
+ UINTN BoolStringSize;
+ CHAR8 BoolString[BOOL_STRING_MAX_SIZE + CRLF_LENGTH];
+ RETURN_STATUS Status;
+ UINTN Idx;
+
+ BoolStringSize = sizeof BoolString;
+ Status = QemuFwCfgGetAsString (FileName, &BoolStringSize, BoolString);
+ if (RETURN_ERROR (Status)) {
+ return Status;
+ }
+
+ StripNewline (&BoolStringSize, BoolString);
+
+ for (Idx = 0; Idx < ARRAY_SIZE (mTrueString); ++Idx) {
+ if (AsciiStriCmp (BoolString, mTrueString[Idx]) == 0) {
+ *Value = TRUE;
+ return RETURN_SUCCESS;
+ }
+ }
+
+ for (Idx = 0; Idx < ARRAY_SIZE (mFalseString); ++Idx) {
+ if (AsciiStriCmp (BoolString, mFalseString[Idx]) == 0) {
+ *Value = FALSE;
+ return RETURN_SUCCESS;
+ }
+ }
+
+ return RETURN_PROTOCOL_ERROR;
+}
+
+RETURN_STATUS
+EFIAPI
+QemuFwCfgParseUint8 (
+ IN CONST CHAR8 *FileName,
+ IN BOOLEAN ParseAsHex,
+ OUT UINT8 *Value
+ )
+{
+ RETURN_STATUS Status;
+ UINT64 Uint64;
+
+ Status = QemuFwCfgParseUint64WithLimit (FileName, ParseAsHex, MAX_UINT8,
+ &Uint64);
+ if (RETURN_ERROR (Status)) {
+ return Status;
+ }
+ *Value = (UINT8)Uint64;
+ return RETURN_SUCCESS;
+}
+
+RETURN_STATUS
+EFIAPI
+QemuFwCfgParseUint16 (
+ IN CONST CHAR8 *FileName,
+ IN BOOLEAN ParseAsHex,
+ OUT UINT16 *Value
+ )
+{
+ RETURN_STATUS Status;
+ UINT64 Uint64;
+
+ Status = QemuFwCfgParseUint64WithLimit (FileName, ParseAsHex, MAX_UINT16,
+ &Uint64);
+ if (RETURN_ERROR (Status)) {
+ return Status;
+ }
+ *Value = (UINT16)Uint64;
+ return RETURN_SUCCESS;
+}
+
+RETURN_STATUS
+EFIAPI
+QemuFwCfgParseUint32 (
+ IN CONST CHAR8 *FileName,
+ IN BOOLEAN ParseAsHex,
+ OUT UINT32 *Value
+ )
+{
+ RETURN_STATUS Status;
+ UINT64 Uint64;
+
+ Status = QemuFwCfgParseUint64WithLimit (FileName, ParseAsHex, MAX_UINT32,
+ &Uint64);
+ if (RETURN_ERROR (Status)) {
+ return Status;
+ }
+ *Value = (UINT32)Uint64;
+ return RETURN_SUCCESS;
+}
+
+RETURN_STATUS
+EFIAPI
+QemuFwCfgParseUint64 (
+ IN CONST CHAR8 *FileName,
+ IN BOOLEAN ParseAsHex,
+ OUT UINT64 *Value
+ )
+{
+ RETURN_STATUS Status;
+ UINT64 Uint64;
+
+ Status = QemuFwCfgParseUint64WithLimit (FileName, ParseAsHex, MAX_UINT64,
+ &Uint64);
+ if (RETURN_ERROR (Status)) {
+ return Status;
+ }
+ *Value = Uint64;
+ return RETURN_SUCCESS;
+}
+
+RETURN_STATUS
+EFIAPI
+QemuFwCfgParseUintn (
+ IN CONST CHAR8 *FileName,
+ IN BOOLEAN ParseAsHex,
+ OUT UINTN *Value
+ )
+{
+ RETURN_STATUS Status;
+ UINT64 Uint64;
+
+ Status = QemuFwCfgParseUint64WithLimit (FileName, ParseAsHex, MAX_UINTN,
+ &Uint64);
+ if (RETURN_ERROR (Status)) {
+ return Status;
+ }
+ *Value = (UINTN)Uint64;
+ return RETURN_SUCCESS;
+}
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 1/7] OvmfPkg: introduce QemuFwCfgSimpleParserLib
2020-04-24 7:53 ` [PATCH 1/7] OvmfPkg: introduce QemuFwCfgSimpleParserLib Laszlo Ersek
@ 2020-04-24 9:13 ` Philippe Mathieu-Daudé
2020-04-28 11:47 ` Laszlo Ersek
0 siblings, 1 reply; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-24 9:13 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: Ard Biesheuvel, Jordan Justen, Per Sundstrom
On 4/24/20 9:53 AM, Laszlo Ersek wrote:
> We already parse some boolean and integer values from named fw_cfg files
> (usually into PCDs), and we're going to cover more. Add a dedicated
> library for centralizing the parsing logic.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Per Sundstrom <per_sundstrom@yahoo.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> OvmfPkg/OvmfPkg.dec | 4 +
> OvmfPkg/OvmfPkgIa32.dsc | 1 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> OvmfPkg/OvmfPkgX64.dsc | 1 +
> OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h | 128 +++++++
> OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf | 27 ++
> OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParser.c | 398 ++++++++++++++++++++
> 7 files changed, 560 insertions(+)
>
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index 28030391cff2..8a46fe73344e 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -60,6 +60,10 @@ [LibraryClasses]
> #
> QemuFwCfgS3Lib|Include/Library/QemuFwCfgS3Lib.h
>
> + ## @libraryclass Parse the contents of named fw_cfg files as simple
> + # (scalar) data types.
> + QemuFwCfgSimpleParserLib|Include/Library/QemuFwCfgSimpleParserLib.h
> +
> ## @libraryclass Rewrite the BootOrder NvVar based on QEMU's "bootorder"
> # fw_cfg file.
> #
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index d5e90c001370..5e2972063110 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -160,6 +160,7 @@ [LibraryClasses]
> UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
> SerializeVariablesLib|OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf
> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
> + QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
> VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
> LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
> MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 066f49aeaee0..18e6909a33fa 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -164,6 +164,7 @@ [LibraryClasses]
> UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
> SerializeVariablesLib|OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf
> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
> + QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
> VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
> LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
> MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index ac510522a9ff..3d24cc4c1cfb 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -164,6 +164,7 @@ [LibraryClasses]
> UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
> SerializeVariablesLib|OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf
> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
> + QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
> VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
> LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
> MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
> diff --git a/OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h b/OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h
> new file mode 100644
> index 000000000000..c6062bae8770
> --- /dev/null
> +++ b/OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h
> @@ -0,0 +1,128 @@
> +/** @file
> + Parse the contents of named fw_cfg files as simple (scalar) data types.
> +
> + Copyright (C) 2020, Red Hat, Inc.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef QEMU_FW_CFG_SIMPLE_PARSER_LIB_H_
> +#define QEMU_FW_CFG_SIMPLE_PARSER_LIB_H_
> +
> +#include <Base.h>
> +
> +/**
> + Look up FileName with QemuFwCfgFindFile() from QemuFwCfgLib. Read the fw_cfg
> + file into a small array with automatic storage duration. Parse the array as
> + the textual representation of a BOOLEAN.
> +
> + @param[in] FileName The name of the fw_cfg file to look up and parse.
> +
> + @param[out] Value On success, Value is TRUE if the contents of the fw_cfg
> + file case-insensitively match "true", "yes", "y",
> + "enable", "enabled", "1".
> +
> + On success, Value is FALSE if the contents of the fw_cfg
> + file case-insensitively match "false", "no", "n",
> + "disable", "disabled", "0".
> +
> + On failure, Value is not changed.
> +
> + @retval RETURN_SUCCESS Parsing successful. Value has been set.
> +
> + @retval RETURN_UNSUPPORTED Firmware configuration is unavailable.
> +
> + @retval RETURN_PROTOCOL_ERROR Parsing failed. Value has not been changed.
> +
> + @return Error codes propagated from
> + QemuFwCfgFindFile(). Value has not been
> + changed.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +QemuFwCfgParseBool (
> + IN CONST CHAR8 *FileName,
> + OUT BOOLEAN *Value
> + );
> +
> +/**
> + Look up FileName with QemuFwCfgFindFile() from QemuFwCfgLib. Read the fw_cfg
> + file into a small array with automatic storage duration. Parse the array as
> + the textual representation of a UINT8.
> +
> + @param[in] FileName The name of the fw_cfg file to look up and parse.
> +
> + @param[in] ParseAsHex If TRUE, call BaseLib's AsciiStrHexToUint64S() for
> + parsing the fw_cfg file.
> +
> + If FALSE, call BaseLib's AsciiStrDecimalToUint64S()
> + for parsing the fw_cfg file.
> +
> + @param[out] Value On success, Value has been parsed with the BaseLib
> + function determined by ParseAsHex, and also
> + range-checked for [0, MAX_UINT8].
> +
> + On failure, Value is not changed.
> +
> + @retval RETURN_SUCCESS Parsing successful. Value has been set.
> +
> + @retval RETURN_UNSUPPORTED Firmware configuration is unavailable.
> +
> + @retval RETURN_PROTOCOL_ERROR Parsing failed. Value has not been changed.
> +
> + @retval RETURN_PROTOCOL_ERROR Parsing succeeded, but the result does not fit
> + in the [0, MAX_UINT8] range. Value has not
> + been changed.
I wondered about RETURN_BAD_BUFFER_SIZE / RETURN_BUFFER_TOO_SMALL for
this case, but RETURN_PROTOCOL_ERROR is less confusing.
> +
> + @return Error codes propagated from
> + QemuFwCfgFindFile() and from the BaseLib
> + function selected by ParseAsHex. Value has not
> + been changed.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +QemuFwCfgParseUint8 (
> + IN CONST CHAR8 *FileName,
> + IN BOOLEAN ParseAsHex,
> + OUT UINT8 *Value
> + );
> +
> +//
> +// The following functions behave identically to QemuFwCfgParseUint8(),
> +// only their range checks use MAX_UINT16, MAX_UINT32, MAX_UINT64, MAX_UINTN,
> +// respectively.
> +//
> +
> +RETURN_STATUS
> +EFIAPI
> +QemuFwCfgParseUint16 (
> + IN CONST CHAR8 *FileName,
> + IN BOOLEAN ParseAsHex,
> + OUT UINT16 *Value
> + );
> +
> +RETURN_STATUS
> +EFIAPI
> +QemuFwCfgParseUint32 (
> + IN CONST CHAR8 *FileName,
> + IN BOOLEAN ParseAsHex,
> + OUT UINT32 *Value
> + );
> +
> +RETURN_STATUS
> +EFIAPI
> +QemuFwCfgParseUint64 (
> + IN CONST CHAR8 *FileName,
> + IN BOOLEAN ParseAsHex,
> + OUT UINT64 *Value
> + );
> +
> +RETURN_STATUS
> +EFIAPI
> +QemuFwCfgParseUintn (
> + IN CONST CHAR8 *FileName,
> + IN BOOLEAN ParseAsHex,
> + OUT UINTN *Value
> + );
> +
> +#endif // QEMU_FW_CFG_SIMPLE_PARSER_LIB_H_
> diff --git a/OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf b/OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
> new file mode 100644
> index 000000000000..c0e6ab3a1303
> --- /dev/null
> +++ b/OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
> @@ -0,0 +1,27 @@
> +## @file
> +# Parse the contents of named fw_cfg files as simple (scalar) data types.
> +#
> +# Copyright (C) 2020, Red Hat, Inc.
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> + INF_VERSION = 1.29
> + BASE_NAME = QemuFwCfgSimpleParserLib
> + FILE_GUID = a9a1211d-061e-4b64-af30-5dd0cac9dc99
> + MODULE_TYPE = BASE
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = QemuFwCfgSimpleParserLib
> + CONSTRUCTOR = QemuFwCfgSimpleParserInit
> +
> +[Sources]
> + QemuFwCfgSimpleParser.c
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + OvmfPkg/OvmfPkg.dec
> +
> +[LibraryClasses]
> + BaseLib
> + QemuFwCfgLib
> diff --git a/OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParser.c b/OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParser.c
> new file mode 100644
> index 000000000000..5ca22fcf0dee
> --- /dev/null
> +++ b/OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParser.c
> @@ -0,0 +1,398 @@
> +/** @file
> + Parse the contents of named fw_cfg files as simple (scalar) data types.
> +
> + Copyright (C) 2020, Red Hat, Inc.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <Library/BaseLib.h>
> +#include <Library/QemuFwCfgLib.h>
> +#include <Library/QemuFwCfgSimpleParserLib.h>
> +
> +//
> +// Size of the longest valid UINT64 string, including the terminating NUL.
> +//
> +#define UINT64_STRING_MAX_SIZE \
> + MAX (sizeof "18446744073709551615", sizeof "0xFFFFFFFFFFFFFFFF")
> +
> +//
> +// Size of the longest valid BOOL string (see the "mTrueString" and
> +// "mFalseString" arrays below), including the terminating NUL.
> +//
> +#define BOOL_STRING_MAX_SIZE (sizeof "disabled")
> +
> +//
> +// Length of "\r\n", not including the terminating NUL.
> +//
> +#define CRLF_LENGTH (sizeof "\r\n" - 1)
> +
> +//
> +// Words recognized as representing TRUE or FALSE.
> +//
> +STATIC CONST CHAR8 * CONST mTrueString[] = {
> + "true", "yes", "y", "enable", "enabled", "1"
> +};
> +STATIC CONST CHAR8 * CONST mFalseString[] = {
> + "false", "no", "n", "disable", "disabled", "0"
> +};
> +
> +//
> +// Helper functions.
> +//
> +
> +/**
> + Look up FileName with QemuFwCfgFindFile() from QemuFwCfgLib. Read the fw_cfg
> + file into the caller-provided CHAR8 array. NUL-terminate the array.
> +
> + @param[in] FileName The name of the fw_cfg file to look up and read.
> +
> + @param[in,out] BufferSize On input, number of bytes available in Buffer.
> +
> + On output, the number of bytes that have been
> + stored to Buffer.
> +
> + On error, BufferSize is indeterminate.
> +
> + @param[out] Buffer The buffer to read the fw_cfg file into. If the
> + fw_cfg file contents are not NUL-terminated, then
> + a NUL character is placed into Buffer after the
> + fw_cfg file contents.
> +
> + On error, Buffer is indeterminate.
> +
> + @retval RETURN_SUCCESS Buffer has been populated with the fw_cfg file
> + contents. Buffer is NUL-terminated regardless
> + of whether the fw_cfg file itself was
> + NUL-terminated.
> +
> + @retval RETURN_UNSUPPORTED Firmware configuration is unavailable.
> +
> + @retval RETURN_PROTOCOL_ERROR The fw_cfg file does not fit into Buffer.
> + (This is considered a QEMU configuration
> + error; BufferSize is considered authoritative
> + for the contents of the fw_cfg file identified
> + by FileName.)
> +
> + @retval RETURN_PROTOCOL_ERROR The fw_cfg file contents are not themselves
> + NUL-terminated, and an extra NUL byte does not
> + fit into Buffer. (Again a QEMU configuration
> + error.)
> +
> + @return Error codes propagated from
> + QemuFwCfgFindFile().
> +**/
> +STATIC
> +RETURN_STATUS
> +QemuFwCfgGetAsString (
> + IN CONST CHAR8 *FileName,
> + IN OUT UINTN *BufferSize,
> + OUT CHAR8 *Buffer
> + )
> +{
> + RETURN_STATUS Status;
> + FIRMWARE_CONFIG_ITEM FwCfgItem;
> + UINTN FwCfgSize;
> +
> + if (!QemuFwCfgIsAvailable ()) {
> + return RETURN_UNSUPPORTED;
> + }
> +
> + Status = QemuFwCfgFindFile (FileName, &FwCfgItem, &FwCfgSize);
> + if (RETURN_ERROR (Status)) {
> + return Status;
> + }
> + if (FwCfgSize > *BufferSize) {
> + return RETURN_PROTOCOL_ERROR;
> + }
> +
> + QemuFwCfgSelectItem (FwCfgItem);
> + QemuFwCfgReadBytes (FwCfgSize, Buffer);
> +
> + //
> + // If Buffer is already NUL-terminated due to fw_cfg contents, we're done.
> + //
> + if (FwCfgSize > 0 && Buffer[FwCfgSize - 1] == '\0') {
> + *BufferSize = FwCfgSize;
> + return RETURN_SUCCESS;
> + }
> + //
> + // Otherwise, append a NUL byte to Buffer (if we have room for it).
> + //
> + if (FwCfgSize == *BufferSize) {
> + return RETURN_PROTOCOL_ERROR;
> + }
> + Buffer[FwCfgSize] = '\0';
> + *BufferSize = FwCfgSize + 1;
> + return RETURN_SUCCESS;
> +}
> +
> +/**
> + Remove a trailing \r\n or \n sequence from a string.
> +
> + @param[in,out] BufferSize On input, the number of bytes in Buffer, including
> + the terminating NUL.
> +
> + On output, the adjusted string size (including the
> + terminating NUL), after stripping the \r\n or \n
> + suffix.
> +
> + @param[in,out] Buffer The NUL-terminated string to trim.
> +**/
> +STATIC
> +VOID
> +StripNewline (
> + IN OUT UINTN *BufferSize,
> + IN OUT CHAR8 *Buffer
> + )
> +{
> + UINTN InSize, OutSize;
> +
> + InSize = *BufferSize;
> + OutSize = InSize;
> +
> + if (InSize >= 3 &&
> + Buffer[InSize - 3] == '\r' && Buffer[InSize - 2] == '\n') {
> + OutSize = InSize - 2;
> + } else if (InSize >= 2 && Buffer[InSize - 2] == '\n') {
> + OutSize = InSize - 1;
> + }
> +
> + if (OutSize < InSize) {
> + Buffer[OutSize - 1] = '\0';
> + *BufferSize = OutSize;
> + }
> +}
> +
> +/**
> + Read the fw_cfg file identified by FileName as a string into a small array
> + with automatic storage duration, using QemuFwCfgGetAsString(). Parse the
> + string as a UINT64. Perform a range-check on the parsed value.
> +
> + @param[in] FileName The name of the fw_cfg file to look up and parse.
> +
> + @param[in] ParseAsHex If TRUE, call BaseLib's AsciiStrHexToUint64S() for
> + parsing the fw_cfg file.
> +
> + If FALSE, call BaseLib's AsciiStrDecimalToUint64S()
> + for parsing the fw_cfg file.
> +
> + @param[in] Limit The inclusive upper bound on the parsed UINT64 value.
> +
> + @param[out] Value On success, Value has been parsed with the BaseLib
> + function determined by ParseAsHex, and has been
> + range-checked against [0, Limit].
> +
> + On failure, Value is not changed.
> +
> + @retval RETURN_SUCCESS Parsing successful. Value has been set.
> +
> + @retval RETURN_UNSUPPORTED Firmware configuration is unavailable.
> +
> + @retval RETURN_PROTOCOL_ERROR Parsing failed. Value has not been changed.
> +
> + @retval RETURN_PROTOCOL_ERROR Parsing succeeded, but the result does not fit
> + in the [0, Limit] range. Value has not been
> + changed.
> +
> + @return Error codes propagated from
> + QemuFwCfgFindFile() and from the BaseLib
> + function selected by ParseAsHex. Value has not
> + been changed.
> +**/
> +STATIC
> +RETURN_STATUS
> +QemuFwCfgParseUint64WithLimit (
> + IN CONST CHAR8 *FileName,
> + IN BOOLEAN ParseAsHex,
> + IN UINT64 Limit,
> + OUT UINT64 *Value
> + )
> +{
> + UINTN Uint64StringSize;
> + CHAR8 Uint64String[UINT64_STRING_MAX_SIZE + CRLF_LENGTH];
> + RETURN_STATUS Status;
> + CHAR8 *EndPointer;
> + UINT64 Uint64;
> +
> + Uint64StringSize = sizeof Uint64String;
> + Status = QemuFwCfgGetAsString (FileName, &Uint64StringSize, Uint64String);
> + if (RETURN_ERROR (Status)) {
> + return Status;
> + }
> +
> + StripNewline (&Uint64StringSize, Uint64String);
> +
> + if (ParseAsHex) {
> + Status = AsciiStrHexToUint64S (Uint64String, &EndPointer, &Uint64);
> + } else {
> + Status = AsciiStrDecimalToUint64S (Uint64String, &EndPointer, &Uint64);
> + }
> + if (RETURN_ERROR (Status)) {
> + return Status;
> + }
> +
> + //
> + // Report a wire protocol error if the subject sequence is empty, or trailing
> + // garbage is present, or Limit is not honored.
> + //
> + if (EndPointer == Uint64String || *EndPointer != '\0' || Uint64 > Limit) {
> + return RETURN_PROTOCOL_ERROR;
> + }
> +
> + *Value = Uint64;
> + return RETURN_SUCCESS;
> +}
> +
> +//
> +// Public functions.
> +//
> +
> +RETURN_STATUS
> +EFIAPI
> +QemuFwCfgSimpleParserInit (
> + VOID
> + )
> +{
> + //
> + // Do nothing, just participate in constructor dependency ordering.
> + //
> + return RETURN_SUCCESS;
> +}
> +
> +RETURN_STATUS
> +EFIAPI
> +QemuFwCfgParseBool (
> + IN CONST CHAR8 *FileName,
> + OUT BOOLEAN *Value
> + )
> +{
> + UINTN BoolStringSize;
> + CHAR8 BoolString[BOOL_STRING_MAX_SIZE + CRLF_LENGTH];
> + RETURN_STATUS Status;
> + UINTN Idx;
> +
> + BoolStringSize = sizeof BoolString;
> + Status = QemuFwCfgGetAsString (FileName, &BoolStringSize, BoolString);
> + if (RETURN_ERROR (Status)) {
> + return Status;
> + }
> +
> + StripNewline (&BoolStringSize, BoolString);
> +
> + for (Idx = 0; Idx < ARRAY_SIZE (mTrueString); ++Idx) {
> + if (AsciiStriCmp (BoolString, mTrueString[Idx]) == 0) {
> + *Value = TRUE;
> + return RETURN_SUCCESS;
> + }
> + }
> +
> + for (Idx = 0; Idx < ARRAY_SIZE (mFalseString); ++Idx) {
> + if (AsciiStriCmp (BoolString, mFalseString[Idx]) == 0) {
> + *Value = FALSE;
> + return RETURN_SUCCESS;
> + }
> + }
> +
> + return RETURN_PROTOCOL_ERROR;
> +}
> +
> +RETURN_STATUS
> +EFIAPI
> +QemuFwCfgParseUint8 (
> + IN CONST CHAR8 *FileName,
> + IN BOOLEAN ParseAsHex,
> + OUT UINT8 *Value
> + )
> +{
> + RETURN_STATUS Status;
> + UINT64 Uint64;
> +
> + Status = QemuFwCfgParseUint64WithLimit (FileName, ParseAsHex, MAX_UINT8,
> + &Uint64);
> + if (RETURN_ERROR (Status)) {
> + return Status;
> + }
> + *Value = (UINT8)Uint64;
> + return RETURN_SUCCESS;
> +}
> +
> +RETURN_STATUS
> +EFIAPI
> +QemuFwCfgParseUint16 (
> + IN CONST CHAR8 *FileName,
> + IN BOOLEAN ParseAsHex,
> + OUT UINT16 *Value
> + )
> +{
> + RETURN_STATUS Status;
> + UINT64 Uint64;
> +
> + Status = QemuFwCfgParseUint64WithLimit (FileName, ParseAsHex, MAX_UINT16,
> + &Uint64);
> + if (RETURN_ERROR (Status)) {
> + return Status;
> + }
> + *Value = (UINT16)Uint64;
> + return RETURN_SUCCESS;
> +}
> +
> +RETURN_STATUS
> +EFIAPI
> +QemuFwCfgParseUint32 (
> + IN CONST CHAR8 *FileName,
> + IN BOOLEAN ParseAsHex,
> + OUT UINT32 *Value
> + )
> +{
> + RETURN_STATUS Status;
> + UINT64 Uint64;
> +
> + Status = QemuFwCfgParseUint64WithLimit (FileName, ParseAsHex, MAX_UINT32,
> + &Uint64);
> + if (RETURN_ERROR (Status)) {
> + return Status;
> + }
> + *Value = (UINT32)Uint64;
> + return RETURN_SUCCESS;
> +}
> +
> +RETURN_STATUS
> +EFIAPI
> +QemuFwCfgParseUint64 (
> + IN CONST CHAR8 *FileName,
> + IN BOOLEAN ParseAsHex,
> + OUT UINT64 *Value
> + )
> +{
> + RETURN_STATUS Status;
> + UINT64 Uint64;
> +
> + Status = QemuFwCfgParseUint64WithLimit (FileName, ParseAsHex, MAX_UINT64,
> + &Uint64);
> + if (RETURN_ERROR (Status)) {
> + return Status;
> + }
> + *Value = Uint64;
> + return RETURN_SUCCESS;
> +}
> +
> +RETURN_STATUS
> +EFIAPI
> +QemuFwCfgParseUintn (
> + IN CONST CHAR8 *FileName,
> + IN BOOLEAN ParseAsHex,
> + OUT UINTN *Value
> + )
> +{
> + RETURN_STATUS Status;
> + UINT64 Uint64;
> +
> + Status = QemuFwCfgParseUint64WithLimit (FileName, ParseAsHex, MAX_UINTN,
> + &Uint64);
> + if (RETURN_ERROR (Status)) {
> + return Status;
> + }
> + *Value = (UINTN)Uint64;
> + return RETURN_SUCCESS;
> +}
>
Neat.
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 1/7] OvmfPkg: introduce QemuFwCfgSimpleParserLib
2020-04-24 9:13 ` Philippe Mathieu-Daudé
@ 2020-04-28 11:47 ` Laszlo Ersek
0 siblings, 0 replies; 20+ messages in thread
From: Laszlo Ersek @ 2020-04-28 11:47 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, edk2-devel-groups-io
Cc: Ard Biesheuvel, Jordan Justen, Per Sundstrom
On 04/24/20 11:13, Philippe Mathieu-Daudé wrote:
> On 4/24/20 9:53 AM, Laszlo Ersek wrote:
>> We already parse some boolean and integer values from named fw_cfg files
>> (usually into PCDs), and we're going to cover more. Add a dedicated
>> library for centralizing the parsing logic.
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Per Sundstrom <per_sundstrom@yahoo.com>
>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> OvmfPkg/OvmfPkg.dec
>> | 4 +
>>
>> OvmfPkg/OvmfPkgIa32.dsc
>> | 1 +
>>
>> OvmfPkg/OvmfPkgIa32X64.dsc
>> | 1 +
>>
>> OvmfPkg/OvmfPkgX64.dsc
>> | 1 +
>>
>> OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h
>> | 128 +++++++
>>
>> OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
>> | 27 ++
>>
>> OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParser.c
>> | 398 ++++++++++++++++++++
>> 7 files changed, 560 insertions(+)
>>
>> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
>> index 28030391cff2..8a46fe73344e 100644
>> --- a/OvmfPkg/OvmfPkg.dec
>> +++ b/OvmfPkg/OvmfPkg.dec
>> @@ -60,6 +60,10 @@ [LibraryClasses]
>> #
>> QemuFwCfgS3Lib|Include/Library/QemuFwCfgS3Lib.h
>> + ## @libraryclass Parse the contents of named fw_cfg files as
>> simple
>> + # (scalar) data types.
>> + QemuFwCfgSimpleParserLib|Include/Library/QemuFwCfgSimpleParserLib.h
>> +
>> ## @libraryclass Rewrite the BootOrder NvVar based on QEMU's
>> "bootorder"
>> # fw_cfg file.
>> #
>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>> index d5e90c001370..5e2972063110 100644
>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>> @@ -160,6 +160,7 @@ [LibraryClasses]
>> UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
>>
>> SerializeVariablesLib|OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf
>>
>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
>> +
>> QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
>>
>> VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
>> LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
>>
>> MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
>>
>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>> index 066f49aeaee0..18e6909a33fa 100644
>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>> @@ -164,6 +164,7 @@ [LibraryClasses]
>> UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
>>
>> SerializeVariablesLib|OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf
>>
>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
>> +
>> QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
>>
>> VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
>> LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
>>
>> MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
>>
>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>> index ac510522a9ff..3d24cc4c1cfb 100644
>> --- a/OvmfPkg/OvmfPkgX64.dsc
>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>> @@ -164,6 +164,7 @@ [LibraryClasses]
>> UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
>>
>> SerializeVariablesLib|OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf
>>
>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
>> +
>> QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
>>
>> VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
>> LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf
>>
>> MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
>>
>> diff --git a/OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h
>> b/OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h
>> new file mode 100644
>> index 000000000000..c6062bae8770
>> --- /dev/null
>> +++ b/OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h
>> @@ -0,0 +1,128 @@
>> +/** @file
>> + Parse the contents of named fw_cfg files as simple (scalar) data
>> types.
>> +
>> + Copyright (C) 2020, Red Hat, Inc.
>> +
>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>> +**/
>> +
>> +#ifndef QEMU_FW_CFG_SIMPLE_PARSER_LIB_H_
>> +#define QEMU_FW_CFG_SIMPLE_PARSER_LIB_H_
>> +
>> +#include <Base.h>
>> +
>> +/**
>> + Look up FileName with QemuFwCfgFindFile() from QemuFwCfgLib. Read
>> the fw_cfg
>> + file into a small array with automatic storage duration. Parse the
>> array as
>> + the textual representation of a BOOLEAN.
>> +
>> + @param[in] FileName The name of the fw_cfg file to look up and parse.
>> +
>> + @param[out] Value On success, Value is TRUE if the contents of
>> the fw_cfg
>> + file case-insensitively match "true", "yes", "y",
>> + "enable", "enabled", "1".
>> +
>> + On success, Value is FALSE if the contents of
>> the fw_cfg
>> + file case-insensitively match "false", "no", "n",
>> + "disable", "disabled", "0".
>> +
>> + On failure, Value is not changed.
>> +
>> + @retval RETURN_SUCCESS Parsing successful. Value has been set.
>> +
>> + @retval RETURN_UNSUPPORTED Firmware configuration is unavailable.
>> +
>> + @retval RETURN_PROTOCOL_ERROR Parsing failed. Value has not been
>> changed.
>> +
>> + @return Error codes propagated from
>> + QemuFwCfgFindFile(). Value has not been
>> + changed.
>> +**/
>> +RETURN_STATUS
>> +EFIAPI
>> +QemuFwCfgParseBool (
>> + IN CONST CHAR8 *FileName,
>> + OUT BOOLEAN *Value
>> + );
>> +
>> +/**
>> + Look up FileName with QemuFwCfgFindFile() from QemuFwCfgLib. Read
>> the fw_cfg
>> + file into a small array with automatic storage duration. Parse the
>> array as
>> + the textual representation of a UINT8.
>> +
>> + @param[in] FileName The name of the fw_cfg file to look up and
>> parse.
>> +
>> + @param[in] ParseAsHex If TRUE, call BaseLib's
>> AsciiStrHexToUint64S() for
>> + parsing the fw_cfg file.
>> +
>> + If FALSE, call BaseLib's
>> AsciiStrDecimalToUint64S()
>> + for parsing the fw_cfg file.
>> +
>> + @param[out] Value On success, Value has been parsed with the
>> BaseLib
>> + function determined by ParseAsHex, and also
>> + range-checked for [0, MAX_UINT8].
>> +
>> + On failure, Value is not changed.
>> +
>> + @retval RETURN_SUCCESS Parsing successful. Value has been set.
>> +
>> + @retval RETURN_UNSUPPORTED Firmware configuration is unavailable.
>> +
>> + @retval RETURN_PROTOCOL_ERROR Parsing failed. Value has not been
>> changed.
>> +
>> + @retval RETURN_PROTOCOL_ERROR Parsing succeeded, but the result
>> does not fit
>> + in the [0, MAX_UINT8] range. Value
>> has not
>> + been changed.
>
> I wondered about RETURN_BAD_BUFFER_SIZE / RETURN_BUFFER_TOO_SMALL for
> this case, but RETURN_PROTOCOL_ERROR is less confusing.
Right. I too considered the ...BUFFER... error codes first. I opted for
"protocol error" instead because the ...BUFFER... codes "blame the
caller", or at least inform the caller to repeat the call with a more
appropriately sized buffer (or integer range).
But in this case, the caller knows best, by design. So "bad buffer size"
is actually a QEMU misconfiguration / information exchange error (hence
"protocol error") between QEMU and firmware. The caller (in edk2) is not
supposed to retry with a differently sized buffer (or integer range).
[...]
> Neat.
> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
>
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/7] OvmfPkg/PlatformPei: parse "X-PciMmio64Mb" with QemuFwCfgSimpleParserLib
2020-04-24 7:53 [PATCH 0/7] OvmfPkg, ArmVirtPkg: control PXE v4/v6 boot support from the QEMU cmdline Laszlo Ersek
2020-04-24 7:53 ` [PATCH 1/7] OvmfPkg: introduce QemuFwCfgSimpleParserLib Laszlo Ersek
@ 2020-04-24 7:53 ` Laszlo Ersek
2020-04-24 8:51 ` Philippe Mathieu-Daudé
2020-04-24 7:53 ` [PATCH 3/7] OvmfPkg/PlatformPei: use QemuFwCfgParseBool in UPDATE_BOOLEAN_PCD_FROM_ Laszlo Ersek
` (6 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Laszlo Ersek @ 2020-04-24 7:53 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Jordan Justen, Per Sundstrom,
Philippe Mathieu-Daudé
Replace the
- QemuFwCfgFindFile(),
- QemuFwCfgSelectItem(),
- QemuFwCfgReadBytes(),
- AsciiStrDecimalToUint64()
sequence in the GetFirstNonAddress() function with a call to
QemuFwCfgSimpleParserLib.
This change is compatible with valid strings accepted previously.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Per Sundstrom <per_sundstrom@yahoo.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/PlatformPei/PlatformPei.inf | 1 +
OvmfPkg/PlatformPei/MemDetect.c | 36 ++++++++++++--------
2 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 19f2424981bc..e72ef7963d97 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -60,6 +60,7 @@ [LibraryClasses]
PeimEntryPoint
QemuFwCfgLib
QemuFwCfgS3Lib
+ QemuFwCfgSimpleParserLib
MtrrLib
MemEncryptSevLib
PcdLib
diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index 47dc9c543719..f32df937f9ba 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -33,6 +33,7 @@ Module Name:
#include <Library/ResourcePublicationLib.h>
#include <Library/MtrrLib.h>
#include <Library/QemuFwCfgLib.h>
+#include <Library/QemuFwCfgSimpleParserLib.h>
#include "Platform.h"
#include "Cmos.h"
@@ -336,7 +337,7 @@ GetFirstNonAddress (
{
UINT64 FirstNonAddress;
UINT64 Pci64Base, Pci64Size;
- CHAR8 MbString[7 + 1];
+ UINT32 FwCfgPciMmio64Mb;
EFI_STATUS Status;
FIRMWARE_CONFIG_ITEM FwCfgItem;
UINTN FwCfgSize;
@@ -379,25 +380,30 @@ GetFirstNonAddress (
//
// See if the user specified the number of megabytes for the 64-bit PCI host
- // aperture. The number of non-NUL characters in MbString allows for
- // 9,999,999 MB, which is approximately 10 TB.
+ // aperture. Accept an aperture size up to 16TB.
//
// As signaled by the "X-" prefix, this knob is experimental, and might go
// away at any time.
//
- Status = QemuFwCfgFindFile ("opt/ovmf/X-PciMmio64Mb", &FwCfgItem,
- &FwCfgSize);
- if (!EFI_ERROR (Status)) {
- if (FwCfgSize >= sizeof MbString) {
- DEBUG ((EFI_D_WARN,
- "%a: ignoring malformed 64-bit PCI host aperture size from fw_cfg\n",
- __FUNCTION__));
- } else {
- QemuFwCfgSelectItem (FwCfgItem);
- QemuFwCfgReadBytes (FwCfgSize, MbString);
- MbString[FwCfgSize] = '\0';
- Pci64Size = LShiftU64 (AsciiStrDecimalToUint64 (MbString), 20);
+ Status = QemuFwCfgParseUint32 ("opt/ovmf/X-PciMmio64Mb", FALSE,
+ &FwCfgPciMmio64Mb);
+ switch (Status) {
+ case EFI_UNSUPPORTED:
+ case EFI_NOT_FOUND:
+ break;
+ case EFI_SUCCESS:
+ if (FwCfgPciMmio64Mb <= 0x1000000) {
+ Pci64Size = LShiftU64 (FwCfgPciMmio64Mb, 20);
+ break;
}
+ //
+ // fall through
+ //
+ default:
+ DEBUG ((DEBUG_WARN,
+ "%a: ignoring malformed 64-bit PCI host aperture size from fw_cfg\n",
+ __FUNCTION__));
+ break;
}
if (Pci64Size == 0) {
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 2/7] OvmfPkg/PlatformPei: parse "X-PciMmio64Mb" with QemuFwCfgSimpleParserLib
2020-04-24 7:53 ` [PATCH 2/7] OvmfPkg/PlatformPei: parse "X-PciMmio64Mb" with QemuFwCfgSimpleParserLib Laszlo Ersek
@ 2020-04-24 8:51 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-24 8:51 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: Ard Biesheuvel, Jordan Justen, Per Sundstrom
On 4/24/20 9:53 AM, Laszlo Ersek wrote:
> Replace the
>
> - QemuFwCfgFindFile(),
> - QemuFwCfgSelectItem(),
> - QemuFwCfgReadBytes(),
> - AsciiStrDecimalToUint64()
>
> sequence in the GetFirstNonAddress() function with a call to
> QemuFwCfgSimpleParserLib.
>
> This change is compatible with valid strings accepted previously.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Per Sundstrom <per_sundstrom@yahoo.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> OvmfPkg/PlatformPei/PlatformPei.inf | 1 +
> OvmfPkg/PlatformPei/MemDetect.c | 36 ++++++++++++--------
> 2 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
> index 19f2424981bc..e72ef7963d97 100644
> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf
> @@ -60,6 +60,7 @@ [LibraryClasses]
> PeimEntryPoint
> QemuFwCfgLib
> QemuFwCfgS3Lib
> + QemuFwCfgSimpleParserLib
> MtrrLib
> MemEncryptSevLib
> PcdLib
> diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
> index 47dc9c543719..f32df937f9ba 100644
> --- a/OvmfPkg/PlatformPei/MemDetect.c
> +++ b/OvmfPkg/PlatformPei/MemDetect.c
> @@ -33,6 +33,7 @@ Module Name:
> #include <Library/ResourcePublicationLib.h>
> #include <Library/MtrrLib.h>
> #include <Library/QemuFwCfgLib.h>
> +#include <Library/QemuFwCfgSimpleParserLib.h>
>
> #include "Platform.h"
> #include "Cmos.h"
> @@ -336,7 +337,7 @@ GetFirstNonAddress (
> {
> UINT64 FirstNonAddress;
> UINT64 Pci64Base, Pci64Size;
> - CHAR8 MbString[7 + 1];
Replaced by UINT64_STRING_MAX_SIZE + CRLF_LENGTH, way safer.
> + UINT32 FwCfgPciMmio64Mb;
> EFI_STATUS Status;
> FIRMWARE_CONFIG_ITEM FwCfgItem;
> UINTN FwCfgSize;
> @@ -379,25 +380,30 @@ GetFirstNonAddress (
>
> //
> // See if the user specified the number of megabytes for the 64-bit PCI host
> - // aperture. The number of non-NUL characters in MbString allows for
> - // 9,999,999 MB, which is approximately 10 TB.
> + // aperture. Accept an aperture size up to 16TB.
> //
> // As signaled by the "X-" prefix, this knob is experimental, and might go
> // away at any time.
> //
> - Status = QemuFwCfgFindFile ("opt/ovmf/X-PciMmio64Mb", &FwCfgItem,
> - &FwCfgSize);
> - if (!EFI_ERROR (Status)) {
> - if (FwCfgSize >= sizeof MbString) {
> - DEBUG ((EFI_D_WARN,
> - "%a: ignoring malformed 64-bit PCI host aperture size from fw_cfg\n",
> - __FUNCTION__));
> - } else {
> - QemuFwCfgSelectItem (FwCfgItem);
> - QemuFwCfgReadBytes (FwCfgSize, MbString);
Up to here is QemuFwCfgGetAsString(), OK.
> - MbString[FwCfgSize] = '\0';
StripNewline(), OK.
> - Pci64Size = LShiftU64 (AsciiStrDecimalToUint64 (MbString), 20);
> + Status = QemuFwCfgParseUint32 ("opt/ovmf/X-PciMmio64Mb", FALSE,
ParseAsHex=False, so we use AsciiStrDecimalToUint64S(), OK.
> + &FwCfgPciMmio64Mb);
> + switch (Status) {
> + case EFI_UNSUPPORTED:
> + case EFI_NOT_FOUND:
> + break;
> + case EFI_SUCCESS:
> + if (FwCfgPciMmio64Mb <= 0x1000000) {
> + Pci64Size = LShiftU64 (FwCfgPciMmio64Mb, 20);
16TB, OK.
> + break;
> }
> + //
> + // fall through
> + //
> + default:
> + DEBUG ((DEBUG_WARN,
> + "%a: ignoring malformed 64-bit PCI host aperture size from fw_cfg\n",
> + __FUNCTION__));
> + break;
> }
>
> if (Pci64Size == 0) {
>
The new API is very clean!
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/7] OvmfPkg/PlatformPei: use QemuFwCfgParseBool in UPDATE_BOOLEAN_PCD_FROM_...
2020-04-24 7:53 [PATCH 0/7] OvmfPkg, ArmVirtPkg: control PXE v4/v6 boot support from the QEMU cmdline Laszlo Ersek
2020-04-24 7:53 ` [PATCH 1/7] OvmfPkg: introduce QemuFwCfgSimpleParserLib Laszlo Ersek
2020-04-24 7:53 ` [PATCH 2/7] OvmfPkg/PlatformPei: parse "X-PciMmio64Mb" with QemuFwCfgSimpleParserLib Laszlo Ersek
@ 2020-04-24 7:53 ` Laszlo Ersek
2020-04-24 8:55 ` Philippe Mathieu-Daudé
2020-04-24 7:53 ` [PATCH 4/7] OvmfPkg/QemuFwCfgDxeLib: allow UEFI_DRIVER modules Laszlo Ersek
` (5 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Laszlo Ersek @ 2020-04-24 7:53 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Jordan Justen, Per Sundstrom,
Philippe Mathieu-Daudé
The UPDATE_BOOLEAN_PCD_FROM_FW_CFG() macro currently calls the
module-private helper function GetNamedFwCfgBoolean(). Replace the latter
with QemuFwCfgParseBool() from QemuFwCfgSimpleParserLib.
This change is compatible with valid strings accepted previously.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Per Sundstrom <per_sundstrom@yahoo.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/PlatformPei/Platform.c | 47 +-------------------
1 file changed, 2 insertions(+), 45 deletions(-)
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 088e616a980c..3b850c2c2626 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -27,6 +27,7 @@
#include <Library/PeiServicesLib.h>
#include <Library/QemuFwCfgLib.h>
#include <Library/QemuFwCfgS3Lib.h>
+#include <Library/QemuFwCfgSimpleParserLib.h>
#include <Library/ResourcePublicationLib.h>
#include <Ppi/MasterBootMode.h>
#include <IndustryStandard/I440FxPiix4.h>
@@ -254,56 +255,12 @@ MemMapInitialization (
ASSERT_RETURN_ERROR (PcdStatus);
}
-EFI_STATUS
-GetNamedFwCfgBoolean (
- IN CHAR8 *FwCfgFileName,
- OUT BOOLEAN *Setting
- )
-{
- EFI_STATUS Status;
- FIRMWARE_CONFIG_ITEM FwCfgItem;
- UINTN FwCfgSize;
- UINT8 Value[3];
-
- Status = QemuFwCfgFindFile (FwCfgFileName, &FwCfgItem, &FwCfgSize);
- if (EFI_ERROR (Status)) {
- return Status;
- }
- if (FwCfgSize > sizeof Value) {
- return EFI_BAD_BUFFER_SIZE;
- }
- QemuFwCfgSelectItem (FwCfgItem);
- QemuFwCfgReadBytes (FwCfgSize, Value);
-
- if ((FwCfgSize == 1) ||
- (FwCfgSize == 2 && Value[1] == '\n') ||
- (FwCfgSize == 3 && Value[1] == '\r' && Value[2] == '\n')) {
- switch (Value[0]) {
- case '0':
- case 'n':
- case 'N':
- *Setting = FALSE;
- return EFI_SUCCESS;
-
- case '1':
- case 'y':
- case 'Y':
- *Setting = TRUE;
- return EFI_SUCCESS;
-
- default:
- break;
- }
- }
- return EFI_PROTOCOL_ERROR;
-}
-
#define UPDATE_BOOLEAN_PCD_FROM_FW_CFG(TokenName) \
do { \
BOOLEAN Setting; \
RETURN_STATUS PcdStatus; \
\
- if (!EFI_ERROR (GetNamedFwCfgBoolean ( \
+ if (!RETURN_ERROR (QemuFwCfgParseBool ( \
"opt/ovmf/" #TokenName, &Setting))) { \
PcdStatus = PcdSetBoolS (TokenName, Setting); \
ASSERT_RETURN_ERROR (PcdStatus); \
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 3/7] OvmfPkg/PlatformPei: use QemuFwCfgParseBool in UPDATE_BOOLEAN_PCD_FROM_...
2020-04-24 7:53 ` [PATCH 3/7] OvmfPkg/PlatformPei: use QemuFwCfgParseBool in UPDATE_BOOLEAN_PCD_FROM_ Laszlo Ersek
@ 2020-04-24 8:55 ` Philippe Mathieu-Daudé
2020-04-28 11:40 ` Laszlo Ersek
0 siblings, 1 reply; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-24 8:55 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: Ard Biesheuvel, Jordan Justen, Per Sundstrom
On 4/24/20 9:53 AM, Laszlo Ersek wrote:
> The UPDATE_BOOLEAN_PCD_FROM_FW_CFG() macro currently calls the
> module-private helper function GetNamedFwCfgBoolean(). Replace the latter
> with QemuFwCfgParseBool() from QemuFwCfgSimpleParserLib.
>
> This change is compatible with valid strings accepted previously.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Per Sundstrom <per_sundstrom@yahoo.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> OvmfPkg/PlatformPei/Platform.c | 47 +-------------------
> 1 file changed, 2 insertions(+), 45 deletions(-)
>
> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
> index 088e616a980c..3b850c2c2626 100644
> --- a/OvmfPkg/PlatformPei/Platform.c
> +++ b/OvmfPkg/PlatformPei/Platform.c
> @@ -27,6 +27,7 @@
> #include <Library/PeiServicesLib.h>
> #include <Library/QemuFwCfgLib.h>
> #include <Library/QemuFwCfgS3Lib.h>
> +#include <Library/QemuFwCfgSimpleParserLib.h>
> #include <Library/ResourcePublicationLib.h>
> #include <Ppi/MasterBootMode.h>
> #include <IndustryStandard/I440FxPiix4.h>
> @@ -254,56 +255,12 @@ MemMapInitialization (
> ASSERT_RETURN_ERROR (PcdStatus);
> }
>
> -EFI_STATUS
> -GetNamedFwCfgBoolean (
> - IN CHAR8 *FwCfgFileName,
> - OUT BOOLEAN *Setting
> - )
> -{
> - EFI_STATUS Status;
> - FIRMWARE_CONFIG_ITEM FwCfgItem;
> - UINTN FwCfgSize;
> - UINT8 Value[3];
> -
> - Status = QemuFwCfgFindFile (FwCfgFileName, &FwCfgItem, &FwCfgSize);
> - if (EFI_ERROR (Status)) {
> - return Status;
> - }
> - if (FwCfgSize > sizeof Value) {
> - return EFI_BAD_BUFFER_SIZE;
> - }
> - QemuFwCfgSelectItem (FwCfgItem);
> - QemuFwCfgReadBytes (FwCfgSize, Value);
Until here QemuFwCfgGetAsString(), OK.
> -
> - if ((FwCfgSize == 1) ||
> - (FwCfgSize == 2 && Value[1] == '\n') ||
> - (FwCfgSize == 3 && Value[1] == '\r' && Value[2] == '\n')) {
StripNewline(), OK.
> - switch (Value[0]) {
> - case '0':
> - case 'n':
> - case 'N':
> - *Setting = FALSE;
mFalseString[], OK.
(And we get disable[d] + false).
> - return EFI_SUCCESS;
> -
> - case '1':
> - case 'y':
> - case 'Y':
> - *Setting = TRUE;
mTrueString[], OK.
(And we get enable[d] + true).
> - return EFI_SUCCESS;
> -
> - default:
> - break;
> - }
> - }
> - return EFI_PROTOCOL_ERROR;
> -}
> -
> #define UPDATE_BOOLEAN_PCD_FROM_FW_CFG(TokenName) \
> do { \
> BOOLEAN Setting; \
> RETURN_STATUS PcdStatus; \
> \
> - if (!EFI_ERROR (GetNamedFwCfgBoolean ( \
> + if (!RETURN_ERROR (QemuFwCfgParseBool ( \
:)
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
> "opt/ovmf/" #TokenName, &Setting))) { \
> PcdStatus = PcdSetBoolS (TokenName, Setting); \
> ASSERT_RETURN_ERROR (PcdStatus); \
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 3/7] OvmfPkg/PlatformPei: use QemuFwCfgParseBool in UPDATE_BOOLEAN_PCD_FROM_...
2020-04-24 8:55 ` Philippe Mathieu-Daudé
@ 2020-04-28 11:40 ` Laszlo Ersek
0 siblings, 0 replies; 20+ messages in thread
From: Laszlo Ersek @ 2020-04-28 11:40 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, edk2-devel-groups-io
Cc: Ard Biesheuvel, Jordan Justen, Per Sundstrom
On 04/24/20 10:55, Philippe Mathieu-Daudé wrote:
> On 4/24/20 9:53 AM, Laszlo Ersek wrote:
>> The UPDATE_BOOLEAN_PCD_FROM_FW_CFG() macro currently calls the
>> module-private helper function GetNamedFwCfgBoolean(). Replace the latter
>> with QemuFwCfgParseBool() from QemuFwCfgSimpleParserLib.
>>
>> This change is compatible with valid strings accepted previously.
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Per Sundstrom <per_sundstrom@yahoo.com>
>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>> OvmfPkg/PlatformPei/Platform.c | 47 +-------------------
>> 1 file changed, 2 insertions(+), 45 deletions(-)
>>
>> diff --git a/OvmfPkg/PlatformPei/Platform.c
>> b/OvmfPkg/PlatformPei/Platform.c
>> index 088e616a980c..3b850c2c2626 100644
>> --- a/OvmfPkg/PlatformPei/Platform.c
>> +++ b/OvmfPkg/PlatformPei/Platform.c
>> @@ -27,6 +27,7 @@
>> #include <Library/PeiServicesLib.h>
>> #include <Library/QemuFwCfgLib.h>
>> #include <Library/QemuFwCfgS3Lib.h>
>> +#include <Library/QemuFwCfgSimpleParserLib.h>
>> #include <Library/ResourcePublicationLib.h>
>> #include <Ppi/MasterBootMode.h>
>> #include <IndustryStandard/I440FxPiix4.h>
>> @@ -254,56 +255,12 @@ MemMapInitialization (
>> ASSERT_RETURN_ERROR (PcdStatus);
>> }
>> -EFI_STATUS
>> -GetNamedFwCfgBoolean (
>> - IN CHAR8 *FwCfgFileName,
>> - OUT BOOLEAN *Setting
>> - )
>> -{
>> - EFI_STATUS Status;
>> - FIRMWARE_CONFIG_ITEM FwCfgItem;
>> - UINTN FwCfgSize;
>> - UINT8 Value[3];
>> -
>> - Status = QemuFwCfgFindFile (FwCfgFileName, &FwCfgItem, &FwCfgSize);
>> - if (EFI_ERROR (Status)) {
>> - return Status;
>> - }
>> - if (FwCfgSize > sizeof Value) {
>> - return EFI_BAD_BUFFER_SIZE;
>> - }
>> - QemuFwCfgSelectItem (FwCfgItem);
>> - QemuFwCfgReadBytes (FwCfgSize, Value);
>
> Until here QemuFwCfgGetAsString(), OK.
>
>> -
>> - if ((FwCfgSize == 1) ||
>> - (FwCfgSize == 2 && Value[1] == '\n') ||
>> - (FwCfgSize == 3 && Value[1] == '\r' && Value[2] == '\n')) {
>
> StripNewline(), OK.
>
>> - switch (Value[0]) {
>> - case '0':
>> - case 'n':
>> - case 'N':
>> - *Setting = FALSE;
>
> mFalseString[], OK.
>
> (And we get disable[d] + false).
>
>> - return EFI_SUCCESS;
>> -
>> - case '1':
>> - case 'y':
>> - case 'Y':
>> - *Setting = TRUE;
>
> mTrueString[], OK.
>
> (And we get enable[d] + true).
>
>> - return EFI_SUCCESS;
>> -
>> - default:
>> - break;
>> - }
>> - }
>> - return EFI_PROTOCOL_ERROR;
>> -}
>> -
>> #define UPDATE_BOOLEAN_PCD_FROM_FW_CFG(TokenName) \
>> do { \
>> BOOLEAN Setting; \
>> RETURN_STATUS PcdStatus; \
>> \
>> - if (!EFI_ERROR (GetNamedFwCfgBoolean ( \
>> + if (!RETURN_ERROR (QemuFwCfgParseBool ( \
>
> :)
>
> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Thanks for the careful review! :)
Laszlo
>
>> "opt/ovmf/" #TokenName, &Setting))) { \
>> PcdStatus = PcdSetBoolS (TokenName, Setting); \
>> ASSERT_RETURN_ERROR (PcdStatus); \
>>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 4/7] OvmfPkg/QemuFwCfgDxeLib: allow UEFI_DRIVER modules
2020-04-24 7:53 [PATCH 0/7] OvmfPkg, ArmVirtPkg: control PXE v4/v6 boot support from the QEMU cmdline Laszlo Ersek
` (2 preceding siblings ...)
2020-04-24 7:53 ` [PATCH 3/7] OvmfPkg/PlatformPei: use QemuFwCfgParseBool in UPDATE_BOOLEAN_PCD_FROM_ Laszlo Ersek
@ 2020-04-24 7:53 ` Laszlo Ersek
2020-04-28 12:48 ` Philippe Mathieu-Daudé
2020-04-24 7:53 ` [PATCH 5/7] OvmfPkg: control PXEv4 / PXEv6 boot support from the QEMU command line Laszlo Ersek
` (4 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Laszlo Ersek @ 2020-04-24 7:53 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Jordan Justen, Per Sundstrom,
Philippe Mathieu-Daudé
We don't distribute UEFI_DRIVER modules stand-alone that were built as
part of an OVMF platform. OVMF's UEFI_DRIVERs are allowed to inherit
platform dependencies.
By enabling UEFI_DRIVERs to consume QemuFwCfgDxeLib, we can hook
fw_cfg-based NULL class libraries into UEFI drivers, e.g. in order to set
dynamic PCDs.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Per Sundstrom <per_sundstrom@yahoo.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
index c331c5e7220a..48899ff1236a 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
@@ -16,7 +16,7 @@ [Defines]
FILE_GUID = 80474090-55e7-4c28-b25c-9f236ba41f28
MODULE_TYPE = BASE
VERSION_STRING = 1.0
- LIBRARY_CLASS = QemuFwCfgLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER
+ LIBRARY_CLASS = QemuFwCfgLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_DRIVER
CONSTRUCTOR = QemuFwCfgInitialize
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 4/7] OvmfPkg/QemuFwCfgDxeLib: allow UEFI_DRIVER modules
2020-04-24 7:53 ` [PATCH 4/7] OvmfPkg/QemuFwCfgDxeLib: allow UEFI_DRIVER modules Laszlo Ersek
@ 2020-04-28 12:48 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-28 12:48 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: Ard Biesheuvel, Jordan Justen, Per Sundstrom
On 4/24/20 9:53 AM, Laszlo Ersek wrote:
> We don't distribute UEFI_DRIVER modules stand-alone that were built as
> part of an OVMF platform. OVMF's UEFI_DRIVERs are allowed to inherit
> platform dependencies.
>
> By enabling UEFI_DRIVERs to consume QemuFwCfgDxeLib, we can hook
> fw_cfg-based NULL class libraries into UEFI drivers, e.g. in order to set
> dynamic PCDs.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Per Sundstrom <per_sundstrom@yahoo.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
> index c331c5e7220a..48899ff1236a 100644
> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
> @@ -16,7 +16,7 @@ [Defines]
> FILE_GUID = 80474090-55e7-4c28-b25c-9f236ba41f28
> MODULE_TYPE = BASE
> VERSION_STRING = 1.0
> - LIBRARY_CLASS = QemuFwCfgLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER
> + LIBRARY_CLASS = QemuFwCfgLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_DRIVER
>
> CONSTRUCTOR = QemuFwCfgInitialize
>
>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 5/7] OvmfPkg: control PXEv4 / PXEv6 boot support from the QEMU command line
2020-04-24 7:53 [PATCH 0/7] OvmfPkg, ArmVirtPkg: control PXE v4/v6 boot support from the QEMU cmdline Laszlo Ersek
` (3 preceding siblings ...)
2020-04-24 7:53 ` [PATCH 4/7] OvmfPkg/QemuFwCfgDxeLib: allow UEFI_DRIVER modules Laszlo Ersek
@ 2020-04-24 7:53 ` Laszlo Ersek
2020-04-28 12:45 ` Philippe Mathieu-Daudé
2020-04-24 7:53 ` [PATCH 6/7] ArmVirtPkg/QemuFwCfgLib: allow UEFI_DRIVER modules Laszlo Ersek
` (3 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Laszlo Ersek @ 2020-04-24 7:53 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Jordan Justen, Per Sundstrom,
Philippe Mathieu-Daudé
Add a minimal, NULL class library called "PxeBcPcdProducerLib" for setting
the "PcdIPv4PXESupport" and "PcdIPv6PXESupport" PCDs of NetworkPkg, from
fw_cfg.
These PCDs control whether the UefiPxeBcDxe driver supports PXEv4 / PXEv6
boot. If a PXE version is disabled, the corresponding LoadFile protocol
instance is not produced by UefiPxeBcDxe, and so
EfiBootManagerRefreshAllBootOption() in UefiBootManagerLib does not
generate corresponding *new* boot options either. (Existent boot options
are not deleted.)
Hook the library into the UefiPxeBcDxe driver. (The driver is already
included from "NetworkComponents.dsc.inc", but we can list it again in the
DSC file, for providing <LibraryClasses> overrides.)
In OVMF, the PCDs could be set in PlatformPei too, but ArmVirtQemu does
not have fw_cfg access in the PEI phase. Hence a NULL class library that
can be linked into UefiPxeBcDxe.
When listing the PCDs under [PcdsDynamicDefault], stick with the DEC
default values.
QEMU switches:
-fw_cfg name=opt/org.tianocore/IPv4PXESupport,string=[yn]
-fw_cfg name=opt/org.tianocore/IPv6PXESupport,string=[yn]
The "opt/org.tianocore" prefix follows the "opt/RFQDN/" recommendation
from QEMU's "docs/specs/fw_cfg.txt".
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Per Sundstrom <per_sundstrom@yahoo.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/OvmfPkgIa32.dsc | 9 +++++
OvmfPkg/OvmfPkgIa32X64.dsc | 10 +++++
OvmfPkg/OvmfPkgX64.dsc | 9 +++++
OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf | 33 +++++++++++++++++
OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c | 39 ++++++++++++++++++++
5 files changed, 100 insertions(+)
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 5e2972063110..fcd9779b5ba2 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -605,6 +605,10 @@ [PcdsDynamicDefault]
gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
!endif
+ # IPv4 and IPv6 PXE Boot support.
+ gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
+ gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01
+
[PcdsDynamicHii]
!if $(TPM_ENABLE) == TRUE && $(TPM_CONFIG_ENABLE) == TRUE
gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
@@ -822,6 +826,11 @@ [Components]
#
!include NetworkPkg/NetworkComponents.dsc.inc
+ NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf {
+ <LibraryClasses>
+ NULL|OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
+ }
+
!if $(NETWORK_TLS_ENABLE) == TRUE
NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf {
<LibraryClasses>
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 18e6909a33fa..1626d2415a2c 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -616,6 +616,11 @@ [PcdsDynamicDefault]
gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
!endif
+[PcdsDynamicDefault.X64]
+ # IPv4 and IPv6 PXE Boot support.
+ gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
+ gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01
+
[PcdsDynamicHii]
!if $(TPM_ENABLE) == TRUE && $(TPM_CONFIG_ENABLE) == TRUE
gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
@@ -834,6 +839,11 @@ [Components.X64]
#
!include NetworkPkg/NetworkComponents.dsc.inc
+ NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf {
+ <LibraryClasses>
+ NULL|OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
+ }
+
!if $(NETWORK_TLS_ENABLE) == TRUE
NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf {
<LibraryClasses>
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 3d24cc4c1cfb..65cfe957761b 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -615,6 +615,10 @@ [PcdsDynamicDefault]
gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
!endif
+ # IPv4 and IPv6 PXE Boot support.
+ gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
+ gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01
+
[PcdsDynamicHii]
!if $(TPM_ENABLE) == TRUE && $(TPM_CONFIG_ENABLE) == TRUE
gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
@@ -832,6 +836,11 @@ [Components]
#
!include NetworkPkg/NetworkComponents.dsc.inc
+ NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf {
+ <LibraryClasses>
+ NULL|OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
+ }
+
!if $(NETWORK_TLS_ENABLE) == TRUE
NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf {
<LibraryClasses>
diff --git a/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf b/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
new file mode 100644
index 000000000000..948133228111
--- /dev/null
+++ b/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
@@ -0,0 +1,33 @@
+## @file
+# Configure some PCDs dynamically for
+# "NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf", from QEMU's fw_cfg.
+#
+# Copyright (C) 2020, Red Hat, Inc.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+ INF_VERSION = 1.29
+ BASE_NAME = PxeBcPcdProducerLib
+ FILE_GUID = 1da2723f-52df-432a-8d03-6e8fa8acc107
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = NULL
+ CONSTRUCTOR = SetPxeBcPcds
+
+[Sources]
+ PxeBcPcd.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ NetworkPkg/NetworkPkg.dec
+ OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+ PcdLib
+ QemuFwCfgSimpleParserLib
+
+[Pcd]
+ gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport ## SOMETIMES_PRODUCES
+ gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport ## SOMETIMES_PRODUCES
diff --git a/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c b/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c
new file mode 100644
index 000000000000..7ce236326fb4
--- /dev/null
+++ b/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c
@@ -0,0 +1,39 @@
+/** @file
+ Configure some PCDs dynamically for
+ "NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf", from QEMU's fw_cfg.
+
+ Copyright (C) 2020, Red Hat, Inc.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Library/PcdLib.h>
+#include <Library/QemuFwCfgSimpleParserLib.h>
+
+RETURN_STATUS
+EFIAPI
+SetPxeBcPcds (
+ VOID
+ )
+{
+ BOOLEAN FwCfgBool;
+ RETURN_STATUS PcdStatus;
+
+ if (!RETURN_ERROR (QemuFwCfgParseBool ("opt/org.tianocore/IPv4PXESupport",
+ &FwCfgBool))) {
+ PcdStatus = PcdSet8S (PcdIPv4PXESupport, FwCfgBool);
+ if (RETURN_ERROR (PcdStatus)) {
+ return PcdStatus;
+ }
+ }
+
+ if (!RETURN_ERROR (QemuFwCfgParseBool ("opt/org.tianocore/IPv6PXESupport",
+ &FwCfgBool))) {
+ PcdStatus = PcdSet8S (PcdIPv6PXESupport, FwCfgBool);
+ if (RETURN_ERROR (PcdStatus)) {
+ return PcdStatus;
+ }
+ }
+
+ return RETURN_SUCCESS;
+}
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 5/7] OvmfPkg: control PXEv4 / PXEv6 boot support from the QEMU command line
2020-04-24 7:53 ` [PATCH 5/7] OvmfPkg: control PXEv4 / PXEv6 boot support from the QEMU command line Laszlo Ersek
@ 2020-04-28 12:45 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-28 12:45 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: Ard Biesheuvel, Jordan Justen, Per Sundstrom
On 4/24/20 9:53 AM, Laszlo Ersek wrote:
> Add a minimal, NULL class library called "PxeBcPcdProducerLib" for setting
> the "PcdIPv4PXESupport" and "PcdIPv6PXESupport" PCDs of NetworkPkg, from
> fw_cfg.
>
> These PCDs control whether the UefiPxeBcDxe driver supports PXEv4 / PXEv6
> boot. If a PXE version is disabled, the corresponding LoadFile protocol
> instance is not produced by UefiPxeBcDxe, and so
> EfiBootManagerRefreshAllBootOption() in UefiBootManagerLib does not
> generate corresponding *new* boot options either. (Existent boot options
> are not deleted.)
>
> Hook the library into the UefiPxeBcDxe driver. (The driver is already
> included from "NetworkComponents.dsc.inc", but we can list it again in the
> DSC file, for providing <LibraryClasses> overrides.)
>
> In OVMF, the PCDs could be set in PlatformPei too, but ArmVirtQemu does
> not have fw_cfg access in the PEI phase. Hence a NULL class library that
> can be linked into UefiPxeBcDxe.
>
> When listing the PCDs under [PcdsDynamicDefault], stick with the DEC
> default values.
>
> QEMU switches:
>
> -fw_cfg name=opt/org.tianocore/IPv4PXESupport,string=[yn]
>
> -fw_cfg name=opt/org.tianocore/IPv6PXESupport,string=[yn]
>
> The "opt/org.tianocore" prefix follows the "opt/RFQDN/" recommendation
> from QEMU's "docs/specs/fw_cfg.txt".
>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Per Sundstrom <per_sundstrom@yahoo.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> OvmfPkg/OvmfPkgIa32.dsc | 9 +++++
> OvmfPkg/OvmfPkgIa32X64.dsc | 10 +++++
> OvmfPkg/OvmfPkgX64.dsc | 9 +++++
> OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf | 33 +++++++++++++++++
> OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c | 39 ++++++++++++++++++++
> 5 files changed, 100 insertions(+)
>
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 5e2972063110..fcd9779b5ba2 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -605,6 +605,10 @@ [PcdsDynamicDefault]
> gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
> !endif
>
> + # IPv4 and IPv6 PXE Boot support.
> + gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
> + gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01
> +
> [PcdsDynamicHii]
> !if $(TPM_ENABLE) == TRUE && $(TPM_CONFIG_ENABLE) == TRUE
> gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
> @@ -822,6 +826,11 @@ [Components]
> #
> !include NetworkPkg/NetworkComponents.dsc.inc
>
> + NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf {
> + <LibraryClasses>
> + NULL|OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
> + }
> +
> !if $(NETWORK_TLS_ENABLE) == TRUE
> NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf {
> <LibraryClasses>
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 18e6909a33fa..1626d2415a2c 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -616,6 +616,11 @@ [PcdsDynamicDefault]
> gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
> !endif
>
> +[PcdsDynamicDefault.X64]
> + # IPv4 and IPv6 PXE Boot support.
> + gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
> + gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01
> +
> [PcdsDynamicHii]
> !if $(TPM_ENABLE) == TRUE && $(TPM_CONFIG_ENABLE) == TRUE
> gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
> @@ -834,6 +839,11 @@ [Components.X64]
> #
> !include NetworkPkg/NetworkComponents.dsc.inc
>
> + NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf {
> + <LibraryClasses>
> + NULL|OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
> + }
> +
> !if $(NETWORK_TLS_ENABLE) == TRUE
> NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf {
> <LibraryClasses>
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 3d24cc4c1cfb..65cfe957761b 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -615,6 +615,10 @@ [PcdsDynamicDefault]
> gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
> !endif
>
> + # IPv4 and IPv6 PXE Boot support.
> + gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
> + gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01
> +
> [PcdsDynamicHii]
> !if $(TPM_ENABLE) == TRUE && $(TPM_CONFIG_ENABLE) == TRUE
> gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
> @@ -832,6 +836,11 @@ [Components]
> #
> !include NetworkPkg/NetworkComponents.dsc.inc
>
> + NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf {
> + <LibraryClasses>
> + NULL|OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
> + }
> +
> !if $(NETWORK_TLS_ENABLE) == TRUE
> NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf {
> <LibraryClasses>
> diff --git a/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf b/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
> new file mode 100644
> index 000000000000..948133228111
> --- /dev/null
> +++ b/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
> @@ -0,0 +1,33 @@
> +## @file
> +# Configure some PCDs dynamically for
> +# "NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf", from QEMU's fw_cfg.
> +#
> +# Copyright (C) 2020, Red Hat, Inc.
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> + INF_VERSION = 1.29
> + BASE_NAME = PxeBcPcdProducerLib
> + FILE_GUID = 1da2723f-52df-432a-8d03-6e8fa8acc107
> + MODULE_TYPE = BASE
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = NULL
> + CONSTRUCTOR = SetPxeBcPcds
> +
> +[Sources]
> + PxeBcPcd.c
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + NetworkPkg/NetworkPkg.dec
> + OvmfPkg/OvmfPkg.dec
> +
> +[LibraryClasses]
> + PcdLib
> + QemuFwCfgSimpleParserLib
> +
> +[Pcd]
> + gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport ## SOMETIMES_PRODUCES
> + gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport ## SOMETIMES_PRODUCES
> diff --git a/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c b/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c
> new file mode 100644
> index 000000000000..7ce236326fb4
> --- /dev/null
> +++ b/OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c
> @@ -0,0 +1,39 @@
> +/** @file
> + Configure some PCDs dynamically for
> + "NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf", from QEMU's fw_cfg.
> +
> + Copyright (C) 2020, Red Hat, Inc.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <Library/PcdLib.h>
> +#include <Library/QemuFwCfgSimpleParserLib.h>
> +
> +RETURN_STATUS
> +EFIAPI
> +SetPxeBcPcds (
> + VOID
> + )
> +{
> + BOOLEAN FwCfgBool;
> + RETURN_STATUS PcdStatus;
> +
> + if (!RETURN_ERROR (QemuFwCfgParseBool ("opt/org.tianocore/IPv4PXESupport",
> + &FwCfgBool))) {
> + PcdStatus = PcdSet8S (PcdIPv4PXESupport, FwCfgBool);
OK this is the 'current' interface, and NetworkPkg/UefiPxeBcDxe still
uses the deprecated one 'PcdGet8()'.
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
> + if (RETURN_ERROR (PcdStatus)) {
> + return PcdStatus;
> + }
> + }
> +
> + if (!RETURN_ERROR (QemuFwCfgParseBool ("opt/org.tianocore/IPv6PXESupport",
> + &FwCfgBool))) {
> + PcdStatus = PcdSet8S (PcdIPv6PXESupport, FwCfgBool);
> + if (RETURN_ERROR (PcdStatus)) {
> + return PcdStatus;
> + }
> + }
> +
> + return RETURN_SUCCESS;
> +}
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 6/7] ArmVirtPkg/QemuFwCfgLib: allow UEFI_DRIVER modules
2020-04-24 7:53 [PATCH 0/7] OvmfPkg, ArmVirtPkg: control PXE v4/v6 boot support from the QEMU cmdline Laszlo Ersek
` (4 preceding siblings ...)
2020-04-24 7:53 ` [PATCH 5/7] OvmfPkg: control PXEv4 / PXEv6 boot support from the QEMU command line Laszlo Ersek
@ 2020-04-24 7:53 ` Laszlo Ersek
2020-04-28 12:48 ` Philippe Mathieu-Daudé
2020-04-24 7:53 ` [PATCH 7/7] ArmVirtPkg: control PXEv4 / PXEv6 boot support from the QEMU command line Laszlo Ersek
` (2 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Laszlo Ersek @ 2020-04-24 7:53 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Leif Lindholm, Per Sundstrom,
Philippe Mathieu-Daudé
We don't distribute UEFI_DRIVER modules stand-alone that were built as
part of an ArmVirtQemu* platform. ArmVirtQemu* UEFI_DRIVERs are allowed to
inherit platform dependencies.
By enabling UEFI_DRIVERs to consume QemuFwCfgLib, we can hook fw_cfg-based
NULL class libraries into UEFI drivers, e.g. in order to set dynamic PCDs.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Per Sundstrom <per_sundstrom@yahoo.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf b/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
index 4d27d7d30bd3..feceed5f9341 100644
--- a/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
+++ b/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
@@ -15,7 +15,7 @@ [Defines]
FILE_GUID = B271F41F-B841-48A9-BA8D-545B4BC2E2BF
MODULE_TYPE = BASE
VERSION_STRING = 1.0
- LIBRARY_CLASS = QemuFwCfgLib|DXE_DRIVER
+ LIBRARY_CLASS = QemuFwCfgLib|DXE_DRIVER UEFI_DRIVER
CONSTRUCTOR = QemuFwCfgInitialize
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 6/7] ArmVirtPkg/QemuFwCfgLib: allow UEFI_DRIVER modules
2020-04-24 7:53 ` [PATCH 6/7] ArmVirtPkg/QemuFwCfgLib: allow UEFI_DRIVER modules Laszlo Ersek
@ 2020-04-28 12:48 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-28 12:48 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: Ard Biesheuvel, Leif Lindholm, Per Sundstrom
On 4/24/20 9:53 AM, Laszlo Ersek wrote:
> We don't distribute UEFI_DRIVER modules stand-alone that were built as
> part of an ArmVirtQemu* platform. ArmVirtQemu* UEFI_DRIVERs are allowed to
> inherit platform dependencies.
>
> By enabling UEFI_DRIVERs to consume QemuFwCfgLib, we can hook fw_cfg-based
> NULL class libraries into UEFI drivers, e.g. in order to set dynamic PCDs.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Per Sundstrom <per_sundstrom@yahoo.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf b/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> index 4d27d7d30bd3..feceed5f9341 100644
> --- a/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> +++ b/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> @@ -15,7 +15,7 @@ [Defines]
> FILE_GUID = B271F41F-B841-48A9-BA8D-545B4BC2E2BF
> MODULE_TYPE = BASE
> VERSION_STRING = 1.0
> - LIBRARY_CLASS = QemuFwCfgLib|DXE_DRIVER
> + LIBRARY_CLASS = QemuFwCfgLib|DXE_DRIVER UEFI_DRIVER
>
> CONSTRUCTOR = QemuFwCfgInitialize
>
>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 7/7] ArmVirtPkg: control PXEv4 / PXEv6 boot support from the QEMU command line
2020-04-24 7:53 [PATCH 0/7] OvmfPkg, ArmVirtPkg: control PXE v4/v6 boot support from the QEMU cmdline Laszlo Ersek
` (5 preceding siblings ...)
2020-04-24 7:53 ` [PATCH 6/7] ArmVirtPkg/QemuFwCfgLib: allow UEFI_DRIVER modules Laszlo Ersek
@ 2020-04-24 7:53 ` Laszlo Ersek
2020-04-28 12:47 ` Philippe Mathieu-Daudé
2020-04-24 9:00 ` [PATCH 0/7] OvmfPkg, ArmVirtPkg: control PXE v4/v6 boot support from the QEMU cmdline Ard Biesheuvel
2020-04-28 22:39 ` [edk2-devel] " Laszlo Ersek
8 siblings, 1 reply; 20+ messages in thread
From: Laszlo Ersek @ 2020-04-24 7:53 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Leif Lindholm, Per Sundstrom,
Philippe Mathieu-Daudé
Port the DSC file changes from the similarly titled OvmfPkg patch in this
series to ArmVirtPkg.
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Per Sundstrom <per_sundstrom@yahoo.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
ArmVirtPkg/ArmVirtQemu.dsc | 13 +++++++++++++
ArmVirtPkg/ArmVirtQemuKernel.dsc | 13 +++++++++++++
2 files changed, 26 insertions(+)
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 1233f1ece484..3f649c91d8d6 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -58,6 +58,7 @@ [LibraryClasses.common]
VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
QemuFwCfgLib|ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
+ QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
@@ -256,6 +257,12 @@ [PcdsDynamicDefault.common]
gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
+ #
+ # IPv4 and IPv6 PXE Boot support.
+ #
+ gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
+ gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01
+
#
# TPM2 support
#
@@ -430,6 +437,12 @@ [Components.common]
# Networking stack
#
!include NetworkPkg/NetworkComponents.dsc.inc
+
+ NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf {
+ <LibraryClasses>
+ NULL|OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
+ }
+
!if $(NETWORK_TLS_ENABLE) == TRUE
NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf {
<LibraryClasses>
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 0af91f21b7b7..2a6fd6bc06be 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -56,6 +56,7 @@ [LibraryClasses.common]
VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
QemuFwCfgLib|ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
+ QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
ArmVirtMemInfoLib|ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
@@ -239,6 +240,12 @@ [PcdsDynamicDefault.common]
gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
+ #
+ # IPv4 and IPv6 PXE Boot support.
+ #
+ gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
+ gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01
+
################################################################################
#
# Components Section - list of all EDK II Modules needed by this Platform
@@ -369,6 +376,12 @@ [Components.common]
# Networking stack
#
!include NetworkPkg/NetworkComponents.dsc.inc
+
+ NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf {
+ <LibraryClasses>
+ NULL|OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
+ }
+
!if $(NETWORK_TLS_ENABLE) == TRUE
NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf {
<LibraryClasses>
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 7/7] ArmVirtPkg: control PXEv4 / PXEv6 boot support from the QEMU command line
2020-04-24 7:53 ` [PATCH 7/7] ArmVirtPkg: control PXEv4 / PXEv6 boot support from the QEMU command line Laszlo Ersek
@ 2020-04-28 12:47 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-28 12:47 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: Ard Biesheuvel, Leif Lindholm, Per Sundstrom
On 4/24/20 9:53 AM, Laszlo Ersek wrote:
> Port the DSC file changes from the similarly titled OvmfPkg patch in this
> series to ArmVirtPkg.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Per Sundstrom <per_sundstrom@yahoo.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> ArmVirtPkg/ArmVirtQemu.dsc | 13 +++++++++++++
> ArmVirtPkg/ArmVirtQemuKernel.dsc | 13 +++++++++++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 1233f1ece484..3f649c91d8d6 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -58,6 +58,7 @@ [LibraryClasses.common]
> VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
> QemuFwCfgLib|ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
> + QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
> QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
>
> ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
> @@ -256,6 +257,12 @@ [PcdsDynamicDefault.common]
> gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
> gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
>
> + #
> + # IPv4 and IPv6 PXE Boot support.
> + #
> + gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
> + gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01
> +
> #
> # TPM2 support
> #
> @@ -430,6 +437,12 @@ [Components.common]
> # Networking stack
> #
> !include NetworkPkg/NetworkComponents.dsc.inc
> +
> + NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf {
> + <LibraryClasses>
> + NULL|OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
> + }
> +
> !if $(NETWORK_TLS_ENABLE) == TRUE
> NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf {
> <LibraryClasses>
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index 0af91f21b7b7..2a6fd6bc06be 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -56,6 +56,7 @@ [LibraryClasses.common]
> VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
> QemuFwCfgLib|ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
> QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
> + QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
> QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
>
> ArmVirtMemInfoLib|ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
> @@ -239,6 +240,12 @@ [PcdsDynamicDefault.common]
> gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
> gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
>
> + #
> + # IPv4 and IPv6 PXE Boot support.
> + #
> + gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x01
> + gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01
> +
> ################################################################################
> #
> # Components Section - list of all EDK II Modules needed by this Platform
> @@ -369,6 +376,12 @@ [Components.common]
> # Networking stack
> #
> !include NetworkPkg/NetworkComponents.dsc.inc
> +
> + NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf {
> + <LibraryClasses>
> + NULL|OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
> + }
> +
> !if $(NETWORK_TLS_ENABLE) == TRUE
> NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf {
> <LibraryClasses>
>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/7] OvmfPkg, ArmVirtPkg: control PXE v4/v6 boot support from the QEMU cmdline
2020-04-24 7:53 [PATCH 0/7] OvmfPkg, ArmVirtPkg: control PXE v4/v6 boot support from the QEMU cmdline Laszlo Ersek
` (6 preceding siblings ...)
2020-04-24 7:53 ` [PATCH 7/7] ArmVirtPkg: control PXEv4 / PXEv6 boot support from the QEMU command line Laszlo Ersek
@ 2020-04-24 9:00 ` Ard Biesheuvel
2020-04-28 22:39 ` [edk2-devel] " Laszlo Ersek
8 siblings, 0 replies; 20+ messages in thread
From: Ard Biesheuvel @ 2020-04-24 9:00 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: Jordan Justen, Leif Lindholm, Per Sundstrom,
Philippe Mathieu-Daudé
On 4/24/20 9:53 AM, Laszlo Ersek wrote:
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
> Repo: https://pagure.io/lersek/edk2.git
> Branch: pxe_fw_cfg
>
> With this series applied, the QEMU command line options listed below
> control whether the guest firmware supports PXEv4 / PXEv6 boot. And
> correspondingly, whether UefiBootManagerLib generates *new* PXEv4 /
> PXEv6 boot options automatically. (Existent boot options are never
> deleted in response to just the flags below.)
>
> -fw_cfg name=opt/org.tianocore/IPv4PXESupport,string=[yn]
>
> -fw_cfg name=opt/org.tianocore/IPv6PXESupport,string=[yn]
>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Per Sundstrom <per_sundstrom@yahoo.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Thanks
> Laszlo
>
> Laszlo Ersek (7):
> OvmfPkg: introduce QemuFwCfgSimpleParserLib
> OvmfPkg/PlatformPei: parse "X-PciMmio64Mb" with
> QemuFwCfgSimpleParserLib
> OvmfPkg/PlatformPei: use QemuFwCfgParseBool in
> UPDATE_BOOLEAN_PCD_FROM_...
> OvmfPkg/QemuFwCfgDxeLib: allow UEFI_DRIVER modules
> OvmfPkg: control PXEv4 / PXEv6 boot support from the QEMU command line
> ArmVirtPkg/QemuFwCfgLib: allow UEFI_DRIVER modules
> ArmVirtPkg: control PXEv4 / PXEv6 boot support from the QEMU command
> line
>
Very useful,thanks!
For the series,
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ArmVirtPkg/ArmVirtQemu.dsc | 13 +
> ArmVirtPkg/ArmVirtQemuKernel.dsc | 13 +
> ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf | 2 +-
> OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h | 128 +++++++
> OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c | 39 ++
> OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf | 33 ++
> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf | 2 +-
> OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParser.c | 398 ++++++++++++++++++++
> OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf | 27 ++
> OvmfPkg/OvmfPkg.dec | 4 +
> OvmfPkg/OvmfPkgIa32.dsc | 10 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 11 +
> OvmfPkg/OvmfPkgX64.dsc | 10 +
> OvmfPkg/PlatformPei/MemDetect.c | 36 +-
> OvmfPkg/PlatformPei/Platform.c | 47 +--
> OvmfPkg/PlatformPei/PlatformPei.inf | 1 +
> 16 files changed, 712 insertions(+), 62 deletions(-)
> create mode 100644 OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h
> create mode 100644 OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c
> create mode 100644 OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
> create mode 100644 OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParser.c
> create mode 100644 OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [edk2-devel] [PATCH 0/7] OvmfPkg, ArmVirtPkg: control PXE v4/v6 boot support from the QEMU cmdline
2020-04-24 7:53 [PATCH 0/7] OvmfPkg, ArmVirtPkg: control PXE v4/v6 boot support from the QEMU cmdline Laszlo Ersek
` (7 preceding siblings ...)
2020-04-24 9:00 ` [PATCH 0/7] OvmfPkg, ArmVirtPkg: control PXE v4/v6 boot support from the QEMU cmdline Ard Biesheuvel
@ 2020-04-28 22:39 ` Laszlo Ersek
2020-04-29 7:21 ` Philippe Mathieu-Daudé
8 siblings, 1 reply; 20+ messages in thread
From: Laszlo Ersek @ 2020-04-28 22:39 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Jordan Justen, Leif Lindholm, Per Sundstrom,
Philippe Mathieu-Daudé
On 04/24/20 09:53, Laszlo Ersek wrote:
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
> Repo: https://pagure.io/lersek/edk2.git
> Branch: pxe_fw_cfg
>
> With this series applied, the QEMU command line options listed below
> control whether the guest firmware supports PXEv4 / PXEv6 boot. And
> correspondingly, whether UefiBootManagerLib generates *new* PXEv4 /
> PXEv6 boot options automatically. (Existent boot options are never
> deleted in response to just the flags below.)
>
> -fw_cfg name=opt/org.tianocore/IPv4PXESupport,string=[yn]
>
> -fw_cfg name=opt/org.tianocore/IPv6PXESupport,string=[yn]
Merged as commit range 64ab457d1f21..cdc3fa54184a, via
<https://github.com/tianocore/edk2/pull/556>.
I thank everyone for the feedback.
Phil, regarding your comment under patch#5: the PCD "get" interfaces do
not have "deprecated" vs. "current" variants. Only the "set" interfaces
do. "Get" is always supposed to succeed. "Set" may fail.
See commit 9a3558419509 ("MdePkg: Add a set of PcdSetXXS APIs into
PcdLib and remove the ASSERT in original PcdSetXX APIs.", 2015-04-10).
See also EFI_PCD_PROTOCOL in the Platform Init spec:
<https://uefi.org/specifications>.
Thanks!
Laszlo
>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Per Sundstrom <per_sundstrom@yahoo.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Thanks
> Laszlo
>
> Laszlo Ersek (7):
> OvmfPkg: introduce QemuFwCfgSimpleParserLib
> OvmfPkg/PlatformPei: parse "X-PciMmio64Mb" with
> QemuFwCfgSimpleParserLib
> OvmfPkg/PlatformPei: use QemuFwCfgParseBool in
> UPDATE_BOOLEAN_PCD_FROM_...
> OvmfPkg/QemuFwCfgDxeLib: allow UEFI_DRIVER modules
> OvmfPkg: control PXEv4 / PXEv6 boot support from the QEMU command line
> ArmVirtPkg/QemuFwCfgLib: allow UEFI_DRIVER modules
> ArmVirtPkg: control PXEv4 / PXEv6 boot support from the QEMU command
> line
>
> ArmVirtPkg/ArmVirtQemu.dsc | 13 +
> ArmVirtPkg/ArmVirtQemuKernel.dsc | 13 +
> ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf | 2 +-
> OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h | 128 +++++++
> OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c | 39 ++
> OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf | 33 ++
> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf | 2 +-
> OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParser.c | 398 ++++++++++++++++++++
> OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf | 27 ++
> OvmfPkg/OvmfPkg.dec | 4 +
> OvmfPkg/OvmfPkgIa32.dsc | 10 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 11 +
> OvmfPkg/OvmfPkgX64.dsc | 10 +
> OvmfPkg/PlatformPei/MemDetect.c | 36 +-
> OvmfPkg/PlatformPei/Platform.c | 47 +--
> OvmfPkg/PlatformPei/PlatformPei.inf | 1 +
> 16 files changed, 712 insertions(+), 62 deletions(-)
> create mode 100644 OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h
> create mode 100644 OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c
> create mode 100644 OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
> create mode 100644 OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParser.c
> create mode 100644 OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [edk2-devel] [PATCH 0/7] OvmfPkg, ArmVirtPkg: control PXE v4/v6 boot support from the QEMU cmdline
2020-04-28 22:39 ` [edk2-devel] " Laszlo Ersek
@ 2020-04-29 7:21 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-29 7:21 UTC (permalink / raw)
To: devel, lersek; +Cc: Ard Biesheuvel, Jordan Justen, Leif Lindholm, Per Sundstrom
On 4/29/20 12:39 AM, Laszlo Ersek wrote:
> On 04/24/20 09:53, Laszlo Ersek wrote:
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
>> Repo: https://pagure.io/lersek/edk2.git
>> Branch: pxe_fw_cfg
>>
>> With this series applied, the QEMU command line options listed below
>> control whether the guest firmware supports PXEv4 / PXEv6 boot. And
>> correspondingly, whether UefiBootManagerLib generates *new* PXEv4 /
>> PXEv6 boot options automatically. (Existent boot options are never
>> deleted in response to just the flags below.)
>>
>> -fw_cfg name=opt/org.tianocore/IPv4PXESupport,string=[yn]
>>
>> -fw_cfg name=opt/org.tianocore/IPv6PXESupport,string=[yn]
>
> Merged as commit range 64ab457d1f21..cdc3fa54184a, via
> <https://github.com/tianocore/edk2/pull/556>.
>
> I thank everyone for the feedback.
>
> Phil, regarding your comment under patch#5: the PCD "get" interfaces do
> not have "deprecated" vs. "current" variants. Only the "set" interfaces
> do. "Get" is always supposed to succeed. "Set" may fail.
Oh I see, it makes sense.
>
> See commit 9a3558419509 ("MdePkg: Add a set of PcdSetXXS APIs into
> PcdLib and remove the ASSERT in original PcdSetXX APIs.", 2015-04-10).
Got it, thanks for the pointer!
>
> See also EFI_PCD_PROTOCOL in the Platform Init spec:
> <https://uefi.org/specifications>.
>
> Thanks!
> Laszlo
>
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Leif Lindholm <leif@nuviainc.com>
>> Cc: Per Sundstrom <per_sundstrom@yahoo.com>
>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>> Thanks
>> Laszlo
>>
>> Laszlo Ersek (7):
>> OvmfPkg: introduce QemuFwCfgSimpleParserLib
>> OvmfPkg/PlatformPei: parse "X-PciMmio64Mb" with
>> QemuFwCfgSimpleParserLib
>> OvmfPkg/PlatformPei: use QemuFwCfgParseBool in
>> UPDATE_BOOLEAN_PCD_FROM_...
>> OvmfPkg/QemuFwCfgDxeLib: allow UEFI_DRIVER modules
>> OvmfPkg: control PXEv4 / PXEv6 boot support from the QEMU command line
>> ArmVirtPkg/QemuFwCfgLib: allow UEFI_DRIVER modules
>> ArmVirtPkg: control PXEv4 / PXEv6 boot support from the QEMU command
>> line
>>
>> ArmVirtPkg/ArmVirtQemu.dsc | 13 +
>> ArmVirtPkg/ArmVirtQemuKernel.dsc | 13 +
>> ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf | 2 +-
>> OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h | 128 +++++++
>> OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c | 39 ++
>> OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf | 33 ++
>> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf | 2 +-
>> OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParser.c | 398 ++++++++++++++++++++
>> OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf | 27 ++
>> OvmfPkg/OvmfPkg.dec | 4 +
>> OvmfPkg/OvmfPkgIa32.dsc | 10 +
>> OvmfPkg/OvmfPkgIa32X64.dsc | 11 +
>> OvmfPkg/OvmfPkgX64.dsc | 10 +
>> OvmfPkg/PlatformPei/MemDetect.c | 36 +-
>> OvmfPkg/PlatformPei/Platform.c | 47 +--
>> OvmfPkg/PlatformPei/PlatformPei.inf | 1 +
>> 16 files changed, 712 insertions(+), 62 deletions(-)
>> create mode 100644 OvmfPkg/Include/Library/QemuFwCfgSimpleParserLib.h
>> create mode 100644 OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcd.c
>> create mode 100644 OvmfPkg/Library/PxeBcPcdProducerLib/PxeBcPcdProducerLib.inf
>> create mode 100644 OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParser.c
>> create mode 100644 OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
>>
>
^ permalink raw reply [flat|nested] 20+ messages in thread