Re: [edk2] Dynamic Pci configuration devices

2018-05-08 Thread Guy Raviv
Thanks for the detailed answer.

if i choose the 2nd approach would it be possible to modify the structrue
itself of the pci tree,
and not only a specific device number?
example: if i add a pci bridge between my board and a graphic card.

Thanks,

On Tue, May 8, 2018 at 5:47 PM, Laszlo Ersek  wrote:

> On 05/08/18 16:16, Guy Raviv wrote:
> > Hi all,
> >
> > currently in
> >
> > \Vlv2DeviceRefCodePkg\AcpiTablesPCAT\HOST_BUS.ASL
> >
> > The PCI devices are declared statically.
> >
> > i want to make this declaration dynamic - so that the device number can
> change
> >
> > according according to my hardware setup. Is it possible?
>
> There are generally two ways for this.
>
> One is to write the bulk of the ASL like seen here, statically, but all
> the customizable values are referenced as external objects / fields.
> Then, a platform ACPI DXE driver in the firmware computes those values,
> and installs a small SSDT with just those objects. The AML is generated
> manually by the firmware, which is super awkward, but due to the small
> size of the integer objects etc, it is tolerable.
>
> A similar approach can be seen e.g. in "OvmfPkg/AcpiPlatformDxe/Qemu.c",
> function QemuInstallAcpiSsdtTable(). And the referring ASL code is in
> "OvmfPkg/AcpiTables/Dsdt.asl". (Search both for "FWDT".)
>
> (Note however that said function is not used nowadays on QEMU, because
> now QEMU generates *all* of the AML dynamically.)
>
> The other approach is to process the (static) AML before installing it
> with EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable(). If you know the exact
> path to / structure of the AML node that you want to modify, the
> EFI_ACPI_SDT_PROTOCOL lets you navigate to the node, and patch it
> in-place, in a memory array. Then you can install the modified table
> blob with EFI_ACPI_TABLE_PROTOCOL. (Important: do not modify a table
> *after* it is installed.)
>
> One example for the 2nd approach should be
> "QuarkPlatformPkg/Acpi/Dxe/AcpiPlatform/AcpiPciUpdate.c".
>
> Thanks
> Laszlo
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [patch] BaseTools/VfrCompile: Avoid using uninitialized pointer

2018-05-08 Thread Bi, Dandan
Thanks for your test work. 
I have created a V2 patch which make the code logic more clean. Would you mind 
to try the V2 patch in your environment?


Thanks,
Dandan

-Original Message-
From: Gary Lin [mailto:g...@suse.com] 
Sent: Wednesday, May 9, 2018 10:32 AM
To: Bi, Dandan 
Cc: edk2-devel@lists.01.org; Dong, Eric ; Gao, Liming 

Subject: Re: [edk2] [patch] BaseTools/VfrCompile: Avoid using uninitialized 
pointer

