Re: [edk2-devel] [PATCH V2] MdeModulePkg/CapsulePei: Optimize the CapsulePei

2019-05-30 Thread Wu, Hao A
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Gao, Zhichao
> Sent: Friday, May 31, 2019 9:48 AM
> To: devel@edk2.groups.io
> Cc: Bret Barkelew; Wang, Jian J; Wu, Hao A; Ni, Ray; Zeng, Star; Gao, Liming;
> Sean Brogan; Michael Turner; Gao, Zhichao
> Subject: [edk2-devel] [PATCH V2] MdeModulePkg/CapsulePei: Optimize the
> CapsulePei
> 
> From: Bret Barkelew 
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1853
> 
> Optimize some function in CapsulePei to make it easier
> to maintian.

maintian -> maintain

> 1. Separate the capsule check function form GetCapsuleDescriptors
> to AreCapsulesStaged. The original logic is unclear.
> 2. Avoid querying the capsule variable twice. First time to count
> the number of SG list and allocate a buffer to save SG list data.
> Second time to save the SG list data to the buffer. Modified:
> Using a template buffer to save the SG list data. After query,
> we get the number of SG list, then allocate memory and copy
> data form template buffer to the allocated memory.
> 3. Using MemoryAllocationLib instead of memory function in Pei
> services.

Not directly related with this patch.

Now the PEIM has a mixed usage of both the PEI service and memory
allocation library to allocate memory.

Maybe a later patch can unify such usage.

> 4, Remain 2 byte(CHAR16) to be the null-terminate of CapsuleVarName.

Sorry for missing your reply in the V1 patch.

Upon successful return of UnicodeValueToStringS(), the input string buffer
is guaranteed to be null-terminated. I think the origin logic is fine.


One more minor comment below.

> 
> Cc: Jian J Wang 
> Cc: Hao A Wu 
> Cc: Ray Ni 
> Cc: Star Zeng 
> Cc: Liming Gao 
> Cc: Sean Brogan 
> Cc: Michael Turner 
> Cc: Bret Barkelew 
> Signed-off-by: Zhichao gao 
> ---
> 
> Code change from
> https://github.com/microsoft/mu_basecore/blob/release/201903/MdeMod
> ulePkg/Universal/CapsulePei/UefiCapsule.c#L801
> 
> Note:
> 1. Change the AreCapsulesStaged: directly return the BOOLEAN result.
> 2. While the template buffer is to small, double its size through memory
> function.
> 
>  MdeModulePkg/Universal/CapsulePei/Capsule.h   |   3 +-
>  .../Universal/CapsulePei/CapsulePei.inf   |   3 +-
>  .../Universal/CapsulePei/UefiCapsule.c| 355 ++
>  3 files changed, 192 insertions(+), 169 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/CapsulePei/Capsule.h
> b/MdeModulePkg/Universal/CapsulePei/Capsule.h
> index baf40423af..fc20dd8b92 100644
> --- a/MdeModulePkg/Universal/CapsulePei/Capsule.h
> +++ b/MdeModulePkg/Universal/CapsulePei/Capsule.h
> @@ -1,6 +1,6 @@
>  /** @file
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -30,6 +30,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include "Common/CommonHeader.h"
> 
> diff --git a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> index 5d43df3075..9c88b3986f 100644
> --- a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> +++ b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> @@ -6,7 +6,7 @@
>  #  This external input must be validated carefully to avoid security issue 
> like
>  #  buffer overflow, integer overflow.
>  #
> -# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +# Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
>  # Copyright (c) 2017, AMD Incorporated. All rights reserved.
>  #
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -43,6 +43,7 @@
>BaseLib
>HobLib
>BaseMemoryLib
> +  MemoryAllocationLib
>PeiServicesLib
>PeimEntryPoint
>DebugLib
> diff --git a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
> b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
> index e967599e96..b3014478a3 100644
> --- a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
> +++ b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
> @@ -1,7 +1,7 @@
>  /** @file
>Capsule update PEIM for UEFI2.0
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
>  Copyright (c) 2017, AMD Incorporated. All rights reserved.
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -10,6 +10,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  #include "Capsule.h"
> 
> +#define DEFAULT_SG_LIST_HEADS   (20)
> +
>  #ifdef MDE_CPU_IA32
>  //
>  // Global Descriptor Table (GDT)
> @@ -791,30 +793,87 @@ BuildMemoryResourceDescriptor (
>  }
> 
>  /**
> -  Checks for the presence of capsule descriptors.
> -  Get capsule descriptors from variable CapsuleUpdateData,
> CapsuleUpdateData1, CapsuleUpdateData2...
> -  and save to DescriptorBuffer.
> +  Check if the capsules are staged.
> 
> -  @param DescriptorBuffer  

Re: [edk2-devel] edk2-stable201905 features and soft/hard freeze and release date

2019-05-30 Thread Liming Gao
Hi, all
  Now, we are still in soft feature freeze. 201905 stable tag depends on 
openssl 1.1.1 update, openssl 1.1.1 depends on ArmSoftFloatLib update, 
ArmSoftFloatLib depends on the discussion of contribution of code licensed. 
Once the conclusion is made, I will send the mail to announce the hard feature 
freeze. I may reduce the hard feature freeze period so that we can create 
201905 stable tag as early as possible. Thank you for your cooperation and 
patience.

  Below is the status of current dependency. 
 Openssl 1.1.1 patch 
https://edk2.groups.io/g/devel/topic/patch_v5_0_9_crypto/31832279?p=,,,20,0,0,0::recentpostdate%2Fsticky,,,20,2,20,31832279,
 Reviewed-by Wang, Jian
 ArmSoftFloatLib patch 
https://edk2.groups.io/g/devel/message/41517?p=,,,20,0,0,0::Created,,ArmSoftFloatLib,20,2,0,31813765,
  Reviewed-by Leif
 contribution of code licensed 
https://edk2.groups.io/g/devel/message/41639?p=,,,20,0,0,0::Created,,contribution,20,2,0,31823110,
 under discussion. 

Thanks
Liming
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Michael 
> D Kinney
> Sent: Thursday, May 23, 2019 10:08 AM
> To: devel@edk2.groups.io; Kinney, Michael D 
> Cc: Laszlo Ersek  (ler...@redhat.com) ; 
> Andrew Fish (af...@apple.com) ;
> leif.lindh...@linaro.org; Cetola, Stephano 
> Subject: [edk2-devel] edk2-stable201905 features and soft/hard freeze and 
> release date
> 
> Hello,
> 
> There have been a few discussion during the soft freeze for
> edk2-stable201905 on changes that can be accepted or not.
> 
> The TianoCore Stewards met today to discuss these topics and
> make some decisions on the following features.
> 
> * Update OpenSSL version to upcoming 1.1.1
>   https://bugzilla.tianocore.org/show_bug.cgi?id=1089
> 
>   This feature must be completed for edk2-stable201905.
>   We are willing to extend the soft freeze, hard freeze,
>   and release date to make sure this feature is completed.
>   Please complete the patches and reviews and perform all
>   validation required for this feature and provide status
>   to the mailing list.  The release date will be pushed out
>   if required to accommodate this feature.
> 
> * Move network related components from MdeModulePkg to NetworkPkg
>   https://bugzilla.tianocore.org/show_bug.cgi?id=1293
> 
>   Updating MdeModulePkg to remove the extra build of the network
>   modules is considered low risk and should be completed for
>   edk2-stable201905.
> 
> * Standardize EDK II PI root-of-trust verification implementation
>   https://bugzilla.tianocore.org/show_bug.cgi?id=1617
> 
>   Move to edk-stable201908
> 
> * FeatureFlagExpression Support in LibraryClasses section of INF file
>   https://bugzilla.tianocore.org/show_bug.cgi?id=1446
> 
>   Move to edk-stable201908
> 
> * Add new tool chain for LLVM/CLANG8.0
>   https://bugzilla.tianocore.org/show_bug.cgi?id=1603
> 
>   Move to edk-stable201908
> 
> Please let us know if there are any other features that require
> consideration for edk2-stable201905.
> 
> Of course critical bug fixes are accepted during the soft/hard
> freeze.  If there is any doubt if a patch is a bug fix or a
> feature, please ask before doing any commits.
> 
> Thanks,
> 
> Mike
> 
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41709): https://edk2.groups.io/g/devel/message/41709
Mute This Topic: https://groups.io/mt/31727161/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [Patch v2 0/3] [edk2-platforms] Add DebugFeaturePkg to keep debug related modules.

2019-05-30 Thread Liming Gao
This version is good. Reviewed-by: Liming Gao 

>-Original Message-
>From: Dong, Eric
>Sent: Wednesday, May 29, 2019 3:34 PM
>To: devel@edk2.groups.io
>Cc: Gao, Liming 
>Subject: [Patch v2 0/3] [edk2-platforms] Add DebugFeaturePkg to keep
>debug related modules.
>
>Add new package in Platform/Intel/ folder to keep debug related modules.
>
>V2 change:
>  Use one gDebugFeaturePkgTokenSpaceGuid for all PCDs in this package.
>  Make PCD TokenNumber continuous.
>
>Signed-off-by: Eric Dong 
>Cc: Liming Gao 
>
>Eric Dong (3):
>  Platform/Intel/DebugFeaturePkg: Add DebugFeaturePkg
>  Platform/Intel/DebugFeaturePkg: Add USB3DebugPort related modules.
>  Platform/Intel/DebugFeaturePkg/AcpiDebug: Change AcpiDebug module
>location.
>
> Maintainers.txt   |   4 +
> .../AdvancedFeaturePkg/AdvancedFeaturePkg.dec |   6 -
> .../AdvancedFeaturePkg/AdvancedFeaturePkg.dsc |   3 -
> .../AcpiDebug/AcpiDebug.asl   |   0
> .../AcpiDebug/AcpiDebug.c |   0
> .../AcpiDebug/AcpiDebugDxe.inf|   8 +-
> .../AcpiDebug/AcpiDebugSmm.inf|   8 +-
> .../AcpiDebug/Readme.txt  |   0
> .../Intel/DebugFeaturePkg/DebugFeaturePkg.dec |  65 ++
> .../Intel/DebugFeaturePkg/DebugFeaturePkg.dsc |  98 ++
> .../Include/Library/Usb3DebugPortLib.h|  76 ++
> .../Library/Usb3DebugPortParameterLib.h   |  56 ++
> .../Library/Usb3DebugPortLib/MiscServices.c   | 385 
> .../Usb3DebugPortDataTransfer.c   | 892 ++
> .../Usb3DebugPortInitialize.c | 726 ++
> .../Usb3DebugPortLib/Usb3DebugPortLibDxe.c| 454 +
> .../Usb3DebugPortLib/Usb3DebugPortLibDxe.inf  |  55 ++
> .../Usb3DebugPortLibDxeIoMmu.c| 828 
> .../Usb3DebugPortLibDxeIoMmu.inf  |  63 ++
> .../Usb3DebugPortLibInternal.h| 887 +
> .../Usb3DebugPortLib/Usb3DebugPortLibNull.c   | 103 ++
> .../Usb3DebugPortLib/Usb3DebugPortLibNull.inf |  28 +
> .../Usb3DebugPortLib/Usb3DebugPortLibPei.c| 236 +
> .../Usb3DebugPortLib/Usb3DebugPortLibPei.inf  |  48 +
> .../Usb3DebugPortLibPeiIoMmu.c| 440 +
> .../Usb3DebugPortLibPeiIoMmu.inf  |  51 +
> .../Usb3DebugPortParameterLibPcd.c|  58 ++
> .../Usb3DebugPortParameterLibPcd.inf  |  31 +
> 28 files changed, 5592 insertions(+), 17 deletions(-)
> rename Platform/Intel/{AdvancedFeaturePkg =>
>DebugFeaturePkg}/AcpiDebug/AcpiDebug.asl (100%)
> rename Platform/Intel/{AdvancedFeaturePkg =>
>DebugFeaturePkg}/AcpiDebug/AcpiDebug.c (100%)
> rename Platform/Intel/{AdvancedFeaturePkg =>
>DebugFeaturePkg}/AcpiDebug/AcpiDebugDxe.inf (79%)
> rename Platform/Intel/{AdvancedFeaturePkg =>
>DebugFeaturePkg}/AcpiDebug/AcpiDebugSmm.inf (80%)
> rename Platform/Intel/{AdvancedFeaturePkg =>
>DebugFeaturePkg}/AcpiDebug/Readme.txt (100%)
> create mode 100644 Platform/Intel/DebugFeaturePkg/DebugFeaturePkg.dec
> create mode 100644 Platform/Intel/DebugFeaturePkg/DebugFeaturePkg.dsc
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Include/Library/Usb3DebugPortLib.h
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Include/Library/Usb3DebugPortParameterL
>ib.h
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Library/Usb3DebugPortLib/MiscServices.c
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPort
>DataTransfer.c
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPort
>Initialize.c
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPort
>LibDxe.c
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPort
>LibDxe.inf
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPort
>LibDxeIoMmu.c
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPort
>LibDxeIoMmu.inf
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPort
>LibInternal.h
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPort
>LibNull.c
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPort
>LibNull.inf
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPort
>LibPei.c
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPort
>LibPei.inf
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPort
>LibPeiIoMmu.c
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPort
>LibPeiIoMmu.inf
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Library/Usb3DebugPortParameterLibPcd/U
>sb3DebugPortParameterLibPcd.c
> create mode 100644
>Platform/Intel/DebugFeaturePkg/Library/Usb3DebugPortParameterLibPcd/U
>sb3DebugPortParameterLibPcd.inf
>
>--

Re: [edk2-devel] Edk2 BaseTools Patches.

2019-05-30 Thread Bob Feng
Thanks for your comments.

I’ll push the below 2 patches.
[PATCH v3 1/1] BaseTools:Extend the binary cache to support library cache
[PATCH] BaseTools:Update binary cache restore time to current time


I have no strong justification to push this patch in this stable tag. I’ll push 
it after the tag.
[PATCH V5] BaseTools:Make BaseTools support new rules to generate RAW
FFS FILE

I’ll push these 2 patches after the tag also.
[Patch V4 2/2] BaseTools: Refactor hash tracking after checking for Sources 
section
[Patch V4 1/2] BaseTools: Add a checking for Sources section in INF file

Thanks,
Bob
From: Johnson, Michael
Sent: Friday, May 31, 2019 8:03 AM
To: devel@edk2.groups.io; af...@apple.com
Cc: Leif Lindholm ; Feng, Bob C 
; Rodriguez, Christian ; 
Laszlo Ersek ; Kinney, Michael D 
; Gao, Liming ; Shi, Steven 
; Fan, ZhijuX 
Subject: RE: [edk2-devel] Edk2 BaseTools Patches.

Andrew,

What about the force include of AutoGen.h?

AutoGen.h (and .c) have contents which are determined by various metadata like 
PCD values or items listed in the INF.  The sources and dependencies can’t be 
involved, since those aren’t known until after the autogen files are already 
complete.  The build calls genc before genmake.  The hash accounts for those by 
incorporating the metadata itself, rather than the autogenerated files.

If there is a rule the tools should enforce the rule with good error messages.

For the case of the build hash feature, we have an EdkLogger.warn in these 
patches.  Invalidating the hash allows the build to continue with up-to-date 
modules by sending the module back to the regular build process, and the 
message informs the user of what we found.


Since the point of the feature is to speed up builds, I think this is right.  
If we instead stopped the build, when without --hash it would’ve completed 
successfully, then we’ve made a more restricted build that’s less useful, 
rather than giving the existing functionality a speed boost via caching.  I’m 
not against broadening the use of this check to regular builds, but that has 
unanswered questions and its outside the scope of the BZs targeted by these 
patches.  Do we want to check for this condition on every build and log when we 
see it?  Do we want an optional build flag for it?  Should another flag cause a 
halt and give an error, maybe something like “--strict” which could check for 
other spec violating conditions as well?  It turns into a whole feature of its 
own, with considerably higher impact since *many* codebases in the wild have 
non-compliant modules sprinkled throughout.

I think Leif and I are both concerned about having two ways to do something as 
complex as make dependencies, as they risk getting out of phase, or breaking 
different ways (like following the .h rules in the INF File).

I understand the concern.  One positive thing about the overly broad nature of 
hashing all possible legal includes and all compiler flags and all etc etc is 
that we don’t need to carry much understanding or complexity.  We just hash 
‘all the inputs’ and don’t bother looking any deeper.  Further, when the hash 
of a module changes, it drops back to the regular path and the ordinary 
build/skip decision is made exactly as it would be in a regular build.  I think 
this is simple enough to not be (too) troubling.

At some point refactoring the build system from the top might be the right 
approach.

Agreed.  The build tools are critical and could use more attention.  Maybe 
someday…

-Michael

From: devel@edk2.groups.io 
[mailto:devel@edk2.groups.io] On Behalf Of Andrew Fish via Groups.Io
Sent: Thursday, May 30, 2019 3:53 PM
To: devel@edk2.groups.io; Johnson, Michael 
mailto:michael.john...@intel.com>>
Cc: Leif Lindholm mailto:leif.lindh...@linaro.org>>; 
Feng, Bob C mailto:bob.c.f...@intel.com>>; Rodriguez, 
Christian 
mailto:christian.rodrig...@intel.com>>; Laszlo 
Ersek mailto:ler...@redhat.com>>; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>; Gao, Liming 
mailto:liming@intel.com>>; Shi, Steven 
mailto:steven@intel.com>>; Fan, ZhijuX 
mailto:zhijux@intel.com>>
Subject: Re: [edk2-devel] Edk2 BaseTools Patches.


On May 30, 2019, at 2:31 PM, Johnson, Michael 
mailto:michael.john...@intel.com>> wrote:

All,

These patches are not required for the stable tag.  They’re improvements needed 
to enable relatively new build options that are not yet widely used.

That said, let me try to clear the air here about what is happening regarding 
the sources/includes and what changes with these patches.

The INF spec (section 
3.9)
 says that all local source files, including .h files, must be included in the 
sources section.  This means a module is not compliant if it includes a header 
file from a directory other than a package include directory and 

Re: [edk2-devel] [PATCH 2/4] Platform/Intel:Add build parameter to support Binary Cache

2019-05-30 Thread Chiu, Chasel


Reviewed-by: Chasel Chiu 

> -Original Message-
> From: Fan, ZhijuX
> Sent: Friday, May 31, 2019 9:38 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming ; Feng, Bob C ;
> Shi, Steven ; Lu, Shifei A ; 
> Zhou,
> Bowen ; Oram, Isaac W ;
> Chiu, Chasel ; Kubacki, Michael A
> ; Desimone, Nathaniel L
> 
> Subject: [PATCH 2/4] Platform/Intel:Add build parameter to support Binary
> Cache
> 
> BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1784
> BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1785
> 
> Need extend the options in the Intel/build_bios.py file to support Binary 
> Cache.
> 
>  --hash:
>Enable hash-based caching during build process.
>  --binary-destination:
>Generate a cache of binary files in the specified directory.
>  --binary-source:
>Consume a cache of binary files from the specified directory.
> 
> Cc: Liming Gao 
> Cc: Bob Feng 
> Cc: Steven Shi 
> Cc: Shifei A Lu 
> Cc: Xiaohu Zhou 
> Cc: Isaac W Oram 
> Cc: Chasel Chiu 
> Cc: Michael Kubacki 
> Cc: Nate DeSimone 
> Signed-off-by: Zhiju.Fan 
> ---
>  Platform/Intel/build_bios.py | 27 +++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/Platform/Intel/build_bios.py b/Platform/Intel/build_bios.py index
> 9f8d78f6e8..628b127417 100644
> --- a/Platform/Intel/build_bios.py
> +++ b/Platform/Intel/build_bios.py
> @@ -333,6 +333,7 @@ def build(config):
>  print(" SILENT_MODE= ", config.get("SILENT_MODE"))
>  print(" REBUILD_MODE   = ", config.get("REBUILD_MODE"))
>  print(" BUILD_ROM_ONLY = ", config.get("BUILD_ROM_ONLY"))
> +print(" BINARY_CACHE_CMD_LINE = ", config.get("HASH"),
> + config.get("BINARY_CACHE_CMD_LINE"))
>  print("==")
> 
>  command = ["build", "-n", config["NUMBER_OF_PROCESSORS"]] @@
> -343,6 +344,10 @@ def build(config):
>  if config["EXT_BUILD_FLAGS"] and config["EXT_BUILD_FLAGS"] != "":
>  command.append(config["EXT_BUILD_FLAGS"])
> 
> +if config.get('BINARY_CACHE_CMD_LINE'):
> +command.append(config['HASH'])
> +command.append(config['BINARY_CACHE_CMD_LINE'])
> +
>  if config.get("SILENT_MODE", "FALSE") == "TRUE":
>  command.append("--silent")
>  command.append("--quiet")
> @@ -848,6 +853,17 @@ def get_cmd_config_arguments(arguments):
>  if arguments.fspapi is True:
>  result["API_MODE_FSP_WRAPPER_BUILD"] = "TRUE"
> 
> +if not arguments.UseHashCache:
> +result['BINARY_CACHE_CMD_LINE'] = ''
> +elif arguments.BinCacheDest:
> +result['HASH'] = '--hash'
> +result['BINARY_CACHE_CMD_LINE'] = '--binary-destination=%s' %
> arguments.BinCacheDest
> +elif arguments.BinCacheSource:
> +result['HASH'] = '--hash'
> +result['BINARY_CACHE_CMD_LINE'] = '--binary-source=%s' %
> arguments.BinCacheSource
> +else:
> +result['BINARY_CACHE_CMD_LINE'] = ''
> +
>  return result
> 
> 
> @@ -924,6 +940,17 @@ def get_cmd_arguments(build_config):
>  parser.add_argument("--fspapi", help="API mode fsp wrapper build
> enabled",
>  action='store_true', dest="fspapi")
> 
> +parser.add_argument("--hash", action="store_true",
> dest="UseHashCache", default=False,
> +help="Enable hash-based caching during build
> + process.")
> +
> +parser.add_argument("--binary-destination", help="Generate a cache of
> binary \
> +files in the specified directory.",
> +action='store', dest="BinCacheDest")
> +
> +parser.add_argument("--binary-source", help="Consume a cache of binary
> files \
> +from the specified directory.",
> +action='store', dest="BinCacheSource")
> +
>  return parser.parse_args()
> 
> 
> --
> 2.14.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41704): https://edk2.groups.io/g/devel/message/41704
Mute This Topic: https://groups.io/mt/31875862/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH 1/4] Intel/Readme.md:Add instructions about Binary Cache in Readme.md

2019-05-30 Thread Chiu, Chasel


Reviewed-by: Chasel Chiu 

> -Original Message-
> From: Fan, ZhijuX
> Sent: Friday, May 31, 2019 9:37 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming ; Feng, Bob C ;
> Shi, Steven ; Lu, Shifei A ; 
> Zhou,
> Bowen ; Oram, Isaac W ;
> Chiu, Chasel ; Kubacki, Michael A
> ; Desimone, Nathaniel L
> 
> Subject: [PATCH 1/4] Intel/Readme.md:Add instructions about Binary Cache in
> Readme.md
> 
> BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1784
> BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1785
> 
> Add detailed instructions about Binary Cache in Readme.md, Extend options to
> support Binary Cache in the Kabylake build bld.bat file, Purley build bld.bat 
> file,
> build_bios.py
> 
> Cc: Liming Gao 
> Cc: Bob Feng 
> Cc: Steven Shi 
> Cc: Shifei A Lu 
> Cc: Xiaohu Zhou 
> Cc: Isaac W Oram 
> Cc: Chasel Chiu 
> Cc: Michael Kubacki 
> Cc: Nate DeSimone 
> Signed-off-by: Zhiju.Fan 
> ---
>  Platform/Intel/Readme.md | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/Intel/Readme.md b/Platform/Intel/Readme.md index
> 443fb409b3..41ae99e8e6 100644
> --- a/Platform/Intel/Readme.md
> +++ b/Platform/Intel/Readme.md
> @@ -133,6 +133,9 @@ return back to the minimum platform caller.
>| --silent  | silent build enabled|
>| --performance | performance build enabled   |
>| --fsp | fsp build enabled   |
> +  | --hash| Enable hash-based caching   |
> +  | --binary-destination  | create cache in specified directory |
> +  | --binary-source   | Consume cache from directory|
>| |
> 
>  * For more information on build options @@ -191,14 +194,18 @@ For
> KabylakeOpenBoardPkg  2. Type "cd
> edk2-platforms\Platform\Intel\KabylakeOpenBoardPkg\KabylakeRvp3".
>  3. Type "GitEdk2MinKabylake.bat" to setup GIT environment.
>  4. Type "prep" and make prebuild finish for debug build, "prep r" for release
> build.
> -5. Type "bld" to build Kaby Lake reference platform UEFI firmware image.
> +5. Type "bld" to build Kaby Lake reference platform UEFI firmware image, "bld
> cache-produce" Generate a cache of binary
> +   files in the specified directory, "bld cache-consume" Consume a cache of
> binary files from the specified directory,
> +   BINARY_CACHE_PATH is empty, used "BinCache" as default path.
> 
>  For PurleyOpenBoardPkg
>  1. Open command window, go to the workspace directory, e.g. c:\Purley.
>  2. Type "cd
> edk2-platforms\Platform\Intel\PurleyOpenBoardPkg\BoardMtOlympus".
>  3. Type "GitEdk2MinMtOlympus.bat" to setup GIT environment.
>  4. Type "bld" to build Purley Mt Olympus board UEFI firmware image, "bld
> release" for release build, "bld clean" to
> -   remove intermediate files.
> +   remove intermediate files. "bld cache-produce" Generate a cache of binary
> files in the specified directory,
> +   "bld cache-consume" Consume a cache of binary files from the specified
> directory, BINARY_CACHE_PATH is empty,
> +   used "BinCache" as default path.
> 
>  The validated version of iasl compiler that can build MinPurley is 20180629.
> Older version may generate ACPI build  errors.
> --
> 2.14.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41703): https://edk2.groups.io/g/devel/message/41703
Mute This Topic: https://groups.io/mt/31875851/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH V2] MdeModulePkg/CapsulePei: Optimize the CapsulePei

2019-05-30 Thread Gao, Zhichao
From: Bret Barkelew 

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

Optimize some function in CapsulePei to make it easier
to maintian.
1. Separate the capsule check function form GetCapsuleDescriptors
to AreCapsulesStaged. The original logic is unclear.
2. Avoid querying the capsule variable twice. First time to count
the number of SG list and allocate a buffer to save SG list data.
Second time to save the SG list data to the buffer. Modified:
Using a template buffer to save the SG list data. After query,
we get the number of SG list, then allocate memory and copy
data form template buffer to the allocated memory.
3. Using MemoryAllocationLib instead of memory function in Pei
services.
4, Remain 2 byte(CHAR16) to be the null-terminate of CapsuleVarName.

Cc: Jian J Wang 
Cc: Hao A Wu 
Cc: Ray Ni 
Cc: Star Zeng 
Cc: Liming Gao 
Cc: Sean Brogan 
Cc: Michael Turner 
Cc: Bret Barkelew 
Signed-off-by: Zhichao gao 
---

Code change from 
https://github.com/microsoft/mu_basecore/blob/release/201903/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c#L801

Note:
1. Change the AreCapsulesStaged: directly return the BOOLEAN result.
2. While the template buffer is to small, double its size through memory 
function.

 MdeModulePkg/Universal/CapsulePei/Capsule.h   |   3 +-
 .../Universal/CapsulePei/CapsulePei.inf   |   3 +-
 .../Universal/CapsulePei/UefiCapsule.c| 355 ++
 3 files changed, 192 insertions(+), 169 deletions(-)

diff --git a/MdeModulePkg/Universal/CapsulePei/Capsule.h 
b/MdeModulePkg/Universal/CapsulePei/Capsule.h
index baf40423af..fc20dd8b92 100644
--- a/MdeModulePkg/Universal/CapsulePei/Capsule.h
+++ b/MdeModulePkg/Universal/CapsulePei/Capsule.h
@@ -1,6 +1,6 @@
 /** @file
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -30,6 +30,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
+#include 
 #include 
 #include "Common/CommonHeader.h"
 
diff --git a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf 
b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
index 5d43df3075..9c88b3986f 100644
--- a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
+++ b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
@@ -6,7 +6,7 @@
 #  This external input must be validated carefully to avoid security issue like
 #  buffer overflow, integer overflow.
 #
-# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
 # Copyright (c) 2017, AMD Incorporated. All rights reserved.
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -43,6 +43,7 @@
   BaseLib
   HobLib
   BaseMemoryLib
+  MemoryAllocationLib
   PeiServicesLib
   PeimEntryPoint
   DebugLib
diff --git a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c 
b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
index e967599e96..b3014478a3 100644
--- a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
+++ b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
@@ -1,7 +1,7 @@
 /** @file
   Capsule update PEIM for UEFI2.0
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
 Copyright (c) 2017, AMD Incorporated. All rights reserved.
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -10,6 +10,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include "Capsule.h"
 
+#define DEFAULT_SG_LIST_HEADS   (20)
+
 #ifdef MDE_CPU_IA32
 //
 // Global Descriptor Table (GDT)
@@ -791,30 +793,87 @@ BuildMemoryResourceDescriptor (
 }
 
 /**
-  Checks for the presence of capsule descriptors.
-  Get capsule descriptors from variable CapsuleUpdateData, CapsuleUpdateData1, 
CapsuleUpdateData2...
-  and save to DescriptorBuffer.
+  Check if the capsules are staged.
 
-  @param DescriptorBufferPointer to the capsule descriptors
+  @retval TRUE  The capsules are staged.
+  @retval FALSE The capsules are not staged.
+
+**/
+BOOLEAN
+AreCapsulesStaged (
+  VOID
+  )
+{
+  EFI_STATUSStatus;
+  UINTN Size;
+  EFI_PEI_READ_ONLY_VARIABLE2_PPI   *PPIVariableServices;
+  EFI_PHYSICAL_ADDRESS  CapsuleDataPtr64;
+
+  CapsuleDataPtr64 = 0;
+
+  Status = PeiServicesLocatePpi (
+,
+0,
+NULL,
+(VOID **)
+);
+
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Failed to find ReadOnlyVariable2PPI\n"));
+return FALSE;
+  }
+
+  //
+  // Check for Update capsule
+  //
+  Size = sizeof (CapsuleDataPtr64);
+  Status = PPIVariableServices->GetVariable(
+  PPIVariableServices,
+  EFI_CAPSULE_VARIABLE_NAME,
+  ,
+  NULL,
+  ,
+ 

Re: [edk2-devel] [PATCH] MdeModulePkg/CapsulePei: Optimize the CapsulePei

2019-05-30 Thread Gao, Zhichao


> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: Wednesday, May 29, 2019 11:09 PM
> To: devel@edk2.groups.io; Gao, Zhichao 
> Cc: Bret Barkelew ; Wang, Jian J
> ; Wu, Hao A ; Ni, Ray
> ; Zeng, Star ; Gao, Liming
> ; Sean Brogan ;
> Michael Turner 
> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/CapsulePei: Optimize the
> CapsulePei
> 
> On Wed, May 29, 2019 at 03:01:12PM +, Gao, Zhichao wrote:
> > I just update the date of copyright. And the code in Mu project didn't add
> its own copyright.
> > If it is required, I would add it for them.
> 
> Well, hopefully Microsoft will add their own copyright to the original
> :)
> 
> Although it would certainly be better to add it here as well anyway.

I think it is better to let MS to add the copyright by themselves.

> 
> So what modifications were made to the code on the way from the project
> Mu repository? That would be useful to mention in the commit message.

I would add this info blow commit message(not in commit message). It is helpful 
for review. But it may not be useful to add them in the commit message.
On my opinion, the commit message should contain the summary and impact of the 
changes.

Thanks,
Zhichao

> 
> Regards,
> 
> Leif
> 
> > And I also make some minor changes on it.
> >
> > Thanks,
> > Zhichao
> >
> > > -Original Message-
> > > From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> > > Sent: Wednesday, May 29, 2019 7:12 PM
> > > To: devel@edk2.groups.io; Gao, Zhichao 
> > > Cc: Bret Barkelew ; Wang, Jian J
> > > ; Wu, Hao A ; Ni, Ray
> > > ; Zeng, Star ; Gao, Liming
> > > ; Sean Brogan ;
> > > Michael Turner 
> > > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/CapsulePei: Optimize
> > > the CapsulePei
> > >
> > > On Wed, May 29, 2019 at 08:45:55AM +0800, Gao, Zhichao wrote:
> > > > From: Bret Barkelew 
> > >
> > > If this code is from Microsoft...
> > >
> > > >
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1853
> > > >
> > > > Sperate the capsule check function from GetCapsuleDescriptors and
> > > > name it to AreCapsulesStaged.
> > > > Rename GetCapsuleDescriptors to GetScatterGatherHeadEntries.
> > > > And optimize its to remove the duplicated code.
> > > >
> > > > Cc: Jian J Wang 
> > > > Cc: Hao A Wu 
> > > > Cc: Ray Ni 
> > > > Cc: Star Zeng 
> > > > Cc: Liming Gao 
> > > > Cc: Sean Brogan 
> > > > Cc: Michael Turner 
> > > > Cc: Bret Barkelew 
> > > > Signed-off-by: Zhichao gao 
> > > > ---
> > > >  MdeModulePkg/Universal/CapsulePei/Capsule.h   |   3 +-
> > > >  .../Universal/CapsulePei/CapsulePei.inf   |   3 +-
> > > >  .../Universal/CapsulePei/UefiCapsule.c| 357 ++
> > > >  3 files changed, 194 insertions(+), 169 deletions(-)
> > > >
> > > > diff --git a/MdeModulePkg/Universal/CapsulePei/Capsule.h
> > > > b/MdeModulePkg/Universal/CapsulePei/Capsule.h
> > > > index baf40423af..fc20dd8b92 100644
> > > > --- a/MdeModulePkg/Universal/CapsulePei/Capsule.h
> > > > +++ b/MdeModulePkg/Universal/CapsulePei/Capsule.h
> > > > @@ -1,6 +1,6 @@
> > > >  /** @file
> > > >
> > > > -Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > > > reserved.
> > > > +Copyright (c) 2006 - 2019, Intel Corporation. All rights
> > > > +reserved.
> > > >
> > > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > > @@ -30,6 +30,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > #include   #include
> > > >  #include 
> > > > +#include 
> > > >  #include   #include
> > > > "Common/CommonHeader.h"
> > > >
> > > > diff --git a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> > > > b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> > > > index 5d43df3075..9c88b3986f 100644
> > > > --- a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> > > > +++ b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
> > > > @@ -6,7 +6,7 @@
> > > >  #  This external input must be validated carefully to avoid
> > > > security issue like  #  buffer overflow, integer overflow.
> > > >  #
> > > > -# Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > > > reserved.
> > > > +# Copyright (c) 2006 - 2019, Intel Corporation. All rights
> > > > +reserved.
> > >
> > > ...why does Intel get the copyright?
> > >
> > > /
> > > Leif
> >
> > 
> >

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41701): https://edk2.groups.io/g/devel/message/41701
Mute This Topic: https://groups.io/mt/31828852/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH 4/4] PurleyOpenBoardPkg:Extend options in bld.bat to support Binary Cache

2019-05-30 Thread Fan, ZhijuX
BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1785

Need extend the options in the Kabylake build bld.bat file to
support Binary Cache.

BINARY_CACHE_PATH:
if BINARY_CACHE_PATH is empty, use BinCache as default path

Add "cache-produce" and "cache-consume" to command line,
Used to generate and use Binary Cache files.

Cc: Liming Gao 
Cc: Bob Feng 
Cc: Steven Shi 
Cc: Shifei A Lu 
Cc: Xiaohu Zhou 
Cc: Isaac W Oram 
---
 Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/bld.bat  | 21 
+++--
 Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/prebuild.bat |  4 ++--
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/bld.bat 
b/Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/bld.bat
index f624be03a9..f02472239d 100644
--- a/Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/bld.bat
+++ b/Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/bld.bat
@@ -10,6 +10,11 @@ REM Run setlocal to take a snapshot of the environment 
variables.  endlocal is c
 setlocal
 set SCRIPT_ERROR=0
 
+@if not defined BINARY_CACHE_PATH (
+  echo Info: BINARY_CACHE_PATH is empty, use BinCache as default
+  SET BINARY_CACHE_PATH=BinCache
+)
+
 REM  Do NOT use :: for comments Inside of code blocks() 
 
 ::**
@@ -28,6 +33,18 @@ if /I "%1"=="clean" (
   goto :EOF
 )
 
+@if "%~1" == "cache-produce" (
+  SET BINARY_CACHE_CMD_LINE= --hash --binary-destination=%BINARY_CACHE_PATH%
+  shift
+  goto BUILD_FLAGS_LOOP
+)
+
+@if "%~1" == "cache-consume" (
+  SET BINARY_CACHE_CMD_LINE= --hash --binary-source=%BINARY_CACHE_PATH%
+  shift
+  goto BUILD_FLAGS_LOOP
+)
+
 shift
 GOTO :parseCmdLine
 
@@ -86,8 +103,8 @@ echo  Build Start
 echo.
 echo 
 echo.
-echo build %BUILD_CMD_LINE% --log=%BUILD_LOG% %BUILD_REPORT_FLAGS%
-call build %BUILD_CMD_LINE% --log=%BUILD_LOG% %BUILD_REPORT_FLAGS%
+echo build %BUILD_CMD_LINE% --log=%BUILD_LOG% %BUILD_REPORT_FLAGS% 
%BINARY_CACHE_CMD_LINE%
+call build %BUILD_CMD_LINE% --log=%BUILD_LOG% %BUILD_REPORT_FLAGS% 
%BINARY_CACHE_CMD_LINE%
 echo 
 echo.
 echo  Build End
diff --git a/Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/prebuild.bat 
b/Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/prebuild.bat
index d9c1442ea1..4aba30adce 100644
--- a/Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/prebuild.bat
+++ b/Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/prebuild.bat
@@ -182,8 +182,8 @@ set PRE_BUILD_CMD_LINE=%BUILD_CMD_LINE% -D 
MAX_SOCKET=%MAX_SOCKET%
 set PRE_BUILD_LOG=%WORKSPACE%\Build\prebuild.log
 set PRE_BUILD_REPORT=%WORKSPACE%\Build\preBuildReport.txt
 
-echo build %PRE_BUILD_CMD_LINE% -m %BOARD_PKG%\Acpi\BoardAcpiDxe\Dsdt.inf -y 
%PRE_BUILD_REPORT% --log=%PRE_BUILD_LOG%
-call build %PRE_BUILD_CMD_LINE% -m %BOARD_PKG%\Acpi\BoardAcpiDxe\Dsdt.inf -y 
%PRE_BUILD_REPORT% --log=%PRE_BUILD_LOG%
+echo build %PRE_BUILD_CMD_LINE% -m %BOARD_PKG%\Acpi\BoardAcpiDxe\Dsdt.inf -y 
%PRE_BUILD_REPORT% --log=%PRE_BUILD_LOG% %BINARY_CACHE_CMD_LINE%
+call build %PRE_BUILD_CMD_LINE% -m %BOARD_PKG%\Acpi\BoardAcpiDxe\Dsdt.inf -y 
%PRE_BUILD_REPORT% --log=%PRE_BUILD_LOG% %BINARY_CACHE_CMD_LINE%
 if %ERRORLEVEL% NEQ 0 EXIT /b %ERRORLEVEL%
 
 @REM PSYS == FIX0
-- 
2.14.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41700): https://edk2.groups.io/g/devel/message/41700
Mute This Topic: https://groups.io/mt/31875898/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

<>

[edk2-devel] [PATCH 3/4] KabylakeOpenBoardPkg:Extend options in bld.bat to support Binary Cache

2019-05-30 Thread Fan, ZhijuX
BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1784

Need extend the options in the Kabylake build bld.bat file to
support Binary Cache.

BINARY_CACHE_PATH:
if BINARY_CACHE_PATH is empty, use BinCache as default path

Add "cache-produce" and "cache-consume" to command line,
Used to generate and use Binary Cache files.

Cc: Liming Gao 
Cc: Bob Feng 
Cc: Steven Shi 
Cc: Chasel Chiu 
Cc: Michael Kubacki 
Cc: Nate DeSimone 
---
 Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/bld.bat | 23 
+--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/bld.bat 
b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/bld.bat
index 449660b75d..af397a3115 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/bld.bat
+++ b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/bld.bat
@@ -62,6 +62,11 @@ copy /y /b 
%WORKSPACE_FSP_BIN%\KabylakeFspBinPkg\Fsp_Rebased_S.fd+%WORKSPACE_FSP
 @SET REBUILD_MODE=
 @SET BUILD_ROM_ONLY=
 
+@if not defined BINARY_CACHE_PATH (
+  echo Info: BINARY_CACHE_PATH is empty, use BinCache as default
+  SET BINARY_CACHE_PATH=BinCache
+)
+
 :: Loop through arguements until all are processed
 
 :BUILD_FLAGS_LOOP
@@ -87,6 +92,19 @@ copy /y /b 
%WORKSPACE_FSP_BIN%\KabylakeFspBinPkg\Fsp_Rebased_S.fd+%WORKSPACE_FSP
   shift
   goto BUILD_FLAGS_LOOP
 )
+
+@if "%~1" == "cache-produce" (
+  SET BINARY_CACHE_CMD_LINE= --hash --binary-destination=%BINARY_CACHE_PATH%
+  shift
+  goto BUILD_FLAGS_LOOP
+)
+
+@if "%~1" == "cache-consume" (
+  SET BINARY_CACHE_CMD_LINE= --hash --binary-source=%BINARY_CACHE_PATH%
+  shift
+  goto BUILD_FLAGS_LOOP
+)
+
 :: Unknown build flag.
 shift
 goto BUILD_FLAGS_LOOP
@@ -99,11 +117,12 @@ goto BUILD_FLAGS_LOOP
 @echoSILENT_MODE = %SILENT_MODE%
 @echoREBUILD_MODE = %REBUILD_MODE%
 @echoBUILD_ROM_ONLY = %BUILD_ROM_ONLY%
+@echoBINARY_CACHE_CMD_LINE = %BINARY_CACHE_CMD_LINE%
 @echo.
 
 @if %SILENT_MODE% EQU TRUE goto BldSilent
 
-call build -n %NUMBER_OF_PROCESSORS% %REBUILD_MODE% %EXT_BUILD_FLAGS%
+call build -n %NUMBER_OF_PROCESSORS% %REBUILD_MODE% %EXT_BUILD_FLAGS% 
%BINARY_CACHE_CMD_LINE%
 
 @if %ERRORLEVEL% NEQ 0 goto BldFail
 @echo.
@@ -122,7 +141,7 @@ call %WORKSPACE_PLATFORM%\%PROJECT%\postbuild.bat 
%BUILD_ROM_ONLY%
 @echo  
>> Build.log
 @echo. >> Build.log
 
-call build -n %NUMBER_OF_PROCESSORS% %REBUILD_MODE% %EXT_BUILD_FLAGS% 
1>>Build.log 2>&1
+call build -n %NUMBER_OF_PROCESSORS% %REBUILD_MODE% %EXT_BUILD_FLAGS% 
%BINARY_CACHE_CMD_LINE% 1>>Build.log 2>&1
 
 @if %ERRORLEVEL% NEQ 0 goto BldFail
 @echo. >> Build.log
-- 
2.14.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41699): https://edk2.groups.io/g/devel/message/41699
Mute This Topic: https://groups.io/mt/31875886/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

<>

[edk2-devel] [PATCH 2/4] Platform/Intel:Add build parameter to support Binary Cache

2019-05-30 Thread Fan, ZhijuX
BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1784
BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1785

Need extend the options in the Intel/build_bios.py file
to support Binary Cache.

 --hash:
   Enable hash-based caching during build process.
 --binary-destination:
   Generate a cache of binary files in the specified directory.
 --binary-source:
   Consume a cache of binary files from the specified directory.

Cc: Liming Gao 
Cc: Bob Feng 
Cc: Steven Shi 
Cc: Shifei A Lu 
Cc: Xiaohu Zhou 
Cc: Isaac W Oram 
Cc: Chasel Chiu 
Cc: Michael Kubacki 
Cc: Nate DeSimone 
Signed-off-by: Zhiju.Fan 
---
 Platform/Intel/build_bios.py | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/Platform/Intel/build_bios.py b/Platform/Intel/build_bios.py
index 9f8d78f6e8..628b127417 100644
--- a/Platform/Intel/build_bios.py
+++ b/Platform/Intel/build_bios.py
@@ -333,6 +333,7 @@ def build(config):
 print(" SILENT_MODE= ", config.get("SILENT_MODE"))
 print(" REBUILD_MODE   = ", config.get("REBUILD_MODE"))
 print(" BUILD_ROM_ONLY = ", config.get("BUILD_ROM_ONLY"))
+print(" BINARY_CACHE_CMD_LINE = ", config.get("HASH"), 
config.get("BINARY_CACHE_CMD_LINE"))
 print("==")
 
 command = ["build", "-n", config["NUMBER_OF_PROCESSORS"]]
@@ -343,6 +344,10 @@ def build(config):
 if config["EXT_BUILD_FLAGS"] and config["EXT_BUILD_FLAGS"] != "":
 command.append(config["EXT_BUILD_FLAGS"])
 
+if config.get('BINARY_CACHE_CMD_LINE'):
+command.append(config['HASH'])
+command.append(config['BINARY_CACHE_CMD_LINE'])
+
 if config.get("SILENT_MODE", "FALSE") == "TRUE":
 command.append("--silent")
 command.append("--quiet")
@@ -848,6 +853,17 @@ def get_cmd_config_arguments(arguments):
 if arguments.fspapi is True:
 result["API_MODE_FSP_WRAPPER_BUILD"] = "TRUE"
 
+if not arguments.UseHashCache:
+result['BINARY_CACHE_CMD_LINE'] = ''
+elif arguments.BinCacheDest:
+result['HASH'] = '--hash'
+result['BINARY_CACHE_CMD_LINE'] = '--binary-destination=%s' % 
arguments.BinCacheDest
+elif arguments.BinCacheSource:
+result['HASH'] = '--hash'
+result['BINARY_CACHE_CMD_LINE'] = '--binary-source=%s' % 
arguments.BinCacheSource
+else:
+result['BINARY_CACHE_CMD_LINE'] = ''
+
 return result
 
 
@@ -924,6 +940,17 @@ def get_cmd_arguments(build_config):
 parser.add_argument("--fspapi", help="API mode fsp wrapper build enabled",
 action='store_true', dest="fspapi")
 
+parser.add_argument("--hash", action="store_true", dest="UseHashCache", 
default=False,
+help="Enable hash-based caching during build process.")
+
+parser.add_argument("--binary-destination", help="Generate a cache of 
binary \
+files in the specified directory.",
+action='store', dest="BinCacheDest")
+
+parser.add_argument("--binary-source", help="Consume a cache of binary 
files \
+from the specified directory.",
+action='store', dest="BinCacheSource")
+
 return parser.parse_args()
 
 
-- 
2.14.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41698): https://edk2.groups.io/g/devel/message/41698
Mute This Topic: https://groups.io/mt/31875862/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

<>

[edk2-devel] [PATCH 1/4] Intel/Readme.md:Add instructions about Binary Cache in Readme.md

2019-05-30 Thread Fan, ZhijuX
BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1784
BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1785

Add detailed instructions about Binary Cache in Readme.md,
Extend options to support Binary Cache in the Kabylake
build bld.bat file, Purley build bld.bat file, build_bios.py

Cc: Liming Gao 
Cc: Bob Feng 
Cc: Steven Shi 
Cc: Shifei A Lu 
Cc: Xiaohu Zhou 
Cc: Isaac W Oram 
Cc: Chasel Chiu 
Cc: Michael Kubacki 
Cc: Nate DeSimone 
Signed-off-by: Zhiju.Fan 
---
 Platform/Intel/Readme.md | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/Platform/Intel/Readme.md b/Platform/Intel/Readme.md
index 443fb409b3..41ae99e8e6 100644
--- a/Platform/Intel/Readme.md
+++ b/Platform/Intel/Readme.md
@@ -133,6 +133,9 @@ return back to the minimum platform caller.
   | --silent  | silent build enabled|
   | --performance | performance build enabled   |
   | --fsp | fsp build enabled   |
+  | --hash| Enable hash-based caching   |
+  | --binary-destination  | create cache in specified directory |
+  | --binary-source   | Consume cache from directory|
   | |
 
 * For more information on build options
@@ -191,14 +194,18 @@ For KabylakeOpenBoardPkg
 2. Type "cd edk2-platforms\Platform\Intel\KabylakeOpenBoardPkg\KabylakeRvp3".
 3. Type "GitEdk2MinKabylake.bat" to setup GIT environment.
 4. Type "prep" and make prebuild finish for debug build, "prep r" for release 
build.
-5. Type "bld" to build Kaby Lake reference platform UEFI firmware image.
+5. Type "bld" to build Kaby Lake reference platform UEFI firmware image, "bld 
cache-produce" Generate a cache of binary
+   files in the specified directory, "bld cache-consume" Consume a cache of 
binary files from the specified directory,
+   BINARY_CACHE_PATH is empty, used "BinCache" as default path.

 For PurleyOpenBoardPkg
 1. Open command window, go to the workspace directory, e.g. c:\Purley.
 2. Type "cd edk2-platforms\Platform\Intel\PurleyOpenBoardPkg\BoardMtOlympus".
 3. Type "GitEdk2MinMtOlympus.bat" to setup GIT environment.
 4. Type "bld" to build Purley Mt Olympus board UEFI firmware image, "bld 
release" for release build, "bld clean" to
-   remove intermediate files.
+   remove intermediate files. "bld cache-produce" Generate a cache of binary 
files in the specified directory,
+   "bld cache-consume" Consume a cache of binary files from the specified 
directory, BINARY_CACHE_PATH is empty,
+   used "BinCache" as default path. 
 
 The validated version of iasl compiler that can build MinPurley is 20180629. 
Older version may generate ACPI build
 errors.
-- 
2.14.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41697): https://edk2.groups.io/g/devel/message/41697
Mute This Topic: https://groups.io/mt/31875851/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

<>

[edk2-devel] [PATCH 0/4] Makes binary cache available in Platform/Intel

2019-05-30 Thread Fan, ZhijuX
Add detailed instructions about Binary Cache in Readme.md,
Extend options to support Binary Cache in the Kabylake
build bld.bat file, Purley build bld.bat file, build_bios.py

Cc: Liming Gao 
Cc: Bob Feng 
Cc: Steven Shi 
Cc: Shifei A Lu 
Cc: Xiaohu Zhou 
Cc: Isaac W Oram 
Cc: Chasel Chiu 
Cc: Michael Kubacki 
Cc: Nate DeSimone 
Signed-off-by: Zhiju.Fan 

Fan, Zhiju (4):
  Intel/Readme.md:Add detailed instructions about Binary Cache in
Readme.md
  Platform/Intel:Add build parameter to support Binary Cache
  KabylakeOpenBoardPkg:Extend options in bld.bat to support Binary Cache
  PurleyOpenBoardPkg:Extend options in bld.bat to support Binary Cache

 Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/bld.bat  | 23 
+--
 Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/bld.bat  | 21 
+++--
 Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/prebuild.bat |  4 ++--
 Platform/Intel/Readme.md  | 11 +--
 Platform/Intel/build_bios.py  | 27 
+++
 5 files changed, 78 insertions(+), 8 deletions(-)

-- 
2.14.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41696): https://edk2.groups.io/g/devel/message/41696
Mute This Topic: https://groups.io/mt/31875841/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

<>

Re: [edk2-devel] Edk2 BaseTools Patches.

2019-05-30 Thread Johnson, Michael
Andrew,

What about the force include of AutoGen.h?

AutoGen.h (and .c) have contents which are determined by various metadata like 
PCD values or items listed in the INF.  The sources and dependencies can’t be 
involved, since those aren’t known until after the autogen files are already 
complete.  The build calls genc before genmake.  The hash accounts for those by 
incorporating the metadata itself, rather than the autogenerated files.

If there is a rule the tools should enforce the rule with good error messages.

For the case of the build hash feature, we have an EdkLogger.warn in these 
patches.  Invalidating the hash allows the build to continue with up-to-date 
modules by sending the module back to the regular build process, and the 
message informs the user of what we found.


Since the point of the feature is to speed up builds, I think this is right.  
If we instead stopped the build, when without --hash it would’ve completed 
successfully, then we’ve made a more restricted build that’s less useful, 
rather than giving the existing functionality a speed boost via caching.  I’m 
not against broadening the use of this check to regular builds, but that has 
unanswered questions and its outside the scope of the BZs targeted by these 
patches.  Do we want to check for this condition on every build and log when we 
see it?  Do we want an optional build flag for it?  Should another flag cause a 
halt and give an error, maybe something like “--strict” which could check for 
other spec violating conditions as well?  It turns into a whole feature of its 
own, with considerably higher impact since *many* codebases in the wild have 
non-compliant modules sprinkled throughout.

I think Leif and I are both concerned about having two ways to do something as 
complex as make dependencies, as they risk getting out of phase, or breaking 
different ways (like following the .h rules in the INF File).

I understand the concern.  One positive thing about the overly broad nature of 
hashing all possible legal includes and all compiler flags and all etc etc is 
that we don’t need to carry much understanding or complexity.  We just hash 
‘all the inputs’ and don’t bother looking any deeper.  Further, when the hash 
of a module changes, it drops back to the regular path and the ordinary 
build/skip decision is made exactly as it would be in a regular build.  I think 
this is simple enough to not be (too) troubling.

At some point refactoring the build system from the top might be the right 
approach.

Agreed.  The build tools are critical and could use more attention.  Maybe 
someday…

-Michael

From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Andrew 
Fish via Groups.Io
Sent: Thursday, May 30, 2019 3:53 PM
To: devel@edk2.groups.io; Johnson, Michael 
Cc: Leif Lindholm ; Feng, Bob C 
; Rodriguez, Christian ; 
Laszlo Ersek ; Kinney, Michael D 
; Gao, Liming ; Shi, Steven 
; Fan, ZhijuX 
Subject: Re: [edk2-devel] Edk2 BaseTools Patches.


On May 30, 2019, at 2:31 PM, Johnson, Michael 
mailto:michael.john...@intel.com>> wrote:

All,

These patches are not required for the stable tag.  They’re improvements needed 
to enable relatively new build options that are not yet widely used.

That said, let me try to clear the air here about what is happening regarding 
the sources/includes and what changes with these patches.

The INF spec (section 
3.9)
 says that all local source files, including .h files, must be included in the 
sources section.  This means a module is not compliant if it includes a header 
file from a directory other than a package include directory and fails to list 
it in its sources section.  We’re already generating a hash which is guaranteed 
to change whenever the module’s source changes - without invoking mkdep - by 
hashing each file from the sources section as well as *all* the contents of 
every include folder belonging to each package that the module is dependent on.

Every possible ‘legally’ included header will change the hash,

Michael,

What about the force include of AutoGen.h?


but the hashes of non-compliant modules will not change when their ‘illegally’ 
included headers change and we will incorrectly re-use stale cached binaries.  
To prevent this, the below patches check for compliance and invalidate the hash 
of any non-compliant module.  In this way, non-compliance is neither supported 
nor allowed to poison the cache.


If there is a rule the tools should enforce the rule with good error messages.


Again, since this only has an effect on builds which have enabled this 
relatively new feature, I don’t expect any production impact if the stable tag 
doesn’t take these patches.  Nobody is using it yet.


I think Leif and I are both concerned about having two ways to do something as 
complex as make dependencies, as they risk getting out 

Re: [edk2-devel] Upcoming Event: TianoCore Bug Triage - APAC / NAMO - Thu, 05/30/2019 5:00pm-5:30pm #cal-reminder

2019-05-30 Thread rebecca

On 5/30/19 5:45 PM, devel@edk2.groups.io Calendar wrote:


*Reminder:* TianoCore Bug Triage - APAC / NAMO

*When:* Thursday, 30 May 2019, 5:00pm to 5:30pm, (GMT-07:00) 
America/Los Angeles


*Where:*https://zoom.us/j/769108409

View Event 

*Organizer:* Stephano Cetola stephano.cet...@intel.com 
 



*Description:*

Join Zoom Meeting

https://zoom.us/j/769108409



I'm getting a message that the meeting hasn't started. And "Please wait 
for the host to start this meeting" in the Zoom app. Is this the correct 
link?



--
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41694): https://edk2.groups.io/g/devel/message/41694
Mute This Topic: https://groups.io/mt/31875040/21656
Mute #cal-reminder: https://groups.io/mk?hashtag=cal-reminder=3846945
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] Cancelled Event: TianoCore Design Meeting - APAC/NAMO - Thursday, 30 May 2019 #cal-cancelled

2019-05-30 Thread devel@edk2.groups.io Calendar
BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Groups.io Inc//Groups.io Calendar//EN
METHOD:CANCEL
CALSCALE:GREGORIAN
BEGIN:VEVENT
STATUS:CANCELLED
UID:calendar.14...@groups.io
DTSTAMP:20190530T234747Z
ORGANIZER;CN=Stephano Cetola:mailto:stephano.cet...@intel.com
DTSTART;TZID=America/Los_Angeles:20190530T183000
DTEND;TZID=America/Los_Angeles:20190530T193000
SUMMARY:TianoCore Design Meeting - APAC/NAMO
DESCRIPTION:Join Zoom Meeting\n\nhttps://zoom.us/j/969264410 ( https://zoom.us/j/969264410 )\n\nOne tap mobile\n\n+16465588656,,969264410# US (New York)\n\n+17207072699,,969264410# US\n\nDial by your location\n\n+1 646 558 8656 US (New York)\n\n+1 720 707 2699 US\n\nMeeting ID: 969 264 410\n\nFind your local number: https://zoom.us/u/abOtdJckxL
LOCATION:https://zoom.us/j/969264410
RECURRENCE-ID;TZID=America/Los_Angeles:20190530T183000
SEQUENCE:0
END:VEVENT
END:VCALENDAR


invite.ics
Description: application/ics


[edk2-devel] Upcoming Event: TianoCore Bug Triage - APAC / NAMO - Thu, 05/30/2019 5:00pm-5:30pm #cal-reminder

2019-05-30 Thread devel@edk2.groups.io Calendar
*Reminder:* TianoCore Bug Triage - APAC / NAMO

*When:* Thursday, 30 May 2019, 5:00pm to 5:30pm, (GMT-07:00) America/Los Angeles

*Where:* https://zoom.us/j/769108409

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=458023 )

*Organizer:* Stephano Cetola stephano.cet...@intel.com ( 
stephano.cet...@intel.com?subject=Re:%20Event:%20TianoCore%20Bug%20Triage%20-%20APAC%20%2F%20NAMO
 )

*Description:*

Join Zoom Meeting

https://zoom.us/j/769108409 ( https://zoom.us/j/769108409 )

One tap mobile

+17207072699,,769108409# US

+16465588656,,769108409# US (New York)

Dial by your location

+1 720 707 2699 US

+1 646 558 8656 US (New York)

Meeting ID: 769 108 409

Find your local number: https://zoom.us/u/abOtdJckxL

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41692): https://edk2.groups.io/g/devel/message/41692
Mute This Topic: https://groups.io/mt/31875040/21656
Mute #cal-reminder: https://groups.io/mk?hashtag=cal-reminder=3846945
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] UefiPayloadPkg: Remove legacy PIC 8259 driver

2019-05-30 Thread Ma, Maurice
Reviewed-by: Maurice Ma 

Regards,
Maurice

> -Original Message-
> From: Dong, Guo
> Sent: Thursday, May 30, 2019 2:52
> To: devel@edk2.groups.io
> Cc: Ma, Maurice ; You, Benjamin
> ; Dong, Guo 
> Subject: [edk2-devel] UefiPayloadPkg: Remove legacy PIC 8259 driver
> 
> Since legacy PIC 8259 driver would be removed from edk2, update UEFI payload
> to remove 8259 driver.
> If required, bootloader could disable 8259.
> 
> Signed-off-by: Guo Dong 
> ---
>  UefiPayloadPkg/UefiPayloadPkg.fdf| 1 -
>  UefiPayloadPkg/UefiPayloadPkgIa32.dsc| 1 -
>  UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc | 1 -
>  3 files changed, 3 deletions(-)
> 
> diff --git a/UefiPayloadPkg/UefiPayloadPkg.fdf
> b/UefiPayloadPkg/UefiPayloadPkg.fdf
> index ce3b34999b..4cd88a3f85 100644
> --- a/UefiPayloadPkg/UefiPayloadPkg.fdf
> +++ b/UefiPayloadPkg/UefiPayloadPkg.fdf
> @@ -104,7 +104,6 @@ INF
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
>  INF UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
>  INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
>  INF
> MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestD
> xe.inf
> -INF PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf
>  INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
>  INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
>  INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
> diff --git a/UefiPayloadPkg/UefiPayloadPkgIa32.dsc
> b/UefiPayloadPkg/UefiPayloadPkgIa32.dsc
> index 5b6ed36e9c..11cf17ca06 100644
> --- a/UefiPayloadPkg/UefiPayloadPkgIa32.dsc
> +++ b/UefiPayloadPkg/UefiPayloadPkgIa32.dsc
> @@ -432,7 +432,6 @@
>UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
>MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
> 
> MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestD
> xe.inf
> -  PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf
>MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
>MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
>MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
> diff --git a/UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc
> b/UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc
> index d57b5241dc..5b7994a62c 100644
> --- a/UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc
> +++ b/UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc
> @@ -433,7 +433,6 @@
>UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
>MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
> 
> MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestD
> xe.inf
> -  PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf
>MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
>MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
>MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
> --
> 2.16.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41691): https://edk2.groups.io/g/devel/message/41691
Mute This Topic: https://groups.io/mt/31837164/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] Maintainers.txt: update UEFI payload information

2019-05-30 Thread Ma, Maurice
Reviewed-by: Maurice Ma 

Regards,
Maurice

> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Guo
> Dong
> Sent: Thursday, May 30, 2019 3:07
> To: devel@edk2.groups.io
> Cc: Ma, Maurice ; You, Benjamin
> ; Dong, Guo ; Agyeman,
> Prince 
> Subject: [edk2-devel] Maintainers.txt: update UEFI payload information
> 
> Remove CorebootModulePkg and CorebootPayloadPkg, and add
> UefiPayloadPkg to reflect recently change in UEFI payload.
> wiki link for UefiPayloadPkg would be available soon.
> 
> Signed-off-by: Guo Dong 
> ---
>  Maintainers.txt | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/Maintainers.txt b/Maintainers.txt index f3ce5edd8a..010ec31404
> 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -86,13 +86,6 @@ M: Bob Feng 
>  M: Liming Gao 
>  R: Yonghong Zhu 
> 
> -CorebootModulePkg, CorebootPayloadPkg
> -W:
> https://github.com/tianocore/tianocore.github.io/wiki/Coreboot_UEFI_payload
> -M: Maurice Ma 
> -M: Prince Agyeman 
> -M: Benjamin You 
> -S: Maintained
> -
>  CryptoPkg
>  W: https://github.com/tianocore/tianocore.github.io/wiki/CryptoPkg
>  M: Jian Wang 
> @@ -238,6 +231,13 @@ M: Eric Dong 
>  M: Ray Ni 
>  R: Laszlo Ersek 
> 
> +UefiPayloadPkg
> +W: https://github.com/tianocore/tianocore.github.io/wiki/UefiPayloadPkg
> +M: Maurice Ma 
> +M: Guo Dong 
> +M: Benjamin You 
> +S: Maintained
> +
>  StandaloneMmPkg
>  M: Achin Gupta 
>  M: Jiewen Yao 
> --
> 2.16.2.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41690): https://edk2.groups.io/g/devel/message/41690
Mute This Topic: https://groups.io/mt/31837296/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] Edk2 BaseTools Patches.

2019-05-30 Thread Andrew Fish via Groups.Io

> On May 30, 2019, at 2:31 PM, Johnson, Michael  
> wrote:
> 
> All,
>
> These patches are not required for the stable tag.  They’re improvements 
> needed to enable relatively new build options that are not yet widely used.
>
> That said, let me try to clear the air here about what is happening regarding 
> the sources/includes and what changes with these patches.
>
> The INF spec (section 3.9 
> )
>  says that all local source files, including .h files, must be included in 
> the sources section.  This means a module is not compliant if it includes a 
> header file from a directory other than a package include directory and fails 
> to list it in its sources section.  We’re already generating a hash which is 
> guaranteed to change whenever the module’s source changes - without invoking 
> mkdep - by hashing each file from the sources section as well as *all* the 
> contents of every include folder belonging to each package that the module is 
> dependent on.
>
> Every possible ‘legally’ included header will change the hash,

Michael,

What about the force include of AutoGen.h? 

> but the hashes of non-compliant modules will not change when their 
> ‘illegally’ included headers change and we will incorrectly re-use stale 
> cached binaries.  To prevent this, the below patches check for compliance and 
> invalidate the hash of any non-compliant module.  In this way, non-compliance 
> is neither supported nor allowed to poison the cache.
>

If there is a rule the tools should enforce the rule with good error messages. 

> Again, since this only has an effect on builds which have enabled this 
> relatively new feature, I don’t expect any production impact if the stable 
> tag doesn’t take these patches.  Nobody is using it yet.
>

I think Leif and I are both concerned about having two ways to do something as 
complex as make dependencies, as they risk getting out of phase, or breaking 
different ways (like following the .h rules in the INF File).

Also I understand some times we hit circular dependencies and that forces 
duplication. It would be good in general to try and make a list of these kind 
of circular dependencies, given they may been caused by a faulty high level 
design decision made long ago. At some point refactoring the build system from 
the top might be the right approach. 

Thanks,

Andrew Fish


> -Michael
>
>   <>
> From: devel@edk2.groups.io  
> [mailto:devel@edk2.groups.io ] On Behalf Of 
> Andrew Fish via Groups.Io
> Sent: Thursday, May 30, 2019 11:10 AM
> To: devel@edk2.groups.io ; Leif Lindholm 
> mailto:leif.lindh...@linaro.org>>
> Cc: Feng, Bob C mailto:bob.c.f...@intel.com>>; 
> Rodriguez, Christian  >; Laszlo Ersek  >; Kinney, Michael D  >; Gao, Liming  >; Shi, Steven  >; Fan, ZhijuX  >
> Subject: Re: [edk2-devel] Edk2 BaseTools Patches.
>
>
> 
> 
> On May 30, 2019, at 9:37 AM, Leif Lindholm  > wrote:
>
> Thanks Bob, Christian,
> 
> On Thu, May 30, 2019 at 03:06:48PM +, Feng, Bob C wrote:
> 
> Thanks Christian. I add some short description for the patches.
> 
> These 5 patches are all for binary cache feature.
> 
> [Patch V4 2/2] BaseTools: Refactor hash tracking after checking for Sources 
> section
> [Patch V4 1/2] BaseTools: Add a checking for Sources section in INF file
> 
> The above 2 patches is to fix the issue that
> The  build tool uses the files list under [sources] section of INF
> file as a input to calculate a module's hash value. But in some INF
> files, [sources] does not list all the "source" files, missing some
> .h files. Path 2/2 use another method to get all source files for a
> module and patch 1/2 do a check whether [sources] list all the
> "source" files.
> 
> I'll be honest - because of the wild variance in whether .h files are
> listed in the [sources] section of .inf files, I have always been
> unsure as to whether they were just being ignored (and extracted on
> the side via mkdep or similar).
> 
>
> Leif,
>
> I'm confused too as you can only really know the set of include files by 
> doing the mkdep?
>
> I don't see the value of hashing the local include files as any include file 
> change in the mkdep path requires the module to be recompiled. It seems to me 
> having one scheme for hashing and anther four building is going to cause a 
> lot of very subtle errors that are really hard to debug. When you have these 
> kind of errors in your build system you teach people they always need to make 
> clean, so they bypass the hashing and dependency checks. 
>
> Seems like we may be fighting the makefiles again, but 

Re: [edk2-devel] Edk2 BaseTools Patches.

2019-05-30 Thread Johnson, Michael
All,

These patches are not required for the stable tag.  They're improvements needed 
to enable relatively new build options that are not yet widely used.

That said, let me try to clear the air here about what is happening regarding 
the sources/includes and what changes with these patches.

The INF spec (section 
3.9)
 says that all local source files, including .h files, must be included in the 
sources section.  This means a module is not compliant if it includes a header 
file from a directory other than a package include directory and fails to list 
it in its sources section.  We're already generating a hash which is guaranteed 
to change whenever the module's source changes - without invoking mkdep - by 
hashing each file from the sources section as well as *all* the contents of 
every include folder belonging to each package that the module is dependent on.

Every possible 'legally' included header will change the hash, but the hashes 
of non-compliant modules will not change when their 'illegally' included 
headers change and we will incorrectly re-use stale cached binaries.  To 
prevent this, the below patches check for compliance and invalidate the hash of 
any non-compliant module.  In this way, non-compliance is neither supported nor 
allowed to poison the cache.

Again, since this only has an effect on builds which have enabled this 
relatively new feature, I don't expect any production impact if the stable tag 
doesn't take these patches.  Nobody is using it yet.

-Michael


From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Andrew 
Fish via Groups.Io
Sent: Thursday, May 30, 2019 11:10 AM
To: devel@edk2.groups.io; Leif Lindholm 
Cc: Feng, Bob C ; Rodriguez, Christian 
; Laszlo Ersek ; Kinney, 
Michael D ; Gao, Liming ; 
Shi, Steven ; Fan, ZhijuX 
Subject: Re: [edk2-devel] Edk2 BaseTools Patches.




On May 30, 2019, at 9:37 AM, Leif Lindholm 
mailto:leif.lindh...@linaro.org>> wrote:

Thanks Bob, Christian,

On Thu, May 30, 2019 at 03:06:48PM +, Feng, Bob C wrote:

Thanks Christian. I add some short description for the patches.

These 5 patches are all for binary cache feature.

[Patch V4 2/2] BaseTools: Refactor hash tracking after checking for Sources 
section
[Patch V4 1/2] BaseTools: Add a checking for Sources section in INF file

The above 2 patches is to fix the issue that
The  build tool uses the files list under [sources] section of INF
file as a input to calculate a module's hash value. But in some INF
files, [sources] does not list all the "source" files, missing some
.h files. Path 2/2 use another method to get all source files for a
module and patch 1/2 do a check whether [sources] list all the
"source" files.

I'll be honest - because of the wild variance in whether .h files are
listed in the [sources] section of .inf files, I have always been
unsure as to whether they were just being ignored (and extracted on
the side via mkdep or similar).


Leif,

I'm confused too as you can only really know the set of include files by doing 
the mkdep?

I don't see the value of hashing the local include files as any include file 
change in the mkdep path requires the module to be recompiled. It seems to me 
having one scheme for hashing and anther four building is going to cause a lot 
of very subtle errors that are really hard to debug. When you have these kind 
of errors in your build system you teach people they always need to make clean, 
so they bypass the hashing and dependency checks.

Seems like we may be fighting the makefiles again, but from a 10,000 point of 
view it seems like the dependency algorithm and the hash need to be tied 
together. Seems like the makefile already knows if it needs to build it, but 
I'm not sure if the makefile can run an action if it does not need to build 
something?

Thanks,

Andrew Fish



If the intent is to speed up build time, would it not be better to
warn the user - so they notice the problem and fix their modules,
rather than adding extra processing time on having the tools work with
broken .inf files?

This does not look like material for edk2-stable201905 to me.


[PATCH v3 1/1] BaseTools:Extend the binary cache to support library cache
This patch is to resolve the problem that
Build tool dose not cache the library binaries now. Whiteout this
patch, there is 25% extra time cost to rebuild the all module
dependency libraries if cache miss happen.

25% is a big number, so I won't argue against this. But I also won't
argue for it - the BZ was raised very late in the cycle.


[PATCH] BaseTools:Update binary cache restore time to current time
This patch is to make the restored binary file have the current time
stamp not the binary file original time stamp.

I can see how the current behaviour could cause problems with some
CI/build systems. If it is properly reviewed and tested, I am OK with

Re: [edk2-devel] [PATCH v2 6/7] EmbeddedPkg: Fix DwEmmc SendCommand polling

2019-05-30 Thread Haojian Zhuang
On Tue, May 28, 2019 at 07:04:09PM +0100, Leif Lindholm wrote:
> +Haojian,
> 
> Haojian - since you are the original author, can you comment on the
> delays? Are these silicon bug workarounds (so we need to add a Pcd),
> or does these changes work on your platforms too?

I'm not in the loop, so I missed the patch series.

The patch series can't work on my platform for the eMMC. Although a
variable is created to identify whether it's a SD or eMMC device, it
doesn't identify the eMMC device by the right way. So the eMMC device
isn't initialized successfully on my platform.

1. Since MMC framework could identify whether it's eMMC device or SD
device, we need to make device driver gets this kind of information from
the MMC framework. And we need to support multiple eMMC/SD instances in
MMC framework.

2. I sent a patch series to support both eMMC device and SD device before.
https://edk2.groups.io/g/devel/message/28572
&&
https://edk2.groups.io/g/devel/message/28615
Maybe it's missed. Could you help to review that patch series?

Best Regards
Haojian

> 
> Regards,
> 
> Leif
> 
> On Mon, May 27, 2019 at 05:30:27PM +0800, tien.hock@intel.com wrote:
> > From: "Tien Hock, Loh" 
> > 
> > Change SendCommand polling mode to remove unnecessary delay, and check
> > for transfer done only when block data is to be read/write. This would
> > also increase performance slightly.
> > 
> > Signed-off-by: "Tien Hock, Loh" 
> > Cc: Leif Lindholm 
> > Cc: Ard Biesheuvel 
> > ---
> >  EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c | 43 +++-
> >  1 file changed, 33 insertions(+), 10 deletions(-)
> > 
> > diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c 
> > b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
> > index c6c8e04917..b57833458f 100644
> > --- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
> > +++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
> > @@ -286,16 +286,13 @@ SendCommand (
> >  DWEMMC_INT_RCRC | DWEMMC_INT_RE;
> >ErrMask |= DWEMMC_INT_DCRC | DWEMMC_INT_DRT | DWEMMC_INT_SBE;
> >do {
> > -MicroSecondDelay(500);
> >  Data = MmioRead32 (DWEMMC_RINTSTS);
> > -
> > -if (Data & ErrMask) {
> > -  return EFI_DEVICE_ERROR;
> > -}
> > -if (Data & DWEMMC_INT_DTO) { // Transfer Done
> > -  break;
> > -}
> >} while (!(Data & DWEMMC_INT_CMD_DONE));
> > +
> > +  if (Data & ErrMask) {
> > +return EFI_DEVICE_ERROR;
> > +  }
> > +
> >return EFI_SUCCESS;
> >  }
> >  
> > @@ -550,8 +547,9 @@ DwEmmcReadBlockData (
> >)
> >  {
> >EFI_STATUS  Status;
> > -  UINT32  DescPages, CountPerPage, Count;
> > +  UINT32  DescPages, CountPerPage, Count, ErrMask;
> >EFI_TPL Tpl;
> > +  UINTN Rintsts = 0;
> >  
> >Tpl = gBS->RaiseTPL (TPL_NOTIFY);
> >  
> > @@ -574,6 +572,18 @@ DwEmmcReadBlockData (
> >  DEBUG ((DEBUG_ERROR, "Failed to read data, mDwEmmcCommand:%x, 
> > mDwEmmcArgument:%x, Status:%r\n", mDwEmmcCommand, mDwEmmcArgument, Status));
> >  goto out;
> >}
> > +
> > +  while(!((MmioRead32(DWEMMC_RINTSTS) & (DWEMMC_INT_DTO {
> > +Rintsts = MmioRead32 (DWEMMC_RINTSTS);
> > +  }
> > +  ErrMask = DWEMMC_INT_EBE | DWEMMC_INT_HLE | DWEMMC_INT_RTO |
> > +DWEMMC_INT_RCRC | DWEMMC_INT_RE | DWEMMC_INT_DCRC |
> > +DWEMMC_INT_DRT | DWEMMC_INT_SBE;
> > +
> > +  if (Rintsts & ErrMask) {
> > +Status = EFI_DEVICE_ERROR;
> > +goto out;
> > +  }
> >  out:
> >// Restore Tpl
> >gBS->RestoreTPL (Tpl);
> > @@ -589,8 +599,9 @@ DwEmmcWriteBlockData (
> >)
> >  {
> >EFI_STATUS  Status;
> > -  UINT32  DescPages, CountPerPage, Count;
> > +  UINT32  DescPages, CountPerPage, Count, ErrMask;
> >EFI_TPL Tpl;
> > +  UINTN Rintsts = 0;
> >  
> >Tpl = gBS->RaiseTPL (TPL_NOTIFY);
> >  
> > @@ -613,6 +624,18 @@ DwEmmcWriteBlockData (
> >  DEBUG ((DEBUG_ERROR, "Failed to write data, mDwEmmcCommand:%x, 
> > mDwEmmcArgument:%x, Status:%r\n", mDwEmmcCommand, mDwEmmcArgument, Status));
> >  goto out;
> >}
> > +
> > +  while(!((MmioRead32(DWEMMC_RINTSTS) & (DWEMMC_INT_DTO {
> > +Rintsts = MmioRead32 (DWEMMC_RINTSTS);
> > +  }
> > +  ErrMask = DWEMMC_INT_EBE | DWEMMC_INT_HLE | DWEMMC_INT_RTO |
> > +DWEMMC_INT_RCRC | DWEMMC_INT_RE | DWEMMC_INT_DCRC |
> > +DWEMMC_INT_DRT | DWEMMC_INT_SBE;
> > +
> > +  if (Rintsts & ErrMask) {
> > +Status = EFI_DEVICE_ERROR;
> > +goto out;
> > +  }
> >  out:
> >// Restore Tpl
> >gBS->RestoreTPL (Tpl);
> > -- 
> > 2.19.0
> > 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41687): https://edk2.groups.io/g/devel/message/41687
Mute This Topic: https://groups.io/mt/31807965/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] Edk2 BaseTools Patches.

2019-05-30 Thread Andrew Fish via Groups.Io


> On May 30, 2019, at 9:37 AM, Leif Lindholm  wrote:
> 
> Thanks Bob, Christian,
> 
> On Thu, May 30, 2019 at 03:06:48PM +, Feng, Bob C wrote:
>> Thanks Christian. I add some short description for the patches.
>> 
>> These 5 patches are all for binary cache feature.
>> 
>> [Patch V4 2/2] BaseTools: Refactor hash tracking after checking for Sources 
>> section
>> [Patch V4 1/2] BaseTools: Add a checking for Sources section in INF file
>> 
>> The above 2 patches is to fix the issue that
>> The  build tool uses the files list under [sources] section of INF
>> file as a input to calculate a module's hash value. But in some INF
>> files, [sources] does not list all the "source" files, missing some
>> .h files. Path 2/2 use another method to get all source files for a
>> module and patch 1/2 do a check whether [sources] list all the
>> "source" files.
> 
> I'll be honest - because of the wild variance in whether .h files are
> listed in the [sources] section of .inf files, I have always been
> unsure as to whether they were just being ignored (and extracted on
> the side via mkdep or similar).
> 

Leif,

I'm confused too as you can only really know the set of include files by doing 
the mkdep?

I don't see the value of hashing the local include files as any include file 
change in the mkdep path requires the module to be recompiled. It seems to me 
having one scheme for hashing and anther four building is going to cause a lot 
of very subtle errors that are really hard to debug. When you have these kind 
of errors in your build system you teach people they always need to make clean, 
so they bypass the hashing and dependency checks. 

Seems like we may be fighting the makefiles again, but from a 10,000 point of 
view it seems like the dependency algorithm and the hash need to be tied 
together. Seems like the makefile already knows if it needs to build it, but 
I'm not sure if the makefile can run an action if it does not need to build 
something? 

Thanks,

Andrew Fish


> If the intent is to speed up build time, would it not be better to
> warn the user - so they notice the problem and fix their modules,
> rather than adding extra processing time on having the tools work with
> broken .inf files?
> 
> This does not look like material for edk2-stable201905 to me.
> 
>> [PATCH v3 1/1] BaseTools:Extend the binary cache to support library cache
>> This patch is to resolve the problem that
>> Build tool dose not cache the library binaries now. Whiteout this
>> patch, there is 25% extra time cost to rebuild the all module
>> dependency libraries if cache miss happen.
> 
> 25% is a big number, so I won't argue against this. But I also won't
> argue for it - the BZ was raised very late in the cycle.
> 
>> [PATCH] BaseTools:Update binary cache restore time to current time
>> This patch is to make the restored binary file have the current time
>> stamp not the binary file original time stamp.
> 
> I can see how the current behaviour could cause problems with some
> CI/build systems. If it is properly reviewed and tested, I am OK with
> this one going in for edk2-stable201903.
> 
>> [PATCH V5] BaseTools:Make BaseTools support new rules to generate RAW FFS 
>> FILE
>> This patch is to support the raw ffs file rule. Now build tool does
>> not correctly handle this case:
>> 
>> [Rule.Common.USER_DEFINED.MicroCode]
>>  FILE RAW = $(NAMED_GUID) {
>> $(INF_OUTPUT)/$(MODULE_NAME).bin
>>  }
> 
> This looks like a new feature - not something that should bypass the
> freeze period for edk2-stable201905.
> Can you explain why this is needed in the stable tag as opposed to
> being available from master the day after the tag is made?
> 
> Best Regards,
> 
> Leif
> 
>> 
>> 
>> Thanks,
>> Bob
>> 
>> -Original Message-
>> From: Rodriguez, Christian 
>> Sent: Thursday, May 30, 2019 10:26 PM
>> To: Leif Lindholm ; Feng, Bob C 
>> 
>> Cc: Andrew Fish ; Laszlo Ersek ; Kinney, 
>> Michael D ; devel@edk2.groups.io; Gao, Liming 
>> ; Shi, Steven ; Fan, ZhijuX 
>> 
>> Subject: RE: Edk2 BaseTools Patches.
>> 
>> Hey Leif,
>> 
>> I thought I'd help Bob and gather those BZs for each thread:
>> 
>> [Patch V4 1/2] BaseTools: Add a checking for Sources section in INF file 
>> [Patch V4 2/2] BaseTools: Refactor hash tracking after checking for Sources 
>> section
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804
>> 
>> [PATCH v3 1/1] BaseTools:Extend the binary cache to support library cache
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1797
>> 
>> [PATCH V5] BaseTools:Make BaseTools support new rules to generate RAW FFS 
>> FILE
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1765
>> 
>> [PATCH] BaseTools:Update binary cache restore time to current time
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1742
>> 
>> Thanks,
>> Christian
>> 
>>> -Original Message-
>>> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
>>> Sent: Thursday, May 30, 2019 2:28 AM
>>> To: Feng, Bob C 
>>> 

Re: [edk2-devel] [PATCH v2] Marvell/Armada80x0McBin: Enable USB keyboard, NVMe boot

2019-05-30 Thread Leif Lindholm
On Thu, May 30, 2019 at 12:39:13PM -0500, Jeremy Linton wrote:
> Add the USB keyboard driver, which is needed when a graphical
> console is used. Also add a NVMe boot option for users that
> choose that instead of SATA or MMC.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jeremy Linton 
> Reviewed-by: Marcin Wojtas 

That one worked much better:
Reviewed-by: Leif Lindholm 
Pushed as 6784b55292.

(Also, *giggle* @ mammon.)

/
Leif

> ---
>  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 3 +++
>  Silicon/Marvell/Armada7k8k/Armada7k8k.fdf | 2 ++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc 
> b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> index 27e52f5af5..89d617fde4 100644
> --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> @@ -90,6 +90,7 @@
>
> UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
>
> UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf
>UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
> +  UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
>  
>PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>  
> @@ -505,6 +506,7 @@
>MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
>MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
>MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
> +  MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
>  
># SD/MMC
>MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
> @@ -516,6 +518,7 @@
>ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
>MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
>MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> +  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf
>  
># Console packages
>MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
> diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf 
> b/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf
> index 47e3bc4f82..f59cc1f263 100644
> --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf
> +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf
> @@ -151,6 +151,7 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
>INF MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
>INF MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
>INF MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
> +  INF MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
>  
># SD/MMC
>INF MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
> @@ -162,6 +163,7 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
>INF ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
>INF MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
>INF MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> +  INF MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf
>  
># Multiple Console IO support
>INF MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
> -- 
> 2.21.0
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41685): https://edk2.groups.io/g/devel/message/41685
Mute This Topic: https://groups.io/mt/31871343/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] Edk2 BaseTools Patches.

2019-05-30 Thread Christian Rodriguez
See below.

>-Original Message-
>From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
>Sent: Thursday, May 30, 2019 9:38 AM
>To: Feng, Bob C 
>Cc: Rodriguez, Christian ; Andrew Fish
>; Laszlo Ersek ; Kinney, Michael D
>; devel@edk2.groups.io; Gao, Liming
>; Shi, Steven ; Fan, ZhijuX
>
>Subject: Re: Edk2 BaseTools Patches.
>
>Thanks Bob, Christian,
>
>On Thu, May 30, 2019 at 03:06:48PM +, Feng, Bob C wrote:
>> Thanks Christian. I add some short description for the patches.
>>
>> These 5 patches are all for binary cache feature.
>>
>>  [Patch V4 2/2] BaseTools: Refactor hash tracking after checking for
>> Sources section  [Patch V4 1/2] BaseTools: Add a checking for Sources
>> section in INF file
>>
>> The above 2 patches is to fix the issue that The  build tool uses the
>> files list under [sources] section of INF file as a input to calculate
>> a module's hash value. But in some INF files, [sources] does not list
>> all the "source" files, missing some .h files. Path 2/2 use another
>> method to get all source files for a module and patch 1/2 do a check
>> whether [sources] list all the "source" files.
>
>I'll be honest - because of the wild variance in whether .h files are listed 
>in the
>[sources] section of .inf files, I have always been unsure as to whether they
>were just being ignored (and extracted on the side via mkdep or similar).
>
>If the intent is to speed up build time, would it not be better to warn the 
>user
>- so they notice the problem and fix their modules, rather than adding extra
>processing time on having the tools work with broken .inf files?
>
>This does not look like material for edk2-stable201905 to me.

The patch does warn the user, so they notice the problem and fix their modules. 
And somewhere in the email thread for that patch set I mentioned the processing 
time is almost negligible since the information is already available in memory 
and it's just a simple set lookup for existence and warning write.

It doesn't really matter if it goes in edk2-stable201905, as long as it goes in 
eventually because it does fix a bug/corner-case in the hash feature.

>
>> [PATCH v3 1/1] BaseTools:Extend the binary cache to support library
>> cache This patch is to resolve the problem that Build tool dose not
>> cache the library binaries now. Whiteout this patch, there is 25%
>> extra time cost to rebuild the all module dependency libraries if
>> cache miss happen.
>
>25% is a big number, so I won't argue against this. But I also won't argue for 
>it -
>the BZ was raised very late in the cycle.
>
>> [PATCH] BaseTools:Update binary cache restore time to current time
>> This patch is to make the restored binary file have the current time
>> stamp not the binary file original time stamp.
>
>I can see how the current behaviour could cause problems with some CI/build
>systems. If it is properly reviewed and tested, I am OK with this one going in
>for edk2-stable201903.
>
>> [PATCH V5] BaseTools:Make BaseTools support new rules to generate RAW
>> FFS FILE This patch is to support the raw ffs file rule. Now build
>> tool does not correctly handle this case:
>>
>> [Rule.Common.USER_DEFINED.MicroCode]
>>   FILE RAW = $(NAMED_GUID) {
>>  $(INF_OUTPUT)/$(MODULE_NAME).bin
>>   }
>
>This looks like a new feature - not something that should bypass the freeze
>period for edk2-stable201905.
>Can you explain why this is needed in the stable tag as opposed to being
>available from master the day after the tag is made?
>
>Best Regards,
>
>Leif
>
>>
>>
>> Thanks,
>> Bob
>>
>> -Original Message-
>> From: Rodriguez, Christian
>> Sent: Thursday, May 30, 2019 10:26 PM
>> To: Leif Lindholm ; Feng, Bob C
>> 
>> Cc: Andrew Fish ; Laszlo Ersek ;
>> Kinney, Michael D ; devel@edk2.groups.io;
>> Gao, Liming ; Shi, Steven
>> ; Fan, ZhijuX 
>> Subject: RE: Edk2 BaseTools Patches.
>>
>> Hey Leif,
>>
>> I thought I'd help Bob and gather those BZs for each thread:
>>
>> [Patch V4 1/2] BaseTools: Add a checking for Sources section in INF
>> file [Patch V4 2/2] BaseTools: Refactor hash tracking after checking
>> for Sources section
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804
>>
>> [PATCH v3 1/1] BaseTools:Extend the binary cache to support library
>> cache
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1797
>>
>> [PATCH V5] BaseTools:Make BaseTools support new rules to generate RAW
>> FFS FILE
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1765
>>
>> [PATCH] BaseTools:Update binary cache restore time to current time
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1742
>>
>> Thanks,
>> Christian
>>
>> >-Original Message-
>> >From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
>> >Sent: Thursday, May 30, 2019 2:28 AM
>> >To: Feng, Bob C 
>> >Cc: Andrew Fish ; Laszlo Ersek ;
>> >Kinney, Michael D ; devel@edk2.groups.io;
>> >Gao, Liming ; Shi, Steven
>> >; Rodriguez, Christian
>> >; Fan, ZhijuX 
>> >Subject: Re: Edk2 BaseTools Patches.
>> >
>> >Hi 

[edk2-devel] [PATCH v2] Marvell/Armada80x0McBin: Enable USB keyboard, NVMe boot

2019-05-30 Thread Jeremy Linton
Add the USB keyboard driver, which is needed when a graphical
console is used. Also add a NVMe boot option for users that
choose that instead of SATA or MMC.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jeremy Linton 
Reviewed-by: Marcin Wojtas 
---
 Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 3 +++
 Silicon/Marvell/Armada7k8k/Armada7k8k.fdf | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc 
b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
index 27e52f5af5..89d617fde4 100644
--- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
+++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
@@ -90,6 +90,7 @@
   
UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
   
UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf
   UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
+  UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
 
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
 
@@ -505,6 +506,7 @@
   MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
   MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
   MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
+  MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
 
   # SD/MMC
   MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
@@ -516,6 +518,7 @@
   ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
   MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
   MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf
 
   # Console packages
   MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf 
b/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf
index 47e3bc4f82..f59cc1f263 100644
--- a/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf
+++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf
@@ -151,6 +151,7 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
   INF MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
   INF MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
   INF MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
+  INF MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
 
   # SD/MMC
   INF MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
@@ -162,6 +163,7 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
   INF ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
   INF MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
   INF MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+  INF MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf
 
   # Multiple Console IO support
   INF MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
-- 
2.21.0


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41683): https://edk2.groups.io/g/devel/message/41683
Mute This Topic: https://groups.io/mt/31871343/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH] Marvell/Armada80x0McBin: Enable usb keyboard, nvme boot

2019-05-30 Thread Jeremy Linton



Hi,


On 5/29/19 6:09 AM, Leif Lindholm wrote:

On Tue, May 28, 2019 at 06:43:13PM -0500, Jeremy Linton wrote:

Add a USB keyboard flag, which is needed when a graphical console
is enabled. Also add a nvme boot option for users that choose
that instead of sata or mmc.

Contributed-under: TianoCore Contribution Agreement 1.1


Just a note in case you had missed:
We are transitioning the licensing of edk2-platforms to
https://spdx.org/licenses/BSD-2-Clause-Patent
(As of yesterday) we have approval from Marvell to relicense these
files, so future contributions won't need the Contributed-under.
(This one does, though.)


Thanks, I didn't know that.




Signed-off-by: Jeremy Linton 


Reviewed-by: Leif Lindholm 

However, the patch is garbled (lines broken and '=20' all over the
place). Could you have a look at your email setup or point me at a
(public) remote tree?

(It's possible your mail server will stop playing silly if you strip
the CRs - and SMTP eats those anyway, so I have a script to add them
back in. >
((It's also possible it's groups.io that's messing things up - you
could work around that by cc:ing me :))


Ok, I will strip the CRs and resend it with you on CC. If that fails I 
will find a place to push it.





/
 Leif


---
  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 3 +++
  Silicon/Marvell/Armada7k8k/Armada7k8k.fdf | 2 ++
  2 files changed, 5 insertions(+)

diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc 
b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
index 27e52f5af5..89d617fde4 100644
--- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
+++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
@@ -90,6 +90,7 @@

UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf

UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf
UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
+  UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
  
PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
  
@@ -505,6 +506,7 @@

MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
+  MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
  
# SD/MMC

MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
@@ -516,6 +518,7 @@
ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf
  
# Console packages

MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf 
b/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf
index 47e3bc4f82..f59cc1f263 100644
--- a/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf
+++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf
@@ -151,6 +151,7 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
INF MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
INF MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
INF MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
+  INF MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
  
# SD/MMC

INF MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
@@ -162,6 +163,7 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
INF ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
INF MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
INF MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+  INF MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf
  
# Multiple Console IO support

INF MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
--
2.13.7







-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41682): https://edk2.groups.io/g/devel/message/41682
Mute This Topic: https://groups.io/mt/31828270/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] Edk2 BaseTools Patches.

2019-05-30 Thread Leif Lindholm
Thanks Bob, Christian,

On Thu, May 30, 2019 at 03:06:48PM +, Feng, Bob C wrote:
> Thanks Christian. I add some short description for the patches.
> 
> These 5 patches are all for binary cache feature.
> 
>  [Patch V4 2/2] BaseTools: Refactor hash tracking after checking for Sources 
> section
>  [Patch V4 1/2] BaseTools: Add a checking for Sources section in INF file
> 
> The above 2 patches is to fix the issue that
> The  build tool uses the files list under [sources] section of INF
> file as a input to calculate a module's hash value. But in some INF
> files, [sources] does not list all the "source" files, missing some
> .h files. Path 2/2 use another method to get all source files for a
> module and patch 1/2 do a check whether [sources] list all the
> "source" files.

I'll be honest - because of the wild variance in whether .h files are
listed in the [sources] section of .inf files, I have always been
unsure as to whether they were just being ignored (and extracted on
the side via mkdep or similar).

If the intent is to speed up build time, would it not be better to
warn the user - so they notice the problem and fix their modules,
rather than adding extra processing time on having the tools work with
broken .inf files?

This does not look like material for edk2-stable201905 to me.

> [PATCH v3 1/1] BaseTools:Extend the binary cache to support library cache
> This patch is to resolve the problem that
> Build tool dose not cache the library binaries now. Whiteout this
> patch, there is 25% extra time cost to rebuild the all module
> dependency libraries if cache miss happen.

25% is a big number, so I won't argue against this. But I also won't
argue for it - the BZ was raised very late in the cycle.

> [PATCH] BaseTools:Update binary cache restore time to current time
> This patch is to make the restored binary file have the current time
> stamp not the binary file original time stamp.

I can see how the current behaviour could cause problems with some
CI/build systems. If it is properly reviewed and tested, I am OK with
this one going in for edk2-stable201903.

> [PATCH V5] BaseTools:Make BaseTools support new rules to generate RAW FFS FILE
> This patch is to support the raw ffs file rule. Now build tool does
> not correctly handle this case:
>
> [Rule.Common.USER_DEFINED.MicroCode]
>   FILE RAW = $(NAMED_GUID) {
>  $(INF_OUTPUT)/$(MODULE_NAME).bin
>   }

This looks like a new feature - not something that should bypass the
freeze period for edk2-stable201905.
Can you explain why this is needed in the stable tag as opposed to
being available from master the day after the tag is made?

Best Regards,

Leif

> 
> 
> Thanks,
> Bob
> 
> -Original Message-
> From: Rodriguez, Christian 
> Sent: Thursday, May 30, 2019 10:26 PM
> To: Leif Lindholm ; Feng, Bob C 
> 
> Cc: Andrew Fish ; Laszlo Ersek ; Kinney, 
> Michael D ; devel@edk2.groups.io; Gao, Liming 
> ; Shi, Steven ; Fan, ZhijuX 
> 
> Subject: RE: Edk2 BaseTools Patches.
> 
> Hey Leif,
> 
> I thought I'd help Bob and gather those BZs for each thread:
> 
> [Patch V4 1/2] BaseTools: Add a checking for Sources section in INF file 
> [Patch V4 2/2] BaseTools: Refactor hash tracking after checking for Sources 
> section
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804
> 
> [PATCH v3 1/1] BaseTools:Extend the binary cache to support library cache
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1797
> 
> [PATCH V5] BaseTools:Make BaseTools support new rules to generate RAW FFS FILE
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1765
> 
> [PATCH] BaseTools:Update binary cache restore time to current time
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1742
> 
> Thanks,
> Christian
> 
> >-Original Message-
> >From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> >Sent: Thursday, May 30, 2019 2:28 AM
> >To: Feng, Bob C 
> >Cc: Andrew Fish ; Laszlo Ersek ; 
> >Kinney, Michael D ; devel@edk2.groups.io; 
> >Gao, Liming ; Shi, Steven ; 
> >Rodriguez, Christian ; Fan, ZhijuX 
> >
> >Subject: Re: Edk2 BaseTools Patches.
> >
> >Hi Bob,
> >
> >On Thu, May 30, 2019 at 06:39:59AM +, Feng, Bob C wrote:
> >> Hi,
> >>
> >> Currently, we have 5 Basetools patches which are ready to push. Since 
> >> we are in the soft-freeze phase, I'd like to ask for your opinions if 
> >> those patches can be pushed to edk2 master.
> >
> >To save me the time of reading through all the threads and getting to 
> >grips with all the code, could you summarise the problem these solve 
> >and the impact of not including these?
> >
> >Is there a BZ?
> >
> >Regards,
> >
> >Leif
> >
> >>
> >> These 5 patches are to fix the issues for the build cache feature.
> >>
> >> [Patch V4 2/2] BaseTools: Refactor hash tracking after checking for 
> >> Sources section
> >> https://edk2.groups.io/g/devel/topic/31835556#41642
> >>
> >> [Patch V4 1/2] BaseTools: Add a checking for Sources section in INF 
> >> file
> >> 

[edk2-devel] [RFC PATCH 2/2] BaseTools: add script to configure local git options

2019-05-30 Thread Leif Lindholm
Patch contribution and review is greatly simplified by following the
steps described in "Laszlo's unkempt guide":
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers
but there are a lot of tedious manual steps in there, so here is a
python script that configures all options I am aware of
*for the repository the script is executed from*.

Signed-off-by: Leif Lindholm 
---
 BaseTools/Scripts/SetupGit.py | 187 ++
 1 file changed, 187 insertions(+)
 create mode 100644 BaseTools/Scripts/SetupGit.py

diff --git a/BaseTools/Scripts/SetupGit.py b/BaseTools/Scripts/SetupGit.py
new file mode 100644
index 00..3b199f08b2
--- /dev/null
+++ b/BaseTools/Scripts/SetupGit.py
@@ -0,0 +1,187 @@
+## @file
+#  Set up the git configuration for contributing to TianoCore projects
+#
+#  Copyright (c) 2019, Linaro Ltd. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+from __future__ import print_function
+import argparse
+import os.path
+import sys
+
+try:
+import git
+except ImportError:
+print('Unable to load gitpython module - please install and try again.')
+sys.exit(1)
+
+try:
+# Try Python 2 'ConfigParser' module first since helpful lib2to3 will
+# otherwise automagically load it with the name 'configparser'
+import ConfigParser
+except ImportError:
+# Otherwise, try loading the Python 3 'configparser' uner an alias
+try:
+import configparser as ConfigParser
+except ImportError:
+print("Unable to load configparser/ConfigParser module - please 
install and try again!")
+sys.exit(1)
+
+
+# Assumptions: Script is in edk2/BaseTools/Scripts,
+#  templates in edk2/BaseTools/Conf
+CONFDIR = 
os.path.join(os.path.dirname(os.path.dirname(os.path.realpath(__file__))),
+   'Conf')
+
+UPSTREAMS = [
+{'name': 'edk2',
+ 'repo': 'https://github.com/tianocore/edk2.git',
+ 'list': 'devel@edk2.groups.io'},
+{'name': 'edk2-platforms',
+ 'repo': 'https://github.com/tianocore/edk2-platforms.git',
+ 'list': 'devel@edk2.groups.io', 'prefix': 'edk2-platforms'},
+{'name': 'edk2-non-osi',
+ 'repo': 'https://github.com/tianocore/edk2-non-osi.git',
+ 'list': 'devel@edk2.groups.io', 'prefix': 'edk2-non-osi'}
+]
+
+# The minimum version required for all of the below options to work
+MIN_GIT_VERSION = (1, 9, 0)
+
+# Set of options to be set identically for all repositories
+OPTIONS = [
+{'section': 'am',  'option': 'keepcr', 'value': True},
+{'section': 'am',  'option': 'signoff','value': True},
+{'section': 'cherry-pick', 'option': 'signoff','value': True},
+{'section': 'color',   'option': 'diff',   'value': True},
+{'section': 'color',   'option': 'grep',   'value': 'auto'},
+{'section': 'commit',  'option': 'signoff','value': True},
+{'section': 'core','option': 'abbrev', 'value': 12},
+{'section': 'core','option': 'attributesFile',
+ 'value': os.path.join(CONFDIR, 'gitattributes').replace('\\', '/')},
+{'section': 'core','option': 'whitespace', 'value': 
'cr-at-eol'},
+{'section': 'diff','option': 'algorithm',  'value': 
'patience'},
+{'section': 'diff','option': 'orderFile',
+ 'value': os.path.join(CONFDIR, 'diff.order').replace('\\', '/')},
+{'section': 'diff','option': 'renames','value': 'copies'},
+{'section': 'diff','option': 'statGraphWidth', 'value': '20'},
+{'section': 'diff "ini"','option': 'xfuncname',
+ 'value': '^[[A-Za-z0-9_., ]+]'}, # or 'diff "ini"'?
+{'section': 'format',  'option': 'coverLetter','value': True},
+{'section': 'format',  'option': 'numbered',   'value': True},
+{'section': 'format',  'option': 'signoff','value': False},
+{'section': 'notes',   'option': 'rewriteRef', 'value': 
'refs/notes/commits'},
+{'section': 'sendemail',   'option': 'chainreplyto',   'value': False},
+{'section': 'sendemail',   'option': 'thread', 'value': True},
+]
+
+
+def locate_repo():
+"""Opens a Repo object for the current tree, searching upwards in the 
directory hierarchy."""
+try:
+repo = git.Repo(path='.', search_parent_directories=True)
+except (git.InvalidGitRepositoryError, git.NoSuchPathError):
+print("It doesn't look like we're inside a git repository - aborting.")
+sys.exit(2)
+return repo
+
+
+def get_upstream(url):
+"""Extracts the dict for the current repo origin."""
+for upstream in UPSTREAMS:
+if upstream['repo'] == url:
+return upstream
+print("Unknown upstream '%s' - aborting!" % url)
+sys.exit(3)
+
+
+def check_versions():
+"""Checks versions of dependencies."""
+version = 

[edk2-devel] [RFC PATCH 1/2] BaseTools: add centralized location for git config files

2019-05-30 Thread Leif Lindholm
Before adding the git environment initialization script, add the
following files that will be pointed to after running said script:

- BaseTools/Conf/diff.order
- BaseTools/Conf/gitattributes

Signed-off-by: Leif Lindholm 
---
 BaseTools/Conf/diff.order|  8 
 BaseTools/Conf/gitattributes | 14 ++
 2 files changed, 22 insertions(+)
 create mode 100644 BaseTools/Conf/diff.order
 create mode 100644 BaseTools/Conf/gitattributes

diff --git a/BaseTools/Conf/diff.order b/BaseTools/Conf/diff.order
new file mode 100644
index 00..1d578ac28c
--- /dev/null
+++ b/BaseTools/Conf/diff.order
@@ -0,0 +1,8 @@
+*.dec
+*.dsc.inc
+*.dsc
+*.fdf
+*.inf
+*.h
+*.vfr
+*.c
diff --git a/BaseTools/Conf/gitattributes b/BaseTools/Conf/gitattributes
new file mode 100644
index 00..a8f923fd8a
--- /dev/null
+++ b/BaseTools/Conf/gitattributes
@@ -0,0 +1,14 @@
+*.efi -diff
+*.EFI -diff
+*.bin -diff
+*.BIN -diff
+*.raw -diff
+*.RAW -diff
+*.bmp -diff
+*.BMP -diff
+*.dec diff=ini
+*.dsc diff=ini
+*.dsc.inc diff=ini
+*.fdf diff=ini
+*.fdf.inc diff=ini
+*.inf diff=ini
-- 
2.11.0


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41679): https://edk2.groups.io/g/devel/message/41679
Mute This Topic: https://groups.io/mt/31870256/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [RFC PATCH 0/2] BaseTools: add script to set up git environment

2019-05-30 Thread Leif Lindholm
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers
is a great resource, but it's a lot of manual steps to go through for each
repository (especially as the number seems to grow).

Script works with python2/3 under both Posix and Windows.

Note: the script does require the 'gitpython' module to be installed.
Under Linux, this can be achieved with your distribution package manager.
Under Windows, you can install this from the Visual Studio
python environment->packages (pypi), and searching for 'gitpython'.

Note2: for simplicity's sake, the script uses a single copy of the
configuration files for each repository - pointing all of them to the
copies in edk2 BaseTools.

Note3: we're hardcoding absolute paths here, so if you move repositories
around, you need to re-run the script.

Note4: all of the settings are done only on a per-repository basis, so as
not to mess with environments for unlelated projects. This also means many
settings that are common across all repositories are set in each of them.

Note4: the script identifies repositories based on their 'origin' URL, so
if someone had a good use-case for something cute, there may be more work
required.


Future plans:
It would be useful to also add common git-hook scripts to install.
I already have some for my own maintainer use.

Even though we only modify settings for the current repository, it would
also make sense to add some sanity checking for global settings (name,
email, mail server config...).

Leif Lindholm (2):
  BaseTools: add centralized location for git config files
  BaseTools: add script to configure local git options

 BaseTools/Conf/diff.order |   8 ++
 BaseTools/Conf/gitattributes  |  14 
 BaseTools/Scripts/SetupGit.py | 187 ++
 3 files changed, 209 insertions(+)
 create mode 100644 BaseTools/Conf/diff.order
 create mode 100644 BaseTools/Conf/gitattributes
 create mode 100644 BaseTools/Scripts/SetupGit.py

-- 
2.11.0


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41678): https://edk2.groups.io/g/devel/message/41678
Mute This Topic: https://groups.io/mt/31870254/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] Edk2 BaseTools Patches.

2019-05-30 Thread Bob Feng
Thanks Christian. I add some short description for the patches.

These 5 patches are all for binary cache feature.

 [Patch V4 2/2] BaseTools: Refactor hash tracking after checking for Sources 
section
 [Patch V4 1/2] BaseTools: Add a checking for Sources section in INF file

The above 2 patches is to fix the issue that
The  build tool uses the files list under [sources] section of INF file as a 
input to calculate a module's hash value. But in some INF files, [sources] does 
not list all the "source" files, missing some .h files. Path 2/2 use another 
method to get all source files for a module and patch 1/2 do a check whether 
[sources] list all the "source" files.

[PATCH v3 1/1] BaseTools:Extend the binary cache to support library cache
This patch is to resolve the problem that
Build tool dose not cache the library binaries now. Whiteout this patch, there 
is 25% extra time cost to rebuild the all module dependency libraries if cache 
miss happen.

[PATCH] BaseTools:Update binary cache restore time to current time
This patch is to make the restored binary file have the current time stamp not 
the binary file original time stamp. 

[PATCH V5] BaseTools:Make BaseTools support new rules to generate RAW FFS FILE
This patch is to support the raw ffs file rule. Now build tool does not 
correctly handle this case:
[Rule.Common.USER_DEFINED.MicroCode]
  FILE RAW = $(NAMED_GUID) {
 $(INF_OUTPUT)/$(MODULE_NAME).bin
  }


Thanks,
Bob

-Original Message-
From: Rodriguez, Christian 
Sent: Thursday, May 30, 2019 10:26 PM
To: Leif Lindholm ; Feng, Bob C 
Cc: Andrew Fish ; Laszlo Ersek ; Kinney, 
Michael D ; devel@edk2.groups.io; Gao, Liming 
; Shi, Steven ; Fan, ZhijuX 

Subject: RE: Edk2 BaseTools Patches.

Hey Leif,

I thought I'd help Bob and gather those BZs for each thread:

[Patch V4 1/2] BaseTools: Add a checking for Sources section in INF file [Patch 
V4 2/2] BaseTools: Refactor hash tracking after checking for Sources section
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804

[PATCH v3 1/1] BaseTools:Extend the binary cache to support library cache
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1797

[PATCH V5] BaseTools:Make BaseTools support new rules to generate RAW FFS FILE
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1765

[PATCH] BaseTools:Update binary cache restore time to current time
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1742

Thanks,
Christian

>-Original Message-
>From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
>Sent: Thursday, May 30, 2019 2:28 AM
>To: Feng, Bob C 
>Cc: Andrew Fish ; Laszlo Ersek ; 
>Kinney, Michael D ; devel@edk2.groups.io; 
>Gao, Liming ; Shi, Steven ; 
>Rodriguez, Christian ; Fan, ZhijuX 
>
>Subject: Re: Edk2 BaseTools Patches.
>
>Hi Bob,
>
>On Thu, May 30, 2019 at 06:39:59AM +, Feng, Bob C wrote:
>> Hi,
>>
>> Currently, we have 5 Basetools patches which are ready to push. Since 
>> we are in the soft-freeze phase, I'd like to ask for your opinions if 
>> those patches can be pushed to edk2 master.
>
>To save me the time of reading through all the threads and getting to 
>grips with all the code, could you summarise the problem these solve 
>and the impact of not including these?
>
>Is there a BZ?
>
>Regards,
>
>Leif
>
>>
>> These 5 patches are to fix the issues for the build cache feature.
>>
>> [Patch V4 2/2] BaseTools: Refactor hash tracking after checking for 
>> Sources section
>> https://edk2.groups.io/g/devel/topic/31835556#41642
>>
>> [Patch V4 1/2] BaseTools: Add a checking for Sources section in INF 
>> file
>> https://edk2.groups.io/g/devel/topic/3183#41641
>>
>> [PATCH v3 1/1] BaseTools:Extend the binary cache to support library 
>> cache
>> https://edk2.groups.io/g/devel/topic/31843505#41655
>>
>> [PATCH V5] BaseTools:Make BaseTools support new rules to generate RAW 
>> FFS FILE
>> https://edk2.groups.io/g/devel/topic/31830807#41571
>>
>> [PATCH] BaseTools:Update binary cache restore time to current time
>> https://edk2.groups.io/g/devel/topic/31819590#41468
>>
>>
>> Thanks,
>> Bob

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41677): https://edk2.groups.io/g/devel/message/41677
Mute This Topic: https://groups.io/mt/31866190/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v2] IntelFsp2Pkg/SplitFspBin.py: Support rebasing 1.x binary.

2019-05-30 Thread Zeng, Star
Reviewed-by: Star Zeng 

-Original Message-
From: Chiu, Chasel 
Sent: Thursday, May 30, 2019 12:58 PM
To: devel@edk2.groups.io
Cc: Ma, Maurice ; Desimone, Nathaniel L 
; Zeng, Star 
Subject: [PATCH v2] IntelFsp2Pkg/SplitFspBin.py: Support rebasing 1.x binary.

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

Support rebasing FSP 1.x binary.
FSP 1.x has single component in binary so not supported by split command and 
rebase can be done with the same command for rebasing FSP-T component in FSP 
2.x image.

Test: both FSP 2.x (Kabylake) and FSP 1.x (BroadwellDE) binary
  can be rebased successfully.

Cc: Maurice Ma 
Cc: Nate DeSimone 
Cc: Star Zeng 
Signed-off-by: Chasel Chiu 
---
 IntelFsp2Pkg/Tools/SplitFspBin.py   | 21 
+
 IntelFsp2Pkg/Tools/UserManuals/SplitFspBinUserManual.md | 47 
+--
 2 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/IntelFsp2Pkg/Tools/SplitFspBin.py 
b/IntelFsp2Pkg/Tools/SplitFspBin.py
index 2458231d09..15c8bebee2 100644
--- a/IntelFsp2Pkg/Tools/SplitFspBin.py
+++ b/IntelFsp2Pkg/Tools/SplitFspBin.py
@@ -1,6 +1,6 @@
 ## @ FspTool.py
 #
-# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2015 - 2019, Intel Corporation. All rights 
+reserved.
 # SPDX-License-Identifier: BSD-2-Clause-Patent  #  ## @@ -14,12 +14,12 @@ 
import argparse
 from   ctypes import *
 
 """
-This utility supports some operations for Intel FSP 2.0 image.
+This utility supports some operations for Intel FSP 1.x/2.x image.
 It supports:
-- Display FSP 2.0 information header
-- Split FSP 2.0 image into individual FSP-T/M/S/O component
-- Rebase FSP 2.0 components to a different base address
-- Generate FSP mapping C header file
+- Display FSP 1.x/2.x information header
+- Split FSP 2.x image into individual FSP-T/M/S/O component
+- Rebase FSP 1.x/2.x components to a different base address
+- Generate FSP 1.x/2.x mapping C header file
 """
 
 CopyRightHeaderFile = """/*
@@ -500,8 +500,6 @@ class FirmwareDevice:
 
 fih = None
 for fsp in self.FspList:
-if fsp.Fih.HeaderRevision < 3:
-raise Exception("ERROR: FSP 1.x is not supported by this tool 
!")
 if not fih:
 fih = fsp.Fih
 else:
@@ -713,6 +711,8 @@ def SplitFspBin (fspfile, outdir, nametemplate):
 fd.ParseFsp ()
 
 for fsp in fd.FspList:
+if fsp.Fih.HeaderRevision < 3:
+raise Exception("ERROR: FSP 1.x is not supported by the 
+ split command !")
 ftype = fsp.Type
 if not nametemplate:
 nametemplate = fspfile
@@ -742,6 +742,11 @@ def RebaseFspBin (FspBinary, FspComponent, FspBase, 
OutputDir, OutputFile):
 
 found = False
 for fsp in fd.FspList:
+# Is this FSP 1.x single binary?
+if fsp.Fih.HeaderRevision < 3:
+found = True
+ftype = 'X'
+break
 ftype = fsp.Type.lower()
 if ftype == fspcomp:
 found = True
diff --git a/IntelFsp2Pkg/Tools/UserManuals/SplitFspBinUserManual.md 
b/IntelFsp2Pkg/Tools/UserManuals/SplitFspBinUserManual.md
index 064e0ac845..06d87bbb2e 100644
--- a/IntelFsp2Pkg/Tools/UserManuals/SplitFspBinUserManual.md
+++ b/IntelFsp2Pkg/Tools/UserManuals/SplitFspBinUserManual.md
@@ -1,68 +1,71 @@
-# SplitFspBin.py is a python script to support some operations on Intel FSP 
2.0 image.
+# SplitFspBin.py is a python script to support some operations on Intel FSP 
1.x/2.x image.
 
 It supports:
 
-- Split Intel FSP 2.0 image into individual FSP-T/M/S/O component
+- Split Intel FSP 2.x image into individual FSP-T/M/S/O component
 
-- Rebase Intel FSP 2.0 components to different base addresses
+- Rebase Intel FSP 1.x/2.x components to different base addresses
 
-- Generate Intel FSP 2.0 C header file
+- Generate Intel FSP 1.x/2.x C header file
 
-- Display Intel FSP 2.0 information header for each FSP component
+- Display Intel FSP 1.x/2.x information header for each FSP component
 
-## Split Intel FSP 2.0 image
+## Split Intel FSP 2.x image
 
-To split individual FSP component in Intel FSP 2.0 image, the following
+FSP 1.x image is not supported by split command.
+To split individual FSP component in Intel FSP 2.x image, the following
 command can be used:
 
**python SplitFspBin.py split [-h] -f FSPBINARY [-o OUTPUTDIR] [-n 
NAMETEMPLATE]**
 
-For example:  
+For example:
 
`python SplitFspBin.py split -f FSP.bin`
 
It will create FSP_T.bin, FSP_M.bin and FSP_S.bin in current directory.
 
-## Rebase Intel FSP 2.0 components
+## Rebase Intel FSP 1.x/2.x components
 
-To rebase one or multiple FSP components in Intel FSP 2.0 image, the following
+To rebase one or multiple FSP components in Intel FSP 1.x/2.x image, 
+the following
 command can be used:
 
**python SplitFspBin.py rebase [-h] -f FSPBINARY 

Re: [edk2-devel] Edk2 BaseTools Patches.

2019-05-30 Thread Christian Rodriguez
Hey Leif,

I thought I'd help Bob and gather those BZs for each thread:

[Patch V4 1/2] BaseTools: Add a checking for Sources section in INF file
[Patch V4 2/2] BaseTools: Refactor hash tracking after checking for Sources 
section
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804

[PATCH v3 1/1] BaseTools:Extend the binary cache to support library cache
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1797

[PATCH V5] BaseTools:Make BaseTools support new rules to generate RAW FFS FILE
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1765

[PATCH] BaseTools:Update binary cache restore time to current time
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1742

Thanks,
Christian

>-Original Message-
>From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
>Sent: Thursday, May 30, 2019 2:28 AM
>To: Feng, Bob C 
>Cc: Andrew Fish ; Laszlo Ersek ;
>Kinney, Michael D ; devel@edk2.groups.io;
>Gao, Liming ; Shi, Steven ;
>Rodriguez, Christian ; Fan, ZhijuX
>
>Subject: Re: Edk2 BaseTools Patches.
>
>Hi Bob,
>
>On Thu, May 30, 2019 at 06:39:59AM +, Feng, Bob C wrote:
>> Hi,
>>
>> Currently, we have 5 Basetools patches which are ready to push. Since
>> we are in the soft-freeze phase, I'd like to ask for your opinions if
>> those patches can be pushed to edk2 master.
>
>To save me the time of reading through all the threads and getting to grips
>with all the code, could you summarise the problem these solve and the
>impact of not including these?
>
>Is there a BZ?
>
>Regards,
>
>Leif
>
>>
>> These 5 patches are to fix the issues for the build cache feature.
>>
>> [Patch V4 2/2] BaseTools: Refactor hash tracking after checking for
>> Sources section
>> https://edk2.groups.io/g/devel/topic/31835556#41642
>>
>> [Patch V4 1/2] BaseTools: Add a checking for Sources section in INF
>> file
>> https://edk2.groups.io/g/devel/topic/3183#41641
>>
>> [PATCH v3 1/1] BaseTools:Extend the binary cache to support library
>> cache
>> https://edk2.groups.io/g/devel/topic/31843505#41655
>>
>> [PATCH V5] BaseTools:Make BaseTools support new rules to generate RAW
>> FFS FILE
>> https://edk2.groups.io/g/devel/topic/31830807#41571
>>
>> [PATCH] BaseTools:Update binary cache restore time to current time
>> https://edk2.groups.io/g/devel/topic/31819590#41468
>>
>>
>> Thanks,
>> Bob

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41675): https://edk2.groups.io/g/devel/message/41675
Mute This Topic: https://groups.io/mt/31866190/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser

2019-05-30 Thread Krzysztof Koch
The ACPI 6.3 specification introduces a 'SPE overflow
Interrupt' field as part of the GICC structure.

Update the MADT parser to decode this field and validate
the interrupt ID used.

References:
- ACPI 6.3 Specification - January 2019
- Arm Generic Interrupt Controller Architecture Specification,
  GIC architecture version 3 and version 4, issue E
- Arm Server Base System Architecture 5.0

Signed-off-by: Krzysztof Koch 
---

Changes can be seen at: 
https://github.com/KrzysztofKoch1/edk2/tree/477_acpiview_spe_v2

Notes:
v2:
- Add include sandwich in MadtParser.h [Zhichao]
- Add references to specifications in commit message [Zhichao]

v1:
- Decode and validate SPE Overflow Interrupt field [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 86 
++--
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h | 40 
+
 2 files changed, 118 insertions(+), 8 deletions(-)

diff --git 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
index 
a1bf86ade5313f954a77b325c13394cfce133939..59c3df0cc8a080497b517baf36fc63f1e4ab866f
 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
@@ -1,17 +1,21 @@
 /** @file
   MADT table parser
 
-  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
+  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
-- ACPI 6.2 Specification - Errata A, September 2017
+- ACPI 6.3 Specification - January 2019
+- Arm Generic Interrupt Controller Architecture Specification,
+  GIC architecture version 3 and version 4, issue E
+- Arm Server Base System Architecture 5.0
 **/
 
 #include 
 #include 
 #include "AcpiParser.h"
 #include "AcpiTableParser.h"
