[edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Clear some semaphores on S3 boot path

[Jeff Fan]

Some semaphores are not cleared on S3 boot path. For example,
mSmmMpSyncData->CpuData[CpuIndex].Present. It may still keeps the value set at
SMM runtime during S3 resume. It may causes BSP have the wrong judgement on SMM
AP's present state.

We have one related fix at e78a2a49ee6b0c0d7c6997c87ace31d7761cf636. But that is
not completed.

This fix is to clear Busy/Run/Present semaphores in InitializeMpSyncData().

Cc: Laszlo Ersek 
Cc: Feng Tian 
Cc: Jiewen Yao 
Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index cfbf59e..a873b68 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -1357,6 +1357,9 @@ InitializeMpSyncData (
 (UINT32 *)((UINTN)mSmmCpuSemaphores.SemaphoreCpu.Run + mSemaphoreSize 
* CpuIndex);
   mSmmMpSyncData->CpuData[CpuIndex].Present =
 (BOOLEAN *)((UINTN)mSmmCpuSemaphores.SemaphoreCpu.Present + 
mSemaphoreSize * CpuIndex);
+  *(mSmmMpSyncData->CpuData[CpuIndex].Busy)= 0;
+  *(mSmmMpSyncData->CpuData[CpuIndex].Run) = 0;
+  *(mSmmMpSyncData->CpuData[CpuIndex].Present) = FALSE;
 }
   }
 }
-- 
2.9.3.windows.2

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] UefiCpuPkg:PiSmmCpu: Set correct attribute on split.

[Jiewen Yao]

PiSmmCpu driver may split page for page attribute request.
Current logic will propagate the super page attribute attribute.
However, it might be wrong because we cannot clear protection
without touch super page attribute.

We should always clear protection on super page and set
protection on end page for easy clear later.

Cc: Jeff Fan 
Cc: Michael D Kinney 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index accc11e..d0f41a8 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -303,7 +303,7 @@ SplitPage (
   for (Index = 0; Index < SIZE_4KB / sizeof(UINT64); Index++) {
 NewPageEntry[Index] = BaseAddress + SIZE_4KB * Index + ((*PageEntry) & 
PAGE_PROGATE_BITS);
   }
-  (*PageEntry) = (UINT64)(UINTN)NewPageEntry + ((*PageEntry) & 
PAGE_PROGATE_BITS);
+  (*PageEntry) = (UINT64)(UINTN)NewPageEntry + PAGE_ATTRIBUTE_BITS;
   return RETURN_SUCCESS;
 } else {
   return RETURN_UNSUPPORTED;
@@ -324,7 +324,7 @@ SplitPage (
   for (Index = 0; Index < SIZE_4KB / sizeof(UINT64); Index++) {
 NewPageEntry[Index] = BaseAddress + SIZE_2MB * Index + IA32_PG_PS + 
((*PageEntry) & PAGE_PROGATE_BITS);
   }
-  (*PageEntry) = (UINT64)(UINTN)NewPageEntry + ((*PageEntry) & 
PAGE_PROGATE_BITS);
+  (*PageEntry) = (UINT64)(UINTN)NewPageEntry + PAGE_ATTRIBUTE_BITS;
   return RETURN_SUCCESS;
 } else {
   return RETURN_UNSUPPORTED;
-- 
2.7.4.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] SourceLevelDebugPkg: Avoid to re-init IDT table again at SMI entry

[Jeff Fan]

Current SmmDebugAgentLib will initialize IDT table to support source debugging
at each time SMI entry on SMM BSP. Actually, we only need to initialize IDT
table at first time SMI entry.

Add one flag to avoid re-initializing IDT table.

Cc: Ruiyu Ni 
Cc: Jiewen Yao 
Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
---
 .../Library/DebugAgent/SmmDebugAgent/SmmDebugAgentLib.c| 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git 
a/SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgent/SmmDebugAgentLib.c 
b/SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgent/SmmDebugAgentLib.c
index 701c4be..6216142 100644
--- a/SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgent/SmmDebugAgentLib.c
+++ b/SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgent/SmmDebugAgentLib.c
@@ -19,6 +19,7 @@ DEBUG_AGENT_MAILBOX mLocalMailbox;
 UINTN   mSavedDebugRegisters[6];
 IA32_IDT_GATE_DESCRIPTORmIdtEntryTable[33];
 BOOLEAN mSkipBreakpoint = FALSE;
+BOOLEAN mSmmDebugIdtInitFlag = FALSE;
 
 CHAR8 mWarningMsgIgnoreSmmEntryBreak[] = "Ignore smmentrybreak setting for SMI 
issued during DXE debugging!\r\n";
 
@@ -276,7 +277,14 @@ InitializeDebugAgent (
 
   case DEBUG_AGENT_INIT_ENTER_SMI:
 SaveDebugRegister ();
-InitializeDebugIdt ();
+if (!mSmmDebugIdtInitFlag) {
+  //
+  // We only need to initialize Debug IDT table at first SMI entry
+  // after SMM relocation.
+  //
+  InitializeDebugIdt ();
+  mSmmDebugIdtInitFlag = TRUE;
+}
 //
 // Check if CPU APIC Timer is working, otherwise initialize it.
 //
-- 
2.9.3.windows.2

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] SignedCapsulePkg: GetImage() return EFI_UNSUPPORTED.

[Dong, Eric]

Reviewed-by: Eric Dong 

> -Original Message-
> From: Yao, Jiewen
> Sent: Tuesday, November 29, 2016 12:56 PM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric
> Subject: [PATCH] SignedCapsulePkg: GetImage() return EFI_UNSUPPORTED.
> 
> According to UEFI spec, unsupported function should return EFI_UNSUPPORTED
> directly.
> 
> Cc: Eric Dong 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao 
> ---
>  .../Universal/SystemFirmwareUpdate/SystemFirmwareCommonDxe.c | 12 
> 
>  1 file changed, 12 deletions(-)
> 
> diff --git 
> a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareCommonDxe.c
> b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareCommonDxe.c
> index 642a99d..60490a4 100644
> --- 
> a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareCommonDxe.c
> +++ 
> b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareCommonDxe.c
> @@ -178,18 +178,6 @@ FmpGetImage (
>IN  OUT  UINTN*ImageSize
>)
>  {
> -  SYSTEM_FMP_PRIVATE_DATA *SystemFmpPrivate;
> -
> -  if (Image == NULL || ImageSize == NULL) {
> -return EFI_INVALID_PARAMETER;
> -  }
> -
> -  SystemFmpPrivate = SYSTEM_FMP_PRIVATE_DATA_FROM_FMP(This);
> -
> -  if (ImageIndex == 0 || ImageIndex > SystemFmpPrivate->DescriptorCount || 
> ImageSize == NULL || Image == NULL) {
> -return EFI_INVALID_PARAMETER;
> -  }
> -
>return EFI_UNSUPPORTED;
>  }
> 
> --
> 2.7.4.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] SignedCapsulePkg: GetImage() return EFI_UNSUPPORTED.

[Jiewen Yao]

According to UEFI spec, unsupported function should return EFI_UNSUPPORTED
directly.

Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 .../Universal/SystemFirmwareUpdate/SystemFirmwareCommonDxe.c | 12 
 1 file changed, 12 deletions(-)

diff --git 
a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareCommonDxe.c 
b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareCommonDxe.c
index 642a99d..60490a4 100644
--- a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareCommonDxe.c
+++ b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareCommonDxe.c
@@ -178,18 +178,6 @@ FmpGetImage (
   IN  OUT  UINTN*ImageSize
   )
 {
-  SYSTEM_FMP_PRIVATE_DATA *SystemFmpPrivate;
-
-  if (Image == NULL || ImageSize == NULL) {
-return EFI_INVALID_PARAMETER;
-  }
-
-  SystemFmpPrivate = SYSTEM_FMP_PRIVATE_DATA_FROM_FMP(This);
-
-  if (ImageIndex == 0 || ImageIndex > SystemFmpPrivate->DescriptorCount || 
ImageSize == NULL || Image == NULL) {
-return EFI_INVALID_PARAMETER;
-  }
-
   return EFI_UNSUPPORTED;
 }
 
-- 
2.7.4.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] ShellPkg: Add error prompt message in Ifconfig6 command.

[Zhang, Lubo]

Thank you for reminding.

Lubo

