date:20171019

[edk2] [Patch] MdeModulePkg/Ip4Dxe: Trigger Ip4Config2 to retrieve the default address.

2017-10-19 Thread Jiaxin Wu

According the UEFI spec 2.7 A:
In section 28.3.2 for the IpConfigData.UseDefaultAddress, "While set to
TRUE, Configure() will trigger the EFI_IP4_CONFIG2_PROTOCOL to retrieve
the default IPv4 address if it is not available yet."
In section 28.5 for the Ip4Config2PolicyDhcp, "...All of these configurations
are retrieved from DHCP server or other auto-configuration mechanism."

This patch is to align with the above description. When the default IPv4
address is not available and IpConfigData.UseDefaultAddress is set to TRUE,
Ip4Config2 protocol will be called to retrieve the default address by setting
the policy to Ip4Config2PolicyDhcp.

Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c | 25 +
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c 
b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
index 3cdf8ec..fc5812e 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
@@ -594,13 +594,17 @@ Ip4ConfigProtocol (
   IP4_INTERFACE *IpIf;
   EFI_STATUSStatus;
   IP4_ADDR  Ip;
   IP4_ADDR  Netmask;
   EFI_ARP_PROTOCOL  *Arp;
+  EFI_IP4_CONFIG2_PROTOCOL  *Ip4Config2;
+  EFI_IP4_CONFIG2_POLICYPolicy;
 
   IpSb = IpInstance->Service;
 
+  Ip4Config2  = NULL;
+
   //
   // User is changing packet filters. It must be stopped
   // before the station address can be changed.
   //
   if (IpInstance->State == IP4_STATE_CONFIGED) {
@@ -675,14 +679,27 @@ Ip4ConfigProtocol (
   } else {
 //
 // Use the default address. Check the state.
 //
 if (IpSb->State == IP4_SERVICE_UNSTARTED) {
-  Status = Ip4StartAutoConfig (&IpSb->Ip4Config2Instance);
-
-  if (EFI_ERROR (Status)) {
-goto ON_ERROR;
+  //
+  // Trigger the EFI_IP4_CONFIG2_PROTOCOL to retrieve the 
+  // default IPv4 address if it is not available yet.
+  //
+  Policy = IpSb->Ip4Config2Instance.Policy;
+  if (Policy != Ip4Config2PolicyDhcp) {
+Ip4Config2 = &IpSb->Ip4Config2Instance.Ip4Config2;
+Policy = Ip4Config2PolicyDhcp;
+Status= Ip4Config2->SetData (
+  Ip4Config2,
+  Ip4Config2DataTypePolicy,
+  sizeof (EFI_IP4_CONFIG2_POLICY),
+  &Policy
+  );
+if (EFI_ERROR (Status)) {
+  goto ON_ERROR;
+}
   }
 }
 
 IpIf = IpSb->DefaultInterface;
 NET_GET_REF (IpSb->DefaultInterface);
-- 
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] UefiCpuPkg/MpInitLib: Avoid call PcdGe* in Ap & Bsp.

2017-10-19 Thread Dong, Eric

Hi Laszlo & Ruiyu,

I have fix the typo and push the change. Thanks.

Thanks,
Eric
> -Original Message-
> From: Ni, Ruiyu
> Sent: Thursday, October 19, 2017 5:23 PM
> To: Laszlo Ersek ; Dong, Eric ;
> edk2-devel@lists.01.org
> Cc: Crystal Lee 
> Subject: RE: [edk2] [Patch] UefiCpuPkg/MpInitLib: Avoid call PcdGe* in Ap &
> Bsp.
> 
> With the subject change suggested by Laszlo, Reviewed-by: Ruiyu Ni
> 
> 
> Thanks/Ray
> 
> > -Original Message-
> > From: Laszlo Ersek [mailto:ler...@redhat.com]
> > Sent: Thursday, October 19, 2017 4:08 PM
> > To: Dong, Eric ; edk2-devel@lists.01.org
> > Cc: Ni, Ruiyu ; Crystal Lee
> > 
> > Subject: Re: [edk2] [Patch] UefiCpuPkg/MpInitLib: Avoid call PcdGe* in
> > Ap & Bsp.
> >
> > On 10/19/17 04:42, Eric Dong wrote:
> > > MicrocodeDetect function will run by every threads, and it will use
> > > PcdGet to get PcdCpuMicrocodePatchAddress and
> > > PcdCpuMicrocodePatchRegionSize, if change both PCD default to
> > > dynamic, system will in non-deterministic behavior.
> > >
> > > By design, UEFI/PI services are single threaded and not re-entrant
> > > so Multi processor code should not use UEFI/PI services. Here, Pcd
> > > protocol/PPI is used to access dynamic PCDs so it would result in
> > > non-deterministic behavior.
> > >
> > > This code get PCD value in BSP and save them in CPU_MP_DATA for Ap.
> > >
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=726
> > >
> > > Cc: Crystal Lee 
> > > Cc: Ruiyu Ni 
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Eric Dong 
> > > ---
> > >  UefiCpuPkg/Library/MpInitLib/Microcode.c | 10 +++---
> > >  UefiCpuPkg/Library/MpInitLib/MpLib.c |  2 ++
> > >  UefiCpuPkg/Library/MpInitLib/MpLib.h |  2 ++
> > >  3 files changed, 7 insertions(+), 7 deletions(-)
> >
> > Eric, can you please fix up the subject line after you get an R-b and
> > are about to push the patch?
> >
> > -UefiCpuPkg/MpInitLib: Avoid call PcdGe* in Ap & Bsp.
> > +UefiCpuPkg/MpInitLib: Avoid call PcdGet* in Ap & Bsp.
> >
> > Thanks!
> > Laszlo
> >
> >
> > > diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > > b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > > index 982995b..35f66f7 100644
> > > --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > > +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > > @@ -42,8 +42,6 @@ MicrocodeDetect (
> > >IN CPU_MP_DATA *CpuMpData
> > >)
> > >  {
> > > -  UINT64  MicrocodePatchAddress;
> > > -  UINT64  MicrocodePatchRegionSize;
> > >UINT32  ExtendedTableLength;
> > >UINT32  ExtendedTableCount;
> > >CPU_MICROCODE_EXTENDED_TABLE*ExtendedTable;
> > > @@ -61,9 +59,7 @@ MicrocodeDetect (
> > >VOID*MicrocodeData;
> > >MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
> > >
> > > -  MicrocodePatchAddress= PcdGet64
> (PcdCpuMicrocodePatchAddress);
> > > -  MicrocodePatchRegionSize = PcdGet64
> > > (PcdCpuMicrocodePatchRegionSize);
> > > -  if (MicrocodePatchRegionSize == 0) {
> > > +  if (CpuMpData->MicrocodePatchRegionSize == 0) {
> > >  //
> > >  // There is no microcode patches
> > >  //
> > > @@ -93,8 +89,8 @@ MicrocodeDetect (
> > >
> > >LatestRevision = 0;
> > >MicrocodeData  = NULL;
> > > -  MicrocodeEnd = (UINTN) (MicrocodePatchAddress +
> > > MicrocodePatchRegionSize);
> > > -  MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN)
> > > MicrocodePatchAddress;
> > > +  MicrocodeEnd = (UINTN) (CpuMpData->MicrocodePatchAddress +
> > > + CpuMpData->MicrocodePatchRegionSize);
> > > +  MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN)
> > > + CpuMpData->MicrocodePatchAddress;
> > >do {
> > >  //
> > >  // Check if the microcode is for the Cpu and the version is
> > > newer diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > > b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > > index 924b909..f3ee6d4 100644
> > > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > > @@ -1458,6 +1458,8 @@ 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);
> > >InitializeSpinLock(&CpuMpData->MpLock);
> > >//
> > >// Save BSP's Control registers to APs diff --git
> > > a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > > b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > > index 19defda..84ae24f 100644
> > > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > > @@ -233,6 +233,8 @@ struct

Re: [edk2] [patch 4/4] UefiCpuPkg/MtrrLib: Make comments align with function

2017-10-19 Thread Ni, Ruiyu

Dandan,
Thanks for fixing that.

Reviewed-by: Ruiyu Ni 

-Original Message-
From: Bi, Dandan 
Sent: Friday, October 20, 2017 9:00 AM
To: edk2-devel@lists.01.org
Cc: Dong, Eric ; Ni, Ruiyu 
Subject: [patch 4/4] UefiCpuPkg/MtrrLib: Make comments align with function

Cc: Eric Dong 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 UefiCpuPkg/Include/Library/MtrrLib.h | 2 +-  
UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/Include/Library/MtrrLib.h 
b/UefiCpuPkg/Include/Library/MtrrLib.h
index d05d839..0bf7d8e 100644
--- a/UefiCpuPkg/Include/Library/MtrrLib.h
+++ b/UefiCpuPkg/Include/Library/MtrrLib.h
@@ -373,11 +373,11 @@ MtrrSetMemoryAttributeInMtrrSettings (
 needs more scratch buffer.
   @param[in]   Ranges   Pointer to an array of MTRR_MEMORY_RANGE.
 When range overlap happens, the last one takes 
higher priority.
 When the function returns, either all the 
attributes are set successfully,
 or none of them is set.
-  @param[in]Count of MTRR_MEMORY_RANGE.
+  @param[in]  RangeCountCount of MTRR_MEMORY_RANGE.
 
   @retval RETURN_SUCCESSThe attributes were set for all the memory 
ranges.
   @retval RETURN_INVALID_PARAMETER  Length in any range is zero.
   @retval RETURN_UNSUPPORTEDThe processor does not support one or more 
bytes of the
 memory resource range specified by 
BaseAddress and Length in any range.
diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c 
b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
index cb22558..579af27 100644
--- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
+++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
@@ -2146,11 +2146,11 @@ MtrrLibSetBelow1MBMemoryAttribute (
 needs more scratch buffer.
   @param[in]   Ranges   Pointer to an array of MTRR_MEMORY_RANGE.
 When range overlap happens, the last one takes 
higher priority.
 When the function returns, either all the 
attributes are set successfully,
 or none of them is set.
-  @param[in]Count of MTRR_MEMORY_RANGE.
+  @param[in]   RangeCount   Count of MTRR_MEMORY_RANGE.
 
   @retval RETURN_SUCCESSThe attributes were set for all the memory 
ranges.
   @retval RETURN_INVALID_PARAMETER  Length in any range is zero.
   @retval RETURN_UNSUPPORTEDThe processor does not support one or more 
bytes of the
 memory resource range specified by 
BaseAddress and Length in any range.
--
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 v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM mode

2017-10-19 Thread Wang, Jian J

I forgot the dependency part. If so, it's ok to me to add a new protocol like 
gEdkiiSmmMemoryAttributeProtocol. I think this protocol will l be implemented 
in PiSmmCpuDxeSmm driver. If no other comments on this, I'll update the code 
and send the v3 patch soon.

From: Yao, Jiewen
Sent: Friday, October 20, 2017 9:37 AM
To: Wang, Jian J ; edk2-devel@lists.01.org
Cc: Zeng, Star ; Dong, Eric ; Kinney, 
Michael D 
Subject: RE: [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard 
feature for SMM mode

I remember MdeModulePkg cannot depend on UefiCpuPkg. Is that correct, Mike?

Maybe we can define a new one in MdeModulePkg, such as 
gEdkiiSmmMemoryAttributeProtocol.

Thank you
Yao Jiewen

From: Wang, Jian J
Sent: Friday, October 20, 2017 8:55 AM
To: Yao, Jiewen mailto:jiewen@intel.com>>; 
edk2-devel@lists.01.org
Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, Eric 
mailto:eric.d...@intel.com>>; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: RE: [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard 
feature for SMM mode

I took a look at current available protocols and found that we have already 
gEfiSmmCpuProtocolGuid and gEfiSmmCpuServiceProtocolGuid. Just like 
gEfiCpuArchProtocolGuid which provides the API to update memory attributes 
(MTRR and paging), how about we add new interfaces to gEfiSmmCpuProtocolGuid or 
gEfiSmmCpuServiceProtocolGuid (I'm not sure which is more appropriate for this 
situation)?

gEfiSmmCpuProtocolGuid is defined in MdePkg. I would assume it's spec related. 
gEfiSmmCpuServiceProtocolGuid is defined in UefiCpuPkg, which looks like to be 
a better candidate.

From: Yao, Jiewen
Sent: Wednesday, October 18, 2017 1:55 PM
To: Wang, Jian J mailto:jian.j.w...@intel.com>>; 
edk2-devel@lists.01.org
Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, Eric 
mailto:eric.d...@intel.com>>; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: RE: [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard 
feature for SMM mode

I do not think there is interface *change*.
We can define a *new* interface in MdeModulePkg\Include\Protocol.

Thank you
Yao Jiewen

From: Wang, Jian J
Sent: Wednesday, October 18, 2017 1:52 PM
To: Yao, Jiewen mailto:jiewen@intel.com>>; 
edk2-devel@lists.01.org
Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, Eric 
mailto:eric.d...@intel.com>>; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: RE: [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard 
feature for SMM mode

Yes, we can. But that also means public interfaces changes, which might affect 
internal/external users. Any formal procedure required to make such kind of 
changes?

From: Yao, Jiewen
Sent: Wednesday, October 18, 2017 1:07 PM
To: Wang, Jian J mailto:jian.j.w...@intel.com>>; 
edk2-devel@lists.01.org
Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, Eric 
mailto:eric.d...@intel.com>>; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: RE: [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard 
feature for SMM mode

Hi
I am a little worried about adding page table management in PiSmmCore directly.

Can we define an interface between PiSmmCore and PiSmmCpu driver to set memory 
attribute? Like what we did in DxeCore and DxeCpu driver.

Thank you
Yao Jiewen

From: Wang, Jian J
Sent: Tuesday, October 17, 2017 9:29 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, Eric 
mailto:eric.d...@intel.com>>; Yao, Jiewen 
mailto:jiewen@intel.com>>; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard feature 
for SMM mode

> According to Eric's feedback:
> a. Remove local variable initializer with memory copy from globals
> b. Change map table dump code to use DEBUG_PAGE|DEBUG_POOL level
>message
> c. Remove unnecessary debug code
> d. Change name of function InitializePageTableLib to
>InitializePageTableGlobals
>
> Other changes:
> e. Fix issues in 32-bit boot mode
> f. Coding style cleanup

This feature makes use of paging mechanism to add a hidden (not present)
page just before and after the allocated memory block. If the code tries
to access memory outside of the allocated part, page fault exception will
be triggered.

This feature is controlled by three PCDs:

gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType

BIT2 and BIT3 of PcdHeapGuardPropertyMask can be used to enable or disable
memory guard for SMM page and pool respectively. PcdHeapGuardPoolType and/or
PcdHeapGuardPageType are used to enable or disable guard for specific type
of memory. For example, we can turn on guard only for EfiRuntimeServicesCode
and EfiRuntimeServicesData

Re: [edk2] [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM mode

2017-10-19 Thread Yao, Jiewen

I remember MdeModulePkg cannot depend on UefiCpuPkg. Is that correct, Mike?

Maybe we can define a new one in MdeModulePkg, such as 
gEdkiiSmmMemoryAttributeProtocol.

Thank you
Yao Jiewen


From: Wang, Jian J
Sent: Friday, October 20, 2017 8:55 AM
To: Yao, Jiewen ; edk2-devel@lists.01.org
Cc: Zeng, Star ; Dong, Eric ; Kinney, 
Michael D 
Subject: RE: [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard 
feature for SMM mode

I took a look at current available protocols and found that we have already 
gEfiSmmCpuProtocolGuid and gEfiSmmCpuServiceProtocolGuid. Just like 
gEfiCpuArchProtocolGuid which provides the API to update memory attributes 
(MTRR and paging), how about we add new interfaces to gEfiSmmCpuProtocolGuid or 
gEfiSmmCpuServiceProtocolGuid (I'm not sure which is more appropriate for this 
situation)?

gEfiSmmCpuProtocolGuid is defined in MdePkg. I would assume it's spec related. 
gEfiSmmCpuServiceProtocolGuid is defined in UefiCpuPkg, which looks like to be 
a better candidate.

From: Yao, Jiewen
Sent: Wednesday, October 18, 2017 1:55 PM
To: Wang, Jian J mailto:jian.j.w...@intel.com>>; 
edk2-devel@lists.01.org
Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, Eric 
mailto:eric.d...@intel.com>>; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: RE: [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard 
feature for SMM mode

I do not think there is interface *change*.
We can define a *new* interface in MdeModulePkg\Include\Protocol.

Thank you
Yao Jiewen

From: Wang, Jian J
Sent: Wednesday, October 18, 2017 1:52 PM
To: Yao, Jiewen mailto:jiewen@intel.com>>; 
edk2-devel@lists.01.org
Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, Eric 
mailto:eric.d...@intel.com>>; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: RE: [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard 
feature for SMM mode

Yes, we can. But that also means public interfaces changes, which might affect 
internal/external users. Any formal procedure required to make such kind of 
changes?

From: Yao, Jiewen
Sent: Wednesday, October 18, 2017 1:07 PM
To: Wang, Jian J mailto:jian.j.w...@intel.com>>; 
edk2-devel@lists.01.org
Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, Eric 
mailto:eric.d...@intel.com>>; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: RE: [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard 
feature for SMM mode

Hi
I am a little worried about adding page table management in PiSmmCore directly.

Can we define an interface between PiSmmCore and PiSmmCpu driver to set memory 
attribute? Like what we did in DxeCore and DxeCpu driver.

Thank you
Yao Jiewen

From: Wang, Jian J
Sent: Tuesday, October 17, 2017 9:29 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, Eric 
mailto:eric.d...@intel.com>>; Yao, Jiewen 
mailto:jiewen@intel.com>>; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard feature 
for SMM mode

> According to Eric's feedback:
> a. Remove local variable initializer with memory copy from globals
> b. Change map table dump code to use DEBUG_PAGE|DEBUG_POOL level
>message
> c. Remove unnecessary debug code
> d. Change name of function InitializePageTableLib to
>InitializePageTableGlobals
>
> Other changes:
> e. Fix issues in 32-bit boot mode
> f. Coding style cleanup

This feature makes use of paging mechanism to add a hidden (not present)
page just before and after the allocated memory block. If the code tries
to access memory outside of the allocated part, page fault exception will
be triggered.

This feature is controlled by three PCDs:

gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType

BIT2 and BIT3 of PcdHeapGuardPropertyMask can be used to enable or disable
memory guard for SMM page and pool respectively. PcdHeapGuardPoolType and/or
PcdHeapGuardPageType are used to enable or disable guard for specific type
of memory. For example, we can turn on guard only for EfiRuntimeServicesCode
and EfiRuntimeServicesData by setting the PCD with value 0x60.

Pool memory is not ususally integer multiple of one page, and is more likely
less than a page. There's no way to monitor the overflow at both top and
bottom of pool memory. BIT7 of PcdHeapGuardPropertyMask is used to control
how to position the head of pool memory so that it's easier to catch memory
overflow in memory growing direction or in decreasing direction.

Cc: Star Zeng mailto:star.z...@intel.com>>
Cc: Eric Dong mailto:eric.d...@intel.com>>
Cc: Jiewen Yao mailto:jiewen@intel.com>>
Cc: Michael Kinney 
mailto:michael.d.kin...@intel.com>>
Suggested-by: Ayellet Wolman 
mailto:ayellet.wol...@intel.com>>
Contribute

[edk2] [patch 3/4] MdeModulePkg/DxeIplPeim: Refine coding style in function comments

2017-10-19 Thread Dandan Bi

Make the comments align with the function.
And add some missing function comments.

Cc: Jian J Wang 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.h| 14 +++---
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 15 +++
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h 
b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
index ecf1866..f3aabdb 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
@@ -239,28 +239,28 @@ Decompress (
   OUT   VOID**OutputBuffer,
   OUT   UINTN   *OutputSize
   );
 
 /**
-   Clear legacy memory located at the first 4K-page.
+  Clear legacy memory located at the first 4K-page.
 
-   This function traverses the whole HOB list to check if memory from 0 to 4095
-   exists and has not been allocated, and then clear it if so.
+  This function traverses the whole HOB list to check if memory from 0 to 4095
+  exists and has not been allocated, and then clear it if so.
 
-   @param HoStart The start of HobList passed to DxeCore.
+  @param HobStart The start of HobList passed to DxeCore.
 
 **/
 VOID
 ClearFirst4KPage (
   IN  VOID *HobStart
   );
 
 /**
-   Return configure status of NULL pointer detection feature
+  Return configure status of NULL pointer detection feature.
 
-   @return TRUE   NULL pointer detection feature is enabled
-   @return FALSE  NULL pointer detection feature is disabled
+  @return TRUE   NULL pointer detection feature is enabled
+  @return FALSE  NULL pointer detection feature is disabled
 **/
 BOOLEAN
 IsNullDetectionEnabled (
   VOID
   );
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c 
b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index a10dea2..29b6205 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -30,16 +30,16 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 #include "DxeIpl.h"
 #include "VirtualMemory.h"
 
 /**
-   Clear legacy memory located at the first 4K-page, if available.
+  Clear legacy memory located at the first 4K-page, if available.
 
-   This function traverses the whole HOB list to check if memory from 0 to 4095
-   exists and has not been allocated, and then clear it if so.
+  This function traverses the whole HOB list to check if memory from 0 to 4095
+  exists and has not been allocated, and then clear it if so.
 
-   @param HoStart   The start of HobList passed to DxeCore.
+  @param HobStart  The start of HobList passed to DxeCore.
 
 **/
 VOID
 ClearFirst4KPage (
   IN  VOID *HobStart
@@ -84,10 +84,17 @@ ClearFirst4KPage (
   }
 
   return;
 }
 
+/**
+  Return configure status of NULL pointer detection feature.
+
+  @return TRUE   NULL pointer detection feature is enabled
+  @return FALSE  NULL pointer detection feature is disabled
+
+**/
 BOOLEAN
 IsNullDetectionEnabled (
   VOID
   )
 {
-- 
1.9.5.msysgit.1

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

[edk2] [patch 4/4] UefiCpuPkg/MtrrLib: Make comments align with function

2017-10-19 Thread Dandan Bi

Cc: Eric Dong 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 UefiCpuPkg/Include/Library/MtrrLib.h | 2 +-
 UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/Include/Library/MtrrLib.h 
b/UefiCpuPkg/Include/Library/MtrrLib.h
index d05d839..0bf7d8e 100644
--- a/UefiCpuPkg/Include/Library/MtrrLib.h
+++ b/UefiCpuPkg/Include/Library/MtrrLib.h
@@ -373,11 +373,11 @@ MtrrSetMemoryAttributeInMtrrSettings (
 needs more scratch buffer.
   @param[in]   Ranges   Pointer to an array of MTRR_MEMORY_RANGE.
 When range overlap happens, the last one takes 
higher priority.
 When the function returns, either all the 
attributes are set successfully,
 or none of them is set.
-  @param[in]Count of MTRR_MEMORY_RANGE.
+  @param[in]  RangeCountCount of MTRR_MEMORY_RANGE.
 
   @retval RETURN_SUCCESSThe attributes were set for all the memory 
ranges.
   @retval RETURN_INVALID_PARAMETER  Length in any range is zero.
   @retval RETURN_UNSUPPORTEDThe processor does not support one or more 
bytes of the
 memory resource range specified by 
BaseAddress and Length in any range.
diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c 
b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
index cb22558..579af27 100644
--- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
+++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
@@ -2146,11 +2146,11 @@ MtrrLibSetBelow1MBMemoryAttribute (
 needs more scratch buffer.
   @param[in]   Ranges   Pointer to an array of MTRR_MEMORY_RANGE.
 When range overlap happens, the last one takes 
higher priority.
 When the function returns, either all the 
attributes are set successfully,
 or none of them is set.
-  @param[in]Count of MTRR_MEMORY_RANGE.
+  @param[in]   RangeCount   Count of MTRR_MEMORY_RANGE.
 
   @retval RETURN_SUCCESSThe attributes were set for all the memory 
ranges.
   @retval RETURN_INVALID_PARAMETER  Length in any range is zero.
   @retval RETURN_UNSUPPORTEDThe processor does not support one or more 
bytes of the
 memory resource range specified by 
BaseAddress and Length in any range.
-- 
1.9.5.msysgit.1

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

[edk2] [patch 2/4] IntelFrameworkModule/LegacyBios: Avoid explicit comparison for BOOLEAN

2017-10-19 Thread Dandan Bi

Cc: Jian J Wang 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c 
b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
index e00..c6461f5 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
@@ -804,11 +804,11 @@ EnableNullDetection (
   EFI_STATUSStatus;
   EFI_GCD_MEMORY_SPACE_DESCRIPTOR   Desc;
 
   if (((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) == 0)
   ||
-  ((mEndOfDxe == TRUE)  &&
+  ((mEndOfDxe)  &&
((PcdGet8 (PcdNullPointerDetectionPropertyMask) & (BIT7|BIT0))
 == (BIT7|BIT0)))
  ) {
 return;
   }
@@ -854,11 +854,11 @@ DisableNullDetection (
   EFI_STATUSStatus;
   EFI_GCD_MEMORY_SPACE_DESCRIPTOR   Desc;
 
   if (((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) == 0)
   ||
-  ((mEndOfDxe == TRUE)  &&
+  ((mEndOfDxe)  &&
((PcdGet8 (PcdNullPointerDetectionPropertyMask) & (BIT7|BIT0))
 == (BIT7|BIT0)))
  ) {
 return;
   }
-- 
1.9.5.msysgit.1

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

[edk2] [patch 1/4] IntelFrameworkModulePkg/Csm: Refine coding style in comments

2017-10-19 Thread Dandan Bi

Make the comments follow Edk2 coding style:
1. Make the comments starts with /** and end with **/.
2. Make the comments descrition end with '.'

Cc: Jian J Wang 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c | 8 
 IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c   | 8 
 IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h  | 8 
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c 
b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
index d2224a2..ebf03d3 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
@@ -1731,12 +1731,12 @@ CheckKeyboardConnect (
 return TRUE;
   }
 }
 
 /**
-   Disable NULL pointer detection
-*/
+  Disable NULL pointer detection.
+**/
 VOID
 DisableNullDetection (
   VOID
   )
 {
@@ -1778,12 +1778,12 @@ DisableNullDetection (
 DEBUG ((DEBUG_WARN, "!!! Page 0 is supposed to be disabled !!!\r\n"));
   }
 }
 
 /**
-   Enable NULL pointer detection
-*/
+   Enable NULL pointer detection.
+**/
 VOID
 EnableNullDetection (
   VOID
   )
 {
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c 
b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
index 3176a98..e00 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
@@ -792,12 +792,12 @@ ToggleEndOfDxeStatus (
 // functions can be used to disable/enable NULL pointer detection before/after
 // accessing those memory.
 //
 
 /**
-   Enable NULL pointer detection
-*/
+   Enable NULL pointer detection.
+**/
 VOID
 EnableNullDetection (
   VOID
   )
 {
@@ -842,12 +842,12 @@ EnableNullDetection (
 ASSERT_EFI_ERROR (Status);
   }
 }
 
 /**
-   Disable NULL pointer detection
-*/
+   Disable NULL pointer detection.
+**/
 VOID
 DisableNullDetection (
   VOID
   )
 {
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h 
b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
index 20dfef3..86a3b09 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
@@ -1543,20 +1543,20 @@ EFI_STATUS
 LegacyBiosInstallVgaRom (
   IN  LEGACY_BIOS_INSTANCE*Private
   );
 
 /**
-   Enable NULL pointer detection
-*/
+   Enable NULL pointer detection.
+**/
 VOID
 EnableNullDetection (
   VOID
   );
 
 /**
-   Disable NULL pointer detection
-*/
+   Disable NULL pointer detection.
+**/
 VOID
 DisableNullDetection (
   VOID
   );
 
-- 
1.9.5.msysgit.1

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

[edk2] [patch 0/4] Fix coding style issue

2017-10-19 Thread Dandan Bi

This pacth series maily refine the coding style,
such as adding some missing comments, making comments
alin with function...

Dandan Bi (4):
  IntelFrameworkModulePkg/Csm: Refine coding style in comments
  IntelFrameworkModule/LegacyBios: not use explicit comparisons for
BOOLEAN type
  MdeModulePkg/DxeIplPeim: Refine coding style in function comments
  UefiCpuPkg/MtrrLib: Make comments align with function

 .../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c  |  8 
 IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c| 12 ++--
 .../Csm/LegacyBiosDxe/LegacyBiosInterface.h   |  8 
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.h | 14 +++---
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c  | 15 +++
 UefiCpuPkg/Include/Library/MtrrLib.h  |  2 +-
 UefiCpuPkg/Library/MtrrLib/MtrrLib.c  |  2 +-
 7 files changed, 34 insertions(+), 27 deletions(-)

-- 
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 v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM mode

2017-10-19 Thread Wang, Jian J

I took a look at current available protocols and found that we have already 
gEfiSmmCpuProtocolGuid and gEfiSmmCpuServiceProtocolGuid. Just like 
gEfiCpuArchProtocolGuid which provides the API to update memory attributes 
(MTRR and paging), how about we add new interfaces to gEfiSmmCpuProtocolGuid or 
gEfiSmmCpuServiceProtocolGuid (I'm not sure which is more appropriate for this 
situation)?

gEfiSmmCpuProtocolGuid is defined in MdePkg. I would assume it's spec related. 
gEfiSmmCpuServiceProtocolGuid is defined in UefiCpuPkg, which looks like to be 
a better candidate.

From: Yao, Jiewen
Sent: Wednesday, October 18, 2017 1:55 PM
To: Wang, Jian J ; edk2-devel@lists.01.org
Cc: Zeng, Star ; Dong, Eric ; Kinney, 
Michael D 
Subject: RE: [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard 
feature for SMM mode

I do not think there is interface *change*.
We can define a *new* interface in MdeModulePkg\Include\Protocol.

Thank you
Yao Jiewen

From: Wang, Jian J
Sent: Wednesday, October 18, 2017 1:52 PM
To: Yao, Jiewen mailto:jiewen@intel.com>>; 
edk2-devel@lists.01.org
Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, Eric 
mailto:eric.d...@intel.com>>; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: RE: [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard 
feature for SMM mode

Yes, we can. But that also means public interfaces changes, which might affect 
internal/external users. Any formal procedure required to make such kind of 
changes?

From: Yao, Jiewen
Sent: Wednesday, October 18, 2017 1:07 PM
To: Wang, Jian J mailto:jian.j.w...@intel.com>>; 
edk2-devel@lists.01.org
Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, Eric 
mailto:eric.d...@intel.com>>; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: RE: [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard 
feature for SMM mode

Hi
I am a little worried about adding page table management in PiSmmCore directly.

Can we define an interface between PiSmmCore and PiSmmCpu driver to set memory 
attribute? Like what we did in DxeCore and DxeCpu driver.

Thank you
Yao Jiewen

From: Wang, Jian J
Sent: Tuesday, October 17, 2017 9:29 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, Eric 
mailto:eric.d...@intel.com>>; Yao, Jiewen 
mailto:jiewen@intel.com>>; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard feature 
for SMM mode

> According to Eric's feedback:
> a. Remove local variable initializer with memory copy from globals
> b. Change map table dump code to use DEBUG_PAGE|DEBUG_POOL level
>message
> c. Remove unnecessary debug code
> d. Change name of function InitializePageTableLib to
>InitializePageTableGlobals
>
> Other changes:
> e. Fix issues in 32-bit boot mode
> f. Coding style cleanup

This feature makes use of paging mechanism to add a hidden (not present)
page just before and after the allocated memory block. If the code tries
to access memory outside of the allocated part, page fault exception will
be triggered.

This feature is controlled by three PCDs:

gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType

BIT2 and BIT3 of PcdHeapGuardPropertyMask can be used to enable or disable
memory guard for SMM page and pool respectively. PcdHeapGuardPoolType and/or
PcdHeapGuardPageType are used to enable or disable guard for specific type
of memory. For example, we can turn on guard only for EfiRuntimeServicesCode
and EfiRuntimeServicesData by setting the PCD with value 0x60.

Pool memory is not ususally integer multiple of one page, and is more likely
less than a page. There's no way to monitor the overflow at both top and
bottom of pool memory. BIT7 of PcdHeapGuardPropertyMask is used to control
how to position the head of pool memory so that it's easier to catch memory
overflow in memory growing direction or in decreasing direction.

Cc: Star Zeng mailto:star.z...@intel.com>>
Cc: Eric Dong mailto:eric.d...@intel.com>>
Cc: Jiewen Yao mailto:jiewen@intel.com>>
Cc: Michael Kinney 
mailto:michael.d.kin...@intel.com>>
Suggested-by: Ayellet Wolman 
mailto:ayellet.wol...@intel.com>>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang mailto:jian.j.w...@intel.com>>
---
 MdeModulePkg/Core/PiSmmCore/Misc/HeapGuard.c | 1446 ++
 MdeModulePkg/Core/PiSmmCore/Misc/HeapGuard.h |  400 +++
 MdeModulePkg/Core/PiSmmCore/Misc/PageTable.c |  704 +
 MdeModulePkg/Core/PiSmmCore/Misc/PageTable.h |  174 
 MdeModulePkg/Core/PiSmmCore/Page.c   |   51 +-
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.c  |   12 +-
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h  |   80 +-
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf|

[edk2] dynamic PCD impact on temporary PEI memory

2017-10-19 Thread Laszlo Ersek

Hi,

I've encountered an interesting phenomenon, and I'd like to discuss it
before I submit an OvmfPkg patch.

* Background #1:

OVMF uses 32KB total memory for temporary SEC/PEI heap and stack,
combined. The fixed PCDs for the base address of this (combined) area,
and for its size, are set in the OVMF FDF files, uniformly between IA32
/ X64 / IA32X64:

- PcdOvmfSecPeiTempRamBase = 0x0081 (8MB + 64KB)
- PcdOvmfSecPeiTempRamSize = 0x8000 (32KB)

See:

> [FD.MEMFD]
> BaseAddress   = $(MEMFD_BASE_ADDRESS)
> [...]
> 0x007000|0x001000
> gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
>
> 0x01|0x008000
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
>
> 0x02|0x0E
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
> [...]

and "MEMFD_BASE_ADDRESS" comes from "OvmfPkg.fdf.inc":

> DEFINE MEMFD_BASE_ADDRESS = 0x80

(There is a reason why I also quoted
"PcdGuidedExtractHandlerTableAddress" and "PcdOvmfPeiMemFvBase" above;
but more on that later.)

Half of this memory (16KB) is used for stack, the other half is used for
heap.


* Background #2:

"MdeModulePkg/Universal/PCD/Pei/Pcd.inf" writes (rewrapping it here for
better readability):

> #3.1 PcdPeim and PcdDxe
> #  PEI PCD database is maintained by PcdPeim driver run from
> #  flash. PcdPeim driver build guid hob in temporary memory and
> #  copy the binary data base from flash to temporary memory for
> #  PEI PCD database.
> #  DXE PCD database is maintained by PcdDxe driver.At entry point
> #  of PcdDxe driver, a new PCD database is allocated in boot-time
> #  memory which including all PEI PCD and DXE PCD entry.

This means that the more dynamic PCDs we have in the PEI phase
(cumulatively across all PEIMs), the larger the PCD HOB will be, and the
more temporary PEI RAM (heap) will be needed.


* Background #3:

Recently we added yet another dynamic PCD to
"OvmfPkg/PlatformPei/PlatformPei.inf" -- and with that, to OVMF's PEI
phase at all --, namely

  gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy

which has type UINT32.

Unfortunately, I messed up the patch review a bit, so I cannot point to
direct neighbor commits, from which the first doesn't have the PCD in
PlatformPei just yet, and the other commit (right after) does. There's a
build error between these two points in the git history, so the commits
that can be tested like described above are not adjacent.

Anyway, the nearest points in history that *can* be used for comparison
are:

- the commit just before adding the PCD to PlatformPei; namely
  071f1d19ddbc ("SecurityPkg: make PcdOptionRomImageVerificationPolicy
  dynamic", 2017-10-05);

- the commit when OVMF can be built again (without -D
  SECURE_BOOT_ENABLE); namely 1958124a6cb0 ("OvmfPkg: fix dynamic
  default for oprom verification policy PCD without SB", 2017-10-17).

Below I'm going to refer to these commits as "before" and "after".


* Symptom #1:

I built OVMF for IA32, IA32X64 and X64, both "before" and "after". Then
I compared the log files, to see the impact of the addition of exactly
one UINT32 dynamic PCD to the PCD HOB, on temporary PEI memory usage.

- Diff between "before" and "after", for IA32:

>  Temp Stack : BaseAddress=0x814000 Length=0x4000
>  Temp Heap  : BaseAddress=0x81 Length=0x4000
>  Total temporary memory:32768 bytes.
>temporary memory stack ever used:   16384 bytes.
> -  temporary memory heap used for HobList: 5616 bytes.
> +  temporary memory heap used for HobList: 5664 bytes.
>temporary memory heap occupied by memory pages: 0 bytes.

- Diff between "before" and "after", for IA32X64:

>  Temp Stack : BaseAddress=0x814000 Length=0x4000
>  Temp Heap  : BaseAddress=0x81 Length=0x4000
>  Total temporary memory:32768 bytes.
>temporary memory stack ever used:   16384 bytes.
> -  temporary memory heap used for HobList: 5616 bytes.
> +  temporary memory heap used for HobList: 5664 bytes.
>temporary memory heap occupied by memory pages: 0 bytes.

- Diff between "before" and "after", for IA32X64:

>  Temp Stack : BaseAddress=0x814000 Length=0x4000
>  Temp Heap  : BaseAddress=0x81 Length=0x4000
>  Total temporary memory:32768 bytes.
>temporary memory stack ever used:   16384 bytes.
> -  temporary memory heap used for HobList: 7952 bytes.
> +  temporary memory heap used for HobList: 8032 bytes.
>temporary memory heap occupied by memory pages: 0 bytes.

Observations:

- The IA32 and IA32X64 builds are identical in this regard (not
  surprisingly; the PEI phase is 32-bit in both).

- When PEI is 32-bit, the increase is 48 bytes.

- When PEI is 64-bit, the increase is 80 bytes.

- The *basis* (i.e., "before") heap consumption in 64-bit PEI is
  significantly higher than in 32-bit PEI (7952 bytes vs. 5616 bytes).
  And, the addition

Re: [edk2] [PATCH] EmbeddedPkg/DtPlatformDxe: remove /chosen/stdout-path on GOP registration

2017-10-19 Thread Leif Lindholm

On Thu, Oct 19, 2017 at 10:19:44PM +0100, Ard Biesheuvel wrote:
> On 19 October 2017 at 21:55, Leif Lindholm  wrote:
> > On Thu, Oct 19, 2017 at 08:21:41PM +0100, Ard Biesheuvel wrote:
> >> The presence of a /chosen/stdout-path property will force Linux to use
> >> the serial port as the primary console, even if a graphical console is
> >> available as well. But the presence of the Graphics Output Protocol (GOP)
> >> is a strong indication that the user may prefer to use his keyboard and
> >> mouse rather than his terminal emulator to interact with the system, so
> >> let's remove /chosen/stdout-path as soon as a GOP instance is registered.
> >
> > This may be the sensible thing to do, but something just doesn't feel
> > right.
> >
> > Also, whatever we do here, we should try to mirror in ACPI with SPCR.
> 
> Yes, but it does not belong in this driver.

Absolutely not - but it ties in with the Pcd suggestion below.

> > Are we guaranteed to always want to disable serial console if there is
> > a graphics adapter?
> 
> This does not disable the serial console, it just makes it more
> difficult to access :-) You will have to add console=ttyAMA0 if you
> have a screen connected but want your console to appear on the serial
> port. (Just like on a normal computer)

Oh, I know - seeing those 8 characters just makes me twitch.

> > I still dream of a world in which I can run consplitter up to the
> > point where an OS takes over, and an OS installer will listen to input
> > from both sources and register its selection from there.
> 
> Yeah. I need this change to get the Debian installer to run on the FB
> console without having to add 'console=tty0' to the kernel command
> line.

Yes, I really want that too. So I'm good with the halfway house of a
dynamic preference Pcd.

> > But if we can't have that, can we have a menu setting to select preference?
> > The default can be a platform-specific Pcd.
> 
> That seems reasonable.

Thanks!

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

Re: [edk2] [PATCH] EmbeddedPkg/DtPlatformDxe: remove /chosen/stdout-path on GOP registration

2017-10-19 Thread Ard Biesheuvel

On 19 October 2017 at 21:55, Leif Lindholm  wrote:
> On Thu, Oct 19, 2017 at 08:21:41PM +0100, Ard Biesheuvel wrote:
>> The presence of a /chosen/stdout-path property will force Linux to use
>> the serial port as the primary console, even if a graphical console is
>> available as well. But the presence of the Graphics Output Protocol (GOP)
>> is a strong indication that the user may prefer to use his keyboard and
>> mouse rather than his terminal emulator to interact with the system, so
>> let's remove /chosen/stdout-path as soon as a GOP instance is registered.
>
> This may be the sensible thing to do, but something just doesn't feel
> right.
>
> Also, whatever we do here, we should try to mirror in ACPI with SPCR.
>

Yes, but it does not belong in this driver.

> Are we guaranteed to always want to disable serial console if there is
> a graphics adapter?
>

This does not disable the serial console, it just makes it more
difficult to access :-) You will have to add console=ttyAMA0 if you
have a screen connected but want your console to appear on the serial
port. (Just like on a normal computer)

> I still dream of a world in which I can run consplitter up to the
> point where an OS takes over, and an OS installer will listen to input
> from both sources and register its selection from there.
>

Yeah. I need this change to get the Debian installer to run on the FB
console without having to add 'console=tty0' to the kernel command
line.

> But if we can't have that, can we have a menu setting to select preference?
> The default can be a platform-specific Pcd.
>

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

Re: [edk2] [PATCH] EmbeddedPkg/DtPlatformDxe: remove /chosen/stdout-path on GOP registration

2017-10-19 Thread Leif Lindholm

On Thu, Oct 19, 2017 at 08:21:41PM +0100, Ard Biesheuvel wrote:
> The presence of a /chosen/stdout-path property will force Linux to use
> the serial port as the primary console, even if a graphical console is
> available as well. But the presence of the Graphics Output Protocol (GOP)
> is a strong indication that the user may prefer to use his keyboard and
> mouse rather than his terminal emulator to interact with the system, so
> let's remove /chosen/stdout-path as soon as a GOP instance is registered.

This may be the sensible thing to do, but something just doesn't feel
right.

Also, whatever we do here, we should try to mirror in ACPI with SPCR.

Are we guaranteed to always want to disable serial console if there is
a graphics adapter?

I still dream of a world in which I can run consplitter up to the
point where an OS takes over, and an OS installer will listen to input
from both sources and register its selection from there.

But if we can't have that, can we have a menu setting to select preference?
The default can be a platform-specific Pcd.

/
Leif

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c   | 41 
>  EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf |  4 ++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c 
> b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
> index 1014be2281d4..4f9ac8090fac 100644
> --- a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
> @@ -12,6 +12,7 @@
>  *
>  **/
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -54,6 +55,9 @@ STATIC HII_VENDOR_DEVICE_PATH 
> mDtPlatformDxeVendorDevicePath = {
>}
>  };
>  
> +STATIC EFI_EVENT  mRegisterGopEvent;
> +STATIC VOID   *mGopRegistration;
> +
>  STATIC
>  EFI_STATUS
>  InstallHiiPages (
> @@ -89,6 +93,32 @@ InstallHiiPages (
>return EFI_SUCCESS;
>  }
>  
> +STATIC
> +VOID
> +OnRegisterGop (
> +  IN EFI_EVENT  Event,
> +  IN VOID   *Dtb
> +  )
> +{
> +  INT32 Node;
> +  INT32 Error;
> +
> +  DEBUG ((DEBUG_INFO,
> +"%a: a GOP has been registered, removing /chosen/stdout-path from DT\n",
> +__FUNCTION__));
> +
> +  Node = fdt_path_offset (Dtb, "/chosen");
> +  if (Node < 0) {
> +return;
> +  }
> +
> +  Error = fdt_delprop (Dtb, Node, "stdout-path");
> +  if (Error) {
> +DEBUG ((DEBUG_INFO, "%a: Failed to delete 'stdout-path' property: %a\n",
> +  __FUNCTION__, fdt_strerror (Error)));
> +  }
> +}
> +
>  /**
>The entry point for DtPlatformDxe driver.
>  
> @@ -181,6 +211,17 @@ DtPlatformDxeEntryPoint (
>  __FUNCTION__));
>goto FreeDtb;
>  }
> +
> +Status = gBS->CreateEvent (EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
> +OnRegisterGop, Dtb, &mRegisterGopEvent);
> +if (EFI_ERROR (Status)) {
> +  DEBUG ((DEBUG_ERROR, "%a: failed to create event - %r\n",
> +__FUNCTION__, Status));
> +}
> +
> +Status = gBS->RegisterProtocolNotify (&gEfiGraphicsOutputProtocolGuid,
> +mRegisterGopEvent, &mGopRegistration);
> +ASSERT_EFI_ERROR (Status);
>} else {
>  ASSERT (FALSE);
>}
> diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf 
> b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
> index 45dfd9088bf0..6b331294df8a 100644
> --- a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
> +++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
> @@ -41,6 +41,7 @@ [LibraryClasses]
>BaseLib
>DebugLib
>DtPlatformDtbLoaderLib
> +  FdtLib
>HiiLib
>MemoryAllocationLib
>UefiBootServicesTableLib
> @@ -53,6 +54,9 @@ [Guids]
>gEdkiiPlatformHasAcpiGuid
>gFdtTableGuid
>  
> +[Protocols]
> +  gEfiGraphicsOutputProtocolGuid
> +
>  [Depex]
>gEfiVariableArchProtocolGuidAND
>gEfiVariableWriteArchProtocolGuid
> -- 
> 2.11.0
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] EmbeddedPkg/DtPlatformDxe: remove /chosen/stdout-path on GOP registration

2017-10-19 Thread Ard Biesheuvel

The presence of a /chosen/stdout-path property will force Linux to use
the serial port as the primary console, even if a graphical console is
available as well. But the presence of the Graphics Output Protocol (GOP)
is a strong indication that the user may prefer to use his keyboard and
mouse rather than his terminal emulator to interact with the system, so
let's remove /chosen/stdout-path as soon as a GOP instance is registered.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c   | 41 
 EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf |  4 ++
 2 files changed, 45 insertions(+)

diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c 
b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
index 1014be2281d4..4f9ac8090fac 100644
--- a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
+++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
@@ -12,6 +12,7 @@
 *
 **/
 
+#include 
 #include 
 #include 
 #include 
@@ -54,6 +55,9 @@ STATIC HII_VENDOR_DEVICE_PATH 
mDtPlatformDxeVendorDevicePath = {
   }
 };
 
+STATIC EFI_EVENT  mRegisterGopEvent;
+STATIC VOID   *mGopRegistration;
+
 STATIC
 EFI_STATUS
 InstallHiiPages (
@@ -89,6 +93,32 @@ InstallHiiPages (
   return EFI_SUCCESS;
 }
 
+STATIC
+VOID
+OnRegisterGop (
+  IN EFI_EVENT  Event,
+  IN VOID   *Dtb
+  )
+{
+  INT32 Node;
+  INT32 Error;
+
+  DEBUG ((DEBUG_INFO,
+"%a: a GOP has been registered, removing /chosen/stdout-path from DT\n",
+__FUNCTION__));
+
+  Node = fdt_path_offset (Dtb, "/chosen");
+  if (Node < 0) {
+return;
+  }
+
+  Error = fdt_delprop (Dtb, Node, "stdout-path");
+  if (Error) {
+DEBUG ((DEBUG_INFO, "%a: Failed to delete 'stdout-path' property: %a\n",
+  __FUNCTION__, fdt_strerror (Error)));
+  }
+}
+
 /**
   The entry point for DtPlatformDxe driver.
 
@@ -181,6 +211,17 @@ DtPlatformDxeEntryPoint (
 __FUNCTION__));
   goto FreeDtb;
 }
+
+Status = gBS->CreateEvent (EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
+OnRegisterGop, Dtb, &mRegisterGopEvent);
+if (EFI_ERROR (Status)) {
+  DEBUG ((DEBUG_ERROR, "%a: failed to create event - %r\n",
+__FUNCTION__, Status));
+}
+
+Status = gBS->RegisterProtocolNotify (&gEfiGraphicsOutputProtocolGuid,
+mRegisterGopEvent, &mGopRegistration);
+ASSERT_EFI_ERROR (Status);
   } else {
 ASSERT (FALSE);
   }
diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf 
b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
index 45dfd9088bf0..6b331294df8a 100644
--- a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
+++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
@@ -41,6 +41,7 @@ [LibraryClasses]
   BaseLib
   DebugLib
   DtPlatformDtbLoaderLib
+  FdtLib
   HiiLib
   MemoryAllocationLib
   UefiBootServicesTableLib
@@ -53,6 +54,9 @@ [Guids]
   gEdkiiPlatformHasAcpiGuid
   gFdtTableGuid
 
+[Protocols]
+  gEfiGraphicsOutputProtocolGuid
+
 [Depex]
   gEfiVariableArchProtocolGuidAND
   gEfiVariableWriteArchProtocolGuid
-- 
2.11.0

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

Re: [edk2] [PATCH v5 1/2] UefiCpuPkg/SmmCpuFeaturesLib: Use global variables to replace macros

2017-10-19 Thread Duran, Leo



> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, October 19, 2017 2:01 AM
> To: Duran, Leo ; 'Yao, Jiewen'
> ; Paolo Bonzini ; edk2-
> de...@lists.01.org
> Cc: Ni, Ruiyu ; Justen, Jordan L
> ; Gao, Liming ; Kinney,
> Michael D 
> Subject: Re: [edk2] [PATCH v5 1/2] UefiCpuPkg/SmmCpuFeaturesLib: Use
> global variables to replace macros
> 
> On 10/18/17 16:36, Duran, Leo wrote:
> > Yao, Paolo, Lazlo, et al,
> >
> > The offset disparity has proved manageable within SmmCpuFeaturesLib, so
> I’m inclined to leave macro definitions as-is and just drop this patch-set.
> >
> > Agreed?
> 
> Sure.
> 
> Out of curiosity, what does it mean for you that the offset disparity is
> manageable in SmmCpuFeaturesLib? Will you send a different patch (set) for
> UefiCpuPkg/SmmCpuFeaturesLib, or does it concern an SmmCpuFeaturesLib
> instance that is private to AMD?
Lazlo,

It meant that I can account for the offset disparities in a platform-specific 
instance of the SmmCpuFeaturesLib library.
And no, I will not be pushing changes to the default SmmCpuFeaturesLib library 
in EDKII... Nor to the PiSmmCpuDxeSmm driver.

Thanks,
Leo.

> 
> Thanks!
> Laszlo
> 
> 
> > From: Yao, Jiewen [mailto:jiewen@intel.com]
> > Sent: Tuesday, October 17, 2017 8:51 PM
> > To: Laszlo Ersek ; Duran, Leo 
> > Cc: Ni, Ruiyu ; Justen, Jordan L
> > ; edk2-devel@lists.01.org; Gao, Liming
> > ; Kinney, Michael D
> > ; Paolo Bonzini 
> > Subject: RE: [edk2] [PATCH v5 1/2] UefiCpuPkg/SmmCpuFeaturesLib: Use
> > global variables to replace macros
> >
> > Thanks. Then I think Paolo’s suggestion makes more sense.
> >
> > If we can use same off, that would be great.
> > If no, just check AMD version ID in SaveState.
> >
> > Thank you
> > Yao Jiewen
> >
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Laszlo Ersek
> > Sent: Tuesday, October 17, 2017 11:15 PM
> > To: Yao, Jiewen mailto:jiewen@intel.com>>;
> > Duran, Leo mailto:leo.du...@amd.com>>
> > Cc: Ni, Ruiyu mailto:ruiyu...@intel.com>>; Justen,
> > Jordan L
> > mailto:jordan.l.jus...@intel.com>>;
> > edk2-devel@lists.01.org; Gao, Liming
> > mailto:liming@intel.com>>; Kinney, Michael D
> > mailto:michael.d.kin...@intel.com>>;
> Paolo
> > Bonzini mailto:pbonz...@redhat.com>>
> > Subject: Re: [edk2] [PATCH v5 1/2] UefiCpuPkg/SmmCpuFeaturesLib: Use
> > global variables to replace macros
> >
> > On 10/17/17 16:50, Yao, Jiewen wrote:
> >> I think it is unnecessary. All intel CPU is using same offset. All amd CPU 
> >> is
> using same offset. It can be identified easily by code.
> >> Adding a new API now is also an incompatible change because that
> requires all existing featurelib change to add a new API. We have lots of 
> close
> source platform using its own featurelib.
> >>
> >> May I know what problem we are trying to resolve by adding a new API?
> >
> > QEMU (and hence OVMF) uses the AMD save state map even when it
> > emulates Intel CPUs.
> >
> > The reason is that the Intel SDM does not specify the Intel save state
> > map in sufficient detail (for emulation by QEMU), while the AMD spec
> > does. So, as an emulation target, only the AMD one is implementable in
> > QEMU (and supportable in OVMF), regardless of whether QEMU reports
> the
> > virtual CPU manufacturer as Intel vs. AMD.
> >
> > If PiSmmCpuDxeSmm used a CPUID check, saw "Intel" as manufacturer,
> and
> > then used the Intel definition of the save state map, then
> > PiSmmCpuDxeSmm would break on QEMU / as part of OVMF.
> >
> > Thanks,
> > Laszlo
> >
> >>
> >>
> >> thank you!
> >> Yao, Jiewen
> >>
> >>
> >>> 在 2017年10月17日，下午10:20，Duran, Leo
> >>> mailto:leo.du...@amd.com>> 写道：
> >>>
> >>> Yao, Lazlo, et al,
> >>>
> >>> For the SRAM_SAVE_STATE_MAP_OFFSET:
> >>> I propose returning the value by a function in SmmCpuFeaturesLib...
> Here's the rationale:
> >>> - The value is fixed per CPU architecture, so this qualifies as a CPU
> feature.
> >>> - The logic for CPU architecture detection would be in one place
> >>> (e.g., PiSmmCpuDxeSmm would call the library function)
> >>> - The OVMF and Quark instances of the library could just return the
> current (default) value, so no compatibility issues.
> >>>
> >>> if you agree with this is an acceptable solution, I will submit a revised
> patch-set to address just the SRAM_SAVE_STATE_MAP_OFFSET.
> >>> (BTW, I'm re-evaluating the changes submitted for the PSD offset,
> >>> with the goal of just using the default value and dropping those
> >>> changes)
> >>>
> >>> Thanks,
> >>> Leo.
> >>>
>  -Original Message-
>  From: Duran, Leo
>  Sent: Wednesday, October 11, 2017 2:46 PM
>  To: edk2-devel@lists.01.org
>  Cc: Duran, Leo mailto:leo.du...@amd.com>>;
>  Jiewen Yao mailto:jiewen@intel.com>>;
>  Ruiyu Ni mailto:ruiyu...@intel.com>>; Michael D
>  Kinney
>  mailto:michael.d.kin...@intel.com>>;
>  Jordan Justen
>

Re: [edk2] [PATCH] ShellPkg/alias: Fix flag parsing logic

2017-10-19 Thread Carsey, Jaben

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Ni, Ruiyu
> Sent: Thursday, October 19, 2017 12:44 AM
> To: edk2-devel@lists.01.org
> Cc: Li, Huajing ; Carsey, Jaben
> 
> Subject: [PATCH] ShellPkg/alias: Fix flag parsing logic
> Importance: High
> 
> From: Huajing Li 
> 
> Existing logic to parse the flags isn't complete and cannot detect
> some invalid combinations of flags.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni 
> Cc: Jaben Carsey 
> ---
>  .../Library/UefiShellLevel3CommandsLib/Alias.c | 210 ++---
> 
>  1 file changed, 145 insertions(+), 65 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellLevel3CommandsLib/Alias.c
> b/ShellPkg/Library/UefiShellLevel3CommandsLib/Alias.c
> index daf46a9f65..3e00eb1d55 100644
> --- a/ShellPkg/Library/UefiShellLevel3CommandsLib/Alias.c
> +++ b/ShellPkg/Library/UefiShellLevel3CommandsLib/Alias.c
> @@ -18,6 +18,37 @@
>  #include 
> 
>  /**
> +  Print out single alias registered with the Shell.
> +
> +  @param[in]  Alias Points to the NULL-terminated shell alias.
> +If this parameter is NULL, then all
> +aliases will be returned in ReturnedData.
> +  @retval SHELL_SUCCESS the printout was sucessful
> +**/
> +SHELL_STATUS
> +PrintSingleShellAlias(
> +  IN  CONST CHAR16 *Alias
> +  )
> +{
> +  CONST CHAR16*ConstAliasVal;
> +  SHELL_STATUSShellStatus;
> +  BOOLEAN Volatile;
> +
> +  ShellStatus = SHELL_SUCCESS;
> +  ConstAliasVal = gEfiShellProtocol->GetAlias (Alias, &Volatile);
> +  if (ConstAliasVal == NULL) {
> +ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV),
> gShellLevel3HiiHandle, L"alias", Alias);
> +ShellStatus = SHELL_INVALID_PARAMETER;
> +  } else {
> +if (ShellCommandIsOnAliasList (Alias)) {
> +  Volatile = FALSE;
> +}
> +ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_ALIAS_OUTPUT),
> gShellLevel3HiiHandle, !Volatile ? L' ' : L'*', Alias, ConstAliasVal);
> +  }
> +  return ShellStatus;
> +}
> +
> +/**
>Print out each alias registered with the Shell.
> 
>@retval STATUS_SUCCESS  the printout was sucessful
> @@ -30,11 +61,7 @@ PrintAllShellAlias(
>  {
>CONST CHAR16  *ConstAllAliasList;
>CHAR16*Alias;
> -  CONST CHAR16  *Command;
>CHAR16*Walker;
> -  BOOLEAN   Volatile;
> -
> -  Volatile = FALSE;
> 
>ConstAllAliasList = gEfiShellProtocol->GetAlias(NULL, NULL);
>if (ConstAllAliasList == NULL) {
> @@ -53,11 +80,7 @@ PrintAllShellAlias(
>Walker[0] = CHAR_NULL;
>Walker = Walker + 1;
>  }
> -Command = gEfiShellProtocol->GetAlias(Alias, &Volatile);
> -if (ShellCommandIsOnAliasList(Alias)) {
> -  Volatile = FALSE;
> -}
> -ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_ALIAS_OUTPUT),
> gShellLevel3HiiHandle, !Volatile?L' ':L'*', Alias, Command);
> +PrintSingleShellAlias(Alias);
>} while (Walker != NULL && Walker[0] != CHAR_NULL);
> 
>FreePool(Alias);
> @@ -65,9 +88,58 @@ PrintAllShellAlias(
>return (SHELL_SUCCESS);
>  }
> 
> +/**
> +  Changes a shell command alias.
> +
> +  This function creates an alias for a shell command or if Alias is NULL it 
> will
> delete an existing alias.
> +
> +
> +  @param[in] CommandPoints to the NULL-terminated shell
> command or existing alias.
> +  @param[in] Alias  Points to the NULL-terminated alias for the 
> shell
> command. If this is NULL, and
> +Command refers to an alias, that alias will 
> be deleted.
> +  @param[in] ReplaceIf TRUE and the alias already exists, then 
> the
> existing alias will be replaced. If
> +FALSE and the alias already exists, then the 
> existing alias is
> unchanged and
> +EFI_ACCESS_DENIED is returned.
> +  @param[in] Volatile   if TRUE the Alias being set will be stored 
> in a
> volatile fashion.  if FALSE the
> +Alias being set will be stored in a 
> non-volatile fashion.
> +
> +  @retval SHELL_SUCCESSAlias created or deleted successfully.
> +  @retval SHELL_NOT_FOUND   the Alias intended to be deleted was not
> found
> +  @retval SHELL_ACCESS_DENIED   The alias is a built-in alias or already
> existed and Replace was set to
> +FALSE.
> +  @retval SHELL_DEVICE_ERRORCommand is null or the empty string.
> +**/
> +SHELL_STATUS
> +ShellLevel3CommandsLibSetAlias(
> +  IN CONST CHAR16 *Command,
> +  IN CONST CHAR16 *Alias,
> +  IN BOOLEAN Replace,
> +  IN BOOLEAN Volatile
> +  )
> +{
> +  SHELL_STATUSShellStatus;
> +  EFI_STATUS  Status;
> +
> +  ShellStatus = SHELL_SUCCESS;
> +  Status = gEfiShellProtocol->SetAlias (Command, Alias, Replace, Volatile);
> +  if (EFI_ERROR(Status)) {
> +if (Status == EFI_ACCESS_DENIED

Re: [edk2] [PATCH] ShellPkg/editor: Fix system hang when console max column > 200

2017-10-19 Thread Carsey, Jaben

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Ni, Ruiyu
> Sent: Wednesday, October 18, 2017 11:15 PM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben 
> Subject: [PATCH] ShellPkg/editor: Fix system hang when console max column
> > 200
> Importance: High
> 
> EditorClearLine() assumes the console max column is less than 200.
> When the max column is bigger than 200, the code incorrectly
> modifies the content out side of Line buffer.
> It may cause system hang or reset.
> 
> The patch changes the function to print several times when
> the max column is bigger than 200.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni 
> Cc: Jaben Carsey 
> ---
>  .../UefiShellDebug1CommandsLib.c   | 31 
> +-
>  1 file changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.c
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.c
> index 8e2141bf43..d26d08f95c 100644
> ---
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.c
> +++
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.c
> @@ -185,6 +185,7 @@ EditorClearLine (
>IN UINTN LastRow
>)
>  {
> +  UINTN  Col;
>CHAR16 Line[200];
> 
>if (Row == 0) {
> @@ -193,22 +194,28 @@ EditorClearLine (
> 
>//
>// prepare a blank line
> +  // If max column is larger, split to multiple prints.
>//
> -  SetMem16(Line, LastCol*sizeof(CHAR16), L' ');
> -
> -  if (Row == LastRow) {
> +  SetMem16 (Line, sizeof (Line), L' ');
> +  Line[ARRAY_SIZE (Line) - 1] = CHAR_NULL;
> +
> +  for (Col = 1; Col <= LastCol; Col += ARRAY_SIZE (Line) - 1) {
> +if (Col + ARRAY_SIZE (Line) - 1 > LastCol) {
> +  if (Row == LastRow) {
> +//
> +// if CHAR_NULL is still at position LastCol, it will cause first 
> line error
> +//
> +Line[(LastCol % (ARRAY_SIZE (Line) - 1)) - 1] = CHAR_NULL;
> +  } else {
> +Line[LastCol % (ARRAY_SIZE (Line) - 1)] = CHAR_NULL;
> +  }
> +}
> +
>  //
> -// if CHAR_NULL is still at position 80, it will cause first line error
> +// print out the blank line
>  //
> -Line[LastCol - 1] = CHAR_NULL;
> -  } else {
> -Line[LastCol] = CHAR_NULL;
> +ShellPrintEx ((INT32) Col - 1, (INT32) Row - 1, Line);
>}
> -
> -  //
> -  // print out the blank line
> -  //
> -  ShellPrintEx (0, ((INT32)Row) - 1, Line);
>  }
> 
>  /**
> --
> 2.12.2.windows.2

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

Re: [edk2] [Patch] UefiCpuPkg/MpInitLib: Avoid call PcdGe* in Ap & Bsp.

2017-10-19 Thread Ni, Ruiyu

With the subject change suggested by Laszlo, Reviewed-by: Ruiyu Ni 


Thanks/Ray

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, October 19, 2017 4:08 PM
> To: Dong, Eric ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Crystal Lee 
> Subject: Re: [edk2] [Patch] UefiCpuPkg/MpInitLib: Avoid call PcdGe* in Ap &
> Bsp.
> 
> On 10/19/17 04:42, Eric Dong wrote:
> > MicrocodeDetect function will run by every threads, and it will use
> > PcdGet to get PcdCpuMicrocodePatchAddress and
> > PcdCpuMicrocodePatchRegionSize, if change both PCD default to dynamic,
> > system will in non-deterministic behavior.
> >
> > By design, UEFI/PI services are single threaded and not re-entrant so
> > Multi processor code should not use UEFI/PI services. Here, Pcd
> > protocol/PPI is used to access dynamic PCDs so it would result in
> > non-deterministic behavior.
> >
> > This code get PCD value in BSP and save them in CPU_MP_DATA for Ap.
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=726
> >
> > Cc: Crystal Lee 
> > Cc: Ruiyu Ni 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Dong 
> > ---
> >  UefiCpuPkg/Library/MpInitLib/Microcode.c | 10 +++---
> >  UefiCpuPkg/Library/MpInitLib/MpLib.c |  2 ++
> >  UefiCpuPkg/Library/MpInitLib/MpLib.h |  2 ++
> >  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> Eric, can you please fix up the subject line after you get an R-b and are 
> about
> to push the patch?
> 
> -UefiCpuPkg/MpInitLib: Avoid call PcdGe* in Ap & Bsp.
> +UefiCpuPkg/MpInitLib: Avoid call PcdGet* in Ap & Bsp.
> 
> Thanks!
> Laszlo
> 
> 
> > diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > index 982995b..35f66f7 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > @@ -42,8 +42,6 @@ MicrocodeDetect (
> >IN CPU_MP_DATA *CpuMpData
> >)
> >  {
> > -  UINT64  MicrocodePatchAddress;
> > -  UINT64  MicrocodePatchRegionSize;
> >UINT32  ExtendedTableLength;
> >UINT32  ExtendedTableCount;
> >CPU_MICROCODE_EXTENDED_TABLE*ExtendedTable;
> > @@ -61,9 +59,7 @@ MicrocodeDetect (
> >VOID*MicrocodeData;
> >MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
> >
> > -  MicrocodePatchAddress= PcdGet64 (PcdCpuMicrocodePatchAddress);
> > -  MicrocodePatchRegionSize = PcdGet64
> > (PcdCpuMicrocodePatchRegionSize);
> > -  if (MicrocodePatchRegionSize == 0) {
> > +  if (CpuMpData->MicrocodePatchRegionSize == 0) {
> >  //
> >  // There is no microcode patches
> >  //
> > @@ -93,8 +89,8 @@ MicrocodeDetect (
> >
> >LatestRevision = 0;
> >MicrocodeData  = NULL;
> > -  MicrocodeEnd = (UINTN) (MicrocodePatchAddress +
> > MicrocodePatchRegionSize);
> > -  MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN)
> > MicrocodePatchAddress;
> > +  MicrocodeEnd = (UINTN) (CpuMpData->MicrocodePatchAddress +
> > + CpuMpData->MicrocodePatchRegionSize);
> > +  MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN)
> > + CpuMpData->MicrocodePatchAddress;
> >do {
> >  //
> >  // Check if the microcode is for the Cpu and the version is newer
> > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > index 924b909..f3ee6d4 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > @@ -1458,6 +1458,8 @@ 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);
> >InitializeSpinLock(&CpuMpData->MpLock);
> >//
> >// Save BSP's Control registers to APs diff --git
> > a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > index 19defda..84ae24f 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > @@ -233,6 +233,8 @@ struct _CPU_MP_DATA {
> >UINT8  Vector;
> >BOOLEANPeriodicMode;
> >BOOLEANTimerInterruptState;
> > +  UINT64 MicrocodePatchAddress;
> > +  UINT64 MicrocodePatchRegionSize;
> >  };
> >
> >  extern EFI_GUID mCpuInitMpLibHobGuid;
> >

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

Re: [edk2] [PATCH] OvmfPkg: fix dynamic default for oprom verification policy PCD without SB

2017-10-19 Thread Laszlo Ersek

On 10/19/17 04:14, Jordan Justen wrote:
> On 2017-10-17 15:23:21, Laszlo Ersek wrote:
>> I missed the following, both while reviewing and while testing commit
>> 6041ac65ae87 ("OvmfPkg/PlatformPei: DENY_EXECUTE_ON_SECURITY_VIOLATION
>> when SEV is active", 2017-10-05):
>>
>> If "-D SECURE_BOOT_ENABLE" is not passed on the "build" command line, then
>> OVMF has no dynamic default at all for
>> "PcdOptionRomImageVerificationPolicy". This means that the PcdSet32S()
>> call added in the subject commit doesn't even compile:
>>
>>> OvmfPkg/PlatformPei/AmdSev.c: In function 'AmdSevInitialize':
>>> OvmfPkg/PlatformPei/AmdSev.c:67:3: error: implicit declaration of
>>> function '_PCD_SET_MODE_32_S_PcdOptionRomImageVerificationPolicy'
>>> [-Werror=implicit-function-declaration]
>>>PcdStatus = PcdSet32S (PcdOptionRomImageVerificationPolicy, 0x4);
>>>^
>>> cc1: all warnings being treated as errors
>>
>> There are three ways to fix the error:
>>
>> (1) Make the current, SB-only, 0x00 dynamic default unconditional.
> 
> Maybe you should say something like, "As implemented in this patch..."
> for item (1). Or, just drop (2) and (3) entirely in the commit
> message. I'll leave that up to you.

Thank you, Jordan; I dropped (2) and (3). Also added a last-minute
reference to TianoCore BZ#737, in which the same issue has now been
reported, independently.

> 
> Reviewed-by: Jordan Justen 

Commit 1958124a6cb0.

Thank you!
Laszlo

> 
>>
>> This is the simplest approach, and it reflects the intent of original
>> commit 1fea9ddb4e3f ("OvmfPkg: execute option ROM images regardless of
>> Secure Boot", 2016-01-07). Without SECURE_BOOT_ENABLE,
>> "SecurityPkg/Library/DxeImageVerificationLib" is not used anyway, so
>> the PCD is never read.
>>
>> (2) Add an !else branch that explicitly sets the SecurityPkg.dec default
>> (0x04) as dynamic default, if SECURE_BOOT_ENABLE is FALSE.
>>
>> This looks awkward because it explicitly sets a dynamic default that
>> is then never read.
>>
>> (3) Set the SecurityPkg.dec default (0x04) as unconditional dynamic
>> default, and invert the logic in AmdSevInitialize()
>> [OvmfPkg/PlatformPei/AmdSev.c] -- set the PCD to 0x00 if SEV is
>> disabled; don't touch it otherwise.
>>
>> I think this sends the wrong message -- for the time being anyway, SEV
>> is the exception, not the rule. We shouldn't rely on the PCD getting
>> its most commonly used value in a function called AmdSevInitialize().
>>
>> This issue was caught and reported by Gerd Hoffmann 's
>> Jenkins CI.
>>
>> Cc: Ard Biesheuvel 
>> Cc: Brijesh Singh 
>> Cc: Jordan Justen 
>> Fixes: 6041ac65ae879389f3ab5c0699f916d3e71c97fe
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek 
>> ---
>>
>> Notes:
>> Repo:   https://github.com/lersek/edk2.git
>> Branch: oprom_policy_build_fix
>>
>>  OvmfPkg/OvmfPkgIa32.dsc| 3 ---
>>  OvmfPkg/OvmfPkgIa32X64.dsc | 3 ---
>>  OvmfPkg/OvmfPkgX64.dsc | 3 ---
>>  3 files changed, 9 deletions(-)
>>
>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>> index 7fb557b7c9cd..c2f534fdbf3b 100644
>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>> @@ -540,10 +540,7 @@ [PcdsDynamicDefault]
>>gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|10
>>  !endif
>>  
>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00
>> -!endif
>> -
>>  
>>  
>> 
>>  #
>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>> index 4bcbddb95768..9f300a2e6f32 100644
>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>> @@ -548,10 +548,7 @@ [PcdsDynamicDefault]
>>gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|10
>>  !endif
>>  
>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00
>> -!endif
>> -
>>  
>>  
>> 
>>  #
>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>> index e52a3bd4db9b..1ffcf37f8b92 100644
>> --- a/OvmfPkg/OvmfPkgX64.dsc
>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>> @@ -547,10 +547,7 @@ [PcdsDynamicDefault]
>>gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|10
>>  !endif
>>  
>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>>gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00
>> -!endif
>> -
>>  
>>  
>> 
>>  #
>> -- 
>> 2.14.1.3.gb7cf6e02401b
>>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

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

Re: [edk2] [Patch] UefiCpuPkg/MpInitLib: Avoid call PcdGe* in Ap & Bsp.

2017-10-19 Thread Laszlo Ersek

On 10/19/17 04:42, Eric Dong wrote:
> MicrocodeDetect function will run by every threads, and it will
> use PcdGet to get PcdCpuMicrocodePatchAddress and
> PcdCpuMicrocodePatchRegionSize, if change both PCD default to dynamic,
> system will in non-deterministic behavior.
> 
> By design, UEFI/PI services are single threaded and not re-entrant
> so Multi processor code should not use UEFI/PI services. Here, Pcd
> protocol/PPI is used to access dynamic PCDs so it would result in
> non-deterministic behavior.
> 
> This code get PCD value in BSP and save them in CPU_MP_DATA for Ap.
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=726
> 
> Cc: Crystal Lee 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 10 +++---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c |  2 ++
>  UefiCpuPkg/Library/MpInitLib/MpLib.h |  2 ++
>  3 files changed, 7 insertions(+), 7 deletions(-)

Eric, can you please fix up the subject line after you get an R-b and
are about to push the patch?

-UefiCpuPkg/MpInitLib: Avoid call PcdGe* in Ap & Bsp.
+UefiCpuPkg/MpInitLib: Avoid call PcdGet* in Ap & Bsp.

Thanks!
Laszlo


> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c 
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index 982995b..35f66f7 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -42,8 +42,6 @@ MicrocodeDetect (
>IN CPU_MP_DATA *CpuMpData
>)
>  {
> -  UINT64  MicrocodePatchAddress;
> -  UINT64  MicrocodePatchRegionSize;
>UINT32  ExtendedTableLength;
>UINT32  ExtendedTableCount;
>CPU_MICROCODE_EXTENDED_TABLE*ExtendedTable;
> @@ -61,9 +59,7 @@ MicrocodeDetect (
>VOID*MicrocodeData;
>MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
>  
> -  MicrocodePatchAddress= PcdGet64 (PcdCpuMicrocodePatchAddress);
> -  MicrocodePatchRegionSize = PcdGet64 (PcdCpuMicrocodePatchRegionSize);
> -  if (MicrocodePatchRegionSize == 0) {
> +  if (CpuMpData->MicrocodePatchRegionSize == 0) {
>  //
>  // There is no microcode patches
>  //
> @@ -93,8 +89,8 @@ MicrocodeDetect (
>  
>LatestRevision = 0;
>MicrocodeData  = NULL;
> -  MicrocodeEnd = (UINTN) (MicrocodePatchAddress + MicrocodePatchRegionSize);
> -  MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN) 
> MicrocodePatchAddress;
> +  MicrocodeEnd = (UINTN) (CpuMpData->MicrocodePatchAddress + 
> CpuMpData->MicrocodePatchRegionSize);
> +  MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN) 
> CpuMpData->MicrocodePatchAddress;
>do {
>  //
>  // Check if the microcode is for the Cpu and the version is newer
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 924b909..f3ee6d4 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -1458,6 +1458,8 @@ 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);
>InitializeSpinLock(&CpuMpData->MpLock);
>//
>// Save BSP's Control registers to APs
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 19defda..84ae24f 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -233,6 +233,8 @@ struct _CPU_MP_DATA {
>UINT8  Vector;
>BOOLEANPeriodicMode;
>BOOLEANTimerInterruptState;
> +  UINT64 MicrocodePatchAddress;
> +  UINT64 MicrocodePatchRegionSize;
>  };
>  
>  extern EFI_GUID mCpuInitMpLibHobGuid;
> 

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

Re: [edk2] [Patch] NetworkPkg/TlsAuthConfigDxe: Remove the extra FreePool

2017-10-19 Thread Long, Qin

Reviewed-by: Long Qin 


Best Regards & Thanks,
LONG, Qin

-Original Message-
From: Wu, Jiaxin 
Sent: Thursday, October 19, 2017 1:58 PM
To: edk2-devel@lists.01.org
Cc: Long, Qin ; Ye, Ting ; Fu, Siyuan 
; Wu, Jiaxin 
Subject: [Patch] NetworkPkg/TlsAuthConfigDxe: Remove the extra FreePool

Cc: Long Qin 
Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
 NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.c 
b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.c
index 351656f..403afbb 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.c
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.c
@@ -1,9 +1,9 @@
 /** @file
   The DriverEntryPoint for TlsAuthConfigDxe driver.
 
-  Copyright (c) 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
   http://opensource.org/licenses/bsd-license.php.
@@ -126,10 +126,9 @@ TlsAuthConfigDxeDriverEntryPoint (
 
   return EFI_SUCCESS;
 
 ON_ERROR:
   TlsAuthConfigFormUnload (PrivateData);
-  FreePool (PrivateData);
-
+  
   return Status;
 }
 
-- 
1.9.5.msysgit.1

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

[edk2] [PATCH] ShellPkg/alias: Fix flag parsing logic

2017-10-19 Thread Ruiyu Ni

From: Huajing Li 

Existing logic to parse the flags isn't complete and cannot detect
some invalid combinations of flags.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Jaben Carsey 
---
 .../Library/UefiShellLevel3CommandsLib/Alias.c | 210 ++---
 1 file changed, 145 insertions(+), 65 deletions(-)

diff --git a/ShellPkg/Library/UefiShellLevel3CommandsLib/Alias.c 
b/ShellPkg/Library/UefiShellLevel3CommandsLib/Alias.c
index daf46a9f65..3e00eb1d55 100644
--- a/ShellPkg/Library/UefiShellLevel3CommandsLib/Alias.c
+++ b/ShellPkg/Library/UefiShellLevel3CommandsLib/Alias.c
@@ -18,6 +18,37 @@
 #include 
 
 /**
+  Print out single alias registered with the Shell.
+
+  @param[in]  Alias Points to the NULL-terminated shell alias.
+If this parameter is NULL, then all
+aliases will be returned in ReturnedData.
+  @retval SHELL_SUCCESS the printout was sucessful
+**/
+SHELL_STATUS
+PrintSingleShellAlias(
+  IN  CONST CHAR16 *Alias
+  )
+{
+  CONST CHAR16*ConstAliasVal;
+  SHELL_STATUSShellStatus;
+  BOOLEAN Volatile;
+
+  ShellStatus = SHELL_SUCCESS;
+  ConstAliasVal = gEfiShellProtocol->GetAlias (Alias, &Volatile);
+  if (ConstAliasVal == NULL) {
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), 
gShellLevel3HiiHandle, L"alias", Alias);
+ShellStatus = SHELL_INVALID_PARAMETER;
+  } else {
+if (ShellCommandIsOnAliasList (Alias)) {
+  Volatile = FALSE;
+}
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_ALIAS_OUTPUT), 
gShellLevel3HiiHandle, !Volatile ? L' ' : L'*', Alias, ConstAliasVal);
+  }
+  return ShellStatus;
+}
+
+/**
   Print out each alias registered with the Shell.
 
   @retval STATUS_SUCCESS  the printout was sucessful
@@ -30,11 +61,7 @@ PrintAllShellAlias(
 {
   CONST CHAR16  *ConstAllAliasList;
   CHAR16*Alias;
-  CONST CHAR16  *Command;
   CHAR16*Walker;
-  BOOLEAN   Volatile;
-
-  Volatile = FALSE;
 
   ConstAllAliasList = gEfiShellProtocol->GetAlias(NULL, NULL);
   if (ConstAllAliasList == NULL) {
@@ -53,11 +80,7 @@ PrintAllShellAlias(
   Walker[0] = CHAR_NULL;
   Walker = Walker + 1;
 }
-Command = gEfiShellProtocol->GetAlias(Alias, &Volatile);
-if (ShellCommandIsOnAliasList(Alias)) {
-  Volatile = FALSE;
-}
-ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_ALIAS_OUTPUT), 
gShellLevel3HiiHandle, !Volatile?L' ':L'*', Alias, Command);
+PrintSingleShellAlias(Alias);
   } while (Walker != NULL && Walker[0] != CHAR_NULL);
 
   FreePool(Alias);
@@ -65,9 +88,58 @@ PrintAllShellAlias(
   return (SHELL_SUCCESS);
 }
 
+/**
+  Changes a shell command alias.
+
+  This function creates an alias for a shell command or if Alias is NULL it 
will delete an existing alias.
+
+
+  @param[in] CommandPoints to the NULL-terminated shell command or 
existing alias.
+  @param[in] Alias  Points to the NULL-terminated alias for the 
shell command. If this is NULL, and
+Command refers to an alias, that alias will be 
deleted.
+  @param[in] ReplaceIf TRUE and the alias already exists, then the 
existing alias will be replaced. If
+FALSE and the alias already exists, then the 
existing alias is unchanged and
+EFI_ACCESS_DENIED is returned.
+  @param[in] Volatile   if TRUE the Alias being set will be stored in 
a volatile fashion.  if FALSE the
+Alias being set will be stored in a 
non-volatile fashion.
+
+  @retval SHELL_SUCCESSAlias created or deleted successfully.
+  @retval SHELL_NOT_FOUND   the Alias intended to be deleted was not found
+  @retval SHELL_ACCESS_DENIED   The alias is a built-in alias or already 
existed and Replace was set to
+FALSE.
+  @retval SHELL_DEVICE_ERRORCommand is null or the empty string.
+**/
+SHELL_STATUS
+ShellLevel3CommandsLibSetAlias(
+  IN CONST CHAR16 *Command,
+  IN CONST CHAR16 *Alias,
+  IN BOOLEAN Replace,
+  IN BOOLEAN Volatile
+  )
+{
+  SHELL_STATUSShellStatus;
+  EFI_STATUS  Status;
+
+  ShellStatus = SHELL_SUCCESS;
+  Status = gEfiShellProtocol->SetAlias (Command, Alias, Replace, Volatile);
+  if (EFI_ERROR(Status)) {
+if (Status == EFI_ACCESS_DENIED) {
+  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_ERR_AD), 
gShellLevel3HiiHandle, L"alias");
+  ShellStatus = SHELL_ACCESS_DENIED;
+} else if (Status == EFI_NOT_FOUND) {
+  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_ERR_NOT_FOUND), 
gShellLevel3HiiHandle, L"alias", Command);
+  ShellStatus = SHELL_NOT_FOUND;
+} else {
+  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_ERR_UK), 
gShellLevel3HiiHandle, L"alias", Status);
+  ShellStatus = SHELL_DEVICE_ERROR;
+}
+  }
+  r

Re: [edk2] [Patch] NetworkPkg/TlsAuthConfigDxe: Remove the extra FreePool

2017-10-19 Thread Ye, Ting

Reviewed-by: Ye Ting  

-Original Message-
From: Wu, Jiaxin 
Sent: Thursday, October 19, 2017 1:58 PM
To: edk2-devel@lists.01.org
Cc: Long, Qin; Ye, Ting; Fu, Siyuan; Wu, Jiaxin
Subject: [Patch] NetworkPkg/TlsAuthConfigDxe: Remove the extra FreePool

Cc: Long Qin 
Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
 NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.c 
b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.c
index 351656f..403afbb 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.c
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.c
@@ -1,9 +1,9 @@
 /** @file
   The DriverEntryPoint for TlsAuthConfigDxe driver.
 
-  Copyright (c) 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
   http://opensource.org/licenses/bsd-license.php.
@@ -126,10 +126,9 @@ TlsAuthConfigDxeDriverEntryPoint (
 
   return EFI_SUCCESS;
 
 ON_ERROR:
   TlsAuthConfigFormUnload (PrivateData);
-  FreePool (PrivateData);
-
+  
   return Status;
 }
 
-- 
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 v5 1/2] UefiCpuPkg/SmmCpuFeaturesLib: Use global variables to replace macros

2017-10-19 Thread Laszlo Ersek

On 10/18/17 16:36, Duran, Leo wrote:
> Yao, Paolo, Lazlo, et al,
> 
> The offset disparity has proved manageable within SmmCpuFeaturesLib, so I’m 
> inclined to leave macro definitions as-is and just drop this patch-set.
> 
> Agreed?

Sure.

Out of curiosity, what does it mean for you that the offset disparity is
manageable in SmmCpuFeaturesLib? Will you send a different patch (set)
for UefiCpuPkg/SmmCpuFeaturesLib, or does it concern an
SmmCpuFeaturesLib instance that is private to AMD?

Thanks!
Laszlo


> From: Yao, Jiewen [mailto:jiewen@intel.com]
> Sent: Tuesday, October 17, 2017 8:51 PM
> To: Laszlo Ersek ; Duran, Leo 
> Cc: Ni, Ruiyu ; Justen, Jordan L 
> ; edk2-devel@lists.01.org; Gao, Liming 
> ; Kinney, Michael D ; Paolo 
> Bonzini 
> Subject: RE: [edk2] [PATCH v5 1/2] UefiCpuPkg/SmmCpuFeaturesLib: Use global 
> variables to replace macros
> 
> Thanks. Then I think Paolo’s suggestion makes more sense.
> 
> If we can use same off, that would be great.
> If no, just check AMD version ID in SaveState.
> 
> Thank you
> Yao Jiewen
> 
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
> Ersek
> Sent: Tuesday, October 17, 2017 11:15 PM
> To: Yao, Jiewen mailto:jiewen@intel.com>>; Duran, 
> Leo mailto:leo.du...@amd.com>>
> Cc: Ni, Ruiyu mailto:ruiyu...@intel.com>>; Justen, Jordan 
> L mailto:jordan.l.jus...@intel.com>>; 
> edk2-devel@lists.01.org; Gao, Liming 
> mailto:liming@intel.com>>; Kinney, Michael D 
> mailto:michael.d.kin...@intel.com>>; Paolo 
> Bonzini mailto:pbonz...@redhat.com>>
> Subject: Re: [edk2] [PATCH v5 1/2] UefiCpuPkg/SmmCpuFeaturesLib: Use global 
> variables to replace macros
> 
> On 10/17/17 16:50, Yao, Jiewen wrote:
>> I think it is unnecessary. All intel CPU is using same offset. All amd CPU 
>> is using same offset. It can be identified easily by code.
>> Adding a new API now is also an incompatible change because that requires 
>> all existing featurelib change to add a new API. We have lots of close 
>> source platform using its own featurelib.
>>
>> May I know what problem we are trying to resolve by adding a new API?
> 
> QEMU (and hence OVMF) uses the AMD save state map even when it emulates
> Intel CPUs.
> 
> The reason is that the Intel SDM does not specify the Intel save state
> map in sufficient detail (for emulation by QEMU), while the AMD spec
> does. So, as an emulation target, only the AMD one is implementable in
> QEMU (and supportable in OVMF), regardless of whether QEMU reports the
> virtual CPU manufacturer as Intel vs. AMD.
> 
> If PiSmmCpuDxeSmm used a CPUID check, saw "Intel" as manufacturer, and
> then used the Intel definition of the save state map, then
> PiSmmCpuDxeSmm would break on QEMU / as part of OVMF.
> 
> Thanks,
> Laszlo
> 
>>
>>
>> thank you!
>> Yao, Jiewen
>>
>>
>>> 在 2017年10月17日，下午10:20，Duran, Leo 
>>> mailto:leo.du...@amd.com>> 写道：
>>>
>>> Yao, Lazlo, et al,
>>>
>>> For the SRAM_SAVE_STATE_MAP_OFFSET:
>>> I propose returning the value by a function in SmmCpuFeaturesLib... Here's 
>>> the rationale:
>>> - The value is fixed per CPU architecture, so this qualifies as a CPU 
>>> feature.
>>> - The logic for CPU architecture detection would be in one place (e.g., 
>>> PiSmmCpuDxeSmm would call the library function)
>>> - The OVMF and Quark instances of the library could just return the current 
>>> (default) value, so no compatibility issues.
>>>
>>> if you agree with this is an acceptable solution, I will submit a revised 
>>> patch-set to address just the SRAM_SAVE_STATE_MAP_OFFSET.
>>> (BTW, I'm re-evaluating the changes submitted for the PSD offset, with the 
>>> goal of just using the default value and dropping those changes)
>>>
>>> Thanks,
>>> Leo.
>>>
 -Original Message-
 From: Duran, Leo
 Sent: Wednesday, October 11, 2017 2:46 PM
 To: edk2-devel@lists.01.org
 Cc: Duran, Leo mailto:leo.du...@amd.com>>; Jiewen Yao
 mailto:jiewen@intel.com>>; Ruiyu Ni 
 mailto:ruiyu...@intel.com>>; Michael D Kinney
 mailto:michael.d.kin...@intel.com>>; Jordan 
 Justen mailto:jordan.l.jus...@intel.com>>;
 Liming Gao mailto:liming@intel.com>>
 Subject: [PATCH v5 1/2] UefiCpuPkg/SmmCpuFeaturesLib: Use global
 variables to replace macros

 Set global variables on Constructor function based on CPUID checks.
 The variables replace Intel macros to allow support on AMD x86 systems.

 Specifically, the replaced macros are:
 1) SRAM_SAVE_STATE_MAP_OFFSET
 2) TXT_SMM_PSD_OFFSET

 Cc: Jiewen Yao mailto:jiewen@intel.com>>
 Cc: Ruiyu Ni mailto:ruiyu...@intel.com>>
 Cc: Michael D Kinney 
 mailto:michael.d.kin...@intel.com>>
 Cc: Jordan Justen 
 mailto:jordan.l.jus...@intel.com>>
 Cc: Liming Gao mailto:liming@intel.com>>
 Contributed-under: TianoCore Contribution Agreement 1.1
 Signed-off-by: Leo Duran mailto:leo.du...@amd.com>>

[edk2] [Patch] MdeModulePkg/Ip4Dxe: Trigger Ip4Config2 to retrieve the default address.

Re: [edk2] [Patch] UefiCpuPkg/MpInitLib: Avoid call PcdGe* in Ap & Bsp.

Re: [edk2] [patch 4/4] UefiCpuPkg/MtrrLib: Make comments align with function

Re: [edk2] [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM mode

Re: [edk2] [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM mode

[edk2] [patch 3/4] MdeModulePkg/DxeIplPeim: Refine coding style in function comments

[edk2] [patch 4/4] UefiCpuPkg/MtrrLib: Make comments align with function

[edk2] [patch 2/4] IntelFrameworkModule/LegacyBios: Avoid explicit comparison for BOOLEAN

[edk2] [patch 1/4] IntelFrameworkModulePkg/Csm: Refine coding style in comments

[edk2] [patch 0/4] Fix coding style issue

Re: [edk2] [PATCH v2 2/6] MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM mode

[edk2] dynamic PCD impact on temporary PEI memory

Re: [edk2] [PATCH] EmbeddedPkg/DtPlatformDxe: remove /chosen/stdout-path on GOP registration

Re: [edk2] [PATCH] EmbeddedPkg/DtPlatformDxe: remove /chosen/stdout-path on GOP registration

Re: [edk2] [PATCH] EmbeddedPkg/DtPlatformDxe: remove /chosen/stdout-path on GOP registration

[edk2] [PATCH] EmbeddedPkg/DtPlatformDxe: remove /chosen/stdout-path on GOP registration

Re: [edk2] [PATCH v5 1/2] UefiCpuPkg/SmmCpuFeaturesLib: Use global variables to replace macros

Re: [edk2] [PATCH] ShellPkg/alias: Fix flag parsing logic

Re: [edk2] [PATCH] ShellPkg/editor: Fix system hang when console max column > 200

Re: [edk2] [Patch] UefiCpuPkg/MpInitLib: Avoid call PcdGe* in Ap & Bsp.

Re: [edk2] [PATCH] OvmfPkg: fix dynamic default for oprom verification policy PCD without SB

Re: [edk2] [Patch] UefiCpuPkg/MpInitLib: Avoid call PcdGe* in Ap & Bsp.

Re: [edk2] [Patch] NetworkPkg/TlsAuthConfigDxe: Remove the extra FreePool

[edk2] [PATCH] ShellPkg/alias: Fix flag parsing logic

Re: [edk2] [Patch] NetworkPkg/TlsAuthConfigDxe: Remove the extra FreePool

Re: [edk2] [PATCH v5 1/2] UefiCpuPkg/SmmCpuFeaturesLib: Use global variables to replace macros

26 matches

Site Navigation

Mail list logo

Footer information