Re: [edk2] How to correctly sign EFI Firmware Volume?

2018-10-02 Thread Andrew Fish
Petr,

Mike Kinney and I just had an interesting conversation that relates to your 
question. 

It has to do with the FV being a file system and not really the FLASH layout. 
In the UEFI PI Spec and edk2 lingo you produce and FD (Flash Device) that 
consists of a set of FVs. There is not a standard way to discover the FVs in an 
FD, and this is something that is missing in the PI Spec. On most platforms 
there are multiple FVs so there should be a defined signing scheme for the 
entire FD. 

> On Oct 2, 2018, at 2:12 PM, Petr Vandrovec  wrote:
> 
> Hi,
>  I've sent this ot fw_os_forum, and was redirected here.  Sorry if you are 
> receiving this twice.
> 
> 
> I'm looking at options how to sign our EFI firmware through some 
> industry-standard embedded signature option, and signing whole firmware 
> volume as described in Platform Initialization spec would definitely fit the 
> bill.
> 
> Unfortunately problem is that I cannot make sense of what should be actually 
> signed.  Chapter 3.2.1.1 of PI_Spec_1_6.pdf says:
> 
> 
> 3.2.1.1 EFI Signed Firmware Volumes
> 
> There may be one or more headers with a FormatType of value 
> EFI_FIRMWARE_CONTENTS_SIGNED_GUID.
> 
> A signed firmware volume is a cryptographic signature across the entire 
> volume. To process the contents and verify the integrity of the volume, the 
> EFI_FIRMWARE_VOLUME_EXT_ENTRY_GUID_TYPE Data[] shall contain an instance of 
> WIN_CERTIFICATE_UEFI_GUID where the CertType = EFI_CERT_TYPE_PKCS7_GUID or 
> EFI_CERT_TYPE_RSA2048_SHA256_GUID.
> 
> 
> Part about WIN_CERTIFICATE_UEFI_GUID is easy.  But what should be signed?
> 
> Text says 'A signed firmware volume is a cryptographic signature across the 
> entire volume.' - beside that 'firmware volume' is not a signature, what is 
> 'the entire volume' ?  Clearly Data[] entry holding signature cannot be part 
> of the signature, as otherwise adding signature would invalidate that very 
> same signature, so it cannot be signature of entire volume from first 16 
> reserved bytes in the header to the last byte of the image, but something 
> else.
> 
> Can someone provide clarification what should be signed?  It seems to me like 
> that intention is to only sign data portion of the volume, from the end of 
> extended header to the end of volume.  But that means that anyone can modify 
> anything in the header or extended header without breaking signature.
> 
> Are there any examples of signed firmware volumes?  Unfortunately there does 
> not seem to be any code in UDK for this feature.
> 
> 
> On fw_os_forum I got recommendation to use EFI capsule format for signing.
> 
> Unfortunately I cannot figure out how to make out-of-band signatures work for 
> firmware volumes in a secure way: firmware module has to be multiple of (at 
> least) 4KB, and must cover last 16 bytes of ROM (as that is where execution 
> starts).  Then I need to prepend capsule header (or wrapping firmware volume 
> header) and signature in front of that.  Dual SHA1/256 signing with 
> timestamps takes about 5KB, so there are 3KB of free unsigned space left.
> 
> If I leave those 3KB unsigned, anybody can remove them, shift signed image 
> down by 3KB, and then have 3KB of unsigned code running at the reset vector 
> :-(

If some one can write your FLASH device they can just skip checking the result 
of your HASH, no need to shift things about. 

A really secure boot usually requires a mask ROM that checks the NOR FLASH. 
Something akin to Intel Boot Guard. 

Thanks,

Andrew Fish

> 
> Or I can do trial signing, figure out how long signature will probably be, 
> and then extend signed area so that only capsule header and signature 
> unsigned.  That could work, but then I'm not signing firmware volume, but 
> firmware volume with 3KB of data prepended to it (or firmware volume that is 
> not multiple of 4KB, if I let our firmware volume to have arbitrary size), 
> which is not exactly industry standard.
> 
> And even if I do this, as image is dual signed, someone can remove SHA1 
> signature, shift rest down, and get about 1KB for the malicious code.
> 
> So for all non-embedded signatures I'm always coming up with  standard thing> and require that signed payload ends with end of ROM, while 
> I'm looking for just , without strings attached.
> 
> 
> Thanks,
> Petr Vandrovec
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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


[edk2] Changes to Intel contribution model

2018-10-02 Thread Doran, Mark
Hi Everyone:

The way Intel's BIOS development community is organized has changed recently.  
This change presents opportunities to rethink some of the ways that we interact 
with the open source community around BIOS and this project in particular.  In 
general, I believe Intel is interested in moving towards a more mainstream open 
source project model of interaction with this community.

As a first step in that process, I'd like to let you know that Intel will 
shortly change the licensing for all our EDK II project contributions.  The 
existing license, composed of Contributor Agreement and BSD terms, will remain. 
 However we will be adding Apache 2.0 terms as a dual license option for all 
contributions that we have made wherever practical to do so.

In practice, these two license formulations are effectively equivalent for both 
contributors and consumers of code from the project.  The value in making this 
change is that it means the affected code will be licensed with an OSI approved 
set of license terms.  This should among other things make it easier for 
participants to get approval to work and contribute in the project and will 
allow us to get rid of the hurdle of a formal agreement to the Contributor 
terms as a precursor to doing any work.

Longer term, I'd like to invite all contributors to consider doing something 
similar with their contributions.  And beyond that, perhaps taking the lead of 
other projects like OpenSSL that have done this, converge on Apache 2.0 as the 
single, preferred license for the code in the project.  That of course is a 
decision for the community rather than any one participant.

With the latter thought in mind, I'd also like to invite discussion of what 
changes might be made to the project to make it function and present as a more 
recognizable mainstream open source project.  I think the changes at Intel 
create an opportunity for us to take on board community feedback, much of which 
we have heard but heretofore not necessarily been able to enact.  In other 
words we are at a point where Intel is willing to make substantive changes to 
its working model with the project but we would like to engage the community to 
decide together what a better, stronger upstream project could look like going 
forward.

I've talked to some of you about these ideas and the proposal right now would 
be to set up some open phone meeting conversations about what aspirations we 
might collectively agree for the project.  I think we will need more than one 
to accommodate schedules and time zones since we have a pretty well spread out 
group of participants.  Watch for logistics proposals on when those 
conversations can take place.  Ideally I'd like to try and get those rolling 
within the next week or two at most.  I'd ask everyone to think about things 
you'd like to see done differently in the EDK II project, be it tree changes, 
tools and process changes, actual code changes or anything else and be ready to 
bring those forward for discussion.  I did talk to the stewards already and I 
know they have a few ideas along these lines already so there's already some 
seeds for the discussion there.

Since this is not exactly a normal thread of discussion for this mailing list, 
I'd suggest that we not initially engage in extensive discussion on this via 
the email reflector - rather let's find those times to talk about ideas in real 
time by phone meeting as I think that will be more efficient to share 
brainstorming ideas. And I don't want to disrupt the work flow here in the 
meanwhile! (well, not any more than I just did perhaps ;))

If you have questions about the above that can't wait for the broader inclusive 
discussions, you are welcome to reply to me direct of course.  I'm really 
looking forward to conversations about how to make a better upstream that will 
benefit our entire ecosystem!
--
Cheers,

Mark.

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


[edk2] How to correctly sign EFI Firmware Volume?

2018-10-02 Thread Petr Vandrovec

Hi,
  I've sent this ot fw_os_forum, and was redirected here.  Sorry if you 
are receiving this twice.



I'm looking at options how to sign our EFI firmware through some 
industry-standard embedded signature option, and signing whole firmware 
volume as described in Platform Initialization spec would definitely fit 
the bill.


Unfortunately problem is that I cannot make sense of what should be 
actually signed.  Chapter 3.2.1.1 of PI_Spec_1_6.pdf says:



3.2.1.1 EFI Signed Firmware Volumes

There may be one or more headers with a FormatType of value 
EFI_FIRMWARE_CONTENTS_SIGNED_GUID.


A signed firmware volume is a cryptographic signature across the entire 
volume. To process the contents and verify the integrity of the volume, 
the EFI_FIRMWARE_VOLUME_EXT_ENTRY_GUID_TYPE Data[] shall contain an 
instance of WIN_CERTIFICATE_UEFI_GUID where the CertType = 
EFI_CERT_TYPE_PKCS7_GUID or EFI_CERT_TYPE_RSA2048_SHA256_GUID.



Part about WIN_CERTIFICATE_UEFI_GUID is easy.  But what should be signed?

