[edk2] [Patch][edk2-platforms/devel-MinnowBoard3-UDK2017] Platform Boot Manager Library.

[zwei4]

Add library instance of PlatformBootManagerLib.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: zwei4 
---
 .../PlatformBootManagerLib/PlatformBootManager.c   | 1052 
 .../PlatformBootManagerLib/PlatformBootManager.h   |  226 +
 .../PlatformBootManagerLib.inf |  109 ++
 .../PlatformBootManagerLib/PlatformBootOption.c|  738 ++
 .../Library/PlatformBootManagerLib/PlatformData.c  |  179 
 5 files changed, 2304 insertions(+)
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.h
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootOption.c
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformData.c

diff --git 
a/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
 
b/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
new file mode 100644
index 0..3dffe7263
--- /dev/null
+++ 
b/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
@@ -0,0 +1,1052 @@
+/** @file
+  This file include all platform action at BDS stage which can be customized 
by IBV/OEM.
+
+  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution.  The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php.
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include "PlatformBootManager.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#pragma optimize("g", off)
+
+#define TIMEOUT_COMMAND 10
+#define BIOS_COLOR_CODING_BAR_HEIGHT  40
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_HII_HANDLE
gPlatformBdsLibStringPackHandle;
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_BOOT_MODE mBootMode;
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_GUID  *mLibTerminalType[] = {
+  ,
+  ,
+  ,
+  
+};
+
+//
+// Internal shell mode
+//
+GLOBAL_REMOVE_IF_UNREFERENCED UINT32 mShellModeColumn;
+GLOBAL_REMOVE_IF_UNREFERENCED UINT32 mShellModeRow;
+GLOBAL_REMOVE_IF_UNREFERENCED UINT32 mShellHorizontalResolution;
+GLOBAL_REMOVE_IF_UNREFERENCED UINT32 mShellVerticalResolution;
+
+CHAR16  *mConsoleVar[] = {L"ConIn", L"ConOut"};
+
+extern USB_CLASS_FORMAT_DEVICE_PATH mUsbClassKeyboardDevicePath;
+extern BOOLEAN  mAnyKeypressed;
+
+/**
+  The handle on the path we get might be not the display device.
+  We must check it.
+
+  @todo fix the parameters
+
+  @retval  TRUE PCI class type is VGA.
+  @retval  FALSEPCI class type isn't VGA.
+**/
+BOOLEAN
+IsVgaHandle (
+  IN EFI_HANDLE Handle
+)
+{
+  EFI_PCI_IO_PROTOCOL *PciIo;
+  PCI_TYPE00 Pci;
+  EFI_STATUS Status;
+
+  Status = gBS->HandleProtocol (
+  Handle,
+  ,
+  (VOID **));
+
+  if (!EFI_ERROR (Status)) {
+Status = PciIo->Pci.Read (
+  PciIo,
+  EfiPciIoWidthUint32,
+  0,
+  sizeof (Pci) / sizeof (UINT32),
+  );
+
+if (!EFI_ERROR (Status)) {
+  DEBUG ((DEBUG_ERROR, "  PCI CLASS CODE= 0x%x\n", Pci.Hdr.ClassCode 
[2]));
+  DEBUG ((DEBUG_ERROR, "  PCI SUBCLASS CODE = 0x%x\n", Pci.Hdr.ClassCode 
[1]));
+
+  if (IS_PCI_VGA () || IS_PCI_OLD_VGA ()) {
+DEBUG ((DEBUG_ERROR, "  \nPCI VGA Device Found\n"));
+return TRUE;
+  }
+}
+  }
+  return FALSE;
+}
+
+/**
+  This function converts an input device structure to a Unicode string.
+
+  @param DevPath  A pointer to the device path structure.
+
+  @return A new allocated Unicode string that represents the device path.
+**/
+CHAR16 *
+DevicePathToStr (
+  IN EFI_DEVICE_PATH_PROTOCOL *DevPath
+  )
+{
+  EFI_STATUS   Status;
+  CHAR16   *ToText;
+  EFI_DEVICE_PATH_TO_TEXT_PROTOCOL *DevPathToText;
+
+  if (DevPath == NULL) {
+return NULL;
+  }
+
+  Status = gBS->LocateProtocol (
+  ,
+  NULL,
+  (VOID **) 
+  );
+  ASSERT_EFI_ERROR (Status);
+  ToText = 

Re: [edk2] [edk2 UsbMassStorageDxe v1 1/1] MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS

[Ming Huang]

Star & Mike,

I will fix the issues mentioned below and send v2 patch later.

Thanks,
Ming


On 2018/3/21 22:22, Kinney, Michael D wrote:
> One minor comment.
>
>   #define USB_BOOT_MAX_CARRY_SIZE 0x1
>
> Should be
>
>   #define USB_BOOT_MAX_CARRY_SIZE SIZE_64KB
>
> Mike
>
>> -Original Message-
>> From: Zeng, Star
>> Sent: Wednesday, March 21, 2018 5:41 AM
>> To: Ming Huang ; Ni, Ruiyu
>> ; leif.lindh...@linaro.org; linaro-
>> u...@lists.linaro.org; edk2-devel@lists.01.org; Dong,
>> Eric 
>> Cc: ard.biesheu...@linaro.org; Kinney, Michael D
>> ; Gao, Liming
>> ; guoh...@huawei.com;
>> wanghuiqi...@huawei.com; huangmin...@huawei.com;
>> zhangjinso...@huawei.com; mengfanr...@huawei.com;
>> huangda...@hisilicon.com; wai...@126.com
>> Subject: RE: [edk2 UsbMassStorageDxe v1 1/1]
>> MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS
>>
>> Ming,
>>
>> Basically, I am ok with the change in this patch. There
>> are two comments.
>>
>> 1. Please add more background information about the
>> specific device in the commit log. For example, you
>> mentioned " like some virtual CD-ROM from BMC " in
>> previous patch.
>> 2. Please make sure building pass with the patch on
>> different tool chains. I tried to build with the patch
>> on VS2015, but met building failure like below.
>> warning C4244: '=': conversion from 'UINT32' to
>> 'UINT16', possible loss of data
>>
>>
>> Ray,
>>
>> Do you have other concern?
>>
>>
>> Thanks,
>> Star
>> -Original Message-
>> From: Ming Huang [mailto:ming.hu...@linaro.org]
>> Sent: Thursday, March 15, 2018 8:30 PM
>> To: leif.lindh...@linaro.org; linaro-
>> u...@lists.linaro.org; edk2-devel@lists.01.org; Zeng,
>> Star ; Dong, Eric
>> 
>> Cc: ard.biesheu...@linaro.org; Kinney, Michael D
>> ; Gao, Liming
>> ; guoh...@huawei.com;
>> wanghuiqi...@huawei.com; huangmin...@huawei.com;
>> zhangjinso...@huawei.com; mengfanr...@huawei.com;
>> huangda...@hisilicon.com; wai...@126.com; Ming Huang
>> 
>> Subject: [edk2 UsbMassStorageDxe v1 1/1]
>> MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS
>>
>> Booting from USB may fail while the macro
>> USB_BOOT_IO_BLOCKS set to 128 because the block size of
>> some USB devices are exceeded 512. So,the count blocks
>> to transfer should be calculated by block size of the
>> USB devices.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang 
>> ---
>>  MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c |
>> 16 
>> MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h |
>> 4 ++--
>>  2 files changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git
>> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
>> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
>> index b84bfd2d7290..b38cb6116bf4 100644
>> ---
>> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
>> +++
>> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
>> @@ -814,11 +814,13 @@ UsbBootReadBlocks (
>>USB_BOOT_READ10_CMD   ReadCmd;
>>EFI_STATUSStatus;
>>UINT16Count;
>> +  UINT16CountMax;
>>UINT32BlockSize;
>>UINT32ByteSize;
>>UINT32Timeout;
>>
>>BlockSize = UsbMass->BlockIoMedia.BlockSize;
>> +  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
>>Status= EFI_SUCCESS;
>>
>>while (TotalBlock > 0) {
>> @@ -827,7 +829,7 @@ UsbBootReadBlocks (
>>  // on the device. We must split the total block
>> because the READ10
>>  // command only has 16 bit transfer length (in the
>> unit of block).
>>  //
>> -Count = (UINT16)((TotalBlock <
>> USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
>> +Count = (UINT16)((TotalBlock < CountMax) ?
>> TotalBlock : CountMax);
>>  ByteSize  = (UINT32)Count * BlockSize;
>>
>>  //
>> @@ -890,11 +892,13 @@ UsbBootWriteBlocks (
>>USB_BOOT_WRITE10_CMD  WriteCmd;
>>EFI_STATUSStatus;
>>UINT16Count;
>> +  UINT16CountMax;
>>UINT32BlockSize;
>>UINT32ByteSize;
>>UINT32Timeout;
>>
>>BlockSize = UsbMass->BlockIoMedia.BlockSize;
>> +  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
>>Status= EFI_SUCCESS;
>>
>>while (TotalBlock > 0) {
>> @@ -903,7 +907,7 @@ UsbBootWriteBlocks (
>>  // on the device. We must split the total block
>> because the WRITE10
>>  // command only has 16 bit transfer length (in the
>> unit of block).
>>  //
>> -Count = (UINT16)((TotalBlock <
>> USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
>> +Count = (UINT16)((TotalBlock < CountMax) ?
>> TotalBlock : 

[edk2] [PATCH] MdePkg/BaseMemoryLib: Fix undefined behavior for left-shift operation

[Hao Wu]

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

Within function InternalMemSetMem(), for line:
  Value32 = (Value << 24) | (Value << 16) | (Value << 8) | Value;

Expression '(Value << 24)' is possible to bring undefined behavior.
Since 'Value' is of type UINT8 (unsigned char), it will be implicitly
promoted to type 'int' before performing the left-shift operation.

It is possible for '(Value << 24)' to exceed the range of type 'int',
which may bring undefined behavior.

This commit add explicit type cast like:
((UINT32)Value << 24)

to resolve the issue.

Cc: Steven Shi 
Cc: Michael D Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdePkg/Library/BaseMemoryLib/SetMem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Library/BaseMemoryLib/SetMem.c 
b/MdePkg/Library/BaseMemoryLib/SetMem.c
index b6fb811c38..124b48fe5d 100644
--- a/MdePkg/Library/BaseMemoryLib/SetMem.c
+++ b/MdePkg/Library/BaseMemoryLib/SetMem.c
@@ -4,7 +4,7 @@
   build for a particular platform easily if an optimized version
   is desired.
 
-  Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
   Copyright (c) 2012 - 2013, ARM Ltd. All rights reserved.
   Copyright (c) 2016, Linaro Ltd. All rights reserved.
 
@@ -54,7 +54,7 @@ InternalMemSetMem (
 
   if UINTN)Buffer & 0x7) == 0) && (Length >= 8)) {
 // Generate the 64bit value
-Value32 = (Value << 24) | (Value << 16) | (Value << 8) | Value;
+Value32 = ((UINT32)Value << 24) | (Value << 16) | (Value << 8) | Value;
 Value64 = LShiftU64 (Value32, 32) | Value32;
 
 Pointer64 = (UINT64*)Buffer;
-- 
2.12.0.windows.1

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

Re: [edk2] internal structure of EFI_TLS_CA_CERTIFICATE_VARIABLE

[Wu, Jiaxin]

Hi Laszlo,

Insert my comments as below: 

> The nested loops that parse the signature list in TlsConfigCertificate()
> currently ignore the contents of the following fields:
> - EFI_SIGNATURE_LIST.SignatureType,
> - EFI_SIGNATURE_DATA.SignatureOwner.
> 
> I'd like the generator / extractor tool to populate these fields
> correctly right from the start, so that it remain compatible with future
> features added to edk2.
> 
> 
> So, I suggest that the tool set "EFI_SIGNATURE_LIST.SignatureType" to
> EFI_CERT_X509_GUID ("This identifies a signature based on a DER-encoded
> X.509 certificate"). Because, this is what the current edk2 code assumes
> anyway -- in TlsSetCaCertificate(), we have a comment saying
> 
>   //
>   // DER-encoded binary X.509 certificate or PEM-encoded X.509
>   // certificate. Determine whether certificate is from DER encoding, if
>   // so, translate it to X509 structure.
>   //
> 
> (1) Do you agree EFI_CERT_X509_GUID is the right setting for
> "EFI_SIGNATURE_LIST.SignatureType" (even though the edk2 code currently
> ignores it)?
> 
> This would also imply that we set
> "EFI_SIGNATURE_LIST.SignatureHeaderSize" to zero, according to the UEFI
> spec.
> 

Yes, exactly, EFI_CERT_X509_GUID is the correct SignatureType for the 
CACertificate. SignatureHeaderSize should be set to zero. We do miss the check 
in HttpDxe driver, I'm fine to add back the  SignatureType check in 
TlsConfigCertificate(). So, can you report the Bugzilla for this fixing? Thanks.


> 
> Furthermore, what would you suggest for
> "EFI_SIGNATURE_DATA.SignatureOwner"? According to the spec, it is "An
> identifier which identifies the agent which added the signature to the
> list", so in theory we could just generate any GUID for the tool with
> "uuidgen". However, based on past experience, this may not be good
> enough; for example, the Secure Boot Logo Test in the Microsoft HCK
> expect the SignatureOwner field (on the Microsoft certificates) to be
> constant 77FA9ABD-0359-4D32-BD60-28F4E78F784B. In other words,
> Microsoft
> want the SignatureOwner field to stand for the organization that issued
> the certificates (i.e., themselves), not for the agent that enrolled the
> certificates.
> 
> (2) Do you foresee any such restrictions for the
> "EFI_SIGNATURE_DATA.SignatureOwner" field in
> EFI_TLS_CA_CERTIFICATE_VARIABLE? Or is it safe if we generate a GUID for
> the tool with "uuidgen"?
> 

I don't think it's necessary to restrict/stand the GUID in the field of 
SignatureOwner for the CA certification (at least for now) since it's only used 
to identify the different venders (i.e, Microsoft) so as to avoid the following 
content check. In the UEFI part, we also didn't check the SignatureOwner for 
the any security consideration. So, I think it's safe to generate a GUID using 
the tool at present.

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

Re: [edk2] [Patch 2/2] MdeModulePkg/CapsuleApp: Center bitmap at bottom of screen

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Kinney, Michael D
> Sent: Thursday, March 22, 2018 5:13 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Zeng, Star ;
> Dong, Eric 
> Subject: [Patch 2/2] MdeModulePkg/CapsuleApp: Center bitmap at bottom of
> screen
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=907
> 
> When -G option is used to convert a BMP file to a UX capsule,
> the bitmap is centered horizontally and placed in the lower
> half of the screen below the boot logo.
> 
> This matches examples shown in the following pages:
> 
> https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/user-exp
> erience-for-uefi-firmware-updates
> https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/boot-scr
> een-components
> 
> Checks are also made to make sure the bitmap provided
> fits in the current GOP mode.
> 
> Cc: Jiewen Yao 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Signed-off-by: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.1
> ---
>  MdeModulePkg/Application/CapsuleApp/CapsuleApp.c   | 63
> --
>  MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf |  3 +-
>  2 files changed, 61 insertions(+), 5 deletions(-)
> 
> diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
> b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
> index b9ff812179..e1e48befc2 100644
> --- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
> +++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -173,15 +174,21 @@ CreateBmpFmp (
>EFI_DISPLAY_CAPSULE   *DisplayCapsule;
>EFI_STATUSStatus;
>EFI_GRAPHICS_OUTPUT_PROTOCOL  *Gop;
> +  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  *Info;
> +  EFI_GRAPHICS_OUTPUT_BLT_PIXEL *GopBlt;
> +  UINTN GopBltSize;
> +  UINTN Height;
> +  UINTN Width;
> 
>Status = gBS->LocateProtocol(, NULL,
> (VOID **));
>if (EFI_ERROR(Status)) {
>  Print(L"CapsuleApp: NO GOP is found.\n");
>  return EFI_UNSUPPORTED;
>}
> +  Info = Gop->Mode->Info;
>Print(L"Current GOP: Mode - %d, ", Gop->Mode->Mode);
> -  Print(L"HorizontalResolution - %d, ",
> Gop->Mode->Info->HorizontalResolution);
> -  Print(L"VerticalResolution - %d\n", Gop->Mode->Info->VerticalResolution);
> +  Print(L"HorizontalResolution - %d, ", Info->HorizontalResolution);
> +  Print(L"VerticalResolution - %d\n", Info->VerticalResolution);
>// HorizontalResolution >= BMP_IMAGE_HEADER.PixelWidth
>// VerticalResolution   >= BMP_IMAGE_HEADER.PixelHeight
> 
> @@ -207,6 +214,35 @@ CreateBmpFmp (
>  goto Done;
>}
> 
> +  GopBlt = NULL;
> +  Status = TranslateBmpToGopBlt (
> + BmpBuffer,
> + FileSize,
> + ,
> + ,
> + ,
> + 
> + );
> +  if (EFI_ERROR(Status)) {
> +Print(L"CapsuleApp: BMP image (%s) is not valid.\n", BmpName);
> +goto Done;
> +  }
> +  if (GopBlt != NULL) {
> +FreePool (GopBlt);
> +  }
> +  Print(L"BMP image (%s), Width - %d, Height - %d\n", BmpName, Width,
> Height);
> +
> +  if (Height > Info->VerticalResolution) {
> +Status = EFI_INVALID_PARAMETER;
> +Print(L"CapsuleApp: BMP image (%s) height is larger than current
> resolution.\n", BmpName);
> +goto Done;
> +  }
> +  if (Width > Info->HorizontalResolution) {
> +Status = EFI_INVALID_PARAMETER;
> +Print(L"CapsuleApp: BMP image (%s) width is larger than current
> resolution.\n", BmpName);
> +goto Done;
> +  }
> +
>FullCapsuleBufferSize = sizeof(EFI_DISPLAY_CAPSULE) + FileSize;
>FullCapsuleBuffer = AllocatePool(FullCapsuleBufferSize);
>if (FullCapsuleBuffer == NULL) {
> @@ -226,8 +262,27 @@ CreateBmpFmp (
>DisplayCapsule->ImagePayload.ImageType = 0; // BMP
>DisplayCapsule->ImagePayload.Reserved = 0;
>DisplayCapsule->ImagePayload.Mode = Gop->Mode->Mode;
> -  DisplayCapsule->ImagePayload.OffsetX = 0;
> -  DisplayCapsule->ImagePayload.OffsetY = 0;
> +
> +  //
> +  // Center the bitmap horizontally
> +  //
> +  DisplayCapsule->ImagePayload.OffsetX =
> (UINT32)((Info->HorizontalResolution - Width) / 2);
> +
> +  //
> +  // Put bitmap 3/4 down the display.  If bitmap is too tall, then align 
> bottom
> +  // of bitmap at bottom of display.
> +  //
> +  DisplayCapsule->ImagePayload.OffsetY =
> +MIN (
> +  (UINT32)(Info->VerticalResolution - Height),
> +  (UINT32)(((3 * Info->VerticalResolution) - (2 * Height)) / 4)
> +  );
> +
> +  Print(L"BMP image (%s), OffsetX - %d, OffsetY - %d\n",
> +BmpName,
> +

Re: [edk2] [Patch] Vlv2TbltDevicePkg: Remove DxeTcg2PhysicalPresenceLibNull

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Kinney, Michael D
> Sent: Thursday, March 22, 2018 6:48 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Yao, Jiewen
> ; Guo, Mang 
> Subject: [Patch] Vlv2TbltDevicePkg: Remove DxeTcg2PhysicalPresenceLibNull
> 
> From: "Kinney, Michael D" 
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=908
> 
> The following commit that to use Tcg2 instead of TrEE breaks the
> build of Vlv2TbltDevicePkg\Library\DxeTcg2PhysicalPresenceLibNull
> 
> https://github.com/tianocore/edk2/commit/9461604e1490f73fdbcc8e957dbe7
> 5f75c73b027#diff-c85873f3649e35873a11936ace983807
> 
> The correct fix is to remove the DxeTcg2PhysicalPresenceLibNull
> library instance and update library mappings in DSC files.
> 
> Cc: Jiewen Yao 
> C: David Wei 
> Cc: Mang Guo 
> Signed-off-by: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.1
> ---
>  .../DxeTcg2PhysicalPresenceLibNull.c   | 242 
> -
>  .../DxeTcg2PhysicalPresenceLibNull.inf |  46 
>  Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc|   4 +-
>  Vlv2TbltDevicePkg/PlatformPkgIA32.dsc  |   4 +-
>  Vlv2TbltDevicePkg/PlatformPkgX64.dsc   |   4 +-
>  5 files changed, 3 insertions(+), 297 deletions(-)
>  delete mode 100644
> Vlv2TbltDevicePkg/Library/DxeTcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPre
> senceLibNull.c
>  delete mode 100644
> Vlv2TbltDevicePkg/Library/DxeTcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPre
> senceLibNull.inf
> 
> diff --git
> a/Vlv2TbltDevicePkg/Library/DxeTcg2PhysicalPresenceLibNull/DxeTcg2PhysicalP
> resenceLibNull.c
> b/Vlv2TbltDevicePkg/Library/DxeTcg2PhysicalPresenceLibNull/DxeTcg2PhysicalP
> resenceLibNull.c
> deleted file mode 100644
> index 96fad05527..00
> ---
> a/Vlv2TbltDevicePkg/Library/DxeTcg2PhysicalPresenceLibNull/DxeTcg2PhysicalP
> resenceLibNull.c
> +++ /dev/null
> @@ -1,242 +0,0 @@
> -/** @file
> -  Execute pending TPM2 requests from OS or BIOS.
> -
> -  Caution: This module requires additional review when modified.
> -  This driver will have external input - variable.
> -  This external input must be validated carefully to avoid security issue.
> -
> -  Tcg2ExecutePendingTpmRequest() will receive untrusted input and do
> validation.
> -
> -Copyright (c) 2013 - 2015, Intel Corporation. All rights reserved.
> -This program and the accompanying materials
> -are licensed and made available under the terms and conditions of the BSD
> License
> -which accompanies this distribution.  The full text of the license may be 
> found
> at
> -http://opensource.org/licenses/bsd-license.php
> -
> -THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS
> OR IMPLIED.
> -
> -**/
> -
> -#include 
> -
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -
> -/**
> -  Get string by string id from HII Interface.
> -
> -  @param[in] Id  String ID.
> -
> -  @retvalCHAR16 *String from ID.
> -  @retvalNULLIf error occurs.
> -
> -**/
> -CHAR16 *
> -Tcg2PhysicalPresenceGetStringById (
> -  IN  EFI_STRING_ID   Id
> -  )
> -{
> -  return NULL;
> -}
> -
> -/**
> -  Send ClearControl and Clear command to TPM.
> -
> -  @param[in]  PlatformAuth  platform auth value. NULL means no
> platform auth change.
> -
> -  @retval EFI_SUCCESS   Operation completed successfully.
> -  @retval EFI_TIMEOUT   The register can't run into the expected
> status in time.
> -  @retval EFI_BUFFER_TOO_SMALL  Response data buffer is too small.
> -  @retval EFI_DEVICE_ERROR  Unexpected device behavior.
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -TpmCommandClear (
> -  IN TPM2B_AUTH*PlatformAuth  OPTIONAL
> -  )
> -{
> -  return EFI_SUCCESS;
> -}
> -
> -/**
> -  Execute physical presence operation requested by the OS.
> -
> -  @param[in]  PlatformAuthplatform auth value. NULL means
> no platform auth change.
> -  @param[in]  CommandCode Physical presence operation
> value.
> -  @param[in, out] PpiFlagsThe physical presence interface flags.
> -
> -  @retval TREE_PP_OPERATION_RESPONSE_BIOS_FAILURE  Unknown
> physical presence operation.
> -  @retval TREE_PP_OPERATION_RESPONSE_BIOS_FAILURE  Error occurred
> during sending command to TPM or
> -   receiving response
> from TPM.
> -  @retval Others   Return code from
> the TPM device after command execution.
> -**/
> -UINT32
> -Tcg2ExecutePhysicalPresence (
> -  IN  TPM2B_AUTH   

Re: [edk2] [Patch 1/2] MdeModulePkg/CapsuleApp: Fix logic bug in CleanGatherList()

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Kinney, Michael D
> Sent: Thursday, March 22, 2018 5:13 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Zeng, Star ;
> Dong, Eric 
> Subject: [Patch 1/2] MdeModulePkg/CapsuleApp: Fix logic bug in
> CleanGatherList()
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=905
> 
> Fix pointer math when more than one capsule is passed
> to the CapsuleApp.  Use the ContinuationPointer from
> the last array entry instead of the first array entry.
> 
> Cc: Jiewen Yao 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Signed-off-by: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.1
> ---
>  MdeModulePkg/Application/CapsuleApp/CapsuleApp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
> b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
> index 393cfe5060..b9ff812179 100644
> --- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
> +++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
> @@ -1,7 +1,7 @@
>  /** @file
>A shell application that triggers capsule update process.
> 
> -  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
> +  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
>which accompanies this distribution.  The full text of the license may be
> found at
> @@ -653,7 +653,7 @@ CleanGatherList (
>  break;
>}
> 
> -  TempBlockPtr2 = (VOID *) ((UINTN)
> TempBlockPtr->Union.ContinuationPointer);
> +  TempBlockPtr2 = (VOID *) ((UINTN)
> TempBlockPtr[Index].Union.ContinuationPointer);
>FreePool(TempBlockPtr1);
>TempBlockPtr1 = TempBlockPtr2;
>  }
> --
> 2.14.2.windows.3

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

[edk2] [Patch V2] BaseTools/BinToPcd: Add support for multiple binary input files

[Michael D Kinney]

From: "Kinney, Michael D" 

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

There are use cases where a VOID * PCD needs to be generated from multiple
binary input files.  This can be in the form of an array of fixed size
elements or a set of variable sized elements.

Update BinToPcd to support multiple one or more -i INPUTFILE arguments.
By default, the contents of each binary input file are concatenated in
the order provided.  This supports generating a PCD that is an array of
fixed size elements

Add -x, --xdr flags to BinToPcd  to encodes the PCD using the
Variable-Length Opaque Data of RFC 4506 External Data Representation
Standard (XDR).

https://tools.ietf.org/html/rfc4506
https://tools.ietf.org/html/rfc4506#section-4.10

The data format from RFC 4506 meets the requirements for a PCD that is a
set of variable sized elements in the Variable-Length Opaque Data format.
The overhead of this format is a 32-bit length and 0 to 3 bytes of padding
to align the next element at a 32-bit boundary.

Cc: Sean Brogan 
Cc: Yonghong Zhu 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney 
---
 BaseTools/Scripts/BinToPcd.py | 83 ---
 1 file changed, 54 insertions(+), 29 deletions(-)

diff --git a/BaseTools/Scripts/BinToPcd.py b/BaseTools/Scripts/BinToPcd.py
index 68a7ac652d..f2485a27fa 100644
--- a/BaseTools/Scripts/BinToPcd.py
+++ b/BaseTools/Scripts/BinToPcd.py
@@ -1,7 +1,7 @@
 ## @file
 # Convert a binary file to a VOID* PCD value or DSC file VOID* PCD statement.
 #
-# Copyright (c) 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
 # This program and the accompanying materials
 # are licensed and made available under the terms and conditions of the BSD 
License
 # which accompanies this distribution.  The full text of the license may be 
found at
@@ -18,14 +18,15 @@ BinToPcd
 import sys
 import argparse
 import re
+import xdrlib
 
 #
 # Globals for help information
 #
 __prog__= 'BinToPcd'
-__version__ = '%s Version %s' % (__prog__, '0.9 ')
-__copyright__   = 'Copyright (c) 2016, Intel Corporation. All rights reserved.'
-__description__ = 'Convert a binary file to a VOID* PCD value or DSC file 
VOID* PCD statement.\n'
+__version__ = '%s Version %s' % (__prog__, '0.91 ')
+__copyright__   = 'Copyright (c) 2016 - 2018, Intel Corporation. All rights 
reserved.'
+__description__ = 'Convert one or more binary files to a VOID* PCD value or 
DSC file VOID* PCD statement.\n'
 
 if __name__ == '__main__':
   def ValidateUnsignedInteger (Argument):
@@ -50,21 +51,35 @@ if __name__ == '__main__':
   Message = '%s is not a valid GUID C name' % (Argument)
   raise argparse.ArgumentTypeError(Message)
 return Argument
-
-  def ByteArray (Buffer):
+
+  def ByteArray (Buffer, Xdr = False):
+if Xdr:
+  #
+  # If Xdr flag is set then encode data using the Variable-Length Opaque
+  # Data format of RFC 4506 External Data Representation Standard (XDR).
+  #
+  XdrEncoder = xdrlib.Packer()
+  for Item in Buffer:
+XdrEncoder.pack_bytes(Item)
+  Buffer = XdrEncoder.get_buffer()
+else:
+  #
+  # If Xdr flag is not set, then concatenate all the data
+  #
+  Buffer = ''.join(Buffer)
 #
-# Append byte array of values of the form '{0x01, 0x02, ...}'
+# Return a PCD value of the form '{0x01, 0x02, ...}' along with the PCD 
length in bytes
 #
-return '{%s}' % (', '.join(['0x%02x' % (ord(Item)) for Item in Buffer]))
-
+return '{%s}' % (', '.join(['0x%02x' % (ord(Item)) for Item in Buffer])), 
len (Buffer)
+
   #
   # Create command line argument parser object
   #
   parser = argparse.ArgumentParser(prog = __prog__, version = __version__,
description = __description__ + 
__copyright__,
conflict_handler = 'resolve')
-  parser.add_argument("-i", "--input", dest = 'InputFile', type = 
argparse.FileType('rb'),
-  help = "Input binary filename", required = True)
+  parser.add_argument("-i", "--input", dest = 'InputFile', type = 
argparse.FileType('rb'), action='append', required = True,
+  help = "Input binary filename.  Multiple input files are 
combined into a single PCD.")
   parser.add_argument("-o", "--output", dest = 'OutputFile', type = 
argparse.FileType('wb'),
   help = "Output filename for PCD value or PCD statement")
   parser.add_argument("-p", "--pcd", dest = 'PcdName', type = ValidatePcdName,
@@ -79,6 +94,8 @@ if __name__ == '__main__':
   help = "UEFI variable name.  Only used with --type HII.")
   parser.add_argument("-g", "--variable-guid", type = ValidateGuidName, dest = 

[edk2] [Patch] Vlv2TbltDevicePkg: Remove DxeTcg2PhysicalPresenceLibNull

[Michael D Kinney]

From: "Kinney, Michael D" 

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

The following commit that to use Tcg2 instead of TrEE breaks the
build of Vlv2TbltDevicePkg\Library\DxeTcg2PhysicalPresenceLibNull

https://github.com/tianocore/edk2/commit/9461604e1490f73fdbcc8e957dbe75f75c73b027#diff-c85873f3649e35873a11936ace983807

The correct fix is to remove the DxeTcg2PhysicalPresenceLibNull
library instance and update library mappings in DSC files.

Cc: Jiewen Yao 
C: David Wei 
Cc: Mang Guo 
Signed-off-by: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 .../DxeTcg2PhysicalPresenceLibNull.c   | 242 -
 .../DxeTcg2PhysicalPresenceLibNull.inf |  46 
 Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc|   4 +-
 Vlv2TbltDevicePkg/PlatformPkgIA32.dsc  |   4 +-
 Vlv2TbltDevicePkg/PlatformPkgX64.dsc   |   4 +-
 5 files changed, 3 insertions(+), 297 deletions(-)
 delete mode 100644 
Vlv2TbltDevicePkg/Library/DxeTcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLibNull.c
 delete mode 100644 
Vlv2TbltDevicePkg/Library/DxeTcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLibNull.inf

diff --git 
a/Vlv2TbltDevicePkg/Library/DxeTcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLibNull.c
 
b/Vlv2TbltDevicePkg/Library/DxeTcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLibNull.c
deleted file mode 100644
index 96fad05527..00
--- 
a/Vlv2TbltDevicePkg/Library/DxeTcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLibNull.c
+++ /dev/null
@@ -1,242 +0,0 @@
-/** @file
-  Execute pending TPM2 requests from OS or BIOS.
-
-  Caution: This module requires additional review when modified.
-  This driver will have external input - variable.
-  This external input must be validated carefully to avoid security issue.
-
-  Tcg2ExecutePendingTpmRequest() will receive untrusted input and do 
validation.
-
-Copyright (c) 2013 - 2015, Intel Corporation. All rights reserved.
-This program and the accompanying materials 
-are licensed and made available under the terms and conditions of the BSD 
License 
-which accompanies this distribution.  The full text of the license may be 
found at 
-http://opensource.org/licenses/bsd-license.php
-
-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, 
-WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-
-**/
-
-#include 
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-
-/**
-  Get string by string id from HII Interface.
-
-  @param[in] Id  String ID.
-
-  @retvalCHAR16 *String from ID.
-  @retvalNULLIf error occurs.
-
-**/
-CHAR16 *
-Tcg2PhysicalPresenceGetStringById (
-  IN  EFI_STRING_ID   Id
-  )
-{
-  return NULL;
-}
-
-/**
-  Send ClearControl and Clear command to TPM.
-
-  @param[in]  PlatformAuth  platform auth value. NULL means no platform 
auth change.
-
-  @retval EFI_SUCCESS   Operation completed successfully.
-  @retval EFI_TIMEOUT   The register can't run into the expected 
status in time.
-  @retval EFI_BUFFER_TOO_SMALL  Response data buffer is too small.
-  @retval EFI_DEVICE_ERROR  Unexpected device behavior.
-
-**/
-EFI_STATUS
-EFIAPI
-TpmCommandClear (
-  IN TPM2B_AUTH*PlatformAuth  OPTIONAL
-  )
-{
-  return EFI_SUCCESS;
-}
-
-/**
-  Execute physical presence operation requested by the OS.
-
-  @param[in]  PlatformAuthplatform auth value. NULL means no 
platform auth change.
-  @param[in]  CommandCode Physical presence operation value.
-  @param[in, out] PpiFlagsThe physical presence interface flags.
-  
-  @retval TREE_PP_OPERATION_RESPONSE_BIOS_FAILURE  Unknown physical presence 
operation.
-  @retval TREE_PP_OPERATION_RESPONSE_BIOS_FAILURE  Error occurred during 
sending command to TPM or 
-   receiving response from TPM.
-  @retval Others   Return code from the TPM 
device after command execution.
-**/
-UINT32
-Tcg2ExecutePhysicalPresence (
-  IN  TPM2B_AUTH   *PlatformAuth,  OPTIONAL
-  IN  UINT32   CommandCode,
-  IN OUT  EFI_TREE_PHYSICAL_PRESENCE_FLAGS *PpiFlags
-  )
-{
-  return 0;
-}
-
-
-/**
-  Read the specified key for user confirmation.
-
-  @param[in]  CautionKey  If true,  F12 is used as confirm key;
-  If false, F10 is used as confirm key.
-
-  @retval TRUEUser confirmed the changes by input.
-  @retval FALSE   User discarded the changes.
-**/
-BOOLEAN
-Tcg2ReadUserKey (
-  IN BOOLEANCautionKey
-  )
-{
-  return FALSE;
-}
-
-/**
-  The constructor function register UNI 

[edk2] [Patch 2/2] MdeModulePkg/CapsuleApp: Center bitmap at bottom of screen

[Michael D Kinney]

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

When -G option is used to convert a BMP file to a UX capsule,
the bitmap is centered horizontally and placed in the lower
half of the screen below the boot logo.

This matches examples shown in the following pages:

https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/user-experience-for-uefi-firmware-updates
https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/boot-screen-components

Checks are also made to make sure the bitmap provided
fits in the current GOP mode.

Cc: Jiewen Yao 
Cc: Star Zeng 
Cc: Eric Dong 
Signed-off-by: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 MdeModulePkg/Application/CapsuleApp/CapsuleApp.c   | 63 --
 MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf |  3 +-
 2 files changed, 61 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c 
b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
index b9ff812179..e1e48befc2 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -173,15 +174,21 @@ CreateBmpFmp (
   EFI_DISPLAY_CAPSULE   *DisplayCapsule;
   EFI_STATUSStatus;
   EFI_GRAPHICS_OUTPUT_PROTOCOL  *Gop;
+  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  *Info;
+  EFI_GRAPHICS_OUTPUT_BLT_PIXEL *GopBlt;
+  UINTN GopBltSize;
+  UINTN Height;
+  UINTN Width;
 
   Status = gBS->LocateProtocol(, NULL, (VOID 
**));
   if (EFI_ERROR(Status)) {
 Print(L"CapsuleApp: NO GOP is found.\n");
 return EFI_UNSUPPORTED;
   }
+  Info = Gop->Mode->Info;
   Print(L"Current GOP: Mode - %d, ", Gop->Mode->Mode);
-  Print(L"HorizontalResolution - %d, ", Gop->Mode->Info->HorizontalResolution);
-  Print(L"VerticalResolution - %d\n", Gop->Mode->Info->VerticalResolution);
+  Print(L"HorizontalResolution - %d, ", Info->HorizontalResolution);
+  Print(L"VerticalResolution - %d\n", Info->VerticalResolution);
   // HorizontalResolution >= BMP_IMAGE_HEADER.PixelWidth
   // VerticalResolution   >= BMP_IMAGE_HEADER.PixelHeight
 
@@ -207,6 +214,35 @@ CreateBmpFmp (
 goto Done;
   }
 
+  GopBlt = NULL;
+  Status = TranslateBmpToGopBlt (
+ BmpBuffer,
+ FileSize,
+ ,
+ ,
+ ,
+ 
+ );
+  if (EFI_ERROR(Status)) {
+Print(L"CapsuleApp: BMP image (%s) is not valid.\n", BmpName);
+goto Done;
+  }
+  if (GopBlt != NULL) {
+FreePool (GopBlt);
+  }
+  Print(L"BMP image (%s), Width - %d, Height - %d\n", BmpName, Width, Height);
+
+  if (Height > Info->VerticalResolution) {
+Status = EFI_INVALID_PARAMETER;
+Print(L"CapsuleApp: BMP image (%s) height is larger than current 
resolution.\n", BmpName);
+goto Done;
+  }
+  if (Width > Info->HorizontalResolution) {
+Status = EFI_INVALID_PARAMETER;
+Print(L"CapsuleApp: BMP image (%s) width is larger than current 
resolution.\n", BmpName);
+goto Done;
+  }
+
   FullCapsuleBufferSize = sizeof(EFI_DISPLAY_CAPSULE) + FileSize;
   FullCapsuleBuffer = AllocatePool(FullCapsuleBufferSize);
   if (FullCapsuleBuffer == NULL) {
@@ -226,8 +262,27 @@ CreateBmpFmp (
   DisplayCapsule->ImagePayload.ImageType = 0; // BMP
   DisplayCapsule->ImagePayload.Reserved = 0;
   DisplayCapsule->ImagePayload.Mode = Gop->Mode->Mode;
-  DisplayCapsule->ImagePayload.OffsetX = 0;
-  DisplayCapsule->ImagePayload.OffsetY = 0;
+
+  //
+  // Center the bitmap horizontally
+  //
+  DisplayCapsule->ImagePayload.OffsetX = (UINT32)((Info->HorizontalResolution 
- Width) / 2);
+
+  //
+  // Put bitmap 3/4 down the display.  If bitmap is too tall, then align bottom
+  // of bitmap at bottom of display.
+  //
+  DisplayCapsule->ImagePayload.OffsetY =
+MIN (
+  (UINT32)(Info->VerticalResolution - Height),
+  (UINT32)(((3 * Info->VerticalResolution) - (2 * Height)) / 4)
+  );
+
+  Print(L"BMP image (%s), OffsetX - %d, OffsetY - %d\n",
+BmpName,
+DisplayCapsule->ImagePayload.OffsetX,
+DisplayCapsule->ImagePayload.OffsetY
+);
 
   CopyMem((DisplayCapsule + 1), BmpBuffer, FileSize);
 
diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf 
b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
index b06c4ea1bc..3a67c6b909 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
@@ -4,7 +4,7 @@
 # This application can trigger capsule update process. It can also
 # generate capsule image, or dump capsule variable information.
 #
-#  Copyright (c) 2016 - 2017, Intel Corporation. All 

[edk2] [Patch 0/2] MdeModulePkg/CapsuleApp: CleanGatherList() fix and UX enhancements

[Michael D Kinney]

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

* Fix logic bug in CleanGatherList()
* Make sure BMP file is valid and fits in current GOP resolution
* Generate UX capsule with BMP centered in lower half of display

Cc: Jiewen Yao 
Cc: Star Zeng 
Cc: Eric Dong 
Signed-off-by: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1

Michael D Kinney (2):
  MdeModulePkg/CapsuleApp: Fix logic bug in CleanGatherList()
  MdeModulePkg/CapsuleApp: Center bitmap at bottom of screen

 MdeModulePkg/Application/CapsuleApp/CapsuleApp.c   | 67 --
 MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf |  3 +-
 2 files changed, 63 insertions(+), 7 deletions(-)

-- 
2.14.2.windows.3

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

[edk2] [Patch 1/2] MdeModulePkg/CapsuleApp: Fix logic bug in CleanGatherList()

[Michael D Kinney]

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

Fix pointer math when more than one capsule is passed
to the CapsuleApp.  Use the ContinuationPointer from
the last array entry instead of the first array entry.

Cc: Jiewen Yao 
Cc: Star Zeng 
Cc: Eric Dong 
Signed-off-by: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 MdeModulePkg/Application/CapsuleApp/CapsuleApp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c 
b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
index 393cfe5060..b9ff812179 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
@@ -1,7 +1,7 @@
 /** @file
   A shell application that triggers capsule update process.
 
-  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
+  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -653,7 +653,7 @@ CleanGatherList (
 break;
   }
 
-  TempBlockPtr2 = (VOID *) ((UINTN) 
TempBlockPtr->Union.ContinuationPointer);
+  TempBlockPtr2 = (VOID *) ((UINTN) 
TempBlockPtr[Index].Union.ContinuationPointer);
   FreePool(TempBlockPtr1);
   TempBlockPtr1 = TempBlockPtr2;
 }
-- 
2.14.2.windows.3

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

Re: [edk2] [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg: Allocate framebuffer using EfiRuntimeServicesData

[Ard Biesheuvel]

On 21 March 2018 at 19:07, Girish Pathak  wrote:
>
>
>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: 21 March 2018 03:38
>> To: Girish Pathak 
>> Cc: edk2-devel@lists.01.org; Leif Lindholm ;
>> Matteo Carlini ; Stephanie Hughes-Fitt
>> ; nd ; Arvind Chauhan
>> ; Daniil Egranov ;
>> Thomas Abraham 
>> Subject: Re: [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg: Allocate
>> framebuffer using EfiRuntimeServicesData
>>
>> On 21 March 2018 at 00:18, Girish Pathak  wrote:
>> > As per the UEFI specification(2.7) section 12.9, the GOP framebuffer
>> > memory can be accessed in the pre-boot and the post boot phase (by OS)
>> > Therefore the memory type EfiBootServicesData is incorrect for the
>> > framebuffer memory allocation. Change EfiBootServicesData with
>> > EfiRuntimeServicesData flag so that allocated memory can be access by
>> > the OS in the post boot phase.
>> >
>>
>> EfiRuntimeServicesData is intended for allocations that the EFI runtime
>> services need to access themselves at runtime, and will hence be virtually
>> remapped by SetVirtualAddressMap().
>>
>> This does not apply to the framebuffer. Even if it may be used at OS runtime,
>> the firmware itself will never access it, so EfiRuntimeServicesData is not
>> appropriate
>>
>> Please use EfiReservedMemory instead.
>
> Specification (UEFI Spec 2_7_A Sept 6.pdf) describes EfiReservedMemoryType as 
>  Not usable before and after ExitBootServices, See Table 28 & 29
> Hence EfiReservedMemoryType is not suitable in this case.  Agree?
>

It is not usable as ordinary memory, given that you turn it into
'special' memory (with side effects) by turning it into a framebuffer.

So EfiReservedMemory is perfectly appropriate here.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] How to build EDK2 for X64

[Richardson, Brian]

Benedikt:

Ok, so you’re trying to build DUET for IA32 & x64 architecture. The command 
line looks correct per the README: 
https://github.com/tianocore/edk2/tree/master/DuetPkg

What version of NASM are you using?

Are you building from the latest code (master edk2 branch) or UDK2017 (stable 
branch)?

Did you make any code changes?

Thanks … br
---
Brian Richardson, Senior Technical Marketing Engineer, Intel Software
brian.richard...@intel.com -- @intel_brian 
(Twitter & WeChat)
https://software.intel.com/en-us/meet-the-developers/evangelists/team/brian-richardson

From: Benedikt Pfautsch [mailto:legc...@aol.com]
Sent: Wednesday, March 21, 2018 11:21 AM
To: Richardson, Brian 
Subject: Re: How to build EDK2 for X64

Hello,
thank you for your quick respose.
IA32: https://pastebin.com/Dq3tXZ1M
X64: https://pastebin.com/eW5TSpE7

Best regards
Benedikt


-Ursprüngliche Mitteilung-
Von: Richardson, Brian 
>
An: Benedikt Pfautsch >; 
edk2-devel-owner 
>
Verschickt: Mi, 21. Mrz 2018 13:54
Betreff: RE: How to build EDK2 for X64
X64 build is well supported. You may have another error in the build config, or 
are trying to build X64 against a target that only supports IA32.

Please provide the complete command line you used for the successful and 
failing builds.

Thanks … br
---
Brian Richardson, Senior Technical Marketing Engineer, Intel Software
brian.richard...@intel.com -- @intel_brian 
(Twitter & WeChat)
https://software.intel.com/en-us/meet-the-developers/evangelists/team/brian-richardson

From: edk2-devel 
[mailto:mailman-boun...@lists.01.org] On 
Behalf Of Benedikt Pfautsch
Sent: Wednesday, March 21, 2018 3:00 AM
To: edk2-devel-ow...@lists.01.org
Subject: How to build EDK2 for X64

Hello,
how can I build edk2 for X64?
IA32 works without a problem. What can I do?
When I want to compile with -a X64 as parameter I get errors:

Building ... c:\myworkspace\IntelFrameworkModulePkg\Universal\BdsDxe\BdsDxe.inf 
[X64]
NMAKE : fatal error U1077: 'C:\Nasm\nasm.EXE' : return code '0x1'
Stop.


Build...
: error 7000: Failed to execute command
C:\Program Files (x86)\Microsoft Visual Studio 14.0\Vc\bin\nmake.exe 
/nologo tbuild 
[c:\myworkspace\Build\DuetPkgX64\DEBUG_VS2015x86\X64\UefiCpuPkg\Library\CpuExceptionHandlerLib\DxeCpuExceptionHandlerLib]
Building ... c:\myworkspace\MdeModulePkg\Universal\EbcDxe\EbcDxe.inf [X64]


Build...
: error F002: Failed to build module

c:\myworkspace\UefiCpuPkg\Library\CpuExceptionHandlerLib\DxeCpuExceptionHandlerLib.inf
 [X64, VS2015x86, DEBUG]

- Failed -
Build end time: 17:11:37, Mar.20 2018
Build total time: 00:00:50


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

Re: [edk2] [PATCH] BaseTools/PosixLike: honor pre-set PYTHONPATH

[Laszlo Ersek]

On 03/21/18 03:22, Gao, Liming wrote:
> The change is good. 
> 
> Reviewed-by: Liming Gao 

Thank you, Liming; commit b12b2c74a3fb.

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

Re: [edk2] [edk2 UsbMassStorageDxe v1 1/1] MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS

[Kinney, Michael D]

One minor comment.

  #define USB_BOOT_MAX_CARRY_SIZE 0x1

Should be

  #define USB_BOOT_MAX_CARRY_SIZE SIZE_64KB

Mike

> -Original Message-
> From: Zeng, Star
> Sent: Wednesday, March 21, 2018 5:41 AM
> To: Ming Huang ; Ni, Ruiyu
> ; leif.lindh...@linaro.org; linaro-
> u...@lists.linaro.org; edk2-devel@lists.01.org; Dong,
> Eric 
> Cc: ard.biesheu...@linaro.org; Kinney, Michael D
> ; Gao, Liming
> ; guoh...@huawei.com;
> wanghuiqi...@huawei.com; huangmin...@huawei.com;
> zhangjinso...@huawei.com; mengfanr...@huawei.com;
> huangda...@hisilicon.com; wai...@126.com
> Subject: RE: [edk2 UsbMassStorageDxe v1 1/1]
> MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS
> 
> Ming,
> 
> Basically, I am ok with the change in this patch. There
> are two comments.
> 
> 1. Please add more background information about the
> specific device in the commit log. For example, you
> mentioned " like some virtual CD-ROM from BMC " in
> previous patch.
> 2. Please make sure building pass with the patch on
> different tool chains. I tried to build with the patch
> on VS2015, but met building failure like below.
> warning C4244: '=': conversion from 'UINT32' to
> 'UINT16', possible loss of data
> 
> 
> Ray,
> 
> Do you have other concern?
> 
> 
> Thanks,
> Star
> -Original Message-
> From: Ming Huang [mailto:ming.hu...@linaro.org]
> Sent: Thursday, March 15, 2018 8:30 PM
> To: leif.lindh...@linaro.org; linaro-
> u...@lists.linaro.org; edk2-devel@lists.01.org; Zeng,
> Star ; Dong, Eric
> 
> Cc: ard.biesheu...@linaro.org; Kinney, Michael D
> ; Gao, Liming
> ; guoh...@huawei.com;
> wanghuiqi...@huawei.com; huangmin...@huawei.com;
> zhangjinso...@huawei.com; mengfanr...@huawei.com;
> huangda...@hisilicon.com; wai...@126.com; Ming Huang
> 
> Subject: [edk2 UsbMassStorageDxe v1 1/1]
> MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS
> 
> Booting from USB may fail while the macro
> USB_BOOT_IO_BLOCKS set to 128 because the block size of
> some USB devices are exceeded 512. So,the count blocks
> to transfer should be calculated by block size of the
> USB devices.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang 
> ---
>  MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c |
> 16 
> MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h |
> 4 ++--
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
> index b84bfd2d7290..b38cb6116bf4 100644
> ---
> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
> +++
> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
> @@ -814,11 +814,13 @@ UsbBootReadBlocks (
>USB_BOOT_READ10_CMD   ReadCmd;
>EFI_STATUSStatus;
>UINT16Count;
> +  UINT16CountMax;
>UINT32BlockSize;
>UINT32ByteSize;
>UINT32Timeout;
> 
>BlockSize = UsbMass->BlockIoMedia.BlockSize;
> +  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
>Status= EFI_SUCCESS;
> 
>while (TotalBlock > 0) {
> @@ -827,7 +829,7 @@ UsbBootReadBlocks (
>  // on the device. We must split the total block
> because the READ10
>  // command only has 16 bit transfer length (in the
> unit of block).
>  //
> -Count = (UINT16)((TotalBlock <
> USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
> +Count = (UINT16)((TotalBlock < CountMax) ?
> TotalBlock : CountMax);
>  ByteSize  = (UINT32)Count * BlockSize;
> 
>  //
> @@ -890,11 +892,13 @@ UsbBootWriteBlocks (
>USB_BOOT_WRITE10_CMD  WriteCmd;
>EFI_STATUSStatus;
>UINT16Count;
> +  UINT16CountMax;
>UINT32BlockSize;
>UINT32ByteSize;
>UINT32Timeout;
> 
>BlockSize = UsbMass->BlockIoMedia.BlockSize;
> +  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
>Status= EFI_SUCCESS;
> 
>while (TotalBlock > 0) {
> @@ -903,7 +907,7 @@ UsbBootWriteBlocks (
>  // on the device. We must split the total block
> because the WRITE10
>  // command only has 16 bit transfer length (in the
> unit of block).
>  //
> -Count = (UINT16)((TotalBlock <
> USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
> +Count = (UINT16)((TotalBlock < CountMax) ?
> TotalBlock : CountMax);
>  ByteSize  = (UINT32)Count * BlockSize;
> 
>  //
> @@ -966,18 +970,20 @@ UsbBootReadBlocks16 (
>UINT8 ReadCmd[16];
>EFI_STATUSStatus;
>UINT16Count;
> +  UINT16

Re: [edk2] internal structure of EFI_TLS_CA_CERTIFICATE_VARIABLE

[Laszlo Ersek]

On 03/21/18 02:30, Fu, Siyuan wrote:
> The data structure of EFI_TLS_CA_CERTIFICATE_VARIABLE is
> EFI_SIGNATURE_LIST and we have documented this in HTTPs Boot wiki
> page:
> https://github.com/tianocore/tianocore.github.io/wiki/HTTPS-Boot
> 
> You can refer section 31.4.1 "Signature Database" in UEFI 2.7 A for a
> detail description of EFI_SIGNATURE_LIST structure.

Thanks! I have two more questions.

The nested loops that parse the signature list in TlsConfigCertificate()
currently ignore the contents of the following fields:
- EFI_SIGNATURE_LIST.SignatureType,
- EFI_SIGNATURE_DATA.SignatureOwner.

I'd like the generator / extractor tool to populate these fields
correctly right from the start, so that it remain compatible with future
features added to edk2.


So, I suggest that the tool set "EFI_SIGNATURE_LIST.SignatureType" to
EFI_CERT_X509_GUID ("This identifies a signature based on a DER-encoded
X.509 certificate"). Because, this is what the current edk2 code assumes
anyway -- in TlsSetCaCertificate(), we have a comment saying

  //
  // DER-encoded binary X.509 certificate or PEM-encoded X.509
  // certificate. Determine whether certificate is from DER encoding, if
  // so, translate it to X509 structure.
  //

(1) Do you agree EFI_CERT_X509_GUID is the right setting for
"EFI_SIGNATURE_LIST.SignatureType" (even though the edk2 code currently
ignores it)?

This would also imply that we set
"EFI_SIGNATURE_LIST.SignatureHeaderSize" to zero, according to the UEFI
spec.


Furthermore, what would you suggest for
"EFI_SIGNATURE_DATA.SignatureOwner"? According to the spec, it is "An
identifier which identifies the agent which added the signature to the
list", so in theory we could just generate any GUID for the tool with
"uuidgen". However, based on past experience, this may not be good
enough; for example, the Secure Boot Logo Test in the Microsoft HCK
expect the SignatureOwner field (on the Microsoft certificates) to be
constant 77FA9ABD-0359-4D32-BD60-28F4E78F784B. In other words, Microsoft
want the SignatureOwner field to stand for the organization that issued
the certificates (i.e., themselves), not for the agent that enrolled the
certificates.

(2) Do you foresee any such restrictions for the
"EFI_SIGNATURE_DATA.SignatureOwner" field in
EFI_TLS_CA_CERTIFICATE_VARIABLE? Or is it safe if we generate a GUID for
the tool with "uuidgen"?

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

Re: [edk2] [staging/dynamictables PATCH 0/2] Dynamic Tables Framework core

[Evan Lloyd]

Hi Nathaniel.

> -Original Message-
> From: edk2-devel  On Behalf Of
> Desimone, Nathaniel L
> Sent: 20 March 2018 22:08
> To: Sami Mujawar ; edk2-devel@lists.01.org
> Cc: nd ; Stephanie Hughes-Fitt  f...@arm.com>; leif.lindh...@linaro.org
> Subject: Re: [edk2] [staging/dynamictables PATCH 0/2] Dynamic Tables
> Framework core
> 
> Please don't use the name DynamicTableManagerDxe. What does it manage?
> DynamicTables? How does adding the word "Manager" provide any useful
> information for the reader? I recommend the name "DynamicAcpiTableDxe".
[[Evan Lloyd]] 
The problem with " DynamicAcpiTableDxe" is that the table manager can be used 
for SMBIOS tables and even, should you be so inclined (perverse?) Device Tree.  
The "DynamicTableManager" name relates to the fact that the component generates 
and submits tables (of whatever sort) as opposed to the ConfigurationManager 
component which obtains, collates and provides the Configuration information.

> It makes it clear this this code is specifically for generating ACPI tables at
> runtime (as opposed to some sort of unnamed table), and it removes the
> unnecessary prose.
[[Evan Lloyd]] As noted (and implied by the name), the code is not 
"specifically for generating ACPI tables"

That being said, we'd be happy to change the names if you can suggest something 
clearer.
DynamicTablesDxe might do - the "Manager" part is, as you indicate, pretty 
redundant.
That implies changing ConfigurationManager to something like "DynamicConfig".

Would that strike you as clearer?

Regards,
Evan



> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Sami Mujawar
> Sent: Monday, March 19, 2018 8:19 AM
> To: edk2-devel@lists.01.org
> Cc: n...@arm.com; leif.lindh...@linaro.org; Stephanie.Hughes-
> f...@arm.com
> Subject: [edk2] [staging/dynamictables PATCH 0/2] Dynamic Tables
> Framework core
> 
> The Dynamic Tables Framework is a prototyped as a solution for
> automatically generating the firmware tables based on hardware
> description.
> 
> This patchset is the Dynamic Tables Framework core and implement the
> generic/standard modules for dynamically generating ACPI 6.2 tables for
> ARM platform. The platform specific modules are in the devel-
> dynamictables branch in the edk2-platforms repository at:
> https://github.com/tianocore/edk2-platforms/tree/devel-dynamictables
> 
> The first patch in this patchset 'MdePkg: SMMUv3 updates for IORT'
> is a precursor for the Dynamic Tables Framework and has been submitted
> independently to the edk2-devel mailing list where it is currently awaiting
> acceptance.
> 
> The sources for this patchset can be seen at:
> https://github.com/samimujawar/edk2-
> staging/tree/187_dynamictables_v1
> 
> Sami Mujawar (2):
>   MdePkg: SMMUv3 updates for IORT table definitions
>   DynamicTablesPkg: Dynamic Tables Framework
> 
> 
> DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/AcpiTableFactory/Acpi
> TableFactory.c |  226 +++
> 
> DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DeviceTreeTableFactor
> y/DeviceTreeTableFactory.c |  225 +++
> 
> DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactory.
> h   |  125 ++
> 
> DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactory
> Dxe.c|   84 +
> 
> DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactory
> Dxe.inf  |   59 +
> 
> DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/SmbiosTableFactory/S
> mbiosTableFactory.c |  226 +++
> 
> DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManag
> erDxe.c|  533 +
> 
> DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManag
> erDxe.inf  |   48 +
>  DynamicTablesPkg/DynamicTables.dsc.inc
> |   46 +
>  DynamicTablesPkg/DynamicTables.fdf.inc
> |   35 +
>  DynamicTablesPkg/DynamicTablesPkg.dec
> |   42 +
>  DynamicTablesPkg/Include/AcpiTableGenerator.h
> |  282 +++
>  DynamicTablesPkg/Include/ArmNameSpaceObjects.h
> |  587 ++
>  DynamicTablesPkg/Include/ConfigurationManagerHelper.h
> |  119 ++
>  DynamicTablesPkg/Include/ConfigurationManagerObject.h
> |  176 ++
>  DynamicTablesPkg/Include/DeviceTreeTableGenerator.h
> |  182 ++
>  DynamicTablesPkg/Include/Library/TableHelperLib.h
> |   70 +
>  DynamicTablesPkg/Include/Protocol/ConfigurationManagerProtocol.h
> |  128 ++
>  DynamicTablesPkg/Include/Protocol/DynamicTableFactoryProtocol.h
> |  140 ++
>  DynamicTablesPkg/Include/SmbiosTableGenerator.h
> |  240 +++
>  DynamicTablesPkg/Include/StandardNameSpaceObjects.h
> |  116 ++
>  DynamicTablesPkg/Include/TableGenerator.h
> |  252 +++
> 
> DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/AcpiDbg2LibArm.inf
> |   47 +
>  DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
> |  440 +
>  

Re: [edk2] [PATCH] ArmPkg/Gic: force GIC driver to run before CPU arch protocol driver

[Shannon Zhao]

Hi Ard,

On 2018/2/6 20:04, Ard Biesheuvel wrote:
> Currently, the GIC driver has a static dependency on the CPU arch protocol
> driver, so it can register its IRQ handler at init time. This means there
> is a window between dispatch of the CPU driver and dispatch of the GIC
> driver where any unexpected GIC state may trigger an interrupt which we
> are not set up to handle yet. Note that this is even the case if we enter
> UEFI with interrupts disabled at the CPU, given that any TPL manipulation
> involving TPL_HIGH_LEVEL will unconditionally enable IRQs at the CPU side
> regardless of whether they were enabled to begin with (but only as soon as
> the CPU arch protocol is actually installed)
> 
> So let's reorder the GIC driver with the CPU driver, and let it run its
> initialization that puts the GIC into a known state before enabling
> interrupts. Move its installation of its IRQ handler to a protocol notify
> callback on the CPU arch protocol so that it runs as soon as it becomes
> available.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
> 
> This fixes an issue observed with GICv3 guests running under KVM.

I'm backporting this patch to the branch UDK2017 with three extra patches.

3ba3528 ArmPkg/Gic: force GIC driver to run before CPU arch protocol driver
c397d52 ArmPkg/ArmGicDxe: Expose HardwareInterrupt2 protocol
9a73ffd EmbeddedPkg: Introduce HardwareInterrupt2 protocol
29d33ba ArmPkg: Tidy GIC code before changes.

But when I start a VM using the new edk2 binary, it throws below error.

ASSERT_EFI_ERROR (Status = Device Error)
ASSERT [Shell]
/root/rpmbuild/BUILD/edk2-2.7.0/Build/ArmVirtQemu-AARCH64/RELEASE_GCC49/AARCH64/ShellPkg/Application/Shell/Shell/DEBUG/AutoGen.c(885):
!EFI_ERROR (Status)


Synchronous Exception at 0x00023866A1F0

  X0 0x00FF   X1 0x000A   X2 0x00AB
  X3 0x00023F2A2B80
  X4 0x0001   X5 0x0001   X6 0x
  X7 0x0002
  X8 0x   X9 0x0007  X10 0x000238B6
 X11 0x00023AD86FFF
 X12 0x  X13 0x0001  X14 0x
 X15 0x
 X16 0x  X17 0x00672E50  X18 0x00672E50
 X19 0x00023B2ECE98
 X20 0x00023BFF0018  X21 0x8007  X22 0x00023F2CB088
 X23 0x
 X24 0x00023B2ED438  X25 0x00023B2ED440  X26 0x00023F2CA3D8
 X27 0x0095
 X28 0x4007E010   FP 0x   LR 0x00023861E844

  V0 0x    V1 0x63702F66

  V2 0x7363732F312C3140 6567646972622D69   V3 0x

  V4 0x0010    V5 0x4010040140100401
4010040140100401
  V6 0x0010 0010   V7 0x

  V8 0x    V9 0x

 V10 0x   V11 0x

 V12 0x   V13 0x

 V14 0x   V15 0x

 V16 0x   V17 0x

 V18 0x   V19 0x

 V20 0x   V21 0x

 V22 0x   V23 0x

 V24 0x   V25 0x

 V26 0x   V27 0x

 V28 0x   V29 0x

 V30 0x   V31 0x


  SP 0x00023F2A2B70  ELR 0x00023866A1F0  SPSR 0x6305  FPSR
0x
 ESR 0x5600DBDB  FAR 0x1DE7EC7EDBADC0DE

 ESR : EC 0x15  IL 0x1  ISS 0xDBDB

 SVC executed in AArch64

Stack dump:
  23F2A2A70: 00023861E820 00023BFF0018 00023F2A2B70
00023F2A2B70
  23F2A2A90: 00023F2A2B40 FF80FFD8 00023F2A2B70
00023F2A2B70
  23F2A2AB0: 00023F2A2B40 FF80FFD8 

  23F2A2AD0:  63702F66 6567646972622D69
7363732F312C3140
  23F2A2AF0:   
0010
  23F2A2B10: 4010040140100401 4010040140100401 0010
0010
  23F2A2B30: 00023F2A2C2A 00023866A74C 00023B2ECE98
00023BFF0018
  23F2A2B50: 8007 00023F2CB088 
00023861E834
> 23F2A2B70: 00023860B4A4 0010 5B20545245535341
2F205D6C6C656853
  23F2A2B90: 6D70722F746F6F72 55422F646C697562 326B64652F444C49
422F302E372E322D
  23F2A2BB0: 

Re: [edk2] [PATCH edk2-platforms v3 00/17] Update GOP

[Evan Lloyd]

To minimise the spam involved, I'm hoping the maintainers will accept this as 
covering all the patches listed.  (Please?)

Reviewed-by: Evan Lloyd 


> -Original Message-
> From: edk2-devel  On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:18
> To: edk2-devel@lists.01.org
> Cc: nd ; ard.biesheu...@linaro.org;
> leif.lindh...@linaro.org; Stephanie Hughes-Fitt  f...@arm.com>; Arvind Chauhan 
> Subject: [edk2] [PATCH edk2-platforms v3 00/17] Update GOP
> 
> This patch series addresses comments on the patch v2
> (https://lists.01.org/pipermail/edk2-devel/2017-December/019406.html)
> reworking of the Graphics Output Protocol code in ArmPlatformPkg.
> It also contains updates for the new SCMI protocol (MTL Library).
> 
> Code is available for examination at:
>   https://github.com/girishpathak/edk2-platforms/tree/201_gop_v3
> 
> 
> Ard Biesheuvel (1):
>   ARM/VExpressPkg: Fix MODULE_TYPE of HDLCD/PL111 platform libraries
> 
> EvanLloyd (2):
>   ARM/VExpressPkg: HdLcdArmVExpressLib: Remove redundant Bpp
>   ARM/VExpressPkg: Redefine LcdPlatformGetTimings function
> 
> Girish Pathak (14):
>   ARM/VExpressPkg: Tidy HDLCD and PL11LCD platform Lib: Coding
> standard
>   ARM/VExpressPkg: Tidy HdLcd/PL111Lcd code: Updated comments
>   ARM/VExpressPkg: Remove unused PcdPL111LcdMaxMode from HDLCD
> inf
>   ARM/VExpressPkg: Add and update debug ASSERTS
>   ARM/VExpressPkg: PL111Lcd/HdLcd plaform libs: Minor code cleanup
>   ARM/VExpressPkg: PL111 and HDLCD: Use FixedPcdGet32
>   ARM/VExpressPkg: HdLcdArmVExpressLib: Remove status check
> EFI_TIMEOUT
>   ARM/VExpressPkg: PL111 and HDLCD: Add PCD to select pixel format
>   ARM/VExpressPkg: Allocate framebuffer using EfiRuntimeServicesData
>   ARM/VExpressPkg: Reserving framebuffer at build
>   ARM/VExpressPkg: Set EFI_MEMORY_XP flag on GOP framebuffer
>   ARM/VExpressPkg: New DP500/DP550/DP650 platform library
>   ARM/JunoPkg: Adding SCMI MTL library
>   ARM/JunoPkg: Add HDLCD platform library
> 
>  Platform/ARM/JunoPkg/ArmJuno.dec 
>   |  17 +-
>  Platform/ARM/JunoPkg/ArmJuno.dsc 
>   |  31 +-
>  Platform/ARM/JunoPkg/ArmJuno.fdf 
>   |  12 +-
>  Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
> |   5 +-
>  Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
> |  29 +-
>  Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.c
> | 198 +++
>  Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.inf
> |  39 ++
>  Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlPrivateLib.h
> |  94 
>  Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJuno.c
> | 555 
>  Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJunoLib.inf
> |  41 ++
>  Platform/ARM/VExpressPkg/ArmVExpressPkg.dec  
>   |
> 3 +-
>  Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c
> | 387 ++
>  Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf
> |  43 ++
> 
> Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib
> .inf |   7 +-
>  Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> |  53 +-
> 
> Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExp
> ress.c| 310 +++
> 
> Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExp
> ressLib.inf   |  14 +-
> 
> Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdAr
> mVExpress.c  | 389 +-
> 
> Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdAr
> mVExpressLib.inf |  10 +-
>  19 files changed, 1945 insertions(+), 292 deletions(-)  create mode 100644
> Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.c
>  create mode 100644
> Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlLib.inf
>  create mode 100644
> Platform/ARM/JunoPkg/Library/ArmJunoMtlLib/ArmJunoMtlPrivateLib.h
>  create mode 100644
> Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJuno.c
>  create mode 100644
> Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJunoLib.inf
>  create mode 100644
> Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c
>  create mode 100644
> Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf
> 
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
> ___
> 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 v3 16/16] ArmPkg: Introduce SCMI protocol

[Evan Lloyd]

Reviewed-by: Evan Lloyd 

> -Original Message-
> From: edk2-devel  On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:12
> To: edk2-devel@lists.01.org
> Cc: nd ; Stephanie Hughes-Fitt  f...@arm.com>; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v3 16/16] ArmPkg: Introduce SCMI protocol
> 
> This change introduces a new SCMI protocol driver for
> Arm systems. The driver currently supports only clock
> and performance management protocols. Other protocols
> will be added as and when needed.
> 
> Clock management protocol is used to configure various clocks
> available on the platform e.g. HDLCD clock on the Juno platforms.
> 
> Whereas performance management protocol allows adjustment
> of various performance domains. Currently this is used to evaluate
> performance of the Juno platform.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak 
> Signed-off-by: Evan Lloyd 
> ---
> 
> Notes:
> v3:
> - Please rename ArmMtl.h to ArmMtlLibi.h, and
>   declare it as a library class in the package file.  [Ard]
> 
>   Done, however ArmMtlLib.h is now part of earlier commit [Girish]
> 
> - Move ArmScmiDxe to ArmPkg from ArmPlatformPkg   [Ard]
> 
>   Done[Girish]
> 
> - Declare gArmScmiBaseProtocolGuid and similar
>   protocols Guids in ArmPkg.dec   [Ard]
> 
>   Done[Girish]
> 
> - Replace flexible array member [] with [1]   [Ard]
> 
>   Done[Girish]
> 
> - Move protocol init function which are not part of
>   of protocol like  ScmiBaseProtocolInit elsewhere[Ard]
> 
>   Done[Girish]
> 
> - Please don't put stuff in Include/Drivers.  [Ard]
> 
>   Moved headers to Include/Protocol.  [Girish]
> 
>  ArmPkg/ArmPkg.dec |  13 +
>  ArmPkg/ArmPkg.dsc |   6 +-
>  ArmPkg/Drivers/ArmScmiDxe/ArmScmiBaseProtocolPrivate.h|  46 ++
>  ArmPkg/Drivers/ArmScmiDxe/ArmScmiClockProtocolPrivate.h   |  84
> 
>  ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf  |  53 +++
>  ArmPkg/Drivers/ArmScmiDxe/ArmScmiPerformanceProtocolPrivate.h |  55
> +++
>  ArmPkg/Drivers/ArmScmiDxe/Scmi.c  | 262 
> +++
>  ArmPkg/Drivers/ArmScmiDxe/ScmiBaseProtocol.c  | 318
> ++
>  ArmPkg/Drivers/ArmScmiDxe/ScmiClockProtocol.c | 418
> ++
>  ArmPkg/Drivers/ArmScmiDxe/ScmiDxe.c   | 138 ++
>  ArmPkg/Drivers/ArmScmiDxe/ScmiDxe.h   |  41 ++
>  ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c   | 457
> 
>  ArmPkg/Drivers/ArmScmiDxe/ScmiPrivate.h   | 174 
>  ArmPkg/Include/Protocol/ArmScmi.h |  27 ++
>  ArmPkg/Include/Protocol/ArmScmiBaseProtocol.h | 174
> 
>  ArmPkg/Include/Protocol/ArmScmiClockProtocol.h| 218
> ++
>  ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h  | 265
> 
>  17 files changed, 2748 insertions(+), 1 deletion(-)
> 
> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> index
> 881751d81c6384a3eb0b4c180c76d01a58266a74..16f7e40046429142b44
> b526043b61a3d5e089d2c 100644
> --- a/ArmPkg/ArmPkg.dec
> +++ b/ArmPkg/ArmPkg.dec
> @@ -51,6 +51,19 @@ [Guids.common]
> 
>gArmGicDxeFileGuid = { 0xde371f7c, 0xdec4, 0x4d21, { 0xad, 0xf1, 0x59,
> 0x3a, 0xbc, 0xc1, 0x58, 0x82 } }
> 
> +[Protocols.common]
> +  ## Arm System Control and Management Interface(SCMI) Base protocol
> +  ## ArmPkg/Include/Protocol/ArmScmiBaseProtocol.h
> +  gArmScmiBaseProtocolGuid = { 0xd7e5abe9, 0x33ab, 0x418e, { 0x9f,
> 0x91, 0x72, 0xda, 0xe2, 0xba, 0x8e, 0x2f } }
> +
> +  ## Arm System Control and Management Interface(SCMI) Clock
> management protocol
> +  ## ArmPkg/Include/Protocol/ArmScmiClockProtocol.h
> +  gArmScmiClockProtocolGuid = { 0x91ce67a8, 0xe0aa, 0x4012, { 0xb9,
> 0x9f, 0xb6, 0xfc, 0xf3, 0x4, 0x8e, 0xaa } }
> +
> +  ## Arm System Control and Management Interface(SCMI) Clock
> management protocol
> +  ## ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
> +  gArmScmiPerformanceProtocolGuid = { 0x9b8ba84, 0x3dd3, 0x49a6,
> { 0xa0, 0x5a, 0x31, 0x34, 0xa5, 0xf0, 0x7b, 0xad } }
> +
>  [Ppis]
>## Include/Ppi/ArmMpCoreInfo.h
>gArmMpCoreInfoPpiGuid = { 0x6847cc74, 0xe9ec, 0x4f8f, {0xa2, 0x9d,
> 0xab, 0x44, 0xe7, 0x54, 0xa8, 0xfc} }
> diff --git a/ArmPkg/ArmPkg.dsc 

Re: [edk2] [PATCH v3 15/16] ArmPkg: MTL Library interface and Null library implementation

[Evan Lloyd]

Reviewed-by: Evan Lloyd 

> -Original Message-
> From: edk2-devel  On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:12
> To: edk2-devel@lists.01.org
> Cc: nd ; Stephanie Hughes-Fitt  f...@arm.com>; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v3 15/16] ArmPkg: MTL Library interface and Null
> library implementation
> 
> Upcoming new component ArmPkg/Drivers/ArmScmiDxe is dependent on
> platform specific ArmMtlLib library implementation, however in order to be
> able to build the ArmScmiDxe component outside of the context of a
> particular platform, this change adds Null implementation of the ArmMtlLib
> along with ARM MTL library header.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak 
> Signed-off-by: Evan Lloyd 
> ---
>  ArmPkg/ArmPkg.dec  |   3 +-
>  ArmPkg/Include/Library/ArmMtlLib.h | 137
> 
>  ArmPkg/Library/ArmMtlNullLib/ArmMtlNullLib.c   | 108
> +++
>  ArmPkg/Library/ArmMtlNullLib/ArmMtlNullLib.inf |  26 
>  4 files changed, 273 insertions(+), 1 deletion(-)
> 
> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec index
> a55b6268ff85ffd7da140be813ec875f7f242c4d..881751d81c6384a3eb0b4c
> 180c76d01a58266a74 100644
> --- a/ArmPkg/ArmPkg.dec
> +++ b/ArmPkg/ArmPkg.dec
> @@ -2,7 +2,7 @@
>  # ARM processor package.
>  #
>  # Copyright (c) 2009 - 2010, Apple Inc. All rights reserved. -#
> Copyright (c) 2011 - 2017, ARM Limited. All rights reserved.
> +# Copyright (c) 2011 - 2018, ARM Limited. All rights reserved.
>  #
>  #This program and the accompanying materials
>  #are licensed and made available under the terms and conditions of the
> BSD License
> @@ -40,6 +40,7 @@ [LibraryClasses.common]
>ArmDisassemblerLib|Include/Library/ArmDisassemblerLib.h
>ArmGicArchLib|Include/Library/ArmGicArchLib.h
>ArmSvcLib|Include/Library/ArmSvcLib.h
> +  ArmMtlLib|ArmPlatformPkg/Include/Library/ArmMtlLib.h
> 
>  [Guids.common]
>gArmTokenSpaceGuid   = { 0xBB11ECFE, 0x820F, 0x4968, { 0xBB, 0xA6,
> 0xF7, 0x6A, 0xFE, 0x30, 0x25, 0x96 } }
> diff --git a/ArmPkg/Include/Library/ArmMtlLib.h
> b/ArmPkg/Include/Library/ArmMtlLib.h
> new file mode 100644
> index
> ..4218a741e5ebddd080
> 22b94354d5ef47576cd3b8
> --- /dev/null
> +++ b/ArmPkg/Include/Library/ArmMtlLib.h
> @@ -0,0 +1,137 @@
> +/** @file
> +
> +  Copyright (c) 2017-2018, Arm Limited. All rights reserved.
> +
> +  This program and the accompanying materials  are licensed and made
> + available under the terms and conditions of the BSD License  which
> + accompanies this distribution.  The full text of the license may be
> + found at  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> +
> +  System Control and Management Interface V1.0
> +http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
> +DEN0056A_System_Control_and_Management_Interface.pdf
> +**/
> +
> +#ifndef ARM_MTL_LIB_H_
> +#define ARM_MTL_LIB_H_
> +
> +#include 
> +
> +// Ideally we don't need packed struct. However we can't rely on compilers.
> +#pragma pack(1)
> +
> +typedef struct {
> +  UINT32 Reserved1;
> +  UINT32 ChannelStatus;
> +  UINT64 Reserved2;
> +  UINT32 Flags;
> +  UINT32 Length;
> +  UINT32 MessageHeader;
> +
> +  // NOTE: Since EDK2 does not allow flexible array member [] we
> +declare
> +  // here array of 1 element length. However below is used as a
> +variable
> +  // length array.
> +  UINT32 Payload[1];// size less object gives offset to payload.
> +} MTL_MAILBOX;
> +
> +#pragma pack()
> +
> +// Channel Type, Low-priority, and High-priority typedef enum {
> +  MTL_CHANNEL_TYPE_LOW = 0,
> +  MTL_CHANNEL_TYPE_HIGH = 1
> +} MTL_CHANNEL_TYPE;
> +
> +typedef struct {
> +  UINT64 PhysicalAddress;
> +  UINT32 ModifyMask;
> +  UINT32 PreserveMask;
> +} MTL_DOORBELL;
> +
> +typedef struct {
> +  MTL_CHANNEL_TYPE ChannelType;
> +  MTL_MAILBOX  * CONST MailBox;
> +  MTL_DOORBELL DoorBell;
> +} MTL_CHANNEL;
> +
> +/** Wait until channel is free.
> +
> +  @param[in] ChannelPointer to a channel.
> +  @param[in] TimeOutInMicroSeconds  Time out in micro seconds.
> +
> +  @retval EFI_SUCCESS   Channel is free.
> +  @retval EFI_TIMEOUT   Time out error.
> +**/
> +EFI_STATUS
> +MtlWaitUntilChannelFree (
> +  IN MTL_CHANNEL  *Channel,
> +  IN UINT64   TimeOutInMicroSeconds
> +  );
> +
> +/** Return the address of the message payload.
> +
> +  @param[in] Channel   Pointer to a channel.
> +
> +  @retval UINT32*  Pointer to the payload.
> +**/
> +UINT32*
> +MtlGetChannelPayload (
> +  IN MTL_CHANNEL  *Channel
> +  );
> +
> +/** Return 

Re: [edk2] [PATCH v3 03/16] ArmPlatformPkg: Tidy Lcd code: Coding standard

[Evan Lloyd]

Reviewed-by: Evan Lloyd 

> -Original Message-
> From: edk2-devel  On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:12
> To: edk2-devel@lists.01.org
> Cc: nd ; Stephanie Hughes-Fitt  f...@arm.com>; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v3 03/16] ArmPlatformPkg: Tidy Lcd code: Coding
> standard
> 
> From: Girish Pathak 
> 
> There is no functional modification in this change As preparation for further
> work, the formatting is corrected to meet the EDKII coding standard.
> Of specific note, some invalid include guards were fixed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak 
> Signed-off-by: Evan Lloyd 
> ---
> 
> Notes:
> v3:
> - Minor coding style changes  [Ard]
> 
>   Done[Girish]
> 
> - Changing one style to the other is just pointless churn [Ard]
> 
>   Fixing the include guards: is a small improvement.
>   (Ideally patchcheck should reject these.)
>   Reducing lines to 80 columns: makes Leif (at least)
>   happy, and aligns with formatter behaviour. Correcting
>   Doxygen format comments: prevents Doxygen generating
>   gibberish. Spaces before '(': Maintains consistency,
>   and aligns  with desired formatter behaviour.   [Evan]
> 
>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.c
> | 187 +++-
> ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.h |
> 10 +-
>  ArmPlatformPkg/Include/Library/LcdPlatformLib.h|  14 +-
>  ArmPlatformPkg/Library/HdLcd/HdLcd.c   |  88 
> +
>  ArmPlatformPkg/Library/HdLcd/HdLcd.h   |  21 ++-
>  ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c |  64 
> ---
>  6 files changed, 208 insertions(+), 176 deletions(-)
> 
> diff --git
> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> c
> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> c
> index
> b721061fc1df5695092e8c71da97ae0b9af46b3f..905eb26ee01b5037dfbaf3
> c054a62593837c8b5f 100644
> ---
> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> c
> +++
> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> c
> @@ -1,6 +1,6 @@
>  /** @file
> 
> - Copyright (c) 2011-2014, ARM Ltd. All rights reserved.
> + Copyright (c) 2011-2018, ARM Ltd. All rights reserved.
>   This program and the accompanying materials
>   are licensed and made available under the terms and conditions of the BSD
> License
>   which accompanies this distribution.  The full text of the license may be
> found at @@ -9,7 +9,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.
> 
> - **/
> +**/
> 
>  #include 
>  #include 
> @@ -22,12 +22,10 @@
> 
>  #include "LcdGraphicsOutputDxe.h"
> 
> -
> /**
> 
> - *
> - *  This file implements the Graphics Output protocol on
> ArmVersatileExpress
> - *  using the Lcd controller
> - *
> -
> **
> /
> +/** This file implements the Graphics Output protocol on
> +ArmVersatileExpress
> +  using the Lcd controller
> +
> +**/
> 
>  //
>  // Global variables
> @@ -64,7 +62,10 @@ LCD_INSTANCE mLcdTemplate = {
>  {
>{
>  HARDWARE_DEVICE_PATH, HW_VENDOR_DP,
> -{ (UINT8) (sizeof(VENDOR_DEVICE_PATH)), (UINT8)
> ((sizeof(VENDOR_DEVICE_PATH)) >> 8) },
> +{
> +  (UINT8)(sizeof (VENDOR_DEVICE_PATH)),
> +  (UINT8)((sizeof (VENDOR_DEVICE_PATH)) >> 8)
> +},
>},
>// Hardware Device Path for Lcd
>EFI_CALLER_ID_GUID // Use the driver's GUID @@ -73,10 +74,13 @@
> LCD_INSTANCE mLcdTemplate = {
>  {
>END_DEVICE_PATH_TYPE,
>END_ENTIRE_DEVICE_PATH_SUBTYPE,
> -  { sizeof(EFI_DEVICE_PATH_PROTOCOL), 0 }
> +  {
> +sizeof (EFI_DEVICE_PATH_PROTOCOL),
> +0
> +  }
>  }
>},
> -  (EFI_EVENT) NULL // ExitBootServicesEvent
> +  (EFI_EVENT)NULL // ExitBootServicesEvent
>  };
> 
>  EFI_STATUS
> @@ -86,7 +90,7 @@ LcdInstanceContructor (  {
>LCD_INSTANCE* Instance;
> 
> -  Instance = AllocateCopyPool (sizeof(LCD_INSTANCE), );
> +  Instance = AllocateCopyPool (sizeof (LCD_INSTANCE), );
>if (Instance == NULL) {
>  return EFI_OUT_OF_RESOURCES;
>}
> @@ -113,23 +117,23 @@ InitializeDisplay (
>UINTN  VramSize;
> 
>Status = LcdPlatformGetVram (, );
> -  if (EFI_ERROR(Status)) {
> +  if (EFI_ERROR (Status)) {
>  return Status;
>}
> 
>// Setup the LCD
>Status = LcdInitialize 

Re: [edk2] [PATCH v3 13/16] ArmPlatformPkg: Reserving framebuffer at build

[Evan Lloyd]

Reviewed-by: Evan Lloyd 

> -Original Message-
> From: edk2-devel  On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:12
> To: edk2-devel@lists.01.org
> Cc: nd ; Stephanie Hughes-Fitt  f...@arm.com>; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v3 13/16] ArmPlatformPkg: Reserving framebuffer
> at build
> 
> From: Girish Pathak 
> 
> Currently framebuffer memory is either reserved in special VRAM or
> dynamically allocated using boot services memory allocation functions.
> When allocated using boot services calls the memory has to be allocated as
> EfiBootServicesData. Unfortunately failures have been seen with this case.
> There is also an unfortunate lack of control on the placement of the
> framebuffer.
> 
> This change introduces two PCDs, PcdArmLcdFrameBufferBase and
> PcdArmLcdFrameBufferSize which enable build time reservation of the
> framebuffer, avoiding the need to allocate dynamically. This allows the
> framebuffer to appear as "I/O memory" outside of the normal RAM map,
> which is similar to the "VRAM" case.
> 
> This change has no impact on current code, only enables the option of build
> time reservation of framebuffers.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak 
> Signed-off-by: Evan Lloyd 
> ---
>  ArmPlatformPkg/ArmPlatformPkg.dec | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec
> b/ArmPlatformPkg/ArmPlatformPkg.dec
> index
> 5231ea822f05c2f281a6190d6eae0fc7d0bc0cb3..5c702718a78a78e6c67ab
> 407e198382e0a0df4be 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> @@ -93,6 +93,11 @@ [PcdsFixedAtBuild.common]
> 
> gArmPlatformTokenSpaceGuid.PcdPL111LcdBase|0x0|UINT32|0x002
> 6
> 
> gArmPlatformTokenSpaceGuid.PcdArmHdLcdBase|0x0|UINT32|0x002
> 7
> 
> +  ## Default size for display modes upto 1920x1080 (1920 * 1080 * 4
> + Bytes Per Pixel)
> +
> +
> gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferSize|0x7E9000|
> UINT32
> + |0x0043  ## If set, framebuffer memory will be reserved and
> mapped
> + in the system RAM
> +
> +
> gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferBase|0x0|UINT6
> 4|0x00
> + 44
> +
>## PL180 MCI
> 
> gArmPlatformTokenSpaceGuid.PcdPL180SysMciRegAddress|0x|
> UINT32|0x0028
> 
> gArmPlatformTokenSpaceGuid.PcdPL180MciBaseAddress|0x|UI
> NT32|0x0029
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
> ___
> 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 v3 14/16] ArmPlatformPkg: New DP500/DP550/DP650 GOP driver

[Evan Lloyd]

Reviewed-by: Evan Lloyd 

> -Original Message-
> From: edk2-devel  On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:12
> To: edk2-devel@lists.01.org
> Cc: nd ; Stephanie Hughes-Fitt  f...@arm.com>; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v3 14/16] ArmPlatformPkg: New
> DP500/DP550/DP650 GOP driver
> 
> From: Girish Pathak 
> 
> This change adds support for the ARM Mali DP500/DP500/DP650 display
> processors using the GOP protocol. It has been tested on FVP base models
> + DP550 support. This change adds platform independant LcdHwLib library.
> A corresponding platform specific library will be submitted to edk-
> platforms/Platform/ARM/VExpressPkg.
> 
> This change does not modify functionality provided by PL111 or HDLCD.
> This LcdHwLib implementation should be suitable for those platforms that
> implement ARM Mali DP500/DP550/DP650 replacing PL111/HDLCD.
> 
> Only graphics layer of the ARM Mali DP is configured for rendering the
> RGB/BGR format frame buffer to satisfy the UEFI GOP requirements Other
> layers e.g. video layers are not configured.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak 
> Signed-off-by: Evan Lloyd 
> ---
> 
> Notes:
> v3:
> - Please add this library as a component to
>   ArmPlatformPkg.dsc as well, so we can do build testing
>   on it.   [Ard]
> 
>   Done [Girish]
> 
> - Please drop references to edk2-platforms.
>   This driver should be able to be used independently
>   from VExpress platform code  [Ard]
> 
>   Done [Girish]
> 
>  ArmPlatformPkg/ArmPlatformPkg.dec  |   4 +
>  ArmPlatformPkg/ArmPlatformPkg.dsc  |   4 +-
>  ArmPlatformPkg/Library/ArmMaliDp/ArmMaliDp.c   | 409
> 
>  ArmPlatformPkg/Library/ArmMaliDp/ArmMaliDp.h   | 243 
>  ArmPlatformPkg/Library/ArmMaliDp/ArmMaliDp.inf |  43 ++
>  5 files changed, 702 insertions(+), 1 deletion(-)
> 
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec
> b/ArmPlatformPkg/ArmPlatformPkg.dec
> index
> 5c702718a78a78e6c67ab407e198382e0a0df4be..28cdc259849da11b172a
> d52045bccc0276669852 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> @@ -98,6 +98,10 @@ [PcdsFixedAtBuild.common]
>## If set, framebuffer memory will be reserved and mapped in the system
> RAM
> 
> gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferBase|0x0|UINT6
> 4|0x0044
> 
> +  ## ARM Mali Display Processor DP500/DP550/DP650
> +
> gArmPlatformTokenSpaceGuid.PcdArmMaliDpBase|0x0|UINT64|0x00
> 50
> +
> +
> gArmPlatformTokenSpaceGuid.PcdArmMaliDpMemoryRegionLength|0x0|U
> INT32|0
> + x0051
> +
>## PL180 MCI
> 
> gArmPlatformTokenSpaceGuid.PcdPL180SysMciRegAddress|0x|
> UINT32|0x0028
> 
> gArmPlatformTokenSpaceGuid.PcdPL180MciBaseAddress|0x|UI
> NT32|0x0029
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dsc
> b/ArmPlatformPkg/ArmPlatformPkg.dsc
> index
> 82adb9ef8891b7ba1628ede2f8eb124c25c2774a..0013106b94c371f827e0
> 1c6d402faa6aa42e4bdd 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dsc
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dsc
> @@ -2,7 +2,7 @@
>  # ARM platform package.
>  #
>  # Copyright (c) 2009 - 2010, Apple Inc. All rights reserved. -#
> Copyright (c) 2011 - 2015, ARM Ltd. All rights reserved.
> +# Copyright (c) 2011 - 2018, ARM Ltd. All rights reserved.
>  # Copyright (c) 2016 - 2017, Linaro Ltd. All rights reserved.  #
>  #This program and the accompanying materials
> @@ -120,3 +120,5 @@ [Components.common]
> 
>ArmPlatformPkg/PrePi/PeiMPCore.inf
>ArmPlatformPkg/PrePi/PeiUniCore.inf
> +
> +  ArmPlatformPkg/Library/ArmMaliDp/ArmMaliDp.inf
> diff --git a/ArmPlatformPkg/Library/ArmMaliDp/ArmMaliDp.c
> b/ArmPlatformPkg/Library/ArmMaliDp/ArmMaliDp.c
> new file mode 100644
> index
> ..69c88416a448caa506
> bf8a772432144d8a138495
> --- /dev/null
> +++ b/ArmPlatformPkg/Library/ArmMaliDp/ArmMaliDp.c
> @@ -0,0 +1,409 @@
> +/** @file
> +
> +  ARM Mali DP 500/550/650 display controller driver
> +
> +  Copyright (c) 2017-2018, Arm Limited. All rights reserved.
> +
> +  This program and the accompanying materials  are licensed and made
> + available under the terms and conditions of the BSD License  which
> + accompanies this distribution.  The full text of the license may be
> + found at  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> 

Re: [edk2] [PATCH v3 12/16] ArmPlatformPkg: Additional display modes

[Evan Lloyd]

Reviewed-by: Evan Lloyd 

> -Original Message-
> From: edk2-devel  On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:12
> To: edk2-devel@lists.01.org
> Cc: nd ; Stephanie Hughes-Fitt  f...@arm.com>; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v3 12/16] ArmPlatformPkg: Additional display
> modes
> 
> From: Girish Pathak 
> 
> Add definitions for new display modes such as HD 720.
> This has no effect on existing display drivers.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak 
> Signed-off-by: Evan Lloyd 
> Reviewed-by: Ard Biesheuvel 
> ---
>  ArmPlatformPkg/Include/Library/LcdPlatformLib.h | 60
> 
>  1 file changed, 60 insertions(+)
> 
> diff --git a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> index
> 8338b327fd2dd0d6b31653e278e25da5ac850939..cc535f0cd42db5673d41
> 8cbec940023927408687 100644
> --- a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> +++ b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> @@ -26,6 +26,11 @@
>  #define WSXGA 4
>  #define UXGA  5
>  #define HD6
> +#define WVGA  7
> +#define QHD   8
> +#define WSVGA 9
> +#define HD720 10
> +#define WXGA  11
> 
>  // VGA Mode: 640 x 480
>  #define VGA_H_RES_PIXELS  640
> @@ -118,6 +123,61 @@
>  #define HD_V_FRONT_PORCH  (  3 - 1)
>  #define HD_V_BACK_PORCH   ( 32 - 1)
> 
> +// WVGA Mode: 800 x 480
> +#define WVGA_H_RES_PIXELS 800
> +#define WVGA_V_RES_PIXELS 480
> +#define WVGA_OSC_FREQUENCY2950   /* 0x01C22260 */
> +#define WVGA_H_SYNC   ( 72 - 1)
> +#define WVGA_H_FRONT_PORCH( 24 - 1)
> +#define WVGA_H_BACK_PORCH ( 96 - 1)
> +#define WVGA_V_SYNC   (  7 - 1)
> +#define WVGA_V_FRONT_PORCH(  3 - 1)
> +#define WVGA_V_BACK_PORCH ( 10 - 1)
> +
> +// QHD Mode: 960 x 540
> +#define QHD_H_RES_PIXELS  960
> +#define QHD_V_RES_PIXELS  540
> +#define QHD_OSC_FREQUENCY 4075   /* 0x026DCBB0 */
> +#define QHD_H_SYNC( 96 - 1)
> +#define QHD_H_FRONT_PORCH ( 32 - 1)
> +#define QHD_H_BACK_PORCH  (128 - 1)
> +#define QHD_V_SYNC(  5 - 1)
> +#define QHD_V_FRONT_PORCH (  3 - 1)
> +#define QHD_V_BACK_PORCH  ( 14 - 1)
> +
> +// WSVGA Mode: 1024 x 600
> +#define WSVGA_H_RES_PIXELS1024
> +#define WSVGA_V_RES_PIXELS600
> +#define WSVGA_OSC_FREQUENCY   4900   /* 0x02EBAE40 */
> +#define WSVGA_H_SYNC  (104 - 1)
> +#define WSVGA_H_FRONT_PORCH   ( 40 - 1)
> +#define WSVGA_H_BACK_PORCH(144 - 1)
> +#define WSVGA_V_SYNC  ( 10 - 1)
> +#define WSVGA_V_FRONT_PORCH   (  3 - 1)
> +#define WSVGA_V_BACK_PORCH( 11 - 1)
> +
> +// HD720 Mode: 1280 x 720
> +#define HD720_H_RES_PIXELS 1280
> +#define HD720_V_RES_PIXELS 720
> +#define HD720_OSC_FREQUENCY7450   /* 0x0470C7A0 */
> +#define HD720_H_SYNC   (128 - 1)
> +#define HD720_H_FRONT_PORCH( 64 - 1)
> +#define HD720_H_BACK_PORCH (192 - 1)
> +#define HD720_V_SYNC   (  5 - 1)
> +#define HD720_V_FRONT_PORCH(  3 - 1)
> +#define HD720_V_BACK_PORCH ( 20 - 1)
> +
> +// WXGA Mode: 1280 x 800
> +#define WXGA_H_RES_PIXELS  1280
> +#define WXGA_V_RES_PIXELS  800
> +#define WXGA_OSC_FREQUENCY 8350  /* 0x04FA1BE0 */
> +#define WXGA_H_SYNC(128 - 1)
> +#define WXGA_H_FRONT_PORCH ( 72 - 1)
> +#define WXGA_H_BACK_PORCH  (200 - 1)
> +#define WXGA_V_SYNC(  6 - 1)
> +#define WXGA_V_FRONT_PORCH (  3 - 1)
> +#define WXGA_V_BACK_PORCH  ( 22 - 1)
> +
>  // Colour Masks
>  #define LCD_24BPP_RED_MASK  0x00FF
>  #define LCD_24BPP_GREEN_MASK0xFF00
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
> ___
> 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

Re: [edk2] [PATCH v3 04/16] ArmPlatformPkg: Tidy Lcd code: Updated comments

[Evan Lloyd]

Reviewed-by: Evan Lloyd 

> -Original Message-
> From: edk2-devel  On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:12
> To: edk2-devel@lists.01.org
> Cc: nd ; Stephanie Hughes-Fitt  f...@arm.com>; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v3 04/16] ArmPlatformPkg: Tidy Lcd code: Updated
> comments
> 
> From: Girish Pathak 
> 
> There is no functional modification in this change some comments are
> modified and a few new comments are added.
> This is to prevent mixing formatting changes with functional changes.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak 
> Signed-off-by: Evan Lloyd 
> ---
> 
> Notes:
> v3:
> - Propagated comments to LcdPlatformNullLib
> 
>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.c
> | 20 ++---
>  ArmPlatformPkg/Include/Library/LcdPlatformLib.h| 92
> +++-
>  ArmPlatformPkg/Library/HdLcd/HdLcd.c   | 26 
> +-
>  ArmPlatformPkg/Library/LcdPlatformNullLib/LcdPlatformNullLib.c | 65
> ++
>  ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c | 24 -
>  5 files changed, 189 insertions(+), 38 deletions(-)
> 
> diff --git
> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> c
> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> c
> index
> 905eb26ee01b5037dfbaf3c054a62593837c8b5f..872361cd23fbdf52c5f128
> d0e172701e76d832b2 100644
> ---
> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> c
> +++
> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> c
> @@ -1,13 +1,14 @@
>  /** @file
> +  This file implements the Graphics Output protocol for Arm platforms
> 
> - Copyright (c) 2011-2018, ARM Ltd. All rights reserved.
> - This program and the accompanying materials
> - are licensed and made available under the terms and conditions of the BSD
> License
> - which accompanies this distribution.  The full text of the license may be
> found at
> - http://opensource.org/licenses/bsd-license.php
> +  Copyright (c) 2011-2018, ARM Ltd. All rights reserved.  This
> + program and the accompanying materials  are licensed and made
> + available under the terms and conditions of the BSD License  which
> + accompanies this distribution.  The full text of the license may be
> + found at  http://opensource.org/licenses/bsd-license.php
> 
> - THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> - WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> 
>  **/
> 
> @@ -22,11 +23,6 @@
> 
>  #include "LcdGraphicsOutputDxe.h"
> 
> -/** This file implements the Graphics Output protocol on
> ArmVersatileExpress
> -  using the Lcd controller
> -
> -**/
> -
>  //
>  // Global variables
>  //
> diff --git a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> index
> 3d13e417972c67cc51ae4410efd548053511e5d1..e51e78640ae7b1acd51a
> c333ba3faa8c78aea5a5 100644
> --- a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> +++ b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> @@ -18,9 +18,7 @@
> 
>  #define LCD_VRAM_SIZE SIZE_8MB
> 
> -//
>  // Modes definitions
> -//
>  #define VGA   0
>  #define SVGA  1
>  #define XGA   2
> @@ -29,9 +27,7 @@
>  #define UXGA  5
>  #define HD6
> 
> -//
>  // VGA Mode: 640 x 480
> -//
>  #define VGA_H_RES_PIXELS  640
>  #define VGA_V_RES_PIXELS  480
>  #define VGA_OSC_FREQUENCY 2375  /* 0x016A6570 */
> @@ -44,9 +40,7 @@
>  #define VGA_V_FRONT_PORCH (  3 - 1)
>  #define VGA_V_BACK_PORCH  ( 13 - 1)
> 
> -//
>  // SVGA Mode: 800 x 600
> -//
>  #define SVGA_H_RES_PIXELS 800
>  #define SVGA_V_RES_PIXELS 600
>  #define SVGA_OSC_FREQUENCY3825  /* 0x0247A610 */
> @@ -59,9 +53,7 @@
>  #define SVGA_V_FRONT_PORCH(  3 - 1)
>  #define SVGA_V_BACK_PORCH ( 17 - 1)
> 
> -//
>  // XGA Mode: 1024 x 768
> -//
>  #define XGA_H_RES_PIXELS  1024
>  #define XGA_V_RES_PIXELS  768
>  #define XGA_OSC_FREQUENCY 6350  /* 0x03C8EEE0 */
> @@ -74,9 +66,7 @@
>  #define XGA_V_FRONT_PORCH (  3 - 1)
>  #define XGA_V_BACK_PORCH  ( 23 - 1)
> 
> -//
>  // SXGA Mode: 1280 x 1024
> -//
>  #define SXGA_H_RES_PIXELS 1280
>  #define SXGA_V_RES_PIXELS

Re: [edk2] [PATCH v3 09/16] ArmPlatformPkg: Redefine LcdPlatformGetTimings function

[Evan Lloyd]

Reviewed-by: Evan Lloyd 

> -Original Message-
> From: edk2-devel  On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:12
> To: edk2-devel@lists.01.org
> Cc: nd ; Stephanie Hughes-Fitt  f...@arm.com>; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v3 09/16] ArmPlatformPkg: Redefine
> LcdPlatformGetTimings function
> 
> From: Girish Pathak 
> 
> The LcdPlatformGetTimings interface function takes similar sets of multiple
> parameters for horizontal and vertical timings which can be aggregated in a
> common data type. This change defines a structure SCAN_TIMINGS for this
> which can be used to describe both horizontal and vertical scan timings,
> and accordingly redefines the LcdPlatformGetTiming interface, greatly
> reducing the amount of data passed about.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak 
> Signed-off-by: Evan Lloyd 
> Reviewed-by: Ard Biesheuvel 
> ---
>  ArmPlatformPkg/Include/Library/LcdPlatformLib.h| 31 
> ++
> --
>  ArmPlatformPkg/Library/HdLcd/HdLcd.c   | 50 
> +
> ---
>  ArmPlatformPkg/Library/LcdPlatformNullLib/LcdPlatformNullLib.c | 10 +---
>  ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c | 43
> +
>  4 files changed, 64 insertions(+), 70 deletions(-)
> 
> diff --git a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> index
> e51e78640ae7b1acd51ac333ba3faa8c78aea5a5..8338b327fd2dd0d6b316
> 53e278e25da5ac850939 100644
> --- a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> +++ b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> @@ -153,6 +153,14 @@ typedef enum {
>LCD_BITS_PER_PIXEL_12_444
>  } LCD_BPP;
> 
> +// Display timing settings.
> +typedef struct {
> +  UINT32  Resolution;
> +  UINT32  Sync;
> +  UINT32  BackPorch;
> +  UINT32  FrontPorch;
> +} SCAN_TIMINGS;
> +
>  /** Platform related initialization function.
> 
>@param[in] Handle  Handle to the LCD device instance.
> @@ -228,14 +236,11 @@ LcdPlatformQueryMode (
> 
>@param[in]  ModeNumber  Mode Number.
> 
> -  @param[out] HResPointer to horizontal resolution.
> -  @param[out] HSync   Pointer to horizontal sync width.
> -  @param[out] HBackPorch  Pointer to horizontal back porch.
> -  @param[out] HFrontPorch Pointer to horizontal front porch.
> -  @param[out] VResPointer to vertical resolution.
> -  @param[out] VSync   Pointer to vertical sync width.
> -  @param[out] VBackPorch  Pointer to vertical back porch.
> -  @param[out] VFrontPorch Pointer to vertical front porch.
> +  @param[out] Horizontal  Pointer to horizontal timing parameters.
> +  (Resolution, Sync, Back porch, Front porch)
> +  @param[out] VerticalPointer to vertical timing parameters.
> +  (Resolution, Sync, Back porch, Front
> + porch)
> +
> 
>@retval EFI_SUCCESS Display timing information for the 
> requested
>mode returned successfully.
> @@ -244,14 +249,8 @@ LcdPlatformQueryMode (  EFI_STATUS
> LcdPlatformGetTimings (
>IN  UINT32  ModeNumber,
> -  OUT UINT32* HRes,
> -  OUT UINT32* HSync,
> -  OUT UINT32* HBackPorch,
> -  OUT UINT32* HFrontPorch,
> -  OUT UINT32* VRes,
> -  OUT UINT32* VSync,
> -  OUT UINT32* VBackPorch,
> -  OUT UINT32* VFrontPorch
> +  OUT SCAN_TIMINGS**Horizontal,
> +  OUT SCAN_TIMINGS**Vertical
>);
> 
>  /** Return bits per pixel information for a mode number.
> diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> index
> 039048398c531ec944bc4b43a5551a554a368481..f5886848ce582b475b5
> 97ccca015c816707ade0e 100644
> --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> @@ -98,34 +98,25 @@ LcdSetMode (
>)
>  {
>EFI_STATUSStatus;
> -  UINT32HRes;
> -  UINT32HSync;
> -  UINT32HBackPorch;
> -  UINT32HFrontPorch;
> -  UINT32VRes;
> -  UINT32VSync;
> -  UINT32VBackPorch;
> -  UINT32VFrontPorch;
> +  SCAN_TIMINGS  *Horizontal;
> +  SCAN_TIMINGS  *Vertical;
>UINT32BytesPerPixel;
>LCD_BPP   LcdBpp;
> 
>// Set the video mode 

Re: [edk2] [PATCH v3 05/16] ArmPlatformPkg: HDLCD and PL111: Update debug ASSERTS

[Evan Lloyd]

Reviewed-by: Evan Lloyd 

> -Original Message-
> From: edk2-devel  On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:12
> To: edk2-devel@lists.01.org
> Cc: nd ; Stephanie Hughes-Fitt  f...@arm.com>; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v3 05/16] ArmPlatformPkg: HDLCD and PL111:
> Update debug ASSERTS
> 
> From: Girish Pathak 
> 
> This change moves some ASSERTs in error handling code to improve
> efficiency in DEBUG build. This change also removes redundant error code
> returns.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak 
> Signed-off-by: Evan Lloyd 
> ---
> 
> Notes:
> v3:
> - Reverted to ASSERT_EFI_ERROR (Status) from ASSERT (FALSE)
> 
>  ArmPlatformPkg/Library/HdLcd/HdLcd.c   | 11 +--
>  ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c | 12 ++--
>  2 files changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> index
> be4ccfdc1f421060faec792c8e8acfcfb3232014..28306c530e08b5e0fcef430
> 8435045da3c9e093c 100644
> --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> @@ -90,8 +90,7 @@ LcdInitialize (
>@param[in] ModeNumber  Display mode number.
> 
>@retval EFI_SUCCESSDisplay mode set successfully.
> -  @retval EFI_DEVICE_ERROR   Reurns an error if display timing
> - information is not available.
> +  @retval !(EFI_SUCCESS) Other errors.
>  **/
>  EFI_STATUS
>  LcdSetMode (
> @@ -122,15 +121,15 @@ LcdSetMode (
>   ,
>   
>   );
> -  ASSERT_EFI_ERROR (Status);
>if (EFI_ERROR (Status)) {
> -return EFI_DEVICE_ERROR;
> +ASSERT_EFI_ERROR (Status);
> +return Status;
>}
> 
>Status = LcdPlatformGetBpp (ModeNumber, );
> -  ASSERT_EFI_ERROR (Status);
>if (EFI_ERROR (Status)) {
> -return EFI_DEVICE_ERROR;
> +ASSERT_EFI_ERROR (Status);
> +return Status;
>}
> 
>BytesPerPixel = GetBytesPerPixel (LcdBpp); diff --git
> a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> index
> ccd7a4d1d43ad5c2f495683ac68236e17f3b55a5..cb64c57dd79f3bb1345e4
> d3dbda7f5b3ce859f40 100644
> --- a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> +++ b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> @@ -75,8 +75,8 @@ LcdInitialize (
>@param[in] ModeNumbe   Display mode number.
> 
>@retval EFI_SUCCESSDisplay mode set successfuly.
> -  @retval EFI_DEVICE_ERROR   It returns an error if display timing
> - information is not available.
> +  @retval !(EFI_SUCCESS) Other errors.
> +
>  **/
>  EFI_STATUS
>  LcdSetMode (
> @@ -107,15 +107,15 @@ LcdSetMode (
>   ,
>   
>   );
> -  ASSERT_EFI_ERROR (Status);
>if (EFI_ERROR (Status)) {
> -return EFI_DEVICE_ERROR;
> +ASSERT_EFI_ERROR (Status);
> +return Status;
>}
> 
>Status = LcdPlatformGetBpp (ModeNumber, );
> -  ASSERT_EFI_ERROR (Status);
>if (EFI_ERROR (Status)) {
> -return EFI_DEVICE_ERROR;
> +ASSERT_EFI_ERROR (Status);
> +return Status;
>}
> 
>// Disable the CLCD_LcdEn bit
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
> ___
> 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 v3 10/16] ArmPlatformPkg: Add PCD to select pixel format

[Evan Lloyd]

Reviewed-by: Evan Lloyd 

> -Original Message-
> From: edk2-devel  On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:12
> To: edk2-devel@lists.01.org
> Cc: nd ; Stephanie Hughes-Fitt  f...@arm.com>; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v3 10/16] ArmPlatformPkg: Add PCD to select pixel
> format
> 
> From: Girish Pathak 
> 
> Current HDLCD and PL111 platform libraries do not support display modes
> with PixelBlueGreenRedReserved8BitPerColor format, i.e. because of
> historical confusion, they do not support the UEFI default
> PixelBlueGreenRedReserved8BitPerColor format
> 
> In LcdPlatformLib for PL111, LcdPlatformQueryMode returns the pixel
> format as PixelRedGreenBlueReserved8BitPerColor which is wrong, because
> that does not match the display controller's pixel format which is set to
> BGR in PL111Lcd LcdHwLib.
> 
> Also it is not possible to configure pixel format as RGB/BGR for the display
> modes for a platform at build time.
> 
> This change adds PcdGopPixelFormat to configure pixel format as
> PixelRedGreenBlueReserved8BitPerColoror
> PixelBlueGreenRedReserved8BitPerColoror
> PixelBitMask.
> With this change, pixel format can be selected in the platform specific .dsc
> file for all supported display modes.
> 
> Support for PixelBitMask is not implemented in PL111 or HDLCD LcdHwLib
> libraries, hence  HDLCD and PL111 platform libraries will return error
> EFI_UNSUPPORTED if PcdGopPixelFormat is set to PixelBitMask.  Indeed, it
> is not clear what selecting PixelBitMask might mean, but the option is
> allowed as it might suit a custom platform.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak 
> Signed-off-by: Evan Lloyd 
> Reviewed-by: Ard Biesheuvel 
> ---
>  ArmPlatformPkg/ArmPlatformPkg.dec  |  9 +++-
>  ArmPlatformPkg/Library/HdLcd/HdLcd.c   | 54 +++-
>  ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c | 15 +-
>  3 files changed, 40 insertions(+), 38 deletions(-)
> 
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec
> b/ArmPlatformPkg/ArmPlatformPkg.dec
> index
> 7cec775abeee219e6821488a2c5abe88d23bbed1..378bee9cbc9e4bd50c37
> b38156016424e24cba73 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> @@ -1,6 +1,6 @@
>  #/** @file
>  #
> -#  Copyright (c) 2011-2017, ARM Limited. All rights reserved.
> +#  Copyright (c) 2011-2018, ARM Limited. All rights reserved.
>  #  Copyright (c) 2015, Intel Corporation. All rights reserved.
>  #
>  #  This program and the accompanying materials @@ -97,6 +97,13 @@
> [PcdsFixedAtBuild.common]
> 
> gArmPlatformTokenSpaceGuid.PcdPL180SysMciRegAddress|0x|
> UINT32|0x0028
> 
> gArmPlatformTokenSpaceGuid.PcdPL180MciBaseAddress|0x|UI
> NT32|0x0029
> 
> +  # Graphics Output Pixel format
> +  # 0 : PixelRedGreenBlueReserved8BitPerColor
> +  # 1 : PixelBlueGreenRedReserved8BitPerColor
> +  # 2 : PixelBitMask
> +  # Default is set to UEFI console font format
> + PixelBlueGreenRedReserved8BitPerColor
> +
> +
> gArmPlatformTokenSpaceGuid.PcdGopPixelFormat|0x0001|UINT32|0
> x0
> + 040
> +
>  [PcdsFixedAtBuild.common,PcdsDynamic.common]
>## PL031 RealTimeClock
> 
> gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x0|UINT32|0x0024
> diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> index
> f5886848ce582b475b597ccca015c816707ade0e..96f2bf437fbabd2509f86
> 0c67c5442def5b5f03d 100644
> --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> @@ -22,31 +22,7 @@
> 
>  #include "HdLcd.h"
> 
> -STATIC
> -UINTN
> -GetBytesPerPixel (
> -  IN  LCD_BPP   Bpp
> -  )
> -{
> -  switch (Bpp) {
> -  case LCD_BITS_PER_PIXEL_24:
> -return 4;
> -
> -  case LCD_BITS_PER_PIXEL_16_565:
> -  case LCD_BITS_PER_PIXEL_16_555:
> -  case LCD_BITS_PER_PIXEL_12_444:
> -return 2;
> -
> -  case LCD_BITS_PER_PIXEL_8:
> -  case LCD_BITS_PER_PIXEL_4:
> -  case LCD_BITS_PER_PIXEL_2:
> -  case LCD_BITS_PER_PIXEL_1:
> -return 1;
> -
> -  default:
> -return 0;
> -  }
> -}
> +#define BYTES_PER_PIXEL 4
> 
>  /** Initialize display.
> 
> @@ -78,10 +54,6 @@ LcdInitialize (
>  HDLCD_LITTLE_ENDIAN | HDLCD_4BYTES_PER_PIXEL
>  );
> 
> -  MmioWrite32 (HDLCD_REG_RED_SELECT,   (0 << 16 | 8 << 8 | 0));
> -  MmioWrite32 (HDLCD_REG_GREEN_SELECT, (0 << 16 | 8 << 8 | 8));
> -  MmioWrite32 (HDLCD_REG_BLUE_SELECT,  (0 << 16 | 8 << 8 | 16));
> -
>return EFI_SUCCESS;
>  }
> 
> @@ -100,8 +72,8 @@ LcdSetMode (
>EFI_STATUSStatus;
>SCAN_TIMINGS  *Horizontal;
>SCAN_TIMINGS  *Vertical;
> -  UINT32BytesPerPixel;
> -  LCD_BPP   LcdBpp;
> +
> +  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  ModeInfo;
> 
>// Set the video mode timings and other 

Re: [edk2] [PATCH v3 11/16] ArmPlatformPkg: PCD to swap red/blue format for HDLCD

[Evan Lloyd]

Reviewed-by: Evan Lloyd 

> -Original Message-
> From: edk2-devel  On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:12
> To: edk2-devel@lists.01.org
> Cc: nd ; Stephanie Hughes-Fitt  f...@arm.com>; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v3 11/16] ArmPlatformPkg: PCD to swap red/blue
> format for HDLCD
> 
> From: Girish Pathak 
> 
> This change adds a new PCD PcdArmHdlcdSwapBlueRedSelect to swap
> values for HDLCD RED_SELECT and BLUE_SELECT registers on platforms
> where blue and red hardware lines are swapped.
> 
> If set to TRUE in the platform dsc, HDLCD library will swap the values while
> setting RED_SELECT and BLUE_SELECT registers. The default value of the
> PCD is FALSE.
> 
> NOTE: The motive for this is that a discrepancy in the Red/Blue lines exists
> between some VersatileExpress platforms.  Rather than have divergent code,
> this build switch allows a simple, pragmatic solution.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak 
> Signed-off-by: Evan Lloyd 
> ---
> 
> Notes:
> v3:
> - Please don't nest CPP and C conditionals like this.
>   It is difficult to follow, and results in poor build
>   time coverage (the non-taken branch at the CPP level
>   is never seen by the compiler)   [Ard]
> 
>   Done [Girish]
> 
>  ArmPlatformPkg/ArmPlatformPkg.dec  |  3 +++
>  ArmPlatformPkg/Library/HdLcd/HdLcd.c   | 11 ++-
>  ArmPlatformPkg/Library/HdLcd/HdLcd.inf |  4 +++-
>  3 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec
> b/ArmPlatformPkg/ArmPlatformPkg.dec
> index
> 378bee9cbc9e4bd50c37b38156016424e24cba73..5231ea822f05c2f281a6
> 190d6eae0fc7d0bc0cb3 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> @@ -104,6 +104,9 @@ [PcdsFixedAtBuild.common]
># Default is set to UEFI console font format
> PixelBlueGreenRedReserved8BitPerColor
> 
> gArmPlatformTokenSpaceGuid.PcdGopPixelFormat|0x0001|UINT32|0
> x0040
> 
> +  ## If set, this will swap settings for HDLCD RED_SELECT and
> + BLUE_SELECT registers
> +
> +
> gArmPlatformTokenSpaceGuid.PcdArmHdLcdSwapBlueRedSelect|FALSE|BO
> OLEAN|
> + 0x0045
> +
>  [PcdsFixedAtBuild.common,PcdsDynamic.common]
>## PL031 RealTimeClock
> 
> gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x0|UINT32|0x0024
> diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> index
> 96f2bf437fbabd2509f860c67c5442def5b5f03d..5396dde3ba6cd147a8333
> 241a9bc71ab05d7fee3 100644
> --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> @@ -73,6 +73,8 @@ LcdSetMode (
>SCAN_TIMINGS  *Horizontal;
>SCAN_TIMINGS  *Vertical;
> 
> +  EFI_GRAPHICS_PIXEL_FORMAT  PixelFormat;
> +
>EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  ModeInfo;
> 
>// Set the video mode timings and other relevant information @@ -96,7
> +98,14 @@ LcdSetMode (
>  return Status;
>}
> 
> -  if (ModeInfo.PixelFormat == PixelBlueGreenRedReserved8BitPerColor) {
> +  // By default PcdArmHdLcdSwapBlueRedSelect is set to false  //
> + However on the Juno platform HW lines for BLUE and RED are swapped  //
> + Therefore PcdArmHdLcdSwapBlueRedSelect is set to TRUE for the Juno
> + platform  PixelFormat = FixedPcdGetBool
> (PcdArmHdLcdSwapBlueRedSelect)
> +? PixelRedGreenBlueReserved8BitPerColor
> +: PixelBlueGreenRedReserved8BitPerColor;
> +
> +  if (ModeInfo.PixelFormat == PixelFormat) {
>  MmioWrite32 (HDLCD_REG_RED_SELECT,  (8 << 8) | 16);
>  MmioWrite32 (HDLCD_REG_BLUE_SELECT, (8 << 8) | 0);
>} else {
> diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.inf
> b/ArmPlatformPkg/Library/HdLcd/HdLcd.inf
> index
> 67aad05d210b95b2d23b8e52e4392685efcf3795..7f2ba7bf1c602f4c214ea
> caa6425bf9ec7e6da15 100644
> --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.inf
> +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.inf
> @@ -2,7 +2,7 @@
>  #
>  #  Component description file for HDLCD module  # -#  Copyright (c) 2011-
> 2012, ARM Ltd. All rights reserved.
> +#  Copyright (c) 2011-2018, ARM Ltd. All rights reserved.
>  #
>  #  This program and the accompanying materials  #  are licensed and made
> available under the terms and conditions of the BSD License @@ -40,3
> +40,5 @@ [LibraryClasses]
> 
>  [FixedPcd]
>gArmPlatformTokenSpaceGuid.PcdArmHdLcdBase
> +  gArmPlatformTokenSpaceGuid.PcdArmHdLcdSwapBlueRedSelect
> +
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list

Re: [edk2] [PATCH v3 01/16] ArmPlatformPkg: Rectify line endings of LcdHwNullLib

[Evan Lloyd]

Reviewed-by: Evan Lloyd 

> -Original Message-
> From: edk2-devel  On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:12
> To: edk2-devel@lists.01.org
> Cc: nd ; Stephanie Hughes-Fitt  f...@arm.com>; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v3 01/16] ArmPlatformPkg: Rectify line endings of
> LcdHwNullLib
> 
> This fix changes line endings of LcdHwNullLib.c to DOS style line endings
> from UNIX style line endings to meet the
> EDK2 coding standard. Note it also fixes an end of line whitespace.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak 
> Signed-off-by: Evan Lloyd 
> ---
>  ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c | 150
> ++--
>  1 file changed, 75 insertions(+), 75 deletions(-)
> 
> diff --git a/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c
> b/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c
> index
> 2d31b5183c959c88eb1dab7703ec9b87f03eb50f..50e1c88112db69054597
> 9e7d008678c4f8ecd949 100644
> --- a/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c
> +++ b/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c
> @@ -1,75 +1,75 @@
> -/** @file
> -
> -  Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> -
> -  This program and the accompanying materials
> -  are licensed and made available under the terms and conditions of the
> BSD License
> -  which accompanies this distribution.  The full text of the license may be
> found at
> -  http://opensource.org/licenses/bsd-license.php
> -
> -  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> -  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> -
> -**/
> -
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -/**
> -  Check for presence of display
> -
> -  @retval EFI_SUCCESSPlatform implements display.
> -  @retval EFI_NOT_FOUND  Display not found on the platform.
> -
> -**/
> -EFI_STATUS
> -LcdIdentify (
> -  VOID
> -  )
> -{
> -  return EFI_SUCCESS;
> -}
> -
> -/**
> -  Initialize display.
> -
> -  @param  FrameBaseAddress   Address of the frame buffer.
> -  @retval EFI_SUCCESSDisplay initialization success.
> -  @retval !(EFI_SUCCESS) Display initialization failure.
> -
> -**/
> -EFI_STATUS
> -LcdInitialize (
> -  EFI_PHYSICAL_ADDRESS  FrameBaseAddress
> -  )
> -{
> -  return EFI_SUCCESS;
> -}
> -
> -/**
> -  Set requested mode of the display.
> -
> -  @param  ModeNumber Display mode number.
> -  @retval EFI_SUCCESSDisplay set mode success.
> -  @retval EFI_DEVICE_ERROR   If mode not found/supported.
> -
> -**/
> -EFI_STATUS
> -LcdSetMode (
> -  IN UINT32  ModeNumber
> -  )
> -{
> -  return EFI_SUCCESS;
> -}
> -
> -/**
> -  De-initializes the display.
> -**/
> -VOID
> -LcdShutdown (
> -  VOID
> -  )
> -{
> -}
> +/** @file
> +
> +  Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> +
> +  This program and the accompanying materials  are licensed and made
> + available under the terms and conditions of the BSD License  which
> + accompanies this distribution.  The full text of the license may be
> + found at  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/**
> +  Check for presence of display
> +
> +  @retval EFI_SUCCESSPlatform implements display.
> +  @retval EFI_NOT_FOUND  Display not found on the platform.
> +
> +**/
> +EFI_STATUS
> +LcdIdentify (
> +  VOID
> +  )
> +{
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Initialize display.
> +
> +  @param  FrameBaseAddress   Address of the frame buffer.
> +  @retval EFI_SUCCESSDisplay initialization success.
> +  @retval !(EFI_SUCCESS) Display initialization failure.
> +
> +**/
> +EFI_STATUS
> +LcdInitialize (
> +  EFI_PHYSICAL_ADDRESS  FrameBaseAddress
> +  )
> +{
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Set requested mode of the display.
> +
> +  @param  ModeNumber Display mode number.
> +  @retval EFI_SUCCESSDisplay set mode success.
> +  @retval EFI_DEVICE_ERROR   If mode not found/supported.
> +
> +**/
> +EFI_STATUS
> +LcdSetMode (
> +  IN UINT32  ModeNumber
> +  )
> +{
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  De-initializes the display.
> +**/
> +VOID
> +LcdShutdown (
> +  VOID
> +  )
> +{
> +}
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
> ___
> 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

Re: [edk2] [PATCH v3 02/16] ArmPlatformPkg: Rectify line endings of LcdPlatformNullLib

[Evan Lloyd]

Reviewed-by: Evan Lloyd 

> -Original Message-
> From: edk2-devel  On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:12
> To: edk2-devel@lists.01.org
> Cc: nd ; Stephanie Hughes-Fitt  f...@arm.com>; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v3 02/16] ArmPlatformPkg: Rectify line endings of
> LcdPlatformNullLib
> 
> This fix changes line endings of LcdPlatformNullLib.c to DOS style line
> endings from UNIX style line endings to meet the EDK2 coding standard.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak 
> Signed-off-by: Evan Lloyd 
> ---
>  ArmPlatformPkg/Library/LcdPlatformNullLib/LcdPlatformNullLib.c | 184
> ++--
>  1 file changed, 92 insertions(+), 92 deletions(-)
> 
> diff --git
> a/ArmPlatformPkg/Library/LcdPlatformNullLib/LcdPlatformNullLib.c
> b/ArmPlatformPkg/Library/LcdPlatformNullLib/LcdPlatformNullLib.c
> index
> 071eb5ffd4be895a0ce2ed5144f1f99e8195d1a0..b78d9a3bbd3e1fac4238f2
> be961a343020360a32 100644
> --- a/ArmPlatformPkg/Library/LcdPlatformNullLib/LcdPlatformNullLib.c
> +++ b/ArmPlatformPkg/Library/LcdPlatformNullLib/LcdPlatformNullLib.c
> @@ -1,92 +1,92 @@
> -/** @file
> -
> -  Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> -
> -  This program and the accompanying materials
> -  are licensed and made available under the terms and conditions of the
> BSD License
> -  which accompanies this distribution.  The full text of the license may be
> found at
> -  http://opensource.org/licenses/bsd-license.php
> -
> -  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> -  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> -
> -**/
> -
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -EFI_STATUS
> -LcdPlatformInitializeDisplay (
> -  IN EFI_HANDLE   Handle
> -  )
> -{
> -  ASSERT (FALSE);
> -  return EFI_UNSUPPORTED;
> -}
> -
> -EFI_STATUS
> -LcdPlatformGetVram (
> -  OUT EFI_PHYSICAL_ADDRESS* VramBaseAddress,
> -  OUT UINTN*VramSize
> -  )
> -{
> -  ASSERT (FALSE);
> -  return EFI_UNSUPPORTED;
> -}
> -
> -UINT32
> -LcdPlatformGetMaxMode (
> -  VOID
> -  )
> -{
> -  ASSERT (FALSE);
> -  return 0;
> -}
> -
> -EFI_STATUS
> -LcdPlatformSetMode (
> -  IN UINT32 ModeNumber
> -  )
> -{
> -  ASSERT (FALSE);
> -  return EFI_UNSUPPORTED;
> -}
> -
> -EFI_STATUS
> -LcdPlatformQueryMode (
> -  IN  UINT32ModeNumber,
> -  OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  *Info
> -  )
> -{
> -  ASSERT (FALSE);
> -  return EFI_UNSUPPORTED;
> -}
> -
> -EFI_STATUS
> -LcdPlatformGetTimings (
> -  IN  UINT32  ModeNumber,
> -  OUT UINT32* HRes,
> -  OUT UINT32* HSync,
> -  OUT UINT32* HBackPorch,
> -  OUT UINT32* HFrontPorch,
> -  OUT UINT32* VRes,
> -  OUT UINT32* VSync,
> -  OUT UINT32* VBackPorch,
> -  OUT UINT32* VFrontPorch
> -  )
> -{
> -  ASSERT (FALSE);
> -  return EFI_UNSUPPORTED;
> -}
> -
> -EFI_STATUS
> -LcdPlatformGetBpp (
> -  IN  UINT32ModeNumber,
> -  OUT LCD_BPP*  Bpp
> -  )
> -{
> -  ASSERT (FALSE);
> -  return EFI_UNSUPPORTED;
> -}
> +/** @file
> +
> +  Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> +
> +  This program and the accompanying materials  are licensed and made
> + available under the terms and conditions of the BSD License  which
> + accompanies this distribution.  The full text of the license may be
> + found at  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +EFI_STATUS
> +LcdPlatformInitializeDisplay (
> +  IN EFI_HANDLE   Handle
> +  )
> +{
> +  ASSERT (FALSE);
> +  return EFI_UNSUPPORTED;
> +}
> +
> +EFI_STATUS
> +LcdPlatformGetVram (
> +  OUT EFI_PHYSICAL_ADDRESS* VramBaseAddress,
> +  OUT UINTN*VramSize
> +  )
> +{
> +  ASSERT (FALSE);
> +  return EFI_UNSUPPORTED;
> +}
> +
> +UINT32
> +LcdPlatformGetMaxMode (
> +  VOID
> +  )
> +{
> +  ASSERT (FALSE);
> +  return 0;
> +}
> +
> +EFI_STATUS
> +LcdPlatformSetMode (
> +  IN UINT32 ModeNumber
> +  )
> +{
> +  ASSERT (FALSE);
> +  return EFI_UNSUPPORTED;
> +}
> +
> +EFI_STATUS
> +LcdPlatformQueryMode (
> +  IN  UINT32ModeNumber,
> +  OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  *Info
> +  )
> +{

Re: [edk2] [edk2 UsbMassStorageDxe v1 1/1] MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS

[Zeng, Star]

Ming,

Basically, I am ok with the change in this patch. There are two comments.

1. Please add more background information about the specific device in the 
commit log. For example, you mentioned " like some virtual CD-ROM from BMC " in 
previous patch.
2. Please make sure building pass with the patch on different tool chains. I 
tried to build with the patch on VS2015, but met building failure like below.
warning C4244: '=': conversion from 'UINT32' to 'UINT16', possible loss of data


Ray,

Do you have other concern?


Thanks,
Star
-Original Message-
From: Ming Huang [mailto:ming.hu...@linaro.org] 
Sent: Thursday, March 15, 2018 8:30 PM
To: leif.lindh...@linaro.org; linaro-u...@lists.linaro.org; 
edk2-devel@lists.01.org; Zeng, Star ; Dong, Eric 

Cc: ard.biesheu...@linaro.org; Kinney, Michael D ; 
Gao, Liming ; guoh...@huawei.com; 
wanghuiqi...@huawei.com; huangmin...@huawei.com; zhangjinso...@huawei.com; 
mengfanr...@huawei.com; huangda...@hisilicon.com; wai...@126.com; Ming Huang 

Subject: [edk2 UsbMassStorageDxe v1 1/1] MdeModulePkg/Usb: Replace macro 
USB_BOOT_IO_BLOCKS

Booting from USB may fail while the macro USB_BOOT_IO_BLOCKS set to 128 because 
the block size of some USB devices are exceeded 512. So,the count blocks to 
transfer should be calculated by block size of the USB devices.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang 
---
 MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c | 16   
MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h |  4 ++--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c 
b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
index b84bfd2d7290..b38cb6116bf4 100644
--- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
+++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
@@ -814,11 +814,13 @@ UsbBootReadBlocks (
   USB_BOOT_READ10_CMD   ReadCmd;
   EFI_STATUSStatus;
   UINT16Count;
+  UINT16CountMax;
   UINT32BlockSize;
   UINT32ByteSize;
   UINT32Timeout;
 
   BlockSize = UsbMass->BlockIoMedia.BlockSize;
+  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
   Status= EFI_SUCCESS;
 
   while (TotalBlock > 0) {
@@ -827,7 +829,7 @@ UsbBootReadBlocks (
 // on the device. We must split the total block because the READ10
 // command only has 16 bit transfer length (in the unit of block).
 //
-Count = (UINT16)((TotalBlock < USB_BOOT_IO_BLOCKS) ? TotalBlock : 
USB_BOOT_IO_BLOCKS);
+Count = (UINT16)((TotalBlock < CountMax) ? TotalBlock : CountMax);
 ByteSize  = (UINT32)Count * BlockSize;
 
 //
@@ -890,11 +892,13 @@ UsbBootWriteBlocks (
   USB_BOOT_WRITE10_CMD  WriteCmd;
   EFI_STATUSStatus;
   UINT16Count;
+  UINT16CountMax;
   UINT32BlockSize;
   UINT32ByteSize;
   UINT32Timeout;
 
   BlockSize = UsbMass->BlockIoMedia.BlockSize;
+  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
   Status= EFI_SUCCESS;
 
   while (TotalBlock > 0) {
@@ -903,7 +907,7 @@ UsbBootWriteBlocks (
 // on the device. We must split the total block because the WRITE10
 // command only has 16 bit transfer length (in the unit of block).
 //
-Count = (UINT16)((TotalBlock < USB_BOOT_IO_BLOCKS) ? TotalBlock : 
USB_BOOT_IO_BLOCKS);
+Count = (UINT16)((TotalBlock < CountMax) ? TotalBlock : CountMax);
 ByteSize  = (UINT32)Count * BlockSize;
 
 //
@@ -966,18 +970,20 @@ UsbBootReadBlocks16 (
   UINT8 ReadCmd[16];
   EFI_STATUSStatus;
   UINT16Count;
+  UINT16CountMax;
   UINT32BlockSize;
   UINT32ByteSize;
   UINT32Timeout;
 
   BlockSize = UsbMass->BlockIoMedia.BlockSize;
+  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
   Status= EFI_SUCCESS;
 
   while (TotalBlock > 0) {
 //
 // Split the total blocks into smaller pieces.
 //
-Count = (UINT16)((TotalBlock < USB_BOOT_IO_BLOCKS) ? TotalBlock : 
USB_BOOT_IO_BLOCKS);
+Count = (UINT16)((TotalBlock < CountMax) ? TotalBlock : CountMax);
 ByteSize  = (UINT32)Count * BlockSize;
 
 //
@@ -1040,18 +1046,20 @@ UsbBootWriteBlocks16 (
   UINT8 WriteCmd[16];
   EFI_STATUSStatus;
   UINT16Count;
+  UINT16CountMax;
   UINT32BlockSize;
   UINT32ByteSize;
   UINT32Timeout;
 
   BlockSize = UsbMass->BlockIoMedia.BlockSize;
+  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
   Status= EFI_SUCCESS;
 
   while (TotalBlock > 0) {
 

Re: [edk2] [PATCH v3 06/16] ArmPlatformPkg: PL111Lcd: Replace magic number with macro

[Evan Lloyd]

Reviewed-by: Evan Lloyd 

> -Original Message-
> From: edk2-devel  On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:12
> To: edk2-devel@lists.01.org
> Cc: nd ; Stephanie Hughes-Fitt  f...@arm.com>; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v3 06/16] ArmPlatformPkg: PL111Lcd: Replace
> magic number with macro
> 
> From: Girish Pathak 
> 
> Minor code change, replaces magic number with macro in LCD disable.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak 
> Signed-off-by: Evan Lloyd 
> Reviewed-by: Ard Biesheuvel 
> ---
>  ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> index
> cb64c57dd79f3bb1345e4d3dbda7f5b3ce859f40..287e3ca272c0c19f8045a
> 3bf4e69a092d8da6fd8 100644
> --- a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> +++ b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> @@ -119,8 +119,7 @@ LcdSetMode (
>}
> 
>// Disable the CLCD_LcdEn bit
> -  LcdControl = MmioRead32 (PL111_REG_LCD_CONTROL);
> -  MmioWrite32 (PL111_REG_LCD_CONTROL, LcdControl & ~1);
> +  MmioAnd32 (PL111_REG_LCD_CONTROL, ~PL111_CTRL_LCD_EN);
> 
>// Set Timings
>MmioWrite32 (
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
> ___
> 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 v3 07/16] ArmPlatformPkg: PL111Lcd: Combine two writes to LCDControl

[Evan Lloyd]

Reviewed-by: Evan Lloyd 

> -Original Message-
> From: edk2-devel  On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:12
> To: edk2-devel@lists.01.org
> Cc: nd ; Stephanie Hughes-Fitt  f...@arm.com>; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v3 07/16] ArmPlatformPkg: PL111Lcd: Combine
> two writes to LCDControl
> 
> Currenty bit LcdPwr of the LCDControl register is enabled immediately after
> setting other bits of the LCDControl register. This two write sequence is
> unnecessary. This change removes this extra write by setting LcdPwr bit
> along with other bits of the LcdControl register.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak 
> Signed-off-by: Evan Lloyd 
> ---
>  ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> index
> 287e3ca272c0c19f8045a3bf4e69a092d8da6fd8..465cb6845437f57d15f05
> a271d1b01f634e11b56 100644
> --- a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> +++ b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> @@ -137,11 +137,7 @@ LcdSetMode (
> 
>// PL111_REG_LCD_CONTROL
>LcdControl = PL111_CTRL_LCD_EN | PL111_CTRL_LCD_BPP (LcdBpp) |
> -   PL111_CTRL_LCD_TFT | PL111_CTRL_BGR;
> -  MmioWrite32 (PL111_REG_LCD_CONTROL, LcdControl);
> -
> -  // Turn on power to the LCD Panel
> -  LcdControl |= PL111_CTRL_LCD_PWR;
> +   PL111_CTRL_LCD_TFT | PL111_CTRL_LCD_PWR |
> + PL111_CTRL_BGR;
>MmioWrite32 (PL111_REG_LCD_CONTROL, LcdControl);
> 
>return EFI_SUCCESS;
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
> ___
> 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 v3 08/16] ArmPlatformPkg: Implement LcdIdentify function for HDLCD GOP

[Evan Lloyd]

Reviewed-by: Evan Lloyd 

> -Original Message-
> From: edk2-devel  On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:12
> To: edk2-devel@lists.01.org
> Cc: nd ; Stephanie Hughes-Fitt  f...@arm.com>; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> Subject: [edk2] [PATCH v3 08/16] ArmPlatformPkg: Implement LcdIdentify
> function for HDLCD GOP
> 
> From: Girish Pathak 
> 
> LcdIdentify function does not currently check presence of HDLCD controller.
> 
> Implement this functionality by reading HDLCD_REG_VERSION and checking
> against the PRODUCT_ID field to detect presence of HDLCD controller.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak 
> Signed-off-by: Evan Lloyd 
> Reviewed-by: Ard Biesheuvel 
> ---
>  ArmPlatformPkg/Library/HdLcd/HdLcd.c | 8 +++-
> ArmPlatformPkg/Library/HdLcd/HdLcd.h | 2 ++
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> index
> 28306c530e08b5e0fcef4308435045da3c9e093c..039048398c531ec944bc4
> b43a5551a554a368481 100644
> --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> @@ -175,11 +175,17 @@ LcdShutdown (
> 
>@retval EFI_SUCCESSReturns success if platform implements a
> HDLCD
>   controller.
> +  @retval EFI_NOT_FOUND  HDLCD display controller not found on the
> + platform.
>  **/
>  EFI_STATUS
>  LcdIdentify (
>VOID
>)
>  {
> -  return EFI_SUCCESS;
> +  if ((MmioRead32 (HDLCD_REG_VERSION) >> 16) == HDLCD_PRODUCT_ID)
> {
> +return EFI_SUCCESS;
> +  }
> +
> +  return EFI_NOT_FOUND;
>  }
> diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.h
> b/ArmPlatformPkg/Library/HdLcd/HdLcd.h
> index
> cd2c0366c7b563d7fb313f82abeef7eb1aa3ef72..1efa78eedc3013b3ab4615
> 181a59c7d15b851ab5 100644
> --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.h
> +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.h
> @@ -85,4 +85,6 @@
>  // Number of bytes per pixel
>  #define HDLCD_4BYTES_PER_PIXEL   ((4 - 1) << 3)
> 
> +#define HDLCD_PRODUCT_ID 0x1CDC
> +
>  #endif /* HDLCD_H_ */
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg: Allocate framebuffer using EfiRuntimeServicesData

[Girish Pathak]

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: 21 March 2018 03:38
> To: Girish Pathak 
> Cc: edk2-devel@lists.01.org; Leif Lindholm ;
> Matteo Carlini ; Stephanie Hughes-Fitt
> ; nd ; Arvind Chauhan
> ; Daniil Egranov ;
> Thomas Abraham 
> Subject: Re: [PATCH edk2-platforms v3 12/17] ARM/VExpressPkg: Allocate
> framebuffer using EfiRuntimeServicesData
> 
> On 21 March 2018 at 00:18, Girish Pathak  wrote:
> > As per the UEFI specification(2.7) section 12.9, the GOP framebuffer
> > memory can be accessed in the pre-boot and the post boot phase (by OS)
> > Therefore the memory type EfiBootServicesData is incorrect for the
> > framebuffer memory allocation. Change EfiBootServicesData with
> > EfiRuntimeServicesData flag so that allocated memory can be access by
> > the OS in the post boot phase.
> >
> 
> EfiRuntimeServicesData is intended for allocations that the EFI runtime
> services need to access themselves at runtime, and will hence be virtually
> remapped by SetVirtualAddressMap().
> 
> This does not apply to the framebuffer. Even if it may be used at OS runtime,
> the firmware itself will never access it, so EfiRuntimeServicesData is not
> appropriate
> 
> Please use EfiReservedMemory instead.

Specification (UEFI Spec 2_7_A Sept 6.pdf) describes EfiReservedMemoryType as  
Not usable before and after ExitBootServices, See Table 28 & 29
Hence EfiReservedMemoryType is not suitable in this case.  Agree? 

> 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Girish Pathak 
> > Signed-off-by: Evan Lloyd 
> > ---
> >
> Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpr
> ess.c   | 2 +-
> >
> >
> Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArm
> VEx
> > press.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git
> >
> a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVEx
> pres
> > s.c
> >
> b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVEx
> pres
> > s.c index
> >
> f7cae39c9cc9954ba4cad1bd597ebfc8baf10f11..c0a25a18d3fcfe91a76ee985ee
> 58
> > 145b97900fa0 100644
> > ---
> >
> a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVEx
> pres
> > s.c
> > +++
> b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVEx
> > +++ press.c
> > @@ -176,7 +176,7 @@ LcdPlatformGetVram (
> >}
> >Status = gBS->AllocatePages (
> >AllocationType,
> > -  EfiBootServicesData,
> > +  EfiRuntimeServicesData,
> >EFI_SIZE_TO_PAGES (((UINTN)LCD_VRAM_SIZE)),
> >VramBaseAddress
> >);
> > diff --git
> >
> a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdAr
> mV
> > Express.c
> >
> b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdAr
> mV
> > Express.c index
> >
> 2f4814a2adbf01517ba14d75ce579ff35c362379..61ddf77e903e6c33a26b2aa8b7
> 61
> > 21e807195a9a 100644
> > ---
> >
> a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdAr
> mV
> > Express.c
> > +++
> b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111Lcd
> > +++ ArmVExpress.c
> > @@ -232,7 +232,7 @@ LcdPlatformGetVram (
> >  // Allocate the VRAM from the DRAM so that nobody else uses it.
> >  Status = gBS->AllocatePages (
> >  AllocateAddress,
> > -EfiBootServicesData,
> > +EfiRuntimeServicesData,
> >  EFI_SIZE_TO_PAGES (((UINTN)LCD_VRAM_SIZE)),
> >  VramBaseAddress
> >  );
> > --
> > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> >
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] BaseTools/ECC: Add a new exception support

[Yonghong Zhu]

From: Hess Chen 

Add a new exception support for the checkPoint of no use C type.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hess Chen 
---
 BaseTools/Source/Python/Ecc/c.py | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/Ecc/c.py b/BaseTools/Source/Python/Ecc/c.py
index 35b7405e55..39a9d8ac36 100644
--- a/BaseTools/Source/Python/Ecc/c.py
+++ b/BaseTools/Source/Python/Ecc/c.py
@@ -1,7 +1,7 @@
 ## @file
 # This file is used to be the c coding style checking of ECC tool
 #
-# Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
+# Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
 # This program and the accompanying materials
 # are licensed and made available under the terms and conditions of the BSD 
License
 # which accompanies this distribution.  The full text of the license may be 
found at
@@ -1858,7 +1858,13 @@ def CheckDeclNoUseCType(FullFileName):
 for Result in ResultSet:
 for Type in CTypeTuple:
 if PatternInModifier(Result[0], Type):
-PrintErrorMsg(ERROR_DECLARATION_DATA_TYPE_CHECK_NO_USE_C_TYPE, 
'Variable type %s' % Type, FileTable, Result[2])
+if 
EccGlobalData.gException.IsException(ERROR_DECLARATION_DATA_TYPE_CHECK_NO_USE_C_TYPE,
+Result[0] + ' ' + 
Result[1]):
+continue
+PrintErrorMsg(ERROR_DECLARATION_DATA_TYPE_CHECK_NO_USE_C_TYPE,
+  'Invalid variable type (%s) in definition [%s]' 
% (Type, Result[0] + ' ' + Result[1]),
+  FileTable,
+  Result[2])
 break
 
 SqlStatement = """ select Modifier, Name, ID, Value
-- 
2.14.2.windows.2

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

Re: [edk2] [Patch] SecurityPkg Tpm2CommandLib: Fix TPM2.0 response memory overflow

[Zhang, Chao B]

Good catch！ Jiewen, I will add more check in CopyAuthSessionResponse()

From: Yao, Jiewen
Sent: Wednesday, March 21, 2018 2:39 PM
To: Zhang, Chao B ; Long, Qin ; 
edk2-devel@lists.01.org
Subject: RE: [Patch] SecurityPkg Tpm2CommandLib: Fix TPM2.0 response memory 
overflow

Some thought:

1) Would you please add debug message on every error check you added? Just like 
the original code does.

2) For below, can we separate the check, and add error message for each failure?
Tpm2Integrity.c:
  if (PcrSelectionOut->count > HASH_COUNT || RecvBufferSize < sizeof 
(TPM2_RESPONSE_HEADER) + sizeof(RecvBuffer.PcrUpdateCounter) + 
sizeof(RecvBuffer.PcrSelectionOut.count) + 
sizeof(RecvBuffer.PcrSelectionOut.pcrSelections[0]) * PcrSelectionOut->count) {
DEBUG ((EFI_D_ERROR, "Tpm2PcrRead - Digests->count -%x or RecvBufferSize 
Error - %x\n", PcrSelectionOut->count, RecvBufferSize));
return EFI_DEVICE_ERROR;
  }

3) For below, can we separate the check, and add error message for each failure?
Tpm2NvStorage.c
  if (NvNameSize > sizeof(TPMU_NAME) ||
  (RecvBufferSize != sizeof(TPM2_RESPONSE_HEADER) + sizeof(UINT16) + 
NvPublicSize + sizeof(UINT16) + NvNameSize)) {
DEBUG ((EFI_D_ERROR, "Tpm2NvReadPublic - RecvBufferSize Error - 
NvPublicSize %x, NvNameSize %x\n", RecvBufferSize, NvNameSize));
return EFI_NOT_FOUND;
  }


4) Do you think if we need add check for nonce.size below as well?
Tpm2Help.c

  // nonce
  AuthSessionOut->nonce.size = SwapBytes16 (ReadUnaligned16 ((UINT16 *)Buffer));
  Buffer += sizeof(UINT16);

  CopyMem (AuthSessionOut->nonce.buffer, Buffer, AuthSessionOut->nonce.size);
  Buffer += AuthSessionOut->nonce.size;


Thank you
Yao Jiewen


> -Original Message-
> From: Zhang, Chao B
> Sent: Wednesday, March 21, 2018 11:03 AM
> To: Long, Qin >; 
> edk2-devel@lists.01.org
> Cc: Yao, Jiewen >
> Subject: RE: [Patch] SecurityPkg Tpm2CommandLib: Fix TPM2.0 response
> memory overflow
>
> Thanks Qin, I will add more comments to explain the magic code
>
> -Original Message-
> From: Long, Qin
> Sent: Wednesday, March 21, 2018 10:58 AM
> To: Zhang, Chao B >; 
> edk2-devel@lists.01.org
> Cc: Yao, Jiewen >
> Subject: RE: [Patch] SecurityPkg Tpm2CommandLib: Fix TPM2.0 response
> memory overflow
>
> Hi, Chao,
>
> One minor suggestion to add the comment to explain the following value "8": 
> the
> number of digests in list is not greater than 8 per TPML_DIGEST definition.
> +  if (PcrValues->count > 8) {
> +return EFI_DEVICE_ERROR;
> +  }
>
> Other looks good to me.
>
> Reviewed-by: Long Qin >
>
>
> Best Regards & Thanks,
> LONG, Qin
>
> -Original Message-
> From: Zhang, Chao B
> Sent: Tuesday, March 20, 2018 4:36 PM
> To: edk2-devel@lists.01.org
> Cc: Long, Qin >; Yao, Jiewen 
> >;
> Zhang, Chao B >
> Subject: [Patch] SecurityPkg Tpm2CommandLib: Fix TPM2.0 response memory
> overflow
>
> TPM2.0 command lib always assumes TPM device and transmission channel can
> respond correctly. But it is not true when communication channel is exploited
> and wrong data is spoofed. Add more logic to prohibit memory overflow attack.
>
> Cc: Long Qin >
> Cc: Yao Jiewen >
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chao Zhang 
> >
> Signed-off-by: Zhang, Chao B 
> >
> ---
>  .../Library/Tpm2CommandLib/Tpm2Capability.c| 21
> ++-
>  .../Tpm2CommandLib/Tpm2EnhancedAuthorization.c | 16 ++-
>  SecurityPkg/Library/Tpm2CommandLib/Tpm2Integrity.c | 19 ++---
> SecurityPkg/Library/Tpm2CommandLib/Tpm2NVStorage.c | 14 --
>  SecurityPkg/Library/Tpm2CommandLib/Tpm2Object.c| 31
> +-
>  SecurityPkg/Library/Tpm2CommandLib/Tpm2Sequences.c | 10 ++-
>  SecurityPkg/Library/Tpm2CommandLib/Tpm2Session.c   |  6 -
>  7 files changed, 107 insertions(+), 10 deletions(-)
>
> diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> index 79e80fb7a9..42afe107a6 100644
> --- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> +++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> @@ -1,9 +1,9 @@
>  /** @file
>Implement TPM2 Capability related command.
>
> -Copyright (c) 2013 - 2016, Intel Corporation. All rights 

Re: [edk2] [Patch] SecurityPkg Tpm12CommandLib: Fix TPM12 GetCapability response error

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zhang,
> Chao B
> Sent: Tuesday, March 20, 2018 11:12 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Zhang, Chao B
> ; Long, Qin 
> Subject: [edk2] [Patch] SecurityPkg Tpm12CommandLib: Fix TPM12
> GetCapability response error
> 
> TPM12 command lib doesn't convert Response Size before using. Add logic
> to fix the issue.
> 
> Cc: Long Qin 
> Cc: Yao Jiewen 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chao Zhang 
> Signed-off-by: Zhang, Chao B 
> ---
>  SecurityPkg/Library/Tpm12CommandLib/Tpm12GetCapability.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/SecurityPkg/Library/Tpm12CommandLib/Tpm12GetCapability.c
> b/SecurityPkg/Library/Tpm12CommandLib/Tpm12GetCapability.c
> index c6eb9e1050..29d7a13edb 100644
> --- a/SecurityPkg/Library/Tpm12CommandLib/Tpm12GetCapability.c
> +++ b/SecurityPkg/Library/Tpm12CommandLib/Tpm12GetCapability.c
> @@ -1,9 +1,9 @@
>  /** @file
>Implement TPM1.2 Get Capabilities related commands.
> 
> -Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved. 
> +Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved. 
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be 
> found
> at
>  http://opensource.org/licenses/bsd-license.php
> 
> @@ -83,11 +83,11 @@ Tpm12GetCapabilityFlagPermanent (
>  DEBUG ((DEBUG_ERROR, "Tpm12GetCapabilityFlagPermanent: Response
> Code error! 0x%08x\r\n", SwapBytes32 (Response.Hdr.returnCode)));
>  return EFI_DEVICE_ERROR;
>}
> 
>ZeroMem (TpmPermanentFlags, sizeof (*TpmPermanentFlags));
> -  CopyMem (TpmPermanentFlags, , MIN (sizeof
> (*TpmPermanentFlags), Response.ResponseSize));
> +  CopyMem (TpmPermanentFlags, , MIN (sizeof
> (*TpmPermanentFlags), SwapBytes32(Response.ResponseSize)));
> 
>return Status;
>  }
> 
>  /**
> @@ -129,9 +129,9 @@ Tpm12GetCapabilityFlagVolatile (
>  DEBUG ((DEBUG_ERROR, "Tpm12GetCapabilityFlagVolatile: Response Code
> error! 0x%08x\r\n", SwapBytes32 (Response.Hdr.returnCode)));
>  return EFI_DEVICE_ERROR;
>}
> 
>ZeroMem (VolatileFlags, sizeof (*VolatileFlags));
> -  CopyMem (VolatileFlags, , MIN (sizeof (*VolatileFlags),
> Response.ResponseSize));
> +  CopyMem (VolatileFlags, , MIN (sizeof (*VolatileFlags),
> SwapBytes32(Response.ResponseSize)));
> 
>return Status;
>  }
> --
> 2.16.2.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] SecurityPkg Tpm2CommandLib: Fix TPM2.0 response memory overflow

[Yao, Jiewen]

Some thought:

1) Would you please add debug message on every error check you added? Just like 
the original code does.

2) For below, can we separate the check, and add error message for each failure?
Tpm2Integrity.c:
  if (PcrSelectionOut->count > HASH_COUNT || RecvBufferSize < sizeof 
(TPM2_RESPONSE_HEADER) + sizeof(RecvBuffer.PcrUpdateCounter) + 
sizeof(RecvBuffer.PcrSelectionOut.count) + 
sizeof(RecvBuffer.PcrSelectionOut.pcrSelections[0]) * PcrSelectionOut->count) {
DEBUG ((EFI_D_ERROR, "Tpm2PcrRead - Digests->count -%x or RecvBufferSize 
Error - %x\n", PcrSelectionOut->count, RecvBufferSize));
return EFI_DEVICE_ERROR;
  }

3) For below, can we separate the check, and add error message for each failure?
Tpm2NvStorage.c
  if (NvNameSize > sizeof(TPMU_NAME) ||
  (RecvBufferSize != sizeof(TPM2_RESPONSE_HEADER) + sizeof(UINT16) + 
NvPublicSize + sizeof(UINT16) + NvNameSize)) {
DEBUG ((EFI_D_ERROR, "Tpm2NvReadPublic - RecvBufferSize Error - 
NvPublicSize %x, NvNameSize %x\n", RecvBufferSize, NvNameSize));
return EFI_NOT_FOUND;
  }


4) Do you think if we need add check for nonce.size below as well?
Tpm2Help.c

  // nonce
  AuthSessionOut->nonce.size = SwapBytes16 (ReadUnaligned16 ((UINT16 *)Buffer));
  Buffer += sizeof(UINT16);

  CopyMem (AuthSessionOut->nonce.buffer, Buffer, AuthSessionOut->nonce.size);
  Buffer += AuthSessionOut->nonce.size;


Thank you
Yao Jiewen


> -Original Message-
> From: Zhang, Chao B
> Sent: Wednesday, March 21, 2018 11:03 AM
> To: Long, Qin ; edk2-devel@lists.01.org
> Cc: Yao, Jiewen 
> Subject: RE: [Patch] SecurityPkg Tpm2CommandLib: Fix TPM2.0 response
> memory overflow
> 
> Thanks Qin, I will add more comments to explain the magic code
> 
> -Original Message-
> From: Long, Qin
> Sent: Wednesday, March 21, 2018 10:58 AM
> To: Zhang, Chao B ; edk2-devel@lists.01.org
> Cc: Yao, Jiewen 
> Subject: RE: [Patch] SecurityPkg Tpm2CommandLib: Fix TPM2.0 response
> memory overflow
> 
> Hi, Chao,
> 
> One minor suggestion to add the comment to explain the following value "8": 
> the
> number of digests in list is not greater than 8 per TPML_DIGEST definition.
> +  if (PcrValues->count > 8) {
> +return EFI_DEVICE_ERROR;
> +  }
> 
> Other looks good to me.
> 
> Reviewed-by: Long Qin 
> 
> 
> Best Regards & Thanks,
> LONG, Qin
> 
> -Original Message-
> From: Zhang, Chao B
> Sent: Tuesday, March 20, 2018 4:36 PM
> To: edk2-devel@lists.01.org
> Cc: Long, Qin ; Yao, Jiewen ;
> Zhang, Chao B 
> Subject: [Patch] SecurityPkg Tpm2CommandLib: Fix TPM2.0 response memory
> overflow
> 
> TPM2.0 command lib always assumes TPM device and transmission channel can
> respond correctly. But it is not true when communication channel is exploited
> and wrong data is spoofed. Add more logic to prohibit memory overflow attack.
> 
> Cc: Long Qin 
> Cc: Yao Jiewen 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chao Zhang 
> Signed-off-by: Zhang, Chao B 
> ---
>  .../Library/Tpm2CommandLib/Tpm2Capability.c| 21
> ++-
>  .../Tpm2CommandLib/Tpm2EnhancedAuthorization.c | 16 ++-
>  SecurityPkg/Library/Tpm2CommandLib/Tpm2Integrity.c | 19 ++---
> SecurityPkg/Library/Tpm2CommandLib/Tpm2NVStorage.c | 14 --
>  SecurityPkg/Library/Tpm2CommandLib/Tpm2Object.c| 31
> +-
>  SecurityPkg/Library/Tpm2CommandLib/Tpm2Sequences.c | 10 ++-
>  SecurityPkg/Library/Tpm2CommandLib/Tpm2Session.c   |  6 -
>  7 files changed, 107 insertions(+), 10 deletions(-)
> 
> diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> index 79e80fb7a9..42afe107a6 100644
> --- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> +++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> @@ -1,9 +1,9 @@
>  /** @file
>Implement TPM2 Capability related command.
> 
> -Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved. 
> +Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved. 
>  This program and the accompanying materials  are licensed and made
> available under the terms and conditions of the BSD License  which
> accompanies this distribution.  The full text of the license may be found at
> http://opensource.org/licenses/bsd-license.php
> 
> @@ -110,10 +110,18 @@ Tpm2GetCapability (
> 
>if (RecvBufferSize <= sizeof (TPM2_RESPONSE_HEADER) + sizeof (UINT8)) {
>  return EFI_DEVICE_ERROR;
>}
> 
> +  //
> +  // Fail if command failed
> +  //
> +  if (SwapBytes32(RecvBuffer.Header.responseCode) != TPM_RC_SUCCESS) {
> +DEBUG ((EFI_D_ERROR, "Tpm2GetCapability: Response Code error!
> 0x%08x\r\n",