On Tue, May 08, 2018 at 07:46:19PM +0800, Dandan Bi wrote:
> _CLEAR_SAVED_OPHDR () is used for initialize the variables.
> We should not update it to free memory.
> It will cause some pointer used before initialization.
> This patch is to fix this issue.
> 
> Cc: Eric Dong 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi 
> ---
>  BaseTools/Source/C/VfrCompile/VfrSyntax.g | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g 
> b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> index 4b0a43606ea..cc042ab4307 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> +++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> @@ -4103,12 +4103,15 @@ vfrStatementExpression [UINT32 RootLevel, UINT32 
> ExpOpCount = 0] :
>}
>  }
>}
>
>if ($RootLevel == 
> 0) {
> -
> _CLEAR_SAVED_OPHDR ();
> -mCIfrOpHdrIndex 
> --;
> +if 
> (mCIfrOpHdr[mCIfrOpHdrIndex] != NULL) {
> +  delete 
> mCIfrOpHdr[mCIfrOpHdrIndex];
> +  
> mCIfrOpHdr[mCIfrOpHdrIndex] = NULL;
> +}

> + 
> + mCIfrOpHdrIndex --;
An extra space was added.

>}
> >>
>;
>  
>  //
> @@ -5082,14 +5085,11 @@ EfiVfrParser::_SAVE_OPHDR_COND (  VOID  
> EfiVfrParser::_CLEAR_SAVED_OPHDR (
>VOID
>)
>  {
> -  if (mCIfrOpHdr[mCIfrOpHdrIndex] != NULL) {
> -delete mCIfrOpHdr[mCIfrOpHdrIndex];
> -mCIfrOpHdr[mCIfrOpHdrIndex] = NULL;
> -  }
> +  mCIfrOpHdr[mCIfrOpHdrIndex]   = NULL;
>mCIfrOpHdrLineNo[mCIfrOpHdrIndex] = 0;  }
>  
>  BOOLEAN
>  EfiVfrParser::_SET_SAVED_OPHDR_SCOPE (
> --
I applied the patch and triggered the rebuild of ovmf. It's now built on all 
versions and arch.

Thanks for fixing it.

Tested-by: Gary Lin 

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


[edk2] [patch v2] BaseTools/VfrCompile: Avoid using uninitialized pointer

2018-05-08 Thread Dandan Bi
V2:
Add function _INIT_OPHDR_COND () for variable initialization.
Make code logic more clean.

Previously _CLEAR_SAVED_OPHDR () is used for variable
initialization, and we updated it to clean memory.
But _CLEAR_SAVED_OPHDR () is still called for variable
initialization. This will cause uninitialized pointer
will be checked to free and cause unexpected issue.

This patch is to add new function for variable initialization
and keep _CLEAR_SAVED_OPHDR () to clean memory which is
aligned with its function name.

Cc: Eric Dong 
Cc: Liming Gao 
Cc: Gary Lin 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 BaseTools/Source/C/VfrCompile/VfrSyntax.g | 23 ---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g 
b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
index 4b0a43606ea..84dd2c3ed3f 100644
--- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
+++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
@@ -4084,11 +4084,19 @@ vfrStatementInvalidSaveRestoreDefaults :
 
 //
 // Root expression extension function called by other function.
 //
 vfrStatementExpression [UINT32 RootLevel, UINT32 ExpOpCount = 0] :
-  << if ($RootLevel == 0) {mCIfrOpHdrIndex ++; if (mCIfrOpHdrIndex >= 
MAX_IFR_EXPRESSION_DEPTH) _PCATCH (VFR_RETURN_INVALID_PARAMETER, 0, "The depth 
of expression exceeds the max supported level 8!"); _CLEAR_SAVED_OPHDR ();} >>
+   <<
+  if ($RootLevel == 0) 
{
+mCIfrOpHdrIndex ++;
+if 
(mCIfrOpHdrIndex >= MAX_IFR_EXPRESSION_DEPTH) {
+  _PCATCH 
(VFR_RETURN_INVALID_PARAMETER, 0, "The depth of expression exceeds the max 
supported level 8!");
+}
+_INIT_OPHDR_COND 
();
+  }
+   >>
   andTerm[$RootLevel, $ExpOpCount]
   (
 L:OR andTerm[$RootLevel, $ExpOpCount]  << $ExpOpCount++; 
CIfrOr OObj(L->getLine()); >>
   )*
<<
@@ -4988,10 +4996,11 @@ private:
   CIfrOpHeader *  mCIfrOpHdr[MAX_IFR_EXPRESSION_DEPTH];
   UINT32  mCIfrOpHdrLineNo[MAX_IFR_EXPRESSION_DEPTH];
   UINT8   mCIfrOpHdrIndex;
   VOID_SAVE_OPHDR_COND (IN CIfrOpHeader &, IN BOOLEAN, UINT32 
LineNo = 0);
   VOID_CLEAR_SAVED_OPHDR (VOID);
+  VOID_INIT_OPHDR_COND (VOID);
   BOOLEAN _SET_SAVED_OPHDR_SCOPE (VOID);
 
 
   EFI_VARSTORE_INFO   mCurrQestVarInfo;
   EFI_GUID*mOverrideClassGuid;
@@ -5077,20 +5086,28 @@ EfiVfrParser::_SAVE_OPHDR_COND (
 mCIfrOpHdr[mCIfrOpHdrIndex]   = new CIfrOpHeader(OpHdr);
 mCIfrOpHdrLineNo[mCIfrOpHdrIndex] = LineNo;
   }
 }
 
+VOID
+EfiVfrParser::_INIT_OPHDR_COND (
+  VOID
+  )
+{
+  mCIfrOpHdr[mCIfrOpHdrIndex]   = NULL;
+  mCIfrOpHdrLineNo[mCIfrOpHdrIndex] = 0;
+}
+
 VOID
 EfiVfrParser::_CLEAR_SAVED_OPHDR (
   VOID
   )
 {
   if (mCIfrOpHdr[mCIfrOpHdrIndex] != NULL) {
 delete mCIfrOpHdr[mCIfrOpHdrIndex];
-mCIfrOpHdr[mCIfrOpHdrIndex] = NULL;
+mCIfrOpHdr[mCIfrOpHdrIndex] = NULL;
   }
-  mCIfrOpHdrLineNo[mCIfrOpHdrIndex] = 0;
 }
 
 BOOLEAN
 EfiVfrParser::_SET_SAVED_OPHDR_SCOPE (
   VOID
-- 
2.14.3.windows.1

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


Re: [edk2] [patch] BaseTools/VfrCompile: Avoid using uninitialized pointer

2018-05-08 Thread Gary Lin
On Tue, May 08, 2018 at 07:46:19PM +0800, Dandan Bi wrote:
> _CLEAR_SAVED_OPHDR () is used for initialize the variables.
> We should not update it to free memory.
> It will cause some pointer used before initialization.
> This patch is to fix this issue.
> 
> Cc: Eric Dong 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi 
> ---
>  BaseTools/Source/C/VfrCompile/VfrSyntax.g | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g 
> b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> index 4b0a43606ea..cc042ab4307 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> +++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> @@ -4103,12 +4103,15 @@ vfrStatementExpression [UINT32 RootLevel, UINT32 
> ExpOpCount = 0] :
>}
>  }
>}
>
>if ($RootLevel == 
> 0) {
> -
> _CLEAR_SAVED_OPHDR ();
> -mCIfrOpHdrIndex 
> --;
> +if 
> (mCIfrOpHdr[mCIfrOpHdrIndex] != NULL) {
> +  delete 
> mCIfrOpHdr[mCIfrOpHdrIndex];
> +  
> mCIfrOpHdr[mCIfrOpHdrIndex] = NULL;
> +}

> + mCIfrOpHdrIndex 
> --;
An extra space was added.

>}
> >>
>;
>  
>  //
> @@ -5082,14 +5085,11 @@ EfiVfrParser::_SAVE_OPHDR_COND (
>  VOID
>  EfiVfrParser::_CLEAR_SAVED_OPHDR (
>VOID
>)
>  {
> -  if (mCIfrOpHdr[mCIfrOpHdrIndex] != NULL) {
> -delete mCIfrOpHdr[mCIfrOpHdrIndex];
> -mCIfrOpHdr[mCIfrOpHdrIndex] = NULL;
> -  }
> +  mCIfrOpHdr[mCIfrOpHdrIndex]   = NULL;
>mCIfrOpHdrLineNo[mCIfrOpHdrIndex] = 0;
>  }
>  
>  BOOLEAN
>  EfiVfrParser::_SET_SAVED_OPHDR_SCOPE (
> -- 
I applied the patch and triggered the rebuild of ovmf. It's now built on
all versions and arch.

Thanks for fixing it.

Tested-by: Gary Lin 

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


Re: [edk2] [patch 1/2] BaseTools/VfrCompile:Fix memory leak issues

2018-05-08 Thread Gary Lin
On Tue, May 08, 2018 at 11:50:46AM +, Bi, Dandan wrote:
> Yes. We have submitted patch to fix it. Sorry for the inconvenience.
> 
Awesome! Thanks for the quick response :)

Gary Lin

> Thanks,
> Dandan
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Gary 
> Lin
> Sent: Tuesday, May 8, 2018 5:46 PM
> To: Bi, Dandan 
> Cc: edk2-devel@lists.01.org; Dong, Eric ; Gao, Liming 
> 
> Subject: Re: [edk2] [patch 1/2] BaseTools/VfrCompile:Fix memory leak issues
> 
> On Tue, Apr 10, 2018 at 03:54:46PM +0800, Dandan Bi wrote:
> > Cc: Eric Dong 
> > Cc: Liming Gao 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Dandan Bi 
> > ---
> >  BaseTools/Source/C/VfrCompile/VfrSyntax.g | 32 
> > ++-
> >  1 file changed, 31 insertions(+), 1 deletion(-)
> > 
> > diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g 
> > b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> > index d48072a8adf..4b0a43606ea 100644
> > --- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> > +++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> Hi Dandan,
> 
> I encountered a build error with our build service:
> 
> [  197s] "VfrCompile" -l -n --string-db 
> /home/abuild/rpmbuild/BUILD/ovmf-2018+git1525681922.053cd183c9f2/Build/OvmfX64/DEBUG_GCC5/X64/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib/OUTPUT/BootMaintenanceManagerUiLibStrDefs.hpk
>  --output-directory 
> /home/abuild/rpmbuild/BUILD/ovmf-2018+git1525681922.053cd183c9f2/Build/OvmfX64/DEBUG_GCC5/X64/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib/DEBUG/.
>  
> /home/abuild/rpmbuild/BUILD/ovmf-2018+git1525681922.053cd183c9f2/Build/OvmfX64/DEBUG_GCC5/X64/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib/OUTPUT/BootMaintenanceManager.i
> [  197s] *** Error in 
> `/home/abuild/rpmbuild/BUILD/ovmf-2018+git1525681922.053cd183c9f2/BaseTools/Source/C/bin/VfrCompile':
>  free(): invalid pointer: 0xbabababababababa ***
> 
> If I reverted the following change, the package can be built again.
> 
> > @@ -5055,11 +5082,14 @@ EfiVfrParser::_SAVE_OPHDR_COND (  VOID  
> > EfiVfrParser::_CLEAR_SAVED_OPHDR (
> >VOID
> >)
> >  {
> > -  mCIfrOpHdr[mCIfrOpHdrIndex]   = NULL;
> > +  if (mCIfrOpHdr[mCIfrOpHdrIndex] != NULL) {
> > +delete mCIfrOpHdr[mCIfrOpHdrIndex];
> > +mCIfrOpHdr[mCIfrOpHdrIndex] = NULL;
> > +  }
> >mCIfrOpHdrLineNo[mCIfrOpHdrIndex] = 0;  }
> >  
> >  BOOLEAN
> >  EfiVfrParser::_SET_SAVED_OPHDR_SCOPE (
> 
> I have no clue now and it happened all the time.
> 
> Would you mind to check the code?
> 
> Thanks,
> 
> Gary Lin
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c

2018-05-08 Thread Tim Lewis
I think that this is a fatal error in EDK2. It basically says, "we are out
of stack space." The alternative is: the system hangs in an unexpected way
since the stack overflows into other pages.

Tim

-Original Message-
From: edk2-devel  On Behalf Of Yao, Jiewen
Sent: Tuesday, May 8, 2018 5:25 PM
To: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c

We discussed internally in Intel.

The quick workaround is: use /Gs65536. :-)

At the same time, our recommendation is to revisit the code which triggers
this error. Why this function need such a big stack? And try to reduce the
local stack usage.

What is why we still use /Gs32768 as default, instead of /Gs65536.

Thank you
Yao Jiewen


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Marvin H?user
> Sent: Tuesday, May 8, 2018 5:21 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen 
> Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> 
> Oh, if you are prefering a good implementation, I will be all for 
> that. This was just a 'quick workaround', same as currently done for GCC.
> I'm actually unsure whether a good implementation is possible with a 
> flat memory model. It will likely be mere luck whether there is enough 
> space free below the stack, except for maybe when it's located very 
> high (preferably past the 32-bit space).
> Has there been any prior discussion on this topic? Would be interested 
> to follow up if there was.
> 
> Thanks,
> Marvin
> 
> > -Original Message-
> > From: Yao, Jiewen 
> > Sent: Wednesday, May 9, 2018 2:13 AM
> > To: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
> > Cc: Gao, Liming 
> > Subject: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> >
> > There are some open source implementation:
> > MSVC: https://github.com/ispc/ispc/issues/542
> > GCC:
> > https://android.googlesource.com/toolchain/gcc/+/b094d6c4bf572654a03
> > 1e cc4afe675154c886dc5/gcc-4.4.3/gcc/config/i386/chkstk.asm
> >
> > The compiler generated code assumes the stack is enlarged after we 
> > can chkstk.
> >
> > I agree empty function can make build pass.
> > But more important, we need make it work if there is a need to 
> > increase the stack.
> > The potential issue is that the later code (after chkstk) assumes 
> > the stack is increased, but actually it is not.
> >
> > That is why I ask how this is validated.
> >
> > Thank you
> > Yao Jiewen
> >
> >
> > > -Original Message-
> > > From: Marvin Häuser [mailto:marvin.haeu...@outlook.com]
> > > Sent: Tuesday, May 8, 2018 4:58 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Yao, Jiewen ; Gao, Liming 
> > > 
> > > Subject: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> > >
> > > Hey Yao,
> > >
> > > As far as I am aware, all __chkstk does is access the stack in 4 
> > > KB intervals from the current location to the maximum requested 
> > > location to eventually hit the Windows Guard Page, which then 
> > > triggers the stack
> > increase.
> > > Because I do not know of any equivalent concept in edk2 and the 
> > > intrinsic was already disabled for GCC, I supposed it was a good 
> > > idea to do so globally. Are the potential issues I am not aware of?
> > >
> > > Thanks,
> > > Marvin.
> > >
> > > > -Original Message-
> > > > From: Yao, Jiewen 
> > > > Sent: Wednesday, May 9, 2018 1:52 AM
> > > > To: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
> > > > Cc: Gao, Liming 
> > > > Subject: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> > > >
> > > > HI Marvin
> > > > Would you mind to share the information on how you test this update?
> > > >
> > > >
> > > > Per my experience, chkstk not only does the check but also does 
> > > > the real work to allocate more stack.
> > > >
> > > > /Gs can be used to indicate the size (/Gs[num] control stack 
> > > > checking calls)
> > > >
> > > > We use /Gs32768 by default, see tools_def.txt.
> > > >
> > > > Usually, we just increase this value to override the default 
> > > > one, if we hit this issue.
> > > >
> > > > Thank you
> > > > Yao Jiewen
> > > >
> > > > > -Original Message-
> > > > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On 
> > > > > Behalf Of Marvin H?user
> > > > > Sent: Tuesday, May 8, 2018 4:37 PM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Gao, Liming 
> > > > > Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include 
> > > > > ChkStk.c
> > > > >
> > > > > Hey Liming,
> > > > >
> > > > > According to the MSDN documentation, the call will be inserted 
> > > > > for a stack usage past a defined threshold - 4 KB for IA32, 8 
> > > > > KB for X64
> > (source:
> > > > > 

Re: [edk2] [PATCH v1 1/1] BaseTools: incorrect calculation for 16M

2018-05-08 Thread Zhu, Yonghong
Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong


-Original Message-
From: Carsey, Jaben 
Sent: Wednesday, May 09, 2018 12:02 AM
To: edk2-devel@lists.01.org
Cc: Gao, Liming ; Zhu, Yonghong 
Subject: [PATCH v1 1/1] BaseTools: incorrect calculation for 16M

the "0x" was missing.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/GenFds/FvImageSection.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/FvImageSection.py 
b/BaseTools/Source/Python/GenFds/FvImageSection.py
index eb84b44bbec4..57ecea0377bf 100644
--- a/BaseTools/Source/Python/GenFds/FvImageSection.py
+++ b/BaseTools/Source/Python/GenFds/FvImageSection.py
@@ -1,7 +1,7 @@
 ## @file
 # process FV image section generation
 #
-#  Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.
+#  Copyright (c) 2007 - 2018, Intel Corporation. All rights 
+reserved.
 #
 #  This program and the accompanying materials  #  are licensed and made 
available under the terms and conditions of the BSD License @@ -83,7 +83,7 @@ 
class FvImageSection(FvImageSectionClassObject):
 if MaxFvAlignment >= 0x400:
 if MaxFvAlignment >= 0x10:
 #The max alignment supported by FFS is 16M.
-if MaxFvAlignment >=100:
+if MaxFvAlignment >= 0x100:
 self.Alignment = "16M"
 else:
 self.Alignment = str(MaxFvAlignment / 0x10) + "M"
--
2.16.2.windows.1

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


Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c

2018-05-08 Thread Yao, Jiewen
We discussed internally in Intel.

The quick workaround is: use /Gs65536. :-)

At the same time, our recommendation is to revisit the code which triggers this 
error. Why this function need such a big stack? And try to reduce the local 
stack usage.

What is why we still use /Gs32768 as default, instead of /Gs65536.

Thank you
Yao Jiewen


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Marvin H?user
> Sent: Tuesday, May 8, 2018 5:21 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen 
> Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> 
> Oh, if you are prefering a good implementation, I will be all for that. This 
> was just
> a 'quick workaround', same as currently done for GCC.
> I'm actually unsure whether a good implementation is possible with a flat
> memory model. It will likely be mere luck whether there is enough space free
> below the stack, except for maybe when it's located very high (preferably past
> the 32-bit space).
> Has there been any prior discussion on this topic? Would be interested to 
> follow
> up if there was.
> 
> Thanks,
> Marvin
> 
> > -Original Message-
> > From: Yao, Jiewen 
> > Sent: Wednesday, May 9, 2018 2:13 AM
> > To: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
> > Cc: Gao, Liming 
> > Subject: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> >
> > There are some open source implementation:
> > MSVC: https://github.com/ispc/ispc/issues/542
> > GCC:
> > https://android.googlesource.com/toolchain/gcc/+/b094d6c4bf572654a031e
> > cc4afe675154c886dc5/gcc-4.4.3/gcc/config/i386/chkstk.asm
> >
> > The compiler generated code assumes the stack is enlarged after we can
> > chkstk.
> >
> > I agree empty function can make build pass.
> > But more important, we need make it work if there is a need to increase the
> > stack.
> > The potential issue is that the later code (after chkstk) assumes the stack 
> > is
> > increased, but actually it is not.
> >
> > That is why I ask how this is validated.
> >
> > Thank you
> > Yao Jiewen
> >
> >
> > > -Original Message-
> > > From: Marvin Häuser [mailto:marvin.haeu...@outlook.com]
> > > Sent: Tuesday, May 8, 2018 4:58 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Yao, Jiewen ; Gao, Liming
> > > 
> > > Subject: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> > >
> > > Hey Yao,
> > >
> > > As far as I am aware, all __chkstk does is access the stack in 4 KB
> > > intervals from the current location to the maximum requested location
> > > to eventually hit the Windows Guard Page, which then triggers the stack
> > increase.
> > > Because I do not know of any equivalent concept in edk2 and the
> > > intrinsic was already disabled for GCC, I supposed it was a good idea
> > > to do so globally. Are the potential issues I am not aware of?
> > >
> > > Thanks,
> > > Marvin.
> > >
> > > > -Original Message-
> > > > From: Yao, Jiewen 
> > > > Sent: Wednesday, May 9, 2018 1:52 AM
> > > > To: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
> > > > Cc: Gao, Liming 
> > > > Subject: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> > > >
> > > > HI Marvin
> > > > Would you mind to share the information on how you test this update?
> > > >
> > > >
> > > > Per my experience, chkstk not only does the check but also does the
> > > > real work to allocate more stack.
> > > >
> > > > /Gs can be used to indicate the size (/Gs[num] control stack
> > > > checking calls)
> > > >
> > > > We use /Gs32768 by default, see tools_def.txt.
> > > >
> > > > Usually, we just increase this value to override the default one, if
> > > > we hit this issue.
> > > >
> > > > Thank you
> > > > Yao Jiewen
> > > >
> > > > > -Original Message-
> > > > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On
> > > > > Behalf Of Marvin H?user
> > > > > Sent: Tuesday, May 8, 2018 4:37 PM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Gao, Liming 
> > > > > Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include
> > > > > ChkStk.c
> > > > >
> > > > > Hey Liming,
> > > > >
> > > > > According to the MSDN documentation, the call will be inserted for
> > > > > a stack usage past a defined threshold - 4 KB for IA32, 8 KB for X64
> > (source:
> > > > > https://msdn.microsoft.com/en-us/library/ms648426(v=vs.85).aspx).
> > > > >
> > > > > Thanks,
> > > > > Marvin
> > > > >
> > > > > > -Ursprüngliche Nachricht-
> > > > > > Von: Gao, Liming 
> > > > > > Gesendet: Montag, 7. Mai 2018 14:16
> > > > > > An: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
> > > > > > Cc: Kinney, Michael D 
> > > > > > Betreff: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> > > > > >
> > > > > > Marvin:
> > > > > >   In VS 

Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c

2018-05-08 Thread Marvin Häuser
Oh, if you are prefering a good implementation, I will be all for that. This 
was just a 'quick workaround', same as currently done for GCC.
I'm actually unsure whether a good implementation is possible with a flat 
memory model. It will likely be mere luck whether there is enough space free 
below the stack, except for maybe when it's located very high (preferably past 
the 32-bit space).
Has there been any prior discussion on this topic? Would be interested to 
follow up if there was.

Thanks,
Marvin

> -Original Message-
> From: Yao, Jiewen 
> Sent: Wednesday, May 9, 2018 2:13 AM
> To: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> 
> There are some open source implementation:
> MSVC: https://github.com/ispc/ispc/issues/542
> GCC:
> https://android.googlesource.com/toolchain/gcc/+/b094d6c4bf572654a031e
> cc4afe675154c886dc5/gcc-4.4.3/gcc/config/i386/chkstk.asm
> 
> The compiler generated code assumes the stack is enlarged after we can
> chkstk.
> 
> I agree empty function can make build pass.
> But more important, we need make it work if there is a need to increase the
> stack.
> The potential issue is that the later code (after chkstk) assumes the stack is
> increased, but actually it is not.
> 
> That is why I ask how this is validated.
> 
> Thank you
> Yao Jiewen
> 
> 
> > -Original Message-
> > From: Marvin Häuser [mailto:marvin.haeu...@outlook.com]
> > Sent: Tuesday, May 8, 2018 4:58 PM
> > To: edk2-devel@lists.01.org
> > Cc: Yao, Jiewen ; Gao, Liming
> > 
> > Subject: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> >
> > Hey Yao,
> >
> > As far as I am aware, all __chkstk does is access the stack in 4 KB
> > intervals from the current location to the maximum requested location
> > to eventually hit the Windows Guard Page, which then triggers the stack
> increase.
> > Because I do not know of any equivalent concept in edk2 and the
> > intrinsic was already disabled for GCC, I supposed it was a good idea
> > to do so globally. Are the potential issues I am not aware of?
> >
> > Thanks,
> > Marvin.
> >
> > > -Original Message-
> > > From: Yao, Jiewen 
> > > Sent: Wednesday, May 9, 2018 1:52 AM
> > > To: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
> > > Cc: Gao, Liming 
> > > Subject: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> > >
> > > HI Marvin
> > > Would you mind to share the information on how you test this update?
> > >
> > >
> > > Per my experience, chkstk not only does the check but also does the
> > > real work to allocate more stack.
> > >
> > > /Gs can be used to indicate the size (/Gs[num] control stack
> > > checking calls)
> > >
> > > We use /Gs32768 by default, see tools_def.txt.
> > >
> > > Usually, we just increase this value to override the default one, if
> > > we hit this issue.
> > >
> > > Thank you
> > > Yao Jiewen
> > >
> > > > -Original Message-
> > > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On
> > > > Behalf Of Marvin H?user
> > > > Sent: Tuesday, May 8, 2018 4:37 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Gao, Liming 
> > > > Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include
> > > > ChkStk.c
> > > >
> > > > Hey Liming,
> > > >
> > > > According to the MSDN documentation, the call will be inserted for
> > > > a stack usage past a defined threshold - 4 KB for IA32, 8 KB for X64
> (source:
> > > > https://msdn.microsoft.com/en-us/library/ms648426(v=vs.85).aspx).
> > > >
> > > > Thanks,
> > > > Marvin
> > > >
> > > > > -Ursprüngliche Nachricht-
> > > > > Von: Gao, Liming 
> > > > > Gesendet: Montag, 7. Mai 2018 14:16
> > > > > An: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
> > > > > Cc: Kinney, Michael D 
> > > > > Betreff: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> > > > >
> > > > > Marvin:
> > > > >   In VS compiler, what case will trig this intrinsic function?
> > > > >
> > > > > Thanks
> > > > > Liming
> > > > > > -Original Message-
> > > > > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On
> > > > > > Behalf Of Marvin H?user
> > > > > > Sent: Saturday, May 5, 2018 10:25 PM
> > > > > > To: edk2-devel@lists.01.org
> > > > > > Cc: Kinney, Michael D ; Gao,
> > > > > > Liming 
> > > > > > Subject: [edk2] [PATCH] MdePkg/BaseLib: Globally include
> > > > > > ChkStk.c
> > > > > >
> > > > > > Initially added for GCC build support, this patch includes the
> > > > > > function for all compilers and all architectures. This is done
> > > > > > as huge variables on the stack may cause the generation of
> > > > > > calls to this intrinsic function for Microsoft compilers, even
> > > > > > for the IA32 

Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c

2018-05-08 Thread Yao, Jiewen
There are some open source implementation:
MSVC: https://github.com/ispc/ispc/issues/542
GCC: 
https://android.googlesource.com/toolchain/gcc/+/b094d6c4bf572654a031ecc4afe675154c886dc5/gcc-4.4.3/gcc/config/i386/chkstk.asm

The compiler generated code assumes the stack is enlarged after we can chkstk.

I agree empty function can make build pass.
But more important, we need make it work if there is a need to increase the 
stack.
The potential issue is that the later code (after chkstk) assumes the stack is 
increased, but actually it is not.

That is why I ask how this is validated.

Thank you
Yao Jiewen


> -Original Message-
> From: Marvin Häuser [mailto:marvin.haeu...@outlook.com]
> Sent: Tuesday, May 8, 2018 4:58 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Gao, Liming 
> Subject: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> 
> Hey Yao,
> 
> As far as I am aware, all __chkstk does is access the stack in 4 KB intervals 
> from
> the current location to the maximum requested location to eventually hit the
> Windows Guard Page, which then triggers the stack increase.
> Because I do not know of any equivalent concept in edk2 and the intrinsic was
> already disabled for GCC, I supposed it was a good idea to do so globally. 
> Are the
> potential issues I am not aware of?
> 
> Thanks,
> Marvin.
> 
> > -Original Message-
> > From: Yao, Jiewen 
> > Sent: Wednesday, May 9, 2018 1:52 AM
> > To: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
> > Cc: Gao, Liming 
> > Subject: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> >
> > HI Marvin
> > Would you mind to share the information on how you test this update?
> >
> >
> > Per my experience, chkstk not only does the check but also does the real
> > work to allocate more stack.
> >
> > /Gs can be used to indicate the size (/Gs[num] control stack checking calls)
> >
> > We use /Gs32768 by default, see tools_def.txt.
> >
> > Usually, we just increase this value to override the default one, if we hit 
> > this
> > issue.
> >
> > Thank you
> > Yao Jiewen
> >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > > Marvin H?user
> > > Sent: Tuesday, May 8, 2018 4:37 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Gao, Liming 
> > > Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> > >
> > > Hey Liming,
> > >
> > > According to the MSDN documentation, the call will be inserted for a
> > > stack usage past a defined threshold - 4 KB for IA32, 8 KB for X64 
> > > (source:
> > > https://msdn.microsoft.com/en-us/library/ms648426(v=vs.85).aspx).
> > >
> > > Thanks,
> > > Marvin
> > >
> > > > -Ursprüngliche Nachricht-
> > > > Von: Gao, Liming 
> > > > Gesendet: Montag, 7. Mai 2018 14:16
> > > > An: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
> > > > Cc: Kinney, Michael D 
> > > > Betreff: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> > > >
> > > > Marvin:
> > > >   In VS compiler, what case will trig this intrinsic function?
> > > >
> > > > Thanks
> > > > Liming
> > > > > -Original Message-
> > > > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On
> > > > > Behalf Of Marvin H?user
> > > > > Sent: Saturday, May 5, 2018 10:25 PM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Kinney, Michael D ; Gao, Liming
> > > > > 
> > > > > Subject: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> > > > >
> > > > > Initially added for GCC build support, this patch includes the
> > > > > function for all compilers and all architectures. This is done as
> > > > > huge variables on the stack may cause the generation of calls to
> > > > > this intrinsic function for Microsoft compilers, even for the IA32
> > > > > architecture, too.
> > > > >
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Marvin Haeuser 
> > > > > ---
> > > > >  MdePkg/Library/BaseLib/{ChkStkGcc.c => ChkStk.c} | 48
> > > > > ++--
> > > > 
> > > > >  MdePkg/Library/BaseLib/BaseLib.inf   |  4 +-
> > > > >  2 files changed, 26 insertions(+), 26 deletions(-)
> > > > >
> > > > > diff --git a/MdePkg/Library/BaseLib/ChkStkGcc.c
> > > > > b/MdePkg/Library/BaseLib/ChkStk.c similarity index 74% rename from
> > > > > MdePkg/Library/BaseLib/ChkStkGcc.c
> > > > > rename to MdePkg/Library/BaseLib/ChkStk.c index
> > > > > ecba3853b159..59e6d73f9331 100644
> > > > > --- a/MdePkg/Library/BaseLib/ChkStkGcc.c
> > > > > +++ b/MdePkg/Library/BaseLib/ChkStk.c
> > > > > @@ -1,24 +1,24 @@
> > > > > -/** @file
> > > > > -  Provides hack function for passng GCC build.
> > > > > -
> > > > > -  Copyright (c) 2006 - 2008, Intel Corporation. All rights
> > > > > 

Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c

2018-05-08 Thread Marvin Häuser
Hey Yao,

As far as I am aware, all __chkstk does is access the stack in 4 KB intervals 
from the current location to the maximum requested location to eventually hit 
the Windows Guard Page, which then triggers the stack increase.
Because I do not know of any equivalent concept in edk2 and the intrinsic was 
already disabled for GCC, I supposed it was a good idea to do so globally. Are 
the potential issues I am not aware of?

Thanks,
Marvin.

> -Original Message-
> From: Yao, Jiewen 
> Sent: Wednesday, May 9, 2018 1:52 AM
> To: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> 
> HI Marvin
> Would you mind to share the information on how you test this update?
> 
> 
> Per my experience, chkstk not only does the check but also does the real
> work to allocate more stack.
> 
> /Gs can be used to indicate the size (/Gs[num] control stack checking calls)
> 
> We use /Gs32768 by default, see tools_def.txt.
> 
> Usually, we just increase this value to override the default one, if we hit 
> this
> issue.
> 
> Thank you
> Yao Jiewen
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Marvin H?user
> > Sent: Tuesday, May 8, 2018 4:37 PM
> > To: edk2-devel@lists.01.org
> > Cc: Gao, Liming 
> > Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> >
> > Hey Liming,
> >
> > According to the MSDN documentation, the call will be inserted for a
> > stack usage past a defined threshold - 4 KB for IA32, 8 KB for X64 (source:
> > https://msdn.microsoft.com/en-us/library/ms648426(v=vs.85).aspx).
> >
> > Thanks,
> > Marvin
> >
> > > -Ursprüngliche Nachricht-
> > > Von: Gao, Liming 
> > > Gesendet: Montag, 7. Mai 2018 14:16
> > > An: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
> > > Cc: Kinney, Michael D 
> > > Betreff: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> > >
> > > Marvin:
> > >   In VS compiler, what case will trig this intrinsic function?
> > >
> > > Thanks
> > > Liming
> > > > -Original Message-
> > > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On
> > > > Behalf Of Marvin H?user
> > > > Sent: Saturday, May 5, 2018 10:25 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Kinney, Michael D ; Gao, Liming
> > > > 
> > > > Subject: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> > > >
> > > > Initially added for GCC build support, this patch includes the
> > > > function for all compilers and all architectures. This is done as
> > > > huge variables on the stack may cause the generation of calls to
> > > > this intrinsic function for Microsoft compilers, even for the IA32
> > > > architecture, too.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Marvin Haeuser 
> > > > ---
> > > >  MdePkg/Library/BaseLib/{ChkStkGcc.c => ChkStk.c} | 48
> > > > ++--
> > > 
> > > >  MdePkg/Library/BaseLib/BaseLib.inf   |  4 +-
> > > >  2 files changed, 26 insertions(+), 26 deletions(-)
> > > >
> > > > diff --git a/MdePkg/Library/BaseLib/ChkStkGcc.c
> > > > b/MdePkg/Library/BaseLib/ChkStk.c similarity index 74% rename from
> > > > MdePkg/Library/BaseLib/ChkStkGcc.c
> > > > rename to MdePkg/Library/BaseLib/ChkStk.c index
> > > > ecba3853b159..59e6d73f9331 100644
> > > > --- a/MdePkg/Library/BaseLib/ChkStkGcc.c
> > > > +++ b/MdePkg/Library/BaseLib/ChkStk.c
> > > > @@ -1,24 +1,24 @@
> > > > -/** @file
> > > > -  Provides hack function for passng GCC build.
> > > > -
> > > > -  Copyright (c) 2006 - 2008, 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.
> > > > -
> > > > -  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS
> IS"
> > > > BASIS,
> > > > -  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> > > EXPRESS OR IMPLIED.
> > > > -
> > > > -**/
> > > > -
> > > > -#include "BaseLibInternals.h"
> > > > -
> > > > -/**
> > > > -  Hack function for passing GCC build.
> > > > -**/
> > > > -VOID
> > > > -__chkstk()
> > > > -{
> > > > -}
> > > > -
> > > > +/** @file
> > > > +  Provides hack function for passing build.
> > > > +
> > > > +  Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > > > + reserved.  This program and the accompanying materials  are
> > > > + licensed and made available under the terms and conditions of
> > > > + the BSD License  which accompanies this distribution.  The full
> > > > + text 

Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c

2018-05-08 Thread Yao, Jiewen
HI Marvin
Would you mind to share the information on how you test this update?


Per my experience, chkstk not only does the check but also does the real work 
to allocate more stack.

/Gs can be used to indicate the size (/Gs[num] control stack checking calls)

We use /Gs32768 by default, see tools_def.txt.

Usually, we just increase this value to override the default one, if we hit 
this issue.

Thank you
Yao Jiewen

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Marvin H?user
> Sent: Tuesday, May 8, 2018 4:37 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> 
> Hey Liming,
> 
> According to the MSDN documentation, the call will be inserted for a stack 
> usage
> past a defined threshold - 4 KB for IA32, 8 KB for X64 (source:
> https://msdn.microsoft.com/en-us/library/ms648426(v=vs.85).aspx).
> 
> Thanks,
> Marvin
> 
> > -Ursprüngliche Nachricht-
> > Von: Gao, Liming 
> > Gesendet: Montag, 7. Mai 2018 14:16
> > An: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
> > Cc: Kinney, Michael D 
> > Betreff: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> >
> > Marvin:
> >   In VS compiler, what case will trig this intrinsic function?
> >
> > Thanks
> > Liming
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > > Marvin H?user
> > > Sent: Saturday, May 5, 2018 10:25 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Kinney, Michael D ; Gao, Liming
> > > 
> > > Subject: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> > >
> > > Initially added for GCC build support, this patch includes the
> > > function for all compilers and all architectures. This is done as huge
> > > variables on the stack may cause the generation of calls to this
> > > intrinsic function for Microsoft compilers, even for the IA32
> > > architecture, too.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Marvin Haeuser 
> > > ---
> > >  MdePkg/Library/BaseLib/{ChkStkGcc.c => ChkStk.c} | 48 ++--
> > 
> > >  MdePkg/Library/BaseLib/BaseLib.inf   |  4 +-
> > >  2 files changed, 26 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/MdePkg/Library/BaseLib/ChkStkGcc.c
> > > b/MdePkg/Library/BaseLib/ChkStk.c similarity index 74% rename from
> > > MdePkg/Library/BaseLib/ChkStkGcc.c
> > > rename to MdePkg/Library/BaseLib/ChkStk.c index
> > > ecba3853b159..59e6d73f9331 100644
> > > --- a/MdePkg/Library/BaseLib/ChkStkGcc.c
> > > +++ b/MdePkg/Library/BaseLib/ChkStk.c
> > > @@ -1,24 +1,24 @@
> > > -/** @file
> > > -  Provides hack function for passng GCC build.
> > > -
> > > -  Copyright (c) 2006 - 2008, 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.
> > > -
> > > -  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > > BASIS,
> > > -  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> > EXPRESS OR IMPLIED.
> > > -
> > > -**/
> > > -
> > > -#include "BaseLibInternals.h"
> > > -
> > > -/**
> > > -  Hack function for passing GCC build.
> > > -**/
> > > -VOID
> > > -__chkstk()
> > > -{
> > > -}
> > > -
> > > +/** @file
> > > +  Provides hack function for passing build.
> > > +
> > > +  Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > > + reserved.  This program and the accompanying materials  are
> > > + licensed and made available under the terms and conditions of the
> > > + BSD License  which accompanies this distribution.  The full text of
> > > + the license may be found at  http://opensource.org/licenses/bsd-
> > license.php.
> > > +
> > > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > > + BASIS,  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> > EITHER EXPRESS OR IMPLIED.
> > > +
> > > +**/
> > > +
> > > +#include "BaseLibInternals.h"
> > > +
> > > +/**
> > > +  Hack function for passing build.
> > > +**/
> > > +VOID
> > > +__chkstk()
> > > +{
> > > +}
> > > +
> > > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf
> > > b/MdePkg/Library/BaseLib/BaseLib.inf
> > > index 5fbbd02a94b6..d23a6db2581a 100644
> > > --- a/MdePkg/Library/BaseLib/BaseLib.inf
> > > +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> > > @@ -1,7 +1,7 @@
> > >  ## @file
> > >  #  Base Library implementation.
> > >  #
> > > -#  Copyright (c) 2007 - 2016, Intel Corporation. All rights
> > > reserved.
> > > +#  Copyright (c) 2007 - 2018, Intel Corporation. All rights
> > > +reserved.
> > >  #  

Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c

2018-05-08 Thread Marvin Häuser
Hey Liming,

According to the MSDN documentation, the call will be inserted for a stack 
usage past a defined threshold - 4 KB for IA32, 8 KB for X64 (source: 
https://msdn.microsoft.com/en-us/library/ms648426(v=vs.85).aspx).

Thanks,
Marvin

> -Ursprüngliche Nachricht-
> Von: Gao, Liming 
> Gesendet: Montag, 7. Mai 2018 14:16
> An: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
> Cc: Kinney, Michael D 
> Betreff: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> 
> Marvin:
>   In VS compiler, what case will trig this intrinsic function?
> 
> Thanks
> Liming
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Marvin H?user
> > Sent: Saturday, May 5, 2018 10:25 PM
> > To: edk2-devel@lists.01.org
> > Cc: Kinney, Michael D ; Gao, Liming
> > 
> > Subject: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> >
> > Initially added for GCC build support, this patch includes the
> > function for all compilers and all architectures. This is done as huge
> > variables on the stack may cause the generation of calls to this
> > intrinsic function for Microsoft compilers, even for the IA32
> > architecture, too.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marvin Haeuser 
> > ---
> >  MdePkg/Library/BaseLib/{ChkStkGcc.c => ChkStk.c} | 48 ++--
> 
> >  MdePkg/Library/BaseLib/BaseLib.inf   |  4 +-
> >  2 files changed, 26 insertions(+), 26 deletions(-)
> >
> > diff --git a/MdePkg/Library/BaseLib/ChkStkGcc.c
> > b/MdePkg/Library/BaseLib/ChkStk.c similarity index 74% rename from
> > MdePkg/Library/BaseLib/ChkStkGcc.c
> > rename to MdePkg/Library/BaseLib/ChkStk.c index
> > ecba3853b159..59e6d73f9331 100644
> > --- a/MdePkg/Library/BaseLib/ChkStkGcc.c
> > +++ b/MdePkg/Library/BaseLib/ChkStk.c
> > @@ -1,24 +1,24 @@
> > -/** @file
> > -  Provides hack function for passng GCC build.
> > -
> > -  Copyright (c) 2006 - 2008, 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.
> > -
> > -  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > BASIS,
> > -  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > -
> > -**/
> > -
> > -#include "BaseLibInternals.h"
> > -
> > -/**
> > -  Hack function for passing GCC build.
> > -**/
> > -VOID
> > -__chkstk()
> > -{
> > -}
> > -
> > +/** @file
> > +  Provides hack function for passing build.
> > +
> > +  Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > + reserved.  This program and the accompanying materials  are
> > + licensed and made available under the terms and conditions of the
> > + BSD License  which accompanies this distribution.  The full text of
> > + the license may be found at  http://opensource.org/licenses/bsd-
> license.php.
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > + BASIS,  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> > +
> > +**/
> > +
> > +#include "BaseLibInternals.h"
> > +
> > +/**
> > +  Hack function for passing build.
> > +**/
> > +VOID
> > +__chkstk()
> > +{
> > +}
> > +
> > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf
> > b/MdePkg/Library/BaseLib/BaseLib.inf
> > index 5fbbd02a94b6..d23a6db2581a 100644
> > --- a/MdePkg/Library/BaseLib/BaseLib.inf
> > +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> > @@ -1,7 +1,7 @@
> >  ## @file
> >  #  Base Library implementation.
> >  #
> > -#  Copyright (c) 2007 - 2016, Intel Corporation. All rights
> > reserved.
> > +#  Copyright (c) 2007 - 2018, Intel Corporation. All rights
> > +reserved.
> >  #  Portions copyright (c) 2008 - 2009, Apple Inc. All rights
> > reserved.  #  Portions copyright (c) 2011 - 2013, ARM Ltd. All
> > rights reserved.  # @@ -64,6 +64,7 @@ [Sources]
> >SafeString.c
> >String.c
> >FilePaths.c
> > +  ChkStk.c
> >BaseLibInternals.h
> >
> >  [Sources.Ia32]
> > @@ -781,7 +782,6 @@ [Sources.X64]
> >X64/DisableCache.S | GCC
> >X64/RdRand.nasm| GCC
> >X64/RdRand.S | GCC
> > -  ChkStkGcc.c  | GCC
> >
> >  [Sources.IPF]
> >Ipf/AccessGp.s
> > --
> > 2.17.0.windows.1
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH 2/2] MdeModulePkg/AcpiPlatformDxe: Consume PlatformAcpiLib.

2018-05-08 Thread Marvin Häuser
This patch updates the generic ACPI Platform driver to consume
PlatformAcpiLib to allow platform specific updates to the ACPI tables
loaded from the configured Firmware Volume. This allows for more
platforms to incorporate the generic ACPI Platform driver.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser 
---
 MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatform.c  | 55 

 MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf |  1 +
 2 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatform.c 
b/MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatform.c
index 8f335bde0d46..f1243279faa2 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatform.c
+++ b/MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatform.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -180,6 +181,7 @@ AcpiPlatformEntryPoint (
   UINT32 FvStatus;
   UINTN  TableSize;
   UINTN  Size;
+  RETURN_STATUS  Return;
 
   Instance = 0;
   CurrentTable = NULL;
@@ -216,35 +218,42 @@ AcpiPlatformEntryPoint (
   );
 if (!EFI_ERROR(Status)) {
   //
-  // Add the table
+  // Perform any platform-specific table updates.
   //
-  TableHandle = 0;
+  Return = PlatformAcpiUpdateTable (CurrentTable);
 
-  TableSize = ((EFI_ACPI_DESCRIPTION_HEADER *) CurrentTable)->Length;
-  ASSERT (Size >= TableSize);
+  if (!RETURN_ERROR (Return)) {
+//
+// Add the table
+//
+TableHandle = 0;
 
-  //
-  // Checksum ACPI table
-  //
-  AcpiPlatformChecksum ((UINT8*)CurrentTable, TableSize);
+TableSize = ((EFI_ACPI_DESCRIPTION_HEADER *) CurrentTable)->Length;
+ASSERT (Size >= TableSize);
 
-  //
-  // Install ACPI table
-  //
-  Status = AcpiTable->InstallAcpiTable (
-AcpiTable,
-CurrentTable,
-TableSize,
-
-);
+//
+// Checksum ACPI table
+//
+AcpiPlatformChecksum ((UINT8*)CurrentTable, TableSize);
 
-  //
-  // Free memory allocated by ReadSection
-  //
-  gBS->FreePool (CurrentTable);
+//
+// Install ACPI table
+//
+Status = AcpiTable->InstallAcpiTable (
+  AcpiTable,
+  CurrentTable,
+  TableSize,
+  
+  );
+
+//
+// Free memory allocated by ReadSection
+//
+gBS->FreePool (CurrentTable);
 
-  if (EFI_ERROR(Status)) {
-return EFI_ABORTED;
+if (EFI_ERROR(Status)) {
+  return EFI_ABORTED;
+}
   }
 
   //
diff --git a/MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf 
b/MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf
index 34b1600171d5..114a12dc3396 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf
+++ b/MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf
@@ -40,6 +40,7 @@ [LibraryClasses]
   PcdLib
   BaseMemoryLib
   DebugLib
+  PlatformAcpiLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
 
-- 
2.17.0.windows.1

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


[edk2] [PATCH 1/2] MdeModulePkg: Add PlatformAcpiLib LibraryClass.

2018-05-08 Thread Marvin Häuser
PlatformAcpiLib can be consumed by the generic ACPI Platform driver
to allow platform specific updates to the ACPI tables loaded from the
configured Firmware Volume. This allows for more platforms to
incorporate the generic ACPI Platform driver.

This commit also provides a NULL implementation of PlatformAcpiLib.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser 
---
 MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull.c   | 36 

 MdeModulePkg/Include/Library/PlatformAcpiLib.h   | 36 

 MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull.inf | 35 
+++
 MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull.uni | 18 
++
 MdeModulePkg/MdeModulePkg.dec|  4 
+++
 MdeModulePkg/MdeModulePkg.dsc|  2 
++
 6 files changed, 131 insertions(+)

diff --git 
a/MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull.c 
b/MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull.c
new file mode 100644
index ..5d5d0d051e1b
--- /dev/null
+++ b/MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull.c
@@ -0,0 +1,36 @@
+/** @file
+  Null Platform ACPI Library instance.
+
+Copyright (c) 2018, Intel Corporation. All rights reserved.
+This program and the accompanying materials are licensed and made available 
under 
+the terms and conditions of the BSD License that accompanies this 
distribution.  
+The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php.

+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,  
   
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+
+#include 
+
+/**
+  Performs platform specific updates to CurrentTable.
+
+  @param[in,out] CurrentTable  The table to perform updates on.
+
+  @retval RETURN_SUCCESS  The platform specific ACPI table updates were applied
+  successfully.
+  @retval other   The platform specific ACPI table updates could not be
+  applied.
+  
+**/
+RETURN_STATUS
+PlatformAcpiUpdateTable (
+  IN OUT EFI_ACPI_COMMON_HEADER  *CurrentTable
+  )
+{
+  return RETURN_SUCCESS;
+}
diff --git a/MdeModulePkg/Include/Library/PlatformAcpiLib.h 
b/MdeModulePkg/Include/Library/PlatformAcpiLib.h
new file mode 100644
index ..a3e367f3ab61
--- /dev/null
+++ b/MdeModulePkg/Include/Library/PlatformAcpiLib.h
@@ -0,0 +1,36 @@
+/** @file
+  Platform ACPI library. Platform can provide an implementation of this
+  library class to provide an ACPI table update routine that may be required
+  for some type of platform initialization.
+
+Copyright (c) 2018, Intel Corporation. All rights reserved.
+This program and the accompanying materials are licensed and made available 
under 
+the terms and conditions of the BSD License that accompanies this 
distribution.  
+The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php.

+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,  
   
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __PLATFORM_ACPI_LIB__
+#define __PLATFORM_ACPI_LIB__
+
+/**
+  Performs platform specific updates to CurrentTable.
+
+  @param[in,out] CurrentTable  The table to perform updates on.
+
+  @retval RETURN_SUCCESS  The platform specific ACPI table updates were applied
+  successfully.
+  @retval other   The platform specific ACPI table updates could not be
+  applied.
+  
+**/
+RETURN_STATUS
+PlatformAcpiUpdateTable (
+  IN OUT EFI_ACPI_COMMON_HEADER  *CurrentTable
+  );
+
+#endif // __PLATFORM_ACPI_LIB__
diff --git 
a/MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull.inf 
b/MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull.inf
new file mode 100644
index ..a548ecdd91a7
--- /dev/null
+++ b/MdeModulePkg/Library/BasePlatformAcpiLibNull/BasePlatformAcpiLibNull.inf
@@ -0,0 +1,35 @@
+## @file
+#  Null Platform ACPI Library instance.
+#
+#  Copyright (c) 2018, Intel Corporation. All rights reserved.
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD 
License
+#  which accompanies this distribution. The full text of the license may be 
found at
+#  http://opensource.org/licenses/bsd-license.php
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+##
+
+[Defines]
+  INF_VERSION

Re: [edk2] Empty function at BaseCacheMaintenanceLib

2018-05-08 Thread Rafael Machado
Got it!

Thanks for the answer Andrew.

Rafael Machado

Em ter, 8 de mai de 2018 às 12:55, Andrew Fish  escreveu:

> Rafael,
>
> I seem to remember those functions are used to manage the cache on a
> Harvard architecture caches  [1].
>
> If you look at InvalidateInstructionCacheRange() you will notice it is
> used when code gets loaded into memory to keep the data and instruction
> caches coherent. If the instruction cache maintains coherency in hardware
> then there is no need for this functions to do anything.
>
> If you notice the IPF version actually does something, and that is why
> these functions exists.
>
> [1]  https://en.wikipedia.org/wiki/Harvard_architecture
>
> Thanks,
>
> Andrew Fish
>
> On May 8, 2018, at 6:46 AM, Rafael Machado <
> rafaelrodrigues.mach...@gmail.com> wrote:
>
> Hi everyone
>
> I have a question. During a research I got to the BaseCacheMaintenanceLib,
> and noticed that there is a function that is not implemented.
>
> The function InvalidateInstructionCache does not have a body, but as far as
> I
> could check it's used in some places.
>
> Is it ok to have this function empty?
>
> Thanks and Regards
> Rafael R. Machado
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH v1 1/1] BaseTools: incorrect calculation for 16M

2018-05-08 Thread Jaben Carsey
the "0x" was missing.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/GenFds/FvImageSection.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/FvImageSection.py 
b/BaseTools/Source/Python/GenFds/FvImageSection.py
index eb84b44bbec4..57ecea0377bf 100644
--- a/BaseTools/Source/Python/GenFds/FvImageSection.py
+++ b/BaseTools/Source/Python/GenFds/FvImageSection.py
@@ -1,7 +1,7 @@
 ## @file
 # process FV image section generation
 #
-#  Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.
+#  Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
@@ -83,7 +83,7 @@ class FvImageSection(FvImageSectionClassObject):
 if MaxFvAlignment >= 0x400:
 if MaxFvAlignment >= 0x10:
 #The max alignment supported by FFS is 16M.
-if MaxFvAlignment >=100:
+if MaxFvAlignment >= 0x100:
 self.Alignment = "16M"
 else:
 self.Alignment = str(MaxFvAlignment / 0x10) + "M"
-- 
2.16.2.windows.1

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


Re: [edk2] Empty function at BaseCacheMaintenanceLib

2018-05-08 Thread Andrew Fish
Rafael,

I seem to remember those functions are used to manage the cache on a Harvard 
architecture caches  [1].

If you look at InvalidateInstructionCacheRange() you will notice it is used 
when code gets loaded into memory to keep the data and instruction caches 
coherent. If the instruction cache maintains coherency in hardware then there 
is no need for this functions to do anything. 

If you notice the IPF version actually does something, and that is why these 
functions exists. 

[1]  https://en.wikipedia.org/wiki/Harvard_architecture 


Thanks,

Andrew Fish

> On May 8, 2018, at 6:46 AM, Rafael Machado 
>  wrote:
> 
> Hi everyone
> 
> I have a question. During a research I got to the BaseCacheMaintenanceLib,
> and noticed that there is a function that is not implemented.
> 
> The function InvalidateInstructionCache does not have a body, but as far as
> I
> could check it's used in some places.
> 
> Is it ok to have this function empty?
> 
> Thanks and Regards
> Rafael R. Machado
> ___
> 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] Dynamic Pci configuration devices

2018-05-08 Thread Laszlo Ersek
On 05/08/18 16:16, Guy Raviv wrote:
> Hi all,
> 
> currently in
> 
> \Vlv2DeviceRefCodePkg\AcpiTablesPCAT\HOST_BUS.ASL
> 
> The PCI devices are declared statically.
> 
> i want to make this declaration dynamic - so that the device number can change
> 
> according according to my hardware setup. Is it possible?

There are generally two ways for this.

One is to write the bulk of the ASL like seen here, statically, but all
the customizable values are referenced as external objects / fields.
Then, a platform ACPI DXE driver in the firmware computes those values,
and installs a small SSDT with just those objects. The AML is generated
manually by the firmware, which is super awkward, but due to the small
size of the integer objects etc, it is tolerable.

A similar approach can be seen e.g. in "OvmfPkg/AcpiPlatformDxe/Qemu.c",
function QemuInstallAcpiSsdtTable(). And the referring ASL code is in
"OvmfPkg/AcpiTables/Dsdt.asl". (Search both for "FWDT".)

(Note however that said function is not used nowadays on QEMU, because
now QEMU generates *all* of the AML dynamically.)

The other approach is to process the (static) AML before installing it
with EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable(). If you know the exact
path to / structure of the AML node that you want to modify, the
EFI_ACPI_SDT_PROTOCOL lets you navigate to the node, and patch it
in-place, in a memory array. Then you can install the modified table
blob with EFI_ACPI_TABLE_PROTOCOL. (Important: do not modify a table
*after* it is installed.)

One example for the 2nd approach should be
"QuarkPlatformPkg/Acpi/Dxe/AcpiPlatform/AcpiPciUpdate.c".

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


[edk2] Dynamic Pci configuration devices

2018-05-08 Thread Guy Raviv
Hi all,

currently in

\Vlv2DeviceRefCodePkg\AcpiTablesPCAT\HOST_BUS.ASL

The PCI devices are declared statically.

i want to make this declaration dynamic - so that the device number can change

according according to my hardware setup. Is it possible?


Thanks,

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


[edk2] Empty function at BaseCacheMaintenanceLib

2018-05-08 Thread Rafael Machado
Hi everyone

I have a question. During a research I got to the BaseCacheMaintenanceLib,
and noticed that there is a function that is not implemented.

The function InvalidateInstructionCache does not have a body, but as far as
I
could check it's used in some places.

Is it ok to have this function empty?

Thanks and Regards
Rafael R. Machado
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] OVMF Logo

2018-05-08 Thread Richardson, Brian
I'm unclear what you mean by "overlap Windows Logo". Can you provide a 
screenshot?

Thanks : br
---
Brian Richardson, Senior Technical Marketing Engineer, Intel Software
brian.richard...@intel.com -- @intel_brian (Twitter & WeChat)
https://software.intel.com/en-us/meet-the-developers/evangelists/team/brian-richardson
 

-Original Message-
From: edk2-devel  On Behalf Of Laszlo Ersek
Sent: Tuesday, May 8, 2018 6:36 AM
To: Дмитрий Степанов ; edk2-devel@lists.01.org
Subject: Re: [edk2] OVMF Logo

Hi Dmitry,

On 05/05/18 12:36, <8B@89 !B5?0=>2 wrote:
> Hello!
> Is it possible to tune Logo's behavior in OVMF? Now it overlaps 
> Windows Logo during system boot

Hmmm, generally that shouldn't happen; the logo's location is made available to 
windows via the BGRT (boot graphics resource table) ACPI table. When we added 
BGRT support to OVMF [*], the location / appearance of the Windows boot 
animation did change (because it started considering the TianoCore logo 
displayed by OVMF), but I don't recall any actual
*conflict* between the logo & the boot animation.

[*] 6e5e544f227f ("OvmfPkg: Install BGRT ACPI table", 2017-01-06):

> Note from Laszlo Ersek : without the BGRT ACPI 
> table, Windows 8 and Windows 10 first clear the screen, then display a 
> blue, slanted Windows picture above the rotating white boot animation.
> With the BGRT ACPI table, Windows 8 and Windows 10 don't clear the 
> screen, the blue Windows image is not displayed, and the rotating 
> white boot animation is shown between the firmware's original 
> TianoCore boot splash and (optional) "Start boot option" progress bar.

In brief, this shouldn't happen, and if it does, it's not specific to OVMF -- 
can you perhaps provide more information so that the MdeModulePkg maintainers 
might help with the investigation? What Windows release? What boot animation 
exactly? What screen resolution?

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


Re: [edk2] [patch 1/2] BaseTools/VfrCompile:Fix memory leak issues

2018-05-08 Thread Bi, Dandan
Yes. We have submitted patch to fix it. Sorry for the inconvenience.

Thanks,
Dandan

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Gary Lin
Sent: Tuesday, May 8, 2018 5:46 PM
To: Bi, Dandan 
Cc: edk2-devel@lists.01.org; Dong, Eric ; Gao, Liming 

Subject: Re: [edk2] [patch 1/2] BaseTools/VfrCompile:Fix memory leak issues

On Tue, Apr 10, 2018 at 03:54:46PM +0800, Dandan Bi wrote:
> Cc: Eric Dong 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi 
> ---
>  BaseTools/Source/C/VfrCompile/VfrSyntax.g | 32 
> ++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g 
> b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> index d48072a8adf..4b0a43606ea 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> +++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
Hi Dandan,

I encountered a build error with our build service:

[  197s] "VfrCompile" -l -n --string-db 
/home/abuild/rpmbuild/BUILD/ovmf-2018+git1525681922.053cd183c9f2/Build/OvmfX64/DEBUG_GCC5/X64/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib/OUTPUT/BootMaintenanceManagerUiLibStrDefs.hpk
 --output-directory 
/home/abuild/rpmbuild/BUILD/ovmf-2018+git1525681922.053cd183c9f2/Build/OvmfX64/DEBUG_GCC5/X64/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib/DEBUG/.
 
/home/abuild/rpmbuild/BUILD/ovmf-2018+git1525681922.053cd183c9f2/Build/OvmfX64/DEBUG_GCC5/X64/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib/OUTPUT/BootMaintenanceManager.i
[  197s] *** Error in 
`/home/abuild/rpmbuild/BUILD/ovmf-2018+git1525681922.053cd183c9f2/BaseTools/Source/C/bin/VfrCompile':
 free(): invalid pointer: 0xbabababababababa ***

If I reverted the following change, the package can be built again.

> @@ -5055,11 +5082,14 @@ EfiVfrParser::_SAVE_OPHDR_COND (  VOID  
> EfiVfrParser::_CLEAR_SAVED_OPHDR (
>VOID
>)
>  {
> -  mCIfrOpHdr[mCIfrOpHdrIndex]   = NULL;
> +  if (mCIfrOpHdr[mCIfrOpHdrIndex] != NULL) {
> +delete mCIfrOpHdr[mCIfrOpHdrIndex];
> +mCIfrOpHdr[mCIfrOpHdrIndex] = NULL;
> +  }
>mCIfrOpHdrLineNo[mCIfrOpHdrIndex] = 0;  }
>  
>  BOOLEAN
>  EfiVfrParser::_SET_SAVED_OPHDR_SCOPE (

I have no clue now and it happened all the time.

Would you mind to check the code?

Thanks,

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


[edk2] [patch] BaseTools/VfrCompile: Avoid using uninitialized pointer

2018-05-08 Thread Dandan Bi
_CLEAR_SAVED_OPHDR () is used for initialize the variables.
We should not update it to free memory.
It will cause some pointer used before initialization.
This patch is to fix this issue.

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 BaseTools/Source/C/VfrCompile/VfrSyntax.g | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g 
b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
index 4b0a43606ea..cc042ab4307 100644
--- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
+++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
@@ -4103,12 +4103,15 @@ vfrStatementExpression [UINT32 RootLevel, UINT32 
ExpOpCount = 0] :
   }
 }
   }
   
   if ($RootLevel == 0) 
{
-_CLEAR_SAVED_OPHDR 
();
-mCIfrOpHdrIndex --;
+if 
(mCIfrOpHdr[mCIfrOpHdrIndex] != NULL) {
+  delete 
mCIfrOpHdr[mCIfrOpHdrIndex];
+  
mCIfrOpHdr[mCIfrOpHdrIndex] = NULL;
+}
+ mCIfrOpHdrIndex 
--;
   }
>>
   ;
 
 //
@@ -5082,14 +5085,11 @@ EfiVfrParser::_SAVE_OPHDR_COND (
 VOID
 EfiVfrParser::_CLEAR_SAVED_OPHDR (
   VOID
   )
 {
-  if (mCIfrOpHdr[mCIfrOpHdrIndex] != NULL) {
-delete mCIfrOpHdr[mCIfrOpHdrIndex];
-mCIfrOpHdr[mCIfrOpHdrIndex] = NULL;
-  }
+  mCIfrOpHdr[mCIfrOpHdrIndex]   = NULL;
   mCIfrOpHdrLineNo[mCIfrOpHdrIndex] = 0;
 }
 
 BOOLEAN
 EfiVfrParser::_SET_SAVED_OPHDR_SCOPE (
-- 
2.14.3.windows.1

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


Re: [edk2] [PATCH] CryptoPkg/CrtLibSupport: add secure_getenv() stub function

2018-05-08 Thread Laszlo Ersek
On 05/08/18 10:51, Long, Qin wrote:
> It's OK for me to add this NULL wrapper. 
> 
> Reviewed-by: Long Qin 

Thank you very much!

Commit ee3198e672e2.

Laszlo

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com] 
> Sent: Tuesday, May 8, 2018 4:21 AM
> To: edk2-devel-01 
> Cc: Long, Qin ; Ye, Ting 
> Subject: [PATCH] CryptoPkg/CrtLibSupport: add secure_getenv() stub function
> 
> The Fedora distro ships a modified OpenSSL 1.1.0 package stream. One of their 
> patches calls the secure_getenv() C library function. We already have a stub 
> for getenv(); it applies trivially to secure_getenv() as well.
> Add the secure_getenv() stub so that edk2 can be built with Fedora's OpenSSL 
> 1.1.0 sources.
> 
> Cc: Qin Long 
> Cc: Ting Ye 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 
> ---
> 
> Notes:
> Repo:   https://github.com/lersek/edk2.git
> Branch: secure_getenv
> 
>  CryptoPkg/Library/Include/CrtLibSupport.h   |  1 +
>  CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c | 13 +
>  2 files changed, 14 insertions(+)
> 
> diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h 
> b/CryptoPkg/Library/Include/CrtLibSupport.h
> index 7f1ec1230206..feaf58b0c79a 100644
> --- a/CryptoPkg/Library/Include/CrtLibSupport.h
> +++ b/CryptoPkg/Library/Include/CrtLibSupport.h
> @@ -163,6 +163,7 @@ gid_t  getgid  (void);
>  gid_t  getegid (void);
>  void   qsort   (void *, size_t, size_t, int (*)(const void *, 
> const void *));
>  char   *getenv (const char *);
> +char   *secure_getenv (const char *);
>  #if defined(__GNUC__) && (__GNUC__ >= 2)
>  void   abort   (void) __attribute__((__noreturn__));
>  #else
> diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c 
> b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> index 20c96563d270..9510a4a383e6 100644
> --- a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> +++ b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> @@ -361,6 +361,19 @@ char *getenv (const char *varname)
>return NULL;
>  }
>  
> +/* Get a value from the current environment */ char *secure_getenv 
> +(const char *varname) {
> +  //
> +  // Null secure_getenv() function implementation to satisfy the 
> +linker, since
> +  // there is no direct functionality logic dependency in present UEFI cases.
> +  //
> +  // From the secure_getenv() manual: 'just like getenv() except that 
> +it
> +  // returns NULL in cases where "secure execution" is required'.
> +  //
> +  return NULL;
> +}
> +
>  //
>  // -- Stream I/O Routines --
>  //
> --
> 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] OVMF Logo

2018-05-08 Thread Laszlo Ersek
Hi Dmitry,

On 05/05/18 12:36, <8B@89 !B5?0=>2 wrote:
> Hello!
> Is it possible to tune Logo's behavior in OVMF? Now it overlaps
> Windows Logo during system boot

Hmmm, generally that shouldn't happen; the logo's location is made
available to windows via the BGRT (boot graphics resource table) ACPI
table. When we added BGRT support to OVMF [*], the location / appearance
of the Windows boot animation did change (because it started considering
the TianoCore logo displayed by OVMF), but I don't recall any actual
*conflict* between the logo & the boot animation.

[*] 6e5e544f227f ("OvmfPkg: Install BGRT ACPI table", 2017-01-06):

> Note from Laszlo Ersek : without the BGRT ACPI
> table, Windows 8 and Windows 10 first clear the screen, then display a
> blue, slanted Windows picture above the rotating white boot animation.
> With the BGRT ACPI table, Windows 8 and Windows 10 don't clear the
> screen, the blue Windows image is not displayed, and the rotating
> white boot animation is shown between the firmware's original
> TianoCore boot splash and (optional) "Start boot option" progress bar.

In brief, this shouldn't happen, and if it does, it's not specific to
OVMF -- can you perhaps provide more information so that the
MdeModulePkg maintainers might help with the investigation? What Windows
release? What boot animation exactly? What screen resolution?

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


Re: [edk2] [patch 1/2] BaseTools/VfrCompile:Fix memory leak issues

2018-05-08 Thread Gary Lin
On Tue, Apr 10, 2018 at 03:54:46PM +0800, Dandan Bi wrote:
> Cc: Eric Dong 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi 
> ---
>  BaseTools/Source/C/VfrCompile/VfrSyntax.g | 32 
> ++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g 
> b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> index d48072a8adf..4b0a43606ea 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> +++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
Hi Dandan,

I encountered a build error with our build service:

[  197s] "VfrCompile" -l -n --string-db 
/home/abuild/rpmbuild/BUILD/ovmf-2018+git1525681922.053cd183c9f2/Build/OvmfX64/DEBUG_GCC5/X64/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib/OUTPUT/BootMaintenanceManagerUiLibStrDefs.hpk
 --output-directory 
/home/abuild/rpmbuild/BUILD/ovmf-2018+git1525681922.053cd183c9f2/Build/OvmfX64/DEBUG_GCC5/X64/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib/DEBUG/.
 
/home/abuild/rpmbuild/BUILD/ovmf-2018+git1525681922.053cd183c9f2/Build/OvmfX64/DEBUG_GCC5/X64/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib/OUTPUT/BootMaintenanceManager.i
[  197s] *** Error in 
`/home/abuild/rpmbuild/BUILD/ovmf-2018+git1525681922.053cd183c9f2/BaseTools/Source/C/bin/VfrCompile':
 free(): invalid pointer: 0xbabababababababa ***

If I reverted the following change, the package can be built again.

> @@ -5055,11 +5082,14 @@ EfiVfrParser::_SAVE_OPHDR_COND (
>  VOID
>  EfiVfrParser::_CLEAR_SAVED_OPHDR (
>VOID
>)
>  {
> -  mCIfrOpHdr[mCIfrOpHdrIndex]   = NULL;
> +  if (mCIfrOpHdr[mCIfrOpHdrIndex] != NULL) {
> +delete mCIfrOpHdr[mCIfrOpHdrIndex];
> +mCIfrOpHdr[mCIfrOpHdrIndex] = NULL;
> +  }
>mCIfrOpHdrLineNo[mCIfrOpHdrIndex] = 0;
>  }
>  
>  BOOLEAN
>  EfiVfrParser::_SET_SAVED_OPHDR_SCOPE (

I have no clue now and it happened all the time.

Would you mind to check the code?

Thanks,

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


Re: [edk2] [PATCH] CryptoPkg/CrtLibSupport: add secure_getenv() stub function

2018-05-08 Thread Long, Qin
It's OK for me to add this NULL wrapper. 

Reviewed-by: Long Qin 


Best Regards & Thanks,
LONG, Qin

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Tuesday, May 8, 2018 4:21 AM
To: edk2-devel-01 
Cc: Long, Qin ; Ye, Ting 
Subject: [PATCH] CryptoPkg/CrtLibSupport: add secure_getenv() stub function

The Fedora distro ships a modified OpenSSL 1.1.0 package stream. One of their 
patches calls the secure_getenv() C library function. We already have a stub 
for getenv(); it applies trivially to secure_getenv() as well.
Add the secure_getenv() stub so that edk2 can be built with Fedora's OpenSSL 
1.1.0 sources.

Cc: Qin Long 
Cc: Ting Ye 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---

Notes:
Repo:   https://github.com/lersek/edk2.git
Branch: secure_getenv

 CryptoPkg/Library/Include/CrtLibSupport.h   |  1 +
 CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c | 13 +
 2 files changed, 14 insertions(+)

diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h 
b/CryptoPkg/Library/Include/CrtLibSupport.h
index 7f1ec1230206..feaf58b0c79a 100644
--- a/CryptoPkg/Library/Include/CrtLibSupport.h
+++ b/CryptoPkg/Library/Include/CrtLibSupport.h
@@ -163,6 +163,7 @@ gid_t  getgid  (void);
 gid_t  getegid (void);
 void   qsort   (void *, size_t, size_t, int (*)(const void *, 
const void *));
 char   *getenv (const char *);
+char   *secure_getenv (const char *);
 #if defined(__GNUC__) && (__GNUC__ >= 2)
 void   abort   (void) __attribute__((__noreturn__));
 #else
diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c 
b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
index 20c96563d270..9510a4a383e6 100644
--- a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
+++ b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
@@ -361,6 +361,19 @@ char *getenv (const char *varname)
   return NULL;
 }
 
+/* Get a value from the current environment */ char *secure_getenv 
+(const char *varname) {
+  //
+  // Null secure_getenv() function implementation to satisfy the 
+linker, since
+  // there is no direct functionality logic dependency in present UEFI cases.
+  //
+  // From the secure_getenv() manual: 'just like getenv() except that 
+it
+  // returns NULL in cases where "secure execution" is required'.
+  //
+  return NULL;
+}
+
 //
 // -- Stream I/O Routines --
 //
--
2.14.1.3.gb7cf6e02401b

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


Re: [edk2] [Patch 0/3] Enable Pyrite 2.0 for opal driver.

2018-05-08 Thread Yao, Jiewen
Thanks!
Please also add what is "Add supports for pyrite 2.0 spec" as detail as 
possible. 

Thank you
Yao Jiewen

> -Original Message-
> From: Dong, Eric
> Sent: Monday, May 7, 2018 10:50 PM
> To: Yao, Jiewen ; edk2-devel@lists.01.org; Wu, Hao A
> 
> Subject: RE: [edk2] [Patch 0/3] Enable Pyrite 2.0 for opal driver.
> 
> Hi Jiewen,
> 
> I do the basic functionality test base on the opal UI. Also I request the QA 
> to do
> the full regression test and no issue found.
> 
> Also will include unit test info in later patches.  Thanks for your comments.
> 
> Thanks,
> Eric
> 
> -Original Message-
> From: Yao, Jiewen
> Sent: Tuesday, May 8, 2018 1:42 PM
> To: Dong, Eric ; edk2-devel@lists.01.org; Wu, Hao A
> 
> Subject: RE: [edk2] [Patch 0/3] Enable Pyrite 2.0 for opal driver.
> 
> Hi Eric
> Would you please add more detail on what unit test has been done for this
> patch?
> 
> Thank you
> Yao Jiewen
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Eric Dong
> > Sent: Wednesday, May 2, 2018 8:17 PM
> > To: edk2-devel@lists.01.org; Wu, Hao A 
> > Subject: [edk2] [Patch 0/3] Enable Pyrite 2.0 for opal driver.
> >
> > Eanble the pyrite 2.0 devices for opal driver.
> >
> > Eric Dong (3):
> >   MdePkg: Add Feature definitions add in pyrite 2.0 spec.
> >   SecurityPkg/TcgStorageOpalLib: Add supports for pyrite 2.0 spec.
> >   SecurityPkg/OpalPassword: Add support for pyrite 2.0 devices.
> >
> >  MdePkg/Include/IndustryStandard/TcgStorageCore.h   |   2 +
> >  MdePkg/Include/IndustryStandard/TcgStorageOpal.h   |  54 +++
> >  SecurityPkg/Include/Library/TcgStorageOpalLib.h|  41 ++
> >  .../Library/TcgStorageOpalLib/TcgStorageOpalCore.c | 426
> > ++---
> >  .../TcgStorageOpalLib/TcgStorageOpalLib.inf|   1 +
> >  .../TcgStorageOpalLib/TcgStorageOpalLibInternal.h  |  98 +
> > .../Library/TcgStorageOpalLib/TcgStorageOpalUtil.c | 214 ++-
> >  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c |  60 ++-
> >  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h |   9 +
> >  SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c|  84 
> >  .../Tcg/Opal/OpalPassword/OpalPasswordPei.c|   1 +
> >  11 files changed, 934 insertions(+), 56 deletions(-)  create mode
> > 100644
> > SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h
> >
> > --
> > 2.15.0.windows.1
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel