Date   

Re: [PATCH 1/4] UefiCpuPkg/CpuExceptionHandler: Make XCODE5 changes toolchain specific

Laszlo Ersek
 

Hi Tom,

On 05/01/20 22:17, Lendacky, Thomas wrote:
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2340

Commit 2db0ccc2d7fe ("UefiCpuPkg: Update CpuExceptionHandlerLib pass
XCODE5 tool chain") introduced binary patching into the exception handling
support. CPU exception handling is allowed during SEC and this results in
binary patching of flash, which should not be done.

Separate the changes from commit 2db0ccc2d7fe into an XCODE5 toolchain
specific file, Xcode5ExceptionHandlerAsm.nasm, and create new INF files
for an XCODE5 version of CpuExceptionHandlerLib. Update the UefiCpuPkg.dsc
file to use the new files when the XCODE5 toolchain is used.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
UefiCpuPkg/UefiCpuPkg.dsc | 23 +
.../Xcode5DxeCpuExceptionHandlerLib.inf | 64 +++
.../Xcode5PeiCpuExceptionHandlerLib.inf | 63 +++
.../Xcode5SecPeiCpuExceptionHandlerLib.inf | 55 +++
.../Xcode5SmmCpuExceptionHandlerLib.inf | 59 +++
I don't think that paralleling all the existent INF files is necessary
for XCODE5.

The binary patching is a problem when the UEFI module containing the
self-patching CpuExceptionHandlerLib instance executes in-place from
flash. That applies to: (a) SEC modules, (b) PEI modules that do *not*
have a DEPEX on "gEfiPeiMemoryDiscoveredPpiGuid". (PEIMs that do have a
DEPEX on that PPI GUID are only dispatched after the permanent PEI RAM
has been discovered / published, so they run out of normal RAM.)

Reviewing the existent INF files, we have:

- DxeCpuExceptionHandlerLib.inf: for DXE_CORE, DXE_DRIVER,
UEFI_APPLICATION modules. Self-patching is fine.

- SmmCpuExceptionHandlerLib.inf: for DXE_SMM_DRIVER modules.
Self-patching is fine.

- SecPeiCpuExceptionHandlerLib.inf: SEC is listed explicitly, so here we
certainly need an alternative.

- PeiCpuExceptionHandlerLib.inf: unfortunately, the differences of this
library instance with "SecPeiCpuExceptionHandlerLib.inf" is not obvious;
only SEC's absence is easily visible.

If we look at the commit that introduced this lib instance
(a81abf161666, "UefiCpuPkg/ExceptionLib: Import
PeiCpuExceptionHandlerLib module", 2016-06-01), we find:

This module could be linked by CpuMpPei driver to handle reserved vector list
and provide spin lock for BSP/APs to prevent dump message corrupted.
So the library was added explicitly for CpuMpPei's sake -- which looks
promising, because CpuMpPei certainly depends on
"gEfiPeiMemoryDiscoveredPpiGuid", as it needs a bunch of RAM for
offering the multi-processing PPI. That suggests the self-patching is OK
in "PeiCpuExceptionHandlerLib.inf" too.

The CpuMpPei DEPEX in question was replaced with a PPI notify callback
in commit 0a0d5296e448 ("UefiCpuPkg/CpuMpPei: support stack guard
feature", 2018-09-10). This would be a problem if the self-patching in
the PeiCpuExceptionHandlerLib instance occurred in the library
constructor, because the CpuMpPei can now actually be dispatched before
permanent PEI RAM is available -- and the constructor would run
immediately.

Luckily, the lib instance has no CONSTRUCTOR at all, and CpuMpPei calls
InitializeCpuExceptionHandlers() explicitly in InitializeCpuMpWorker(),
which is the PPI notify in question. (And per
<https://bugzilla.tianocore.org/show_bug.cgi?id=2340#c0>, the
self-patching occurs in InitializeCpuExceptionHandlers().)

Therefore, having two variants of PeiCpuExceptionHandlerLib.inf is also
unnecessary.

(1) We only need two variants for "SecPeiCpuExceptionHandlerLib.inf", in
my opinion.

(Note: if we check OVMF, we see that PeiCpuExceptionHandlerLib.inf is
used universally for PEIMs. That's because OVMF is special -- its PEI
phase runs entirely out of RAM. See also commit f0e6a56a9a2f, "OvmfPkg:
include UefiCpuPkg/CpuMpPei", 2016-07-15.)

--*--

With this patch applied:

$ diff -u \
SecPeiCpuExceptionHandlerLib.inf \
Xcode5SecPeiCpuExceptionHandlerLib.inf

--- SecPeiCpuExceptionHandlerLib.inf 2020-05-05 18:36:12.813156743 +0200
+++ Xcode5SecPeiCpuExceptionHandlerLib.inf 2020-05-05 23:25:24.578572971 +0200
@@ -8,7 +8,7 @@

[Defines]
INF_VERSION = 0x00010005
- BASE_NAME = SecPeiCpuExceptionHandlerLib
+ BASE_NAME = Xcode5SecPeiCpuExceptionHandlerLib
OK

MODULE_UNI_FILE = SecPeiCpuExceptionHandlerLib.uni
(2) We'll need a separate UNI file here -- also we should customize the
file-top comment in the INF file -- that explains the difference between
the XCODE5 and non-XCODE5 variants, briefly.

FILE_GUID = CA4BBC99-DFC6-4234-B553-8B6586B7B113
(3) Please generate a new FILE_GUID with "uuidgen".

MODULE_TYPE = PEIM
@@ -26,16 +26,20 @@
Ia32/ExceptionTssEntryAsm.nasm
Ia32/ArchExceptionHandler.c
Ia32/ArchInterruptDefs.h
+ Ia32/ArchAMDSevVcHandler.c
(4) Even though the blurb says that this series is based on edk2 commit
e54310451f1a, some SEV-ES specific parts remain in this patch, and
should be eliminated. The first example is above.


[Sources.X64]
- X64/ExceptionHandlerAsm.nasm
+ X64/Xcode5ExceptionHandlerAsm.nasm
X64/ArchExceptionHandler.c
X64/ArchInterruptDefs.h
+ X64/ArchAMDSevVcHandler.c
(5) Another SEV-ES change.


[Sources.common]
CpuExceptionCommon.h
CpuExceptionCommon.c
SecPeiCpuException.c
+ AMDSevVcHandler.c
+ AMDSevVcCommon.h
(6) ditto


[Packages]
MdePkg/MdePkg.dec
@@ -48,3 +52,4 @@
PrintLib
LocalApicLib
PeCoffGetEntryPointLib
+ VmgExitLib
(7) ditto

Furthermore:

$ diff -u \
ExceptionHandlerAsm.nasm \
Xcode5ExceptionHandlerAsm.nasm

--- ExceptionHandlerAsm.nasm 2020-05-05 23:26:30.941784203 +0200
+++ Xcode5ExceptionHandlerAsm.nasm 2020-05-05 23:25:24.578572971 +0200
@@ -18,6 +18,8 @@
; CommonExceptionHandler()
;

+%define VC_EXCEPTION 29
+
extern ASM_PFX(mErrorCodeFlag) ; Error code flags for exceptions
extern ASM_PFX(mDoFarReturnFlag) ; Do far return flag
extern ASM_PFX(CommonExceptionHandler)
@@ -225,6 +227,9 @@
push rax

;; UINT64 Dr0, Dr1, Dr2, Dr3, Dr6, Dr7;
+ cmp qword [rbp + 8], VC_EXCEPTION
+ je VcDebugRegs ; For SEV-ES (#VC) Debug registers ignored
+
mov rax, dr7
push rax
mov rax, dr6
@@ -237,7 +242,19 @@
push rax
mov rax, dr0
push rax
+ jmp DrFinish
+
+VcDebugRegs:
+;; UINT64 Dr0, Dr1, Dr2, Dr3, Dr6, Dr7 are skipped for #VC to avoid exception recursion
+ xor rax, rax
+ push rax
+ push rax
+ push rax
+ push rax
+ push rax
+ push rax

+DrFinish:
;; FX_SAVE_STATE_X64 FxSaveState;
sub rsp, 512
mov rdi, rsp
(8) All of these should be removed -- they should be part of your SEV-ES
series, on top of this set.

Thanks,
Laszlo


Re: [PATCH v6 00/12] OvmfPkg: Support booting from Fusion-MPT SCSI controllers

Laszlo Ersek
 

On 05/04/20 23:05, Nikita Leshenko wrote:
This series adds driver support for:
- LSI53C1030
- SAS1068
- SAS1068E

These controllers are widely supported by QEMU, VirtualBox and VMWare.
This work is part of the more general agenda of enhancing OVMF boot
device support to have feature parity with SeaBIOS.

I pushed a copy of these patches to
https://github.com/nikital/edk2/tree/mptscsi_v6
Previous versions:
https://github.com/nikital/edk2/tree/mptscsi_v5
https://github.com/nikital/edk2/tree/mptscsi_v4
https://github.com/nikital/edk2/tree/mptscsi_v3
https://github.com/nikital/edk2/tree/mptscsi (v2)

v5->v6:
- Use for "other" error ReportHostAdapterError
- Add alignment for init request
- Add PcdLib
- Use RShiftU64
- Use Pages for FreeBuffer
- Use STATIC_ASSERT for ReplyWord
- Code convention fixes
Github.com pull request:

https://github.com/tianocore/edk2/pull/582

Commit range: f159102a130f..c635a56384bf.

Thanks!
Laszlo


v4->v5:
- Sort maintainers and protocols
- Fix bug when restoring PCI attributes (Use Set instead of Enable)
- Separate packed structs and aligned unions
- STATIC_ASSERT for init request size
- Add support for multiple targets from the beginning
- Use PCI_BAR_IDX0 in door bell
- Code convention improvements
- Add DEBUG_VERBOSE message seen in PvScsiExitBoot
- Return EFI_INVALID_PARAMETER in GetNextTarget
- STATIC_ASSERT for MaxTarget
- Move PCD near PvScsi
- A lot of fixes for PassThru (comments, error handling, casting)
- Support 64-bit DMA

v3->v4:
- Add ExitBootServices
- Rework error handling in PassThru
- SPDX license
- Made compilation conditional
- Squash GetTargetLun and BuildDevicePath commits
- Added #include <Uefi/UefiSpec.h>
- Use PCI_BAR_IDX0
- Code convention improvements

v2->v3:
- Change error handling style
- Add comments about target size and zero unused target bytes
- Remove internal Reviewed-by
- Fix problems reported by PatchCheck.py
- Use SetupGit.py

v1->v2:
- Map() DMAed buffers
- Fixed various code convention issues
- Newer debug macros
- Updated INF version

Thanks,
Nikita

Nikita Leshenko (12):
OvmfPkg/MptScsiDxe: Create empty driver
OvmfPkg/MptScsiDxe: Install DriverBinding Protocol
OvmfPkg/MptScsiDxe: Report name of driver
OvmfPkg/MptScsiDxe: Probe PCI devices and look for MptScsi
OvmfPkg/MptScsiDxe: Install stubbed EXT_SCSI_PASS_THRU
OvmfPkg/MptScsiDxe: Report targets and one LUN
OvmfPkg/MptScsiDxe: Build and decode DevicePath
OvmfPkg/MptScsiDxe: Open PciIo protocol for later use
OvmfPkg/MptScsiDxe: Set and restore PCI attributes
OvmfPkg/MptScsiDxe: Initialize hardware
OvmfPkg/MptScsiDxe: Implement the PassThru method
OvmfPkg/MptScsiDxe: Reset device on ExitBootServices()

Maintainers.txt | 3 +-
.../Include/IndustryStandard/FusionMptScsi.h | 160 +++
OvmfPkg/MptScsiDxe/MptScsi.c | 1211 +++++++++++++++++
OvmfPkg/MptScsiDxe/MptScsiDxe.inf | 44 +
OvmfPkg/OvmfPkg.dec | 7 +
OvmfPkg/OvmfPkgIa32.dsc | 4 +
OvmfPkg/OvmfPkgIa32.fdf | 3 +
OvmfPkg/OvmfPkgIa32X64.dsc | 4 +
OvmfPkg/OvmfPkgIa32X64.fdf | 3 +
OvmfPkg/OvmfPkgX64.dsc | 4 +
OvmfPkg/OvmfPkgX64.fdf | 3 +
11 files changed, 1445 insertions(+), 1 deletion(-)
create mode 100644 OvmfPkg/Include/IndustryStandard/FusionMptScsi.h
create mode 100644 OvmfPkg/MptScsiDxe/MptScsi.c
create mode 100644 OvmfPkg/MptScsiDxe/MptScsiDxe.inf


Re: [PATCH v6 11/12] OvmfPkg/MptScsiDxe: Implement the PassThru method

Laszlo Ersek
 

On 05/04/20 23:06, Nikita Leshenko wrote:
Machines should be able to boot after this commit. Tested with different
Linux distributions (Ubuntu, CentOS) and different Windows
versions (Windows 7, Windows 10, Server 2016).

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2390
Signed-off-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
.../Include/IndustryStandard/FusionMptScsi.h | 9 +
OvmfPkg/MptScsiDxe/MptScsi.c | 409 +++++++++++++++++-
OvmfPkg/MptScsiDxe/MptScsiDxe.inf | 4 +
OvmfPkg/OvmfPkg.dec | 3 +
4 files changed, 423 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/Include/IndustryStandard/FusionMptScsi.h b/OvmfPkg/Include/IndustryStandard/FusionMptScsi.h
index 655d629d902e..99778d1537da 100644
--- a/OvmfPkg/Include/IndustryStandard/FusionMptScsi.h
+++ b/OvmfPkg/Include/IndustryStandard/FusionMptScsi.h
@@ -44,6 +44,15 @@

#define MPT_IOC_WHOINIT_ROM_BIOS 0x02

+#define MPT_SCSIIO_REQUEST_CONTROL_TXDIR_NONE (0x00 << 24)
+#define MPT_SCSIIO_REQUEST_CONTROL_TXDIR_WRITE (0x01 << 24)
+#define MPT_SCSIIO_REQUEST_CONTROL_TXDIR_READ (0x02 << 24)
+
+#define MPT_SCSI_IOCSTATUS_SUCCESS 0x0000
+#define MPT_SCSI_IOCSTATUS_DEVICE_NOT_THERE 0x0043
+#define MPT_SCSI_IOCSTATUS_DATA_OVERRUN 0x0044
+#define MPT_SCSI_IOCSTATUS_DATA_UNDERRUN 0x0045
+
//
// Device structures
//
diff --git a/OvmfPkg/MptScsiDxe/MptScsi.c b/OvmfPkg/MptScsiDxe/MptScsi.c
index 2cc69b88dab3..842c5a1b5921 100644
--- a/OvmfPkg/MptScsiDxe/MptScsi.c
+++ b/OvmfPkg/MptScsiDxe/MptScsi.c
@@ -11,6 +11,7 @@

#include <IndustryStandard/FusionMptScsi.h>
#include <IndustryStandard/Pci.h>
+#include <Library/BaseLib.h>
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
#include <Library/MemoryAllocationLib.h>
@@ -18,6 +19,7 @@
#include <Library/UefiBootServicesTableLib.h>
#include <Library/UefiLib.h>
#include <Protocol/PciIo.h>
+#include <Protocol/PciRootBridgeIo.h>
#include <Protocol/ScsiPassThruExt.h>
#include <Uefi/UefiSpec.h>

@@ -31,19 +33,50 @@
// Runtime Structures
//

+typedef struct {
+ MPT_SCSI_REQUEST_ALIGNED IoRequest;
+ MPT_SCSI_IO_REPLY_ALIGNED IoReply;
+ //
+ // As EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET.SenseDataLength is defined
+ // as UINT8, defining here SenseData size to MAX_UINT8 will guarantee it
+ // cannot overflow when passed to device.
+ //
+ UINT8 Sense[MAX_UINT8];
+ //
+ // This size of the data is arbitrarily chosen.
+ // It seems to be sufficient for all I/O requests sent through
+ // EFI_SCSI_PASS_THRU_PROTOCOL.PassThru() for common boot scenarios.
+ //
+ UINT8 Data[0x2000];
+} MPT_SCSI_DMA_BUFFER;
+
#define MPT_SCSI_DEV_SIGNATURE SIGNATURE_32 ('M','P','T','S')
typedef struct {
UINT32 Signature;
EFI_EXT_SCSI_PASS_THRU_PROTOCOL PassThru;
EFI_EXT_SCSI_PASS_THRU_MODE PassThruMode;
UINT8 MaxTarget;
+ UINT32 StallPerPollUsec;
EFI_PCI_IO_PROTOCOL *PciIo;
UINT64 OriginalPciAttributes;
+ MPT_SCSI_DMA_BUFFER *Dma;
+ EFI_PHYSICAL_ADDRESS DmaPhysical;
+ VOID *DmaMapping;
+ BOOLEAN IoReplyEnqueued;
} MPT_SCSI_DEV;

#define MPT_SCSI_FROM_PASS_THRU(PassThruPtr) \
CR (PassThruPtr, MPT_SCSI_DEV, PassThru, MPT_SCSI_DEV_SIGNATURE)

+#define MPT_SCSI_DMA_ADDR(Dev, MemberName) \
+ (Dev->DmaPhysical + OFFSET_OF (MPT_SCSI_DMA_BUFFER, MemberName))
+
+#define MPT_SCSI_DMA_ADDR_HIGH(Dev, MemberName) \
+ ((UINT32)RShiftU64 (MPT_SCSI_DMA_ADDR (Dev, MemberName), 32))
(1) One too many space characters before "32", but I'll fix this up for you.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks,
Laszlo


+
+#define MPT_SCSI_DMA_ADDR_LOW(Dev, MemberName) \
+ ((UINT32)MPT_SCSI_DMA_ADDR (Dev, MemberName))
+
//
// Hardware functions
//
@@ -165,6 +198,9 @@ MptScsiInit (
);
Req->MaxDevices = Dev->MaxTarget + 1;
Req->MaxBuses = 1;
+ Req->ReplyFrameSize = sizeof Dev->Dma->IoReply.Data;
+ Req->HostMfaHighAddr = MPT_SCSI_DMA_ADDR_HIGH (Dev, IoRequest);
+ Req->SenseBufferHighAddr = MPT_SCSI_DMA_ADDR_HIGH (Dev, Sense);

//
// Send controller init through doorbell
@@ -230,6 +266,288 @@ MptScsiInit (
return EFI_SUCCESS;
}

+STATIC
+EFI_STATUS
+ReportHostAdapterError (
+ OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet
+ )
+{
+ DEBUG ((DEBUG_ERROR, "%a: fatal error in scsi request\n", __FUNCTION__));
+ Packet->InTransferLength = 0;
+ Packet->OutTransferLength = 0;
+ Packet->SenseDataLength = 0;
+ Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OTHER;
+ Packet->TargetStatus = EFI_EXT_SCSI_STATUS_TARGET_TASK_ABORTED;
+ return EFI_DEVICE_ERROR;
+}
+
+STATIC
+EFI_STATUS
+ReportHostAdapterOverrunError (
+ OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet
+ )
+{
+ Packet->SenseDataLength = 0;
+ Packet->HostAdapterStatus =
+ EFI_EXT_SCSI_STATUS_HOST_ADAPTER_DATA_OVERRUN_UNDERRUN;
+ Packet->TargetStatus = EFI_EXT_SCSI_STATUS_TARGET_GOOD;
+ return EFI_BAD_BUFFER_SIZE;
+}
+
+STATIC
+EFI_STATUS
+MptScsiPopulateRequest (
+ IN MPT_SCSI_DEV *Dev,
+ IN UINT8 Target,
+ IN UINT64 Lun,
+ IN OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet
+ )
+{
+ MPT_SCSI_REQUEST_WITH_SG *Request;
+
+ Request = &Dev->Dma->IoRequest.Data;
+
+ if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_BIDIRECTIONAL ||
+ (Packet->InTransferLength > 0 && Packet->OutTransferLength > 0) ||
+ Packet->CdbLength > sizeof (Request->Header.Cdb)) {
+ return EFI_UNSUPPORTED;
+ }
+
+ if (Target > Dev->MaxTarget || Lun > 0 ||
+ Packet->DataDirection > EFI_EXT_SCSI_DATA_DIRECTION_BIDIRECTIONAL ||
+ //
+ // Trying to receive, but destination pointer is NULL, or contradicting
+ // transfer direction
+ //
+ (Packet->InTransferLength > 0 &&
+ (Packet->InDataBuffer == NULL ||
+ Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_WRITE
+ )
+ ) ||
+
+ //
+ // Trying to send, but source pointer is NULL, or contradicting transfer
+ // direction
+ //
+ (Packet->OutTransferLength > 0 &&
+ (Packet->OutDataBuffer == NULL ||
+ Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ
+ )
+ )
+ ) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (Packet->InTransferLength > sizeof (Dev->Dma->Data)) {
+ Packet->InTransferLength = sizeof (Dev->Dma->Data);
+ return ReportHostAdapterOverrunError (Packet);
+ }
+ if (Packet->OutTransferLength > sizeof (Dev->Dma->Data)) {
+ Packet->OutTransferLength = sizeof (Dev->Dma->Data);
+ return ReportHostAdapterOverrunError (Packet);
+ }
+
+ ZeroMem (Request, sizeof (*Request));
+ Request->Header.TargetId = Target;
+ //
+ // Only LUN 0 is currently supported, hence the cast is safe
+ //
+ Request->Header.Lun[1] = (UINT8)Lun;
+ Request->Header.Function = MPT_MESSAGE_HDR_FUNCTION_SCSI_IO_REQUEST;
+ Request->Header.MessageContext = 1; // We handle one request at a time
+
+ Request->Header.CdbLength = Packet->CdbLength;
+ CopyMem (Request->Header.Cdb, Packet->Cdb, Packet->CdbLength);
+
+ //
+ // SenseDataLength is UINT8, Sense[] is MAX_UINT8, so we can't overflow
+ //
+ ZeroMem (Dev->Dma->Sense, Packet->SenseDataLength);
+ Request->Header.SenseBufferLength = Packet->SenseDataLength;
+ Request->Header.SenseBufferLowAddress = MPT_SCSI_DMA_ADDR_LOW (Dev, Sense);
+
+ Request->Sg.EndOfList = 1;
+ Request->Sg.EndOfBuffer = 1;
+ Request->Sg.LastElement = 1;
+ Request->Sg.ElementType = MPT_SG_ENTRY_TYPE_SIMPLE;
+ Request->Sg.Is64BitAddress = 1;
+ Request->Sg.DataBufferAddress = MPT_SCSI_DMA_ADDR (Dev, Data);
+
+ //
+ // "MPT_SG_ENTRY_SIMPLE.Length" is a 24-bit quantity.
+ //
+ STATIC_ASSERT (
+ sizeof (Dev->Dma->Data) < SIZE_16MB,
+ "MPT_SCSI_DMA_BUFFER.Data must be smaller than 16MB"
+ );
+
+ if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) {
+ Request->Header.DataLength = Packet->InTransferLength;
+ Request->Sg.Length = Packet->InTransferLength;
+ Request->Header.Control = MPT_SCSIIO_REQUEST_CONTROL_TXDIR_READ;
+ } else {
+ Request->Header.DataLength = Packet->OutTransferLength;
+ Request->Sg.Length = Packet->OutTransferLength;
+ Request->Header.Control = MPT_SCSIIO_REQUEST_CONTROL_TXDIR_WRITE;
+
+ CopyMem (Dev->Dma->Data, Packet->OutDataBuffer, Packet->OutTransferLength);
+ Request->Sg.BufferContainsData = 1;
+ }
+
+ if (Request->Header.DataLength == 0) {
+ Request->Header.Control = MPT_SCSIIO_REQUEST_CONTROL_TXDIR_NONE;
+ }
+
+ return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+MptScsiSendRequest (
+ IN MPT_SCSI_DEV *Dev,
+ IN OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet
+ )
+{
+ EFI_STATUS Status;
+
+ if (!Dev->IoReplyEnqueued) {
+ //
+ // Put one free reply frame on the reply queue, the hardware may use it to
+ // report an error to us.
+ //
+ Status = Out32 (Dev, MPT_REG_REP_Q, MPT_SCSI_DMA_ADDR_LOW (Dev, IoReply));
+ if (EFI_ERROR (Status)) {
+ return EFI_DEVICE_ERROR;
+ }
+ Dev->IoReplyEnqueued = TRUE;
+ }
+
+ Status = Out32 (Dev, MPT_REG_REQ_Q, MPT_SCSI_DMA_ADDR_LOW (Dev, IoRequest));
+ if (EFI_ERROR (Status)) {
+ return EFI_DEVICE_ERROR;
+ }
+
+ return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+MptScsiGetReply (
+ IN MPT_SCSI_DEV *Dev,
+ OUT UINT32 *Reply
+ )
+{
+ EFI_STATUS Status;
+ UINT32 Istatus;
+ UINT32 EmptyReply;
+
+ //
+ // Timeouts are not supported for
+ // EFI_EXT_SCSI_PASS_THRU_PROTOCOL.PassThru() in this implementation.
+ //
+ for (;;) {
+ Status = In32 (Dev, MPT_REG_ISTATUS, &Istatus);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ //
+ // Interrupt raised
+ //
+ if (Istatus & MPT_IMASK_REPLY) {
+ break;
+ }
+
+ gBS->Stall (Dev->StallPerPollUsec);
+ }
+
+ Status = In32 (Dev, MPT_REG_REP_Q, Reply);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ //
+ // The driver is supposed to fetch replies until 0xffffffff is returned, which
+ // will reset the interrupt status. We put only one request, so we expect the
+ // next read reply to be the last.
+ //
+ Status = In32 (Dev, MPT_REG_REP_Q, &EmptyReply);
+ if (EFI_ERROR (Status) || EmptyReply != MAX_UINT32) {
+ return EFI_DEVICE_ERROR;
+ }
+
+ return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+MptScsiHandleReply (
+ IN MPT_SCSI_DEV *Dev,
+ IN UINT32 Reply,
+ OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet
+ )
+{
+ if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) {
+ CopyMem (Packet->InDataBuffer, Dev->Dma->Data, Packet->InTransferLength);
+ }
+
+ if (Reply == Dev->Dma->IoRequest.Data.Header.MessageContext) {
+ //
+ // This is a turbo reply, everything is good
+ //
+ Packet->SenseDataLength = 0;
+ Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OK;
+ Packet->TargetStatus = EFI_EXT_SCSI_STATUS_TARGET_GOOD;
+
+ } else if ((Reply & BIT31) != 0) {
+ DEBUG ((DEBUG_INFO, "%a: Full reply returned\n", __FUNCTION__));
+ //
+ // When reply MSB is set, we got a full reply. Since we submitted only one
+ // reply frame, we know it's IoReply.
+ //
+ Dev->IoReplyEnqueued = FALSE;
+
+ Packet->TargetStatus = Dev->Dma->IoReply.Data.ScsiStatus;
+ //
+ // Make sure device only lowers SenseDataLength before copying sense
+ //
+ ASSERT (Dev->Dma->IoReply.Data.SenseCount <= Packet->SenseDataLength);
+ Packet->SenseDataLength =
+ (UINT8)MIN (Dev->Dma->IoReply.Data.SenseCount, Packet->SenseDataLength);
+ CopyMem (Packet->SenseData, Dev->Dma->Sense, Packet->SenseDataLength);
+
+ if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) {
+ Packet->InTransferLength = Dev->Dma->IoReply.Data.TransferCount;
+ } else {
+ Packet->OutTransferLength = Dev->Dma->IoReply.Data.TransferCount;
+ }
+
+ switch (Dev->Dma->IoReply.Data.IocStatus) {
+ case MPT_SCSI_IOCSTATUS_SUCCESS:
+ Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OK;
+ break;
+ case MPT_SCSI_IOCSTATUS_DEVICE_NOT_THERE:
+ Packet->HostAdapterStatus =
+ EFI_EXT_SCSI_STATUS_HOST_ADAPTER_SELECTION_TIMEOUT;
+ return EFI_TIMEOUT;
+ case MPT_SCSI_IOCSTATUS_DATA_UNDERRUN:
+ case MPT_SCSI_IOCSTATUS_DATA_OVERRUN:
+ Packet->HostAdapterStatus =
+ EFI_EXT_SCSI_STATUS_HOST_ADAPTER_DATA_OVERRUN_UNDERRUN;
+ break;
+ default:
+ Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OTHER;
+ return EFI_DEVICE_ERROR;
+ }
+
+ } else {
+ DEBUG ((DEBUG_ERROR, "%a: unexpected reply (%x)\n", __FUNCTION__, Reply));
+ return ReportHostAdapterError (Packet);
+ }
+
+ return EFI_SUCCESS;
+}
+
//
// Ext SCSI Pass Thru
//
@@ -245,7 +563,33 @@ MptScsiPassThru (
IN EFI_EVENT Event OPTIONAL
)
{
- return EFI_UNSUPPORTED;
+ EFI_STATUS Status;
+ MPT_SCSI_DEV *Dev;
+ UINT32 Reply;
+
+ Dev = MPT_SCSI_FROM_PASS_THRU (This);
+ //
+ // We only use first byte of target identifer
+ //
+ Status = MptScsiPopulateRequest (Dev, *Target, Lun, Packet);
+ if (EFI_ERROR (Status)) {
+ //
+ // MptScsiPopulateRequest modified packet according to the error
+ //
+ return Status;
+ }
+
+ Status = MptScsiSendRequest (Dev, Packet);
+ if (EFI_ERROR (Status)) {
+ return ReportHostAdapterError (Packet);
+ }
+
+ Status = MptScsiGetReply (Dev, &Reply);
+ if (EFI_ERROR (Status)) {
+ return ReportHostAdapterError (Packet);
+ }
+
+ return MptScsiHandleReply (Dev, Reply, Packet);
}

STATIC
@@ -500,6 +844,8 @@ MptScsiControllerStart (
{
EFI_STATUS Status;
MPT_SCSI_DEV *Dev;
+ UINTN Pages;
+ UINTN BytesMapped;

Dev = AllocateZeroPool (sizeof (*Dev));
if (Dev == NULL) {
@@ -509,6 +855,7 @@ MptScsiControllerStart (
Dev->Signature = MPT_SCSI_DEV_SIGNATURE;

Dev->MaxTarget = PcdGet8 (PcdMptScsiMaxTargetLimit);
+ Dev->StallPerPollUsec = PcdGet32 (PcdMptScsiStallPerPollUsec);

Status = gBS->OpenProtocol (
ControllerHandle,
@@ -569,11 +916,45 @@ MptScsiControllerStart (
));
}

- Status = MptScsiInit (Dev);
+ //
+ // Create buffers for data transfer
+ //
+ Pages = EFI_SIZE_TO_PAGES (sizeof (*Dev->Dma));
+ Status = Dev->PciIo->AllocateBuffer (
+ Dev->PciIo,
+ AllocateAnyPages,
+ EfiBootServicesData,
+ Pages,
+ (VOID **)&Dev->Dma,
+ EFI_PCI_ATTRIBUTE_MEMORY_CACHED
+ );
if (EFI_ERROR (Status)) {
goto RestoreAttributes;
}

+ BytesMapped = EFI_PAGES_TO_SIZE (Pages);
+ Status = Dev->PciIo->Map (
+ Dev->PciIo,
+ EfiPciIoOperationBusMasterCommonBuffer,
+ Dev->Dma,
+ &BytesMapped,
+ &Dev->DmaPhysical,
+ &Dev->DmaMapping
+ );
+ if (EFI_ERROR (Status)) {
+ goto FreeBuffer;
+ }
+
+ if (BytesMapped != EFI_PAGES_TO_SIZE (Pages)) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto Unmap;
+ }
+
+ Status = MptScsiInit (Dev);
+ if (EFI_ERROR (Status)) {
+ goto Unmap;
+ }
+
//
// Host adapter channel, doesn't exist
//
@@ -606,6 +987,19 @@ MptScsiControllerStart (
UninitDev:
MptScsiReset (Dev);

+Unmap:
+ Dev->PciIo->Unmap (
+ Dev->PciIo,
+ Dev->DmaMapping
+ );
+
+FreeBuffer:
+ Dev->PciIo->FreeBuffer (
+ Dev->PciIo,
+ Pages,
+ Dev->Dma
+ );
+
RestoreAttributes:
Dev->PciIo->Attributes (
Dev->PciIo,
@@ -667,6 +1061,17 @@ MptScsiControllerStop (

MptScsiReset (Dev);

+ Dev->PciIo->Unmap (
+ Dev->PciIo,
+ Dev->DmaMapping
+ );
+
+ Dev->PciIo->FreeBuffer (
+ Dev->PciIo,
+ EFI_SIZE_TO_PAGES (sizeof (*Dev->Dma)),
+ Dev->Dma
+ );
+
Dev->PciIo->Attributes (
Dev->PciIo,
EfiPciIoAttributeOperationSet,
diff --git a/OvmfPkg/MptScsiDxe/MptScsiDxe.inf b/OvmfPkg/MptScsiDxe/MptScsiDxe.inf
index d5fd2516e475..09108939a5a2 100644
--- a/OvmfPkg/MptScsiDxe/MptScsiDxe.inf
+++ b/OvmfPkg/MptScsiDxe/MptScsiDxe.inf
@@ -24,6 +24,7 @@ [Packages]
OvmfPkg/OvmfPkg.dec

[LibraryClasses]
+ BaseLib
BaseMemoryLib
DebugLib
MemoryAllocationLib
@@ -38,3 +39,6 @@ [Protocols]

[FixedPcd]
gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiMaxTargetLimit ## CONSUMES
+
+[Pcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiStallPerPollUsec ## CONSUMES
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 2d09444bbb16..3bf26e8df82d 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -167,6 +167,9 @@ [PcdsFixedAtBuild]
# by ScsiBusDxe.
gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiMaxTargetLimit|7|UINT8|0x39

+ ## Microseconds to stall between polling for MptScsi request result
+ gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiStallPerPollUsec|5|UINT32|0x40
+
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase|0x0|UINT32|0x8
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize|0x0|UINT32|0x9
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize|0x0|UINT32|0xa


Re: [PATCH v6 10/12] OvmfPkg/MptScsiDxe: Initialize hardware

Laszlo Ersek
 

On 05/04/20 23:06, Nikita Leshenko wrote:
Reset and send the IO controller initialization request. The reply is
read back to complete the doorbell function but it isn't useful to us
because it doesn't contain relevant data or status codes.

See "LSI53C1030 PCI-X to Dual Channel Ultra320 SCSI Multifunction
Controller" technical manual for more information.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2390
Signed-off-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
---
.../Include/IndustryStandard/FusionMptScsi.h | 128 +++++++++++
OvmfPkg/MptScsiDxe/MptScsi.c | 198 +++++++++++++++++-
2 files changed, 325 insertions(+), 1 deletion(-)
Reviewed-by: Laszlo Ersek <lersek@redhat.com>


Re: [PATCH v6 06/12] OvmfPkg/MptScsiDxe: Report targets and one LUN

Laszlo Ersek
 

On 05/04/20 23:06, Nikita Leshenko wrote:
The controller supports up to 8 targets in practice (Not reported by the
controller, but based on the implementation of the virtual device),
report them in GetNextTarget and GetNextTargetLun. The firmware will
then try to communicate with them and create a block device for each one
that responds.

Support for multiple LUNs will be implemented in another series.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2390
Signed-off-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/MptScsiDxe/MptScsi.c | 63 ++++++++++++++++++++++++++++++-
OvmfPkg/MptScsiDxe/MptScsiDxe.inf | 5 +++
OvmfPkg/OvmfPkg.dec | 4 ++
3 files changed, 70 insertions(+), 2 deletions(-)
Thanks for the PcdLib-related updates!
Laszlo

diff --git a/OvmfPkg/MptScsiDxe/MptScsi.c b/OvmfPkg/MptScsiDxe/MptScsi.c
index 40d392c2346f..d396bff85cb6 100644
--- a/OvmfPkg/MptScsiDxe/MptScsi.c
+++ b/OvmfPkg/MptScsiDxe/MptScsi.c
@@ -11,8 +11,10 @@

#include <IndustryStandard/FusionMptScsi.h>
#include <IndustryStandard/Pci.h>
+#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
#include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Library/UefiLib.h>
#include <Protocol/PciIo.h>
@@ -34,6 +36,7 @@ typedef struct {
UINT32 Signature;
EFI_EXT_SCSI_PASS_THRU_PROTOCOL PassThru;
EFI_EXT_SCSI_PASS_THRU_MODE PassThruMode;
+ UINT8 MaxTarget;
} MPT_SCSI_DEV;

#define MPT_SCSI_FROM_PASS_THRU(PassThruPtr) \
@@ -57,6 +60,22 @@ MptScsiPassThru (
return EFI_UNSUPPORTED;
}

+STATIC
+BOOLEAN
+IsTargetInitialized (
+ IN UINT8 *Target
+ )
+{
+ UINTN Idx;
+
+ for (Idx = 0; Idx < TARGET_MAX_BYTES; ++Idx) {
+ if (Target[Idx] != 0xFF) {
+ return TRUE;
+ }
+ }
+ return FALSE;
+}
+
STATIC
EFI_STATUS
EFIAPI
@@ -66,7 +85,28 @@ MptScsiGetNextTargetLun (
IN OUT UINT64 *Lun
)
{
- return EFI_UNSUPPORTED;
+ MPT_SCSI_DEV *Dev;
+
+ Dev = MPT_SCSI_FROM_PASS_THRU (This);
+ //
+ // Currently support only LUN 0, so hardcode it
+ //
+ if (!IsTargetInitialized (*Target)) {
+ ZeroMem (*Target, TARGET_MAX_BYTES);
+ *Lun = 0;
+ } else if (**Target > Dev->MaxTarget || *Lun > 0) {
+ return EFI_INVALID_PARAMETER;
+ } else if (**Target < Dev->MaxTarget) {
+ //
+ // This device interface support 256 targets only, so it's enough to
+ // increment the LSB of Target, as it will never overflow.
+ //
+ **Target += 1;
+ } else {
+ return EFI_NOT_FOUND;
+ }
+
+ return EFI_SUCCESS;
}

STATIC
@@ -77,7 +117,24 @@ MptScsiGetNextTarget (
IN OUT UINT8 **Target
)
{
- return EFI_UNSUPPORTED;
+ MPT_SCSI_DEV *Dev;
+
+ Dev = MPT_SCSI_FROM_PASS_THRU (This);
+ if (!IsTargetInitialized (*Target)) {
+ ZeroMem (*Target, TARGET_MAX_BYTES);
+ } else if (**Target > Dev->MaxTarget) {
+ return EFI_INVALID_PARAMETER;
+ } else if (**Target < Dev->MaxTarget) {
+ //
+ // This device interface support 256 targets only, so it's enough to
+ // increment the LSB of Target, as it will never overflow.
+ //
+ **Target += 1;
+ } else {
+ return EFI_NOT_FOUND;
+ }
+
+ return EFI_SUCCESS;
}

STATIC
@@ -206,6 +263,8 @@ MptScsiControllerStart (

Dev->Signature = MPT_SCSI_DEV_SIGNATURE;

+ Dev->MaxTarget = PcdGet8 (PcdMptScsiMaxTargetLimit);
+
//
// Host adapter channel, doesn't exist
//
diff --git a/OvmfPkg/MptScsiDxe/MptScsiDxe.inf b/OvmfPkg/MptScsiDxe/MptScsiDxe.inf
index 9f7c98829ee1..d5fd2516e475 100644
--- a/OvmfPkg/MptScsiDxe/MptScsiDxe.inf
+++ b/OvmfPkg/MptScsiDxe/MptScsiDxe.inf
@@ -24,8 +24,10 @@ [Packages]
OvmfPkg/OvmfPkg.dec

[LibraryClasses]
+ BaseMemoryLib
DebugLib
MemoryAllocationLib
+ PcdLib
UefiBootServicesTableLib
UefiDriverEntryPoint
UefiLib
@@ -33,3 +35,6 @@ [LibraryClasses]
[Protocols]
gEfiExtScsiPassThruProtocolGuid ## BY_START
gEfiPciIoProtocolGuid ## TO_START
+
+[FixedPcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiMaxTargetLimit ## CONSUMES
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 28030391cff2..2d09444bbb16 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -163,6 +163,10 @@ [PcdsFixedAtBuild]
# polling loop iteration.
gUefiOvmfPkgTokenSpaceGuid.PcdPvScsiWaitForCmpStallInUsecs|5|UINT32|0x38

+ ## Set the *inclusive* number of targets that MptScsi exposes for scan
+ # by ScsiBusDxe.
+ gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiMaxTargetLimit|7|UINT8|0x39
+
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase|0x0|UINT32|0x8
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize|0x0|UINT32|0x9
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize|0x0|UINT32|0xa


Re: [PATCH 1/1] OvmfPkg: Add BaseResetSystemLibBhyve.inf

Laszlo Ersek
 

On 05/04/20 04:18, Rebecca Cran wrote:
Introduce BaseResetSystemLibBhyve.inf, to support powering off
bhyve guests.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
Laszlo: I couldn't find your email specifying specific changes to
make,
It's archived here:

http://mid.mail-archive.com/f1dacfcb-fd50-6e58-965b-656181d3d6d2@redhat.com
https://edk2.groups.io/g/devel/message/58076


(1) The ".inf" should be stripped from the subject line.

so I hope I've caught all of them.

OvmfPkg/Include/IndustryStandard/Bhyve.h | 2 +
.../ResetSystemLib/BaseResetShutdownBhyve.c | 34 ++++++++++++++++
.../BaseResetSystemLibBhyve.inf | 40 +++++++++++++++++++
3 files changed, 76 insertions(+)
create mode 100644 OvmfPkg/Library/ResetSystemLib/BaseResetShutdownBhyve.c
create mode 100644 OvmfPkg/Library/ResetSystemLib/BaseResetSystemLibBhyve.inf

diff --git a/OvmfPkg/Include/IndustryStandard/Bhyve.h b/OvmfPkg/Include/IndustryStandard/Bhyve.h
index 02ce5587ee0f..ab8f2b0729b4 100644
--- a/OvmfPkg/Include/IndustryStandard/Bhyve.h
+++ b/OvmfPkg/Include/IndustryStandard/Bhyve.h
@@ -13,4 +13,6 @@

#define BHYVE_ACPI_TIMER_IO_ADDR 0x408

+#define BHYVE_PM_REG 0x404
+
#endif // __BHYVE_H__
diff --git a/OvmfPkg/Library/ResetSystemLib/BaseResetShutdownBhyve.c b/OvmfPkg/Library/ResetSystemLib/BaseResetShutdownBhyve.c
new file mode 100644
index 000000000000..156e5ad0d797
--- /dev/null
+++ b/OvmfPkg/Library/ResetSystemLib/BaseResetShutdownBhyve.c
@@ -0,0 +1,34 @@
+/** @file
+ Base Reset System Library Shutdown API implementation for bhyve.
+
+ Copyright (C) 2020, Rebecca Cran <rebecca@bsdio.com>
+ Copyright (C) 2020, Red Hat, Inc.
+ Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Base.h> // BIT13
+
+#include <Library/BaseLib.h> // CpuDeadLoop()
+#include <Library/IoLib.h> // IoOr16()
+#include <Library/ResetSystemLib.h> // ResetShutdown()
+#include <OvmfPlatforms.h> // BHYVE_PM_REG
(2) Even though <OvmfPlatforms.h> does include
<IndustryStandard/Bhyve.h> now, from commit 91dee771fc0d ("OvmfPkg: Add
bhyve support into AcpiTimerLib", 2020-04-30), there's still no reason
for this library instance to prefer the more general <OvmfPlatforms.h>
to <IndustryStandard/Bhyve.h>.

+
+/**
+ Calling this function causes the system to enter a power state equivalent
+ to the ACPI G2/S5 or G3 states.
+
+ System shutdown should not return, if it returns, it means the system does
+ not support shut down reset.
+**/
+VOID
+EFIAPI
+ResetShutdown (
+ VOID
+ )
+{
+ IoBitFieldWrite16 (BHYVE_PM_REG, 10, 13, 5);
+ IoOr16 (BHYVE_PM_REG, BIT13);
+ CpuDeadLoop ();
+}
diff --git a/OvmfPkg/Library/ResetSystemLib/BaseResetSystemLibBhyve.inf b/OvmfPkg/Library/ResetSystemLib/BaseResetSystemLibBhyve.inf
new file mode 100644
index 000000000000..33e4e2689e57
--- /dev/null
+++ b/OvmfPkg/Library/ResetSystemLib/BaseResetSystemLibBhyve.inf
@@ -0,0 +1,40 @@
+## @file
+# Base library instance for ResetSystem library class for bhyve
+#
+# Copyright (C) 2020, Rebecca Cran <rebecca@bsdio.com>
+# Copyright (C) 2020, Red Hat, Inc.
+# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 1.29
+ BASE_NAME = BaseResetSystemLibBhyve
+ FILE_GUID = 5c71b08f-0ade-4607-8b9d-946c2757fee8
+ MODULE_TYPE = DXE_DRIVER
(3) Should be BASE.


I've fixed up the above three warts for you:

[lersek@redhat.com: MODULE_TYPE: replace DXE_DRIVER with BASE]
[lersek@redhat.com: replace <OvmfPlatforms.h> with <IndustryStandard/Bhyve.h>]
[lersek@redhat.com: strip ".inf" from subject line]

With those:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

(I also build-tested the library after my fixups.)

github.com pull request: https://github.com/tianocore/edk2/pull/580

Commit: f159102a130fac9b416418eb9b6fa35b63426ca5

Thanks,
Laszlo

+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = ResetSystemLib
+
+#
+# The following information is for reference only and not required by the build
+# tools.
+#
+# VALID_ARCHITECTURES = IA32 X64
+#
+
+[Sources]
+ BaseResetShutdownBhyve.c
+ ResetSystemLib.c
+
+[Packages]
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ IoLib
+ TimerLib
+


Re: [PATCH 1/4] UefiCpuPkg/CpuExceptionHandler: Make XCODE5 changes toolchain specific

Lendacky, Thomas
 

On 5/1/20 3:17 PM, Lendacky, Thomas via groups.io wrote:
BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2340&;data=02%7C01%7Cthomas.lendacky%40amd.com%7Ca1268b8e6d554b6c55ca08d7ee0cbf84%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637239610850064292&amp;sdata=soYzRSutJ%2Bepugf2XnzRUpMCLa1GaKoBD%2FX1F4HPCt8%3D&amp;reserved=0
Commit 2db0ccc2d7fe ("UefiCpuPkg: Update CpuExceptionHandlerLib pass
XCODE5 tool chain") introduced binary patching into the exception handling
support. CPU exception handling is allowed during SEC and this results in
binary patching of flash, which should not be done.
Separate the changes from commit 2db0ccc2d7fe into an XCODE5 toolchain
specific file, Xcode5ExceptionHandlerAsm.nasm, and create new INF files
for an XCODE5 version of CpuExceptionHandlerLib. Update the UefiCpuPkg.dsc
file to use the new files when the XCODE5 toolchain is used.
I used the same FILE_GUID for the new INF files as the old ones. I wasn't sure if they should get new GUIDs or use the same GUID as the original since only one of the libraries will/should be used at a time.

Let me know if they need new GUIDs and I'll update and repost the series.

Thanks,
Tom

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
UefiCpuPkg/UefiCpuPkg.dsc | 23 +
.../Xcode5DxeCpuExceptionHandlerLib.inf | 64 +++
.../Xcode5PeiCpuExceptionHandlerLib.inf | 63 +++
.../Xcode5SecPeiCpuExceptionHandlerLib.inf | 55 +++
.../Xcode5SmmCpuExceptionHandlerLib.inf | 59 +++
.../X64/Xcode5ExceptionHandlerAsm.nasm | 413 ++++++++++++++++++
6 files changed, 677 insertions(+)
create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf
create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf
create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf
create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm
diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc
index d28cb5cccb52..ad298011232d 100644
--- a/UefiCpuPkg/UefiCpuPkg.dsc
+++ b/UefiCpuPkg/UefiCpuPkg.dsc
@@ -59,7 +59,11 @@ [LibraryClasses]
[LibraryClasses.common.SEC]
PlatformSecLib|UefiCpuPkg/Library/PlatformSecLibNull/PlatformSecLibNull.inf
+!if $(TOOL_CHAIN_TAG) != "XCODE5"
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
+!else
+ CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
+!endif
HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
@@ -73,12 +77,20 @@ [LibraryClasses.common.PEIM]
[LibraryClasses.IA32.PEIM, LibraryClasses.X64.PEIM]
PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
+!if $(TOOL_CHAIN_TAG) != "XCODE5"
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
+!else
+ CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf
+!endif
[LibraryClasses.common.DXE_DRIVER]
MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
+!if $(TOOL_CHAIN_TAG) != "XCODE5"
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
+!else
+ CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf
+!endif
MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
RegisterCpuFeaturesLib|UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.inf
@@ -86,7 +98,11 @@ [LibraryClasses.common.DXE_SMM_DRIVER]
SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf
MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf
HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
+!if $(TOOL_CHAIN_TAG) != "XCODE5"
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
+!else
+ CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf
+!endif
[LibraryClasses.common.UEFI_APPLICATION]
UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
@@ -122,10 +138,17 @@ [Components.IA32, Components.X64]
UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf
+!if $(TOOL_CHAIN_TAG) != "XCODE5"
UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
+!else
+ UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf
+ UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
+ UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf
+ UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf
+!endif
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.inf
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf
new file mode 100644
index 000000000000..ef37efec6246
--- /dev/null
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5DxeCpuExceptionHandlerLib.inf
@@ -0,0 +1,64 @@
+## @file
+# CPU Exception Handler library instance for DXE modules.
+#
+# Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = Xcode5DxeCpuExceptionHandlerLib
+ MODULE_UNI_FILE = DxeCpuExceptionHandlerLib.uni
+ FILE_GUID = B6E9835A-EDCF-4748-98A8-27D3C722E02D
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.1
+ LIBRARY_CLASS = CpuExceptionHandlerLib|DXE_CORE DXE_DRIVER UEFI_APPLICATION
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = IA32 X64
+#
+
+[Sources.Ia32]
+ Ia32/ExceptionHandlerAsm.nasm
+ Ia32/ExceptionTssEntryAsm.nasm
+ Ia32/ArchExceptionHandler.c
+ Ia32/ArchInterruptDefs.h
+ Ia32/ArchAMDSevVcHandler.c
+
+[Sources.X64]
+ X64/Xcode5ExceptionHandlerAsm.nasm
+ X64/ArchExceptionHandler.c
+ X64/ArchInterruptDefs.h
+ X64/ArchAMDSevVcHandler.c
+
+[Sources.common]
+ CpuExceptionCommon.h
+ CpuExceptionCommon.c
+ PeiDxeSmmCpuException.c
+ DxeException.c
+ AMDSevVcHandler.c
+ AMDSevVcCommon.h
+
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ SerialPortLib
+ PrintLib
+ SynchronizationLib
+ LocalApicLib
+ PeCoffGetEntryPointLib
+ MemoryAllocationLib
+ DebugLib
+ VmgExitLib
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf
new file mode 100644
index 000000000000..830ed1eb8bad
--- /dev/null
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5PeiCpuExceptionHandlerLib.inf
@@ -0,0 +1,63 @@
+## @file
+# CPU Exception Handler library instance for PEI module.
+#
+# Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = Xcode5PeiCpuExceptionHandlerLib
+ MODULE_UNI_FILE = PeiCpuExceptionHandlerLib.uni
+ FILE_GUID = 980DDA67-44A6-4897-99E6-275290B71F9E
+ MODULE_TYPE = PEIM
+ VERSION_STRING = 1.1
+ LIBRARY_CLASS = CpuExceptionHandlerLib|PEI_CORE PEIM
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = IA32 X64
+#
+
+[Sources.Ia32]
+ Ia32/ExceptionHandlerAsm.nasm
+ Ia32/ExceptionTssEntryAsm.nasm
+ Ia32/ArchExceptionHandler.c
+ Ia32/ArchInterruptDefs.h
+ Ia32/ArchAMDSevVcHandler.c
+
+[Sources.X64]
+ X64/Xcode5ExceptionHandlerAsm.nasm
+ X64/ArchExceptionHandler.c
+ X64/ArchInterruptDefs.h
+ X64/ArchAMDSevVcHandler.c
+
+[Sources.common]
+ CpuExceptionCommon.h
+ CpuExceptionCommon.c
+ PeiCpuException.c
+ PeiDxeSmmCpuException.c
+ AMDSevVcHandler.c
+ AMDSevVcCommon.h
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ SerialPortLib
+ PrintLib
+ LocalApicLib
+ PeCoffGetEntryPointLib
+ HobLib
+ MemoryAllocationLib
+ SynchronizationLib
+ VmgExitLib
+
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard # CONSUMES
+
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
new file mode 100644
index 000000000000..36420be22faa
--- /dev/null
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
@@ -0,0 +1,55 @@
+## @file
+# CPU Exception Handler library instance for SEC/PEI modules.
+#
+# Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = Xcode5SecPeiCpuExceptionHandlerLib
+ MODULE_UNI_FILE = SecPeiCpuExceptionHandlerLib.uni
+ FILE_GUID = CA4BBC99-DFC6-4234-B553-8B6586B7B113
+ MODULE_TYPE = PEIM
+ VERSION_STRING = 1.1
+ LIBRARY_CLASS = CpuExceptionHandlerLib|SEC PEI_CORE PEIM
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = IA32 X64
+#
+
+[Sources.Ia32]
+ Ia32/ExceptionHandlerAsm.nasm
+ Ia32/ExceptionTssEntryAsm.nasm
+ Ia32/ArchExceptionHandler.c
+ Ia32/ArchInterruptDefs.h
+ Ia32/ArchAMDSevVcHandler.c
+
+[Sources.X64]
+ X64/Xcode5ExceptionHandlerAsm.nasm
+ X64/ArchExceptionHandler.c
+ X64/ArchInterruptDefs.h
+ X64/ArchAMDSevVcHandler.c
+
+[Sources.common]
+ CpuExceptionCommon.h
+ CpuExceptionCommon.c
+ SecPeiCpuException.c
+ AMDSevVcHandler.c
+ AMDSevVcCommon.h
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ SerialPortLib
+ PrintLib
+ LocalApicLib
+ PeCoffGetEntryPointLib
+ VmgExitLib
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf
new file mode 100644
index 000000000000..8f71a45c86d5
--- /dev/null
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SmmCpuExceptionHandlerLib.inf
@@ -0,0 +1,59 @@
+## @file
+# CPU Exception Handler library instance for SMM modules.
+#
+# Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = Xcode5SmmCpuExceptionHandlerLib
+ MODULE_UNI_FILE = SmmCpuExceptionHandlerLib.uni
+ FILE_GUID = 8D2C439B-3981-42ff-9CE5-1B50ECA502D6
+ MODULE_TYPE = DXE_SMM_DRIVER
+ VERSION_STRING = 1.1
+ LIBRARY_CLASS = CpuExceptionHandlerLib|DXE_SMM_DRIVER
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = IA32 X64
+#
+
+[Sources.Ia32]
+ Ia32/ExceptionHandlerAsm.nasm
+ Ia32/ExceptionTssEntryAsm.nasm
+ Ia32/ArchExceptionHandler.c
+ Ia32/ArchInterruptDefs.h
+ Ia32/ArchAMDSevVcHandler.c
+
+[Sources.X64]
+ X64/Xcode5ExceptionHandlerAsm.nasm
+ X64/ArchExceptionHandler.c
+ X64/ArchInterruptDefs.h
+ X64/ArchAMDSevVcHandler.c
+
+[Sources.common]
+ CpuExceptionCommon.h
+ CpuExceptionCommon.c
+ PeiDxeSmmCpuException.c
+ SmmException.c
+ AMDSevVcHandler.c
+ AMDSevVcCommon.h
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ SerialPortLib
+ PrintLib
+ SynchronizationLib
+ LocalApicLib
+ PeCoffGetEntryPointLib
+ DebugLib
+ VmgExitLib
+
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm
new file mode 100644
index 000000000000..26cae56cc5cf
--- /dev/null
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm
@@ -0,0 +1,413 @@
+;------------------------------------------------------------------------------ ;
+; Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+; Module Name:
+;
+; ExceptionHandlerAsm.Asm
+;
+; Abstract:
+;
+; x64 CPU Exception Handler
+;
+; Notes:
+;
+;------------------------------------------------------------------------------
+
+;
+; CommonExceptionHandler()
+;
+
+%define VC_EXCEPTION 29
+
+extern ASM_PFX(mErrorCodeFlag) ; Error code flags for exceptions
+extern ASM_PFX(mDoFarReturnFlag) ; Do far return flag
+extern ASM_PFX(CommonExceptionHandler)
+
+SECTION .data
+
+DEFAULT REL
+SECTION .text
+
+ALIGN 8
+
+AsmIdtVectorBegin:
+%rep 32
+ db 0x6a ; push #VectorNum
+ db ($ - AsmIdtVectorBegin) / ((AsmIdtVectorEnd - AsmIdtVectorBegin) / 32) ; VectorNum
+ push rax
+ mov rax, strict qword 0 ; mov rax, ASM_PFX(CommonInterruptEntry)
+ jmp rax
+%endrep
+AsmIdtVectorEnd:
+
+HookAfterStubHeaderBegin:
+ db 0x6a ; push
+@VectorNum:
+ db 0 ; 0 will be fixed
+ push rax
+ mov rax, strict qword 0 ; mov rax, HookAfterStubHeaderEnd
+JmpAbsoluteAddress:
+ jmp rax
+HookAfterStubHeaderEnd:
+ mov rax, rsp
+ and sp, 0xfff0 ; make sure 16-byte aligned for exception context
+ sub rsp, 0x18 ; reserve room for filling exception data later
+ push rcx
+ mov rcx, [rax + 8]
+ bt [ASM_PFX(mErrorCodeFlag)], ecx
+ jnc .0
+ push qword [rsp] ; push additional rcx to make stack alignment
+.0:
+ xchg rcx, [rsp] ; restore rcx, save Exception Number in stack
+ push qword [rax] ; push rax into stack to keep code consistence
+
+;---------------------------------------;
+; CommonInterruptEntry ;
+;---------------------------------------;
+; The follow algorithm is used for the common interrupt routine.
+; Entry from each interrupt with a push eax and eax=interrupt number
+; Stack frame would be as follows as specified in IA32 manuals:
+;
+; +---------------------+ <-- 16-byte aligned ensured by processor
+; + Old SS +
+; +---------------------+
+; + Old RSP +
+; +---------------------+
+; + RFlags +
+; +---------------------+
+; + CS +
+; +---------------------+
+; + RIP +
+; +---------------------+
+; + Error Code +
+; +---------------------+
+; + Vector Number +
+; +---------------------+
+; + RBP +
+; +---------------------+ <-- RBP, 16-byte aligned
+; The follow algorithm is used for the common interrupt routine.
+global ASM_PFX(CommonInterruptEntry)
+ASM_PFX(CommonInterruptEntry):
+ cli
+ pop rax
+ ;
+ ; All interrupt handlers are invoked through interrupt gates, so
+ ; IF flag automatically cleared at the entry point
+ ;
+ xchg rcx, [rsp] ; Save rcx into stack and save vector number into rcx
+ and rcx, 0xFF
+ cmp ecx, 32 ; Intel reserved vector for exceptions?
+ jae NoErrorCode
+ bt [ASM_PFX(mErrorCodeFlag)], ecx
+ jc HasErrorCode
+
+NoErrorCode:
+
+ ;
+ ; Push a dummy error code on the stack
+ ; to maintain coherent stack map
+ ;
+ push qword [rsp]
+ mov qword [rsp + 8], 0
+HasErrorCode:
+ push rbp
+ mov rbp, rsp
+ push 0 ; clear EXCEPTION_HANDLER_CONTEXT.OldIdtHandler
+ push 0 ; clear EXCEPTION_HANDLER_CONTEXT.ExceptionDataFlag
+
+ ;
+ ; Stack:
+ ; +---------------------+ <-- 16-byte aligned ensured by processor
+ ; + Old SS +
+ ; +---------------------+
+ ; + Old RSP +
+ ; +---------------------+
+ ; + RFlags +
+ ; +---------------------+
+ ; + CS +
+ ; +---------------------+
+ ; + RIP +
+ ; +---------------------+
+ ; + Error Code +
+ ; +---------------------+
+ ; + RCX / Vector Number +
+ ; +---------------------+
+ ; + RBP +
+ ; +---------------------+ <-- RBP, 16-byte aligned
+ ;
+
+ ;
+ ; Since here the stack pointer is 16-byte aligned, so
+ ; EFI_FX_SAVE_STATE_X64 of EFI_SYSTEM_CONTEXT_x64
+ ; is 16-byte aligned
+ ;
+
+;; UINT64 Rdi, Rsi, Rbp, Rsp, Rbx, Rdx, Rcx, Rax;
+;; UINT64 R8, R9, R10, R11, R12, R13, R14, R15;
+ push r15
+ push r14
+ push r13
+ push r12
+ push r11
+ push r10
+ push r9
+ push r8
+ push rax
+ push qword [rbp + 8] ; RCX
+ push rdx
+ push rbx
+ push qword [rbp + 48] ; RSP
+ push qword [rbp] ; RBP
+ push rsi
+ push rdi
+
+;; UINT64 Gs, Fs, Es, Ds, Cs, Ss; insure high 16 bits of each is zero
+ movzx rax, word [rbp + 56]
+ push rax ; for ss
+ movzx rax, word [rbp + 32]
+ push rax ; for cs
+ mov rax, ds
+ push rax
+ mov rax, es
+ push rax
+ mov rax, fs
+ push rax
+ mov rax, gs
+ push rax
+
+ mov [rbp + 8], rcx ; save vector number
+
+;; UINT64 Rip;
+ push qword [rbp + 24]
+
+;; UINT64 Gdtr[2], Idtr[2];
+ xor rax, rax
+ push rax
+ push rax
+ sidt [rsp]
+ mov bx, word [rsp]
+ mov rax, qword [rsp + 2]
+ mov qword [rsp], rax
+ mov word [rsp + 8], bx
+
+ xor rax, rax
+ push rax
+ push rax
+ sgdt [rsp]
+ mov bx, word [rsp]
+ mov rax, qword [rsp + 2]
+ mov qword [rsp], rax
+ mov word [rsp + 8], bx
+
+;; UINT64 Ldtr, Tr;
+ xor rax, rax
+ str ax
+ push rax
+ sldt ax
+ push rax
+
+;; UINT64 RFlags;
+ push qword [rbp + 40]
+
+;; UINT64 Cr0, Cr1, Cr2, Cr3, Cr4, Cr8;
+ mov rax, cr8
+ push rax
+ mov rax, cr4
+ or rax, 0x208
+ mov cr4, rax
+ push rax
+ mov rax, cr3
+ push rax
+ mov rax, cr2
+ push rax
+ xor rax, rax
+ push rax
+ mov rax, cr0
+ push rax
+
+;; UINT64 Dr0, Dr1, Dr2, Dr3, Dr6, Dr7;
+ cmp qword [rbp + 8], VC_EXCEPTION
+ je VcDebugRegs ; For SEV-ES (#VC) Debug registers ignored
+
+ mov rax, dr7
+ push rax
+ mov rax, dr6
+ push rax
+ mov rax, dr3
+ push rax
+ mov rax, dr2
+ push rax
+ mov rax, dr1
+ push rax
+ mov rax, dr0
+ push rax
+ jmp DrFinish
+
+VcDebugRegs:
+;; UINT64 Dr0, Dr1, Dr2, Dr3, Dr6, Dr7 are skipped for #VC to avoid exception recursion
+ xor rax, rax
+ push rax
+ push rax
+ push rax
+ push rax
+ push rax
+ push rax
+
+DrFinish:
+;; FX_SAVE_STATE_X64 FxSaveState;
+ sub rsp, 512
+ mov rdi, rsp
+ db 0xf, 0xae, 0x7 ;fxsave [rdi]
+
+;; UEFI calling convention for x64 requires that Direction flag in EFLAGs is clear
+ cld
+
+;; UINT32 ExceptionData;
+ push qword [rbp + 16]
+
+;; Prepare parameter and call
+ mov rcx, [rbp + 8]
+ mov rdx, rsp
+ ;
+ ; Per X64 calling convention, allocate maximum parameter stack space
+ ; and make sure RSP is 16-byte aligned
+ ;
+ sub rsp, 4 * 8 + 8
+ call ASM_PFX(CommonExceptionHandler)
+ add rsp, 4 * 8 + 8
+
+ cli
+;; UINT64 ExceptionData;
+ add rsp, 8
+
+;; FX_SAVE_STATE_X64 FxSaveState;
+
+ mov rsi, rsp
+ db 0xf, 0xae, 0xE ; fxrstor [rsi]
+ add rsp, 512
+
+;; UINT64 Dr0, Dr1, Dr2, Dr3, Dr6, Dr7;
+;; Skip restoration of DRx registers to support in-circuit emualators
+;; or debuggers set breakpoint in interrupt/exception context
+ add rsp, 8 * 6
+
+;; UINT64 Cr0, Cr1, Cr2, Cr3, Cr4, Cr8;
+ pop rax
+ mov cr0, rax
+ add rsp, 8 ; not for Cr1
+ pop rax
+ mov cr2, rax
+ pop rax
+ mov cr3, rax
+ pop rax
+ mov cr4, rax
+ pop rax
+ mov cr8, rax
+
+;; UINT64 RFlags;
+ pop qword [rbp + 40]
+
+;; UINT64 Ldtr, Tr;
+;; UINT64 Gdtr[2], Idtr[2];
+;; Best not let anyone mess with these particular registers...
+ add rsp, 48
+
+;; UINT64 Rip;
+ pop qword [rbp + 24]
+
+;; UINT64 Gs, Fs, Es, Ds, Cs, Ss;
+ pop rax
+ ; mov gs, rax ; not for gs
+ pop rax
+ ; mov fs, rax ; not for fs
+ ; (X64 will not use fs and gs, so we do not restore it)
+ pop rax
+ mov es, rax
+ pop rax
+ mov ds, rax
+ pop qword [rbp + 32] ; for cs
+ pop qword [rbp + 56] ; for ss
+
+;; UINT64 Rdi, Rsi, Rbp, Rsp, Rbx, Rdx, Rcx, Rax;
+;; UINT64 R8, R9, R10, R11, R12, R13, R14, R15;
+ pop rdi
+ pop rsi
+ add rsp, 8 ; not for rbp
+ pop qword [rbp + 48] ; for rsp
+ pop rbx
+ pop rdx
+ pop rcx
+ pop rax
+ pop r8
+ pop r9
+ pop r10
+ pop r11
+ pop r12
+ pop r13
+ pop r14
+ pop r15
+
+ mov rsp, rbp
+ pop rbp
+ add rsp, 16
+ cmp qword [rsp - 32], 0 ; check EXCEPTION_HANDLER_CONTEXT.OldIdtHandler
+ jz DoReturn
+ cmp qword [rsp - 40], 1 ; check EXCEPTION_HANDLER_CONTEXT.ExceptionDataFlag
+ jz ErrorCode
+ jmp qword [rsp - 32]
+ErrorCode:
+ sub rsp, 8
+ jmp qword [rsp - 24]
+
+DoReturn:
+ cmp qword [ASM_PFX(mDoFarReturnFlag)], 0 ; Check if need to do far return instead of IRET
+ jz DoIret
+ push rax
+ mov rax, rsp ; save old RSP to rax
+ mov rsp, [rsp + 0x20]
+ push qword [rax + 0x10] ; save CS in new location
+ push qword [rax + 0x8] ; save EIP in new location
+ push qword [rax + 0x18] ; save EFLAGS in new location
+ mov rax, [rax] ; restore rax
+ popfq ; restore EFLAGS
+ DB 0x48 ; prefix to composite "retq" with next "retf"
+ retf ; far return
+DoIret:
+ iretq
+
+;-------------------------------------------------------------------------------------
+; GetTemplateAddressMap (&AddressMap);
+;-------------------------------------------------------------------------------------
+; comments here for definition of address map
+global ASM_PFX(AsmGetTemplateAddressMap)
+ASM_PFX(AsmGetTemplateAddressMap):
+ lea rax, [AsmIdtVectorBegin]
+ mov qword [rcx], rax
+ mov qword [rcx + 0x8], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32
+ lea rax, [HookAfterStubHeaderBegin]
+ mov qword [rcx + 0x10], rax
+
+; Fix up CommonInterruptEntry address
+ lea rax, [ASM_PFX(CommonInterruptEntry)]
+ lea rcx, [AsmIdtVectorBegin]
+%rep 32
+ mov qword [rcx + (JmpAbsoluteAddress - 8 - HookAfterStubHeaderBegin)], rax
+ add rcx, (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32
+%endrep
+; Fix up HookAfterStubHeaderEnd
+ lea rax, [HookAfterStubHeaderEnd]
+ lea rcx, [JmpAbsoluteAddress]
+ mov qword [rcx - 8], rax
+
+ ret
+
+;-------------------------------------------------------------------------------------
+; AsmVectorNumFixup (*NewVectorAddr, VectorNum, *OldVectorAddr);
+;-------------------------------------------------------------------------------------
+global ASM_PFX(AsmVectorNumFixup)
+ASM_PFX(AsmVectorNumFixup):
+ mov rax, rdx
+ mov [rcx + (@VectorNum - HookAfterStubHeaderBegin)], al
+ ret
+


Re: [PATCH v5][edk2-platforms 00/17] Platform/Arm/Sgi: Add platform support for RD-Daniel

Ard Biesheuvel
 

On 5/5/20 3:01 PM, Aditya Angadi via groups.io wrote:
Changes since v4:
- Addressed all the comments from Ard.
- Split the patches into multiple smaller patches where applicable.
- Reworded the commit messages to be more accurate about the change
being introduced.
- Picked up Ard's review tags
Changes since v3:
- Addressed all the comments from Ard.
- Each platform with SgiPkg would be built independently of the other.
- Removed all references to ACPI tables of platforms that are not part
of the build of a platform.
- Added support for multi-chip NUMA memory nodes.
- Added SRAT table for RdN1EdgeX2 platform.
- Did not pickup the review tags from Ard because the code change from
last version.
This patch series adds support for Arm's RD-Daniel platform. There are two
configurations of this platform being added in this series - Config-M and
Config-XLR. RD-Daniel is the next Arm's reference design subsystem.
Config XLR is a multi chip platform.
The first five patches rework the SgiPkg code to move away from being
able to build a single binary for all the supported platforms to
independent binary for each platform. This was requried as the first
step to add newer RD platform support in SgiPkg as these newer platforms
have differences in the programming interface.
Patches 6 to 11 in the series refactor and consolidate the code in
SgiPkg in preparation for adding support for numa memory nodes and the
RD-Daniel platform.
Patches 12 and 13 in the series add SRAT table for RD-N1-Edge platform
and removes the hard-code chip count on the platform.
The next four patches add support for the RD-Daniel platforms. There are
two configuration of RD-Daniel platform.
- RD-Daniel Config-M: This is single chip platform that includes
16 Neoverse cores.
- RD-Daniel Config-XLR: This is multi-chip platform that includes
four identical chip connected over a coherent link with all the
four chips put together into a single package. Each chip includes
four Neoverse cores and it attached to 8GB of RAM.
Aditya Angadi (15):
Platform/ARM/SgiPkg: Create platform specific dsc files
Platform/ARM/SgiPkg: Let platforms define core and cluster count
Platform/ARM/SgiPkg: Let platforms define GIC related PCD values
Platform/ARM/SgiPkg: Create platform specific fd include file
Platform/ARM/SgiPkg: Let platform specify the ACPI tables to include
Platform/ARM/SgiPkg: Obtain rd-e1-edge platform core count from PCD
Platform/ARM/SgiPkg: Refactor GIC related ACPI helper macros
Platform/ARM/SgiPkg: Update SGI-575 MADT table to ACPI 6.2
Platform/ARM/SgiPkg: Move common platform description to SSDT
Platform/ARM/SgiPkg: Add helper macros for SRAT table
Platform/ARM/SgiPkg: Use chip count constant on rdn1edgex2 platform
Platform/ARM/SgiPkg: Add ACPI tables for Rd-Daniel Config-M
Platform/ARM/SgiPkg: Add initial support for RD-Daniel Config-M
platform
Platform/ARM/SgiPkg: Add ACPI tables for RD-Daniel Config-XLR
Platform/ARM/SgiPkg: Add initial support for RD-Daniel Config-XLR
platform
Vijayenthiran Subramaniam (2):
Platform/ARM/SgiPkg: Add support for remote numa memory nodes
Platform/ARM/SgiPkg: Add SRAT table for RdN1Edge dual-chip platform
For the series,

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

Pushed as 03fb86be4146..ca1a7187861c

Thanks!


Re: [PATCH edk2-platforms 4/5] Platform/RaspberryPi3: query firmware for 16550 input clock at boot

Ard Biesheuvel
 

On 5/5/20 4:50 PM, Ard Biesheuvel wrote:
Query the firmware for the clock rate that is used to drive the
16550 baud clock, so that we can program the correct baud rate.
Co-authored-by: Pete Batard <pete@akeo.ie>
Co-authored-by: Andrei Warkentin <andrey.warkentin@gmail.com>
Co-authored-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Pete Batard <pete@akeo.ie>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S | 25 +++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
index 91dfe1bb981e..35580e4ed73a 100644
--- a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
+++ b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
@@ -3,7 +3,7 @@
* Copyright (c) 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
* Copyright (c) 2019-2020, Pete Batard <pete@akeo.ie>
* Copyright (c) 2016, Linaro Limited. All rights reserved.
- * Copyright (c) 2011-2013, ARM Limited. All rights reserved.
+ * Copyright (c) 2011-2020, ARM Limited. All rights reserved.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*
@@ -85,6 +85,14 @@ ASM_FUNC (ArmPlatformPeiBootAction)
adr x2, mBoardRevision
str w0, [x2]
+#if (RPI_MODEL == 3)
As noted by Pete off-list, doing this doesn't work unless we add something like

GCC:*_*_*_PP_FLAGS = -DRPI_MODEL=3

to the [BuildOptions] in RPi3.dsc



+ run .Lclkinfo_buffer
+
+ ldr w0, .Lfrequency
+ adrp x2, _gPcd_BinaryPatch_PcdSerialClockRate
+ str w0, [x2, :lo12:_gPcd_BinaryPatch_PcdSerialClockRate]
+#endif
+
ret
.align 4
@@ -127,6 +135,21 @@ ASM_FUNC (ArmPlatformPeiBootAction)
.long 0 // end tag
.set .Lrevinfo_size, . - .Lrevinfo_buffer
+#if (RPI_MODEL == 3)
+ .align 4
+.Lclkinfo_buffer:
+ .long .Lclkinfo_size
+ .long 0x0
+ .long RPI_MBOX_GET_CLOCK_RATE
+ .long 8 // buf size
+ .long 4 // input len
+ .long 4 // clock id: 0x04 = Core/VPU
+.Lfrequency:
+ .long 0 // frequency
+ .long 0 // end tag
+ .set .Lclkinfo_size, . - .Lclkinfo_buffer
+#endif
+
//UINTN
//ArmPlatformGetPrimaryCoreMpId (
// VOID


Re: [PATCH edk2-platforms 3/5] Platform/RaspberryPi: fix 16550 divisor calculation logic

Ard Biesheuvel
 

On 5/5/20 4:50 PM, Ard Biesheuvel wrote:
The 16550 'miniUART' on the Raspberry Pi gets its input clock from
different sources on RPi3 and RPi3. Fix the logic that derives the
This should be 'Rpi3 and RPi4'

divisor for the 16550 baud clock on the respective platforms.
While at it, make the input clock PCD patchable for RPi3 so we can
manipulate it at runtime in a future patch.
Co-authored-by: Pete Batard <pete@akeo.ie>
Co-authored-by: Andrei Warkentin <andrey.warkentin@gmail.com>
Co-authored-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Pete Batard <pete@akeo.ie>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
Platform/RaspberryPi/RPi3/RPi3.dsc | 4 +++-
Platform/RaspberryPi/RPi4/RPi4.dsc | 2 +-
Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c | 14 ++++++++------
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
index d7218219fc5a..96b27400eef8 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -409,7 +409,6 @@ [PcdsFixedAtBuild.common]
gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4
- gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
@@ -441,6 +440,9 @@ [PcdsFixedAtBuild.common]
gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"EDK2"
gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
+[PcdsPatchableInModule]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|250000000
+
[PcdsDynamicHii.common.DEFAULT]
#
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 4fb015b077e6..5d8bd88d7e34 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -420,7 +420,7 @@ [PcdsFixedAtBuild.common]
gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4
- gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1000000000
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
index b1d17d3fa04a..5e83bbf022eb 100644
--- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
+++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
@@ -41,18 +41,20 @@ SerialPortGetDivisor (
// On the Raspberry Pi, the clock to use for the 16650-compatible UART
// is the base clock divided by the 12.12 fixed point VPU clock divisor.
//
- BaseClockRate = (UINT64)PcdGet32 (PcdSerialClockRate) * 4;
+ BaseClockRate = (UINT64)PcdGet32 (PcdSerialClockRate);
+#if (RPI_MODEL == 4)
Divisor = MmioRead32(BCM2836_CM_BASE + BCM2836_CM_VPU_CLOCK_DIVISOR) & 0xFFFFFF;
if (Divisor != 0)
BaseClockRate = (BaseClockRate << 12) / Divisor;
+#endif
//
- // Now calculate divisor for baud generator
- // Ref_Clk_Rate / Baud_Rate / 16
+ // As per the BCM2xxx datasheets:
+ // baudrate = system_clock_freq / (8 * (divisor + 1)).
//
- Divisor = (UINT32)BaseClockRate / (SerialBaudRate * 16);
- if (((UINT32)BaseClockRate % (SerialBaudRate * 16)) >= SerialBaudRate * 8) {
- Divisor++;
+ Divisor = (UINT32)BaseClockRate / (SerialBaudRate * 8);
+ if (Divisor != 0) {
+ Divisor--;
}
return Divisor;
}


Re: [edk2-staging/EdkRepo] [PATCH v1 0/3] Manifest parser cleanup

Bjorge, Erik C
 

Nate, if you run into this issue again please let me know and I can send out a new version.

Thanks,
-Erik

-----Original Message-----
From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Sent: Monday, May 4, 2020 8:59 PM
To: Bjorge, Erik C <erik.c.bjorge@intel.com>; devel@edk2.groups.io
Cc: Desimone, Ashley E <ashley.e.desimone@intel.com>; Pandya, Puja <puja.pandya@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Agyeman, Prince <prince.agyeman@intel.com>
Subject: RE: [edk2-staging/EdkRepo] [PATCH v1 0/3] Manifest parser cleanup

After a some rebasing pain... Pushed as f2f6f5a2~.. 6153e4a2

-----Original Message-----
From: Bjorge, Erik C <erik.c.bjorge@intel.com>
Sent: Thursday, April 30, 2020 3:53 PM
To: devel@edk2.groups.io
Cc: Desimone, Ashley E <ashley.e.desimone@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Pandya, Puja <puja.pandya@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>; Agyeman, Prince <prince.agyeman@intel.com>
Subject: [edk2-staging/EdkRepo] [PATCH v1 0/3] Manifest parser cleanup

General cleanup of the manifest parser and minor improvements to make the code more flexible.

Cc: Ashley E Desimone <ashley.e.desimone@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Puja Pandya <puja.pandya@intel.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Prince Agyeman <prince.agyeman@intel.com>

Erik Bjorge (3):
EdkRepo: Flake8 cleanup
EdkRepo: Improve error message with invalid XML
EdkRepo: Refactoring private class members

edkrepo_manifest_parser/edk_manifest.py | 249 +++++++++++++-----------
1 file changed, 139 insertions(+), 110 deletions(-)

--
2.21.0.windows.1


Re: [PATCH v3 1/1] OvmfPkg: Add QemuFwCfgLibNull

Laszlo Ersek
 

On 05/04/20 13:45, Philippe Mathieu-Daudé wrote:
On 5/4/20 1:09 AM, Rebecca Cran wrote:
Add a null implementation library for QemuFwCfgLib, in order to
support building PciHostBridgeLib for bhyve.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
---
I think I've addressed all previous review feedback this time.

  .../Library/QemuFwCfgLib/QemuFwCfgLibNull.inf |  30 +++
  OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c  | 209 ++++++++++++++++++
  2 files changed, 239 insertions(+)
  create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibNull.inf
  create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>

github.com pull request: https://github.com/tianocore/edk2/pull/578

commit: 245bdd2cb94beef807ff12263b6060d6fdb40c9a

Thanks,
Laszlo


[PATCH] Platform/SoftIron/Overdrive1000Board: remove PcdEnableKcs

kettenis@...
 

From: Mark Kettenis <kettenis@xs4all.nl>

The SoftIron Overdrive 1000 does not have a BMC so don't advertise
the IPMI KCS interface.

Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
---
Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc | 2 --
1 file changed, 2 deletions(-)

diff --git a/Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc b/=
Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc
index 005ffad..5cf865d 100644
--- a/Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc
+++ b/Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc
@@ -405,8 +405,6 @@ DEFINE NUM_CORES =3D 4
# map the stack as non-executable when entering the DXE phase=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE=0D
=0D
- gAmdStyxTokenSpaceGuid.PcdEnableKcs|TRUE=0D
-=0D
[PcdsPatchableInModule]=0D
# PCIe Configuration: x4x2x2 (=3D2 See Include/FDKGionb.h)=0D
gAmdModulePkgTokenSpaceGuid.PcdPcieCoreConfiguration|2=0D
--=20
1.9.1


Re: [PATCH 1/1] Update mailing list links from lists.01.org to groups.io

Laszlo Ersek
 

On 05/04/20 13:48, Philippe Mathieu-Daudé wrote:
On 4/30/20 11:06 PM, Rebecca Cran wrote:
I only updated links to the top-level mailing list, not any links to
individual messages.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
---
  HardFeatureFreeze.md                                          | 2 +-
  ...unkempt-git-guide-for-edk2-contributors-and-maintainers.md | 2 +-
  Member-FAQ.md                                                 | 4 ++--
  SoftFeatureFreeze.md                                          | 2 +-
  4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/HardFeatureFreeze.md b/HardFeatureFreeze.md
index dae9f664b5d9..e0334f69ed7e 100644
--- a/HardFeatureFreeze.md
+++ b/HardFeatureFreeze.md
@@ -10,7 +10,7 @@
freeze](https://wiki.qemu.org/Planning/HardFeatureFreeze)).
  The proposed schedule for the active development cycle is shown in
the [EDK II
  Release Planning](EDK-II-Release-Planning) article. Shortly before
the hard
  feature freeze, an announcement email is sent to the
-[edk2-devel](https://lists.01.org/mailman/listinfo/edk2-devel)
mailing list.
+[edk2-devel](https://edk2.groups.io/g/devel) mailing list.
  The announcement is posted by one of the maintainers that are listed in
 
[Maintainers.txt](https://github.com/tianocore/edk2/blob/master/Maintainers.txt),

  in section `EDK II Releases`.
diff --git
a/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers.md
b/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers.md
index 3469ef7b1845..6f85c782a710 100644
--- a/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers.md
+++ b/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers.md
@@ -80,7 +80,7 @@ Contributor workflow
       git config notes.rewriteRef       refs/notes/commits
       git config sendemail.chainreplyto false
       git config sendemail.thread       true
-     git config sendemail.to           edk2-devel@lists.01.org
+     git config sendemail.to           devel@edk2.groups.io
       ```
    6.   <a name="contrib-06" href="#contrib-06">&sect;</a>
diff --git a/Member-FAQ.md b/Member-FAQ.md
index f3e5798f23ae..f277744ff6ce 100644
--- a/Member-FAQ.md
+++ b/Member-FAQ.md
@@ -7,6 +7,6 @@ TianoCore has accumulated a lot of information over
the years. We keep several F
  * [[Terms and Acronyms|Acronyms and Glossary]]
  * [[EDK II Documents]]
  -If you have a question and cannot find the answer, please try the
[EDK II developer e-mail
list](https://github.com/tianocore/tianocore.github.io/wiki/edk2-devel).
You can also [search the e-mail list
archive](https://lists.01.org/pipermail/edk2-devel/) for questions
already asked in email.
+If you have a question and cannot find the answer, please try the
[EDK II developer e-mail
list](https://github.com/tianocore/tianocore.github.io/wiki/edk2-devel).
You can also [search the e-mail list
archive](https://edk2.groups.io/g/devel) for questions already asked
in email.
  -_Note that e-mails prior to September 2015 are on a [sourceforge
archive](https://sourceforge.net/p/edk2/mailman/edk2-devel/)._
\ No newline at end of file
+_Note that e-mails prior to September 2015 are on a [sourceforge
archive](https://sourceforge.net/p/edk2/mailman/edk2-devel/)._
diff --git a/SoftFeatureFreeze.md b/SoftFeatureFreeze.md
index 9047a73c1d69..7a0f40bbeaf4 100644
--- a/SoftFeatureFreeze.md
+++ b/SoftFeatureFreeze.md
@@ -46,7 +46,7 @@
freeze](https://wiki.qemu.org/Planning/SoftFeatureFreeze).)
  The proposed schedule for the active development cycle is shown in
the [EDK II
  Release Planning](EDK-II-Release-Planning) article. Shortly before
the soft
  feature freeze, an announcement email is sent to the
-[edk2-devel](https://lists.01.org/mailman/listinfo/edk2-devel)
mailing list.
+[edk2-devel](https://edk2.groups.io/g/devel) mailing list.
  The announcement is posted by one of the maintainers that are listed in
 
[Maintainers.txt](https://github.com/tianocore/edk2/blob/master/Maintainers.txt),

  in section `EDK II Releases`.
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>

edk2-wiki commit ae81cb24e5ba.

Thanks!
Laszlo


Re: [PATCH] Platform/SoftIron/Overdrive1000Board: remove PcdEnableKcs

Ard Biesheuvel
 

On 5/5/20 6:02 PM, kettenis@xs4all.nl wrote:
From: Mark Kettenis <kettenis@xs4all.nl>
The SoftIron Overdrive 1000 does not have a BMC so don't advertise
the IPMI KCS interface.
Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
Thanks Mark

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

Pushed as 72d6a1e804cb..03fb86be4146

---
Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc | 2 --
1 file changed, 2 deletions(-)
diff --git a/Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc b/Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc
index 005ffad..5cf865d 100644
--- a/Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc
+++ b/Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc
@@ -405,8 +405,6 @@ DEFINE NUM_CORES = 4
# map the stack as non-executable when entering the DXE phase
gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
- gAmdStyxTokenSpaceGuid.PcdEnableKcs|TRUE
-
[PcdsPatchableInModule]
# PCIe Configuration: x4x2x2 (=2 See Include/FDKGionb.h)
gAmdModulePkgTokenSpaceGuid.PcdPcieCoreConfiguration|2


[PATCH v1 6/6] ShellPkg: acpiview: Make PPTT parsing logic data driven

Krzysztof Koch
 

Replace the switch statement in the main parser loop with a table-driven
approach. Use the ParseAcpiStruct () method to resolve how each
Processor Topology Structure given should be parsed.

Enumerate all structures found in the Processor Properties Topology
Table (PPTT) on a per-type basis.

Consolidate all metadata about each Processor Topology Structure.
Define an array of ACPI_STRUCT_INFO structures to store the name,
instance count, architecture support and handling information. Use this
array to construct the ACPI_STRUCT_DATABASE for PPTT.

Count the number of instances of each Processor Topology Structure type.
Optionally report these counts after PPTT table parsing is finished.

Remove the definition of the DumpCacheTypeStructure and DumpIDStructure
functions. Their only purpose was to call ParseAcpi () and now this
process is streamlined.

Make DumpProcessorHierarchyNodeStructure function signature match that
of ACPI_STRUCT_PARSER_FUNC. This way, the function can be called from
ParseAcpiStruct ().

References:
- ACPI 6.3 Specification - January 2019, Section 5.2.29

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
v1:
- Make PPTT parsing logic data driven [Krzysztof]

ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c | 152 ++++++++++----------
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.h | 2 +-
2 files changed, 74 insertions(+), 80 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
index 0db272c16af0ad8824c8da4c88dd409c8550112a..b62f79b52cab989942f84b020e1d737e8ef65439 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
@@ -21,6 +21,11 @@ STATIC CONST UINT8* ProcessorTopologyStructureLength;
STATIC CONST UINT32* NumberOfPrivateResources;
STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;

+/**
+ Handler for each Processor Topology Structure
+**/
+STATIC ACPI_STRUCT_INFO PpttStructs[];
+
/**
This function validates the Cache Type Structure (Type 1) 'Number of sets'
field.
@@ -243,22 +248,34 @@ STATIC CONST ACPI_PARSER IdStructureParser[] = {
@param [in] Ptr Pointer to the start of the Processor Hierarchy Node
Structure data.
@param [in] Length Length of the Processor Hierarchy Node Structure.
+ @param [in] OptArg0 First optional argument (Not used).
+ @param [in] OptArg1 Second optional argument (Not used).
**/
STATIC
VOID
DumpProcessorHierarchyNodeStructure (
- IN UINT8* Ptr,
- IN UINT8 Length
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0 OPTIONAL,
+ IN CONST VOID* OptArg1 OPTIONAL
)
{
UINT32 Offset;
UINT32 Index;
CHAR16 Buffer[OUTPUT_FIELD_COLUMN_WIDTH];
+ CHAR8 AsciiBuffer[80];
+
+ PrintAcpiStructName (
+ PpttStructs[EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR].Name,
+ PpttStructs[EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR].Count,
+ sizeof (AsciiBuffer),
+ AsciiBuffer
+ );

Offset = ParseAcpi (
TRUE,
2,
- "Processor Hierarchy Node Structure",
+ AsciiBuffer,
Ptr,
Length,
PARSER_PARAMS (ProcessorHierarchyNodeStructureParser)
@@ -269,7 +286,8 @@ DumpProcessorHierarchyNodeStructure (
if (NumberOfPrivateResources == NULL) {
IncrementErrorCount ();
Print (
- L"ERROR: Insufficient Processor Hierarchy Node length. Length = %d.\n",
+ L"ERROR: Insufficient %a Structure length. Length = %d.\n",
+ PpttStructs[EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR].Name,
Length
);
return;
@@ -296,7 +314,7 @@ DumpProcessorHierarchyNodeStructure (
UnicodeSPrint (
Buffer,
sizeof (Buffer),
- L"Private resources [%d]",
+ L"Private resource [%d]",
Index
);

@@ -312,50 +330,37 @@ DumpProcessorHierarchyNodeStructure (
}

/**
- This function parses the Cache Type Structure (Type 1).
-
- @param [in] Ptr Pointer to the start of the Cache Type Structure data.
- @param [in] Length Length of the Cache Type Structure.
+ Information about each Processor Topology Structure type.
**/
-STATIC
-VOID
-DumpCacheTypeStructure (
- IN UINT8* Ptr,
- IN UINT8 Length
- )
-{
- ParseAcpi (
- TRUE,
- 2,
- "Cache Type Structure",
- Ptr,
- Length,
- PARSER_PARAMS (CacheTypeStructureParser)
- );
-}
+STATIC ACPI_STRUCT_INFO PpttStructs[] = {
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "Processor",
+ EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64 | ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpProcessorHierarchyNodeStructure
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "Cache",
+ EFI_ACPI_6_3_PPTT_TYPE_CACHE,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64 | ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ CacheTypeStructureParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "ID",
+ EFI_ACPI_6_3_PPTT_TYPE_ID,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64 | ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ IdStructureParser
+ )
+};

/**
- This function parses the ID Structure (Type 2).
-
- @param [in] Ptr Pointer to the start of the ID Structure data.
- @param [in] Length Length of the ID Structure.
+ PPTT structure database
**/
-STATIC
-VOID
-DumpIDStructure (
- IN UINT8* Ptr,
- IN UINT8 Length
- )
-{
- ParseAcpi (
- TRUE,
- 2,
- "ID Structure",
- Ptr,
- Length,
- PARSER_PARAMS (IdStructureParser)
- );
-}
+STATIC ACPI_STRUCT_DATABASE PpttDatabase = {
+ "Processor Topology Structure",
+ PpttStructs,
+ ARRAY_SIZE (PpttStructs)
+};

/**
This function parses the ACPI PPTT table.
@@ -390,6 +395,8 @@ ParseAcpiPptt (
return;
}

+ ResetAcpiStructCounts (&PpttDatabase);
+
Offset = ParseAcpi (
TRUE,
0,
@@ -419,7 +426,8 @@ ParseAcpiPptt (
IncrementErrorCount ();
Print (
L"ERROR: Insufficient remaining table buffer length to read the " \
- L"processor topology structure header. Length = %d.\n",
+ L"%a header. Length = %d.\n",
+ PpttDatabase.Name,
AcpiTableLength - Offset
);
return;
@@ -430,8 +438,9 @@ ParseAcpiPptt (
((Offset + (*ProcessorTopologyStructureLength)) > AcpiTableLength)) {
IncrementErrorCount ();
Print (
- L"ERROR: Invalid Processor Topology Structure length. " \
- L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
+ L"ERROR: Invalid %a length. Length = %d. Offset = %d. " \
+ L"AcpiTableLength = %d.\n",
+ PpttDatabase.Name,
*ProcessorTopologyStructureLength,
Offset,
AcpiTableLength
@@ -439,39 +448,24 @@ ParseAcpiPptt (
return;
}

- PrintFieldName (2, L"* Structure Offset *");
- Print (L"0x%x\n", Offset);
-
- switch (*ProcessorTopologyStructureType) {
- case EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR:
- DumpProcessorHierarchyNodeStructure (
- ProcessorTopologyStructurePtr,
- *ProcessorTopologyStructureLength
- );
- break;
- case EFI_ACPI_6_2_PPTT_TYPE_CACHE:
- DumpCacheTypeStructure (
- ProcessorTopologyStructurePtr,
- *ProcessorTopologyStructureLength
- );
- break;
- case EFI_ACPI_6_2_PPTT_TYPE_ID:
- DumpIDStructure (
- ProcessorTopologyStructurePtr,
- *ProcessorTopologyStructureLength
- );
- break;
- default:
- IncrementErrorCount ();
- Print (
- L"ERROR: Unknown processor topology structure:"
- L" Type = %d, Length = %d\n",
- *ProcessorTopologyStructureType,
- *ProcessorTopologyStructureLength
- );
- }
+ // Parse the Processor Topology Structure
+ ParseAcpiStruct (
+ 2,
+ ProcessorTopologyStructurePtr,
+ &PpttDatabase,
+ Offset,
+ *ProcessorTopologyStructureType,
+ *ProcessorTopologyStructureLength,
+ NULL,
+ NULL
+ );

ProcessorTopologyStructurePtr += *ProcessorTopologyStructureLength;
Offset += *ProcessorTopologyStructureLength;
} // while
+
+ // Report and validate processor topology structure counts
+ if (GetConsistencyChecking ()) {
+ ValidateAcpiStructCounts (&PpttDatabase);
+ }
}
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.h
index 2a671203fb0035bbc407ff4bb0ca9960706fa588..a0b0622ba00bda63379b670527145359b738bb7d 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.h
@@ -1,7 +1,7 @@
/** @file
Header file for PPTT parser

- Copyright (c) 2019, ARM Limited. All rights reserved.
+ Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent

@par Reference(s):
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH v1 5/6] ShellPkg: acpiview: Make IORT parsing logic data driven

Krzysztof Koch
 

Replace the switch statement in the main parser loop with a table-driven
approach. Use the ParseAcpiStruct () method to resolve how each IORT
Node given should be parsed.

Enumerate all structures found in the IO Remapping Table (IORT) on a
per-type basis. Replace calls to AsciiSPrint () with the
PrintAcpiStructName () function.

Consolidate all metadata about each IORT Node. Define an array of
ACPI_STRUCT_INFO structures to store the name, instance count,
architecture support and handling information. Use this array to
construct the ACPI_STRUCT_DATABASE for IORT.

Count the number of instances of each IORT Node. Optionally report
these counts after IORT table parsing is finished.

Modify dedicated functions for parsing each IORT Node type such that
their signatures match ACPI_STRUCT_PARSER_FUNC. This way, they can be
used in the ParseAcpiStruct () call.

References:
- IO Remapping Table - Platform Design Document, Issue D, March 2018

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
v1:
- Make IORT parsing logic data driven [Krzysztof]

ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c | 353 +++++++++++++-------
1 file changed, 234 insertions(+), 119 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
index 9a006a01448b897865cd7cd85651c816933acf05..0c40447b4363f10c7ea5c9eeb283cebeab24243a 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
@@ -9,10 +9,12 @@
**/

#include <IndustryStandard/IoRemappingTable.h>
+#include <Library/DebugLib.h>
#include <Library/PrintLib.h>
#include <Library/UefiLib.h>
#include "AcpiParser.h"
#include "AcpiTableParser.h"
+#include "AcpiView.h"

// Local variables
STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
@@ -32,6 +34,11 @@ STATIC CONST UINT32* PmuInterruptOffset;

STATIC CONST UINT32* ItsCount;

+/**
+ Handler for each IORT Node type
+**/
+STATIC ACPI_STRUCT_INFO IortStructs[];
+
/**
This function validates the ID Mapping array count for the ITS node.

@@ -273,12 +280,7 @@ DumpIortNodeIdMappings (

while ((Index < MappingCount) &&
(Offset < Length)) {
- AsciiSPrint (
- Buffer,
- sizeof (Buffer),
- "ID Mapping [%d]",
- Index
- );
+ PrintAcpiStructName ("ID Mapping", Index, sizeof (Buffer), Buffer);
Offset += ParseAcpi (
TRUE,
4,
@@ -296,27 +298,42 @@ DumpIortNodeIdMappings (

@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
- @param [in] MappingCount The ID Mapping count.
- @param [in] MappingOffset The offset of the ID Mapping array
+ @param [in] OptArg0 The ID Mapping count.
+ @param [in] OptArg1 The offset of the ID Mapping array
from the start of the IORT table.
**/
STATIC
VOID
DumpIortNodeSmmuV1V2 (
- IN UINT8* Ptr,
- IN UINT16 Length,
- IN UINT32 MappingCount,
- IN UINT32 MappingOffset
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0,
+ IN CONST VOID* OptArg1
)
{
UINT32 Index;
UINT32 Offset;
- CHAR8 Buffer[50]; // Used for AsciiName param of ParseAcpi
+ CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
+ UINT32 MappingCount;
+ UINT32 MappingOffset;
+
+ ASSERT (OptArg0 != NULL);
+ ASSERT (OptArg1 != NULL);
+
+ MappingCount = *((UINT32*)OptArg0);
+ MappingOffset = *((UINT32*)OptArg1);
+
+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_SMMUv1v2].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_SMMUv1v2].Count,
+ sizeof (Buffer),
+ Buffer
+ );

ParseAcpi (
TRUE,
2,
- "SMMUv1 or SMMUv2 Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodeSmmuV1V2Parser)
@@ -330,7 +347,8 @@ DumpIortNodeSmmuV1V2 (
(PmuInterruptOffset == NULL)) {
IncrementErrorCount ();
Print (
- L"ERROR: Insufficient SMMUv1/2 node length. Length = %d\n",
+ L"ERROR: Insufficient %a Node length. Length = %d\n",
+ IortStructs[EFI_ACPI_IORT_TYPE_SMMUv1v2].Name,
Length
);
return;
@@ -341,11 +359,11 @@ DumpIortNodeSmmuV1V2 (

while ((Index < *InterruptContextCount) &&
(Offset < Length)) {
- AsciiSPrint (
- Buffer,
+ PrintAcpiStructName (
+ "Context Interrupts Array",
+ Index,
sizeof (Buffer),
- "Context Interrupts Array [%d]",
- Index
+ Buffer
);
Offset += ParseAcpi (
TRUE,
@@ -363,11 +381,11 @@ DumpIortNodeSmmuV1V2 (

while ((Index < *PmuInterruptCount) &&
(Offset < Length)) {
- AsciiSPrint (
- Buffer,
+ PrintAcpiStructName (
+ "PMU Interrupts Array",
+ Index,
sizeof (Buffer),
- "PMU Interrupts Array [%d]",
- Index
+ Buffer
);
Offset += ParseAcpi (
TRUE,
@@ -392,23 +410,40 @@ DumpIortNodeSmmuV1V2 (

@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
- @param [in] MappingCount The ID Mapping count.
- @param [in] MappingOffset The offset of the ID Mapping array
+ @param [in] OptArg0 The ID Mapping count.
+ @param [in] OptArg1 The offset of the ID Mapping array
from the start of the IORT table.
**/
STATIC
VOID
DumpIortNodeSmmuV3 (
- IN UINT8* Ptr,
- IN UINT16 Length,
- IN UINT32 MappingCount,
- IN UINT32 MappingOffset
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0,
+ IN CONST VOID* OptArg1
)
{
+ UINT32 MappingCount;
+ UINT32 MappingOffset;
+ CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
+
+ ASSERT (OptArg0 != NULL);
+ ASSERT (OptArg1 != NULL);
+
+ MappingCount = *((UINT32*)OptArg0);
+ MappingOffset = *((UINT32*)OptArg1);
+
+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_SMMUv3].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_SMMUv3].Count,
+ sizeof (Buffer),
+ Buffer
+ );
+
ParseAcpi (
TRUE,
2,
- "SMMUV3 Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodeSmmuV3Parser)
@@ -424,24 +459,37 @@ DumpIortNodeSmmuV3 (
/**
This function parses the IORT ITS node.

+ ITS nodes have no ID mappings.
+
@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
+ @param [in] OptArg0 First optional argument (Not used).
+ @param [in] OptArg1 Second optional argument (Not used)..
**/
STATIC
VOID
DumpIortNodeIts (
- IN UINT8* Ptr,
- IN UINT16 Length
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0 OPTIONAL,
+ IN CONST VOID* OptArg1 OPTIONAL
)
{
UINT32 Offset;
UINT32 Index;
CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi

+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_ITS_GROUP].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_ITS_GROUP].Count,
+ sizeof (Buffer),
+ Buffer
+ );
+
Offset = ParseAcpi (
TRUE,
2,
- "ITS Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodeItsParser)
@@ -452,7 +500,8 @@ DumpIortNodeIts (
if (ItsCount == NULL) {
IncrementErrorCount ();
Print (
- L"ERROR: Insufficient ITS group length. Length = %d.\n",
+ L"ERROR: Insufficient %a Node length. Length = %d.\n",
+ IortStructs[EFI_ACPI_IORT_TYPE_ITS_GROUP].Name,
Length
);
return;
@@ -462,11 +511,11 @@ DumpIortNodeIts (

while ((Index < *ItsCount) &&
(Offset < Length)) {
- AsciiSPrint (
- Buffer,
+ PrintAcpiStructName (
+ "GIC ITS Identifier Array",
+ Index,
sizeof (Buffer),
- "GIC ITS Identifier Array [%d]",
- Index
+ Buffer
);
Offset += ParseAcpi (
TRUE,
@@ -488,25 +537,41 @@ DumpIortNodeIts (

@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
- @param [in] MappingCount The ID Mapping count.
- @param [in] MappingOffset The offset of the ID Mapping array
+ @param [in] OptArg0 The ID Mapping count.
+ @param [in] OptArg1 The offset of the ID Mapping array
from the start of the IORT table.
**/
STATIC
VOID
DumpIortNodeNamedComponent (
- IN UINT8* Ptr,
- IN UINT16 Length,
- IN UINT32 MappingCount,
- IN UINT32 MappingOffset
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0,
+ IN CONST VOID* OptArg1
)
{
UINT32 Offset;
+ UINT32 MappingCount;
+ UINT32 MappingOffset;
+ CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
+
+ ASSERT (OptArg0 != NULL);
+ ASSERT (OptArg1 != NULL);
+
+ MappingCount = *((UINT32*)OptArg0);
+ MappingOffset = *((UINT32*)OptArg1);
+
+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_NAMED_COMP].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_NAMED_COMP].Count,
+ sizeof (Buffer),
+ Buffer
+ );

Offset = ParseAcpi (
TRUE,
2,
- "Named Component Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodeNamedComponentParser)
@@ -534,23 +599,40 @@ DumpIortNodeNamedComponent (

@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
- @param [in] MappingCount The ID Mapping count.
- @param [in] MappingOffset The offset of the ID Mapping array
+ @param [in] OptArg0 The ID Mapping count.
+ @param [in] OptArg1 The offset of the ID Mapping array
from the start of the IORT table.
**/
STATIC
VOID
DumpIortNodeRootComplex (
- IN UINT8* Ptr,
- IN UINT16 Length,
- IN UINT32 MappingCount,
- IN UINT32 MappingOffset
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0,
+ IN CONST VOID* OptArg1
)
{
+ UINT32 MappingCount;
+ UINT32 MappingOffset;
+ CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
+
+ ASSERT (OptArg0 != NULL);
+ ASSERT (OptArg1 != NULL);
+
+ MappingCount = *((UINT32*)OptArg0);
+ MappingOffset = *((UINT32*)OptArg1);
+
+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_ROOT_COMPLEX].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_ROOT_COMPLEX].Count,
+ sizeof (Buffer),
+ Buffer
+ );
+
ParseAcpi (
TRUE,
2,
- "Root Complex Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodeRootComplexParser)
@@ -568,23 +650,40 @@ DumpIortNodeRootComplex (

@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
- @param [in] MappingCount The ID Mapping count.
- @param [in] MappingOffset The offset of the ID Mapping array
+ @param [in] OptArg0 The ID Mapping count.
+ @param [in] OptArg1 The offset of the ID Mapping array
from the start of the IORT table.
**/
STATIC
VOID
DumpIortNodePmcg (
- IN UINT8* Ptr,
- IN UINT16 Length,
- IN UINT32 MappingCount,
- IN UINT32 MappingOffset
-)
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0,
+ IN CONST VOID* OptArg1
+ )
{
+ UINT32 MappingCount;
+ UINT32 MappingOffset;
+ CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
+
+ ASSERT (OptArg0 != NULL);
+ ASSERT (OptArg1 != NULL);
+
+ MappingCount = *((UINT32*)OptArg0);
+ MappingOffset = *((UINT32*)OptArg1);
+
+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_PMCG].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_PMCG].Count,
+ sizeof (Buffer),
+ Buffer
+ );
+
ParseAcpi (
TRUE,
2,
- "PMCG Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodePmcgParser)
@@ -597,6 +696,57 @@ DumpIortNodePmcg (
);
}

+/**
+ Information about each IORT Node type
+**/
+STATIC ACPI_STRUCT_INFO IortStructs[] = {
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "ITS Group",
+ EFI_ACPI_IORT_TYPE_ITS_GROUP,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodeIts
+ ),
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "Named Component",
+ EFI_ACPI_IORT_TYPE_NAMED_COMP,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodeNamedComponent
+ ),
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "Root Complex",
+ EFI_ACPI_IORT_TYPE_ROOT_COMPLEX,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodeRootComplex
+ ),
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "SMMUv1 or SMMUv2",
+ EFI_ACPI_IORT_TYPE_SMMUv1v2,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodeSmmuV1V2
+ ),
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "SMMUv3",
+ EFI_ACPI_IORT_TYPE_SMMUv3,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodeSmmuV3
+ ),
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "PMCG",
+ EFI_ACPI_IORT_TYPE_PMCG,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodePmcg
+ )
+};
+
+/**
+ IORT structure database
+**/
+STATIC ACPI_STRUCT_DATABASE IortDatabase = {
+ "IORT Node",
+ IortStructs,
+ ARRAY_SIZE (IortStructs)
+};
+
/**
This function parses the ACPI IORT table.
When trace is enabled this function parses the IORT table and traces the ACPI fields.
@@ -633,6 +783,8 @@ ParseAcpiIort (
return;
}

+ ResetAcpiStructCounts (&IortDatabase);
+
ParseAcpi (
TRUE,
0,
@@ -666,7 +818,7 @@ ParseAcpiIort (
ParseAcpi (
FALSE,
0,
- "IORT Node Header",
+ NULL,
NodePtr,
AcpiTableLength - Offset,
PARSER_PARAMS (IortNodeHeaderParser)
@@ -681,7 +833,8 @@ ParseAcpiIort (
IncrementErrorCount ();
Print (
L"ERROR: Insufficient remaining table buffer length to read the " \
- L"IORT node header. Length = %d.\n",
+ L"%a header. Length = %d.\n",
+ IortDatabase.Name,
AcpiTableLength - Offset
);
return;
@@ -692,8 +845,9 @@ ParseAcpiIort (
((Offset + (*IortNodeLength)) > AcpiTableLength)) {
IncrementErrorCount ();
Print (
- L"ERROR: Invalid IORT Node length. " \
- L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
+ L"ERROR: Invalid %a length. Length = %d. Offset = %d. " \
+ L"AcpiTableLength = %d.\n",
+ IortDatabase.Name,
*IortNodeLength,
Offset,
AcpiTableLength
@@ -701,63 +855,24 @@ ParseAcpiIort (
return;
}

- PrintFieldName (2, L"* Node Offset *");
- Print (L"0x%x\n", Offset);
-
- switch (*IortNodeType) {
- case EFI_ACPI_IORT_TYPE_ITS_GROUP:
- DumpIortNodeIts (
- NodePtr,
- *IortNodeLength
- );
- break;
- case EFI_ACPI_IORT_TYPE_NAMED_COMP:
- DumpIortNodeNamedComponent (
- NodePtr,
- *IortNodeLength,
- *IortIdMappingCount,
- *IortIdMappingOffset
- );
- break;
- case EFI_ACPI_IORT_TYPE_ROOT_COMPLEX:
- DumpIortNodeRootComplex (
- NodePtr,
- *IortNodeLength,
- *IortIdMappingCount,
- *IortIdMappingOffset
- );
- break;
- case EFI_ACPI_IORT_TYPE_SMMUv1v2:
- DumpIortNodeSmmuV1V2 (
- NodePtr,
- *IortNodeLength,
- *IortIdMappingCount,
- *IortIdMappingOffset
- );
- break;
- case EFI_ACPI_IORT_TYPE_SMMUv3:
- DumpIortNodeSmmuV3 (
- NodePtr,
- *IortNodeLength,
- *IortIdMappingCount,
- *IortIdMappingOffset
- );
- break;
- case EFI_ACPI_IORT_TYPE_PMCG:
- DumpIortNodePmcg (
- NodePtr,
- *IortNodeLength,
- *IortIdMappingCount,
- *IortIdMappingOffset
- );
- break;
-
- default:
- IncrementErrorCount ();
- Print (L"ERROR: Unsupported IORT Node type = %d\n", *IortNodeType);
- } // switch
+ // Parse the IORT Node
+ ParseAcpiStruct (
+ 2,
+ NodePtr,
+ &IortDatabase,
+ Offset,
+ *IortNodeType,
+ *IortNodeLength,
+ IortIdMappingCount,
+ IortIdMappingOffset
+ );

NodePtr += (*IortNodeLength);
Offset += (*IortNodeLength);
} // while
+
+ // Report and validate IORT Node counts
+ if (GetConsistencyChecking ()) {
+ ValidateAcpiStructCounts (&IortDatabase);
+ }
}
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH v1 4/6] ShellPkg: acpiview: Make GTDT parsing logic data driven

Krzysztof Koch
 

Replace the switch statement in the main parser loop with a table-driven
approach. Use the ParseAcpiStruct () method to resolve how each
Platform Timer Structure given should be parsed.

Enumerate all structures found in the Generic Timer Description Table
(GTDT) on a per-type basis. Print the offset from the start of the table
for each structure.

Consolidate all metadata about each Platform Timer Structure.
Define an array of ACPI_STRUCT_INFO structures to store the name,
instance count, architecture support and handling information. Use this
array to construct the ACPI_STRUCT_DATABASE for GTDT.

Count the number of instances of each Platform Timer Structure type.
Optionally report these counts after GTDT table parsing is finished.

Modify DumpGTBlock () funtion signature so that it matches that of
ACPI_STRUCT_PARSER_FUNC. This way, the function can be used in the
ParseAcpiStruct () call.

Remove the definition of the DumpWatchdogTimer (). Its only purpose was
to call ParseAcpi () and now this process is streamlined.

References:
- ACPI 6.3 Specification - January 2019, Section 5.2.24

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
v1:
- Make GTDT parsing logic data driven [Krzysztof]

ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c | 123 ++++++++++++--------
1 file changed, 77 insertions(+), 46 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
index bdd30ff45c61142c071ead63a27babab8998721b..9a9f8fda442081507768b1540f0b9b3c6c254329 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
@@ -9,13 +9,20 @@
**/

#include <IndustryStandard/Acpi.h>
+#include <Library/PrintLib.h>
#include <Library/UefiLib.h>
#include "AcpiParser.h"
#include "AcpiTableParser.h"
+#include "AcpiView.h"

// "The number of GT Block Timers must be less than or equal to 8"
#define GT_BLOCK_TIMER_COUNT_MAX 8

+/**
+ Handler for each Platform Timer Structure type
+**/
+STATIC ACPI_STRUCT_INFO GtdtStructs[];
+
// Local variables
STATIC CONST UINT32* GtdtPlatformTimerCount;
STATIC CONST UINT32* GtdtPlatformTimerOffset;
@@ -167,23 +174,35 @@ STATIC CONST ACPI_PARSER SBSAGenericWatchdogParser[] = {
/**
This function parses the Platform GT Block.

- @param [in] Ptr Pointer to the start of the GT Block data.
- @param [in] Length Length of the GT Block structure.
+ @param [in] Ptr Pointer to the start of structure's buffer.
+ @param [in] Length Length of the buffer.
+ @param [in] OptArg0 First optional argument (Not used).
+ @param [in] OptArg1 Second optional argument (Not used).
**/
STATIC
VOID
DumpGTBlock (
- IN UINT8* Ptr,
- IN UINT16 Length
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0 OPTIONAL,
+ IN CONST VOID* OptArg1 OPTIONAL
)
{
UINT32 Index;
UINT32 Offset;
+ CHAR8 Buffer[80];
+
+ PrintAcpiStructName (
+ GtdtStructs[EFI_ACPI_6_3_GTDT_GT_BLOCK].Name,
+ GtdtStructs[EFI_ACPI_6_3_GTDT_GT_BLOCK].Count,
+ sizeof (Buffer),
+ Buffer
+ );

ParseAcpi (
TRUE,
2,
- "GT Block",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (GtBlockParser)
@@ -195,7 +214,8 @@ DumpGTBlock (
(GtBlockTimerOffset == NULL)) {
IncrementErrorCount ();
Print (
- L"ERROR: Insufficient GT Block Structure length. Length = %d.\n",
+ L"ERROR: Insufficient %a Structure length. Length = %d.\n",
+ GtdtStructs[EFI_ACPI_6_3_GTDT_GT_BLOCK].Name,
Length
);
return;
@@ -206,41 +226,47 @@ DumpGTBlock (

// Parse the specified number of GT Block Timer Structures or the GT Block
// Structure buffer length. Whichever is minimum.
- while ((Index++ < *GtBlockTimerCount) &&
+ while ((Index < *GtBlockTimerCount) &&
(Offset < Length)) {
+ PrintAcpiStructName ("GT Block Timer", Index, sizeof (Buffer), Buffer);
Offset += ParseAcpi (
TRUE,
- 2,
- "GT Block Timer",
+ 4,
+ Buffer,
Ptr + Offset,
Length - Offset,
PARSER_PARAMS (GtBlockTimerParser)
);
+ Index++;
}
}

/**
- This function parses the Platform Watchdog timer.
-
- @param [in] Ptr Pointer to the start of the watchdog timer data.
- @param [in] Length Length of the watchdog timer structure.
+ Information about each Platform Timer Structure type.
**/
-STATIC
-VOID
-DumpWatchdogTimer (
- IN UINT8* Ptr,
- IN UINT16 Length
- )
-{
- ParseAcpi (
- TRUE,
- 2,
+STATIC ACPI_STRUCT_INFO GtdtStructs[] = {
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "GT Block",
+ EFI_ACPI_6_3_GTDT_GT_BLOCK,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpGTBlock
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
"SBSA Generic Watchdog",
- Ptr,
- Length,
- PARSER_PARAMS (SBSAGenericWatchdogParser)
- );
-}
+ EFI_ACPI_6_3_GTDT_SBSA_GENERIC_WATCHDOG,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ SBSAGenericWatchdogParser
+ )
+};
+
+/**
+ GTDT structure database
+**/
+STATIC ACPI_STRUCT_DATABASE GtdtDatabase = {
+ "Platform Timer Structure",
+ GtdtStructs,
+ ARRAY_SIZE (GtdtStructs)
+};

/**
This function parses the ACPI GTDT table.
@@ -275,6 +301,8 @@ ParseAcpiGtdt (
return;
}

+ ResetAcpiStructCounts (&GtdtDatabase);
+
ParseAcpi (
TRUE,
0,
@@ -321,7 +349,8 @@ ParseAcpiGtdt (
IncrementErrorCount ();
Print (
L"ERROR: Insufficient remaining table buffer length to read the " \
- L"Platform Timer Structure header. Length = %d.\n",
+ L"%a header. Length = %d.\n",
+ GtdtDatabase.Name,
AcpiTableLength - Offset
);
return;
@@ -332,8 +361,9 @@ ParseAcpiGtdt (
((Offset + (*PlatformTimerLength)) > AcpiTableLength)) {
IncrementErrorCount ();
Print (
- L"ERROR: Invalid Platform Timer Structure length. " \
- L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
+ L"ERROR: Invalid %a length. Length = %d. Offset = %d. " \
+ L"AcpiTableLength = %d.\n",
+ GtdtDatabase.Name,
*PlatformTimerLength,
Offset,
AcpiTableLength
@@ -341,23 +371,24 @@ ParseAcpiGtdt (
return;
}

- switch (*PlatformTimerType) {
- case EFI_ACPI_6_3_GTDT_GT_BLOCK:
- DumpGTBlock (TimerPtr, *PlatformTimerLength);
- break;
- case EFI_ACPI_6_3_GTDT_SBSA_GENERIC_WATCHDOG:
- DumpWatchdogTimer (TimerPtr, *PlatformTimerLength);
- break;
- default:
- IncrementErrorCount ();
- Print (
- L"ERROR: Invalid Platform Timer Type = %d\n",
- *PlatformTimerType
- );
- break;
- } // switch
+ // Parse the Platform Timer Structure
+ ParseAcpiStruct (
+ 2,
+ TimerPtr,
+ &GtdtDatabase,
+ Offset,
+ *PlatformTimerType,
+ *PlatformTimerLength,
+ NULL,
+ NULL
+ );

TimerPtr += *PlatformTimerLength;
Offset += *PlatformTimerLength;
} // while
+
+ // Report and validate Platform Timer Type Structure counts
+ if (GetConsistencyChecking ()) {
+ ValidateAcpiStructCounts (&GtdtDatabase);
+ }
}
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH v1 2/6] ShellPkg: acpiview: Make MADT parsing logic data driven

Krzysztof Koch
 

Replace the switch statement in the main parser loop with a table-driven
approach. Use the ParseAcpiStruct () method to resolve how each
Interrupt Controller Structure given should be parsed.

Enumerate all structures found in the Multiple APIC Description Table
(MADT) on a per-type basis. Print the offset from the start of the table
for each structure.

Consolidate all metadata about each Interrupt Controller Structure.
Define an array of ACPI_STRUCT_INFO structures to store the name,
instance count, architecture support and handling information. Use this
array to construct the ACPI_STRUCT_DATABASE for MADT.

Count the number of instances of each Interrupt Controller Structure
type. Optionally report these counts after MADT table parsing is
finished. Validate that Advanced Programmable Interrupt Controller
(APIC) structures are not present on Arm-based platforms.

For Arm-based platforms, make existing GIC Distributor (GICD) instance
count validation code use ACPI_STRUCT_INFO.

References:
- ACPI 6.3 Specification - January 2019, Section 5.2.12

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
v1:
- Make MADT parsing logic data driven [Krzysztof]

ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 217 ++++++++++++--------
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h | 3 +-
2 files changed, 134 insertions(+), 86 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
index f85d2b36532cfc5db36fe7bef9830cccc64969cc..00898a8853f45de1f813d71fe52920185bc92e2a 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
@@ -15,6 +15,7 @@
#include <Library/UefiLib.h>
#include "AcpiParser.h"
#include "AcpiTableParser.h"
+#include "AcpiView.h"
#include "MadtParser.h"

// Local Variables
@@ -200,6 +201,106 @@ STATIC CONST ACPI_PARSER MadtInterruptControllerHeaderParser[] = {
{L"Reserved", 2, 2, NULL, NULL, NULL, NULL, NULL}
};

+/**
+ Information about each Interrupt Controller Structure type.
+**/
+STATIC ACPI_STRUCT_INFO MadtStructs[] = {
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Processor Local APIC",
+ EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "I/O APIC",
+ EFI_ACPI_6_3_IO_APIC,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Interrupt Source Override",
+ EFI_ACPI_6_3_INTERRUPT_SOURCE_OVERRIDE,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "NMI Source",
+ EFI_ACPI_6_3_NON_MASKABLE_INTERRUPT_SOURCE,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Local APIC NMI",
+ EFI_ACPI_6_3_LOCAL_APIC_NMI,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Local APIC Address Override",
+ EFI_ACPI_6_3_LOCAL_APIC_ADDRESS_OVERRIDE,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "I/O SAPIC",
+ EFI_ACPI_6_3_IO_SAPIC,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Local SAPIC",
+ EFI_ACPI_6_3_LOCAL_SAPIC,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Platform Interrupt Sources",
+ EFI_ACPI_6_3_PLATFORM_INTERRUPT_SOURCES,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Processor Local x2APIC",
+ EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Local x2APIC NMI",
+ EFI_ACPI_6_3_LOCAL_X2APIC_NMI,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GICC",
+ EFI_ACPI_6_3_GIC,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ GicCParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GICD",
+ EFI_ACPI_6_3_GICD,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ GicDParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GIC MSI Frame",
+ EFI_ACPI_6_3_GIC_MSI_FRAME,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ GicMSIFrameParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GICR",
+ EFI_ACPI_6_3_GICR,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ GicRParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GIC ITS",
+ EFI_ACPI_6_3_GIC_ITS,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ GicITSParser
+ )
+};
+
+/**
+ MADT structure database
+**/
+STATIC ACPI_STRUCT_DATABASE MadtDatabase = {
+ "Interrupt Controller Structure",
+ MadtStructs,
+ ARRAY_SIZE (MadtStructs)
+};
+
/**
This function parses the ACPI MADT table.
When trace is enabled this function parses the MADT table and
@@ -231,14 +332,13 @@ ParseAcpiMadt (
{
UINT32 Offset;
UINT8* InterruptContollerPtr;
- UINT32 GICDCount;
-
- GICDCount = 0;

if (!Trace) {
return;
}

+ ResetAcpiStructCounts (&MadtDatabase);
+
Offset = ParseAcpi (
TRUE,
0,
@@ -267,7 +367,8 @@ ParseAcpiMadt (
IncrementErrorCount ();
Print (
L"ERROR: Insufficient remaining table buffer length to read the " \
- L"Interrupt Controller Structure header. Length = %d.\n",
+ L"%a header. Length = %d.\n",
+ MadtDatabase.Name,
AcpiTableLength - Offset
);
return;
@@ -278,8 +379,9 @@ ParseAcpiMadt (
((Offset + (*MadtInterruptControllerLength)) > AcpiTableLength)) {
IncrementErrorCount ();
Print (
- L"ERROR: Invalid Interrupt Controller Structure length. " \
- L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
+ L"ERROR: Invalid %a length. Length = %d. Offset = %d. " \
+ "AcpiTableLength = %d.\n",
+ MadtDatabase.Name,
*MadtInterruptControllerLength,
Offset,
AcpiTableLength
@@ -287,87 +389,32 @@ ParseAcpiMadt (
return;
}

- switch (*MadtInterruptControllerType) {
- case EFI_ACPI_6_3_GIC: {
- ParseAcpi (
- TRUE,
- 2,
- "GICC",
- InterruptContollerPtr,
- *MadtInterruptControllerLength,
- PARSER_PARAMS (GicCParser)
- );
- break;
- }
-
- case EFI_ACPI_6_3_GICD: {
- if (++GICDCount > 1) {
- IncrementErrorCount ();
- Print (
- L"ERROR: Only one GICD must be present,"
- L" GICDCount = %d\n",
- GICDCount
- );
- }
- ParseAcpi (
- TRUE,
- 2,
- "GICD",
- InterruptContollerPtr,
- *MadtInterruptControllerLength,
- PARSER_PARAMS (GicDParser)
- );
- break;
- }
-
- case EFI_ACPI_6_3_GIC_MSI_FRAME: {
- ParseAcpi (
- TRUE,
- 2,
- "GIC MSI Frame",
- InterruptContollerPtr,
- *MadtInterruptControllerLength,
- PARSER_PARAMS (GicMSIFrameParser)
- );
- break;
- }
-
- case EFI_ACPI_6_3_GICR: {
- ParseAcpi (
- TRUE,
- 2,
- "GICR",
- InterruptContollerPtr,
- *MadtInterruptControllerLength,
- PARSER_PARAMS (GicRParser)
- );
- break;
- }
-
- case EFI_ACPI_6_3_GIC_ITS: {
- ParseAcpi (
- TRUE,
- 2,
- "GIC ITS",
- InterruptContollerPtr,
- *MadtInterruptControllerLength,
- PARSER_PARAMS (GicITSParser)
- );
- break;
- }
-
- default: {
- IncrementErrorCount ();
- Print (
- L"ERROR: Unknown Interrupt Controller Structure,"
- L" Type = %d, Length = %d\n",
- *MadtInterruptControllerType,
- *MadtInterruptControllerLength
- );
- }
- } // switch
+ // Parse the Interrupt Controller Structure
+ ParseAcpiStruct (
+ 2,
+ InterruptContollerPtr,
+ &MadtDatabase,
+ Offset,
+ *MadtInterruptControllerType,
+ *MadtInterruptControllerLength,
+ NULL,
+ NULL
+ );

InterruptContollerPtr += *MadtInterruptControllerLength;
Offset += *MadtInterruptControllerLength;
} // while
+
+ // Report and validate Interrupt Controller Structure counts
+ if (GetConsistencyChecking ()) {
+ ValidateAcpiStructCounts (&MadtDatabase);
+
+ if (MadtStructs[EFI_ACPI_6_3_GICD].Count > 1) {
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: Only one %a must be present\n",
+ MadtStructs[EFI_ACPI_6_3_GICD].Name
+ );
+ }
+ }
}
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h
index fbbc43e09adbdf9fea302a03a61e6dc179f06a62..25128081816459106e43ef5c98acd23dc1f910c3 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h
@@ -1,13 +1,14 @@
/** @file
Header file for MADT table parser

- Copyright (c) 2019, ARM Limited. All rights reserved.
+ Copyright (c) 2020, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent

@par Reference(s):
- Arm Generic Interrupt Controller Architecture Specification,
GIC architecture version 3 and version 4, issue E
- Arm Server Base System Architecture 5.0
+ - ACPI 6.3 Specification - January 2019, Section 5.2.12
**/

#ifndef MADT_PARSER_H_
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH v1 0/6] ShellPkg: acpiview: Refactor ACPI table parsing loops

Krzysztof Koch
 

This patch series modifies existing ACPI table parsers. Now, structure
type values are used as indexes to access a centralized database
containing information on how to parse each structure in the table.

Replacing a 'switch' statements with arrays indexed by the Type value
allows consolidation of metadata about buiding blocks of an ACPI table.
The additional data stored about each structure includes:
- ACPI-defined name
- instance count
- compatible architectures (x86, AArch64, etc...)
- information on how to parse the structure

The new metadata allows more extensive ACPI table contents validation in
acpiview, which is implemented in this patch series.

Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/tree/616_refactor_acpiview_parser_loops_v1

Krzysztof Koch (6):
ShellPkg: acpiview: Add interface for data-driven table parsing
ShellPkg: acpiview: Make MADT parsing logic data driven
ShellPkg: acpiview: Make SRAT parsing logic data driven
ShellPkg: acpiview: Make GTDT parsing logic data driven
ShellPkg: acpiview: Make IORT parsing logic data driven
ShellPkg: acpiview: Make PPTT parsing logic data driven

ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c | 263 +++++++++++++++
ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h | 234 +++++++++++++
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c | 123 ++++---
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c | 353 +++++++++++++-------
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 217 +++++++-----
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h | 3 +-
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c | 152 ++++-----
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.h | 2 +-
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c | 204 +++++------
9 files changed, 1093 insertions(+), 458 deletions(-)

--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'