+#include "MadtParser.h"
 
 // Local Variables
 STATIC CONST UINT8* MadtInterruptControllerType;
@@ -33,6 +37,21 @@ ValidateGICDSystemVectorBase (
   IN VOID*  Context
   );
 
+/**
+  This function validates the SPE Overflow Interrupt in the GICC.
+
+  @param [in] Ptr Pointer to the start of the field data.
+  @param [in] Context Pointer to context specific information e.g. this
+  could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateSpeOverflowInterrupt (
+  IN UINT8* Ptr,
+  IN VOID*  Context
+  );
+
 /**
   An ACPI_PARSER array describing the GICC Interrupt Controller Structure.
 **/
@@ -56,7 +75,9 @@ STATIC CONST ACPI_PARSER GicCParser[] = {
   {L"MPIDR", 8, 68, L"0x%lx", NULL, NULL, NULL, NULL},
   {L"Processor Power Efficiency Class", 1, 76, L"0x%x", NULL, NULL, NULL,
NULL},
-  {L"Reserved", 3, 77, L"%x %x %x", Dump3Chars, NULL, NULL, NULL}
+  {L"Reserved", 1, 77, L"0x%x", NULL, NULL, NULL, NULL},
+  {L"SPE overflow Interrupt", 2, 78, L"0x%x", NULL, NULL,
+ValidateSpeOverflowInterrupt, NULL}
 };
 
 /**
@@ -160,6 +181,55 @@ ValidateGICDSystemVectorBase (
   }
 }
 
+/**
+  This function validates the SPE Overflow Interrupt in the GICC.
+
+  @param [in] Ptr Pointer to the start of the field data.
+  @param [in] Context Pointer to context specific information e.g. this
+  could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateSpeOverflowInterrupt (
+  IN UINT8* Ptr,
+  IN VOID*  Context
+  )
+{
+  UINT16 SpeOverflowInterrupt;
+
+  SpeOverflowInterrupt = *(UINT16*)Ptr;
+
+  // SPE not supported by this processor
+  if (SpeOverflowInterrupt == 0) {
+return;
+  }
+
+  if ((SpeOverflowInterrupt < ARM_PPI_ID_MIN) ||
+  ((SpeOverflowInterrupt > ARM_PPI_ID_MAX) &&
+   (SpeOverflowInterrupt < ARM_PPI_ID_EXTENDED_MIN)) ||
+  (SpeOverflowInterrupt > ARM_PPI_ID_EXTENDED_MAX)) {
+IncrementErrorCount ();
+Print (
+  L"\nERROR: SPE Overflow Interrupt ID of %d is not in the allowed PPI ID "
+L"ranges of %d-%d or %d-%d (for GICv3.1 or later).",
+  SpeOverflowInterrupt,
+  ARM_PPI_ID_MIN,
+  ARM_PPI_ID_MAX,
+  ARM_PPI_ID_EXTENDED_MIN,
+  ARM_PPI_ID_EXTENDED_MAX
+);
+  } else if (SpeOverflowInterrupt != ARM_PPI_ID_PMBIRQ) {
+IncrementWarningCount();
+Print (
+  L"\nWARNING: SPE Overflow Interrupt ID of %d is not compliant with SBSA "
+L"Level 3 PPI ID assignment: %d.",
+  SpeOverflowInterrupt,
+  ARM_PPI_ID_PMBIRQ
+);
+  }
+}
+
 /**
   This function parses the ACPI MADT table.
   When trace is enabled this function parses the MADT table and
@@ -233,7 +303,7 @@ ParseAcpiMadt (
 }
 
 switch (*MadtInterruptControllerType) {
-  case EFI_ACPI_6_2_GIC: {
+  case EFI_ACPI_6_3_GIC: {
 ParseAcpi (
   TRUE,
   2,
@@ -245,7 +315,7 @@ ParseAcpiMadt (
 break;
   }
 
-  case EFI_ACPI_6_2_GICD: {
+  case EFI_ACPI_6_3_GICD: {

Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser

2019-05-30 Thread Krzysztof Koch
Hi Zhichao,

Thanks for the review. My responses are added inline as [Krzysztof].

Kind regards,

Krzysztof

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Gao, Zhichao via 
Groups.Io
Sent: Thursday, May 30, 2019 9:55
To: devel@edk2.groups.io; Krzysztof Koch 
Cc: Sami Mujawar ; Carsey, Jaben 
; Ni, Ray ; Matteo Carlini 
; Stephanie Hughes-Fitt 
; nd 
Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: ACPI 6.3 update 
for MADT parser

Sorry for late update.

Some minor comments below.
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of 
> Krzysztof Koch
> Sent: Friday, May 17, 2019 12:06 AM
> To: devel@edk2.groups.io
> Cc: sami.muja...@arm.com; Carsey, Jaben ; Ni, 
> Ray ; matteo.carl...@arm.com; Stephanie.Hughes- 
> f...@arm.com; n...@arm.com
> Subject: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: ACPI 6.3 
> update for MADT parser
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1820
> 
> The ACPI 6.3 specification introduces a 'SPE overflow Interrupt' field 
> as part of the GICC structure.
> 
> Update the MADT parser to decode this field and validate the interrupt 
> ID used.
> 
> Signed-off-by: Krzysztof Koch 
> ---
> 
> Changes can be seen at:
> https://github.com/KrzysztofKoch1/edk2/tree/477_acpiview_spe_v1
> 
> Notes:
> v1:
> - Decode and validate SPE Overflow Interrupt field [Krzysztof]
> 
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> c | 86 ++--
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> h | 35 
>  2 files changed, 113 insertions(+), 8 deletions(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> index
> a1bf86ade5313f954a77b325c13394cfce133939..59c3df0cc8a080497b517baf36f
> c63f1e4ab866f 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> +++ er.c
> @@ -1,17 +1,21 @@
>  /** @file
>MADT table parser
> 
> -  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
> +  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
>SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>@par Reference(s):
> -- ACPI 6.2 Specification - Errata A, September 2017
> +- ACPI 6.3 Specification - January 2019
> +- Arm Generic Interrupt Controller Architecture Specification,
> +  GIC architecture version 3 and version 4, issue E
> +- Arm Server Base System Architecture 5.0
>  **/
> 
>  #include 
>  #include 
>  #include "AcpiParser.h"
>  #include "AcpiTableParser.h"
> +#include "MadtParser.h"
> 
>  // Local Variables
>  STATIC CONST UINT8* MadtInterruptControllerType; @@ -33,6 +37,21 @@ 
> ValidateGICDSystemVectorBase (
>IN VOID*  Context
>);
> 
> +/**
> +  This function validates the SPE Overflow Interrupt in the GICC.
> +
> +  @param [in] Ptr Pointer to the start of the field data.
> +  @param [in] Context Pointer to context specific information e.g. this
> +  could be a pointer to the ACPI table header.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ValidateSpeOverflowInterrupt (
> +  IN UINT8* Ptr,
> +  IN VOID*  Context
> +  );
> +

>From ACPI 6.3 Table 5-60:
Statistical Profiling Extension buffer overflow GSIV. This interrupt is a level 
triggered PPI. Zero if SPE is not supported by this processor.

Seems it did descript which value is invalid. If it is mentioned in other spec, 
please help to figure out in the commit message.
Maybe I miss something. If so, please help to point.

[Krzysztof] 
This validation is carried out against ARM-related specs. I will now add them 
to the commit message as references. I'm already referencing them at the 
beginning of MadtParser.c:
- Arm Generic Interrupt Controller Architecture Specification, GIC architecture 
version 3 and version 4, issue E
- Arm Server Base System Architecture 5.0

>  /**
>An ACPI_PARSER array describing the GICC Interrupt Controller Structure.
>  **/
> @@ -56,7 +75,9 @@ STATIC CONST ACPI_PARSER GicCParser[] = {
>{L"MPIDR", 8, 68, L"0x%lx", NULL, NULL, NULL, NULL},
>{L"Processor Power Efficiency Class", 1, 76, L"0x%x", NULL, NULL, NULL,
> NULL},
> -  {L"Reserved", 3, 77, L"%x %x %x", Dump3Chars, NULL, NULL, NULL}
> +  {L"Reserved", 1, 77, L"0x%x", NULL, NULL, NULL, NULL},  {L"SPE 
> + overflow Interrupt", 2, 78, L"0x%x", NULL, NULL,
> +ValidateSpeOverflowInterrupt, NULL}
>  };
> 
>  /**
> @@ -160,6 +181,55 @@ ValidateGICDSystemVectorBase (
>}
>  }
> 
> +/**
> +  This function validates the SPE Overflow Interrupt in the GICC.
> +
> +  @param [in] Ptr Pointer to the start of the field data.
> +  @param [in] Context Pointer to context specific information e.g. this
> +  could be a pointer to the ACPI table header.
> +**/
> 

