Re: [edk2] [PATCH 1/1] ShellPkg/UefiShellLevel2CommandsLib: Fix "cp" for GCC systems

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 

Regards,
Ray

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Thomas 
>Palmer
>Sent: Wednesday, February 1, 2017 11:35 AM
>To: edk2-devel@lists.01.org
>Cc: Ni, Ruiyu 
>Subject: [edk2] [PATCH 1/1] ShellPkg/UefiShellLevel2CommandsLib: Fix "cp" for 
>GCC systems
>
>On GCC built UEFIs, the "cp" command fails with "The source and
>destination are the same".
>
>The root cause is that StrniCmp failed to detect a case-
>insensitive string difference due to the InternalCharToUpper function
>being improperly declared.
>
>Fix is to add missing EFIAPI token to match the original
>implementation.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Thomas Palmer 
>---
> .../Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git 
>a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c
>b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c
>index 7948e53cfc46..4dbff4de26f8 100644
>--- a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c
>+++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c
>@@ -21,6 +21,7 @@
>
>   * functions are non-interactive only
>
>+  (C) Copyright 2017 Hewlett Packard Enterprise Development LP
>   Copyright (c) 2014 Hewlett-Packard Development Company, L.P.
>   Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
>   This program and the accompanying materials
>@@ -268,6 +269,7 @@ VerifyIntermediateDirectories (
>   @return Char as an upper case character.
> **/
> CHAR16
>+EFIAPI
> InternalCharToUpper (
>   IN CONST CHAR16Char
>   );
>--
>2.7.4
>
>___
>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 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros

[Laszlo Ersek]

On 02/03/17 04:38, Gao, Liming wrote:
> Ray:
>   How about wrap them by macro DISABLE_NEW_DEPRECATED_INTERFACES to avoid 
> them be used any more?

OvmfPkg uses some of these macros, and also defines
DISABLE_NEW_DEPRECATED_INTERFACES. So if the above suggestion is
followed, then a conversion patch for OvmfPkg becomes necessary as first
step, to the new values.

Thanks
Laszlo

> 
> Thanks
> Liming
>> -Original Message-
>> From: Ni, Ruiyu
>> Sent: Thursday, January 26, 2017 2:09 PM
>> To: edk2-devel@lists.01.org
>> Cc: Gao, Liming ; Fan, Jeff 
>> Subject: [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
>> IncompatiblePciDevice macros
>>
>> DEVICE_ID_NOCARE is defined as 0x but Spec says (UINT64) -1
>> should be used to match any VendorId/DeviceId/RevisionId/
>> SubsystemVendorId/SubsystemDeviceId.
>>
>> PCI_BAR_OLD_ALIGN/PCI_BAR_EVEN_ALIGN/PCI_BAR_SQUAD_ALIGN/
>> PCI_BAR_DQUAD_ALIGN are defined but Spec doesn't have such
>> definitions.
>>
>> PCI_BAR_ALL is defined as 0xFF but Spec says (UINT64) -1 should be
>> used to match all BARs.
>>
>> All of the above macros are marked as deprecated.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ruiyu Ni 
>> Cc: Liming Gao 
>> Cc: Jeff Fan 
>> ---
>> MdePkg/Include/IndustryStandard/Pci22.h | 16 ++--
>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/MdePkg/Include/IndustryStandard/Pci22.h
>> b/MdePkg/Include/IndustryStandard/Pci22.h
>> index 4cf8389..8f87b04 100644
>> --- a/MdePkg/Include/IndustryStandard/Pci22.h
>> +++ b/MdePkg/Include/IndustryStandard/Pci22.h
>> @@ -780,14 +780,18 @@ typedef struct {
>>   ///
>> } EFI_PCI_CAPABILITY_HOTPLUG;
>>
>> -#define DEVICE_ID_NOCARE0x
>> +///
>> +/// Below macros (till PCI_BAR_ALL) were used by
>> EfiIncompatiblePciDeviceSupport Protocol.
>> +/// Some of them don't match Spec or are not defined in Spec. Those are
>> marked as deprecated.
>> +///
>> +#define DEVICE_ID_NOCARE0x  ///< Deprecated. Value doesn't
>> match Spec.
>>
>> #define PCI_ACPI_UNUSED 0
>> #define PCI_BAR_NOCHANGE0
>> -#define PCI_BAR_OLD_ALIGN   0xULL
>> -#define PCI_BAR_EVEN_ALIGN  0xFFFEULL
>> -#define PCI_BAR_SQUAD_ALIGN 0xFFFDULL
>> -#define PCI_BAR_DQUAD_ALIGN 0xFFFCULL
>> +#define PCI_BAR_OLD_ALIGN   0xULL  ///< Deprecated.
>> Value isn't defined in Spec.
>> +#define PCI_BAR_EVEN_ALIGN  0xFFFEULL  ///< Deprecated.
>> Value isn't defined in Spec.
>> +#define PCI_BAR_SQUAD_ALIGN 0xFFFDULL  ///< Deprecated.
>> Value isn't defined in Spec.
>> +#define PCI_BAR_DQUAD_ALIGN 0xFFFCULL  ///< Deprecated.
>> Value isn't defined in Spec.
>>
>> #define PCI_BAR_IDX00x00
>> #define PCI_BAR_IDX10x01
>> @@ -795,7 +799,7 @@ typedef struct {
>> #define PCI_BAR_IDX30x03
>> #define PCI_BAR_IDX40x04
>> #define PCI_BAR_IDX50x05
>> -#define PCI_BAR_ALL 0xFF
>> +#define PCI_BAR_ALL 0xFF///< Deprecated. Value doesn't match 
>> Spec.
>>
>> ///
>> /// EFI PCI Option ROM definitions
>> --
>> 2.9.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

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

Re: [edk2] [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros

[Ni, Ruiyu]

Laszlo,
Sure I will make sure OVMF build is fine with this change.

Liming,
The three deprecated macros PCI_BAR_EVEN_ALIGN,
PCI_BAR_SQUAD_ALIGN and PCI_BAR_DQUAD_ALIGN don't have replacement.
If wrapping them with DISABLE_NEW_DEPRECATED_INTERFACES,
MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe build will break.
What's your opinion about this driver?


Thanks/Ray

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Friday, February 3, 2017 4:13 PM
> To: Gao, Liming ; Ni, Ruiyu ;
> edk2-devel@lists.01.org 
> Cc: Fan, Jeff 
> Subject: Re: [edk2] [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
> IncompatiblePciDevice macros
> 
> On 02/03/17 04:38, Gao, Liming wrote:
> > Ray:
> >   How about wrap them by macro
> DISABLE_NEW_DEPRECATED_INTERFACES to avoid them be used any more?
> 
> OvmfPkg uses some of these macros, and also defines
> DISABLE_NEW_DEPRECATED_INTERFACES. So if the above suggestion is
> followed, then a conversion patch for OvmfPkg becomes necessary as first
> step, to the new values.
> 
> Thanks
> Laszlo
> 
> >
> > Thanks
> > Liming
> >> -Original Message-
> >> From: Ni, Ruiyu
> >> Sent: Thursday, January 26, 2017 2:09 PM
> >> To: edk2-devel@lists.01.org
> >> Cc: Gao, Liming ; Fan, Jeff
> >> 
> >> Subject: [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
> >> IncompatiblePciDevice macros
> >>
> >> DEVICE_ID_NOCARE is defined as 0x but Spec says (UINT64) -1
> >> should be used to match any VendorId/DeviceId/RevisionId/
> >> SubsystemVendorId/SubsystemDeviceId.
> >>
> >> PCI_BAR_OLD_ALIGN/PCI_BAR_EVEN_ALIGN/PCI_BAR_SQUAD_ALIGN/
> >> PCI_BAR_DQUAD_ALIGN are defined but Spec doesn't have such
> >> definitions.
> >>
> >> PCI_BAR_ALL is defined as 0xFF but Spec says (UINT64) -1 should be
> >> used to match all BARs.
> >>
> >> All of the above macros are marked as deprecated.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Ruiyu Ni 
> >> Cc: Liming Gao 
> >> Cc: Jeff Fan 
> >> ---
> >> MdePkg/Include/IndustryStandard/Pci22.h | 16 ++--
> >> 1 file changed, 10 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/MdePkg/Include/IndustryStandard/Pci22.h
> >> b/MdePkg/Include/IndustryStandard/Pci22.h
> >> index 4cf8389..8f87b04 100644
> >> --- a/MdePkg/Include/IndustryStandard/Pci22.h
> >> +++ b/MdePkg/Include/IndustryStandard/Pci22.h
> >> @@ -780,14 +780,18 @@ typedef struct {
> >>   ///
> >> } EFI_PCI_CAPABILITY_HOTPLUG;
> >>
> >> -#define DEVICE_ID_NOCARE0x
> >> +///
> >> +/// Below macros (till PCI_BAR_ALL) were used by
> >> EfiIncompatiblePciDeviceSupport Protocol.
> >> +/// Some of them don't match Spec or are not defined in Spec. Those
> >> +are
> >> marked as deprecated.
> >> +///
> >> +#define DEVICE_ID_NOCARE0x  ///< Deprecated. Value doesn't
> >> match Spec.
> >>
> >> #define PCI_ACPI_UNUSED 0
> >> #define PCI_BAR_NOCHANGE0
> >> -#define PCI_BAR_OLD_ALIGN   0xULL
> >> -#define PCI_BAR_EVEN_ALIGN  0xFFFEULL -#define
> >> PCI_BAR_SQUAD_ALIGN 0xFFFDULL -#define
> >> PCI_BAR_DQUAD_ALIGN 0xFFFCULL
> >> +#define PCI_BAR_OLD_ALIGN   0xULL  ///<
> Deprecated.
> >> Value isn't defined in Spec.
> >> +#define PCI_BAR_EVEN_ALIGN  0xFFFEULL  ///<
> Deprecated.
> >> Value isn't defined in Spec.
> >> +#define PCI_BAR_SQUAD_ALIGN 0xFFFDULL  ///<
> Deprecated.
> >> Value isn't defined in Spec.
> >> +#define PCI_BAR_DQUAD_ALIGN 0xFFFCULL  ///<
> Deprecated.
> >> Value isn't defined in Spec.
> >>
> >> #define PCI_BAR_IDX00x00
> >> #define PCI_BAR_IDX10x01
> >> @@ -795,7 +799,7 @@ typedef struct {
> >> #define PCI_BAR_IDX30x03
> >> #define PCI_BAR_IDX40x04
> >> #define PCI_BAR_IDX50x05
> >> -#define PCI_BAR_ALL 0xFF
> >> +#define PCI_BAR_ALL 0xFF///< Deprecated. Value doesn't match
> Spec.
> >>
> >> ///
> >> /// EFI PCI Option ROM definitions
> >> --
> >> 2.9.0.windows.1
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> >

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

Re: [edk2] [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros

[Gao, Liming]

Ray:
  If they are not recommended to be used any longer, I suggest to update 
IncompatiblePciDeviceSupportDxe driver not use them. The driver can have its 
incompatible definition. 

Thanks
Liming
>-Original Message-
>From: Ni, Ruiyu
>Sent: Friday, February 03, 2017 4:29 PM
>To: Laszlo Ersek ; Gao, Liming ;
>edk2-devel@lists.01.org 
>Cc: Fan, Jeff 
>Subject: RE: [edk2] [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
>IncompatiblePciDevice macros
>
>Laszlo,
>Sure I will make sure OVMF build is fine with this change.
>
>Liming,
>The three deprecated macros PCI_BAR_EVEN_ALIGN,
>PCI_BAR_SQUAD_ALIGN and PCI_BAR_DQUAD_ALIGN don't have
>replacement.
>If wrapping them with DISABLE_NEW_DEPRECATED_INTERFACES,
>MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe build will break.
>What's your opinion about this driver?
>
>
>Thanks/Ray
>
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Friday, February 3, 2017 4:13 PM
>> To: Gao, Liming ; Ni, Ruiyu ;
>> edk2-devel@lists.01.org 
>> Cc: Fan, Jeff 
>> Subject: Re: [edk2] [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
>> IncompatiblePciDevice macros
>>
>> On 02/03/17 04:38, Gao, Liming wrote:
>> > Ray:
>> >   How about wrap them by macro
>> DISABLE_NEW_DEPRECATED_INTERFACES to avoid them be used any more?
>>
>> OvmfPkg uses some of these macros, and also defines
>> DISABLE_NEW_DEPRECATED_INTERFACES. So if the above suggestion is
>> followed, then a conversion patch for OvmfPkg becomes necessary as first
>> step, to the new values.
>>
>> Thanks
>> Laszlo
>>
>> >
>> > Thanks
>> > Liming
>> >> -Original Message-
>> >> From: Ni, Ruiyu
>> >> Sent: Thursday, January 26, 2017 2:09 PM
>> >> To: edk2-devel@lists.01.org
>> >> Cc: Gao, Liming ; Fan, Jeff
>> >> 
>> >> Subject: [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
>> >> IncompatiblePciDevice macros
>> >>
>> >> DEVICE_ID_NOCARE is defined as 0x but Spec says (UINT64) -1
>> >> should be used to match any VendorId/DeviceId/RevisionId/
>> >> SubsystemVendorId/SubsystemDeviceId.
>> >>
>> >> PCI_BAR_OLD_ALIGN/PCI_BAR_EVEN_ALIGN/PCI_BAR_SQUAD_ALIGN/
>> >> PCI_BAR_DQUAD_ALIGN are defined but Spec doesn't have such
>> >> definitions.
>> >>
>> >> PCI_BAR_ALL is defined as 0xFF but Spec says (UINT64) -1 should be
>> >> used to match all BARs.
>> >>
>> >> All of the above macros are marked as deprecated.
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.0
>> >> Signed-off-by: Ruiyu Ni 
>> >> Cc: Liming Gao 
>> >> Cc: Jeff Fan 
>> >> ---
>> >> MdePkg/Include/IndustryStandard/Pci22.h | 16 ++--
>> >> 1 file changed, 10 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/MdePkg/Include/IndustryStandard/Pci22.h
>> >> b/MdePkg/Include/IndustryStandard/Pci22.h
>> >> index 4cf8389..8f87b04 100644
>> >> --- a/MdePkg/Include/IndustryStandard/Pci22.h
>> >> +++ b/MdePkg/Include/IndustryStandard/Pci22.h
>> >> @@ -780,14 +780,18 @@ typedef struct {
>> >>   ///
>> >> } EFI_PCI_CAPABILITY_HOTPLUG;
>> >>
>> >> -#define DEVICE_ID_NOCARE0x
>> >> +///
>> >> +/// Below macros (till PCI_BAR_ALL) were used by
>> >> EfiIncompatiblePciDeviceSupport Protocol.
>> >> +/// Some of them don't match Spec or are not defined in Spec. Those
>> >> +are
>> >> marked as deprecated.
>> >> +///
>> >> +#define DEVICE_ID_NOCARE0x  ///< Deprecated. Value doesn't
>> >> match Spec.
>> >>
>> >> #define PCI_ACPI_UNUSED 0
>> >> #define PCI_BAR_NOCHANGE0
>> >> -#define PCI_BAR_OLD_ALIGN   0xULL
>> >> -#define PCI_BAR_EVEN_ALIGN  0xFFFEULL -#define
>> >> PCI_BAR_SQUAD_ALIGN 0xFFFDULL -#define
>> >> PCI_BAR_DQUAD_ALIGN 0xFFFCULL
>> >> +#define PCI_BAR_OLD_ALIGN   0xULL  ///<
>> Deprecated.
>> >> Value isn't defined in Spec.
>> >> +#define PCI_BAR_EVEN_ALIGN  0xFFFEULL  ///<
>> Deprecated.
>> >> Value isn't defined in Spec.
>> >> +#define PCI_BAR_SQUAD_ALIGN 0xFFFDULL  ///<
>> Deprecated.
>> >> Value isn't defined in Spec.
>> >> +#define PCI_BAR_DQUAD_ALIGN 0xFFFCULL  ///<
>> Deprecated.
>> >> Value isn't defined in Spec.
>> >>
>> >> #define PCI_BAR_IDX00x00
>> >> #define PCI_BAR_IDX10x01
>> >> @@ -795,7 +799,7 @@ typedef struct {
>> >> #define PCI_BAR_IDX30x03
>> >> #define PCI_BAR_IDX40x04
>> >> #define PCI_BAR_IDX50x05
>> >> -#define PCI_BAR_ALL 0xFF
>> >> +#define PCI_BAR_ALL 0xFF///< Deprecated. Value doesn't match
>> Spec.
>> >>
>> >> ///
>> >> /// EFI PCI Option ROM definitions
>> >> --
>> >> 2.9.0.windows.1
>> >
>> > ___
>> > edk2-devel mailing list
>> > edk2-devel@lists.01.org
>> > https://lists.01.org/mailman/listinfo/edk2-devel
>> >

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

Re: [edk2] [PATCH 3/3] MdeModulePkg/DxeCore: Add UEFI image protection.

[Gao, Liming]

Jiewen:
  I have one comment. If VS linker option /MERGE:.data=.text is set, the 
generated PE image will have one .text section. If this section is set to RO, 
its data will be RO. Does this patch specially handle this case? 

Thanks
Liming
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Jiewen Yao
>Sent: Thursday, January 26, 2017 4:39 PM
>To: edk2-devel@lists.01.org
>Cc: Kinney, Michael D ; Tian, Feng
>; Zeng, Star 
>Subject: [edk2] [PATCH 3/3] MdeModulePkg/DxeCore: Add UEFI image
>protection.
>
>If the UEFI image is page aligned, the image code section is set to read
>only and the image data section is set to non-executable.
>
>1) This policy is applied for all UEFI image including boot service driver,
>runtime driver or application.
>2) This policy is applied only if the UEFI image meets the page alignment
>requirement.
>3) This policy is applied only if the UEFI image is from firmware volume.
>If the image is from OPROM, file system, or network, this policy is not
>applied, even if the image meets the page alignment requirement.
>4) This policy is not applied to the non-PE image region.
>
>The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
>the image. If the CpuArch protocol is not installed yet, the DxeCore
>enqueues the protection request. Once the CpuArch is installed, the
>DxeCore dequeues the protection request and applies policy.
>
>Once the image is unloaded, the protection is removed automatically.
>
>Cc: Star Zeng 
>Cc: Feng Tian 
>Cc: Michael Kinney 
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Jiewen Yao 
>---
> MdeModulePkg/Core/Dxe/DxeMain.h   |  53 ++
> MdeModulePkg/Core/Dxe/DxeMain.inf |   2 +
> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c   |   1 +
> MdeModulePkg/Core/Dxe/Image/Image.c   |   5 +
> MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 822
>
> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c  |  22 -
> 6 files changed, 883 insertions(+), 22 deletions(-)
>
>diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h
>b/MdeModulePkg/Core/Dxe/DxeMain.h
>index ae35fbb..67b5a5a 100644
>--- a/MdeModulePkg/Core/Dxe/DxeMain.h
>+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
>@@ -267,6 +267,26 @@ typedef struct {
> #define LOADED_IMAGE_PRIVATE_DATA_FROM_THIS(a) \
>   CR(a, LOADED_IMAGE_PRIVATE_DATA, Info,
>LOADED_IMAGE_PRIVATE_DATA_SIGNATURE)
>
>+#define IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE
>SIGNATURE_32 ('I','P','R','C')
>+
>+typedef struct {
>+  UINT32 Signature;
>+  LIST_ENTRY Link;
>+  EFI_PHYSICAL_ADDRESS   CodeSegmentBase;
>+  UINT64 CodeSegmentSize;
>+} IMAGE_PROPERTIES_RECORD_CODE_SECTION;
>+
>+#define IMAGE_PROPERTIES_RECORD_SIGNATURE SIGNATURE_32
>('I','P','R','D')
>+
>+typedef struct {
>+  UINT32 Signature;
>+  LIST_ENTRY Link;
>+  EFI_PHYSICAL_ADDRESS   ImageBase;
>+  UINT64 ImageSize;
>+  UINTN  CodeSegmentCount;
>+  LIST_ENTRY CodeSegmentList;
>+} IMAGE_PROPERTIES_RECORD;
>+
> //
> // DXE Core Global Variables
> //
>@@ -2859,6 +2879,15 @@ CoreInitializeMemoryAttributesTable (
>   );
>
> /**
>+  Initialize Memory Protection support.
>+**/
>+VOID
>+EFIAPI
>+CoreInitializeMemoryProtection (
>+  VOID
>+  );
>+
>+/**
>   Install MemoryAttributesTable on memory allocation.
>
>   @param[in] MemoryType EFI memory type.
>@@ -2888,4 +2917,28 @@ RemoveImageRecord (
>   IN EFI_RUNTIME_IMAGE_ENTRY  *RuntimeImage
>   );
>
>+/**
>+  Protect UEFI image.
>+
>+  @param[in]  LoadedImage  The loaded image protocol
>+  @param[in]  LoadedImageDevicePathThe loaded image device path
>protocol
>+**/
>+VOID
>+ProtectUefiImage (
>+  IN EFI_LOADED_IMAGE_PROTOCOL   *LoadedImage,
>+  IN EFI_DEVICE_PATH_PROTOCOL*LoadedImageDevicePath
>+  );
>+
>+/**
>+  Unprotect UEFI image.
>+
>+  @param[in]  LoadedImage  The loaded image protocol
>+  @param[in]  LoadedImageDevicePathThe loaded image device path
>protocol
>+**/
>+VOID
>+UnprotectUefiImage (
>+  IN EFI_LOADED_IMAGE_PROTOCOL   *LoadedImage,
>+  IN EFI_DEVICE_PATH_PROTOCOL*LoadedImageDevicePath
>+  );
>+
> #endif
>diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
>b/MdeModulePkg/Core/Dxe/DxeMain.inf
>index 13a2381..e62bc42 100644
>--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
>+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
>@@ -42,6 +42,7 @@
>   Misc/InstallConfigurationTable.c
>   Misc/PropertiesTable.c
>   Misc/MemoryAttributesTable.c
>+  Misc/MemoryProtection.c
>   Library/Library.c
>   Hand/DriverSupport.c
>   Hand/Notify.c
>@@ -159,6 +160,7 @@
>   gEfiHiiPackageListProtocolGuid## SOMETIMES_PRODUCES
>   gEfiEbcProtocolGuid   ## SOMETIMES_CONSUMES
>   gEfiSmmBase2ProtocolGuid  ## SOMETIMES_CONSUMES
>+  gEfiBlockIoProtocolGuid   ## SOMETIMES_CONSUMES
>
>   # Arch Protocols
>   gEfiBdsArchProtocolGuid

Re: [edk2] [PATCH 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting.

[Gao, Liming]

Jiewen:
  This patch adds PageTableLib implementation into CpuDxe driver. The purpose 
of this change is to update SetMemoryAttributes() API in CpuDxe driver. If so, 
new function AssignMemoryPageAttributes() will be enough. Other 
SetMemoryPageAttributes(),ClearMemoryPageAttributes() and 
GetMemoryPageAttributes() are not required. If they are useful, you can 
separate PageTableLib implementation from this patch. 

Thanks
Liming
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Jiewen Yao
>Sent: Thursday, January 26, 2017 4:39 PM
>To: edk2-devel@lists.01.org
>Cc: Kinney, Michael D ; Fan, Jeff
>
>Subject: [edk2] [PATCH 1/3] UefiCpuPkg/CpuDxe: Add memory attribute
>setting.
>
>Add memory attribute setting in CpuArch protocol.
>Previous SetMemoryAttributes() API only supports cache attribute setting.
>
>This patch updated SetMemoryAttributes() API to support memory attribute
>setting by updating CPU page table.
>
>Cc: Jeff Fan 
>Cc: Michael Kinney 
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Jiewen Yao 
>---
> UefiCpuPkg/CpuDxe/CpuDxe.c | 137 +--
> UefiCpuPkg/CpuDxe/CpuDxe.inf   |   4 +-
> UefiCpuPkg/CpuDxe/PageTableLib.h   | 204 
> UefiCpuPkg/CpuDxe/PageTableLibX86Pae.c | 997 
> 4 files changed, 1283 insertions(+), 59 deletions(-)
>
>diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
>index f6d0a67..4a1d27e 100644
>--- a/UefiCpuPkg/CpuDxe/CpuDxe.c
>+++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
>@@ -14,6 +14,10 @@
>
> #include "CpuDxe.h"
> #include "CpuMp.h"
>+#include "PageTableLib.h"
>+
>+#define CACHE_ATTRIBUTE_MASK   (EFI_MEMORY_UC | EFI_MEMORY_WC
>| EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE |
>EFI_MEMORY_WP)
>+#define MEMORY_ATTRIBUTE_MASK  (EFI_MEMORY_RP |
>EFI_MEMORY_XP | EFI_MEMORY_RO)
>
> //
> // Global Variables
>@@ -368,10 +372,9 @@ CpuSetMemoryAttributes (
>   EFI_STATUSMpStatus;
>   EFI_MP_SERVICES_PROTOCOL  *MpService;
>   MTRR_SETTINGS MtrrSettings;
>-
>-  if (!IsMtrrSupported ()) {
>-return EFI_UNSUPPORTED;
>-  }
>+  UINT64CacheAttributes;
>+  UINT64MemoryAttributes;
>+  MTRR_MEMORY_CACHE_TYPECurrentCacheType;
>
>   //
>   // If this function is called because GCD SetMemorySpaceAttributes () is
>called
>@@ -384,69 +387,87 @@ CpuSetMemoryAttributes (
> return EFI_SUCCESS;
>   }
>
>-  switch (Attributes) {
>-  case EFI_MEMORY_UC:
>-CacheType = CacheUncacheable;
>-break;
>
>-  case EFI_MEMORY_WC:
>-CacheType = CacheWriteCombining;
>-break;
>+  CacheAttributes = Attributes & CACHE_ATTRIBUTE_MASK;
>+  MemoryAttributes = Attributes & MEMORY_ATTRIBUTE_MASK;
>
>-  case EFI_MEMORY_WT:
>-CacheType = CacheWriteThrough;
>-break;
>+  if (Attributes != (CacheAttributes | MemoryAttributes)) {
>+return EFI_INVALID_PARAMETER;
>+  }
>
>-  case EFI_MEMORY_WP:
>-CacheType = CacheWriteProtected;
>-break;
>+  if (CacheAttributes != 0) {
>+if (!IsMtrrSupported ()) {
>+  return EFI_UNSUPPORTED;
>+}
>
>-  case EFI_MEMORY_WB:
>-CacheType = CacheWriteBack;
>-break;
>+switch (CacheAttributes) {
>+case EFI_MEMORY_UC:
>+  CacheType = CacheUncacheable;
>+  break;
>
>-  case EFI_MEMORY_UCE:
>-  case EFI_MEMORY_RP:
>-  case EFI_MEMORY_XP:
>-  case EFI_MEMORY_RUNTIME:
>-return EFI_UNSUPPORTED;
>+case EFI_MEMORY_WC:
>+  CacheType = CacheWriteCombining;
>+  break;
>
>-  default:
>-return EFI_INVALID_PARAMETER;
>-  }
>-  //
>-  // call MTRR libary function
>-  //
>-  Status = MtrrSetMemoryAttribute (
>- BaseAddress,
>- Length,
>- CacheType
>- );
>+case EFI_MEMORY_WT:
>+  CacheType = CacheWriteThrough;
>+  break;
>
>-  if (!RETURN_ERROR (Status)) {
>-MpStatus = gBS->LocateProtocol (
>-  &gEfiMpServiceProtocolGuid,
>-  NULL,
>-  (VOID **)&MpService
>-  );
>-//
>-// Synchronize the update with all APs
>-//
>-if (!EFI_ERROR (MpStatus)) {
>-  MtrrGetAllMtrrs (&MtrrSettings);
>-  MpStatus = MpService->StartupAllAPs (
>-  MpService,  // This
>-  SetMtrrsFromBuffer, // Procedure
>-  FALSE,  // SingleThread
>-  NULL,   // WaitEvent
>-  0,  // TimeoutInMicrosecsond
>-  &MtrrSettings,  // ProcedureArgument
>-  NULL// FailedCpuList
>-  );
>-  ASSERT (MpStatus == EFI_SUCCESS || MpStatus == EFI_NOT_STARTED);
>+case EFI_MEMORY_WP:
>+  CacheType = CacheWriteProtected;
>+  break;
>+
>+case EFI_MEMORY_WB:
>+  CacheType = CacheWriteBack;
>+  break;
>+
>+default

Re: [edk2] [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros

[Ni, Ruiyu]

yes. That's a good idea to move the definitions to local.

Regards,
Ray

From: Gao, Liming
Sent: Friday, February 3, 2017 4:36 PM
To: Ni, Ruiyu ; Laszlo Ersek ; 
edk2-devel@lists.01.org 
Cc: Fan, Jeff 
Subject: RE: [edk2] [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec 
IncompatiblePciDevice macros

Ray:
  If they are not recommended to be used any longer, I suggest to update 
IncompatiblePciDeviceSupportDxe driver not use them. The driver can have its 
incompatible definition.

Thanks
Liming
>-Original Message-
>From: Ni, Ruiyu
>Sent: Friday, February 03, 2017 4:29 PM
>To: Laszlo Ersek mailto:ler...@redhat.com>>; Gao, Liming 
>mailto:liming@intel.com>>;
>edk2-devel@lists.01.org 
>mailto:edk2-de...@ml01.01.org>>
>Cc: Fan, Jeff mailto:jeff@intel.com>>
>Subject: RE: [edk2] [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
>IncompatiblePciDevice macros
>
>Laszlo,
>Sure I will make sure OVMF build is fine with this change.
>
>Liming,
>The three deprecated macros PCI_BAR_EVEN_ALIGN,
>PCI_BAR_SQUAD_ALIGN and PCI_BAR_DQUAD_ALIGN don't have
>replacement.
>If wrapping them with DISABLE_NEW_DEPRECATED_INTERFACES,
>MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe build will break.
>What's your opinion about this driver?
>
>
>Thanks/Ray
>
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Friday, February 3, 2017 4:13 PM
>> To: Gao, Liming mailto:liming@intel.com>>; Ni, 
>> Ruiyu mailto:ruiyu...@intel.com>>;
>> edk2-devel@lists.01.org 
>> mailto:edk2-de...@ml01.01.org>>
>> Cc: Fan, Jeff mailto:jeff@intel.com>>
>> Subject: Re: [edk2] [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
>> IncompatiblePciDevice macros
>>
>> On 02/03/17 04:38, Gao, Liming wrote:
>> > Ray:
>> >   How about wrap them by macro
>> DISABLE_NEW_DEPRECATED_INTERFACES to avoid them be used any more?
>>
>> OvmfPkg uses some of these macros, and also defines
>> DISABLE_NEW_DEPRECATED_INTERFACES. So if the above suggestion is
>> followed, then a conversion patch for OvmfPkg becomes necessary as first
>> step, to the new values.
>>
>> Thanks
>> Laszlo
>>
>> >
>> > Thanks
>> > Liming
>> >> -Original Message-
>> >> From: Ni, Ruiyu
>> >> Sent: Thursday, January 26, 2017 2:09 PM
>> >> To: edk2-devel@lists.01.org
>> >> Cc: Gao, Liming mailto:liming@intel.com>>; Fan, 
>> >> Jeff
>> >> mailto:jeff@intel.com>>
>> >> Subject: [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
>> >> IncompatiblePciDevice macros
>> >>
>> >> DEVICE_ID_NOCARE is defined as 0x but Spec says (UINT64) -1
>> >> should be used to match any VendorId/DeviceId/RevisionId/
>> >> SubsystemVendorId/SubsystemDeviceId.
>> >>
>> >> PCI_BAR_OLD_ALIGN/PCI_BAR_EVEN_ALIGN/PCI_BAR_SQUAD_ALIGN/
>> >> PCI_BAR_DQUAD_ALIGN are defined but Spec doesn't have such
>> >> definitions.
>> >>
>> >> PCI_BAR_ALL is defined as 0xFF but Spec says (UINT64) -1 should be
>> >> used to match all BARs.
>> >>
>> >> All of the above macros are marked as deprecated.
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.0
>> >> Signed-off-by: Ruiyu Ni mailto:ruiyu...@intel.com>>
>> >> Cc: Liming Gao mailto:liming@intel.com>>
>> >> Cc: Jeff Fan mailto:jeff@intel.com>>
>> >> ---
>> >> MdePkg/Include/IndustryStandard/Pci22.h | 16 ++--
>> >> 1 file changed, 10 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/MdePkg/Include/IndustryStandard/Pci22.h
>> >> b/MdePkg/Include/IndustryStandard/Pci22.h
>> >> index 4cf8389..8f87b04 100644
>> >> --- a/MdePkg/Include/IndustryStandard/Pci22.h
>> >> +++ b/MdePkg/Include/IndustryStandard/Pci22.h
>> >> @@ -780,14 +780,18 @@ typedef struct {
>> >>   ///
>> >> } EFI_PCI_CAPABILITY_HOTPLUG;
>> >>
>> >> -#define DEVICE_ID_NOCARE0x
>> >> +///
>> >> +/// Below macros (till PCI_BAR_ALL) were used by
>> >> EfiIncompatiblePciDeviceSupport Protocol.
>> >> +/// Some of them don't match Spec or are not defined in Spec. Those
>> >> +are
>> >> marked as deprecated.
>> >> +///
>> >> +#define DEVICE_ID_NOCARE0x  ///< Deprecated. Value doesn't
>> >> match Spec.
>> >>
>> >> #define PCI_ACPI_UNUSED 0
>> >> #define PCI_BAR_NOCHANGE0
>> >> -#define PCI_BAR_OLD_ALIGN   0xULL
>> >> -#define PCI_BAR_EVEN_ALIGN  0xFFFEULL -#define
>> >> PCI_BAR_SQUAD_ALIGN 0xFFFDULL -#define
>> >> PCI_BAR_DQUAD_ALIGN 0xFFFCULL
>> >> +#define PCI_BAR_OLD_ALIGN   0xULL  ///<
>> Deprecated.
>> >> Value isn't defined in Spec.
>> >> +#define PCI_BAR_EVEN_ALIGN  0xFFFEULL  ///<
>> Deprecated.
>> >> Value isn't defined in Spec.
>> >> +#define PCI_BAR_SQUAD_ALIGN 0xFFFDULL  ///<
>> Deprecated.
>> >> Value isn't defined in Spec.
>> >> +#define PCI_BAR_DQUAD_ALIGN 0xFFFCULL  ///<
>> Deprecated.
>> >> Value isn't defined in Spec.
>> >>
>> >> #define PCI_BAR_IDX00x00
>> >> #define PCI_BAR_IDX1   

Re: [edk2] [PATCH v4 3/3] ShellPkg UefiDpLib: Remove TimerLib dependency

[Kinney, Michael D]

Jiewen,

If we do this, then we would also need to update all the 
platform DSC files that build the shell from sources too.

Maybe we should consider this in a different patch series?

Mike

> -Original Message-
> From: Yao, Jiewen
> Sent: Thursday, February 2, 2017 9:35 PM
> To: Kinney, Michael D ; edk2-devel@lists.01.org
> Cc: Andrew Fish ; Gao, Liming ; 
> Carsey, Jaben
> ; Zeng, Star 
> Subject: RE: [edk2] [PATCH v4 3/3] ShellPkg UefiDpLib: Remove TimerLib 
> dependency
> 
> I have one more suggestion.
> Can we use MdeModulePkg/Library/DxeSmmPerformanceLib/DxeSmmPerformanceLib.inf 
> as
> default one in DSC?
> 
> So that the binary built with default DSC can be used on any platform.
> 
> ShellPkg.dsc:
>ShellPkg/Library/UefiDpLib/UefiDpLib.inf {
>  
> -  
> TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf
> 
> PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf
>DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
>}
> 
> 
> Thank you
> Yao Jiewen
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Michael Kinney
> > Sent: Friday, February 3, 2017 12:56 PM
> > To: edk2-devel@lists.01.org
> > Cc: Andrew Fish ; Gao, Liming ; Yao,
> > Jiewen ; Carsey, Jaben ; Zeng,
> > Star 
> > Subject: [edk2] [PATCH v4 3/3] ShellPkg UefiDpLib: Remove TimerLib 
> > dependency
> >
> > From: Star Zeng 
> >
> > Current UefiDpLib implementation depends on TimerLib,
> > as different platforms may implement and use their
> > own TimerLib, it makes the dp command needs to be built
> > by platform. The TimerLib dependency can be removed by
> > using performance property configuration table to make
> > UefiDpLib to be generic.
> >
> > Cc: Andrew Fish 
> > Cc: Michael Kinney 
> > Cc: Liming Gao 
> > Cc: Jiewen Yao 
> > Cc: Cinnamon Shia 
> > Cc: Jaben Carsey 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Star Zeng 
> > ---
> >  ShellPkg/Library/UefiDpLib/Dp.c  | 29 +++--
> >  ShellPkg/Library/UefiDpLib/DpInternal.h  |  6 ++
> >  ShellPkg/Library/UefiDpLib/DpProfile.c   |  3 +--
> >  ShellPkg/Library/UefiDpLib/DpTrace.c | 25 ++---
> >  ShellPkg/Library/UefiDpLib/DpUtilities.c |  3 +--
> >  ShellPkg/Library/UefiDpLib/Literals.c|  3 +--
> >  ShellPkg/Library/UefiDpLib/UefiDpLib.inf |  6 --
> >  ShellPkg/Library/UefiDpLib/UefiDpLib.uni |  3 ++-
> >  ShellPkg/ShellPkg.dsc|  3 +--
> >  9 files changed, 25 insertions(+), 56 deletions(-)
> >
> > diff --git a/ShellPkg/Library/UefiDpLib/Dp.c 
> > b/ShellPkg/Library/UefiDpLib/Dp.c
> > index 75c7d11..444c136 100644
> > --- a/ShellPkg/Library/UefiDpLib/Dp.c
> > +++ b/ShellPkg/Library/UefiDpLib/Dp.c
> > @@ -13,7 +13,7 @@
> >Dp uses this information to group records in different ways.  It also 
> > uses
> >timer information to calculate elapsed time for each measurement.
> >
> > -  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
> > +  Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
> >(C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP
> >This program and the accompanying materials
> >are licensed and made available under the terms and conditions of the BSD
> > License
> > @@ -25,16 +25,10 @@
> >  **/
> >
> >  #include "UefiDpLib.h"
> > -#include 
> > -#include 
> > -#include 
> > -#include 
> > -
> >  #include 
> >  #include 
> >  #include 
> >  #include 
> > -#include 
> >  #include 
> >
> >  #include 
> > @@ -153,8 +147,7 @@ ShellCommandRunDp (
> >CONST CHAR16  *CmdLineArg;
> >EFI_STATUSStatus;
> >
> > -  UINT64Freq;
> > -  UINT64Ticker;
> > +  PERFORMANCE_PROPERTY  *PerformanceProperty;
> >UINTN Number2Display;
> >
> >EFI_STRINGStringPtr;
> > @@ -183,11 +176,6 @@ ShellCommandRunDp (
> >CustomCumulativeData = NULL;
> >ShellStatus = SHELL_SUCCESS;
> >
> > -  // Get DP's entry time as soon as possible.
> > -  // This is used as the Shell-Phase end time.
> > -  //
> > -  Ticker  = GetPerformanceCounter ();
> > -
> >//
> >// initialize the shell lib (we must be in non-auto-init...)
> >//
> > @@ -283,10 +271,15 @@ ShellCommandRunDp (
> >//StartCount = Value loaded into the counter when it starts counting
> >//  EndCount = Value counter counts to before it needs to be reset
> >//
> > -  Freq = GetPerformanceCounterProperties (&TimerInfo.StartCount,
> > &TimerInfo.EndCount);
> > +  Status = EfiGetSystemConfigurationTable (&gPerformanceProtocolGuid,
> > &PerformanceProperty);
> > +  if (EFI_ERROR (Status)) {
> > +ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_PERF_PROPERTY_NOT_FOUND), gDpHiiHandle);
> > +goto Done;
> > +  }
> >
> > -  // Convert the Frequency from Hz to KHz

Re: [edk2] [PATCH 1/1] ShellPkg/UefiShellLevel2CommandsLib: Fix "cp" for GCC systems

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ni,
> Ruiyu
> Sent: Friday, February 3, 2017 12:06 AM
> To: Thomas Palmer ; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH 1/1] ShellPkg/UefiShellLevel2CommandsLib: Fix "cp"
> for GCC systems
> Importance: High
> 
> Reviewed-by: Ruiyu Ni 
> 
> Regards,
> Ray
> 
> >-Original Message-
> >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> >Thomas Palmer
> >Sent: Wednesday, February 1, 2017 11:35 AM
> >To: edk2-devel@lists.01.org
> >Cc: Ni, Ruiyu 
> >Subject: [edk2] [PATCH 1/1] ShellPkg/UefiShellLevel2CommandsLib: Fix
> >"cp" for GCC systems
> >
> >On GCC built UEFIs, the "cp" command fails with "The source and
> >destination are the same".
> >
> >The root cause is that StrniCmp failed to detect a case- insensitive
> >string difference due to the InternalCharToUpper function being
> >improperly declared.
> >
> >Fix is to add missing EFIAPI token to match the original
> >implementation.
> >
> >Contributed-under: TianoCore Contribution Agreement 1.0
> >Signed-off-by: Thomas Palmer 
> >---
> > .../Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c | 2
> ++
> > 1 file changed, 2 insertions(+)
> >
> >diff --git
> >a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLi
> >b.c
> >b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLi
> >b.c index 7948e53cfc46..4dbff4de26f8 100644
> >---
> >a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLi
> >b.c
> >+++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Comman
> >+++ dsLib.c
> >@@ -21,6 +21,7 @@
> >
> >   * functions are non-interactive only
> >
> >+  (C) Copyright 2017 Hewlett Packard Enterprise Development LP
> >   Copyright (c) 2014 Hewlett-Packard Development Company, L.P.
> >   Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
> >   This program and the accompanying materials @@ -268,6 +269,7 @@
> >VerifyIntermediateDirectories (
> >   @return Char as an upper case character.
> > **/
> > CHAR16
> >+EFIAPI
> > InternalCharToUpper (
> >   IN CONST CHAR16Char
> >   );
> >--
> >2.7.4
> >
> >___
> >edk2-devel mailing list
> >edk2-devel@lists.01.org
> >https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v4 0/3] Remove TimerLib dependency from DP

[Carsey, Jaben]

I am good with this change.  I like the idea of changing platforms that build 
the shell from source in a separate patch series.

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Michael Kinney
> Sent: Thursday, February 2, 2017 8:56 PM
> To: edk2-devel@lists.01.org
> Cc: Andrew Fish ; Gao, Liming ; Yao,
> Jiewen ; Carsey, Jaben ;
> Zeng, Star 
> Subject: [edk2] [PATCH v4 0/3] Remove TimerLib dependency from DP
> Importance: High
> 
> Current DP implementation depends on TimerLib, as different platforms may
> implement and use their own TimerLib, it makes the dp needs to be built by
> platform. The TimerLib dependency can be removed by using performance
> property configuration table to make DP to be generic.
> 
> There was a discussion on edk2-devel about supporting use of different 
> TimerLib
> instances in different modules and updating the performance log to support
> measurements with different timer rates.  A complete solution for this has not
> been found.
> 
> This patch series that allows the DP command to get the timer rate from a
> system configuration table is useful on its own because it allows the DP shell
> command to be independent of the TimerLib used to collect the performance
> logs.
> 
> V3:
> Define and install performance property configuration table instead of
> extending PEI performance log HOB. As user may want to only dump DXE or
> SMM performance data, then PeiPerformanceLib will be not linked and PEI
> performance log HOB will be not built.
> 
> V4:
> Change name of field in PERFORMANCE_PROPERTY from CpuFreq to Frequency.
> 
> Cc: Andrew Fish 
> Cc: Michael Kinney 
> Cc: Liming Gao 
> Cc: Jiewen Yao 
> Cc: Cinnamon Shia 
> Cc: Jaben Carsey 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> 
> Star Zeng (3):
>   MdeModulePkg: Add performance property configuration table
>   PerformancePkg Dp_App: Remove TimerLib dependency
>   ShellPkg UefiDpLib: Remove TimerLib dependency
> 
>  MdeModulePkg/Include/Guid/Performance.h| 12 +++-
>  .../DxeCorePerformanceLib/DxeCorePerformanceLib.c  | 23 --
>  .../DxeCorePerformanceLib.inf  |  2 ++
>  .../DxeCorePerformanceLibInternal.h|  3 +-
>  .../SmmCorePerformanceLib/SmmCorePerformanceLib.c  | 18 +++
>  .../SmmCorePerformanceLib.inf  |  5 +++-
>  PerformancePkg/Dp_App/Dp.c | 35 
> +++---
>  PerformancePkg/Dp_App/Dp.inf   |  6 ++--
>  PerformancePkg/Dp_App/DpInternal.h |  6 ++--
>  PerformancePkg/Dp_App/DpProfile.c  |  3 +-
>  PerformancePkg/Dp_App/DpStrings.uni| 10 ---
>  PerformancePkg/Dp_App/DpTrace.c| 25 ++--
>  PerformancePkg/Dp_App/DpUtilities.c|  3 +-
>  PerformancePkg/Dp_App/Literals.c   |  3 +-
>  ShellPkg/Library/UefiDpLib/Dp.c| 29 +++---
>  ShellPkg/Library/UefiDpLib/DpInternal.h|  6 ++--
>  ShellPkg/Library/UefiDpLib/DpProfile.c |  3 +-
>  ShellPkg/Library/UefiDpLib/DpTrace.c   | 25 ++--
>  ShellPkg/Library/UefiDpLib/DpUtilities.c   |  3 +-
>  ShellPkg/Library/UefiDpLib/Literals.c  |  3 +-
>  ShellPkg/Library/UefiDpLib/UefiDpLib.inf   |  6 ++--
>  ShellPkg/Library/UefiDpLib/UefiDpLib.uni   |  3 +-
>  ShellPkg/ShellPkg.dsc  |  3 +-
>  23 files changed, 117 insertions(+), 118 deletions(-)
> 
> --
> 2.6.3.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

[edk2] UEFI Secure Technologies

[Bill Paul]

This is not strictly an EDK development question, but it may be the right 
audience to ask. The UEFI 2.5 specification introduced a section called Secure 
Technologies, which includes the definition for an EFI_PKCS7_VERIFY_PROTOCOL 
(among others).

My question is: what are the odds of this protocol being available in a given 
UEFI firmware build for a fielded system?

The context for this question has to do with how secure boot would be handled 
for OSes other than Windows. Obviously, once UEFI validates the BOOTxxx.EFI 
loader image, the next step would be for the boot loader to validate the OS 
image that comes after it, which requires the same kind of cryptographic 
signature validation that the UEFI firmware performs on loader. But the 
signature check is built into the BS->LoadImage() service and the firmware 
only knows how to check signatures on Microsoft PE/COFF images (signed 
according to the Microsoft Authenticode spec).

I'm assuming that somehow the Microsoft loader takes advantage of the fact 
that Windows executables (including the kernel and its DLLs) are also PE/COFF, 
and it somehow loads those with BS->LoadImage() too. That's great, if you're 
Microsoft.

But if you're not Microsoft, you can't use this strategy, which means your 
loader needs its own custom crypto code.

In theory the presence of EFI_PKCS7_VERIFY_PROTOCOL would mitigate this, but 
only on systems where the firmware includes it.

My concern is that since Windows doesn't depend on it, the odds of this 
protocol being included in a given build might be fairly slim. I'd like to 
hear some other (hopefully better-informed) opinions on this matter.

-Bill

-- 
=
-Bill Paul(510) 749-2329 | Senior Member of Technical Staff,
 wp...@windriver.com | Master of Unix-Fu - Wind River Systems
=
   "I put a dollar in a change machine. Nothing changed." - George Carlin
=
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] UEFI Secure Technologies

[Laszlo Ersek]

On 02/03/17 22:15, Bill Paul wrote:
> This is not strictly an EDK development question, but it may be the right 
> audience to ask. The UEFI 2.5 specification introduced a section called 
> Secure 
> Technologies, which includes the definition for an EFI_PKCS7_VERIFY_PROTOCOL 
> (among others).
> 
> My question is: what are the odds of this protocol being available in a given 
> UEFI firmware build for a fielded system?
> 
> The context for this question has to do with how secure boot would be handled 
> for OSes other than Windows. Obviously, once UEFI validates the BOOTxxx.EFI 
> loader image, the next step would be for the boot loader to validate the OS 
> image that comes after it, which requires the same kind of cryptographic 
> signature validation that the UEFI firmware performs on loader. But the 
> signature check is built into the BS->LoadImage() service and the firmware 
> only knows how to check signatures on Microsoft PE/COFF images (signed 
> according to the Microsoft Authenticode spec).
> 
> I'm assuming that somehow the Microsoft loader takes advantage of the fact 
> that Windows executables (including the kernel and its DLLs) are also 
> PE/COFF, 
> and it somehow loads those with BS->LoadImage() too. That's great, if you're 
> Microsoft.
> 
> But if you're not Microsoft, you can't use this strategy, which means your 
> loader needs its own custom crypto code.
> 
> In theory the presence of EFI_PKCS7_VERIFY_PROTOCOL would mitigate this, but 
> only on systems where the firmware includes it.
> 
> My concern is that since Windows doesn't depend on it, the odds of this 
> protocol being included in a given build might be fairly slim. I'd like to 
> hear some other (hopefully better-informed) opinions on this matter.

(Not overly well informed:)

- In the "Linux ecosystem", each stage of the boot has its own dedicated
crypto code, to my knowledge (shim, grub2, kernel (for signed kernel
modules)).

- Perhaps the next version of the spec should require that if a firmware
supports Secure Boot, then it expose EFI_PKCS7_VERIFY_PROTOCOL too?
(Just a random thought, not at all researched.)

Anyway, for runtime verification of drivers, modules etc, the kernel
will need its own crypto stuff.

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

Re: [edk2] [PATCH v4 3/3] ShellPkg UefiDpLib: Remove TimerLib dependency

[Yao, Jiewen]

Sure, it is OK to enable this in a different patch series.

The V4 series is good enough and I have given Reviewed-by.

Thank you
Yao Jiewen

From: Kinney, Michael D
Sent: Saturday, February 4, 2017 1:36 AM
To: Yao, Jiewen ; edk2-devel@lists.01.org; Kinney, 
Michael D 
Cc: Andrew Fish ; Gao, Liming ; Carsey, 
Jaben ; Zeng, Star 
Subject: RE: [edk2] [PATCH v4 3/3] ShellPkg UefiDpLib: Remove TimerLib 
dependency

Jiewen,

If we do this, then we would also need to update all the
platform DSC files that build the shell from sources too.

Maybe we should consider this in a different patch series?

Mike

> -Original Message-
> From: Yao, Jiewen
> Sent: Thursday, February 2, 2017 9:35 PM
> To: Kinney, Michael D 
> mailto:michael.d.kin...@intel.com>>; 
> edk2-devel@lists.01.org
> Cc: Andrew Fish mailto:af...@apple.com>>; Gao, Liming 
> mailto:liming@intel.com>>; Carsey, Jaben
> mailto:jaben.car...@intel.com>>; Zeng, Star 
> mailto:star.z...@intel.com>>
> Subject: RE: [edk2] [PATCH v4 3/3] ShellPkg UefiDpLib: Remove TimerLib 
> dependency
>
> I have one more suggestion.
> Can we use MdeModulePkg/Library/DxeSmmPerformanceLib/DxeSmmPerformanceLib.inf 
> as
> default one in DSC?
>
> So that the binary built with default DSC can be used on any platform.
>
> ShellPkg.dsc:
>ShellPkg/Library/UefiDpLib/UefiDpLib.inf {
>  
> -  
> TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf
>
> PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf
>DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
>}
>
>
> Thank you
> Yao Jiewen
>
>
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Michael Kinney
> > Sent: Friday, February 3, 2017 12:56 PM
> > To: edk2-devel@lists.01.org
> > Cc: Andrew Fish mailto:af...@apple.com>>; Gao, Liming 
> > mailto:liming@intel.com>>; Yao,
> > Jiewen mailto:jiewen@intel.com>>; Carsey, Jaben 
> > mailto:jaben.car...@intel.com>>; Zeng,
> > Star mailto:star.z...@intel.com>>
> > Subject: [edk2] [PATCH v4 3/3] ShellPkg UefiDpLib: Remove TimerLib 
> > dependency
> >
> > From: Star Zeng mailto:star.z...@intel.com>>
> >
> > Current UefiDpLib implementation depends on TimerLib,
> > as different platforms may implement and use their
> > own TimerLib, it makes the dp command needs to be built
> > by platform. The TimerLib dependency can be removed by
> > using performance property configuration table to make
> > UefiDpLib to be generic.
> >
> > Cc: Andrew Fish mailto:af...@apple.com>>
> > Cc: Michael Kinney 
> > mailto:michael.d.kin...@intel.com>>
> > Cc: Liming Gao mailto:liming@intel.com>>
> > Cc: Jiewen Yao mailto:jiewen@intel.com>>
> > Cc: Cinnamon Shia mailto:cinnamon.s...@hpe.com>>
> > Cc: Jaben Carsey mailto:jaben.car...@intel.com>>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Star Zeng mailto:star.z...@intel.com>>
> > ---
> >  ShellPkg/Library/UefiDpLib/Dp.c  | 29 +++--
> >  ShellPkg/Library/UefiDpLib/DpInternal.h  |  6 ++
> >  ShellPkg/Library/UefiDpLib/DpProfile.c   |  3 +--
> >  ShellPkg/Library/UefiDpLib/DpTrace.c | 25 ++---
> >  ShellPkg/Library/UefiDpLib/DpUtilities.c |  3 +--
> >  ShellPkg/Library/UefiDpLib/Literals.c|  3 +--
> >  ShellPkg/Library/UefiDpLib/UefiDpLib.inf |  6 --
> >  ShellPkg/Library/UefiDpLib/UefiDpLib.uni |  3 ++-
> >  ShellPkg/ShellPkg.dsc|  3 +--
> >  9 files changed, 25 insertions(+), 56 deletions(-)
> >
> > diff --git a/ShellPkg/Library/UefiDpLib/Dp.c 
> > b/ShellPkg/Library/UefiDpLib/Dp.c
> > index 75c7d11..444c136 100644
> > --- a/ShellPkg/Library/UefiDpLib/Dp.c
> > +++ b/ShellPkg/Library/UefiDpLib/Dp.c
> > @@ -13,7 +13,7 @@
> >Dp uses this information to group records in different ways.  It also 
> > uses
> >timer information to calculate elapsed time for each measurement.
> >
> > -  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
> > +  Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
> >(C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP
> >This program and the accompanying materials
> >are licensed and made available under the terms and conditions of the BSD
> > License
> > @@ -25,16 +25,10 @@
> >  **/
> >
> >  #include "UefiDpLib.h"
> > -#include 
> > -#include 
> > -#include 
> > -#include 
> > -
> >  #include 
> >  #include 
> >  #include 
> >  #include 
> > -#include 
> >  #include 
> >
> >  #include 
> > @@ -153,8 +147,7 @@ ShellCommandRunDp (
> >CONST CHAR16  *CmdLineArg;
> >EFI_STATUSStatus;
> >
> > -  UINT64Freq;
> > -  UINT64Ticker;
> > +  PERFORMANCE_PROPERTY  *PerformanceProperty;
> >UINTN Number2Display;
> >
> >EFI_STRING  

Re: [edk2] [PATCH 3/3] MdeModulePkg/DxeCore: Add UEFI image protection.

[Yao, Jiewen]

It is pre-requisite that code section and data section should not be not merged.
That is same criteria for SMM and Runtime driver.

The problem is that I am not able to see the difference. I can only get LINK 
warning below:
LINK : warning LNK4254: section '.data' (C040) merged into '.text' 
(6020) with different attributes
But final attribute in PE code section is same.

Thank you
Yao Jiewen

From: Gao, Liming
Sent: Friday, February 3, 2017 4:40 PM
To: Yao, Jiewen ; edk2-devel@lists.01.org
Cc: Kinney, Michael D ; Tian, Feng 
; Zeng, Star 
Subject: RE: [edk2] [PATCH 3/3] MdeModulePkg/DxeCore: Add UEFI image protection.

Jiewen:
  I have one comment. If VS linker option /MERGE:.data=.text is set, the 
generated PE image will have one .text section. If this section is set to RO, 
its data will be RO. Does this patch specially handle this case?

Thanks
Liming
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Jiewen Yao
>Sent: Thursday, January 26, 2017 4:39 PM
>To: edk2-devel@lists.01.org
>Cc: Kinney, Michael D 
>mailto:michael.d.kin...@intel.com>>; Tian, Feng
>mailto:feng.t...@intel.com>>; Zeng, Star 
>mailto:star.z...@intel.com>>
>Subject: [edk2] [PATCH 3/3] MdeModulePkg/DxeCore: Add UEFI image
>protection.
>
>If the UEFI image is page aligned, the image code section is set to read
>only and the image data section is set to non-executable.
>
>1) This policy is applied for all UEFI image including boot service driver,
>runtime driver or application.
>2) This policy is applied only if the UEFI image meets the page alignment
>requirement.
>3) This policy is applied only if the UEFI image is from firmware volume.
>If the image is from OPROM, file system, or network, this policy is not
>applied, even if the image meets the page alignment requirement.
>4) This policy is not applied to the non-PE image region.
>
>The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
>the image. If the CpuArch protocol is not installed yet, the DxeCore
>enqueues the protection request. Once the CpuArch is installed, the
>DxeCore dequeues the protection request and applies policy.
>
>Once the image is unloaded, the protection is removed automatically.
>
>Cc: Star Zeng mailto:star.z...@intel.com>>
>Cc: Feng Tian mailto:feng.t...@intel.com>>
>Cc: Michael Kinney 
>mailto:michael.d.kin...@intel.com>>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Jiewen Yao mailto:jiewen@intel.com>>
>---
> MdeModulePkg/Core/Dxe/DxeMain.h   |  53 ++
> MdeModulePkg/Core/Dxe/DxeMain.inf |   2 +
> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c   |   1 +
> MdeModulePkg/Core/Dxe/Image/Image.c   |   5 +
> MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 822
>
> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c  |  22 -
> 6 files changed, 883 insertions(+), 22 deletions(-)
>
>diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h
>b/MdeModulePkg/Core/Dxe/DxeMain.h
>index ae35fbb..67b5a5a 100644
>--- a/MdeModulePkg/Core/Dxe/DxeMain.h
>+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
>@@ -267,6 +267,26 @@ typedef struct {
> #define LOADED_IMAGE_PRIVATE_DATA_FROM_THIS(a) \
>   CR(a, LOADED_IMAGE_PRIVATE_DATA, Info,
>LOADED_IMAGE_PRIVATE_DATA_SIGNATURE)
>
>+#define IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE
>SIGNATURE_32 ('I','P','R','C')
>+
>+typedef struct {
>+  UINT32 Signature;
>+  LIST_ENTRY Link;
>+  EFI_PHYSICAL_ADDRESS   CodeSegmentBase;
>+  UINT64 CodeSegmentSize;
>+} IMAGE_PROPERTIES_RECORD_CODE_SECTION;
>+
>+#define IMAGE_PROPERTIES_RECORD_SIGNATURE SIGNATURE_32
>('I','P','R','D')
>+
>+typedef struct {
>+  UINT32 Signature;
>+  LIST_ENTRY Link;
>+  EFI_PHYSICAL_ADDRESS   ImageBase;
>+  UINT64 ImageSize;
>+  UINTN  CodeSegmentCount;
>+  LIST_ENTRY CodeSegmentList;
>+} IMAGE_PROPERTIES_RECORD;
>+
> //
> // DXE Core Global Variables
> //
>@@ -2859,6 +2879,15 @@ CoreInitializeMemoryAttributesTable (
>   );
>
> /**
>+  Initialize Memory Protection support.
>+**/
>+VOID
>+EFIAPI
>+CoreInitializeMemoryProtection (
>+  VOID
>+  );
>+
>+/**
>   Install MemoryAttributesTable on memory allocation.
>
>   @param[in] MemoryType EFI memory type.
>@@ -2888,4 +2917,28 @@ RemoveImageRecord (
>   IN EFI_RUNTIME_IMAGE_ENTRY  *RuntimeImage
>   );
>
>+/**
>+  Protect UEFI image.
>+
>+  @param[in]  LoadedImage  The loaded image protocol
>+  @param[in]  LoadedImageDevicePathThe loaded image device path
>protocol
>+**/
>+VOID
>+ProtectUefiImage (
>+  IN EFI_LOADED_IMAGE_PROTOCOL   *LoadedImage,
>+  IN EFI_DEVICE_PATH_PROTOCOL*LoadedImageDevicePath
>+  );
>+
>+/**
>+  Unprotect UEFI image.
>+
>+  @param[in]  LoadedImage  The loaded image protocol
>+  @param[in]  LoadedImageDevicePathThe loaded image device path
>protocol
>+**/
>+VOID
>+UnprotectUefiImage (
>+  IN E

Re: [edk2] [PATCH 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting.

[Yao, Jiewen]

Agree. I will remove SetMemoryPageAttributes(),ClearMemoryPageAttributes() and 
GetMemoryPageAttributes()

Thank you
Yao Jiewen

From: Gao, Liming
Sent: Friday, February 3, 2017 4:51 PM
To: Yao, Jiewen ; edk2-devel@lists.01.org
Cc: Kinney, Michael D ; Fan, Jeff 

Subject: RE: [edk2] [PATCH 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting.

Jiewen:
  This patch adds PageTableLib implementation into CpuDxe driver. The purpose 
of this change is to update SetMemoryAttributes() API in CpuDxe driver. If so, 
new function AssignMemoryPageAttributes() will be enough. Other 
SetMemoryPageAttributes(),ClearMemoryPageAttributes() and 
GetMemoryPageAttributes() are not required. If they are useful, you can 
separate PageTableLib implementation from this patch.

Thanks
Liming
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Jiewen Yao
>Sent: Thursday, January 26, 2017 4:39 PM
>To: edk2-devel@lists.01.org
>Cc: Kinney, Michael D 
>mailto:michael.d.kin...@intel.com>>; Fan, Jeff
>mailto:jeff@intel.com>>
>Subject: [edk2] [PATCH 1/3] UefiCpuPkg/CpuDxe: Add memory attribute
>setting.
>
>Add memory attribute setting in CpuArch protocol.
>Previous SetMemoryAttributes() API only supports cache attribute setting.
>
>This patch updated SetMemoryAttributes() API to support memory attribute
>setting by updating CPU page table.
>
>Cc: Jeff Fan mailto:jeff@intel.com>>
>Cc: Michael Kinney 
>mailto:michael.d.kin...@intel.com>>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Jiewen Yao mailto:jiewen@intel.com>>
>---
> UefiCpuPkg/CpuDxe/CpuDxe.c | 137 +--
> UefiCpuPkg/CpuDxe/CpuDxe.inf   |   4 +-
> UefiCpuPkg/CpuDxe/PageTableLib.h   | 204 
> UefiCpuPkg/CpuDxe/PageTableLibX86Pae.c | 997 
> 4 files changed, 1283 insertions(+), 59 deletions(-)
>
>diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
>index f6d0a67..4a1d27e 100644
>--- a/UefiCpuPkg/CpuDxe/CpuDxe.c
>+++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
>@@ -14,6 +14,10 @@
>
> #include "CpuDxe.h"
> #include "CpuMp.h"
>+#include "PageTableLib.h"
>+
>+#define CACHE_ATTRIBUTE_MASK   (EFI_MEMORY_UC | EFI_MEMORY_WC
>| EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE |
>EFI_MEMORY_WP)
>+#define MEMORY_ATTRIBUTE_MASK  (EFI_MEMORY_RP |
>EFI_MEMORY_XP | EFI_MEMORY_RO)
>
> //
> // Global Variables
>@@ -368,10 +372,9 @@ CpuSetMemoryAttributes (
>   EFI_STATUSMpStatus;
>   EFI_MP_SERVICES_PROTOCOL  *MpService;
>   MTRR_SETTINGS MtrrSettings;
>-
>-  if (!IsMtrrSupported ()) {
>-return EFI_UNSUPPORTED;
>-  }
>+  UINT64CacheAttributes;
>+  UINT64MemoryAttributes;
>+  MTRR_MEMORY_CACHE_TYPECurrentCacheType;
>
>   //
>   // If this function is called because GCD SetMemorySpaceAttributes () is
>called
>@@ -384,69 +387,87 @@ CpuSetMemoryAttributes (
> return EFI_SUCCESS;
>   }
>
>-  switch (Attributes) {
>-  case EFI_MEMORY_UC:
>-CacheType = CacheUncacheable;
>-break;
>
>-  case EFI_MEMORY_WC:
>-CacheType = CacheWriteCombining;
>-break;
>+  CacheAttributes = Attributes & CACHE_ATTRIBUTE_MASK;
>+  MemoryAttributes = Attributes & MEMORY_ATTRIBUTE_MASK;
>
>-  case EFI_MEMORY_WT:
>-CacheType = CacheWriteThrough;
>-break;
>+  if (Attributes != (CacheAttributes | MemoryAttributes)) {
>+return EFI_INVALID_PARAMETER;
>+  }
>
>-  case EFI_MEMORY_WP:
>-CacheType = CacheWriteProtected;
>-break;
>+  if (CacheAttributes != 0) {
>+if (!IsMtrrSupported ()) {
>+  return EFI_UNSUPPORTED;
>+}
>
>-  case EFI_MEMORY_WB:
>-CacheType = CacheWriteBack;
>-break;
>+switch (CacheAttributes) {
>+case EFI_MEMORY_UC:
>+  CacheType = CacheUncacheable;
>+  break;
>
>-  case EFI_MEMORY_UCE:
>-  case EFI_MEMORY_RP:
>-  case EFI_MEMORY_XP:
>-  case EFI_MEMORY_RUNTIME:
>-return EFI_UNSUPPORTED;
>+case EFI_MEMORY_WC:
>+  CacheType = CacheWriteCombining;
>+  break;
>
>-  default:
>-return EFI_INVALID_PARAMETER;
>-  }
>-  //
>-  // call MTRR libary function
>-  //
>-  Status = MtrrSetMemoryAttribute (
>- BaseAddress,
>- Length,
>- CacheType
>- );
>+case EFI_MEMORY_WT:
>+  CacheType = CacheWriteThrough;
>+  break;
>
>-  if (!RETURN_ERROR (Status)) {
>-MpStatus = gBS->LocateProtocol (
>-  &gEfiMpServiceProtocolGuid,
>-  NULL,
>-  (VOID **)&MpService
>-  );
>-//
>-// Synchronize the update with all APs
>-//
>-if (!EFI_ERROR (MpStatus)) {
>-  MtrrGetAllMtrrs (&MtrrSettings);
>-  MpStatus = MpService->StartupAllAPs (
>-  MpService,  // This
>-  SetMtrrsFromBuffer, // Procedure
>-  FALSE,  // SingleThread
>-  NULL

[edk2] [PATCH 2/2] MdePkg: UefiTcgPlatform.h: Add TCG_PCR_EVENT2_HDR definition

[Zhang, Chao B]

Add TCG_PCR_EVENT2_HDR definition.

Cc: Long Qin 
Cc: Yao Jiewen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang 
---
 MdePkg/Include/IndustryStandard/UefiTcgPlatform.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h 
b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
index 8a3e170..cd1dea8 100644
--- a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
+++ b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
@@ -206,6 +206,16 @@ typedef struct tdTCG_PCR_EVENT2 {
 } TCG_PCR_EVENT2;
 
 //
+// TCG PCR Event2 Header
+//
+typedef struct tdTCG_PCR_EVENT2_HDR{
+  TCG_PCRINDEXPCRIndex;
+  TCG_EVENTTYPE   EventType;
+  TPML_DIGEST_VALUES  Digest;
+  UINT32  EventSize;
+} TCG_PCR_EVENT2_HDR;
+
+//
 // Log Header Entry Data
 //
 typedef struct {
-- 
1.9.5.msysgit.1

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

[edk2] [PATCH 1/2] SecurityPkg: Tcg2Dxe: Log EV_NO_ACTION event in TCG_PCR_EVENT2 format

[Zhang, Chao B]

All the event log entry in TCG2 Event log format should be TCG_PCR_EVENT2
except event log header. Defined by TCG EFI Spec 2.0 00.13
https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf

Cc: Long Qin 
Cc: Yao Jiewen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang 
---
 SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 114 --
 1 file changed, 96 insertions(+), 18 deletions(-)

diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c 
b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
index 860ee59..08c40db 100644
--- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
+++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
@@ -169,6 +169,82 @@ InternalDumpData (
 
 /**
 
+  This function initialize TCG_PCR_EVENT2 for EV_NO_ACTION Event Type other 
than EFI Specification ID event
+  The behavior is defined by TCG PC Client PFP Spec. Section 9.3.4 
EV_NO_ACTION Event Types
+
+  @param[in, out]   NoActionEvent  Event Header of EV_NO_ACTION Event
+  @param[in]EventSize  Event Size of the EV_NO_ACTION Event
+
+**/
+VOID
+InitNoActionEvent (
+  IN OUT TCG_PCR_EVENT2_HDR  *NoActionEvent,
+  IN UINT32  EventSize
+ )
+{
+  UINT32  DigestListCount;
+  TPMI_ALG_HASH   HashAlgId;
+  UINT8   *DigestBuffer;
+
+  DigestBuffer= (UINT8 *)NoActionEvent->Digest.digests;
+  DigestListCount = 0;
+
+  NoActionEvent->PCRIndex  = 0;
+  NoActionEvent->EventType = EV_NO_ACTION;
+
+  //
+  // Set Hash count & hashAlg accordingly, while Digest.digests[n].digest to 
all 0
+  //
+  ZeroMem (&NoActionEvent->Digest, sizeof(NoActionEvent->Digest));
+
+  if ((mTcgDxeData.BsCap.ActivePcrBanks & EFI_TCG2_BOOT_HASH_ALG_SHA1) != 0) {
+ HashAlgId = TPM_ALG_SHA1;
+ CopyMem (DigestBuffer, &HashAlgId, sizeof(TPMI_ALG_HASH));
+ DigestBuffer += sizeof(TPMI_ALG_HASH) + GetHashSizeFromAlgo (HashAlgId);
+ DigestListCount++;
+  }
+
+  if ((mTcgDxeData.BsCap.ActivePcrBanks & EFI_TCG2_BOOT_HASH_ALG_SHA256) != 0) 
{
+ HashAlgId = TPM_ALG_SHA256;
+ CopyMem (DigestBuffer, &HashAlgId, sizeof(TPMI_ALG_HASH));
+ DigestBuffer += sizeof(TPMI_ALG_HASH) + GetHashSizeFromAlgo (HashAlgId);
+ DigestListCount++;
+  }
+
+  if ((mTcgDxeData.BsCap.ActivePcrBanks & EFI_TCG2_BOOT_HASH_ALG_SHA384) != 0) 
{
+HashAlgId = TPM_ALG_SHA384;
+CopyMem (DigestBuffer, &HashAlgId, sizeof(TPMI_ALG_HASH));
+DigestBuffer += sizeof(TPMI_ALG_HASH) + GetHashSizeFromAlgo (HashAlgId);
+DigestListCount++;
+  }
+
+  if ((mTcgDxeData.BsCap.ActivePcrBanks & EFI_TCG2_BOOT_HASH_ALG_SHA512) != 0) 
{
+HashAlgId = TPM_ALG_SHA512;
+CopyMem (DigestBuffer, &HashAlgId, sizeof(TPMI_ALG_HASH));
+DigestBuffer += sizeof(TPMI_ALG_HASH) + GetHashSizeFromAlgo (HashAlgId);
+DigestListCount++;
+  }
+
+  if ((mTcgDxeData.BsCap.ActivePcrBanks & EFI_TCG2_BOOT_HASH_ALG_SM3_256) != 
0) {
+HashAlgId = TPM_ALG_SM3_256;
+CopyMem (DigestBuffer, &HashAlgId, sizeof(TPMI_ALG_HASH));
+DigestBuffer += sizeof(TPMI_ALG_HASH) + GetHashSizeFromAlgo (HashAlgId);
+DigestListCount++;
+  }
+
+  //
+  // Set Digests Count
+  //
+  WriteUnaligned32 ((UINT32 *)&NoActionEvent->Digest.count, DigestListCount);
+
+  //
+  // Set Event Size
+  //
+  WriteUnaligned32((UINT32 *)DigestBuffer, EventSize);
+}
+
+/**
+
   This function dump raw data with colume format.
 
   @param  Data  raw data
@@ -1381,7 +1457,8 @@ SetupEventLog (
   UINT32  HashAlgorithmMaskCopied;
   TCG_EfiSpecIDEventStruct*TcgEfiSpecIdEventStruct;
   UINT8   TempBuf[sizeof(TCG_EfiSpecIDEventStruct) + 
sizeof(UINT32) + (HASH_COUNT * sizeof(TCG_EfiSpecIdEventAlgorithmSize)) + 
sizeof(UINT8)];
-  TCG_PCR_EVENT_HDR   NoActionEvent;
+  TCG_PCR_EVENT_HDR   SpecIdEvent;
+  TCG_PCR_EVENT2_HDR  NoActionEvent;
   TCG_EfiSpecIdEventAlgorithmSize *DigestSize;
   TCG_EfiSpecIdEventAlgorithmSize *TempDigestSize;
   UINT8   *VendorInfoSize;
@@ -1469,25 +1546,26 @@ SetupEventLog (
 VendorInfoSize = (UINT8 *)TempDigestSize;
 *VendorInfoSize = 0;
 
-NoActionEvent.PCRIndex = 0;
-NoActionEvent.EventType = EV_NO_ACTION;
-ZeroMem (&NoActionEvent.Digest, sizeof(NoActionEvent.Digest));
-NoActionEvent.EventSize = (UINT32)GetTcgEfiSpecIdEventStructSize 
(TcgEfiSpecIdEventStruct);
+SpecIdEvent.PCRIndex = 0;
+SpecIdEvent.EventType = EV_NO_ACTION;
+ZeroMem (&SpecIdEvent.Digest, sizeof(SpecIdEvent.Digest));
+SpecIdEvent.EventSize = (UINT32)GetTcgEfiSpecIdEventStructSize 
(TcgEfiSpecIdEventStruct);
 
 //
-// Log TcgEfiSpecIdEventStruct as the first Event
+// Log TcgEfiSpecIdEventStruct as the first Event. Event format is 
TCG_PCR_EVENT.
+//   TCG EFI Protocol Spec. Section 5.3 Event Log Header
 //   TCG PC Client PFP spec. Section 9.2 Measurement Event Entries and 
Log
 //
 Status = TcgD

Re: [edk2] [PATCH 5/6] SecurityPkg/Password: Add Password based UserAuthentication modules.

[Zhang, Chao B]

Jiewen:
  Why SmmCommunicationBufferProtocol is not placed in UserAuthenticationDxe 
[DEPEX]?

-Original Message-
From: Yao, Jiewen 
Sent: Thursday, January 26, 2017 7:50 PM
To: edk2-devel@lists.01.org
Cc: Long, Qin ; Zhang, Chao B 
Subject: [PATCH 5/6] SecurityPkg/Password: Add Password based 
UserAuthentication modules.

This password based user authentication is to verify user when a user
wants to enter BIOS setup page.

The DXE driver registers report status code listener.
When it gets (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_PC_USER_SETUP) progress
code, it will let the user input password.
If and only if the user inputs the right password, the status code handler
will return and let the setup driver continue running.
If the user inputs the wrong password, the status code handler will let
user try again.
If the user inputs the wrong password 3 times, the status code handler will
reset the system.

The DXE driver also register a setup page in setup browser, so that
user may update the password.

The DXE driver uses SMI to let SMM driver do the password verification
and management.

The SMM driver registers SMI handler to perform the request from DXE.
The password must meet below criteria:
1) Length >= 8 char
2) It must include lower case, upper case, number and symbol.
3) It must not duplicate with 5 previous password.
If above criteria is met, the password will be saved to a read only UEFI
variable. The format is password hash+salt, which is generated by
Pkcs5HashPassword algorithm (SHA256+1000 iteration).

If the SMM driver gets wrong password 3 times, the interface is locked
and does not accept more request.

If the SMM driver will detect IsPasswordCleared() at the entry point and
clear the password if IsPasswordCleared() is TRUE. This can be used when
the user forgets the password.

Cc: Qin Long 
Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 SecurityPkg/Password/UserAuthentication/KeyLib.c | 209 
++
 SecurityPkg/Password/UserAuthentication/KeyLib.h | 121 

 SecurityPkg/Password/UserAuthentication/UserAuthenticationDxe.c  | 718 

 SecurityPkg/Password/UserAuthentication/UserAuthenticationDxe.h  | 115 

 SecurityPkg/Password/UserAuthentication/UserAuthenticationDxe.inf|  76 
+++
 SecurityPkg/Password/UserAuthentication/UserAuthenticationDxe.uni|  20 
+
 SecurityPkg/Password/UserAuthentication/UserAuthenticationDxeExtra.uni   |  20 
+
 SecurityPkg/Password/UserAuthentication/UserAuthenticationDxeFormset.h   |  30 
+
 SecurityPkg/Password/UserAuthentication/UserAuthenticationDxePassword.c  | 300 

 SecurityPkg/Password/UserAuthentication/UserAuthenticationDxeStrings.uni |  29 
+
 SecurityPkg/Password/UserAuthentication/UserAuthenticationDxeVfr.vfr |  38 
++
 SecurityPkg/Password/UserAuthentication/UserAuthenticationGuid.h |  65 
++
 SecurityPkg/Password/UserAuthentication/UserAuthenticationSmm.c  | 672 
++
 SecurityPkg/Password/UserAuthentication/UserAuthenticationSmm.inf|  74 
++
 SecurityPkg/Password/UserAuthentication/UserAuthenticationSmmExtra.uni   |  20 
+
 15 files changed, 2507 insertions(+)

diff --git a/SecurityPkg/Password/UserAuthentication/KeyLib.c 
b/SecurityPkg/Password/UserAuthentication/KeyLib.c
new file mode 100644
index 000..295c73d
--- /dev/null
+++ b/SecurityPkg/Password/UserAuthentication/KeyLib.c
@@ -0,0 +1,209 @@
+/** @file
+
+Copyright (c) 2016, 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 "KeyLib.h"
+
+#define DEFAULT_AES_KEY_BIT_SIZE   256
+#define DEFAULT_PBKDF2_ITERATION_COUNT 1000
+
+/**
+  Compares the contents of two buffers with slow algorithm
+
+  This function compares Length bytes of SourceBuffer to Length bytes of 
DestinationBuffer.
+  If all Length bytes of the two buffers are identical, then 0 is returned.  
Otherwise, the
+  value returned is the first mismatched byte in SourceBuffer subtracted from 
the first
+  mismatched byte in DestinationBuffer.
+
+  If Length > 0 and DestinationBuffer is NULL, then ASSERT().
+  If Length > 0 and SourceBuffer is NULL, then ASSERT().
+  If Length is greater than (MAX_ADDRESS - DestinationBuffer + 1), then 
ASSERT().
+  If Length is greater than (MAX_ADDRESS - SourceBuffer + 1), then ASSERT().
+
+  @param  DestinationBuffer The pointer to the destination buffer to compare.
+  @param  SourceBuffer  The pointer to the source buffer

Re: [edk2] [PATCH 5/6] SecurityPkg/Password: Add Password based UserAuthentication modules.

[Yao, Jiewen]

Good catch. I will add it.

Thank you
Yao Jiewen


> -Original Message-
> From: Zhang, Chao B
> Sent: Saturday, February 4, 2017 11:05 AM
> To: Yao, Jiewen ; edk2-devel@lists.01.org
> Cc: Long, Qin 
> Subject: RE: [PATCH 5/6] SecurityPkg/Password: Add Password based
> UserAuthentication modules.
> 
> Jiewen:
>   Why SmmCommunicationBufferProtocol is not placed in
> UserAuthenticationDxe [DEPEX]?
> 
> -Original Message-
> From: Yao, Jiewen
> Sent: Thursday, January 26, 2017 7:50 PM
> To: edk2-devel@lists.01.org
> Cc: Long, Qin ; Zhang, Chao B 
> Subject: [PATCH 5/6] SecurityPkg/Password: Add Password based
> UserAuthentication modules.
> 
> This password based user authentication is to verify user when a user
> wants to enter BIOS setup page.
> 
> The DXE driver registers report status code listener.
> When it gets (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_PC_USER_SETUP)
> progress
> code, it will let the user input password.
> If and only if the user inputs the right password, the status code handler
> will return and let the setup driver continue running.
> If the user inputs the wrong password, the status code handler will let
> user try again.
> If the user inputs the wrong password 3 times, the status code handler will
> reset the system.
> 
> The DXE driver also register a setup page in setup browser, so that
> user may update the password.
> 
> The DXE driver uses SMI to let SMM driver do the password verification
> and management.
> 
> The SMM driver registers SMI handler to perform the request from DXE.
> The password must meet below criteria:
> 1) Length >= 8 char
> 2) It must include lower case, upper case, number and symbol.
> 3) It must not duplicate with 5 previous password.
> If above criteria is met, the password will be saved to a read only UEFI
> variable. The format is password hash+salt, which is generated by
> Pkcs5HashPassword algorithm (SHA256+1000 iteration).
> 
> If the SMM driver gets wrong password 3 times, the interface is locked
> and does not accept more request.
> 
> If the SMM driver will detect IsPasswordCleared() at the entry point and
> clear the password if IsPasswordCleared() is TRUE. This can be used when
> the user forgets the password.
> 
> Cc: Qin Long 
> Cc: Chao Zhang 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao 
> ---
>  SecurityPkg/Password/UserAuthentication/KeyLib.c
> | 209 ++
>  SecurityPkg/Password/UserAuthentication/KeyLib.h
> | 121 
>  SecurityPkg/Password/UserAuthentication/UserAuthenticationDxe.c
> | 718 
>  SecurityPkg/Password/UserAuthentication/UserAuthenticationDxe.h
> | 115 
>  SecurityPkg/Password/UserAuthentication/UserAuthenticationDxe.inf
> |  76 +++
>  SecurityPkg/Password/UserAuthentication/UserAuthenticationDxe.uni
> |  20 +
>  SecurityPkg/Password/UserAuthentication/UserAuthenticationDxeExtra.uni
> |  20 +
>  SecurityPkg/Password/UserAuthentication/UserAuthenticationDxeFormset.h
> |  30 +
>  SecurityPkg/Password/UserAuthentication/UserAuthenticationDxePassword.c
> | 300 
>  SecurityPkg/Password/UserAuthentication/UserAuthenticationDxeStrings.uni |
> 29 +
>  SecurityPkg/Password/UserAuthentication/UserAuthenticationDxeVfr.vfr
> |  38 ++
>  SecurityPkg/Password/UserAuthentication/UserAuthenticationGuid.h
> |  65 ++
>  SecurityPkg/Password/UserAuthentication/UserAuthenticationSmm.c
> | 672 ++
>  SecurityPkg/Password/UserAuthentication/UserAuthenticationSmm.inf
> |  74 ++
>  SecurityPkg/Password/UserAuthentication/UserAuthenticationSmmExtra.uni
> |  20 +
>  15 files changed, 2507 insertions(+)
> 
> diff --git a/SecurityPkg/Password/UserAuthentication/KeyLib.c
> b/SecurityPkg/Password/UserAuthentication/KeyLib.c
> new file mode 100644
> index 000..295c73d
> --- /dev/null
> +++ b/SecurityPkg/Password/UserAuthentication/KeyLib.c
> @@ -0,0 +1,209 @@
> +/** @file
> +
> +Copyright (c) 2016, 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 "KeyLib.h"
> +
> +#define DEFAULT_AES_KEY_BIT_SIZE   256
> +#define DEFAULT_PBKDF2_ITERATION_COUNT 1000
> +
> +/**
> +  Compares the contents of two buffers with slow algorithm
> +
> +  This function compares Length bytes of SourceBuffer to Length bytes of
> DestinationBuffer.
> +  If all Length bytes of the two buffers are identical, then 0 is returned.
> Otherwise, the
> +  value returned is the first mismatched byte in SourceBuffer subtracted from
> the first
> +  mismatched byte in DestinationBuffer.
> +

Re: [edk2] [PATCH 3/3] MdeModulePkg/DxeCore: Add UEFI image protection.

[Gao, Liming]

Jiewen:
  I agree to keep the same policy to SMM and Runtime driver.

Thanks
Liming
From: Yao, Jiewen
Sent: Saturday, February 04, 2017 8:42 AM
To: Gao, Liming ; edk2-devel@lists.01.org
Cc: Kinney, Michael D ; Tian, Feng 
; Zeng, Star 
Subject: RE: [edk2] [PATCH 3/3] MdeModulePkg/DxeCore: Add UEFI image protection.

It is pre-requisite that code section and data section should not be not merged.
That is same criteria for SMM and Runtime driver.

The problem is that I am not able to see the difference. I can only get LINK 
warning below:
LINK : warning LNK4254: section '.data' (C040) merged into '.text' 
(6020) with different attributes
But final attribute in PE code section is same.

Thank you
Yao Jiewen

From: Gao, Liming
Sent: Friday, February 3, 2017 4:40 PM
To: Yao, Jiewen mailto:jiewen@intel.com>>; 
edk2-devel@lists.01.org
Cc: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>; Tian, Feng 
mailto:feng.t...@intel.com>>; Zeng, Star 
mailto:star.z...@intel.com>>
Subject: RE: [edk2] [PATCH 3/3] MdeModulePkg/DxeCore: Add UEFI image protection.

Jiewen:
  I have one comment. If VS linker option /MERGE:.data=.text is set, the 
generated PE image will have one .text section. If this section is set to RO, 
its data will be RO. Does this patch specially handle this case?

Thanks
Liming
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Jiewen Yao
>Sent: Thursday, January 26, 2017 4:39 PM
>To: edk2-devel@lists.01.org
>Cc: Kinney, Michael D 
>mailto:michael.d.kin...@intel.com>>; Tian, Feng
>mailto:feng.t...@intel.com>>; Zeng, Star 
>mailto:star.z...@intel.com>>
>Subject: [edk2] [PATCH 3/3] MdeModulePkg/DxeCore: Add UEFI image
>protection.
>
>If the UEFI image is page aligned, the image code section is set to read
>only and the image data section is set to non-executable.
>
>1) This policy is applied for all UEFI image including boot service driver,
>runtime driver or application.
>2) This policy is applied only if the UEFI image meets the page alignment
>requirement.
>3) This policy is applied only if the UEFI image is from firmware volume.
>If the image is from OPROM, file system, or network, this policy is not
>applied, even if the image meets the page alignment requirement.
>4) This policy is not applied to the non-PE image region.
>
>The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
>the image. If the CpuArch protocol is not installed yet, the DxeCore
>enqueues the protection request. Once the CpuArch is installed, the
>DxeCore dequeues the protection request and applies policy.
>
>Once the image is unloaded, the protection is removed automatically.
>
>Cc: Star Zeng mailto:star.z...@intel.com>>
>Cc: Feng Tian mailto:feng.t...@intel.com>>
>Cc: Michael Kinney 
>mailto:michael.d.kin...@intel.com>>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Jiewen Yao mailto:jiewen@intel.com>>
>---
> MdeModulePkg/Core/Dxe/DxeMain.h   |  53 ++
> MdeModulePkg/Core/Dxe/DxeMain.inf |   2 +
> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c   |   1 +
> MdeModulePkg/Core/Dxe/Image/Image.c   |   5 +
> MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 822
>
> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c  |  22 -
> 6 files changed, 883 insertions(+), 22 deletions(-)
>
>diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h
>b/MdeModulePkg/Core/Dxe/DxeMain.h
>index ae35fbb..67b5a5a 100644
>--- a/MdeModulePkg/Core/Dxe/DxeMain.h
>+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
>@@ -267,6 +267,26 @@ typedef struct {
> #define LOADED_IMAGE_PRIVATE_DATA_FROM_THIS(a) \
>   CR(a, LOADED_IMAGE_PRIVATE_DATA, Info,
>LOADED_IMAGE_PRIVATE_DATA_SIGNATURE)
>
>+#define IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE
>SIGNATURE_32 ('I','P','R','C')
>+
>+typedef struct {
>+  UINT32 Signature;
>+  LIST_ENTRY Link;
>+  EFI_PHYSICAL_ADDRESS   CodeSegmentBase;
>+  UINT64 CodeSegmentSize;
>+} IMAGE_PROPERTIES_RECORD_CODE_SECTION;
>+
>+#define IMAGE_PROPERTIES_RECORD_SIGNATURE SIGNATURE_32
>('I','P','R','D')
>+
>+typedef struct {
>+  UINT32 Signature;
>+  LIST_ENTRY Link;
>+  EFI_PHYSICAL_ADDRESS   ImageBase;
>+  UINT64 ImageSize;
>+  UINTN  CodeSegmentCount;
>+  LIST_ENTRY CodeSegmentList;
>+} IMAGE_PROPERTIES_RECORD;
>+
> //
> // DXE Core Global Variables
> //
>@@ -2859,6 +2879,15 @@ CoreInitializeMemoryAttributesTable (
>   );
>
> /**
>+  Initialize Memory Protection support.
>+**/
>+VOID
>+EFIAPI
>+CoreInitializeMemoryProtection (
>+  VOID
>+  );
>+
>+/**
>   Install MemoryAttributesTable on memory allocation.
>
>   @param[in] MemoryType EFI memory type.
>@@ -2888,4 +2917,28 @@ RemoveImageRecord (
>   IN EFI_RUNTIME_IMAGE_ENTRY  *RuntimeImage
>   );
>
>+/**
>+  Protect UEFI image.
>+
>+  @param[in]  LoadedImage  The loaded

Re: [edk2] [PATCH v1 1/1] BaseTools: AutoGen: Expand macros in INF files

[Gao, Liming]

Kyle:
  I agree the inconsistent behavior causes the confuse. Per INF spec 2.2.6 
Macro Statements, Use of MACRO statements in the EDK II INF files is limited to 
local usage only; global or external macros are not permitted. So, 
BASE_NAME=Test$(ARCH) is a wrong usage. We will update BaseTools report error.

Thanks
Liming
From: Kyle Pugh [mailto:kyletp...@gmail.com]
Sent: Saturday, February 04, 2017 12:21 AM
To: Gao, Liming 
Cc: edk2-devel@lists.01.org; Laszlo Ersek ; Zhu, Yonghong 

Subject: Re: [PATCH v1 1/1] BaseTools: AutoGen: Expand macros in INF files

Liming,

The thing is that macros are expanded in DEC/INF files but only in some cases, 
and not even consistently. I wanted to name all my files appropriately so I 
don't have to create a map to copy the original name to the actual name it 
needs to be and instead can just copy the *.efi to the destination. Like, if 
you write BASE_NAME=Test$(ARCH) for a driver or application, it creates the 
file TestX64.efi, etc, but however if you write that for a base library then it 
fails to link because it creates the correct library named TestX64.lib but it 
can't link because its looking for Test$(ARCH).lib. This appears to happen if 
you use any macro, like I don't want to hard code the project name since it 
could change, or the version, so these are both macros. Setting 
$(PROJECT_NAME)$(ARCH) works exactly the same way as above. If you check out 
https://bugzilla.tianocore.org/show_bug.cgi?id=334, there's more explanation. 
It's not a super priority or anything, I can make it work without it's just 
more wor
 k.

Thanks,
Kyle


On Fri, Feb 3, 2017 at 12:31 AM, Gao, Liming 
mailto:liming@intel.com>> wrote:
Pugh:
  Environment macros are allowed in DSC/FDF, but not in DEC/INF. DEC and INF 
belongs to Packages. They are required to be the fixed information, like the 
definition in C header file. I understand your usage is to change BASE_NAME 
defined in module INF. Why need to the different BASE_NAME?

Thanks
Liming
From: Kyle Pugh [mailto:kyletp...@gmail.com]
Sent: Wednesday, January 25, 2017 6:58 PM
To: Gao, Liming mailto:liming@intel.com>>
Cc: edk2-devel@lists.01.org; Laszlo Ersek 
mailto:ler...@redhat.com>>; Zhu, Yonghong 
mailto:yonghong@intel.com>>
Subject: Re: [PATCH v1 1/1] BaseTools: AutoGen: Expand macros in INF files

Hello,
I'm not sure I understand, the macros are expanded elsewhere I was just fixing 
a few places where they are not. I don't see anywhere in that document where it 
says anything about expanding macros. Also there is already PACKAGES_PATH, so 
I'm not sure what you mean. I am using macros to generate the base name of 
modules, there is no way around this and the modules need to built with a 
script since it generates extra stuff that's needed. I ran into an issue where 
the name was correct in all but two places, one caused failed link because it 
didn't expand the name macro correctly but it did expand the ARCH macro right 
next to it. The other was the gEfiCallerBaseName string global, it didn't 
expand the macros either. I have no problem still building any package and now 
it correctly names the binaries.

Thanks

On Tue, Jan 24, 2017 at 10:21 PM, Gao, Liming 
mailto:liming@intel.com>> wrote:
Hi,
  This usage will break UDP (UEFI Platform Initialization Distribution 
Packaging Specification).  It can be downloaded from 
http://www.uefi.org/specifications. This spec defines the packaging format. It 
defines SourceFiles.Filename as the path (relative to the Module "root" 
directory) and filename of the file (as specified in the Distribution Content 
File.) That means source files must be in the module directory.
  And, EDK2 BaseTools UPT tool follows UDP to create Package.dist based on edk2 
source Package, and install Package.dist to edk2 source package. If source 
package has no change, UPT should always create the same package.dist file. 
But, this feature will break it. So, I don't suggest to add this support.

Thanks
Liming
>-Original Message-
>From: kyletp...@gmail.com 
>[mailto:kyletp...@gmail.com]
>Sent: Wednesday, January 25, 2017 3:16 AM
>To: edk2-devel@lists.01.org
>Cc: Laszlo Ersek mailto:ler...@redhat.com>>; Zhu, Yonghong
>mailto:yonghong@intel.com>>; Gao, Liming 
>mailto:liming@intel.com>>
>Subject: [PATCH v1 1/1] BaseTools: AutoGen: Expand macros in INF files
>
>From: apianti 
>mailto:apia...@users.sourceforge.net>>
>
>Expand environment macros used for paths and names in INF files
>
>Cc: Laszlo Ersek mailto:ler...@redhat.com>>
>Cc: Yonghong Zhu mailto:yonghong@intel.com>>
>Cc: Liming Gao mailto:liming@intel.com>>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: apianti 
>mailto:apia...@users.sourceforge.net>>
>---
> BaseTools/Source/Python/AutoGen/GenC.py| 4 ++--
> BaseTools/Source/Python/AutoGen/Gen