Re: [edk2] [PATCH v2] ArmVirtPkg/ArmVirtXen: Add ACPI support for Virt Xen ARM

2016-06-24 Thread Shannon Zhao 


On 2016/6/25 11:05, Andrew Fish wrote:
>> > On Jun 24, 2016, at 7:50 PM, Shannon Zhao  wrote:
>> > 
>> > 
>> > 
>> > On 2016/6/23 21:42, Ard Biesheuvel wrote:
>  +  //
>  +  // Get the RSDP structure address from DeviceTree
>  +  //
>  +  Status = gBS->LocateProtocol (, NULL,
>>> >> Please add gFdtClientProtocolGuid to the [Depex] section of this module
>>> >> 
>> > Hi Ard,
>> > 
>> > I try to add gFdtClientProtocolGuid to the [Depex] but I got a compiling
>> > error.
>> > 
>> > build.py...
>> > : error F001: Invalid dependency expression: missing operator before {
>> > 0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2, 0x01, 0xBA, 0xA2, 0x59, 0x1B,
>> > 0x4C } }
>> >Near { 0xFFE06BDD, 0x6107, 0x46A6, { 0x7B, 0xB2, 0x5A, 0x9C,
>> > 0x7E, 0xC5, 0x27, 0x5C }}
>> > 
> That is not a common error. Can you attach you INF file? It sounds like a 
> syntax error in the [Depex]?

Ah, right. I missed the AND between two elements of [Depex].

Thanks so much :)

-- 
Shannon

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

Re: [edk2] [PATCH v2] ArmVirtPkg/ArmVirtXen: Add ACPI support for Virt Xen ARM

2016-06-24 Thread Andrew Fish 

> On Jun 24, 2016, at 7:50 PM, Shannon Zhao  wrote:
> 
> 
> 
> On 2016/6/23 21:42, Ard Biesheuvel wrote:
 +  //
 +  // Get the RSDP structure address from DeviceTree
 +  //
 +  Status = gBS->LocateProtocol (, NULL,
>> Please add gFdtClientProtocolGuid to the [Depex] section of this module
>> 
> Hi Ard,
> 
> I try to add gFdtClientProtocolGuid to the [Depex] but I got a compiling
> error.
> 
> build.py...
> : error F001: Invalid dependency expression: missing operator before {
> 0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2, 0x01, 0xBA, 0xA2, 0x59, 0x1B,
> 0x4C } }
>Near { 0xFFE06BDD, 0x6107, 0x46A6, { 0x7B, 0xB2, 0x5A, 0x9C,
> 0x7E, 0xC5, 0x27, 0x5C }}
> 

That is not a common error. Can you attach you INF file? It sounds like a 
syntax error in the [Depex]?

Thanks,

Andrew Fish

> How to deal with this problem?
> 
> Thanks,
> -- 
> Shannon
> 
> ___
> 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] ArmVirtPkg/ArmVirtXen: Add ACPI support for Virt Xen ARM

2016-06-24 Thread Shannon Zhao 


On 2016/6/23 21:42, Ard Biesheuvel wrote:
>> > +  //
>> > +  // Get the RSDP structure address from DeviceTree
>> > +  //
>> > +  Status = gBS->LocateProtocol (, NULL,
> Please add gFdtClientProtocolGuid to the [Depex] section of this module
> 
Hi Ard,

I try to add gFdtClientProtocolGuid to the [Depex] but I got a compiling
error.

build.py...
 : error F001: Invalid dependency expression: missing operator before {
0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2, 0x01, 0xBA, 0xA2, 0x59, 0x1B,
0x4C } }
Near { 0xFFE06BDD, 0x6107, 0x46A6, { 0x7B, 0xB2, 0x5A, 0x9C,
0x7E, 0xC5, 0x27, 0x5C }}

How to deal with this problem?

Thanks,
-- 
Shannon

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

Re: [edk2] [PATCH] [edk2-staging/HTTPS-TLS][PATCH]: Centralize TLS var cert name and guid

2016-06-24 Thread Palmer, Thomas 
Jiaxin, et al ~

I noticed while using the TLS feature that the GUID and Variable Name 
define were being re-defined in multiple spots.  Currently, if someone were to 
write a UEFI application, there is no single include file that would provide 
the variable name in a define. As a matter of sheer better programming 
practices, the Variable Name define and GUID should be put into central 
locations and not copied all over the codebase.  

With regards to GlobalVariable.h: I realize now this is not the place 
to put it.   Because our variable has a unique GUID, we would have to create a 
new MdePkg/Include/Guid/ header file to hold the define, much like 
ImageAuthentication.h which is also used by VarCheckUefiLibNullClass.c.

I put the GUID definition into CryptoPkg.dec because the TlsLib and 
OpenSSL library are there.  I can be persuaded to have it in NetworkPkg.dec as 
it's modules are the ultimate consumers of the variable. CryptoPkg is simply 
providing libraries.

With regards to "[TlsCaCertificate is] only a private variable":   This 
variable is super critical to secure TLS communication.  It is so critical that 
we are even discussing how to protect it in runtime from malicious/careless 
modifications.  We understand that if this variable were compromised that there 
could be severe security implications that follow.  This variable must be 
respected properly.

