Date   

[PATCH 13/35] MdeModulePkg: PEI Core: clean up "AprioriFile" handling in FindFileEx()

Laszlo Ersek
 

Clean up two issues around FindFileEx():

- The "AprioriFile" parameter's type differs between the function
declaration and the function definition. The correct type is
(EFI_PEI_FILE_HANDLE*).

- "FfsFileHeader" has type (EFI_FFS_FILE_HEADER*); for clarity, we should
cast it explicitly to EFI_PEI_FILE_HANDLE when assigning it to
(*AprioriFile).

This is a semantic cleanup, there is no functional change.

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
lightly tested: OVMF uses APRIORI PEI in the FDF files

MdeModulePkg/Core/Pei/FwVol/FwVol.h | 2 +-
MdeModulePkg/Core/Pei/FwVol/FwVol.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/FwVol/FwVol.h b/MdeModulePkg/Core/Pei/=
FwVol/FwVol.h
index 4082cfbec1f8..ca80e84e0fcb 100644
--- a/MdeModulePkg/Core/Pei/FwVol/FwVol.h
+++ b/MdeModulePkg/Core/Pei/FwVol/FwVol.h
@@ -299,7 +299,7 @@ FindFileEx (
IN CONST EFI_GUID *FileName, OPTIONAL
IN EFI_FV_FILETYPE SearchType,
IN OUT EFI_PEI_FILE_HANDLE *FileHandle,
- IN OUT EFI_PEI_FV_HANDLE *AprioriFile OPTIONAL
+ IN OUT EFI_PEI_FILE_HANDLE *AprioriFile OPTIONAL
);
=20
/**
diff --git a/MdeModulePkg/Core/Pei/FwVol/FwVol.c b/MdeModulePkg/Core/Pei/=
FwVol/FwVol.c
index 709db00694c2..f4642c47c13a 100644
--- a/MdeModulePkg/Core/Pei/FwVol/FwVol.c
+++ b/MdeModulePkg/Core/Pei/FwVol/FwVol.c
@@ -407,7 +407,7 @@ FindFileEx (
} else if (AprioriFile !=3D NULL) {
if (FfsFileHeader->Type =3D=3D EFI_FV_FILETYPE_FREEFORM) {
if (CompareGuid (&FfsFileHeader->Name, &gPeiAprioriFileNameG=
uid)) {
- *AprioriFile =3D FfsFileHeader;
+ *AprioriFile =3D (EFI_PEI_FILE_HANDLE)FfsFileHeader;
}
}
}
--=20
2.19.1.3.g30247aa5d201


[PATCH 12/35] MdeModulePkg: stop abusing EFI_HANDLE for keystroke notify registration

Laszlo Ersek
 

EFI_REGISTER_KEYSTROKE_NOTIFY and EFI_UNREGISTER_KEYSTROKE_NOTIFY require
the notification handle to have type (VOID*). The notification handle has
nothing to do with the EFI_HANDLE type.

This change is a semantic fix; functionally, it's a no-op.

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
lightly tested: ConSplitterDxe is part of the ArmVirt and OVMF platfo=
rms

MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c | 2 +-
MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c =
b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
index 63c814ae1816..9c38271b65f9 100644
--- a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
+++ b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
@@ -4026,7 +4026,7 @@ ConSplitterTextInRegisterKeyNotify (
if (NewNotify =3D=3D NULL) {
return EFI_OUT_OF_RESOURCES;
}
- NewNotify->NotifyHandleList =3D (EFI_HANDLE *) AllocateZeroPool (sizeo=
f (EFI_HANDLE) * Private->TextInExListCount);
+ NewNotify->NotifyHandleList =3D (VOID **) AllocateZeroPool (sizeof (VO=
ID *) * Private->TextInExListCount);
if (NewNotify->NotifyHandleList =3D=3D NULL) {
gBS->FreePool (NewNotify);
return EFI_OUT_OF_RESOURCES;
diff --git a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c b/MdeM=
odulePkg/Universal/DriverSampleDxe/DriverSample.c
index 7cfd5c178861..f98797225b63 100644
--- a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
+++ b/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
@@ -143,7 +143,7 @@ InternalStartMonitor(
EFI_HANDLE *Handles;
UINTN HandleCount;
UINTN HandleIndex;
- EFI_HANDLE NotifyHandle;
+ VOID *NotifyHandle;
=20
Status =3D gBS->LocateHandleBuffer (
ByProtocol,
@@ -202,7 +202,7 @@ InternalStopMonitor(
EFI_KEY_DATA KeyData;
UINTN HandleCount;
UINTN HandleIndex;
- EFI_HANDLE NotifyHandle;
+ VOID *NotifyHandle;
=20
Status =3D gBS->LocateHandleBuffer (
ByProtocol,
--=20
2.19.1.3.g30247aa5d201


[PATCH 11/35] MdeModulePkg: document workaround for EFI_RUNTIME_EVENT_ENTRY PI spec bug

Laszlo Ersek
 

The PI spec (v1.7) correctly specifies "EFI_RUNTIME_EVENT_ENTRY.Event" in
natural language, but the field type in the structure definition itself i=
s
wrong -- it should be EFI_EVENT, not (EFI_EVENT*).

This spec bug is likely unfixable for compatibility reasons, and so edk2
works it around already. We should clearly document the workaround.

Functionally, this patch is a no-op.

(I've also requested a non-normative (informative) clarification for the
PI spec: <https://mantis.uefi.org/mantis/view.php?id=3D2017>.)

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
lightly tested, as these modules are part of the ArmVirt and/or OVMF
platforms

MdeModulePkg/Core/Dxe/Event/Event.c | 8 ++++++++
MdeModulePkg/Core/RuntimeDxe/Runtime.c | 10 +++++++++-
2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/Event/Event.c b/MdeModulePkg/Core/Dxe/=
Event/Event.c
index 21db38aaf037..c83c572c8f84 100644
--- a/MdeModulePkg/Core/Dxe/Event/Event.c
+++ b/MdeModulePkg/Core/Dxe/Event/Event.c
@@ -485,6 +485,14 @@ CoreCreateEventInternal (
IEvent->RuntimeData.NotifyTpl =3D NotifyTpl;
IEvent->RuntimeData.NotifyFunction =3D NotifyFunction;
IEvent->RuntimeData.NotifyContext =3D (VOID *) NotifyContext;
+ //
+ // Work around the bug in the Platform Init specification (v1.7), re=
ported
+ // as Mantis#2017: "EFI_RUNTIME_EVENT_ENTRY.Event" should have type
+ // EFI_EVENT, not (EFI_EVENT*). The PI spec documents the field corr=
ectly
+ // as "The EFI_EVENT returned by CreateEvent()", but the type of the=
field
+ // doesn't match the natural language description. Therefore we need=
an
+ // explicit cast here.
+ //
IEvent->RuntimeData.Event =3D (EFI_EVENT *) IEvent;
InsertTailList (&gRuntime->EventHead, &IEvent->RuntimeData.Link);
}
diff --git a/MdeModulePkg/Core/RuntimeDxe/Runtime.c b/MdeModulePkg/Core/R=
untimeDxe/Runtime.c
index c52b2b7ecf68..f7220a205d1e 100644
--- a/MdeModulePkg/Core/RuntimeDxe/Runtime.c
+++ b/MdeModulePkg/Core/RuntimeDxe/Runtime.c
@@ -285,8 +285,16 @@ RuntimeDriverSetVirtualAddressMap (
for (Link =3D mRuntime.EventHead.ForwardLink; Link !=3D &mRuntime.Even=
tHead; Link =3D Link->ForwardLink) {
RuntimeEvent =3D BASE_CR (Link, EFI_RUNTIME_EVENT_ENTRY, Link);
if ((RuntimeEvent->Type & EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE) =3D=3D =
EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE) {
+ //
+ // Work around the bug in the Platform Init specification (v1.7),
+ // reported as Mantis#2017: "EFI_RUNTIME_EVENT_ENTRY.Event" should=
have
+ // type EFI_EVENT, not (EFI_EVENT*). The PI spec documents the fie=
ld
+ // correctly as "The EFI_EVENT returned by CreateEvent()", but the=
type
+ // of the field doesn't match the natural language description. Th=
erefore
+ // we need an explicit cast here.
+ //
RuntimeEvent->NotifyFunction (
- RuntimeEvent->Event,
+ (EFI_EVENT) RuntimeEvent->Event,
RuntimeEvent->NotifyContext
);
}
--=20
2.19.1.3.g30247aa5d201


[PATCH 10/35] MdeModulePkg/PlatformVarCleanupLib: fix HiiConstructConfigHdr() call

Laszlo Ersek
 

The HiiConstructConfigHdr() function takes the "DriverHandle" parameter i=
n
order to fetch the device path from it, and then turn the device path int=
o
PATH routing information.

The HiiConstructConfigHdr() function is called from
VariableCleanupHiiExtractConfig(), which is only installed when "Type" is
"VarCleanupManually" in PlatformVarCleanup().

In that case, we create "Private->DriverHandle" as a new handle, and
install "mVarCleanupHiiVendorDevicePath" on it. Then we pass
"Private->DriverHandle" to HiiAddPackages(), which consumes the device
path for routing purposes.

It follows that the "DriverHandle" argument pased to
HiiConstructConfigHdr() should be the same driver handle, for matching
routing.

Currently we pass "Private->HiiHandle", which is clearly a typo, because
it is the return value of HiiAddPackages(), and stands for the published
HII package list.

Therefore this patch addresses an actual bug.

The typo has not been flagged by compilers because the UEFI spec
regrettably defines both EFI_HANDLE and EFI_HII_HANDLE as (VOID*).

Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
build-tested only

MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c | 6 +++++=
-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib=
.c b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
index 968c044a316a..3875d614bb41 100644
--- a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
+++ b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
@@ -609,7 +609,11 @@ VariableCleanupHiiExtractConfig (
// Allocate and fill a buffer large enough to hold the <ConfigHdr> t=
emplate
// followed by "&OFFSET=3D0&WIDTH=3DWWWWWWWWWWWWWWWW" followed by a =
Null-terminator.
//
- ConfigRequestHdr =3D HiiConstructConfigHdr (&mVariableCleanupHiiGuid=
, mVarStoreName, Private->HiiHandle);
+ ConfigRequestHdr =3D HiiConstructConfigHdr (
+ &mVariableCleanupHiiGuid,
+ mVarStoreName,
+ Private->DriverHandle
+ );
Size =3D (StrLen (ConfigRequestHdr) + 32 + 1) * sizeof (CHAR16);
ConfigRequest =3D AllocateZeroPool (Size);
ASSERT (ConfigRequest !=3D NULL);
--=20
2.19.1.3.g30247aa5d201


[PATCH 09/35] MdeModulePkg: stop abusing EFI_EVENT for protocol notify registration

Laszlo Ersek
 

EfiCreateProtocolNotifyEvent() takes a (VOID**) for "Registration",
similarly to gBS->RegisterProtocolNotify(). We should pass the address of
an actual pointer-to-VOID, and not the address of an EFI_EVENT. EFI_EVENT
just happens to be specified as (VOID*), and has nothing to do with the
registration.

The same applies to gMmst->MmRegisterProtocolNotify().

"mFtwRegistration", "mFvRegistration", and "mFvbRegistration" are used fo=
r
nothing else.

This change is a no-op in practice; it's a semantic improvement.

Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
lightly tested, as these modules (except LoadFileOnFv2) are part of t=
he
ArmVirt and/or OVMF platforms

MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c | 2=
+-
MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c | 2=
+-
MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c | 2=
+-
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c | 2=
+-
4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWr=
iteDxe.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrit=
eDxe.c
index ae8f117905cd..de38ea028af1 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.=
c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.=
c
@@ -47,7 +47,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
=20
#include <Library/UefiBootServicesTableLib.h>
#include "FaultTolerantWrite.h"
-EFI_EVENT mFvbRegistration =3D NULL;
+VOID *mFvbRegistration =3D NULL;
=20
=20
/**
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWr=
iteSmm.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrit=
eSmm.c
index e8e935a85b5b..9612b394865b 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.=
c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.=
c
@@ -56,7 +56,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include "FaultTolerantWriteSmmCommon.h"
#include <Protocol/MmEndOfDxe.h>
=20
-EFI_EVENT mFvbRegistration =3D NULL;
+VOID *mFvbRegistration =3D NULL;
EFI_FTW_DEVICE *mFtwDevice =3D NULL;
=20
///
diff --git a/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c b/MdeMo=
dulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
index 4af2da05e145..43fa6ce12875 100644
--- a/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
+++ b/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
@@ -36,7 +36,7 @@ typedef struct {
#define LOAD_FILE_ON_FV2_PRIVATE_DATA_FROM_THIS(a) CR (a, LOAD_FILE_ON_F=
V2_PRIVATE_DATA, LoadFile, LOAD_FILE_ON_FV2_PRIVATE_DATA_SIGNATURE)
#define LOAD_FILE_ON_FV2_PRIVATE_DATA_FROM_LINK(a) CR (a, LOAD_FILE_ON_F=
V2_PRIVATE_DATA, Link, LOAD_FILE_ON_FV2_PRIVATE_DATA_SIGNATURE)
=20
-EFI_EVENT mFvRegistration;
+VOID *mFvRegistration;
LIST_ENTRY mPrivateDataList;
=20
/**
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c b/M=
deModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
index 3d232bb36cb4..7d2b6c8e1fad 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
@@ -13,7 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
=20
EFI_HANDLE mHandle =3D NULL;
EFI_EVENT mVirtualAddressChangeEvent =3D NULL;
-EFI_EVENT mFtwRegistration =3D NULL;
+VOID *mFtwRegistration =3D NULL;
VOID ***mVarCheckAddressPointer =3D NULL;
UINTN mVarCheckAddressPointerCount =3D 0;
EDKII_VARIABLE_LOCK_PROTOCOL mVariableLock =3D { Var=
iableLockRequestToLock };
--=20
2.19.1.3.g30247aa5d201


[PATCH 08/35] MdeModulePkg/UefiHiiLib: stop using EFI_HANDLE in place of EFI_HII_HANDLE

Laszlo Ersek
 

HiiGetHiiHandles() returns an array of EFI_HII_HANDLEs, not EFI_HANDLEs.
HiiGetString() takes an EFI_HII_HANDLE, not an EFI_HANDLE.

This change is a no-op in practice; it's a semantic improvement.

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
lightly tested, as UefiHiiLib is used by both ArmVirt and OVMF

MdeModulePkg/Library/UefiHiiLib/HiiString.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Library/UefiHiiLib/HiiString.c b/MdeModulePkg/L=
ibrary/UefiHiiLib/HiiString.c
index 498f245dce1f..95229f8a8c9f 100644
--- a/MdeModulePkg/Library/UefiHiiLib/HiiString.c
+++ b/MdeModulePkg/Library/UefiHiiLib/HiiString.c
@@ -173,8 +173,8 @@ HiiGetPackageString (
IN CONST CHAR8 *Language OPTIONAL
)
{
- EFI_HANDLE *HiiHandleBuffer;
- EFI_HANDLE HiiHandle;
+ EFI_HII_HANDLE *HiiHandleBuffer;
+ EFI_HII_HANDLE HiiHandle;
=20
ASSERT (PackageListGuid !=3D NULL);
=20
--=20
2.19.1.3.g30247aa5d201


[PATCH 07/35] MdeModulePkg: fix cast in GetModuleInfoFromHandle() calls

Laszlo Ersek
 

GetModuleInfoFromHandle() takes an EFI_HANDLE -- (VOID*) -- as first
parameter, but InsertFpdtRecord() passes (EFI_HANDLE*) -- (VOID**).
(VOID**) converts silently to (VOID*), which is why the wrong cast is
masked.

Note that the *value* that is passed is alright -- therefore this patch
does not change behavior --, it's just semantically wrong to pass an
(EFI_HANDLE*) where an EFI_HANDLE is expected.

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
lightly tested, as DxeCorePerformanceLib is linked into ArmVirtQemu's
DxeCore

MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 12 =
++++++------
MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c | 8 =
++++----
2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanc=
eLib.c b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib=
.c
index 0d507c445210..f500e20b320b 100644
--- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
+++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
@@ -998,7 +998,7 @@ InsertFpdtRecord (
switch (PerfId) {
case MODULE_START_ID:
case MODULE_END_ID:
- GetModuleInfoFromHandle ((EFI_HANDLE *)CallerIdentifier, ModuleName,=
sizeof (ModuleName), &ModuleGuid);
+ GetModuleInfoFromHandle ((EFI_HANDLE)CallerIdentifier, ModuleName, s=
izeof (ModuleName), &ModuleGuid);
StringPtr =3D ModuleName;
//
// Cache the offset of start image start record and use to update th=
e start image end record if needed.
@@ -1031,7 +1031,7 @@ InsertFpdtRecord (
=20
case MODULE_LOADIMAGE_START_ID:
case MODULE_LOADIMAGE_END_ID:
- GetModuleInfoFromHandle ((EFI_HANDLE *)CallerIdentifier, ModuleName,=
sizeof (ModuleName), &ModuleGuid);
+ GetModuleInfoFromHandle ((EFI_HANDLE)CallerIdentifier, ModuleName, s=
izeof (ModuleName), &ModuleGuid);
StringPtr =3D ModuleName;
if (PerfId =3D=3D MODULE_LOADIMAGE_START_ID) {
mLoadImageCount ++;
@@ -1071,7 +1071,7 @@ InsertFpdtRecord (
case MODULE_DB_SUPPORT_END_ID:
case MODULE_DB_STOP_START_ID:
case MODULE_DB_STOP_END_ID:
- GetModuleInfoFromHandle ((EFI_HANDLE *)CallerIdentifier, ModuleName,=
sizeof (ModuleName), &ModuleGuid);
+ GetModuleInfoFromHandle ((EFI_HANDLE)CallerIdentifier, ModuleName, s=
izeof (ModuleName), &ModuleGuid);
StringPtr =3D ModuleName;
if (!PcdGetBool (PcdEdkiiFpdtStringRecordEnableOnly)) {
FpdtRecordPtr.GuidQwordEvent->Header.Type =3D FPDT_GUID_=
QWORD_EVENT_TYPE;
@@ -1085,7 +1085,7 @@ InsertFpdtRecord (
break;
=20
case MODULE_DB_END_ID:
- GetModuleInfoFromHandle ((EFI_HANDLE *)CallerIdentifier, ModuleName,=
sizeof (ModuleName), &ModuleGuid);
+ GetModuleInfoFromHandle ((EFI_HANDLE)CallerIdentifier, ModuleName, s=
izeof (ModuleName), &ModuleGuid);
StringPtr =3D ModuleName;
if (!PcdGetBool (PcdEdkiiFpdtStringRecordEnableOnly)) {
FpdtRecordPtr.GuidQwordStringEvent->Header.Type =3D FPDT_GUID_=
QWORD_STRING_EVENT_TYPE;
@@ -1131,7 +1131,7 @@ InsertFpdtRecord (
case PERF_INMODULE_END_ID:
case PERF_CROSSMODULE_START_ID:
case PERF_CROSSMODULE_END_ID:
- GetModuleInfoFromHandle ((EFI_HANDLE *)CallerIdentifier, ModuleName,=
sizeof (ModuleName), &ModuleGuid);
+ GetModuleInfoFromHandle ((EFI_HANDLE)CallerIdentifier, ModuleName, s=
izeof (ModuleName), &ModuleGuid);
if (String !=3D NULL) {
StringPtr =3D String;
} else {
@@ -1153,7 +1153,7 @@ InsertFpdtRecord (
=20
default:
if (Attribute !=3D PerfEntry) {
- GetModuleInfoFromHandle ((EFI_HANDLE *)CallerIdentifier, ModuleNam=
e, sizeof (ModuleName), &ModuleGuid);
+ GetModuleInfoFromHandle ((EFI_HANDLE)CallerIdentifier, ModuleName,=
sizeof (ModuleName), &ModuleGuid);
if (String !=3D NULL) {
StringPtr =3D String;
} else {
diff --git a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanc=
eLib.c b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib=
.c
index 5f07464c4ec7..b4f22c14ae73 100644
--- a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
+++ b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
@@ -587,7 +587,7 @@ InsertFpdtRecord (
switch (PerfId) {
case MODULE_START_ID:
case MODULE_END_ID:
- GetModuleInfoFromHandle ((EFI_HANDLE *)CallerIdentifier, ModuleName,=
sizeof (ModuleName), &ModuleGuid);
+ GetModuleInfoFromHandle ((EFI_HANDLE)CallerIdentifier, ModuleName, s=
izeof (ModuleName), &ModuleGuid);
StringPtr =3D ModuleName;
//
// Cache the offset of start image start record and use to update th=
e start image end record if needed.
@@ -612,7 +612,7 @@ InsertFpdtRecord (
=20
case MODULE_LOADIMAGE_START_ID:
case MODULE_LOADIMAGE_END_ID:
- GetModuleInfoFromHandle ((EFI_HANDLE *)CallerIdentifier, ModuleName,=
sizeof (ModuleName), &ModuleGuid);
+ GetModuleInfoFromHandle ((EFI_HANDLE)CallerIdentifier, ModuleName, s=
izeof (ModuleName), &ModuleGuid);
StringPtr =3D ModuleName;
if (PerfId =3D=3D MODULE_LOADIMAGE_START_ID) {
mLoadImageCount++;
@@ -669,7 +669,7 @@ InsertFpdtRecord (
case PERF_INMODULE_END_ID:
case PERF_CROSSMODULE_START_ID:
case PERF_CROSSMODULE_END_ID:
- GetModuleInfoFromHandle ((EFI_HANDLE *)CallerIdentifier, ModuleName,=
sizeof (ModuleName), &ModuleGuid);
+ GetModuleInfoFromHandle ((EFI_HANDLE)CallerIdentifier, ModuleName, s=
izeof (ModuleName), &ModuleGuid);
if (String !=3D NULL) {
StringPtr =3D String;
} else {
@@ -691,7 +691,7 @@ InsertFpdtRecord (
=20
default:
if (Attribute !=3D PerfEntry) {
- GetModuleInfoFromHandle ((EFI_HANDLE *)CallerIdentifier, ModuleNam=
e, sizeof (ModuleName), &ModuleGuid);
+ GetModuleInfoFromHandle ((EFI_HANDLE)CallerIdentifier, ModuleName,=
sizeof (ModuleName), &ModuleGuid);
if (String !=3D NULL) {
StringPtr =3D String;
} else {
--=20
2.19.1.3.g30247aa5d201


[PATCH 06/35] EmulatorPkg: stop abusing EFI_HANDLE for keystroke notify registration

Laszlo Ersek
 

EFI_REGISTER_KEYSTROKE_NOTIFY and EFI_UNREGISTER_KEYSTROKE_NOTIFY require
the notification handle to have type (VOID*). The notification handle has
nothing to do with the EFI_HANDLE type.

This change is a semantic fix; functionally, it's a no-op.

Cc: Andrew Fish <afish@apple.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
build-tested only

EmulatorPkg/EmuGopDxe/GopInput.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/EmulatorPkg/EmuGopDxe/GopInput.c b/EmulatorPkg/EmuGopDxe/Gop=
Input.c
index fdd0b4911555..2a23564a2173 100644
--- a/EmulatorPkg/EmuGopDxe/GopInput.c
+++ b/EmulatorPkg/EmuGopDxe/GopInput.c
@@ -517,7 +517,7 @@ EmuGopSimpleTextInExRegisterKeyNotify (
IN EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL *This,
IN EFI_KEY_DATA *KeyData,
IN EFI_KEY_NOTIFY_FUNCTION KeyNotificationFunction,
- OUT EFI_HANDLE *NotifyHandle
+ OUT VOID **NotifyHandle
)
{
EFI_STATUS Status;
@@ -600,7 +600,7 @@ EFI_STATUS
EFIAPI
EmuGopSimpleTextInExUnregisterKeyNotify (
IN EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL *This,
- IN EFI_HANDLE NotificationHandle
+ IN VOID *NotificationHandle
)
/*++
=20
--=20
2.19.1.3.g30247aa5d201


[PATCH 05/35] EmulatorPkg/DxeTimerLib: drop superfluous cast

Laszlo Ersek
 

"gTimerEvent" has type EFI_EVENT already, drop the superfluous cast.

Cc: Andrew Fish <afish@apple.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
build-tested only

EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c b/EmulatorPkg/=
Library/DxeTimerLib/DxeTimerLib.c
index 14cae4214c66..6fb5d8f3aaea 100644
--- a/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
+++ b/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
@@ -40,7 +40,7 @@ RegisterTimerArchProtocol (
gTimerPeriod =3D MultU64x32 (gTimerPeriod, 100);
=20
if (gTimerEvent =3D=3D NULL) {
- Status =3D gBS->CreateEvent (EVT_TIMER, 0, NULL, NULL, (VOID **)&g=
TimerEvent);
+ Status =3D gBS->CreateEvent (EVT_TIMER, 0, NULL, NULL, &gTimerEven=
t);
ASSERT_EFI_ERROR (Status);
}
}
--=20
2.19.1.3.g30247aa5d201


[PATCH 04/35] EmbeddedPkg/Universal/MmcDxe: "fix" CloseProtocol() call in BindingStop()

Laszlo Ersek
 

The 3rd and 4th parameters of the CloseProtocol() call are wrong.

Given that we're not dissociating a child controller from a parent
controller (=3D closing a BY_CHILD_CONTROLLER open), but closing a BY_DRI=
VER
open, the 4th parameter (ControllerHandle) should equal the 1st parameter
(Handle).

It's unclear why this code hasn't crashed before.

Note that the patch doesn't fix the underlying driver model bug. I don't
understand what the loop in MmcDriverBindingStop() attempts to do. Is thi=
s
driver supposed to be a bus driver? It seems to create new handles, and t=
o
append device path nodes. But it doesn't set up proper parent/child
protocol opens, and it doesn't close them.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
build-tested only

EmbeddedPkg/Universal/MmcDxe/Mmc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.c b/EmbeddedPkg/Universal/M=
mcDxe/Mmc.c
index 2f9ec9c7e7c1..c6170880debd 100644
--- a/EmbeddedPkg/Universal/MmcDxe/Mmc.c
+++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.c
@@ -329,8 +329,9 @@ MmcDriverBindingStop (
// Close gEfiMmcHostProtocolGuid
Status =3D gBS->CloseProtocol (
Controller,
- &gEfiMmcHostProtocolGuid,(VOID **) &MmcHostInstance->Mmc=
Host,
- This->DriverBindingHandle
+ &gEfiMmcHostProtocolGuid,
+ This->DriverBindingHandle,
+ Controller
);
=20
// Remove MMC Host Instance from the pool
--=20
2.19.1.3.g30247aa5d201


[PATCH 03/35] EmbeddedPkg/AndroidFastbootTransportTcpDxe: fix DestroyChild() call

Laszlo Ersek
 

- The 2nd parameter of EFI_SERVICE_BINDING_CREATE_CHILD is:

IN OUT EFI_HANDLE *ChildHandle

- The 2nd parameter of EFI_SERVICE_BINDING_DESTROY_CHILD is:

IN EFI_HANDLE ChildHandle

Fix the DestroyChild() call in TcpFastbootTransportStop().

This is an actual bugfix; I don't know why the current code doesn't crash=
.
Perhaps the function is never reached in practice? (It could be tied to a=
n
error path.)

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
build-tested only

EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcp.=
c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootT=
ransportTcp.c b/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/Fastbo=
otTransportTcp.c
index 29f23a82c75f..34f9ba74e4db 100644
--- a/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTranspor=
tTcp.c
+++ b/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTranspor=
tTcp.c
@@ -503,7 +503,7 @@ TcpFastbootTransportStop (
Status =3D mTcpListener->Configure (mTcpListener, NULL);
ASSERT_EFI_ERROR (Status);
=20
- Status =3D mTcpServiceBinding->DestroyChild (mTcpServiceBinding, &mTcp=
Handle);
+ Status =3D mTcpServiceBinding->DestroyChild (mTcpServiceBinding, mTcpH=
andle);
=20
// Free any data the user didn't pick up
Entry =3D (FASTBOOT_TCP_PACKET_LIST *) GetFirstNode (&mPacketListHead)=
;
--=20
2.19.1.3.g30247aa5d201


[PATCH 02/35] EmbeddedPkg: add missing EFIAPI calling convention specifiers

Laszlo Ersek
 

This patch is unrelated to the rest of the series; it just makes sure tha=
t
"EmbeddedPkg/EmbeddedPkg.dsc" builds for all platforms advertised in
SUPPORTED_ARCHITECTURES (in particular, X64).

No functional changes.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
build-tested only

EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.h | 32 +++++++++++=
++++-----
EmbeddedPkg/GdbStub/GdbStubInternal.h | 9 ++++++
EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.c | 1 +
EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c | 1 +
EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c | 8 +++++
EmbeddedPkg/MetronomeDxe/Metronome.c | 1 +
6 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.h b/EmbeddedP=
kg/Drivers/SataSiI3132Dxe/SataSiI3132.h
index e3db0821c38f..20636574c271 100644
--- a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.h
+++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.h
@@ -205,7 +205,9 @@ SataSiI3132DriverBindingStop (
IN EFI_HANDLE *ChildHandleBuffer
);
=20
-EFI_STATUS SiI3132AtaPassThruCommand (
+EFI_STATUS
+EFIAPI
+SiI3132AtaPassThruCommand (
IN SATA_SI3132_INSTANCE *pSataSiI3132Instance,
IN SATA_SI3132_PORT *pSataPort,
IN UINT16 PortMultiplierPort,
@@ -216,7 +218,9 @@ EFI_STATUS SiI3132AtaPassThruCommand (
/**
* EFI ATA Pass Thru Protocol
*/
-EFI_STATUS SiI3132AtaPassThru (
+EFI_STATUS
+EFIAPI
+SiI3132AtaPassThru (
IN EFI_ATA_PASS_THRU_PROTOCOL *This,
IN UINT16 Port,
IN UINT16 PortMultiplierPort,
@@ -224,37 +228,49 @@ EFI_STATUS SiI3132AtaPassThru (
IN EFI_EVENT Event OPTIONAL
);
=20
-EFI_STATUS SiI3132GetNextPort (
+EFI_STATUS
+EFIAPI
+SiI3132GetNextPort (
IN EFI_ATA_PASS_THRU_PROTOCOL *This,
IN OUT UINT16 *Port
);
=20
-EFI_STATUS SiI3132GetNextDevice (
+EFI_STATUS
+EFIAPI
+SiI3132GetNextDevice (
IN EFI_ATA_PASS_THRU_PROTOCOL *This,
IN UINT16 Port,
IN OUT UINT16 *PortMultiplierPort
);
=20
-EFI_STATUS SiI3132BuildDevicePath (
+EFI_STATUS
+EFIAPI
+SiI3132BuildDevicePath (
IN EFI_ATA_PASS_THRU_PROTOCOL *This,
IN UINT16 Port,
IN UINT16 PortMultiplierPort,
IN OUT EFI_DEVICE_PATH_PROTOCOL **DevicePath
);
=20
-EFI_STATUS SiI3132GetDevice (
+EFI_STATUS
+EFIAPI
+SiI3132GetDevice (
IN EFI_ATA_PASS_THRU_PROTOCOL *This,
IN EFI_DEVICE_PATH_PROTOCOL *DevicePath,
OUT UINT16 *Port,
OUT UINT16 *PortMultiplierPort
);
=20
-EFI_STATUS SiI3132ResetPort (
+EFI_STATUS
+EFIAPI
+SiI3132ResetPort (
IN EFI_ATA_PASS_THRU_PROTOCOL *This,
IN UINT16 Port
);
=20
-EFI_STATUS SiI3132ResetDevice (
+EFI_STATUS
+EFIAPI
+SiI3132ResetDevice (
IN EFI_ATA_PASS_THRU_PROTOCOL *This,
IN UINT16 Port,
IN UINT16 PortMultiplierPort
diff --git a/EmbeddedPkg/GdbStub/GdbStubInternal.h b/EmbeddedPkg/GdbStub/=
GdbStubInternal.h
index b8346d7a545f..b08159302cfa 100644
--- a/EmbeddedPkg/GdbStub/GdbStubInternal.h
+++ b/EmbeddedPkg/GdbStub/GdbStubInternal.h
@@ -323,6 +323,7 @@ SendError (
Send 'OK' when the function is done executing successfully.
**/
VOID
+EFIAPI
SendSuccess (
VOID
);
@@ -332,6 +333,7 @@ SendSuccess (
Send empty packet to specify that particular command/functionality is n=
ot supported.
**/
VOID
+EFIAPI
SendNotSupported (
VOID
);
@@ -353,6 +355,7 @@ ReadNthRegister (
@param SystemContext Register content at time of the exce=
ption
**/
VOID
+EFIAPI
ReadGeneralRegisters (
IN EFI_SYSTEM_CONTEXT SystemContext
);
@@ -364,6 +367,7 @@ ReadGeneralRegisters (
@param InBuffer This is the input buffer received from g=
db server
**/
VOID
+EFIAPI
WriteNthRegister (
IN EFI_SYSTEM_CONTEXT SystemContext,
IN CHAR8 *InBuffer
@@ -377,6 +381,7 @@ WriteNthRegister (
**/
=20
VOID
+EFIAPI
WriteGeneralRegisters (
IN EFI_SYSTEM_CONTEXT SystemContext,
IN CHAR8 *InBuffer
@@ -391,6 +396,7 @@ WriteGeneralRegisters (
@param *PacketData Pointer to Payload data for the packet
**/
VOID
+EFIAPI
ReadFromMemory (
IN CHAR8 *PacketData
);
@@ -404,6 +410,7 @@ ReadFromMemory (
@param PacketData Pointer to Payload data for the packet
**/
VOID
+EFIAPI
WriteToMemory (
IN CHAR8 *PacketData
);
@@ -418,6 +425,7 @@ WriteToMemory (
**/
=20
VOID
+EFIAPI
ContinueAtAddress (
IN EFI_SYSTEM_CONTEXT SystemContext,
IN CHAR8 *PacketData
@@ -432,6 +440,7 @@ ContinueAtAddress (
@param PacketData Pointer to Payload data for the packet
**/
VOID
+EFIAPI
SingleStep (
IN EFI_SYSTEM_CONTEXT SystemContext,
IN CHAR8 *PacketData
diff --git a/EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.c b/Embedd=
edPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.c
index c250844eda74..08bba1bbf111 100644
--- a/EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.c
+++ b/EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.c
@@ -174,6 +174,7 @@ RemoveSpcrTable (
=20
STATIC
VOID
+EFIAPI
OnReadyToBoot (
IN EFI_EVENT Event,
IN VOID *Context
diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c b/EmbeddedPkg/Dr=
ivers/Lan9118Dxe/Lan9118Dxe.c
index a0fca4d6a335..2138f7576bec 100644
--- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c
+++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c
@@ -34,6 +34,7 @@ LAN9118_DEVICE_PATH Lan9118PathTemplate =3D {
**
*/
EFI_STATUS
+EFIAPI
Lan9118DxeEntry (
IN EFI_HANDLE Handle,
IN EFI_SYSTEM_TABLE *SystemTable
diff --git a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c b/Em=
beddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c
index f6a723adfb28..0e2905c1ebb0 100644
--- a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c
+++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c
@@ -39,6 +39,7 @@ GetSataDevice (
}
=20
EFI_STATUS
+EFIAPI
SiI3132AtaPassThruCommand (
IN SATA_SI3132_INSTANCE *SataSiI3132Instance,
IN SATA_SI3132_PORT *SataPort,
@@ -310,6 +311,7 @@ SiI3132AtaPassThruCommand (
=20
**/
EFI_STATUS
+EFIAPI
SiI3132AtaPassThru (
IN EFI_ATA_PASS_THRU_PROTOCOL *This,
IN UINT16 Port,
@@ -371,6 +373,7 @@ SiI3132AtaPassThru (
=20
**/
EFI_STATUS
+EFIAPI
SiI3132GetNextPort (
IN EFI_ATA_PASS_THRU_PROTOCOL *This,
IN OUT UINT16 *Port
@@ -442,6 +445,7 @@ SiI3132GetNextPort (
=20
**/
EFI_STATUS
+EFIAPI
SiI3132GetNextDevice (
IN EFI_ATA_PASS_THRU_PROTOCOL *This,
IN UINT16 Port,
@@ -523,6 +527,7 @@ SiI3132GetNextDevice (
=20
**/
EFI_STATUS
+EFIAPI
SiI3132BuildDevicePath (
IN EFI_ATA_PASS_THRU_PROTOCOL *This,
IN UINT16 Port,
@@ -601,6 +606,7 @@ SiI3132BuildDevicePath (
port number does not exist.
**/
EFI_STATUS
+EFIAPI
SiI3132GetDevice (
IN EFI_ATA_PASS_THRU_PROTOCOL *This,
IN EFI_DEVICE_PATH_PROTOCOL *DevicePath,
@@ -717,6 +723,7 @@ SiI3132HwResetPort (
=20
**/
EFI_STATUS
+EFIAPI
SiI3132ResetPort (
IN EFI_ATA_PASS_THRU_PROTOCOL *This,
IN UINT16 Port
@@ -772,6 +779,7 @@ SiI3132ResetPort (
=20
**/
EFI_STATUS
+EFIAPI
SiI3132ResetDevice (
IN EFI_ATA_PASS_THRU_PROTOCOL *This,
IN UINT16 Port,
diff --git a/EmbeddedPkg/MetronomeDxe/Metronome.c b/EmbeddedPkg/Metronome=
Dxe/Metronome.c
index 579332169507..13db25168fac 100644
--- a/EmbeddedPkg/MetronomeDxe/Metronome.c
+++ b/EmbeddedPkg/MetronomeDxe/Metronome.c
@@ -110,6 +110,7 @@ EFI_HANDLE gMetronomeHandle =3D NULL;
=20
**/
EFI_STATUS
+EFIAPI
MetronomeInitialize (
IN EFI_HANDLE ImageHandle,
IN EFI_SYSTEM_TABLE *SystemTable
--=20
2.19.1.3.g30247aa5d201


[PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID

Laszlo Ersek
 

Unfortunately, the UEFI / PI / Shell specs define a number of handle type=
s
as pointers to VOID. This is a design mistake; those types should have
been pointers to incomplete union or structure types. Any
pointer-to-object type converts implicitly to, and from, pointer-to-void,
which prevents compilers from catching at least the following two types o=
f
mistakes:

- mixing up one handle type with another (for example, EFI_HANDLE with
EFI_EVENT),

- getting the depth of indirection wrong (for example, mixing up
(EFI_HANDLE*) with EFI_HANDLE).

In order to root out such mistakes in the edk2 codebase, introduce
incomplete structure types with unique tags, such as:

struct EFI_FOOBAR_OBJECT;
typedef struct EFI_FOOBAR_OBJECT *EFI_FOOBAR_HANDLE;

replacing the spec mandated

typedef VOID *EFI_FOOBAR_HANDLE;

(For some types, such as:

- EFI_ACPI_HANDLE,
- EFI_EVENT,
- EFI_FONT_HANDLE,
- EFI_HANDLE,
- EFI_HII_HANDLE,
- EFI_S3_BOOT_SCRIPT_POSITION,
- SHELL_FILE_HANDLE,

we connect the actual complete type (the internal, implementation-specifi=
c
type) to the typedef. Some of these also demonstrate how the code could
have looked in practice if the specs had used proper opaque (=3Dincomplet=
e)
types.)

Then, unleash "build" on the package DSC files. This causes the compiler
to warn about incompatible pointer assignments, and to stop the build.

The rest of the series addresses the resultant warnings. Each patch
belongs in one of two categories:

- semantic cleanups (no functional / behavioral changes),
- actual bugfixes.

As the subject line of this patch states, this specific patch is *not*
meant to be applied. It is just a "what if" patch that temporarily
isolates the standard types from each other, the way the specs should
have, so that the compiler have more information to work with.

Cc: Achin Gupta <achin.gupta@arm.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jian Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdePkg/Include/Pi/PiPeiCis.h | 6 ++++--
MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h | 3 ++-
MdePkg/Include/Protocol/Bis.h | 3 ++-
MdePkg/Include/Protocol/Eap.h | 3 ++-
MdePkg/Include/Protocol/HiiFont.h | 3 +--
MdePkg/Include/Protocol/MmMp.h | 3 ++-
MdePkg/Include/Protocol/S3SaveState.h | 2 +-
MdePkg/Include/Protocol/Shell.h | 3 ++-
MdePkg/Include/Protocol/UserManager.h | 9 ++++++---
MdePkg/Include/Uefi/UefiBaseType.h | 6 ++++--
MdePkg/Include/Uefi/UefiInternalFormRepresentation.h | 3 ++-
MdeModulePkg/Core/Dxe/Event/Event.h | 2 +-
MdeModulePkg/Core/Dxe/Hand/Handle.h | 2 +-
MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 2 +-
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h | 2 +-
MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h | 2 +-
StandaloneMmPkg/Core/StandaloneMmCore.h | 2 +-
17 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/MdePkg/Include/Pi/PiPeiCis.h b/MdePkg/Include/Pi/PiPeiCis.h
index d9d4ed7d413a..3e9e82b62ae9 100644
--- a/MdePkg/Include/Pi/PiPeiCis.h
+++ b/MdePkg/Include/Pi/PiPeiCis.h
@@ -18,12 +18,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
///
/// The handles of EFI FV.
///
-typedef VOID *EFI_PEI_FV_HANDLE;
+struct EFI_PEI_FV_OBJECT;
+typedef struct EFI_PEI_FV_OBJECT *EFI_PEI_FV_HANDLE;
=20
///
/// The handles of EFI FFS.
///
-typedef VOID *EFI_PEI_FILE_HANDLE;
+struct EFI_PEI_FILE_OBJECT;
+typedef struct EFI_PEI_FILE_OBJECT *EFI_PEI_FILE_HANDLE;
=20
///
/// Declare the forward reference data structure for EFI_PEI_SERVICE.
diff --git a/MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h b/MdePk=
g/Include/Protocol/AcpiSystemDescriptionTable.h
index a8e0b24c6c8d..8a1863f3e03d 100644
--- a/MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h
+++ b/MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h
@@ -16,7 +16,8 @@
{ 0xeb97088e, 0xcfdf, 0x49c6, { 0xbe, 0x4b, 0xd9, 0x6, 0xa5, 0xb2, 0xe=
, 0x86 }}
=20
typedef UINT32 EFI_ACPI_TABLE_VERSION;
-typedef VOID *EFI_ACPI_HANDLE;
+struct EFI_ACPI_OBJECT;
+typedef struct EFI_ACPI_OBJECT *EFI_ACPI_HANDLE;
=20
#define EFI_ACPI_TABLE_VERSION_NONE (1 << 0)
#define EFI_ACPI_TABLE_VERSION_1_0B (1 << 1)
diff --git a/MdePkg/Include/Protocol/Bis.h b/MdePkg/Include/Protocol/Bis.=
h
index 2be6718f4bc2..8eca94512d03 100644
--- a/MdePkg/Include/Protocol/Bis.h
+++ b/MdePkg/Include/Protocol/Bis.h
@@ -37,7 +37,8 @@ typedef struct _EFI_BIS_PROTOCOL EFI_BIS_PROTOCOL;
//
// Basic types
//
-typedef VOID *BIS_APPLICATION_HANDLE;
+struct BIS_APPLICATION_OBJECT;
+typedef struct BIS_APPLICATION_OBJECT *BIS_APPLICATION_HANDLE;
typedef UINT16 BIS_ALG_ID;
typedef UINT32 BIS_CERT_ID;
=20
diff --git a/MdePkg/Include/Protocol/Eap.h b/MdePkg/Include/Protocol/Eap.=
h
index 203d0f40b0dd..06584ef409d0 100644
--- a/MdePkg/Include/Protocol/Eap.h
+++ b/MdePkg/Include/Protocol/Eap.h
@@ -28,7 +28,8 @@ typedef struct _EFI_EAP_PROTOCOL EFI_EAP_PROTOCOL;
/// Type for the identification number assigned to the Port by the
/// System in which the Port resides.
///
-typedef VOID * EFI_PORT_HANDLE;
+struct EFI_PORT_OBJECT;
+typedef struct EFI_PORT_OBJECT *EFI_PORT_HANDLE;
=20
///
/// EAP Authentication Method Type (RFC 3748)
diff --git a/MdePkg/Include/Protocol/HiiFont.h b/MdePkg/Include/Protocol/=
HiiFont.h
index 1f2e321ea4e2..450cad9ada70 100644
--- a/MdePkg/Include/Protocol/HiiFont.h
+++ b/MdePkg/Include/Protocol/HiiFont.h
@@ -19,8 +19,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
{ 0xe9ca4775, 0x8657, 0x47fc, { 0x97, 0xe7, 0x7e, 0xd6, 0x5a, 0x8, 0x43,=
0x24 } }
=20
typedef struct _EFI_HII_FONT_PROTOCOL EFI_HII_FONT_PROTOCOL;
-
-typedef VOID *EFI_FONT_HANDLE;
+typedef LIST_ENTRY *EFI_FONT_HANDLE;
=20
///
/// EFI_HII_OUT_FLAGS.
diff --git a/MdePkg/Include/Protocol/MmMp.h b/MdePkg/Include/Protocol/MmM=
p.h
index beace1386cbe..cd4e0db47e08 100644
--- a/MdePkg/Include/Protocol/MmMp.h
+++ b/MdePkg/Include/Protocol/MmMp.h
@@ -36,7 +36,8 @@
//
// Completion token
//
-typedef VOID* MM_COMPLETION;
+struct MM_COMPLETION_OBJECT;
+typedef struct MM_COMPLETION_OBJECT* MM_COMPLETION;
=20
typedef struct {
MM_COMPLETION Completion;
diff --git a/MdePkg/Include/Protocol/S3SaveState.h b/MdePkg/Include/Proto=
col/S3SaveState.h
index c1b8f8b9e08d..235c36be6737 100644
--- a/MdePkg/Include/Protocol/S3SaveState.h
+++ b/MdePkg/Include/Protocol/S3SaveState.h
@@ -21,7 +21,7 @@
{ 0xe857caf6, 0xc046, 0x45dc, { 0xbe, 0x3f, 0xee, 0x7, 0x65, 0xfb, 0=
xa8, 0x87 }}
=20
=20
-typedef VOID *EFI_S3_BOOT_SCRIPT_POSITION;
+typedef UINT8 *EFI_S3_BOOT_SCRIPT_POSITION;
=20
typedef struct _EFI_S3_SAVE_STATE_PROTOCOL EFI_S3_SAVE_STATE_PROTOCOL;
=20
diff --git a/MdePkg/Include/Protocol/Shell.h b/MdePkg/Include/Protocol/Sh=
ell.h
index cfb7878228c5..bf791792b4f2 100644
--- a/MdePkg/Include/Protocol/Shell.h
+++ b/MdePkg/Include/Protocol/Shell.h
@@ -11,12 +11,13 @@
#define __EFI_SHELL_PROTOCOL_H__
=20
#include <Guid/FileInfo.h>
+#include <Protocol/SimpleFileSystem.h>
=20
#define EFI_SHELL_PROTOCOL_GUID \
{ \
0x6302d008, 0x7f9b, 0x4f30, { 0x87, 0xac, 0x60, 0xc9, 0xfe, 0xf5, 0xda=
, 0x4e } \
}
-typedef VOID *SHELL_FILE_HANDLE;
+typedef EFI_FILE_PROTOCOL *SHELL_FILE_HANDLE;
=20
typedef enum {
///
diff --git a/MdePkg/Include/Protocol/UserManager.h b/MdePkg/Include/Proto=
col/UserManager.h
index 26ac4955f1ec..9abfcffbeebf 100644
--- a/MdePkg/Include/Protocol/UserManager.h
+++ b/MdePkg/Include/Protocol/UserManager.h
@@ -24,8 +24,10 @@
0xbaf1e6de, 0x209e, 0x4adb, { 0x8d, 0x96, 0xfd, 0x8b, 0x71, 0xf3, 0x=
f6, 0x83 } \
}
=20
-typedef VOID *EFI_USER_PROFILE_HANDLE;
-typedef VOID *EFI_USER_INFO_HANDLE;
+struct EFI_USER_PROFILE_OBJECT;
+typedef struct EFI_USER_PROFILE_OBJECT *EFI_USER_PROFILE_HANDLE;
+struct EFI_USER_INFO_OBJECT;
+typedef struct EFI_USER_INFO_OBJECT *EFI_USER_INFO_HANDLE;
=20
///
/// The attributes of the user profile information.
@@ -157,7 +159,8 @@ typedef CHAR16 *EFI_USER_INFO_CREDENTIAL_PROVIDER_NAM=
E;
/// Biometric Exchange Formats Framework) specification.
///
#define EFI_USER_INFO_CBEFF_RECORD 0x0B
-typedef VOID *EFI_USER_INFO_CBEFF;
+struct EFI_USER_INFO_CBEFF_OBJECT;
+typedef struct EFI_USER_INFO_CBEFF_OBJECT *EFI_USER_INFO_CBEFF;
///
/// Indicates how close of a match the fingerprint must be in order to b=
e considered a match.
///
diff --git a/MdePkg/Include/Uefi/UefiBaseType.h b/MdePkg/Include/Uefi/Uef=
iBaseType.h
index a62f13dd064f..be5831991b52 100644
--- a/MdePkg/Include/Uefi/UefiBaseType.h
+++ b/MdePkg/Include/Uefi/UefiBaseType.h
@@ -28,11 +28,13 @@ typedef RETURN_STATUS EFI_STATUS;
///
/// A collection of related interfaces.
///
-typedef VOID *EFI_HANDLE;
+struct EFI_OBJECT;
+typedef struct EFI_OBJECT *EFI_HANDLE;
///
/// Handle to an event structure.
///
-typedef VOID *EFI_EVENT;
+struct EFI_EVENT_OBJECT;
+typedef struct EFI_EVENT_OBJECT *EFI_EVENT;
///
/// Task priority level.
///
diff --git a/MdePkg/Include/Uefi/UefiInternalFormRepresentation.h b/MdePk=
g/Include/Uefi/UefiInternalFormRepresentation.h
index 4a1346a599d0..93bf9e9e0f13 100644
--- a/MdePkg/Include/Uefi/UefiInternalFormRepresentation.h
+++ b/MdePkg/Include/Uefi/UefiInternalFormRepresentation.h
@@ -20,7 +20,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
///
/// The following types are currently defined:
///
-typedef VOID* EFI_HII_HANDLE;
+struct EFI_HII_OBJECT;
+typedef struct EFI_HII_OBJECT* EFI_HII_HANDLE;
typedef CHAR16* EFI_STRING;
typedef UINT16 EFI_IMAGE_ID;
typedef UINT16 EFI_QUESTION_ID;
diff --git a/MdeModulePkg/Core/Dxe/Event/Event.h b/MdeModulePkg/Core/Dxe/=
Event/Event.h
index 8141c5003eec..42590cb1dd09 100644
--- a/MdeModulePkg/Core/Dxe/Event/Event.h
+++ b/MdeModulePkg/Core/Dxe/Event/Event.h
@@ -37,7 +37,7 @@ typedef struct {
} TIMER_EVENT_INFO;
=20
#define EVENT_SIGNATURE SIGNATURE_32('e','v','n','t')
-typedef struct {
+typedef struct EFI_EVENT_OBJECT {
UINTN Signature;
UINT32 Type;
UINT32 SignalCount;
diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.h b/MdeModulePkg/Core/Dxe/=
Hand/Handle.h
index 83eb2b9f3afe..1f1ab3274e8a 100644
--- a/MdeModulePkg/Core/Dxe/Hand/Handle.h
+++ b/MdeModulePkg/Core/Dxe/Hand/Handle.h
@@ -15,7 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
///
/// IHANDLE - contains a list of protocol handles
///
-typedef struct {
+typedef struct EFI_OBJECT {
UINTN Signature;
/// All handles list of IHANDLE
LIST_ENTRY AllHandles;
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/=
PiSmmCore/PiSmmCore.h
index 0908e7f4e9e7..c55da58d465e 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
@@ -145,7 +145,7 @@ typedef struct {
///
/// IHANDLE - contains a list of protocol handles
///
-typedef struct {
+typedef struct EFI_OBJECT {
UINTN Signature;
/// All handles list of IHANDLE
LIST_ENTRY AllHandles;
diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h b/MdeModu=
lePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
index 50d4c96edb63..bfebbb1f8182 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
@@ -88,7 +88,7 @@ struct _EFI_AML_NODE_LIST {
// This buffer should not be freed.
// Size is the total size of this ACPI node buffer.
//
-typedef struct {
+typedef struct EFI_ACPI_OBJECT {
UINT32 Signature;
UINT8 *Buffer;
UINTN Size;
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h b/MdeMod=
ulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
index 4a3feab94df5..48972d0fcad6 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
@@ -274,7 +274,7 @@ typedef struct _HII_DATABASE_PACKAGE_LIST_INSTANCE {
=20
#define HII_HANDLE_SIGNATURE SIGNATURE_32 ('h','i','h','l')
=20
-typedef struct {
+typedef struct EFI_HII_OBJECT {
UINTN Signature;
LIST_ENTRY Handle;
UINTN Key;
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h b/StandaloneMmPkg/Co=
re/StandaloneMmCore.h
index 4d0eed273f50..dcf91bc5e916 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.h
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
@@ -105,7 +105,7 @@ typedef struct {
///
/// IHANDLE - contains a list of protocol handles
///
-typedef struct {
+typedef struct EFI_OBJECT {
UINTN Signature;
/// All handles list of IHANDLE
LIST_ENTRY AllHandles;
--=20
2.19.1.3.g30247aa5d201


[PATCH 00/35] edk2: clean up the usage of standardized (VOID*) typedefs

Laszlo Ersek
 

Repository: https://github.com/lersek/edk2.git
Branch: voidptr

The UEFI / PI / Shell specifications define a number of standard types
as pointers to VOID. This is arguably a design mistake; those types
should have been pointers to distinct incomplete union or structure
types. Here's why:

Roughly paraphrasing the constraints from ISO C99 "6.5.16.1 Simple
assignment" and "6.5.4 Cast operators", any pointer-to-object type
converts implicitly to, and from, pointer-to-void, provided const /
volatile qualifications are not relaxed. Such implicit conversions
prevent compilers from catching at least the following two kinds of
coding mistakes:

- mixing up one type with another (for example, EFI_HANDLE with
EFI_EVENT),

- getting the depth of indirection wrong (for example, mixing up
(EFI_HANDLE*) with EFI_HANDLE).

This series first separates these standard types from each other, in the
first patch, which is *not* being proposed for merging. This unmasks a
number of warts (semantic issues, or actual bugs) in the source code, in
the form of build breakages. The rest of the series works through those
breakages, cleaning and fixing the code.

Every DSC file in the edk2 tree was built for at least one of the NOOPT,
DEBUG, RELEASE targets (NOOPT being preferred), with the GCC48 toolchain
(for IA32 / X64) and the GCC5 toolchain (for ARM / AARCH64). Of course,
the build arches were restricted to the SUPPORTED_ARCHITECTURES stated
in the individual DSC files.

There were two exceptions to the above rule: DynamicTablesPkg was only
build-tested with AARCH64 (despite its SUPPORTED_ARCHITECTURES), given
that 32-bit ARM has no ACPI bindings. StandaloneMmPkg too was only
build-tested with AARCH64; it doesn't actually support IA32/X64 yet.

Regarding boot & runtime tests, ArmVirtQemu on AARCH64 was tested with
booting to the OS (RHEL7). Furthermore, I exercised OVMF with my usual
boot and S3 tests, covering IA32, IA32X64, and X64. Finally, some other
individual tests (noted per patch) were done with OVMF.

Cc: Achin Gupta <achin.gupta@arm.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jian Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>

Thanks
Laszlo

Laszlo Ersek (35):
DO NOT APPLY: edk2: turn standard handle types into pointers to
non-VOID
EmbeddedPkg: add missing EFIAPI calling convention specifiers
EmbeddedPkg/AndroidFastbootTransportTcpDxe: fix DestroyChild() call
EmbeddedPkg/Universal/MmcDxe: "fix" CloseProtocol() call in
BindingStop()
EmulatorPkg/DxeTimerLib: drop superfluous cast
EmulatorPkg: stop abusing EFI_HANDLE for keystroke notify registration
MdeModulePkg: fix cast in GetModuleInfoFromHandle() calls
MdeModulePkg/UefiHiiLib: stop using EFI_HANDLE in place of
EFI_HII_HANDLE
MdeModulePkg: stop abusing EFI_EVENT for protocol notify registration
MdeModulePkg/PlatformVarCleanupLib: fix HiiConstructConfigHdr() call
MdeModulePkg: document workaround for EFI_RUNTIME_EVENT_ENTRY PI spec
bug
MdeModulePkg: stop abusing EFI_HANDLE for keystroke notify
registration
MdeModulePkg: PEI Core: clean up "AprioriFile" handling in
FindFileEx()
MdeModulePkg: fix UninstallMultipleProtocolInterfaces() calls
MdeModulePkg/PiSmmCore: make type punning consistent
MdeModulePkg/S3SaveState: cast Position for S3BootScriptLib explicitly
MdePkg/DxeServicesLib: remove bogus cast
NetworkPkg/DxeNetLib: fix type typo in NetLibGetMacAddress()
NetworkPkg: fix CloseProtocol & UninstallMultipleProtocolInterfaces
calls
NetworkPkg/Ip4Dxe: fix NetLibDestroyServiceChild() call
NetworkPkg/TcpDxe: fix SockFreeFoo() parameter list
OvmfPkg/XenBusDxe: fix UninstallMultipleProtocolInterfaces() call
OvmfPkg/VirtioNetDxe: fix SignalEvent() call
OvmfPkg/PlatformDxe: fix EFI_HII_HANDLE parameters of internal
functions
OvmfPkg/VideoDxe: document EFI_EDID_OVERRIDE_PROTOCOL.GetEdid() call
SecurityPkg: fix UninstallMultipleProtocolInterfaces() calls
SecurityPkg: stop abusing EFI_EVENT for protocol notify registration
ShellPkg/UefiShellDriver1CommandsLib: fix parameter list typo
ShellPkg: stop using EFI_HANDLE in place of EFI_HII_HANDLE
ShellPkg: stop taking EFI_HANDLE in place of SHELL_FILE_HANDLE
ShellPkg/UefiShellDebug1CommandsLib: fix ShellCloseFile() call
ShellPkg/UefiShellLib: clarify workaround for unfixable EdkShell bug
StandaloneMmPkg/Core: stop abusing EFI_HANDLE for FwVolHeader tracking
UefiPayloadPkg/BlSupportPei: fix MMCONFIG assignment from XSDT
UefiPayloadPkg/BlSupportDxe: fix ReserveResourceInGcd() calls

EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcp.=
c | 2 +-
EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.c =
| 1 +
EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c =
| 1 +
EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.h =
| 32 ++++++--
EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c =
| 8 ++
EmbeddedPkg/GdbStub/GdbStubInternal.h =
| 9 +++
EmbeddedPkg/MetronomeDxe/Metronome.c =
| 1 +
EmbeddedPkg/Universal/MmcDxe/Mmc.c =
| 5 +-
EmulatorPkg/EmuGopDxe/GopInput.c =
| 4 +-
EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c =
| 2 +-
MdeModulePkg/Bus/I2c/I2cDxe/I2cBus.c =
| 2 +-
MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c =
| 2 +-
MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c =
| 6 +-
MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c =
| 2 +-
MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.c =
| 2 +-
MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c =
| 2 +-
MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassImpl.c =
| 2 +-
MdeModulePkg/Core/Dxe/Event/Event.c =
| 8 ++
MdeModulePkg/Core/Dxe/Event/Event.h =
| 2 +-
MdeModulePkg/Core/Dxe/Hand/Handle.h =
| 2 +-
MdeModulePkg/Core/Pei/FwVol/FwVol.c =
| 2 +-
MdeModulePkg/Core/Pei/FwVol/FwVol.h =
| 2 +-
MdeModulePkg/Core/PiSmmCore/PiSmmCore.h =
| 2 +-
MdeModulePkg/Core/PiSmmCore/Smi.c =
| 8 +-
MdeModulePkg/Core/RuntimeDxe/Runtime.c =
| 10 ++-
MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c =
| 12 +--
MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c =
| 6 +-
MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c =
| 8 +-
MdeModulePkg/Library/UefiHiiLib/HiiString.c =
| 4 +-
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h =
| 2 +-
MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveState.c =
| 4 +-
MdeModulePkg/Universal/Acpi/SmmS3SaveState/SmmS3SaveState.c =
| 4 +-
MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c =
| 2 +-
MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c =
| 4 +-
MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c =
| 2 +-
MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c =
| 2 +-
MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h =
| 2 +-
MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c =
| 2 +-
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c =
| 2 +-
MdePkg/Include/Pi/PiPeiCis.h =
| 6 +-
MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h =
| 3 +-
MdePkg/Include/Protocol/Bis.h =
| 3 +-
MdePkg/Include/Protocol/Eap.h =
| 3 +-
MdePkg/Include/Protocol/HiiFont.h =
| 3 +-
MdePkg/Include/Protocol/MmMp.h =
| 3 +-
MdePkg/Include/Protocol/S3SaveState.h =
| 2 +-
MdePkg/Include/Protocol/Shell.h =
| 3 +-
MdePkg/Include/Protocol/UserManager.h =
| 9 ++-
MdePkg/Include/Uefi/UefiBaseType.h =
| 6 +-
MdePkg/Include/Uefi/UefiInternalFormRepresentation.h =
| 3 +-
MdePkg/Library/DxeServicesLib/DxeServicesLib.c =
| 2 +-
NetworkPkg/DnsDxe/DnsDriver.c =
| 4 +-
NetworkPkg/IScsiDxe/IScsiConfig.c =
| 2 +-
NetworkPkg/Ip4Dxe/Ip4Driver.c =
| 2 +-
NetworkPkg/Ip4Dxe/Ip4If.c =
| 4 +-
NetworkPkg/Ip6Dxe/Ip6Driver.c =
| 2 +-
NetworkPkg/Library/DxeNetLib/DxeNetLib.c =
| 2 +-
NetworkPkg/Mtftp4Dxe/Mtftp4Driver.c =
| 2 +-
NetworkPkg/TcpDxe/SockImpl.c =
| 4 +-
NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.c =
| 2 +-
OvmfPkg/Csm/BiosThunk/VideoDxe/BiosVideo.c =
| 6 +-
OvmfPkg/PlatformDxe/Platform.c =
| 4 +-
OvmfPkg/VirtioNetDxe/Events.c =
| 2 +-
OvmfPkg/XenBusDxe/XenBus.c =
| 2 +-
SecurityPkg/HddPassword/HddPasswordDxe.c =
| 2 +-
SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c =
| 2 +-
SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c =
| 2 +-
SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDr=
iver.c | 2 +-
ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c =
| 6 +-
ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h =
| 4 +-
ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c =
| 6 +-
ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.h =
| 4 +-
ShellPkg/Include/Library/ShellCommandLib.h =
| 2 +-
ShellPkg/Include/Library/ShellLib.h =
| 4 +-
ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c =
| 2 +-
ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c =
| 2 +-
ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c =
| 2 +-
ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.h =
| 2 +-
ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/FileBuffer.c =
| 2 +-
ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c=
| 2 +-
ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.h=
| 2 +-
ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c =
| 4 +-
ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib=
.c | 2 +-
ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib=
.h | 2 +-
ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.c=
| 2 +-
ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.h=
| 2 +-
ShellPkg/Library/UefiShellLevel2CommandsLib/Rm.c =
| 2 +-
ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c=
| 2 +-
ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.h=
| 2 +-
ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c =
| 2 +-
ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.c=
| 2 +-
ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.h=
| 2 +-
ShellPkg/Library/UefiShellLib/UefiShellLib.c =
| 26 ++++++-
ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsL=
ib.c | 2 +-
ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsL=
ib.h | 2 +-
ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsL=
ib.c | 2 +-
ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsL=
ib.h | 2 +-
StandaloneMmPkg/Core/Dispatcher.c =
| 80 +++++++++++---------
StandaloneMmPkg/Core/FwVol.c =
| 16 ++--
StandaloneMmPkg/Core/StandaloneMmCore.h =
| 4 +-
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c =
| 4 +-
UefiPayloadPkg/BlSupportPei/BlSupportPei.c =
| 19 +++--
102 files changed, 294 insertions(+), 194 deletions(-)

--=20
2.19.1.3.g30247aa5d201


Re: [PATCH 0/2] q35: mch: allow to lock down 128K RAM at default SMBASE address

no-reply@...
 

Patchew URL: https://patchew.org/QEMU/20190917130708.10281-1-imammedo@redhat.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

./tests/docker/docker.py --engine auto build qemu:fedora tests/docker/dockerfiles/fedora.docker --add-current-user
Image is up to date.
LD docker-test-debug@fedora.mo
cc: fatal error: no input files
compilation terminated.
make: *** [docker-test-debug@fedora.mo] Error 4



The full log is available at
http://patchew.org/logs/20190917130708.10281-1-imammedo@redhat.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com


Re: [PATCH 0/2] q35: mch: allow to lock down 128K RAM at default SMBASE address

no-reply@...
 

Patchew URL: https://patchew.org/QEMU/20190917130708.10281-1-imammedo@redhat.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

./tests/docker/docker.py --engine auto build qemu:fedora tests/docker/dockerfiles/fedora.docker --add-current-user
Image is up to date.
LD docker-test-mingw@fedora.mo
cc: fatal error: no input files
compilation terminated.
make: *** [docker-test-mingw@fedora.mo] Error 4



The full log is available at
http://patchew.org/logs/20190917130708.10281-1-imammedo@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com


Re: [PATCH edk2-CCSS 0/3] Coding Standards: add rule for documenting spurious variable assignments

Michael D Kinney
 

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

I also agree that the macros would be cleaner, easy to review, and
and fewer lines of code without the comment block. If I objected
previously, then I have also changed my mind. I agree we can go
ahead and push the series in its current form and continue the
discussion on the macros.

Mike

-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com>
Sent: Wednesday, September 11, 2019 10:51 AM
To: devel@edk2.groups.io; ryszard.knop@linux.intel.com;
leif.lindholm@linaro.org
Cc: Andrew Fish <afish@apple.com>; Kinney, Michael D
<michael.d.kinney@intel.com>; Rebecca Cran
<rebecca@bsdio.com>; Philippe Mathieu-Daude
<philmd@redhat.com>
Subject: Re: [edk2-devel] [PATCH edk2-CCSS 0/3] Coding
Standards: add rule for documenting spurious variable
assignments

On 09/10/19 17:44, Ryszard Knop wrote:
On Tue, 2019-09-10 at 16:33 +0100, Leif Lindholm
wrote:
On Mon, Sep 09, 2019 at 02:35:15PM +0200, Laszlo
Ersek wrote:
On 09/06/19 14:26, Leif Lindholm wrote:
On Thu, Sep 05, 2019 at 08:38:17PM +0200, Laszlo
Ersek wrote:
Repo:
https://github.com/lersek/edk2-
CCodingStandardsSpecification.git
Branch: spurious_assign_bz_607

HTML-rendered views of the modified pages:
-
https://lersek.gitbooks.io/laszlo-s-fork-of-the-
edk-ii-c-coding-st
andards-sp/content/v/spurious_assign_bz_607
-
https://lersek.gitbooks.io/laszlo-s-fork-of-the-
edk-ii-c-coding-st
andards-
sp/content/v/spurious_assign_bz_607/6_documenting_softw
are
/62_comments.html
-
https://lersek.gitbooks.io/laszlo-s-fork-of-the-
edk-ii-c-coding-st
andards-
sp/content/v/spurious_assign_bz_607/6_documenting_softw
are
/64_what_you_must_comment.html

The first two patches are cleanups for things
that popped up in
the discussion in <
https://bugzilla.tianocore.org/show_bug.cgi?id=607>;.

The third patch is the one fixing the BZ.
For 1 and 2,
Reviewed-by: Leif Lindholm
<leif.lindholm@linaro.org>

For 3, I see no issue with it, but I do feel
tempted by Phil's
input of using explicit macros (obviating the need
for specific
comment).
I seem to recall back in the mists of time we
considered something
similar.
Yes, I remember similarly.

Vaguely. Am I misremembering, or did we disount
that option?

Phil's current recommendation is what I would have
preferred back
then, but it was rejected, as far as I recall. If I
remember
correctly, most developers preferred naked NULLs /
zeroes. I
insisted on the comment as a fallback / compromise,
so that we'd
have at least some visual cue.
I'm not even sure I wasn't one of the people opposed
to it then.
But if I was, I would appear to have changed my
mind.

I could be mis-remembering; we can restart that
discussion if now
the macros are preferred.
I would be all for that.
If my 2 cents are worth anything, that'd be preferred
by some folks in
my team too. Although something shorter like
"UNINITIALIZED_INT/PTR"
would be nicer, IMO. Both work of course.
Thanks everyone for the feedback thus far on this
series. It looks like I could go ahead and push the
patches, minimally for bringing the CCSS in closer sync
with reality -- and then we could improve
incrementally, for example with macros.

But, before I push the set, I'd really like hear Mike's
opinion too -- I vaguely recall he was active in the
original discussion. I wouldn't like to back out the
patches in case Mike rejected them retroactively.

I believe Mike will have a bit of an email backlog to
process ;) so I'll wait some more in this thread.

Thanks!
Laszlo

However, I see no reason why we shouldn't document
the current
process in the meantime, so for 3/3 also:
Reviewed-by: Leif Lindholm
<leif.lindholm@linaro.org>

Best Regards,

Leif

Thanks,
Laszlo

Regards,

Leif

Thanks,
Laszlo

Cc: Andrew Fish <afish@apple.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Rebecca Cran <rebecca@bsdio.com>

Laszlo Ersek (3):
comments: remove "Horror Vacui" rule
comments: restrict and clarify applicability of
"/*" comments
must comment: add rule for documenting spurious
variable
assignments

6_documenting_software/62_comments.md
| 20 +-----
----
6_documenting_software/64_what_you_must_comment.md | 39
++++++++++++++++++++
README.md
| 1 +
3 files changed, 42 insertions(+), 18
deletions(-)

--
2.19.1.3.g30247aa5d201




Re: [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Allow other NSIDs for Admin commands

Tyler J Erickson <tyler.j.erickson@...>
 

Hi Hao,

Sorry for the late reply.
I tested and can confirm that if the NSID of the command and the provided NamespaceID as an input match, the commands do go through on the EDK2 driver. This is not the case with a driver built into a motherboard that I have here, as it only accepts values for the current NSID and nothing else. This is ok since I can load the EDK2 driver in place instead.

Since this is unclear in the documentation for the passthru function, can we make a documentation change instead?

Thanks,
-Tyler


On Wed, Sep 4, 2019 at 8:07 PM Wu, Hao A <hao.a.wu@...> wrote:

Hello Tyler,

 

Some inline comments below.

 

Best Regards,

Hao Wu

 

From: Tyler J Erickson [mailto:tyler.j.erickson@...]
Sent: Wednesday, September 04, 2019 11:07 PM
To: Wu, Hao A
Cc: devel@edk2.groups.io; Ni, Ray
Subject: Re: [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Allow other NSIDs for Admin commands

 

Hi Hao,

 

I tried making both the NamespaceID and NSID values the same when calling the passthru function for these admin commands and it didn't work. I think that is due to another place in the passthru code filtering the NamespaceId input values on line 517-520.

The NamespaceId parameter is being checked to make sure it isn't greater than the number of supported namespaces by the controller and it makes sure it isn't set to (UINT32)-1 (All F's). 

 

 

[Hao]: I think the check on line 517-520 blocks ‘NamespaceId’ with value greater than the number of namespace. But it allows the value to be MAX_UINT32, which means the command takes effect on all namespaces.

 

 

I think this is correct since the NamespaceId input is what is discovered when calling the EFI_NVM_EXPRESS_PASS_THRU_GET_NEXT_NAMESPACE function. This is what I used to get the available namespaces available in the system.

 

In my case I'm sending some commands in my application with the NSID set to UINT32_MAX in order to request controller wide SMART/Health log data among other things. There are some commands where setting the NSID to another value may also be useful. For example, DST can use the NSID in the command to change between testing only the controller (0), testing all namespaces (UINT32_MAX), and a specific namespace.

 

 

[Hao]: Yes. Just as you said, commands like DST or Get Log Page will have different target when different Namespace values are provided.

I think for your case, you can:

1.       Set both the 'NamespaceId' parameter and 'Nsid' field to 0 if the target is the controller;

2.       Set both of them MAX_UINT32 if the target is all the namespaces;

3.       Set both of them to a corresponding ID if the target is a specific namespace.

 

As you can see from the current implementation of NvmExpressPassThru() function, the 'NamespaceId' parameter does not affect the construct of the submission queue item.

The 'NamespaceId' parameter only serves as a valid check for the 'Nsid' field in the EFI_NVM_EXPRESS_COMMAND structure.

 

 

The modification I made was done where the passthru command's NSID was actually being checked, which seemed like the best place to add this exception for admin commands.


-Tyler

 

 

On Tue, Sep 3, 2019 at 9:39 PM Wu, Hao A <hao.a.wu@...> wrote:

> -----Original Message-----
> From: Wu, Hao A
> Sent: Wednesday, September 04, 2019 11:39 AM
> To: devel@edk2.groups.io; Tyler Erickson
> Cc: Wu, Hao A; Ni, Ray
> Subject: [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Allow other NSIDs
> for Admin commands
>
> Repost the mail to the list.
>
> Best Regards,
> Hao Wu
>
> -----Original Message-----
> From: Tyler Erickson [mailto:tyler.j.erickson@...]
> Sent: Tuesday, September 03, 2019 9:55 PM
> To: edk2-devel@...
> Cc: Wu, Hao A; Ni, Ray
> Subject: [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Allow other NSIDs
> for Admin commands
>
> Cc: Hao A Wu <hao.a.wu@...>
> Cc: Ray Ni <ray.ni@...>
> Signed-off-by: Tyler Erickson <tyler.j.erickson@...>
> ---
>  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> index 8e721379466a..78a3c663ded4 100644
> --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> @@ -561,7 +561,7 @@ NvmExpressPassThru (
>    Sq  = Private->SqBuffer[QueueId] + Private->SqTdbl[QueueId].Sqt;
>    Cq  = Private->CqBuffer[QueueId] + Private->CqHdbl[QueueId].Cqh;
>
> -  if (Packet->NvmeCmd->Nsid != NamespaceId) {
> +  if (Packet->QueueType != NVME_ADMIN_QUEUE && Packet->NvmeCmd-
> >Nsid != NamespaceId) {


Hello,

Per my understanding to the codes, I think the:

* Input parameter 'NamespaceId' for the PassThru() service
* The 'Nsid' field of the EFI_NVM_EXPRESS_COMMAND

are of the same meaning.

Do you think setting these 2 values identical when calling the PassThru()
service can resolve the issue you met?

Best Regards,
Hao Wu


>      return EFI_INVALID_PARAMETER;
>    }
>
> --
> 2.17.1


Re: [External] Re: [PATCH v1 1/1] Drivers/DisplayLink/DisplayLinkPkg DisplayLinkGop

Andy Hayes
 

That’s right, the only (current) request was index 0 – that is why it didn’t show up. It was a refactoring error.

 

It was picked up when we ported some of the changes back into our “closed source” version of the driver and the unit tests failed.

 

Thanks for pushing this.

 

From: Leif Lindholm <leif.lindholm@...>
Sent: 17 September 2019 16:28
To: Andy Hayes <andy.hayes@...>
Cc: devel@edk2.groups.io; Ard Biesheuvel <ard.biesheuvel@...>
Subject: [External] Re: [PATCH v1 1/1] Drivers/DisplayLink/DisplayLinkPkg DisplayLinkGop

 

On Wed, Sep 11, 2019 at 07:42:03AM +0000, Andy Hayes wrote:
> Corrected initialisation of one of data structures used to transmit USB
> control messages. Mistake had no practical effects but fixing to be on safe
> side.

So, was the only request used index 0? Or why didn't this cause an
issue? Nevertheless, a clear fix.

> Cc: Leif Lindholm <leif.lindholm@...>
> Cc: Ard Biesheuvel <ard.biesheuvel@...>
> Signed-off-by: Andy Hayes <andy.hayes@...>

Reviewed-by: Leif Lindholm <leif.lindholm@...>
Pushed as 958aaf600728.

/
Leif

> ---
> Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c b/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c
> index 252293da39d4..9871ab0378ce 100644
> --- a/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c
> +++ b/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c
> @@ -107,7 +107,7 @@ DlUsbSendControlWriteMessage (
> UINT32 UsbStatus;
> EFI_USB_DEVICE_REQUEST UsbRequest;
>
> - ZeroMem (&Request, sizeof (Request));
> + ZeroMem (&UsbRequest, sizeof (UsbRequest));
> UsbRequest.RequestType = USB_REQ_TYPE_VENDOR | USB_TARGET_INTERFACE;
> UsbRequest.Index = Device->InterfaceDescriptor.InterfaceNumber;
> UsbRequest.Request = Request;
> --
> 1.8.3.1
>


Re: [PATCH v1 1/1] Drivers/DisplayLink/DisplayLinkPkg DisplayLinkGop

Leif Lindholm
 

On Wed, Sep 11, 2019 at 07:42:03AM +0000, Andy Hayes wrote:
Corrected initialisation of one of data structures used to transmit USB
control messages. Mistake had no practical effects but fixing to be on safe
side.
So, was the only request used index 0? Or why didn't this cause an
issue? Nevertheless, a clear fix.

Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Andy Hayes <andy.hayes@displaylink.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Pushed as 958aaf600728.

/
Leif

---
Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c b/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c
index 252293da39d4..9871ab0378ce 100644
--- a/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c
+++ b/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c
@@ -107,7 +107,7 @@ DlUsbSendControlWriteMessage (
UINT32 UsbStatus;
EFI_USB_DEVICE_REQUEST UsbRequest;

- ZeroMem (&Request, sizeof (Request));
+ ZeroMem (&UsbRequest, sizeof (UsbRequest));
UsbRequest.RequestType = USB_REQ_TYPE_VENDOR | USB_TARGET_INTERFACE;
UsbRequest.Index = Device->InterfaceDescriptor.InterfaceNumber;
UsbRequest.Request = Request;
--
1.8.3.1