Re: [edk2] [PATCH 2/2] OvmfPkg/QemuBootOrderLib: let an OFW devpath match multiple UEFI boot opts

[Ard Biesheuvel]

On 27 November 2017 at 19:03, Laszlo Ersek  wrote:
> This means that SetBootOrderFromQemu() will preserve all UEFI boot options
> matched by any given OFW devpath, such as PXEv4, HTTPv4, PXEv6 and HTTPv6
> boot options for the same NIC. Currently we stop the matching / appending
> for the OFW devpath coming from the outer loop whenever we find the first
> UEFI boot option match in the inner loop.
>
> (The previous patch was about multiple OFW devpaths matching a single UEFI
> boot option (which should never happen). This patch is about a single OFW
> devpath matching multiple UEFI boot options. With the "break" statement
> removed here, the small optimization from the last patch becomes a bit
> more relevant, because now the inner loop always counts up to
> ActiveCount.)
>
> Cc: Ard Biesheuvel 
> Cc: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 

Acked-by: Ard Biesheuvel 

> ---
>  OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c 
> b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
> index a9a62e9d4007..366104adf535 100644
> --- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
> +++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
> @@ -1863,31 +1863,30 @@ SetBootOrderFromQemu (
>for (Idx = 0; Idx < ActiveCount; ++Idx) {
>  if (!ActiveOption[Idx].Appended &&
>  Match (
>Translated,
>TranslatedSize, // contains length, not size, in CHAR16's here
>ActiveOption[Idx].BootOption->FilePath
>)
>  ) {
>//
>// match found, store ID and continue with next OpenFirmware path
>//
>Status = BootOrderAppend (, [Idx]);
>if (Status != RETURN_SUCCESS) {
>  goto ErrorFreeExtraPciRoots;
>}
> -  break;
>  }
>} // scanned all active boot options
>  }   // translation successful
>
>  TranslatedSize = ARRAY_SIZE (Translated);
>  Status = TranslateOfwPath (, ExtraPciRoots, Translated,
> );
>} // scanning of OpenFirmware paths done
>
>if (Status == RETURN_NOT_FOUND && BootOrder.Produced > 0) {
>  //
>  // No more OpenFirmware paths, some matches found: rewrite BootOrder 
> NvVar.
>  // Some of the active boot options that have not been selected over 
> fw_cfg
>  // should be preserved at the end of the boot order.
>  //
> --
> 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 1/2] OvmfPkg/QemuBootOrderLib: skip already matched / appended UEFI boot opts

[Ard Biesheuvel]

On 27 November 2017 at 19:03, Laszlo Ersek  wrote:
> The SetBootOrderFromQemu() function implements a nested loop where
>
> - the outer loop iterates over all OpenFirmware (OFW) device paths in the
>   QEMU boot order, and translates each to a UEFI device path prefix;
>
> - the inner loop matches the current (translated) prefix against all
>   active UEFI boot options in turn;
>
> - if the UEFI boot option is matched by the translated prefix, the UEFI
>   boot option is appended to the "new" UEFI boot order, and marked as
>   "has been appended".
>
> This patch adds a micro-optimization where already matched / appended UEFI
> boot options are skipped in the inner loop. This is not a functional
> change. A functional change would be if, as a consequence of the patch,
> some UEFI boot options would no longer be *doubly* matched.
>
> For a UEFI boot option to be matched by two translated prefixes, one of
> those prefixes would have to be a (proper, or equal) prefix of the other
> prefix. The PCI and MMIO OFW translation routines output such only in the
> following cases:
>
> - When the original OFW device paths are prefixes of each other. This is
>   not possible from the QEMU side. (Only leaf devices are bootable.)
>
> - When the translation rules in the routines are incomplete, and don't
>   look at the OFW device paths for sufficient length (i.e., at nodes where
>   they would already differ, and the difference would show up in the
>   translation output).
>
>   This would be a shortcoming of the translation routines and should be
>   fixed in TranslatePciOfwNodes() and TranslateMmioOfwNodes(), whenever
>   identified.
>
> Even in the second case, this patch would replace the double appending of
> a single UEFI boot option (matched by two different OFW device paths) with
> a correct, or cross-, matching of two different UEFI boot options. Again,
> this is not expected, but arguably it would be more correct than duplicate
> boot option appending, should it occur due to any (unexpected, unknown)
> lack of detail in the translation routines.
>
> Cc: Ard Biesheuvel 
> Cc: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 

Acked-by: Ard Biesheuvel 

> ---
>  OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c 
> b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
> index 7c1f375beb20..a9a62e9d4007 100644
> --- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
> +++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
> @@ -1849,31 +1849,32 @@ SetBootOrderFromQemu (
>//
>TranslatedSize = ARRAY_SIZE (Translated);
>Status = TranslateOfwPath (, ExtraPciRoots, Translated,
>   );
>while (Status == RETURN_SUCCESS ||
>   Status == RETURN_UNSUPPORTED ||
>   Status == RETURN_PROTOCOL_ERROR ||
>   Status == RETURN_BUFFER_TOO_SMALL) {
>  if (Status == RETURN_SUCCESS) {
>UINTN Idx;
>
>//
>// match translated OpenFirmware path against all active boot options
>//
>for (Idx = 0; Idx < ActiveCount; ++Idx) {
> -if (Match (
> +if (!ActiveOption[Idx].Appended &&
> +Match (
>Translated,
>TranslatedSize, // contains length, not size, in CHAR16's here
>ActiveOption[Idx].BootOption->FilePath
>)
>  ) {
>//
>// match found, store ID and continue with next OpenFirmware path
>//
>Status = BootOrderAppend (, [Idx]);
>if (Status != RETURN_SUCCESS) {
>  goto ErrorFreeExtraPciRoots;
>}
>break;
>  }
>} // scanned all active boot options
> --
> 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 2/3] ShellPkg/tftp: Convert from NULL class library to Dynamic Command

[Ni, Ruiyu]

I just realized OVMF platform is referencing the INF.
I will search in all edkII code.

Thanks/Ray

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Tuesday, November 28, 2017 3:35 PM
> To: Ni, Ruiyu 
> Cc: Carsey, Jaben ; Kinney, Michael D
> ; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH v2 2/3] ShellPkg/tftp: Convert from NULL class
> library to Dynamic Command
> 
> On 27 November 2017 at 05:55, Ruiyu Ni  wrote:
> > UEFI Shell spec defines Shell Dynamic Command protocol which is just
> > for the purpose to extend internal command.
> > So tftp command is changed from NULL class library to be a driver
> > producing DynamicCommand protocol.
> >
> > The guideline is:
> > 1. Only use NULL class library for Shell spec defined commands.
> > 2. New commands can be provided as not only a standalone application
> >but also a dynamic command. So it can be used either as an
> >internal command, but also as a standalone application.
> >
> > TftpApp.inf is to provide a standalone application.
> > TftpDynamicCommand.inf is to provide a standalone driver producing
> > Dynamic Command protocol.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ruiyu Ni 
> > Cc: Jaben Carsey 
> > Cc: Michael D Kinney 
> > ---
> >  .../TftpDynamicCommand}/Tftp.c |  92 +++
> >  .../TftpDynamicCommand/Tftp.h} |  40 +--
> >  .../TftpDynamicCommand/Tftp.uni}   |   0
> >  .../DynamicCommand/TftpDynamicCommand/TftpApp.c|  54
> +
> >  .../TftpDynamicCommand/TftpApp.inf}|  34 +++---
> >  .../TftpDynamicCommand/TftpDynamicCommand.c| 131
> +
> >  .../TftpDynamicCommand/TftpDynamicCommand.inf} |  39 +++---
> >  .../UefiShellTftpCommandLib.c  |  97 ---
> >  ShellPkg/ShellPkg.dsc  |  11 +-
> >  9 files changed, 325 insertions(+), 173 deletions(-)
> 
> Please make sure that you fix platforms that use .inf files when renaming
> them.
> The ArmVirtQemu build is currently broken due to this patch.
> ___
> 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 0/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes

[Ni, Ruiyu]

Laszlo,
The Windows resume issue is urgent to fix.
I'd like to check-in the patches first.
If I didn't know the change may cause combability issue, I'd be very fine
to change it to AhciReset() then submit the patch, then after got a R-b,
I'd be very happy to check in that patch.
But since now I know there might be some combability issue, I don't
want to switch to AhciReset() solution without thoroughly testing.
And frankly I don't know what a thoroughly testing would be like.
There are many OSes and many PCI storage cards! ☹

Thanks/Ray

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Monday, November 27, 2017 10:10 PM
> To: Ni, Ruiyu 
> Cc: edk2-devel@lists.01.org; Paolo Bonzini ; Yao,
> Jiewen ; Zeng, Star 
> Subject: Re: [edk2] [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert
> patch to disable PCI attributes
> 
> Hello Ray,
> 
> On 11/27/17 02:19, Ruiyu Ni wrote:
> > The patches caused Windows 10 S4 resume failure.
> > Considering the similar changes are reverted from PciBus driver,
> > revert the patches from AtaAtapiPassThru as well.
> >
> > Ruiyu Ni (2):
> >   MdeModulePkg/AtaAtapiPassThru: Revert patch to disable Bus Master
> >   MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI
> > attributes
> >
> >  .../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c| 58 
> > +
> -
> >  .../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h|  5 --
> >  2 files changed, 1 insertion(+), 62 deletions(-)
> >
> 
> it looks like these patches have not been committed yet, which is a good
> thing, because apparently there's a better solution than a full revert.
> Namely, in the other sub-thread at
>  7fd1ad742...@redhat.com>
> (alternative link:
> ),
> Jiewen and Star seem to accept AhciReset() as a better way to abort pending
> DMA.
> 
> This means that we need not revert the EBS-handler altogether, only change
> what it does. Could you give that a try please?
> 
> (If the Windows regression is very urgent to fix, then I don't mind if the Bus
> Master disabling is removed separately, before AhciReset() is added; but in
> that case, a full revert looks unjustified, since the EBS handler will have 
> to be
> re-added for AhciReset() anyway.)
> 
> Thanks,
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] [edk2-platforms]:Enabling Secure boot feature support on hikey platfrom

[Kalyan Nagabhirava]

HI Leif,
we didn't implemented enabling USB host and  Secure boot support  on Hikey
, we just took the code from openplatfrompkg (hikey branch)
 , we have implemented secureboot and DRI -disaster recovery image (HTTP
image download) application
and tested on HIkey platform , so for that purpose we are trying to
upstream the hikey code.

but hikey platform code  looks in bad shape (as per ard and your comments)
,so  we are planning to upstream
only our application code which is independent of platform.

Regards,
kalyan.


On 27 November 2017 at 22:27, Leif Lindholm 
wrote:

> On Mon, Nov 27, 2017 at 02:02:32PM +0100, Laszlo Ersek wrote:
> > On 11/26/17 16:22, Leif Lindholm wrote:
> > > (Adding Laszlo to cc based on a single comment I make below.)
> > >
> > > On Tue, Nov 21, 2017 at 04:23:36PM +0530, kalyan-nagabhirava wrote:
> >
> > >>  [Guids.common]
> > >>gHiKeyTokenSpaceGuid  =  { 0x91148425, 0xcdd2, 0x4830, {
> 0x8b, 0xd0, 0xc6, 0x1c, 0x6d, 0xea, 0x36, 0x21 } }
> > >> +  gHwTokenSpaceGuid =  { 0x, 0x74c5, 0x4043, {
> 0xb4, 0x17, 0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }
> > >
> > > This very much looks like a not properly generated GUID.
> > > GUIDs must always be generated using an RFC4122-compliant algorithm.
> > > I generally recommend using
> > > https://www.guidgenerator.com/online-guid-generator.aspx.
> >
> > I just run "uuidgen" in a terminal window.
>
> Yeah, I just prefer pointing to someone that does not require
> installing anything, or requires specific operating systems.
>
> > >> +EFI_STATUS
> > >> +EFIAPI
> > >> +FvbSetAttributes(
> > >> +  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL  *This,
> > >> +  IN OUTEFI_FVB_ATTRIBUTES_2 *Attributes
> > >> +  )
> > >> +{
> > >> +  DEBUG ((DEBUG_BLKIO, "FvbSetAttributes(0x%X) is not
> supported\n",*Attributes));
> > >> +  return EFI_UNSUPPORTED;
> > >
> > > As per my (very) recent comment to Marcin, I do not believe returning
> > > EFI_UNSUPPORTED is a valid thing to do here. Which to me suggests the
> > > implementation of FvbGetAttributes is also incorrect.
> > >
> > > Laszlo - what's your take on this in conjunction with PI 1.6 section
> > > 3.4.2? OvmfPkg does something very similar in
> > > EmuVariableFvbRuntimeDxe/Fvb.c.
> >
> > I guess you are right. The particular OvmfPkg code that you mention is
> > likely also spec-breaking.
> >
> > FWIW, in the OVMF flash driver that actually uses pflash, namely
> >
> >   OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
> >
> > the FvbSetVolumeAttributes() function appears both appropriate for the
> > spec and generic enough to copy elsewhere.
>
> Yes, that looks good, thanks!
>
> Marcin, Kalyan - please have a look at that implementation for
> inspiration.
>
> /
> Leif
>



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

Re: [edk2] [PATCH v2 2/3] ShellPkg/tftp: Convert from NULL class library to Dynamic Command

[Ard Biesheuvel]

On 27 November 2017 at 05:55, Ruiyu Ni  wrote:
> UEFI Shell spec defines Shell Dynamic Command protocol which is just
> for the purpose to extend internal command.
> So tftp command is changed from NULL class library to be a driver
> producing DynamicCommand protocol.
>
> The guideline is:
> 1. Only use NULL class library for Shell spec defined commands.
> 2. New commands can be provided as not only a standalone application
>but also a dynamic command. So it can be used either as an
>internal command, but also as a standalone application.
>
> TftpApp.inf is to provide a standalone application.
> TftpDynamicCommand.inf is to provide a standalone driver producing
> Dynamic Command protocol.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni 
> Cc: Jaben Carsey 
> Cc: Michael D Kinney 
> ---
>  .../TftpDynamicCommand}/Tftp.c |  92 +++
>  .../TftpDynamicCommand/Tftp.h} |  40 +--
>  .../TftpDynamicCommand/Tftp.uni}   |   0
>  .../DynamicCommand/TftpDynamicCommand/TftpApp.c|  54 +
>  .../TftpDynamicCommand/TftpApp.inf}|  34 +++---
>  .../TftpDynamicCommand/TftpDynamicCommand.c| 131 
> +
>  .../TftpDynamicCommand/TftpDynamicCommand.inf} |  39 +++---
>  .../UefiShellTftpCommandLib.c  |  97 ---
>  ShellPkg/ShellPkg.dsc  |  11 +-
>  9 files changed, 325 insertions(+), 173 deletions(-)

Please make sure that you fix platforms that use .inf files when renaming them.
The ArmVirtQemu build is currently broken due to this patch.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] [edk2 EmbeddedPkg]:Adding secure boot and HTTP image download (Disaster recovery image) Applications for Linux based platforms

[kalyan-nagabhirava]

Linaro and RDK are  working on standardizing the boot process for RDK  STB 
boxes using Uefi.
Added applications are reference implementation of RDK STB boot process on Arm 
platforms

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: kalyan-nagabhirava 
---
 EmbeddedPkg/Application/Dri/Dri.c  |  26 +
 EmbeddedPkg/Application/Dri/Dri.inf|  56 ++
 .../Application/DriSecureBoot/DriSecureBoot.c  |  32 ++
 .../Application/DriSecureBoot/DriSecureBoot.inf|  57 ++
 EmbeddedPkg/Application/SecureBoot/SecureBoot.c|  30 ++
 EmbeddedPkg/Application/SecureBoot/SecureBoot.inf  |  57 ++
 EmbeddedPkg/Drivers/RdkDxe/RdkDxe.c|  97 
 EmbeddedPkg/Drivers/RdkDxe/RdkDxe.h|  14 +
 EmbeddedPkg/Drivers/RdkDxe/RdkDxe.inf  |  45 ++
 EmbeddedPkg/Library/RdkBootManagerLib/DiskIo.c | 536 +++
 EmbeddedPkg/Library/RdkBootManagerLib/HttpBoot.c   | 315 +++
 .../Library/RdkBootManagerLib/Include/DiskIo.h |  20 +
 .../Library/RdkBootManagerLib/Include/HttpBoot.h   |   7 +
 .../Library/RdkBootManagerLib/Include/List.h   | 136 +
 .../RdkBootManagerLib/Include/RdkBootManagerLib.h  |  31 ++
 .../Library/RdkBootManagerLib/Include/RdkFile.h|  20 +
 .../Library/RdkBootManagerLib/Include/SecureBoot.h |  40 ++
 .../RdkBootManagerLib/RdkBootManagerLib.dec|  52 ++
 .../RdkBootManagerLib/RdkBootManagerLib.inf|  81 +++
 EmbeddedPkg/Library/RdkBootManagerLib/RdkFile.c| 259 +
 EmbeddedPkg/Library/RdkBootManagerLib/SecureBoot.c | 577 +
 21 files changed, 2488 insertions(+)
 create mode 100644 EmbeddedPkg/Application/Dri/Dri.c
 create mode 100644 EmbeddedPkg/Application/Dri/Dri.inf
 create mode 100644 EmbeddedPkg/Application/DriSecureBoot/DriSecureBoot.c
 create mode 100644 EmbeddedPkg/Application/DriSecureBoot/DriSecureBoot.inf
 create mode 100644 EmbeddedPkg/Application/SecureBoot/SecureBoot.c
 create mode 100644 EmbeddedPkg/Application/SecureBoot/SecureBoot.inf
 create mode 100644 EmbeddedPkg/Drivers/RdkDxe/RdkDxe.c
 create mode 100644 EmbeddedPkg/Drivers/RdkDxe/RdkDxe.h
 create mode 100644 EmbeddedPkg/Drivers/RdkDxe/RdkDxe.inf
 create mode 100644 EmbeddedPkg/Library/RdkBootManagerLib/DiskIo.c
 create mode 100644 EmbeddedPkg/Library/RdkBootManagerLib/HttpBoot.c
 create mode 100644 EmbeddedPkg/Library/RdkBootManagerLib/Include/DiskIo.h
 create mode 100644 EmbeddedPkg/Library/RdkBootManagerLib/Include/HttpBoot.h
 create mode 100644 EmbeddedPkg/Library/RdkBootManagerLib/Include/List.h
 create mode 100644 
EmbeddedPkg/Library/RdkBootManagerLib/Include/RdkBootManagerLib.h
 create mode 100644 EmbeddedPkg/Library/RdkBootManagerLib/Include/RdkFile.h
 create mode 100644 EmbeddedPkg/Library/RdkBootManagerLib/Include/SecureBoot.h
 create mode 100644 EmbeddedPkg/Library/RdkBootManagerLib/RdkBootManagerLib.dec
 create mode 100644 EmbeddedPkg/Library/RdkBootManagerLib/RdkBootManagerLib.inf
 create mode 100644 EmbeddedPkg/Library/RdkBootManagerLib/RdkFile.c
 create mode 100644 EmbeddedPkg/Library/RdkBootManagerLib/SecureBoot.c

