Re: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

[Bi, Dandan]

Hi Star,

Yes. You are right. mAcpiBootPerformanceTable->Header.Length will be updated 
accordingly. We can get the real table length.
Currently there are some contents in the table are not updated, just use the 
value in the memory.
Previously the memory value is zero.  But without ZeroMem(), the value may be 
non-zero. That's the difference. It may impact the code to parse the table.

Thanks,
Dandan

-Original Message-
From: Zeng, Star 
Sent: Friday, May 25, 2018 10:45 AM
To: Bi, Dandan ; Ard Biesheuvel 
; Laszlo Ersek 
Cc: Dong, Eric ; edk2-devel@lists.01.org; Gao, Liming 
; Leif Lindholm ; Kinney, 
Michael D ; Zeng, Star 
Subject: RE: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use 
AllocatePeiAccessiblePages

mAcpiBootPerformanceTable->Header.Length could reflect the real table length, 
right?
When the padding (from the PCD) memory is used, 
mAcpiBootPerformanceTable->Header.Length will be updated accordingly.
If that is the case, it seems safe without ZeroMem.
Anyway it needs to be verified.


But, to be simple, I do not object to have ZeroMem to be equivalent with 
previous code.



Thanks,
Star
-Original Message-
From: Bi, Dandan
Sent: Friday, May 25, 2018 10:01 AM
To: Ard Biesheuvel ; Laszlo Ersek 
Cc: Dong, Eric ; edk2-devel@lists.01.org; Gao, Liming 
; Leif Lindholm ; Kinney, 
Michael D ; Zeng, Star 
Subject: RE: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use 
AllocatePeiAccessiblePages

Hi Ard,

Thank you very much for helping fix this issue.

And for ZeroMem(), it should be added back.
Because after the memory is allocated, the optional padding (from the PCD) 
memory will be used directly. 
If ZeoMem() is dropped, then some contents in the padding memory will not be 
zero. Thus will impact the tool to parse the contents.

Thanks,
Dandan
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: Thursday, May 24, 2018 8:54 PM
To: Laszlo Ersek 
Cc: Dong, Eric ; edk2-devel@lists.01.org; Gao, Liming 
; Bi, Dandan ; Leif Lindholm 
; Kinney, Michael D ; 
Zeng, Star 
Subject: Re: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use 
AllocatePeiAccessiblePages

On 24 May 2018 at 14:49, Laszlo Ersek  wrote:
> On 05/24/18 11:09, Ard Biesheuvel wrote:
>> Replace the call to and implementation of the function
>> FpdtAllocateReservedMemoryBelow4G() with a call to 
>> AllocatePeiAccessiblePages, which boils down to the same on X64, but 
>> does not crash non-X64 systems that lack memory below 4 GB.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>> ---
>> Note that the ZeroMem() call is dropped, but it is redundant anyway, 
>> given that the subsequent CopyMem() call supersedes it immediately.
>
> I'm not sure the ZeroMem() is redundant. The allocation size -- before 
> rounding up to full pages -- is computed like this:
>
>   BootPerformanceDataSize = sizeof (BOOT_PERFORMANCE_TABLE) + 
> mPerformanceLength + PcdGet32 (PcdExtFpdtBootRecordPadSize);
>   if (SmmCommData != NULL && SmmBootRecordData != NULL) {
> BootPerformanceDataSize += SmmBootRecordDataSize;
>   }
>
> ZeroMem() covers all of the above, but the CopyMem() calls don't seem to 
> cover the optional padding (from the PCD).
>
> I'm unsure if that matters, of course.
>

You're quite right. I'm not sure how I missed that.

In any case, I can add back the ZeroMem () quite easily after the single 
invocation of AllocatePeiAccessiblePages() that I am adding in this file.