Re: [edk2-devel] Edk2 BaseTools Patches.

2019-05-30 Thread Leif Lindholm
Hi Bob,

On Thu, May 30, 2019 at 06:39:59AM +, Feng, Bob C wrote:
> Hi,
> 
> Currently, we have 5 Basetools patches which are ready to
> push. Since we are in the soft-freeze phase, I'd like to ask for
> your opinions if those patches can be pushed to edk2 master.

To save me the time of reading through all the threads and getting to
grips with all the code, could you summarise the problem these solve
and the impact of not including these?

Is there a BZ?

Regards,

Leif

> 
> These 5 patches are to fix the issues for the build cache feature.
> 
> [Patch V4 2/2] BaseTools: Refactor hash tracking after checking for Sources 
> section
> https://edk2.groups.io/g/devel/topic/31835556#41642
> 
> [Patch V4 1/2] BaseTools: Add a checking for Sources section in INF file
> https://edk2.groups.io/g/devel/topic/3183#41641
> 
> [PATCH v3 1/1] BaseTools:Extend the binary cache to support library cache
> https://edk2.groups.io/g/devel/topic/31843505#41655
> 
> [PATCH V5] BaseTools:Make BaseTools support new rules to generate RAW FFS FILE
> https://edk2.groups.io/g/devel/topic/31830807#41571
> 
> [PATCH] BaseTools:Update binary cache restore time to current time
> https://edk2.groups.io/g/devel/topic/31819590#41468
> 
> 
> Thanks,
> Bob

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41672): https://edk2.groups.io/g/devel/message/41672
Mute This Topic: https://groups.io/mt/31866190/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v2 7/7] ArmPkg/ArmLib ARM: set .fpu to let Clang 7 assemble ArmV7Support.S

