[edk2] [Patch] BaseTools: Fix a bug for Unused PCDs section display in the report
From: zhijufan Fix a regression issue caused by ac4578af364, when there doesn't exist not used PCD, it also display the not used Pcd section in the report. Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=1170 Cc: Liming Gao Cc: Yonghong Zhu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Zhiju.Fan --- BaseTools/Source/Python/build/BuildReport.py | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/BaseTools/Source/Python/build/BuildReport.py b/BaseTools/Source/Python/build/BuildReport.py index c7fa1b9..49bcd9c 100644 --- a/BaseTools/Source/Python/build/BuildReport.py +++ b/BaseTools/Source/Python/build/BuildReport.py @@ -886,11 +886,21 @@ class PcdReport(object): def GenerateReport(self, File, ModulePcdSet): if not ModulePcdSet: if self.ConditionalPcds: self.GenerateReportDetail(File, ModulePcdSet, 1) if self.UnusedPcds: -self.GenerateReportDetail(File, ModulePcdSet, 2) +IsEmpty = True +for Token in self.UnusedPcds: +TokenDict = self.UnusedPcds[Token] +for Type in TokenDict: +if TokenDict[Type]: +IsEmpty = False +break +if not IsEmpty: +break +if not IsEmpty: +self.GenerateReportDetail(File, ModulePcdSet, 2) self.GenerateReportDetail(File, ModulePcdSet) ## # Generate report for PCD information # -- 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] IntelSiliconPkg IntelVTdDxe: Optimize when func 0 is not implemented
I checked the UEFI shell implementation. It uses below:
//
// If this is not a multi-function device, we can leave the
loop
// to deal with the next device.
//
if (Func == 0 && ((PciHeader.HeaderType &
HEADER_TYPE_MULTI_FUNCTION) == 0x00)) {
break;
}
Thank you
Yao Jiewen
> -Original Message-
> From: Ni, Ruiyu
> Sent: Thursday, September 13, 2018 11:11 AM
> To: Zeng, Star ; edk2-devel@lists.01.org
> Cc: Chang, Tomson ; Yao, Jiewen
> ; Huang, Jenny ; Chan,
> Amy
> Subject: Re: [edk2] [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when func
> 0 is not implemented
>
> On 9/13/2018 10:10 AM, Star Zeng wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1169
> >
> > PCI spec:
> > They are also required to always implement function 0 in the device.
> > Implementing other functions is optional and may be assigned in any
> > order (i.e., a two-function device must respond to function 0 but
> > can choose any of the other possible function numbers (1-7) for the
> > second function).
> >
> > This patch updates ScanPciBus() to not scan other functions if
> > function 0 is not implemented.
> >
> > Test done:
> > Added debug code below in the second loop of ScanPciBus(),
> > compared the debug logs with and without this patch, many
> > non-0 unimplemented functions are skipped correctly.
> >
> >DEBUG ((
> > DEBUG_INFO,
> > "%a() B%02xD%02xF%02x VendorId: %04x DeviceId: %04x\n",
> > __FUNCTION__,
> > Bus,
> > Device,
> > Function,
> > VendorID,
> > DeviceID
> > ));
> >
> > Cc: Jiewen Yao
> > Cc: Rangasai V Chaganty
> > Cc: Tomson Chang
> > Cc: Jenny Huang
> > Cc: Amy Chan
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Star Zeng
> > ---
> > IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c | 8 +++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> > index 36750b3f1d9c..305995de032c 100644
> > --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> > +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
> > @@ -1,6 +1,6 @@
> > /** @file
> >
> > - 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
> > which accompanies this distribution. The full text of the license may
> be found at
> > @@ -247,6 +247,12 @@ ScanPciBus (
> > VendorID = PciSegmentRead16
> (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus, Device, Function,
> PCI_VENDOR_ID_OFFSET));
> > DeviceID = PciSegmentRead16
> (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus, Device, Function,
> PCI_DEVICE_ID_OFFSET));
> > if (VendorID == 0x && DeviceID == 0x) {
> > +if (Function == 0) {
> > + //
> > + // If function 0 is not implemented, do not scan other
> functions.
> > + //
> > + break;
> > +}
> > continue;
> > }
> >
> >
> 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 0/4] MdeModulePkg: add support for dispatching foreign arch PE/COFF images
On 13 September 2018 at 02:47, Shi, Steven wrote: > Hi Ard, > Just my curious, are you supporting the below idea of QEMU in UEFI? > > QEMU in UEFI - Alexander Graf > https://www.youtube.com/watch?v=uxvAH1Q4Mx0 > http://events17.linuxfoundation.org/sites/events/files/slides/QEMU%20in%20UEFI.pdf > Yes. X86EmulatorPkg was developed by Alexander and myself. >> -Original Message- >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of >> Ard Biesheuvel >> Sent: Wednesday, September 12, 2018 9:22 PM >> To: edk2-devel@lists.01.org >> Cc: Ni, Ruiyu ; Zimmer, Vincent >> ; Dong, Eric ; Andrew >> Fish ; ag...@suse.de; Richardson, Brian >> ; Kinney, Michael D >> ; Laszlo Ersek ; Zeng, >> Star >> Subject: [edk2] [PATCH 0/4] MdeModulePkg: add support for dispatching >> foreign arch PE/COFF images >> >> Add the basic plumbing to DXE core, the PCI bus driver and the boot manager >> to allow PE/COFF images to be dispatched that target an architecture that is >> not native for the platform, but which is supported by an emulator. >> >> One implementation of such an emulator can be found here: >> https://github.com/ardbiesheuvel/X86EmulatorPkg >> >> Cc: Zimmer Vincent >> Cc: Brian Richardson >> Cc: Michael D Kinney >> Cc: Andrew Fish >> Cc: Laszlo Ersek >> Cc: Leif Lindholm >> Cc: Star Zeng >> Cc: Eric Dong >> Cc: Ruiyu Ni >> >> Ard Biesheuvel (4): >> MdeModulePkg: introduce PE/COFF image emulator protocol >> MdeModulePkg/DxeCore: invoke the emulator protocol for foreign images >> MdeModulePkg/PciBusDxe: invoke PE/COFF emulator for foreign option >> ROMs >> MdeModulePkg/UefiBootManagerLib: allow foreign Driver images >> >> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 + >> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 + >> .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c | 16 +- >> MdeModulePkg/Core/Dxe/DxeMain.h | 3 ++ >> MdeModulePkg/Core/Dxe/DxeMain.inf | 1 + >> MdeModulePkg/Core/Dxe/Image/Image.c | 39 +++--- >> .../Include/Protocol/PeCoffImageEmulator.h| 51 +++ >> .../Library/UefiBootManagerLib/BmLoadOption.c | 26 +- >> .../Library/UefiBootManagerLib/InternalBm.h | 1 + >> .../UefiBootManagerLib/UefiBootManagerLib.inf | 1 + >> MdeModulePkg/MdeModulePkg.dec | 4 ++ >> 11 files changed, 136 insertions(+), 8 deletions(-) >> create mode 100644 >> MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h >> >> -- >> 2.17.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][edk2-platforms/devel-MinnowBoardMax-UDK2017 Change BIOS version
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Guo Mang --- Vlv2TbltDevicePkg/BiosId.env | 2 +- Vlv2TbltDevicePkg/BiosIdD.env| 2 +- Vlv2TbltDevicePkg/BiosIdR.env| 2 +- Vlv2TbltDevicePkg/BiosIdx64D.env | 2 +- Vlv2TbltDevicePkg/BiosIdx64R.env | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Vlv2TbltDevicePkg/BiosId.env b/Vlv2TbltDevicePkg/BiosId.env index 0a826b3..35314db 100644 --- a/Vlv2TbltDevicePkg/BiosId.env +++ b/Vlv2TbltDevicePkg/BiosId.env @@ -24,6 +24,6 @@ BOARD_ID = MNW2MAX BOARD_REV = 1 BUILD_TYPE= D -VERSION_MAJOR = 0098 +VERSION_MAJOR = 0099 VERSION_MINOR = 01 BOARD_EXT = X64 diff --git a/Vlv2TbltDevicePkg/BiosIdD.env b/Vlv2TbltDevicePkg/BiosIdD.env index 2a36a93..0e8f680 100644 --- a/Vlv2TbltDevicePkg/BiosIdD.env +++ b/Vlv2TbltDevicePkg/BiosIdD.env @@ -35,5 +35,5 @@ OEM_ID= I32 BUILD_TYPE= D BOARD_ID = BLAKCRB -VERSION_MAJOR = 0098 +VERSION_MAJOR = 0099 VERSION_MINOR = 01 diff --git a/Vlv2TbltDevicePkg/BiosIdR.env b/Vlv2TbltDevicePkg/BiosIdR.env index 8219763..1ba47cb 100644 --- a/Vlv2TbltDevicePkg/BiosIdR.env +++ b/Vlv2TbltDevicePkg/BiosIdR.env @@ -35,5 +35,5 @@ OEM_ID= I32 BUILD_TYPE= R BOARD_ID = BLAKCRB -VERSION_MAJOR = 0098 +VERSION_MAJOR = 0099 VERSION_MINOR = 01 diff --git a/Vlv2TbltDevicePkg/BiosIdx64D.env b/Vlv2TbltDevicePkg/BiosIdx64D.env index bfcefd6..ba9ce76 100644 --- a/Vlv2TbltDevicePkg/BiosIdx64D.env +++ b/Vlv2TbltDevicePkg/BiosIdx64D.env @@ -25,6 +25,6 @@ BOARD_REV = 1 OEM_ID= X64 BUILD_TYPE= D -VERSION_MAJOR = 0098 +VERSION_MAJOR = 0099 VERSION_MINOR = 01 BOARD_ID = BBAYCRB diff --git a/Vlv2TbltDevicePkg/BiosIdx64R.env b/Vlv2TbltDevicePkg/BiosIdx64R.env index 14266c5..4ccb2a0 100644 --- a/Vlv2TbltDevicePkg/BiosIdx64R.env +++ b/Vlv2TbltDevicePkg/BiosIdx64R.env @@ -25,6 +25,6 @@ BOARD_REV = 1 OEM_ID= X64 BUILD_TYPE= R -VERSION_MAJOR = 0098 +VERSION_MAJOR = 0099 VERSION_MINOR = 01 BOARD_ID = BBAYCRB -- 2.10.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch][edk2-platforms/devel-MinnowBoardMax-UDK2017] Generate UUID
Correct one field of UUID.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Guo Mang
CC: David Wei
---
.../SmBiosMiscDxe/MiscSystemManufacturerFunction.c | 30 ++
1 file changed, 13 insertions(+), 17 deletions(-)
diff --git a/Vlv2TbltDevicePkg/SmBiosMiscDxe/MiscSystemManufacturerFunction.c
b/Vlv2TbltDevicePkg/SmBiosMiscDxe/MiscSystemManufacturerFunction.c
index 26da5ab..1199374 100644
--- a/Vlv2TbltDevicePkg/SmBiosMiscDxe/MiscSystemManufacturerFunction.c
+++ b/Vlv2TbltDevicePkg/SmBiosMiscDxe/MiscSystemManufacturerFunction.c
@@ -414,34 +414,30 @@ AddSmbiosManuCallback (
ForType1InputData->SystemUuid.Data1 = (UINT32) StrHexToUint64 (Uuid);
ForType1InputData->SystemUuid.Data2 = (UINT16) StrHexToUint64 (Uuid + 9);
ForType1InputData->SystemUuid.Data3 = (UINT16) StrHexToUint64 (Uuid + 14);
-ForType1InputData->SystemUuid.Data4[0] = (UINT8) StrHexToUint64 (Uuid +
19);
+ForType1InputData->SystemUuid.Data4[0] = (UINT8) (StrHexToUint64 (Uuid +
19) >> 8);
ForType1InputData->SystemUuid.Data4[1] = (UINT8) StrHexToUint64 (Uuid +
21);
TempData = StrHexToUint64 (Uuid + 24);
for(Index = sizeof(ForType1InputData->SystemUuid.Data4)/sizeof(UINT8);
Index > 2; Index--) {
ForType1InputData->SystemUuid.Data4[Index-1] = (UINT8) TempData;
TempData = TempData >> 8;
}
-
-ForType1InputData->SystemUuid.Data4[0] = (UINT8) StrHexToUint64 (Uuid +
19);
-
-} else if (MacAddressString != NULL) {
- ForType1InputData->SystemUuid.Data1 = (UINT32)MacAddressString [0] +
(((UINT32)MacAddressString [1]) << 16);
- ForType1InputData->SystemUuid.Data2 = (UINT16)MacAddressString [2];
- ForType1InputData->SystemUuid.Data3 = (UINT16)MacAddressString [3];
- ForType1InputData->SystemUuid.Data4[0] = (UINT8)MacAddressString [4];
- ForType1InputData->SystemUuid.Data4[1] = (UINT8)(MacAddressString [5]);
- ForType1InputData->SystemUuid.Data4[2] = (UINT8)MacAddressString [6];
- ForType1InputData->SystemUuid.Data4[3] = (UINT8)(MacAddressString [7]);
- ForType1InputData->SystemUuid.Data4[4] = (UINT8)(MacAddressString [8]);
- ForType1InputData->SystemUuid.Data4[5] = (UINT8)(MacAddressString [9]);
- ForType1InputData->SystemUuid.Data4[6] = (UINT8)(MacAddressString [10]);
- ForType1InputData->SystemUuid.Data4[7] = (UINT8)(MacAddressString [11]);
+ } else if (MacAddressString != NULL) {
+ForType1InputData->SystemUuid.Data1 = (UINT32)MacAddressString [0] +
(((UINT32)MacAddressString [1]) << 16);
+ForType1InputData->SystemUuid.Data2 = (UINT16)MacAddressString [2];
+ForType1InputData->SystemUuid.Data3 = (UINT16)MacAddressString [3];
+ForType1InputData->SystemUuid.Data4[0] = (UINT8)MacAddressString [4];
+ForType1InputData->SystemUuid.Data4[1] = (UINT8)(MacAddressString [5]);
+ForType1InputData->SystemUuid.Data4[2] = (UINT8)MacAddressString [6];
+ForType1InputData->SystemUuid.Data4[3] = (UINT8)(MacAddressString [7]);
+ForType1InputData->SystemUuid.Data4[4] = (UINT8)(MacAddressString [8]);
+ForType1InputData->SystemUuid.Data4[5] = (UINT8)(MacAddressString [9]);
+ForType1InputData->SystemUuid.Data4[6] = (UINT8)(MacAddressString [10]);
+ForType1InputData->SystemUuid.Data4[7] = (UINT8)(MacAddressString [11]);
}
CopyMem ((UINT8 *) (>Uuid),>SystemUuid,16);
-
SmbiosRecord->WakeUpType = (UINT8)ForType1InputData->SystemWakeupType;
OptionalStrStart = (CHAR8 *)(SmbiosRecord + 1);
--
2.10.1.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when func 0 is not implemented
On 9/13/2018 10:10 AM, Star Zeng wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1169
PCI spec:
They are also required to always implement function 0 in the device.
Implementing other functions is optional and may be assigned in any
order (i.e., a two-function device must respond to function 0 but
can choose any of the other possible function numbers (1-7) for the
second function).
This patch updates ScanPciBus() to not scan other functions if
function 0 is not implemented.
Test done:
Added debug code below in the second loop of ScanPciBus(),
compared the debug logs with and without this patch, many
non-0 unimplemented functions are skipped correctly.
DEBUG ((
DEBUG_INFO,
"%a() B%02xD%02xF%02x VendorId: %04x DeviceId: %04x\n",
__FUNCTION__,
Bus,
Device,
Function,
VendorID,
DeviceID
));
Cc: Jiewen Yao
Cc: Rangasai V Chaganty
Cc: Tomson Chang
Cc: Jenny Huang
Cc: Amy Chan
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng
---
IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
index 36750b3f1d9c..305995de032c 100644
--- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
+++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
@@ -1,6 +1,6 @@
/** @file
- 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
which accompanies this distribution. The full text of the license may be
found at
@@ -247,6 +247,12 @@ ScanPciBus (
VendorID = PciSegmentRead16 (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus,
Device, Function, PCI_VENDOR_ID_OFFSET));
DeviceID = PciSegmentRead16 (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus,
Device, Function, PCI_DEVICE_ID_OFFSET));
if (VendorID == 0x && DeviceID == 0x) {
+if (Function == 0) {
+ //
+ // If function 0 is not implemented, do not scan other functions.
+ //
+ break;
+}
continue;
}
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 1/1] FatPkg/EnhancedFatDxe Fix Double Cluster Allocation
Reviewed-by: Ruiyu Ni
Thanks/Ray
> -Original Message-
> From: edk2-devel On Behalf Of
> Robinson, Herbie
> Sent: Friday, September 7, 2018 8:07 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 1/1] FatPkg/EnhancedFatDxe Fix Double Cluster
> Allocation
>
> This is a fix for a double cluster allocation when the disk is full. The
> double
> allocation happens because FatGrowEof calls FatAllocateCluster without
> immediately marking the each returned cluster as allocated. The fix is to
> move the FatSetFatEntry call inside the loop. I've also include some
> improvements to the sanity checks that I added while tracking this down.
> They are optional.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by:Herbie Robinson
> ---
> diff --git a/FatPkg/EnhancedFatDxe/FileSpace.c
> b/FatPkg/EnhancedFatDxe/FileSpace.c
> index 1254cd6..e17d3b6 100644
> --- a/FatPkg/EnhancedFatDxe/FileSpace.c
> +++ b/FatPkg/EnhancedFatDxe/FileSpace.c
> @@ -467,7 +467,7 @@ FatGrowEof (
>ClusterCount = 0;
>
>while (!FAT_END_OF_FAT_CHAIN (Cluster)) {
> -if (Cluster == FAT_CLUSTER_FREE || Cluster >= FAT_CLUSTER_SPECIAL) {
> +if (Cluster < FAT_MIN_CLUSTER || Cluster > Volume->MaxCluster + 1) {
>
>DEBUG (
> (EFI_D_INIT | EFI_D_ERROR,
> @@ -509,6 +509,11 @@ FatGrowEof (
> goto Done;
>}
>
> + if (NewCluster < FAT_MIN_CLUSTER || NewCluster > Volume-
> >MaxCluster + 1) {
> +Status = EFI_VOLUME_CORRUPTED;
> +goto Done;
> + }
> +
>if (LastCluster != 0) {
> FatSetFatEntry (Volume, LastCluster, NewCluster);
>} else {
> @@ -518,12 +523,21 @@ FatGrowEof (
>
>LastCluster = NewCluster;
>CurSize += 1;
> +
> + //
> + // Terminate the cluster list
> + //
> + // Note that we must do this EVERY time we allocate a cluster, because
> + // FatAllocateCluster scans the FAT looking for a free cluster and
> + // "LastCluster" is no longer free! Usually, FatAllocateCluster will
> + // start looking with the cluster after "LastCluster"; however, when
> + // there is only one free cluster left, it will find "LastCluster"
> + // a second time. There are other, less predictable scenarios
> + // where this could happen, as well.
> + //
> + FatSetFatEntry (Volume, LastCluster, (UINTN) FAT_CLUSTER_LAST);
> + OFile->FileLastCluster = LastCluster;
> }
> -//
> -// Terminate the cluster list
> -//
> -FatSetFatEntry (Volume, LastCluster, (UINTN) FAT_CLUSTER_LAST);
> -OFile->FileLastCluster = LastCluster;
>}
>
>OFile->FileSize = (UINTN) NewSizeInBytes;
> @@ -603,7 +617,7 @@ FatOFilePosition (
>Cluster = FatGetFatEntry (Volume, Cluster);
> }
>
> -if (Cluster < FAT_MIN_CLUSTER) {
> +if (Cluster < FAT_MIN_CLUSTER || Cluster > Volume->MaxCluster + 1) {
>return EFI_VOLUME_CORRUPTED;
> }
>
> ___
> 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] UefiCpuPkg/MtrrLib: Add flag to skip disabling MTRRs prior to MTRR change.
Leo, Sorry I was in leave yesterday so didn't see the mail. Which MSRs are shared? Only the MSR_IA32_MTRR_DEF_TYPE_REGISTER? Or all the MSRs that configures the CPU MTRR setting? I also agree with Laszlo's comments to fix the caller if all MSRs relating to MTRR are shared. That will be to fix MpInitLib and CpuDxe driver. Thanks/Ray > -Original Message- > From: Duran, Leo > Sent: Thursday, September 13, 2018 2:22 AM > To: Laszlo Ersek ; edk2-devel@lists.01.org > Cc: Dong, Eric ; Ni, Ruiyu > Subject: RE: [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling MTRRs > prior to MTRR change. > > > > > -Original Message- > > From: Laszlo Ersek > > Sent: Wednesday, September 12, 2018 12:59 PM > > To: Duran, Leo ; edk2-devel@lists.01.org > > Cc: Eric Dong ; Ruiyu Ni > > Subject: Re: [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling > > MTRRs prior to MTRR change. > > > > On 09/12/18 17:17, Duran, Leo wrote: > > > Laszlo, et al, > > > > > > Perhaps it would be best if I provide an example of the problem I'm > > > trying > > to solve, and perhaps we may chime in with suggestions. > > > > > > Again, > > > The fundamental issue has to do with shared MTRR control by set of > > threads within a core with SMT enabled. > > > So ideally only one thread (the first thread that wakes up) from a > > > set would > > configure the MSR, and other threads in the set would not need to. > > > > > > The problem with the existing code is that once the first thread > > > configures > > the MSR, another thread in the set follows and clears the ENABLE bit > > in MtrrLibPreMtrrChange(). > > > > Right, I think I understood that. What I don't understand is: > > > > > (Basically, the second thread pulls the rug from under the first thread). > > > > why it is a problem that, when the second thread wakes up on the same > > core, it repeats the whole MTRR configuration? It does clear the > > Enable bit temporarily, yes, but in the end it re-stores the exact > > same MTRR config. And while this happens, the first thread on the core > > (already past the MTRR > > setup) shouldn't be doing anything at all, because all logical CPUs > > should sit tight until all of them complete the MTRR setup. > > > > > > Now, of course, if the MTRR setup runs in parallel on all logical CPUs > > (I can't recall off-hand whether they are started up in parallel or > > one-by-one -- I think it's "free for all" though), and on various CPU > > models this modifies resources that are shared, then we have a more > > serious problem. Because then two threads on the same core may modify > > parts of the shared MTRR settings *other* than just the Enable bit. > > > > I also think it's a 'free for all" > (My understanding is that the BSP issues a broadcast to the APs.) > > Perhaps if the second thread waits for the first thread to complete and get > safely parked, the first thread may be then immune to the subsequent > changes. > > > Perhaps this should be solved by adding suitable exclusion, so that > > only one thread on each core configure the MTRR. In other words, > > continue to permit full parallelism between cores, but restrict each > > core to just one logical thread, when doing the MTRR changes. > > > > Yes, agreed! > That would be ideal for the case where threads in a core share MTRR > resources. > > Unfortunately, MtrrSetAllMtrrs() gets called from quite a few places, so > orchestrating the "only one logical thread" logic may be quite complicated. > Again, I presume that while the worker thread makes the MTRR changes, the > other threads must be safely parked and immune to toggling of the Enable > bit. > > > I wonder if the right approach is to discover the CPU topology > > up-front (based on the initial APIC IDs perhaps?), and to modify the > > MTRR setup callback for StartupAllAPs -- on the affected CPU models -- > > so that the callback returns without doing anything on the initially > "blacklisted" > > threads. > > > > By the time we are in MtrrLib, it's likely too late to catch the > > non-favored theads. > > > > (I'd very much like others to chime in too.) > > > > Thanks > > Laszlo > > > > > > > > I hope that helps explain what I'm after. > > > Leo. > > > > > >> -Original Message- > > >> From: Laszlo Ersek > > >> Sent: Wednesday, September 12, 2018 4:55 AM > > >> To: Duran, Leo ; edk2-devel@lists.01.org > > >> Cc: Eric Dong ; Ruiyu Ni > > >> Subject: Re: [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling > > >> MTRRs prior to MTRR change. > > >> > > >> On 09/11/18 21:47, Duran, Leo wrote: > > >>> > > >>> > > -Original Message- > > From: Laszlo Ersek > > Sent: Tuesday, September 11, 2018 1:50 PM > > To: Duran, Leo ; edk2-devel@lists.01.org > > Cc: Eric Dong ; Ruiyu Ni > > > > Subject: Re: [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip > > disabling MTRRs prior to MTRR change. > > > > On 09/11/18 17:41, Leo Duran wrote: > > > The default behavior is
Re: [edk2] [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value in HTTP header.
Reviewed-by: Ye Ting
-Original Message-
From: Wu, Jiaxin
Sent: Tuesday, September 4, 2018 3:17 PM
To: edk2-devel@lists.01.org
Cc: Stephen Benjamin ; Laszlo Ersek ;
Ye, Ting ; Fu, Siyuan ; Wu, Jiaxin
Subject: [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value in
HTTP header.
This patch is to resolve the lock-up issue if the value of HTTP header is
blank. The issue is recorded @
https://bugzilla.tianocore.org/show_bug.cgi?id=1102.
Cc: Stephen Benjamin
Cc: Laszlo Ersek
Cc: Ye Ting
Cc: Fu Siyuan
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wu Jiaxin
---
MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 57 +++-
1 file changed, 44 insertions(+), 13 deletions(-)
diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
index 5fbb50d03a..2fc3da8a2d 100644
--- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
+++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
@@ -1595,63 +1595,94 @@ HttpGetFieldNameAndValue (
return NULL;
}
//
// Each header field consists of a name followed by a colon (":") and the
field value.
+ // The field value MAY be preceded by any amount of LWS, though a single SP
is preferred.
+ //
+ // message-header = field-name ":" [ field-value ] // field-name =
+ token // field-value = *( field-content | LWS ) // // Note:
+ "*(element)" allows any number element, including zero; "1*(element)"
requires at least one element.
+ // [element] means element is optional.
+ // LWS = [CRLF] 1*(SP|HT), it can be ' ' or '\t' or '\r\n ' or
'\r\n\t'.
+ // CRLF = '\r\n'.
+ // SP = ' '.
+ // HT = '\t' (Tab).
//
FieldNameStr = String;
FieldValueStr = AsciiStrGetNextToken (FieldNameStr, ':');
if (FieldValueStr == NULL) {
return NULL;
}
//
- // Replace ':' with 0
+ // Replace ':' with 0, then FieldName has been retrived from String.
//
*(FieldValueStr - 1) = 0;
//
- // The field value MAY be preceded by any amount of LWS, though a single SP
is preferred.
- // Note: LWS = [CRLF] 1*(SP|HT), it can be '\r\n ' or '\r\n\t' or ' ' or
'\t'.
- // CRLF = '\r\n'.
- // SP = ' '.
- // HT = '\t' (Tab).
+ // Handle FieldValueStr, skip all the preceded LWS.
//
while (TRUE) {
if (*FieldValueStr == ' ' || *FieldValueStr == '\t') {
//
// Boundary condition check.
//
if ((UINTN) EndofHeader - (UINTN) FieldValueStr < 1) {
+//
+// Wrong String format!
+//
return NULL;
}
FieldValueStr ++;
} else if (*FieldValueStr == '\r') {
//
// Boundary condition check.
//
if ((UINTN) EndofHeader - (UINTN) FieldValueStr < 3) {
-return NULL;
+//
+// No more preceded LWS, so break here.
+//
+break;
}
- if (*(FieldValueStr + 1) == '\n' && (*(FieldValueStr + 2) == ' ' ||
*(FieldValueStr + 2) == '\t')) {
-FieldValueStr = FieldValueStr + 3;
+ if (*(FieldValueStr + 1) == '\n' ) {
+if (*(FieldValueStr + 2) == ' ' || *(FieldValueStr + 2) == '\t') {
+ FieldValueStr = FieldValueStr + 3;
+} else {
+ //
+ // No more preceded LWS, so break here.
+ //
+ break;
+}
+ } else {
+//
+// Wrong String format!
+//
+return NULL;
}
} else {
+ //
+ // No more preceded LWS, so break here.
+ //
break;
}
}
- //
- // Header fields can be extended over multiple lines by preceding each extra
- // line with at least one SP or HT.
- //
StrPtr = FieldValueStr;
do {
+//
+// Handle the LWS within the field value.
+//
StrPtr = AsciiStrGetNextToken (StrPtr, '\r');
if (StrPtr == NULL || *StrPtr != '\n') {
+ //
+ // Wrong String format!
+ //
return NULL;
}
StrPtr++;
} while (*StrPtr == ' ' || *StrPtr == '\t');
--
2.17.1.windows.2
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch 3/3] MdeModulePkg: Avoid key notification called more than once
Reviewed-by: Ruiyu Ni
Thanks/Ray
> -Original Message-
> From: Bi, Dandan
> Sent: Monday, September 10, 2018 3:12 PM
> To: edk2-devel@lists.01.org
> Cc: Bi, Dandan ; Ni, Ruiyu ; Zeng,
> Star
> Subject: [patch 3/3] MdeModulePkg: Avoid key notification called more than
> once
>
> From: Dandan Bi
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=996
>
> Issue:
> In current code logic, when a key is pressed, it will search the whole
> NotifyList to find whether a notification has been registered with the
> keystroke. if yes, it will en-queue the key for notification execution later.
> And now if different notification functions have been registered with the
> same key, then the key will be en-queued more than once. Then it will cause
> the notification executed more than once.
>
> This patch is to enhance the code logic to fix this issue.
>
> Cc: Ruiyu Ni
> Cc: Star Zeng
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi
> ---
> MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c| 1 +
> MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 1 +
> MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c | 1 +
> 3 files changed, 3 insertions(+)
>
> diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
> b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
> index 2ee293d64d..53cb6f0b48 100644
> --- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
> +++ b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
> @@ -1449,10 +1449,11 @@ KeyGetchar (
>// while current TPL is TPL_NOTIFY. It will be invoked in
>// KeyNotifyProcessHandler() which runs at TPL_CALLBACK.
>//
>PushEfikeyBufTail (>EfiKeyQueueForNotify, );
>gBS->SignalEvent (ConsoleIn->KeyNotifyProcessEvent);
> + break;
> }
>}
>
>PushEfikeyBufTail (>EfiKeyQueue, ); } diff --git
> a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> index b3b5fb9ff4..9cb4b5db6b 100644
> --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> @@ -1693,10 +1693,11 @@ UsbKeyCodeToEfiInputKey (
>// while current TPL is TPL_NOTIFY. It will be invoked in
>// KeyNotifyProcessHandler() which runs at TPL_CALLBACK.
>//
>Enqueue (>EfiKeyQueueForNotify, KeyData, sizeof
> (*KeyData));
>gBS->SignalEvent (UsbKeyboardDevice->KeyNotifyProcessEvent);
> + break;
> }
>}
>
>return EFI_SUCCESS;
> }
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
> index 33f9b6e585..d681a3039e 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
> @@ -985,10 +985,11 @@ EfiKeyFiFoInsertOneKey (
>// while current TPL is TPL_NOTIFY. It will be invoked in
>// KeyNotifyProcessHandler() which runs at TPL_CALLBACK.
>//
>EfiKeyFiFoForNotifyInsertOneKey (TerminalDevice->EfiKeyFiFoForNotify,
> Key);
>gBS->SignalEvent (TerminalDevice->KeyNotifyProcessEvent);
> + break;
> }
>}
>if (IsEfiKeyFiFoFull (TerminalDevice)) {
> //
> // Efi Key FIFO is full
> --
> 2.14.3.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch 1/3] EmbeddedPkg/VirtualKeyboard: Avoid notification called more than once
Reviewed-by: Ruiyu Ni Thanks/Ray > -Original Message- > From: Bi, Dandan > Sent: Monday, September 10, 2018 3:12 PM > To: edk2-devel@lists.01.org > Cc: Bi, Dandan ; Ni, Ruiyu ; Ard > Biesheuvel ; Leif Lindholm > > Subject: [patch 1/3] EmbeddedPkg/VirtualKeyboard: Avoid notification called > more than once > > From: Dandan Bi > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=996 > > Issue: > In current code logic, when a key is pressed, it will search the whole > NotifyList to find whether a notification has been registered with the > keystroke. if yes, it will en-queue the key for notification execution later. > And now if different notification functions have been registered with the > same key, then the key will be en-queued more than once. Then it will cause > the notification executed more than once. > > This patch is to enhance the code logic to fix this issue. > > Cc: Ruiyu Ni > Cc: Ard Biesheuvel > Cc: Leif Lindholm > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Dandan Bi > --- > EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.c > b/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.c > index 6609bc8dbe..daea9c47d2 100644 > --- a/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.c > +++ b/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.c > @@ -1,9 +1,9 @@ > /** @file >VirtualKeyboard driver > > -Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved. > +Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. > Copyright (c) 2018, Linaro Ltd. All rights reserved. > > This program and the accompanying materials are licensed and made > available under the terms and conditions of the BSD License which > accompanies this distribution. The @@ -1043,10 +1043,11 @@ > VirtualKeyboardTimerHandler ( >// while current TPL is TPL_NOTIFY. It will be invoked in >// KeyNotifyProcessHandler() which runs at TPL_CALLBACK. >// >Enqueue (>QueueForNotify, ); >gBS->SignalEvent (VirtualKeyboardPrivate->KeyNotifyProcessEvent); > + break; > } >} > >Enqueue (>Queue, ); > > -- > 2.14.3.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch 2/3] IntelFrameworkModulePkg: Avoid key notification called more than once
Reviewed-by: Ruiyu Ni Thanks/Ray > -Original Message- > From: Bi, Dandan > Sent: Monday, September 10, 2018 3:12 PM > To: edk2-devel@lists.01.org > Cc: Bi, Dandan ; Ni, Ruiyu ; Gao, > Liming > Subject: [patch 2/3] IntelFrameworkModulePkg: Avoid key notification called > more than once > > From: Dandan Bi > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=996 > > Issue: > In current code logic, when a key is pressed, it will search the whole > NotifyList to find whether a notification has been registered with the > keystroke. if yes, it will en-queue the key for notification execution later. > And now if different notification functions have been registered with the > same key, then the key will be en-queued more than once. Then it will cause > the notification executed more than once. > > This patch is to enhance the code logic to fix this issue. > > Cc: Ruiyu Ni > Cc: Liming Gao > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Dandan Bi > --- > IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c | 1 + > IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c > | 1 + > 2 files changed, 2 insertions(+) > > diff --git > a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c > b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c > index 202588191e..fddb0b21fb 100644 > --- a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c > +++ b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c > @@ -1485,10 +1485,11 @@ KeyGetchar ( >// while current TPL is TPL_NOTIFY. It will be invoked in >// KeyNotifyProcessHandler() which runs at TPL_CALLBACK. >// >PushEfikeyBufTail (>EfiKeyQueueForNotify, ); >gBS->SignalEvent (ConsoleIn->KeyNotifyProcessEvent); > + break; > } >} > >PushEfikeyBufTail (>EfiKeyQueue, ); } diff --git > a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard. > c > b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard. > c > index 63f6303995..bee5f8f5e5 100644 > --- > a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard. > c > +++ > b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard. > c > @@ -1984,10 +1984,11 @@ BiosKeyboardTimerHandler ( >// while current TPL is TPL_NOTIFY. It will be invoked in >// KeyNotifyProcessHandler() which runs at TPL_CALLBACK. >// >Enqueue (>QueueForNotify, ); >gBS->SignalEvent (BiosKeyboardPrivate->KeyNotifyProcessEvent); > + break; > } >} > >Enqueue (>Queue, ); > > -- > 2.14.3.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] IntelSiliconPkg IntelVTdDxe: Optimize when func 0 is not implemented
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1169
PCI spec:
They are also required to always implement function 0 in the device.
Implementing other functions is optional and may be assigned in any
order (i.e., a two-function device must respond to function 0 but
can choose any of the other possible function numbers (1-7) for the
second function).
This patch updates ScanPciBus() to not scan other functions if
function 0 is not implemented.
Test done:
Added debug code below in the second loop of ScanPciBus(),
compared the debug logs with and without this patch, many
non-0 unimplemented functions are skipped correctly.
DEBUG ((
DEBUG_INFO,
"%a() B%02xD%02xF%02x VendorId: %04x DeviceId: %04x\n",
__FUNCTION__,
Bus,
Device,
Function,
VendorID,
DeviceID
));
Cc: Jiewen Yao
Cc: Rangasai V Chaganty
Cc: Tomson Chang
Cc: Jenny Huang
Cc: Amy Chan
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng
---
IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
index 36750b3f1d9c..305995de032c 100644
--- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
+++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
@@ -1,6 +1,6 @@
/** @file
- 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
which accompanies this distribution. The full text of the license may be
found at
@@ -247,6 +247,12 @@ ScanPciBus (
VendorID = PciSegmentRead16 (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus,
Device, Function, PCI_VENDOR_ID_OFFSET));
DeviceID = PciSegmentRead16 (PCI_SEGMENT_LIB_ADDRESS(Segment, Bus,
Device, Function, PCI_DEVICE_ID_OFFSET));
if (VendorID == 0x && DeviceID == 0x) {
+if (Function == 0) {
+ //
+ // If function 0 is not implemented, do not scan other functions.
+ //
+ break;
+}
continue;
}
--
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 1/1] BaseTools\GenFds: remove extra content
Reviewed-by: Yonghong Zhu
Best Regards,
Zhu Yonghong
-Original Message-
From: Carsey, Jaben
Sent: Tuesday, September 11, 2018 6:16 AM
To: edk2-devel@lists.01.org
Cc: Zhu, Yonghong ; Gao, Liming
Subject: [PATCH v1 1/1] BaseTools\GenFds: remove extra content
remove uncalled functions
remove extra blank lines
remove commented out code
Cc: Yonghong Zhu
Cc: Liming Gao
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey
---
BaseTools/Source/Python/GenFds/FdfParser.py | 56
1 file changed, 56 deletions(-)
diff --git a/BaseTools/Source/Python/GenFds/FdfParser.py
b/BaseTools/Source/Python/GenFds/FdfParser.py
index 7e1be659fca5..1f7d59bb51b0 100644
--- a/BaseTools/Source/Python/GenFds/FdfParser.py
+++ b/BaseTools/Source/Python/GenFds/FdfParser.py
@@ -1120,28 +1120,6 @@ class FdfParser:
else:
return False
-def __GetNextOp(self):
-# Skip leading spaces, if exist.
-self.__SkipWhiteSpace()
-if self.__EndOfFile():
-return False
-# Record the token start position, the position of the first non-space
char.
-StartPos = self.CurrentOffsetWithinLine
-while not self.__EndOfLine():
-TempChar = self.__CurrentChar()
-# Try to find the end char that is not a space
-if not str(TempChar).isspace():
-self.__GetOneChar()
-else:
-break
-else:
-return False
-
-if StartPos != self.CurrentOffsetWithinLine:
-self.__Token = self.__CurrentLine()[StartPos :
self.CurrentOffsetWithinLine]
-return True
-else:
-return False
## __GetNextGuid() method
#
# Get next token unit before a seperator
@@ -1247,28 +1225,6 @@ class FdfParser:
self.__UndoToken()
return False
-## __GetNextPcdName() method
-#
-# Get next PCD token space C name and PCD C name pair before a seperator
-# If found, the decimal data is put into self.__Token
-#
-# @param selfThe object pointer
-# @retval Tuple PCD C name and PCD token space C name pair
-#
-def __GetNextPcdName(self):
-if not self.__GetNextWord():
-raise Warning("expected format of
.", self.FileName, self.CurrentLineNumber)
-pcdTokenSpaceCName = self.__Token
-
-if not self.__IsToken( "."):
-raise Warning("expected format of
.", self.FileName, self.CurrentLineNumber)
-
-if not self.__GetNextWord():
-raise Warning("expected format of
.", self.FileName, self.CurrentLineNumber)
-pcdCName = self.__Token
-
-return (pcdCName, pcdTokenSpaceCName)
-
def __GetNextPcdSettings(self):
if not self.__GetNextWord():
raise Warning("expected format of
.", self.FileName, self.CurrentLineNumber) @@
-3681,7 +3637,6 @@ class FdfParser:
ModuleType.upper() + \
'.'+ \
TemplateName.upper() ] = RuleObj
-#self.Profile.RuleList.append(rule)
return True
## __GetModuleType() method
@@ -4139,7 +4094,6 @@ class FdfParser:
# @retval False Not able to find section statement
#
def __GetRuleEncapsulationSection(self, Rule):
-
if self.__IsKeyword( "COMPRESS"):
Type = "PI_STD"
if self.__IsKeyword("PI_STD") or self.__IsKeyword("PI_NONE"):
@@ -4207,7 +4161,6 @@ class FdfParser:
# @retval False Not able to find a VTF
#
def __GetVtf(self):
-
if not self.__GetNextToken():
return False
@@ -4279,7 +4232,6 @@ class FdfParser:
# @retval False Not able to find a component
#
def __GetComponentStatement(self, VtfObj):
-
if not self.__IsKeyword("COMP_NAME"):
return False
@@ -4413,7 +4365,6 @@ class FdfParser:
# @retval False Not able to find a OptionROM
#
def __GetOptionRom(self):
-
if not self.__GetNextToken():
return False
@@ -4454,7 +4405,6 @@ class FdfParser:
# @retval False Not able to find inf statement
#
def __GetOptRomInfStatement(self, Obj):
-
if not self.__IsKeyword( "INF"):
return False
@@ -4557,7 +4507,6 @@ class FdfParser:
# @retval False Not able to find FILE statement
#
def __GetOptRomFileStatement(self, Obj):
-
if not self.__IsKeyword( "FILE"):
return False
@@ -4592,7 +4541,6 @@ class FdfParser:
# @retval CapList List of Capsule in FD
#
def __GetCapInFd (self, FdName):
-
CapList = []
if FdName.upper() in self.Profile.FdDict:
FdObj = self.Profile.FdDict[FdName.upper()]
@@ -4615,7 +4563,6 @@ class FdfParser:
# @param RefFvList referenced FV
[edk2] [PATCH] [edk2-platforms/devel-IntelAtomProcessorE3900] Fixed the klocwork issues.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunshan Tu
---
.../Common/Features/UsbDeviceDxe/XdciDWC.c | 12 ++--
.../PeiFspPolicyInitLib/PeiFspCpuPolicyInitLib.c | 1 +
.../PlatformBootManagerLib/PlatformBootManager.c | 2 ++
.../PlatformSettings/PlatformDxe/PciDevice.c | 2 +-
.../PlatformPreMemPei/BootMode.c | 7 ---
.../PlatformPreMemPei/FvCallback.c | 2 +-
.../Common/PlatformSmm/Platform.c| 2 +-
.../FspmWrapperPeim/FspmWrapperPeim.c| 3 ++-
.../NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c | 4 ++--
.../Cpu/PowerManagement/Smm/PowerMgmtDts.c | 16
.../Cpu/SmmAccess/Dxe/SmmAccessDriver.c | 6 +++---
.../Cpu/SmmAccess/Pei/SmmAccessDriver.c | 4 ++--
.../BaseConfigBlockLib/BaseConfigBlockLib.c | 4 ++--
.../Library/DxeSmbiosMemoryLib/SmbiosType16.c| 4 ++--
.../Universal/Variable/RuntimeDxe/Variable.c | 4 ++--
.../Library/BaseScSpiCommonLib/SpiCommon.c | 2 +-
.../SouthCluster/Library/DxeVtdLib/DxeVtdLib.c | 1 +
.../Library/Private/DxeScHdaLib/ScHdaLib.c | 4 +++-
.../ScPciExpressHelpersLibrary.c | 4 ++--
.../Library/ScPlatformLib/ScPlatformLibrary.c| 2 +-
.../BroxtonSiPkg/Txe/Heci/Smm/HeciSmm.c | 8
.../BroxtonSiPkg/Txe/Library/SeCLib/HeciMsgLib.c | 2 +-
22 files changed, 48 insertions(+), 48 deletions(-)
diff --git a/Platform/BroxtonPlatformPkg/Common/Features/UsbDeviceDxe/XdciDWC.c
b/Platform/BroxtonPlatformPkg/Common/Features/UsbDeviceDxe/XdciDWC.c
index 2c1e929ab7..2ade4434bd 100644
--- a/Platform/BroxtonPlatformPkg/Common/Features/UsbDeviceDxe/XdciDWC.c
+++ b/Platform/BroxtonPlatformPkg/Common/Features/UsbDeviceDxe/XdciDWC.c
@@ -22,7 +22,7 @@ UsbRegRead (
IN UINT32Offset
)
{
- volatile UINT32 *addr = (volatile UINT32 *)(UINTN)(Base + Offset);
+ volatile UINT32 *addr = (volatile UINT32 *)((UINTN)(Base) + (UINTN)(Offset));
return *addr;
}
@@ -33,7 +33,7 @@ UsbRegWrite (
IN UINT32val
)
{
- volatile UINT32 *addr = (volatile UINT32 *)(UINTN)(Base + Offset);
+ volatile UINT32 *addr = (volatile UINT32 *)((UINTN)(Base) + (UINTN)(Offset));
*addr = val;
}
@@ -1852,9 +1852,9 @@ DwcXdciCoreInit (
//
// Prepare a Buffer for SETUP packet
//
- LocalCoreHandle->Trbs = (DWC_XDCI_TRB *)(UINTN)((UINT32)(UINTN)
+ LocalCoreHandle->Trbs = (DWC_XDCI_TRB *)((UINTN)
LocalCoreHandle->UnalignedTrbs +
-(DWC_XDCI_TRB_BYTE_ALIGNMENT -
+(UINTN)(DWC_XDCI_TRB_BYTE_ALIGNMENT -
((UINT32)(UINTN)LocalCoreHandle->UnalignedTrbs %
DWC_XDCI_TRB_BYTE_ALIGNMENT)));
@@ -3954,9 +3954,9 @@ UsbXdciCoreReinit (
//
// Prepare a Buffer for SETUP packet
//
- LocalCoreHandle->Trbs = (DWC_XDCI_TRB *)(UINTN)((UINT32)(UINTN)
+ LocalCoreHandle->Trbs = (DWC_XDCI_TRB *)((UINTN)
LocalCoreHandle->UnalignedTrbs +
-(DWC_XDCI_TRB_BYTE_ALIGNMENT -
+(UINTN)(DWC_XDCI_TRB_BYTE_ALIGNMENT -
((UINT32)(UINTN)LocalCoreHandle->UnalignedTrbs %
DWC_XDCI_TRB_BYTE_ALIGNMENT)));
diff --git
a/Platform/BroxtonPlatformPkg/Common/Library/PeiFspPolicyInitLib/PeiFspCpuPolicyInitLib.c
b/Platform/BroxtonPlatformPkg/Common/Library/PeiFspPolicyInitLib/PeiFspCpuPolicyInitLib.c
index c404116029..7bf54ca37d 100644
---
a/Platform/BroxtonPlatformPkg/Common/Library/PeiFspPolicyInitLib/PeiFspCpuPolicyInitLib.c
+++
b/Platform/BroxtonPlatformPkg/Common/Library/PeiFspPolicyInitLib/PeiFspCpuPolicyInitLib.c
@@ -89,6 +89,7 @@ PeiFspCpuPolicyInit (
VariableSize = sizeof (SYSTEM_CONFIGURATION);
SystemConfiguration = AllocateZeroPool (VariableSize);
+ ASSERT (SystemConfiguration != NULL);
Status = VariableServices->GetVariable (
VariableServices,
diff --git
a/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
b/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
index 5201ac080d..821ed95bc4 100644
---
a/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
+++
b/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
@@ -524,6 +524,8 @@ ProcessTcgPp (
EFI_TCG2_PHYSICAL_PRESENCE Tcg2PpData;
EFI_PHYSICAL_PRESENCE TcgPpData;
UINTN TcgPpDataSize;
+
+ Status = EFI_SUCCESS;
TcgPpData.PPRequest = TCG_PHYSICAL_PRESENCE_NO_ACTION;
Tcg2PpData.PPRequest = TCG2_PHYSICAL_PRESENCE_NO_ACTION;
diff --git
a/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformDxe/PciDevice.c
b/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformDxe/PciDevice.c
index
Re: [edk2] [Patch] BaseTools: Align the boolean type PCD value's display in the report
Reviewed-by: Yonghong Zhu
Best Regards,
Zhu Yonghong
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yonghong
Zhu
Sent: Wednesday, September 12, 2018 9:50 AM
To: edk2-devel@lists.01.org
Cc: Gao, Liming
Subject: [edk2] [Patch] BaseTools: Align the boolean type PCD value's display
in the report
From: zhijufan
This patch align the boolean type PCD value's display in the build report.
Original it may display 0x0, also may use 0 for the same PCD.
Cc: Liming Gao
Cc: Yonghong Zhu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhiju.Fan
---
BaseTools/Source/Python/build/BuildReport.py | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/BaseTools/Source/Python/build/BuildReport.py
b/BaseTools/Source/Python/build/BuildReport.py
index a598d64..c7fa1b9 100644
--- a/BaseTools/Source/Python/build/BuildReport.py
+++ b/BaseTools/Source/Python/build/BuildReport.py
@@ -1012,11 +1012,11 @@ class PcdReport(object):
FileWrite(File, "")
FileWrite(File, Key)
First = False
-if Pcd.DatumType in TAB_PCD_CLEAN_NUMERIC_TYPES:
+if Pcd.DatumType in TAB_PCD_NUMERIC_TYPES:
PcdValueNumber = int(PcdValue.strip(), 0)
if DecDefaultValue is None:
DecMatch = True
else:
DecDefaultValueNumber = int(DecDefaultValue.strip(),
0) @@ -1100,10 +1100,19 @@ class PcdReport(object):
DecMatch = False
#
# Report PCD item according to their override relationship
#
+if Pcd.DatumType == 'BOOLEAN':
+if DscDefaultValue:
+DscDefaultValue = str(int(DscDefaultValue, 0))
+if DecDefaultValue:
+DecDefaultValue = str(int(DecDefaultValue, 0))
+if InfDefaultValue:
+InfDefaultValue = str(int(InfDefaultValue, 0))
+if Pcd.DefaultValue:
+Pcd.DefaultValue = str(int(Pcd.DefaultValue,
+ 0))
if DecMatch:
self.PrintPcdValue(File, Pcd, PcdTokenCName, TypeName,
IsStructure, DscMatch, DscDefaultValBak, InfMatch, InfDefaultValue, DecMatch,
DecDefaultValue, ' ')
elif InfDefaultValue and InfMatch:
self.PrintPcdValue(File, Pcd, PcdTokenCName, TypeName,
IsStructure, DscMatch, DscDefaultValBak, InfMatch, InfDefaultValue, DecMatch,
DecDefaultValue, '*M')
elif BuildOptionMatch:
@@ -1124,13 +1133,15 @@ class PcdReport(object):
continue
if not BuildOptionMatch:
ModuleOverride =
self.ModulePcdOverride.get((Pcd.TokenCName, Pcd.TokenSpaceGuidCName), {})
for ModulePath in ModuleOverride:
ModuleDefault = ModuleOverride[ModulePath]
-if Pcd.DatumType in TAB_PCD_CLEAN_NUMERIC_TYPES:
+if Pcd.DatumType in TAB_PCD_NUMERIC_TYPES:
ModulePcdDefaultValueNumber =
int(ModuleDefault.strip(), 0)
Match = (ModulePcdDefaultValueNumber ==
PcdValueNumber)
+if Pcd.DatumType == 'BOOLEAN':
+ModuleDefault =
+ str(ModulePcdDefaultValueNumber)
else:
Match = (ModuleDefault.strip() ==
PcdValue.strip())
if Match:
continue
IsByteArray, ArrayList =
ByteArrayForamt(ModuleDefault.strip())
@@ -1245,10 +1256,12 @@ class PcdReport(object):
if SkuInfo.DefaultStoreDict:
DefaultStoreList =
sorted(SkuInfo.DefaultStoreDict.keys())
for DefaultStore in DefaultStoreList:
Value = SkuInfo.DefaultStoreDict[DefaultStore]
IsByteArray, ArrayList = ByteArrayForamt(Value)
+if Pcd.DatumType == 'BOOLEAN':
+Value = str(int(Value, 0))
if FirstPrint:
FirstPrint = False
if IsByteArray:
if self.DefaultStoreSingle and
self.SkuSingle:
FileWrite(File, ' %-*s : %6s %10s =
%s' % (self.MaxLen, Flag + ' ' + PcdTokenCName, TypeName, '(' + Pcd.DatumType +
')', '{'))
@@ -1307,10 +1320,12 @@ class PcdReport(object):
self.PrintStructureInfo(File,
OverrideFieldStruct)
Re: [edk2] [PATCH 0/4] MdeModulePkg: add support for dispatching foreign arch PE/COFF images
Hi Ard, Just my curious, are you supporting the below idea of QEMU in UEFI? QEMU in UEFI - Alexander Graf https://www.youtube.com/watch?v=uxvAH1Q4Mx0 http://events17.linuxfoundation.org/sites/events/files/slides/QEMU%20in%20UEFI.pdf Steven Shi Intel\SSG\STO\UEFI Firmware Tel: +86 021-61166522 iNet: 821-6522 > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Ard Biesheuvel > Sent: Wednesday, September 12, 2018 9:22 PM > To: edk2-devel@lists.01.org > Cc: Ni, Ruiyu ; Zimmer, Vincent > ; Dong, Eric ; Andrew > Fish ; ag...@suse.de; Richardson, Brian > ; Kinney, Michael D > ; Laszlo Ersek ; Zeng, > Star > Subject: [edk2] [PATCH 0/4] MdeModulePkg: add support for dispatching > foreign arch PE/COFF images > > Add the basic plumbing to DXE core, the PCI bus driver and the boot manager > to allow PE/COFF images to be dispatched that target an architecture that is > not native for the platform, but which is supported by an emulator. > > One implementation of such an emulator can be found here: > https://github.com/ardbiesheuvel/X86EmulatorPkg > > Cc: Zimmer Vincent > Cc: Brian Richardson > Cc: Michael D Kinney > Cc: Andrew Fish > Cc: Laszlo Ersek > Cc: Leif Lindholm > Cc: Star Zeng > Cc: Eric Dong > Cc: Ruiyu Ni > > Ard Biesheuvel (4): > MdeModulePkg: introduce PE/COFF image emulator protocol > MdeModulePkg/DxeCore: invoke the emulator protocol for foreign images > MdeModulePkg/PciBusDxe: invoke PE/COFF emulator for foreign option > ROMs > MdeModulePkg/UefiBootManagerLib: allow foreign Driver images > > MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 + > MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 + > .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c | 16 +- > MdeModulePkg/Core/Dxe/DxeMain.h | 3 ++ > MdeModulePkg/Core/Dxe/DxeMain.inf | 1 + > MdeModulePkg/Core/Dxe/Image/Image.c | 39 +++--- > .../Include/Protocol/PeCoffImageEmulator.h| 51 +++ > .../Library/UefiBootManagerLib/BmLoadOption.c | 26 +- > .../Library/UefiBootManagerLib/InternalBm.h | 1 + > .../UefiBootManagerLib/UefiBootManagerLib.inf | 1 + > MdeModulePkg/MdeModulePkg.dec | 4 ++ > 11 files changed, 136 insertions(+), 8 deletions(-) > create mode 100644 > MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h > > -- > 2.17.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 2/5] OvmfPkg/PlatformPei: expire the use of PcdSetNxForStack
Laszlo,
> My understanding has been that it's not about enabling/disabling a CPU
> feature, but about marking specific pages as non-executable. Under that
> interpretation, it should be possible to mark pages used for stack
> purposes as non-executable, and leave everything else executable, even
> on x86.
I mentioned that because, for x86 cpu, IA32_EFER.NXE must be set before
marking any page as non-executable. But it's not true for current implementation
relating to PcdDxeNxMemoryProtectionPolicy. Anyway, it's not the key
point of what we've been talking about.
Thank you very much for the comments. I'll send out new patch soon.
Regards,
Jian
From: Laszlo Ersek [mailto:ler...@redhat.com]
Sent: Wednesday, September 12, 2018 6:42 PM
To: Wang, Jian J ; edk2-devel@lists.01.org
Cc: Ni, Ruiyu ; Justen, Jordan L
; Yao, Jiewen ; Zeng, Star
; Ard Biesheuvel
Subject: Re: [edk2] [PATCH 2/5] OvmfPkg/PlatformPei: expire the use of
PcdSetNxForStack
On 09/12/18 04:11, Wang, Jian J wrote:
> Hi Laszlo and Ard,
>
> Retiring the PcdSetNxForStack is not the intention of this patch series. It's
>just
> a side effect of solving problem stated in BZ#1116. Sorry again for the
>misleading
> title. I'm not insisting that we have to remove this PCD anyway (My personal
> opinion. Others might have different ones).
>
> I think I understand the importance of PcdSetNxForStack to arm/aarch64 now.
>And I
> agree that it'd be better to enable NX for stack independent of enabling NX
>for
> EfiBootServcesData memory. But since the stack is type of EfiBootServicesData,
> I think we should avoid any confusion in enabling/disabling NX for them.
>That's what
> BZ#1116 tries to complain about. But I'm still not clear any concrete
>suggestion on
> how to solve the BZ#1116 from all comment so far, if my patch series cannot
>satisfy
> all kinds of platforms. Simply keep PcdSetNxForStack doesn't help, I think.
>(It doesn't
> imply we must remove it.)
>
> As I commented in the BZ#1116, maybe we can just simply assert if there's one
> trying to disable NX for stack but enable NX for EfiBootServicesData at the
>same time,
> because it doesn't make sense.
Yes, that's what I thought as well. Catch the one case with an assert
and/or CpuDeadLoop() that is an invalid configuration.
> I think all other setting combinations won't have
> such confusion and don't need to take care specifically.
>
> And for x86 CPU, we'll always enable CPU NX feature if any NX related PCDs
>have bits
> set. Current DxeIpl will just enable NX for PcdSetNxForStack only.
My understanding has been that it's not about enabling/disabling a CPU
feature, but about marking specific pages as non-executable. Under that
interpretation, it should be possible to mark pages used for stack
purposes as non-executable, and leave everything else executable, even
on x86.
Laszlo
>
> Regards,
> Jian
>
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Tuesday, September 11, 2018 11:53 PM
> To: Wang, Jian J ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Justen, Jordan L
>; Yao, Jiewen ; Zeng, Star
>; Ard Biesheuvel
> Subject: Re: [edk2] [PATCH 2/5] OvmfPkg/PlatformPei: expire the use of
>PcdSetNxForStack
>
> On 09/11/18 07:16, Jian J Wang wrote:
>> BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116
>>
>> Since PcdSetNxForStack is expired, remove related config code.
>> PcdDxeNxMemoryProtectionPolicy cannot be used as dynamic PCD.
>> There's no need to add it in code to replace PcdSetNxForStack.
>>
>> Cc: Laszlo Ersek
>> Cc: Star Zeng
>> Cc: Jordan Justen
>> Cc: Ard Biesheuvel
>> Cc: Ruiyu Ni
>> Cc: Jiewen Yao
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Jian J Wang
>> ---
>> OvmfPkg/PlatformPei/Platform.c | 1 -
>> OvmfPkg/PlatformPei/PlatformPei.inf | 1 -
>> 2 files changed, 2 deletions(-)
>>
>> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
>> index 5a78668126..6627d236e0 100644
>> --- a/OvmfPkg/PlatformPei/Platform.c
>> +++ b/OvmfPkg/PlatformPei/Platform.c
>> @@ -340,7 +340,6 @@ NoexecDxeInitialization (
>> )
>> {
>> UPDATE_BOOLEAN_PCD_FROM_FW_CFG (PcdPropertiesTableEnable);
>> - UPDATE_BOOLEAN_PCD_FROM_FW_CFG (PcdSetNxForStack);
>> }
>>
>> VOID
>> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf
>>b/OvmfPkg/PlatformPei/PlatformPei.inf
>> index 9c5ad9961c..ef5dcb7693 100644
>> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
>> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf
>> @@ -96,7 +96,6 @@
>> gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
>> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode
>> gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable
>> - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack
>> gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable
>> gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable
>> gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask
>>
>
> I disagree with this
Re: [edk2] [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Update SmiEntry function run the same position
On 09/12/18 17:42, Gao, Liming wrote: > Laszlo: > Before commit e21e355e2ca7fefb15b4df7078f995d3fb9c2b89, jmp _SmiHandler is > commented. And below code, ASM_PFX(CpuSmmDebugEntry) is moved into rax, then > call it. But, this code doesn't work in XCODE5 tool chain. Like you say, > XCODE5 doesn't generated the absolute address in the EFI image. So, rax > stores the relative address. Once this logic is moved to another place, it > will not work. > ; jmp _SmiHandler ; instruction is not needed > ... > mov rax, ASM_PFX(CpuSmmDebugEntry) > callrax > > Commit e21e355e2ca7fefb15b4df7078f995d3fb9c2b89 is to support XCODE5. I > choose one tricky way to fix it. Although SmiEntry logic is copied to another > place and run, but here I enable jmp _SmiHandler to jmp the original code > location, then call ASM_PFX(CpuSmmDebugEntry) with the relative address can > work. > mov rax, strict qword 0 ; mov rax, _SmiHandler > _SmiHandlerAbsAddr: > jmp rax > ... > callASM_PFX(CpuSmmDebugEntry) > > Today, Jiewen raises the issue that SmiHandler should run in the copied > address, can't run in the common address. So, I update its logic and remove > jmp _SmiHandler, then keep code continue run in copied address. But, I still > need to fix up CpuSmmDebugEntry address to the absolute address. They are > both for this issue. They can't be separated. > > ... > mov rax, strict qword 0 ; callASM_PFX(CpuSmmDebugEntry) > CpuSmmDebugEntryAbsAddr: > callrax Thank you very much for the explanation. I understand now. I also understand why I got confused so much earlier. The reason is that the code was not commented sufficiently. It should have been pointed out what part of the instruction stream was meant to be executed from the copied address space (that is, not from the PiSmmCpuDxeSmm image itself), and what part of the instruction stream was meant to execute from within the the PiSmmCpuDxeSmm image. Without such comments, it's too hard to understand the transitions. You or Jiewen should please file a TianoCore BZ describing the current issue. Namely that, due to circumstances you are not allowed to share, no part of the _SmiHandler routine should execute from within the PiSmmCpuDxeSmm image; instead, all of it should execute from the copied-to address space. This requires eliminating the jump back into the PiSmmCpuDxeSmm image, and also patching up the relative addresses (to absolute addresses), for those instructions that used to run from within the PiSmmCpuDxeSmm image, but now no longer will. The necessary changes should not affect the behavior of platforms that already consume PiSmmCpuDxeSmm from edk2. The historical overview you provide above is also valuable, please include it in the commit message and/or the BZ as well. It would still be nice to comment what parts of the source file run from within the PiSmmCpuDxeSmm image. For example, PiSmmCpuSmiEntryFixupAddress() does, right? --*-- Please understand that my issue here is more serious than just missing the explanation / motivation, for this specific patch. My issue is that the posting of this work to edk2-devel should have *started* with the above explanation. You and Jiewen understand the issue. Noone else on the list (that doesn't work at your office) does. I've wasted half a day because you didn't write up the background up-front. I don't need to know your specific internal proprietary feature that requires this change. I do need to know that an internal feature with such a requirement exists, and that it is your motivation. Every ten minutes you save for yourselves on documentation is amplified to several hours of struggle, for each reviewer. And we specifically discussed this scenario with Mike at one of the earlier stewards' meetings. (The general topic back then was my raising that you [plural] liberally commit whatever you want to MdePkg / MdeModulePkg, without really naming the use cases, let alone adding client code to edk2. Whereas non-Intel contributors with a demonstrated need are heavily gated from adding code to MdePkg / MdeModulePkg. Compare: the addition of the "PciSegmentLibSegmentInfo" instances (commit 5c9bb86f171c), without any consumer in edk2 whatsoever, versus BZ#957, where the new libraries -- originally proposed for MdePkg -- had demonstrated consumers, just not from Intel. Do you see the double standard?) I wish I could just ignore UefiCpuPkg. I can't do that; it is very brittle / sensitive, and has caused numerous regressions for OVMF. I must keep a close eye on it. But patches from you [plural] certainly don't make that easy for me. It kills me that after years of us begging on the list, you [plural] still don't care about non-Intel participants as first class citizens up-front, until they raise a stink. Here's another recent example: https://bugzilla.tianocore.org/show_bug.cgi?id=1116#c3
Re: [edk2] [PATCH 0/4] MdeModulePkg: add support for dispatching foreign arch PE/COFF images
I like this approach too Vincent > On Sep 12, 2018, at 5:48 PM, Carsey, Jaben wrote: > > Reviewed-by: Jaben Carsey > > Code looks good to me. > > >> -Original Message- >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of >> Ard Biesheuvel >> Sent: Wednesday, September 12, 2018 6:22 AM >> To: edk2-devel@lists.01.org >> Cc: Ni, Ruiyu ; Zimmer, Vincent >> ; Dong, Eric ; Andrew >> Fish ; ag...@suse.de; Richardson, Brian >> ; Kinney, Michael D >> ; Laszlo Ersek ; Zeng, >> Star >> Subject: [edk2] [PATCH 0/4] MdeModulePkg: add support for dispatching >> foreign arch PE/COFF images >> >> Add the basic plumbing to DXE core, the PCI bus driver and the boot manager >> to allow PE/COFF images to be dispatched that target an architecture that is >> not native for the platform, but which is supported by an emulator. >> >> One implementation of such an emulator can be found here: >> https://github.com/ardbiesheuvel/X86EmulatorPkg >> >> Cc: Zimmer Vincent >> Cc: Brian Richardson >> Cc: Michael D Kinney >> Cc: Andrew Fish >> Cc: Laszlo Ersek >> Cc: Leif Lindholm >> Cc: Star Zeng >> Cc: Eric Dong >> Cc: Ruiyu Ni >> >> Ard Biesheuvel (4): >> MdeModulePkg: introduce PE/COFF image emulator protocol >> MdeModulePkg/DxeCore: invoke the emulator protocol for foreign images >> MdeModulePkg/PciBusDxe: invoke PE/COFF emulator for foreign option >>ROMs >> MdeModulePkg/UefiBootManagerLib: allow foreign Driver images >> >> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 + >> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 + >> .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c | 16 +- >> MdeModulePkg/Core/Dxe/DxeMain.h | 3 ++ >> MdeModulePkg/Core/Dxe/DxeMain.inf | 1 + >> MdeModulePkg/Core/Dxe/Image/Image.c | 39 +++--- >> .../Include/Protocol/PeCoffImageEmulator.h| 51 +++ >> .../Library/UefiBootManagerLib/BmLoadOption.c | 26 +- >> .../Library/UefiBootManagerLib/InternalBm.h | 1 + >> .../UefiBootManagerLib/UefiBootManagerLib.inf | 1 + >> MdeModulePkg/MdeModulePkg.dec | 4 ++ >> 11 files changed, 136 insertions(+), 8 deletions(-) >> create mode 100644 >> MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h >> >> -- >> 2.17.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] UefiCpuPkg/MtrrLib: Add flag to skip disabling MTRRs prior to MTRR change.
> -Original Message- > From: Laszlo Ersek > Sent: Wednesday, September 12, 2018 12:59 PM > To: Duran, Leo ; edk2-devel@lists.01.org > Cc: Eric Dong ; Ruiyu Ni > Subject: Re: [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling MTRRs > prior to MTRR change. > > On 09/12/18 17:17, Duran, Leo wrote: > > Laszlo, et al, > > > > Perhaps it would be best if I provide an example of the problem I'm trying > to solve, and perhaps we may chime in with suggestions. > > > > Again, > > The fundamental issue has to do with shared MTRR control by set of > threads within a core with SMT enabled. > > So ideally only one thread (the first thread that wakes up) from a set would > configure the MSR, and other threads in the set would not need to. > > > > The problem with the existing code is that once the first thread configures > the MSR, another thread in the set follows and clears the ENABLE bit in > MtrrLibPreMtrrChange(). > > Right, I think I understood that. What I don't understand is: > > > (Basically, the second thread pulls the rug from under the first thread). > > why it is a problem that, when the second thread wakes up on the same > core, it repeats the whole MTRR configuration? It does clear the Enable bit > temporarily, yes, but in the end it re-stores the exact same MTRR config. And > while this happens, the first thread on the core (already past the MTRR > setup) shouldn't be doing anything at all, because all logical CPUs should sit > tight until all of them complete the MTRR setup. > > > Now, of course, if the MTRR setup runs in parallel on all logical CPUs (I > can't > recall off-hand whether they are started up in parallel or one-by-one -- I > think it's "free for all" though), and on various CPU models this modifies > resources that are shared, then we have a more serious problem. Because > then two threads on the same core may modify parts of the shared MTRR > settings *other* than just the Enable bit. > I also think it's a 'free for all" (My understanding is that the BSP issues a broadcast to the APs.) Perhaps if the second thread waits for the first thread to complete and get safely parked, the first thread may be then immune to the subsequent changes. > Perhaps this should be solved by adding suitable exclusion, so that only one > thread on each core configure the MTRR. In other words, continue to permit > full parallelism between cores, but restrict each core to just one logical > thread, when doing the MTRR changes. > Yes, agreed! That would be ideal for the case where threads in a core share MTRR resources. Unfortunately, MtrrSetAllMtrrs() gets called from quite a few places, so orchestrating the "only one logical thread" logic may be quite complicated. Again, I presume that while the worker thread makes the MTRR changes, the other threads must be safely parked and immune to toggling of the Enable bit. > I wonder if the right approach is to discover the CPU topology up-front > (based on the initial APIC IDs perhaps?), and to modify the MTRR setup > callback for StartupAllAPs -- on the affected CPU models -- so that the > callback returns without doing anything on the initially "blacklisted" > threads. > > By the time we are in MtrrLib, it's likely too late to catch the non-favored > theads. > > (I'd very much like others to chime in too.) > > Thanks > Laszlo > > > > > I hope that helps explain what I'm after. > > Leo. > > > >> -Original Message- > >> From: Laszlo Ersek > >> Sent: Wednesday, September 12, 2018 4:55 AM > >> To: Duran, Leo ; edk2-devel@lists.01.org > >> Cc: Eric Dong ; Ruiyu Ni > >> Subject: Re: [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling > >> MTRRs prior to MTRR change. > >> > >> On 09/11/18 21:47, Duran, Leo wrote: > >>> > >>> > -Original Message- > From: Laszlo Ersek > Sent: Tuesday, September 11, 2018 1:50 PM > To: Duran, Leo ; edk2-devel@lists.01.org > Cc: Eric Dong ; Ruiyu Ni > Subject: Re: [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling > MTRRs prior to MTRR change. > > On 09/11/18 17:41, Leo Duran wrote: > > The default behavior is to disable MTRRs prior to an MTRR change. > > However, on SMT platforms with shared CPU resources it may be > > desirable to skip the default behavior, and leave the current > > state of the > Enable bit. > > > > Cc: Eric Dong > > Cc: Laszlo Ersek > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Leo Duran > > --- > > UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 10 +++--- > > UefiCpuPkg/Library/MtrrLib/MtrrLib.inf | 3 +++ > > UefiCpuPkg/UefiCpuPkg.dec | 7 +++ > > 3 files changed, 17 insertions(+), 3 deletions(-) > > > > diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c > > b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c > > index dfce9a9..baf9a0f 100644 > > --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c > >
Re: [edk2] [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling MTRRs prior to MTRR change.
On 09/12/18 17:17, Duran, Leo wrote:
> Laszlo, et al,
>
> Perhaps it would be best if I provide an example of the problem I'm trying to
> solve, and perhaps we may chime in with suggestions.
>
> Again,
> The fundamental issue has to do with shared MTRR control by set of threads
> within a core with SMT enabled.
> So ideally only one thread (the first thread that wakes up) from a set would
> configure the MSR, and other threads in the set would not need to.
>
> The problem with the existing code is that once the first thread configures
> the MSR, another thread in the set follows and clears the ENABLE bit in
> MtrrLibPreMtrrChange().
Right, I think I understood that. What I don't understand is:
> (Basically, the second thread pulls the rug from under the first thread).
why it is a problem that, when the second thread wakes up on the same
core, it repeats the whole MTRR configuration? It does clear the Enable
bit temporarily, yes, but in the end it re-stores the exact same MTRR
config. And while this happens, the first thread on the core (already
past the MTRR setup) shouldn't be doing anything at all, because all
logical CPUs should sit tight until all of them complete the MTRR setup.
Now, of course, if the MTRR setup runs in parallel on all logical CPUs
(I can't recall off-hand whether they are started up in parallel or
one-by-one -- I think it's "free for all" though), and on various CPU
models this modifies resources that are shared, then we have a more
serious problem. Because then two threads on the same core may modify
parts of the shared MTRR settings *other* than just the Enable bit.
Perhaps this should be solved by adding suitable exclusion, so that only
one thread on each core configure the MTRR. In other words, continue to
permit full parallelism between cores, but restrict each core to just
one logical thread, when doing the MTRR changes.
I wonder if the right approach is to discover the CPU topology up-front
(based on the initial APIC IDs perhaps?), and to modify the MTRR setup
callback for StartupAllAPs -- on the affected CPU models -- so that the
callback returns without doing anything on the initially "blacklisted"
threads.
By the time we are in MtrrLib, it's likely too late to catch the
non-favored theads.
(I'd very much like others to chime in too.)
Thanks
Laszlo
>
> I hope that helps explain what I'm after.
> Leo.
>
>> -Original Message-
>> From: Laszlo Ersek
>> Sent: Wednesday, September 12, 2018 4:55 AM
>> To: Duran, Leo ; edk2-devel@lists.01.org
>> Cc: Eric Dong ; Ruiyu Ni
>> Subject: Re: [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling MTRRs
>> prior to MTRR change.
>>
>> On 09/11/18 21:47, Duran, Leo wrote:
>>>
>>>
-Original Message-
From: Laszlo Ersek
Sent: Tuesday, September 11, 2018 1:50 PM
To: Duran, Leo ; edk2-devel@lists.01.org
Cc: Eric Dong ; Ruiyu Ni
Subject: Re: [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling
MTRRs prior to MTRR change.
On 09/11/18 17:41, Leo Duran wrote:
> The default behavior is to disable MTRRs prior to an MTRR change.
> However, on SMT platforms with shared CPU resources it may be
> desirable to skip the default behavior, and leave the current state
> of the
Enable bit.
>
> Cc: Eric Dong
> Cc: Laszlo Ersek
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Leo Duran
> ---
> UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 10 +++---
> UefiCpuPkg/Library/MtrrLib/MtrrLib.inf | 3 +++
> UefiCpuPkg/UefiCpuPkg.dec | 7 +++
> 3 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> index dfce9a9..baf9a0f 100644
> --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> @@ -6,6 +6,8 @@
> except for MtrrSetAllMtrrs() which is used to sync BSP's MTRR
> setting to
APs.
>
>Copyright (c) 2008 - 2018, Intel Corporation. All rights
> reserved.
> + Copyright (c) 2018, AMD Inc. All rights reserved.
> +
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of
> the
BSD License
>which accompanies this distribution. The full text of the
> license may be found at @@ -310,9 +312,11 @@ MtrrLibPreMtrrChange
>> (
>//
>// Disable MTRRs
>//
> - DefType.Uint64 = AsmReadMsr64 (MSR_IA32_MTRR_DEF_TYPE);
> - DefType.Bits.E = 0;
> - AsmWriteMsr64 (MSR_IA32_MTRR_DEF_TYPE, DefType.Uint64);
> + if (!PcdGetBool (PcdSkipDisableMtrrsOnPreMtrrChange)) {
> +DefType.Uint64 = AsmReadMsr64 (MSR_IA32_MTRR_DEF_TYPE);
> +DefType.Bits.E = 0;
> +AsmWriteMsr64 (MSR_IA32_MTRR_DEF_TYPE, DefType.Uint64); }
> }
>
> /**
> diff --git
Re: [edk2] [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling MTRRs prior to MTRR change.
Laszlo, et al,
Perhaps it would be best if I provide an example of the problem I'm trying to
solve, and perhaps we may chime in with suggestions.
Again,
The fundamental issue has to do with shared MTRR control by set of threads
within a core with SMT enabled.
So ideally only one thread (the first thread that wakes up) from a set would
configure the MSR, and other threads in the set would not need to.
The problem with the existing code is that once the first thread configures the
MSR, another thread in the set follows and clears the ENABLE bit in
MtrrLibPreMtrrChange().
(Basically, the second thread pulls the rug from under the first thread).
I hope that helps explain what I'm after.
Leo.
> -Original Message-
> From: Laszlo Ersek
> Sent: Wednesday, September 12, 2018 4:55 AM
> To: Duran, Leo ; edk2-devel@lists.01.org
> Cc: Eric Dong ; Ruiyu Ni
> Subject: Re: [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling MTRRs
> prior to MTRR change.
>
> On 09/11/18 21:47, Duran, Leo wrote:
> >
> >
> >> -Original Message-
> >> From: Laszlo Ersek
> >> Sent: Tuesday, September 11, 2018 1:50 PM
> >> To: Duran, Leo ; edk2-devel@lists.01.org
> >> Cc: Eric Dong ; Ruiyu Ni
> >> Subject: Re: [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling
> >> MTRRs prior to MTRR change.
> >>
> >> On 09/11/18 17:41, Leo Duran wrote:
> >>> The default behavior is to disable MTRRs prior to an MTRR change.
> >>> However, on SMT platforms with shared CPU resources it may be
> >>> desirable to skip the default behavior, and leave the current state
> >>> of the
> >> Enable bit.
> >>>
> >>> Cc: Eric Dong
> >>> Cc: Laszlo Ersek
> >>> Contributed-under: TianoCore Contribution Agreement 1.1
> >>> Signed-off-by: Leo Duran
> >>> ---
> >>> UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 10 +++---
> >>> UefiCpuPkg/Library/MtrrLib/MtrrLib.inf | 3 +++
> >>> UefiCpuPkg/UefiCpuPkg.dec | 7 +++
> >>> 3 files changed, 17 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> >>> b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> >>> index dfce9a9..baf9a0f 100644
> >>> --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> >>> +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> >>> @@ -6,6 +6,8 @@
> >>> except for MtrrSetAllMtrrs() which is used to sync BSP's MTRR
> >>> setting to
> >> APs.
> >>>
> >>>Copyright (c) 2008 - 2018, Intel Corporation. All rights
> >>> reserved.
> >>> + Copyright (c) 2018, AMD Inc. All rights reserved.
> >>> +
> >>>This program and the accompanying materials
> >>>are licensed and made available under the terms and conditions of
> >>> the
> >> BSD License
> >>>which accompanies this distribution. The full text of the
> >>> license may be found at @@ -310,9 +312,11 @@ MtrrLibPreMtrrChange
> (
> >>>//
> >>>// Disable MTRRs
> >>>//
> >>> - DefType.Uint64 = AsmReadMsr64 (MSR_IA32_MTRR_DEF_TYPE);
> >>> - DefType.Bits.E = 0;
> >>> - AsmWriteMsr64 (MSR_IA32_MTRR_DEF_TYPE, DefType.Uint64);
> >>> + if (!PcdGetBool (PcdSkipDisableMtrrsOnPreMtrrChange)) {
> >>> +DefType.Uint64 = AsmReadMsr64 (MSR_IA32_MTRR_DEF_TYPE);
> >>> +DefType.Bits.E = 0;
> >>> +AsmWriteMsr64 (MSR_IA32_MTRR_DEF_TYPE, DefType.Uint64); }
> >>> }
> >>>
> >>> /**
> >>> diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> >> b/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> >>> index a97cc61..06f33e8 100644
> >>> --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> >>> +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> >>> @@ -2,6 +2,8 @@
> >>> # MTRR library provides APIs for MTRR operation.
> >>> #
> >>> # Copyright (c) 2006 - 2018, Intel Corporation. All rights
> >>> reserved.
> >>> +# Copyright (c) 2018, AMD Inc. All rights reserved. #
> >>> # This program and the accompanying materials # are licensed and
> >>> made available under the terms and conditions of the
> >> BSD License
> >>> # which accompanies this distribution. The full text of the
> >>> license may be
> >> found at
> >>> @@ -43,4 +45,5 @@
> >>>
> >>> [Pcd]
> >>>
> gUefiCpuPkgTokenSpaceGuid.PcdCpuNumberOfReservedVariableMtrrs
> >> ## SOMETIMES_CONSUMES
> >>> + gUefiCpuPkgTokenSpaceGuid.PcdSkipDisableMtrrsOnPreMtrrChange
> >> ## CONSUMES
> >>>
> >>> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
> >>> index 69d777a..64ec763 100644
> >>> --- a/UefiCpuPkg/UefiCpuPkg.dec
> >>> +++ b/UefiCpuPkg/UefiCpuPkg.dec
> >>> @@ -2,6 +2,7 @@
> >>> # This Package provides UEFI compatible CPU modules and libraries.
> >>> #
> >>> # Copyright (c) 2007 - 2017, Intel Corporation. All rights
> >>> reserved.
> >>> +# Copyright (c) 2018, AMD Inc. All rights reserved.
> >>> #
> >>> # This program and the accompanying materials are licensed and made
> >> available under
> >>> # the terms and conditions of the BSD License which accompanies
> >>> this
> >> distribution.
> >>> @@ -273,6 +274,12 @@
> >>># @Prompt Current boot is a power-on reset.
> >>>
> >>
>
Re: [edk2] [PATCH] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value
Liming, Your analysis is correct. In the end there will be 2. Mike > -Original Message- > From: Gao, Liming > Sent: Wednesday, September 12, 2018 8:08 AM > To: Kinney, Michael D ; Ni, > Ruiyu ; 'edk2-devel@lists.01.org' > > Cc: Yao, Jiewen > Subject: RE: [PATCH] MdePkg/SynchronizationLib: fix > Interlocked[De|In]crement return value > > Mike: > For this case InternalSyncDecrement, there are three > implementations in SynchronizationLib library. > One is C inline assembly for GCC, another is intrinsic > function for MSFT, last one is nasm implementation for > INTEL compiler. We would like to keep one > implementation. If intrinsic one is required to be kept, > that means at least two implementations are required. > > Thanks > Liming > > -Original Message- > > From: Kinney, Michael D > > Sent: Wednesday, September 12, 2018 10:53 PM > > To: Ni, Ruiyu ; 'edk2- > de...@lists.01.org' ; Kinney, > Michael D > > > > Cc: Yao, Jiewen ; Gao, Liming > > > Subject: RE: [PATCH] MdePkg/SynchronizationLib: fix > Interlocked[De|In]crement return value > > > > Ray, > > > > Yes. This provides best size with optimizations > enabled. > > > > Mike > > > > > -Original Message- > > > From: Ni, Ruiyu > > > Sent: Tuesday, September 11, 2018 7:16 PM > > > To: Ni, Ruiyu ; Kinney, Michael > D > > > ; 'edk2- > de...@lists.01.org' > > > > > > Cc: Yao, Jiewen ; Gao, Liming > > > > > > Subject: RE: [PATCH] MdePkg/SynchronizationLib: fix > > > Interlocked[De|In]crement return value > > > > > > Mike, > > > Do you require to still use MSVC intrinsic function? > > > > > > > -Original Message- > > > > From: edk2-devel [mailto:edk2-devel- > > > boun...@lists.01.org] On Behalf Of Ni, > > > > Ruiyu > > > > Sent: Tuesday, September 11, 2018 10:27 AM > > > > To: Kinney, Michael D > ; > > > 'edk2-devel@lists.01.org' > > > > > > > > Cc: Yao, Jiewen ; Gao, > Liming > > > > > > > Subject: Re: [edk2] [PATCH] > MdePkg/SynchronizationLib: > > > fix > > > > Interlocked[De|In]crement return value > > > > > > > > The reason I didn't remove the GCC version is > because > > > Liming told me that there > > > > is a XCODE issue which prevents using NASM for > > > library. > > > > > > > > Thanks/Ray > > > > > > > > > -Original Message- > > > > > From: Ni, Ruiyu > > > > > Sent: Tuesday, September 11, 2018 10:25 AM > > > > > To: Kinney, Michael D > ; > > > edk2- > > > > > de...@lists.01.org > > > > > Cc: Yao, Jiewen ; Gao, > Liming > > > > > > > > > > Subject: RE: [PATCH] MdePkg/SynchronizationLib: > fix > > > > > Interlocked[De|In]crement return value > > > > > > > > > > The NASM does the exactly same as the MSFT > > > intrinsic. > > > > > I'd like to remove them because I was almost > lost in > > > so many versions > > > > > of Interlocked[De|In]crement. > > > > > I'd like to merge them to one version. > > > > > > > > > > Thanks/Ray > > > > > > > > > > > -Original Message- > > > > > > From: Kinney, Michael D > > > > > > Sent: Tuesday, September 11, 2018 12:39 AM > > > > > > To: Ni, Ruiyu ; edk2- > > > de...@lists.01.org; Kinney, > > > > > > Michael D > > > > > > Cc: Yao, Jiewen ; Gao, > > > Liming > > > > > > > > > > > > Subject: RE: [PATCH] > MdePkg/SynchronizationLib: > > > fix > > > > > > Interlocked[De|In]crement return value > > > > > > > > > > > > Ray, > > > > > > > > > > > > Why are we removing the use of intrinsics from > > > MSFT builds? We > > > > > > should keep those for more efficient code > > > generation if the > > > > > > intrinsic has the correct behavior. > > > > > > > > > > > > Thanks, > > > > > > > > > > > > Mike > > > > > > > > > > > > > -Original Message- > > > > > > > From: Ni, Ruiyu > > > > > > > Sent: Monday, September 10, 2018 3:06 AM > > > > > > > To: edk2-devel@lists.01.org > > > > > > > Cc: Yao, Jiewen ; Gao, > > > Liming > > > > > > > ; Kinney, Michael D > > > > > > > > > > > > > > Subject: [PATCH] MdePkg/SynchronizationLib: > fix > > > > > > > Interlocked[De|In]crement return value > > > > > > > > > > > > > > Today's > > > InterlockedIncrement()/InterlockedDecrement() > > > > > > > guarantees to > > > > > > > perform atomic increment/decrement but > doesn't > > > guarantee the > > > > > > > return value equals to the new value. > > > > > > > > > > > > > > The patch fixes the behavior to use "XADD" > > > instruction to > > > > > > > guarantee the return value equals to the new > > > value. > > > > > > > > > > > > > > Contributed-under: TianoCore Contribution > > > Agreement 1.1 > > > > > > > Signed-off-by: Ruiyu Ni > > > > > > > Cc: Jiewen Yao > > > > > > > Cc: Liming Gao > > > > > > > Cc: Michael D Kinney > > > > > > > > > > --- > > > > > > > MdePkg/Include/Library/SynchronizationLib.h > > > | 6 > > > > > > > +-- > > > > > > > .../BaseSynchronizationLib.inf > > > | 18 > > > > > > > - > > > > > > > .../BaseSynchronizationLibInternals.h > > > | 6 > > > > > > > +-- > > > > > > > .../BaseSynchronizationLib/Ia32/GccInline.c > > > | 18 > > > >
Re: [edk2] [PATCH 0/4] MdeModulePkg: add support for dispatching foreign arch PE/COFF images
On 12 September 2018 at 17:07, Carsey, Jaben wrote: > Ard, > > What happens when more than one emulators want to co-exist? > The protocol does not support that at the moment, and it is doubtful that it would work in practice: the X86-on-AARCH64 emulator works by mapping the X86 PE/COFF code regions as non-executable, and trapping the resulting exception to invoke the emulator. Running multiple emulators side by side would require a considerable amount of synchronization, so it only makes sense to consider that if there is a compelling use case. And if there is, we could still abstract from this at the top level (i.e., this protocol) ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 0/4] MdeModulePkg: add support for dispatching foreign arch PE/COFF images
Ard, I think there may be a lot of assumptions in this proposal that are not documented. I do not see any description of how an image is started or how calls between native code and emulated code are handled. Also, are multiple emulated CPUs supported? It looks like there can only be one instance of this new protocol. Can you please provide more detailed comments for the functions in the new protocol and document the assumptions and known limitation in the header? >From one perspective, EBC is an emulated instruction set. Would it make sense to have EBC be one of the emulated CPU types that can be plugged in? Thanks, Mike > -Original Message- > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > Sent: Wednesday, September 12, 2018 6:22 AM > To: edk2-devel@lists.01.org > Cc: ag...@suse.de; Ard Biesheuvel > ; Zimmer, Vincent > ; Richardson, Brian > ; Kinney, Michael D > ; Andrew Fish > ; Laszlo Ersek ; > Leif Lindholm ; Zeng, Star > ; Dong, Eric ; > Ni, Ruiyu > Subject: [PATCH 0/4] MdeModulePkg: add support for > dispatching foreign arch PE/COFF images > > Add the basic plumbing to DXE core, the PCI bus driver > and the boot manager > to allow PE/COFF images to be dispatched that target an > architecture that is > not native for the platform, but which is supported by > an emulator. > > One implementation of such an emulator can be found > here: > https://github.com/ardbiesheuvel/X86EmulatorPkg > > Cc: Zimmer Vincent > Cc: Brian Richardson > Cc: Michael D Kinney > Cc: Andrew Fish > Cc: Laszlo Ersek > Cc: Leif Lindholm > Cc: Star Zeng > Cc: Eric Dong > Cc: Ruiyu Ni > > Ard Biesheuvel (4): > MdeModulePkg: introduce PE/COFF image emulator > protocol > MdeModulePkg/DxeCore: invoke the emulator protocol for > foreign images > MdeModulePkg/PciBusDxe: invoke PE/COFF emulator for > foreign option > ROMs > MdeModulePkg/UefiBootManagerLib: allow foreign > Driver images > > MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 + > MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 + > .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c | 16 > +- > MdeModulePkg/Core/Dxe/DxeMain.h | 3 ++ > MdeModulePkg/Core/Dxe/DxeMain.inf | 1 + > MdeModulePkg/Core/Dxe/Image/Image.c | 39 > +++--- > .../Include/Protocol/PeCoffImageEmulator.h| 51 > +++ > .../Library/UefiBootManagerLib/BmLoadOption.c | 26 > +- > .../Library/UefiBootManagerLib/InternalBm.h | 1 + > .../UefiBootManagerLib/UefiBootManagerLib.inf | 1 + > MdeModulePkg/MdeModulePkg.dec | 4 ++ > 11 files changed, 136 insertions(+), 8 deletions(-) > create mode 100644 > MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h > > -- > 2.17.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value
Mike: For this case InternalSyncDecrement, there are three implementations in SynchronizationLib library. One is C inline assembly for GCC, another is intrinsic function for MSFT, last one is nasm implementation for INTEL compiler. We would like to keep one implementation. If intrinsic one is required to be kept, that means at least two implementations are required. Thanks Liming > -Original Message- > From: Kinney, Michael D > Sent: Wednesday, September 12, 2018 10:53 PM > To: Ni, Ruiyu ; 'edk2-devel@lists.01.org' > ; Kinney, Michael D > > Cc: Yao, Jiewen ; Gao, Liming > Subject: RE: [PATCH] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement > return value > > Ray, > > Yes. This provides best size with optimizations enabled. > > Mike > > > -Original Message- > > From: Ni, Ruiyu > > Sent: Tuesday, September 11, 2018 7:16 PM > > To: Ni, Ruiyu ; Kinney, Michael D > > ; 'edk2-devel@lists.01.org' > > > > Cc: Yao, Jiewen ; Gao, Liming > > > > Subject: RE: [PATCH] MdePkg/SynchronizationLib: fix > > Interlocked[De|In]crement return value > > > > Mike, > > Do you require to still use MSVC intrinsic function? > > > > > -Original Message- > > > From: edk2-devel [mailto:edk2-devel- > > boun...@lists.01.org] On Behalf Of Ni, > > > Ruiyu > > > Sent: Tuesday, September 11, 2018 10:27 AM > > > To: Kinney, Michael D ; > > 'edk2-devel@lists.01.org' > > > > > > Cc: Yao, Jiewen ; Gao, Liming > > > > > Subject: Re: [edk2] [PATCH] MdePkg/SynchronizationLib: > > fix > > > Interlocked[De|In]crement return value > > > > > > The reason I didn't remove the GCC version is because > > Liming told me that there > > > is a XCODE issue which prevents using NASM for > > library. > > > > > > Thanks/Ray > > > > > > > -Original Message- > > > > From: Ni, Ruiyu > > > > Sent: Tuesday, September 11, 2018 10:25 AM > > > > To: Kinney, Michael D ; > > edk2- > > > > de...@lists.01.org > > > > Cc: Yao, Jiewen ; Gao, Liming > > > > > > > > Subject: RE: [PATCH] MdePkg/SynchronizationLib: fix > > > > Interlocked[De|In]crement return value > > > > > > > > The NASM does the exactly same as the MSFT > > intrinsic. > > > > I'd like to remove them because I was almost lost in > > so many versions > > > > of Interlocked[De|In]crement. > > > > I'd like to merge them to one version. > > > > > > > > Thanks/Ray > > > > > > > > > -Original Message- > > > > > From: Kinney, Michael D > > > > > Sent: Tuesday, September 11, 2018 12:39 AM > > > > > To: Ni, Ruiyu ; edk2- > > de...@lists.01.org; Kinney, > > > > > Michael D > > > > > Cc: Yao, Jiewen ; Gao, > > Liming > > > > > > > > > > Subject: RE: [PATCH] MdePkg/SynchronizationLib: > > fix > > > > > Interlocked[De|In]crement return value > > > > > > > > > > Ray, > > > > > > > > > > Why are we removing the use of intrinsics from > > MSFT builds? We > > > > > should keep those for more efficient code > > generation if the > > > > > intrinsic has the correct behavior. > > > > > > > > > > Thanks, > > > > > > > > > > Mike > > > > > > > > > > > -Original Message- > > > > > > From: Ni, Ruiyu > > > > > > Sent: Monday, September 10, 2018 3:06 AM > > > > > > To: edk2-devel@lists.01.org > > > > > > Cc: Yao, Jiewen ; Gao, > > Liming > > > > > > ; Kinney, Michael D > > > > > > > > > > > > Subject: [PATCH] MdePkg/SynchronizationLib: fix > > > > > > Interlocked[De|In]crement return value > > > > > > > > > > > > Today's > > InterlockedIncrement()/InterlockedDecrement() > > > > > > guarantees to > > > > > > perform atomic increment/decrement but doesn't > > guarantee the > > > > > > return value equals to the new value. > > > > > > > > > > > > The patch fixes the behavior to use "XADD" > > instruction to > > > > > > guarantee the return value equals to the new > > value. > > > > > > > > > > > > Contributed-under: TianoCore Contribution > > Agreement 1.1 > > > > > > Signed-off-by: Ruiyu Ni > > > > > > Cc: Jiewen Yao > > > > > > Cc: Liming Gao > > > > > > Cc: Michael D Kinney > > > > > > > > --- > > > > > > MdePkg/Include/Library/SynchronizationLib.h > > | 6 > > > > > > +-- > > > > > > .../BaseSynchronizationLib.inf > > | 18 > > > > > > - > > > > > > .../BaseSynchronizationLibInternals.h > > | 6 > > > > > > +-- > > > > > > .../BaseSynchronizationLib/Ia32/GccInline.c > > | 18 > > > > > > - > > > > > > .../Ia32/InterlockedDecrement.c > > | 42 > > > > > > > > > > > > .../Ia32/InterlockedDecrement.nasm > > | 10 > > > > > > ++--- > > > > > > .../Ia32/InterlockedIncrement.c > > | 43 > > > > > > > > > > > > .../Ia32/InterlockedIncrement.nasm > > | 7 > > > > > > ++-- > > > > > > .../BaseSynchronizationLib/Synchronization.c > > | 6 > > > > > > +-- > > > > > > .../BaseSynchronizationLib/SynchronizationGcc.c > > | 6 > > > > > > +-- > > > > > > .../BaseSynchronizationLib/SynchronizationMsc.c > > | 6 > > > > > > +-- > > > > > > > > .../Library/BaseSynchronizationLib/X64/GccInline.c | 19 > >
Re: [edk2] [PATCH 0/4] MdeModulePkg: add support for dispatching foreign arch PE/COFF images
Ard, What happens when more than one emulators want to co-exist? -Jaben > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Ard Biesheuvel > Sent: Wednesday, September 12, 2018 7:57 AM > To: Gao, Liming > Cc: Ni, Ruiyu ; Zimmer, Vincent > ; Dong, Eric ; edk2- > de...@lists.01.org; ag...@suse.de; Andrew Fish ; > Richardson, Brian ; Kinney, Michael D > ; Laszlo Ersek ; Zeng, > Star > Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg: add support for dispatching > foreign arch PE/COFF images > > On 12 September 2018 at 16:55, Gao, Liming wrote: > > Ard: > > What's purpose to run the not native image in emulator? And, what > should be done in the emulator driver, such as X64Emulator? Is the emulator > responsible to translate the instruction between the different arch? I see > X64Emualtor bases on qemu code. > > > > The primary use case is option ROMs on PCIe cards. > ___ > 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 0/5] expire the use of PcdSetNxForStack
On 12 September 2018 at 02:55, Ni, Ruiyu wrote: > > >> -Original Message- >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard >> Biesheuvel >> Sent: Wednesday, September 12, 2018 5:03 AM >> To: Ni, Ruiyu >> Cc: edk2-devel@lists.01.org >> Subject: Re: [edk2] [PATCH 0/5] expire the use of PcdSetNxForStack >> >> On 11 September 2018 at 11:13, Ni, Ruiyu wrote: >> > On 9/11/2018 4:57 PM, Ard Biesheuvel wrote: >> >> >> >> On 11 September 2018 at 07:16, Jian J Wang wrote: >> >>> >> >>> BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116 >> >>> >> >>> Since the stack memory is allocated as EfiBootServicesData, its NX >> >>> protection can be covered by BIT4 of PcdDxeNxMemoryProtectionPolicy. >> >>> To avoid confusing in setting related PCDs, PcdSetNxForStack will be >> >>> expired. Instead, If >> >>> BIT4 >> >>> of PcdDxeNxMemoryProtectionPolicy is set, the DxeIpl will set NX bit >> >>> in page table entries mapping the stack memory. >> >>> >> >> >> >> I disagree. This removes the possibility to map EfiBootServicesData >> >> as executable while still mapping the stack NX. As we all know, an >> >> executable stack is in a class of its own when it comes to >> >> exploitability, and should *never* be mapped executable unless in >> >> highly exceptional cases. Mapping all EfiBootServicesData as >> >> non-executable may cause backward compatibility problems. >> > >> > Ard, >> > Are you saying you want the capability of setting certain range of BS >> > data as executable? Why does ARM need such capability? >> > >> >> No, I am saying that mapping all BS data executable should be a separate >> decision from mapping the stack executable: if your platform cannot support >> the >> former (for historical reasons) you will likely still want the latter. > > Let me try to understand the specific problem in ARM64: > ARM64 uses 64KB page size to support 2^52 memory space. With 4KB page size, > it can only support 2^48 memory space. > But due to the DXE core AllocatePages() implementation, the hard-code 4KB > granularity > (defined by UEFI spec) causes the page table protection for BS_DATA/BS_CODE > is impossible. > So ARM64 chooses to disable the BS_DATA/BS_CODE protection, but only enable > the stack protection. > Correct? > If so, is changing spec to allow page-size platform configurable a better > option? > I don't think so. We need to retain compatibility with PE/COFF and third party UEFI images, so changing the page size is likely to result in much more pain than it cures. ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 0/4] MdeModulePkg: add support for dispatching foreign arch PE/COFF images
On 12 September 2018 at 16:55, Gao, Liming wrote: > Ard: > What's purpose to run the not native image in emulator? And, what should be > done in the emulator driver, such as X64Emulator? Is the emulator responsible > to translate the instruction between the different arch? I see X64Emualtor > bases on qemu code. > The primary use case is option ROMs on PCIe cards. ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 0/4] MdeModulePkg: add support for dispatching foreign arch PE/COFF images
Ard: What's purpose to run the not native image in emulator? And, what should be done in the emulator driver, such as X64Emulator? Is the emulator responsible to translate the instruction between the different arch? I see X64Emualtor bases on qemu code. Thanks Liming > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard > Biesheuvel > Sent: Wednesday, September 12, 2018 9:22 PM > To: edk2-devel@lists.01.org > Cc: Ni, Ruiyu ; Zimmer, Vincent > ; Dong, Eric ; Andrew Fish > ; ag...@suse.de; Richardson, Brian > ; Kinney, Michael D > ; Laszlo Ersek ; Zeng, Star > > Subject: [edk2] [PATCH 0/4] MdeModulePkg: add support for dispatching foreign > arch PE/COFF images > > Add the basic plumbing to DXE core, the PCI bus driver and the boot manager > to allow PE/COFF images to be dispatched that target an architecture that is > not native for the platform, but which is supported by an emulator. > > One implementation of such an emulator can be found here: > https://github.com/ardbiesheuvel/X86EmulatorPkg > > Cc: Zimmer Vincent > Cc: Brian Richardson > Cc: Michael D Kinney > Cc: Andrew Fish > Cc: Laszlo Ersek > Cc: Leif Lindholm > Cc: Star Zeng > Cc: Eric Dong > Cc: Ruiyu Ni > > Ard Biesheuvel (4): > MdeModulePkg: introduce PE/COFF image emulator protocol > MdeModulePkg/DxeCore: invoke the emulator protocol for foreign images > MdeModulePkg/PciBusDxe: invoke PE/COFF emulator for foreign option > ROMs > MdeModulePkg/UefiBootManagerLib: allow foreign Driver images > > MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 + > MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 + > .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c | 16 +- > MdeModulePkg/Core/Dxe/DxeMain.h | 3 ++ > MdeModulePkg/Core/Dxe/DxeMain.inf | 1 + > MdeModulePkg/Core/Dxe/Image/Image.c | 39 +++--- > .../Include/Protocol/PeCoffImageEmulator.h| 51 +++ > .../Library/UefiBootManagerLib/BmLoadOption.c | 26 +- > .../Library/UefiBootManagerLib/InternalBm.h | 1 + > .../UefiBootManagerLib/UefiBootManagerLib.inf | 1 + > MdeModulePkg/MdeModulePkg.dec | 4 ++ > 11 files changed, 136 insertions(+), 8 deletions(-) > create mode 100644 MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h > > -- > 2.17.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] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value
Ray, Yes. This provides best size with optimizations enabled. Mike > -Original Message- > From: Ni, Ruiyu > Sent: Tuesday, September 11, 2018 7:16 PM > To: Ni, Ruiyu ; Kinney, Michael D > ; 'edk2-devel@lists.01.org' > > Cc: Yao, Jiewen ; Gao, Liming > > Subject: RE: [PATCH] MdePkg/SynchronizationLib: fix > Interlocked[De|In]crement return value > > Mike, > Do you require to still use MSVC intrinsic function? > > > -Original Message- > > From: edk2-devel [mailto:edk2-devel- > boun...@lists.01.org] On Behalf Of Ni, > > Ruiyu > > Sent: Tuesday, September 11, 2018 10:27 AM > > To: Kinney, Michael D ; > 'edk2-devel@lists.01.org' > > > > Cc: Yao, Jiewen ; Gao, Liming > > > Subject: Re: [edk2] [PATCH] MdePkg/SynchronizationLib: > fix > > Interlocked[De|In]crement return value > > > > The reason I didn't remove the GCC version is because > Liming told me that there > > is a XCODE issue which prevents using NASM for > library. > > > > Thanks/Ray > > > > > -Original Message- > > > From: Ni, Ruiyu > > > Sent: Tuesday, September 11, 2018 10:25 AM > > > To: Kinney, Michael D ; > edk2- > > > de...@lists.01.org > > > Cc: Yao, Jiewen ; Gao, Liming > > > > > > Subject: RE: [PATCH] MdePkg/SynchronizationLib: fix > > > Interlocked[De|In]crement return value > > > > > > The NASM does the exactly same as the MSFT > intrinsic. > > > I'd like to remove them because I was almost lost in > so many versions > > > of Interlocked[De|In]crement. > > > I'd like to merge them to one version. > > > > > > Thanks/Ray > > > > > > > -Original Message- > > > > From: Kinney, Michael D > > > > Sent: Tuesday, September 11, 2018 12:39 AM > > > > To: Ni, Ruiyu ; edk2- > de...@lists.01.org; Kinney, > > > > Michael D > > > > Cc: Yao, Jiewen ; Gao, > Liming > > > > > > > > Subject: RE: [PATCH] MdePkg/SynchronizationLib: > fix > > > > Interlocked[De|In]crement return value > > > > > > > > Ray, > > > > > > > > Why are we removing the use of intrinsics from > MSFT builds? We > > > > should keep those for more efficient code > generation if the > > > > intrinsic has the correct behavior. > > > > > > > > Thanks, > > > > > > > > Mike > > > > > > > > > -Original Message- > > > > > From: Ni, Ruiyu > > > > > Sent: Monday, September 10, 2018 3:06 AM > > > > > To: edk2-devel@lists.01.org > > > > > Cc: Yao, Jiewen ; Gao, > Liming > > > > > ; Kinney, Michael D > > > > > > > > > > Subject: [PATCH] MdePkg/SynchronizationLib: fix > > > > > Interlocked[De|In]crement return value > > > > > > > > > > Today's > InterlockedIncrement()/InterlockedDecrement() > > > > > guarantees to > > > > > perform atomic increment/decrement but doesn't > guarantee the > > > > > return value equals to the new value. > > > > > > > > > > The patch fixes the behavior to use "XADD" > instruction to > > > > > guarantee the return value equals to the new > value. > > > > > > > > > > Contributed-under: TianoCore Contribution > Agreement 1.1 > > > > > Signed-off-by: Ruiyu Ni > > > > > Cc: Jiewen Yao > > > > > Cc: Liming Gao > > > > > Cc: Michael D Kinney > > > > > > --- > > > > > MdePkg/Include/Library/SynchronizationLib.h > | 6 > > > > > +-- > > > > > .../BaseSynchronizationLib.inf > | 18 > > > > > - > > > > > .../BaseSynchronizationLibInternals.h > | 6 > > > > > +-- > > > > > .../BaseSynchronizationLib/Ia32/GccInline.c > | 18 > > > > > - > > > > > .../Ia32/InterlockedDecrement.c > | 42 > > > > > > > > > > .../Ia32/InterlockedDecrement.nasm > | 10 > > > > > ++--- > > > > > .../Ia32/InterlockedIncrement.c > | 43 > > > > > > > > > > .../Ia32/InterlockedIncrement.nasm > | 7 > > > > > ++-- > > > > > .../BaseSynchronizationLib/Synchronization.c > | 6 > > > > > +-- > > > > > .../BaseSynchronizationLib/SynchronizationGcc.c > | 6 > > > > > +-- > > > > > .../BaseSynchronizationLib/SynchronizationMsc.c > | 6 > > > > > +-- > > > > > > .../Library/BaseSynchronizationLib/X64/GccInline.c | 19 > > > > > + > > > > > .../X64/InterlockedDecrement.c > | 46 > > > > > -- > > > > > .../X64/InterlockedDecrement.nasm > | 8 > > > > > ++-- > > > > > .../X64/InterlockedIncrement.c > | 46 > > > > > -- > > > > > .../X64/InterlockedIncrement.nasm > | 5 > > > > > ++- > > > > > 16 files changed, 52 insertions(+), 240 > deletions(-) delete mode > > > > > 100644 > MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedDe > > > > > crement.c > > > > > delete mode 100644 > > > > > > MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIn > > > > > crement.c > > > > > delete mode 100644 > > > > > > MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDec > > > > > rement.c > > > > > delete mode 100644 > > > > > > MdePkg/Library/BaseSynchronizationLib/X64/InterlockedInc > > > > > rement.c > > > > > > > > > > diff --git > a/MdePkg/Include/Library/SynchronizationLib.h > > > > > b/MdePkg/Include/Library/SynchronizationLib.h > > > > > index
Re: [edk2] [Patch V2] INF spec: Correct some items in the Table 1 EDK II [Defines] Section
Reviewed-by: Liming Gao > -Original Message- > From: Zhu, Yonghong > Sent: Wednesday, September 12, 2018 8:45 PM > To: edk2-devel@lists.01.org > Cc: Gao, Liming ; Kinney, Michael D > ; Shaw, Kevin W > Subject: [Patch V2] INF spec: Correct some items in the Table 1 EDK II > [Defines] Section > > V2: list the updated items > Remove 'EDK_RELEASE_VERSION', 'DEFINE' > Add 'PCI_REVISION' > Update 'VERSION_STRING''s required attrubute in the table to Optional. > Update 'LIBRARY_CLASS' and UEFI PCI Option ROM's value in the table per 3.4 > section > Update the table's format because current in the PDF format spec this table's > display was cut off. > > Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=1162 > Cc: Liming Gao > Cc: Michael Kinney > Cc: Kevin W Shaw > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Yonghong Zhu > --- > 2_inf_overview/24_[defines]_section.md | 59 > > 3_edk_ii_inf_file_format/34_[defines]_section.md | 2 +- > README.md| 1 + > 3 files changed, 31 insertions(+), 31 deletions(-) > > diff --git a/2_inf_overview/24_[defines]_section.md > b/2_inf_overview/24_[defines]_section.md > index 37b0135..0afdfed 100644 > --- a/2_inf_overview/24_[defines]_section.md > +++ b/2_inf_overview/24_[defines]_section.md > @@ -106,35 +106,34 @@ the PEI Core or the Dxe Core. EDK II only references > the first possible > dispatch instance. > ** > > ## Table 1 EDK II [Defines] Section Elements > > -| Tag | Required > | Value > | Notes > | > -| | > | > - | > - > - > - > | > -| `INF_VERSION`| REQUIRED > | 1.27 > or 0x0001001B| This identifies the INF spec. > version. It is decimal value with fraction or two-nibble > hexadecimal representation of the same, for example: 1.27. Tools use this > value to handle parsing of previous releases of the > specification if there are incompatible changes. > | > -| `BASE_NAME` | REQUIRED > | A > single word | This is a single word > identifier that will be used for the component name. > | > -| `EDK_RELEASE_VERSION`| Not required > | Hex > Double Word | The minimum revision value across > the module and all its dependent libraries. If a > revision value is not declared in the module or any of the dependent > libraries, then the tool may use the value of 0, which disables > checking. > | > -| `PI_SPECIFICATION_VERSION` | Not required > | Decimal > or special format of hex | The minimum revision value across the > module and all its dependent libraries. If a revision > value is not declared in the module or any of the dependent libraries, then > tools may use the value of 0, which disables checking. > | > -| | > | > | The `PI_SPECIFICATION_VERSION` must only be set in the INF file if the > module depends on services or system table fields or PI core > behaviors that are not present in the PI 1.0 version. For example, if a > module depends on definitions in PI 1.1 that are not in PI 1.0, then > `PI_SPECIFICATION_VERSION` must be 0x0001000A > | > -| `UEFI_SPECIFICATION_VERSION` | Not required > | Decimal > or special format of hex | The minimum revision value across the > module and all its dependent libraries. If a revision > value is not declared in the module or any of the dependent libraries, then > tools may use the value of 0, which disables checking. > | > -| | > | > | The `UEFI_SPECIFICATION_VERSIon` must only be set in the INF file if the > module depends on UEFI Boot Services or UEFI Runtime > Services or UEFI System Table fields or UEFI core behaviors that are not > present in the UEFI 2.1
Re: [edk2] [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Update SmiEntry function run the same position
On 09/12/18 07:13, Liming Gao wrote: > 1. Remove jmp _SmiHandler, and run the code at the same position. > 2. Fix up the function call address as the absolute address. > Verify OVMF SMM boot to shell with VS2017, GCC5 and XCODE5 tool chain. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Liming Gao > Cc: Laszlo Ersek > Cc: Eric Dong > Cc: Jiewen Yao > --- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 34 > + > 1 file changed, 25 insertions(+), 9 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > index 315d0f8..d8259de 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > @@ -173,9 +173,6 @@ SmiHandlerIdtrAbsAddr: > mov gs, eax > mov ax, [rbx + DSC_SS] > mov ss, eax > -mov rax, strict qword 0 ; mov rax, _SmiHandler > -_SmiHandlerAbsAddr: > -jmp rax > > _SmiHandler: > mov rbx, [rsp + 0x8] ; rcx <- CpuIndex > @@ -189,13 +186,19 @@ _SmiHandler: > add rsp, -0x20 > > mov rcx, rbx > -callASM_PFX(CpuSmmDebugEntry) > +mov rax, strict qword 0 ; callASM_PFX(CpuSmmDebugEntry) > +CpuSmmDebugEntryAbsAddr: > +callrax > > mov rcx, rbx > -callASM_PFX(SmiRendezvous) > +mov rax, strict qword 0 ; callASM_PFX(SmiRendezvous) > +SmiRendezvousAbsAddr: > +callrax > > mov rcx, rbx > -callASM_PFX(CpuSmmDebugExit) > +mov rax, strict qword 0 ; callASM_PFX(CpuSmmDebugExit) > +CpuSmmDebugExitAbsAddr: > +callrax > > add rsp, 0x20 > > @@ -206,7 +209,8 @@ _SmiHandler: > > add rsp, 0x200 > > -lea rax, [ASM_PFX(mXdSupported)] > +mov rax, strict qword 0 ; lea rax, > [ASM_PFX(mXdSupported)] > +mXdSupportedAbsAddr: > mov al, [rax] > cmp al, 0 > jz .1 > @@ -230,7 +234,19 @@ ASM_PFX(PiSmmCpuSmiEntryFixupAddress): > learcx, [SmiHandlerIdtrAbsAddr] > movqword [rcx - 8], rax > > -learax, [_SmiHandler] > -learcx, [_SmiHandlerAbsAddr] > +learax, [ASM_PFX(CpuSmmDebugEntry)] > +learcx, [CpuSmmDebugEntryAbsAddr] > +movqword [rcx - 8], rax > + > +learax, [ASM_PFX(SmiRendezvous)] > +learcx, [SmiRendezvousAbsAddr] > +movqword [rcx - 8], rax > + > +learax, [ASM_PFX(CpuSmmDebugExit)] > +learcx, [CpuSmmDebugExitAbsAddr] > +movqword [rcx - 8], rax > + > +learax, [ASM_PFX(mXdSupported)] > +learcx, [mXdSupportedAbsAddr] > movqword [rcx - 8], rax > ret > (a) The patch seems to do two things (listed as (1) and (2) in the commit message). They should be implemented in separate patches, in a series. (b) Regarding part (1): why did "jmp _SmiHandler" exist in the first place? That jump instruction was originally added in commit 9a36d4dc3f5e: "UefiCpuPkg PiSmmCpuDxeSmm: Convert X64/SmiEntry.asm to NASM", 2016-06-28 with the comment "instruction is not needed". If it was not needed, then why had we added it? For that, let's look at "UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm" instead (that is, the pre-NASM file from which the NASM version was created). In that file, the same comment was introduced as part of commit 427e3573426f: "UefiCpuPkg: Add PiSmmCpuDxeSmm module X64 files", 2015-10-19 That means that we've had this unnecessary jump ever since the inception of PiSmmCpuDxeSmm (in the open source edk2 tree). And now we are removing the jump, for code simplification. That's a good thing. But it should be a separate patch, with the argument clearly spelled out: Remove the _SmiHandler label and all the instructions that facilitate the jump to it, because the jump to _SmiHandler has always been unnecessary, as stated by the comment in original commit 427e3573426f as well. Note: the _SmiHandler label itself should likely be removed too, not just _SmiHandlerAbsAddr. It is never referenced, after the change. (c) I must say that the description of part (2) is extremely lacking. I've had to spend more than two hours to reconstruct the argument. Normally the commit message should describe - what the expected behavior is, for what use case, - what the current (problematic) behavior is, on the master branch, - how the change addresses the issue, - if there is a BZ for the issue, it should be referenced. So here's my understanding of part (2) -- which should be a separate patch, again --: * Before commit e21e355e2ca7f, we had mov rax, ASM_PFX(CpuSmmDebugEntry) callrax This was not good, because XCODE can only handle RIP-relative addressing in assembly code. So in bug 849 / commit e21e355e2ca7f, we changed it to: callASM_PFX(CpuSmmDebugEntry) * While
[edk2] [PATCH 2/4] MdeModulePkg/DxeCore: invoke the emulator protocol for foreign images
When encountering PE/COFF images that cannot be supported natively,
attempt to locate an instance of the PE/COFF image emulator protocol,
and if it supports the image, proceed with loading it and register it
with the emulator.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel
---
MdeModulePkg/Core/Dxe/DxeMain.h | 3 ++
MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
MdeModulePkg/Core/Dxe/Image/Image.c | 39 +---
3 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
index 7ec82388a3f9..57b3861d9813 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -53,6 +53,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
EXPRESS OR IMPLIED.
#include
#include
#include
+#include
#include
#include
#include
@@ -229,6 +230,8 @@ typedef struct {
UINT16 Machine;
/// EBC Protocol pointer
EFI_EBC_PROTOCOL*Ebc;
+ /// PE/COFF Image Emulator Protocol pointer
+ EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL *Emu;
/// Runtime image list
EFI_RUNTIME_IMAGE_ENTRY *RuntimeData;
/// Pointer to Loaded Image Device Path Protocol
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 68fa0a01d9bd..d7591aa0da6d 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -180,6 +180,7 @@
gEfiVariableArchProtocolGuid ## CONSUMES
gEfiCapsuleArchProtocolGuid ## CONSUMES
gEfiWatchdogTimerArchProtocolGuid ## CONSUMES
+ gEdkiiPeCoffImageEmulatorProtocolGuid ## SOMETIMES_CONSUMES
[FeaturePcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdFrameworkCompatibilitySupport ##
CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
b/MdeModulePkg/Core/Dxe/Image/Image.c
index eddca140ee1a..e2dd80790657 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -67,6 +67,7 @@ LOADED_IMAGE_PRIVATE_DATA mCorePrivateImage = {
NULL, // JumpContext
0, // Machine
NULL, // Ebc
+ NULL, // Emu
NULL, // RuntimeData
NULL// LoadedImageDevicePath
};
@@ -476,12 +477,23 @@ CoreLoadPeImage (
if (!EFI_IMAGE_MACHINE_TYPE_SUPPORTED (Image->ImageContext.Machine)) {
if (!EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED (Image->ImageContext.Machine))
{
//
- // The PE/COFF loader can support loading image types that can be
executed.
- // If we loaded an image type that we can not execute return
EFI_UNSUPORTED.
+ // Locate the emulator protocol to check whether it supports this
+ // image.
//
- DEBUG ((EFI_D_ERROR, "Image type %s can't be loaded ",
GetMachineTypeName(Image->ImageContext.Machine)));
- DEBUG ((EFI_D_ERROR, "on %s UEFI system.\n",
GetMachineTypeName(mDxeCoreImageMachineType)));
- return EFI_UNSUPPORTED;
+ Status = CoreLocateProtocol (,
+ NULL, (VOID **)>Emu);
+ if (EFI_ERROR (Status) ||
+ !Image->Emu->IsImageSupported (Image->Emu,
+ Image->ImageContext.Machine,
+ Image->ImageContext.ImageType)) {
+//
+// The PE/COFF loader can support loading image types that can be
executed.
+// If we loaded an image type that we can not execute return
EFI_UNSUPORTED.
+//
+DEBUG ((EFI_D_ERROR, "Image type %s can't be loaded ",
GetMachineTypeName(Image->ImageContext.Machine)));
+DEBUG ((EFI_D_ERROR, "on %s UEFI system.\n",
GetMachineTypeName(mDxeCoreImageMachineType)));
+return EFI_UNSUPPORTED;
+ }
}
}
@@ -687,6 +699,14 @@ CoreLoadPeImage (
if (EFI_ERROR(Status)) {
goto Done;
}
+ } else if (Image->Emu != NULL) {
+Status = Image->Emu->RegisterImage (Image->Emu, Image->ImageBasePage,
+ EFI_PAGES_TO_SIZE (Image->NumberOfPages));
+if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_LOAD | DEBUG_ERROR,
+"CoreLoadPeImage: Failed to load register foreign image with
emulator.\n"));
+ goto Done;
+}
}
//
@@ -874,6 +894,13 @@ CoreUnloadAndCloseImage (
Image->Ebc->UnloadImage (Image->Ebc, Image->Handle);
}
+ if (Image->Emu != NULL) {
+//
+// If the PE/COFF Emulator protocol exists we must unregister the image.
+//
+Image->Emu->UnregisterImage (Image->Emu, Image->ImageBasePage);
+ }
+
//
// Unload image, free Image->ImageContext->ModHandle
//
@@ -1599,7 +1626,7 @@ CoreStartImage (
//
// The image to be started must have the machine type supported by DxeCore.
//
- if (!EFI_IMAGE_MACHINE_TYPE_SUPPORTED (Image->Machine)) {
+ if (!EFI_IMAGE_MACHINE_TYPE_SUPPORTED (Image->Machine) && Image->Emu
[edk2] [PATCH 0/4] MdeModulePkg: add support for dispatching foreign arch PE/COFF images
Add the basic plumbing to DXE core, the PCI bus driver and the boot manager to allow PE/COFF images to be dispatched that target an architecture that is not native for the platform, but which is supported by an emulator. One implementation of such an emulator can be found here: https://github.com/ardbiesheuvel/X86EmulatorPkg Cc: Zimmer Vincent Cc: Brian Richardson Cc: Michael D Kinney Cc: Andrew Fish Cc: Laszlo Ersek Cc: Leif Lindholm Cc: Star Zeng Cc: Eric Dong Cc: Ruiyu Ni Ard Biesheuvel (4): MdeModulePkg: introduce PE/COFF image emulator protocol MdeModulePkg/DxeCore: invoke the emulator protocol for foreign images MdeModulePkg/PciBusDxe: invoke PE/COFF emulator for foreign option ROMs MdeModulePkg/UefiBootManagerLib: allow foreign Driver images MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 + MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 + .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c | 16 +- MdeModulePkg/Core/Dxe/DxeMain.h | 3 ++ MdeModulePkg/Core/Dxe/DxeMain.inf | 1 + MdeModulePkg/Core/Dxe/Image/Image.c | 39 +++--- .../Include/Protocol/PeCoffImageEmulator.h| 51 +++ .../Library/UefiBootManagerLib/BmLoadOption.c | 26 +- .../Library/UefiBootManagerLib/InternalBm.h | 1 + .../UefiBootManagerLib/UefiBootManagerLib.inf | 1 + MdeModulePkg/MdeModulePkg.dec | 4 ++ 11 files changed, 136 insertions(+), 8 deletions(-) create mode 100644 MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h -- 2.17.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 3/4] MdeModulePkg/PciBusDxe: invoke PE/COFF emulator for foreign option ROMs
When enumerating option ROM images, take into account whether an emulator
exists that would allow dispatch of PE/COFF images built for foreign
architectures.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel
---
MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 +
MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 +
MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c | 16 +++-
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
index 55eb3a5a8070..dc57d4876c0f 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
@@ -33,6 +33,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
EXPRESS OR IMPLIED.
#include
#include
#include
+#include
#include
#include
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
index a21dd2b5edf4..3d99ea0c1047 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
@@ -97,6 +97,7 @@
gEfiLoadFile2ProtocolGuid ## SOMETIMES_PRODUCES
gEdkiiIoMmuProtocolGuid ## SOMETIMES_CONSUMES
gEfiLoadedImageDevicePathProtocolGuid ## CONSUMES
+ gEdkiiPeCoffImageEmulatorProtocolGuid ## SOMETIMES_CONSUMES
[FeaturePcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdPciBusHotplugDeviceSupport ## CONSUMES
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
index c2be85a906af..07236afd327d 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
@@ -678,6 +678,7 @@ ProcessOpRomImage (
MEDIA_RELATIVE_OFFSET_RANGE_DEVICE_PATH EfiOpRomImageNode;
VOID *Buffer;
UINTNBufferSize;
+ EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL *PeCoffEmulator;
Indicator = 0;
@@ -693,6 +694,7 @@ ProcessOpRomImage (
}
ASSERT (((EFI_PCI_EXPANSION_ROM_HEADER *) RomBarOffset)->Signature ==
PCI_EXPANSION_ROM_HEADER_SIGNATURE);
+ PeCoffEmulator = NULL;
do {
EfiRomHeader = (EFI_PCI_EXPANSION_ROM_HEADER *) RomBarOffset;
if (EfiRomHeader->Signature != PCI_EXPANSION_ROM_HEADER_SIGNATURE) {
@@ -716,7 +718,19 @@ ProcessOpRomImage (
// Skip the EFI PCI Option ROM image if its machine type is not supported
//
if (!EFI_IMAGE_MACHINE_TYPE_SUPPORTED (EfiRomHeader->EfiMachineType)) {
- goto NextImage;
+ //
+ // Check whether we have a PE/COFF emulator that supports this image
+ //
+ if (PeCoffEmulator == NULL) {
+gBS->LocateProtocol (, NULL,
+ (VOID **));
+ }
+ if (PeCoffEmulator == NULL ||
+ !PeCoffEmulator->IsImageSupported (PeCoffEmulator,
+ EfiRomHeader->EfiMachineType,
+ EfiRomHeader->EfiSubsystem)) {
+goto NextImage;
+ }
}
//
--
2.17.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 4/4] MdeModulePkg/UefiBootManagerLib: allow foreign Driver#### images
Allow PE/COFF images that must execute under emulation for Driver
options, by relaxing the machine type check to include support for
machine types that is provided by the emulator.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel
---
MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c | 26
+++-
MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h | 1 +
MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf | 1 +
3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index 7bf96646c690..67e3dfd7e9e1 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -1185,6 +1185,29 @@ EfiBootManagerFreeLoadOptions (
return EFI_SUCCESS;
}
+STATIC
+BOOLEAN
+BmIsImageTypeSupported (
+ IN UINT16MachineType,
+ IN UINT16SubSystem
+ )
+{
+ EFI_STATUSStatus;
+ EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL *Emu;
+
+ if (EFI_IMAGE_MACHINE_TYPE_SUPPORTED (MachineType)) {
+return TRUE;
+ }
+
+ Status = gBS->LocateProtocol (,
+ NULL, (VOID **));
+ if (EFI_ERROR (Status)) {
+return FALSE;
+ }
+
+ return Emu->IsImageSupported (Emu, MachineType, SubSystem);
+}
+
/**
Return whether the PE header of the load option is valid or not.
@@ -1235,7 +1258,8 @@ BmIsLoadOptionPeHeaderValid (
OptionalHeader = (EFI_IMAGE_OPTIONAL_HEADER32 *)
>Pe32.OptionalHeader;
if ((OptionalHeader->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC ||
OptionalHeader->Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) &&
- EFI_IMAGE_MACHINE_TYPE_SUPPORTED (PeHeader->Pe32.FileHeader.Machine)
+ BmIsImageTypeSupported (PeHeader->Pe32.FileHeader.Machine,
+ OptionalHeader->Subsystem)
) {
//
// Check the Subsystem:
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
index 978fbff966f6..5f64ef304b87 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
+++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
@@ -47,6 +47,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
EXPRESS OR IMPLIED.
#include
#include
#include
+#include
#include
#include
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
index 72c5ca1cd59e..09e2134acf8e 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
+++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
@@ -104,6 +104,7 @@
gEfiDevicePathProtocolGuid## SOMETIMES_CONSUMES
gEfiBootLogoProtocolGuid ## SOMETIMES_CONSUMES
gEfiSimpleTextInputExProtocolGuid ## SOMETIMES_CONSUMES
+ gEdkiiPeCoffImageEmulatorProtocolGuid ## SOMETIMES_CONSUMES
gEdkiiVariableLockProtocolGuid## SOMETIMES_CONSUMES
gEfiGraphicsOutputProtocolGuid## SOMETIMES_CONSUMES
gEfiUsbIoProtocolGuid ## SOMETIMES_CONSUMES
--
2.17.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/4] MdeModulePkg: introduce PE/COFF image emulator protocol
Introduce a protocol that can be invoked by the image loading services
to execute foreign architecture PE/COFF images via an emulator.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel
---
MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h | 51
MdeModulePkg/MdeModulePkg.dec | 4 ++
2 files changed, 55 insertions(+)
diff --git a/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
b/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
new file mode 100644
index ..3391e68557b9
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
@@ -0,0 +1,51 @@
+/** @file
+ Copyright (c) 2018, Linaro, Ltd. All rights reserved.
+
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD
License
+ which accompanies this distribution. The full text of the license may be
found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __PECOFF_IMAGE_EMULATOR_PROTOCOL_GUID_H__
+#define __PECOFF_IMAGE_EMULATOR_PROTOCOL_GUID_H__
+
+#define EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL_GUID \
+ { 0x96F46153, 0x97A7, 0x4793, { 0xAC, 0xC1, 0xFA, 0x19, 0xBF, 0x78, 0xEA,
0x97 } }
+
+typedef struct _EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL
EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL;
+
+typedef
+BOOLEAN
+(EFIAPI *IS_PECOFF_IMAGE_SUPPORTED) (
+ IN EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL*This,
+ IN UINT16 MachineType,
+ IN UINT16 ImageType
+ );
+
+typedef
+EFI_STATUS
+(EFIAPI *REGISTER_PECOFF_IMAGE) (
+ IN EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL*This,
+ IN EFI_PHYSICAL_ADDRESSImageBase,
+ IN UINT64 ImageSize
+ );
+
+typedef
+EFI_STATUS
+(EFIAPI *UNREGISTER_PECOFF_IMAGE) (
+ IN EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL*This,
+ IN EFI_PHYSICAL_ADDRESSImageBase
+ );
+
+typedef struct _EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL {
+ IS_PECOFF_IMAGE_SUPPORTED IsImageSupported;
+ REGISTER_PECOFF_IMAGE RegisterImage;
+ UNREGISTER_PECOFF_IMAGE UnregisterImage;
+} EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 6a6d9660edc2..be307329f901 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -617,6 +617,10 @@
## Include/Protocol/AtaAtapiPolicy.h
gEdkiiAtaAtapiPolicyProtocolGuid = { 0xe59cd769, 0x5083, 0x4f26,{ 0x90,
0x94, 0x6c, 0x91, 0x9f, 0x91, 0x6c, 0x4e } }
+
+ ## Include/Protocol/PeCoffImageEmulator.h
+ gEdkiiPeCoffImageEmulatorProtocolGuid = { 0x96f46153, 0x97a7, 0x4793, {
0xac, 0xc1, 0xfa, 0x19, 0xbf, 0x78, 0xea, 0x97 } }
+
#
# [Error.gEfiMdeModulePkgTokenSpaceGuid]
# 0x8001 | Invalid value provided.
--
2.17.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch V2] INF spec: Correct some items in the Table 1 EDK II [Defines] Section
V2: list the updated items Remove 'EDK_RELEASE_VERSION', 'DEFINE' Add 'PCI_REVISION' Update 'VERSION_STRING''s required attrubute in the table to Optional. Update 'LIBRARY_CLASS' and UEFI PCI Option ROM's value in the table per 3.4 section Update the table's format because current in the PDF format spec this table's display was cut off. Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=1162 Cc: Liming Gao Cc: Michael Kinney Cc: Kevin W Shaw Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Yonghong Zhu --- 2_inf_overview/24_[defines]_section.md | 59 3_edk_ii_inf_file_format/34_[defines]_section.md | 2 +- README.md| 1 + 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/2_inf_overview/24_[defines]_section.md b/2_inf_overview/24_[defines]_section.md index 37b0135..0afdfed 100644 --- a/2_inf_overview/24_[defines]_section.md +++ b/2_inf_overview/24_[defines]_section.md @@ -106,35 +106,34 @@ the PEI Core or the Dxe Core. EDK II only references the first possible dispatch instance. ** ## Table 1 EDK II [Defines] Section Elements -| Tag | Required | Value | Notes | -| | | - | --- | -| `INF_VERSION`| REQUIRED | 1.27 or 0x0001001B| This identifies the INF spec. version. It is decimal value with fraction or two-nibble hexadecimal representation of the same, for example: 1.27. Tools use this value to handle parsing of previous releases of the specification if there are incompatible changes. | -| `BASE_NAME` | REQUIRED | A single word | This is a single word identifier that will be used for the component name. | -| `EDK_RELEASE_VERSION`| Not required | Hex Double Word | The minimum revision value across the module and all its dependent libraries. If a revision value is not declared in the module or any of the dependent libraries, then the tool may use the value of 0, which disables checking. | -| `PI_SPECIFICATION_VERSION` | Not required | Decimal or special format of hex | The minimum revision value across the module and all its dependent libraries. If a revision value is not declared in the module or any of the dependent libraries, then tools may use the value of 0, which disables checking. | -| |
Re: [edk2] [PATCH 2/5] OvmfPkg/PlatformPei: expire the use of PcdSetNxForStack
On 09/12/18 04:11, Wang, Jian J wrote:
> Hi Laszlo and Ard,
>
> Retiring the PcdSetNxForStack is not the intention of this patch series. It's
> just
> a side effect of solving problem stated in BZ#1116. Sorry again for the
> misleading
> title. I'm not insisting that we have to remove this PCD anyway (My personal
> opinion. Others might have different ones).
>
> I think I understand the importance of PcdSetNxForStack to arm/aarch64 now.
> And I
> agree that it'd be better to enable NX for stack independent of enabling NX
> for
> EfiBootServcesData memory. But since the stack is type of EfiBootServicesData,
> I think we should avoid any confusion in enabling/disabling NX for them.
> That's what
> BZ#1116 tries to complain about. But I'm still not clear any concrete
> suggestion on
> how to solve the BZ#1116 from all comment so far, if my patch series cannot
> satisfy
> all kinds of platforms. Simply keep PcdSetNxForStack doesn't help, I think.
> (It doesn't
> imply we must remove it.)
>
> As I commented in the BZ#1116, maybe we can just simply assert if there's one
> trying to disable NX for stack but enable NX for EfiBootServicesData at the
> same time,
> because it doesn't make sense.
Yes, that's what I thought as well. Catch the one case with an assert
and/or CpuDeadLoop() that is an invalid configuration.
> I think all other setting combinations won't have
> such confusion and don't need to take care specifically.
>
> And for x86 CPU, we'll always enable CPU NX feature if any NX related PCDs
> have bits
> set. Current DxeIpl will just enable NX for PcdSetNxForStack only.
My understanding has been that it's not about enabling/disabling a CPU
feature, but about marking specific pages as non-executable. Under that
interpretation, it should be possible to mark pages used for stack
purposes as non-executable, and leave everything else executable, even
on x86.
Laszlo
>
> Regards,
> Jian
>
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Tuesday, September 11, 2018 11:53 PM
> To: Wang, Jian J ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Justen, Jordan L
> ; Yao, Jiewen ; Zeng, Star
> ; Ard Biesheuvel
> Subject: Re: [edk2] [PATCH 2/5] OvmfPkg/PlatformPei: expire the use of
> PcdSetNxForStack
>
> On 09/11/18 07:16, Jian J Wang wrote:
>> BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116
>>
>> Since PcdSetNxForStack is expired, remove related config code.
>> PcdDxeNxMemoryProtectionPolicy cannot be used as dynamic PCD.
>> There's no need to add it in code to replace PcdSetNxForStack.
>>
>> Cc: Laszlo Ersek
>> Cc: Star Zeng
>> Cc: Jordan Justen
>> Cc: Ard Biesheuvel
>> Cc: Ruiyu Ni
>> Cc: Jiewen Yao
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Jian J Wang
>> ---
>> OvmfPkg/PlatformPei/Platform.c | 1 -
>> OvmfPkg/PlatformPei/PlatformPei.inf | 1 -
>> 2 files changed, 2 deletions(-)
>>
>> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
>> index 5a78668126..6627d236e0 100644
>> --- a/OvmfPkg/PlatformPei/Platform.c
>> +++ b/OvmfPkg/PlatformPei/Platform.c
>> @@ -340,7 +340,6 @@ NoexecDxeInitialization (
>> )
>> {
>> UPDATE_BOOLEAN_PCD_FROM_FW_CFG (PcdPropertiesTableEnable);
>> - UPDATE_BOOLEAN_PCD_FROM_FW_CFG (PcdSetNxForStack);
>> }
>>
>> VOID
>> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf
>> b/OvmfPkg/PlatformPei/PlatformPei.inf
>> index 9c5ad9961c..ef5dcb7693 100644
>> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
>> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf
>> @@ -96,7 +96,6 @@
>> gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
>> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode
>> gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable
>> - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack
>> gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable
>> gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable
>> gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask
>>
>
> I disagree with this change. I explained my reasons in
> , but for the
> sake of discussion, I'll state the same here:
>
>> Ard's got a point here. Just because one PCD implies another, the
>> reverse is not necessarily true.
>>
>> For example, in OVMF, we set PcdSetNxForStack to TRUE originally, but
>> then we had to make it conditional, due to old GRUB variants breaking
>> with a non-executable stack. (Please refer to commit d20b06a3afdf,
>> "OvmfPkg: disable no-exec DXE stack by default", 2015-09-15).
>> Therefore we now default to FALSE, and let the user set it to TRUE on
>> the QEMU command line.
>>
>> In addition, we intentionally inherit a zero
>> PcdDxeNxMemoryProtectionPolicy from "MdeModulePkg.dec".
>>
>> Both of the above configurations satisfy the requirement
>>
>> ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) &
>> (1 << EfiBootServicesData)) == 0 ||
>> PcdGetBool
Re: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: add message for S3 config error
On 09/12/18 02:49, Wang, Jian J wrote:
> Laszlo,
>
> Thanks for the comments.
>
>
> (1)Agree. It’ll be updated in v2.
>
> (2)DEBUG_ERROR level won’t print word “ERROR” on console. I think an
> “ERROR”
>
> word in log should get the attention more easily.
>
> (3)How about log both of them? GUID value may be more friendly to log
> parser but
> a GUID name is more friendly to person.
>
> (4)Good idea. I’ll add it in v2.
Those work for me as well.
Thanks
Laszlo
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Tuesday, September 11, 2018 10:01 PM
> To: Wang, Jian J ; edk2-devel@lists.01.org
> Cc: Zeng, Star ; You, Benjamin ;
> Dong, Eric
> Subject: Re: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: add message for S3 config
> error
>
> On 09/10/18 05:22, Jian J Wang wrote:
>> BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1165
>>
>> HOB gEfiAcpiVariableGuid is a must have data for S3 resume if
>> PcdAcpiS3Enable is set to TRUE. Current code in CpuS3.c doesn't
>> embody this strong binding between them. An error message and
>> ASSERT is added by this patch to warn platform developer about
>> it.
>>
>> Cc: Star Zeng mailto:star.z...@intel.com>>
>> Cc: Benjamin You mailto:benjamin@intel.com>>
>> Cc: Eric Dong mailto:eric.d...@intel.com>>
>> Cc: Laszlo Ersek mailto:ler...@redhat.com>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Jian J Wang
>> mailto:jian.j.w...@intel.com>>
>> ---
>> UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
>> b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
>> index abd8a5a07b..f371667c44 100644
>> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
>> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
>> @@ -744,6 +744,9 @@ InitSmmS3ResumeState (
>> if (sizeof (UINTN) == sizeof (UINT32)) {
>>SmmS3ResumeState->Signature = SMM_S3_RESUME_SMM_32;
>> }
>> + } else {
>> +DEBUG ((DEBUG_ERROR, "ERROR: HOB(gEfiAcpiVariableGuid) needed by S3
>> resume doesn't exist!\n"));
>> +ASSERT (FALSE);
>>}
>>
>>//
>>
>
> I have some superficial comments for this patch.
>
> (1) in case the "if" has an "else" branch, I think it's better style to
> use "==" in the condition than "!=". To me,
>
> if (GuidHob != NULL) {
> //
> // do a bunch of stuff
> //
> } else {
> //
> // error
> //
> }
>
> is more difficult to read than:
>
> if (GuidHob == NULL) {
> //
> // error
> //
> } else {
> //
> // do a bunch of stuff
> //
> }
>
> in particular if the "bunch of stuff" includes nested "if" statements.
>
>
> (2) I think the error message could be improved. I'd suggest removing
> the word "ERROR", since DEBUG_ERROR already sets the error level / mask.
>
> (3) Furthermore, I suggest not logging the name "gEfiAcpiVariableGuid"
> textually, as a string -- in edk2 we generally log GUIDs by value, and
> log parsers / translators usually look for GUID values. Thus I suggest
> using %g with
>
> (4) Please consider logging the module name and/or the function name
> too, with "%a", and gEfiCallerBaseName and/or __FUNCTION__.
> "gEfiCallerBaseName" is usually only helpful with libraries (because
> they can be linked into multiple drivers), but __FUNCTION__ is more
> frequently helpful.
>
> Thanks
> Laszlo
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: add message for S3 config error
On 09/12/18 02:32, Wang, Jian J wrote:
> Laszlo,
>
> Thanks for the comment. I think it’ll ok to add it in a separate patch.
>
> Just a little confuse about “a separate patch”. Does it mean a separate patch
> file
> in the same patch series or a separate patch which needs a separate BZ
> tracker?
Separate patch in the same series should be fine.
IMO the second patch can refer to the same BZ, or not even refer to any
BZ at all.
Thanks,
Laszlo
>
> Regards,
> Jian
>
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Tuesday, September 11, 2018 9:52 PM
> To: Zeng, Star ; Wang, Jian J ;
> edk2-devel@lists.01.org
> Cc: You, Benjamin ; Dong, Eric
> Subject: Re: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: add message for S3 config
> error
>
> On 09/10/18 07:07, Zeng, Star wrote:
>> I agree to add the ASSERT, but even with the ASSERT, I still suggest moving
>> //
>> // Patch SmmS3ResumeState->SmmS3Cr3
>> //
>> InitSmmS3Cr3 ();
>>
>> into
>> GuidHob = GetFirstGuidHob ();
>> if (GuidHob != NULL) {
>> ...
>> }
>>
>> With that, Reviewed-by: Star Zeng
>> mailto:star.z...@intel.com>>
>
> I think that's a valid idea, but shouldn't it be done in a separate
> patch? One patch for the assert, and another moving InitSmmS3Cr3() under
> the right condition. Does that sound OK?
>
> Thanks
> Laszlo
>
>
>>
>>
>> Thanks,
>> Star
>> -Original Message-
>> From: Wang, Jian J
>> Sent: Monday, September 10, 2018 11:22 AM
>> To: edk2-devel@lists.01.org
>> Cc: Zeng, Star mailto:star.z...@intel.com>>; You,
>> Benjamin mailto:benjamin@intel.com>>; Dong, Eric
>> mailto:eric.d...@intel.com>>; Laszlo Ersek
>> mailto:ler...@redhat.com>>
>> Subject: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: add message for S3 config error
>>
>> BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1165
>>
>> HOB gEfiAcpiVariableGuid is a must have data for S3 resume if
>> PcdAcpiS3Enable is set to TRUE. Current code in CpuS3.c doesn't embody this
>> strong binding between them. An error message and ASSERT is added by this
>> patch to warn platform developer about it.
>>
>> Cc: Star Zeng mailto:star.z...@intel.com>>
>> Cc: Benjamin You mailto:benjamin@intel.com>>
>> Cc: Eric Dong mailto:eric.d...@intel.com>>
>> Cc: Laszlo Ersek mailto:ler...@redhat.com>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Jian J Wang
>> mailto:jian.j.w...@intel.com>>
>> ---
>> UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
>> b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
>> index abd8a5a07b..f371667c44 100644
>> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
>> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
>> @@ -744,6 +744,9 @@ InitSmmS3ResumeState (
>> if (sizeof (UINTN) == sizeof (UINT32)) {
>>SmmS3ResumeState->Signature = SMM_S3_RESUME_SMM_32;
>> }
>> + } else {
>> +DEBUG ((DEBUG_ERROR, "ERROR: HOB(gEfiAcpiVariableGuid) needed by S3
>> resume doesn't exist!\n"));
>> +ASSERT (FALSE);
>>}
>>
>>//
>> --
>> 2.16.2.windows.1
>>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling MTRRs prior to MTRR change.
On 09/11/18 21:47, Duran, Leo wrote:
>
>
>> -Original Message-
>> From: Laszlo Ersek
>> Sent: Tuesday, September 11, 2018 1:50 PM
>> To: Duran, Leo ; edk2-devel@lists.01.org
>> Cc: Eric Dong ; Ruiyu Ni
>> Subject: Re: [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling MTRRs
>> prior to MTRR change.
>>
>> On 09/11/18 17:41, Leo Duran wrote:
>>> The default behavior is to disable MTRRs prior to an MTRR change.
>>> However, on SMT platforms with shared CPU resources it may be
>>> desirable to skip the default behavior, and leave the current state of the
>> Enable bit.
>>>
>>> Cc: Eric Dong
>>> Cc: Laszlo Ersek
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Leo Duran
>>> ---
>>> UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 10 +++---
>>> UefiCpuPkg/Library/MtrrLib/MtrrLib.inf | 3 +++
>>> UefiCpuPkg/UefiCpuPkg.dec | 7 +++
>>> 3 files changed, 17 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
>>> b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
>>> index dfce9a9..baf9a0f 100644
>>> --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
>>> +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
>>> @@ -6,6 +6,8 @@
>>> except for MtrrSetAllMtrrs() which is used to sync BSP's MTRR setting
>>> to
>> APs.
>>>
>>>Copyright (c) 2008 - 2018, Intel Corporation. All rights
>>> reserved.
>>> + Copyright (c) 2018, AMD Inc. All rights reserved.
>>> +
>>>This program and the accompanying materials
>>>are licensed and made available under the terms and conditions of the
>> BSD License
>>>which accompanies this distribution. The full text of the license
>>> may be found at @@ -310,9 +312,11 @@ MtrrLibPreMtrrChange (
>>>//
>>>// Disable MTRRs
>>>//
>>> - DefType.Uint64 = AsmReadMsr64 (MSR_IA32_MTRR_DEF_TYPE);
>>> - DefType.Bits.E = 0;
>>> - AsmWriteMsr64 (MSR_IA32_MTRR_DEF_TYPE, DefType.Uint64);
>>> + if (!PcdGetBool (PcdSkipDisableMtrrsOnPreMtrrChange)) {
>>> +DefType.Uint64 = AsmReadMsr64 (MSR_IA32_MTRR_DEF_TYPE);
>>> +DefType.Bits.E = 0;
>>> +AsmWriteMsr64 (MSR_IA32_MTRR_DEF_TYPE, DefType.Uint64); }
>>> }
>>>
>>> /**
>>> diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
>> b/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
>>> index a97cc61..06f33e8 100644
>>> --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
>>> +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
>>> @@ -2,6 +2,8 @@
>>> # MTRR library provides APIs for MTRR operation.
>>> #
>>> # Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
>>> +# Copyright (c) 2018, AMD Inc. All rights reserved.
>>> +#
>>> # This program and the accompanying materials
>>> # are licensed and made available under the terms and conditions of the
>> BSD License
>>> # which accompanies this distribution. The full text of the license may
>>> be
>> found at
>>> @@ -43,4 +45,5 @@
>>>
>>> [Pcd]
>>>gUefiCpuPkgTokenSpaceGuid.PcdCpuNumberOfReservedVariableMtrrs
>> ## SOMETIMES_CONSUMES
>>> + gUefiCpuPkgTokenSpaceGuid.PcdSkipDisableMtrrsOnPreMtrrChange
>> ## CONSUMES
>>>
>>> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
>>> index 69d777a..64ec763 100644
>>> --- a/UefiCpuPkg/UefiCpuPkg.dec
>>> +++ b/UefiCpuPkg/UefiCpuPkg.dec
>>> @@ -2,6 +2,7 @@
>>> # This Package provides UEFI compatible CPU modules and libraries.
>>> #
>>> # Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.
>>> +# Copyright (c) 2018, AMD Inc. All rights reserved.
>>> #
>>> # This program and the accompanying materials are licensed and made
>> available under
>>> # the terms and conditions of the BSD License which accompanies this
>> distribution.
>>> @@ -273,6 +274,12 @@
>>># @Prompt Current boot is a power-on reset.
>>>
>> gUefiCpuPkgTokenSpaceGuid.PcdIsPowerOnReset|FALSE|BOOLEAN|0x
>> 001B
>>>
>>> + ## Indicates desired behavior for disabling MTRRs prior to MTRR
>> change.
>>> + # TRUE - Skip disabling MTRRs prior to MTRR change.
>>> + # FALSE - Disable MTRRs prior to MTRR change.
>>> + # @Prompt Desired behavior for disabling MTRRs prior to MTRR change.
>>> +
>> gUefiCpuPkgTokenSpaceGuid.PcdSkipDisableMtrrsOnPreMtrrChange|FALSE
>> |BOOLEAN|0x001E
>>> +
>>> [PcdsDynamic, PcdsDynamicEx]
>>>## Contains the pointer to a CPU S3 data buffer of structure
>> ACPI_CPU_DATA.
>>># @Prompt The pointer to a CPU S3 data buffer.
>>>
>>
>> Recently, Ray has written several & significant patches for MtrrLib; I'm
>> adding him.
>>
>> I don't understand the motivation behind this patch. As far as I
>> remember (which is admittedly "quite vaguely"), the SDM requires all
>> logical processors to program their MTRRs identically in parallel. That
>> is, there should be a start line where all the CPUs wait for each other,
>> then they all set up their MTRRs, then they all wait until they all
>> finish, then they all go their merry ways. AIUI the CPU MP PPI and
>> protocol implement this already. I don't understand in what
[edk2] [PATCH V2] BaseTools: Latter full value should overwrite the former field value.
For structure Pcd, the latter full assign value in commandLine should
override the former field assign value. For example in commandLine,
build --pcd Token.pcd.field="haha" --pcd Token.pcd=H"{0x01,0x02}",
the former field value "haha" will be ignored and overwrite by the latter
full value "{0x01,0x02}".
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: ZhiqiangX Zhao
Cc: Liming Gao
Cc: Yonghong Zhu
Cc: Bob Feng
---
BaseTools/Source/Python/Workspace/DscBuildData.py | 17 +
BaseTools/Source/Python/build/BuildReport.py | 20
2 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index 88ba415c5a..4e505c1e99 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -1032,6 +1032,23 @@ class DscBuildData(PlatformBuildClassObject):
PcdItem = BuildData.Pcds[key]
if (TokenSpaceGuidCName, TokenCName) ==
(PcdItem.TokenSpaceGuidCName, PcdItem.TokenCName) and FieldName =="":
PcdItem.DefaultValue = pcdvalue
+#In command line, the latter full assign value in commandLine should
override the former field assign value.
+#For example, --pcd Token.pcd.field="" --pcd Token.pcd=H"{}"
+delete_assign = []
+field_assign = {}
+if GlobalData.BuildOptionPcd:
+for pcdTuple in GlobalData.BuildOptionPcd:
+TokenSpaceGuid, Token, Field = pcdTuple[0], pcdTuple[1],
pcdTuple[2]
+if Field:
+if (TokenSpaceGuid, Token) not in field_assign:
+field_assign[TokenSpaceGuid, Token] = []
+field_assign[TokenSpaceGuid, Token].append(pcdTuple)
+else:
+if (TokenSpaceGuid, Token) in field_assign:
+delete_assign.extend(field_assign[TokenSpaceGuid,
Token])
+field_assign[TokenSpaceGuid, Token] = []
+for item in delete_assign:
+GlobalData.BuildOptionPcd.remove(item)
@staticmethod
def HandleFlexiblePcd(TokenSpaceGuidCName, TokenCName, PcdValue,
PcdDatumType, GuidDict, FieldName=''):
diff --git a/BaseTools/Source/Python/build/BuildReport.py
b/BaseTools/Source/Python/build/BuildReport.py
index a598d64244..3886a7a55e 100644
--- a/BaseTools/Source/Python/build/BuildReport.py
+++ b/BaseTools/Source/Python/build/BuildReport.py
@@ -982,12 +982,16 @@ class PcdReport(object):
PcdValue = DecDefaultValue
if DscDefaultValue:
PcdValue = DscDefaultValue
-Pcd.DefaultValue = PcdValue
+#The DefaultValue of StructurePcd already be the latest, no
need to update.
+if not self.IsStructurePcd(Pcd.TokenCName,
Pcd.TokenSpaceGuidCName):
+Pcd.DefaultValue = PcdValue
if ModulePcdSet is not None:
if (Pcd.TokenCName, Pcd.TokenSpaceGuidCName, Type) not in
ModulePcdSet:
continue
InfDefaultValue, PcdValue = ModulePcdSet[Pcd.TokenCName,
Pcd.TokenSpaceGuidCName, Type]
-Pcd.DefaultValue = PcdValue
+#The DefaultValue of StructurePcd already be the latest,
no need to update.
+if not self.IsStructurePcd(Pcd.TokenCName,
Pcd.TokenSpaceGuidCName):
+Pcd.DefaultValue = PcdValue
if InfDefaultValue:
try:
InfDefaultValue =
ValueExpressionEx(InfDefaultValue, Pcd.DatumType, self._GuidDict)(True)
@@ -1003,7 +1007,9 @@ class PcdReport(object):
if pcd[2]:
continue
PcdValue = pcd[3]
-Pcd.DefaultValue = PcdValue
+#The DefaultValue of StructurePcd already be the
latest, no need to update.
+if not self.IsStructurePcd(Pcd.TokenCName,
Pcd.TokenSpaceGuidCName):
+Pcd.DefaultValue = PcdValue
BuildOptionMatch = True
break
@@ -1050,7 +1056,7 @@ class PcdReport(object):
DscMatch = (DscDefaultValue.strip() ==
PcdValue.strip())
IsStructure = False
-if GlobalData.gStructurePcd and (self.Arch in
GlobalData.gStructurePcd) and ((Pcd.TokenCName, Pcd.TokenSpaceGuidCName) in
GlobalData.gStructurePcd[self.Arch]):
+if self.IsStructurePcd(Pcd.TokenCName,
Pcd.TokenSpaceGuidCName):
IsStructure = True
if TypeName in ('DYNVPD', 'DEXVPD'):
SkuInfoList = Pcd.SkuInfoList
@@
Re: [edk2] [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value in HTTP header.
Reviewed-by: Fu Siyuan
> -Original Message-
> From: Wu, Jiaxin
> Sent: Tuesday, September 4, 2018 3:17 PM
> To: edk2-devel@lists.01.org
> Cc: Stephen Benjamin ; Laszlo Ersek
> ; Ye, Ting ; Fu, Siyuan
> ; Wu, Jiaxin
> Subject: [Patch] MdeModulePkg/Library/DxeHttpLib: Handle the blank value
> in HTTP header.
>
> This patch is to resolve the lock-up issue if the value of HTTP header
> is blank. The issue is recorded @
> https://bugzilla.tianocore.org/show_bug.cgi?id=1102.
>
> Cc: Stephen Benjamin
> Cc: Laszlo Ersek
> Cc: Ye Ting
> Cc: Fu Siyuan
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin
> ---
> MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 57 +++-
> 1 file changed, 44 insertions(+), 13 deletions(-)
>
> diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> index 5fbb50d03a..2fc3da8a2d 100644
> --- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> +++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> @@ -1595,63 +1595,94 @@ HttpGetFieldNameAndValue (
> return NULL;
>}
>
>//
>// Each header field consists of a name followed by a colon (":") and
> the field value.
> + // The field value MAY be preceded by any amount of LWS, though a
> single SP is preferred.
> + //
> + // message-header = field-name ":" [ field-value ]
> + // field-name = token
> + // field-value = *( field-content | LWS )
> + //
> + // Note: "*(element)" allows any number element, including zero;
> "1*(element)" requires at least one element.
> + // [element] means element is optional.
> + // LWS = [CRLF] 1*(SP|HT), it can be ' ' or '\t' or '\r\n ' or
> '\r\n\t'.
> + // CRLF = '\r\n'.
> + // SP = ' '.
> + // HT = '\t' (Tab).
>//
>FieldNameStr = String;
>FieldValueStr = AsciiStrGetNextToken (FieldNameStr, ':');
>if (FieldValueStr == NULL) {
> return NULL;
>}
>
>//
> - // Replace ':' with 0
> + // Replace ':' with 0, then FieldName has been retrived from String.
>//
>*(FieldValueStr - 1) = 0;
>
>//
> - // The field value MAY be preceded by any amount of LWS, though a
> single SP is preferred.
> - // Note: LWS = [CRLF] 1*(SP|HT), it can be '\r\n ' or '\r\n\t' or ' '
> or '\t'.
> - // CRLF = '\r\n'.
> - // SP = ' '.
> - // HT = '\t' (Tab).
> + // Handle FieldValueStr, skip all the preceded LWS.
>//
>while (TRUE) {
> if (*FieldValueStr == ' ' || *FieldValueStr == '\t') {
>//
>// Boundary condition check.
>//
>if ((UINTN) EndofHeader - (UINTN) FieldValueStr < 1) {
> +//
> +// Wrong String format!
> +//
> return NULL;
>}
>
>FieldValueStr ++;
> } else if (*FieldValueStr == '\r') {
>//
>// Boundary condition check.
>//
>if ((UINTN) EndofHeader - (UINTN) FieldValueStr < 3) {
> -return NULL;
> +//
> +// No more preceded LWS, so break here.
> +//
> +break;
>}
>
> - if (*(FieldValueStr + 1) == '\n' && (*(FieldValueStr + 2) == ' ' ||
> *(FieldValueStr + 2) == '\t')) {
> -FieldValueStr = FieldValueStr + 3;
> + if (*(FieldValueStr + 1) == '\n' ) {
> +if (*(FieldValueStr + 2) == ' ' || *(FieldValueStr + 2) == '\t')
> {
> + FieldValueStr = FieldValueStr + 3;
> +} else {
> + //
> + // No more preceded LWS, so break here.
> + //
> + break;
> +}
> + } else {
> +//
> +// Wrong String format!
> +//
> +return NULL;
>}
> } else {
> + //
> + // No more preceded LWS, so break here.
> + //
>break;
> }
>}
>
> - //
> - // Header fields can be extended over multiple lines by preceding each
> extra
> - // line with at least one SP or HT.
> - //
>StrPtr = FieldValueStr;
>do {
> +//
> +// Handle the LWS within the field value.
> +//
> StrPtr = AsciiStrGetNextToken (StrPtr, '\r');
> if (StrPtr == NULL || *StrPtr != '\n') {
> + //
> + // Wrong String format!
> + //
>return NULL;
> }
>
> StrPtr++;
>} while (*StrPtr == ' ' || *StrPtr == '\t');
> --
> 2.17.1.windows.2
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch v2] MdeModulePkg/Ip4Dxe: Sync the direct route entry setting.
Reviewed-by: Fu Siyuan
> -Original Message-
> From: Wu, Jiaxin
> Sent: Tuesday, September 4, 2018 3:38 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Wu,
> Jiaxin
> Subject: [Patch v2] MdeModulePkg/Ip4Dxe: Sync the direct route entry
> setting.
>
> v2: use "IP & Netmask" directly instead of defining an additional variable.
>
> This patch is to sync the direct route entry setting in both the default
> and Instance route table {Subnet, Mask, NextHope} (
> https://bugzilla.tianocore.org/show_bug.cgi?id=1143).
>
> Cc: Ye Ting
> Cc: Fu Siyuan
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin
> Reviewed-by: Ye Ting
> ---
> MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 7 ---
> MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c| 10 +++---
> 2 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> index c19a72730e..b52542cd84 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> @@ -557,17 +557,10 @@ Ip4Config2SetDefaultAddr (
> return Status;
>}
> }
>}
>
> - Ip4AddRoute (
> -IpSb->DefaultRouteTable,
> -StationAddress,
> -SubnetMask,
> -IP4_ALLZERO_ADDRESS
> -);
> -
>//
>// Add a route for the connected network.
>//
>Subnet = StationAddress & SubnetMask;
>
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> index 6a26143e30..13ebeab1be 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> @@ -670,14 +670,18 @@ Ip4ConfigProtocol (
>
>InsertTailList (>Interfaces, >Link);
> }
>
> //
> -// Add a route to this connected network in the route table
> +// Add a route to this connected network in the instance route table.
> //
> -Ip4AddRoute (IpInstance->RouteTable, Ip, Netmask,
> IP4_ALLZERO_ADDRESS);
> -
> +Ip4AddRoute (
> + IpInstance->RouteTable,
> + Ip & Netmask,
> + Netmask,
> + IP4_ALLZERO_ADDRESS
> + );
>} else {
> //
> // Use the default address. Check the state.
> //
> if (IpSb->State == IP4_SERVICE_UNSTARTED) {
> --
> 2.17.1.windows.2
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH V2] BaseTools: Check the array index and the array size of structure PCD.
V2:
1. Add comments for each ASSERT.
2. ASSERT need to skip the case of array size of array as zero. For
example, TestArray[] in struct in header file.
V1:
For structure PCD,
1. use compiler time assert to check the array index, report error
if the buffer overflow happens.
2. use compiler time assert to check the array size, report error
if the user declared size in header file is smaller than the user
defined in DEC/DSC file.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: ZhiqiangX Zhao
Cc: Liming Gao
Cc: Yonghong Zhu
Cc: Bob Feng
---
BaseTools/Source/C/Common/PcdValueCommon.h| 7 +++
BaseTools/Source/Python/Workspace/DscBuildData.py | 8
2 files changed, 15 insertions(+)
diff --git a/BaseTools/Source/C/Common/PcdValueCommon.h
b/BaseTools/Source/C/Common/PcdValueCommon.h
index 3922428ded..255afdfcc3 100644
--- a/BaseTools/Source/C/Common/PcdValueCommon.h
+++ b/BaseTools/Source/C/Common/PcdValueCommon.h
@@ -22,6 +22,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
EXPRESS OR IMPLIED.
#define __ARRAY_ELEMENT_SIZE(TYPE, Field) (sizeof((TYPE *)0)->Field[0])
#define __OFFSET_OF(TYPE, Field) ((UINT32) &(((TYPE *)0)->Field))
#define __FLEXIBLE_SIZE(Size, TYPE, Field, MaxIndex) if (__FIELD_SIZE(TYPE,
Field) == 0) Size = MAX((__OFFSET_OF(TYPE, Field) + __ARRAY_ELEMENT_SIZE(TYPE,
Field) * (MaxIndex)), Size)
+#define __ARRAY_SIZE(Array) (sizeof(Array)/sizeof(Array[0]))
+
+#if defined(_MSC_EXTENSIONS)
+#define __STATIC_ASSERT static_assert
+#else
+#define __STATIC_ASSERT _Static_assert
+#endif
VOID
PcdEntryPoint (
diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index 88ba415c5a..9c4da91151 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -1773,8 +1773,12 @@ class DscBuildData(PlatformBuildClassObject):
#
CApp = CApp + ' FieldSize = __FIELD_SIZE(%s, %s);\n' %
(Pcd.DatumType, FieldName)
CApp = CApp + ' Value = %s; // From %s Line %d Value
%s\n' % (DscBuildData.IntToCString(Value, ValueSize), FieldList[FieldName][1],
FieldList[FieldName][2], FieldList[FieldName][0])
+CApp = CApp + ' __STATIC_ASSERT((__FIELD_SIZE(%s, %s) >=
%d) || (__FIELD_SIZE(%s, %s) == 0), "Input buffer exceeds the buffer array");
// From %s Line %d Value %s\n' % (Pcd.DatumType, FieldName, ValueSize,
Pcd.DatumType, FieldName, FieldList[FieldName][1], FieldList[FieldName][2],
FieldList[FieldName][0])
CApp = CApp + ' memcpy (>%s, Value, (FieldSize > 0
&& FieldSize < %d) ? FieldSize : %d);\n' % (FieldName, ValueSize, ValueSize)
else:
+if '[' in FieldName and ']' in FieldName:
+Index = int(FieldName.split('[')[1].split(']')[0])
+CApp = CApp + ' __STATIC_ASSERT((%d <
__ARRAY_SIZE(Pcd->%s)) || (__ARRAY_SIZE(Pcd->%s) == 0), "array checker error");
// From %s Line %d Index of %s\n' % (Index, FieldName.split('[')[0],
FieldName.split('[')[0], FieldList[FieldName][1], FieldList[FieldName][2],
FieldName)
if ValueSize > 4:
CApp = CApp + ' Pcd->%s = %dULL; // From %s Line %d
Value %s\n' % (FieldName, Value, FieldList[FieldName][1],
FieldList[FieldName][2], FieldList[FieldName][0])
else:
@@ -1852,8 +1856,12 @@ class DscBuildData(PlatformBuildClassObject):
#
CApp = CApp + ' FieldSize = __FIELD_SIZE(%s, %s);\n'
% (Pcd.DatumType, FieldName)
CApp = CApp + ' Value = %s; // From %s Line %d
Value %s\n' % (DscBuildData.IntToCString(Value, ValueSize),
FieldList[FieldName][1], FieldList[FieldName][2], FieldList[FieldName][0])
+CApp = CApp + ' __STATIC_ASSERT((__FIELD_SIZE(%s, %s)
>= %d) || (__FIELD_SIZE(%s, %s) == 0), "Input buffer exceeds the buffer
array"); // From %s Line %d Value %s\n' % (Pcd.DatumType, FieldName, ValueSize,
Pcd.DatumType, FieldName, FieldList[FieldName][1], FieldList[FieldName][2],
FieldList[FieldName][0])
CApp = CApp + ' memcpy (>%s, Value, (FieldSize >
0 && FieldSize < %d) ? FieldSize : %d);\n' % (FieldName, ValueSize, ValueSize)
else:
+if '[' in FieldName and ']' in FieldName:
+Index = int(FieldName.split('[')[1].split(']')[0])
+CApp = CApp + ' __STATIC_ASSERT((%d <
__ARRAY_SIZE(Pcd->%s)) || (__ARRAY_SIZE(Pcd->%s) == 0), "array checker error");
// From %s Line %d Index of %s\n' % (Index, FieldName.split('[')[0],
FieldName.split('[')[0], FieldList[FieldName][1], FieldList[FieldName][2],
FieldName)
if ValueSize > 4:
CApp = CApp + ' Pcd->%s = %dULL; // From %s Line
%d
Re: [edk2] [PATCH] BaseTools: Check the index of array.
Zhiqiang:
I have two comments.
1. Please update commit title with the detail message.
2. Please update the patch to skip the case of file size as zero, and also add
comments for each ASSERT.
Thanks
Liming
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Zhaozh1x
>Sent: Tuesday, September 04, 2018 5:26 PM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming
>Subject: [edk2] [PATCH] BaseTools: Check the index of array.
>
>For structure PCD, add the checker for the ARRAY
>element assignment and Buffer FieldSize.
>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: ZhiqiangX Zhao
>Cc: Liming Gao
>Cc: Yonghong Zhu
>Cc: Bob Feng
>---
> BaseTools/Source/C/Common/PcdValueCommon.h| 7 +++
> BaseTools/Source/Python/Workspace/DscBuildData.py | 8
> 2 files changed, 15 insertions(+)
>
>diff --git a/BaseTools/Source/C/Common/PcdValueCommon.h
>b/BaseTools/Source/C/Common/PcdValueCommon.h
>index 3922428ded..255afdfcc3 100644
>--- a/BaseTools/Source/C/Common/PcdValueCommon.h
>+++ b/BaseTools/Source/C/Common/PcdValueCommon.h
>@@ -22,6 +22,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
>KIND, EITHER EXPRESS OR IMPLIED.
> #define __ARRAY_ELEMENT_SIZE(TYPE, Field) (sizeof((TYPE *)0)->Field[0])
> #define __OFFSET_OF(TYPE, Field) ((UINT32) &(((TYPE *)0)->Field))
> #define __FLEXIBLE_SIZE(Size, TYPE, Field, MaxIndex) if (__FIELD_SIZE(TYPE,
>Field) == 0) Size = MAX((__OFFSET_OF(TYPE, Field) +
>__ARRAY_ELEMENT_SIZE(TYPE, Field) * (MaxIndex)), Size)
>+#define __ARRAY_SIZE(Array) (sizeof(Array)/sizeof(Array[0]))
>+
>+#if defined(_MSC_EXTENSIONS)
>+#define __STATIC_ASSERT static_assert
>+#else
>+#define __STATIC_ASSERT _Static_assert
>+#endif
>
> VOID
> PcdEntryPoint (
>diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
>b/BaseTools/Source/Python/Workspace/DscBuildData.py
>index 748452623f..98055bcb50 100644
>--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
>+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
>@@ -1766,8 +1766,12 @@ class DscBuildData(PlatformBuildClassObject):
> #
> CApp = CApp + ' FieldSize = __FIELD_SIZE(%s, %s);\n' %
>(Pcd.DatumType, FieldName)
> CApp = CApp + ' Value = %s; // From %s Line %d Value
> %s\n' %
>(DscBuildData.IntToCString(Value, ValueSize), FieldList[FieldName][1],
>FieldList[FieldName][2], FieldList[FieldName][0])
>+CApp = CApp + ' __STATIC_ASSERT(__FIELD_SIZE(%s, %s) >=
>%d,
>"Input buffer exceeds the buffer array");\n' % (Pcd.DatumType, FieldName,
>ValueSize)
> CApp = CApp + ' memcpy (>%s, Value, (FieldSize > 0
> &&
>FieldSize < %d) ? FieldSize : %d);\n' % (FieldName, ValueSize, ValueSize)
> else:
>+if '[' in FieldName and ']' in FieldName:
>+Index = int(FieldName.split('[')[1].split(']')[0])
>+CApp = CApp + ' __STATIC_ASSERT((%d <
>__ARRAY_SIZE(Pcd-
>>%s)) || (__ARRAY_SIZE(Pcd->%s) == 0), "array checker error");\n' % (Index,
>FieldName.split('[')[0], FieldName.split('[')[0])
> if ValueSize > 4:
> CApp = CApp + ' Pcd->%s = %dULL; // From %s Line %d
>Value %s\n' % (FieldName, Value, FieldList[FieldName][1],
>FieldList[FieldName][2], FieldList[FieldName][0])
> else:
>@@ -1848,8 +1852,12 @@ class DscBuildData(PlatformBuildClassObject):
> #
> CApp = CApp + ' FieldSize = __FIELD_SIZE(%s, %s);\n'
> %
>(Pcd.DatumType, FieldName)
> CApp = CApp + ' Value = %s; // From %s Line %d
> Value %s\n' %
>(DscBuildData.IntToCString(Value, ValueSize), FieldList[FieldName][1],
>FieldList[FieldName][2], FieldList[FieldName][0])
>+CApp = CApp + ' __STATIC_ASSERT(__FIELD_SIZE(%s, %s)
>>= %d,
>"Input buffer exceeds the buffer array");\n' % (Pcd.DatumType, FieldName,
>ValueSize)
> CApp = CApp + ' memcpy (>%s, Value, (FieldSize
> > 0 &&
>FieldSize < %d) ? FieldSize : %d);\n' % (FieldName, ValueSize, ValueSize)
> else:
>+if '[' in FieldName and ']' in FieldName:
>+Index = int(FieldName.split('[')[1].split(']')[0])
>+CApp = CApp + ' __STATIC_ASSERT((%d <
>__ARRAY_SIZE(Pcd-
>>%s)) || (__ARRAY_SIZE(Pcd->%s) == 0), "array checker error");\n' % (Index,
>FieldName.split('[')[0], FieldName.split('[')[0])
> if ValueSize > 4:
> CApp = CApp + ' Pcd->%s = %dULL; // From %s Line
> %d
>Value %s\n' % (FieldName, Value, FieldList[FieldName][1],
>FieldList[FieldName][2], FieldList[FieldName][0])
> else:
>--
>2.14.1.windows.1
>
>___
>edk2-devel mailing list
>edk2-devel@lists.01.org
Re: [edk2] [PATCH v2 0/4] SdMmc fixes
Hi, So far there was one minor remark from Ard (about macro definition) - I would appreciate any comments before I can submit v3. Thanks, Marcin pt., 7 wrz 2018 o 13:04 Ard Biesheuvel napisał(a): > +Hao > > On 7 September 2018 at 11:10, Marcin Wojtas wrote: > > Hi, > > > > Answering the review request, I extracted SdMmcPciHcDxe driver fixes > > from SdMmcOverride protocol modification. Comparing to v1, > > patches are rebased onto the newest master branch and also a macro > > is used instead of the raw value in SdMmcHcReset. > > > > Patches are available in the github: > > > https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/sdmmc-fixes-r20180907 > > > > I'm looking forward to the comments and remarks. > > > > Best regards, > > Marcin > > > > Changelog: > > v1 -> v2 > > * rebase on top of the newest master > > * resolve conflicts after taking fixes out from new features > > * 3/4 - use macro instead of raw value in SdMmcHcReset > > > > Marcin Wojtas (3): > > MdeModulePkg/SdMmcPciHcDxe: Fix HS200 operation > > MdeModulePkg/SdMmcPciHcDxe: Adjust eMMC clock and bus width sequence > > MdeModulePkg/SdMmcPciHcDxe: Execute card detect only for RemovableSlot > > > > Tomasz Michalec (1): > > MdeModulePkg/SdMmcPciHcDxe: Fix SdMmcHcReset to set only necesery bits > > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 10 + > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c| 39 > +--- > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 18 ++--- > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 6 +-- > > 4 files changed, 34 insertions(+), 39 deletions(-) > > > > -- > > 2.7.4 > > > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 0/2] Add ArmPkg/Optee library APIs
On Mon, 27 Aug 2018 at 17:20, Sumit Garg wrote: > > Changes in v2: > 1. Separate patch for MdePkg/Include/IndustryStandard/GlobalPlatform.h. > 2. Correct comments style for struct members. > 3. Update commit message. > > Sumit Garg (2): > MdePkg/IndustryStandard: Add Global Plaform header file > ArmPkg/OpteeLib: Add APIs to communicate with OP-TEE > I hope you have reviewed this patch-set. Please let me know if you have any more review comments. -Sumit > ArmPkg/Include/Library/OpteeLib.h| 87 ++ > ArmPkg/Library/OpteeLib/Optee.c | 358 > +++ > ArmPkg/Library/OpteeLib/OpteeLib.inf | 2 + > ArmPkg/Library/OpteeLib/OpteeSmc.h | 43 +++ > MdePkg/Include/IndustryStandard/GlobalPlatform.h | 27 ++ > 5 files changed, 517 insertions(+) > create mode 100644 ArmPkg/Library/OpteeLib/OpteeSmc.h > create mode 100644 MdePkg/Include/IndustryStandard/GlobalPlatform.h > > -- > 2.7.4 > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch][edk2-platforms/devel-IntelAtomProcessorE3900] Enable Vtd in setup option
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: xianhu2x --- .../Common/PlatformSettings/PlatformSetupDxe/Cpu.vfi | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformSetupDxe/Cpu.vfi b/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformSetupDxe/Cpu.vfi index 9094794de2..f98673c65a 100644 --- a/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformSetupDxe/Cpu.vfi +++ b/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformSetupDxe/Cpu.vfi @@ -42,16 +42,16 @@ form formid = CPU_CONFIGURATION_FORM_ID, oneof varid = Setup.VTdEnable, prompt = STRING_TOKEN(STR_VTD_PROMPT), help= STRING_TOKEN(STR_VTD_HELP), -option text = STRING_TOKEN(STR_DISABLE), value = 0, flags = MANUFACTURING | DEFAULT | RESET_REQUIRED; -option text = STRING_TOKEN(STR_ENABLE), value = 1, flags = RESET_REQUIRED; +option text = STRING_TOKEN(STR_DISABLE), value = 0, flags = RESET_REQUIRED; +option text = STRING_TOKEN(STR_ENABLE), value = 1, flags = MANUFACTURING | DEFAULT | RESET_REQUIRED; endoneof; grayoutif ideqval Setup.VTdEnable == 0; oneof varid = Setup.PrebootVTdEnable, prompt = STRING_TOKEN(STR_PREBOOT_VTD_PROMPT), help= STRING_TOKEN(STR_PREBOOT_VTD_HELP), -option text = STRING_TOKEN(STR_DISABLE), value = 0, flags = MANUFACTURING | DEFAULT | RESET_REQUIRED; -option text = STRING_TOKEN(STR_ENABLE), value = 1, flags = RESET_REQUIRED; +option text = STRING_TOKEN(STR_DISABLE), value = 0, flags = RESET_REQUIRED; +option text = STRING_TOKEN(STR_ENABLE), value = 1, flags = MANUFACTURING | DEFAULT | RESET_REQUIRED; endoneof; endif; endif; -- 2.14.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch][edk2-platforms/devel-IntelAtomProcessorE3900] Disable the fourth PCIE port
Disable the fourth PCIE port for UP2 board because it caused Yocto S3 failure. It is a temporary solution of platform. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Guo Mang CC: David Wei --- Platform/BroxtonPlatformPkg/Board/UP2/BoardInitPostMem/BoardInit.c | 5 + .../Board/UP2/BoardInitPostMem/BoardInitPostMem.inf | 3 ++- .../Common/Library/PeiFspPolicyInitLib/PeiFspPolicyInitLib.inf | 3 ++- .../Common/Library/PeiFspPolicyInitLib/PeiFspScPolicyInitLib.c | 4 +++- Platform/BroxtonPlatformPkg/PlatformPkg.dec | 2 ++ 5 files changed, 14 insertions(+), 3 deletions(-) diff --git a/Platform/BroxtonPlatformPkg/Board/UP2/BoardInitPostMem/BoardInit.c b/Platform/BroxtonPlatformPkg/Board/UP2/BoardInitPostMem/BoardInit.c index de07429..b7d526a 100644 --- a/Platform/BroxtonPlatformPkg/Board/UP2/BoardInitPostMem/BoardInit.c +++ b/Platform/BroxtonPlatformPkg/Board/UP2/BoardInitPostMem/BoardInit.c @@ -151,6 +151,11 @@ Up2PostMemInitCallback ( PcdSet8(HdaEndpointI2sRenderSKPVirtualBusId, 5); //I2S6 PcdSet8(HdaEndpointI2sRenderHPVirtualBusId, 5); //I2S6 PcdSet8(HdaEndpointI2sCaptureVirtualBusId, 5); //I2S6 + + // + // Set PcdPciePort3Enable + // + PcdSetBool(PcdPciePort3Enable, FALSE); // // Add init steps here diff --git a/Platform/BroxtonPlatformPkg/Board/UP2/BoardInitPostMem/BoardInitPostMem.inf b/Platform/BroxtonPlatformPkg/Board/UP2/BoardInitPostMem/BoardInitPostMem.inf index be10d85..4cfa196 100644 --- a/Platform/BroxtonPlatformPkg/Board/UP2/BoardInitPostMem/BoardInitPostMem.inf +++ b/Platform/BroxtonPlatformPkg/Board/UP2/BoardInitPostMem/BoardInitPostMem.inf @@ -3,7 +3,7 @@ # # It will detect the board ID. # -# Copyright (c) 2014 - 2017, 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 @@ -73,6 +73,7 @@ gEfiBxtTokenSpaceGuid.HdaEndpointI2sRenderSKPVirtualBusId gEfiBxtTokenSpaceGuid.HdaEndpointI2sRenderHPVirtualBusId gEfiBxtTokenSpaceGuid.HdaEndpointI2sCaptureVirtualBusId + gPlatformModuleTokenSpaceGuid.PcdPciePort3Enable [Guids] gEfiPlatformInfoGuid diff --git a/Platform/BroxtonPlatformPkg/Common/Library/PeiFspPolicyInitLib/PeiFspPolicyInitLib.inf b/Platform/BroxtonPlatformPkg/Common/Library/PeiFspPolicyInitLib/PeiFspPolicyInitLib.inf index 36e1b1d..9259152 100644 --- a/Platform/BroxtonPlatformPkg/Common/Library/PeiFspPolicyInitLib/PeiFspPolicyInitLib.inf +++ b/Platform/BroxtonPlatformPkg/Common/Library/PeiFspPolicyInitLib/PeiFspPolicyInitLib.inf @@ -1,7 +1,7 @@ ## @file # Library functions for Fsp Policy Initialization Library. # -# Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved. +# Copyright (c) 2015 - 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 @@ -90,6 +90,7 @@ gPlatformModuleTokenSpaceGuid.PcdeMMCHostMaxSpeed gPlatformModuleTokenSpaceGuid.PcdHdaVerbTablePtr gPlatformModuleTokenSpaceGuid.HdaVerbTableEntryNum + gPlatformModuleTokenSpaceGuid.PcdPciePort3Enable [Ppis] gSiPolicyPpiGuid ## CONSUMES diff --git a/Platform/BroxtonPlatformPkg/Common/Library/PeiFspPolicyInitLib/PeiFspScPolicyInitLib.c b/Platform/BroxtonPlatformPkg/Common/Library/PeiFspPolicyInitLib/PeiFspScPolicyInitLib.c index f9055db..f541ce4 100644 --- a/Platform/BroxtonPlatformPkg/Common/Library/PeiFspPolicyInitLib/PeiFspScPolicyInitLib.c +++ b/Platform/BroxtonPlatformPkg/Common/Library/PeiFspPolicyInitLib/PeiFspScPolicyInitLib.c @@ -1,7 +1,7 @@ /** @file Implementation of Fsp SC Policy Initialization. - Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved. + Copyright (c) 2015 - 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 @@ -282,6 +282,8 @@ PeiFspScPolicyInit ( FspsUpd->FspsConfig.PcieRpNonSnoopLatencyOverrideValue[PortIndex] = SystemConfiguration->PchPcieNonSnoopLatencyOverrideValue[PortIndex]; FspsUpd->FspsConfig.PtmEnable[PortIndex] = TRUE; } + FspsUpd->FspsConfig.PcieRootPortEn[3] = PcdGetBool(PcdPciePort3Enable); + #if (ENBDT_PF_ENABLE == 1) FspsUpd->FspsConfig.PcieRpClkReqSupported[0] = TRUE; FspsUpd->FspsConfig.PcieRpClkReqNumber [0] = 2; diff --git a/Platform/BroxtonPlatformPkg/PlatformPkg.dec b/Platform/BroxtonPlatformPkg/PlatformPkg.dec index e42b6e7..e7b26a9 100644 --- a/Platform/BroxtonPlatformPkg/PlatformPkg.dec +++ b/Platform/BroxtonPlatformPkg/PlatformPkg.dec @@ -212,6 +212,8 @@
Re: [edk2] [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Update SmiEntry function run the same position
Thank you! Reviewed-by: jiewen@intel.com > -Original Message- > From: Gao, Liming > Sent: Wednesday, September 12, 2018 1:14 PM > To: edk2-devel@lists.01.org > Cc: Laszlo Ersek ; Dong, Eric ; > Yao, Jiewen > Subject: [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Update SmiEntry function > run the same position > > 1. Remove jmp _SmiHandler, and run the code at the same position. > 2. Fix up the function call address as the absolute address. > Verify OVMF SMM boot to shell with VS2017, GCC5 and XCODE5 tool chain. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Liming Gao > Cc: Laszlo Ersek > Cc: Eric Dong > Cc: Jiewen Yao > --- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 34 > + > 1 file changed, 25 insertions(+), 9 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > index 315d0f8..d8259de 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > @@ -173,9 +173,6 @@ SmiHandlerIdtrAbsAddr: > mov gs, eax > mov ax, [rbx + DSC_SS] > mov ss, eax > -mov rax, strict qword 0 ; mov rax, > _SmiHandler > -_SmiHandlerAbsAddr: > -jmp rax > > _SmiHandler: > mov rbx, [rsp + 0x8] ; rcx <- CpuIndex > @@ -189,13 +186,19 @@ _SmiHandler: > add rsp, -0x20 > > mov rcx, rbx > -callASM_PFX(CpuSmmDebugEntry) > +mov rax, strict qword 0 ; call > ASM_PFX(CpuSmmDebugEntry) > +CpuSmmDebugEntryAbsAddr: > +callrax > > mov rcx, rbx > -callASM_PFX(SmiRendezvous) > +mov rax, strict qword 0 ; call > ASM_PFX(SmiRendezvous) > +SmiRendezvousAbsAddr: > +callrax > > mov rcx, rbx > -callASM_PFX(CpuSmmDebugExit) > +mov rax, strict qword 0 ; call > ASM_PFX(CpuSmmDebugExit) > +CpuSmmDebugExitAbsAddr: > +callrax > > add rsp, 0x20 > > @@ -206,7 +209,8 @@ _SmiHandler: > > add rsp, 0x200 > > -lea rax, [ASM_PFX(mXdSupported)] > +mov rax, strict qword 0 ; lea rax, > [ASM_PFX(mXdSupported)] > +mXdSupportedAbsAddr: > mov al, [rax] > cmp al, 0 > jz .1 > @@ -230,7 +234,19 @@ ASM_PFX(PiSmmCpuSmiEntryFixupAddress): > learcx, [SmiHandlerIdtrAbsAddr] > movqword [rcx - 8], rax > > -learax, [_SmiHandler] > -learcx, [_SmiHandlerAbsAddr] > +learax, [ASM_PFX(CpuSmmDebugEntry)] > +learcx, [CpuSmmDebugEntryAbsAddr] > +movqword [rcx - 8], rax > + > +learax, [ASM_PFX(SmiRendezvous)] > +learcx, [SmiRendezvousAbsAddr] > +movqword [rcx - 8], rax > + > +learax, [ASM_PFX(CpuSmmDebugExit)] > +learcx, [CpuSmmDebugExitAbsAddr] > +movqword [rcx - 8], rax > + > +learax, [ASM_PFX(mXdSupported)] > +learcx, [mXdSupportedAbsAddr] > movqword [rcx - 8], rax > ret > -- > 2.10.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] BaseTool: Support different PCDs to refer to the same EFI variable.
From: "bob.c.f...@intel.com"
If Structure PCD and Normal Pcd refer to the
same EFI variable, do EFI variable merge, otherwise, do
EFI variable combination.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng
Cc: Liming Gao
---
BaseTools/Source/Python/AutoGen/AutoGen.py| 3 +-
BaseTools/Source/Python/AutoGen/GenVar.py | 71 ++-
.../Python/Workspace/BuildClassObject.py | 2 +
.../Source/Python/Workspace/DscBuildData.py | 2 +
4 files changed, 60 insertions(+), 18 deletions(-)
diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 4ce0489446..064b85fafe 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -1213,7 +1213,7 @@ class PlatformAutoGen(AutoGen):
VariableGuidStructure = Sku.VariableGuidValue
VariableGuid =
GuidStructureStringToGuidString(VariableGuidStructure)
for StorageName in Sku.DefaultStoreDict:
-VariableInfo.append_variable(var_info(Index, pcdname,
StorageName, SkuName, StringToArray(Sku.VariableName), VariableGuid,
Sku.VariableOffset, Sku.VariableAttribute, Sku.HiiDefaultValue,
Sku.DefaultStoreDict[StorageName], Pcd.DatumType))
+VariableInfo.append_variable(var_info(Index, pcdname,
StorageName, SkuName, StringToArray(Sku.VariableName), VariableGuid,
Sku.VariableOffset, Sku.VariableAttribute, Sku.HiiDefaultValue,
Sku.DefaultStoreDict[StorageName], Pcd.DatumType,
Pcd.CustomAttribute['DscPosition'], Pcd.CustomAttribute.get('IsStru',False)))
Index += 1
return VariableInfo
@@ -2112,6 +2112,7 @@ class PlatformAutoGen(AutoGen):
ToPcd.validateranges = FromPcd.validateranges
ToPcd.validlists = FromPcd.validlists
ToPcd.expressions = FromPcd.expressions
+ToPcd.CustomAttribute = FromPcd.CustomAttribute
if FromPcd is not None and ToPcd.DatumType == TAB_VOID and not
ToPcd.MaxDatumSize:
EdkLogger.debug(EdkLogger.DEBUG_9, "No MaxDatumSize specified for
PCD %s.%s" \
diff --git a/BaseTools/Source/Python/AutoGen/GenVar.py
b/BaseTools/Source/Python/AutoGen/GenVar.py
index 75d455b407..036f00e2bb 100644
--- a/BaseTools/Source/Python/AutoGen/GenVar.py
+++ b/BaseTools/Source/Python/AutoGen/GenVar.py
@@ -22,7 +22,7 @@ from Common.Misc import *
import collections
import Common.DataType as DataType
-var_info = collections.namedtuple("uefi_var",
"pcdindex,pcdname,defaultstoragename,skuname,var_name, var_guid,
var_offset,var_attribute,pcd_default_value, default_value, data_type")
+var_info = collections.namedtuple("uefi_var",
"pcdindex,pcdname,defaultstoragename,skuname,var_name, var_guid,
var_offset,var_attribute,pcd_default_value, default_value,
data_type,PcdDscLine,StructurePcd")
NvStorageHeaderSize = 28
VariableHeaderSize = 32
@@ -56,6 +56,51 @@ 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]
+
