Re: [edk2] [PATCH v2 0/6] Code refinements in UdfDxe
Series Reviewed-by: Star Zeng Thanks, Star -Original Message- From: Wu, Hao A Sent: Tuesday, October 16, 2018 4:15 PM To: edk2-devel@lists.01.org Cc: Wu, Hao A ; Paulo Alcantara ; Paulo Alcantara ; Ni, Ruiyu ; Zeng, Star ; Yao, Jiewen Subject: [PATCH v2 0/6] Code refinements in UdfDxe Note: Since PATCH v2 1/6 ~ 5/6 have not been changed, add the 'Reviewed-by:' tag during the v1 series review. V2 changes: A. Drop PATCH v1 6/7, since removing those codes will make the function MangleFileName() not matching its original implementation purpose (according to the function description). B. Drop change C in PATCH v1 7/7. It is reasonable for function SetFileInfo() to handle the expected value for the input parameters. C. Refine the log message for PATCH v1 7/7. V1 history: This series will refine the codes in MdeModulePkg/Universal/Disk/UdfDxe for: A. Refine asserts used for memory allocation failure and error cases that are possible to happen. Will use error handling logic for them; B. Address some dead codes within this module. Cc: Paulo Alcantara Cc: Paulo Alcantara Cc: Ruiyu Ni Cc: Star Zeng Cc: Jiewen Yao Hao Wu (6): MdeModulePkg/UdfDxe: Use error handling for memory allocation failure MdeModulePkg/UdfDxe: ASSERT for false positives of NULL ptr deref MdeModulePkg/UdfDxe: Use error handling when fail to return LSN MdeModulePkg/UdfDxe: Use debug msg instead of ASSERT in UdfOpen() MdeModulePkg/UdfDxe: Handle dead codes in File.c MdeModulePkg/UdfDxe: Handle dead codes in FileSystemOperations.c MdeModulePkg/Universal/Disk/UdfDxe/File.c | 19 ++- MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 158 +++- 2 files changed, 138 insertions(+), 39 deletions(-) -- 2.12.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 00/12] Need to validate data from HW
Oh, Remember the missing update in UsbMassBoot.h about adding back '.', you can do that when pushing patches. - Read some blocks from the device by SCSI 16 byte cmd. + Read or write some blocks from the device by SCSI 16 byte cmd Thanks, Star -Original Message- From: Zeng, Star Sent: Wednesday, October 17, 2018 10:44 AM To: Ni, Ruiyu ; edk2-devel@lists.01.org Cc: Zeng, Star Subject: RE: [edk2] [PATCH v2 00/12] Need to validate data from HW Series Reviewed-by: Star Zeng Thanks, Star -Original Message- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni Sent: Tuesday, October 16, 2018 1:43 PM To: edk2-devel@lists.01.org Subject: [edk2] [PATCH v2 00/12] Need to validate data from HW The patches contain logic to check the data from HW before using. It can avoid corrupted data from HW causes software behave abnormally. V2: adopt all comments from Star. The block size 0/64K handle is in a separate patch "MdeModulePkg/UsbMass: Reject device whose block size is 0 or > 64K" Hao Wu (1): MdeModulePkg/Bus/Ufs: Ensure device not return more data than expected Ruiyu Ni (11): MdeModulePkg/UsbMass: Merge UsbBoot(Read|Write)Blocks(16) MdeModulePkg/UsbMass: Fix integer overflow when BlockSize is 1 MdeModulePkg/UsbBus: Fix out-of-bound read access to descriptors MdeModulePkg/UsbBus: Reject descriptor whose length is bad MdeModulePkg/Usb: Make sure data from HW is no more than expected SourceLevelDebugPkg/Usb3: Make sure data from HW can fit in buffer MdeModulePkg/UsbKb: Don't access key codes when length is wrong MdeModulePkg/AbsPointer: Don't access key codes when length is wrong MdeModulePkg/UsbMouse: Don't access key codes when length is wrong MdeModulePkg/UsbBus: Deny when the string descriptor length is odd MdeModulePkg/UsbMass: Reject device whose block size is 0 or > 64K MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c | 9 +- MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c | 7 +- MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 9 +- MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c| 19 +- .../Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c| 30 ++- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 70 +- MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 4 + .../Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c| 276 ++--- .../Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h| 76 ++ .../Bus/Usb/UsbMassStorageDxe/UsbMassImpl.c| 8 +- .../UsbMouseAbsolutePointer.c | 8 +- MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c| 8 +- .../DebugCommunicationLibUsb3Transfer.c| 7 + 13 files changed, 237 insertions(+), 294 deletions(-) -- 2.16.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] CorebootPayloadPkg: don't use serial output for Release build
Hi, Reviewed-by: Benjamin You Thanks, - ben > -Original Message- > From: Wonkyu Kim [mailto:norwayfores...@gmail.com] > Sent: Tuesday, October 16, 2018 6:45 AM > To: edk2-devel@lists.01.org > Cc: You, Benjamin ; Agyeman, Prince > ; gauml...@gmail.com; > stefan.reina...@coreboot.org; Williams, Hannah ; > Kim, Wonkyu ; Zhao, Lijian > Subject: [PATCH] CorebootPayloadPkg: don't use serial output for Release build > > From: Wonkyu Kim > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Wonkyu Kim > --- > CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc| 4 > CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 4 > 2 files changed, 8 insertions(+) > > diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc > b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc > index b6cdb697a5b0..7d5052be9301 100644 > --- a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc > +++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc > @@ -263,7 +263,11 @@ > # > > # > ### > [PcdsFeatureFlag] > +!if $(TARGET) == DEBUG >gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|TRUE > +!else > + gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE > +!endif >gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FALSE >gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode|FALSE >gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE > diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc > b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc > index c3fe099e5fec..0484e941cce7 100644 > --- a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc > +++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc > @@ -263,7 +263,11 @@ > # > > # > ### > [PcdsFeatureFlag] > +!if $(TARGET) == DEBUG >gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|TRUE > +!else > + gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE > +!endif >gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FALSE >gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode|TRUE >gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE > -- > 2.17.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 00/12] Need to validate data from HW
Series Reviewed-by: Star Zeng Thanks, Star -Original Message- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni Sent: Tuesday, October 16, 2018 1:43 PM To: edk2-devel@lists.01.org Subject: [edk2] [PATCH v2 00/12] Need to validate data from HW The patches contain logic to check the data from HW before using. It can avoid corrupted data from HW causes software behave abnormally. V2: adopt all comments from Star. The block size 0/64K handle is in a separate patch "MdeModulePkg/UsbMass: Reject device whose block size is 0 or > 64K" Hao Wu (1): MdeModulePkg/Bus/Ufs: Ensure device not return more data than expected Ruiyu Ni (11): MdeModulePkg/UsbMass: Merge UsbBoot(Read|Write)Blocks(16) MdeModulePkg/UsbMass: Fix integer overflow when BlockSize is 1 MdeModulePkg/UsbBus: Fix out-of-bound read access to descriptors MdeModulePkg/UsbBus: Reject descriptor whose length is bad MdeModulePkg/Usb: Make sure data from HW is no more than expected SourceLevelDebugPkg/Usb3: Make sure data from HW can fit in buffer MdeModulePkg/UsbKb: Don't access key codes when length is wrong MdeModulePkg/AbsPointer: Don't access key codes when length is wrong MdeModulePkg/UsbMouse: Don't access key codes when length is wrong MdeModulePkg/UsbBus: Deny when the string descriptor length is odd MdeModulePkg/UsbMass: Reject device whose block size is 0 or > 64K MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c | 9 +- MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c | 7 +- MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 9 +- MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c| 19 +- .../Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c| 30 ++- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 70 +- MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 4 + .../Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c| 276 ++--- .../Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h| 76 ++ .../Bus/Usb/UsbMassStorageDxe/UsbMassImpl.c| 8 +- .../UsbMouseAbsolutePointer.c | 8 +- MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c| 8 +- .../DebugCommunicationLibUsb3Transfer.c| 7 + 13 files changed, 237 insertions(+), 294 deletions(-) -- 2.16.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] BaseTools/ECC: Add a checkpoint to check no usage for deprecated functions.
From: Hess Chen
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hess Chen
---
BaseTools/Source/Python/Ecc/Check.py | 60
BaseTools/Source/Python/Ecc/Configuration.py | 3 ++
BaseTools/Source/Python/Ecc/EccToolError.py | 2 +
BaseTools/Source/Python/Ecc/config.ini | 2 +
4 files changed, 67 insertions(+)
diff --git a/BaseTools/Source/Python/Ecc/Check.py
b/BaseTools/Source/Python/Ecc/Check.py
index eb086362bd..3baf81fa44 100644
--- a/BaseTools/Source/Python/Ecc/Check.py
+++ b/BaseTools/Source/Python/Ecc/Check.py
@@ -270,6 +270,66 @@ class Check(object):
self.FunctionLayoutCheckPrototype()
self.FunctionLayoutCheckBody()
self.FunctionLayoutCheckLocalVariable()
+self.FunctionLayoutCheckDeprecated()
+
+# To check if the deprecated functions are used
+def FunctionLayoutCheckDeprecated(self):
+if EccGlobalData.gConfig.CFunctionLayoutCheckNoDeprecated == '1' or
EccGlobalData.gConfig.CFunctionLayoutCheckAll == '1' or
EccGlobalData.gConfig.CheckAll == '1':
+EdkLogger.quiet("Checking function no deprecated one being used
...")
+
+DeprecatedFunctionList = ['UnicodeValueToString',
+ 'AsciiValueToString',
+ 'StrCpy',
+ 'StrnCpy',
+ 'StrCat',
+ 'StrnCat',
+ 'UnicodeStrToAsciiStr',
+ 'AsciiStrCpy',
+ 'AsciiStrnCpy',
+ 'AsciiStrCat',
+ 'AsciiStrnCat',
+ 'AsciiStrToUnicodeStr',
+ 'PcdSet8',
+ 'PcdSet16',
+ 'PcdSet32',
+ 'PcdSet64',
+ 'PcdSetPtr',
+ 'PcdSetBool',
+ 'PcdSetEx8',
+ 'PcdSetEx16',
+ 'PcdSetEx32',
+ 'PcdSetEx64',
+ 'PcdSetExPtr',
+ 'PcdSetExBool',
+ 'LibPcdSet8',
+ 'LibPcdSet16',
+ 'LibPcdSet32',
+ 'LibPcdSet64',
+ 'LibPcdSetPtr',
+ 'LibPcdSetBool',
+ 'LibPcdSetEx8',
+ 'LibPcdSetEx16',
+ 'LibPcdSetEx32',
+ 'LibPcdSetEx64',
+ 'LibPcdSetExPtr',
+ 'LibPcdSetExBool',
+ 'GetVariable',
+ 'GetEfiGlobalVariable',
+ ]
+
+for IdentifierTable in EccGlobalData.gIdentifierTableList:
+SqlCommand = """select ID, Name, BelongsToFile from %s
+where Model = %s """ % (IdentifierTable,
MODEL_IDENTIFIER_FUNCTION_CALLING)
+RecordSet = EccGlobalData.gDb.TblFile.Exec(SqlCommand)
+for Record in RecordSet:
+for Key in DeprecatedFunctionList:
+if Key == Record[1]:
+if not
EccGlobalData.gException.IsException(ERROR_C_FUNCTION_LAYOUT_CHECK_NO_DEPRECATE,
Key):
+OtherMsg = 'The function [%s] is deprecated
which should NOT be used' % Key
+
EccGlobalData.gDb.TblReport.Insert(ERROR_C_FUNCTION_LAYOUT_CHECK_NO_DEPRECATE,
+
OtherMsg=OtherMsg,
+
BelongsToTable=IdentifierTable,
+
BelongsToItem=Record[0])
def WalkTree(self):
IgnoredPattern = c.GetIgnoredDirListPattern()
diff --git a/BaseTools/Source/Python/Ecc/Configuration.py
b/BaseTools/Source/Python/Ecc/Configuration.py
index c19a3990c7..8f6886169c 100644
--- a/BaseTools/Source/Python/Ecc/Configuration.py
+++ b/BaseTools/Source/Python/Ecc/Configuration.py
@@ -34,6 +34,7 @@ _ConfigFileToInternalTranslation = {
"CFunctionLayoutCheckFunctionBody":"CFunctionLayoutCheckFunctionBody",
"CFunctionLayoutCheckFunctionName":"CFunctionLayoutCheckFunctionName",
[edk2] [Patch v2 3/6] UefiCpuPkg/RegisterCpuFeaturesLib: Add logic to support semaphore type.
V2 changes include:
1. Add more description for the code part which need easy to understand.
2. Refine some code base on feedback for V1 changes.
V1 changes include:
In a system which has multiple cores, current set register value task costs
huge times.
After investigation, current set MSR task costs most of the times. Current
logic uses
SpinLock to let set MSR task as an single thread task for all cores. Because
MSR has
scope attribute which may cause GP fault if multiple APs set MSR at the same
time,
current logic use an easiest solution (use SpinLock) to avoid this issue, but
it will
cost huge times.
In order to fix this performance issue, new solution will set MSRs base on
their scope
attribute. After this, the SpinLock will not needed. Without SpinLock, new
issue raised
which is caused by MSR dependence. For example, MSR A depends on MSR B which
means MSR A
must been set after MSR B has been set. Also MSR B is package scope level and
MSR A is
thread scope level. If system has multiple threads, Thread 1 needs to set the
thread level
MSRs and thread 2 needs to set thread and package level MSRs. Set MSRs task for
thread 1
and thread 2 like below:
Thread 1 Thread 2
MSR B NY
MSR A YY
If driver don't control execute MSR order, for thread 1, it will execute MSR A
first, but
at this time, MSR B not been executed yet by thread 2. system may trig
exception at this
time.
In order to fix the above issue, driver introduces semaphore logic to control
the MSR
execute sequence. For the above case, a semaphore will be add between MSR A and
B for
all threads. Semaphore has scope info for it. The possible scope value is core
or package.
For each thread, when it meets a semaphore during it set registers, it will 1)
release
semaphore (+1) for each threads in this core or package(based on the scope info
for this
semaphore) 2) acquire semaphore (-1) for all the threads in this core or
package(based
on the scope info for this semaphore). With these two steps, driver can control
MSR
sequence. Sample code logic like below:
//
// First increase semaphore count by 1 for processors in this package.
//
for (ProcessorIndex = 0; ProcessorIndex < PackageThreadsCount ;
ProcessorIndex ++) {
LibReleaseSemaphore ((UINT32 *) [PackageOffset +
ProcessorIndex]);
}
//
// Second, check whether the count has reach the check number.
//
for (ProcessorIndex = 0; ProcessorIndex < ValidApCount; ProcessorIndex ++) {
LibWaitForSemaphore ([ApOffset]);
}
Platform Requirement:
1. This change requires register MSR setting base on MSR scope info. If still
register MSR
for all threads, exception may raised.
Known limitation:
1. Current CpuFeatures driver supports DXE instance and PEI instance. But
semaphore logic
requires Aps execute in async mode which is not supported by PEI driver. So
CpuFeature
PEI instance not works after this change. We plan to support async mode for
PEI in phase
2 for this task.
Cc: Ruiyu Ni
Cc: Laszlo Ersek
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong
---
.../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 438 ---
.../DxeRegisterCpuFeaturesLib.c| 71 ++-
.../DxeRegisterCpuFeaturesLib.inf | 3 +
.../PeiRegisterCpuFeaturesLib.c| 55 ++-
.../PeiRegisterCpuFeaturesLib.inf | 1 +
.../RegisterCpuFeaturesLib/RegisterCpuFeatures.h | 61 ++-
.../RegisterCpuFeaturesLib.c | 484 ++---
7 files changed, 980 insertions(+), 133 deletions(-)
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index ba3fb3250f..2bf2254602 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -145,6 +145,19 @@ CpuInitDataInitialize (
CPU_FEATURES_INIT_ORDER *InitOrder;
CPU_FEATURES_DATA*CpuFeaturesData;
LIST_ENTRY *Entry;
+ UINT32 Core;
+ UINT32 Package;
+ UINT32 Thread;
+ EFI_CPU_PHYSICAL_LOCATION*Location;
+ BOOLEAN *CoresVisited;
+ UINTNIndex;
+ ACPI_CPU_DATA*AcpiCpuData;
+ CPU_STATUS_INFORMATION *CpuStatus;
+ UINT32 *ValidCorePerPackage;
+
+ Core= 0;
+ Package = 0;
+ Thread = 0;
CpuFeaturesData = GetCpuFeaturesData ();
CpuFeaturesData->InitOrder = AllocateZeroPool (sizeof
(CPU_FEATURES_INIT_ORDER) * NumberOfCpus);
@@ -163,6 +176,17 @@ CpuInitDataInitialize (
Entry = Entry->ForwardLink;
}
+
[edk2] [Patch v2 5/6] UefiCpuPkg/CpuS3DataDxe: Keep old data if value already existed.
AcpiCpuData add new fields, keep these fields if old data already existed.
Cc: Ruiyu Ni
Cc: Laszlo Ersek
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong
---
UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
index ef98239844..1b847e453a 100644
--- a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
+++ b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
@@ -259,6 +259,8 @@ CpuS3DataInitialize (
if (OldAcpiCpuData != NULL) {
AcpiCpuData->RegisterTable = OldAcpiCpuData->RegisterTable;
AcpiCpuData->PreSmmInitRegisterTable =
OldAcpiCpuData->PreSmmInitRegisterTable;
+AcpiCpuData->ApLocation = OldAcpiCpuData->ApLocation;
+CopyMem (>CpuStatus, >CpuStatus, sizeof
(CPU_STATUS_INFORMATION));
} else {
//
// Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable for
all CPUs
--
2.15.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch v2 6/6] UefiCpuPkg/CpuCommonFeaturesLib: Register MSR base on scope Info.
Because MSR has scope attribute, driver has no needs to set
MSR for all APs if MSR scope is core or package type. This patch
updates code to base on the MSR scope value to add MSR to the register
table.
Cc: Ruiyu Ni
Cc: Laszlo Ersek
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong
---
UefiCpuPkg/Library/CpuCommonFeaturesLib/C1e.c | 8 +
UefiCpuPkg/Library/CpuCommonFeaturesLib/Eist.c | 12 +++
.../Library/CpuCommonFeaturesLib/ExecuteDisable.c | 10 ++
.../Library/CpuCommonFeaturesLib/FastStrings.c | 12 +++
.../Library/CpuCommonFeaturesLib/FeatureControl.c | 38 ++
.../CpuCommonFeaturesLib/LimitCpuIdMaxval.c| 14
.../Library/CpuCommonFeaturesLib/MachineCheck.c| 38 ++
.../Library/CpuCommonFeaturesLib/MonitorMwait.c| 15 +
.../Library/CpuCommonFeaturesLib/PendingBreak.c| 11 +++
UefiCpuPkg/Library/CpuCommonFeaturesLib/Ppin.c | 11 +++
.../Library/CpuCommonFeaturesLib/ProcTrace.c | 11 +++
UefiCpuPkg/Library/CpuCommonFeaturesLib/X2Apic.c | 10 ++
12 files changed, 190 insertions(+)
diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/C1e.c
b/UefiCpuPkg/Library/CpuCommonFeaturesLib/C1e.c
index 47116355a8..1beaebe69c 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/C1e.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/C1e.c
@@ -67,6 +67,14 @@ C1eInitialize (
IN BOOLEAN State
)
{
+ //
+ // The scope of C1EEnable bit in the MSR_NEHALEM_POWER_CTL is Package, only
program
+ // MSR_FEATURE_CONFIG for thread 0 core 0 in each package.
+ //
+ if ((CpuInfo->ProcessorInfo.Location.Thread != 0) ||
(CpuInfo->ProcessorInfo.Location.Core != 0)) {
+ return RETURN_SUCCESS;
+ }
+
CPU_REGISTER_TABLE_WRITE_FIELD (
ProcessorNumber,
Msr,
diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/Eist.c
b/UefiCpuPkg/Library/CpuCommonFeaturesLib/Eist.c
index 2038171a14..f30117d2c5 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/Eist.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/Eist.c
@@ -69,6 +69,18 @@ EistInitialize (
IN BOOLEAN State
)
{
+ //
+ // The scope of the MSR_IA32_MISC_ENABLE is core for below processor type,
only program
+ // MSR_IA32_MISC_ENABLE for thread 0 in each core.
+ //
+ if (IS_ATOM_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) ||
+ IS_CORE_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) ||
+ IS_CORE2_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) {
+if (CpuInfo->ProcessorInfo.Location.Thread != 0) {
+ return RETURN_SUCCESS;
+}
+ }
+
CPU_REGISTER_TABLE_WRITE_FIELD (
ProcessorNumber,
Msr,
diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ExecuteDisable.c
b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ExecuteDisable.c
index 921656a1e8..ff06cb9b60 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ExecuteDisable.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ExecuteDisable.c
@@ -79,6 +79,16 @@ ExecuteDisableInitialize (
IN BOOLEAN State
)
{
+ //
+ // The scope of the MSR_IA32_EFER is core for below processor type, only
program
+ // MSR_IA32_EFER for thread 0 in each core.
+ //
+ if (IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel))
{
+if (CpuInfo->ProcessorInfo.Location.Thread != 0) {
+ return RETURN_SUCCESS;
+}
+ }
+
CPU_REGISTER_TABLE_WRITE_FIELD (
ProcessorNumber,
Msr,
diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/FastStrings.c
b/UefiCpuPkg/Library/CpuCommonFeaturesLib/FastStrings.c
index 029bcf87b3..2682093c23 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/FastStrings.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/FastStrings.c
@@ -40,6 +40,18 @@ FastStringsInitialize (
IN BOOLEAN State
)
{
+ //
+ // The scope of FastStrings bit in the MSR_IA32_MISC_ENABLE is core for
below processor type, only program
+ // MSR_IA32_MISC_ENABLE for thread 0 in each core.
+ //
+ if (IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)
||
+ IS_GOLDMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) ||
+ IS_PENTIUM_4_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) {
+if (CpuInfo->ProcessorInfo.Location.Thread != 0) {
+ return RETURN_SUCCESS;
+}
+ }
+
CPU_REGISTER_TABLE_WRITE_FIELD (
ProcessorNumber,
Msr,
diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/FeatureControl.c
b/UefiCpuPkg/Library/CpuCommonFeaturesLib/FeatureControl.c
index d28c4ec51a..8c1eb5eb4f 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/FeatureControl.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/FeatureControl.c
@@ -96,6 +96,19 @@ VmxInitialize (
{
MSR_IA32_FEATURE_CONTROL_REGISTER*MsrRegister;
+ //
+ // The scope of EnableVmxOutsideSmx bit in the MSR_IA32_FEATURE_CONTROL is
core
[edk2] [Patch v2 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Add logic to support semaphore type.
Because this driver needs to set MSRs saved in normal boot phase, sync semaphore
logic from RegisterCpuFeaturesLib code which used for normal boot phase.
Detail see below change for RegisterCpuFeaturesLib:
UefiCpuPkg/RegisterCpuFeaturesLib: Add logic to support semaphore type.
Cc: Ruiyu Ni
Cc: Laszlo Ersek
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong
---
UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 406 +++--
UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 3 -
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 3 +-
3 files changed, 268 insertions(+), 144 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index 52ff9679d5..42a889f08f 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -38,9 +38,12 @@ typedef struct {
} MP_ASSEMBLY_ADDRESS_MAP;
//
-// Spin lock used to serialize MemoryMapped operation
+// Flags used when program the register.
//
-SPIN_LOCK*mMemoryMappedLock = NULL;
+typedef struct {
+ volatile UINTN MemoryMappedLock; // Spinlock used to program
mmio
+ volatile UINT32 *SemaphoreCount; // Semaphore used to program
semaphore.
+} PROGRAM_CPU_REGISTER_FLAGS;
//
// Signal that SMM BASE relocation is complete.
@@ -62,13 +65,11 @@ AsmGetAddressMap (
#define LEGACY_REGION_SIZE(2 * 0x1000)
#define LEGACY_REGION_BASE(0xA - LEGACY_REGION_SIZE)
+PROGRAM_CPU_REGISTER_FLAGS mCpuFlags;
ACPI_CPU_DATAmAcpiCpuData;
volatile UINT32 mNumberToFinish;
MP_CPU_EXCHANGE_INFO *mExchangeInfo;
BOOLEAN mRestoreSmmConfigurationInS3 = FALSE;
-MP_MSR_LOCK *mMsrSpinLocks = NULL;
-UINTNmMsrSpinLockCount;
-UINTNmMsrCount = 0;
//
// S3 boot flag
@@ -91,89 +92,6 @@ UINT8mApHltLoopCodeTemplate[] = {
0xEB, 0xFC // jmp $-2
};
-/**
- Get MSR spin lock by MSR index.
-
- @param MsrIndex MSR index value.
-
- @return Pointer to MSR spin lock.
-
-**/
-SPIN_LOCK *
-GetMsrSpinLockByIndex (
- IN UINT32 MsrIndex
- )
-{
- UINTN Index;
- for (Index = 0; Index < mMsrCount; Index++) {
-if (MsrIndex == mMsrSpinLocks[Index].MsrIndex) {
- return mMsrSpinLocks[Index].SpinLock;
-}
- }
- return NULL;
-}
-
-/**
- Initialize MSR spin lock by MSR index.
-
- @param MsrIndex MSR index value.
-
-**/
-VOID
-InitMsrSpinLockByIndex (
- IN UINT32 MsrIndex
- )
-{
- UINTNMsrSpinLockCount;
- UINTNNewMsrSpinLockCount;
- UINTNIndex;
- UINTNAddedSize;
-
- if (mMsrSpinLocks == NULL) {
-MsrSpinLockCount = mSmmCpuSemaphores.SemaphoreMsr.AvailableCounter;
-mMsrSpinLocks = (MP_MSR_LOCK *) AllocatePool (sizeof (MP_MSR_LOCK) *
MsrSpinLockCount);
-ASSERT (mMsrSpinLocks != NULL);
-for (Index = 0; Index < MsrSpinLockCount; Index++) {
- mMsrSpinLocks[Index].SpinLock =
- (SPIN_LOCK *)((UINTN)mSmmCpuSemaphores.SemaphoreMsr.Msr + Index *
mSemaphoreSize);
- mMsrSpinLocks[Index].MsrIndex = (UINT32)-1;
-}
-mMsrSpinLockCount = MsrSpinLockCount;
-mSmmCpuSemaphores.SemaphoreMsr.AvailableCounter = 0;
- }
- if (GetMsrSpinLockByIndex (MsrIndex) == NULL) {
-//
-// Initialize spin lock for MSR programming
-//
-mMsrSpinLocks[mMsrCount].MsrIndex = MsrIndex;
-InitializeSpinLock (mMsrSpinLocks[mMsrCount].SpinLock);
-mMsrCount ++;
-if (mMsrCount == mMsrSpinLockCount) {
- //
- // If MSR spin lock buffer is full, enlarge it
- //
- AddedSize = SIZE_4KB;
- mSmmCpuSemaphores.SemaphoreMsr.Msr =
-AllocatePages (EFI_SIZE_TO_PAGES(AddedSize));
- ASSERT (mSmmCpuSemaphores.SemaphoreMsr.Msr != NULL);
- NewMsrSpinLockCount = mMsrSpinLockCount + AddedSize / mSemaphoreSize;
- mMsrSpinLocks = ReallocatePool (
-sizeof (MP_MSR_LOCK) * mMsrSpinLockCount,
-sizeof (MP_MSR_LOCK) * NewMsrSpinLockCount,
-mMsrSpinLocks
-);
- ASSERT (mMsrSpinLocks != NULL);
- mMsrSpinLockCount = NewMsrSpinLockCount;
- for (Index = mMsrCount; Index < mMsrSpinLockCount; Index++) {
-mMsrSpinLocks[Index].SpinLock =
- (SPIN_LOCK *)((UINTN)mSmmCpuSemaphores.SemaphoreMsr.Msr +
- (Index - mMsrCount) * mSemaphoreSize);
-mMsrSpinLocks[Index].MsrIndex = (UINT32)-1;
- }
-}
- }
-}
-
/**
Sync up the MTRR values for all processors.
@@ -204,42 +122,131 @@ Returns:
}
/**
- Programs registers for the calling processor.
+ Increment semaphore by 1.
+
+ @param SemIN: 32-bit unsigned integer
+
+**/
+VOID
+S3ReleaseSemaphore (
+ IN OUT volatile UINT32 *Sem
+ )
+{
+ InterlockedIncrement
[edk2] [Patch v2 2/6] UefiCpuPkg/RegisterCpuFeaturesLib.h: Add new dependence types.
Add new core/package dependence types which consumed by different MSRs.
Cc: Ruiyu Ni
Cc: Laszlo Ersek
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong
---
.../Include/Library/RegisterCpuFeaturesLib.h | 25 ++
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
index 9331e49d13..e6f0ebe4bc 100644
--- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
+++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
@@ -73,10 +73,17 @@
#define CPU_FEATURE_PPIN(32+11)
#define CPU_FEATURE_PROC_TRACE (32+12)
-#define CPU_FEATURE_BEFORE_ALL BIT27
-#define CPU_FEATURE_AFTER_ALL BIT28
-#define CPU_FEATURE_BEFORE BIT29
-#define CPU_FEATURE_AFTER BIT30
+#define CPU_FEATURE_BEFORE_ALL BIT23
+#define CPU_FEATURE_AFTER_ALL BIT24
+#define CPU_FEATURE_BEFORE BIT25
+#define CPU_FEATURE_AFTER BIT26
+
+#define CPU_FEATURE_THREAD_BEFORE CPU_FEATURE_BEFORE
+#define CPU_FEATURE_THREAD_AFTERCPU_FEATURE_AFTER
+#define CPU_FEATURE_CORE_BEFORE BIT27
+#define CPU_FEATURE_CORE_AFTER BIT28
+#define CPU_FEATURE_PACKAGE_BEFORE BIT29
+#define CPU_FEATURE_PACKAGE_AFTER BIT30
#define CPU_FEATURE_END MAX_UINT32
/// @}
@@ -116,6 +123,16 @@ typedef struct {
CPUID_VERSION_INFO_EDX CpuIdVersionInfoEdx;
} REGISTER_CPU_FEATURE_INFORMATION;
+//
+// Describe the dependency type for different features.
+//
+typedef enum {
+ NoneDepType,
+ ThreadDepType,
+ CoreDepType,
+ PackageDepType
+} CPU_FEATURE_DEPENDENCE_TYPE;
+
/**
Determines if a CPU feature is enabled in PcdCpuFeaturesSupport bit mask.
If a CPU feature is disabled in PcdCpuFeaturesSupport then all the code/data
--
2.15.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch v2 0/6] Fix performance issue caused by Set MSR task.
V2 changes include:
1. Include the change for CpuCommonFeaturesLib which used to set MSR base on
its scope info.
2. Include the change for CpuS3DataDxe driver which also handle the AcpiCpuData
data.
3. Update code base on feedback for V1 changes.
V1 changes include:
In a system which has multiple cores, current set register value task costs
huge times.
After investigation, current set MSR task costs most of the times. Current
logic uses SpinLock to let set MSR task as an single thread task for all cores.
Because MSR has scope attribute which may cause GP fault if multiple APs set
MSR at the same time, current logic use an easiest solution (use SpinLock) to
avoid this issue, but it will cost huge times.
In order to fix this performance issue, new solution will set MSRs base on
their scope attribute. After this, the SpinLock will not needed. Without
SpinLock, new issue raised which is caused by MSR dependence. For example, MSR
A depends on MSR B which means MSR A must been set after MSR B has been set.
Also MSR B is package scope level and MSR A is thread scope level. If system
has multiple threads, Thread 1 needs to set the thread level MSRs and thread 2
needs to set thread and package level MSRs. Set MSRs task for thread 1 and
thread 2 like below:
Thread 1 Thread 2
MSR B NY
MSR A YY
If driver don't control execute MSR order, for thread 1, it will execute MSR A
first, but at this time, MSR B not been executed yet by thread 2. system may
trig exception at this time.
In order to fix the above issue, driver introduces semaphore logic to control
the MSR execute sequence. For the above case, a semaphore will be add between
MSR A and B for all threads. Semaphore has scope info for it. The possible
scope value is core or package.
For each thread, when it meets a semaphore during it set registers, it will 1)
release semaphore (+1) for each threads in this core or package(based on the
scope info for this
semaphore) 2) acquire semaphore (-1) for all the threads in this core or
package(based on the scope info for this semaphore). With these two steps,
driver can control MSR sequence. Sample code logic like below:
//
// First increase semaphore count by 1 for processors in this package.
//
for (ProcessorIndex = 0; ProcessorIndex < PackageThreadsCount ;
ProcessorIndex ++) {
LibReleaseSemaphore ((UINT32 *) [PackageOffset +
ProcessorIndex]);
}
//
// Second, check whether the count has reach the check number.
//
for (ProcessorIndex = 0; ProcessorIndex < ValidApCount; ProcessorIndex ++) {
LibWaitForSemaphore ([ApOffset]);
}
Platform Requirement:
1. This change requires register MSR setting base on MSR scope info. If still
register MSR
for all threads, exception may raised.
Known limitation:
1. Current CpuFeatures driver supports DXE instance and PEI instance. But
semaphore logic
requires Aps execute in async mode which is not supported by PEI driver. So
CpuFeature
PEI instance not works after this change. We plan to support async mode for
PEI in phase
2 for this task.
2. Current execute MSR task code in duplicated in PiSmmCpuDxeSmm driver and
RegisterCpuFeaturesLib library because the schedule limitation. Will merge
the code to
RegisterCpuFeaturesLib and export as an API in phase 2 for this task.
Cc: Ruiyu Ni
Cc: Laszlo Ersek
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong
Eric Dong (6):
UefiCpuPkg/Include/AcpiCpuData.h: Add Semaphore related Information.
UefiCpuPkg/RegisterCpuFeaturesLib.h: Add new dependence types.
UefiCpuPkg/RegisterCpuFeaturesLib: Add logic to support semaphore
type.
UefiCpuPkg/PiSmmCpuDxeSmm: Add logic to support semaphore type.
UefiCpuPkg/CpuS3DataDxe: Keep old data if value already existed.
UefiCpuPkg/CpuCommonFeaturesLib: Register MSR base on scope Info.
UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c| 2 +
UefiCpuPkg/Include/AcpiCpuData.h | 45 +-
.../Include/Library/RegisterCpuFeaturesLib.h | 25 +-
UefiCpuPkg/Library/CpuCommonFeaturesLib/C1e.c | 8 +
UefiCpuPkg/Library/CpuCommonFeaturesLib/Eist.c | 12 +
.../Library/CpuCommonFeaturesLib/ExecuteDisable.c | 10 +
.../Library/CpuCommonFeaturesLib/FastStrings.c | 12 +
.../Library/CpuCommonFeaturesLib/FeatureControl.c | 38 ++
.../CpuCommonFeaturesLib/LimitCpuIdMaxval.c| 14 +
.../Library/CpuCommonFeaturesLib/MachineCheck.c| 38 ++
.../Library/CpuCommonFeaturesLib/MonitorMwait.c| 15 +
.../Library/CpuCommonFeaturesLib/PendingBreak.c| 11 +
UefiCpuPkg/Library/CpuCommonFeaturesLib/Ppin.c | 11 +
.../Library/CpuCommonFeaturesLib/ProcTrace.c | 11 +
UefiCpuPkg/Library/CpuCommonFeaturesLib/X2Apic.c | 10 +
.../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 438 ---
.../DxeRegisterCpuFeaturesLib.c
[edk2] [Patch v2 1/6] UefiCpuPkg/Include/AcpiCpuData.h: Add Semaphore related Information.
v2 changes:
1. Add more description about why we do this change.
2. Change structure field type from pointer to EFI_PHYSICAL_ADDRESS because it
will
be share between PEI and DXE.
In order to support semaphore related logic, add new definition for it.
In a system which has multiple cores, current set register value task costs
huge times.
After investigation, current set MSR task costs most of the times. Current
logic uses
SpinLock to let set MSR task as an single thread task for all cores. Because
MSR has
scope attribute which may cause GP fault if multiple APs set MSR at the same
time,
current logic use an easiest solution (use SpinLock) to avoid this issue, but
it will
cost huge times.
In order to fix this performance issue, new solution will set MSRs base on
their scope
attribute. After this, the SpinLock will not needed. Without SpinLock, new
issue raised
which is caused by MSR dependence. For example, MSR A depends on MSR B which
means MSR A
must been set after MSR B has been set. Also MSR B is package scope level and
MSR A is
thread scope level. If system has multiple threads, Thread 1 needs to set the
thread level
MSRs and thread 2 needs to set thread and package level MSRs. Set MSRs task for
thread 1
and thread 2 like below:
Thread 1 Thread 2
MSR B NY
MSR A YY
If driver don't control execute MSR order, for thread 1, it will execute MSR A
first, but
at this time, MSR B not been executed yet by thread 2. system may trig
exception at this
time.
In order to fix the above issue, driver introduces semaphore logic to control
the MSR
execute sequence. For the above case, a semaphore will be add between MSR A and
B for
all threads. Semaphore has scope info for it. The possible scope value is core
or package.
For each thread, when it meets a semaphore during it set registers, it will 1)
release
semaphore (+1) for each threads in this core or package(based on the scope info
for this
semaphore) 2) acquire semaphore (-1) for all the threads in this core or
package(based
on the scope info for this semaphore). With these two steps, driver can control
MSR
sequence. Sample code logic like below:
//
// First increase semaphore count by 1 for processors in this package.
//
for (ProcessorIndex = 0; ProcessorIndex < PackageThreadsCount ;
ProcessorIndex ++) {
LibReleaseSemaphore ((UINT32 *) [PackageOffset +
ProcessorIndex]);
}
//
// Second, check whether the count has reach the check number.
//
for (ProcessorIndex = 0; ProcessorIndex < ValidApCount; ProcessorIndex ++) {
LibWaitForSemaphore ([ApOffset]);
}
Platform Requirement:
1. This change requires register MSR setting base on MSR scope info. If still
register MSR
for all threads, exception may raised.
Known limitation:
1. Current CpuFeatures driver supports DXE instance and PEI instance. But
semaphore logic
requires Aps execute in async mode which is not supported by PEI driver. So
CpuFeature
PEI instance not works after this change. We plan to support async mode for
PEI in phase
2 for this task.
Cc: Ruiyu Ni
Cc: Laszlo Ersek
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong
---
UefiCpuPkg/Include/AcpiCpuData.h | 45 +++-
1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/UefiCpuPkg/Include/AcpiCpuData.h b/UefiCpuPkg/Include/AcpiCpuData.h
index 9e51145c08..f1439dcf9a 100644
--- a/UefiCpuPkg/Include/AcpiCpuData.h
+++ b/UefiCpuPkg/Include/AcpiCpuData.h
@@ -22,9 +22,42 @@ typedef enum {
Msr,
ControlRegister,
MemoryMapped,
- CacheControl
+ CacheControl,
+ //
+ // Semaphore type used to control the execute sequence of the Msr.
+ // It will be insert between two Msr which has execute dependence.
+ //
+ Semaphore
} REGISTER_TYPE;
+//
+// CPU information.
+//
+typedef struct {
+ //
+ // Record the package count in this CPU.
+ //
+ UINT32 PackageCount;
+ //
+ // Record the max core count in this CPU.
+ // Different packages may have different core count, this value
+ // save the max core count in all the packages.
+ //
+ UINT32 MaxCoreCount;
+ //
+ // Record the max thread count in this CPU.
+ // Different cores may have different thread count, this value
+ // save the max thread count in all the cores.
+ //
+ UINT32 MaxThreadCount;
+ //
+ // This fild is an pointer type which point to an array.
+ // This array used to save the valid cores in different packages in this CPU.
+ // The array count is the package number in this CPU.
+ //
+ EFI_PHYSICAL_ADDRESSValidCoresPerPackages;
+} CPU_STATUS_INFORMATION;
+
//
// Element of register table entry
//
@@ -147,6 +180,16 @@ typedef struct {
// provided.
//
UINT32ApMachineCheckHandlerSize;
+ //
+ // CPU information which is required when set
[edk2] [Patch] BaseTools: Fix a bug --pcd option enable and use the pcd in expression
the case is: in the DSC: [PcdsFixedAtBuild.common] TokenSpaceGuid.TestFixedPcd|0xFFEAA000 [PcdsDynamicExDefault.common.DEFAULT] !if TokenSpaceGuid.PcdFlag == TRUE TokenSpaceGuid.PcdTest|TokenSpaceGuid.TestFixedPcd !endif Then build with --pcd TokenSpaceGuid.PcdFlag=TRUE, it report failure, but if we build without this --pcd option, it could build success. we found when the --pcd is enabled, the fixedatbuild pcds are not be collected into expression to calculate. Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=1256 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Yonghong Zhu --- BaseTools/Source/Python/Workspace/DscBuildData.py | 1 + 1 file changed, 1 insertion(+) diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py b/BaseTools/Source/Python/Workspace/DscBuildData.py index f41038e..ace348b 100644 --- a/BaseTools/Source/Python/Workspace/DscBuildData.py +++ b/BaseTools/Source/Python/Workspace/DscBuildData.py @@ -2942,6 +2942,7 @@ class DscBuildData(PlatformBuildClassObject): if ModuleFile in self._Modules: continue ModuleData = self._Bdb[ModuleFile, self._Arch, self._Target, self._Toolchain] PkgSet.update(ModuleData.Packages) self._DecPcds, self._GuidDict = GetDeclaredPcd(self, self._Bdb, self._Arch, self._Target, self._Toolchain, PkgSet) +self._GuidDict.update(GlobalData.gPlatformPcds) return self._DecPcds -- 2.6.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 4/4] UefiCpuPkg/PiSmmCpuDxeSmm: Add logic to support semaphore type.
Hi Ruiyu,
> -Original Message-
> From: Ni, Ruiyu
> Sent: Tuesday, October 16, 2018 11:16 AM
> To: Dong, Eric ; edk2-devel@lists.01.org
> Cc: Laszlo Ersek
> Subject: Re: [Patch 4/4] UefiCpuPkg/PiSmmCpuDxeSmm: Add logic to
> support semaphore type.
>
> On 10/15/2018 10:49 AM, Eric Dong wrote:
> > Because this driver needs to set MSRs saved in normal boot phase, sync
> > semaphore logic from RegisterCpuFeaturesLib code which used for normal
> boot phase.
> >
> > Detail see change SHA-1: dcdf1774212d87e2d7feb36286a408ea7475fd7b for
> > RegisterCpuFeaturesLib.
> >
> > Cc: Ruiyu Ni
> > Cc: Laszlo Ersek
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Dong
> > ---
> > UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 316 -
>
> > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 3 -
> > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 3 +-
> > 3 files changed, 180 insertions(+), 142 deletions(-)
> >
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> > index 52ff9679d5..5a35f7a634 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> > @@ -38,9 +38,12 @@ typedef struct {
> > } MP_ASSEMBLY_ADDRESS_MAP;
> >
> > //
> > -// Spin lock used to serialize MemoryMapped operation
> > +// Flags used when program the register.
> > //
> > -SPIN_LOCK*mMemoryMappedLock = NULL;
> > +typedef struct {
> > + volatile UINTN MemoryMappedLock; // Spinlock used to
> > program
> mmio
> > + volatile UINT32 *SemaphoreCount; // Semaphore used to
> program semaphore.
> > +} PROGRAM_CPU_REGISTER_FLAGS;
> >
> > //
> > // Signal that SMM BASE relocation is complete.
> > @@ -62,13 +65,11 @@ AsmGetAddressMap (
> > #define LEGACY_REGION_SIZE(2 * 0x1000)
> > #define LEGACY_REGION_BASE(0xA - LEGACY_REGION_SIZE)
> >
> > +PROGRAM_CPU_REGISTER_FLAGS mCpuFlags;
> > ACPI_CPU_DATAmAcpiCpuData;
> > volatile UINT32 mNumberToFinish;
> > MP_CPU_EXCHANGE_INFO *mExchangeInfo;
> > BOOLEAN mRestoreSmmConfigurationInS3 = FALSE;
> > -MP_MSR_LOCK *mMsrSpinLocks = NULL;
> > -UINTNmMsrSpinLockCount;
> > -UINTNmMsrCount = 0;
> >
> > //
> > // S3 boot flag
> > @@ -91,89 +92,6 @@ UINT8mApHltLoopCodeTemplate[] = {
> > 0xEB, 0xFC // jmp $-2
> > };
> >
> > -/**
> > - Get MSR spin lock by MSR index.
> > -
> > - @param MsrIndex MSR index value.
> > -
> > - @return Pointer to MSR spin lock.
> > -
> > -**/
> > -SPIN_LOCK *
> > -GetMsrSpinLockByIndex (
> > - IN UINT32 MsrIndex
> > - )
> > -{
> > - UINTN Index;
> > - for (Index = 0; Index < mMsrCount; Index++) {
> > -if (MsrIndex == mMsrSpinLocks[Index].MsrIndex) {
> > - return mMsrSpinLocks[Index].SpinLock;
> > -}
> > - }
> > - return NULL;
> > -}
> > -
> > -/**
> > - Initialize MSR spin lock by MSR index.
> > -
> > - @param MsrIndex MSR index value.
> > -
> > -**/
> > -VOID
> > -InitMsrSpinLockByIndex (
> > - IN UINT32 MsrIndex
> > - )
> > -{
> > - UINTNMsrSpinLockCount;
> > - UINTNNewMsrSpinLockCount;
> > - UINTNIndex;
> > - UINTNAddedSize;
> > -
> > - if (mMsrSpinLocks == NULL) {
> > -MsrSpinLockCount =
> mSmmCpuSemaphores.SemaphoreMsr.AvailableCounter;
> > -mMsrSpinLocks = (MP_MSR_LOCK *) AllocatePool (sizeof
> (MP_MSR_LOCK) * MsrSpinLockCount);
> > -ASSERT (mMsrSpinLocks != NULL);
> > -for (Index = 0; Index < MsrSpinLockCount; Index++) {
> > - mMsrSpinLocks[Index].SpinLock =
> > - (SPIN_LOCK *)((UINTN)mSmmCpuSemaphores.SemaphoreMsr.Msr +
> Index * mSemaphoreSize);
> > - mMsrSpinLocks[Index].MsrIndex = (UINT32)-1;
> > -}
> > -mMsrSpinLockCount = MsrSpinLockCount;
> > -mSmmCpuSemaphores.SemaphoreMsr.AvailableCounter = 0;
> > - }
> > - if (GetMsrSpinLockByIndex (MsrIndex) == NULL) {
> > -//
> > -// Initialize spin lock for MSR programming
> > -//
> > -mMsrSpinLocks[mMsrCount].MsrIndex = MsrIndex;
> > -InitializeSpinLock (mMsrSpinLocks[mMsrCount].SpinLock);
> > -mMsrCount ++;
> > -if (mMsrCount == mMsrSpinLockCount) {
> > - //
> > - // If MSR spin lock buffer is full, enlarge it
> > - //
> > - AddedSize = SIZE_4KB;
> > - mSmmCpuSemaphores.SemaphoreMsr.Msr =
> > -AllocatePages (EFI_SIZE_TO_PAGES(AddedSize));
> > - ASSERT (mSmmCpuSemaphores.SemaphoreMsr.Msr != NULL);
> > - NewMsrSpinLockCount = mMsrSpinLockCount + AddedSize /
> mSemaphoreSize;
> > - mMsrSpinLocks = ReallocatePool (
> > -sizeof (MP_MSR_LOCK) * mMsrSpinLockCount,
> > -sizeof (MP_MSR_LOCK) * NewMsrSpinLockCount,
> > -
Re: [edk2] TianoCore Community Meeting Minutes (stephano)
Thank you for the feedback. Comments below. On 10/15/2018 4:58 PM, Jeremiah Cox wrote: Some additional feedback... General --- We would like to have a discussion around goals, what are the top issues we are trying to solve with these solutions? We believe a primary challenge is getting code integrated downstream. We would like to see security patches flow to more systems, faster, with higher confidence. The same applies to new UEFI features. Part of making downstream integration efficient is getting downstream changes upstreamed, thus we support efforts to improve upstream contribution efficiency & quality. This is high on our list of concerns, along with generally making life easier for current developers. We would like to address ease of use for newcomers, but I've made it clear in calls that making our current community's workflow more streamlined and efficient takes priority. Patch Workflow Improvement -- We would like to propose adding Azure DevOps (previously Visual Studio Online) to the list. Azure DevOps is free for OSS and more feature rich than GitHub: https://azure.microsoft.com/en-us/pricing/details/devops/azure-devops-services/ Public CI - With respect to using Azure DevOps for CI, we have an example of this working here: https://dev.azure.com/projectmu/mu/_build?definitionId=4 I've added Azure DevOps to the list of items to be discussed in our CI email, along with Cirrus and (I'm sure) Jenkins/Travis. Andrew started that and I'll be pulling it out into its own thread shortly. Repos & Submodules -- In an upcoming meeting, we would like to discuss code layout and repositories. We propose to introduce layering and separation for a variety of reasons, best articulated by visiting the following: https://microsoft.github.io/mu/ I'd like to specifically call out Project Mu on our next call. That will be happening next week, after the Plugfest in Taipei. Kind regards, Jeremiah Cox ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch] BaseTools: Remove the step to freeze python tool
https://bugzilla.tianocore.org/show_bug.cgi?id=1257 Binary python tool is not supported anymore. So, the freeze python tool step is not required. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Liming Gao Cc: Yonghong Zhu --- BaseTools/BuildNotes.txt | 3 +- BaseTools/Makefile| 4 +- BaseTools/ReadMe.txt | 1 - BaseTools/Scripts/ShowEnvironment.bat | 1 - BaseTools/Source/Python/Makefile | 315 -- BaseTools/Source/Python/UPT/Makefile | 41 - BaseTools/toolsetup.bat | 36 +--- 7 files changed, 4 insertions(+), 397 deletions(-) delete mode 100644 BaseTools/Source/Python/UPT/Makefile diff --git a/BaseTools/BuildNotes.txt b/BaseTools/BuildNotes.txt index 0d77df0..e2b10fd 100644 --- a/BaseTools/BuildNotes.txt +++ b/BaseTools/BuildNotes.txt @@ -13,8 +13,7 @@ Quick Start --- Windows: - a) Set the PYTHON_FREEZER_PATH to the cx_Freeze installation directory - b) Go to the /BaseTools and run "toolsetup" script + a) Go to the /BaseTools and run "toolsetup" script Unix-like: a) make -C /BaseTools diff --git a/BaseTools/Makefile b/BaseTools/Makefile index b98cd85..e6932c7 100644 --- a/BaseTools/Makefile +++ b/BaseTools/Makefile @@ -17,13 +17,11 @@ SUBDIRS = $(BASE_TOOLS_PATH)\Source\C $(BASE_TOOLS_PATH)\Source\Python -all: c python +all: c c : @$(PYTHON_HOME)\python.exe $(BASE_TOOLS_PATH)\Source\C\Makefiles\NmakeSubdirs.py all $(BASE_TOOLS_PATH)\Source\C -python: - @$(PYTHON_HOME)\python.exe $(BASE_TOOLS_PATH)\Source\C\Makefiles\NmakeSubdirs.py all $(BASE_TOOLS_PATH)\Source\Python subdirs: $(SUBDIRS) @$(PYTHON_HOME)\python.exe $(BASE_TOOLS_PATH)\Source\C\Makefiles\NmakeSubdirs.py all $** diff --git a/BaseTools/ReadMe.txt b/BaseTools/ReadMe.txt index db632f7..7d0486b 100644 --- a/BaseTools/ReadMe.txt +++ b/BaseTools/ReadMe.txt @@ -16,7 +16,6 @@ In addition to this, you should set the following environment variables: * EDK_TOOLS_PATH - Path to the BaseTools sub directory under the edk2 tree * BASE_TOOLS_PATH - The directory where the BaseTools source is located. (It is the same directory where this README.txt is located.) - * PYTHON_FREEZER_PATH - Path to where the python freezer tool is installed After this, you can run the toolsetup.bat file, which is in the same directory as this file. It should setup the remainder of the environment, diff --git a/BaseTools/Scripts/ShowEnvironment.bat b/BaseTools/Scripts/ShowEnvironment.bat index 5dd30b4..759a74d 100755 --- a/BaseTools/Scripts/ShowEnvironment.bat +++ b/BaseTools/Scripts/ShowEnvironment.bat @@ -52,7 +52,6 @@ if defined SRC_CONF @goto SetEnv @if not defined EDK_TOOLS_PATH @echo EDK_TOOLS_PATH = Not Set @if defined BASE_TOOLS_PATH @echo BASE_TOOLS_PATH = %BASE_TOOLS_PATH% @if defined EDK_TOOLS_BIN @echo EDK_TOOLS_BIN= %EDK_TOOLS_BIN% -@if defined PYTHON_FREEZER_PATH @echo PYTHON_FREEZER_PATH = %PYTHON_FREEZER_PATH% @if "%NT32PKG%"=="TRUE" ( @echo. @echo NOTE: Please configure your build to use the following TOOL_CHAIN_TAG diff --git a/BaseTools/Source/Python/Makefile b/BaseTools/Source/Python/Makefile index ac99259..b413d23 100644 --- a/BaseTools/Source/Python/Makefile +++ b/BaseTools/Source/Python/Makefile @@ -11,324 +11,9 @@ # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. # -!IFNDEF PYTHON_HOME -!ERROR PYTHON_HOME must be defined! -!ENDIF - -!IFDEF PYTHON_FREEZER_PATH -!IF EXIST ($(PYTHON_FREEZER_PATH)\cxfreeze) -# Using cx_Freeze 4.2.3 with Python 2.7.2 -FREEZE=$(PYTHON_HOME)\python $(PYTHON_FREEZER_PATH)\cxfreeze -!ELSE -!ERROR PYTHON_FREEZER_PATH does not exist! -!ENDIF -!ENDIF - -MODULES=encodings.cp437,encodings.gbk,encodings.utf_16,encodings.utf_8,encodings.utf_16_le,encodings.latin_1,encodings.ascii - -# DOS del command doesn't support ":\\" in the file path, such as j:\\BaseTools. Convert ":\\" to ":\" -BASE_TOOLS_PATH = $(BASE_TOOLS_PATH::\\=:\) -EDK_TOOLS_PATH = $(EDK_TOOLS_PATH::\\=:\) - -BIN_DIR=$(EDK_TOOLS_PATH)\Bin\Win32 - -APPLICATIONS=$(BIN_DIR)\build.exe $(BIN_DIR)\GenFds.exe $(BIN_DIR)\Trim.exe $(BIN_DIR)\TargetTool.exe $(BIN_DIR)\GenDepex.exe $(BIN_DIR)\GenPatchPcdTable.exe $(BIN_DIR)\PatchPcdValue.exe $(BIN_DIR)\BPDG.exe $(BIN_DIR)\UPT.exe $(BIN_DIR)\Rsa2048Sha256Sign.exe $(BIN_DIR)\Rsa2048Sha256GenerateKeys.exe $(BIN_DIR)\Pkcs7Sign.exe $(BIN_DIR)\Ecc.exe - -COMMON_PYTHON=$(BASE_TOOLS_PATH)\Source\Python\Common\BuildToolError.py \ - $(BASE_TOOLS_PATH)\Source\Python\Common\Database.py \ - $(BASE_TOOLS_PATH)\Source\Python\Common\DataType.py \ - $(BASE_TOOLS_PATH)\Source\Python\Common\EdkLogger.py \ - $(BASE_TOOLS_PATH)\Source\Python\Common\Expression.py \ - $(BASE_TOOLS_PATH)\Source\Python\Common\GlobalData.py \ - $(BASE_TOOLS_PATH)\Source\Python\Common\Identification.py \ -
Re: [edk2] [Patch 4/4] UefiCpuPkg/PiSmmCpuDxeSmm: Add logic to support semaphore type.
Hi Laszlo,
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Tuesday, October 16, 2018 1:13 AM
> To: Dong, Eric ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu
> Subject: Re: [Patch 4/4] UefiCpuPkg/PiSmmCpuDxeSmm: Add logic to support
> semaphore type.
>
> On 10/15/18 04:49, Eric Dong wrote:
> > Because this driver needs to set MSRs saved in normal boot phase, sync
> > semaphore logic from RegisterCpuFeaturesLib code which used for normal
> boot phase.
>
> (My review of this patch is going to be superficial. I'm not trying to
> validate the
> actual algorithm. I'm mostly sanity-checking the code, and gauging whether it
> will break platforms that use CpuS3DataDxe.)
>
Reasonable, thanks for your efforts.
>
> > Detail see change SHA-1: dcdf1774212d87e2d7feb36286a408ea7475fd7b for
> > RegisterCpuFeaturesLib.
>
> (1) I think it is valid to reference other patches in the same series.
> However, the commit hashes are not stable yet -- when you rebase the series,
> the commit hashes will change. Therefore, when we refer to a patch that is not
> upstream yet (i.e. it is part of the same series), it is best to spell out
> the full
> subject, such as:
>
> UefiCpuPkg/RegisterCpuFeaturesLib: Add logic to support semaphore type.
>
I aware this value change when do the rebase action. I plan to update the value
when I do the rebase action. Your suggestion is good. I can also use the
change header to specify the change. I will use it in my next change.
>
> >
> > Cc: Ruiyu Ni
> > Cc: Laszlo Ersek
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Dong
> > ---
> > UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 316 ---
> --
> > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 3 -
> > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 3 +-
> > 3 files changed, 180 insertions(+), 142 deletions(-)
> >
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> > index 52ff9679d5..5a35f7a634 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> > @@ -38,9 +38,12 @@ typedef struct {
> > } MP_ASSEMBLY_ADDRESS_MAP;
> >
> > //
> > -// Spin lock used to serialize MemoryMapped operation
> > +// Flags used when program the register.
> > //
> > -SPIN_LOCK*mMemoryMappedLock = NULL;
> > +typedef struct {
> > + volatile UINTN MemoryMappedLock; // Spinlock used to
> > program
> mmio
> > + volatile UINT32 *SemaphoreCount; // Semaphore used to
> > program
> semaphore.
> > +} PROGRAM_CPU_REGISTER_FLAGS;
> >
> > //
> > // Signal that SMM BASE relocation is complete.
> > @@ -62,13 +65,11 @@ AsmGetAddressMap (
> > #define LEGACY_REGION_SIZE(2 * 0x1000)
> > #define LEGACY_REGION_BASE(0xA - LEGACY_REGION_SIZE)
> >
> > +PROGRAM_CPU_REGISTER_FLAGS mCpuFlags;
> > ACPI_CPU_DATAmAcpiCpuData;
> > volatile UINT32 mNumberToFinish;
> > MP_CPU_EXCHANGE_INFO *mExchangeInfo;
> > BOOLEAN mRestoreSmmConfigurationInS3 = FALSE;
> > -MP_MSR_LOCK *mMsrSpinLocks = NULL;
> > -UINTNmMsrSpinLockCount;
> > -UINTNmMsrCount = 0;
> >
> > //
> > // S3 boot flag
> > @@ -91,89 +92,6 @@ UINT8mApHltLoopCodeTemplate[] = {
> > 0xEB, 0xFC // jmp $-2
> > };
> >
> > -/**
> > - Get MSR spin lock by MSR index.
> > -
> > - @param MsrIndex MSR index value.
> > -
> > - @return Pointer to MSR spin lock.
> > -
> > -**/
> > -SPIN_LOCK *
> > -GetMsrSpinLockByIndex (
> > - IN UINT32 MsrIndex
> > - )
> > -{
> > - UINTN Index;
> > - for (Index = 0; Index < mMsrCount; Index++) {
> > -if (MsrIndex == mMsrSpinLocks[Index].MsrIndex) {
> > - return mMsrSpinLocks[Index].SpinLock;
> > -}
> > - }
> > - return NULL;
> > -}
> > -
> > -/**
> > - Initialize MSR spin lock by MSR index.
> > -
> > - @param MsrIndex MSR index value.
> > -
> > -**/
> > -VOID
> > -InitMsrSpinLockByIndex (
> > - IN UINT32 MsrIndex
> > - )
> > -{
> > - UINTNMsrSpinLockCount;
> > - UINTNNewMsrSpinLockCount;
> > - UINTNIndex;
> > - UINTNAddedSize;
> > -
> > - if (mMsrSpinLocks == NULL) {
> > -MsrSpinLockCount =
> mSmmCpuSemaphores.SemaphoreMsr.AvailableCounter;
> > -mMsrSpinLocks = (MP_MSR_LOCK *) AllocatePool (sizeof (MP_MSR_LOCK)
> * MsrSpinLockCount);
> > -ASSERT (mMsrSpinLocks != NULL);
> > -for (Index = 0; Index < MsrSpinLockCount; Index++) {
> > - mMsrSpinLocks[Index].SpinLock =
> > - (SPIN_LOCK *)((UINTN)mSmmCpuSemaphores.SemaphoreMsr.Msr +
> Index * mSemaphoreSize);
> > - mMsrSpinLocks[Index].MsrIndex = (UINT32)-1;
> > -}
> > -mMsrSpinLockCount = MsrSpinLockCount;
> > -mSmmCpuSemaphores.SemaphoreMsr.AvailableCounter = 0;
> > - }
> > -
Re: [edk2] Where to find the fix for security issue id 686
Thanks a lot Liming! Em seg, 15 de out de 2018 às 23:10, Gao, Liming escreveu: > Rafael: > https://bugzilla.tianocore.org/show_bug.cgi?id=686 public now. You can > view it. I also send the patches to fix it. Please check. > > Thanks > Liming > >-Original Message- > >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > >Rafael Machado > >Sent: Tuesday, October 16, 2018 8:41 AM > >To: Zimmer, Vincent > >Cc: edk2-devel@lists.01.org > >Subject: Re: [edk2] Where to find the fix for security issue id 686 > > > >I understood this issue's fix was already released at some branch. > >With your message things make sense again. > > > >In this case I can wait for this fix to be publicly available. > >Thanks for the clarification! > > > >Best Regards > >Rafael > > > >Em seg, 15 de out de 2018 às 16:42, Zimmer, Vincent < > >vincent.zim...@intel.com> escreveu: > > > >> Ah ok > >> > >> > >> > >> From > >> > https://github.com/tianocore/tianocore.github.io/wiki/Reporting-Security- > >Issues > >> you will see that issues are only visible to the report and infosec > group > >> of Bugzilla, namely “Issues in the *Tianocore Security Issue* product > are > >> only visible to the *Reporter* of the issue and the members of the > >> *infosec* group. ” > >> > >> > >> > >> Since you were not the reporter of 686 and are not part of infosec, you > >> cannot see it. > >> > >> > >> > >> If you or anyone in the community would like to help work these issues > >> while in triage and embargo, let me know and we can add you to the > infosec > >> group. > >> > >> > >> > >> Vincent > >> > >> > >> > >> *From:* Rafael Machado [mailto:rafaelrodrigues.mach...@gmail.com] > >> *Sent:* Monday, October 15, 2018 12:17 PM > >> *To:* Zimmer, Vincent > >> *Cc:* edk2-devel@lists.01.org > >> *Subject:* Re: [edk2] Where to find the fix for security issue id 686 > >> > >> > >> > >> Hi Vincent > >> > >> > >> > >> Thanks for the answer. > >> > >> The problem is that when I try to access this link I have this message: > "You > >> are not authorized to access bug #686." > >> > >> > >> > >> Any idea? > >> > >> > >> > >> Em seg, 15 de out de 2018 às 14:28, Zimmer, Vincent < > >> vincent.zim...@intel.com> escreveu: > >> > >> You can find reference to patches via the advisory entry > >> > >> "31. EDK II TIANOCOMPRESS BOUNDS CHECKING ISSUES" advisory entry > >> https://edk2-docs.gitbooks.io/security-advisory/content/edk-ii- > >tianocompress-bounds-checking-issues.html > >> has an embedded link to > >> https://bugzilla.tianocore.org/attachment.cgi?id=150 > >> > >> Vincent > >> > >> -Original Message- > >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > >> Rafael Machado > >> Sent: Monday, October 15, 2018 5:39 AM > >> To: edk2-devel@lists.01.org > >> Subject: [edk2] Where to find the fix for security issue id 686 > >> > >> Hi everyone > >> > >> I was tying to find the patch to fix the reported security issue id 686 > ( > >> https://bugzilla.tianocore.org/show_bug.cgi?id=686), > >> but was not able to access it. > >> > >> Could someone please tell if this patch, or series of patches, was > already > >> merged to some branch that is public available? > >> > >> Thanks and Regards > >> Rafael R. Machado > >> ___ > >> edk2-devel mailing list > >> edk2-devel@lists.01.org > >> https://lists.01.org/mailman/listinfo/edk2-devel > >> > >> > >___ > >edk2-devel mailing list > >edk2-devel@lists.01.org > >https://lists.01.org/mailman/listinfo/edk2-devel > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] uefi-sct/SctPkg:The original design for the 'EraseLengthGranularity' test need consider this case
FYI
On 10/15/2018 03:08 AM, Supreeth Venkatesh wrote:
Commit Message less than 80 cols please.
On 10/13/2018 04:51 PM, Eric Jin wrote:
For the SD device, no same as the eMMC, the 'EraseLengthGranularity' is 1.
Cc: Supreeth Venkatesh
Cc: Jiaxin Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin
---
.../BlackBoxTest/EraseBlockBBTestFunction.c | 154 +-
1 file changed, 78 insertions(+), 76 deletions(-)
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestFunction.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestFunction.c
index bc16a473..ea081625 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestFunction.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestFunction.c
@@ -214,54 +214,56 @@ BBTestEraseBlocksFunctionTest (
}
// Erase Blocks with non-EraseLengthGranularity blocks
- Token.Event = NULL;
- Token.TransactionStatus = EFI_NOT_READY;
- EraseStatus = EraseBlock->EraseBlocks (EraseBlock, MediaId, Lba+1,
, BufferSize - 2 * BlockSize);
-
- // Read the data with 0, the first/last block should not be erased
- ReadStatus = BlockIo->ReadBlocks (BlockIo, MediaId, Lba, BufferSize,
(VOID*)Buffer2);
- if (ReadStatus == EFI_SUCCESS) {
-for (Index1 = 0; Index1 < BlockSize; Index1++) {
- if (Buffer2[Index1] != 0) {
-IsZero1 = FALSE;
-break;
+ if (BufferSize > 2 * BlockSize) {
Magic Number 2.
+Token.Event = NULL;
+Token.TransactionStatus = EFI_NOT_READY;
+
+EraseStatus = EraseBlock->EraseBlocks (EraseBlock, MediaId, Lba+1,
, BufferSize - 2 * BlockSize);
Magic number 2.
+
+// Read the data with 0, the first/last block should not be erased
+ReadStatus = BlockIo->ReadBlocks (BlockIo, MediaId, Lba, BufferSize,
(VOID*)Buffer2);
+if (ReadStatus == EFI_SUCCESS) {
+ for (Index1 = 0; Index1 < BlockSize; Index1++) {
+if (Buffer2[Index1] != 0) {
+ IsZero1 = FALSE;
+ break;
+}
}
-}
-for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
- if (Buffer2[Index1] != 0) {
-IsZero2 = FALSE;
-break;
+ for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
+if (Buffer2[Index1] != 0) {
+ IsZero2 = FALSE;
+ break;
+}
}
-}
-for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) {
- if (Buffer2[Index1] != 0) {
-IsZero3 = FALSE;
-break;
+ for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++)
{
+if (Buffer2[Index1] != 0) {
+ IsZero3 = FALSE;
+ break;
+}
}
-}
-if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 == TRUE) && ((IsZero3 == FALSE)))
- AssertionType = EFI_TEST_ASSERTION_PASSED;
-else
- AssertionType = EFI_TEST_ASSERTION_FAILED;
+ if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 == TRUE)
&& ((IsZero3 == FALSE)))
Less than 80 cols.
+ AssertionType = EFI_TEST_ASSERTION_PASSED;
+ else
+AssertionType = EFI_TEST_ASSERTION_FAILED;
-StandardLib->RecordAssertion (
- StandardLib,
- AssertionType,
- gEraseBlockBBTestFunctionAssertionGuid003,
- L"EraseBlocks - EraseBlocks for testing, the first/last
block should not be erased",
- L"%a:%d:EraseBlocks Status - %r, IsZero1 - %d, IsZero2 - %d,
IsZero3 - %d",
- __FILE__,
- (UINTN)__LINE__,
- Status,
- IsZero1, IsZero2, IsZero3
- );
+ StandardLib->RecordAssertion (
+ StandardLib,
+ AssertionType,
+ gEraseBlockBBTestFunctionAssertionGuid003,
+ L"EraseBlocks - EraseBlocks for testing, the first/last
block should not be erased",
+ L"%a:%d:EraseBlocks Status - %r, IsZero1 - %d, IsZero2 -
%d, IsZero3 - %d",
+ __FILE__,
+ (UINTN)__LINE__,
+ EraseStatus,
+ IsZero1, IsZero2, IsZero3
+ );
+}
}
-
//
// Erase Blocks with the EraseLengthGranularity blocks
//
@@ -453,13 +455,13 @@ BlockIo2:
//
// Erase Blocks with non EraseLengthGranularity blocks
//
+ if (BufferSize > 2 *
Re: [edk2] [PATCH] uefi-sct/SctPkg:Fix the incorrect buffer free in SctAPrint()
Reviewed-by: Supreeth Venkatesh
On 10/14/2018 02:58 AM, Eric Jin wrote:
Cc: Supreeth Venkatesh
Cc: Jiaxin Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin
---
uefi-sct/SctPkg/Library/SctLib/Print.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/uefi-sct/SctPkg/Library/SctLib/Print.c
b/uefi-sct/SctPkg/Library/SctLib/Print.c
index c25aff11..e523073a 100644
--- a/uefi-sct/SctPkg/Library/SctLib/Print.c
+++ b/uefi-sct/SctPkg/Library/SctLib/Print.c
@@ -2,7 +2,7 @@
Copyright 2006 - 2015 Unified EFI, Inc.
Copyright (c) 2013 - 2014, ARM Ltd. All rights reserved.
- Copyright (c) 2014 - 2015, Intel Corporation. All rights reserved.
+ Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -676,7 +676,9 @@ _IPrint (
}
ret = _Print ();
- SctFreePool(ps.fmt.u.pw);
+ if (fmt) {
+SctFreePool(ps.fmt.u.pw);
+ }
return ret;
}
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] uefi-sct/SctPkg:Enhance the EraseBlock Test
On 10/14/2018 05:26 PM, Eric Jin wrote:
The EraseSize in the EraseBlocks conf test should be bytes.
Cover the case that the size of the data to erase is a
multiple of the 'EraseLengthGranularity' value of an Erase Block
Protocol instance. And check whether the data on adjacent blocks
are mistakenly erased.
Cc: Supreeth Venkatesh
Cc: Jiaxin Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin
---
.../EraseBlockBBTestConformance.c | 25 +-
.../BlackBoxTest/EraseBlockBBTestFunction.c | 600 --
.../BlackBoxTest/EraseBlockBBTestMain.h | 16 +-
.../Protocol/EraseBlock/BlackBoxTest/Guid.c | 4 +-
.../Protocol/EraseBlock/BlackBoxTest/Guid.h | 7 +
5 files changed, 589 insertions(+), 63 deletions(-)
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestConformance.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestConformance.c
index df057b26..7e848239 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestConformance.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestConformance.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2017 Unified EFI, Inc.
- Copyright (c) 2017, Intel Corporation. All rights reserved.
+ Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -51,8 +51,8 @@ BBTestEraseBlocksConformanceTest (
UINT32BlockSize;
UINT32IoAlign;
EFI_LBA LastBlock;
-
- UINT32BlockNumber;
+ UINT32EraseLengthGranularity;
+ UINTN EraseSize;
EFI_ERASE_BLOCK_TOKEN Token;
@@ -121,10 +121,11 @@ BBTestEraseBlocksConformanceTest (
IoAlign = Media->IoAlign;
LastBlock = Media->LastBlock;
- BlockNumber = (UINT32) MINIMUM(LastBlock, MAX_NUMBER_OF_READ_BLOCK_BUFFER);
+ EraseLengthGranularity = EraseBlock->EraseLengthGranularity;
+ EraseSize = (UINTN)SctMultU64x32 (EraseLengthGranularity,
BlockSize);
if (MediaPresent == FALSE) {
-Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, 0, ,
BlockNumber);
+Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, 0, ,
EraseSize);
if (Status == EFI_NO_MEDIA)
AssertionType = EFI_TEST_ASSERTION_PASSED;
else
@@ -141,7 +142,7 @@ BBTestEraseBlocksConformanceTest (
Status
);
-Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, LastBlock + 1, , BlockNumber);
+Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, LastBlock + 1,
, EraseSize);
if (Status == EFI_NO_MEDIA)
AssertionType = EFI_TEST_ASSERTION_PASSED;
else
@@ -158,7 +159,7 @@ BBTestEraseBlocksConformanceTest (
Status
);
-Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, LastBlock - 10, , BlockNumber + 1);
+Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, LastBlock - 10,
, EraseSize + 1);
Why -10? Magic Number.
if (Status == EFI_NO_MEDIA)
AssertionType = EFI_TEST_ASSERTION_PASSED;
else
@@ -177,7 +178,7 @@ BBTestEraseBlocksConformanceTest (
} else {
if (ReadOnly == TRUE) {
- Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, 0, ,
BlockNumber);
+ Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, 0, ,
EraseSize);
if (Status == EFI_WRITE_PROTECTED)
AssertionType = EFI_TEST_ASSERTION_PASSED;
else
@@ -195,7 +196,7 @@ BBTestEraseBlocksConformanceTest (
);
} else {
- Status = EraseBlock->EraseBlocks(EraseBlock, MediaId + 1, 0, ,
BlockNumber);
+ Status = EraseBlock->EraseBlocks(EraseBlock, MediaId + 1, 0, ,
EraseSize);
if (Status == EFI_MEDIA_CHANGED)
AssertionType = EFI_TEST_ASSERTION_PASSED;
else
@@ -212,7 +213,7 @@ BBTestEraseBlocksConformanceTest (
Status
);
- Status = EraseBlock->EraseBlocks(EraseBlock, MediaId + 1, LastBlock + 1, , BlockNumber);
+ Status = EraseBlock->EraseBlocks(EraseBlock, MediaId + 1, LastBlock + 1,
, EraseSize);
if (Status == EFI_MEDIA_CHANGED)
AssertionType = EFI_TEST_ASSERTION_PASSED;
else
@@ -229,7 +230,7 @@ BBTestEraseBlocksConformanceTest (
Status
);
- Status = EraseBlock->EraseBlocks(EraseBlock, MediaId + 1, LastBlock - 10, , BlockNumber + 1);
+ Status = EraseBlock->EraseBlocks(EraseBlock, MediaId + 1, LastBlock - 10,
, EraseSize + 1);
Magic Number
Re: [edk2] [PATCH] uefi-sct/SctPkg:The original design for the 'EraseLengthGranularity' test need consider this case
Commit Message less than 80 cols please.
On 10/13/2018 04:51 PM, Eric Jin wrote:
For the SD device, no same as the eMMC, the 'EraseLengthGranularity' is 1.
Cc: Supreeth Venkatesh
Cc: Jiaxin Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin
---
.../BlackBoxTest/EraseBlockBBTestFunction.c | 154 +-
1 file changed, 78 insertions(+), 76 deletions(-)
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestFunction.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestFunction.c
index bc16a473..ea081625 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestFunction.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestFunction.c
@@ -214,54 +214,56 @@ BBTestEraseBlocksFunctionTest (
}
// Erase Blocks with non-EraseLengthGranularity blocks
- Token.Event = NULL;
- Token.TransactionStatus = EFI_NOT_READY;
- EraseStatus = EraseBlock->EraseBlocks (EraseBlock, MediaId, Lba+1,
, BufferSize - 2 * BlockSize);
-
- // Read the data with 0, the first/last block should not be erased
- ReadStatus = BlockIo->ReadBlocks (BlockIo, MediaId, Lba, BufferSize,
(VOID*)Buffer2);
- if (ReadStatus == EFI_SUCCESS) {
-for (Index1 = 0; Index1 < BlockSize; Index1++) {
- if (Buffer2[Index1] != 0) {
-IsZero1 = FALSE;
-break;
+ if (BufferSize > 2 * BlockSize) {
Magic Number 2.
+Token.Event = NULL;
+Token.TransactionStatus = EFI_NOT_READY;
+
+EraseStatus = EraseBlock->EraseBlocks (EraseBlock, MediaId, Lba+1,
, BufferSize - 2 * BlockSize);
Magic number 2.
+
+// Read the data with 0, the first/last block should not be erased
+ReadStatus = BlockIo->ReadBlocks (BlockIo, MediaId, Lba, BufferSize,
(VOID*)Buffer2);
+if (ReadStatus == EFI_SUCCESS) {
+ for (Index1 = 0; Index1 < BlockSize; Index1++) {
+if (Buffer2[Index1] != 0) {
+ IsZero1 = FALSE;
+ break;
+}
}
-}
-for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
- if (Buffer2[Index1] != 0) {
-IsZero2 = FALSE;
-break;
+ for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
+if (Buffer2[Index1] != 0) {
+ IsZero2 = FALSE;
+ break;
+}
}
-}
-for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) {
- if (Buffer2[Index1] != 0) {
-IsZero3 = FALSE;
-break;
+ for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++)
{
+if (Buffer2[Index1] != 0) {
+ IsZero3 = FALSE;
+ break;
+}
}
-}
-if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 == TRUE) && ((IsZero3 == FALSE)))
- AssertionType = EFI_TEST_ASSERTION_PASSED;
-else
- AssertionType = EFI_TEST_ASSERTION_FAILED;
+ if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 == TRUE)
&& ((IsZero3 == FALSE)))
Less than 80 cols.
+ AssertionType = EFI_TEST_ASSERTION_PASSED;
+ else
+AssertionType = EFI_TEST_ASSERTION_FAILED;
-StandardLib->RecordAssertion (
- StandardLib,
- AssertionType,
- gEraseBlockBBTestFunctionAssertionGuid003,
- L"EraseBlocks - EraseBlocks for testing, the first/last
block should not be erased",
- L"%a:%d:EraseBlocks Status - %r, IsZero1 - %d, IsZero2 - %d,
IsZero3 - %d",
- __FILE__,
- (UINTN)__LINE__,
- Status,
- IsZero1, IsZero2, IsZero3
- );
+ StandardLib->RecordAssertion (
+ StandardLib,
+ AssertionType,
+ gEraseBlockBBTestFunctionAssertionGuid003,
+ L"EraseBlocks - EraseBlocks for testing, the first/last
block should not be erased",
+ L"%a:%d:EraseBlocks Status - %r, IsZero1 - %d, IsZero2 -
%d, IsZero3 - %d",
+ __FILE__,
+ (UINTN)__LINE__,
+ EraseStatus,
+ IsZero1, IsZero2, IsZero3
+ );
+}
}
-
//
// Erase Blocks with the EraseLengthGranularity blocks
//
@@ -453,13 +455,13 @@ BlockIo2:
//
// Erase Blocks with non EraseLengthGranularity blocks
//
+ if (BufferSize > 2 * BlockSize) {
Magic Number 2.
+Token.Event
Re: [edk2] [PATCH] uefi-sct/SctPkg:Fix the flaw in BBTestCreateEventEx_Func_Sub3 on certain situation. Besides AllocatePages(), CreateEventEx may cause the memorymap change itself. Enhance the test to
FYI
On 10/15/2018 02:33 AM, Supreeth Venkatesh wrote:
Please use a commit message less than 80 Cols.
On 10/13/2018 04:42 PM, Eric Jin wrote:
Cc: Supreeth Venkatesh
Cc: Jiaxin Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin
---
...rTaskPriorityServicesBBTestCreateEventEx.c | 26 +++
.../BlackBoxTest/Support.c | 19 +-
2 files changed, 33 insertions(+), 12 deletions(-)
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
index e2e173ab..25d1ed97 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2016 Unified EFI, Inc.
- Copyright (c) 2010 - 2016, Intel Corporation. All rights
reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights
reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of
the BSD License
@@ -192,6 +192,10 @@ BBTestCreateEventEx_Func (
BBTestCreateEventEx_Func_Sub2 (StandardLib);
#endif
+ //
+ // The test for the EFI_EVENT_GROUP_MEMORY_MAP_CHANGE
+ // This event group is notified by the system when the memory map
has changed.
+ //
BBTestCreateEventEx_Func_Sub3 (StandardLib);
//
@@ -599,12 +603,12 @@ BBTestCreateEventEx_Func_Sub1 (
UINTN Buffer[MAX_TEST_EVENT_NUM +
MAX_TEST_EVENT_NUM*2];
//
- // Initialize Buffer
+ // Initialize Buffer and the 0xAA is only for the Sub3 test
//
for (Index = 0; Index < MAX_TEST_EVENT_NUM; Index ++) {
Strange Logic here. Needs re-look.
Buffer[Index] = Index;
- Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(-1);
- Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(-1);
+ Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(0xAA);
Magic Number 0xAA
+ Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(0xAA);
Magic Number 0xAA. Please define Macro or const.
}
//
@@ -755,12 +759,12 @@ BBTestCreateEventEx_Func_Sub2 (
UINTN Buffer[MAX_TEST_EVENT_NUM +
MAX_TEST_EVENT_NUM*2];
//
- // Initialize Buffer
+ // Initialize Buffer and the 0xAA is only for the Sub3 test
//
for (Index = 0; Index < MAX_TEST_EVENT_NUM; Index ++) {
Buffer[Index] = Index;
- Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(-1);
- Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(-1);
+ Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(0xAA);
Magic Number 0xAA. Please define Macro or const.
+ Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(0xAA);
Magic Number 0xAA. Please define Macro or const.
}
//
@@ -914,12 +918,12 @@ BBTestCreateEventEx_Func_Sub3 (
UINTN Buffer[MAX_TEST_EVENT_NUM +
MAX_TEST_EVENT_NUM*2];
//
- // Initialize Buffer
+ // Initialize Buffer and the trick to initial it as 0xAA
//
for (Index = 0; Index < MAX_TEST_EVENT_NUM; Index ++) {
Buffer[Index] = Index;
- Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(-1);
- Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(-1);
+ Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(0xAA);
Strange Logic here. Needs re-look. Also, Magic Number AA.
+ Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(0xAA);
Strange Logic here. Needs re-look. Also, Magic Number AA.
}
//
@@ -974,7 +978,7 @@ BBTestCreateEventEx_Func_Sub3 (
}
//
- // Install a configuration table at TPL_NOTIFY
+ // Call AllocatePage to change the memorymap
//
OldTpl = gtBS->RaiseTPL (TPL_NOTIFY);
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c
index aa02383e..823e16ab 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2010 Unified EFI, Inc.
- Copyright (c) 2010, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights
reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of
the BSD License
@@ -64,6 +64,23 @@ NotifyFunctionTplEx(
Re: [edk2] [PATCH] uefi-sct/SctPkg:Add the checkpoint of Toggle state of ReadKeyStrokeEx
On 10/13/2018 05:21 PM, Eric Jin wrote:
UEFI Spec clarify the Toggle state
Cc: Supreeth Venkatesh
Cc: Jiaxin Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin
---
.../SimpleTextInputEx/BlackBoxTest/Guid.c | 7 +-
.../SimpleTextInputEx/BlackBoxTest/Guid.h | 12 +-
.../SimpleTextInputExBBTestFunction.c | 210 +
.../SimpleTextInputExBBTestMain.c | 13 +-
.../SimpleTextInputExBBTestMain.h | 22 +-
.../SimpleTextInputEx/BlackBoxTest/Guid.c | 7 +-
.../SimpleTextInputEx/BlackBoxTest/Guid.h | 12 +-
.../SimpleTextInputExBBTestFunction.c | 212 +-
.../SimpleTextInputExBBTestMain.c | 11 +-
.../SimpleTextInputExBBTestMain.h | 20 +-
10 files changed, 513 insertions(+), 13 deletions(-)
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/Guid.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/Guid.c
index 9cb19f48..ff2d50fa 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/Guid.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/Guid.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2012 Unified EFI, Inc.
- Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -63,3 +63,8 @@ EFI_GUID gSimpleTextInputExBBTestFunctionAssertionGuid007 =
EFI_TEST_SIMPLETEXTI
EFI_GUID gSimpleTextInputExBBTestFunctionAssertionGuid008 =
EFI_TEST_SIMPLETEXTINPUTEXBBTESTFUNCTION_ASSERTION_008_GUID;
EFI_GUID gSimpleTextInputExBBTestFunctionAssertionGuid009 = EFI_TEST_SIMPLETEXTINPUTEXBBTESTFUNCTION_ASSERTION_009_GUID;
+
+EFI_GUID gSimpleTextInputExBBTestFunctionAssertionGuid010 =
EFI_TEST_SIMPLETEXTINPUTEXBBTESTFUNCTION_ASSERTION_010_GUID;
+
+EFI_GUID gSimpleTextInputExBBTestFunctionAssertionGuid011 =
EFI_TEST_SIMPLETEXTINPUTEXBBTESTFUNCTION_ASSERTION_011_GUID;
+
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/Guid.h
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/Guid.h
index 6c90fca3..2a6be48b 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/Guid.h
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/Guid.h
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2010 Unified EFI, Inc.
- Copyright (c) 2010, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -119,3 +119,13 @@ extern EFI_GUID
gSimpleTextInputExBBTestFunctionAssertionGuid008;
{ 0x534369f7, 0x8399, 0x4353, { 0x94, 0xad, 0xc4, 0x48, 0xfa, 0xda, 0xeb,
0x84 } }
extern EFI_GUID gSimpleTextInputExBBTestFunctionAssertionGuid009;
+
+#define EFI_TEST_SIMPLETEXTINPUTEXBBTESTFUNCTION_ASSERTION_010_GUID \
+{ 0xcf4d54eb, 0x6696, 0x4794, { 0x91, 0x74, 0x59, 0xd, 0x1c, 0x22, 0xa8, 0x67
} }
+
+extern EFI_GUID gSimpleTextInputExBBTestFunctionAssertionGuid010;
+
+#define EFI_TEST_SIMPLETEXTINPUTEXBBTESTFUNCTION_ASSERTION_011_GUID \
+{ 0xf8e8f879, 0xa6d4, 0x4fd3, { 0x8b, 0x8e, 0xba, 0x1d, 0x18, 0xf1, 0x40, 0x71
} }
+
+extern EFI_GUID gSimpleTextInputExBBTestFunctionAssertionGuid011;
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c
index 153ade03..48f91002 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextInputEx/BlackBoxTest/SimpleTextInputExBBTestFunction.c
@@ -456,6 +456,78 @@ BBTestUnregisterKeyNotifyFunctionManualTest (
}
+EFI_STATUS
+BBTestReadKeyStrokeExFunctionAutoTest (
+ IN EFI_BB_TEST_PROTOCOL *This,
+ IN VOID *ClientInterface,
+ IN EFI_TEST_LEVEL TestLevel,
+ IN EFI_HANDLE SupportHandle
+ )
+{
+ EFI_STANDARD_TEST_LIBRARY_PROTOCOL*StandardLib;
+ EFI_STATUSStatus;
+ EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL *SimpleTextInputEx;
+
+ EFI_DEVICE_PATH_PROTOCOL *DevicePath;
+ CHAR16 *DevicePathStr;
+
+ //
+ // init
+ //
+ SimpleTextInputEx = (EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL*)ClientInterface;
+
+ //
+ // Get the Standard Library Interface
+ //
+ Status = gtBS->HandleProtocol (
+ SupportHandle,
+ ,
+
Re: [edk2] [PATCH] uefi-sct/SctPkg:Implement the iSCSI devicepath to text
On 10/14/2018 03:25 AM, Eric Jin wrote:
Cc: Supreeth Venkatesh
Cc: Jiaxin Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin
---
.../DevicePathToTextBBTestCoverage.c | 43 ++-
.../BlackBoxTest/DevicePathToTextBBTestMain.c | 26 +--
.../DevicePathToText/BlackBoxTest/Guid.c | 1 +
.../DevicePathToText/BlackBoxTest/Guid.h | 7 +++
4 files changed, 73 insertions(+), 4 deletions(-)
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestCoverage.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestCoverage.c
index c30af434..14c4c460 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestCoverage.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestCoverage.c
@@ -2202,7 +2202,48 @@ DevicePathToTextConvertDeviceNodeToTextCoverageTest (
SctFreePool (Text);
}
-
+ //
+ // iSCSI(Name,0x12AB,0x00567800,CRC32C,None,CHAP_BI,TCP)
+ //
+ pDeviceNode1 = DevicePathUtilities->CreateDeviceNode (0x3, 0x13, sizeof
(ISCSI_DEVICE_PATH_WITH_NAME) + 4);
Why 0x03, 0x13 and + 4? - Magic Numbers.
+ ((ISCSI_DEVICE_PATH_WITH_NAME *)pDeviceNode1)->NetworkProtocol = 0x0; //TCP
+ ((ISCSI_DEVICE_PATH_WITH_NAME *)pDeviceNode1)->LoginOption = 0x0002;
Magic Number 2.
+ ((ISCSI_DEVICE_PATH_WITH_NAME *)pDeviceNode1)->Lun = 0x00785600;
Magic Number
+ ((ISCSI_DEVICE_PATH_WITH_NAME *)pDeviceNode1)->TargetPortalGroupTag = 0x12AB;
Magic Number.
+ ((ISCSI_DEVICE_PATH_WITH_NAME *)pDeviceNode1)->iSCSITargetName[0] = 'N';
+ ((ISCSI_DEVICE_PATH_WITH_NAME *)pDeviceNode1)->iSCSITargetName[1] = 'a';
+ ((ISCSI_DEVICE_PATH_WITH_NAME *)pDeviceNode1)->iSCSITargetName[2] = 'm';
+ ((ISCSI_DEVICE_PATH_WITH_NAME *)pDeviceNode1)->iSCSITargetName[3] = 'e';
+
+ Text = DevicePathToText->ConvertDeviceNodeToText (pDeviceNode1, FALSE,
FALSE);
+ pDeviceNode2 = SctConvertTextToDeviceNode(Text);
+
+ if ((pDeviceNode2 != NULL) && (SctCompareMem (pDeviceNode2, pDeviceNode1,
SctDevicePathNodeLength(pDeviceNode1)) == 0)) {
+AssertionType = EFI_TEST_ASSERTION_PASSED;
+ } else {
+AssertionType = EFI_TEST_ASSERTION_FAILED;
+ }
+
+ StandardLib->RecordAssertion (
+StandardLib,
+AssertionType,
+gDevicePathToTextBBTestFunctionAssertionGuid135,
+L"EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL - ConvertDeviceNodeToText must
correctly recover the converting ConvertTextToDeviceNode has acted on the device node
string",
+L"%a:%d: Convert result: %s -
Expected:iSCSI(MyTargetName,0x12AB,0x00567800,CRC32C,None,CHAP_BI,TCP)",
Magic Numbers.
+__FILE__,
+(UINTN)__LINE__,
+Text
+);
+ if (pDeviceNode1 != NULL) {
+SctFreePool (pDeviceNode1);
+ }
+ if (pDeviceNode2 != NULL) {
+SctFreePool (pDeviceNode2);
+ }
+ if (Text != NULL) {
+SctFreePool (Text);
+ }
+
return EFI_SUCCESS;
}
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestMain.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestMain.c
index 41cf217b..d755227c 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestMain.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestMain.c
@@ -2437,6 +2437,7 @@ BuildiSCSIDeviceNode (
CHAR16 *LunStr;
UINT16 Options;
UINT64 LunNum;
+ UINT64 TempLunNum;
Status = GetNextRequiredParam(, L"TargetName", , );
if ((!EFI_ERROR(Status)) && (ParamIdentifierVal != NULL)) {
@@ -2459,7 +2460,7 @@ BuildiSCSIDeviceNode (
return NULL;
}
- Length = sizeof (ISCSI_DEVICE_PATH_WITH_NAME) + (UINT16) (SctStrLen (NameStr) * 2 + 2);
+ Length = sizeof (ISCSI_DEVICE_PATH_WITH_NAME) + (UINT16) (SctStrLen
(NameStr));
iSCSI = (ISCSI_DEVICE_PATH_WITH_NAME *) CreateDeviceNode (0x3, 0x13,
Length);
if (iSCSI == NULL) {
return NULL;
@@ -2499,7 +2500,7 @@ BuildiSCSIDeviceNode (
if (SctStrCmp (ParamIdentifierVal, L"CRC32C") == 0) {
Options |= 0x0002;
}
- break;
+break;
case 1: // DataDigest
if (SctStrCmp (ParamIdentifierVal, L"CRC32C") == 0) {
@@ -2514,6 +2515,9 @@ BuildiSCSIDeviceNode (
if (SctStrCmp (ParamIdentifierVal, L"CHAP_UNI") == 0) {
Options |= 0x1000;
}
+if (SctStrCmp (ParamIdentifierVal, L"CHAP_BI") == 0) {
+ Options |= 0x;
+}
break;
case 3: // Protocol
@@ -2533,8 +2537,24 @@
Re: [edk2] [PATCH] uefi-sct/SctPkg:One checkpoint in the ExtractConfigFunction need be removed
Reviewed-by: Supreeth Venkatesh
On 10/13/2018 04:30 PM, Eric Jin wrote:
The Results output from ExtractConfigFunction() may be different during two
calls in some case.
Cc: Supreeth Venkatesh
Cc: Jiaxin Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin
---
.../HIIConfigRouting/BlackBoxTest/Guid.c | 4 +---
.../HIIConfigRouting/BlackBoxTest/Guid.h | 6 +
.../HIIConfigRoutingBBTestFunction.c | 23 +--
3 files changed, 3 insertions(+), 30 deletions(-)
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/HIIConfigRouting/BlackBoxTest/Guid.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/HIIConfigRouting/BlackBoxTest/Guid.c
index 18282f30..93265947 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/HIIConfigRouting/BlackBoxTest/Guid.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/HIIConfigRouting/BlackBoxTest/Guid.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2011 Unified EFI, Inc.
- Copyright (c) 2010 - 2011, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -88,7 +88,5 @@ EFI_GUID gHIIConfigRoutingBBTestFunctionAssertionGuid009 =
EFI_TEST_HIICONFIGROU
EFI_GUID gHIIConfigRoutingBBTestFunctionAssertionGuid010 = EFI_TEST_HIICONFIGROUTINGBBTESTFUNCTION_ASSERTION_010_GUID;
-EFI_GUID gHIIConfigRoutingBBTestFunctionAssertionGuid011 = EFI_TEST_HIICONFIGROUTINGBBTESTFUNCTION_ASSERTION_011_GUID;
-
EFI_GUID gHIIConfigRoutingBBTestFunctionAssertionGuid012 =
EFI_TEST_HIICONFIGROUTINGBBTESTFUNCTION_ASSERTION_012_GUID;
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/HIIConfigRouting/BlackBoxTest/Guid.h b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/HIIConfigRouting/BlackBoxTest/Guid.h
index 97e257e7..7ade1a0f 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/HIIConfigRouting/BlackBoxTest/Guid.h
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/HIIConfigRouting/BlackBoxTest/Guid.h
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2011 Unified EFI, Inc.
- Copyright (c) 2010 - 2011, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -180,10 +180,6 @@ extern EFI_GUID
gHIIConfigRoutingBBTestFunctionAssertionGuid009;
extern EFI_GUID gHIIConfigRoutingBBTestFunctionAssertionGuid010;
-#define EFI_TEST_HIICONFIGROUTINGBBTESTFUNCTION_ASSERTION_011_GUID \
-{ 0xf91ef5f3, 0xe0c6, 0x4aca, { 0xa0, 0xd0, 0x5, 0xf9, 0xb1, 0x6a, 0x13, 0xbd
} }
-
-extern EFI_GUID gHIIConfigRoutingBBTestFunctionAssertionGuid011;
#define EFI_TEST_HIICONFIGROUTINGBBTESTFUNCTION_ASSERTION_012_GUID \
{ 0xf732d246, 0x9fa5, 0x4ed3, { 0x88, 0x95, 0x28, 0x63, 0xba, 0xf4, 0x68,
0x5d } }
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/HIIConfigRouting/BlackBoxTest/HIIConfigRoutingBBTestFunction.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/HIIConfigRouting/BlackBoxTest/HIIConfigRoutingBBTestFunction.c
index 5eed6c6c..d4bd23d1 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/HIIConfigRouting/BlackBoxTest/HIIConfigRoutingBBTestFunction.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/HIIConfigRouting/BlackBoxTest/HIIConfigRoutingBBTestFunction.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2016 Unified EFI, Inc.
- Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -418,27 +418,6 @@ BBTestExtractConfigFunctionTestCheckpoint1 (
Status
);
- //
- // Since ExtractConfig may not append at string tail.
- // We check whether Results is a substring of MultiConfigAltResp from
ExportConfig
- //
- if (Status == EFI_SUCCESS && (SctStrStr (MultiConfigAltResp, Results) !=
NULL)) {
-AssertionType = EFI_TEST_ASSERTION_PASSED;
- } else if (EFI_OUT_OF_RESOURCES == Status){
-AssertionType = EFI_TEST_ASSERTION_WARNING;
- } else {
-AssertionType = EFI_TEST_ASSERTION_FAILED;
- }
- StandardLib->RecordAssertion (
- StandardLib,
- AssertionType,
- gHIIConfigRoutingBBTestFunctionAssertionGuid011,
- L"HII_CONFIG_ROUTING_PROTOCOL.ExtractConfig - ExtractConfig() Check if
Results is in format.",
- L"%a:%d:",
- __FILE__,
- (UINTN)__LINE__
- );
-
FUNC_EXIT:
if (Request != NULL) {
___
edk2-devel mailing list
edk2-devel@lists.01.org
Re: [edk2] [PATCH] uefi-sct/SctPkg:Fix the flaw in BBTestCreateEventEx_Func_Sub3 on certain situation. Besides AllocatePages(), CreateEventEx may cause the memorymap change itself. Enhance the test to
Please use a commit message less than 80 Cols.
On 10/13/2018 04:42 PM, Eric Jin wrote:
Cc: Supreeth Venkatesh
Cc: Jiaxin Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin
---
...rTaskPriorityServicesBBTestCreateEventEx.c | 26 +++
.../BlackBoxTest/Support.c| 19 +-
2 files changed, 33 insertions(+), 12 deletions(-)
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
index e2e173ab..25d1ed97 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2016 Unified EFI, Inc.
- Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -192,6 +192,10 @@ BBTestCreateEventEx_Func (
BBTestCreateEventEx_Func_Sub2 (StandardLib);
#endif
+ //
+ // The test for the EFI_EVENT_GROUP_MEMORY_MAP_CHANGE
+ // This event group is notified by the system when the memory map has
changed.
+ //
BBTestCreateEventEx_Func_Sub3 (StandardLib);
//
@@ -599,12 +603,12 @@ BBTestCreateEventEx_Func_Sub1 (
UINTN Buffer[MAX_TEST_EVENT_NUM + MAX_TEST_EVENT_NUM*2];
//
- // Initialize Buffer
+ // Initialize Buffer and the 0xAA is only for the Sub3 test
//
for (Index = 0; Index < MAX_TEST_EVENT_NUM; Index ++) {
Strange Logic here. Needs re-look.
Buffer[Index] = Index;
-Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(-1);
-Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(-1);
+Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(0xAA);
Magic Number 0xAA
+Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(0xAA);
Magic Number 0xAA. Please define Macro or const.
}
//
@@ -755,12 +759,12 @@ BBTestCreateEventEx_Func_Sub2 (
UINTN Buffer[MAX_TEST_EVENT_NUM + MAX_TEST_EVENT_NUM*2];
//
- // Initialize Buffer
+ // Initialize Buffer and the 0xAA is only for the Sub3 test
//
for (Index = 0; Index < MAX_TEST_EVENT_NUM; Index ++) {
Buffer[Index] = Index;
-Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(-1);
-Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(-1);
+Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(0xAA);
Magic Number 0xAA. Please define Macro or const.
+Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(0xAA);
Magic Number 0xAA. Please define Macro or const.
}
//
@@ -914,12 +918,12 @@ BBTestCreateEventEx_Func_Sub3 (
UINTN Buffer[MAX_TEST_EVENT_NUM + MAX_TEST_EVENT_NUM*2];
//
- // Initialize Buffer
+ // Initialize Buffer and the trick to initial it as 0xAA
//
for (Index = 0; Index < MAX_TEST_EVENT_NUM; Index ++) {
Buffer[Index] = Index;
-Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(-1);
-Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(-1);
+Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(0xAA);
Strange Logic here. Needs re-look. Also, Magic Number AA.
+Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(0xAA);
Strange Logic here. Needs re-look. Also, Magic Number AA.
}
//
@@ -974,7 +978,7 @@ BBTestCreateEventEx_Func_Sub3 (
}
//
- // Install a configuration table at TPL_NOTIFY
+ // Call AllocatePage to change the memorymap
//
OldTpl = gtBS->RaiseTPL (TPL_NOTIFY);
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c
index aa02383e..823e16ab 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2010 Unified EFI, Inc.
- Copyright (c) 2010, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -64,6 +64,23 @@ NotifyFunctionTplEx(
EventIndex = Buffer[0];
+//
+// The
Re: [edk2] [PATCH] uefi-sct/SctPkg:The Lun display order issue in iSCSI device path text
On 10/13/2018 05:33 PM, Eric Jin wrote:
Cc: Supreeth Venkatesh
Cc: Jiaxin Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin
---
.../BlackBoxTest/DevicePathFromTextBBTestCoverage.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c
index fc099d8e..6f97924a 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathFromText/BlackBoxTest/DevicePathFromTextBBTestCoverage.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2017 Unified EFI, Inc.
- Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -1442,7 +1442,7 @@ CreateiScsiDeviceNode (
CHAR16 *DataDigestStr;
CHAR16 *AuthenticationStr;
CHAR16 *ProtocolStr;
- UINT64 LunNum;
+ UINT64 LunNum = 0;
EFI coding convention does not allow initialization during definition.
ISCSI_DEVICE_PATH_WITH_NAME *iSCSI;
NameStr = SctSplitStr (, L',');
@@ -1459,7 +1459,7 @@ CreateiScsiDeviceNode (
);
SctUnicodeToAscii (iSCSI->iSCSITargetName, NameStr, SctStrLen (NameStr));
iSCSI->TargetPortalGroupTag = (UINT16) SctStrToUInt (PortalGroupStr);
- SctStrToUInt64 (LunStr, );
+ StrToUInt8Array(LunStr, );
iSCSI->Lun = LunNum;
Options = 0x;
@@ -2846,12 +2846,12 @@ DevicePathFromTextConvertTextToDeviceNodeCoverageTest (
(UINTN)__LINE__
);
//
- // TDS 3.10.1.2.26
+ // TDS 3.10.1.2.26 0x5678 - byte 3 is 0x56 and byte4 is 0x78
//
- SctStrCpy (text, L"MyTargetName,0x12AB,5678,CRC32C,None,CHAP_BI,TCP");
+ SctStrCpy (text,
L"MyTargetName,0x12AB,0x00567800,CRC32C,None,CHAP_BI,TCP");
Magic String.
pDevicePath = CreateiScsiDeviceNode(text);
- SctStrCpy (text, L"iSCSI(MyTargetName,0x12AB,5678,CRC32C,None,CHAP_BI,TCP)");
+ SctStrCpy (text,
L"iSCSI(MyTargetName,0x12AB,0x00567800,CRC32C,None,CHAP_BI,TCP)");
Magic String.
pReDevicePath = DevicePathFromText->ConvertTextToDeviceNode (text);
if (SctCompareMem (pDevicePath, pReDevicePath, SctDevicePathNodeLength
((EFI_DEVICE_PATH_PROTOCOL *) pReDevicePath)) == 0) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] uefi-sct/SctPkg:Assign 0 to the tail of HwErrRecVariableName
On 10/14/2018 02:31 AM, Eric Jin wrote: Ensure the HwErrRecVariable could be deleted before the test exit. Cc: Supreeth Venkatesh Cc: Jiaxin Wu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Jin --- .../BlackBoxTest/VariableServicesBBTestFunction.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestFunction.c index d1064cec..1be1720a 100644 --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestFunction.c +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestFunction.c @@ -1,7 +1,7 @@ /** @file Copyright 2006 - 2012 Unified EFI, Inc. - Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved. + Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved. This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License @@ -3052,6 +3052,7 @@ HardwareErrorRecordFuncTest ( // step2: DataSize = 255; + HwErrRecVariableName[12] = L'\0'; Magic Number 12. SctStrnCpy ( HwErrRecVariableName, (CHAR16*)(RecoveryData+2), 12 ); Status = RT->GetVariable ( HwErrRecVariableName, ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] uefi-sct/SctPkg: Fix the BlueTooth Guid and Enable BLE test
On 10/14/2018 04:54 AM, Eric Jin wrote:
Correct the guid of EFI_GUID gEfiBlueToothIoProtocolGuid and
gEfiBlueToothConfigProtocolGuid
Add BlueToothLE support test in the EfiCompliant part
Cc: Supreeth Venkatesh
Cc: Jiaxin Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin
---
.../Dependency/Config/EfiCompliant.ini| 9 +-
.../EfiCompliantBBTestPlatform_uefi.c | 127 --
.../EfiCompliant/BlackBoxTest/Guid_uefi.c | 4 +-
.../EfiCompliant/BlackBoxTest/Guid_uefi.h | 7 +-
4 files changed, 132 insertions(+), 15 deletions(-)
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/Dependency/Config/EfiCompliant.ini
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/Dependency/Config/EfiCompliant.ini
index 78b5f7b5..7c0bdcd6 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/Dependency/Config/EfiCompliant.ini
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/Dependency/Config/EfiCompliant.ini
@@ -1,7 +1,7 @@
## @file
#
# Copyright 2006 - 2016 Unified EFI, Inc.
-# Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD
License
@@ -84,7 +84,9 @@
#
# EAPSupport=
#
-# BlueToothSupport =
+# BlueToothClassicSupport =
+#
+# BlueToothLESupport=
#
# IPSecSupport =
#
@@ -120,6 +122,7 @@ DNS6Support = yes
TLSSupport= yes
HTTPSupport = yes
EAPSupport= yes
-BlueToothSupport = yes
+BlueToothClassicSupport = yes
+BlueToothLESupport= yes
IPSecSupport = yes
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/EfiCompliantBBTestPlatform_uefi.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/EfiCompliantBBTestPlatform_uefi.c
index 17df564b..2d45a7c0 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/EfiCompliantBBTestPlatform_uefi.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Generic/EfiCompliant/BlackBoxTest/EfiCompliantBBTestPlatform_uefi.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2016 Unified EFI, Inc.
- Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -152,9 +152,9 @@ EFI_GUID gEfiBlueToothHcProtocolGuid = { 0xb3930571,
0xbeba, 0x4fc5, {0x92, 0x3,
EFI_GUID gEfiBlueToothServiceBindingProtocolGuid = { 0x388278d3, 0x7b85, 0x42f0, {0xab, 0xa9, 0xfb, 0x4b, 0xfd, 0x69, 0xf5, 0xab }};
-EFI_GUID gEfiBlueToothIoProtocolGuid = { 0x388278d3, 0x7b85, 0x42f0, {0xab, 0xa9, 0xfb, 0x4b, 0xfd, 0x69, 0xf5, 0xab }};
+EFI_GUID gEfiBlueToothIoProtocolGuid = { 0x467313de, 0x4e30, 0x43f1,{ 0x94,
0x3e, 0x32, 0x3f, 0x89, 0x84, 0x5d, 0xb5 }};
-EFI_GUID gEfiBlueToothConfigProtocolGuid = { 0xb3930571, 0xbeba, 0x4fc5, {0x92, 0x3, 0x94, 0x27, 0x24, 0x2e, 0x6a, 0x43 }};
+EFI_GUID gEfiBlueToothConfigProtocolGuid = { 0x62960cf3, 0x40ff, 0x4263,{0xa7,
0x7c, 0xdf, 0xde, 0xbd, 0x19, 0x1b, 0x4b }};
EFI_GUID gEfiEapProtocolGuid = { 0x5d9f96db, 0xe731, 0x4caa, {0xa0, 0x0d, 0x72, 0xe1, 0x87, 0xcd, 0x77, 0x62 }};
@@ -166,6 +166,10 @@ EFI_GUID gEfiIPSecConfigProtocolGuid = { 0xce5e5929, 0xc7a3, 0x4602, {0xad, 0x9e
EFI_GUID gEfiIPSec2ProtocolGuid = { 0xa3979e64, 0xace8, 0x4ddc, {0xbc, 0x07, 0x4d, 0x66, 0xb8, 0xfd, 0x09, 0x77 }};
+EFI_GUID gEfiBlueToothAttributeProtocolGuid = { 0x898890e9, 0x84b2, 0x4f3a, { 0x8c, 0x58, 0xd8, 0x57, 0x78, 0x13, 0xe0, 0xac }};
+
+EFI_GUID gEfiBlueToothLEConfigProtocolGuid = { 0x8f76da58, 0x1f99, 0x4275, {
0xa4, 0xec, 0x47, 0x56, 0x51, 0x5b, 0x1c, 0xe8 }};
+
//
// Internal functions declarations
//
@@ -353,7 +357,13 @@ CheckEAPProtocols (
);
EFI_STATUS
-CheckBlueToothProtocols (
+CheckBlueToothClassicProtocols (
+ IN EFI_STANDARD_TEST_LIBRARY_PROTOCOL *StandardLib,
+ IN EFI_INI_FILE_HANDLE IniFile
+ );
+
+EFI_STATUS
+CheckBlueToothLEProtocols (
IN EFI_STANDARD_TEST_LIBRARY_PROTOCOL *StandardLib,
IN EFI_INI_FILE_HANDLE IniFile
);
@@ -564,7 +574,8 @@ Routine Description:
//
// Check the BlueTooth protocols
//
- CheckBlueToothProtocols (StandardLib, IniFile);
+ CheckBlueToothClassicProtocols (StandardLib, IniFile);
+ CheckBlueToothLEProtocols (StandardLib, IniFile);
//
// Check the IPSec protocols
@@ -3534,7 +3545,7 @@ CheckEAPProtocols (
}
EFI_STATUS
-CheckBlueToothProtocols (
+CheckBlueToothClassicProtocols
Re: [edk2] [PATCH] uefi-sct/SctPkg: Fix the Possible numeric underflow
Reviewed-by: Supreeth Venkatesh
On 10/13/2018 05:27 PM, Eric Jin wrote:
Possible numeric underflow when calculating the starting LBA for
ReadBlocks_Func test
Cc: Supreeth Venkatesh
Cc: Jiaxin Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin
---
.../Protocol/BlockIo/BlackBoxTest/BlockIoBBTestFunction.c | 6 +-
.../Protocol/BlockIo/BlackBoxTest/BlockIoBBTestFunction.c | 6 +-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/BlockIo/BlackBoxTest/BlockIoBBTestFunction.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/BlockIo/BlackBoxTest/BlockIoBBTestFunction.c
index 847ff9cb..e25743b7 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/BlockIo/BlackBoxTest/BlockIoBBTestFunction.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/BlockIo/BlackBoxTest/BlockIoBBTestFunction.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2016 Unified EFI, Inc.
- Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -316,9 +316,13 @@ BBTestReadBlocksFunctionAutoTest (
NewLba = IndexJ;
} else if (IndexJ < 2 * MAX_DIFFERENT_LBA_FOR_TEST) {
// from (LastBlock - MAX_DIFFERENT_LBA_FOR_TEST -
MAX_DIFFERENT_BUFFERSIZE_FOR_TEST) to (LastBlock -
MAX_DIFFERENT_BUFFERSIZE_FOR_TEST)
+ if (LastBlock < MAX_DIFFERENT_LBA_FOR_TEST +
MAX_DIFFERENT_BUFFERSIZE_FOR_TEST)
+continue;
NewLba = IndexJ + LastBlock - 2 * MAX_DIFFERENT_LBA_FOR_TEST -
MAX_DIFFERENT_BUFFERSIZE_FOR_TEST;
} else {
// from (LastBlock/2 - MAX_DIFFERENT_LBA_FOR_TEST/2) to
(LastBlock/2 + MAX_DIFFERENT_LBA_FOR_TEST/2)
+ if (LastBlock < MAX_DIFFERENT_LBA_FOR_TEST)
+continue;
NewLba = IndexJ - 2 * MAX_DIFFERENT_LBA_FOR_TEST + (SctDivU64x32
(LastBlock, 2, ) - MAX_DIFFERENT_LBA_FOR_TEST / 2);
}
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/BlockIo/BlackBoxTest/BlockIoBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/BlockIo/BlackBoxTest/BlockIoBBTestFunction.c
index 2590c035..e8d4295e 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/BlockIo/BlackBoxTest/BlockIoBBTestFunction.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/BlockIo/BlackBoxTest/BlockIoBBTestFunction.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2016 Unified EFI, Inc.
- Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -316,9 +316,13 @@ BBTestReadBlocksFunctionAutoTest (
NewLba = IndexJ;
} else if (IndexJ < 2 * MAX_DIFFERENT_LBA_FOR_TEST) {
// from (LastBlock - MAX_DIFFERENT_LBA_FOR_TEST -
MAX_DIFFERENT_BUFFERSIZE_FOR_TEST) to (LastBlock -
MAX_DIFFERENT_BUFFERSIZE_FOR_TEST)
+ if (LastBlock < MAX_DIFFERENT_LBA_FOR_TEST +
MAX_DIFFERENT_BUFFERSIZE_FOR_TEST)
+continue;
NewLba = IndexJ + LastBlock - 2 * MAX_DIFFERENT_LBA_FOR_TEST -
MAX_DIFFERENT_BUFFERSIZE_FOR_TEST;
} else {
// from (LastBlock/2 - MAX_DIFFERENT_LBA_FOR_TEST/2) to
(LastBlock/2 + MAX_DIFFERENT_LBA_FOR_TEST/2)
+ if (LastBlock < MAX_DIFFERENT_LBA_FOR_TEST)
+continue;
NewLba = IndexJ - 2 * MAX_DIFFERENT_LBA_FOR_TEST + (SctDivU64x32
(LastBlock, 2, ) - MAX_DIFFERENT_LBA_FOR_TEST / 2);
}
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] uefi-sct/SctPkg:Enhance the SimpleNetwork Test
On 10/14/2018 03:06 AM, Eric Jin wrote:
Add the EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST bit in the Enable parameter
Add one checkpoint to MCastFilterCount is zero
Cc: Supreeth Venkatesh
Cc: Jiaxin Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin
---
.../SimpleNetwork/BlackBoxTest/Guid.c | 4 +-
.../SimpleNetwork/BlackBoxTest/Guid.h | 7 +-
.../SimpleNetworkBBTestConformance.c | 66 +--
.../SimpleNetwork/BlackBoxTest/Guid.c | 4 +-
.../SimpleNetwork/BlackBoxTest/Guid.h | 7 +-
.../SimpleNetworkBBTestConformance.c | 66 +--
6 files changed, 110 insertions(+), 44 deletions(-)
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid.c
index 6ea6c4cb..72343236 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2016 Unified EFI, Inc.
- Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -112,6 +112,8 @@ EFI_GUID gSimpleNetworkBBTestConformanceAssertionGuid041 =
EFI_TEST_SIMPLENETWOR
EFI_GUID gSimpleNetworkBBTestConformanceAssertionGuid042 = EFI_TEST_SIMPLENETWORKBBTESTCONFORMANCE_ASSERTION_042_GUID;
+EFI_GUID gSimpleNetworkBBTestConformanceAssertionGuid043 = EFI_TEST_SIMPLENETWORKBBTESTCONFORMANCE_ASSERTION_043_GUID;
+
EFI_GUID gSimpleNetworkBBTestFunctionAssertionGuid001 =
EFI_TEST_SIMPLENETWORKBBTESTFUNCTION_ASSERTION_001_GUID;
EFI_GUID gSimpleNetworkBBTestFunctionAssertionGuid002 = EFI_TEST_SIMPLENETWORKBBTESTFUNCTION_ASSERTION_002_GUID;
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid.h
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid.h
index 281d893a..bf909d1c 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid.h
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid.h
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2016 Unified EFI, Inc.
- Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -235,6 +235,11 @@ extern EFI_GUID
gSimpleNetworkBBTestConformanceAssertionGuid041;
extern EFI_GUID gSimpleNetworkBBTestConformanceAssertionGuid042;
+#define EFI_TEST_SIMPLENETWORKBBTESTCONFORMANCE_ASSERTION_043_GUID \
+{ 0x8cec0b86, 0x7773, 0x4d3c, {0x84, 0x13, 0x26, 0x37, 0xfb, 0xd0, 0x8e, 0x1b
}}
+
+extern EFI_GUID gSimpleNetworkBBTestConformanceAssertionGuid043;
+
#define EFI_TEST_SIMPLENETWORKBBTESTFUNCTION_ASSERTION_001_GUID \
{ 0xf58651fe, 0x0538, 0x4407, {0x88, 0xe0, 0x88, 0xb8, 0xda, 0x18, 0x38, 0x3a
}}
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/SimpleNetworkBBTestConformance.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/SimpleNetworkBBTestConformance.c
index ccbbad08..b65d7d3b 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/SimpleNetworkBBTestConformance.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/SimpleNetworkBBTestConformance.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2016 Unified EFI, Inc.
- Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -581,11 +581,12 @@ BBTestReceiveFilterConformanceTest (
{
EFI_STANDARD_TEST_LIBRARY_PROTOCOL*StandardLib;
EFI_STATUSStatus;
- EFI_STATUSStatusBuf[5];
- EFI_TEST_ASSERTIONAssertionType[5];
+ EFI_STATUSStatusBuf[6];
Magic Number 6.
+ EFI_TEST_ASSERTIONAssertionType[6];
Magic number 6.
EFI_SIMPLE_NETWORK_PROTOCOL *SnpInterface;
EFI_SIMPLE_NETWORK_STATE State1, State2;
-
+ EFI_MAC_ADDRESS MAC;
+
//
// Get the Standard Library Interface
//
@@ -673,23 +674,37 @@ BBTestReceiveFilterConformanceTest (
//
// Call ReceiveFilters with invalide MCastFilterCnt
//
- StatusBuf[3] = SnpInterface->ReceiveFilters (SnpInterface, 0, 0, FALSE,
Re: [edk2] [PATCH] uefi-sct/SctPkg:Add conformance test cases for deprecated EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS attribute.
Reviewed-by: Supreeth Venkatesh
If the commit message is broken down into multiple lines less than 80 cols.
On 10/13/2018 04:19 PM, Eric Jin wrote:
Cc: Supreeth Venkatesh
Cc: Jiaxin Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin
---
.../AuthVariableServicesBBTestConformance.c | 143 ++
.../VariableServices/BlackBoxTest/Guid.c | 6 +-
.../VariableServices/BlackBoxTest/Guid.h | 11 +-
3 files changed, 132 insertions(+), 28 deletions(-)
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/AuthVariableServicesBBTestConformance.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/AuthVariableServicesBBTestConformance.c
index 05281054..a1d1c4c3 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/AuthVariableServicesBBTestConformance.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/AuthVariableServicesBBTestConformance.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2012 Unified EFI, Inc.
- Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -151,6 +151,44 @@ AuthVariableDERConfTest (
EFI_TEST_LOGGING_LIBRARY_PROTOCOL *LoggingLib;
UINT32 Attr;
EFI_TEST_ASSERTION Result;
+ UINTN Index;
+ UINTN MaximumVariableStorageSize;
+ UINTN RemainingVariableStorageSize;
+ UINTN MaximumVariableSize;
+ UINT32 AttrArray[] = {
+//
+// For 1 attribute.
+//
+EFI_VARIABLE_NON_VOLATILE,
+EFI_VARIABLE_RUNTIME_ACCESS,
+EFI_VARIABLE_BOOTSERVICE_ACCESS,
+EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS,
+
+//
+// For 2 attributes.
+//
+EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS,
+EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
+EFI_VARIABLE_NON_VOLATILE |
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS,
+
+EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS,
+EFI_VARIABLE_RUNTIME_ACCESS |
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS,
+
+EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS,
+
+//
+// For 3 attributes.
+//
+EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS |
EFI_VARIABLE_BOOTSERVICE_ACCESS,
+EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS |
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS,
+EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS,
+EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS,
+
+//
+// For 4 attributes.
+//
+EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS |
EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS,
+ };
Status = GetTestSupportLibrary (
SupportHandle,
@@ -192,33 +230,86 @@ AuthVariableDERConfTest (
Status
);
- Attr = EFI_VARIABLE_NON_VOLATILE |
- EFI_VARIABLE_RUNTIME_ACCESS |
- EFI_VARIABLE_BOOTSERVICE_ACCESS |
- EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS;
+ for (Index = 0; Index < sizeof (AttrArray) / sizeof (AttrArray[0]); Index =
Index + 1) {
+Attr = AttrArray[Index];
+Attr |= EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS;
+
+Status = RT->SetVariable (
+ L"AuthVarDER",
+ ,
+ Attr,
+ sizeof (mValidAuthVarDERCreate),
+ (VOID *) mValidAuthVarDERCreate
+ );
+if (Status == EFI_UNSUPPORTED) {
+ Result = EFI_TEST_ASSERTION_PASSED;
+} else {
+ Result = EFI_TEST_ASSERTION_FAILED;
+}
+
+StandardLib->RecordAssertion (
+ StandardLib,
+ Result,
+ gVariableServicesBbTestConformanceAssertionGuid020,
+ L"RT.SetVariable - Set Auth Variable with valid cert.",
+ L"Attributes = Array[%d]. %a:%d:Status - %r",
+ Index,
+ __FILE__,
+ (UINTN)__LINE__,
+ Status
+ );
+
+Status = RT->SetVariable (
+ L"AuthVarDER",
+ ,
+ Attr,
+ sizeof (mInvalidAuthVarDERCreate),
+ (VOID *) mInvalidAuthVarDERCreate
+ );
+if (Status == EFI_UNSUPPORTED) {
+ Result =
Re: [edk2] [PATCH] uefi-sct/SctPkg:The original design for the 'EraseLengthGranularity' test need consider this case
Commit Message less than 80 cols please.
On 10/13/2018 04:51 PM, Eric Jin wrote:
For the SD device, no same as the eMMC, the 'EraseLengthGranularity' is 1.
Cc: Supreeth Venkatesh
Cc: Jiaxin Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin
---
.../BlackBoxTest/EraseBlockBBTestFunction.c | 154 +-
1 file changed, 78 insertions(+), 76 deletions(-)
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestFunction.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestFunction.c
index bc16a473..ea081625 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestFunction.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestFunction.c
@@ -214,54 +214,56 @@ BBTestEraseBlocksFunctionTest (
}
// Erase Blocks with non-EraseLengthGranularity blocks
- Token.Event = NULL;
- Token.TransactionStatus = EFI_NOT_READY;
- EraseStatus = EraseBlock->EraseBlocks (EraseBlock, MediaId, Lba+1,
, BufferSize - 2 * BlockSize);
-
- // Read the data with 0, the first/last block should not be erased
- ReadStatus = BlockIo->ReadBlocks (BlockIo, MediaId, Lba, BufferSize,
(VOID*)Buffer2);
- if (ReadStatus == EFI_SUCCESS) {
-for (Index1 = 0; Index1 < BlockSize; Index1++) {
- if (Buffer2[Index1] != 0) {
-IsZero1 = FALSE;
-break;
+ if (BufferSize > 2 * BlockSize) {
Magic Number 2.
+Token.Event = NULL;
+Token.TransactionStatus = EFI_NOT_READY;
+
+EraseStatus = EraseBlock->EraseBlocks (EraseBlock, MediaId, Lba+1,
, BufferSize - 2 * BlockSize);
Magic number 2.
+
+// Read the data with 0, the first/last block should not be erased
+ReadStatus = BlockIo->ReadBlocks (BlockIo, MediaId, Lba, BufferSize,
(VOID*)Buffer2);
+if (ReadStatus == EFI_SUCCESS) {
+ for (Index1 = 0; Index1 < BlockSize; Index1++) {
+if (Buffer2[Index1] != 0) {
+ IsZero1 = FALSE;
+ break;
+}
}
-}
-for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
- if (Buffer2[Index1] != 0) {
-IsZero2 = FALSE;
-break;
+ for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
+if (Buffer2[Index1] != 0) {
+ IsZero2 = FALSE;
+ break;
+}
}
-}
-for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) {
- if (Buffer2[Index1] != 0) {
-IsZero3 = FALSE;
-break;
+ for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++)
{
+if (Buffer2[Index1] != 0) {
+ IsZero3 = FALSE;
+ break;
+}
}
-}
-if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 == TRUE) && ((IsZero3 == FALSE)))
- AssertionType = EFI_TEST_ASSERTION_PASSED;
-else
- AssertionType = EFI_TEST_ASSERTION_FAILED;
+ if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 == TRUE)
&& ((IsZero3 == FALSE)))
Less than 80 cols.
+ AssertionType = EFI_TEST_ASSERTION_PASSED;
+ else
+AssertionType = EFI_TEST_ASSERTION_FAILED;
-StandardLib->RecordAssertion (
- StandardLib,
- AssertionType,
- gEraseBlockBBTestFunctionAssertionGuid003,
- L"EraseBlocks - EraseBlocks for testing, the first/last
block should not be erased",
- L"%a:%d:EraseBlocks Status - %r, IsZero1 - %d, IsZero2 - %d,
IsZero3 - %d",
- __FILE__,
- (UINTN)__LINE__,
- Status,
- IsZero1, IsZero2, IsZero3
- );
+ StandardLib->RecordAssertion (
+ StandardLib,
+ AssertionType,
+ gEraseBlockBBTestFunctionAssertionGuid003,
+ L"EraseBlocks - EraseBlocks for testing, the first/last
block should not be erased",
+ L"%a:%d:EraseBlocks Status - %r, IsZero1 - %d, IsZero2 -
%d, IsZero3 - %d",
+ __FILE__,
+ (UINTN)__LINE__,
+ EraseStatus,
+ IsZero1, IsZero2, IsZero3
+ );
+}
}
-
//
// Erase Blocks with the EraseLengthGranularity blocks
//
@@ -453,13 +455,13 @@ BlockIo2:
//
// Erase Blocks with non EraseLengthGranularity blocks
//
+ if (BufferSize > 2 * BlockSize) {
Magic Number 2.
+Token.Event
Re: [edk2] [PATCH] uefi-sct/SctPkg:Fix the flaw in BBTestCreateEventEx_Func_Sub3 on certain situation. Besides AllocatePages(), CreateEventEx may cause the memorymap change itself. Enhance the test to
On 10/15/2018 02:33 AM, Supreeth Venkatesh wrote:
Please use a commit message less than 80 Cols.
On 10/13/2018 04:42 PM, Eric Jin wrote:
Cc: Supreeth Venkatesh
Cc: Jiaxin Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin
---
...rTaskPriorityServicesBBTestCreateEventEx.c | 26 +++
.../BlackBoxTest/Support.c | 19 +-
2 files changed, 33 insertions(+), 12 deletions(-)
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
index e2e173ab..25d1ed97 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2016 Unified EFI, Inc.
- Copyright (c) 2010 - 2016, Intel Corporation. All rights
reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights
reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of
the BSD License
@@ -192,6 +192,10 @@ BBTestCreateEventEx_Func (
BBTestCreateEventEx_Func_Sub2 (StandardLib);
#endif
+ //
+ // The test for the EFI_EVENT_GROUP_MEMORY_MAP_CHANGE
+ // This event group is notified by the system when the memory map
has changed.
+ //
BBTestCreateEventEx_Func_Sub3 (StandardLib);
//
@@ -599,12 +603,12 @@ BBTestCreateEventEx_Func_Sub1 (
UINTN Buffer[MAX_TEST_EVENT_NUM +
MAX_TEST_EVENT_NUM*2];
//
- // Initialize Buffer
+ // Initialize Buffer and the 0xAA is only for the Sub3 test
//
for (Index = 0; Index < MAX_TEST_EVENT_NUM; Index ++) {
Strange Logic here. Needs re-look.
Buffer[Index] = Index;
- Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(-1);
- Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(-1);
+ Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(0xAA);
Magic Number 0xAA
+ Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(0xAA);
Magic Number 0xAA. Please define Macro or const.
}
//
@@ -755,12 +759,12 @@ BBTestCreateEventEx_Func_Sub2 (
UINTN Buffer[MAX_TEST_EVENT_NUM +
MAX_TEST_EVENT_NUM*2];
//
- // Initialize Buffer
+ // Initialize Buffer and the 0xAA is only for the Sub3 test
//
for (Index = 0; Index < MAX_TEST_EVENT_NUM; Index ++) {
Buffer[Index] = Index;
- Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(-1);
- Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(-1);
+ Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(0xAA);
Magic Number 0xAA. Please define Macro or const.
+ Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(0xAA);
Magic Number 0xAA. Please define Macro or const.
}
//
@@ -914,12 +918,12 @@ BBTestCreateEventEx_Func_Sub3 (
UINTN Buffer[MAX_TEST_EVENT_NUM +
MAX_TEST_EVENT_NUM*2];
//
- // Initialize Buffer
+ // Initialize Buffer and the trick to initial it as 0xAA
//
for (Index = 0; Index < MAX_TEST_EVENT_NUM; Index ++) {
Buffer[Index] = Index;
- Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(-1);
- Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(-1);
+ Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(0xAA);
Strange Logic here. Needs re-look. Also, Magic Number AA.
+ Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(0xAA);
Strange Logic here. Needs re-look. Also, Magic Number AA.
}
//
@@ -974,7 +978,7 @@ BBTestCreateEventEx_Func_Sub3 (
}
//
- // Install a configuration table at TPL_NOTIFY
+ // Call AllocatePage to change the memorymap
//
OldTpl = gtBS->RaiseTPL (TPL_NOTIFY);
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c
index aa02383e..823e16ab 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2010 Unified EFI, Inc.
- Copyright (c) 2010, Intel Corporation. All rights reserved.
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights
reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of
the BSD License
@@ -64,6 +64,23 @@ NotifyFunctionTplEx(
Re: [edk2] [PATCH] ShellPkg/dmem: Only dump sizeof (EFI_SYSTEM_TABLE) bytes for gST
On 10/11/2018 9:05 PM, jim.dai...@dell.com wrote:
Is the line:
Size = gST->Hdr.HeaderSize;
possibly a better way of handling this? Either way:
Reviewed-by: Jim Dailey
Thanks! I will stick to use sizeof().
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Thursday, October 11, 2018 2:53 AM
To: edk2-devel@lists.01.org
Cc: Jaben Carsey
Subject: [edk2] [PATCH] ShellPkg/dmem: Only dump sizeof (EFI_SYSTEM_TABLE)
bytes for gST
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1236
When "dmem" runs without additional arguments, it dumps the memory
content of EFI_SYSTEM_TABLE. But today's implementation dumps 512
bytes. It's not correct because sizeof (EFI_SYSTEM_TABLE) is less
than 512, the 512-read causes page fault exception in a heap-guard
enabled environment.
The patch changes the implementation to only dump
sizeof (EFI_SYSTEM_TABLE) bytes for gST.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni
Cc: Jaben Carsey
---
ShellPkg/Library/UefiShellDebug1CommandsLib/Dmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dmem.c
b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dmem.c
index f38593a9e9..a4c18c9b68 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dmem.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dmem.c
@@ -149,7 +149,7 @@ ShellCommandRunDmem (
Temp1 = ShellCommandLineGetRawValue(Package, 1);
if (Temp1 == NULL) {
Address = gST;
-Size = 512;
+Size= sizeof (*gST);
} else {
if (!ShellIsHexOrDecimalNumber(Temp1, TRUE, FALSE) ||
EFI_ERROR(ShellConvertStringToUint64(Temp1, (UINT64*), TRUE, FALSE))) {
ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV),
gShellDebug1HiiHandle, L"dmem", Temp1);
--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] BaseTools/ECC: Fix an identification issue of typedef function.
From: Hess Chen
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hess Chen
---
BaseTools/Source/Python/Ecc/Check.py | 12 +++-
BaseTools/Source/Python/Ecc/c.py | 8 ++--
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/BaseTools/Source/Python/Ecc/Check.py
b/BaseTools/Source/Python/Ecc/Check.py
index 3bf86b42cd..eb086362bd 100644
--- a/BaseTools/Source/Python/Ecc/Check.py
+++ b/BaseTools/Source/Python/Ecc/Check.py
@@ -586,13 +586,23 @@ class Check(object):
if EccGlobalData.gConfig.IncludeFileCheckData == '1' or
EccGlobalData.gConfig.IncludeFileCheckAll == '1' or
EccGlobalData.gConfig.CheckAll == '1':
EdkLogger.quiet("Checking header file data ...")
+# Get all typedef functions
+gAllTypedefFun = []
+for IdentifierTable in EccGlobalData.gIdentifierTableList:
+SqlCommand = """select Name from %s
+where Model = %s """ % (IdentifierTable,
MODEL_IDENTIFIER_TYPEDEF)
+RecordSet = EccGlobalData.gDb.TblFile.Exec(SqlCommand)
+for Record in RecordSet:
+if Record[0].startswith('('):
+gAllTypedefFun.append(Record[0])
+
#for Dirpath, Dirnames, Filenames in self.WalkTree():
#for F in Filenames:
#if os.path.splitext(F)[1] in ('.h'):
#FullName = os.path.join(Dirpath, F)
#MsgList = c.CheckHeaderFileData(FullName)
for FullName in EccGlobalData.gHFileList:
-MsgList = c.CheckHeaderFileData(FullName)
+MsgList = c.CheckHeaderFileData(FullName, gAllTypedefFun)
# Doxygen document checking
def DoxygenCheck(self):
diff --git a/BaseTools/Source/Python/Ecc/c.py b/BaseTools/Source/Python/Ecc/c.py
index 953f1630b6..b8d6adde16 100644
--- a/BaseTools/Source/Python/Ecc/c.py
+++ b/BaseTools/Source/Python/Ecc/c.py
@@ -2144,7 +2144,7 @@ def CheckBooleanValueComparison(FullFileName):
PrintErrorMsg(ERROR_PREDICATE_EXPRESSION_CHECK_BOOLEAN_VALUE, 'Predicate
Expression: %s' % Exp, FileTable, Str[2])
-def CheckHeaderFileData(FullFileName):
+def CheckHeaderFileData(FullFileName, AllTypedefFun=[]):
ErrorMsgList = []
FileID = GetTableID(FullFileName, ErrorMsgList)
@@ -2160,7 +2160,11 @@ def CheckHeaderFileData(FullFileName):
ResultSet = Db.TblFile.Exec(SqlStatement)
for Result in ResultSet:
if not Result[1].startswith('extern'):
-PrintErrorMsg(ERROR_INCLUDE_FILE_CHECK_DATA, 'Variable definition
appears in header file', FileTable, Result[0])
+for Item in AllTypedefFun:
+if '(%s)' % Result[1] in Item:
+break
+else:
+PrintErrorMsg(ERROR_INCLUDE_FILE_CHECK_DATA, 'Variable
definition appears in header file', FileTable, Result[0])
SqlStatement = """ select ID
from Function
--
2.14.2.windows.2
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] BaseTools/UPT: Fix an issue of UNI string checking.
From: Hess Chen
The tool now can detect the error that the content between double
quotes contains another double quotes or enter key.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hess Chen
---
.../Source/Python/UPT/Library/UniClassObject.py| 23 ++
1 file changed, 23 insertions(+)
diff --git a/BaseTools/Source/Python/UPT/Library/UniClassObject.py
b/BaseTools/Source/Python/UPT/Library/UniClassObject.py
index 670cf3b4ee..cd575d5a34 100644
--- a/BaseTools/Source/Python/UPT/Library/UniClassObject.py
+++ b/BaseTools/Source/Python/UPT/Library/UniClassObject.py
@@ -566,6 +566,22 @@ class UniFileClassObject(object):
if Line.startswith(u'#language') and len(Line.split()) == 2:
MultiLineFeedExits = True
+#
+# Check the situation that there only has one '"' for the language
entry
+#
+if Line.startswith(u'#string') and Line.find(u'#language') > 0 and
Line.count(u'"') == 1:
+EdkLogger.Error("Unicode File Parser",
ToolError.FORMAT_INVALID,
+ExtraData='''The line %s misses '"' at the end
of it in file %s'''
+% (LineCount, File.Path))
+
+#
+# Check the situation that there has more than 2 '"' for the
language entry
+#
+if Line.startswith(u'#string') and Line.find(u'#language') > 0 and
Line.replace(u'\\"', '').count(u'"') > 2:
+EdkLogger.Error("Unicode File Parser",
ToolError.FORMAT_INVALID,
+ExtraData='''The line %s has more than 2 '"'
for language entry in file %s'''
+% (LineCount, File.Path))
+
#
# Between two String entry, can not contain line feed
#
@@ -727,6 +743,13 @@ class UniFileClassObject(object):
else:
EdkLogger.Error("Unicode File Parser",
ToolError.FORMAT_INVALID, ExtraData=File.Path)
elif Line.startswith(u'"'):
+#
+# Check the situation that there has more than 2 '"' for the
language entry
+#
+if Line.replace(u'\\"', '').count(u'"') > 2:
+EdkLogger.Error("Unicode File Parser",
ToolError.FORMAT_INVALID,
+ExtraData='''The line %s has more than 2
'"' for language entry in file %s'''
+% (LineCount, File.Path))
if u'#string' in Line or u'#language' in Line:
EdkLogger.Error("Unicode File Parser",
ToolError.FORMAT_INVALID, ExtraData=File.Path)
NewLines.append(Line)
--
2.14.2.windows.2
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] BaseTools/Ecc: Update a checkpoint criteria.
From: Hess Chen
Change the criteria of the checkpoint of "#ifndef" to remove the requirement of
prefix '_'.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hess Chen
---
BaseTools/Source/Python/Ecc/Check.py| 2 +-
BaseTools/Source/Python/Ecc/EccToolError.py | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/BaseTools/Source/Python/Ecc/Check.py
b/BaseTools/Source/Python/Ecc/Check.py
index fc86ad96f2..3bf86b42cd 100644
--- a/BaseTools/Source/Python/Ecc/Check.py
+++ b/BaseTools/Source/Python/Ecc/Check.py
@@ -1374,7 +1374,7 @@ class Check(object):
RecordSet = EccGlobalData.gDb.TblFile.Exec(SqlCommand)
for Record in RecordSet:
Name = Record[1].replace('#ifndef', '').strip()
-if Name[0] != '_' or Name[-1] != '_':
+if Name[-1] != '_':
if not
EccGlobalData.gException.IsException(ERROR_NAMING_CONVENTION_CHECK_IFNDEF_STATEMENT,
Name):
EccGlobalData.gDb.TblReport.Insert(ERROR_NAMING_CONVENTION_CHECK_IFNDEF_STATEMENT,
OtherMsg="The #ifndef name [%s] does not follow the rules" % (Name),
BelongsToTable=FileTable, BelongsToItem=Record[0])
diff --git a/BaseTools/Source/Python/Ecc/EccToolError.py
b/BaseTools/Source/Python/Ecc/EccToolError.py
index ae0a31af8a..e327a7888d 100644
--- a/BaseTools/Source/Python/Ecc/EccToolError.py
+++ b/BaseTools/Source/Python/Ecc/EccToolError.py
@@ -167,7 +167,7 @@ gEccErrorMessage = {
ERROR_NAMING_CONVENTION_CHECK_ALL : "",
ERROR_NAMING_CONVENTION_CHECK_DEFINE_STATEMENT : "Only capital letters are
allowed to be used for #define declarations",
ERROR_NAMING_CONVENTION_CHECK_TYPEDEF_STATEMENT : "Only capital letters
are allowed to be used for typedef declarations",
-ERROR_NAMING_CONVENTION_CHECK_IFNDEF_STATEMENT : "The #ifndef at the start
of an include file should use both prefix and postfix underscore characters,
'_'",
+ERROR_NAMING_CONVENTION_CHECK_IFNDEF_STATEMENT : "The #ifndef at the start
of an include file should use a postfix underscore characters, '_'",
ERROR_NAMING_CONVENTION_CHECK_PATH_NAME : """Path name does not follow the
rules: 1. First character should be upper case 2. Must contain lower case
characters 3. No white space characters""",
ERROR_NAMING_CONVENTION_CHECK_VARIABLE_NAME : """Variable name does not
follow the rules: 1. First character should be upper case 2. Must contain lower
case characters 3. No white space characters 4. Global variable name must start
with a 'g'""",
ERROR_NAMING_CONVENTION_CHECK_FUNCTION_NAME : """Function name does not
follow the rules: 1. First character should be upper case 2. Must contain lower
case characters 3. No white space characters""",
--
2.14.2.windows.2
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 5/6] MdeModulePkg/UdfDxe: Handle dead codes in File.c
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1249
We found potential dead codes within File.c during the code coverage test.
After manual review, we think the below ones are positive reports:
A. In function UdfRead():
else if (IS_FID_DELETED_FILE (Parent->FileIdentifierDesc)) {
Status = EFI_DEVICE_ERROR;
}
A File Identifier Descriptor will be get from the UDF media only by
function ReadDirectoryEntry(). And within this function, all the File
Identifier Descriptor with 'DELETED_FILE' characteristics will be skipped
and will not be returned. Hence, the above codes in function UdfRead()
will never be hit.
This commit will add "ASSERT (FALSE);" before the above line to indicate
this.
Cc: Ruiyu Ni
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu
Reviewed-by: Paulo Alcantara
Reviewed-by: Star Zeng
---
MdeModulePkg/Universal/Disk/UdfDxe/File.c | 4
1 file changed, 4 insertions(+)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 0730e6a3fa..eb7d79692b 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -500,6 +500,10 @@ UdfRead (
PrivFileData->FilePosition++;
Status = EFI_SUCCESS;
} else if (IS_FID_DELETED_FILE (Parent->FileIdentifierDesc)) {
+//
+// Code should never reach here.
+//
+ASSERT (FALSE);
Status = EFI_DEVICE_ERROR;
}
--
2.12.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 6/6] MdeModulePkg/UdfDxe: Handle dead codes in FileSystemOperations.c
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1249 We found potential dead codes within File.c during the code coverage test. After manual review, we think the below ones are positive reports: A. For function GetAllocationDescriptor(): Due to the all the calling places for this function, the input parameter 'RecordingFlags' can only with value 'LongAdsSequence' or 'ShortAdsSequence'. Moreover, this is also mentioned in the function description comments for GetAllocationDescriptor(). So the below code will never be reached: return EFI_DEVICE_ERROR; This commit will add "ASSERT (FALSE);" before the above line to indicate this and thus matching the function description comments. B. For function GetAllocationDescriptorLsn(): Due to the all the calling places for this function, the input parameter 'RecordingFlags' can only with value 'LongAdsSequence' or 'ShortAdsSequence'. Moreover, this is also mentioned in the function description comments for GetAllocationDescriptorLsn(). So the below code will never be reached: return EFI_UNSUPPORTED; This commit will add "ASSERT (FALSE);" before the above line to indicate this and thus matching the function description comments. Cc: Paulo Alcantara Cc: Paulo Alcantara Cc: Ruiyu Ni Cc: Star Zeng Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu --- MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 8 1 file changed, 8 insertions(+) diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c index 1400846bf1..9048a18f64 100644 --- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c +++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c @@ -738,6 +738,10 @@ GetAllocationDescriptor ( ); } + // + // Code should never reach here. + // + ASSERT (FALSE); return EFI_DEVICE_ERROR; } @@ -784,6 +788,10 @@ GetAllocationDescriptorLsn ( return EFI_SUCCESS; } + // + // Code should never reach here. + // + ASSERT (FALSE); return EFI_UNSUPPORTED; } -- 2.12.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 4/6] MdeModulePkg/UdfDxe: Use debug msg instead of ASSERT in UdfOpen()
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1248
Within function UdfOpen():
This commit will use debug messages instead of using ASSERT when an error
occurs after calling GetFileSize().
Cc: Ruiyu Ni
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu
Reviewed-by: Paulo Alcantara
Reviewed-by: Star Zeng
---
MdeModulePkg/Universal/Disk/UdfDxe/File.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 2249f4ea0e..0730e6a3fa 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -257,8 +257,12 @@ UdfOpen (
>File,
>FileSize
);
- ASSERT_EFI_ERROR (Status);
if (EFI_ERROR (Status)) {
+DEBUG ((
+ DEBUG_ERROR,
+ "%a: GetFileSize() fails with status - %r.\n",
+ __FUNCTION__, Status
+ ));
goto Error_Get_File_Size;
}
--
2.12.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 1/6] MdeModulePkg/UdfDxe: Use error handling for memory allocation failure
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1247
For functions DuplicateFid() and DuplicateFe(), this commit will use error
handling logic instead of ASSERTs for memory allocation failure.
Cc: Ruiyu Ni
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu
Reviewed-by: Paulo Alcantara
Reviewed-by: Star Zeng
---
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 40
+---
1 file changed, 34 insertions(+), 6 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index ecc172303e..638f31bd82 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -468,8 +468,6 @@ DuplicateFid (
*NewFileIdentifierDesc =
(UDF_FILE_IDENTIFIER_DESCRIPTOR *)AllocateCopyPool (
(UINTN) GetFidDescriptorLength (FileIdentifierDesc), FileIdentifierDesc);
-
- ASSERT (*NewFileIdentifierDesc != NULL);
}
/**
@@ -490,8 +488,6 @@ DuplicateFe (
)
{
*NewFileEntry = AllocateCopyPool (Volume->FileEntrySize, FileEntry);
-
- ASSERT (*NewFileEntry != NULL);
}
/**
@@ -1370,7 +1366,15 @@ InternalFindFile (
}
DuplicateFe (BlockIo, Volume, Parent->FileEntry, >FileEntry);
+if (File->FileEntry == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+}
+
DuplicateFid (Parent->FileIdentifierDesc, >FileIdentifierDesc);
+if (File->FileIdentifierDesc == NULL) {
+ FreePool (File->FileEntry);
+ return EFI_OUT_OF_RESOURCES;
+}
return EFI_SUCCESS;
}
@@ -1732,9 +1736,20 @@ FindFile (
// We've already a file pointer (Root) for the root directory.
Duplicate
// its FE/EFE and FID descriptors.
//
-DuplicateFe (BlockIo, Volume, Root->FileEntry, >FileEntry);
-DuplicateFid (Root->FileIdentifierDesc, >FileIdentifierDesc);
Status = EFI_SUCCESS;
+DuplicateFe (BlockIo, Volume, Root->FileEntry, >FileEntry);
+if (File->FileEntry == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+} else {
+ //
+ // File->FileEntry is not NULL.
+ //
+ DuplicateFid (Root->FileIdentifierDesc, >FileIdentifierDesc);
+ if (File->FileIdentifierDesc == NULL) {
+FreePool (File->FileEntry);
+Status = EFI_OUT_OF_RESOURCES;
+ }
+}
}
} else {
//
@@ -1874,6 +1889,9 @@ ReadDirectoryEntry (
} while (FileIdentifierDesc->FileCharacteristics & DELETED_FILE);
DuplicateFid (FileIdentifierDesc, FoundFid);
+ if (*FoundFid == NULL) {
+return EFI_OUT_OF_RESOURCES;
+ }
return EFI_SUCCESS;
}
@@ -2031,8 +2049,18 @@ ResolveSymlink (
// "." (current file). Duplicate both FE/EFE and FID of this file.
//
DuplicateFe (BlockIo, Volume, PreviousFile.FileEntry, >FileEntry);
+ if (File->FileEntry == NULL) {
+Status = EFI_OUT_OF_RESOURCES;
+goto Error_Find_File;
+ }
+
DuplicateFid (PreviousFile.FileIdentifierDesc,
>FileIdentifierDesc);
+ if (File->FileIdentifierDesc == NULL) {
+FreePool (File->FileEntry);
+Status = EFI_OUT_OF_RESOURCES;
+goto Error_Find_File;
+ }
goto Next_Path_Component;
case 5:
//
--
2.12.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 3/6] MdeModulePkg/UdfDxe: Use error handling when fail to return LSN
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1248
This commit will refine the ASSERTs in function GetLongAdLsn() and
GetAllocationDescriptorLsn() when the logical sector number cannot be
returned due to unrecognized media format.
Error handling logic will be used for those ASSERTs.
Cc: Ruiyu Ni
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu
Reviewed-by: Paulo Alcantara
Reviewed-by: Star Zeng
---
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 101
+---
1 file changed, 69 insertions(+), 32 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 8b58cc9eb1..1400846bf1 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -271,26 +271,39 @@ GetPdFromLongAd (
/**
Return logical sector number of a given Long Allocation Descriptor.
- @param[in] Volume Volume information pointer.
- @param[in] LongAd Long Allocation Descriptor pointer.
+ @param[in] Volume Volume information pointer.
+ @param[in] LongAd Long Allocation Descriptor pointer.
+ @param[out] LsnLogical sector number pointer.
- @return The logical sector number of a given Long Allocation Descriptor.
+ @retval EFI_SUCCESS Logical sector number successfully returned.
+ @retval EFI_UNSUPPORTED Logical sector number is not returned due to
+ unrecognized format.
**/
-UINT64
+EFI_STATUS
GetLongAdLsn (
- IN UDF_VOLUME_INFO *Volume,
- IN UDF_LONG_ALLOCATION_DESCRIPTOR *LongAd
+ IN UDF_VOLUME_INFO *Volume,
+ IN UDF_LONG_ALLOCATION_DESCRIPTOR *LongAd,
+ OUT UINT64 *Lsn
)
{
UDF_PARTITION_DESCRIPTOR *PartitionDesc;
PartitionDesc = GetPdFromLongAd (Volume, LongAd);
- ASSERT (PartitionDesc != NULL);
+ if (PartitionDesc == NULL) {
+DEBUG ((
+ DEBUG_ERROR,
+ "%a: Fail to get the Partition Descriptor from the given Long Allocation
Descriptor.\n",
+ __FUNCTION__
+ ));
+return EFI_UNSUPPORTED;
+ }
- return (UINT64)PartitionDesc->PartitionStartingLocation -
-Volume->MainVdsStartLocation +
-LongAd->ExtentLocation.LogicalBlockNumber;
+ *Lsn = (UINT64)PartitionDesc->PartitionStartingLocation -
+ Volume->MainVdsStartLocation +
+ LongAd->ExtentLocation.LogicalBlockNumber;
+
+ return EFI_SUCCESS;
}
/**
@@ -342,7 +355,10 @@ FindFileSetDescriptor (
UDF_DESCRIPTOR_TAG *DescriptorTag;
LogicalVolDesc = >LogicalVolDesc;
- Lsn = GetLongAdLsn (Volume, >LogicalVolumeContentsUse);
+ Status = GetLongAdLsn (Volume, >LogicalVolumeContentsUse,
);
+ if (EFI_ERROR (Status)) {
+return Status;
+ }
//
// As per UDF 2.60 specification:
@@ -732,34 +748,43 @@ GetAllocationDescriptor (
@param[in] Volume Volume information pointer.
@param[in] ParentIcb Long Allocation Descriptor pointer.
@param[in] Ad Allocation Descriptor pointer.
+ @param[out] Lsn Logical sector number pointer.
- @return The logical sector number of the given Allocation Descriptor.
+ @retval EFI_SUCCESS Logical sector number of the given Allocation
+ Descriptor successfully returned.
+ @retval EFI_UNSUPPORTED Logical sector number of the given Allocation
+ Descriptor is not returned due to
unrecognized
+ format.
**/
-UINT64
+EFI_STATUS
GetAllocationDescriptorLsn (
- IN UDF_FE_RECORDING_FLAGS RecordingFlags,
- IN UDF_VOLUME_INFO *Volume,
- IN UDF_LONG_ALLOCATION_DESCRIPTOR *ParentIcb,
- IN VOID*Ad
+ IN UDF_FE_RECORDING_FLAGS RecordingFlags,
+ IN UDF_VOLUME_INFO *Volume,
+ IN UDF_LONG_ALLOCATION_DESCRIPTOR *ParentIcb,
+ IN VOID*Ad,
+ OUT UINT64 *Lsn
)
{
UDF_PARTITION_DESCRIPTOR *PartitionDesc;
if (RecordingFlags == LongAdsSequence) {
-return GetLongAdLsn (Volume, (UDF_LONG_ALLOCATION_DESCRIPTOR *)Ad);
+return GetLongAdLsn (Volume, (UDF_LONG_ALLOCATION_DESCRIPTOR *)Ad, Lsn);
} else if (RecordingFlags == ShortAdsSequence) {
PartitionDesc = GetPdFromLongAd (Volume, ParentIcb);
-ASSERT (PartitionDesc != NULL);
+if (PartitionDesc == NULL) {
+ return EFI_UNSUPPORTED;
+}
-return GetShortAdLsn (
- Volume,
- PartitionDesc,
- (UDF_SHORT_ALLOCATION_DESCRIPTOR *)Ad
- );
+*Lsn = GetShortAdLsn (
+ Volume,
+ PartitionDesc,
+ (UDF_SHORT_ALLOCATION_DESCRIPTOR *)Ad
+ );
+return EFI_SUCCESS;
}
- return 0;
+ return
[edk2] [PATCH v2 2/6] MdeModulePkg/UdfDxe: ASSERT for false positives of NULL ptr deref
This commit adds ASSERTs to address false positive reports of NULL
pointer dereference issues raised from static analysis with regard to
function ReadDirectoryEntry().
Cc: Ruiyu Ni
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu
Reviewed-by: Paulo Alcantara
Reviewed-by: Star Zeng
---
MdeModulePkg/Universal/Disk/UdfDxe/File.c | 9 +
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 9 +
2 files changed, 18 insertions(+)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 6f07bf2066..2249f4ea0e 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -408,6 +408,15 @@ UdfRead (
goto Done;
}
+ //
+ // After calling function ReadDirectoryEntry(), if
'NewFileIdentifierDesc'
+ // is NULL, then the 'Status' must be EFI_OUT_OF_RESOURCES. Hence, if the
+ // code reaches here, 'NewFileIdentifierDesc' must be not NULL.
+ //
+ // The ASSERT here is for addressing a false positive NULL pointer
+ // dereference issue raised from static analysis.
+ //
+ ASSERT (NewFileIdentifierDesc != NULL);
if (!IS_FID_PARENT_FILE (NewFileIdentifierDesc)) {
break;
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 638f31bd82..8b58cc9eb1 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -1404,6 +1404,15 @@ InternalFindFile (
break;
}
+//
+// After calling function ReadDirectoryEntry(), if 'FileIdentifierDesc' is
+// NULL, then the 'Status' must be EFI_OUT_OF_RESOURCES. Hence, if the code
+// reaches here, 'FileIdentifierDesc' must be not NULL.
+//
+// The ASSERT here is for addressing a false positive NULL pointer
+// dereference issue raised from static analysis.
+//
+ASSERT (FileIdentifierDesc != NULL);
if (FileIdentifierDesc->FileCharacteristics & PARENT_FILE) {
//
--
2.12.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 0/6] Code refinements in UdfDxe
Note: Since PATCH v2 1/6 ~ 5/6 have not been changed, add the 'Reviewed-by:' tag during the v1 series review. V2 changes: A. Drop PATCH v1 6/7, since removing those codes will make the function MangleFileName() not matching its original implementation purpose (according to the function description). B. Drop change C in PATCH v1 7/7. It is reasonable for function SetFileInfo() to handle the expected value for the input parameters. C. Refine the log message for PATCH v1 7/7. V1 history: This series will refine the codes in MdeModulePkg/Universal/Disk/UdfDxe for: A. Refine asserts used for memory allocation failure and error cases that are possible to happen. Will use error handling logic for them; B. Address some dead codes within this module. Cc: Paulo Alcantara Cc: Paulo Alcantara Cc: Ruiyu Ni Cc: Star Zeng Cc: Jiewen Yao Hao Wu (6): MdeModulePkg/UdfDxe: Use error handling for memory allocation failure MdeModulePkg/UdfDxe: ASSERT for false positives of NULL ptr deref MdeModulePkg/UdfDxe: Use error handling when fail to return LSN MdeModulePkg/UdfDxe: Use debug msg instead of ASSERT in UdfOpen() MdeModulePkg/UdfDxe: Handle dead codes in File.c MdeModulePkg/UdfDxe: Handle dead codes in FileSystemOperations.c MdeModulePkg/Universal/Disk/UdfDxe/File.c | 19 ++- MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 158 +++- 2 files changed, 138 insertions(+), 39 deletions(-) -- 2.12.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v1 6/7] MdeModulePkg/UdfDxe: Remove dead codes in FileName.c
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Zeng, Star
> Sent: Tuesday, October 16, 2018 3:46 PM
> To: Wu, Hao A; Leif Lindholm
> Cc: Ni, Ruiyu; edk2-devel@lists.01.org; Zeng, Star
> Subject: Re: [edk2] [PATCH v1 6/7] MdeModulePkg/UdfDxe: Remove dead
> codes in FileName.c
>
> Hao,
>
> If these code removing will make the function to be not matched with its
> original implementation purpose according to the function description,
> and the description is not updated, the code's readability will be
> sacrificed. I prefer to keep the code's readability and follow the
> function's original implementation purpose.
>
OK, I will drop this patch in the next series.
Best Regards,
Hao Wu
>
> Thanks,
> Star
>
> On 2018/10/16 14:28, Wu, Hao A wrote:
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> >> Leif Lindholm
> >> Sent: Tuesday, October 16, 2018 2:20 PM
> >> To: Wu, Hao A
> >> Cc: Ni, Ruiyu; edk2-devel@lists.01.org; Zeng, Star
> >> Subject: Re: [edk2] [PATCH v1 6/7] MdeModulePkg/UdfDxe: Remove
> dead
> >> codes in FileName.c
> >>
> >> On Mon, Oct 15, 2018 at 12:55:21PM +0800, Hao Wu wrote:
> >>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1249
> >>>
> >>> We found potential dead codes within File.c during the code coverage
> test.
> >>>
> >>> After manual review, we think the below ones are positive reports:
> >>>
> >>> A. In function MangleFileName():
> >>>
> >>>FileName = TrimString (FileName);
> >>>// Begin of dead codes
> >>>if (*FileName == L'\0') {
> >>> goto Exit;
> >>>}
> >>>// End of dead codes
> >>>
> >>> When the code reaches the TrimString() call, the string in 'FileName' is
> >>> guaranteed to have a '\' character due to the call patterns of
> >>
> >> What in the call pattern guarantees this? Please be precise.
> >
> > OK, I will update the log message.
> >
> >>
> >>> MangleFileName(). So after trimming the lead-off/tailing white spaces,
> >>> string in 'FileName' will not be an empty string.
> >>>
> >>> B. In function MangleFileName():
> >>>
> >>>if (FileName[0] == L'.') {
> >>> if (FileName[1] == L'.') {
> >>>if (FileName[2] == L'\0') {
> >>> goto Exit;
> >>>} else {
> >>> FileName += 2;
> >>>}
> >>> } else if (FileName[1] == L'\0') {
> >>>goto Exit;
> >>> }
> >>>}
> >>>
> >>> When the code hits the above checks, string in 'FileName' will always
> have
> >>> a leading '\' character (denoting an absolute path) due to the call
> >>> patterns of MangleFileName(). So no leading '.' can be there in string
> >>> 'FileName'.
> >>
> >> What in the call pattern guarantees this? Please be precise.
> >
> > OK, I will update the log message.
> >
> > Thanks for the comments.
> >
> > Best Regards,
> > Hao Wu
> >
> >>
> >> Regards,
> >>
> >> Leif
> >> ___
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v1 7/7] MdeModulePkg/UdfDxe: Handle dead codes in FileSystemOperations.c
On 2018/10/16 14:59, Zeng, Star wrote:
On 2018/10/15 12:55, Hao Wu wrote:
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1249
We found potential dead codes within File.c during the code coverage
test.
After manual review, we think the below ones are positive reports:
A. For function GetAllocationDescriptor():
Due to the all the calling places for this function, the input parameter
'RecordingFlags' can only with value 'LongAdsSequence' or
'ShortAdsSequence'.
So the below code will never be reached:
return EFI_DEVICE_ERROR;
This commit will add "ASSERT (FALSE);" before the above line to indicate
this.
B. For function GetAllocationDescriptorLsn():
Due to the all the calling places for this function, the input parameter
'RecordingFlags' can only with value 'LongAdsSequence' or
'ShortAdsSequence'.
So the below code will never be reached:
return EFI_UNSUPPORTED;
This commit will add "ASSERT (FALSE);" before the above line to indicate
this.
C. For function SetFileInfo():
Due to the all the calling places for this function, the input parameter
'FileName' will never be a NULL pointer.
So the below codes will never be reached:
} else {
FileInfo->FileName[0] = '\0';
}
This commit will add "ASSERT (FALSE);" before the above lines to indicate
this.
Hao,
Thanks for the patch.
I think we should see what is the expected value for the parameter, but
not see how caller uses the parameter.
From this point of view, I think the C case is valid and may be no need
to change.
More information about the C case. There are two places in the function
to handle FileName == NULL, but this patch only updates one place. If
the patch wants to forbid the function to accept FileName == NULL, it
should update those two places and update function description at the
same time. Otherwise keep the function no change.
Thanks,
Star
Thanks,
Star
Cc: Paulo Alcantara
Cc: Ruiyu Ni
Cc: Star Zeng
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu
---
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 12
1 file changed, 12 insertions(+)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 1400846bf1..19acd0554c 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -738,6 +738,10 @@ GetAllocationDescriptor (
);
}
+ //
+ // Code should never reach here.
+ //
+ ASSERT (FALSE);
return EFI_DEVICE_ERROR;
}
@@ -784,6 +788,10 @@ GetAllocationDescriptorLsn (
return EFI_SUCCESS;
}
+ //
+ // Code should never reach here.
+ //
+ ASSERT (FALSE);
return EFI_UNSUPPORTED;
}
@@ -2413,6 +2421,10 @@ SetFileInfo (
if (FileName != NULL) {
StrCpyS (FileInfo->FileName, StrLen (FileName) + 1, FileName);
} else {
+ //
+ // Code should never reach here.
+ //
+ ASSERT (FALSE);
FileInfo->FileName[0] = '\0';
}
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] FatBinPkg: Remove FatBinPkg and modify document
On 10/16/2018 3:11 PM, shenglei wrote: Remove FatBinPkg and modify Maintainers.txt. https://bugzilla.tianocore.org/show_bug.cgi?id=1105 Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Shenglei Zhang --- FatBinPkg/EnhancedFatDxe/AArch64/Fat.efi | Bin 26752 -> 0 bytes FatBinPkg/EnhancedFatDxe/Arm/Fat.efi | Bin 17152 -> 0 bytes FatBinPkg/EnhancedFatDxe/Ebc/Fat.efi | Bin 79136 -> 0 bytes FatBinPkg/EnhancedFatDxe/Fat.inf | 48 --- FatBinPkg/EnhancedFatDxe/Ia32/Fat.efi| Bin 21088 -> 0 bytes FatBinPkg/EnhancedFatDxe/Ipf/Fat.efi | Bin 154400 -> 0 bytes FatBinPkg/EnhancedFatDxe/X64/Fat.efi | Bin 28576 -> 0 bytes FatBinPkg/FatBinPkg.dec | 20 -- FatBinPkg/ReadMe.txt | 9 - Maintainers.txt | 2 +- 10 files changed, 1 insertion(+), 78 deletions(-) delete mode 100644 FatBinPkg/EnhancedFatDxe/AArch64/Fat.efi delete mode 100755 FatBinPkg/EnhancedFatDxe/Arm/Fat.efi delete mode 100644 FatBinPkg/EnhancedFatDxe/Ebc/Fat.efi delete mode 100644 FatBinPkg/EnhancedFatDxe/Fat.inf delete mode 100644 FatBinPkg/EnhancedFatDxe/Ia32/Fat.efi delete mode 100644 FatBinPkg/EnhancedFatDxe/Ipf/Fat.efi delete mode 100644 FatBinPkg/EnhancedFatDxe/X64/Fat.efi delete mode 100644 FatBinPkg/FatBinPkg.dec delete mode 100644 FatBinPkg/ReadMe.txt Reviewed-by: Ruiyu Ni -- Thanks, Ray ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v1 6/7] MdeModulePkg/UdfDxe: Remove dead codes in FileName.c
Hao,
If these code removing will make the function to be not matched with its
original implementation purpose according to the function description,
and the description is not updated, the code's readability will be
sacrificed. I prefer to keep the code's readability and follow the
function's original implementation purpose.
Thanks,
Star
On 2018/10/16 14:28, Wu, Hao A wrote:
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
Leif Lindholm
Sent: Tuesday, October 16, 2018 2:20 PM
To: Wu, Hao A
Cc: Ni, Ruiyu; edk2-devel@lists.01.org; Zeng, Star
Subject: Re: [edk2] [PATCH v1 6/7] MdeModulePkg/UdfDxe: Remove dead
codes in FileName.c
On Mon, Oct 15, 2018 at 12:55:21PM +0800, Hao Wu wrote:
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1249
We found potential dead codes within File.c during the code coverage test.
After manual review, we think the below ones are positive reports:
A. In function MangleFileName():
FileName = TrimString (FileName);
// Begin of dead codes
if (*FileName == L'\0') {
goto Exit;
}
// End of dead codes
When the code reaches the TrimString() call, the string in 'FileName' is
guaranteed to have a '\' character due to the call patterns of
What in the call pattern guarantees this? Please be precise.
OK, I will update the log message.
MangleFileName(). So after trimming the lead-off/tailing white spaces,
string in 'FileName' will not be an empty string.
B. In function MangleFileName():
if (FileName[0] == L'.') {
if (FileName[1] == L'.') {
if (FileName[2] == L'\0') {
goto Exit;
} else {
FileName += 2;
}
} else if (FileName[1] == L'\0') {
goto Exit;
}
}
When the code hits the above checks, string in 'FileName' will always have
a leading '\' character (denoting an absolute path) due to the call
patterns of MangleFileName(). So no leading '.' can be there in string
'FileName'.
What in the call pattern guarantees this? Please be precise.
OK, I will update the log message.
Thanks for the comments.
Best Regards,
Hao Wu
Regards,
Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 3/4] UefiCpuPkg/RegisterCpuFeaturesLib: Add logic to support semaphore type.
Hi Ruiyu,
> -Original Message-
> From: Ni, Ruiyu
> Sent: Tuesday, October 16, 2018 11:05 AM
> To: Dong, Eric ; edk2-devel@lists.01.org
> Cc: Laszlo Ersek
> Subject: Re: [Patch 3/4] UefiCpuPkg/RegisterCpuFeaturesLib: Add logic to
> support semaphore type.
>
> On 10/15/2018 10:49 AM, Eric Dong wrote:
> > In a system which has multiple cores, current set register value task costs
> huge times.
> > After investigation, current set MSR task costs most of the times. Current
> logic uses
> > SpinLock to let set MSR task as an single thread task for all cores. Because
> MSR has
> > scope attribute which may cause GP fault if multiple APs set MSR at the
> same time,
> > current logic use an easiest solution (use SpinLock) to avoid this issue,
> > but it
> will
> > cost huge times.
> >
> > In order to fix this performance issue, new solution will set MSRs base on
> their scope
> > attribute. After this, the SpinLock will not needed. Without SpinLock, new
> issue raised
> > which is caused by MSR dependence. For example, MSR A depends on
> MSR B which means MSR A
> > must been set after MSR B has been set. Also MSR B is package scope level
> and MSR A is
> > thread scope level. If system has multiple threads, Thread 1 needs to set
> the thread level
> > MSRs and thread 2 needs to set thread and package level MSRs. Set MSRs
> task for thread 1
> > and thread 2 like below:
> >
> > Thread 1 Thread 2
> > MSR B NY
> > MSR A YY
> >
> > If driver don't control execute MSR order, for thread 1, it will execute MSR
> A first, but
> > at this time, MSR B not been executed yet by thread 2. system may trig
> exception at this
> > time.
> >
> > In order to fix the above issue, driver introduces semaphore logic to
> > control
> the MSR
> > execute sequence. For the above case, a semaphore will be add between
> MSR A and B for
> > all threads. Semaphore has scope info for it. The possible scope value is
> core or package.
> > For each thread, when it meets a semaphore during it set registers, it will
> > 1)
> release
> > semaphore (+1) for each threads in this core or package(based on the
> scope info for this
> > semaphore) 2) acquire semaphore (-1) for all the threads in this core or
> package(based
> > on the scope info for this semaphore). With these two steps, driver can
> control MSR
> > sequence. Sample code logic like below:
> >
> >//
> >// First increase semaphore count by 1 for processors in this package.
> >//
> >for (ProcessorIndex = 0; ProcessorIndex < PackageThreadsCount ;
> ProcessorIndex ++) {
> > LibReleaseSemaphore ((UINT32 *) [PackageOffset +
> ProcessorIndex]);
> >}
> >//
> >// Second, check whether the count has reach the check number.
> >//
> >for (ProcessorIndex = 0; ProcessorIndex < ValidApCount; ProcessorIndex
> ++) {
> > LibWaitForSemaphore ([ApOffset]);
> >}
> >
> > Platform Requirement:
> > 1. This change requires register MSR setting base on MSR scope info. If
> > still
> register MSR
> > for all threads, exception may raised.
> >
> > Known limitation:
> > 1. Current CpuFeatures driver supports DXE instance and PEI instance. But
> semaphore logic
> > requires Aps execute in async mode which is not supported by PEI driver.
> So CpuFeature
> > PEI instance not works after this change. We plan to support async mode
> for PEI in phase
> > 2 for this task.
> >
> > Cc: Ruiyu Ni
> > Cc: Laszlo Ersek
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Dong
> > ---
> > .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 324 --
> -
> > .../DxeRegisterCpuFeaturesLib.c| 71 +++-
> > .../DxeRegisterCpuFeaturesLib.inf | 3 +
> > .../PeiRegisterCpuFeaturesLib.c| 55 ++-
> > .../PeiRegisterCpuFeaturesLib.inf | 1 +
> > .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h | 51 ++-
> > .../RegisterCpuFeaturesLib.c | 452
> > ++---
> > 7 files changed, 840 insertions(+), 117 deletions(-)
> >
> > diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > index ba3fb3250f..f820b4fed7 100644
> > --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > @@ -145,6 +145,20 @@ CpuInitDataInitialize (
> > CPU_FEATURES_INIT_ORDER *InitOrder;
> > CPU_FEATURES_DATA*CpuFeaturesData;
> > LIST_ENTRY *Entry;
> > + UINT32 Core;
> > + UINT32 Package;
> > + UINT32 Thread;
> > + EFI_CPU_PHYSICAL_LOCATION*Location;
> > + UINT32
[edk2] [Patch] MdeModulePkg BrotliDecompressLib: Add the checker to avoid array out of bound
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao
---
MdeModulePkg/Library/BrotliCustomDecompressLib/dec/decode.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/MdeModulePkg/Library/BrotliCustomDecompressLib/dec/decode.c
b/MdeModulePkg/Library/BrotliCustomDecompressLib/dec/decode.c
index fd42b3b..f3b3cb8 100644
--- a/MdeModulePkg/Library/BrotliCustomDecompressLib/dec/decode.c
+++ b/MdeModulePkg/Library/BrotliCustomDecompressLib/dec/decode.c
@@ -858,6 +858,7 @@ static BROTLI_INLINE uint32_t ReadBlockLength(const
HuffmanCode* table,
uint32_t code;
uint32_t nbits;
code = ReadSymbol(table, br);
+ ASSERT (code < BROTLI_NUM_BLOCK_LEN_SYMBOLS);
nbits = kBlockLengthPrefixCode[code].nbits; /* nbits == 2..24 */
return kBlockLengthPrefixCode[code].offset + BrotliReadBits(br, nbits);
}
@@ -910,6 +911,7 @@ static BROTLI_NOINLINE void InverseMoveToFrontTransform(
uint32_t upper_bound = state->mtf_upper_bound;
uint32_t* mtf = >mtf[1]; /* Make mtf[-1] addressable. */
uint8_t* mtf_u8 = (uint8_t*)mtf;
+ uint8_t* mtf_u8t = mtf_u8 - 1;
/* Load endian-aware constant. */
const uint8_t b0123[4] = {0, 1, 2, 3};
uint32_t pattern;
@@ -928,13 +930,13 @@ static BROTLI_NOINLINE void InverseMoveToFrontTransform(
for (i = 0; i < v_len; ++i) {
int index = v[i];
uint8_t value = mtf_u8[index];
-upper_bound |= v[i];
+upper_bound |= (uint32_t) v[i];
v[i] = value;
-mtf_u8[-1] = value;
-do {
+mtf_u8t[0] = value;
+while (index >= 0) {
+ mtf_u8t[index + 1] = mtf_u8t[index];
index--;
- mtf_u8[index + 1] = mtf_u8[index];
-} while (index >= 0);
+}
}
/* Remember amount of elements to be reinitialized. */
state->mtf_upper_bound = upper_bound >> 2;
@@ -1566,6 +1568,7 @@ static BROTLI_INLINE BROTLI_BOOL ReadCommandInternal(
BrotliBitReaderState memento;
if (!safe) {
cmd_code = ReadSymbol(s->htree_command, br);
+ASSERT (cmd_code < BROTLI_NUM_COMMAND_SYMBOLS);
} else {
BrotliBitReaderSaveState(br, );
if (!SafeReadSymbol(s->htree_command, br, _code)) {
--
2.10.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v1 10/10] MdeModulePkg/UdfDxe: Avoid possible use of already-freed data
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1255
For function ReadFile():
If the line
Status = GetAedAdsData (
...
);
is reached multiple times during the 'for' loop, freeing the data pointed
by variable 'Data' may potentially lead to variable 'Ad' referencing the
already-freed data.
After calling function GetAllocationDescriptor(), 'Data' and 'Ad' may
point to the same memory (with some possible offset). Hence, this commit
will move the FreePool() call backwards to ensure the data will no longer
be used.
Cc: Paulo Alcantara
Cc: Ruiyu Ni
Cc: Star Zeng
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu
---
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 7526de79b2..bf73ab4252 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -1044,6 +1044,7 @@ ReadFile (
EFI_STATUS Status;
UINT32 LogicalBlockSize;
VOID*Data;
+ VOID*DataBak;
UINT64 Length;
VOID*Ad;
UINT64 AdOffset;
@@ -1184,12 +1185,7 @@ ReadFile (
// Descriptor and its extents (ADs).
//
if (GET_EXTENT_FLAGS (RecordingFlags, Ad) == ExtentIsNextExtent) {
-if (!DoFreeAed) {
- DoFreeAed = TRUE;
-} else {
- FreePool (Data);
-}
-
+DataBak = Data;
Status = GetAedAdsData (
BlockIo,
DiskIo,
@@ -1200,6 +1196,13 @@ ReadFile (
,
);
+
+if (!DoFreeAed) {
+ DoFreeAed = TRUE;
+} else {
+ FreePool (DataBak);
+}
+
if (EFI_ERROR (Status)) {
goto Error_Get_Aed;
}
--
2.12.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v1 04/10] MdeModulePkg/UdfDxe: Add boundary check for ComponentIdentifier decode
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=828
Within ResolveSymlink():
The boundary check will validate the 'LengthofComponentIdentifier' field
of a Path Component matches the data within the relating (Extended) File
Entry.
Cc: Paulo Alcantara
Cc: Ruiyu Ni
Cc: Jiewen Yao
Cc: Star Zeng
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu
---
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 4
1 file changed, 4 insertions(+)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index d758b798f1..7611d28b5a 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -2136,6 +2136,10 @@ ResolveSymlink (
return EFI_VOLUME_CORRUPTED;
}
+ if ((UINTN)PathComp->ComponentIdentifier + PathCompLength >
(UINTN)EndData) {
+return EFI_VOLUME_CORRUPTED;
+ }
+
Char = FileName;
for (Index = 1; Index < PathCompLength; Index++) {
if (CompressionId == 16) {
--
2.12.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v1 07/10] MdeModulePkg/UdfDxe: Fix a typo within SetFileInfo()
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1253 Within function SetFileInfo(): This commit will fix a typo where 'Minute' should be used instead of 'Second'. Cc: Paulo Alcantara Cc: Ruiyu Ni Cc: Star Zeng Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu --- MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c index 826ffccf81..ac6e0a8ff7 100644 --- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c +++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c @@ -2370,7 +2370,7 @@ SetFileInfo ( FileInfo->CreateTime.Month = FileEntry->AccessTime.Month; FileInfo->CreateTime.Day = FileEntry->AccessTime.Day; FileInfo->CreateTime.Hour= FileEntry->AccessTime.Hour; -FileInfo->CreateTime.Minute = FileEntry->AccessTime.Second; +FileInfo->CreateTime.Minute = FileEntry->AccessTime.Minute; FileInfo->CreateTime.Second = FileEntry->AccessTime.Second; FileInfo->CreateTime.Nanosecond = FileEntry->AccessTime.HundredsOfMicroseconds; -- 2.12.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v1 06/10] MdeModulePkg/UdfDxe: Correct behavior for UdfSetPosition()
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1252
Calling the 'SetPosition' service of the EFI_FILE_PROTOCOL with 'Position'
equals to 0x for a file is to set the current position to
the end of the file. But the current implementation of function
UdfSetPosition() is to set it to the last byte (not EOF).
This commit will resolve this issue.
Cc: Paulo Alcantara
Cc: Ruiyu Ni
Cc: Star Zeng
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu
---
MdeModulePkg/Universal/Disk/UdfDxe/File.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 80db734f86..54c2400243 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -704,7 +704,7 @@ UdfSetPosition (
// set to the EOF.
//
if (Position == 0x) {
- PrivFileData->FilePosition = PrivFileData->FileSize - 1;
+ PrivFileData->FilePosition = PrivFileData->FileSize;
} else {
PrivFileData->FilePosition = Position;
}
--
2.12.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v1 09/10] MdeModulePkg/UdfDxe: Add more check when getting PD from LongAd
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1254
This commit will add an additional check within function GetPdFromLongAd()
when getting a Partition Descriptor from given a Long Allocation
Descriptor.
According to UDF 2.60 Spec, Section 2.2.13:
> The partition reference numbers used are determined by the order of the
> Partition Maps in the LVD.
(Also the picture comes before the above contents)
And a more detailed explanation of the partition reference numbers is at
https://sites.google.com/site/udfintro/ (seems not a formal documentation
though), Section 5.3.6.
Based on the above findings, the 'PartitionReferenceNumber' field in a
Long Allocation Descriptor is used as an index to access the Partition
Maps data within a Logical Volume Descriptor.
Hence, the new check focuses on the validity of this
'PartitionReferenceNumber' field in a Long Allocation Descriptor. Since
the current implementation of UdfDxe driver supports only one partition on
a Logical Volume, so the value of 'PartitionReferenceNumber' should be 0.
Cc: Paulo Alcantara
Cc: Ruiyu Ni
Cc: Star Zeng
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu
---
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 562a7d983c..7526de79b2 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -241,11 +241,16 @@ GetPdFromLongAd (
//
// NOTE: Only one Type 1 (Physical) Partition is supported. It has been
// checked already in Partition driver for existence of a single Type 1
-// Partition map, so we don't have to double check here.
+// Partition map. Hence, the 'PartitionReferenceNumber' field (the index
+// used to access Partition Maps data within the Logical Volume Descriptor)
+// in the Long Allocation Descriptor should be 0 to indicate there is only
+// one partition.
//
-// Partition reference number can also be retrieved from
-// LongAd->ExtentLocation.PartitionReferenceNumber, however the spec says
-// it may be 0, so let's not rely on it.
+if (LongAd->ExtentLocation.PartitionReferenceNumber != 0) {
+ return NULL;
+}
+//
+// Since only one partition, get the first one directly.
//
PartitionNum = *(UINT16 *)((UINTN)>PartitionMaps[4]);
break;
--
2.12.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v1 05/10] MdeModulePkg/UdfDxe: Add boundary check for getting volume (free) size
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=828
Within GetVolumeSize():
The boundary check will validate the 'NumberOfPartitions' field of a
Logical Volume Integrity Descriptor matches the data within the relating
Logical Volume Descriptor.
Cc: Paulo Alcantara
Cc: Ruiyu Ni
Cc: Jiewen Yao
Cc: Star Zeng
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu
---
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 17
-
MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 7 +++
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 7611d28b5a..826ffccf81 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -2450,6 +2450,13 @@ SetFileInfo (
/**
Get volume and free space size information of an UDF volume.
+ @attention This is boundary function that may receive untrusted input.
+ @attention The input is from FileSystem.
+
+ The Logical Volume Descriptor and the Logical Volume Integrity Descriptor are
+ external inputs, so this routine will do basic validation for both
descriptors
+ and report status.
+
@param[in] BlockIoBlockIo interface.
@param[in] DiskIo DiskIo interface.
@param[in] Volume UDF volume information structure.
@@ -2488,7 +2495,8 @@ GetVolumeSize (
ExtentAd = >IntegritySequenceExtent;
- if (ExtentAd->ExtentLength == 0) {
+ if ((ExtentAd->ExtentLength == 0) ||
+ (ExtentAd->ExtentLength < sizeof (UDF_LOGICAL_VOLUME_INTEGRITY))) {
return EFI_VOLUME_CORRUPTED;
}
@@ -2528,6 +2536,13 @@ GetVolumeSize (
goto Out_Free;
}
+ if ((LogicalVolInt->NumberOfPartitions > MAX_UINT32 / sizeof (UINT32) / 2) ||
+ (LogicalVolInt->NumberOfPartitions * sizeof (UINT32) * 2 >
+ ExtentAd->ExtentLength - sizeof (UDF_LOGICAL_VOLUME_INTEGRITY))) {
+Status = EFI_VOLUME_CORRUPTED;
+goto Out_Free;
+ }
+
*VolumeSize = 0;
*FreeSpaceSize = 0;
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
index 85bf5e7733..9c3f21fd05 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
@@ -902,6 +902,13 @@ SetFileInfo (
/**
Get volume and free space size information of an UDF volume.
+ @attention This is boundary function that may receive untrusted input.
+ @attention The input is from FileSystem.
+
+ The Logical Volume Descriptor and the Logical Volume Integrity Descriptor are
+ external inputs, so this routine will do basic validation for both
descriptors
+ and report status.
+
@param[in] BlockIoBlockIo interface.
@param[in] DiskIo DiskIo interface.
@param[in] Volume UDF volume information structure.
--
2.12.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v1 08/10] MdeModulePkg/UdfDxe: Update GetInfo() for FS VolumeLabel info request
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1175
This commit will update the UdfGetInfo() function with the support of
EFI_FILE_SYSTEM_VOLUME_LABEL data information request.
Cc: Paulo Alcantara
Cc: Ruiyu Ni
Cc: Star Zeng
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu
---
MdeModulePkg/Universal/Disk/UdfDxe/File.c | 97
+++-
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 84
+
MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 27 ++
MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf | 1 +
4 files changed, 146 insertions(+), 63 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 54c2400243..eb91e877ee 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -718,12 +718,6 @@ UdfSetPosition (
/**
Get information about a file.
- @attention This is boundary function that may receive untrusted input.
- @attention The input is from FileSystem.
-
- The File Set Descriptor is external input, so this routine will do basic
- validation for File Set Descriptor and report status.
-
@param ThisProtocol instance pointer.
@param InformationType Type of information to return in Buffer.
@param BufferSize On input size of buffer, on output amount of data in
@@ -750,19 +744,16 @@ UdfGetInfo (
OUT VOID *Buffer
)
{
- EFI_STATUS Status;
- PRIVATE_UDF_FILE_DATA *PrivFileData;
- PRIVATE_UDF_SIMPLE_FS_DATA *PrivFsData;
- EFI_FILE_SYSTEM_INFO*FileSystemInfo;
- UINTN FileSystemInfoLength;
- CHAR16 *String;
- UDF_FILE_SET_DESCRIPTOR *FileSetDesc;
- UINTN Index;
- UINT8 *OstaCompressed;
- UINT8 CompressionId;
- UINT64 VolumeSize;
- UINT64 FreeSpaceSize;
- CHAR16 VolumeLabel[64];
+ EFI_STATUSStatus;
+ PRIVATE_UDF_FILE_DATA *PrivFileData;
+ PRIVATE_UDF_SIMPLE_FS_DATA*PrivFsData;
+ EFI_FILE_SYSTEM_INFO *FileSystemInfo;
+ UINTN FileSystemInfoLength;
+ UINT64VolumeSize;
+ UINT64FreeSpaceSize;
+ EFI_FILE_SYSTEM_VOLUME_LABEL *FileSystemVolumeLabel;
+ UINTN FileSystemVolumeLabelLength;
+ CHAR16VolumeLabel[64];
if (This == NULL || InformationType == NULL || BufferSize == NULL ||
(*BufferSize != 0 && Buffer == NULL)) {
@@ -784,50 +775,10 @@ UdfGetInfo (
Buffer
);
} else if (CompareGuid (InformationType, )) {
-String = VolumeLabel;
-
-FileSetDesc = >Volume.FileSetDesc;
-
-OstaCompressed = >LogicalVolumeIdentifier[0];
-
-CompressionId = OstaCompressed[0];
-if (!IS_VALID_COMPRESSION_ID (CompressionId)) {
- return EFI_VOLUME_CORRUPTED;
-}
-
-for (Index = 1; Index < 128; Index++) {
- if (CompressionId == 16) {
-*String = *(UINT8 *)(OstaCompressed + Index) << 8;
-Index++;
- } else {
-if (Index > ARRAY_SIZE (VolumeLabel)) {
- return EFI_VOLUME_CORRUPTED;
-}
-
-*String = 0;
- }
-
- if (Index < 128) {
-*String |= (CHAR16)(*(UINT8 *)(OstaCompressed + Index));
- }
-
- //
- // Unlike FID Identifiers, Logical Volume Identifier is stored in a
- // NULL-terminated OSTA compressed format, so we must check for the NULL
- // character.
- //
- if (*String == L'\0') {
-break;
- }
-
- String++;
-}
-
-Index = ((UINTN)String - (UINTN)VolumeLabel) / sizeof (CHAR16);
-if (Index > ARRAY_SIZE (VolumeLabel) - 1) {
- Index = ARRAY_SIZE (VolumeLabel) - 1;
+Status = GetVolumeLabel (>Volume, ARRAY_SIZE (VolumeLabel),
VolumeLabel);
+if (EFI_ERROR (Status)) {
+ return Status;
}
-VolumeLabel[Index] = L'\0';
FileSystemInfoLength = StrSize (VolumeLabel) +
sizeof (EFI_FILE_SYSTEM_INFO);
@@ -839,7 +790,7 @@ UdfGetInfo (
FileSystemInfo = (EFI_FILE_SYSTEM_INFO *)Buffer;
StrCpyS (
FileSystemInfo->VolumeLabel,
- (*BufferSize - OFFSET_OF (EFI_FILE_SYSTEM_INFO, VolumeLabel)) / sizeof
(CHAR16),
+ (*BufferSize - SIZE_OF_EFI_FILE_SYSTEM_INFO) / sizeof (CHAR16),
VolumeLabel
);
Status = GetVolumeSize (
@@ -862,6 +813,26 @@ UdfGetInfo (
*BufferSize = FileSystemInfoLength;
Status = EFI_SUCCESS;
+ } else if (CompareGuid (InformationType,
)) {
+Status = GetVolumeLabel (>Volume, ARRAY_SIZE (VolumeLabel),
VolumeLabel);
+if (EFI_ERROR (Status)) {
+ return Status;
+}
+
+FileSystemVolumeLabelLength = StrSize (VolumeLabel) +
+ sizeof
[edk2] [PATCH v1 03/10] MdeModulePkg/UdfDxe: Add boundary check the read of FE/EFE
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=828
Within ReadFile():
Add checks to ensure that when getting the raw data or the Allocation
Descriptors' data from a FE/EFE, it will not consume data beyond the
size of a FE/EFE.
Cc: Paulo Alcantara
Cc: Ruiyu Ni
Cc: Jiewen Yao
Cc: Star Zeng
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu
---
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 54
++--
1 file changed, 50 insertions(+), 4 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 5fb88c2ff3..d758b798f1 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -503,15 +503,27 @@ DuplicateFe (
NOTE: The FE/EFE can be thought it was an inode.
+ @attention This is boundary function that may receive untrusted input.
+ @attention The input is from FileSystem.
+
+ The (Extended) File Entry is external input, so this routine will do basic
+ validation for (Extended) File Entry and report status.
+
@param[in] FileEntryData (Extended) File Entry pointer.
+ @param[in] FileEntrySize Size of the (Extended) File Entry specified
+ by FileEntryData.
@param[out] DataBuffer contains the raw data of a given
(Extended) File Entry.
@param[out] Length Length of the data in Buffer.
+ @retval EFI_SUCCESS Raw data and size of the FE/EFE was read.
+ @retval EFI_VOLUME_CORRUPTEDThe file system structures are corrupted.
+
**/
-VOID
+EFI_STATUS
GetFileEntryData (
IN VOID*FileEntryData,
+ IN UINTN FileEntrySize,
OUT VOID**Data,
OUT UINT64 *Length
)
@@ -535,20 +547,40 @@ GetFileEntryData (
*Data= (VOID *)((UINT8 *)FileEntry->Data +
FileEntry->LengthOfExtendedAttributes);
}
+
+ if ((*Length > FileEntrySize) ||
+ ((UINTN)FileEntryData > (UINTN)(*Data)) ||
+ ((UINTN)(*Data) - (UINTN)FileEntryData > FileEntrySize - *Length)) {
+return EFI_VOLUME_CORRUPTED;
+ }
+ return EFI_SUCCESS;
}
/**
Get Allocation Descriptors' data information from a given FE/EFE.
+ @attention This is boundary function that may receive untrusted input.
+ @attention The input is from FileSystem.
+
+ The (Extended) File Entry is external input, so this routine will do basic
+ validation for (Extended) File Entry and report status.
+
@param[in] FileEntryData (Extended) File Entry pointer.
+ @param[in] FileEntrySize Size of the (Extended) File Entry specified
+ by FileEntryData.
@param[out] AdsData Buffer contains the Allocation Descriptors'
data from a given FE/EFE.
@param[out] Length Length of the data in AdsData.
+ @retval EFI_SUCCESS The data and size of Allocation Descriptors
+ were read from the FE/EFE.
+ @retval EFI_VOLUME_CORRUPTEDThe file system structures are corrupted.
+
**/
-VOID
+EFI_STATUS
GetAdsInformation (
IN VOID*FileEntryData,
+ IN UINTN FileEntrySize,
OUT VOID**AdsData,
OUT UINT64 *Length
)
@@ -572,6 +604,13 @@ GetAdsInformation (
*AdsData = (VOID *)((UINT8 *)FileEntry->Data +
FileEntry->LengthOfExtendedAttributes);
}
+
+ if ((*Length > FileEntrySize) ||
+ ((UINTN)FileEntryData > (UINTN)(*AdsData)) ||
+ ((UINTN)(*AdsData) - (UINTN)FileEntryData > FileEntrySize - *Length)) {
+return EFI_VOLUME_CORRUPTED;
+ }
+ return EFI_SUCCESS;
}
/**
@@ -1065,7 +1104,10 @@ ReadFile (
//
// There are no extents for this FE/EFE. All data is inline.
//
-GetFileEntryData (FileEntryData, , );
+Status = GetFileEntryData (FileEntryData, Volume->FileEntrySize, ,
);
+if (EFI_ERROR (Status)) {
+ return Status;
+}
if (ReadFileInfo->Flags == ReadFileGetFileSize) {
ReadFileInfo->ReadLength = Length;
@@ -1109,7 +1151,11 @@ ReadFile (
// This FE/EFE contains a run of Allocation Descriptors. Get data + size
// for start reading them out.
//
-GetAdsInformation (FileEntryData, , );
+Status = GetAdsInformation (FileEntryData, Volume->FileEntrySize, ,
);
+if (EFI_ERROR (Status)) {
+ return Status;
+}
+
AdOffset = 0;
for (;;) {
--
2.12.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v1 01/10] MdeModulePkg/PartitionDxe: Add check for underlying device block size
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=828
Within FindAnchorVolumeDescriptorPointer():
Add a check for the underlying device block size to ensure it is greater
than the size of an Anchor Volume Descriptor Pointer.
Cc: Paulo Alcantara
Cc: Ruiyu Ni
Cc: Jiewen Yao
Cc: Star Zeng
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu
---
MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 28
1 file changed, 28 insertions(+)
diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
index 83bd174231..0fd56b75b4 100644
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
@@ -1,6 +1,14 @@
/** @file
Scan for an UDF file system on a formatted media.
+ Caution: This file requires additional review when modified.
+ This driver will have external input - CD/DVD media.
+ This external input must be validated carefully to avoid security issue like
+ buffer overflow, integer overflow.
+
+ FindUdfFileSystem() routine will consume the media properties and do basic
+ validations.
+
Copyright (c) 2018 Qualcomm Datacenter Technologies, Inc.
Copyright (C) 2014-2017 Paulo Alcantara
@@ -102,6 +110,20 @@ FindAnchorVolumeDescriptorPointer (
AvdpsCount = 0;
//
+ // Check if the block size of the underlying media can hold the data of an
+ // Anchor Volume Descriptor Pointer
+ //
+ if (BlockSize < sizeof (UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER)) {
+DEBUG ((
+ DEBUG_ERROR,
+ "%a: Media block size 0x%x unable to hold an AVDP.\n",
+ __FUNCTION__,
+ BlockSize
+ ));
+return EFI_UNSUPPORTED;
+ }
+
+ //
// Find AVDP at block 256
//
Status = DiskIo->ReadDisk (
@@ -598,6 +620,12 @@ Out_Free:
/**
Find a supported UDF file system in block device.
+ @attention This is boundary function that may receive untrusted input.
+ @attention The input is from Partition.
+
+ The CD/DVD media is the external input, so this routine will do basic
+ validations for the media.
+
@param[in] BlockIo BlockIo interface.
@param[in] DiskIo DiskIo interface.
@param[out] StartingLBA UDF file system starting LBA.
--
2.12.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v1 02/10] MdeModulePkg/UdfDxe: Refine boundary checks for file/path name string
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=828
The commit refines the boundary checks for file/path name string to
prevent possible buffer overrun.
Cc: Paulo Alcantara
Cc: Ruiyu Ni
Cc: Jiewen Yao
Cc: Star Zeng
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu
---
MdeModulePkg/Universal/Disk/UdfDxe/File.c | 29 +++--
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 64
+---
MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 29 -
3 files changed, 107 insertions(+), 15 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 6f07bf2066..80db734f86 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -248,7 +248,7 @@ UdfOpen (
FileName = TempFileName + 1;
}
- StrCpyS (NewPrivFileData->FileName, UDF_PATH_LENGTH, FileName);
+ StrCpyS (NewPrivFileData->FileName, UDF_FILENAME_LENGTH, FileName);
Status = GetFileSize (
PrivFsData->BlockIo,
@@ -444,7 +444,7 @@ UdfRead (
FreePool ((VOID *)NewFileEntryData);
NewFileEntryData = FoundFile.FileEntry;
- Status = GetFileNameFromFid (NewFileIdentifierDesc, FileName);
+ Status = GetFileNameFromFid (NewFileIdentifierDesc, ARRAY_SIZE
(FileName), FileName);
if (EFI_ERROR (Status)) {
FreePool ((VOID *)FoundFile.FileIdentifierDesc);
goto Error_Get_FileName;
@@ -456,7 +456,7 @@ UdfRead (
FoundFile.FileIdentifierDesc = NewFileIdentifierDesc;
FoundFile.FileEntry = NewFileEntryData;
- Status = GetFileNameFromFid (FoundFile.FileIdentifierDesc, FileName);
+ Status = GetFileNameFromFid (FoundFile.FileIdentifierDesc, ARRAY_SIZE
(FileName), FileName);
if (EFI_ERROR (Status)) {
goto Error_Get_FileName;
}
@@ -718,6 +718,12 @@ UdfSetPosition (
/**
Get information about a file.
+ @attention This is boundary function that may receive untrusted input.
+ @attention The input is from FileSystem.
+
+ The File Set Descriptor is external input, so this routine will do basic
+ validation for File Set Descriptor and report status.
+
@param ThisProtocol instance pointer.
@param InformationType Type of information to return in Buffer.
@param BufferSize On input size of buffer, on output amount of data in
@@ -794,6 +800,10 @@ UdfGetInfo (
*String = *(UINT8 *)(OstaCompressed + Index) << 8;
Index++;
} else {
+if (Index > ARRAY_SIZE (VolumeLabel)) {
+ return EFI_VOLUME_CORRUPTED;
+}
+
*String = 0;
}
@@ -813,7 +823,11 @@ UdfGetInfo (
String++;
}
-*String = L'\0';
+Index = ((UINTN)String - (UINTN)VolumeLabel) / sizeof (CHAR16);
+if (Index > ARRAY_SIZE (VolumeLabel) - 1) {
+ Index = ARRAY_SIZE (VolumeLabel) - 1;
+}
+VolumeLabel[Index] = L'\0';
FileSystemInfoLength = StrSize (VolumeLabel) +
sizeof (EFI_FILE_SYSTEM_INFO);
@@ -823,8 +837,11 @@ UdfGetInfo (
}
FileSystemInfo = (EFI_FILE_SYSTEM_INFO *)Buffer;
-StrCpyS (FileSystemInfo->VolumeLabel, ARRAY_SIZE (VolumeLabel),
- VolumeLabel);
+StrCpyS (
+ FileSystemInfo->VolumeLabel,
+ (*BufferSize - OFFSET_OF (EFI_FILE_SYSTEM_INFO, VolumeLabel)) / sizeof
(CHAR16),
+ VolumeLabel
+ );
Status = GetVolumeSize (
PrivFsData->BlockIo,
PrivFsData->DiskIo,
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index ecc172303e..5fb88c2ff3 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -1412,7 +1412,7 @@ InternalFindFile (
break;
}
} else {
- Status = GetFileNameFromFid (FileIdentifierDesc, FoundFileName);
+ Status = GetFileNameFromFid (FileIdentifierDesc, ARRAY_SIZE
(FoundFileName), FoundFileName);
if (EFI_ERROR (Status)) {
break;
}
@@ -1705,6 +1705,11 @@ FindFile (
while (*FilePath != L'\0') {
FileNamePointer = FileName;
while (*FilePath != L'\0' && *FilePath != L'\\') {
+ if UINTN)FileNamePointer - (UINTN)FileName) / sizeof (CHAR16)) >=
+ (ARRAY_SIZE (FileName) - 1)) {
+return EFI_NOT_FOUND;
+ }
+
*FileNamePointer++ = *FilePath++;
}
@@ -1882,22 +1887,38 @@ ReadDirectoryEntry (
Get a filename (encoded in OSTA-compressed format) from a File Identifier
Descriptor on an UDF volume.
+ @attention This is boundary function that may receive untrusted input.
+ @attention The input is from FileSystem.
+
+ The File Identifier Descriptor is external input, so this routine will do
+ basic validation for File Identifier Descriptor and report status.
+
@param[in] FileIdentifierDesc File
[edk2] [PATCH v1 00/10] UDF: Bugfixes
The series will address a couple of bugs within the UDF related codes. Please refer to the log message of each commit for more details. Cc: Paulo Alcantara Cc: Ruiyu Ni Cc: Jiewen Yao Cc: Star Zeng Hao Wu (10): MdeModulePkg/PartitionDxe: Add check for underlying device block size MdeModulePkg/UdfDxe: Refine boundary checks for file/path name string MdeModulePkg/UdfDxe: Add boundary check the read of FE/EFE MdeModulePkg/UdfDxe: Add boundary check for ComponentIdentifier decode MdeModulePkg/UdfDxe: Add boundary check for getting volume (free) size MdeModulePkg/UdfDxe: Correct behavior for UdfSetPosition() MdeModulePkg/UdfDxe: Fix a typo within SetFileInfo() MdeModulePkg/UdfDxe: Update GetInfo() for FS VolumeLabel info request MdeModulePkg/UdfDxe: Add more check when getting PD from LongAd MdeModulePkg/UdfDxe: Avoid possible use of already-freed data MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c| 28 +++ MdeModulePkg/Universal/Disk/UdfDxe/File.c | 96 MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 253 ++-- MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 63 - MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf | 1 + 5 files changed, 362 insertions(+), 79 deletions(-) -- 2.12.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] BaseTool: Support different PCDs that refers to the same EFI variable.
Reviewed-by: Feng, Bob C
-Original Message-
From: Zhao, ZhiqiangX
Sent: Wednesday, October 10, 2018 4:41 PM
To: edk2-devel@lists.01.org
Cc: Zhao, ZhiqiangX ; Gao, Liming
; Zhu, Yonghong ; Feng, Bob C
Subject: [PATCH] BaseTool: Support different PCDs that refers to the same EFI
variable.
If different PCDs refer to the same EFI variable, then do EFI variable
combination, according to the VariableOffset of different PCDS.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: ZhiqiangX Zhao
Cc: Liming Gao
Cc: Yonghong Zhu
Cc: Bob Feng
---
BaseTools/Source/Python/AutoGen/GenVar.py | 96 ++-
1 file changed, 30 insertions(+), 66 deletions(-)
diff --git a/BaseTools/Source/Python/AutoGen/GenVar.py
b/BaseTools/Source/Python/AutoGen/GenVar.py
index 036f00e2bb..5ce7892d72 100644
--- a/BaseTools/Source/Python/AutoGen/GenVar.py
+++ b/BaseTools/Source/Python/AutoGen/GenVar.py
@@ -56,51 +56,7 @@ class VariableMgr(object):
value_str += ",".join(default_var_bin_strip)
value_str += "}"
return value_str
-def Do_combine(self,sku_var_info_offset_list):
-newvalue = {}
-for item in sku_var_info_offset_list:
-data_type = item.data_type
-value_list = item.default_value.strip("{").strip("}").split(",")
-if data_type in DataType.TAB_PCD_NUMERIC_TYPES:
-data_flag =
DataType.PACK_CODE_BY_SIZE[MAX_SIZE_TYPE[data_type]]
-data = value_list[0]
-value_list = []
-for data_byte in pack(data_flag, int(data, 16) if
data.upper().startswith('0X') else int(data)):
-value_list.append(hex(unpack("B", data_byte)[0]))
-newvalue[int(item.var_offset, 16) if
item.var_offset.upper().startswith("0X") else int(item.var_offset)] = value_list
-try:
-newvaluestr = "{" +
",".join(VariableMgr.assemble_variable(newvalue)) +"}"
-except:
-EdkLogger.error("build", AUTOGEN_ERROR, "Variable offset conflict
in PCDs: %s \n" % (" and ".join(item.pcdname for item in
sku_var_info_offset_list)))
-return newvaluestr
-def Do_Merge(self,sku_var_info_offset_list):
-StructrurePcds = sorted([item for item in sku_var_info_offset_list if
item.StructurePcd], key = lambda x: x.PcdDscLine, reverse =True )
-Base = StructrurePcds[0]
-BaseValue = Base.default_value.strip("{").strip("}").split(",")
-Override = [item for item in sku_var_info_offset_list if not
item.StructurePcd and item.PcdDscLine > Base.PcdDscLine]
-newvalue = {}
-for item in Override:
-data_type = item.data_type
-value_list = item.default_value.strip("{").strip("}").split(",")
-if data_type in DataType.TAB_PCD_NUMERIC_TYPES:
-data_flag =
DataType.PACK_CODE_BY_SIZE[MAX_SIZE_TYPE[data_type]]
-data = value_list[0]
-value_list = []
-for data_byte in pack(data_flag, int(data, 16) if
data.upper().startswith('0X') else int(data)):
-value_list.append(hex(unpack("B", data_byte)[0]))
-newvalue[int(item.var_offset, 16) if
item.var_offset.upper().startswith("0X") else int(item.var_offset)] =
(value_list,item.pcdname,item.PcdDscLine)
-for offset in newvalue:
-value_list,itemPcdname,itemPcdDscLine = newvalue[offset]
-if offset > len(BaseValue) or (offset + len(value_list) >
len(BaseValue)):
-EdkLogger.error("build", AUTOGEN_ERROR, "The EFI Variable
referred by PCD %s in line %s exceeds variable size: %s\n" %
(itemPcdname,itemPcdDscLine,hex(len(BaseValue
-for i in xrange(len(value_list)):
-BaseValue[offset + i] = value_list[i]
-newvaluestr = "{" + ",".join(BaseValue) +"}"
-return newvaluestr
-def NeedMerge(self,sku_var_info_offset_list):
-if [item for item in sku_var_info_offset_list if item.StructurePcd]:
-return True
-return False
+
def combine_variable(self):
indexedvarinfo = collections.OrderedDict()
for item in self.VarInfo:
@@ -109,31 +65,39 @@ class VariableMgr(object):
indexedvarinfo[(item.skuname, item.defaultstoragename,
item.var_name, item.var_guid)].append(item)
for key in indexedvarinfo:
sku_var_info_offset_list = indexedvarinfo[key]
-if len(sku_var_info_offset_list) == 1:
-continue
-
+sku_var_info_offset_list.sort(key=lambda x:x.PcdDscLine)
+FirstOffset = int(sku_var_info_offset_list[0].var_offset, 16) if
sku_var_info_offset_list[0].var_offset.upper().startswith("0X") else
int(sku_var_info_offset_list[0].var_offset)
+fisrtvalue_list =
sku_var_info_offset_list[0].default_value.strip("{").strip("}").split(",")
+firstdata_type = sku_var_info_offset_list[0].data_type
+
Re: [edk2] [PATCH] MdeModulePkg Variable: Fix Timestamp zeroing issue on APPEND_WRITE
Reviewed-by: jiewen@intel.com
> -Original Message-
> From: Zeng, Star
> Sent: Tuesday, October 16, 2018 10:41 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Yao, Jiewen ;
> Zhang, Chao B ; Wang, Jian J
>
> Subject: [PATCH] MdeModulePkg Variable: Fix Timestamp zeroing issue on
> APPEND_WRITE
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=415
>
> When SetVariable() to a time based auth variable with APPEND_WRITE
> attribute, and if the EFI_VARIABLE_AUTHENTICATION_2.TimeStamp in
> the input Data is earlier than current value, it will cause timestamp
> zeroing.
>
> This issue may bring time based auth variable downgrade problem.
> For example:
> A vendor released three certs at 2014, 2015, and 2016, and system
> integrated the 2016 cert. User can SetVariable() with 2015 cert and
> APPEND_WRITE attribute to cause timestamp zeroing first, then
> SetVariable() with 2014 cert to downgrade the cert.
>
> This patch fixes this issue.
>
> Cc: Jiewen Yao
> Cc: Chao Zhang
> Cc: Jian J Wang
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng
> ---
> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> index a2d61c8cd618..8e8db71bd201 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> @@ -2462,6 +2462,8 @@ UpdateVariable (
> if (Variable->CurrPtr != NULL) {
>if (VariableCompareTimeStampInternal
> (&(((AUTHENTICATED_VARIABLE_HEADER *)
> CacheVariable->CurrPtr)->TimeStamp), TimeStamp)) {
> CopyMem (>TimeStamp, TimeStamp, sizeof
> (EFI_TIME));
> + } else {
> +CopyMem (>TimeStamp,
> &(((AUTHENTICATED_VARIABLE_HEADER *)
> CacheVariable->CurrPtr)->TimeStamp), sizeof (EFI_TIME));
>}
> }
>}
> --
> 2.7.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v1 7/7] MdeModulePkg/UdfDxe: Handle dead codes in FileSystemOperations.c
On 2018/10/15 12:55, Hao Wu wrote:
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1249
We found potential dead codes within File.c during the code coverage test.
After manual review, we think the below ones are positive reports:
A. For function GetAllocationDescriptor():
Due to the all the calling places for this function, the input parameter
'RecordingFlags' can only with value 'LongAdsSequence' or
'ShortAdsSequence'.
So the below code will never be reached:
return EFI_DEVICE_ERROR;
This commit will add "ASSERT (FALSE);" before the above line to indicate
this.
B. For function GetAllocationDescriptorLsn():
Due to the all the calling places for this function, the input parameter
'RecordingFlags' can only with value 'LongAdsSequence' or
'ShortAdsSequence'.
So the below code will never be reached:
return EFI_UNSUPPORTED;
This commit will add "ASSERT (FALSE);" before the above line to indicate
this.
C. For function SetFileInfo():
Due to the all the calling places for this function, the input parameter
'FileName' will never be a NULL pointer.
So the below codes will never be reached:
} else {
FileInfo->FileName[0] = '\0';
}
This commit will add "ASSERT (FALSE);" before the above lines to indicate
this.
Hao,
Thanks for the patch.
I think we should see what is the expected value for the parameter, but
not see how caller uses the parameter.
From this point of view, I think the C case is valid and may be no need
to change.
Thanks,
Star
Cc: Paulo Alcantara
Cc: Ruiyu Ni
Cc: Star Zeng
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu
---
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 12
1 file changed, 12 insertions(+)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 1400846bf1..19acd0554c 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -738,6 +738,10 @@ GetAllocationDescriptor (
);
}
+ //
+ // Code should never reach here.
+ //
+ ASSERT (FALSE);
return EFI_DEVICE_ERROR;
}
@@ -784,6 +788,10 @@ GetAllocationDescriptorLsn (
return EFI_SUCCESS;
}
+ //
+ // Code should never reach here.
+ //
+ ASSERT (FALSE);
return EFI_UNSUPPORTED;
}
@@ -2413,6 +2421,10 @@ SetFileInfo (
if (FileName != NULL) {
StrCpyS (FileInfo->FileName, StrLen (FileName) + 1, FileName);
} else {
+//
+// Code should never reach here.
+//
+ASSERT (FALSE);
FileInfo->FileName[0] = '\0';
}
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] Expression spec: Add clarification for String compare
Reviewed-by: Liming Gao > -Original Message- > From: Zhu, Yonghong > Sent: Tuesday, October 16, 2018 9:12 AM > To: edk2-devel@lists.01.org > Cc: Gao, Liming ; Kinney, Michael D > ; Shaw, Kevin W > Subject: [Patch] Expression spec: Add clarification for String compare > > Relational and equality operators require both operands to be of > the same type. > Treat the string 'A' and "A" as same type, but for "A" and L"A" > are not same type since one is general string, another is unicode > string. > > Cc: Liming Gao > Cc: Michael Kinney > Cc: Kevin W Shaw > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Yonghong Zhu > --- > 2_expression_overview.md | 13 - > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/2_expression_overview.md b/2_expression_overview.md > index c29a632..bf66ffe 100644 > --- a/2_expression_overview.md > +++ b/2_expression_overview.md > @@ -1,9 +1,9 @@ > 2.6.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] BaseTools/EOT: Change to call a program instead of calling Python API.
Reviewed-by: Yonghong Zhu
Best Regards,
Zhu Yonghong
-Original Message-
From: Carsey, Jaben
Sent: Tuesday, October 16, 2018 5:42 AM
To: Chen, Hesheng ; Zhu, Yonghong
; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] BaseTools/EOT: Change to call a program instead of
calling Python API.
Hess,
Thanks for the clarification. Makes sense!
I glanced at, but didn't read in detail the code... so.
Acked-by: Jaben Carsey
> -Original Message-
> From: Chen, Hesheng
> Sent: Monday, October 15, 2018 2:34 PM
> To: Carsey, Jaben ; Zhu, Yonghong
> ; edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH] BaseTools/EOT: Change to call a program
> instead of calling Python API.
> Importance: High
>
> Hello Jaben,
> The API is provided by C code and we want Python tool to use it. The
> tool used to call Python API from DLL files and now we need run Python
> tools from source so we can't build a specific version of DLL binary
> for it. The DLL file may be different for different version of Python.
> So now we just call the C program directly for the API.
>
> Best Regards,
> Chen, Hess
> Intel China Software Center
> Tel: +86-21-6116-6740
> Email: hesheng.c...@intel.com
>
> -Original Message-
> From: Carsey, Jaben
> Sent: Tuesday, October 16, 2018 1:45 AM
> To: Zhu, Yonghong ; edk2-devel@lists.01.org
> Cc: Chen, Hesheng
> Subject: RE: [edk2] [PATCH] BaseTools/EOT: Change to call a program
> instead of calling Python API.
>
>
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> > Of Yonghong Zhu
> > Sent: Monday, October 15, 2018 3:24 AM
> > To: edk2-devel@lists.01.org
> > Cc: Chen, Hesheng
> > Subject: [edk2] [PATCH] BaseTools/EOT: Change to call a program
> > instead of calling Python API.
> >
> > From: hchen30
> >
> > Update the EOT tool to call the program itself instead of calling
> > the Python API when parsing FV images.
>
> Why do we prefer to launch the separate python program instead of
> calling the APIs?
>
>
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Hess Chen
> > ---
> > BaseTools/Source/Python/Eot/{Eot.py => EotMain.py} | 465
> > +++--
> >
> > BaseTools/Source/Python/Eot/InfParserLite.py | 26 +-
> > BaseTools/Source/Python/Eot/Parser.py | 28 +-
> > BaseTools/Source/Python/Eot/Report.py | 6 +-
> > BaseTools/Source/Python/build/BuildReport.py | 2 +-
> > 5 files changed, 84 insertions(+), 443 deletions(-) rename
> > BaseTools/Source/Python/Eot/{Eot.py => EotMain.py} (75%)
> >
> > diff --git a/BaseTools/Source/Python/Eot/Eot.py
> > b/BaseTools/Source/Python/Eot/EotMain.py
> > similarity index 75%
> > rename from BaseTools/Source/Python/Eot/Eot.py
> > rename to BaseTools/Source/Python/Eot/EotMain.py
> > index ce83da1495..49a1494126 100644
> > --- a/BaseTools/Source/Python/Eot/Eot.py
> > +++ b/BaseTools/Source/Python/Eot/EotMain.py
> > @@ -17,18 +17,20 @@
> > from __future__ import absolute_import import
> Common.LongFilePathOs
> > as os, time, glob import Common.EdkLogger as EdkLogger -from .
> > import EotGlobalData
> > +import Eot.EotGlobalData as EotGlobalData
> > from optparse import OptionParser
> > from Common.StringUtils import NormPath from Common import
> > BuildToolError from Common.Misc import
> > GuidStructureStringToGuidString, sdict -from .InfParserLite import *
> > -from . import c -from . import Database
> > +from Eot.Parser import *
> > +from Eot.InfParserLite import EdkInfParser from Common.StringUtils
> > +import GetSplitValueList from Eot import c from Eot import Database
> > from array import array
> > -from .Report import Report
> > +from Eot.Report import Report
> > from Common.BuildVersion import gBUILD_VERSION -from .Parser import
> > ConvertGuid
> > +from Eot.Parser import ConvertGuid
> > from Common.LongFilePathSupport import OpenLongFilePath as open
> > import struct import uuid @@ -58,14 +60,14 @@ class Image(array):
> >
> > self._SubImages = sdict() # {offset: Image()}
> >
> > -array.__init__(self, 'B')
> > +array.__init__(self)
> >
> > def __repr__(self):
> > return self._ID_
> >
> > def __len__(self):
> > Len = array.__len__(self)
> > -for Offset in self._SubImages:
> > +for Offset in self._SubImages.keys():
> > Len += len(self._SubImages[Offset])
> > return Len
> >
> > @@ -154,19 +156,11 @@ class CompressedImage(Image):
> >
> > def _GetSections(self):
> > try:
> > -from . import EfiCompressor
> > -TmpData = EfiCompressor.FrameworkDecompress(
> > -self[self._HEADER_SIZE_:],
> > -len(self) - self._HEADER_SIZE_
> > -)
> > +TmpData = DeCompress('Efi', self[self._HEADER_SIZE_:])
> >
Re: [edk2] [PATCH v1 6/7] MdeModulePkg/UdfDxe: Remove dead codes in FileName.c
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Leif Lindholm
> Sent: Tuesday, October 16, 2018 2:20 PM
> To: Wu, Hao A
> Cc: Ni, Ruiyu; edk2-devel@lists.01.org; Zeng, Star
> Subject: Re: [edk2] [PATCH v1 6/7] MdeModulePkg/UdfDxe: Remove dead
> codes in FileName.c
>
> On Mon, Oct 15, 2018 at 12:55:21PM +0800, Hao Wu wrote:
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1249
> >
> > We found potential dead codes within File.c during the code coverage test.
> >
> > After manual review, we think the below ones are positive reports:
> >
> > A. In function MangleFileName():
> >
> > FileName = TrimString (FileName);
> > // Begin of dead codes
> > if (*FileName == L'\0') {
> > goto Exit;
> > }
> > // End of dead codes
> >
> > When the code reaches the TrimString() call, the string in 'FileName' is
> > guaranteed to have a '\' character due to the call patterns of
>
> What in the call pattern guarantees this? Please be precise.
OK, I will update the log message.
>
> > MangleFileName(). So after trimming the lead-off/tailing white spaces,
> > string in 'FileName' will not be an empty string.
> >
> > B. In function MangleFileName():
> >
> > if (FileName[0] == L'.') {
> > if (FileName[1] == L'.') {
> > if (FileName[2] == L'\0') {
> > goto Exit;
> > } else {
> > FileName += 2;
> > }
> > } else if (FileName[1] == L'\0') {
> > goto Exit;
> > }
> > }
> >
> > When the code hits the above checks, string in 'FileName' will always have
> > a leading '\' character (denoting an absolute path) due to the call
> > patterns of MangleFileName(). So no leading '.' can be there in string
> > 'FileName'.
>
> What in the call pattern guarantees this? Please be precise.
OK, I will update the log message.
Thanks for the comments.
Best Regards,
Hao Wu
>
> Regards,
>
> Leif
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v1 0/7] Code refinements in UdfDxe
On 2018/10/15 12:55, Hao Wu wrote: This series will refine the codes in MdeModulePkg/Universal/Disk/UdfDxe for: A. Refine asserts used for memory allocation failure and error cases that are possible to happen. Will use error handling logic for them; B. Address some dead codes within this module. Cc: Paulo Alcantara Cc: Ruiyu Ni Cc: Star Zeng Hao Wu (7): MdeModulePkg/UdfDxe: Use error handling for memory allocation failure MdeModulePkg/UdfDxe: ASSERT for false positives of NULL ptr deref MdeModulePkg/UdfDxe: Use error handling when fail to return LSN MdeModulePkg/UdfDxe: Use debug msg instead of ASSERT in UdfOpen() MdeModulePkg/UdfDxe: Handle dead codes in File.c MdeModulePkg/UdfDxe: Remove dead codes in FileName.c MdeModulePkg/UdfDxe: Handle dead codes in FileSystemOperations.c MdeModulePkg/Universal/Disk/UdfDxe/File.c | 19 ++- MdeModulePkg/Universal/Disk/UdfDxe/FileName.c | 15 -- MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 162 +++- 3 files changed, 142 insertions(+), 54 deletions(-) Hao, Thanks for the patches. Reviewed-by: Star Zeng to patch 1 ~ 6. Some feedback will be sent for patch 7. Star ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v1 6/7] MdeModulePkg/UdfDxe: Remove dead codes in FileName.c
On Mon, Oct 15, 2018 at 12:55:21PM +0800, Hao Wu wrote:
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1249
>
> We found potential dead codes within File.c during the code coverage test.
>
> After manual review, we think the below ones are positive reports:
>
> A. In function MangleFileName():
>
> FileName = TrimString (FileName);
> // Begin of dead codes
> if (*FileName == L'\0') {
> goto Exit;
> }
> // End of dead codes
>
> When the code reaches the TrimString() call, the string in 'FileName' is
> guaranteed to have a '\' character due to the call patterns of
What in the call pattern guarantees this? Please be precise.
> MangleFileName(). So after trimming the lead-off/tailing white spaces,
> string in 'FileName' will not be an empty string.
>
> B. In function MangleFileName():
>
> if (FileName[0] == L'.') {
> if (FileName[1] == L'.') {
> if (FileName[2] == L'\0') {
> goto Exit;
> } else {
> FileName += 2;
> }
> } else if (FileName[1] == L'\0') {
> goto Exit;
> }
> }
>
> When the code hits the above checks, string in 'FileName' will always have
> a leading '\' character (denoting an absolute path) due to the call
> patterns of MangleFileName(). So no leading '.' can be there in string
> 'FileName'.
What in the call pattern guarantees this? Please be precise.
Regards,
Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] PACKAGES_PATH in !include path in Dsc files
> -Original Message- > From: Gao, Liming [mailto:liming@intel.com] > Sent: Tuesday, October 16, 2018 10:58 AM > To: Pankaj Bansal ; Ard Biesheuvel > > Cc: Zhu, Yonghong ; Leif Lindholm > ; Kinney, Michael D ; > edk2-devel@lists.01.org; Udit Kumar ; Varun Sethi > > Subject: RE: PACKAGES_PATH in !include path in Dsc files > > Hi, > You can directly include it. BaseTools will search it from WORKSPACE and > PACKAGES_PATH. So, you only need to set edk2-platforms directory into > PACKAGES_PATH env. > > !include Silicon/NXP/.dsc Thanks You Liming Gao. It worked for me. > > Thanks > Liming > >-Original Message- > >From: Pankaj Bansal [mailto:pankaj.ban...@nxp.com] > >Sent: Tuesday, October 16, 2018 1:24 PM > >To: Ard Biesheuvel > >Cc: Gao, Liming ; Zhu, Yonghong > >; Leif Lindholm ; > >Kinney, Michael D ; > >edk2-devel@lists.01.org; Udit Kumar ; Varun Sethi > > > >Subject: RE: PACKAGES_PATH in !include path in Dsc files > > > > > > > >> -Original Message- > >> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > >> Sent: Tuesday, October 16, 2018 8:41 AM > >> To: Pankaj Bansal > >> Cc: Gao, Liming ; Zhu, Yonghong > >> ; Leif Lindholm ; > >Michael > >> D Kinney ; edk2-devel@lists.01.org; Udit > >Kumar > >> ; Varun Sethi > >> Subject: Re: PACKAGES_PATH in !include path in Dsc files > >> > >> On 16 October 2018 at 10:40, Pankaj Bansal > >wrote: > >> > +edk2-platforms maintainers in To list > >> > > >> > > >> > > >> > Thank you Liming for replying. > >> > > >> > > >> > > >> > Our entire code is in edk2-platforms > >> > > >(https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > >> > thub.com%2Ftianocore%2Fedk2- > >> > >platformsdata=02%7C01%7Cpankaj.bansal%40nxp.com%7C552da3f22b > >5 > >> > 84b7fac6008d63315ec8b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0 > >> > %7C636752566592695047sdata=JJWbAcZkj%2FtFaZC0bWONPb7ulCcj1 > >L2 > >> 4VKwCDGDx9OE%3Dreserved=0) which is denoted by > >PACKAGES_PATH. > >> > > >> > The PACKAGES_PATH directory can be anywhere in WORKSPACE > >depending on > >> > the sync directory defined by user. > >> > > >> > i.e. it can be $(WORKSPACE)/edk2-platforms or $(WORKSPACE)/ >> > directory name that user can define during git sync> > >> > > >> > As our dsc files are relative to PACKAGES_PATH, I want to specify > >> > their path in dsc file like this: > >> > > >> > > >> > > >> > !include $(PACKAGES_PATH)/Silicon/NXP/.dsc > >> > > >> > > >> > > >> > Using $(WORKSPACE), I cannot specify above path, as it can be at > >> > place other than $(WORKSPACE)/edk2-platforms > >> > > >> > >> But why do you need to !include things in the first place? > >> > >> Can you explain how you are trying to organize the files, and which > >> file > >includes > >> which? > > > >I am trying to keep Silicon (SOC) specific dsc file in > >Silicon/NXP/ >Name>/ > >This silicon can be used in multiple Boards (Platforms). > >All these Platforms are present in Platform/NXP/ fd/fv > >binaries would be created for each platform. > >The chassis dsc file has description of components/PCDs that are > >specific to chassis to which the silicon belongs. It would be same for > >all silicons that belong to same chassis. > >The Silicon dsc file has description of components/PCDs that are > >specific to silicon and would be same for all platforms that use this > >silicon. It would include chassis dsc file The Platform dsc file would > >include the silicon dsc file. > > > >___ > >| Platform (in Platform/NXP)| > >|_ | > >| | Silicon (in Silicon/NXP/) | | > >| | ___| | > >| | | Chassis (in Silicon/NXP) | || > >| | |__| | | > >| || | > >|_| > > > >Regards, > >Pankaj Bansal ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
