Series Reviewed-by: Michael D Kinney <michael.d.kinney@...>

Series Reviewed-by: Michael D Kinney <michael.d.kinney@...>

Mike

From: Michael Kubacki <michael.kubacki@...>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D4091

Adds SourceLevelDebugPkg to the "OTHER" CI matrix job so it is built
in edk2 CI.

Cc: Sean Brogan <sean.brogan@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Hao A Wu <hao.a.wu@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
.azurepipelines/templates/pr-gate-build-job.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.azurepipelines/templates/pr-gate-build-job.yml b/.azurepipe=
lines/templates/pr-gate-build-job.yml
index 54a74a1a9873..d1628f1aa55d 100644
--- a/.azurepipelines/templates/pr-gate-build-job.yml
+++ b/.azurepipelines/templates/pr-gate-build-job.yml
@@ -41,7 +41,7 @@ jobs:
Build.Pkgs: 'NetworkPkg,RedfishPkg'
Build.Targets: 'DEBUG,RELEASE,NO-TARGET,NOOPT'
TARGET_OTHER:
- Build.Pkgs: 'PcAtChipsetPkg,PrmPkg,ShellPkg,StandaloneMmPkg'
+ Build.Pkgs: 'PcAtChipsetPkg,PrmPkg,ShellPkg,SourceLevelDebugPkg,=
StandaloneMmPkg'
Build.Targets: 'DEBUG,RELEASE,NO-TARGET,NOOPT'
TARGET_FMP_FAT_TEST:
Build.Pkgs: 'FmpDevicePkg,FatPkg,UnitTestFrameworkPkg,DynamicTab=
lesPkg'
--=20
2.28.0.windows.1

From: Michael Kubacki <michael.kubacki@...>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D4091

Adds the package as a supported package to .pytool/CISettings.py
and adds a CI YAML for the package so it can be run in CI.

Cc: Hao A Wu <hao.a.wu@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
.pytool/CISettings.py | 1 +
SourceLevelDebugPkg/SourceLevelDebugPkg.ci.yaml | 117 ++++++++++++++++++=
++
2 files changed, 118 insertions(+)

diff --git a/.pytool/CISettings.py b/.pytool/CISettings.py
index aef850a54549..f2c6303b2a08 100644
--- a/.pytool/CISettings.py
+++ b/.pytool/CISettings.py
@@ -70,6 +70,7 @@ class Settings(CiBuildSettingsManager, UpdateSettingsMa=
nager, SetupSettingsManag
"UnitTestFrameworkPkg",
"OvmfPkg",
"RedfishPkg",
+ "SourceLevelDebugPkg",
"UefiPayloadPkg"
)
=20
diff --git a/SourceLevelDebugPkg/SourceLevelDebugPkg.ci.yaml b/SourceLeve=
lDebugPkg/SourceLevelDebugPkg.ci.yaml
new file mode 100644
index 000000000000..8887a6d10bc5
--- /dev/null
+++ b/SourceLevelDebugPkg/SourceLevelDebugPkg.ci.yaml
@@ -0,0 +1,117 @@
+## @file
+# Core CI configuration for SourceLevelDebugPkg
+#
+# Copyright (c) Microsoft Corporation
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+{
+ ## options defined .pytool/Plugin/LicenseCheck
+ "LicenseCheck": {
+ "IgnoreFiles": []
+ },
+
+ "EccCheck": {
+ ## Exception sample looks like below:
+ ## "ExceptionList": [
+ ## "<ErrorID>", "<KeyWord>"
+ ## ]
+ "ExceptionList": [
+ ],
+ ## Both file path and directory path are accepted.
+ "IgnoreFiles": []
+ },
+
+ ## options defined .pytool/Plugin/CompilerPlugin
+ "CompilerPlugin": {
+ "DscPath": "SourceLevelDebugPkg.dsc"
+ },
+
+ ## options defined .pytool/Plugin/HostUnitTestCompilerPlugin
+ "HostUnitTestCompilerPlugin": {
+ "DscPath": "" # Don't support this test
+ },
+
+ ## options defined .pytool/Plugin/CharEncodingCheck
+ "CharEncodingCheck": {
+ "IgnoreFiles": []
+ },
+
+ ## options defined .pytool/Plugin/DependencyCheck
+ "DependencyCheck": {
+ "AcceptableDependencies": [
+ "MdeModulePkg/MdeModulePkg.dec",
+ "MdePkg/MdePkg.dec",
+ "SecurityPkg/SecurityPkg.dec",
+ "SourceLevelDebugPkg/SourceLevelDebugPkg.dec",
+ "UefiCpuPkg/UefiCpuPkg.dec"
+ ],
+ # For host based unit tests
+ "AcceptableDependencies-HOST_APPLICATION":[
+ "UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec"
+ ],
+ # For UEFI shell based apps
+ "AcceptableDependencies-UEFI_APPLICATION":[],
+ "IgnoreInf": []
+ },
+
+ ## options defined .pytool/Plugin/DscCompleteCheck
+ "DscCompleteCheck": {
+ "IgnoreInf": [""],
+ "DscPath": "SourceLevelDebugPkg.dsc"
+ },
+
+ ## options defined .pytool/Plugin/HostUnitTestDscCompleteCheck
+ "HostUnitTestDscCompleteCheck": {
+ "IgnoreInf": [""],
+ "DscPath": "" # Don't support this test
+ },
+
+ ## options defined .pytool/Plugin/GuidCheck
+ "GuidCheck": {
+ "IgnoreGuidName": [],
+ "IgnoreGuidValue": [],
+ "IgnoreFoldersAndFiles": [],
+ "IgnoreDuplicates": [],
+ },
+
+ ## options defined .pytool/Plugin/LibraryClassCheck
+ "LibraryClassCheck": {
+ "IgnoreHeaderFile": []
+ },
+
+ ## options defined .pytool/Plugin/SpellCheck
+ "SpellCheck": {
+ "AuditOnly": False, # All failures were addressed when =
SpellCheck was enabled in this package
+ "IgnoreFiles": [], # use gitignore syntax to ignore er=
rors in matching files
+ "ExtendWords": [ # words to extend to the dictionary=
for this package
+ "bidir",
+ "bsp's",
+ "capbility", # comes from external package
+ "dcddi",
+ "dcerstba",
+ "dcportsc",
+ "dcerstsz",
+ "epring",
+ "evalu",
+ "fxrestor",
+ "hccparams",
+ "hcsparams",
+ "iretd",
+ "iretq",
+ "isoch",
+ "mfindex",
+ "ompressed",
+ "portsc",
+ "sequenceno",
+ "smmentrybreak",
+ "stosd",
+ "stosq",
+ "ttach",
+ "urb's",
+ "xhc's"
+ ],
+ "IgnoreStandardPaths": [], # Standard Plugin defined paths tha=
t should be ignore
+ "AdditionalIncludePaths": [] # Additional paths to spell check (=
wildcards supported)
+ }
+}
--=20
2.28.0.windows.1

From: Michael Kubacki <michael.kubacki@...>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D4091

Fixes spelling errors in the package so the SpellCheck CI plugin can
be enabled.