Text says 'A signed firmware volume is a cryptographic signature across 
the entire volume.' - beside that 'firmware volume' is not a signature, 
what is 'the entire volume' ?  Clearly Data[] entry holding signature 
cannot be part of the signature, as otherwise adding signature would 
invalidate that very same signature, so it cannot be signature of entire 
volume from first 16 reserved bytes in the header to the last byte of 
the image, but something else.


Can someone provide clarification what should be signed?  It seems to me 
like that intention is to only sign data portion of the volume, from the 
end of extended header to the end of volume.  But that means that anyone 
can modify anything in the header or extended header without breaking 
signature.


Are there any examples of signed firmware volumes?  Unfortunately there 
does not seem to be any code in UDK for this feature.



On fw_os_forum I got recommendation to use EFI capsule format for signing.

Unfortunately I cannot figure out how to make out-of-band signatures 
work for firmware volumes in a secure way: firmware module has to be 
multiple of (at least) 4KB, and must cover last 16 bytes of ROM (as that 
is where execution starts).  Then I need to prepend capsule header (or 
wrapping firmware volume header) and signature in front of that.  Dual 
SHA1/256 signing with timestamps takes about 5KB, so there are 3KB of 
free unsigned space left.


If I leave those 3KB unsigned, anybody can remove them, shift signed 
image down by 3KB, and then have 3KB of unsigned code running at the 
reset vector :-(


Or I can do trial signing, figure out how long signature will probably 
be, and then extend signed area so that only capsule header and 
signature unsigned.  That could work, but then I'm not signing firmware 
volume, but firmware volume with 3KB of data prepended to it (or 
firmware volume that is not multiple of 4KB, if I let our firmware 
volume to have arbitrary size), which is not exactly industry standard.


And even if I do this, as image is dual signed, someone can remove SHA1 
signature, shift rest down, and get about 1KB for the malicious code.


So for all non-embedded signatures I'm always coming up with standard thing> and require that signed payload ends with end of ROM, 
while I'm looking for just , without strings 
attached.



Thanks,
Petr Vandrovec

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


Re: [edk2] [PATCH] Maintainers.txt: add DynamicTablesPkg

2018-10-02 Thread Laszlo Ersek
On 10/02/18 18:03, Leif Lindholm wrote:
> DynamicTablesPkg has been in edk2-staging[1] for some time now, and it is
> time for it to move into the main tree.
> [1] https://github.com/tianocore/edk2-staging/tree/dynamictables
> 
> Add Evan and Sami as maintainers of the new package, and let them bring
> it in themselves.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Leif Lindholm 
> ---
> 
> We'll need Reviewed-by: from both new maintainers.
> Github write permissions need to be added separately.
> 
> Please add some documentation at the specified URL before
> you import any code :)
> 
>  Maintainers.txt | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 7ebd53f662..072cf3ce8f 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -105,6 +105,11 @@ W: 
> https://github.com/tianocore/tianocore.github.io/wiki/DuetPkg
>  M: Ruiyu Ni 
>  M: Hao Wu 
>  
> +DynamicTablesPkg
> +W: https://github.com/tianocore/tianocore.github.io/wiki/DynamicTablesPkg
> +M: Evan Lloyd 
> +M: Sami Mujawar 
> +
>  EdkCompatibilityPkg
>  W: https://github.com/tianocore/tianocore.github.io/wiki/EdkCompatibilityPkg
>  M: Liming Gao 
> 

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


[edk2] [PATCH] Maintainers.txt: add DynamicTablesPkg

2018-10-02 Thread Leif Lindholm
DynamicTablesPkg has been in edk2-staging[1] for some time now, and it is
time for it to move into the main tree.
[1] https://github.com/tianocore/edk2-staging/tree/dynamictables

Add Evan and Sami as maintainers of the new package, and let them bring
it in themselves.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leif Lindholm 
---

We'll need Reviewed-by: from both new maintainers.
Github write permissions need to be added separately.

Please add some documentation at the specified URL before
you import any code :)

 Maintainers.txt | 5 +
 1 file changed, 5 insertions(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index 7ebd53f662..072cf3ce8f 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -105,6 +105,11 @@ W: 
https://github.com/tianocore/tianocore.github.io/wiki/DuetPkg
 M: Ruiyu Ni 
 M: Hao Wu 
 
+DynamicTablesPkg
+W: https://github.com/tianocore/tianocore.github.io/wiki/DynamicTablesPkg
+M: Evan Lloyd 
+M: Sami Mujawar 
+
 EdkCompatibilityPkg
 W: https://github.com/tianocore/tianocore.github.io/wiki/EdkCompatibilityPkg
 M: Liming Gao 
-- 
2.11.0

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


Re: [edk2] [PATCH v3 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-02 Thread Laszlo Ersek
On 10/02/18 14:17, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> This is for conformance with the TCG "Platform Reset Attack Mitigation
> Specification". Because clearing the CPU caches at boot doesn't impact
> performance significantly, do it unconditionally, for simplicity's
> sake.
> 
> Flush the cache on all logical processors, thanks to
> EFI_PEI_MP_SERVICES_PPI and CacheMaintenanceLib.
> 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Anthony Perard 
> Cc: Julien Grall 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau 
> Message-Id: <20181002120730.13013-1-marcandre.lur...@redhat.com>

I'm going to remove the Message-Id line on push. I don't know why it's
there, but either way, it's not correct. (The msgid of your posting is
<20181002121725.17178-1-marcandre.lur...@redhat.com>.)

With that:

Reviewed-by: Laszlo Ersek 

Also,

Regression-tested-by: Laszlo Ersek 

I'm ready to push this, but I'd still like to get an A-b from Mike (CC'd).

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


Re: [edk2] [PATCH v3 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-02 Thread Laszlo Ersek
On 10/02/18 15:27, Anthony PERARD wrote:
> On Tue, Oct 02, 2018 at 04:17:25PM +0400, marcandre.lur...@redhat.com wrote:
>> From: Marc-André Lureau 
>>
>> This is for conformance with the TCG "Platform Reset Attack Mitigation
>> Specification". Because clearing the CPU caches at boot doesn't impact
>> performance significantly, do it unconditionally, for simplicity's
>> sake.
>>
>> Flush the cache on all logical processors, thanks to
>> EFI_PEI_MP_SERVICES_PPI and CacheMaintenanceLib.
>>
>> Cc: Jordan Justen 
>> Cc: Laszlo Ersek 
>> Cc: Ard Biesheuvel 
>> Cc: Anthony Perard 
>> Cc: Julien Grall 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marc-André Lureau 
> 
> That patch works for me on Xen. I can still boot guests with the patch
> applied, with either 1 or 4 vcpus assigned to the guest:
> 
> Tested-by: Anthony PERARD 

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


Re: [edk2] [PATCH] BaseTools: Allow multiple PciDeviceId in Fdf OptionROM override

2018-10-02 Thread Laszlo Ersek
+Yonghong, +Liming

On 10/02/18 16:46, Tomas Pilar (tpilar) wrote:
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Tomas Pilar 
> ---
>  BaseTools/Source/Python/GenFds/FdfParser.py | 11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/GenFds/FdfParser.py 
> b/BaseTools/Source/Python/GenFds/FdfParser.py
> index 63687e98bb..a65f2cfd2d 100644
> --- a/BaseTools/Source/Python/GenFds/FdfParser.py
> +++ b/BaseTools/Source/Python/GenFds/FdfParser.py
> @@ -4469,10 +4469,15 @@ class FdfParser:
>  if self.__IsKeyword( "PCI_DEVICE_ID"):
>  if not self.__IsToken( "="):
>  raise Warning("expected '='", self.FileName, 
> self.CurrentLineNumber)
> -if not self.__GetNextHexNumber():
> -raise Warning("expected Hex device id", 
> self.FileName, self.CurrentLineNumber)
>  
> -Overrides.PciDeviceId = self.__Token
> +# Get a list of PCI IDs
> +Overrides.PciDeviceId = ""
> +
> +while self.__GetNextHexNumber():
> +Overrides.PciDeviceId += " " + self.__Token
> +
> +if not Overrides.PciDeviceId:
> +raise Warning("expected one or more Hex device ids", 
> self.FileName, self.CurrentLineNumber)
>  continue
>  
>  if self.__IsKeyword( "PCI_REVISION"):
> 

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


[edk2] [PATCH] BaseTools: Allow multiple PciDeviceId in Fdf OptionROM override

2018-10-02 Thread Tomas Pilar (tpilar)
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Tomas Pilar 
---
 BaseTools/Source/Python/GenFds/FdfParser.py | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/FdfParser.py 
b/BaseTools/Source/Python/GenFds/FdfParser.py
index 63687e98bb..a65f2cfd2d 100644
--- a/BaseTools/Source/Python/GenFds/FdfParser.py
+++ b/BaseTools/Source/Python/GenFds/FdfParser.py
@@ -4469,10 +4469,15 @@ class FdfParser:
 if self.__IsKeyword( "PCI_DEVICE_ID"):
 if not self.__IsToken( "="):
 raise Warning("expected '='", self.FileName, 
self.CurrentLineNumber)
-if not self.__GetNextHexNumber():
-raise Warning("expected Hex device id", self.FileName, 
self.CurrentLineNumber)
 
-Overrides.PciDeviceId = self.__Token
+# Get a list of PCI IDs
+Overrides.PciDeviceId = ""
+
+while self.__GetNextHexNumber():
+Overrides.PciDeviceId += " " + self.__Token
+
+if not Overrides.PciDeviceId:
+raise Warning("expected one or more Hex device ids", 
self.FileName, self.CurrentLineNumber)
 continue
 
 if self.__IsKeyword( "PCI_REVISION"):
-- 
2.17.1

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


Re: [edk2] Problems with FDF for OptionROM when using GCC

2018-10-02 Thread Tomas Pilar (tpilar)
So the solution turns out to swap the Compress and Guided around, Compress 
apparently requires a LeafSection while Guided is happy to contain 
EncapSection. Odd.

Anyway, I had to hack up GenFds to get a sensible python backtrace:

  1 diff --git a/BaseTools/Source/Python/GenFds/GenFds.py 
b/BaseTools/Source/Python/GenFds/GenFds.py
  2 index 9dec9c5eb5..adcadce5a5 100644
  3 --- a/BaseTools/Source/Python/GenFds/GenFds.py
  4 +++ b/BaseTools/Source/Python/GenFds/GenFds.py
  5 @@ -348,6 +348,7 @@ def main():
  6  ReturnCode = X.args[0]
  7  except:
  8  import traceback
  9 +    raise^M
 10  EdkLogger.error(
 11  "\nPython",
 12  CODE_ERROR,

to get

Traceback (most recent call last):
  File "/usr/lib64/python2.7/runpy.py", line 174, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib64/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/local/sauce/uefi/BuildTree/BaseTools/Source/Python/GenFds/GenFds.py", 
line 736, in 
    r = main()
  File "/local/sauce/uefi/BuildTree/BaseTools/Source/Python/GenFds/GenFds.py", 
line 333, in main
    GenFds.GenFd('', FdfParserObj, BuildWorkSpace, ArchList)
  File "/local/sauce/uefi/BuildTree/BaseTools/Source/Python/GenFds/GenFds.py", 
line 473, in GenFd
    OptRomObj.AddToBuffer(None)
  File 
"/home/tp/sauce/uefi/BuildTree/BaseTools/Source/Python/GenFds/OptionRom.py", 
line 62, in AddToBuffer
    FilePathNameList = FfsFile.GenFfs(IsMakefile=Flag)
  File 
"/home/tp/sauce/uefi/BuildTree/BaseTools/Source/Python/GenFds/OptRomInfStatement.py",
 line 105, in GenFfs
    EfiOutputList = self.__GenComplexFileSection__(Rule, IsMakefile=IsMakefile)
  File 
"/home/tp/sauce/uefi/BuildTree/BaseTools/Source/Python/GenFds/OptRomInfStatement.py",
 line 143, in __GenComplexFileSection__
    if Sect.SectionType == BINARY_FILE_TYPE_PE32:
AttributeError: CompressSection instance has no attribute 'SectionType'

which is a much more helpful backtrace than what the logger was producing.

I assume I have misconfigured my EdkLogger somehow, or forgot some runes? Has 
anyone else had a similar experience?

Cheers,
Tom

On 02/10/18 14:09, Tomas Pilar (tpilar) wrote:
> Hi,
>
> I am trying to move the OptionROM flash specification in my package to a FDF 
> file. I use the following snippet to define the OptionROM:
>
> [Defines]
>   FDF_VERSION = 0x0100
>   FDF_SPECIFICATION = 0x0001001B
>  
> [OptionROM.SfcNicDriver]
>   INF USE = $(ARCH) SfcPkg/SfcNicDriver/SfcNicDriver.inf {
>     PCI_VENDOR_ID = 0x1924
>     PCI_DEVICE_ID = 0x0903
>     PCI_CLASS_CODE    = 0x02 # Network Controller
>     PCI_REVISION  = 0x00 # Vendor defined Revision ID
>     PCI_COMPRESS  = TRUE
>   }
>
> which results in a
>
> error F003: Don't Find common rule RULE.COMMON.UEFI_DRIVER for INF 
> SfcPkg/SfcNicDriver/SfcNicDriver.inf
>
> so then I cargo cult copy an example rule from FDF:
>
> [Rule.Common.UEFI_DRIVER]
> FILE DRIVER = $(NAMED_GUID) {
>   COMPRESS PI_STD {
>     GUIDED {
>   PE32 PE32 $(INF_OUTPUT)/$(MODULE_NAME).efi
>   UI STRING = "$(MODULE_NAME)" Optional
>   VERSION STRING = "$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
>     }
>   }
> }
>
> which results in the following error:
>
> GenFds.py...
>  : error C0DE: Tools code failure
>     Please send email to edk2-devel@lists.01.org for help, attaching 
> following call stack trace!
>
> Traceback (most recent call last):
>   File "/usr/lib64/python2.7/runpy.py", line 174, in _run_module_as_main
>     "__main__", fname, loader, pkg_name)
>   File "/usr/lib64/python2.7/runpy.py", line 72, in _run_code
>     exec code in run_globals
>   File 
> "/local/sauce/uefi/BuildTree/BaseTools/Source/Python/GenFds/GenFds.py", line 
> 735, in 
>     r = main()
>   File 
> "/local/sauce/uefi/BuildTree/BaseTools/Source/Python/GenFds/GenFds.py", line 
> 356, in main
>     RaiseError=False
>   File 
> "/home/tp/sauce/uefi/BuildTree/BaseTools/Source/Python/Common/EdkLogger.py", 
> line 203, in error
>     raise FatalError(ErrorCode)
> Common.BuildToolError.FatalError: 49374
>
>
> build.py...
>  : error 7000: Failed to execute command
>     GenFds -f /home/tp/sauce/uefi/BuildTree/SfcPkg/SfcPkg.fdf 
> --conf=/home/tp/sauce/uefi/BuildTree/Conf -o 
> /home/tp/sauce/uefi/BuildTree/Build/SfcPkg/DEBUG_GCC49 -t GCC49 -b DEBUG -p 
> /home/tp/sauce/uefi/BuildTree/SfcPkg/SfcPkg.dsc -a X64 -D 
> "EFI_SOURCE=/home/tp/sauce/uefi/BuildTree/EdkCompatibilityPkg" -D 
> "EDK_SOURCE=/home/tp/sauce/uefi/BuildTree/EdkCompatibilityPkg" -D 
> "TOOL_CHAIN_TAG=GCC49" -D "TOOLCHAIN=GCC49" -D "TARGET=DEBUG" -D "FAMILY=GCC" 
> -D "WORKSPACE=/home/tp/sauce/uefi/BuildTree" -D 
> "EDK_TOOLS_PATH=/home/tp/sauce/uefi/BuildTree/BaseTools" -D "ARCH=X64" -D 
> "ECP_SOURCE=/home/tp/sauce/uefi/BuildTree/EdkCompatibilityPkg" 
> [/local/sauce/uefi/BuildTree]
>
> - F

Re: [edk2] [PATCH v3 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-02 Thread Anthony PERARD
On Tue, Oct 02, 2018 at 04:17:25PM +0400, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> This is for conformance with the TCG "Platform Reset Attack Mitigation
> Specification". Because clearing the CPU caches at boot doesn't impact
> performance significantly, do it unconditionally, for simplicity's
> sake.
> 
> Flush the cache on all logical processors, thanks to
> EFI_PEI_MP_SERVICES_PPI and CacheMaintenanceLib.
> 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Anthony Perard 
> Cc: Julien Grall 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau 

That patch works for me on Xen. I can still boot guests with the patch
applied, with either 1 or 4 vcpus assigned to the guest:

Tested-by: Anthony PERARD 

Thanks,

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


[edk2] Problems with FDF for OptionROM when using GCC

2018-10-02 Thread Tomas Pilar (tpilar)
Hi,

I am trying to move the OptionROM flash specification in my package to a FDF 
file. I use the following snippet to define the OptionROM:

[Defines]
  FDF_VERSION = 0x0100
  FDF_SPECIFICATION = 0x0001001B
 
[OptionROM.SfcNicDriver]
  INF USE = $(ARCH) SfcPkg/SfcNicDriver/SfcNicDriver.inf {
    PCI_VENDOR_ID = 0x1924
    PCI_DEVICE_ID = 0x0903
    PCI_CLASS_CODE    = 0x02 # Network Controller
    PCI_REVISION  = 0x00 # Vendor defined Revision ID
    PCI_COMPRESS  = TRUE
  }

which results in a

error F003: Don't Find common rule RULE.COMMON.UEFI_DRIVER for INF 
SfcPkg/SfcNicDriver/SfcNicDriver.inf

so then I cargo cult copy an example rule from FDF:

[Rule.Common.UEFI_DRIVER]
FILE DRIVER = $(NAMED_GUID) {
  COMPRESS PI_STD {
    GUIDED {
  PE32 PE32 $(INF_OUTPUT)/$(MODULE_NAME).efi
  UI STRING = "$(MODULE_NAME)" Optional
  VERSION STRING = "$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
    }
  }
}

which results in the following error:

GenFds.py...
 : error C0DE: Tools code failure
    Please send email to edk2-devel@lists.01.org for help, attaching 
following call stack trace!

Traceback (most recent call last):
  File "/usr/lib64/python2.7/runpy.py", line 174, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib64/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/local/sauce/uefi/BuildTree/BaseTools/Source/Python/GenFds/GenFds.py", 
line 735, in 
    r = main()
  File "/local/sauce/uefi/BuildTree/BaseTools/Source/Python/GenFds/GenFds.py", 
line 356, in main
    RaiseError=False
  File 
"/home/tp/sauce/uefi/BuildTree/BaseTools/Source/Python/Common/EdkLogger.py", 
line 203, in error
    raise FatalError(ErrorCode)
Common.BuildToolError.FatalError: 49374


build.py...
 : error 7000: Failed to execute command
    GenFds -f /home/tp/sauce/uefi/BuildTree/SfcPkg/SfcPkg.fdf 
--conf=/home/tp/sauce/uefi/BuildTree/Conf -o 
/home/tp/sauce/uefi/BuildTree/Build/SfcPkg/DEBUG_GCC49 -t GCC49 -b DEBUG -p 
/home/tp/sauce/uefi/BuildTree/SfcPkg/SfcPkg.dsc -a X64 -D 
"EFI_SOURCE=/home/tp/sauce/uefi/BuildTree/EdkCompatibilityPkg" -D 
"EDK_SOURCE=/home/tp/sauce/uefi/BuildTree/EdkCompatibilityPkg" -D 
"TOOL_CHAIN_TAG=GCC49" -D "TOOLCHAIN=GCC49" -D "TARGET=DEBUG" -D "FAMILY=GCC" 
-D "WORKSPACE=/home/tp/sauce/uefi/BuildTree" -D 
"EDK_TOOLS_PATH=/home/tp/sauce/uefi/BuildTree/BaseTools" -D "ARCH=X64" -D 
"ECP_SOURCE=/home/tp/sauce/uefi/BuildTree/EdkCompatibilityPkg" 
[/local/sauce/uefi/BuildTree]

- Failed -
Build end time: 14:07:26, Oct.02 2018
Build total time: 00:00:04

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


Re: [edk2] [PATCH v3 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-02 Thread Laszlo Ersek
On 10/02/18 14:17, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> This is for conformance with the TCG "Platform Reset Attack Mitigation
> Specification". Because clearing the CPU caches at boot doesn't impact
> performance significantly, do it unconditionally, for simplicity's
> sake.
> 
> Flush the cache on all logical processors, thanks to
> EFI_PEI_MP_SERVICES_PPI and CacheMaintenanceLib.
> 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Anthony Perard 
> Cc: Julien Grall 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau 
> Message-Id: <20181002120730.13013-1-marcandre.lur...@redhat.com>
> ---
> 
> v3:
>   - update top comment with notes about TCG spec
>   - sort headers inclusion

So, meta comments first...

I got this patch (v3) in three variants:


(a) the one you sent me off-list, as quoted-printable:

Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable


(b) this message, reaching me directly from you:

Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit


(c) this message, reflected by the edk2-devel list software, from you to me:

Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64


Results:
- git-am fails with (a).
- git-am *also* fails with (c).
- git-am only works on (b).

Which means that your setup is now correct, but some mail server on path
(c) is broken, and corrupts your patch email when it transcodes the
email to base64. Yay!

I guess I'll work with copy (b), in my inbox.

Sigh, is the mailing list workflow actually *more* broken than github
pull requests? I thought that was impossible, but I guess I'm being
proven wrong.

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


Re: [edk2] [PATCH v2 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-02 Thread Marc-André Lureau
Hi

On Tue, Oct 2, 2018 at 4:19 PM Laszlo Ersek  wrote:
>
> On 10/02/18 14:10, Marc-André Lureau wrote:
> > Hi
> >
> > On Tue, Oct 2, 2018 at 3:55 PM Laszlo Ersek  wrote:
> >>
> >> On 10/02/18 13:37, Marc-André Lureau wrote:
> >>> On Tue, Oct 2, 2018 at 2:55 PM Laszlo Ersek  wrote:
> >>
>  Please fix your git settings for your local edk2 clone. Your patch
>  contains context lines with LF (not CRLF) line endings, and that's not
>  correct for edk2.
> >>>
> >>> Shouldn't it be catched by: python BaseTools/Scripts/PatchCheck.py -1 ?
> >>
> >> Yes, it should be caught by "PatchCheck.py". The "force_crlf" member is
> >> set iff the pathname doesn't end in ".sh", and then the
> >> check_added_line() method verifies CRLF.
> >>
> >> I don't know why it doesn't work in practice. Can you submit a TianoCore
> >> BZ about it?
> >>
> >> (Your patch uses LF on both context lines and new (added) lines, so the
> >> current script logic should complain, yes.)
> >>
> >>> Hmm, this is weird, the original patch has crlf (see attach file).
> >>> send-email or the mail server somehow stripped it?
> >>>
> >>> I use your recommend git settings, I don't know what I am missing here.
> >>
> >> You are right, the attachment looks fine.
> >>
> >> ... Can you resend the v2 patch (just to me directly, off-list) with
> >>
> >>   --transfer-encoding=quoted-printable
> >>
> >> ? My guess is that the base64 encoding in git-send-email includes an
> >> automatic CR stripping phase.
> >
> > Hmm, actually it's my emacs settings that stripped the crlf from the patch.
> >
> > I edited the patch manually to add some notes...
>
> Haha, serves you right then ;)
>
> I suggest never editing patches after formatting them. In the first
> place, there's no reason to: you can add, edit and remove notes on
> commits with "git notes" (without changing the commit hashes / git
> history). And "git-format-patch" takes a "--notes" option. (Same for
> "git show".)
>
> I do mention git-notes in the "unkempt guide":
>
> git config notes.rewriteRef   refs/notes/commits
>
> git format-patch   \
>   --notes  \
>
> git notes edit COMMIT_HASH_OF_THAT_PATCH
>

Yes, somehow git-notes doesn't fit with my idea of how patch mail
notes for v2/v3... should look:

Notes:
v3:
 - foo

v2:
 - blablah

 If there is a way to avoid the "Notes:" quoting, that would be nice.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v2 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-02 Thread Laszlo Ersek
On 10/02/18 14:10, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Oct 2, 2018 at 3:55 PM Laszlo Ersek  wrote:
>>
>> On 10/02/18 13:37, Marc-André Lureau wrote:
>>> On Tue, Oct 2, 2018 at 2:55 PM Laszlo Ersek  wrote:
>>
 Please fix your git settings for your local edk2 clone. Your patch
 contains context lines with LF (not CRLF) line endings, and that's not
 correct for edk2.
>>>
>>> Shouldn't it be catched by: python BaseTools/Scripts/PatchCheck.py -1 ?
>>
>> Yes, it should be caught by "PatchCheck.py". The "force_crlf" member is
>> set iff the pathname doesn't end in ".sh", and then the
>> check_added_line() method verifies CRLF.
>>
>> I don't know why it doesn't work in practice. Can you submit a TianoCore
>> BZ about it?
>>
>> (Your patch uses LF on both context lines and new (added) lines, so the
>> current script logic should complain, yes.)
>>
>>> Hmm, this is weird, the original patch has crlf (see attach file).
>>> send-email or the mail server somehow stripped it?
>>>
>>> I use your recommend git settings, I don't know what I am missing here.
>>
>> You are right, the attachment looks fine.
>>
>> ... Can you resend the v2 patch (just to me directly, off-list) with
>>
>>   --transfer-encoding=quoted-printable
>>
>> ? My guess is that the base64 encoding in git-send-email includes an
>> automatic CR stripping phase.
> 
> Hmm, actually it's my emacs settings that stripped the crlf from the patch.
> 
> I edited the patch manually to add some notes...

Haha, serves you right then ;)

I suggest never editing patches after formatting them. In the first
place, there's no reason to: you can add, edit and remove notes on
commits with "git notes" (without changing the commit hashes / git
history). And "git-format-patch" takes a "--notes" option. (Same for
"git show".)

I do mention git-notes in the "unkempt guide":

git config notes.rewriteRef   refs/notes/commits

git format-patch   \
  --notes  \

git notes edit COMMIT_HASH_OF_THAT_PATCH


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


[edk2] [PATCH v3 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-02 Thread marcandre . lureau
From: Marc-André Lureau 

This is for conformance with the TCG "Platform Reset Attack Mitigation
Specification". Because clearing the CPU caches at boot doesn't impact
performance significantly, do it unconditionally, for simplicity's
sake.

Flush the cache on all logical processors, thanks to
EFI_PEI_MP_SERVICES_PPI and CacheMaintenanceLib.

Cc: Jordan Justen 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Anthony Perard 
Cc: Julien Grall 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc-André Lureau 
Message-Id: <20181002120730.13013-1-marcandre.lur...@redhat.com>
---

v3:
  - update top comment with notes about TCG spec
  - sort headers inclusion

 OvmfPkg/PlatformPei/PlatformPei.inf |   2 +
 OvmfPkg/PlatformPei/Platform.h  |   5 +
 OvmfPkg/PlatformPei/ClearCache.c| 117 
 OvmfPkg/PlatformPei/Platform.c  |   1 +
 4 files changed, 125 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
b/OvmfPkg/PlatformPei/PlatformPei.inf
index 9c5ad9961c4a..5c8dd0fe6d72 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -30,6 +30,7 @@
 
 [Sources]
   AmdSev.c
+  ClearCache.c
   Cmos.c
   Cmos.h
   FeatureControl.c
@@ -54,6 +55,7 @@
 
 [LibraryClasses]
   BaseLib
+  CacheMaintenanceLib
   DebugLib
   HobLib
   IoLib
diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
index f942e61bb4f9..b12a5c1f5f78 100644
--- a/OvmfPkg/PlatformPei/Platform.h
+++ b/OvmfPkg/PlatformPei/Platform.h
@@ -83,6 +83,11 @@ InstallFeatureControlCallback (
   VOID
   );
 
+VOID
+InstallClearCacheCallback (
+  VOID
+  );
+
 EFI_STATUS
 InitializeXen (
   VOID
diff --git a/OvmfPkg/PlatformPei/ClearCache.c b/OvmfPkg/PlatformPei/ClearCache.c
new file mode 100644
index ..7d15fd925c3c
--- /dev/null
+++ b/OvmfPkg/PlatformPei/ClearCache.c
@@ -0,0 +1,117 @@
+/**@file
+  Install a callback to clear cache on all processors.
+  This is for conformance with the TCG "Platform Reset Attack Mitigation
+  Specification". Because clearing the CPU caches at boot doesn't impact
+  performance significantly, do it unconditionally, for simplicity's
+  sake.
+
+  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.
+**/
+
+#include 
+#include 
+#include 
+#include 
+
+#include "Platform.h"
+
+/**
+  Invalidate data & instruction caches.
+  All APs execute this function in parallel. The BSP executes the function
+  separately.
+
+  @param[in,out] WorkSpace  Pointer to the input/output argument workspace
+shared by all processors.
+**/
+STATIC
+VOID
+EFIAPI
+ClearCache (
+  IN OUT VOID *WorkSpace
+  )
+{
+  WriteBackInvalidateDataCache ();
+  InvalidateInstructionCache ();
+}
+
+/**
+  Notification function called when EFI_PEI_MP_SERVICES_PPI becomes available.
+
+  @param[in] PeiServices  Indirect reference to the PEI Services Table.
+  @param[in] NotifyDescriptor Address of the notification descriptor data
+  structure.
+  @param[in] Ppi  Address of the PPI that was installed.
+
+  @return  Status of the notification. The status code returned from this
+   function is ignored.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+ClearCacheOnMpServicesAvailable (
+  IN EFI_PEI_SERVICES   **PeiServices,
+  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
+  IN VOID   *Ppi
+  )
+{
+  EFI_PEI_MP_SERVICES_PPI *MpServices;
+  EFI_STATUS  Status;
+
+  DEBUG ((DEBUG_INFO, "%a: %a\n", gEfiCallerBaseName, __FUNCTION__));
+
+  //
+  // Clear cache on all the APs in parallel.
+  //
+  MpServices = Ppi;
+  Status = MpServices->StartupAllAPs (
+ (CONST EFI_PEI_SERVICES **)PeiServices,
+ MpServices,
+ ClearCache,  // Procedure
+ FALSE,   // SingleThread
+ 0,   // TimeoutInMicroSeconds: inf.
+ NULL // ProcedureArgument
+ );
+  if (EFI_ERROR (Status) && Status != EFI_NOT_STARTED) {
+DEBUG ((DEBUG_ERROR, "%a: StartupAllAps(): %r\n", __FUNCTION__, Status));
+return Status;
+  }
+
+  //
+  // Now clear cache on the BSP too.
+  //
+  ClearCache (NULL);
+  return EFI_SUCCESS;
+}
+
+//
+// Notification object for registering the callback, for when
+// EFI_PEI_MP_SERVICES_PPI becomes available.
+//
+STATIC CONST EFI_PEI_NOTIFY_DESCRIPTOR mMpServicesNotify = {
+  EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | // Flags
+  EFI_PEI_PPI_DE

Re: [edk2] [PATCH v2 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-02 Thread Marc-André Lureau
Hi

On Tue, Oct 2, 2018 at 3:55 PM Laszlo Ersek  wrote:
>
> On 10/02/18 13:37, Marc-André Lureau wrote:
> > On Tue, Oct 2, 2018 at 2:55 PM Laszlo Ersek  wrote:
>
> >> Please fix your git settings for your local edk2 clone. Your patch
> >> contains context lines with LF (not CRLF) line endings, and that's not
> >> correct for edk2.
> >
> > Shouldn't it be catched by: python BaseTools/Scripts/PatchCheck.py -1 ?
>
> Yes, it should be caught by "PatchCheck.py". The "force_crlf" member is
> set iff the pathname doesn't end in ".sh", and then the
> check_added_line() method verifies CRLF.
>
> I don't know why it doesn't work in practice. Can you submit a TianoCore
> BZ about it?
>
> (Your patch uses LF on both context lines and new (added) lines, so the
> current script logic should complain, yes.)
>
> > Hmm, this is weird, the original patch has crlf (see attach file).
> > send-email or the mail server somehow stripped it?
> >
> > I use your recommend git settings, I don't know what I am missing here.
>
> You are right, the attachment looks fine.
>
> ... Can you resend the v2 patch (just to me directly, off-list) with
>
>   --transfer-encoding=quoted-printable
>
> ? My guess is that the base64 encoding in git-send-email includes an
> automatic CR stripping phase.

Hmm, actually it's my emacs settings that stripped the crlf from the patch.

I edited the patch manually to add some notes...

>
> If indeed bas64 is the culprit, I'll update the wiki article to specify
> quoted-printable.
>
> Thanks
> Laszlo
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v2 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-02 Thread Laszlo Ersek
On 10/02/18 13:55, Laszlo Ersek wrote:
> On 10/02/18 13:37, Marc-André Lureau wrote:
>> On Tue, Oct 2, 2018 at 2:55 PM Laszlo Ersek  wrote:
> 
>>> Please fix your git settings for your local edk2 clone. Your patch
>>> contains context lines with LF (not CRLF) line endings, and that's not
>>> correct for edk2.
>>
>> Shouldn't it be catched by: python BaseTools/Scripts/PatchCheck.py -1 ?
> 
> Yes, it should be caught by "PatchCheck.py". The "force_crlf" member is
> set iff the pathname doesn't end in ".sh", and then the
> check_added_line() method verifies CRLF.
> 
> I don't know why it doesn't work in practice. Can you submit a TianoCore
> BZ about it?

Nevermind:

> You are right, the attachment looks fine.

... so that's the reason "PatchCheck.py" does not complain.

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


Re: [edk2] [PATCH v2 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-02 Thread Laszlo Ersek
On 10/02/18 13:37, Marc-André Lureau wrote:
> On Tue, Oct 2, 2018 at 2:55 PM Laszlo Ersek  wrote:

>> Please fix your git settings for your local edk2 clone. Your patch
>> contains context lines with LF (not CRLF) line endings, and that's not
>> correct for edk2.
> 
> Shouldn't it be catched by: python BaseTools/Scripts/PatchCheck.py -1 ?

Yes, it should be caught by "PatchCheck.py". The "force_crlf" member is
set iff the pathname doesn't end in ".sh", and then the
check_added_line() method verifies CRLF.

I don't know why it doesn't work in practice. Can you submit a TianoCore
BZ about it?

(Your patch uses LF on both context lines and new (added) lines, so the
current script logic should complain, yes.)

> Hmm, this is weird, the original patch has crlf (see attach file).
> send-email or the mail server somehow stripped it?
> 
> I use your recommend git settings, I don't know what I am missing here.

You are right, the attachment looks fine.

... Can you resend the v2 patch (just to me directly, off-list) with

  --transfer-encoding=quoted-printable

? My guess is that the base64 encoding in git-send-email includes an
automatic CR stripping phase.

If indeed bas64 is the culprit, I'll update the wiki article to specify
quoted-printable.

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


Re: [edk2] [PATCH v2 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-02 Thread Laszlo Ersek
Anthony, Julien, Gary,

On 10/02/18 13:16, Laszlo Ersek wrote:
> On 10/02/18 10:36, marcandre.lur...@redhat.com wrote:

[...]

> The rest looks good, thanks. I'll regression-test the patch (with SMP
> and UP guests) once you post v3.

can one of you guys please regression-test v3 as well, in a Xen guest?
(UP and SMP.) This patch is different from the FeatureControl stuff in
that this one is not gated by fw_cfg. That implies CpuMpPei will be put
to use in Xen guests as well. We had better check whether it works
before we push the patch.

I'd also like to receive an ACK from Mike, for v3.

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


Re: [edk2] [PATCH v2 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-02 Thread Marc-André Lureau
Hi

On Tue, Oct 2, 2018 at 2:55 PM Laszlo Ersek  wrote:
>
> Hi Marc-André,
>
> On 10/02/18 10:36, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau 
> >
> > This is for conformance with the TCG "Platform Reset Attack Mitigation
> > Specification". Because clearing the CPU caches at boot doesn't impact
> > performance significantly, do it unconditionally, for simplicity's
> > sake.
> >
> > Flush the cache on all logical processors, thanks to
> > EFI_PEI_MP_SERVICES_PPI and CacheMaintenanceLib.
> >
> > Cc: Anthony Perard 
> > Cc: Ard Biesheuvel 
> > Cc: Jordan Justen 
> > Cc: Julien Grall 
> > Cc: Kinney Michael D 
> > Cc: Laszlo Ersek 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marc-André Lureau 
> > ---
> >
> > v2:
> >   - use CacheMaintenanceLib, instead of WBINVD usage
> >   - rename OnMpServicesAvailable->ClearCacheOnMpServicesAvailable
> >   - commit message & comments update
> >
> >  OvmfPkg/PlatformPei/PlatformPei.inf |   2 +
> >  OvmfPkg/PlatformPei/Platform.h  |   5 +
> >  OvmfPkg/PlatformPei/ClearCache.c| 113 
> >  OvmfPkg/PlatformPei/Platform.c  |   1 +
> >  4 files changed, 121 insertions(+)
>
> Please fix your git settings for your local edk2 clone. Your patch
> contains context lines with LF (not CRLF) line endings, and that's not
> correct for edk2.

Shouldn't it be catched by: python BaseTools/Scripts/PatchCheck.py -1 ?

>
> I decoded your base64-encoded patch email manually, hence my statement
> about the line endings. Here's a snippet from
> "OvmfPkg/PlatformPei/PlatformPei.inf":
>
> 0d90  64 69 66 66 20 2d 2d 67  69 74 20 61 2f 4f 76 6d  |diff --git a/Ovm|
> 0da0  66 50 6b 67 2f 50 6c 61  74 66 6f 72 6d 50 65 69  |fPkg/PlatformPei|
> 0db0  2f 50 6c 61 74 66 6f 72  6d 50 65 69 2e 69 6e 66  |/PlatformPei.inf|
> 0dc0  20 62 2f 4f 76 6d 66 50  6b 67 2f 50 6c 61 74 66  | b/OvmfPkg/Platf|
> 0dd0  6f 72 6d 50 65 69 2f 50  6c 61 74 66 6f 72 6d 50  |ormPei/PlatformP|
> 0de0  65 69 2e 69 6e 66 0a 69  6e 64 65 78 20 39 63 35  |ei.inf.index 9c5|
> 0df0  61 64 39 39 36 31 63 34  61 2e 2e 35 63 38 64 64  |ad9961c4a..5c8dd|
> 0e00  30 66 65 36 64 37 32 20  31 30 30 36 34 34 0a 2d  |0fe6d72 100644.-|
> 0e10  2d 2d 20 61 2f 4f 76 6d  66 50 6b 67 2f 50 6c 61  |-- a/OvmfPkg/Pla|
> 0e20  74 66 6f 72 6d 50 65 69  2f 50 6c 61 74 66 6f 72  |tformPei/Platfor|
> 0e30  6d 50 65 69 2e 69 6e 66  0a 2b 2b 2b 20 62 2f 4f  |mPei.inf.+++ b/O|
> 0e40  76 6d 66 50 6b 67 2f 50  6c 61 74 66 6f 72 6d 50  |vmfPkg/PlatformP|
> 0e50  65 69 2f 50 6c 61 74 66  6f 72 6d 50 65 69 2e 69  |ei/PlatformPei.i|
> 0e60  6e 66 0a 40 40 20 2d 33  30 2c 36 20 2b 33 30 2c  |nf.@@ -30,6 +30,|
> 0e70  37 20 40 40 0a 20 0a 20  5b 53 6f 75 72 63 65 73  |7 @@. . [Sources|
> 0e80  5d 0a 20 20 20 41 6d 64  53 65 76 2e 63 0a 2b 20  |].   AmdSev.c.+ |
> 0e90  20 43 6c 65 61 72 43 61  63 68 65 2e 63 0a 20 20  | ClearCache.c.  |
> 0ea0  20 43 6d 6f 73 2e 63 0a  20 20 20 43 6d 6f 73 2e  | Cmos.c.   Cmos.|
> 0eb0  68 0a 20 20 20 46 65 61  74 75 72 65 43 6f 6e 74  |h.   FeatureCont|
> 0ec0  72 6f 6c 2e 63 0a 40 40  20 2d 35 34 2c 36 20 2b  |rol.c.@@ -54,6 +|
> 0ed0  35 35 2c 37 20 40 40 0a  20 0a 20 5b 4c 69 62 72  |55,7 @@. . [Libr|
> 0ee0  61 72 79 43 6c 61 73 73  65 73 5d 0a 20 20 20 42  |aryClasses].   B|
> 0ef0  61 73 65 4c 69 62 0a 2b  20 20 43 61 63 68 65 4d  |aseLib.+  CacheM|
> 0f00  61 69 6e 74 65 6e 61 6e  63 65 4c 69 62 0a 20 20  |aintenanceLib.  |
>
> Due to this issue, git-am fails to apply the patch.
>
> I mentioned this earlier (including a request to push your patches being
> submitted to a personal repo/branch as well):
>
>   http://mid.mail-archive.com/f5c15a1e-25c2-c2e4-e6db-374107e3b488@redhat.com
>
> The wiki article at
>
>   
> https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers
>
> explains the git settings in order for git not to hide the CRLFs in the
> working tree. In particular, the git concept for (a) representing the
> source code internally with LFs, and (b) converting to/from CRLF on
> checkout/commit, is *not* to be used.
>
> Is this edk2 peculiarity broken, considering git standards and other
> open source repositories? It sure is.
>
> Can we change it? No, we can't.

Hmm, this is weird, the original patch has crlf (see attach file).
send-email or the mail server somehow stripped it?

I use your recommend git settings, I don't know what I am missing here.

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



-- 
Marc-André Lureau
From ec72ca9774cef9f89fbe721fc77d750107b332c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= 
Date: Mon, 1 Oct 2018 15:26:40 +0400
Subject: [PATCH v2 1/1] OvmfPkg/PlatformPei: clear CPU caches
MIME-Version: 1.0
Cont

Re: [edk2] [PATCH v2 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-02 Thread Laszlo Ersek
On 10/02/18 10:36, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> This is for conformance with the TCG "Platform Reset Attack Mitigation
> Specification". Because clearing the CPU caches at boot doesn't impact
> performance significantly, do it unconditionally, for simplicity's
> sake.
> 
> Flush the cache on all logical processors, thanks to
> EFI_PEI_MP_SERVICES_PPI and CacheMaintenanceLib.

The commit message looks OK to me, thanks.

> 
> Cc: Anthony Perard 
> Cc: Ard Biesheuvel 
> Cc: Jordan Justen 
> Cc: Julien Grall 
> Cc: Kinney Michael D 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau 
> ---
> 
> v2:
>   - use CacheMaintenanceLib, instead of WBINVD usage
>   - rename OnMpServicesAvailable->ClearCacheOnMpServicesAvailable
>   - commit message & comments update
> 
>  OvmfPkg/PlatformPei/PlatformPei.inf |   2 +
>  OvmfPkg/PlatformPei/Platform.h  |   5 +
>  OvmfPkg/PlatformPei/ClearCache.c| 113 
>  OvmfPkg/PlatformPei/Platform.c  |   1 +
>  4 files changed, 121 insertions(+)
> 
> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
> b/OvmfPkg/PlatformPei/PlatformPei.inf
> index 9c5ad9961c4a..5c8dd0fe6d72 100644
> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf
> @@ -30,6 +30,7 @@
>  
>  [Sources]
>AmdSev.c
> +  ClearCache.c
>Cmos.c
>Cmos.h
>FeatureControl.c
> @@ -54,6 +55,7 @@
>  
>  [LibraryClasses]
>BaseLib
> +  CacheMaintenanceLib
>DebugLib
>HobLib
>IoLib

OK.

> diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
> index f942e61bb4f9..b12a5c1f5f78 100644
> --- a/OvmfPkg/PlatformPei/Platform.h
> +++ b/OvmfPkg/PlatformPei/Platform.h
> @@ -83,6 +83,11 @@ InstallFeatureControlCallback (
>VOID
>);
>  
> +VOID
> +InstallClearCacheCallback (
> +  VOID
> +  );
> +
>  EFI_STATUS
>  InitializeXen (
>VOID
> diff --git a/OvmfPkg/PlatformPei/ClearCache.c 
> b/OvmfPkg/PlatformPei/ClearCache.c
> new file mode 100644
> index ..d333b7dcdd88
> --- /dev/null
> +++ b/OvmfPkg/PlatformPei/ClearCache.c
> @@ -0,0 +1,113 @@
> +/**@file
> +  Install a callback to clear cache on all processors.

The short explanation I requested in my previous point (2) is still missing.

(I think you may have misunderstood my previous points (1) and (2). In
(1), I asked for addressing Mike's observation in the commit message.
That was basically about replacing WBINVD with a reference to
CacheMaintenanceLib. In (2), I asked for the commit message to be fused
into a short sentence *here*, in this source file. IOW, my point (2)
wasn't about modifying the commit message itself; it was about
distilling a file comment from the commit message.

Anyway, I'm fine with the new commit message too, but the file comment
is still missing.)

> +
> +  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.
> +**/
> +
> +#include 
> +#include 
> +#include 
> +#include 

You've kept the INF file sections sorted, thanks a lot for that; please
keep this Library/* include list sorted as well.

The rest looks good, thanks. I'll regression-test the patch (with SMP
and UP guests) once you post v3.

Thanks,
Laszlo

> +
> +#include "Platform.h"
> +
> +/**
> +  Invalidate data & instruction caches.
> +  All APs execute this function in parallel. The BSP executes the function
> +  separately.
> +
> +  @param[in,out] WorkSpace  Pointer to the input/output argument workspace
> +shared by all processors.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ClearCache (
> +  IN OUT VOID *WorkSpace
> +  )
> +{
> +  WriteBackInvalidateDataCache ();
> +  InvalidateInstructionCache ();
> +}
> +
> +/**
> +  Notification function called when EFI_PEI_MP_SERVICES_PPI becomes 
> available.
> +
> +  @param[in] PeiServices  Indirect reference to the PEI Services Table.
> +  @param[in] NotifyDescriptor Address of the notification descriptor data
> +  structure.
> +  @param[in] Ppi  Address of the PPI that was installed.
> +
> +  @return  Status of the notification. The status code returned from this
> +   function is ignored.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +ClearCacheOnMpServicesAvailable (
> +  IN EFI_PEI_SERVICES   **PeiServices,
> +  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
> +  IN VOID   *Ppi
> +  )
> +{
> +  EFI_PEI_MP_SERVICES_PPI *MpServices;
> +  EFI_STATUS  Status;
> +
> +  DEBUG ((DEBUG_INFO, "%a: %a\n", g

Re: [edk2] [PATCH v2 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-02 Thread Laszlo Ersek
Hi Marc-André,

On 10/02/18 10:36, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
>
> This is for conformance with the TCG "Platform Reset Attack Mitigation
> Specification". Because clearing the CPU caches at boot doesn't impact
> performance significantly, do it unconditionally, for simplicity's
> sake.
>
> Flush the cache on all logical processors, thanks to
> EFI_PEI_MP_SERVICES_PPI and CacheMaintenanceLib.
>
> Cc: Anthony Perard 
> Cc: Ard Biesheuvel 
> Cc: Jordan Justen 
> Cc: Julien Grall 
> Cc: Kinney Michael D 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau 
> ---
>
> v2:
>   - use CacheMaintenanceLib, instead of WBINVD usage
>   - rename OnMpServicesAvailable->ClearCacheOnMpServicesAvailable
>   - commit message & comments update
>
>  OvmfPkg/PlatformPei/PlatformPei.inf |   2 +
>  OvmfPkg/PlatformPei/Platform.h  |   5 +
>  OvmfPkg/PlatformPei/ClearCache.c| 113 
>  OvmfPkg/PlatformPei/Platform.c  |   1 +
>  4 files changed, 121 insertions(+)

Please fix your git settings for your local edk2 clone. Your patch
contains context lines with LF (not CRLF) line endings, and that's not
correct for edk2.

I decoded your base64-encoded patch email manually, hence my statement
about the line endings. Here's a snippet from
"OvmfPkg/PlatformPei/PlatformPei.inf":

0d90  64 69 66 66 20 2d 2d 67  69 74 20 61 2f 4f 76 6d  |diff --git a/Ovm|
0da0  66 50 6b 67 2f 50 6c 61  74 66 6f 72 6d 50 65 69  |fPkg/PlatformPei|
0db0  2f 50 6c 61 74 66 6f 72  6d 50 65 69 2e 69 6e 66  |/PlatformPei.inf|
0dc0  20 62 2f 4f 76 6d 66 50  6b 67 2f 50 6c 61 74 66  | b/OvmfPkg/Platf|
0dd0  6f 72 6d 50 65 69 2f 50  6c 61 74 66 6f 72 6d 50  |ormPei/PlatformP|
0de0  65 69 2e 69 6e 66 0a 69  6e 64 65 78 20 39 63 35  |ei.inf.index 9c5|
0df0  61 64 39 39 36 31 63 34  61 2e 2e 35 63 38 64 64  |ad9961c4a..5c8dd|
0e00  30 66 65 36 64 37 32 20  31 30 30 36 34 34 0a 2d  |0fe6d72 100644.-|
0e10  2d 2d 20 61 2f 4f 76 6d  66 50 6b 67 2f 50 6c 61  |-- a/OvmfPkg/Pla|
0e20  74 66 6f 72 6d 50 65 69  2f 50 6c 61 74 66 6f 72  |tformPei/Platfor|
0e30  6d 50 65 69 2e 69 6e 66  0a 2b 2b 2b 20 62 2f 4f  |mPei.inf.+++ b/O|
0e40  76 6d 66 50 6b 67 2f 50  6c 61 74 66 6f 72 6d 50  |vmfPkg/PlatformP|
0e50  65 69 2f 50 6c 61 74 66  6f 72 6d 50 65 69 2e 69  |ei/PlatformPei.i|
0e60  6e 66 0a 40 40 20 2d 33  30 2c 36 20 2b 33 30 2c  |nf.@@ -30,6 +30,|
0e70  37 20 40 40 0a 20 0a 20  5b 53 6f 75 72 63 65 73  |7 @@. . [Sources|
0e80  5d 0a 20 20 20 41 6d 64  53 65 76 2e 63 0a 2b 20  |].   AmdSev.c.+ |
0e90  20 43 6c 65 61 72 43 61  63 68 65 2e 63 0a 20 20  | ClearCache.c.  |
0ea0  20 43 6d 6f 73 2e 63 0a  20 20 20 43 6d 6f 73 2e  | Cmos.c.   Cmos.|
0eb0  68 0a 20 20 20 46 65 61  74 75 72 65 43 6f 6e 74  |h.   FeatureCont|
0ec0  72 6f 6c 2e 63 0a 40 40  20 2d 35 34 2c 36 20 2b  |rol.c.@@ -54,6 +|
0ed0  35 35 2c 37 20 40 40 0a  20 0a 20 5b 4c 69 62 72  |55,7 @@. . [Libr|
0ee0  61 72 79 43 6c 61 73 73  65 73 5d 0a 20 20 20 42  |aryClasses].   B|
0ef0  61 73 65 4c 69 62 0a 2b  20 20 43 61 63 68 65 4d  |aseLib.+  CacheM|
0f00  61 69 6e 74 65 6e 61 6e  63 65 4c 69 62 0a 20 20  |aintenanceLib.  |

Due to this issue, git-am fails to apply the patch.

I mentioned this earlier (including a request to push your patches being
submitted to a personal repo/branch as well):

  http://mid.mail-archive.com/f5c15a1e-25c2-c2e4-e6db-374107e3b488@redhat.com

The wiki article at

  
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers

explains the git settings in order for git not to hide the CRLFs in the
working tree. In particular, the git concept for (a) representing the
source code internally with LFs, and (b) converting to/from CRLF on
checkout/commit, is *not* to be used.

Is this edk2 peculiarity broken, considering git standards and other
open source repositories? It sure is.

Can we change it? No, we can't.

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


Re: [edk2] [PATCH v3 00/16] Removed unused PCDs

2018-10-02 Thread Laszlo Ersek
Ray, Chao,

guys, you keep breaking the development process. Please fix your email
clients *now*.


On 08/28/18 05:42, shenglei wrote:
> shenglei (16):
>   IntelFsp2Pkg FspSecCore: Remove unused PCDs
>   IntelFsp2Pkg/BaseFspCommonLib: Remove unused PCDs
>   IntelFsp2Pkg/BaseFspPlatformLib: Remove unused PCDs
>   IntelFsp2Pkg/BaseFspSwitchStackLib: Remove unused PCDs
>   IntelFsp2WrapperPkg/FspWrapperNotifyDxe: Remove an unused PCD
>   IntelFsp2WrapperPkg/BaseFspWrapperPlatformLibSample: Remove PCDs
>   SecurityPkg/Tcg2ConfigPei: Remove an unused PCD

This was patch #07 in this series. I had never reviewed it, yet Chao
pushed it with my R-b as commit

  https://github.com/tianocore/edk2/commit/3e11c27f67ea


>   SecurityPkg/Tcg2Dxe: Remove unused PCDs
>   UefiCpuPkg/CpuCommonFeaturesLib: Remove an unused PCD
>   MdePkg/BaseLib: Remove an unused PCD
>   MdeModulePkg/DxeCapsuleLibFmp: Remove unused PCDs
>   MdeModulePkg/FirmwarePerformanceDataTableDxe: Remove an unused PCD
>   ShellPkg/Shell: Remove unused PCDs

This was patch #13 in this series. I reviewed it:

  70dfa56d-6781-e8c0-f3f4-aa12558672b9@redhat.com">http://mid.mail-archive.com/70dfa56d-6781-e8c0-f3f4-aa12558672b9@redhat.com

but Ray pushed it as commit

  https://github.com/tianocore/edk2/commit/a9dfe53f56bb

without my R-b tag. (Note: there was 1 month between my feedback and the
push date.)

The commit message now suggests that I ignored the patch (because I was
on CC, but seemingly didn't respond). It mis-represents my acts.


>   ShellPkg/DpDynamicCommand: Remove unused PCDs
>   ShellPkg/UefiHandleParsingLib: Remove an unused PCD

Same here. Patch #15, pushed as commit

  https://github.com/tianocore/edk2/commit/42a7c2871a65

My review was at:

  a70f17d9-b937-2835-4d71-5464bad82219@redhat.com">http://mid.mail-archive.com/a70f17d9-b937-2835-4d71-5464bad82219@redhat.com

but it was dropped from the commit.


>   ShellPkg/UefiShellDebug1CommandsLib: Remove unused PCDs

Ditto. Patch #16. My review was at:

  d23e7c95-96e9-4088-4e95-5dbc0a331cb1@redhat.com">http://mid.mail-archive.com/d23e7c95-96e9-4088-4e95-5dbc0a331cb1@redhat.com

but the patch was pushed as commit

  https://github.com/tianocore/edk2/commit/aa9986651bfe

with my review lost.


This is not the first time it has happened. If I remember correctly, Ray
blamed his email client last time (not showing message threads
correctly, or something similar).

I'm sorry, but this is unacceptable. This is on-going, systemic
disregard for the project's other participants.

Please fix your mail user agents *now*.

Here's my promise. Next time, I'm going to revert such commits (assuming
I manage to catch them again). They do not represent the facts from the
mailing list.

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


[edk2] [PATCH v2 1/1] OvmfPkg/PlatformPei: clear CPU caches

2018-10-02 Thread marcandre . lureau
From: Marc-André Lureau 

This is for conformance with the TCG "Platform Reset Attack Mitigation
Specification". Because clearing the CPU caches at boot doesn't impact
performance significantly, do it unconditionally, for simplicity's
sake.

Flush the cache on all logical processors, thanks to
EFI_PEI_MP_SERVICES_PPI and CacheMaintenanceLib.

Cc: Anthony Perard 
Cc: Ard Biesheuvel 
Cc: Jordan Justen 
Cc: Julien Grall 
Cc: Kinney Michael D 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc-André Lureau 
---

v2:
  - use CacheMaintenanceLib, instead of WBINVD usage
  - rename OnMpServicesAvailable->ClearCacheOnMpServicesAvailable
  - commit message & comments update

 OvmfPkg/PlatformPei/PlatformPei.inf |   2 +
 OvmfPkg/PlatformPei/Platform.h  |   5 +
 OvmfPkg/PlatformPei/ClearCache.c| 113 
 OvmfPkg/PlatformPei/Platform.c  |   1 +
 4 files changed, 121 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
b/OvmfPkg/PlatformPei/PlatformPei.inf
index 9c5ad9961c4a..5c8dd0fe6d72 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -30,6 +30,7 @@
 
 [Sources]
   AmdSev.c
+  ClearCache.c
   Cmos.c
   Cmos.h
   FeatureControl.c
@@ -54,6 +55,7 @@
 
 [LibraryClasses]
   BaseLib
+  CacheMaintenanceLib
   DebugLib
   HobLib
   IoLib
diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
index f942e61bb4f9..b12a5c1f5f78 100644
--- a/OvmfPkg/PlatformPei/Platform.h
+++ b/OvmfPkg/PlatformPei/Platform.h
@@ -83,6 +83,11 @@ InstallFeatureControlCallback (
   VOID
   );
 
+VOID
+InstallClearCacheCallback (
+  VOID
+  );
+
 EFI_STATUS
 InitializeXen (
   VOID
diff --git a/OvmfPkg/PlatformPei/ClearCache.c b/OvmfPkg/PlatformPei/ClearCache.c
new file mode 100644
index ..d333b7dcdd88
--- /dev/null
+++ b/OvmfPkg/PlatformPei/ClearCache.c
@@ -0,0 +1,113 @@
+/**@file
+  Install a callback to clear cache on all processors.
+
+  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.
+**/
+
+#include 
+#include 
+#include 
+#include 
+
+#include "Platform.h"
+
+/**
+  Invalidate data & instruction caches.
+  All APs execute this function in parallel. The BSP executes the function
+  separately.
+
+  @param[in,out] WorkSpace  Pointer to the input/output argument workspace
+shared by all processors.
+**/
+STATIC
+VOID
+EFIAPI
+ClearCache (
+  IN OUT VOID *WorkSpace
+  )
+{
+  WriteBackInvalidateDataCache ();
+  InvalidateInstructionCache ();
+}
+
+/**
+  Notification function called when EFI_PEI_MP_SERVICES_PPI becomes available.
+
+  @param[in] PeiServices  Indirect reference to the PEI Services Table.
+  @param[in] NotifyDescriptor Address of the notification descriptor data
+  structure.
+  @param[in] Ppi  Address of the PPI that was installed.
+
+  @return  Status of the notification. The status code returned from this
+   function is ignored.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+ClearCacheOnMpServicesAvailable (
+  IN EFI_PEI_SERVICES   **PeiServices,
+  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
+  IN VOID   *Ppi
+  )
+{
+  EFI_PEI_MP_SERVICES_PPI *MpServices;
+  EFI_STATUS  Status;
+
+  DEBUG ((DEBUG_INFO, "%a: %a\n", gEfiCallerBaseName, __FUNCTION__));
+
+  //
+  // Clear cache on all the APs in parallel.
+  //
+  MpServices = Ppi;
+  Status = MpServices->StartupAllAPs (
+ (CONST EFI_PEI_SERVICES **)PeiServices,
+ MpServices,
+ ClearCache,  // Procedure
+ FALSE,   // SingleThread
+ 0,   // TimeoutInMicroSeconds: inf.
+ NULL // ProcedureArgument
+ );
+  if (EFI_ERROR (Status) && Status != EFI_NOT_STARTED) {
+DEBUG ((DEBUG_ERROR, "%a: StartupAllAps(): %r\n", __FUNCTION__, Status));
+return Status;
+  }
+
+  //
+  // Now clear cache on the BSP too.
+  //
+  ClearCache (NULL);
+  return EFI_SUCCESS;
+}
+
+//
+// Notification object for registering the callback, for when
+// EFI_PEI_MP_SERVICES_PPI becomes available.
+//
+STATIC CONST EFI_PEI_NOTIFY_DESCRIPTOR mMpServicesNotify = {
+  EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | // Flags
+  EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
+  &gEfiPeiMpServicesPpiGuid,   // Guid
+  ClearCacheOnMpServicesAvailable  // Notify
+};
+
+VOID
+InstallClearCacheCallback (
+  VOID
+  )
+{
+