diff --git a/EmbeddedPkg/Application/Dri/Dri.c 
b/EmbeddedPkg/Application/Dri/Dri.c
new file mode 100644
index 00..affbac08b6
--- /dev/null
+++ b/EmbeddedPkg/Application/Dri/Dri.c
@@ -0,0 +1,26 @@
+/*
+#  Copyright (c) 2016-2017, Linaro Limited. All rights reserved.
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD 
License
+#  which accompanies this distribution.  The full text of the license may be 
found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+ */
+#include 
+
+EFI_STATUS
+EFIAPI
+DriEntryPoint (
+  IN EFI_HANDLEImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS Status;
+
+  Status = RdkHttpBoot ();
+  return Status;
+}
diff --git a/EmbeddedPkg/Application/Dri/Dri.inf 
b/EmbeddedPkg/Application/Dri/Dri.inf
new file mode 100644
index 00..d6f24b48a6
--- /dev/null
+++ b/EmbeddedPkg/Application/Dri/Dri.inf
@@ -0,0 +1,56 @@
+#
+#  Copyright (c) 2016-2017, Linaro Limited. All rights reserved.
+#  Copyright (c) 2016-2017, comcast . 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 Section - statements that will be 

Re: [edk2] [RFC v1 00/15] *** Proposal for StandaloneMmPkg ***

[Yao, Jiewen]

HI
I am sorry that I missed this mail before.

