Re: [edk2] [Patch v3 2/3] UefiCpuPkg/MpInitLib: Remove StartCount and volatile definition.
Reviewed-by: Ruiyu Ni
Thanks/Ray
> -Original Message-
> From: edk2-devel On Behalf Of Eric Dong
> Sent: Wednesday, July 25, 2018 3:50 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Laszlo Ersek
> Subject: [edk2] [Patch v3 2/3] UefiCpuPkg/MpInitLib: Remove StartCount and
> volatile definition.
>
> The StartCount is duplicated with RunningCount, replace it with RunningCount.
> Also the volatile for RunningCount is not needed.
>
> Cc: Laszlo Ersek
> Cc: Ruiyu Ni
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong
> ---
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 11 +--
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +--
> 2 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index ff09a0e9e7..0e57cc86bf 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -1424,7 +1424,7 @@ CheckAllAPs (
> // value of state after setting the it to CpuStateIdle, so BSP can
> safely make
> use of its value.
> //
> if (GetApState(CpuData) == CpuStateIdle) {
> - CpuMpData->RunningCount ++;
> + CpuMpData->RunningCount --;
>CpuMpData->CpuData[ProcessorNumber].Waiting = FALSE;
>
>//
> @@ -1449,7 +1449,7 @@ CheckAllAPs (
>//
>// If all APs finish, return EFI_SUCCESS.
>//
> - if (CpuMpData->RunningCount == CpuMpData->StartCount) {
> + if (CpuMpData->RunningCount == 0) {
> return EFI_SUCCESS;
>}
>
> @@ -1466,7 +1466,7 @@ CheckAllAPs (
> //
> if (CpuMpData->FailedCpuList != NULL) {
>*CpuMpData->FailedCpuList =
> - AllocatePool ((CpuMpData->StartCount - CpuMpData->FinishedCount + 1)
> * sizeof (UINTN));
> + AllocatePool ((CpuMpData->RunningCount + 1) * sizeof (UINTN));
>ASSERT (*CpuMpData->FailedCpuList != NULL);
> }
> ListIndex = 0;
> @@ -2212,7 +2212,7 @@ StartupAllAPsWorker (
> return EFI_NOT_STARTED;
>}
>
> - CpuMpData->StartCount = 0;
> + CpuMpData->RunningCount = 0;
>for (ProcessorNumber = 0; ProcessorNumber < ProcessorCount;
> ProcessorNumber++) {
> CpuData = >CpuData[ProcessorNumber];
> CpuData->Waiting = FALSE;
> @@ -,7 +,7 @@ StartupAllAPsWorker (
> // Mark this processor as responsible for current calling.
> //
> CpuData->Waiting = TRUE;
> -CpuMpData->StartCount++;
> +CpuMpData->RunningCount++;
>}
> }
>}
> @@ -2231,7 +2231,6 @@ StartupAllAPsWorker (
>CpuMpData->ProcArguments = ProcedureArgument;
>CpuMpData->SingleThread = SingleThread;
>CpuMpData->FinishedCount = 0;
> - CpuMpData->RunningCount = 0;
>CpuMpData->FailedCpuList = FailedCpuList;
>CpuMpData->ExpectedTime = CalculateTimeout (
> TimeoutInMicroseconds, diff --git
> a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 962bce685d..5002b7e9c0 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -211,9 +211,8 @@ struct _CPU_MP_DATA {
>UINTN BackupBuffer;
>UINTN BackupBufferSize;
>
> - volatile UINT32StartCount;
>volatile UINT32FinishedCount;
> - volatile UINT32RunningCount;
> + UINT32 RunningCount;
>BOOLEANSingleThread;
>EFI_AP_PROCEDURE Procedure;
>VOID *ProcArguments;
> --
> 2.15.0.windows.1
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch v3 1/3] UefiCpuPkg/MpInitLib: Remove redundant CpuStateFinished State.
Eric,
Please also include the state machine in comments for ENUM CPU_STATE definition.
Reviewed-by: Ruiyu Ni
Thanks/Ray
> -Original Message-
> From: Dong, Eric
> Sent: Wednesday, July 25, 2018 7:15 PM
> To: Laszlo Ersek ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu
> Subject: RE: [edk2] [Patch v3 1/3] UefiCpuPkg/MpInitLib: Remove redundant
> CpuStateFinished State.
>
> Hi Laszlo,
>
>
> > -Original Message-
> > From: Laszlo Ersek [mailto:ler...@redhat.com]
> > Sent: Wednesday, July 25, 2018 6:55 PM
> > To: Dong, Eric ; edk2-devel@lists.01.org
> > Cc: Ni, Ruiyu
> > Subject: Re: [edk2] [Patch v3 1/3] UefiCpuPkg/MpInitLib: Remove
> > redundant CpuStateFinished State.
> >
> > Hi Eric,
> >
> > On 07/25/18 09:50, Eric Dong wrote:
> > > Current CPU state definition include CpuStateIdle and CpuStateFinished.
> > > After investigation, current code can use CpuStateIdle to replace
> > > the CpuStateFinished. It will reduce the state number and easy for
> maintenance.
> > >
> > > Cc: Laszlo Ersek
> > > Cc: Ruiyu Ni
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Eric Dong
> > > ---
> > > UefiCpuPkg/Library/MpInitLib/MpLib.c | 18 --
> > > UefiCpuPkg/Library/MpInitLib/MpLib.h | 1 -
> > > 2 files changed, 8 insertions(+), 11 deletions(-)
> >
> > After looking over this patch, it seems that you are preserving the
> > CpuStateReady enum constant, relative to:
> >
> >
> > 20180628112920.5296-1-eric.dong@intel.com">http://mid.mail-archive.com/20180628112920.5296-1-eric.dong@intel.com
> >
> > However, based on your analysis in
> >
> > http://mid.mail-
> > archive.com/ED077930C258884BBCB450DB737E66224AC5A453@shsmsx102.
> > ccr.corp.intel.com
> >
> > isn't it still possible to run into the exact same issue? (Namely, BSP
> > thinks the AP has gone through Idle -> Busy -> Idle, but the AP has
> > never actually left
> > Idle?)
> >
> > Hm, wait, is it the case that the BSP first sets Ready, and so if the
> > check for an AP returns Idle, it implies the AP must have gone through:
> >
> > Idle > Ready > Busy > Idle
> >
> > ?
>
> Correct! The Ready state is begin state and the Idle is the finish state.
>
> >
> > If this is correct, can you please include the following in the commit
> > message:
> >
> > > Before this patch, the state transitions for an AP are:
> > >
> > > Idle > Ready > Busy > Finished > Idle
> > >[BSP] [AP] [AP] [BSP]
> > >
> > > After the patch, the state transitions for an AP are:
> > >
> > > Idle > Ready > Busy > Idle
> > >[BSP] [AP] [AP]
> >
> > Do you agree?
>
> Good suggestion, I will include this info in the commit message.
>
> >
> > I have another question:
> >
> > On 07/25/18 09:50, Eric Dong wrote:
> > > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > > b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > > index c82b985943..ff09a0e9e7 100644
> > > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > > @@ -696,7 +696,7 @@ ApWakeupFunction (
> > > }
> > >}
> > > }
> > > -SetApState (>CpuData[ProcessorNumber],
> > CpuStateFinished);
> > > +SetApState (>CpuData[ProcessorNumber],
> > > + CpuStateIdle);
> > >}
> > > }
> > >
> > > @@ -1352,18 +1352,17 @@ CheckThisAP (
> > >CpuData = >CpuData[ProcessorNumber];
> > >
> > >//
> > > - // Check the CPU state of AP. If it is CpuStateFinished, then
> > > the AP has
> > finished its task.
> > > + // Check the CPU state of AP. If it is CpuStateIdle, then the AP
> > > + has
> > finished its task.
> > >// Only BSP and corresponding AP access this unit of CPU Data.
> > > This means the AP will not modify the
> > > - // value of state after setting the it to CpuStateFinished, so
> > > BSP can safely
> > make use of its value.
> > > + // value of state after setting the it to CpuStateIdle, so BSP
> > > + can safely
> > make use of its value.
> > >//
> > >//
> > >// If the AP finishes for StartupThisAP(), return EFI_SUCCESS.
> > >//
> > > - if (GetApState(CpuData) == CpuStateFinished) {
> > > + if (GetApState(CpuData) == CpuStateIdle) {
> > > if (CpuData->Finished != NULL) {
> > >*(CpuData->Finished) = TRUE;
> > > }
> > > -SetApState (CpuData, CpuStateIdle);
> > > return EFI_SUCCESS;
> > >} else {
> > > //
> > > @@ -1420,14 +1419,13 @@ CheckAllAPs (
> > >
> > > CpuData = >CpuData[ProcessorNumber];
> > > //
> > > -// Check the CPU state of AP. If it is CpuStateFinished, then the AP
> > > has
> > finished its task.
> > > +// Check the CPU state of AP. If it is CpuStateIdle, then the
> > > + AP has
> > finished its task.
> > > // Only BSP and corresponding AP access this unit of CPU Data.
> > > This
> > means the AP will not modify the
> > > -// value of state after setting the it to
[edk2] [PATCH 5/5] MdeModulePkg CapsuleApp: Prompt info for -C option
Cc: Michael D Kinney
Cc: Jiewen Yao
Cc: Yonghong Zhu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng
---
MdeModulePkg/Application/CapsuleApp/CapsuleApp.c | 10 ++
MdeModulePkg/Application/CapsuleApp/CapsuleDump.c | 2 +-
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
index 93bf4252bac6..cf81eee71235 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
@@ -70,7 +70,7 @@ DumpCapsule (
@retval EFI_UNSUPPORTEDInput parameter is not valid.
**/
EFI_STATUS
-DmpCapsuleStatusVariable (
+DumpCapsuleStatusVariable (
VOID
);
@@ -481,13 +481,15 @@ ClearCapsuleStatusVariable (
0,
(VOID *)NULL
);
-if (EFI_ERROR(Status)) {
+if (Status == EFI_NOT_FOUND) {
//
- // There is no capsule variables, quit
+ // There is no more capsule variables, quit
//
break;
}
+Print (L"%s is cleared (%r)\n", CapsuleVarName, Status);
+
Index++;
if (Index > 0x) {
break;
@@ -850,7 +852,7 @@ UefiMain (
return Status;
}
if (StrCmp(Argv[1], L"-S") == 0) {
-Status = DmpCapsuleStatusVariable();
+Status = DumpCapsuleStatusVariable();
return EFI_SUCCESS;
}
if (StrCmp(Argv[1], L"-C") == 0) {
diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
b/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
index 4c85e8c23dff..11bf2e1d4530 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
@@ -283,7 +283,7 @@ Done:
@retval EFI_UNSUPPORTEDInput parameter is not valid.
**/
EFI_STATUS
-DmpCapsuleStatusVariable (
+DumpCapsuleStatusVariable (
VOID
)
{
--
2.7.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 3/5] MdeModulePkg CapsuleApp: Refine -N option help information
-N option is used to append a Capsule Header to an existing FMP capsule image with its ImageTypeId supported by the system. Cc: Michael D Kinney Cc: Jiewen Yao Cc: Yonghong Zhu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng --- MdeModulePkg/Application/CapsuleApp/CapsuleApp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c index 95aa20760560..3a87439f104e 100644 --- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c +++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c @@ -779,7 +779,8 @@ PrintUsage ( Print(L" -E: Dump UEFI ESRT table info.\n"); Print(L" -G: Convert a BMP file to be an UX capsule,\n"); Print(L" according to Windows Firmware Update document\n"); - Print(L" -N: Append a Capsule Header to an existing capsule image,\n"); + Print(L" -N: Append a Capsule Header to an existing FMP capsule image\n"); + Print(L" with its ImageTypeId supported by the system,\n"); Print(L" according to Windows Firmware Update document\n"); Print(L" -O: Output new Capsule file name\n"); Print(L" -D: Dump Capsule image header information, image payload information if it is an UX capsule\n"); -- 2.7.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 4/5] MdeModulePkg CapsuleApp: Index need be decimal for -P GET option
Also adjust the help information to be not too long to be suitable for different display resolutions. Cc: Michael D Kinney Cc: Jiewen Yao Cc: Yonghong Zhu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng --- MdeModulePkg/Application/CapsuleApp/CapsuleApp.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c index 3a87439f104e..93bf4252bac6 100644 --- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c +++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c @@ -768,14 +768,16 @@ PrintUsage ( Print(L" CapsuleApp -D \n"); Print(L" CapsuleApp -P GET -O \n"); Print(L"Parameter:\n"); - Print(L" -NR: No reset will be triggered for the capsule\n"); - Print(L" with CAPSULE_FLAGS_PERSIST_ACROSS_RESET and without CAPSULE_FLAGS_INITIATE_RESET.\n"); + Print(L" -NR: No reset will be triggered for the capsule with\n"); + Print(L" CAPSULE_FLAGS_PERSIST_ACROSS_RESET and without\n"); + Print(L" CAPSULE_FLAGS_INITIATE_RESET.\n"); Print(L" -S: Dump capsule report variable (EFI_CAPSULE_REPORT_GUID),\n"); Print(L" which is defined in UEFI specification.\n"); Print(L" -C: Clear capsule report variable (EFI_CAPSULE_REPORT_GUID),\n"); Print(L" which is defined in UEFI specification.\n"); Print(L" -P: Dump UEFI FMP protocol info, or get image with specified\n"); - Print(L" ImageTypeId and index to a file if 'GET' option is used.\n"); + Print(L" ImageTypeId and Index (decimal format) to a file if 'GET'\n"); + Print(L" option is used.\n"); Print(L" -E: Dump UEFI ESRT table info.\n"); Print(L" -G: Convert a BMP file to be an UX capsule,\n"); Print(L" according to Windows Firmware Update document\n"); @@ -783,8 +785,9 @@ PrintUsage ( Print(L" with its ImageTypeId supported by the system,\n"); Print(L" according to Windows Firmware Update document\n"); Print(L" -O: Output new Capsule file name\n"); - Print(L" -D: Dump Capsule image header information, image payload information if it is an UX capsule\n"); - Print(L" and FMP header information if it is a FMP capsule.\n"); + Print(L" -D: Dump Capsule image header information, image payload\n"); + Print(L" information if it is an UX capsule and FMP header\n"); + Print(L" information if it is a FMP capsule.\n"); } /** -- 2.7.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/5] MdeModulePkg CapsuleApp: Fix -D failed to dump Nest FMP capsule
Cc: Michael D Kinney
Cc: Jiewen Yao
Cc: Yonghong Zhu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng
---
MdeModulePkg/Application/CapsuleApp/CapsuleDump.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
b/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
index 97f1893ef433..4c85e8c23dff 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
@@ -214,7 +214,7 @@ IsNestedFmpCapsule (
// FMP GUID after ESRT one
//
NestedCapsuleHeader = (EFI_CAPSULE_HEADER *)((UINT8 *)CapsuleHeader +
CapsuleHeader->HeaderSize);
- NestedCapsuleSize = (UINTN)CapsuleHeader + CapsuleHeader->HeaderSize -
(UINTN)NestedCapsuleHeader;
+ NestedCapsuleSize = (UINTN)CapsuleHeader + CapsuleHeader->CapsuleImageSize-
(UINTN)NestedCapsuleHeader;
if (NestedCapsuleSize < sizeof(EFI_CAPSULE_HEADER)) {
return FALSE;
}
--
2.7.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/5] MdeModulePkg CapsuleApp: Fix VS2012 build failure caused by 5410502
The build failure is like below.
xxx\CapsuleApp.c(868) : error C2275: 'EFI_GUID' :
illegal use of this type as an expression
xxx/UefiBaseType.h(29) : see declaration of 'EFI_GUID'
xxx\CapsuleApp.c(868) : error C2146: syntax error :
missing ';' before identifier 'ImageTypeId'
xxx\CapsuleApp.c(868) : error C2065: 'ImageTypeId' : undeclared identifier
xxx\CapsuleApp.c(869) : error C2275: 'UINTN' :
illegal use of this type as an expression
xxx\ProcessorBind.h(224) : see declaration of 'UINTN'
xxx\CapsuleApp.c(869) : error C2146: syntax error :
missing ';' before identifier 'ImageIndex'
xxx\CapsuleApp.c(869) : error C2065: 'ImageIndex' : undeclared identifier
Cc: Jiewen Yao
Cc: Dandan Bi
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng
---
MdeModulePkg/Application/CapsuleApp/CapsuleApp.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
index df5de91ef524..95aa20760560 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
@@ -809,14 +809,16 @@ UefiMain (
UINTN FileSize[MAX_CAPSULE_NUM];
VOID *CapsuleBuffer[MAX_CAPSULE_NUM];
EFI_CAPSULE_BLOCK_DESCRIPTOR *BlockDescriptors;
- EFI_CAPSULE_HEADER *CapsuleHeaderArray[MAX_CAPSULE_NUM + 1];
- UINT64 MaxCapsuleSize;
- EFI_RESET_TYPE ResetType;
- BOOLEANNeedReset;
- BOOLEANNoReset;
- CHAR16 *CapsuleName;
- UINTN CapsuleNum;
- UINTN Index;
+ EFI_CAPSULE_HEADER*CapsuleHeaderArray[MAX_CAPSULE_NUM + 1];
+ UINT64MaxCapsuleSize;
+ EFI_RESET_TYPEResetType;
+ BOOLEAN NeedReset;
+ BOOLEAN NoReset;
+ CHAR16*CapsuleName;
+ UINTN CapsuleNum;
+ UINTN Index;
+ EFI_GUID ImageTypeId;
+ UINTN ImageIndex;
Status = GetArg();
if (EFI_ERROR(Status)) {
@@ -865,8 +867,6 @@ UefiMain (
return EFI_UNSUPPORTED;
}
-EFI_GUID ImageTypeId;
-UINTN ImageIndex;
//
// FMP->GetImage()
//
--
2.7.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/5] CapsuleApp: Some enhancements
Cc: Michael D Kinney Cc: Jiewen Yao Cc: Yonghong Zhu Star Zeng (5): MdeModulePkg CapsuleApp: Fix VS2012 build failure caused by 5410502 MdeModulePkg CapsuleApp: Fix -D failed to dump Nest FMP capsule MdeModulePkg CapsuleApp: Refine -N option help information MdeModulePkg CapsuleApp: Index need be decimal for -P GET option MdeModulePkg CapsuleApp: Prompt info for -C option MdeModulePkg/Application/CapsuleApp/CapsuleApp.c | 46 +-- MdeModulePkg/Application/CapsuleApp/CapsuleDump.c | 4 +- 2 files changed, 28 insertions(+), 22 deletions(-) -- 2.7.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch][edk2-platforms/devel-IntelAtomProcessorE3900] Add I2C Library.
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: David Wei CC: Kelly Steele CC: Mike Wu CC: Mang Guo --- .../SouthCluster/Include/Library/I2CLib.h | 4 +- .../SouthCluster/Include/ScRegs/RegsI2c.h | 31 +- .../SouthCluster/Library/I2CLib/I2CLib.c | 719 + .../SouthCluster/Library/I2CLib/I2CLib.inf | 50 ++ .../SouthCluster/Library/I2CLibPei/I2CLibPei.c | 6 +- 5 files changed, 802 insertions(+), 8 deletions(-) create mode 100644 Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Library/I2CLib/I2CLib.c create mode 100644 Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Library/I2CLib/I2CLib.inf diff --git a/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Include/Library/I2CLib.h b/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Include/Library/I2CLib.h index b897cb9dc4..0c50a3d9a5 100644 --- a/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Include/Library/I2CLib.h +++ b/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Include/Library/I2CLib.h @@ -1,7 +1,7 @@ /** @file Register Definitions for I2C Library. - Copyright (c) 1999 - 2017, Intel Corporation. All rights reserved. + Copyright (c) 1999 - 2018, Intel Corporation. All rights reserved. This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License @@ -30,7 +30,7 @@ **/ EFI_STATUS ProgramPciLpssI2C ( - VOID + IN UINT8BusNo ); /** diff --git a/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Include/ScRegs/RegsI2c.h b/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Include/ScRegs/RegsI2c.h index 5fc758dd07..8a7463b538 100644 --- a/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Include/ScRegs/RegsI2c.h +++ b/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Include/ScRegs/RegsI2c.h @@ -106,10 +106,10 @@ #defineR_IC_CLR_START_DET(0x64) ///< Clear START_DET interrupt #defineR_IC_CLR_GEN_CALL (0x68) ///< Clear GEN_CALL interrupt #defineR_IC_ENABLE (0x6C) ///< I2C Enable +#defineI2C_ENABLE_ABORT BIT1 +#defineI2C_ENABLE_ENABLE BIT0 #defineR_IC_STATUS (0x70) ///< I2C Status -#defineR_IC_SDA_HOLD (0x7C) ///< I2C IC_DEFAULT_SDA_HOLD//16bits - #defineSTAT_MST_ACTIVITY BIT5 ///< Master FSM Activity Status. #defineSTAT_RFF BIT4 ///< RX FIFO is completely full #defineSTAT_RFNE BIT3 ///< RX FIFO is not empty @@ -118,7 +118,24 @@ #defineR_IC_TXFLR(0x74) ///< Transmit FIFO Level Register #defineR_IC_RXFLR(0x78) ///< Receive FIFO Level Register +#defineR_IC_SDA_HOLD (0x7C) ///< I2C IC_DEFAULT_SDA_HOLD//16bits #defineR_IC_TX_ABRT_SOURCE (0x80) ///< I2C Transmit Abort Status Register +#defineI2C_ABRT_SLVRD_INTX BIT15 +#defineI2C_ABRT_SLV_ARBLOST BIT14 +#defineI2C_ABRT_SLVFLUSH_TXFIFO BIT13 +#defineI2C_ARB_LOST BIT12 +#defineI2C_ABRT_MASTER_DIS BIT11 +#defineI2C_ABRT_10B_RD_NORSTRT BIT10 +#defineI2C_ABRT_SBYTE_NORSTRTBIT9 +#defineI2C_ABRT_HS_NORSTRT BIT8 +#defineI2C_ABRT_SBYTE_ACKDET BIT7 +#defineI2C_ABRT_HS_ACKDETBIT6 +#defineI2C_ABRT_GCALL_READ BIT5 +#defineI2C_ABRT_GCALL_NOACK BIT4 +#defineI2C_ABRT_TXDATA_NOACK BIT3 +#defineI2C_ABRT_10ADDR2_NOACKBIT2 +#defineI2C_ABRT_10ADDR1_NOACKBIT1 +#defineI2C_ABRT_7B_ADDR_NOACKBIT0 #defineR_IC_SLV_DATA_NACK_ONLY (0x84) ///< Generate SLV_DATA_NACK Register #defineR_IC_DMA_CR (0x88) ///< DMA Control Register #defineR_IC_DMA_TDLR (0x8C) ///< DMA Transmit Data Level @@ -130,7 +147,15 @@ #defineR_IC_COMP_VERSION (0xF8) ///< Component Version ID #defineR_IC_COMP_TYPE(0xFC) ///< Component Type -#defineR_IC_CLK_GATE (0xC0) ///< Clock Gate +#defineR_IC_RESET_CONTROL(0x204) ///< Reset control +#defineI2C_RESET_CONTROLLER (BIT0 | BIT1) +#defineI2C_RESET_IDMA(BIT2) + +#defineR_IC_CLK_GATE (0x238) ///< Clock Gate +#defineI2C_FORCE_CLOCK_ON(BIT0 | BIT1) +#defineI2C_FORCE_CLOCK_OFF (BIT1) +#defineI2C_FORCE_IDMA_CLOCK_ON (BIT2 | BIT3) +#defineI2C_FORCE_IDMA_CLOCK_OFF (BIT3) #defineI2C_SS_SCL_HCNT_VALUE_100M 0x1DD #defineI2C_SS_SCL_LCNT_VALUE_100M 0x1E4 diff --git a/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Library/I2CLib/I2CLib.c
[edk2] [PATCH] BaseTools: Fix build crash when fdf is empty file
From: Yunhua Feng
Fix build crash when fdf is empty file
Fix https://bugzilla.tianocore.org/show_bug.cgi?id=912
Cc: Liming Gao
Cc: Yonghong Zhu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng
---
BaseTools/Source/Python/GenFds/FdfParser.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/BaseTools/Source/Python/GenFds/FdfParser.py
b/BaseTools/Source/Python/GenFds/FdfParser.py
index 4be790a819..ae6b10f1e3 100644
--- a/BaseTools/Source/Python/GenFds/FdfParser.py
+++ b/BaseTools/Source/Python/GenFds/FdfParser.py
@@ -421,10 +421,12 @@ class FdfParser:
def __CurrentLine(self):
return self.Profile.FileLinesList[self.CurrentLineNumber - 1]
def __StringToList(self):
self.Profile.FileLinesList = [list(s) for s in
self.Profile.FileLinesList]
+if not self.Profile.FileLinesList:
+EdkLogger.error('FdfParser', FILE_READ_FAILURE, 'The file is
empty!', File=self.FileName)
self.Profile.FileLinesList[-1].append(' ')
def __ReplaceFragment(self, StartPos, EndPos, Value = ' '):
if StartPos[0] == EndPos[0]:
Offset = StartPos[1]
--
2.12.2.windows.2
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms v3 4/6] Hisilicon/D05: Add PlatformMiscDxe driver
在 7/25/2018 6:51 PM, Ard Biesheuvel 写道:
> On 13 July 2018 at 10:15, Ming Huang wrote:
>> Fix the issue of onboard Nic not work kerenl with AMD GPU and
>> NVME SSD in board. The GPU don't support 64 MSI, so need to
>> allocate INTx, but the default interrupt number 255 is invalid,
>> so Change all the PCI Device interrupt number to 0.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang
>> Signed-off-by: Heyi Guo
>
> I don't understand why this issue is specific to this platform.
>
> Can you explain in more detail what the failure mode is, and why
> setting the PCI interrupt line is necessary here, while it doesn't
> seem to be on other platforms, even when falling back to INTx
> interrupts?
>
I don't know exactly why setting the PCI interrupt line is necessary in uefi.
This issue is analyzed by kernel guy DongDong.Liu.
@DongDong,
Can you explain the questions?
Thanks.
>> ---
>> Platform/Hisilicon/D05/D05.dsc | 1 +
>> Platform/Hisilicon/D05/D05.fdf | 1 +
>> Platform/Hisilicon/D05/Drivers/PlatformMiscDxe/PlatformMiscDxe.c | 99
>>
>> Platform/Hisilicon/D05/Drivers/PlatformMiscDxe/PlatformMiscDxe.inf | 47
>> ++
>> 4 files changed, 148 insertions(+)
>>
>> diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
>> index b6e1a9d98a..0e6d5912a0 100644
>> --- a/Platform/Hisilicon/D05/D05.dsc
>> +++ b/Platform/Hisilicon/D05/D05.dsc
>> @@ -629,6 +629,7 @@
>>
>>
>>
>> Silicon/Hisilicon/Drivers/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf
>> + Platform/Hisilicon/D05/Drivers/PlatformMiscDxe/PlatformMiscDxe.inf
>>
>>#
>># Memory test
>> diff --git a/Platform/Hisilicon/D05/D05.fdf b/Platform/Hisilicon/D05/D05.fdf
>> index 4503776d63..61e8d907f9 100644
>> --- a/Platform/Hisilicon/D05/D05.fdf
>> +++ b/Platform/Hisilicon/D05/D05.fdf
>> @@ -354,6 +354,7 @@ READ_LOCK_STATUS = TRUE
>>INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
>>INF IntelFrameworkModulePkg/Universal/StatusCode/Pei/StatusCodePei.inf
>>INF Platform/Hisilicon/D05/EarlyConfigPeim/EarlyConfigPeimD05.inf
>> + INF Platform/Hisilicon/D05/Drivers/PlatformMiscDxe/PlatformMiscDxe.inf
>>
>>INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>>
>> diff --git
>> a/Platform/Hisilicon/D05/Drivers/PlatformMiscDxe/PlatformMiscDxe.c
>> b/Platform/Hisilicon/D05/Drivers/PlatformMiscDxe/PlatformMiscDxe.c
>> new file mode 100644
>> index 00..8519b7139d
>> --- /dev/null
>> +++ b/Platform/Hisilicon/D05/Drivers/PlatformMiscDxe/PlatformMiscDxe.c
>> @@ -0,0 +1,99 @@
>> +/** @file
>> +*
>> +* Copyright (c) 2018, Hisilicon Limited. All rights reserved.
>> +* Copyright (c) 2016, Linaro Limited. All rights reserved.
>> +*
>> +* This program and the accompanying materials
>> +* are licensed and made available under the terms and conditions of the
>> BSD License
>> +* which accompanies this distribution. The full text of the license may
>> be found at
>> +* http://opensource.org/licenses/bsd-license.php
>> +*
>> +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
>> IMPLIED.
>> +*
>> +**/
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +VOID
>> +SetIntLine (
>> + )
>> +{
>> + EFI_STATUS Status;
>> + UINTN HandleIndex;
>> + EFI_HANDLE *HandleBuffer;
>> + UINTN HandleCount;
>> + EFI_PCI_IO_PROTOCOL*PciIo;
>> + UINT8 INTLine;
>> + UINTN Segment;
>> + UINTN Bus;
>> + UINTN Device;
>> + UINTN Fun;
>> +
>> + Status = gBS->LocateHandleBuffer (
>> + ByProtocol,
>> + ,
>> + NULL,
>> + ,
>> +
>> + );
>> + if (EFI_ERROR (Status)) {
>> + DEBUG ((DEBUG_ERROR, " Locate gEfiPciIoProtocol Failed.\n"));
>> + gBS->FreePool ((VOID *)HandleBuffer);
>> + return;
>> + }
>> +
>> + for (HandleIndex = 0; HandleIndex < HandleCount; HandleIndex++) {
>> + Status = gBS->HandleProtocol (
>> + HandleBuffer[HandleIndex],
>> + ,
>> + (VOID **)
>> + );
>> + if (EFI_ERROR (Status)) {
>> + continue;
>> + }
>> +
>> + INTLine = 0;
>> + (VOID)PciIo->Pci.Write (
>> + PciIo,
>> + EfiPciIoWidthUint8,
>> + PCI_INT_LINE_OFFSET,
>> + 1,
>> + );
>> + (VOID)PciIo->GetLocation (PciIo, , , , );
>> + DEBUG ((DEBUG_INFO, "Set BDF(%x-%x-%x) IntLine
[edk2] [PATCH] MdeModulePkg: Remove redundant library classes and guides in inf files
Some redundant libraray classes and guides have been removed in inf files. https://bugzilla.tianocore.org/show_bug.cgi?id=1017 https://bugzilla.tianocore.org/show_bug.cgi?id=1035 https://bugzilla.tianocore.org/show_bug.cgi?id=1033 https://bugzilla.tianocore.org/show_bug.cgi?id=1012 https://bugzilla.tianocore.org/show_bug.cgi?id=1011 Cc:Star Zeng Cc:Eric Dong Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Shenglei Zhang --- MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf | 1 - MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 - MdeModulePkg/Core/Dxe/DxeMain.inf | 1 - MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf | 1 - MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf | 1 - 5 files changed, 5 deletions(-) diff --git a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf index 4aab75bab7..d067df0400 100644 --- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf +++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf @@ -60,7 +60,6 @@ ReportStatusCodeLib [Guids] - gEfiDiskInfoIdeInterfaceGuid ## SOMETIMES_PRODUCES ## UNDEFINED gEfiDiskInfoAhciInterfaceGuid ## SOMETIMES_PRODUCES ## UNDEFINED [Protocols] diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf index a21dd2b5ed..faf68c7d90 100644 --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf @@ -78,7 +78,6 @@ BaseLib UefiDriverEntryPoint DebugLib - PeCoffLib [Protocols] gEfiPciHotPlugRequestProtocolGuid ## SOMETIMES_PRODUCES diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf index 68fa0a01d9..fc8af3d293 100644 --- a/MdeModulePkg/Core/Dxe/DxeMain.inf +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf @@ -163,7 +163,6 @@ gEfiHiiPackageListProtocolGuid## SOMETIMES_PRODUCES gEfiEbcProtocolGuid ## SOMETIMES_CONSUMES gEfiSmmBase2ProtocolGuid ## SOMETIMES_CONSUMES - gEfiBlockIoProtocolGuid ## SOMETIMES_CONSUMES # Arch Protocols gEfiBdsArchProtocolGuid ## CONSUMES diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf index a2ff773a74..f11ccb0e07 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf @@ -66,7 +66,6 @@ TimerLib HobLib SmmMemLib - DxeServicesLib [Protocols] gEfiDxeSmmReadyToLockProtocolGuid ## UNDEFINED # SmiHandlerRegister diff --git a/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf b/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf index a4184212bb..36ed80cc79 100644 --- a/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf +++ b/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf @@ -42,7 +42,6 @@ [LibraryClasses] UefiBootServicesTableLib - MemoryAllocationLib UefiDriverEntryPoint BaseMemoryLib BaseLib -- 2.18.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] BaseTools: Parse decimal format INF_VERSION incorrect
Good, the patch is not push, I can update it.
Best Regards,
Zhu Yonghong
-Original Message-
From: Carsey, Jaben
Sent: Thursday, July 26, 2018 9:24 AM
To: Zhu, Yonghong
Cc: edk2-devel@lists.01.org; Gao, Liming
Subject: Re: [edk2] [PATCH] BaseTools: Parse decimal format INF_VERSION
incorrect
Can we change the code to allocate fewer strings?
'0x' + '{0:04x}{1:04x}'.format(...)
Could be just:
'0x{0:04x}{1:04x}'.format(...
Jaben
> On Jul 25, 2018, at 5:41 PM, Zhu, Yonghong wrote:
>
> Reviewed-by: Yonghong Zhu
>
> Best Regards,
> Zhu Yonghong
>
>
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Yonghong Zhu
> Sent: Wednesday, July 25, 2018 11:41 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming
> Subject: [edk2] [PATCH] BaseTools: Parse decimal format INF_VERSION
> incorrect
>
> From: Yunhua Feng
>
> hex number 0x00010019, the major number is 0001, the minor number is 0019.
> the decimal number 1.25, the major number is 1, and the minor number
> is 25
>
> Fix https://bugzilla.tianocore.org/show_bug.cgi?id=921
>
> Cc: Liming Gao
> Cc: Yonghong Zhu
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Yunhua Feng
> ---
> BaseTools/Source/Python/Workspace/MetaFileParser.py | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py
> b/BaseTools/Source/Python/Workspace/MetaFileParser.py
> index fbfc182c8b..250cbf79a9 100644
> --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py
> +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py
> @@ -374,13 +374,16 @@ class MetaFileParser(object):
> if Name == 'INF_VERSION':
> if hexVersionPattern.match(Value):
> self._Version = int(Value, 0)
> elif decVersionPattern.match(Value):
> ValueList = Value.split('.')
> -Major = '%04o' % int(ValueList[0], 0)
> -Minor = '%04o' % int(ValueList[1], 0)
> -self._Version = int('0x' + Major + Minor, 0)
> +Major = int(ValueList[0], 0)
> +Minor = int(ValueList[1], 0)
> +if Major > 0x or Minor > 0x:
> +EdkLogger.error('Parser', FORMAT_INVALID, "Invalid
> version number",
> +ExtraData=self._CurrentLine,
> File=self.MetaFile, Line=self._LineIndex + 1)
> +self._Version = int('0x' +
> + '{0:04x}{1:04x}'.format(Major, Minor), 0)
> else:
> EdkLogger.error('Parser', FORMAT_INVALID, "Invalid version
> number",
> ExtraData=self._CurrentLine,
> File=self.MetaFile, Line=self._LineIndex + 1)
>
> if isinstance(self, InfParser) and self._Version < 0x00010005:
> --
> 2.12.2.windows.2
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] BaseTools: Parse decimal format INF_VERSION incorrect
Can we change the code to allocate fewer strings?
'0x' + '{0:04x}{1:04x}'.format(...)
Could be just:
'0x{0:04x}{1:04x}'.format(...
Jaben
> On Jul 25, 2018, at 5:41 PM, Zhu, Yonghong wrote:
>
> Reviewed-by: Yonghong Zhu
>
> Best Regards,
> Zhu Yonghong
>
>
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Yonghong Zhu
> Sent: Wednesday, July 25, 2018 11:41 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming
> Subject: [edk2] [PATCH] BaseTools: Parse decimal format INF_VERSION incorrect
>
> From: Yunhua Feng
>
> hex number 0x00010019, the major number is 0001, the minor number is 0019.
> the decimal number 1.25, the major number is 1, and the minor number is 25
>
> Fix https://bugzilla.tianocore.org/show_bug.cgi?id=921
>
> Cc: Liming Gao
> Cc: Yonghong Zhu
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Yunhua Feng
> ---
> BaseTools/Source/Python/Workspace/MetaFileParser.py | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py
> b/BaseTools/Source/Python/Workspace/MetaFileParser.py
> index fbfc182c8b..250cbf79a9 100644
> --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py
> +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py
> @@ -374,13 +374,16 @@ class MetaFileParser(object):
> if Name == 'INF_VERSION':
> if hexVersionPattern.match(Value):
> self._Version = int(Value, 0)
> elif decVersionPattern.match(Value):
> ValueList = Value.split('.')
> -Major = '%04o' % int(ValueList[0], 0)
> -Minor = '%04o' % int(ValueList[1], 0)
> -self._Version = int('0x' + Major + Minor, 0)
> +Major = int(ValueList[0], 0)
> +Minor = int(ValueList[1], 0)
> +if Major > 0x or Minor > 0x:
> +EdkLogger.error('Parser', FORMAT_INVALID, "Invalid
> version number",
> +ExtraData=self._CurrentLine,
> File=self.MetaFile, Line=self._LineIndex + 1)
> +self._Version = int('0x' +
> + '{0:04x}{1:04x}'.format(Major, Minor), 0)
> else:
> EdkLogger.error('Parser', FORMAT_INVALID, "Invalid version
> number",
> ExtraData=self._CurrentLine,
> File=self.MetaFile, Line=self._LineIndex + 1)
>
> if isinstance(self, InfParser) and self._Version < 0x00010005:
> --
> 2.12.2.windows.2
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/6] BaseTools/header.makefile: remove "-c" from BUILD_CFLAGS
Option "-c" is a mode selection flag (choosing between compiling and linking); it should not be in BUILD_CFLAGS, which applies only to compiling anyway. The compilation rule for C source files, in "footer.makefile", already includes "-c" -- currently we have double "-c" options. This patch doesn't change behavior. Cc: Liming Gao Cc: Yonghong Zhu Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1540244 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek --- BaseTools/Source/C/Makefiles/header.makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile index db436773cf40..08421ba24cd9 100644 --- a/BaseTools/Source/C/Makefiles/header.makefile +++ b/BaseTools/Source/C/Makefiles/header.makefile @@ -71,9 +71,9 @@ INCLUDE = $(TOOL_INCLUDE) -I $(MAKEROOT) -I $(MAKEROOT)/Include/Common -I $(MAKE BUILD_CPPFLAGS = $(INCLUDE) -O2 ifeq ($(DARWIN),Darwin) # assume clang or clang compatible flags on OS X -BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-self-assign -Wno-unused-result -nostdlib -c -g +BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-self-assign -Wno-unused-result -nostdlib -g else -BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-stringop-truncation -Wno-restrict -Wno-unused-result -nostdlib -c -g +BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-stringop-truncation -Wno-restrict -Wno-unused-result -nostdlib -g endif BUILD_LFLAGS = BUILD_CXXFLAGS = -Wno-unused-result -- 2.14.1.3.gb7cf6e02401b ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 5/6] BaseTools/Source/C: take EXTRA_OPTFLAGS from the caller
Allow the caller of the top-level makefile either to set EXTRA_OPTFLAGS in
the environment or to pass EXTRA_OPTFLAGS as a macro definition on the
command line. EXTRA_OPTFLAGS extends (and potentially overrides) default C
compilation flags set in the makefiles.
Cc: Liming Gao
Cc: Yonghong Zhu
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1540244
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek
---
BaseTools/Source/C/Makefiles/header.makefile | 5 -
BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 5 -
BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile | 5 -
3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/BaseTools/Source/C/Makefiles/header.makefile
b/BaseTools/Source/C/Makefiles/header.makefile
index 498c6cf48b4a..1b4cad5497ec 100644
--- a/BaseTools/Source/C/Makefiles/header.makefile
+++ b/BaseTools/Source/C/Makefiles/header.makefile
@@ -69,7 +69,10 @@ endif
INCLUDE = $(TOOL_INCLUDE) -I $(MAKEROOT) -I $(MAKEROOT)/Include/Common -I
$(MAKEROOT)/Include/ -I $(MAKEROOT)/Include/IndustryStandard -I
$(MAKEROOT)/Common/ -I .. -I . $(ARCH_INCLUDE)
BUILD_CPPFLAGS = $(INCLUDE)
-BUILD_OPTFLAGS = -O2
+
+# keep EXTRA_OPTFLAGS last
+BUILD_OPTFLAGS = -O2 $(EXTRA_OPTFLAGS)
+
ifeq ($(DARWIN),Darwin)
# assume clang or clang compatible flags on OS X
BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror
-Wno-deprecated-declarations -Wno-self-assign -Wno-unused-result -nostdlib -g
diff --git a/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
b/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
index 94e6e7292309..cefe6a078b39 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
+++ b/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
@@ -165,7 +165,10 @@ PCCTS_H=../h
# UNIX (default)
#
BUILD_CC?=gcc
-COPT=-O
+
+# keep EXTRA_OPTFLAGS last
+COPT=-O $(EXTRA_OPTFLAGS)
+
ANTLR=${BIN_DIR}/antlr
DLG=${BIN_DIR}/dlg
OBJ_EXT=o
diff --git a/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
b/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
index f4babb4b0790..a9d1771d4c5b 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
+++ b/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
@@ -115,7 +115,10 @@ PCCTS_H=../h
# UNIX
#
BUILD_CC?=cc
-COPT=-O
+
+# keep EXTRA_OPTFLAGS last
+COPT=-O $(EXTRA_OPTFLAGS)
+
ANTLR=${BIN_DIR}/antlr
DLG=${BIN_DIR}/dlg
--
2.14.1.3.gb7cf6e02401b
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 3/6] BaseTools/Source/C: split "-O2" to BUILD_OPTFLAGS
The option "-O2" is not a preprocessor flag, but a code generation (compilation) flag. Move it from BUILD_CPPFLAGS to BUILD_CFLAGS and BUILD_CXXFLAGS. Because "VfrCompile/GNUmakefile" uses "-O2" through BUILD_CPPFLAGS, and because it doesn't use BUILD_CXXFLAGS, we have to introduce BUILD_OPTFLAGS separately, so that "VfrCompile/GNUmakefile" can continue using just this flag. This patch doesn't change behavior. Cc: Liming Gao Cc: Yonghong Zhu Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1540244 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek --- BaseTools/Source/C/Makefiles/header.makefile | 6 +- BaseTools/Source/C/VfrCompile/GNUmakefile| 11 +++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile index 08421ba24cd9..498c6cf48b4a 100644 --- a/BaseTools/Source/C/Makefiles/header.makefile +++ b/BaseTools/Source/C/Makefiles/header.makefile @@ -68,7 +68,8 @@ $(error Bad HOST_ARCH) endif INCLUDE = $(TOOL_INCLUDE) -I $(MAKEROOT) -I $(MAKEROOT)/Include/Common -I $(MAKEROOT)/Include/ -I $(MAKEROOT)/Include/IndustryStandard -I $(MAKEROOT)/Common/ -I .. -I . $(ARCH_INCLUDE) -BUILD_CPPFLAGS = $(INCLUDE) -O2 +BUILD_CPPFLAGS = $(INCLUDE) +BUILD_OPTFLAGS = -O2 ifeq ($(DARWIN),Darwin) # assume clang or clang compatible flags on OS X BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-self-assign -Wno-unused-result -nostdlib -g @@ -91,6 +92,9 @@ ifeq ($(DARWIN),Darwin) endif endif +# keep BUILD_OPTFLAGS last +BUILD_CFLAGS += $(BUILD_OPTFLAGS) +BUILD_CXXFLAGS += $(BUILD_OPTFLAGS) .PHONY: all .PHONY: install diff --git a/BaseTools/Source/C/VfrCompile/GNUmakefile b/BaseTools/Source/C/VfrCompile/GNUmakefile index c4ec61aa6c86..bbe562cbc54f 100644 --- a/BaseTools/Source/C/VfrCompile/GNUmakefile +++ b/BaseTools/Source/C/VfrCompile/GNUmakefile @@ -25,6 +25,9 @@ OBJECTS = AParser.o DLexerBase.o ATokenBuffer.o EfiVfrParser.o VfrLexer.o VfrSyn VFR_CPPFLAGS = -DPCCTS_USE_NAMESPACE_STD $(BUILD_CPPFLAGS) +# keep BUILD_OPTFLAGS last +VFR_CXXFLAGS = $(BUILD_OPTFLAGS) + LINKER = $(BUILD_CXX) EXTRA_CLEAN_OBJECTS = EfiVfrParser.cpp EfiVfrParser.h VfrParser.dlg VfrTokens.h VfrLexer.cpp VfrLexer.h VfrSyntax.cpp tokens.h @@ -58,16 +61,16 @@ Pccts/dlg/dlg: BIN_DIR='.' $(MAKE) -C Pccts/dlg ATokenBuffer.o: Pccts/h/ATokenBuffer.cpp - $(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $? -o $@ + $(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $(VFR_CXXFLAGS) $? -o $@ DLexerBase.o: Pccts/h/DLexerBase.cpp - $(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $? -o $@ + $(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $(VFR_CXXFLAGS) $? -o $@ AParser.o: Pccts/h/AParser.cpp - $(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $? -o $@ + $(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $(VFR_CXXFLAGS) $? -o $@ VfrSyntax.o: VfrSyntax.cpp - $(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $? -o $@ + $(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $(VFR_CXXFLAGS) $? -o $@ clean: localClean -- 2.14.1.3.gb7cf6e02401b ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller
Repo: https://github.com/lersek/edk2.git Branch: extra_flags_rhbz1540244 In the Fedora distribution, we'd like to pass system-wide flags related to optimization and linking when the C and C++ language base tools are built. This series lets the outermost "make" command push the EXTRA_OPTFLAGS and EXTRA_LDFLAGS macros into the BaseTools build. Cc: Liming Gao Cc: Yonghong Zhu Thanks Laszlo Laszlo Ersek (6): BaseTools/footer.makefile: expand BUILD_CFLAGS last for C files too BaseTools/header.makefile: remove "-c" from BUILD_CFLAGS BaseTools/Source/C: split "-O2" to BUILD_OPTFLAGS BaseTools/Pccts: clean up antlr and dlg makefiles BaseTools/Source/C: take EXTRA_OPTFLAGS from the caller BaseTools/Source/C: take EXTRA_LDFLAGS from the caller BaseTools/Source/C/Makefiles/footer.makefile | 2 +- BaseTools/Source/C/Makefiles/header.makefile | 16 --- BaseTools/Source/C/VfrCompile/GNUmakefile | 11 +--- BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 22 ++- BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile | 28 +--- 5 files changed, 56 insertions(+), 23 deletions(-) -- 2.14.1.3.gb7cf6e02401b ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 6/6] BaseTools/Source/C: take EXTRA_LDFLAGS from the caller
Allow the caller of the top-level makefile either to set EXTRA_LDFLAGS in
the environment or to pass EXTRA_LDFLAGS as a macro definition on the
command line. EXTRA_LDFLAGS extends (and potentially overrides) default
link-editing flags set in the makefiles.
Cc: Liming Gao
Cc: Yonghong Zhu
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1540244
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek
---
BaseTools/Source/C/Makefiles/header.makefile | 3 +++
BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 6 +-
BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile | 6 +-
3 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/BaseTools/Source/C/Makefiles/header.makefile
b/BaseTools/Source/C/Makefiles/header.makefile
index 1b4cad5497ec..7f283d6464a8 100644
--- a/BaseTools/Source/C/Makefiles/header.makefile
+++ b/BaseTools/Source/C/Makefiles/header.makefile
@@ -99,6 +99,9 @@ endif
BUILD_CFLAGS += $(BUILD_OPTFLAGS)
BUILD_CXXFLAGS += $(BUILD_OPTFLAGS)
+# keep EXTRA_LDFLAGS last
+BUILD_LFLAGS += $(EXTRA_LDFLAGS)
+
.PHONY: all
.PHONY: install
.PHONY: clean
diff --git a/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
b/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
index cefe6a078b39..68897cb81e20 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
+++ b/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
@@ -178,6 +178,10 @@ OUT_OBJ = -o
BUILD_CFLAGS=$(COPT)
BUILD_CPPFLAGS=-I. -I$(SET) -I$(PCCTS_H) -DUSER_ZZSYN -DZZLEXBUFSIZE=65536
+
+# keep EXTRA_LDFLAGS last
+BUILD_LFLAGS = $(EXTRA_LDFLAGS)
+
#
# SGI Users, use this CFLAGS
#
@@ -186,7 +190,7 @@ OBJ=antlr.o scan.o err.o bits.o build.o fset2.o fset.o
gen.o \
globals.o hash.o lex.o main.o misc.o set.o pred.o egman.o mrhoist.o
fcache.o
$(BIN_DIR)/antlr : $(OBJ) $(SRC)
- $(BUILD_CC) -o $(BIN_DIR)/antlr $(OBJ)
+ $(BUILD_CC) -o $(BIN_DIR)/antlr $(BUILD_LFLAGS) $(OBJ)
# what files does PCCTS generate (both ANTLR and DLG)
PCCTS_GEN=antlr.c scan.c err.c tokens.h mode.h parser.dlg stdpccts.h remap.h
diff --git a/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
b/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
index a9d1771d4c5b..9a185b4b9c7a 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
+++ b/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
@@ -126,13 +126,17 @@ DLG=${BIN_DIR}/dlg
BUILD_CFLAGS=$(COPT)
BUILD_CPPFLAGS=-I. -I$(SET) -I$(PCCTS_H) -DUSER_ZZSYN -DZZLEXBUFSIZE=65536
+
+# keep EXTRA_LDFLAGS last
+BUILD_LFLAGS = $(EXTRA_LDFLAGS)
+
OBJ_EXT=o
OUT_OBJ = -o
OBJ = dlg_p.o dlg_a.o main.o err.o set.o support.o output.o \
relabel.o automata.o
$(BIN_DIR)/dlg : $(OBJ) $(SRC)
- $(BUILD_CC) -o $(BIN_DIR)/dlg $(OBJ)
+ $(BUILD_CC) -o $(BIN_DIR)/dlg $(BUILD_LFLAGS) $(OBJ)
SRC = dlg_p.c dlg_a.c main.c err.c $(SET)/set.c support.c output.c \
relabel.c automata.c
--
2.14.1.3.gb7cf6e02401b
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 4/6] BaseTools/Pccts: clean up antlr and dlg makefiles
(1) "-I" and "-D" options are for the preprocessor; move them to
BUILD_CPPFLAGS. (This unifies BUILD_CPPFLAGS between both makefiles.)
(2) COTHER is never set, drop it from "antlr". (This unifies BUILD_CFLAGS
between both makefiles, as COPT.)
(3) For linking "antlr" and "dlg", both BUILD_CFLAGS and BUILD_CPPFLAGS
are useless, so drop BUILD_CFLAGS, and don't add BUILD_CPPFLAGS.
(4) For compiling C source files:
(4a) Move the "-c" mode selector to the front.
(4b) Expand both BUILD_CPPFLAGS and BUILD_CFLAGS, in this order. (This
results in COPT being expanded last.)
(4c) Turn the source file operand into the last argument on the command
line.
The only change in behavior from this patch is that the following options
disappear from the link-editing steps (due to (3)):
-O -I. -I../support/set -I../h -DUSER_ZZSYN -DZZLEXBUFSIZE=65536
However these options made no difference for linking in the first place.
Cc: Liming Gao
Cc: Yonghong Zhu
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1540244
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek
---
BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 13 -
BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile | 19 +++
2 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
b/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
index 8f2cc78c5947..94e6e7292309 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
+++ b/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
@@ -170,8 +170,11 @@ ANTLR=${BIN_DIR}/antlr
DLG=${BIN_DIR}/dlg
OBJ_EXT=o
OUT_OBJ = -o
-BUILD_CFLAGS= $(COPT) -I. -I$(SET) -I$(PCCTS_H) -DUSER_ZZSYN $(COTHER)
-DZZLEXBUFSIZE=65536
-BUILD_CPPFLAGS=
+
+# keep COPT last
+BUILD_CFLAGS=$(COPT)
+
+BUILD_CPPFLAGS=-I. -I$(SET) -I$(PCCTS_H) -DUSER_ZZSYN -DZZLEXBUFSIZE=65536
#
# SGI Users, use this CFLAGS
#
@@ -180,7 +183,7 @@ OBJ=antlr.o scan.o err.o bits.o build.o fset2.o fset.o
gen.o \
globals.o hash.o lex.o main.o misc.o set.o pred.o egman.o mrhoist.o
fcache.o
$(BIN_DIR)/antlr : $(OBJ) $(SRC)
- $(BUILD_CC) $(BUILD_CFLAGS) -o $(BIN_DIR)/antlr $(OBJ)
+ $(BUILD_CC) -o $(BIN_DIR)/antlr $(OBJ)
# what files does PCCTS generate (both ANTLR and DLG)
PCCTS_GEN=antlr.c scan.c err.c tokens.h mode.h parser.dlg stdpccts.h remap.h
@@ -203,10 +206,10 @@ scan.o : scan.c mode.h tokens.h
# $(DLG) -C2 parser.dlg scan.c
set.o : $(SET)/set.c
- $(BUILD_CC) $(BUILD_CFLAGS) -c -o set.o $(SET)/set.c
+ $(BUILD_CC) -c $(BUILD_CPPFLAGS) $(BUILD_CFLAGS) -o set.o $(SET)/set.c
%.o : %.c
- $(BUILD_CC) -c $(BUILD_CFLAGS) $(BUILD_CPPFLAGS) $< -o $@
+ $(BUILD_CC) -c $(BUILD_CPPFLAGS) $(BUILD_CFLAGS) -o $@ $<
#
# ** These next targets are common to UNIX and PC world
diff --git a/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
b/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
index b3a34d3b4613..f4babb4b0790 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
+++ b/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
@@ -118,15 +118,18 @@ BUILD_CC?=cc
COPT=-O
ANTLR=${BIN_DIR}/antlr
DLG=${BIN_DIR}/dlg
-BUILD_CFLAGS= $(COPT) -I. -I$(SET) -I$(PCCTS_H) -DUSER_ZZSYN
-DZZLEXBUFSIZE=65536
-BUILD_CPPFLAGS=
+
+# keep COPT last
+BUILD_CFLAGS=$(COPT)
+
+BUILD_CPPFLAGS=-I. -I$(SET) -I$(PCCTS_H) -DUSER_ZZSYN -DZZLEXBUFSIZE=65536
OBJ_EXT=o
OUT_OBJ = -o
OBJ = dlg_p.o dlg_a.o main.o err.o set.o support.o output.o \
relabel.o automata.o
$(BIN_DIR)/dlg : $(OBJ) $(SRC)
- $(BUILD_CC) $(BUILD_CFLAGS) -o $(BIN_DIR)/dlg $(OBJ)
+ $(BUILD_CC) -o $(BIN_DIR)/dlg $(OBJ)
SRC = dlg_p.c dlg_a.c main.c err.c $(SET)/set.c support.c output.c \
relabel.c automata.c
@@ -138,19 +141,19 @@ SRC = dlg_p.c dlg_a.c main.c err.c $(SET)/set.c support.c
output.c \
# $(DLG) -C2 parser.dlg dlg_a.c
dlg_p.$(OBJ_EXT) : dlg_p.c dlg.h tokens.h mode.h
- $(BUILD_CC) $(BUILD_CFLAGS) -c dlg_p.c
+ $(BUILD_CC) -c $(BUILD_CPPFLAGS) $(BUILD_CFLAGS) dlg_p.c
dlg_a.$(OBJ_EXT) : dlg_a.c dlg.h tokens.h mode.h
- $(BUILD_CC) $(BUILD_CFLAGS) -c dlg_a.c
+ $(BUILD_CC) -c $(BUILD_CPPFLAGS) $(BUILD_CFLAGS) dlg_a.c
main.$(OBJ_EXT) : main.c dlg.h
- $(BUILD_CC) $(BUILD_CFLAGS) -c main.c
+ $(BUILD_CC) -c $(BUILD_CPPFLAGS) $(BUILD_CFLAGS) main.c
set.$(OBJ_EXT) : $(SET)/set.c
- $(BUILD_CC) -c $(BUILD_CFLAGS) $(SET)/set.c
+ $(BUILD_CC) -c $(BUILD_CPPFLAGS) $(BUILD_CFLAGS) $(SET)/set.c
%.o : %.c
- $(BUILD_CC) -c $(BUILD_CFLAGS) $(BUILD_CPPFLAGS) $< -o $@
+ $(BUILD_CC) -c $(BUILD_CPPFLAGS) $(BUILD_CFLAGS) -o $@ $<
lint:
lint *.c
--
2.14.1.3.gb7cf6e02401b
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/6] BaseTools/footer.makefile: expand BUILD_CFLAGS last for C files too
BUILD_CPPFLAGS should be expanded before BUILD_CFLAGS. (The rule for C++ source files already does this, with BUILD_CPPFLAGS and BUILD_CXXFLAGS.) This patch doesn't change behavior. Cc: Liming Gao Cc: Yonghong Zhu Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1540244 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek --- BaseTools/Source/C/Makefiles/footer.makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BaseTools/Source/C/Makefiles/footer.makefile b/BaseTools/Source/C/Makefiles/footer.makefile index 0926aa964547..5bda9e4e36d5 100644 --- a/BaseTools/Source/C/Makefiles/footer.makefile +++ b/BaseTools/Source/C/Makefiles/footer.makefile @@ -24,7 +24,7 @@ $(LIBRARY): $(OBJECTS) $(BUILD_AR) crs $@ $^ %.o : %.c - $(BUILD_CC) -c $(BUILD_CFLAGS) $(BUILD_CPPFLAGS) $< -o $@ + $(BUILD_CC) -c $(BUILD_CPPFLAGS) $(BUILD_CFLAGS) $< -o $@ %.o : %.cpp $(BUILD_CXX) -c $(BUILD_CPPFLAGS) $(BUILD_CXXFLAGS) $< -o $@ -- 2.14.1.3.gb7cf6e02401b ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] BaseTools: Fix bug about *M value not display decimal and hexadecimal
Reviewed-by: Yonghong Zhu
Best Regards,
Zhu Yonghong
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yonghong
Zhu
Sent: Wednesday, July 25, 2018 8:47 AM
To: edk2-devel@lists.01.org
Cc: Gao, Liming
Subject: [edk2] [Patch] BaseTools: Fix bug about *M value not display decimal
and hexadecimal
From: Yunhua Feng
V2: Add the check for Pcd DatumType
report format like as below:
*M Shell.inf = 0xFF (255)
Cc: Liming Gao
Cc: Yonghong Zhu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng
---
BaseTools/Source/Python/build/BuildReport.py | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/BaseTools/Source/Python/build/BuildReport.py
b/BaseTools/Source/Python/build/BuildReport.py
index 176a390..dd5d1c0 100644
--- a/BaseTools/Source/Python/build/BuildReport.py
+++ b/BaseTools/Source/Python/build/BuildReport.py
@@ -1117,11 +1117,17 @@ class PcdReport(object):
if IsByteArray:
FileWrite(File, ' *M %-*s = %s' %
(self.MaxLen + 15, ModulePath, '{'))
for Array in ArrayList:
FileWrite(File, Array)
else:
-FileWrite(File, ' *M %-*s = %s' %
(self.MaxLen + 15, ModulePath, ModuleDefault.strip()))
+Value = ModuleDefault.strip()
+if Pcd.DatumType in
TAB_PCD_CLEAN_NUMERIC_TYPES:
+if Value.startswith(('0x', '0X')):
+Value = '{} ({:d})'.format(Value,
int(Value, 0))
+else:
+Value = "0x{:X}
({})".format(int(Value, 0), Value)
+FileWrite(File, ' *M %-*s = %s' %
(self.MaxLen + 15, ModulePath, Value))
if ModulePcdSet is None:
FileWrite(File, gSectionEnd)
else:
if not ReportSubType and ModulePcdSet:
--
2.6.1.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] BaseTools: Parse decimal format INF_VERSION incorrect
Reviewed-by: Yonghong Zhu
Best Regards,
Zhu Yonghong
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yonghong
Zhu
Sent: Wednesday, July 25, 2018 11:41 AM
To: edk2-devel@lists.01.org
Cc: Gao, Liming
Subject: [edk2] [PATCH] BaseTools: Parse decimal format INF_VERSION incorrect
From: Yunhua Feng
hex number 0x00010019, the major number is 0001, the minor number is 0019.
the decimal number 1.25, the major number is 1, and the minor number is 25
Fix https://bugzilla.tianocore.org/show_bug.cgi?id=921
Cc: Liming Gao
Cc: Yonghong Zhu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng
---
BaseTools/Source/Python/Workspace/MetaFileParser.py | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py
b/BaseTools/Source/Python/Workspace/MetaFileParser.py
index fbfc182c8b..250cbf79a9 100644
--- a/BaseTools/Source/Python/Workspace/MetaFileParser.py
+++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py
@@ -374,13 +374,16 @@ class MetaFileParser(object):
if Name == 'INF_VERSION':
if hexVersionPattern.match(Value):
self._Version = int(Value, 0)
elif decVersionPattern.match(Value):
ValueList = Value.split('.')
-Major = '%04o' % int(ValueList[0], 0)
-Minor = '%04o' % int(ValueList[1], 0)
-self._Version = int('0x' + Major + Minor, 0)
+Major = int(ValueList[0], 0)
+Minor = int(ValueList[1], 0)
+if Major > 0x or Minor > 0x:
+EdkLogger.error('Parser', FORMAT_INVALID, "Invalid version
number",
+ExtraData=self._CurrentLine,
File=self.MetaFile, Line=self._LineIndex + 1)
+self._Version = int('0x' +
+ '{0:04x}{1:04x}'.format(Major, Minor), 0)
else:
EdkLogger.error('Parser', FORMAT_INVALID, "Invalid version
number",
ExtraData=self._CurrentLine,
File=self.MetaFile, Line=self._LineIndex + 1)
if isinstance(self, InfParser) and self._Version < 0x00010005:
--
2.12.2.windows.2
___
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] BaseTools/Ecc: Add some new checkpoints
Reviewed-by: Yonghong Zhu
Best Regards,
Zhu Yonghong
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yonghong
Zhu
Sent: Monday, July 23, 2018 2:03 PM
To: edk2-devel@lists.01.org
Cc: Chen, Hesheng
Subject: [edk2] [PATCH] BaseTools/Ecc: Add some new checkpoints
From: hchen30
1. Add a checkpoint to check NO TABs.
2. Add a checkpoint to check line ending with CRLF.
3. Add a checkpoint to check no trailing spaces.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hess Chen
---
BaseTools/Source/Python/Ecc/Check.py | 54
BaseTools/Source/Python/Ecc/Configuration.py | 4 +++
BaseTools/Source/Python/Ecc/EccToolError.py | 8 +++--
BaseTools/Source/Python/Ecc/config.ini | 4 +++
4 files changed, 68 insertions(+), 2 deletions(-)
diff --git a/BaseTools/Source/Python/Ecc/Check.py
b/BaseTools/Source/Python/Ecc/Check.py
index 0b81013d77..6803afdfdd 100644
--- a/BaseTools/Source/Python/Ecc/Check.py
+++ b/BaseTools/Source/Python/Ecc/Check.py
@@ -188,6 +188,60 @@ class Check(object):
def GeneralCheck(self):
self.GeneralCheckNonAcsii()
self.UniCheck()
+self.GeneralCheckNoTab()
+self.GeneralCheckLineEnding()
+self.GeneralCheckTrailingWhiteSpaceLine()
+
+# Check whether NO Tab is used, replaced with spaces
+def GeneralCheckNoTab(self):
+if EccGlobalData.gConfig.GeneralCheckNoTab == '1' or
EccGlobalData.gConfig.GeneralCheckAll == '1' or EccGlobalData.gConfig.CheckAll
== '1':
+EdkLogger.quiet("Checking No TAB used in file ...")
+SqlCommand = """select ID, FullPath, ExtName from File where
ExtName in ('.dec', '.inf', '.dsc', 'c', 'h')"""
+RecordSet = EccGlobalData.gDb.TblFile.Exec(SqlCommand)
+for Record in RecordSet:
+if Record[2].upper() not in
EccGlobalData.gConfig.BinaryExtList:
+op = open(Record[1]).readlines()
+IndexOfLine = 0
+for Line in op:
+IndexOfLine += 1
+IndexOfChar = 0
+for Char in Line:
+IndexOfChar += 1
+if Char == '\t':
+OtherMsg = "File %s has TAB char at line %s
column %s" % (Record[1], IndexOfLine, IndexOfChar)
+
+ EccGlobalData.gDb.TblReport.Insert(ERROR_GENERAL_CHECK_NO_TAB,
+ OtherMsg=OtherMsg, BelongsToTable='File', BelongsToItem=Record[0])
+
+# Check Only use CRLF (Carriage Return Line Feed) line endings.
+def GeneralCheckLineEnding(self):
+if EccGlobalData.gConfig.GeneralCheckLineEnding == '1' or
EccGlobalData.gConfig.GeneralCheckAll == '1' or EccGlobalData.gConfig.CheckAll
== '1':
+EdkLogger.quiet("Checking line ending in file ...")
+SqlCommand = """select ID, FullPath, ExtName from File where
ExtName in ('.dec', '.inf', '.dsc', 'c', 'h')"""
+RecordSet = EccGlobalData.gDb.TblFile.Exec(SqlCommand)
+for Record in RecordSet:
+if Record[2].upper() not in
EccGlobalData.gConfig.BinaryExtList:
+op = open(Record[1], 'rb').readlines()
+IndexOfLine = 0
+for Line in op:
+IndexOfLine += 1
+if not Line.endswith('\r\n'):
+OtherMsg = "File %s has invalid line ending at
line %s" % (Record[1], IndexOfLine)
+
+ EccGlobalData.gDb.TblReport.Insert(ERROR_GENERAL_CHECK_INVALID_LINE_EN
+ DING, OtherMsg=OtherMsg, BelongsToTable='File',
+ BelongsToItem=Record[0])
+
+# Check if there is no trailing white space in one line.
+def GeneralCheckTrailingWhiteSpaceLine(self):
+if EccGlobalData.gConfig.GeneralCheckTrailingWhiteSpaceLine == '1' or
EccGlobalData.gConfig.GeneralCheckAll == '1' or EccGlobalData.gConfig.CheckAll
== '1':
+EdkLogger.quiet("Checking trailing white space line in file ...")
+SqlCommand = """select ID, FullPath, ExtName from File where
ExtName in ('.dec', '.inf', '.dsc', 'c', 'h')"""
+RecordSet = EccGlobalData.gDb.TblFile.Exec(SqlCommand)
+for Record in RecordSet:
+if Record[2].upper() not in
EccGlobalData.gConfig.BinaryExtList:
+op = open(Record[1], 'rb').readlines()
+IndexOfLine = 0
+for Line in op:
+IndexOfLine += 1
+if Line.replace('\r', '').replace('\n', '').endswith('
'):
+OtherMsg = "File %s has trailing white spaces at
line %s" % (Record[1], IndexOfLine)
+
+ EccGlobalData.gDb.TblReport.Insert(ERROR_GENERAL_CHECK_TRAILING_WHITE_
+ SPACE_LINE, OtherMsg=OtherMsg, BelongsToTable='File',
+
Re: [edk2] [PATCH v1] ShellPkg: add UefiShellAcpiViewCommandLib.inf to ShellPkg.dsc
Never mind most of that. I have to figure out how to configure gitk to show .dsc changes. It's all good. > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Carsey, Jaben > Sent: Wednesday, July 25, 2018 1:47 PM > To: Alexei Fedorov ; Ni, Ruiyu > ; edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH v1] ShellPkg: add > UefiShellAcpiViewCommandLib.inf to ShellPkg.dsc > Importance: High > > Alexei, > > I did it, but I just realized it looks like it didn't make a change. Shoot. > I think it > was already pushed, but I am not sure. > > -Jaben > > > -Original Message- > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > > Alexei Fedorov > > Sent: Wednesday, July 25, 2018 9:29 AM > > To: Ni, Ruiyu ; edk2-devel@lists.01.org > > Subject: Re: [edk2] [PATCH v1] ShellPkg: add > > UefiShellAcpiViewCommandLib.inf to ShellPkg.dsc > > Importance: High > > > > Hello, > > > > > > Is there anything else to be done to get this patch pushed? > > > > > > Thanks. > > > > Alexei > > > > > > From: edk2-devel on behalf of Ni, > Ruiyu > > > > Sent: 18 July 2018 03:38:58 > > To: edk2-devel@lists.01.org > > Subject: Re: [edk2] [PATCH v1] ShellPkg: add > > UefiShellAcpiViewCommandLib.inf to ShellPkg.dsc > > > > On 7/17/2018 6:59 PM, AlexeiFedorov wrote: > > > This patch adds UefiShellAcpiViewCommandLib INF file into > > > [Components] section of ShellPkg.dsc so this library can be built > > > in ShellPkg level build. > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Alexei Fedorov > > > --- > > > All the changes can be reviewed at > > > https://github.com/AlexeiFedorov/edk2/tree/298_add_acpiview_lib_v1 > > > > > > Notes: > > > v1: > > > - add UefiShellAcpiViewCommandLib.inf to ShellPkg.dsc > > > > > > ShellPkg/ShellPkg.dsc | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/ShellPkg/ShellPkg.dsc b/ShellPkg/ShellPkg.dsc > > > index > > > 21038ae8d596a9b5a520aca704494280cf8afd55..cb2a2422edd7b5ea6279ea81f > > e0ba9cc78511216 100644 > > > --- a/ShellPkg/ShellPkg.dsc > > > +++ b/ShellPkg/ShellPkg.dsc > > > @@ -2,6 +2,7 @@ > > > # Shell Package > > > # > > > # Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved. > > > +# Copyright (c) 2018, Arm Limited. All rights reserved. > > > # > > > #This program and the accompanying materials > > > #are licensed and made available under the terms and conditions of > the > > BSD License > > > @@ -90,6 +91,7 @@ [Components] > > > # This helps developers test changes and how they affect the package. > > > # > > > ShellPkg/Library/UefiShellLib/UefiShellLib.inf > > > + > > > ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman > > dLib.inf > > > ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf > > > ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf > > > ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf > > > > > Reviewed-by: Ruiyu Ni > > > > -- > > Thanks, > > Ray > > ___ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > > IMPORTANT NOTICE: The contents of this email and any attachments are > > confidential and may also be privileged. If you are not the intended > recipient, > > please notify the sender immediately and do not disclose the contents to > any > > other person, use it for any purpose, or store or copy the information in > any > > medium. Thank you. > > ___ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > ___ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v1] ShellPkg: add UefiShellAcpiViewCommandLib.inf to ShellPkg.dsc
Alexei, I did it, but I just realized it looks like it didn't make a change. Shoot. I think it was already pushed, but I am not sure. -Jaben > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Alexei Fedorov > Sent: Wednesday, July 25, 2018 9:29 AM > To: Ni, Ruiyu ; edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH v1] ShellPkg: add > UefiShellAcpiViewCommandLib.inf to ShellPkg.dsc > Importance: High > > Hello, > > > Is there anything else to be done to get this patch pushed? > > > Thanks. > > Alexei > > > From: edk2-devel on behalf of Ni, Ruiyu > > Sent: 18 July 2018 03:38:58 > To: edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH v1] ShellPkg: add > UefiShellAcpiViewCommandLib.inf to ShellPkg.dsc > > On 7/17/2018 6:59 PM, AlexeiFedorov wrote: > > This patch adds UefiShellAcpiViewCommandLib INF file into > > [Components] section of ShellPkg.dsc so this library can be built > > in ShellPkg level build. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Alexei Fedorov > > --- > > All the changes can be reviewed at > > https://github.com/AlexeiFedorov/edk2/tree/298_add_acpiview_lib_v1 > > > > Notes: > > v1: > > - add UefiShellAcpiViewCommandLib.inf to ShellPkg.dsc > > > > ShellPkg/ShellPkg.dsc | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/ShellPkg/ShellPkg.dsc b/ShellPkg/ShellPkg.dsc > > index > 21038ae8d596a9b5a520aca704494280cf8afd55..cb2a2422edd7b5ea6279ea81f > e0ba9cc78511216 100644 > > --- a/ShellPkg/ShellPkg.dsc > > +++ b/ShellPkg/ShellPkg.dsc > > @@ -2,6 +2,7 @@ > > # Shell Package > > # > > # Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved. > > +# Copyright (c) 2018, Arm Limited. All rights reserved. > > # > > #This program and the accompanying materials > > #are licensed and made available under the terms and conditions of the > BSD License > > @@ -90,6 +91,7 @@ [Components] > > # This helps developers test changes and how they affect the package. > > # > > ShellPkg/Library/UefiShellLib/UefiShellLib.inf > > + > ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman > dLib.inf > > ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf > > ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf > > ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf > > > Reviewed-by: Ruiyu Ni > > -- > Thanks, > Ray > ___ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, > please notify the sender immediately and do not disclose the contents to any > other person, use it for any purpose, or store or copy the information in any > medium. Thank you. > ___ > 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 v1] MdeModulePkg: Fix memory leak in FvSimpleFileSystem driver
FvSimpleFileSystem on read always allocates a FileBuffer, and never frees
it. This causes memory leaks. It is especially bad for reading scripts
line-by-line. In some cases memory leak can exceed 1GB.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Vladimir Olovyannikiov
---
.../Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
index 746f2ced708a..fde208594737 100644
--- a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
+++ b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
@@ -704,6 +704,7 @@ FvSimpleFileSystemRead (
Status = FvFsReadFile (File->Instance->FvProtocol, File->FvFileInfo,
, );
if (EFI_ERROR (Status)) {
+ FreePool (FileBuffer);
return EFI_DEVICE_ERROR;
}
@@ -714,6 +715,8 @@ FvSimpleFileSystemRead (
CopyMem (Buffer, (UINT8*)FileBuffer + File->Position, *BufferSize);
File->Position += *BufferSize;
+FreePool (FileBuffer);
+
return EFI_SUCCESS;
}
}
--
2.18.0
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v1] ShellPkg: add UefiShellAcpiViewCommandLib.inf to ShellPkg.dsc
Hello, Is there anything else to be done to get this patch pushed? Thanks. Alexei From: edk2-devel on behalf of Ni, Ruiyu Sent: 18 July 2018 03:38:58 To: edk2-devel@lists.01.org Subject: Re: [edk2] [PATCH v1] ShellPkg: add UefiShellAcpiViewCommandLib.inf to ShellPkg.dsc On 7/17/2018 6:59 PM, AlexeiFedorov wrote: > This patch adds UefiShellAcpiViewCommandLib INF file into > [Components] section of ShellPkg.dsc so this library can be built > in ShellPkg level build. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Alexei Fedorov > --- > All the changes can be reviewed at > https://github.com/AlexeiFedorov/edk2/tree/298_add_acpiview_lib_v1 > > Notes: > v1: > - add UefiShellAcpiViewCommandLib.inf to ShellPkg.dsc > > ShellPkg/ShellPkg.dsc | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/ShellPkg/ShellPkg.dsc b/ShellPkg/ShellPkg.dsc > index > 21038ae8d596a9b5a520aca704494280cf8afd55..cb2a2422edd7b5ea6279ea81fe0ba9cc78511216 > 100644 > --- a/ShellPkg/ShellPkg.dsc > +++ b/ShellPkg/ShellPkg.dsc > @@ -2,6 +2,7 @@ > # Shell Package > # > # Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved. > +# Copyright (c) 2018, Arm Limited. All rights reserved. > # > #This program and the accompanying materials > #are licensed and made available under the terms and conditions of the > BSD License > @@ -90,6 +91,7 @@ [Components] > # This helps developers test changes and how they affect the package. > # > ShellPkg/Library/UefiShellLib/UefiShellLib.inf > + > ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf > ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf > ShellPkg/Library/UefiShellCEntryLib/UefiShellCEntryLib.inf > ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf > Reviewed-by: Ruiyu Ni -- Thanks, Ray ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Library class misname
Note: I changed the subject as this is not about Laszlo's patch at this point. Liming, I agree. We definitely need to balance performance and features. I didn't say we should add it; I assumed it was already there. Was there any side effect of a misnamed library class? Could a module have used the library with the wrong name? What if there was a library with correct name also included? -Jaben > -Original Message- > From: Gao, Liming > Sent: Tuesday, July 24, 2018 5:45 PM > To: Carsey, Jaben ; Laszlo Ersek > ; edk2-devel-01 > Subject: RE: [edk2] [PATCH] ArmVirtPkg: remove wrong and superfluous > ResourcePublicationLib resolution > Importance: High > > Jaben: > Yes. BaseTools can generate the error message for it. But, more checker in > BaseTools parser phase will also increase the build performance. This is the > balance here. > > Thanks > Liming > > -Original Message- > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Carsey, Jaben > > Sent: Wednesday, July 25, 2018 6:25 AM > > To: Laszlo Ersek ; edk2-devel-01 de...@lists.01.org> > > Subject: Re: [edk2] [PATCH] ArmVirtPkg: remove wrong and superfluous > ResourcePublicationLib resolution > > > > Laszlo, > > > > Do you think that using the incorrect name for a library instance should > generate an error? I would have thought it would have... > > > > -Jaben > > > > > -Original Message- > > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > > > Laszlo Ersek > > > Sent: Tuesday, July 24, 2018 6:21 AM > > > To: edk2-devel-01 > > > Subject: [edk2] [PATCH] ArmVirtPkg: remove wrong and superfluous > > > ResourcePublicationLib resolution > > > > > > The class name for the "PeiResourcePublicationLib" instance is just > > > "ResourcePublicationLib", not "PeiResourcePublicationLib". However, no > > > module included in the ArmVirtPkg platforms depends on this lib class; > > > remove its resolution. > > > > > > Cc: Ard Biesheuvel > > > Cc: Julien Grall > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Laszlo Ersek > > > --- > > > > > > Notes: > > > built on all six platforms: > > > > > > ArmVirtQemu-AARCH64 > > > ArmVirtQemu-ARM > > > ArmVirtQemuKernel-AARCH64 > > > ArmVirtQemuKernel-ARM > > > ArmVirtXen-AARCH64 > > > ArmVirtXen-ARM > > > > > > ArmVirtPkg/ArmVirt.dsc.inc | 1 - > > > 1 file changed, 1 deletion(-) > > > > > > diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc > > > index 8bb54c5e6579..70a0ac4d786c 100644 > > > --- a/ArmVirtPkg/ArmVirt.dsc.inc > > > +++ b/ArmVirtPkg/ArmVirt.dsc.inc > > > @@ -211,7 +211,6 @@ [LibraryClasses.common.PEIM] > > > > > > > PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanc > > > eLib.inf > > > > > > > OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibN > > > ull/OemHookStatusCodeLibNull.inf > > > > > > > PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/Base > > > PeCoffGetEntryPointLib.inf > > > - > > > > PeiResourcePublicationLib|MdePkg/Library/PeiResourcePublicationLib/PeiR > > > esourcePublicationLib.inf > > > > > > > UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDeco > > > mpressLib.inf > > > > > > > ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiEx > > > tractGuidedSectionLib.inf > > > > > > -- > > > 2.14.1.3.gb7cf6e02401b > > > > > > ___ > > > edk2-devel mailing list > > > edk2-devel@lists.01.org > > > https://lists.01.org/mailman/listinfo/edk2-devel > > ___ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch V2] UefiCpuPkg/MpInitLib: Remove redundant parameter.
On 07/25/18 13:35, Dong, Eric wrote: >> From: Laszlo Ersek [mailto:ler...@redhat.com] >> Sent: Wednesday, July 25, 2018 6:14 PM >> To: Dong, Eric ; edk2-devel@lists.01.org >> Cc: Ni, Ruiyu >> On 07/25/18 05:50, Dong, Eric wrote: >>> But some AP may wake up later and it failed to return from the >>> procedure. >> >> Ah! So that explains another symptom I've since seen as well -- >> although *very* rarely. Namely, if an AP wakes up *after* >> PiSmmCpuDxeSmm moves on, thinking that all APs are finished, the AP >> can execute garbage in "no man's land" -- and that crashes the guest. >> Basically, QEMU/KVM pause the guest with "emulation failure", and >> QEMU dumps the VCPU register state to the standard error on the host >> side. In particular, the register state indicates that the crashed >> VCPU is *not* in SMM. > > Where to get these QEMU dumps info? QEMU simply writes the register dump to stderr. If you use QEMU together with libvirt, then QEMU's stderr is saved in: /var/log/libvirt/qemu/$DOMAIN.log Here's an example: KVM internal error. Suberror: 1 emulation failure RAX= RBX=0005 RCX=7eab9828 RDX=0002 RSI=0009f000 RDI=7eef2ae0 RBP=7eef2e40 RSP=7eef2a08 R8 =0002 R9 = R10= R11= R12= R13= R14= R15= RIP=7ea9cdb2 RFL=00010046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES =0030 00c09300 DPL=0 DS [-WA] CS =0038 00a09b00 DPL=0 CS64 [-RA] SS =0030 00c09300 DPL=0 DS [-WA] DS =0030 00c09300 DPL=0 DS [-WA] FS =0030 00c09300 DPL=0 DS [-WA] GS =0030 00c09300 DPL=0 DS [-WA] LDT= 8200 DPL=0 LDT TR = 8b00 DPL=0 TSS64-busy GDT= 7ebed998 0047 IDT= 7e237280 0fff CR0=c0010033 CR2= CR3=7ec01000 CR4=0668 DR0= DR1= DR2= DR3= DR6=0ff0 DR7=0400 EFER=0500 Code=89 d0 5f c3 cc cc cc cc cc cc cc cc cc cc cc cc cc cc f3 90 cc cc cc cc cc cc cc cc cc cc cc cc cc 53 89 c8 89 d1 50 0f a2 4c 8b 54 24 38 4d 85 d2 (Anyway, this is just for general discussion now -- your v3 series works perfectly for me.) > I'm not clear who calls StartAllAPs function when I send this mail, I > just based on the debug message found StartAllAps trigged right after > PiSmmCpuDxeSmm driver. but I think this not blocks my patches for this > issue. So I sent the patches and back to dig it more. Sure, I think your analysis was spot-on, regardless of how you produced it. The register dumps / emulation failures that I mentioned in my previous email in this thread are not a new issue with your v3 posting. The v3 series is fine. Instead, these failures were a *further* (but less frequent) symptom with the *original* series, and I encountered them separately from the boot hangs that I reported at first. > Now I found it's PiSmmIpl driver calls gDS->SetMemorySpaceAttributes > in SmmIplDxeDispatchEventNotify function which finally calls the > StartAllAps. > The Ap procedure is SetMtrrsFromBuffer function in CpuDxe driver. In > SetMtrrsFromBuffer function, it just call MtrrSetAllMtrrs to update > the MTRR, and it use local variable to save the MTRR settings. > > When the hang issue raised, in the time the AP begin to run the > procedure, the BSP has leave current function. So the saved MTRR > setting in the local variable is not valid anymore. So the > MtrrSetAllMtrrs function use an invalid Mtrr Setttings to set mtrrs > and never exit the procedure. Do you think it's reasonable? Absolutely. Basically, if an AP is allowed to continue running the user procedure after the BSP in MpInitLib thinks that the AP is finished, anything at all can happen -- the escapes control, it may access memory that has been freed, and perhaps it might even execute code that has been freed and overwritten. The actual symptoms can vary wildly. > I found till the ExitBootService point, the AP still in busy state. I > check the ApWakeupFunction function, found between the AP procedure > and set AP state to Idle, no other possible code exist. So I think the > AP should not return back from procedure. I try to add debug message > to know where the AP is stopped at. But after I add debug message in > MtrrSetAllMtrrs, the issue can't reproduce anymore. I tried about 30 > times. Do you have other message to get to know where the AP is? No, I don't have any other ideas at hand to deduce where the AP was stuck -- as far as I'm concerned, it had just wandered off into the weeds :) I think your idea to check the AP state at the time of the
Re: [edk2] [Patch V3 0/3] StartAllAPs should not use disabled APs
On 07/25/18 14:12, Laszlo Ersek wrote: > On 07/25/18 09:50, Eric Dong wrote: >> This patch series include changes: >> 1. StartAllAPs should not use disabled APs, this is required by UEFI spec. >> 2. Refine the code to remove the redundant definitions. >> >> V2 changes: >> Use CpuStateReady to distinguish the AP state from CpuStateIdle. >> >> V3 Changes: >> Only change 3/3 patch. Only not use disabled APs when WakeUpAP called >> by StartAllAps function. In other cases, also include disabled APs. >> >> Eric Dong (3): >> UefiCpuPkg/MpInitLib: Remove redundant CpuStateFinished State. >> UefiCpuPkg/MpInitLib: Remove StartCount and volatile definition. >> UefiCpuPkg/MpInitLib: Not use disabled AP when call StartAllAPs. >> >> UefiCpuPkg/Library/MpInitLib/MpLib.c | 33 +++-- >> UefiCpuPkg/Library/MpInitLib/MpLib.h | 4 +--- >> 2 files changed, 16 insertions(+), 21 deletions(-) >> > > I requested commit message updates for all three patches. With those > implemented, please add: > > Reviewed-by: Laszlo Ersek > > (No need to repost.) > > Please give me some more time to regression-test this series as well. This series works nicely on my end. I performed my usual Linux guest tests, in all my usual guest environments, plus 10 cold boots in each environment. Everything worked fine. For patches #1 and #2: Tested-by: Laszlo Ersek For patch #3: Regression-tested-by: Laszlo Ersek (I'm giving R-t-b and not T-b for patch #3 because OVMF doesn't exercise the MP PPI/Protocol member functions that disable APs.) >From my side, please update the commit messages as agreed, and go ahead with the pushing. Thanks! Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch v3 2/3] UefiCpuPkg/MpInitLib: Remove StartCount and volatile definition.
On 07/25/18 14:09, Dong, Eric wrote: > Hi Laszlo, > > >> -Original Message- >> From: Laszlo Ersek [mailto:ler...@redhat.com] >> Sent: Wednesday, July 25, 2018 7:47 PM >> To: Dong, Eric ; edk2-devel@lists.01.org >> Cc: Ni, Ruiyu >> Subject: Re: [edk2] [Patch v3 2/3] UefiCpuPkg/MpInitLib: Remove StartCount >> and volatile definition. >> >> Hi Eric, >> >> On 07/25/18 09:50, Eric Dong wrote: >>> The StartCount is duplicated with RunningCount, replace it with >>> RunningCount. Also the volatile for RunningCount is not needed. >> >> after staring at this patch for a long time, I think it is correct. >> However, I suggest that we improve the commit message, because this patch >> actually does three things: >> >> (1) It removes "volatile" from RunningCount. >> >> [This is OK, because only the BSP modifies it.] >> >> (2) [This is the tricky part.] >> >> When we detect a timeout in CheckAllAPs(), and collect the list of failed >> CPUs, >> the size of the list is derived from the following difference, before the >> patch: >> >> StartCount - FinishedCount >> >> where "StartCount" is set by the BSP at startup, and FinishedCount is >> incremented by the APs themselves. >> > > I think FinishedCount used here is a typo. What the logic from the code > context here is want to get the failed Aps and return them. So it should use > RunningCount here, right? Also the FinishedCount may be bigger than > StartCount if many Aps has been disabled. Right? I agree FinishedCount looks questionable / out of place in the original code. Thanks Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch 2/2] SecurityPkg/Tcg: Add use case for new Perf macro
Reviewed-by: Liming Gao
> -Original Message-
> From: Bi, Dandan
> Sent: Thursday, July 19, 2018 2:44 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming ; Zhang, Chao B
> Subject: [patch 2/2] SecurityPkg/Tcg: Add use case for new Perf macro
>
> Add an example case for the usage of
> PERF_CALLBACK_BEGIN/PERF_CALLBACK_END
>
> Cc: Liming Gao
> Cc: Chao Zhang
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi
> ---
> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 4
> SecurityPkg/Tcg/TcgPei/TcgPei.c | 5 +
> SecurityPkg/Tcg/TcgPei/TcgPei.inf | 1 +
> 3 files changed, 10 insertions(+)
>
> diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> index 74cdd1fa88..09ef0c70a5 100644
> --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> @@ -159,10 +159,12 @@ EndofPeiSignalNotifyCallBack (
> {
>MEASURED_HOB_DATA *MeasuredHobData;
>
>MeasuredHobData = NULL;
>
> + PERF_CALLBACK_BEGIN ();
> +
>//
>// Create a Guid hob to save all measured Fv
>//
>MeasuredHobData = BuildGuidHob(
>,
> @@ -184,10 +186,12 @@ EndofPeiSignalNotifyCallBack (
> // Save measured child Fv info
> //
> CopyMem (>MeasuredFvBuf[mMeasuredBaseFvIndex] ,
> mMeasuredChildFvInfo,
> sizeof(EFI_PLATFORM_FIRMWARE_BLOB) * (mMeasuredChildFvIndex));
>}
>
> + PERF_CALLBACK_END ();
> +
>return EFI_SUCCESS;
> }
>
> /**
>Make sure that the current PCR allocations, the TPM supported PCRs,
> diff --git a/SecurityPkg/Tcg/TcgPei/TcgPei.c b/SecurityPkg/Tcg/TcgPei/TcgPei.c
> index 1ed11a1b29..d07047580c 100644
> --- a/SecurityPkg/Tcg/TcgPei/TcgPei.c
> +++ b/SecurityPkg/Tcg/TcgPei/TcgPei.c
> @@ -39,10 +39,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> #include
> #include
> #include
> #include
> #include
> +#include
>
> BOOLEAN mImageInMemory = FALSE;
>
> EFI_PEI_PPI_DESCRIPTOR mTpmInitializedPpiList = {
>EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
> @@ -168,10 +169,12 @@ EndofPeiSignalNotifyCallBack (
> {
>MEASURED_HOB_DATA *MeasuredHobData;
>
>MeasuredHobData = NULL;
>
> + PERF_CALLBACK_BEGIN ();
> +
>//
>// Create a Guid hob to save all measured Fv
>//
>MeasuredHobData = BuildGuidHob(
>,
> @@ -193,10 +196,12 @@ EndofPeiSignalNotifyCallBack (
> // Save measured child Fv info
> //
> CopyMem (>MeasuredFvBuf[mMeasuredBaseFvIndex] ,
> mMeasuredChildFvInfo,
> sizeof(EFI_PLATFORM_FIRMWARE_BLOB) * (mMeasuredChildFvIndex));
>}
>
> + PERF_CALLBACK_END ();
> +
>return EFI_SUCCESS;
> }
>
> /**
> Single function calculates SHA1 digest value for all raw data. It
> diff --git a/SecurityPkg/Tcg/TcgPei/TcgPei.inf
> b/SecurityPkg/Tcg/TcgPei/TcgPei.inf
> index 0252511391..4c8a055c6c 100644
> --- a/SecurityPkg/Tcg/TcgPei/TcgPei.inf
> +++ b/SecurityPkg/Tcg/TcgPei/TcgPei.inf
> @@ -54,10 +54,11 @@
>BaseLib
>PcdLib
>MemoryAllocationLib
>ReportStatusCodeLib
>Tpm12CommandLib
> + PerformanceLib
>
> [Guids]
>gTcgEventEntryHobGuid ##
> PRODUCES ## HOB
>gTpmErrorHobGuid##
> SOMETIMES_PRODUCES ## HOB
>gMeasuredFvHobGuid ##
> PRODUCES ## HOB
> --
> 2.14.3.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch 1/2] MdeModulePkg/DxeLoadFunc: Add use case for new Perf macro
Reviewed-by: Liming Gao
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Dandan
> Bi
> Sent: Thursday, July 19, 2018 2:44 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Gao, Liming
> Subject: [edk2] [patch 1/2] MdeModulePkg/DxeLoadFunc: Add use case for new
> Perf macro
>
> Add an example case for the usage of
> PERF_EVENT_SIGNAL_BEGIN/PERF_EVENT_SIGNAL_END
>
> Cc: Liming Gao
> Cc: Star Zeng
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi
> ---
> MdeModulePkg/Core/DxeIplPeim/DxeIpl.h | 3 ++-
> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 +
> MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 4
> 3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
> b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
> index 6f8e13d213..9ea88a399b 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
> @@ -1,10 +1,10 @@
> /** @file
>Master header file for DxeIpl PEIM. All source files in this module should
>include this file for common definitions.
>
> -Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be
> found at
> http://opensource.org/licenses/bsd-license.php
>
> @@ -46,10 +46,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> #include
> #include
> #include
> #include
> #include
> +#include
>
> #define STACK_SIZE 0x2
> #define BSP_STORE_SIZE 0x4000
>
>
> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> index 7deeb8f270..302934283a 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> @@ -77,10 +77,11 @@
>BaseLib
>PeimEntryPoint
>DebugLib
>DebugAgentLib
>PeiServicesTablePointerLib
> + PerformanceLib
>
> [LibraryClasses.ARM, LibraryClasses.AARCH64]
>ArmMmuLib
>
> [Ppis]
> diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
> b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
> index d5aa0474b0..8a939b6c24 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
> @@ -318,11 +318,13 @@ HandOffToDxeCore (
> PageTables = CreateIdentityMappingPageTables (BaseOfStack, STACK_SIZE);
>
> //
> // End of PEI phase signal
> //
> +PERF_EVENT_SIGNAL_BEGIN (gEndOfPeiSignalPpi.Guid);
> Status = PeiServicesInstallPpi ();
> +PERF_EVENT_SIGNAL_END (gEndOfPeiSignalPpi.Guid);
> ASSERT_EFI_ERROR (Status);
>
> AsmWriteCr3 (PageTables);
>
> //
> @@ -435,11 +437,13 @@ HandOffToDxeCore (
> }
>
> //
> // End of PEI phase signal
> //
> +PERF_EVENT_SIGNAL_BEGIN (gEndOfPeiSignalPpi.Guid);
> Status = PeiServicesInstallPpi ();
> +PERF_EVENT_SIGNAL_END (gEndOfPeiSignalPpi.Guid);
> ASSERT_EFI_ERROR (Status);
>
> if (BuildPageTablesIa32Pae) {
>AsmWriteCr3 (PageTables);
>//
> --
> 2.14.3.windows.1
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch v3 3/3] UefiCpuPkg/MpInitLib: Not use disabled AP when call StartAllAPs.
Hi Laszlo,
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, July 25, 2018 8:12 PM
> To: Dong, Eric ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu
> Subject: Re: [edk2] [Patch v3 3/3] UefiCpuPkg/MpInitLib: Not use disabled AP
> when call StartAllAPs.
>
> Hi Eric,
>
> On 07/25/18 09:50, Eric Dong wrote:
> > Base on UEFI spec requirement, StartAllAPs function should not use the
> > APs which has been disabled before. This patch just change current
> > code to follow this rule.
> >
> > V3 changes:
> > Only called by StartUpAllAps, WakeUpAp will not wake up the disabled
> > APs, in other cases also need to include the disabled APs, such as
> > CpuDxe driver start up and ChangeApLoopCallback function.
> >
> > Cc: Laszlo Ersek
> > Cc: Ruiyu Ni
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Dong
> > ---
> > UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 2 +-
> > UefiCpuPkg/Library/MpInitLib/MpLib.c| 27 +--
> > UefiCpuPkg/Library/MpInitLib/MpLib.h| 4 +++-
> > 3 files changed, 21 insertions(+), 12 deletions(-)
> >
> > diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> > b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> > index d82e9aea45..c17daa0fc0 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> > +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> > @@ -306,7 +306,7 @@ MpInitChangeApLoopCallback (
> >CpuMpData->PmCodeSegment = GetProtectedModeCS ();
> >CpuMpData->ApLoopMode = PcdGet8 (PcdCpuApLoopMode);
> >mNumberToFinish = CpuMpData->CpuCount - 1;
> > - WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, NULL);
> > + WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, NULL, TRUE);
> >while (mNumberToFinish > 0) {
> > CpuPause ();
> >}
> > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > index 0e57cc86bf..1a81062a3f 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > @@ -470,7 +470,7 @@ CollectProcessorCount (
> >//
> >CpuMpData->InitFlag = ApInitConfig;
> >CpuMpData->X2ApicEnable = FALSE;
> > - WakeUpAP (CpuMpData, TRUE, 0, NULL, NULL);
> > + WakeUpAP (CpuMpData, TRUE, 0, NULL, NULL, TRUE);
> >CpuMpData->InitFlag = ApInitDone;
> >ASSERT (CpuMpData->CpuCount <= PcdGet32
> (PcdCpuMaxLogicalProcessorNumber));
> >//
> > @@ -491,7 +491,7 @@ CollectProcessorCount (
> > //
> > // Wakeup all APs to enable x2APIC mode
> > //
> > -WakeUpAP (CpuMpData, TRUE, 0, ApFuncEnableX2Apic, NULL);
> > +WakeUpAP (CpuMpData, TRUE, 0, ApFuncEnableX2Apic, NULL, TRUE);
> > //
> > // Wait for all known APs finished
> > //
> > @@ -969,6 +969,7 @@ FreeResetVector (
> >@param[in] ProcessorNumberThe handle number of specified
> processor
> >@param[in] Procedure The function to be invoked by AP
> >@param[in] ProcedureArgument The argument to be passed into AP
> > function
> > + @param[in] WakeUpDisabledAps Whether need to wake up disabled APs
> in broadcast mode.
> > **/
> > VOID
> > WakeUpAP (
> > @@ -976,7 +977,8 @@ WakeUpAP (
> >IN BOOLEAN Broadcast,
> >IN UINTN ProcessorNumber,
> >IN EFI_AP_PROCEDURE Procedure, OPTIONAL
> > - IN VOID *ProcedureArgument OPTIONAL
> > + IN VOID *ProcedureArgument, OPTIONAL
> > + IN BOOLEAN WakeUpDisabledAps
> >)
> > {
> >volatile MP_CPU_EXCHANGE_INFO*ExchangeInfo;
> > @@ -1010,6 +1012,10 @@ WakeUpAP (
> > for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
> >if (Index != CpuMpData->BspNumber) {
> > CpuData = >CpuData[Index];
> > +if (GetApState (CpuData) == CpuStateDisabled
> && !WakeUpDisabledAps) {
> > + continue;
> > +}
> > +
> > CpuData->ApFunction = (UINTN) Procedure;
> > CpuData->ApFunctionArgument = (UINTN) ProcedureArgument;
> > SetApState (CpuData, CpuStateReady);
>
> I think something *might* be missing from the patch here, but I'm not sure.
>
> Namely, is it possible that we can reach WakeUpAP() with:
>
> (Broadcast && WakeUpDisabledAps)
>
> *after* some APs have been disabled?
>
> Because, in that case, we set the AP state from Disabled to Ready (and the AP
> will perform the Procedure as necessary) -- however, once the AP is done, we
> do not seem to re-set its state to Disabled.
>
> Is that correct?
Yes, for current use cases, we don't have a valid case need to re-set the AP to
Disabled state.
>
> I tried to collect all the WakeUpAp() calls that pass TRUE for both booleans
> mentioned above. Their callers are as follows:
>
> - MpInitLibInitialize()
> - CollectProcessorCount()
> - MpInitChangeApLoopCallback()
>
> From these three, MpInitLibInitialize() and CollectProcessorCount() run
> before the protocol or
Re: [edk2] [Patch V3 0/3] StartAllAPs should not use disabled APs
On 07/25/18 09:50, Eric Dong wrote: > This patch series include changes: > 1. StartAllAPs should not use disabled APs, this is required by UEFI spec. > 2. Refine the code to remove the redundant definitions. > > V2 changes: > Use CpuStateReady to distinguish the AP state from CpuStateIdle. > > V3 Changes: > Only change 3/3 patch. Only not use disabled APs when WakeUpAP called > by StartAllAps function. In other cases, also include disabled APs. > > Eric Dong (3): > UefiCpuPkg/MpInitLib: Remove redundant CpuStateFinished State. > UefiCpuPkg/MpInitLib: Remove StartCount and volatile definition. > UefiCpuPkg/MpInitLib: Not use disabled AP when call StartAllAPs. > > UefiCpuPkg/Library/MpInitLib/MpLib.c | 33 +++-- > UefiCpuPkg/Library/MpInitLib/MpLib.h | 4 +--- > 2 files changed, 16 insertions(+), 21 deletions(-) > I requested commit message updates for all three patches. With those implemented, please add: Reviewed-by: Laszlo Ersek (No need to repost.) Please give me some more time to regression-test this series as well. Thanks! Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch v3 3/3] UefiCpuPkg/MpInitLib: Not use disabled AP when call StartAllAPs.
Hi Eric,
On 07/25/18 09:50, Eric Dong wrote:
> Base on UEFI spec requirement, StartAllAPs function should not
> use the APs which has been disabled before. This patch just
> change current code to follow this rule.
>
> V3 changes:
> Only called by StartUpAllAps, WakeUpAp will not wake up
> the disabled APs, in other cases also need to include the
> disabled APs, such as CpuDxe driver start up and
> ChangeApLoopCallback function.
>
> Cc: Laszlo Ersek
> Cc: Ruiyu Ni
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong
> ---
> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 2 +-
> UefiCpuPkg/Library/MpInitLib/MpLib.c| 27 +--
> UefiCpuPkg/Library/MpInitLib/MpLib.h| 4 +++-
> 3 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> index d82e9aea45..c17daa0fc0 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> @@ -306,7 +306,7 @@ MpInitChangeApLoopCallback (
>CpuMpData->PmCodeSegment = GetProtectedModeCS ();
>CpuMpData->ApLoopMode = PcdGet8 (PcdCpuApLoopMode);
>mNumberToFinish = CpuMpData->CpuCount - 1;
> - WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, NULL);
> + WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, NULL, TRUE);
>while (mNumberToFinish > 0) {
> CpuPause ();
>}
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 0e57cc86bf..1a81062a3f 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -470,7 +470,7 @@ CollectProcessorCount (
>//
>CpuMpData->InitFlag = ApInitConfig;
>CpuMpData->X2ApicEnable = FALSE;
> - WakeUpAP (CpuMpData, TRUE, 0, NULL, NULL);
> + WakeUpAP (CpuMpData, TRUE, 0, NULL, NULL, TRUE);
>CpuMpData->InitFlag = ApInitDone;
>ASSERT (CpuMpData->CpuCount <= PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
>//
> @@ -491,7 +491,7 @@ CollectProcessorCount (
> //
> // Wakeup all APs to enable x2APIC mode
> //
> -WakeUpAP (CpuMpData, TRUE, 0, ApFuncEnableX2Apic, NULL);
> +WakeUpAP (CpuMpData, TRUE, 0, ApFuncEnableX2Apic, NULL, TRUE);
> //
> // Wait for all known APs finished
> //
> @@ -969,6 +969,7 @@ FreeResetVector (
>@param[in] ProcessorNumberThe handle number of specified processor
>@param[in] Procedure The function to be invoked by AP
>@param[in] ProcedureArgument The argument to be passed into AP function
> + @param[in] WakeUpDisabledAps Whether need to wake up disabled APs in
> broadcast mode.
> **/
> VOID
> WakeUpAP (
> @@ -976,7 +977,8 @@ WakeUpAP (
>IN BOOLEAN Broadcast,
>IN UINTN ProcessorNumber,
>IN EFI_AP_PROCEDURE Procedure, OPTIONAL
> - IN VOID *ProcedureArgument OPTIONAL
> + IN VOID *ProcedureArgument, OPTIONAL
> + IN BOOLEAN WakeUpDisabledAps
>)
> {
>volatile MP_CPU_EXCHANGE_INFO*ExchangeInfo;
> @@ -1010,6 +1012,10 @@ WakeUpAP (
> for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
>if (Index != CpuMpData->BspNumber) {
> CpuData = >CpuData[Index];
> +if (GetApState (CpuData) == CpuStateDisabled && !WakeUpDisabledAps) {
> + continue;
> +}
> +
> CpuData->ApFunction = (UINTN) Procedure;
> CpuData->ApFunctionArgument = (UINTN) ProcedureArgument;
> SetApState (CpuData, CpuStateReady);
I think something *might* be missing from the patch here, but I'm not sure.
Namely, is it possible that we can reach WakeUpAP() with:
(Broadcast && WakeUpDisabledAps)
*after* some APs have been disabled?
Because, in that case, we set the AP state from Disabled to Ready (and
the AP will perform the Procedure as necessary) -- however, once the AP
is done, we do not seem to re-set its state to Disabled.
Is that correct?
I tried to collect all the WakeUpAp() calls that pass TRUE for both
booleans mentioned above. Their callers are as follows:
- MpInitLibInitialize()
- CollectProcessorCount()
- MpInitChangeApLoopCallback()
>From these three, MpInitLibInitialize() and CollectProcessorCount() run
before the protocol or PPI user has any chance to disable a CPU, so it
looks impossible to reach the problematic code path I'm describing from
those functions.
What about MpInitChangeApLoopCallback()?... Hm, that function is only
called as a notification function for:
- the exit-boot-services event, and
- the legacy boot event
After which the MP protocol is unusable anyway, so it doesn't matter if
we don't flip the AP status back to Disabled.
OK, so this patch looks correct to me; can you please update the commit
message:
"WakeUpAP() is called with (Broadcast && WakeUpDisabledAps) from
MpInitLibInitialize(), CollectProcessorCount()
Re: [edk2] [Patch v3 2/3] UefiCpuPkg/MpInitLib: Remove StartCount and volatile definition.
Hi Laszlo,
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, July 25, 2018 7:47 PM
> To: Dong, Eric ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu
> Subject: Re: [edk2] [Patch v3 2/3] UefiCpuPkg/MpInitLib: Remove StartCount
> and volatile definition.
>
> Hi Eric,
>
> On 07/25/18 09:50, Eric Dong wrote:
> > The StartCount is duplicated with RunningCount, replace it with
> > RunningCount. Also the volatile for RunningCount is not needed.
>
> after staring at this patch for a long time, I think it is correct.
> However, I suggest that we improve the commit message, because this patch
> actually does three things:
>
> (1) It removes "volatile" from RunningCount.
>
> [This is OK, because only the BSP modifies it.]
>
> (2) [This is the tricky part.]
>
> When we detect a timeout in CheckAllAPs(), and collect the list of failed
> CPUs,
> the size of the list is derived from the following difference, before the
> patch:
>
> StartCount - FinishedCount
>
> where "StartCount" is set by the BSP at startup, and FinishedCount is
> incremented by the APs themselves.
>
I think FinishedCount used here is a typo. What the logic from the code context
here is want to get the failed Aps and return them. So it should use
RunningCount here, right? Also the FinishedCount may be bigger than StartCount
if many Aps has been disabled. Right?
> The patch replaces this difference with
>
> StartCount - RunningCount
>
> that is, the difference is no more calculated from the BSP's startup counter
> and the AP's shared finish counter, but from the RunningCount measurement
> that the BSP does itself, in CheckAllAPs().
>
> [This change is OK to me as well, but we should be clear about it.]
>
> (3) Finally, the patch changes the meaning of RunningCount. Before the patch,
> we have:
>
> - StartCount: the number of APs the BSP stars up,
> - RunningCount: the number of finished APs that the BSP collected
>
> After the patch, StartCount is removed, and RunningCount is *redefined* as
> the following difference:
>
> OLD_StartCount - OLD_RunningCount
>
> Giving the number of APs that the BSP started up but hasn't collected yet.
>
> [This change looks good to me as well.]
>
> *
>
> Importantly, what we see in the AllocatePool() argument, is the
> *composition* of steps (2) and (3).
>
> If you agree, can you please update the commit message to include my points
> (1) through (3)? It's OK if you leave out my remarks in brackets [].
Agreed. Will use your content when commit the changes.
Thanks,
Eric
>
> No need to repost just because of this, of course.
>
> Thanks!
> Laszlo
>
> >
> > Cc: Laszlo Ersek
> > Cc: Ruiyu Ni
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Dong
> > ---
> > UefiCpuPkg/Library/MpInitLib/MpLib.c | 11 +--
> > UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +--
> > 2 files changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > index ff09a0e9e7..0e57cc86bf 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > @@ -1424,7 +1424,7 @@ CheckAllAPs (
> > // value of state after setting the it to CpuStateIdle, so BSP can
> > safely
> make use of its value.
> > //
> > if (GetApState(CpuData) == CpuStateIdle) {
> > - CpuMpData->RunningCount ++;
> > + CpuMpData->RunningCount --;
> >CpuMpData->CpuData[ProcessorNumber].Waiting = FALSE;
> >
> >//
> > @@ -1449,7 +1449,7 @@ CheckAllAPs (
> >//
> >// If all APs finish, return EFI_SUCCESS.
> >//
> > - if (CpuMpData->RunningCount == CpuMpData->StartCount) {
> > + if (CpuMpData->RunningCount == 0) {
> > return EFI_SUCCESS;
> >}
> >
> > @@ -1466,7 +1466,7 @@ CheckAllAPs (
> > //
> > if (CpuMpData->FailedCpuList != NULL) {
> >*CpuMpData->FailedCpuList =
> > - AllocatePool ((CpuMpData->StartCount - CpuMpData->FinishedCount
> + 1) * sizeof (UINTN));
> > + AllocatePool ((CpuMpData->RunningCount + 1) * sizeof
> > + (UINTN));
> >ASSERT (*CpuMpData->FailedCpuList != NULL);
> > }
> > ListIndex = 0;
> > @@ -2212,7 +2212,7 @@ StartupAllAPsWorker (
> > return EFI_NOT_STARTED;
> >}
> >
> > - CpuMpData->StartCount = 0;
> > + CpuMpData->RunningCount = 0;
> >for (ProcessorNumber = 0; ProcessorNumber < ProcessorCount;
> ProcessorNumber++) {
> > CpuData = >CpuData[ProcessorNumber];
> > CpuData->Waiting = FALSE;
> > @@ -,7 +,7 @@ StartupAllAPsWorker (
> > // Mark this processor as responsible for current calling.
> > //
> > CpuData->Waiting = TRUE;
> > -CpuMpData->StartCount++;
> > +CpuMpData->RunningCount++;
> >}
> > }
> >}
> > @@ -2231,7 +2231,6 @@ StartupAllAPsWorker (
> >CpuMpData->ProcArguments = ProcedureArgument;
> >
Re: [edk2] [Patch v3 2/3] UefiCpuPkg/MpInitLib: Remove StartCount and volatile definition.
Hi Eric,
On 07/25/18 09:50, Eric Dong wrote:
> The StartCount is duplicated with RunningCount, replace it with
> RunningCount. Also the volatile for RunningCount is not needed.
after staring at this patch for a long time, I think it is correct.
However, I suggest that we improve the commit message, because this
patch actually does three things:
(1) It removes "volatile" from RunningCount.
[This is OK, because only the BSP modifies it.]
(2) [This is the tricky part.]
When we detect a timeout in CheckAllAPs(), and collect the list of
failed CPUs, the size of the list is derived from the following
difference, before the patch:
StartCount - FinishedCount
where "StartCount" is set by the BSP at startup, and FinishedCount is
incremented by the APs themselves.
The patch replaces this difference with
StartCount - RunningCount
that is, the difference is no more calculated from the BSP's startup
counter and the AP's shared finish counter, but from the RunningCount
measurement that the BSP does itself, in CheckAllAPs().
[This change is OK to me as well, but we should be clear about it.]
(3) Finally, the patch changes the meaning of RunningCount. Before the
patch, we have:
- StartCount: the number of APs the BSP stars up,
- RunningCount: the number of finished APs that the BSP collected
After the patch, StartCount is removed, and RunningCount is *redefined*
as the following difference:
OLD_StartCount - OLD_RunningCount
Giving the number of APs that the BSP started up but hasn't collected yet.
[This change looks good to me as well.]
*
Importantly, what we see in the AllocatePool() argument, is the
*composition* of steps (2) and (3).
If you agree, can you please update the commit message to include my
points (1) through (3)? It's OK if you leave out my remarks in brackets [].
No need to repost just because of this, of course.
Thanks!
Laszlo
>
> Cc: Laszlo Ersek
> Cc: Ruiyu Ni
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong
> ---
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 11 +--
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +--
> 2 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index ff09a0e9e7..0e57cc86bf 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -1424,7 +1424,7 @@ CheckAllAPs (
> // value of state after setting the it to CpuStateIdle, so BSP can
> safely make use of its value.
> //
> if (GetApState(CpuData) == CpuStateIdle) {
> - CpuMpData->RunningCount ++;
> + CpuMpData->RunningCount --;
>CpuMpData->CpuData[ProcessorNumber].Waiting = FALSE;
>
>//
> @@ -1449,7 +1449,7 @@ CheckAllAPs (
>//
>// If all APs finish, return EFI_SUCCESS.
>//
> - if (CpuMpData->RunningCount == CpuMpData->StartCount) {
> + if (CpuMpData->RunningCount == 0) {
> return EFI_SUCCESS;
>}
>
> @@ -1466,7 +1466,7 @@ CheckAllAPs (
> //
> if (CpuMpData->FailedCpuList != NULL) {
>*CpuMpData->FailedCpuList =
> - AllocatePool ((CpuMpData->StartCount - CpuMpData->FinishedCount +
> 1) * sizeof (UINTN));
> + AllocatePool ((CpuMpData->RunningCount + 1) * sizeof (UINTN));
>ASSERT (*CpuMpData->FailedCpuList != NULL);
> }
> ListIndex = 0;
> @@ -2212,7 +2212,7 @@ StartupAllAPsWorker (
> return EFI_NOT_STARTED;
>}
>
> - CpuMpData->StartCount = 0;
> + CpuMpData->RunningCount = 0;
>for (ProcessorNumber = 0; ProcessorNumber < ProcessorCount;
> ProcessorNumber++) {
> CpuData = >CpuData[ProcessorNumber];
> CpuData->Waiting = FALSE;
> @@ -,7 +,7 @@ StartupAllAPsWorker (
> // Mark this processor as responsible for current calling.
> //
> CpuData->Waiting = TRUE;
> -CpuMpData->StartCount++;
> +CpuMpData->RunningCount++;
>}
> }
>}
> @@ -2231,7 +2231,6 @@ StartupAllAPsWorker (
>CpuMpData->ProcArguments = ProcedureArgument;
>CpuMpData->SingleThread = SingleThread;
>CpuMpData->FinishedCount = 0;
> - CpuMpData->RunningCount = 0;
>CpuMpData->FailedCpuList = FailedCpuList;
>CpuMpData->ExpectedTime = CalculateTimeout (
> TimeoutInMicroseconds,
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 962bce685d..5002b7e9c0 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -211,9 +211,8 @@ struct _CPU_MP_DATA {
>UINTN BackupBuffer;
>UINTN BackupBufferSize;
>
> - volatile UINT32StartCount;
>volatile UINT32FinishedCount;
> - volatile UINT32RunningCount;
> + UINT32 RunningCount;
>BOOLEANSingleThread;
>
Re: [edk2] [PATCH edk2-platforms v2 00/12] Switching to generic PciHostBridge driver
On 24 July 2018 at 08:32, Ming Huang wrote:
> The major features of this patchset include:
> 1 switching to generic PciHostBridge driver;
> 2 Change DEBUG log level;
>
> Code can also be found in github:
> https://github.com/hisilicon/OpenPlatformPkg.git
> branch: pcihostbridge-v2
>
>
> Heyi Guo (9):
> Hisilicon: Enable WARN and INFO debug message
> Hisilicon/D05/PlatformPciLib: fix misuse of macro
> Hisilicon/Pci: Move PciPlatform to common directory
> Hisilicon/PlatformPciLib: add segment for each root bridge
> Hisilicon: add PciHostBridgeLib
> Hisilicon: add PciSegmentLib for Hi161x
> Hisilicon/D0x: Switch to generic PciHostBridge driver
> Hisilicon: remove platform specific PciHostBridge
> Hisilicon/PlatformPciLib: clear redundant felds in RESOURCE_APPETURE
>
> Ming Huang (3):
> Hisilicon/Pci: Add two api for PciPlatform driver
> Hisilicon/Pci: move ATU configuration to PciPlatformLib
> Hisilicon/Pci: move EnlargeAtuConfig0() to PciPlatformLib
>
For the series
Reviewed-by: Ard Biesheuvel
Pushed as a34ea15dbf31..016d55843a01
> Platform/Hisilicon/D03/D03.dsc| 14 +-
> Platform/Hisilicon/D03/D03.fdf|5 +-
> .../D03/Drivers/PciPlatform/PciPlatform.h | 180 --
> .../Library/PlatformPciLib/PlatformPciLib.c | 24 +-
> Platform/Hisilicon/D05/D05.dsc| 14 +-
> Platform/Hisilicon/D05/D05.fdf|5 +-
> .../Library/PlatformPciLib/PlatformPciLib.c | 66 +-
> .../PciHostBridgeLib/PciHostBridgeLib.c | 304 +++
> .../PciHostBridgeLib/PciHostBridgeLib.inf | 51 +
> .../Drivers/PciHostBridgeDxe/PciHostBridge.c | 1659
> .../Drivers/PciHostBridgeDxe/PciHostBridge.h | 528
> .../PciHostBridgeDxe/PciHostBridgeDxe.inf | 74 -
> .../PciHostBridgeDxe/PciRootBridgeIo.c| 2405 -
> .../Drivers/PciPlatform/PciPlatform.c | 45 +
> .../Drivers/PciPlatform/PciPlatform.inf |1 +
> .../Hi161xPciPlatformLib.c| 384 +++
> .../Hi161xPciPlatformLib.inf | 42 +
> .../Hi161xPciSegmentLib.inf | 36 +
> .../Hi161xPciSegmentLib/PciSegmentLib.c | 1503 ++
> Silicon/Hisilicon/Hisilicon.dsc.inc |8 +-
> .../Include/Library/PlatformPciLib.h |3 +-
> 21 files changed, 2434 insertions(+), 4917 deletions(-)
> delete mode 100644 Platform/Hisilicon/D03/Drivers/PciPlatform/PciPlatform.h
> create mode 100644
> Platform/Hisilicon/Library/PciHostBridgeLib/PciHostBridgeLib.c
> create mode 100644
> Platform/Hisilicon/Library/PciHostBridgeLib/PciHostBridgeLib.inf
> delete mode 100644 Silicon/Hisilicon/Drivers/PciHostBridgeDxe/PciHostBridge.c
> delete mode 100644 Silicon/Hisilicon/Drivers/PciHostBridgeDxe/PciHostBridge.h
> delete mode 100644
> Silicon/Hisilicon/Drivers/PciHostBridgeDxe/PciHostBridgeDxe.inf
> delete mode 100644
> Silicon/Hisilicon/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c
> rename {Platform/Hisilicon/D03 =>
> Silicon/Hisilicon}/Drivers/PciPlatform/PciPlatform.c (89%)
> rename {Platform/Hisilicon/D03 =>
> Silicon/Hisilicon}/Drivers/PciPlatform/PciPlatform.inf (94%)
> create mode 100644
> Silicon/Hisilicon/Hi1610/Library/Hi161xPciPlatformLib/Hi161xPciPlatformLib.c
> create mode 100644
> Silicon/Hisilicon/Hi1610/Library/Hi161xPciPlatformLib/Hi161xPciPlatformLib.inf
> create mode 100644
> Silicon/Hisilicon/Hi1610/Library/Hi161xPciSegmentLib/Hi161xPciSegmentLib.inf
> create mode 100644
> Silicon/Hisilicon/Hi1610/Library/Hi161xPciSegmentLib/PciSegmentLib.c
>
> --
> 2.17.0
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch V2] UefiCpuPkg/MpInitLib: Remove redundant parameter.
Hi Laszlo, > -Original Message- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Wednesday, July 25, 2018 6:14 PM > To: Dong, Eric ; edk2-devel@lists.01.org > Cc: Ni, Ruiyu > Subject: Re: [edk2] [Patch V2] UefiCpuPkg/MpInitLib: Remove redundant > parameter. > > On 07/25/18 05:50, Dong, Eric wrote: > > Hi Laszlo, > > > > I have root cause this issue, the AP hangs in the procedure when > > PiSmmCpuDxeSmm driver start up trigged this issue. > > > > When PiSmmCpuDxeSmm driver start up, it will call StartAllAps to set > > memory attribute. In StartAllAps function, after call WakeUpAp to > > start Aps, it calls CheckAllAps to wait all Aps finished the task. In > > CheckAllAps function, it detect AP state to know whether the AP has > > finished its task. In old code, it check whether the AP state is > > CpuStateFinished to know whether AP has finished tasks. This state is > > only set by AP when it truly finished task. In new logic, > > CpuStateFinished been replace with CpuStateIdle. And CpuStateIdle is > > also the begin state of the AP. AP will change state from CpuStateIdle > > to CpuStateBusy when it start execute the procedure. And after it > > finished the procedure, it will change state back to CpuStateIdle. > > > > So when the hang issue raised, AP state is not been changed to > > CpuStateBusy when BSP calls CheckAllAps to check whether the AP has > > finished its task. So the state for the AP still in CpuStateIdle, but > > BSP think AP has finished its task. In this case, BSP think all the > > Aps has finished their tasks and it continues boot. > > Awesome analysis! > > So, this looks like an "inverse" variant of the classic "ABA problem": > > https://en.wikipedia.org/wiki/ABA_problem Yes, it's an ABA problem. New patch keep the CpuStateReady to distinguish from the CpuStateIdle state. > > > But some AP may wake up later and it failed to return from the > > procedure. > > Ah! So that explains another symptom I've since seen as well -- although > *very* rarely. Namely, if an AP wakes up *after* PiSmmCpuDxeSmm moves > on, thinking that all APs are finished, the AP can execute garbage in "no > man's > land" -- and that crashes the guest. Basically, QEMU/KVM pause the guest > with "emulation failure", and QEMU dumps the VCPU register state to the > standard error on the host side. In particular, the register state indicates > that > the crashed VCPU is *not* in SMM. Where to get these QEMU dumps info? I'm not clear who calls StartAllAPs function when I send this mail, I just based on the debug message found StartAllAps trigged right after PiSmmCpuDxeSmm driver. but I think this not blocks my patches for this issue. So I sent the patches and back to dig it more. Now I found it's PiSmmIpl driver calls gDS->SetMemorySpaceAttributes in SmmIplDxeDispatchEventNotify function which finally calls the StartAllAps. The Ap procedure is SetMtrrsFromBuffer function in CpuDxe driver. In SetMtrrsFromBuffer function, it just call MtrrSetAllMtrrs to update the MTRR, and it use local variable to save the MTRR settings. When the hang issue raised, in the time the AP begin to run the procedure, the BSP has leave current function. So the saved MTRR setting in the local variable is not valid anymore. So the MtrrSetAllMtrrs function use an invalid Mtrr Setttings to set mtrrs and never exit the procedure. Do you think it's reasonable? I found till the ExitBootService point, the AP still in busy state. I check the ApWakeupFunction function, found between the AP procedure and set AP state to Idle, no other possible code exist. So I think the AP should not return back from procedure. I try to add debug message to know where the AP is stopped at. But after I add debug message in MtrrSetAllMtrrs, the issue can't reproduce anymore. I tried about 30 times. Do you have other message to get to know where the AP is? Thanks, Eric > > When I first encountered this symptom now, while playing some more with > your patches, it reminded me of earlier problems with MpInitLib. And now > your analysis makes perfect sense of this additional symptom! > > > In this case, the AP state keeps at CpuStateBusy. So later in > > ChangeApLoopCallback function, because this AP state still in > > CpuStateBusy, this AP will not trig the procedure. But BSP wait all > > APs to trig the procedure(BSP wait the Aps to reduce the > > mNumberToFinish value in procedure to continue boot) to continue the > > boot, so the hang occurred. > > This completes the explanation. > > > I think we should keep a middle state to let us know whether the AP > > truly finished its task. I will send another serial patch for this > > issue. Please help to check the new patches. > > Yes, I'll test them too. > > Thanks! > Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch v3 1/3] UefiCpuPkg/MpInitLib: Remove redundant CpuStateFinished State.
Hi Laszlo,
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, July 25, 2018 6:55 PM
> To: Dong, Eric ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu
> Subject: Re: [edk2] [Patch v3 1/3] UefiCpuPkg/MpInitLib: Remove redundant
> CpuStateFinished State.
>
> Hi Eric,
>
> On 07/25/18 09:50, Eric Dong wrote:
> > Current CPU state definition include CpuStateIdle and CpuStateFinished.
> > After investigation, current code can use CpuStateIdle to replace the
> > CpuStateFinished. It will reduce the state number and easy for maintenance.
> >
> > Cc: Laszlo Ersek
> > Cc: Ruiyu Ni
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Dong
> > ---
> > UefiCpuPkg/Library/MpInitLib/MpLib.c | 18 --
> > UefiCpuPkg/Library/MpInitLib/MpLib.h | 1 -
> > 2 files changed, 8 insertions(+), 11 deletions(-)
>
> After looking over this patch, it seems that you are preserving the
> CpuStateReady enum constant, relative to:
>
> 20180628112920.5296-1-eric.dong@intel.com">http://mid.mail-archive.com/20180628112920.5296-1-eric.dong@intel.com
>
> However, based on your analysis in
>
> http://mid.mail-
> archive.com/ED077930C258884BBCB450DB737E66224AC5A453@shsmsx102.
> ccr.corp.intel.com
>
> isn't it still possible to run into the exact same issue? (Namely, BSP thinks
> the
> AP has gone through Idle -> Busy -> Idle, but the AP has never actually left
> Idle?)
>
> Hm, wait, is it the case that the BSP first sets Ready, and so if the check
> for an
> AP returns Idle, it implies the AP must have gone through:
>
> Idle > Ready > Busy > Idle
>
> ?
Correct! The Ready state is begin state and the Idle is the finish state.
>
> If this is correct, can you please include the following in the commit
> message:
>
> > Before this patch, the state transitions for an AP are:
> >
> > Idle > Ready > Busy > Finished > Idle
> >[BSP] [AP] [AP] [BSP]
> >
> > After the patch, the state transitions for an AP are:
> >
> > Idle > Ready > Busy > Idle
> >[BSP] [AP] [AP]
>
> Do you agree?
Good suggestion, I will include this info in the commit message.
>
> I have another question:
>
> On 07/25/18 09:50, Eric Dong wrote:
> > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > index c82b985943..ff09a0e9e7 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > @@ -696,7 +696,7 @@ ApWakeupFunction (
> > }
> >}
> > }
> > -SetApState (>CpuData[ProcessorNumber],
> CpuStateFinished);
> > +SetApState (>CpuData[ProcessorNumber],
> > + CpuStateIdle);
> >}
> > }
> >
> > @@ -1352,18 +1352,17 @@ CheckThisAP (
> >CpuData = >CpuData[ProcessorNumber];
> >
> >//
> > - // Check the CPU state of AP. If it is CpuStateFinished, then the AP has
> finished its task.
> > + // Check the CPU state of AP. If it is CpuStateIdle, then the AP has
> finished its task.
> >// Only BSP and corresponding AP access this unit of CPU Data.
> > This means the AP will not modify the
> > - // value of state after setting the it to CpuStateFinished, so BSP can
> > safely
> make use of its value.
> > + // value of state after setting the it to CpuStateIdle, so BSP can
> > safely
> make use of its value.
> >//
> >//
> >// If the AP finishes for StartupThisAP(), return EFI_SUCCESS.
> >//
> > - if (GetApState(CpuData) == CpuStateFinished) {
> > + if (GetApState(CpuData) == CpuStateIdle) {
> > if (CpuData->Finished != NULL) {
> >*(CpuData->Finished) = TRUE;
> > }
> > -SetApState (CpuData, CpuStateIdle);
> > return EFI_SUCCESS;
> >} else {
> > //
> > @@ -1420,14 +1419,13 @@ CheckAllAPs (
> >
> > CpuData = >CpuData[ProcessorNumber];
> > //
> > -// Check the CPU state of AP. If it is CpuStateFinished, then the AP
> > has
> finished its task.
> > +// Check the CPU state of AP. If it is CpuStateIdle, then the AP has
> finished its task.
> > // Only BSP and corresponding AP access this unit of CPU Data. This
> means the AP will not modify the
> > -// value of state after setting the it to CpuStateFinished, so BSP can
> safely make use of its value.
> > +// value of state after setting the it to CpuStateIdle, so BSP can
> > safely
> make use of its value.
> > //
> > -if (GetApState(CpuData) == CpuStateFinished) {
> > +if (GetApState(CpuData) == CpuStateIdle) {
> >CpuMpData->RunningCount ++;
> >CpuMpData->CpuData[ProcessorNumber].Waiting = FALSE;
> > - SetApState(CpuData, CpuStateIdle);
> >
> >//
> >// If in Single Thread mode, then search for the next waiting AP for
> execution.
>
> This part of the code, after the patch, does not seem idempotent; in other
> words, if the BSP calls
Re: [edk2] [PATCH edk2-non-osi v1 0/1] Fix invoke SetMemorySpaceAttributes error bug
On 4 July 2018 at 09:49, Ming Huang wrote: > Code can also be found in github: > https://github.com/hisilicon/OpenPlatformPkg.git > branch: edk2-non-osi-20180627-v1 > > Ming Huang (1): > Hisilicon/D0x: Fix invoke SetMemorySpaceAttributes error bug > Reviewed-by: Ard Biesheuvel Pushed to edk2-non-osi as 2c4d662506bd..a34ea15dbf31 > Platform/Hisilicon/D05/Drivers/SFC/SFCDriver.depex | Bin 2 -> 36 bytes > 1 file changed, 0 insertions(+), 0 deletions(-) > > -- > 2.17.0 > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms v3 0/6] Improve D0x platforms and bug fix
On 13 July 2018 at 10:15, Ming Huang wrote: > The major features of this patchset include: > 1 Fix invoke SetMemorySpaceAttributes error bug > 2 Correct ATU Cfg0/Cfg1 base address > 3 Fix SetAtuConfig1RW bug > 4 Add PlatformMiscDxe driver > 5 optimize two pcie prots space > 6 Correct smbios product name > > BTW: > 1 D06 source will upstream in July; > 2 Installing OS by iso is supported by edk2 commit(824b6e3b5f). > > Code can also be found in github: > https://github.com/hisilicon/OpenPlatformPkg.git > branch: platforms-20180627-v3 > > > Jason Zhang (1): > Hisilicon/D03/D05: Correct ATU Cfg0/Cfg1 base address > > Ming Huang (5): > Hisilicon/D0x: Fix invoke SetMemorySpaceAttributes error bug > Hisilicon/D0x: Fix SetAtuConfig1RW bug > Hisilicon/D05: Add PlatformMiscDxe driver > Hisilicon/D05/Pcie: optimize two pcie ports space > Hisilicon/D0x: Correct smbios product name > Patches 1-4 and 6 Reviewed-by: Ard Biesheuvel Pushed as 2c4d662506bd..a34ea15dbf31 For the legacy INTx issue, I would like to gain a better understanding first of why this issue is particular to D0x. > .../DS3231RealTimeClockLib.inf| 2 + > Platform/Hisilicon/D05/D05.dsc| 13 +-- > Platform/Hisilicon/D05/D05.fdf| 1 + > .../Drivers/PlatformMiscDxe/PlatformMiscDxe.c | 99 +++ > .../PlatformMiscDxe/PlatformMiscDxe.inf | 47 + > .../Library/PlatformPciLib/PlatformPciLib.c | 8 +- > .../PciHostBridgeDxe/PciRootBridgeIo.c| 13 +-- > .../Type01/MiscSystemManufacturerFunction.c | 1 - > .../Hi1616/D05AcpiTables/D05Iort.asl | 8 +- > .../Hi1616/D05AcpiTables/D05Mcfg.aslc | 8 +- > .../Hi1616/D05AcpiTables/Dsdt/D05Pci.asl | 32 +++--- > 11 files changed, 191 insertions(+), 41 deletions(-) > create mode 100644 > Platform/Hisilicon/D05/Drivers/PlatformMiscDxe/PlatformMiscDxe.c > create mode 100644 > Platform/Hisilicon/D05/Drivers/PlatformMiscDxe/PlatformMiscDxe.inf > > -- > 2.17.0 > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch v3 1/3] UefiCpuPkg/MpInitLib: Remove redundant CpuStateFinished State.
Hi Eric,
On 07/25/18 09:50, Eric Dong wrote:
> Current CPU state definition include CpuStateIdle and CpuStateFinished.
> After investigation, current code can use CpuStateIdle to replace the
> CpuStateFinished. It will reduce the state number and easy for maintenance.
>
> Cc: Laszlo Ersek
> Cc: Ruiyu Ni
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong
> ---
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 18 --
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 1 -
> 2 files changed, 8 insertions(+), 11 deletions(-)
After looking over this patch, it seems that you are preserving the
CpuStateReady enum constant, relative to:
20180628112920.5296-1-eric.dong@intel.com">http://mid.mail-archive.com/20180628112920.5296-1-eric.dong@intel.com
However, based on your analysis in
ED077930C258884BBCB450DB737E66224AC5A453@shsmsx102.ccr.corp.intel.com">http://mid.mail-archive.com/ED077930C258884BBCB450DB737E66224AC5A453@shsmsx102.ccr.corp.intel.com
isn't it still possible to run into the exact same issue? (Namely, BSP
thinks the AP has gone through Idle -> Busy -> Idle, but the AP has
never actually left Idle?)
Hm, wait, is it the case that the BSP first sets Ready, and so if the
check for an AP returns Idle, it implies the AP must have gone through:
Idle > Ready > Busy > Idle
?
If this is correct, can you please include the following in the commit
message:
> Before this patch, the state transitions for an AP are:
>
> Idle > Ready > Busy > Finished > Idle
>[BSP] [AP] [AP] [BSP]
>
> After the patch, the state transitions for an AP are:
>
> Idle > Ready > Busy > Idle
>[BSP] [AP] [AP]
Do you agree?
I have another question:
On 07/25/18 09:50, Eric Dong wrote:
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index c82b985943..ff09a0e9e7 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -696,7 +696,7 @@ ApWakeupFunction (
> }
>}
> }
> -SetApState (>CpuData[ProcessorNumber], CpuStateFinished);
> +SetApState (>CpuData[ProcessorNumber], CpuStateIdle);
>}
> }
>
> @@ -1352,18 +1352,17 @@ CheckThisAP (
>CpuData = >CpuData[ProcessorNumber];
>
>//
> - // Check the CPU state of AP. If it is CpuStateFinished, then the AP has
> finished its task.
> + // Check the CPU state of AP. If it is CpuStateIdle, then the AP has
> finished its task.
>// Only BSP and corresponding AP access this unit of CPU Data. This means
> the AP will not modify the
> - // value of state after setting the it to CpuStateFinished, so BSP can
> safely make use of its value.
> + // value of state after setting the it to CpuStateIdle, so BSP can safely
> make use of its value.
>//
>//
>// If the AP finishes for StartupThisAP(), return EFI_SUCCESS.
>//
> - if (GetApState(CpuData) == CpuStateFinished) {
> + if (GetApState(CpuData) == CpuStateIdle) {
> if (CpuData->Finished != NULL) {
>*(CpuData->Finished) = TRUE;
> }
> -SetApState (CpuData, CpuStateIdle);
> return EFI_SUCCESS;
>} else {
> //
> @@ -1420,14 +1419,13 @@ CheckAllAPs (
>
> CpuData = >CpuData[ProcessorNumber];
> //
> -// Check the CPU state of AP. If it is CpuStateFinished, then the AP has
> finished its task.
> +// Check the CPU state of AP. If it is CpuStateIdle, then the AP has
> finished its task.
> // Only BSP and corresponding AP access this unit of CPU Data. This
> means the AP will not modify the
> -// value of state after setting the it to CpuStateFinished, so BSP can
> safely make use of its value.
> +// value of state after setting the it to CpuStateIdle, so BSP can
> safely make use of its value.
> //
> -if (GetApState(CpuData) == CpuStateFinished) {
> +if (GetApState(CpuData) == CpuStateIdle) {
>CpuMpData->RunningCount ++;
>CpuMpData->CpuData[ProcessorNumber].Waiting = FALSE;
> - SetApState(CpuData, CpuStateIdle);
>
>//
>// If in Single Thread mode, then search for the next waiting AP for
> execution.
This part of the code, after the patch, does not seem idempotent; in
other words, if the BSP calls CheckAllAPs() multiple times, then
RunningCount will be increased every time. Before the patch, this wasn't
the case, because after the Finished -> Idle transition, the increment
wouldn't be reached again.
Hmmm, wait, I'm wrong: we set the Waiting field to FALSE as well, so at
the next call to CheckAllAPs(), we'll take the early "continue" branch.
Looks OK after all.
I'll follow up with test results.
Thanks,
Laszlo
> @@ -1923,7 +1921,7 @@ SwitchBSPWorker (
>//
>// Wait for old BSP finished AP task
>//
> - while (GetApState (>CpuData[CallerNumber]) != CpuStateFinished)
> {
> + while (GetApState
Re: [edk2] [PATCH edk2-platforms v3 4/6] Hisilicon/D05: Add PlatformMiscDxe driver
On 13 July 2018 at 10:15, Ming Huang wrote:
> Fix the issue of onboard Nic not work kerenl with AMD GPU and
> NVME SSD in board. The GPU don't support 64 MSI, so need to
> allocate INTx, but the default interrupt number 255 is invalid,
> so Change all the PCI Device interrupt number to 0.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang
> Signed-off-by: Heyi Guo
I don't understand why this issue is specific to this platform.
Can you explain in more detail what the failure mode is, and why
setting the PCI interrupt line is necessary here, while it doesn't
seem to be on other platforms, even when falling back to INTx
interrupts?
> ---
> Platform/Hisilicon/D05/D05.dsc | 1 +
> Platform/Hisilicon/D05/D05.fdf | 1 +
> Platform/Hisilicon/D05/Drivers/PlatformMiscDxe/PlatformMiscDxe.c | 99
>
> Platform/Hisilicon/D05/Drivers/PlatformMiscDxe/PlatformMiscDxe.inf | 47
> ++
> 4 files changed, 148 insertions(+)
>
> diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
> index b6e1a9d98a..0e6d5912a0 100644
> --- a/Platform/Hisilicon/D05/D05.dsc
> +++ b/Platform/Hisilicon/D05/D05.dsc
> @@ -629,6 +629,7 @@
>
>
>
> Silicon/Hisilicon/Drivers/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf
> + Platform/Hisilicon/D05/Drivers/PlatformMiscDxe/PlatformMiscDxe.inf
>
>#
># Memory test
> diff --git a/Platform/Hisilicon/D05/D05.fdf b/Platform/Hisilicon/D05/D05.fdf
> index 4503776d63..61e8d907f9 100644
> --- a/Platform/Hisilicon/D05/D05.fdf
> +++ b/Platform/Hisilicon/D05/D05.fdf
> @@ -354,6 +354,7 @@ READ_LOCK_STATUS = TRUE
>INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
>INF IntelFrameworkModulePkg/Universal/StatusCode/Pei/StatusCodePei.inf
>INF Platform/Hisilicon/D05/EarlyConfigPeim/EarlyConfigPeimD05.inf
> + INF Platform/Hisilicon/D05/Drivers/PlatformMiscDxe/PlatformMiscDxe.inf
>
>INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>
> diff --git a/Platform/Hisilicon/D05/Drivers/PlatformMiscDxe/PlatformMiscDxe.c
> b/Platform/Hisilicon/D05/Drivers/PlatformMiscDxe/PlatformMiscDxe.c
> new file mode 100644
> index 00..8519b7139d
> --- /dev/null
> +++ b/Platform/Hisilicon/D05/Drivers/PlatformMiscDxe/PlatformMiscDxe.c
> @@ -0,0 +1,99 @@
> +/** @file
> +*
> +* Copyright (c) 2018, Hisilicon Limited. All rights reserved.
> +* Copyright (c) 2016, Linaro Limited. All rights reserved.
> +*
> +* This program and the accompanying materials
> +* are licensed and made available under the terms and conditions of the BSD
> License
> +* which accompanies this distribution. The full text of the license may be
> found at
> +* http://opensource.org/licenses/bsd-license.php
> +*
> +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> IMPLIED.
> +*
> +**/
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +VOID
> +SetIntLine (
> + )
> +{
> + EFI_STATUS Status;
> + UINTN HandleIndex;
> + EFI_HANDLE *HandleBuffer;
> + UINTN HandleCount;
> + EFI_PCI_IO_PROTOCOL*PciIo;
> + UINT8 INTLine;
> + UINTN Segment;
> + UINTN Bus;
> + UINTN Device;
> + UINTN Fun;
> +
> + Status = gBS->LocateHandleBuffer (
> + ByProtocol,
> + ,
> + NULL,
> + ,
> +
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, " Locate gEfiPciIoProtocol Failed.\n"));
> + gBS->FreePool ((VOID *)HandleBuffer);
> + return;
> + }
> +
> + for (HandleIndex = 0; HandleIndex < HandleCount; HandleIndex++) {
> + Status = gBS->HandleProtocol (
> + HandleBuffer[HandleIndex],
> + ,
> + (VOID **)
> + );
> + if (EFI_ERROR (Status)) {
> + continue;
> + }
> +
> + INTLine = 0;
> + (VOID)PciIo->Pci.Write (
> + PciIo,
> + EfiPciIoWidthUint8,
> + PCI_INT_LINE_OFFSET,
> + 1,
> + );
> + (VOID)PciIo->GetLocation (PciIo, , , , );
> + DEBUG ((DEBUG_INFO, "Set BDF(%x-%x-%x) IntLine to 0\n", Bus, Device,
> Fun));
> + }
> +
> + gBS->FreePool ((VOID *)HandleBuffer);
> + return;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +PlatformMiscDxeEntry (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + EFI_STATUS Status;
> + EFI_EVENT Event;
> +
> + Status = gBS->CreateEventEx (
> +
Re: [edk2] [platforms: PATCH 2/6] Marvell/Library: ComPhyLib: Configure PCIE in ARM-TF
2018-07-25 12:01 GMT+02:00 Ard Biesheuvel :
> On 13 July 2018 at 16:09, Marcin Wojtas wrote:
>> From: Grzegorz Jaszczyk
>>
>> Replace the ComPhy initialization for PCIE with appropriate SMC call,
>> so the firmware will execute required serdes configuration.
>>
>
> 'the firmware' ?
Will change to 'ARMT-TF' then.
Best regards,
Marcin
>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas
>> ---
>>
>> Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.h
>> | 22 -
>> Silicon/Marvell/Include/Library/SampleAtResetLib.h
>>| 7 -
>> Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h
>>| 3 +-
>>
>> Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.c
>> | 19 -
>> Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c
>>| 491 +---
>> 5 files changed, 8 insertions(+), 534 deletions(-)
>>
>> diff --git
>> a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.h
>>
>> b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.h
>> index 323399f..e47396d 100644
>> ---
>> a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.h
>> +++
>> b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.h
>> @@ -44,18 +44,6 @@ SAR - Sample At Reset
>>
>> #define CP110_SAR_BASE(_CpIndex) (0xf200 + (0x200 * (_CpIndex)) +
>> 0x400200)
>>
>> -#define MAX_CP_COUNT 2
>> -#define MAX_PCIE_CLK_TYPE_COUNT 2
>> -
>> -#define CP0_PCIE0_CLK_OFFSET 2
>> -#define CP0_PCIE1_CLK_OFFSET 3
>> -#define CP1_PCIE0_CLK_OFFSET 0
>> -#define CP1_PCIE1_CLK_OFFSET 1
>> -#define CP0_PCIE0_CLK_MASK(1 << CP0_PCIE0_CLK_OFFSET)
>> -#define CP0_PCIE1_CLK_MASK(1 << CP0_PCIE1_CLK_OFFSET)
>> -#define CP1_PCIE0_CLK_MASK(1 << CP1_PCIE0_CLK_OFFSET)
>> -#define CP1_PCIE1_CLK_MASK(1 << CP1_PCIE1_CLK_OFFSET)
>> -
>> typedef enum {
>>CPU_2000_DDR_1200_RCLK_1200 = 0x0,
>>CPU_2000_DDR_1050_RCLK_1050 = 0x1,
>> @@ -97,13 +85,3 @@ STATIC CONST PLL_FREQUENCY_DESCRIPTION
>> PllFrequencyTable[SAR_MAX_OPTIONS] = {
>>{800 , 800 , 800 , CPU_800_DDR_800_RCLK_800},
>>{1000, 800 , 800 , CPU_1000_DDR_800_RCLK_800}
>> };
>> -
>> -STATIC CONST UINT32 PcieClockMask[MAX_CP_COUNT][MAX_PCIE_CLK_TYPE_COUNT] = {
>> - {CP0_PCIE0_CLK_MASK, CP0_PCIE1_CLK_MASK},
>> - {CP1_PCIE0_CLK_MASK, CP1_PCIE1_CLK_MASK}
>> -};
>> -
>> -STATIC CONST UINT32 PcieClockOffset[MAX_CP_COUNT][MAX_PCIE_CLK_TYPE_COUNT]
>> = {
>> - {CP0_PCIE0_CLK_OFFSET, CP0_PCIE1_CLK_OFFSET},
>> - {CP1_PCIE0_CLK_OFFSET, CP1_PCIE1_CLK_OFFSET}
>> -};
>> diff --git a/Silicon/Marvell/Include/Library/SampleAtResetLib.h
>> b/Silicon/Marvell/Include/Library/SampleAtResetLib.h
>> index 1be3a6a..1e7b27c 100644
>> --- a/Silicon/Marvell/Include/Library/SampleAtResetLib.h
>> +++ b/Silicon/Marvell/Include/Library/SampleAtResetLib.h
>> @@ -47,11 +47,4 @@ SampleAtResetGetDramFrequency (
>>VOID
>>);
>>
>> -UINT32
>> -EFIAPI
>> -SampleAtResetGetPcieClockDirection (
>> - IN UINT32CpIndex,
>> - IN UINT32PcieIndex
>> - );
>> -
>> #endif /* __SAMPLE_AT_RESET_LIB_H__ */
>> diff --git a/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h
>> b/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h
>> index 34c1e9b..20a9767 100644
>> --- a/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h
>> +++ b/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h
>> @@ -125,7 +125,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
>> DAMAGE.
>>
>> #define COMPHY_FW_FORMAT(mode, idx, speeds) \
>> ((mode << 12) | (idx << 8) | (speeds <<
>> 2))
>> -
>> +#define COMPHY_FW_PCIE_FORMAT(pcie_width, mode, speeds) \
>> + ((pcie_width << 18) | COMPHY_FW_FORMAT (mode, 0,
>> speeds))
>>
>> #define COMPHY_POLARITY_NO_INVERT0
>> #define COMPHY_POLARITY_TXD_INVERT 1
>> diff --git
>> a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.c
>>
>> b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.c
>> index 3ebff56..5a9a5f9 100644
>> ---
>> a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.c
>> +++
>> b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.c
>> @@ -90,22 +90,3 @@ SampleAtResetGetDramFrequency (
>>
>>return PllFrequencies->DdrFrequency;
>> }
>> -
>> -UINT32
>> -EFIAPI
>> -SampleAtResetGetPcieClockDirection (
>> - IN UINT32 CpIndex,
>> - IN UINT32 PcieIndex
>> - )
>> -{
>> - UINT32 ClockDirection;
>> -
>> - ASSERT (CpIndex < MAX_CP_COUNT);
>> - ASSERT (PcieIndex <
[edk2] [staging/FmpDevicePkg-master][PATCH] FmpDevicePkg FmpDxe: Check Progress!= NULL before calling Progress(100)
Cc: Michael D Kinney
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng
---
FmpDevicePkg/FmpDxe/FmpDxe.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
index 091f950b95b2..f0e8b0da823a 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -1121,10 +1121,12 @@ cleanup:
mProgressSupported = FALSE;
SetLastAttemptStatusInVariable (LastAttemptStatus);
- //
- // Set progress to 100 after everything is done including recording Status.
- //
- Progress (100);
+ if (Progress != NULL) {
+//
+// Set progress to 100 after everything is done including recording Status.
+//
+Progress (100);
+ }
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] [staging/FmpDevicePkg-master][PATCH] FmpDevicePkg: Add DISABLE_NEW_DEPRECATED_INTERFACES build options
Add DISABLE_NEW_DEPRECATED_INTERFACES build options to make sure no deprecated interface used in this package. Cc: Michael D Kinney Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng --- FmpDevicePkg/FmpDevicePkg.dsc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/FmpDevicePkg/FmpDevicePkg.dsc b/FmpDevicePkg/FmpDevicePkg.dsc index a4eac1e60de0..525640de2e1b 100644 --- a/FmpDevicePkg/FmpDevicePkg.dsc +++ b/FmpDevicePkg/FmpDevicePkg.dsc @@ -132,3 +132,6 @@ [Components] DisplayUpdateProgressLib|MdeModulePkg/Library/DisplayUpdateProgressLibText/DisplayUpdateProgressLibText.inf } + +[BuildOptions] + *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES -- 2.7.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [staging/FmpDevicePkg-master][PATCH] FmpDevicePkg FmpDxe: Add EFI_ABORTED in retval of CheckTheImage()
Cc: Michael D Kinney Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng --- FmpDevicePkg/FmpDxe/FmpDxe.c | 1 + 1 file changed, 1 insertion(+) diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c index f0e8b0da823a..69b6cb7d4c0d 100644 --- a/FmpDevicePkg/FmpDxe/FmpDxe.c +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c @@ -626,6 +626,7 @@ GetAllHeaderSize ( if available, additional information if the image is invalid. @retval EFI_SUCCESSThe image was successfully checked. + @retval EFI_ABORTEDThe operation is aborted. @retval EFI_INVALID_PARAMETER The Image was NULL. @retval EFI_UNSUPPORTEDThe operation is not supported. @retval EFI_SECURITY_VIOLATIO The operation could not be performed due to an authentication failure. -- 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 V2] UefiCpuPkg/MpInitLib: Remove redundant parameter.
On 07/25/18 05:50, Dong, Eric wrote: > Hi Laszlo, > > I have root cause this issue, the AP hangs in the procedure when > PiSmmCpuDxeSmm driver start up trigged this issue. > > When PiSmmCpuDxeSmm driver start up, it will call StartAllAps to set > memory attribute. In StartAllAps function, after call WakeUpAp to > start Aps, it calls CheckAllAps to wait all Aps finished the task. In > CheckAllAps function, it detect AP state to know whether the AP has > finished its task. In old code, it check whether the AP state is > CpuStateFinished to know whether AP has finished tasks. This state is > only set by AP when it truly finished task. In new logic, > CpuStateFinished been replace with CpuStateIdle. And CpuStateIdle is > also the begin state of the AP. AP will change state from CpuStateIdle > to CpuStateBusy when it start execute the procedure. And after it > finished the procedure, it will change state back to CpuStateIdle. > > So when the hang issue raised, AP state is not been changed to > CpuStateBusy when BSP calls CheckAllAps to check whether the AP has > finished its task. So the state for the AP still in CpuStateIdle, but > BSP think AP has finished its task. In this case, BSP think all the > Aps has finished their tasks and it continues boot. Awesome analysis! So, this looks like an "inverse" variant of the classic "ABA problem": https://en.wikipedia.org/wiki/ABA_problem > But some AP may wake up later and it failed to return from the > procedure. Ah! So that explains another symptom I've since seen as well -- although *very* rarely. Namely, if an AP wakes up *after* PiSmmCpuDxeSmm moves on, thinking that all APs are finished, the AP can execute garbage in "no man's land" -- and that crashes the guest. Basically, QEMU/KVM pause the guest with "emulation failure", and QEMU dumps the VCPU register state to the standard error on the host side. In particular, the register state indicates that the crashed VCPU is *not* in SMM. When I first encountered this symptom now, while playing some more with your patches, it reminded me of earlier problems with MpInitLib. And now your analysis makes perfect sense of this additional symptom! > In this case, the AP state keeps at CpuStateBusy. So later in > ChangeApLoopCallback function, because this AP state still in > CpuStateBusy, this AP will not trig the procedure. But BSP wait all > APs to trig the procedure(BSP wait the Aps to reduce the > mNumberToFinish value in procedure to continue boot) to continue the > boot, so the hang occurred. This completes the explanation. > I think we should keep a middle state to let us know whether the AP > truly finished its task. I will send another serial patch for this > issue. Please help to check the new patches. Yes, I'll test them too. Thanks! Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms v2 1/1] Silicon/SynQuacer: add optional OP-TEE DT node
On 23 July 2018 at 15:19, Sumit Garg wrote:
> OP-TEE is optional on Developerbox controlled via SCP firmware. To check
> if we need to delete OP-TEE DT node, we use DRAM1 region info as SCP
> firmware conditionally carves out Secure memory from DRAM1 region.
>
> Cc: Ard Biesheuvel
> Cc: Leif Lindholm
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sumit Garg
> ---
>
As discussed on IRC, i am not a fan of inferring the presence of
OP-TEE from the base/size values of the first DRAM region.
Please refer to the existing PCIe code how to read a GPIO in PEI and
set a dynamic PCD accordingly, so you can use its value in
PlatformDxe.
> Changes since v1:
> - Add support for optional OP-TEE DT node addition
>
>
> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> | 3 ++
>
> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
>| 33
> Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
>| 7 +
> 3 files changed, 43 insertions(+)
>
> diff --git
> a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
>
> b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> index 548d62fd5c0a..46cd3f85e509 100644
> ---
> a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> +++
> b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> @@ -35,6 +35,9 @@ [LibraryClasses]
>FdtLib
>MemoryAllocationLib
>
> +[FixedPcd]
> + gSynQuacerTokenSpaceGuid.PcdDramInfoBase
> +
> [Pcd]
>gSynQuacerTokenSpaceGuid.PcdPcieEnableMask
>gSynQuacerTokenSpaceGuid.PcdPlatformSettings
> diff --git
> a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
>
> b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
> index 897d06743708..da1209b4a613 100644
> ---
> a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
> +++
> b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
> @@ -19,10 +19,13 @@
> #include
> #include
> #include
> +#include
> #include
>
> // add enough space for three instances of 'status = "disabled"'
> #define DTB_PADDING 64
> +// base address for OP-TEE used to determine its presence
> +#define OPTEE_BASE_ADDR 0xFC00
>
> STATIC
> VOID
> @@ -47,6 +50,29 @@ DisableDtNode (
>}
> }
>
> +STATIC
> +VOID
> +DeleteDtNode (
> + IN VOID*Dtb,
> + IN CONST CHAR8 *NodePath
> + )
> +{
> + INT32 Node;
> + INT32 Rc;
> +
> + Node = fdt_path_offset (Dtb, NodePath);
> + if (Node < 0) {
> +DEBUG ((DEBUG_ERROR, "%a: failed to locate DT path '%a': %a\n",
> + __FUNCTION__, NodePath, fdt_strerror (Node)));
> +return;
> + }
> + Rc = fdt_del_node (Dtb, Node);
> + if (Rc < 0) {
> +DEBUG ((DEBUG_ERROR, "%a: failed to delete node on '%a': %a\n",
> + __FUNCTION__, NodePath, fdt_strerror (Rc)));
> + }
> +}
> +
> /**
>Return a pool allocated copy of the DTB image that is appropriate for
>booting the current platform via DT.
> @@ -73,6 +99,7 @@ DtPlatformLoadDtb (
>UINTN CopyDtbSize;
>INT32 Rc;
>UINT64SettingsVal;
> + DRAM_INFO *DramInfo;
>SYNQUACER_PLATFORM_VARSTORE_DATA *Settings;
>
>Status = GetSectionFromAnyFv (,
> @@ -107,6 +134,12 @@ DtPlatformLoadDtb (
> DisableDtNode (CopyDtb, "/sdhci@5230");
>}
>
> + DramInfo = (VOID *)(UINTN)FixedPcdGet64 (PcdDramInfoBase);
> +
> + if ((DramInfo->Entry[0].Base + DramInfo->Entry[0].Size) > OPTEE_BASE_ADDR)
> {
> +DeleteDtNode (CopyDtb, "/firmware/optee");
> + }
> +
>*Dtb = CopyDtb;
>*DtbSize = CopyDtbSize;
>
> diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> index 37d642e4b237..d109a5742793 100644
> --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> @@ -574,6 +574,13 @@
> #address-cells = <1>;
> #size-cells = <0>;
> };
> +
> +firmware {
> +optee {
> +compatible = "linaro,optee-tz";
> +method = "smc";
> +};
> +};
> };
>
> #include "SynQuacerCaches.dtsi"
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 0/6] Armada7k8k ComPhy rework
On 13 July 2018 at 16:09, Marcin Wojtas wrote: > Hi, > > This patchset makes use of the feature present in newest, > publicly available ARM-TF of the Armada7k8k SoCs > (https://github.com/MarvellEmbeddedProcessors/atf-marvell/commits/atf-v1.4-armada-18.06) > which is a common configuration of the serdes lanes. > > It is triggered by the users (UEFI/U-Boot/OS) using > the dedicated smc calls. Thanks to that the library > could have been cleaned-up and reduced to the per-board > configuration parsing part and calling the according > sequences to be done in EL3. > > The patches are available in the github: > https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/comphy-upstream-r20180713 > > I'm looking forward to review and any comments/remarks. > > Best regards, > Marcin > > Grzegorz Jaszczyk (6): > Marvell/Library: ComPhyLib: Configure SATA, SGMII and SFI in ARM-TF > Marvell/Library: ComPhyLib: Configure PCIE in ARM-TF > Marvell/Library: ComPhyLib: Configure RXAUI in ARM-TF > Marvell/Library: ComPhyLib: Configure USB in ARM-TF > Marvell/Library: ComPhyLib: Clean up the library after rework > Marvell/Library: ComPhyLib: Remove both PHY and PIPE selector config > This looks fine except for the remarks I made in reply to patches #1 and #2 > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > | 18 +- > Silicon/Marvell/Library/ComPhyLib/ComPhyLib.inf > |2 +- > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.h > | 22 - > Silicon/Marvell/Include/Library/SampleAtResetLib.h > |7 - > Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h > | 545 +- > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.c > | 19 - > Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c > | 1777 +--- > Silicon/Marvell/Library/ComPhyLib/ComPhyLib.c > | 41 +- > Silicon/Marvell/Library/ComPhyLib/ComPhyMux.c > | 132 -- > 9 files changed, 116 insertions(+), 2447 deletions(-) > delete mode 100644 Silicon/Marvell/Library/ComPhyLib/ComPhyMux.c > > -- > 2.7.4 > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 2/6] Marvell/Library: ComPhyLib: Configure PCIE in ARM-TF
On 13 July 2018 at 16:09, Marcin Wojtas wrote:
> From: Grzegorz Jaszczyk
>
> Replace the ComPhy initialization for PCIE with appropriate SMC call,
> so the firmware will execute required serdes configuration.
>
'the firmware' ?
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas
> ---
>
> Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.h
> | 22 -
> Silicon/Marvell/Include/Library/SampleAtResetLib.h
> | 7 -
> Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h
> | 3 +-
>
> Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.c
> | 19 -
> Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c
> | 491 +---
> 5 files changed, 8 insertions(+), 534 deletions(-)
>
> diff --git
> a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.h
>
> b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.h
> index 323399f..e47396d 100644
> ---
> a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.h
> +++
> b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.h
> @@ -44,18 +44,6 @@ SAR - Sample At Reset
>
> #define CP110_SAR_BASE(_CpIndex) (0xf200 + (0x200 * (_CpIndex)) +
> 0x400200)
>
> -#define MAX_CP_COUNT 2
> -#define MAX_PCIE_CLK_TYPE_COUNT 2
> -
> -#define CP0_PCIE0_CLK_OFFSET 2
> -#define CP0_PCIE1_CLK_OFFSET 3
> -#define CP1_PCIE0_CLK_OFFSET 0
> -#define CP1_PCIE1_CLK_OFFSET 1
> -#define CP0_PCIE0_CLK_MASK(1 << CP0_PCIE0_CLK_OFFSET)
> -#define CP0_PCIE1_CLK_MASK(1 << CP0_PCIE1_CLK_OFFSET)
> -#define CP1_PCIE0_CLK_MASK(1 << CP1_PCIE0_CLK_OFFSET)
> -#define CP1_PCIE1_CLK_MASK(1 << CP1_PCIE1_CLK_OFFSET)
> -
> typedef enum {
>CPU_2000_DDR_1200_RCLK_1200 = 0x0,
>CPU_2000_DDR_1050_RCLK_1050 = 0x1,
> @@ -97,13 +85,3 @@ STATIC CONST PLL_FREQUENCY_DESCRIPTION
> PllFrequencyTable[SAR_MAX_OPTIONS] = {
>{800 , 800 , 800 , CPU_800_DDR_800_RCLK_800},
>{1000, 800 , 800 , CPU_1000_DDR_800_RCLK_800}
> };
> -
> -STATIC CONST UINT32 PcieClockMask[MAX_CP_COUNT][MAX_PCIE_CLK_TYPE_COUNT] = {
> - {CP0_PCIE0_CLK_MASK, CP0_PCIE1_CLK_MASK},
> - {CP1_PCIE0_CLK_MASK, CP1_PCIE1_CLK_MASK}
> -};
> -
> -STATIC CONST UINT32 PcieClockOffset[MAX_CP_COUNT][MAX_PCIE_CLK_TYPE_COUNT] =
> {
> - {CP0_PCIE0_CLK_OFFSET, CP0_PCIE1_CLK_OFFSET},
> - {CP1_PCIE0_CLK_OFFSET, CP1_PCIE1_CLK_OFFSET}
> -};
> diff --git a/Silicon/Marvell/Include/Library/SampleAtResetLib.h
> b/Silicon/Marvell/Include/Library/SampleAtResetLib.h
> index 1be3a6a..1e7b27c 100644
> --- a/Silicon/Marvell/Include/Library/SampleAtResetLib.h
> +++ b/Silicon/Marvell/Include/Library/SampleAtResetLib.h
> @@ -47,11 +47,4 @@ SampleAtResetGetDramFrequency (
>VOID
>);
>
> -UINT32
> -EFIAPI
> -SampleAtResetGetPcieClockDirection (
> - IN UINT32CpIndex,
> - IN UINT32PcieIndex
> - );
> -
> #endif /* __SAMPLE_AT_RESET_LIB_H__ */
> diff --git a/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h
> b/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h
> index 34c1e9b..20a9767 100644
> --- a/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h
> +++ b/Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h
> @@ -125,7 +125,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
>
> #define COMPHY_FW_FORMAT(mode, idx, speeds) \
> ((mode << 12) | (idx << 8) | (speeds <<
> 2))
> -
> +#define COMPHY_FW_PCIE_FORMAT(pcie_width, mode, speeds) \
> + ((pcie_width << 18) | COMPHY_FW_FORMAT (mode, 0,
> speeds))
>
> #define COMPHY_POLARITY_NO_INVERT0
> #define COMPHY_POLARITY_TXD_INVERT 1
> diff --git
> a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.c
>
> b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.c
> index 3ebff56..5a9a5f9 100644
> ---
> a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.c
> +++
> b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.c
> @@ -90,22 +90,3 @@ SampleAtResetGetDramFrequency (
>
>return PllFrequencies->DdrFrequency;
> }
> -
> -UINT32
> -EFIAPI
> -SampleAtResetGetPcieClockDirection (
> - IN UINT32 CpIndex,
> - IN UINT32 PcieIndex
> - )
> -{
> - UINT32 ClockDirection;
> -
> - ASSERT (CpIndex < MAX_CP_COUNT);
> - ASSERT (PcieIndex < MAX_PCIE_CLK_TYPE_COUNT);
> -
> - ClockDirection = MmioAnd32 (CP110_SAR_BASE (CpIndex),
> - PcieClockMask[CpIndex][PcieIndex] >>
> - PcieClockOffset[CpIndex][PcieIndex]);
> -
> - return ClockDirection;
> -}
> diff
Re: [edk2] [platforms: PATCH v2 0/6] Armada7k8k ICU support
2018-07-25 11:33 GMT+02:00 Ard Biesheuvel :
> On 13 July 2018 at 10:12, Marcin Wojtas wrote:
>> Hi,
>>
>> The second version of the ICU patchset brings all corrections
>> according to all review remarks. They were mostly style / naming
>> - detailed list can be found in the changelog below.
>>
>> The patches are available in the github:
>> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/icu-upstream-r20180713
>>
>> I'm looking forward to review and any comments/remarks.
>>
>> Best regards,
>> Marcin
>>
>> Changelog
>> v1 -> v2
>> * 1,6
>> - Add Ard's RB
>>
>> * 2
>> - Change ICU_IRQ_TYPE to IcuIrqTypeLevel / IcuIrqTypeEdge
>>
>> * 3
>> - Use EFI_PHYSICAL_ADDRESS
>>
>> * 4
>> - Put a space after { and before }
>> - Change enum values to IcuGroupXxx format
>>
>> * 5
>> - Use ICU_GROUP_REGISTER_BASE_OFFSET instead of a raw value in macros
>> - Add missing parentheses and use sizeof (UINT32) in ICU_INT_CFG macro
>> - Use mEfiExitBootServicesEvent - not initialized and STATIC
>> - s/IcuConfigDefault/IcuInitialConfig/
>> - Break to long lines
>>
>> Marcin Wojtas (6):
>> Marvell/Armada70x0Db: Set correct CP110 count
>> Marvell/Library: Introduce ArmadaIcuLib class
>> Marvell/Library: Armada7k8kSoCDescLib: Enable getting CP base address
>> Marvell/Library: Armada7k8kSoCDescLib: Introduce ICU information
>> Marvell/Library: Implement common ArmadaIcuLib
>> Marvell/Armada7k8k: Enable ICU configuration
>>
>
> Reviewed-by: Ard Biesheuvel
>
> Pushed as 80f6be6eb12b..bd325517ba42 (with 5/6 updated to the v3 version)
>
Thanks!
>> Silicon/Marvell/Marvell.dec
>>| 1 +
>> Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
>>| 1 +
>> Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
>>| 7 +-
>> Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.inf
>>| 1 +
>> Silicon/Marvell/Library/IcuLib/IcuLib.inf
>>| 38 +++
>>
>> Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
>> | 12 +
>> Silicon/Marvell/Include/Library/ArmadaIcuLib.h
>>| 45 +++
>> Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
>>| 39 +++
>> Silicon/Marvell/Library/IcuLib/IcuLib.h
>>| 47 +++
>> Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.c
>>| 2 +
>>
>> Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
>> | 50 +++
>> Silicon/Marvell/Library/IcuLib/IcuLib.c
>>| 317
>> 12 files changed, 558 insertions(+), 2 deletions(-)
>> create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.inf
>> create mode 100644 Silicon/Marvell/Include/Library/ArmadaIcuLib.h
>> create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.h
>> create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.c
>>
>> --
>> 2.7.4
>>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 edk2-platforms 0/4] Platform/Arm/Sgi: platform support updates
On 24 July 2018 at 06:51, Thomas Abraham wrote: > Changes since v1: > - Removed 'Change-Id' from the patch description > > This patch series includes few updates for Arm's SGI platform. The virtio > block device representation in the ACPI table, setting default fifo size > for the uart controller, handling the increase in code size and adding > support for the upper 6GB memory block are included in this update. > > Daniil Egranov (2): > Platform/ARM/Sgi: Add ACPI description of virtio block device > Platform/ARM/Sgi: Allow use of default fifo size for PL011 controller > > Thomas Abraham (2): > Platform/ARM/Sgi: Increase the size of flash image > Platform/ARM/Sgi: allow access the second DRAM block > Reviewed-by: Ard Biesheuvel Pushed as bd325517ba42..2c4d662506bd Thanks, > Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl | 12 +++ > .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf | 4 > .../SgiPkg/Library/PlatformLib/PlatformLibMem.c| 24 > +- > Platform/ARM/SgiPkg/SgiPlatform.dec| 4 > Platform/ARM/SgiPkg/SgiPlatform.dsc| 5 + > Platform/ARM/SgiPkg/SgiPlatform.fdf| 8 > 6 files changed, 52 insertions(+), 5 deletions(-) > > -- > 2.7.4 > > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH v2 0/6] Armada7k8k ICU support
On 13 July 2018 at 10:12, Marcin Wojtas wrote:
> Hi,
>
> The second version of the ICU patchset brings all corrections
> according to all review remarks. They were mostly style / naming
> - detailed list can be found in the changelog below.
>
> The patches are available in the github:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/icu-upstream-r20180713
>
> I'm looking forward to review and any comments/remarks.
>
> Best regards,
> Marcin
>
> Changelog
> v1 -> v2
> * 1,6
> - Add Ard's RB
>
> * 2
> - Change ICU_IRQ_TYPE to IcuIrqTypeLevel / IcuIrqTypeEdge
>
> * 3
> - Use EFI_PHYSICAL_ADDRESS
>
> * 4
> - Put a space after { and before }
> - Change enum values to IcuGroupXxx format
>
> * 5
> - Use ICU_GROUP_REGISTER_BASE_OFFSET instead of a raw value in macros
> - Add missing parentheses and use sizeof (UINT32) in ICU_INT_CFG macro
> - Use mEfiExitBootServicesEvent - not initialized and STATIC
> - s/IcuConfigDefault/IcuInitialConfig/
> - Break to long lines
>
> Marcin Wojtas (6):
> Marvell/Armada70x0Db: Set correct CP110 count
> Marvell/Library: Introduce ArmadaIcuLib class
> Marvell/Library: Armada7k8kSoCDescLib: Enable getting CP base address
> Marvell/Library: Armada7k8kSoCDescLib: Introduce ICU information
> Marvell/Library: Implement common ArmadaIcuLib
> Marvell/Armada7k8k: Enable ICU configuration
>
Reviewed-by: Ard Biesheuvel
Pushed as 80f6be6eb12b..bd325517ba42 (with 5/6 updated to the v3 version)
> Silicon/Marvell/Marvell.dec
> | 1 +
> Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> | 1 +
> Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
> | 7 +-
> Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.inf
> | 1 +
> Silicon/Marvell/Library/IcuLib/IcuLib.inf
> | 38 +++
>
> Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
> | 12 +
> Silicon/Marvell/Include/Library/ArmadaIcuLib.h
> | 45 +++
> Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
> | 39 +++
> Silicon/Marvell/Library/IcuLib/IcuLib.h
> | 47 +++
> Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.c
> | 2 +
>
> Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
> | 50 +++
> Silicon/Marvell/Library/IcuLib/IcuLib.c
> | 317
> 12 files changed, 558 insertions(+), 2 deletions(-)
> create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.inf
> create mode 100644 Silicon/Marvell/Include/Library/ArmadaIcuLib.h
> create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.h
> create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.c
>
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] reg: HTTP Request Failure over Internet
Ting:
Please find the patch for reference.
Index: HttpProto.c
===
--- HttpProto.c
+++ HttpProto.c
@@ -622,12 +622,20 @@
Status = HttpInstance->Tcp4->Configure (HttpInstance->Tcp4, Tcp4CfgData);
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "HttpConfigureTcp4 - %r\n", Status));
return Status;
}
+ HttpInstance->Tcp4->Routes (
+HttpInstance->Tcp4,
+FALSE,
+>RemoteAddr,
+>SubnetMask,
+>RouterAddr
+);
+
Status = HttpCreateTcp4ConnCloseEvent (HttpInstance);
if (EFI_ERROR (Status)) {
return Status;
}
Status = HttpCreateTcp4TxEvent (Wrap);
-Siva
-Original Message-
From: Ye, Ting [mailto:ting...@intel.com]
Sent: Wednesday, July 25, 2018 1:36 PM
To: Laszlo Ersek; Sivaraman Nainar; edk2-devel@lists.01.org
Subject: RE: [edk2] reg: HTTP Request Failure over Internet
Hi Siva,
I didn't receive your patch either. Thanks for reporting the issue, we will try
to reproduce it firstly.
Thanks,
Ting
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo
Ersek
Sent: Tuesday, July 24, 2018 8:43 PM
To: Sivaraman Nainar ; edk2-devel@lists.01.org
Subject: Re: [edk2] reg: HTTP Request Failure over Internet
On 07/24/18 14:05, Sivaraman Nainar wrote:
> Hello all,
>
> When an application tried to download the remote file over internet with the
> HTTP Get Request it getting failed. If we try via the Intranet then
> application downloads the target file.
>
> The remote file is available in the Apache server. With the attached patch
> the download works fine in Internet and Intranet.
>
> Could you review the solution and feedback?
The edk2-devel list software does not reflect attachments to subscribers.
While I disagree with that practice in general -- it breaks conversations where
people justifiedly post small attachments, such as PNG screenshots, compressed
log files and such --, for posting patches specifically, please use
git-format-patch and git-send-email. The patch should be in the body of the
email (please do not copy the patch though; that is guaranteed not to
work -- please use the git tools).
Official guidelines:
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process
Personal ones from yours truly:
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers
Thanks,
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
[edk2] [Patch][edk2-platforms/devel-IntelAtomProcessorE3900] Gcc Build Fix
From: xianhuix
Reduce fv size for gcc debug build
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: xianhuix
---
.../BroxtonPlatformPkg/PlatformDsc/Components.dsc | 30 +-
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/Platform/BroxtonPlatformPkg/PlatformDsc/Components.dsc
b/Platform/BroxtonPlatformPkg/PlatformDsc/Components.dsc
index 15b75be4a5..8f5d833668 100644
--- a/Platform/BroxtonPlatformPkg/PlatformDsc/Components.dsc
+++ b/Platform/BroxtonPlatformPkg/PlatformDsc/Components.dsc
@@ -501,12 +501,30 @@
# MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouseDxe.inf
# MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
- MdeModulePkg/Bus/Pci/EhciDxe/EhciDxe.inf
- MdeModulePkg/Bus/Pci/UhciDxe/UhciDxe.inf
- MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
- MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
- MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouseDxe.inf
- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
+ MdeModulePkg/Bus/Pci/EhciDxe/EhciDxe.inf {
+
+ DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
+ }
+ MdeModulePkg/Bus/Pci/UhciDxe/UhciDxe.inf {
+
+ DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
+ }
+ MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf {
+
+ DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
+ }
+ MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf {
+
+ DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
+ }
+ MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouseDxe.inf {
+
+ DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
+ }
+ MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf {
+
+ DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
+ }
MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf {
!if $(SIMICS_ENABLE) == TRUE
--
2.14.1.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] reg: HTTP Request Failure over Internet
Hi Siva, I didn't receive your patch either. Thanks for reporting the issue, we will try to reproduce it firstly. Thanks, Ting -Original Message- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo Ersek Sent: Tuesday, July 24, 2018 8:43 PM To: Sivaraman Nainar ; edk2-devel@lists.01.org Subject: Re: [edk2] reg: HTTP Request Failure over Internet On 07/24/18 14:05, Sivaraman Nainar wrote: > Hello all, > > When an application tried to download the remote file over internet with the > HTTP Get Request it getting failed. If we try via the Intranet then > application downloads the target file. > > The remote file is available in the Apache server. With the attached patch > the download works fine in Internet and Intranet. > > Could you review the solution and feedback? The edk2-devel list software does not reflect attachments to subscribers. While I disagree with that practice in general -- it breaks conversations where people justifiedly post small attachments, such as PNG screenshots, compressed log files and such --, for posting patches specifically, please use git-format-patch and git-send-email. The patch should be in the body of the email (please do not copy the patch though; that is guaranteed not to work -- please use the git tools). Official guidelines: https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process Personal ones from yours truly: https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers Thanks, 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
[edk2] [Patch V3 0/3] StartAllAPs should not use disabled APs
This patch series include changes: 1. StartAllAPs should not use disabled APs, this is required by UEFI spec. 2. Refine the code to remove the redundant definitions. V2 changes: Use CpuStateReady to distinguish the AP state from CpuStateIdle. V3 Changes: Only change 3/3 patch. Only not use disabled APs when WakeUpAP called by StartAllAps function. In other cases, also include disabled APs. Eric Dong (3): UefiCpuPkg/MpInitLib: Remove redundant CpuStateFinished State. UefiCpuPkg/MpInitLib: Remove StartCount and volatile definition. UefiCpuPkg/MpInitLib: Not use disabled AP when call StartAllAPs. UefiCpuPkg/Library/MpInitLib/MpLib.c | 33 +++-- UefiCpuPkg/Library/MpInitLib/MpLib.h | 4 +--- 2 files changed, 16 insertions(+), 21 deletions(-) -- 2.15.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch v3 2/3] UefiCpuPkg/MpInitLib: Remove StartCount and volatile definition.
The StartCount is duplicated with RunningCount, replace it with
RunningCount. Also the volatile for RunningCount is not needed.
Cc: Laszlo Ersek
Cc: Ruiyu Ni
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong
---
UefiCpuPkg/Library/MpInitLib/MpLib.c | 11 +--
UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +--
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index ff09a0e9e7..0e57cc86bf 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1424,7 +1424,7 @@ CheckAllAPs (
// value of state after setting the it to CpuStateIdle, so BSP can safely
make use of its value.
//
if (GetApState(CpuData) == CpuStateIdle) {
- CpuMpData->RunningCount ++;
+ CpuMpData->RunningCount --;
CpuMpData->CpuData[ProcessorNumber].Waiting = FALSE;
//
@@ -1449,7 +1449,7 @@ CheckAllAPs (
//
// If all APs finish, return EFI_SUCCESS.
//
- if (CpuMpData->RunningCount == CpuMpData->StartCount) {
+ if (CpuMpData->RunningCount == 0) {
return EFI_SUCCESS;
}
@@ -1466,7 +1466,7 @@ CheckAllAPs (
//
if (CpuMpData->FailedCpuList != NULL) {
*CpuMpData->FailedCpuList =
- AllocatePool ((CpuMpData->StartCount - CpuMpData->FinishedCount + 1)
* sizeof (UINTN));
+ AllocatePool ((CpuMpData->RunningCount + 1) * sizeof (UINTN));
ASSERT (*CpuMpData->FailedCpuList != NULL);
}
ListIndex = 0;
@@ -2212,7 +2212,7 @@ StartupAllAPsWorker (
return EFI_NOT_STARTED;
}
- CpuMpData->StartCount = 0;
+ CpuMpData->RunningCount = 0;
for (ProcessorNumber = 0; ProcessorNumber < ProcessorCount;
ProcessorNumber++) {
CpuData = >CpuData[ProcessorNumber];
CpuData->Waiting = FALSE;
@@ -,7 +,7 @@ StartupAllAPsWorker (
// Mark this processor as responsible for current calling.
//
CpuData->Waiting = TRUE;
-CpuMpData->StartCount++;
+CpuMpData->RunningCount++;
}
}
}
@@ -2231,7 +2231,6 @@ StartupAllAPsWorker (
CpuMpData->ProcArguments = ProcedureArgument;
CpuMpData->SingleThread = SingleThread;
CpuMpData->FinishedCount = 0;
- CpuMpData->RunningCount = 0;
CpuMpData->FailedCpuList = FailedCpuList;
CpuMpData->ExpectedTime = CalculateTimeout (
TimeoutInMicroseconds,
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h
b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 962bce685d..5002b7e9c0 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -211,9 +211,8 @@ struct _CPU_MP_DATA {
UINTN BackupBuffer;
UINTN BackupBufferSize;
- volatile UINT32StartCount;
volatile UINT32FinishedCount;
- volatile UINT32RunningCount;
+ UINT32 RunningCount;
BOOLEANSingleThread;
EFI_AP_PROCEDURE Procedure;
VOID *ProcArguments;
--
2.15.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch v3 1/3] UefiCpuPkg/MpInitLib: Remove redundant CpuStateFinished State.
Current CPU state definition include CpuStateIdle and CpuStateFinished.
After investigation, current code can use CpuStateIdle to replace the
CpuStateFinished. It will reduce the state number and easy for maintenance.
Cc: Laszlo Ersek
Cc: Ruiyu Ni
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong
---
UefiCpuPkg/Library/MpInitLib/MpLib.c | 18 --
UefiCpuPkg/Library/MpInitLib/MpLib.h | 1 -
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index c82b985943..ff09a0e9e7 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -696,7 +696,7 @@ ApWakeupFunction (
}
}
}
-SetApState (>CpuData[ProcessorNumber], CpuStateFinished);
+SetApState (>CpuData[ProcessorNumber], CpuStateIdle);
}
}
@@ -1352,18 +1352,17 @@ CheckThisAP (
CpuData = >CpuData[ProcessorNumber];
//
- // Check the CPU state of AP. If it is CpuStateFinished, then the AP has
finished its task.
+ // Check the CPU state of AP. If it is CpuStateIdle, then the AP has
finished its task.
// Only BSP and corresponding AP access this unit of CPU Data. This means
the AP will not modify the
- // value of state after setting the it to CpuStateFinished, so BSP can
safely make use of its value.
+ // value of state after setting the it to CpuStateIdle, so BSP can safely
make use of its value.
//
//
// If the AP finishes for StartupThisAP(), return EFI_SUCCESS.
//
- if (GetApState(CpuData) == CpuStateFinished) {
+ if (GetApState(CpuData) == CpuStateIdle) {
if (CpuData->Finished != NULL) {
*(CpuData->Finished) = TRUE;
}
-SetApState (CpuData, CpuStateIdle);
return EFI_SUCCESS;
} else {
//
@@ -1420,14 +1419,13 @@ CheckAllAPs (
CpuData = >CpuData[ProcessorNumber];
//
-// Check the CPU state of AP. If it is CpuStateFinished, then the AP has
finished its task.
+// Check the CPU state of AP. If it is CpuStateIdle, then the AP has
finished its task.
// Only BSP and corresponding AP access this unit of CPU Data. This means
the AP will not modify the
-// value of state after setting the it to CpuStateFinished, so BSP can
safely make use of its value.
+// value of state after setting the it to CpuStateIdle, so BSP can safely
make use of its value.
//
-if (GetApState(CpuData) == CpuStateFinished) {
+if (GetApState(CpuData) == CpuStateIdle) {
CpuMpData->RunningCount ++;
CpuMpData->CpuData[ProcessorNumber].Waiting = FALSE;
- SetApState(CpuData, CpuStateIdle);
//
// If in Single Thread mode, then search for the next waiting AP for
execution.
@@ -1923,7 +1921,7 @@ SwitchBSPWorker (
//
// Wait for old BSP finished AP task
//
- while (GetApState (>CpuData[CallerNumber]) != CpuStateFinished) {
+ while (GetApState (>CpuData[CallerNumber]) != CpuStateIdle) {
CpuPause ();
}
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h
b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 9d0b866d09..962bce685d 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -85,7 +85,6 @@ typedef enum {
CpuStateIdle,
CpuStateReady,
CpuStateBusy,
- CpuStateFinished,
CpuStateDisabled
} CPU_STATE;
--
2.15.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch v3 3/3] UefiCpuPkg/MpInitLib: Not use disabled AP when call StartAllAPs.
Base on UEFI spec requirement, StartAllAPs function should not
use the APs which has been disabled before. This patch just
change current code to follow this rule.
V3 changes:
Only called by StartUpAllAps, WakeUpAp will not wake up
the disabled APs, in other cases also need to include the
disabled APs, such as CpuDxe driver start up and
ChangeApLoopCallback function.
Cc: Laszlo Ersek
Cc: Ruiyu Ni
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong
---
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 2 +-
UefiCpuPkg/Library/MpInitLib/MpLib.c| 27 +--
UefiCpuPkg/Library/MpInitLib/MpLib.h| 4 +++-
3 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index d82e9aea45..c17daa0fc0 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -306,7 +306,7 @@ MpInitChangeApLoopCallback (
CpuMpData->PmCodeSegment = GetProtectedModeCS ();
CpuMpData->ApLoopMode = PcdGet8 (PcdCpuApLoopMode);
mNumberToFinish = CpuMpData->CpuCount - 1;
- WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, NULL);
+ WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, NULL, TRUE);
while (mNumberToFinish > 0) {
CpuPause ();
}
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 0e57cc86bf..1a81062a3f 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -470,7 +470,7 @@ CollectProcessorCount (
//
CpuMpData->InitFlag = ApInitConfig;
CpuMpData->X2ApicEnable = FALSE;
- WakeUpAP (CpuMpData, TRUE, 0, NULL, NULL);
+ WakeUpAP (CpuMpData, TRUE, 0, NULL, NULL, TRUE);
CpuMpData->InitFlag = ApInitDone;
ASSERT (CpuMpData->CpuCount <= PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
//
@@ -491,7 +491,7 @@ CollectProcessorCount (
//
// Wakeup all APs to enable x2APIC mode
//
-WakeUpAP (CpuMpData, TRUE, 0, ApFuncEnableX2Apic, NULL);
+WakeUpAP (CpuMpData, TRUE, 0, ApFuncEnableX2Apic, NULL, TRUE);
//
// Wait for all known APs finished
//
@@ -969,6 +969,7 @@ FreeResetVector (
@param[in] ProcessorNumberThe handle number of specified processor
@param[in] Procedure The function to be invoked by AP
@param[in] ProcedureArgument The argument to be passed into AP function
+ @param[in] WakeUpDisabledAps Whether need to wake up disabled APs in
broadcast mode.
**/
VOID
WakeUpAP (
@@ -976,7 +977,8 @@ WakeUpAP (
IN BOOLEAN Broadcast,
IN UINTN ProcessorNumber,
IN EFI_AP_PROCEDURE Procedure, OPTIONAL
- IN VOID *ProcedureArgument OPTIONAL
+ IN VOID *ProcedureArgument, OPTIONAL
+ IN BOOLEAN WakeUpDisabledAps
)
{
volatile MP_CPU_EXCHANGE_INFO*ExchangeInfo;
@@ -1010,6 +1012,10 @@ WakeUpAP (
for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
if (Index != CpuMpData->BspNumber) {
CpuData = >CpuData[Index];
+if (GetApState (CpuData) == CpuStateDisabled && !WakeUpDisabledAps) {
+ continue;
+}
+
CpuData->ApFunction = (UINTN) Procedure;
CpuData->ApFunctionArgument = (UINTN) ProcedureArgument;
SetApState (CpuData, CpuStateReady);
@@ -1289,7 +1295,7 @@ ResetProcessorToIdleState (
CpuMpData = GetCpuMpData ();
CpuMpData->InitFlag = ApInitReconfig;
- WakeUpAP (CpuMpData, FALSE, ProcessorNumber, NULL, NULL);
+ WakeUpAP (CpuMpData, FALSE, ProcessorNumber, NULL, NULL, TRUE);
while (CpuMpData->FinishedCount < 1) {
CpuPause ();
}
@@ -1439,7 +1445,8 @@ CheckAllAPs (
FALSE,
(UINT32) NextProcessorNumber,
CpuMpData->Procedure,
-CpuMpData->ProcArguments
+CpuMpData->ProcArguments,
+TRUE
);
}
}
@@ -1711,7 +1718,7 @@ MpInitLibInitialize (
//
// Wakeup APs to do some AP initialize sync
//
- WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData);
+ WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
//
// Wait for all APs finished initialization
//
@@ -1906,7 +1913,7 @@ SwitchBSPWorker (
//
// Need to wakeUp AP (future BSP).
//
- WakeUpAP (CpuMpData, FALSE, ProcessorNumber, FutureBSPProc, CpuMpData);
+ WakeUpAP (CpuMpData, FALSE, ProcessorNumber, FutureBSPProc, CpuMpData, TRUE);
AsmExchangeRole (>BSPInfo, >APInfo);
@@ -2240,14 +2247,14 @@ StartupAllAPsWorker (
CpuMpData->WaitEvent = WaitEvent;
if (!SingleThread) {
-WakeUpAP (CpuMpData, TRUE, 0, Procedure, ProcedureArgument);
+WakeUpAP (CpuMpData, TRUE, 0, Procedure, ProcedureArgument, FALSE);
} else {
for (ProcessorNumber = 0; ProcessorNumber < ProcessorCount;
ProcessorNumber++) {
if