-Original Message-
From: Hegde, Nagaraj P [mailto:nagaraj-p.he...@hpe.com] 
Sent: Tuesday, November 29, 2016 11:59 AM
To: Zhang, Lubo ; edk2-devel@lists.01.org
Cc: Ye, Ting ; Fu, Siyuan ; Wu, Jiaxin 

Subject: RE: [edk2] [PATCH v2] ShellPkg: Add error prompt message in Ifconfig6 
command.

Spelling correction in the uni file for STR_IFCONFIG6_ERR_MAN_GW. Should be 
"Gateway" instead of "Getway".
Reviewed-by: Hegde, Nagaraj P 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zhang 
Lubo
Sent: Tuesday, November 29, 2016 8:28 AM
To: edk2-devel@lists.01.org
Cc: Ye Ting ; Fu Siyuan ; Wu Jiaxin 

Subject: [edk2] [PATCH v2] ShellPkg: Add error prompt message in Ifconfig6 
command.

v2: update the prompt message more readable.

It should display error prompt message when Ifconfig6 can not configure 
correctly.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Wu Jiaxin 
---
 ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c | 15 +++
 .../UefiShellNetwork2CommandsLib.uni  |  9 +
 2 files changed, 24 insertions(+)

diff --git a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c 
b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
index 32dd284..fb308cc 100644
--- a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
+++ b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
@@ -1315,10 +1315,24 @@ IfConfig6SetInterfaceInfo (
 goto ON_EXIT;
   }
 
   VarArg= VarArg->Next;
 
+  if (StrCmp (VarArg->Arg, L"host") == 0) {
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
(STR_IFCONFIG6_ERR_INVALID_IP_CONFIG), gShellNetwork2HiiHandle, Status);
+ShellStatus = EFI_INVALID_PARAMETER;
+goto ON_EXIT;
+  } else if (StrCmp (VarArg->Arg, L"gw") == 0) {
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
(STR_IFCONFIG6_ERR_INVALID_GW_CONFIG), gShellNetwork2HiiHandle, Status);
+ShellStatus = EFI_INVALID_PARAMETER;
+goto ON_EXIT;
+  } else if (StrCmp (VarArg->Arg, L"dns") == 0) {
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
(STR_IFCONFIG6_ERR_INVALID_DNS_CONFIG), gShellNetwork2HiiHandle, Status);
+ShellStatus = EFI_INVALID_PARAMETER;
+goto ON_EXIT;
+  }
+
 } else if (StrCmp (VarArg->Arg, L"man") == 0) {
   //
   // Set manual config policy.
   //
   Policy = Ip6ConfigPolicyManual;
@@ -1509,10 +1523,11 @@ IfConfig6SetInterfaceInfo (
   CfgAddr
   );
 
   if (EFI_ERROR (Status)) {
 ShellStatus = SHELL_ACCESS_DENIED;
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
+ (STR_IFCONFIG6_ERR_MAN_GW), gShellNetwork2HiiHandle, Status);
 goto ON_EXIT;
   }
 
 } else if (StrCmp (VarArg->Arg, L"dns") == 0) {
   //
diff --git 
a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.uni
 
b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.uni
index c3445bb..79af7f9 100644
--- 
a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.uni
+++ b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Com
+++ mandsLib.uni
@@ -75,10 +75,19 @@
 #string STR_IFCONFIG6_ERR_LACK_ARGUMENTS   #language en-US"Lack 
arguments. Bad command %H%s%N is skipped.\r\n"
   "Hint: 
Please type 'IfConfig6 -?' for help info.\r\n"
 #string STR_IFCONFIG6_ERR_LACK_OPTION  #language en-US"Lack 
options.\r\n"
   "Hint: 
Please type 'IfConfig6 -?' for help info.\r\n"
 #string STR_IFCONFIG6_ERR_MAN_HOST #language en-US"Manual 
address configuration failed. Please retry.\r\n"
+
+#string STR_IFCONFIG6_ERR_MAN_GW   #language en-US"Getway 
address configuration failed. Please check the argument.\r\n"
+
+#string STR_IFCONFIG6_ERR_INVALID_IP_CONFIG#language en-US"The IP 
address is not configurable when the policy is Ip6ConfigPolicyAutomatic.\r\n"
+
+#string STR_IFCONFIG6_ERR_INVALID_GW_CONFIG#language en-US"The gateway 
address is not configurable when the policy is Ip6ConfigPolicyAutomatic.\r\n"
+
+#string STR_IFCONFIG6_ERR_INVALID_DNS_CONFIG   #language en-US"The DNS 
server address is not configurable when the policy is 
Ip6ConfigPolicyAutomatic.\r\n"
+
 #string STR_IFCONFIG6_ERR_DUPLICATE_COMMAND#language en-US"Duplicate 
commands. Bad command %H%s%N is skipped.\r\n"
   "Hint: 
Please type 'IfConfig6 -?' for help info.\r\n"
 #string STR_IFCONFIG6_ERR_CONFLICT_COMMAND #language en-US"Conflict 
commands. Bad command %H%s%N is skipped.\r\n"
   

Re: [edk2] [PATCH v2] ShellPkg: Add error prompt message in Ifconfig6 command.

[Hegde, Nagaraj P]

Spelling correction in the uni file for STR_IFCONFIG6_ERR_MAN_GW. Should be 
"Gateway" instead of "Getway".
Reviewed-by: Hegde, Nagaraj P 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zhang 
Lubo
Sent: Tuesday, November 29, 2016 8:28 AM
To: edk2-devel@lists.01.org
Cc: Ye Ting ; Fu Siyuan ; Wu Jiaxin 

Subject: [edk2] [PATCH v2] ShellPkg: Add error prompt message in Ifconfig6 
command.

v2: update the prompt message more readable.

It should display error prompt message when Ifconfig6 can not configure 
correctly.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Wu Jiaxin 
---
 ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c | 15 +++
 .../UefiShellNetwork2CommandsLib.uni  |  9 +
 2 files changed, 24 insertions(+)

diff --git a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c 
b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
index 32dd284..fb308cc 100644
--- a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
+++ b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
@@ -1315,10 +1315,24 @@ IfConfig6SetInterfaceInfo (
 goto ON_EXIT;
   }
 
   VarArg= VarArg->Next;
 
+  if (StrCmp (VarArg->Arg, L"host") == 0) {
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
(STR_IFCONFIG6_ERR_INVALID_IP_CONFIG), gShellNetwork2HiiHandle, Status);
+ShellStatus = EFI_INVALID_PARAMETER;
+goto ON_EXIT;
+  } else if (StrCmp (VarArg->Arg, L"gw") == 0) {
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
(STR_IFCONFIG6_ERR_INVALID_GW_CONFIG), gShellNetwork2HiiHandle, Status);
+ShellStatus = EFI_INVALID_PARAMETER;
+goto ON_EXIT;
+  } else if (StrCmp (VarArg->Arg, L"dns") == 0) {
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
(STR_IFCONFIG6_ERR_INVALID_DNS_CONFIG), gShellNetwork2HiiHandle, Status);
+ShellStatus = EFI_INVALID_PARAMETER;
+goto ON_EXIT;
+  }
+
 } else if (StrCmp (VarArg->Arg, L"man") == 0) {
   //
   // Set manual config policy.
   //
   Policy = Ip6ConfigPolicyManual;
@@ -1509,10 +1523,11 @@ IfConfig6SetInterfaceInfo (
   CfgAddr
   );
 
   if (EFI_ERROR (Status)) {
 ShellStatus = SHELL_ACCESS_DENIED;
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
+ (STR_IFCONFIG6_ERR_MAN_GW), gShellNetwork2HiiHandle, Status);
 goto ON_EXIT;
   }
 
 } else if (StrCmp (VarArg->Arg, L"dns") == 0) {
   //
diff --git 
a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.uni
 
b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.uni
index c3445bb..79af7f9 100644
--- 
a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.uni
+++ b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Com
+++ mandsLib.uni
@@ -75,10 +75,19 @@
 #string STR_IFCONFIG6_ERR_LACK_ARGUMENTS   #language en-US"Lack 
arguments. Bad command %H%s%N is skipped.\r\n"
   "Hint: 
Please type 'IfConfig6 -?' for help info.\r\n"
 #string STR_IFCONFIG6_ERR_LACK_OPTION  #language en-US"Lack 
options.\r\n"
   "Hint: 
Please type 'IfConfig6 -?' for help info.\r\n"
 #string STR_IFCONFIG6_ERR_MAN_HOST #language en-US"Manual 
address configuration failed. Please retry.\r\n"
+
+#string STR_IFCONFIG6_ERR_MAN_GW   #language en-US"Getway 
address configuration failed. Please check the argument.\r\n"
+
+#string STR_IFCONFIG6_ERR_INVALID_IP_CONFIG#language en-US"The IP 
address is not configurable when the policy is Ip6ConfigPolicyAutomatic.\r\n"
+
+#string STR_IFCONFIG6_ERR_INVALID_GW_CONFIG#language en-US"The gateway 
address is not configurable when the policy is Ip6ConfigPolicyAutomatic.\r\n"
+
+#string STR_IFCONFIG6_ERR_INVALID_DNS_CONFIG   #language en-US"The DNS 
server address is not configurable when the policy is 
Ip6ConfigPolicyAutomatic.\r\n"
+
 #string STR_IFCONFIG6_ERR_DUPLICATE_COMMAND#language en-US"Duplicate 
commands. Bad command %H%s%N is skipped.\r\n"
   "Hint: 
Please type 'IfConfig6 -?' for help info.\r\n"
 #string STR_IFCONFIG6_ERR_CONFLICT_COMMAND #language en-US"Conflict 
commands. Bad command %H%s%N is skipped.\r\n"
   "Hint: 
Please type 'IfConfig6 -?' for help info.\r\n"
 #string STR_IFCONFIG6_ERR_UNKNOWN_COMMAND  #language en-US"Unknown 
commands. Bad command %H%s%N is skipped.\r\n"
--
1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo

Re: [edk2] [Patch] BaseTools CommonLib: Update ReadMemoryFileLine() to read line in file scope

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong

-Original Message-
From: Gao, Liming 
Sent: Friday, November 25, 2016 3:23 PM
To: edk2-devel@lists.01.org
Cc: Zhu, Yonghong 
Subject: [Patch] BaseTools CommonLib: Update ReadMemoryFileLine() to read line 
in file scope

https://bugzilla.tianocore.org/show_bug.cgi?id=255

Check CurrentFilePointer to make sure it not exceed the end of file.

Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 BaseTools/Source/C/Common/MemoryFile.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/BaseTools/Source/C/Common/MemoryFile.c 
b/BaseTools/Source/C/Common/MemoryFile.c
index 1d90688..ec27619 100644
--- a/BaseTools/Source/C/Common/MemoryFile.c
+++ b/BaseTools/Source/C/Common/MemoryFile.c
@@ -222,6 +222,9 @@ Returns:
   // Increment the current file pointer (include the 0x0A)
   //
   InputFile->CurrentFilePointer += CharsToCopy + 1;
+  if (InputFile->CurrentFilePointer > InputFile->Eof) {
+InputFile->CurrentFilePointer = InputFile->Eof;  }
   CheckMemoryFileState (InputMemoryFile);
 
   //
-- 
2.8.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2] ShellPkg: Add error prompt message in Ifconfig6 command.

[Zhang Lubo]

v2: update the prompt message more readable.

It should display error prompt message when Ifconfig6 can
not configure correctly.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Wu Jiaxin 
---
 ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c | 15 +++
 .../UefiShellNetwork2CommandsLib.uni  |  9 +
 2 files changed, 24 insertions(+)

diff --git a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c 
b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
index 32dd284..fb308cc 100644
--- a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
+++ b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
@@ -1315,10 +1315,24 @@ IfConfig6SetInterfaceInfo (
 goto ON_EXIT;
   }
 
   VarArg= VarArg->Next;
 
+  if (StrCmp (VarArg->Arg, L"host") == 0) {
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
(STR_IFCONFIG6_ERR_INVALID_IP_CONFIG), gShellNetwork2HiiHandle, Status);
+ShellStatus = EFI_INVALID_PARAMETER;
+goto ON_EXIT;
+  } else if (StrCmp (VarArg->Arg, L"gw") == 0) {
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
(STR_IFCONFIG6_ERR_INVALID_GW_CONFIG), gShellNetwork2HiiHandle, Status);
+ShellStatus = EFI_INVALID_PARAMETER;
+goto ON_EXIT;
+  } else if (StrCmp (VarArg->Arg, L"dns") == 0) {
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
(STR_IFCONFIG6_ERR_INVALID_DNS_CONFIG), gShellNetwork2HiiHandle, Status);
+ShellStatus = EFI_INVALID_PARAMETER;
+goto ON_EXIT;
+  }
+
 } else if (StrCmp (VarArg->Arg, L"man") == 0) {
   //
   // Set manual config policy.
   //
   Policy = Ip6ConfigPolicyManual;
@@ -1509,10 +1523,11 @@ IfConfig6SetInterfaceInfo (
   CfgAddr
   );
 
   if (EFI_ERROR (Status)) {
 ShellStatus = SHELL_ACCESS_DENIED;
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
(STR_IFCONFIG6_ERR_MAN_GW), gShellNetwork2HiiHandle, Status);
 goto ON_EXIT;
   }
 
 } else if (StrCmp (VarArg->Arg, L"dns") == 0) {
   //
diff --git 
a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.uni
 
b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.uni
index c3445bb..79af7f9 100644
--- 
a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.uni
+++ 
b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.uni
@@ -75,10 +75,19 @@
 #string STR_IFCONFIG6_ERR_LACK_ARGUMENTS   #language en-US"Lack 
arguments. Bad command %H%s%N is skipped.\r\n"
   "Hint: 
Please type 'IfConfig6 -?' for help info.\r\n"
 #string STR_IFCONFIG6_ERR_LACK_OPTION  #language en-US"Lack 
options.\r\n"
   "Hint: 
Please type 'IfConfig6 -?' for help info.\r\n"
 #string STR_IFCONFIG6_ERR_MAN_HOST #language en-US"Manual 
address configuration failed. Please retry.\r\n"
+
+#string STR_IFCONFIG6_ERR_MAN_GW   #language en-US"Getway 
address configuration failed. Please check the argument.\r\n"
+
+#string STR_IFCONFIG6_ERR_INVALID_IP_CONFIG#language en-US"The IP 
address is not configurable when the policy is Ip6ConfigPolicyAutomatic.\r\n"
+
+#string STR_IFCONFIG6_ERR_INVALID_GW_CONFIG#language en-US"The gateway 
address is not configurable when the policy is Ip6ConfigPolicyAutomatic.\r\n"
+
+#string STR_IFCONFIG6_ERR_INVALID_DNS_CONFIG   #language en-US"The DNS 
server address is not configurable when the policy is 
Ip6ConfigPolicyAutomatic.\r\n"
+
 #string STR_IFCONFIG6_ERR_DUPLICATE_COMMAND#language en-US"Duplicate 
commands. Bad command %H%s%N is skipped.\r\n"
   "Hint: 
Please type 'IfConfig6 -?' for help info.\r\n"
 #string STR_IFCONFIG6_ERR_CONFLICT_COMMAND #language en-US"Conflict 
commands. Bad command %H%s%N is skipped.\r\n"
   "Hint: 
Please type 'IfConfig6 -?' for help info.\r\n"
 #string STR_IFCONFIG6_ERR_UNKNOWN_COMMAND  #language en-US"Unknown 
commands. Bad command %H%s%N is skipped.\r\n"
-- 
1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 1/2] UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP startup

[Fan, Jeff]

Reviewed-by: Jeff Fan 

Thanks!

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Monday, November 28, 2016 9:09 PM
To: edk2-devel-01
Cc: Igor Mammedov; Fan, Jeff; Justen, Jordan L; Kinney, Michael D
Subject: [PATCH v3 1/2] UefiCpuPkg/MpInitLib: wait no longer than necessary for 
initial AP startup

Sometimes a platform knows exactly how many CPUs it has at boot. It should be 
able to
- set PcdCpuMaxLogicalProcessorNumber dynamically to this number,
- set PcdCpuApInitTimeOutInMicroSeconds to a very long time (for example
  MAX_UINT32, approx. 71 minutes),
- and expect that MpInitLib wait exactly as long as necessary for all APs
  to report in.

Other platforms should be able to continue setting a reasonably large upper 
bound on supported CPUs, and waiting for a reasonable, fixed amount of time for 
all APs to report in.

Add this functionality. The TimedWaitForApFinish() function will return when 
all APs have reported in, or the timeout has expired -- whichever happens first.

(Accessing these PCDs dynamically is safe. The PEI and DXE phase instances of 
this library are restricted to PEIM and DXE_DRIVER client modules, thus the PCD 
accesses cannot be linked into runtime code.)

Cc: Igor Mammedov 
Cc: Jeff Fan 
Cc: Jordan Justen 
Cc: Michael Kinney 
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=116
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---

Notes:
v3:
- preserve original behavior for PcdCpuApInitTimeOutInMicroSeconds==0
  [Jeff]

 UefiCpuPkg/Library/MpInitLib/MpLib.c | 73 +++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 15dbfa1e7d6c..ed22ce67782e 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -684,6 +684,21 @@ FillExchangeInfoData (  }
 
 /**
+  Helper function that waits until the finished AP count reaches the 
+ specified  limit, or the specified timeout elapses (whichever comes first).
+
+  @param[in] CpuMpDataPointer to CPU MP Data.
+  @param[in] FinishedApLimit  The number of finished APs to wait for.
+  @param[in] TimeLimitThe number of microseconds to wait for.
+**/
+VOID
+TimedWaitForApFinish (
+  IN CPU_MP_DATA   *CpuMpData,
+  IN UINT32FinishedApLimit,
+  IN UINT32TimeLimit
+  );
+
+/**
   This function will be called by BSP to wakeup AP.
 
   @param[in] CpuMpData  Pointer to CPU MP Data
@@ -748,7 +763,11 @@ WakeUpAP (
   //
   // Wait for all potential APs waken up in one specified period
   //
-  MicroSecondDelay (PcdGet32(PcdCpuApInitTimeOutInMicroSeconds));
+  TimedWaitForApFinish (
+CpuMpData,
+PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1,
+PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds)
+);
 } else {
   //
   // Wait all APs waken up if this is not the 1st broadcast of SIPI @@ 
-895,6 +914,58 @@ CheckTimeout (  }
 
 /**
+  Helper function that waits until the finished AP count reaches the 
+ specified  limit, or the specified timeout elapses (whichever comes first).
+
+  @param[in] CpuMpDataPointer to CPU MP Data.
+  @param[in] FinishedApLimit  The number of finished APs to wait for.
+  @param[in] TimeLimitThe number of microseconds to wait for.
+**/
+VOID
+TimedWaitForApFinish (
+  IN CPU_MP_DATA   *CpuMpData,
+  IN UINT32FinishedApLimit,
+  IN UINT32TimeLimit
+  )
+{
+  //
+  // CalculateTimeout() and CheckTimeout() consider a TimeLimit of 0
+  // "infinity", so check for (TimeLimit == 0) explicitly.
+  //
+  if (TimeLimit == 0) {
+return;
+  }
+
+  CpuMpData->TotalTime = 0;
+  CpuMpData->ExpectedTime = CalculateTimeout (
+  TimeLimit,
+  &CpuMpData->CurrentTime
+  );
+  while (CpuMpData->FinishedCount < FinishedApLimit &&
+ !CheckTimeout (
+&CpuMpData->CurrentTime,
+&CpuMpData->TotalTime,
+CpuMpData->ExpectedTime
+)) {
+CpuPause ();
+  }
+
+  if (CpuMpData->FinishedCount >= FinishedApLimit) {
+DEBUG ((
+  DEBUG_VERBOSE,
+  "%a: reached FinishedApLimit=%u in %Lu microseconds\n",
+  __FUNCTION__,
+  FinishedApLimit,
+  DivU64x64Remainder (
+MultU64x32 (CpuMpData->TotalTime, 100),
+GetPerformanceCounterProperties (NULL, NULL),
+NULL
+)
+  ));
+  }
+}
+
+/**
   Reset an AP to Idle state.
 
   Any task being executed by the AP will be aborted and the AP
--
2.9.2


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] EXT FS support

[Carsey, Jaben]

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Wednesday, November 23, 2016 11:50 AM
> To: Rod Smith 
> Cc: Michael Zimmermann ; edk2-
> de...@ml01.01.org
> Subject: Re: [edk2] EXT FS support
> Importance: High
> 
> On 11/22/16 22:31, Rod Smith wrote:
> > On 11/22/2016 01:25 PM, Pete Batard wrote:
> >> On 2016.11.22 17:16, Marcin Wojtas wrote:
> >>> There's no GRUB, platform simply boots to the Shell.
> >>
> >> I think there may be some confusion.
> >>
> >> All the drivers we linked you to are pure UEFI drivers. It doesn't
> >> matter if they were a port from GRUB or something else, the code was
> >> converted to work as a standalone UEFI driver. Especially you don't need
> >> to have GRUB running or anything. You can just use the "load" command
> in
> >> the shell to load the driver, and then access the file system.
> >>
> >> I believe you should be able to download any of the ext binary drivers
> >> mentioned and use them in your shell right away to access your file
> >> system from it.
> >
> > Yes, but it sounds like Marcin may want to embed the ext4fs driver in a
> > custom EFI build. AFAIK, none of the drivers in question were designed
> > with that in mind; however, the VirtualBox project has incorporated
> > ISO-9660 and HFS+ drivers, both of which are built on the same framework
> > (rEFIt's) as rEFInd's drivers, into its own firmware. Thus, Marcin might
> > be able to look at the VirtualBox code and use whatever techniques or
> > glue it uses to incorporate something else. (I can't point to specific
> > files, though.) The rEFInd drivers might be easiest to build in this
> > way, but that's just a guess.
> 
> Any valid EFI binary can be built into edk2 platforms easily. As an example, I
> recommend looking at
> 
>   EdkShellBinPkg/FullShell/FullShell.inf

I would recommend ShellBinPkg over the EdkShellBinPkg.  One is the UEFI Shell 
Spec compliant, the other is not.

> 
> and the following portions from OvmfPkg/OvmfPkgX64.fdf:
> 
> INF  RuleOverride = BINARY EdkShellBinPkg/FullShell/FullShell.inf
> ...
> 
> [Rule.Common.UEFI_DRIVER.BINARY]
>   FILE DRIVER = $(NAMED_GUID) {
> DXE_DEPEX DXE_DEPEX Optional  |.depex
> PE32  PE32|.efi
> UISTRING="$(MODULE_NAME)" Optional
> VERSION   STRING="$(INF_VERSION)" Optional
> BUILD_NUM=$(BUILD_NUMBER)
>   }
> [Rule.Common.UEFI_APPLICATION.BINARY]
>   FILE APPLICATION = $(NAMED_GUID) {
> PE32  PE32|.efi
> UISTRING="$(MODULE_NAME)" Optional
> VERSION   STRING="$(INF_VERSION)" Optional
> BUILD_NUM=$(BUILD_NUMBER)
>   }
> 
> >
> > Note, however, that all of the drivers referenced so far in this thread
> > are licensed under the GPL. Thus, building an EFI in this way would
> > cause the EFI as a whole to be GPL-licensed.
> 
> Yes.
> 
> Also, as far as I'm aware, OpenSSL and GPL don't mix, so a firmware binary
> that combined edk2's OpenSSL-based Secure Boot stack and GPL drivers
> might be undistributable. (It would be fine for in-house use.)
> 
> https://people.gnome.org/~markmc/openssl-and-the-gpl.html
> 
> There's another option (and while it requires initial setup on the end-user's
> side, it doesn't require constant fiddling): copy the stand-alone, 
> GPL-licensed
> UEFI_DRIVER binary to a small, separate VFAT filesystem, then create a
> Driver option that points to it.
> 
> (At least with edk2 and OVMF, the driver options are dispatched between
> BeforeConsole and AfterConsole, and OVMF's AfterConsole calls
> EfiBootManagerConnectAll(). The result is that filesystems recognized by the
> drivers loaded from the small VFAT partition would all be bound, and would
> become available for booting off of.)
> 
> This VFAT + Driver setup could even be done by customized OS
> installers, without user interaction.
> 
> Now I understand that the point might be to eliminate a VFAT partition
> altogether, but after the initial driver installation, it could become 
> read-only
> from the OS runtime POV as well, and the real boot loader stuff could reside
> on ext2.
> 
> Separately, a small note on ext4 (because you mention it above). I seem to
> recall a filesystem expert colleague of mine advise *against* using journaled
> filesystems for booting with e.g. grub2. The argument goes (if I recall 
> right),
> XFS is considered to be in clean state if the data has made it to the final
> location *or* the persistent journal. When you cleanly unmount (or remount
> r/o) and shut down, the journal will be flushed to the final location, so a 
> boot
> loader that doesn't know about the journal will read consistent data.
> However, if you crash *without* data loss, then part of the data might be in
> the journal only, and only clients that can read the journal will see 
> consistent
> data.
> 
> > This might or might not be
> > an issue, depending on what the point of the exercise is.
> >
> > Of course,

Re: [edk2] [PATCH v3 0/2] UefiCpuPkg, OvmfPkg: set maximum (V)CPU count dynamically

[Laszlo Ersek]

On 11/28/16 20:24, Jordan Justen wrote:
> Series Reviewed-by: Jordan Justen 

Thanks Jordan! :) I hope Jeff too will accept this version.

Cheers!
Laszlo

> On 2016-11-28 05:08:37, Laszlo Ersek wrote:
>> This is the v3 update on
>> .
>>
>> Changes in this version [Jeff]:
>> - preserve original behavior for PcdCpuApInitTimeOutInMicroSeconds==0
>> - use MAX_UINT32 (~71 minutes) as "infinite wait"
>>
>> Repo:   https://github.com/lersek/edk2/
>> Branch: mpinit_v3
>>
>> Cc: Igor Mammedov 
>> Cc: Jeff Fan 
>> Cc: Jordan Justen 
>> Cc: Michael Kinney 
>>
>> Thanks
>> Laszlo
>>
>> Laszlo Ersek (2):
>>   UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP
>> startup
>>   OvmfPkg/PlatformPei: take VCPU count from QEMU and configure MpInitLib
>>
>>  OvmfPkg/OvmfPkgIa32.dsc  |  4 ++
>>  OvmfPkg/OvmfPkgIa32X64.dsc   |  4 ++
>>  OvmfPkg/OvmfPkgX64.dsc   |  4 ++
>>  OvmfPkg/PlatformPei/PlatformPei.inf  |  1 +
>>  OvmfPkg/PlatformPei/Platform.h   |  2 +
>>  OvmfPkg/PlatformPei/MemDetect.c  |  2 +-
>>  OvmfPkg/PlatformPei/Platform.c   | 43 
>>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 73 +++-
>>  8 files changed, 131 insertions(+), 2 deletions(-)
>>
>> -- 
>> 2.9.2
>>

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 0/2] UefiCpuPkg, OvmfPkg: set maximum (V)CPU count dynamically

[Jordan Justen]

Series Reviewed-by: Jordan Justen 

On 2016-11-28 05:08:37, Laszlo Ersek wrote:
> This is the v3 update on
> .
> 
> Changes in this version [Jeff]:
> - preserve original behavior for PcdCpuApInitTimeOutInMicroSeconds==0
> - use MAX_UINT32 (~71 minutes) as "infinite wait"
> 
> Repo:   https://github.com/lersek/edk2/
> Branch: mpinit_v3
> 
> Cc: Igor Mammedov 
> Cc: Jeff Fan 
> Cc: Jordan Justen 
> Cc: Michael Kinney 
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (2):
>   UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP
> startup
>   OvmfPkg/PlatformPei: take VCPU count from QEMU and configure MpInitLib
> 
>  OvmfPkg/OvmfPkgIa32.dsc  |  4 ++
>  OvmfPkg/OvmfPkgIa32X64.dsc   |  4 ++
>  OvmfPkg/OvmfPkgX64.dsc   |  4 ++
>  OvmfPkg/PlatformPei/PlatformPei.inf  |  1 +
>  OvmfPkg/PlatformPei/Platform.h   |  2 +
>  OvmfPkg/PlatformPei/MemDetect.c  |  2 +-
>  OvmfPkg/PlatformPei/Platform.c   | 43 
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 73 +++-
>  8 files changed, 131 insertions(+), 2 deletions(-)
> 
> -- 
> 2.9.2
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Error while doing Hello World on Ubuntu 16.04

[Jarlstrom, Laurie]

Hi 

It looks like there is an error with the "FILE_GUID" you have in the file 
MyHelloWorld.inf.
Make sure to copy and paste a new GUID from the http://www.guidgen.com/  in 
this file
INF_VERSION= 1.25
  BASE_NAME  = MyHelloWorld
  FILE_GUID  =   "Copy and paste GUID here"

Example:
FILE_GUID  =   05fe4409-66b3-4b0d-9acf-68a21a87186e


thanks,
Laurie
 
laurie.jarlst...@intel.com

Intel SSG/STO/EBP
(503) 712-9395


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jannis 
Ötjengerdes
Sent: Monday, November 28, 2016 3:35 AM
To: edk2-devel@lists.01.org
Subject: [edk2] Error while doing Hello World on Ubuntu 16.04

Hello,

i followed your guide which is described here: https://github.com/tianocore/ 
tianocore.github.io/wiki/Getting-Started-Writing-Simple-Application. When I try 
to build for X64 I get the following error which I should send to you:

(Python 2.7.12 on linux2) Traceback (most recent call last):
  File 
"/opt/edk2_new/BaseTools/BinWrappers/PosixLike/../../Source/Python/build/build.py",
line 2276, in Main
MyBuild.Launch()
  File 
"/opt/edk2_new/BaseTools/BinWrappers/PosixLike/../../Source/Python/build/build.py",
line 2028, in Launch
self._MultiThreadBuildPlatform()
  File 
"/opt/edk2_new/BaseTools/BinWrappers/PosixLike/../../Source/Python/build/build.py",
line 1858, in _MultiThreadBuildPlatform
Ma.CreateCodeFile(True)
  File "/opt/edk2_new/BaseTools/Source/Python/AutoGen/AutoGen.py", line 3997, 
in CreateCodeFile
for File in self.AutoGenFileList:
  File "/opt/edk2_new/BaseTools/Source/Python/AutoGen/AutoGen.py", line 3302, 
in _GetAutoGenFileList
GenC.CreateCode(self, AutoGenC, AutoGenH, StringH, UniStringAutoGenC, 
UniStringBinBuffer, StringIdf, IdfStringAutoGenC, IdfGenBinBuffer)
  File "/opt/edk2_new/BaseTools/Source/Python/AutoGen/GenC.py", line 1937, in 
CreateCode
CreateHeaderCode(Info, AutoGenC, AutoGenH)
  File "/opt/edk2_new/BaseTools/Source/Python/AutoGen/GenC.py", line 1895, in 
CreateHeaderCode
AutoGenH.Append("#define EFI_CALLER_ID_GUID \\\n  %s\n" %
GuidStringToGuidStructureString(Info.Guid))
  File "/opt/edk2_new/BaseTools/Source/Python/Common/Misc.py", line 297, in 
GuidStringToGuidStructureString
Result = Result + '0x' + GuidList[Index] + ', '
IndexError: list index out of range


- Failed -
Build end time: 12:30:42, Nov.28 2016
Build total time: 00:00:00
___
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/2] UefiCpuPkg, OvmfPkg: set maximum (V)CPU count dynamically

[Laszlo Ersek]

This is the v3 update on
.

Changes in this version [Jeff]:
- preserve original behavior for PcdCpuApInitTimeOutInMicroSeconds==0
- use MAX_UINT32 (~71 minutes) as "infinite wait"

Repo:   https://github.com/lersek/edk2/
Branch: mpinit_v3

Cc: Igor Mammedov 
Cc: Jeff Fan 
Cc: Jordan Justen 
Cc: Michael Kinney 

Thanks
Laszlo

Laszlo Ersek (2):
  UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP
startup
  OvmfPkg/PlatformPei: take VCPU count from QEMU and configure MpInitLib

 OvmfPkg/OvmfPkgIa32.dsc  |  4 ++
 OvmfPkg/OvmfPkgIa32X64.dsc   |  4 ++
 OvmfPkg/OvmfPkgX64.dsc   |  4 ++
 OvmfPkg/PlatformPei/PlatformPei.inf  |  1 +
 OvmfPkg/PlatformPei/Platform.h   |  2 +
 OvmfPkg/PlatformPei/MemDetect.c  |  2 +-
 OvmfPkg/PlatformPei/Platform.c   | 43 
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 73 +++-
 8 files changed, 131 insertions(+), 2 deletions(-)

-- 
2.9.2

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v3 1/2] UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP startup

[Laszlo Ersek]

Sometimes a platform knows exactly how many CPUs it has at boot. It should
be able to
- set PcdCpuMaxLogicalProcessorNumber dynamically to this number,
- set PcdCpuApInitTimeOutInMicroSeconds to a very long time (for example
  MAX_UINT32, approx. 71 minutes),
- and expect that MpInitLib wait exactly as long as necessary for all APs
  to report in.

Other platforms should be able to continue setting a reasonably large
upper bound on supported CPUs, and waiting for a reasonable, fixed amount
of time for all APs to report in.

Add this functionality. The TimedWaitForApFinish() function will return
when all APs have reported in, or the timeout has expired -- whichever
happens first.

(Accessing these PCDs dynamically is safe. The PEI and DXE phase instances
of this library are restricted to PEIM and DXE_DRIVER client modules, thus
the PCD accesses cannot be linked into runtime code.)

Cc: Igor Mammedov 
Cc: Jeff Fan 
Cc: Jordan Justen 
Cc: Michael Kinney 
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=116
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---

Notes:
v3:
- preserve original behavior for PcdCpuApInitTimeOutInMicroSeconds==0
  [Jeff]

 UefiCpuPkg/Library/MpInitLib/MpLib.c | 73 +++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 15dbfa1e7d6c..ed22ce67782e 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -684,6 +684,21 @@ FillExchangeInfoData (
 }
 
 /**
+  Helper function that waits until the finished AP count reaches the specified
+  limit, or the specified timeout elapses (whichever comes first).
+
+  @param[in] CpuMpDataPointer to CPU MP Data.
+  @param[in] FinishedApLimit  The number of finished APs to wait for.
+  @param[in] TimeLimitThe number of microseconds to wait for.
+**/
+VOID
+TimedWaitForApFinish (
+  IN CPU_MP_DATA   *CpuMpData,
+  IN UINT32FinishedApLimit,
+  IN UINT32TimeLimit
+  );
+
+/**
   This function will be called by BSP to wakeup AP.
 
   @param[in] CpuMpData  Pointer to CPU MP Data
@@ -748,7 +763,11 @@ WakeUpAP (
   //
   // Wait for all potential APs waken up in one specified period
   //
-  MicroSecondDelay (PcdGet32(PcdCpuApInitTimeOutInMicroSeconds));
+  TimedWaitForApFinish (
+CpuMpData,
+PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1,
+PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds)
+);
 } else {
   //
   // Wait all APs waken up if this is not the 1st broadcast of SIPI
@@ -895,6 +914,58 @@ CheckTimeout (
 }
 
 /**
+  Helper function that waits until the finished AP count reaches the specified
+  limit, or the specified timeout elapses (whichever comes first).
+
+  @param[in] CpuMpDataPointer to CPU MP Data.
+  @param[in] FinishedApLimit  The number of finished APs to wait for.
+  @param[in] TimeLimitThe number of microseconds to wait for.
+**/
+VOID
+TimedWaitForApFinish (
+  IN CPU_MP_DATA   *CpuMpData,
+  IN UINT32FinishedApLimit,
+  IN UINT32TimeLimit
+  )
+{
+  //
+  // CalculateTimeout() and CheckTimeout() consider a TimeLimit of 0
+  // "infinity", so check for (TimeLimit == 0) explicitly.
+  //
+  if (TimeLimit == 0) {
+return;
+  }
+
+  CpuMpData->TotalTime = 0;
+  CpuMpData->ExpectedTime = CalculateTimeout (
+  TimeLimit,
+  &CpuMpData->CurrentTime
+  );
+  while (CpuMpData->FinishedCount < FinishedApLimit &&
+ !CheckTimeout (
+&CpuMpData->CurrentTime,
+&CpuMpData->TotalTime,
+CpuMpData->ExpectedTime
+)) {
+CpuPause ();
+  }
+
+  if (CpuMpData->FinishedCount >= FinishedApLimit) {
+DEBUG ((
+  DEBUG_VERBOSE,
+  "%a: reached FinishedApLimit=%u in %Lu microseconds\n",
+  __FUNCTION__,
+  FinishedApLimit,
+  DivU64x64Remainder (
+MultU64x32 (CpuMpData->TotalTime, 100),
+GetPerformanceCounterProperties (NULL, NULL),
+NULL
+)
+  ));
+  }
+}
+
+/**
   Reset an AP to Idle state.
 
   Any task being executed by the AP will be aborted and the AP
-- 
2.9.2


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v3 2/2] OvmfPkg/PlatformPei: take VCPU count from QEMU and configure MpInitLib

[Laszlo Ersek]

These settings will allow CpuMpPei and CpuDxe to wait for the initial AP
check-ins exactly as long as necessary.

It is safe to set PcdCpuMaxLogicalProcessorNumber and
PcdCpuApInitTimeOutInMicroSeconds in OvmfPkg/PlatformPei.
OvmfPkg/PlatformPei installs the permanent PEI RAM, producing
gEfiPeiMemoryDiscoveredPpiGuid, and UefiCpuPkg/CpuMpPei has a depex on
gEfiPeiMemoryDiscoveredPpiGuid.

It is safe to read the fw_cfg item QemuFwCfgItemSmpCpuCount (0x0005). It
was added to QEMU in 2008 as key FW_CFG_NB_CPUS, in commit 905fdcb5264c
("Add common keys to firmware configuration"). Even if the key is
unavailable (or if fw_cfg is entirely unavailable, for example on Xen),
QemuFwCfgRead16() will return 0, and then we stick with the current
behavior.

Cc: Igor Mammedov 
Cc: Jeff Fan 
Cc: Jordan Justen 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---

Notes:
v3:
- We now preserve the original behavior for
  PcdCpuApInitTimeOutInMicroSeconds==0 in UefiCpuPkg/MpInitLib, in the
  previous patch, so represent the "infinite" timeout with the longest
  representable timeout (MAX_UINT32 microseconds, cca. 71 minutes)
  [Jeff]

 OvmfPkg/OvmfPkgIa32.dsc |  4 ++
 OvmfPkg/OvmfPkgIa32X64.dsc  |  4 ++
 OvmfPkg/OvmfPkgX64.dsc  |  4 ++
 OvmfPkg/PlatformPei/PlatformPei.inf |  1 +
 OvmfPkg/PlatformPei/Platform.h  |  2 +
 OvmfPkg/PlatformPei/MemDetect.c |  2 +-
 OvmfPkg/PlatformPei/Platform.c  | 43 
 7 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index ed43c4514491..d91303052263 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -488,6 +488,10 @@ [PcdsDynamicDefault]
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE
 
+  # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|5
+
 

 #
 # Components Section - list of all EDK II Modules needed by this Platform.
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index ccd156d9231a..8143ea9c2837 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -496,6 +496,10 @@ [PcdsDynamicDefault]
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE
 
+  # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|5
+
 

 #
 # Components Section - list of all EDK II Modules needed by this Platform.
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 012ce85462c5..d48d603d33d4 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -495,6 +495,10 @@ [PcdsDynamicDefault]
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE
 
+  # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|5
+
 

 #
 # Components Section - list of all EDK II Modules needed by this Platform.
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
b/OvmfPkg/PlatformPei/PlatformPei.inf
index 776a4ab11f79..fbaed3182dcf 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -95,6 +95,7 @@ [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable
   gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize
 
 [FixedPcd]
diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
index eda765be30de..18f42c3f0ea8 100644
--- a/OvmfPkg/PlatformPei/Platform.h
+++ b/OvmfPkg/PlatformPei/Platform.h
@@ -101,4 +101,6 @@ extern BOOLEAN mS3Supported;
 
 extern UINT8 mPhysMemAddressWidth;
 
+extern UINT32 mMaxCpuCount;
+
 #endif // _PLATFORM_PEI_H_INCLUDED_
diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index d00a570d4381..af96a04d194a 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -358,7 +358,7 @@ PublishPeiMemory (
   //
   if (mS3Supported) {
 mS3AcpiReservedMemorySize = SIZE_512KB +
-  PcdGet32 (PcdCpuMaxLogicalProcessorNumber) *
+  mMaxCpuCount *
   PcdGet32 (PcdCpuAp

Re: [edk2] [PATCH v2 2/3] UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP startup

[Igor Mammedov]

On Mon, 28 Nov 2016 11:15:15 +0100
Laszlo Ersek  wrote:

> On 11/28/16 06:37, Fan, Jeff wrote:
> > Laszlo,
> > 
> > Setting PcdCpuApInitTimeOutInMicroSeconds to infinity (marked with 0) is 
> > behavior changing. 
> > 
> > Even there is no any multi-processor supporting platform set this PCD to 
> > zero, this PCD maybe set to 0 on single processor platform.  
> 
> On a uniprocessor platform, PcdCpuApInitTimeOutInMicroSeconds makes no
> difference, since the code that uses the PCD is not reached, due to
> commit 14e8137c8223 ("UefiCpuPkg/MpInitLib: Do not wakeup AP if only one
> processor supported").
> 
> > I prefer to setting PcdCpuApInitTimeOutInMicroSeconds to 0x (about 
> > 1.1 hour) for such purpose.  
> 
> In practice that is good enough for OVMF too, I guess.
> 
> However, I'm unsure how you would like me to update the patch. The patch
> does not add any specific code for handling the 0 timeout as infinity.
> Instead, it simply documents the existing behavior of CalculateTimeout()
> and CheckTimeout(). Those helper functions are just right for
> implementing the feature at hand, and they already handle a zero timeout
> as infinity.
> 
> In fact, my original approach to implement TimedWaitForApFinish() was to
> add brand new code for the timer loop, using the performance counter.
> However then I noticed CalculateTimeout() and CheckTimeout(), and
> thought that you would ask me to implement TimedWaitForApFinish() with
> those two functions. This way the code reuse is good, and a zero timeout
> happens to mean infinity (which is just right for OVMF, is not used by
> any physical multiprocessor platform, and is irrelevant for uniprocessor).
> 
> ... Hm, wait, do you mean that a uniprocessor platform might leave
> PcdCpuMaxLogicalProcessorNumber > 1 (for example, the default 64), and
> set *only* the timeout to zero (because they "know" there aren't more CPUs)?
> 
> If that's what you mean, I consider it a platform bug: if they know
> there's only one CPU, they should set PcdCpuMaxLogicalProcessorNumber to
> 1 -- for saving memory.
In my understanding PcdCpuMaxLogicalProcessorNumber is max possible CPUs
number and not currently present CPUs which platform might not know about
in advance, so it sends broadcast INIT/SIPI to find out how many CPUs are
present upto PcdCpuMaxLogicalProcessorNumber limit.

It's just we are cannibalizing PcdCpuMaxLogicalProcessorNumber for present
CPUs number as it were suggested in v1 comments to make some progress
and fix OVMF crash when booting with more than 64 CPU
and optimize memory usage/boot and resume time.

I'd prefer v1 where PDCs were used the way they've been meant to but
this route also sufficient as intermediate step as it allows to boot
with more than 64 CPUs.

> 
> Anyway, if you prefer, I can modify TimedWaitForApFinish() like this:
> add an explicit check for TimeLimit==0 in the beginning, and return
> immediately if that condition evaluates to true. (Plus, update the
> comments, and modify patch #3 to use MAX_UINT32 as TimeLinit.)
> 
> What do you think?
> 
> Thanks
> Laszlo
> 
> > 
> > Thanks!
> > Jeff
> > 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> > Laszlo Ersek
> > Sent: Friday, November 25, 2016 4:57 AM
> > To: edk2-devel-01
> > Cc: Kinney, Michael D; Fan, Jeff; Justen, Jordan L
> > Subject: [edk2] [PATCH v2 2/3] UefiCpuPkg/MpInitLib: wait no longer than 
> > necessary for initial AP startup
> > 
> > Sometimes a platform knows exactly how many CPUs it has at boot. It should 
> > be able to
> > - set PcdCpuMaxLogicalProcessorNumber dynamically to this number,
> > - set PcdCpuApInitTimeOutInMicroSeconds to infinity (marked with 0),
> > - and expect that MpInitLib wait exactly as long as necessary for all APs
> >   to report in.
> > 
> > Other platforms should be able to continue setting a reasonably large upper 
> > bound on supported CPUs, and waiting for a reasonable, fixed amount of time 
> > for all APs to report in.
> > 
> > Add this functionality. The TimedWaitForApFinish() function will return 
> > when all APs have reported in, or the timeout has expired -- whichever 
> > happens first.
> > 
> > (Accessing these PCDs dynamically is safe. The PEI and DXE phase instances 
> > of this library are restricted to PEIM and DXE_DRIVER client modules, thus 
> > the PCD accesses cannot be linked into runtime code.)
> > 
> > Cc: Igor Mammedov 
> > Cc: Jeff Fan 
> > Cc: Jordan Justen 
> > Cc: Michael Kinney 
> > Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=116
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Laszlo Ersek 
> > ---
> >  UefiCpuPkg/UefiCpuPkg.dec|  3 +-
> >  UefiCpuPkg/Library/MpInitLib/MpLib.c | 67 +++-
> >  2 files changed, 68 insertions(+), 2 deletions(-)
> > 
> > diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec index 
> > ca560398bbef..8607ae8b7ae8 100644
> > --- a/UefiCpuPkg/Uefi

[edk2] Error while doing Hello World on Ubuntu 16.04

[Jannis Ötjengerdes]

Hello,

i followed your guide which is described here: https://github.com/tianocore/
tianocore.github.io/wiki/Getting-Started-Writing-Simple-Application. When I
try to build for X64 I get the following error which I should send to you:

(Python 2.7.12 on linux2) Traceback (most recent call last):
  File 
"/opt/edk2_new/BaseTools/BinWrappers/PosixLike/../../Source/Python/build/build.py",
line 2276, in Main
MyBuild.Launch()
  File 
"/opt/edk2_new/BaseTools/BinWrappers/PosixLike/../../Source/Python/build/build.py",
line 2028, in Launch
self._MultiThreadBuildPlatform()
  File 
"/opt/edk2_new/BaseTools/BinWrappers/PosixLike/../../Source/Python/build/build.py",
line 1858, in _MultiThreadBuildPlatform
Ma.CreateCodeFile(True)
  File "/opt/edk2_new/BaseTools/Source/Python/AutoGen/AutoGen.py", line
3997, in CreateCodeFile
for File in self.AutoGenFileList:
  File "/opt/edk2_new/BaseTools/Source/Python/AutoGen/AutoGen.py", line
3302, in _GetAutoGenFileList
GenC.CreateCode(self, AutoGenC, AutoGenH, StringH, UniStringAutoGenC,
UniStringBinBuffer, StringIdf, IdfStringAutoGenC, IdfGenBinBuffer)
  File "/opt/edk2_new/BaseTools/Source/Python/AutoGen/GenC.py", line 1937,
in CreateCode
CreateHeaderCode(Info, AutoGenC, AutoGenH)
  File "/opt/edk2_new/BaseTools/Source/Python/AutoGen/GenC.py", line 1895,
in CreateHeaderCode
AutoGenH.Append("#define EFI_CALLER_ID_GUID \\\n  %s\n" %
GuidStringToGuidStructureString(Info.Guid))
  File "/opt/edk2_new/BaseTools/Source/Python/Common/Misc.py", line 297, in
GuidStringToGuidStructureString
Result = Result + '0x' + GuidList[Index] + ', '
IndexError: list index out of range


- Failed -
Build end time: 12:30:42, Nov.28 2016
Build total time: 00:00:00
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: handle dynamic PcdCpuMaxLogicalProcessorNumber

[Laszlo Ersek]

On 11/28/16 03:15, Fan, Jeff wrote:
> Laszlo,
> 
> Even getting PcdCpuMaxLogicalProcessorNumber is legal in 
> InitSmmProfileInternal() that is only invoked on boot time, I agree to do 
> this updating in this patch.
> We need to try to use mMaxNumberOfCpus as possible for size saving and 
> performance improvement.
> 
> Reviewed-by: Jeff Fan 

Thanks, I committed this patch (for PiSmmCpuDxeSmm) as bb767506b265.
I'll submit a new version for the rest.

Cheers
Laszlo

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com] 
> Sent: Friday, November 25, 2016 4:57 AM
> To: edk2-devel-01
> Cc: Igor Mammedov; Fan, Jeff; Justen, Jordan L; Kinney, Michael D
> Subject: [PATCH v2 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: handle dynamic 
> PcdCpuMaxLogicalProcessorNumber
> 
> "UefiCpuPkg/UefiCpuPkg.dec" already allows platforms to make 
> PcdCpuMaxLogicalProcessorNumber dynamic, however PiSmmCpuDxeSmm does not take 
> this into account everywhere. As soon as a platform turns the PCD into a 
> dynamic one, at least S3 fails. When the PCD is dynamic, all
> PcdGet() calls translate into PCD DXE protocol calls, which are only 
> permitted at boot time, not at runtime or during S3 resume.
> 
> We already have a variable called mMaxNumberOfCpus; it is initialized in the 
> entry point function like this:
> 
>> //
>> // If support CPU hot plug, we need to allocate resources for possibly 
>> // hot-added processors // if (FeaturePcdGet (PcdCpuHotPlugSupport)) {
>>   mMaxNumberOfCpus = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
>> } else {
>>   mMaxNumberOfCpus = mNumberOfCpus;
>> }
> 
> There's another use of the PCD a bit higher up, also in the entry point
> function:
> 
>> //
>> // Use MP Services Protocol to retrieve the number of processors and 
>> // number of enabled processors // Status = 
>> MpServices->GetNumberOfProcessors (MpServices, &mNumberOfCpus,
>>&NumberOfEnabledProcessors); ASSERT_EFI_ERROR 
>> (Status); ASSERT (mNumberOfCpus <= PcdGet32 
>> (PcdCpuMaxLogicalProcessorNumber));
> 
> Preserve these calls in the entry point function, and replace all other uses 
> of PcdCpuMaxLogicalProcessorNumber -- there are only reads -- with 
> mMaxNumberOfCpus.
> 
> For PcdCpuHotPlugSupport==TRUE, this is an unobservable change.
> 
> For PcdCpuHotPlugSupport==FALSE, we even save SMRAM, because we no longer 
> allocate resources needlessly for CPUs that can never appear in the system.
> 
> PcdCpuMaxLogicalProcessorNumber is also retrieved in 
> "UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c",  but only in the 
> library instance constructor, which runs even before the entry point function 
> is called.
> 
> Cc: Igor Mammedov 
> Cc: Jeff Fan 
> Cc: Jordan Justen 
> Cc: Michael Kinney 
> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=116
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c |  2 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 18 +-
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> index 4baba1e9f8dc..f957de1f4764 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> @@ -344,7 +344,7 @@ SmmInitHandler (
>AsmWriteIdtr (&gcSmiIdtr);
>ApicId = GetApicId ();
>  
> -  ASSERT (mNumberOfCpus <= PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
> +  ASSERT (mNumberOfCpus <= mMaxNumberOfCpus);
>  
>for (Index = 0; Index < mNumberOfCpus; Index++) {
>  if (ApicId == (UINT32)gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId) 
> { diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> index c1a48d100e0f..f53819ee24c2 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> @@ -139,7 +139,7 @@ GetCpuIndex (
>  
>ApicId = GetApicId ();
>  
> -  for (Index = 0; Index < PcdGet32 (PcdCpuMaxLogicalProcessorNumber); 
> Index++) {
> +  for (Index = 0; Index < mMaxNumberOfCpus; Index++) {
>  if (gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId == ApicId) {
>return Index;
>  }
> @@ -825,13 +825,13 @@ InitSmmProfileInternal (
>UINTN  MsrDsAreaSizePerCpu;
>UINTN  TotalSize;
>  
> -  mPFEntryCount = (UINTN *)AllocateZeroPool (sizeof (UINTN) * PcdGet32 
> (PcdCpuMaxLogicalProcessorNumber));
> +  mPFEntryCount = (UINTN *)AllocateZeroPool (sizeof (UINTN) * 
> + mMaxNumberOfCpus);
>ASSERT (mPFEntryCount != NULL);
>mLastPFEntryValue = (UINT64  (*)[MAX_PF_ENTRY_COUNT])AllocateZeroPool (
> - sizeof 
> (mLastPFEntryValue[0]) * PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
> + sizeof 
> + (mLastPFEntryValue[0]) * mMaxNumberOfCpus);
>

Re: [edk2] [PATCH v2 2/3] UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP startup

[Laszlo Ersek]

On 11/28/16 06:37, Fan, Jeff wrote:
> Laszlo,
> 
> Setting PcdCpuApInitTimeOutInMicroSeconds to infinity (marked with 0) is 
> behavior changing. 
> 
> Even there is no any multi-processor supporting platform set this PCD to 
> zero, this PCD maybe set to 0 on single processor platform.

On a uniprocessor platform, PcdCpuApInitTimeOutInMicroSeconds makes no
difference, since the code that uses the PCD is not reached, due to
commit 14e8137c8223 ("UefiCpuPkg/MpInitLib: Do not wakeup AP if only one
processor supported").

> I prefer to setting PcdCpuApInitTimeOutInMicroSeconds to 0x (about 
> 1.1 hour) for such purpose.

In practice that is good enough for OVMF too, I guess.

However, I'm unsure how you would like me to update the patch. The patch
does not add any specific code for handling the 0 timeout as infinity.
Instead, it simply documents the existing behavior of CalculateTimeout()
and CheckTimeout(). Those helper functions are just right for
implementing the feature at hand, and they already handle a zero timeout
as infinity.

In fact, my original approach to implement TimedWaitForApFinish() was to
add brand new code for the timer loop, using the performance counter.
However then I noticed CalculateTimeout() and CheckTimeout(), and
thought that you would ask me to implement TimedWaitForApFinish() with
those two functions. This way the code reuse is good, and a zero timeout
happens to mean infinity (which is just right for OVMF, is not used by
any physical multiprocessor platform, and is irrelevant for uniprocessor).

... Hm, wait, do you mean that a uniprocessor platform might leave
PcdCpuMaxLogicalProcessorNumber > 1 (for example, the default 64), and
set *only* the timeout to zero (because they "know" there aren't more CPUs)?

If that's what you mean, I consider it a platform bug: if they know
there's only one CPU, they should set PcdCpuMaxLogicalProcessorNumber to
1 -- for saving memory.

Anyway, if you prefer, I can modify TimedWaitForApFinish() like this:
add an explicit check for TimeLimit==0 in the beginning, and return
immediately if that condition evaluates to true. (Plus, update the
comments, and modify patch #3 to use MAX_UINT32 as TimeLinit.)

What do you think?

Thanks
Laszlo

> 
> Thanks!
> Jeff
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
> Ersek
> Sent: Friday, November 25, 2016 4:57 AM
> To: edk2-devel-01
> Cc: Kinney, Michael D; Fan, Jeff; Justen, Jordan L
> Subject: [edk2] [PATCH v2 2/3] UefiCpuPkg/MpInitLib: wait no longer than 
> necessary for initial AP startup
> 
> Sometimes a platform knows exactly how many CPUs it has at boot. It should be 
> able to
> - set PcdCpuMaxLogicalProcessorNumber dynamically to this number,
> - set PcdCpuApInitTimeOutInMicroSeconds to infinity (marked with 0),
> - and expect that MpInitLib wait exactly as long as necessary for all APs
>   to report in.
> 
> Other platforms should be able to continue setting a reasonably large upper 
> bound on supported CPUs, and waiting for a reasonable, fixed amount of time 
> for all APs to report in.
> 
> Add this functionality. The TimedWaitForApFinish() function will return when 
> all APs have reported in, or the timeout has expired -- whichever happens 
> first.
> 
> (Accessing these PCDs dynamically is safe. The PEI and DXE phase instances of 
> this library are restricted to PEIM and DXE_DRIVER client modules, thus the 
> PCD accesses cannot be linked into runtime code.)
> 
> Cc: Igor Mammedov 
> Cc: Jeff Fan 
> Cc: Jordan Justen 
> Cc: Michael Kinney 
> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=116
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek 
> ---
>  UefiCpuPkg/UefiCpuPkg.dec|  3 +-
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 67 +++-
>  2 files changed, 68 insertions(+), 2 deletions(-)
> 
> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec index 
> ca560398bbef..8607ae8b7ae8 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dec
> +++ b/UefiCpuPkg/UefiCpuPkg.dec
> @@ -174,7 +174,8 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, 
> PcdsDynamicEx]
># @Prompt Configure max supported number of Logical Processors
>
> gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64|UINT32|0x0002
>## Specifies timeout value in microseconds for the BSP to detect all APs 
> for the first time.
> -  # @Prompt Timeout for the BSP to detect all APs for the first time.
> +  #  When set to zero, the BSP will wait forever for all 
> (PcdCpuMaxLogicalProcessorNumber-1) APs to report in.
> +  # @Prompt Timeout for the BSP to detect all APs for the first time (zero 
> means await maximum supported AP count).
>
> gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|5|UINT32|0x0004
>## Specifies the base address of the first microcode Patch in the 
> microcode Region.
># @Prompt Microcode Region base address.
> diff