[edk2] [staging/HTTPS-TLS][PATCH] NetworkPkg: Handle HTTPS indefinite poll case

[Jiaxin Wu]

This patch is used to handle handle HTTPS indefinite poll
case.

Cc: El-Haj-Mahmoud Samer 
Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Long Qin 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/HttpDxe/HttpImpl.c  | 55 ++
 NetworkPkg/HttpDxe/HttpProto.c | 68 ++
 2 files changed, 93 insertions(+), 30 deletions(-)

diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index cf58b13..cd8fe05 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -1179,47 +1179,50 @@ HttpResponseWorker (
 }
   }
 
   ASSERT (HttpInstance->MsgParser != NULL);
 
-  //
-  // We still need receive more data when there is no cache data and MsgParser 
is not NULL;
-  //
-  if (!HttpInstance->UseHttps) {
-if (HttpInstance->TimeoutEvent == NULL) {
-  //
-  // Create TimeoutEvent for response
-  //
-  Status = gBS->CreateEvent (
-  EVT_TIMER,
-  TPL_CALLBACK,
-  NULL,
-  NULL,
-  &HttpInstance->TimeoutEvent
-  );
-  if (EFI_ERROR (Status)) {
-goto Error;
-  }
-}
-
+  if (HttpInstance->TimeoutEvent == NULL) {
 //
-// Start the timer, and wait Timeout seconds to receive the body packet.
+// Create TimeoutEvent for response
 //
-Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, 
HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND);
+Status = gBS->CreateEvent (
+EVT_TIMER,
+TPL_CALLBACK,
+NULL,
+NULL,
+&HttpInstance->TimeoutEvent
+);
 if (EFI_ERROR (Status)) {
   goto Error;
 }
-  
+  }
+
+  //
+  // Start the timer, and wait Timeout seconds to receive the body packet.
+  //
+  Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, 
HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND);
+  if (EFI_ERROR (Status)) {
+goto Error;
+  }
+
+  //
+  // We still need receive more data when there is no cache data and MsgParser 
is not NULL;
+  //
+  if (!HttpInstance->UseHttps) {
 Status = HttpTcpReceiveBody (Wrap, HttpMsg, HttpInstance->TimeoutEvent);
 
 gBS->SetTimer (HttpInstance->TimeoutEvent, TimerCancel, 0);
 
 if (EFI_ERROR (Status)) {
   goto Error;
 }
   } else {
-Status = HttpsReceive (HttpInstance, &Fragment, NULL);
+Status = HttpsReceive (HttpInstance, &Fragment, 
HttpInstance->TimeoutEvent);
+
+gBS->SetTimer (HttpInstance->TimeoutEvent, TimerCancel, 0);
+
 if (EFI_ERROR (Status)) {
   goto Error;
 }
 
 //
@@ -1315,11 +1318,13 @@ Exit:
 
 Error2:
   NetMapInsertHead (&HttpInstance->TxTokens, ValueInItem->HttpToken, 
ValueInItem);
 
 Error:
-  HttpTcpTokenCleanup (Wrap);
+  if (!HttpInstance->UseHttps) {
+HttpTcpTokenCleanup (Wrap);
+  }
   
   if (HttpHeaders != NULL) {
 FreePool (HttpHeaders);
 HttpHeaders = NULL;
   }
diff --git a/NetworkPkg/HttpDxe/HttpProto.c b/NetworkPkg/HttpDxe/HttpProto.c
index 965a49f..ebb9dd9 100644
--- a/NetworkPkg/HttpDxe/HttpProto.c
+++ b/NetworkPkg/HttpDxe/HttpProto.c
@@ -1230,11 +1230,40 @@ HttpConnectTcp4 (
   
   //
   // Tls session connection.
   //
   if (HttpInstance->UseHttps) {
-Status = TlsConnectSession (HttpInstance, NULL); 
+if (HttpInstance->TimeoutEvent == NULL) {
+  //
+  // Create TimeoutEvent for TLS connection.
+  //
+  Status = gBS->CreateEvent (
+  EVT_TIMER,
+  TPL_CALLBACK,
+  NULL,
+  NULL,
+  &HttpInstance->TimeoutEvent
+  );
+  if (EFI_ERROR (Status)) {
+TlsCloseTxRxEvent (HttpInstance);
+return Status;
+  }
+}
+
+//
+// Start the timer, and wait Timeout seconds for connection.
+//
+Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, 
HTTP_CONNECTION_TIMEOUT * TICKS_PER_SECOND);
+if (EFI_ERROR (Status)) {
+  TlsCloseTxRxEvent (HttpInstance);
+  return Status;
+}
+
+Status = TlsConnectSession (HttpInstance, HttpInstance->TimeoutEvent);
+
+gBS->SetTimer (HttpInstance->TimeoutEvent, TimerCancel, 0);
+
 if (EFI_ERROR (Status)) {
   TlsCloseTxRxEvent (HttpInstance);
   return Status;
 }
   }
@@ -1293,11 +1322,40 @@ HttpConnectTcp6 (
   
   //
   // Tls session connection.
   //
   if (HttpInstance->UseHttps) {
-Status = TlsConnectSession (HttpInstance, NULL); 
+if (HttpInstance->TimeoutEvent == NULL) {
+  //
+  // Create TimeoutEvent for TLS connection.
+  //
+  Status = gBS->CreateEvent (
+  EVT_TIMER,
+  TPL_CALLBACK,
+  NULL,
+  NULL,
+  &HttpInstance->TimeoutEvent
+

Re: [edk2] Two PCI IO protocols getting produced by same GUID, how to open correct one?

[Shaveta Leekha]

Thanks for the reply
My replies are in-lined.

Thanks and Regards,
Shaveta

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Kinney, 
Michael D
Sent: Tuesday, May 03, 2016 9:29 PM
To: Shaveta Leekha ; Laszlo Ersek ; 
Kinney, Michael D 
Cc: edk2-devel@lists.01.org 
Subject: Re: [edk2] Two PCI IO protocols getting produced by same GUID, how to 
open correct one?

Shaveta,

As long as all PCI I/O Protocol instances in the platform conform to UEFI spec, 
it can work.  

[Shaveta] Correct. 

Your original question was about how to tell that a handle has the PCI I/O 
Protocol produced By PCI Bus Driver or your emulation of PCI.  If the PCI I/O 
Protocols are conformant, then It should not matter.  Just write a PCI Device 
Driver that manages a PCI controller based on PCI Class code or Vendor 
ID/Device ID matching and it should work.

[Shaveta] Yes, eventually correct the PCI I/O Protocol produced my PCI 
emulation layer code is getting opened.
As and when a valid protocol gets opened, I am checking the CLASS code and 
Sub-CLASS code before proceeding.
And that's working.

But what if, some other device's driver in my system, intended to open "PCI I/O 
Protocol produced By PCI Bus Driver"
Doesn't test CLASS CODE or any SUB-CLASS/VENDOR etc., then it may land up using 
API's provided with protocol
Of my PCI emulation layer code? is that understanding correct ??

So in such scenarios, where more than one protocol instances (with different 
APIs associated) are installed with same GUID in the system,
Then all the drivers should check VENDOR ID/CLASS etc immediately after opening 
the protocol?



Mike


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Shaveta Leekha
> Sent: Tuesday, May 3, 2016 3:27 AM
> To: Laszlo Ersek 
> Cc: edk2-devel@lists.01.org 
> Subject: Re: [edk2] Two PCI IO protocols getting produced by same 
> GUID, how to open correct one?
> 
> 
> 
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Tuesday, May 03, 2016 1:15 PM
> To: Shaveta Leekha 
> Cc: edk2-devel@lists.01.org 
> Subject: Re: [edk2] Two PCI IO protocols getting produced by same 
> GUID, how to open correct one?
> 
> On 05/03/16 08:54, Shaveta Leekha wrote:
> > Hi,
> >
> > I have a scenario where two separate drivers are installing/producing "PCI 
> > IO"
> protocol with same GUID (gEfiPciIoProtocolGuid).
> >
> > Where:
> >
> > (1)One of the PCI Io protocol instance is produced by
> "MdeModulePkg/Bus/Pci/PciBus" driver for real PCI devices to use( like 
> E1000/NIC card to use)
> >
> > (2)Other PCI Io protocol instance is produced by "Pci Emulation" layer 
> > created
> for USB and SATA kind of controllers. This protocol (APIs) is intended 
> to be used by USB and SATA controller drivers.
> >
> > Now when I use "OpenProtocol" in my "Platform specific Sata controller 
> > driver" using:
> >
> > Status = gBS->OpenProtocol (
> >   Controller,
> >   &gEfiPciIoProtocolGuid,
> >   (VOID **) &PciIo,
> >   This->DriverBindingHandle,
> >   Controller,
> >   EFI_OPEN_PROTOCOL_GET_PROTOCOL
> >   );
> >
> > How can I make sure that PCI Io protocol that is produced by 
> > "PciEmulation" layer
> driver is getting opened?
> >
> > As "gEfiPciIoProtocolGuid" is same for both protocol instances.
> >
> > How to handle such scenarios?
> 
> The UEFI spec forbids installing more than one protocol interface with 
> the same GUID on the same handle. See
> EFI_BOOT_SERVICES.InstallProtocolInterface():
> 
>   [... The same GUID cannot be installed more than once onto the same
>   handle. If installation of a duplicate GUID on a handle is attempted,
>   an EFI_INVALID_PARAMETER will result. [...]
> 
>   EFI_INVALID_PARAMETER  Protocol is already installed on the handle
>  specified by Handle.
> 
> If you have two separate UEFI drivers that install PciIo interfaces on 
> handles, then their EFI_DRIVER_BINDING_PROTOCOL.Supported() methods 
> may report success only on distinct sets of handles. There must be no overlap.
> 
> But, in any case, I think it's very unusual to have two drivers that 
> produce PciIo interfaces. According to the "Driver Writer's Guide for 
> UEFI 2.3.1" (03/08/2012, Version 1.01), section "18.2 PCI Bus Drivers":
> 
>   TIP:  PCI Bus Driver customizations are *strongly discouraged*
> because the PCI Bus Driver is designed to be conformant with
> the /PCI Specification/. Instead, focus platform specific
> customizations on the Root Bridge Driver that produced
> EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL and its PCI Device Drivers.
> 
> 
> [Shaveta]
> Yes, it is un-usual and we had discussed over it also.
> 
> So the scenario is:
> 
> On LS2080 platform, we have real PCI controllers that have E1000 card 
> on them(used for net

Re: [edk2] Two PCI IO protocols getting produced by same GUID, how to open correct one?

[Shaveta Leekha]

-Original Message-
From: af...@apple.com [mailto:af...@apple.com] 
Sent: Tuesday, May 03, 2016 9:44 PM
To: Mike Kinney 
Cc: Shaveta Leekha ; Laszlo Ersek ; 
edk2-devel@lists.01.org 
Subject: Re: [edk2] Two PCI IO protocols getting produced by same GUID, how to 
open correct one?


> On May 3, 2016, at 8:59 AM, Kinney, Michael D  
> wrote:
> 
> Shaveta,
> 
> As long as all PCI I/O Protocol instances in the platform conform to UEFI 
> spec, it can work.  
> 
> Your original question was about how to tell that a handle has the PCI 
> I/O Protocol produced By PCI Bus Driver or your emulation of PCI.  If 
> the PCI I/O Protocols are conformant, then It should not matter.  Just 
> write a PCI Device Driver that manages a PCI controller based on PCI Class 
> code or Vendor ID/Device ID matching and it should work.
> 

I would also point out that Subsystem ID and Subsystem Vendor ID can be used 
for this type of binding too. The Subsystem values exist to deal with cases 
when the same chip is used on different vendors PCI plug in cards. 

Thanks,

Andrew Fish

[Shaveta]
Thanks for the reply
My replies are in another mail pasted again here:


[Shaveta] Yes, eventually correct the PCI I/O Protocol produced my PCI 
emulation layer code is getting opened.
As and when a valid protocol gets opened, I am checking the CLASS code and 
Sub-CLASS code before proceeding.
And that's working.

But what if, some other device's driver in my system, intended to open "PCI I/O 
Protocol produced By PCI Bus Driver"
Doesn't test CLASS CODE or any SUB-CLASS/VENDOR etc., then it may land up using 
API's provided with protocol Of my PCI emulation layer code? is that 
understanding correct ??

So in such scenarios, where more than one protocol instances (with different 
APIs associated) are installed with same GUID in the system, Then all the 
drivers should check VENDOR ID/CLASS etc immediately after opening the protocol?


Thanks and Regards,
Shaveta







> Mike
> 
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
>> Of Shaveta Leekha
>> Sent: Tuesday, May 3, 2016 3:27 AM
>> To: Laszlo Ersek 
>> Cc: edk2-devel@lists.01.org 
>> Subject: Re: [edk2] Two PCI IO protocols getting produced by same 
>> GUID, how to open correct one?
>> 
>> 
>> 
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Tuesday, May 03, 2016 1:15 PM
>> To: Shaveta Leekha 
>> Cc: edk2-devel@lists.01.org 
>> Subject: Re: [edk2] Two PCI IO protocols getting produced by same 
>> GUID, how to open correct one?
>> 
>> On 05/03/16 08:54, Shaveta Leekha wrote:
>>> Hi,
>>> 
>>> I have a scenario where two separate drivers are installing/producing "PCI 
>>> IO"
>> protocol with same GUID (gEfiPciIoProtocolGuid).
>>> 
>>> Where:
>>> 
>>> (1)One of the PCI Io protocol instance is produced by
>> "MdeModulePkg/Bus/Pci/PciBus" driver for real PCI devices to use( 
>> like E1000/NIC card to use)
>>> 
>>> (2)Other PCI Io protocol instance is produced by "Pci Emulation" layer 
>>> created
>> for USB and SATA kind of controllers. This protocol (APIs) is 
>> intended to be used by USB and SATA controller drivers.
>>> 
>>> Now when I use "OpenProtocol" in my "Platform specific Sata controller 
>>> driver" using:
>>> 
>>> Status = gBS->OpenProtocol (
>>>  Controller,
>>>  &gEfiPciIoProtocolGuid,
>>>  (VOID **) &PciIo,
>>>  This->DriverBindingHandle,
>>>  Controller,
>>>  EFI_OPEN_PROTOCOL_GET_PROTOCOL
>>>  );
>>> 
>>> How can I make sure that PCI Io protocol that is produced by 
>>> "PciEmulation" layer
>> driver is getting opened?
>>> 
>>> As "gEfiPciIoProtocolGuid" is same for both protocol instances.
>>> 
>>> How to handle such scenarios?
>> 
>> The UEFI spec forbids installing more than one protocol interface 
>> with the same GUID on the same handle. See
>> EFI_BOOT_SERVICES.InstallProtocolInterface():
>> 
>>  [... The same GUID cannot be installed more than once onto the same  
>> handle. If installation of a duplicate GUID on a handle is attempted,  
>> an EFI_INVALID_PARAMETER will result. [...]
>> 
>>  EFI_INVALID_PARAMETER  Protocol is already installed on the handle
>> specified by Handle.
>> 
>> If you have two separate UEFI drivers that install PciIo interfaces 
>> on handles, then their EFI_DRIVER_BINDING_PROTOCOL.Supported() 
>> methods may report success only on distinct sets of handles. There must be 
>> no overlap.
>> 
>> But, in any case, I think it's very unusual to have two drivers that 
>> produce PciIo interfaces. According to the "Driver Writer's Guide for 
>> UEFI 2.3.1" (03/08/2012, Version 1.01), section "18.2 PCI Bus Drivers":
>> 
>>  TIP:  PCI Bus Driver customizations are *strongly discouraged*
>>because the PCI Bus Driver is designed to be conformant with
>>the /PCI Specification/. Instead, focus platf

Re: [edk2] [PATCH v2] NetworkPkg: Use UefiBootManagerLib API to create load option.

[Ye, Ting]

Reviewed-by: Ye Ting  

-Original Message-
From: Fu, Siyuan 
Sent: Wednesday, May 04, 2016 1:58 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Wu, Jiaxin ; Ni, Ruiyu 

Subject: [PATCH v2] NetworkPkg: Use UefiBootManagerLib API to create load 
option.

V2:
Remove unnecessary ZeroMem and free load option.

This patch updates the HTTP boot driver to use the API in UefiBootManagerLib to 
create new load option, to avoid duplicate code.

Cc: Ye Ting 
Cc: Wu Jiaxin 
Cc: Ni Ruiyu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 NetworkPkg/HttpBootDxe/HttpBootConfig.c | 136 ++--
 NetworkPkg/HttpBootDxe/HttpBootDxe.inf  |   1 +
 NetworkPkg/NetworkPkg.dsc   |   8 ++
 3 files changed, 33 insertions(+), 112 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootConfig.c 
b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
index 00e4782..2ca38b5 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootConfig.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
@@ -13,6 +13,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 **/
 
 #include "HttpBootDxe.h"
+#include 
 
 CHAR16  mHttpBootConfigStorageName[] = L"HTTP_BOOT_CONFIG_IFR_NVDATA";
 
@@ -36,31 +37,18 @@ HttpBootAddBootOption (
   IN   CHAR16   *Uri
   )
 {
-  EFI_DEV_PATH   *Node;
-  EFI_DEVICE_PATH_PROTOCOL   *TmpDevicePath;
-  EFI_DEVICE_PATH_PROTOCOL   *NewDevicePath;
-  UINTN  Length;
-  CHAR8  AsciiUri[URI_STR_MAX_SIZE];
-  CHAR16 *CurrentOrder;
-  EFI_STATUS Status;
-  UINTN  OrderCount;
-  UINTN  TargetLocation;
-  BOOLEANFound;
-  UINT8  *TempByteBuffer;
-  UINT8  *TempByteStart;
-  UINTN  DescSize;
-  UINTN  FilePathSize;
-  CHAR16 OptionStr[10];
-  UINT16 *NewOrder;
-  UINTN  Index;
-
-  NewOrder  = NULL;
-  TempByteStart = NULL;
+  EFI_DEV_PATH  *Node;
+  EFI_DEVICE_PATH_PROTOCOL  *TmpDevicePath;
+  EFI_DEVICE_PATH_PROTOCOL  *NewDevicePath;
+  UINTN Length;
+  CHAR8 AsciiUri[URI_STR_MAX_SIZE];
+  EFI_STATUSStatus;
+  UINTN Index;
+  EFI_BOOT_MANAGER_LOAD_OPTION  NewOption;
+
   NewDevicePath = NULL;
-  NewOrder  = NULL;
   Node  = NULL;
   TmpDevicePath = NULL;
-  CurrentOrder  = NULL;
 
   if (StrLen (Description) == 0) {
 return EFI_INVALID_PARAMETER;
@@ -137,105 +125,29 @@ HttpBootAddBootOption (
   }
 
   //
-  // Get current "BootOrder" variable and find a free target.
-  //
-  Length = 0;
-  Status = GetVariable2 (
- L"BootOrder",
- &gEfiGlobalVariableGuid,
- (VOID **)&CurrentOrder,
- &Length 
- );
-  if (EFI_ERROR (Status) && Status != EFI_NOT_FOUND) {
-goto ON_EXIT;
-  }
-  OrderCount = Length / sizeof (UINT16);
-  Found = FALSE;
-  for (TargetLocation=0; TargetLocation < 0x; TargetLocation++) {
-Found = TRUE;
-for (Index = 0; Index < OrderCount; Index++) {
-  if (CurrentOrder[Index] == TargetLocation) {
-Found = FALSE;
-break;
-  }
-}
-if (Found) {
-  break;
-}
-  }
-
-  if (TargetLocation == 0x) {
-DEBUG ((EFI_D_ERROR, "Could not find unused target index.\n"));
-Status = EFI_OUT_OF_RESOURCES;
-goto ON_EXIT;
-  } else {
-DEBUG ((EFI_D_INFO, "TargetIndex = %04x.\n", TargetLocation));
-  }
-
-  //
-  // Construct and set the "Boot" variable
+  // Add a new load option.
   //
-  DescSize = StrSize(Description);
-  FilePathSize = GetDevicePathSize (NewDevicePath);
-  TempByteBuffer = AllocateZeroPool(sizeof(EFI_LOAD_OPTION) + DescSize + 
FilePathSize);
-  if (TempByteBuffer == NULL) {
-Status = EFI_OUT_OF_RESOURCES;
-goto ON_EXIT;
-  }
-
-  TempByteStart = TempByteBuffer;
-  *((UINT32 *) TempByteBuffer) = LOAD_OPTION_ACTIVE;  // Attributes
-  TempByteBuffer += sizeof (UINT32);
-
-  *((UINT16 *) TempByteBuffer) = (UINT16)FilePathSize;// FilePathListLength
-  TempByteBuffer += sizeof (UINT16);
-
-  CopyMem (TempByteBuffer, Description, DescSize);
-  TempByteBuffer += DescSize;
-  CopyMem (TempByteBuffer, NewDevicePath, FilePathSize);
-
-  UnicodeSPrint (OptionStr, sizeof(OptionStr), L"%s%04x", L"Boot", 
TargetLocation);
-  Status = gRT->SetVariable (
-  OptionStr,
-  &gEfiGlobalVariableGuid,
-  EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS 
| EFI_VARIABLE_RUNTIME_ACCESS,
-  sizeof(UINT32) + sizeof(UINT16) + DescSize + FilePathSize,
-  TempByteStart
-  );
+  Status = EfiBootManagerInitializeLoadOption (
+ &NewOpti

Re: [edk2] [Patch] MdeModulePkg FileExplorerLib: Add UefiHiiServicesLib dependency.

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: Dong, Eric
> Sent: Wednesday, May 04, 2016 2:28 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [Patch] MdeModulePkg FileExplorerLib: Add UefiHiiServicesLib
> dependency.
> 
> FileExplorerLib depends on UefiHiiServicesLib, so add this missing
> library dependency back.
> 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Eric Dong 
> ---
>  MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
> b/MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
> index a97fd4a..9c1f28b 100644
> --- a/MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
> +++ b/MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
> @@ -47,6 +47,7 @@
>BaseMemoryLib
>DebugLib
>HiiLib
> +  UefiHiiServicesLib
> 
>  [Guids]
>gEfiFileSystemVolumeLabelInfoIdGuid   ## CONSUMES ## GUID
> (Indicate the information type is volume)
> --
> 2.6.4.windows.1

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

[edk2] [patch] MdeModulePkg/HiiDatabaseDxe: Fix memory leak issues in HiiDatabaseDxe

[Dandan Bi]

Cc: Qiu Shumin 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
Reviewed-by: Eric Dong 
---
 .../Universal/HiiDatabaseDxe/ConfigRouting.c| 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c 
b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
index cc155cc..a704734 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
@@ -547,10 +547,12 @@ FindSameBlockElement(
   return Status;
 }
 ASSERT (TempBuffer != NULL);
 if ((BufferLen == Length) && (0 == CompareMem (Buffer, TempBuffer, 
Length))) {
   *Found = TRUE;
+  FreePool (TempBuffer);
+  TempBuffer = NULL;
   return EFI_SUCCESS;
 } else {
   FreePool (TempBuffer);
   TempBuffer = NULL;
   BlockPtr = StrStr (BlockPtr + 1, BlockName);
@@ -1885,10 +1887,13 @@ IsThisPackageList (
   AsciiStrToUnicodeStr ((CHAR8 *)IfrVarStore->Name, VarStoreName);
 
   if (IsThisVarstore((VOID *)&IfrVarStore->Guid, VarStoreName, ConfigHdr)) 
{
 FindVarstore = TRUE;
 goto Done;
+  } else {
+FreePool (VarStoreName);
+VarStoreName = NULL;
   }
   break;
 
 case EFI_IFR_VARSTORE_EFI_OP:
   IfrEfiVarStore = (EFI_IFR_VARSTORE_EFI *) IfrOpHdr;
@@ -1899,10 +1904,13 @@ IsThisPackageList (
   AsciiStrToUnicodeStr ((CHAR8 *)IfrEfiVarStore->Name, VarStoreName);
 
   if (IsThisVarstore (&IfrEfiVarStore->Guid, VarStoreName, ConfigHdr)) {
 FindVarstore = TRUE;
 goto Done;
+  } else {
+FreePool (VarStoreName);
+VarStoreName = NULL;
   }
   break;
 
 case EFI_IFR_VARSTORE_NAME_VALUE_OP:
   IfrNameValueVarStore = (EFI_IFR_VARSTORE_NAME_VALUE *) IfrOpHdr;
@@ -2092,10 +2100,11 @@ ParseIfrData (
   BlockData= NULL;
   DefaultDataPtr   = NULL;
   FirstOneOfOption = FALSE;
   VarStoreId   = 0;
   FirstOrderedList = FALSE;
+  VarStoreName = NULL;
   ZeroMem (&DefaultData, sizeof (IFR_DEFAULT_DATA));
 
   //
   // Go through the form package to parse OpCode one by one.
   //
@@ -2149,10 +2158,13 @@ ParseIfrData (
 CopyGuid (&VarStorageData->Guid, (EFI_GUID *) (VOID *) 
&IfrVarStore->Guid);
 VarStorageData->Size   = IfrVarStore->Size;
 VarStorageData->Name   = VarStoreName;
 VarStorageData->Type   = EFI_HII_VARSTORE_BUFFER;
 VarStoreId = IfrVarStore->VarStoreId;
+  } else {
+FreePool (VarStoreName);
+VarStoreName = NULL;
   }
   break;
 
 case EFI_IFR_VARSTORE_EFI_OP:
   //
@@ -2187,10 +2199,13 @@ ParseIfrData (
 CopyGuid (&VarStorageData->Guid, (EFI_GUID *) (VOID *) 
&IfrEfiVarStore->Guid);
 VarStorageData->Size   = IfrEfiVarStore->Size;
 VarStorageData->Name   = VarStoreName;
 VarStorageData->Type   = EFI_HII_VARSTORE_EFI_VARIABLE_BUFFER;
 VarStoreId = IfrEfiVarStore->VarStoreId;
+  } else {
+FreePool (VarStoreName);
+VarStoreName = NULL;
   }
   break;
 
 case EFI_IFR_VARSTORE_NAME_VALUE_OP:
   //
@@ -3012,11 +3027,11 @@ GetBlockElement (
   //
   Status = GetValueOfNumber (StringPtr, &TmpBuffer, &Length);
   if (EFI_ERROR (Status)) {
 goto Done;
   }
-
+  FreePool (TmpBuffer);
   StringPtr += Length;
   if (*StringPtr != 0 && *StringPtr != L'&') {
 goto Done;
   }
 }
@@ -3866,10 +3881,14 @@ Done:
 RemoveEntryList (&DefaultValueData->Entry);
 FreePool (DefaultValueData);
   }
   FreePool (BlockData);
 }
+if (VarStorageData ->Name != NULL) {
+  FreePool (VarStorageData ->Name);
+  VarStorageData ->Name = NULL;
+}
 FreePool (VarStorageData);
   }
 
   if (DefaultIdArray != NULL) {
 //
-- 
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] MdeModulePkg: Refine SNP driver's media status check logic.

[Ye, Ting]

Siyuan,

Some minor comments to the patch:
1. +  // Whether UNDI support cable detect for INITIALIZE command,  // i.e. 
+ PXE_STATFLAGS_CABLE_DETECT_NOT_SUPPORTED

Suggest to also include PXE_STATFLAGS_CABLE_DETECT_SUPPORTED if uses "i.e." 
here.

2. In the function header of PxeGetStatus(), there is a redundant \ need be 
removed. (MediaPresent\  field...)
Also since we add the function declaration to header file, I suggest we update 
the style of param to param[in] or param[out].

Others are good to me.

Reviewed-by: Ye Ting 

Thanks,
Ting


-Original Message-
From: Fu, Siyuan 
Sent: Wednesday, May 04, 2016 9:13 AM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Wu, Jiaxin 
Subject: [Patch] MdeModulePkg: Refine SNP driver's media status check logic.

Some UNDI drivers may not support the cable detect in UNDI INITIALIZE command, 
but support the media present check in UNDI GET_STATUS command. Current SNP 
driver will set the MediaPresentSupported field to FALSE in 
EFI_SIMPLE_NETWORK_MODE for such case, which forbid the media detect from the 
callers.

This patch updates the SNP driver to support such kind of UNDIs for media 
detect.
MediaPresentSupported will be set to TRUE, and a GET_STATUS command will be 
issued immediately after INITIALIZE command in SNP->Initialize() function, to 
refresh the value of MediaPresent in SNP mode data.

Cc: Ye Ting 
Cc: Wu Jiaxin 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 MdeModulePkg/Universal/Network/SnpDxe/Get_status.c |  3 ++-  
MdeModulePkg/Universal/Network/SnpDxe/Initialize.c | 13 +-
 MdeModulePkg/Universal/Network/SnpDxe/Snp.c|  8 --
 MdeModulePkg/Universal/Network/SnpDxe/Snp.h| 30 ++
 4 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Get_status.c 
b/MdeModulePkg/Universal/Network/SnpDxe/Get_status.c
index edbc0f2..3c6bf39 100644
--- a/MdeModulePkg/Universal/Network/SnpDxe/Get_status.c
+++ b/MdeModulePkg/Universal/Network/SnpDxe/Get_status.c
@@ -18,7 +18,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 /**
   Call undi to get the status of the interrupts, get the list of recycled 
transmit
   buffers that completed transmitting. The recycled transmit buffer address 
will
-  be saved into Snp->RecycledTxBuf.
+  be saved into Snp->RecycledTxBuf. This function will also update the 
+ MediaPresent  field of EFI_SIMPLE_NETWORK_MODE if UNDI support it.
 
   @param  Snp Pointer to snp driver structure.
   @param  InterruptStatusPtr  A non null pointer to contain the interrupt
diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c 
b/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c
index 3f40ef3..2151375 100644
--- a/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c
+++ b/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c
@@ -229,7 +229,10 @@ SnpUndi32Initialize (
   //
   Snp->TxRxBufferSize = (UINT32) (Snp->InitInfo.MemoryRequired + 
ExtraRxBufferSize + ExtraTxBufferSize);
 
-  if (Snp->Mode.MediaPresentSupported) {
+  //
+  // If UNDI support cable detect for INITIALIZE command, try it first.
+  //
+  if (Snp->CableDetectSupported) {
 if (PxeInit (Snp, PXE_OPFLAGS_INITIALIZE_DETECT_CABLE) == EFI_SUCCESS) {
   Snp->Mode.MediaPresent = TRUE;
   goto ON_EXIT;
@@ -242,6 +245,14 @@ SnpUndi32Initialize (
 
   if (EFI_ERROR (EfiStatus)) {
 gBS->CloseEvent (Snp->Snp.WaitForPacket);
+goto ON_EXIT;
+  }
+
+  //
+  // Try to update the MediaPresent field of EFI_SIMPLE_NETWORK_MODE if the 
UNDI support it.
+  //
+  if (Snp->MediaStatusSupported) {
+PxeGetStatus (Snp, NULL, FALSE);
   }
 
 ON_EXIT:
diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Snp.c 
b/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
index 5ff294f..9f61aee 100644
--- a/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
+++ b/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
@@ -554,12 +554,12 @@ SimpleNetworkDriverStart (
 
   switch (InitStatFlags & PXE_STATFLAGS_CABLE_DETECT_MASK) {
   case PXE_STATFLAGS_CABLE_DETECT_SUPPORTED:
-Snp->Mode.MediaPresentSupported = TRUE;
+Snp->CableDetectSupported = TRUE;
 break;
 
   case PXE_STATFLAGS_CABLE_DETECT_NOT_SUPPORTED:
   default:
-Snp->Mode.MediaPresentSupported = FALSE;
+Snp->CableDetectSupported = FALSE;
   }
 
   switch (InitStatFlags & PXE_STATFLAGS_GET_STATUS_NO_MEDIA_MASK) { @@ -572,6 
+572,10 @@ SimpleNetworkDriverStart (
 Snp->MediaStatusSupported = FALSE;
   }
 
+  if (Snp->CableDetectSupported || Snp->MediaStatusSupported) {
+Snp->Mode.MediaPresentSupported = TRUE;  }
+
   if ((Pxe->hw.Implementation & PXE_ROMID_IMP_STATION_ADDR_SETTABLE) != 0) {
 Snp->Mode.MacAddressChangeable = TRUE;
   } else {
diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Snp.h 
b/MdeModulePkg/Universal/Network/SnpDxe/Snp.h
index 67f65ff..f802dfb 100644
--- a/MdeModulePkg/Universal/Network/SnpDxe/Snp.h
+++ b/MdeModulePkg

[edk2] [PATCH] MdeModulePkg NvmExpressDxe: Initialize IoAlign info for an NVMe device

[Hao Wu]

The "IoAlign" field in EFI_BLOCK_IO_MEDIA of an NVMe device is not
initialized properly, leading to a zero value for this field.

It should be initialized from the "IoAlign" field in the
EFI_NVM_EXPRESS_PASS_THRU_MODE structure maintained by the NVMe
controller.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c 
b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
index 321dbde..cad061b 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
@@ -136,6 +136,7 @@ EnumerateNvmeDevNamespace (
 Device->Media.LogicalPartition = FALSE;
 Device->Media.ReadOnly   = FALSE;
 Device->Media.WriteCaching   = FALSE;
+Device->Media.IoAlign= Private->PassThruMode.IoAlign;
 
 Flbas = NamespaceData->Flbas;
 LbaFmtIdx = Flbas & 0xF;
-- 
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] MdeModulePkg NvmExpressDxe: Initialize IoAlign info for an NVMe device

[Tian, Feng]

Reviewed-by: Feng Tian 

Thanks
Feng

-Original Message-
From: Wu, Hao A 
Sent: Wednesday, May 4, 2016 4:29 PM
To: edk2-devel@lists.01.org; Tian, Feng 
Cc: Wu, Hao A 
Subject: [PATCH] MdeModulePkg NvmExpressDxe: Initialize IoAlign info for an 
NVMe device

The "IoAlign" field in EFI_BLOCK_IO_MEDIA of an NVMe device is not initialized 
properly, leading to a zero value for this field.

It should be initialized from the "IoAlign" field in the 
EFI_NVM_EXPRESS_PASS_THRU_MODE structure maintained by the NVMe controller.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c 
b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
index 321dbde..cad061b 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
@@ -136,6 +136,7 @@ EnumerateNvmeDevNamespace (
 Device->Media.LogicalPartition = FALSE;
 Device->Media.ReadOnly   = FALSE;
 Device->Media.WriteCaching   = FALSE;
+Device->Media.IoAlign= Private->PassThruMode.IoAlign;
 
 Flbas = NamespaceData->Flbas;
 LbaFmtIdx = Flbas & 0xF;
--
1.9.5.msysgit.0

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

[edk2] [PATCH v2] MdeModulePkg: Refine SNP driver's media status check logic.

[Fu Siyuan]

V2 Update:
Refine variable and function description per Ye, Ting's comments.

Some UNDI drivers may not support the cable detect in UNDI INITIALIZE command,
but support the media present check in UNDI GET_STATUS command. Current SNP
driver will set the MediaPresentSupported field to FALSE in 
EFI_SIMPLE_NETWORK_MODE
for such case, which forbid the media detect from the callers.

This patch updates the SNP driver to support such kind of UNDIs for media 
detect.
MediaPresentSupported will be set to TRUE, and a GET_STATUS command will be 
issued
immediately after INITIALIZE command in SNP->Initialize() function, to refresh
the value of MediaPresent in SNP mode data.

Cc: Ye Ting 
Cc: Wu Jiaxin 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 MdeModulePkg/Universal/Network/SnpDxe/Get_status.c | 25 
 MdeModulePkg/Universal/Network/SnpDxe/Initialize.c | 13 -
 MdeModulePkg/Universal/Network/SnpDxe/Snp.c|  8 +++--
 MdeModulePkg/Universal/Network/SnpDxe/Snp.h| 34 +-
 4 files changed, 64 insertions(+), 16 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Get_status.c 
b/MdeModulePkg/Universal/Network/SnpDxe/Get_status.c
index edbc0f2..4c3bdae 100644
--- a/MdeModulePkg/Universal/Network/SnpDxe/Get_status.c
+++ b/MdeModulePkg/Universal/Network/SnpDxe/Get_status.c
@@ -18,24 +18,25 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 /**
   Call undi to get the status of the interrupts, get the list of recycled 
transmit
   buffers that completed transmitting. The recycled transmit buffer address 
will
-  be saved into Snp->RecycledTxBuf.
+  be saved into Snp->RecycledTxBuf. This function will also update the 
MediaPresent
+  field of EFI_SIMPLE_NETWORK_MODE if UNDI support it.
 
-  @param  Snp Pointer to snp driver structure.
-  @param  InterruptStatusPtr  A non null pointer to contain the interrupt
-  status.
-  @param  GetTransmittedBuf   Set to TRUE to retrieve the recycled transmit
-  buffer address.
+  @param[in]   Snp Pointer to snp driver structure.
+  @param[out]  InterruptStatusPtr  A non null pointer to contain the 
interrupt
+   status.
+  @param[in]   GetTransmittedBuf   Set to TRUE to retrieve the recycled 
transmit
+   buffer address.
 
-  @retval EFI_SUCCESS The status of the network interface was 
retrieved.
-  @retval EFI_DEVICE_ERRORThe command could not be sent to the network
-  interface.
+  @retval  EFI_SUCCESS The status of the network interface was 
retrieved.
+  @retval  EFI_DEVICE_ERRORThe command could not be sent to the 
network
+   interface.
 
 **/
 EFI_STATUS
 PxeGetStatus (
-  SNP_DRIVER *Snp,
-  UINT32 *InterruptStatusPtr,
-  BOOLEANGetTransmittedBuf
+  IN SNP_DRIVER *Snp,
+ OUT UINT32 *InterruptStatusPtr,
+  IN BOOLEANGetTransmittedBuf
   )
 {
   PXE_DB_GET_STATUS *Db;
diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c 
b/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c
index 3f40ef3..2151375 100644
--- a/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c
+++ b/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c
@@ -229,7 +229,10 @@ SnpUndi32Initialize (
   //
   Snp->TxRxBufferSize = (UINT32) (Snp->InitInfo.MemoryRequired + 
ExtraRxBufferSize + ExtraTxBufferSize);
 
-  if (Snp->Mode.MediaPresentSupported) {
+  //
+  // If UNDI support cable detect for INITIALIZE command, try it first.
+  //
+  if (Snp->CableDetectSupported) {
 if (PxeInit (Snp, PXE_OPFLAGS_INITIALIZE_DETECT_CABLE) == EFI_SUCCESS) {
   Snp->Mode.MediaPresent = TRUE;
   goto ON_EXIT;
@@ -242,6 +245,14 @@ SnpUndi32Initialize (
 
   if (EFI_ERROR (EfiStatus)) {
 gBS->CloseEvent (Snp->Snp.WaitForPacket);
+goto ON_EXIT;
+  }
+
+  //
+  // Try to update the MediaPresent field of EFI_SIMPLE_NETWORK_MODE if the 
UNDI support it.
+  //
+  if (Snp->MediaStatusSupported) {
+PxeGetStatus (Snp, NULL, FALSE);
   }
 
 ON_EXIT:
diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Snp.c 
b/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
index 5ff294f..9f61aee 100644
--- a/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
+++ b/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
@@ -554,12 +554,12 @@ SimpleNetworkDriverStart (
 
   switch (InitStatFlags & PXE_STATFLAGS_CABLE_DETECT_MASK) {
   case PXE_STATFLAGS_CABLE_DETECT_SUPPORTED:
-Snp->Mode.MediaPresentSupported = TRUE;
+Snp->CableDetectSupported = TRUE;
 break;
 
   case PXE_STATFLAGS_CABLE_DETECT_NOT_SUPPORTED:
   default:
-Snp->Mode.MediaPresentSupported = FALSE;
+Snp->CableDetectSupported = FALSE;
   }
 
   switch (InitStatFlags & PXE_STATFLAGS_GET_STATUS_NO_MEDIA_MASK) {
@@ -572,6 

[edk2] [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in HttpGenRequestMessage API

[Nagaraj Hegde]

HttpGenRequestMessage assumes that HTTP message would always
contain a request-line, headers and an optional message body.
However, subsequent to a HTTP PUT/POST request, HTTP requests
would contain just the message body. This patch supports
creation of such request messages with additional checks.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hegde, Nagaraj P nagaraj-p.he...@hpe.com
---
 MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 219 +++-
 1 file changed, 124 insertions(+), 95 deletions(-)

diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c 
b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
index 8d61d0b..fa0f591 100644
--- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
+++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
@@ -1681,61 +1681,86 @@ HttpGenRequestMessage (
   HttpUtilitiesProtocol = NULL;
 
   //
-  // Locate the HTTP_UTILITIES protocol.
+  // If we have a Request, we cannot have a NULL Url
   //
-  Status = gBS->LocateProtocol (
-  &gEfiHttpUtilitiesProtocolGuid,
-  NULL,
-  (VOID **)&HttpUtilitiesProtocol
-  );
-
-  if (EFI_ERROR (Status)) {
-DEBUG ((DEBUG_ERROR,"Failed to locate Http Utilities protocol. Status = 
%r.\n", Status));
-return Status;
+  if (Message->Data.Request != NULL && Url == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (Message->HeaderCount != 0) {
+//
+// Locate the HTTP_UTILITIES protocol.
+//
+Status = gBS->LocateProtocol (
+&gEfiHttpUtilitiesProtocolGuid,
+NULL,
+(VOID **)&HttpUtilitiesProtocol
+);
+
+if (EFI_ERROR (Status)) {
+  DEBUG ((DEBUG_ERROR,"Failed to locate Http Utilities protocol. Status = 
%r.\n", Status));
+  return Status;
+}
+
+//
+// Build AppendList to send into HttpUtilitiesBuild
+//
+AppendList = AllocateZeroPool (sizeof (EFI_HTTP_HEADER *) * 
(Message->HeaderCount));
+if (AppendList == NULL) {
+  return EFI_OUT_OF_RESOURCES;
+}
+
+for(Index = 0; Index < Message->HeaderCount; Index++){
+  AppendList[Index] = &Message->Headers[Index];
+}
+
+//
+// Build raw HTTP Headers
+//
+Status = HttpUtilitiesProtocol->Build (
+HttpUtilitiesProtocol,
+0,
+NULL,
+0,
+NULL,
+Message->HeaderCount,
+AppendList,
+&HttpHdrSize,
+&HttpHdr
+);
+
+if (AppendList != NULL) {
+  FreePool (AppendList);
+}
+
+if (EFI_ERROR (Status) || HttpHdr == NULL){
+  return Status;
+}
   }
 
   //
-  // Build AppendList to send into HttpUtilitiesBuild
+  // If we have headers to be sent, account for it.
   //
-  AppendList = AllocateZeroPool (sizeof (EFI_HTTP_HEADER *) * 
(Message->HeaderCount));
-  if (AppendList == NULL) {
-return EFI_OUT_OF_RESOURCES;
-  }
-
-  for(Index = 0; Index < Message->HeaderCount; Index++){
-AppendList[Index] = &Message->Headers[Index];
+  if (Message->HeaderCount != 0) {
+MsgSize = HttpHdrSize;
   }
 
   //
-  // Build raw HTTP Headers
+  // If we have a request line, account for the fields.
   //
-  Status = HttpUtilitiesProtocol->Build (
-  HttpUtilitiesProtocol,
-  0,
-  NULL,
-  0,
-  NULL,
-  Message->HeaderCount,
-  AppendList,
-  &HttpHdrSize,
-  &HttpHdr
-  );
-
-  if (AppendList != NULL) {
-FreePool (AppendList);
-  }
-
-  if (EFI_ERROR (Status) || HttpHdr == NULL){
-return Status;
+  if (Message->Data.Request != NULL) {
+MsgSize += HTTP_METHOD_MAXIMUM_LEN + AsciiStrLen (HTTP_VERSION_CRLF_STR) + 
AsciiStrLen (Url);
   }
 
+
   //
-  // Calculate HTTP message length.
+  // If we have a message body to be sent, account for it.
   //
-  MsgSize = Message->BodyLength + HTTP_METHOD_MAXIMUM_LEN + AsciiStrLen (Url) +
-AsciiStrLen (HTTP_VERSION_CRLF_STR) + HttpHdrSize;
-
+  MsgSize += Message->BodyLength;
 
+  //
+  // memory for the string that needs to be sent to TCP
+  //
   *RequestMsg = AllocateZeroPool (MsgSize);
   if (*RequestMsg == NULL) {
 Status = EFI_OUT_OF_RESOURCES;
@@ -1746,60 +1771,64 @@ HttpGenRequestMessage (
   //
   // Construct header request
   //
-  switch (Message->Data.Request->Method) {
-  case HttpMethodGet:
-StrLength = sizeof (HTTP_METHOD_GET) - 1;
-CopyMem (RequestPtr, HTTP_METHOD_GET, StrLength);
-RequestPtr += StrLength;
-break;
-  case HttpMethodPut:
-StrLength = sizeof (HTTP_METHOD_PUT) - 1;
-CopyMem (RequestPtr, HTTP_METHOD_PUT, StrLength);
-RequestPtr += StrLength;
-break;
-  case HttpMethodPatch:
-StrLength = sizeof (HTTP_METHOD_PATCH) - 1;
-CopyMem (RequestPtr, HTTP_METHOD_PATCH, StrLength);
-RequestPtr += StrLength;
-break;
-  case HttpMethodP

[edk2] [PATCH] NetworkPkg: Discard TCP segment when no TCB found

[Michael Chang]

The TCP driver should discard(ignore) the incoming TCP segment when there's no
TCB found to handle it, because that TCP segment may be subjected to be
received by other running Managed Network instance with it's own network stack
to handle it. Force sending reset segment in this case would just discrupt any
TCP connection except the one established by EFI TCP protocol, which is not
make sense imho.

Thanks.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Chang 

---
 NetworkPkg/TcpDxe/TcpInput.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/TcpDxe/TcpInput.c b/NetworkPkg/TcpDxe/TcpInput.c
index 745ee4c..7aa6f7e 100644
--- a/NetworkPkg/TcpDxe/TcpInput.c
+++ b/NetworkPkg/TcpDxe/TcpInput.c
@@ -794,10 +794,10 @@ TcpInput (
   );
 
   if ((Tcb == NULL) || (Tcb->State == TCP_CLOSED)) {
-DEBUG ((EFI_D_INFO, "TcpInput: send reset because no TCB found\n"));
+DEBUG ((EFI_D_INFO, "TcpInput: discard a segment because no TCB found\n"));
 
 Tcb = NULL;
-goto SEND_RESET;
+goto DISCARD;
   }
 
   Seg = TcpFormatNetbuf (Tcb, Nbuf);
-- 
2.6.6

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

[edk2] [PATCH v1 1/1] NetworkPkg:HttpDxe: Code changes to support HTTP PUT/POST operations

[Nagaraj Hegde]

Code changes enables HttpDxe to handle PUT/POST operations.
EfiHttpRequest assumes "Request" and "HttpMsg->Headers" can
never be NULL. Also, HttpResponseWorker assumes HTTP Reponse
will contain headers. We could have response which could contain
only a string (HTTP 100 Continue) and no headers. Code changes
tries to do-away from these assumptions, which would enable
HttpDxe to support PUT/POST operations.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hegde, Nagaraj P nagaraj-p.he...@hpe.com
---
 NetworkPkg/HttpDxe/HttpDriver.c |   3 +
 NetworkPkg/HttpDxe/HttpImpl.c   | 399 +++-
 NetworkPkg/HttpDxe/HttpProto.h  |   1 +
 3 files changed, 226 insertions(+), 177 deletions(-)

diff --git a/NetworkPkg/HttpDxe/HttpDriver.c b/NetworkPkg/HttpDxe/HttpDriver.c
index 2518f4e..0bde012 100644
--- a/NetworkPkg/HttpDxe/HttpDriver.c
+++ b/NetworkPkg/HttpDxe/HttpDriver.c
@@ -2,6 +2,7 @@
   The driver binding and service binding protocol for HttpDxe driver.
 
   Copyright (c) 2015, 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
@@ -939,6 +940,8 @@ HttpServiceBindingCreateChild (
   
   HttpInstance->Signature = HTTP_PROTOCOL_SIGNATURE;
   HttpInstance->Service   = HttpService;
+  HttpInstance->Method = HttpMethodMax;
+
   CopyMem (&HttpInstance->Http, &mEfiHttpTemplate, sizeof 
(HttpInstance->Http));
   NetMapInit (&HttpInstance->TxTokens);
   NetMapInit (&HttpInstance->RxTokens);
diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index 7a236f4..9360355 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -248,151 +248,178 @@ EfiHttpRequest (
   HTTP_TOKEN_WRAP   *Wrap;
   CHAR8 *FileUrl;
   UINTN RequestMsgSize;
-  
+
+  Url = NULL;
+  HostName = NULL;
+  RequestMsg = NULL;
+  HostNameStr = NULL;
+  Wrap = NULL;
+  FileUrl = NULL;
   if ((This == NULL) || (Token == NULL)) {
 return EFI_INVALID_PARAMETER;
   }
 
   HttpMsg = Token->Message;
-  if ((HttpMsg == NULL) || (HttpMsg->Headers == NULL)) {
+  if (HttpMsg == NULL) {
 return EFI_INVALID_PARAMETER;
   }
 
-  //
-  // Current implementation does not support POST/PUT method.
-  // If future version supports these two methods, Request could be NULL for a 
special case that to send large amounts
-  // of data. For this case, the implementation need check whether previous 
call to Request() has been completed or not.
-  // 
-  //
   Request = HttpMsg->Data.Request;
-  if ((Request == NULL) || (Request->Url == NULL)) {
-return EFI_INVALID_PARAMETER;
-  }
 
   //
-  // Only support GET and HEAD method in current implementation.
+  // Only support GET, HEAD, PUT and POST method in current implementation.
   //
-  if ((Request->Method != HttpMethodGet) && (Request->Method != 
HttpMethodHead)) {
+  if ((Request != NULL) && (Request->Method != HttpMethodGet) &&
+  (Request->Method != HttpMethodHead) && (Request->Method != 
HttpMethodPut) && (Request->Method != HttpMethodPost)) {
 return EFI_UNSUPPORTED;
   }
 
   HttpInstance = HTTP_INSTANCE_FROM_PROTOCOL (This);
   ASSERT (HttpInstance != NULL);
 
+  if (Request != NULL) {
+HttpInstance->Method = Request->Method;
+  }
+
   if (HttpInstance->State < HTTP_STATE_HTTP_CONFIGED) {
 return EFI_NOT_STARTED;
   }
 
-  //
-  // Check whether the token already existed.
-  //
-  if (EFI_ERROR (NetMapIterate (&HttpInstance->TxTokens, HttpTokenExist, 
Token))) {
-return EFI_ACCESS_DENIED;   
-  }  
+  if (Request == NULL) {
+//
+// Request would be NULL only for PUT/POST operation (in the current 
implementation)
+//
+if ((HttpInstance->Method != HttpMethodPut) && (HttpInstance->Method != 
HttpMethodPost)) {
+  return EFI_INVALID_PARAMETER;
+}
 
-  HostName= NULL;
-  Wrap= NULL;
-  HostNameStr = NULL;
+//
+// For PUT/POST, we need to have the TCP already configured. Bail out if 
it is not!
+//
+if (HttpInstance->State < HTTP_STATE_TCP_CONFIGED) {
+  return EFI_INVALID_PARAMETER;
+}
 
-  //
-  // Parse the URI of the remote host.
-  //
-  Url = HttpInstance->Url;
-  UrlLen = StrLen (Request->Url) + 1;
-  if (UrlLen > HTTP_URL_BUFFER_LEN) {
-Url = AllocateZeroPool (UrlLen);
-if (Url == NULL) {
-  return EFI_OUT_OF_RESOURCES;
+//
+// We need to have the Message Body for sending the HTTP message across in 
these cases.
+//
+if (HttpMsg->Body == NULL || HttpMsg->BodyLength == 0) {
+  return EFI_INVALID_PARAMETER;
 }
-FreePool (HttpInstance->Url);
-HttpInstance->Url = Url;
-  } 
 
+//
+// Use existing TCP instance to transmit the packet.
+//
+Configure   = FALSE;
+ReConfigure = FALSE;
+  } else {
+//
+// Check whether the token already existed

Re: [edk2] [PATCH] MdeModulePkg: ScsiDiskDxe: cope with broken "Supported VPD Pages" VPD page

[Laszlo Ersek]

On 05/04/16 02:58, Tian, Feng wrote:
> Laszlo, 
> 
> Could you explain more? 
> 
> Usb Flash drive is managed by UsbMassStorage driver, which is irrelevant with 
> UefiScsiLib lib or ScsiDisk driver.

Yeah, sorry about missing that part of the context.

So, the environment is the following:

- The USB flash drive with VendorId/ProductId 0x1516/0x6221 is plugged
into a Linux host.

- The usb-storage driver in Linux presents the USB flash drive to the
rest of the system as a SCSI disk. The device node that is generated for
it looks like /dev/sd*, for example, /dev/sdi.

- This device node can be used like any other SCSI device.

- Using the virtio-scsi HBA, QEMU can provide a virtual machine with
various types of virtual SCSI devices. One of those is "scsi-block",
which is also known as "SCSI passthrough". It means that most of the
SCSI commands are forwarded transparently from guest device to physical
host device (/dev/sdi, for example), using the SG_IO ioctl() call. Only
a few SCSI commands are captured and emulated by QEMU.

This feature is useful e.g. for burning physical optical media, or
writing physical tapes, from a virtual machine.

- In this scenario, the USB stick that is plugged in the host is exposed
to the guest with this feature. So, when the ScsiDiskInquiryDevice()
function runs in the guest (as part of OVMF), the INQUIRY command
(asking for the Supported VPD Pages" VPD page) is forwarded to the USB
stick on the host. And that device returns garbage.

- It is important to note that the "garbage" is not inserted by either
the host kernel's usb-storage driver, or by QEMU. Even without
virtualization, the host kernel consciously avoids asking SCSI disks
that are actually backed by USB mass-storage devices for VPD pages,
because the kernel developers have realized that most (if not all) of
USB mass-storage devices fail to respond correctly. Please see this host
kernel commit:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=09b6b51b0b6c

- In , I
demonstrate what happens when such an INQUIRY is sent manually to the
SCSI disk that is backed by the USB flash drive. For a genuine SCSI
disk, the command completes:

# sg_vpd -v /dev/sda
Supported VPD pages VPD page:
inquiry cdb: 12 01 00 00 fc 00
   [PQual=0  Peripheral device type: disk]
  Supported VPD pages [sv]
  Unit serial number [sn]
  Device identification [di]
  ATA information (SAT) [ai]
  Block limits (SBC) [bl]
  Block device characteristics (SBC) [bdc]
  Logical block provisioning (SBC) [lbpv]

Buth the USB flash drive responds with garbage:

# sg_vpd -v /dev/sdi
Supported VPD pages VPD page:
inquiry cdb: 12 01 00 00 fc 00
invalid VPD response; probably a STANDARD INQUIRY response
First 32 bytes of bad response
 00   00 80 06 02 1f 00 00 00  00 00 00 00 00 00 00 00  
 10   00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  
fetching VPD page failed

- So this patch is motivated by the case when a USB mass-storage device
is presented as a SCSI disk, directly on the (virtual) hardware level,
and it provides a bogus 0x00 VPD page.

Thanks
Laszlo

> Thanks
> Feng
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
> Ersek
> Sent: Wednesday, May 4, 2016 12:30 AM
> To: edk2-devel-01 
> Cc: Ni, Ruiyu ; Paolo Bonzini ; 
> Tian, Feng ; Zeng, Star 
> Subject: [edk2] [PATCH] MdeModulePkg: ScsiDiskDxe: cope with broken 
> "Supported VPD Pages" VPD page
> 
> The USB flash drive with Vendor ID 0x1516 (CompUSA) and Product ID 0x6221 
> returns a broken "Supported VPD Pages" VPD page. In particular, the 
> PageLength field has the invalid value 0x0602 (decimal 1538).
> 
> This prevents the loop from terminating that scans for the Block Limits VPD 
> page code in ScsiDiskInquiryDevice():
> 
> for (Index = 0; Index < PageLength; Index++) {
> 
> because the Index variable has type UINT8, and it wraps from 255 to 0, 
> without ever reaching PageLength (1538), and because 
> EFI_SCSI_PAGE_CODE_BLOCK_LIMITS_VPD does not occur at offsets 0 through 255.
> 
> * The fix is not to change the type of Index to UINT16 or a wider type.
>   Namely, section
> 
> 7.8.14 Supported VPD Pages VPD page
> 
>   in the "SCSI Primary Commands - 4" (SPC-4) specification names the
>   following requirement:
> 
> The supported VPD page list shall contain a list of all VPD page codes
> (see 7.8) implemented by the logical unit in ascending order beginning
> with page code 00h.
> 
>   Since page codes are 8-bit unsigned quantities, it follows that the
>   maximum size for the Supported VPD Pages VPD page is 0x100 bytes, in
>   which every possible page code (0x00 through 0xFF) will be found, before
>   the UINT8 offset wraps around.
> 
>   (EFI_SCSI_SUPPORTED_VPD_PAGES_VPD_PAGE.SupportedVpdPageList is correctly
>   sized as well, in "MdePkg/Include/IndustryStandard/Scsi.h".)
> 
> *

Re: [edk2] Csm16.bin(seabios) failed to work in OVMF.

[Laszlo Ersek]

On 05/04/16 06:44, Ni, Ruiyu wrote:
> I encountered the same issue weeks ago.
> A quick but dirty fix is to change:
> UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c: CpuS3DataInitialize()
>   //
>   // Allocate a 4KB reserved page below 1MB
>   //
> -  AcpiCpuData->StartupVector = BASE_1MB - 1;
> +  ApiCpuData->StartupVector = BASE_512KB - 1;

Hm, I don't think so.

OVMF includes CpuS3DataDxe with SMM_REQUIRE=TRUE only. My understanding
is that the reporter encountered the issue with CSM_ENABLE=TRUE,
SMM_REQUIRE=FALSE.

I think OVMF should be recompiled with DEBUG_POOL and DEBUG_PAGE set in
PcdDebugPrintErrorLevel. Then the OVMF log might expose the driver that
allocates memory under 640 KB (so that LegacyBiosDxe cannot allocate
32KB (i.e., PcdEbdaReservedMemorySize) at 608 KB later).

Thanks
Laszlo

> Copying Jeff.
> 
> Regards,
> Ray
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>> Laszlo Ersek
>> Sent: Monday, April 25, 2016 7:22 PM
>> To: wq...@aliyun.com
>> Cc: edk2-devel ; Kevin O'Connor 
>> ; David Woodhouse
>> 
>> Subject: Re: [edk2] Csm16.bin(seabios) failed to work in OVMF.
>>
>> On 04/23/16 08:39, wq...@aliyun.com wrote:
>>> Hi everyone,
>>> I build the Csm16.bin from source code of seabios-1.9.2.tar.gz . And 
>>> copy the Csm16.bin to
>> OvmfPkg/Csm/Csm16/Csm16.bin , Build the OVMF_CODE.fd and OVMF_VARS.fd with 
>> command build -D CSM_ENABLE.
>>>Unfortunately, I test the OVMF_CODE.fd and OVMF_VARS.fd in qemu. It 
>>> failed!
>>> The log is:
>>> ASSERT_EFI_ERROR (Status = Not Found)
>>> ASSERT 
>>> /root/tianocore-edk2/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c(982):
>>>  !EFI_ERROR
>> (Status)
>>> Any information you can provide me would be greatly appreciated.
>>
>> Sorry, no idea. The assert is about a memory allocation failure (you can
>> check the source code at the location captured in the assert):
>>
>>  Status = AllocateLegacyMemory (
>> AllocateAddress,
>> CONVENTIONAL_MEMORY_TOP - MemorySize,
>> EFI_SIZE_TO_PAGES (MemorySize),
>> &MemoryAddress
>> );
>>  ASSERT_EFI_ERROR (Status);
>>
>> I don't know why it fails.
>>
>> Perhaps you can experiment with older SeaBIOS releases and narrow it
>> down a little. (You could also experiment with OVMF builds at different
>> git commits, but I don't readily recall anything that could cause this.)
>>
>> I'm adding David, Kevin and Gerd to the CC list.
>>
>> Thanks
>> Laszlo
>>
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel

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

Re: [edk2] Csm16.bin(seabios) failed to work in OVMF.

[David Woodhouse]

On Wed, 2016-05-04 at 13:27 +0200, Laszlo Ersek wrote:
> 
> Hm, I don't think so.
> 
> OVMF includes CpuS3DataDxe with SMM_REQUIRE=TRUE only. My understanding
> is that the reporter encountered the issue with CSM_ENABLE=TRUE,
> SMM_REQUIRE=FALSE.



Of course, now we have SMM, perhaps we should revisit the model where
the SeaBIOS CSM just takes over the hardware completely (and cannot
return to UEFI).

I understand that the "traditional" model is that the CSM does
basically nothing for itself, and all its drivers (like the INT 13h
services) actually just thunk through SMM to the UEFI runtime?

-- 
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com  Intel Corporation



smime.p7s
Description: S/MIME cryptographic signature
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Csm16.bin(seabios) failed to work in OVMF.

[Laszlo Ersek]

On 05/04/16 13:32, David Woodhouse wrote:
> On Wed, 2016-05-04 at 13:27 +0200, Laszlo Ersek wrote:
>>
>> Hm, I don't think so.
>>
>> OVMF includes CpuS3DataDxe with SMM_REQUIRE=TRUE only. My understanding
>> is that the reporter encountered the issue with CSM_ENABLE=TRUE,
>> SMM_REQUIRE=FALSE.
> 
> 
> 
> Of course, now we have SMM, perhaps we should revisit the model where
> the SeaBIOS CSM just takes over the hardware completely (and cannot
> return to UEFI).
> 
> I understand that the "traditional" model is that the CSM does
> basically nothing for itself, and all its drivers (like the INT 13h
> services) actually just thunk through SMM to the UEFI runtime?

What benefit would it provide if SeaBIOS used SMIs to call into UEFI /
if SeaBIOS could return to UEFI?

Thanks
Laszlo

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

Re: [edk2] Csm16.bin(seabios) failed to work in OVMF.

[Laszlo Ersek]

On 04/25/16 13:21, Laszlo Ersek wrote:
> On 04/23/16 08:39, wq...@aliyun.com wrote:
>> Hi everyone,
>> I build the Csm16.bin from source code of seabios-1.9.2.tar.gz . And 
>> copy the Csm16.bin to OvmfPkg/Csm/Csm16/Csm16.bin , Build the OVMF_CODE.fd 
>> and OVMF_VARS.fd with command build -D CSM_ENABLE.
>>Unfortunately, I test the OVMF_CODE.fd and OVMF_VARS.fd in qemu. It 
>> failed!
>> The log is:
>> ASSERT_EFI_ERROR (Status = Not Found)
>> ASSERT 
>> /root/tianocore-edk2/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c(982):
>>  !EFI_ERROR (Status)
>> Any information you can provide me would be greatly appreciated.
> 
> Sorry, no idea. The assert is about a memory allocation failure (you can
> check the source code at the location captured in the assert):
> 
>   Status = AllocateLegacyMemory (
>  AllocateAddress,
>  CONVENTIONAL_MEMORY_TOP - MemorySize,
>  EFI_SIZE_TO_PAGES (MemorySize),
>  &MemoryAddress
>  );
>   ASSERT_EFI_ERROR (Status);
> 
> I don't know why it fails.
> 
> Perhaps you can experiment with older SeaBIOS releases and narrow it
> down a little. (You could also experiment with OVMF builds at different
> git commits, but I don't readily recall anything that could cause this.)
> 
> I'm adding David, Kevin and Gerd to the CC list.

I tried this. I didn't hit this specific assert, but I hit another one:

ASSERT IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c(311):
Private->BiosStart > Private->OptionRom

The problem was that SeaBIOS had grown to 256KB in size:

https://code.coreboot.org/p/seabios/source/commit/85f8fac875263

which was too big for LegacyBiosDxe.

Manually lowering CONFIG_ROM_SIZE to 128 broke the SeaBIOS CSM build for
me. However, setting the same to 192 resulted in a succesful SeaBIOS CSM
build, and a working OVMF startup as well. (I didn't try to boot an OS,
just the UEFI shell.)

The SeaBIOS commit I built at is: c8e105a4d5e52e8e7539ab1f2cd07ebe0ae9033a.

Commands:

cat >.config <<-EOT
CONFIG_CSM=y
CONFIG_QEMU_HARDWARE=y
CONFIG_PERMIT_UNALIGNED_PCIROM=y
CONFIG_DEBUG_LEVEL=20
CONFIG_ROM_SIZE=192
EOT
yes "" | make oldconfig
make -j2 out/bios.bin


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

Re: [edk2] [Patch v5 00/24] Use MdeModulePkg/BDS in OVMF platform

[Laszlo Ersek]

On 05/03/16 07:35, Ruiyu Ni wrote:
> The patch serials creates a flag USE_OLD_BDS and by default the value
> of the flag is FALSE so that the new MdeModulePkg/BDS is used.
> User can define USE_OLD_BDS as TRUE to force to use IntelFrameworkModulePkg
> /BDS.
> 
> https://github.com/niruiyu/edk2/tree/Ovmf_Bds
> 
> 
> Ruiyu Ni (24):
>   MdeModulePkg/UefiBootManagerLib: Expose *GetLoadOptionBuffer() API
>   OvmfPkg/PlatformPei: Add memory above 4GB as tested
>   OvmfPkg: Duplicate QemuBootOrderLib to QemuNewBootOrderLib
>   OvmfPkg/QemuNewBootOrderLib: Build with UefiBootManagerLib
>   OvmfPkg: Duplicate PlatformBdsLib to PlatformBootManagerLib
>   OvmfPkg/PlatformBootManagerLib: Follow PlatformBootManagerLib
> interfaces
>   OvmfPkg/PlatformBootManagerLib: use
> EfiBootManagerUpdateConsoleVariable
>   OvmfPkg/PlatformBootManagerLib: link to UefiBootManagerLib
>   OvmfPkg/PlatformBootManagerLib: Use ConvertDevicePathToText()
>   OvmfPkg/PlatformBootManagerLib: Init console vars in *BeforeConsole()
>   OvmfPkg/PlatformBootManagerLib: Do not launch Boot Manager Menu
>   OvmfPkg/PlatformBootManagerLib: Register boot options and hot keys
>   OvmfPkg/PlatformBootManagerLib: Remove unused local functions.
>   OvmfPkg/PlatformBootManagerLib: port PlatformBdsConnectSequence to
> UefiBootManagerLib
>   OvmfPkg/PlatformBootManagerLib: Use
> EfiBootManagerRefreshAllBootOption()
>   OvmfPkg/PlatformBootManagerLib: Remove PlatformBdsGetDriverOption()
>   OvmfPkg/PlatformBootManagerLib: Use GetBootModeHob() in HobLib
>   OvmfPkg/PlatformBootManagerLib: Remove unnecessary memory test
>   OvmfPkg/PlatformBootManagerLib: Remove unused vars and func prototypes
>   OvmfPkg/PlatformBootManagerLib: Add EnableQuietBoot & DisableQuietBoot
>   OvmfPkg/PlatformBootManagerLib: Fix gcc-4.8 Ia32 build failure
>   OvmfPkg/PlatformBootManagerLib: Remove unused C structures definitions
>   OvmfPkg: Use MdeModulePkg/BDS
>   OvmfPkg/OvmfPkgIa32X64.dsc: Move PcdShellFile to
> [PcdsFixedAtBuild.X64]
> 
>  MdeModulePkg/Include/Library/UefiBootManagerLib.h  |   23 +-
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c   |   11 +-
>  .../Library/UefiBootManagerLib/BmLoadOption.c  |2 +-
>  .../Library/UefiBootManagerLib/InternalBm.h|   19 -
>  .../Library/PlatformBootManagerLib/BdsPlatform.c   | 1423 ++
>  .../Library/PlatformBootManagerLib/BdsPlatform.h   |  214 +++
>  .../PlatformBootManagerLib.inf |   83 +
>  .../Library/PlatformBootManagerLib/PlatformData.c  |   41 +
>  .../Library/PlatformBootManagerLib/QemuKernel.c|  170 ++
>  OvmfPkg/Library/PlatformBootManagerLib/QuietBoot.c |  673 +++
>  .../Library/QemuNewBootOrderLib/ExtraRootBusMap.c  |  313 
>  .../Library/QemuNewBootOrderLib/ExtraRootBusMap.h  |   40 +
>  .../Library/QemuNewBootOrderLib/QemuBootOrderLib.c | 1953 
> 
>  .../QemuNewBootOrderLib/QemuBootOrderLib.inf   |   68 +
>  OvmfPkg/OvmfPkgIa32.dsc|   41 +-
>  OvmfPkg/OvmfPkgIa32.fdf|5 +
>  OvmfPkg/OvmfPkgIa32X64.dsc |   43 +-
>  OvmfPkg/OvmfPkgIa32X64.fdf |5 +
>  OvmfPkg/OvmfPkgX64.dsc |   41 +-
>  OvmfPkg/OvmfPkgX64.fdf |5 +
>  OvmfPkg/PlatformPei/MemDetect.c|4 +-
>  OvmfPkg/PlatformPei/Platform.c |   29 -
>  OvmfPkg/PlatformPei/Platform.h |   14 +-
>  OvmfPkg/PlatformPei/Xen.c  |8 +-
>  24 files changed, 5139 insertions(+), 89 deletions(-)
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h
>  create mode 100644 
> OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/QuietBoot.c
>  create mode 100644 OvmfPkg/Library/QemuNewBootOrderLib/ExtraRootBusMap.c
>  create mode 100644 OvmfPkg/Library/QemuNewBootOrderLib/ExtraRootBusMap.h
>  create mode 100644 OvmfPkg/Library/QemuNewBootOrderLib/QemuBootOrderLib.c
>  create mode 100644 OvmfPkg/Library/QemuNewBootOrderLib/QemuBootOrderLib.inf
> 

Thanks again for this, Ray. I filed
 for ArmVirtPkg, and
assigned it to myself.

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

Re: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei to FspmWrapperPeim and FspsWrapperPeim.

[Tim Lewis]

Thank you.

Tim

Sent from my Windows 10 phone

From: Mudusuru, Giri P
Sent: Tuesday, May 3, 2016 11:06 PM
To: Tim Lewis; Yao, 
Jiewen; 
edk2-devel@lists.01.org
Cc: Rangarajan, Ravi P; Yarlagadda, Satya 
P; Zimmer, 
Vincent; Mudusuru, Giri 
P
Subject: RE: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei 
to FspmWrapperPeim and FspsWrapperPeim.

Thanks for info on the policy and usage. Let me follow up internally and get 
back to you.

Thanks,
-Giri
From: Tim Lewis [mailto:tim.le...@insyde.com]
Sent: Tuesday, May 3, 2016 4:24 PM
To: Mudusuru, Giri P ; Yao, Jiewen 
; edk2-devel@lists.01.org
Cc: Rangarajan, Ravi P ; Yarlagadda, Satya P 
; Zimmer, Vincent 
Subject: RE: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei 
to FspmWrapperPeim and FspsWrapperPeim.


1.  Because we, per our custom tools and per our company policy, require 
that all source code exist underneath $(WORKSPACE).  As previously discussed on 
this list, we think multiple workspaces makes our projects harder to maintain 
and harder to scan. Multiple workspaces was meant to be an OPTIONAL feature 
and, while some may use it, making it a requirement that all codebases follow 
this path to support what is today already supported seems a bit steep.

2.  There is NO PRODUCTION silicon that uses FSP 2.0 at this time that I am 
aware of. That means Intel is upgrading the Wrapper package to support silicon 
that is not yet shipping and removing support for the (by my count) 5 publicly 
announced chipsets, and probably a few others that are in alpha or beta.

3.  Many of these chipsets have a long life, which means new projects start 
on them well after initial product launch. Because of features and security 
concerns, we continually upgrade our chipsets to use newer kernel code. This 
puts an extra variable.

Regards,

Tim Lewis
CTO, Insyde Software
www.insyde.com

From: Mudusuru, Giri P [mailto:giri.p.mudus...@intel.com]
Sent: Tuesday, May 03, 2016 4:07 PM
To: Tim Lewis mailto:tim.le...@insyde.com>>; Yao, Jiewen 
mailto:jiewen@intel.com>>; 
edk2-devel@lists.01.org
Cc: Rangarajan, Ravi P 
mailto:ravi.p.rangara...@intel.com>>; Yarlagadda, 
Satya P mailto:satya.p.yarlaga...@intel.com>>; 
Zimmer, Vincent mailto:vincent.zim...@intel.com>>; 
Mudusuru, Giri P mailto:giri.p.mudus...@intel.com>>
Subject: RE: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei 
to FspmWrapperPeim and FspsWrapperPeim.

Hi Tim,
Yes we considered creating a separate package name along with moving the 
package under Deprecated folder and just leave it in the UDK2015 branch.

There are pro’s and con’s for all options.

Can you please help me understand on why multi-workspace is not an option?

Thanks,
-Giri

From: Tim Lewis [mailto:tim.le...@insyde.com]
Sent: Tuesday, May 03, 2016 8:47 AM
To: Mudusuru, Giri P 
mailto:giri.p.mudus...@intel.com>>; Yao, Jiewen 
mailto:jiewen@intel.com>>; 
edk2-devel@lists.01.org
Cc: Rangarajan, Ravi P 
mailto:ravi.p.rangara...@intel.com>>; Yarlagadda, 
Satya P mailto:satya.p.yarlaga...@intel.com>>; 
Zimmer, Vincent mailto:vincent.zim...@intel.com>>; 
Mudusuru, Giri P mailto:giri.p.mudus...@intel.com>>
Subject: RE: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei 
to FspmWrapperPeim and FspsWrapperPeim.


Giri ,

I suggest you create a new package name, rather than use the same one with 
different semantics.



Multiple workspaces is not ah option for us.



Tim



Sent from my Windows 10 phone



From: Mudusuru, Giri P
Sent: Tuesday, May 3, 2016 8:07 AM
To: Tim Lewis; Yao, 
Jiewen; 
edk2-devel@lists.01.org
Cc: Rangarajan, Ravi P; Yarlagadda, Satya 
P; Zimmer, 
Vincent; Mudusuru, Giri 
P
Subject: RE: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei 
to FspmWrapperPeim and FspsWrapperPeim.


Hi Tim,
The main reason to deprecate the v1.x support was to reduce the complexity and 
focus on v2.0 going forward. At the same time as Jiewen mentioned below our 
plan is to continue maintaining v1.1 in the UDK2015 branch for the existing 
chipsets.

Understand your usage with single tree. For this usage, suggest to use the 
multi-workspace capability so you can have both in a single tree.

Thanks,
-Giri

-Original Message-
From: Tim Lewis [mailto:tim.le...@insyde.com]
Sent: Tuesday, May 3, 2016 4:48 AM
To: Yao, Jiewen mailto:jiewen@intel.com>>; 

[edk2] [PATCH] BaseTools AARCH64: set compiler flag -fno-pic explicitly

[Vishal Bhoj]

Android toolchain has fpic enabled by default so
explicitly disabling it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Vishal Bhoj 
---
 BaseTools/Conf/tools_def.template | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 2065fa3..d03a8bf 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -4324,7 +4324,7 @@ DEFINE GCC_IA32_CC_FLAGS   = 
DEF(GCC_ALL_CC_FLAGS) -m32 -malign-double -
 DEFINE GCC_X64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mno-red-zone 
-Wno-address -mno-stack-arg-probe
 DEFINE GCC_IPF_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) 
-minline-int-divide-min-latency
 DEFINE GCC_ARM_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-mabi=aapcs -fno-short-enums -save-temps -funsigned-char -ffunction-sections 
-fdata-sections -fomit-frame-pointer -Wno-address -mthumb -mfloat-abi=soft
-DEFINE GCC_AARCH64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-fno-short-enums -save-temps -fverbose-asm -funsigned-char  -ffunction-sections 
-fdata-sections -fomit-frame-pointer -fno-builtin -Wno-address 
-fno-asynchronous-unwind-tables
+DEFINE GCC_AARCH64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-fno-short-enums -save-temps -fverbose-asm -funsigned-char  -ffunction-sections 
-fdata-sections -fomit-frame-pointer -fno-builtin -Wno-address 
-fno-asynchronous-unwind-tables -fno-pic
 DEFINE GCC_AARCH64_CC_XIPFLAGS = -mstrict-align
 DEFINE GCC_DLINK_FLAGS_COMMON  = -nostdlib --pie
 DEFINE GCC_DLINK2_FLAGS_COMMON = 
--script=$(EDK_TOOLS_PATH)/Scripts/GccBase.lds
-- 
1.9.1

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

Re: [edk2] [PATCH] MdeModulePkg: ScsiDiskDxe: cope with broken "Supported VPD Pages" VPD page

[Laszlo Ersek]

On 05/04/16 13:11, Laszlo Ersek wrote:
> On 05/04/16 02:58, Tian, Feng wrote:
>> Laszlo, 
>>
>> Could you explain more? 
>>
>> Usb Flash drive is managed by UsbMassStorage driver, which is irrelevant 
>> with UefiScsiLib lib or ScsiDisk driver.
> 
> Yeah, sorry about missing that part of the context.
> 
> So, the environment is the following:
> 
> - The USB flash drive with VendorId/ProductId 0x1516/0x6221 is plugged
> into a Linux host.
> 
> - The usb-storage driver in Linux presents the USB flash drive to the
> rest of the system as a SCSI disk. The device node that is generated for
> it looks like /dev/sd*, for example, /dev/sdi.
> 
> - This device node can be used like any other SCSI device.
> 
> - Using the virtio-scsi HBA, QEMU can provide a virtual machine with
> various types of virtual SCSI devices. One of those is "scsi-block",
> which is also known as "SCSI passthrough". It means that most of the
> SCSI commands are forwarded transparently from guest device to physical
> host device (/dev/sdi, for example), using the SG_IO ioctl() call. Only
> a few SCSI commands are captured and emulated by QEMU.
> 
> This feature is useful e.g. for burning physical optical media, or
> writing physical tapes, from a virtual machine.
> 
> - In this scenario, the USB stick that is plugged in the host is exposed
> to the guest with this feature. So, when the ScsiDiskInquiryDevice()
> function runs in the guest (as part of OVMF), the INQUIRY command
> (asking for the Supported VPD Pages" VPD page) is forwarded to the USB
> stick on the host. And that device returns garbage.
> 
> - It is important to note that the "garbage" is not inserted by either
> the host kernel's usb-storage driver, or by QEMU. Even without
> virtualization, the host kernel consciously avoids asking SCSI disks
> that are actually backed by USB mass-storage devices for VPD pages,
> because the kernel developers have realized that most (if not all) of
> USB mass-storage devices fail to respond correctly. Please see this host
> kernel commit:
> 
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=09b6b51b0b6c
> 
> - In , I
> demonstrate what happens when such an INQUIRY is sent manually to the
> SCSI disk that is backed by the USB flash drive. For a genuine SCSI
> disk, the command completes:
> 
> # sg_vpd -v /dev/sda
> Supported VPD pages VPD page:
> inquiry cdb: 12 01 00 00 fc 00
>[PQual=0  Peripheral device type: disk]
>   Supported VPD pages [sv]
>   Unit serial number [sn]
>   Device identification [di]
>   ATA information (SAT) [ai]
>   Block limits (SBC) [bl]
>   Block device characteristics (SBC) [bdc]
>   Logical block provisioning (SBC) [lbpv]
> 
> Buth the USB flash drive responds with garbage:
> 
> # sg_vpd -v /dev/sdi
> Supported VPD pages VPD page:
> inquiry cdb: 12 01 00 00 fc 00
> invalid VPD response; probably a STANDARD INQUIRY response
> First 32 bytes of bad response
>  00   00 80 06 02 1f 00 00 00  00 00 00 00 00 00 00 00  
>  10   00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  
> fetching VPD page failed
> 
> - So this patch is motivated by the case when a USB mass-storage device
> is presented as a SCSI disk, directly on the (virtual) hardware level,
> and it provides a bogus 0x00 VPD page.

I'm no longer certain that edk2's ScsiDiskDxe is the right place to fix
this problem. It seems that the usb-storage driver in Linux could be the
culprit -- it should be watching for INQUIRY commands with EVPD set, and
reject them before forwarding them to the device. That is the method
that complies with both SPC-4 and the UFI command spec. (And, edk2
handles this case correctly, already.) Please see
.

So, for the time being, I'm withdrawing this patch.

Thanks!
Laszlo

> Thanks
> Laszlo
> 
>> Thanks
>> Feng
>>
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>> Laszlo Ersek
>> Sent: Wednesday, May 4, 2016 12:30 AM
>> To: edk2-devel-01 
>> Cc: Ni, Ruiyu ; Paolo Bonzini ; 
>> Tian, Feng ; Zeng, Star 
>> Subject: [edk2] [PATCH] MdeModulePkg: ScsiDiskDxe: cope with broken 
>> "Supported VPD Pages" VPD page
>>
>> The USB flash drive with Vendor ID 0x1516 (CompUSA) and Product ID 0x6221 
>> returns a broken "Supported VPD Pages" VPD page. In particular, the 
>> PageLength field has the invalid value 0x0602 (decimal 1538).
>>
>> This prevents the loop from terminating that scans for the Block Limits VPD 
>> page code in ScsiDiskInquiryDevice():
>>
>> for (Index = 0; Index < PageLength; Index++) {
>>
>> because the Index variable has type UINT8, and it wraps from 255 to 0, 
>> without ever reaching PageLength (1538), and because 
>> EFI_SCSI_PAGE_CODE_BLOCK_LIMITS_VPD does not occur at offsets 0 through 255.
>>
>> * The fix is not to change the type of Index to UINT16 or a wider type.
>>   Namely, 

Re: [edk2] [PATCH] MdeModulePkg: ScsiDiskDxe: cope with broken "Supported VPD Pages" VPD page

[Laszlo Ersek]

On 05/04/16 18:03, Laszlo Ersek wrote:
> On 05/04/16 13:11, Laszlo Ersek wrote:
>> On 05/04/16 02:58, Tian, Feng wrote:
>>> Laszlo, 
>>>
>>> Could you explain more? 
>>>
>>> Usb Flash drive is managed by UsbMassStorage driver, which is irrelevant 
>>> with UefiScsiLib lib or ScsiDisk driver.
>>
>> Yeah, sorry about missing that part of the context.
>>
>> So, the environment is the following:
>>
>> - The USB flash drive with VendorId/ProductId 0x1516/0x6221 is plugged
>> into a Linux host.
>>
>> - The usb-storage driver in Linux presents the USB flash drive to the
>> rest of the system as a SCSI disk. The device node that is generated for
>> it looks like /dev/sd*, for example, /dev/sdi.
>>
>> - This device node can be used like any other SCSI device.
>>
>> - Using the virtio-scsi HBA, QEMU can provide a virtual machine with
>> various types of virtual SCSI devices. One of those is "scsi-block",
>> which is also known as "SCSI passthrough". It means that most of the
>> SCSI commands are forwarded transparently from guest device to physical
>> host device (/dev/sdi, for example), using the SG_IO ioctl() call. Only
>> a few SCSI commands are captured and emulated by QEMU.
>>
>> This feature is useful e.g. for burning physical optical media, or
>> writing physical tapes, from a virtual machine.
>>
>> - In this scenario, the USB stick that is plugged in the host is exposed
>> to the guest with this feature. So, when the ScsiDiskInquiryDevice()
>> function runs in the guest (as part of OVMF), the INQUIRY command
>> (asking for the Supported VPD Pages" VPD page) is forwarded to the USB
>> stick on the host. And that device returns garbage.
>>
>> - It is important to note that the "garbage" is not inserted by either
>> the host kernel's usb-storage driver, or by QEMU. Even without
>> virtualization, the host kernel consciously avoids asking SCSI disks
>> that are actually backed by USB mass-storage devices for VPD pages,
>> because the kernel developers have realized that most (if not all) of
>> USB mass-storage devices fail to respond correctly. Please see this host
>> kernel commit:
>>
>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=09b6b51b0b6c
>>
>> - In , I
>> demonstrate what happens when such an INQUIRY is sent manually to the
>> SCSI disk that is backed by the USB flash drive. For a genuine SCSI
>> disk, the command completes:
>>
>> # sg_vpd -v /dev/sda
>> Supported VPD pages VPD page:
>> inquiry cdb: 12 01 00 00 fc 00
>>[PQual=0  Peripheral device type: disk]
>>   Supported VPD pages [sv]
>>   Unit serial number [sn]
>>   Device identification [di]
>>   ATA information (SAT) [ai]
>>   Block limits (SBC) [bl]
>>   Block device characteristics (SBC) [bdc]
>>   Logical block provisioning (SBC) [lbpv]
>>
>> Buth the USB flash drive responds with garbage:
>>
>> # sg_vpd -v /dev/sdi
>> Supported VPD pages VPD page:
>> inquiry cdb: 12 01 00 00 fc 00
>> invalid VPD response; probably a STANDARD INQUIRY response
>> First 32 bytes of bad response
>>  00   00 80 06 02 1f 00 00 00  00 00 00 00 00 00 00 00  
>>  10   00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  
>> fetching VPD page failed
>>
>> - So this patch is motivated by the case when a USB mass-storage device
>> is presented as a SCSI disk, directly on the (virtual) hardware level,
>> and it provides a bogus 0x00 VPD page.
> 
> I'm no longer certain that edk2's ScsiDiskDxe is the right place to fix
> this problem. It seems that the usb-storage driver in Linux could be the
> culprit -- it should be watching for INQUIRY commands with EVPD set, and
> reject them before forwarding them to the device. That is the method
> that complies with both SPC-4 and the UFI command spec. (And, edk2
> handles this case correctly, already.) Please see
> .
> 
> So, for the time being, I'm withdrawing this patch.

Sigh, scratch that please (and please excuse the noise).

It turns out that the USB device in question has:
- one configuration
- for that configuration, one interface
- for that interface, a /bInterfaceSubClass/ value of 6
  (implying "SCSI transparent command set")

This implies that the USB device in question is supposed to parse and
handle *all* valid SCSI commands that are sent to it.

Meaning, if the device gets an INQUIRY+EVPD command from ScsiDiskDxe --
transparently forwarded by QEMU and the host Linux kernel --, then the
device is responsible for rejecting the command with CHECK CONDITION (if
the device doesn't support INQUIRY+EVPD).

Instead of this, however, the device silently returns a standard INQUIRY
response (rather than a CHECK CONDITION, or even the requested VPD page).

Hence, this device is *genuinely* broken. And, I would like to get this
patch into ScsiDiskDxe after all.

Thank you
Laszlo

___
edk2-devel mailing list

Re: [edk2] [RFC] EDK2 Platforms Proposal

[Kinney, Michael D]

Liming,

Responses below.

Mike

> -Original Message-
> From: Gao, Liming
> Sent: Tuesday, May 3, 2016 10:43 PM
> To: Kinney, Michael D ; edk2-devel@lists.01.org; 
> Kinney,
> Michael D 
> Cc: Wu, Mike 
> Subject: RE: [RFC] EDK2 Platforms Proposal
> 
> Mike:
>   I have some comments.
> 1) If the platforms wants to base on edk2 master, it is not suggested to be 
> placed into
> edk2-platform repo. Right?

No.  A platform can still be based on edk2/master and be in edk2-platforms.  
Edk2-platforms
is a fork of edk2, so edk2-platforms/master can track edk2/master and any 
branch in edk2-platforms
can choose to sync to edk2/master.

> 2) On add a new platform to edk2-platforms, who approves the request? Edk2 
> maintainer?
> Now, edk2 maintainer are edk2 package owner. So, for new platform, their 
> owner will
> become edk2 maintainer.

Just like edk2-staging, a platform branch in edk2-staging requires one or more 
maintainer(s)
With write access to create and maintain the branch.  If a developer for a 
platform port is 
not a maintainer, they can do their work in a developer owned github fork or 
find a maintainer
to help add platform to edk2-platforms.

> 3) On update source in edk2-platform branch, the change in core package is 
> allowed? For
> example, Platform feature may detect core issue, and this issue is not fixed 
> in edk2
> master. Could platform owners add their fix first or they need wait for the 
> formal fix
> in edk2 master, then sync the fix to their branch?

Just like edk2-staging, each branch owner gets to decide how to manage their 
branch.  If
A temp change is required in a package from edk2/master, then they could be 
done in the 
platform branch and later considered for an edk2 package change or removal of 
temp fix if
a better solution is found.  Platforms are recommended to use edk2/master or a 
stable release
of edk2.

> 3) On remove an edk2-platforms branch, I think we need to collect the 
> feedback from the
> platform owner.


I agree.  That is in the process step 7b to send email to edk2-devel.  

> 
> Thanks
> Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Kinney, Michael
> D
> Sent: Wednesday, May 4, 2016 12:31 AM
> To: edk2-devel@lists.01.org; Kinney, Michael D 
> Subject: [edk2] [RFC] EDK2 Platforms Proposal
> 
> Hello,
> 
> 
> 
> Similar to edk2-staging, we also have a need to manage platforms that have 
> been
> 
> ported to edk2.  Jordan has created a repository called edk2-platforms and
> 
> has created a branch for the minnowboard-max that uses a validated release
> 
> of the UDK 2015 for the dependent packages:
> 
> 
> 
> https://github.com/tianocore/edk2-platforms
> 
> 
> 
> https://github.com/tianocore/edk2-platforms/tree/minnowboard-max-udk2015
> 
> 
> 
> 
> 
> Instead of creating a branch per feature in edk2-staging, the proposal is to
> 
> create a branch per platform in edk2-platforms.  The maintainer(s) that create
> 
> and support a platform branch can decide if the platform is synced to 
> edk2/master
> 
> for dependent packages, or uses a stable release of the edk2 for dependent 
> packages.
> 
> 
> 
> This proposal provides an area for platform development so we can minimize the
> 
> number of platforms that are included in edk2/master.  It is important to keep
> 
> some platforms in edk2/master so we can use those platforms to validate 
> features
> 
> in non-platform packages in edk2/master.  If a new platform does not add 
> feature
> 
> coverage to edk2/master, then a new edk2-platforms branch would be 
> recommended.
> 
> 
> 
> Please review the proposal below for edk2-platforms.
> 
> 
> 
> If this proposal is accepted, then a review of the platform in edk2/master can
> 
> Be done to see if any of them should be moved to branches in edk2-platforms.
> 
> 
> 
> 
> 
> 
> 
> Problem statement
> 
> =
> 
> Need place on tianocore.org where platforms can be maintained by the EDK II
> 
> community.  This serves several purposes:
> 
> 
> 
> * Encourage more platforms sources to be shared earlier in the development 
> process
> 
> * Allow platform sources to be shared that may not yet meet all edk2 required 
> quality
> criteria
> 
> * Allow platform source to be shared so the EDK II community may choose to 
> help finish
> and validate
> 
> * Allow more platforms to be used as part of the edk2 validation and release 
> cycle.
> 
> * Not intended to be used for bug fixes.
> 
> 
> 
> Proposal
> 
> 
> 
> 1) Create a new repo called edk2-platforms
> 
>  a) edk2-platforms is a fork of edk2
> 
>  b) edk2-platforms/master tracks edk2/master
> 
> 
> 
> 2) edk2-platforms discussions can use the existing edk2-devel mailing list for
> design/patch/test.
> 
>  Use the following style for discussion of a platform branch in 
> edk2-platforms
> repo.
> 
> 
> 
>  [platforms/branch]: Subject
> 
> 
> 
> 3) All commits to edk2-platforms must follow same edk2 rules (e.g.

Re: [edk2] Two PCI IO protocols getting produced by same GUID, how to open correct one?

[Kinney, Michael D]

Shaveta,

All UEFI drivers (not just ones for PCI controllers) are responsible for 
evaluating the
ControllerHandle in their EFI Driver Binding Supported() and Start() APIs to 
make sure the
ControllerHandle is the type the driver knows how to manage.  If there is more 
than one
driver that can correctly manage the same ControllerHandle, then there are 
precedence rules
for the order that each driver's Supported() and Stop() APIs are called.

This is described in the UEFP Spec and the UEFI Driver Writer's Guide.  See 
UEFI DWG 
Section 3.14.1 for precedence rules.  See UEFI DWG Section 18.3 for details on 
what a 
PCI Driver is recommended to do in their Supported() and Start() functions.

https://github.com/tianocore/tianocore.github.io/wiki/UEFI-Driver-Writer's-Guide

Mike


> -Original Message-
> From: Shaveta Leekha [mailto:shaveta.lee...@nxp.com]
> Sent: Wednesday, May 4, 2016 12:23 AM
> To: Kinney, Michael D ; Laszlo Ersek 
> 
> Cc: edk2-devel@lists.01.org 
> Subject: RE: [edk2] Two PCI IO protocols getting produced by same GUID, how 
> to open
> correct one?
> 
> Thanks for the reply
> My replies are in-lined.
> 
> Thanks and Regards,
> Shaveta
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Kinney, Michael
> D
> Sent: Tuesday, May 03, 2016 9:29 PM
> To: Shaveta Leekha ; Laszlo Ersek 
> ; Kinney,
> Michael D 
> Cc: edk2-devel@lists.01.org 
> Subject: Re: [edk2] Two PCI IO protocols getting produced by same GUID, how 
> to open
> correct one?
> 
> Shaveta,
> 
> As long as all PCI I/O Protocol instances in the platform conform to UEFI 
> spec, it can
> work.
> 
> [Shaveta] Correct.
> 
> Your original question was about how to tell that a handle has the PCI I/O 
> Protocol
> produced By PCI Bus Driver or your emulation of PCI.  If the PCI I/O 
> Protocols are
> conformant, then It should not matter.  Just write a PCI Device Driver that 
> manages a
> PCI controller based on PCI Class code or Vendor ID/Device ID matching and it 
> should
> work.
> 
> [Shaveta] Yes, eventually correct the PCI I/O Protocol produced my PCI 
> emulation layer
> code is getting opened.
> As and when a valid protocol gets opened, I am checking the CLASS code and 
> Sub-CLASS
> code before proceeding.
> And that's working.
> 
> But what if, some other device's driver in my system, intended to open "PCI 
> I/O
> Protocol produced By PCI Bus Driver"
> Doesn't test CLASS CODE or any SUB-CLASS/VENDOR etc., then it may land up 
> using API's
> provided with protocol
> Of my PCI emulation layer code? is that understanding correct ??
> 
> So in such scenarios, where more than one protocol instances (with different 
> APIs
> associated) are installed with same GUID in the system,
> Then all the drivers should check VENDOR ID/CLASS etc immediately after 
> opening the
> protocol?
> 
> 
> 
> Mike
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Shaveta Leekha
> > Sent: Tuesday, May 3, 2016 3:27 AM
> > To: Laszlo Ersek 
> > Cc: edk2-devel@lists.01.org 
> > Subject: Re: [edk2] Two PCI IO protocols getting produced by same
> > GUID, how to open correct one?
> >
> >
> >
> > -Original Message-
> > From: Laszlo Ersek [mailto:ler...@redhat.com]
> > Sent: Tuesday, May 03, 2016 1:15 PM
> > To: Shaveta Leekha 
> > Cc: edk2-devel@lists.01.org 
> > Subject: Re: [edk2] Two PCI IO protocols getting produced by same
> > GUID, how to open correct one?
> >
> > On 05/03/16 08:54, Shaveta Leekha wrote:
> > > Hi,
> > >
> > > I have a scenario where two separate drivers are installing/producing 
> > > "PCI IO"
> > protocol with same GUID (gEfiPciIoProtocolGuid).
> > >
> > > Where:
> > >
> > > (1)One of the PCI Io protocol instance is produced by
> > "MdeModulePkg/Bus/Pci/PciBus" driver for real PCI devices to use( like
> > E1000/NIC card to use)
> > >
> > > (2)Other PCI Io protocol instance is produced by "Pci Emulation" 
> > > layer created
> > for USB and SATA kind of controllers. This protocol (APIs) is intended
> > to be used by USB and SATA controller drivers.
> > >
> > > Now when I use "OpenProtocol" in my "Platform specific Sata controller 
> > > driver"
> using:
> > >
> > > Status = gBS->OpenProtocol (
> > >   Controller,
> > >   &gEfiPciIoProtocolGuid,
> > >   (VOID **) &PciIo,
> > >   This->DriverBindingHandle,
> > >   Controller,
> > >   EFI_OPEN_PROTOCOL_GET_PROTOCOL
> > >   );
> > >
> > > How can I make sure that PCI Io protocol that is produced by
> > > "PciEmulation" layer
> > driver is getting opened?
> > >
> > > As "gEfiPciIoProtocolGuid" is same for both protocol instances.
> > >
> > > How to handle such scenarios?
> >
> > The UEFI spec forbids installing more than one protocol interface with
> > the same GUID on the same handle. See
> > EFI_BOOT_SERVICES.InstallProtocolInterface()

Re: [edk2] [PATCH v1 1/1] NetworkPkg:HttpDxe: Code changes to support HTTP PUT/POST operations

[El-Haj-Mahmoud, Samer]

Series Reviewed-By: Samer El-Haj-Mahmoud 


-Original Message-
From: Hegde, Nagaraj P 
Sent: Wednesday, May 4, 2016 5:46 AM
To: edk2-devel@lists.01.org
Cc: jiaxin...@intel.com; siyuan...@intel.com; ting...@intel.com; 
El-Haj-Mahmoud, Samer 
Subject: [PATCH v1 1/1] NetworkPkg:HttpDxe: Code changes to support HTTP 
PUT/POST operations

Code changes enables HttpDxe to handle PUT/POST operations.
EfiHttpRequest assumes "Request" and "HttpMsg->Headers" can never be NULL. 
Also, HttpResponseWorker assumes HTTP Reponse will contain headers. We could 
have response which could contain only a string (HTTP 100 Continue) and no 
headers. Code changes tries to do-away from these assumptions, which would 
enable HttpDxe to support PUT/POST operations.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hegde, Nagaraj P nagaraj-p.he...@hpe.com
---
 NetworkPkg/HttpDxe/HttpDriver.c |   3 +
 NetworkPkg/HttpDxe/HttpImpl.c   | 399 +++-
 NetworkPkg/HttpDxe/HttpProto.h  |   1 +
 3 files changed, 226 insertions(+), 177 deletions(-)

diff --git a/NetworkPkg/HttpDxe/HttpDriver.c b/NetworkPkg/HttpDxe/HttpDriver.c 
index 2518f4e..0bde012 100644
--- a/NetworkPkg/HttpDxe/HttpDriver.c
+++ b/NetworkPkg/HttpDxe/HttpDriver.c
@@ -2,6 +2,7 @@
   The driver binding and service binding protocol for HttpDxe driver.
 
   Copyright (c) 2015, 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 @@ -939,6 +940,8 @@ HttpServiceBindingCreateChild (
   
   HttpInstance->Signature = HTTP_PROTOCOL_SIGNATURE;
   HttpInstance->Service   = HttpService;
+  HttpInstance->Method = HttpMethodMax;
+
   CopyMem (&HttpInstance->Http, &mEfiHttpTemplate, sizeof 
(HttpInstance->Http));
   NetMapInit (&HttpInstance->TxTokens);
   NetMapInit (&HttpInstance->RxTokens); diff --git 
a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c index 
7a236f4..9360355 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -248,151 +248,178 @@ EfiHttpRequest (
   HTTP_TOKEN_WRAP   *Wrap;
   CHAR8 *FileUrl;
   UINTN RequestMsgSize;
-  
+
+  Url = NULL;
+  HostName = NULL;
+  RequestMsg = NULL;
+  HostNameStr = NULL;
+  Wrap = NULL;
+  FileUrl = NULL;
   if ((This == NULL) || (Token == NULL)) {
 return EFI_INVALID_PARAMETER;
   }
 
   HttpMsg = Token->Message;
-  if ((HttpMsg == NULL) || (HttpMsg->Headers == NULL)) {
+  if (HttpMsg == NULL) {
 return EFI_INVALID_PARAMETER;
   }
 
-  //
-  // Current implementation does not support POST/PUT method.
-  // If future version supports these two methods, Request could be NULL for a 
special case that to send large amounts
-  // of data. For this case, the implementation need check whether previous 
call to Request() has been completed or not.
-  //
-  //
   Request = HttpMsg->Data.Request;
-  if ((Request == NULL) || (Request->Url == NULL)) {
-return EFI_INVALID_PARAMETER;
-  }
 
   //
-  // Only support GET and HEAD method in current implementation.
+  // Only support GET, HEAD, PUT and POST method in current implementation.
   //
-  if ((Request->Method != HttpMethodGet) && (Request->Method != 
HttpMethodHead)) {
+  if ((Request != NULL) && (Request->Method != HttpMethodGet) &&
+  (Request->Method != HttpMethodHead) && (Request->Method != 
+ HttpMethodPut) && (Request->Method != HttpMethodPost)) {
 return EFI_UNSUPPORTED;
   }
 
   HttpInstance = HTTP_INSTANCE_FROM_PROTOCOL (This);
   ASSERT (HttpInstance != NULL);
 
+  if (Request != NULL) {
+HttpInstance->Method = Request->Method;  }
+
   if (HttpInstance->State < HTTP_STATE_HTTP_CONFIGED) {
 return EFI_NOT_STARTED;
   }
 
-  //
-  // Check whether the token already existed.
-  //
-  if (EFI_ERROR (NetMapIterate (&HttpInstance->TxTokens, HttpTokenExist, 
Token))) {
-return EFI_ACCESS_DENIED;   
-  }  
+  if (Request == NULL) {
+//
+// Request would be NULL only for PUT/POST operation (in the current 
implementation)
+//
+if ((HttpInstance->Method != HttpMethodPut) && (HttpInstance->Method != 
HttpMethodPost)) {
+  return EFI_INVALID_PARAMETER;
+}
 
-  HostName= NULL;
-  Wrap= NULL;
-  HostNameStr = NULL;
+//
+// For PUT/POST, we need to have the TCP already configured. Bail out if 
it is not!
+//
+if (HttpInstance->State < HTTP_STATE_TCP_CONFIGED) {
+  return EFI_INVALID_PARAMETER;
+}
 
-  //
-  // Parse the URI of the remote host.
-  //
-  Url = HttpInstance->Url;
-  UrlLen = StrLen (Request->Url) + 1;
-  if (UrlLen > HTTP_URL_BUFFER_LEN) {
-Url = AllocateZeroPool (UrlLen);
-if (Url == NULL) {
-  return EFI_OUT_OF_RESOURCES;
+//
+// We need to have the Message Body for sending the HTTP message across in 
these cases.
+//
+if (HttpMsg->Body == 

Re: [edk2] [Patch 0/2] Do not use hard coded TTL/ToS in PXE driver.

[El-Haj-Mahmoud, Samer]

Thanks for fixing this. We noticed the same and were about to submit a patch.

Series Reviewed-By: Samer El-Haj-Mahmoud 


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu Siyuan
Sent: Monday, May 2, 2016 9:26 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [Patch 0/2] Do not use hard coded TTL/ToS in PXE driver.

EFI_PXE_BASE_CODE_PROTOCOL has interface to set the TTL and ToS value, but not 
used by the UdpWrite() interface. The code always use a hard coded 16 for the 
TTL and 0 for ToS.
This patch update the UpdWrite() to use the TTL and ToS which have been set by 
the SetParameters().

Fu Siyuan (2):
  NetworkPkg: Do not use hard coded TTL/ToS in PXE driver.
  MdeModulePkg: Do not use hard coded TTL/ToS in PXE driver.

 .../Universal/Network/UefiPxeBcDxe/PxeBcImpl.c |  6 --
 .../Universal/Network/UefiPxeBcDxe/PxeBcSupport.c  | 12   
.../Universal/Network/UefiPxeBcDxe/PxeBcSupport.h  | 14 +-
 NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c|  6 --
 NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c | 12 
 NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h | 22 +-
 6 files changed, 46 insertions(+), 26 deletions(-)

--
2.7.4.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] [RFC 0/1] Complete the Pkcs7VerifyDxe protocol

[James Bottomley]

On Wed, 2016-03-30 at 06:53 +, Long, Qin wrote:
> Hi, James,
> 
> Do we have final conclusion against the Pkcs7 protocol updates?
> Will the VerifySignature interface be deprecated or re-enabled for 
> signed-image verification?

We do now.  We'll add a note to the standard explaining the problems
with the interface but, since it can be used safely if you know what
you're doing, we'll keep it in the spec and not deprecate it, so I'll
resubmit the RFC as a patch.

James


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

[edk2] [PATCH 0/1] Complete the Pkcs7VerifyDxe protocol

[James Bottomley]

The tianocore implementation is currently incomplete, it does
VerifyBuffer but not VerifySignature.  We have a use for
VerifySignature in some Linux projects because we currently roll our
own openssl implementations for verifying authenticode signatures, but
we'd like to drop all of our internal ssl code in favour of a platform
provided interface.  The first step to doing this is to use Tianocore
to demonstrate viability.  I'm currently building my OVMF package with
this patch:

https://build.opensuse.org/package/show/home:jejb1:UEFI/OVMF

So I can experiment with a version of efitools that's using the
VerifySignature function to perform all of the code signing
verifications:

http://git.kernel.org/cgit/linux/kernel/git/jejb/efitools.git/

Since we can now use pkcs7verifyDxe to load this protocol, we'd really
like it to become an official part of tianocore so we can install it
even on EFI versions that don't have it natively, meaning that we can
ship it along with our shim/preloader systems without having to carry
our own separate version of openssl.

James

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

[edk2] [PATCH 1/1] Complete the Pkcs7VerifyDxe protocol

[James Bottomley]

VerifySignature can be implemented using a mirror of the
AuthenticodeVerify function that's already in use in the
ImageVerificationDXE environment, so this patch simply wires up
VerifySignature using that code.

Signed-off-by: James Bottomley 
---
 .../Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c| 378 -
 1 file changed, 375 insertions(+), 3 deletions(-)

diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c 
b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
index 07fdf55..319e11a 100644
--- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
+++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
@@ -116,6 +116,81 @@ _Exit:
 /**
   Check whether the hash of data content is revoked by the revocation database.
 
+  @param[in]  Hash  Pointer to the hash that is searched for.
+  @param[in]  HashSize  The size of the hash in bytes.
+  @param[in]  RevokedDb Pointer to a list of pointers to EFI_SIGNATURE_LIST
+structure which contains list of X.509 certificates
+of revoked signers and revoked content hashes.
+
+  @return TRUE   The matched content hash is found in the revocation database.
+  @return FALSE  The matched content hash is not found in the revocation 
database.
+
+**/
+BOOLEAN
+IsContentHashRevokedByHash (
+  IN  UINT8  *Hash,
+  IN  UINTN  HashSize,
+  IN  EFI_SIGNATURE_LIST **RevokedDb
+  )
+{
+  EFI_SIGNATURE_LIST  *SigList;
+  EFI_SIGNATURE_DATA  *SigData;
+  UINTN   Index;
+  UINTN   EntryIndex;
+  UINTN   EntryCount;
+  BOOLEAN Status;
+
+  if (RevokedDb == NULL) {
+return FALSE;
+  }
+
+  Status = FALSE;
+  //
+  // Check if any hash matching content hash can be found in RevokedDB
+  //
+  for (Index = 0; ; Index++) {
+SigList = (EFI_SIGNATURE_LIST *)(RevokedDb[Index]);
+
+//
+// The list is terminated by a NULL pointer.
+//
+if (SigList == NULL) {
+  break;
+}
+
+//
+// Search the signature database to search the revoked content hash
+//
+SigData= (EFI_SIGNATURE_DATA *) ((UINT8 *) SigList + sizeof 
(EFI_SIGNATURE_LIST) +
+SigList->SignatureHeaderSize);
+EntryCount = (SigList->SignatureListSize - SigList->SignatureHeaderSize -
+ sizeof (EFI_SIGNATURE_LIST)) / SigList->SignatureSize;
+for (EntryIndex = 0; EntryIndex < EntryCount; EntryIndex++) {
+  //
+  // The problem case.  There's a revocation hash but the sizes
+  // don't match, meaning it's a different hash algorithm and we
+  // can't tell if it's revoking our binary or not.  Assume not.
+  //
+  if (SigList->SignatureSize - sizeof(EFI_GUID) == HashSize) {
+   //
+   // Compare Data Hash with Signature Data
+   //
+   if (CompareMem (SigData->SignatureData, Hash, HashSize) == 0) {
+ Status = TRUE;
+ goto _Exit;
+   }
+  }
+
+  SigData = (EFI_SIGNATURE_DATA *) ((UINT8 *) SigData + 
SigList->SignatureSize);
+}
+  }
+
+_Exit:
+  return Status;
+}
+/**
+  Check whether the hash of data content is revoked by the revocation database.
+
   @param[in]  Content   Pointer to the content buffer that is searched for.
   @param[in]  ContentSize   The size of data content in bytes.
   @param[in]  RevokedDb Pointer to a list of pointers to EFI_SIGNATURE_LIST
@@ -449,6 +524,171 @@ IsValidTimestamp (
   @param[in]  SignedData  Pointer to buffer containing ASN.1 DER-encoded 
PKCS7
   signature.
   @param[in]  SignedDataSize  The size of SignedData buffer in bytes.
+  @param[in]  InHash  Pointer to the buffer containing the hash of the 
mesage data
+  previously signed and to be verified.
+  @param[in]  InHashSize  The size of InHash buffer in bytes.
+  @param[in]  RevokedDb   Pointer to a list of pointers to 
EFI_SIGNATURE_LIST
+  structure which contains list of X.509 
certificates
+  of revoked signers and revoked content hashes.
+  @param[in]  TimeStampDb Pointer to a list of pointers to 
EFI_SIGNATURE_LIST
+  structures which is used to pass a list of X.509
+  certificates of trusted timestamp signers.
+
+  @retval  EFI_SUCCESS The PKCS7 signedData is revoked.
+  @retval  EFI_SECURITY_VIOLATION  Fail to verify the signature in PKCS7 
signedData.
+  @retval  EFI_INVALID_PARAMETER   SignedData is NULL or SignedDataSize is 
zero.
+   AllowedDb is NULL.
+   Content is not NULL and ContentSize is NULL.
+  @retval  EFI_NOT_FOUND   Content not found because InData is NULL 
and no
+   content embedded in PKCS7 signedData.
+  @retval  EFI_UNSUPPORTED The PKCS7 s

Re: [edk2] [PATCH v2 2/3] IntelFrameworkModulePkg/BdsDxe: Fix for the BDS boot timeout

[Daniil Egranov]

Hi Ray,

Thank you for review and comments. Please see my answers below.

Regards,
Daniil

On 05/04/2016 12:04 AM, Ni, Ruiyu wrote:

3 comments below.

Regards,
Ray


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Daniil 
Egranov
Sent: Wednesday, May 4, 2016 9:34 AM
To: edk2-devel@lists.01.org
Cc: Fan, Jeff 
Subject: [edk2] [PATCH v2 2/3] IntelFrameworkModulePkg/BdsDxe: Fix for the BDS 
boot timeout

The patch loads timeout value from the "Timeout" global variable and passes
it to PlatformBdsEnterFrontPage(), which handles delay and key input.
The PcdPlatformBootTimeOut is only used at the BDS entry point and updates
the "Timeout" value. This will allow the modification of the timeout value
through the BDS menu and overwrite it if PcdPlatformBootTimeOut has been
set.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daniil Egranov 
---
IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c| 18 +++---
.../Universal/BdsDxe/BootMaint/BootMaint.c | 17 -
.../Universal/BdsDxe/BootMaint/UpdatePage.c| 13 +++--
3 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
index ae7ad21..1d80fca 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -123,6 +123,7 @@ BdsBootDeviceSelect (
   BOOLEAN   BootNextExist;
   LIST_ENTRY*LinkBootNext;
   EFI_EVENT ConnectConInEvent;
+  UINTN Size;

   //
   // Got the latest boot option
@@ -214,6 +215,18 @@ BdsBootDeviceSelect (
   if (Link == NULL) {
 return ;
   }
+
+  //Read boot timeout variable
+  Status = gRT->GetVariable (L"Timeout",
+ &gEfiGlobalVariableGuid,
+ NULL,
+ &Size,
+ (VOID *) &Timeout);
+
+  if (EFI_ERROR (Status)) {
+Timeout = 0x;
+  }
+
   //
   // Here we make the boot in a loop, every boot success will
   // return to the front page
@@ -222,7 +235,7 @@ BdsBootDeviceSelect (
 //
 // Check the boot option list first
 //
-if (Link == &BootLists) {
+if (Link == &BootLists || Timeout != 0x) {
   //
   // When LazyConIn enabled, signal connect ConIn event before enter UI
   //
@@ -238,12 +251,11 @@ BdsBootDeviceSelect (
   //one is success to boot, then we back here to allow user
   //add new active boot option
   //
-  Timeout = 0x;
   PlatformBdsEnterFrontPage (Timeout, FALSE);
+  Timeout = 0x;


1. The old code is to enter front page unconditionally and immediately while you
changed the behavior to wait for Timeout seconds before entering front page.
Why?
The previous logic allows entry into the BDS boot menu in two cases: no 
active boot options and all attempts to boot from the current boot 
options fail. If there is a valid bootable device in the boot options, i 
found no way to interrupt the boot process. In a lot of cases, the boot 
options have to be changed before any boot attempts have been made. This 
patch checks if boot timeout is set and uses PlatformBdsEnterFrontPage 
to manage time and key inputs before using current boot options.




   InitializeListHead (&BootLists);
   BdsLibBuildOptionFromVar (&BootLists, L"BootOrder");
   Link = BootLists.ForwardLink;
-  continue;
 }
 //
 // Get the boot option from the link list
diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c
b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c
index d4b4475..cc2d656 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c
@@ -111,6 +111,9 @@ InitializeBmmConfig (
   BM_MENU_ENTRY   *NewMenuEntry;
   BM_LOAD_CONTEXT *NewLoadContext;
   UINT16  Index;
+  UINT16  BootTimeOut;
+  EFI_STATUS  Status;
+  UINTN   Size;

   ASSERT (CallbackData != NULL);

@@ -128,7 +131,19 @@ InitializeBmmConfig (
 }
   }

-  CallbackData->BmmFakeNvData.BootTimeOut = PcdGet16 (PcdPlatformBootTimeOut);
+  //Read boot timeout variable. If PcdPlatformBootTimeOut has been set,
+  //the Timeout variable will be initialized as part of the BDS startup 
procedure
+  Status = gRT->GetVariable ( L"Timeout",
+  &gEfiGlobalVariableGuid,
+  NULL,
+  &Size,
+  (VOID *) &BootTimeOut);
+
+  if (EFI_ERROR (Status)) {
+  BootTimeOut = 0;
+  }
+
+  CallbackData->BmmFakeNvData.BootTimeOut = BootTimeOut;


2. With the platform DSC maps the PcdPlatformBootTimeout to L"Timeout" variable 
correctly,
the above change is not necessary. right?
I agree, but what about the case

Re: [edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot timeout message

[Daniil Egranov]

Hi Ray,

Thanks for the review. My answers below.

Thanks,
Daniil

On 05/04/2016 12:07 AM, Ni, Ruiyu wrote:

2 comments below.

Regards,
Ray


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Daniil 
Egranov
Sent: Wednesday, May 4, 2016 9:34 AM
To: edk2-devel@lists.01.org
Cc: Fan, Jeff 
Subject: [edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot 
timeout message

The PlatformBdsShowProgress() supports graphics mode only, which is not
applicable for RS-232 serial console. Show the progress message as a console
text message in case PlatformBdsShowProgress() fails.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daniil Egranov 
---
IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
index 6958979..d1a5c05 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
@@ -925,7 +925,7 @@ ShowProgress (
 // Show progress
 //
 if (TmpStr != NULL) {
-  PlatformBdsShowProgress (
+  Status = PlatformBdsShowProgress (
 Foreground,
 Background,
 TmpStr,
@@ -933,12 +933,19 @@ ShowProgress (
 ((TimeoutDefault - TimeoutRemain) * 100 / TimeoutDefault),
 0
 );
+  if (EFI_ERROR(Status)) {
+//if graphics mode is not supported (serial console) show text 
progress message
+AsciiPrint ("\rPress any key to enter Boot Menu in %d seconds 
", TimeoutRemain);
+  }

1. Why use AsciiPrint but not Print(L"")?
I agree they are the same but normally we use Print().



I was not sure which one to use. I'll correct it.


 }
   }
 }

 if (TmpStr != NULL) {
   gBS->FreePool (TmpStr);
+if (EFI_ERROR(Status)) {
+  AsciiPrint ("\n");
+}

2. What's the purpose of the EOL here?



The AsciiPrint above uses cartridge return without a new line so this 
EOL preserves last message from being erased by other console outputs.



 }

 //
--
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

[edk2] [PATCH 2/7] CorebootPayloadPkg: Make shell selectable

[Lee Leahy]

Add all of the shell options from ShellBinPkg including building the
shell from source.

Enable link time optimization for GCC debug builds to keep the size
under 0x3e.

Test: Use -DSHELL_TYPE=BUILD_SHELL command line options to build the
shell from source.  Run the result on Galileo Gen2.

Change-Id: I1e12adb57960ac5e75e682073540a9322aa03081
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Lee Leahy 
---
 CorebootPayloadPkg/CorebootPayloadPkg.fdf|  28 +++-
 CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc| 189 +++---
 CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 193 ---
 3 files changed, 287 insertions(+), 123 deletions(-)

diff --git a/CorebootPayloadPkg/CorebootPayloadPkg.fdf 
b/CorebootPayloadPkg/CorebootPayloadPkg.fdf
index 3cc5a4d..2072602 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkg.fdf
+++ b/CorebootPayloadPkg/CorebootPayloadPkg.fdf
@@ -153,17 +153,33 @@ INF 
MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
 #
 # Shell
 #
-#!if $(ARCH) == IA32
-#INF  RuleOverride = BINARY USE = IA32 ShellBinPkg/UefiShell/UefiShell.inf
-#!else
-#INF  RuleOverride = BINARY USE = X64 ShellBinPkg/UefiShell/UefiShell.inf
-#!endif
-#
+!if $(SHELL_TYPE) == BUILD_SHELL
+INF  ShellPkg/Application/Shell/Shell.inf
+!endif
+
+!if $(SHELL_TYPE) == FULL_BIN
 !if $(ARCH) == IA32
 INF  RuleOverride = BINARY USE = IA32 EdkShellBinPkg/FullShell/FullShell.inf
 !else
 INF  RuleOverride = BINARY USE = X64 EdkShellBinPkg/FullShell/FullShell.inf
 !endif
+!endif
+
+!if $(SHELL_TYPE) == MIN_BIN
+!if $(ARCH) == IA32
+INF  RuleOverride = BINARY USE = IA32 ShellBinPkg/MinUefiShell/MinUefiShell.inf
+!else
+INF  RuleOverride = BINARY USE = X64 ShellBinPkg/MinUefiShell/MinUefiShell.inf
+!endif
+!endif
+
+!if $(SHELL_TYPE) == UEFI_BIN
+!if $(ARCH) == IA32
+INF  RuleOverride = BINARY USE = IA32 ShellBinPkg/UefiShell/UefiShell.inf
+!else
+INF  RuleOverride = BINARY USE = X64 ShellBinPkg/UefiShell/UefiShell.inf
+!endif
+!endif
 
 FILE FREEFORM= PCD(gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile) {
   SECTION RAW = MdeModulePkg/Logo/Logo.bmp
diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc 
b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
index 49afa73..58cb063 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
@@ -1,16 +1,16 @@
 ## @file
 # Coreboot Payload Package
 #
-# Provides drivers and definitions to create uefi payload for coreboot.
+# Provides drivers and definitions to create uefi payload for coreboot.
 #
-# Copyright (c) 2014 - 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 that accompanies this 
distribution.
+# Copyright (c) 2014 - 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 that 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.
+# 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.
 #
 ##
 
@@ -23,16 +23,21 @@
   PLATFORM_NAME   = CorebootPayloadPkg
   PLATFORM_GUID   = F71608AB-D63D-4491-B744-A8C8CD96
   PLATFORM_VERSION= 0.1
-  DSC_SPECIFICATION   = 0x00010005
+  DSC_SPECIFICATION   = 0x00010005
   SUPPORTED_ARCHITECTURES = IA32
   BUILD_TARGETS   = DEBUG|RELEASE|NOOPT
   SKUID_IDENTIFIER= DEFAULT
   OUTPUT_DIRECTORY= Build/CorebootPayloadPkgIA32
   FLASH_DEFINITION= 
CorebootPayloadPkg/CorebootPayloadPkg.fdf
-
+
   DEFINE SECURE_BOOT_ENABLE  = FALSE
   DEFINE SOURCE_DEBUG_ENABLE = FALSE
-
+
+  #
+  # Shell options: [BUILD_SHELL, FULL_BIN, MIN_BIN, NONE, UEFI]
+  #
+  DEFINE SHELL_TYPE  = FULL_BIN
+
 [BuildOptions]
   GCC:*_UNIXGCC_*_CC_FLAGS   = -DMDEPKG_NDEBUG
   GCC:RELEASE_*_*_CC_FLAGS   = -DMDEPKG_NDEBUG
@@ -65,7 +70,7 @@
   #
   # Basic
   #
-  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
+  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
   BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
   
SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
   PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
@@ -76,7 +81,7 @@
   PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
   
PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeC

[edk2] [PATCH 1/7] CorebootPayloadPkg/PlatformHelperLib: Remove unreferenced function

[Lee Leahy]

Remove the PlatformFlashEraseWrite function which is not used within
CorebootPayloadPkg.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Lee Leahy 
---
 .../Include/Library/PlatformHelperLib.h|  38 +-
 .../Library/PlatformHelperLib/PlatformHelperDxe.c  | 141 -
 2 files changed, 1 insertion(+), 178 deletions(-)

diff --git a/QuarkPlatformPkg/Include/Library/PlatformHelperLib.h 
b/QuarkPlatformPkg/Include/Library/PlatformHelperLib.h
index a6cc863..28dc096 100644
--- a/QuarkPlatformPkg/Include/Library/PlatformHelperLib.h
+++ b/QuarkPlatformPkg/Include/Library/PlatformHelperLib.h
@@ -1,7 +1,7 @@
 /** @file
 PlatformHelperLib function prototype definitions.
 
-Copyright (c) 2013 Intel Corporation.
+Copyright (c) 2013 - 2016 Intel Corporation.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
@@ -106,42 +106,6 @@ PlatformFlashLockPolicy (
   IN CONST BOOLEANPreBootPolicy
   );
 
-/**
-  Erase and Write to platform flash.
-
-  Routine accesses one flash block at a time, each access consists
-  of an erase followed by a write of FLASH_BLOCK_SIZE. One or both
-  of DoErase & DoWrite params must be TRUE.
-
-  Limitations:-
-CpuWriteAddress must be aligned to FLASH_BLOCK_SIZE.
-DataSize must be a multiple of FLASH_BLOCK_SIZE.
-
-  @param   Smst   If != NULL then InSmm and use to locate
-  SpiProtocol.
-  @param   CpuWriteAddressAddress in CPU memory map of flash region.
-  @param   Data   The buffer containing the data to be written.
-  @param   DataSize   Amount of data to write.
-  @param   DoEraseEarse each block.
-  @param   DoWriteWrite to each block.
-
-  @retval  EFI_SUCCESSOperation successful.
-  @retval  EFI_NOT_READY  Required resources not setup.
-  @retval  EFI_INVALID_PARAMETER  Invalid parameter.
-  @retval  Others Unexpected error happened.
-
-**/
-EFI_STATUS
-EFIAPI
-PlatformFlashEraseWrite (
-  IN  VOID  *Smst,
-  IN  UINTN CpuWriteAddress,
-  IN  UINT8 *Data,
-  IN  UINTN DataSize,
-  IN  BOOLEAN   DoErase,
-  IN  BOOLEAN   DoWrite
-  );
-
 /** Check if System booted with recovery Boot Stage1 image.
 
   @retval  TRUEIf system booted with recovery Boot Stage1 image.
diff --git a/QuarkPlatformPkg/Library/PlatformHelperLib/PlatformHelperDxe.c 
b/QuarkPlatformPkg/Library/PlatformHelperLib/PlatformHelperDxe.c
index 18dbd8b..441f760 100644
--- a/QuarkPlatformPkg/Library/PlatformHelperLib/PlatformHelperDxe.c
+++ b/QuarkPlatformPkg/Library/PlatformHelperLib/PlatformHelperDxe.c
@@ -325,147 +325,6 @@ PlatformFlashLockPolicy (
   }
 }
 
-/**
-  Erase and Write to platform flash.
-
-  Routine accesses one flash block at a time, each access consists
-  of an erase followed by a write of FLASH_BLOCK_SIZE. One or both
-  of DoErase & DoWrite params must be TRUE.
-
-  Limitations:-
-CpuWriteAddress must be aligned to FLASH_BLOCK_SIZE.
-DataSize must be a multiple of FLASH_BLOCK_SIZE.
-
-  @param   Smst   If != NULL then InSmm and use to locate
-  SpiProtocol.
-  @param   CpuWriteAddressAddress in CPU memory map of flash region.
-  @param   Data   The buffer containing the data to be written.
-  @param   DataSize   Amount of data to write.
-  @param   DoEraseEarse each block.
-  @param   DoWriteWrite to each block.
-
-  @retval  EFI_SUCCESSOperation successful.
-  @retval  EFI_NOT_READY  Required resources not setup.
-  @retval  EFI_INVALID_PARAMETER  Invalid parameter.
-  @retval  Others Unexpected error happened.
-
-**/
-EFI_STATUS
-EFIAPI
-PlatformFlashEraseWrite (
-  IN  VOID  *Smst,
-  IN  UINTN CpuWriteAddress,
-  IN  UINT8 *Data,
-  IN  UINTN DataSize,
-  IN  BOOLEAN   DoErase,
-  IN  BOOLEAN   DoWrite
-  )
-{
-  EFI_STATUSStatus;
-  UINT64CpuBaseAddress;
-  SPI_INIT_INFO *SpiInfo;
-  UINT8 *WriteBuf;
-  UINTN Index;
-  UINTN SpiWriteAddress;
-  EFI_SPI_PROTOCOL  *SpiProtocol;
-
-  if (!DoErase && !DoWrite) {
-return EFI_INVALID_PARAMETER;
-  }
-  if (DoWrite && Data == NULL) {
-return EFI_INVALID_PARAMETER;
-  }
-  if ((CpuWriteAddress % FLASH_BLOCK_SIZE) != 0) {
-return EFI_INVALID_PARAMETER;
-  }
-  if ((DataSize % FLASH_BLOCK_SIZ

[edk2] [PATCH 3/7] CorebootPayloadPkg: Make serial I/O configurable

[Lee Leahy]

Allow the serial port configuration to be overriden from the command
line.
Make the debug serial PCDs patchable in module.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Lee Leahy 
---
 CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc| 73 ---
 CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 74 +---
 2 files changed, 131 insertions(+), 16 deletions(-)

diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc 
b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
index 58cb063..7c9a1eb 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
@@ -34,6 +34,40 @@
   DEFINE SOURCE_DEBUG_ENABLE = FALSE
 
   #
+  # Serial port set up
+  #
+  DEFINE BAUD_RATE= 115200
+  DEFINE SERIAL_CLOCK_RATE= 1843200
+  DEFINE SERIAL_LINE_CONTROL  = 3 # 8-bits, no parity
+  DEFINE SERIAL_HARDWARE_FLOW_CONTROL = FALSE
+  DEFINE SERIAL_DETECT_CABLE  = FALSE
+  DEFINE SERIAL_FIFO_CONTROL  = 7 # Enable FIFO
+  DEFINE SERIAL_EXTENDED_TX_FIFO_SIZE = 16
+  DEFINE UART_DEFAULT_BAUD_RATE   = $(BAUD_RATE)
+  DEFINE UART_DEFAULT_DATA_BITS   = 8
+  DEFINE UART_DEFAULT_PARITY  = 1
+  DEFINE UART_DEFAULT_STOP_BITS   = 1
+  DEFINE DEFAULT_TERMINAL_TYPE= 0
+
+  #
+  #  typedef struct {
+  #UINT16  VendorId;  ///< Vendor ID to match the PCI device.  The 
value 0x terminates the list of entries.
+  #UINT16  DeviceId;  ///< Device ID to match the PCI device
+  #UINT32  ClockRate; ///< UART clock rate.  Set to 0 for default 
clock rate of 1843200 Hz
+  #UINT64  Offset;///< The byte offset into to the BAR
+  #UINT8   BarIndex;  ///< Which BAR to get the UART base address
+  #UINT8   RegisterStride;///< UART register stride in bytes.  Set to 
0 for default register stride of 1 byte.
+  #UINT16  ReceiveFifoDepth;  ///< UART receive FIFO depth in bytes. Set 
to 0 for a default FIFO depth of 16 bytes.
+  #UINT16  TransmitFifoDepth; ///< UART transmit FIFO depth in bytes. Set 
to 0 for a default FIFO depth of 16 bytes.
+  #UINT8   Reserved[2];
+  #  } PCI_SERIAL_PARAMETER;
+  #
+  # Vendor  Device  Prog Interface 1, BAR #0, Offset 0, Stride = 1, 
Clock 1843200 (0x1c2000)
+  #
+  #   [Vendor]   [Device]  
[ClockRate---]  [Offset---] [Bar] [Stride] [RxFifo] 
[TxFifo]   [Rsvd]   [Vendor]
+  DEFINE PCI_SERIAL_PARAMETERS= {0x00,0x00, 0x00,0x00, 
0x0,0x20,0x1c,0x00, 0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0, 0x00,0x01, 0x0,0x0, 
0x0,0x0, 0x0,0x0, 0xff,0xff}
+
+  #
   # Shell options: [BUILD_SHELL, FULL_BIN, MIN_BIN, NONE, UEFI]
   #
   DEFINE SHELL_TYPE  = FULL_BIN
@@ -203,13 +237,6 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
 
 [PcdsFixedAtBuild]
-  gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x7
-  gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x804F
-!if $(SOURCE_DEBUG_ENABLE)
-  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x17
-!else
-  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2F
-!endif
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x1
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize|0x8000
   gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x1
@@ -221,8 +248,38 @@
 !endif
 
 [PcdsPatchableInModule.common]
+  gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x7
+  gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x804F
+!if $(SOURCE_DEBUG_ENABLE)
+  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x17
+!else
+  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2F
+!endif
+
+  #
+  # The following parameters are set by Library/PlatformHookLib
+  #
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|FALSE
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x03F8
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x3f8
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialBaudRate|$(BAUD_RATE)
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|1
+
+  #
+  # Enable these parameters to be set on the command line
+  #
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|$(SERIAL_CLOCK_RATE)
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialLineControl|$(SERIAL_LINE_CONTROL)
+  
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseHardwareFlowControl|$(SERIAL_HARDWARE_FLOW_CONTROL)
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialDetectCable|$(SERIAL_DETECT_CABLE)
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|$(SERIAL_FIFO_CONTROL)
+  
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|$(SERIAL_EXTENDED_TX_FIFO_SIZE)
+
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|$(UART_DEFAULT_BAUD_RATE)
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits|$(UART_DEFAULT_DATA_BITS)
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity|$(UART_DEFAULT_PARITY)
+  gEfi

[edk2] [PATCH 6/7] CorebootModulePkg/SerialDxe: Use PlatformHookLib

[Lee Leahy]

Copy the driver from MdeModulePkg/Universal/SerialDxe.  Add
PlatformHookLib to the Library section of the .inf file to adjust the
PCDs for the UART.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Lee Leahy 
---
 CorebootModulePkg/SerialDxe/SerialDxe.inf  |  55 +++
 CorebootModulePkg/SerialDxe/SerialDxe.uni  |  21 +
 CorebootModulePkg/SerialDxe/SerialDxeExtra.uni |  19 +
 CorebootModulePkg/SerialDxe/SerialIo.c | 528 +
 4 files changed, 623 insertions(+)
 create mode 100644 CorebootModulePkg/SerialDxe/SerialDxe.inf
 create mode 100644 CorebootModulePkg/SerialDxe/SerialDxe.uni
 create mode 100644 CorebootModulePkg/SerialDxe/SerialDxeExtra.uni
 create mode 100644 CorebootModulePkg/SerialDxe/SerialIo.c

diff --git a/CorebootModulePkg/SerialDxe/SerialDxe.inf 
b/CorebootModulePkg/SerialDxe/SerialDxe.inf
new file mode 100644
index 000..8489e06
--- /dev/null
+++ b/CorebootModulePkg/SerialDxe/SerialDxe.inf
@@ -0,0 +1,55 @@
+## @file
+#  Serial driver that layers on top of a Serial Port Library instance.
+#
+#  Copyright (c) 2008 - 2015, Intel Corporation. All rights reserved.
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD 
License
+#  which accompanies this distribution.  The full text of the license may be 
found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+#
+##
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = SerialDxe
+  MODULE_UNI_FILE= SerialDxe.uni
+  FILE_GUID  = D3987D4B-971A-435F-8CAF-4967EB627241
+  MODULE_TYPE= DXE_DRIVER
+  VERSION_STRING = 1.0
+
+  ENTRY_POINT= SerialDxeInitialize
+
+[Sources.common]
+  SerialIo.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  CorebootModulePkg/CorebootModulePkg.dec
+
+[LibraryClasses]
+  UefiDriverEntryPoint
+  UefiBootServicesTableLib
+  DebugLib
+  PcdLib
+  PlatformHookLib
+  SerialPortLib
+
+[Protocols]
+  gEfiSerialIoProtocolGuid  ## PRODUCES
+  gEfiDevicePathProtocolGuid## PRODUCES
+
+[Pcd]
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity   ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES
+
+[Depex]
+  TRUE
+
+[UserExtensions.TianoCore."ExtraFiles"]
+  SerialDxeExtra.uni
diff --git a/CorebootModulePkg/SerialDxe/SerialDxe.uni 
b/CorebootModulePkg/SerialDxe/SerialDxe.uni
new file mode 100644
index 000..e2daf27
--- /dev/null
+++ b/CorebootModulePkg/SerialDxe/SerialDxe.uni
@@ -0,0 +1,21 @@
+// /** @file
+// Serial driver that layers on top of a Serial Port Library instance.
+//
+// Serial driver that layers on top of a Serial Port Library instance.
+//
+// Copyright (c) 2008 - 2015, Intel Corporation. All rights reserved.
+//
+// This program and the accompanying materials
+// are licensed and made available under the terms and conditions of the BSD 
License
+// which accompanies this distribution. The full text of the license may be 
found at
+// http://opensource.org/licenses/bsd-license.php
+// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+// WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+//
+// **/
+
+
+#string STR_MODULE_ABSTRACT #language en-US "Serial driver that 
layers on top of a Serial Port Library instance"
+
+#string STR_MODULE_DESCRIPTION  #language en-US "Serial driver that 
layers on top of a Serial Port Library instance."
+
diff --git a/CorebootModulePkg/SerialDxe/SerialDxeExtra.uni 
b/CorebootModulePkg/SerialDxe/SerialDxeExtra.uni
new file mode 100644
index 000..3790487
--- /dev/null
+++ b/CorebootModulePkg/SerialDxe/SerialDxeExtra.uni
@@ -0,0 +1,19 @@
+// /** @file
+// SerialDxe Localized Strings and Content
+//
+// Copyright (c) 2008 - 2015, Intel Corporation. All rights reserved.
+//
+// This program and the accompanying materials
+// are licensed and made available under the terms and conditions of the BSD 
License
+// which accompanies this distribution. The full text of the license may be 
found at
+// http://opensource.org/licenses/bsd-license.php
+// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+// WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+//
+// **/
+
+#string STR_PROPERTIES_MODULE_NAME 
+#language en-US 
+"SerialDxe Driver"
+
+
diff --git a/CorebootModulePkg/SerialDxe/SerialIo.c 
b/CorebootModulePkg/SerialDxe/SerialIo.c
new file mode 100644
index 000..171eb46
--- /dev/null
+++ b/CorebootModulePkg/SerialDxe/SerialIo.c
@@ -0,0 +1,528 @@
+/** @file
+  Serial driver that layers on top of a Ser

[edk2] [PATCH 5/7] CorebootModulePkg/PciSioSerialDxe: Use PlatformHookLib

[Lee Leahy]

Copy the driver from MdeModulePkg/Bus/Pci/PciSioSerialDxe.  Add
PlatformHookLib to the Library section of the .inf file to adjust the
PCDs for the UART.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Lee Leahy 
---
 CorebootModulePkg/PciSioSerialDxe/ComponentName.c  |  288 +
 .../PciSioSerialDxe/PciSioSerialDxe.inf|   82 ++
 .../PciSioSerialDxe/PciSioSerialDxe.uni|   21 +
 .../PciSioSerialDxe/PciSioSerialDxeExtra.uni   |   18 +
 CorebootModulePkg/PciSioSerialDxe/Serial.c | 1248 ++
 CorebootModulePkg/PciSioSerialDxe/Serial.h |  789 
 CorebootModulePkg/PciSioSerialDxe/SerialIo.c   | 1320 
 7 files changed, 3766 insertions(+)
 create mode 100644 CorebootModulePkg/PciSioSerialDxe/ComponentName.c
 create mode 100644 CorebootModulePkg/PciSioSerialDxe/PciSioSerialDxe.inf
 create mode 100644 CorebootModulePkg/PciSioSerialDxe/PciSioSerialDxe.uni
 create mode 100644 CorebootModulePkg/PciSioSerialDxe/PciSioSerialDxeExtra.uni
 create mode 100644 CorebootModulePkg/PciSioSerialDxe/Serial.c
 create mode 100644 CorebootModulePkg/PciSioSerialDxe/Serial.h
 create mode 100644 CorebootModulePkg/PciSioSerialDxe/SerialIo.c

diff --git a/CorebootModulePkg/PciSioSerialDxe/ComponentName.c 
b/CorebootModulePkg/PciSioSerialDxe/ComponentName.c
new file mode 100644
index 000..994dc84
--- /dev/null
+++ b/CorebootModulePkg/PciSioSerialDxe/ComponentName.c
@@ -0,0 +1,288 @@
+/** @file
+  UEFI Component Name and Name2 protocol for Isa serial driver.
+
+Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD 
License
+which accompanies this distribution.  The full text of the license may be 
found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include "Serial.h"
+
+//
+// EFI Component Name Protocol
+//
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_COMPONENT_NAME_PROTOCOL  
gPciSioSerialComponentName = {
+  SerialComponentNameGetDriverName,
+  SerialComponentNameGetControllerName,
+  "eng"
+};
+
+//
+// EFI Component Name 2 Protocol
+//
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_COMPONENT_NAME2_PROTOCOL 
gPciSioSerialComponentName2 = {
+  (EFI_COMPONENT_NAME2_GET_DRIVER_NAME) SerialComponentNameGetDriverName,
+  (EFI_COMPONENT_NAME2_GET_CONTROLLER_NAME) 
SerialComponentNameGetControllerName,
+  "en"
+};
+
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_UNICODE_STRING_TABLE 
mSerialDriverNameTable[] = {
+  {
+"eng;en",
+L"PCI SIO Serial Driver"
+  },
+  {
+NULL,
+NULL
+  }
+};
+
+GLOBAL_REMOVE_IF_UNREFERENCED CHAR16 mSioSerialPortName[] = L"SIO Serial Port 
#%d";
+GLOBAL_REMOVE_IF_UNREFERENCED CHAR16 mPciSerialPortName[] = L"PCI Serial Port 
#%d";
+
+/**
+  Retrieves a Unicode string that is the user readable name of the driver.
+
+  This function retrieves the user readable name of a driver in the form of a
+  Unicode string. If the driver specified by This has a user readable name in
+  the language specified by Language, then a pointer to the driver name is
+  returned in DriverName, and EFI_SUCCESS is returned. If the driver specified
+  by This does not support the language specified by Language,
+  then EFI_UNSUPPORTED is returned.
+
+  @param  This[in]  A pointer to the EFI_COMPONENT_NAME2_PROTOCOL 
or
+EFI_COMPONENT_NAME_PROTOCOL instance.
+
+  @param  Language[in]  A pointer to a Null-terminated ASCII string
+array indicating the language. This is the
+language of the driver name that the caller is
+requesting, and it must match one of the
+languages specified in SupportedLanguages. The
+number of languages supported by a driver is up
+to the driver writer. Language is specified
+in RFC 4646 or ISO 639-2 language code format.
+
+  @param  DriverName[out]   A pointer to the Unicode string to return.
+This Unicode string is the name of the
+driver specified by This in the language
+specified by Language.
+
+  @retval EFI_SUCCESS   The Unicode string for the Driver specified by
+This and the language specified by Language was
+returned in DriverName.
+
+  @retval EFI_INVALID_PARAMETER Language is NULL.
+
+  @retval EFI_INVALID_PARAMETER DriverName is NULL.
+
+  @retval EFI_UNSUPPORTED   The driver specified by This does not support
+the language specified 

[edk2] [PATCH 4/7] CorebootPayloadPkg: Allow MaxLogicalProcessorNumber to be changed

[Lee Leahy]

Add a define and use it with MaxLogicalProcessorNumber to enable this
PCD to be changed via the command line.  Quark needs to set this value
to one during the builds.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Lee Leahy 
---
 CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc| 7 +++
 CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 7 +++
 2 files changed, 14 insertions(+)

diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc 
b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
index 7c9a1eb..e0c14f3 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
@@ -34,6 +34,11 @@
   DEFINE SOURCE_DEBUG_ENABLE = FALSE
 
   #
+  # CPU options
+  #
+  DEFINE MAX_LOGICAL_PROCESSORS  = 64
+
+  #
   # Serial port set up
   #
   DEFINE BAUD_RATE= 115200
@@ -281,6 +286,8 @@
   gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|$(DEFAULT_TERMINAL_TYPE)
   
gEfiMdeModulePkgTokenSpaceGuid.PcdPciSerialParameters|$(PCI_SERIAL_PARAMETERS)
 
+  
gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|$(MAX_LOGICAL_PROCESSORS)
+
 

 #
 # Pcd Dynamic Section - list of all EDK II PCD Entries defined by this Platform
diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc 
b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
index 884a4e2..540d591 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
@@ -34,6 +34,11 @@
   DEFINE SOURCE_DEBUG_ENABLE = FALSE
 
   #
+  # CPU options
+  #
+  DEFINE MAX_LOGICAL_PROCESSORS  = 64
+
+  #
   # Serial port set up
   #
   DEFINE BAUD_RATE= 115200
@@ -286,6 +291,8 @@
   gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|$(DEFAULT_TERMINAL_TYPE)
   
gEfiMdeModulePkgTokenSpaceGuid.PcdPciSerialParameters|$(PCI_SERIAL_PARAMETERS)
 
+  
gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|$(MAX_LOGICAL_PROCESSORS)
+
 

 #
 # Pcd Dynamic Section - list of all EDK II PCD Entries defined by this Platform
-- 
1.9.1

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

[edk2] [PATCH 7/7] CorebootPayloadPkg: Use serial drivers with PlatformHookLib

[Lee Leahy]

Use the serial drivers which update the serial PCDs from
PlatformHookLib.

Change-Id: Ie6a3526d56332ee1cf07edb24ff39634a981183f
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Lee Leahy 
---
 CorebootPayloadPkg/CorebootPayloadPkg.fdf| 39 
 CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc|  3 +-
 CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc |  3 +-
 3 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/CorebootPayloadPkg/CorebootPayloadPkg.fdf 
b/CorebootPayloadPkg/CorebootPayloadPkg.fdf
index 2072602..bb8d1f6 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkg.fdf
+++ b/CorebootPayloadPkg/CorebootPayloadPkg.fdf
@@ -1,16 +1,16 @@
 ## @file
 # Coreboot Payload Package
 #
-# Provides drivers and definitions to create uefi payload for coreboot.
+# Provides drivers and definitions to create uefi payload for coreboot.
 #
-# Copyright (c) 2014 - 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 that accompanies this 
distribution.
+# Copyright (c) 2014 - 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 that 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.
+# 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.
 #
 ##
 
@@ -93,30 +93,31 @@ INF 
MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
 INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
 INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
 INF 
MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
-INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
-INF PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
+INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
+INF PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
 INF MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
 
 INF UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
 INF MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
-INF PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf
+INF PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf
 INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
-INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
+INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
 INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
-INF CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
+INF CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
 
 INF MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
 #
 # PCI Support
 #
-INF 
CorebootModulePkg/PciRootBridgeNoEnumerationDxe/PciRootBridgeNoEnumeration.inf
-INF CorebootModulePkg/PciBusNoEnumerationDxe/PciBusNoEnumeration.inf
+INF 
CorebootModulePkg/PciRootBridgeNoEnumerationDxe/PciRootBridgeNoEnumeration.inf
+INF CorebootModulePkg/PciBusNoEnumerationDxe/PciBusNoEnumeration.inf
+INF CorebootModulePkg/PciSioSerialDxe/PciSioSerialDxe.inf
 
 #
 # ISA Support
 #
-INF MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
+INF CorebootModulePkg/SerialDxe/SerialDxe.inf
 
 #
 # Console Support
@@ -132,13 +133,13 @@ INF 
MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
 INF MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
 INF MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
 INF MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
-INF CorebootModulePkg/SataControllerDxe/SataControllerDxe.inf
+INF CorebootModulePkg/SataControllerDxe/SataControllerDxe.inf
 INF MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf
 INF MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
 INF MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
 INF MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
 
-INF FatPkg/EnhancedFatDxe/Fat.inf
+INF FatPkg/EnhancedFatDxe/Fat.inf
 
 #
 # Usb Support
@@ -186,7 +187,7 @@ FILE FREEFORM= 
PCD(gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile) {
 }
 
 #
-# Framebuffer Gop
+# Framebuffer Gop
 #
 INF CorebootPayloadPkg/FbGop/FbGop.inf
 
diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc 
b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
index e0c14f3..e1420fe 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
@@ -388,6 +388,7 @@
   #
   
CorebootModulePkg/PciRootBridgeNoEnumeratio

Re: [edk2] [patch] Nt32Pkg: Fix SnpNt32 GetStatus bug

[Wu, Jiaxin]

Reviewed-by: Jiaxin Wu 


> -Original Message-
> From: Zhang, Lubo
> Sent: Friday, April 29, 2016 10:09 AM
> To: Fu, Siyuan ; edk2-devel@lists.01.org
> Cc: Ye, Ting ; Wu, Jiaxin 
> Subject: RE: [edk2] [patch] Nt32Pkg: Fix SnpNt32 GetStatus bug
> 
> A good comment to me, I will change it at the commit time, thanks
> 
> -Original Message-
> From: Fu, Siyuan
> Sent: Friday, April 29, 2016 10:04 AM
> To: Zhang, Lubo ; edk2-devel@lists.01.org
> Cc: Ye, Ting ; Wu, Jiaxin 
> Subject: RE: [edk2] [patch] Nt32Pkg: Fix SnpNt32 GetStatus bug
> 
> Hi, Lubo
> 
> The allocate memory in SnpNt32InitializeGlobalData is better to use
>   AllocatePool (sizeof (UINT64) * This->MaxRecycledTxBuf); to avoid
> possible macro value update in future.
> Other parts are good to me.
> 
> 
> Reviewed-by: Fu Siyuan 
> 
> 
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Zhang Lubo
> > Sent: Friday, April 29, 2016 9:50 AM
> > To: edk2-devel@lists.01.org
> > Cc: Ye, Ting ; Fu, Siyuan ;
> > Wu, Jiaxin 
> > Subject: [edk2] [patch] Nt32Pkg: Fix SnpNt32 GetStatus bug
> >
> > According to UEFI spec, the Snp.GetStatus should return the recycled
> > transmit buffer address, while the NT32 SNP always return value 1 for
> > the Txbuffer.
> >
> > Cc: Fu Siyuan 
> > Cc: Ye Ting 
> > Cc: Wu Jiaxin 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Zhang Lubo 
> > ---
> >  Nt32Pkg/SnpNt32Dxe/SnpNt32.c | 42
> > --
> >  Nt32Pkg/SnpNt32Dxe/SnpNt32.h | 22 +-
> >  2 files changed, 61 insertions(+), 3 deletions(-)
> >
> > diff --git a/Nt32Pkg/SnpNt32Dxe/SnpNt32.c
> > b/Nt32Pkg/SnpNt32Dxe/SnpNt32.c index 4dee182..6d22c2f 100644
> > --- a/Nt32Pkg/SnpNt32Dxe/SnpNt32.c
> > +++ b/Nt32Pkg/SnpNt32Dxe/SnpNt32.c
> > @@ -1,8 +1,8 @@
> >  /** @file
> >
> > -Copyright (c) 2006 - 2013, Intel Corporation. All rights
> > reserved.
> > +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 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
> >
> > @@ -42,10 +42,13 @@ SNPNT32_GLOBAL_DATA gSnpNt32GlobalData
> = {
> >{
> >  0,
> >  0,
> >  EfiLockUninitialized
> >},  //  Lock
> > +  NULL,   //  RecycledTxBuf
> > +  0,  //  RecycledTxBufCount
> > +  32, //  MaxRecycledTxBuf
> >//
> >//  Private functions
> >//
> >SnpNt32InitializeGlobalData,//  InitializeGlobalData
> >SnpNt32InitializeInstanceData,  //  InitializeInstanceData
> > @@ -859,13 +862,24 @@ SnpNt32GetStatus (
> >IN EFI_SIMPLE_NETWORK_PROTOCOL *This,
> >OUT UINT32 *InterruptStatus,
> >OUT VOID   **TxBuffer
> >)
> >  {
> > +  SNPNT32_INSTANCE_DATA *Instance;
> > +  SNPNT32_GLOBAL_DATA   *GlobalData;
> > +
> > +  Instance= SNP_NT32_INSTANCE_DATA_FROM_SNP_THIS (This);
> > +
> > +  GlobalData  = Instance->GlobalData;
> >
> >if (TxBuffer != NULL) {
> > -*((UINT8 **) TxBuffer) = (UINT8 *)(UINTN) 1;
> > +if (GlobalData->RecycledTxBufCount != 0) {
> > +  GlobalData->RecycledTxBufCount --;
> > +  *((UINT8 **) TxBuffer)= (UINT8 *) (UINTN)GlobalData-
> > >RecycledTxBuf[GlobalData->RecycledTxBufCount];
> > +} else {
> > +  *((UINT8 **) TxBuffer)= NULL;
> > +}
> >}
> >
> >if (InterruptStatus != NULL) {
> >  *InterruptStatus = EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT;
> >}
> > @@ -916,10 +930,11 @@ SnpNt32Transmit (
> >)
> >  {
> >SNPNT32_INSTANCE_DATA *Instance;
> >SNPNT32_GLOBAL_DATA   *GlobalData;
> >INT32 ReturnValue;
> > +  UINT64*Tmp;
> >
> >Instance= SNP_NT32_INSTANCE_DATA_FROM_SNP_THIS (This);
> >
> >GlobalData  = Instance->GlobalData;
> >
> > @@ -943,10 +958,28 @@ SnpNt32Transmit (
> >
> >EfiReleaseLock (&GlobalData->Lock);
> >
> >if (ReturnValue < 0) {
> >  return EFI_DEVICE_ERROR;
> > +  } else {
> > +if ((GlobalData->MaxRecycledTxBuf +
> > SNP_TX_BUFFER_INCREASEMENT) >= SNP_MAX_TX_BUFFER_NUM) {
> > +  return EFI_NOT_READY;
> > +}
> > +
> > +if (GlobalData->RecycledTxBufCount < GlobalData->MaxRecycledTxBuf)
> {
> > +  GlobalData->RecycledTxBuf[GlobalData->RecycledTxBufCount] =
> > + (UINT64)
> > Buffer;
> > +  GlobalData->RecycledTxBufCount ++;
> > +} else {
> > +  Tmp = AllocatePool (sizeof (UINT64) *
> > + (GlobalData->MaxRecycledTxBuf +
> > SNP_TX_BUFFER_INCREASEMENT));
> > +  if (Tmp == NULL) {
> > +return EFI_DEVICE_ERROR;
> > +  }
> > +  CopyMem (Tmp, GlobalData->RecycledTxBuf, sizeof (UINT64) *
> > GlobalData->Rec

[edk2] [Patch] SecurityPkg TcgStorageOpalLib: Check the capability before use.

[Eric Dong]

For Pyrite SSC device, it may not supports Active Key,  So
add check logic before enable it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Eric Dong 
Cc: Feng, Tian 
---
 .../Library/TcgStorageOpalLib/TcgStorageOpalCore.c | 50 --
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c 
b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
index 2db5ffe..f4f5f30 100644
--- a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
+++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
@@ -814,6 +814,7 @@ OpalSetLockingSpAuthorityEnabledAndPin(
   TCG_PARSE_STRUCT  ParseStruct;
   UINT32Size;
   TCG_UID   ActiveKey;
+  TCG_RESULTRet;
 
   NULL_CHECK(LockingSpSession);
   NULL_CHECK(NewPin);
@@ -901,30 +902,35 @@ OpalSetLockingSpAuthorityEnabledAndPin(
   ERROR_CHECK(OpalCreateRetrieveGlobalLockingRangeActiveKey(LockingSpSession, 
&CreateStruct, &Size));
   ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), 
&ParseStruct, MethodStatus));
 
-  ERROR_CHECK(OpalParseRetrieveGlobalLockingRangeActiveKey(&ParseStruct, 
&ActiveKey));
-
-  ERROR_CHECK(TcgInitTcgCreateStruct(&CreateStruct, Buf, sizeof(Buf)));
-  ERROR_CHECK(TcgCreateSetAce(
-  &CreateStruct,
-  &Size,
-  LockingSpSession->OpalBaseComId,
-  LockingSpSession->ComIdExtension,
-  LockingSpSession->TperSessionId,
-  LockingSpSession->HostSessionId,
-  (ActiveKey == OPAL_LOCKING_SP_K_AES_256_GLOBALRANGE_KEY) ? 
OPAL_LOCKING_SP_ACE_K_AES_256_GLOBALRANGE_GENKEY : 
OPAL_LOCKING_SP_ACE_K_AES_128_GLOBALRANGE_GENKEY,
-  OPAL_LOCKING_SP_USER1_AUTHORITY,
-  TCG_ACE_EXPRESSION_OR,
-  OPAL_LOCKING_SP_ADMINS_AUTHORITY
-  ));
+  //
+  // For Pyrite type SSC, it not supports Active Key. 
+  // So here add check logic before enable it.
+  //
+  Ret = OpalParseRetrieveGlobalLockingRangeActiveKey(&ParseStruct, &ActiveKey);
+  if (Ret == TcgResultSuccess) {
+ERROR_CHECK(TcgInitTcgCreateStruct(&CreateStruct, Buf, sizeof(Buf)));
+ERROR_CHECK(TcgCreateSetAce(
+&CreateStruct,
+&Size,
+LockingSpSession->OpalBaseComId,
+LockingSpSession->ComIdExtension,
+LockingSpSession->TperSessionId,
+LockingSpSession->HostSessionId,
+(ActiveKey == OPAL_LOCKING_SP_K_AES_256_GLOBALRANGE_KEY) ? 
OPAL_LOCKING_SP_ACE_K_AES_256_GLOBALRANGE_GENKEY : 
OPAL_LOCKING_SP_ACE_K_AES_128_GLOBALRANGE_GENKEY,
+OPAL_LOCKING_SP_USER1_AUTHORITY,
+TCG_ACE_EXPRESSION_OR,
+OPAL_LOCKING_SP_ADMINS_AUTHORITY
+));
 
-  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), 
&ParseStruct, MethodStatus));
+ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), 
&ParseStruct, MethodStatus));
 
-  if (*MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
-DEBUG ((DEBUG_INFO, "Update ACE for GLOBALRANGE_GENKEY failed\n"));
-//
-//TODO do we want to disable user1 if all permissions are not granted
-//
-return TcgResultFailure;
+if (*MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
+  DEBUG ((DEBUG_INFO, "Update ACE for GLOBALRANGE_GENKEY failed\n"));
+  //
+  // TODO do we want to disable user1 if all permissions are not granted
+  //
+  return TcgResultFailure;
+}
   }
 
   ERROR_CHECK(TcgInitTcgCreateStruct(&CreateStruct, Buf, sizeof(Buf)));
-- 
2.6.4.windows.1

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

Re: [edk2] [Patch 0/2] Do not use hard coded TTL/ToS in PXE driver.

[Wu, Jiaxin]

Series Reviewed-By: Wu Jiaxin 


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu
> Siyuan
> Sent: Tuesday, May 3, 2016 10:26 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch 0/2] Do not use hard coded TTL/ToS in PXE driver.
> 
> EFI_PXE_BASE_CODE_PROTOCOL has interface to set the TTL and ToS value,
> but not used by the UdpWrite() interface. The code always use a hard coded
> 16 for the TTL and 0 for ToS.
> This patch update the UpdWrite() to use the TTL and ToS which have been
> set by the SetParameters().
> 
> Fu Siyuan (2):
>   NetworkPkg: Do not use hard coded TTL/ToS in PXE driver.
>   MdeModulePkg: Do not use hard coded TTL/ToS in PXE driver.
> 
>  .../Universal/Network/UefiPxeBcDxe/PxeBcImpl.c |  6 --
>  .../Universal/Network/UefiPxeBcDxe/PxeBcSupport.c  | 12 
>   .../Universal/Network/UefiPxeBcDxe/PxeBcSupport.h  | 14 +-
>  NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c|  6 --
>  NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c | 12 
>  NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h | 22 +
> -
>  6 files changed, 46 insertions(+), 26 deletions(-)
> 
> --
> 2.7.4.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] NetworkPkg: Discard TCP segment when no TCB found

[Fu, Siyuan]

Hi, Michael

Sending this RST is required by TCP RFC793 as below:

http://tools.ietf.org/html/rfc793#section-3.9

SEGMENT ARRIVES

If the state is CLOSED (i.e., TCB does not exist) then

  all data in the incoming segment is discarded.  An incoming
  segment containing a RST is discarded.  *An incoming segment not
  containing a RST causes a RST to be sent in response*.  The
  acknowledgment and sequence field values are selected to make the
  reset sequence acceptable to the TCP that sent the offending
  segment.

For the case that "other running Managed Network instance with it's own network 
stack", I think it's not TCP layer's responsibility, instead, it should be 
handled by SNP or MNP driver. They shouldn't deliver those network packets 
which are not owned by the network stack about it. If other network stack want 
to take over the device from edk2 network stack drivers, it had better to 
disconnect the edk2 network stack first, to avoid such races between the two 
network stacks.

Best Regards
Siyuan

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Michael Chang
> Sent: Wednesday, May 4, 2016 6:23 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH] NetworkPkg: Discard TCP segment when no TCB
> found
> 
> The TCP driver should discard(ignore) the incoming TCP segment when there's
> no
> TCB found to handle it, because that TCP segment may be subjected to be
> received by other running Managed Network instance with it's own network
> stack
> to handle it. Force sending reset segment in this case would just discrupt any
> TCP connection except the one established by EFI TCP protocol, which is not
> make sense imho.
> 
> Thanks.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Michael Chang 
> 
> ---
>  NetworkPkg/TcpDxe/TcpInput.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/NetworkPkg/TcpDxe/TcpInput.c b/NetworkPkg/TcpDxe/TcpInput.c
> index 745ee4c..7aa6f7e 100644
> --- a/NetworkPkg/TcpDxe/TcpInput.c
> +++ b/NetworkPkg/TcpDxe/TcpInput.c
> @@ -794,10 +794,10 @@ TcpInput (
>);
> 
>if ((Tcb == NULL) || (Tcb->State == TCP_CLOSED)) {
> -DEBUG ((EFI_D_INFO, "TcpInput: send reset because no TCB found\n"));
> +DEBUG ((EFI_D_INFO, "TcpInput: discard a segment because no TCB
> found\n"));
> 
>  Tcb = NULL;
> -goto SEND_RESET;
> +goto DISCARD;
>}
> 
>Seg = TcpFormatNetbuf (Tcb, Nbuf);
> --
> 2.6.6
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch 0/2] 0001-NetworkPkg-Ignore-BootFileName-if-it-is-overloaded

[Wu, Jiaxin]

Series Reviewed-By: Wu Jiaxin 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu
> Siyuan
> Sent: Tuesday, May 3, 2016 1:36 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch 0/2] 0001-NetworkPkg-Ignore-BootFileName-if-it-is-
> overloaded
> 
> 
> Fu Siyuan (2):
>   NetworkPkg: Ignore BootFileName if it is overloaded.
>   NetworkPkg: Ignore BootFileName if it is overloaded.
> 
>  NetworkPkg/HttpBootDxe/HttpBootDhcp4.c | 5 -
>  NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c   | 7 +--
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> --
> 2.7.4.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 v2] NetworkPkg: Use UefiBootManagerLib API to create load option.

[Wang, Sunny (HPS SW)]

Hi Siyuan, 
I just found a possible memory leak issue with the following code block. Others 
look good to me. 
+  Status = EfiBootManagerAddLoadOptionVariable (&NewOption, (UINTN) -1);  
+  if (EFI_ERROR (Status)) {
+EfiBootManagerFreeLoadOption (&NewOption);
+   }

I think we always need to call EfiBootManagerFreeLoadOption no matter 
EfiBootManagerAddLoadOptionVariable returns error or not because the memory 
allocated for NewOption (description and device path) is no longer needed. We 
already set NewOption to variable, so the data in memory will not be used 
anymore. Therefore, we can modify the code block above to the following, what 
do you think? 
+  EfiBootManagerAddLoadOptionVariable (&NewOption, (UINTN) -1);  
+  EfiBootManagerFreeLoadOption (&NewOption);
 
By the way, thanks for addressing my comment in the other review.

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu Siyuan
Sent: Wednesday, May 04, 2016 1:58 PM
To: edk2-devel@lists.01.org
Cc: Ye Ting ; Ni Ruiyu ; Wu Jiaxin 

Subject: [edk2] [PATCH v2] NetworkPkg: Use UefiBootManagerLib API to create 
load option.

V2:
Remove unnecessary ZeroMem and free load option.

This patch updates the HTTP boot driver to use the API in UefiBootManagerLib to 
create new load option, to avoid duplicate code.

Cc: Ye Ting 
Cc: Wu Jiaxin 
Cc: Ni Ruiyu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 NetworkPkg/HttpBootDxe/HttpBootConfig.c | 136 ++--
 NetworkPkg/HttpBootDxe/HttpBootDxe.inf  |   1 +
 NetworkPkg/NetworkPkg.dsc   |   8 ++
 3 files changed, 33 insertions(+), 112 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootConfig.c 
b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
index 00e4782..2ca38b5 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootConfig.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
@@ -13,6 +13,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 **/
 
 #include "HttpBootDxe.h"
+#include 
 
 CHAR16  mHttpBootConfigStorageName[] = L"HTTP_BOOT_CONFIG_IFR_NVDATA";
 
@@ -36,31 +37,18 @@ HttpBootAddBootOption (
   IN   CHAR16   *Uri
   )
 {
-  EFI_DEV_PATH   *Node;
-  EFI_DEVICE_PATH_PROTOCOL   *TmpDevicePath;
-  EFI_DEVICE_PATH_PROTOCOL   *NewDevicePath;
-  UINTN  Length;
-  CHAR8  AsciiUri[URI_STR_MAX_SIZE];
-  CHAR16 *CurrentOrder;
-  EFI_STATUS Status;
-  UINTN  OrderCount;
-  UINTN  TargetLocation;
-  BOOLEANFound;
-  UINT8  *TempByteBuffer;
-  UINT8  *TempByteStart;
-  UINTN  DescSize;
-  UINTN  FilePathSize;
-  CHAR16 OptionStr[10];
-  UINT16 *NewOrder;
-  UINTN  Index;
-
-  NewOrder  = NULL;
-  TempByteStart = NULL;
+  EFI_DEV_PATH  *Node;
+  EFI_DEVICE_PATH_PROTOCOL  *TmpDevicePath;
+  EFI_DEVICE_PATH_PROTOCOL  *NewDevicePath;
+  UINTN Length;
+  CHAR8 AsciiUri[URI_STR_MAX_SIZE];
+  EFI_STATUSStatus;
+  UINTN Index;
+  EFI_BOOT_MANAGER_LOAD_OPTION  NewOption;
+
   NewDevicePath = NULL;
-  NewOrder  = NULL;
   Node  = NULL;
   TmpDevicePath = NULL;
-  CurrentOrder  = NULL;
 
   if (StrLen (Description) == 0) {
 return EFI_INVALID_PARAMETER;
@@ -137,105 +125,29 @@ HttpBootAddBootOption (
   }
 
   //
-  // Get current "BootOrder" variable and find a free target.
-  //
-  Length = 0;
-  Status = GetVariable2 (
- L"BootOrder",
- &gEfiGlobalVariableGuid,
- (VOID **)&CurrentOrder,
- &Length 
- );
-  if (EFI_ERROR (Status) && Status != EFI_NOT_FOUND) {
-goto ON_EXIT;
-  }
-  OrderCount = Length / sizeof (UINT16);
-  Found = FALSE;
-  for (TargetLocation=0; TargetLocation < 0x; TargetLocation++) {
-Found = TRUE;
-for (Index = 0; Index < OrderCount; Index++) {
-  if (CurrentOrder[Index] == TargetLocation) {
-Found = FALSE;
-break;
-  }
-}
-if (Found) {
-  break;
-}
-  }
-
-  if (TargetLocation == 0x) {
-DEBUG ((EFI_D_ERROR, "Could not find unused target index.\n"));
-Status = EFI_OUT_OF_RESOURCES;
-goto ON_EXIT;
-  } else {
-DEBUG ((EFI_D_INFO, "TargetIndex = %04x.\n", TargetLocation));
-  }
-
-  //
-  // Construct and set the "Boot" variable
+  // Add a new load option.
   //
-  DescSize = StrSize(Description);
-  FilePathSize = GetDevicePathSize (NewDevicePath);
-  TempByteBuffer = AllocateZeroPool(sizeof(EFI_LOAD_OPTION) + DescSize + 
FilePathSize);
-  if (TempByteBuffer == NULL) {
-Status = EFI_OUT_OF_RESOURCES;
-goto ON_EXIT;
-  }
-
-  TempBy

Re: [edk2] [patch] MdeModulePkg/HiiDatabaseDxe: Fix memory leak issues in HiiDatabaseDxe

[Qiu, Shumin]

Reviewed-by: Qiu Shumin 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Dandan Bi
Sent: Wednesday, May 04, 2016 4:05 PM
To: edk2-devel@lists.01.org
Cc: Qiu, Shumin; Dong, Eric
Subject: [edk2] [patch] MdeModulePkg/HiiDatabaseDxe: Fix memory leak issues in 
HiiDatabaseDxe


Cc: Qiu Shumin 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
Reviewed-by: Eric Dong 
---
 .../Universal/HiiDatabaseDxe/ConfigRouting.c| 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c 
b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
index cc155cc..a704734 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
@@ -547,10 +547,12 @@ FindSameBlockElement(
   return Status;
 }
 ASSERT (TempBuffer != NULL);
 if ((BufferLen == Length) && (0 == CompareMem (Buffer, TempBuffer, 
Length))) {
   *Found = TRUE;
+  FreePool (TempBuffer);
+  TempBuffer = NULL;
   return EFI_SUCCESS;
 } else {
   FreePool (TempBuffer);
   TempBuffer = NULL;
   BlockPtr = StrStr (BlockPtr + 1, BlockName); @@ -1885,10 +1887,13 @@ 
IsThisPackageList (
   AsciiStrToUnicodeStr ((CHAR8 *)IfrVarStore->Name, VarStoreName);
 
   if (IsThisVarstore((VOID *)&IfrVarStore->Guid, VarStoreName, ConfigHdr)) 
{
 FindVarstore = TRUE;
 goto Done;
+  } else {
+FreePool (VarStoreName);
+VarStoreName = NULL;
   }
   break;
 
 case EFI_IFR_VARSTORE_EFI_OP:
   IfrEfiVarStore = (EFI_IFR_VARSTORE_EFI *) IfrOpHdr; @@ -1899,10 +1904,13 
@@ IsThisPackageList (
   AsciiStrToUnicodeStr ((CHAR8 *)IfrEfiVarStore->Name, VarStoreName);
 
   if (IsThisVarstore (&IfrEfiVarStore->Guid, VarStoreName, ConfigHdr)) {
 FindVarstore = TRUE;
 goto Done;
+  } else {
+FreePool (VarStoreName);
+VarStoreName = NULL;
   }
   break;
 
 case EFI_IFR_VARSTORE_NAME_VALUE_OP:
   IfrNameValueVarStore = (EFI_IFR_VARSTORE_NAME_VALUE *) IfrOpHdr; @@ 
-2092,10 +2100,11 @@ ParseIfrData (
   BlockData= NULL;
   DefaultDataPtr   = NULL;
   FirstOneOfOption = FALSE;
   VarStoreId   = 0;
   FirstOrderedList = FALSE;
+  VarStoreName = NULL;
   ZeroMem (&DefaultData, sizeof (IFR_DEFAULT_DATA));
 
   //
   // Go through the form package to parse OpCode one by one.
   //
@@ -2149,10 +2158,13 @@ ParseIfrData (
 CopyGuid (&VarStorageData->Guid, (EFI_GUID *) (VOID *) 
&IfrVarStore->Guid);
 VarStorageData->Size   = IfrVarStore->Size;
 VarStorageData->Name   = VarStoreName;
 VarStorageData->Type   = EFI_HII_VARSTORE_BUFFER;
 VarStoreId = IfrVarStore->VarStoreId;
+  } else {
+FreePool (VarStoreName);
+VarStoreName = NULL;
   }
   break;
 
 case EFI_IFR_VARSTORE_EFI_OP:
   //
@@ -2187,10 +2199,13 @@ ParseIfrData (
 CopyGuid (&VarStorageData->Guid, (EFI_GUID *) (VOID *) 
&IfrEfiVarStore->Guid);
 VarStorageData->Size   = IfrEfiVarStore->Size;
 VarStorageData->Name   = VarStoreName;
 VarStorageData->Type   = EFI_HII_VARSTORE_EFI_VARIABLE_BUFFER;
 VarStoreId = IfrEfiVarStore->VarStoreId;
+  } else {
+FreePool (VarStoreName);
+VarStoreName = NULL;
   }
   break;
 
 case EFI_IFR_VARSTORE_NAME_VALUE_OP:
   //
@@ -3012,11 +3027,11 @@ GetBlockElement (
   //
   Status = GetValueOfNumber (StringPtr, &TmpBuffer, &Length);
   if (EFI_ERROR (Status)) {
 goto Done;
   }
-
+  FreePool (TmpBuffer);
   StringPtr += Length;
   if (*StringPtr != 0 && *StringPtr != L'&') {
 goto Done;
   }
 }
@@ -3866,10 +3881,14 @@ Done:
 RemoveEntryList (&DefaultValueData->Entry);
 FreePool (DefaultValueData);
   }
   FreePool (BlockData);
 }
+if (VarStorageData ->Name != NULL) {
+  FreePool (VarStorageData ->Name);
+  VarStorageData ->Name = NULL;
+}
 FreePool (VarStorageData);
   }
 
   if (DefaultIdArray != NULL) {
 //
--
1.9.5.msysgit.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] NetworkPkg/HttpBootDxe: Fix for the issue that the HTTP boot option can't be booted more than once

[Sunny Wang]

This issue can be reproduced by the following steps:
1. Boot to HTTP boot option and the boot file is a ISO file like Ubuntu PE 
image.
2. Exit from boot option (GRUB) and then back to boot manager menu.
3. Boot to the same HTTP boot option again or a HTTP boot option pointing to 
the same HTTP ISO file. It will fail to boot.
Root cause:
When booting a HTTP boot option, the HTTP boot driver will save the Boot File's 
information in its private data as cache data for skipping the Boot file 
discovery from next time boot. However, the cache data doesn't include 
ImageType data, which would cause HTTP boot driver using the invalid ImageType 
(ImageTypeMax) and then fail to boot the cached boot file. In other words, for 
the second time boot, the HttpBootLoadFile() doesn't update ImageType (it 
returns a valid ImageType), which causes that the HttpBootDxeLoadFile() skips 
to Register a RAM Disk for downloaded HTTP ISO file and then BDS code can't 
find the RAM disk to boot.
Solution:
Save ImageType to private data for next time HTTP boot.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Sunny Wang 
---
 NetworkPkg/HttpBootDxe/HttpBootDxe.h  | 1 +
 NetworkPkg/HttpBootDxe/HttpBootImpl.c | 6 --
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.h 
b/NetworkPkg/HttpBootDxe/HttpBootDxe.h
index 76b7943..806429c 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootDxe.h
+++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.h
@@ -189,6 +189,7 @@ struct _HTTP_BOOT_PRIVATE_DATA {
   VOID  *BootFileUriParser;
   UINTN BootFileSize;
   BOOLEAN   NoGateway;
+  HTTP_BOOT_IMAGE_TYPE  ImageType;
 
   //
   // URI string extracted from the input FilePath parameter.
diff --git a/NetworkPkg/HttpBootDxe/HttpBootImpl.c 
b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
index cf643d8..4b850b6 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootImpl.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
@@ -2,6 +2,7 @@
   The implementation of EFI_LOAD_FILE_PROTOCOL for UEFI HTTP boot.
 
 Copyright (c) 2015 - 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 that accompanies this 
distribution.  
 The full text of the license may be found at
@@ -271,7 +272,7 @@ HttpBootLoadFile (
TRUE,
&Private->BootFileSize,
NULL,
-   ImageType
+   &Private->ImageType
);
 if (EFI_ERROR (Status) && Status != EFI_BUFFER_TOO_SMALL) {
   //
@@ -283,7 +284,7 @@ HttpBootLoadFile (
  FALSE,
  &Private->BootFileSize,
  NULL,
- ImageType
+ &Private->ImageType
  );
   if (EFI_ERROR (Status) && Status != EFI_BUFFER_TOO_SMALL) {
 return Status;
@@ -293,6 +294,7 @@ HttpBootLoadFile (
 
   if (*BufferSize < Private->BootFileSize) {
 *BufferSize = Private->BootFileSize;
+*ImageType = Private->ImageType;
 return EFI_BUFFER_TOO_SMALL;
   }
 
-- 
2.5.0.windows.1

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

Re: [edk2] [PATCH v2] NetworkPkg: Use UefiBootManagerLib API to create load option.

[Fu, Siyuan]

You are right, I will fix it.

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Wang, Sunny (HPS SW)
> Sent: Thursday, May 5, 2016 9:57 AM
> To: Fu, Siyuan ; edk2-devel@lists.01.org
> Cc: Ye, Ting ; Ni, Ruiyu ; Wu, Jiaxin
> 
> Subject: Re: [edk2] [PATCH v2] NetworkPkg: Use UefiBootManagerLib API to
> create load option.
> 
> Hi Siyuan,
> I just found a possible memory leak issue with the following code block.
> Others look good to me.
> +  Status = EfiBootManagerAddLoadOptionVariable (&NewOption, (UINTN) -
> 1);
> +  if (EFI_ERROR (Status)) {
> +EfiBootManagerFreeLoadOption (&NewOption);
> +   }
> 
> I think we always need to call EfiBootManagerFreeLoadOption no matter
> EfiBootManagerAddLoadOptionVariable returns error or not because the
> memory allocated for NewOption (description and device path) is no longer
> needed. We already set NewOption to variable, so the data in memory will
> not be used anymore. Therefore, we can modify the code block above to the
> following, what do you think?
> +  EfiBootManagerAddLoadOptionVariable (&NewOption, (UINTN) -1);
> +  EfiBootManagerFreeLoadOption (&NewOption);
> 
> By the way, thanks for addressing my comment in the other review.
> 
> Regards,
> Sunny Wang
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu
> Siyuan
> Sent: Wednesday, May 04, 2016 1:58 PM
> To: edk2-devel@lists.01.org
> Cc: Ye Ting ; Ni Ruiyu ; Wu Jiaxin
> 
> Subject: [edk2] [PATCH v2] NetworkPkg: Use UefiBootManagerLib API to
> create load option.
> 
> V2:
> Remove unnecessary ZeroMem and free load option.
> 
> This patch updates the HTTP boot driver to use the API in
> UefiBootManagerLib to create new load option, to avoid duplicate code.
> 
> Cc: Ye Ting 
> Cc: Wu Jiaxin 
> Cc: Ni Ruiyu 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan 
> ---
>  NetworkPkg/HttpBootDxe/HttpBootConfig.c | 136 
> ++--
>  NetworkPkg/HttpBootDxe/HttpBootDxe.inf  |   1 +
>  NetworkPkg/NetworkPkg.dsc   |   8 ++
>  3 files changed, 33 insertions(+), 112 deletions(-)
> 
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootConfig.c
> b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
> index 00e4782..2ca38b5 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootConfig.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
> @@ -13,6 +13,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  **/
> 
>  #include "HttpBootDxe.h"
> +#include 
> 
>  CHAR16  mHttpBootConfigStorageName[] =
> L"HTTP_BOOT_CONFIG_IFR_NVDATA";
> 
> @@ -36,31 +37,18 @@ HttpBootAddBootOption (
>IN   CHAR16   *Uri
>)
>  {
> -  EFI_DEV_PATH   *Node;
> -  EFI_DEVICE_PATH_PROTOCOL   *TmpDevicePath;
> -  EFI_DEVICE_PATH_PROTOCOL   *NewDevicePath;
> -  UINTN  Length;
> -  CHAR8  AsciiUri[URI_STR_MAX_SIZE];
> -  CHAR16 *CurrentOrder;
> -  EFI_STATUS Status;
> -  UINTN  OrderCount;
> -  UINTN  TargetLocation;
> -  BOOLEANFound;
> -  UINT8  *TempByteBuffer;
> -  UINT8  *TempByteStart;
> -  UINTN  DescSize;
> -  UINTN  FilePathSize;
> -  CHAR16 OptionStr[10];
> -  UINT16 *NewOrder;
> -  UINTN  Index;
> -
> -  NewOrder  = NULL;
> -  TempByteStart = NULL;
> +  EFI_DEV_PATH  *Node;
> +  EFI_DEVICE_PATH_PROTOCOL  *TmpDevicePath;
> +  EFI_DEVICE_PATH_PROTOCOL  *NewDevicePath;
> +  UINTN Length;
> +  CHAR8 AsciiUri[URI_STR_MAX_SIZE];
> +  EFI_STATUSStatus;
> +  UINTN Index;
> +  EFI_BOOT_MANAGER_LOAD_OPTION  NewOption;
> +
>NewDevicePath = NULL;
> -  NewOrder  = NULL;
>Node  = NULL;
>TmpDevicePath = NULL;
> -  CurrentOrder  = NULL;
> 
>if (StrLen (Description) == 0) {
>  return EFI_INVALID_PARAMETER;
> @@ -137,105 +125,29 @@ HttpBootAddBootOption (
>}
> 
>//
> -  // Get current "BootOrder" variable and find a free target.
> -  //
> -  Length = 0;
> -  Status = GetVariable2 (
> - L"BootOrder",
> - &gEfiGlobalVariableGuid,
> - (VOID **)&CurrentOrder,
> - &Length
> - );
> -  if (EFI_ERROR (Status) && Status != EFI_NOT_FOUND) {
> -goto ON_EXIT;
> -  }
> -  OrderCount = Length / sizeof (UINT16);
> -  Found = FALSE;
> -  for (TargetLocation=0; TargetLocation < 0x; TargetLocation++) {
> -Found = TRUE;
> -for (Index = 0; Index < OrderCount; Index++) {
> -  if (CurrentOrder[Index] == TargetLocation) {
> -Found = FALSE;
> -break;
> -  }
> -}
> -if (Found) {
> -  break;
> -  

[edk2] [Patch] NetworkPkg: Fix a memory leak in HTTP boot driver.

[Fu Siyuan]

We always need to call EfiBootManagerFreeLoadOption because the memory allocated
for NewOption (description and device path) is no longer needed.

Cc: Ye Ting 
Cc: Wu Jiaxin 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 NetworkPkg/HttpBootDxe/HttpBootConfig.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootConfig.c 
b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
index 2ca38b5..04c2f3e 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootConfig.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
@@ -142,9 +142,7 @@ HttpBootAddBootOption (
   }
 
   Status = EfiBootManagerAddLoadOptionVariable (&NewOption, (UINTN) -1);
-  if (EFI_ERROR (Status)) {
-EfiBootManagerFreeLoadOption (&NewOption);
-  }
+  EfiBootManagerFreeLoadOption (&NewOption);
 
 ON_EXIT:
 
-- 
2.7.4.windows.1

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

Re: [edk2] [PATCH] NetworkPkg/HttpBootDxe: Fix for the issue that the HTTP boot option can't be booted more than once

[Fu, Siyuan]

The patch is good to me.
Reviewed-by: Fu Siyuan 


> -Original Message-
> From: Sunny Wang [mailto:sunnyw...@hpe.com]
> Sent: Thursday, May 5, 2016 10:05 AM
> To: edk2-devel@lists.01.org
> Cc: el...@hpe.com; Fu, Siyuan ; Sunny Wang
> 
> Subject: [PATCH] NetworkPkg/HttpBootDxe: Fix for the issue that the HTTP
> boot option can't be booted more than once
> 
> This issue can be reproduced by the following steps:
> 1. Boot to HTTP boot option and the boot file is a ISO file like Ubuntu PE
> image.
> 2. Exit from boot option (GRUB) and then back to boot manager menu.
> 3. Boot to the same HTTP boot option again or a HTTP boot option pointing
> to the same HTTP ISO file. It will fail to boot.
> Root cause:
> When booting a HTTP boot option, the HTTP boot driver will save the Boot
> File's information in its private data as cache data for skipping the Boot 
> file
> discovery from next time boot. However, the cache data doesn't include
> ImageType data, which would cause HTTP boot driver using the invalid
> ImageType (ImageTypeMax) and then fail to boot the cached boot file. In
> other words, for the second time boot, the HttpBootLoadFile() doesn't update
> ImageType (it returns a valid ImageType), which causes that the
> HttpBootDxeLoadFile() skips to Register a RAM Disk for downloaded HTTP ISO
> file and then BDS code can't find the RAM disk to boot.
> Solution:
> Save ImageType to private data for next time HTTP boot.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Sunny Wang 
> ---
>  NetworkPkg/HttpBootDxe/HttpBootDxe.h  | 1 +
>  NetworkPkg/HttpBootDxe/HttpBootImpl.c | 6 --
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.h
> b/NetworkPkg/HttpBootDxe/HttpBootDxe.h
> index 76b7943..806429c 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootDxe.h
> +++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.h
> @@ -189,6 +189,7 @@ struct _HTTP_BOOT_PRIVATE_DATA {
>VOID  *BootFileUriParser;
>UINTN BootFileSize;
>BOOLEAN   NoGateway;
> +  HTTP_BOOT_IMAGE_TYPE  ImageType;
> 
>//
>// URI string extracted from the input FilePath parameter.
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootImpl.c
> b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
> index cf643d8..4b850b6 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootImpl.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
> @@ -2,6 +2,7 @@
>The implementation of EFI_LOAD_FILE_PROTOCOL for UEFI HTTP boot.
> 
>  Copyright (c) 2015 - 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 that accompanies this
> distribution.
>  The full text of the license may be found at
> @@ -271,7 +272,7 @@ HttpBootLoadFile (
> TRUE,
> &Private->BootFileSize,
> NULL,
> -   ImageType
> +   &Private->ImageType
> );
>  if (EFI_ERROR (Status) && Status != EFI_BUFFER_TOO_SMALL) {
>//
> @@ -283,7 +284,7 @@ HttpBootLoadFile (
>   FALSE,
>   &Private->BootFileSize,
>   NULL,
> - ImageType
> + &Private->ImageType
>   );
>if (EFI_ERROR (Status) && Status != EFI_BUFFER_TOO_SMALL) {
>  return Status;
> @@ -293,6 +294,7 @@ HttpBootLoadFile (
> 
>if (*BufferSize < Private->BootFileSize) {
>  *BufferSize = Private->BootFileSize;
> +*ImageType = Private->ImageType;
>  return EFI_BUFFER_TOO_SMALL;
>}
> 
> --
> 2.5.0.windows.1

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

Re: [edk2] [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in HttpGenRequestMessage API

[Wu, Jiaxin]

Hi  Nagaraj,

Constructing header parts seems should be outside the conditional of "if 
(Message->Data.Request != NULL)".

if (HttpHdr != NULL) {
  //
  // Construct header
  //
  CopyMem (RequestPtr, HttpHdr, HttpHdrSize);
  RequestPtr += HttpHdrSize;
}

Others is good to me.

Reviewed-By: Wu Jiaxin 

Thanks.
Jiaxin

> -Original Message-
> From: Nagaraj Hegde [mailto:nagaraj-p.he...@hpe.com]
> Sent: Wednesday, May 4, 2016 5:33 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin ; Fu, Siyuan ; Ye,
> Ting ; samer.el-haj-mahm...@hpe.com
> Subject: [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in
> HttpGenRequestMessage API
> 
> HttpGenRequestMessage assumes that HTTP message would always contain
> a request-line, headers and an optional message body.
> However, subsequent to a HTTP PUT/POST request, HTTP requests would
> contain just the message body. This patch supports creation of such request
> messages with additional checks.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hegde, Nagaraj P nagaraj-p.he...@hpe.com
> ---
>  MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 219 +++---
> --
>  1 file changed, 124 insertions(+), 95 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> index 8d61d0b..fa0f591 100644
> --- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> +++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> @@ -1681,61 +1681,86 @@ HttpGenRequestMessage (
>HttpUtilitiesProtocol = NULL;
> 
>//
> -  // Locate the HTTP_UTILITIES protocol.
> +  // If we have a Request, we cannot have a NULL Url
>//
> -  Status = gBS->LocateProtocol (
> -  &gEfiHttpUtilitiesProtocolGuid,
> -  NULL,
> -  (VOID **)&HttpUtilitiesProtocol
> -  );
> -
> -  if (EFI_ERROR (Status)) {
> -DEBUG ((DEBUG_ERROR,"Failed to locate Http Utilities protocol. Status
> = %r.\n", Status));
> -return Status;
> +  if (Message->Data.Request != NULL && Url == NULL) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (Message->HeaderCount != 0) {
> +//
> +// Locate the HTTP_UTILITIES protocol.
> +//
> +Status = gBS->LocateProtocol (
> +&gEfiHttpUtilitiesProtocolGuid,
> +NULL,
> +(VOID **)&HttpUtilitiesProtocol
> +);
> +
> +if (EFI_ERROR (Status)) {
> +  DEBUG ((DEBUG_ERROR,"Failed to locate Http Utilities protocol. Status
> = %r.\n", Status));
> +  return Status;
> +}
> +
> +//
> +// Build AppendList to send into HttpUtilitiesBuild
> +//
> +AppendList = AllocateZeroPool (sizeof (EFI_HTTP_HEADER *) * (Message-
> >HeaderCount));
> +if (AppendList == NULL) {
> +  return EFI_OUT_OF_RESOURCES;
> +}
> +
> +for(Index = 0; Index < Message->HeaderCount; Index++){
> +  AppendList[Index] = &Message->Headers[Index];
> +}
> +
> +//
> +// Build raw HTTP Headers
> +//
> +Status = HttpUtilitiesProtocol->Build (
> +HttpUtilitiesProtocol,
> +0,
> +NULL,
> +0,
> +NULL,
> +Message->HeaderCount,
> +AppendList,
> +&HttpHdrSize,
> +&HttpHdr
> +);
> +
> +if (AppendList != NULL) {
> +  FreePool (AppendList);
> +}
> +
> +if (EFI_ERROR (Status) || HttpHdr == NULL){
> +  return Status;
> +}
>}
> 
>//
> -  // Build AppendList to send into HttpUtilitiesBuild
> +  // If we have headers to be sent, account for it.
>//
> -  AppendList = AllocateZeroPool (sizeof (EFI_HTTP_HEADER *) * (Message-
> >HeaderCount));
> -  if (AppendList == NULL) {
> -return EFI_OUT_OF_RESOURCES;
> -  }
> -
> -  for(Index = 0; Index < Message->HeaderCount; Index++){
> -AppendList[Index] = &Message->Headers[Index];
> +  if (Message->HeaderCount != 0) {
> +MsgSize = HttpHdrSize;
>}
> 
>//
> -  // Build raw HTTP Headers
> +  // If we have a request line, account for the fields.
>//
> -  Status = HttpUtilitiesProtocol->Build (
> -  HttpUtilitiesProtocol,
> -  0,
> -  NULL,
> -  0,
> -  NULL,
> -  Message->HeaderCount,
> -  AppendList,
> -  &HttpHdrSize,
> -  &HttpHdr
> -  );
> -
> -  if (AppendList != NULL) {
> -FreePool (AppendList);
> -  }
> -
> -  if (EFI_ERROR (Status) || HttpHdr == NULL){
> -return Status;
> +  if (Message->Data.Request != NULL) {
> +MsgSize += HTTP_METHOD_MAXIMUM_LEN + AsciiStrLen
> + (HTTP_VERSION_CRLF_STR) + AsciiStrLen (Url);
>}
> 
> +
>//
> -  // Calculate HTTP message length.
> +  // If we have a message body to be sent, account for it.
>//
> -  MsgSize = Message->BodyLength + HTTP_METHOD_MAXIMUM_LEN +
> AsciiStrLen (U

Re: [edk2] [Patch 0/7] Enhance BlockSid releated logic.

[Tian, Feng]

Two minor comments:
1. please use a new macro to describe the ComId used for BlockSID.
2. It looks the patch 7/7's comment is not very matched with the code. The code 
always does BlockSid operation no matter if it's in unlock process.

Others look good to me. 

Reviewed-by: Feng Tian 

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Eric Dong
Sent: Wednesday, May 4, 2016 1:33 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [Patch 0/7] Enhance BlockSid releated logic.

Enhance the BlockSid related logic.

Eric Dong (7):
  SecurityPkg TcgStorageOpalLib: Update ComId for Block SID command.
  MdePkg: Add TCG_BLOCK_SID_FEATURE_DESCRIPTOR definition.
  SecurityPkg TcgStorageOpalLib: Check BlockSid capability.
  SecurityPkg OpalPasswordDxe: Change BlockSid position.
  SecurityPkg OpalPasswordDxe: Check BlockSid capability before send
command.
  SecurityPkg OpalPasswordSmm: Enhance BlockSid Logic.
  SecurityPkg OpalPasswordSmm: Move BlockSid out of unlock process.

 MdePkg/Include/IndustryStandard/TcgStorageCore.h   | 12 ++
 MdePkg/Include/IndustryStandard/TcgStorageOpal.h   |  1 +
 SecurityPkg/Include/Library/TcgStorageOpalLib.h|  6 +
 .../Library/TcgStorageOpalLib/TcgStorageOpalCore.c |  8 ++-  
SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c  | 25 +---
 SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.c |  9 
 .../Tcg/Opal/OpalPasswordDxe/OpalHiiFormValues.h   |  1 -
 .../Tcg/Opal/OpalPasswordDxe/OpalPasswordForm.vfr  | 22 +-
 .../Tcg/Opal/OpalPasswordSmm/OpalPasswordSmm.c | 27 +-
 9 files changed, 69 insertions(+), 42 deletions(-)

--
2.6.4.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] MdeModulePkg/FileExplore: Make LibraryClass & Depex module type consistent

[Dandan Bi]

Cc: Liming Gao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf 
b/MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
index a97fd4a..5cf9398 100644
--- a/MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
+++ b/MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
@@ -57,7 +57,7 @@ [Protocols]
   gEfiLoadFileProtocolGuid  ## CONSUMES
   gEfiHiiConfigAccessProtocolGuid   ## CONSUMES
   gEfiFormBrowser2ProtocolGuid  ## CONSUMES
   gEfiDevicePathToTextProtocolGuid  ## CONSUMES
 
-[Depex.common.DXE_DRIVER, Depex.common.DXE_RUNTIME_DRIVER, 
Depex.common.DXE_SAL_DRIVER, Depex.common.DXE_SMM_DRIVER]
+[Depex.common.DXE_DRIVER]
   gEfiFormBrowser2ProtocolGuid AND gEfiHiiDatabaseProtocolGuid
\ No newline at end of file
-- 
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 v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in HttpGenRequestMessage API

[Hegde, Nagaraj P]

Hi Jiaxin,

I believe in HTTP 1.1, servers do not accept Request methods without any 
header. In other words, "Host:" header is a must for all request methods. That 
was the reason I planned to put the construction of header parts inside the "if 
(Message->Data.Request != NULL" condition. Now, I wonder if we need to add this 
condition in the entry of this function itself and return a 
EFI_INVALID_PARAMETER or allow it to go to HTTP server and return back as "400 
Bad Request". In case we allow, I see an issue. We would miss sending the 
additional "\r\n" which would mark the end of request and headers (if any). 
Adding additional "\r\n" is embedded into HttpUtilities Build function.

Regards,
Nagaraj.

-Original Message-
From: Wu, Jiaxin [mailto:jiaxin...@intel.com] 
Sent: Thursday, May 05, 2016 8:10 AM
To: Hegde, Nagaraj P ; edk2-devel@lists.01.org
Cc: Fu, Siyuan ; Ye, Ting ; 
El-Haj-Mahmoud, Samer 
Subject: RE: [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in 
HttpGenRequestMessage API

Hi  Nagaraj,

Constructing header parts seems should be outside the conditional of "if 
(Message->Data.Request != NULL)".

if (HttpHdr != NULL) {
  //
  // Construct header
  //
  CopyMem (RequestPtr, HttpHdr, HttpHdrSize);
  RequestPtr += HttpHdrSize;
}

Others is good to me.

Reviewed-By: Wu Jiaxin 

Thanks.
Jiaxin

> -Original Message-
> From: Nagaraj Hegde [mailto:nagaraj-p.he...@hpe.com]
> Sent: Wednesday, May 4, 2016 5:33 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin ; Fu, Siyuan 
> ; Ye, Ting ; 
> samer.el-haj-mahm...@hpe.com
> Subject: [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in 
> HttpGenRequestMessage API
> 
> HttpGenRequestMessage assumes that HTTP message would always contain a 
> request-line, headers and an optional message body.
> However, subsequent to a HTTP PUT/POST request, HTTP requests would 
> contain just the message body. This patch supports creation of such 
> request messages with additional checks.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hegde, Nagaraj P nagaraj-p.he...@hpe.com
> ---
>  MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 219 +++---
> --
>  1 file changed, 124 insertions(+), 95 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> index 8d61d0b..fa0f591 100644
> --- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> +++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> @@ -1681,61 +1681,86 @@ HttpGenRequestMessage (
>HttpUtilitiesProtocol = NULL;
> 
>//
> -  // Locate the HTTP_UTILITIES protocol.
> +  // If we have a Request, we cannot have a NULL Url
>//
> -  Status = gBS->LocateProtocol (
> -  &gEfiHttpUtilitiesProtocolGuid,
> -  NULL,
> -  (VOID **)&HttpUtilitiesProtocol
> -  );
> -
> -  if (EFI_ERROR (Status)) {
> -DEBUG ((DEBUG_ERROR,"Failed to locate Http Utilities protocol. Status
> = %r.\n", Status));
> -return Status;
> +  if (Message->Data.Request != NULL && Url == NULL) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (Message->HeaderCount != 0) {
> +//
> +// Locate the HTTP_UTILITIES protocol.
> +//
> +Status = gBS->LocateProtocol (
> +&gEfiHttpUtilitiesProtocolGuid,
> +NULL,
> +(VOID **)&HttpUtilitiesProtocol
> +);
> +
> +if (EFI_ERROR (Status)) {
> +  DEBUG ((DEBUG_ERROR,"Failed to locate Http Utilities protocol. 
> + Status
> = %r.\n", Status));
> +  return Status;
> +}
> +
> +//
> +// Build AppendList to send into HttpUtilitiesBuild
> +//
> +AppendList = AllocateZeroPool (sizeof (EFI_HTTP_HEADER *) * 
> + (Message-
> >HeaderCount));
> +if (AppendList == NULL) {
> +  return EFI_OUT_OF_RESOURCES;
> +}
> +
> +for(Index = 0; Index < Message->HeaderCount; Index++){
> +  AppendList[Index] = &Message->Headers[Index];
> +}
> +
> +//
> +// Build raw HTTP Headers
> +//
> +Status = HttpUtilitiesProtocol->Build (
> +HttpUtilitiesProtocol,
> +0,
> +NULL,
> +0,
> +NULL,
> +Message->HeaderCount,
> +AppendList,
> +&HttpHdrSize,
> +&HttpHdr
> +);
> +
> +if (AppendList != NULL) {
> +  FreePool (AppendList);
> +}
> +
> +if (EFI_ERROR (Status) || HttpHdr == NULL){
> +  return Status;
> +}
>}
> 
>//
> -  // Build AppendList to send into HttpUtilitiesBuild
> +  // If we have headers to be sent, account for it.
>//
> -  AppendList = AllocateZeroPool (sizeof (EFI_HTTP_HEADER *) * 
> (Message-
> >HeaderCount));
> -  if (AppendList == NULL) {
> -return EFI_OUT_OF_RESOURCES;
> -  }
> -
> -  for(Index = 0; Index < Message->HeaderCount; Index++){
> -AppendList[

Re: [edk2] [PATCH] NetworkPkg: Discard TCP segment when no TCB found

[Michael Chang]

On Thu, May 05, 2016 at 01:05:25AM +, Fu, Siyuan wrote:
> Hi, Michael
> 
> Sending this RST is required by TCP RFC793 as below:
> 
> http://tools.ietf.org/html/rfc793#section-3.9
> 
> SEGMENT ARRIVES
> 
> If the state is CLOSED (i.e., TCB does not exist) then
> 
>   all data in the incoming segment is discarded.  An incoming
>   segment containing a RST is discarded.  *An incoming segment not
>   containing a RST causes a RST to be sent in response*.  The
>   acknowledgment and sequence field values are selected to make the
>   reset sequence acceptable to the TCP that sent the offending
>   segment.

That's fair enough. Now that I understood the TCP driver behaves correctly by
following this well defined standard. Thanks.

> 
> For the case that "other running Managed Network instance with it's own 
> network stack", I think it's not TCP layer's responsibility, instead, it 
> should be handled by SNP or MNP driver. They shouldn't deliver those network 
> packets which are not owned by the network stack about it.

I think this a good point. And that's what a user (at least I am) would expect
generally when they have created instances (child handle) in their efi
application to work on EFI Managed Network Protcol interfaces on its own. It's
not only for sharing the access to SNP but also should provide isolation for
packets per instances and applying filter settings. I think also one more thing
to consider is security, it makes sniffing network activity from one
application way too easy,

Could we consider putting the effort for making isolation in MNP (at least for
this case ..) and make it obvious in the spec ?

> If other network stack want to take over the device from edk2 network stack 
> drivers, it had better to disconnect the edk2 network stack first, to avoid 
> such races between the two network stacks.

That sounds like the same purpose to move to use SNP with exclusive open, which
is the opposite for why I started looking into MNP. We are back to squre one.

Acutally I was motivated to use MNP to replace SNP (with exclusive open) in
grub2 because people advertising it better solution to integrate and co-operate
with firmware network stack, you don't have to worry about side effects from
killing off all running network instances and doing all setting from scatch.

And the most important idea is I would like to see a chance that both grub2's
native and firmware's network stack could coexist to enable the possibility to
use protocols only provided by firmware (like the https is on the road) while
not stopping the other from working.

Thanks,
Michael

> 
> Best Regards
> Siyuan
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Michael Chang
> > Sent: Wednesday, May 4, 2016 6:23 PM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] [PATCH] NetworkPkg: Discard TCP segment when no TCB
> > found
> > 
> > The TCP driver should discard(ignore) the incoming TCP segment when there's
> > no
> > TCB found to handle it, because that TCP segment may be subjected to be
> > received by other running Managed Network instance with it's own network
> > stack
> > to handle it. Force sending reset segment in this case would just discrupt 
> > any
> > TCP connection except the one established by EFI TCP protocol, which is not
> > make sense imho.
> > 
> > Thanks.
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Michael Chang 
> > 
> > ---
> >  NetworkPkg/TcpDxe/TcpInput.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/NetworkPkg/TcpDxe/TcpInput.c b/NetworkPkg/TcpDxe/TcpInput.c
> > index 745ee4c..7aa6f7e 100644
> > --- a/NetworkPkg/TcpDxe/TcpInput.c
> > +++ b/NetworkPkg/TcpDxe/TcpInput.c
> > @@ -794,10 +794,10 @@ TcpInput (
> >);
> > 
> >if ((Tcb == NULL) || (Tcb->State == TCP_CLOSED)) {
> > -DEBUG ((EFI_D_INFO, "TcpInput: send reset because no TCB found\n"));
> > +DEBUG ((EFI_D_INFO, "TcpInput: discard a segment because no TCB
> > found\n"));
> > 
> >  Tcb = NULL;
> > -goto SEND_RESET;
> > +goto DISCARD;
> >}
> > 
> >Seg = TcpFormatNetbuf (Tcb, Nbuf);
> > --
> > 2.6.6
> > 
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 2/3] IntelFrameworkModulePkg/BdsDxe: Fix for the BDS boot timeout

[Ni, Ruiyu]

Regards,
Ray

>-Original Message-
>From: Daniil Egranov [mailto:daniil.egra...@arm.com]
>Sent: Thursday, May 5, 2016 7:57 AM
>To: Ni, Ruiyu ; edk2-devel@lists.01.org
>Cc: Fan, Jeff 
>Subject: Re: [edk2] [PATCH v2 2/3] IntelFrameworkModulePkg/BdsDxe: Fix for the 
>BDS boot timeout
>
>Hi Ray,
>
>Thank you for review and comments. Please see my answers below.
>
>Regards,
>Daniil
>
>On 05/04/2016 12:04 AM, Ni, Ruiyu wrote:
>> 3 comments below.
>>
>> Regards,
>> Ray
>>
>>> -Original Message-
>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>>> Daniil Egranov
>>> Sent: Wednesday, May 4, 2016 9:34 AM
>>> To: edk2-devel@lists.01.org
>>> Cc: Fan, Jeff 
>>> Subject: [edk2] [PATCH v2 2/3] IntelFrameworkModulePkg/BdsDxe: Fix for the 
>>> BDS boot timeout
>>>
>>> The patch loads timeout value from the "Timeout" global variable and passes
>>> it to PlatformBdsEnterFrontPage(), which handles delay and key input.
>>> The PcdPlatformBootTimeOut is only used at the BDS entry point and updates
>>> the "Timeout" value. This will allow the modification of the timeout value
>>> through the BDS menu and overwrite it if PcdPlatformBootTimeOut has been
>>> set.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Daniil Egranov 
>>> ---
>>> IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c| 18 
>>> +++---
>>> .../Universal/BdsDxe/BootMaint/BootMaint.c | 17 
>>> -
>>> .../Universal/BdsDxe/BootMaint/UpdatePage.c| 13 +++--
>>> 3 files changed, 38 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
>>> b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
>>> index ae7ad21..1d80fca 100644
>>> --- a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
>>> +++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
>>> @@ -123,6 +123,7 @@ BdsBootDeviceSelect (
>>>BOOLEAN   BootNextExist;
>>>LIST_ENTRY*LinkBootNext;
>>>EFI_EVENT ConnectConInEvent;
>>> +  UINTN Size;
>>>
>>>//
>>>// Got the latest boot option
>>> @@ -214,6 +215,18 @@ BdsBootDeviceSelect (
>>>if (Link == NULL) {
>>>  return ;
>>>}
>>> +
>>> +  //Read boot timeout variable
>>> +  Status = gRT->GetVariable (L"Timeout",
>>> + &gEfiGlobalVariableGuid,
>>> + NULL,
>>> + &Size,
>>> + (VOID *) &Timeout);
>>> +
>>> +  if (EFI_ERROR (Status)) {
>>> +Timeout = 0x;
>>> +  }
>>> +
>>>//
>>>// Here we make the boot in a loop, every boot success will
>>>// return to the front page
>>> @@ -222,7 +235,7 @@ BdsBootDeviceSelect (
>>>  //
>>>  // Check the boot option list first
>>>  //
>>> -if (Link == &BootLists) {
>>> +if (Link == &BootLists || Timeout != 0x) {
>>>//
>>>// When LazyConIn enabled, signal connect ConIn event before enter UI
>>>//
>>> @@ -238,12 +251,11 @@ BdsBootDeviceSelect (
>>>//one is success to boot, then we back here to allow user
>>>//add new active boot option
>>>//
>>> -  Timeout = 0x;
>>>PlatformBdsEnterFrontPage (Timeout, FALSE);
>>> +  Timeout = 0x;
>>
>> 1. The old code is to enter front page unconditionally and immediately while 
>> you
>> changed the behavior to wait for Timeout seconds before entering front page.
>> Why?
>The previous logic allows entry into the BDS boot menu in two cases: no
>active boot options and all attempts to boot from the current boot
>options fail. If there is a valid bootable device in the boot options, i
>found no way to interrupt the boot process. In a lot of cases, the boot
>options have to be changed before any boot attempts have been made. This
>patch checks if boot timeout is set and uses PlatformBdsEnterFrontPage
>to manage time and key inputs before using current boot options.

You could refer to the OvmfPkg/Library/PlatformBdsLib.
PlatformBdsEnterFrontPage() is called by PlatformBdsPolicyBehavior()
with the timeout.

>>
>>
>>>InitializeListHead (&BootLists);
>>>BdsLibBuildOptionFromVar (&BootLists, L"BootOrder");
>>>Link = BootLists.ForwardLink;
>>> -  continue;
>>>  }
>>>  //
>>>  // Get the boot option from the link list
>>> diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c
>>> b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c
>>> index d4b4475..cc2d656 100644
>>> --- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c
>>> +++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c
>>> @@ -111,6 +111,9 @@ InitializeBmmConfig (
>>>BM_MENU_ENTRY   *NewMenuEntry;
>>>BM_LOAD_CONTEXT *NewLoadContext;
>>>UINT16  Index;
>>> +  UINT16  BootTimeOut;
>>> +  EFI_STATUS  Status;
>>> +  UINTN   Size;
>>>
>>> 

Re: [edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot timeout message

[Ni, Ruiyu]

Regards,
Ray

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Daniil 
>Egranov
>Sent: Thursday, May 5, 2016 8:07 AM
>To: Ni, Ruiyu ; edk2-devel@lists.01.org
>Cc: Fan, Jeff 
>Subject: Re: [edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot 
>timeout message
>
>Hi Ray,
>
>Thanks for the review. My answers below.
>
>Thanks,
>Daniil
>
>On 05/04/2016 12:07 AM, Ni, Ruiyu wrote:
>> 2 comments below.
>>
>> Regards,
>> Ray
>>
>>> -Original Message-
>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>>> Daniil Egranov
>>> Sent: Wednesday, May 4, 2016 9:34 AM
>>> To: edk2-devel@lists.01.org
>>> Cc: Fan, Jeff 
>>> Subject: [edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot 
>>> timeout message
>>>
>>> The PlatformBdsShowProgress() supports graphics mode only, which is not
>>> applicable for RS-232 serial console. Show the progress message as a console
>>> text message in case PlatformBdsShowProgress() fails.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Daniil Egranov 
>>> ---
>>> IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c | 9 -
>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>>> b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>>> index 6958979..d1a5c05 100644
>>> --- a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>>> +++ b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>>> @@ -925,7 +925,7 @@ ShowProgress (
>>>  // Show progress
>>>  //
>>>  if (TmpStr != NULL) {
>>> -  PlatformBdsShowProgress (
>>> +  Status = PlatformBdsShowProgress (
>>>  Foreground,
>>>  Background,
>>>  TmpStr,
>>> @@ -933,12 +933,19 @@ ShowProgress (
>>>  ((TimeoutDefault - TimeoutRemain) * 100 / TimeoutDefault),
>>>  0
>>>  );
>>> +  if (EFI_ERROR(Status)) {
>>> +//if graphics mode is not supported (serial console) show text 
>>> progress message
>>> +AsciiPrint ("\rPress any key to enter Boot Menu in %d seconds  
>>>", TimeoutRemain);
>>> +  }
>> 1. Why use AsciiPrint but not Print(L"")?
>> I agree they are the same but normally we use Print().
>>
>
>I was not sure which one to use. I'll correct it.
>
Thanks!


>>>  }
>>>}
>>>  }
>>>
>>>  if (TmpStr != NULL) {
>>>gBS->FreePool (TmpStr);
>>> +if (EFI_ERROR(Status)) {
>>> +  AsciiPrint ("\n");
>>> +}
>> 2. What's the purpose of the EOL here?
>>
>
>The AsciiPrint above uses cartridge return without a new line so this
>EOL preserves last message from being erased by other console outputs.
>
I see. Thanks! I agree!

>>>  }
>>>
>>>  //
>>> --
>>> 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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Two PCI IO protocols getting produced by same GUID, how to open correct one?

[Shaveta Leekha]

Thanks Mike for the response!

Hi All,

Few doubts in SATA stack. 

I have seen at some places that in UEFI/EDK2, The complete ATA stack should be 
like:

IdeControllerDxe + 
AtaAtapiPassThru+ 
AtaBus  + 
ScsiBus + 
ScsiDisk+ 
PartitionDriver + 
Fat

What is the role of "ScsiBus + ScsiDisk" in this stack?
Complete AHCI specific code is there in "AtaAtapiPassThru"? is tha correct?
Then why ScSi bus layer is required?

Thanks and Regards,
Shaveta


-Original Message-
From: Kinney, Michael D [mailto:michael.d.kin...@intel.com] 
Sent: Wednesday, May 04, 2016 10:58 PM
To: Shaveta Leekha ; Laszlo Ersek ; 
Kinney, Michael D 
Cc: edk2-devel@lists.01.org 
Subject: RE: [edk2] Two PCI IO protocols getting produced by same GUID, how to 
open correct one?

Shaveta,

All UEFI drivers (not just ones for PCI controllers) are responsible for 
evaluating the ControllerHandle in their EFI Driver Binding Supported() and 
Start() APIs to make sure the ControllerHandle is the type the driver knows how 
to manage.  If there is more than one driver that can correctly manage the same 
ControllerHandle, then there are precedence rules for the order that each 
driver's Supported() and Stop() APIs are called.

This is described in the UEFP Spec and the UEFI Driver Writer's Guide.  See 
UEFI DWG Section 3.14.1 for precedence rules.  See UEFI DWG Section 18.3 for 
details on what a PCI Driver is recommended to do in their Supported() and 
Start() functions.

https://github.com/tianocore/tianocore.github.io/wiki/UEFI-Driver-Writer's-Guide

Mike


> -Original Message-
> From: Shaveta Leekha [mailto:shaveta.lee...@nxp.com]
> Sent: Wednesday, May 4, 2016 12:23 AM
> To: Kinney, Michael D ; Laszlo Ersek 
> 
> Cc: edk2-devel@lists.01.org 
> Subject: RE: [edk2] Two PCI IO protocols getting produced by same 
> GUID, how to open correct one?
> 
> Thanks for the reply
> My replies are in-lined.
> 
> Thanks and Regards,
> Shaveta
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Kinney, Michael D
> Sent: Tuesday, May 03, 2016 9:29 PM
> To: Shaveta Leekha ; Laszlo Ersek 
> ; Kinney, Michael D 
> Cc: edk2-devel@lists.01.org 
> Subject: Re: [edk2] Two PCI IO protocols getting produced by same 
> GUID, how to open correct one?
> 
> Shaveta,
> 
> As long as all PCI I/O Protocol instances in the platform conform to 
> UEFI spec, it can work.
> 
> [Shaveta] Correct.
> 
> Your original question was about how to tell that a handle has the PCI 
> I/O Protocol produced By PCI Bus Driver or your emulation of PCI.  If 
> the PCI I/O Protocols are conformant, then It should not matter.  Just 
> write a PCI Device Driver that manages a PCI controller based on PCI 
> Class code or Vendor ID/Device ID matching and it should work.
> 
> [Shaveta] Yes, eventually correct the PCI I/O Protocol produced my PCI 
> emulation layer code is getting opened.
> As and when a valid protocol gets opened, I am checking the CLASS code 
> and Sub-CLASS code before proceeding.
> And that's working.
> 
> But what if, some other device's driver in my system, intended to open 
> "PCI I/O Protocol produced By PCI Bus Driver"
> Doesn't test CLASS CODE or any SUB-CLASS/VENDOR etc., then it may land 
> up using API's provided with protocol Of my PCI emulation layer code? 
> is that understanding correct ??
> 
> So in such scenarios, where more than one protocol instances (with 
> different APIs
> associated) are installed with same GUID in the system, Then all the 
> drivers should check VENDOR ID/CLASS etc immediately after opening the 
> protocol?
> 
> 
> 
> Mike
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> > Of Shaveta Leekha
> > Sent: Tuesday, May 3, 2016 3:27 AM
> > To: Laszlo Ersek 
> > Cc: edk2-devel@lists.01.org 
> > Subject: Re: [edk2] Two PCI IO protocols getting produced by same 
> > GUID, how to open correct one?
> >
> >
> >
> > -Original Message-
> > From: Laszlo Ersek [mailto:ler...@redhat.com]
> > Sent: Tuesday, May 03, 2016 1:15 PM
> > To: Shaveta Leekha 
> > Cc: edk2-devel@lists.01.org 
> > Subject: Re: [edk2] Two PCI IO protocols getting produced by same 
> > GUID, how to open correct one?
> >
> > On 05/03/16 08:54, Shaveta Leekha wrote:
> > > Hi,
> > >
> > > I have a scenario where two separate drivers are installing/producing 
> > > "PCI IO"
> > protocol with same GUID (gEfiPciIoProtocolGuid).
> > >
> > > Where:
> > >
> > > (1)One of the PCI Io protocol instance is produced by
> > "MdeModulePkg/Bus/Pci/PciBus" driver for real PCI devices to use( 
> > like E1000/NIC card to use)
> > >
> > > (2)Other PCI Io protocol instance is produced by "Pci Emulation" 
> > > layer created
> > for USB and SATA kind of controllers. This protocol (APIs) is 
> > intended to be used by USB and SATA con

Re: [edk2] [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in HttpGenRequestMessage API

[Wu, Jiaxin]

Yeah, according UEFI Spec, in my understanding, only two extra cases are 
allowed:
A. Body is not NULL and BodyLength is non-zero and all other fields are NULL or 
0 (POST and PUT methods, depending on the size of the data)
B. Body is NULL and BodyLength is 0 and all other fields are not NULL or 
non-zero (Not POST or PUT method)

If so, keep this condition inside is ok, how about we update the below piece 
code:  
  "if (Message->Data.Request != NULL && Url == NULL) {
return EFI_INVALID_PARAMETER;
  }" 
To:
  "if ((Message->Data.Request != NULL && Url == NULL) || (Message->Data.Request 
!= NULL && Message->HeaderCount == 0)) {
return EFI_INVALID_PARAMETER;
  }"

Thanks.
Jiaxin

> -Original Message-
> From: Hegde, Nagaraj P [mailto:nagaraj-p.he...@hpe.com]
> Sent: Thursday, May 5, 2016 11:48 AM
> To: Wu, Jiaxin ; edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Ye, Ting ; El-Haj-
> Mahmoud, Samer 
> Subject: RE: [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in
> HttpGenRequestMessage API
> 
> Hi Jiaxin,
> 
> I believe in HTTP 1.1, servers do not accept Request methods without any
> header. In other words, "Host:" header is a must for all request methods.
> That was the reason I planned to put the construction of header parts inside
> the "if (Message->Data.Request != NULL" condition. Now, I wonder if we
> need to add this condition in the entry of this function itself and return a
> EFI_INVALID_PARAMETER or allow it to go to HTTP server and return back as
> "400 Bad Request". In case we allow, I see an issue. We would miss sending
> the additional "\r\n" which would mark the end of request and headers (if
> any). Adding additional "\r\n" is embedded into HttpUtilities Build function.
> 
> Regards,
> Nagaraj.
> 
> -Original Message-
> From: Wu, Jiaxin [mailto:jiaxin...@intel.com]
> Sent: Thursday, May 05, 2016 8:10 AM
> To: Hegde, Nagaraj P ; edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Ye, Ting ; El-Haj-
> Mahmoud, Samer 
> Subject: RE: [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in
> HttpGenRequestMessage API
> 
> Hi  Nagaraj,
> 
> Constructing header parts seems should be outside the conditional of "if
> (Message->Data.Request != NULL)".
> 
> if (HttpHdr != NULL) {
>   //
>   // Construct header
>   //
>   CopyMem (RequestPtr, HttpHdr, HttpHdrSize);
>   RequestPtr += HttpHdrSize;
> }
> 
> Others is good to me.
> 
> Reviewed-By: Wu Jiaxin 
> 
> Thanks.
> Jiaxin
> 
> > -Original Message-
> > From: Nagaraj Hegde [mailto:nagaraj-p.he...@hpe.com]
> > Sent: Wednesday, May 4, 2016 5:33 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Jiaxin ; Fu, Siyuan
> > ; Ye, Ting ;
> > samer.el-haj-mahm...@hpe.com
> > Subject: [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in
> > HttpGenRequestMessage API
> >
> > HttpGenRequestMessage assumes that HTTP message would always
> contain a
> > request-line, headers and an optional message body.
> > However, subsequent to a HTTP PUT/POST request, HTTP requests would
> > contain just the message body. This patch supports creation of such
> > request messages with additional checks.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Hegde, Nagaraj P nagaraj-p.he...@hpe.com
> > ---
> >  MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 219 +++-
> --
> > --
> >  1 file changed, 124 insertions(+), 95 deletions(-)
> >
> > diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> > b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> > index 8d61d0b..fa0f591 100644
> > --- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> > +++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> > @@ -1681,61 +1681,86 @@ HttpGenRequestMessage (
> >HttpUtilitiesProtocol = NULL;
> >
> >//
> > -  // Locate the HTTP_UTILITIES protocol.
> > +  // If we have a Request, we cannot have a NULL Url
> >//
> > -  Status = gBS->LocateProtocol (
> > -  &gEfiHttpUtilitiesProtocolGuid,
> > -  NULL,
> > -  (VOID **)&HttpUtilitiesProtocol
> > -  );
> > -
> > -  if (EFI_ERROR (Status)) {
> > -DEBUG ((DEBUG_ERROR,"Failed to locate Http Utilities protocol. Status
> > = %r.\n", Status));
> > -return Status;
> > +  if (Message->Data.Request != NULL && Url == NULL) {
> > +return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  if (Message->HeaderCount != 0) {
> > +//
> > +// Locate the HTTP_UTILITIES protocol.
> > +//
> > +Status = gBS->LocateProtocol (
> > +&gEfiHttpUtilitiesProtocolGuid,
> > +NULL,
> > +(VOID **)&HttpUtilitiesProtocol
> > +);
> > +
> > +if (EFI_ERROR (Status)) {
> > +  DEBUG ((DEBUG_ERROR,"Failed to locate Http Utilities protocol.
> > + Status
> > = %r.\n", Status));
> > +  return Status;
> > +}
> > +
> > +//
> > +// Build AppendList to send into HttpUtilitiesBuild
> > +//
> > +AppendList = AllocateZ

Re: [edk2] [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in HttpGenRequestMessage API

[Fu, Siyuan]

Nagaraj,

You are right, the HTTP 1.1 requests the client to put at least a Host header 
in all request messages, that means the Request and Header should be either 
both non-zero for a new request message, or bother zero for a subsequent 
message body. So I vote for the EFI_INVALID_PARAMETER, if the input parameters 
not follow this rule.

Best Regards,
Siyuan



From: Hegde, Nagaraj P [mailto:nagaraj-p.he...@hpe.com]
Sent: Thursday, May 5, 2016 11:48 AM
To: Wu, Jiaxin ; edk2-devel@lists.01.org
Cc: Fu, Siyuan ; Ye, Ting ; 
El-Haj-Mahmoud, Samer 
Subject: RE: [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in 
HttpGenRequestMessage API

Hi Jiaxin,

I believe in HTTP 1.1, servers do not accept Request methods without any 
header. In other words, "Host:" header is a must for all request methods. That 
was the reason I planned to put the construction of header parts inside the "if 
(Message->Data.Request != NULL" condition. Now, I wonder if we need to add this 
condition in the entry of this function itself and return a 
EFI_INVALID_PARAMETER or allow it to go to HTTP server and return back as "400 
Bad Request". In case we allow, I see an issue. We would miss sending the 
additional "\r\n" which would mark the end of request and headers (if any). 
Adding additional "\r\n" is embedded into HttpUtilities Build function.

Regards,
Nagaraj.

-Original Message-
From: Wu, Jiaxin [mailto:jiaxin...@intel.com]
Sent: Thursday, May 05, 2016 8:10 AM
To: Hegde, Nagaraj P mailto:nagaraj-p.he...@hpe.com>>; 
edk2-devel@lists.01.org
Cc: Fu, Siyuan mailto:siyuan...@intel.com>>; Ye, Ting 
mailto:ting...@intel.com>>; El-Haj-Mahmoud, Samer 
mailto:samer.el-haj-mahm...@hpe.com>>
Subject: RE: [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in 
HttpGenRequestMessage API

Hi  Nagaraj,

Constructing header parts seems should be outside the conditional of "if 
(Message->Data.Request != NULL)".

if (HttpHdr != NULL) {
  //
  // Construct header
  //
  CopyMem (RequestPtr, HttpHdr, HttpHdrSize);
  RequestPtr += HttpHdrSize;
}

Others is good to me.

Reviewed-By: Wu Jiaxin mailto:jiaxin...@intel.com>>

Thanks.
Jiaxin

> -Original Message-
> From: Nagaraj Hegde [mailto:nagaraj-p.he...@hpe.com]
> Sent: Wednesday, May 4, 2016 5:33 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin mailto:jiaxin...@intel.com>>; Fu, Siyuan
> mailto:siyuan...@intel.com>>; Ye, Ting 
> mailto:ting...@intel.com>>;
> samer.el-haj-mahm...@hpe.com
> Subject: [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in
> HttpGenRequestMessage API
>
> HttpGenRequestMessage assumes that HTTP message would always contain a
> request-line, headers and an optional message body.
> However, subsequent to a HTTP PUT/POST request, HTTP requests would
> contain just the message body. This patch supports creation of such
> request messages with additional checks.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hegde, Nagaraj P 
> nagaraj-p.he...@hpe.com
> ---
>  MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 219 +++---
> --
>  1 file changed, 124 insertions(+), 95 deletions(-)
>
> diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> index 8d61d0b..fa0f591 100644
> --- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> +++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> @@ -1681,61 +1681,86 @@ HttpGenRequestMessage (
>HttpUtilitiesProtocol = NULL;
>
>//
> -  // Locate the HTTP_UTILITIES protocol.
> +  // If we have a Request, we cannot have a NULL Url
>//
> -  Status = gBS->LocateProtocol (
> -  &gEfiHttpUtilitiesProtocolGuid,
> -  NULL,
> -  (VOID **)&HttpUtilitiesProtocol
> -  );
> -
> -  if (EFI_ERROR (Status)) {
> -DEBUG ((DEBUG_ERROR,"Failed to locate Http Utilities protocol. Status
> = %r.\n", Status));
> -return Status;
> +  if (Message->Data.Request != NULL && Url == NULL) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (Message->HeaderCount != 0) {
> +//
> +// Locate the HTTP_UTILITIES protocol.
> +//
> +Status = gBS->LocateProtocol (
> +&gEfiHttpUtilitiesProtocolGuid,
> +NULL,
> +(VOID **)&HttpUtilitiesProtocol
> +);
> +
> +if (EFI_ERROR (Status)) {
> +  DEBUG ((DEBUG_ERROR,"Failed to locate Http Utilities protocol.
> + Status
> = %r.\n", Status));
> +  return Status;
> +}
> +
> +//
> +// Build AppendList to send into HttpUtilitiesBuild
> +//
> +AppendList = AllocateZeroPool (sizeof (EFI_HTTP_HEADER *) *
> + (Message-
> >HeaderCount));
> +if (AppendList == NULL) {
> +  return EFI_OUT_OF_RESOURCES;
> +}
> +
> +for(Index = 0; Index < Message->HeaderCount; Index++){
> +   

Re: [edk2] Two PCI IO protocols getting produced by same GUID, how to open correct one?

[Tian, Feng]

AtaAtapiPassThru produces ATA_PASS_THRU and EXT_SCSI_PASS_THRU two protocol 
instances.

AtaBus consumes ATA_PASS_THRU protocol to convert upper layer BlockIo request 
to ATA cmd and sent the cmd to specified ATA device.

ScsiBus + ScsiDisk consume EXT_SCSI_PASS_THRU protocol to convert upper layer 
BlockIo request to ATAPI cmd and sent the cmd to specified ATAPI device. Why we 
can do this is because ATAPI cmd is derived from SCSI cmd.

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Shaveta 
Leekha
Sent: Thursday, May 5, 2016 1:34 PM
To: Kinney, Michael D ; Laszlo Ersek 
; edk2-devel@lists.01.org 
Subject: Re: [edk2] Two PCI IO protocols getting produced by same GUID, how to 
open correct one?

Thanks Mike for the response!

Hi All,

Few doubts in SATA stack. 

I have seen at some places that in UEFI/EDK2, The complete ATA stack should be 
like:

IdeControllerDxe + 
AtaAtapiPassThru+ 
AtaBus  + 
ScsiBus + 
ScsiDisk+ 
PartitionDriver + 
Fat

What is the role of "ScsiBus + ScsiDisk" in this stack?
Complete AHCI specific code is there in "AtaAtapiPassThru"? is tha correct?
Then why ScSi bus layer is required?

Thanks and Regards,
Shaveta


-Original Message-
From: Kinney, Michael D [mailto:michael.d.kin...@intel.com]
Sent: Wednesday, May 04, 2016 10:58 PM
To: Shaveta Leekha ; Laszlo Ersek ; 
Kinney, Michael D 
Cc: edk2-devel@lists.01.org 
Subject: RE: [edk2] Two PCI IO protocols getting produced by same GUID, how to 
open correct one?

Shaveta,

All UEFI drivers (not just ones for PCI controllers) are responsible for 
evaluating the ControllerHandle in their EFI Driver Binding Supported() and 
Start() APIs to make sure the ControllerHandle is the type the driver knows how 
to manage.  If there is more than one driver that can correctly manage the same 
ControllerHandle, then there are precedence rules for the order that each 
driver's Supported() and Stop() APIs are called.

This is described in the UEFP Spec and the UEFI Driver Writer's Guide.  See 
UEFI DWG Section 3.14.1 for precedence rules.  See UEFI DWG Section 18.3 for 
details on what a PCI Driver is recommended to do in their Supported() and 
Start() functions.

https://github.com/tianocore/tianocore.github.io/wiki/UEFI-Driver-Writer's-Guide

Mike


> -Original Message-
> From: Shaveta Leekha [mailto:shaveta.lee...@nxp.com]
> Sent: Wednesday, May 4, 2016 12:23 AM
> To: Kinney, Michael D ; Laszlo Ersek 
> 
> Cc: edk2-devel@lists.01.org 
> Subject: RE: [edk2] Two PCI IO protocols getting produced by same 
> GUID, how to open correct one?
> 
> Thanks for the reply
> My replies are in-lined.
> 
> Thanks and Regards,
> Shaveta
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Kinney, Michael D
> Sent: Tuesday, May 03, 2016 9:29 PM
> To: Shaveta Leekha ; Laszlo Ersek 
> ; Kinney, Michael D 
> Cc: edk2-devel@lists.01.org 
> Subject: Re: [edk2] Two PCI IO protocols getting produced by same 
> GUID, how to open correct one?
> 
> Shaveta,
> 
> As long as all PCI I/O Protocol instances in the platform conform to 
> UEFI spec, it can work.
> 
> [Shaveta] Correct.
> 
> Your original question was about how to tell that a handle has the PCI 
> I/O Protocol produced By PCI Bus Driver or your emulation of PCI.  If 
> the PCI I/O Protocols are conformant, then It should not matter.  Just 
> write a PCI Device Driver that manages a PCI controller based on PCI 
> Class code or Vendor ID/Device ID matching and it should work.
> 
> [Shaveta] Yes, eventually correct the PCI I/O Protocol produced my PCI 
> emulation layer code is getting opened.
> As and when a valid protocol gets opened, I am checking the CLASS code 
> and Sub-CLASS code before proceeding.
> And that's working.
> 
> But what if, some other device's driver in my system, intended to open 
> "PCI I/O Protocol produced By PCI Bus Driver"
> Doesn't test CLASS CODE or any SUB-CLASS/VENDOR etc., then it may land 
> up using API's provided with protocol Of my PCI emulation layer code?
> is that understanding correct ??
> 
> So in such scenarios, where more than one protocol instances (with 
> different APIs
> associated) are installed with same GUID in the system, Then all the 
> drivers should check VENDOR ID/CLASS etc immediately after opening the 
> protocol?
> 
> 
> 
> Mike
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> > Of Shaveta Leekha
> > Sent: Tuesday, May 3, 2016 3:27 AM
> > To: Laszlo Ersek 
> > Cc: edk2-devel@lists.01.org 
> > Subject: Re: [edk2] Two PCI IO protocols getting produced by same 
> > GUID, how to open correct one?
> >
> >
> >
> > -Original Message-
> > From: Laszlo Ersek [mailto:ler...@redhat.com]
> > Sent: Tuesday, May 03, 2016 1:15 PM
> > To: Sh

Re: [edk2] Two PCI IO protocols getting produced by same GUID, how to open correct one?

[Shaveta Leekha]

Thanks for the response!

So to access a SATA HDD, to send any command to SATA HDD or to probe SATA 
devices, we need this complete stack in place?

And is there any existing command (any shell command that scans/discover SATA 
devices) in EDK2 to check whether HDD is scanned in the system?

Thanks and Regards,
Shaveta

-Original Message-
From: Tian, Feng [mailto:feng.t...@intel.com] 
Sent: Thursday, May 05, 2016 11:18 AM
To: Shaveta Leekha ; Kinney, Michael D 
; Laszlo Ersek ; 
edk2-devel@lists.01.org 
Cc: Tian, Feng 
Subject: RE: [edk2] Two PCI IO protocols getting produced by same GUID, how to 
open correct one?

AtaAtapiPassThru produces ATA_PASS_THRU and EXT_SCSI_PASS_THRU two protocol 
instances.

AtaBus consumes ATA_PASS_THRU protocol to convert upper layer BlockIo request 
to ATA cmd and sent the cmd to specified ATA device.

ScsiBus + ScsiDisk consume EXT_SCSI_PASS_THRU protocol to convert upper layer 
BlockIo request to ATAPI cmd and sent the cmd to specified ATAPI device. Why we 
can do this is because ATAPI cmd is derived from SCSI cmd.

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Shaveta 
Leekha
Sent: Thursday, May 5, 2016 1:34 PM
To: Kinney, Michael D ; Laszlo Ersek 
; edk2-devel@lists.01.org 
Subject: Re: [edk2] Two PCI IO protocols getting produced by same GUID, how to 
open correct one?

Thanks Mike for the response!

Hi All,

Few doubts in SATA stack. 

I have seen at some places that in UEFI/EDK2, The complete ATA stack should be 
like:

IdeControllerDxe + 
AtaAtapiPassThru+ 
AtaBus  + 
ScsiBus + 
ScsiDisk+ 
PartitionDriver + 
Fat

What is the role of "ScsiBus + ScsiDisk" in this stack?
Complete AHCI specific code is there in "AtaAtapiPassThru"? is tha correct?
Then why ScSi bus layer is required?

Thanks and Regards,
Shaveta


-Original Message-
From: Kinney, Michael D [mailto:michael.d.kin...@intel.com]
Sent: Wednesday, May 04, 2016 10:58 PM
To: Shaveta Leekha ; Laszlo Ersek ; 
Kinney, Michael D 
Cc: edk2-devel@lists.01.org 
Subject: RE: [edk2] Two PCI IO protocols getting produced by same GUID, how to 
open correct one?

Shaveta,

All UEFI drivers (not just ones for PCI controllers) are responsible for 
evaluating the ControllerHandle in their EFI Driver Binding Supported() and 
Start() APIs to make sure the ControllerHandle is the type the driver knows how 
to manage.  If there is more than one driver that can correctly manage the same 
ControllerHandle, then there are precedence rules for the order that each 
driver's Supported() and Stop() APIs are called.

This is described in the UEFP Spec and the UEFI Driver Writer's Guide.  See 
UEFI DWG Section 3.14.1 for precedence rules.  See UEFI DWG Section 18.3 for 
details on what a PCI Driver is recommended to do in their Supported() and 
Start() functions.

https://github.com/tianocore/tianocore.github.io/wiki/UEFI-Driver-Writer's-Guide

Mike


> -Original Message-
> From: Shaveta Leekha [mailto:shaveta.lee...@nxp.com]
> Sent: Wednesday, May 4, 2016 12:23 AM
> To: Kinney, Michael D ; Laszlo Ersek 
> 
> Cc: edk2-devel@lists.01.org 
> Subject: RE: [edk2] Two PCI IO protocols getting produced by same 
> GUID, how to open correct one?
> 
> Thanks for the reply
> My replies are in-lined.
> 
> Thanks and Regards,
> Shaveta
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Kinney, Michael D
> Sent: Tuesday, May 03, 2016 9:29 PM
> To: Shaveta Leekha ; Laszlo Ersek 
> ; Kinney, Michael D 
> Cc: edk2-devel@lists.01.org 
> Subject: Re: [edk2] Two PCI IO protocols getting produced by same 
> GUID, how to open correct one?
> 
> Shaveta,
> 
> As long as all PCI I/O Protocol instances in the platform conform to 
> UEFI spec, it can work.
> 
> [Shaveta] Correct.
> 
> Your original question was about how to tell that a handle has the PCI 
> I/O Protocol produced By PCI Bus Driver or your emulation of PCI.  If 
> the PCI I/O Protocols are conformant, then It should not matter.  Just 
> write a PCI Device Driver that manages a PCI controller based on PCI 
> Class code or Vendor ID/Device ID matching and it should work.
> 
> [Shaveta] Yes, eventually correct the PCI I/O Protocol produced my PCI 
> emulation layer code is getting opened.
> As and when a valid protocol gets opened, I am checking the CLASS code 
> and Sub-CLASS code before proceeding.
> And that's working.
> 
> But what if, some other device's driver in my system, intended to open 
> "PCI I/O Protocol produced By PCI Bus Driver"
> Doesn't test CLASS CODE or any SUB-CLASS/VENDOR etc., then it may land 
> up using API's provided with protocol Of my PCI emulation layer code?
> is that understanding correct ??
> 
> So in such scenarios, where more than one protocol instances (with 
> different APIs
> associated) are

[edk2] [PATCH] ShellPkg: Remove debug message in release binaries.

[Qiu Shumin]

Use BaseDebugLibNull instance instead of UefiDebugLibConOut to
remove the debug message in release Shell binaries.

Cc: Jaben Carsey 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin 
---
 ShellPkg/ShellPkg.dsc | 4 
 1 file changed, 4 insertions(+)

diff --git a/ShellPkg/ShellPkg.dsc b/ShellPkg/ShellPkg.dsc
index 364a622..25c4fad 100644
--- a/ShellPkg/ShellPkg.dsc
+++ b/ShellPkg/ShellPkg.dsc
@@ -27,7 +27,11 @@
   
UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
   
UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
   
DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.inf
+!if $(TARGET) == RELEASE
+  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
+!else
   DebugLib|MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
+!endif
   
DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   
MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
-- 
2.7.1.windows.2

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

Re: [edk2] [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in HttpGenRequestMessage API

[Hegde, Nagaraj P]

As Jiaxin pointed out in his reply,


if ((Message->Data.Request != NULL && Url == NULL) || (Message->Data.Request != 
NULL && Message->HeaderCount == 0)) {

return EFI_INVALID_PARAMETER;
}

looks to be a meaningful check to be in place. I will send a v2 patch.

Regards,
Nagaraj.

From: Fu, Siyuan [mailto:siyuan...@intel.com]
Sent: Thursday, May 05, 2016 11:16 AM
To: Hegde, Nagaraj P ; Wu, Jiaxin 
; edk2-devel@lists.01.org
Cc: Ye, Ting ; El-Haj-Mahmoud, Samer 

Subject: RE: [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in 
HttpGenRequestMessage API

Nagaraj,

You are right, the HTTP 1.1 requests the client to put at least a Host header 
in all request messages, that means the Request and Header should be either 
both non-zero for a new request message, or bother zero for a subsequent 
message body. So I vote for the EFI_INVALID_PARAMETER, if the input parameters 
not follow this rule.

Best Regards,
Siyuan



From: Hegde, Nagaraj P [mailto:nagaraj-p.he...@hpe.com]
Sent: Thursday, May 5, 2016 11:48 AM
To: Wu, Jiaxin mailto:jiaxin...@intel.com>>; 
edk2-devel@lists.01.org
Cc: Fu, Siyuan mailto:siyuan...@intel.com>>; Ye, Ting 
mailto:ting...@intel.com>>; El-Haj-Mahmoud, Samer 
mailto:samer.el-haj-mahm...@hpe.com>>
Subject: RE: [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in 
HttpGenRequestMessage API

Hi Jiaxin,

I believe in HTTP 1.1, servers do not accept Request methods without any 
header. In other words, "Host:" header is a must for all request methods. That 
was the reason I planned to put the construction of header parts inside the "if 
(Message->Data.Request != NULL" condition. Now, I wonder if we need to add this 
condition in the entry of this function itself and return a 
EFI_INVALID_PARAMETER or allow it to go to HTTP server and return back as "400 
Bad Request". In case we allow, I see an issue. We would miss sending the 
additional "\r\n" which would mark the end of request and headers (if any). 
Adding additional "\r\n" is embedded into HttpUtilities Build function.

Regards,
Nagaraj.

-Original Message-
From: Wu, Jiaxin [mailto:jiaxin...@intel.com]
Sent: Thursday, May 05, 2016 8:10 AM
To: Hegde, Nagaraj P mailto:nagaraj-p.he...@hpe.com>>; 
edk2-devel@lists.01.org
Cc: Fu, Siyuan mailto:siyuan...@intel.com>>; Ye, Ting 
mailto:ting...@intel.com>>; El-Haj-Mahmoud, Samer 
mailto:samer.el-haj-mahm...@hpe.com>>
Subject: RE: [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in 
HttpGenRequestMessage API

Hi  Nagaraj,

Constructing header parts seems should be outside the conditional of "if 
(Message->Data.Request != NULL)".

if (HttpHdr != NULL) {
  //
  // Construct header
  //
  CopyMem (RequestPtr, HttpHdr, HttpHdrSize);
  RequestPtr += HttpHdrSize;
}

Others is good to me.

Reviewed-By: Wu Jiaxin mailto:jiaxin...@intel.com>>

Thanks.
Jiaxin

> -Original Message-
> From: Nagaraj Hegde [mailto:nagaraj-p.he...@hpe.com]
> Sent: Wednesday, May 4, 2016 5:33 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin mailto:jiaxin...@intel.com>>; Fu, Siyuan
> mailto:siyuan...@intel.com>>; Ye, Ting 
> mailto:ting...@intel.com>>;
> samer.el-haj-mahm...@hpe.com
> Subject: [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in
> HttpGenRequestMessage API
>
> HttpGenRequestMessage assumes that HTTP message would always contain a
> request-line, headers and an optional message body.
> However, subsequent to a HTTP PUT/POST request, HTTP requests would
> contain just the message body. This patch supports creation of such
> request messages with additional checks.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hegde, Nagaraj P 
> nagaraj-p.he...@hpe.com
> ---
>  MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 219 +++---
> --
>  1 file changed, 124 insertions(+), 95 deletions(-)
>
> diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> index 8d61d0b..fa0f591 100644
> --- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> +++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> @@ -1681,61 +1681,86 @@ HttpGenRequestMessage (
>HttpUtilitiesProtocol = NULL;
>
>//
> -  // Locate the HTTP_UTILITIES protocol.
> +  // If we have a Request, we cannot have a NULL Url
>//
> -  Status = gBS->LocateProtocol (
> -  &gEfiHttpUtilitiesProtocolGuid,
> -  NULL,
> -  (VOID **)&HttpUtilitiesProtocol
> -  );
> -
> -  if (EFI_ERROR (Status)) {
> -DEBUG ((DEBUG_ERROR,"Failed to locate Http Utilities protocol. Status
> = %r.\n", Status));
> -return Status;
> +  if (Message->Data.Request != NULL && Url == NULL) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (Message->HeaderCount != 0) {
> +//
> +// Locate the HTTP_UTILITIES protocol.
> + 

Re: [edk2] [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in HttpGenRequestMessage API

[Fu, Siyuan]

Nagaraj,

The check is correct but incomplete, the Request and HeaderCount should both 
zero or both not zero.

Best Regards,
Siyuan


From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Hegde, 
Nagaraj P
Sent: Thursday, May 5, 2016 2:13 PM
To: Fu, Siyuan ; Wu, Jiaxin ; 
edk2-devel@lists.01.org
Cc: Ye, Ting 
Subject: Re: [edk2] [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in 
HttpGenRequestMessage API

As Jiaxin pointed out in his reply,


if ((Message->Data.Request != NULL && Url == NULL) || (Message->Data.Request != 
NULL && Message->HeaderCount == 0)) {

return EFI_INVALID_PARAMETER;
}

looks to be a meaningful check to be in place. I will send a v2 patch.

Regards,
Nagaraj.

From: Fu, Siyuan [mailto:siyuan...@intel.com]
Sent: Thursday, May 05, 2016 11:16 AM
To: Hegde, Nagaraj P mailto:nagaraj-p.he...@hpe.com>>; 
Wu, Jiaxin mailto:jiaxin...@intel.com>>; 
edk2-devel@lists.01.org
Cc: Ye, Ting mailto:ting...@intel.com>>; El-Haj-Mahmoud, 
Samer mailto:samer.el-haj-mahm...@hpe.com>>
Subject: RE: [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in 
HttpGenRequestMessage API

Nagaraj,

You are right, the HTTP 1.1 requests the client to put at least a Host header 
in all request messages, that means the Request and Header should be either 
both non-zero for a new request message, or bother zero for a subsequent 
message body. So I vote for the EFI_INVALID_PARAMETER, if the input parameters 
not follow this rule.

Best Regards,
Siyuan



From: Hegde, Nagaraj P [mailto:nagaraj-p.he...@hpe.com]
Sent: Thursday, May 5, 2016 11:48 AM
To: Wu, Jiaxin 
mailto:jiaxin...@intel.com>>;
 
edk2-devel@lists.01.org>
Cc: Fu, Siyuan 
mailto:siyuan...@intel.com>>;
 Ye, Ting 
mailto:ting...@intel.com>>;
 El-Haj-Mahmoud, Samer 
mailto:samer.el-haj-mahm...@hpe.com>>
Subject: RE: [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in 
HttpGenRequestMessage API

Hi Jiaxin,

I believe in HTTP 1.1, servers do not accept Request methods without any 
header. In other words, "Host:" header is a must for all request methods. That 
was the reason I planned to put the construction of header parts inside the "if 
(Message->Data.Request != NULL" condition. Now, I wonder if we need to add this 
condition in the entry of this function itself and return a 
EFI_INVALID_PARAMETER or allow it to go to HTTP server and return back as "400 
Bad Request". In case we allow, I see an issue. We would miss sending the 
additional "\r\n" which would mark the end of request and headers (if any). 
Adding additional "\r\n" is embedded into HttpUtilities Build function.

Regards,
Nagaraj.

-Original Message-
From: Wu, Jiaxin [mailto:jiaxin...@intel.com]
Sent: Thursday, May 05, 2016 8:10 AM
To: Hegde, Nagaraj P 
mailto:nagaraj-p.he...@hpe.com>>;
 
edk2-devel@lists.01.org>
Cc: Fu, Siyuan 
mailto:siyuan...@intel.com>>;
 Ye, Ting 
mailto:ting...@intel.com>>;
 El-Haj-Mahmoud, Samer 
mailto:samer.el-haj-mahm...@hpe.com>>
Subject: RE: [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in 
HttpGenRequestMessage API

Hi  Nagaraj,

Constructing header parts seems should be outside the conditional of "if 
(Message->Data.Request != NULL)".

if (HttpHdr != NULL) {
  //
  // Construct header
  //
  CopyMem (RequestPtr, HttpHdr, HttpHdrSize);
  RequestPtr += HttpHdrSize;
}

Others is good to me.

Reviewed-By: Wu Jiaxin 
mailto:jiaxin...@intel.com>>

Thanks.
Jiaxin

> -Original Message-
> From: Nagaraj Hegde [mailto:nagaraj-p.he...@hpe.com]
> Sent: Wednesday, May 4, 2016 5:33 PM
> To: 
> edk2-devel@lists.01.org>
> Cc: Wu, Jiaxin 
> mailto:jiaxin...@intel.com>>;
>  Fu, Siyuan
> mailto:siyuan...@intel.com>>;
>  Ye, Ting 
> mailto:ting...@intel.com>>;
> samer.el-haj-mahm...@hpe.com>
> Subject: [PATCH v1 1/1] MdeModulePkg:DxeHttpLib: Add checks in
> HttpGenRequestMessage API
>
> HttpGen

Re: [edk2] Two PCI IO protocols getting produced by same GUID, how to open correct one?

[Tian, Feng]

If you only need to support SATA/IDE Hard Drive, then you don't need 
ScsiBus/ScsiDisk. If you want to support SATA/IDE CDROM, you need add them in.

If a HDD has been managed by EDKII ATA stack, a device path to it would be 
created. So one way to check is to see if such device path has existed.

Thanks
Feng

-Original Message-
From: Shaveta Leekha [mailto:shaveta.lee...@nxp.com] 
Sent: Thursday, May 5, 2016 2:02 PM
To: Tian, Feng ; Kinney, Michael D 
; Laszlo Ersek ; 
edk2-devel@lists.01.org 
Subject: RE: [edk2] Two PCI IO protocols getting produced by same GUID, how to 
open correct one?

Thanks for the response!

So to access a SATA HDD, to send any command to SATA HDD or to probe SATA 
devices, we need this complete stack in place?

And is there any existing command (any shell command that scans/discover SATA 
devices) in EDK2 to check whether HDD is scanned in the system?

Thanks and Regards,
Shaveta

-Original Message-
From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Thursday, May 05, 2016 11:18 AM
To: Shaveta Leekha ; Kinney, Michael D 
; Laszlo Ersek ; 
edk2-devel@lists.01.org 
Cc: Tian, Feng 
Subject: RE: [edk2] Two PCI IO protocols getting produced by same GUID, how to 
open correct one?

AtaAtapiPassThru produces ATA_PASS_THRU and EXT_SCSI_PASS_THRU two protocol 
instances.

AtaBus consumes ATA_PASS_THRU protocol to convert upper layer BlockIo request 
to ATA cmd and sent the cmd to specified ATA device.

ScsiBus + ScsiDisk consume EXT_SCSI_PASS_THRU protocol to convert upper layer 
BlockIo request to ATAPI cmd and sent the cmd to specified ATAPI device. Why we 
can do this is because ATAPI cmd is derived from SCSI cmd.

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Shaveta 
Leekha
Sent: Thursday, May 5, 2016 1:34 PM
To: Kinney, Michael D ; Laszlo Ersek 
; edk2-devel@lists.01.org 
Subject: Re: [edk2] Two PCI IO protocols getting produced by same GUID, how to 
open correct one?

Thanks Mike for the response!

Hi All,

Few doubts in SATA stack. 

I have seen at some places that in UEFI/EDK2, The complete ATA stack should be 
like:

IdeControllerDxe + 
AtaAtapiPassThru+ 
AtaBus  + 
ScsiBus + 
ScsiDisk+ 
PartitionDriver + 
Fat

What is the role of "ScsiBus + ScsiDisk" in this stack?
Complete AHCI specific code is there in "AtaAtapiPassThru"? is tha correct?
Then why ScSi bus layer is required?

Thanks and Regards,
Shaveta


-Original Message-
From: Kinney, Michael D [mailto:michael.d.kin...@intel.com]
Sent: Wednesday, May 04, 2016 10:58 PM
To: Shaveta Leekha ; Laszlo Ersek ; 
Kinney, Michael D 
Cc: edk2-devel@lists.01.org 
Subject: RE: [edk2] Two PCI IO protocols getting produced by same GUID, how to 
open correct one?

Shaveta,

All UEFI drivers (not just ones for PCI controllers) are responsible for 
evaluating the ControllerHandle in their EFI Driver Binding Supported() and 
Start() APIs to make sure the ControllerHandle is the type the driver knows how 
to manage.  If there is more than one driver that can correctly manage the same 
ControllerHandle, then there are precedence rules for the order that each 
driver's Supported() and Stop() APIs are called.

This is described in the UEFP Spec and the UEFI Driver Writer's Guide.  See 
UEFI DWG Section 3.14.1 for precedence rules.  See UEFI DWG Section 18.3 for 
details on what a PCI Driver is recommended to do in their Supported() and 
Start() functions.

https://github.com/tianocore/tianocore.github.io/wiki/UEFI-Driver-Writer's-Guide

Mike


> -Original Message-
> From: Shaveta Leekha [mailto:shaveta.lee...@nxp.com]
> Sent: Wednesday, May 4, 2016 12:23 AM
> To: Kinney, Michael D ; Laszlo Ersek 
> 
> Cc: edk2-devel@lists.01.org 
> Subject: RE: [edk2] Two PCI IO protocols getting produced by same 
> GUID, how to open correct one?
> 
> Thanks for the reply
> My replies are in-lined.
> 
> Thanks and Regards,
> Shaveta
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Kinney, Michael D
> Sent: Tuesday, May 03, 2016 9:29 PM
> To: Shaveta Leekha ; Laszlo Ersek 
> ; Kinney, Michael D 
> Cc: edk2-devel@lists.01.org 
> Subject: Re: [edk2] Two PCI IO protocols getting produced by same 
> GUID, how to open correct one?
> 
> Shaveta,
> 
> As long as all PCI I/O Protocol instances in the platform conform to 
> UEFI spec, it can work.
> 
> [Shaveta] Correct.
> 
> Your original question was about how to tell that a handle has the PCI 
> I/O Protocol produced By PCI Bus Driver or your emulation of PCI.  If 
> the PCI I/O Protocols are conformant, then It should not matter.  Just 
> write a PCI Device Driver that manages a PCI controller based on PCI 
> Class code or Vendor ID/Device ID matching and it should work.
> 
> [Shaveta] Yes, eventually correct the PCI I/O Protocol pr

[edk2] [PATCH v2 1/1] MdeModulePkg:DxeHttpLib: Add checks in HttpGenRequestMessage API

[Nagaraj Hegde]

v2:
More input validation based on Request and HeaderCount.

HttpGenRequestMessage assumes that HTTP message would always
contain a request-line, headers and an optional message body.
However, subsequent to a HTTP PUT/POST request, HTTP requests
would contain just the message body. This patch supports
creation of such request messages with additional checks.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hegde, Nagaraj P nagaraj-p.he...@hpe.com
---
 MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 225 +++-
 1 file changed, 130 insertions(+), 95 deletions(-)

diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c 
b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
index 8d61d0b..1b0858c 100644
--- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
+++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
@@ -1681,61 +1681,92 @@ HttpGenRequestMessage (
   HttpUtilitiesProtocol = NULL;
 
   //
-  // Locate the HTTP_UTILITIES protocol.
+  // 1. If we have a Request, we cannot have a NULL Url
+  // 2. If we have a Request, HeaderCount can not be non-zero
+  // 3. If we do not have a Request, HeaderCount should be zero
+  // 4. If we do not have Request and Headers, we need at least a message-body
   //
-  Status = gBS->LocateProtocol (
-  &gEfiHttpUtilitiesProtocolGuid,
-  NULL,
-  (VOID **)&HttpUtilitiesProtocol
-  );
-
-  if (EFI_ERROR (Status)) {
-DEBUG ((DEBUG_ERROR,"Failed to locate Http Utilities protocol. Status = 
%r.\n", Status));
-return Status;
+  if ((Message->Data.Request != NULL && Url == NULL) ||
+  (Message->Data.Request != NULL && Message->HeaderCount == 0) ||
+  (Message->Data.Request == NULL && Message->HeaderCount != 0) ||
+  (Message->Data.Request == NULL && Message->HeaderCount == 0 && 
Message->BodyLength == 0)) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (Message->HeaderCount != 0) {
+//
+// Locate the HTTP_UTILITIES protocol.
+//
+Status = gBS->LocateProtocol (
+&gEfiHttpUtilitiesProtocolGuid,
+NULL,
+(VOID **)&HttpUtilitiesProtocol
+);
+
+if (EFI_ERROR (Status)) {
+  DEBUG ((DEBUG_ERROR,"Failed to locate Http Utilities protocol. Status = 
%r.\n", Status));
+  return Status;
+}
+
+//
+// Build AppendList to send into HttpUtilitiesBuild
+//
+AppendList = AllocateZeroPool (sizeof (EFI_HTTP_HEADER *) * 
(Message->HeaderCount));
+if (AppendList == NULL) {
+  return EFI_OUT_OF_RESOURCES;
+}
+
+for(Index = 0; Index < Message->HeaderCount; Index++){
+  AppendList[Index] = &Message->Headers[Index];
+}
+
+//
+// Build raw HTTP Headers
+//
+Status = HttpUtilitiesProtocol->Build (
+HttpUtilitiesProtocol,
+0,
+NULL,
+0,
+NULL,
+Message->HeaderCount,
+AppendList,
+&HttpHdrSize,
+&HttpHdr
+);
+
+if (AppendList != NULL) {
+  FreePool (AppendList);
+}
+
+if (EFI_ERROR (Status) || HttpHdr == NULL){
+  return Status;
+}
   }
 
   //
-  // Build AppendList to send into HttpUtilitiesBuild
+  // If we have headers to be sent, account for it.
   //
-  AppendList = AllocateZeroPool (sizeof (EFI_HTTP_HEADER *) * 
(Message->HeaderCount));
-  if (AppendList == NULL) {
-return EFI_OUT_OF_RESOURCES;
-  }
-
-  for(Index = 0; Index < Message->HeaderCount; Index++){
-AppendList[Index] = &Message->Headers[Index];
+  if (Message->HeaderCount != 0) {
+MsgSize = HttpHdrSize;
   }
 
   //
-  // Build raw HTTP Headers
+  // If we have a request line, account for the fields.
   //
-  Status = HttpUtilitiesProtocol->Build (
-  HttpUtilitiesProtocol,
-  0,
-  NULL,
-  0,
-  NULL,
-  Message->HeaderCount,
-  AppendList,
-  &HttpHdrSize,
-  &HttpHdr
-  );
-
-  if (AppendList != NULL) {
-FreePool (AppendList);
-  }
-
-  if (EFI_ERROR (Status) || HttpHdr == NULL){
-return Status;
+  if (Message->Data.Request != NULL) {
+MsgSize += HTTP_METHOD_MAXIMUM_LEN + AsciiStrLen (HTTP_VERSION_CRLF_STR) + 
AsciiStrLen (Url);
   }
 
+
   //
-  // Calculate HTTP message length.
+  // If we have a message body to be sent, account for it.
   //
-  MsgSize = Message->BodyLength + HTTP_METHOD_MAXIMUM_LEN + AsciiStrLen (Url) +
-AsciiStrLen (HTTP_VERSION_CRLF_STR) + HttpHdrSize;
-
+  MsgSize += Message->BodyLength;
 
+  //
+  // memory for the string that needs to be sent to TCP
+  //
   *RequestMsg = AllocateZeroPool (MsgSize);
   if (*RequestMsg == NULL) {
 Status = EFI_OUT_OF_RESOURCES;
@@ -1746,60 +1777,64 @@ HttpGenRequestMessage (
   //
   // Construct header request
   //
-  switch (Message->Data.Request->Method) {
-  case Ht