For that reason, I'd argue that we should put the TlsCaCertificate into 
the UEFI Spec.  When it gets put into the spec I do not know, but we should be 
aiming for that. It is too important to security to not be in the spec.  

Not only that, but once this is in the spec it will enable 3rd party 
applications to re-use this variable too. I've personally talked with one such 
developer who is eagerly awaiting a variable that is a secure UEFI standard for 
Certificate Authority storage.

Thomas

-Original Message-
From: Wu, Jiaxin [mailto:jiaxin...@intel.com] 
Sent: Thursday, June 23, 2016 9:56 PM
To: Palmer, Thomas ; edk2-devel@lists.01.org
Cc: El-Haj-Mahmoud, Samer ; Zimmer, Vincent 
; Li, Ruth ; Fu, Siyuan 
; Ye, Ting ; Hsiung, Harry L 

Subject: RE: [PATCH] [edk2-staging/HTTPS-TLS][PATCH]: Centralize TLS var cert 
name and guid

Hi Thomas,
One point we should know that TLS cert variable is not defined in UEFI Spec, 
it's only private variable used to configure the CA certificate. So, we can't 
add this variable check into VarCheckUefiLib.  VarCheckUefiLib only contain the 
variables defined in UEFI spec,  private variable is not allowed. 
EDKII_VAR_CHECK_PROTOCOL  could be located directly If we truly want to check 
one private variable.

In addition, I think defining TlsCaCertificate in GlobalVariable.h is also 
unreasonable. This file should only contain globally defined variables with 
gEfiGlobalVariableGuid. What do you think?

Moreover, Why put the GUID definition in CryptoPkg.dec? It looks so strange.

Thanks.
Jiaxin


> -Original Message-
> From: Thomas Palmer [mailto:thomas.pal...@hpe.com]
> Sent: Friday, June 24, 2016 2:14 AM
> To: edk2-devel@lists.01.org
> Cc: samer.el-haj-mahm...@hpe.com; Wu, Jiaxin ; 
> Zimmer, Vincent ; Li, Ruth 
> ; Fu, Siyuan ; Ye, Ting 
> ; Hsiung, Harry L ; 
> Thomas Palmer 
> Subject: [PATCH] [edk2-staging/HTTPS-TLS][PATCH]: Centralize TLS var 
> cert name and guid
> 
> Put the TLS cert variable name define into GlobalVariable.h and create 
> a GUID for it in CryptoPkg.dec. Describe the minimum size and expected 
> variable attributes in VarCheckUefiLib.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Thomas Palmer 
> ---
>  CryptoPkg/CryptoPkg.dec|  5 
>  .../Library/VarCheckUefiLib/VarCheckUefiLib.inf|  3 +++
>  .../VarCheckUefiLib/VarCheckUefiLibNullClass.c | 28
> +-
>  MdePkg/Include/Guid/GlobalVariable.h   |  7 ++
>  NetworkPkg/HttpDxe/HttpDxe.inf |  7 +-
>  NetworkPkg/HttpDxe/HttpsSupport.c  |  7 +++---
>  NetworkPkg/HttpDxe/HttpsSupport.h  | 11 +
>  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf   |  3 +++
>  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c| 11 -
>  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.h| 11 +
>  10 files changed, 61 insertions(+), 32 deletions(-)
> 
> diff --git a/CryptoPkg/CryptoPkg.dec b/CryptoPkg/CryptoPkg.dec index 
> ea02ad7..fe04b7d 100644
> --- a/CryptoPkg/CryptoPkg.dec
> +++ b/CryptoPkg/CryptoPkg.dec
> @@ -5,6 +5,7 @@
>  #  It also provides a test application to test

Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length check

2016-06-24 Thread Evgeny Yakovlev 
Hi! We verified that xHCI with your patch correctly handles handshaking
with the problem device.

2016-06-14 3:42 GMT+03:00 Tian, Feng :