> The patch looks good to me otherwise.
>
> Thanks
> Laszlo
>
>
>>
>>  MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c |
>> 45 ++--
>>  1 file changed, 4 insertions(+), 41 deletions(-)
>>
>> diff --git
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> index 71d624fc9ce9..b1f09710b65c 100644
>> ---
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLi
>> +++ b.c
>> @@ -165,46 +165,6 @@ IsKnownID (
>>}
>>  }
>>
>> -/**
>> -  Allocate EfiReservedMemoryType below 4G memory address.
>> -
>> -  This function allocates EfiReservedMemoryType below 4G memory address.
>> -
>> -  @param[in]  Size   Size of memory to allocate.
>> -
>> -  @return Allocated address 

Re: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

[Zeng, Star]

mAcpiBootPerformanceTable->Header.Length could reflect the real table length, 
right?
When the padding (from the PCD) memory is used, 
mAcpiBootPerformanceTable->Header.Length will be updated accordingly.
If that is the case, it seems safe without ZeroMem.
Anyway it needs to be verified.


But, to be simple, I do not object to have ZeroMem to be equivalent with 
previous code.



Thanks,
Star
-Original Message-
From: Bi, Dandan 
Sent: Friday, May 25, 2018 10:01 AM
To: Ard Biesheuvel ; Laszlo Ersek 
Cc: Dong, Eric ; edk2-devel@lists.01.org; Gao, Liming 
; Leif Lindholm ; Kinney, 
Michael D ; Zeng, Star 
Subject: RE: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use 
AllocatePeiAccessiblePages

Hi Ard,

Thank you very much for helping fix this issue.

And for ZeroMem(), it should be added back.
Because after the memory is allocated, the optional padding (from the PCD) 
memory will be used directly. 
If ZeoMem() is dropped, then some contents in the padding memory will not be 
zero. Thus will impact the tool to parse the contents.

Thanks,
Dandan
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: Thursday, May 24, 2018 8:54 PM
To: Laszlo Ersek 
Cc: Dong, Eric ; edk2-devel@lists.01.org; Gao, Liming 
; Bi, Dandan ; Leif Lindholm 
; Kinney, Michael D ; 
Zeng, Star 
Subject: Re: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use 
AllocatePeiAccessiblePages

On 24 May 2018 at 14:49, Laszlo Ersek  wrote:
> On 05/24/18 11:09, Ard Biesheuvel wrote:
>> Replace the call to and implementation of the function
>> FpdtAllocateReservedMemoryBelow4G() with a call to 
>> AllocatePeiAccessiblePages, which boils down to the same on X64, but 
>> does not crash non-X64 systems that lack memory below 4 GB.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>> ---
>> Note that the ZeroMem() call is dropped, but it is redundant anyway, 
>> given that the subsequent CopyMem() call supersedes it immediately.
>
> I'm not sure the ZeroMem() is redundant. The allocation size -- before 
> rounding up to full pages -- is computed like this:
>
>   BootPerformanceDataSize = sizeof (BOOT_PERFORMANCE_TABLE) + 
> mPerformanceLength + PcdGet32 (PcdExtFpdtBootRecordPadSize);
>   if (SmmCommData != NULL && SmmBootRecordData != NULL) {
> BootPerformanceDataSize += SmmBootRecordDataSize;
>   }
>
> ZeroMem() covers all of the above, but the CopyMem() calls don't seem to 
> cover the optional padding (from the PCD).
>
> I'm unsure if that matters, of course.
>

You're quite right. I'm not sure how I missed that.

In any case, I can add back the ZeroMem () quite easily after the single 
invocation of AllocatePeiAccessiblePages() that I am adding in this file.

> The patch looks good to me otherwise.
>
> Thanks
> Laszlo
>
>
>>
>>  MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c |
>> 45 ++--
>>  1 file changed, 4 insertions(+), 41 deletions(-)
>>
>> diff --git
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> index 71d624fc9ce9..b1f09710b65c 100644
>> ---
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLi
>> +++ b.c
>> @@ -165,46 +165,6 @@ IsKnownID (
>>}
>>  }
>>
>> -/**
>> -  Allocate EfiReservedMemoryType below 4G memory address.
>> -
>> -  This function allocates EfiReservedMemoryType below 4G memory address.
>> -
>> -  @param[in]  Size   Size of memory to allocate.
>> -
>> -  @return Allocated address for output.
>> -
>> -**/
>> -VOID *
>> -FpdtAllocateReservedMemoryBelow4G (
>> -  IN UINTN   Size
>> -  )
>> -{
>> -  UINTN Pages;
>> -  EFI_PHYSICAL_ADDRESS  Address;
>> -  EFI_STATUSStatus;
>> -  VOID  *Buffer;
>> -
>> -  Buffer  = NULL;
>> -  Pages   = EFI_SIZE_TO_PAGES (Size);
>> -  Address = 0x;
>> -
>> -  Status = gBS->AllocatePages (
>> -  AllocateMaxAddress,
>> -  EfiReservedMemoryType,
>> -  Pages,
>> -  
>> -  );
>> -  ASSERT_EFI_ERROR (Status);
>> -
>> -  if (!EFI_ERROR (Status)) {
>> -Buffer = (VOID *) (UINTN) Address;
>> -ZeroMem (Buffer, Size);
>> -  }
>> -
>> -  return Buffer;
>> -}
>> -
>>  /**
>>Allocate buffer for Boot Performance table.
>>
>> @@ -348,7 +308,10 @@ AllocateBootPerformanceTable (
>>  //
>>  // Fail to allocate at specified address, 

Re: [edk2] [RFC] Formalize source files to follow DOS format

[Gao, Liming]

I get your point. We will update this script and send version 2. 

> -Original Message-
> From: Carsey, Jaben
> Sent: Thursday, May 24, 2018 10:14 PM
> To: Gao, Liming ; Kinney, Michael D 
> 
> Cc: edk2-devel@lists.01.org
> Subject: RE: [edk2] [RFC] Formalize source files to follow DOS format
> 
> We could do something like we do for compiler flags... append or overwrite 
> depending on syntax.
> 
> > -Original Message-
> > From: Gao, Liming
> > Sent: Thursday, May 24, 2018 1:35 AM
> > To: Kinney, Michael D ; Carsey, Jaben
> > 
> > Cc: edk2-devel@lists.01.org
> > Subject: RE: [edk2] [RFC] Formalize source files to follow DOS format
> > Importance: High
> >
> > Mike:
> >   I agree your comments. On default file set, this script can have the 
> > default
> > ones. User can specify more set to append the default ones instead of
> > override the default ones.
> >
> > Thanks
> > Liming
> > >-Original Message-
> > >From: Kinney, Michael D
> > >Sent: Tuesday, May 22, 2018 6:59 AM
> > >To: Carsey, Jaben ; Kinney, Michael D
> > >
> > >Cc: Gao, Liming ; edk2-devel@lists.01.org
> > >Subject: RE: [edk2] [RFC] Formalize source files to follow DOS format
> > >
> > >Jaben,
> > >
> > >Yes.  With default behavior is default set and
> > >specifying one or more extensions overrides the
> > >default set.
> > >
> > >Mike
> > >
> > >> -Original Message-
> > >> From: Carsey, Jaben
> > >> Sent: Monday, May 21, 2018 3:43 PM
> > >> To: Kinney, Michael D 
> > >> Cc: Gao, Liming ; edk2-
> > >> de...@lists.01.org
> > >> Subject: Re: [edk2] [RFC] Formalize source files to
> > >> follow DOS format
> > >>
> > >> Mike,
> > >>
> > >> Perhaps a default set of file extensions that can be
> > >> overridden?
> > >>
> > >> -Jaben
> > >>
> > >>
> > >> > On May 21, 2018, at 3:41 PM, Kinney, Michael D
> > >>  wrote:
> > >> >
> > >> > Liming,
> > >> >
> > >> > We have a set of standard flags for tools that
> > >> > should always be present.
> > >> >
> > >> > --help
> > >> > -v
> > >> > -q
> > >> > --debug
> > >> >
> > >> > We should also always have the program name,
> > >> > description, version, and copyright.
> > >> >
> > >> > Please see BaseTools/Scripts/BinToPcd.py as
> > >> > an example.
> > >> >
> > >> > It might be useful to have a way to run this tool
> > >> > on a single file when BaseTools/Scripts/PatchCheck.py
> > >> > reports an issue.
> > >> >
> > >> > Do you think it would be good to have one option to
> > >> > scan path for file extensions that are documented as
> > >> > DOS line endings so the extensions do not have to be
> > >> > entered?
> > >> >
> > >> > Mike
> > >> >
> > >> >
> > >> >> -Original Message-
> > >> >> From: edk2-devel [mailto:edk2-devel-
> > >> >> boun...@lists.01.org] On Behalf Of Carsey, Jaben
> > >> >> Sent: Monday, May 21, 2018 7:50 AM
> > >> >> To: Gao, Liming ; edk2-
> > >> >> de...@lists.01.org
> > >> >> Subject: Re: [edk2] [RFC] Formalize source files to
> > >> >> follow DOS format
> > >> >>
> > >> >> Liming,
> > >> >>
> > >> >> One Pep8 thing.
> > >> >> Can you change to use the with statement for the file
> > >> >> read/write?
> > >> >>
> > >> >> Other small thoughts.
> > >> >> I think that FileList should be changed to a set as
> > >> >> order is not important.
> > >> >> Maybe wrapper the re.sub function with your own so
> > >> all
> > >> >> the .encode() are in one location?  As we move to
> > >> python
> > >> >> 3 we will have fewer changes to make.
> > >> >>
> > >> >>
> > >> >>> -Original Message-
> > >> >>> From: edk2-devel [mailto:edk2-devel-
> > >> >> boun...@lists.01.org] On Behalf Of
> > >> >>> Liming Gao
> > >> >>> Sent: Sunday, May 20, 2018 9:52 PM
> > >> >>> To: edk2-devel@lists.01.org
> > >> >>> Subject: [edk2] [RFC] Formalize source files to
> > >> follow
> > >> >> DOS format
> > >> >>>
> > >> >>> FormatDosFiles.py is added to clean up dos source
> > >> >> files. It bases on
> > >> >>> the rules defined in EDKII C Coding Standards
> > >> >> Specification.
> > >> >>> 5.1.2 Do not use tab characters
> > >> >>> 5.1.6 Only use CRLF (Carriage Return Line Feed) line
> > >> >> endings.
> > >> >>> 5.1.7 All files must end with CRLF
> > >> >>> No trailing white space in one line. (To be added in
> > >> >> spec)
> > >> >>>
> > >> >>> The source files in edk2 project with the below
> > >> >> postfix are dos format.
> > >> >>> .h .c .nasm .nasmb .asm .S .inf .dec .dsc .fdf .uni
> > >> >> .asl .aslc .vfr .idf
> > >> >>> .txt .bat .py
> > >> >>>
> > >> >>> The package maintainer can use this script to clean
> > >> up
> > >> >> all files in his
> > >> >>> package. The prefer way is to create one patch per
> > >> one
> > >> >> package.
> > >> >>>
> > >> >>> Contributed-under: TianoCore 

Re: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

[Bi, Dandan]

Hi Ard,

Thank you very much for helping fix this issue.

And for ZeroMem(), it should be added back.
Because after the memory is allocated, the optional padding (from the PCD) 
memory will be used directly. 
If ZeoMem() is dropped, then some contents in the padding memory will not be 
zero. Thus will impact the tool to parse the contents.

Thanks,
Dandan
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: Thursday, May 24, 2018 8:54 PM
To: Laszlo Ersek 
Cc: Dong, Eric ; edk2-devel@lists.01.org; Gao, Liming 
; Bi, Dandan ; Leif Lindholm 
; Kinney, Michael D ; 
Zeng, Star 
Subject: Re: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use 
AllocatePeiAccessiblePages

On 24 May 2018 at 14:49, Laszlo Ersek  wrote:
> On 05/24/18 11:09, Ard Biesheuvel wrote:
>> Replace the call to and implementation of the function
>> FpdtAllocateReservedMemoryBelow4G() with a call to 
>> AllocatePeiAccessiblePages, which boils down to the same on X64, but 
>> does not crash non-X64 systems that lack memory below 4 GB.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>> ---
>> Note that the ZeroMem() call is dropped, but it is redundant anyway, 
>> given that the subsequent CopyMem() call supersedes it immediately.
>
> I'm not sure the ZeroMem() is redundant. The allocation size -- before 
> rounding up to full pages -- is computed like this:
>
>   BootPerformanceDataSize = sizeof (BOOT_PERFORMANCE_TABLE) + 
> mPerformanceLength + PcdGet32 (PcdExtFpdtBootRecordPadSize);
>   if (SmmCommData != NULL && SmmBootRecordData != NULL) {
> BootPerformanceDataSize += SmmBootRecordDataSize;
>   }
>
> ZeroMem() covers all of the above, but the CopyMem() calls don't seem to 
> cover the optional padding (from the PCD).
>
> I'm unsure if that matters, of course.
>

You're quite right. I'm not sure how I missed that.

In any case, I can add back the ZeroMem () quite easily after the single 
invocation of AllocatePeiAccessiblePages() that I am adding in this file.

> The patch looks good to me otherwise.
>
> Thanks
> Laszlo
>
>
>>
>>  MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 
>> 45 ++--
>>  1 file changed, 4 insertions(+), 41 deletions(-)
>>
>> diff --git 
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c 
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> index 71d624fc9ce9..b1f09710b65c 100644
>> --- 
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLi
>> +++ b.c
>> @@ -165,46 +165,6 @@ IsKnownID (
>>}
>>  }
>>
>> -/**
>> -  Allocate EfiReservedMemoryType below 4G memory address.
>> -
>> -  This function allocates EfiReservedMemoryType below 4G memory address.
>> -
>> -  @param[in]  Size   Size of memory to allocate.
>> -
>> -  @return Allocated address for output.
>> -
>> -**/
>> -VOID *
>> -FpdtAllocateReservedMemoryBelow4G (
>> -  IN UINTN   Size
>> -  )
>> -{
>> -  UINTN Pages;
>> -  EFI_PHYSICAL_ADDRESS  Address;
>> -  EFI_STATUSStatus;
>> -  VOID  *Buffer;
>> -
>> -  Buffer  = NULL;
>> -  Pages   = EFI_SIZE_TO_PAGES (Size);
>> -  Address = 0x;
>> -
>> -  Status = gBS->AllocatePages (
>> -  AllocateMaxAddress,
>> -  EfiReservedMemoryType,
>> -  Pages,
>> -  
>> -  );
>> -  ASSERT_EFI_ERROR (Status);
>> -
>> -  if (!EFI_ERROR (Status)) {
>> -Buffer = (VOID *) (UINTN) Address;
>> -ZeroMem (Buffer, Size);
>> -  }
>> -
>> -  return Buffer;
>> -}
>> -
>>  /**
>>Allocate buffer for Boot Performance table.
>>
>> @@ -348,7 +308,10 @@ AllocateBootPerformanceTable (
>>  //
>>  // Fail to allocate at specified address, continue to allocate at any 
>> address.
>>  //
>> -mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
>> FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
>> +mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
>> AllocatePeiAccessiblePages (
>> + 
>> EfiReservedMemoryType,
>> + 
>> EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
>> + );
>>}
>>DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance 
>> Table address = 0x%x\n", mAcpiBootPerformanceTable));
>>
>>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch] BaseTools/GenFds: Remove redundant GetRealFileLine call

[Yonghong Zhu]

From: Zurcher, Christopher J 

The EvaluateConditional function should not call GetRealFileLine
because this is already done in Warning init and only needs to be
calculated in the event of a parsing failure. This fix stops
InsertedLines from being subtracted twice during error handling.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zurcher, Christopher J 
---
 BaseTools/Source/Python/GenFds/FdfParser.py | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/FdfParser.py 
b/BaseTools/Source/Python/GenFds/FdfParser.py
index 8a9296c..4518d8f 100644
--- a/BaseTools/Source/Python/GenFds/FdfParser.py
+++ b/BaseTools/Source/Python/GenFds/FdfParser.py
@@ -912,11 +912,10 @@ class FdfParser:
 # Highest priority
 
 return MacroDict
 
 def __EvaluateConditional(self, Expression, Line, Op = None, Value = None):
-FileLineTuple = GetRealFileLine(self.FileName, Line)
 MacroPcdDict = self.__CollectMacroPcd()
 if Op == 'eval':
 try:
 if Value:
 return ValueExpression(Expression, MacroPcdDict)(True)
@@ -937,16 +936,16 @@ class FdfParser:
 Info = GlobalData.gPlatformOtherPcds[Excpt.Pcd]
 raise Warning("Cannot use this PCD (%s) in an 
expression as"
   " it must be defined in a 
[PcdsFixedAtBuild] or [PcdsFeatureFlag] section"
   " of the DSC file (%s), and it is 
currently defined in this section:"
   " %s, line #: %d." % (Excpt.Pcd, 
GlobalData.gPlatformOtherPcds['DSCFILE'], Info[0], Info[1]),
-  *FileLineTuple)
+  self.FileName, Line)
 else:
 raise Warning("PCD (%s) is not defined in DSC file 
(%s)" % (Excpt.Pcd, GlobalData.gPlatformOtherPcds['DSCFILE']),
-  *FileLineTuple)
+  self.FileName, Line)
 else:
-raise Warning(str(Excpt), *FileLineTuple)
+raise Warning(str(Excpt), self.FileName, Line)
 else:
 if Expression.startswith('$(') and Expression[-1] == ')':
 Expression = Expression[2:-1]
 return Expression in MacroPcdDict
 
-- 
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] EmulatorPkg/SmbiosLib: Declare the correct library class.

[Marvin Häuser]

> -Original Message-
> From: Laszlo Ersek 
> Sent: Thursday, May 24, 2018 10:51 PM
> To: Jordan Justen ; edk2-devel@lists.01.org;
> Marvin Häuser 
> Cc: af...@apple.com
> Subject: Re: [edk2] [PATCH v2] EmulatorPkg/SmbiosLib: Declare the correct
> library class.
> 
> On 05/24/18 22:36, Jordan Justen wrote:
> > Reviewed-by: Jordan Justen 
> >
> > Pushed as 7dc7c7435e.
> >
> > On 2018-05-17 05:43:30, Marvin Häuser wrote:
> >> Currently, SmbiosLib declares the PcdLib library class. Update the
> >> declaration to declare SmbiosLib.
> >>
> >> V2:
> >>   - Do not change the copyright date as requested.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Marvin Haeuser 
> >> ---
> >>  EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf
> >> b/EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf
> >> index adcd7ef08e20..36d5c350f51a 100644
> >> --- a/EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf
> >> +++ b/EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf
> >> @@ -20,7 +20,7 @@ [Defines]
> >>FILE_GUID  = 881863A2-09FD-3E44-8D62-7AE038D03747
> >>MODULE_TYPE= DXE_DRIVER
> >>VERSION_STRING = 1.0
> >> -  LIBRARY_CLASS  = PcdLib|DXE_CORE DXE_DRIVER
> DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER SMM_CORE
> UEFI_APPLICATION UEFI_DRIVER
> >> +  LIBRARY_CLASS  = SmbiosLib|DXE_CORE DXE_DRIVER
> DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER SMM_CORE
> UEFI_APPLICATION UEFI_DRIVER
> >>
> >>CONSTRUCTOR= SmbiosLibConstructor
> 
> (Just because I pondered the question a few days ago, independently, I'll
> voice it here:)
> 
> Should BaseTools catch this? "EmulatorPkg/EmulatorPkg.dsc" contains the
> following library resolution:
> 
>   SmbiosLib|EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf
> 
> I think it should be possible to flag that the class of the lib instance 
> doesn't
> match the lib class that the platform DSC is resolving.

I think it definitely should, there is no case (known to me) where those two 
values diverging would be a supported or making any sense.

> 
> (Or is Marvin's patch the result of such an error message already?)

No, I was just checking the library for reference and noticed the mistake. I 
did not actually build EmulatorPkg though.

Regards,
Marvin

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

Re: [edk2] [PATCH v2] EmulatorPkg/SmbiosLib: Declare the correct library class.

[Laszlo Ersek]

On 05/24/18 22:36, Jordan Justen wrote:
> Reviewed-by: Jordan Justen 
> 
> Pushed as 7dc7c7435e.
> 
> On 2018-05-17 05:43:30, Marvin Häuser wrote:
>> Currently, SmbiosLib declares the PcdLib library class. Update the
>> declaration to declare SmbiosLib.
>>
>> V2:
>>   - Do not change the copyright date as requested.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marvin Haeuser 
>> ---
>>  EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf 
>> b/EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf
>> index adcd7ef08e20..36d5c350f51a 100644
>> --- a/EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf
>> +++ b/EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf
>> @@ -20,7 +20,7 @@ [Defines]
>>FILE_GUID  = 881863A2-09FD-3E44-8D62-7AE038D03747
>>MODULE_TYPE= DXE_DRIVER
>>VERSION_STRING = 1.0
>> -  LIBRARY_CLASS  = PcdLib|DXE_CORE DXE_DRIVER 
>> DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION 
>> UEFI_DRIVER 
>> +  LIBRARY_CLASS  = SmbiosLib|DXE_CORE DXE_DRIVER 
>> DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION 
>> UEFI_DRIVER 
>>  
>>CONSTRUCTOR= SmbiosLibConstructor

(Just because I pondered the question a few days ago, independently,
I'll voice it here:)

Should BaseTools catch this? "EmulatorPkg/EmulatorPkg.dsc" contains the
following library resolution:

  SmbiosLib|EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf

I think it should be possible to flag that the class of the lib instance
doesn't match the lib class that the platform DSC is resolving.

(Or is Marvin's patch the result of such an error message already?)

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

Re: [edk2] [PATCH v2] EmulatorPkg/SmbiosLib: Declare the correct library class.

[Jordan Justen]

Reviewed-by: Jordan Justen 

Pushed as 7dc7c7435e.

On 2018-05-17 05:43:30, Marvin Häuser wrote:
> Currently, SmbiosLib declares the PcdLib library class. Update the
> declaration to declare SmbiosLib.
> 
> V2:
>   - Do not change the copyright date as requested.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marvin Haeuser 
> ---
>  EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf 
> b/EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf
> index adcd7ef08e20..36d5c350f51a 100644
> --- a/EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf
> +++ b/EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf
> @@ -20,7 +20,7 @@ [Defines]
>FILE_GUID  = 881863A2-09FD-3E44-8D62-7AE038D03747
>MODULE_TYPE= DXE_DRIVER
>VERSION_STRING = 1.0
> -  LIBRARY_CLASS  = PcdLib|DXE_CORE DXE_DRIVER 
> DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION 
> UEFI_DRIVER 
> +  LIBRARY_CLASS  = SmbiosLib|DXE_CORE DXE_DRIVER 
> DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION 
> UEFI_DRIVER 
>  
>CONSTRUCTOR= SmbiosLibConstructor
>  
> -- 
> 2.17.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 0/7] OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library

[Laszlo Ersek]

On 05/23/18 22:21, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: pci_cap_v2
>
> In v2, the new libs are initially introduced under OvmfPkg, rather
> than MdePkg. v1 was posted at
> <20180504213637.11266-1-lersek@redhat.com">http://mid.mail-archive.com/20180504213637.11266-1-lersek@redhat.com>.
>
> Cc: Ard Biesheuvel 
> Cc: Jordan Justen 

I've addressed Ard's requests with the following (cumulative) diff:

> diff --git a/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c
> b/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c
> index 6789359f0a54..c059264b322d 100644
> --- a/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c
> +++ b/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c
> @@ -874,6 +874,8 @@ PciCapGetInfo (
>  {
>PCI_CAP *InstanceZero;
>
> +  ASSERT (Info != NULL);
> +
>InstanceZero = (Cap->Key.Instance == 0 ? Cap :
>Cap->NumInstancesUnion.InstanceZero);
>
> diff --git a/OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c
> b/OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c
> index 84369e4dc3a8..358d87f93103 100644
> --- a/OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c
> +++ b/OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c
> @@ -73,10 +73,10 @@ ProtoDevTransferConfig (
>  // possible in one iteration of the loop. Otherwise, transfer only one
>  // unit, to improve the alignment.
>  //
> -if (Size >= BIT2 && (ConfigOffset & (BIT2 - 1)) == 0) {
> +if (Size >= 4 && (ConfigOffset & 3) == 0) {
>Width = EfiPciIoWidthUint32;
>Count = Size >> Width;
> -} else if (Size >= BIT1 && (ConfigOffset & (BIT1 - 1)) == 0) {
> +} else if (Size >= 2 && (ConfigOffset & 1) == 0) {
>Width = EfiPciIoWidthUint16;
>Count = 1;
>  } else {

After retesting the series, I pushed it as commit range
4b8552d794e7..5685a243b6f8.

I've also filed 
about upstreaming the libraries from the first three patches to a more
central package.

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

[edk2] [PATCH] MdeModulePkg/Library/BaseSerialPortLib16550: Ensure FIFO Polled Mode

[Leo Duran]

Put the UART in FIFO Polled Mode by clearing IER after setting FCR.
Also, add comments to show DLAB state for registers 0 and 1.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran 
Cc: Star Zeng 
CC: Eric Dong 
---
 .../BaseSerialPortLib16550/BaseSerialPortLib16550.c  | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git 
a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c 
b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
index 0ccac96..6532c4d 100644
--- a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
+++ b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
@@ -3,6 +3,8 @@
 
   (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
   Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2018, AMD Incorporated. 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
@@ -30,10 +32,11 @@
 //
 // 16550 UART register offsets and bitfields
 //
-#define R_UART_RXBUF  0
-#define R_UART_TXBUF  0
-#define R_UART_BAUD_LOW   0
-#define R_UART_BAUD_HIGH  1
+#define R_UART_RXBUF  0   // LCR_DLAB = 0
+#define R_UART_TXBUF  0   // LCR_DLAB = 0
+#define R_UART_BAUD_LOW   0   // LCR_DLAB = 1
+#define R_UART_BAUD_HIGH  1   // LCR_DLAB = 1
+#define R_UART_IER1   // LCR_DLAB = 0
 #define R_UART_FCR2
 #define   B_UART_FCR_FIFOEBIT0
 #define   B_UART_FCR_FIFO64   BIT5
@@ -554,6 +557,11 @@ SerialPortInitialize (
   SerialPortWriteRegister (SerialRegisterBase, R_UART_FCR, (UINT8)(PcdGet8 
(PcdSerialFifoControl) & (B_UART_FCR_FIFOE | B_UART_FCR_FIFO64)));
 
   //
+  // Set FIFO Polled Mode by clearing IER after setting FCR
+  //
+  SerialPortWriteRegister (SerialRegisterBase, R_UART_IER, 0x00);
+
+  //
   // Put Modem Control Register(MCR) into its reset state of 0x00.
   //  
   SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, 0x00);
-- 
2.7.4

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

[edk2] [PATCH] Set FIFO Polled Mode on 16550 UART

[Leo Duran]

This patch ensures the 16500 UART is initialiazed in FIFO Polled Mode.
For an example, please see:

http://www.ti.com/lit/ds/symlink/pc16550d.pdf

8.4.2 FIFO Polled Mode Operation
With FCR0=1 resetting IER0, IER1, IER2, IER3 or all to zero puts the UART
in the FIFO Polled Mode of operation.

Leo Duran (1):
  MdeModulePkg/Library/BaseSerialPortLib16550: Ensure FIFO Polled Mode

 .../BaseSerialPortLib16550/BaseSerialPortLib16550.c  | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

-- 
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/7] OvmfPkg: introduce PciCapLib

[Laszlo Ersek]

On 05/24/18 16:41, Ard Biesheuvel wrote:
> On 24 May 2018 at 16:39, Laszlo Ersek  wrote:
>> On 05/24/18 09:53, Ard Biesheuvel wrote:

 +RETURN_STATUS
 +EFIAPI
 +PciCapGetInfo (
 +  IN  PCI_CAP  *Cap,
 +  OUT PCI_CAP_INFO *Info
 +  )
 +{
 +  PCI_CAP *InstanceZero;
 +
>>>
>>> Nit: add
>>>
>>> ASSERT (Info != NULL);
>>>
>>> here?
>>>
>>> I know it seems rather arbitrary to add it here and not anywhere else,
>>> but PciCapGetInfo() is part of the API, and dereferencing Info [which
>>> may be the result of e.g., a pool allocation] for writing is
>>> particularly bad.
>>
>>
>> I will add the ASSERT().
>>
>> (I hope I didn't miss any of your comments!)
>>
> 
> It's just a nit, feel free to ignore.
> 
> In any case,
> 
> Reviewed-by: Ard Biesheuvel 
> 

Thanks! I'll add the ASSERT.
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/7] OvmfPkg: introduce PciCapPciIoLib

[Laszlo Ersek]

On 05/24/18 16:54, Ard Biesheuvel wrote:
> On 24 May 2018 at 16:50, Laszlo Ersek  wrote:
>> On 05/24/18 10:13, Ard Biesheuvel wrote:
>>> On 23 May 2018 at 22:21, Laszlo Ersek  wrote:
 Add a library class, and a UEFI_DRIVER lib instance, that are layered on
 top of PciCapLib, and allow clients to plug an EFI_PCI_IO_PROTOCOL backend
 into PciCapLib, for config space access.

 (Side note:

>>>
>>> Again, please retain the below.
>>
>> Will do.
>>
 +STATIC
 +EFI_STATUS
 +ProtoDevTransferConfig (
 +  IN EFI_PCI_IO_PROTOCOL*PciIo,
 +  IN EFI_PCI_IO_PROTOCOL_CONFIG TransferFunction,
 +  IN UINT16 ConfigOffset,
 +  IN OUT UINT8  *Buffer,
 +  IN UINT16 Size
 +  )
 +{
 +  while (Size > 0) {
 +EFI_PCI_IO_PROTOCOL_WIDTH Width;
 +UINT16Count;
 +EFI_STATUSStatus;
 +UINT16Progress;
 +
 +//
 +// Pick the largest access size that is allowed by the remaining 
 transfer
 +// Size and by the alignment of ConfigOffset.
 +//
 +// When the largest access size is available, transfer as many bytes 
 as
 +// possible in one iteration of the loop. Otherwise, transfer only one
 +// unit, to improve the alignment.
 +//
 +if (Size >= BIT2 && (ConfigOffset & (BIT2 - 1)) == 0) {
>>>
>>> Ugh. Just use '4' or sizeof(UINT32).
>>>
 +  Width = EfiPciIoWidthUint32;
 +  Count = Size >> Width;
 +} else if (Size >= BIT1 && (ConfigOffset & (BIT1 - 1)) == 0) {
 +  Width = EfiPciIoWidthUint16;
 +  Count = 1;
 +} else {
 +  Width = EfiPciIoWidthUint8;
 +  Count = 1;
 +}
>>
>> I used "BITx" and "(BITx - 1)" for consistency, and because they seemed
>> to convey the idea well (namely, shifting down the alignment).
>>
>> I'm fine replacing "BIT2" with "4", but then I believe I should also
>> replace "(BIT2 - 1)" with "3". Similarly, replace "BIT1" with "2", and
>> "(BIT1 -1)" with 1.
>>
>> Do you prefer the current code or the decimal constants?
>>
> 
> IMHO, BITx is for bitmasks, not for numerical constants.
> 
> So yes, if you think (as do I) that sizeof(UINTnn) is too wordy, just
> use the plain numbers please.
> 

OK, I'll do that. Thanks!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 0/5] Abstract allocation of PEI accessible memory

[Kinney, Michael D]

Ard,

Thanks for the update.  This looks really close.

One comment is on the use of #ifdef MDE_CPU_X64
in a C file.

In general, the #ifdef for a specific type of CPU
should be limited to .h files and we should split
out CPU specific sources into their own source
files and use the INF file [Sources.] to
list the CPU specific source files.

Only other minor comment is that the summary still
mentions adding the new API to the UefiLib.

Mike


> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Thursday, May 24, 2018 2:10 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel ; Laszlo
> Ersek ; Leif Lindholm
> ; Kinney, Michael D
> ; Gao, Liming
> ; Zeng, Star
> ; Dong, Eric ;
> Bi, Dandan 
> Subject: [PATCH v2 0/5] Abstract allocation of PEI
> accessible memory
> 
> At the moment, FirmwarePerformanceTableDataDxe or
> DxeCorePerformanceLib
> are unusable on systems such as AMD Seattle, because
> they unconditionally
> attempt to allocate memory below 4 GB, and ASSERT() if
> this fails. On AMD
> Seattle, no 32-bit addressable DRAM exists, and so the
> driver will always
> assert, and crash a running DEBUG build.
> 
> The reason for this is that some platforms (i.e., X64
> builds consisting of
> a 32-bit PEI stage and 64-bit remaining stages) require
> these allocations
> to be below 4 GB, and for some reason, open coding this
> practice throughout
> the code without regard for other architectures has been
> regarded as an
> acceptable approach.
> 
> Instead, I would like to propose the approach
> implemented in this series:
> - create an abstracted EfiAllocatePeiAccessiblePages()
> routine in UefiLib
>   that simply allocates pages from any region on all
> architectures except
>   X64, and allocate below 4 GB for X64
> - update the various call sites with calls to this new
> function.
> 
> The above is implemented in patches #3 - #6. Patches #1
> and #2 are fixes
> for issues that I observed in ArmVirtPkg and OvmfPkg
> while working on
> these patches.
> 
> Code can be found here:
> https://github.com/ardbiesheuvel/edk2/tree/allocate-pei-
> v2
> 
> Changes since v1:
> - add Laszlo's ack to #1 - #2
> - move EfiAllocatePeiPages() to DxeServicesLib and drop
> Efi prefix
> - update implementation to take EfiFreeMemoryTop into
> account when built
>   for X64
> 
> Cc: Laszlo Ersek 
> Cc: Leif Lindholm 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Dandan Bi 
> 
> Ard Biesheuvel (5):
>   OvmfPkg/PlatformBootManagerLib: add missing report
> status code call
>   ArmVirtPkg/PlatformBootManagerLib: add missing report
> status code call
>   MdePkg/DxeServicesLib: introduce
> AllocatePeiAccessiblePages routine
>   MdeModulePkg/DxeCorePerformanceLib: use
> AllocatePeiAccessiblePages
>   MdeModulePkg/FirmwarePerformanceDataTableDxe: use
> AllocatePeiAccessiblePages
> 
>  .../PlatformBootManagerLib.inf|  1 +
>  .../PlatformBootManagerLib/QemuKernel.c   |  4 ++
>  .../DxeCorePerformanceLib.c   | 45 ++--
> ---
>  .../FirmwarePerformanceDxe.c  | 51 +++-
> -
>  .../FirmwarePerformanceDxe.inf|  1 +
>  MdePkg/Include/Library/DxeServicesLib.h   | 23
> +++-
>  .../Library/DxeServicesLib/DxeServicesLib.c   | 55
> +++
>  .../Library/DxeServicesLib/DxeServicesLib.inf |  3 +
>  .../PlatformBootManagerLib.inf|  1 +
>  .../PlatformBootManagerLib/QemuKernel.c   |  4 ++
>  10 files changed, 104 insertions(+), 84 deletions(-)
> 
> --
> 2.17.0

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

Re: [edk2] [PATCH v2 2/7] OvmfPkg: introduce PciCapPciSegmentLib

[Ard Biesheuvel]

On 24 May 2018 at 16:43, Laszlo Ersek  wrote:
> On 05/24/18 10:08, Ard Biesheuvel wrote:
>> On 23 May 2018 at 22:21, Laszlo Ersek  wrote:
>>> Add a library class, and a BASE lib instance, that are layered on top of
>>> PciCapLib, and allow clients to plug a PciSegmentLib backend into
>>> PciCapLib, for config space access.
>>>
>>> (Side note:
>>>
>>
>> For the record: I am fine with keeping this side note in the commit log.
>
> Appreciated! :)
>
> (I've found no other comments from you on this patch, so I guess you'll
> state something general towards the end.)
>

Ah yes

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

Re: [edk2] [PATCH v2 7/7] OvmfPkg/Virtio10Dxe: convert to PciCapLib

[Laszlo Ersek]

On 05/24/18 10:16, Ard Biesheuvel wrote:
> On 23 May 2018 at 22:21, Laszlo Ersek  wrote:
>> Replace the manual capability list parsing in OvmfPkg/Virtio10Dxe with
>> PciCapLib and PciCapPciIoLib API calls.
>>
>> The VIRTIO_PCI_CAP_LINK structure type is now superfluous. (Well, it
>> always has been; we should have used EFI_PCI_CAPABILITY_HDR.)
>>
>> Also, EFI_PCI_CAPABILITY_VENDOR_HDR is now included at the front of
>> VIRTIO_PCI_CAP. No driver other than Virtio10Dxe relies on VIRTIO_PCI_CAP.
>>
>> Cc: Ard Biesheuvel 
>> Cc: Jordan Justen 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek 
> 
> Reviewed-by: Ard Biesheuvel 

Thanks for the super quick review!

Can you please clarify your review of 2/7? You agreed with the commit
msg (thanks!) but didn't comment on the code in either direction.

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

Re: [edk2] [PATCH v2 3/7] OvmfPkg: introduce PciCapPciIoLib

[Ard Biesheuvel]

On 24 May 2018 at 16:54, Ard Biesheuvel  wrote:
> On 24 May 2018 at 16:50, Laszlo Ersek  wrote:
>> On 05/24/18 10:13, Ard Biesheuvel wrote:
>>> On 23 May 2018 at 22:21, Laszlo Ersek  wrote:
 Add a library class, and a UEFI_DRIVER lib instance, that are layered on
 top of PciCapLib, and allow clients to plug an EFI_PCI_IO_PROTOCOL backend
 into PciCapLib, for config space access.

 (Side note:

>>>
>>> Again, please retain the below.
>>
>> Will do.
>>
 +STATIC
 +EFI_STATUS
 +ProtoDevTransferConfig (
 +  IN EFI_PCI_IO_PROTOCOL*PciIo,
 +  IN EFI_PCI_IO_PROTOCOL_CONFIG TransferFunction,
 +  IN UINT16 ConfigOffset,
 +  IN OUT UINT8  *Buffer,
 +  IN UINT16 Size
 +  )
 +{
 +  while (Size > 0) {
 +EFI_PCI_IO_PROTOCOL_WIDTH Width;
 +UINT16Count;
 +EFI_STATUSStatus;
 +UINT16Progress;
 +
 +//
 +// Pick the largest access size that is allowed by the remaining 
 transfer
 +// Size and by the alignment of ConfigOffset.
 +//
 +// When the largest access size is available, transfer as many bytes 
 as
 +// possible in one iteration of the loop. Otherwise, transfer only one
 +// unit, to improve the alignment.
 +//
 +if (Size >= BIT2 && (ConfigOffset & (BIT2 - 1)) == 0) {
>>>
>>> Ugh. Just use '4' or sizeof(UINT32).
>>>
 +  Width = EfiPciIoWidthUint32;
 +  Count = Size >> Width;
 +} else if (Size >= BIT1 && (ConfigOffset & (BIT1 - 1)) == 0) {
 +  Width = EfiPciIoWidthUint16;
 +  Count = 1;
 +} else {
 +  Width = EfiPciIoWidthUint8;
 +  Count = 1;
 +}
>>
>> I used "BITx" and "(BITx - 1)" for consistency, and because they seemed
>> to convey the idea well (namely, shifting down the alignment).
>>
>> I'm fine replacing "BIT2" with "4", but then I believe I should also
>> replace "(BIT2 - 1)" with "3". Similarly, replace "BIT1" with "2", and
>> "(BIT1 -1)" with 1.
>>
>> Do you prefer the current code or the decimal constants?
>>
>
> IMHO, BITx is for bitmasks, not for numerical constants.
>
> So yes, if you think (as do I) that sizeof(UINTnn) is too wordy, just
> use the plain numbers please.


With that,

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

Re: [edk2] [PATCH v2 3/7] OvmfPkg: introduce PciCapPciIoLib

[Ard Biesheuvel]

On 24 May 2018 at 16:50, Laszlo Ersek  wrote:
> On 05/24/18 10:13, Ard Biesheuvel wrote:
>> On 23 May 2018 at 22:21, Laszlo Ersek  wrote:
>>> Add a library class, and a UEFI_DRIVER lib instance, that are layered on
>>> top of PciCapLib, and allow clients to plug an EFI_PCI_IO_PROTOCOL backend
>>> into PciCapLib, for config space access.
>>>
>>> (Side note:
>>>
>>
>> Again, please retain the below.
>
> Will do.
>
>>> +STATIC
>>> +EFI_STATUS
>>> +ProtoDevTransferConfig (
>>> +  IN EFI_PCI_IO_PROTOCOL*PciIo,
>>> +  IN EFI_PCI_IO_PROTOCOL_CONFIG TransferFunction,
>>> +  IN UINT16 ConfigOffset,
>>> +  IN OUT UINT8  *Buffer,
>>> +  IN UINT16 Size
>>> +  )
>>> +{
>>> +  while (Size > 0) {
>>> +EFI_PCI_IO_PROTOCOL_WIDTH Width;
>>> +UINT16Count;
>>> +EFI_STATUSStatus;
>>> +UINT16Progress;
>>> +
>>> +//
>>> +// Pick the largest access size that is allowed by the remaining 
>>> transfer
>>> +// Size and by the alignment of ConfigOffset.
>>> +//
>>> +// When the largest access size is available, transfer as many bytes as
>>> +// possible in one iteration of the loop. Otherwise, transfer only one
>>> +// unit, to improve the alignment.
>>> +//
>>> +if (Size >= BIT2 && (ConfigOffset & (BIT2 - 1)) == 0) {
>>
>> Ugh. Just use '4' or sizeof(UINT32).
>>
>>> +  Width = EfiPciIoWidthUint32;
>>> +  Count = Size >> Width;
>>> +} else if (Size >= BIT1 && (ConfigOffset & (BIT1 - 1)) == 0) {
>>> +  Width = EfiPciIoWidthUint16;
>>> +  Count = 1;
>>> +} else {
>>> +  Width = EfiPciIoWidthUint8;
>>> +  Count = 1;
>>> +}
>
> I used "BITx" and "(BITx - 1)" for consistency, and because they seemed
> to convey the idea well (namely, shifting down the alignment).
>
> I'm fine replacing "BIT2" with "4", but then I believe I should also
> replace "(BIT2 - 1)" with "3". Similarly, replace "BIT1" with "2", and
> "(BIT1 -1)" with 1.
>
> Do you prefer the current code or the decimal constants?
>

IMHO, BITx is for bitmasks, not for numerical constants.

So yes, if you think (as do I) that sizeof(UINTnn) is too wordy, just
use the plain numbers please.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/7] OvmfPkg: introduce PciCapPciIoLib

[Laszlo Ersek]

On 05/24/18 10:13, Ard Biesheuvel wrote:
> On 23 May 2018 at 22:21, Laszlo Ersek  wrote:
>> Add a library class, and a UEFI_DRIVER lib instance, that are layered on
>> top of PciCapLib, and allow clients to plug an EFI_PCI_IO_PROTOCOL backend
>> into PciCapLib, for config space access.
>>
>> (Side note:
>>
> 
> Again, please retain the below.

Will do.

>> +STATIC
>> +EFI_STATUS
>> +ProtoDevTransferConfig (
>> +  IN EFI_PCI_IO_PROTOCOL*PciIo,
>> +  IN EFI_PCI_IO_PROTOCOL_CONFIG TransferFunction,
>> +  IN UINT16 ConfigOffset,
>> +  IN OUT UINT8  *Buffer,
>> +  IN UINT16 Size
>> +  )
>> +{
>> +  while (Size > 0) {
>> +EFI_PCI_IO_PROTOCOL_WIDTH Width;
>> +UINT16Count;
>> +EFI_STATUSStatus;
>> +UINT16Progress;
>> +
>> +//
>> +// Pick the largest access size that is allowed by the remaining 
>> transfer
>> +// Size and by the alignment of ConfigOffset.
>> +//
>> +// When the largest access size is available, transfer as many bytes as
>> +// possible in one iteration of the loop. Otherwise, transfer only one
>> +// unit, to improve the alignment.
>> +//
>> +if (Size >= BIT2 && (ConfigOffset & (BIT2 - 1)) == 0) {
> 
> Ugh. Just use '4' or sizeof(UINT32).
> 
>> +  Width = EfiPciIoWidthUint32;
>> +  Count = Size >> Width;
>> +} else if (Size >= BIT1 && (ConfigOffset & (BIT1 - 1)) == 0) {
>> +  Width = EfiPciIoWidthUint16;
>> +  Count = 1;
>> +} else {
>> +  Width = EfiPciIoWidthUint8;
>> +  Count = 1;
>> +}

I used "BITx" and "(BITx - 1)" for consistency, and because they seemed
to convey the idea well (namely, shifting down the alignment).

I'm fine replacing "BIT2" with "4", but then I believe I should also
replace "(BIT2 - 1)" with "3". Similarly, replace "BIT1" with "2", and
"(BIT1 -1)" with 1.

Do you prefer the current code or the decimal constants?

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

Re: [edk2] [PATCH v2 2/7] OvmfPkg: introduce PciCapPciSegmentLib

[Laszlo Ersek]

On 05/24/18 10:08, Ard Biesheuvel wrote:
> On 23 May 2018 at 22:21, Laszlo Ersek  wrote:
>> Add a library class, and a BASE lib instance, that are layered on top of
>> PciCapLib, and allow clients to plug a PciSegmentLib backend into
>> PciCapLib, for config space access.
>>
>> (Side note:
>>
> 
> For the record: I am fine with keeping this side note in the commit log.

Appreciated! :)

(I've found no other comments from you on this patch, so I guess you'll
state something general towards the end.)

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

Re: [edk2] [PATCH v2 1/7] OvmfPkg: introduce PciCapLib

[Ard Biesheuvel]

On 24 May 2018 at 16:39, Laszlo Ersek  wrote:
> On 05/24/18 09:53, Ard Biesheuvel wrote:
>
>>> +STATIC
>>> +VOID
>>> +EFIAPI
>>> +DebugDumpPciCapList (
>>> +  IN PCI_CAP_LIST *CapList
>>> +  )
>>> +{
>>> +  DEBUG_CODE_BEGIN ();
>>> +  ORDERED_COLLECTION_ENTRY *PciCapEntry;
>>> +
>>> +  for (PciCapEntry = OrderedCollectionMin (CapList->Capabilities);
>>> +   PciCapEntry != NULL;
>>> +   PciCapEntry = OrderedCollectionNext (PciCapEntry)) {
>>> +PCI_CAP   *PciCap;
>>> +RETURN_STATUS Status;
>>> +PCI_CAP_INFO  Info;
>>> +
>>
>> Move these out of the loop?
>
>>> +STATIC
>>> +VOID
>>> +EmptyAndUninitPciCapCollection (
>>> +  IN OUT ORDERED_COLLECTION *PciCapCollection,
>>> +  IN BOOLEANFreePciCap
>>> +  )
>>> +{
>>> +  ORDERED_COLLECTION_ENTRY *PciCapEntry;
>>> +  ORDERED_COLLECTION_ENTRY *NextEntry;
>>> +
>>> +  for (PciCapEntry = OrderedCollectionMin (PciCapCollection);
>>> +   PciCapEntry != NULL;
>>> +   PciCapEntry = NextEntry) {
>>> +PCI_CAP *PciCap;
>>> +
>>
>> and this one
>
>>> +RETURN_STATUS
>>> +EFIAPI
>>> +PciCapListInit (
>>> +  IN  PCI_CAP_DEV  *PciDevice,
>>> +  OUT PCI_CAP_LIST **CapList
>>> +  )
>>> +{
>>> +  PCI_CAP_LIST *OutCapList;
>>> +  RETURN_STATUSStatus;
>>> +  ORDERED_COLLECTION   *CapHdrOffsets;
>>> +  UINT16   PciStatusReg;
>>> +  BOOLEAN  DeviceIsExpress;
>>> +  ORDERED_COLLECTION_ENTRY *OffsetEntry;
>>> +
>>> +  //
>>> +  // Allocate the output structure.
>>> +  //
>>> +  OutCapList = AllocatePool (sizeof *OutCapList);
>>> +  if (OutCapList == NULL) {
>>> +return RETURN_OUT_OF_RESOURCES;
>>> +  }
>>> +  //
>>> +  // The OutCapList->Capabilities collection owns the PCI_CAP structures 
>>> and
>>> +  // orders them based on PCI_CAP.Key.
>>> +  //
>>> +  OutCapList->Capabilities = OrderedCollectionInit (ComparePciCap,
>>> +   ComparePciCapKey);
>>> +  if (OutCapList->Capabilities == NULL) {
>>> +Status = RETURN_OUT_OF_RESOURCES;
>>> +goto FreeOutCapList;
>>> +  }
>>> +
>>> +  //
>>> +  // The (temporary) CapHdrOffsets collection only references PCI_CAP
>>> +  // structures, and orders them based on PCI_CAP.Offset.
>>> +  //
>>> +  CapHdrOffsets = OrderedCollectionInit (ComparePciCapOffset,
>>> +ComparePciCapOffsetKey);
>>> +  if (CapHdrOffsets == NULL) {
>>> +Status = RETURN_OUT_OF_RESOURCES;
>>> +goto FreeCapabilities;
>>> +  }
>>> +
>>> +  //
>>> +  // Whether the device is PCI Express depends on the normal capability 
>>> with
>>> +  // identifier EFI_PCI_CAPABILITY_ID_PCIEXP.
>>> +  //
>>> +  DeviceIsExpress = FALSE;
>>> +
>>> +  //
>>> +  // Check whether a normal capabilities list is present. If there's none,
>>> +  // that's not an error; we'll just return OutCapList->Capabilities empty.
>>> +  //
>>> +  Status = PciDevice->ReadConfig (PciDevice, PCI_PRIMARY_STATUS_OFFSET,
>>> +, sizeof PciStatusReg);
>>> +  if (RETURN_ERROR (Status)) {
>>> +goto FreeCapHdrOffsets;
>>> +  }
>>> +  if ((PciStatusReg & EFI_PCI_STATUS_CAPABILITY) != 0) {
>>> +UINT8 NormalCapHdrOffset;
>>> +
>>
>> and this one
>>
>>> +//
>>> +// Fetch the start offset of the normal capabilities list.
>>> +//
>>> +Status = PciDevice->ReadConfig (PciDevice, 
>>> PCI_CAPBILITY_POINTER_OFFSET,
>>> +  , sizeof NormalCapHdrOffset);
>>> +if (RETURN_ERROR (Status)) {
>>> +  goto FreeCapHdrOffsets;
>>> +}
>>> +
>>> +//
>>> +// Traverse the normal capabilities list.
>>> +//
>>> +NormalCapHdrOffset &= 0xFC;
>>> +while (NormalCapHdrOffset > 0) {
>>> +  EFI_PCI_CAPABILITY_HDR NormalCapHdr;
>>> +
>>
>> and this one.
>>
>> Perhaps I am missing something? After four instances, it seems
>> deliberate rather than accidental :-)
>
> It's totally deliberate. C89 supports scoping auto variables more
> tightly than at the function scope, and I liberally use that feature --
> scoping local variables as tightly as possible helps humans reason about
> data flow. This is characteristic of all C code I write.
>
> The coding style writes,
>
> https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/5_source_files/54_code_file_structure.html#54-code-file-structure
>
> "Data declarations may follow the opening brace of a compound statement,
> regardless of nesting depth, and before any code generating statements
> have been entered. Other than at the outermost block of a function body,
> this type of declaration is strongly discouraged."
>
> It's discouraged, but not outright forbidden, and personally I find that
> lumping all local variables together at the top decreases readability
> and harms my ability to reason about data flow.
>
> If you'd really like me to move these variable definitions to the tops
> of their respective functions, knowing my motive, I can do it, of
> course. Do you want me to? :)
>

Please don't. 

Re: [edk2] [PATCH v2 1/7] OvmfPkg: introduce PciCapLib

[Laszlo Ersek]

On 05/24/18 09:53, Ard Biesheuvel wrote:

>> +STATIC
>> +VOID
>> +EFIAPI
>> +DebugDumpPciCapList (
>> +  IN PCI_CAP_LIST *CapList
>> +  )
>> +{
>> +  DEBUG_CODE_BEGIN ();
>> +  ORDERED_COLLECTION_ENTRY *PciCapEntry;
>> +
>> +  for (PciCapEntry = OrderedCollectionMin (CapList->Capabilities);
>> +   PciCapEntry != NULL;
>> +   PciCapEntry = OrderedCollectionNext (PciCapEntry)) {
>> +PCI_CAP   *PciCap;
>> +RETURN_STATUS Status;
>> +PCI_CAP_INFO  Info;
>> +
> 
> Move these out of the loop?

>> +STATIC
>> +VOID
>> +EmptyAndUninitPciCapCollection (
>> +  IN OUT ORDERED_COLLECTION *PciCapCollection,
>> +  IN BOOLEANFreePciCap
>> +  )
>> +{
>> +  ORDERED_COLLECTION_ENTRY *PciCapEntry;
>> +  ORDERED_COLLECTION_ENTRY *NextEntry;
>> +
>> +  for (PciCapEntry = OrderedCollectionMin (PciCapCollection);
>> +   PciCapEntry != NULL;
>> +   PciCapEntry = NextEntry) {
>> +PCI_CAP *PciCap;
>> +
> 
> and this one

>> +RETURN_STATUS
>> +EFIAPI
>> +PciCapListInit (
>> +  IN  PCI_CAP_DEV  *PciDevice,
>> +  OUT PCI_CAP_LIST **CapList
>> +  )
>> +{
>> +  PCI_CAP_LIST *OutCapList;
>> +  RETURN_STATUSStatus;
>> +  ORDERED_COLLECTION   *CapHdrOffsets;
>> +  UINT16   PciStatusReg;
>> +  BOOLEAN  DeviceIsExpress;
>> +  ORDERED_COLLECTION_ENTRY *OffsetEntry;
>> +
>> +  //
>> +  // Allocate the output structure.
>> +  //
>> +  OutCapList = AllocatePool (sizeof *OutCapList);
>> +  if (OutCapList == NULL) {
>> +return RETURN_OUT_OF_RESOURCES;
>> +  }
>> +  //
>> +  // The OutCapList->Capabilities collection owns the PCI_CAP structures and
>> +  // orders them based on PCI_CAP.Key.
>> +  //
>> +  OutCapList->Capabilities = OrderedCollectionInit (ComparePciCap,
>> +   ComparePciCapKey);
>> +  if (OutCapList->Capabilities == NULL) {
>> +Status = RETURN_OUT_OF_RESOURCES;
>> +goto FreeOutCapList;
>> +  }
>> +
>> +  //
>> +  // The (temporary) CapHdrOffsets collection only references PCI_CAP
>> +  // structures, and orders them based on PCI_CAP.Offset.
>> +  //
>> +  CapHdrOffsets = OrderedCollectionInit (ComparePciCapOffset,
>> +ComparePciCapOffsetKey);
>> +  if (CapHdrOffsets == NULL) {
>> +Status = RETURN_OUT_OF_RESOURCES;
>> +goto FreeCapabilities;
>> +  }
>> +
>> +  //
>> +  // Whether the device is PCI Express depends on the normal capability with
>> +  // identifier EFI_PCI_CAPABILITY_ID_PCIEXP.
>> +  //
>> +  DeviceIsExpress = FALSE;
>> +
>> +  //
>> +  // Check whether a normal capabilities list is present. If there's none,
>> +  // that's not an error; we'll just return OutCapList->Capabilities empty.
>> +  //
>> +  Status = PciDevice->ReadConfig (PciDevice, PCI_PRIMARY_STATUS_OFFSET,
>> +, sizeof PciStatusReg);
>> +  if (RETURN_ERROR (Status)) {
>> +goto FreeCapHdrOffsets;
>> +  }
>> +  if ((PciStatusReg & EFI_PCI_STATUS_CAPABILITY) != 0) {
>> +UINT8 NormalCapHdrOffset;
>> +
> 
> and this one
> 
>> +//
>> +// Fetch the start offset of the normal capabilities list.
>> +//
>> +Status = PciDevice->ReadConfig (PciDevice, PCI_CAPBILITY_POINTER_OFFSET,
>> +  , sizeof NormalCapHdrOffset);
>> +if (RETURN_ERROR (Status)) {
>> +  goto FreeCapHdrOffsets;
>> +}
>> +
>> +//
>> +// Traverse the normal capabilities list.
>> +//
>> +NormalCapHdrOffset &= 0xFC;
>> +while (NormalCapHdrOffset > 0) {
>> +  EFI_PCI_CAPABILITY_HDR NormalCapHdr;
>> +
> 
> and this one.
> 
> Perhaps I am missing something? After four instances, it seems
> deliberate rather than accidental :-)

It's totally deliberate. C89 supports scoping auto variables more
tightly than at the function scope, and I liberally use that feature --
scoping local variables as tightly as possible helps humans reason about
data flow. This is characteristic of all C code I write.

The coding style writes,

https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/5_source_files/54_code_file_structure.html#54-code-file-structure

"Data declarations may follow the opening brace of a compound statement,
regardless of nesting depth, and before any code generating statements
have been entered. Other than at the outermost block of a function body,
this type of declaration is strongly discouraged."

It's discouraged, but not outright forbidden, and personally I find that
lumping all local variables together at the top decreases readability
and harms my ability to reason about data flow.

If you'd really like me to move these variable definitions to the tops
of their respective functions, knowing my motive, I can do it, of
course. Do you want me to? :)

>> +RETURN_STATUS
>> +EFIAPI
>> +PciCapGetInfo (
>> +  IN  PCI_CAP  *Cap,
>> +  OUT PCI_CAP_INFO *Info
>> +  )
>> +{
>> +  PCI_CAP *InstanceZero;
>> +
> 
> Nit: add
> 
> ASSERT (Info != NULL);
> 
> here?
> 
> I know it seems rather arbitrary to add it 

[edk2] [PATCH v1 1/1] BaseTools: Add missing content to EOT tool.

[Jaben Carsey]

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/Eot/Eot.py   | 1444 +++-
 BaseTools/Source/Python/Eot/EotGlobalData.py |   17 +
 2 files changed, 1460 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/Eot/Eot.py 
b/BaseTools/Source/Python/Eot/Eot.py
index fcde8fd3e22f..786868d55d99 100644
--- a/BaseTools/Source/Python/Eot/Eot.py
+++ b/BaseTools/Source/Python/Eot/Eot.py
@@ -20,7 +20,7 @@ import EotGlobalData
 from optparse import OptionParser
 from Common.String import NormPath
 from Common import BuildToolError
-from Common.Misc import GuidStructureStringToGuidString
+from Common.Misc import GuidStructureStringToGuidString, sdict
 from InfParserLite import *
 import c
 import Database
@@ -29,6 +29,1446 @@ from Report import Report
 from Common.BuildVersion import gBUILD_VERSION
 from Parser import ConvertGuid
 from Common.LongFilePathSupport import OpenLongFilePath as open
+import struct
+import uuid
+import copy
+import codecs
+
+gGuidStringFormat = "%08X-%04X-%04X-%02X%02X-%02X%02X%02X%02X%02X%02X"
+gPeiAprioriFileNameGuid = '1b45cc0a-156a-428a-af62-49864da0e6e6'
+gAprioriGuid = 'fc510ee7-ffdc-11d4-bd41-0080c73c8881'
+gIndention = -4
+
+class Image(array):
+_HEADER_ = struct.Struct("")
+_HEADER_SIZE_ = _HEADER_.size
+
+def __new__(cls, *args, **kwargs):
+return array.__new__(cls, 'B')
+
+def __init__(self, ID=None):
+if ID is None:
+self._ID_ = str(uuid.uuid1()).upper()
+else:
+self._ID_ = ID
+self._BUF_ = None
+self._LEN_ = None
+self._OFF_ = None
+
+self._SubImages = sdict() # {offset: Image()}
+
+array.__init__(self, 'B')
+
+def __repr__(self):
+return self._ID_
+
+def __len__(self):
+Len = array.__len__(self)
+for Offset in self._SubImages:
+Len += len(self._SubImages[Offset])
+return Len
+
+def _Unpack(self):
+self.extend(self._BUF_[self._OFF_ : self._OFF_ + self._LEN_])
+return len(self)
+
+def _Pack(self, PadByte=0xFF):
+raise NotImplementedError
+
+def frombuffer(self, Buffer, Offset=0, Size=None):
+self._BUF_ = Buffer
+self._OFF_ = Offset
+# we may need the Size information in advance if it's given
+self._LEN_ = Size
+self._LEN_ = self._Unpack()
+
+def empty(self):
+del self[0:]
+
+def GetField(self, FieldStruct, Offset=0):
+return FieldStruct.unpack_from(self, Offset)
+
+def SetField(self, FieldStruct, Offset, *args):
+# check if there's enough space
+Size = FieldStruct.size
+if Size > len(self):
+self.extend([0] * (Size - len(self)))
+FieldStruct.pack_into(self, Offset, *args)
+
+def _SetData(self, Data):
+if len(self) < self._HEADER_SIZE_:
+self.extend([0] * (self._HEADER_SIZE_ - len(self)))
+else:
+del self[self._HEADER_SIZE_:]
+self.extend(Data)
+
+def _GetData(self):
+if len(self) > self._HEADER_SIZE_:
+return self[self._HEADER_SIZE_:]
+return None
+
+Data = property(_GetData, _SetData)
+
+## CompressedImage() class
+#
+#  A class for Compressed Image
+#
+class CompressedImage(Image):
+# UncompressedLength = 4-byte
+# CompressionType = 1-byte
+_HEADER_ = struct.Struct("1I 1B")
+_HEADER_SIZE_ = _HEADER_.size
+
+_ORIG_SIZE_ = struct.Struct("1I")
+_CMPRS_TYPE_= struct.Struct("4x 1B")
+
+def __init__(m, CompressedData=None, CompressionType=None, 
UncompressedLength=None):
+Image.__init__(m)
+if UncompressedLength is not None:
+m.UncompressedLength = UncompressedLength
+if CompressionType is not None:
+m.CompressionType = CompressionType
+if CompressedData is not None:
+m.Data = CompressedData
+
+def __str__(m):
+global gIndention
+S = "algorithm=%s uncompressed=%x" % (m.CompressionType, 
m.UncompressedLength)
+for Sec in m.Sections:
+S += '\n' + str(Sec)
+
+return S
+
+def _SetOriginalSize(m, Size):
+m.SetField(m._ORIG_SIZE_, 0, Size)
+
+def _GetOriginalSize(m):
+return m.GetField(m._ORIG_SIZE_)[0]
+
+def _SetCompressionType(m, Type):
+m.SetField(m._CMPRS_TYPE_, 0, Type)
+
+def _GetCompressionType(m):
+return m.GetField(m._CMPRS_TYPE_)[0]
+
+def _GetSections(m):
+try:
+import EfiCompressor
+TmpData = EfiCompressor.FrameworkDecompress(
+m[m._HEADER_SIZE_:],
+len(m) - m._HEADER_SIZE_
+)
+DecData = array('B')
+

Re: [edk2] [RFC] Formalize source files to follow DOS format

[Carsey, Jaben]

We could do something like we do for compiler flags... append or overwrite 
depending on syntax.

> -Original Message-
> From: Gao, Liming
> Sent: Thursday, May 24, 2018 1:35 AM
> To: Kinney, Michael D ; Carsey, Jaben
> 
> Cc: edk2-devel@lists.01.org
> Subject: RE: [edk2] [RFC] Formalize source files to follow DOS format
> Importance: High
> 
> Mike:
>   I agree your comments. On default file set, this script can have the default
> ones. User can specify more set to append the default ones instead of
> override the default ones.
> 
> Thanks
> Liming
> >-Original Message-
> >From: Kinney, Michael D
> >Sent: Tuesday, May 22, 2018 6:59 AM
> >To: Carsey, Jaben ; Kinney, Michael D
> >
> >Cc: Gao, Liming ; edk2-devel@lists.01.org
> >Subject: RE: [edk2] [RFC] Formalize source files to follow DOS format
> >
> >Jaben,
> >
> >Yes.  With default behavior is default set and
> >specifying one or more extensions overrides the
> >default set.
> >
> >Mike
> >
> >> -Original Message-
> >> From: Carsey, Jaben
> >> Sent: Monday, May 21, 2018 3:43 PM
> >> To: Kinney, Michael D 
> >> Cc: Gao, Liming ; edk2-
> >> de...@lists.01.org
> >> Subject: Re: [edk2] [RFC] Formalize source files to
> >> follow DOS format
> >>
> >> Mike,
> >>
> >> Perhaps a default set of file extensions that can be
> >> overridden?
> >>
> >> -Jaben
> >>
> >>
> >> > On May 21, 2018, at 3:41 PM, Kinney, Michael D
> >>  wrote:
> >> >
> >> > Liming,
> >> >
> >> > We have a set of standard flags for tools that
> >> > should always be present.
> >> >
> >> > --help
> >> > -v
> >> > -q
> >> > --debug
> >> >
> >> > We should also always have the program name,
> >> > description, version, and copyright.
> >> >
> >> > Please see BaseTools/Scripts/BinToPcd.py as
> >> > an example.
> >> >
> >> > It might be useful to have a way to run this tool
> >> > on a single file when BaseTools/Scripts/PatchCheck.py
> >> > reports an issue.
> >> >
> >> > Do you think it would be good to have one option to
> >> > scan path for file extensions that are documented as
> >> > DOS line endings so the extensions do not have to be
> >> > entered?
> >> >
> >> > Mike
> >> >
> >> >
> >> >> -Original Message-
> >> >> From: edk2-devel [mailto:edk2-devel-
> >> >> boun...@lists.01.org] On Behalf Of Carsey, Jaben
> >> >> Sent: Monday, May 21, 2018 7:50 AM
> >> >> To: Gao, Liming ; edk2-
> >> >> de...@lists.01.org
> >> >> Subject: Re: [edk2] [RFC] Formalize source files to
> >> >> follow DOS format
> >> >>
> >> >> Liming,
> >> >>
> >> >> One Pep8 thing.
> >> >> Can you change to use the with statement for the file
> >> >> read/write?
> >> >>
> >> >> Other small thoughts.
> >> >> I think that FileList should be changed to a set as
> >> >> order is not important.
> >> >> Maybe wrapper the re.sub function with your own so
> >> all
> >> >> the .encode() are in one location?  As we move to
> >> python
> >> >> 3 we will have fewer changes to make.
> >> >>
> >> >>
> >> >>> -Original Message-
> >> >>> From: edk2-devel [mailto:edk2-devel-
> >> >> boun...@lists.01.org] On Behalf Of
> >> >>> Liming Gao
> >> >>> Sent: Sunday, May 20, 2018 9:52 PM
> >> >>> To: edk2-devel@lists.01.org
> >> >>> Subject: [edk2] [RFC] Formalize source files to
> >> follow
> >> >> DOS format
> >> >>>
> >> >>> FormatDosFiles.py is added to clean up dos source
> >> >> files. It bases on
> >> >>> the rules defined in EDKII C Coding Standards
> >> >> Specification.
> >> >>> 5.1.2 Do not use tab characters
> >> >>> 5.1.6 Only use CRLF (Carriage Return Line Feed) line
> >> >> endings.
> >> >>> 5.1.7 All files must end with CRLF
> >> >>> No trailing white space in one line. (To be added in
> >> >> spec)
> >> >>>
> >> >>> The source files in edk2 project with the below
> >> >> postfix are dos format.
> >> >>> .h .c .nasm .nasmb .asm .S .inf .dec .dsc .fdf .uni
> >> >> .asl .aslc .vfr .idf
> >> >>> .txt .bat .py
> >> >>>
> >> >>> The package maintainer can use this script to clean
> >> up
> >> >> all files in his
> >> >>> package. The prefer way is to create one patch per
> >> one
> >> >> package.
> >> >>>
> >> >>> Contributed-under: TianoCore Contribution Agreement
> >> >> 1.1
> >> >>> Signed-off-by: Liming Gao 
> >> >>> ---
> >> >>> BaseTools/Scripts/FormatDosFiles.py | 93
> >> >>> +
> >> >>> 1 file changed, 93 insertions(+)
> >> >>> create mode 100644
> >> >> BaseTools/Scripts/FormatDosFiles.py
> >> >>>
> >> >>> diff --git a/BaseTools/Scripts/FormatDosFiles.py
> >> >>> b/BaseTools/Scripts/FormatDosFiles.py
> >> >>> new file mode 100644
> >> >>> index 000..c3a5476
> >> >>> --- /dev/null
> >> >>> +++ b/BaseTools/Scripts/FormatDosFiles.py
> >> >>> @@ -0,0 +1,93 @@
> >> >>> +# @file FormatDosFiles.py
> >> >>> +# This 

Re: [edk2] [RFC] Formalize source files to follow DOS format

[Carsey, Jaben]

Follow pep8 for coding style.

The technical benefit is things like that If an exception occurs we still close 
the file.

> -Original Message-
> From: Gao, Liming
> Sent: Thursday, May 24, 2018 1:31 AM
> To: Carsey, Jaben ; edk2-devel@lists.01.org
> Subject: RE: [edk2] [RFC] Formalize source files to follow DOS format
> Importance: High
> 
> Jaben:
>   What difference of statement for file read/write?
> 
>   Besides, we use .encode() here to support python 3. After we move to
> python 3, this script is not changed.
> 
> Thanks
> Liming
> >-Original Message-
> >From: Carsey, Jaben
> >Sent: Monday, May 21, 2018 10:50 PM
> >To: Gao, Liming ; edk2-devel@lists.01.org
> >Subject: RE: [edk2] [RFC] Formalize source files to follow DOS format
> >
> >Liming,
> >
> >One Pep8 thing.
> >Can you change to use the with statement for the file read/write?
> >
> >Other small thoughts.
> >I think that FileList should be changed to a set as order is not important.
> >Maybe wrapper the re.sub function with your own so all the .encode() are
> in
> >one location?  As we move to python 3 we will have fewer changes to
> make.
> >
> >
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> >> Liming Gao
> >> Sent: Sunday, May 20, 2018 9:52 PM
> >> To: edk2-devel@lists.01.org
> >> Subject: [edk2] [RFC] Formalize source files to follow DOS format
> >>
> >> FormatDosFiles.py is added to clean up dos source files. It bases on
> >> the rules defined in EDKII C Coding Standards Specification.
> >> 5.1.2 Do not use tab characters
> >> 5.1.6 Only use CRLF (Carriage Return Line Feed) line endings.
> >> 5.1.7 All files must end with CRLF
> >> No trailing white space in one line. (To be added in spec)
> >>
> >> The source files in edk2 project with the below postfix are dos format.
> >> .h .c .nasm .nasmb .asm .S .inf .dec .dsc .fdf .uni .asl .aslc .vfr .idf
> >> .txt .bat .py
> >>
> >> The package maintainer can use this script to clean up all files in his
> >> package. The prefer way is to create one patch per one package.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Liming Gao 
> >> ---
> >>  BaseTools/Scripts/FormatDosFiles.py | 93
> >> +
> >>  1 file changed, 93 insertions(+)
> >>  create mode 100644 BaseTools/Scripts/FormatDosFiles.py
> >>
> >> diff --git a/BaseTools/Scripts/FormatDosFiles.py
> >> b/BaseTools/Scripts/FormatDosFiles.py
> >> new file mode 100644
> >> index 000..c3a5476
> >> --- /dev/null
> >> +++ b/BaseTools/Scripts/FormatDosFiles.py
> >> @@ -0,0 +1,93 @@
> >> +# @file FormatDosFiles.py
> >> +# This script format the source files to follow dos style.
> >> +# It supports Python2.x and Python3.x both.
> >> +#
> >> +#  Copyright (c) 2018, Intel Corporation. All rights reserved.
> >> +#
> >> +#  This program and the accompanying materials
> >> +#  are licensed and made available under the terms and conditions of the
> >> BSD License
> >> +#  which accompanies this distribution.  The full text of the license may
> be
> >> found at
> >> +#  http://opensource.org/licenses/bsd-license.php
> >> +#
> >> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS
> IS"
> >> BASIS,
> >> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> >> EXPRESS OR IMPLIED.
> >> +#
> >> +
> >> +#
> >> +# Import Modules
> >> +#
> >> +import argparse
> >> +import os
> >> +import os.path
> >> +import re
> >> +import sys
> >> +
> >> +"""
> >> +difference of string between python2 and python3:
> >> +
> >> +there is a large difference of string in python2 and python3.
> >> +
> >> +in python2,there are two type string,unicode string (unicode type) and
> 8-
> >bit
> >> string (str type).
> >> +  us = u"abcd",
> >> +  unicode string,which is internally stored as unicode code point.
> >> +  s = "abcd",s = b"abcd",s = r"abcd",
> >> +  all of them are 8-bit string,which is internally stored as bytes.
> >> +
> >> +in python3,a new type called bytes replace 8-bit string,and str type is
> >> regarded as unicode string.
> >> +  s = "abcd", s = u"abcd", s = r"abcd",
> >> +  all of them are str type,which is internally stored unicode code point.
> >> +  bs = b"abcd",
> >> +  bytes type,which is interally stored as bytes
> >> +
> >> +in python2 ,the both type string can be mixed use,but in python3 it could
> >> not,
> >> +which means the pattern and content in re match should be the same
> type
> >> in python3.
> >> +in function FormatFile,it read file in binary mode so that the content is
> >bytes
> >> type,so the pattern should also be bytes type.
> >> +As a result,I add encode() to make it compitable among python2 and
> >> python3.
> >> +
> >> +difference of encode,decode in python2 and python3:
> >> +the builtin function str.encode(encoding) and str.decode(encoding) are
> >> used for convert between 8-bit string and unicode 

Re: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

[Laszlo Ersek]

On 05/24/18 14:54, Ard Biesheuvel wrote:
> On 24 May 2018 at 14:49, Laszlo Ersek  wrote:
>> On 05/24/18 11:09, Ard Biesheuvel wrote:
>>> Replace the call to and implementation of the function
>>> FpdtAllocateReservedMemoryBelow4G() with a call to
>>> AllocatePeiAccessiblePages, which boils down to the same on X64,
>>> but does not crash non-X64 systems that lack memory below 4 GB.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Ard Biesheuvel 
>>> ---
>>> Note that the ZeroMem() call is dropped, but it is redundant anyway, given
>>> that the subsequent CopyMem() call supersedes it immediately.
>>
>> I'm not sure the ZeroMem() is redundant. The allocation size -- before 
>> rounding up to full pages -- is computed like this:
>>
>>   BootPerformanceDataSize = sizeof (BOOT_PERFORMANCE_TABLE) + 
>> mPerformanceLength + PcdGet32 (PcdExtFpdtBootRecordPadSize);
>>   if (SmmCommData != NULL && SmmBootRecordData != NULL) {
>> BootPerformanceDataSize += SmmBootRecordDataSize;
>>   }
>>
>> ZeroMem() covers all of the above, but the CopyMem() calls don't seem to 
>> cover the optional padding (from the PCD).
>>
>> I'm unsure if that matters, of course.
>>
> 
> You're quite right. I'm not sure how I missed that.
> 
> In any case, I can add back the ZeroMem () quite easily after the
> single invocation of AllocatePeiAccessiblePages() that I am adding in
> this file.

Thanks! With that:

Reviewed-by: Laszlo Ersek 

Laszlo

> 
>> The patch looks good to me otherwise.
>>
>> Thanks
>> Laszlo
>>
>>
>>>
>>>  MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 45 
>>> ++--
>>>  1 file changed, 4 insertions(+), 41 deletions(-)
>>>
>>> diff --git 
>>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c 
>>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>>> index 71d624fc9ce9..b1f09710b65c 100644
>>> --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>>> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>>> @@ -165,46 +165,6 @@ IsKnownID (
>>>}
>>>  }
>>>
>>> -/**
>>> -  Allocate EfiReservedMemoryType below 4G memory address.
>>> -
>>> -  This function allocates EfiReservedMemoryType below 4G memory address.
>>> -
>>> -  @param[in]  Size   Size of memory to allocate.
>>> -
>>> -  @return Allocated address for output.
>>> -
>>> -**/
>>> -VOID *
>>> -FpdtAllocateReservedMemoryBelow4G (
>>> -  IN UINTN   Size
>>> -  )
>>> -{
>>> -  UINTN Pages;
>>> -  EFI_PHYSICAL_ADDRESS  Address;
>>> -  EFI_STATUSStatus;
>>> -  VOID  *Buffer;
>>> -
>>> -  Buffer  = NULL;
>>> -  Pages   = EFI_SIZE_TO_PAGES (Size);
>>> -  Address = 0x;
>>> -
>>> -  Status = gBS->AllocatePages (
>>> -  AllocateMaxAddress,
>>> -  EfiReservedMemoryType,
>>> -  Pages,
>>> -  
>>> -  );
>>> -  ASSERT_EFI_ERROR (Status);
>>> -
>>> -  if (!EFI_ERROR (Status)) {
>>> -Buffer = (VOID *) (UINTN) Address;
>>> -ZeroMem (Buffer, Size);
>>> -  }
>>> -
>>> -  return Buffer;
>>> -}
>>> -
>>>  /**
>>>Allocate buffer for Boot Performance table.
>>>
>>> @@ -348,7 +308,10 @@ AllocateBootPerformanceTable (
>>>  //
>>>  // Fail to allocate at specified address, continue to allocate at any 
>>> address.
>>>  //
>>> -mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
>>> FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
>>> +mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
>>> AllocatePeiAccessiblePages (
>>> + 
>>> EfiReservedMemoryType,
>>> + 
>>> EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
>>> + );
>>>}
>>>DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance Table 
>>> address = 0x%x\n", mAcpiBootPerformanceTable));
>>>
>>>
>>

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

Re: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

[Ard Biesheuvel]

On 24 May 2018 at 14:49, Laszlo Ersek  wrote:
> On 05/24/18 11:09, Ard Biesheuvel wrote:
>> Replace the call to and implementation of the function
>> FpdtAllocateReservedMemoryBelow4G() with a call to
>> AllocatePeiAccessiblePages, which boils down to the same on X64,
>> but does not crash non-X64 systems that lack memory below 4 GB.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>> ---
>> Note that the ZeroMem() call is dropped, but it is redundant anyway, given
>> that the subsequent CopyMem() call supersedes it immediately.
>
> I'm not sure the ZeroMem() is redundant. The allocation size -- before 
> rounding up to full pages -- is computed like this:
>
>   BootPerformanceDataSize = sizeof (BOOT_PERFORMANCE_TABLE) + 
> mPerformanceLength + PcdGet32 (PcdExtFpdtBootRecordPadSize);
>   if (SmmCommData != NULL && SmmBootRecordData != NULL) {
> BootPerformanceDataSize += SmmBootRecordDataSize;
>   }
>
> ZeroMem() covers all of the above, but the CopyMem() calls don't seem to 
> cover the optional padding (from the PCD).
>
> I'm unsure if that matters, of course.
>

You're quite right. I'm not sure how I missed that.

In any case, I can add back the ZeroMem () quite easily after the
single invocation of AllocatePeiAccessiblePages() that I am adding in
this file.

> The patch looks good to me otherwise.
>
> Thanks
> Laszlo
>
>
>>
>>  MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 45 
>> ++--
>>  1 file changed, 4 insertions(+), 41 deletions(-)
>>
>> diff --git 
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c 
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> index 71d624fc9ce9..b1f09710b65c 100644
>> --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> @@ -165,46 +165,6 @@ IsKnownID (
>>}
>>  }
>>
>> -/**
>> -  Allocate EfiReservedMemoryType below 4G memory address.
>> -
>> -  This function allocates EfiReservedMemoryType below 4G memory address.
>> -
>> -  @param[in]  Size   Size of memory to allocate.
>> -
>> -  @return Allocated address for output.
>> -
>> -**/
>> -VOID *
>> -FpdtAllocateReservedMemoryBelow4G (
>> -  IN UINTN   Size
>> -  )
>> -{
>> -  UINTN Pages;
>> -  EFI_PHYSICAL_ADDRESS  Address;
>> -  EFI_STATUSStatus;
>> -  VOID  *Buffer;
>> -
>> -  Buffer  = NULL;
>> -  Pages   = EFI_SIZE_TO_PAGES (Size);
>> -  Address = 0x;
>> -
>> -  Status = gBS->AllocatePages (
>> -  AllocateMaxAddress,
>> -  EfiReservedMemoryType,
>> -  Pages,
>> -  
>> -  );
>> -  ASSERT_EFI_ERROR (Status);
>> -
>> -  if (!EFI_ERROR (Status)) {
>> -Buffer = (VOID *) (UINTN) Address;
>> -ZeroMem (Buffer, Size);
>> -  }
>> -
>> -  return Buffer;
>> -}
>> -
>>  /**
>>Allocate buffer for Boot Performance table.
>>
>> @@ -348,7 +308,10 @@ AllocateBootPerformanceTable (
>>  //
>>  // Fail to allocate at specified address, continue to allocate at any 
>> address.
>>  //
>> -mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
>> FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
>> +mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
>> AllocatePeiAccessiblePages (
>> + 
>> EfiReservedMemoryType,
>> + 
>> EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
>> + );
>>}
>>DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance Table 
>> address = 0x%x\n", mAcpiBootPerformanceTable));
>>
>>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 5/5] MdeModulePkg/FirmwarePerformanceDataTableDxe: use AllocatePeiAccessiblePages

[Laszlo Ersek]

On 05/24/18 11:09, Ard Biesheuvel wrote:
> Replace the call to and implementation of the function
> FpdtAllocateReservedMemoryBelow4G() with a call to
> AllocatePeiAccessiblePages, which boils down to the same on X64,
> but does not crash non-X64 systems that lack memory below 4 GB.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
> Note that the ZeroMem() call is dropped, but it is redundant anyway, given
> that in both cases, the subsequent CopyMem() call supersedes it immediately.
> 
>  
> MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
>| 51 
>  
> MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
>  |  1 +
>  2 files changed, 10 insertions(+), 42 deletions(-)
> 
> diff --git 
> a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
>  
> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
> index e719e9e482cb..ded817f37301 100644
> --- 
> a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
> +++ 
> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
> @@ -32,6 +32,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -179,46 +180,6 @@ FpdtAcpiTableChecksum (
>Buffer[ChecksumOffset] = CalculateCheckSum8 (Buffer, Size);
>  }
>  
> -/**
> -  Allocate EfiReservedMemoryType below 4G memory address.
> -
> -  This function allocates EfiReservedMemoryType below 4G memory address.
> -
> -  @param[in]  Size   Size of memory to allocate.
> -
> -  @return Allocated address for output.
> -
> -**/
> -VOID *
> -FpdtAllocateReservedMemoryBelow4G (
> -  IN UINTN   Size
> -  )
> -{
> -  UINTN Pages;
> -  EFI_PHYSICAL_ADDRESS  Address;
> -  EFI_STATUSStatus;
> -  VOID  *Buffer;
> -
> -  Buffer  = NULL;
> -  Pages   = EFI_SIZE_TO_PAGES (Size);
> -  Address = 0x;
> -
> -  Status = gBS->AllocatePages (
> -  AllocateMaxAddress,
> -  EfiReservedMemoryType,
> -  Pages,
> -  
> -  );
> -  ASSERT_EFI_ERROR (Status);
> -
> -  if (!EFI_ERROR (Status)) {
> -Buffer = (VOID *) (UINTN) Address;
> -ZeroMem (Buffer, Size);
> -  }
> -
> -  return Buffer;
> -}
> -
>  /**
>Callback function upon VariableArchProtocol and LockBoxProtocol
>to allocate S3 performance table memory and save the pointer to LockBox.
> @@ -287,7 +248,10 @@ FpdtAllocateS3PerformanceTableMemory (
>  //
>  // Fail to allocate at specified address, continue to allocate at 
> any address.
>  //
> -mAcpiS3PerformanceTable = (S3_PERFORMANCE_TABLE *) 
> FpdtAllocateReservedMemoryBelow4G (sizeof (S3_PERFORMANCE_TABLE));
> +mAcpiS3PerformanceTable = (S3_PERFORMANCE_TABLE *) 
> AllocatePeiAccessiblePages (
> + 
> EfiReservedMemoryType,
> + 
> EFI_SIZE_TO_PAGES (sizeof (S3_PERFORMANCE_TABLE))
> + );
>}
>DEBUG ((EFI_D_INFO, "FPDT: ACPI S3 Performance Table address = 
> 0x%x\n", mAcpiS3PerformanceTable));
>if (mAcpiS3PerformanceTable != NULL) {
> @@ -368,7 +332,10 @@ InstallFirmwarePerformanceDataTable (
>//
>// Fail to allocate at specified address, continue to allocate at any 
> address.
>//
> -  mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
> FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
> +  mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
> AllocatePeiAccessiblePages (
> +   
> EfiReservedMemoryType,
> +   
> EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
> +   );
>  }
>  DEBUG ((DEBUG_INFO, "FPDT: ACPI Boot Performance Table address = 
> 0x%x\n", mAcpiBootPerformanceTable));
>  if (mAcpiBootPerformanceTable == NULL) {
> diff --git 
> a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
>  
> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
> index 8757bbd0aaa9..3d2dd6eb732f 100644
> --- 
> a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
> +++ 
> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
> @@ -44,6 +44,7 @@ [LibraryClasses]
>UefiRuntimeServicesTableLib
>BaseLib
>DebugLib
> +  DxeServicesLib
>TimerLib
>BaseMemoryLib
>MemoryAllocationLib
> 

Reviewed-by: Laszlo Ersek 

Re: [edk2] gdb reload-uefi missing EFI_SYSTEM_TABLE_POINTER [was: Source code debugging of OVMF]

[Johannes Swoboda]

Hello,

I asked you for help for source level debugging of ovmf some time ago.
Things got in the way after that, but now I was able to follow your
advice and achieved my goal.

In particular,

1. I added the debug-lines below to GdbSyms.c
2. I started debugging later in the BDS phase (as suggested by several
of you)
3. I got rid of the qemu flag -enable-kvm (it screwed up the stepping in
gdb)

Thank you very much!

Kind regards,
Johannes


On 2018-04-24 04:13, Gary Lin wrote:
> On Fri, Apr 20, 2018 at 04:54:06PM +0200, Johannes Swoboda wrote:
>> gdb complained:
>>> Python Exception  No type named
>>> EFI_SYSTEM_TABLE_POINTER.:
>>> [...]
> I encountered the issue before and it seems caused by the linker option,
> "--whole-archive", which drops the symbols not used.
> 
> As a workaround, I add the following lines to Initialize():
> 
>   DEBUG ((DEBUG_VERBOSE, "%a: %llx\n", __FUNCTION__, ));
>   DEBUG ((DEBUG_VERBOSE, "%a: %llx\n", __FUNCTION__, ));
>   DEBUG ((DEBUG_VERBOSE, "%a: %llx\n", __FUNCTION__, ));
>   DEBUG ((DEBUG_VERBOSE, "%a: %llx\n", __FUNCTION__, ));
>   DEBUG ((DEBUG_VERBOSE, "%a: %llx\n", __FUNCTION__, ));
>   DEBUG ((DEBUG_VERBOSE, "%a: %llx\n", __FUNCTION__, ));
>   DEBUG ((DEBUG_VERBOSE, "%a: %llx\n", __FUNCTION__, ));
>   DEBUG ((DEBUG_VERBOSE, "%a: %llx\n", __FUNCTION__, ));
> 
> It at least makes the linker believe the symbols are important.
> Maybe you can try it.
> 
> Cheers,
> 
> Gary Lin
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

[Laszlo Ersek]

On 05/24/18 11:09, Ard Biesheuvel wrote:
> Replace the call to and implementation of the function
> FpdtAllocateReservedMemoryBelow4G() with a call to
> AllocatePeiAccessiblePages, which boils down to the same on X64,
> but does not crash non-X64 systems that lack memory below 4 GB.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
> Note that the ZeroMem() call is dropped, but it is redundant anyway, given
> that the subsequent CopyMem() call supersedes it immediately.

I'm not sure the ZeroMem() is redundant. The allocation size -- before rounding 
up to full pages -- is computed like this:

  BootPerformanceDataSize = sizeof (BOOT_PERFORMANCE_TABLE) + 
mPerformanceLength + PcdGet32 (PcdExtFpdtBootRecordPadSize);
  if (SmmCommData != NULL && SmmBootRecordData != NULL) {
BootPerformanceDataSize += SmmBootRecordDataSize;
  }

ZeroMem() covers all of the above, but the CopyMem() calls don't seem to cover 
the optional padding (from the PCD).

I'm unsure if that matters, of course.

The patch looks good to me otherwise.

Thanks
Laszlo


> 
>  MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 45 
> ++--
>  1 file changed, 4 insertions(+), 41 deletions(-)
> 
> diff --git 
> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c 
> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> index 71d624fc9ce9..b1f09710b65c 100644
> --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> @@ -165,46 +165,6 @@ IsKnownID (
>}
>  }
>  
> -/**
> -  Allocate EfiReservedMemoryType below 4G memory address.
> -
> -  This function allocates EfiReservedMemoryType below 4G memory address.
> -
> -  @param[in]  Size   Size of memory to allocate.
> -
> -  @return Allocated address for output.
> -
> -**/
> -VOID *
> -FpdtAllocateReservedMemoryBelow4G (
> -  IN UINTN   Size
> -  )
> -{
> -  UINTN Pages;
> -  EFI_PHYSICAL_ADDRESS  Address;
> -  EFI_STATUSStatus;
> -  VOID  *Buffer;
> -
> -  Buffer  = NULL;
> -  Pages   = EFI_SIZE_TO_PAGES (Size);
> -  Address = 0x;
> -
> -  Status = gBS->AllocatePages (
> -  AllocateMaxAddress,
> -  EfiReservedMemoryType,
> -  Pages,
> -  
> -  );
> -  ASSERT_EFI_ERROR (Status);
> -
> -  if (!EFI_ERROR (Status)) {
> -Buffer = (VOID *) (UINTN) Address;
> -ZeroMem (Buffer, Size);
> -  }
> -
> -  return Buffer;
> -}
> -
>  /**
>Allocate buffer for Boot Performance table.
>  
> @@ -348,7 +308,10 @@ AllocateBootPerformanceTable (
>  //
>  // Fail to allocate at specified address, continue to allocate at any 
> address.
>  //
> -mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
> FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
> +mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
> AllocatePeiAccessiblePages (
> + 
> EfiReservedMemoryType,
> + 
> EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
> + );
>}
>DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance Table 
> address = 0x%x\n", mAcpiBootPerformanceTable));
>  
> 

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

Re: [edk2] [PATCH v2 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine

[Laszlo Ersek]

On 05/24/18 11:09, Ard Biesheuvel wrote:
> Add a routine to DxeServicesLib that abstracts the allocation of memory
> that should be accessible by PEI after a warm reboot. We will use it to
> replace open coded implementations that limit the address to < 4 GB,
> which may not be possible on non-Intel systems that have no 32-bit
> addressable memory at all.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  MdePkg/Include/Library/DxeServicesLib.h  | 23 +++-
>  MdePkg/Library/DxeServicesLib/DxeServicesLib.c   | 55 
>  MdePkg/Library/DxeServicesLib/DxeServicesLib.inf |  3 ++
>  3 files changed, 80 insertions(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Include/Library/DxeServicesLib.h 
> b/MdePkg/Include/Library/DxeServicesLib.h
> index 7c1c62236d96..20aee68af558 100644
> --- a/MdePkg/Include/Library/DxeServicesLib.h
> +++ b/MdePkg/Include/Library/DxeServicesLib.h
> @@ -305,5 +305,26 @@ GetFileDevicePathFromAnyFv (
>OUT   EFI_DEVICE_PATH_PROTOCOL  **FvFileDevicePath
>);
>  
> -#endif
> +/**
> +  Allocates one or more 4KB pages of a given type from a memory region that 
> is
> +  accessible to PEI.
> +
> +  Allocates the number of 4KB pages of type 'MemoryType' and returns a
> +  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
> +  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
> +  memory remaining to satisfy the request, then NULL is returned.
>  
> +  @param[in]  MemoryTypeThe memory type to allocate
> +  @param[in]  Pages The number of 4 KB pages to allocate.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocatePeiAccessiblePages (
> +  IN EFI_MEMORY_TYPE  MemoryType,
> +  IN UINTNPages
> +  );
> +
> +#endif
> diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c 
> b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
> index 1827c9216fbc..6719aabe5d04 100644
> --- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
> +++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
> @@ -16,6 +16,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1093,3 +1094,57 @@ Done:
>  
>return Status;
>  }
> +
> +/**
> +  Allocates one or more 4KB pages of a given type from a memory region that 
> is
> +  accessible to PEI.
> +
> +  Allocates the number of 4KB pages of type 'MemoryType' and returns a
> +  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
> +  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
> +  memory remaining to satisfy the request, then NULL is returned.
> +
> +  @param[in]  MemoryTypeThe memory type to allocate
> +  @param[in]  Pages The number of 4 KB pages to allocate.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocatePeiAccessiblePages (
> +  IN EFI_MEMORY_TYPE  MemoryType,
> +  IN UINTNPages
> +  )
> +{
> +  EFI_STATUS  Status;
> +  EFI_ALLOCATE_TYPE   AllocType;
> +  EFI_PHYSICAL_ADDRESSMemory;
> +#ifdef MDE_CPU_X64
> +  EFI_HOB_HANDOFF_INFO_TABLE  *PhitHob;
> +#endif
> +
> +  if (Pages == 0) {
> +return NULL;
> +  }
> +
> +  AllocType = AllocateAnyPages;
> +#ifdef MDE_CPU_X64
> +  //
> +  // On X64 systems, a X64 build of DXE may be combined with a 32-bit build 
> of
> +  // PEI, and so we need to check the memory limit set by PEI, and allocate
> +  // below 4 GB if the limit is set to 4 GB or lower.
> +  //
> +  PhitHob = (EFI_HOB_HANDOFF_INFO_TABLE *)GetHobList ();
> +  if (PhitHob->EfiFreeMemoryTop <= MAX_UINT32) {
> +AllocType = AllocateMaxAddress;
> +Memory = MAX_UINT32;
> +  }
> +#endif
> +
> +  Status = gBS->AllocatePages (AllocType, MemoryType, Pages, );
> +  if (EFI_ERROR (Status)) {
> +return NULL;
> +  }
> +  return (VOID *)(UINTN)Memory;
> +}
> diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf 
> b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> index bd2faf2f6f2d..b0306c09872f 100644
> --- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> +++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> @@ -44,6 +44,9 @@ [LibraryClasses]
>UefiLib
>UefiBootServicesTableLib
>  
> +[LibraryClasses.common.X64]
> +  HobLib
> +
>  [Guids]
>gEfiFileInfoGuid  ## SOMETIMES_CONSUMES ## 
> UNDEFINED
>  
> 

Perhaps the #include  directive could be guarded by
#ifdef MDE_CPU_X64 as well, just for symmetry with the HobLib lib class,
but it's not too important.

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

Re: [edk2] [PATCH edk2-platforms v6 0/9] Platform/ARM/Sgi: Add Arm's SGI platform support

[Thomas Abraham]

On Thu, May 24, 2018 at 5:57 PM, Ard Biesheuvel
 wrote:
> On 23 May 2018 at 07:45, Thomas Abraham  wrote:
>> Changes since v5:
>> - removed 'Change-Id' from all the patches in this series.
>>
>> Changes since v4:
>> - addressed all the review comments from Ard and Alexei
>>
>> Changes since v3:
>> - adds support for SMSC9118 ethernet controller
>> - adds support for PCIe and SATA controller to enable SATA disk access
>>
>> Changes since v2:
>> - addressed all the comments from Ard.
>> - PrePeiCore is used instead of PrePi.
>>
>> Changes since v1:
>> - minor update to commit messages
>>
>> Arm CoreLink System Guidance for Infrastructure is a collection of
>> resources to provide a representative view of typical compute subsystems
>> that can be designed and implemented using specific generations of Arm IP.
>> These compute subsystems address the expected requirements of a specific
>> segment of the infrastructure market which includes servers, storage and
>> networking.
>>
>> This patch series adds initial platform port support for Arm'S SGI-575
>> platform. This platform has 8x Cortex-A75 CPUs, supports DynamIQ
>> with L3 cache options, supports 2x DDR4-3200 (DMC-620) memory controller
>> and is SBSAv3 compliant. This series includes support for GIC, Serial,
>> smsc91x and virtio block device.
>>
>> Daniil Egranov (4):
>>   Platform/ARM/Sgi: add initial platform dxe driver implementation
>>   Platform/ARM/Sgi: add support for virtio block device
>>   Platform/ARM/Sgi: add the initial set of acpi tables
>>   Platform/ARM/Sgi: add support for smsc91x ethernet controller
>>
>> Thomas Abraham (3):
>>   Platform/ARM/Sgi: Add Platform library implementation
>>   Platform/ARM/Sgi: implement PciHostBridgeLib support
>>   Platform/ARM/Sgi: Add Ssdt, Iort and Mcfg tables
>>
>> Vishwanatha HG (2):
>>   Platform/ARM/Sgi: add NOR flash platform library implementation
>>   Platform/ARM/Sgi: add initial support for ARM SGI platform
>>
>
> For the series
>
> Reviewed-by: Ard Biesheuvel 
>
> Pushed as e174fc77d3bb..eecb5e2f58a9
>
> Thanks,

Thank you Ard.

Regards,
Thomas.

> ___
> 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 v6 0/9] Platform/ARM/Sgi: Add Arm's SGI platform support

[Ard Biesheuvel]

On 23 May 2018 at 07:45, Thomas Abraham  wrote:
> Changes since v5:
> - removed 'Change-Id' from all the patches in this series.
>
> Changes since v4:
> - addressed all the review comments from Ard and Alexei
>
> Changes since v3:
> - adds support for SMSC9118 ethernet controller
> - adds support for PCIe and SATA controller to enable SATA disk access
>
> Changes since v2:
> - addressed all the comments from Ard.
> - PrePeiCore is used instead of PrePi.
>
> Changes since v1:
> - minor update to commit messages
>
> Arm CoreLink System Guidance for Infrastructure is a collection of
> resources to provide a representative view of typical compute subsystems
> that can be designed and implemented using specific generations of Arm IP.
> These compute subsystems address the expected requirements of a specific
> segment of the infrastructure market which includes servers, storage and
> networking.
>
> This patch series adds initial platform port support for Arm'S SGI-575
> platform. This platform has 8x Cortex-A75 CPUs, supports DynamIQ
> with L3 cache options, supports 2x DDR4-3200 (DMC-620) memory controller
> and is SBSAv3 compliant. This series includes support for GIC, Serial,
> smsc91x and virtio block device.
>
> Daniil Egranov (4):
>   Platform/ARM/Sgi: add initial platform dxe driver implementation
>   Platform/ARM/Sgi: add support for virtio block device
>   Platform/ARM/Sgi: add the initial set of acpi tables
>   Platform/ARM/Sgi: add support for smsc91x ethernet controller
>
> Thomas Abraham (3):
>   Platform/ARM/Sgi: Add Platform library implementation
>   Platform/ARM/Sgi: implement PciHostBridgeLib support
>   Platform/ARM/Sgi: Add Ssdt, Iort and Mcfg tables
>
> Vishwanatha HG (2):
>   Platform/ARM/Sgi: add NOR flash platform library implementation
>   Platform/ARM/Sgi: add initial support for ARM SGI platform
>

For the series

Reviewed-by: Ard Biesheuvel 

Pushed as e174fc77d3bb..eecb5e2f58a9

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

Re: [edk2] [PATCH edk2-platforms v6 5/9] Platform/ARM/Sgi: add the initial set of acpi tables

[Ard Biesheuvel]

On 24 May 2018 at 13:33, Thomas Abraham  wrote:
> From: Daniil Egranov 
>
> Add the initial set of Acpi tables for the SGI-575 platform. These tables
> conform to the ACPI specification version 6.1. Some of the mandatory tables
> required for SBBR v1.0 compilance are not included in this initial set
> of Acpi tables.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Daniil Egranov 
> Signed-off-by: Thomas Abraham 
> ---
>  .../ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf|  51 ++
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc|  90 +++
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl |  99 
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Fadt.aslc|  87 +++
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc| 152 ++
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Madt.aslc| 173 
> +
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Spcr.aslc|  77 +
>  .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   |   7 +
>  .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf |   6 +
>  Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h|  41 +
>  .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf |   4 +
>  11 files changed, 787 insertions(+)
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Fadt.aslc
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Madt.aslc
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Spcr.aslc
>  create mode 100644 Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h
>
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf 
> b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf
> new file mode 100644
> index 000..ec47f94
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf
> @@ -0,0 +1,51 @@
> +## @file
> +#  ACPI table data and ASL sources required to boot the platform.
> +#
> +#  Copyright (c) 2018, ARM Ltd. All rights reserved.
> +#
> +#  This program and the accompanying materials are licensed and made 
> available
> +#  under the terms and conditions of the BSD License which accompanies this
> +#  distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION= 0x0001001A
> +  BASE_NAME  = Sgi575AcpiTables
> +  FILE_GUID  = c712719a-0aaf-438c-9cdd-35ab4d60207d  # 
> gSgi575AcpiTablesiFileGuid
> +  MODULE_TYPE= USER_DEFINED
> +  VERSION_STRING = 1.0
> +
> +[Sources]
> +  Dbg2.aslc
> +  Dsdt.asl
> +  Fadt.aslc
> +  Gtdt.aslc
> +  Madt.aslc
> +  Spcr.aslc
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  Platform/ARM/SgiPkg/SgiPlatform.dec
> +
> +[FixedPcd]
> +  gArmPlatformTokenSpaceGuid.PcdCoreCount
> +  gArmPlatformTokenSpaceGuid.PcdClusterCount
> +  gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase
> +  gArmPlatformTokenSpaceGuid.PL011UartInterrupt
> +
> +  gArmTokenSpaceGuid.PcdArmArchTimerSecIntrNum
> +  gArmTokenSpaceGuid.PcdArmArchTimerIntrNum
> +  gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum
> +  gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum
> +  gArmTokenSpaceGuid.PcdGicDistributorBase
> +  gArmTokenSpaceGuid.PcdGicRedistributorsBase
> +  gArmTokenSpaceGuid.PcdGenericWatchdogControlBase
> +  gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc 
> b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc
> new file mode 100644
> index 000..ed671f3
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc
> @@ -0,0 +1,90 @@
> +/** @file
> +*  Debug Port Table 2 (DBG2)
> +*
> +*  Copyright (c) 2018, ARM Limited. All rights reserved.
> +*
> +*  This program and the accompanying materials are licensed and made 
> available
> +*  under the terms and conditions of the BSD License 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 "SgiAcpiHeader.h"
> +#include 
> +#include 
> +#include 
> +
> +#define SGI_DBG2_NUM_DEBUG_PORTS   1
> +#define 

Re: [edk2] [PATCH edk2-platforms v6 5/9] Platform/ARM/Sgi: add the initial set of acpi tables

[Alexei Fedorov]

Hi Thomas,

GTDT looks OK now.

Regards.
Alexei.

> -Original Message-
> From: Thomas Abraham 
> Sent: 24 May 2018 12:36
> To: Alexei Fedorov 
> Cc: Thomas Abraham ; edk2-devel@lists.01.org;
> leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: Re: [edk2] [PATCH edk2-platforms v6 5/9] Platform/ARM/Sgi: add the
> initial set of acpi tables
>
> Hi Alexei,
>
> On Wed, May 23, 2018 at 4:20 PM, Alexei Fedorov 
> wrote:
> > Please see my comment in-lined.
> >
> >> -Original Message-
> >> From: edk2-devel  On Behalf Of
> >> Thomas Abraham
> >> Sent: 23 May 2018 06:46
> >> To: edk2-devel@lists.01.org
> >> Cc: leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> >> Subject: [edk2] [PATCH edk2-platforms v6 5/9] Platform/ARM/Sgi: add
> >> the initial set of acpi tables
> >>
> >> From: Daniil Egranov 
> >>
> >> Add the initial set of Acpi tables for the SGI-575 platform. These
> >> tables conform to the ACPI specification version 6.1. Some of the
> >> mandatory tables required for SBBR v1.0 compilance are not included in this
> initial set of Acpi tables.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Daniil Egranov 
> >> Signed-off-by: Thomas Abraham 
> >> ---
> >>  .../ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf|  51 ++
> >>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc|  90 +++
> >>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl |  99 
> >>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Fadt.aslc|  87 +++
> >>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc| 151
> >> ++
> >>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Madt.aslc| 173
> >> +
> >>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Spcr.aslc|  77 +
> >>  .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   |   7 +
> >>  .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf |   6 +
> >>  Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h|  41 +
> >>  .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf |   4 +
> >>  11 files changed, 786 insertions(+)
> >>  create mode 100644
> >> Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf
> >>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc
> >>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl
> >>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Fadt.aslc
> >>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc
> >>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Madt.aslc
> >>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Spcr.aslc
> >>  create mode 100644 Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h
> >>
>
> 
>
> >> diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc
> >> b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc
> >> new file mode 100644
> >> index 000..40657c9
> >> --- /dev/null
> >> +++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc
> >> @@ -0,0 +1,151 @@
> >> +/** @file
> >> +*  Generic Timer Description Table (GTDT)
> >> +*
> >> +*  Copyright (c) 2018, ARM Limited. All rights reserved.
> >> +*
> >> +*  This program and the accompanying materials are licensed and made
> >> +available
> >> +*  under the terms and conditions of the BSD License 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 "SgiAcpiHeader.h"
> >> +#include 
> >> +#include 
> >> +#include 
> >> +
> >> +#define SGI_PLATFORM_WATCHDOG_COUNT   2
> >> +#define SGI_TIMER_FRAMES_COUNT2
> >> +
> >> +#define SYSTEM_TIMER_BASE_ADDRESS 0x
> >> +#define GTDT_GLOBAL_FLAGS 0
> >> +#define GTDT_GTIMER_FLAGS
> >> EFI_ACPI_6_1_GTDT_TIMER_FLAG_TIMER_INTERRUPT_POLARITY
> >> +
> >> +#define SGI_GT_BLOCK_CTL_BASE 0x2A81
> >> +#define SGI_GT_BLOCK_FRAME1_CTL_BASE  0x2A82
> >> +#define SGI_GT_BLOCK_FRAME1_CTL_EL0_BASE  0x
> >> +#define SGI_GT_BLOCK_FRAME1_GSIV  0x5B
> >> +
> >> +#define SGI_GT_BLOCK_FRAME0_CTL_BASE  0x2A83
> >> +#define SGI_GT_BLOCK_FRAME0_CTL_EL0_BASE  0x
> >> +#define SGI_GT_BLOCK_FRAME0_GSIV  0x5C
> >> +
> >> +#define SGI_GTX_TIMER_FLAGS   0
> >> +#define GTX_TIMER_SECURE
> >> EFI_ACPI_6_1_GTDT_GT_BLOCK_COMMON_FLAG_SECURE_TIMER
> >> +#define GTX_TIMER_NON_SECURE  0
> >> +#define GTX_TIMER_SAVE_CONTEXT
> >> EFI_ACPI_6_1_GTDT_GT_BLOCK_COMMON_FLAG_ALWAYS_ON_CAPABILITY
> >> +#define SGI_GTX_COMMON_FLAGS_S  

Re: [edk2] [PATCH edk2-platforms v6 5/9] Platform/ARM/Sgi: add the initial set of acpi tables

[Thomas Abraham]

Hi Alexei,

On Wed, May 23, 2018 at 4:20 PM, Alexei Fedorov  wrote:
> Please see my comment in-lined.
>
>> -Original Message-
>> From: edk2-devel  On Behalf Of Thomas
>> Abraham
>> Sent: 23 May 2018 06:46
>> To: edk2-devel@lists.01.org
>> Cc: leif.lindh...@linaro.org; ard.biesheu...@linaro.org
>> Subject: [edk2] [PATCH edk2-platforms v6 5/9] Platform/ARM/Sgi: add the 
>> initial
>> set of acpi tables
>>
>> From: Daniil Egranov 
>>
>> Add the initial set of Acpi tables for the SGI-575 platform. These tables 
>> conform
>> to the ACPI specification version 6.1. Some of the mandatory tables required 
>> for
>> SBBR v1.0 compilance are not included in this initial set of Acpi tables.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Daniil Egranov 
>> Signed-off-by: Thomas Abraham 
>> ---
>>  .../ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf|  51 ++
>>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc|  90 +++
>>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl |  99 
>>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Fadt.aslc|  87 +++
>>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc| 151
>> ++
>>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Madt.aslc| 173
>> +
>>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Spcr.aslc|  77 +
>>  .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   |   7 +
>>  .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf |   6 +
>>  Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h|  41 +
>>  .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf |   4 +
>>  11 files changed, 786 insertions(+)
>>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf
>>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc
>>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl
>>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Fadt.aslc
>>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc
>>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Madt.aslc
>>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Spcr.aslc
>>  create mode 100644 Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h
>>



>> diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc
>> b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc
>> new file mode 100644
>> index 000..40657c9
>> --- /dev/null
>> +++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc
>> @@ -0,0 +1,151 @@
>> +/** @file
>> +*  Generic Timer Description Table (GTDT)
>> +*
>> +*  Copyright (c) 2018, ARM Limited. All rights reserved.
>> +*
>> +*  This program and the accompanying materials are licensed and made
>> +available
>> +*  under the terms and conditions of the BSD License 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 "SgiAcpiHeader.h"
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define SGI_PLATFORM_WATCHDOG_COUNT   2
>> +#define SGI_TIMER_FRAMES_COUNT2
>> +
>> +#define SYSTEM_TIMER_BASE_ADDRESS 0x
>> +#define GTDT_GLOBAL_FLAGS 0
>> +#define GTDT_GTIMER_FLAGS
>> EFI_ACPI_6_1_GTDT_TIMER_FLAG_TIMER_INTERRUPT_POLARITY
>> +
>> +#define SGI_GT_BLOCK_CTL_BASE 0x2A81
>> +#define SGI_GT_BLOCK_FRAME1_CTL_BASE  0x2A82
>> +#define SGI_GT_BLOCK_FRAME1_CTL_EL0_BASE  0x
>> +#define SGI_GT_BLOCK_FRAME1_GSIV  0x5B
>> +
>> +#define SGI_GT_BLOCK_FRAME0_CTL_BASE  0x2A83
>> +#define SGI_GT_BLOCK_FRAME0_CTL_EL0_BASE  0x
>> +#define SGI_GT_BLOCK_FRAME0_GSIV  0x5C
>> +
>> +#define SGI_GTX_TIMER_FLAGS   0
>> +#define GTX_TIMER_SECURE
>> EFI_ACPI_6_1_GTDT_GT_BLOCK_COMMON_FLAG_SECURE_TIMER
>> +#define GTX_TIMER_NON_SECURE  0
>> +#define GTX_TIMER_SAVE_CONTEXT
>> EFI_ACPI_6_1_GTDT_GT_BLOCK_COMMON_FLAG_ALWAYS_ON_CAPABILITY
>> +#define SGI_GTX_COMMON_FLAGS_S(GTX_TIMER_SAVE_CONTEXT |
>> GTX_TIMER_SECURE)
>> +#define SGI_GTX_COMMON_FLAGS_NS   (GTX_TIMER_SAVE_CONTEXT |
>> GTX_TIMER_NON_SECURE)
>> +
>> +#define EFI_ACPI_6_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(\
>> +  RefreshFramePhysicalAddress, ControlFramePhysicalAddress,   \
>> +  WatchdogTimerGSIV, WatchdogTimerFlags)  \
>> +  {   \
>> +EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG,  \
>> +sizeof (EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE),   \
>> +EFI_ACPI_RESERVED_WORD,

[edk2] [PATCH edk2-platforms v6 5/9] Platform/ARM/Sgi: add the initial set of acpi tables

[Thomas Abraham]

From: Daniil Egranov 

Add the initial set of Acpi tables for the SGI-575 platform. These tables
conform to the ACPI specification version 6.1. Some of the mandatory tables
required for SBBR v1.0 compilance are not included in this initial set
of Acpi tables.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Daniil Egranov 
Signed-off-by: Thomas Abraham 
---
 .../ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf|  51 ++
 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc|  90 +++
 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl |  99 
 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Fadt.aslc|  87 +++
 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc| 152 ++
 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Madt.aslc| 173 +
 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Spcr.aslc|  77 +
 .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   |   7 +
 .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf |   6 +
 Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h|  41 +
 .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf |   4 +
 11 files changed, 787 insertions(+)
 create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf
 create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc
 create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl
 create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Fadt.aslc
 create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc
 create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Madt.aslc
 create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Spcr.aslc
 create mode 100644 Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h

diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf 
b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf
new file mode 100644
index 000..ec47f94
--- /dev/null
+++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf
@@ -0,0 +1,51 @@
+## @file
+#  ACPI table data and ASL sources required to boot the platform.
+#
+#  Copyright (c) 2018, ARM Ltd. All rights reserved.
+#
+#  This program and the accompanying materials are licensed and made available
+#  under the terms and conditions of the BSD License which accompanies this
+#  distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+##
+
+[Defines]
+  INF_VERSION= 0x0001001A
+  BASE_NAME  = Sgi575AcpiTables
+  FILE_GUID  = c712719a-0aaf-438c-9cdd-35ab4d60207d  # 
gSgi575AcpiTablesiFileGuid
+  MODULE_TYPE= USER_DEFINED
+  VERSION_STRING = 1.0
+
+[Sources]
+  Dbg2.aslc
+  Dsdt.asl
+  Fadt.aslc
+  Gtdt.aslc
+  Madt.aslc
+  Spcr.aslc
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
+  Platform/ARM/SgiPkg/SgiPlatform.dec
+
+[FixedPcd]
+  gArmPlatformTokenSpaceGuid.PcdCoreCount
+  gArmPlatformTokenSpaceGuid.PcdClusterCount
+  gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase
+  gArmPlatformTokenSpaceGuid.PL011UartInterrupt
+
+  gArmTokenSpaceGuid.PcdArmArchTimerSecIntrNum
+  gArmTokenSpaceGuid.PcdArmArchTimerIntrNum
+  gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum
+  gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum
+  gArmTokenSpaceGuid.PcdGicDistributorBase
+  gArmTokenSpaceGuid.PcdGicRedistributorsBase
+  gArmTokenSpaceGuid.PcdGenericWatchdogControlBase
+  gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase
diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc 
b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc
new file mode 100644
index 000..ed671f3
--- /dev/null
+++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc
@@ -0,0 +1,90 @@
+/** @file
+*  Debug Port Table 2 (DBG2)
+*
+*  Copyright (c) 2018, ARM Limited. All rights reserved.
+*
+*  This program and the accompanying materials are licensed and made available
+*  under the terms and conditions of the BSD License 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 "SgiAcpiHeader.h"
+#include 
+#include 
+#include 
+
+#define SGI_DBG2_NUM_DEBUG_PORTS   1
+#define SGI_DBG2_NUM_GAS   1
+#define SGI_DBG2_NS_STR_LENGTH 8
+#define SGI_PL011_REGISTER_SPACE   0x1000
+
+#define NAME_STR_UART1 {'C', 'O', 'M', '1', '\0', '\0', '\0', '\0'}
+
+#pragma pack(1)
+
+typedef struct {
+  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT Dbg2Device;
+  

[edk2] [PATCH edk2-platforms v6 5/9] Platform/ARM/Sgi: add the initial set of acpi tables

[Thomas Abraham]

From: Daniil Egranov 

Add the initial set of Acpi tables for the SGI-575 platform. These tables
conform to the ACPI specification version 6.1. Some of the mandatory tables
required for SBBR v1.0 compilance are not included in this initial set
of Acpi tables.

Change-Id: I5c0edf8c98e758a925ea95acec1f9e4ca85eea46
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Daniil Egranov 
Signed-off-by: Thomas Abraham 
---
 .../ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf|  51 ++
 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc|  90 +++
 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl |  99 
 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Fadt.aslc|  87 +++
 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc| 152 ++
 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Madt.aslc| 173 +
 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Spcr.aslc|  77 +
 .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   |   7 +
 .../ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf |   6 +
 Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h|  41 +
 .../ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf |   4 +
 11 files changed, 787 insertions(+)
 create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf
 create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc
 create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl
 create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Fadt.aslc
 create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc
 create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Madt.aslc
 create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Spcr.aslc
 create mode 100644 Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h

diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf 
b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf
new file mode 100644
index 000..ec47f94
--- /dev/null
+++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf
@@ -0,0 +1,51 @@
+## @file
+#  ACPI table data and ASL sources required to boot the platform.
+#
+#  Copyright (c) 2018, ARM Ltd. All rights reserved.
+#
+#  This program and the accompanying materials are licensed and made available
+#  under the terms and conditions of the BSD License which accompanies this
+#  distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+##
+
+[Defines]
+  INF_VERSION= 0x0001001A
+  BASE_NAME  = Sgi575AcpiTables
+  FILE_GUID  = c712719a-0aaf-438c-9cdd-35ab4d60207d  # 
gSgi575AcpiTablesiFileGuid
+  MODULE_TYPE= USER_DEFINED
+  VERSION_STRING = 1.0
+
+[Sources]
+  Dbg2.aslc
+  Dsdt.asl
+  Fadt.aslc
+  Gtdt.aslc
+  Madt.aslc
+  Spcr.aslc
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
+  Platform/ARM/SgiPkg/SgiPlatform.dec
+
+[FixedPcd]
+  gArmPlatformTokenSpaceGuid.PcdCoreCount
+  gArmPlatformTokenSpaceGuid.PcdClusterCount
+  gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase
+  gArmPlatformTokenSpaceGuid.PL011UartInterrupt
+
+  gArmTokenSpaceGuid.PcdArmArchTimerSecIntrNum
+  gArmTokenSpaceGuid.PcdArmArchTimerIntrNum
+  gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum
+  gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum
+  gArmTokenSpaceGuid.PcdGicDistributorBase
+  gArmTokenSpaceGuid.PcdGicRedistributorsBase
+  gArmTokenSpaceGuid.PcdGenericWatchdogControlBase
+  gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase
diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc 
b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc
new file mode 100644
index 000..ed671f3
--- /dev/null
+++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc
@@ -0,0 +1,90 @@
+/** @file
+*  Debug Port Table 2 (DBG2)
+*
+*  Copyright (c) 2018, ARM Limited. All rights reserved.
+*
+*  This program and the accompanying materials are licensed and made available
+*  under the terms and conditions of the BSD License 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 "SgiAcpiHeader.h"
+#include 
+#include 
+#include 
+
+#define SGI_DBG2_NUM_DEBUG_PORTS   1
+#define SGI_DBG2_NUM_GAS   1
+#define SGI_DBG2_NS_STR_LENGTH 8
+#define SGI_PL011_REGISTER_SPACE   0x1000
+
+#define NAME_STR_UART1 {'C', 'O', 'M', '1', '\0', '\0', '\0', '\0'}
+
+#pragma pack(1)
+
+typedef struct {
+  

[edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

[Ard Biesheuvel]

Replace the call to and implementation of the function
FpdtAllocateReservedMemoryBelow4G() with a call to
AllocatePeiAccessiblePages, which boils down to the same on X64,
but does not crash non-X64 systems that lack memory below 4 GB.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
Note that the ZeroMem() call is dropped, but it is redundant anyway, given
that the subsequent CopyMem() call supersedes it immediately.

 MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 45 
++--
 1 file changed, 4 insertions(+), 41 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c 
b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
index 71d624fc9ce9..b1f09710b65c 100644
--- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
+++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
@@ -165,46 +165,6 @@ IsKnownID (
   }
 }
 
-/**
-  Allocate EfiReservedMemoryType below 4G memory address.
-
-  This function allocates EfiReservedMemoryType below 4G memory address.
-
-  @param[in]  Size   Size of memory to allocate.
-
-  @return Allocated address for output.
-
-**/
-VOID *
-FpdtAllocateReservedMemoryBelow4G (
-  IN UINTN   Size
-  )
-{
-  UINTN Pages;
-  EFI_PHYSICAL_ADDRESS  Address;
-  EFI_STATUSStatus;
-  VOID  *Buffer;
-
-  Buffer  = NULL;
-  Pages   = EFI_SIZE_TO_PAGES (Size);
-  Address = 0x;
-
-  Status = gBS->AllocatePages (
-  AllocateMaxAddress,
-  EfiReservedMemoryType,
-  Pages,
-  
-  );
-  ASSERT_EFI_ERROR (Status);
-
-  if (!EFI_ERROR (Status)) {
-Buffer = (VOID *) (UINTN) Address;
-ZeroMem (Buffer, Size);
-  }
-
-  return Buffer;
-}
-
 /**
   Allocate buffer for Boot Performance table.
 
@@ -348,7 +308,10 @@ AllocateBootPerformanceTable (
 //
 // Fail to allocate at specified address, continue to allocate at any 
address.
 //
-mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
+mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
AllocatePeiAccessiblePages (
+ 
EfiReservedMemoryType,
+ EFI_SIZE_TO_PAGES 
(BootPerformanceDataSize)
+ );
   }
   DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance Table 
address = 0x%x\n", mAcpiBootPerformanceTable));
 
-- 
2.17.0

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

[edk2] [PATCH v2 0/5] Abstract allocation of PEI accessible memory

[Ard Biesheuvel]

At the moment, FirmwarePerformanceTableDataDxe or DxeCorePerformanceLib
are unusable on systems such as AMD Seattle, because they unconditionally
attempt to allocate memory below 4 GB, and ASSERT() if this fails. On AMD
Seattle, no 32-bit addressable DRAM exists, and so the driver will always
assert, and crash a running DEBUG build.

The reason for this is that some platforms (i.e., X64 builds consisting of
a 32-bit PEI stage and 64-bit remaining stages) require these allocations
to be below 4 GB, and for some reason, open coding this practice throughout
the code without regard for other architectures has been regarded as an
acceptable approach.

Instead, I would like to propose the approach implemented in this series:
- create an abstracted EfiAllocatePeiAccessiblePages() routine in UefiLib
  that simply allocates pages from any region on all architectures except
  X64, and allocate below 4 GB for X64
- update the various call sites with calls to this new function.

The above is implemented in patches #3 - #6. Patches #1 and #2 are fixes
for issues that I observed in ArmVirtPkg and OvmfPkg while working on
these patches.

Code can be found here:
https://github.com/ardbiesheuvel/edk2/tree/allocate-pei-v2

Changes since v1:
- add Laszlo's ack to #1 - #2
- move EfiAllocatePeiPages() to DxeServicesLib and drop Efi prefix
- update implementation to take EfiFreeMemoryTop into account when built
  for X64

Cc: Laszlo Ersek 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Dandan Bi 

Ard Biesheuvel (5):
  OvmfPkg/PlatformBootManagerLib: add missing report status code call
  ArmVirtPkg/PlatformBootManagerLib: add missing report status code call
  MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine
  MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages
  MdeModulePkg/FirmwarePerformanceDataTableDxe: use
AllocatePeiAccessiblePages

 .../PlatformBootManagerLib.inf|  1 +
 .../PlatformBootManagerLib/QemuKernel.c   |  4 ++
 .../DxeCorePerformanceLib.c   | 45 ++-
 .../FirmwarePerformanceDxe.c  | 51 +++--
 .../FirmwarePerformanceDxe.inf|  1 +
 MdePkg/Include/Library/DxeServicesLib.h   | 23 +++-
 .../Library/DxeServicesLib/DxeServicesLib.c   | 55 +++
 .../Library/DxeServicesLib/DxeServicesLib.inf |  3 +
 .../PlatformBootManagerLib.inf|  1 +
 .../PlatformBootManagerLib/QemuKernel.c   |  4 ++
 10 files changed, 104 insertions(+), 84 deletions(-)

-- 
2.17.0

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

[edk2] [PATCH v2 1/5] OvmfPkg/PlatformBootManagerLib: add missing report status code call

[Ard Biesheuvel]

Consumers of status code reports may rely on a status code to be
reported when the ReadyToBoot event is signalled. For instance,
FirmwarePerformanceDxe will fail to install the FPDT ACPI table
in this case. So add the missing call.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Laszlo Ersek 
---
 OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 +
 OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c   | 4 
 2 files changed, 5 insertions(+)

diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 27789b7377bc..f10b68424b91 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -55,6 +55,7 @@ [LibraryClasses]
   QemuFwCfgS3Lib
   LoadLinuxLib
   QemuBootOrderLib
+  ReportStatusCodeLib
   UefiLib
 
 [Pcd]
diff --git a/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c 
b/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
index ef728dfdeb60..f20df9533fda 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
+++ b/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -149,6 +150,9 @@ TryRunningQemuKernel (
   //
   EfiSignalEventReadyToBoot();
 
+  REPORT_STATUS_CODE (EFI_PROGRESS_CODE,
+(EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_PC_READY_TO_BOOT_EVENT));
+
   Status = LoadLinux (KernelBuf, SetupBuf);
 
 FreeAndReturn:
-- 
2.17.0

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

[edk2] [PATCH v2 2/5] ArmVirtPkg/PlatformBootManagerLib: add missing report status code call

[Ard Biesheuvel]

Consumers of status code reports may rely on a status code to be
reported when the ReadyToBoot event is signalled. For instance,
FirmwarePerformanceDxe will fail to install the FPDT ACPI table
in this case. So add the missing call.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Laszlo Ersek 
---
 ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 +
 ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c   | 4 
 2 files changed, 5 insertions(+)

diff --git 
a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index d6c1ef95dc44..0cbc82f5d27d 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -53,6 +53,7 @@ [LibraryClasses]
   PrintLib
   QemuBootOrderLib
   QemuFwCfgLib
+  ReportStatusCodeLib
   UefiBootManagerLib
   UefiBootServicesTableLib
   UefiLib
diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c 
b/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
index ac47d21e71c8..7b59f57eb19f 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1072,6 +1073,9 @@ TryRunningQemuKernel (
   //
   EfiSignalEventReadyToBoot();
 
+  REPORT_STATUS_CODE (EFI_PROGRESS_CODE,
+(EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_PC_READY_TO_BOOT_EVENT));
+
   //
   // Start the image.
   //
-- 
2.17.0

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

[edk2] [PATCH v2 5/5] MdeModulePkg/FirmwarePerformanceDataTableDxe: use AllocatePeiAccessiblePages

[Ard Biesheuvel]

Replace the call to and implementation of the function
FpdtAllocateReservedMemoryBelow4G() with a call to
AllocatePeiAccessiblePages, which boils down to the same on X64,
but does not crash non-X64 systems that lack memory below 4 GB.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
Note that the ZeroMem() call is dropped, but it is redundant anyway, given
that in both cases, the subsequent CopyMem() call supersedes it immediately.

 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
   | 51 
 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
 |  1 +
 2 files changed, 10 insertions(+), 42 deletions(-)

diff --git 
a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
 
b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
index e719e9e482cb..ded817f37301 100644
--- 
a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
+++ 
b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -179,46 +180,6 @@ FpdtAcpiTableChecksum (
   Buffer[ChecksumOffset] = CalculateCheckSum8 (Buffer, Size);
 }
 
-/**
-  Allocate EfiReservedMemoryType below 4G memory address.
-
-  This function allocates EfiReservedMemoryType below 4G memory address.
-
-  @param[in]  Size   Size of memory to allocate.
-
-  @return Allocated address for output.
-
-**/
-VOID *
-FpdtAllocateReservedMemoryBelow4G (
-  IN UINTN   Size
-  )
-{
-  UINTN Pages;
-  EFI_PHYSICAL_ADDRESS  Address;
-  EFI_STATUSStatus;
-  VOID  *Buffer;
-
-  Buffer  = NULL;
-  Pages   = EFI_SIZE_TO_PAGES (Size);
-  Address = 0x;
-
-  Status = gBS->AllocatePages (
-  AllocateMaxAddress,
-  EfiReservedMemoryType,
-  Pages,
-  
-  );
-  ASSERT_EFI_ERROR (Status);
-
-  if (!EFI_ERROR (Status)) {
-Buffer = (VOID *) (UINTN) Address;
-ZeroMem (Buffer, Size);
-  }
-
-  return Buffer;
-}
-
 /**
   Callback function upon VariableArchProtocol and LockBoxProtocol
   to allocate S3 performance table memory and save the pointer to LockBox.
@@ -287,7 +248,10 @@ FpdtAllocateS3PerformanceTableMemory (
 //
 // Fail to allocate at specified address, continue to allocate at any 
address.
 //
-mAcpiS3PerformanceTable = (S3_PERFORMANCE_TABLE *) 
FpdtAllocateReservedMemoryBelow4G (sizeof (S3_PERFORMANCE_TABLE));
+mAcpiS3PerformanceTable = (S3_PERFORMANCE_TABLE *) 
AllocatePeiAccessiblePages (
+ 
EfiReservedMemoryType,
+ EFI_SIZE_TO_PAGES 
(sizeof (S3_PERFORMANCE_TABLE))
+ );
   }
   DEBUG ((EFI_D_INFO, "FPDT: ACPI S3 Performance Table address = 0x%x\n", 
mAcpiS3PerformanceTable));
   if (mAcpiS3PerformanceTable != NULL) {
@@ -368,7 +332,10 @@ InstallFirmwarePerformanceDataTable (
   //
   // Fail to allocate at specified address, continue to allocate at any 
address.
   //
-  mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
+  mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
AllocatePeiAccessiblePages (
+   
EfiReservedMemoryType,
+   
EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
+   );
 }
 DEBUG ((DEBUG_INFO, "FPDT: ACPI Boot Performance Table address = 0x%x\n", 
mAcpiBootPerformanceTable));
 if (mAcpiBootPerformanceTable == NULL) {
diff --git 
a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
 
b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
index 8757bbd0aaa9..3d2dd6eb732f 100644
--- 
a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
+++ 
b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
@@ -44,6 +44,7 @@ [LibraryClasses]
   UefiRuntimeServicesTableLib
   BaseLib
   DebugLib
+  DxeServicesLib
   TimerLib
   BaseMemoryLib
   MemoryAllocationLib
-- 
2.17.0

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

[edk2] [PATCH v2 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine

[Ard Biesheuvel]

Add a routine to DxeServicesLib that abstracts the allocation of memory
that should be accessible by PEI after a warm reboot. We will use it to
replace open coded implementations that limit the address to < 4 GB,
which may not be possible on non-Intel systems that have no 32-bit
addressable memory at all.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 MdePkg/Include/Library/DxeServicesLib.h  | 23 +++-
 MdePkg/Library/DxeServicesLib/DxeServicesLib.c   | 55 
 MdePkg/Library/DxeServicesLib/DxeServicesLib.inf |  3 ++
 3 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Library/DxeServicesLib.h 
b/MdePkg/Include/Library/DxeServicesLib.h
index 7c1c62236d96..20aee68af558 100644
--- a/MdePkg/Include/Library/DxeServicesLib.h
+++ b/MdePkg/Include/Library/DxeServicesLib.h
@@ -305,5 +305,26 @@ GetFileDevicePathFromAnyFv (
   OUT   EFI_DEVICE_PATH_PROTOCOL  **FvFileDevicePath
   );
 
-#endif
+/**
+  Allocates one or more 4KB pages of a given type from a memory region that is
+  accessible to PEI.
+
+  Allocates the number of 4KB pages of type 'MemoryType' and returns a
+  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
+  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
+  memory remaining to satisfy the request, then NULL is returned.
 
+  @param[in]  MemoryTypeThe memory type to allocate
+  @param[in]  Pages The number of 4 KB pages to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocatePeiAccessiblePages (
+  IN EFI_MEMORY_TYPE  MemoryType,
+  IN UINTNPages
+  );
+
+#endif
diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c 
b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
index 1827c9216fbc..6719aabe5d04 100644
--- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
+++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
@@ -16,6 +16,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1093,3 +1094,57 @@ Done:
 
   return Status;
 }
+
+/**
+  Allocates one or more 4KB pages of a given type from a memory region that is
+  accessible to PEI.
+
+  Allocates the number of 4KB pages of type 'MemoryType' and returns a
+  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
+  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
+  memory remaining to satisfy the request, then NULL is returned.
+
+  @param[in]  MemoryTypeThe memory type to allocate
+  @param[in]  Pages The number of 4 KB pages to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocatePeiAccessiblePages (
+  IN EFI_MEMORY_TYPE  MemoryType,
+  IN UINTNPages
+  )
+{
+  EFI_STATUS  Status;
+  EFI_ALLOCATE_TYPE   AllocType;
+  EFI_PHYSICAL_ADDRESSMemory;
+#ifdef MDE_CPU_X64
+  EFI_HOB_HANDOFF_INFO_TABLE  *PhitHob;
+#endif
+
+  if (Pages == 0) {
+return NULL;
+  }
+
+  AllocType = AllocateAnyPages;
+#ifdef MDE_CPU_X64
+  //
+  // On X64 systems, a X64 build of DXE may be combined with a 32-bit build of
+  // PEI, and so we need to check the memory limit set by PEI, and allocate
+  // below 4 GB if the limit is set to 4 GB or lower.
+  //
+  PhitHob = (EFI_HOB_HANDOFF_INFO_TABLE *)GetHobList ();
+  if (PhitHob->EfiFreeMemoryTop <= MAX_UINT32) {
+AllocType = AllocateMaxAddress;
+Memory = MAX_UINT32;
+  }
+#endif
+
+  Status = gBS->AllocatePages (AllocType, MemoryType, Pages, );
+  if (EFI_ERROR (Status)) {
+return NULL;
+  }
+  return (VOID *)(UINTN)Memory;
+}
diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf 
b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
index bd2faf2f6f2d..b0306c09872f 100644
--- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
+++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
@@ -44,6 +44,9 @@ [LibraryClasses]
   UefiLib
   UefiBootServicesTableLib
 
+[LibraryClasses.common.X64]
+  HobLib
+
 [Guids]
   gEfiFileInfoGuid  ## SOMETIMES_CONSUMES ## 
UNDEFINED
 
-- 
2.17.0

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

Re: [edk2] [RFC] Formalize source files to follow DOS format

[Gao, Liming]

Mike:
  I agree your comments. On default file set, this script can have the default 
ones. User can specify more set to append the default ones instead of override 
the default ones.

Thanks
Liming
>-Original Message-
>From: Kinney, Michael D
>Sent: Tuesday, May 22, 2018 6:59 AM
>To: Carsey, Jaben ; Kinney, Michael D
>
>Cc: Gao, Liming ; edk2-devel@lists.01.org
>Subject: RE: [edk2] [RFC] Formalize source files to follow DOS format
>
>Jaben,
>
>Yes.  With default behavior is default set and
>specifying one or more extensions overrides the
>default set.
>
>Mike
>
>> -Original Message-
>> From: Carsey, Jaben
>> Sent: Monday, May 21, 2018 3:43 PM
>> To: Kinney, Michael D 
>> Cc: Gao, Liming ; edk2-
>> de...@lists.01.org
>> Subject: Re: [edk2] [RFC] Formalize source files to
>> follow DOS format
>>
>> Mike,
>>
>> Perhaps a default set of file extensions that can be
>> overridden?
>>
>> -Jaben
>>
>>
>> > On May 21, 2018, at 3:41 PM, Kinney, Michael D
>>  wrote:
>> >
>> > Liming,
>> >
>> > We have a set of standard flags for tools that
>> > should always be present.
>> >
>> > --help
>> > -v
>> > -q
>> > --debug
>> >
>> > We should also always have the program name,
>> > description, version, and copyright.
>> >
>> > Please see BaseTools/Scripts/BinToPcd.py as
>> > an example.
>> >
>> > It might be useful to have a way to run this tool
>> > on a single file when BaseTools/Scripts/PatchCheck.py
>> > reports an issue.
>> >
>> > Do you think it would be good to have one option to
>> > scan path for file extensions that are documented as
>> > DOS line endings so the extensions do not have to be
>> > entered?
>> >
>> > Mike
>> >
>> >
>> >> -Original Message-
>> >> From: edk2-devel [mailto:edk2-devel-
>> >> boun...@lists.01.org] On Behalf Of Carsey, Jaben
>> >> Sent: Monday, May 21, 2018 7:50 AM
>> >> To: Gao, Liming ; edk2-
>> >> de...@lists.01.org
>> >> Subject: Re: [edk2] [RFC] Formalize source files to
>> >> follow DOS format
>> >>
>> >> Liming,
>> >>
>> >> One Pep8 thing.
>> >> Can you change to use the with statement for the file
>> >> read/write?
>> >>
>> >> Other small thoughts.
>> >> I think that FileList should be changed to a set as
>> >> order is not important.
>> >> Maybe wrapper the re.sub function with your own so
>> all
>> >> the .encode() are in one location?  As we move to
>> python
>> >> 3 we will have fewer changes to make.
>> >>
>> >>
>> >>> -Original Message-
>> >>> From: edk2-devel [mailto:edk2-devel-
>> >> boun...@lists.01.org] On Behalf Of
>> >>> Liming Gao
>> >>> Sent: Sunday, May 20, 2018 9:52 PM
>> >>> To: edk2-devel@lists.01.org
>> >>> Subject: [edk2] [RFC] Formalize source files to
>> follow
>> >> DOS format
>> >>>
>> >>> FormatDosFiles.py is added to clean up dos source
>> >> files. It bases on
>> >>> the rules defined in EDKII C Coding Standards
>> >> Specification.
>> >>> 5.1.2 Do not use tab characters
>> >>> 5.1.6 Only use CRLF (Carriage Return Line Feed) line
>> >> endings.
>> >>> 5.1.7 All files must end with CRLF
>> >>> No trailing white space in one line. (To be added in
>> >> spec)
>> >>>
>> >>> The source files in edk2 project with the below
>> >> postfix are dos format.
>> >>> .h .c .nasm .nasmb .asm .S .inf .dec .dsc .fdf .uni
>> >> .asl .aslc .vfr .idf
>> >>> .txt .bat .py
>> >>>
>> >>> The package maintainer can use this script to clean
>> up
>> >> all files in his
>> >>> package. The prefer way is to create one patch per
>> one
>> >> package.
>> >>>
>> >>> Contributed-under: TianoCore Contribution Agreement
>> >> 1.1
>> >>> Signed-off-by: Liming Gao 
>> >>> ---
>> >>> BaseTools/Scripts/FormatDosFiles.py | 93
>> >>> +
>> >>> 1 file changed, 93 insertions(+)
>> >>> create mode 100644
>> >> BaseTools/Scripts/FormatDosFiles.py
>> >>>
>> >>> diff --git a/BaseTools/Scripts/FormatDosFiles.py
>> >>> b/BaseTools/Scripts/FormatDosFiles.py
>> >>> new file mode 100644
>> >>> index 000..c3a5476
>> >>> --- /dev/null
>> >>> +++ b/BaseTools/Scripts/FormatDosFiles.py
>> >>> @@ -0,0 +1,93 @@
>> >>> +# @file FormatDosFiles.py
>> >>> +# This script format the source files to follow dos
>> >> style.
>> >>> +# It supports Python2.x and Python3.x both.
>> >>> +#
>> >>> +#  Copyright (c) 2018, Intel Corporation. All
>> rights
>> >> reserved.
>> >>> +#
>> >>> +#  This program and the accompanying materials
>> >>> +#  are licensed and made available under the terms
>> >> and conditions of the
>> >>> BSD License
>> >>> +#  which accompanies this distribution.  The full
>> >> text of the license may be
>> >>> found at
>> >>> +#  http://opensource.org/licenses/bsd-license.php
>> >>> +#
>> >>> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE
>> >> ON AN "AS IS"
>> >>> BASIS,
>> >>> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
>> 

Re: [edk2] [RFC] Formalize source files to follow DOS format

[Gao, Liming]

Jaben:
  What difference of statement for file read/write? 

  Besides, we use .encode() here to support python 3. After we move to python 
3, this script is not changed. 

Thanks
Liming
>-Original Message-
>From: Carsey, Jaben
>Sent: Monday, May 21, 2018 10:50 PM
>To: Gao, Liming ; edk2-devel@lists.01.org
>Subject: RE: [edk2] [RFC] Formalize source files to follow DOS format
>
>Liming,
>
>One Pep8 thing.
>Can you change to use the with statement for the file read/write?
>
>Other small thoughts.
>I think that FileList should be changed to a set as order is not important.
>Maybe wrapper the re.sub function with your own so all the .encode() are in
>one location?  As we move to python 3 we will have fewer changes to make.
>
>
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Liming Gao
>> Sent: Sunday, May 20, 2018 9:52 PM
>> To: edk2-devel@lists.01.org
>> Subject: [edk2] [RFC] Formalize source files to follow DOS format
>>
>> FormatDosFiles.py is added to clean up dos source files. It bases on
>> the rules defined in EDKII C Coding Standards Specification.
>> 5.1.2 Do not use tab characters
>> 5.1.6 Only use CRLF (Carriage Return Line Feed) line endings.
>> 5.1.7 All files must end with CRLF
>> No trailing white space in one line. (To be added in spec)
>>
>> The source files in edk2 project with the below postfix are dos format.
>> .h .c .nasm .nasmb .asm .S .inf .dec .dsc .fdf .uni .asl .aslc .vfr .idf
>> .txt .bat .py
>>
>> The package maintainer can use this script to clean up all files in his
>> package. The prefer way is to create one patch per one package.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Liming Gao 
>> ---
>>  BaseTools/Scripts/FormatDosFiles.py | 93
>> +
>>  1 file changed, 93 insertions(+)
>>  create mode 100644 BaseTools/Scripts/FormatDosFiles.py
>>
>> diff --git a/BaseTools/Scripts/FormatDosFiles.py
>> b/BaseTools/Scripts/FormatDosFiles.py
>> new file mode 100644
>> index 000..c3a5476
>> --- /dev/null
>> +++ b/BaseTools/Scripts/FormatDosFiles.py
>> @@ -0,0 +1,93 @@
>> +# @file FormatDosFiles.py
>> +# This script format the source files to follow dos style.
>> +# It supports Python2.x and Python3.x both.
>> +#
>> +#  Copyright (c) 2018, Intel Corporation. All rights reserved.
>> +#
>> +#  This program and the accompanying materials
>> +#  are licensed and made available under the terms and conditions of the
>> BSD License
>> +#  which accompanies this distribution.  The full text of the license may be
>> found at
>> +#  http://opensource.org/licenses/bsd-license.php
>> +#
>> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
>> BASIS,
>> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>> EXPRESS OR IMPLIED.
>> +#
>> +
>> +#
>> +# Import Modules
>> +#
>> +import argparse
>> +import os
>> +import os.path
>> +import re
>> +import sys
>> +
>> +"""
>> +difference of string between python2 and python3:
>> +
>> +there is a large difference of string in python2 and python3.
>> +
>> +in python2,there are two type string,unicode string (unicode type) and 8-
>bit
>> string (str type).
>> +us = u"abcd",
>> +unicode string,which is internally stored as unicode code point.
>> +s = "abcd",s = b"abcd",s = r"abcd",
>> +all of them are 8-bit string,which is internally stored as bytes.
>> +
>> +in python3,a new type called bytes replace 8-bit string,and str type is
>> regarded as unicode string.
>> +s = "abcd", s = u"abcd", s = r"abcd",
>> +all of them are str type,which is internally stored unicode code point.
>> +bs = b"abcd",
>> +bytes type,which is interally stored as bytes
>> +
>> +in python2 ,the both type string can be mixed use,but in python3 it could
>> not,
>> +which means the pattern and content in re match should be the same type
>> in python3.
>> +in function FormatFile,it read file in binary mode so that the content is
>bytes
>> type,so the pattern should also be bytes type.
>> +As a result,I add encode() to make it compitable among python2 and
>> python3.
>> +
>> +difference of encode,decode in python2 and python3:
>> +the builtin function str.encode(encoding) and str.decode(encoding) are
>> used for convert between 8-bit string and unicode string.
>> +
>> +in python2
>> +encode convert unicode type to str type.decode vice versa.default
>> encoding is ascii.
>> +for example: s = us.encode()
>> +but if the us is str type,the code will also work.it will be firstly 
>> convert
>> to unicode type,
>> +in this situation,the call equals s = us.decode().encode().
>> +
>> +in python3
>> +encode convert str type to bytes type,decode vice versa.default
>> encoding is utf8.
>> +fpr example:
>> +bs = s.encode(),only str type has encode method,so that won't be
>> used wrongly.decode is the same.
>> +
>> +in conclusion:
>> +this 

Re: [edk2] [PATCH v2 7/7] OvmfPkg/Virtio10Dxe: convert to PciCapLib

[Ard Biesheuvel]

On 23 May 2018 at 22:21, Laszlo Ersek  wrote:
> Replace the manual capability list parsing in OvmfPkg/Virtio10Dxe with
> PciCapLib and PciCapPciIoLib API calls.
>
> The VIRTIO_PCI_CAP_LINK structure type is now superfluous. (Well, it
> always has been; we should have used EFI_PCI_CAPABILITY_HDR.)
>
> Also, EFI_PCI_CAPABILITY_VENDOR_HDR is now included at the front of
> VIRTIO_PCI_CAP. No driver other than Virtio10Dxe relies on VIRTIO_PCI_CAP.
>
> Cc: Ard Biesheuvel 
> Cc: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 

Reviewed-by: Ard Biesheuvel 

> ---
>
> Notes:
> v2:
> - no changes
>
>  OvmfPkg/Virtio10Dxe/Virtio10.inf|   2 +
>  OvmfPkg/Include/IndustryStandard/Virtio10.h |   7 +-
>  OvmfPkg/Virtio10Dxe/Virtio10.c  | 135 +++-
>  3 files changed, 52 insertions(+), 92 deletions(-)
>
> diff --git a/OvmfPkg/Virtio10Dxe/Virtio10.inf 
> b/OvmfPkg/Virtio10Dxe/Virtio10.inf
> index c4ef15d94bfc..db0cb1189a29 100644
> --- a/OvmfPkg/Virtio10Dxe/Virtio10.inf
> +++ b/OvmfPkg/Virtio10Dxe/Virtio10.inf
> @@ -32,6 +32,8 @@ [LibraryClasses]
>BaseMemoryLib
>DebugLib
>MemoryAllocationLib
> +  PciCapLib
> +  PciCapPciIoLib
>UefiBootServicesTableLib
>UefiDriverEntryPoint
>UefiLib
> diff --git a/OvmfPkg/Include/IndustryStandard/Virtio10.h 
> b/OvmfPkg/Include/IndustryStandard/Virtio10.h
> index c5efb5cfcb8a..7d51aa36b326 100644
> --- a/OvmfPkg/Include/IndustryStandard/Virtio10.h
> +++ b/OvmfPkg/Include/IndustryStandard/Virtio10.h
> @@ -16,6 +16,7 @@
>  #ifndef _VIRTIO_1_0_H_
>  #define _VIRTIO_1_0_H_
>
> +#include 
>  #include 
>
>  //
> @@ -29,11 +30,7 @@
>  //
>  #pragma pack (1)
>  typedef struct {
> -  UINT8 CapId;   // Capability identifier (generic)
> -  UINT8 CapNext; // Link to next capability (generic)
> -} VIRTIO_PCI_CAP_LINK;
> -
> -typedef struct {
> +  EFI_PCI_CAPABILITY_VENDOR_HDR VendorHdr;
>UINT8  ConfigType; // Identifies the specific VirtIo 1.0 config structure
>UINT8  Bar;// The BAR that contains the structure
>UINT8  Padding[3];
> diff --git a/OvmfPkg/Virtio10Dxe/Virtio10.c b/OvmfPkg/Virtio10Dxe/Virtio10.c
> index e9b50b6e437b..9ebb72c76bfd 100644
> --- a/OvmfPkg/Virtio10Dxe/Virtio10.c
> +++ b/OvmfPkg/Virtio10Dxe/Virtio10.c
> @@ -21,6 +21,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>
> @@ -184,48 +186,6 @@ GetBarType (
>  }
>
>
> -/**
> -  Read a slice from PCI config space at the given offset, then advance the
> -  offset.
> -
> -  @param [in] PciIo   The EFI_PCI_IO_PROTOCOL instance that represents 
> the
> -  device.
> -
> -  @param [in,out] Offset  On input, the offset in PCI config space to start
> -  reading from. On output, the offset of the first 
> byte
> -  that was not read. On error, Offset is not 
> modified.
> -
> -  @param [in] SizeThe number of bytes to read.
> -
> -  @param [out]Buffer  On output, the bytes read from PCI config space are
> -  stored in this object.
> -
> -  @retval EFI_SUCCESS  Size bytes have been transferred from PCI config space
> -   (from Offset) to Buffer, and Offset has been 
> incremented
> -   by Size.
> -
> -  @return  Error codes from PciIo->Pci.Read().
> -**/
> -STATIC
> -EFI_STATUS
> -ReadConfigSpace (
> -  IN EFI_PCI_IO_PROTOCOL *PciIo,
> -  IN OUT UINT32  *Offset,
> -  IN UINTN   Size,
> - OUT VOID*Buffer
> -  )
> -{
> -  EFI_STATUS Status;
> -
> -  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint8, *Offset, Size, 
> Buffer);
> -  if (EFI_ERROR (Status)) {
> -return Status;
> -  }
> -  *Offset += (UINT32)Size;
> -  return EFI_SUCCESS;
> -}
> -
> -
>  /*
>Traverse the PCI capabilities list of a virtio-1.0 device, and capture the
>locations of the interesting virtio-1.0 register blocks.
> @@ -239,57 +199,51 @@ ReadConfigSpace (
>  will have been updated from the PCI
>  capabilities found.
>
> -  @param[in] CapabilityPtr  The offset of the first capability in PCI
> -config space, taken from the standard PCI
> -device header.
> -
>@retval EFI_SUCCESS  Traversal successful.
>
> -  @return  Error codes from the ReadConfigSpace() and 
> GetBarType()
> -   helper functions.
> +  @return  Error codes from PciCapPciIoLib, PciCapLib, and the
> +   GetBarType() helper function.
>  */
>  STATIC
>  EFI_STATUS
>  ParseCapabilities (
> -  IN OUT VIRTIO_1_0_DEV *Device,
> -  IN UINT8  CapabilityPtr
> +  IN OUT VIRTIO_1_0_DEV 

Re: [edk2] [PATCH v2 6/7] OvmfPkg/PciHotPlugInitDxe: convert to PciCapLib

[Ard Biesheuvel]

On 23 May 2018 at 22:21, Laszlo Ersek  wrote:
> Replace the manual capability list parsing in OvmfPkg/PciHotPlugInitDxe
> with PciCapLib and PciCapPciSegmentLib API calls.
>
> Cc: Ard Biesheuvel 
> Cc: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 

Reviewed-by: Ard Biesheuvel 

> ---
>
> Notes:
> v2:
> - no changes
>
>  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf |   5 +
>  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c   | 267 +++-
>  2 files changed, 102 insertions(+), 170 deletions(-)
>
> diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf 
> b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
> index 38043986eb67..cc2b60d44263 100644
> --- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
> +++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
> @@ -35,6 +35,8 @@ [LibraryClasses]
>DebugLib
>DevicePathLib
>MemoryAllocationLib
> +  PciCapLib
> +  PciCapPciSegmentLib
>PciLib
>UefiBootServicesTableLib
>UefiDriverEntryPoint
> @@ -42,5 +44,8 @@ [LibraryClasses]
>  [Protocols]
>gEfiPciHotPlugInitProtocolGuid ## ALWAYS_PRODUCES
>
> +[Pcd]
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId ## CONSUMES
> +
>  [Depex]
>TRUE
> diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c 
> b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
> index 177e1a62120d..3449796878ef 100644
> --- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
> +++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
> @@ -14,6 +14,7 @@
>  **/
>
>  #include 
> +#include 
>  #include 
>
>  #include 
> @@ -21,12 +22,20 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>
>  #include 
>  #include 
>
> +//
> +// TRUE if the PCI platform supports extended config space, FALSE otherwise.
> +//
> +STATIC BOOLEAN mPciExtConfSpaceSupported;
> +
> +
>  //
>  // The protocol interface this driver produces.
>  //
> @@ -248,91 +257,11 @@ HighBitSetRoundUp64 (
>  }
>
>
> -/**
> -  Read a slice from conventional PCI config space at the given offset, then
> -  advance the offset.
> -
> -  @param[in] PciAddress  The address of the PCI Device -- Bus, Device, 
> Function
> - -- in UEFI (not PciLib) encoding.
> -
> -  @param[in,out] Offset  On input, the offset in conventional PCI config 
> space
> - to start reading from. On output, the offset of the
> - first byte that was not read.
> -
> -  @param[in] SizeThe number of bytes to read.
> -
> -  @param[out] Buffer On output, the bytes read from PCI config space are
> - stored in this object.
> -**/
> -STATIC
> -VOID
> -ReadConfigSpace (
> -  IN CONST EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *PciAddress,
> -  IN OUT UINT8 *Offset,
> -  IN UINT8 Size,
> -  OUTVOID  *Buffer
> -  )
> -{
> -  PciReadBuffer (
> -PCI_LIB_ADDRESS (
> -  PciAddress->Bus,
> -  PciAddress->Device,
> -  PciAddress->Function,
> -  *Offset
> -  ),
> -Size,
> -Buffer
> -);
> -  *Offset += Size;
> -}
> -
> -
> -/**
> -  Convenience wrapper macro for ReadConfigSpace().
> -
> -  Given the following conditions:
> -
> -  - HeaderField is the first field in the structure pointed-to by Struct,
> -
> -  - Struct->HeaderField has been populated from the conventional PCI config
> -space of the PCI device identified by PciAddress,
> -
> -  - *Offset points one past HeaderField in the conventional PCI config space 
> of
> -the PCI device identified by PciAddress,
> -
> -  populate the rest of *Struct from conventional PCI config space, starting 
> at
> -  *Offset. Finally, increment *Offset so that it point one past *Struct.
> -
> -  @param[in] PciAddress  The address of the PCI Device -- Bus, Device, 
> Function
> - -- in UEFI (not PciLib) encoding. Type: pointer to
> - CONST EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS.
> -
> -  @param[in,out] Offset  On input, the offset in conventional PCI config 
> space
> - to start reading from; one past Struct->HeaderField.
> - On output, the offset of the first byte that was not
> - read; one past *Struct. Type: pointer to UINT8.
> -
> -  @param[out] Struct The structure to complete. Type: pointer to 
> structure
> - object.
> -
> -  @param[in] HeaderField The name of the first field in *Struct, after which
> - *Struct should be populated. Type: structure member
> - identifier.
> -**/
> -#define COMPLETE_CONFIG_SPACE_STRUCT(PciAddress, Offset, Struct, 
> HeaderField) \
> -  

