[edk2] [PATCH] SecurityPkg : Tpm12DeviceLibDTpm: Fix TPM12 wrong Response Tag check

[Zhang, Chao B]

TcgDxePassThroughToTpm should be able to handle all TPM12 Command & Response 
correctly.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang 
---
 SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12Tis.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12Tis.c 
b/SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12Tis.c
index 4e04299..5e154d0 100644
--- a/SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12Tis.c
+++ b/SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12Tis.c
@@ -267,6 +267,7 @@ Tpm12TisTpmCommand (
   UINT32TpmOutSize;
   UINT16Data16;
   UINT32Data32;
+  UINT16RspTag;
 
   DEBUG_CODE (
 UINTN  DebugSize;
@@ -367,8 +368,9 @@ Tpm12TisTpmCommand (
   // Check the reponse data header (tag,parasize and returncode )
   //
   CopyMem (, BufferOut, sizeof (UINT16));
-  if (SwapBytes16 (Data16) != TPM_TAG_RSP_COMMAND) {
-DEBUG ((EFI_D_ERROR, "TPM12: TPM_ST_RSP error - %x\n", 
TPM_TAG_RSP_COMMAND));
+  RspTag = SwapBytes16 (Data16);
+  if (RspTag != TPM_TAG_RSP_COMMAND && RspTag != TPM_TAG_RSP_AUTH1_COMMAND && 
RspTag != TPM_TAG_RSP_AUTH2_COMMAND) {
+DEBUG ((EFI_D_ERROR, "TPM12: Response tag error - current tag value is 
%x\n", RspTag));
 Status = EFI_UNSUPPORTED;
 goto Exit;
   }
-- 
1.9.5.msysgit.1

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

[edk2] [PATCH v2 2/2] MdeModulePkg/RamDiskDxe: Add Memory Type selection support in Ramdisk HII

[Hao Wu]

From: Tapan Shah 

Adding an option in HII menu so user can choose memory type to use when
creating a RAM Disk in system.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Tapan Shah 
---
 .../Universal/Disk/RamDiskDxe/RamDiskHii.vfr   | 12 +
 .../Disk/RamDiskDxe/RamDiskHiiStrings.uni  |  7 ++-
 .../Universal/Disk/RamDiskDxe/RamDiskImpl.c| 52 ++
 .../Universal/Disk/RamDiskDxe/RamDiskNVData.h  | 10 +
 4 files changed, 72 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHii.vfr 
b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHii.vfr
index 270f791..5f3d0fa 100644
--- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHii.vfr
+++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHii.vfr
@@ -2,6 +2,7 @@
 //  VFR file used by the RamDiskDxe driver.
 //
 //  Copyright (c) 2016, Intel Corporation. All rights reserved.
+//  (C) Copyright 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
 //  which accompanies this distribution.  The full text of the license may be 
found at
@@ -26,6 +27,17 @@ formset
   form formid = MAIN_FORM_ID,
 title  = STRING_TOKEN(STR_MAIN_FORM_TITLE);
 
+oneof
+  questionid  = CREATE_RAW_MEMORY_TYPE_QUESTION_ID,
+prompt  = STRING_TOKEN(STR_MEMORY_TYPE_PROMPT),
+help= STRING_TOKEN(STR_MEMORY_TYPE_HELP),
+flags   = NUMERIC_SIZE_1 | INTERACTIVE,
+option text = STRING_TOKEN(STR_RAM_DISK_BOOT_SERVICE_DATA_MEMORY), 
value = RAM_DISK_BOOT_SERVICE_DATA_MEMORY, flags = DEFAULT;
+option text = STRING_TOKEN(STR_RAM_DISK_RESERVED_MEMORY), value = 
RAM_DISK_RESERVED_MEMORY, flags = 0;
+endoneof;
+
+subtitle text = STRING_TOKEN(STR_RAM_DISK_NULL_STRING);
+
 goto CREATE_RAW_RAM_DISK_FORM_ID,
   prompt = STRING_TOKEN(STR_GOTO_ADD_RAW_FORM),
   help   = STRING_TOKEN(STR_GOTO_ADD_RAW_FORM_HELP);
diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHiiStrings.uni 
b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHiiStrings.uni
index ee5f6e6..9cc0b08 100644
--- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHiiStrings.uni
+++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHiiStrings.uni
@@ -2,7 +2,7 @@
 // String definitions for RamDiskDxe driver form.
 //
 // Copyright (c) 2016, Intel Corporation. All rights reserved.
-//
+// (C) Copyright 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
 // which accompanies this distribution. The full text of the license may be 
found at
@@ -36,6 +36,11 @@
 #string STR_SIZE_PROMPT#language en-US "Size (Hex):"
 #string STR_SIZE_HELP  #language en-US "The valid RAM disk 
size should be multiples of the RAM disk block size."
 
+#string STR_MEMORY_TYPE_PROMPT#language en-US "Disk Memory 
Type:"
+#string STR_MEMORY_TYPE_HELP  #language en-US "Specifies type 
of memory to use from available memory pool in system to create a disk."
+#string STR_RAM_DISK_BOOT_SERVICE_DATA_MEMORY #language en-US "Boot Service 
Data"
+#string STR_RAM_DISK_RESERVED_MEMORY  #language en-US "Reserved"
+
 #string STR_CREATE_AND_EXIT_HELP   #language en-US "Create a new RAM disk 
with the given starting and ending address."
 #string STR_CREATE_AND_EXIT_PROMPT #language en-US "Create & Exit"
 #string STR_DISCARD_AND_EXIT_HELP  #language en-US "Discard and exit."
diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c 
b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c
index 304d5d7..b562bc1 100644
--- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c
+++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c
@@ -2,6 +2,7 @@
   HII Config Access protocol implementation of RamDiskDxe driver.
 
   Copyright (c) 2016, Intel Corporation. All rights reserved.
+  (C) Copyright 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
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -19,7 +20,8 @@ CHAR16  mRamDiskStorageName[] = L"RAM_DISK_CONFIGURATION";
 RAM_DISK_CONFIG_PRIVATE_DATA mRamDiskConfigPrivateDataTemplate = {
   RAM_DISK_CONFIG_PRIVATE_DATA_SIGNATURE,
   {
-EFI_PAGE_SIZE
+EFI_PAGE_SIZE,
+RAM_DISK_BOOT_SERVICE_DATA_MEMORY
   },
   {
 RamDiskExtractConfig,
@@ -287,6 +289,7 @@ RamDiskRouteConfig (
  If creating from file, zero.
   @param[in] FileHandle  If creating raw, NULL. If creating from file, the
  file handle.
+  @param[in] MemoryType  Type of memory to 

[edk2] [PATCH v2 1/2] MdeModulePkg RamDiskDxe: Do not save 'Size' numeric value by varstore

[Hao Wu]

The 'Size' numeric value used when creating a raw RAM disk does not
require a varstore to save its previous value in the create raw RAM disk
HII page.

The expecting behavior is that after a user created a raw RAM disk, the
next time when the create raw RAM disk page is entered, the 'Size' numeric
will be the default value (EFI_PAGE_SIZE).

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 .../Universal/Disk/RamDiskDxe/RamDiskHii.vfr   |   9 +-
 .../Universal/Disk/RamDiskDxe/RamDiskImpl.c| 146 -
 .../Universal/Disk/RamDiskDxe/RamDiskImpl.h|   2 +
 .../Universal/Disk/RamDiskDxe/RamDiskNVData.h  |   2 -
 4 files changed, 33 insertions(+), 126 deletions(-)

diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHii.vfr 
b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHii.vfr
index 9c3e3e4..270f791 100644
--- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHii.vfr
+++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHii.vfr
@@ -20,11 +20,6 @@ formset
   help  = STRING_TOKEN(STR_FORM_SET_TITLE_HELP),
   classguid = EFI_HII_PLATFORM_SETUP_FORMSET_GUID,
 
-  varstore RAM_DISK_CONFIGURATION,
-varid = RAM_DISK_CONFIGURATION_VARSTORE_ID,
-name  = RAM_DISK_CONFIGURATION,
-guid  = RAM_DISK_FORM_SET_GUID;
-
   //
   // Form #1 "Main Form - Add/Remove/Show RAM Disks"
   //
@@ -65,11 +60,11 @@ formset
 
 subtitle text = STRING_TOKEN(STR_RAM_DISK_NULL_STRING);
 
-numeric varid   = RAM_DISK_CONFIGURATION.Size,
+numeric
   questionid = CREATE_RAW_SIZE_QUESTION_ID,
   prompt  = STRING_TOKEN(STR_SIZE_PROMPT),
   help= STRING_TOKEN(STR_SIZE_HELP),
-  flags   = DISPLAY_UINT_HEX | INTERACTIVE,
+  flags   = NUMERIC_SIZE_8 | DISPLAY_UINT_HEX | INTERACTIVE,
   minimum = 1,
   maximum = 0x,
 endnumeric;
diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c 
b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c
index f402440..304d5d7 100644
--- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c
+++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c
@@ -19,6 +19,9 @@ CHAR16  mRamDiskStorageName[] = L"RAM_DISK_CONFIGURATION";
 RAM_DISK_CONFIG_PRIVATE_DATA mRamDiskConfigPrivateDataTemplate = {
   RAM_DISK_CONFIG_PRIVATE_DATA_SIGNATURE,
   {
+EFI_PAGE_SIZE
+  },
+  {
 RamDiskExtractConfig,
 RamDiskRouteConfig,
 RamDiskCallback
@@ -234,87 +237,11 @@ RamDiskExtractConfig (
OUT EFI_STRING   *Results
   )
 {
-  EFI_STATUS   Status;
-  UINTNBufferSize;
-  RAM_DISK_CONFIGURATION   *Configuration;
-  EFI_STRING   ConfigRequest;
-  EFI_STRING   ConfigRequestHdr;
-  RAM_DISK_CONFIG_PRIVATE_DATA *ConfigPrivate;
-  UINTNSize;
-  BOOLEAN  AllocatedRequest;
-
   if (Progress == NULL || Results == NULL) {
 return EFI_INVALID_PARAMETER;
   }
-
   *Progress = Request;
-  if ((Request != NULL) &&
-!HiiIsConfigHdrMatch (Request, , mRamDiskStorageName)) 
{
-return EFI_NOT_FOUND;
-  }
-
-  ConfigRequestHdr = NULL;
-  ConfigRequest= NULL;
-  AllocatedRequest = FALSE;
-  Size = 0;
-
-  //
-  // Convert buffer data to  by helper function BlockToConfig()
-  //
-  ConfigPrivate = RAM_DISK_CONFIG_PRIVATE_FROM_THIS (This);
-  BufferSize = sizeof (RAM_DISK_CONFIGURATION);
-  Configuration = AllocateZeroPool (BufferSize);
-  if (Configuration == NULL) {
-return EFI_OUT_OF_RESOURCES;
-  }
-
-  ConfigRequest = Request;
-  if ((Request == NULL) || (StrStr (Request, L"OFFSET") == NULL)) {
-//
-// Request has no request element, construct full request string.
-// Allocate and fill a buffer large enough to hold the  template
-// followed by "=0=" followed by a 
Null-terminator
-//
-ConfigRequestHdr = HiiConstructConfigHdr (
- ,
- mRamDiskStorageName,
- ConfigPrivate->DriverHandle
- );
-Size = (StrLen (ConfigRequestHdr) + 32 + 1) * sizeof (CHAR16);
-ConfigRequest = AllocateZeroPool (Size);
-ASSERT (ConfigRequest != NULL);
-AllocatedRequest = TRUE;
-UnicodeSPrint (ConfigRequest, Size, L"%s=0=%016LX", 
ConfigRequestHdr, (UINT64)BufferSize);
-FreePool (ConfigRequestHdr);
-  }
-
-  Status = gHiiConfigRouting->BlockToConfig (
-gHiiConfigRouting,
-ConfigRequest,
-(UINT8 *) ,
-BufferSize,
-Results,
-Progress
-);
-  //
-  // Free the allocated config request string and RAM disk configuration data.
-  //
-  if (AllocatedRequest) {
-FreePool (ConfigRequest);
-ConfigRequest = 

[edk2] [PATCH v2 0/2] Add Memory Type selection support in Ramdisk HII

[Hao Wu]

Changes made comparing to v1:

Stop using varstore to track the values of 'Size' numeric and 'MemoryType'
oneof in HII.

The reason for this change is that the expecting behavior for these two
HII elements during raw RAM disk creation is that after a user created (or
canceled the creation) a RAM disk, their values will be back to their
default value (i.e. EFI_PAGE_SIZE for 'Size' and
RAM_DISK_BOOT_SERVICE_DATA_MEMORY for 'MemoryType'). Therefore, the
varstore seems unnecessary here and removing it will help the codes being
less confusing.


Hao Wu (1):
  MdeModulePkg RamDiskDxe: Do not save 'Size' numeric value by varstore

Tapan Shah (1):
  MdeModulePkg/RamDiskDxe: Add Memory Type selection support in Ramdisk
HII

 .../Universal/Disk/RamDiskDxe/RamDiskHii.vfr   |  21 ++-
 .../Disk/RamDiskDxe/RamDiskHiiStrings.uni  |   7 +-
 .../Universal/Disk/RamDiskDxe/RamDiskImpl.c| 194 -
 .../Universal/Disk/RamDiskDxe/RamDiskImpl.h|   2 +
 .../Universal/Disk/RamDiskDxe/RamDiskNVData.h  |  12 +-
 5 files changed, 103 insertions(+), 133 deletions(-)

-- 
1.9.5.msysgit.0

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

Re: [edk2] [Patch] NetworkPkg: Fix DNS GeneralLookUp failure in some case

[Zhang, Lubo]

Reviewed-by: Zhang Lubo 

-Original Message-
From: Wu, Jiaxin 
Sent: Monday, June 06, 2016 1:33 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Zhang, Lubo 
Subject: [Patch] NetworkPkg: Fix DNS GeneralLookUp failure in some case

It's incorrect to check the value 1(DNS_CLASS_INET) of QClass.
Moreover, the 'Identification' and 'Type' filed in Query packet should not be 
changed to little endian since the packet maybe retransmitted while there is 
any error happened.

Cc: Ye Ting 
Cc: Zhang Lubo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/DnsDxe/DnsImpl.c | 29 +  
NetworkPkg/DnsDxe/DnsImpl.h |  2 ++
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c index 
4f10e17..360f68e 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -1041,10 +1041,11 @@ AddDns6ServerIp (
   Find out whether the response is valid or invalid.
 
   @param  TokensMap   All DNS transmittal Tokens entry.  
   @param  Identification  Identification for queried packet.  
   @param  TypeType for queried packet.
+  @param  Class   Class for queried packet.
   @param  ItemReturn corresponding Token entry.
 
   @retval TRUEThe response is valid.
   @retval FALSE   The response is invalid.
   
@@ -1052,10 +1053,11 @@ AddDns6ServerIp (  BOOLEAN  IsValidDnsResponse (
   IN NET_MAP  *TokensMap,
   IN UINT16   Identification,
   IN UINT16   Type,
+  IN UINT16   Class,
  OUT NET_MAP_ITEM **Item
   )
 {
   LIST_ENTRY  *Entry;
 
@@ -1075,21 +1077,20 @@ IsValidDnsResponse (
   TxString = NetbufGetByte (Packet, 0, NULL);
   ASSERT (TxString != NULL);
   DnsHeader = (DNS_HEADER *) TxString;
   QueryName = (CHAR8 *) (TxString + sizeof (*DnsHeader));
   QuerySection = (DNS_QUERY_SECTION *) (QueryName + AsciiStrLen 
(QueryName) + 1);
-
-  DnsHeader->Identification = NTOHS (DnsHeader->Identification);
-  QuerySection->Type = NTOHS (QuerySection->Type);
 
-  if (DnsHeader->Identification == Identification && QuerySection->Type == 
Type) {
+  if (NTOHS (DnsHeader->Identification) == Identification &&
+  NTOHS (QuerySection->Type) == Type && 
+  NTOHS (QuerySection->Class) == Class) {
 return TRUE;
   }
 } 
   }
   
-  *Item =NULL;
+  *Item = NULL;
   
   return FALSE;
 }
 
 /**
@@ -1193,19 +1194,31 @@ ParseDnsResponse (
 
   //
   // Check DnsResponse Validity, if so, also get a valid NET_MAP_ITEM.
   //
   if (Instance->Service->IpVersion == IP_VERSION_4) {
-if (!IsValidDnsResponse (>Dns4TxTokens, 
DnsHeader->Identification, QuerySection->Type, )) {
+if (!IsValidDnsResponse (
+   >Dns4TxTokens, 
+   DnsHeader->Identification, 
+   QuerySection->Type,
+   QuerySection->Class,
+   
+   )) {
   *Completed = FALSE;
   Status = EFI_ABORTED;
   goto ON_EXIT;
 }
 ASSERT (Item != NULL);
 Dns4TokenEntry = (DNS4_TOKEN_ENTRY *) (Item->Key);
   } else {
-if (!IsValidDnsResponse (>Dns6TxTokens, 
DnsHeader->Identification, QuerySection->Type, )) {
+if (!IsValidDnsResponse (
+   >Dns6TxTokens, 
+   DnsHeader->Identification, 
+   QuerySection->Type,
+   QuerySection->Class,
+   
+   )) {
   *Completed = FALSE;
   Status = EFI_ABORTED;
   goto ON_EXIT;
 }
 ASSERT (Item != NULL);
@@ -1214,11 +1227,11 @@ ParseDnsResponse (

   //
   // Continue Check Some Errors.
   //
   if (DnsHeader->Flags.Bits.RCode != DNS_FLAGS_RCODE_NO_ERROR || 
DnsHeader->AnswersNum < 1 || \
-  DnsHeader->Flags.Bits.QR != DNS_FLAGS_QR_RESPONSE || QuerySection->Class 
!= DNS_CLASS_INET) {
+  DnsHeader->Flags.Bits.QR != DNS_FLAGS_QR_RESPONSE) {
   Status = EFI_ABORTED;
   goto ON_EXIT;
   }
 
   //
diff --git a/NetworkPkg/DnsDxe/DnsImpl.h b/NetworkPkg/DnsDxe/DnsImpl.h index 
0ef8255..5fa7f24 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.h
+++ b/NetworkPkg/DnsDxe/DnsImpl.h
@@ -559,10 +559,11 @@ AddDns6ServerIp (
   Find out whether the response is valid or invalid.
 
   @param  TokensMap   All DNS transmittal Tokens entry.  
   @param  Identification  Identification for queried packet.  
   @param  TypeType for queried packet.
+  @param  Class   Class for queried packet.
   @param  ItemReturn corresponding Token entry.
 
   @retval TRUEThe response is valid.
   @retval FALSE   The response is invalid.
   
@@ -570,10 +571,11 @@ AddDns6ServerIp (
 BOOLEAN
 IsValidDnsResponse (
   IN NET_MAP  *TokensMap,
   IN UINT16   Identification,
   IN UINT16   Type,
+  IN UINT16   

Re: [edk2] [patch][edk2-platforms/branch]: Add new platform branch pentium-celeron-n-udk2015

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: Guo, Mang
> Sent: Friday, June 03, 2016 3:43 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming ; Wei, David ;
> Guo, Mang ; Wu, Mike 
> Subject: [edk2][patch][edk2-platforms/branch]: Add new platform branch
> pentium-celeron-n-udk2015
> 
> This branch is used to contribute N-series Intel Pentium Processor and Intel
> Celeron Processor Families (formerly Braswell) to EDK2.
> All code are ready in https://github.com/mangguo321/edk2-platforms.git -b
> pentium-celeron-n-udk2015
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Guo Mang 
> ---
>  Readme.txt | 22 ++
>  1 file changed, 22 insertions(+)
>  create mode 100644 Readme.txt
> 
> diff --git a/Readme.txt b/Readme.txt
> new file mode 100644
> index 000..cd5e4d3
> --- /dev/null
> +++ b/Readme.txt
> @@ -0,0 +1,22 @@
> +This branch is used to contribute N-series Intel Pentium Processor and Intel
> Celeron Processor
> +Families (formerly Braswell) to EDK2.
> +
> +The branch owner:
> +
> +
> +
> +
> +
> +
> +
> +Feature Introduction:
> +pentium-celeron-n-udk2015 is designed for N-series Intel Pentium
> processor and Intel Celeron processor
> +families(formerly Braswell), and has been verified on Intel CherryHill
> CRB(Customer Reference Board).
> +It is based on Intel UDK2015 (EDKII) core and Intel Braswell FSP. It is a
> sample codebase to show how
> +FSP + open source can be used to build a full solution.
> +
> +Related Modules:
> +The following modules are related to pentium-celeron-n-udk2015
> +BraswellPlatformPkg
> +ChvFspBinPkg
> +ChvRefCodePkg
> \ No newline at end of file
> --
> 2.8.1.windows.1

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

Re: [edk2] [Patch] NetworkPkg: Remove TokenEntry from Token list before freed

[Zhang, Lubo]

Reviewed-by: Zhang Lubo 

-Original Message-
From: Wu, Jiaxin 
Sent: Thursday, June 02, 2016 4:26 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Zhang, Lubo 
Subject: [Patch] NetworkPkg: Remove TokenEntry from Token list before freed

TokenEntry should be removed from Token list before freed.
Otherwise, invalid TokenEntry will be existed in Token list.

Cc: Ye Ting 
Cc: Zhang Lubo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/DnsDxe/DnsProtocol.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/NetworkPkg/DnsDxe/DnsProtocol.c b/NetworkPkg/DnsDxe/DnsProtocol.c 
index 11009fd..e9101d6 100644
--- a/NetworkPkg/DnsDxe/DnsProtocol.c
+++ b/NetworkPkg/DnsDxe/DnsProtocol.c
@@ -491,10 +491,12 @@ Dns4HostNameToIp (
   //
   // Dns Query Ip
   //
   Status = DoDnsQuery (Instance, Packet);
   if (EFI_ERROR (Status)) {
+Dns4RemoveTokenEntry (>Dns4TxTokens, TokenEntry);
+
 if (TokenEntry != NULL) {
   FreePool (TokenEntry);
 }
 
 NetbufFree (Packet);
@@ -673,10 +675,12 @@ Dns4GeneralLookUp (
   //
   // Dns Query Ip
   //
   Status = DoDnsQuery (Instance, Packet);
   if (EFI_ERROR (Status)) {
+Dns4RemoveTokenEntry (>Dns4TxTokens, TokenEntry);
+
 if (TokenEntry != NULL) {
   FreePool (TokenEntry);
 }
 
 NetbufFree (Packet);
@@ -1301,10 +1305,12 @@ Dns6HostNameToIp (
   //
   // Dns Query Ip
   //
   Status = DoDnsQuery (Instance, Packet);
   if (EFI_ERROR (Status)) {
+Dns6RemoveTokenEntry (>Dns6TxTokens, TokenEntry);
+
 if (TokenEntry != NULL) {
   FreePool (TokenEntry);
 }
 
 NetbufFree (Packet);
@@ -1486,10 +1492,12 @@ Dns6GeneralLookUp (
   //
   // Dns Query Ip
   //
   Status = DoDnsQuery (Instance, Packet);
   if (EFI_ERROR (Status)) {
+Dns6RemoveTokenEntry (>Dns6TxTokens, TokenEntry);
+
 if (TokenEntry != NULL) {
   FreePool (TokenEntry);
 }
 
 NetbufFree (Packet);
--
1.9.5.msysgit.1

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

Re: [edk2] [Patch v2] UefiCpuPkg/MtrrLib: Fixed bug if length is less than Fixed-MTRR range

[Dong, Eric]

Reviewed_by: Eric Dong 

> -Original Message-
> From: Fan, Jeff
> Sent: Monday, June 06, 2016 2:00 PM
> To: edk2-de...@ml01.01.org
> Cc: Dong, Eric; Tian, Feng; Kinney, Michael D
> Subject: [Patch v2] UefiCpuPkg/MtrrLib: Fixed bug if length is less than 
> Fixed-MTRR range
> 
> Currently, if the memory length to be programmed is less than the remaining 
> size
> of one Fixed-MTRR supported, RETURN_UNSUPPORTED returned. This is not correct.
> This is one regression at 07e889209034ba94bfac9e765b8a50ef344daef2 when we
> updated ProgramFixedMtrr() to remove the loop of calculating Fixed-MTRR Mask.
> 
> This fix will calculate Right offset in Fixed-MTRR beside left offset. It
> supports small length (less than remaining size supported by Fixed-MTRR) to be
> programmed.
> 
> v2:
>  1) Remove unnecessary local variable OrMaskTemplate.
>  2) Exchange LeftByteShift and RightByteShift programm order and correct the
> comments.
> 
> Cc: Eric Dong 
> Cc: Feng Tian 
> Cc: Michael Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> ---
>  UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 71 
> 
>  1 file changed, 48 insertions(+), 23 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c 
> b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> index c4a39b5..6a6bf76 100644
> --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> @@ -20,8 +20,8 @@
>  #include 
>  #include 
> 
> -#define OR_SEED  0x01010101
> -#define CLEAR_SEED   0x
> +#define OR_SEED  0x0101010101010101ull
> +#define CLEAR_SEED   0xull
> 
>  //
>  // Context to save and restore when MTRRs are programmed
> @@ -462,14 +462,15 @@ ProgramFixedMtrr (
>)
>  {
>UINT32  MsrNum;
> -  UINT32  ByteShift;
> -  UINT32  OrMask[2];
> -  UINT32  ClearMask[2];
> +  UINT32  LeftByteShift;
> +  UINT32  RightByteShift;
> +  UINT64  OrMask;
> +  UINT64  ClearMask;
>UINT64  SubLength;
> 
> -  *(UINT64 *)OrMask= 0;
> -  *(UINT64 *)ClearMask = 0;
> -
> +  //
> +  // Find the fixed MTRR index to be programmed
> +  //
>for (MsrNum = *LastMsrNum + 1; MsrNum < MTRR_NUMBER_OF_FIXED_MTRR; 
> MsrNum++) {
>  if ((*Base >= mMtrrLibFixedMtrrTable[MsrNum].BaseAddress) &&
>  (*Base <
> @@ -488,36 +489,60 @@ ProgramFixedMtrr (
>}
> 
>//
> -  // We found the fixed MTRR to be programmed
> +  // Find the begin offset in fixed MTRR and calculate byte offset of left 
> shift
>//
> -  ByteShift = ((UINT32)*Base - mMtrrLibFixedMtrrTable[MsrNum].BaseAddress)
> +  LeftByteShift = ((UINT32)*Base - 
> mMtrrLibFixedMtrrTable[MsrNum].BaseAddress)
> / mMtrrLibFixedMtrrTable[MsrNum].Length;
> 
> -  if (ByteShift >= 8) {
> +  if (LeftByteShift >= 8) {
>  return RETURN_UNSUPPORTED;
>}
> 
> -  if (ByteShift < 4) {
> -OrMask[0]= OR_SEED * (UINT32)MemoryCacheType;
> -ClearMask[0] = CLEAR_SEED;
> -OrMask[1]= (OR_SEED * (UINT32)MemoryCacheType) >> ((4 - ByteShift) * 
> 8);
> -ClearMask[1] = CLEAR_SEED >> ((4 - ByteShift) * 8);
> +  //
> +  // Find the end offset in fixed MTRR and calculate byte offset of right 
> shift
> +  //
> +  SubLength = mMtrrLibFixedMtrrTable[MsrNum].Length * (8 - LeftByteShift);
> +  if (*Length >= SubLength) {
> +RightByteShift = 0;
>} else {
> -OrMask[0]= (OR_SEED * (UINT32)MemoryCacheType) >> ((8 - ByteShift) * 
> 8);
> -ClearMask[0] = CLEAR_SEED >> ((8 - ByteShift) * 8);
> +RightByteShift = 8 - LeftByteShift -
> +(UINT32)(*Length) / mMtrrLibFixedMtrrTable[MsrNum].Length;
> +if ((LeftByteShift >= 8) ||
> +(((UINT32)(*Length) % mMtrrLibFixedMtrrTable[MsrNum].Length) != 0)
> +) {
> +  return RETURN_UNSUPPORTED;
> +}
> +//
> +// Update SubLength by actual length
> +//
> +SubLength = *Length;
>}
> 
> -  SubLength = mMtrrLibFixedMtrrTable[MsrNum].Length * (8 - ByteShift);
> -  if (*Length < SubLength) {
> -return RETURN_UNSUPPORTED;
> +  ClearMask = CLEAR_SEED;
> +  OrMask= MultU64x32 (OR_SEED, (UINT32)MemoryCacheType);
> +
> +  if (LeftByteShift != 0) {
> +//
> +// Clear the low bits by LeftByteShift
> +//
> +ClearMask &= LShiftU64 (ClearMask, LeftByteShift * 8);
> +OrMask&= LShiftU64 (OrMask, LeftByteShift * 8);
> +  }
> +
> +  if (RightByteShift != 0) {
> +//
> +// Clear the high bits by RightByteShift
> +//
> +ClearMask &= RShiftU64 (ClearMask, RightByteShift * 8);
> +OrMask&= RShiftU64 (OrMask, RightByteShift * 8);
>}
> 
>*Length -= SubLength;
>*Base   += SubLength;
> 
>*LastMsrNum  = MsrNum;
> -  *ReturnClearMask = *(UINT64 *)ClearMask;
> -  *ReturnOrMask= *(UINT64 *)OrMask;
> +  *ReturnClearMask = ClearMask;
> +  *ReturnOrMask= OrMask;
> 
>return RETURN_SUCCESS;
>  

Re: [edk2] [PATCH] MdeModulePkg/RamDiskDxe: Add Memory Type selection support in Ramdisk HII

[Wu, Hao A]

Hi Tapan,

I think you are right that there will be issues for not saving the changed
'Size' numeric and 'MemoryType' oneof back to the varstore.

My expecting behavior for these two HII elements during raw RAM disk
creation is that after a user created (or canceled the creation) a RAM
disk, their values will be back to their default value (i.e. 0x1000 for
'Size' and RAM_DISK_BOOT_SERVICE_DATA_MEMORY for 'MemoryType').

So I think using varstore to save the previous 'Size' and 'MemoryType'
values is not necessary here. Eliminating the varstore will also remove
the HiiGetBrowserData/HiiSetBrowserData calls to make the code less
confusing.

I will send out a new patch series based on your patch. The first patch of
the series will remove the varstore used in the driver. The second patch
will be your proposed patch (with some small modifications).

Best Regards,
Hao Wu

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wu,
> Hao A
> Sent: Friday, June 03, 2016 10:14 AM
> To: Tapan Shah; edk2-devel@lists.01.org
> Cc: Carsey, Jaben; Tian, Feng
> Subject: Re: [edk2] [PATCH] MdeModulePkg/RamDiskDxe: Add Memory Type
> selection support in Ramdisk HII
> 
> Hi Tapan,
> 
> Two comments below.
> 
> Best Regards
> Hao Wu
> 
> > -Original Message-
> > From: Tapan Shah [mailto:tapands...@hpe.com]
> > Sent: Friday, June 03, 2016 2:34 AM
> > To: edk2-devel@lists.01.org
> > Cc: samer.el-haj-mahm...@hpe.com; Carsey, Jaben; Wu, Hao A; Tian, Feng;
> > Tapan Shah
> > Subject: [PATCH] MdeModulePkg/RamDiskDxe: Add Memory Type selection
> > support in Ramdisk HII
> >
> > Adding an option in HII menu so user can choose memory type to use when
> > creating a RAM Disk in system.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Tapan Shah 
> > ---
> >  .../Universal/Disk/RamDiskDxe/RamDiskHii.vfr   | 12 
> >  .../Disk/RamDiskDxe/RamDiskHiiStrings.uni  |  7 -
> >  .../Universal/Disk/RamDiskDxe/RamDiskImpl.c| 33
> +--
> > ---
> >  .../Universal/Disk/RamDiskDxe/RamDiskNVData.h  | 10 +++
> >  4 files changed, 54 insertions(+), 8 deletions(-)
> >
> > diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHii.vfr
> > b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHii.vfr
> > index 9c3e3e4..c8bb78b 100644
> > --- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHii.vfr
> > +++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHii.vfr
> > @@ -2,6 +2,7 @@
> >  //  VFR file used by the RamDiskDxe driver.
> >  //
> >  //  Copyright (c) 2016, Intel Corporation. All rights reserved.
> > +//  (C) Copyright 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
> >  //  which accompanies this distribution.  The full text of the license may 
> > be
> > found at
> > @@ -31,6 +32,17 @@ formset
> >form formid = MAIN_FORM_ID,
> >  title  = STRING_TOKEN(STR_MAIN_FORM_TITLE);
> >
> > +oneof varid = RAM_DISK_CONFIGURATION.MemType,
> > +  questionid  = CREATE_RAW_MEMORY_TYPE_QUESTION_ID,
> > +prompt  = STRING_TOKEN(STR_MEMORY_TYPE_PROMPT),
> > +help= STRING_TOKEN(STR_MEMORY_TYPE_HELP),
> > +flags   = INTERACTIVE,
> > +option text =
> > STRING_TOKEN(STR_RAM_DISK_BOOT_SERVICE_DATA_MEMORY), value =
> > RAM_DISK_BOOT_SERVICE_DATA_MEMORY, flags = DEFAULT;
> > +option text = STRING_TOKEN(STR_RAM_DISK_RESERVED_MEMORY),
> > value = RAM_DISK_RESERVED_MEMORY, flags = 0;
> > +endoneof;
> > +
> > +subtitle text = STRING_TOKEN(STR_RAM_DISK_NULL_STRING);
> > +
> >  goto CREATE_RAW_RAM_DISK_FORM_ID,
> >prompt = STRING_TOKEN(STR_GOTO_ADD_RAW_FORM),
> >help   = STRING_TOKEN(STR_GOTO_ADD_RAW_FORM_HELP);
> > diff --git
> a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHiiStrings.uni
> > b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHiiStrings.uni
> > index ee5f6e6..9cc0b08 100644
> > --- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHiiStrings.uni
> > +++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskHiiStrings.uni
> > @@ -2,7 +2,7 @@
> >  // String definitions for RamDiskDxe driver form.
> >  //
> >  // Copyright (c) 2016, Intel Corporation. All rights reserved.
> > -//
> > +// (C) Copyright 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
> >  // which accompanies this distribution. The full text of the license may be
> > found at
> > @@ -36,6 +36,11 @@
> >  #string STR_SIZE_PROMPT#language en-US "Size (Hex):"
> >  #string STR_SIZE_HELP  #language en-US "The valid RAM disk 
> > size
> > should be multiples of the RAM disk block size."
> >
> > +#string STR_MEMORY_TYPE_PROMPT

Re: [edk2] [PATCH] MdeModulePkg/AtaBusDxe: Fix some ATA hard drives cannot be discovered

[Wu, Hao A]

Pushed at 2d273c8db95430b680e542e38cf07b97f9b57d11.

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Cinnamon Shia
> Sent: Monday, June 06, 2016 12:08 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A; el...@hpe.com; Tian, Feng; Zeng, Star
> Subject: [edk2] [PATCH] MdeModulePkg/AtaBusDxe: Fix some ATA hard drives
> cannot be discovered
> 
> If there is no multiplier, the DEV bit of the ATA device register would
> always be set. It causes that some ATA hard drives don't response the
> ATA identity command sent to them.
> 
> Below is the description about DEV bit in ATA spec:
> A device is selected when the DEV bit of the Device register is equal to
> the device number assigned to the device by means of a Device 0/Device 1
> jumper or switch, or use of the CSEL signal.
> 
> Below is the description about DEV bit in SATA spec:
> When the DEV bit in the Device register is set to one, selecting the
> non-existent Device 1, the host adapter shall respond to register reads
> and writes as specified for a Device 0 with no Device 1 present, as
> defined in the ATA/ATAPI-5 standard.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Cinnamon Shia 
> ---
>  MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> index a3008f9..a3739fc 100644
> --- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> +++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> @@ -11,6 +11,7 @@
>Cylinder register.
> 
>Copyright (c) 2009 - 2013, Intel Corporation. All rights reserved.
> +  (C) Copyright 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
>which accompanies this distribution.  The full text of the license may be 
> found
> at
> @@ -413,7 +414,7 @@ DiscoverAtaDevice (
>//
>Acb = ZeroMem (>Acb, sizeof (EFI_ATA_COMMAND_BLOCK));
>Acb->AtaCommand = ATA_CMD_IDENTIFY_DRIVE;
> -  Acb->AtaDeviceHead = (UINT8) (BIT7 | BIT6 | BIT5 | (AtaDevice-
> >PortMultiplierPort << 4));
> +  Acb->AtaDeviceHead = (UINT8) (BIT7 | BIT6 | BIT5 | (AtaDevice-
> >PortMultiplierPort == 0x ? 0 : (AtaDevice->PortMultiplierPort << 4)));
> 
>//
>// Prepare for ATA pass through packet.
> --
> 2.8.1.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] MdeModulePkg/AtaBusDxe: Fix some ATA hard drives cannot be discovered

[Wu, Hao A]

The patch is good to me.

Reviewed-by: Hao Wu 

Best Regards,
Hao Wu

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Cinnamon Shia
> Sent: Monday, June 06, 2016 12:08 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A; el...@hpe.com; Tian, Feng; Zeng, Star
> Subject: [edk2] [PATCH] MdeModulePkg/AtaBusDxe: Fix some ATA hard drives
> cannot be discovered
> 
> If there is no multiplier, the DEV bit of the ATA device register would
> always be set. It causes that some ATA hard drives don't response the
> ATA identity command sent to them.
> 
> Below is the description about DEV bit in ATA spec:
> A device is selected when the DEV bit of the Device register is equal to
> the device number assigned to the device by means of a Device 0/Device 1
> jumper or switch, or use of the CSEL signal.
> 
> Below is the description about DEV bit in SATA spec:
> When the DEV bit in the Device register is set to one, selecting the
> non-existent Device 1, the host adapter shall respond to register reads
> and writes as specified for a Device 0 with no Device 1 present, as
> defined in the ATA/ATAPI-5 standard.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Cinnamon Shia 
> ---
>  MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> index a3008f9..a3739fc 100644
> --- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> +++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> @@ -11,6 +11,7 @@
>Cylinder register.
> 
>Copyright (c) 2009 - 2013, Intel Corporation. All rights reserved.
> +  (C) Copyright 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
>which accompanies this distribution.  The full text of the license may be 
> found
> at
> @@ -413,7 +414,7 @@ DiscoverAtaDevice (
>//
>Acb = ZeroMem (>Acb, sizeof (EFI_ATA_COMMAND_BLOCK));
>Acb->AtaCommand = ATA_CMD_IDENTIFY_DRIVE;
> -  Acb->AtaDeviceHead = (UINT8) (BIT7 | BIT6 | BIT5 | (AtaDevice-
> >PortMultiplierPort << 4));
> +  Acb->AtaDeviceHead = (UINT8) (BIT7 | BIT6 | BIT5 | (AtaDevice-
> >PortMultiplierPort == 0x ? 0 : (AtaDevice->PortMultiplierPort << 4)));
> 
>//
>// Prepare for ATA pass through packet.
> --
> 2.8.1.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] MdeModulePkg/AtaBusDxe: Fix some ATA hard drives cannot be discovered

[Tian, Feng]

I think you are right

Reviewed-by: Feng Tian 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Cinnamon 
Shia
Sent: Monday, June 6, 2016 12:08 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A ; el...@hpe.com; Tian, Feng 
; Zeng, Star 
Subject: [edk2] [PATCH] MdeModulePkg/AtaBusDxe: Fix some ATA hard drives cannot 
be discovered

If there is no multiplier, the DEV bit of the ATA device register would always 
be set. It causes that some ATA hard drives don't response the ATA identity 
command sent to them.

Below is the description about DEV bit in ATA spec:
A device is selected when the DEV bit of the Device register is equal to the 
device number assigned to the device by means of a Device 0/Device 1 jumper or 
switch, or use of the CSEL signal.

Below is the description about DEV bit in SATA spec:
When the DEV bit in the Device register is set to one, selecting the 
non-existent Device 1, the host adapter shall respond to register reads and 
writes as specified for a Device 0 with no Device 1 present, as defined in the 
ATA/ATAPI-5 standard.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Cinnamon Shia 
---
 MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c 
b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
index a3008f9..a3739fc 100644
--- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
+++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
@@ -11,6 +11,7 @@
   Cylinder register.
 
   Copyright (c) 2009 - 2013, Intel Corporation. All rights reserved.
+  (C) Copyright 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
   which accompanies this distribution.  The full text of the license may be 
found at @@ -413,7 +414,7 @@ DiscoverAtaDevice (
   //
   Acb = ZeroMem (>Acb, sizeof (EFI_ATA_COMMAND_BLOCK));
   Acb->AtaCommand = ATA_CMD_IDENTIFY_DRIVE;
-  Acb->AtaDeviceHead = (UINT8) (BIT7 | BIT6 | BIT5 | 
(AtaDevice->PortMultiplierPort << 4));
+  Acb->AtaDeviceHead = (UINT8) (BIT7 | BIT6 | BIT5 | 
+ (AtaDevice->PortMultiplierPort == 0x ? 0 : 
+ (AtaDevice->PortMultiplierPort << 4)));
 
   //
   // Prepare for ATA pass through packet.
--
2.8.1.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] [PATCH 2/2] EmbeddedPkg: Added device configuration protocol

[Daniil Egranov]

The device configuration protocol definition.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daniil Egranov 
---
 EmbeddedPkg/EmbeddedPkg.dec|   1 +
 .../Include/Protocol/EmbeddedDeviceConfig.h| 127 +
 2 files changed, 128 insertions(+)
 create mode 100644 EmbeddedPkg/Include/Protocol/EmbeddedDeviceConfig.h

diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index 775d863..7e1d9ed 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -69,6 +69,7 @@
   gAndroidFastbootPlatformProtocolGuid =  { 0x524685a0, 0x89a0, 0x11e3, {0x9d, 
0x4d, 0xbf, 0xa9, 0xf6, 0xa4, 0x03, 0x08}}
   gUsbDeviceProtocolGuid =  { 0x021bd2ca, 0x51d2, 0x11e3, {0x8e, 0x56, 0xb7, 
0x54, 0x17, 0xc7,  0x0b, 0x44 }}
   gPlatformGpioProtocolGuid = { 0x52ce9845, 0x5af4, 0x43e2, {0xba, 0xfd, 0x23, 
0x08, 0x12, 0x54, 0x7a, 0xc2 }}
+  gEfiEmbeddedDeviceConfigProtocolGuid = { 0x735F8C64, 0xD696, 0x44D0, { 0xBD, 
0xF2, 0x44, 0x7F, 0xD0, 0x5A, 0x54, 0x06 }}
 
 [PcdsFeatureFlag.common]
   gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|FALSE|BOOLEAN|0x0001
diff --git a/EmbeddedPkg/Include/Protocol/EmbeddedDeviceConfig.h 
b/EmbeddedPkg/Include/Protocol/EmbeddedDeviceConfig.h
new file mode 100644
index 000..f397036
--- /dev/null
+++ b/EmbeddedPkg/Include/Protocol/EmbeddedDeviceConfig.h
@@ -0,0 +1,127 @@
+/** @file
+*
+*  Copyright (c) 2016, 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.
+*
+**/
+
+#ifndef __EMBEDDED_DEVICE_CONFIG_H__
+#define __EMBEDDED_DEVICE_CONFIG_H__
+
+//
+// Protocol GUID
+//
+#define EFI_EMBEDDED_DEVICE_CONFIG_PROTOCOL_GUID { 0x735F8C64, 0xD696, 0x44D0, 
{ 0xBD, 0xF2, 0x44, 0x7F, 0xD0, 0x5A, 0x54, 0x06 }}
+
+//
+// Protocol interface structure
+//
+typedef struct _EFI_EMBEDDED_DEVICE_CONFIG_PROTOCOL 
EFI_EMBEDDED_DEVICE_CONFIG_PROTOCOL;
+
+#define EFI_EMBEDDED_DEVICE_CONFIG_REVISION  0x0001
+
+// Device configuration types and structures
+typedef enum {
+  EfiConfigMacAddress,
+  EfiMaxDeviceConfigurationType
+} EFI_DEVICE_CONFIGURATION_TYPE;
+
+typedef struct {
+  BOOLEANWritable;
+  EFI_DEVICE_CONFIGURATION_TYPE  Type;
+} EFI_DEVICE_CONFIGURATION_BLOCK;
+
+typedef struct {
+  EFI_HANDLE Controller;
+  UINT32 SupportedConfigs;
+  EFI_DEVICE_CONFIGURATION_BLOCK ConfigList[EfiMaxDeviceConfigurationType];
+} EFI_DEVICE_CONFIGURATION_RECORD;
+
+//
+// Function Prototypes
+//
+
+/**
+  Check for device configuration support.
+
+  @param This   Instance pointer for this protocol
+
+  @retval BufferArray of configuration records.
+  @retval RecordCount   Number of configuration records.
+  @retval EFI_SUCCESS   Device supports external configuration.
+  @retval EFI_UNSUPPORTED   Device does not support external configuration.
+  @retval EFI_DEVICE_ERROR  Device configuration read error.
+**/
+
+typedef
+EFI_STATUS
+(EFIAPI *EFI_EMBEDDED_DEVICE_CONFIG_SUPPORTED) (
+IN  EFI_EMBEDDED_DEVICE_CONFIG_PROTOCOL  *This,
+OUT EFI_DEVICE_CONFIGURATION_RECORD  **Buffer,
+OUT UINTN*RecordCount
+);
+
+/**
+  Set device configuration.
+
+  @param This   Instance pointer for this protocol.
+  @param Controller Handle of device to configure.
+  @param ConfigType Configuration type.
+  @param Buffer Configuration data.
+
+  @retval EFI_SUCCESS   Device configured successfully.
+  @retval EFI_UNSUPPORTED   Device does not support specified configuration or
+device configuration is read only.
+  @retval EFI_DEVICE_ERROR  Device configuration failed.
+
+**/
+
+typedef
+EFI_STATUS
+(EFIAPI *EFI_EMBEDDED_DEVICE_CONFIG_SET) (
+IN EFI_EMBEDDED_DEVICE_CONFIG_PROTOCOL  *This,
+IN EFI_HANDLE   Controller,
+IN EFI_DEVICE_CONFIGURATION_TYPEConfigType,
+IN VOID *Buffer
+);
+
+/**
+  Get device configuration.
+
+  @param This   Instance pointer for this protocol
+  @param Controller Handle of device to get a configuration.
+  @param ConfigType Configuration type to update.
+
+  @retval BufferConfiguration data.
+  @retval EFI_SUCCESS   Configuration data is available.
+  @retval EFI_UNSUPPORTED   Device does not support specified configuration.
+  @retval EFI_DEVICE_ERROR  Device configuration read error.
+
+**/
+
+typedef
+EFI_STATUS
+(EFIAPI 

[edk2] [PATCH 1/2] ArmPlatformPkg/ArmJunoDxe: Configure Marvell Yukon MAC address on Juno

[Daniil Egranov]

The patch is using device configuration protocol to push new MAC address 
to the Marvell Yukon NIC. A MAC address has been read from FPGA registers 
and pushed to the NIC before its driver started by BDS. 

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daniil Egranov 
---
 .../ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 100 -
 .../ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf   |   1 +
 .../Drivers/ArmJunoDxe/ArmJunoDxeInternal.h|   3 +
 3 files changed, 101 insertions(+), 3 deletions(-)

diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c 
b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
index dba1fcd..4acfbb4 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
@@ -17,6 +17,10 @@
 
 #include 
 #include 
+#include 
+#include 
+
+#include 
 
 #include 
 #include 
@@ -97,9 +101,22 @@ OnEndOfDxe (
   IN VOID   *Context
   )
 {
-  EFI_DEVICE_PATH_PROTOCOL* PciRootComplexDevicePath;
-  EFI_HANDLEHandle;
-  EFI_STATUSStatus;
+  EFI_DEVICE_PATH_PROTOCOL*PciRootComplexDevicePath;
+  EFI_HANDLE  Handle;
+  EFI_STATUS  Status;
+  EFI_EMBEDDED_DEVICE_CONFIG_PROTOCOL *DeviceConfig;
+  UINTN   HandleCount;
+  EFI_HANDLE  *HandleBuffer;
+  UINTN   HIndex;
+  UINTN   CIndex;
+  UINTN   TIndex;
+  EFI_PCI_IO_PROTOCOL*PciIo;
+  UINT64  PciID;
+  EFI_DEVICE_CONFIGURATION_RECORD *ConfigBuffer;
+  UINTN   ConfigBufferSize;
+  EFI_MAC_ADDRESS MacAddress;
+  UINT32  MacHigh;
+  UINT32  MacLow;
 
   //
   // PCI Root Complex initialization
@@ -114,6 +131,83 @@ OnEndOfDxe (
 
   Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath, 
FALSE);
   ASSERT_EFI_ERROR (Status);
+
+  //
+  // Apply platform specific device configurations through the device 
configuration
+  // protocol before switching to the BDS
+  //
+  Status = gBS->LocateHandleBuffer (ByProtocol,
+,
+NULL, , );
+
+  if (!EFI_ERROR (Status)) {
+for (HIndex = 0; HIndex < HandleCount; ++HIndex) {
+  Status = gBS->OpenProtocol (
+  HandleBuffer[HIndex],
+  ,
+  (VOID **) ,
+  NULL,
+  NULL,
+  EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+
+  if (EFI_ERROR (Status)) {
+continue;
+  }
+
+  Status = DeviceConfig->Supported (DeviceConfig,
+,
+);
+
+  if (!EFI_ERROR (Status)) {
+if (ConfigBufferSize > 0 && ConfigBuffer != NULL) {
+  for (CIndex = 0; CIndex < ConfigBufferSize; CIndex++) {
+
+//check for the Juno Marvell Yukon controller
+Status = gBS->OpenProtocol (
+ConfigBuffer[CIndex].Controller,
+,
+(VOID **) ,
+NULL,
+NULL,
+EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+if (EFI_ERROR (Status)) {
+  continue;
+}
+
+Status = PciIo->Pci.Read (PciIo,
+  EfiPciIoWidthUint32,
+  PCI_VENDOR_ID_OFFSET,
+  1,
+  );
+if (EFI_ERROR (Status)) {
+  continue;
+}
+
+for (TIndex = 0; TIndex < ConfigBuffer[CIndex].SupportedConfigs; 
TIndex++) {
+  if ((PciID & 0x) == JUNO_MARVELL_YUKON_ID &&
+  ConfigBuffer[CIndex].ConfigList[TIndex].Type == 
EfiConfigMacAddress) {
+
+MacHigh= MmioRead32 (ARM_JUNO_SYS_PCIGBE_H);
+MacLow = MmioRead32 (ARM_JUNO_SYS_PCIGBE_L);
+CopyMem((VOID *) [0], (CONST VOID *), 
4);
+CopyMem((VOID *) [4], (CONST VOID *), 
2);
+
+Status = DeviceConfig->Set (DeviceConfig,
+ConfigBuffer[CIndex].Controller,
+EfiConfigMacAddress,
+(VOID *));
+if (EFI_ERROR (Status)) {
+  DEBUG ((EFI_D_ERROR, "ArmJunoDxe: Failed to configure 
Marvell Yukon controller\n"));
+}
+  }
+}
+  }
+  FreePool(ConfigBuffer);
+}
+  }
+}
+

[edk2] [PATCH 0/2] Marvell Yukon MAC address configuration on Juno platform

[Daniil Egranov]

The patches add a new device configuration protocol and use it to configure 
a Marvell Yukon controller on the Juno platform with a valid MAC address. 

The device configuration protocol will enable a driver to reconfigure device
specific settings before driver is started. As an example, the Juno Marvell
Yukon NIC MAC address which has to be configured before the driver bound to
the controller. The device configuration protocol defined as generic and
supports multiple controllers per driver and multiple types of configuration 
per controller. Depending on the driver's protocol implementation, the 
configuration can be defined as "read/write" or "read only" and set to or 
get from a driver based on specified controller and its supported 
configuration type. 

Daniil Egranov (2):
  ArmPlatformPkg/ArmJunoDxe: Configure Marvell Yukon MAC address on Juno
  EmbeddedPkg: Added device configure protocol

 .../ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 100 +++-
 .../ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf   |   1 +
 .../Drivers/ArmJunoDxe/ArmJunoDxeInternal.h|   3 +
 EmbeddedPkg/EmbeddedPkg.dec|   1 +
 .../Include/Protocol/EmbeddedDeviceConfig.h| 127 +
 5 files changed, 229 insertions(+), 3 deletions(-)
 create mode 100644 EmbeddedPkg/Include/Protocol/EmbeddedDeviceConfig.h

-- 
2.7.4

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

Re: [edk2] [PATCH V2 0/4] Add DESTRUCTOR for SmmLockBoxSmmLib and PiDxeS3BootScriptLib

[Laszlo Ersek]

On 06/06/16 12:38, Star Zeng wrote:
> V2:
> 1. Fix typo in SmmLockBoxSmmConstructuor and SmmLockBoxSmmDestructuor.
> 2. Add comment for S3BootScriptLibDeinitialize to explain that the desturctor 
> doesn't
> support unloading as a separate action, and it only supports unloading if the
> containing driver's entry point function fails.
> 3. Revert git commit 058196bbb345.
> 
> Cc: Jiewen Yao 
> Cc: Laszlo Ersek 
> Star Zeng (4):
>   MdeModulePkg SmmLockBoxSmmLib: Fix typo in SmmLockBoxSmmConstructuor
>   MdeModulePkg SmmLockBoxSmmLib: Add DESTRUCTOR SmmLockBoxSmmDestructor
>   MdeModulePkg DxeS3BootScriptLib: Add DESTRUCTOR
> S3BootScriptLibDeinitialize
>   MdeModulePkg DxeS3BootScriptLib: Revert git commit 058196bbb345
> 
>  .../Library/PiDxeS3BootScriptLib/BootScriptSave.c  | 159 
> -
>  .../PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf|   4 +-
>  .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.c   |  49 ++-
>  .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf |   5 +-
>  4 files changed, 171 insertions(+), 46 deletions(-)

I tested the series in the following scenarios:

- Ia32 build of OVMF, Q35, SMM_REQUIRE, S3 disabled and enabled
- Ia32X64 build of OVMF, Q35, SMM_REQUIRE, S3 disabled and enabled
- X64 build of OVMF, i440fx, S3 disabled and enabled

Everything seems to work fine. I can see the
S3BootScriptLibDeinitialize() message (from patch #3) in all three
S3-disabled cases, every time from two modules (S3SaveStateDxe and
BootScriptExecutorDxe).

Series
Tested-by: Laszlo Ersek 

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

Re: [edk2] [PATCH V2 4/4] MdeModulePkg DxeS3BootScriptLib: Revert git commit 058196bbb345

[Laszlo Ersek]

On 06/06/16 12:38, Star Zeng wrote:
> With a destructor implemented, the shortcut from 058196bbb345
> should be unnecessary.
> 
> Cc: Jiewen Yao 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> Suggested-by: Laszlo Ersek 
> ---
>  MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c   | 4 
>  MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf | 1 -
>  2 files changed, 5 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c 
> b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> index e84195ded547..625d141481fb 100644
> --- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> @@ -443,10 +443,6 @@ S3BootScriptLibInitialize (
>BOOLEANInSmm;
>EFI_PHYSICAL_ADDRESS   Buffer;
>  
> -  if (!PcdGetBool (PcdAcpiS3Enable)) {
> -return RETURN_SUCCESS;
> -  }
> -
>S3TablePtr = 
> (SCRIPT_TABLE_PRIVATE_DATA*)(UINTN)PcdGet64(PcdS3BootScriptTablePrivateDataPtr);
>//
>// The Boot script private data is not be initialized. create it
> diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf 
> b/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
> index 0f4151180f6a..de314db47922 100644
> --- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
> +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
> @@ -71,5 +71,4 @@ [Pcd]
>## SOMETIMES_PRODUCES
>gEfiMdeModulePkgTokenSpaceGuid.PcdS3BootScriptTablePrivateSmmDataPtr
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdS3BootScriptRuntimeTableReservePageNumber   
> ## CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable 
>## CONSUMES
>  
> 

Reviewed-by: Laszlo Ersek 

I'll provide testing feedback as well, shortly.

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

Re: [edk2] [PATCH V2 3/4] MdeModulePkg DxeS3BootScriptLib: Add DESTRUCTOR S3BootScriptLibDeinitialize

[Laszlo Ersek]

Two cosmetic comments:

On 06/06/16 12:38, Star Zeng wrote:
> PiDxeS3BootScriptLib has a constructor S3BootScriptLibInitialize() that
> registers ready-to-lock callback S3BootScriptSmmEventCallBack() and several
> more. The library is linked to SMM modules. If the module entry-point
> function returns error (because of lack of resources, unsupported,
> whatever), the module will be unloaded and the notify callback pointers
> will point to undefined memory. On ready-to-lock exception occurs when
> calling S3BootScriptSmmEventCallBack(), and probably all the other
> callbacks registered by the constructor would also cause exception.
> 
> This patch is to implement library Destructor to free the resources
> allocated by S3BootScriptLibInitialize() and unregister callbacks.
> 
> Cc: Jiewen Yao 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> Reviewed-by: Jiewen Yao 
> ---
>  .../Library/PiDxeS3BootScriptLib/BootScriptSave.c  | 155 
> -
>  .../PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf|   3 +-
>  2 files changed, 124 insertions(+), 34 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c 
> b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> index d954503f864d..e84195ded547 100644
> --- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> @@ -1,7 +1,7 @@
>  /** @file
> -  Save the S3 data to S3 boot script. 
> - 
> -  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
> +  Save the S3 data to S3 boot script.
> +
> +  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
>  
>This program and the accompanying materials
>are licensed and made available under the terms and conditions
> @@ -124,6 +124,14 @@ EFI_GUID 
> mBootScriptSmmPrivateDataGuid = {
>0x627ee2da, 0x3bf9, 0x439b, { 0x92, 0x9f, 0x2e, 0xe, 0x6e, 0x9d, 0xba, 
> 0x62 }
>  };
>  
> +EFI_EVENTmEventDxeSmmReadyToLock = NULL;
> +VOID *mRegistrationSmmExitBootServices = NULL;
> +VOID *mRegistrationSmmLegacyBoot = NULL;
> +VOID *mRegistrationSmmReadyToLock = NULL;
> +BOOLEAN  mS3BootScriptTableAllocated = FALSE;
> +BOOLEAN  mS3BootScriptTableSmmAllocated = FALSE;
> +EFI_SMM_SYSTEM_TABLE2*mSmst = NULL;
> +
>  /**
>This is an internal function to add a terminate node the entry, 
> recalculate the table 
>length and fill into the table. 
> @@ -417,8 +425,8 @@ S3BootScriptSmmAtRuntimeCallBack (
>@param  ImageHandle   The firmware allocated handle for the EFI image.
>@param  SystemTable   A pointer to the EFI System Table.
>  
> -  @retval  RETURN_SUCCESSAllocate the global memory space to 
> store S3 boot script table private data
> -  @retval  RETURN_OUT_OF_RESOURCES   No enough memory to allocated.
> +  @retval RETURN_SUCCESSThe constructor always returns EFI_SUCCESS.
> +
>  **/
>  RETURN_STATUS
>  EFIAPI
> @@ -433,9 +441,7 @@ S3BootScriptLibInitialize (
>VOID   *Registration;
>EFI_SMM_BASE2_PROTOCOL *SmmBase2;
>BOOLEANInSmm;
> -  EFI_SMM_SYSTEM_TABLE2  *Smst;
>EFI_PHYSICAL_ADDRESS   Buffer;
> -  EFI_EVENT  Event;
>  
>if (!PcdGetBool (PcdAcpiS3Enable)) {
>  return RETURN_SUCCESS;
> @@ -453,25 +459,24 @@ S3BootScriptLibInitialize (
>  EFI_SIZE_TO_PAGES(sizeof(SCRIPT_TABLE_PRIVATE_DATA)),
>  
>  );
> -if (EFI_ERROR (Status)) {
> -  return RETURN_OUT_OF_RESOURCES;
> -}
> +ASSERT_EFI_ERROR (Status);
> +mS3BootScriptTableAllocated = TRUE;
>  S3TablePtr = (VOID *) (UINTN) Buffer;
>  
>  Status = PcdSet64S (PcdS3BootScriptTablePrivateDataPtr, (UINT64) 
> (UINTN)S3TablePtr);
>  ASSERT_EFI_ERROR (Status);
> -ZeroMem (S3TablePtr, sizeof(SCRIPT_TABLE_PRIVATE_DATA));  
> +ZeroMem (S3TablePtr, sizeof(SCRIPT_TABLE_PRIVATE_DATA));
>  //
>  // Create event to notify the library system enter the SmmLocked phase.
>  //
> -Event = EfiCreateProtocolNotifyEvent  (
> -  ,
> -  TPL_CALLBACK,
> -  S3BootScriptEventCallBack,
> -  NULL,
> -  
> -  );
> -ASSERT (Event != NULL);
> +mEventDxeSmmReadyToLock = EfiCreateProtocolNotifyEvent (
> +,
> +TPL_CALLBACK,
> +S3BootScriptEventCallBack,
> +NULL,
> +
> +);
> +ASSERT 

Re: [edk2] [PATCH V2 1/4] MdeModulePkg SmmLockBoxSmmLib: Fix typo in SmmLockBoxSmmConstructuor

[Laszlo Ersek]

On 06/06/16 12:38, Star Zeng wrote:
> SmmLockBoxSmmConstructuor should be SmmLockBoxSmmConstructor.
> 
> Cc: Jiewen Yao 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> ---
>  MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c   | 10 +-
>  MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf |  4 ++--
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c 
> b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
> index 23a786d56ec5..38be18560fe9 100644
> --- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
> +++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
> @@ -1,6 +1,6 @@
>  /** @file
>  
> -Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
>  
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions
> @@ -73,7 +73,7 @@ InternalGetSmmLockBoxContext (
>  **/
>  EFI_STATUS
>  EFIAPI
> -SmmLockBoxSmmConstructuor (
> +SmmLockBoxSmmConstructor (
>IN EFI_HANDLEImageHandle,
>IN EFI_SYSTEM_TABLE  *SystemTable
>)
> @@ -81,7 +81,7 @@ SmmLockBoxSmmConstructuor (
>EFI_STATUS   Status;
>SMM_LOCK_BOX_CONTEXT *SmmLockBoxContext;
>  
> -  DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxSmmConstructuor - 
> Enter\n"));
> +  DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxSmmConstructor - 
> Enter\n"));
>  
>//
>// Check if gEfiSmmLockBoxCommunicationGuid is installed by someone
> @@ -93,7 +93,7 @@ SmmLockBoxSmmConstructuor (
>  // No need to install again, just return.
>  //
>  DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxContext - already 
> installed\n"));
> -DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxSmmConstructuor - 
> Exit\n"));
> +DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxSmmConstructor - 
> Exit\n"));
>  return EFI_SUCCESS;
>}
>  
> @@ -117,7 +117,7 @@ SmmLockBoxSmmConstructuor (
>  
>DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxContext - %x\n", 
> (UINTN)));
>DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib LockBoxDataAddress - %x\n", 
> (UINTN)));
> -  DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxSmmConstructuor - 
> Exit\n"));
> +  DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxSmmConstructor - Exit\n"));
>  
>return Status;
>  }
> diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf 
> b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf
> index 6edb47519a86..d722f57a66bd 100644
> --- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf
> +++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  SMM LockBox library instance.
>  #
> -#  Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
>  #
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions
> @@ -22,7 +22,7 @@ [Defines]
>MODULE_TYPE= DXE_SMM_DRIVER
>VERSION_STRING = 1.0
>LIBRARY_CLASS  = LockBoxLib|DXE_SMM_DRIVER
> -  CONSTRUCTOR= SmmLockBoxSmmConstructuor
> +  CONSTRUCTOR= SmmLockBoxSmmConstructor
>  
>  #
>  # The following information is for reference only and not required by the 
> build tools.
> 

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

Re: [edk2] [PATCH V2 2/4] MdeModulePkg SmmLockBoxSmmLib: Add DESTRUCTOR SmmLockBoxSmmDestructor

[Laszlo Ersek]

On 06/06/16 12:38, Star Zeng wrote:
> SmmLockBoxSmmLib is linked to SMM modules. If the module entry-point
> function returns error, the module will be unloaded and the global
> variables will point to undefined memory.
> 
> This patch is to add DESTRUCTOR SmmLockBoxSmmDestructor to uninstall
> SmmLockBoxCommunication configuration table if it has been installed
> in Constructor.
> 
> Cc: Jiewen Yao 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> Reviewed-by: Jiewen Yao 
> ---
>  .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.c   | 39 
> ++
>  .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf |  1 +
>  2 files changed, 40 insertions(+)
> 
> diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c 
> b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
> index 38be18560fe9..04afe54accb8 100644
> --- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
> +++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
> @@ -30,6 +30,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>  SMM_LOCK_BOX_CONTEXT mSmmLockBoxContext;
>  LIST_ENTRY   mLockBoxQueue = INITIALIZE_LIST_HEAD_VARIABLE 
> (mLockBoxQueue);
>  
> +BOOLEAN  mSmmConfigurationTableInstalled = FALSE;
> +
>  /**
>This function return SmmLockBox context from SMST.
>  
> @@ -114,6 +116,7 @@ SmmLockBoxSmmConstructor (
>  sizeof(mSmmLockBoxContext)
>  );
>ASSERT_EFI_ERROR (Status);
> +  mSmmConfigurationTableInstalled = TRUE;
>  
>DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxContext - %x\n", 
> (UINTN)));
>DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib LockBoxDataAddress - %x\n", 
> (UINTN)));
> @@ -123,6 +126,42 @@ SmmLockBoxSmmConstructor (
>  }
>  
>  /**
> +  Destructor for SmmLockBox library.
> +  This is used to uninstall SmmLockBoxCommunication configuration table
> +  if it has been installed in Constructor.
> +
> +  @param[in] ImageHandle  Image handle of this driver.
> +  @param[in] SystemTable  A Pointer to the EFI System Table.
> +
> +  @retval EFI_SUCEESS 
> +  @return Others  Some error occurs.
> +**/
> +EFI_STATUS
> +EFIAPI
> +SmmLockBoxSmmDestructor (
> +  IN EFI_HANDLEImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  EFI_STATUS   Status;
> +
> +  DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxSmmDestructor in %a 
> module\n", gEfiCallerBaseName));
> +
> +  if (mSmmConfigurationTableInstalled) {
> +Status = gSmst->SmmInstallConfigurationTable (
> +  gSmst,
> +  ,
> +  NULL,
> +  0
> +  );
> +ASSERT_EFI_ERROR (Status);
> +DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib uninstall SmmLockBoxCommunication 
> configuration table\n"));
> +  }
> +
> +  return Status;
> +}

The value returned above should be constant EFI_SUCCESS.

Namely, if "mSmmConfigurationTableInstalled" is FALSE, then Status will
never be assigned, and the function will return an indeterminate value.

With that fix:

Reviewed-by: Laszlo Ersek 

> +
> +/**
>This function return SmmLockBox queue address.
>  
>@return SmmLockBox queue address.
> diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf 
> b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf
> index d722f57a66bd..eb7ba0bb2e89 100644
> --- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf
> +++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf
> @@ -23,6 +23,7 @@ [Defines]
>VERSION_STRING = 1.0
>LIBRARY_CLASS  = LockBoxLib|DXE_SMM_DRIVER
>CONSTRUCTOR= SmmLockBoxSmmConstructor
> +  DESTRUCTOR = SmmLockBoxSmmDestructor
>  
>  #
>  # The following information is for reference only and not required by the 
> build tools.
> 

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

Re: [edk2] [PATCH v2 0/2] ArmVirtPkg: introduce the progress bar

[Laszlo Ersek]

On 06/06/16 11:50, Ard Biesheuvel wrote:
> On 27 May 2016 at 12:42, Laszlo Ersek  wrote:
>> Version 2 of .
>> The Nt32Pkg, MdeModulePkg, and OvmfPkg patches have been committed; so
>> this series only carries the ArmVirtPkg patches, with the second of
>> those being adapted to Nt32Pkg (see the commit reference in the notes on
>> patch #2).
>>
>> Public branch: .
>>
>> Copying Leif because Ard is on vacation, and because Ard might want to
>> port these changes to ArmPkg (in which case Leif will review them
>> anyway).
>>
>> Cc: Ard Biesheuvel 
>> Cc: Leif Lindholm 
>>
> 
> Reviewed-by: Ard Biesheuvel 

Thank you. Commit range 0d0c245dfb14..be266b10907a.

Also, welcome back! :)

Laszlo

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

[edk2] [PATCH V2 3/4] MdeModulePkg DxeS3BootScriptLib: Add DESTRUCTOR S3BootScriptLibDeinitialize

[Star Zeng]

PiDxeS3BootScriptLib has a constructor S3BootScriptLibInitialize() that
registers ready-to-lock callback S3BootScriptSmmEventCallBack() and several
more. The library is linked to SMM modules. If the module entry-point
function returns error (because of lack of resources, unsupported,
whatever), the module will be unloaded and the notify callback pointers
will point to undefined memory. On ready-to-lock exception occurs when
calling S3BootScriptSmmEventCallBack(), and probably all the other
callbacks registered by the constructor would also cause exception.

This patch is to implement library Destructor to free the resources
allocated by S3BootScriptLibInitialize() and unregister callbacks.

Cc: Jiewen Yao 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
Reviewed-by: Jiewen Yao 
---
 .../Library/PiDxeS3BootScriptLib/BootScriptSave.c  | 155 -
 .../PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf|   3 +-
 2 files changed, 124 insertions(+), 34 deletions(-)

diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c 
b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
index d954503f864d..e84195ded547 100644
--- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
+++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
@@ -1,7 +1,7 @@
 /** @file
-  Save the S3 data to S3 boot script. 
- 
-  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+  Save the S3 data to S3 boot script.
+
+  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions
@@ -124,6 +124,14 @@ EFI_GUID 
mBootScriptSmmPrivateDataGuid = {
   0x627ee2da, 0x3bf9, 0x439b, { 0x92, 0x9f, 0x2e, 0xe, 0x6e, 0x9d, 0xba, 0x62 }
 };
 
+EFI_EVENTmEventDxeSmmReadyToLock = NULL;
+VOID *mRegistrationSmmExitBootServices = NULL;
+VOID *mRegistrationSmmLegacyBoot = NULL;
+VOID *mRegistrationSmmReadyToLock = NULL;
+BOOLEAN  mS3BootScriptTableAllocated = FALSE;
+BOOLEAN  mS3BootScriptTableSmmAllocated = FALSE;
+EFI_SMM_SYSTEM_TABLE2*mSmst = NULL;
+
 /**
   This is an internal function to add a terminate node the entry, recalculate 
the table 
   length and fill into the table. 
@@ -417,8 +425,8 @@ S3BootScriptSmmAtRuntimeCallBack (
   @param  ImageHandle   The firmware allocated handle for the EFI image.
   @param  SystemTable   A pointer to the EFI System Table.
 
-  @retval  RETURN_SUCCESSAllocate the global memory space to store 
S3 boot script table private data
-  @retval  RETURN_OUT_OF_RESOURCES   No enough memory to allocated.
+  @retval RETURN_SUCCESSThe constructor always returns EFI_SUCCESS.
+
 **/
 RETURN_STATUS
 EFIAPI
@@ -433,9 +441,7 @@ S3BootScriptLibInitialize (
   VOID   *Registration;
   EFI_SMM_BASE2_PROTOCOL *SmmBase2;
   BOOLEANInSmm;
-  EFI_SMM_SYSTEM_TABLE2  *Smst;
   EFI_PHYSICAL_ADDRESS   Buffer;
-  EFI_EVENT  Event;
 
   if (!PcdGetBool (PcdAcpiS3Enable)) {
 return RETURN_SUCCESS;
@@ -453,25 +459,24 @@ S3BootScriptLibInitialize (
 EFI_SIZE_TO_PAGES(sizeof(SCRIPT_TABLE_PRIVATE_DATA)),
 
 );
-if (EFI_ERROR (Status)) {
-  return RETURN_OUT_OF_RESOURCES;
-}
+ASSERT_EFI_ERROR (Status);
+mS3BootScriptTableAllocated = TRUE;
 S3TablePtr = (VOID *) (UINTN) Buffer;
 
 Status = PcdSet64S (PcdS3BootScriptTablePrivateDataPtr, (UINT64) 
(UINTN)S3TablePtr);
 ASSERT_EFI_ERROR (Status);
-ZeroMem (S3TablePtr, sizeof(SCRIPT_TABLE_PRIVATE_DATA));  
+ZeroMem (S3TablePtr, sizeof(SCRIPT_TABLE_PRIVATE_DATA));
 //
 // Create event to notify the library system enter the SmmLocked phase.
 //
-Event = EfiCreateProtocolNotifyEvent  (
-  ,
-  TPL_CALLBACK,
-  S3BootScriptEventCallBack,
-  NULL,
-  
-  );
-ASSERT (Event != NULL);
+mEventDxeSmmReadyToLock = EfiCreateProtocolNotifyEvent (
+,
+TPL_CALLBACK,
+S3BootScriptEventCallBack,
+NULL,
+
+);
+ASSERT (mEventDxeSmmReadyToLock != NULL);
   }
   mS3BootScriptTablePtr = S3TablePtr;
 
@@ -492,7 +497,7 @@ S3BootScriptLibInitialize (
   //
   // Good, we are in SMM
   //
-  Status = SmmBase2->GetSmstLocation (SmmBase2, );
+  Status = SmmBase2->GetSmstLocation (SmmBase2, );
   if (EFI_ERROR (Status)) {
 return 

[edk2] [PATCH V2 4/4] MdeModulePkg DxeS3BootScriptLib: Revert git commit 058196bbb345

[Star Zeng]

With a destructor implemented, the shortcut from 058196bbb345
should be unnecessary.

Cc: Jiewen Yao 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
Suggested-by: Laszlo Ersek 
---
 MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c   | 4 
 MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf | 1 -
 2 files changed, 5 deletions(-)

diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c 
b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
index e84195ded547..625d141481fb 100644
--- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
+++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
@@ -443,10 +443,6 @@ S3BootScriptLibInitialize (
   BOOLEANInSmm;
   EFI_PHYSICAL_ADDRESS   Buffer;
 
-  if (!PcdGetBool (PcdAcpiS3Enable)) {
-return RETURN_SUCCESS;
-  }
-
   S3TablePtr = 
(SCRIPT_TABLE_PRIVATE_DATA*)(UINTN)PcdGet64(PcdS3BootScriptTablePrivateDataPtr);
   //
   // The Boot script private data is not be initialized. create it
diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf 
b/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
index 0f4151180f6a..de314db47922 100644
--- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
+++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
@@ -71,5 +71,4 @@ [Pcd]
   ## SOMETIMES_PRODUCES
   gEfiMdeModulePkgTokenSpaceGuid.PcdS3BootScriptTablePrivateSmmDataPtr
   gEfiMdeModulePkgTokenSpaceGuid.PcdS3BootScriptRuntimeTableReservePageNumber  
 ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable   
 ## CONSUMES
 
-- 
2.7.0.windows.1

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

[edk2] [PATCH V2 2/4] MdeModulePkg SmmLockBoxSmmLib: Add DESTRUCTOR SmmLockBoxSmmDestructor

[Star Zeng]

SmmLockBoxSmmLib is linked to SMM modules. If the module entry-point
function returns error, the module will be unloaded and the global
variables will point to undefined memory.

This patch is to add DESTRUCTOR SmmLockBoxSmmDestructor to uninstall
SmmLockBoxCommunication configuration table if it has been installed
in Constructor.

Cc: Jiewen Yao 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
Reviewed-by: Jiewen Yao 
---
 .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.c   | 39 ++
 .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf |  1 +
 2 files changed, 40 insertions(+)

diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
index 38be18560fe9..04afe54accb8 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
@@ -30,6 +30,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 SMM_LOCK_BOX_CONTEXT mSmmLockBoxContext;
 LIST_ENTRY   mLockBoxQueue = INITIALIZE_LIST_HEAD_VARIABLE 
(mLockBoxQueue);
 
+BOOLEAN  mSmmConfigurationTableInstalled = FALSE;
+
 /**
   This function return SmmLockBox context from SMST.
 
@@ -114,6 +116,7 @@ SmmLockBoxSmmConstructor (
 sizeof(mSmmLockBoxContext)
 );
   ASSERT_EFI_ERROR (Status);
+  mSmmConfigurationTableInstalled = TRUE;
 
   DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxContext - %x\n", 
(UINTN)));
   DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib LockBoxDataAddress - %x\n", 
(UINTN)));
@@ -123,6 +126,42 @@ SmmLockBoxSmmConstructor (
 }
 
 /**
+  Destructor for SmmLockBox library.
+  This is used to uninstall SmmLockBoxCommunication configuration table
+  if it has been installed in Constructor.
+
+  @param[in] ImageHandle  Image handle of this driver.
+  @param[in] SystemTable  A Pointer to the EFI System Table.
+
+  @retval EFI_SUCEESS 
+  @return Others  Some error occurs.
+**/
+EFI_STATUS
+EFIAPI
+SmmLockBoxSmmDestructor (
+  IN EFI_HANDLEImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS   Status;
+
+  DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxSmmDestructor in %a 
module\n", gEfiCallerBaseName));
+
+  if (mSmmConfigurationTableInstalled) {
+Status = gSmst->SmmInstallConfigurationTable (
+  gSmst,
+  ,
+  NULL,
+  0
+  );
+ASSERT_EFI_ERROR (Status);
+DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib uninstall SmmLockBoxCommunication 
configuration table\n"));
+  }
+
+  return Status;
+}
+
+/**
   This function return SmmLockBox queue address.
 
   @return SmmLockBox queue address.
diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf
index d722f57a66bd..eb7ba0bb2e89 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf
@@ -23,6 +23,7 @@ [Defines]
   VERSION_STRING = 1.0
   LIBRARY_CLASS  = LockBoxLib|DXE_SMM_DRIVER
   CONSTRUCTOR= SmmLockBoxSmmConstructor
+  DESTRUCTOR = SmmLockBoxSmmDestructor
 
 #
 # The following information is for reference only and not required by the 
build tools.
-- 
2.7.0.windows.1

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

[edk2] [PATCH V2 1/4] MdeModulePkg SmmLockBoxSmmLib: Fix typo in SmmLockBoxSmmConstructuor

[Star Zeng]

SmmLockBoxSmmConstructuor should be SmmLockBoxSmmConstructor.

Cc: Jiewen Yao 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c   | 10 +-
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
index 23a786d56ec5..38be18560fe9 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
@@ -1,6 +1,6 @@
 /** @file
 
-Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions
@@ -73,7 +73,7 @@ InternalGetSmmLockBoxContext (
 **/
 EFI_STATUS
 EFIAPI
-SmmLockBoxSmmConstructuor (
+SmmLockBoxSmmConstructor (
   IN EFI_HANDLEImageHandle,
   IN EFI_SYSTEM_TABLE  *SystemTable
   )
@@ -81,7 +81,7 @@ SmmLockBoxSmmConstructuor (
   EFI_STATUS   Status;
   SMM_LOCK_BOX_CONTEXT *SmmLockBoxContext;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxSmmConstructuor - Enter\n"));
+  DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxSmmConstructor - Enter\n"));
 
   //
   // Check if gEfiSmmLockBoxCommunicationGuid is installed by someone
@@ -93,7 +93,7 @@ SmmLockBoxSmmConstructuor (
 // No need to install again, just return.
 //
 DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxContext - already 
installed\n"));
-DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxSmmConstructuor - 
Exit\n"));
+DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxSmmConstructor - Exit\n"));
 return EFI_SUCCESS;
   }
 
@@ -117,7 +117,7 @@ SmmLockBoxSmmConstructuor (
 
   DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxContext - %x\n", 
(UINTN)));
   DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib LockBoxDataAddress - %x\n", 
(UINTN)));
-  DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxSmmConstructuor - Exit\n"));
+  DEBUG ((EFI_D_INFO, "SmmLockBoxSmmLib SmmLockBoxSmmConstructor - Exit\n"));
 
   return Status;
 }
diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf
index 6edb47519a86..d722f57a66bd 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf
@@ -1,7 +1,7 @@
 ## @file
 #  SMM LockBox library instance.
 #
-#  Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.
+#  Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions
@@ -22,7 +22,7 @@ [Defines]
   MODULE_TYPE= DXE_SMM_DRIVER
   VERSION_STRING = 1.0
   LIBRARY_CLASS  = LockBoxLib|DXE_SMM_DRIVER
-  CONSTRUCTOR= SmmLockBoxSmmConstructuor
+  CONSTRUCTOR= SmmLockBoxSmmConstructor
 
 #
 # The following information is for reference only and not required by the 
build tools.
-- 
2.7.0.windows.1

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

[edk2] [PATCH V2 0/4] Add DESTRUCTOR for SmmLockBoxSmmLib and PiDxeS3BootScriptLib

[Star Zeng]

V2:
1. Fix typo in SmmLockBoxSmmConstructuor and SmmLockBoxSmmDestructuor.
2. Add comment for S3BootScriptLibDeinitialize to explain that the desturctor 
doesn't
support unloading as a separate action, and it only supports unloading if the
containing driver's entry point function fails.
3. Revert git commit 058196bbb345.

Cc: Jiewen Yao 
Cc: Laszlo Ersek 
Star Zeng (4):
  MdeModulePkg SmmLockBoxSmmLib: Fix typo in SmmLockBoxSmmConstructuor
  MdeModulePkg SmmLockBoxSmmLib: Add DESTRUCTOR SmmLockBoxSmmDestructor
  MdeModulePkg DxeS3BootScriptLib: Add DESTRUCTOR
S3BootScriptLibDeinitialize
  MdeModulePkg DxeS3BootScriptLib: Revert git commit 058196bbb345

 .../Library/PiDxeS3BootScriptLib/BootScriptSave.c  | 159 -
 .../PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf|   4 +-
 .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.c   |  49 ++-
 .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf |   5 +-
 4 files changed, 171 insertions(+), 46 deletions(-)

-- 
2.7.0.windows.1

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

Re: [edk2] [PATCH 2/2] MdeModulePkg DxeS3BootScriptLib: Add DESTRUCTOR S3BootScriptLibDeinitialize

[Laszlo Ersek]

On 06/04/16 01:26, Yao, Jiewen wrote:
> HI Laszlo/Star
> 
> That is a good catch that this patch does not consider DriverX load,
> DriverY load, then DriverX unload.
> 
>  
> 
> Technically, it is possible for a UEFI driver according UEFI spec, by
> using *EFI_BOOT_SERVICES.Exit()*to terminates a loaded EFI image and
> returns control to boot services.
> 
> It need *EFI_LOADED_IMAGE_PROTOCOL.Unload()*
> 
> I did see some UEFI driver, like Network, USB, support this function.
> And we did that before to unload these drivers in UEFI shell.
> 
>  
> 
> However, BootScript is an PI interface and it should not be linked by an
> UEFI driver.
> 
> And there might be another issue need consider, for example, if we need
> remove the boot script registered by this driver?
> 
>  
> 
> Personally, I think it is too theoretical. And the behavior is undocumented.
> 
>  
> 
> I recommend we document this as an unsupported case.
> 

Works for me!

(And, in this case, 058196bbb345 should be reverted as third patch.)

Thanks!
Laszlo

>  
> 
> If we really want to support this case, we can have PI spec to clarify
> how to handle registered boot script when driver is unload.
> 
>  
> 
> Thank you
> 
> Yao Jiewen
> 
>  
> 
> *From:*Zeng, Star
> *Sent:* Friday, June 3, 2016 9:22 PM
> *To:* Laszlo Ersek ; edk2-de...@ml01.01.org
> *Cc:* Yao, Jiewen 
> *Subject:* Re: [edk2] [PATCH 2/2] MdeModulePkg DxeS3BootScriptLib: Add
> DESTRUCTOR S3BootScriptLibDeinitialize
> 
>  
> 
> On 2016/6/3 16:42, Laszlo Ersek wrote:
>> On 06/03/16 04:09, Star Zeng wrote:
>>> PiDxeS3BootScriptLib has a constructor S3BootScriptLibInitialize() that
>>> registers ready-to-lock callback S3BootScriptSmmEventCallBack() and several
>>> more. The library is linked to SMM modules. If the module entry-point
>>> function returns error (because of lack of resources, unsupported,
>>> whatever), the module will be unloaded and the notify callback pointers
>>> will point to undefined memory. On ready-to-lock exception occurs when
>>> calling S3BootScriptSmmEventCallBack(), and probably all the other
>>> callbacks registered by the constructor would also cause exception.
>>>
>>> This patch is to implement library Destructor to free the resources
>>> allocated by S3BootScriptLibInitialize() and unregister callbacks.
>>>
>>> Cc: Jiewen Yao >
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Star Zeng >
>>> ---
>>>  .../Library/PiDxeS3BootScriptLib/BootScriptSave.c  | 138 
>>> +
>>>  .../PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf|   3 +-
>>>  2 files changed, 115 insertions(+), 26 deletions(-)
>>
>> (Apologies about sending a second message; I should have included this
>> in my other email, but I hadn't thought of it just yet.)
> 
> That is fine.
> 
>>
>> I think this patch may not do the right thing when drivers that it gets
>> linked into are unloadable. For example, assume that driver X is loaded
>> first, allocating resources, setting PCDs, and registering events. Then
>> driver Y is loaded, reusing the resources allocated by driver X, through
>> the PCDs that were set by driver X.
>>
>> If, at this point, driver X is unloaded, then driver Y will have
>> dangling references.
>>
>> Perhaps the above situation is purely theoretical -- I'm not sure.
>> Reference counting would probably be overkill.
>>
>> Can we perhaps add a conspicuous comment somewhere that explains that
>> the desturctor doesn't support unloading as a separate action, and it
>> only supports unloading if the containing driver's entry point function
>> fails?
>>
>> Do you think this is a justified concern? I grepped
>> BootScriptExecutorDxe, S3SaveStateDxe, and SmmS3SaveState for the string
>> "unload" (case-insensitively), and found no match. So in practice the
>> patch is correct, but maybe we should warn against false impressions the
>> destructor might give.
> 
> Ok, there are two approaches for the concern.
> 1. As you suggested, add a conspicuous comment somewhere.
> 2. Only close event and unregister callbacks in the destructor, and do 
> not free the allocated buffer.
> 
> How do you think?
> 
> Thanks,
> Star
> 
>>
>> Thanks
>> Laszlo
>>
>>
>>> diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c 
>>> b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
>>> index d954503f864d..2d53d57668b8 100644
>>> --- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
>>> +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
>>> @@ -1,7 +1,7 @@
>>>  /** @file
>>> -  Save the S3 data to S3 boot script.
>>> -
>>> -  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
>>> +  Save the S3 data to S3 boot script.
>>> +
>>> +  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
>>>
>>>This program and the accompanying materials
>>>are 

Re: [edk2] [Patch] NetworkPkg: Fix DNS GeneralLookUp failure in some case

[Ye, Ting]

Reviewed-by: Ye Ting  

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jiaxin Wu
Sent: Monday, June 06, 2016 1:33 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Zhang, Lubo 
Subject: [edk2] [Patch] NetworkPkg: Fix DNS GeneralLookUp failure in some case

It's incorrect to check the value 1(DNS_CLASS_INET) of QClass.
Moreover, the 'Identification' and 'Type' filed in Query packet should not be 
changed to little endian since the packet maybe retransmitted while there is 
any error happened.

Cc: Ye Ting 
Cc: Zhang Lubo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/DnsDxe/DnsImpl.c | 29 +  
NetworkPkg/DnsDxe/DnsImpl.h |  2 ++
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c index 
4f10e17..360f68e 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -1041,10 +1041,11 @@ AddDns6ServerIp (
   Find out whether the response is valid or invalid.
 
   @param  TokensMap   All DNS transmittal Tokens entry.  
   @param  Identification  Identification for queried packet.  
   @param  TypeType for queried packet.
+  @param  Class   Class for queried packet.
   @param  ItemReturn corresponding Token entry.
 
   @retval TRUEThe response is valid.
   @retval FALSE   The response is invalid.
   
@@ -1052,10 +1053,11 @@ AddDns6ServerIp (  BOOLEAN  IsValidDnsResponse (
   IN NET_MAP  *TokensMap,
   IN UINT16   Identification,
   IN UINT16   Type,
+  IN UINT16   Class,
  OUT NET_MAP_ITEM **Item
   )
 {
   LIST_ENTRY  *Entry;
 
@@ -1075,21 +1077,20 @@ IsValidDnsResponse (
   TxString = NetbufGetByte (Packet, 0, NULL);
   ASSERT (TxString != NULL);
   DnsHeader = (DNS_HEADER *) TxString;
   QueryName = (CHAR8 *) (TxString + sizeof (*DnsHeader));
   QuerySection = (DNS_QUERY_SECTION *) (QueryName + AsciiStrLen 
(QueryName) + 1);
-
-  DnsHeader->Identification = NTOHS (DnsHeader->Identification);
-  QuerySection->Type = NTOHS (QuerySection->Type);
 
-  if (DnsHeader->Identification == Identification && QuerySection->Type == 
Type) {
+  if (NTOHS (DnsHeader->Identification) == Identification &&
+  NTOHS (QuerySection->Type) == Type && 
+  NTOHS (QuerySection->Class) == Class) {
 return TRUE;
   }
 } 
   }
   
-  *Item =NULL;
+  *Item = NULL;
   
   return FALSE;
 }
 
 /**
@@ -1193,19 +1194,31 @@ ParseDnsResponse (
 
   //
   // Check DnsResponse Validity, if so, also get a valid NET_MAP_ITEM.
   //
   if (Instance->Service->IpVersion == IP_VERSION_4) {
-if (!IsValidDnsResponse (>Dns4TxTokens, 
DnsHeader->Identification, QuerySection->Type, )) {
+if (!IsValidDnsResponse (
+   >Dns4TxTokens, 
+   DnsHeader->Identification, 
+   QuerySection->Type,
+   QuerySection->Class,
+   
+   )) {
   *Completed = FALSE;
   Status = EFI_ABORTED;
   goto ON_EXIT;
 }
 ASSERT (Item != NULL);
 Dns4TokenEntry = (DNS4_TOKEN_ENTRY *) (Item->Key);
   } else {
-if (!IsValidDnsResponse (>Dns6TxTokens, 
DnsHeader->Identification, QuerySection->Type, )) {
+if (!IsValidDnsResponse (
+   >Dns6TxTokens, 
+   DnsHeader->Identification, 
+   QuerySection->Type,
+   QuerySection->Class,
+   
+   )) {
   *Completed = FALSE;
   Status = EFI_ABORTED;
   goto ON_EXIT;
 }
 ASSERT (Item != NULL);
@@ -1214,11 +1227,11 @@ ParseDnsResponse (

   //
   // Continue Check Some Errors.
   //
   if (DnsHeader->Flags.Bits.RCode != DNS_FLAGS_RCODE_NO_ERROR || 
DnsHeader->AnswersNum < 1 || \
-  DnsHeader->Flags.Bits.QR != DNS_FLAGS_QR_RESPONSE || QuerySection->Class 
!= DNS_CLASS_INET) {
+  DnsHeader->Flags.Bits.QR != DNS_FLAGS_QR_RESPONSE) {
   Status = EFI_ABORTED;
   goto ON_EXIT;
   }
 
   //
diff --git a/NetworkPkg/DnsDxe/DnsImpl.h b/NetworkPkg/DnsDxe/DnsImpl.h index 
0ef8255..5fa7f24 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.h
+++ b/NetworkPkg/DnsDxe/DnsImpl.h
@@ -559,10 +559,11 @@ AddDns6ServerIp (
   Find out whether the response is valid or invalid.
 
   @param  TokensMap   All DNS transmittal Tokens entry.  
   @param  Identification  Identification for queried packet.  
   @param  TypeType for queried packet.
+  @param  Class   Class for queried packet.
   @param  ItemReturn corresponding Token entry.
 
   @retval TRUEThe response is valid.
   @retval FALSE   The response is invalid.
   
@@ -570,10 +571,11 @@ AddDns6ServerIp (
 BOOLEAN
 IsValidDnsResponse (
   IN NET_MAP  *TokensMap,
   IN UINT16   

Re: [edk2] [PATCH 0/2] UefiCpuPkg, OvmfPkg: prevent (unchecked) SMM stack overflow

[Fan, Jeff]

Laszlo,

I agree. Please go ahead to check-in this serial of patches. We may revisit it 
if we receive other concern in the future.

Thanks!
Jeff

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Thursday, June 02, 2016 6:13 PM
To: Fan, Jeff; edk2-devel-01
Cc: Yao, Jiewen; Justen, Jordan L; Kinney, Michael D
Subject: Re: [PATCH 0/2] UefiCpuPkg, OvmfPkg: prevent (unchecked) SMM stack 
overflow

On 06/02/16 04:15, Fan, Jeff wrote:
> Reviewed-by: Jeff Fan 
> 
> Good suggestion to set PcdCpuSmmStackGuard default to TRUE. 

Thanks!

> But please hold on the check-in in UefiCpuPkg.dec till next week, I 
> want more platforms to be validated on this change.

Actually, even if this change causes problems for those out-of-tree platforms, 
I think it should be committed to the open source repo, and those other 
platforms should set PcdCpuSmmStackGuard to FALSE in their DSC files.

This is actually the entire point of writing patch #1 for UefiCpuPkg (the .dec 
file) and not for the OVMF DSC files. I don't just want to make it safe for 
OVMF -- instead, I'd like to make it safe for all other future platforms that 
consume the open source edk2 tree, so that their developers don't have to go 
through the same analysis that I had to.

This is what "opt out" means -- if PcdCpuSmmStackGuard=TRUE breaks those 
external platforms, then
(a) they should be fixed anyway, or
(b) if they aren't broken, just cannot afford the extra guard page, then they 
should opt out of the stack guard.

I'm aware that this could be an incompatible change for external platforms with 
a very tight SMRAM budget, but even that way, it is the right thing to do in 
the open source tree, for future consumers of edk2.

Thanks
Laszlo

> 
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, June 02, 2016 3:03 AM
> To: edk2-devel-01
> Cc: Fan, Jeff; Yao, Jiewen; Justen, Jordan L; Kinney, Michael D
> Subject: [PATCH 0/2] UefiCpuPkg, OvmfPkg: prevent (unchecked) SMM 
> stack overflow
> 
> These patches are the result of the investigation at 
> .
> 
> Cc: Jeff Fan 
> Cc: Jiewen Yao 
> Cc: Jordan Justen 
> Cc: Michael D Kinney 
> 
> Laszlo Ersek (2):
>   UefiCpuPkg: change PcdCpuSmmStackGuard default to TRUE
>   OvmfPkg: set SMM stack size to 16KB
> 
>  UefiCpuPkg/UefiCpuPkg.dec  | 4 ++--
>  OvmfPkg/OvmfPkgIa32.dsc| 1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
>  OvmfPkg/OvmfPkgX64.dsc | 1 +
>  4 files changed, 5 insertions(+), 2 deletions(-)
> 
> --
> 1.8.3.1
> 

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