Re: [edk2] PerformancePkg on multiple platform -

[prabin ca]

Hi Andrew,

I have already enabled that PCD in my DSC file, because of that its working
in some platform.

What I'm doing is building my DP.efi and DXE driver from that dsc file and
testing it from UEFI shell.

thanks for your help as well.

On Sat, Aug 4, 2018 at 12:08 AM, Andrew Fish  wrote:

> Prabin,
>
> There is a PCD setting to configure performance collection.
>
> PCDs are defined in the .DEC file and it looks like the default is zero
> and that means disabled.
> MdePkg/MdePkg.dec
> ...
>   ## The mask is used to control PerformanceLib behavior.
>   #  BIT0 - Enable Performance Measurement.
>   # @Prompt Performance Measurement Property.
>   # @Expression  0x8002 | (gEfiMdePkgTokenSpaceGuid.
> PcdPerformanceLibraryPropertyMask & 0xFE) == 0
>   gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyM
> ask|0|UINT8|0x0009
>
> If you search the code you will see some platforms enabling performance
> measurement in their DSC files.
>
> /Volumes/Case/UDK2018(vUDK2018)>git grep gEfiMdePkgTokenSpaceGuid.
> PcdPerformanceLibraryPropertyMask -- *.dsc
> BeagleBoardPkg/BeagleBoardPkg.dsc:272:
> *gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask*|1
> EmbeddedPkg/EmbeddedPkg.dsc:175:
> *gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask*|0
> QuarkPlatformPkg/Quark.dsc:373:
> *gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask*|0x1
> QuarkPlatformPkg/Quark.dsc:376:
> *gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask*|0x00
> QuarkPlatformPkg/QuarkMin.dsc:334:
> *gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask*|0x1
> QuarkPlatformPkg/QuarkMin.dsc:337:
> *gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask*|0x00
> Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc:682:
> *gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask*|0x1
> Vlv2TbltDevicePkg/PlatformPkgIA32.dsc:682:
> *gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask*|0x1
> Vlv2TbltDevicePkg/PlatformPkgX64.dsc:682:
> *gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask*|0x1
>
> So I'd check that 1st. If that does not work I recommend
> adding -report-file=build.log to your build command. You can look at the
> given driver/app you care about and see what the PCD settings are and which
> instance of the PerformanceLib got linked.
>
> Thanks,
>
> Andrew Fish
>
> On Aug 3, 2018, at 11:17 AM, Laszlo Ersek  wrote:
>
> Hi Prabin,
>
> On 08/03/18 09:29, prabin ca wrote:
>
> Hi Team,
>
> I’m new to uefi and edk. Currently I’m trying to get performance of my dxe
> driver using PerformancePkg of EDK source code.
>
> I’m using perf_start and perf_end T respected check points, it’s hot build
> and tested well in 2/3 platform. It’s giving proper response.
>
> But when I’m Checking with multiple platform, in some of platforms it’s
> not working and got hang in uefi she’ll itself.
>
> Please help me to resolve asap, it will be really helpful.
>
>
> I can only give you some hints, because thus far I have also failed to
> figure out how performance measurements should be enabled for a platform
> from scratch.
>
> Earlier I thought that an interested platform should include modules
> from PerformancePkg. I got some good results that way; however:
>
> - it wouldn't work for AARCH64, and so I filed
>  
>
> - ultimately PerformancePkg was removed in commit 922c1e94e4bb
>  completely. (And for that reason I now closed TianoCore#679 as
>  WONTFIX.)
>
> Commit 922c1e94e4bb doesn't really give pointers for enabling
> performance measurements in a platform -- it refers the user to the DP
> shell command instead of the standalone DP application, but that's only
> for displaying the stats, not for recording them.
>
> One document where I found information is the Intel whitepaper
>
>  A Tour Beyond BIOS
>  Implementing Profiling in with EDK II
>
> (just google it). "Part III – Performance Profile" is relevant.
>
> The first section of that talks about enabling ACPI FPDT (ACPI Firmware
> Performance Data Table) support in edk2, for exposing firmware
> performance data to the OS. Again, that's not about *recording* the stats.
>
> The second section of the same chapter seems to describe how stats can
> be recorded however. AIUI, the DXE Core can provide the PERFORMANCE[_EX]
> protocols, if the DxeCorePerformanceLib instance is linked into it by
> the platform DSC file. In turn DXE modules can send measurements to the
> protocol via the PerformanceLib APIs / DxePerformanceLib instance.
>
> In fact, the relevant library INF files have good documentation, we just
> have to know what to look at.
>
> Recording stats in the PEI phase, via HOBs:
> - MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
>
> Recording stats in the DXE phase (protocol provider and consumer):
> - MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
> - MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
>
> Recording stats in SMM (protocol provider and consumer):
> 

Re: [edk2] PerformancePkg on multiple platform

[prabin ca]

Hi Laszlo,

Thank you for your valid input, Its really help full for me. I will look in
the " A Tour Beyond BIOSImplementing Profiling in with EDK II" with
your points.

have a nice day.

On Fri, Aug 3, 2018 at 11:47 PM, Laszlo Ersek  wrote:

> Hi Prabin,
>
> On 08/03/18 09:29, prabin ca wrote:
> > Hi Team,
> >
> > I’m new to uefi and edk. Currently I’m trying to get performance of my
> dxe driver using PerformancePkg of EDK source code.
> >
> > I’m using perf_start and perf_end T respected check points, it’s hot
> build and tested well in 2/3 platform. It’s giving proper response.
> >
> > But when I’m Checking with multiple platform, in some of platforms it’s
> not working and got hang in uefi she’ll itself.
> >
> > Please help me to resolve asap, it will be really helpful.
>
> I can only give you some hints, because thus far I have also failed to
> figure out how performance measurements should be enabled for a platform
> from scratch.
>
> Earlier I thought that an interested platform should include modules
> from PerformancePkg. I got some good results that way; however:
>
> - it wouldn't work for AARCH64, and so I filed
>   
>
> - ultimately PerformancePkg was removed in commit 922c1e94e4bb
>   completely. (And for that reason I now closed TianoCore#679 as
>   WONTFIX.)
>
> Commit 922c1e94e4bb doesn't really give pointers for enabling
> performance measurements in a platform -- it refers the user to the DP
> shell command instead of the standalone DP application, but that's only
> for displaying the stats, not for recording them.
>
> One document where I found information is the Intel whitepaper
>
>   A Tour Beyond BIOS
>   Implementing Profiling in with EDK II
>
> (just google it). "Part III – Performance Profile" is relevant.
>
> The first section of that talks about enabling ACPI FPDT (ACPI Firmware
> Performance Data Table) support in edk2, for exposing firmware
> performance data to the OS. Again, that's not about *recording* the stats.
>
> The second section of the same chapter seems to describe how stats can
> be recorded however. AIUI, the DXE Core can provide the PERFORMANCE[_EX]
> protocols, if the DxeCorePerformanceLib instance is linked into it by
> the platform DSC file. In turn DXE modules can send measurements to the
> protocol via the PerformanceLib APIs / DxePerformanceLib instance.
>
> In fact, the relevant library INF files have good documentation, we just
> have to know what to look at.
>
> Recording stats in the PEI phase, via HOBs:
> - MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
>
> Recording stats in the DXE phase (protocol provider and consumer):
> - MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
> - MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
>
> Recording stats in SMM (protocol provider and consumer):
> - MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.inf
> - MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.inf
>
> Fetching the collected stats (for display or otherwise):
> - MdeModulePkg/Library/DxeSmmPerformanceLib/DxeSmmPerformanceLib.inf
>
> "Do nothing" library instance for all module types:
> - MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf
>
>
> Now obviously this doesn't explain why it works for you on some
> platforms and why it doesn't on others. You haven't shared much
> information about the platforms in question. I can make one guess: if
> the performance protocol provided by the platform's DXE Core
> (DxeCorePerformanceLib) is incompatible with the DxePerformanceLib
> instance that is linked into your DXE driver, then communication between
> them will likely fail.
>
> To my understanding, the performance protocol is not standard (PI or
> UEFI), so it's likely best if you build your DXE driver together with
> the DXE Core, on all platforms that you want to check. It's probably
> unsafe to link a DXE driver against DxePerformanceLib (rather than
> BasePerformanceLibNull) and run it on a random platform.
>
> Hopefully this helps a little.
> Laszlo
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch] BaseTools/BinToPcd: Open output file as text file

[Kinney, Michael D]

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

Undo changes from following commit:

https://github.com/tianocore/edk2/commit/83964ebc5e74549d6efc7134af19150a0b2079aa

Change the open mode for the output file from 'wb' to 'w' so the
output file is written as a text file and not a binary file.

This resolves the issue where the text file was not writable from
Python 3.x and also removes b'' from output file when the string
was encoded as a bytearray.

Cc: YanYan Sun 
Cc: Yonghong Zhu 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney 
---
 BaseTools/Scripts/BinToPcd.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Scripts/BinToPcd.py b/BaseTools/Scripts/BinToPcd.py
index 1495a36933..316cc6117f 100644
--- a/BaseTools/Scripts/BinToPcd.py
+++ b/BaseTools/Scripts/BinToPcd.py
@@ -70,8 +70,7 @@ if __name__ == '__main__':
 #
 # Return a PCD value of the form '{0x01, 0x02, ...}' along with the 
PCD length in bytes
 #
-PcdValue = '{' + ', '.join (['0x{Byte:02X}'.format (Byte = Item) for 
Item in Buffer]) + '}'
-return PcdValue.encode (), len (Buffer)
+return '{' + (', '.join (['0x{Byte:02X}'.format (Byte = Item) for Item 
in Buffer])) + '}', len (Buffer)
 
 #
 # Create command line argument parser object