Re: [edk2] [PATCH v2 5/7] ArmVirtPkg: resolve PciCapLib, PciCapPciSegmentLib, PciCapPciIoLib

[Ard Biesheuvel]

On 23 May 2018 at 22:21, Laszlo Ersek  wrote:
> Resolve the PciCapLib, PciCapPciSegmentLib, and PciCapPciIoLib classes to
> their single respective instances under OvmfPkg. Later patches will use
> those lib classes in OvmfPkg drivers, some of which are included in
> ArmVirt platforms.
>
> Cc: Ard Biesheuvel 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 

Reviewed-by: Ard Biesheuvel 

> ---
>
> Notes:
> v2:
> - update references from MdePkg to OvmfPkg
>
>  ArmVirtPkg/ArmVirt.dsc.inc | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index 35bccb3dc1f4..766e4f598a07 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -131,6 +131,9 @@ [LibraryClasses.common]
># PCI Libraries
>PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
>
> PciExpressLib|ArmVirtPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf
> +  PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
> +  
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> +  PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>
># USB Libraries
>UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
> --
> 2.14.1.3.gb7cf6e02401b
>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 4/7] OvmfPkg: resolve PciCapLib, PciCapPciSegmentLib, PciCapPciIoLib

[Ard Biesheuvel]

On 23 May 2018 at 22:21, Laszlo Ersek  wrote:
> Resolve the PciCapLib, PciCapPciSegmentLib, and PciCapPciIoLib classes to
> their single respective instances. Later patches will use these lib
> classes in OvmfPkg drivers.
>
> Cc: Ard Biesheuvel 
> Cc: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 

Reviewed-by: Ard Biesheuvel 

> ---
>
> Notes:
> v2:
> - update references from MdePkg to OvmfPkg
>
>  OvmfPkg/OvmfPkgIa32.dsc| 3 +++
>  OvmfPkg/OvmfPkgIa32X64.dsc | 3 +++
>  OvmfPkg/OvmfPkgX64.dsc | 3 +++
>  3 files changed, 9 insertions(+)
>
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 251434a9ff7c..a2c995b910cd 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -127,6 +127,9 @@ [LibraryClasses]
>PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
>PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
>PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
> +  PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
> +  
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> +  PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index ce247a59d61a..bc7db229d2d9 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -132,6 +132,9 @@ [LibraryClasses]
>PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
>PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
>PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
> +  PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
> +  
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> +  PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 67f7e155ee3e..0767b34d1877 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -132,6 +132,9 @@ [LibraryClasses]
>PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
>PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
>PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
> +  PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
> +  
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> +  PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> --
> 2.14.1.3.gb7cf6e02401b
>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/7] OvmfPkg: introduce PciCapPciIoLib

[Ard Biesheuvel]

On 23 May 2018 at 22:21, Laszlo Ersek  wrote:
> Add a library class, and a UEFI_DRIVER lib instance, that are layered on
> top of PciCapLib, and allow clients to plug an EFI_PCI_IO_PROTOCOL backend
> into PciCapLib, for config space access.
>
> (Side note:
>

Again, please retain the below.

> Although the UEFI spec says that EFI_PCI_IO_PROTOCOL_CONFIG() returns
> EFI_UNSUPPORTED if "[t]he address range specified by Offset, Width, and
> Count is not valid for the PCI configuration header of the PCI
> controller", this patch doesn't directly document the EFI_UNSUPPORTED
> error code, for ProtoDevTransferConfig() and its callers
> ProtoDevReadConfig() and ProtoDevWriteConfig(). Instead, the patch refers
> to "unspecified error codes". The reason is that in edk2, the
> PciIoConfigRead() and PciIoConfigWrite() functions [1] can also return
> EFI_INVALID_PARAMETER for the above situation.
>
> Namely, PciIoConfigRead() and PciIoConfigWrite() first call
> PciIoVerifyConfigAccess(), which indeed produces the standard
> EFI_UNSUPPORTED error code, if the device's config space is exceeded.
> However, if PciIoVerifyConfigAccess() passes, and we reach
> RootBridgeIoPciRead() and RootBridgeIoPciWrite() [2], then
> RootBridgeIoCheckParameter() can still fail, e.g. if the root bridge
> doesn't support extended config space (see commit 014b472053ae3).
>
> For all kinds of Limit violations in IO, MMIO, and config space,
> RootBridgeIoCheckParameter() returns EFI_INVALID_PARAMETER, not
> EFI_UNSUPPORTED. That error code is then propagated up to, and out of,
> PciIoConfigRead() and PciIoConfigWrite().
>
> [1] MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> [2] MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> )
>
> Cc: Ard Biesheuvel 
> Cc: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 
> ---
>
> Notes:
> v2:
> - move library from MdePkg to OvmfPkg, for initial introduction
>
>  OvmfPkg/OvmfPkg.dec   |   5 +
>  OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf |  36 +++
>  OvmfPkg/Include/Library/PciCapPciIoLib.h  |  58 +
>  OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.h   |  44 
>  OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c   | 243 
> 
>  5 files changed, 386 insertions(+)
>
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index fbec1cfe4a8e..dc5597db4136 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -35,6 +35,11 @@ [LibraryClasses]
>#  config space.
>PciCapLib|Include/Library/PciCapLib.h
>
> +  ##  @libraryclass  Layered on top of PciCapLib, allows clients to plug an
> +  #  EFI_PCI_IO_PROTOCOL backend into PciCapLib, for config
> +  #  space access.
> +  PciCapPciIoLib|Include/Library/PciCapPciIoLib.h
> +
>##  @libraryclass  Layered on top of PciCapLib, allows clients to plug a
>#  PciSegmentLib backend into PciCapLib, for config space
>#  access.
> diff --git a/OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf 
> b/OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> new file mode 100644
> index ..2e14acb0ab75
> --- /dev/null
> +++ b/OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> @@ -0,0 +1,36 @@
> +## @file
> +# Plug an EFI_PCI_IO_PROTOCOL backend into PciCapLib, for config space 
> access.
> +#
> +# Copyright (C) 2018, Red Hat, Inc.
> +#
> +# This program and the accompanying materials are licensed and made available
> +# under the terms and conditions of the BSD License which accompanies this
> +# distribution.  The full text of the license may be found at
> +# http://opensource.org/licenses/bsd-license.php
> +#
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, 
> WITHOUT
> +# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +##
> +
> +[Defines]
> +  INF_VERSION= 1.27
> +  BASE_NAME  = UefiPciCapPciIoLib
> +  FILE_GUID  = 4102F4FE-DA10-4F0F-AC18-4982ED506154
> +  MODULE_TYPE= UEFI_DRIVER
> +  VERSION_STRING = 1.0
> +  LIBRARY_CLASS  = PciCapPciIoLib
> +
> +[Sources]
> +  UefiPciCapPciIoLib.h
> +  UefiPciCapPciIoLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +
> +[LibraryClasses]
> +  DebugLib
> +  MemoryAllocationLib
> +
> +[Protocols]
> +  gEfiPciIoProtocolGuid ## CONSUMES
> diff --git a/OvmfPkg/Include/Library/PciCapPciIoLib.h 
> b/OvmfPkg/Include/Library/PciCapPciIoLib.h
> new file mode 100644
> index ..553715fd5080
> --- /dev/null
> +++ b/OvmfPkg/Include/Library/PciCapPciIoLib.h
> @@ -0,0 +1,58 @@
> +/** @file
> +  Library class layered on top of PciCapLib that allows clients to plug an
> +  EFI_PCI_IO_PROTOCOL backend into PciCapLib, for config space access.
> +
> +  Copyright 

[edk2] [PATCH] CryptoPkg: Remove deprecated function usage in X509GetCommonName()

[Long Qin]

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

X509_NAME_get_text_by_NID() used in X509GetCommonName() implementation
is one legacy function which have various limitations. The returned
data may be not usable  when the target cert contains multicharacter
string type like a BMPString or a UTF8String.
This patch replaced the legacy function usage with more general
X509_NAME_get_index_by_NID() / X509_NAME_get_entry() APIs for X509
CommonName retrieving.

Tests: Validated the commonName retrieving with test certificates
   containing PrintableString or BMPString data.

Cc: Ye Ting 
Cc: Michael Turner 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Long Qin 
---
 CryptoPkg/Include/Library/BaseCryptLib.h  |  4 +-
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 53 ++-
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c |  4 +-
 3 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h 
b/CryptoPkg/Include/Library/BaseCryptLib.h
index 027ea09feb..dc6aaf0635 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -4,7 +4,7 @@
   primitives (Hash Serials, HMAC, RSA, Diffie-Hellman, etc) for UEFI security
   functionality enabling.
 
-Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 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
@@ -2177,7 +2177,7 @@ X509GetSubjectName (
   @param[in]  Cert Pointer to the DER-encoded X509 certificate.
   @param[in]  CertSize Size of the X509 certificate in bytes.
   @param[out] CommonName   Buffer to contain the retrieved certificate 
common
-   name string. At most CommonNameSize bytes 
will be
+   name string (UTF8). At most CommonNameSize 
bytes will be
written and the string will be null 
terminated. May be
NULL in order to determine the size buffer 
needed.
   @param[in,out]  CommonNameSize   The size in bytes of the CommonName buffer 
on input,
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c 
b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
index 56e66308ae..c137df357f 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -1,7 +1,7 @@
 /** @file
   X.509 Certificate Handler Wrapper Implementation over OpenSSL.
 
-Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 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
@@ -303,7 +303,7 @@ _Exit:
   @param[in]  Cert Pointer to the DER-encoded X509 certificate.
   @param[in]  CertSize Size of the X509 certificate in bytes.
   @param[out] CommonName   Buffer to contain the retrieved certificate 
common
-   name string. At most CommonNameSize bytes 
will be
+   name string (UTF8). At most CommonNameSize 
bytes will be
written and the string will be null 
terminated. May be
NULL in order to determine the size buffer 
needed.
   @param[in,out]  CommonNameSize   The size in bytes of the CommonName buffer 
on input,
@@ -332,13 +332,18 @@ X509GetCommonName (
   IN OUT  UINTN*CommonNameSize
   )
 {
-  RETURN_STATUS  ReturnStatus;
-  BOOLEANStatus;
-  X509   *X509Cert;
-  X509_NAME  *X509Name;
-  INTN   Length;
+  RETURN_STATUSReturnStatus;
+  BOOLEAN  Status;
+  X509 *X509Cert;
+  X509_NAME*X509Name;
+  INT32Index;
+  INTN Length;
+  X509_NAME_ENTRY  *Entry;
+  ASN1_STRING  *EntryData;
+  UINT8*UTF8Name;
 
   ReturnStatus = RETURN_INVALID_PARAMETER;
+  UTF8Name = NULL;
 
   //
   // Check input parameters.
@@ -378,8 +383,8 @@ X509GetCommonName (
   //
   // Retrieve the CommonName information from X.509 Subject
   //
-  Length = (INTN) X509_NAME_get_text_by_NID (X509Name, NID_commonName, 
CommonName, (int)(*CommonNameSize));
-  if (Length < 0) {
+  Index = X509_NAME_get_index_by_NID (X509Name, NID_commonName, -1);
+  if (Index < 0) {
 //
 // No CommonName entry exists in X509_NAME object
 //
@@ -388,10 +393,35 @@ X509GetCommonName (
 goto _Exit;
   }
 
-  *CommonNameSize = (UINTN)(Length + 

Re: [edk2] [PATCH v2 2/7] OvmfPkg: introduce PciCapPciSegmentLib

[Ard Biesheuvel]

On 23 May 2018 at 22:21, Laszlo Ersek  wrote:
> Add a library class, and a BASE lib instance, that are layered on top of
> PciCapLib, and allow clients to plug a PciSegmentLib backend into
> PciCapLib, for config space access.
>
> (Side note:
>

For the record: I am fine with keeping this side note in the commit log.

> The "MaxDomain" parameter is provided because, in practice, platforms
> exist where a PCI Express device may show up on a root bridge such that
> the root bridge doesn't support access to extended config space. Earlier
> the same issue was handled for MdeModulePkg/PciHostBridgeDxe in commit
> 014b472053ae3. However, that solution does not apply to the PciSegmentLib
> class, because:
>
> (1) The config space accessor functions of the PciSegmentLib class, such
> as PciSegmentReadBuffer(), have no way of informing the caller whether
> access to extended config space actually succeeds.
>
> (For example, in the UefiPciSegmentLibPciRootBridgeIo instace, which
> could in theory benefit from commit 014b472053ae3, the
> EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL.Pci.Read() status code is explicitly
> ignored, because there's no way for the lib instance to propagate it
> to the PciSegmentLib caller. If the
> EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL.Pci.Read() call fails, then
> DxePciSegmentLibPciRootBridgeIoReadWorker() returns Data with
> indeterminate value.)
>
> (2) There is no *general* way for any firmware platform to provide, or
> use, a PciSegmentLib instance in which access to extended config space
> always succeeds.
>
> In brief, on a platform where config space may be limited to 256 bytes,
> access to extended config space through PciSegmentLib may invoke undefined
> behavior; therefore PciCapPciSegmentLib must give platforms a way to
> prevent such access.)
>
> Cc: Ard Biesheuvel 
> Cc: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 
> ---
>
> Notes:
> v2:
> - move library from MdePkg to OvmfPkg, for initial introduction
>
>  OvmfPkg/OvmfPkg.dec |   5 +
>  OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf |  35 +++
>  OvmfPkg/Include/Library/PciCapPciSegmentLib.h   |  82 
> +++
>  OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.h   |  47 
> 
>  OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.c   | 226 
> 
>  5 files changed, 395 insertions(+)
>
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index 74818a2e2a19..fbec1cfe4a8e 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -35,6 +35,11 @@ [LibraryClasses]
>#  config space.
>PciCapLib|Include/Library/PciCapLib.h
>
> +  ##  @libraryclass  Layered on top of PciCapLib, allows clients to plug a
> +  #  PciSegmentLib backend into PciCapLib, for config space
> +  #  access.
> +  PciCapPciSegmentLib|Include/Library/PciCapPciSegmentLib.h
> +
>##  @libraryclass  Access QEMU's firmware configuration interface
>#
>QemuFwCfgLib|Include/Library/QemuFwCfgLib.h
> diff --git 
> a/OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf 
> b/OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> new file mode 100644
> index ..e3cf5de49b15
> --- /dev/null
> +++ b/OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> @@ -0,0 +1,35 @@
> +## @file
> +# Plug a PciSegmentLib backend into PciCapLib, for config space access.
> +#
> +# Copyright (C) 2018, Red Hat, Inc.
> +#
> +# This program and the accompanying materials are licensed and made available
> +# under the terms and conditions of the BSD License which accompanies this
> +# distribution.  The full text of the license may be found at
> +# http://opensource.org/licenses/bsd-license.php
> +#
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, 
> WITHOUT
> +# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +##
> +
> +[Defines]
> +  INF_VERSION= 1.27
> +  BASE_NAME  = BasePciCapPciSegmentLib
> +  FILE_GUID  = ED011855-AA31-43B9-ACC0-BF45A05C5985
> +  MODULE_TYPE= BASE
> +  VERSION_STRING = 1.0
> +  LIBRARY_CLASS  = PciCapPciSegmentLib
> +
> +[Sources]
> +  BasePciCapPciSegmentLib.h
> +  BasePciCapPciSegmentLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  MemoryAllocationLib
> +  PciSegmentLib
> diff --git a/OvmfPkg/Include/Library/PciCapPciSegmentLib.h 
> b/OvmfPkg/Include/Library/PciCapPciSegmentLib.h
> new file mode 100644
> index ..6b6930288d16
> --- /dev/null
> +++ b/OvmfPkg/Include/Library/PciCapPciSegmentLib.h
> @@ -0,0 +1,82 @@
> +/** @file
> +  Library class layered on top of 

Re: [edk2] [PATCH v2 1/7] OvmfPkg: introduce PciCapLib

[Ard Biesheuvel]

On 23 May 2018 at 22:21, Laszlo Ersek  wrote:
> Add a library class, and a BASE lib instance, to work more easily with PCI
> capabilities in PCI config space. Functions are provided to parse
> capabilities lists, and to locate, describe, read and write capabilities.
> PCI config space access is abstracted away.
>
> Cc: Ard Biesheuvel 
> Cc: Jordan Justen 
> Suggested-by: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 
> ---
>
> Notes:
> v2:
> - move library from MdePkg to OvmfPkg, for initial introduction
>
>  OvmfPkg/OvmfPkg.dec |4 +
>  OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf |   38 +
>  OvmfPkg/Include/Library/PciCapLib.h |  429 +
>  OvmfPkg/Library/BasePciCapLib/BasePciCapLib.h   |   60 ++
>  OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c   | 1007 
>  5 files changed, 1538 insertions(+)
>
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index c01a2ca7219a..74818a2e2a19 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -31,6 +31,10 @@ [LibraryClasses]
>#
>NvVarsFileLib|Include/Library/NvVarsFileLib.h
>
> +  ##  @libraryclass  Provides services to work with PCI capabilities in PCI
> +  #  config space.
> +  PciCapLib|Include/Library/PciCapLib.h
> +
>##  @libraryclass  Access QEMU's firmware configuration interface
>#
>QemuFwCfgLib|Include/Library/QemuFwCfgLib.h
> diff --git a/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf 
> b/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
> new file mode 100644
> index ..9a7428a589c2
> --- /dev/null
> +++ b/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
> @@ -0,0 +1,38 @@
> +## @file
> +# Work with PCI capabilities in PCI config space.
> +#
> +# Provides functions to parse capabilities lists, and to locate, describe, 
> read
> +# and write capabilities. PCI config space access is abstracted away.
> +#
> +# Copyright (C) 2018, Red Hat, Inc.
> +#
> +# This program and the accompanying materials are licensed and made available
> +# under the terms and conditions of the BSD License which accompanies this
> +# distribution.  The full text of the license may be found at
> +# http://opensource.org/licenses/bsd-license.php
> +#
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, 
> WITHOUT
> +# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +##
> +
> +[Defines]
> +  INF_VERSION= 1.27
> +  BASE_NAME  = BasePciCapLib
> +  FILE_GUID  = 6957540D-F7B5-4D5B-BEE4-FC14114DCD3C
> +  MODULE_TYPE= BASE
> +  VERSION_STRING = 1.0
> +  LIBRARY_CLASS  = PciCapLib
> +
> +[Sources]
> +  BasePciCapLib.h
> +  BasePciCapLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +
> +[LibraryClasses]
> +  BaseMemoryLib
> +  DebugLib
> +  MemoryAllocationLib
> +  OrderedCollectionLib
> diff --git a/OvmfPkg/Include/Library/PciCapLib.h 
> b/OvmfPkg/Include/Library/PciCapLib.h
> new file mode 100644
> index ..22a1ad624bd3
> --- /dev/null
> +++ b/OvmfPkg/Include/Library/PciCapLib.h
> @@ -0,0 +1,429 @@
> +/** @file
> +  Library class to work with PCI capabilities in PCI config space.
> +
> +  Provides functions to parse capabilities lists, and to locate, describe, 
> read
> +  and write capabilities. PCI config space access is abstracted away.
> +
> +  Copyright (C) 2018, Red Hat, Inc.
> +
> +  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 __PCI_CAP_LIB_H__
> +#define __PCI_CAP_LIB_H__
> +
> +#include 
> +
> +//
> +// Base structure for representing a PCI device -- down to the PCI function
> +// level -- for the purposes of this library class. This is a forward
> +// declaration that is completed below. Concrete implementations are supposed
> +// to inherit and extend this type.
> +//
> +typedef struct PCI_CAP_DEV PCI_CAP_DEV;
> +
> +/**
> +  Read the config space of a given PCI device (both normal and extended).
> +
> +  PCI_CAP_DEV_READ_CONFIG performs as few config space accesses as possible
> +  (without attempting 64-bit wide accesses).
> +
> +  PCI_CAP_DEV_READ_CONFIG returns an unspecified error if accessing Size 
> bytes
> +  from SourceOffset exceeds the config space limit of the PCI device. Fewer
> +  than Size bytes may have been read in this case.
> +
> +  @param[in] PciDevice   Implementation-specific unique 
> representation
> +