> Thanks for your info, Yakovlev.
>
> Did you see problem with XHCI? If yes, I can provide a patch and could you
> help verify it?
>
> Thanks
> Feng
>
> -Original Message-
> From: Evgeny Yakovlev [mailto:insorei...@gmail.com]
> Sent: Monday, June 13, 2016 5:29 PM
> To: Tian, Feng 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor
> length check
>
> Hi Feng.
>
> Sorry, i was AFK for the past week.
>
> We have encountered this with Platronix USB headset. This device was
> reporting *USB Interface* descriptor sizes 1 byte larger than what the spec
> says. Headset was simply plugged in when we booted our firmware and
> UsbCreateDesc failed to parse that and returned NULL which eventually
> crashed the firmware later (i will try and send a separate patch to fix
> that). We fixed UsbCreateDesc and device enumeration went fine, all
> descriptors it sends are correct except for this odd size field.
>
> Evgeny
>
> > On 13 Jun 2016, at 05:32, Tian, Feng  wrote:
> >
> > Hi, Yakovlev
> >
> > Any response on this?
> >
> > Thanks
> > Feng
> >
> > -Original Message-
> > From: Tian, Feng
> > Sent: Monday, June 6, 2016 9:27 AM
> > To: Evgeny Yakovlev ; edk2-devel@lists.01.org
> > Cc: Tian, Feng 
> > Subject: RE: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor
> length check
> >
> > Hi, Evgenv
> >
> > Thanks for your contribution, Evgenv
> >
> > Could you let me know what error case you met? Is the Device Desc or
> Config Desc larger than expected or other descs?
> >
> > Why I ask this question is because it would impact Xhci driver's
> implementation in which we store some desc contents at internal. So looks
> like XHCI driver also needs to be updated.
> >
> > Thanks
> > Feng
> >
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Evgeny Yakovlev
> > Sent: Sunday, June 5, 2016 10:29 PM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor
> length check
> >
> > According to spec if the length of a descriptor is smaller than what the
> specification defines, then the host shall ignore it.
> > However if the size is greater than expected the host will ignore the
> extra bytes and start looking for the next descriptor at the end of actual
> length returned. Original check did not handle the latter case correctly
> and only allowed descriptors with lengths exactly as defined in
> specification.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Evgeny Yakovlev 
> > ---
> > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> > index 5b8b1aa..fba60da 100644
> > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> > @@ -199,8 +199,8 @@ UsbCreateDesc (
> > }
> >   }
> >
> > -  if ((Len <= Offset)  || (Len < Offset + DescLen) ||
> > -  (Head->Type != Type) || (Head->Len != DescLen)) {
> > +  if ((Len <= Offset)  || (Len < Offset + Head->Len) ||
> > +  (Head->Type != Type) || (Head->Len < DescLen)) {
> > DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor\n"));
> > return NULL;
> >   }
> > --
> > 2.7.4 (Apple Git-66)
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 2/2] EdkCompatibilityPkg: Fix the typo in the comment

2016-06-24 Thread Gary Lin 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Gary Lin 
---
 .../Compatibility/FrameworkHiiOnUefiHiiThunk/UefiIfrParser.c| 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/EdkCompatibilityPkg/Compatibility/FrameworkHiiOnUefiHiiThunk/UefiIfrParser.c 