I found only 0/15 and 13/15 are in my mail box. :-(
I do not know why others are filtered.

Is that possible to post the whole patch to your private git?
As such, we can review it in more efficient way.

Basically, I think it is a good idea to have StandaloneMmPkg to hold all these 
features.

Thank you
Yao Jiewen

> -Original Message-
> From: Supreeth Venkatesh [mailto:supreeth.venkat...@arm.com]
> Sent: Saturday, November 18, 2017 7:08 AM
> To: edk2-devel@lists.01.org
> Cc: leif.lindh...@linaro.org; Yao, Jiewen ; Supreeth
> Venkatesh 
> Subject: [RFC v1 00/15] *** Proposal for StandaloneMmPkg ***
> 
> ***
> PI Specification v1.5  "Volume 4: Management Mode Core Interface"
> introduces the concept of MM Standalone Mode. Initialization of this mode
> can be done during the SEC phase (Section 1.5.2).
> On ARMv8-A systems, ARM Trusted Firmware is responsible for launching
> the normal world firmware e.g. UEFI.
> 
> The Standalone MM environment is instantiated in Secure EL0 as a separate
> firmware volume. It executes as BL32 Image under control of ARM TF
> which is instantiated in EL3. Both components execute in the AArch64 execution
> state.
> This patchset will build upon the StandaloneSmmPkg module originally
> contributed by Intel.
> 
> This package can be used in conjunction with ARM Trusted Firmware
> to recreate a simple MM secure environment that demonstrates communication
> between two UEFI images where one is executing in the normal world and the
> other is
> executing in the secure world.
> 
> The normal world image includes:
> MM Communication DXE runtime driver that implements the protocol for
> communication
> with the MM environment in the secure world.
> 
> The secure world image includes:
> The MM Standalone framework.
> 
> This RFC patchset includes the proposed organization/structure and
> has architecture agnostic core changes only as a first pass.
> Once the organization/structure have been agreed upon, patchset for
> supporting libraries and AARCH64 implementation will be sent.
> 
> Steps to build MM Standalone images
> In user preferred "work" directory, execute the following shell commands
> 
> git clone https://github.com/tianocore/edk2.git
> git checkout master
> 
> git clone https://github.com/tianocore/edk2-platforms.git
> git checkout master
> 
> mkdir arm-tf
> cd arm-tf
> git clone https://github.com/ARM-software/arm-trusted-firmware.git .
> git checkout master
> cd ..
> 
> git clone https://git.linaro.org/uefi/uefi-tools.git .
> git checkout master
> 
> The following will build the MM Standalone image which runs in secure world.
> ./uefi-tools/edk2-build.sh -b DEBUG fvp_mm_standalone
> 
> The follwing will build the normal world UEFI image, ARM Trusted Firmware and
> a Firmware Image Package (FIP) that includes both the UEFI images.
> ./uefi-tools/edk2-build.sh -a ./arm-tf -b DEBUG fvp_mm_normal
> 
> Boot Loader Stage 1 (BL1) binary and combined arm-tf/uefi firmware image
> package (fip) binary will be generated at:
> 
> Build Output
> Build/ArmVExpress-FVP-AArch64-MM-Normal/DEBUG_GCC5/FV/bl1.bin
> Build/ArmVExpress-FVP-AArch64-MM-Normal/DEBUG_GCC5/FV/fip.bin
> 
> Steps to run MM Standalone image
> 1. Download the ARMv8 Architecture FVP from
> 
> https://silver.arm.com/download/download.tm?pv=3744408=1424570
>  For more information, please refer
> 
> https://developer.arm.com/products/system-design/fixed-virtual-platforms
> 2.  Install FVP into preferred "work" directory.
> 3.  Create a shell script "run_mm.sh" in the same folder where
> "FVP_Base_AEMv8A-AEMv8A" is present.
>  Sample Shell script below:
> ./FVP_Base_AEMv8A-AEMv8A
> -C cache_state_modelled=0
> -C bp.secure_memory=1
> -C bp.tzc_400.diagnostics=1
> -C bp.pl011_uart0.untimed_fifos=0
> -C cluster1.NUM_CORES=4
> -C cluster0.NUM_CORES=4
> -C bp.pl011_uart0.out_file=uart0.output
> -C bp.pl011_uart1.out_file=uart1.output
> -C bp.pl011_uart2.out_file=uart2.output
> -C bp.pl011_uart3.out_file=uart3.output
> -C bp.secureflashloader.fname=""
> -C bp.flashloader0.fname=""
> -S -R
> 4. ./run_mm.sh
> 5. Output can be seen on FVP console.
> 6. The normal world will boot to the UEFI shell.
> 
> Sample Output
> 
> MM Standalone Output (FVP UART2)
> SPM Version: Major=0x0, Minor=0x1
> NumSpMemRegions - 0x6
> SpMemBase   - 0xFF20
> SpMemLimit  - 0x1
> SpImageBase - 0xFF20
> SpStackBase - 0xFF61
> SpHeapBase  - 0xFF62
> SpNsCommBufBase - 0xFF60
> SpSharedBufBase - 0xFF50
> SpImageSize - 0x30
> SpPcpuStackSize - 0x2000
> SpHeapSize  - 0x9E
> SpNsCommBufSize - 0x1
> SpPcpuSharedBufSize - 0x2
> NumCpus - 0x8
> CpuInfo - 0xFF500680
> Mpidr   - 0x8000
> LinearId- 0x0
> Flags   - 0x1
> Mpidr   - 0x8001
> LinearId- 0x1
> Flags 

Re: [edk2] [patch] PcAtChipsetPkg: Add description for new added PCD in commit e78aab9d2

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 

Thanks/Ray

> -Original Message-
> From: Bi, Dandan
> Sent: Tuesday, November 28, 2017 11:51 AM
> To: edk2-devel@lists.01.org
> Cc: Leo Duran ; Ni, Ruiyu 
> Subject: [patch] PcAtChipsetPkg: Add description for new added PCD in
> commit e78aab9d2
> 
> Cc: Leo Duran 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi 
> ---
>  PcAtChipsetPkg/PcAtChipsetPkg.uni | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.uni
> b/PcAtChipsetPkg/PcAtChipsetPkg.uni
> index 530e9ae..00169ce 100644
> --- a/PcAtChipsetPkg/PcAtChipsetPkg.uni
> +++ b/PcAtChipsetPkg/PcAtChipsetPkg.uni
> @@ -154,5 +154,14 @@
>  "Reset Control Register value for cold reset"
> 
>  #string
> STR_gPcAtChipsetPkgTokenSpaceGuid_PcdResetControlValueColdReset_HE
> LP
>  #language en-US
>  "8bit Reset Control Register value for cold reset."
> +
> +#string
> STR_gPcAtChipsetPkgTokenSpaceGuid_PcdInitialValueRtcRegisterA_PROMP
> T  #language en-US "Initial value for Register_A in RTC."
> +#string
> STR_gPcAtChipsetPkgTokenSpaceGuid_PcdInitialValueRtcRegisterA_HELP
> #language en-US "Specifies the initial value for Register_A in RTC."
> +
> +#string
> STR_gPcAtChipsetPkgTokenSpaceGuid_PcdInitialValueRtcRegisterB_PROMP
> T  #language en-US "Initial value for Register_B in RTC."
> +#string
> STR_gPcAtChipsetPkgTokenSpaceGuid_PcdInitialValueRtcRegisterB_HELP
> #language en-US "Specifies the initial value for Register_B in RTC."
> +
> +#string
> STR_gPcAtChipsetPkgTokenSpaceGuid_PcdInitialValueRtcRegisterD_PROMP
> T  #language en-US "Initial value for Register_D in RTC."
> +#string
> STR_gPcAtChipsetPkgTokenSpaceGuid_PcdInitialValueRtcRegisterD_HELP
> #language en-US "Specifies the initial value for Register_D in RTC."
> --
> 1.9.5.msysgit.1

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

[edk2] [patch] PcAtChipsetPkg: Add description for new added PCD in commit e78aab9d2

[Dandan Bi]

Cc: Leo Duran 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 PcAtChipsetPkg/PcAtChipsetPkg.uni | 9 +
 1 file changed, 9 insertions(+)

diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.uni 
b/PcAtChipsetPkg/PcAtChipsetPkg.uni
index 530e9ae..00169ce 100644
--- a/PcAtChipsetPkg/PcAtChipsetPkg.uni
+++ b/PcAtChipsetPkg/PcAtChipsetPkg.uni
@@ -154,5 +154,14 @@
 "Reset Control Register value for cold reset"
 
 #string STR_gPcAtChipsetPkgTokenSpaceGuid_PcdResetControlValueColdReset_HELP
 #language en-US
 "8bit Reset Control Register value for cold reset."
+
+#string STR_gPcAtChipsetPkgTokenSpaceGuid_PcdInitialValueRtcRegisterA_PROMPT  
#language en-US "Initial value for Register_A in RTC."
+#string STR_gPcAtChipsetPkgTokenSpaceGuid_PcdInitialValueRtcRegisterA_HELP
#language en-US "Specifies the initial value for Register_A in RTC."
+
+#string STR_gPcAtChipsetPkgTokenSpaceGuid_PcdInitialValueRtcRegisterB_PROMPT  
#language en-US "Initial value for Register_B in RTC."
+#string STR_gPcAtChipsetPkgTokenSpaceGuid_PcdInitialValueRtcRegisterB_HELP
#language en-US "Specifies the initial value for Register_B in RTC."
+
+#string STR_gPcAtChipsetPkgTokenSpaceGuid_PcdInitialValueRtcRegisterD_PROMPT  
#language en-US "Initial value for Register_D in RTC."
+#string STR_gPcAtChipsetPkgTokenSpaceGuid_PcdInitialValueRtcRegisterD_HELP
#language en-US "Specifies the initial value for Register_D in RTC."
-- 
1.9.5.msysgit.1

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

[edk2] [Patch] SignedCapsulePkg: Update EdkiiSystemCapsuleLib to check PCD value

[Liming Gao]

If PCD value is not set, register PcdCallBack to hook PCD value set

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Jiewen Yao 
---
 .../EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.c  | 86 +-
 .../EdkiiSystemCapsuleLib.inf  |  3 +
 2 files changed, 87 insertions(+), 2 deletions(-)

diff --git 
a/SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.c 
b/SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.c
index 62be8eb..876d225 100644
--- a/SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.c
+++ b/SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -600,6 +601,10 @@ CapsuleAuthenticateSystemFirmware (
   // NOTE: This function need run in an isolated environment.
   // Do not touch FMP protocol and its private structure.
   //
+  if (mImageFmpInfo == NULL) {
+DEBUG((DEBUG_INFO, "ImageFmpInfo is not set\n"));
+return EFI_SECURITY_VIOLATION;
+  }
 
   Result = ExtractAuthenticatedImage((VOID *)Image, ImageSize, 
LastAttemptStatus, AuthenticatedImage, AuthenticatedImageSize);
   if (!Result) {
@@ -655,6 +660,53 @@ CapsuleAuthenticateSystemFirmware (
 }
 
 /**
+  PcdCallBack gets the real set PCD value
+
+  @param[in]  CallBackGuidThe PCD token GUID being set.
+  @param[in]  CallBackToken   The PCD token number being set.
+  @param[in, out] TokenData   A pointer to the token data being set.
+  @param[in]  TokenDataSize   The size, in bytes, of the data being set.
+
+**/
+VOID
+EFIAPI
+EdkiiSystemCapsuleLibPcdCallBack (
+  INCONST GUID*CallBackGuid, OPTIONAL
+  INUINTN CallBackToken,
+  IN  OUT   VOID  *TokenData,
+  INUINTN TokenDataSize
+  )
+{
+  if (CompareGuid (CallBackGuid, ) &&
+  CallBackToken == PcdToken (PcdEdkiiSystemFirmwareImageDescriptor)) {
+mImageFmpInfoSize = TokenDataSize;
+mImageFmpInfo = AllocateCopyPool (mImageFmpInfoSize, TokenData);
+ASSERT(mImageFmpInfo != NULL);
+//
+// Cancel Callback after get the real set value
+//
+LibPcdCancelCallback (
+  ,
+  PcdToken (PcdEdkiiSystemFirmwareImageDescriptor),
+  EdkiiSystemCapsuleLibPcdCallBack
+  );
+  }
+
+  if (CompareGuid (CallBackGuid, ) &&
+  CallBackToken == PcdToken (PcdEdkiiSystemFirmwareFileGuid)) {
+CopyGuid(, TokenData);
+//
+// Cancel Callback after get the real set value
+//
+LibPcdCancelCallback (
+  ,
+  PcdToken (PcdEdkiiSystemFirmwareFileGuid),
+  EdkiiSystemCapsuleLibPcdCallBack
+  );
+  }
+}
+
+/**
   The constructor function.
 
   @retval EFI_SUCCESS   The constructor successfully .
@@ -666,8 +718,38 @@ EdkiiSystemCapsuleLibConstructor (
   )
 {
   mImageFmpInfoSize = PcdGetSize(PcdEdkiiSystemFirmwareImageDescriptor);
-  mImageFmpInfo = AllocateCopyPool (mImageFmpInfoSize, 
PcdGetPtr(PcdEdkiiSystemFirmwareImageDescriptor));
-  ASSERT(mImageFmpInfo != NULL);
+  mImageFmpInfo = PcdGetPtr(PcdEdkiiSystemFirmwareImageDescriptor);
+  //
+  // Verify Firmware Image Descriptor first
+  //
+  if (mImageFmpInfoSize < sizeof (EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR) ||
+  mImageFmpInfo->Signature != 
EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR_SIGNATURE) {
+//
+// SystemFirmwareImageDescriptor is not set.
+// Register PCD set callback to hook PCD value set.
+//
+mImageFmpInfo = NULL;
+mImageFmpInfoSize = 0;
+LibPcdCallbackOnSet (
+  ,
+  PcdToken (PcdEdkiiSystemFirmwareImageDescriptor),
+  EdkiiSystemCapsuleLibPcdCallBack
+  );
+  } else {
+mImageFmpInfo = AllocateCopyPool (mImageFmpInfoSize, mImageFmpInfo);
+ASSERT(mImageFmpInfo != NULL);
+  }
+
   CopyGuid(, 
PcdGetPtr(PcdEdkiiSystemFirmwareFileGuid));
+  //
+  // Verify GUID value first
+  //
+  if (CompareGuid (, )) {
+LibPcdCallbackOnSet (
+  ,
+  PcdToken (PcdEdkiiSystemFirmwareFileGuid),
+  EdkiiSystemCapsuleLibPcdCallBack
+  );
+  }
   return EFI_SUCCESS;
 }
diff --git 
a/SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.inf 
b/SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.inf
index a21e75c..a721619 100644
--- a/SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.inf
+++ b/SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.inf
@@ -43,6 +43,7 @@
   BaseLib
   BaseMemoryLib
   DebugLib
+  PcdLib
   MemoryAllocationLib
   FmpAuthenticationLib
 
@@ -58,4 +59,6 @@
   gEdkiiSystemFmpCapsuleDriverFvFileGuid   ## SOMETIMES_CONSUMES   
## GUID
   gEfiCertPkcs7Guid## SOMETIMES_CONSUMES   
## GUID
   gEfiCertTypeRsa2048Sha256Guid## SOMETIMES_CONSUMES   
## GUID
+  gEfiSignedCapsulePkgTokenSpaceGuid  

[edk2] [Patch] MdeModulePkg: Update PeiCore to pass XCODE tool chain

[Liming Gao]

It fixes the warning for loop has empty body [-Werror,-Wempty-body].

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liang Vincent 
---
 MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c 
b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
index 467066a..c223c22 100644
--- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
@@ -676,7 +676,8 @@ PeiCheckAndSwitchStack (
   for (StackPointer = (UINT32*)SecCoreData->StackBase;
(StackPointer < (UINT32*)((UINTN)SecCoreData->StackBase + 
SecCoreData->StackSize)) \
&& (*StackPointer == PcdGet32 (PcdInitValueInTempStack));
-   StackPointer ++);
+   StackPointer ++) {
+}
 
   DEBUG ((DEBUG_INFO, "Temp Stack : BaseAddress=0x%p Length=0x%X\n", 
SecCoreData->StackBase, (UINT32)SecCoreData->StackSize));
   DEBUG ((DEBUG_INFO, "Temp Heap  : BaseAddress=0x%p Length=0x%X\n", 
SecCoreData->PeiTemporaryRamBase, (UINT32)SecCoreData->PeiTemporaryRamSize));
-- 
2.8.0.windows.1

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

[edk2] [Patch] NetworkPkg: Update IScsiDxe to pass XCODE build

[Liming Gao]

Fix the warning equality comparison with extraneous parentheses
[-Werror,-Wparentheses-equality].

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liang Vincent 
---
 NetworkPkg/IScsiDxe/IScsiDhcp.c| 2 +-
 NetworkPkg/IScsiDxe/IScsiExtScsiPassThru.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiDhcp.c b/NetworkPkg/IScsiDxe/IScsiDhcp.c
index 6587a05..309ce0d 100644
--- a/NetworkPkg/IScsiDxe/IScsiDhcp.c
+++ b/NetworkPkg/IScsiDxe/IScsiDhcp.c
@@ -266,7 +266,7 @@ IScsiDhcpSelectOffer (
 break;
   }
 
-  if ((Index == OptionCount)) {
+  if (Index == OptionCount) {
 Status = EFI_NOT_READY;
   }
 
diff --git a/NetworkPkg/IScsiDxe/IScsiExtScsiPassThru.c 
b/NetworkPkg/IScsiDxe/IScsiExtScsiPassThru.c
index f971244..90e26d4 100644
--- a/NetworkPkg/IScsiDxe/IScsiExtScsiPassThru.c
+++ b/NetworkPkg/IScsiDxe/IScsiExtScsiPassThru.c
@@ -1,7 +1,7 @@
 /** @file
   The implementation of EFI_EXT_SCSI_PASS_THRU_PROTOCOL.
 
-Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2004 - 2017, 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
@@ -219,7 +219,7 @@ IScsiExtScsiPassThruBuildDevicePath (
   EFI_DEV_PATH  *Node;
   UINTN DevPathNodeLen;
 
-  if ((DevicePath == NULL)) {
+  if (DevicePath == NULL) {
 return EFI_INVALID_PARAMETER;
   }
 
-- 
2.8.0.windows.1

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

[edk2] [PATCH v2 3/3] BaseTools: Update C tools top GNUMakefile to adjust tool order

[Liming Gao]

Place the tool that takes much build time at the first. This can improve
build performance when make -j N used.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
---
 BaseTools/Source/C/GNUmakefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/C/GNUmakefile b/BaseTools/Source/C/GNUmakefile
index 50b6759..0dc7482 100644
--- a/BaseTools/Source/C/GNUmakefile
+++ b/BaseTools/Source/C/GNUmakefile
@@ -51,9 +51,10 @@ LIBRARIES = Common
 VFRAUTOGEN = VfrCompile/VfrLexer.h
 # NON_BUILDABLE_APPLICATIONS = GenBootSector BootSectImage
 APPLICATIONS = \
+  BrotliCompress \
+  VfrCompile \
   GnuGenBootSector \
   BootSectImage \
-  BrotliCompress \
   EfiLdrImage \
   EfiRom \
   GenFfs \
@@ -66,8 +67,7 @@ APPLICATIONS = \
   LzmaCompress \
   Split \
   TianoCompress \
-  VolInfo \
-  VfrCompile
+  VolInfo
 
 SUBDIRS := $(LIBRARIES) $(APPLICATIONS)
 
-- 
2.8.0.windows.1

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

[edk2] [PATCH v2 1/3] BaseTools: Replace ARCH with HOST_ARCH in C Makefile to avoid conflict

[Liming Gao]

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

ARCH is too generic. It may cause confuse of target arch or host arch.
To be clarified, replace it with HOST_ARCH in BaseTools C Makefile.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
---
 BaseTools/Source/C/BootSectImage/GNUmakefile|  2 +-
 BaseTools/Source/C/BrotliCompress/GNUmakefile   |  2 +-
 BaseTools/Source/C/Common/GNUmakefile   |  2 +-
 BaseTools/Source/C/EfiLdrImage/GNUmakefile  |  2 +-
 BaseTools/Source/C/EfiRom/GNUmakefile   |  2 +-
 BaseTools/Source/C/GNUmakefile  | 28 -
 BaseTools/Source/C/GenCrc32/GNUmakefile |  2 +-
 BaseTools/Source/C/GenFfs/GNUmakefile   |  2 +-
 BaseTools/Source/C/GenFv/GNUmakefile|  2 +-
 BaseTools/Source/C/GenFw/GNUmakefile|  2 +-
 BaseTools/Source/C/GenPage/GNUmakefile  |  2 +-
 BaseTools/Source/C/GenSec/GNUmakefile   |  2 +-
 BaseTools/Source/C/GenVtf/GNUmakefile   |  2 +-
 BaseTools/Source/C/GnuGenBootSector/GNUmakefile |  2 +-
 BaseTools/Source/C/LzmaCompress/GNUmakefile |  2 +-
 BaseTools/Source/C/Makefile |  2 +-
 BaseTools/Source/C/Makefiles/footer.makefile|  8 +++
 BaseTools/Source/C/Makefiles/header.makefile| 20 +-
 BaseTools/Source/C/Makefiles/ms.common  |  8 +++
 BaseTools/Source/C/Split/GNUmakefile|  2 +-
 BaseTools/Source/C/TianoCompress/GNUmakefile|  2 +-
 BaseTools/Source/C/VfrCompile/GNUmakefile   |  2 +-
 BaseTools/Source/C/VolInfo/GNUmakefile  |  2 +-
 23 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/BaseTools/Source/C/BootSectImage/GNUmakefile 
b/BaseTools/Source/C/BootSectImage/GNUmakefile
index 5f7cb98..90800a4 100644
--- a/BaseTools/Source/C/BootSectImage/GNUmakefile
+++ b/BaseTools/Source/C/BootSectImage/GNUmakefile
@@ -10,7 +10,7 @@
 # THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
 # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #
-ARCH ?= IA32
+HOST_ARCH ?= IA32
 MAKEROOT ?= ..
 
 APPNAME = BootSectImage
diff --git a/BaseTools/Source/C/BrotliCompress/GNUmakefile 
b/BaseTools/Source/C/BrotliCompress/GNUmakefile
index 368edbd..3426a00 100644
--- a/BaseTools/Source/C/BrotliCompress/GNUmakefile
+++ b/BaseTools/Source/C/BrotliCompress/GNUmakefile
@@ -10,7 +10,7 @@
 # THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
 # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #
-ARCH ?= IA32
+HOST_ARCH ?= IA32
 MAKEROOT ?= ..
 
 APPNAME = Brotli
diff --git a/BaseTools/Source/C/Common/GNUmakefile 
b/BaseTools/Source/C/Common/GNUmakefile
index a193557..5cbca9a 100644
--- a/BaseTools/Source/C/Common/GNUmakefile
+++ b/BaseTools/Source/C/Common/GNUmakefile
@@ -10,7 +10,7 @@
 # THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
 # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #
-ARCH ?= IA32
+HOST_ARCH ?= IA32
 MAKEROOT ?= ..
 
 # VPATH = ..
diff --git a/BaseTools/Source/C/EfiLdrImage/GNUmakefile 
b/BaseTools/Source/C/EfiLdrImage/GNUmakefile
index 99f786f..75c04ea 100644
--- a/BaseTools/Source/C/EfiLdrImage/GNUmakefile
+++ b/BaseTools/Source/C/EfiLdrImage/GNUmakefile
@@ -10,7 +10,7 @@
 # THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
 # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #
-ARCH ?= IA32
+HOST_ARCH ?= IA32
 MAKEROOT ?= ..
 
 APPNAME = EfiLdrImage
diff --git a/BaseTools/Source/C/EfiRom/GNUmakefile 
b/BaseTools/Source/C/EfiRom/GNUmakefile
index 433c126..a13111c 100644
--- a/BaseTools/Source/C/EfiRom/GNUmakefile
+++ b/BaseTools/Source/C/EfiRom/GNUmakefile
@@ -10,7 +10,7 @@
 # THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
 # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #
-ARCH ?= IA32
+HOST_ARCH ?= IA32
 MAKEROOT ?= ..
 
 APPNAME = EfiRom
diff --git a/BaseTools/Source/C/GNUmakefile b/BaseTools/Source/C/GNUmakefile
index 83e188c..53ddb67 100644
--- a/BaseTools/Source/C/GNUmakefile
+++ b/BaseTools/Source/C/GNUmakefile
@@ -12,40 +12,40 @@
 #  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
 #
 
-ifndef ARCH
+ifndef HOST_ARCH
   #
-  # If ARCH is not defined, then we use 'uname -m' to attempt
-  # try to figure out the appropriate ARCH.
+  # If HOST_ARCH is not defined, then we use 'uname -m' to attempt
+  # try to figure out the appropriate HOST_ARCH.
   #
   uname_m = $(shell uname -m)
-  $(info Attempting to detect ARCH from 'uname -m': $(uname_m))
+  $(info Attempting to detect HOST_ARCH from 'uname -m': $(uname_m))
   ifneq (,$(strip $(filter $(uname_m), x86_64 amd64)))
-ARCH=X64
+HOST_ARCH=X64
   endif
   ifeq ($(patsubst i%86,IA32,$(uname_m)),IA32)
-ARCH=IA32
+HOST_ARCH=IA32
   endif
   ifneq 

[edk2] [PATCH v2 2/3] BaseTools: Update BaseTools top GNUMakefile with the clear dependency

[Liming Gao]

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

After GNUmakefile dependency is fixed up, it can make with -j N to enable
multiple thread build in base tools C source and save build time.

In my linux host machine, make -j 4 to compile BaseTools and save ~60% time.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
---
 BaseTools/GNUmakefile  |  8 +++-
 BaseTools/Source/C/GNUmakefile | 11 +--
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/BaseTools/GNUmakefile b/BaseTools/GNUmakefile
index 790d33a..6325e40 100644
--- a/BaseTools/GNUmakefile
+++ b/BaseTools/GNUmakefile
@@ -1,7 +1,7 @@
 ## @file
 # GNU/Linux makefile for Base Tools project build.
 #
-# Copyright (c) 2009 - 2010, Intel Corporation. All rights reserved.
+# Copyright (c) 2009 - 2017, 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
@@ -24,12 +24,10 @@ subdirs: $(SUBDIRS)
 $(SUBDIRS):
$(MAKE) -C $@
 
+Tests: $(SOURCE_SUBDIRS)
+
 .PHONY: $(CLEAN_SUBDIRS)
 $(CLEAN_SUBDIRS):
-$(MAKE) -C $(@:-clean=) clean
 
 clean:  $(CLEAN_SUBDIRS)
-
-test:
-   @$(MAKE) -C Tests
-
diff --git a/BaseTools/Source/C/GNUmakefile b/BaseTools/Source/C/GNUmakefile
index 53ddb67..50b6759 100644
--- a/BaseTools/Source/C/GNUmakefile
+++ b/BaseTools/Source/C/GNUmakefile
@@ -1,7 +1,7 @@
 ## @file
 #  GNU/Linux makefile for C tools build.
 #
-#  Copyright (c) 2007 - 2012, Intel Corporation. All rights reserved.
+#  Copyright (c) 2007 - 2017, 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
@@ -44,10 +44,11 @@ MAKEROOT = .
 
 include Makefiles/header.makefile
 
-all: makerootdir subdirs $(MAKEROOT)/libs
+all: makerootdir subdirs
@echo Finished building BaseTools C Tools with HOST_ARCH=$(HOST_ARCH)
 
 LIBRARIES = Common
+VFRAUTOGEN = VfrCompile/VfrLexer.h
 # NON_BUILDABLE_APPLICATIONS = GenBootSector BootSectImage
 APPLICATIONS = \
   GnuGenBootSector \
@@ -70,6 +71,9 @@ APPLICATIONS = \
 
 SUBDIRS := $(LIBRARIES) $(APPLICATIONS)
 
+$(LIBRARIES): $(MAKEROOT)/libs
+$(APPLICATIONS): $(LIBRARIES) $(MAKEROOT)/bin $(VFRAUTOGEN)
+
 .PHONY: outputdirs
 makerootdir:
-mkdir -p $(MAKEROOT)
@@ -83,6 +87,9 @@ $(SUBDIRS):
 $(patsubst %,%-clean,$(sort $(SUBDIRS))):
-$(MAKE) -C $(@:-clean=) clean
 
+$(VFRAUTOGEN): VfrCompile/VfrSyntax.g 
+   $(MAKE) -C VfrCompile VfrLexer.h
+
 clean:  $(patsubst %,%-clean,$(sort $(SUBDIRS)))
 
 clean: localClean
-- 
2.8.0.windows.1

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

[edk2] [PATCH v2 0/3] BaseTools: Update Makefiles with clear ARCH and dependency

[Liming Gao]

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

In V2: 
  Update BaseTools top GNUmakefile with the clear dependency

Liming Gao (3):
  BaseTools: Replace ARCH with HOST_ARCH in C Makefile to avoid conflict
  BaseTools: Update BaseTools top GNUMakefile with the clear dependency
  BaseTools: Update C tools top GNUMakefile to adjust tool order

 BaseTools/GNUmakefile   |  8 ++---
 BaseTools/Source/C/BootSectImage/GNUmakefile|  2 +-
 BaseTools/Source/C/BrotliCompress/GNUmakefile   |  2 +-
 BaseTools/Source/C/Common/GNUmakefile   |  2 +-
 BaseTools/Source/C/EfiLdrImage/GNUmakefile  |  2 +-
 BaseTools/Source/C/EfiRom/GNUmakefile   |  2 +-
 BaseTools/Source/C/GNUmakefile  | 45 ++---
 BaseTools/Source/C/GenCrc32/GNUmakefile |  2 +-
 BaseTools/Source/C/GenFfs/GNUmakefile   |  2 +-
 BaseTools/Source/C/GenFv/GNUmakefile|  2 +-
 BaseTools/Source/C/GenFw/GNUmakefile|  2 +-
 BaseTools/Source/C/GenPage/GNUmakefile  |  2 +-
 BaseTools/Source/C/GenSec/GNUmakefile   |  2 +-
 BaseTools/Source/C/GenVtf/GNUmakefile   |  2 +-
 BaseTools/Source/C/GnuGenBootSector/GNUmakefile |  2 +-
 BaseTools/Source/C/LzmaCompress/GNUmakefile |  2 +-
 BaseTools/Source/C/Makefile |  2 +-
 BaseTools/Source/C/Makefiles/footer.makefile|  8 ++---
 BaseTools/Source/C/Makefiles/header.makefile| 20 +--
 BaseTools/Source/C/Makefiles/ms.common  |  8 ++---
 BaseTools/Source/C/Split/GNUmakefile|  2 +-
 BaseTools/Source/C/TianoCompress/GNUmakefile|  2 +-
 BaseTools/Source/C/VfrCompile/GNUmakefile   |  2 +-
 BaseTools/Source/C/VolInfo/GNUmakefile  |  2 +-
 24 files changed, 66 insertions(+), 61 deletions(-)

-- 
2.8.0.windows.1

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

Re: [edk2] [PATCH] IntelFsp2WrapperPkg: Support UPD allocation outside FspWrapper

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Chasel,
> Chiu
> Sent: Thursday, November 23, 2017 10:30 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen 
> Subject: [edk2] [PATCH] IntelFsp2WrapperPkg: Support UPD allocation outside
> FspWrapper
> 
> UPD allocation and patching can be done outside FspWrapper
> as implementation choice so adding a PCD to select between
> original FspWrapper allocation model or outside model
> 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chasel Chiu 
> ---
>  .../FspmWrapperPeim/FspmWrapperPeim.c  | 25 ---
>  .../FspmWrapperPeim/FspmWrapperPeim.inf|  3 +-
>  .../FspsWrapperPeim/FspsWrapperPeim.c  | 84
> +++---
>  .../FspsWrapperPeim/FspsWrapperPeim.inf|  3 +-
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec| 13 +++-
>  5 files changed, 76 insertions(+), 52 deletions(-)
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> index f1d1cd6421..7b7c5f5d86 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> @@ -3,7 +3,7 @@
>register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi
>notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.
> +  Copyright (c) 2014 - 2017, 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
> @@ -63,20 +63,29 @@ PeiFspMemoryInit (
>DEBUG ((DEBUG_INFO, "PeiFspMemoryInit enter\n"));
> 
>FspHobListPtr = NULL;
> +  FspmUpdDataPtr = NULL;
> 
> -  //
> -  // Copy default FSP-M UPD data from Flash
> -  //
>FspmHeaderPtr = (FSP_INFO_HEADER *)FspFindFspHeader (PcdGet32
> (PcdFspmBaseAddress));
>DEBUG ((DEBUG_INFO, "FspmHeaderPtr - 0x%x\n", FspmHeaderPtr));
>if (FspmHeaderPtr == NULL) {
>  return EFI_DEVICE_ERROR;
>}
> 
> -  FspmUpdDataPtr = (FSPM_UPD_COMMON *)AllocateZeroPool
> ((UINTN)FspmHeaderPtr->CfgRegionSize);
> -  ASSERT (FspmUpdDataPtr != NULL);
> -  SourceData = (UINTN *)((UINTN)FspmHeaderPtr->ImageBase +
> (UINTN)FspmHeaderPtr->CfgRegionOffset);
> -  CopyMem (FspmUpdDataPtr, SourceData,
> (UINTN)FspmHeaderPtr->CfgRegionSize);
> +  if (PcdGet32 (PcdFspmUpdDataAddress) == 0 &&
> (FspmHeaderPtr->CfgRegionSize != 0) && (FspmHeaderPtr->CfgRegionOffset !=
> 0)) {
> +//
> +// Copy default FSP-M UPD data from Flash
> +//
> +FspmUpdDataPtr = (FSPM_UPD_COMMON *)AllocateZeroPool
> ((UINTN)FspmHeaderPtr->CfgRegionSize);
> +ASSERT (FspmUpdDataPtr != NULL);
> +SourceData = (UINTN *)((UINTN)FspmHeaderPtr->ImageBase +
> (UINTN)FspmHeaderPtr->CfgRegionOffset);
> +CopyMem (FspmUpdDataPtr, SourceData,
> (UINTN)FspmHeaderPtr->CfgRegionSize);
> +  } else {
> +//
> +// External UPD is ready, get the buffer from PCD pointer.
> +//
> +FspmUpdDataPtr = (FSPM_UPD_COMMON *)PcdGet32
> (PcdFspmUpdDataAddress);
> +ASSERT (FspmUpdDataPtr != NULL);
> +  }
> 
>DEBUG ((DEBUG_INFO, "UpdateFspmUpdData enter\n"));
>UpdateFspmUpdData ((VOID *)FspmUpdDataPtr);
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> index 2b3d240d08..542356b582 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> @@ -59,7 +59,8 @@
>IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> 
>  [Pcd]
> -  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress  ## CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress ##
> CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress  ##
> CONSUMES
> 
>  [Sources]
>FspmWrapperPeim.c
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> index ddc19c7e8f..70dac7a414 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> @@ -3,7 +3,7 @@
>register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi
>notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.
> +  Copyright (c) 2014 - 2017, 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

Re: [edk2] [PATCH v2 7/8] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support

[Yao, Jiewen]

Thanks Jian.

I just talked to some other person, who think it is valuable to have stack 
guard in PEI as well, because they have seen stack overflow issue in PEI. I do 
think we need consider that, although it is not implemented in this patch 
series.

My thought for API is that the API design for PEI/DXE/SMM should be consistent.
When people look at the MdeModulePkg\Include\Library\CpuExceptionHandlerLib.h, 
he can know clearly on which API should be called and what is done in each API.

If we call InitializeCpuExceptionStackSwitchHandlers() inside 
InitializeCpuExceptionHandlers(), we should document this in .H file and make 
PEI/DXE/SMM version all implement in this way.
If we did one way for DXE, the other way for PEI, and another way for SMM, it 
might bring confusing.


Thank you
Yao Jiewen

> -Original Message-
> From: Wang, Jian J
> Sent: Tuesday, November 28, 2017 9:39 AM
> To: Yao, Jiewen ; edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Dong, Eric
> ; Zeng, Star 
> Subject: RE: [PATCH v2 7/8] UefiCpuPkg/CpuExceptionHandlerLib: Add stack
> switch support
> 
> Sorry, 1.3 should be for your 1.4. I just noticed I missed your 1.3 comment.
> Here's my opinion for it:
> 
> Current changes (use global variables to reserve resources) to
> InitializeCpuExceptionHandlers() is for DXE only. For PEI, if we really need 
> to
> worry
> about the stack overflow and stack switch, we can have a different way to do 
> it.
> For
> example, we don't need to call InitializeCpuExceptionStackSwitchHandlers()
> inside
> InitializeCpuExceptionHandlers(). We could call it whenever we can reserve
> memory blocks
> and pass them to InitializeCpuExceptionStackSwitchHandlers() via parameter. I
> think this
> is one of reason we have a separate method to initialize the exception 
> handlers
> for the
> sake of stack switch.
> 
> Calling InitializeCpuExceptionStackSwitchHandlers() inside
> InitializeCpuExceptionHandlers()
> is just for the consideration of backward compatibility. Because this new API 
> is
> just
> implemented for IA32 processor at this time, calling it in DXE core will 
> break the
> build of
> other type of processors. This is another reason we have a separate method to
> do
> exception handler initialization in a different way.
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Wang,
> > Jian J
> > Sent: Thursday, November 23, 2017 2:44 PM
> > To: Yao, Jiewen ; edk2-devel@lists.01.org
> > Cc: Kinney, Michael D ; Dong, Eric
> > ; Zeng, Star 
> > Subject: Re: [edk2] [PATCH v2 7/8] UefiCpuPkg/CpuExceptionHandlerLib: Add
> > stack switch support
> >
> > 1.1) Got your point. I'll add dummy function in this patch.
> > 1.2) Yep, we're on the same page.
> > 1.3) Here's my opinion:
> >
> > Actually almost all MP code has such assumption: any AP configuration will
> copy
> >  from BSP. If we allow AP to call InitializeCpuExceptionHandlers(), we have 
> > to
> do
> > a lot
> > of other changes than just updating InitializeCpuExceptionHandlers(). If 
> > so, it
> > may
> > be premature to figure out a solution at this patch.
> >
> > In addition, CpuDxe actually calls InitializeCpuInterruptHandlers() which 
> > covers
> > the
> > functionalities of InitializeCpuExceptionHandlers() (its settings will be
> > overwritten).
> > If we want AP to initialize interrupt and exception individually, maybe we
> should
> > let AP call InitializeCpuInterruptHandlers() instead.
> >
> > > -Original Message-
> > > From: Yao, Jiewen
> > > Sent: Thursday, November 23, 2017 2:16 PM
> > > To: Wang, Jian J ; edk2-devel@lists.01.org
> > > Cc: Zeng, Star ; Dong, Eric ;
> > > Kinney, Michael D 
> > > Subject: RE: [PATCH v2 7/8] UefiCpuPkg/CpuExceptionHandlerLib: Add stack
> > > switch support
> > >
> > > Here is my thought for 1)
> > >
> > > 1.1) We must provide the InitializeCpuExceptionStackSwitchHandlers()
> > > implementation in Pei instance and Smm instance.
> > >
> > > The basic requirement is a library instance must provide symbol for 
> > > functions
> > > declared in header file.
> > > It is ok to return unsupported. But we MUST provide the symbol.
> > >
> > > 1.2) For SMM, I think our ultimate goal is to remove SMM specific stack
> guard,
> > > and use the common one. Duplicating code is completely unnecessary, and it
> is
> > > easy to introduce bug. And unfortunately, we already have bug in existing
> > SMM
> > > exception handler. -- That is a good reason to remove duplication.
> > >
> > > Again, it is not necessary to do it in this patch. I am totally OK to do 
> > > it in
> > another
> > > patch.
> > >
> > > 1.3) For PEI, I do not think we can use current way to allocate stack in 

Re: [edk2] [PATCH v3 0/4] Add VS2017 tool chain for evaluation

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu 

Best Regards,
Zhu Yonghong

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Pete 
Batard
Sent: Thursday, November 23, 2017 12:26 AM
To: edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: [edk2] [PATCH v3 0/4] Add VS2017 tool chain for evaluation

This is a re-send of v2, with a small comment update to indicate that
VS2017 v15.2 or later is required, due to the vswhere.exe requirement.

This patch serial adds VS2017 tool chain in BaseTools tools_def.template.
It enables /WHOLEARCHIVE option to detect the potential code issue.
It can be used to build source code with Visual Studio 2017 on 32bit or 64bit 
Windows OS. After this tool chain is evaluated, ASL, ARM and ARM64 support can 
be provided.
And, to avoid more duplicated informations be introduced, the proposal to 
simplify tools_def.template will be provided.


Liming Gao (4):
  MdePkg: Disable VS warning 4701 & 4703 for VS2017
  BaseTools: Add VS2017 tool chain in BaseTools tools_def.template
  BaseTools: Update VS batch file to auto detect VS2017
  Nt32Pkg: Add VS2017 support in SecMain

 BaseTools/Conf/tools_def.template   | 126 
 BaseTools/get_vsvars.bat|   8 ++
 BaseTools/set_vsprefix_envs.bat |  33 -
 MdePkg/Include/Ia32/ProcessorBind.h |   4 +-
 MdePkg/Include/X64/ProcessorBind.h  |   4 +-
 Nt32Pkg/Sec/SecMain.inf |   2 +
 6 files changed, 172 insertions(+), 5 deletions(-)

--
2.9.3.windows.2

___
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 7/8] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support

[Wang, Jian J]

Sorry, 1.3 should be for your 1.4. I just noticed I missed your 1.3 comment.
Here's my opinion for it:

Current changes (use global variables to reserve resources) to  
InitializeCpuExceptionHandlers() is for DXE only. For PEI, if we really need to 
worry 
about the stack overflow and stack switch, we can have a different way to do 
it. For
example, we don't need to call InitializeCpuExceptionStackSwitchHandlers() 
inside
InitializeCpuExceptionHandlers(). We could call it whenever we can reserve 
memory blocks
and pass them to InitializeCpuExceptionStackSwitchHandlers() via parameter. I 
think this
is one of reason we have a separate method to initialize the exception handlers 
for the
sake of stack switch.

Calling InitializeCpuExceptionStackSwitchHandlers() inside 
InitializeCpuExceptionHandlers()
is just for the consideration of backward compatibility. Because this new API 
is just
implemented for IA32 processor at this time, calling it in DXE core will break 
the build of
other type of processors. This is another reason we have a separate method to do
exception handler initialization in a different way.

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wang,
> Jian J
> Sent: Thursday, November 23, 2017 2:44 PM
> To: Yao, Jiewen ; edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Dong, Eric
> ; Zeng, Star 
> Subject: Re: [edk2] [PATCH v2 7/8] UefiCpuPkg/CpuExceptionHandlerLib: Add
> stack switch support
> 
> 1.1) Got your point. I'll add dummy function in this patch.
> 1.2) Yep, we're on the same page.
> 1.3) Here's my opinion:
> 
> Actually almost all MP code has such assumption: any AP configuration will 
> copy
>  from BSP. If we allow AP to call InitializeCpuExceptionHandlers(), we have 
> to do
> a lot
> of other changes than just updating InitializeCpuExceptionHandlers(). If so, 
> it
> may
> be premature to figure out a solution at this patch.
> 
> In addition, CpuDxe actually calls InitializeCpuInterruptHandlers() which 
> covers
> the
> functionalities of InitializeCpuExceptionHandlers() (its settings will be
> overwritten).
> If we want AP to initialize interrupt and exception individually, maybe we 
> should
> let AP call InitializeCpuInterruptHandlers() instead.
> 
> > -Original Message-
> > From: Yao, Jiewen
> > Sent: Thursday, November 23, 2017 2:16 PM
> > To: Wang, Jian J ; edk2-devel@lists.01.org
> > Cc: Zeng, Star ; Dong, Eric ;
> > Kinney, Michael D 
> > Subject: RE: [PATCH v2 7/8] UefiCpuPkg/CpuExceptionHandlerLib: Add stack
> > switch support
> >
> > Here is my thought for 1)
> >
> > 1.1) We must provide the InitializeCpuExceptionStackSwitchHandlers()
> > implementation in Pei instance and Smm instance.
> >
> > The basic requirement is a library instance must provide symbol for 
> > functions
> > declared in header file.
> > It is ok to return unsupported. But we MUST provide the symbol.
> >
> > 1.2) For SMM, I think our ultimate goal is to remove SMM specific stack 
> > guard,
> > and use the common one. Duplicating code is completely unnecessary, and it 
> > is
> > easy to introduce bug. And unfortunately, we already have bug in existing
> SMM
> > exception handler. -- That is a good reason to remove duplication.
> >
> > Again, it is not necessary to do it in this patch. I am totally OK to do it 
> > in
> another
> > patch.
> >
> > 1.3) For PEI, I do not think we can use current way to allocate stack in 
> > data
> > section, because it might be readonly in pre-mem phase. We must use some
> > other way.
> >
> > 1.4) I believe this patch has a hidden assumption is that:
> > InitializeCpuExceptionHandlers() won't be called by multiple APs.
> > If 2 or more APs call the it at same time, it might be broken because you 
> > use
> > mNewStack for all the callers
> > Is that right?
> >
> >
> > Thank you
> > Yao Jiewen
> >
> >
> > > -Original Message-
> > > From: Wang, Jian J
> > > Sent: Thursday, November 23, 2017 2:06 PM
> > > To: Yao, Jiewen ; edk2-devel@lists.01.org
> > > Cc: Zeng, Star ; Dong, Eric ;
> > Kinney,
> > > Michael D 
> > > Subject: RE: [PATCH v2 7/8] UefiCpuPkg/CpuExceptionHandlerLib: Add stack
> > > switch support
> > >
> > >
> > >
> > > > -Original Message-
> > > > From: Yao, Jiewen
> > > > Sent: Thursday, November 23, 2017 1:50 PM
> > > > To: Wang, Jian J ; edk2-devel@lists.01.org
> > > > Cc: Zeng, Star ; Dong, Eric ;
> > > > Kinney, Michael D 
> > > > Subject: RE: [PATCH v2 7/8] UefiCpuPkg/CpuExceptionHandlerLib: Add
> stack
> > > > switch support
> > > >
> > > > Some thought:
> > > >
> > > > 1) I found 

Re: [edk2] [PATCH] MdeModulePkg/Core: Merge memory map after filtering paging capability

[Wang, Jian J]

Make sense. Thanks for the comment.

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Tuesday, November 28, 2017 2:21 AM
> To: Wang, Jian J ; edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Zeng, Star 
> Subject: Re: [edk2] [PATCH] MdeModulePkg/Core: Merge memory map after
> filtering paging capability
> 
> Hello Jian,
> 
> On 11/27/17 06:34, Jian J Wang wrote:
> > Once the paging capabilities were filtered out, there might be some adjacent
> entries
> > sharing the same capabilities. It's recommended to merge those entries for 
> > the
> OS
> > compatibility purpose.
> >
> > This patch makes use of existing method MergeMemoryMap() to do it. This is
> done by
> > simply turning this method from static to extern, and call it after filter 
> > code.
> >
> > This patch is related to an issue described at
> >https://bugzilla.tianocore.org/show_bug.cgi?id=753
> >
> > This patch is also passed test of booting follow OSs:
> > Windows 10
> > Windows Server 2016
> > Fedora 26
> > Fedora 25
> >
> > Cc: Jiewen Yao 
> > Cc: Star Zeng 
> > Cc: Laszlo Ersek 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jian J Wang 
> > ---
> >  MdeModulePkg/Core/Dxe/DxeMain.h  | 18 ++
> >  MdeModulePkg/Core/Dxe/Mem/Page.c |  1 +
> >  MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c |  1 -
> >  3 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h
> b/MdeModulePkg/Core/Dxe/DxeMain.h
> > index 1a0babba71..07b86ba696 100644
> > --- a/MdeModulePkg/Core/Dxe/DxeMain.h
> > +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
> > @@ -2948,4 +2948,22 @@ ApplyMemoryProtectionPolicy (
> >IN  UINT64Length
> >);
> >
> > +/**
> > +  Merge continous memory map entries whose have same attributes.
> > +
> > +  @param  MemoryMap   A pointer to the buffer in which firmware places
> > +  the current memory map.
> > +  @param  MemoryMapSize   A pointer to the size, in bytes, of the
> > +  MemoryMap buffer. On input, this is the size of
> > +  the current memory map.  On output,
> > +  it is the size of new memory map after merge.
> > +  @param  DescriptorSize  Size, in bytes, of an individual
> EFI_MEMORY_DESCRIPTOR.
> > +**/
> > +VOID
> > +MergeMemoryMap (
> > +  IN OUT EFI_MEMORY_DESCRIPTOR  *MemoryMap,
> > +  IN OUT UINTN  *MemoryMapSize,
> > +  IN UINTN  DescriptorSize
> > +  );
> > +
> >  #endif
> > diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c
> b/MdeModulePkg/Core/Dxe/Mem/Page.c
> > index 962ae90d3d..ca4ce69a3f 100644
> > --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> > +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> > @@ -1915,6 +1915,7 @@ CoreGetMemoryMap (
> >EFI_MEMORY_XP);
> >  MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, Size);
> >}
> > +  MergeMemoryMap (MemoryMapStart, , Size);
> >
> >Status = EFI_SUCCESS;
> >
> > diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> > index 6cf5edcbe5..75d9b14c1f 100644
> > --- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> > +++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> > @@ -182,7 +182,6 @@ SortMemoryMap (
> >   it is the size of new memory map after 
> > merge.
> >@param  DescriptorSize Size, in bytes, of an individual
> EFI_MEMORY_DESCRIPTOR.
> >  **/
> > -STATIC
> >  VOID
> >  MergeMemoryMap (
> >IN OUT EFI_MEMORY_DESCRIPTOR  *MemoryMap,
> >
> 
> This patch looks good to me -- I expect merging the memmap can only
> collapse more entries into fewer entries, so the repro / test
> instructions that I added to the BZ earlier, and the expected OS
> behavior should remain unchanged.
> 
> I have one small suggestion: like before, I suggest keeping the local
> variables up-to-date after adding the new code, so that further code
> need not hunt down invariants as a starting step. Therefore, after the
> MergeMemoryMap() call, how about updating MemoryMapEnd, like this:
> 
>   MemoryMapEnd = (EFI_MEMORY_DESCRIPTOR *)(
>(UINT8 *)MemoryMapStart + BufferSize
>);
> 
> I think (if Jiewen and Star agree with this suggestion) that you don't
> need to post a v2 just for this.
> 
> I'm also fine with the patch if the MemoryMapEnd update is rejected.
> 
> Acked-by: Laszlo Ersek 
> 
> Can you please add a note to TianoCore BZ#753 when you push this?
> 
> Thanks!
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 1/2] OvmfPkg/QemuBootOrderLib: skip already matched / appended UEFI boot opts

[Laszlo Ersek]

The SetBootOrderFromQemu() function implements a nested loop where

- the outer loop iterates over all OpenFirmware (OFW) device paths in the
  QEMU boot order, and translates each to a UEFI device path prefix;

- the inner loop matches the current (translated) prefix against all
  active UEFI boot options in turn;

- if the UEFI boot option is matched by the translated prefix, the UEFI
  boot option is appended to the "new" UEFI boot order, and marked as
  "has been appended".

This patch adds a micro-optimization where already matched / appended UEFI
boot options are skipped in the inner loop. This is not a functional
change. A functional change would be if, as a consequence of the patch,
some UEFI boot options would no longer be *doubly* matched.

For a UEFI boot option to be matched by two translated prefixes, one of
those prefixes would have to be a (proper, or equal) prefix of the other
prefix. The PCI and MMIO OFW translation routines output such only in the
following cases:

- When the original OFW device paths are prefixes of each other. This is
  not possible from the QEMU side. (Only leaf devices are bootable.)

- When the translation rules in the routines are incomplete, and don't
  look at the OFW device paths for sufficient length (i.e., at nodes where
  they would already differ, and the difference would show up in the
  translation output).

  This would be a shortcoming of the translation routines and should be
  fixed in TranslatePciOfwNodes() and TranslateMmioOfwNodes(), whenever
  identified.

Even in the second case, this patch would replace the double appending of
a single UEFI boot option (matched by two different OFW device paths) with
a correct, or cross-, matching of two different UEFI boot options. Again,
this is not expected, but arguably it would be more correct than duplicate
boot option appending, should it occur due to any (unexpected, unknown)
lack of detail in the translation routines.

Cc: Ard Biesheuvel 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c 
b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
index 7c1f375beb20..a9a62e9d4007 100644
--- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
+++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
@@ -1849,31 +1849,32 @@ SetBootOrderFromQemu (
   //
   TranslatedSize = ARRAY_SIZE (Translated);
   Status = TranslateOfwPath (, ExtraPciRoots, Translated,
  );
   while (Status == RETURN_SUCCESS ||
  Status == RETURN_UNSUPPORTED ||
  Status == RETURN_PROTOCOL_ERROR ||
  Status == RETURN_BUFFER_TOO_SMALL) {
 if (Status == RETURN_SUCCESS) {
   UINTN Idx;
 
   //
   // match translated OpenFirmware path against all active boot options
   //
   for (Idx = 0; Idx < ActiveCount; ++Idx) {
-if (Match (
+if (!ActiveOption[Idx].Appended &&
+Match (
   Translated,
   TranslatedSize, // contains length, not size, in CHAR16's here
   ActiveOption[Idx].BootOption->FilePath
   )
 ) {
   //
   // match found, store ID and continue with next OpenFirmware path
   //
   Status = BootOrderAppend (, [Idx]);
   if (Status != RETURN_SUCCESS) {
 goto ErrorFreeExtraPciRoots;
   }
   break;
 }
   } // scanned all active boot options
-- 
2.14.1.3.gb7cf6e02401b


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

[edk2] [PATCH 0/2] OvmfPkg/QemuBootOrderLib: multiplicity update for OFW <-> UEFI matching

[Laszlo Ersek]

Two small patches with verbose commit messages.

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

Cc: Ard Biesheuvel 
Cc: Jordan Justen 

Laszlo Ersek (2):
  OvmfPkg/QemuBootOrderLib: skip already matched / appended UEFI boot
opts
  OvmfPkg/QemuBootOrderLib: let an OFW devpath match multiple UEFI boot
opts

 OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.14.1.3.gb7cf6e02401b

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

[edk2] [PATCH 2/2] OvmfPkg/QemuBootOrderLib: let an OFW devpath match multiple UEFI boot opts

[Laszlo Ersek]

This means that SetBootOrderFromQemu() will preserve all UEFI boot options
matched by any given OFW devpath, such as PXEv4, HTTPv4, PXEv6 and HTTPv6
boot options for the same NIC. Currently we stop the matching / appending
for the OFW devpath coming from the outer loop whenever we find the first
UEFI boot option match in the inner loop.

(The previous patch was about multiple OFW devpaths matching a single UEFI
boot option (which should never happen). This patch is about a single OFW
devpath matching multiple UEFI boot options. With the "break" statement
removed here, the small optimization from the last patch becomes a bit
more relevant, because now the inner loop always counts up to
ActiveCount.)

Cc: Ard Biesheuvel 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c 
b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
index a9a62e9d4007..366104adf535 100644
--- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
+++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
@@ -1863,31 +1863,30 @@ SetBootOrderFromQemu (
   for (Idx = 0; Idx < ActiveCount; ++Idx) {
 if (!ActiveOption[Idx].Appended &&
 Match (
   Translated,
   TranslatedSize, // contains length, not size, in CHAR16's here
   ActiveOption[Idx].BootOption->FilePath
   )
 ) {
   //
   // match found, store ID and continue with next OpenFirmware path
   //
   Status = BootOrderAppend (, [Idx]);
   if (Status != RETURN_SUCCESS) {
 goto ErrorFreeExtraPciRoots;
   }
-  break;
 }
   } // scanned all active boot options
 }   // translation successful
 
 TranslatedSize = ARRAY_SIZE (Translated);
 Status = TranslateOfwPath (, ExtraPciRoots, Translated,
);
   } // scanning of OpenFirmware paths done
 
   if (Status == RETURN_NOT_FOUND && BootOrder.Produced > 0) {
 //
 // No more OpenFirmware paths, some matches found: rewrite BootOrder NvVar.
 // Some of the active boot options that have not been selected over fw_cfg
 // should be preserved at the end of the boot order.
 //
-- 
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] ArmVirtPkg/PrePi: don't export PE/COFF and LZMA libraries via HOBs

[Ard Biesheuvel]

On 27 November 2017 at 12:32, Laszlo Ersek  wrote:
> On 11/24/17 10:51, Ard Biesheuvel wrote:
>> The PrePi code we inherited from ArmPlatformPkg contains a rather
>> obscure optimization, where entry points of the PE/COFF and LZMA
>> handling routines are recorded in special HOBs, allowing DXE core
>> to call into that code directly rather than carry its own copy of
>> these libraries.
>>
>> Given that no ArmVirtPkg platforms actually include the library
>> resolutions* that take advantage of these optimizations, let's not
>> bother with them, and remove the associated code.
>>
>> * 
>> EmbeddedPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf
>>   EmbeddedPkg/Library/DxeHobPeCoffLib/DxeHobPeCoffLib.inf
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf |   1 -
>>  ArmVirtPkg/PrePi/LzmaDecompress.h   | 103 
>> 
>>  ArmVirtPkg/PrePi/PrePi.c|  11 ---
>>  3 files changed, 115 deletions(-)
>>
>> diff --git a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf 
>> b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
>> index 58290d2d1b76..b3a3f5da065e 100755
>> --- a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
>> +++ b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
>> @@ -51,7 +51,6 @@ [LibraryClasses]
>>SerialPortLib
>>ExtractGuidedSectionLib
>>LzmaDecompressLib
>> -  PeCoffGetEntryPointLib
>>PrePiLib
>>MemoryAllocationLib
>>HobLib
>> diff --git a/ArmVirtPkg/PrePi/LzmaDecompress.h 
>> b/ArmVirtPkg/PrePi/LzmaDecompress.h
>> deleted file mode 100644
>> index a79ff343d231..
>> --- a/ArmVirtPkg/PrePi/LzmaDecompress.h
>> +++ /dev/null
>> @@ -1,103 +0,0 @@
>> -/** @file
>> -  LZMA Decompress Library header file
>> -
>> -  Copyright (c) 2006 - 2010, 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.
>> -
>> -**/
>> -
>> -#ifndef __LZMA_DECOMPRESS_H___
>> -#define __LZMA_DECOMPRESS_H___
>> -
>> -/**
>> -  Examines a GUIDed section and returns the size of the decoded buffer and 
>> the
>> -  size of an scratch buffer required to actually decode the data in a 
>> GUIDed section.
>> -
>> -  Examines a GUIDed section specified by InputSection.
>> -  If GUID for InputSection does not match the GUID that this handler 
>> supports,
>> -  then RETURN_UNSUPPORTED is returned.
>> -  If the required information can not be retrieved from InputSection,
>> -  then RETURN_INVALID_PARAMETER is returned.
>> -  If the GUID of InputSection does match the GUID that this handler 
>> supports,
>> -  then the size required to hold the decoded buffer is returned in 
>> OututBufferSize,
>> -  the size of an optional scratch buffer is returned in ScratchSize, and 
>> the Attributes field
>> -  from EFI_GUID_DEFINED_SECTION header of InputSection is returned in 
>> SectionAttribute.
>> -
>> -  If InputSection is NULL, then ASSERT().
>> -  If OutputBufferSize is NULL, then ASSERT().
>> -  If ScratchBufferSize is NULL, then ASSERT().
>> -  If SectionAttribute is NULL, then ASSERT().
>> -
>> -
>> -  @param[in]  InputSection   A pointer to a GUIDed section of an FFS 
>> formatted file.
>> -  @param[out] OutputBufferSize   A pointer to the size, in bytes, of an 
>> output buffer required
>> - if the buffer specified by InputSection 
>> were decoded.
>> -  @param[out] ScratchBufferSize  A pointer to the size, in bytes, required 
>> as scratch space
>> - if the buffer specified by InputSection 
>> were decoded.
>> -  @param[out] SectionAttribute   A pointer to the attributes of the GUIDed 
>> section. See the Attributes
>> - field of EFI_GUID_DEFINED_SECTION in the 
>> PI Specification.
>> -
>> -  @retval  RETURN_SUCCESSThe information about InputSection was 
>> returned.
>> -  @retval  RETURN_UNSUPPORTEDThe section specified by InputSection 
>> does not match the GUID this handler supports.
>> -  @retval  RETURN_INVALID_PARAMETER  The information can not be retrieved 
>> from the section specified by InputSection.
>> -
>> -**/
>> -RETURN_STATUS
>> -EFIAPI
>> -LzmaGuidedSectionGetInfo (
>> -  IN  CONST VOID  *InputSection,
>> -  OUT UINT32  *OutputBufferSize,
>> -  OUT UINT32  *ScratchBufferSize,
>> -  OUT UINT16  *SectionAttribute
>> -  );
>> -
>> -/**
>> -  Decompress a LZAM compressed GUIDed 

Re: [edk2] [PATCH] MdeModulePkg/Core: Merge memory map after filtering paging capability

[Laszlo Ersek]

Hello Jian,

On 11/27/17 06:34, Jian J Wang wrote:
> Once the paging capabilities were filtered out, there might be some adjacent 
> entries
> sharing the same capabilities. It's recommended to merge those entries for 
> the OS
> compatibility purpose.
> 
> This patch makes use of existing method MergeMemoryMap() to do it. This is 
> done by
> simply turning this method from static to extern, and call it after filter 
> code.
> 
> This patch is related to an issue described at
>https://bugzilla.tianocore.org/show_bug.cgi?id=753
> 
> This patch is also passed test of booting follow OSs:
> Windows 10
> Windows Server 2016
> Fedora 26
> Fedora 25
> 
> Cc: Jiewen Yao 
> Cc: Star Zeng 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang 
> ---
>  MdeModulePkg/Core/Dxe/DxeMain.h  | 18 ++
>  MdeModulePkg/Core/Dxe/Mem/Page.c |  1 +
>  MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c |  1 -
>  3 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
> index 1a0babba71..07b86ba696 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.h
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
> @@ -2948,4 +2948,22 @@ ApplyMemoryProtectionPolicy (
>IN  UINT64Length
>);
>  
> +/**
> +  Merge continous memory map entries whose have same attributes.
> +
> +  @param  MemoryMap   A pointer to the buffer in which firmware places
> +  the current memory map.
> +  @param  MemoryMapSize   A pointer to the size, in bytes, of the
> +  MemoryMap buffer. On input, this is the size of
> +  the current memory map.  On output,
> +  it is the size of new memory map after merge.
> +  @param  DescriptorSize  Size, in bytes, of an individual 
> EFI_MEMORY_DESCRIPTOR.
> +**/
> +VOID
> +MergeMemoryMap (
> +  IN OUT EFI_MEMORY_DESCRIPTOR  *MemoryMap,
> +  IN OUT UINTN  *MemoryMapSize,
> +  IN UINTN  DescriptorSize
> +  );
> +
>  #endif
> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c 
> b/MdeModulePkg/Core/Dxe/Mem/Page.c
> index 962ae90d3d..ca4ce69a3f 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> @@ -1915,6 +1915,7 @@ CoreGetMemoryMap (
>EFI_MEMORY_XP);
>  MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, Size);
>}
> +  MergeMemoryMap (MemoryMapStart, , Size);
>  
>Status = EFI_SUCCESS;
>  
> diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c 
> b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> index 6cf5edcbe5..75d9b14c1f 100644
> --- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> +++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> @@ -182,7 +182,6 @@ SortMemoryMap (
>   it is the size of new memory map after 
> merge.
>@param  DescriptorSize Size, in bytes, of an individual 
> EFI_MEMORY_DESCRIPTOR.
>  **/
> -STATIC
>  VOID
>  MergeMemoryMap (
>IN OUT EFI_MEMORY_DESCRIPTOR  *MemoryMap,
> 

This patch looks good to me -- I expect merging the memmap can only
collapse more entries into fewer entries, so the repro / test
instructions that I added to the BZ earlier, and the expected OS
behavior should remain unchanged.

I have one small suggestion: like before, I suggest keeping the local
variables up-to-date after adding the new code, so that further code
need not hunt down invariants as a starting step. Therefore, after the
MergeMemoryMap() call, how about updating MemoryMapEnd, like this:

  MemoryMapEnd = (EFI_MEMORY_DESCRIPTOR *)(
   (UINT8 *)MemoryMapStart + BufferSize
   );

I think (if Jiewen and Star agree with this suggestion) that you don't
need to post a v2 just for this.

I'm also fine with the patch if the MemoryMapEnd update is rejected.

Acked-by: Laszlo Ersek 

Can you please add a note to TianoCore BZ#753 when you push this?

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

Re: [edk2] [PATCH] [edk2-platforms]:Enabling Secure boot feature support on hikey platfrom

[Leif Lindholm]

On Mon, Nov 27, 2017 at 02:02:32PM +0100, Laszlo Ersek wrote:
> On 11/26/17 16:22, Leif Lindholm wrote:
> > (Adding Laszlo to cc based on a single comment I make below.)
> > 
> > On Tue, Nov 21, 2017 at 04:23:36PM +0530, kalyan-nagabhirava wrote:
> 
> >>  [Guids.common]
> >>gHiKeyTokenSpaceGuid  =  { 0x91148425, 0xcdd2, 0x4830, { 0x8b, 
> >> 0xd0, 0xc6, 0x1c, 0x6d, 0xea, 0x36, 0x21 } }
> >> +  gHwTokenSpaceGuid =  { 0x, 0x74c5, 0x4043, { 0xb4, 
> >> 0x17, 0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }
> > 
> > This very much looks like a not properly generated GUID.
> > GUIDs must always be generated using an RFC4122-compliant algorithm.
> > I generally recommend using
> > https://www.guidgenerator.com/online-guid-generator.aspx.
> 
> I just run "uuidgen" in a terminal window.

Yeah, I just prefer pointing to someone that does not require
installing anything, or requires specific operating systems.

> >> +EFI_STATUS
> >> +EFIAPI
> >> +FvbSetAttributes(
> >> +  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL  *This,
> >> +  IN OUTEFI_FVB_ATTRIBUTES_2 *Attributes
> >> +  )
> >> +{
> >> +  DEBUG ((DEBUG_BLKIO, "FvbSetAttributes(0x%X) is not 
> >> supported\n",*Attributes));
> >> +  return EFI_UNSUPPORTED;
> > 
> > As per my (very) recent comment to Marcin, I do not believe returning
> > EFI_UNSUPPORTED is a valid thing to do here. Which to me suggests the
> > implementation of FvbGetAttributes is also incorrect.
> > 
> > Laszlo - what's your take on this in conjunction with PI 1.6 section
> > 3.4.2? OvmfPkg does something very similar in
> > EmuVariableFvbRuntimeDxe/Fvb.c.
> 
> I guess you are right. The particular OvmfPkg code that you mention is
> likely also spec-breaking.
> 
> FWIW, in the OVMF flash driver that actually uses pflash, namely
> 
>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
> 
> the FvbSetVolumeAttributes() function appears both appropriate for the
> spec and generic enough to copy elsewhere.

Yes, that looks good, thanks!

Marcin, Kalyan - please have a look at that implementation for
inspiration.

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

Re: [edk2] [PATCH 1/5] MdeModulePkg/BdsDxe: fall back to a Boot Manager Menu loop before hanging

[Laszlo Ersek]

Hi Ray,

On 11/23/17 14:09, Laszlo Ersek wrote:
> On 11/23/17 04:43, Ni, Ruiyu wrote:
>> Thanks for the patch. It follows the idea described in:
>> https://lists.01.org/pipermail/edk2-devel/2017-April/010294.html.
>>
>> Reviewed-by: Ruiyu Ni 
> 
> Thank you, Ray! I might commit this first patch in isolation (to close
> #513) while we continue discussing the rest. Not sure yet. (Depends on
> my workload.)

thanks again for your original suggestion and your review; I've now
committed this single patch, isolated the rest of the series. Commit
d1de487dd2e7.

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

Re: [edk2] EDK2 build issues

[Andrew Fish]

Liming,

We have a top level GNUmakefile vs. having script files to build targets. Thus 
building BaseTools, or using build are subtasks of this higher level makefile. 

Looks like you have added some fixes to the BaseTools!

Thanks,

Andrew Fish

> On Nov 22, 2017, at 9:28 PM, Gao, Liming  wrote:
> 
> Andrew:
>  To compile BaseTools C tools is a separate step. It happens before build. I 
> review BaseTools C tool top GNUmakefile. It can be enhanced to support make 
> -j N that has the better performance. I will provide my patch for code 
> review. 
> 
> Thanks
> Liming
>> -Original Message-
>> From: af...@apple.com [mailto:af...@apple.com]
>> Sent: Thursday, November 23, 2017 8:02 AM
>> To: Desimone, Nathaniel L 
>> Cc: Gao, Liming ; Aaron Durbin
>> ; edk2-devel@lists.01.org
>> Subject: Re: [edk2] EDK2 build issues
>> 
>> 
>>> On Nov 16, 2017, at 1:37 PM, Desimone, Nathaniel L
>>  wrote:
>>> 
>>> Hi Liming,
>>> 
>>> I chatted with Aaron on the phone today. The VfrCompiler race condition
>> was discovered using "make -j " (where n > 1). I have filed the bug in
>> Bugzilla:
>>> 
>> 
>> I think there are a few issues like that in BaseTools. We have a parallel
>> makefile and we had to turn off parallelism when calling the BaseTools.
>> 
>> In general the edk2 build system fights against "make -j " since the 
>> Python
>> build command is trying to do the threading and invoking make in parallel.
>> Given we ported EDK to parallel build, the Python parallel edk2 build is a 
>> lot
>> slower than the EDK make parallel build, even when adjusting for the extra
>> work the edk2 does.
>> 
>> I even noticed a 1,000,000 calls to regex in Python during the ... phase of 
>> the
>> build.
>> 
>> I think the correct short term fix may be to put .NOTPARALLEL: in the
>> BaseTools GNUmakefile given it is constructed in a way that it does not
>> support parallelism.
>> 
>> Thanks,
>> 
>> Andrew Fish
>> 
>> 
>> 
>>> https://bugzilla.tianocore.org/show_bug.cgi?id=786
>>> 
>>> For the ARCH environment variable, we brainstormed two possible new
>> names HOST_ARCH or BASETOOLS_ARCH. Should I file the ARCH variable
>> request in Bugzilla as well?
>>> 
>>> Thanks,
>>> 
>>> Nate
>>> 
>>> -Original Message-
>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Gao, Liming
>>> Sent: Monday, November 13, 2017 5:46 PM
>>> To: Aaron Durbin 
>>> Cc: edk2-devel@lists.01.org
>>> Subject: Re: [edk2] EDK2 build issues
>>> 
>>> I add my comments.
>>> 
 -Original Message-
 From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
 Aaron Durbin
 Sent: Tuesday, November 14, 2017 1:38 AM
 To: Gao, Liming 
 Cc: edk2-devel@lists.01.org
 Subject: Re: [edk2] EDK2 build issues
 
 On Sun, Nov 12, 2017 at 7:15 PM, Gao, Liming 
>> wrote:
> Yes. BaseTools needs some environments. Before build BaseTools, we
>> need to call edkset.sh to setup them.
> 
> 1. BaseTools Soure tools are compiled in serial instead of parallel.
> And, VfrCompiler depends on Anltr tool. So, Anltr is required to be
 built first. I agree that this way takes some time to compile
 BaseTools source. I have one proposal to compile C source tools with
>> multiple thread. I can share my draft patch.
 
 If the depedencies are correctly met in the makefiles then why are
 they built serially? It sounds like you understand the dependencies so
 why aren't those explicitly noted so one can build in parallel?
 
>>> Seemly, VfrCompiler Makefile doesn't list those dependency clearly. Could
>> you submit this issue in bugzillar (https://bugzilla.tianocore.org/)?
>>> Besides, do you use make -j option to enable Parallel Execution? I want to
>> know how to verify it.
>>> 
> 2. BaseTools C source are compiled to the executable files. They run
> in host machine. Here ARCH is for the executable files those can
 run in host machine. edk2\BaseTools\Source\C\GNUmakefile auto detects
>> ARCH based on 'uname -m' command.
 
 ARCH != host is my point. Why are those being conflated? Or are you
 saying edk2 tools define ARCH to be what the host is?
 
>>> Yes. Edk2 BaseTools C source uses it as host arch. I agree ARCH name is too
>> generic. In your environment, ARCH may be defined for the different purpose.
>>> 
> 3. There are more GCC compiler distribution. We try to use the
> generic options in GCC tool chain. If you find the option doesn't
> work
 on your GCC compiler, please report it. And, we also notice
 tools_def.txt is too big to be hard to be maintain. We have one proposal to
>> simplify it. I will send this RFC to edk2 community.
> 
>> -Original Message-
>> From: edk2-devel 

Re: [edk2] Unable to Loacte EdkiiIoMmuProtocol.

[Andrew Fish]

Should drivers call gEdkiiIoMmuProtocolGuid directly or just use PciIo 
Protocol? 

Thanks,

Andrew Fish


> On Nov 27, 2017, at 3:24 AM, Amit kumar  wrote:
> 
> Hi.
> I am trying to allocate a mem buffer above 4Gb address but the allocation
> since i am unable to locate EdkiiIoMmuProtocol.
> 
> Status = gBS->LocateProtocol (, NULL, (VOID 
> **));
>  Status = mIoMmuProtocol->AllocateBuffer (
>   mIoMmuProtocol,
>   MaxAllocateType,
>   EfiBootServicesData,
>   1,
>  HostAddress,
>  EDKII_IOMMU_ATTRIBUTE_DUAL_ADDRESS_CYCLE
>  );
> 
> 
> can somebody tell me the right way to do it. ?
> 
> 
> Best Regards
> Amit Kumar
> 
> ___
> 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] Accessing memory above 0xFFFFFFFF limit.

[Laszlo Ersek]

On 11/27/17 13:20, Amit kumar wrote:
> 
> Hi.
> I am trying to allocate a mem buffer above 4Gb address but the allocation
> since i am unable to locate EdkiiIoMmuProtocol.
> 
> Status = gBS->LocateProtocol (, NULL, (VOID 
> **));
>   Status = mIoMmuProtocol->AllocateBuffer (
>mIoMmuProtocol,
>MaxAllocateType,
>EfiBootServicesData,
>1,
>   HostAddress,
>   EDKII_IOMMU_ATTRIBUTE_DUAL_ADDRESS_CYCLE
>   );

PciHostBridgeDxe can perfectly well allocate >4GB memory for DMA
purposes even in the absence of the IOMMU protocol. For that, your PCI
device driver has to advertize the PCI device as 64-bit capable. In your
Driver Binding Start() function, call PciIo->Attributes() with the
EfiPciIoAttributeOperationEnable operation, and set
EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE. From the v2.7 UEFI spec (14.4
EFI PCI I/O Protocol):

If this bit is set, then the PCI controller is capable of producing
PCI Dual Address Cycles, so it is able to access a 64-bit address
space. If this bit is not set, then the PCI controller is not
capable of producing PCI Dual Address Cycles, so it is only able to
access a 32-bit address space.

PciHostBridgeDxe will ignore the bit if the root bridge that the device
belongs to is generally uncapable of 64-bit access. Please see
"DmaAbove4G" in the PciHostBridgeLib interface:

  MdeModulePkg/Include/Library/PciHostBridgeLib.h

(You might notice that the PciHostBridgeDxe source code uses
EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE, but above I wrote
EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE. The PciBusDxe driver maps the
latter to the former, in the PciIoAllocateBuffer() function.)

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

Re: [edk2] [PATCH] OvmfPkg/Sec: Fix 64bit SEC build failure

[Laszlo Ersek]

Hi Ray,

adding Ard, Jordan, Liming and Yonghong:

On 11/27/17 02:38, Ruiyu Ni wrote:
> Original code breaks a single assembly code to multiple lines.
> But build tool doesn't support such usage in Windows OS environment.
> 
> Changing the multiple lines to one line to resolve the build failure.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni 
> Cc: Laszlo Ersek 
> ---
>  OvmfPkg/Sec/X64/SecEntry.nasm | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/OvmfPkg/Sec/X64/SecEntry.nasm b/OvmfPkg/Sec/X64/SecEntry.nasm
> index 7c55032ac9..d76adcffd8 100644
> --- a/OvmfPkg/Sec/X64/SecEntry.nasm
> +++ b/OvmfPkg/Sec/X64/SecEntry.nasm
> @@ -45,10 +45,8 @@ ASM_PFX(_ModuleEntryPoint):
>  ; Fill the temporary RAM with the initial stack value.
>  ; The loop below will seed the heap as well, but that's harmless.
>  ;
> -mov rax, (FixedPcdGet32 (\
> -PcdInitValueInTempStack  \
> -) << 32) |   \
> - FixedPcdGet32 (PcdInitValueInTempStack)  ; qword to 
> store
> +mov rax, (FixedPcdGet32 (PcdInitValueInTempStack) << 32) | 
> FixedPcdGet32 (PcdInitValueInTempStack)
> +  ; qword to 
> store
>  mov rdi, FixedPcdGet32 (PcdOvmfSecPeiTempRamBase) ; base address,
>;   relative to
>;   ES
> 

I'd like to understand more about the build failure. As far as I understand, 
the NASM tool itself has no problem with the backslash line continuation; 
before I posted the patch, I checked the NASM manual for this feature.

http://www.nasm.us/doc/nasmdoc3.html#section-3.1

NASM uses backslash (\) as the line continuation character; if a
line ends with backslash, the next line is considered to be a part
of the backslash-ended line.

Now, in "BaseTools/Conf/build_rule.template", I see that we have the following 
rule for NASM source files:

[Nasm-Assembly-Code-File.COMMON.COMMON]

?.nasm


$(MAKE_FILE)


$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj


"$(PP)" $(PP_FLAGS) $(INC) ${src} > ${d_path}(+)${s_base}.i
Trim --trim-long --source-code -o ${d_path}(+)${s_base}.iii 
${d_path}(+)${s_base}.i
"$(NASM)" -I${s_path}(+) $(NASM_FLAGS) -o $dst ${d_path}(+)${s_base}.iii

I guess it's actually the pre-processor (PP) that splices the broken-up lines 
together, as any C language pre-processor is supposed to do.

Why does that not work with the VS toolchains ("cl.exe")?

Is it perhaps a problem with the _PP_FLAGS?

https://msdn.microsoft.com/en-us/library/3xkfswhy.aspx
https://msdn.microsoft.com/en-us/library/032xwy55.aspx

"/E" seems right (pre-process). Is /TC the problem? ("C language source code").

If the problem is impossible to fix in BaseTools, I don't mind the patch, but 
I'd like to see more explanation in the commit message.

(Other assembly files use backslash line continuations too, they just haven't 
been exposed to VS toolchains yet, apparently.)

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

Re: [edk2] [PATCH v2 3/3] ShellPkg/dp: Convert from NULL class library to Dynamic Command

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Ni, Ruiyu
> Sent: Sunday, November 26, 2017 9:56 PM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben ; Kinney, Michael D
> 
> Subject: [PATCH v2 3/3] ShellPkg/dp: Convert from NULL class library to
> Dynamic Command
> Importance: High
> 
> UEFI Shell spec defines Shell Dynamic Command protocol which is just
> for the purpose to extend internal command.
> So dp command is changed from NULL class library to be a driver
> producing DynamicCommand protocol.
> 
> The guideline is:
> 1. Only use NULL class library for Shell spec defined commands.
> 2. New commands can be provided as not only a standalone application
>but also a dynamic command. So it can be used either as an
>internal command, but also as a standalone application.
> 
> DpApp.inf is to provide a standalone application.
> DpDynamicCommand.inf is to provide a standalone driver producing
> Dynamic Command protocol.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni 
> Cc: Jaben Carsey 
> Cc: Michael D Kinney 
> ---
>  .../DpDynamicCommand}/Dp.c | 100 +++
>  .../DpDynamicCommand}/Dp.h |  63 ++-
>  .../DpDynamicCommand/Dp.uni}   |   0
>  ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.c   |  53 ++
>  ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf |  73
> +
>  .../DpDynamicCommand/DpDynamicCommand.c| 130
> +++
>  .../DpDynamicCommand/DpDynamicCommand.inf  |  78 +
>  .../DpDynamicCommand}/DpInternal.h |   2 +-
>  .../DpDynamicCommand}/DpProfile.c  |  34 ++--
>  .../DpDynamicCommand}/DpTrace.c| 182 
> ++---
>  .../DpDynamicCommand}/DpUtilities.c|  44 +++--
>  .../DpDynamicCommand}/Literals.c   |   0
>  .../DpDynamicCommand}/Literals.h   |   0
>  .../DpDynamicCommand}/PerformanceTokens.h  |   0
>  ShellPkg/Library/UefiDpLib/Readme.txt  |   2 -
>  ShellPkg/Library/UefiDpLib/UefiDpLib.c | 101 
>  ShellPkg/Library/UefiDpLib/UefiDpLib.h |  64 
>  ShellPkg/Library/UefiDpLib/UefiDpLib.inf   |  77 -
>  ShellPkg/ShellPkg.dsc  |  17 +-
>  19 files changed, 605 insertions(+), 415 deletions(-)
>  rename ShellPkg/{Library/UefiDpLib =>
> DynamicCommand/DpDynamicCommand}/Dp.c (84%)
>  rename ShellPkg/{Library/UefiDpLib =>
> DynamicCommand/DpDynamicCommand}/Dp.h (70%)
>  rename ShellPkg/{Library/UefiDpLib/UefiDpLib.uni =>
> DynamicCommand/DpDynamicCommand/Dp.uni} (100%)
>  create mode 100644
> ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.c
>  create mode 100644
> ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf
>  create mode 100644
> ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.c
>  create mode 100644
> ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf
>  rename ShellPkg/{Library/UefiDpLib =>
> DynamicCommand/DpDynamicCommand}/DpInternal.h (96%)
>  rename ShellPkg/{Library/UefiDpLib =>
> DynamicCommand/DpDynamicCommand}/DpProfile.c (87%)
>  rename ShellPkg/{Library/UefiDpLib =>
> DynamicCommand/DpDynamicCommand}/DpTrace.c (89%)
>  rename ShellPkg/{Library/UefiDpLib =>
> DynamicCommand/DpDynamicCommand}/DpUtilities.c (87%)
>  rename ShellPkg/{Library/UefiDpLib =>
> DynamicCommand/DpDynamicCommand}/Literals.c (100%)
>  rename ShellPkg/{Library/UefiDpLib =>
> DynamicCommand/DpDynamicCommand}/Literals.h (100%)
>  rename ShellPkg/{Library/UefiDpLib =>
> DynamicCommand/DpDynamicCommand}/PerformanceTokens.h (100%)
>  delete mode 100644 ShellPkg/Library/UefiDpLib/Readme.txt
>  delete mode 100644 ShellPkg/Library/UefiDpLib/UefiDpLib.c
>  delete mode 100644 ShellPkg/Library/UefiDpLib/UefiDpLib.h
>  delete mode 100644 ShellPkg/Library/UefiDpLib/UefiDpLib.inf
> 
> diff --git a/ShellPkg/Library/UefiDpLib/Dp.c
> b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> similarity index 84%
> rename from ShellPkg/Library/UefiDpLib/Dp.c
> rename to ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> index 94fa61c710..3ecc753d0c 100644
> --- a/ShellPkg/Library/UefiDpLib/Dp.c
> +++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> @@ -24,20 +24,13 @@
>WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
>  **/
> 
> -#include "UefiDpLib.h"
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -#include 
> -
>  #include "PerformanceTokens.h"
>  #include "Dp.h"
>  #include "Literals.h"
>  #include "DpInternal.h"
> 
> +EFI_HANDLE   mDpHiiHandle;
> +
>  //
>  /// Module-Global Variables
>  ///@{
> @@ -89,18 +82,18 @@ DumpStatistics( void )
>  {
>EFI_STRINGStringPtr;
>EFI_STRING

Re: [edk2] [PATCH 0/2] MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes

[Laszlo Ersek]

Hello Ray,

On 11/27/17 02:19, Ruiyu Ni wrote:
> The patches caused Windows 10 S4 resume failure.
> Considering the similar changes are reverted from PciBus driver,
> revert the patches from AtaAtapiPassThru as well.
> 
> Ruiyu Ni (2):
>   MdeModulePkg/AtaAtapiPassThru: Revert patch to disable Bus Master
>   MdeModulePkg/AtaAtapiPassThru: Revert patch to disable PCI attributes
> 
>  .../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c| 58 
> +-
>  .../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h|  5 --
>  2 files changed, 1 insertion(+), 62 deletions(-)
> 

it looks like these patches have not been committed yet, which is a good
thing, because apparently there's a better solution than a full revert.
Namely, in the other sub-thread at
<0236afa2-e365-af7a-9374-7fd1ad742c36@redhat.com">http://mid.mail-archive.com/0236afa2-e365-af7a-9374-7fd1ad742c36@redhat.com>
(alternative link:
),
Jiewen and Star seem to accept AhciReset() as a better way to abort
pending DMA.

This means that we need not revert the EBS-handler altogether, only
change what it does. Could you give that a try please?

(If the Windows regression is very urgent to fix, then I don't mind if
the Bus Master disabling is removed separately, before AhciReset() is
added; but in that case, a full revert looks unjustified, since the EBS
handler will have to be re-added for AhciReset() anyway.)

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

Re: [edk2] Unable to Loacte EdkiiIoMmuProtocol.

[Yao, Jiewen]

You need include a driver to produce gEdkiiIoMmuProtocolGuid.

One sample is IntelSiliconPkg\Feature\VTd\IntelVTdDxe

Thank you
Yao Jiewen

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Amit
> kumar
> Sent: Monday, November 27, 2017 7:24 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] Unable to Loacte EdkiiIoMmuProtocol.
> 
> Hi.
> I am trying to allocate a mem buffer above 4Gb address but the allocation
> since i am unable to locate EdkiiIoMmuProtocol.
> 
> Status = gBS->LocateProtocol (, NULL, (VOID
> **));
>   Status = mIoMmuProtocol->AllocateBuffer (
>mIoMmuProtocol,
>MaxAllocateType,
>EfiBootServicesData,
>1,
>   HostAddress,
>   EDKII_IOMMU_ATTRIBUTE_DUAL_ADDRESS_CYCLE
>   );
> 
> 
> can somebody tell me the right way to do it. ?
> 
> 
> Best Regards
> Amit Kumar
> 
> ___
> 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] MdeModulePkg/AtaAtapiPassThru: disable only BM-DMA at ExitBootServices()