@@ -81,7 +80,7 @@ if __name__ == '__main__':
   conflict_handler = 'resolve')
 parser.add_argument ("-i", "--input", dest = 'InputFile', type = 
argparse.FileType ('rb'), action='append', required = True,
  help = "Input binary filename.  Multiple input files 
are combined into a single PCD.")
-parser.add_argument ("-o", "--output", dest = 'OutputFile', type = 
argparse.FileType ('wb'),
+parser.add_argument ("-o", "--output", dest = 'OutputFile', type = 
argparse.FileType ('w'),
  help = "Output filename for PCD value or PCD 
statement")
 parser.add_argument ("-p", "--pcd", dest = 'PcdName', type = 
ValidatePcdName,
  help = "Name of the PCD in the form 
.")
-- 
2.14.2.windows.3

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

Re: [edk2] PerformancePkg on multiple platform -

[Andrew Fish]

Prabin,

There is a PCD setting to configure performance collection. 

PCDs are defined in the .DEC file and it looks like the default is zero and 
that means disabled. 
MdePkg/MdePkg.dec
...
  ## The mask is used to control PerformanceLib behavior.
  #  BIT0 - Enable Performance Measurement.
  # @Prompt Performance Measurement Property.
  # @Expression  0x8002 | 
(gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask & 0xFE) == 0
  gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|0|UINT8|0x0009

If you search the code you will see some platforms enabling performance 
measurement in their DSC files. 

/Volumes/Case/UDK2018(vUDK2018)>git grep 
gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask -- *.dsc
BeagleBoardPkg/BeagleBoardPkg.dsc:272:  
gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|1
EmbeddedPkg/EmbeddedPkg.dsc:175:  
gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|0
QuarkPlatformPkg/Quark.dsc:373:  
gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|0x1
QuarkPlatformPkg/Quark.dsc:376:  
gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|0x00
QuarkPlatformPkg/QuarkMin.dsc:334:  
gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|0x1
QuarkPlatformPkg/QuarkMin.dsc:337:  
gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|0x00
Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc:682:  
gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|0x1
Vlv2TbltDevicePkg/PlatformPkgIA32.dsc:682:  
gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|0x1
Vlv2TbltDevicePkg/PlatformPkgX64.dsc:682:  
gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|0x1

So I'd check that 1st. If that does not work I recommend adding 
-report-file=build.log to your build command. You can look at the given 
driver/app you care about and see what the PCD settings are and which instance 
of the PerformanceLib got linked. 

Thanks,

Andrew Fish

> On Aug 3, 2018, at 11:17 AM, Laszlo Ersek  wrote:
> 
> Hi Prabin,
> 
> On 08/03/18 09:29, prabin ca wrote:
>> Hi Team,
>> 
>> I’m new to uefi and edk. Currently I’m trying to get performance of my dxe 
>> driver using PerformancePkg of EDK source code.
>> 
>> I’m using perf_start and perf_end T respected check points, it’s hot build 
>> and tested well in 2/3 platform. It’s giving proper response.
>> 
>> But when I’m Checking with multiple platform, in some of platforms it’s not 
>> working and got hang in uefi she’ll itself.
>> 
>> Please help me to resolve asap, it will be really helpful.
> 
> I can only give you some hints, because thus far I have also failed to
> figure out how performance measurements should be enabled for a platform
> from scratch.
> 
> Earlier I thought that an interested platform should include modules
> from PerformancePkg. I got some good results that way; however:
> 
> - it wouldn't work for AARCH64, and so I filed
>  
> 
> - ultimately PerformancePkg was removed in commit 922c1e94e4bb
>  completely. (And for that reason I now closed TianoCore#679 as
>  WONTFIX.)
> 
> Commit 922c1e94e4bb doesn't really give pointers for enabling
> performance measurements in a platform -- it refers the user to the DP
> shell command instead of the standalone DP application, but that's only
> for displaying the stats, not for recording them.
> 
> One document where I found information is the Intel whitepaper
> 
>  A Tour Beyond BIOS
>  Implementing Profiling in with EDK II
> 
> (just google it). "Part III – Performance Profile" is relevant.
> 
> The first section of that talks about enabling ACPI FPDT (ACPI Firmware
> Performance Data Table) support in edk2, for exposing firmware
> performance data to the OS. Again, that's not about *recording* the stats.
> 
> The second section of the same chapter seems to describe how stats can
> be recorded however. AIUI, the DXE Core can provide the PERFORMANCE[_EX]
> protocols, if the DxeCorePerformanceLib instance is linked into it by
> the platform DSC file. In turn DXE modules can send measurements to the
> protocol via the PerformanceLib APIs / DxePerformanceLib instance.
> 
> In fact, the relevant library INF files have good documentation, we just
> have to know what to look at.
> 
> Recording stats in the PEI phase, via HOBs:
> - MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
> 
> Recording stats in the DXE phase (protocol provider and consumer):
> - MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
> - MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> 
> Recording stats in SMM (protocol provider and consumer):
> - MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.inf
> - MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.inf
> 
> Fetching the collected stats (for display or otherwise):
> - MdeModulePkg/Library/DxeSmmPerformanceLib/DxeSmmPerformanceLib.inf
> 
> "Do nothing" library instance for all module types:
> - 

Re: [edk2] PerformancePkg on multiple platform

[Laszlo Ersek]

Hi Prabin,

On 08/03/18 09:29, prabin ca wrote:
> Hi Team,
> 
> I’m new to uefi and edk. Currently I’m trying to get performance of my dxe 
> driver using PerformancePkg of EDK source code.
> 
> I’m using perf_start and perf_end T respected check points, it’s hot build 
> and tested well in 2/3 platform. It’s giving proper response.
> 
> But when I’m Checking with multiple platform, in some of platforms it’s not 
> working and got hang in uefi she’ll itself.
> 
> Please help me to resolve asap, it will be really helpful.

I can only give you some hints, because thus far I have also failed to
figure out how performance measurements should be enabled for a platform
from scratch.

Earlier I thought that an interested platform should include modules
from PerformancePkg. I got some good results that way; however:

- it wouldn't work for AARCH64, and so I filed
  

- ultimately PerformancePkg was removed in commit 922c1e94e4bb
  completely. (And for that reason I now closed TianoCore#679 as
  WONTFIX.)

Commit 922c1e94e4bb doesn't really give pointers for enabling
performance measurements in a platform -- it refers the user to the DP
shell command instead of the standalone DP application, but that's only
for displaying the stats, not for recording them.

One document where I found information is the Intel whitepaper

  A Tour Beyond BIOS
  Implementing Profiling in with EDK II

(just google it). "Part III – Performance Profile" is relevant.

The first section of that talks about enabling ACPI FPDT (ACPI Firmware
Performance Data Table) support in edk2, for exposing firmware
performance data to the OS. Again, that's not about *recording* the stats.

The second section of the same chapter seems to describe how stats can
be recorded however. AIUI, the DXE Core can provide the PERFORMANCE[_EX]
protocols, if the DxeCorePerformanceLib instance is linked into it by
the platform DSC file. In turn DXE modules can send measurements to the
protocol via the PerformanceLib APIs / DxePerformanceLib instance.

In fact, the relevant library INF files have good documentation, we just
have to know what to look at.

Recording stats in the PEI phase, via HOBs:
- MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf

Recording stats in the DXE phase (protocol provider and consumer):
- MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
- MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf

Recording stats in SMM (protocol provider and consumer):
- MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.inf
- MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.inf

Fetching the collected stats (for display or otherwise):
- MdeModulePkg/Library/DxeSmmPerformanceLib/DxeSmmPerformanceLib.inf

"Do nothing" library instance for all module types:
- MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf


Now obviously this doesn't explain why it works for you on some
platforms and why it doesn't on others. You haven't shared much
information about the platforms in question. I can make one guess: if
the performance protocol provided by the platform's DXE Core
(DxeCorePerformanceLib) is incompatible with the DxePerformanceLib
instance that is linked into your DXE driver, then communication between
them will likely fail.

To my understanding, the performance protocol is not standard (PI or
UEFI), so it's likely best if you build your DXE driver together with
the DXE Core, on all platforms that you want to check. It's probably
unsafe to link a DXE driver against DxePerformanceLib (rather than
BasePerformanceLibNull) and run it on a random platform.

Hopefully this helps a little.
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Performancepkg in edk2

[prabin ca]

It will be helpful if any one can give me a idea

> On 03-Aug-2018, at 12:00 PM, prabin ca  wrote:
> 
> Hi Team,
> 
> I’m new to uefi and edk. I’m facing or figure out one issue in PerformancePkg 
> in edk source code.
> 
> My aim is to get performance of my dxe driver by using PerformancePkg API’s 
> (perf_start and perf_end).
> 
> It’s got build and successfully tested and verified in some platforms well 
> without any failure. It giving proper output.
> 
> But the issue I’m facing is, it’s not working in some other platform. Please 
> help me to resolve this ASAP. It will be really helpful.
> 
> Is it a known issue with PerformancePkg
> 
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] UefiCpuPkg/RegisterCpuFeaturesLib: Implement AllocateAcpiCpuData function.

[Laszlo Ersek]

Hello Eric,

OVMF does not include CpuFeaturesPei / CpuFeaturesDxe, and so it doesn't
consume this library. I can't provide test results, but I have some
superficial comments.

First, I'm adding Marvin and Jeff -- I *vaguely* recall that this issue
may have been raised by Marvin. Hm... yes, it seems so:

  [edk2] CpuS3DataDxe / DxeRegisterCpuFeaturesLib dependency.

  
VI1PR0801MB17908DB1F03A3C5F84545E1080690@VI1PR0801MB1790.eurprd08.prod.outlook.com">http://mid.mail-archive.com/VI1PR0801MB17908DB1F03A3C5F84545E1080690@VI1PR0801MB1790.eurprd08.prod.outlook.com
  https://lists.01.org/pipermail/edk2-devel/2018-May/025163.html

I have three questions here:

(1) Do we have a TianoCore BZ about this?

(2) If not, does the currently proposed commit message capture the issue
in enough detail? Should we reference Marvin's initial report and/or a
TianoCore BZ (if we have one)?

(3) The implementation seems to follow Jeff's idea. Marvin and Jeff, can
you please help with the review?

Either way, I propose the following two tags to be appended:

Reported-by: Marvin Häuser 
Suggested-by: Fan Jeff 


And one technical question follows below:

On 08/03/18 04:10, Eric Dong wrote:
> Current code logic can't confirm CpuS3DataDxe driver start before
> CpuFeaturesDxe driver. So the assumption in CpuFeaturesDxe not
> valid. Add implementation for AllocateAcpiCpuData function to remove
> this assumption.
>
> Pass OS boot and resume from S3 test.
>
> Cc: Laszlo Ersek 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  .../DxeRegisterCpuFeaturesLib.c| 57 
> --
>  1 file changed, 54 insertions(+), 3 deletions(-)
>
> diff --git 
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c 
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
> index 902a339529..0722db6c64 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
> @@ -207,11 +207,62 @@ AllocateAcpiCpuData (
>VOID
>)
>  {
> +  EFI_STATUS   Status;
> +  UINTNNumberOfCpus;
> +  UINTNNumberOfEnabledProcessors;
> +  ACPI_CPU_DATA*AcpiCpuData;
> +  EFI_PHYSICAL_ADDRESS Address;
> +  UINTNTableSize;
> +  CPU_REGISTER_TABLE   *RegisterTable;
> +  UINTNIndex;
> +  EFI_PROCESSOR_INFORMATIONProcessorInfoBuffer;
> +
> +  Address = BASE_4GB - 1;
> +  Status  = gBS->AllocatePages (
> +   AllocateMaxAddress,
> +   EfiACPIMemoryNVS,
> +   EFI_SIZE_TO_PAGES (sizeof (ACPI_CPU_DATA)),
> +   
> +   );
> +  ASSERT_EFI_ERROR (Status);

I understand that this code is for IA32 and X64 only, but still, Ard has
recently introduced a DxeServicesLib API for this kind of allocation:
it's called AllocatePeiAccessiblePages(). See commit a40e0b7aa918.

(4) Is it feasible to use that function here (and in the second instance
below)?

From a library dependency standpoint, I think it should be fine: we are
modifying the DXE instance of the RegisterCpuFeaturesLib class, so a
dependency on DxeServicesLib should be in order.

Now, if this allocation *must* be satisfied from below 4GB, even if the
PEI phase has access to >=4GB RAM (as determined by

  PhitHob->EfiFreeMemoryTop > MAX_UINT32

), then my suggestion is wrong (because in that case,
AllocatePeiAccessiblePages() wouldn't restrict the allocation under
4GB).

CC'ing Ard too.

Thanks!
Laszlo

> +  AcpiCpuData = (ACPI_CPU_DATA *) (UINTN) Address;
> +  ASSERT (AcpiCpuData != NULL);
> +
> +  GetNumberOfProcessor (, );
> +  AcpiCpuData->NumberOfCpus = (UINT32) NumberOfCpus;
> +
>//
> -  // CpuS3DataDxe will do it.
> +  // Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable for 
> all CPUs
>//
> -  ASSERT (FALSE);
> -  return NULL;
> +  TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE);
> +  Address = BASE_4GB - 1;
> +  Status  = gBS->AllocatePages (
> +   AllocateMaxAddress,
> +   EfiACPIMemoryNVS,
> +   EFI_SIZE_TO_PAGES (TableSize),
> +   
> +   );
> +  ASSERT_EFI_ERROR (Status);
> +  RegisterTable = (CPU_REGISTER_TABLE *) (UINTN) Address;
> +
> +  for (Index = 0; Index < NumberOfCpus; Index++) {
> +Status = GetProcessorInformation (Index, );
> +ASSERT_EFI_ERROR (Status);
> +
> +RegisterTable[Index].InitialApicId  = 
> (UINT32)ProcessorInfoBuffer.ProcessorId;
> +RegisterTable[Index].TableLength= 0;
> +RegisterTable[Index].AllocatedSize  = 0;
> +RegisterTable[Index].RegisterTableEntry = 0;
> +
> +RegisterTable[NumberOfCpus + Index].InitialApicId  = 
> 

Re: [edk2] [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs

[Laszlo Ersek]

On 08/03/18 14:15, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: open_file_by_devpath_tiano_1008_v2
> 
> This is version 2 of the patch set that was originally posted at:
> 
>   https://lists.01.org/pipermail/edk2-devel/2018-July/027253.html
> 
> for .
> 
> Changes are noted on every patch.
> 
> The cumulative code difference is very small (not counting the
> FrameworkUefiLib copy of the function), so I'm including it here for
> easier review:
> 
>> [...]

I should use this opportunity to highlight an awesome community feature
from Paolo and Fam (CC'd): the patchew.org website has been tracking
edk2 patch submissions for a while:

https://patchew.org/
https://patchew.org/EDK2/

and now it offers side-by-side comparison between versions of the same
patch set ("interdiff"). For example, the link for this (v2) series is:

https://patchew.org/EDK2/20180803121537.32123-1-ler...@redhat.com/

and if you click the "Diff against v1" link, you get:

https://patchew.org/EDK2/20180718205043.17574-1-ler...@redhat.com/diff/20180803121537.32123-1-ler...@redhat.com/

I recommend that all edk2 reviewers make use of this feature, for
incrementally reviewing patch series.

(This is another good reason for keeping the source code lines limited
to 80 columns -- if you write 200 character long lines, you won't have a
good time looking at side-by-side diffs!)

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

Re: [edk2] [PATCH v1 0/5] Refactor AutoGen - class ModuleAutoGen

[Carsey, Jaben]

Please ignore the numbering. My error. There are only 3 parts of this series.

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Jaben Carsey
> Sent: Friday, August 03, 2018 8:11 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH v1 0/5] Refactor AutoGen - class ModuleAutoGen
> 
> This adds a decorator based caching mechanism for general use.
> This then changes ModuleAutoGen to use the mechanism via
> decorators for some functions and properties.  This allows for reduction
> in object size for objects as the functions are replaced by smaller
> data blobs.
> Also some small cleanups.
> 
> Jaben Carsey (5):
>   BaseTools: AutoGen refactor ModuleAutoGen caching
>   BaseTools: AutoGen - tag a function as cachable
>   BaseTools: AutoGen refactor to iterate less
> 
>  BaseTools/Source/Python/AutoGen/AutoGen.py | 1852 +---
> 
>  BaseTools/Source/Python/AutoGen/BuildEngine.py |   36 +-
>  BaseTools/Source/Python/AutoGen/GenC.py|2 +-
>  BaseTools/Source/Python/AutoGen/GenMake.py |   12 +-
>  BaseTools/Source/Python/Common/caching.py  |   47 +
>  5 files changed, 888 insertions(+), 1061 deletions(-)
>  create mode 100644 BaseTools/Source/Python/Common/caching.py
> 
> --
> 2.16.2.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v1 24/38] Platform/Hisilicon/D06: Add OemNicConfig2P Driver

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:09:08PM +0800, Ming Huang wrote:
> From: shaochangliang 
> 
> This Driver provide Get/Set Mac function.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: shaochangliang 
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 
> ---
>  Platform/Hisilicon/D06/D06.dsc |  2 
> +-
>  Platform/Hisilicon/D06/D06.fdf |  2 
> +-
>  Platform/Hisilicon/D06/Drivers/OemNicConfig2PHi1620/OemNicConfig.h | 25 
> +++
>  Platform/Hisilicon/D06/Drivers/OemNicConfig2PHi1620/OemNicConfig2P.c   | 71 
> 
>  Platform/Hisilicon/D06/Drivers/OemNicConfig2PHi1620/OemNicConfig2P.inf | 43 
> 
>  5 files changed, 141 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
> index 744a4a0d6d..49322f8304 100644
> --- a/Platform/Hisilicon/D06/D06.dsc
> +++ b/Platform/Hisilicon/D06/D06.dsc
> @@ -289,7 +289,7 @@
>#
>ArmPkg/Drivers/CpuDxe/CpuDxe.inf
>MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
> -
> +  Platform/Hisilicon/D06/Drivers/OemNicConfig2PHi1620/OemNicConfig2P.inf
>  
>  !if $(SECURE_BOOT_ENABLE) == TRUE
>MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
> diff --git a/Platform/Hisilicon/D06/D06.fdf b/Platform/Hisilicon/D06/D06.fdf
> index 1c6ee4e0e4..e654e9 100644
> --- a/Platform/Hisilicon/D06/D06.fdf
> +++ b/Platform/Hisilicon/D06/D06.fdf
> @@ -174,7 +174,7 @@ READ_LOCK_STATUS   = TRUE
>INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
>INF Platform/Hisilicon/D06/Drivers/SFC/SfcDxeDriver.inf
>  
> -
> +  INF Platform/Hisilicon/D06/Drivers/OemNicConfig2PHi1620/OemNicConfig2P.inf
>INF Silicon/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.inf
>INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
>INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> diff --git 
> a/Platform/Hisilicon/D06/Drivers/OemNicConfig2PHi1620/OemNicConfig.h 
> b/Platform/Hisilicon/D06/Drivers/OemNicConfig2PHi1620/OemNicConfig.h
> new file mode 100644
> index 00..35228fdf1d
> --- /dev/null
> +++ b/Platform/Hisilicon/D06/Drivers/OemNicConfig2PHi1620/OemNicConfig.h
> @@ -0,0 +1,25 @@
> +/** @file
> +*
> +*  Copyright (c) 2016-2018, Hisilicon Limited. All rights reserved.
> +*  Copyright (c) 2016-2018, Linaro Limited. All rights reserved.
> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD 
> License
> +*  which accompanies this distribution.  The full text of the license may be 
> found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +*
> +**/
> +
> +#ifndef __OEM_NIC_CONFIG_H__
> +#define __OEM_NIC_CONFIG_H__
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#endif
> diff --git 
> a/Platform/Hisilicon/D06/Drivers/OemNicConfig2PHi1620/OemNicConfig2P.c 
> b/Platform/Hisilicon/D06/Drivers/OemNicConfig2PHi1620/OemNicConfig2P.c
> new file mode 100644
> index 00..7e2cee3b86
> --- /dev/null
> +++ b/Platform/Hisilicon/D06/Drivers/OemNicConfig2PHi1620/OemNicConfig2P.c
> @@ -0,0 +1,71 @@
> +/** @file
> +*
> +*  Copyright (c) 2016-2018, Hisilicon Limited. All rights reserved.
> +*  Copyright (c) 2016-2018, Linaro Limited. All rights reserved.
> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD 
> License
> +*  which accompanies this distribution.  The full text of the license may be 
> found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +*
> +**/
> +
> +#include 
> +
> +
> +EFI_STATUS
> +EFIAPI OemGetMac2P (
> +  IN OUT EFI_MAC_ADDRESS *Mac,
> +  IN UINTN   Port
> +  )
> +{
> +  OemGetMac (Mac, Port);
> +
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +EFIAPI OemSetMac2P (
> +  IN EFI_MAC_ADDRESS *Mac,
> +  IN UINTN   Port
> +  )
> +{
> +  OemSetMac (Mac, Port);
> +
> +  return EFI_SUCCESS;
> +}
> +
> +HISI_BOARD_NIC_PROTOCOL mHisiBoardNicProtocol2P = {
> +  .GetMac = OemGetMac2P,
> +  .SetMac = OemSetMac2P,
> +};
> +
> +
> +EFI_STATUS
> +EFIAPI
> +OemNicConfigEntry (
> +  IN EFI_HANDLE   ImageHandle,
> +  IN EFI_SYSTEM_TABLE *SystemTable
> +  )
> +{
> +  EFI_STATUS Status;
> +
> +  Status = gBS->InstallProtocolInterface (
> +,

Indent to function name (InstallP...), not variable name.

/
Leif

> +,
> +EFI_NATIVE_INTERFACE,
> +
> +);
> +
> +  if (EFI_ERROR (Status)) {
> +DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Install 

Re: [edk2] [PATCH edk2-platforms v1 23/38] Hisilicon/D0X: Rename the global variable gDS3231RtcDevice

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:09:07PM +0800, Ming Huang wrote:
> The global variable gDS3231RtcDevice is used by several
> modules included common module in other Pkg. Renaming it
> with a general name is proper.

Ah, this was why the variable name did not match the patch set.
Good. MEans I can trust your tree again.

But please do this change _before_ introducing it for D06.

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 
> ---
>  
> Platform/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.c
>| 8 
>  Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c   
> | 2 +-
>  Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c   
> | 2 +-
>  
> Platform/Hisilicon/D06/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c
>| 8 
>  
> Platform/Hisilicon/D06/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf
>  | 1 +
>  Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c   
> | 2 +-
>  Silicon/Hisilicon/Include/Library/OemMiscLib.h   
> | 2 +-
>  Silicon/Hisilicon/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.c
> | 8 
>  8 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git 
> a/Platform/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.c
>  
> b/Platform/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.c
> index 07fa52aa78..ed866e46b5 100644
> --- 
> a/Platform/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.c
> +++ 
> b/Platform/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.c
> @@ -41,7 +41,7 @@
>  #include 
>  #include 
>  
> -extern I2C_DEVICE gDS3231RtcDevice;
> +extern I2C_DEVICE gRtcDevice;
>  
>  STATIC BOOLEAN   mDS3231Initialized = FALSE;
>  
> @@ -117,7 +117,7 @@ InitializeDS3231 (
>// Prepare the hardware
>(VOID)IdentifyDS3231();
>  
> -  (VOID) CopyMem(, , sizeof(Dev));
> +  (VOID) CopyMem(, , sizeof(Dev));

Space before (.

>  
>Status = I2CInit(Dev.Socket,Dev.Port,Normal);
>if (EFI_ERROR (Status)) {
> @@ -199,7 +199,7 @@ LibGetTime (
>  }
>}
>  
> -  (VOID) CopyMem(, , sizeof(Dev));
> +  (VOID) CopyMem(, , sizeof(Dev));

Space before (.

>  
>Status |= I2CRead(,DS3231_REGADDR_MONTH,1,);
>  
> @@ -299,7 +299,7 @@ LibSetTime (
>  }
>}
>  
> -  (VOID) CopyMem(, , sizeof(Dev));
> +  (VOID) CopyMem(, , sizeof(Dev));

Space before (.

/
Leif

>  
>Temp = ((Time->Second/10)<<4) | (Time->Second%10);
>MicroSecondDelay(1000);
> diff --git 
> a/Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c 
> b/Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c
> index 66d62895a6..4771cb900c 100644
> --- a/Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c
> +++ b/Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c
> @@ -25,7 +25,7 @@
>  #include 
>  #include 
>  
> -I2C_DEVICE gDS3231RtcDevice = {
> +I2C_DEVICE gRtcDevice = {
>  .Socket = 0,
>  .Port = 6,
>  .DeviceType = DEVICE_TYPE_SPD,
> diff --git a/Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c 
> b/Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c
> index 15a509be5d..ae4c194070 100644
> --- a/Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c
> +++ b/Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c
> @@ -26,7 +26,7 @@
>  #include 
>  
>  
> -I2C_DEVICE gDS3231RtcDevice = {
> +I2C_DEVICE gRtcDevice = {
>.Socket = 0,
>.Port = 4,
>.DeviceType = DEVICE_TYPE_SPD,
> diff --git 
> a/Platform/Hisilicon/D06/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c
>  
> b/Platform/Hisilicon/D06/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c
> index 9b1d7c00e8..d680dd0393 100644
> --- 
> a/Platform/Hisilicon/D06/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c
> +++ 
> b/Platform/Hisilicon/D06/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c
> @@ -32,7 +32,7 @@
>  #include 
>  #include "M41T83RealTimeClock.h"
>  
> -extern I2C_DEVICE gDS3231RtcDevice;
> +extern I2C_DEVICE gRtcDevice;
>  
>  EFI_STATUS
>  SwitchRtcI2cChannelAndLock (
> @@ -108,7 +108,7 @@ RtcRead (
>  {
>EFI_STATUS  Status;
>  
> -  Status = I2CRead (, Address, Size, Data);
> +  Status = I2CRead (, Address, Size, Data);
>MicroSecondDelay (1000);
>return Status;
>  }
> @@ -131,7 +131,7 @@ RtcWrite (
>  {
>EFI_STATUS  Status;
>  
> -  Status = I2CWrite(, Address, Size, Data);
> +  Status = I2CWrite(, Address, Size, Data);
>MicroSecondDelay (1000);
>return Status;
>  }
> @@ -162,7 +162,7 @@ InitializeM41T83 (
>  EfiAcquireLock ();
>}
>  
> -  Status = I2CInit (gDS3231RtcDevice.Socket, gDS3231RtcDevice.Port, Normal);
> +  Status = I2CInit (gRtcDevice.Socket, gRtcDevice.Port, Normal);
>MicroSecondDelay (1000);
>if (EFI_ERROR 

[edk2] [PATCH v1 2/5] BaseTools: AutoGen - tag a function as cachable

[Jaben Carsey]

MakeFile generation is once per module, so mark it as such.
also move the time stamp creation function inside as it's
only called from one place.

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

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 1515a52758de..55c84fe4fbc2 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -54,7 +54,7 @@ from collections import OrderedDict
 from collections import defaultdict
 from Workspace.WorkspaceCommon import OrderedListDict
 
-from Common.caching import cached_property
+from Common.caching import cached_property, cached_class_function
 
 ## Regular expression for splitting Dependency Expression string into tokens
 gDepexTokenPattern = re.compile("(\(|\)|\w+| \S+\.inf)")
@@ -2677,7 +2677,6 @@ class ModuleAutoGen(AutoGen):
 self.ToolChainFamily = self.PlatformInfo.ToolChainFamily
 self.BuildRuleFamily = self.PlatformInfo.BuildRuleFamily
 
-self.IsMakeFileCreated = False
 self.IsCodeFileCreated = False
 self.IsAsBuiltInfCreated = False
 self.DepexGenerated = False
@@ -4084,13 +4083,31 @@ class ModuleAutoGen(AutoGen):
 #   @param  CreateLibraryMakeFile   Flag indicating if or not the 
makefiles of
 #   dependent libraries will be created
 #
+@cached_class_function
 def CreateMakeFile(self, CreateLibraryMakeFile=True, GenFfsList = []):
+# nest this function inside it's only caller.
+def CreateTimeStamp():
+FileSet = {self.MetaFile.Path}
+
+for SourceFile in self.Module.Sources:
+FileSet.add (SourceFile.Path)
+
+for Lib in self.DependentLibraryList:
+FileSet.add (Lib.MetaFile.Path)
+
+for f in self.AutoGenDepSet:
+FileSet.add (f.Path)
+
+if os.path.exists (self.TimeStampPath):
+os.remove (self.TimeStampPath)
+with open(self.TimeStampPath, 'w+') as file:
+for f in FileSet:
+print(f, file=file)
+
 # Ignore generating makefile when it is a binary module
 if self.IsBinaryModule:
 return
 
-if self.IsMakeFileCreated:
-return
 self.GenFfsList = GenFfsList
 if not self.IsLibrary and CreateLibraryMakeFile:
 for LibraryAutoGen in self.LibraryAutoGenList:
@@ -4110,8 +4127,7 @@ class ModuleAutoGen(AutoGen):
 EdkLogger.debug(EdkLogger.DEBUG_9, "Skipped the generation of 
makefile for module %s [%s]" %
 (self.Name, self.Arch))
 
-self.CreateTimeStamp()
-self.IsMakeFileCreated = True
+CreateTimeStamp()
 
 def CopyBinaryFiles(self):
 for File in self.Module.Binaries:
@@ -4284,21 +4300,3 @@ class ModuleAutoGen(AutoGen):
 @cached_property
 def TimeStampPath(self):
 return os.path.join(self.MakeFileDir, 'AutoGenTimeStamp')
-
-def CreateTimeStamp(self):
-FileSet = {self.MetaFile.Path}
-
-for SourceFile in self.Module.Sources:
-FileSet.add (SourceFile.Path)
-
-for Lib in self.DependentLibraryList:
-FileSet.add (Lib.MetaFile.Path)
-
-for f in self.AutoGenDepSet:
-FileSet.add (f.Path)
-
-if os.path.exists (self.TimeStampPath):
-os.remove (self.TimeStampPath)
-with open(self.TimeStampPath, 'w+') as file:
-for f in FileSet:
-print(f, file=file)
-- 
2.16.2.windows.1

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

[edk2] [PATCH v1 1/5] BaseTools: AutoGen refactor ModuleAutoGen caching

[Jaben Carsey]

1) Add a new file Common/caching.py
a.  Allows for automated caching of repeated class functions, class
properties, and non-class functions
b.  When called the first time the value is cached and if called a
second time, the cached result is called, not the function.
c.  When used, this saves lots of memory since the actual function
pointers are replaced with smaller data elements.
d.  note that not all features are used yet.
2) Fix AutoGen/GenMake and AutoGen/GetC to not call into private member
variables of ModuleAutoGen class
a. use the existing accessor properties for the data
3) Change AutoGen classes to remove a exception for duplicate members in
__new__ and use “in” testing to speed up
4)  Work on ModuleAutoGen class
a.  Change all properties that use caching to use @caching_property
(see #1 above)
b.  Change all properties that do not use caching to use standard python
decorator "@property"
c.  Change all cases where a dictionary/set/list/object was created
and then immediately updated to use constructor parameters
d.  Refactor each property function to remove the internal variable
that is no longer needed (this helps find items like #2 above)
e.  Refactor _ApplyBuildRule with optional parameter to work around
circular dependency with BinaryFileList.

Note that 4e was almost certainly unintended as the functions were acting on
incomplete information since they were directly accessing private instance
variables at the same time another function on the stack was creating the same
private isntance data.

This overall changes behavior slightly, but makes the codebase smaller and
easier to read.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Cc: Bob Feng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 1147 +---
 BaseTools/Source/Python/AutoGen/GenC.py|2 +-
 BaseTools/Source/Python/AutoGen/GenMake.py |   12 +-
 BaseTools/Source/Python/Common/caching.py  |   47 +
 4 files changed, 552 insertions(+), 656 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 06ff84b4cdbc..1515a52758de 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -54,6 +54,8 @@ from collections import OrderedDict
 from collections import defaultdict
 from Workspace.WorkspaceCommon import OrderedListDict
 
+from Common.caching import cached_property
+
 ## Regular expression for splitting Dependency Expression string into tokens
 gDepexTokenPattern = re.compile("(\(|\)|\w+| \S+\.inf)")
 
@@ -195,13 +197,12 @@ class AutoGen(object):
 def __new__(cls, Workspace, MetaFile, Target, Toolchain, Arch, *args, 
**kwargs):
 # check if the object has been created
 Key = (Target, Toolchain, Arch, MetaFile)
-try:
+if Key in cls.__ObjectCache:
 # if it exists, just return it directly
 return cls.__ObjectCache[Key]
-except:
 # it didnt exist. create it, cache it, then return it
-cls.__ObjectCache[Key] = super(AutoGen, cls).__new__(cls)
-return cls.__ObjectCache[Key]
+RetVal = cls.__ObjectCache[Key] = super(AutoGen, cls).__new__(cls)
+return RetVal
 
 def __init__ (self, Workspace, MetaFile, Target, Toolchain, Arch, *args, 
**kwargs):
 super(AutoGen, self).__init__(self, Workspace, MetaFile, Target, 
Toolchain, Arch, *args, **kwargs)
@@ -236,9 +237,7 @@ class AutoGen(object):
 class WorkspaceAutoGen(AutoGen):
 # call super().__init__ then call the worker function with different 
parameter count
 def __init__(self, Workspace, MetaFile, Target, Toolchain, Arch, *args, 
**kwargs):
-try:
-self._Init
-except:
+if not hasattr(self, "_Init"):
 super(WorkspaceAutoGen, self).__init__(Workspace, MetaFile, 
Target, Toolchain, Arch, *args, **kwargs)
 self._InitWorker(Workspace, MetaFile, Target, Toolchain, Arch, 
*args, **kwargs)
 self._Init = True
@@ -442,7 +441,7 @@ class WorkspaceAutoGen(AutoGen):
 MGen = ModuleAutoGen(self, 
BuildData.MetaFile, Target, Toolchain, Arch, self.MetaFile)
 if MGen and MGen.IsLibrary:
 if MGen in PGen.LibraryAutoGenList:
-ReferenceModules = 
MGen._ReferenceModules
+ReferenceModules = 
MGen.ReferenceModules
 for ReferenceModule in 
ReferenceModules:
 if ReferenceModule.MetaFile in 
Platform.Modules:
 RefPlatformModule = 

[edk2] [PATCH v1 3/5] BaseTools: AutoGen refactor to iterate less

[Jaben Carsey]

Don't iterate over new dictionaries with one item

Create the data and then add to dictionary.

Note: if you diff ignoring whitespace changes you
can more easily see the relevant changes.

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

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 55c84fe4fbc2..7c67f40bff00 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -2983,80 +2983,74 @@ class ModuleAutoGen(AutoGen):
 if self.DxsFile or self.IsLibrary or TAB_DEPENDENCY_EXPRESSION_FILE in 
self.FileTypes:
 return {}
 
-RetVal = {self.ModuleType:[]}
-
-for ModuleType in RetVal:
-DepexList = RetVal[ModuleType]
-#
-# Append depex from dependent libraries, if not "BEFORE", "AFTER" 
expresion
-#
-for M in [self.Module] + self.DependentLibraryList:
-Inherited = False
-for D in M.Depex[self.Arch, ModuleType]:
-if DepexList != []:
-DepexList.append('AND')
-DepexList.append('(')
-#replace D with value if D is FixedAtBuild PCD
-NewList = []
-for item in D:
-if '.' not in item:
-NewList.append(item)
+DepexList = []
+#
+# Append depex from dependent libraries, if not "BEFORE", "AFTER" 
expresion
+#
+for M in [self.Module] + self.DependentLibraryList:
+Inherited = False
+for D in M.Depex[self.Arch, self.ModuleType]:
+if DepexList != []:
+DepexList.append('AND')
+DepexList.append('(')
+#replace D with value if D is FixedAtBuild PCD
+NewList = []
+for item in D:
+if '.' not in item:
+NewList.append(item)
+else:
+if item not in self._FixedPcdVoidTypeDict:
+EdkLogger.error("build", FORMAT_INVALID, "{} used 
in [Depex] section should be used as FixedAtBuild type and VOID* datum type in 
the module.".format(item))
 else:
-if item not in self._FixedPcdVoidTypeDict:
-EdkLogger.error("build", FORMAT_INVALID, "{} 
used in [Depex] section should be used as FixedAtBuild type and VOID* datum 
type in the module.".format(item))
-else:
-Value = self._FixedPcdVoidTypeDict[item]
-if len(Value.split(',')) != 16:
-EdkLogger.error("build", FORMAT_INVALID,
-"{} used in [Depex] 
section should be used as FixedAtBuild type and VOID* datum type and 16 bytes 
in the module.".format(item))
-NewList.append(Value)
-DepexList.extend(NewList)
-if DepexList[-1] == 'END':  # no need of a END at this time
-DepexList.pop()
-DepexList.append(')')
-Inherited = True
-if Inherited:
-EdkLogger.verbose("DEPEX[%s] (+%s) = %s" % (self.Name, 
M.BaseName, DepexList))
-if 'BEFORE' in DepexList or 'AFTER' in DepexList:
-break
-if len(DepexList) > 0:
-EdkLogger.verbose('')
-return RetVal
-
-## Merge dependency expression
-#
-#   @retval listThe token list of the dependency expression after 
parsed
-#
-@cached_property
-def DepexExpressionTokenList(self):
-if self.DxsFile or self.IsLibrary or TAB_DEPENDENCY_EXPRESSION_FILE in 
self.FileTypes:
-return {}
-
-RetVal = {self.ModuleType:''}
-
-for ModuleType in RetVal:
-DepexExpressionString = RetVal[ModuleType]
-#
-# Append depex from dependent libraries, if not "BEFORE", "AFTER" 
expresion
-#
-for M in [self.Module] + self.DependentLibraryList:
-Inherited = False
-for D in M.DepexExpression[self.Arch, ModuleType]:
-if DepexExpressionString != '':
-DepexExpressionString += ' AND '
-DepexExpressionString += '('
-DepexExpressionString += D
-DepexExpressionString = 
DepexExpressionString.rstrip('END').strip()
-DepexExpressionString += ')'
-

[edk2] [PATCH v1 0/5] Refactor AutoGen - class ModuleAutoGen

[Jaben Carsey]

This adds a decorator based caching mechanism for general use.
This then changes ModuleAutoGen to use the mechanism via 
decorators for some functions and properties.  This allows for reduction
in object size for objects as the functions are replaced by smaller
data blobs.
Also some small cleanups.

Jaben Carsey (5):
  BaseTools: AutoGen refactor ModuleAutoGen caching
  BaseTools: AutoGen - tag a function as cachable
  BaseTools: AutoGen refactor to iterate less

 BaseTools/Source/Python/AutoGen/AutoGen.py | 1852 +---
 BaseTools/Source/Python/AutoGen/BuildEngine.py |   36 +-
 BaseTools/Source/Python/AutoGen/GenC.py|2 +-
 BaseTools/Source/Python/AutoGen/GenMake.py |   12 +-
 BaseTools/Source/Python/Common/caching.py  |   47 +
 5 files changed, 888 insertions(+), 1061 deletions(-)
 create mode 100644 BaseTools/Source/Python/Common/caching.py

-- 
2.16.2.windows.1

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

Re: [edk2] [PATCH] OvmfPkg/PlatformDebugLibIoPort: fix port detection for use in the DXE Core

[Laszlo Ersek]

On 08/03/18 08:42, Jordan Justen wrote:
> On 2018-08-02 17:30:45, Laszlo Ersek wrote:
>> The DXE Core is one of those modules that call
>> ProcessLibraryConstructorList() manually.
>>
>> Before DxeMain() [MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c] calls
>> ProcessLibraryConstructorList(), and through it, our
>> PlatformDebugLibIoPortConstructor() function, DxeMain() invokes the
>> DEBUG() macro multiple times. That macro lands in our
>> PlatformDebugLibIoPortFound() function -- which currently relies on the
>> "mDebugIoPortFound" global variable that has (not yet) been set by the
>> constructor. As a result, early debug messages from the DXE Core are lost.
>>
>> Move the device detection into PlatformDebugLibIoPortFound(), also caching
>> the fact (not just the result) of the device detection.
>>
>> (We could introduce a separate DebugLib instance just for the DXE Core,
>> but the above approach works for all modules that currently consume the
>> PlatformDebugLibIoPort instance (which means "everything but SEC").)
>>
>> This restores messages such as:
>>
>>> CoreInitializeMemoryServices:
>>>   BaseAddress - 0x7AF21000 Length - 0x3CDE000 MinimalMemorySizeNeeded - 
>>> 0x10F4000
>>
>> Keep the empty constructor function -- OVMF's DebugLib instances have
>> always had constructors; we had better not upset constructor dependency
>> ordering by making our instance(s) constructor-less.
>>
>> Cc: Ard Biesheuvel 
>> Cc: Brijesh Singh 
>> Cc: Jordan Justen 
>> Fixes: c09d9571300a089c35f5df2773b70edc25050d0d
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek 
>> ---
>>
>> Notes:
>> Brijesh, can you please test this patch on SEV, with and without
>> capturing the debug port? (In the first case, the debug log should just
>> work; in the second case, the boot should remain fast.) Thanks!
>> 
>> Repo:   https://github.com/lersek/edk2.git
>> Branch: debuglib_dxecore
>>
>>  OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c | 24 
>> 
>>  1 file changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c 
>> b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
>> index 81c44eece95f..74aef2e37b42 100644
>> --- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
>> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
>> @@ -16,14 +16,26 @@
>>  #include 
>>  #include "DebugLibDetect.h"
>>  
>> +
>> +//
>> +// Set to TRUE if the debug I/O port has been checked
>> +//
>> +STATIC BOOLEAN mDebugIoPortChecked = FALSE;
> 
> Could this be a static variable in the function?

Yes, both variables could be defined in PlatformDebugLibIoPortFound()
now -- and that would actually be superior coding practice in any other
C-language project :)

In edk2, objects with block scope declaration, static storage duration,
and no linkage (aka "function global variables") basically don't exist.

I've sneakily added a handful of them to OvmfPkg, over time, but only so
I could attach names to string literals that I'd use multiple times. See
"Fallback" in "Library/PlatformBootManagerLib/BdsPlatform.c", and
"ConvFallBack" in "Library/QemuBootOrderLib/QemuBootOrderLib.c".

(Such variables don't get the "m" prefix though.)

> If not, should it be follow by a blank line?

In edk2 there is "prior art" for both keeping and not keeping [*] a
blank line just before a comment block. I strive to decide consciously,
and this was no exception -- I felt that the 1st and 3rd lines in the
subsequent comment

//
// Set to TRUE if the debug I/O port is enabled
//

gave enough separation.

[*] One example of the many: see near the macro and enum constant
definitions in "MdePkg/Include/Uefi/UefiMultiPhase.h".

> 
> I agree that Brijesh should verify it, but pending that:
> 
> Reviewed-by: Jordan Justen 

Thanks!

Should I resubmit the patch for function-scoping (and renaming) the
global variables, or for inserting the blank linke?

(I guess I could insert the blank line right before I push, too, if you
prefer that.)

Thanks!
Laszlo

>>  //
>>  // Set to TRUE if the debug I/O port is enabled
>>  //
>>  STATIC BOOLEAN mDebugIoPortFound = FALSE;
>>  
>>  /**
>> -  This constructor function checks if the debug I/O port device is present,
>> -  caching the result for later use.
>> +  This constructor function must not do anything.
>> +
>> +  Some modules consuming this library instance, such as the DXE Core, invoke
>> +  the DEBUG() macro before they explicitly call
>> +  ProcessLibraryConstructorList(). Therefore the auto-generated call from
>> +  ProcessLibraryConstructorList() to this constructor function may be 
>> preceded
>> +  by some calls to PlatformDebugLibIoPortFound() below. Hence
>> +  PlatformDebugLibIoPortFound() must not rely on anything this constructor
>> +  could set up.
>>  
>>@retval RETURN_SUCCESS   The constructor always returns RETURN_SUCCESS.
>>  
>> @@ -34,12 +46,12 @@ 

Re: [edk2] [PATCH v1 1/1] PatchCheck - add error message for invalid parameter

[Carsey, Jaben]

Absolutely.  I should have thought to add that.

"Python ScriptCheck.py t"

Assuming there is no commit or file called "t".  Basically just anything that 
is not a commit identifier nor a filename.  I found it when I tried to select a 
commit and misspelled it.

-Jaben

> -Original Message-
> From: Gao, Liming
> Sent: Friday, August 03, 2018 2:44 AM
> To: Carsey, Jaben ; edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH v1 1/1] PatchCheck - add error message for
> invalid parameter
> Importance: High
> 
> Jaben:
>   Could you give me one failure case? Then, I can further understand the
> patch.
> 
> Thanks
> Liming
> >-Original Message-
> >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> >Jaben Carsey
> >Sent: Friday, August 03, 2018 7:21 AM
> >To: edk2-devel@lists.01.org
> >Cc: Gao, Liming 
> >Subject: [edk2] [PATCH v1 1/1] PatchCheck - add error message for invalid
> >parameter
> >
> >Currently if an invalid parameter is passed, it gives a stack trace.
> >This changes it to an error message.
> >
> >Cc: Liming Gao 
> >Cc: Yonghong Zhu 
> >Contributed-under: TianoCore Contribution Agreement 1.1
> >Signed-off-by: Jaben Carsey 
> >---
> > BaseTools/Scripts/PatchCheck.py | 9 ++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> >diff --git a/BaseTools/Scripts/PatchCheck.py
> >b/BaseTools/Scripts/PatchCheck.py
> >index 7b7fba8b7044..96b3cdf1fd8a 100755
> >--- a/BaseTools/Scripts/PatchCheck.py
> >+++ b/BaseTools/Scripts/PatchCheck.py
> >@@ -1,7 +1,7 @@
> > ## @file
> > #  Check a patch for various format issues
> > #
> >-#  Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
> >+#  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
> > #
> > #  This program and the accompanying materials are licensed and made
> > #  available under the terms and conditions of the BSD License which
> >@@ -528,6 +528,8 @@ class CheckGitCommits:
> > print('Checking git commit:', commit)
> > patch = self.read_patch_from_git(commit)
> > self.ok &= CheckOnePatch(commit, patch).ok
> >+if not commits:
> >+print("Couldn't find commit matching: '{}'".format(rev_spec))
> >
> > def read_commit_list_from_git(self, rev_spec, max_count):
> > # Run git to get the commit patch
> >@@ -536,7 +538,7 @@ class CheckGitCommits:
> > cmd.append('--max-count=' + str(max_count))
> > cmd.append(rev_spec)
> > out = self.run_git(*cmd)
> >-return out.split()
> >+return out.split() if out else []
> >
> > def read_patch_from_git(self, commit):
> > # Run git to get the commit patch
> >@@ -548,7 +550,8 @@ class CheckGitCommits:
> > p = subprocess.Popen(cmd,
> >  stdout=subprocess.PIPE,
> >  stderr=subprocess.STDOUT)
> >-return p.communicate()[0].decode('utf-8', 'ignore')
> >+Result = p.communicate()
> >+return Result[0].decode('utf-8', 'ignore') if Result[0] and
> >Result[0].find("fatal")!=0 else None
> >
> > class CheckOnePatchFile:
> > """Performs a patch check for a single file.
> >--
> >2.16.2.windows.1
> >
> >___
> >edk2-devel mailing list
> >edk2-devel@lists.01.org
> >https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 with OVMF

[Laszlo Ersek]

On 08/03/18 15:39, Ricardo Araújo wrote:
> Hi folks, sorry for the delay! 
> 
> I've just applied the patch and got the same error message and empty PCRs. 

Thanks for the feedback -- although it's not the kind I had hoped for :)

I have now filed "[regression] SecurityPkg commit f15cb995bb38 breaks
TPM2_ENABLE in OvmfPkg":

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

Ricardo, please consider registering in the TianoCore Bugzilla, and
adding yourself to the CC list of BZ#1075.

For now, I have assigned the BZ to Marc-André, for triaging / analysis.
swtpm is not set up on my end, and the TPM2 enablement for OvmfPkg was
contributed by Marc-André. Marc-André, are you OK with this? The BZ
assignment is about root-causing the issue, at the moment.

Once we know more closely what the problem is, we can decide what to do.
If it's hard to fix, my argument will be that we should roll back
SecurityPkg commit f15cb995bb38 first (it's a regression after all), and
re-apply it only when it no longer breaks OVMF. If the issue is not hard
to fix and we can commit the solution quickly, then I'll be fine with
leaving f15cb995bb38 applied.

Thanks,
Laszlo

> 
> De: "Zhang, Chao B"  
> Para: "Laszlo Ersek" , "Ricardo Araújo" 
> , "Marc-André Lureau"  
> Cc: edk2-devel@lists.01.org, "Gao, Liming" , "Zeng, 
> Star"  
> Enviadas: Quinta-feira, 2 de agosto de 2018 21:22:18 
> Assunto: RE: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 
> with OVMF 
> 
> 
> 
> Tks Lazslo. And please make sure PcdLib is correctly lined in OVMF 
> 
> 
> 
> 
> From: Laszlo Ersek [mailto:ler...@redhat.com] 
> Sent: Thursday, August 2, 2018 9:14 PM 
> To: Zhang, Chao B ; Ricardo Araújo 
> ; Marc-André Lureau  
> Cc: edk2-devel@lists.01.org; Gao, Liming ; Zeng, Star 
>  
> Subject: Re: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 
> with OVMF 
> 
> 
> 
> 
> On 08/02/18 04:04, Zhang, Chao B wrote: 
>> Hi Laszlo & Ricardo 
>> The patch was intended to reduce the time to read TPM interface ID register. 
>> The interface type should not change within boot cycle according to PTP 
>> spec. 
>> I agree to add some ASSERT after PCDSetxxsS. 
>> But It is a core solution without platform change as PCD has been configured 
>> as DYN, DYNEx in DEC. I don’t know why you meet Set Failure 
>> In OVMF. Here, I include PCD expert to explain this. 
> 
> As far as I recall, dynamic PCDs have never behaved as actually settable 
> for me unless I added dynamic defaults for them in the OVMF DSC files. 
> 
> I never really researched why this was the case, I just accepted that 
> the dynamic defaults were apparently necessary. 
> 
> Let's wait for Ricardo's response. Perhaps my analysis / suspicion were 
> incorrect and it's not actually the "dynamism" of the PCD that's missing 
> for OVMF. Ricardo's answer will tell us if there's another issue. 
> 
> Thanks 
> Laszlo 
> 
>> From: Laszlo Ersek [ mailto:ler...@redhat.com ] 
>> Sent: Thursday, August 2, 2018 5:49 AM 
>> To: Ricardo Araújo < rica...@lsd.ufcg.edu.br >; Zhang, Chao B < 
>> chao.b.zh...@intel.com >; Marc-André Lureau < marcandre.lur...@redhat.com > 
>> Cc: edk2-devel@lists.01.org 
>> Subject: Re: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 
>> with OVMF 
>>
>> On 08/01/18 19:50, Ricardo Araújo wrote: 
>>> The commit I was referring to is: 
>>> https://github.com/tianocore/edk2/commit/f15cb995bb3880b77e15afe6facd3da05e599a17
>>>  
>>>
>>> Regards, 
>>>
>>> Ricardo Araujo - 
>>> www.lsd.ufcg.edu.br/~ricardo 
>>>
>>> - Mensagem original - 
>>> De: "Ricardo Araújo" < 
>>> rica...@lsd.ufcg.edu.br> 
>>> Para: edk2-devel@lists.01.org 
>>> Enviadas: Quarta-feira, 1 de agosto de 2018 14:33:45 
>>> Assunto: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 
>>> with OVMF 
>>>
>>> Hi everyone, 
>>>
>>> I'm using OVMF with a simulated TPM 2.0 (from 
>>> https://github.com/stefanberger/swtpm ) and I noticed lately that PCRs 
>>> 0-7 are zeroed after booting the vm (ubuntu 18.04) and the only 
>>> message related to this in dmesg is: 
>>>
>>> [ 2.286690] tpm_tis 00:06: 2.0 TPM (device-id 0x1, rev-id 1) 
>>> [ 2.303753] tpm tpm0: A TPM error (256) occurred continue selftest 
>>> [ 2.314199] tpm tpm0: starting up the TPM manually 
>>>
>>> I found this started to happen after this commit , previous commits to 
>>> that are showing boot time measurements on PCR 0-7 normally and the 
>>> error message is gone. Has anyone experienced the same behavior? I 
>>> followed the instructions here for building OVMF but I added the 
>>> parameters -D TPM2_ENABLE=TRUE -D SECURE_BOOT_ENABLE=TRUE -D 
>>> HTTP_BOOT_ENABLE=TRUE. Is there anything else I need to add to enable 
>>> these measurements? 
>>>
>>> Regards, 
>>>
>>> Ricardo Araujo 
>>> www.lsd.ufcg.edu.br/~ricardo 
>>
>> Thank you for the bug report. It looks like a 

Re: [edk2] [PATCH edk2-platforms v1 22/38] Platform/Hisilicon/D06: Add OemNicLib

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:09:06PM +0800, Ming Huang wrote:
> OemNicLib provide nic related api like GetMac,SetMac.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 
> ---
>  Platform/Hisilicon/D06/D06.dsc |   1 +
>  Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c   | 571 
> 
>  Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.inf |  35 ++
>  3 files changed, 607 insertions(+)
> 
> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
> index 43af043cfd..744a4a0d6d 100644
> --- a/Platform/Hisilicon/D06/D06.dsc
> +++ b/Platform/Hisilicon/D06/D06.dsc
> @@ -91,6 +91,7 @@
>  
>LpcLib|Silicon/Hisilicon/Hi1620/Library/LpcLibHi1620/LpcLib.inf
>
> SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> +  OemNicLib|Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.inf
>  !if $(SECURE_BOOT_ENABLE) == TRUE
>FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
>  !endif
> diff --git a/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c 
> b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
> new file mode 100644
> index 00..55ed1625ce
> --- /dev/null
> +++ b/Platform/Hisilicon/D06/Library/OemNicLib/OemNicLib.c
> @@ -0,0 +1,571 @@
> +/** @file
> +*
> +*  Copyright (c) 2018, Hisilicon Limited. All rights reserved.
> +*  Copyright (c) 2017, Linaro Limited. All rights reserved.
> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD 
> License
> +*  which accompanies this distribution.  The full text of the license may be 
> found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +*
> +**/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define CPU2_SFP2_100G_CARD_OFFSET   0x25
> +#define CPU1_SFP1_LOCATE_OFFSET  0x16
> +#define CPU1_SFP0_LOCATE_OFFSET  0x12
> +#define CPU2_SFP1_LOCATE_OFFSET  0x21
> +#define CPU2_SFP0_LOCATE_OFFSET  0x19
> +#define CPU2_SFP2_10G_GE_CARD_OFFSET 0x25
> +
> +#define SFP_10G_SPEED   10
> +#define SFP_25G_SPEED   25
> +#define SFP_100G_SPEED  100
> +#define SFP_GE_SPEED1
> +
> +#define SFP_GE_SPEED_VAL_VENDOR_FINISAR 0x0C
> +#define SFP_GE_SPEED_VAL0x0D
> +#define SFP_10G_SPEED_VAL   0x67
> +#define SFP_25G_SPEED_VAL   0xFF
> +
> +#define CPU1_9545_I2C_ADDR 0x70
> +#define CPU2_9545_I2C_ADDR 0x71
> +
> +#define FIBER_PRESENT 0
> +#define CARD_PRESENT  1
> +#define I2C_PORT_SFP  4
> +#define CPU2_I2C_PORT_SFP 5
> +
> +#define SOCKET_0 0
> +#define SOCKET_1 1
> +#define EEPROM_I2C_PORT  4
> +#define EEPROM_PAGE_SIZE 0x40
> +#define MAC_ADDR_LEN 6
> +#define I2C_OFFSET_EEPROM_ETH0   (0xc00)
> +#define I2C_SLAVEADDR_EEPROM (0x52)
> +
> +#pragma pack(1)
> +typedef struct {
> +  UINT16 Crc16;
> +  UINT16 MacLen;
> +  UINT8  Mac[MAC_ADDR_LEN];
> +} NIC_MAC_ADDRESS;
> +#pragma pack()
> +
> +ETH_PRODUCT_DESC gEthPdtDesc[ETH_MAX_PORT] =
> +{
> +{TRUE,   ETH_SPEED_10KM,  ETH_FULL_DUPLEX, ETH_INVALID, ETH_INVALID},
> +{TRUE,   ETH_SPEED_10KM,  ETH_FULL_DUPLEX, ETH_INVALID, ETH_INVALID},
> +{FALSE,  ETH_INVALID, ETH_INVALID, ETH_INVALID, ETH_INVALID},
> +{FALSE,  ETH_INVALID, ETH_INVALID, ETH_INVALID, ETH_INVALID},
> +{TRUE,   ETH_SPEED_1000M, ETH_FULL_DUPLEX, ETH_PHY_MVL88E1512_ID, 0},
> +{TRUE,   ETH_SPEED_1000M, ETH_FULL_DUPLEX, ETH_PHY_MVL88E1512_ID, 1},
> +{FALSE,  ETH_INVALID, ETH_INVALID, ETH_INVALID, ETH_INVALID},
> +{FALSE,  ETH_INVALID, ETH_INVALID, ETH_INVALID, ETH_INVALID}
> +};
> +
> +volatile unsigned char g_2pserveraddr[4][6] = {

VOLATILE UINT8.
No '_' in variable name.
Also, can we please have #defines for that 4 and 6?

> +  {0x00, 0x18, 0x16, 0x29, 0x11, 0x00},
> +  {0x00, 0x18, 0x16, 0x29, 0x11, 0x01},
> +  {0x00, 0x18, 0x16, 0x29, 0x11, 0x02},
> +  {0x00, 0x18, 0x16, 0x29, 0x11, 0x03}
> +};
> +
> +UINT16 crc_tab[256] = {

CrcTable.
Hmm.
This might be useful to add to a core library/driver/protocol. We
don't appear to have it yet. This is another note to the universe.

> +  0x, 0x1021, 0x2042, 0x3063, 0x4084, 0x50A5, 0x60C6, 0x70E7,
> +  0x8108, 0x9129, 0xA14A, 0xB16B, 0xC18C, 0xD1AD, 0xE1CE, 0xF1EF,
> +  0x1231, 0x0210, 0x3273, 0x2252, 0x52B5, 0x4294, 0x72F7, 0x62D6,
> +  0x9339, 0x8318, 0xB37B, 0xA35A, 0xD3BD, 0xC39C, 0xF3FF, 0xE3DE,
> +  0x2462, 0x3443, 0x0420, 0x1401, 0x64E6, 0x74C7, 0x44A4, 0x5485,
> +  0xA56A, 0xB54B, 0x8528, 0x9509, 0xE5EE, 0xF5CF, 0xC5AC, 0xD58D,
> +  0x3653, 0x2672, 0x1611, 0x0630, 0x76D7, 0x66F6, 0x5695, 0x46B4,
> +  0xB75B, 0xA77A, 0x9719, 0x8738, 0xF7DF, 

Re: [edk2] [PATCH] OvmfPkg/PlatformDebugLibIoPort: fix port detection for use in the DXE Core

[Brijesh Singh]

On 08/02/2018 07:30 PM, Laszlo Ersek wrote:

The DXE Core is one of those modules that call
ProcessLibraryConstructorList() manually.

Before DxeMain() [MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c] calls
ProcessLibraryConstructorList(), and through it, our
PlatformDebugLibIoPortConstructor() function, DxeMain() invokes the
DEBUG() macro multiple times. That macro lands in our
PlatformDebugLibIoPortFound() function -- which currently relies on the
"mDebugIoPortFound" global variable that has (not yet) been set by the
constructor. As a result, early debug messages from the DXE Core are lost.

Move the device detection into PlatformDebugLibIoPortFound(), also caching
the fact (not just the result) of the device detection.

(We could introduce a separate DebugLib instance just for the DXE Core,
but the above approach works for all modules that currently consume the
PlatformDebugLibIoPort instance (which means "everything but SEC").)

This restores messages such as:


CoreInitializeMemoryServices:
   BaseAddress - 0x7AF21000 Length - 0x3CDE000 MinimalMemorySizeNeeded - 
0x10F4000


Keep the empty constructor function -- OVMF's DebugLib instances have
always had constructors; we had better not upset constructor dependency
ordering by making our instance(s) constructor-less.

Cc: Ard Biesheuvel 
Cc: Brijesh Singh 
Cc: Jordan Justen 
Fixes: c09d9571300a089c35f5df2773b70edc25050d0d
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---

Notes:
 Brijesh, can you please test this patch on SEV, with and without
 capturing the debug port? (In the first case, the debug log should just
 work; in the second case, the boot should remain fast.) Thanks!



I have tested the patch on SEV and it works well with and without the 
debug flag. thank you!


Tested-by: Brijesh Singh 

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

Re: [edk2] [PATCH edk2-platforms v1 21/38] Silicon/Hisilicon/D0x: Move macro definition to PlatformArch.h

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:09:05PM +0800, Ming Huang wrote:
> From: Sun Yuanchen 
> 
> Unify MemorySubClassDxe by Moving macro definition
> to PlatformArch.h
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sun Yuanchen 
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 

Reviewed-by: Leif Lindholm 
(If you drop the extra Signed-off-by:s)

/
Leif

> ---
>  Silicon/Hisilicon/Drivers/Smbios/MemorySubClassDxe/MemorySubClass.h | 2 --
>  Silicon/Hisilicon/Hi1610/Include/PlatformArch.h | 1 +
>  Silicon/Hisilicon/Hi1616/Include/PlatformArch.h | 1 +
>  Silicon/Hisilicon/Hi1620/Include/PlatformArch.h | 1 +
>  4 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git 
> a/Silicon/Hisilicon/Drivers/Smbios/MemorySubClassDxe/MemorySubClass.h 
> b/Silicon/Hisilicon/Drivers/Smbios/MemorySubClassDxe/MemorySubClass.h
> index c35ce39d61..0c201b4870 100644
> --- a/Silicon/Hisilicon/Drivers/Smbios/MemorySubClassDxe/MemorySubClass.h
> +++ b/Silicon/Hisilicon/Drivers/Smbios/MemorySubClassDxe/MemorySubClass.h
> @@ -44,8 +44,6 @@
>  
>  extern UINT8 MemorySubClassStrings[];
>  
> -#define MAX_DIMM_SIZE   32  // In GB
> -
>  struct SPD_JEDEC_MANUFACTURER
>  {
>  UINT8  MfgIdLSB;
> diff --git a/Silicon/Hisilicon/Hi1610/Include/PlatformArch.h 
> b/Silicon/Hisilicon/Hi1610/Include/PlatformArch.h
> index 03e96cfd31..4843b60536 100644
> --- a/Silicon/Hisilicon/Hi1610/Include/PlatformArch.h
> +++ b/Silicon/Hisilicon/Hi1610/Include/PlatformArch.h
> @@ -26,6 +26,7 @@
>  #define MAX_DIMM3
>  #define MAX_RANK_CH 12
>  #define MAX_RANK_DIMM   4
> +#define MAX_DIMM_SIZE   32  // In GB
>  // Max NUMA node number for each node type
>  #define MAX_NUM_PER_TYPE 8
>  
> diff --git a/Silicon/Hisilicon/Hi1616/Include/PlatformArch.h 
> b/Silicon/Hisilicon/Hi1616/Include/PlatformArch.h
> index 14e9b483af..49618f6559 100644
> --- a/Silicon/Hisilicon/Hi1616/Include/PlatformArch.h
> +++ b/Silicon/Hisilicon/Hi1616/Include/PlatformArch.h
> @@ -26,6 +26,7 @@
>  #define MAX_DIMM3
>  #define MAX_RANK_CH 12
>  #define MAX_RANK_DIMM   4
> +#define MAX_DIMM_SIZE   32  // In GB
>  // Max NUMA node number for each node type
>  #define MAX_NUM_PER_TYPE 8
>  
> diff --git a/Silicon/Hisilicon/Hi1620/Include/PlatformArch.h 
> b/Silicon/Hisilicon/Hi1620/Include/PlatformArch.h
> index ac90e9dfb5..2626751a0d 100644
> --- a/Silicon/Hisilicon/Hi1620/Include/PlatformArch.h
> +++ b/Silicon/Hisilicon/Hi1620/Include/PlatformArch.h
> @@ -26,6 +26,7 @@
>  #define MAX_DIMM2
>  #define MAX_RANK_CH 8
>  #define MAX_RANK_DIMM   4
> +#define MAX_DIMM_SIZE   256  // In GB
>  // Max NUMA node number for each node type
>  #define MAX_NUM_PER_TYPE 8
>  
> -- 
> 2.17.0
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v1 20/38] Platform/Hisilicon/D06: Enable ACPI PPTT

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:09:04PM +0800, Ming Huang wrote:
> From: Heyi Guo 
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Heyi Guo 
> Signed-off-by: Ming Huang 

Reviewed-by: Leif Lindholm 
(if you drop Heyi's Signed-off-by)

/
Leif

> ---
>  Platform/Hisilicon/D06/D06.dsc | 1 +
>  Platform/Hisilicon/D06/D06.fdf | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
> index 6f3786f0eb..43af043cfd 100644
> --- a/Platform/Hisilicon/D06/D06.dsc
> +++ b/Platform/Hisilicon/D06/D06.dsc
> @@ -342,6 +342,7 @@
>Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/AcpiTablesHi1620.inf
>Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf
>  
> +  Silicon/Hisilicon/Hi1620/Pptt/Pptt.inf
>#
># Usb Support
>#
> diff --git a/Platform/Hisilicon/D06/D06.fdf b/Platform/Hisilicon/D06/D06.fdf
> index 586e9ed77e..1c6ee4e0e4 100644
> --- a/Platform/Hisilicon/D06/D06.fdf
> +++ b/Platform/Hisilicon/D06/D06.fdf
> @@ -252,6 +252,8 @@ READ_LOCK_STATUS   = TRUE
>INF RuleOverride=ACPITABLE 
> Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/AcpiTablesHi1620.inf
>INF Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf
>  
> +  INF Silicon/Hisilicon/Hi1620/Pptt/Pptt.inf
> +
>#
>#Network
>#
> -- 
> 2.17.0
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v1 19/38] Hisilicon/Hi1620: Add ACPI PPTT table

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:09:03PM +0800, Ming Huang wrote:
> From: Heyi Guo 
> 
> This driver fetches information from MADT,  so it is adaptable for
> partial good and 1P/2P, since MADT is updated for different
> configurations by certain mechanism.
> 
> Since L2 cache is also private resource of core, so we need to set the
> next level of cache for L1I and L1D, which is important for OS to
> parse cache hierarchy.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Heyi Guo 
> Signed-off-by: Ming Huang 
> ---
>  Silicon/Hisilicon/Hi1620/Pptt/Pptt.c   | 543 
>  Silicon/Hisilicon/Hi1620/Pptt/Pptt.h   |  69 +++
>  Silicon/Hisilicon/Hi1620/Pptt/Pptt.inf |  48 ++
>  3 files changed, 660 insertions(+)
> 
> diff --git a/Silicon/Hisilicon/Hi1620/Pptt/Pptt.c 
> b/Silicon/Hisilicon/Hi1620/Pptt/Pptt.c
> new file mode 100644
> index 00..944b4b9507
> --- /dev/null
> +++ b/Silicon/Hisilicon/Hi1620/Pptt/Pptt.c
> @@ -0,0 +1,543 @@
> +/** @file
> +*
> +*  Copyright (c) 2018, Hisilicon Limited. All rights reserved.
> +*  Copyright (c) 2018, Linaro Limited. All rights reserved.
> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD 
> License
> +*  which accompanies this distribution.  The full text of the license may be 
> found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +*
> +*  Based on the files under Platform/ARM/JunoPkg/AcpiTables/
> +*
> +**/
> +
> +#include "Pptt.h"
> +
> +typedef EFI_ACPI_5_1_GIC_STRUCTURE  
> ACPI_GIC_STRUCTURE;
> +typedef EFI_ACPI_5_1_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER 
> ACPI_MADT_TABLE_HEADER;
> +
> +EFI_ACPI_TABLE_PROTOCOL   *mAcpiTableProtocol = NULL;
> +EFI_ACPI_SDT_PROTOCOL *mAcpiSdtProtocol   = NULL;
> +
> +EFI_ACPI_DESCRIPTION_HEADER mPpttHeader =
> +  ARM_ACPI_HEADER (
> +EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE,
> +EFI_ACPI_DESCRIPTION_HEADER,
> +EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_REVISION
> +  );
> +
> +EFI_ACPI_6_2_PPTT_STRUCTURE_ID mPpttSocketType2[PPTT_SOCKET_COMPONENT_NO] =
> +{
> +  {2, sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_ID), {0, 0}, PPTT_VENDOR_ID, 0, 0, 
> 0, 0, 0}
> +};
> +
> +EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE mPpttCacheType1[PPTT_CACHE_NO];
> +
> +STATIC UINT32 mSocketOffset[MAX_SOCKET];
> +STATIC UINT32 mScclOffset[MAX_SCL];
> +STATIC UINT32 mClusterOffset[MAX_SCL][MAX_CLUSTER_PER_SCL];
> +
> +STATIC
> +VOID
> +InitCacheInfo (
> +  VOID
> +  )
> +{
> +  UINT8Index;
> +  EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE_ATTRIBUTES Type1Attributes;
> +  CSSELR_DATA  CsselrData;
> +  CCSIDR_DATA  CcsidrData;
> +
> +  for (Index = 0; Index < PPTT_CACHE_NO; Index++) {
> +CsselrData.Data = 0;
> +CcsidrData.Data = 0;
> +SetMem (
> +  ,
> +  sizeof (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE_ATTRIBUTES),
> +  0
> +  );
> +
> +if (Index == 0) { //L1I
> +  CsselrData.Bits.InD = 1;
> +  CsselrData.Bits.Level = 0;
> +  Type1Attributes.CacheType  = 1;
> +} else if (Index == 1) {
> +  Type1Attributes.CacheType  = 0;
> +  CsselrData.Bits.Level = Index - 1;
> +} else {
> +  Type1Attributes.CacheType  = 2;
> +  CsselrData.Bits.Level = Index - 1;
> +}
> +
> +CcsidrData.Data = ReadCCSIDR (CsselrData.Data);
> +
> +if (CcsidrData.Bits.Wa == 1) {
> +  Type1Attributes.AllocationType = 
> EFI_ACPI_6_2_CACHE_ATTRIBUTES_ALLOCATION_WRITE;
> +  if (CcsidrData.Bits.Ra == 1) {
> +Type1Attributes.AllocationType = 
> EFI_ACPI_6_2_CACHE_ATTRIBUTES_ALLOCATION_READ_WRITE;
> +  }
> +}
> +
> +if (CcsidrData.Bits.Wt == 1) {
> +  Type1Attributes.WritePolicy = 1;
> +}
> +DEBUG ((DEBUG_INFO,
> +"[Acpi PPTT] Level = %x!CcsidrData = %x!\n",
> +CsselrData.Bits.Level,
> +CcsidrData.Data));
> +
> +mPpttCacheType1[Index].Type = EFI_ACPI_6_2_PPTT_TYPE_CACHE;
> +mPpttCacheType1[Index].Length = sizeof 
> (EFI_ACPI_6_2_PPTT_STRUCTURE_CACHE);
> +mPpttCacheType1[Index].Reserved[0] = 0;
> +mPpttCacheType1[Index].Reserved[1] = 0;
> +mPpttCacheType1[Index].Flags.SizePropertyValid = 1;
> +mPpttCacheType1[Index].Flags.NumberOfSetsValid = 1;
> +mPpttCacheType1[Index].Flags.AssociativityValid = 1;
> +mPpttCacheType1[Index].Flags.AllocationTypeValid = 1;
> +mPpttCacheType1[Index].Flags.CacheTypeValid = 1;
> +mPpttCacheType1[Index].Flags.WritePolicyValid = 1;
> +mPpttCacheType1[Index].Flags.LineSizeValid = 1;
> +mPpttCacheType1[Index].Flags.Reserved = 0;
> +mPpttCacheType1[Index].NextLevelOfCache = 0;
> +
> +if (Index != PPTT_CACHE_NO - 1) {
> + 

Re: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 with OVMF

[Ricardo Araújo]

Hi folks, sorry for the delay! 

I've just applied the patch and got the same error message and empty PCRs. 

Regards, 

Ricardo Araújo Santos - 
www.lsd.ufcg.edu.br/~ricardo 



De: "Zhang, Chao B"  
Para: "Laszlo Ersek" , "Ricardo Araújo" 
, "Marc-André Lureau"  
Cc: edk2-devel@lists.01.org, "Gao, Liming" , "Zeng, Star" 
 
Enviadas: Quinta-feira, 2 de agosto de 2018 21:22:18 
Assunto: RE: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 with 
OVMF 



Tks Lazslo. And please make sure PcdLib is correctly lined in OVMF 




From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Thursday, August 2, 2018 9:14 PM 
To: Zhang, Chao B ; Ricardo Araújo 
; Marc-André Lureau  
Cc: edk2-devel@lists.01.org; Gao, Liming ; Zeng, Star 
 
Subject: Re: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 with 
OVMF 




On 08/02/18 04:04, Zhang, Chao B wrote: 
> Hi Laszlo & Ricardo 
> The patch was intended to reduce the time to read TPM interface ID register. 
> The interface type should not change within boot cycle according to PTP spec. 
> I agree to add some ASSERT after PCDSetxxsS. 
> But It is a core solution without platform change as PCD has been configured 
> as DYN, DYNEx in DEC. I don’t know why you meet Set Failure 
> In OVMF. Here, I include PCD expert to explain this. 

As far as I recall, dynamic PCDs have never behaved as actually settable 
for me unless I added dynamic defaults for them in the OVMF DSC files. 

I never really researched why this was the case, I just accepted that 
the dynamic defaults were apparently necessary. 

Let's wait for Ricardo's response. Perhaps my analysis / suspicion were 
incorrect and it's not actually the "dynamism" of the PCD that's missing 
for OVMF. Ricardo's answer will tell us if there's another issue. 

Thanks 
Laszlo 

> From: Laszlo Ersek [ mailto:ler...@redhat.com ] 
> Sent: Thursday, August 2, 2018 5:49 AM 
> To: Ricardo Araújo < rica...@lsd.ufcg.edu.br >; Zhang, Chao B < 
> chao.b.zh...@intel.com >; Marc-André Lureau < marcandre.lur...@redhat.com > 
> Cc: edk2-devel@lists.01.org 
> Subject: Re: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 
> with OVMF 
> 
> On 08/01/18 19:50, Ricardo Araújo wrote: 
>> The commit I was referring to is: 
>> https://github.com/tianocore/edk2/commit/f15cb995bb3880b77e15afe6facd3da05e599a17
>>  
>> 
>> Regards, 
>> 
>> Ricardo Araujo - 
>> www.lsd.ufcg.edu.br/~ricardo 
>> 
>> - Mensagem original - 
>> De: "Ricardo Araújo" < 
>> rica...@lsd.ufcg.edu.br> 
>> Para: edk2-devel@lists.01.org 
>> Enviadas: Quarta-feira, 1 de agosto de 2018 14:33:45 
>> Assunto: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 
>> with OVMF 
>> 
>> Hi everyone, 
>> 
>> I'm using OVMF with a simulated TPM 2.0 (from 
>> https://github.com/stefanberger/swtpm ) and I noticed lately that PCRs 
>> 0-7 are zeroed after booting the vm (ubuntu 18.04) and the only 
>> message related to this in dmesg is: 
>> 
>> [ 2.286690] tpm_tis 00:06: 2.0 TPM (device-id 0x1, rev-id 1) 
>> [ 2.303753] tpm tpm0: A TPM error (256) occurred continue selftest 
>> [ 2.314199] tpm tpm0: starting up the TPM manually 
>> 
>> I found this started to happen after this commit , previous commits to 
>> that are showing boot time measurements on PCR 0-7 normally and the 
>> error message is gone. Has anyone experienced the same behavior? I 
>> followed the instructions here for building OVMF but I added the 
>> parameters -D TPM2_ENABLE=TRUE -D SECURE_BOOT_ENABLE=TRUE -D 
>> HTTP_BOOT_ENABLE=TRUE. Is there anything else I need to add to enable 
>> these measurements? 
>> 
>> Regards, 
>> 
>> Ricardo Araujo 
>> www.lsd.ufcg.edu.br/~ricardo 
> 
> Thank you for the bug report. It looks like a regression to me, but the 
> details aren't immediately clear. 
> 
> Adding Marc-André who contributed the TPM enablement for OVMF, and Chao 
> Zhang who authored the commit in question. 
> 
> If I recall correctly, in OVMF we decided to never cache the TPM type 
> but always detect it. I could be remembering wrong though. See commit 
> 6cf1880fb5b6 ("OvmfPkg: add customized Tcg2ConfigPei clone", 
> 2018-03-09). 
> 
> Chao Zhang: can you please explain what additional requirements are 
> presented for a platform by commit f15cb995bb38? In OVMF we use a 
> customized Tcg2ConfigPei module (see the commit above). 
> 
> 
> Oh wait, I suspect what's wrong. I believe there are two bugs in commit 
> f15cb995bb38 ("SecurityPkg: Cache TPM interface type info", 2018-06-25): 
> 
> * Bug#1: 
> 
> Commit f15cb995bb38 introduces a new PCD, called 
> "PcdActiveTpmInterfaceType", in section [PcdsDynamic, PcdsDynamicEx] of 
> "SecurityPkg.dec", and makes core modules from SecurityPkg dependent on 
> it. 
> 
> Obviously this means that platforms are required to provide a Dynamic 
> Default for the new PCD in their DSC files, if they 

Re: [edk2] [PATCH edk2-platforms v1 18/38] Silicon/Hisilicon/Setup: Add Setup Item "EnableGOP"

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:09:02PM +0800, Ming Huang wrote:
> From: Yang XinYi 
> 
> Add Setup Item "EnableGOP" for D06, This Item only takes
> effect on SM750

What is SM750? Please add more detail to commit message.

> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Yang XinYi 
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 
> ---
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/MiscConfig.hfr | 7 +++
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/MiscConfig.uni | 8 
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c| 2 +-
>  Silicon/Hisilicon/Include/Library/OemConfigData.h| 1 +
>  4 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/MiscConfig.hfr 
> b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/MiscConfig.hfr
> index 9e3ac73116..c0b6e294a6 100644
> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/MiscConfig.hfr
> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/MiscConfig.hfr
> @@ -38,4 +38,11 @@ form formid = MISC_CONFIG_FORM_ID,
>option text = STRING_TOKEN(STR_ENABLED), value = 1, flags = 0;
>  endoneof;
>  
> +oneof varid   = OEM_CONFIG_DATA.EnableGOP,
> +  prompt  = STRING_TOKEN(STR_ENABLE_GOP_FRAME_BUFFER),
> +  help= STRING_TOKEN(STR_ENABLE_GOP_FRAME_BUFFER_HELP),
> +  option text = STRING_TOKEN(STR_DISABLED), value = 0, flags = DEFAULT;
> +  option text = STRING_TOKEN(STR_ENABLED), value = 1, flags = 0;
> +endoneof;
> +
>  endform;
> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/MiscConfig.uni 
> b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/MiscConfig.uni
> index 5ad1d1df30..0170c84ff6 100644
> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/MiscConfig.uni
> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/MiscConfig.uni
> @@ -30,7 +30,7 @@
> #language fr-FR  "Support SPCR"
>  #string STR_ENABLE_SPCR_HELP   #language en-US  "Enable or Disable 
> SPCR Table."
> #language fr-FR  "Activer ou 
> désactiver la table SPCR."
> -#string STR_ENABLE_GOP_FRAME_BUFFER#language en-US  "Support GOP FB"
> -   #language fr-FR  "Support GOP FB"
> -#string STR_ENABLE_GOP_FRAME_BUFFER_HELP #language en-US  "Enable or 
> Disable GOP frame buffer."
> - #language fr-FR  "Activer ou 
> désactiver Buffer de frame GOP."
> +#string STR_ENABLE_GOP_FRAME_BUFFER#language en-US  "Support GOP FB for 
> SM750"
> +   #language fr-FR  "Soutien GOP FB pour 
> SM750"
> +#string STR_ENABLE_GOP_FRAME_BUFFER_HELP #language en-US  "Enable or 
> Disable GOP frame buffer for SM750."
> + #language fr-FR  "Activer ou 
> désactiver Buffer de frame GOP pour SM750."
> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c 
> b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
> index 586094dfbf..88051493cf 100644
> --- a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c
> @@ -311,7 +311,7 @@ OemConfigUiLibConstructor (
>Configuration.EnableSmmu = 1;
>Configuration.EnableFdtTable = 0;
>Configuration.EnableSpcr = 0;
> -  //Configuration.EnableGOP=0;
> +  Configuration.EnableGOP = 0;

I _think_ I commented to delete that line on a previous patch. If not,
please delete the commented out line from wherever it was introduced.

/
Leif

>//
>//Set the default value of the Ras option
>//
> diff --git a/Silicon/Hisilicon/Include/Library/OemConfigData.h 
> b/Silicon/Hisilicon/Include/Library/OemConfigData.h
> index 478821ae2c..e4d5917046 100644
> --- a/Silicon/Hisilicon/Include/Library/OemConfigData.h
> +++ b/Silicon/Hisilicon/Include/Library/OemConfigData.h
> @@ -61,6 +61,7 @@ typedef struct {
>UINT8 EnableSmmu;
>UINT8 EnableFdtTable;
>UINT8 EnableSpcr;
> +  UINT8 EnableGOP;
>/*RAS Config*/
>UINT8 EnRasSupport;
>UINT8 EnPoison;
> -- 
> 2.17.0
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v1 17/38] Silicon/Hisilicon/D06: Optimize HNS config CDR post time

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:09:01PM +0800, Ming Huang wrote:
> From: shaochangliang 
> 
> Use I2C 400KB speed for config CDR
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: shaochangliang 
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 
> ---
>  Silicon/Hisilicon/Library/I2CLib/I2CLib.c | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/Silicon/Hisilicon/Library/I2CLib/I2CLib.c 
> b/Silicon/Hisilicon/Library/I2CLib/I2CLib.c
> index fa8c510f36..8d87336375 100644
> --- a/Silicon/Hisilicon/Library/I2CLib/I2CLib.c
> +++ b/Silicon/Hisilicon/Library/I2CLib/I2CLib.c
> @@ -28,6 +28,9 @@
>  #include "I2CLibInternal.h"
>  #include "I2CHw.h"
>  
> +#define I2C_100KB_SPEED 0x1
> +#define I2C_400KB_SPEED 0x2
> +
>  VOID I2C_Delay(UINT32 ulCount)
>  {
>  MicroSecondDelay(ulCount);
> @@ -149,7 +152,14 @@ I2CInit(UINT32 Socket, UINT32 Port, SPEED_MODE SpeedMode)
>  
>  I2C_REG_READ(Base + I2C_CON_OFFSET, I2cControlReg.Val32);
>  I2cControlReg.bits.master = 1;
> -I2cControlReg.bits.spedd = 0x1;
> +if(Normal == SpeedMode)
> +{

'{' on previous line.

> +I2cControlReg.bits.spedd = I2C_100KB_SPEED;
> +}
> +else

'else' on previous line.

> +{

'{' on previous line.

/
Leif

> +I2cControlReg.bits.spedd = I2C_400KB_SPEED;

spedd?
That looks as a typo in the struct definition.
It should probably be Speed.

Can you please provide a separate patch for that preceding this one?

/
Leif

> +}
>  I2cControlReg.bits.restart_en = 1;
>  I2cControlReg.bits.slave_disable = 1;
>  I2C_REG_WRITE(Base + I2C_CON_OFFSET,I2cControlReg.Val32);
> -- 
> 2.17.0
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v1 15/38] Silicon/Hisilicon/I2C: Optimize I2C library

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:08:59PM +0800, Ming Huang wrote:
> The hunt of waiting TX/TX finish is used by ten times,
> so move there hunts to a function CheckI2CTimeOut.

I approve of this change, but the subject is inaccurate.

Please change 'optimize' to 'refactor'.

Some style comments below.

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 
> ---
>  Silicon/Hisilicon/Library/I2CLib/I2CHw.h  |   4 +
>  Silicon/Hisilicon/Library/I2CLib/I2CLib.c | 176 +++-
>  2 files changed, 65 insertions(+), 115 deletions(-)
> 
> diff --git a/Silicon/Hisilicon/Library/I2CLib/I2CHw.h 
> b/Silicon/Hisilicon/Library/I2CLib/I2CHw.h
> index aa561e929c..fa954c7937 100644
> --- a/Silicon/Hisilicon/Library/I2CLib/I2CHw.h
> +++ b/Silicon/Hisilicon/Library/I2CLib/I2CHw.h
> @@ -265,5 +265,9 @@
>   UINT32  Val32;
>   } I2C0_ENABLE_STATUS_U;
>  
> +typedef enum {
> +  I2CTx,
> +  I2CRx
> +} I2CTransfer;
>  
>  #endif
> diff --git a/Silicon/Hisilicon/Library/I2CLib/I2CLib.c 
> b/Silicon/Hisilicon/Library/I2CLib/I2CLib.c
> index ecd2f07c4d..16636987a6 100644
> --- a/Silicon/Hisilicon/Library/I2CLib/I2CLib.c
> +++ b/Silicon/Hisilicon/Library/I2CLib/I2CLib.c
> @@ -234,6 +234,42 @@ I2C_GetRxStatus(UINT32 Socket,UINT8 Port)
>  return ulFifo.bits.rxflr;
>  }
>  
> +EFI_STATUS
> +EFIAPI
> +CheckI2CTimeOut (
> +  UINT32 Socket,
> +  UINT8  Port,
> +  I2CTransfer Transfer
> +)
> +{
> +  UINT32 ulTimes = 0;
> +  UINT32 ulFifo;

Are these ul short for unsigned long?
That's called Hungarian notation and explicitly forbidden by the
coding style. Please drop, throughout the patch.

> +
> +  if (Transfer == I2CTx) {
> +ulFifo = I2C_GetTxStatus (Socket,Port);

Space after ','.

> +while (ulFifo != 0) {
> +  I2C_Delay(2);
> +  if (++ulTimes > I2C_READ_TIMEOUT) {
> +(VOID)I2C_Disable (Socket, Port);
> +return EFI_TIMEOUT;
> +  }
> +  ulFifo = I2C_GetTxStatus (Socket,Port);

Space after ','.

> +}
> +  }
> +  else {
> +ulFifo = I2C_GetRxStatus (Socket,Port);

Space after ','.

> +while (ulFifo == 0) {
> +  I2C_Delay(2);
> +  if (++ulTimes > I2C_READ_TIMEOUT) {
> +(VOID)I2C_Disable (Socket, Port);
> +return EFI_TIMEOUT;
> +  }
> +  ulFifo = I2C_GetRxStatus (Socket,Port);
> +}
> +  }
> +  return EFI_SUCCESS;
> +}
> +
>  EFI_STATUS
>  EFIAPI
>  WriteBeforeRead(I2C_DEVICE *I2cInfo, UINT32 ulLength, UINT8 *pBuf)
> @@ -247,17 +283,11 @@ WriteBeforeRead(I2C_DEVICE *I2cInfo, UINT32 ulLength, 
> UINT8 *pBuf)
>  
>  I2C_SetTarget(I2cInfo->Socket,I2cInfo->Port,I2cInfo->SlaveDeviceAddress);
>  
> -ulFifo = I2C_GetTxStatus(I2cInfo->Socket,I2cInfo->Port);
> -while(0 != ulFifo)
> -{
> -I2C_Delay(2);
> -if(++ulTimes > I2C_READ_TIMEOUT)
> -{
> -return EFI_TIMEOUT;
> -}
> -ulFifo = I2C_GetTxStatus(I2cInfo->Socket,I2cInfo->Port);
> +if (CheckI2CTimeOut (I2cInfo->Socket,I2cInfo->Port,I2CTx) == 
> EFI_TIMEOUT) {

Space after ','.

> +  return EFI_TIMEOUT;
>  }
>  
> +ulFifo = 0;
>  for(ulCnt = 0; ulCnt < ulLength; ulCnt++)
>  {
>  ulTimes = 0;
> @@ -275,17 +305,8 @@ WriteBeforeRead(I2C_DEVICE *I2cInfo, UINT32 ulLength, 
> UINT8 *pBuf)
>  ulFifo = I2C_GetTxStatus(I2cInfo->Socket,I2cInfo->Port);
>  }
>  
> -ulFifo = I2C_GetTxStatus(I2cInfo->Socket,I2cInfo->Port);
> -ulTimes = 0;
> -while(0 != ulFifo)
> -{
> -I2C_Delay(2);
> -
> -if(++ulTimes > I2C_READ_TIMEOUT)
> -{
> -return EFI_TIMEOUT;
> -}
> -ulFifo = I2C_GetTxStatus(I2cInfo->Socket,I2cInfo->Port);
> +if (CheckI2CTimeOut (I2cInfo->Socket,I2cInfo->Port,I2CTx) == 
> EFI_TIMEOUT) {

Space after ','.

> +  return EFI_TIMEOUT;
>  }
>  
>  return EFI_SUCCESS;
> @@ -313,16 +334,8 @@ I2CWrite(I2C_DEVICE *I2cInfo, UINT16 InfoOffset, UINT32 
> ulLength, UINT8 *pBuf)
>  
>  I2C_SetTarget(I2cInfo->Socket,I2cInfo->Port,I2cInfo->SlaveDeviceAddress);
>  
> -ulFifo = I2C_GetTxStatus(I2cInfo->Socket,I2cInfo->Port);
> -while(0 != ulFifo)
> -{
> -I2C_Delay(2);
> -if(++ulTimes > I2C_READ_TIMEOUT)
> -{
> -(VOID)I2C_Disable(I2cInfo->Socket, I2cInfo->Port);
> -return EFI_TIMEOUT;
> -}
> -ulFifo = I2C_GetTxStatus(I2cInfo->Socket,I2cInfo->Port);
> +if (CheckI2CTimeOut (I2cInfo->Socket,I2cInfo->Port,I2CTx) == 
> EFI_TIMEOUT) {

Space after ','.

> +  return EFI_TIMEOUT;
>  }
>  
>  
> @@ -336,17 +349,8 @@ I2CWrite(I2C_DEVICE *I2cInfo, UINT16 InfoOffset, UINT32 
> ulLength, UINT8 *pBuf)
>  I2C_REG_WRITE(Base + I2C_DATA_CMD_OFFSET, InfoOffset & 0xff);
>  }
>  
> -ulFifo = I2C_GetTxStatus(I2cInfo->Socket,I2cInfo->Port);
> -ulTimes = 0;
> -while(0 != ulFifo)
> -{
> -I2C_Delay(2);
> -if(++ulTimes > I2C_READ_TIMEOUT)
> -{
> -

[edk2] [PATCH v2 7/7] ShellPkg/UefiShellLib: rebase ShellOpenFileByDevicePath() to UefiLib API

[Laszlo Ersek]

Replace the "old shell method" implementation in
ShellOpenFileByDevicePath() with EfiOpenFileByDevicePath() from UefiLib,
correcting the following issues:

- code duplication between this module and other modules,
- local variable name "EfiSimpleFileSystemProtocol" starting with "Efi"
  prefix,
- bogus "FileHandle = NULL" assignments,
- leaking "Handle1" when the device path type/subtype check or the
  realignment-motivated AllocateCopyPool() fails in the loop.

Cc: Jaben Carsey 
Cc: Ruiyu Ni 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
Reviewed-by: Jaben Carsey 
---

Notes:
v2:
- pick up Jaben's R-b

 ShellPkg/Library/UefiShellLib/UefiShellLib.inf |   1 -
 ShellPkg/Library/UefiShellLib/UefiShellLib.c   | 113 +---
 2 files changed, 3 insertions(+), 111 deletions(-)

diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.inf 
b/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
index 38d9a4b81f5f..aacddbbf9765 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
@@ -51,7 +51,6 @@ [LibraryClasses]
   SortLib
 
 [Protocols]
-  gEfiSimpleFileSystemProtocolGuid  ## SOMETIMES_CONSUMES
   gEfiUnicodeCollation2ProtocolGuid ## CONSUMES
 
   # shell 2.0
diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c 
b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
index 18c3be4a8bc7..f04adbb63ffe 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
@@ -504,12 +504,7 @@ ShellOpenFileByDevicePath(
 {
   CHAR16  *FileName;
   EFI_STATUS  Status;
-  EFI_SIMPLE_FILE_SYSTEM_PROTOCOL *EfiSimpleFileSystemProtocol;
-  EFI_FILE_PROTOCOL   *Handle1;
-  EFI_FILE_PROTOCOL   *Handle2;
-  CHAR16  *FnafPathName;
-  UINTN   PathLen;
-  EFI_HANDLE  DeviceHandle;
+  EFI_FILE_PROTOCOL   *File;
 
   if (FilePath == NULL || FileHandle == NULL) {
 return (EFI_INVALID_PARAMETER);
@@ -535,117 +530,15 @@ ShellOpenFileByDevicePath(
   //
   // use old shell method.
   //
-  Status = gBS->LocateDevicePath (,
-  FilePath,
-  );
+  Status = EfiOpenFileByDevicePath (FilePath, , OpenMode, Attributes);
   if (EFI_ERROR (Status)) {
 return Status;
   }
-  Status = gBS->OpenProtocol(DeviceHandle,
- ,
- (VOID**),
- gImageHandle,
- NULL,
- EFI_OPEN_PROTOCOL_GET_PROTOCOL);
-  if (EFI_ERROR (Status)) {
-return Status;
-  }
-  Status = 
EfiSimpleFileSystemProtocol->OpenVolume(EfiSimpleFileSystemProtocol, );
-  if (EFI_ERROR (Status)) {
-FileHandle = NULL;
-return Status;
-  }
-
-  //
-  // go down directories one node at a time.
-  //
-  while (!IsDevicePathEnd (*FilePath)) {
-//
-// For file system access each node should be a file path component
-//
-if (DevicePathType(*FilePath) != MEDIA_DEVICE_PATH ||
-DevicePathSubType (*FilePath) != MEDIA_FILEPATH_DP
-   ) {
-  FileHandle = NULL;
-  return (EFI_INVALID_PARAMETER);
-}
-//
-// Open this file path node
-//
-Handle2  = Handle1;
-Handle1 = NULL;
-
-//
-// File Name Alignment Fix (FNAF)
-// Handle2->Open may be incapable of handling a unaligned CHAR16 data.
-// The structure pointed to by FilePath may be not CHAR16 aligned.
-// This code copies the potentially unaligned PathName data from the
-// FilePath structure to the aligned FnafPathName for use in the
-// calls to Handl2->Open.
-//
-
-//
-// Determine length of PathName, in bytes.
-//
-PathLen = DevicePathNodeLength (*FilePath) - SIZE_OF_FILEPATH_DEVICE_PATH;
-
-//
-// Allocate memory for the aligned copy of the string Extra allocation is 
to allow for forced alignment
-// Copy bytes from possibly unaligned location to aligned location
-//
-FnafPathName = AllocateCopyPool(PathLen, (UINT8 
*)((FILEPATH_DEVICE_PATH*)*FilePath)->PathName);
-if (FnafPathName == NULL) {
-  return EFI_OUT_OF_RESOURCES;
-}
-
-//
-// Try to test opening an existing file
-//
-Status = Handle2->Open (
-  Handle2,
-  ,
-  FnafPathName,
-  OpenMode &~EFI_FILE_MODE_CREATE,
-  0
- );
-
-//
-// see if the error was that it needs to be created
-//
-if ((EFI_ERROR (Status)) && (OpenMode != (OpenMode 
&~EFI_FILE_MODE_CREATE))) {
-  Status = Handle2->Open (
-Handle2,
-,
-

[edk2] [PATCH v2 2/7] IntelFrameworkPkg/FrameworkUefiLib: introduce EfiOpenFileByDevicePath()

[Laszlo Ersek]

Copy the EfiOpenFileByDevicePath() implementation from the previous
(MdePkg/UefiLib) patch to FrameworkUefiLib.

(Note that the FrameworkUefiLib instance too will be updated for
.)

Cc: Liming Gao 
Cc: Michael D Kinney 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---

Notes:
v2:

- new patch: with the MdePkg instance finalized, include the
  implementation in IntelFrameworkPkg too [Liming, Laszlo]

- build-tested with:

  build -a IA32 -a X64 -b NOOPT -t GCC48 \
-p IntelFrameworkPkg/IntelFrameworkPkg.dsc \
-m IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf

 IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf |   1 +
 IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c| 227 

 2 files changed, 228 insertions(+)

diff --git a/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf 
b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
index 8cff19fa3124..182d20fca051 100644
--- a/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
+++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
@@ -61,6 +61,7 @@ [Protocols]
   gEfiSimpleTextOutProtocolGuid ## SOMETIMES_CONSUMES
   gEfiGraphicsOutputProtocolGuid## SOMETIMES_CONSUMES
   gEfiHiiFontProtocolGuid   ## SOMETIMES_CONSUMES
+  gEfiSimpleFileSystemProtocolGuid  ## SOMETIMES_CONSUMES
   gEfiComponentNameProtocolGuid ## SOMETIMES_PRODUCES
   gEfiComponentName2ProtocolGuid## SOMETIMES_PRODUCES
   gEfiDriverConfigurationProtocolGuid   ## SOMETIMES_PRODUCES
diff --git a/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c 
b/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
index 0aa4506b8f4d..b283d775b470 100644
--- a/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
+++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
@@ -1691,3 +1691,230 @@ EfiLocateProtocolBuffer (
 
   return EFI_SUCCESS;
 }
+
+/**
+  Open or create a file or directory, possibly creating the chain of
+  directories leading up to the directory.
+
+  EfiOpenFileByDevicePath() first locates EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on
+  FilePath, and opens the root directory of that filesystem with
+  EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.OpenVolume().
+
+  On the remaining device path, the longest initial sequence of
+  FILEPATH_DEVICE_PATH nodes is node-wise traversed with
+  EFI_FILE_PROTOCOL.Open(). For the pathname fragment specified by each
+  traversed FILEPATH_DEVICE_PATH node, EfiOpenFileByDevicePath() first masks
+  EFI_FILE_MODE_CREATE out of OpenMode, and passes 0 for Attributes. If
+  EFI_FILE_PROTOCOL.Open() fails, and OpenMode includes EFI_FILE_MODE_CREATE,
+  then the operation is retried with the caller's OpenMode and Attributes
+  unmodified.
+
+  (As a consequence, if OpenMode includes EFI_FILE_MODE_CREATE, and Attributes
+  includes EFI_FILE_DIRECTORY, and each FILEPATH_DEVICE_PATH specifies a single
+  pathname component, then EfiOpenFileByDevicePath() ensures that the specified
+  series of subdirectories exist on return.)
+
+  The EFI_FILE_PROTOCOL identified by the last FILEPATH_DEVICE_PATH node is
+  output to the caller; intermediate EFI_FILE_PROTOCOL instances are closed. If
+  there are no FILEPATH_DEVICE_PATH nodes past the node that identifies the
+  filesystem, then the EFI_FILE_PROTOCOL of the root directory of the
+  filesystem is output to the caller. If a device path node that is different
+  from FILEPATH_DEVICE_PATH is encountered relative to the filesystem, the
+  traversal is stopped with an error, and a NULL EFI_FILE_PROTOCOL is output.
+
+  @param[in,out] FilePath  On input, the device path to the file or directory
+   to open or create. The caller is responsible for
+   ensuring that the device path pointed-to by FilePath
+   is well-formed. On output, FilePath points one past
+   the last node in the original device path that has
+   been successfully processed. FilePath is set on
+   output even if EfiOpenFileByDevicePath() returns an
+   error.
+
+  @param[out] File On error, File is set to NULL. On success, File is
+   set to the EFI_FILE_PROTOCOL of the root directory
+   of the filesystem, if there are no
+   FILEPATH_DEVICE_PATH nodes in FilePath; otherwise,
+   File is set to the EFI_FILE_PROTOCOL identified by
+   the last node in FilePath.
+
+  @param[in] OpenMode  The OpenMode parameter to pass to
+   

[edk2] [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs

[Laszlo Ersek]

Repo:   https://github.com/lersek/edk2.git
Branch: open_file_by_devpath_tiano_1008_v2

This is version 2 of the patch set that was originally posted at:

  https://lists.01.org/pipermail/edk2-devel/2018-July/027253.html

for .

Changes are noted on every patch.

The cumulative code difference is very small (not counting the
FrameworkUefiLib copy of the function), so I'm including it here for
easier review:

> diff --git a/MdePkg/Include/Library/UefiLib.h 
> b/MdePkg/Include/Library/UefiLib.h
> index 2468bf2aee80..f950f1c9c648 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -1554,9 +1554,11 @@ EfiLocateProtocolBuffer (
>traversal is stopped with an error, and a NULL EFI_FILE_PROTOCOL is output.
>
>@param[in,out] FilePath  On input, the device path to the file or directory
> -   to open or create. On output, FilePath points one
> -   past the last node in the original device path 
> that
> -   has been successfully processed. FilePath is set 
> on
> +   to open or create. The caller is responsible for
> +   ensuring that the device path pointed-to by 
> FilePath
> +   is well-formed. On output, FilePath points one 
> past
> +   the last node in the original device path that has
> +   been successfully processed. FilePath is set on
> output even if EfiOpenFileByDevicePath() returns 
> an
> error.
>
> diff --git a/MdePkg/Library/UefiLib/UefiLib.c 
> b/MdePkg/Library/UefiLib/UefiLib.c
> index d3e290178cd9..7bcac5613768 100644
> --- a/MdePkg/Library/UefiLib/UefiLib.c
> +++ b/MdePkg/Library/UefiLib/UefiLib.c
> @@ -1751,9 +1751,11 @@ EfiLocateProtocolBuffer (
>traversal is stopped with an error, and a NULL EFI_FILE_PROTOCOL is output.
>
>@param[in,out] FilePath  On input, the device path to the file or directory
> -   to open or create. On output, FilePath points one
> -   past the last node in the original device path 
> that
> -   has been successfully processed. FilePath is set 
> on
> +   to open or create. The caller is responsible for
> +   ensuring that the device path pointed-to by 
> FilePath
> +   is well-formed. On output, FilePath points one 
> past
> +   the last node in the original device path that has
> +   been successfully processed. FilePath is set on
> output even if EfiOpenFileByDevicePath() returns 
> an
> error.
>
> @@ -1808,6 +1810,10 @@ EfiOpenFileByDevicePath (
>EFI_HANDLE  FileSystemHandle;
>EFI_SIMPLE_FILE_SYSTEM_PROTOCOL *FileSystem;
>EFI_FILE_PROTOCOL   *LastFile;
> +  FILEPATH_DEVICE_PATH*FilePathNode;
> +  CHAR16  *AlignedPathName;
> +  CHAR16  *PathName;
> +  EFI_FILE_PROTOCOL   *NextFile;
>
>if (File == NULL) {
>  return EFI_INVALID_PARAMETER;
> @@ -1854,15 +1860,6 @@ EfiOpenFileByDevicePath (
>// Traverse the device path nodes relative to the filesystem.
>//
>while (!IsDevicePathEnd (*FilePath)) {
> -//
> -// Keep local variables that relate to the current device path node 
> tightly
> -// scoped.
> -//
> -FILEPATH_DEVICE_PATH *FilePathNode;
> -CHAR16   *AlignedPathName;
> -CHAR16   *PathName;
> -EFI_FILE_PROTOCOL*NextFile;
> -
>  if (DevicePathType (*FilePath) != MEDIA_DEVICE_PATH ||
>  DevicePathSubType (*FilePath) != MEDIA_FILEPATH_DP) {
>Status = EFI_INVALID_PARAMETER;
> @@ -1942,6 +1939,10 @@ EfiOpenFileByDevicePath (
>  CloseLastFile:
>LastFile->Close (LastFile);
>
> +  //
> +  // We are on the error path; we must have set an error Status for returning
> +  // to the caller.
> +  //
>ASSERT (EFI_ERROR (Status));
>return Status;
>  }
> diff --git 
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
>  
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
> index 312a92d7461a..aef85c470143 100644
> --- 
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
> +++ 
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
> @@ -163,7 +163,7 @@ UpdatePage(
>
>gSecureBootPrivateData->FileContext->FileName = FileName;
>
> -  EfiOpenFileByDevicePath(
> +  EfiOpenFileByDevicePath (
>  ,
>  >FileContext->FHandle,
>  EFI_FILE_MODE_READ,

Cc: Chao Zhang 
Cc: Eric Dong 
Cc: Jaben Carsey 
Cc: Jiaxin Wu 
Cc: 

[edk2] [PATCH v2 5/7] SecurityPkg/SecureBootConfigDxe: replace OpenFileByDevicePath() with UefiLib API

[Laszlo Ersek]

Replace the OpenFileByDevicePath() function with EfiOpenFileByDevicePath()
from UefiLib, correcting the following issues:

- imprecise comments on OpenFileByDevicePath(),
- code duplication between this module and other modules,
- local variable name "EfiSimpleFileSystemProtocol" starting with "Efi"
  prefix,
- bogus "FileHandle = NULL" assignments,
- leaking "Handle1" when the device path type/subtype check or the
  realignment-motivated AllocateCopyPool() fails in the loop,
- stale SHELL_FILE_HANDLE reference in a comment.

Cc: Chao Zhang 
Cc: Jiewen Yao 
Cc: Roman Bacik 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
Reviewed-by: Chao Zhang 
Reviewed-by: Jaben Carsey 
---

Notes:
v2:

- pick up Chao's and Jaben's R-b's

- insert a space character between "EfiOpenFileByDevicePath" and "(" --
  it was missing from the pre-patch code too

 SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf  
  |   1 -
 
SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
 | 151 +---
 2 files changed, 1 insertion(+), 151 deletions(-)

diff --git 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
index 487fc8cda917..caf95ddac7d9 100644
--- 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+++ 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
@@ -114,7 +114,6 @@ [Guids]
 [Protocols]
   gEfiHiiConfigAccessProtocolGuid   ## PRODUCES
   gEfiDevicePathProtocolGuid## PRODUCES
-  gEfiSimpleFileSystemProtocolGuid  ## SOMETIMES_CONSUMES
   gEfiBlockIoProtocolGuid   ## SOMETIMES_CONSUMES
 
 [Depex]
diff --git 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
index 2a26c20f394c..aef85c470143 100644
--- 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
+++ 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
@@ -80,155 +80,6 @@ CleanUpPage (
 );
 }
 
-/**
-  This function will open a file or directory referenced by DevicePath.
-
-  This function opens a file with the open mode according to the file path. The
-  Attributes is valid only for EFI_FILE_MODE_CREATE.
-
-  @param[in, out]  FilePathOn input, the device path to the file.
-   On output, the remaining device path.
-  @param[out]  FileHandle  Pointer to the file handle.
-  @param[in]   OpenModeThe mode to open the file with.
-  @param[in]   Attributes  The file's file attributes.
-
-  @retval EFI_SUCCESS  The information was set.
-  @retval EFI_INVALID_PARAMETEROne of the parameters has an invalid value.
-  @retval EFI_UNSUPPORTED  Could not open the file path.
-  @retval EFI_NOT_FOUNDThe specified file could not be found on the
-   device or the file system could not be 
found on
-   the device.
-  @retval EFI_NO_MEDIA The device has no medium.
-  @retval EFI_MEDIA_CHANGEDThe device has a different medium in it or 
the
-   medium is no longer supported.
-  @retval EFI_DEVICE_ERROR The device reported an error.
-  @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
-  @retval EFI_WRITE_PROTECTED  The file or medium is write protected.
-  @retval EFI_ACCESS_DENIEDThe file was opened read only.
-  @retval EFI_OUT_OF_RESOURCES Not enough resources were available to open 
the
-   file.
-  @retval EFI_VOLUME_FULL  The volume is full.
-**/
-EFI_STATUS
-EFIAPI
-OpenFileByDevicePath(
-  IN OUT EFI_DEVICE_PATH_PROTOCOL **FilePath,
-  OUT EFI_FILE_HANDLE *FileHandle,
-  IN UINT64   OpenMode,
-  IN UINT64   Attributes
-  )
-{
-  EFI_STATUS  Status;
-  EFI_SIMPLE_FILE_SYSTEM_PROTOCOL *EfiSimpleFileSystemProtocol;
-  EFI_FILE_PROTOCOL   *Handle1;
-  EFI_FILE_PROTOCOL   *Handle2;
-  EFI_HANDLE  DeviceHandle;
-  CHAR16  *PathName;
-  UINTN   PathLength;
-
-  if ((FilePath == NULL || FileHandle == NULL)) {
-return EFI_INVALID_PARAMETER;
-  }
-
-  Status = gBS->LocateDevicePath (
-  ,
-  FilePath,
-  
-  );
-  if (EFI_ERROR (Status)) {
-return Status;
-  }
-
-  Status = gBS->OpenProtocol(
-  DeviceHandle,

[edk2] [PATCH v2 4/7] NetworkPkg/TlsAuthConfigDxe: replace OpenFileByDevicePath() with UefiLib API

[Laszlo Ersek]

Replace the OpenFileByDevicePath() function with EfiOpenFileByDevicePath()
from UefiLib, correcting the following issues:

- imprecise comments on OpenFileByDevicePath(),
- code duplication between this module and other modules,
- local variable name "EfiSimpleFileSystemProtocol" starting with "Efi"
  prefix,
- bogus "FileHandle = NULL" assignments,
- passing a potentially unaligned "FILEPATH_DEVICE_PATH.PathName" field to
  a protocol member function (forbidden by the UEFI spec),
- leaking "Handle1" when the device path type/subtype check fails in the
  loop,
- stale SHELL_FILE_HANDLE reference in a comment.

Cc: Jiaxin Wu 
Cc: Siyuan Fu 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
Reviewed-by: Jiaxin Wu 
Reviewed-by: Jaben Carsey 
---

Notes:
v2:
- pick up Jiaxin's and Jaben's R-b's

 NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf |   1 -
 NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c  | 141 +---
 2 files changed, 1 insertion(+), 141 deletions(-)

diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf 
b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
index 3cfcdb9983f1..e5face7b4de5 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
@@ -57,7 +57,6 @@ [LibraryClasses]
 [Protocols]
   gEfiDevicePathProtocolGuid## PRODUCES
   gEfiHiiConfigAccessProtocolGuid   ## PRODUCES
-  gEfiSimpleFileSystemProtocolGuid  ## SOMETIMES_CONSUMES
 
 [Guids]
   gTlsAuthConfigGuid## PRODUCES  ## GUID
diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c 
b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
index 31450b3e4a1b..7259c5e82f61 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
@@ -574,145 +574,6 @@ ON_EXIT:
   return Status;
 }
 
-/**
-  This function will open a file or directory referenced by DevicePath.
-
-  This function opens a file with the open mode according to the file path. The
-  Attributes is valid only for EFI_FILE_MODE_CREATE.
-
-  @param[in, out]  FilePathOn input, the device path to the file.
-   On output, the remaining device path.
-  @param[out]  FileHandle  Pointer to the file handle.
-  @param[in]   OpenModeThe mode to open the file with.
-  @param[in]   Attributes  The file's file attributes.
-
-  @retval EFI_SUCCESS  The information was set.
-  @retval EFI_INVALID_PARAMETEROne of the parameters has an invalid value.
-  @retval EFI_UNSUPPORTED  Could not open the file path.
-  @retval EFI_NOT_FOUNDThe specified file could not be found on the
-   device or the file system could not be 
found on
-   the device.
-  @retval EFI_NO_MEDIA The device has no medium.
-  @retval EFI_MEDIA_CHANGEDThe device has a different medium in it or 
the
-   medium is no longer supported.
-  @retval EFI_DEVICE_ERROR The device reported an error.
-  @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
-  @retval EFI_WRITE_PROTECTED  The file or medium is write protected.
-  @retval EFI_ACCESS_DENIEDThe file was opened read only.
-  @retval EFI_OUT_OF_RESOURCES Not enough resources were available to open 
the
-   file.
-  @retval EFI_VOLUME_FULL  The volume is full.
-**/
-EFI_STATUS
-EFIAPI
-OpenFileByDevicePath (
-  IN OUT EFI_DEVICE_PATH_PROTOCOL **FilePath,
-  OUT EFI_FILE_HANDLE *FileHandle,
-  IN UINT64   OpenMode,
-  IN UINT64   Attributes
-  )
-{
-  EFI_STATUS  Status;
-  EFI_SIMPLE_FILE_SYSTEM_PROTOCOL *EfiSimpleFileSystemProtocol;
-  EFI_FILE_PROTOCOL   *Handle1;
-  EFI_FILE_PROTOCOL   *Handle2;
-  EFI_HANDLE  DeviceHandle;
-
-  if ((FilePath == NULL || FileHandle == NULL)) {
-return EFI_INVALID_PARAMETER;
-  }
-
-  Status = gBS->LocateDevicePath (
-  ,
-  FilePath,
-  
-  );
-  if (EFI_ERROR (Status)) {
-return Status;
-  }
-
-  Status = gBS->OpenProtocol(
-  DeviceHandle,
-  ,
-  (VOID**),
-  gImageHandle,
-  NULL,
-  EFI_OPEN_PROTOCOL_GET_PROTOCOL
-  );
-  if (EFI_ERROR (Status)) {
-return Status;
-  }
-
-  Status = 
EfiSimpleFileSystemProtocol->OpenVolume(EfiSimpleFileSystemProtocol, );
-  if (EFI_ERROR (Status)) {
-FileHandle = NULL;
-return Status;
-  }
-
-  //
-  // go down directories one node at a time.
-  //
-  while (!IsDevicePathEnd 

[edk2] [PATCH v2 1/7] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()

[Laszlo Ersek]

The EfiOpenFileByDevicePath() function centralizes functionality from

- MdeModulePkg/Universal/Disk/RamDiskDxe
- NetworkPkg/TlsAuthConfigDxe
- SecurityPkg/VariableAuthenticated/SecureBootConfigDxe
- ShellPkg/Library/UefiShellLib

unifying the implementation and fixing various bugs.

(Ray suggested that we eliminate the special handling of
EFI_FILE_MODE_CREATE in the "OpenMode" input parameter as well. We plan to
implement that separately, under
.)

Cc: Chao Zhang 
Cc: Eric Dong 
Cc: Jaben Carsey 
Cc: Jiaxin Wu 
Cc: Jiewen Yao 
Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Roman Bacik 
Cc: Ruiyu Ni 
Cc: Siyuan Fu 
Cc: Star Zeng 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---

Notes:
v2:

- add the following sentence to the FilePath parameter's documentation:
  "The caller is responsible for ensuring that the device path
  pointed-to by FilePath is well-formed." [Jiewen]

- lift the definition of the local variables that relate to the current
  device path node from the loop to the top of the function
  ("FilePathNode", "AlignedPathName", "PathName", "NextFile") [Ray]

- report new TianoCore BZ
   -- "don't
  distinguish EFI_FILE_MODE_CREATE in EfiOpenFileByDevicePath() /
  OpenMode" --, and reference it in the commit message, as future work
  [Ray]

- explain ASSERT (EFI_ERROR (Status)) -- as opposed to
  ASSERT_EFI_ERROR (Status) -- with a code comment [Jaben, Ray]

- pick up none of the received Reviewed-by tags (namely from Jaben,
  Liming, and Ray) due to the *sum* of changes above

 MdePkg/Library/UefiLib/UefiLib.inf |   1 +
 MdePkg/Include/Library/UefiLib.h   |  88 
 MdePkg/Library/UefiLib/UefiLib.c   | 227 
 3 files changed, 316 insertions(+)

diff --git a/MdePkg/Library/UefiLib/UefiLib.inf 
b/MdePkg/Library/UefiLib/UefiLib.inf
index f69f0a43b576..a6c739ef3d6d 100644
--- a/MdePkg/Library/UefiLib/UefiLib.inf
+++ b/MdePkg/Library/UefiLib/UefiLib.inf
@@ -68,6 +68,7 @@ [Protocols]
   gEfiSimpleTextOutProtocolGuid   ## SOMETIMES_CONSUMES
   gEfiGraphicsOutputProtocolGuid  ## SOMETIMES_CONSUMES
   gEfiHiiFontProtocolGuid ## SOMETIMES_CONSUMES
+  gEfiSimpleFileSystemProtocolGuid## SOMETIMES_CONSUMES
   gEfiUgaDrawProtocolGuid | gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport  
   ## SOMETIMES_CONSUMES # Consumes if gEfiGraphicsOutputProtocolGuid 
uninstalled
   gEfiComponentNameProtocolGuid  | NOT 
gEfiMdePkgTokenSpaceGuid.PcdComponentNameDisable   ## SOMETIMES_PRODUCES # User 
chooses to produce it
   gEfiComponentName2ProtocolGuid | NOT 
gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable  ## SOMETIMES_PRODUCES # User 
chooses to produce it
diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
index 7c6fde620c74..f950f1c9c648 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -33,6 +33,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include 
 
@@ -1520,4 +1522,90 @@ EfiLocateProtocolBuffer (
   OUT UINTN *NoProtocols,
   OUT VOID  ***Buffer
   );
+
+/**
+  Open or create a file or directory, possibly creating the chain of
+  directories leading up to the directory.
+
+  EfiOpenFileByDevicePath() first locates EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on
+  FilePath, and opens the root directory of that filesystem with
+  EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.OpenVolume().
+
+  On the remaining device path, the longest initial sequence of
+  FILEPATH_DEVICE_PATH nodes is node-wise traversed with
+  EFI_FILE_PROTOCOL.Open(). For the pathname fragment specified by each
+  traversed FILEPATH_DEVICE_PATH node, EfiOpenFileByDevicePath() first masks
+  EFI_FILE_MODE_CREATE out of OpenMode, and passes 0 for Attributes. If
+  EFI_FILE_PROTOCOL.Open() fails, and OpenMode includes EFI_FILE_MODE_CREATE,
+  then the operation is retried with the caller's OpenMode and Attributes
+  unmodified.
+
+  (As a consequence, if OpenMode includes EFI_FILE_MODE_CREATE, and Attributes
+  includes EFI_FILE_DIRECTORY, and each FILEPATH_DEVICE_PATH specifies a single
+  pathname component, then EfiOpenFileByDevicePath() ensures that the specified
+  series of subdirectories exist on return.)
+
+  The EFI_FILE_PROTOCOL identified by the last FILEPATH_DEVICE_PATH node is
+  output to the caller; intermediate EFI_FILE_PROTOCOL instances are closed. If
+  there are no FILEPATH_DEVICE_PATH nodes past the node that identifies the
+  filesystem, then the EFI_FILE_PROTOCOL of the root directory of the
+  filesystem is output to the caller. If a device path node that is different
+  from 

[edk2] [PATCH v2 6/7] ShellPkg/UefiShellLib: drop DeviceHandle param of ShellOpenFileByDevicePath()

[Laszlo Ersek]

The ShellOpenFileByDevicePath() API promises to set the DeviceHandle
output parameter to the handle of the filesystem identified by the
FilePath input parameter. However, this doesn't actually happen when the
UEFI Shell 2.0 method is used (which is basically "always" nowadays).

Accordingly, the only caller of ShellOpenFileByDevicePath(), namely
ShellOpenFileByName(), defines a (dummy) local DeviceHandle variable just
so it can call ShellOpenFileByDevicePath().

Remove the useless output parameter.

Cc: Jaben Carsey 
Cc: Ruiyu Ni 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
Reviewed-by: Jaben Carsey 
---

Notes:
v2:
- pick up Jaben's R-b

 ShellPkg/Library/UefiShellLib/UefiShellLib.inf |  2 +-
 ShellPkg/Include/Library/ShellLib.h|  2 --
 ShellPkg/Library/UefiShellLib/UefiShellLib.c   | 11 ---
 3 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.inf 
b/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
index 0df632378fe6..38d9a4b81f5f 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
@@ -19,7 +19,7 @@ [Defines]
   BASE_NAME  = UefiShellLib
   FILE_GUID  = 449D0F00-2148-4a43-9836-F10B3980ECF5
   MODULE_TYPE= UEFI_DRIVER
-  VERSION_STRING = 1.1
+  VERSION_STRING = 1.2
   LIBRARY_CLASS  = ShellLib|UEFI_APPLICATION UEFI_DRIVER 
DXE_RUNTIME_DRIVER DXE_DRIVER
   CONSTRUCTOR= ShellLibConstructor
   DESTRUCTOR = ShellLibDestructor
diff --git a/ShellPkg/Include/Library/ShellLib.h 
b/ShellPkg/Include/Library/ShellLib.h
index e360a67ac751..92fddc50f5dd 100644
--- a/ShellPkg/Include/Library/ShellLib.h
+++ b/ShellPkg/Include/Library/ShellLib.h
@@ -89,7 +89,6 @@ ShellSetFileInfo (
 
   @param[in, out]  FilePath  On input, the device path to the file.  On 
output,
  the remaining device path.
-  @param[out]   DeviceHandle Pointer to the system device handle.
   @param[out]   FileHandle   Pointer to the file handle.
   @param[in]OpenMode The mode to open the file with.
   @param[in]Attributes   The file's file attributes.
@@ -115,7 +114,6 @@ EFI_STATUS
 EFIAPI
 ShellOpenFileByDevicePath(
   IN OUT EFI_DEVICE_PATH_PROTOCOL **FilePath,
-  OUT EFI_HANDLE  *DeviceHandle,
   OUT SHELL_FILE_HANDLE   *FileHandle,
   IN UINT64   OpenMode,
   IN UINT64   Attributes
diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c 
b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
index 3c24ba1742bf..18c3be4a8bc7 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
@@ -472,7 +472,6 @@ ShellSetFileInfo (
 
   @param  FilePathon input the device path to the file.  On output
   the remaining device path.
-  @param  DeviceHandlepointer to the system device handle.
   @param  FileHandle  pointer to the file handle.
   @param  OpenModethe mode to open the file with.
   @param  Attributes  the file's file attributes.
@@ -498,7 +497,6 @@ EFI_STATUS
 EFIAPI
 ShellOpenFileByDevicePath(
   IN OUT EFI_DEVICE_PATH_PROTOCOL **FilePath,
-  OUT EFI_HANDLE  *DeviceHandle,
   OUT SHELL_FILE_HANDLE   *FileHandle,
   IN UINT64   OpenMode,
   IN UINT64   Attributes
@@ -511,8 +509,9 @@ ShellOpenFileByDevicePath(
   EFI_FILE_PROTOCOL   *Handle2;
   CHAR16  *FnafPathName;
   UINTN   PathLen;
+  EFI_HANDLE  DeviceHandle;
 
-  if (FilePath == NULL || FileHandle == NULL || DeviceHandle == NULL) {
+  if (FilePath == NULL || FileHandle == NULL) {
 return (EFI_INVALID_PARAMETER);
   }
 
@@ -538,11 +537,11 @@ ShellOpenFileByDevicePath(
   //
   Status = gBS->LocateDevicePath (,
   FilePath,
-  DeviceHandle);
+  );
   if (EFI_ERROR (Status)) {
 return Status;
   }
-  Status = gBS->OpenProtocol(*DeviceHandle,
+  Status = gBS->OpenProtocol(DeviceHandle,
  ,
  (VOID**),
  gImageHandle,
@@ -690,7 +689,6 @@ ShellOpenFileByName(
   IN UINT64 Attributes
   )
 {
-  EFI_HANDLEDeviceHandle;
   EFI_DEVICE_PATH_PROTOCOL  *FilePath;
   EFI_STATUSStatus;
   EFI_FILE_INFO *FileInfo;
@@ -774,7 +772,6 @@ ShellOpenFileByName(
   FilePath = mEfiShellEnvironment2->NameToPath ((CHAR16*)FileName);
   if (FilePath != NULL) {
 return 

[edk2] [PATCH v2 3/7] MdeModulePkg/RamDiskDxe: replace OpenFileByDevicePath() with UefiLib API

[Laszlo Ersek]

Replace the OpenFileByDevicePath() function with EfiOpenFileByDevicePath()
from UefiLib, correcting the following issues:

- imprecise comments on OpenFileByDevicePath(),
- code duplication between this module and other modules,
- local variable name "EfiSimpleFileSystemProtocol" starting with "Efi"
  prefix,
- bogus "FileHandle = NULL" assignments,
- passing a potentially unaligned "FILEPATH_DEVICE_PATH.PathName" field to
  a protocol member function (forbidden by the UEFI spec),
- leaking "Handle1" when the device path type/subtype check fails in the
  loop,
- stale SHELL_FILE_HANDLE reference in a comment.

Cc: Eric Dong 
Cc: Jiaxin Wu 
Cc: Ruiyu Ni 
Cc: Siyuan Fu 
Cc: Star Zeng 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
Reviewed-by: Star Zeng 
Reviewed-by: Jaben Carsey 
---

Notes:
v2:
- pick up Star's and Jaben's R-b's

 MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf|   1 -
 MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h |  39 --
 MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskFileExplorer.c | 140 

 MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c |   2 +-
 4 files changed, 1 insertion(+), 181 deletions(-)

diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf 
b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
index cdd2da690411..da00e4a420e7 100644
--- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
+++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
@@ -75,7 +75,6 @@ [Protocols]
   gEfiDevicePathProtocolGuid ## PRODUCES
   gEfiBlockIoProtocolGuid## PRODUCES
   gEfiBlockIo2ProtocolGuid   ## PRODUCES
-  gEfiSimpleFileSystemProtocolGuid   ## SOMETIMES_CONSUMES
   gEfiAcpiTableProtocolGuid  ## SOMETIMES_CONSUMES
   gEfiAcpiSdtProtocolGuid## SOMETIMES_CONSUMES
 
diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h 
b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h
index 077bb77b25bf..08a8ca94c9db 100644
--- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h
+++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h
@@ -605,45 +605,6 @@ FileInfo (
   );
 
 
-/**
-  This function will open a file or directory referenced by DevicePath.
-
-  This function opens a file with the open mode according to the file path. The
-  Attributes is valid only for EFI_FILE_MODE_CREATE.
-
-  @param[in, out] FilePath   On input, the device path to the file.
- On output, the remaining device path.
-  @param[out] FileHandle Pointer to the file handle.
-  @param[in]  OpenMode   The mode to open the file with.
-  @param[in]  Attributes The file's file attributes.
-
-  @retval EFI_SUCCESS The information was set.
-  @retval EFI_INVALID_PARAMETER   One of the parameters has an invalid value.
-  @retval EFI_UNSUPPORTED Could not open the file path.
-  @retval EFI_NOT_FOUND   The specified file could not be found on the
-  device or the file system could not be found
-  on the device.
-  @retval EFI_NO_MEDIAThe device has no medium.
-  @retval EFI_MEDIA_CHANGED   The device has a different medium in it or
-  the medium is no longer supported.
-  @retval EFI_DEVICE_ERRORThe device reported an error.
-  @retval EFI_VOLUME_CORRUPTEDThe file system structures are corrupted.
-  @retval EFI_WRITE_PROTECTED The file or medium is write protected.
-  @retval EFI_ACCESS_DENIED   The file was opened read only.
-  @retval EFI_OUT_OF_RESOURCESNot enough resources were available to open
-  the file.
-  @retval EFI_VOLUME_FULL The volume is full.
-**/
-EFI_STATUS
-EFIAPI
-OpenFileByDevicePath(
-  IN OUT EFI_DEVICE_PATH_PROTOCOL   **FilePath,
-  OUT EFI_FILE_HANDLE   *FileHandle,
-  IN UINT64 OpenMode,
-  IN UINT64 Attributes
-  );
-
-
 /**
   Publish the RAM disk NVDIMM Firmware Interface Table (NFIT) to the ACPI
   table.
diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskFileExplorer.c 
b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskFileExplorer.c
index 2cfd4bbf6ce8..95d676fc3939 100644
--- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskFileExplorer.c
+++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskFileExplorer.c
@@ -111,143 +111,3 @@ FileInfo (
 
   return Buffer;
 }
-
-
-/**
-  This function will open a file or directory referenced by DevicePath.
-
-  This function opens a file with the open mode according to the file path. The
-  Attributes is valid only for EFI_FILE_MODE_CREATE.
-
-  @param[in, out] FilePath   On input, the device path to the file.
- 

Re: [edk2] [PATCH edk2-platforms v1 14/38] Silicon/Hisilicon/D06: Fix I2C enable fail issue for D06

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:08:58PM +0800, Ming Huang wrote:
> From: shaochangliang 
> 
> I2C may enable failed in D06, so retry I2C enable while
> enable failed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: shaochangliang 
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 
> ---
>  Silicon/Hisilicon/Library/I2CLib/I2CLib.c | 22 
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/Silicon/Hisilicon/Library/I2CLib/I2CLib.c 
> b/Silicon/Hisilicon/Library/I2CLib/I2CLib.c
> index b5b388d756..ecd2f07c4d 100644
> --- a/Silicon/Hisilicon/Library/I2CLib/I2CLib.c
> +++ b/Silicon/Hisilicon/Library/I2CLib/I2CLib.c
> @@ -83,6 +83,7 @@ I2C_Enable(UINT32 Socket,UINT8 Port)
>  {
>  I2C0_ENABLE_U   I2cEnableReg;

Ugh, indentation is incorrect in the original.
Can you correct that in a patch preceding this one?

>  I2C0_ENABLE_STATUS_UI2cEnableStatusReg;
> +UINT32  ulTimeCnt = I2C_READ_TIMEOUT;

What is ul?

>  
>  UINTN Base = GetI2cBase(Socket, Port);
>  
> @@ -91,16 +92,19 @@ I2C_Enable(UINT32 Socket,UINT8 Port)
>  I2cEnableReg.bits.enable = 1;
>  I2C_REG_WRITE(Base + I2C_ENABLE_OFFSET, I2cEnableReg.Val32);
>  
> -
> -I2C_REG_READ(Base + I2C_ENABLE_STATUS_OFFSET, I2cEnableStatusReg.Val32);
> -if (1 == I2cEnableStatusReg.bits.ic_en)
> +do
>  {

Move that brace up to the previous line.

> -return EFI_SUCCESS;
> -}
> -else
> -{
> -return EFI_DEVICE_ERROR;
> -}
> +I2C_Delay(1);

Why 1?
Do we need a MemoryFence ()?

> +
> +ulTimeCnt--;
> +I2C_REG_READ(Base + I2C_ENABLE_STATUS_OFFSET, 
> I2cEnableStatusReg.Val32);
> +if (0 == ulTimeCnt)
> +{

Move that brace up to previous line.

> +return EFI_DEVICE_ERROR;
> +}
> +}while (0 == I2cEnableStatusReg.bits.ic_en);

Space after }

/
Leif

> +
> +return EFI_SUCCESS;
>  }
>  
>  void I2C_SetTarget(UINT32 Socket,UINT8 Port,UINT32 I2cDeviceAddr)
> -- 
> 2.17.0
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v1 13/38] Silicon/Hisilicon/Acpi: Move some macro to PlatformArch.h

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:08:57PM +0800, Ming Huang wrote:
> From: Sun Yuanchen 
> 
> ARM_ACPI_HEADER is used by a unify module in other Pkg,

What other Pkg?

> so move some macro to PlatformArch.h for unify D0x.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sun Yuanchen 
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 

On the whole, I think this should be implemented for 1610/1616
separately, and then introduced in the first version of the 1620
header.

> ---
>  Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/Hi1610Platform.h | 27 
> +++-
>  Silicon/Hisilicon/Hi1610/Include/PlatformArch.h| 24 
> +
>  Silicon/Hisilicon/Hi1616/D05AcpiTables/Hi1616Platform.h| 24 
> +
>  Silicon/Hisilicon/Hi1616/Include/PlatformArch.h| 23 
> +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Platform.h | 25 
> ++
>  Silicon/Hisilicon/Hi1620/Include/PlatformArch.h| 23 
> +
>  6 files changed, 76 insertions(+), 70 deletions(-)
> 
> diff --git a/Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/Hi1610Platform.h 
> b/Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/Hi1610Platform.h
> index 5a95b02055..28546bea99 100644
> --- a/Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/Hi1610Platform.h
> +++ b/Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/Hi1610Platform.h
> @@ -1,8 +1,8 @@
>  /** @file
>  *
>  *  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
> -*  Copyright (c) 2015, Hisilicon Limited. All rights reserved.
> -*  Copyright (c) 2015, Linaro Limited. All rights reserved.
> +*  Copyright (c) 2015-2018, Hisilicon Limited. All rights reserved.
> +*  Copyright (c) 2015-2018, Linaro Limited. All rights reserved.
>  *
>  *  This program and the accompanying materials
>  *  are licensed and made available under the terms and conditions of the BSD 
> License
> @@ -20,28 +20,7 @@
>  #ifndef _HI1610_PLATFORM_H_
>  #define _HI1610_PLATFORM_H_
>  
> -//
> -// ACPI table information used to initialize tables.
> -//
> -#define EFI_ACPI_ARM_OEM_ID   'H','I','S','I',' ',' '   // OEMID 6 
> bytes long
> -#define EFI_ACPI_ARM_OEM_TABLE_ID SIGNATURE_64('H','I','P','0','6',' ',' 
> ',' ') // OEM table id 8 bytes long
> -#define EFI_ACPI_ARM_OEM_REVISION 0x
> -#define EFI_ACPI_ARM_CREATOR_ID   SIGNATURE_32('I','N','T','L')
> -#define EFI_ACPI_ARM_CREATOR_REVISION 0x20151124
> -
> -// A macro to initialise the common header part of EFI ACPI tables as 
> defined by
> -// EFI_ACPI_DESCRIPTION_HEADER structure.
> -#define ARM_ACPI_HEADER(Signature, Type, Revision) {  \
> -Signature,  /* UINT32  Signature */   \
> -sizeof (Type),  /* UINT32  Length */  \
> -Revision,   /* UINT8   Revision */\
> -0,  /* UINT8   Checksum */\
> -{ EFI_ACPI_ARM_OEM_ID },/* UINT8   OemId[6] */\
> -EFI_ACPI_ARM_OEM_TABLE_ID,  /* UINT64  OemTableId */  \
> -EFI_ACPI_ARM_OEM_REVISION,  /* UINT32  OemRevision */ \
> -EFI_ACPI_ARM_CREATOR_ID,/* UINT32  CreatorId */   \
> -EFI_ACPI_ARM_CREATOR_REVISION   /* UINT32  CreatorRevision */ \
> -  }
> +#include <../Include/PlatformArch.h>

No relative #includes.

>  
>  #define HI1610_WATCHDOG_COUNT  2
>  
> diff --git a/Silicon/Hisilicon/Hi1610/Include/PlatformArch.h 
> b/Silicon/Hisilicon/Hi1610/Include/PlatformArch.h
> index f2e931f30b..03e96cfd31 100644
> --- a/Silicon/Hisilicon/Hi1610/Include/PlatformArch.h
> +++ b/Silicon/Hisilicon/Hi1610/Include/PlatformArch.h
> @@ -37,5 +37,29 @@
>  
>  #define S1_BASE   0x400
>  
> +
> +//
> +// ACPI table information used to initialize tables.
> +//
> +#define EFI_ACPI_ARM_OEM_ID   'H','I','S','I',' ',' '   // OEMID 6 
> bytes long
> +#define EFI_ACPI_ARM_OEM_TABLE_ID SIGNATURE_64 ('H','I','P','0','6',' 
> ',' ',' ') // OEM table id 8 bytes long
> +#define EFI_ACPI_ARM_OEM_REVISION 0x
> +#define EFI_ACPI_ARM_CREATOR_ID   SIGNATURE_32 ('I','N','T','L')

I realise this is just moving, but ... why are we claiming that Intel
is the creator of these tables?

> +#define EFI_ACPI_ARM_CREATOR_REVISION 0x20151124
> +
> +// A macro to initialise the common header part of EFI ACPI tables as 
> defined by
> +// EFI_ACPI_DESCRIPTION_HEADER structure.
> +#define ARM_ACPI_HEADER(Signature, Type, Revision) {\
> +  Signature,  /* UINT32  Signature */   \
> +  sizeof (Type),  /* UINT32  Length */  \
> +  Revision,   /* UINT8   Revision */\
> +  0,  /* UINT8   Checksum */\
> +  { EFI_ACPI_ARM_OEM_ID },/* UINT8   OemId[6] */\
> +  EFI_ACPI_ARM_OEM_TABLE_ID,  /* UINT64  OemTableId */  \
> +  EFI_ACPI_ARM_OEM_REVISION,  /* UINT32  

Re: [edk2] [PATCH edk2-platforms v1 12/38] Silicon/Hisilicon/D06: Stop watchdog

[Ard Biesheuvel]

On 24 July 2018 at 09:08, Ming Huang  wrote:
> From: Sun Yuanchen 
>
> according as watchdog design on D06, watchdog should be
> stoped befor boot a option.
>

Why? The DXE core already handles the watchdog, why do you need
something special here?

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wang Yue 
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 
> ---
>  Silicon/Hisilicon/Include/Library/IpmiCmdLib.h  
> | 16 ++
>  Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c   
> | 22 
>  Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
> |  2 ++
>  3 files changed, 40 insertions(+)
>
> diff --git a/Silicon/Hisilicon/Include/Library/IpmiCmdLib.h 
> b/Silicon/Hisilicon/Include/Library/IpmiCmdLib.h
> index 8868b76135..b956ee6d07 100644
> --- a/Silicon/Hisilicon/Include/Library/IpmiCmdLib.h
> +++ b/Silicon/Hisilicon/Include/Library/IpmiCmdLib.h
> @@ -19,6 +19,17 @@
>  #define BOOT_OPTION_BOOT_FLAG_VALID 1
>  #define BOOT_OPTION_BOOT_FLAG_INVALID   0
>
> +typedef enum {
> +  EfiReserved,
> +  EfiBiosFrb2,
> +  EfiBiosPost,
> +  EfiOsLoad,
> +  EfiSmsOs,
> +  EfiOem,
> +  EfiFrbReserved1,
> +  EfiFrbReserved2
> +} EFI_WDT_USER_TYPE;
> +
>  typedef enum {
>NoOverride = 0x0,
>ForcePxe,
> @@ -91,4 +102,9 @@ IpmiCmdGetSysBootOptions (
>IN IPMI_GET_BOOT_OPTION   *BootOption
>);
>
> +EFI_STATUS
> +IpmiCmdStopWatchdogTimer (
> +  IN EFI_WDT_USER_TYPE  UserType
> +  );
> +
>  #endif
> diff --git a/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c 
> b/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c
> index f7536bfea3..9636f29dce 100644
> --- a/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -19,8 +19,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -615,6 +617,8 @@ PlatformBootManagerAfterConsole (
>  {
>EFI_STATUS Status;
>ESRT_MANAGEMENT_PROTOCOL   *EsrtManagement = NULL;
> +  OEM_CONFIG_DATASetupData;
> +  UINTN  DataSize = sizeof (OEM_CONFIG_DATA);
>
>//
>// Show the splash screen.
> @@ -651,6 +655,24 @@ PlatformBootManagerAfterConsole (
>  );
>
>HandleBmcBootType ();
> +
> +  //Disable POST Watch Dog before enter setup
> +  Status = gRT->GetVariable (
> +  OEM_CONFIG_NAME,
> +  ,
> +  NULL,
> +  ,
> +  
> +  );
> +
> +  if (!EFI_ERROR (Status)) {
> +if (SetupData.BmcWdtEnable) {
> +  Status = IpmiCmdStopWatchdogTimer (EfiBiosPost);
> +  if (EFI_ERROR (Status)) {
> +DEBUG ((DEBUG_ERROR, "%a:%r\n", __FUNCTION__, Status));
> +  }
> +}
> +  }
>  }
>
>  /**
> diff --git 
> a/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
> b/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index a093f13fb0..21afb53fc5 100644
> --- 
> a/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ 
> b/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -47,6 +47,7 @@
>DevicePathLib
>DxeServicesLib
>MemoryAllocationLib
> +  IpmiCmdLib
>PcdLib
>PrintLib
>TimerLib
> @@ -69,6 +70,7 @@
>gEfiEndOfDxeEventGroupGuid
>gEfiTtyTermGuid
>gHisiOemVariableGuid
> +  gOemConfigGuid
>
>  [Protocols]
>gEfiGenericMemTestProtocolGuid
> --
> 2.17.0
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v1 12/38] Silicon/Hisilicon/D06: Stop watchdog

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:08:56PM +0800, Ming Huang wrote:
> From: Sun Yuanchen 
> 
> according as watchdog design on D06, watchdog should be
> stoped befor boot a option.

stopped before

> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wang Yue 
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 
> ---
>  Silicon/Hisilicon/Include/Library/IpmiCmdLib.h  
> | 16 ++
>  Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c   
> | 22 
>  Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
> |  2 ++
>  3 files changed, 40 insertions(+)
> 
> diff --git a/Silicon/Hisilicon/Include/Library/IpmiCmdLib.h 
> b/Silicon/Hisilicon/Include/Library/IpmiCmdLib.h
> index 8868b76135..b956ee6d07 100644
> --- a/Silicon/Hisilicon/Include/Library/IpmiCmdLib.h
> +++ b/Silicon/Hisilicon/Include/Library/IpmiCmdLib.h
> @@ -19,6 +19,17 @@
>  #define BOOT_OPTION_BOOT_FLAG_VALID 1
>  #define BOOT_OPTION_BOOT_FLAG_INVALID   0
>  
> +typedef enum {
> +  EfiReserved,
> +  EfiBiosFrb2,
> +  EfiBiosPost,
> +  EfiOsLoad,
> +  EfiSmsOs,
> +  EfiOem,
> +  EfiFrbReserved1,
> +  EfiFrbReserved2
> +} EFI_WDT_USER_TYPE;
> +
>  typedef enum {
>NoOverride = 0x0,
>ForcePxe,
> @@ -91,4 +102,9 @@ IpmiCmdGetSysBootOptions (
>IN IPMI_GET_BOOT_OPTION   *BootOption
>);
>  
> +EFI_STATUS
> +IpmiCmdStopWatchdogTimer (
> +  IN EFI_WDT_USER_TYPE  UserType
> +  );
> +
>  #endif
> diff --git a/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c 
> b/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c
> index f7536bfea3..9636f29dce 100644
> --- a/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -19,8 +19,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
> +#include 

Please put this line after Library/DevicePathLib.h.

>  #include 
>  #include 
>  #include 
> @@ -615,6 +617,8 @@ PlatformBootManagerAfterConsole (
>  {
>EFI_STATUS Status;
>ESRT_MANAGEMENT_PROTOCOL   *EsrtManagement = NULL;
> +  OEM_CONFIG_DATASetupData;
> +  UINTN  DataSize = sizeof (OEM_CONFIG_DATA);
>  
>//
>// Show the splash screen.
> @@ -651,6 +655,24 @@ PlatformBootManagerAfterConsole (
>  );
>  
>HandleBmcBootType ();
> +
> +  //Disable POST Watch Dog before enter setup

Before entering setup (as stated here) or before attempting boot (as
stated in commit message)?

> +  Status = gRT->GetVariable (
> +  OEM_CONFIG_NAME,
> +  ,
> +  NULL,
> +  ,
> +  
> +  );
> +
> +  if (!EFI_ERROR (Status)) {
> +if (SetupData.BmcWdtEnable) {
> +  Status = IpmiCmdStopWatchdogTimer (EfiBiosPost);
> +  if (EFI_ERROR (Status)) {
> +DEBUG ((DEBUG_ERROR, "%a:%r\n", __FUNCTION__, Status));
> +  }
> +}
> +  }
>  }
>  
>  /**
> diff --git 
> a/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
> b/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index a093f13fb0..21afb53fc5 100644
> --- 
> a/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ 
> b/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -47,6 +47,7 @@
>DevicePathLib
>DxeServicesLib
>MemoryAllocationLib
> +  IpmiCmdLib

Please move up one line.

/
Leif

>PcdLib
>PrintLib
>TimerLib
> @@ -69,6 +70,7 @@
>gEfiEndOfDxeEventGroupGuid
>gEfiTtyTermGuid
>gHisiOemVariableGuid
> +  gOemConfigGuid
>  
>  [Protocols]
>gEfiGenericMemTestProtocolGuid
> -- 
> 2.17.0
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v1 11/38] Hisilicon/D06: Add Hi1620OemConfigUiLib

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:08:55PM +0800, Ming Huang wrote:
> From: Yang XinYi 
> 
> This library is added for oem setup menu item.

Please add some detail about what settings are made available here.

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Yang XinYi 
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 
> ---
>  Platform/Hisilicon/D06/D06.dsc  |   
> 5 +-
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/MemoryConfig.hfr  | 
> 154 
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/MemoryConfig.uni  | 
> 172 +
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/MiscConfig.hfr|  
> 41 +++
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/MiscConfig.uni|  
> 36 ++
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.c   | 
> 380 
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfig.h   | 
> 141 
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigUi.h |  
> 64 
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigUiLib.inf|  
> 67 
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigUiLib.uni|  
> 24 ++
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigUiLibStrings.uni |  
> 64 
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigVfr.Vfr  |  
> 89 +
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfig.hfr| 
> 219 +++
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PcieConfigStrings.uni | 
> 185 ++
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/PciePortConfig.hfr| 
> 167 +
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/RasConfig.hfr | 
> 171 +
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/RasConfig.uni | 
> 135 +++
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/iBMCConfig.hfr|  
> 80 +
>  Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/iBMCConfig.uni|  
> 49 +++
>  Silicon/Hisilicon/HisiPkg.dec   |   
> 1 +
>  Silicon/Hisilicon/Include/Library/OemConfigData.h   |  
> 84 +
>  21 files changed, 2327 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
> index 392225250f..6f3786f0eb 100644
> --- a/Platform/Hisilicon/D06/D06.dsc
> +++ b/Platform/Hisilicon/D06/D06.dsc
> @@ -334,7 +334,10 @@
>#ACPI
>#
>MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
> -  Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/AcpiPlatformDxe.inf
> +  Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/AcpiPlatformDxe.inf {
> +
> +NULL|Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/OemConfigUiLib.inf
> +  }
>  
>Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/AcpiTablesHi1620.inf
>Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf
> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/MemoryConfig.hfr 
> b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/MemoryConfig.hfr
> new file mode 100644
> index 00..c709975c38
> --- /dev/null
> +++ b/Silicon/Hisilicon/Hi1620/Hi1620OemConfigUiLib/MemoryConfig.hfr
> @@ -0,0 +1,154 @@
> +/** @file
> +*
> +*  Memory Config form at Oem Config fromset.
> +*
> +*  Copyright (c) 2017 - 2018, Hisilicon Limited. All rights reserved.
> +*  Copyright (c) 2017 - 2018, Linaro Limited. All rights reserved.
> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD 
> License
> +*  which accompanies this distribution.  The full text of the license may be 
> found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +*
> +**/
> +
> +form formid = MEMORY_CONFIG_FORM_ID,
> +  title  = STRING_TOKEN(STR_MEMORY_CONFIG_FORM_TITLE);
> +
> +  oneof varid  = OEM_CONFIG_DATA.DdrDebugLevel,
> +prompt   = STRING_TOKEN (STR_MEM_PRINT_LEVEL_PROMPT),
> +help = STRING_TOKEN (STR_MEM_PRINT_LEVEL_HELP),
> +option text = STRING_TOKEN (STR_MEM_PRINT_LEVEL_DISABLE),  value = 
> 0, flags = RESET_REQUIRED;
> +option text = STRING_TOKEN (STR_MEM_PRINT_LEVEL_MINIMUM), value = 1, 
> flags = RESET_REQUIRED | MANUFACTURING | DEFAULT;
> +option text = STRING_TOKEN (STR_MEM_PRINT_LEVEL_MINMAX),  value = 2, 
> flags = RESET_REQUIRED;
> +option text = STRING_TOKEN (STR_MEM_PRINT_LEVEL_MAXIMUM),  value = 
> 3, flags = RESET_REQUIRED;
> +  endoneof;
> +
> +  oneof varid = OEM_CONFIG_DATA.DdrFreqLimit,
> +prompt  = STRING_TOKEN(STR_XMP_DDR_FREQ_LIMIT_PROMPT),
> +help= STRING_TOKEN(STR_XMP_DDR_FREQ_LIMIT_HELP),
> +option text = STRING_TOKEN(STR_AUTO), value = 0, flags = 

Re: [edk2] [PATCH v1 1/1] PatchCheck - add error message for invalid parameter

[Gao, Liming]

Jaben:
  Could you give me one failure case? Then, I can further understand the patch. 

Thanks
Liming
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Jaben Carsey
>Sent: Friday, August 03, 2018 7:21 AM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming 
>Subject: [edk2] [PATCH v1 1/1] PatchCheck - add error message for invalid
>parameter
>
>Currently if an invalid parameter is passed, it gives a stack trace.
>This changes it to an error message.
>
>Cc: Liming Gao 
>Cc: Yonghong Zhu 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Jaben Carsey 
>---
> BaseTools/Scripts/PatchCheck.py | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
>diff --git a/BaseTools/Scripts/PatchCheck.py
>b/BaseTools/Scripts/PatchCheck.py
>index 7b7fba8b7044..96b3cdf1fd8a 100755
>--- a/BaseTools/Scripts/PatchCheck.py
>+++ b/BaseTools/Scripts/PatchCheck.py
>@@ -1,7 +1,7 @@
> ## @file
> #  Check a patch for various format issues
> #
>-#  Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
>+#  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
> #
> #  This program and the accompanying materials are licensed and made
> #  available under the terms and conditions of the BSD License which
>@@ -528,6 +528,8 @@ class CheckGitCommits:
> print('Checking git commit:', commit)
> patch = self.read_patch_from_git(commit)
> self.ok &= CheckOnePatch(commit, patch).ok
>+if not commits:
>+print("Couldn't find commit matching: '{}'".format(rev_spec))
>
> def read_commit_list_from_git(self, rev_spec, max_count):
> # Run git to get the commit patch
>@@ -536,7 +538,7 @@ class CheckGitCommits:
> cmd.append('--max-count=' + str(max_count))
> cmd.append(rev_spec)
> out = self.run_git(*cmd)
>-return out.split()
>+return out.split() if out else []
>
> def read_patch_from_git(self, commit):
> # Run git to get the commit patch
>@@ -548,7 +550,8 @@ class CheckGitCommits:
> p = subprocess.Popen(cmd,
>  stdout=subprocess.PIPE,
>  stderr=subprocess.STDOUT)
>-return p.communicate()[0].decode('utf-8', 'ignore')
>+Result = p.communicate()
>+return Result[0].decode('utf-8', 'ignore') if Result[0] and
>Result[0].find("fatal")!=0 else None
>
> class CheckOnePatchFile:
> """Performs a patch check for a single file.
>--
>2.16.2.windows.1
>
>___
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [platforms: PATCH v2 0/6] Armada7k8k ComPhy rework

[Marcin Wojtas]

Thanks a lot!
Marcin
czw., 2 sie 2018 o 21:02 Ard Biesheuvel  napisał(a):
>
> On 26 July 2018 at 09:21, Marcin Wojtas  wrote:
> > Hi,
> >
> > The second version of the patchset brings minor corrections,
> > that were pointed out during review. Details can be found
> > int the commit logs.
> >
> > The patches are available in the github:
> > https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/comphy-upstream-r20180726
> >
> > I'm looking forward to review and any comments/remarks.
> >
> > Best regards,
> > Marcin
> >
> > Changelog:
> > v1 -> v2
> >  * 1/6
> >- improve commit message
> >- create dedicated header for externally defined SiP services
> >  ComPhy parameters
> >- add comments about external definitions
> >
> >  * 2,3,4/6
> >- s/firmware/ARM-TF/ in the commit messages
> >
> >  * 5,6/6
> >- swap order without any changes in the patches - in the first
> >  series compilation was broken on 5th and immediately fixed in
> >  the last patch. Now every patch compiles.
> >
> > Grzegorz Jaszczyk (5):
> >   Marvell/Library: ComPhyLib: Configure SATA, SGMII and SFI in ARM-TF
> >   Marvell/Library: ComPhyLib: Configure PCIE in ARM-TF
> >   Marvell/Library: ComPhyLib: Configure RXAUI in ARM-TF
> >   Marvell/Library: ComPhyLib: Configure USB in ARM-TF
> >   Marvell/Library: ComPhyLib: Remove both PHY and PIPE selector config
> >   Marvell/Library: ComPhyLib: Clean up the library after rework
> >
>
> Reviewed-by: Ard Biesheuvel 
>
> Pushed as 016d55843a01..9dae9a0c7996
>
> Thanks!
>
> >  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc  
> > |   23 +-
> >  Silicon/Marvell/Library/ComPhyLib/ComPhyLib.inf
> > |2 +-
> >  
> > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.h
> >  |   22 -
> >  Silicon/Marvell/Include/Library/SampleAtResetLib.h 
> > |7 -
> >  Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h  
> > |  512 +-
> >  Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h   
> > |   86 +
> >  
> > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.c
> >  |   19 -
> >  Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c
> > | 1777 +---
> >  Silicon/Marvell/Library/ComPhyLib/ComPhyLib.c  
> > |   41 +-
> >  Silicon/Marvell/Library/ComPhyLib/ComPhyMux.c  
> > |  132 --
> >  10 files changed, 170 insertions(+), 2451 deletions(-)
>
> (/me does a little dance after reading this line)
>
> >  create mode 100644 Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h
> >  delete mode 100644 Silicon/Marvell/Library/ComPhyLib/ComPhyMux.c
> >
> > --
> > 2.7.4
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 1/1] ArmPkg/GenericWatchdogDxe: Split 64bit register write to 2x32bit

[Marcin Wojtas]

pt., 3 sie 2018 o 09:33 Ard Biesheuvel  napisał(a):
>
> On 2 August 2018 at 22:50, Marcin Wojtas  wrote:
> > According to the SBSA specification the Watchdog Compare
> > Register is split into two separate 32bit registers.
> > EDK2 code uses a single 64bit transaction to update
> > them, which can be problematic, depending on the SoC
> > implementation and could result in an unpredicted behavior.
> >
> > Fix this by modifying WatchdogWriteCompareRegister routine to
> > use two consecutive 32bit writes to the Watchdog Compare Register
> > Low and High, using new dedicated macros.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marcin Wojtas 
> > Reviewed-by: Leif Lindholm 
>
> Pushed as dd4cae4d82c7
>

Thanks a lot!
Marcin

>
> > ---
> > Changelog v1 -> v2:
> > - use separate macros for WCV register low and high
> > - improve commit message
> > - add Leif's RB
> >
> >  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h| 3 ++-
> >  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 3 ++-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h 
> > b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
> > index 9e2aebc..4f42c56 100644
> > --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
> > +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
> > @@ -20,7 +20,8 @@
> >  // Control Frame:
> >  #define GENERIC_WDOG_CONTROL_STATUS_REG   ((UINTN)FixedPcdGet64 
> > (PcdGenericWatchdogControlBase) + 0x000)
> >  #define GENERIC_WDOG_OFFSET_REG   ((UINTN)FixedPcdGet64 
> > (PcdGenericWatchdogControlBase) + 0x008)
> > -#define GENERIC_WDOG_COMPARE_VALUE_REG((UINTN)FixedPcdGet64 
> > (PcdGenericWatchdogControlBase) + 0x010)
> > +#define GENERIC_WDOG_COMPARE_VALUE_REG_LOW((UINTN)FixedPcdGet64 
> > (PcdGenericWatchdogControlBase) + 0x010)
> > +#define GENERIC_WDOG_COMPARE_VALUE_REG_HIGH   ((UINTN)FixedPcdGet64 
> > (PcdGenericWatchdogControlBase) + 0x014)
> >
> >  // Values of bit 0 of the Control/Status Register
> >  #define GENERIC_WDOG_ENABLED  1
> > diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
> > b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> > index 3180f01..8ccf153 100644
> > --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> > +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> > @@ -56,7 +56,8 @@ WatchdogWriteCompareRegister (
> >UINT64  Value
> >)
> >  {
> > -  MmioWrite64 (GENERIC_WDOG_COMPARE_VALUE_REG, Value);
> > +  MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG_LOW, Value & MAX_UINT32);
> > +  MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG_HIGH, (Value >> 32) & 
> > MAX_UINT32);
> >  }
> >
> >  VOID
> > --
> > 2.7.4
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [patch 0/2] Enahnce Perf data transfer between Dxe Perf Lib and driver

[Gao, Liming]

Reviewed-by: Liming Gao 

>-Original Message-
>From: Bi, Dandan
>Sent: Monday, July 30, 2018 3:00 PM
>To: edk2-devel@lists.01.org
>Cc: Dmitry Antipov ; Kinney, Michael D
>; Gao, Liming ; Zeng,
>Star 
>Subject: [patch 0/2] Enahnce Perf data transfer between Dxe Perf Lib and
>driver
>
>Add an internal protocol Firmware Boot Performance Table protocol to
>get the performance data from DxeCorePerformanceLib.
>Thus the FirmwarePerformanceDxe driver can call the API in the
>new protocl to get the performance data instead of through
>hook status code.
>
>Cc: Dmitry Antipov 
>Cc: Michael D Kinney 
>Cc: Liming Gao 
>Cc: Star Zeng 
>Dandan Bi (2):
>  MdeModulePkg: Add definition of Boot Performance Table protocol
>  MdeModulePkg: Implement/use Boot Performance Table protocol
>
> .../Include/Guid/FirmwareBootPerformanceTable.h| 47 ++
> .../DxeCorePerformanceLib/DxeCorePerformanceLib.c  | 74 ++---
>-
> .../DxeCorePerformanceLib.inf  |  6 +-
> .../DxeCorePerformanceLibInternal.h| 15 +
> MdeModulePkg/MdeModulePkg.dec  |  4 ++
> .../FirmwarePerformanceDxe.c   | 71 -
> .../FirmwarePerformanceDxe.inf |  5 +-
> 7 files changed, 144 insertions(+), 78 deletions(-)
> create mode 100644
>MdeModulePkg/Include/Guid/FirmwareBootPerformanceTable.h
>
>--
>2.14.3.windows.1

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

Re: [edk2] [patch 4/4] MdeModulePkg: Remove DxeSmmPerformanceLib

[Gao, Liming]

Reviewed-by: Liming Gao 

>-Original Message-
>From: Bi, Dandan
>Sent: Monday, July 30, 2018 1:45 PM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming ; Zeng, Star 
>Subject: [patch 4/4] MdeModulePkg: Remove DxeSmmPerformanceLib
>
>DxeSmmPerformanceLib previously is used by DP tool.
>But in new pweformance infrastructure, we have updated
>Dp tool to get the performance data from firmware
>performance data table in ACPI.
>Now the DxeSmmPerformanceLib is not used by
>any module. So remove it from edk2 code base to
>avoid being used by mistake.
>
>Cc: Liming Gao 
>Cc: Star Zeng 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Dandan Bi 
>---
> .../DxeSmmPerformanceLib/DxeSmmPerformanceLib.c| 866 -
>
> .../DxeSmmPerformanceLib/DxeSmmPerformanceLib.inf  |  68 --
> .../DxeSmmPerformanceLib/DxeSmmPerformanceLib.uni  |  24 -
> MdeModulePkg/MdeModulePkg.dsc  |   1 -
> 4 files changed, 959 deletions(-)
> delete mode 100644
>MdeModulePkg/Library/DxeSmmPerformanceLib/DxeSmmPerformanceLib.c
> delete mode 100644
>MdeModulePkg/Library/DxeSmmPerformanceLib/DxeSmmPerformanceLib.i
>nf
> delete mode 100644
>MdeModulePkg/Library/DxeSmmPerformanceLib/DxeSmmPerformanceLib.u
>ni
>
>diff --git
>a/MdeModulePkg/Library/DxeSmmPerformanceLib/DxeSmmPerformanceLib
>.c
>b/MdeModulePkg/Library/DxeSmmPerformanceLib/DxeSmmPerformanceLib
>.c
>deleted file mode 100644
>index 353174724e..00
>---
>a/MdeModulePkg/Library/DxeSmmPerformanceLib/DxeSmmPerformanceLib
>.c
>+++ /dev/null
>@@ -1,866 +0,0 @@
>-/** @file
>-  Performance library instance used in DXE phase to dump both PEI/DXE and
>SMM performance data.
>-
>-  This library instance allows a DXE driver or UEFI application to dump both
>PEI/DXE and SMM performance data.
>-  StartPerformanceMeasurement(), EndPerformanceMeasurement(),
>StartPerformanceMeasurementEx()
>-  and EndPerformanceMeasurementEx() are not implemented.
>-
>-  Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.
>-This program and the accompanying materials
>-are licensed and made available under the terms and conditions of the BSD
>License
>-which accompanies this distribution.  The full text of the license may be
>found at
>-http://opensource.org/licenses/bsd-license.php
>-
>-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>-WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>EXPRESS OR IMPLIED.
>-
>-**/
>-
>-
>-#include 
>-
>-#include 
>-
>-#include 
>-#include 
>-#include 
>-#include 
>-#include 
>-#include 
>-#include 
>-#include 
>-
>-#include 
>-
>-#include 
>-#include 
>-
>-#define SMM_PERFORMANCE_COMMUNICATION_BUFFER_SIZE
>(OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data)  + sizeof
>(SMM_PERF_COMMUNICATE))
>-
>-EFI_SMM_COMMUNICATION_PROTOCOL  *mSmmCommunication = NULL;
>-UINT8   *mSmmPerformanceBuffer;
>-GAUGE_DATA_ENTRY*mGaugeData = NULL;
>-UINTN   mGaugeNumberOfEntries = 0;
>-GAUGE_DATA_ENTRY_EX *mGaugeDataEx = NULL;
>-UINTN   mGaugeNumberOfEntriesEx = 0;
>-
>-BOOLEAN mNoSmmPerfHandler = FALSE;
>-BOOLEAN mNoSmmPerfExHandler = FALSE;
>-
>-//
>-// The cached Performance Protocol and PerformanceEx Protocol interface.
>-//
>-PERFORMANCE_PROTOCOL*mPerformance = NULL;
>-PERFORMANCE_EX_PROTOCOL *mPerformanceEx = NULL;
>-
>-/**
>-  The function caches the pointer to SMM Communication protocol.
>-
>-  The function locates SMM Communication protocol from protocol database.
>-
>-  @retval EFI_SUCCESS SMM Communication protocol is successfully
>located.
>-  @retval Other   SMM Communication protocol is not located to log
>performance.
>-
>-**/
>-EFI_STATUS
>-GetCommunicationProtocol (
>-  VOID
>-  )
>-{
>-  EFI_STATUS  Status;
>-  EFI_SMM_COMMUNICATION_PROTOCOL  *Communication;
>-
>-  if (mSmmCommunication != NULL) {
>-return EFI_SUCCESS;
>-  }
>-
>-  Status = gBS->LocateProtocol (, NULL,
>(VOID **) );
>-  if (!EFI_ERROR (Status)) {
>-ASSERT (Communication != NULL);
>-//
>-// Cache SMM Communication protocol.
>-//
>-mSmmCommunication = Communication;
>-  }
>-
>-  return Status;
>-}
>-
>-/**
>-  The function caches the pointers to PerformanceEx protocol and
>Performance Protocol.
>-
>-  The function locates PerformanceEx protocol and Performance Protocol
>from protocol database.
>-
>-  @retval EFI_SUCCESS PerformanceEx protocol or Performance Protocol is
>successfully located.
>-  @retval EFI_NOT_FOUND   Both PerformanceEx protocol and Performance
>Protocol are not located to log performance.
>-
>-**/
>-EFI_STATUS
>-GetPerformanceProtocol (
>-  VOID
>-  )
>-{
>-  EFI_STATUSStatus;
>-  PERFORMANCE_PROTOCOL  *Performance;
>-  PERFORMANCE_EX_PROTOCOL   *PerformanceEx;
>-
>-  if (mPerformanceEx != NULL || mPerformance != NULL) {
>-return EFI_SUCCESS;
>-  }
>-
>- 

[edk2] [PATCH] UefiCpuPkg: Remove redundant library classes, Ppis and GUIDs

[shenglei]

Some redundant library classes Ppis and GUIDs
have been removed in inf, .c and .h files.

https://bugzilla.tianocore.org/show_bug.cgi?id=1043
https://bugzilla.tianocore.org/show_bug.cgi?id=1013
https://bugzilla.tianocore.org/show_bug.cgi?id=1032
https://bugzilla.tianocore.org/show_bug.cgi?id=1016

Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: shenglei 
---
 UefiCpuPkg/CpuDxe/CpuPageTable.c |  6 --
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h   |  1 -
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |  2 --
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h   |  1 -
 UefiCpuPkg/SecCore/FindPeiCore.c |  2 --
 UefiCpuPkg/SecCore/SecCore.inf   | 16 
 UefiCpuPkg/SecCore/SecMain.h |  2 --
 .../Universal/Acpi/S3Resume2Pei/S3Resume.c   |  1 -
 .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf |  3 ---
 9 files changed, 34 deletions(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c
index df021798c0..1453a06338 100644
--- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
@@ -17,12 +17,6 @@
 #include 
 #include 
 #include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
 #include 
 #include 
 #include 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index e3c7cff81c..8c7f4996d1 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -38,7 +38,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index a7fb7b0b14..95a4511225 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -77,7 +77,6 @@
 [LibraryClasses]
   UefiDriverEntryPoint
   UefiRuntimeServicesTableLib
-  CacheMaintenanceLib
   PcdLib
   DebugLib
   BaseLib
@@ -113,7 +112,6 @@
 
 [Guids]
   gEfiAcpiVariableGuid ## SOMETIMES_CONSUMES ## HOB # it 
is used for S3 boot.
-  gEfiGlobalVariableGuid   ## SOMETIMES_PRODUCES ## 
Variable:L"SmmProfileData"
   gEfiAcpi20TableGuid  ## SOMETIMES_CONSUMES ## SystemTable
   gEfiAcpi10TableGuid  ## SOMETIMES_CONSUMES ## SystemTable
   gEdkiiPiSmmMemoryAttributesTableGuid ## CONSUMES ## SystemTable
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
index 1613e9cd5c..2c3cf425bd 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
@@ -15,7 +15,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #ifndef _SMM_PROFILE_INTERNAL_H_
 #define _SMM_PROFILE_INTERNAL_H_
 
-#include 
 #include 
 #include 
 #include 
diff --git a/UefiCpuPkg/SecCore/FindPeiCore.c b/UefiCpuPkg/SecCore/FindPeiCore.c
index 60ccaa9667..bb9c434d1e 100644
--- a/UefiCpuPkg/SecCore/FindPeiCore.c
+++ b/UefiCpuPkg/SecCore/FindPeiCore.c
@@ -13,8 +13,6 @@
 **/
 
 #include 
-#include 
-#include 
 
 #include "SecMain.h"
 
diff --git a/UefiCpuPkg/SecCore/SecCore.inf b/UefiCpuPkg/SecCore/SecCore.inf
index 7bcd4f8a96..4ae9146165 100644
--- a/UefiCpuPkg/SecCore/SecCore.inf
+++ b/UefiCpuPkg/SecCore/SecCore.inf
@@ -50,34 +50,18 @@
 [LibraryClasses]
   BaseMemoryLib
   DebugLib
-  BaseLib
   PlatformSecLib
   PcdLib
-  DebugAgentLib
   UefiCpuLib
   PeCoffGetEntryPointLib
   PeCoffExtraActionLib
-  CpuExceptionHandlerLib
   ReportStatusCodeLib
   PeiServicesLib
   PeiServicesTablePointerLib
   HobLib
 
 [Ppis]
-  ## SOMETIMES_CONSUMES
-  ## PRODUCES
-  gEfiSecPlatformInformationPpiGuid
-  ## SOMETIMES_CONSUMES
-  ## SOMETIMES_PRODUCES
-  gEfiSecPlatformInformation2PpiGuid
   gEfiTemporaryRamDonePpiGuid  ## PRODUCES
-  ## NOTIFY
-  ## SOMETIMES_CONSUMES
-  gPeiSecPerformancePpiGuid
-
-[Guids]
-  ## SOMETIMES_PRODUCES   ## HOB
-  gEfiFirmwarePerformanceGuid
 
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdPeiTemporaryRamStackSize  ## CONSUMES
diff --git a/UefiCpuPkg/SecCore/SecMain.h b/UefiCpuPkg/SecCore/SecMain.h
index 4bc6606044..83244af119 100644
--- a/UefiCpuPkg/SecCore/SecMain.h
+++ b/UefiCpuPkg/SecCore/SecMain.h
@@ -17,14 +17,12 @@
 
 #include 
 
-#include 
 #include 
 #include 
 #include 
 
 #include 
 
-#include 
 #include 
 #include 
 #include 
diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c 
b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
index 28e53ac5d3..2168ece967 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
@@ -34,7 +34,6 @@
 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git 

[edk2] [PATCH] MdeModulePkg: Remove redundant library classes and GUIDs

[shenglei]

Some redundant library classes and GUIDs
have been removed in inf, .c and .h files.
https://bugzilla.tianocore.org/show_bug.cgi?id=1044
https://bugzilla.tianocore.org/show_bug.cgi?id=1045
https://bugzilla.tianocore.org/show_bug.cgi?id=1047
https://bugzilla.tianocore.org/show_bug.cgi?id=1049
https://bugzilla.tianocore.org/show_bug.cgi?id=1051
https://bugzilla.tianocore.org/show_bug.cgi?id=1052
https://bugzilla.tianocore.org/show_bug.cgi?id=1053
https://bugzilla.tianocore.org/show_bug.cgi?id=1054
https://bugzilla.tianocore.org/show_bug.cgi?id=1055
https://bugzilla.tianocore.org/show_bug.cgi?id=1056
https://bugzilla.tianocore.org/show_bug.cgi?id=1017
https://bugzilla.tianocore.org/show_bug.cgi?id=1035
https://bugzilla.tianocore.org/show_bug.cgi?id=1033
https://bugzilla.tianocore.org/show_bug.cgi?id=1012
https://bugzilla.tianocore.org/show_bug.cgi?id=1011

Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: shenglei 
---
 MdeModulePkg/Application/CapsuleApp/CapsuleApp.c   | 1 -
 MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf | 1 -
 .../Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c  | 1 -
 .../SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf| 1 -
 MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf   | 1 -
 MdeModulePkg/Bus/I2c/I2cDxe/I2cHostDxe.inf | 1 -
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h| 1 -
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf   | 1 -
 MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.h | 1 -
 MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.inf   | 1 -
 MdeModulePkg/Core/Dxe/DxeMain.inf  | 1 -
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c  | 1 -
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf  | 1 -
 MdeModulePkg/Core/PiSmmCore/SmiHandlerProfile.c| 1 -
 .../Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf | 3 +--
 .../FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c   | 2 --
 .../FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf | 2 --
 MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf  | 1 -
 MdeModulePkg/Universal/EbcDxe/EbcDebugger.inf  | 2 --
 MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCommon.h  | 2 --
 MdeModulePkg/Universal/EsrtDxe/EsrtDxe.inf | 1 -
 MdeModulePkg/Universal/EsrtDxe/EsrtImpl.h  | 1 -
 MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.h | 1 -
 .../Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf  | 1 -
 .../SmmCommunicationBufferDxe/SmmCommunicationBufferDxe.c  | 1 -
 .../SmmCommunicationBufferDxe/SmmCommunicationBufferDxe.inf| 1 -
 26 files changed, 1 insertion(+), 31 deletions(-)

diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c 
b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
index 894da2f2d9..4d907242f3 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
@@ -23,7 +23,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf 
b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
index 3a67c6b909..d1b11318bb 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
@@ -40,7 +40,6 @@
   MdeModulePkg/MdeModulePkg.dec
 
 [Guids]
-  gEfiGlobalVariableGuid ## CONSUMES   ## GUID
   gEfiCapsuleReportGuid  ## CONSUMES   ## GUID
   gEfiFmpCapsuleGuid ## CONSUMES   ## GUID
   gWindowsUxCapsuleGuid  ## CONSUMES   ## GUID
diff --git 
a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c 
b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
index 96e9977aad..a77164b436 100644
--- a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
+++ b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
@@ -22,7 +22,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git 
a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf 
b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf
index 73cc052cc3..cc189fd480 100644
--- a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf
+++ b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf
@@ -42,7 +42,6 @@
   UefiLib
   PrintLib
   DevicePathLib
-  PeCoffGetEntryPointLib
   DxeServicesLib
 
 [Protocols]
diff --git a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf 
b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf
index 4aab75bab7..d067df0400 100644
--- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf
+++ 

[edk2] [Patch] BaseTools: Add check for VOID* PCD Max Size

[Yonghong Zhu]

Per spec VOID* PCD max size should be a UINT16 value. so this patch
add the value check whether it is in range 0x0 .. 0x.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/Workspace/DscBuildData.py | 21 +
 1 file changed, 21 insertions(+)

diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py 
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index e8b36a3..c7f07f4 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -1534,10 +1534,17 @@ class DscBuildData(PlatformBuildClassObject):
 for PcdCName, TokenSpaceGuid, SkuName, Dummy4 in PcdSet:
 Setting = PcdDict[self._Arch, PcdCName, TokenSpaceGuid, SkuName]
 if Setting is None:
 continue
 PcdValue, DatumType, MaxDatumSize = self._ValidatePcd(PcdCName, 
TokenSpaceGuid, Setting, Type, Dummy4)
+if MaxDatumSize:
+if int(MaxDatumSize, 0) > 0x:
+EdkLogger.error('build', FORMAT_INVALID, "The size value 
must not exceed the maximum value of 0x (UINT16) for %s." % 
".".join((TokenSpaceGuid, PcdCName)),
+File=self.MetaFile, Line=Dummy4)
+if int(MaxDatumSize, 0) < 0:
+EdkLogger.error('build', FORMAT_INVALID, "The size value 
can't be set to negative value for %s." % ".".join((TokenSpaceGuid, PcdCName)),
+File=self.MetaFile, Line=Dummy4)
 if (PcdCName, TokenSpaceGuid) in PcdValueDict:
 PcdValueDict[PcdCName, TokenSpaceGuid][SkuName] = (PcdValue, 
DatumType, MaxDatumSize)
 else:
 PcdValueDict[PcdCName, TokenSpaceGuid] = {SkuName:(PcdValue, 
DatumType, MaxDatumSize)}
 
@@ -2379,10 +2386,17 @@ class DscBuildData(PlatformBuildClassObject):
 Setting = PcdDict[self._Arch, SkuName, PcdCName, TokenSpaceGuid]
 if Setting is None:
 continue
 
 PcdValue, DatumType, MaxDatumSize = self._ValidatePcd(PcdCName, 
TokenSpaceGuid, Setting, Type, Dummy4)
+if MaxDatumSize:
+if int(MaxDatumSize, 0) > 0x:
+EdkLogger.error('build', FORMAT_INVALID, "The size value 
must not exceed the maximum value of 0x (UINT16) for %s." % 
".".join((TokenSpaceGuid, PcdCName)),
+File=self.MetaFile, Line=Dummy4)
+if int(MaxDatumSize, 0) < 0:
+EdkLogger.error('build', FORMAT_INVALID, "The size value 
can't be set to negative value for %s." % ".".join((TokenSpaceGuid, PcdCName)),
+File=self.MetaFile, Line=Dummy4)
 SkuInfo = SkuInfoClass(SkuName, self.SkuIds[SkuName][0], '', '', 
'', '', '', PcdValue)
 if (PcdCName, TokenSpaceGuid) in Pcds:
 pcdObject = Pcds[PcdCName, TokenSpaceGuid]
 pcdObject.SkuInfoList[SkuName] = SkuInfo
 if MaxDatumSize.strip():
@@ -2712,10 +2726,17 @@ class DscBuildData(PlatformBuildClassObject):
 # For the Integer & Boolean type, the optional data can only be 
InitialValue.
 # At this point, we put all the data into the PcdClssObject for we 
don't know the PCD's datumtype
 # until the DEC parser has been called.
 #
 VpdOffset, MaxDatumSize, InitialValue = 
self._ValidatePcd(PcdCName, TokenSpaceGuid, Setting, Type, Dummy4)
+if MaxDatumSize:
+if int(MaxDatumSize, 0) > 0x:
+EdkLogger.error('build', FORMAT_INVALID, "The size value 
must not exceed the maximum value of 0x (UINT16) for %s." % 
".".join((TokenSpaceGuid, PcdCName)),
+File=self.MetaFile, Line=Dummy4)
+if int(MaxDatumSize, 0) < 0:
+EdkLogger.error('build', FORMAT_INVALID, "The size value 
can't be set to negative value for %s." % ".".join((TokenSpaceGuid, PcdCName)),
+File=self.MetaFile, Line=Dummy4)
 SkuInfo = SkuInfoClass(SkuName, self.SkuIds[SkuName][0], '', '', 
'', '', VpdOffset, InitialValue)
 if (PcdCName, TokenSpaceGuid) in Pcds:
 pcdObject = Pcds[PcdCName, TokenSpaceGuid]
 pcdObject.SkuInfoList[SkuName] = SkuInfo
 if MaxDatumSize.strip():
-- 
2.6.1.windows.1

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

Re: [edk2] [PATCH v2 1/1] ArmPkg/GenericWatchdogDxe: Split 64bit register write to 2x32bit

[Ard Biesheuvel]

On 2 August 2018 at 22:50, Marcin Wojtas  wrote:
> According to the SBSA specification the Watchdog Compare
> Register is split into two separate 32bit registers.
> EDK2 code uses a single 64bit transaction to update
> them, which can be problematic, depending on the SoC
> implementation and could result in an unpredicted behavior.
>
> Fix this by modifying WatchdogWriteCompareRegister routine to
> use two consecutive 32bit writes to the Watchdog Compare Register
> Low and High, using new dedicated macros.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas 
> Reviewed-by: Leif Lindholm 

Pushed as dd4cae4d82c7

Thanks

> ---
> Changelog v1 -> v2:
> - use separate macros for WCV register low and high
> - improve commit message
> - add Leif's RB
>
>  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h| 3 ++-
>  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h 
> b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
> index 9e2aebc..4f42c56 100644
> --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
> +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
> @@ -20,7 +20,8 @@
>  // Control Frame:
>  #define GENERIC_WDOG_CONTROL_STATUS_REG   ((UINTN)FixedPcdGet64 
> (PcdGenericWatchdogControlBase) + 0x000)
>  #define GENERIC_WDOG_OFFSET_REG   ((UINTN)FixedPcdGet64 
> (PcdGenericWatchdogControlBase) + 0x008)
> -#define GENERIC_WDOG_COMPARE_VALUE_REG((UINTN)FixedPcdGet64 
> (PcdGenericWatchdogControlBase) + 0x010)
> +#define GENERIC_WDOG_COMPARE_VALUE_REG_LOW((UINTN)FixedPcdGet64 
> (PcdGenericWatchdogControlBase) + 0x010)
> +#define GENERIC_WDOG_COMPARE_VALUE_REG_HIGH   ((UINTN)FixedPcdGet64 
> (PcdGenericWatchdogControlBase) + 0x014)
>
>  // Values of bit 0 of the Control/Status Register
>  #define GENERIC_WDOG_ENABLED  1
> diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
> b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> index 3180f01..8ccf153 100644
> --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> @@ -56,7 +56,8 @@ WatchdogWriteCompareRegister (
>UINT64  Value
>)
>  {
> -  MmioWrite64 (GENERIC_WDOG_COMPARE_VALUE_REG, Value);
> +  MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG_LOW, Value & MAX_UINT32);
> +  MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG_HIGH, (Value >> 32) & 
> MAX_UINT32);
>  }
>
>  VOID
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] PerformancePkg on multiple platform

[prabin ca]

Hi Team,

I’m new to uefi and edk. Currently I’m trying to get performance of my dxe 
driver using PerformancePkg of EDK source code.

I’m using perf_start and perf_end T respected check points, it’s hot build and 
tested well in 2/3 platform. It’s giving proper response.

But when I’m Checking with multiple platform, in some of platforms it’s not 
working and got hang in uefi she’ll itself.

Please help me to resolve asap, it will be really helpful.

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

[edk2] [Patch][edk2-platforms/devel-MinnowBoard3-UDK2017] Boot Option Fix

[xianhu2x]

Detect if EFI Shell file is present before adding boot option for it.

Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: xianhu2x 
---
 .../PlatformBootManagerLib/PlatformBootOption.c| 51 ++
 1 file changed, 22 insertions(+), 29 deletions(-)

diff --git 
a/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootOption.c
 
b/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootOption.c
index c77b20f686..0eec6ca081 100644
--- 
a/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootOption.c
+++ 
b/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootOption.c
@@ -272,35 +272,34 @@ CreateFvBootOption (
 
   EfiInitializeFwVolDevicepathNode (, FileGuid);
 
-  if (!CompareGuid (, FileGuid)) {
+  Status = gBS->HandleProtocol (
+  gImageHandle,
+  ,
+  (VOID **) 
+  );
+  if (!EFI_ERROR (Status)) {
 Status = gBS->HandleProtocol (
-gImageHandle,
-,
-(VOID **) 
+LoadedImage->DeviceHandle,
+,
+(VOID **) 
 );
 if (!EFI_ERROR (Status)) {
-  Status = gBS->HandleProtocol (
-  LoadedImage->DeviceHandle,
-  ,
-  (VOID **) 
+  Buffer  = NULL;
+  Size= 0;
+  Status  = Fv->ReadSection (
+  Fv,
+  FileGuid,
+  EFI_SECTION_PE32,
+  0,
+  ,
+  ,
+  
   );
-  if (!EFI_ERROR (Status)) {
-Buffer  = NULL;
-Size= 0;
-Status  = Fv->ReadSection (
-Fv,
-FileGuid,
-EFI_SECTION_PE32,
-0,
-,
-,
-
-);
-if (Buffer != NULL) {
-  FreePool (Buffer);
-}
+  if (Buffer != NULL) {
+FreePool (Buffer);
   }
 }
+  }
 if (EFI_ERROR (Status)) {
   return EFI_NOT_FOUND;
 }
@@ -309,12 +308,6 @@ CreateFvBootOption (
DevicePathFromHandle (LoadedImage->DeviceHandle),
(EFI_DEVICE_PATH_PROTOCOL *) 
);
-  } else {
-DevicePath = AppendDevicePathNode (
-   BdsCreateShellDevicePath (),
-   (EFI_DEVICE_PATH_PROTOCOL *) 
-   );
-  }
 
   Status = EfiBootManagerInitializeLoadOption (
  BootOption,
-- 
2.14.1.windows.1

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

[edk2] [Patch][edk2-platforms/devel-MinnowBoard3-UDK2017] Network Setup Fix

[xianhu2x]

Remove setup variable EfiNetworkSupport since it's not used by BdsEntry anymore.

Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: xianhu2x 
---
 .../Common/PlatformSettings/PlatformSetupDxe/Boot.vfi  | 7 ---
 1 file changed, 7 deletions(-)

diff --git 
a/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformSetupDxe/Boot.vfi 
b/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformSetupDxe/Boot.vfi
index 65f772c6c5..6b6f262efd 100644
--- 
a/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformSetupDxe/Boot.vfi
+++ 
b/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformSetupDxe/Boot.vfi
@@ -74,13 +74,6 @@ form formid = BOOT_CONFIGURATION_FORM_ID,
   key= 0;
   endif;
 
-  oneof varid = Setup.EfiNetworkSupport,
-prompt   = STRING_TOKEN(STR_EFI_NETWORK_CONTROL),
-help = STRING_TOKEN(STR_EFI_NETWORK_CONTROL_HELP),
-option text = STRING_TOKEN(STR_DISABLE), value = 0, flags = DEFAULT | 
MANUFACTURING | RESET_REQUIRED;
-option text = STRING_TOKEN(STR_ENABLE), value = 1, flags = 0 | 
RESET_REQUIRED;
-  endoneof;
-
 endform;
 
 
-- 
2.14.1.windows.1

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

Re: [edk2] [Patch][edk2-platforms/devel-IntelAtomProcessorE3900] Get boot mode

[Wei, David]

Reviewed-by: zwei4 


Thanks,
David  Wei

Intel SSG/STO/UEFI BIOS 

-Original Message-
From: Guo, Mang 
Sent: Friday, August 3, 2018 2:18 PM
To: edk2-devel@lists.01.org
Cc: Wei, David 
Subject: [Patch][edk2-platforms/devel-IntelAtomProcessorE3900] Get boot mode

Get boot mode in platformBootManagerBeforeConsole.

Cc: Wei, David 

Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: Guo Mang 
---
 .../Common/Library/PlatformBootManagerLib/PlatformBootManager.c  | 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
 
b/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
index 6715d90..c9c2cbf 100644
--- 
a/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
+++ 
b/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
@@ -858,6 +858,7 @@ PlatformBootManagerBeforeConsole (
   //
   // Fill ConIn/ConOut in Full Configuration boot mode.
   //
+  mBootMode = GetBootModeHob();
   DEBUG ((DEBUG_ERROR, "PlatformBootManagerInit - %x\n", mBootMode));
   if (mBootMode == BOOT_WITH_FULL_CONFIGURATION ||
   mBootMode == BOOT_WITH_DEFAULT_SETTINGS ||
-- 
2.10.1.windows.1

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

Re: [edk2] [PATCH] OvmfPkg/PlatformDebugLibIoPort: fix port detection for use in the DXE Core

[Jordan Justen]

On 2018-08-02 17:30:45, Laszlo Ersek wrote:
> The DXE Core is one of those modules that call
> ProcessLibraryConstructorList() manually.
> 
> Before DxeMain() [MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c] calls
> ProcessLibraryConstructorList(), and through it, our
> PlatformDebugLibIoPortConstructor() function, DxeMain() invokes the
> DEBUG() macro multiple times. That macro lands in our
> PlatformDebugLibIoPortFound() function -- which currently relies on the
> "mDebugIoPortFound" global variable that has (not yet) been set by the
> constructor. As a result, early debug messages from the DXE Core are lost.
> 
> Move the device detection into PlatformDebugLibIoPortFound(), also caching
> the fact (not just the result) of the device detection.
> 
> (We could introduce a separate DebugLib instance just for the DXE Core,
> but the above approach works for all modules that currently consume the
> PlatformDebugLibIoPort instance (which means "everything but SEC").)
> 
> This restores messages such as:
> 
> > CoreInitializeMemoryServices:
> >   BaseAddress - 0x7AF21000 Length - 0x3CDE000 MinimalMemorySizeNeeded - 
> > 0x10F4000
> 
> Keep the empty constructor function -- OVMF's DebugLib instances have
> always had constructors; we had better not upset constructor dependency
> ordering by making our instance(s) constructor-less.
> 
> Cc: Ard Biesheuvel 
> Cc: Brijesh Singh 
> Cc: Jordan Justen 
> Fixes: c09d9571300a089c35f5df2773b70edc25050d0d
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 
> ---
> 
> Notes:
> Brijesh, can you please test this patch on SEV, with and without
> capturing the debug port? (In the first case, the debug log should just
> work; in the second case, the boot should remain fast.) Thanks!
> 
> Repo:   https://github.com/lersek/edk2.git
> Branch: debuglib_dxecore
> 
>  OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c | 24 
> 
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c 
> b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
> index 81c44eece95f..74aef2e37b42 100644
> --- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
> @@ -16,14 +16,26 @@
>  #include 
>  #include "DebugLibDetect.h"
>  
> +
> +//
> +// Set to TRUE if the debug I/O port has been checked
> +//
> +STATIC BOOLEAN mDebugIoPortChecked = FALSE;

Could this be a static variable in the function? If not, should it be
follow by a blank line?

I agree that Brijesh should verify it, but pending that:

Reviewed-by: Jordan Justen 

>  //
>  // Set to TRUE if the debug I/O port is enabled
>  //
>  STATIC BOOLEAN mDebugIoPortFound = FALSE;
>  
>  /**
> -  This constructor function checks if the debug I/O port device is present,
> -  caching the result for later use.
> +  This constructor function must not do anything.
> +
> +  Some modules consuming this library instance, such as the DXE Core, invoke
> +  the DEBUG() macro before they explicitly call
> +  ProcessLibraryConstructorList(). Therefore the auto-generated call from
> +  ProcessLibraryConstructorList() to this constructor function may be 
> preceded
> +  by some calls to PlatformDebugLibIoPortFound() below. Hence
> +  PlatformDebugLibIoPortFound() must not rely on anything this constructor
> +  could set up.
>  
>@retval RETURN_SUCCESS   The constructor always returns RETURN_SUCCESS.
>  
> @@ -34,12 +46,12 @@ PlatformDebugLibIoPortConstructor (
>VOID
>)
>  {
> -  mDebugIoPortFound = PlatformDebugLibIoPortDetect();
>return RETURN_SUCCESS;
>  }
>  
>  /**
> -  Return the cached result of detecting the debug I/O port device.
> +  At the first call, check if the debug I/O port device is present, and cache
> +  the result for later use. At subsequent calls, return the cached result.
>  
>@retval TRUE   if the debug I/O port device was detected.
>@retval FALSE  otherwise
> @@ -51,5 +63,9 @@ PlatformDebugLibIoPortFound (
>VOID
>)
>  {
> +  if (!mDebugIoPortChecked) {
> +mDebugIoPortFound = PlatformDebugLibIoPortDetect ();
> +mDebugIoPortChecked = TRUE;
> +  }
>return mDebugIoPortFound;
>  }
> -- 
> 2.14.1.3.gb7cf6e02401b
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH edk2-platforms 2/2] Silicon/SynQuacer: Add status property in PCIe & SDHC DT nodes

[Sumit Garg]

Add status = "disabled" property by default for PCIe and SDHC DT nodes.
If required, update them at runtime with status = "okay". Using this
method we don't need extra DTB_PADDING.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sumit Garg 
Cc: Leif Lindholm 
Reviewed-by: Ard Biesheuvel 
---
 .../Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi  |  3 ++
 .../SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c  | 40 --
 2 files changed, 10 insertions(+), 33 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi 
b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
index d6a5f013e58c..003e21bd6f85 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
@@ -473,6 +473,7 @@
 
 msi-map = <0x000  0x0 0x7f00>;
 dma-coherent;
+status = "disabled";
 };
 
 pcie1: pcie@7000 {
@@ -492,6 +493,7 @@
 
 msi-map = <0x0  0x1 0x7f00>;
 dma-coherent;
+status = "disabled";
 };
 
 gpio: gpio@5100 {
@@ -537,6 +539,7 @@
 clocks = <_alw_c_0 _alw_b_0>;
 clock-names = "core", "iface";
 dma-coherent;
+status = "disabled";
 };
 
 clk_alw_1_8: spi_ihclk {
diff --git 
a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
 
b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
index 77db30c204fe..96090c20502c 100644
--- 
a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
+++ 
b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
@@ -22,32 +22,6 @@
 #include 
 #include 
 
-// add enough space for three instances of 'status = "disabled"'
-#define DTB_PADDING   64
-
-STATIC
-VOID
-DisableDtNode (
-  IN  VOID*Dtb,
-  IN  CONST CHAR8 *NodePath
-  )
-{
-  INT32   Node;
-  INT32   Rc;
-
-  Node = fdt_path_offset (Dtb, NodePath);
-  if (Node < 0) {
-DEBUG ((DEBUG_ERROR, "%a: failed to locate DT path '%a': %a\n",
-  __FUNCTION__, NodePath, fdt_strerror (Node)));
-return;
-  }
-  Rc = fdt_setprop_string (Dtb, Node, "status", "disabled");
-  if (Rc < 0) {
-DEBUG ((DEBUG_ERROR, "%a: failed to set status to 'disabled' on '%a': 
%a\n",
-  __FUNCTION__, NodePath, fdt_strerror (Rc)));
-  }
-}
-
 STATIC
 VOID
 EnableDtNode (
@@ -105,7 +79,7 @@ DtPlatformLoadDtb (
 return EFI_NOT_FOUND;
   }
 
-  CopyDtbSize = OrigDtbSize + DTB_PADDING;
+  CopyDtbSize = OrigDtbSize;
   CopyDtb = AllocatePool (CopyDtbSize);
   if (CopyDtb == NULL) {
 return EFI_OUT_OF_RESOURCES;
@@ -118,17 +92,17 @@ DtPlatformLoadDtb (
 return EFI_NOT_FOUND;
   }
 
-  if (!(PcdGet8 (PcdPcieEnableMask) & BIT0)) {
-DisableDtNode (CopyDtb, "/pcie@6000");
+  if (PcdGet8 (PcdPcieEnableMask) & BIT0) {
+EnableDtNode (CopyDtb, "/pcie@6000");
   }
-  if (!(PcdGet8 (PcdPcieEnableMask) & BIT1)) {
-DisableDtNode (CopyDtb, "/pcie@7000");
+  if (PcdGet8 (PcdPcieEnableMask) & BIT1) {
+EnableDtNode (CopyDtb, "/pcie@7000");
   }
 
   SettingsVal = PcdGet64 (PcdPlatformSettings);
   Settings = (SYNQUACER_PLATFORM_VARSTORE_DATA *)
-  if (Settings->EnableEmmc == EMMC_DISABLED) {
-DisableDtNode (CopyDtb, "/sdhci@5230");
+  if (Settings->EnableEmmc == EMMC_ENABLED) {
+EnableDtNode (CopyDtb, "/sdhci@5230");
   }
 
   if (IsOpteePresent()) {
-- 
2.7.4

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

[edk2] [PATCH edk2-platforms 1/2] Silicon/SynQuacer: add optional OP-TEE DT node

[Sumit Garg]

OP-TEE is optional on Developerbox controlled via SCP firmware. To check
if we need to enable OP-TEE DT node, we use "IsOpteePresent" OpteeLib api.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sumit Garg 
Cc: Leif Lindholm 
Reviewed-by: Ard Biesheuvel 
---
 Platform/Socionext/DeveloperBox/DeveloperBox.dsc   |  1 +
 .../Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi  |  8 +++
 .../SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c  | 28 ++
 .../SynQuacerDtbLoaderLib.inf  |  2 ++
 4 files changed, 39 insertions(+)

diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc 
b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
index fc498eb65217..4ff5df978e8e 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
@@ -76,6 +76,7 @@ [LibraryClasses.common]
   
ArmPlatformStackLib|ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
   ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
   
ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
+  OpteeLib|ArmPkg/Library/OpteeLib/OpteeLib.inf
 
   BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
   BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi 
b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
index 37d642e4b237..d6a5f013e58c 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
@@ -574,6 +574,14 @@
 #address-cells = <1>;
 #size-cells = <0>;
 };
+
+firmware {
+optee {
+compatible = "linaro,optee-tz";
+method = "smc";
+status = "disabled";
+};
+};
 };
 
 #include "SynQuacerCaches.dtsi"
diff --git 
a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
 
b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
index 897d06743708..77db30c204fe 100644
--- 
a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
+++ 
b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 // add enough space for three instances of 'status = "disabled"'
@@ -47,6 +48,29 @@ DisableDtNode (
   }
 }
 
+STATIC
+VOID
+EnableDtNode (
+  IN  VOID*Dtb,
+  IN  CONST CHAR8 *NodePath
+  )
+{
+  INT32   Node;
+  INT32   Rc;
+
+  Node = fdt_path_offset (Dtb, NodePath);
+  if (Node < 0) {
+DEBUG ((DEBUG_ERROR, "%a: failed to locate DT path '%a': %a\n",
+  __FUNCTION__, NodePath, fdt_strerror (Node)));
+return;
+  }
+  Rc = fdt_setprop_string (Dtb, Node, "status", "okay");
+  if (Rc < 0) {
+DEBUG ((DEBUG_ERROR, "%a: failed to set status to 'disabled' on '%a': 
%a\n",
+  __FUNCTION__, NodePath, fdt_strerror (Rc)));
+  }
+}
+
 /**
   Return a pool allocated copy of the DTB image that is appropriate for
   booting the current platform via DT.
@@ -107,6 +131,10 @@ DtPlatformLoadDtb (
 DisableDtNode (CopyDtb, "/sdhci@5230");
   }
 
+  if (IsOpteePresent()) {
+EnableDtNode (CopyDtb, "/firmware/optee");
+  }
+
   *Dtb = CopyDtb;
   *DtbSize = CopyDtbSize;
 
diff --git 
a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
 
b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
index 548d62fd5c0a..fd21f7c376ce 100644
--- 
a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
+++ 
b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
@@ -24,6 +24,7 @@ [Sources]
   SynQuacerDtbLoaderLib.c
 
 [Packages]
+  ArmPkg/ArmPkg.dec
   MdePkg/MdePkg.dec
   EmbeddedPkg/EmbeddedPkg.dec
   Silicon/Socionext/SynQuacer/SynQuacer.dec
@@ -34,6 +35,7 @@ [LibraryClasses]
   DxeServicesLib
   FdtLib
   MemoryAllocationLib
+  OpteeLib
 
 [Pcd]
   gSynQuacerTokenSpaceGuid.PcdPcieEnableMask
-- 
2.7.4

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

[edk2] [PATCH edk2-platforms 0/2] Silicon/SynQuacer: update Device Tree

[Sumit Garg]

Following are DT updates:
- Add optional OP-TEE DT node
- Add status property in PCIe & SDHC DT nodes

Sumit Garg (2):
  Silicon/SynQuacer: add optional OP-TEE DT node
  Silicon/SynQuacer: Add status property in PCIe & SDHC DT nodes

 Platform/Socionext/DeveloperBox/DeveloperBox.dsc   |  1 +
 .../Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi  | 11 +
 .../SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c  | 26 --
 .../SynQuacerDtbLoaderLib.inf  |  2 ++
 4 files changed, 28 insertions(+), 12 deletions(-)

-- 
2.7.4

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

[edk2] [Patch][edk2-platforms/devel-IntelAtomProcessorE3900] Get boot mode

[Guo, Mang]

Get boot mode in platformBootManagerBeforeConsole.

Cc: Wei, David 

Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: Guo Mang 
---
 .../Common/Library/PlatformBootManagerLib/PlatformBootManager.c  | 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
 
b/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
index 6715d90..c9c2cbf 100644
--- 
a/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
+++ 
b/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
@@ -858,6 +858,7 @@ PlatformBootManagerBeforeConsole (
   //
   // Fill ConIn/ConOut in Full Configuration boot mode.
   //
+  mBootMode = GetBootModeHob();
   DEBUG ((DEBUG_ERROR, "PlatformBootManagerInit - %x\n", mBootMode));
   if (mBootMode == BOOT_WITH_FULL_CONFIGURATION ||
   mBootMode == BOOT_WITH_DEFAULT_SETTINGS ||
-- 
2.10.1.windows.1

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

[edk2] [Patch] DSC Spec: limit the VOID* PCD max size to UINT16

[Yonghong Zhu]

current the max size of VOID* PCD in the spec is a number that doesn't
no limitation, now this patch update to limit the max size to UINT16,
besides, also update some typo to make sure the format in chapter 2
and chapter 3 are align.

Cc: Liming Gao 
Cc: Michael Kinney 
Cc: Kevin W Shaw 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 2_dsc_overview/29_pcd_sections.md| 6 +++---
 3_edk_ii_dsc_file_format/310_pcd_sections.md | 8 
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/2_dsc_overview/29_pcd_sections.md 
b/2_dsc_overview/29_pcd_sections.md
index 6547656..b33026c 100644
--- a/2_dsc_overview/29_pcd_sections.md
+++ b/2_dsc_overview/29_pcd_sections.md
@@ -1,9 +1,9 @@