2019-05-30 Thread Philippe Mathieu-Daudé
Hi Ard,

On 5/27/19 10:51 PM, Ard Biesheuvel wrote:
> Clang 7 complains about the vmsr instruction in ArmV7Support.S,
> which is only available on cores that implement some flavour of
> VFP. So set the .fpu to NEON like we do in some other places.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
> Note that the !clang alternative does not assembler for Clang-7 either
> so this is probably the most straightforward approach.
> 
>  ArmPkg/Library/ArmLib/Arm/ArmV7Support.S | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S 
> b/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S
> index 16c56f72e973..b5a6b9ea487d 100644
> --- a/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S
> +++ b/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S
> @@ -262,6 +262,7 @@ ASM_FUNC(ArmEnableVFP)
>  #ifndef __clang__
>mcr p10,#0x7,r0,c8,c0,#0
>  #else
> +  .fpuneon

If possible, can you add a short '@ reason why ...' comment on the same
line, previous to push this?

Regardless:
Reviewed-by: Philippe Mathieu-Daude 

>vmsrfpexc, r0
>  #endif
>bx  lr
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41671): https://edk2.groups.io/g/devel/message/41671
Mute This Topic: https://groups.io/mt/31813773/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser

2019-05-30 Thread Gao, Zhichao
Sorry for late update.