[Yao, Jiewen]

Yes, Laszlo, you are right. Back to that time, we did not realize there will be 
S4 issue.

Now, I agree with you that AhciReset is a better way.
And I think we also need test different UEFI OS (normal boot/S4) to see if 
there is other issue.

Hope AhciReset() is good enough, and won't bring compatibility issue.


Thank you
Yao Jiewen


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo
> Ersek
> Sent: Monday, November 27, 2017 8:30 PM
> To: Yao, Jiewen ; Ni, Ruiyu ; Paolo
> Bonzini 
> Cc: edk2-devel-01 ; Dann Frazier
> ; Dong, Eric ; Zeng, Star
> ; Ard Biesheuvel 
> Subject: Re: [edk2] [PATCH] MdeModulePkg/AtaAtapiPassThru: disable only
> BM-DMA at ExitBootServices()
> 
> Hi Jiewen,
> 
> On 11/24/17 02:40, Yao, Jiewen wrote:
> > Maybe, can we revisit the original requirement on why we need disable BME at
> ExitBootService for OVMF?
> >
> > I recall we have lots of discussion at September. It is good to refresh.
> >
> > ==
> > [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common
> buffers at ExitBootServices()
> > https://lists.01.org/pipermail/edk2-devel/2017-September/014099.html
> >
> > At ExitBootServices(), PCI and VirtIo drivers should only care about
> > aborting pending DMA on the devices. Cleaning up PciIo mappings (which
> > ultimately boil down to IOMMU mappings) for those aborted DMA operations
> > should be the job of the IOMMU driver.
> > ==
> >
> > I think the Driver Writer's Guide recommend to stop the transaction.
> > But it does not say you must turn off BME.
> > Clear BME is just one way to meet the recommendation.
> > Maybe we can figure out other way to halt the controller, or stop DMA
> transaction?
> > Such as stop timer event, set device reset/halt register, etc.
> > I think USB has already done that.
> >
> >
> > On the other hand, I do not think "OVMF does not support S4" is a good
> justification to add PCD.
> > Yes, it does not support at this moment. But who knows the status after 3 
> > or 5
> years?
> > I also heard some VMM do support S4 resume Guest.
> >
> >
> > I also recommend to rollback all BME operation at EBS as first step, then go
> back to see what is best way to
> 
> I agree that, if the device and the driver offer another way to abort
> pending DMA, we can use that too.
> 
> In fact, my very first patch for AtaAtapiPassThru on this topic used
> AhciReset():
> 
> [1]
> http://mid.mail-archive.com/20170903195449.30261-5-lersek@redhat.com
> 
> But then you recommended clearing BusMasterEnable:
> 
> [2]
> http://mid.mail-archive.com/74D8A39837DF1E4DA445A8C0B3885C503A9A79
> b...@shsmsx102.ccr.corp.intel.com
> 
> 
> (The old patch [1] I referenced above also called PciIo->Unmap(). We've
> since agreed that *that* part of the idea was wrong, so I'm not
> suggesting to return to PciIo->Unmap().)
> 
> So, perhaps AhciReset() would be good enough after all, for aborting
> pending DMA.
> 
> Thanks,
> Laszlo
> 
> 
> 
> 
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ni,
> >> Ruiyu
> >> Sent: Friday, November 24, 2017 9:04 AM
> >> To: Paolo Bonzini 
> >> Cc: Dong, Eric ; Ard Biesheuvel
> >> ; edk2-devel-01 ;
> Dann
> >> Frazier ; Laszlo Ersek ; Zeng,
> Star
> >> 
> >> Subject: Re: [edk2] [PATCH] MdeModulePkg/AtaAtapiPassThru: disable only
> >> BM-DMA at ExitBootServices()
> >>
> >> Maybe win10 does some optimization in S4 path.
> >>
> >> Sent from a small-screen device
> >>
> >> 在 2017年11月24日，上午8:01，Paolo Bonzini
> >> > 写道：
> >>
> >> On 23/11/2017 14:08, Laszlo Ersek wrote:
> >> On 11/23/17 03:20, Ni, Ruiyu wrote:
> >> I cannot explain precisely why the S4 resume fails.
> >> I can just guess: Windows might have some assumptions on the BM bit.
> >> Can we make this configurable on the platform level somehow?
> >>
> >> On one hand, I certainly don't want to break Windows 10, even in case
> >> this issue ultimately turns out to be a Windows 10 bug.
> >>
> >> On the other hand, OVMF does not support S4, and disabling BMDMA at
> >> ExitBootServices() in PCI drivers is specifically what the Driver
> >> Writers' Guide recommends. Otherwise pending DMA could corrupt OS
> memory.
> >>
> >> S4 can be done by the OS even if firmware says it doesn't support it.
> >>
> >> Once hibernation is done, it is merely a "courtesy" for the OSPM to turn
> >> off the computer using the _S4 ACPI object rather than _S5.
> >>
> >> Paolo
> >> ___
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> 

Re: [edk2] [RFC] ACPI table HID/CID allocation

[Udit Kumar]

> >> Hmnnn it sounds like jedec,spi-nor meets this test.
> >>
> >> There is only one property in the DT bindings that describes the
> >> device itself (fast read support) rather than its "bus address" (chip
> >> select, frequency). Further, that single property is obsolete, at
> >> least for Linux; the kernel driver now contains a quirks tables to
> >> look up by device ID whether fast read is supported and will use that
> >> on non-DT systems (and also to censor broken DT systems ;-) ).
> >
> > You mean, this more on bus frame work, how to probe slave.
> > Example rtc-ds3232.c , when is spi mode just need name but for i2c
> > mode it needs of_match_table
> I mean nothing more or less than that PRP0001 + compatible:jedec,spi-nor is
> tolerable within the ACPI tables (rather than allocating a HID) because it 
> has no
> properties[1] beyond the bus configuration that ACPI can already describe