b/EdkCompatibilityPkg/Compatibility/FrameworkHiiOnUefiHiiThunk/UefiIfrParser.c
index 07b99fb..105c3b5 100644
--- 
a/EdkCompatibilityPkg/Compatibility/FrameworkHiiOnUefiHiiThunk/UefiIfrParser.c
+++ 
b/EdkCompatibilityPkg/Compatibility/FrameworkHiiOnUefiHiiThunk/UefiIfrParser.c
@@ -97,7 +97,7 @@ DestoryOneOfOptionMap (
   Initialize Statement header members.
 
   @param  OpCodeData Pointer of the raw OpCode data.
-  @param  FormSetPointer of the current FormSe.
+  @param  FormSetPointer of the current FormSet.
   @param  Form   Pointer of the current Form.
 
   @return The Statement.
-- 
2.8.4

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

[edk2] [PATCH 1/2] MdeModulePkg/SetupBrowser: Fix the typo in the comment

2016-06-24 Thread Gary Lin 
Cc: Feng Tian 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Gary Lin 
---
 MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c 
b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
index 8e8607d..11a8fdc 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
@@ -22,7 +22,7 @@ extern LIST_ENTRY  gBrowserStorageList;
   Initialize Statement header members.
 
   @param  OpCodeData Pointer of the raw OpCode data.
-  @param  FormSetPointer of the current FormSe.
+  @param  FormSetPointer of the current FormSet.
   @param  Form   Pointer of the current Form.
 
   @return The Statement.
-- 
2.8.4

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

[edk2] [PATCH] [MdePkg ] Additional Atapi.h definitions complying Industry Standard specifications.

2016-06-24 Thread Anandakrishnan Loganathan 
Dear MdePkg maintainer,


Atapi.h  has only limited ATA/ATAPI related specification definitions and this 
attached patch includes various commonly usage Industry Standard definitions in 
Atapi.h. Please review and update to the MdePkg.


Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Anandakrishnan Loganathan 
anandakrishn...@ami.com


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

Re: [edk2] iPXE invalidates the OVMF network device options

2016-06-24 Thread Gerd Hoffmann 
  Hi,

> Thanks, I guess we can remove these if building with
> DEBUG=virtio-net:2 was intentional and it interferes with other
> output. But then again, if you build for example with DEBUG=tcp:2,
> you'll see a debug message for each transmitted or received packet
> also.

Sorry for the trouble.  Everything is fine.  My test configuration was
broken, romfile= didn't point to the file I intended to test with.

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

Re: [edk2] [Patch] NetworkPkg: Avoid potential NULL pointer dereference

2016-06-24 Thread Wu, Jiaxin 

> -Original Message-
> From: Ye, Ting
> Sent: Friday, June 24, 2016 4:25 PM
> To: Wu, Jiaxin ; edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Zhang, Lubo 
> Subject: RE: [Patch] NetworkPkg: Avoid potential NULL pointer dereference
> 
> Looks good to me. I only have one small comment:
> 
> +  if (EFI_ERROR (Ikev2EncodePacket ((IKEV2_SESSION_COMMON *)
> SessionCommon, IkePacket, IkeType))) {
> +return NULL;
> +  }

Ting,
Do you mean any compilers will meet failure or warnings with this one line code?



> 
> Do we need divide it into two code lines to satisfy some compilers?
> Status = Ikev2EncodePacket ((IKEV2_SESSION_COMMON *)
> SessionCommon, IkePacket, IkeType)); if (EFI_ERROR (Status)) ...
> 
> As long as our supported compilers will not report build errors or warnings, I
> am fine with the update.
> 
> Reviewed-by: Ye Ting 
> 
> 
> 
> -Original Message-
> From: Wu, Jiaxin
> Sent: Friday, June 24, 2016 3:33 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Zhang,
> Lubo 
> Subject: [Patch] NetworkPkg: Avoid potential NULL pointer dereference
> 
> The commit of 6b16c9e7 removes ASSERT and use error handling in IpSecDxe
> driver, but may cause the potential NULL pointer dereference. So, this patch
> is used to avoid NULL pointer dereference.
> 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Cc: Zhang Lubo 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu 
> ---
>  NetworkPkg/IpSecDxe/IkePacket.c  |   6 +-
>  NetworkPkg/IpSecDxe/Ikev2/ChildSa.c  |  19 ++--
> NetworkPkg/IpSecDxe/Ikev2/Exchange.c |  10 ++-
>  NetworkPkg/IpSecDxe/Ikev2/Payload.c  |   3 +
>  NetworkPkg/IpSecDxe/Ikev2/Sa.c   | 163
> ++-
>  5 files changed, 188 insertions(+), 13 deletions(-)
> 
> diff --git a/NetworkPkg/IpSecDxe/IkePacket.c
> b/NetworkPkg/IpSecDxe/IkePacket.c index 8fd395d..d5a938e 100644
> --- a/NetworkPkg/IpSecDxe/IkePacket.c
> +++ b/NetworkPkg/IpSecDxe/IkePacket.c
> @@ -1,9 +1,9 @@
>  /** @file
>IKE Packet related operation.
> 
> -  Copyright (c) 2010, Intel Corporation. All rights reserved.
> +  Copyright (c) 2010 - 2016, Intel Corporation. All rights
> + reserved.
> 
>This program and the accompanying materials
>are licensed and made available under the terms and conditions 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.
> @@ -201,11 +201,13 @@ IkeNetbufFromPacket (
>  //
>  // Convert Host order to Network order for IKE_PACKET header and
> payloads
>  // Encryption payloads if needed
>  //
>  if (((IKEV2_SESSION_COMMON *) SessionCommon)->IkeVer == 2) {
> -  Ikev2EncodePacket ((IKEV2_SESSION_COMMON *) SessionCommon,
> IkePacket, IkeType);
> +  if (EFI_ERROR (Ikev2EncodePacket ((IKEV2_SESSION_COMMON *)
> SessionCommon, IkePacket, IkeType))) {
> +return NULL;
> +  }
>  } else {
>//
>//If IKEv1 support, check it here.
>//
>return NULL;
> diff --git a/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
> b/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
> index d3859e2..1f0199b 100644
> --- a/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
> +++ b/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
> @@ -1,9 +1,9 @@
>  /** @file
>The operations for Child SA.
> 
> -  Copyright (c) 2010, Intel Corporation. All rights reserved.
> +  Copyright (c) 2010 - 2016, Intel Corporation. All rights
> + reserved.
> 
>This program and the accompanying materials
>are licensed and made available under the terms and conditions 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.
> @@ -37,22 +37,25 @@ Ikev2CreateChildGenerator (
>IKEV2_CHILD_SA_SESSION  *ChildSaSession;
>IKEV2_SA_SESSION*IkeSaSession;
>IKE_PACKET  *IkePacket;
>IKE_PAYLOAD *NotifyPayload;
>UINT32  *MessageId;
> +
> +  NotifyPayload   = NULL;
> +  MessageId   = NULL;
> 
>ChildSaSession  = (IKEV2_CHILD_SA_SESSION *) SaSession;
> -  IkePacket   = IkePacketAlloc();
> -  MessageId   = NULL;
> -
> -  if (IkePacket == NULL) {
> +  if (ChildSaSession == NULL) {
>  return NULL;
>}
> -  if (ChildSaSession == NULL) {
> +
> +  IkePacket   = IkePacketAlloc();
> +  if (IkePacket == NULL) {
>  return NULL;
>}
> 
> +
>if (Context != NULL) {
>  MessageId = (UINT32 *) Context;
>}
> 
>IkePacket->Header->Version  = (UINT8) (2 << 4);
> @@ -111,10 +114,14 @@ Ikev2CreateChildGenerator (
>  IKEV2_NOTIFICATION_NO_ADDITIONAL_SAS,
>  NULL,
>

Re: [edk2] [Patch] NetworkPkg: Avoid potential NULL pointer dereference

2016-06-24 Thread Ye, Ting 
Looks good to me. I only have one small comment:

+  if (EFI_ERROR (Ikev2EncodePacket ((IKEV2_SESSION_COMMON *) 
SessionCommon, IkePacket, IkeType))) {
+return NULL;
+  }

Do we need divide it into two code lines to satisfy some compilers? 
Status = Ikev2EncodePacket ((IKEV2_SESSION_COMMON *) SessionCommon, IkePacket, 
IkeType));
if (EFI_ERROR (Status)) 
...

As long as our supported compilers will not report build errors or warnings, I 
am fine with the update.

Reviewed-by: Ye Ting 



-Original Message-
From: Wu, Jiaxin 
Sent: Friday, June 24, 2016 3:33 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Fu, Siyuan ; Zhang, Lubo 

Subject: [Patch] NetworkPkg: Avoid potential NULL pointer dereference

The commit of 6b16c9e7 removes ASSERT and use error handling in IpSecDxe 
driver, but may cause the potential NULL pointer dereference. So, this patch is 
used to avoid NULL pointer dereference.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Zhang Lubo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/IpSecDxe/IkePacket.c  |   6 +-
 NetworkPkg/IpSecDxe/Ikev2/ChildSa.c  |  19 ++--  
NetworkPkg/IpSecDxe/Ikev2/Exchange.c |  10 ++-
 NetworkPkg/IpSecDxe/Ikev2/Payload.c  |   3 +
 NetworkPkg/IpSecDxe/Ikev2/Sa.c   | 163 ++-
 5 files changed, 188 insertions(+), 13 deletions(-)

diff --git a/NetworkPkg/IpSecDxe/IkePacket.c b/NetworkPkg/IpSecDxe/IkePacket.c 
index 8fd395d..d5a938e 100644
--- a/NetworkPkg/IpSecDxe/IkePacket.c
+++ b/NetworkPkg/IpSecDxe/IkePacket.c
@@ -1,9 +1,9 @@
 /** @file
   IKE Packet related operation.
 
-  Copyright (c) 2010, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2016, Intel Corporation. All rights 
+ reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions 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.
@@ -201,11 +201,13 @@ IkeNetbufFromPacket (
 //
 // Convert Host order to Network order for IKE_PACKET header and payloads
 // Encryption payloads if needed
 //
 if (((IKEV2_SESSION_COMMON *) SessionCommon)->IkeVer == 2) {
-  Ikev2EncodePacket ((IKEV2_SESSION_COMMON *) SessionCommon, IkePacket, 
IkeType);
+  if (EFI_ERROR (Ikev2EncodePacket ((IKEV2_SESSION_COMMON *) 
SessionCommon, IkePacket, IkeType))) {
+return NULL;
+  }
 } else {
   //
   //If IKEv1 support, check it here.
   //
   return NULL;
diff --git a/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c 
b/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
index d3859e2..1f0199b 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
@@ -1,9 +1,9 @@
 /** @file
   The operations for Child SA.
 
-  Copyright (c) 2010, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2016, Intel Corporation. All rights 
+ reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions 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.
@@ -37,22 +37,25 @@ Ikev2CreateChildGenerator (
   IKEV2_CHILD_SA_SESSION  *ChildSaSession;
   IKEV2_SA_SESSION*IkeSaSession;
   IKE_PACKET  *IkePacket;
   IKE_PAYLOAD *NotifyPayload;
   UINT32  *MessageId;
+
+  NotifyPayload   = NULL;
+  MessageId   = NULL;
   
   ChildSaSession  = (IKEV2_CHILD_SA_SESSION *) SaSession;
-  IkePacket   = IkePacketAlloc();
-  MessageId   = NULL;
-
-  if (IkePacket == NULL) {
+  if (ChildSaSession == NULL) {
 return NULL;
   }
-  if (ChildSaSession == NULL) {
+
+  IkePacket   = IkePacketAlloc();
+  if (IkePacket == NULL) {
 return NULL;
   }
 
+
   if (Context != NULL) {
 MessageId = (UINT32 *) Context;
   }
   
   IkePacket->Header->Version  = (UINT8) (2 << 4);
@@ -111,10 +114,14 @@ Ikev2CreateChildGenerator (
 IKEV2_NOTIFICATION_NO_ADDITIONAL_SAS,
 NULL,
 NULL,
 0
 );
+  if (NotifyPayload == NULL) { 
+IkePacketFree (IkePacket);
+return NULL;
+  }
 
   IKE_PACKET_APPEND_PAYLOAD (IkePacket, NotifyPayload);
   //
   // TODO: Support the CREATE_CHILD_SA exchange. 
   //
diff --git a/NetworkPkg/IpSecDxe/Ikev2/Exchange.c 
b/NetworkPkg/IpSecDxe/Ikev2/Exchange.c
index 9d58ab0a..1eddbfb 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Exchange.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/Exchange.c
@@ -1,9 +1,9 @@
 /** @file
   The general interfaces of the IKEv2.
 
-  Copyright (c) 2010 - 2015, Intel Corporation. All

Re: [edk2] iPXE invalidates the OVMF network device options

2016-06-24 Thread Ladi Prosek 
On Fri, Jun 24, 2016 at 10:02 AM, Gerd Hoffmann  wrote:
>   Hi,
>
>> > First thing I've noticed is that the virtio-net driver floods the
>> > terminal with debug messages for each and every package.  That is
>> > clearly a nonstarter and must be fixed.
>>
>> All the more verbose virtio-net messages require log level 2, and
>> logging every packet at level 2 seems to be in line with the rest of
>> the iPXE networking stack. What debug messages are you seeing?
>
> VIRTIO-NET 0x3e9ba378 rx complete iobuf 0x3e9c85f8 len 52
> VIRTIO-NET 0x3e9ba378 enqueuing iobuf 0x3e9c7df8 on vq 0
> vring_add_buf: 0x3e9be022 14 - 12 10  8  6  4
> VIRTIO-NET 0x3e9ba378 rx complete iobuf 0x3e9c8df8 len 52
> VIRTIO-NET 0x3e9ba378 enqueuing iobuf 0x3e9c85f8 on vq 0
> vring_add_buf: 0x3e9be024  0 - 14 12 10  8  6
> VIRTIO-NET 0x3e9ba378 rx complete iobuf 0x3e9c4df8 len 52
> VIRTIO-NET 0x3e9ba378 enqueuing iobuf 0x3e9c8df8 on vq 0
> vring_add_buf: 0x3e9be026  2 -  0 14 12 10  8
> VIRTIO-NET 0x3e9ba378 rx complete iobuf 0x3e9c55f8 len 52
> VIRTIO-NET 0x3e9ba378 enqueuing iobuf 0x3e9c4df8 on vq 0
> vring_add_buf: 0x3e9be028  4 -  2  0 14 12 10
> VIRTIO-NET 0x3e9ba378 rx complete iobuf 0x3e9c5df8 len 52
> VIRTIO-NET 0x3e9ba378 enqueuing iobuf 0x3e9c55f8 on vq 0
> vring_add_buf: 0x3e9be02a  6 -  4  2  0 14 12
> VIRTIO-NET 0x3e9ba378 rx complete iobuf 0x3e9c65f8 len 52
> VIRTIO-NET 0x3e9ba378 enqueuing iobuf 0x3e9c5df8 on vq 0
> vring_add_buf: 0x3e9be02c  8 -  6  4  2  0 14
> VIRTIO-NET 0x3e9ba378 rx complete iobuf 0x3e9c6df8 len 52
> VIRTIO-NET 0x3e9ba378 enqueuing iobuf 0x3e9c65f8 on vq 0
> vring_add_buf: 0x3e9be02e 10 -  8  6  4  2  0
>
> When booting e1000 or rtl8139 I don't see any messages.

Thanks, I guess we can remove these if building with
DEBUG=virtio-net:2 was intentional and it interferes with other
output. But then again, if you build for example with DEBUG=tcp:2,
you'll see a debug message for each transmitted or received packet
also.

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

Re: [edk2] iPXE invalidates the OVMF network device options

2016-06-24 Thread Gerd Hoffmann 
  Hi,

> > First thing I've noticed is that the virtio-net driver floods the
> > terminal with debug messages for each and every package.  That is
> > clearly a nonstarter and must be fixed.
> 
> All the more verbose virtio-net messages require log level 2, and
> logging every packet at level 2 seems to be in line with the rest of
> the iPXE networking stack. What debug messages are you seeing?

VIRTIO-NET 0x3e9ba378 rx complete iobuf 0x3e9c85f8 len 52
VIRTIO-NET 0x3e9ba378 enqueuing iobuf 0x3e9c7df8 on vq 0
vring_add_buf: 0x3e9be022 14 - 12 10  8  6  4
VIRTIO-NET 0x3e9ba378 rx complete iobuf 0x3e9c8df8 len 52
VIRTIO-NET 0x3e9ba378 enqueuing iobuf 0x3e9c85f8 on vq 0
vring_add_buf: 0x3e9be024  0 - 14 12 10  8  6
VIRTIO-NET 0x3e9ba378 rx complete iobuf 0x3e9c4df8 len 52
VIRTIO-NET 0x3e9ba378 enqueuing iobuf 0x3e9c8df8 on vq 0
vring_add_buf: 0x3e9be026  2 -  0 14 12 10  8
VIRTIO-NET 0x3e9ba378 rx complete iobuf 0x3e9c55f8 len 52
VIRTIO-NET 0x3e9ba378 enqueuing iobuf 0x3e9c4df8 on vq 0
vring_add_buf: 0x3e9be028  4 -  2  0 14 12 10
VIRTIO-NET 0x3e9ba378 rx complete iobuf 0x3e9c5df8 len 52
VIRTIO-NET 0x3e9ba378 enqueuing iobuf 0x3e9c55f8 on vq 0
vring_add_buf: 0x3e9be02a  6 -  4  2  0 14 12
VIRTIO-NET 0x3e9ba378 rx complete iobuf 0x3e9c65f8 len 52
VIRTIO-NET 0x3e9ba378 enqueuing iobuf 0x3e9c5df8 on vq 0
vring_add_buf: 0x3e9be02c  8 -  6  4  2  0 14
VIRTIO-NET 0x3e9ba378 rx complete iobuf 0x3e9c6df8 len 52
VIRTIO-NET 0x3e9ba378 enqueuing iobuf 0x3e9c65f8 on vq 0
vring_add_buf: 0x3e9be02e 10 -  8  6  4  2  0

When booting e1000 or rtl8139 I don't see any messages.

cheers,
  Gerd

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

[edk2] [Patch] NetworkPkg: Avoid potential NULL pointer dereference

2016-06-24 Thread Jiaxin Wu 
The commit of 6b16c9e7 removes ASSERT and use error handling
in IpSecDxe driver, but may cause the potential NULL pointer
dereference. So, this patch is used to avoid NULL pointer
dereference.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Zhang Lubo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/IpSecDxe/IkePacket.c  |   6 +-
 NetworkPkg/IpSecDxe/Ikev2/ChildSa.c  |  19 ++--
 NetworkPkg/IpSecDxe/Ikev2/Exchange.c |  10 ++-
 NetworkPkg/IpSecDxe/Ikev2/Payload.c  |   3 +
 NetworkPkg/IpSecDxe/Ikev2/Sa.c   | 163 ++-
 5 files changed, 188 insertions(+), 13 deletions(-)

diff --git a/NetworkPkg/IpSecDxe/IkePacket.c b/NetworkPkg/IpSecDxe/IkePacket.c
index 8fd395d..d5a938e 100644
--- a/NetworkPkg/IpSecDxe/IkePacket.c
+++ b/NetworkPkg/IpSecDxe/IkePacket.c
@@ -1,9 +1,9 @@
 /** @file
   IKE Packet related operation.
 
-  Copyright (c) 2010, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions 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.
@@ -201,11 +201,13 @@ IkeNetbufFromPacket (
 //
 // Convert Host order to Network order for IKE_PACKET header and payloads
 // Encryption payloads if needed
 //
 if (((IKEV2_SESSION_COMMON *) SessionCommon)->IkeVer == 2) {
-  Ikev2EncodePacket ((IKEV2_SESSION_COMMON *) SessionCommon, IkePacket, 
IkeType);
+  if (EFI_ERROR (Ikev2EncodePacket ((IKEV2_SESSION_COMMON *) 
SessionCommon, IkePacket, IkeType))) {
+return NULL;
+  }
 } else {
   //
   //If IKEv1 support, check it here.
   //
   return NULL;
diff --git a/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c 
b/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
index d3859e2..1f0199b 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
@@ -1,9 +1,9 @@
 /** @file
   The operations for Child SA.
 
-  Copyright (c) 2010, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions 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.
@@ -37,22 +37,25 @@ Ikev2CreateChildGenerator (
   IKEV2_CHILD_SA_SESSION  *ChildSaSession;
   IKEV2_SA_SESSION*IkeSaSession;
   IKE_PACKET  *IkePacket;
   IKE_PAYLOAD *NotifyPayload;
   UINT32  *MessageId;
+
+  NotifyPayload   = NULL;
+  MessageId   = NULL;
   
   ChildSaSession  = (IKEV2_CHILD_SA_SESSION *) SaSession;
-  IkePacket   = IkePacketAlloc();
-  MessageId   = NULL;
-
-  if (IkePacket == NULL) {
+  if (ChildSaSession == NULL) {
 return NULL;
   }
-  if (ChildSaSession == NULL) {
+
+  IkePacket   = IkePacketAlloc();
+  if (IkePacket == NULL) {
 return NULL;
   }
 
+
   if (Context != NULL) {
 MessageId = (UINT32 *) Context;
   }
   
   IkePacket->Header->Version  = (UINT8) (2 << 4);
@@ -111,10 +114,14 @@ Ikev2CreateChildGenerator (
 IKEV2_NOTIFICATION_NO_ADDITIONAL_SAS,
 NULL,
 NULL,
 0
 );
+  if (NotifyPayload == NULL) { 
+IkePacketFree (IkePacket);
+return NULL;
+  }
 
   IKE_PACKET_APPEND_PAYLOAD (IkePacket, NotifyPayload);
   //
   // TODO: Support the CREATE_CHILD_SA exchange. 
   // 
diff --git a/NetworkPkg/IpSecDxe/Ikev2/Exchange.c 
b/NetworkPkg/IpSecDxe/Ikev2/Exchange.c
index 9d58ab0a..1eddbfb 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Exchange.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/Exchange.c
@@ -1,9 +1,9 @@
 /** @file
   The general interfaces of the IKEv2.
 
-  Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions 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.
@@ -493,10 +493,14 @@ Ikev2HandleSa (
 //
 ASSERT (IsListEmpty (>ChildSaSessionList) &&
 IsListEmpty (>ChildSaEstablishSessionList));
 
 ChildSaSession = Ikev2ChildSaSessionCreate (IkeSaSession, UdpService);
+if (ChildSaSession == NULL) {
+  goto ON_ERROR;
+}
+
 ChildSaCommon  = >SessionCommon;
   }
 
   //
   // Parse the IKE request packet according to the auth method and current 
state.
@@ -517,10 +521,14 @@

[edk2] [patch v3] MdeModulePkg/SdMmc: update TPL to notify to fix UEFI SCT hang

2016-06-24 Thread Feng Tian 
Compared with v2, we add missing critical region protection to error handling
path of SdRwSingleBlock & SdRwMultiBlocks functions.

We also fix an issue found at v1 & v2, in which the enumeration timer should
work at TPL_CALLBACK level as DiskIo is required to be <= TPL_CALLBACK.

Compared with v1, we added critical region protection on queue access made
in EraseBlock support.

We have to upgrade the TPL level used by SdMmc stack because the
following flow:

DiskIo2ReadWriteDisk() in logical partition -> PartitionReadBlocksEx()
in logical partition at TPL callback level -> ProbeMediaStatusEx()
with sync request -> DiskIo2ReadWriteDisk() in physical partition ->
 waiting for async task completion.

if the low layer driver doesn't run at TPL_NOTIFY level, it will have
no time to trigger async task and cause system hang.

Cc: Hao Wu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian 
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  9 ++--
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   |  2 +-
 MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c  | 60 +-
 MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c  | 42 +++
 MdeModulePkg/Bus/Sd/SdDxe/SdDxe.c  |  2 +-
 5 files changed, 75 insertions(+), 40 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index ed6b557..0be081d 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -238,6 +238,7 @@ SdMmcPciHcEnumerateDevice (
   LIST_ENTRY  *Link;
   LIST_ENTRY  *NextLink;
   SD_MMC_HC_TRB   *Trb;
+  EFI_TPL OldTpl;
 
   Private = (SD_MMC_HC_PRIVATE_DATA*)Context;
 
@@ -251,6 +252,7 @@ SdMmcPciHcEnumerateDevice (
 //
 // Signal all async task events at the slot with EFI_NO_MEDIA status.
 //
+OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
 for (Link = GetFirstNode (>Queue);
  !IsNull (>Queue, Link);
  Link = NextLink) {
@@ -263,6 +265,7 @@ SdMmcPciHcEnumerateDevice (
 SdMmcFreeTrb (Trb);
   }
 }
+gBS->RestoreTPL (OldTpl);
 //
 // Notify the upper layer the connect state change through 
ReinstallProtocolInterface.
 //
@@ -665,7 +668,7 @@ SdMmcPciHcDriverBindingStart (
   //
   Status = gBS->CreateEvent (
   EVT_TIMER | EVT_NOTIFY_SIGNAL,
-  TPL_CALLBACK,
+  TPL_NOTIFY,
   ProcessAsyncTaskList,
   Private,
   >TimerEvent
@@ -961,7 +964,7 @@ SdMmcPassThruPassThru (
   // Wait async I/O list is empty before execute sync I/O operation.
   //
   while (TRUE) {
-OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
+OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
 if (IsListEmpty (>Queue)) {
   gBS->RestoreTPL (OldTpl);
   break;
@@ -1273,7 +1276,7 @@ SdMmcPassThruResetDevice (
   //
   // Free all async I/O requests in the queue
   //
-  OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
+  OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
 
   for (Link = GetFirstNode (>Queue);
!IsNull (>Queue, Link);
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index 8978182..b4ff2af 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -1315,7 +1315,7 @@ SdMmcCreateTrb (
   }
 
   if (Event != NULL) {
-OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
+OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
 InsertTailList (>Queue, >TrbList);
 gBS->RestoreTPL (OldTpl);
   }
diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c 
b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
index 5fe710d..fc705e1 100644
--- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
+++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
@@ -339,7 +339,7 @@ EmmcSetExtCsd (
   }
 
   SetExtCsdReq->Signature = EMMC_REQUEST_SIGNATURE;
-  OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
+  OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
   InsertTailList (>Queue, >Link);
   gBS->RestoreTPL (OldTpl);
   SetExtCsdReq->Packet.SdMmcCmdBlk= >SdMmcCmdBlk;
@@ -361,7 +361,7 @@ EmmcSetExtCsd (
   if ((Token != NULL) && (Token->Event != NULL)) {
 Status = gBS->CreateEvent (
 EVT_NOTIFY_SIGNAL,
-TPL_CALLBACK,
+TPL_NOTIFY,
 AsyncIoCallback,
 SetExtCsdReq,
 >Event
@@ -382,7 +382,7 @@ Error:
 // The request and event will be freed in asynchronous callback for 
success case.
 //
 if (EFI_ERROR (Status) && (SetExtCsdReq != NULL)) {
-  OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
+  OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
   RemoveEntryList (>Link);

Re: [edk2] iPXE invalidates the OVMF network device options

2016-06-24 Thread Gerd Hoffmann 
  Hi,

> > Assuming you can sneak an iPXE rebuild into the QEMU soft freeze, I
> > think it makes sense to wait a bit longer -- let's hope I can come up
> > with something sensible for that error too...
> 
> can you please retest with a fresh build of the current iPXE master (at
> 04186319181298083ef28695a8309028b26fe83c presently)? I can no longer
> reproduce the ASSERT -- the iPXE form can be entered just fine.
> 
> I don't know what changed. o_O
> 
> Michael committed an improved version of my patch, but I fail to see how
> that change could have been relevant for this... Michael's patch copies
> 6 bytes (rather than 20) into the target array, but I thought bytes in
> that source array past offset 5 were zeroed anyway.
> 
> Anyway, if it looks all good to you as well, then I think we can call
> this solved, and Gerd could go ahead with the rebuild.

Built updates today and ran a bunch of tests.

First thing I've noticed is that the virtio-net driver floods the
terminal with debug messages for each and every package.  That is
clearly a nonstarter and must be fixed.

Second thing I've noticed is that automatic network boot doesn't work
with ipxe (i.e. -device $nic,bootindex=1).  When going into the boot
menu in ovmf I find three entries now:  "EFI Network", ranked first,
doesn't work.  Returns to the boot menu instantly, without any network
activity.  Then "UEFI PXEv4" and "UEFI HTTPv4" at the bottom of the
list.  Selecting the "UEFI PXEv4" works as expected.  Selecting "UEFI
HTTPv4" has no visible effect, but at least I see dhcp requests on the
wire so I guess with the correct server side configuration this one
would work too.  That happens with both old and new ipxe, so it is
probably related to the recent network boot updates in edk2.

cheers,
  Gerd

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