Some minor comments below.
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Krzysztof Koch
> Sent: Friday, May 17, 2019 12:06 AM
> To: devel@edk2.groups.io
> Cc: sami.muja...@arm.com; Carsey, Jaben ; Ni,
> Ray ; matteo.carl...@arm.com; Stephanie.Hughes-
> f...@arm.com; n...@arm.com
> Subject: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: ACPI 6.3 update for
> MADT parser
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1820
> 
> The ACPI 6.3 specification introduces a 'SPE overflow Interrupt' field as part
> of the GICC structure.
> 
> Update the MADT parser to decode this field and validate the interrupt ID
> used.
> 
> Signed-off-by: Krzysztof Koch 
> ---
> 
> Changes can be seen at:
> https://github.com/KrzysztofKoch1/edk2/tree/477_acpiview_spe_v1
> 
> Notes:
> v1:
> - Decode and validate SPE Overflow Interrupt field [Krzysztof]
> 
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> c | 86 ++--
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> h | 35 
>  2 files changed, 113 insertions(+), 8 deletions(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> index
> a1bf86ade5313f954a77b325c13394cfce133939..59c3df0cc8a080497b517baf36f
> c63f1e4ab866f 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> +++ er.c
> @@ -1,17 +1,21 @@
>  /** @file
>MADT table parser
> 
> -  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
> +  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
>SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>@par Reference(s):
> -- ACPI 6.2 Specification - Errata A, September 2017
> +- ACPI 6.3 Specification - January 2019
> +- Arm Generic Interrupt Controller Architecture Specification,
> +  GIC architecture version 3 and version 4, issue E
> +- Arm Server Base System Architecture 5.0
>  **/
> 
>  #include 
>  #include 
>  #include "AcpiParser.h"
>  #include "AcpiTableParser.h"
> +#include "MadtParser.h"
> 
>  // Local Variables
>  STATIC CONST UINT8* MadtInterruptControllerType; @@ -33,6 +37,21 @@
> ValidateGICDSystemVectorBase (
>IN VOID*  Context
>);
> 
> +/**
> +  This function validates the SPE Overflow Interrupt in the GICC.
> +
> +  @param [in] Ptr Pointer to the start of the field data.
> +  @param [in] Context Pointer to context specific information e.g. this
> +  could be a pointer to the ACPI table header.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ValidateSpeOverflowInterrupt (
> +  IN UINT8* Ptr,
> +  IN VOID*  Context
> +  );
> +

>From ACPI 6.3 Table 5-60:
Statistical Profiling Extension buffer overflow GSIV. This interrupt is a level 
triggered PPI. Zero if SPE is not supported by this processor.

Seems it did descript which value is invalid. If it is mentioned in other spec, 
please help to figure out in the commit message.
Maybe I miss something. If so, please help to point.

>  /**
>An ACPI_PARSER array describing the GICC Interrupt Controller Structure.
>  **/
> @@ -56,7 +75,9 @@ STATIC CONST ACPI_PARSER GicCParser[] = {
>{L"MPIDR", 8, 68, L"0x%lx", NULL, NULL, NULL, NULL},
>{L"Processor Power Efficiency Class", 1, 76, L"0x%x", NULL, NULL, NULL,
> NULL},
> -  {L"Reserved", 3, 77, L"%x %x %x", Dump3Chars, NULL, NULL, NULL}
> +  {L"Reserved", 1, 77, L"0x%x", NULL, NULL, NULL, NULL},  {L"SPE
> + overflow Interrupt", 2, 78, L"0x%x", NULL, NULL,
> +ValidateSpeOverflowInterrupt, NULL}
>  };
> 
>  /**
> @@ -160,6 +181,55 @@ ValidateGICDSystemVectorBase (
>}
>  }
> 
> +/**
> +  This function validates the SPE Overflow Interrupt in the GICC.
> +
> +  @param [in] Ptr Pointer to the start of the field data.
> +  @param [in] Context Pointer to context specific information e.g. this
> +  could be a pointer to the ACPI table header.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ValidateSpeOverflowInterrupt (
> +  IN UINT8* Ptr,
> +  IN VOID*  Context
> +  )
> +{
> +  UINT16 SpeOverflowInterrupt;
> +
> +  SpeOverflowInterrupt = *(UINT16*)Ptr;
> +
> +  // SPE not supported by this processor  if (SpeOverflowInterrupt ==
> + 0) {
> +return;
> +  }
> +
> +  if ((SpeOverflowInterrupt < ARM_PPI_ID_MIN) ||
> +  ((SpeOverflowInterrupt > ARM_PPI_ID_MAX) &&
> +   (SpeOverflowInterrupt < ARM_PPI_ID_EXTENDED_MIN)) ||
> +  (SpeOverflowInterrupt > ARM_PPI_ID_EXTENDED_MAX)) {
> +IncrementErrorCount ();
> +Print (
> +  L"\nERROR: SPE Overflow Interrupt ID of %d is not in the allowed PPI 
> ID "
> +L"ranges of %d-%d or %d-%d (for GICv3.1 or later).",
> +  SpeOverflowInterrupt,
> +  ARM_PPI_ID_MIN,
> +  ARM_PPI_ID_MAX,
> +  

[edk2-devel] [PATCH v1 1/1] CryptoPkg/BaseCryptLib: Wrap OpenSSL SM3 algorithm

2019-05-30 Thread Xiaoyu Lu
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1861

1. Implement OpenSSL SM3 wrapped functions in CryptSm3.c file.
2. Add wrapped SM3 functions declaration to BaseCryptLib.h file.
3. Add CryptSm3.c to each module information file.

Cc: Jian J Wang 
Signed-off-by: Xiaoyu Lu 
---
 CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf|   1 +
 CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf |   1 +
 CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf |   1 +
 CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf |   1 +
 CryptoPkg/Include/Library/BaseCryptLib.h   | 136 
 CryptoPkg/Library/BaseCryptLib/Hash/CryptSm3.c | 240 +
 6 files changed, 380 insertions(+)
 create mode 100644 CryptoPkg/Library/BaseCryptLib/Hash/CryptSm3.c

diff --git a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf 
b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
index 2a581ceac70c..964e6db73161 100644
--- a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -33,6 +33,7 @@ [Sources]
   Hash/CryptSha1.c
   Hash/CryptSha256.c
   Hash/CryptSha512.c
+  Hash/CryptSm3.c
   Hmac/CryptHmacMd5.c
   Hmac/CryptHmacSha1.c
   Hmac/CryptHmacSha256.c
diff --git a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf 
b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
index 8fdc6920ec2e..b469334b11d0 100644
--- a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
@@ -38,6 +38,7 @@ [Sources]
   Hash/CryptMd5.c
   Hash/CryptSha1.c
   Hash/CryptSha256.c
+  Hash/CryptSm3.c
   Hash/CryptSha512.c
   Hmac/CryptHmacMd5Null.c
   Hmac/CryptHmacSha1Null.c
diff --git a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf 
b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
index 9d639fd01eae..fdae33a1a52b 100644
--- a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
@@ -37,6 +37,7 @@ [Sources]
   Hash/CryptMd5.c
   Hash/CryptSha1.c
   Hash/CryptSha256.c
+  Hash/CryptSm3.c
   Hash/CryptSha512Null.c
   Hmac/CryptHmacMd5Null.c
   Hmac/CryptHmacSha1Null.c
diff --git a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf 
b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
index c9f4abb22aea..f3255ebc7e95 100644
--- a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
@@ -37,6 +37,7 @@ [Sources]
   Hash/CryptMd5.c
   Hash/CryptSha1.c
   Hash/CryptSha256.c
+  Hash/CryptSm3.c
   Hash/CryptSha512Null.c
   Hmac/CryptHmacMd5Null.c
   Hmac/CryptHmacSha1Null.c
diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h 
b/CryptoPkg/Include/Library/BaseCryptLib.h
index 84374b283b7a..19d1afe3c8c0 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -45,6 +45,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define SHA512_DIGEST_SIZE  64
 
 ///
+/// SM3 digest size in bytes
+///
+#define SM3_256_DIGEST_SIZE 32
+
+///
 /// TDES block size in bytes
 ///
 #define TDES_BLOCK_SIZE 8
@@ -885,6 +890,137 @@ Sha512HashAll (
   OUT  UINT8   *HashValue
   );
 
+/**
+  Retrieves the size, in bytes, of the context buffer required for SM3 hash 
operations.
+
+  @return  The size, in bytes, of the context buffer required for SM3 hash 
operations.
+
+**/
+UINTN
+EFIAPI
+Sm3GetContextSize (
+  VOID
+  );
+
+/**
+  Initializes user-supplied memory pointed by Sm3Context as SM3 hash context 
for
+  subsequent use.
+
+  If Sm3Context is NULL, then return FALSE.
+
+  @param[out]  Sm3Context  Pointer to SM3 context being initialized.
+
+  @retval TRUE   SM3 context initialization succeeded.
+  @retval FALSE  SM3 context initialization failed.
+
+**/
+BOOLEAN
+EFIAPI
+Sm3Init (
+  OUT  VOID  *Sm3Context
+  );
+
+/**
+  Makes a copy of an existing SM3 context.
+
+  If Sm3Context is NULL, then return FALSE.
+  If NewSm3Context is NULL, then return FALSE.
+  If this interface is not supported, then return FALSE.
+
+  @param[in]  Sm3Context Pointer to SM3 context being copied.
+  @param[out] NewSm3Context  Pointer to new SM3 context.
+
+  @retval TRUE   SM3 context copy succeeded.
+  @retval FALSE  SM3 context copy failed.
+  @retval FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+Sm3Duplicate (
+  IN   CONST VOID  *Sm3Context,
+  OUT  VOID*NewSm3Context
+  );
+
+/**
+  Digests the input data and updates SM3 context.
+
+  This function performs SM3 digest on a data buffer of the specified size.
+  It can be called multiple times to compute the digest of long or 
discontinuous data streams.
+  SM3 context should be already correctly initialized by Sm3Init(), and should 
not be finalized
+  by Sm3Final(). Behavior with invalid context is undefined.
+
+  If Sm3Context is NULL, then return FALSE.
+
+  @param[in, out]  Sm3Context Pointer to the SM3 context.
+  @param[in]   Data   Pointer to the buffer containing the data to 
be hashed.
+  @param[in]   DataSize   Size 

[edk2-devel] [PATCH v1] MdeModulePkg/AhciPei: Fix device cannot be found in non-S3 path

2019-05-30 Thread Wu, Hao A
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1862

Current implementation of function AhciModeInitialization() has an
incorrect assumption that the value in the CAP (offset 00h) register will
always be greater than the highest bit set for the value in the PI (offset
0Ch) register.

This will lead to an issue that hard disk devices may not be found in the
non-S3 boot path for some AHCI controller capabilities.

More specifically, variable 'PortInitializeBitMap' will have the value
from 'Private->PortBitMap', which will be 0x in non-S3 boot path.
When the CAP register is of value 0x1 and PI register with value 0x4
(meaning port 2 is available), the current logic will only enumerate port
0. And the device attached behind port 2 will not be enumerated.

To address this issue, variable 'PortInitializeBitMap' will now take the
bitwise and result between 'Private->PortBitMap' and the value read from
the PI register.

Please note that there will be no function impact for S3 path, since in
this case, the bits being set in 'Private->PortBitMap' will be a subset
of the bits being set in the PI register. Their bitwise and operation will
still be the value of 'Private->PortBitMap'.

Cc: Ray Ni 
Cc: Maggie Chu 
Cc: Jian J Wang 
Signed-off-by: Hao A Wu 
---
 MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c 
b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
index 7287f8290e..8c491bd138 100644
--- a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
+++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
@@ -1713,7 +1713,7 @@ AhciModeInitialization (
   MaxPortNumber = MIN (MaxPortNumber, 
(UINT8)(UINTN)(HighBitSet32(PortImplementBitMap) + 1));
   MaxPortNumber = MIN (MaxPortNumber, AhciGetNumberOfPortsFromMap 
(Private->PortBitMap));
 
-  PortInitializeBitMap = Private->PortBitMap;
+  PortInitializeBitMap = Private->PortBitMap & PortImplementBitMap;
   AhciRegisters= >AhciRegisters;
   DeviceIndex  = 0;
   //
@@ -1721,6 +1721,13 @@ AhciModeInitialization (
   //
   for (PortIndex = 1; PortIndex <= MaxPortNumber; PortIndex ++) {
 Status = AhciGetPortFromMap (PortInitializeBitMap, PortIndex, );
+if (EFI_ERROR (Status)) {
+  //
+  // No more available port, just break out of the loop.
+  //
+  break;
+}
+
 if ((PortImplementBitMap & (BIT0 << Port)) != 0) {
   //
   // Initialize FIS Base Address Register and Command List Base Address
-- 
2.12.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41668): https://edk2.groups.io/g/devel/message/41668
Mute This Topic: https://groups.io/mt/31866595/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v2 6/7] EmbeddedPkg: Fix DwEmmc SendCommand polling

2019-05-30 Thread Loh, Tien Hock
> -Original Message-
> From: Haojian Zhuang 
> Sent: Thursday, May 30, 2019 3:06 PM
> To: Leif Lindholm 
> Cc: Loh, Tien Hock ; devel@edk2.groups.io;
> thlo...@gmail.com; Ard Biesheuvel 
> Subject: Re: [PATCH v2 6/7] EmbeddedPkg: Fix DwEmmc SendCommand
> polling
> 
> On Tue, May 28, 2019 at 07:04:09PM +0100, Leif Lindholm wrote:
> > +Haojian,
> >
> > Haojian - since you are the original author, can you comment on the
> > delays? Are these silicon bug workarounds (so we need to add a Pcd),
> > or does these changes work on your platforms too?
> 
> I'm not in the loop, so I missed the patch series.
> 
> The patch series can't work on my platform for the eMMC. Although a
> variable is created to identify whether it's a SD or eMMC device, it doesn't
> identify the eMMC device by the right way. So the eMMC device isn't
> initialized successfully on my platform.
> 
> 1. Since MMC framework could identify whether it's eMMC device or SD
> device, we need to make device driver gets this kind of information from the
> MMC framework. And we need to support multiple eMMC/SD instances in
> MMC framework.

Yeah my bad I didn't read through the SD/MMC specs on that. Now I check the 
specs and you're right, the information should be read from MMC framework.

> 
> 2. I sent a patch series to support both eMMC device and SD device before.
> https://edk2.groups.io/g/devel/message/28572
> &&
> https://edk2.groups.io/g/devel/message/28615
> Maybe it's missed. Could you help to review that patch series?

> 
> Best Regards
> Haojian
> 
> >
> > Regards,
> >
> > Leif
> >
> > On Mon, May 27, 2019 at 05:30:27PM +0800, tien.hock@intel.com
> wrote:
> > > From: "Tien Hock, Loh" 
> > >
> > > Change SendCommand polling mode to remove unnecessary delay, and
> > > check for transfer done only when block data is to be read/write.
> > > This would also increase performance slightly.
> > >
> > > Signed-off-by: "Tien Hock, Loh" 
> > > Cc: Leif Lindholm 
> > > Cc: Ard Biesheuvel 
> > > ---
> > >  EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c | 43
> +++-
> > >  1 file changed, 33 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
> > > b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
> > > index c6c8e04917..b57833458f 100644
> > > --- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
> > > +++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
> > > @@ -286,16 +286,13 @@ SendCommand (
> > >  DWEMMC_INT_RCRC | DWEMMC_INT_RE;
> > >ErrMask |= DWEMMC_INT_DCRC | DWEMMC_INT_DRT |
> DWEMMC_INT_SBE;
> > >do {
> > > -MicroSecondDelay(500);
> > >  Data = MmioRead32 (DWEMMC_RINTSTS);
> > > -
> > > -if (Data & ErrMask) {
> > > -  return EFI_DEVICE_ERROR;
> > > -}
> > > -if (Data & DWEMMC_INT_DTO) { // Transfer Done
> > > -  break;
> > > -}
> > >} while (!(Data & DWEMMC_INT_CMD_DONE));
> > > +
> > > +  if (Data & ErrMask) {
> > > +return EFI_DEVICE_ERROR;
> > > +  }
> > > +
> > >return EFI_SUCCESS;
> > >  }
> > >
> > > @@ -550,8 +547,9 @@ DwEmmcReadBlockData (
> > >)
> > >  {
> > >EFI_STATUS  Status;
> > > -  UINT32  DescPages, CountPerPage, Count;
> > > +  UINT32  DescPages, CountPerPage, Count, ErrMask;
> > >EFI_TPL Tpl;
> > > +  UINTN Rintsts = 0;
> > >
> > >Tpl = gBS->RaiseTPL (TPL_NOTIFY);
> > >
> > > @@ -574,6 +572,18 @@ DwEmmcReadBlockData (
> > >  DEBUG ((DEBUG_ERROR, "Failed to read data,
> mDwEmmcCommand:%x, mDwEmmcArgument:%x, Status:%r\n",
> mDwEmmcCommand, mDwEmmcArgument, Status));
> > >  goto out;
> > >}
> > > +
> > > +  while(!((MmioRead32(DWEMMC_RINTSTS) & (DWEMMC_INT_DTO
> {
> > > +Rintsts = MmioRead32 (DWEMMC_RINTSTS);  }  ErrMask =
> > > + DWEMMC_INT_EBE | DWEMMC_INT_HLE | DWEMMC_INT_RTO |
> > > +DWEMMC_INT_RCRC | DWEMMC_INT_RE |
> DWEMMC_INT_DCRC |
> > > +DWEMMC_INT_DRT | DWEMMC_INT_SBE;
> > > +
> > > +  if (Rintsts & ErrMask) {
> > > +Status = EFI_DEVICE_ERROR;
> > > +goto out;
> > > +  }
> > >  out:
> > >// Restore Tpl
> > >gBS->RestoreTPL (Tpl);
> > > @@ -589,8 +599,9 @@ DwEmmcWriteBlockData (
> > >)
> > >  {
> > >EFI_STATUS  Status;
> > > -  UINT32  DescPages, CountPerPage, Count;
> > > +  UINT32  DescPages, CountPerPage, Count, ErrMask;
> > >EFI_TPL Tpl;
> > > +  UINTN Rintsts = 0;
> > >
> > >Tpl = gBS->RaiseTPL (TPL_NOTIFY);
> > >
> > > @@ -613,6 +624,18 @@ DwEmmcWriteBlockData (
> > >  DEBUG ((DEBUG_ERROR, "Failed to write data,
> mDwEmmcCommand:%x, mDwEmmcArgument:%x, Status:%r\n",
> mDwEmmcCommand, mDwEmmcArgument, Status));
> > >  goto out;
> > >}
> > > +
> > > +  while(!((MmioRead32(DWEMMC_RINTSTS) & (DWEMMC_INT_DTO
> {
> > > +Rintsts = MmioRead32 (DWEMMC_RINTSTS);  }  ErrMask =
> > > + DWEMMC_INT_EBE | DWEMMC_INT_HLE | DWEMMC_INT_RTO |
> > > +DWEMMC_INT_RCRC | DWEMMC_INT_RE |
> DWEMMC_INT_DCRC |
> > > +DWEMMC_INT_DRT | DWEMMC_INT_SBE;
> > > +
> > > 

[edk2-devel] Edk2 BaseTools Patches.

2019-05-30 Thread Bob Feng
Hi,

Currently, we have 5 Basetools patches which are ready to push. Since we are in 
the soft-freeze phase, I'd like to ask for your opinions if those patches can 
be pushed to edk2 master.

These 5 patches are to fix the issues for the build cache feature.

[Patch V4 2/2] BaseTools: Refactor hash tracking after checking for Sources 
section
https://edk2.groups.io/g/devel/topic/31835556#41642

[Patch V4 1/2] BaseTools: Add a checking for Sources section in INF file
https://edk2.groups.io/g/devel/topic/3183#41641

[PATCH v3 1/1] BaseTools:Extend the binary cache to support library cache
https://edk2.groups.io/g/devel/topic/31843505#41655

[PATCH V5] BaseTools:Make BaseTools support new rules to generate RAW FFS FILE
https://edk2.groups.io/g/devel/topic/31830807#41571

[PATCH] BaseTools:Update binary cache restore time to current time
https://edk2.groups.io/g/devel/topic/31819590#41468


Thanks,
Bob

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41666): https://edk2.groups.io/g/devel/message/41666
Mute This Topic: https://groups.io/mt/31866190/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v3 1/1] BaseTools:Extend the binary cache to support library cache

2019-05-30 Thread Steven Shi
Yes. The "entend" should be "extend", please help to fix it when pushing. 
Thanks!

Steven Shi


> -Original Message-
> From: Feng, Bob C
> Sent: Thursday, May 30, 2019 10:44 AM
> To: devel@edk2.groups.io; Shi, Steven 
> Cc: Gao, Liming ; Rodriguez, Christian
> ; Fan, ZhijuX 
> Subject: RE: [edk2-devel] [PATCH v3 1/1] BaseTools:Extend the binary cache to
> support library cache
> 
> The "entend" should be "extend", right? I can fix it when I push this patch.
> 
> Reviewed-by: Bob Feng 
> 
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Steven Shi
> Sent: Thursday, May 30, 2019 10:31 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming ; Feng, Bob C ;
> Rodriguez, Christian ; Fan, ZhijuX
> 
> Subject: [edk2-devel] [PATCH v3 1/1] BaseTools:Extend the binary cache to
> support library cache
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1797
> 
> Current binary cache doesn't support to save and restore the library module.
> If a driver module cache miss happen, all its dependency library modules
> need rebuild which is very time-consuming. This patch is to entend the binary
> cache to support library.
> 
> Cc: Liming Gao 
> Cc: Bob Feng 
> Cc: Christian Rodriguez 
> Signed-off-by: Steven Shi 
> ---
>  BaseTools/Source/Python/AutoGen/AutoGen.py | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> b/BaseTools/Source/Python/AutoGen/AutoGen.py
> index a5bef4f7c6..7b35f837f5 100644
> --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> @@ -3906,6 +3906,12 @@ class ModuleAutoGen(AutoGen):
>  ModuleFile = path.join(self.OutputDir, self.Name + '.inf')
>  if os.path.exists(ModuleFile):
>  shutil.copy2(ModuleFile, FileDir)
> +else:
> +OutputDir = self.OutputDir.replace('\\', '/').strip('/')
> +DebugDir = self.DebugDir.replace('\\', '/').strip('/')
> +for Item in self.CodaTargetList:
> +File = Item.Target.Path.replace('\\', 
> '/').strip('/').replace(DebugDir,
> '').replace(OutputDir, '').strip('/')
> +self.OutputFile.add(File)
>  if not self.OutputFile:
>  Ma = self.BuildDatabase[self.MetaFile, self.Arch, 
> self.BuildTarget,
> self.ToolChain]
>  self.OutputFile = Ma.Binaries
> --
> 2.17.1.windows.2
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41665): https://edk2.groups.io/g/devel/message/41665
Mute This Topic: https://groups.io/mt/31843505/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [Patch V4 2/2] BaseTools: Refactor hash tracking after checking for Sources section

2019-05-30 Thread Bob Feng
Reviewed-by: Bob Feng 

-Original Message-
From: Rodriguez, Christian 
Sent: Thursday, May 30, 2019 12:27 AM
To: devel@edk2.groups.io
Cc: Feng, Bob C ; Gao, Liming ; 
Zhu, Yonghong 
Subject: [Patch V4 2/2] BaseTools: Refactor hash tracking after checking for 
Sources section

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

After adding a check to see if [Sources] section lists all the header type 
files of a module, track module and library hashes for --hash feature. If above 
check is not in compilance for a library or module, force hash invalidation on 
that library or module.

Signed-off-by: Christian Rodriguez 
Cc: Bob Feng 
Cc: Liming Gao 
Cc: Yonghong Zhu 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py   |  6 +-
 BaseTools/Source/Python/AutoGen/GenMake.py   |  6 ++
 BaseTools/Source/Python/Common/GlobalData.py |  3 +-
 BaseTools/Source/Python/build/build.py   | 65 
 4 files changed, 53 insertions(+), 27 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index a5bef4f7c6..a376bc24d6 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -3989,7 +3989,8 @@ class ModuleAutoGen(AutoGen):
 for LibraryAutoGen in self.LibraryAutoGenList:
 LibraryAutoGen.CreateMakeFile()
 
-if self.CanSkip():
+# Don't enable if hash feature enabled, CanSkip uses timestamps to 
determine build skipping
+if not GlobalData.gUseHashCache and self.CanSkip():
 return
 
 if len(self.CustomMakefile) == 0:
@@ -4032,7 +4033,8 @@ class ModuleAutoGen(AutoGen):
 for LibraryAutoGen in self.LibraryAutoGenList:
 LibraryAutoGen.CreateCodeFile()
 
-if self.CanSkip():
+# Don't enable if hash feature enabled, CanSkip uses timestamps to 
determine build skipping
+if not GlobalData.gUseHashCache and self.CanSkip():
 return
 
 AutoGenList = []
diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 5c992d7c26..212ca0fa7f 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -935,10 +935,16 @@ cleanlib:
 continue
 headerFileDependencySet.add(aFileName)
 
+# Ensure that gModuleBuildTracking has been initialized per 
architecture
+if self._AutoGenObject.Arch not in GlobalData.gModuleBuildTracking:
+GlobalData.gModuleBuildTracking[self._AutoGenObject.Arch] = 
+ dict()
+
 # Check if a module dependency header file is missing from the 
module's MetaFile
 for aFile in headerFileDependencySet:
 if aFile in headerFilesInMetaFileSet:
 continue
+if GlobalData.gUseHashCache:
+
GlobalData.gModuleBuildTracking[self._AutoGenObject.Arch][self._AutoGenObject] 
= 'FAIL_METAFILE'
 EdkLogger.warn("build","Module MetaFile [Sources] is missing local 
header!",
 ExtraData = "Local Header: " + aFile + " not found in 
" + self._AutoGenObject.MetaFile.Path
 )
diff --git a/BaseTools/Source/Python/Common/GlobalData.py 
b/BaseTools/Source/Python/Common/GlobalData.py
index 95e28a988f..bd45a43728 100644
--- a/BaseTools/Source/Python/Common/GlobalData.py
+++ b/BaseTools/Source/Python/Common/GlobalData.py
@@ -110,7 +110,8 @@ gEnableGenfdsMultiThread = False  gSikpAutoGenCache = set()
 
 # Dictionary for tracking Module build status as success or failure -# False 
-> Fail : True -> Success
+# Top Dict: Key: Arch Type  Value: Dictionary
+# Second Dict:  Key: AutoGen ObjValue: 'SUCCESS'\'FAIL'\'FAIL_METAFILE'
 gModuleBuildTracking = dict()
 
 # Dictionary of booleans that dictate whether a module or diff --git 
a/BaseTools/Source/Python/build/build.py 
b/BaseTools/Source/Python/build/build.py
index 80ceb98310..0855d4561c 100644
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -625,8 +625,16 @@ class BuildTask:
 BuildTask._ErrorFlag.set()
 BuildTask._ErrorMessage = "%s broken\n%s [%s]" % \
   (threading.currentThread().getName(), 
Command, WorkingDir)
-if self.BuildItem.BuildObject in GlobalData.gModuleBuildTracking and 
not BuildTask._ErrorFlag.isSet():
-GlobalData.gModuleBuildTracking[self.BuildItem.BuildObject] = True
+
+# Set the value used by hash invalidation flow in 
GlobalData.gModuleBuildTracking to 'SUCCESS'
+# If Module or Lib is being tracked, it did not fail header check 
test, and built successfully
+if (self.BuildItem.BuildObject.Arch in GlobalData.gModuleBuildTracking 
and
+   self.BuildItem.BuildObject in 
GlobalData.gModuleBuildTracking[self.BuildItem.BuildObject.Arch] and
+   

Re: [edk2-devel] [Patch V4 1/2] BaseTools: Add a checking for Sources section in INF file

2019-05-30 Thread Bob Feng
Reviewed-by: Bob Feng 

-Original Message-
From: Rodriguez, Christian 
Sent: Thursday, May 30, 2019 12:27 AM
To: devel@edk2.groups.io
Cc: Feng, Bob C ; Gao, Liming ; 
Zhu, Yonghong 
Subject: [Patch V4 1/2] BaseTools: Add a checking for Sources section in INF 
file

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

Add a check to see if [Sources] section lists all the header type files of a 
module. Performance impact should be minimal with this patch since information 
is already being fetched for Makefile purposes. All other information is 
already cached in memory. No extra IO time is needed.

Signed-off-by: Christian Rodriguez 
Cc: Bob Feng 
Cc: Liming Gao 
Cc: Yonghong Zhu 
---
 BaseTools/Source/Python/AutoGen/GenMake.py | 38 
 1 file changed, 38 insertions(+)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 0e0f9fd9b0..5c992d7c26 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -905,6 +905,44 @@ cleanlib:
 ForceIncludedFile,
 self._AutoGenObject.IncludePathList + 
self._AutoGenObject.BuildOptionIncPathList
 )
+
+# Check if header files are listed in metafile
+# Get a list of unique module header source files from MetaFile
+headerFilesInMetaFileSet = set()
+for aFile in self._AutoGenObject.SourceFileList:
+aFileName = str(aFile)
+if not aFileName.endswith('.h'):
+continue
+headerFilesInMetaFileSet.add(aFileName.lower())
+
+# Get a list of unique module autogen files
+localAutoGenFileSet = set()
+for aFile in self._AutoGenObject.AutoGenFileList:
+localAutoGenFileSet.add(str(aFile).lower())
+
+# Get a list of unique module dependency header files
+# Exclude autogen files and files not in the source directory
+headerFileDependencySet = set()
+localSourceDir = str(self._AutoGenObject.SourceDir).lower()
+for Dependency in FileDependencyDict.values():
+for aFile in Dependency:
+aFileName = str(aFile).lower()
+if not aFileName.endswith('.h'):
+continue
+if aFileName in localAutoGenFileSet:
+continue
+if localSourceDir not in aFileName:
+continue
+headerFileDependencySet.add(aFileName)
+
+# Check if a module dependency header file is missing from the 
module's MetaFile
+for aFile in headerFileDependencySet:
+if aFile in headerFilesInMetaFileSet:
+continue
+EdkLogger.warn("build","Module MetaFile [Sources] is missing local 
header!",
+ExtraData = "Local Header: " + aFile + " not found in 
" + self._AutoGenObject.MetaFile.Path
+)
+
 DepSet = None
 for File,Dependency in FileDependencyDict.items():
 if not Dependency:
--
2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41663): https://edk2.groups.io/g/devel/message/41663
Mute This Topic: https://groups.io/mt/3183/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-