Thanks.  So tables are tolerable with PRP0001 + compatible
 
> I'm afraid I don't understand how rtc-ds3232.c fits into this topic; there 
> are no

Not directly , but just to mention that this is more on bus frame work (or say
controller driver) how they are probing their slaves. 

> documented device tree bindings for the SPI variant of this part and, in any 
> case,
> an RTC should use the ACPI interfaces
> (rtc-cmos.c) so the choice HID is a moot point; it should not be in the ACPI 
> tables
> at all.

Sure, RTC will not be exposed by acpi tables. 

> 
> Daniel.
> 
> 
> [1] Assuming one accepts the argument that the m25p,fast-read property
>  can be ignored.
>  ignored.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] [edk2-platforms]:Enabling Secure boot feature support on hikey platfrom

[Laszlo Ersek]

On 11/26/17 16:22, Leif Lindholm wrote:
> (Adding Laszlo to cc based on a single comment I make below.)
> 
> On Tue, Nov 21, 2017 at 04:23:36PM +0530, kalyan-nagabhirava wrote:

>>  [Guids.common]
>>gHiKeyTokenSpaceGuid  =  { 0x91148425, 0xcdd2, 0x4830, { 0x8b, 
>> 0xd0, 0xc6, 0x1c, 0x6d, 0xea, 0x36, 0x21 } }
>> +  gHwTokenSpaceGuid =  { 0x, 0x74c5, 0x4043, { 0xb4, 
>> 0x17, 0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }
> 
> This very much looks like a not properly generated GUID.
> GUIDs must always be generated using an RFC4122-compliant algorithm.
> I generally recommend using
> https://www.guidgenerator.com/online-guid-generator.aspx.

I just run "uuidgen" in a terminal window.

>> +EFI_STATUS
>> +EFIAPI
>> +FvbSetAttributes(
>> +  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL  *This,
>> +  IN OUTEFI_FVB_ATTRIBUTES_2 *Attributes
>> +  )
>> +{
>> +  DEBUG ((DEBUG_BLKIO, "FvbSetAttributes(0x%X) is not 
>> supported\n",*Attributes));
>> +  return EFI_UNSUPPORTED;
> 
> As per my (very) recent comment to Marcin, I do not believe returning
> EFI_UNSUPPORTED is a valid thing to do here. Which to me suggests the
> implementation of FvbGetAttributes is also incorrect.
> 
> Laszlo - what's your take on this in conjunction with PI 1.6 section
> 3.4.2? OvmfPkg does something very similar in
> EmuVariableFvbRuntimeDxe/Fvb.c.

I guess you are right. The particular OvmfPkg code that you mention is
likely also spec-breaking.

FWIW, in the OVMF flash driver that actually uses pflash, namely

  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c

the FvbSetVolumeAttributes() function appears both appropriate for the
spec and generic enough to copy elsewhere.

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

Re: [edk2] [PATCH] MdeModulePkg/AtaAtapiPassThru: disable only BM-DMA at ExitBootServices()

[Zeng, Star]

It is a way also I am thinking. Hope no OS takes any assumption related to 
AhciReset().

Thanks,
Star
-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Monday, November 27, 2017 8:30 PM
To: Yao, Jiewen ; Ni, Ruiyu ; Paolo 
Bonzini 
Cc: Dong, Eric ; Ard Biesheuvel 
; edk2-devel-01 ; Dann 
Frazier ; Zeng, Star 
Subject: Re: [edk2] [PATCH] MdeModulePkg/AtaAtapiPassThru: disable only BM-DMA 
at ExitBootServices()

Hi Jiewen,

On 11/24/17 02:40, Yao, Jiewen wrote:
> Maybe, can we revisit the original requirement on why we need disable BME at 
> ExitBootService for OVMF?
> 
> I recall we have lots of discussion at September. It is good to refresh.
> 
> ==
> [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common 
> buffers at ExitBootServices() 
> https://lists.01.org/pipermail/edk2-devel/2017-September/014099.html
> 
> At ExitBootServices(), PCI and VirtIo drivers should only care about 
> aborting pending DMA on the devices. Cleaning up PciIo mappings (which 
> ultimately boil down to IOMMU mappings) for those aborted DMA 
> operations should be the job of the IOMMU driver.
> ==
> 
> I think the Driver Writer's Guide recommend to stop the transaction.
> But it does not say you must turn off BME.
> Clear BME is just one way to meet the recommendation.
> Maybe we can figure out other way to halt the controller, or stop DMA 
> transaction?
> Such as stop timer event, set device reset/halt register, etc.
> I think USB has already done that.
> 
> 
> On the other hand, I do not think "OVMF does not support S4" is a good 
> justification to add PCD.
> Yes, it does not support at this moment. But who knows the status after 3 or 
> 5 years?
> I also heard some VMM do support S4 resume Guest.
> 
> 
> I also recommend to rollback all BME operation at EBS as first step, 
> then go back to see what is best way to

I agree that, if the device and the driver offer another way to abort pending 
DMA, we can use that too.

In fact, my very first patch for AtaAtapiPassThru on this topic used
AhciReset():

[1] http://mid.mail-archive.com/20170903195449.30261-5-lersek@redhat.com

But then you recommended clearing BusMasterEnable:

[2]
http://mid.mail-archive.com/74D8A39837DF1E4DA445A8C0B3885C503A9A79BD@shsmsx102.ccr.corp.intel.com


(The old patch [1] I referenced above also called PciIo->Unmap(). We've since 
agreed that *that* part of the idea was wrong, so I'm not suggesting to return 
to PciIo->Unmap().)

So, perhaps AhciReset() would be good enough after all, for aborting pending 
DMA.

Thanks,
Laszlo




>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
>> Of Ni, Ruiyu
>> Sent: Friday, November 24, 2017 9:04 AM
>> To: Paolo Bonzini 
>> Cc: Dong, Eric ; Ard Biesheuvel 
>> ; edk2-devel-01 ; 
>> Dann Frazier ; Laszlo Ersek ; 
>> Zeng, Star 
>> Subject: Re: [edk2] [PATCH] MdeModulePkg/AtaAtapiPassThru: disable 
>> only BM-DMA at ExitBootServices()
>>
>> Maybe win10 does some optimization in S4 path.
>>
>> Sent from a small-screen device
>>
>> 在 2017年11月24日，上午8:01，Paolo Bonzini
>> > 写道：
>>
>> On 23/11/2017 14:08, Laszlo Ersek wrote:
>> On 11/23/17 03:20, Ni, Ruiyu wrote:
>> I cannot explain precisely why the S4 resume fails.
>> I can just guess: Windows might have some assumptions on the BM bit.
>> Can we make this configurable on the platform level somehow?
>>
>> On one hand, I certainly don't want to break Windows 10, even in case 
>> this issue ultimately turns out to be a Windows 10 bug.
>>
>> On the other hand, OVMF does not support S4, and disabling BMDMA at
>> ExitBootServices() in PCI drivers is specifically what the Driver 
>> Writers' Guide recommends. Otherwise pending DMA could corrupt OS memory.
>>
>> S4 can be done by the OS even if firmware says it doesn't support it.
>>
>> Once hibernation is done, it is merely a "courtesy" for the OSPM to 
>> turn off the computer using the _S4 ACPI object rather than _S5.
>>
>> Paolo
>> ___
>> 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] ArmVirtPkg/PrePi: don't export PE/COFF and LZMA libraries via HOBs

[Laszlo Ersek]

On 11/24/17 10:51, Ard Biesheuvel wrote:
> The PrePi code we inherited from ArmPlatformPkg contains a rather
> obscure optimization, where entry points of the PE/COFF and LZMA
> handling routines are recorded in special HOBs, allowing DXE core
> to call into that code directly rather than carry its own copy of
> these libraries.
> 
> Given that no ArmVirtPkg platforms actually include the library
> resolutions* that take advantage of these optimizations, let's not
> bother with them, and remove the associated code.
> 
> * 
> EmbeddedPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf
>   EmbeddedPkg/Library/DxeHobPeCoffLib/DxeHobPeCoffLib.inf
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf |   1 -
>  ArmVirtPkg/PrePi/LzmaDecompress.h   | 103 
> 
>  ArmVirtPkg/PrePi/PrePi.c|  11 ---
>  3 files changed, 115 deletions(-)
> 
> diff --git a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf 
> b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
> index 58290d2d1b76..b3a3f5da065e 100755
> --- a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
> +++ b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
> @@ -51,7 +51,6 @@ [LibraryClasses]
>SerialPortLib
>ExtractGuidedSectionLib
>LzmaDecompressLib
> -  PeCoffGetEntryPointLib
>PrePiLib
>MemoryAllocationLib
>HobLib
> diff --git a/ArmVirtPkg/PrePi/LzmaDecompress.h 
> b/ArmVirtPkg/PrePi/LzmaDecompress.h
> deleted file mode 100644
> index a79ff343d231..
> --- a/ArmVirtPkg/PrePi/LzmaDecompress.h
> +++ /dev/null
> @@ -1,103 +0,0 @@
> -/** @file
> -  LZMA Decompress Library header file
> -
> -  Copyright (c) 2006 - 2010, 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.
> -
> -**/
> -
> -#ifndef __LZMA_DECOMPRESS_H___
> -#define __LZMA_DECOMPRESS_H___
> -
> -/**
> -  Examines a GUIDed section and returns the size of the decoded buffer and 
> the
> -  size of an scratch buffer required to actually decode the data in a GUIDed 
> section.
> -
> -  Examines a GUIDed section specified by InputSection.
> -  If GUID for InputSection does not match the GUID that this handler 
> supports,
> -  then RETURN_UNSUPPORTED is returned.
> -  If the required information can not be retrieved from InputSection,
> -  then RETURN_INVALID_PARAMETER is returned.
> -  If the GUID of InputSection does match the GUID that this handler supports,
> -  then the size required to hold the decoded buffer is returned in 
> OututBufferSize,
> -  the size of an optional scratch buffer is returned in ScratchSize, and the 
> Attributes field
> -  from EFI_GUID_DEFINED_SECTION header of InputSection is returned in 
> SectionAttribute.
> -
> -  If InputSection is NULL, then ASSERT().
> -  If OutputBufferSize is NULL, then ASSERT().
> -  If ScratchBufferSize is NULL, then ASSERT().
> -  If SectionAttribute is NULL, then ASSERT().
> -
> -
> -  @param[in]  InputSection   A pointer to a GUIDed section of an FFS 
> formatted file.
> -  @param[out] OutputBufferSize   A pointer to the size, in bytes, of an 
> output buffer required
> - if the buffer specified by InputSection 
> were decoded.
> -  @param[out] ScratchBufferSize  A pointer to the size, in bytes, required 
> as scratch space
> - if the buffer specified by InputSection 
> were decoded.
> -  @param[out] SectionAttribute   A pointer to the attributes of the GUIDed 
> section. See the Attributes
> - field of EFI_GUID_DEFINED_SECTION in the PI 
> Specification.
> -
> -  @retval  RETURN_SUCCESSThe information about InputSection was 
> returned.
> -  @retval  RETURN_UNSUPPORTEDThe section specified by InputSection 
> does not match the GUID this handler supports.
> -  @retval  RETURN_INVALID_PARAMETER  The information can not be retrieved 
> from the section specified by InputSection.
> -
> -**/
> -RETURN_STATUS
> -EFIAPI
> -LzmaGuidedSectionGetInfo (
> -  IN  CONST VOID  *InputSection,
> -  OUT UINT32  *OutputBufferSize,
> -  OUT UINT32  *ScratchBufferSize,
> -  OUT UINT16  *SectionAttribute
> -  );
> -
> -/**
> -  Decompress a LZAM compressed GUIDed section into a caller allocated output 
> buffer.
> -
> -  Decodes the GUIDed section specified by InputSection.
> -  If GUID for InputSection does not match the GUID that this handler 
> supports, 

Re: [edk2] [PATCH] MdeModulePkg/AtaAtapiPassThru: disable only BM-DMA at ExitBootServices()

[Laszlo Ersek]

Hi Jiewen,

On 11/24/17 02:40, Yao, Jiewen wrote:
> Maybe, can we revisit the original requirement on why we need disable BME at 
> ExitBootService for OVMF?
> 
> I recall we have lots of discussion at September. It is good to refresh.
> 
> ==
> [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common buffers at 
> ExitBootServices()
> https://lists.01.org/pipermail/edk2-devel/2017-September/014099.html
> 
> At ExitBootServices(), PCI and VirtIo drivers should only care about
> aborting pending DMA on the devices. Cleaning up PciIo mappings (which
> ultimately boil down to IOMMU mappings) for those aborted DMA operations
> should be the job of the IOMMU driver.
> ==
> 
> I think the Driver Writer's Guide recommend to stop the transaction.
> But it does not say you must turn off BME.
> Clear BME is just one way to meet the recommendation.
> Maybe we can figure out other way to halt the controller, or stop DMA 
> transaction?
> Such as stop timer event, set device reset/halt register, etc.
> I think USB has already done that.
> 
> 
> On the other hand, I do not think "OVMF does not support S4" is a good 
> justification to add PCD.
> Yes, it does not support at this moment. But who knows the status after 3 or 
> 5 years?
> I also heard some VMM do support S4 resume Guest.
> 
> 
> I also recommend to rollback all BME operation at EBS as first step, then go 
> back to see what is best way to

I agree that, if the device and the driver offer another way to abort
pending DMA, we can use that too.

In fact, my very first patch for AtaAtapiPassThru on this topic used
AhciReset():

[1] http://mid.mail-archive.com/20170903195449.30261-5-lersek@redhat.com

But then you recommended clearing BusMasterEnable:

[2]
http://mid.mail-archive.com/74D8A39837DF1E4DA445A8C0B3885C503A9A79BD@shsmsx102.ccr.corp.intel.com


(The old patch [1] I referenced above also called PciIo->Unmap(). We've
since agreed that *that* part of the idea was wrong, so I'm not
suggesting to return to PciIo->Unmap().)

So, perhaps AhciReset() would be good enough after all, for aborting
pending DMA.

Thanks,
Laszlo




>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ni,
>> Ruiyu
>> Sent: Friday, November 24, 2017 9:04 AM
>> To: Paolo Bonzini 
>> Cc: Dong, Eric ; Ard Biesheuvel
>> ; edk2-devel-01 ; Dann
>> Frazier ; Laszlo Ersek ; Zeng, Star
>> 
>> Subject: Re: [edk2] [PATCH] MdeModulePkg/AtaAtapiPassThru: disable only
>> BM-DMA at ExitBootServices()
>>
>> Maybe win10 does some optimization in S4 path.
>>
>> Sent from a small-screen device
>>
>> 在 2017年11月24日，上午8:01，Paolo Bonzini
>> > 写道：
>>
>> On 23/11/2017 14:08, Laszlo Ersek wrote:
>> On 11/23/17 03:20, Ni, Ruiyu wrote:
>> I cannot explain precisely why the S4 resume fails.
>> I can just guess: Windows might have some assumptions on the BM bit.
>> Can we make this configurable on the platform level somehow?
>>
>> On one hand, I certainly don't want to break Windows 10, even in case
>> this issue ultimately turns out to be a Windows 10 bug.
>>
>> On the other hand, OVMF does not support S4, and disabling BMDMA at
>> ExitBootServices() in PCI drivers is specifically what the Driver
>> Writers' Guide recommends. Otherwise pending DMA could corrupt OS memory.
>>
>> S4 can be done by the OS even if firmware says it doesn't support it.
>>
>> Once hibernation is done, it is merely a "courtesy" for the OSPM to turn
>> off the computer using the _S4 ACPI object rather than _S5.
>>
>> Paolo
>> ___
>> 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] Accessing memory above 0xFFFFFFFF limit.

[Amit kumar]

Hi.
I am trying to allocate a mem buffer above 4Gb address but the allocation
since i am unable to locate EdkiiIoMmuProtocol.

Status = gBS->LocateProtocol (, NULL, (VOID 
**));
  Status = mIoMmuProtocol->AllocateBuffer (
   mIoMmuProtocol,
   MaxAllocateType,
   EfiBootServicesData,
   1,
  HostAddress,
  EDKII_IOMMU_ATTRIBUTE_DUAL_ADDRESS_CYCLE
  );

Best Regards
Amit Kumar

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

Re: [edk2] [RFC] ACPI table HID/CID allocation

[Daniel Thompson]

On 25/11/17 12:56, Udit Kumar wrote:>> -Original Message-

From: Daniel Thompson [mailto:daniel.thomp...@linaro.org]
Sent: Thursday, November 23, 2017 1:42 AM
To: Ard Biesheuvel 
Cc: Udit Kumar ; Leif Lindholm
; edk2-devel@lists.01.org; Varun Sethi
; Graeme Gregory 
Subject: Re: [RFC] ACPI table HID/CID allocation





PRP0001 + compatible was invented to avoid the need to allocate a _HID
for each and every component in existence that can already be
identified by a DT compatible string (and little else except, e.g., a
I2C slave address) and is not deeply engrained in the SoC in terms of
clock tree, power states etc. So while internal/external may not be
the most accurate distinction, it is still a useful one IMHO.


Hmnnn it sounds like jedec,spi-nor meets this test.

There is only one property in the DT bindings that describes the device
itself (fast read support) rather than its "bus address" (chip select,
frequency). Further, that single property is obsolete, at least for
Linux; the kernel driver now contains a quirks tables to look up by
device ID whether fast read is supported and will use that on non-DT
systems (and also to censor broken DT systems ;-) ).


You mean, this more on bus frame work, how to probe slave.
Example rtc-ds3232.c , when is spi mode just need name
but for i2c mode it needs of_match_table
I mean nothing more or less than that PRP0001 + compatible:jedec,spi-nor 
is tolerable within the ACPI tables (rather than allocating a HID) 
because it has no properties[1] beyond the bus configuration that ACPI 
can already describe


I'm afraid I don't understand how rtc-ds3232.c fits into this topic; 
there are no documented device tree bindings for the SPI variant of this 
part and, in any case, an RTC should use the ACPI interfaces 
(rtc-cmos.c) so the choice HID is a moot point; it should not be in the 
ACPI tables at all.



Daniel.


[1] Assuming one accepts the argument that the m25p,fast-read property
can be ignored.
ignored.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms] [PATCH v3 0/9] Platform/NXP

[Leif Lindholm]

On Mon, Nov 27, 2017 at 04:21:48PM +0530, Meenakshi Aggarwal wrote:
> v3:
> Added patch-prefix

I only meant you could apply this in future.
However, this new posting has ended up with ([PATCH ...][PATCH...]),
which is not ideal. There is no need to edit the patch subject for
each version - just specify (for a v3)
git format-patch --subject-prefix="PATCH edk2-platforms" -v3

If there is a need for a v4, or for future submissions, please also
look into
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contributor-workflow
for how to configure your cloned git repository to generate summaries
and diffs that are easier to review.

> v2:
> 1. Incorporated styling comments
> 2. Removed/Rewrite function referred from linux
> 3. Created DS1307 Library under Silicon/Maxim and make it i2c driver based.
> 4. Created i2c driver
> 
> Meenakshi Aggarwal (9):
>   Platform/NXP: Add support for Big Endian Mmio APIs
>   Platform/NXP : Add support for Watchdog driver
>   SocLib : Add support for initialization of peripherals
>   Platform/NXP : Add support for DUART library
>   Platform/NXP: Add support for I2c driver
>   Silicon/Maxim : Add support for DS1307 RTC library
>   Platform/NXP: Add support for ArmPlatformLib
>   Compilation : Add the fdf, dsc and dec files.
>   Build : Add build script and environment script
> 
>  Platform/NXP/Drivers/I2cDxe/I2cDxe.c   | 728 
> +
>  Platform/NXP/Drivers/I2cDxe/I2cDxe.h   |  64 ++
>  Platform/NXP/Drivers/I2cDxe/I2cDxe.inf |  57 ++
>  Platform/NXP/Drivers/WatchDog/WatchDog.c   | 421 
>  Platform/NXP/Drivers/WatchDog/WatchDog.h   |  37 ++
>  Platform/NXP/Drivers/WatchDog/WatchDogDxe.inf  |  47 ++
>  Platform/NXP/Env.cshrc |  77 +++
>  Platform/NXP/Include/Bitops.h  | 179 +
>  Platform/NXP/Include/Library/BeIoLib.h | 332 ++
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dec   |  29 +
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc   |  77 +++
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf   | 281 
>  .../Library/PlatformLib/ArmPlatformLib.c   | 105 +++
>  .../Library/PlatformLib/ArmPlatformLib.inf |  70 ++
>  .../Library/PlatformLib/NxpQoriqLsHelper.S |  38 ++
>  .../Library/PlatformLib/NxpQoriqLsMem.c| 184 ++

In particular these ..., meaning reviewers need to start guessing
which files are affected.

Best Regards,

Leif

>  Platform/NXP/Library/BeIoLib/BeIoLib.c | 400 +++
>  Platform/NXP/Library/BeIoLib/BeIoLib.inf   |  31 +
>  Platform/NXP/Library/DUartPortLib/DUart.h  | 128 
>  Platform/NXP/Library/DUartPortLib/DUartPortLib.c   | 331 ++
>  Platform/NXP/Library/DUartPortLib/DUartPortLib.inf |  39 ++
>  Platform/NXP/NxpQoriqLs.dec| 248 +++
>  Platform/NXP/NxpQoriqLs.dsc| 453 +
>  Platform/NXP/Readme.md |  15 +
>  Platform/NXP/build.sh  | 103 +++
>  Silicon/Maxim/Library/Ds1307RtcLib/Ds1307Rtc.h |  59 ++
>  Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c  | 327 +
>  .../Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.dec|  26 +
>  .../Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.inf|  45 ++
>  Silicon/NXP/Chassis/Chassis.c  | 413 
>  Silicon/NXP/Chassis/Chassis.h  | 144 
>  Silicon/NXP/Chassis/Chassis2/Chassis2.dec  |  19 +
>  Silicon/NXP/Chassis/Chassis2/SerDes.h  |  69 ++
>  Silicon/NXP/Chassis/Chassis2/Soc.c | 145 
>  Silicon/NXP/Chassis/Chassis2/Soc.h | 376 +++
>  Silicon/NXP/Chassis/LS1043aSocLib.inf  |  47 ++
>  Silicon/NXP/Chassis/SerDes.c   | 254 +++
>  Silicon/NXP/LS1043A/Include/SocSerDes.h|  55 ++
>  Silicon/NXP/LS1043A/LS1043A.dec|  22 +
>  Silicon/NXP/LS1043A/LS1043A.dsc|  82 +++
>  40 files changed, 6557 insertions(+)
>  create mode 100644 Platform/NXP/Drivers/I2cDxe/I2cDxe.c
>  create mode 100644 Platform/NXP/Drivers/I2cDxe/I2cDxe.h
>  create mode 100644 Platform/NXP/Drivers/I2cDxe/I2cDxe.inf
>  create mode 100644 Platform/NXP/Drivers/WatchDog/WatchDog.c
>  create mode 100644 Platform/NXP/Drivers/WatchDog/WatchDog.h
>  create mode 100644 Platform/NXP/Drivers/WatchDog/WatchDogDxe.inf
>  create mode 100755 Platform/NXP/Env.cshrc
>  create mode 100644 Platform/NXP/Include/Bitops.h
>  create mode 100644 Platform/NXP/Include/Library/BeIoLib.h
>  create mode 100644 Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dec
>  create mode 100644 Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
>  create mode 100644 Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
>  create mode 100644 
> 

[edk2] Unable to Loacte EdkiiIoMmuProtocol.

[Amit kumar]

Hi.
I am trying to allocate a mem buffer above 4Gb address but the allocation
since i am unable to locate EdkiiIoMmuProtocol.

Status = gBS->LocateProtocol (, NULL, (VOID 
**));
  Status = mIoMmuProtocol->AllocateBuffer (
   mIoMmuProtocol,
   MaxAllocateType,
   EfiBootServicesData,
   1,
  HostAddress,
  EDKII_IOMMU_ATTRIBUTE_DUAL_ADDRESS_CYCLE
  );


can somebody tell me the right way to do it. ?


Best Regards
Amit Kumar

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

Re: [edk2] [PATCH 1/1] MdePkg/IoLib: Add support for big-endian MMIO

[Leif Lindholm]

On Mon, Nov 27, 2017 at 06:06:33AM +, Meenakshi Aggarwal wrote:
> Hi Leif,
> 
> This is regarding Big-Endian Library patch ([PATCH v2 1/9]
> Platform/NXP: Add support for Big Endian Mmio APIs)
> 
> We have started this discussion before and the suggestion was to
> create a separate .inf file keeping APIs name same
> e.g. MmioRead/MmioWrite in MdePkg.
> 
> But we can't go with this approach (reason mentioned by Udit).
> 
> 
> So please suggest if we should keep this library under Platform/NXP
> or I send a new patch moving this library in MdePkg.

For now, absolutely keep it under Platform/NXP.

I do believe this will naturally make its way into edk2 at some point
in the future. Perhaps then as an alternative IoLib implementation to
override specific drivers (as suggested in the original thread).

> But we have to keep a different name for Big Endian MMIO APIs.

Sure.

Regards,

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

Re: [edk2] [PATCH] MdeModulePkg/PerformanceLib: add lock protection

[Zeng, Star]

Reviewed-by: Star Zeng  with the redundant with at " with 
with " in the commit log.

Thanks,
Star
-Original Message-
From: Heyi Guo [mailto:heyi@linaro.org] 
Sent: Monday, November 27, 2017 11:32 AM
To: linaro-u...@lists.linaro.org; edk2-devel@lists.01.org
Cc: Heyi Guo ; Zeng, Star ; Dong, 
Eric ; Gao, Liming 
Subject: [PATCH] MdeModulePkg/PerformanceLib: add lock protection

DXE performance gauge record access functions might be reentered since we are 
supporting something like USB hot-plug, which is a timer event where 
gBS->ConnectController might be called and then PERF will be called in 
CoreConnectSingleController.

When StartGaugeEx is being reentered, not only the gauge record might be 
overwritten, more serious situation will be caused if gauge data buffer 
reallocation procedure is interrupted, between line 180 and 187 in 
DxeCorePerformanceLib.c specifically. There, mMaxGaugeRecords will be doubled 
twice (denoted as 4X), but mGaugeData only points to a buffer of size 2X, which 
will probably cause the following 2X memory to be overflowed when gauge records 
are increased.

So we add EFI lock with with TPL_NOTIFY in StartGaugeEx/EndGaugeEx/GetGaugeEx 
to avoid memory overflow and gauge data corruption.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Liming Gao 
---
 .../DxeCorePerformanceLib/DxeCorePerformanceLib.c  | 67 +-
 1 file changed, 66 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c 
b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
index 51f488a..7c0e207 100644
--- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
+++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
@@ -63,6 +63,11 @@ PERFORMANCE_EX_PROTOCOL mPerformanceExInterface = {
 
 PERFORMANCE_PROPERTY  mPerformanceProperty;
 
+//
+//  Gauge record lock to avoid data corruption or even memory overflow 
+// STATIC EFI_LOCK mPerfRecordLock = EFI_INITIALIZE_LOCK_VARIABLE 
+(TPL_NOTIFY);
+
 /**
   Searches in the gauge array with keyword Handle, Token, Module and 
Identifier.
 
@@ -162,6 +167,12 @@ StartGaugeEx (
   UINTN OldGaugeDataSize;
   GAUGE_DATA_HEADER *OldGaugeData;
   UINT32Index;
+  EFI_STATUSStatus;
+
+  Status = EfiAcquireLockOrFail ();  if (EFI_ERROR 
+ (Status)) {
+return Status;
+  }
 
   Index = mGaugeData->NumberOfEntries;
   if (Index >= mMaxGaugeRecords) {
@@ -175,6 +186,7 @@ StartGaugeEx (
 
 NewGaugeData = AllocateZeroPool (GaugeDataSize);
 if (NewGaugeData == NULL) {
+  EfiReleaseLock ();
   return EFI_OUT_OF_RESOURCES;
 }
 
@@ -209,6 +221,8 @@ StartGaugeEx (
 
   mGaugeData->NumberOfEntries++;
 
+  EfiReleaseLock ();
+
   return EFI_SUCCESS;
 }
 
@@ -250,6 +264,12 @@ EndGaugeEx (
 {
   GAUGE_DATA_ENTRY_EX *GaugeEntryExArray;
   UINT32  Index;
+  EFI_STATUS  Status;
+
+  Status = EfiAcquireLockOrFail ();  if (EFI_ERROR 
+ (Status)) {
+return Status;
+  }
 
   if (TimeStamp == 0) {
 TimeStamp = GetPerformanceCounter (); @@ -257,11 +277,13 @@ EndGaugeEx (
 
   Index = InternalSearchForGaugeEntry (Handle, Token, Module, Identifier);
   if (Index >= mGaugeData->NumberOfEntries) {
+EfiReleaseLock ();
 return EFI_NOT_FOUND;
   }
   GaugeEntryExArray = (GAUGE_DATA_ENTRY_EX *) (mGaugeData + 1);
   GaugeEntryExArray[Index].EndTimeStamp = TimeStamp;
 
+  EfiReleaseLock ();
   return EFI_SUCCESS;
 }
 
@@ -274,6 +296,8 @@ EndGaugeEx (
   If it stands for a valid entry, then EFI_SUCCESS is returned and
   GaugeDataEntryEx stores the pointer to that entry.
 
+  This internal function is added to avoid releasing lock before each return 
statement.
+
   @param  LogEntryKey The key for the previous performance 
measurement log entry.
   If 0, then the first performance measurement 
log entry is retrieved.
   @param  GaugeDataEntryExThe indirect pointer to the extended gauge 
data entry specified by LogEntryKey
@@ -287,7 +311,7 @@ EndGaugeEx (
 **/
 EFI_STATUS
 EFIAPI
-GetGaugeEx (
+InternalGetGaugeEx (
   IN  UINTN LogEntryKey,
   OUT GAUGE_DATA_ENTRY_EX   **GaugeDataEntryEx
   )
@@ -314,6 +338,47 @@ GetGaugeEx (
 }
 
 /**
+  Retrieves a previously logged performance measurement.
+  It can also retrieve the log created by StartGauge and EndGauge of 
+ PERFORMANCE_PROTOCOL,  and then assign the Identifier with 0.
+
+  Retrieves the performance log entry from the performance log specified by 
LogEntryKey.
+  If it stands for a valid entry, then EFI_SUCCESS is returned and  
+ GaugeDataEntryEx stores the pointer to that entry.
+
+  

Re: [edk2] [PATCH v2 0/3] MdeModulePkg/SerialDxe: Do not fail reset when SetAttributes is not supported

[Zeng, Star]

I mean not mind.

-Original Message-
From: Zeng, Star 
Sent: Monday, November 27, 2017 6:47 PM
To: Julien Grall ; Dong, Eric ; 
pankaj.ban...@nxp.com; ler...@redhat.com; leif.lindh...@linaro.org
Cc: edk2-devel@lists.01.org; Ni, Ruiyu ; Zeng, Star 

Subject: RE: [PATCH v2 0/3] MdeModulePkg/SerialDxe: Do not fail reset when 
SetAttributes is not supported

Sure. :)


Thanks,
Star
-Original Message-
From: Julien Grall [mailto:julien.gr...@linaro.org]
Sent: Monday, November 27, 2017 6:39 PM
To: Zeng, Star ; Dong, Eric ; 
pankaj.ban...@nxp.com; ler...@redhat.com; leif.lindh...@linaro.org
Cc: edk2-devel@lists.01.org; Ni, Ruiyu 
Subject: Re: [PATCH v2 0/3] MdeModulePkg/SerialDxe: Do not fail reset when 
SetAttributes is not supported

Hi Star,

On 27/11/17 01:46, Zeng, Star wrote:
> Reviewed-by: Star Zeng  with some minor comments in the 
> separated patches.

Thank you for the review. I will resend the series with updating the 
description in the corresponding headers. Would you mind if I keep you 
reviewed-by?

Regards,

> 
> Thanks,
> Star
> -Original Message-
> From: Julien Grall [mailto:julien.gr...@linaro.org]
> Sent: Saturday, November 25, 2017 12:20 AM
> To: Zeng, Star ; Dong, Eric 
> ; pankaj.ban...@nxp.com; ler...@redhat.com; 
> leif.lindh...@linaro.org
> Cc: edk2-devel@lists.01.org; Julien Grall 
> Subject: [PATCH v2 0/3] MdeModulePkg/SerialDxe: Do not fail reset when 
> SetAttributes is not supported
> 
> Hi all,
> 
> This is another attempt to fix the console issue when using UEFI in Xen guest.
> This new series is based on Laszlo suggestions on the previous version (see 
> [1]).
> 
> Cheers,
> 
> [1] https://lists.01.org/pipermail/edk2-devel/2017-October/016181.html
> 
> 
> Julien Grall (3):
>MdeModulePkg/SerialDxe: Describe correctly EFI_DEVICE_ERROR for
>  SetAttributes
>MdeModulePkg/SerialDxe: Fix return valued in SerialSetAttributes
>MdeModulePkg/SerialDxe: Do not fail reset when SetAttributes is not
>  supported
> 
>   MdeModulePkg/Universal/SerialDxe/SerialIo.c | 18 +++---
>   1 file changed, 15 insertions(+), 3 deletions(-)
> 
> --
> 2.11.0
> 

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

Re: [edk2] [PATCH v2 0/3] MdeModulePkg/SerialDxe: Do not fail reset when SetAttributes is not supported

[Zeng, Star]

Sure. :)


Thanks,
Star
-Original Message-
From: Julien Grall [mailto:julien.gr...@linaro.org] 
Sent: Monday, November 27, 2017 6:39 PM
To: Zeng, Star ; Dong, Eric ; 
pankaj.ban...@nxp.com; ler...@redhat.com; leif.lindh...@linaro.org
Cc: edk2-devel@lists.01.org; Ni, Ruiyu 
Subject: Re: [PATCH v2 0/3] MdeModulePkg/SerialDxe: Do not fail reset when 
SetAttributes is not supported

Hi Star,

On 27/11/17 01:46, Zeng, Star wrote:
> Reviewed-by: Star Zeng  with some minor comments in the 
> separated patches.

Thank you for the review. I will resend the series with updating the 
description in the corresponding headers. Would you mind if I keep you 
reviewed-by?

Regards,

> 
> Thanks,
> Star
> -Original Message-
> From: Julien Grall [mailto:julien.gr...@linaro.org]
> Sent: Saturday, November 25, 2017 12:20 AM
> To: Zeng, Star ; Dong, Eric 
> ; pankaj.ban...@nxp.com; ler...@redhat.com; 
> leif.lindh...@linaro.org
> Cc: edk2-devel@lists.01.org; Julien Grall 
> Subject: [PATCH v2 0/3] MdeModulePkg/SerialDxe: Do not fail reset when 
> SetAttributes is not supported
> 
> Hi all,
> 
> This is another attempt to fix the console issue when using UEFI in Xen guest.
> This new series is based on Laszlo suggestions on the previous version (see 
> [1]).
> 
> Cheers,
> 
> [1] https://lists.01.org/pipermail/edk2-devel/2017-October/016181.html
> 
> 
> Julien Grall (3):
>MdeModulePkg/SerialDxe: Describe correctly EFI_DEVICE_ERROR for
>  SetAttributes
>MdeModulePkg/SerialDxe: Fix return valued in SerialSetAttributes
>MdeModulePkg/SerialDxe: Do not fail reset when SetAttributes is not
>  supported
> 
>   MdeModulePkg/Universal/SerialDxe/SerialIo.c | 18 +++---
>   1 file changed, 15 insertions(+), 3 deletions(-)
> 
> --
> 2.11.0
> 

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

Re: [edk2] [PATCH v2 0/3] MdeModulePkg/SerialDxe: Do not fail reset when SetAttributes is not supported

[Julien Grall]

Hi Star,

On 27/11/17 01:46, Zeng, Star wrote:

Reviewed-by: Star Zeng  with some minor comments in the 
separated patches.


Thank you for the review. I will resend the series with updating the 
description in the corresponding headers. Would you mind if I keep you 
reviewed-by?


Regards,



Thanks,
Star
-Original Message-
From: Julien Grall [mailto:julien.gr...@linaro.org]
Sent: Saturday, November 25, 2017 12:20 AM
To: Zeng, Star ; Dong, Eric ; 
pankaj.ban...@nxp.com; ler...@redhat.com; leif.lindh...@linaro.org
Cc: edk2-devel@lists.01.org; Julien Grall 
Subject: [PATCH v2 0/3] MdeModulePkg/SerialDxe: Do not fail reset when 
SetAttributes is not supported

Hi all,

This is another attempt to fix the console issue when using UEFI in Xen guest.
This new series is based on Laszlo suggestions on the previous version (see 
[1]).

Cheers,

[1] https://lists.01.org/pipermail/edk2-devel/2017-October/016181.html


Julien Grall (3):
   MdeModulePkg/SerialDxe: Describe correctly EFI_DEVICE_ERROR for
 SetAttributes
   MdeModulePkg/SerialDxe: Fix return valued in SerialSetAttributes
   MdeModulePkg/SerialDxe: Do not fail reset when SetAttributes is not
 supported

  MdeModulePkg/Universal/SerialDxe/SerialIo.c | 18 +++---
  1 file changed, 15 insertions(+), 3 deletions(-)

--
2.11.0



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

[edk2] [Patch] MdeModulePkg/NetLib: Add NetLibDetectMediaWaitTimeout() API to support EFI_NOT_READY media state detection

[fanwang2]

In wireless connection, connecting state needs to be cared more
about. ECR 1772 redefined the state EFI_NOT_READY to represent
connecting state and can be retrieved from Aip protocol. This
patch adds a new API to check media state at a specified time
interval when network is connecting until the connection process
finishes or timeout.

Cc: Fu Siyuan 
Cc: Ye Ting 
Cc: Jiaxin Wu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wang Fan 
---
 MdeModulePkg/Include/Library/NetLib.h|  40 +++
 MdeModulePkg/Library/DxeNetLib/DxeNetLib.c   | 160 +++
 MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf |   2 +
 3 files changed, 202 insertions(+)

diff --git a/MdeModulePkg/Include/Library/NetLib.h 
b/MdeModulePkg/Include/Library/NetLib.h
index b9df46c..7862df9 100644
--- a/MdeModulePkg/Include/Library/NetLib.h
+++ b/MdeModulePkg/Include/Library/NetLib.h
@@ -93,10 +93,16 @@ typedef UINT16  TCP_PORTNO;
 #define  DNS_CLASS_INET1
 #define  DNS_CLASS_CH  3
 #define  DNS_CLASS_HS  4
 #define  DNS_CLASS_ANY 255
 
+//
+// Number of 100ns units time Interval for network media state detect
+//
+#define MEDIA_STATE_DETECT_TIME_INTERVAL  100U
+
+
 #pragma pack(1)
 
 //
 // Ethernet head definition
 //
@@ -1246,10 +1252,44 @@ NetLibDetectMedia (
   IN  EFI_HANDLEServiceHandle,
   OUT BOOLEAN   *MediaPresent
   );
 
 /**
+
+  Detect media state for a network device. This routine will wait for a period 
of time at 
+  a specified checking interval when a certain network is under connecting 
until connection 
+  process finishes or timeout. If Aip protocol is supported by low layer 
drivers, three kinds
+  of media states can be detected: EFI_SUCCESS, EFI_NOT_READY and 
EFI_NO_MEDIA, represents
+  connected state, connecting state and no media state respectively. When 
function detects 
+  the current state is EFI_NOT_READY, it will loop to wait for next time's 
check until state 
+  turns to be EFI_SUCCESS or EFI_NO_MEDIA. If Aip protocol is not supported, 
function will 
+  call NetLibDetectMedia() and return state directly.
+
+  @param[in]   ServiceHandleThe handle where network service binding 
protocols are
+installed on.
+  @param[in]   Timeout  The maximum number of 100ns units to wait when 
network
+is connecting. Zero value means detect once 
and return
+immediately.
+  @param[out]  MediaState   The pointer to the detected media state.
+
+  @retval EFI_SUCCESS   Media detection success.
+  @retval EFI_INVALID_PARAMETER ServiceHandle is not a valid network device 
handle or 
+MediaState pointer is NULL.
+  @retval EFI_DEVICE_ERROR  A device error occurred.
+  @retval EFI_TIMEOUT   Network is connecting but timeout.
+
+**/
+EFI_STATUS
+EFIAPI
+NetLibDetectMediaWaitTimeout (
+  IN  EFI_HANDLEServiceHandle,
+  IN  UINT64Timeout,
+  OUT EFI_STATUS*MediaState
+  );
+
+
+/**
   Create an IPv4 device path node.
 
   The header type of IPv4 device path node is MESSAGING_DEVICE_PATH.
   The header subtype of IPv4 device path node is MSG_IPv4_DP.
   The length of the IPv4 device path node in bytes is 19.
diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c 
b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
index b8544b8..3030147 100644
--- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
+++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
@@ -17,10 +17,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 
@@ -2501,10 +2502,169 @@ Exit:
 
   return Status;
 }
 
 /**
+
+  Detect media state for a network device. This routine will wait for a period 
of time at 
+  a specified checking interval when a certain network is under connecting 
until connection 
+  process finishes or timeout. If Aip protocol is supported by low layer 
drivers, three kinds
+  of media states can be detected: EFI_SUCCESS, EFI_NOT_READY and 
EFI_NO_MEDIA, represents
+  connected state, connecting state and no media state respectively. When 
function detects 
+  the current state is EFI_NOT_READY, it will loop to wait for next time's 
check until state 
+  turns to be EFI_SUCCESS or EFI_NO_MEDIA. If Aip protocol is not supported, 
function will 
+  call NetLibDetectMedia() and return state directly.
+
+  @param[in]   ServiceHandleThe handle where network service binding 
protocols are
+installed on.
+  @param[in]   Timeout  The maximum number of 100ns units to wait when 
network
+is connecting. Zero value means detect once 
and return
+