Cc: Hao A Wu <hao.a.wu@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.c =
| 8 +++----
SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgent/SerialIo.c =
| 22 ++++++++++----------
SourceLevelDebugPkg/Library/DebugCommunicationLibSerialPort/DebugCommuni=
cationLibSerialPort.c | 2 +-
SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationL=
ibUsb.c | 2 +-
SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/Ia32/IntHandlerFun=
cs.c | 2 +-
SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/X64/IntHandlerFunc=
s.c | 2 +-
SourceLevelDebugPkg/Include/Library/DebugCommunicationLib.h =
| 2 +-
SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.h =
| 2 +-
SourceLevelDebugPkg/Library/DebugCommunicationLibUsb3/DebugCommunication=
LibUsb3Internal.h | 2 +-
SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/PeCoffExtraActionL=
ib.h | 2 +-
10 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/Debu=
gAgent.c b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugA=
gent.c
index a1e61a6ef90e..b553a2a9aa0c 100644
--- a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.=
c
+++ b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.=
c
@@ -1,5 +1,5 @@
/** @file
- Commond Debug Agent library implementation. It mainly includes
+ Common Debug Agent library implementation. It mainly includes
the first C function called by exception/interrupt handlers,
read/write debug packet to communication with HOST based on transfer
protocol.
@@ -608,7 +608,7 @@ DebugAgentDataMsgPrint (
}
=20
/**
- Read remaing debug packet except for the start symbol
+ Read remaining debug packet except for the start symbol
=20
@param[in] Handle Pointer to Debug Port handle.
@param[in, out] DebugHeader Debug header buffer including start symb=
ol.
@@ -616,7 +616,7 @@ DebugAgentDataMsgPrint (
@retval EFI_SUCCESS Read the symbol in BreakSymbol.
@retval EFI_CRC_ERROR CRC check fail.
@retval EFI_TIMEOUT Timeout occurs when reading debug packet.
- @retval EFI_DEVICE_ERROR Receive the old or responsed packet.
+ @retval EFI_DEVICE_ERROR Receive the old or response packet.
=20
**/
EFI_STATUS
@@ -651,7 +651,7 @@ ReadRemainingBreakPacket (
if (IS_REQUEST (DebugHeader)) {
if (DebugHeader->SequenceNo =3D=3D (UINT8)(Mailbox->HostSequenceNo +=
1)) {
//
- // Only updagte HostSequenceNo for new command packet
+ // Only update HostSequenceNo for new command packet
//
UpdateMailboxContent (Mailbox, DEBUG_MAILBOX_HOST_SEQUENCE_NO_INDE=
X, DebugHeader->SequenceNo);
return EFI_SUCCESS;
diff --git a/SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgent/SerialI=
o.c b/SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgent/SerialIo.c
index 6661275cc343..97d30c802df4 100644
--- a/SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgent/SerialIo.c
+++ b/SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgent/SerialIo.c
@@ -213,10 +213,10 @@ SERIAL_IO_DEVICE_PATH mSerialIoDevicePath =3D {
}
};
=20
-#define DEBGU_SERIAL_IO_FIFO_DEPTH 10
+#define DEBUG_SERIAL_IO_FIFO_DEPTH 10
//
// Data buffer for Terminal input character and Debug Symbols.
-// The depth is DEBGU_SERIAL_IO_FIFO_DEPTH.
+// The depth is DEBUG_SERIAL_IO_FIFO_DEPTH.
// Fields:
// First UINT8: The index of the first data in array Data[].
// Last UINT8: The index, which you can put a new data into arra=
y Data[].
@@ -227,7 +227,7 @@ typedef struct {
UINT8 First;
UINT8 Last;
UINT8 Surplus;
- UINT8 Data[DEBGU_SERIAL_IO_FIFO_DEPTH];
+ UINT8 Data[DEBUG_SERIAL_IO_FIFO_DEPTH];
} DEBUG_SERIAL_FIFO;
=20
//
@@ -236,10 +236,10 @@ typedef struct {
EFI_HANDLE mSerialIoHandle =3D NULL;
UINTN mLoopbackBuffer =3D 0;
DEBUG_SERIAL_FIFO mSerialFifoForTerminal =3D {
- 0, 0, DEBGU_SERIAL_IO_FIFO_DEPTH, { 0 }
+ 0, 0, DEBUG_SERIAL_IO_FIFO_DEPTH, { 0 }
};
DEBUG_SERIAL_FIFO mSerialFifoForDebug =3D {
- 0, 0, DEBGU_SERIAL_IO_FIFO_DEPTH, { 0 }
+ 0, 0, DEBUG_SERIAL_IO_FIFO_DEPTH, { 0 }
};
=20
/**
@@ -251,11 +251,11 @@ DEBUG_SERIAL_FIFO mSerialFifoForDebug =3D {
=20
**/
BOOLEAN
-IsDebugTermianlFifoEmpty (
+IsDebugTerminalFifoEmpty (
IN DEBUG_SERIAL_FIFO *Fifo
)
{
- if (Fifo->Surplus =3D=3D DEBGU_SERIAL_IO_FIFO_DEPTH) {
+ if (Fifo->Surplus =3D=3D DEBUG_SERIAL_IO_FIFO_DEPTH) {
return TRUE;
}
=20
@@ -313,7 +313,7 @@ DebugTerminalFifoAdd (
Fifo->Data[Fifo->Last] =3D Data;
Fifo->Surplus--;
Fifo->Last++;
- if (Fifo->Last =3D=3D DEBGU_SERIAL_IO_FIFO_DEPTH) {
+ if (Fifo->Last =3D=3D DEBUG_SERIAL_IO_FIFO_DEPTH) {
Fifo->Last =3D 0;
}
=20
@@ -339,7 +339,7 @@ DebugTerminalFifoRemove (
//
// if FIFO is empty, no data can remove
//
- if (IsDebugTermianlFifoEmpty (Fifo)) {
+ if (IsDebugTerminalFifoEmpty (Fifo)) {
return EFI_OUT_OF_RESOURCES;
}
=20
@@ -349,7 +349,7 @@ DebugTerminalFifoRemove (
*Data =3D Fifo->Data[Fifo->First];
Fifo->Surplus++;
Fifo->First++;
- if (Fifo->First =3D=3D DEBGU_SERIAL_IO_FIFO_DEPTH) {
+ if (Fifo->First =3D=3D DEBUG_SERIAL_IO_FIFO_DEPTH) {
Fifo->First =3D 0;
}
=20
@@ -532,7 +532,7 @@ SerialGetControl (
// Check to see if the Terminal FIFO is empty and
// check to see if the input buffer in the Debug Communication Library=
is empty
//
- if (!IsDebugTermianlFifoEmpty (&mSerialFifoForTerminal) || DebugPortPo=
llBuffer (Handle)) {
+ if (!IsDebugTerminalFifoEmpty (&mSerialFifoForTerminal) || DebugPortPo=
llBuffer (Handle)) {
*Control &=3D ~EFI_SERIAL_INPUT_BUFFER_EMPTY;
}
=20
diff --git a/SourceLevelDebugPkg/Library/DebugCommunicationLibSerialPort/=
DebugCommunicationLibSerialPort.c b/SourceLevelDebugPkg/Library/DebugComm=
unicationLibSerialPort/DebugCommunicationLibSerialPort.c
index 34c269e6b527..9953904725ac 100644
--- a/SourceLevelDebugPkg/Library/DebugCommunicationLibSerialPort/DebugCo=
mmunicationLibSerialPort.c
+++ b/SourceLevelDebugPkg/Library/DebugCommunicationLibSerialPort/DebugCo=
mmunicationLibSerialPort.c
@@ -70,7 +70,7 @@ DebugPortInitialize (
}
=20
/**
- Read data from debug device and save the datas in buffer.
+ Read data from debug device and save the data in a buffer.
=20
Reads NumberOfBytes data bytes from a debug device into the buffer
specified by Buffer. The number of bytes actually read is returned.
diff --git a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCo=
mmunicationLibUsb.c b/SourceLevelDebugPkg/Library/DebugCommunicationLibUs=
b/DebugCommunicationLibUsb.c
index 479757f5bae2..da55f3e69ef0 100644
--- a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunica=
tionLibUsb.c
+++ b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunica=
tionLibUsb.c
@@ -768,7 +768,7 @@ InitializeUsbDebugHardware (
}
=20
/**
- Read data from debug device and save the datas in buffer.
+ Read data from debug device and save the data in a buffer.
=20
Reads NumberOfBytes data bytes from a debug device into the buffer
specified by Buffer. The number of bytes actually read is returned.
diff --git a/SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/Ia32/I=
ntHandlerFuncs.c b/SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/=
Ia32/IntHandlerFuncs.c
index 600bde10d656..3e6db0db8679 100644
--- a/SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/Ia32/IntHandl=
erFuncs.c
+++ b/SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/Ia32/IntHandl=
erFuncs.c
@@ -15,7 +15,7 @@
@param[in] InterruptType Interrupt type.
=20
@retval TRUE IDT entries were setup by Debug Agent.
- @retval FALSE IDT entries were not setuo by Debug Agent.
+ @retval FALSE IDT entries were not setup by Debug Agent.
=20
**/
BOOLEAN
diff --git a/SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/X64/In=
tHandlerFuncs.c b/SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/X=
64/IntHandlerFuncs.c
index 952285a8eac0..1baa88206b06 100644
--- a/SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/X64/IntHandle=
rFuncs.c
+++ b/SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/X64/IntHandle=
rFuncs.c
@@ -15,7 +15,7 @@
@param[in] InterruptType Interrupt type.
=20
@retval TRUE IDT entries were setup by Debug Agent.
- @retval FALSE IDT entries were not setuo by Debug Agent.
+ @retval FALSE IDT entries were not setup by Debug Agent.
=20
**/
BOOLEAN
diff --git a/SourceLevelDebugPkg/Include/Library/DebugCommunicationLib.h =
b/SourceLevelDebugPkg/Include/Library/DebugCommunicationLib.h
index ad7bccda285c..ebb2168632da 100644
--- a/SourceLevelDebugPkg/Include/Library/DebugCommunicationLib.h
+++ b/SourceLevelDebugPkg/Include/Library/DebugCommunicationLib.h
@@ -71,7 +71,7 @@ DebugPortInitialize (
);
=20
/**
- Read data from debug device and save the datas in buffer.
+ Read data from debug device and save the data in a buffer.
=20
Reads NumberOfBytes data bytes from a debug device into the buffer
specified by Buffer. The number of bytes actually read is returned.
diff --git a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/Debu=
gAgent.h b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugA=
gent.h
index a0ede308efee..4c72f8f3a90c 100644
--- a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.=
h
+++ b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.=
h
@@ -452,7 +452,7 @@ DebugAgentDataMsgPrint (
);
=20
/**
- Read remaing debug packet except for the start symbol
+ Read remaining debug packet except for the start symbol
=20
@param[in] Handle Pointer to Debug Port handle.
@param[in, out] DebugHeader Debug header buffer including start symb=
ol.
diff --git a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb3/DebugC=
ommunicationLibUsb3Internal.h b/SourceLevelDebugPkg/Library/DebugCommunic=
ationLibUsb3/DebugCommunicationLibUsb3Internal.h
index c73233ea49b0..cc5f294f334d 100644
--- a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb3/DebugCommunic=
ationLibUsb3Internal.h
+++ b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb3/DebugCommunic=
ationLibUsb3Internal.h
@@ -586,7 +586,7 @@ XhcWriteDebugReg (
/**
Verifies if the bit positions specified by a mask are set in a registe=
r.
=20
- @param[in, out] Register UNITN register
+ @param[in, out] Register UINTN register
@param[in] BitMask 32-bit mask
=20
@return BOOLEAN - TRUE if all bits specified by the mask are enable=
d.
diff --git a/SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/PeCoff=
ExtraActionLib.h b/SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/=
PeCoffExtraActionLib.h
index 149186b226f4..f428af963231 100644
--- a/SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/PeCoffExtraAc=
tionLib.h
+++ b/SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/PeCoffExtraAc=
tionLib.h
@@ -34,7 +34,7 @@ extern UINTN AsmInterruptHandle;
@param[in] InterruptType Interrupt type.
=20
@retval TRUE IDT entries were setup by Debug Agent.
- @retval FALSE IDT entries were not setuo by Debug Agent.
+ @retval FALSE IDT entries were not setup by Debug Agent.
=20
**/
BOOLEAN
--=20
2.28.0.windows.1

From: Michael Kubacki <michael.kubacki@...>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D4091

1. Fixes pre-existing spelling errors in the package.
2. Adds SourceLevelDebugPkg to edk2 CI.

CI results are available in the PR:
https://github.com/tianocore/edk2/pull/3439

Cc: Hao A Wu <hao.a.wu@...>
Cc: Liming Gao <gaoliming@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Sean Brogan <sean.brogan@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>

Michael Kubacki (3):
SourceLevelDebugPkg: Fix spelling errors
SourceLevelDebugPkg: Add package CI YAML file
.azurepipelines: Add SourceLevelDebugPkg to CI

SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.c =
| 8 +-
SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgent/SerialIo.c =
| 22 ++--
SourceLevelDebugPkg/Library/DebugCommunicationLibSerialPort/DebugCommuni=
cationLibSerialPort.c | 2 +-
SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationL=
ibUsb.c | 2 +-
SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/Ia32/IntHandlerFun=
cs.c | 2 +-
SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/X64/IntHandlerFunc=
s.c | 2 +-
.azurepipelines/templates/pr-gate-build-job.yml =
| 2 +-
.pytool/CISettings.py =
| 1 +
SourceLevelDebugPkg/Include/Library/DebugCommunicationLib.h =
| 2 +-
SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.h =
| 2 +-
SourceLevelDebugPkg/Library/DebugCommunicationLibUsb3/DebugCommunication=
LibUsb3Internal.h | 2 +-
SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/PeCoffExtraActionL=
ib.h | 2 +-
SourceLevelDebugPkg/SourceLevelDebugPkg.ci.yaml =
| 117 ++++++++++++++++++++
13 files changed, 142 insertions(+), 24 deletions(-)
create mode 100644 SourceLevelDebugPkg/SourceLevelDebugPkg.ci.yaml

--
2.28.0.windows.1

Hi Sami

Response inline.[GM]

Best Regards
Girish

On 10/4/2022 2:16 AM, Sami Mujawar wrote:

External email: Use caution opening links or attachments
Hi Girish,
There are 2 cases that need handling. Please see my response inline
marked [SAMI].
Regards,
Sami Mujawar
On 04/10/2022 04:01 am, Girish Mahadevan wrote:

Hello Sami,

My apologies for the late response. I had one question/comment.

Comment marked with [GM] inline.

Best Regards
Girish

On 9/12/2022 8:18 AM, Sami Mujawar wrote:

External email: Use caution opening links or attachments


The Section 6.1.3, SMBIOS specification version 3.6.0 describes the
handling of test strings in SMBIOS tables.

Test strings are added at the end of the formatted portion of the SMBIOS
structure and are referenced by index in the SMBIOS structure.

Therefore, introduce a SmbiosStringTableLib to simplify the publishing
of the string set.

SmbiosStringTableLib introduces a concept of string table which records
the references to the SMBIOS strings as they are added and returns an
string reference which is then assigned to the string field in the
formatted portion of the SMBIOS structure. Once all strings are added,
the library provides an interface to get the required size for the
string
set. This allows sufficient memory to be allocated for the SMBIOS table.
The library also provides an interface to publish the string set in
accordance with the SMBIOS specification.

Example:
EFI_STATUS
BuildSmbiosType17Table () {
   STRING_TABLE         StrTable;
   UINT8                DevLocatorRef;
   UINT8                BankLocatorRef;
   SMBIOS_TABLE_TYPE17  *SmbiosRecord;
   CHAR8                *StringSet;
   ...

   // Initialize string table for 7 strings
   StringTableInitialize (&StrTable, 7);

   StringTableAddString (&StrTable, "SIMM 3", &DevLocatorRef);
   StringTableAddString (&StrTable, "Bank 0", &BankLocatorRef);
   ...

   SmbiosRecord = AllocateZeroPool (
                    sizeof (SMBIOS_TABLE_TYPE17) +
                      StringTableGetStringSetSize (&StrTable)
                    );
   ...
   SmbiosRecord->DeviceLocator = DevLocatorRef;
   SmbiosRecord->BankLocator = BankLocatorRef;
   ...
   // get the string set area
   StringSet = (CHAR8*)(SmbiosRecord + 1);

   // publish the string set
   StringTablePublishStringSet (
     &StrTable,
     StringSet,
     StringTableGetStringSetSize (&StrTable)
     );

   // free string table
   StringTableFree (&StrTable);

   return EFI_SUCCESS;
}

Signed-off-by: Sami Mujawar <sami.mujawar@...>
Cc: Alexei Fedorov <Alexei.Fedorov@...>
Cc: Pierre Gondois <pierre.gondois@...>
Cc: Girish Mahadevan <gmahadevan@...>
Cc: Jeff Brasen <jbrasen@...>
Cc: Ashish Singhal <ashishsingha@...>
Cc: Nick Ramirez <nramirez@...>
Cc: William Watson <wwatson@...>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
---
The changes can be seen at:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fsamimujawar%2Fedk2%2Ftree%2F2370_smbios_stringlib_v1&;data=05%7C01%7Cgmahadevan%40nvidia.com%7Cc983eff139e143b459ea08daa5e0c308%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638004682041794981%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=S5QKosbQutvQs17jTnZUHrxrTcxJmClwuUpdMRVoJKU%3D&amp;reserved=0

DynamicTablesPkg/DynamicTables.dsc.inc |   3 +-
DynamicTablesPkg/DynamicTablesPkg.dec |   5 +-
DynamicTablesPkg/DynamicTablesPkg.dsc |   3 +-
DynamicTablesPkg/Include/Library/SmbiosStringTableLib.h | 119 ++++++++++
DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.c
| 227 ++++++++++++++++++++
DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf
|  25 +++
  6 files changed, 379 insertions(+), 3 deletions(-)

diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc
b/DynamicTablesPkg/DynamicTables.dsc.inc
index
3d4fa0c4c4b67d6770aee8705c80cc18d20c823a..d35acc1788f2ddc5e2b5fc3e4bedfd48251f7ec8
100644
--- a/DynamicTablesPkg/DynamicTables.dsc.inc
+++ b/DynamicTablesPkg/DynamicTables.dsc.inc
@@ -1,7 +1,7 @@
  ## @file
  #  Dsc include file for Dynamic Tables Framework.
  #
-#  Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
+#  Copyright (c) 2017 - 2022, Arm Limited. All rights reserved.<BR>
  #
  #  SPDX-License-Identifier: BSD-2-Clause-Patent
  #
@@ -18,6 +18,7 @@ [LibraryClasses.common]
SsdtPcieSupportLib|DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
SsdtSerialPortFixupLib|DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf
TableHelperLib|DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
+
SmbiosStringTableLib|DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf

  [Components.common]
    #
diff --git a/DynamicTablesPkg/DynamicTablesPkg.dec
b/DynamicTablesPkg/DynamicTablesPkg.dec
index
cc34c2bdd6ff9b5ca508961b3d0fe85ffbb73c12..2a79cfd4edebbdff05bee66fa01a17d68252e8a7
100644
--- a/DynamicTablesPkg/DynamicTablesPkg.dec
+++ b/DynamicTablesPkg/DynamicTablesPkg.dec
@@ -1,7 +1,7 @@
  ## @file
  # dec file for Dynamic Tables Framework.
  #
-# Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
+# Copyright (c) 2017 - 2022, Arm Limited. All rights reserved.<BR>
  #
  # SPDX-License-Identifier: BSD-2-Clause-Patent
  #
@@ -39,6 +39,9 @@ [LibraryClasses]
    ##  @libraryclass  Defines a set of helper methods.
    TableHelperLib|Include/Library/TableHelperLib.h

+  ##  @libraryclass  Defines a set of SMBIOS string helper methods.
+  SmbiosStringTableLib|Include/Library/SmbiosStringTableLib.h
+
  [Protocols]
    # Configuration Manager Protocol GUID
    gEdkiiConfigurationManagerProtocolGuid = { 0xd85a4835, 0x5a82,
0x4894, { 0xac, 0x2, 0x70, 0x6f, 0x43, 0xd5, 0x97, 0x8e } }
diff --git a/DynamicTablesPkg/DynamicTablesPkg.dsc
b/DynamicTablesPkg/DynamicTablesPkg.dsc
index
07cc837552f587fe5bf9031e0061b0234e8698d4..bd5084a9008f040acdd16200ae8cdb23455ac101
100644
--- a/DynamicTablesPkg/DynamicTablesPkg.dsc
+++ b/DynamicTablesPkg/DynamicTablesPkg.dsc
@@ -2,7 +2,7 @@
  #  Dsc file for Dynamic Tables Framework.
  #
  #  Copyright (c) 2019, Linaro Limited. All rights reserved.<BR>
-#  Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.<BR>
+#  Copyright (c) 2019 - 2022, Arm Limited. All rights reserved.<BR>
  #
  #  SPDX-License-Identifier: BSD-2-Clause-Patent
  #
@@ -46,6 +46,7 @@ [Components.common]
DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtHwInfoParserLib.inf
DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
+
DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf

  [BuildOptions]
    *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
diff --git a/DynamicTablesPkg/Include/Library/SmbiosStringTableLib.h
b/DynamicTablesPkg/Include/Library/SmbiosStringTableLib.h
new file mode 100644
index
0000000000000000000000000000000000000000..246d4d30ddf901640ea720c108e2971552ec6c96
--- /dev/null
+++ b/DynamicTablesPkg/Include/Library/SmbiosStringTableLib.h
@@ -0,0 +1,119 @@
+/** @file
+  SMBIOS String Table Helper library.
+
+  Copyright (c) 2022, Arm Limited. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef SMBIOS_STRING_TABLE_H_
+#define SMBIOS_STRING_TABLE_H_
+
+/** A structure representing a string in the string table.
+*/
+typedef struct StringElement {
+  /// Length of the string (does not include the NULL termination)
+  UINTN          StringLen;
+
+  /// Reference to the string
+  CONST CHAR8    *String;
+} STRING_ELEMENT;
+
+/** A structure representing a string table.
+*/
+typedef struct StringTable {
+  /// Count of strings in the table
+  UINT8             StrCount;
+
+  /// Total length of all strings in the table (does not include
+  // the NULL termination for each string)
+  UINTN             TotalStrLen;
+
+  /// Maximum string count
+  UINT8             MaxStringElements;
+
+  /// Pointer to the string table elements
+  STRING_ELEMENT    *Elements;
+} STRING_TABLE;
+
+/** Add a string to the string table
+
+  @param [IN]   StrTable  Pointer to the string table
+  @param [IN]   Str       Pointer to the string
+  @param [OUT]  StrRef    Optional pointer to retrieve the string field
+                          reference of the string in the string table
+
+  @return EFI_SUCCESS            Success
+  @return EFI_INVALID_PARAMETER  Invalid string table pointer
+  @return EFI_BUFFER_TOO_SMALL   Insufficient space to add string
+**/
+EFI_STATUS
+EFIAPI
+StringTableAddString (
+  IN        STRING_TABLE *CONST  StrTable,
+  IN  CONST CHAR8                *Str,
+  OUT       UINT8                *StrRef      OPTIONAL
+  );
+
+/** Returns the total size required to publish the strings to the
SMBIOS
+    string area.
+
+  @param [IN] StrTable              Pointer to the string table
+
+  @return Total size required to publish the strings in the SMBIOS
string area.
+**/
+UINTN
+EFIAPI
+StringTableGetStringSetSize (
+  IN  STRING_TABLE *CONST  StrTable
+  );
+
+/** Iterate through the string table and publish the strings in the
SMBIOS
+    string area.
+
+  @param [IN] StrTable              Pointer to the string table
+  @param [IN] SmbiosStringAreaStart Start address of the SMBIOS
string area.
+  @param [IN] SmbiosStringAreaSize  Size of the SMBIOS string area.
+
+  @return EFI_SUCCESS            Success
+  @return EFI_INVALID_PARAMETER  Invalid string table pointer
+  @return EFI_BUFFER_TOO_SMALL   Insufficient space to publish strings
+**/
+EFI_STATUS
+EFIAPI
+StringTablePublishStringSet (
+  IN        STRING_TABLE  *CONST  StrTable,
+  IN        CHAR8         *CONST  SmbiosStringAreaStart,
+  IN  CONST UINTN                 SmbiosStringAreaSize
+  );
+
+/** Initialise the string table and allocate memory for the string
elements.
+
+  @param [IN] StrTable           Pointer to the string table
+  @param [IN] MaxStringElements  Maximum number of strings that the
string
+                                 table can hold.
+
+  @return EFI_SUCCESS            Success
+  @return EFI_INVALID_PARAMETER  Invalid string table pointer
+  @return EFI_OUT_OF_RESOURCES   Failed to allocate memory for
string elements
+**/
+EFI_STATUS
+EFIAPI
+StringTableInitialize (
+  IN STRING_TABLE *CONST  StrTable,
+  IN UINTN                MaxStringElements
+  );
+
+/** Free memory allocated for the string elements in the string table.
+
+  @param [IN] StrTable           Pointer to the string table
+
+  @return EFI_SUCCESS            Success
+  @return EFI_INVALID_PARAMETER  Invalid string table pointer or
string elements
+**/
+EFI_STATUS
+EFIAPI
+StringTableFree (
+  IN STRING_TABLE *CONST  StrTable
+  );
+
+#endif // SMBIOS_STRING_TABLE_H_
diff --git
a/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.c
b/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.c

new file mode 100644
index
0000000000000000000000000000000000000000..bd186f27fe1613cf819b6600e62960c003a796d6
--- /dev/null
+++
b/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.c
@@ -0,0 +1,227 @@
+/** @file
+  SMBIOS String Table Helper
+
+  Copyright (c) 2022, Arm Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Reference(s):
+  - DSP0134 - SMBIOS Specification Version 3.6.0, 2022-06-17
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/SmbiosStringTableLib.h>
+
+/** Add a string to the string table
+
+  @param [IN]   StrTable  Pointer to the string table
+  @param [IN]   Str       Pointer to the string
+  @param [OUT]  StrRef    Optional pointer to retrieve the string field
+                          reference of the string in the string table
+
+  @return EFI_SUCCESS            Success
+  @return EFI_INVALID_PARAMETER  Invalid string table pointer
+  @return EFI_BUFFER_TOO_SMALL   Insufficient space to add string
+**/
+EFI_STATUS
+EFIAPI
+StringTableAddString (
+  IN        STRING_TABLE *CONST  StrTable,
+  IN  CONST CHAR8                *Str,
+  OUT       UINT8                *StrRef      OPTIONAL
+  )
+{
+  UINTN           StrLength;
+  STRING_ELEMENT  *StrElement;
+
+  if ((StrTable == NULL) || (Str == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (StrTable->StrCount >= StrTable->MaxStringElements) {
+    return EFI_BUFFER_TOO_SMALL;
+  }
+
+  StrLength = AsciiStrLen (Str);
+  if (StrLength == 0) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  // Update the string element
+  StrElement            = &StrTable->Elements[StrTable->StrCount];
+  StrElement->StringLen = StrLength;
+  StrElement->String    = Str;
+
+  // Update String table information
+  StrTable->TotalStrLen += StrLength;
+  StrTable->StrCount++;
+
+  // Return the index of the string in the string table if requested
+  if (StrRef != NULL) {
+    // Note: SMBIOS string field references start at 1. So, return the
+    // StrCount as the string refrence after it is updated.
+    *StrRef = StrTable->StrCount;
+  }
+
+  return EFI_SUCCESS;
+}
+
+/** Returns the total size required to publish the strings to the
SMBIOS
+    string area.
+
+  @param [IN] StrTable              Pointer to the string table
+
+  @return Total size required to publish the strings in the SMBIOS
string area.
+**/
+UINTN
+EFIAPI
+StringTableGetStringSetSize (
+  IN  STRING_TABLE *CONST  StrTable
+  )
+{
+  if (StrTable == NULL) {
+    ASSERT (0);
+    return 0;
+  }
+
+  // See Section 6.1.3 Text strings, SMBIOS Specification Version 3.6.0
+  // - If the formatted portion of the structure contains
string-reference
+  //   fields and all the string fields are set to 0 (no string
references),
+  //   the formatted section of the structure is followed by two
null (00h)
+  //   BYTES.
+  // - Each string is terminated with a null (00h) BYTE
+  // - and the set of strings is terminated with an additional null
(00h) BYTE.
+
+  // Therefore, if string count = 0, return 2
+  // if string count > 0, the string set size =
+  // StrTable->TotalStrLen (total length of the strings in the
string table)
+  // + StrTable->StrCount (add string count to include '\0' for each
string)
+  // +1 (an additional '\0' is required at the end of the string set).
+  return (StrTable->StrCount == 0) ? 2 :
+         (StrTable->TotalStrLen + StrTable->StrCount + 1);
+}
+
+/** Iterate through the string table and publish the strings in the
SMBIOS
+    string area.
+
+  @param [IN] StrTable              Pointer to the string table
+  @param [IN] SmbiosStringAreaStart Start address of the SMBIOS
string area.
+  @param [IN] SmbiosStringAreaSize  Size of the SMBIOS string area.
+
+  @return EFI_SUCCESS            Success
+  @return EFI_INVALID_PARAMETER  Invalid string table pointer
+  @return EFI_BUFFER_TOO_SMALL   Insufficient space to publish strings
+**/
+EFI_STATUS
+EFIAPI
+StringTablePublishStringSet (
+  IN        STRING_TABLE  *CONST  StrTable,
+  IN        CHAR8         *CONST  SmbiosStringAreaStart,
+  IN  CONST UINTN                 SmbiosStringAreaSize
+  )
+{
+  UINT8           Index;
+  STRING_ELEMENT  *StrElement;
+  CHAR8           *SmbiosString;
+  UINTN           BytesRemaining;
+  UINTN           BytesCopied;
+
+  if ((StrTable == NULL) || (SmbiosStringAreaStart == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (SmbiosStringAreaSize < StringTableGetStringSetSize (StrTable)) {
+    return EFI_BUFFER_TOO_SMALL;
+  }
+
+  SmbiosString   = SmbiosStringAreaStart;
+  BytesRemaining = SmbiosStringAreaSize;

[SAMI] SmbiosString points to the startof the sting storage area.

+
+  if (StrTable->StrCount == 0) {
+    // See Section 6.1.3 Text strings, SMBIOS Specification Version
3.6.0
+    // If the formatted portion of the structure contains
string-reference
+    // fields and all the string fields are set to 0 (no string
references),
+    // the formatted section of the structure is followed by two
null (00h)
+    // BYTES.
+    *SmbiosString++ = '\0';

[SAMI] Case 1: SmbiosString[0] is set to '\0' and the pointer is
incremented. So now it points to SmbiosString[1] which will be updated
with the String table null terminator before the function returns.

+  } else {
+    for (Index = 0; Index < StrTable->StrCount; Index++) {
+      StrElement = &StrTable->Elements[Index];
+      AsciiStrCpyS (SmbiosString, BytesRemaining, StrElement->String);
+
+      // See Section 6.1.3 Text strings, SMBIOS Specification
Version 3.6.0
+      // - Each string is terminated with a null (00h) BYTE
+      // Bytes Copied = String length + 1 for the string NULL
terminator.
+      BytesCopied     = StrElement->StringLen + 1;
+      BytesRemaining -= BytesCopied;
+      SmbiosString   += BytesCopied;

[SAMI] Case 2: AsciiStrCpyS() copies the string including the
terrminating null character and BytesCopied is incremented accordingly.
SmbiosString is then incremented by BytesCopied. So, SmbiosString should
point to the byte where the next string starts or will be updated with
the String table null terminator before the function returns.

+    }
+  }
+
+  // See Section 6.1.3 Text strings, SMBIOS Specification Version 3.6.0
+  // - the set of strings is terminated with an additional null
(00h) BYTE.
+  *SmbiosString = '\0';

[GM] Shouldn't you advance the SmbiosString pointer by one more ?
After the loop isn't SmbiosString going to be at the NULL char of the
last string ? And we're trying to add one more NULL character ?
Should it be:
SmbiosString++;

[SAMI] I don't think this increment is required for the reasons
explained above in Case 1 & Case 2. However, please let me know if I
have missed something.

*SmbiosString = '\0';

[GM]
Got it. Thanks for detail.
Patch looks good to me, I've tested it as well. Thanks a lot.

+  return EFI_SUCCESS;
+}
+
+/** Initialise the string table and allocate memory for the string
elements.
+
+  @param [IN] StrTable           Pointer to the string table
+  @param [IN] MaxStringElements  Maximum number of strings that the
string
+                                 table can hold.
+
+  @return EFI_SUCCESS            Success
+  @return EFI_INVALID_PARAMETER  Invalid string table pointer
+  @return EFI_OUT_OF_RESOURCES   Failed to allocate memory for
string elements
+**/
+EFI_STATUS
+EFIAPI
+StringTableInitialize (
+  IN STRING_TABLE *CONST  StrTable,
+  IN UINTN                MaxStringElements
+  )
+{
+  STRING_ELEMENT  *Elements;
+
+  if ((StrTable == NULL) || (MaxStringElements > MAX_UINT8)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  ZeroMem (StrTable, sizeof (STRING_TABLE));
+
+  Elements = (STRING_ELEMENT *)AllocateZeroPool (
+                                 sizeof (STRING_ELEMENT) *
MaxStringElements
+                                 );
+  if (Elements == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  StrTable->Elements          = Elements;
+  StrTable->MaxStringElements = (UINT8)MaxStringElements;
+  return EFI_SUCCESS;
+}
+
+/** Free memory allocated for the string elements in the string table.
+
+  @param [IN] StrTable           Pointer to the string table
+
+  @return EFI_SUCCESS            Success
+  @return EFI_INVALID_PARAMETER  Invalid string table pointer or
string elements
+**/
+EFI_STATUS
+EFIAPI
+StringTableFree (
+  IN STRING_TABLE *CONST  StrTable
+  )
+{
+  if ((StrTable == NULL) || (StrTable->Elements == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  FreePool (StrTable->Elements);
+  ZeroMem (StrTable, sizeof (STRING_TABLE));
+  return EFI_SUCCESS;
+}
diff --git
a/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf
b/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf

new file mode 100644
index
0000000000000000000000000000000000000000..88624c50e3f3e930074222b6d686f75485538b26
--- /dev/null
+++
b/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf
@@ -0,0 +1,25 @@
+## @file
+#  SMBIOS String Table Helper library.
+#
+#  Copyright (c) 2022, Arm Limited. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION    = 0x0001001B
+  BASE_NAME      = SmbiosStringTableLib
+  FILE_GUID      = 8C570DD8-531E-473F-85C6-9252746DBAC1
+  VERSION_STRING = 1.0
+  MODULE_TYPE    = DXE_DRIVER
+  LIBRARY_CLASS  = SmbiosStringTableLib
+
+[Sources]
+  SmbiosStringTableLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  DynamicTablesPkg/DynamicTablesPkg.dec
+
+[LibraryClasses]
+  BaseLib
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Prevents potential math over and underflows when comparing buffers
for SMM validity.

Original patch uploaded to bugzilla by Bret Barkelew <bret@...>.
I've adapted the patch to latest master, uncristified, dropped MU
comments.

Ref: CVE-2021-38578
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3387
Signed-off-by: Gerd Hoffmann <kraxel@...>
---
MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf | 1 +
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | 1 +
MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 1 +
MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 32 ++++++++++++++++-------
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 10 +++++--
5 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
index c8bfae3860fc..3df44b38f13c 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
@@ -60,6 +60,7 @@ [LibraryClasses]
PerformanceLib
HobLib
SmmMemLib
+ SafeIntLib

[Protocols]
gEfiDxeSmmReadyToLockProtocolGuid ## UNDEFINED # SmiHandlerRegister
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
index 6109d6b5449c..ddeb39cee266 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
@@ -46,6 +46,7 @@ [LibraryClasses]
DxeServicesLib
PcdLib
ReportStatusCodeLib
+ SafeIntLib

[Protocols]
gEfiSmmBase2ProtocolGuid ## PRODUCES
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
index 71422b9dfcdf..b8a490a8c3b5 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
@@ -54,6 +54,7 @@
#include <Library/PerformanceLib.h>
#include <Library/HobLib.h>
#include <Library/SmmMemLib.h>
+#include <Library/SafeIntLib.h>

#include "PiSmmCorePrivateData.h"
#include "HeapGuard.h"
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
index 9e5c6cbe33dd..5ef8b31ee361 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
@@ -610,6 +610,7 @@ SmmEndOfS3ResumeHandler (
@param[in] Size2 Size of Buff2

@retval TRUE Buffers overlap in memory.
+ @retval TRUE Math error.
@retval FALSE Buffer doesn't overlap.

**/
@@ -621,11 +622,21 @@ InternalIsBufferOverlapped (
IN UINTN Size2
)
{
+ UINTN End1;
+ UINTN End2;
+
+ // Prevents potential math over and underflows.
+ if (EFI_ERROR (SafeUintnAdd ((UINTN)Buff1, Size1, &End1)) ||
+ EFI_ERROR (SafeUintnAdd ((UINTN)Buff2, Size2, &End2)))
+ {
+ return TRUE;
+ }
+
//
// If buff1's end is less than the start of buff2, then it's ok.
// Also, if buff1's start is beyond buff2's end, then it's ok.
//
- if (((Buff1 + Size1) <= Buff2) || (Buff1 >= (Buff2 + Size2))) {
+ if ((End1 <= (UINTN)Buff2) || ((UINTN)Buff1 >= End2)) {
return FALSE;
}

@@ -699,7 +710,10 @@ SmmEntryPoint (
(UINT8 *)gSmmCorePrivate,
sizeof (*gSmmCorePrivate)
);
- if (!SmmIsBufferOutsideSmmValid ((UINTN)CommunicationBuffer, BufferSize) || IsOverlapped) {
+ if (!SmmIsBufferOutsideSmmValid ((UINTN)CommunicationBuffer, BufferSize) ||
+ IsOverlapped ||
+ EFI_ERROR (SafeUintnSub (BufferSize, OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data), &BufferSize)))
+ {
//
// If CommunicationBuffer is not in valid address scope,
// or there is overlap between gSmmCorePrivate and CommunicationBuffer,
@@ -709,13 +723,13 @@ SmmEntryPoint (
gSmmCorePrivate->ReturnStatus = EFI_ACCESS_DENIED;
} else {
CommunicateHeader = (EFI_SMM_COMMUNICATE_HEADER *)CommunicationBuffer;
- BufferSize -= OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data);
- Status = SmiManage (
- &CommunicateHeader->HeaderGuid,
- NULL,
- CommunicateHeader->Data,
- &BufferSize
- );
+ // BufferSize was updated by the SafeUintnSub() call above.
+ Status = SmiManage (
+ &CommunicateHeader->HeaderGuid,
+ NULL,
+ CommunicateHeader->Data,
+ &BufferSize
+ );
//
// Update CommunicationBuffer, BufferSize and ReturnStatus
// Communicate service finished, reset the pointer to CommBuffer to NULL
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
index 4f00cebaf5ed..f08ca55e26b7 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
@@ -34,6 +34,7 @@
#include <Library/UefiRuntimeLib.h>
#include <Library/PcdLib.h>
#include <Library/ReportStatusCodeLib.h>
+#include <Library/SafeIntLib.h>

#include "PiSmmCorePrivateData.h"

@@ -1354,6 +1355,7 @@ SmmSplitSmramEntry (
@param[in] ReservedRangeToCompare Pointer to EFI_SMM_RESERVED_SMRAM_REGION to compare.

@retval TRUE There is overlap.
+ @retval TRUE Math error.
@retval FALSE There is no overlap.

**/
@@ -1366,8 +1368,12 @@ SmmIsSmramOverlap (
UINT64 RangeToCompareEnd;
UINT64 ReservedRangeToCompareEnd;

- RangeToCompareEnd = RangeToCompare->CpuStart + RangeToCompare->PhysicalSize;
- ReservedRangeToCompareEnd = ReservedRangeToCompare->SmramReservedStart + ReservedRangeToCompare->SmramReservedSize;
+ // Prevents potential math over and underflows.
+ if (EFI_ERROR (SafeUint64Add ((UINT64)RangeToCompare->CpuStart, RangeToCompare->PhysicalSize, &RangeToCompareEnd)) ||
+ EFI_ERROR (SafeUint64Add ((UINT64)ReservedRangeToCompare->SmramReservedStart, ReservedRangeToCompare->SmramReservedSize, &ReservedRangeToCompareEnd)))
+ {
+ return TRUE;
+ }

if ((RangeToCompare->CpuStart >= ReservedRangeToCompare->SmramReservedStart) &&
(RangeToCompare->CpuStart < ReservedRangeToCompareEnd))
--
2.37.3

I would not add link to Wiki from EDK II C Coding Standard Specification.

We want documents published from tianocore-docs to support standalone
formats such as PDF and if there is a link in one of those documents,
we want to know that it is a permanent link. I am concerned we may
reorganize Wiki pages and links in PDF would become stale.

Links from Wiki to specs makes sense.

Mike

[AMD Official Use Only - General]

Try detect physical address space, when successful use it.
Otherwise go continue using the current guesswork code path.

Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/Library/PlatformInitLib/MemDetect.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/OvmfPkg/Library/PlatformInitLib/MemDetect.c b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
index 143a01ceb01e..16ecbfadc30c 100644
--- a/OvmfPkg/Library/PlatformInitLib/MemDetect.c
+++ b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
@@ -761,6 +761,19 @@ PlatformAddressWidthInitialization (
FirstNonAddress = PlatformGetFirstNonAddress (PlatformInfoHob);
}

+ PlatformAddressWidthFromCpuid (PlatformInfoHob, TRUE);
+ if (PlatformInfoHob->PhysMemAddressWidth != 0) {
+ // physical address width is known
+ PlatformInfoHob->FirstNonAddress = FirstNonAddress;
+ return;
+ }
+
+ //
+ // physical address width is NOT known
+ // -> do some guess work, mostly based on installed memory
+ // -> try be conservstibe to stay below the guaranteed minimum of
+ // 36 phys bits (aka 64 GB).
+ //
PhysMemAddressWidth = (UINT8)HighBitSet64 (FirstNonAddress);

//
--
2.37.3

In case the 64-bit pci mmio window is larger than the default size
of 32G be generous and hand out larger chunks of address space for
prefetchable mmio bridge windows.

Cc: Laszlo Ersek <lersek@...>
Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf | 1 +
OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c | 21 ++++++++++++++++++--
2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
index 78b95faa7a7a..f56b1de6fd92 100644
--- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
+++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
@@ -40,6 +40,7 @@ [Protocols]

[Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId ## CONSUMES
+ gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size ## CONSUMES

[Depex]
TRUE
diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
index c122855b735d..3f9c84cf2b54 100644
--- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
+++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
@@ -556,6 +556,7 @@ GetResourcePadding (
EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *Address;
BOOLEAN DefaultIo;
BOOLEAN DefaultMmio;
+ BOOLEAN DefaultPrefMmio;
RESOURCE_PADDING ReservationRequest;
EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *FirstResource;
EFI_STATUS ReservationHintStatus;
@@ -588,8 +589,9 @@ GetResourcePadding (
return EFI_INVALID_PARAMETER;
}

- DefaultIo = TRUE;
- DefaultMmio = TRUE;
+ DefaultIo = TRUE;
+ DefaultMmio = TRUE;
+ DefaultPrefMmio = TRUE;

//
// Init ReservationRequest, and point FirstResource one past the last
@@ -722,6 +724,7 @@ GetResourcePadding (
HighBit = HighBitSetRoundUp32 (ReservationHint.Prefetchable32BitMmio);
if (HighBit != -1) {
SetMmioPadding (--FirstResource, TRUE, TRUE, (UINTN)HighBit);
+ DefaultPrefMmio = FALSE;
}
} else if ((ReservationHint.Prefetchable64BitMmio > 0) &&
(ReservationHint.Prefetchable64BitMmio < MAX_UINT64))
@@ -729,6 +732,7 @@ GetResourcePadding (
HighBit = HighBitSetRoundUp64 (ReservationHint.Prefetchable64BitMmio);
if (HighBit != -1) {
SetMmioPadding (--FirstResource, TRUE, FALSE, (UINTN)HighBit);
+ DefaultPrefMmio = FALSE;
}
}
}
@@ -752,6 +756,19 @@ GetResourcePadding (
);
}

+ if (DefaultPrefMmio) {
+ UINT64 Pci64Size = PcdGet64 (PcdPciMmio64Size);
+
+ if (Pci64Size > SIZE_32GB) {
+ SetMmioPadding (
+ --FirstResource,
+ TRUE,
+ FALSE,
+ (UINTN)HighBitSetRoundUp64 (RShiftU64 (Pci64Size, 8))
+ );
+ }
+ }
+
//
// Output a copy of ReservationRequest from the lowest-address populated
// entry until the end of the structure (including
--
2.37.3

In case we have a reliable PhysMemAddressWidth use that to dynamically
size the 64bit address window. Allocate 1/8 of the physical address
space and place the window at the upper end of the address space.

Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/Library/PlatformInitLib/MemDetect.c | 28 +++++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/OvmfPkg/Library/PlatformInitLib/MemDetect.c b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
index 16ecbfadc30c..ae217d0242ed 100644
--- a/OvmfPkg/Library/PlatformInitLib/MemDetect.c
+++ b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
@@ -604,6 +604,33 @@ PlatformAddressWidthFromCpuid (
}
}

+VOID
+EFIAPI
+PlatformDynamicMmioWindow (
+ IN OUT EFI_HOB_PLATFORM_INFO *PlatformInfoHob
+ )
+{
+ UINT64 AddrSpace, MmioSpace;
+
+ AddrSpace = LShiftU64 (1, PlatformInfoHob->PhysMemAddressWidth);
+ MmioSpace = LShiftU64 (1, PlatformInfoHob->PhysMemAddressWidth - 3);
+
+ if ((PlatformInfoHob->PcdPciMmio64Size < MmioSpace) &&
+ (PlatformInfoHob->PcdPciMmio64Base + MmioSpace < AddrSpace))
+ {
+ DEBUG ((DEBUG_INFO, "%a: using dynamic mmio window\n", __func__));
+ DEBUG ((DEBUG_INFO, "%a: Addr Space 0x%Lx (%Ld GB)\n", __func__, AddrSpace, RShiftU64 (AddrSpace, 30)));
+ DEBUG ((DEBUG_INFO, "%a: MMIO Space 0x%Lx (%Ld GB)\n", __func__, MmioSpace, RShiftU64 (MmioSpace, 30)));
+ PlatformInfoHob->PcdPciMmio64Size = MmioSpace;
+ PlatformInfoHob->PcdPciMmio64Base = AddrSpace - MmioSpace;
+ } else {
+ DEBUG ((DEBUG_INFO, "%a: using classic mmio window\n", __func__));
+ }
+
+ DEBUG ((DEBUG_INFO, "%a: Pci64 Base 0x%Lx\n", __func__, PlatformInfoHob->PcdPciMmio64Base));
+ DEBUG ((DEBUG_INFO, "%a: Pci64 Size 0x%Lx\n", __func__, PlatformInfoHob->PcdPciMmio64Size));
+}
+
/**
Iterate over the PCI host bridges resources information optionally provided
in fw-cfg and find the highest address contained in the PCI MMIO windows. If
@@ -765,6 +792,7 @@ PlatformAddressWidthInitialization (
if (PlatformInfoHob->PhysMemAddressWidth != 0) {
// physical address width is known
PlatformInfoHob->FirstNonAddress = FirstNonAddress;
+ PlatformDynamicMmioWindow (PlatformInfoHob);
return;
}

--
2.37.3

Add some qemu specific quirks to PlatformAddressWidthFromCpuid()
to figure whenever the PhysBits value returned by CPUID is
something real we can work with or not.

See the source code comment for details on the logic.

Also apply some limits to the address space we are going to use:
* Place a hard cap at 47 PhysBits (128 TB) to avoid using addresses
which require 5-level paging support.
* Cap at 40 PhysBits (1 TB) in case the CPU has no support for
gigabyte pages, to avoid excessive amounts of pages being
used for page tables.

Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/Library/PlatformInitLib/MemDetect.c | 102 ++++++++++++++++----
1 file changed, 84 insertions(+), 18 deletions(-)

diff --git a/OvmfPkg/Library/PlatformInitLib/MemDetect.c b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
index d1a4f4b20791..143a01ceb01e 100644
--- a/OvmfPkg/Library/PlatformInitLib/MemDetect.c
+++ b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
@@ -503,39 +503,105 @@ PlatformGetFirstNonAddress (
}

/*
- * Use CPUID to figure physical address width. Does *not* work
- * reliable on qemu. For historical reasons qemu returns phys-bits=40
- * even in case the host machine supports less than that.
- *
- * qemu has a cpu property (host-phys-bits={on,off}) to change that
- * and make sure guest phys-bits are not larger than host phys-bits.,
- * but it is off by default. Exception: microvm machine type
- * hard-wires that property to on.
+ * Use CPUID to figure physical address width.
+ *
+ * Does *not* work reliable on qemu. For historical reasons qemu
+ * returns phys-bits=40 by default even in case the host machine
+ * supports less than that.
+ *
+ * So we apply the following rules (which can be enabled/disabled
+ * using the QemuQuirk parameter) to figure whenever we can work with
+ * the returned physical address width or not:
+ *
+ * (1) If it is 41 or higher consider it valid.
+ * (2) If it is 40 or lower consider it valid in case it matches a
+ * known-good value for the CPU vendor, which is:
+ * -> 36 or 39 for Intel
+ * -> 40 for AMD
+ * (3) Otherwise consider it invalid.
+ *
+ * Recommendation: Run qemu with host-phys-bits=on. That will make
+ * sure guest phys-bits is not larger than host phys-bits. Some
+ * distro builds do that by default.
*/
VOID
EFIAPI
PlatformAddressWidthFromCpuid (
- IN OUT EFI_HOB_PLATFORM_INFO *PlatformInfoHob
+ IN OUT EFI_HOB_PLATFORM_INFO *PlatformInfoHob,
+ IN BOOLEAN QemuQuirk
)
{
- UINT32 RegEax;
+ UINT32 RegEax, RegEbx, RegEcx, RegEdx, Max;
+ UINT8 PhysBits;
+ CHAR8 Signature[13] = { 0 };
+ BOOLEAN Valid = FALSE;
+ BOOLEAN Page1GSupport = FALSE;

- AsmCpuid (0x80000000, &RegEax, NULL, NULL, NULL);
- if (RegEax >= 0x80000008) {
+ AsmCpuid (0x80000000, &RegEax, &RegEbx, &RegEcx, &RegEdx);
+ *(UINT32 *)(Signature + 0) = RegEbx;
+ *(UINT32 *)(Signature + 4) = RegEdx;
+ *(UINT32 *)(Signature + 8) = RegEcx;
+ Max = RegEax;
+
+ if (Max >= 0x80000001) {
+ AsmCpuid (0x80000001, NULL, NULL, NULL, &RegEdx);
+ if ((RegEdx & BIT26) != 0) {
+ Page1GSupport = TRUE;
+ }
+ }
+
+ if (Max >= 0x80000008) {
AsmCpuid (0x80000008, &RegEax, NULL, NULL, NULL);
- PlatformInfoHob->PhysMemAddressWidth = (UINT8)RegEax;
+ PhysBits = (UINT8)RegEax;
} else {
- PlatformInfoHob->PhysMemAddressWidth = 36;
+ PhysBits = 36;
}

- PlatformInfoHob->FirstNonAddress = LShiftU64 (1, PlatformInfoHob->PhysMemAddressWidth);
+ if (!QemuQuirk) {
+ Valid = TRUE;
+ } else if (PhysBits >= 41) {
+ Valid = TRUE;
+ } else if (AsciiStrCmp (Signature, "GenuineIntel") == 0) {
+ if ((PhysBits == 36) || (PhysBits == 39)) {
+ Valid = TRUE;
+ }
+ } else if (AsciiStrCmp (Signature, "AuthenticAMD") == 0) {
+ if (PhysBits == 40) {
+ Valid = TRUE;
+ }
+ }

DEBUG ((
DEBUG_INFO,
- "%a: cpuid: phys-bits is %d\n",
+ "%a: Signature: '%a', PhysBits: %d, QemuQuirk: %a, Valid: %a\n",
__FUNCTION__,
- PlatformInfoHob->PhysMemAddressWidth
+ Signature,
+ PhysBits,
+ QemuQuirk ? "On" : "Off",
+ Valid ? "Yes" : "No"
));
+
+ if (Valid) {
+ if (PhysBits > 47) {
+ /*
+ * Avoid 5-level paging altogether for now, which limits
+ * PhysBits to 48. Also avoid using address bit 48, due to sign
+ * extension we can't identity-map these addresses (and lots of
+ * places in edk2 assume we have everything identity-mapped).
+ * So the actual limit is 47.
+ */
+ DEBUG ((DEBUG_INFO, "%a: limit PhysBits to 47 (avoid 5-level paging)\n", __func__));
+ PhysBits = 47;
+ }
+
+ if (!Page1GSupport && (PhysBits > 40)) {
+ DEBUG ((DEBUG_INFO, "%a: limit PhysBits to 40 (no 1G pages available)\n", __func__));
+ PhysBits = 40;
+ }
+
+ PlatformInfoHob->PhysMemAddressWidth = PhysBits;
+ PlatformInfoHob->FirstNonAddress = LShiftU64 (1, PlatformInfoHob->PhysMemAddressWidth);
+ }
}

/**
@@ -672,7 +738,7 @@ PlatformAddressWidthInitialization (
EFI_STATUS Status;

if (PlatformInfoHob->HostBridgeDevId == 0xffff /* microvm */) {
- PlatformAddressWidthFromCpuid (PlatformInfoHob);
+ PlatformAddressWidthFromCpuid (PlatformInfoHob, FALSE);
return;
}

--
2.37.3

For historical reasons ovmf is quite conservative on address space
usage, to play safe and avoid using more than 36 physical address bits
(the guaranteed minimum) if possible. With devices (specifically GPUs)
becoming larger and larger pci memory bars this becomes increasingly
problematic.

This patch series address that by trying to figure what the physical
address space size is (which is a bit tricky, see patch #1 for details).
If that worked scale up the 64-bit mmio window and also pcie bridge
windows dynamically with the available address space.

v2:
- fix pcie bridge window logic, track prefetchable and non-prefetchable
default state separately (Laszlo).

Gerd Hoffmann (4):
OvmfPkg/PlatformInitLib: qemu cpuid physbits detection
OvmfPkg/PlatformInitLib: detect physical address space
OvmfPkg/PlatformInitLib: dynamic mmio window size
OvmfPkg/PciHotPlugInitDxe: reserve more mmio space

OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf | 1 +
OvmfPkg/Library/PlatformInitLib/MemDetect.c | 143 ++++++++++++++++---
OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c | 21 ++-
3 files changed, 145 insertions(+), 20 deletions(-)

--
2.37.3

While the actual implementation (using qemu fw_cfg) is qemu-specific,
the idea to store the boot order as configured by the VMM in EFI
variables is not. So lets give the variables a more neutral name while
we still can (i.e. no stable tag yet with the new feature).

While being at it also fix the NNNN format (use %x instead of %d for
consistency with BootNNNN).

Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/OvmfPkg.dec | 2 +-
OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf | 2 +-
OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c | 6 +++---
3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 6d689ecc5d55..f13dd4a61f01 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -146,7 +146,7 @@ [Guids]
gConfidentialComputingSecretGuid = {0xadf956ad, 0xe98c, 0x484c, {0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47}}
gConfidentialComputingSevSnpBlobGuid = {0x067b1f5f, 0xcf26, 0x44c5, {0x85, 0x54, 0x93, 0xd7, 0x77, 0x91, 0x2d, 0x42}}
gUefiOvmfPkgPlatformInfoGuid = {0xdec9b486, 0x1f16, 0x47c7, {0x8f, 0x68, 0xdf, 0x1a, 0x41, 0x88, 0x8b, 0xa5}}
- gQemuBootOrderGuid = {0x668f4529, 0x63d0, 0x4bb5, {0xb6, 0x5d, 0x6f, 0xbb, 0x9d, 0x36, 0xa4, 0x4a}}
+ gVMMBootOrderGuid = {0x668f4529, 0x63d0, 0x4bb5, {0xb6, 0x5d, 0x6f, 0xbb, 0x9d, 0x36, 0xa4, 0x4a}}

[Ppis]
# PPI whose presence in the PPI database signals that the TPM base address
diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
index 211344fb0b89..6e320e3e8514 100644
--- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
+++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
@@ -49,7 +49,7 @@ [LibraryClasses]
[Guids]
gEfiGlobalVariableGuid
gVirtioMmioTransportGuid
- gQemuBootOrderGuid
+ gVMMBootOrderGuid

[FeaturePcd]
gUefiOvmfPkgTokenSpaceGuid.PcdQemuBootOrderPciTranslation
diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
index 18646daa67e3..cea4b7a099e3 100644
--- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
+++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
@@ -1709,7 +1709,7 @@ ConnectDevicesFromQemu (
Attempt to retrieve the "bootorder" fw_cfg file from QEMU. Translate
the OpenFirmware device paths therein to UEFI device path fragments.

- On Success store the device path in QemuBootOrderNNNN variables.
+ On Success store the device path in VMMBootOrderNNNN variables.
**/
VOID
EFIAPI
@@ -1794,13 +1794,13 @@ StoreQemuBootOrder (
UnicodeSPrint (
VariableName,
sizeof (VariableName),
- L"QemuBootOrder%04d",
+ L"VMMBootOrder%04x",
VariableIndex++
);
DEBUG ((DEBUG_INFO, "%a: %s = %s\n", __FUNCTION__, VariableName, Translated));
gRT->SetVariable (
VariableName,
- &gQemuBootOrderGuid,
+ &gVMMBootOrderGuid,
EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
GetDevicePathSize (DevicePath),
DevicePath
--
2.37.3

Hello Girish,

On 10/4/22 05:01, Girish Mahadevan wrote:

Hello Sami,
My apologies for the late response. I had one question/comment.
Comment marked with [GM] inline.
Best Regards
Girish
On 9/12/2022 8:18 AM, Sami Mujawar wrote:

External email: Use caution opening links or attachments


The Section 6.1.3, SMBIOS specification version 3.6.0 describes the
handling of test strings in SMBIOS tables.

Test strings are added at the end of the formatted portion of the SMBIOS
structure and are referenced by index in the SMBIOS structure.

Therefore, introduce a SmbiosStringTableLib to simplify the publishing
of the string set.

SmbiosStringTableLib introduces a concept of string table which records
the references to the SMBIOS strings as they are added and returns an
string reference which is then assigned to the string field in the
formatted portion of the SMBIOS structure. Once all strings are added,
the library provides an interface to get the required size for the string
set. This allows sufficient memory to be allocated for the SMBIOS table.
The library also provides an interface to publish the string set in
accordance with the SMBIOS specification.

Example:
EFI_STATUS
BuildSmbiosType17Table () {
STRING_TABLE StrTable;
UINT8 DevLocatorRef;
UINT8 BankLocatorRef;
SMBIOS_TABLE_TYPE17 *SmbiosRecord;
CHAR8 *StringSet;
...

// Initialize string table for 7 strings
StringTableInitialize (&StrTable, 7);

StringTableAddString (&StrTable, "SIMM 3", &DevLocatorRef);
StringTableAddString (&StrTable, "Bank 0", &BankLocatorRef);
...

SmbiosRecord = AllocateZeroPool (
sizeof (SMBIOS_TABLE_TYPE17) +
StringTableGetStringSetSize (&StrTable)
);
...
SmbiosRecord->DeviceLocator = DevLocatorRef;
SmbiosRecord->BankLocator = BankLocatorRef;
...
// get the string set area
StringSet = (CHAR8*)(SmbiosRecord + 1);

// publish the string set
StringTablePublishStringSet (
&StrTable,
StringSet,
StringTableGetStringSetSize (&StrTable)
);

// free string table
StringTableFree (&StrTable);

return EFI_SUCCESS;
}

Signed-off-by: Sami Mujawar <sami.mujawar@...>
Cc: Alexei Fedorov <Alexei.Fedorov@...>
Cc: Pierre Gondois <pierre.gondois@...>
Cc: Girish Mahadevan <gmahadevan@...>
Cc: Jeff Brasen <jbrasen@...>
Cc: Ashish Singhal <ashishsingha@...>
Cc: Nick Ramirez <nramirez@...>
Cc: William Watson <wwatson@...>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
---
The changes can be seen at:
https://github.com/samimujawar/edk2/tree/2370_smbios_stringlib_v1

DynamicTablesPkg/DynamicTables.dsc.inc | 3 +-
DynamicTablesPkg/DynamicTablesPkg.dec | 5 +-
DynamicTablesPkg/DynamicTablesPkg.dsc | 3 +-
DynamicTablesPkg/Include/Library/SmbiosStringTableLib.h | 119 ++++++++++
DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.c | 227 ++++++++++++++++++++
DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf | 25 +++
6 files changed, 379 insertions(+), 3 deletions(-)

diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc b/DynamicTablesPkg/DynamicTables.dsc.inc
index 3d4fa0c4c4b67d6770aee8705c80cc18d20c823a..d35acc1788f2ddc5e2b5fc3e4bedfd48251f7ec8 100644
--- a/DynamicTablesPkg/DynamicTables.dsc.inc
+++ b/DynamicTablesPkg/DynamicTables.dsc.inc
@@ -1,7 +1,7 @@
## @file
# Dsc include file for Dynamic Tables Framework.
#
-# Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
+# Copyright (c) 2017 - 2022, Arm Limited. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -18,6 +18,7 @@ [LibraryClasses.common]
SsdtPcieSupportLib|DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
SsdtSerialPortFixupLib|DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf
TableHelperLib|DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
+ SmbiosStringTableLib|DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf

[Components.common]
#
diff --git a/DynamicTablesPkg/DynamicTablesPkg.dec b/DynamicTablesPkg/DynamicTablesPkg.dec
index cc34c2bdd6ff9b5ca508961b3d0fe85ffbb73c12..2a79cfd4edebbdff05bee66fa01a17d68252e8a7 100644
--- a/DynamicTablesPkg/DynamicTablesPkg.dec
+++ b/DynamicTablesPkg/DynamicTablesPkg.dec
@@ -1,7 +1,7 @@
## @file
# dec file for Dynamic Tables Framework.
#
-# Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
+# Copyright (c) 2017 - 2022, Arm Limited. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -39,6 +39,9 @@ [LibraryClasses]
## @libraryclass Defines a set of helper methods.
TableHelperLib|Include/Library/TableHelperLib.h

+ ## @libraryclass Defines a set of SMBIOS string helper methods.
+ SmbiosStringTableLib|Include/Library/SmbiosStringTableLib.h
+
[Protocols]
# Configuration Manager Protocol GUID
gEdkiiConfigurationManagerProtocolGuid = { 0xd85a4835, 0x5a82, 0x4894, { 0xac, 0x2, 0x70, 0x6f, 0x43, 0xd5, 0x97, 0x8e } }
diff --git a/DynamicTablesPkg/DynamicTablesPkg.dsc b/DynamicTablesPkg/DynamicTablesPkg.dsc
index 07cc837552f587fe5bf9031e0061b0234e8698d4..bd5084a9008f040acdd16200ae8cdb23455ac101 100644
--- a/DynamicTablesPkg/DynamicTablesPkg.dsc
+++ b/DynamicTablesPkg/DynamicTablesPkg.dsc
@@ -2,7 +2,7 @@
# Dsc file for Dynamic Tables Framework.
#
# Copyright (c) 2019, Linaro Limited. All rights reserved.<BR>
-# Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.<BR>
+# Copyright (c) 2019 - 2022, Arm Limited. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -46,6 +46,7 @@ [Components.common]
DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtHwInfoParserLib.inf
DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
+ DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf

[BuildOptions]
*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
diff --git a/DynamicTablesPkg/Include/Library/SmbiosStringTableLib.h b/DynamicTablesPkg/Include/Library/SmbiosStringTableLib.h
new file mode 100644
index 0000000000000000000000000000000000000000..246d4d30ddf901640ea720c108e2971552ec6c96
--- /dev/null
+++ b/DynamicTablesPkg/Include/Library/SmbiosStringTableLib.h
@@ -0,0 +1,119 @@
+/** @file
+ SMBIOS String Table Helper library.
+
+ Copyright (c) 2022, Arm Limited. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef SMBIOS_STRING_TABLE_H_
+#define SMBIOS_STRING_TABLE_H_
+
+/** A structure representing a string in the string table.
+*/
+typedef struct StringElement {
+ /// Length of the string (does not include the NULL termination)
+ UINTN StringLen;
+
+ /// Reference to the string
+ CONST CHAR8 *String;
+} STRING_ELEMENT;
+
+/** A structure representing a string table.
+*/
+typedef struct StringTable {
+ /// Count of strings in the table
+ UINT8 StrCount;
+
+ /// Total length of all strings in the table (does not include
+ // the NULL termination for each string)
+ UINTN TotalStrLen;
+
+ /// Maximum string count
+ UINT8 MaxStringElements;
+
+ /// Pointer to the string table elements
+ STRING_ELEMENT *Elements;
+} STRING_TABLE;
+
+/** Add a string to the string table
+
+ @param [IN] StrTable Pointer to the string table
+ @param [IN] Str Pointer to the string
+ @param [OUT] StrRef Optional pointer to retrieve the string field
+ reference of the string in the string table
+
+ @return EFI_SUCCESS Success
+ @return EFI_INVALID_PARAMETER Invalid string table pointer
+ @return EFI_BUFFER_TOO_SMALL Insufficient space to add string
+**/
+EFI_STATUS
+EFIAPI
+StringTableAddString (
+ IN STRING_TABLE *CONST StrTable,
+ IN CONST CHAR8 *Str,
+ OUT UINT8 *StrRef OPTIONAL
+ );
+
+/** Returns the total size required to publish the strings to the SMBIOS
+ string area.
+
+ @param [IN] StrTable Pointer to the string table
+
+ @return Total size required to publish the strings in the SMBIOS string area.
+**/
+UINTN
+EFIAPI
+StringTableGetStringSetSize (
+ IN STRING_TABLE *CONST StrTable
+ );
+
+/** Iterate through the string table and publish the strings in the SMBIOS
+ string area.
+
+ @param [IN] StrTable Pointer to the string table
+ @param [IN] SmbiosStringAreaStart Start address of the SMBIOS string area.
+ @param [IN] SmbiosStringAreaSize Size of the SMBIOS string area.
+
+ @return EFI_SUCCESS Success
+ @return EFI_INVALID_PARAMETER Invalid string table pointer
+ @return EFI_BUFFER_TOO_SMALL Insufficient space to publish strings
+**/
+EFI_STATUS
+EFIAPI
+StringTablePublishStringSet (
+ IN STRING_TABLE *CONST StrTable,
+ IN CHAR8 *CONST SmbiosStringAreaStart,
+ IN CONST UINTN SmbiosStringAreaSize
+ );
+
+/** Initialise the string table and allocate memory for the string elements.
+
+ @param [IN] StrTable Pointer to the string table
+ @param [IN] MaxStringElements Maximum number of strings that the string
+ table can hold.
+
+ @return EFI_SUCCESS Success
+ @return EFI_INVALID_PARAMETER Invalid string table pointer
+ @return EFI_OUT_OF_RESOURCES Failed to allocate memory for string elements
+**/
+EFI_STATUS
+EFIAPI
+StringTableInitialize (
+ IN STRING_TABLE *CONST StrTable,
+ IN UINTN MaxStringElements
+ );
+
+/** Free memory allocated for the string elements in the string table.
+
+ @param [IN] StrTable Pointer to the string table
+
+ @return EFI_SUCCESS Success
+ @return EFI_INVALID_PARAMETER Invalid string table pointer or string elements
+**/
+EFI_STATUS
+EFIAPI
+StringTableFree (
+ IN STRING_TABLE *CONST StrTable
+ );
+
+#endif // SMBIOS_STRING_TABLE_H_
diff --git a/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.c b/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.c
new file mode 100644
index 0000000000000000000000000000000000000000..bd186f27fe1613cf819b6600e62960c003a796d6
--- /dev/null
+++ b/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.c
@@ -0,0 +1,227 @@
+/** @file
+ SMBIOS String Table Helper
+
+ Copyright (c) 2022, Arm Limited. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Reference(s):
+ - DSP0134 - SMBIOS Specification Version 3.6.0, 2022-06-17
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/SmbiosStringTableLib.h>
+
+/** Add a string to the string table
+
+ @param [IN] StrTable Pointer to the string table
+ @param [IN] Str Pointer to the string
+ @param [OUT] StrRef Optional pointer to retrieve the string field
+ reference of the string in the string table
+
+ @return EFI_SUCCESS Success
+ @return EFI_INVALID_PARAMETER Invalid string table pointer
+ @return EFI_BUFFER_TOO_SMALL Insufficient space to add string
+**/
+EFI_STATUS
+EFIAPI
+StringTableAddString (
+ IN STRING_TABLE *CONST StrTable,
+ IN CONST CHAR8 *Str,
+ OUT UINT8 *StrRef OPTIONAL
+ )
+{
+ UINTN StrLength;
+ STRING_ELEMENT *StrElement;
+
+ if ((StrTable == NULL) || (Str == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (StrTable->StrCount >= StrTable->MaxStringElements) {
+ return EFI_BUFFER_TOO_SMALL;
+ }
+
+ StrLength = AsciiStrLen (Str);
+ if (StrLength == 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ // Update the string element
+ StrElement = &StrTable->Elements[StrTable->StrCount];
+ StrElement->StringLen = StrLength;
+ StrElement->String = Str;
+
+ // Update String table information
+ StrTable->TotalStrLen += StrLength;
+ StrTable->StrCount++;
+
+ // Return the index of the string in the string table if requested
+ if (StrRef != NULL) {
+ // Note: SMBIOS string field references start at 1. So, return the
+ // StrCount as the string refrence after it is updated.
+ *StrRef = StrTable->StrCount;
+ }
+
+ return EFI_SUCCESS;
+}
+
+/** Returns the total size required to publish the strings to the SMBIOS
+ string area.
+
+ @param [IN] StrTable Pointer to the string table
+
+ @return Total size required to publish the strings in the SMBIOS string area.
+**/
+UINTN
+EFIAPI
+StringTableGetStringSetSize (
+ IN STRING_TABLE *CONST StrTable
+ )
+{
+ if (StrTable == NULL) {
+ ASSERT (0);
+ return 0;
+ }
+
+ // See Section 6.1.3 Text strings, SMBIOS Specification Version 3.6.0
+ // - If the formatted portion of the structure contains string-reference
+ // fields and all the string fields are set to 0 (no string references),
+ // the formatted section of the structure is followed by two null (00h)
+ // BYTES.
+ // - Each string is terminated with a null (00h) BYTE
+ // - and the set of strings is terminated with an additional null (00h) BYTE.
+
+ // Therefore, if string count = 0, return 2
+ // if string count > 0, the string set size =
+ // StrTable->TotalStrLen (total length of the strings in the string table)
+ // + StrTable->StrCount (add string count to include '\0' for each string)
+ // +1 (an additional '\0' is required at the end of the string set).
+ return (StrTable->StrCount == 0) ? 2 :
+ (StrTable->TotalStrLen + StrTable->StrCount + 1);
+}
+
+/** Iterate through the string table and publish the strings in the SMBIOS
+ string area.
+
+ @param [IN] StrTable Pointer to the string table
+ @param [IN] SmbiosStringAreaStart Start address of the SMBIOS string area.
+ @param [IN] SmbiosStringAreaSize Size of the SMBIOS string area.
+
+ @return EFI_SUCCESS Success
+ @return EFI_INVALID_PARAMETER Invalid string table pointer
+ @return EFI_BUFFER_TOO_SMALL Insufficient space to publish strings
+**/
+EFI_STATUS
+EFIAPI
+StringTablePublishStringSet (
+ IN STRING_TABLE *CONST StrTable,
+ IN CHAR8 *CONST SmbiosStringAreaStart,
+ IN CONST UINTN SmbiosStringAreaSize
+ )
+{
+ UINT8 Index;
+ STRING_ELEMENT *StrElement;
+ CHAR8 *SmbiosString;
+ UINTN BytesRemaining;
+ UINTN BytesCopied;
+
+ if ((StrTable == NULL) || (SmbiosStringAreaStart == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (SmbiosStringAreaSize < StringTableGetStringSetSize (StrTable)) {
+ return EFI_BUFFER_TOO_SMALL;
+ }
+
+ SmbiosString = SmbiosStringAreaStart;
+ BytesRemaining = SmbiosStringAreaSize;
+
+ if (StrTable->StrCount == 0) {
+ // See Section 6.1.3 Text strings, SMBIOS Specification Version 3.6.0
+ // If the formatted portion of the structure contains string-reference
+ // fields and all the string fields are set to 0 (no string references),
+ // the formatted section of the structure is followed by two null (00h)
+ // BYTES.
+ *SmbiosString++ = '\0';
+ } else {
+ for (Index = 0; Index < StrTable->StrCount; Index++) {
+ StrElement = &StrTable->Elements[Index];
+ AsciiStrCpyS (SmbiosString, BytesRemaining, StrElement->String);
+
+ // See Section 6.1.3 Text strings, SMBIOS Specification Version 3.6.0
+ // - Each string is terminated with a null (00h) BYTE
+ // Bytes Copied = String length + 1 for the string NULL terminator.
+ BytesCopied = StrElement->StringLen + 1;
+ BytesRemaining -= BytesCopied;
+ SmbiosString += BytesCopied;
+ }
+ }
+
+ // See Section 6.1.3 Text strings, SMBIOS Specification Version 3.6.0
+ // - the set of strings is terminated with an additional null (00h) BYTE.
+ *SmbiosString = '\0';

[GM] Shouldn't you advance the SmbiosString pointer by one more ? After
the loop isn't SmbiosString going to be at the NULL char of the last
string ? And we're trying to add one more NULL character ?
Should it be:
SmbiosString++;
*SmbiosString = '\0';

I didn't try to run the code, but it seems ok to me. Assuming the string has 9 chars:
"""
// Copy 9 chars + 1 NULL char
AsciiStrCpyS (SmbiosString, BytesRemaining, StrElement->String);

// We have copied 10 chars
BytesCopied = StrElement->StringLen + 1;
BytesRemaining -= BytesCopied;
// Increment SmbiosString of 10 chars, so SmbiosString is pointing
// to an un-initialized char now and we can continue.
SmbiosString += BytesCopied;
"""

However, maybe we need to add an extra check/ASSERT for BytesRemaining before
writing the last NULL char.

+ return EFI_SUCCESS;
+}
+
+/** Initialise the string table and allocate memory for the string elements.
+
+ @param [IN] StrTable Pointer to the string table
+ @param [IN] MaxStringElements Maximum number of strings that the string
+ table can hold.
+
+ @return EFI_SUCCESS Success
+ @return EFI_INVALID_PARAMETER Invalid string table pointer
+ @return EFI_OUT_OF_RESOURCES Failed to allocate memory for string elements
+**/
+EFI_STATUS
+EFIAPI
+StringTableInitialize (
+ IN STRING_TABLE *CONST StrTable,
+ IN UINTN MaxStringElements
+ )
+{
+ STRING_ELEMENT *Elements;
+
+ if ((StrTable == NULL) || (MaxStringElements > MAX_UINT8)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ ZeroMem (StrTable, sizeof (STRING_TABLE));
+
+ Elements = (STRING_ELEMENT *)AllocateZeroPool (
+ sizeof (STRING_ELEMENT) * MaxStringElements
+ );
+ if (Elements == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ StrTable->Elements = Elements;
+ StrTable->MaxStringElements = (UINT8)MaxStringElements;
+ return EFI_SUCCESS;
+}
+
+/** Free memory allocated for the string elements in the string table.
+
+ @param [IN] StrTable Pointer to the string table
+
+ @return EFI_SUCCESS Success
+ @return EFI_INVALID_PARAMETER Invalid string table pointer or string elements
+**/
+EFI_STATUS
+EFIAPI
+StringTableFree (
+ IN STRING_TABLE *CONST StrTable
+ )
+{
+ if ((StrTable == NULL) || (StrTable->Elements == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ FreePool (StrTable->Elements);
+ ZeroMem (StrTable, sizeof (STRING_TABLE));
+ return EFI_SUCCESS;
+}
diff --git a/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf b/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf
new file mode 100644
index 0000000000000000000000000000000000000000..88624c50e3f3e930074222b6d686f75485538b26
--- /dev/null
+++ b/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf
@@ -0,0 +1,25 @@
+## @file
+# SMBIOS String Table Helper library.
+#
+# Copyright (c) 2022, Arm Limited. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+ INF_VERSION = 0x0001001B
+ BASE_NAME = SmbiosStringTableLib
+ FILE_GUID = 8C570DD8-531E-473F-85C6-9252746DBAC1
+ VERSION_STRING = 1.0
+ MODULE_TYPE = DXE_DRIVER
+ LIBRARY_CLASS = SmbiosStringTableLib
+
+[Sources]
+ SmbiosStringTableLib.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ DynamicTablesPkg/DynamicTablesPkg.dec
+
+[LibraryClasses]
+ BaseLib
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Hi Girish,

There are 2 cases that need handling. Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On 04/10/2022 04:01 am, Girish Mahadevan wrote:

Hello Sami,

My apologies for the late response. I had one question/comment.

Comment marked with [GM] inline.

Best Regards
Girish

On 9/12/2022 8:18 AM, Sami Mujawar wrote:

External email: Use caution opening links or attachments


The Section 6.1.3, SMBIOS specification version 3.6.0 describes the
handling of test strings in SMBIOS tables.

Test strings are added at the end of the formatted portion of the SMBIOS
structure and are referenced by index in the SMBIOS structure.

Therefore, introduce a SmbiosStringTableLib to simplify the publishing
of the string set.

SmbiosStringTableLib introduces a concept of string table which records
the references to the SMBIOS strings as they are added and returns an
string reference which is then assigned to the string field in the
formatted portion of the SMBIOS structure. Once all strings are added,
the library provides an interface to get the required size for the string
set. This allows sufficient memory to be allocated for the SMBIOS table.
The library also provides an interface to publish the string set in
accordance with the SMBIOS specification.

Example:
EFI_STATUS
BuildSmbiosType17Table () {
   STRING_TABLE         StrTable;
   UINT8                DevLocatorRef;
   UINT8                BankLocatorRef;
   SMBIOS_TABLE_TYPE17  *SmbiosRecord;
   CHAR8                *StringSet;
   ...

   // Initialize string table for 7 strings
   StringTableInitialize (&StrTable, 7);

   StringTableAddString (&StrTable, "SIMM 3", &DevLocatorRef);
   StringTableAddString (&StrTable, "Bank 0", &BankLocatorRef);
   ...

   SmbiosRecord = AllocateZeroPool (
                    sizeof (SMBIOS_TABLE_TYPE17) +
                      StringTableGetStringSetSize (&StrTable)
                    );
   ...
   SmbiosRecord->DeviceLocator = DevLocatorRef;
   SmbiosRecord->BankLocator = BankLocatorRef;
   ...
   // get the string set area
   StringSet = (CHAR8*)(SmbiosRecord + 1);

   // publish the string set
   StringTablePublishStringSet (
     &StrTable,
     StringSet,
     StringTableGetStringSetSize (&StrTable)
     );

   // free string table
   StringTableFree (&StrTable);

   return EFI_SUCCESS;
}

Signed-off-by: Sami Mujawar <sami.mujawar@...>
Cc: Alexei Fedorov <Alexei.Fedorov@...>
Cc: Pierre Gondois <pierre.gondois@...>
Cc: Girish Mahadevan <gmahadevan@...>
Cc: Jeff Brasen <jbrasen@...>
Cc: Ashish Singhal <ashishsingha@...>
Cc: Nick Ramirez <nramirez@...>
Cc: William Watson <wwatson@...>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
---
The changes can be seen at:
https://github.com/samimujawar/edk2/tree/2370_smbios_stringlib_v1

DynamicTablesPkg/DynamicTables.dsc.inc |   3 +-
DynamicTablesPkg/DynamicTablesPkg.dec |   5 +-
DynamicTablesPkg/DynamicTablesPkg.dsc |   3 +-
DynamicTablesPkg/Include/Library/SmbiosStringTableLib.h | 119 ++++++++++
DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.c | 227 ++++++++++++++++++++
DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf |  25 +++
  6 files changed, 379 insertions(+), 3 deletions(-)

diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc b/DynamicTablesPkg/DynamicTables.dsc.inc
index 3d4fa0c4c4b67d6770aee8705c80cc18d20c823a..d35acc1788f2ddc5e2b5fc3e4bedfd48251f7ec8 100644
--- a/DynamicTablesPkg/DynamicTables.dsc.inc
+++ b/DynamicTablesPkg/DynamicTables.dsc.inc
@@ -1,7 +1,7 @@
  ## @file
  #  Dsc include file for Dynamic Tables Framework.
  #
-#  Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
+#  Copyright (c) 2017 - 2022, Arm Limited. All rights reserved.<BR>
  #
  #  SPDX-License-Identifier: BSD-2-Clause-Patent
  #
@@ -18,6 +18,7 @@ [LibraryClasses.common]
SsdtPcieSupportLib|DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
SsdtSerialPortFixupLib|DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf
TableHelperLib|DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
+ SmbiosStringTableLib|DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf

  [Components.common]
    #
diff --git a/DynamicTablesPkg/DynamicTablesPkg.dec b/DynamicTablesPkg/DynamicTablesPkg.dec
index cc34c2bdd6ff9b5ca508961b3d0fe85ffbb73c12..2a79cfd4edebbdff05bee66fa01a17d68252e8a7 100644
--- a/DynamicTablesPkg/DynamicTablesPkg.dec
+++ b/DynamicTablesPkg/DynamicTablesPkg.dec
@@ -1,7 +1,7 @@
  ## @file
  # dec file for Dynamic Tables Framework.
  #
-# Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
+# Copyright (c) 2017 - 2022, Arm Limited. All rights reserved.<BR>
  #
  # SPDX-License-Identifier: BSD-2-Clause-Patent
  #
@@ -39,6 +39,9 @@ [LibraryClasses]
    ##  @libraryclass  Defines a set of helper methods.
    TableHelperLib|Include/Library/TableHelperLib.h

+  ##  @libraryclass  Defines a set of SMBIOS string helper methods.
+  SmbiosStringTableLib|Include/Library/SmbiosStringTableLib.h
+
  [Protocols]
    # Configuration Manager Protocol GUID
    gEdkiiConfigurationManagerProtocolGuid = { 0xd85a4835, 0x5a82, 0x4894, { 0xac, 0x2, 0x70, 0x6f, 0x43, 0xd5, 0x97, 0x8e } }
diff --git a/DynamicTablesPkg/DynamicTablesPkg.dsc b/DynamicTablesPkg/DynamicTablesPkg.dsc
index 07cc837552f587fe5bf9031e0061b0234e8698d4..bd5084a9008f040acdd16200ae8cdb23455ac101 100644
--- a/DynamicTablesPkg/DynamicTablesPkg.dsc
+++ b/DynamicTablesPkg/DynamicTablesPkg.dsc
@@ -2,7 +2,7 @@
  #  Dsc file for Dynamic Tables Framework.
  #
  #  Copyright (c) 2019, Linaro Limited. All rights reserved.<BR>
-#  Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.<BR>
+#  Copyright (c) 2019 - 2022, Arm Limited. All rights reserved.<BR>
  #
  #  SPDX-License-Identifier: BSD-2-Clause-Patent
  #
@@ -46,6 +46,7 @@ [Components.common]
DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtHwInfoParserLib.inf
DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
+ DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf

  [BuildOptions]
    *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
diff --git a/DynamicTablesPkg/Include/Library/SmbiosStringTableLib.h b/DynamicTablesPkg/Include/Library/SmbiosStringTableLib.h
new file mode 100644
index 0000000000000000000000000000000000000000..246d4d30ddf901640ea720c108e2971552ec6c96
--- /dev/null
+++ b/DynamicTablesPkg/Include/Library/SmbiosStringTableLib.h
@@ -0,0 +1,119 @@
+/** @file
+  SMBIOS String Table Helper library.
+
+  Copyright (c) 2022, Arm Limited. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef SMBIOS_STRING_TABLE_H_
+#define SMBIOS_STRING_TABLE_H_
+
+/** A structure representing a string in the string table.
+*/
+typedef struct StringElement {
+  /// Length of the string (does not include the NULL termination)
+  UINTN          StringLen;
+
+  /// Reference to the string
+  CONST CHAR8    *String;
+} STRING_ELEMENT;
+
+/** A structure representing a string table.
+*/
+typedef struct StringTable {
+  /// Count of strings in the table
+  UINT8             StrCount;
+
+  /// Total length of all strings in the table (does not include
+  // the NULL termination for each string)
+  UINTN             TotalStrLen;
+
+  /// Maximum string count
+  UINT8             MaxStringElements;
+
+  /// Pointer to the string table elements
+  STRING_ELEMENT    *Elements;
+} STRING_TABLE;
+
+/** Add a string to the string table
+
+  @param [IN]   StrTable  Pointer to the string table
+  @param [IN]   Str       Pointer to the string
+  @param [OUT]  StrRef    Optional pointer to retrieve the string field
+                          reference of the string in the string table
+
+  @return EFI_SUCCESS            Success
+  @return EFI_INVALID_PARAMETER  Invalid string table pointer
+  @return EFI_BUFFER_TOO_SMALL   Insufficient space to add string
+**/
+EFI_STATUS
+EFIAPI
+StringTableAddString (
+  IN        STRING_TABLE *CONST  StrTable,
+  IN  CONST CHAR8                *Str,
+  OUT       UINT8                *StrRef      OPTIONAL
+  );
+
+/** Returns the total size required to publish the strings to the SMBIOS
+    string area.
+
+  @param [IN] StrTable              Pointer to the string table
+
+  @return Total size required to publish the strings in the SMBIOS string area.
+**/
+UINTN
+EFIAPI
+StringTableGetStringSetSize (
+  IN  STRING_TABLE *CONST  StrTable
+  );
+
+/** Iterate through the string table and publish the strings in the SMBIOS
+    string area.
+
+  @param [IN] StrTable              Pointer to the string table
+  @param [IN] SmbiosStringAreaStart Start address of the SMBIOS string area.
+  @param [IN] SmbiosStringAreaSize  Size of the SMBIOS string area.
+
+  @return EFI_SUCCESS            Success
+  @return EFI_INVALID_PARAMETER  Invalid string table pointer
+  @return EFI_BUFFER_TOO_SMALL   Insufficient space to publish strings
+**/
+EFI_STATUS
+EFIAPI
+StringTablePublishStringSet (
+  IN        STRING_TABLE  *CONST  StrTable,
+  IN        CHAR8         *CONST  SmbiosStringAreaStart,
+  IN  CONST UINTN                 SmbiosStringAreaSize
+  );
+
+/** Initialise the string table and allocate memory for the string elements.
+
+  @param [IN] StrTable           Pointer to the string table
+  @param [IN] MaxStringElements  Maximum number of strings that the string
+                                 table can hold.
+
+  @return EFI_SUCCESS            Success
+  @return EFI_INVALID_PARAMETER  Invalid string table pointer
+  @return EFI_OUT_OF_RESOURCES   Failed to allocate memory for string elements
+**/
+EFI_STATUS
+EFIAPI
+StringTableInitialize (
+  IN STRING_TABLE *CONST  StrTable,
+  IN UINTN                MaxStringElements
+  );
+
+/** Free memory allocated for the string elements in the string table.
+
+  @param [IN] StrTable           Pointer to the string table
+
+  @return EFI_SUCCESS            Success
+  @return EFI_INVALID_PARAMETER  Invalid string table pointer or string elements
+**/
+EFI_STATUS
+EFIAPI
+StringTableFree (
+  IN STRING_TABLE *CONST  StrTable
+  );
+
+#endif // SMBIOS_STRING_TABLE_H_
diff --git a/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.c b/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.c
new file mode 100644
index 0000000000000000000000000000000000000000..bd186f27fe1613cf819b6600e62960c003a796d6
--- /dev/null
+++ b/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.c
@@ -0,0 +1,227 @@
+/** @file
+  SMBIOS String Table Helper
+
+  Copyright (c) 2022, Arm Limited. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Reference(s):
+  - DSP0134 - SMBIOS Specification Version 3.6.0, 2022-06-17
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/SmbiosStringTableLib.h>
+
+/** Add a string to the string table
+
+  @param [IN]   StrTable  Pointer to the string table
+  @param [IN]   Str       Pointer to the string
+  @param [OUT]  StrRef    Optional pointer to retrieve the string field
+                          reference of the string in the string table
+
+  @return EFI_SUCCESS            Success
+  @return EFI_INVALID_PARAMETER  Invalid string table pointer
+  @return EFI_BUFFER_TOO_SMALL   Insufficient space to add string
+**/
+EFI_STATUS
+EFIAPI
+StringTableAddString (
+  IN        STRING_TABLE *CONST  StrTable,
+  IN  CONST CHAR8                *Str,
+  OUT       UINT8                *StrRef      OPTIONAL
+  )
+{
+  UINTN           StrLength;
+  STRING_ELEMENT  *StrElement;
+
+  if ((StrTable == NULL) || (Str == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (StrTable->StrCount >= StrTable->MaxStringElements) {
+    return EFI_BUFFER_TOO_SMALL;
+  }
+
+  StrLength = AsciiStrLen (Str);
+  if (StrLength == 0) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  // Update the string element
+  StrElement            = &StrTable->Elements[StrTable->StrCount];
+  StrElement->StringLen = StrLength;
+  StrElement->String    = Str;
+
+  // Update String table information
+  StrTable->TotalStrLen += StrLength;
+  StrTable->StrCount++;
+
+  // Return the index of the string in the string table if requested
+  if (StrRef != NULL) {
+    // Note: SMBIOS string field references start at 1. So, return the
+    // StrCount as the string refrence after it is updated.
+    *StrRef = StrTable->StrCount;
+  }
+
+  return EFI_SUCCESS;
+}
+
+/** Returns the total size required to publish the strings to the SMBIOS
+    string area.
+
+  @param [IN] StrTable              Pointer to the string table
+
+  @return Total size required to publish the strings in the SMBIOS string area.
+**/
+UINTN
+EFIAPI
+StringTableGetStringSetSize (
+  IN  STRING_TABLE *CONST  StrTable
+  )
+{
+  if (StrTable == NULL) {
+    ASSERT (0);
+    return 0;
+  }
+
+  // See Section 6.1.3 Text strings, SMBIOS Specification Version 3.6.0
+  // - If the formatted portion of the structure contains string-reference
+  //   fields and all the string fields are set to 0 (no string references),
+  //   the formatted section of the structure is followed by two null (00h)
+  //   BYTES.
+  // - Each string is terminated with a null (00h) BYTE
+  // - and the set of strings is terminated with an additional null (00h) BYTE.
+
+  // Therefore, if string count = 0, return 2
+  // if string count > 0, the string set size =
+  // StrTable->TotalStrLen (total length of the strings in the string table)
+  // + StrTable->StrCount (add string count to include '\0' for each string)
+  // +1 (an additional '\0' is required at the end of the string set).
+  return (StrTable->StrCount == 0) ? 2 :
+         (StrTable->TotalStrLen + StrTable->StrCount + 1);
+}
+
+/** Iterate through the string table and publish the strings in the SMBIOS
+    string area.
+
+  @param [IN] StrTable              Pointer to the string table
+  @param [IN] SmbiosStringAreaStart Start address of the SMBIOS string area.
+  @param [IN] SmbiosStringAreaSize  Size of the SMBIOS string area.
+
+  @return EFI_SUCCESS            Success
+  @return EFI_INVALID_PARAMETER  Invalid string table pointer
+  @return EFI_BUFFER_TOO_SMALL   Insufficient space to publish strings
+**/
+EFI_STATUS
+EFIAPI
+StringTablePublishStringSet (
+  IN        STRING_TABLE  *CONST  StrTable,
+  IN        CHAR8         *CONST  SmbiosStringAreaStart,
+  IN  CONST UINTN                 SmbiosStringAreaSize
+  )
+{
+  UINT8           Index;
+  STRING_ELEMENT  *StrElement;
+  CHAR8           *SmbiosString;
+  UINTN           BytesRemaining;
+  UINTN           BytesCopied;
+
+  if ((StrTable == NULL) || (SmbiosStringAreaStart == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (SmbiosStringAreaSize < StringTableGetStringSetSize (StrTable)) {
+    return EFI_BUFFER_TOO_SMALL;
+  }
+
+  SmbiosString   = SmbiosStringAreaStart;
+  BytesRemaining = SmbiosStringAreaSize;

[SAMI] SmbiosString points to the startof the sting storage area.

+
+  if (StrTable->StrCount == 0) {
+    // See Section 6.1.3 Text strings, SMBIOS Specification Version 3.6.0
+    // If the formatted portion of the structure contains string-reference
+    // fields and all the string fields are set to 0 (no string references),
+    // the formatted section of the structure is followed by two null (00h)
+    // BYTES.
+    *SmbiosString++ = '\0';

[SAMI] Case 1: SmbiosString[0] is set to '\0' and the pointer is incremented. So now it points to SmbiosString[1] which will be updated with the String table null terminator before the function returns.

+  } else {
+    for (Index = 0; Index < StrTable->StrCount; Index++) {
+      StrElement = &StrTable->Elements[Index];
+      AsciiStrCpyS (SmbiosString, BytesRemaining, StrElement->String);
+
+      // See Section 6.1.3 Text strings, SMBIOS Specification Version 3.6.0
+      // - Each string is terminated with a null (00h) BYTE
+      // Bytes Copied = String length + 1 for the string NULL terminator.
+      BytesCopied     = StrElement->StringLen + 1;
+      BytesRemaining -= BytesCopied;
+      SmbiosString   += BytesCopied;

[SAMI] Case 2: AsciiStrCpyS() copies the string including the terrminating null character and BytesCopied is incremented accordingly. SmbiosString is then incremented by BytesCopied. So, SmbiosString should point to the byte where the next string starts or will be updated with the String table null terminator before the function returns.

+    }
+  }
+
+  // See Section 6.1.3 Text strings, SMBIOS Specification Version 3.6.0
+  // - the set of strings is terminated with an additional null (00h) BYTE.
+  *SmbiosString = '\0';

[GM] Shouldn't you advance the SmbiosString pointer by one more ? After the loop isn't SmbiosString going to be at the NULL char of the last string ? And we're trying to add one more NULL character ?
Should it be:
SmbiosString++;

[SAMI] I don't think this increment is required for the reasons explained above in Case 1 & Case 2. However, please let me know if I have missed something.

*SmbiosString = '\0';

+  return EFI_SUCCESS;
+}
+
+/** Initialise the string table and allocate memory for the string elements.
+
+  @param [IN] StrTable           Pointer to the string table
+  @param [IN] MaxStringElements  Maximum number of strings that the string
+                                 table can hold.
+
+  @return EFI_SUCCESS            Success
+  @return EFI_INVALID_PARAMETER  Invalid string table pointer
+  @return EFI_OUT_OF_RESOURCES   Failed to allocate memory for string elements
+**/
+EFI_STATUS
+EFIAPI
+StringTableInitialize (
+  IN STRING_TABLE *CONST  StrTable,
+  IN UINTN                MaxStringElements
+  )
+{
+  STRING_ELEMENT  *Elements;
+
+  if ((StrTable == NULL) || (MaxStringElements > MAX_UINT8)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  ZeroMem (StrTable, sizeof (STRING_TABLE));
+
+  Elements = (STRING_ELEMENT *)AllocateZeroPool (
+                                 sizeof (STRING_ELEMENT) * MaxStringElements
+                                 );
+  if (Elements == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  StrTable->Elements          = Elements;
+  StrTable->MaxStringElements = (UINT8)MaxStringElements;
+  return EFI_SUCCESS;
+}
+
+/** Free memory allocated for the string elements in the string table.
+
+  @param [IN] StrTable           Pointer to the string table
+
+  @return EFI_SUCCESS            Success
+  @return EFI_INVALID_PARAMETER  Invalid string table pointer or string elements
+**/
+EFI_STATUS
+EFIAPI
+StringTableFree (
+  IN STRING_TABLE *CONST  StrTable
+  )
+{
+  if ((StrTable == NULL) || (StrTable->Elements == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  FreePool (StrTable->Elements);
+  ZeroMem (StrTable, sizeof (STRING_TABLE));
+  return EFI_SUCCESS;
+}
diff --git a/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf b/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf
new file mode 100644
index 0000000000000000000000000000000000000000..88624c50e3f3e930074222b6d686f75485538b26
--- /dev/null
+++ b/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf
@@ -0,0 +1,25 @@
+## @file
+#  SMBIOS String Table Helper library.
+#
+#  Copyright (c) 2022, Arm Limited. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION    = 0x0001001B
+  BASE_NAME      = SmbiosStringTableLib
+  FILE_GUID      = 8C570DD8-531E-473F-85C6-9252746DBAC1
+  VERSION_STRING = 1.0
+  MODULE_TYPE    = DXE_DRIVER
+  LIBRARY_CLASS  = SmbiosStringTableLib
+
+[Sources]
+  SmbiosStringTableLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  DynamicTablesPkg/DynamicTablesPkg.dec
+
+[LibraryClasses]
+  BaseLib
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Changed 4 incorrect instances of "RANDOM_NAME_PROTOCOL" in
RandomNumberBBTestConformance and RandomNumberBBTestFunction
to "RANDOM_NUMBER_PROTOCOL".

Cc: G Edhaya Chandran <Edhaya.Chandran@...>
Cc: Barton Gao <gaojie@...>
Cc: Carolyn Gjertsen <Carolyn.Gjertsen@...>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Cc: Eric Jin <eric.jin@...>
Cc: Arvin Chen <arvinx.chen@...>
Cc: Supreeth Venkatesh <Supreeth.Venkatesh@...>

Signed-off-by: Sam Kaynor <sam.kaynor@...>
---
uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/RandomNumber/BlackBoxTest/Ran=
domNumberBBTestConformance.c | 4 ++--
uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/RandomNumber/BlackBoxTest/Ran=
domNumberBBTestFunction.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/RandomNumber/Blac=
kBoxTest/RandomNumberBBTestConformance.c b/uefi-sct/SctPkg/TestCase/UEFI/=
EFI/Protocol/RandomNumber/BlackBoxTest/RandomNumberBBTestConformance.c
index 2738a4899457..364aaca925e0 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/RandomNumber/BlackBoxTes=
t/RandomNumberBBTestConformance.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/RandomNumber/BlackBoxTes=
t/RandomNumberBBTestConformance.c
@@ -160,7 +160,7 @@ BBTestGetInfoConformanceTestCheckpoint1 (
StandardLib,
EFI_TEST_ASSERTION_WARNING,
gConformanceTestAssertionGuid001,
- L"RANDOM_NAME_PROTOCOL.GetInfo - GetInfo() is not sup=
ported or EFI_DEVICE_ERROR",
+ L"RANDOM_NUMBER_PROTOCOL.GetInfo - GetInfo() is not s=
upported or EFI_DEVICE_ERROR",
L"%a:%d: Status - %r",
__FILE__,
(UINTN)__LINE__,
@@ -180,7 +180,7 @@ BBTestGetInfoConformanceTestCheckpoint1 (
StandardLib,
AssertionType,
gConformanceTestAssertionGuid001,
- L"RANDOM_NAME_PROTOCOL.GetInfo - GetInfo() returns EFI_=
BUFFER_TOO_SMALL with small RNGAlgorithmListSize and returns valid size",
+ L"RANDOM_NUMBER_PROTOCOL.GetInfo - GetInfo() returns EF=
I_BUFFER_TOO_SMALL with small RNGAlgorithmListSize and returns valid size=
",
L"%a:%d: Status - %r, RNGAlgorithmListSize - %d",
__FILE__,
(UINTN)__LINE__,
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/RandomNumber/Blac=
kBoxTest/RandomNumberBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI=
/Protocol/RandomNumber/BlackBoxTest/RandomNumberBBTestFunction.c
index 3d41085d2243..0ba003449dc6 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/RandomNumber/BlackBoxTes=
t/RandomNumberBBTestFunction.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/RandomNumber/BlackBoxTes=
t/RandomNumberBBTestFunction.c
@@ -188,7 +188,7 @@ BBTestGetInfoFunctionTestCheckpoint1 (
StandardLib,
EFI_TEST_ASSERTION_FAILED,
gTestGenericFailureGuid,
- L"RANDOM_NAME_PROTOCOL.GetInfo - GetInfo() is not sup=
ported or EFI_DEVICE_ERROR",
+ L"RANDOM_NUMBER_PROTOCOL.GetInfo - GetInfo() is not s=
upported or EFI_DEVICE_ERROR",
L"%a:%d: Status - %r",
__FILE__,
(UINTN)__LINE__,
@@ -201,7 +201,7 @@ BBTestGetInfoFunctionTestCheckpoint1 (
StandardLib,
EFI_TEST_ASSERTION_FAILED,
gTestGenericFailureGuid,
- L"RANDOM_NAME_PROTOCOL.GetInfo - GetInfo() could not =
return the correct RNGAlgorithmListSize",
+ L"RANDOM_NUMBER_PROTOCOL.GetInfo - GetInfo() could no=
t return the correct RNGAlgorithmListSize",
L"%a:%d: Status - %r",
__FILE__,
(UINTN)__LINE__,
--=20
2.34.1