Re: [edk2] [PATCH 0/5] expire the use of PcdSetNxForStack
Thanks to simply the PCD usage. Reviewed-by: jiewen@intel.com > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] > Sent: Tuesday, September 11, 2018 1:17 PM > To: edk2-devel@lists.01.org > Subject: [edk2] [PATCH 0/5] expire the use of PcdSetNxForStack > > 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. > > Jian J Wang (5): > MdeModulePkg/DxeIplPeim: expire the use of PcdSetNxForStack > OvmfPkg/PlatformPei: expire the use of PcdSetNxForStack > OvmfPkg: expire the use of PcdSetNxForStack > ArmVirtPkg/ArmVirt.dsc.inc: expire the use of PcdSetNxForStack > MdeModulePkg: expire PcdSetNxForStack > > ArmVirtPkg/ArmVirt.dsc.inc | 5 - > MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c | 6 +- > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 2 +- > MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 3 ++- > MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 2 +- > MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 14 > +++--- > MdeModulePkg/MdeModulePkg.dec| 10 +- > MdeModulePkg/MdeModulePkg.uni| 10 +- > OvmfPkg/OvmfPkgIa32.dsc | 1 - > OvmfPkg/OvmfPkgIa32X64.dsc | 1 - > OvmfPkg/OvmfPkgX64.dsc | 1 - > OvmfPkg/PlatformPei/Platform.c | 1 - > OvmfPkg/PlatformPei/PlatformPei.inf | 1 - > 13 files changed, 22 insertions(+), 35 deletions(-) > > -- > 2.16.2.windows.1 > > ___ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 5/5] MdeModulePkg: expire PcdSetNxForStack
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. Set BIT4 of PcdDxeNxMemoryProtectionPolicy if NX protection is needed for stack. Cc: Star Zeng Cc: Laszlo Ersek Cc: Ard Biesheuvel Cc: Ruiyu Ni Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/MdeModulePkg.dec | 10 +- MdeModulePkg/MdeModulePkg.uni | 10 +- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 74a699cbb7..b1f208909c 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -1320,6 +1320,7 @@ # # NOTE: User must NOT set NX protection for EfiLoaderCode / EfiBootServicesCode / EfiRuntimeServicesCode. # User MUST set the same NX protection for EfiBootServicesData and EfiConventionalMemory. + # Stack is allocated as type of EfiBootServicesData. Enable NX protection for it will also enable NX protection for stack. # # e.g. 0x7FD5 can be used for all memory except Code. # e.g. 0x7BD4 can be used for all memory except Code and ACPINVS/Reserved. @@ -1886,15 +1887,6 @@ # @Prompt Default Creator Revision for ACPI table creation. gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision|0x0113|UINT32|0x30001038 - ## Indicates if to set NX for stack. - # For the DxeIpl and the DxeCore are both X64, set NX for stack feature also require PcdDxeIplBuildPageTables be TRUE. - # For the DxeIpl and the DxeCore are both IA32 (PcdDxeIplSwitchToLongMode is FALSE), set NX for stack feature also require - # IA32 PAE is supported and Execute Disable Bit is available. - # TRUE - to set NX for stack. - # FALSE - Not to set NX for stack. - # @Prompt Set NX for stack. - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE|BOOLEAN|0x0001006f - ## This PCD specifies the PCI-based SD/MMC host controller mmio base address. # Define the mmio base address of the pci-based SD/MMC host controller. If there are multiple SD/MMC # host controllers, their mmio base addresses are calculated one by one from this base address. diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index 080b8a62c0..6b26b21f00 100644 --- a/MdeModulePkg/MdeModulePkg.uni +++ b/MdeModulePkg/MdeModulePkg.uni @@ -339,15 +339,6 @@ #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSerialRegisterStride_HELP #language en-US "The number of bytes between registers in serial device. The default is 1 byte." -#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSetNxForStack_PROMPT #language en-US "Set NX for stack" - -#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSetNxForStack_HELP #language en-US "Indicates if to set NX for stack." - "For the DxeIpl and the DxeCore are both X64, set NX for stack feature also require PcdDxeIplBuildPageTables be TRUE." - "For the DxeIpl and the DxeCore are both IA32 (PcdDxeIplSwitchToLongMode is FALSE), set NX for stack feature also require" - "IA32 PAE is supported and Execute Disable Bit is available." - "TRUE - to set NX for stack." - "FALSE - Not to set NX for stack." - #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiS3Enable_PROMPT #language en-US "ACPI S3 Enable" #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiS3Enable_HELP #language en-US "Indicates if ACPI S3 will be enabled." @@ -1129,6 +1120,7 @@ "\n" "NOTE: User must NOT set NX protection for EfiLoaderCode / EfiBootServicesCode / EfiRuntimeServicesCode. \n" "User MUST set the same NX protection for EfiBootServicesData and EfiConventionalMemory. \n" + "Stack is allocated as type of EfiBootServicesData. Enable NX protection for it will also enable NX protection for stack. \n" "\n" "e.g. 0x7FD5 can be used for all memo
[edk2] [PATCH 4/5] ArmVirtPkg/ArmVirt.dsc.inc: expire the use of PcdSetNxForStack
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116 Since PcdSetNxForStack is expired, remove all references. The default setting of PcdDxeNxMemoryProtectionPolicy has covered PcdSetNxForStack. Cc: Laszlo Ersek Cc: Star Zeng Cc: Ard Biesheuvel Cc: Ruiyu Ni Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- ArmVirtPkg/ArmVirt.dsc.inc | 5 - 1 file changed, 5 deletions(-) diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc index 70a0ac4d78..d87ae5a32d 100644 --- a/ArmVirtPkg/ArmVirt.dsc.inc +++ b/ArmVirtPkg/ArmVirt.dsc.inc @@ -383,11 +383,6 @@ # gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC0007FD1 - # - # Enable the non-executable DXE stack. (This gets set up by DxeIpl) - # - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE - [PcdsFixedAtBuild.ARM] gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40 -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/5] expire the use of PcdSetNxForStack
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. Jian J Wang (5): MdeModulePkg/DxeIplPeim: expire the use of PcdSetNxForStack OvmfPkg/PlatformPei: expire the use of PcdSetNxForStack OvmfPkg: expire the use of PcdSetNxForStack ArmVirtPkg/ArmVirt.dsc.inc: expire the use of PcdSetNxForStack MdeModulePkg: expire PcdSetNxForStack ArmVirtPkg/ArmVirt.dsc.inc | 5 - MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c | 6 +- MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 2 +- MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 3 ++- MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 2 +- MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 14 +++--- MdeModulePkg/MdeModulePkg.dec| 10 +- MdeModulePkg/MdeModulePkg.uni| 10 +- OvmfPkg/OvmfPkgIa32.dsc | 1 - OvmfPkg/OvmfPkgIa32X64.dsc | 1 - OvmfPkg/OvmfPkgX64.dsc | 1 - OvmfPkg/PlatformPei/Platform.c | 1 - OvmfPkg/PlatformPei/PlatformPei.inf | 1 - 13 files changed, 22 insertions(+), 35 deletions(-) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/5] MdeModulePkg/DxeIplPeim: expire the use of PcdSetNxForStack
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. Cc: Star Zeng Cc: Laszlo Ersek Cc: Ard Biesheuvel Cc: Ruiyu Ni Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c | 6 +- MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 2 +- MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 3 ++- MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 2 +- MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 14 +++--- 5 files changed, 20 insertions(+), 7 deletions(-) diff --git a/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c index 176d361f19..d44b845b76 100644 --- a/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c +++ b/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c @@ -45,7 +45,11 @@ HandOffToDxeCore ( BaseOfStack = AllocatePages (EFI_SIZE_TO_PAGES (STACK_SIZE)); ASSERT (BaseOfStack != NULL); - if (PcdGetBool (PcdSetNxForStack)) { + // + // Set stack to non-executable, if EfiBootServicesData type of memory is + // set for NX protection. + // + if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & BIT4) != 0) { Status = ArmSetMemoryRegionNoExec ((UINTN)BaseOfStack, STACK_SIZE); ASSERT_EFI_ERROR (Status); } diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf index fd82657404..44b6ea84ff 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf @@ -116,7 +116,7 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64] - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ## SOMETIMES_CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## SOMETIMES_CONSUMES [Depex] gEfiPeiLoadFilePpiGuid AND gEfiPeiMasterBootModePpiGuid diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c index d28baa3615..854078e6dd 100644 --- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c +++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c @@ -245,7 +245,8 @@ ToBuildPageTable ( return TRUE; } - if (PcdGetBool (PcdSetNxForStack) && IsExecuteDisableBitAvailable ()) { + if (PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0 && + IsExecuteDisableBitAvailable ()) { return TRUE; } diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c index 81efcfe93d..eb53bc9417 100644 --- a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c +++ b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c @@ -94,7 +94,7 @@ HandOffToDxeCore ( // Set NX for stack feature also require PcdDxeIplBuildPageTables be TRUE // for the DxeIpl and the DxeCore are both X64. // -ASSERT (PcdGetBool (PcdSetNxForStack) == FALSE); +ASSERT (PcdGet64 (PcdDxeNxMemoryProtectionPolicy) == 0); ASSERT (PcdGetBool (PcdCpuStackGuard) == FALSE); } diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c index 496e219913..27e9d6955d 100644 --- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c +++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c @@ -152,7 +152,11 @@ ToSplitPageTable ( } } - if (PcdGetBool (PcdSetNxForStack)) { + // + // Set stack to non-executable, if EfiBootServicesData type of memory is + // set for NX protection. + // + if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & BIT4) != 0) { if ((Address < StackBase + StackSize) && ((Address + Size) > StackBase)) { return TRUE; } @@ -314,7 +318,11 @@ Split2MPageTo4K ( PageTableEntry->Bits.Present = 1; } -if (PcdGetBool (PcdSetNxForStack) +// +// Set stack to non-executable, if EfiBootServicesData type of memory is +// set for NX protection. +// +if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & BIT4) != 0 && (PhysicalAddress4K >= StackBase) && (PhysicalAddress4K < StackBase + StackSize)) { // @@ -755,7 +763,7 @@ CreateIdentityMappingPageTables ( // EnablePageTableProtection ((UINTN)PageMap, TRUE); - if (PcdGetBool (PcdSetNxForStack)) { + if (PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0) { EnableExecuteDisableBit (); } -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/5] OvmfPkg/PlatformPei: expire the use of PcdSetNxForStack
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 -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 3/5] OvmfPkg: expire the use of PcdSetNxForStack
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116 Since PcdSetNxForStack is expired, remove related references in dsc file. 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/OvmfPkgIa32.dsc| 1 - OvmfPkg/OvmfPkgIa32X64.dsc | 1 - OvmfPkg/OvmfPkgX64.dsc | 1 - 3 files changed, 3 deletions(-) diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index 9f07e75050..1eaefbfd6e 100644 --- a/OvmfPkg/OvmfPkgIa32.dsc +++ b/OvmfPkg/OvmfPkgIa32.dsc @@ -559,7 +559,6 @@ gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE # Noexec settings for DXE. - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE # UefiCpuPkg PCDs related to initial AP bringup and general AP management. diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc index a4eaeb808c..6f0424c862 100644 --- a/OvmfPkg/OvmfPkgIa32X64.dsc +++ b/OvmfPkg/OvmfPkgIa32X64.dsc @@ -567,7 +567,6 @@ gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE # Noexec settings for DXE. - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE # UefiCpuPkg PCDs related to initial AP bringup and general AP management. diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index aa3efc5e73..da6b1293e3 100644 --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc @@ -566,7 +566,6 @@ gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE # Noexec settings for DXE. - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE # UefiCpuPkg PCDs related to initial AP bringup and general AP management. -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] UefiCpuPkg/CpuMpPei: suppress compiler complaining
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1166 Cc: Dandan Bi Cc: Hao A Wu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuMpPei/CpuPaging.c | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c index bcb942a8e5..a63421a1af 100644 --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c @@ -517,7 +517,7 @@ GetStackBase ( IN OUT VOID *Buffer ) { - EFI_PHYSICAL_ADDRESSStackBase; + volatile EFI_PHYSICAL_ADDRESS StackBase; StackBase = (EFI_PHYSICAL_ADDRESS)(UINTN)&StackBase; StackBase += BASE_4KB; @@ -554,6 +554,8 @@ SetupStackGuardPage ( MpInitLibGetNumberOfProcessors(&NumberOfProcessors, NULL); MpInitLibWhoAmI (&Bsp); for (Index = 0; Index < NumberOfProcessors; ++Index) { +StackBase = 0; + if (Index == Bsp) { Hob.Raw = GetHobList (); while ((Hob.Raw = GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, Hob.Raw)) != NULL) { @@ -570,12 +572,19 @@ SetupStackGuardPage ( // MpInitLibStartupThisAP(GetStackBase, Index, NULL, 0, (VOID *)&StackBase, NULL); } -// -// Set Guard page at stack base address. -// -ConvertMemoryPageAttributes(StackBase, EFI_PAGE_SIZE, 0); -DEBUG ((DEBUG_INFO, "Stack Guard set at %lx [cpu%lu]!\n", -(UINT64)StackBase, (UINT64)Index)); + +if (StackBase == 0) { + DEBUG ((DEBUG_ERROR, "Stack base address was not found for [cpu%lu]!\n", + (UINT64)Index)); + ASSERT(StackBase != 0); +} else { + // + // Set Guard page at stack base address. + // + ConvertMemoryPageAttributes(StackBase, EFI_PAGE_SIZE, 0); + DEBUG ((DEBUG_INFO, "Stack Guard set at %lx [cpu%lu]!\n", + (UINT64)StackBase, (UINT64)Index)); +} } // -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch] BaseTools: Report error for incorrect hex value format
From: zhijufan The case is user use 0x1} as a hex value for Pcd, it directly cause tool report traceback info. This patch add more error info. Cc: Liming Gao Cc: Yonghong Zhu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Zhiju.Fan --- BaseTools/Source/Python/Common/Misc.py | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/BaseTools/Source/Python/Common/Misc.py b/BaseTools/Source/Python/Common/Misc.py index 2cf9574..430bf6b 100644 --- a/BaseTools/Source/Python/Common/Misc.py +++ b/BaseTools/Source/Python/Common/Misc.py @@ -1412,11 +1412,14 @@ def ParseFieldValue (Value): if Value.startswith('DEVICE_PATH(') and Value.endswith(')'): Value = Value.replace("DEVICE_PATH(", '').rstrip(')') Value = Value.strip().strip('"') return ParseDevPathValue(Value) if Value.lower().startswith('0x'): -Value = int(Value, 16) +try: +Value = int(Value, 16) +except: +raise BadExpression("invalid hex value: %s" % Value) if Value == 0: return 0, 1 return Value, (Value.bit_length() + 7) / 8 if Value[0].isdigit(): Value = int(Value, 10) -- 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] MdeModulePkg XhciDxe: Set HSEE Bit if SERR# Enable Bit is set
Reviewed-by: Ruiyu Ni Thanks/Ray > -Original Message- > From: Zeng, Star > Sent: Tuesday, September 11, 2018 10:04 AM > To: edk2-devel@lists.01.org > Cc: Zeng, Star ; Ni, Ruiyu ; Wang, > Jian J ; Wang, Fei1 > Subject: [PATCH] MdeModulePkg XhciDxe: Set HSEE Bit if SERR# Enable Bit is > set > > When the HSEE in the USBCMD bit is a '1' and the HSE bit in the USBSTS > register is a '1', the xHC shall assert out-of-band error signaling to the > host > and assert the SERR# pin. > To prevent masking any potential issues with SERR, this patch is to set > USBCMD Host System Error Enable(HSEE) Bit if PCICMD SERR# Enable Bit is > set. > > Cc: Ruiyu Ni > Cc: Jian J Wang > Cc: Fei1 Wang > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Star Zeng > --- > MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 41 > ++ > 1 file changed, 41 insertions(+) > > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > index 5f0736a516b6..89f073e1d83f 100644 > --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > @@ -587,6 +587,39 @@ XhcIsSysError ( > } > > /** > + Set USBCMD Host System Error Enable(HSEE) Bit if PCICMD SERR# Enable > Bit is set. > + > + The USBCMD HSEE Bit will be reset to default 0 by USBCMD Host Controller > Reset(HCRST). > + This function is to set USBCMD HSEE Bit if PCICMD SERR# Enable Bit is set. > + > + @param XhcThe XHCI Instance. > + > +**/ > +VOID > +XhcSetHsee ( > + IN USB_XHCI_INSTANCE *Xhc > + ) > +{ > + EFI_STATUSStatus; > + EFI_PCI_IO_PROTOCOL *PciIo; > + UINT16XhciCmd; > + > + PciIo = Xhc->PciIo; > + Status = PciIo->Pci.Read ( > +PciIo, > +EfiPciIoWidthUint16, > +PCI_COMMAND_OFFSET, > +sizeof (XhciCmd), > +&XhciCmd > +); > + if (!EFI_ERROR (Status)) { > +if ((XhciCmd & EFI_PCI_COMMAND_SERR) != 0) { > + XhcSetOpRegBit (Xhc, XHC_USBCMD_OFFSET, XHC_USBCMD_HSEE); > +} > + } > +} > + > +/** >Reset the XHCI host controller. > >@param Xhc The XHCI Instance. > @@ -628,6 +661,14 @@ XhcResetHC ( > // > gBS->Stall (XHC_1_MILLISECOND); > Status = XhcWaitOpRegBit (Xhc, XHC_USBCMD_OFFSET, > XHC_USBCMD_RESET, FALSE, Timeout); > + > +if (!EFI_ERROR (Status)) { > + // > + // The USBCMD HSEE Bit will be reset to default 0 by USBCMD HCRST. > + // Set USBCMD HSEE Bit if PCICMD SERR# Enable Bit is set. > + // > + XhcSetHsee (Xhc); > +} >} > >return Status; > -- > 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] 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-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, > > > > 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 da69f6ff5e..ce3bce04f5 100644 > > > --- a/MdePkg/Include/Library/SynchronizationLib.h > > > +++ b/MdePkg/Include/Library/SynchronizationLib.h > > > @@ -144,8 +144,7 @@ ReleaseSpinLock ( > > > > > >Performs an atomic increment of the 32-bit unsigned integer > > > specified by > > >Value and returns the incremented value. The increment operation > > > must be > > > - performed using MP safe mechanisms. The state of the return value > > > is not > > > - guaranteed to be MP safe. > > > + performed using MP safe mechanisms. > > > > > >If Value is NULL, then ASSERT(). > > > > > > @@ -166,8 +165,7 @@ InterlockedIncrement ( > > > > > >Performs an atomic decrement of the 32-bit unsigned integer > > > specified by > > >Value and returns the decremented value. The decrement operation > > > must be > > > - performed using MP safe mechanisms. The state of the return value > > > is not > > > - guaranteed to be MP safe. > > > + performed using MP safe mechanisms. > > > > > >If Value is NULL, then ASSERT(). > > > > > > diff --git > > > a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat > > > ionLib.inf
Re: [edk2] [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-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, > > 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 da69f6ff5e..ce3bce04f5 100644 > > --- a/MdePkg/Include/Library/SynchronizationLib.h > > +++ b/MdePkg/Include/Library/SynchronizationLib.h > > @@ -144,8 +144,7 @@ ReleaseSpinLock ( > > > >Performs an atomic increment of the 32-bit unsigned integer > > specified by > >Value and returns the incremented value. The increment operation > > must be > > - performed using MP safe mechanisms. The state of the return value > > is not > > - guaranteed to be MP safe. > > + performed using MP safe mechanisms. > > > >If Value is NULL, then ASSERT(). > > > > @@ -166,8 +165,7 @@ InterlockedIncrement ( > > > >Performs an atomic decrement of the 32-bit unsigned integer > > specified by > >Value and returns the decremented value. The decrement operation > > must be > > - performed using MP safe mechanisms. The state of the return value > > is not > > - guaranteed to be MP safe. > > + performed using MP safe mechanisms. > > > >If Value is NULL, then ASSERT(). > > > > diff --git > > a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat > > ionLib.inf > > b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat > > ionLib.inf > > index 0be1d4331f..b971cd138d 100755 > > --- > > a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat > > ionLib.inf > > +++ > > b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat > > ionLib.inf > > @@ -34,9 +34,9 @@ [Sources.IA32] > >Ia32/InterlockedCompareExchange64.c | MSFT > >Ia32/InterlockedCompareExchange32.c | MSFT > >Ia32/InterlockedCompareExchange16.c | MSFT > > - Ia32/InterlockedDecrement.c | MSFT > > - Ia32/InterlockedIncrement.c | MSFT > > - SynchronizationMsc.c | MSFT > > + Ia32/InterlockedDecrement.nasm | MSFT > > + Ia32/InterlockedIncrement.nasm | MSFT Synch
[edk2] [PATCH] MdeModulePkg XhciDxe: Set HSEE Bit if SERR# Enable Bit is set
When the HSEE in the USBCMD bit is a ‘1’ and the HSE bit in the USBSTS register is a ‘1’, the xHC shall assert out-of-band error signaling to the host and assert the SERR# pin. To prevent masking any potential issues with SERR, this patch is to set USBCMD Host System Error Enable(HSEE) Bit if PCICMD SERR# Enable Bit is set. Cc: Ruiyu Ni Cc: Jian J Wang Cc: Fei1 Wang Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng --- MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 41 ++ 1 file changed, 41 insertions(+) diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c index 5f0736a516b6..89f073e1d83f 100644 --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c @@ -587,6 +587,39 @@ XhcIsSysError ( } /** + Set USBCMD Host System Error Enable(HSEE) Bit if PCICMD SERR# Enable Bit is set. + + The USBCMD HSEE Bit will be reset to default 0 by USBCMD Host Controller Reset(HCRST). + This function is to set USBCMD HSEE Bit if PCICMD SERR# Enable Bit is set. + + @param XhcThe XHCI Instance. + +**/ +VOID +XhcSetHsee ( + IN USB_XHCI_INSTANCE *Xhc + ) +{ + EFI_STATUSStatus; + EFI_PCI_IO_PROTOCOL *PciIo; + UINT16XhciCmd; + + PciIo = Xhc->PciIo; + Status = PciIo->Pci.Read ( +PciIo, +EfiPciIoWidthUint16, +PCI_COMMAND_OFFSET, +sizeof (XhciCmd), +&XhciCmd +); + if (!EFI_ERROR (Status)) { +if ((XhciCmd & EFI_PCI_COMMAND_SERR) != 0) { + XhcSetOpRegBit (Xhc, XHC_USBCMD_OFFSET, XHC_USBCMD_HSEE); +} + } +} + +/** Reset the XHCI host controller. @param Xhc The XHCI Instance. @@ -628,6 +661,14 @@ XhcResetHC ( // gBS->Stall (XHC_1_MILLISECOND); Status = XhcWaitOpRegBit (Xhc, XHC_USBCMD_OFFSET, XHC_USBCMD_RESET, FALSE, Timeout); + +if (!EFI_ERROR (Status)) { + // + // The USBCMD HSEE Bit will be reset to default 0 by USBCMD HCRST. + // Set USBCMD HSEE Bit if PCICMD SERR# Enable Bit is set. + // + XhcSetHsee (Xhc); +} } return Status; -- 2.7.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 2/9] BaseTools: AutoGen refactor WorkspaceAutoGen class
Update the WorkspaceAutoGen class to use caching decorators and remove the no longer needed private variables. Cc: Liming Gao Cc: Yonghong Zhu Cc: Bob Feng Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jaben Carsey --- BaseTools/Source/Python/AutoGen/AutoGen.py | 69 1 file changed, 28 insertions(+), 41 deletions(-) diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py b/BaseTools/Source/Python/AutoGen/AutoGen.py index 726b1c98f5bc..35dab75785eb 100644 --- a/BaseTools/Source/Python/AutoGen/AutoGen.py +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py @@ -278,10 +278,6 @@ class WorkspaceAutoGen(AutoGen): self.FvTargetList = Fvs if Fvs else [] self.CapTargetList = Caps if Caps else [] self.AutoGenObjectList = [] -self._BuildDir = None -self._FvDir = None -self._MakeFileDir = None -self._BuildCommand = None self._GuidDict = {} # there's many relative directory operations, so ... @@ -810,54 +806,56 @@ class WorkspaceAutoGen(AutoGen): return "%s [%s]" % (self.MetaFile, ", ".join(self.ArchList)) ## Return the directory to store FV files -def _GetFvDir(self): -if self._FvDir is None: -self._FvDir = path.join(self.BuildDir, TAB_FV_DIRECTORY) -return self._FvDir +@cached_property +def FvDir(self): +return path.join(self.BuildDir, TAB_FV_DIRECTORY) ## Return the directory to store all intermediate and final files built -def _GetBuildDir(self): -if self._BuildDir is None: -return self.AutoGenObjectList[0].BuildDir +@cached_property +def BuildDir(self): +return self.AutoGenObjectList[0].BuildDir ## Return the build output directory platform specifies -def _GetOutputDir(self): +@cached_property +def OutputDir(self): return self.Platform.OutputDirectory ## Return platform name -def _GetName(self): +@cached_property +def Name(self): return self.Platform.PlatformName ## Return meta-file GUID -def _GetGuid(self): +@cached_property +def Guid(self): return self.Platform.Guid ## Return platform version -def _GetVersion(self): +@cached_property +def Version(self): return self.Platform.Version ## Return paths of tools -def _GetToolDefinition(self): +@cached_property +def ToolDefinition(self): return self.AutoGenObjectList[0].ToolDefinition ## Return directory of platform makefile # # @retval string Makefile directory # -def _GetMakeFileDir(self): -if self._MakeFileDir is None: -self._MakeFileDir = self.BuildDir -return self._MakeFileDir +@cached_property +def MakeFileDir(self): +return self.BuildDir ## Return build command string # # @retval string Build command string # -def _GetBuildCommand(self): -if self._BuildCommand is None: -# BuildCommand should be all the same. So just get one from platform AutoGen -self._BuildCommand = self.AutoGenObjectList[0].BuildCommand -return self._BuildCommand +@cached_property +def BuildCommand(self): +# BuildCommand should be all the same. So just get one from platform AutoGen +return self.AutoGenObjectList[0].BuildCommand ## Check the PCDs token value conflict in each DEC file. # @@ -933,7 +931,8 @@ class WorkspaceAutoGen(AutoGen): ) Count += 1 ## Generate fds command -def _GenFdsCommand(self): +@property +def GenFdsCommand(self): return (GenMake.TopLevelMakefile(self)._TEMPLATE_.Replace(GenMake.TopLevelMakefile(self)._TemplateDict)).strip() ## Create makefile for the platform and modules in it @@ -966,18 +965,6 @@ class WorkspaceAutoGen(AutoGen): def CreateAsBuiltInf(self): return -Name= property(_GetName) -Guid= property(_GetGuid) -Version = property(_GetVersion) -OutputDir = property(_GetOutputDir) - -ToolDefinition = property(_GetToolDefinition) # toolcode : tool path - -BuildDir= property(_GetBuildDir) -FvDir = property(_GetFvDir) -MakeFileDir = property(_GetMakeFileDir) -BuildCommand= property(_GetBuildCommand) -GenFdsCommand = property(_GenFdsCommand) ## AutoGen class for platform # @@ -2587,7 +2574,7 @@ class ModuleAutoGen(AutoGen): ## Return the module build data object @cached_property def Module(self): -return self.Workspace.BuildDatabase[self.MetaFile, self.Arch, self.BuildTarget, self.ToolChain] +return self.BuildDatabase[self.MetaFile, self.Arch, self.BuildTarget, self.ToolChain] ## Return the modu
[edk2] [PATCH v2 0/9] BaseTools: refactor Workspace classes
update the classes for the following: 1) use decorators for property 2) use decorators for caching property and caching function - this allows for objects to reduce in size as they get used 3) remove unused variables and properties 4) use tuple instead of custom class when apropriate 5) remove callers from accessing "private" data and use the existing properties 6) removed a circular dependency between APIs v2: fix error where class attribute M was accidentally removed. Jaben Carsey (9): BaseTools: Refactor PlatformAutoGen BaseTools: AutoGen refactor WorkspaceAutoGen class BaseTools: AutoGen - refactor class properties BaseTools: refactor class properties BaseTools: Workspace classes refactor properties BaseTools: refactor Build Database objects BaseTools: Don't save unused workspace data BaseTools: refactor to not overcreate ModuleAutoGen objects BaseTools: refactor to cache InfBuildData data BaseTools/Source/Python/AutoGen/AutoGen.py | 692 +++--- BaseTools/Source/Python/AutoGen/GenMake.py | 20 +- BaseTools/Source/Python/Common/Misc.py | 90 +- BaseTools/Source/Python/GenFds/FfsInfStatement.py | 4 +- BaseTools/Source/Python/Workspace/BuildClassObject.py | 39 +- BaseTools/Source/Python/Workspace/DecBuildData.py | 65 +- BaseTools/Source/Python/Workspace/DscBuildData.py | 151 ++-- BaseTools/Source/Python/Workspace/InfBuildData.py | 954 +--- BaseTools/Source/Python/Workspace/MetaFileParser.py| 18 +- BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 16 +- BaseTools/Source/Python/build/build.py | 4 +- 11 files changed, 933 insertions(+), 1120 deletions(-) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 5/9] BaseTools: Workspace classes refactor properties
1) use decorators 2) also change some private functions to public when all callers are external 3) change external callers to use functions instead of directly accessing private data. Cc: Liming Gao Cc: Yonghong Zhu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jaben Carsey --- BaseTools/Source/Python/GenFds/FfsInfStatement.py | 4 +- BaseTools/Source/Python/Workspace/DecBuildData.py | 58 +++--- BaseTools/Source/Python/Workspace/DscBuildData.py | 151 +++--- BaseTools/Source/Python/Workspace/InfBuildData.py | 212 ++-- BaseTools/Source/Python/Workspace/MetaFileParser.py| 18 +- BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 16 +- BaseTools/Source/Python/build/build.py | 4 +- 7 files changed, 228 insertions(+), 235 deletions(-) diff --git a/BaseTools/Source/Python/GenFds/FfsInfStatement.py b/BaseTools/Source/Python/GenFds/FfsInfStatement.py index 56bb966698ad..6149bc81b4ef 100644 --- a/BaseTools/Source/Python/GenFds/FfsInfStatement.py +++ b/BaseTools/Source/Python/GenFds/FfsInfStatement.py @@ -341,9 +341,7 @@ class FfsInfStatement(FfsInfStatementClassObject): self.InfModule = Inf self.PcdIsDriver = Inf.PcdIsDriver self.IsBinaryModule = Inf.IsBinaryModule -Inf._GetDepex() -Inf._GetDepexExpression() -if len(Inf._Depex.data) > 0 and len(Inf._DepexExpression.data) > 0: +if len(Inf.Depex.data) > 0 and len(Inf.DepexExpression.data) > 0: self.Depex = True GenFdsGlobalVariable.VerboseLogger("BaseName : %s" % self.BaseName) diff --git a/BaseTools/Source/Python/Workspace/DecBuildData.py b/BaseTools/Source/Python/Workspace/DecBuildData.py index 45beaebc63ef..1f74e898f2ef 100644 --- a/BaseTools/Source/Python/Workspace/DecBuildData.py +++ b/BaseTools/Source/Python/Workspace/DecBuildData.py @@ -99,21 +99,22 @@ class DecBuildData(PackageBuildClassObject): self._CommonIncludes= None self._LibraryClasses= None self._Pcds = None -self.__Macros = None +self._MacroDict = None self._PrivateProtocols = None self._PrivatePpis = None self._PrivateGuids = None self._PrivateIncludes = None ## Get current effective macros -def _GetMacros(self): -if self.__Macros is None: -self.__Macros = {} -self.__Macros.update(GlobalData.gGlobalDefines) -return self.__Macros +@property +def _Macros(self): +if self._MacroDict is None: +self._MacroDict = dict(GlobalData.gGlobalDefines) +return self._MacroDict ## Get architecture -def _GetArch(self): +@property +def Arch(self): return self._Arch ## Retrieve all information in [Defines] section @@ -129,7 +130,8 @@ class DecBuildData(PackageBuildClassObject): self._Header = 'DUMMY' ## Retrieve package name -def _GetPackageName(self): +@property +def PackageName(self): if self._PackageName is None: if self._Header is None: self._GetHeaderInfo() @@ -138,7 +140,8 @@ class DecBuildData(PackageBuildClassObject): return self._PackageName ## Retrieve file guid -def _GetFileGuid(self): +@property +def PackageName(self): if self._Guid is None: if self._Header is None: self._GetHeaderInfo() @@ -147,7 +150,8 @@ class DecBuildData(PackageBuildClassObject): return self._Guid ## Retrieve package version -def _GetVersion(self): +@property +def Version(self): if self._Version is None: if self._Header is None: self._GetHeaderInfo() @@ -156,7 +160,8 @@ class DecBuildData(PackageBuildClassObject): return self._Version ## Retrieve protocol definitions (name/value pairs) -def _GetProtocol(self): +@property +def Protocols(self): if self._Protocols is None: # # tdict is a special kind of dict, used for selecting correct @@ -198,7 +203,8 @@ class DecBuildData(PackageBuildClassObject): return self._Protocols ## Retrieve PPI definitions (name/value pairs) -def _GetPpi(self): +@property +def Ppis(self): if self._Ppis is None: # # tdict is a special kind of dict, used for selecting correct @@ -240,7 +246,8 @@ class DecBuildData(PackageBuildClassObject): return self._Ppis ## Retrieve GUID definitions (name/value pairs) -def _GetGuid(self): +@property +def Guids(self): if self._Guids is None: # # tdict is a special kind of dict, used for selecting correct @@ -282,7 +289,8 @@ class DecBuildData(PackageBuildClassObject): return self._Guids ## Retrieve public include paths declared in this p
[edk2] [PATCH v2 8/9] BaseTools: refactor to not overcreate ModuleAutoGen objects
currently created for 3 different purposes and saved once. this makes it created once and saved and then referenced. Cc: Liming Gao Cc: Yonghong Zhu Cc: Bob Feng Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jaben Carsey --- BaseTools/Source/Python/AutoGen/AutoGen.py | 58 +--- 1 file changed, 25 insertions(+), 33 deletions(-) diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py b/BaseTools/Source/Python/AutoGen/AutoGen.py index 35dab75785eb..4e9fc54dbaf3 100644 --- a/BaseTools/Source/Python/AutoGen/AutoGen.py +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py @@ -1085,21 +1085,19 @@ class PlatformAutoGen(AutoGen): def GenFdsCommand(self): return self.Workspace.GenFdsCommand -## Create makefile for the platform and mdoules in it +## Create makefile for the platform and modules in it # # @param CreateModuleMakeFileFlag indicating if the makefile for # modules will be created as well # def CreateMakeFile(self, CreateModuleMakeFile=False, FfsCommand = {}): if CreateModuleMakeFile: -for ModuleFile in self.Platform.Modules: -Ma = ModuleAutoGen(self.Workspace, ModuleFile, self.BuildTarget, - self.ToolChain, self.Arch, self.MetaFile) -if (ModuleFile.File, self.Arch) in FfsCommand: -Ma.CreateMakeFile(True, FfsCommand[ModuleFile.File, self.Arch]) +for Ma in self._MaList: +key = (Ma.MetaFile.File, self.Arch) +if key in FfsCommand: +Ma.CreateMakeFile(True, FfsCommand[key]) else: Ma.CreateMakeFile(True) -#Ma.CreateAsBuiltInf() # no need to create makefile for the platform more than once if self.IsMakeFileCreated: @@ -1231,16 +1229,12 @@ class PlatformAutoGen(AutoGen): for InfName in self._AsBuildInfList: InfName = mws.join(self.WorkspaceDir, InfName) FdfModuleList.append(os.path.normpath(InfName)) -for F in self.Platform.Modules.keys(): -M = ModuleAutoGen(self.Workspace, F, self.BuildTarget, self.ToolChain, self.Arch, self.MetaFile) -#GuidValue.update(M.Guids) - -self.Platform.Modules[F].M = M - +for M in self._MaList: +#F is the Module for which M is the module autogen for PcdFromModule in M.ModulePcdList + M.LibraryPcdList: # make sure that the "VOID*" kind of datum has MaxDatumSize set if PcdFromModule.DatumType == TAB_VOID and not PcdFromModule.MaxDatumSize: -NoDatumTypePcdList.add("%s.%s [%s]" % (PcdFromModule.TokenSpaceGuidCName, PcdFromModule.TokenCName, F)) +NoDatumTypePcdList.add("%s.%s [%s]" % (PcdFromModule.TokenSpaceGuidCName, PcdFromModule.TokenCName, M.MetaFile)) # Check the PCD from Binary INF or Source INF if M.IsBinaryModule == True: @@ -1250,7 +1244,7 @@ class PlatformAutoGen(AutoGen): PcdFromModule.IsFromDsc = (PcdFromModule.TokenCName, PcdFromModule.TokenSpaceGuidCName) in self.Platform.Pcds if PcdFromModule.Type in PCD_DYNAMIC_TYPE_SET or PcdFromModule.Type in PCD_DYNAMIC_EX_TYPE_SET: -if F.Path not in FdfModuleList: +if M.MetaFile.Path not in FdfModuleList: # If one of the Source built modules listed in the DSC is not listed # in FDF modules, and the INF lists a PCD can only use the PcdsDynamic # access method (it is only listed in the DEC file that declares the @@ -1930,19 +1924,25 @@ class PlatformAutoGen(AutoGen): TokenNumber += 1 return RetVal +@cached_property +def _MaList(self): +for ModuleFile in self.Platform.Modules: +Ma = ModuleAutoGen( + self.Workspace, + ModuleFile, + self.BuildTarget, + self.ToolChain, + self.Arch, + self.MetaFile + ) +self.Platform.Modules[ModuleFile].M = Ma +return [x.M for x in self.Platform.Modules.values()] + ## Summarize ModuleAutoGen objects of all modules to be built for this platform @cached_property def ModuleAutoGenList(self): RetVal = [] -for ModuleFile in self.Platform.Modules: -Ma = ModuleAutoGen( -self.Workspace, -ModuleFile, -self.BuildTarget, -self.ToolChain, -self.Arch, -self.MetaFile -) +for Ma in self._MaList: if Ma not in RetVal: RetVal.append(Ma) return RetVal @@ -1951
[edk2] [PATCH v2 7/9] BaseTools: Don't save unused workspace data
FlexibleFieldName was never used not set. DefinitionPosition (file and line number) are recalculated and never used outside the function. remove the saving of the data. Cc: Liming Gao Cc: Yonghong Zhu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jaben Carsey --- BaseTools/Source/Python/Workspace/BuildClassObject.py | 7 --- BaseTools/Source/Python/Workspace/DecBuildData.py | 7 --- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/BaseTools/Source/Python/Workspace/BuildClassObject.py b/BaseTools/Source/Python/Workspace/BuildClassObject.py index e7259b2d3d50..f4ffd05324f4 100644 --- a/BaseTools/Source/Python/Workspace/BuildClassObject.py +++ b/BaseTools/Source/Python/Workspace/BuildClassObject.py @@ -66,7 +66,6 @@ class PcdClassObject(object): self.DscDefaultValue = Value self.PcdValueFromComm = "" self.PcdValueFromFdf = "" -self.DefinitionPosition = ("","") ## Get the maximum number of bytes def GetPcdMaxSize(self): @@ -168,7 +167,6 @@ class StructurePcd(PcdClassObject): self.DefaultValues = OrderedDict() self.PcdMode = None self.SkuOverrideValues = OrderedDict() -self.FlexibleFieldName = None self.StructName = None self.PcdDefineLineNo = 0 self.PkgPath = "" @@ -200,9 +198,6 @@ class StructurePcd(PcdClassObject): def SetPcdMode (self, PcdMode): self.PcdMode = PcdMode -def SetFlexibleFieldName (self, FlexibleFieldName): -self.FlexibleFieldName = FlexibleFieldName - def copy(self, PcdObject): self.TokenCName = PcdObject.TokenCName if PcdObject.TokenCName else self.TokenCName self.TokenSpaceGuidCName = PcdObject.TokenSpaceGuidCName if PcdObject.TokenSpaceGuidCName else PcdObject.TokenSpaceGuidCName @@ -224,7 +219,6 @@ class StructurePcd(PcdClassObject): self.DscRawValue = PcdObject.DscRawValue if PcdObject.DscRawValue else self.DscRawValue self.PcdValueFromComm = PcdObject.PcdValueFromComm if PcdObject.PcdValueFromComm else self.PcdValueFromComm self.PcdValueFromFdf = PcdObject.PcdValueFromFdf if PcdObject.PcdValueFromFdf else self.PcdValueFromFdf -self.DefinitionPosition = PcdObject.DefinitionPosition if PcdObject.DefinitionPosition else self.DefinitionPosition if isinstance(PcdObject, StructurePcd): self.StructuredPcdIncludeFile = PcdObject.StructuredPcdIncludeFile if PcdObject.StructuredPcdIncludeFile else self.StructuredPcdIncludeFile self.PackageDecs = PcdObject.PackageDecs if PcdObject.PackageDecs else self.PackageDecs @@ -233,7 +227,6 @@ class StructurePcd(PcdClassObject): self.DefaultFromDSC=None self.DefaultValueFromDec = PcdObject.DefaultValueFromDec if PcdObject.DefaultValueFromDec else self.DefaultValueFromDec self.SkuOverrideValues = PcdObject.SkuOverrideValues if PcdObject.SkuOverrideValues else self.SkuOverrideValues -self.FlexibleFieldName = PcdObject.FlexibleFieldName if PcdObject.FlexibleFieldName else self.FlexibleFieldName self.StructName = PcdObject.DatumType if PcdObject.DatumType else self.StructName self.PcdDefineLineNo = PcdObject.PcdDefineLineNo if PcdObject.PcdDefineLineNo else self.PcdDefineLineNo self.PkgPath = PcdObject.PkgPath if PcdObject.PkgPath else self.PkgPath diff --git a/BaseTools/Source/Python/Workspace/DecBuildData.py b/BaseTools/Source/Python/Workspace/DecBuildData.py index 1f74e898f2ef..31ee13eca91f 100644 --- a/BaseTools/Source/Python/Workspace/DecBuildData.py +++ b/BaseTools/Source/Python/Workspace/DecBuildData.py @@ -410,6 +410,7 @@ class DecBuildData(PackageBuildClassObject): if not (PcdCName, TokenSpaceGuid) in PcdSet: PcdSet.append((PcdCName, TokenSpaceGuid)) +DefinitionPosition = {} for PcdCName, TokenSpaceGuid in PcdSet: # # limit the ARCH to self._Arch, if no self._Arch found, tdict @@ -436,7 +437,7 @@ class DecBuildData(PackageBuildClassObject): list(validlists), list(expressions) ) -PcdObj.DefinitionPosition = (self.MetaFile.File, LineNo) +DefinitionPosition[PcdObj] = (self.MetaFile.File, LineNo) if "." in TokenSpaceGuid: StrPcdSet.append((PcdObj, LineNo)) else: @@ -449,10 +450,10 @@ class DecBuildData(PackageBuildClassObject): for pcd in Pcds.values(): if pcd.DatumType not in [TAB_UINT8, TAB_UINT16, TAB_UINT32, TAB_UINT64, TAB_VOID, "BOOLEAN"]: if StructPattern.match(pcd.DatumType) is None: -EdkLogger.error('build', FORMAT_INVALID, "DatumType only support BOOLEAN, UINT8, UINT16, UINT32, UINT64, VOID* or a valid struct name.", pcd.DefinitionP
[edk2] [PATCH v2 9/9] BaseTools: refactor to cache InfBuildData data
use Common.caching and auto cache properties and functions of InfBuildData Cc: Liming Gao Cc: Yonghong Zhu Cc: Bob Feng Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jaben Carsey --- BaseTools/Source/Python/Workspace/InfBuildData.py | 840 +--- 1 file changed, 389 insertions(+), 451 deletions(-) diff --git a/BaseTools/Source/Python/Workspace/InfBuildData.py b/BaseTools/Source/Python/Workspace/InfBuildData.py index 0016cd30ce7a..44d44d24eb6b 100644 --- a/BaseTools/Source/Python/Workspace/InfBuildData.py +++ b/BaseTools/Source/Python/Workspace/InfBuildData.py @@ -13,14 +13,14 @@ # from __future__ import absolute_import -from Common.StringUtils import * from Common.DataType import * from Common.Misc import * +from Common.caching import cached_property, cached_class_function from types import * from .MetaFileParser import * from collections import OrderedDict - from Workspace.BuildClassObject import ModuleBuildClassObject, LibraryClassObject, PcdClassObject + ## Module build information from INF file # # This class is used to retrieve information stored in database and convert them @@ -77,9 +77,9 @@ class InfBuildData(ModuleBuildClassObject): } -## Constructor of DscBuildData +## Constructor of InfBuildData # -# Initialize object of DscBuildData +# Initialize object of InfBuildData # # @param FilePathThe path of platform description file # @param RawData The raw data of DSC file @@ -97,10 +97,37 @@ class InfBuildData(ModuleBuildClassObject): self._Target = Target self._Toolchain = Toolchain self._Platform = TAB_COMMON -self._SourceOverridePath = None if FilePath.Key in GlobalData.gOverrideDir: self._SourceOverridePath = GlobalData.gOverrideDir[FilePath.Key] -self._Clear() +else: +self._SourceOverridePath = None +self._TailComments = None +self._BaseName = None +self._DxsFile = None +self._ModuleType = None +self._ComponentType = None +self._BuildType = None +self._Guid = None +self._Version = None +self._PcdIsDriver = None +self._BinaryModule = None +self._Shadow = None +self._MakefileName = None +self._CustomMakefile = None +self._Specification = None +self._LibraryClass = None +self._ModuleEntryPointList = None +self._ModuleUnloadImageList = None +self._ConstructorList = None +self._DestructorList = None +self._Defs = OrderedDict() +self._ProtocolComments = None +self._PpiComments = None +self._GuidsUsedByPcd = OrderedDict() +self._GuidComments = None +self._PcdComments = None +self._BuildOptions = None +self._DependencyFileList = None ## XXX[key] = value def __setitem__(self, key, value): @@ -114,89 +141,39 @@ class InfBuildData(ModuleBuildClassObject): def __contains__(self, key): return key in self._PROPERTY_ -## Set all internal used members of InfBuildData to None -def _Clear(self): -self._HeaderComments = None -self._TailComments = None -self._Header_ = None -self._AutoGenVersion= None -self._BaseName = None -self._DxsFile = None -self._ModuleType= None -self._ComponentType = None -self._BuildType = None -self._Guid = None -self._Version = None -self._PcdIsDriver = None -self._BinaryModule = None -self._Shadow= None -self._MakefileName = None -self._CustomMakefile= None -self._Specification = None -self._LibraryClass = None -self._ModuleEntryPointList = None -self._ModuleUnloadImageList = None -self._ConstructorList = None -self._DestructorList= None -self._Defs = OrderedDict() -self._Binaries = None -self._Sources = None -self._LibraryClasses= None -self._Libraries = None -self._Protocols = None -self._ProtocolComments = None -self._Ppis = None -self._PpiComments = None -self._Guids = None -self._GuidsUsedByPcd= OrderedDict() -self._GuidComments = None -self._Includes = None -self._Packages = None -self._Pcds = None -self._PcdComments = None -self._BuildOptions = None -self._Depex = None -self._DepexExp
[edk2] [PATCH v2 3/9] BaseTools: AutoGen - refactor class properties
use function decorators Cc: Liming Gao Cc: Yonghong Zhu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jaben Carsey --- BaseTools/Source/Python/AutoGen/GenMake.py | 20 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py b/BaseTools/Source/Python/AutoGen/GenMake.py index 35ee98c82bb1..b4377eef17d7 100644 --- a/BaseTools/Source/Python/AutoGen/GenMake.py +++ b/BaseTools/Source/Python/AutoGen/GenMake.py @@ -455,7 +455,8 @@ cleanlib: self.FfsOutputFileList = [] # Compose a dict object containing information used to do replacement in template -def _CreateTemplateDict(self): +@property +def _TemplateDict(self): if self._FileType not in self._SEP_: EdkLogger.error("build", PARAMETER_INVALID, "Invalid Makefile type [%s]" % self._FileType, ExtraData="[%s]" % str(self._AutoGenObject)) @@ -1095,8 +1096,6 @@ cleanlib: return DependencyList -_TemplateDict = property(_CreateTemplateDict) - ## CustomMakefile class # # This class encapsules makefie and its generation for module. It uses template to generate @@ -1205,7 +1204,8 @@ ${BEGIN}\t-@${create_directory_command}\n${END}\ self.IntermediateDirectoryList = ["$(DEBUG_DIR)", "$(OUTPUT_DIR)"] # Compose a dict object containing information used to do replacement in template -def _CreateTemplateDict(self): +@property +def _TemplateDict(self): Separator = self._SEP_[self._FileType] MyAgo = self._AutoGenObject if self._FileType not in MyAgo.CustomMakefile: @@ -1278,8 +1278,6 @@ ${BEGIN}\t-@${create_directory_command}\n${END}\ return MakefileTemplateDict -_TemplateDict = property(_CreateTemplateDict) - ## PlatformMakefile class # # This class encapsules makefie and its generation for platform. It uses @@ -1396,7 +1394,8 @@ cleanlib: self.LibraryMakeCommandList = [] # Compose a dict object containing information used to do replacement in template -def _CreateTemplateDict(self): +@property +def _TemplateDict(self): Separator = self._SEP_[self._FileType] MyAgo = self._AutoGenObject @@ -1481,8 +1480,6 @@ cleanlib: DirList.append(os.path.join(self._AutoGenObject.BuildDir, LibraryAutoGen.BuildDir)) return DirList -_TemplateDict = property(_CreateTemplateDict) - ## TopLevelMakefile class # # This class encapsules makefie and its generation for entrance makefile. It @@ -1502,7 +1499,8 @@ class TopLevelMakefile(BuildFile): self.IntermediateDirectoryList = [] # Compose a dict object containing information used to do replacement in template -def _CreateTemplateDict(self): +@property +def _TemplateDict(self): Separator = self._SEP_[self._FileType] # any platform autogen object is ok because we just need common information @@ -1622,8 +1620,6 @@ class TopLevelMakefile(BuildFile): DirList.append(os.path.join(self._AutoGenObject.BuildDir, LibraryAutoGen.BuildDir)) return DirList -_TemplateDict = property(_CreateTemplateDict) - # This acts like the main() function for the script, unless it is 'import'ed into another script. if __name__ == '__main__': pass -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 1/9] BaseTools: Refactor PlatformAutoGen
use decorators for property and automatic caching remove circular dependency between some APIs Cc: Liming Gao Cc: Yonghong Zhu Cc: Bob Feng Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jaben Carsey --- BaseTools/Source/Python/AutoGen/AutoGen.py | 587 +--- 1 file changed, 264 insertions(+), 323 deletions(-) diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py b/BaseTools/Source/Python/AutoGen/AutoGen.py index 95370d182157..726b1c98f5bc 100644 --- a/BaseTools/Source/Python/AutoGen/AutoGen.py +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py @@ -1046,41 +1046,13 @@ class PlatformAutoGen(AutoGen): # get the original module/package/platform objects self.BuildDatabase = Workspace.BuildDatabase self.DscBuildDataObj = Workspace.Platform -self._GuidDict = Workspace._GuidDict # flag indicating if the makefile/C-code file has been created or not self.IsMakeFileCreated = False -self.IsCodeFileCreated = False -self._Platform = None -self._Name = None -self._Guid = None -self._Version= None - -self._BuildRule = None -self._SourceDir = None -self._BuildDir = None -self._OutputDir = None -self._FvDir = None -self._MakeFileDir = None -self._FdfFile = None - -self._PcdTokenNumber = None# (TokenCName, TokenSpaceGuidCName) : GeneratedTokenNumber self._DynamicPcdList = None# [(TokenCName1, TokenSpaceGuidCName1), (TokenCName2, TokenSpaceGuidCName2), ...] self._NonDynamicPcdList = None # [(TokenCName1, TokenSpaceGuidCName1), (TokenCName2, TokenSpaceGuidCName2), ...] -self._NonDynamicPcdDict = {} -self._ToolDefinitions = None -self._ToolDefFile = None # toolcode : tool path -self._ToolChainFamily = None -self._BuildRuleFamily = None -self._BuildOption = None # toolcode : option -self._EdkBuildOption = None # edktoolcode : option -self._EdkIIBuildOption = None # edkiitoolcode : option -self._PackageList = None -self._ModuleAutoGenList = None -self._LibraryAutoGenList = None -self._BuildCommand = None self._AsBuildInfList = [] self._AsBuildModuleList = [] @@ -1100,6 +1072,7 @@ class PlatformAutoGen(AutoGen): return True +@cached_class_function def __repr__(self): return "%s [%s]" % (self.MetaFile, self.Arch) @@ -,19 +1084,18 @@ class PlatformAutoGen(AutoGen): # @param CreateModuleCodeFileFlag indicating if creating module's # autogen code file or not # +@cached_class_function def CreateCodeFile(self, CreateModuleCodeFile=False): # only module has code to be greated, so do nothing if CreateModuleCodeFile is False -if self.IsCodeFileCreated or not CreateModuleCodeFile: +if not CreateModuleCodeFile: return for Ma in self.ModuleAutoGenList: Ma.CreateCodeFile(True) -# don't do this twice -self.IsCodeFileCreated = True - ## Generate Fds Command -def _GenFdsCommand(self): +@cached_property +def GenFdsCommand(self): return self.Workspace.GenFdsCommand ## Create makefile for the platform and mdoules in it @@ -1185,7 +1157,6 @@ class PlatformAutoGen(AutoGen): LibAuto.ConstPcd[key] = FixedAtBuildPcds[key] def CollectVariables(self, DynamicPcdSet): - VpdRegionSize = 0 VpdRegionBase = 0 if self.Workspace.FdfFile: @@ -1197,7 +1168,6 @@ class PlatformAutoGen(AutoGen): VpdRegionBase = FdRegion.Offset break - VariableInfo = VariableMgr(self.DscBuildDataObj._GetDefaultStores(), self.DscBuildDataObj._GetSkuIds()) VariableInfo.SetVpdRegionMaxSize(VpdRegionSize) VariableInfo.SetVpdRegionOffset(VpdRegionBase) @@ -1250,7 +1220,6 @@ class PlatformAutoGen(AutoGen): # This interface should be invoked explicitly when platform action is created. # def CollectPlatformDynamicPcds(self): - for key in self.Platform.Pcds: for SinglePcd in GlobalData.MixedPcd: if (self.Platform.Pcds[key].TokenCName, self.Platform.Pcds[key].TokenSpaceGuidCName) == SinglePcd: @@ -1683,308 +1652,318 @@ class PlatformAutoGen(AutoGen): EdkLogger.error("Build", FILE_NOT_FOUND, "Fail to find third-party BPDG tool to process VPD PCDs. BPDG Guid tool need to be defined in tools_def.txt and VPD_TOOL_GUID need to be provided in DSC file.") ## Return the platform build data object -def _GetPlatform(self): -if self._Platform is None: -self._Platform = self.BuildDatabase[self.MetaFile, self.Arch, self.BuildTarget, self.ToolChain] -
[edk2] [PATCH v2 4/9] BaseTools: refactor class properties
use decorators and auto cache those that were cached manually remove properties never used Cc: Liming Gao Cc: Yonghong Zhu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jaben Carsey --- BaseTools/Source/Python/Common/Misc.py | 90 +--- 1 file changed, 38 insertions(+), 52 deletions(-) diff --git a/BaseTools/Source/Python/Common/Misc.py b/BaseTools/Source/Python/Common/Misc.py index 2cf9574326d5..372fdf01805b 100644 --- a/BaseTools/Source/Python/Common/Misc.py +++ b/BaseTools/Source/Python/Common/Misc.py @@ -38,6 +38,7 @@ from Common.LongFilePathSupport import OpenLongFilePath as open from Common.MultipleWorkspace import MultipleWorkspace as mws import uuid from CommonDataClass.Exceptions import BadExpression +from Common.caching import cached_property import subprocess ## Regular expression used to find out place holders in string template gPlaceholderPattern = re.compile("\$\{([^$()\s]+)\}", re.MULTILINE | re.UNICODE) @@ -1719,8 +1720,6 @@ class PathClass(object): self.ToolCode = ToolCode self.ToolChainFamily = ToolChainFamily -self._Key = None - ## Convert the object of this class to a string # # Convert member Path of the class to a string @@ -1773,12 +1772,12 @@ class PathClass(object): def __hash__(self): return hash(self.Path) -def _GetFileKey(self): -if self._Key is None: -self._Key = self.Path.upper() # + self.ToolChainFamily + self.TagName + self.ToolCode + self.Target -return self._Key +@cached_property +def Key(self): +return self.Path.upper() -def _GetTimeStamp(self): +@property +def TimeStamp(self): return os.stat(self.Path)[8] def Validate(self, Type='', CaseSensitive=True): @@ -1817,9 +1816,6 @@ class PathClass(object): self.Path = os.path.join(RealRoot, RealFile) return ErrorCode, ErrorInfo -Key = property(_GetFileKey) -TimeStamp = property(_GetTimeStamp) - ## Parse PE image to get the required PE informaion. # class PeImageClass(): @@ -1929,8 +1925,8 @@ class DefaultStore(): for sid, name in self.DefaultStores.values(): if sid == minid: return name + class SkuClass(): - DEFAULT = 0 SINGLE = 1 MULTIPLE =2 @@ -1951,8 +1947,8 @@ class SkuClass(): self.SkuIdSet = [] self.SkuIdNumberSet = [] self.SkuData = SkuIds -self.__SkuInherit = {} -self.__SkuIdentifier = SkuIdentifier +self._SkuInherit = {} +self._SkuIdentifier = SkuIdentifier if SkuIdentifier == '' or SkuIdentifier is None: self.SkuIdSet = ['DEFAULT'] self.SkuIdNumberSet = ['0U'] @@ -1976,7 +1972,7 @@ class SkuClass(): EdkLogger.error("build", PARAMETER_INVALID, ExtraData="SKU-ID [%s] is not supported by the platform. [Valid SKU-ID: %s]" % (each, " | ".join(SkuIds.keys( -if self.SkuUsageType != self.SINGLE: +if self.SkuUsageType != SkuClass.SINGLE: self.AvailableSkuIds.update({'DEFAULT':0, 'COMMON':0}) if self.SkuIdSet: GlobalData.gSkuids = (self.SkuIdSet) @@ -1990,11 +1986,11 @@ class SkuClass(): GlobalData.gSkuids.sort() def GetNextSkuId(self, skuname): -if not self.__SkuInherit: -self.__SkuInherit = {} +if not self._SkuInherit: +self._SkuInherit = {} for item in self.SkuData.values(): -self.__SkuInherit[item[1]]=item[2] if item[2] else "DEFAULT" -return self.__SkuInherit.get(skuname, "DEFAULT") +self._SkuInherit[item[1]]=item[2] if item[2] else "DEFAULT" +return self._SkuInherit.get(skuname, "DEFAULT") def GetSkuChain(self, sku): if sku == "DEFAULT": @@ -2024,55 +2020,45 @@ class SkuClass(): return skuorder -def __SkuUsageType(self): - -if self.__SkuIdentifier.upper() == "ALL": +@property +def SkuUsageType(self): +if self._SkuIdentifier.upper() == "ALL": return SkuClass.MULTIPLE if len(self.SkuIdSet) == 1: if self.SkuIdSet[0] == 'DEFAULT': return SkuClass.DEFAULT -else: -return SkuClass.SINGLE -elif len(self.SkuIdSet) == 2: -if 'DEFAULT' in self.SkuIdSet: -return SkuClass.SINGLE -else: -return SkuClass.MULTIPLE -else: -return SkuClass.MULTIPLE +return SkuClass.SINGLE +if len(self.SkuIdSet) == 2 and 'DEFAULT' in self.SkuIdSet: +return SkuClass.SINGLE +return SkuClass.MULTIPLE + def DumpSkuIdArrary(self): - +if self.SkuUsageType == SkuClass.SINGLE: +return "{0x0}" ArrayStrList = [] -if self.SkuUsageType
[edk2] [PATCH v2 6/9] BaseTools: refactor Build Database objects
1) use namedtuple instead of custom class when apropriate 2) rename collections.OrderedDict to OrderedDict since we import it already Cc: Liming Gao Cc: Yonghong Zhu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jaben Carsey --- BaseTools/Source/Python/Workspace/BuildClassObject.py | 32 ++-- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/BaseTools/Source/Python/Workspace/BuildClassObject.py b/BaseTools/Source/Python/Workspace/BuildClassObject.py index 88465c59ea30..e7259b2d3d50 100644 --- a/BaseTools/Source/Python/Workspace/BuildClassObject.py +++ b/BaseTools/Source/Python/Workspace/BuildClassObject.py @@ -11,8 +11,8 @@ # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. # +from collections import OrderedDict, namedtuple from Common.DataType import * -import collections ## PcdClassObject # @@ -165,17 +165,17 @@ class StructurePcd(PcdClassObject): self.StructuredPcdIncludeFile = [] if StructuredPcdIncludeFile is None else StructuredPcdIncludeFile self.PackageDecs = Packages self.DefaultStoreName = [default_store] -self.DefaultValues = collections.OrderedDict() +self.DefaultValues = OrderedDict() self.PcdMode = None -self.SkuOverrideValues = collections.OrderedDict() +self.SkuOverrideValues = OrderedDict() self.FlexibleFieldName = None self.StructName = None self.PcdDefineLineNo = 0 self.PkgPath = "" self.DefaultValueFromDec = "" self.ValueChain = set() -self.PcdFieldValueFromComm = collections.OrderedDict() -self.PcdFieldValueFromFdf = collections.OrderedDict() +self.PcdFieldValueFromComm = OrderedDict() +self.PcdFieldValueFromFdf = OrderedDict() def __repr__(self): return self.TypeName @@ -189,9 +189,9 @@ class StructurePcd(PcdClassObject): self.DefaultValueFromDec = DefaultValue def AddOverrideValue (self, FieldName, Value, SkuName, DefaultStoreName, FileName="", LineNo=0): if SkuName not in self.SkuOverrideValues: -self.SkuOverrideValues[SkuName] = collections.OrderedDict() +self.SkuOverrideValues[SkuName] = OrderedDict() if DefaultStoreName not in self.SkuOverrideValues[SkuName]: -self.SkuOverrideValues[SkuName][DefaultStoreName] = collections.OrderedDict() +self.SkuOverrideValues[SkuName][DefaultStoreName] = OrderedDict() if FieldName in self.SkuOverrideValues[SkuName][DefaultStoreName]: del self.SkuOverrideValues[SkuName][DefaultStoreName][FieldName] self.SkuOverrideValues[SkuName][DefaultStoreName][FieldName] = [Value.strip(), FileName, LineNo] @@ -241,21 +241,7 @@ class StructurePcd(PcdClassObject): self.PcdFieldValueFromComm = PcdObject.PcdFieldValueFromComm if PcdObject.PcdFieldValueFromComm else self.PcdFieldValueFromComm self.PcdFieldValueFromFdf = PcdObject.PcdFieldValueFromFdf if PcdObject.PcdFieldValueFromFdf else self.PcdFieldValueFromFdf -## LibraryClassObject -# -# This Class defines LibraryClassObject used in BuildDatabase -# -# @param object: Inherited from object class -# @param Name:Input value for LibraryClassName, default is None -# @param SupModList: Input value for SupModList, default is [] -# -# @var LibraryClass: To store value for LibraryClass -# @var SupModList:To store value for SupModList -# -class LibraryClassObject(object): -def __init__(self, Name = None, SupModList = []): -self.LibraryClass = Name -self.SupModList = SupModList +LibraryClassObject = namedtuple('LibraryClassObject', ['LibraryClass','SupModList'], verbose=False) ## ModuleBuildClassObject # @@ -323,7 +309,7 @@ class ModuleBuildClassObject(object): self.Binaries= [] self.Sources = [] -self.LibraryClasses = collections.OrderedDict() +self.LibraryClasses = OrderedDict() self.Libraries = [] self.Protocols = [] self.Ppis= [] -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [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 by section # def __GetReferencedFdCapTuple(self, CapObj, RefFdList = [], RefFvList = []): - for CapsuleDataObj in CapObj.CapsuleDataList : if hasattr(CapsuleDataObj, 'FvName') and CapsuleDataObj.FvName is not None and CapsuleDataObj.FvName.
Re: [edk2] [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 da69f6ff5e..ce3bce04f5 100644 > --- a/MdePkg/Include/Library/SynchronizationLib.h > +++ b/MdePkg/Include/Library/SynchronizationLib.h > @@ -144,8 +144,7 @@ ReleaseSpinLock ( > >Performs an atomic increment of the 32-bit unsigned > integer specified by >Value and returns the incremented value. The > increment operation must be > - performed using MP safe mechanisms. The state of the > return value is not > - guaranteed to be MP safe. > + performed using MP safe mechanisms. > >If Value is NULL, then ASSERT(). > > @@ -166,8 +165,7 @@ InterlockedIncrement ( > >Performs an atomic decrement of the 32-bit unsigned > integer specified by >Value and returns the decremented value. The > decrement operation must be > - performed using MP safe mechanisms. The state of the > return value is not > - guaranteed to be MP safe. > + performed using MP safe mechanisms. > >If Value is NULL, then ASSERT(). > > diff --git > a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat > ionLib.inf > b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat > ionLib.inf > index 0be1d4331f..b971cd138d 100755 > --- > a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat > ionLib.inf > +++ > b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizat > ionLib.inf > @@ -34,9 +34,9 @@ [Sources.IA32] >Ia32/InterlockedCompareExchange64.c | MSFT >Ia32/InterlockedCompareExchange32.c | MSFT >Ia32/InterlockedCompareExchange16.c | MSFT > - Ia32/InterlockedDecrement.c | MSFT > - Ia32/InterlockedIncrement.c | MSFT > - SynchronizationMsc.c | MSFT > + Ia32/InterlockedDecrement.nasm | MSFT > + Ia32/InterlockedIncrement.nasm | MSFT > + SynchronizationMsc.c | MSFT > >Ia32/InterlockedCompareExchange64.nasm| INTEL >Ia32/InterlockedCompareExchange32.nasm| INTEL > @@ -54,17 +54,15 @@ [Sources.X64] >X64/InterlockedCompareExchange64.c | MSFT >X64/InterlockedCompareExchange32.c | MSFT >X64/InterlockedCompareExchange16.c | MSFT > + X64/InterlockedDecrement.nasm | MSFT > + X64/InterlockedIncrement.nasm | MSFT > + SynchronizationMsc.c | MSFT > >X64/InterlockedCompareExchange64.nasm| INTEL >X64/InterlockedCompareExchange32.nasm| INTEL >X64/InterlockedCompareExchange16.nasm| INTEL > - > - X64/InterlockedDecrement.c | MSFT > - X64/InterlockedIncrement.c | MSFT > - SynchronizationMsc.c | MSFT > - > - X64/InterlockedDecrement.nasm| INTEL > -
Re: [edk2] [PATCH] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value
You are right. My mistake. > -Original Message- > From: Ni, Ruiyu > Sent: Monday, September 10, 2018 10:59 PM > To: Yao, Jiewen > Cc: edk2-devel@lists.01.org; Gao, Liming ; Kinney, > Michael D > Subject: Re: [PATCH] MdePkg/SynchronizationLib: fix > Interlocked[De|In]crement return value > > I didn’t find such instruction in SDM. > > 发自我的 iPhone > > > 在 2018年9月10日,下午7:37,Yao, Jiewen > 写道: > > > > Hi > > Can we use XSUB for decrement? > > > > Thank you > > Yao Jiewen > > > >> -Original Message- > >> From: Ni, Ruiyu > >> Sent: Monday, September 10, 2018 6:06 PM > >> 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/InterlockedDecrement.c > >> delete mode 100644 > >> MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c > >> delete mode 100644 > >> MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c > >> delete mode 100644 > >> MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c > >> > >> diff --git a/MdePkg/Include/Library/SynchronizationLib.h > >> b/MdePkg/Include/Library/SynchronizationLib.h > >> index da69f6ff5e..ce3bce04f5 100644 > >> --- a/MdePkg/Include/Library/SynchronizationLib.h > >> +++ b/MdePkg/Include/Library/SynchronizationLib.h > >> @@ -144,8 +144,7 @@ ReleaseSpinLock ( > >> > >> Performs an atomic increment of the 32-bit unsigned integer specified > by > >> Value and returns the incremented value. The increment operation > must > >> be > >> - performed using MP safe mechanisms. The state of the return value is > >> not > >> - guaranteed to be MP safe. > >> + performed using MP safe mechanisms. > >> > >> If Value is NULL, then ASSERT(). > >> > >> @@ -166,8 +165,7 @@ InterlockedIncrement ( > >> > >> Performs an atomic decrement of the 32-bit unsigned integer specified > >> by > >> Value and returns the decremented value. The decrement operation > must > >> be > >> - performed using MP safe mechanisms. The state of the return value is > >> not > >> - guaranteed to be MP safe. > >> + performed using MP safe mechanisms. > >> > >> If Value is NULL, then ASSERT(). > >> > >> diff --git > >> a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf > >> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf > >> index 0be1d4331f..b971cd138d 100755 > >> --- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf > >> +++ > b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf > >> @@ -34,9 +34,9 @@ [Sources.IA32] > >> Ia32/InterlockedCompareExchange64.c | MSFT > >> Ia32/InterlockedCompareExchange32.c | MSFT > >> Ia32/InterlockedCompareExchange16.c | MSFT > >> - Ia32/InterlockedDecrement.c | MSFT > >> - Ia32/InterlockedIncrement.c | MSFT > >> - SynchronizationMsc.c | MSFT > >> + Ia32/InterlockedDecrement.nasm | MSFT > >> + Ia32/InterlockedIncrement.nasm | MSFT > >> + SynchronizationMsc.c | MSFT > >> > >> Ia32/InterlockedCompareExchange64.nasm| INTEL > >> Ia32/InterlockedCompareExchange32.nasm| INTEL > >> @@ -54,17 +54,15 @@ [Sources.X64] > >> X64/InterlockedCompareExchange64.c | MSFT > >> X
Re: [edk2] Performance enabling of Event handler
Hi Team, Thank it’s got worked on dp -t 0. > On 07-Sep-2018, at 9:27 PM, prabin ca wrote: > > Hi > > Yes it is included in same module (both event handler and function handler), > and I’m not perf_start and perf_end only two times (one is by event handler > and one is by normal function handler). > > And I’m trying to print result using DP.efi, it shows entry for normal > function. > >> On 07-Sep-2018, at 8:46 AM, Bi, Dandan wrote: >> >> Hi Prabin, >> >> The Performance logging for the normal functions handlers and event handlers >> should be the same. >> Are the normal function calls and the event handler function calls you >> tested in the same module? >> If not, please double check to make sure the performance library instance >> used correctly for each module. >> >> >> Thanks, >> Dandan >> >> -Original Message- >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of >> prabin ca >> Sent: Friday, September 7, 2018 10:30 AM >> To: Laszlo Ersek >> Cc: Bi, Dandan ; edk2-devel@lists.01.org; >> af...@apple.com >> Subject: Re: [edk2] Performance enabling of Event handler >> >> Hi, >> PerformancePkg is not working with event handlers, but it’s working with >> normal functions handlers. >> On 06-Sep-2018, at 3:28 PM, Laszlo Ersek wrote: On 09/06/18 08:10, prabin ca wrote: Hi Team, I’m used edk2 PerformancePkg for profiling cpu execution time taken by a event handler. Event is created successfully and event handler is also called successfully, but I can capture the performance of this event handler with PerformancePkg (by using perf_start and perf_end check points). This PerformancePkg is working fine with normal function calls. >>> >>> Do you mean "can not", instead of "can"? (Sorry, I don't understand.) >>> Please help me to enable PerformancePkg action on event handler also. >>> >>> Hmmm, even with the suggested typo correction, I wouldn't know what to >>> suggest. Sorry! >>> >>> Laszlo >> ___ >> 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
I didn’t find such instruction in SDM. 发自我的 iPhone > 在 2018年9月10日,下午7:37,Yao, Jiewen 写道: > > Hi > Can we use XSUB for decrement? > > Thank you > Yao Jiewen > >> -Original Message- >> From: Ni, Ruiyu >> Sent: Monday, September 10, 2018 6:06 PM >> 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/InterlockedDecrement.c >> delete mode 100644 >> MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c >> delete mode 100644 >> MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c >> delete mode 100644 >> MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c >> >> diff --git a/MdePkg/Include/Library/SynchronizationLib.h >> b/MdePkg/Include/Library/SynchronizationLib.h >> index da69f6ff5e..ce3bce04f5 100644 >> --- a/MdePkg/Include/Library/SynchronizationLib.h >> +++ b/MdePkg/Include/Library/SynchronizationLib.h >> @@ -144,8 +144,7 @@ ReleaseSpinLock ( >> >> Performs an atomic increment of the 32-bit unsigned integer specified by >> Value and returns the incremented value. The increment operation must >> be >> - performed using MP safe mechanisms. The state of the return value is >> not >> - guaranteed to be MP safe. >> + performed using MP safe mechanisms. >> >> If Value is NULL, then ASSERT(). >> >> @@ -166,8 +165,7 @@ InterlockedIncrement ( >> >> Performs an atomic decrement of the 32-bit unsigned integer specified >> by >> Value and returns the decremented value. The decrement operation must >> be >> - performed using MP safe mechanisms. The state of the return value is >> not >> - guaranteed to be MP safe. >> + performed using MP safe mechanisms. >> >> If Value is NULL, then ASSERT(). >> >> diff --git >> a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf >> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf >> index 0be1d4331f..b971cd138d 100755 >> --- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf >> +++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf >> @@ -34,9 +34,9 @@ [Sources.IA32] >> Ia32/InterlockedCompareExchange64.c | MSFT >> Ia32/InterlockedCompareExchange32.c | MSFT >> Ia32/InterlockedCompareExchange16.c | MSFT >> - Ia32/InterlockedDecrement.c | MSFT >> - Ia32/InterlockedIncrement.c | MSFT >> - SynchronizationMsc.c | MSFT >> + Ia32/InterlockedDecrement.nasm | MSFT >> + Ia32/InterlockedIncrement.nasm | MSFT >> + SynchronizationMsc.c | MSFT >> >> Ia32/InterlockedCompareExchange64.nasm| INTEL >> Ia32/InterlockedCompareExchange32.nasm| INTEL >> @@ -54,17 +54,15 @@ [Sources.X64] >> X64/InterlockedCompareExchange64.c | MSFT >> X64/InterlockedCompareExchange32.c | MSFT >> X64/InterlockedCompareExchange16.c | MSFT >> + X64/InterlockedDecrement.nasm | MSFT >> + X64/InterlockedIncrement.nasm | MSFT >> + SynchronizationMsc.c | MSFT >> >> X64/InterlockedCompareExchange64.nasm| INTEL >> X64/InterlockedCompareExchange32.nasm| INTEL >> X64/InterlockedCompareExchange16.nasm| INTEL >> - >> - X64/InterlockedDecrement.c | MSFT >> - X64/InterlockedIncrement.c | MSFT >> - SynchronizationMsc.c | MSFT >> - >> - X64/InterlockedDecrement.nasm| INTEL >> -
[edk2] [Patch][edk2-platforms/devel-IntelAtomProcessorE3900] Logo Update.
(1) Update EDK2 TianoCore logo to the latest. (2) Extend logo showing period to the late stage of BDS. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: David Wei CC: Mike Wu CC: Mang Guo --- .../AuroraGlacier/BoardInitPostMem/BoardInit.c | 2 +- .../BoardInitPostMem/BoardInitPostMem.inf | 2 +- .../BensonGlacier/BoardInitPostMem/BoardInit.c | 2 +- .../BoardInitPostMem/BoardInitPostMem.inf | 2 +- .../Board/LeafHill/BoardInitPostMem/BoardInit.c| 2 +- .../LeafHill/BoardInitPostMem/BoardInitPostMem.inf | 2 +- .../MinnowBoard3/BoardInitPostMem/BoardInit.c | 2 +- .../BoardInitPostMem/BoardInitPostMem.inf | 2 +- .../BoardInitPostMem/BoardInit.c | 2 +- .../BoardInitPostMem/BoardInitPostMem.inf | 2 +- .../Board/UP2/BoardInitPostMem/BoardInit.c | 2 +- .../UP2/BoardInitPostMem/BoardInitPostMem.inf | 2 +- .../Common/Binaries/Logo/Tianocore.bmp | Bin 0 -> 11958 bytes .../Common/Library/DxeLogoLib/DxeLogoLib.inf | 1 - .../Common/Library/DxeLogoLib/Logo.c | 299 - .../PeiFspPolicyInitLib/PeiFspPolicyInitLib.inf| 1 - .../PlatformBootManagerLib/PlatformBootManager.c | 10 +- .../PlatformBootManagerLib.inf | 4 +- Platform/BroxtonPlatformPkg/PlatformPkg.dec| 3 +- Platform/BroxtonPlatformPkg/PlatformPkg.fdf| 13 +- 20 files changed, 83 insertions(+), 272 deletions(-) create mode 100644 Platform/BroxtonPlatformPkg/Common/Binaries/Logo/Tianocore.bmp diff --git a/Platform/BroxtonPlatformPkg/Board/AuroraGlacier/BoardInitPostMem/BoardInit.c b/Platform/BroxtonPlatformPkg/Board/AuroraGlacier/BoardInitPostMem/BoardInit.c index 302edb8301..ce98086895 100644 --- a/Platform/BroxtonPlatformPkg/Board/AuroraGlacier/BoardInitPostMem/BoardInit.c +++ b/Platform/BroxtonPlatformPkg/Board/AuroraGlacier/BoardInitPostMem/BoardInit.c @@ -110,7 +110,7 @@ AuroraGlacierPostMemInitCallback ( // BufferSize = sizeof (EFI_GUID); PcdSetPtr(PcdBoardVbtFileGuid, &BufferSize, (UINT8 *)&gPeiAuroraGlacierVbtGuid); - PcdSetPtr(PcdOemLogoFileGuid, &BufferSize, (UINT8 *)&gPeiLogoGuid); + PcdSetPtr(PcdOemLogoFileGuid, &BufferSize, (UINT8 *)PcdGetPtr(PcdTianoCoreLogoFileGuid)); // // Set PcdSueCreek diff --git a/Platform/BroxtonPlatformPkg/Board/AuroraGlacier/BoardInitPostMem/BoardInitPostMem.inf b/Platform/BroxtonPlatformPkg/Board/AuroraGlacier/BoardInitPostMem/BoardInitPostMem.inf index 8fe5ea7e95..c619332cb1 100644 --- a/Platform/BroxtonPlatformPkg/Board/AuroraGlacier/BoardInitPostMem/BoardInitPostMem.inf +++ b/Platform/BroxtonPlatformPkg/Board/AuroraGlacier/BoardInitPostMem/BoardInitPostMem.inf @@ -64,6 +64,7 @@ gPlatformModuleTokenSpaceGuid.PcdResetType gPlatformModuleTokenSpaceGuid.PcdBoardVbtFileGuid gPlatformModuleTokenSpaceGuid.PcdOemLogoFileGuid + gPlatformModuleTokenSpaceGuid.PcdTianoCoreLogoFileGuid gPlatformModuleTokenSpaceGuid.PcdSueCreek gPlatformModuleTokenSpaceGuid.PcdMaxPkgCState gPlatformModuleTokenSpaceGuid.PcdTi3100AudioCodecEnable @@ -85,7 +86,6 @@ gEfiTpmDeviceInstanceTpm20DtpmGuid gTpmDeviceInstanceTpm20PttPtpGuid gPeiAuroraGlacierVbtGuid - gPeiLogoGuid [Ppis] gBoardPostMemInitStartGuid diff --git a/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInit.c b/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInit.c index 60d2324d95..856d7739b0 100644 --- a/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInit.c +++ b/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInit.c @@ -110,7 +110,7 @@ BensonGlacierPostMemInitCallback ( // BufferSize = sizeof (EFI_GUID); PcdSetPtr(PcdBoardVbtFileGuid, &BufferSize, (UINT8 *)&gPeiBensonGlacierVbtGuid); - PcdSetPtr(PcdOemLogoFileGuid, &BufferSize, (UINT8 *)&gPeiLogoGuid); + PcdSetPtr(PcdOemLogoFileGuid, &BufferSize, (UINT8 *)PcdGetPtr(PcdTianoCoreLogoFileGuid)); // // Set PcdSueCreek diff --git a/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInitPostMem.inf b/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInitPostMem.inf index 782bf0d2bd..01d7f27898 100644 --- a/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInitPostMem.inf +++ b/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInitPostMem.inf @@ -64,6 +64,7 @@ gPlatformModuleTokenSpaceGuid.PcdResetType gPlatformModuleTokenSpaceGuid.PcdBoardVbtFileGuid gPlatformModuleTokenSpaceGuid.PcdOemLogoFileGuid + gPlatformModuleTokenSpaceGuid.PcdTianoCoreLogoFileGuid gPlatformModuleTokenSpaceGuid.PcdSueCreek gPlatformModuleTokenSpaceGuid.PcdMaxPkgCState gPlatformModuleTokenSpaceGuid.PcdTi3100AudioCodecEnable @@ -85,7 +86,6 @@ gEfiTpmDeviceInstanceTpm20DtpmGuid gTpmDeviceInstanceTpm20PttPtpGuid gPeiBensonGlacierVbtGuid - gPei
Re: [edk2] [PATCH] MdePkg/SynchronizationLib: fix Interlocked[De|In]crement return value
Hi Can we use XSUB for decrement? Thank you Yao Jiewen > -Original Message- > From: Ni, Ruiyu > Sent: Monday, September 10, 2018 6:06 PM > 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/InterlockedDecrement.c > delete mode 100644 > MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c > delete mode 100644 > MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c > delete mode 100644 > MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c > > diff --git a/MdePkg/Include/Library/SynchronizationLib.h > b/MdePkg/Include/Library/SynchronizationLib.h > index da69f6ff5e..ce3bce04f5 100644 > --- a/MdePkg/Include/Library/SynchronizationLib.h > +++ b/MdePkg/Include/Library/SynchronizationLib.h > @@ -144,8 +144,7 @@ ReleaseSpinLock ( > >Performs an atomic increment of the 32-bit unsigned integer specified by >Value and returns the incremented value. The increment operation must > be > - performed using MP safe mechanisms. The state of the return value is > not > - guaranteed to be MP safe. > + performed using MP safe mechanisms. > >If Value is NULL, then ASSERT(). > > @@ -166,8 +165,7 @@ InterlockedIncrement ( > >Performs an atomic decrement of the 32-bit unsigned integer specified > by >Value and returns the decremented value. The decrement operation must > be > - performed using MP safe mechanisms. The state of the return value is > not > - guaranteed to be MP safe. > + performed using MP safe mechanisms. > >If Value is NULL, then ASSERT(). > > diff --git > a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf > b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf > index 0be1d4331f..b971cd138d 100755 > --- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf > +++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf > @@ -34,9 +34,9 @@ [Sources.IA32] >Ia32/InterlockedCompareExchange64.c | MSFT >Ia32/InterlockedCompareExchange32.c | MSFT >Ia32/InterlockedCompareExchange16.c | MSFT > - Ia32/InterlockedDecrement.c | MSFT > - Ia32/InterlockedIncrement.c | MSFT > - SynchronizationMsc.c | MSFT > + Ia32/InterlockedDecrement.nasm | MSFT > + Ia32/InterlockedIncrement.nasm | MSFT > + SynchronizationMsc.c | MSFT > >Ia32/InterlockedCompareExchange64.nasm| INTEL >Ia32/InterlockedCompareExchange32.nasm| INTEL > @@ -54,17 +54,15 @@ [Sources.X64] >X64/InterlockedCompareExchange64.c | MSFT >X64/InterlockedCompareExchange32.c | MSFT >X64/InterlockedCompareExchange16.c | MSFT > + X64/InterlockedDecrement.nasm | MSFT > + X64/InterlockedIncrement.nasm | MSFT > + SynchronizationMsc.c | MSFT > >X64/InterlockedCompareExchange64.nasm| INTEL >X64/InterlockedCompareExchange32.nasm| INTEL >X64/InterlockedCompareExchange16.nasm| INTEL > - > - X64/InterlockedDecrement.c | MSFT > - X64/InterlockedIncrement.c | MSFT > - SynchronizationMsc.c | MSFT > - > - X64/InterlockedDecrement.nasm| INTEL > - X64/InterlockedIncrement.nasm| INTEL > + X64/InterlockedDecrement.nasm | INTEL > + X64/InterlockedIncrement.nasm | INTEL >Synchronization.c | INTEL > >Ia32/InternalGetSpinLockP
[edk2] [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/InterlockedDecrement.c delete mode 100644 MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedIncrement.c delete mode 100644 MdePkg/Library/BaseSynchronizationLib/X64/InterlockedDecrement.c delete mode 100644 MdePkg/Library/BaseSynchronizationLib/X64/InterlockedIncrement.c diff --git a/MdePkg/Include/Library/SynchronizationLib.h b/MdePkg/Include/Library/SynchronizationLib.h index da69f6ff5e..ce3bce04f5 100644 --- a/MdePkg/Include/Library/SynchronizationLib.h +++ b/MdePkg/Include/Library/SynchronizationLib.h @@ -144,8 +144,7 @@ ReleaseSpinLock ( Performs an atomic increment of the 32-bit unsigned integer specified by Value and returns the incremented value. The increment operation must be - performed using MP safe mechanisms. The state of the return value is not - guaranteed to be MP safe. + performed using MP safe mechanisms. If Value is NULL, then ASSERT(). @@ -166,8 +165,7 @@ InterlockedIncrement ( Performs an atomic decrement of the 32-bit unsigned integer specified by Value and returns the decremented value. The decrement operation must be - performed using MP safe mechanisms. The state of the return value is not - guaranteed to be MP safe. + performed using MP safe mechanisms. If Value is NULL, then ASSERT(). diff --git a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf index 0be1d4331f..b971cd138d 100755 --- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf +++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf @@ -34,9 +34,9 @@ [Sources.IA32] Ia32/InterlockedCompareExchange64.c | MSFT Ia32/InterlockedCompareExchange32.c | MSFT Ia32/InterlockedCompareExchange16.c | MSFT - Ia32/InterlockedDecrement.c | MSFT - Ia32/InterlockedIncrement.c | MSFT - SynchronizationMsc.c | MSFT + Ia32/InterlockedDecrement.nasm | MSFT + Ia32/InterlockedIncrement.nasm | MSFT + SynchronizationMsc.c | MSFT Ia32/InterlockedCompareExchange64.nasm| INTEL Ia32/InterlockedCompareExchange32.nasm| INTEL @@ -54,17 +54,15 @@ [Sources.X64] X64/InterlockedCompareExchange64.c | MSFT X64/InterlockedCompareExchange32.c | MSFT X64/InterlockedCompareExchange16.c | MSFT + X64/InterlockedDecrement.nasm | MSFT + X64/InterlockedIncrement.nasm | MSFT + SynchronizationMsc.c | MSFT X64/InterlockedCompareExchange64.nasm| INTEL X64/InterlockedCompareExchange32.nasm| INTEL X64/InterlockedCompareExchange16.nasm| INTEL - - X64/InterlockedDecrement.c | MSFT - X64/InterlockedIncrement.c | MSFT - SynchronizationMsc.c | MSFT - - X64/InterlockedDecrement.nasm| INTEL - X64/InterlockedIncrement.nasm| INTEL + X64/InterlockedDecrement.nasm | INTEL + X64/InterlockedIncrement.nasm | INTEL Synchronization.c | INTEL Ia32/InternalGetSpinLockProperties.c | GCC diff --git a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternals.h b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternals.h index 37edd7c188..8c363a8585 100644 --- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternals.h +++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLibInternals.h @@ -27,8 +27,7 @@ Performs an atomic increment of the 32-bit unsigned integer specified by Value and returns the incremented value. The increment operation must b
[edk2] [PATCH] MdeModulePkg/ConPlatform: Support short-form USB device path
Today's implementation does an exact device path match to check whether the device path of a console is in ConIn/ConOut/ErrOut. But that doesn't work for the USB keyboard. Because when a platform have multiple USB port, ConIn needs to carry all device paths corresponding to each port. Even worse, today's BDS core logic removes the device path from ConIn/ConOut/ErrOut when the connection to that device path fails. So if user switches the USB keyboard from one port to another across boot, the USB keyboard doesn't work in the second boot. ConPlatform driver solved this problem by adding the IsHotPlugDevice() function. So that for USB keyboard, ConPlatform doesn't care whether its device path is in ConIn or not. But the rule is too loose, and now causes platform BDS cannot control whether to enable USB keyboard as an active console. The patch changes ConPlatform to support USB short-form device path when checking whether the device path of a console is in ConIn/ConOut/ErrOut. The logic to always accept USB/PCCARD device as active console is removed. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Cc: Hao A Wu Cc: Michael D Kinney Cc: Star Zeng --- .../Universal/Console/ConPlatformDxe/ConPlatform.c | 513 ++--- .../Universal/Console/ConPlatformDxe/ConPlatform.h | 24 +- .../Console/ConPlatformDxe/ConPlatformDxe.inf | 1 + 3 files changed, 339 insertions(+), 199 deletions(-) diff --git a/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c b/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c index 5fa7facfca..27df8a4b56 100644 --- a/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c +++ b/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c @@ -202,8 +202,7 @@ ConPlatformDriverBindingSupported ( Start this driver on ControllerHandle by opening Simple Text Input Protocol, reading Device Path, and installing Console In Devcice GUID on ControllerHandle. - If this devcie is not one hot-plug devce, append its device path into the - console environment variables ConInDev. + Append its device path into the console environment variables ConInDev. @param This Protocol instance pointer. @param ControllerHandle Handle of device to bind driver to @@ -270,58 +269,32 @@ ConPlatformTextInDriverBindingStart ( } // - // Check the device path, if it is a hot plug device, - // do not put the device path into ConInDev, and install - // gEfiConsoleInDeviceGuid to the device handle directly. - // The policy is, make hot plug device plug in and play immediately. + // Append the device path to the ConInDev environment variable // - if (IsHotPlugDevice (DevicePath)) { + ConPlatformUpdateDeviceVariable ( +L"ConInDev", +DevicePath, +Append +); + + // + // If the device path is an instance in the ConIn environment variable, + // then install EfiConsoleInDeviceGuid onto ControllerHandle + // + if (IsInConInVariable) { gBS->InstallMultipleProtocolInterfaces ( &ControllerHandle, &gEfiConsoleInDeviceGuid, NULL, NULL ); -// -// Append the device path to ConInDev only if it is in ConIn variable. -// -if (IsInConInVariable) { - ConPlatformUpdateDeviceVariable ( -L"ConInDev", -DevicePath, -Append -); -} } else { -// -// If it is not a hot-plug device, append the device path to the -// ConInDev environment variable -// -ConPlatformUpdateDeviceVariable ( - L"ConInDev", - DevicePath, - Append - ); - -// -// If the device path is an instance in the ConIn environment variable, -// then install EfiConsoleInDeviceGuid onto ControllerHandle -// -if (IsInConInVariable) { - gBS->InstallMultipleProtocolInterfaces ( - &ControllerHandle, - &gEfiConsoleInDeviceGuid, - NULL, - NULL - ); -} else { - gBS->CloseProtocol ( - ControllerHandle, - &gEfiSimpleTextInProtocolGuid, - This->DriverBindingHandle, - ControllerHandle - ); -} +gBS->CloseProtocol ( + ControllerHandle, + &gEfiSimpleTextInProtocolGuid, + This->DriverBindingHandle, + ControllerHandle + ); } return EFI_SUCCESS; @@ -334,8 +307,7 @@ ConPlatformTextInDriverBindingStart ( reading Device Path, and installing Console Out Devcic GUID, Standard Error Device GUID on ControllerHandle. - If this devcie is not one hot-plug devce, append its device path into the - console environment variables ConOutDev, ErrOutDev. + Append its device path into the console environment variables ConOutDev, ErrOutDev. @param This Protocol instance pointer. @param ControllerHandle Handle of device to bind driver to @@ -416,95 +388,59 @@
[edk2] [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Remove unnecessary jmp _SmiHandler
This change is wrong introduced by e21e355e2ca7fefb15b4df7078f995d3fb9c2b89 It is not required. So, revert it. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Liming Gao Cc: Jiewen Yao --- UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm index 315d0f8..7b1b3ca 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm @@ -173,9 +173,8 @@ SmiHandlerIdtrAbsAddr: mov gs, eax mov ax, [rbx + DSC_SS] mov ss, eax -mov rax, strict qword 0 ; mov rax, _SmiHandler -_SmiHandlerAbsAddr: -jmp rax + +; jmp _SmiHandler ; instruction is not needed _SmiHandler: mov rbx, [rsp + 0x8] ; rcx <- CpuIndex @@ -229,8 +228,4 @@ ASM_PFX(PiSmmCpuSmiEntryFixupAddress): learax, [ASM_PFX(gSmiHandlerIdtr)] learcx, [SmiHandlerIdtrAbsAddr] movqword [rcx - 8], rax - -learax, [_SmiHandler] -learcx, [_SmiHandlerAbsAddr] -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 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 (&ConsoleIn->EfiKeyQueueForNotify, &KeyData); gBS->SignalEvent (ConsoleIn->KeyNotifyProcessEvent); + break; } } PushEfikeyBufTail (&ConsoleIn->EfiKeyQueue, &KeyData); } 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 (&UsbKeyboardDevice->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
[edk2] [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 (&VirtualKeyboardPrivate->QueueForNotify, &KeyData); gBS->SignalEvent (VirtualKeyboardPrivate->KeyNotifyProcessEvent); + break; } } Enqueue (&VirtualKeyboardPrivate->Queue, &KeyData); -- 2.14.3.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [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 (&ConsoleIn->EfiKeyQueueForNotify, &KeyData); gBS->SignalEvent (ConsoleIn->KeyNotifyProcessEvent); + break; } } PushEfikeyBufTail (&ConsoleIn->EfiKeyQueue, &KeyData); } 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 (&BiosKeyboardPrivate->QueueForNotify, &KeyData); gBS->SignalEvent (BiosKeyboardPrivate->KeyNotifyProcessEvent); + break; } } Enqueue (&BiosKeyboardPrivate->Queue, &KeyData); -- 2.14.3.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel