Re: [edk2] Reg: Intel Rangley Support in EDK

2018-07-17 Thread You, Benjamin
Hi, Dhanasekar,

Linux boot parameters allow specifying serial baudrate. An example would be: 
earlycon=uart8250,mmio32,0x,115200n81 console=ttyS2,115200

Please make sure the serial device info (including MMIO or I/O address and
 the device name) matches your platform in above settings.

I guess character set is not an issue here since Linux only outputs basic 
ASCII codes.

Thanks,

- ben

> From: Dhanasekar Jaganathan [mailto:jdhanasekar...@gmail.com] 
> Sent: Tuesday, July 17, 2018 9:32 PM
> To: You, Benjamin 
> Cc: Desimone, Nathaniel L ; 
> edk2-devel@lists.01.org
> Subject: Re: [edk2] Reg: Intel Rangley Support in EDK
> 
> Hi Nate, Ben,
> 
> After changing   Windows -> Translation = CP437 in Putty, I can able to see 
> proper Shell and BIOS setup. 
> After Grub, I am not seeing OS installation Process. I have changed Kernal 
> Parameters too.  
> 
> May Linux Server Installation(via serial) use different character setting or 
> baud rate?.
> 
> Thanks,
> Dhanasekar
> 
> On Mon, Jul 16, 2018 at 10:58 AM, Dhanasekar Jaganathan 
>  wrote:
> Hi Nate/ Ben,
> 
> Thanks for the info. 
> 
> My host is Ubuntu system and I am using "minicom" for serial messages. 
> I will use putty with the values you mentioned.
> I have been trying with different Kernel parameters  (in grub.cfg) while 
> installing Ubuntu Server OS. 
> 
> I will update the result. 
> 
> Thanks,
> Dhanasekar
> 
> On Sat, Jul 14, 2018 at 10:48 AM, You, Benjamin  
> wrote:
> Hi Dhanasekar,
> 
> The Coreboot UEFI Payload does support serial and terminal. As Nate said 
> below, there may be some configuration issue causing the display not correctly
> formatted through serial terminal.
> 
> I also agree with Nate that since your board does not have a VGA connection, 
> you might have to configure Ubuntu to use serial console to monitor the boot 
> process.
> 
> Thanks,
> 
> - ben
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Desimone, Nathaniel L
> > Sent: Saturday, July 14, 2018 8:51 AM
> > To: Dhanasekar Jaganathan 
> > Cc: edk2-devel@lists.01.org
> > Subject: Re: [edk2] Reg: Intel Rangley Support in EDK
> > 
> > Hi Dhanasekar,
> > 
> > I don't know very much about coreboot or the coreboot UEFI payload, so I
> > won't be able to help you much with that. From what you mention in your
> > message it sounds like either the coreboot UEFI payload does not include the
> > TerminalDxe driver, there is a configuration issue with it. MinPlatformPkg 
> > is a
> > pure EDK2 only implementation that does not have any coreboot dependencies,
> > but it does not support Rangley at the moment.
> > 
> > With regard to the setup menu graphical corruption, the setup menu uses the
> > CP437 character set, since the setup menu is often displayed using VGA text
> > mode, whereas most current terminal emulators assume UTF-8 encoding. To
> > make it render properly, use PuTTY and in the connection configuration under
> > Windows -> Translation change the remote character set to CP437.
> > 
> > I suspect there are kernel parameters you will need to pass to make Ubuntu 
> > use
> > the serial port as the terminal, I suggest looking at Ubuntu documentation 
> > or
> > message boards.
> > 
> > Thanks,
> > 
> > Nate
> > 
> > On 7/12/18, 10:21 PM, "edk2-devel on behalf of Dhanasekar Jaganathan"
> > 
> > wrote:
> > 
> > Hi Nate,
> > 
> > Thanks for the info. I will try this.
> > 
> > My Rangley platform has no onboard or offboard vga support. Com Port / 
> > Serial
> > console is used for display and communication.
> > I am booting the platform by coreboot with UEFI payload. I am trying to 
> > install
> > Ubuntu server OS.
> > 
> > When I boot into Shell, I am not seeing actually shell (graphics are not 
> > good) and
> > keystroke are working properly.
> > When I boot into EDK Setup, I am not getting proper setup page (graphics are
> > not good) and keystroke are not working.
> > 
> > When I try to boot Ubuntu Server, I am getting below error,
> > 
> > "error: no suitable video mode found.
> > Booting in blind mode"
> > 
> > It seems I am missing some video/graphics settings in UEFI payload. If you 
> > know
> > the fix, Please provide me.
> > 
> > Thanks,
> > Dhanasekar
> > 
> > 
> > 
> > On Fri, Jul 13, 2018 at 7:14 AM, Desimone, Nathaniel L
> >  wrote:
> > Hi Dhanasekar,
> > 
> > There is nothing pre-built and off-the-shelf ready today. But we do have a
> > generalized infrastructure for open source Intel UEFI platforms called
> > MinPlatformPkg. Please see the following:
> > 
> > https://github.com/tianocore/edk2-platforms/tree/devel-
> > MinPlatform/Platform/Intel
> > 
> > Notice that MinPlatformPkg requires a *OpenBoardPkg that supports the
> > desired platform. To my knowledge no one at Intel has looked at implementing
> > a RangleyOpenBoardPkg thus far. The focus has been on recently released
> > platforms, Rangley is 4 years old at this point. If you are so inclined, 
> > you are
> > 

[edk2] [Patch v2] UefiCpuPkg/MpInitLib: Optimize get processor number performance.

2018-07-17 Thread Eric Dong
Current function has low performance because it calls GetApicId
many times.

New logic call GetApicId once and base on this value to search
the processor.

V2 changes:
Rollback V1 change which base on stack info to get AP index because
this solution may return error AP index if stack buffer overflow.

Cc: Ruiyu Ni 
Cc: Jeff Fan 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 722db2a01f..0bb0582985 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -435,16 +435,19 @@ GetProcessorNumber (
   UINTN   TotalProcessorNumber;
   UINTN   Index;
   CPU_INFO_IN_HOB *CpuInfoInHob;
+  UINT32  CurrentApicId;
 
   CpuInfoInHob = (CPU_INFO_IN_HOB *) (UINTN) CpuMpData->CpuInfoInHob;
 
   TotalProcessorNumber = CpuMpData->CpuCount;
+  CurrentApicId = GetApicId ();
   for (Index = 0; Index < TotalProcessorNumber; Index ++) {
-if (CpuInfoInHob[Index].ApicId == GetApicId ()) {
+if (CpuInfoInHob[Index].ApicId == CurrentApicId) {
   *ProcessorNumber = Index;
   return EFI_SUCCESS;
 }
   }
+
   return EFI_NOT_FOUND;
 }
 
-- 
2.15.0.windows.1

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


[edk2] UefiCpuPkg/MpInitLib: Fix S3 resume hang issue.

2018-07-17 Thread Eric Dong
When resume from S3 and CPU loop mode is MWait mode,
if driver calls APs to do task at EndOfPei point, the
APs can't been wake up and bios hang at that point.

The root cause is PiSmmCpuDxeSmm driver wakes up APs
with HLT mode during S3 resume phase to do SMM relocation.
After this task, PiSmmCpuDxeSmm driver not restore APs
context which make CpuMpPei driver saved wake up buffer
not works.

The solution for this issue is let CpuMpPei driver hook
S3SmmInitDone ppi notification. In this notify function,
it check whether Cpu Loop mode is not HLT mode. If yes,
CpuMpPei driver will set a flag to force BSP use INIT-SIPI
-SIPI command to wake up the APs.

Cc: Laszlo Ersek 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c  | 16 -
 UefiCpuPkg/Library/MpInitLib/MpLib.h  |  9 +++
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |  4 ++
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c   | 89 +++
 4 files changed, 116 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 722db2a01f..e5c701ddeb 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -985,13 +985,15 @@ WakeUpAP (
   CpuMpData->FinishedCount = 0;
   ResetVectorRequired = FALSE;
 
-  if (CpuMpData->ApLoopMode == ApInHltLoop ||
+  if (CpuMpData->WakeUpByInitSipiSipi ||
   CpuMpData->InitFlag   != ApInitDone) {
 ResetVectorRequired = TRUE;
 AllocateResetVector (CpuMpData);
 FillExchangeInfoData (CpuMpData);
 SaveLocalApicTimerSetting (CpuMpData);
-  } else if (CpuMpData->ApLoopMode == ApInMwaitLoop) {
+  }
+
+  if (CpuMpData->ApLoopMode == ApInMwaitLoop) {
 //
 // Get AP target C-state each time when waking up AP,
 // for it maybe updated by platform again
@@ -1076,6 +1078,13 @@ WakeUpAP (
   if (ResetVectorRequired) {
 FreeResetVector (CpuMpData);
   }
+
+  //
+  // After one round of Wakeup Ap actions, need to re-sync ApLoopMode with
+  // WakeUpByInitSipiSipi flag. WakeUpByInitSipiSipi flag maybe changed by
+  // S3SmmInitDone Ppi.
+  //
+  CpuMpData->WakeUpByInitSipiSipi = (CpuMpData->ApLoopMode == ApInHltLoop);
 }
 
 /**
@@ -1648,6 +1657,9 @@ MpInitLibInitialize (
   //
   CpuMpData->ApLoopMode = ApLoopMode;
   DEBUG ((DEBUG_INFO, "AP Loop Mode is %d\n", CpuMpData->ApLoopMode));
+
+  CpuMpData->WakeUpByInitSipiSipi = (CpuMpData->ApLoopMode == ApInHltLoop);
+
   //
   // Set up APs wakeup signal buffer
   //
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 6958080ac1..9d0b866d09 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -250,6 +250,15 @@ struct _CPU_MP_DATA {
   UINT32 ProcessorFlags;
   UINT64 MicrocodeDataAddress;
   UINT32 MicrocodeRevision;
+
+  //
+  // Whether need to use Init-Sipi-Sipi to wake up the APs.
+  // Two cases need to set this value to TRUE. One is in HLT
+  // loop mode, the other is resume from S3 which loop mode
+  // will be hardcode change to HLT mode by PiSmmCpuDxeSmm 
+  // driver.
+  //
+  BOOLEANWakeUpByInitSipiSipi;
 };
 
 extern EFI_GUID mCpuInitMpLibHobGuid;
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf 
b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index fa84e392af..43a3b3b036 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -44,6 +44,7 @@
 [Packages]
   MdePkg/MdePkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
 
 [LibraryClasses]
   BaseLib
@@ -54,6 +55,7 @@
   CpuLib
   UefiCpuLib
   SynchronizationLib
+  PeiServicesLib
 
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber## CONSUMES
@@ -64,3 +66,5 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode   ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate   ## 
SOMETIMES_CONSUMES
 
+[Guids]
+  gEdkiiS3SmmInitDoneGuid
\ No newline at end of file
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c 
b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 92f28681e4..06d966b227 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -13,6 +13,87 @@
 **/
 
 #include "MpLib.h"
+#include 
+#include 
+
+/**
+  S3 SMM Init Done notification function.
+
+  @param  PeiServices  Indirect reference to the PEI Services Table.
+  @param  NotifyDesc   Address of the notification descriptor data 
structure.
+  @param  InvokePpiAddress of the PPI that was invoked.
+
+  @retval EFI_SUCCESS  The function completes successfully.
+
+**/
+EFI_STATUS
+EFIAPI
+NotifyOnS3SmmInitDonePpi (
+  IN  EFI_PEI_SERVICES  **PeiServices,
+  IN  EFI_PEI_NOTIFY_DESCRIPTOR 

Re: [edk2] [PATCH 1/1] MdeModulePkg/PciBusDxe: Fix small memory leak in FreePciDevice

2018-07-17 Thread Ni, Ruiyu

On 7/3/2018 11:32 PM, Thomas Palmer wrote:

When cleaning the PciIoDevice, also free the BusNumberRange

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Thomas Palmer 
---
  MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
index ad7a2337f578..48cf57a24f8f 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
@@ -2,6 +2,7 @@
Supporting functions implementaion for PCI devices management.
  
  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.

+(C) Copyright 2018 Hewlett Packard Enterprise Development LP
  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
@@ -105,6 +106,10 @@ FreePciDevice (
  FreePool (PciIoDevice->DevicePath);
}
  
+  if (PciIoDevice->BusNumberRanges != NULL) {

+FreePool (PciIoDevice->BusNumberRanges);
+  }
+
FreePool (PciIoDevice);
  }
  


Reviewed-by: Ruiyu Ni 

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


Re: [edk2] [Patch] BaseTools: Remove the duplicate Pcd items

2018-07-17 Thread Gao, Liming
Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Yonghong Zhu
> Sent: Tuesday, July 17, 2018 10:04 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch] BaseTools: Remove the duplicate Pcd items
> 
> The case is the Pcd item both used in 1 module inf and 1 lib inf, and
> in the DSC component section, it override the Pcd value.
> In the module, the pcd value is the override value, but in the lib inf
> the pcd value is the value that in the DSC PCD section's value, then it
> cause the Pcd value is different in the module and lib. but actually we
> only need use the Pcd value in the module to decide whether it use the
> same value.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/AutoGen/AutoGen.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
> b/BaseTools/Source/Python/AutoGen/AutoGen.py
> index 289309f..cf53c2b 100644
> --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> @@ -1289,11 +1289,11 @@ class PlatformAutoGen(AutoGen):
>  def CollectFixedAtBuildPcds(self):
>  for LibAuto in self.LibraryAutoGenList:
>  FixedAtBuildPcds = {}
>  ShareFixedAtBuildPcdsSameValue = {}
>  for Module in LibAuto._ReferenceModules:
> -for Pcd in Module.FixedAtBuildPcds + 
> LibAuto.FixedAtBuildPcds:
> +for Pcd in set(Module.FixedAtBuildPcds + 
> LibAuto.FixedAtBuildPcds):
>  DefaultValue = Pcd.DefaultValue
>  # Cover the case: DSC component override the Pcd value 
> and the Pcd only used in one Lib
>  if Pcd in Module.LibraryPcdList:
>  Index = Module.LibraryPcdList.index(Pcd)
>  DefaultValue = 
> Module.LibraryPcdList[Index].DefaultValue
> --
> 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] UefiCpuPkg/MpInitLib: Optimize get processor number performance.

2018-07-17 Thread Dong, Eric
Hi All,

As stack maybe overflow in some case and it will caused error AP index been 
returned. Also I don't find any register will be used different in each Aps. So 
I will update my patch to only base on APIC ID to get the AP.

Thanks,
Eric

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Dong, Eric
> Sent: Wednesday, July 11, 2018 7:32 PM
> To: Yao, Jiewen ; Laszlo Ersek ;
> Fan Jeff ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu 
> Subject: Re: [edk2] 答复: [Patch] UefiCpuPkg/MpInitLib: Optimize get
> processor number performance.
> 
> Hi Jiewen,
> 
> I checked the code, found in the AP function (ApWakeupFunction), it updated
> the GDT value with the saved GDT value from BSP. So I think we can't use GDT
> in this case. Right?
> 
>   //
>   // Sync BSP's Control registers to APs
>   //
>   RestoreVolatileRegisters (>CpuData[0].VolatileRegisters,
> FALSE);
> 
> Thanks,
> Eric
> 
> > -Original Message-
> > From: Yao, Jiewen
> > Sent: Wednesday, July 11, 2018 3:45 PM
> > To: Dong, Eric ; Dong, Eric
> > ; Laszlo Ersek ; Fan Jeff
> > ; edk2-devel@lists.01.org
> > Cc: Ni, Ruiyu 
> > Subject: RE: [edk2] 答复: [Patch] UefiCpuPkg/MpInitLib: Optimize get
> > processor number performance.
> >
> > Hi
> > I believe using stack pointer is not a robust way if the stack guard
> > feature is not enabled. Stack pointer may overflow.
> >
> > Can we use GDT? Each AP has its own GDT.
> >
> > Thank you
> > Yao Jiewen
> >
> >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> > > Of Dong, Eric
> > > Sent: Monday, July 9, 2018 2:13 PM
> > > To: Dong, Eric ; Laszlo Ersek
> > > ; Fan Jeff ;
> > > edk2-devel@lists.01.org
> > > Cc: Ni, Ruiyu 
> > > Subject: Re: [edk2] 答复: [Patch] UefiCpuPkg/MpInitLib: Optimize get
> > > processor number performance.
> > >
> > > Hi Laszlo,
> > >
> > > I have created https://bugzilla.tianocore.org/show_bug.cgi?id=1002
> > > to request to add AsmReadEsp() / AsmReadRsp().
> > >
> > > Thanks,
> > > Eric
> > >
> > > > -Original Message-
> > > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On
> > > > Behalf Of Dong, Eric
> > > > Sent: Monday, July 9, 2018 11:04 AM
> > > > To: Laszlo Ersek ; Fan Jeff
> > > > ; edk2-devel@lists.01.org
> > > > Cc: Ni, Ruiyu 
> > > > Subject: Re: [edk2] 答复: [Patch] UefiCpuPkg/MpInitLib: Optimize get
> > > > processor number performance.
> > > >
> > > > Hi Laszlo,
> > > >
> > > > > -Original Message-
> > > > > From: Laszlo Ersek [mailto:ler...@redhat.com]
> > > > > Sent: Thursday, July 5, 2018 9:04 PM
> > > > > To: Fan Jeff ; Dong, Eric
> > > > > ; edk2-devel@lists.01.org
> > > > > Cc: Ni, Ruiyu 
> > > > > Subject: Re: [edk2] 答复: [Patch] UefiCpuPkg/MpInitLib: Optimize
> > > > > get processor number performance.
> > > > >
> > > > > Hi Jeff,
> > > > >
> > > > > On 07/04/18 11:39, Fan Jeff wrote:
> > > > > > Eric,
> > > > > >
> > > > > > Current implementation does not call GetApicid() many times,
> > > > > > Please
> > > > > correct you commit message. Your fix is to improve the
> > > > > performance against the current implementation.
> > > > >
> > > > > I think the original commit message does make sense. Without the
> > > > > patch,
> > > > > GetProcessorNumber() may call GetApicId() up to
> > > > > TotalProcessorNumber times. With the patch, even if we skip the
> > > > > stack range search,
> > > > > GetProcessorNumber() will call GetApicId() just once.
> > > > >
> > > > > [...]
> > > > >
> > > > > Some more questions below, for the patch:
> > > > >
> > > > > > 发件人: Eric Dong 
> > > > > > 发送时间: Wednesday, July 4, 2018 4:37:36 PM
> > > > > > 收件人: edk2-devel@lists.01.org
> > > > > > 抄送: Ruiyu Ni; Jeff Fan; Laszlo Ersek
> > > > > > 主题: [Patch] UefiCpuPkg/MpInitLib: Optimize get processor
> > > > > > number
> > > > > performance.
> > > > > >
> > > > > > Current function has low performance because it calls
> > > > > > GetApicId many times.
> > > > > >
> > > > > > New logic first try to base on the stack range used by AP to
> > > > > > find the processor number. If this solution failed, then call
> > > > > > GetApicId once and base on this value to search the processor.
> > > > > >
> > > > > > Cc: Ruiyu Ni 
> > > > > > Cc: Jeff Fan 
> > > > > > Cc: Laszlo Ersek 
> > > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > > Signed-off-by: Eric Dong 
> > > > > > ---
> > > > > >  UefiCpuPkg/Library/MpInitLib/MpLib.c | 25
> > > ++---
> > > > > >  1 file changed, 22 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > > > > > b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > > > > > index eb2765910c..abd65bee1a 100644
> > > > > > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > > > > > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > > > > > @@ -418,7 +418,8 @@ ApInitializeSync (  }
> > > > > >
> > > > > >  /**
> > > > > > -  Find the current 

Re: [edk2] [PATCH v1] ShellPkg: add UefiShellAcpiViewCommandLib.inf to ShellPkg.dsc

2018-07-17 Thread Ni, Ruiyu

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


Re: [edk2] [PATCH] UefiCpuPkg/CpuDxe: fix incorrect check of SMM mode

2018-07-17 Thread Wang, Jian J
Hi Laszlo,


Regards,
Jian


> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Tuesday, July 17, 2018 10:37 PM
> To: Wang, Jian J ; edk2-devel@lists.01.org
> Cc: Dong, Eric ; Yao, Jiewen ;
> Zeng, Star 
> Subject: Re: [PATCH] UefiCpuPkg/CpuDxe: fix incorrect check of SMM mode
> 
> On 07/13/18 07:53, Jian J Wang wrote:
> > Current IsInSmm() method makes use of gEfiSmmBase2ProtocolGuid.InSmm()
> to
> > check if current processor is in SMM mode or not. But this is not correct
> > because gEfiSmmBase2ProtocolGuid.InSmm() can only detect if the caller is
> > running in SMRAM or from SMM driver. It cannot guarantee if the caller is
> > running in SMM mode.
> 
> Wow. This is the exact difference which I asked about, in my question
> (9b), and I was assured that InSmm() actually determined whether we were
> executing in Management Mode (meaning the processor operating mode).
> 
> http://mid.mail-
> archive.com/0c09afa07dd0434d9e2a0c6aeb0483103bb55...@shsmsx102.cc
> r.corp.intel.com
> 
> (Scroll down in that message to see my original question (9b).)
> 
> So why doesn't Star's explanation, from the original thread, apply
> ultimately?
> 

We did many tests for this and didn't found any issue. So I took a risk. (I 
thought
a very precise check of SMM mode is hard and time consuming.) 

> > Because SMM mode will load its own page table, adding
> > an extra check of saved DXE page table base address against current CR3
> > register value can help to get the correct answer for sure (in SMM mode or
> > not in SMM mode).
> 
> So, apparently, the PI spec offers no standard way for a platform module
> to determine whether it runs in Management Mode, despite protocol member
> being called "InSmm". Do we need a PI spec ECR for introducing the
> needed facility?
> 
> Alternatively, the PI spec might already intend to specify that, but the
> edk2 implementation doesn't do what the PI spec requires.
> 
> Which one is the case?
> 

The implementation conforms to the spec. It's my misunderstanding. But it's true
that there's no specific protocol API to determine if it's in SMM mode or not.

> >
> > This is an issue caused by check-in at
> >
> >   d106cf71eabaacff63c14626a4a87346b93074dd
> 
> I disagree; I think the issue was introduced in commit 2a1408d1d739
> ("UefiCpuPkg/CpuDxe: allow accessing (DXE) page table in SMM mode",
> 2018-06-19).
> 

You're right. Thanks for pointing this out.

> 
> How did you encounter / find this issue?
> 

I didn't find it. The issue came to me. In other words, I think it's random and 
hard
to reproduce it. Maybe a subtle change in boot sequence will hide it away.

> >
> > Cc: Eric Dong 
> > Cc: Laszlo Ersek 
> > Cc: Jiewen Yao 
> > Cc: Star Zeng 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jian J Wang 
> > ---
> >  UefiCpuPkg/CpuDxe/CpuPageTable.c | 9 -
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > index 850eed60e7..df021798c0 100644
> > --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > @@ -136,7 +136,14 @@ IsInSmm (
> >  mSmmBase2->InSmm (mSmmBase2, );
> >}
> >
> > -  return InSmm;
> > +  //
> > +  // mSmmBase2->InSmm() can only detect if the caller is running in SMRAM
> > +  // or from SMM driver. It cannot tell if the caller is running in SMM 
> > mode.
> > +  // Check page table base address to guarantee that because SMM mode
> willl
> > +  // load its own page table.
> > +  //
> > +  return (InSmm &&
> > +  mPagingContext.ContextData.X64.PageTableBase !=
> (UINT64)AsmReadCr3());
> >  }
> >
> >  /**
> >
> 
> Shouldn't we consider Ia32.PageTableBase when that's appropriate? From
> "UefiCpuPkg/CpuDxe/CpuPageTable.h":
> 
> typedef struct {
>   UINT32  PageTableBase;
>   UINT32  Reserved;
>   UINT32  Attributes;
> } PAGE_TABLE_LIB_PAGING_CONTEXT_IA32;
> 
> typedef struct {
>   UINT64  PageTableBase;
>   UINT32  Attributes;
> } PAGE_TABLE_LIB_PAGING_CONTEXT_X64;
> 
> typedef union {
>   PAGE_TABLE_LIB_PAGING_CONTEXT_IA32  Ia32;
>   PAGE_TABLE_LIB_PAGING_CONTEXT_X64   X64;
> } PAGE_TABLE_LIB_PAGING_CONTEXT_DATA;
> 
> The Ia32/X64 structure types are not packed, and even if they were, I
> wouldn't think we should rely on "Reserved" being zero.
> 

mPagingContext is zero-ed at each update in GetCurrentPagingContext().
I think it should be no problem to use X64.

> 
> All in all, I think that determining whether the processor is operating
> in Management Mode (regardless of where in RAM the processor is
> executing code from) is a core functionality that should be offered as a
> central service, not just an internal CpuDxe function. I think we need
> either a PI spec addition, or at least an EDKII extension protocol. It's
> obvious that the InSmm() behavior is unclear to developers! (Me
> included, of course.)
> 

I agree.

> Thanks,
> Laszlo

Re: [edk2] [Patch v3 0/3] Optimize load uCode performance

2018-07-17 Thread Dong, Eric
Hi Laszlo,

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Wednesday, July 18, 2018 12:38 AM
> To: Dong, Eric 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [Patch v3 0/3] Optimize load uCode performance
> 
> Hi Eric,
> 
> On 07/16/18 05:08, Eric Dong wrote:
> > Use below three rules to optimize load uCode performance:
> > 1. Let BSP relocate uCode from flash to memory for better performance.
> > 2. BSP caches the CPU ID and address of uCode so AP doesn’t need to look
> >for the uCode again if the CPU ID is same as BSP’s.
> > 3. Only apply uCode in one thread of a core when hyper threading is
> enabled.
> >
> > v2 changes:
> >   Fix potential issue if allocate memory failed.
> >
> > V3 Changes:
> >   Remove the ASSERT code which is not correct.
> 
> Based on the above, my understanding is that you didn't modify patches
> #2 and #3, from v2 to v3. Is that correct?
> 
> If it's correct, then you should have picked up my Acked-by tags from the v2
> review, for the v3 posting:
> 
> http://mid.mail-archive.com/dcf4df85-1d35-65e9-2c9b-
> d5d47a098...@redhat.com
> 
> http://mid.mail-archive.com/dbc8439f-448e-306c-cdbd-
> 1b2edc1f4...@redhat.com
> 
> If you don't pick up my previous review tags for un-modified patches in new
> postings of the patch series, then I have to re-review those patches every
> single time. I described this workflow element here:
> 

Got it, will follow this rule later. Thanks.

> https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-
> guide-for-edk2-contributors-and-maintainers#contrib-26
> 
> --
> 26. § More frequently though, you will get requests for changes for
> *some* of your patches, while *others* of your patches will be
> fine, and garner Reviewed-by, Acked-by, and Tested-by tags. What
> you need to do in this case is:
> 
> * create the next version of your local branch
> * pick up the tags that you got on the list
> * implement the requested changes
> * mark the v2 changes on each patch outside of the commit
>   message
> * push the next version to your personal repo again
> * post the next version to the list
> 
> In the following steps, we'll go through each of these in more
> detail.
> --
> 
> Thanks
> Laszlo
> 
> > Test:
> > Use an sample platform which has 1 socket, 4 core, 8 threads, the
> > CpuMpPei driver cost time reduce from 108.4ms to 27.2ms
> >
> >
> > Eric Dong (3):
> >   UefiCpuPkg/MpInitLib: Use BSP uCode for APs if possible.
> >   UefiCpuPkg/MpInitLib: Load uCode once for each core.
> >   UefiCpuPkg/MpInitLib: Relocate uCode to memory to save time.
> >
> >  UefiCpuPkg/Library/MpInitLib/Microcode.c | 43
> +---
> >  UefiCpuPkg/Library/MpInitLib/MpLib.c | 38
> +---
> >  UefiCpuPkg/Library/MpInitLib/MpLib.h | 11 ++--
> >  3 files changed, 84 insertions(+), 8 deletions(-)
> >
> >
> >
> > ___
> > 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 v3 3/3] UefiCpuPkg/MpInitLib: Load uCode once for each core.

2018-07-17 Thread Dong, Eric
Hi Laszlo & Ray,

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Tuesday, July 17, 2018 10:07 PM
> To: Ni, Ruiyu ; Dong, Eric 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [Patch v3 3/3] UefiCpuPkg/MpInitLib: Load uCode once for
> each core.
> 
> On 07/17/18 12:02, Ni, Ruiyu wrote:
> > On 7/16/2018 11:08 AM, Eric Dong wrote:
> >> GetProcessorLocationByApicId (GetInitialApicId (), NULL, NULL,
> >> );
> >> +  if (ThreadId != 0) {
> >> +    //
> >> +    // Skip loading microcode if it is not the first thread in one core.
> >> +    //
> >> +    return;
> >> +  }
> >> +
> >
> > Eric,
> > Is it possible that Thread#0 is disabled while Thread#1 is enabled? It
> > may cause the certain core with no uCode applied.
> >
> 
> I thought of this (superficially) but I figured the code would run anyway at
> CpuMpPei / CpuDxe startup only, at which point no logical processors could
> have been disabled yet -- programmatically, through the MP PPI / protocol. Is
> there any other reason why ThreadId=0 could be missing?
> 

Yes, I think at this point, all the threads should at the ready state.  The 
only case I can image is that the hardware has broken and the core can't be 
wake up.  But I think in this case, the core should can't work, not only thread 
0 can't work.  What do you think?

> (I plan to check the rest of the series later.)
> 
> Thanks
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch v3 0/3] Optimize load uCode performance

2018-07-17 Thread Laszlo Ersek
On 07/17/18 18:38, Laszlo Ersek wrote:
> Hi Eric,
> 
> On 07/16/18 05:08, Eric Dong wrote:
>> Use below three rules to optimize load uCode performance:
>> 1. Let BSP relocate uCode from flash to memory for better performance.
>> 2. BSP caches the CPU ID and address of uCode so AP doesn’t need to look 
>>for the uCode again if the CPU ID is same as BSP’s.
>> 3. Only apply uCode in one thread of a core when hyper threading is enabled.
>>
>> v2 changes:
>>   Fix potential issue if allocate memory failed.
>>
>> V3 Changes:
>>   Remove the ASSERT code which is not correct.
> 
> Based on the above, my understanding is that you didn't modify patches
> #2 and #3, from v2 to v3. Is that correct?

I compared the v2 and v3 patches pair-wise; indeed the ASSERT()'s
removal in patch #1 is the only difference.

> If it's correct, then you should have picked up my Acked-by tags from
> the v2 review, for the v3 posting:
> 
> dcf4df85-1d35-65e9-2c9b-d5d47a0988aa@redhat.com">http://mid.mail-archive.com/dcf4df85-1d35-65e9-2c9b-d5d47a0988aa@redhat.com
> 
> dbc8439f-448e-306c-cdbd-1b2edc1f4aef@redhat.com">http://mid.mail-archive.com/dbc8439f-448e-306c-cdbd-1b2edc1f4aef@redhat.com
> 
> If you don't pick up my previous review tags for un-modified patches in
> new postings of the patch series, then I have to re-review those patches
> every single time. I described this workflow element here:
> 
> https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-26
> 
> --
> 26. § More frequently though, you will get requests for changes for
> *some* of your patches, while *others* of your patches will be
> fine, and garner Reviewed-by, Acked-by, and Tested-by tags. What
> you need to do in this case is:
> 
> * create the next version of your local branch
> * pick up the tags that you got on the list
> * implement the requested changes
> * mark the v2 changes on each patch outside of the commit
>   message
> * push the next version to your personal repo again
> * post the next version to the list
> 
> In the following steps, we'll go through each of these in more
> detail.
> --

* For patch #1:

Reviewed-by: Laszlo Ersek 
Regression-tested-by: Laszlo Ersek 

* For patches #2 and #3:

Acked-by: Laszlo Ersek 
Regression-tested-by: Laszlo Ersek 

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


Re: [edk2] [Patch v3 0/3] Optimize load uCode performance

2018-07-17 Thread Laszlo Ersek
Hi Eric,

On 07/16/18 05:08, Eric Dong wrote:
> Use below three rules to optimize load uCode performance:
> 1. Let BSP relocate uCode from flash to memory for better performance.
> 2. BSP caches the CPU ID and address of uCode so AP doesn’t need to look 
>for the uCode again if the CPU ID is same as BSP’s.
> 3. Only apply uCode in one thread of a core when hyper threading is enabled.
> 
> v2 changes:
>   Fix potential issue if allocate memory failed.
> 
> V3 Changes:
>   Remove the ASSERT code which is not correct.

Based on the above, my understanding is that you didn't modify patches
#2 and #3, from v2 to v3. Is that correct?

If it's correct, then you should have picked up my Acked-by tags from
the v2 review, for the v3 posting:

dcf4df85-1d35-65e9-2c9b-d5d47a0988aa@redhat.com">http://mid.mail-archive.com/dcf4df85-1d35-65e9-2c9b-d5d47a0988aa@redhat.com

dbc8439f-448e-306c-cdbd-1b2edc1f4aef@redhat.com">http://mid.mail-archive.com/dbc8439f-448e-306c-cdbd-1b2edc1f4aef@redhat.com

If you don't pick up my previous review tags for un-modified patches in
new postings of the patch series, then I have to re-review those patches
every single time. I described this workflow element here:

https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-26

--
26. § More frequently though, you will get requests for changes for
*some* of your patches, while *others* of your patches will be
fine, and garner Reviewed-by, Acked-by, and Tested-by tags. What
you need to do in this case is:

* create the next version of your local branch
* pick up the tags that you got on the list
* implement the requested changes
* mark the v2 changes on each patch outside of the commit
  message
* push the next version to your personal repo again
* post the next version to the list

In the following steps, we'll go through each of these in more
detail.
--

Thanks
Laszlo

> Test:
> Use an sample platform which has 1 socket, 4 core, 8 threads, the 
> CpuMpPei driver cost time reduce from 108.4ms to 27.2ms
> 
> 
> Eric Dong (3):
>   UefiCpuPkg/MpInitLib: Use BSP uCode for APs if possible.
>   UefiCpuPkg/MpInitLib: Load uCode once for each core.
>   UefiCpuPkg/MpInitLib: Relocate uCode to memory to save time.
> 
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 43 
> +---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 38 +---
>  UefiCpuPkg/Library/MpInitLib/MpLib.h | 11 ++--
>  3 files changed, 84 insertions(+), 8 deletions(-)
> 
> 
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

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


Re: [edk2] [PATCH] UefiCpuPkg/CpuDxe: fix incorrect check of SMM mode

2018-07-17 Thread Laszlo Ersek
On 07/13/18 07:53, Jian J Wang wrote:
> Current IsInSmm() method makes use of gEfiSmmBase2ProtocolGuid.InSmm() to
> check if current processor is in SMM mode or not. But this is not correct
> because gEfiSmmBase2ProtocolGuid.InSmm() can only detect if the caller is
> running in SMRAM or from SMM driver. It cannot guarantee if the caller is
> running in SMM mode.

Wow. This is the exact difference which I asked about, in my question
(9b), and I was assured that InSmm() actually determined whether we were
executing in Management Mode (meaning the processor operating mode).

0C09AFA07DD0434D9E2A0C6AEB0483103BB55B46@shsmsx102.ccr.corp.intel.com">http://mid.mail-archive.com/0C09AFA07DD0434D9E2A0C6AEB0483103BB55B46@shsmsx102.ccr.corp.intel.com

(Scroll down in that message to see my original question (9b).)

So why doesn't Star's explanation, from the original thread, apply
ultimately?

> Because SMM mode will load its own page table, adding
> an extra check of saved DXE page table base address against current CR3
> register value can help to get the correct answer for sure (in SMM mode or
> not in SMM mode).

So, apparently, the PI spec offers no standard way for a platform module
to determine whether it runs in Management Mode, despite protocol member
being called "InSmm". Do we need a PI spec ECR for introducing the
needed facility?

Alternatively, the PI spec might already intend to specify that, but the
edk2 implementation doesn't do what the PI spec requires.

Which one is the case?

> 
> This is an issue caused by check-in at
> 
>   d106cf71eabaacff63c14626a4a87346b93074dd

I disagree; I think the issue was introduced in commit 2a1408d1d739
("UefiCpuPkg/CpuDxe: allow accessing (DXE) page table in SMM mode",
2018-06-19).


How did you encounter / find this issue?

> 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Cc: Jiewen Yao 
> Cc: Star Zeng 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang 
> ---
>  UefiCpuPkg/CpuDxe/CpuPageTable.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c 
> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> index 850eed60e7..df021798c0 100644
> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> @@ -136,7 +136,14 @@ IsInSmm (
>  mSmmBase2->InSmm (mSmmBase2, );
>}
>  
> -  return InSmm;
> +  //
> +  // mSmmBase2->InSmm() can only detect if the caller is running in SMRAM
> +  // or from SMM driver. It cannot tell if the caller is running in SMM mode.
> +  // Check page table base address to guarantee that because SMM mode willl
> +  // load its own page table.
> +  //
> +  return (InSmm &&
> +  mPagingContext.ContextData.X64.PageTableBase != 
> (UINT64)AsmReadCr3());
>  }
>  
>  /**
> 

Shouldn't we consider Ia32.PageTableBase when that's appropriate? From
"UefiCpuPkg/CpuDxe/CpuPageTable.h":

typedef struct {
  UINT32  PageTableBase;
  UINT32  Reserved;
  UINT32  Attributes;
} PAGE_TABLE_LIB_PAGING_CONTEXT_IA32;

typedef struct {
  UINT64  PageTableBase;
  UINT32  Attributes;
} PAGE_TABLE_LIB_PAGING_CONTEXT_X64;

typedef union {
  PAGE_TABLE_LIB_PAGING_CONTEXT_IA32  Ia32;
  PAGE_TABLE_LIB_PAGING_CONTEXT_X64   X64;
} PAGE_TABLE_LIB_PAGING_CONTEXT_DATA;

The Ia32/X64 structure types are not packed, and even if they were, I
wouldn't think we should rely on "Reserved" being zero.


All in all, I think that determining whether the processor is operating
in Management Mode (regardless of where in RAM the processor is
executing code from) is a core functionality that should be offered as a
central service, not just an internal CpuDxe function. I think we need
either a PI spec addition, or at least an EDKII extension protocol. It's
obvious that the InSmm() behavior is unclear to developers! (Me
included, of course.)

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: Load uCode once for each core.

2018-07-17 Thread Laszlo Ersek
On 07/17/18 12:02, Ni, Ruiyu wrote:
> On 7/16/2018 11:08 AM, Eric Dong wrote:
>> GetProcessorLocationByApicId (GetInitialApicId (), NULL, NULL,
>> );
>> +  if (ThreadId != 0) {
>> +    //
>> +    // Skip loading microcode if it is not the first thread in one core.
>> +    //
>> +    return;
>> +  }
>> +
> 
> Eric,
> Is it possible that Thread#0 is disabled while Thread#1 is enabled? It
> may cause the certain core with no uCode applied.
> 

I thought of this (superficially) but I figured the code would run
anyway at CpuMpPei / CpuDxe startup only, at which point no logical
processors could have been disabled yet -- programmatically, through the
MP PPI / protocol. Is there any other reason why ThreadId=0 could be
missing?

(I plan to check the rest of the series later.)

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


Re: [edk2] [PATCH 0/2] ArmVirtPkg/ArmVirtQemu: enable the IPv6 stack

2018-07-17 Thread Laszlo Ersek
On 07/13/18 08:45, Ard Biesheuvel wrote:
> On 13 July 2018 at 01:41, Laszlo Ersek  wrote:
>> Repo:   https://github.com/lersek/edk2.git
>> Branch: armvirt_ipv6_bz1007
>>
>> Tested the "-D NETWORK_IP6_ENABLE" ArmVirtQemu build with PXEv4, PXEv6,
>> HTTPv4, HTTPv6 netboot.
>>
>> Build-tested the ArmVirtQemuKernel build (32- and 64-bit), and the
>> ArmVirtXen build (64-bit).
>>
>> Ard, if the series is OK, can you please push it for me as well? Thank
>> you.
>>
> 
> Looks good to me
> 
> Reviewed-by: Ard Biesheuvel 
> 
> Pushed as 280f49b81170..ae08ea246fe9

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


Re: [edk2] Reg: Intel Rangley Support in EDK

2018-07-17 Thread Dhanasekar Jaganathan
Hi Nate, Ben,

After changing   Windows -> Translation = CP437 in Putty, I can able to see
proper Shell and BIOS setup.
After Grub, I am not seeing OS installation Process. I have changed Kernal
Parameters too.

May Linux Server Installation(via serial) use different character setting
or baud rate?.

Thanks,
Dhanasekar

On Mon, Jul 16, 2018 at 10:58 AM, Dhanasekar Jaganathan <
jdhanasekar...@gmail.com> wrote:

> Hi Nate/ Ben,
>
> Thanks for the info.
>
> My host is Ubuntu system and I am using "minicom" for serial messages.
> I will use putty with the values you mentioned.
> I have been trying with different Kernel parameters  (in grub.cfg) while
> installing Ubuntu Server OS.
>
> I will update the result.
>
> Thanks,
> Dhanasekar
>
> On Sat, Jul 14, 2018 at 10:48 AM, You, Benjamin 
> wrote:
>
>> Hi Dhanasekar,
>>
>> The Coreboot UEFI Payload does support serial and terminal. As Nate said
>> below, there may be some configuration issue causing the display not
>> correctly
>> formatted through serial terminal.
>>
>> I also agree with Nate that since your board does not have a VGA
>> connection,
>> you might have to configure Ubuntu to use serial console to monitor the
>> boot
>> process.
>>
>> Thanks,
>>
>> - ben
>>
>> > -Original Message-
>> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> > Desimone, Nathaniel L
>> > Sent: Saturday, July 14, 2018 8:51 AM
>> > To: Dhanasekar Jaganathan 
>> > Cc: edk2-devel@lists.01.org
>> > Subject: Re: [edk2] Reg: Intel Rangley Support in EDK
>> >
>> > Hi Dhanasekar,
>> >
>> > I don't know very much about coreboot or the coreboot UEFI payload, so I
>> > won't be able to help you much with that. From what you mention in your
>> > message it sounds like either the coreboot UEFI payload does not
>> include the
>> > TerminalDxe driver, there is a configuration issue with it.
>> MinPlatformPkg is a
>> > pure EDK2 only implementation that does not have any coreboot
>> dependencies,
>> > but it does not support Rangley at the moment.
>> >
>> > With regard to the setup menu graphical corruption, the setup menu uses
>> the
>> > CP437 character set, since the setup menu is often displayed using VGA
>> text
>> > mode, whereas most current terminal emulators assume UTF-8 encoding. To
>> > make it render properly, use PuTTY and in the connection configuration
>> under
>> > Windows -> Translation change the remote character set to CP437.
>> >
>> > I suspect there are kernel parameters you will need to pass to make
>> Ubuntu use
>> > the serial port as the terminal, I suggest looking at Ubuntu
>> documentation or
>> > message boards.
>> >
>> > Thanks,
>> >
>> > Nate
>> >
>> > On 7/12/18, 10:21 PM, "edk2-devel on behalf of Dhanasekar Jaganathan"
>> > 
>> > wrote:
>> >
>> > Hi Nate,
>> >
>> > Thanks for the info. I will try this.
>> >
>> > My Rangley platform has no onboard or offboard vga support. Com Port /
>> Serial
>> > console is used for display and communication.
>> > I am booting the platform by coreboot with UEFI payload. I am trying to
>> install
>> > Ubuntu server OS.
>> >
>> > When I boot into Shell, I am not seeing actually shell (graphics are
>> not good) and
>> > keystroke are working properly.
>> > When I boot into EDK Setup, I am not getting proper setup page
>> (graphics are
>> > not good) and keystroke are not working.
>> >
>> > When I try to boot Ubuntu Server, I am getting below error,
>> >
>> > "error: no suitable video mode found.
>> > Booting in blind mode"
>> >
>> > It seems I am missing some video/graphics settings in UEFI payload. If
>> you know
>> > the fix, Please provide me.
>> >
>> > Thanks,
>> > Dhanasekar
>> >
>> >
>> >
>> > On Fri, Jul 13, 2018 at 7:14 AM, Desimone, Nathaniel L
>> >  wrote:
>> > Hi Dhanasekar,
>> >
>> > There is nothing pre-built and off-the-shelf ready today. But we do
>> have a
>> > generalized infrastructure for open source Intel UEFI platforms called
>> > MinPlatformPkg. Please see the following:
>> >
>> > https://github.com/tianocore/edk2-platforms/tree/devel-
>> > MinPlatform/Platform/Intel
>> >
>> > Notice that MinPlatformPkg requires a *OpenBoardPkg that supports the
>> > desired platform. To my knowledge no one at Intel has looked at
>> implementing
>> > a RangleyOpenBoardPkg thus far. The focus has been on recently released
>> > platforms, Rangley is 4 years old at this point. If you are so
>> inclined, you are
>> > welcome to try implementing a RangleyOpenBoardPkg. I would recommend
>> > using KabylakeOpenBoardPkg as a starting point.
>> >
>> > Thanks,
>> > Nate
>> >
>> > On 7/11/18, 8:52 PM, "edk2-devel on behalf of Dhanasekar Jaganathan"
>> > > devel-boun...@lists.01.org on behalf of jdhanasekar...@gmail.com>
>> wrote:
>> >
>> > Hi,
>> >
>> > I have booted Intel Rangley / MohonPeak platform in coreboot with
>> Intel
>> > FSP.
>> > I am unable to install UEFI OS in coreboot (sometime).
>> >
>> > EDK bios will install both UEFI and Legacy OS.
>> > Does Open EDK 

[edk2] [PATCH v1] ShellPkg: add UefiShellAcpiViewCommandLib.inf to ShellPkg.dsc

2018-07-17 Thread AlexeiFedorov
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
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


___
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: Load uCode once for each core.

2018-07-17 Thread Ni, Ruiyu

On 7/16/2018 11:08 AM, Eric Dong wrote:

GetProcessorLocationByApicId (GetInitialApicId (), NULL, NULL, );
+  if (ThreadId != 0) {
+//
+// Skip loading microcode if it is not the first thread in one core.
+//
+return;
+  }
+


Eric,
Is it possible that Thread#0 is disabled while Thread#1 is enabled? It 
may cause the certain core with no uCode applied.


--
Thanks,
Ray
___
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: Relocate uCode to memory to save time.

2018-07-17 Thread Ni, Ruiyu

On 7/16/2018 11:08 AM, Eric Dong wrote:

Read uCode from memory has better performance than from flash.
But it needs extra effort to let BSP copy uCode from flash to
memory. Also BSP already enable cache in SEC phase, so it use
less time to relocate uCode from flash to memory. After
verification, if system has more than one processor, it will
reduce some time if load uCode from memory.

This change enable this optimization.

V3 changes:
   Remove the ASSERT which is not correct.

Cc: Laszlo Ersek 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
  UefiCpuPkg/Library/MpInitLib/MpLib.c | 33 -
  1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 108eea0a6f..d8b56f149f 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1520,6 +1520,7 @@ MpInitLibInitialize (
UINTNApResetVectorSize;
UINTNBackupBufferAddr;
UINTNApIdtBase;
+  VOID *MicrocodePatchInRam;
  
OldCpuMpData = GetCpuMpDataFromGuidedHob ();

if (OldCpuMpData == NULL) {
@@ -1587,8 +1588,38 @@ MpInitLibInitialize (
CpuMpData->SwitchBspFlag= FALSE;
CpuMpData->CpuData  = (CPU_AP_DATA *) (CpuMpData + 1);
CpuMpData->CpuInfoInHob = (UINT64) (UINTN) (CpuMpData->CpuData + 
MaxLogicalProcessorNumber);
-  CpuMpData->MicrocodePatchAddress= PcdGet64 (PcdCpuMicrocodePatchAddress);
CpuMpData->MicrocodePatchRegionSize = PcdGet64 
(PcdCpuMicrocodePatchRegionSize);
+  //
+  // If platform has more than one CPU, relocate microcode to memory to reduce
+  // loading microcode time.
+  //
+  MicrocodePatchInRam = NULL;
+  if (MaxLogicalProcessorNumber > 1) {
+MicrocodePatchInRam = AllocatePages (
+EFI_SIZE_TO_PAGES (
+  (UINTN)CpuMpData->MicrocodePatchRegionSize
+  )
+);
+  }
+  if (MicrocodePatchInRam == NULL) {
+//
+// there is only one processor, or no microcode patch is available, or
+// memory allocation failed
+//
+CpuMpData->MicrocodePatchAddress = PcdGet64 (PcdCpuMicrocodePatchAddress);
+  } else {
+//
+// there are multiple processors, and a microcode patch is available, and
+// memory allocation succeeded
+//
+CopyMem (
+  MicrocodePatchInRam,
+  (VOID *)(UINTN)PcdGet64 (PcdCpuMicrocodePatchAddress),
+  (UINTN)CpuMpData->MicrocodePatchRegionSize
+  );
+CpuMpData->MicrocodePatchAddress = (UINTN)MicrocodePatchInRam;
+  }
+
InitializeSpinLock(>MpLock);
  
//



Reviewed-by: Ruiyu Ni 

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


Re: [edk2] [Patch v3 2/3] UefiCpuPkg/MpInitLib: Use BSP uCode for APs if possible.

2018-07-17 Thread Ni, Ruiyu

On 7/16/2018 11:08 AM, Eric Dong wrote:

+
+  if (IsBspCallIn && (LatestRevision != 0)) {
+CpuMpData->ProcessorSignature = Eax.Uint32;
+CpuMpData->ProcessorFlags = ProcessorFlags;
+CpuMpData->MicrocodeDataAddress = (UINTN) MicrocodeData;
+CpuMpData->MicrocodeRevision = LatestRevision;
+DEBUG ((DEBUG_INFO, "BSP Microcode:: signature [0x%08x], ProcessorFlags 
[0x%08x], \
+   MicroData [0x%08x], Revision [0x%08x]\n", Eax.Uint32, ProcessorFlags, 
(UINTN) MicrocodeData, LatestRevision));
+  }

Reviewed-by: Ruiyu Ni 
Can you add more comments for the above logic when check in the patch?
And make the "=" aligned.



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


[edk2] [Patch][edk2-platforms/devel-IntelAtomProcessorE3900] Enable VT-d DMA remapping for DMA protection.

2018-07-17 Thread zwei4
(1) Add VT-d modules into FDF file.
(2) Add Setup option and configuration policy for pre-boot VT-d enabling.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: David Wei 
CC: Mang Guo 
---
 .../Common/Include/Guid/SetupVariable.h|   4 ++-
 .../Library/PeiPolicyUpdateLib/PeiScPolicyUpdate.c |   2 ++
 .../Common/PlatformSettings/PlatformDxe/Platform.c |  31 +++-
 .../PlatformSettings/PlatformDxe/PlatformDxe.inf   |   5 +++-
 .../PlatformSettings/PlatformSetupDxe/Cpu.vfi  |   9 ++
 .../PlatformSetupDxe/PlatformSetupDxe.c|   5 
 .../PlatformSettings/PlatformSetupDxe/UqiList.uni  | Bin 126916 -> 127232 bytes
 .../PlatformSetupDxe/VfrStrings.uni| Bin 306044 -> 306466 bytes
 .../PlatformVTdInfoPei/PlatformVTdInfoPei.c|  32 +++--
 .../PlatformVTdInfoPei/PlatformVTdInfoPei.inf  |   6 
 Platform/BroxtonPlatformPkg/PlatformPkg.fdf|   8 ++
 .../SouthCluster/Include/ConfigBlock/VtdConfig.h   |   3 +-
 .../SouthCluster/Library/DxeVtdLib/DxeVtdLib.c |  21 +-
 .../Library/PeiScPolicyLib/ScPrintPolicy.c |   2 +-
 14 files changed, 120 insertions(+), 8 deletions(-)

diff --git a/Platform/BroxtonPlatformPkg/Common/Include/Guid/SetupVariable.h 
b/Platform/BroxtonPlatformPkg/Common/Include/Guid/SetupVariable.h
index 60ffb1beb5..4f5eeb1c0f 100644
--- a/Platform/BroxtonPlatformPkg/Common/Include/Guid/SetupVariable.h
+++ b/Platform/BroxtonPlatformPkg/Common/Include/Guid/SetupVariable.h
@@ -1,7 +1,7 @@
 /** @file
   Header file for Setup Variable.
 
-  Copyright (c) 2012 - 2017, Intel Corporation. All rights reserved.
+  Copyright (c) 2012 - 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
@@ -1130,6 +1130,8 @@ typedef struct {
   UINT8 HotThermalTripPointSen2;
   
   UINT8 SueCreekBypass;
+
+  UINT8 PrebootVTdEnable;
 } SYSTEM_CONFIGURATION;
 #pragma pack(pop)
 
diff --git 
a/Platform/BroxtonPlatformPkg/Common/Library/PeiPolicyUpdateLib/PeiScPolicyUpdate.c
 
b/Platform/BroxtonPlatformPkg/Common/Library/PeiPolicyUpdateLib/PeiScPolicyUpdate.c
index e7714d189a..faf478ace8 100644
--- 
a/Platform/BroxtonPlatformPkg/Common/Library/PeiPolicyUpdateLib/PeiScPolicyUpdate.c
+++ 
b/Platform/BroxtonPlatformPkg/Common/Library/PeiPolicyUpdateLib/PeiScPolicyUpdate.c
@@ -640,6 +640,8 @@ UpdatePeiScPolicy (
   }
 
   VtdConfig->VtdEnable = SystemConfiguration.VTdEnable;
+  VtdConfig->PrebootVTdEnable = SystemConfiguration.PrebootVTdEnable;
+  
   //
   // Power management Configuration
   //
diff --git 
a/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformDxe/Platform.c 
b/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformDxe/Platform.c
index 712d5cd14f..31f8bedc31 100644
--- a/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformDxe/Platform.c
+++ b/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformDxe/Platform.c
@@ -1,7 +1,7 @@
 /** @file
   Platform Initialization Driver.
 
-  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
@@ -38,6 +38,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 
 #if (ENBDT_PF_ENABLE == 1) //BXTP
   #include 
@@ -496,6 +499,31 @@ InitPlatformBootMode (
 
 }
 
+VOID
+InitPlatformVtdDxePolicy (
+  VOID
+  )
+{
+  SC_POLICY_HOB *ScPolicy;
+  EFI_PEI_HOB_POINTERS  HobPtr;
+  SC_VTD_CONFIG *VtdConfig;
+  EFI_STATUSStatus;
+
+  //
+  // Get SC VT-d config block
+  //
+  HobPtr.Guid = GetFirstGuidHob ();
+  ASSERT (HobPtr.Guid != NULL);
+  ScPolicy = (SC_POLICY_HOB*) GET_GUID_HOB_DATA (HobPtr.Guid);
+  Status = GetConfigBlock ((VOID *) ScPolicy, , (VOID *) 
);
+  ASSERT_EFI_ERROR (Status);
+  DEBUG ((DEBUG_INFO, "Set ScPolicy VtdConfig PrebootVTdEnable.\n"));
+  
+  VtdConfig->PrebootVTdEnable = 0;
+  if ((BOOLEAN)(mSystemConfiguration.PrebootVTdEnable) == TRUE) {
+VtdConfig->PrebootVTdEnable = 1;
+  }
+}
 
 VOID
 InitPlatformUsbPolicy (
@@ -839,6 +867,7 @@ InitializePlatform (
   IoWrite8 (PCAT_RTC_ADDRESS_REGISTER, RTC_ADDRESS_REGISTER_B);
   IoWrite8 (PCAT_RTC_DATA_REGISTER, IoRead8 (PCAT_RTC_DATA_REGISTER) & 
~B_RTC_ALARM_INT_ENABLE);
 
+  InitPlatformVtdDxePolicy();
   InitPlatformIdePolicy ();
   InitPlatformUsbPolicy ();
   InitSioPlatformPolicy ();
diff --git 
a/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformDxe/PlatformDxe.inf
 
b/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformDxe/PlatformDxe.inf
index 440071fd41..8eb6a7aa82 100644
--- 
a/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformDxe/PlatformDxe.inf
+++