Re: [edk2] [Patch] ShellPkg: Add error message if failed to place receive token in ping command.

2017-11-14 Thread Ye, Ting 
Reviewed-by: Ye Ting  

-Original Message-
From: Fu, Siyuan 
Sent: Wednesday, November 15, 2017 12:10 PM
To: edk2-devel@lists.01.org
Cc: Wu, Jiaxin ; Ye, Ting ; Ni, Ruiyu 

Subject: [Patch] ShellPkg: Add error message if failed to place receive token 
in ping command.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
Cc: Wu Jiaxin 
Cc: Ye Ting 
Cc: Ruiyu Ni 
---
 ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c | 9 +++--
 .../UefiShellNetwork1CommandsLib.uni | 1 +
 ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c| 9 +++--
 .../UefiShellNetwork2CommandsLib.uni | 1 +
 4 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c 
b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
index ca5c22a..10d291c 100644
--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
@@ -2,7 +2,7 @@
   The implementation for Ping shell command.
 
   (C) Copyright 2015 Hewlett-Packard Development Company, L.P.
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2017, Intel Corporation. All rights 
+ reserved.
   (C) Copyright 2016 Hewlett Packard Enterprise Development LP
 
   This program and the accompanying materials @@ -629,6 +629,7 @@ ON_EXIT:
 Status = Private->ProtocolPointers.Receive (Private->IpProtocol, 
>RxToken);
 
 if (EFI_ERROR (Status)) {
+  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_PING_RECEIVE), 
+ gShellNetwork1HiiHandle, Status);
   Private->Status = EFI_ABORTED;
 }
   } else {
@@ -828,7 +829,11 @@ Ping6ReceiveEchoReply (
 
   Private->RxToken.Status = EFI_NOT_READY;
 
-  return (Private->ProtocolPointers.Receive (Private->IpProtocol, 
>RxToken));
+  Status = Private->ProtocolPointers.Receive (Private->IpProtocol, 
+ >RxToken);  if (EFI_ERROR (Status)) {
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_PING_RECEIVE), 
+ gShellNetwork1HiiHandle, Status);  }  return Status;
 }
 
 /**
diff --git 
a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
 
b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
index 70c3465..7a43ad5 100644
--- 
a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Com
+++ mandsLib.uni
@@ -50,6 +50,7 @@
 #string STR_PING_CONFIG  #language en-US "Config %r\r\n"
 #string STR_PING_GETMODE #language en-US "GetModeData %r\r\n"
 #string STR_PING_GETDATA #language en-US "GetData %r\r\n"
+#string STR_PING_RECEIVE #language en-US "Receive %r\r\n"
 #string STR_PING_SEND_REQUEST#language en-US "Echo request sequence %d 
did not complete successfully.\r\n"
 #string STR_PING_NOSOURCE_INDO   #language en-US "There are no sources in 
%s's multicast domain.\r\n"
 #string STR_PING_NETWORK_ERROR   #language en-US "%H%s%N: Network function 
failed with %r\r\n"
diff --git a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c 
b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
index 2961fd7..b784696 100644
--- a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
+++ b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
@@ -1,7 +1,7 @@
 /** @file
   The implementation for Ping6 application.
 
-  Copyright (c) 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2016 - 2017, 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 @@ -474,6 +474,7 @@ ON_EXIT:
 Status = Private->Ip6->Receive (Private->Ip6, RxToken);
 
 if (EFI_ERROR (Status)) {
+  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
+ (STR_PING6_IP6_RECEIVE), gShellNetwork2HiiHandle, Status);
   Private->Status = EFI_ABORTED;
 }
   } else {
@@ -658,7 +659,11 @@ Ping6OnReceiveEchoReply (
 
   Private->RxToken.Status = EFI_NOT_READY;
 
-  return Private->Ip6->Receive (Private->Ip6, >RxToken);
+  Status = Private->Ip6->Receive (Private->Ip6, >RxToken);  if 
+ (EFI_ERROR (Status)) {
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
+ (STR_PING6_IP6_RECEIVE), gShellNetwork2HiiHandle, Status);  }  return 
+ Status;
 }
 
 /**
diff --git 
a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.uni
 
b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.uni
index 2b2327b..9a535ad 100644
--- 
a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.uni
+++ b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Com
+++ mandsLib.uni
@@

Re: [edk2] [Patch] MdeModulePkg/SNP: remove redundant DEBUG print in SNP Transmit.c

2017-11-14 Thread Ye, Ting 
Reviewed-by: Ye Ting  

-Original Message-
From: Fu, Siyuan 
Sent: Wednesday, November 15, 2017 11:15 AM
To: edk2-devel@lists.01.org
Cc: Wu, Jiaxin ; Ye, Ting 
Subject: [Patch] MdeModulePkg/SNP: remove redundant DEBUG print in SNP 
Transmit.c

This patch is to remove some redundant DEBUG output in SNP transmit function.
In case of return EFI_NOT_READY in PxeTransmit, the SNP driver is indicate the 
caller that the transmit queue is full, it's a very common situation druing 
transmit, not a critical error. So the patch move the DEBUG lever to EFI_D_NET.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
Cc: Wu Jiaxin 
Cc: Ye Ting 
---
 MdeModulePkg/Universal/Network/SnpDxe/Transmit.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Transmit.c 
b/MdeModulePkg/Universal/Network/SnpDxe/Transmit.c
index 73461bc..2c7083e 100644
--- a/MdeModulePkg/Universal/Network/SnpDxe/Transmit.c
+++ b/MdeModulePkg/Universal/Network/SnpDxe/Transmit.c
@@ -1,7 +1,7 @@
 /** @file
 Implementation of transmitting a packet.
  
-Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2004 - 2017, 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 @@ -186,7 +186,6 @@ 
PxeTransmit (
   (*Snp->IssueUndi32Command) ((UINT64) (UINTN) >Cdb);
 
   DEBUG ((EFI_D_NET, "\nexit Snp->undi.transmit()  "));
-  DEBUG ((EFI_D_NET, "\nSnp->Cdb.StatCode == %r", Snp->Cdb.StatCode));
 
   //
   // we will unmap the buffers in get_status call, not here @@ -199,19 +198,24 
@@ PxeTransmit (
   case PXE_STATCODE_QUEUE_FULL:
   case PXE_STATCODE_BUSY:
 Status = EFI_NOT_READY;
+DEBUG (
+  (EFI_D_NET,
+  "\nSnp->undi.transmit()  %xh:%xh\n",
+  Snp->Cdb.StatFlags,
+  Snp->Cdb.StatCode)
+  );
 break;
 
   default:
+DEBUG (
+  (EFI_D_ERROR,
+  "\nSnp->undi.transmit()  %xh:%xh\n",
+  Snp->Cdb.StatFlags,
+  Snp->Cdb.StatCode)
+  );
 Status = EFI_DEVICE_ERROR;
   }
 
-  DEBUG (
-(EFI_D_ERROR,
-"\nSnp->undi.transmit()  %xh:%xh\n",
-Snp->Cdb.StatFlags,
-Snp->Cdb.StatCode)
-);
-
   return Status;
 }
 
--
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 v5] UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in memory map

2017-11-14 Thread Zeng, Star 
How about the code to filter out paging capabilities from UEFI memory map in 
Page.c CoreGetMemoryMap(), then with maximum compatibility, the UEFI memory map 
could be same with before 14dde9e903bb9a719ebb8f3381da72b19509bc36 
[MdeModulePkg/Core: Fix out-of-sync issue in GCD].

Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wang, 
Jian J
Sent: Tuesday, November 14, 2017 10:36 PM
To: Laszlo Ersek 
Cc: Matt Fleming ; edk2-devel@lists.01.org; Yao, 
Jiewen ; Dong, Eric ; Ard Biesheuvel 

Subject: Re: [edk2] [PATCH v5] UefiCpuPkg/CpuDxe: Fix multiple entries of 
RT_CODE in memory map

Hi Laszlo,

I did some investigation works on this issue. I think I found the root cause 
and tend to believe this is a Fedora kernel issue. Here're proves:

memmap output patterns in which Fedora 26 failed to boot:
1) BS_DataAE20-AE20EFFF 000F 
0002600F
RT_DataAE20F000-AE38EFFF 0180 
8002600F
RT_CodeAE38F000-AE48EFFF 0100 
8002600F
Reserved   AE48F000-AE58EFFF 0100 
0002600F

2) BS_DataAE20-AE20EFFF 000F 
000F
RT_DataAE20F000-AE38EFFF 0180 
800F
RT_CodeAE38F000-AE467FFF 00D9 
800F
RT_CodeAE468000-AE469FFF 0002 
8002600F
RT_CodeAE46A000-AE46EFFF 0005 
800F
RT_CodeAE46F000-AE470FFF 0002 
8002600F
RT_CodeAE471000-AE475FFF 0005 
800F
RT_CodeAE476000-AE479FFF 0004 
8002600F
RT_CodeAE47A000-AE47 0006 
800F
RT_CodeAE48-AE481FFF 0002 
8002600F
RT_CodeAE482000-AE487FFF 0006 
800F
RT_CodeAE488000-AE489FFF 0002 
8002600F
RT_CodeAE48A000-AE48EFFF 0005 
800F
Reserved   AE48F000-AE58EFFF 0100 
000F

3) RT_DataAE20F000-AE38EFFF 0180 
800F
RT_CodeAE38F000-AE418FFF 008A 
800F
RT_CodeAE419000-AE48EFFF 0076 
8002600F
Reserved   AE48F000-AE58EFFF 0100 
000F

4) BS_DataAE20-AE20EFFF 000F 
0002400F
RT_DataAE20F000-AE38EFFF 0180 
8002400F
RT_CodeAE38F000-AE48EFFF 0100 
8002400F
Reserved   AE48F000-AE58EFFF 0100 
0002400F


memmap output pattern in which Fedora 26 booted successfully
5) BS_DataAE20-AE20EFFF 000F 
000F
RT_DataAE20F000-AE38EFFF 0180 
800F
RT_CodeAE38F000-AE48EFFF 0100 
800F
Reserved   AE48F000-AE58EFFF 0100 
000F

6) BS_DataAE20-AE20EFFF 000F 
000F
RT_DataAE20F000-AE38EFFF 0180 
800F
RT_CodeAE38F000-AE418FFF 008A 
800F
RT_CodeAE419000-AE419FFF 0001 
8000400F
RT_CodeAE41A000-AE41BFFF 0002 
8002000F
RT_CodeAE41C000-AE420FFF 0005 
8000400F
RT_CodeAE421000-AE422FFF 0002 
8002000F
RT_CodeAE423000-AE427FFF 0005 
8000400F
RT_CodeAE428000-AE42AFFF 0003 
8002000F
RT_CodeAE42B000-AE42 0005 
8000400F
RT_CodeAE43-AE432FFF 0003 
8002000F
RT_CodeAE433000-AE439FFF 0007 
8000400F
RT_CodeAE43A000-AE43DFFF 0004 
8002000F
RT_CodeAE43E000-AE444FFF 0007 
8000400F
RT_CodeAE445000-AE448FFF 0004 
8002000F
RT_CodeAE449000-AE44EFFF 0006 
8000400F
RT_CodeAE44F000-AE450FFF 0002 
8002000F

Re: [edk2] [Patch] ShellPkg: Add error message if failed to place receive token in ping command.

2017-11-14 Thread Wu, Jiaxin 
Reviewed-by: Wu Jiaxin 


> -Original Message-
> From: Fu, Siyuan
> Sent: Wednesday, November 15, 2017 12:10 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin ; Ye, Ting ; Ni,
> Ruiyu 
> Subject: [Patch] ShellPkg: Add error message if failed to place receive token
> in ping command.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan 
> Cc: Wu Jiaxin 
> Cc: Ye Ting 
> Cc: Ruiyu Ni 
> ---
>  ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c | 9
> +++--
>  .../UefiShellNetwork1CommandsLib.uni | 1 +
>  ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c| 9
> +++--
>  .../UefiShellNetwork2CommandsLib.uni | 1 +
>  4 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> index ca5c22a..10d291c 100644
> --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> @@ -2,7 +2,7 @@
>The implementation for Ping shell command.
> 
>(C) Copyright 2015 Hewlett-Packard Development Company, L.P.
> -  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
> +  Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
>(C) Copyright 2016 Hewlett Packard Enterprise Development LP
> 
>This program and the accompanying materials
> @@ -629,6 +629,7 @@ ON_EXIT:
>  Status = Private->ProtocolPointers.Receive (Private->IpProtocol, 
> 
> >RxToken);
> 
>  if (EFI_ERROR (Status)) {
> +  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_PING_RECEIVE),
> gShellNetwork1HiiHandle, Status);
>Private->Status = EFI_ABORTED;
>  }
>} else {
> @@ -828,7 +829,11 @@ Ping6ReceiveEchoReply (
> 
>Private->RxToken.Status = EFI_NOT_READY;
> 
> -  return (Private->ProtocolPointers.Receive (Private->IpProtocol, 
> >RxToken));
> +  Status = Private->ProtocolPointers.Receive (Private->IpProtocol, 
> >RxToken);
> +  if (EFI_ERROR (Status)) {
> +ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_PING_RECEIVE),
> gShellNetwork1HiiHandle, Status);
> +  }
> +  return Status;
>  }
> 
>  /**
> diff --git
> a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Com
> mandsLib.uni
> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Com
> mandsLib.uni
> index 70c3465..7a43ad5 100644
> ---
> a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Com
> mandsLib.uni
> +++
> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Com
> mandsLib.uni
> @@ -50,6 +50,7 @@
>  #string STR_PING_CONFIG  #language en-US "Config %r\r\n"
>  #string STR_PING_GETMODE #language en-US
> "GetModeData %r\r\n"
>  #string STR_PING_GETDATA #language en-US "GetData %r\r\n"
> +#string STR_PING_RECEIVE #language en-US "Receive %r\r\n"
>  #string STR_PING_SEND_REQUEST#language en-US "Echo request
> sequence %d did not complete successfully.\r\n"
>  #string STR_PING_NOSOURCE_INDO   #language en-US "There are no
> sources in %s's multicast domain.\r\n"
>  #string STR_PING_NETWORK_ERROR   #language en-US "%H%s%N:
> Network function failed with %r\r\n"
> diff --git a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
> b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
> index 2961fd7..b784696 100644
> --- a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
> +++ b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
> @@ -1,7 +1,7 @@
>  /** @file
>The implementation for Ping6 application.
> 
> -  Copyright (c) 2016, Intel Corporation. All rights reserved.
> +  Copyright (c) 2016 - 2017, 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
> @@ -474,6 +474,7 @@ ON_EXIT:
>  Status = Private->Ip6->Receive (Private->Ip6, RxToken);
> 
>  if (EFI_ERROR (Status)) {
> +  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_PING6_IP6_RECEIVE),
> gShellNetwork2HiiHandle, Status);
>Private->Status = EFI_ABORTED;
>  }
>} else {
> @@ -658,7 +659,11 @@ Ping6OnReceiveEchoReply (
> 
>Private->RxToken.Status = EFI_NOT_READY;
> 
> -  return Private->Ip6->Receive (Private->Ip6, >RxToken);
> +  Status = Private->Ip6->Receive (Private->Ip6, >RxToken);
> +  if (EFI_ERROR (Status)) {
> +ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_PING6_IP6_RECEIVE),
> gShellNetwork2HiiHandle, Status);
> +  }
> +  return Status;
>  }
> 
>  /**
> diff --git
> a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Com
> mandsLib.uni
>

Re: [edk2] [Patch V2] NetworkPkg: Print error message to screen if error occurs during HTTP boot.

2017-11-14 Thread Wu, Jiaxin 
Reviewed-by: Wu Jiaxin 


> -Original Message-
> From: Fu, Siyuan
> Sent: Wednesday, November 15, 2017 9:52 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin ; Ye, Ting 
> Subject: [Patch V2] NetworkPkg: Print error message to screen if error occurs
> during HTTP boot.
> 
> V2 update:
> Add error message is URI address is not correct.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan 
> Cc: Wu Jiaxin 
> Cc: Ye Ting 
> ---
>  NetworkPkg/HttpBootDxe/HttpBootImpl.c| 21
> +
>  NetworkPkg/HttpBootDxe/HttpBootSupport.c |  2 ++
>  NetworkPkg/HttpDxe/HttpImpl.c|  1 +
>  3 files changed, 24 insertions(+)
> 
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootImpl.c
> b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
> index 06a8a6a..d591db5 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootImpl.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
> @@ -327,6 +327,7 @@ HttpBootLoadFile (
>  //
>  Status = HttpBootDiscoverBootInfo (Private);
>  if (EFI_ERROR (Status)) {
> +  AsciiPrint ("\n  Error: Could not discover the boot information for 
> DHCP
> server.\n");
>goto ON_EXIT;
>  }
>}
> @@ -369,6 +370,7 @@ HttpBootLoadFile (
>   >ImageType
>   );
>if (EFI_ERROR (Status) && Status != EFI_BUFFER_TOO_SMALL) {
> +AsciiPrint ("\n  Error: Could not retrieve NBP file size from HTTP
> server.\n");
>  goto ON_EXIT;
>}
>  }
> @@ -394,6 +396,22 @@ HttpBootLoadFile (
> 
>  ON_EXIT:
>HttpBootUninstallCallback (Private);
> +
> +  if (Status == EFI_ACCESS_DENIED) {
> +AsciiPrint ("\n  Error: Could not establish connection with HTTP 
> server.\n");
> +  } else if (Status == EFI_BUFFER_TOO_SMALL && Buffer != NULL) {
> +AsciiPrint ("\n  Error: Buffer size is smaller than the requested 
> file.\n");
> +  } else if (Status == EFI_OUT_OF_RESOURCES) {
> +AsciiPrint ("\n  Error: Could not allocate I/O buffers.\n");
> +  } else if (Status == EFI_DEVICE_ERROR) {
> +AsciiPrint ("\n  Error: Network device error.\n");
> +  } else if (Status == EFI_TIMEOUT) {
> +AsciiPrint ("\n  Error: Server response timeout.\n");
> +  } else if (Status == EFI_ABORTED) {
> +AsciiPrint ("\n  Error: Remote boot cancelled.\n");
> +  } else if (Status != EFI_BUFFER_TOO_SMALL) {
> +AsciiPrint ("\n  Error: Unexpected network error.\n");
> +  }
>return Status;
>  }
> 
> @@ -555,6 +573,7 @@ HttpBootDxeLoadFile (
>MediaPresent = TRUE;
>NetLibDetectMedia (Private->Controller, );
>if (!MediaPresent) {
> +AsciiPrint ("\n  Error: Could not detect network connection.\n");
>  return EFI_NO_MEDIA;
>}
> 
> @@ -595,6 +614,8 @@ HttpBootDxeLoadFile (
>  Status = HttpBootRegisterRamDisk (Private, *BufferSize, Buffer,
> ImageType);
>  if (!EFI_ERROR (Status)) {
>Status = EFI_WARN_FILE_SYSTEM;
> +} else {
> +  AsciiPrint ("\n  Error: Could not register RAM disk to the system.\n");
>  }
>}
> 
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootSupport.c
> b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
> index d508e2c..7ca48f3 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootSupport.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
> @@ -1093,6 +1093,7 @@ HttpBootCheckUriScheme (
>// Return EFI_INVALID_PARAMETER if the URI is not HTTP or HTTPS.
>//
>if ((AsciiStrnCmp (Uri, "http://;, 7) != 0) && (AsciiStrnCmp (Uri, 
> "https://;,
> 8) != 0)) {
> +AsciiPrint ("\n  Error: Invalid URI address.\n");
>  DEBUG ((EFI_D_ERROR, "HttpBootCheckUriScheme: Invalid Uri.\n"));
>  return EFI_INVALID_PARAMETER;
>}
> @@ -1101,6 +1102,7 @@ HttpBootCheckUriScheme (
>// HTTP is disabled, return EFI_ACCESS_DENIED if the URI is HTTP.
>//
>if (!PcdGetBool (PcdAllowHttpConnections) && (AsciiStrnCmp (Uri,
> "http://;, 7) == 0)) {
> +AsciiPrint ("\n  Error: Access forbidden, only HTTPS connection is
> allowed.\n");
>  DEBUG ((EFI_D_ERROR, "HttpBootCheckUriScheme: HTTP is disabled.\n"));
>  return EFI_ACCESS_DENIED;
>}
> diff --git a/NetworkPkg/HttpDxe/HttpImpl.c
> b/NetworkPkg/HttpDxe/HttpImpl.c
> index 46d0323..57fa39f 100644
> --- a/NetworkPkg/HttpDxe/HttpImpl.c
> +++ b/NetworkPkg/HttpDxe/HttpImpl.c
> @@ -523,6 +523,7 @@ EfiHttpRequest (
> 
>FreePool (HostNameStr);
>if (EFI_ERROR (Status)) {
> +DEBUG ((EFI_D_ERROR, "Error: Could not retrieve the host address
> from DNS server.\n"));
>  goto Error1;
>}
>  }
> --
> 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] NetworkPkg: Fix incorrect SizeofHeaders returned from HttpTcpReceiveHeader().

2017-11-14 Thread Wu, Jiaxin 
Reviewed-by: Wu Jiaxin 


> -Original Message-
> From: Fu, Siyuan
> Sent: Wednesday, November 15, 2017 10:45 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin ; Ye, Ting ;
> Karunakar P 
> Subject: [Patch] NetworkPkg: Fix incorrect SizeofHeaders returned from
> HttpTcpReceiveHeader().
> 
> This patch is to fix a bug that the HttpTcpReceiveHeader() may return
> incorrect
> SizeofHeaders, which will include some already received message-body.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan 
> Cc: Wu Jiaxin 
> Cc: Ye Ting 
> Cc: Karunakar P 
> ---
>  NetworkPkg/HttpDxe/HttpProto.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/NetworkPkg/HttpDxe/HttpProto.c
> b/NetworkPkg/HttpDxe/HttpProto.c
> index ab00f3d..1aa1816 100644
> --- a/NetworkPkg/HttpDxe/HttpProto.c
> +++ b/NetworkPkg/HttpDxe/HttpProto.c
> @@ -1876,7 +1876,10 @@ HttpTcpReceiveHeader (
>//
>// Check whether we received end of HTTP headers.
>//
> -  *EndofHeader = AsciiStrStr (*HttpHeaders, HTTP_END_OF_HDR_STR);
> +  *EndofHeader = AsciiStrStr (*HttpHeaders, HTTP_END_OF_HDR_STR);
> +  if (*EndofHeader != NULL) {
> +*SizeofHeaders = *EndofHeader - *HttpHeaders;
> +  }
>  };
> 
>  //
> @@ -1976,6 +1979,9 @@ HttpTcpReceiveHeader (
>// Check whether we received end of HTTP headers.
>//
>*EndofHeader = AsciiStrStr (*HttpHeaders, HTTP_END_OF_HDR_STR);
> +  if (*EndofHeader != NULL) {
> +*SizeofHeaders = *EndofHeader - *HttpHeaders;
> +  }
>  };
> 
>  //
> --
> 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/SNP: remove redundant DEBUG print in SNP Transmit.c

2017-11-14 Thread Wu, Jiaxin 
Reviewed-by: Wu Jiaxin 


> -Original Message-
> From: Fu, Siyuan
> Sent: Wednesday, November 15, 2017 11:15 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin ; Ye, Ting 
> Subject: [Patch] MdeModulePkg/SNP: remove redundant DEBUG print in
> SNP Transmit.c
> 
> This patch is to remove some redundant DEBUG output in SNP transmit
> function.
> In case of return EFI_NOT_READY in PxeTransmit, the SNP driver is indicate
> the caller that the transmit queue is full, it's a very common situation 
> druing
> transmit, not a critical error. So the patch move the DEBUG lever to
> EFI_D_NET.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan 
> Cc: Wu Jiaxin 
> Cc: Ye Ting 
> ---
>  MdeModulePkg/Universal/Network/SnpDxe/Transmit.c | 22
> +-
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Transmit.c
> b/MdeModulePkg/Universal/Network/SnpDxe/Transmit.c
> index 73461bc..2c7083e 100644
> --- a/MdeModulePkg/Universal/Network/SnpDxe/Transmit.c
> +++ b/MdeModulePkg/Universal/Network/SnpDxe/Transmit.c
> @@ -1,7 +1,7 @@
>  /** @file
>  Implementation of transmitting a packet.
> 
> -Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
> +Copyright (c) 2004 - 2017, 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
> @@ -186,7 +186,6 @@ PxeTransmit (
>(*Snp->IssueUndi32Command) ((UINT64) (UINTN) >Cdb);
> 
>DEBUG ((EFI_D_NET, "\nexit Snp->undi.transmit()  "));
> -  DEBUG ((EFI_D_NET, "\nSnp->Cdb.StatCode == %r", Snp->Cdb.StatCode));
> 
>//
>// we will unmap the buffers in get_status call, not here
> @@ -199,19 +198,24 @@ PxeTransmit (
>case PXE_STATCODE_QUEUE_FULL:
>case PXE_STATCODE_BUSY:
>  Status = EFI_NOT_READY;
> +DEBUG (
> +  (EFI_D_NET,
> +  "\nSnp->undi.transmit()  %xh:%xh\n",
> +  Snp->Cdb.StatFlags,
> +  Snp->Cdb.StatCode)
> +  );
>  break;
> 
>default:
> +DEBUG (
> +  (EFI_D_ERROR,
> +  "\nSnp->undi.transmit()  %xh:%xh\n",
> +  Snp->Cdb.StatFlags,
> +  Snp->Cdb.StatCode)
> +  );
>  Status = EFI_DEVICE_ERROR;
>}
> 
> -  DEBUG (
> -(EFI_D_ERROR,
> -"\nSnp->undi.transmit()  %xh:%xh\n",
> -Snp->Cdb.StatFlags,
> -Snp->Cdb.StatCode)
> -);
> -
>return Status;
>  }
> 
> --
> 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][edk2-platforms/devel-MinnowBoard3-UDK2017] BensonGlacier: Enable generic SPI device

2017-11-14 Thread Wei, David 
Reviewed-by: zwei4  

Thanks,
David  Wei

Intel SSG/STO/UEFI BIOS 

> -Original Message-
> From: Rytkonen, Teemu S
> Sent: Wednesday, November 15, 2017 1:16 AM
> To: edk2-devel@lists.01.org
> Cc: Ryu, Misun ; Loeppert, Anthony
> ; Jones, Mark L ;
> Wei, David ; Guo, Mang 
> Subject: [Patch][edk2-platforms/devel-MinnowBoard3-UDK2017]
> BensonGlacier: Enable generic SPI device
> 
> -Enable generic SPI device for BensonGlacier config to be
> used with SenseHat board.
> -Enable GPIO config to enable SenseHat board programming
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Teemu Rytkonen 
> ---
>  .../BensonGlacier/BoardInitPostMem/BoardGpios.h| 20 ++--
>  .../AcpiTablesPCAT/PlatformSsdt/PlatformSsdt.asl   |  2 ++
>  .../PlatformSsdt/Sensors/GenericSpi3.asl   | 38
> ++
>  3 files changed, 50 insertions(+), 10 deletions(-)
>  create mode 100644
> Platform/BroxtonPlatformPkg/Common/Acpi/AcpiTablesPCAT/PlatformSsdt
> /Sensors/GenericSpi3.asl
> 
> diff --git
> a/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/B
> oardGpios.h
> b/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/B
> oardGpios.h
> index d72cd80c9..db48c4e85 100644
> ---
> a/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/B
> oardGpios.h
> +++
> b/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/B
> oardGpios.h
> @@ -80,13 +80,13 @@ BXT_GPIO_PAD_INIT  mBenson_GpioInitData_N[] =
>BXT_GPIO_PAD_CONF(L"GPIO_14",  M1   ,NA, NA,  
> NA,
> NA   , Wake_Disabled, P_20K_L,NA   ,NA ,NA   , NA,
> GPIO_PADBAR+0x0070,  NORTH),//Feature: LB
>BXT_GPIO_PAD_CONF(L"GPIO_15",  M1   ,NA, NA,  
> NA,
> NA   , Wake_Disabled, P_20K_L,NA   ,NA ,NA   , NA,
> GPIO_PADBAR+0x0078,  NORTH),//Feature: LB
>BXT_GPIO_PAD_CONF(L"GPIO_16",  M0   ,GPI   ,  NA   ,  
> NA,
> Edge , Wake_Disabled, P_20K_H, Inverted,IOAPIC,  HizRx0I ,DisPuPd,
> GPIO_PADBAR+0x0080,  NORTH),//Feature:SIM Card DetectNet in Sch:
> SIM_CON_CD1, falling edge trigger
> -  BXT_GPIO_PAD_CONF(L"GPIO_17",  M0   ,GPI   , GPIO_D,  
> NA,
> Edge , Wake_Disabled, P_NONE ,NA   ,IOAPIC, NA   ,DisPuPd,
> GPIO_PADBAR+0x0088, NORTH), // SOC_LSE_HOST_IRQ_N
> +  BXT_GPIO_PAD_CONF(L"GPIO_17",  M0   ,GPI   , GPIO_D,  
> NA,
> Edge , Wake_Disabled, P_NONE ,NA   ,IOAPIC, NA   ,DisPuPd,
> GPIO_PADBAR+0x0088, NORTH), // SOC_LSE_HOST_IRQ_N
>BXT_GPIO_PAD_CONF(L"GPIO_18",  M1   ,NA, NA,  
> NA,
> NA   , Wake_Disabled, P_20K_H,NA   ,NA, NA   , NA,
> GPIO_PADBAR+0x0090,  NORTH),//Feature: LB
>BXT_GPIO_PAD_CONF(L"GPIO_19",  M1   ,NA, NA,  
> NA,
> NA   , Wake_Disabled, P_20K_H,NA   ,NA, NA   , NA,
> GPIO_PADBAR+0x0098,  NORTH),//Feature: LB
>BXT_GPIO_PAD_CONF(L"GPIO_20",  M1   ,NA, NA,  
> NA,
> NA   , Wake_Disabled, P_20K_L,NA   ,NA, NA   , NA,
> GPIO_PADBAR+0x00A0,  NORTH),//Feature: LB
>BXT_GPIO_PAD_CONF(L"GPIO_21",  M1   ,NA, NA,  
> NA,
> NA   , Wake_Disabled, P_20K_L,NA   ,NA, NA   , NA,
> GPIO_PADBAR+0x00A8,  NORTH),//Feature: LB
>BXT_GPIO_PAD_CONF(L"GPIO_22",  M0   ,GPIO  ,GPIO_D ,  
> NA,
> NA   , Wake_Disabled, P_20K_L,NA   ,NA, NA   , NA,
> GPIO_PADBAR+0x00B0,  NORTH),//Feature: LB
> -  BXT_GPIO_PAD_CONF(L"GPIO_23",  M0   ,GPO   ,GPIO_D,   
> LO,
> NA   , Wake_Disabled, P_20K_H,NA   ,NA, NA   ,   EnPu,
> GPIO_PADBAR+0x00B8, NORTH), // SOC_SUE_RST_N
> +  BXT_GPIO_PAD_CONF(L"GPIO_23",  M0   ,GPO   ,GPIO_D,   
> LO,
> NA   , Wake_Disabled, P_20K_H,NA   ,NA, NA   ,   EnPu,
> GPIO_PADBAR+0x00B8, NORTH), // SOC_SUE_RST_N
>BXT_GPIO_PAD_CONF(L"GPIO_24",  M0   ,GPO   ,GPIO_D ,  
> NA,
> NA   , Wake_Disabled, P_20K_H,   NA,NA, NA   , NA,
> GPIO_PADBAR+0x00C0,  NORTH),//SATA_DEVSLP0
>BXT_GPIO_PAD_CONF(L"GPIO_25",  M0   ,GPO   ,GPIO_D ,  
> NA,
> Level, Wake_Disabled, P_20K_H, Inverted,   SCI, NA   , NA,
> GPIO_PADBAR+0x00C8,  NORTH),//Feature:ODD MD/DA SCI  Net in Sch:
> SATA_ODD_DA_IN
>BXT_GPIO_PAD_CONF(L"GPIO_26",  M0   ,GPIO  ,GPIO_D ,  
> NA,
> NA   , Wake_Disabled, P_20K_L,   NA,NA, NA   , NA,
> GPIO_PADBAR+0x00D0,  NORTH),//SATA_LEDN
> @@ -216,16 +216,16 @@ BXT_GPIO_PAD_INIT

[edk2] [Patch][edk2-platforms/devel-MinnowBoard3-UDK2017] SueCreek Bypass

2017-11-14 Thread zwei4 
Add code in ACPI table for TI audio codec under I2C5.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: zwei4 
---
 .../BensonGlacier/BoardInitPostMem/BoardInit.c |  33 +++-
 .../BoardInitPostMem/BoardInitPostMem.inf  |   1 +
 .../Common/Acpi/AcpiPlatformDxe/AcpiPlatform.c |   4 +-
 .../Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf   |   1 +
 .../Common/Acpi/AcpiTablesPCAT/GloblNvs.asl|   1 +
 .../PlatformSsdt/Audio/AudioCodec10TI3100.asl  |  44 +
 .../AcpiTablesPCAT/PlatformSsdt/PlatformSsdt.asl   |   2 +
 .../Common/Include/Guid/SetupVariable.h|   3 +-
 .../PlatformSetupDxe/SouthClusterConfig.vfi|   7 
 .../PlatformSettings/PlatformSetupDxe/UqiList.uni  | Bin 126596 -> 126916 bytes
 .../PlatformSetupDxe/VfrStrings.uni| Bin 305486 -> 305886 bytes
 Platform/BroxtonPlatformPkg/PlatformPkg.dec|   3 +-
 .../NorthCluster/Include/Protocol/GlobalNvsArea.h  |   3 +-
 13 files changed, 96 insertions(+), 6 deletions(-)
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/Acpi/AcpiTablesPCAT/PlatformSsdt/Audio/AudioCodec10TI3100.asl

diff --git 
a/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInit.c 
b/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInit.c
index 07246c155..324baf9ad 100644
--- 
a/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInit.c
+++ 
b/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInit.c
@@ -51,6 +51,29 @@ BensonGlacierPostMemInitCallback (
   UINT8ResetType;
   UINTNBufferSize;
   UINT8MaxPkgCState;
+  UINTNVariableSize;
+  EFI_PEI_READ_ONLY_VARIABLE2_PPI  *VariableServices;
+  SYSTEM_CONFIGURATION SystemConfiguration;
+
+  VariableSize = sizeof (SYSTEM_CONFIGURATION);
+  ZeroMem (, sizeof (SYSTEM_CONFIGURATION));
+
+ (*PeiServices)->LocatePpi (
+PeiServices,
+,
+0,
+NULL,
+(VOID **) 
+ );
+
+  VariableServices->GetVariable (
+  VariableServices,
+  PLATFORM_SETUP_VARIABLE_NAME,
+  ,
+  NULL,
+  ,
+  
+  );
 
   Status = PeiServicesLocatePpi (
  ,
@@ -91,7 +114,15 @@ BensonGlacierPostMemInitCallback (
   //
   // Set PcdSueCreek
   //
-  PcdSetBool (PcdSueCreek, TRUE);
+  if (SystemConfiguration.SueCreekBypass) {
+PcdSetBool (PcdSueCreek, FALSE);
+PcdSetBool (PcdTi3100AudioCodecEnable, TRUE);
+DEBUG ((EFI_D_INFO,  "Bypass SueCreek \n"));
+  } else {
+PcdSetBool (PcdSueCreek, TRUE);
+PcdSetBool (PcdTi3100AudioCodecEnable, FALSE); 
+DEBUG ((EFI_D_INFO,  "Use SueCreek \n"));
+  }
 
   //
   // Set PcdMaxPkgCState
diff --git 
a/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInitPostMem.inf
 
b/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInitPostMem.inf
index 55ec5b75f..e15e61293 100644
--- 
a/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInitPostMem.inf
+++ 
b/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardInitPostMem.inf
@@ -65,6 +65,7 @@
   gPlatformModuleTokenSpaceGuid.PcdBoardVbtFileGuid
   gPlatformModuleTokenSpaceGuid.PcdSueCreek
   gPlatformModuleTokenSpaceGuid.PcdMaxPkgCState
+  gPlatformModuleTokenSpaceGuid.PcdTi3100AudioCodecEnable
 
 [Guids]
   gEfiPlatformInfoGuid
diff --git 
a/Platform/BroxtonPlatformPkg/Common/Acpi/AcpiPlatformDxe/AcpiPlatform.c 
b/Platform/BroxtonPlatformPkg/Common/Acpi/AcpiPlatformDxe/AcpiPlatform.c
index d0c668ef5..c18753b61 100644
--- a/Platform/BroxtonPlatformPkg/Common/Acpi/AcpiPlatformDxe/AcpiPlatform.c
+++ b/Platform/BroxtonPlatformPkg/Common/Acpi/AcpiPlatformDxe/AcpiPlatform.c
@@ -1439,8 +1439,8 @@ AcpiPlatformEntryPoint (
 mGlobalNvsArea.Area->BatteryCapacity0   = 100;
 mGlobalNvsArea.Area->Mmio32Base = (MmioRead32 ((UINTN) 
PcdGet64 (PcdPciExpressBaseAddress) + 0xBC) & 0xFFF0);;
 mGlobalNvsArea.Area->Mmio32Length   = ACPI_MMIO_BASE_ADDRESS - 
mGlobalNvsArea.Area->Mmio32Base;
-mGlobalNvsArea.Area->SueCreekEnable   = 
PcdGetBool(PcdSueCreek);
-
+mGlobalNvsArea.Area->SueCreekEnable = PcdGetBool(PcdSueCreek);
+mGlobalNvsArea.Area->Ti3100AudioCodecEnable = 
PcdGetBool(PcdTi3100AudioCodecEnable );
 //
 // Initialize IGD state by checking if IGD Device 2 Function 0 is enabled 
in the chipset
 //
diff --git 
a/Platform/BroxtonPlatformPkg/Common/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf 
b/Platform/BroxtonPlatformPkg/Common/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf
index 9d451f598..21ce93822 100644
---

Re: [edk2] [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error check

2017-11-14 Thread Zeng, Star 
Mike,

UEFI spec, EFI_USB_IO_PROTOCOL.UsbControlTransfer(), in the description.

Thanks,
Star
-Original Message-
From: Kinney, Michael D 
Sent: Wednesday, November 15, 2017 12:00 PM
To: Zeng, Star ; edk2-devel@lists.01.org; Kinney, Michael 
D 
Cc: Dong, Eric 
Subject: RE: [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() 
error check

Star,

I am curious.  Which spec statement are you referring to?

Mike

> -Original Message-
> From: Zeng, Star
> Sent: Tuesday, November 14, 2017 6:59 PM
> To: Kinney, Michael D ; 
> edk2-devel@lists.01.org
> Cc: Dong, Eric ; Zeng, Star 
> Subject: RE: [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add
> UsbControlTransfer() error check
> 
> Mike,
> 
> I do not insist on the check that the Direction is not EfiUsbNoData, 
> and I know the functionality should have no improvement with the 
> check.
> So, you can have my Reviewed-by: Star Zeng .
> 
> I raised the question just for consideration from literally, as 
> according to the spec, the code could not touch DataLength at all when 
> Direction is EfiUsbNoData.
> 
> You can decide to have / have not the check when pushing. :)
> 
> 
> Thanks,
> Star
> -Original Message-
> From: Kinney, Michael D
> Sent: Wednesday, November 15, 2017 10:46 AM
> To: Zeng, Star ; edk2- de...@lists.01.org; 
> Kinney, Michael D 
> Cc: Dong, Eric 
> Subject: RE: [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add
> UsbControlTransfer() error check
> 
> Star,
> 
> For that case both DataLength and RequestedDataLength will be 0 and 
> the new path will not be taken.
> 
> Are you concerned that the USB HC Protocol could return a non zero 
> DataLength for the EfiUsbNoData case?  Even if it does, the new path 
> can never be taken because 0 is less than all UINTN values.
> 
> Mike
> 
> > -Original Message-
> > From: Zeng, Star
> > Sent: Tuesday, November 14, 2017 6:40 PM
> > To: Kinney, Michael D ; 
> > edk2-devel@lists.01.org
> > Cc: Dong, Eric ; Zeng, Star
> 
> > Subject: RE: [Patch V2 1/2] MdeModulePkg/UsbBusDxe:
> Add
> > UsbControlTransfer() error check
> >
> > Mike,
> >
> > Do you think it is better or not to also check the
> Direction is not
> > EfiUsbNoData?
> >
> > UEFI spec, EFI_USB_IO_PROTOCOL.UsbControlTransfer():
> > If the Direction parameter is EfiUsbNoData, Data is
> NULL, and
> > DataLength is 0, then no data phase exists for the
> control transfer.
> >
> > Thanks,
> > Star
> > -Original Message-
> > From: Kinney, Michael D
> > Sent: Wednesday, November 15, 2017 9:03 AM
> > To: edk2-devel@lists.01.org
> > Cc: Zeng, Star ; Dong, Eric
> 
> > Subject: [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add
> > UsbControlTransfer() error check
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=767
> >
> > The USB I/O Protocol function ControlTransfer() has a
> DataLength
> > parameter that specifies the size of the Data buffer.
> The UsbBusDxe
> > module implements the USB I/O Protocol using the
> services of the USB2
> > Host Controller Protocol.  The DataLength parameter
> in the
> > USB2 Host Controller Protocol ControlTransfer()
> service is an IN OUT
> > parameter so the number of bytes actually transferred
> is returned.
> > Since the USB I/O Protocol
> > ControlTransfer() service can not return the number
> of bytes actually
> > transferred, the only option if the number of bytes
> actually
> > transferred is less than the number of bytes
> requested is to return
> > EFI_DEVICE_ERROR.
> >
> > The change fixes an issue with a USB mass storage
> device that responds
> > with 0 bytes to the Get MAX LUN command.
> >
> > Cc: Star Zeng 
> > Cc: Eric Dong 
> > Contributed-under: TianoCore Contribution Agreement
> 1.1
> > Signed-off-by: Michael D Kinney
> > 
> > ---
> >  MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 8 +++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> > b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> > index 78220222f6..b0ad7938e9 100644
> > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> > @@ -2,7 +2,7 @@
> >
> >  Usb Bus Driver Binding and Bus IO Protocol.
> >
> > -Copyright (c) 2004 - 2016, Intel Corporation. All
> rights
> > reserved.
> > +Copyright (c) 2004 - 2017, 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 @@ -76,6 +76,7 @@

[edk2] [Patch] ShellPkg: Add error message if failed to place receive token in ping command.

2017-11-14 Thread Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
Cc: Wu Jiaxin 
Cc: Ye Ting 
Cc: Ruiyu Ni 
---
 ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c | 9 +++--
 .../UefiShellNetwork1CommandsLib.uni | 1 +
 ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c| 9 +++--
 .../UefiShellNetwork2CommandsLib.uni | 1 +
 4 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c 
b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
index ca5c22a..10d291c 100644
--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
@@ -2,7 +2,7 @@
   The implementation for Ping shell command.
 
   (C) Copyright 2015 Hewlett-Packard Development Company, L.P.
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
   (C) Copyright 2016 Hewlett Packard Enterprise Development LP
 
   This program and the accompanying materials
@@ -629,6 +629,7 @@ ON_EXIT:
 Status = Private->ProtocolPointers.Receive (Private->IpProtocol, 
>RxToken);
 
 if (EFI_ERROR (Status)) {
+  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_PING_RECEIVE), 
gShellNetwork1HiiHandle, Status);
   Private->Status = EFI_ABORTED;
 }
   } else {
@@ -828,7 +829,11 @@ Ping6ReceiveEchoReply (
 
   Private->RxToken.Status = EFI_NOT_READY;
 
-  return (Private->ProtocolPointers.Receive (Private->IpProtocol, 
>RxToken));
+  Status = Private->ProtocolPointers.Receive (Private->IpProtocol, 
>RxToken);
+  if (EFI_ERROR (Status)) {
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_PING_RECEIVE), 
gShellNetwork1HiiHandle, Status);
+  }
+  return Status;
 }
 
 /**
diff --git 
a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
 
b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
index 70c3465..7a43ad5 100644
--- 
a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
+++ 
b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
@@ -50,6 +50,7 @@
 #string STR_PING_CONFIG  #language en-US "Config %r\r\n"
 #string STR_PING_GETMODE #language en-US "GetModeData %r\r\n"
 #string STR_PING_GETDATA #language en-US "GetData %r\r\n"
+#string STR_PING_RECEIVE #language en-US "Receive %r\r\n"
 #string STR_PING_SEND_REQUEST#language en-US "Echo request sequence %d 
did not complete successfully.\r\n"
 #string STR_PING_NOSOURCE_INDO   #language en-US "There are no sources in 
%s's multicast domain.\r\n"
 #string STR_PING_NETWORK_ERROR   #language en-US "%H%s%N: Network function 
failed with %r\r\n"
diff --git a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c 
b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
index 2961fd7..b784696 100644
--- a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
+++ b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
@@ -1,7 +1,7 @@
 /** @file
   The implementation for Ping6 application.
 
-  Copyright (c) 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2016 - 2017, 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
@@ -474,6 +474,7 @@ ON_EXIT:
 Status = Private->Ip6->Receive (Private->Ip6, RxToken);
 
 if (EFI_ERROR (Status)) {
+  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_PING6_IP6_RECEIVE), 
gShellNetwork2HiiHandle, Status);
   Private->Status = EFI_ABORTED;
 }
   } else {
@@ -658,7 +659,11 @@ Ping6OnReceiveEchoReply (
 
   Private->RxToken.Status = EFI_NOT_READY;
 
-  return Private->Ip6->Receive (Private->Ip6, >RxToken);
+  Status = Private->Ip6->Receive (Private->Ip6, >RxToken);
+  if (EFI_ERROR (Status)) {
+ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_PING6_IP6_RECEIVE), 
gShellNetwork2HiiHandle, Status);
+  }
+  return Status;
 }
 
 /**
diff --git 
a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.uni
 
b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.uni
index 2b2327b..9a535ad 100644
--- 
a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.uni
+++ 
b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.uni
@@ -33,6 +33,7 @@
 #string STR_PING6_INVALID_BUFFER_SIZE  #language en-US  "%Ping6: Invalid 
buffer size, %s\r\n"
 #string STR_PING6_INVALID_SOURCE   #language en-US  "%Ping6: Require 
source interface option\r\n"
 #string STR_PING6_IP6_CONFIG   #language en-US  "%Ping6: The 
process of Ip6 Configure %r\r\n"
+#string STR_PING6_IP6_RECEIVE

Re: [edk2] [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error check

2017-11-14 Thread Kinney, Michael D 
Star,

I am curious.  Which spec statement are you referring to?

Mike

> -Original Message-
> From: Zeng, Star
> Sent: Tuesday, November 14, 2017 6:59 PM
> To: Kinney, Michael D ;
> edk2-devel@lists.01.org
> Cc: Dong, Eric ; Zeng, Star
> 
> Subject: RE: [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add
> UsbControlTransfer() error check
> 
> Mike,
> 
> I do not insist on the check that the Direction is not
> EfiUsbNoData, and I know the functionality should have
> no improvement with the check.
> So, you can have my Reviewed-by: Star Zeng
> .
> 
> I raised the question just for consideration from
> literally, as according to the spec, the code could not
> touch DataLength at all when Direction is EfiUsbNoData.
> 
> You can decide to have / have not the check when
> pushing. :)
> 
> 
> Thanks,
> Star
> -Original Message-
> From: Kinney, Michael D
> Sent: Wednesday, November 15, 2017 10:46 AM
> To: Zeng, Star ; edk2-
> de...@lists.01.org; Kinney, Michael D
> 
> Cc: Dong, Eric 
> Subject: RE: [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add
> UsbControlTransfer() error check
> 
> Star,
> 
> For that case both DataLength and RequestedDataLength
> will be 0 and the new path will not be taken.
> 
> Are you concerned that the USB HC Protocol could return
> a non zero DataLength for the EfiUsbNoData case?  Even
> if it does, the new path can never be taken because 0
> is less than all UINTN values.
> 
> Mike
> 
> > -Original Message-
> > From: Zeng, Star
> > Sent: Tuesday, November 14, 2017 6:40 PM
> > To: Kinney, Michael D ;
> > edk2-devel@lists.01.org
> > Cc: Dong, Eric ; Zeng, Star
> 
> > Subject: RE: [Patch V2 1/2] MdeModulePkg/UsbBusDxe:
> Add
> > UsbControlTransfer() error check
> >
> > Mike,
> >
> > Do you think it is better or not to also check the
> Direction is not
> > EfiUsbNoData?
> >
> > UEFI spec, EFI_USB_IO_PROTOCOL.UsbControlTransfer():
> > If the Direction parameter is EfiUsbNoData, Data is
> NULL, and
> > DataLength is 0, then no data phase exists for the
> control transfer.
> >
> > Thanks,
> > Star
> > -Original Message-
> > From: Kinney, Michael D
> > Sent: Wednesday, November 15, 2017 9:03 AM
> > To: edk2-devel@lists.01.org
> > Cc: Zeng, Star ; Dong, Eric
> 
> > Subject: [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add
> > UsbControlTransfer() error check
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=767
> >
> > The USB I/O Protocol function ControlTransfer() has a
> DataLength
> > parameter that specifies the size of the Data buffer.
> The UsbBusDxe
> > module implements the USB I/O Protocol using the
> services of the USB2
> > Host Controller Protocol.  The DataLength parameter
> in the
> > USB2 Host Controller Protocol ControlTransfer()
> service is an IN OUT
> > parameter so the number of bytes actually transferred
> is returned.
> > Since the USB I/O Protocol
> > ControlTransfer() service can not return the number
> of bytes actually
> > transferred, the only option if the number of bytes
> actually
> > transferred is less than the number of bytes
> requested is to return
> > EFI_DEVICE_ERROR.
> >
> > The change fixes an issue with a USB mass storage
> device that responds
> > with 0 bytes to the Get MAX LUN command.
> >
> > Cc: Star Zeng 
> > Cc: Eric Dong 
> > Contributed-under: TianoCore Contribution Agreement
> 1.1
> > Signed-off-by: Michael D Kinney
> > 
> > ---
> >  MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 8 +++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> > b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> > index 78220222f6..b0ad7938e9 100644
> > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> > @@ -2,7 +2,7 @@
> >
> >  Usb Bus Driver Binding and Bus IO Protocol.
> >
> > -Copyright (c) 2004 - 2016, Intel Corporation. All
> rights
> > reserved.
> > +Copyright (c) 2004 - 2017, 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 @@ -76,6 +76,7 @@ UsbIoControlTransfer (
> >USB_ENDPOINT_DESC   *EpDesc;
> >EFI_TPL OldTpl;
> >EFI_STATUS  Status;
> > +  UINTN   RequestedDataLength;
> >
> >if (UsbStatus == NULL) {
> >  return EFI_INVALID_PARAMETER;
> > @@ -86,6 +87,7 @@ UsbIoControlTransfer (
> >UsbIf  = USB_INTERFACE_FROM_USBIO (This);
> >Dev= UsbIf->Device;
> >
> > +  RequestedDataLength = DataLength;
> >

Re: [edk2] [Patch] NetworkPkg: Fix incorrect SizeofHeaders returned from HttpTcpReceiveHeader().

2017-11-14 Thread Ye, Ting 
Reviewed-by: Ye Ting  

-Original Message-
From: Fu, Siyuan 
Sent: Wednesday, November 15, 2017 10:45 AM
To: edk2-devel@lists.01.org
Cc: Wu, Jiaxin ; Ye, Ting ; Karunakar P 

Subject: [Patch] NetworkPkg: Fix incorrect SizeofHeaders returned from 
HttpTcpReceiveHeader().

This patch is to fix a bug that the HttpTcpReceiveHeader() may return incorrect 
SizeofHeaders, which will include some already received message-body.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
Cc: Wu Jiaxin 
Cc: Ye Ting 
Cc: Karunakar P 
---
 NetworkPkg/HttpDxe/HttpProto.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/NetworkPkg/HttpDxe/HttpProto.c b/NetworkPkg/HttpDxe/HttpProto.c 
index ab00f3d..1aa1816 100644
--- a/NetworkPkg/HttpDxe/HttpProto.c
+++ b/NetworkPkg/HttpDxe/HttpProto.c
@@ -1876,7 +1876,10 @@ HttpTcpReceiveHeader (
   //
   // Check whether we received end of HTTP headers.
   //
-  *EndofHeader = AsciiStrStr (*HttpHeaders, HTTP_END_OF_HDR_STR); 
+  *EndofHeader = AsciiStrStr (*HttpHeaders, HTTP_END_OF_HDR_STR);
+  if (*EndofHeader != NULL) {
+*SizeofHeaders = *EndofHeader - *HttpHeaders;
+  }
 };
 
 //
@@ -1976,6 +1979,9 @@ HttpTcpReceiveHeader (
   // Check whether we received end of HTTP headers.
   //
   *EndofHeader = AsciiStrStr (*HttpHeaders, HTTP_END_OF_HDR_STR); 
+  if (*EndofHeader != NULL) {
+*SizeofHeaders = *EndofHeader - *HttpHeaders;
+  }
 };
 
 //
--
1.9.5.msysgit.1

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

[edk2] [Patch] MdeModulePkg/SNP: remove redundant DEBUG print in SNP Transmit.c

2017-11-14 Thread Fu Siyuan 
This patch is to remove some redundant DEBUG output in SNP transmit function.
In case of return EFI_NOT_READY in PxeTransmit, the SNP driver is indicate
the caller that the transmit queue is full, it's a very common situation druing
transmit, not a critical error. So the patch move the DEBUG lever to EFI_D_NET.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
Cc: Wu Jiaxin 
Cc: Ye Ting 
---
 MdeModulePkg/Universal/Network/SnpDxe/Transmit.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Transmit.c 
b/MdeModulePkg/Universal/Network/SnpDxe/Transmit.c
index 73461bc..2c7083e 100644
--- a/MdeModulePkg/Universal/Network/SnpDxe/Transmit.c
+++ b/MdeModulePkg/Universal/Network/SnpDxe/Transmit.c
@@ -1,7 +1,7 @@
 /** @file
 Implementation of transmitting a packet.
  
-Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2004 - 2017, 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 
@@ -186,7 +186,6 @@ PxeTransmit (
   (*Snp->IssueUndi32Command) ((UINT64) (UINTN) >Cdb);
 
   DEBUG ((EFI_D_NET, "\nexit Snp->undi.transmit()  "));
-  DEBUG ((EFI_D_NET, "\nSnp->Cdb.StatCode == %r", Snp->Cdb.StatCode));
 
   //
   // we will unmap the buffers in get_status call, not here
@@ -199,19 +198,24 @@ PxeTransmit (
   case PXE_STATCODE_QUEUE_FULL:
   case PXE_STATCODE_BUSY:
 Status = EFI_NOT_READY;
+DEBUG (
+  (EFI_D_NET,
+  "\nSnp->undi.transmit()  %xh:%xh\n",
+  Snp->Cdb.StatFlags,
+  Snp->Cdb.StatCode)
+  );
 break;
 
   default:
+DEBUG (
+  (EFI_D_ERROR,
+  "\nSnp->undi.transmit()  %xh:%xh\n",
+  Snp->Cdb.StatFlags,
+  Snp->Cdb.StatCode)
+  );
 Status = EFI_DEVICE_ERROR;
   }
 
-  DEBUG (
-(EFI_D_ERROR,
-"\nSnp->undi.transmit()  %xh:%xh\n",
-Snp->Cdb.StatFlags,
-Snp->Cdb.StatCode)
-);
-
   return Status;
 }
 
-- 
1.9.5.msysgit.1

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

Re: [edk2] [Patch V2] NetworkPkg: Print error message to screen if error occurs during HTTP boot.

2017-11-14 Thread Ye, Ting 
Reviewed-by: Ye Ting  

-Original Message-
From: Fu, Siyuan 
Sent: Wednesday, November 15, 2017 9:52 AM
To: edk2-devel@lists.01.org
Cc: Wu, Jiaxin ; Ye, Ting 
Subject: [Patch V2] NetworkPkg: Print error message to screen if error occurs 
during HTTP boot.

V2 update:
Add error message is URI address is not correct.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
Cc: Wu Jiaxin 
Cc: Ye Ting 
---
 NetworkPkg/HttpBootDxe/HttpBootImpl.c| 21 +
 NetworkPkg/HttpBootDxe/HttpBootSupport.c |  2 ++
 NetworkPkg/HttpDxe/HttpImpl.c|  1 +
 3 files changed, 24 insertions(+)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootImpl.c 
b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
index 06a8a6a..d591db5 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootImpl.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
@@ -327,6 +327,7 @@ HttpBootLoadFile (
 //
 Status = HttpBootDiscoverBootInfo (Private);
 if (EFI_ERROR (Status)) {
+  AsciiPrint ("\n  Error: Could not discover the boot information 
+ for DHCP server.\n");
   goto ON_EXIT;
 }
   }
@@ -369,6 +370,7 @@ HttpBootLoadFile (
  >ImageType
  );
   if (EFI_ERROR (Status) && Status != EFI_BUFFER_TOO_SMALL) {
+AsciiPrint ("\n  Error: Could not retrieve NBP file size from 
+ HTTP server.\n");
 goto ON_EXIT;
   }
 }
@@ -394,6 +396,22 @@ HttpBootLoadFile (
   
 ON_EXIT:
   HttpBootUninstallCallback (Private);
+
+  if (Status == EFI_ACCESS_DENIED) {
+AsciiPrint ("\n  Error: Could not establish connection with HTTP 
+ server.\n");  } else if (Status == EFI_BUFFER_TOO_SMALL && Buffer != NULL) {
+AsciiPrint ("\n  Error: Buffer size is smaller than the requested 
+ file.\n");  } else if (Status == EFI_OUT_OF_RESOURCES) {
+AsciiPrint ("\n  Error: Could not allocate I/O buffers.\n");  } 
+ else if (Status == EFI_DEVICE_ERROR) {
+AsciiPrint ("\n  Error: Network device error.\n");  } else if 
+ (Status == EFI_TIMEOUT) {
+AsciiPrint ("\n  Error: Server response timeout.\n");  } else if 
+ (Status == EFI_ABORTED) {
+AsciiPrint ("\n  Error: Remote boot cancelled.\n");  } else if 
+ (Status != EFI_BUFFER_TOO_SMALL) {
+AsciiPrint ("\n  Error: Unexpected network error.\n");  }
   return Status;
 }
 
@@ -555,6 +573,7 @@ HttpBootDxeLoadFile (
   MediaPresent = TRUE;
   NetLibDetectMedia (Private->Controller, );
   if (!MediaPresent) {
+AsciiPrint ("\n  Error: Could not detect network connection.\n");
 return EFI_NO_MEDIA;
   }
   
@@ -595,6 +614,8 @@ HttpBootDxeLoadFile (
 Status = HttpBootRegisterRamDisk (Private, *BufferSize, Buffer, ImageType);
 if (!EFI_ERROR (Status)) {
   Status = EFI_WARN_FILE_SYSTEM;
+} else {
+  AsciiPrint ("\n  Error: Could not register RAM disk to the 
+ system.\n");
 }
   }
 
diff --git a/NetworkPkg/HttpBootDxe/HttpBootSupport.c 
b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
index d508e2c..7ca48f3 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootSupport.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
@@ -1093,6 +1093,7 @@ HttpBootCheckUriScheme (
   // Return EFI_INVALID_PARAMETER if the URI is not HTTP or HTTPS.
   //
   if ((AsciiStrnCmp (Uri, "http://;, 7) != 0) && (AsciiStrnCmp (Uri, 
"https://;, 8) != 0)) {
+AsciiPrint ("\n  Error: Invalid URI address.\n");
 DEBUG ((EFI_D_ERROR, "HttpBootCheckUriScheme: Invalid Uri.\n"));
 return EFI_INVALID_PARAMETER;
   }
@@ -1101,6 +1102,7 @@ HttpBootCheckUriScheme (
   // HTTP is disabled, return EFI_ACCESS_DENIED if the URI is HTTP.
   //
   if (!PcdGetBool (PcdAllowHttpConnections) && (AsciiStrnCmp (Uri, "http://;, 
7) == 0)) {
+AsciiPrint ("\n  Error: Access forbidden, only HTTPS connection is 
+ allowed.\n");
 DEBUG ((EFI_D_ERROR, "HttpBootCheckUriScheme: HTTP is disabled.\n"));
 return EFI_ACCESS_DENIED;
   }
diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c 
index 46d0323..57fa39f 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -523,6 +523,7 @@ EfiHttpRequest (
   
   FreePool (HostNameStr);
   if (EFI_ERROR (Status)) {
+DEBUG ((EFI_D_ERROR, "Error: Could not retrieve the host 
+ address from DNS server.\n"));
 goto Error1;
   }
 }
--
1.9.5.msysgit.1

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

Re: [edk2] [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error check

2017-11-14 Thread Zeng, Star 
Mike,

I do not insist on the check that the Direction is not EfiUsbNoData, and I know 
the functionality should have no improvement with the check.
So, you can have my Reviewed-by: Star Zeng .

I raised the question just for consideration from literally, as according to 
the spec, the code could not touch DataLength at all when Direction is 
EfiUsbNoData.

You can decide to have / have not the check when pushing. :)


Thanks,
Star
-Original Message-
From: Kinney, Michael D 
Sent: Wednesday, November 15, 2017 10:46 AM
To: Zeng, Star ; edk2-devel@lists.01.org; Kinney, Michael 
D 
Cc: Dong, Eric 
Subject: RE: [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() 
error check

Star,

For that case both DataLength and RequestedDataLength will be 0 and the new 
path will not be taken.

Are you concerned that the USB HC Protocol could return a non zero DataLength 
for the EfiUsbNoData case?  Even if it does, the new path can never be taken 
because 0 is less than all UINTN values.

Mike

> -Original Message-
> From: Zeng, Star
> Sent: Tuesday, November 14, 2017 6:40 PM
> To: Kinney, Michael D ; 
> edk2-devel@lists.01.org
> Cc: Dong, Eric ; Zeng, Star 
> Subject: RE: [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add
> UsbControlTransfer() error check
> 
> Mike,
> 
> Do you think it is better or not to also check the Direction is not 
> EfiUsbNoData?
> 
> UEFI spec, EFI_USB_IO_PROTOCOL.UsbControlTransfer():
> If the Direction parameter is EfiUsbNoData, Data is NULL, and 
> DataLength is 0, then no data phase exists for the control transfer.
> 
> Thanks,
> Star
> -Original Message-
> From: Kinney, Michael D
> Sent: Wednesday, November 15, 2017 9:03 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Dong, Eric 
> Subject: [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add
> UsbControlTransfer() error check
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=767
> 
> The USB I/O Protocol function ControlTransfer() has a DataLength 
> parameter that specifies the size of the Data buffer.  The UsbBusDxe 
> module implements the USB I/O Protocol using the services of the USB2 
> Host Controller Protocol.  The DataLength parameter in the
> USB2 Host Controller Protocol ControlTransfer() service is an IN OUT 
> parameter so the number of bytes actually transferred is returned.  
> Since the USB I/O Protocol
> ControlTransfer() service can not return the number of bytes actually 
> transferred, the only option if the number of bytes actually 
> transferred is less than the number of bytes requested is to return 
> EFI_DEVICE_ERROR.
> 
> The change fixes an issue with a USB mass storage device that responds 
> with 0 bytes to the Get MAX LUN command.
> 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Michael D Kinney
> 
> ---
>  MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> index 78220222f6..b0ad7938e9 100644
> --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> @@ -2,7 +2,7 @@
> 
>  Usb Bus Driver Binding and Bus IO Protocol.
> 
> -Copyright (c) 2004 - 2016, Intel Corporation. All rights 
> reserved.
> +Copyright (c) 2004 - 2017, 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 @@ -76,6 +76,7 @@ UsbIoControlTransfer (
>USB_ENDPOINT_DESC   *EpDesc;
>EFI_TPL OldTpl;
>EFI_STATUS  Status;
> +  UINTN   RequestedDataLength;
> 
>if (UsbStatus == NULL) {
>  return EFI_INVALID_PARAMETER;
> @@ -86,6 +87,7 @@ UsbIoControlTransfer (
>UsbIf  = USB_INTERFACE_FROM_USBIO (This);
>Dev= UsbIf->Device;
> 
> +  RequestedDataLength = DataLength;
>Status = UsbHcControlTransfer (
>   Dev->Bus,
>   Dev->Address,
> @@ -99,6 +101,10 @@ UsbIoControlTransfer (
>   >Translator,
>   UsbStatus
>   );
> +  if (!EFI_ERROR (Status) && DataLength <
> RequestedDataLength) {
> +Status = EFI_DEVICE_ERROR;
> +goto ON_EXIT;
> +  }
> 
>if (EFI_ERROR (Status) || (*UsbStatus !=
> EFI_USB_NOERROR)) {
>  //
> --
> 2.14.2.windows.3

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

[edk2] [Patch] NetworkPkg: Fix incorrect SizeofHeaders returned from HttpTcpReceiveHeader().

2017-11-14 Thread Fu Siyuan 
This patch is to fix a bug that the HttpTcpReceiveHeader() may return incorrect
SizeofHeaders, which will include some already received message-body.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
Cc: Wu Jiaxin 
Cc: Ye Ting 
Cc: Karunakar P 
---
 NetworkPkg/HttpDxe/HttpProto.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/NetworkPkg/HttpDxe/HttpProto.c b/NetworkPkg/HttpDxe/HttpProto.c
index ab00f3d..1aa1816 100644
--- a/NetworkPkg/HttpDxe/HttpProto.c
+++ b/NetworkPkg/HttpDxe/HttpProto.c
@@ -1876,7 +1876,10 @@ HttpTcpReceiveHeader (
   //
   // Check whether we received end of HTTP headers.
   //
-  *EndofHeader = AsciiStrStr (*HttpHeaders, HTTP_END_OF_HDR_STR); 
+  *EndofHeader = AsciiStrStr (*HttpHeaders, HTTP_END_OF_HDR_STR);
+  if (*EndofHeader != NULL) {
+*SizeofHeaders = *EndofHeader - *HttpHeaders;
+  }
 };
 
 //
@@ -1976,6 +1979,9 @@ HttpTcpReceiveHeader (
   // Check whether we received end of HTTP headers.
   //
   *EndofHeader = AsciiStrStr (*HttpHeaders, HTTP_END_OF_HDR_STR); 
+  if (*EndofHeader != NULL) {
+*SizeofHeaders = *EndofHeader - *HttpHeaders;
+  }
 };
 
 //
-- 
1.9.5.msysgit.1

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

Re: [edk2] [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error check

2017-11-14 Thread Kinney, Michael D 
Star,

For that case both DataLength and RequestedDataLength
will be 0 and the new path will not be taken.

Are you concerned that the USB HC Protocol could return
a non zero DataLength for the EfiUsbNoData case?  Even
if it does, the new path can never be taken because 0 is
less than all UINTN values.

Mike

> -Original Message-
> From: Zeng, Star
> Sent: Tuesday, November 14, 2017 6:40 PM
> To: Kinney, Michael D ;
> edk2-devel@lists.01.org
> Cc: Dong, Eric ; Zeng, Star
> 
> Subject: RE: [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add
> UsbControlTransfer() error check
> 
> Mike,
> 
> Do you think it is better or not to also check the
> Direction is not EfiUsbNoData?
> 
> UEFI spec, EFI_USB_IO_PROTOCOL.UsbControlTransfer():
> If the Direction parameter is EfiUsbNoData, Data is
> NULL, and DataLength is 0, then no data phase exists
> for the control transfer.
> 
> Thanks,
> Star
> -Original Message-
> From: Kinney, Michael D
> Sent: Wednesday, November 15, 2017 9:03 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Dong, Eric
> 
> Subject: [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add
> UsbControlTransfer() error check
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=767
> 
> The USB I/O Protocol function ControlTransfer() has a
> DataLength parameter that specifies the size of the
> Data buffer.  The UsbBusDxe module implements the USB
> I/O Protocol using the services of the USB2 Host
> Controller Protocol.  The DataLength parameter in the
> USB2 Host Controller Protocol ControlTransfer() service
> is an IN OUT parameter so the number of bytes actually
> transferred is returned.  Since the USB I/O Protocol
> ControlTransfer() service can not return the number of
> bytes actually transferred, the only option if the
> number of bytes actually transferred is less than the
> number of bytes requested is to return
> EFI_DEVICE_ERROR.
> 
> The change fixes an issue with a USB mass storage
> device that responds with 0 bytes to the Get MAX LUN
> command.
> 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Michael D Kinney
> 
> ---
>  MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> index 78220222f6..b0ad7938e9 100644
> --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> @@ -2,7 +2,7 @@
> 
>  Usb Bus Driver Binding and Bus IO Protocol.
> 
> -Copyright (c) 2004 - 2016, Intel Corporation. All
> rights reserved.
> +Copyright (c) 2004 - 2017, 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 @@ -76,6 +76,7 @@ UsbIoControlTransfer (
>USB_ENDPOINT_DESC   *EpDesc;
>EFI_TPL OldTpl;
>EFI_STATUS  Status;
> +  UINTN   RequestedDataLength;
> 
>if (UsbStatus == NULL) {
>  return EFI_INVALID_PARAMETER;
> @@ -86,6 +87,7 @@ UsbIoControlTransfer (
>UsbIf  = USB_INTERFACE_FROM_USBIO (This);
>Dev= UsbIf->Device;
> 
> +  RequestedDataLength = DataLength;
>Status = UsbHcControlTransfer (
>   Dev->Bus,
>   Dev->Address,
> @@ -99,6 +101,10 @@ UsbIoControlTransfer (
>   >Translator,
>   UsbStatus
>   );
> +  if (!EFI_ERROR (Status) && DataLength <
> RequestedDataLength) {
> +Status = EFI_DEVICE_ERROR;
> +goto ON_EXIT;
> +  }
> 
>if (EFI_ERROR (Status) || (*UsbStatus !=
> EFI_USB_NOERROR)) {
>  //
> --
> 2.14.2.windows.3

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

Re: [edk2] [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error check

2017-11-14 Thread Zeng, Star 
Mike,

Do you think it is better or not to also check the Direction is not 
EfiUsbNoData?

UEFI spec, EFI_USB_IO_PROTOCOL.UsbControlTransfer():
If the Direction parameter is EfiUsbNoData, Data is NULL, and DataLength is 0, 
then no data phase exists for the control transfer.

Thanks,
Star
-Original Message-
From: Kinney, Michael D 
Sent: Wednesday, November 15, 2017 9:03 AM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Dong, Eric 
Subject: [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error 
check

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

The USB I/O Protocol function ControlTransfer() has a DataLength parameter that 
specifies the size of the Data buffer.  The UsbBusDxe module implements the USB 
I/O Protocol using the services of the USB2 Host Controller Protocol.  The 
DataLength parameter in the USB2 Host Controller Protocol ControlTransfer() 
service is an IN OUT parameter so the number of bytes actually transferred is 
returned.  Since the USB I/O Protocol
ControlTransfer() service can not return the number of bytes actually 
transferred, the only option if the number of bytes actually transferred is 
less than the number of bytes requested is to return EFI_DEVICE_ERROR.

The change fixes an issue with a USB mass storage device that responds with 0 
bytes to the Get MAX LUN command.

Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney 
---
 MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c 
b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
index 78220222f6..b0ad7938e9 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
@@ -2,7 +2,7 @@
 
 Usb Bus Driver Binding and Bus IO Protocol.
 
-Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2004 - 2017, 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 @@ -76,6 +76,7 @@ 
UsbIoControlTransfer (
   USB_ENDPOINT_DESC   *EpDesc;
   EFI_TPL OldTpl;
   EFI_STATUS  Status;
+  UINTN   RequestedDataLength;
 
   if (UsbStatus == NULL) {
 return EFI_INVALID_PARAMETER;
@@ -86,6 +87,7 @@ UsbIoControlTransfer (
   UsbIf  = USB_INTERFACE_FROM_USBIO (This);
   Dev= UsbIf->Device;
 
+  RequestedDataLength = DataLength;
   Status = UsbHcControlTransfer (
  Dev->Bus,
  Dev->Address,
@@ -99,6 +101,10 @@ UsbIoControlTransfer (
  >Translator,
  UsbStatus
  );
+  if (!EFI_ERROR (Status) && DataLength < RequestedDataLength) {
+Status = EFI_DEVICE_ERROR;
+goto ON_EXIT;
+  }
 
   if (EFI_ERROR (Status) || (*UsbStatus != EFI_USB_NOERROR)) {
 //
--
2.14.2.windows.3

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

Re: [edk2] [Patch V2 2/2] MdeModulePkg/UsbMassStorageDxe: Check Get Max LUN status/value

2017-11-14 Thread Zeng, Star 
Mike,

With a minor typo "that o not" fixed to "that do not" in the commit log, 
Reviewed-by: Star Zeng 

Thanks,
Star
-Original Message-
From: Kinney, Michael D 
Sent: Wednesday, November 15, 2017 9:03 AM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Dong, Eric 
Subject: [Patch V2 2/2] MdeModulePkg/UsbMassStorageDxe: Check Get Max LUN 
status/value

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

If a USB Mass Storage device does not support the Get Max LUN command, then the 
USB I/O Protocol ControlTransfer() service may return an error.  If an error is 
returned for this command, then assume that the device does not support 
multiple LUNs and return a maximum LUN value of 0.

The USB Mass Storage Class Specification states that a maximum LUN value larger 
than 0x0F is invalid.  Add a check to make sure this maximum LUN value is in 
this valid range, and if it is not, then assume that the device does not 
support multiple LUNs and return a maximum LUN value of 0.

This change improves compatibility with USB FLASH drives that o not support the 
Get Max LUN command or return an invalid maximum LUN value.

Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney 
---
 MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c 
b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c
index 4bb7222b89..6afe2ae235 100644
--- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c
+++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c
@@ -2,7 +2,7 @@
   Implementation of the USB mass storage Bulk-Only Transport protocol,
   according to USB Mass Storage Class Bulk-Only Transport, Revision 1.0.
 
-Copyright (c) 2007 - 2011, Intel Corporation. All rights reserved.
+Copyright (c) 2007 - 2017, 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 @@ -576,6 +576,18 
@@ UsbBotGetMaxLun (
 1,
 
 );
+  if (EFI_ERROR (Status) || *MaxLun > USB_BOT_MAX_LUN) {
+//
+// If the Get LUN request returns an error or the MaxLun is larger than
+// the maximum LUN value (0x0f) supported by the USB Mass Storage Class
+// Bulk-Only Transport Spec, then set MaxLun to 0.
+//
+// This improves compatibility with USB FLASH drives that have a single LUN
+// and either do not return a max LUN value or return an invalid maximum 
LUN
+// value.
+//
+*MaxLun = 0;
+  }
 
   return Status;
 }
--
2.14.2.windows.3

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

Re: [edk2] [staging/Customized-Secure-Boot] Sync Customized-Secure-Boot From edk2/master

2017-11-14 Thread Zhang, Chao B 
HI Cinnamon:
 We plan to update staging with latest EDK2 core and sync ECR1689 Out Of 
band Auth Variable Management, 
ECR 1263 Customized Secure Boot, ECR1557 and TCG PFP spec 00.21 PCR[7] 
measurement  to staging. That requires big effort. 
We will notify the community after all the jobs are done.  


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Shia, 
Cinnamon
Sent: Monday, November 13, 2017 5:24 PM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D 
Subject: [edk2] [staging/Customized-Secure-Boot] Sync Customized-Secure-Boot 
From edk2/master

Hi



The Customized-Secure-Boot branch is behind EDK2 master, and there are build 
errors when building it with latest basetools. Can you please sync the branch 
from EDK2 master?

Thanks,
Cinnamon Shia
___
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 V2] NetworkPkg: Print error message to screen if error occurs during HTTP boot.

2017-11-14 Thread Fu Siyuan 
V2 update:
Add error message is URI address is not correct.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
Cc: Wu Jiaxin 
Cc: Ye Ting 
---
 NetworkPkg/HttpBootDxe/HttpBootImpl.c| 21 +
 NetworkPkg/HttpBootDxe/HttpBootSupport.c |  2 ++
 NetworkPkg/HttpDxe/HttpImpl.c|  1 +
 3 files changed, 24 insertions(+)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootImpl.c 
b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
index 06a8a6a..d591db5 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootImpl.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
@@ -327,6 +327,7 @@ HttpBootLoadFile (
 //
 Status = HttpBootDiscoverBootInfo (Private);
 if (EFI_ERROR (Status)) {
+  AsciiPrint ("\n  Error: Could not discover the boot information for DHCP 
server.\n");
   goto ON_EXIT;
 }
   }
@@ -369,6 +370,7 @@ HttpBootLoadFile (
  >ImageType
  );
   if (EFI_ERROR (Status) && Status != EFI_BUFFER_TOO_SMALL) {
+AsciiPrint ("\n  Error: Could not retrieve NBP file size from HTTP 
server.\n");
 goto ON_EXIT;
   }
 }
@@ -394,6 +396,22 @@ HttpBootLoadFile (
   
 ON_EXIT:
   HttpBootUninstallCallback (Private);
+
+  if (Status == EFI_ACCESS_DENIED) {
+AsciiPrint ("\n  Error: Could not establish connection with HTTP 
server.\n");
+  } else if (Status == EFI_BUFFER_TOO_SMALL && Buffer != NULL) {
+AsciiPrint ("\n  Error: Buffer size is smaller than the requested 
file.\n");
+  } else if (Status == EFI_OUT_OF_RESOURCES) {
+AsciiPrint ("\n  Error: Could not allocate I/O buffers.\n");
+  } else if (Status == EFI_DEVICE_ERROR) {
+AsciiPrint ("\n  Error: Network device error.\n");
+  } else if (Status == EFI_TIMEOUT) {
+AsciiPrint ("\n  Error: Server response timeout.\n");
+  } else if (Status == EFI_ABORTED) {
+AsciiPrint ("\n  Error: Remote boot cancelled.\n");
+  } else if (Status != EFI_BUFFER_TOO_SMALL) {
+AsciiPrint ("\n  Error: Unexpected network error.\n");
+  }
   return Status;
 }
 
@@ -555,6 +573,7 @@ HttpBootDxeLoadFile (
   MediaPresent = TRUE;
   NetLibDetectMedia (Private->Controller, );
   if (!MediaPresent) {
+AsciiPrint ("\n  Error: Could not detect network connection.\n");
 return EFI_NO_MEDIA;
   }
   
@@ -595,6 +614,8 @@ HttpBootDxeLoadFile (
 Status = HttpBootRegisterRamDisk (Private, *BufferSize, Buffer, ImageType);
 if (!EFI_ERROR (Status)) {
   Status = EFI_WARN_FILE_SYSTEM;
+} else {
+  AsciiPrint ("\n  Error: Could not register RAM disk to the system.\n");
 }
   }
 
diff --git a/NetworkPkg/HttpBootDxe/HttpBootSupport.c 
b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
index d508e2c..7ca48f3 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootSupport.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
@@ -1093,6 +1093,7 @@ HttpBootCheckUriScheme (
   // Return EFI_INVALID_PARAMETER if the URI is not HTTP or HTTPS.
   //
   if ((AsciiStrnCmp (Uri, "http://;, 7) != 0) && (AsciiStrnCmp (Uri, 
"https://;, 8) != 0)) {
+AsciiPrint ("\n  Error: Invalid URI address.\n");
 DEBUG ((EFI_D_ERROR, "HttpBootCheckUriScheme: Invalid Uri.\n"));
 return EFI_INVALID_PARAMETER;
   }
@@ -1101,6 +1102,7 @@ HttpBootCheckUriScheme (
   // HTTP is disabled, return EFI_ACCESS_DENIED if the URI is HTTP.
   //
   if (!PcdGetBool (PcdAllowHttpConnections) && (AsciiStrnCmp (Uri, "http://;, 
7) == 0)) {
+AsciiPrint ("\n  Error: Access forbidden, only HTTPS connection is 
allowed.\n");
 DEBUG ((EFI_D_ERROR, "HttpBootCheckUriScheme: HTTP is disabled.\n"));
 return EFI_ACCESS_DENIED;
   }
diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index 46d0323..57fa39f 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -523,6 +523,7 @@ EfiHttpRequest (
   
   FreePool (HostNameStr);
   if (EFI_ERROR (Status)) {
+DEBUG ((EFI_D_ERROR, "Error: Could not retrieve the host address from 
DNS server.\n"));
 goto Error1;
   }
 }
-- 
1.9.5.msysgit.1

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

[edk2] [Patch] MdeModulePkg/UsbMassStorageDxe: Enhance Request Sense Handling

2017-11-14 Thread Michael D Kinney 
https://bugzilla.tianocore.org/show_bug.cgi?id=782

Update the Request Sense check for the Request Sense Key of
USB_BOOT_SENSE_UNIT_ATTENTION.  For this Sense Key, the
Additional Sense Key to EFI_STATUS mappings are:

USB_BOOT_ASC_MEDIA_CHANGE -> EFI_MEDIA_CHANGE
USB_BOOT_ASC_NOT_READY-> EFI_NOT_READY
USB_BOOT_ASC_NO_MEDIA -> EFI_NOT_READY
All others-> EFI_DEVICE_ERROR

A USB flash drive is returning Request Sense Key of
USB_BOOT_SENSE_UNIT_ATTENTION and an Additional Sense Key of
USB_BOOT_ASC_NO_MEDIA for a few seconds before returning an
Additional Sense Key of USB_BOOT_ASC_MEDIA_CHANGE.

The current logic treats this initial Request Sense info as an
error and reties the command 5 times before failing completely.

With this change the USB Flash Drive works correctly.

Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney 
---
 MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c 
b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
index 2eb30f0c5f..a8b6a1c5f1 100644
--- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
+++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
@@ -120,6 +120,10 @@ UsbBootRequestSense (
   Status = EFI_MEDIA_CHANGED;
   Media->ReadOnly = FALSE;
   Media->MediaId++;
+} else if (SenseData.Asc == USB_BOOT_ASC_NOT_READY) {
+  Status = EFI_NOT_READY;
+} else if (SenseData.Asc == USB_BOOT_ASC_NO_MEDIA) {
+  Status = EFI_NOT_READY;
 }
 break;
 
-- 
2.14.2.windows.3

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

[edk2] [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error check

2017-11-14 Thread Michael D Kinney 
https://bugzilla.tianocore.org/show_bug.cgi?id=767

The USB I/O Protocol function ControlTransfer() has a DataLength
parameter that specifies the size of the Data buffer.  The
UsbBusDxe module implements the USB I/O Protocol using the
services of the USB2 Host Controller Protocol.  The DataLength
parameter in the USB2 Host Controller Protocol ControlTransfer()
service is an IN OUT parameter so the number of bytes actually
transferred is returned.  Since the USB I/O Protocol
ControlTransfer() service can not return the number of bytes
actually transferred, the only option if the number of bytes
actually transferred is less than the number of bytes requested
is to return EFI_DEVICE_ERROR.

The change fixes an issue with a USB mass storage device that
responds with 0 bytes to the Get MAX LUN command.

Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney 
---
 MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c 
b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
index 78220222f6..b0ad7938e9 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
@@ -2,7 +2,7 @@
 
 Usb Bus Driver Binding and Bus IO Protocol.
 
-Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2004 - 2017, 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
@@ -76,6 +76,7 @@ UsbIoControlTransfer (
   USB_ENDPOINT_DESC   *EpDesc;
   EFI_TPL OldTpl;
   EFI_STATUS  Status;
+  UINTN   RequestedDataLength;
 
   if (UsbStatus == NULL) {
 return EFI_INVALID_PARAMETER;
@@ -86,6 +87,7 @@ UsbIoControlTransfer (
   UsbIf  = USB_INTERFACE_FROM_USBIO (This);
   Dev= UsbIf->Device;
 
+  RequestedDataLength = DataLength;
   Status = UsbHcControlTransfer (
  Dev->Bus,
  Dev->Address,
@@ -99,6 +101,10 @@ UsbIoControlTransfer (
  >Translator,
  UsbStatus
  );
+  if (!EFI_ERROR (Status) && DataLength < RequestedDataLength) {
+Status = EFI_DEVICE_ERROR;
+goto ON_EXIT;
+  }
 
   if (EFI_ERROR (Status) || (*UsbStatus != EFI_USB_NOERROR)) {
 //
-- 
2.14.2.windows.3

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

[edk2] [Patch V2 0/2] Add Max LUN status/value checks

2017-11-14 Thread Michael D Kinney 
https://bugzilla.tianocore.org/show_bug.cgi?id=767

Add error check to USB I/O Protocol UsbControlTransfer() for the
number of bytes actually transfered.  If less than requested, then
return EFI_DEVICE_ERROR.

Check Get Max LUN status/value in USB Mass Storage Driver to handle
cases where USB device does not support Get Max LUN command or
returned an invalud Max LUN value.

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

Michael D Kinney (2):
  MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error check
  MdeModulePkg/UsbMassStorageDxe: Check Get Max LUN status/value

 MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c |  8 +++-
 MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c | 14 +-
 2 files changed, 20 insertions(+), 2 deletions(-)

-- 
2.14.2.windows.3

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

[edk2] [Patch V2 2/2] MdeModulePkg/UsbMassStorageDxe: Check Get Max LUN status/value

2017-11-14 Thread Michael D Kinney 
https://bugzilla.tianocore.org/show_bug.cgi?id=767

If a USB Mass Storage device does not support the Get
Max LUN command, then the USB I/O Protocol ControlTransfer()
service may return an error.  If an error is returned for
this command, then assume that the device does not support
multiple LUNs and return a maximum LUN value of 0.

The USB Mass Storage Class Specification states that a
maximum LUN value larger than 0x0F is invalid.  Add
a check to make sure this maximum LUN value is in this
valid range, and if it is not, then assume that the
device does not support multiple LUNs and return a
maximum LUN value of 0.

This change improves compatibility with USB FLASH drives
that o not support the Get Max LUN command or return
an invalid maximum LUN value.

Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney 
---
 MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c 
b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c
index 4bb7222b89..6afe2ae235 100644
--- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c
+++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c
@@ -2,7 +2,7 @@
   Implementation of the USB mass storage Bulk-Only Transport protocol,
   according to USB Mass Storage Class Bulk-Only Transport, Revision 1.0.
 
-Copyright (c) 2007 - 2011, Intel Corporation. All rights reserved.
+Copyright (c) 2007 - 2017, 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
@@ -576,6 +576,18 @@ UsbBotGetMaxLun (
 1,
 
 );
+  if (EFI_ERROR (Status) || *MaxLun > USB_BOT_MAX_LUN) {
+//
+// If the Get LUN request returns an error or the MaxLun is larger than
+// the maximum LUN value (0x0f) supported by the USB Mass Storage Class
+// Bulk-Only Transport Spec, then set MaxLun to 0.
+//
+// This improves compatibility with USB FLASH drives that have a single LUN
+// and either do not return a max LUN value or return an invalid maximum 
LUN
+// value.
+//
+*MaxLun = 0;
+  }
 
   return Status;
 }
-- 
2.14.2.windows.3

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

Re: [edk2] [RFC 0/1] Stack trace support in X64 exception handling

2017-11-14 Thread Paulo Alcantara 

Hi,

On 14/11/2017 15:41, Brian J. Johnson wrote:

On 11/14/2017 11:23 AM, Andrew Fish wrote:


On Nov 14, 2017, at 8:33 AM, Brian J. Johnson > wrote:


On 11/14/2017 09:37 AM, Paulo Alcantara wrote:

Hi Fan,
On 14/11/2017 12:03, Fan Jeff wrote:

Paul,

I like this feature very much. Actually, I did some POC one year 
ago but I did finalize it.


In my POC, I could use EBP to tack the stack frame on IAS32 arch.

But for x64, I tried to use –keepexceptiontable flag to explain 
stack frame from the debug section of image.


I may workson MSFT toolchain, but it did now work well for GCC 
toolchain.


I think Eric could help to verify MSFT for your patch. If it works 
well, that’s will be great!


Say again, I like this feature!!!:-)
Cool! Your help would be really appreciable! If we get this working 
for X64 in both toolchains, that should be easy to port it to IA-3 2 
as well.

Thank you very much for willing to help on that.
Paulo


Great feature!  You do need some sort of sanity check on the RIP and 
RBP values, though, so if the stack gets corrupted or the RIP is 
nonsense from following a bad pointer, you don't start dereferencing 
garbage addresses and trigger an exception loop.




Brian,

This was a long time ago and my memory might be fuzzy I think we 
talked to some debugger folks about unwinding the stack and they 
mentioned it was common for the C runtime to have a return address or 
frame pointer have a zero value so the unwind logic knows when to 
stop. This is in addition to generic sanity checking.


We got an extra push $0 added to the stack switch to help with stack 
unwind.
https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseLib/X64/SwitchStack.S 



If might be a good idea to have a PCD for the max number of stack 
frames to display as a fallback for the error check. For X64 you may 
also have to add a check for a non-cononical address as that will GP 
fault.




Good idea.

Regarding sanity checks:  I've had good luck validating code locations 
(EIP values) by using a modified PeCoffExtraActionLib to track the top 
and bottom of the range where images have been loaded.  (I've actually 
used two ranges:  one for code executed from firmware space, and one for 
code executed from RAM.)


I'm not sure offhand if there's a platform-independent way to validate 
stack pointer values.  For most PC-like systems, just ensuring that it's 
larger than 1 or 2M (to avoid NULL pointers and the legacy spaces) and 
less than about 3G (or the low memory size, if that's known) may be 
enough to avoid an exception loop.


Yeah, I agree with you guys. We certainly should be validating the RIP 
and RSP values and then avoiding the exception loop.


For the RIP value, I think we should validate it in 
PeCoffSearchImageBase(), so if it's outside PE/COFF image's address 
space, then we should return an address of zero and no trace would be 
printed out.


Since we already have a "SizeOfImage" field in PE/COFF Optional Header 
and it's available in the process' image, we might end up with checking 
whether RIP is between ImageBase through ImageBase + SizeOfImage - 1.


For the RSP value, I have no idea :-)

Thanks!
Paulo



Brian


Thanks,

Andrew Fish


For at least some versions of Microsoft's IA32 compiler, it's 
possible to compile using EBP as a stack frame base pointer (like 
gcc) by using the "/Oy-" switch.  The proposed unwind code should 
work in that case. The X64 compiler doesn't support this switch, though.


AFAIK the only way to unwind the stack with Microsoft's X64 compilers 
is to parse the unwind info in the .pdata and .xdata sections. 
 Genfw.exe usually strips those sections, but the 
"--keepexceptiontable" flag will preserve them, as Jeff pointed out. 
 I've looked hard for open source code to decode them, but haven't 
found any, even though the format is well documented.  And I haven't 
gotten around to writing it myself.  I'd love it if someone could 
contribute the code!


Another possibility is to use the branch history MSRs available on 
some x86-family processors.  Recent Intel processors can use them as 
a stack, as opposed to a circular list, so they can record a 
backtrace directly. (I'm not familiar with AMD processors' 
capabilities.)  You can enable call stack recording like this:


 #define LBR_ON_FLAG   0x0001
 #define IA32_DEBUGCTL 0x1D9
 #define CALL_STACK_SET_FLAG 0x3C4
 #define CALL_STACK_CLR_FLAG 0xFC7
 #define MSR_LBR_SELECT 0x1C8

 //
 // Enable branch recording
 //
 LbControl = AsmReadMsr64 ((UINT32)IA32_DEBUGCTL);
 LbControl |= LBR_ON_FLAG;
 AsmWriteMsr64 ((UINT32)IA32_DEBUGCTL, LbControl);

 //
 // Configure for call stack
 //
 LbSelect = AsmReadMsr64 ((UINT32)MSR_LBR_SELECT);
 LbSelect &= CALL_STACK_CLR_FLAG;
 LbSelect |= CALL_STACK_SET_FLAG;
 AsmWriteMsr64((UINT32)MSR_LBR_SELECT, LbSelect);

The EIP/RIP values are logged in 
MSR_SANDY_BRIDGE_LASTBRANCH_n_FROM_IP and

Re: [edk2] [RFC 0/1] Stack trace support in X64 exception handling

2017-11-14 Thread Brian J. Johnson 

On 11/14/2017 11:23 AM, Andrew Fish wrote:


On Nov 14, 2017, at 8:33 AM, Brian J. Johnson > wrote:


On 11/14/2017 09:37 AM, Paulo Alcantara wrote:

Hi Fan,
On 14/11/2017 12:03, Fan Jeff wrote:

Paul,

I like this feature very much. Actually, I did some POC one year ago 
but I did finalize it.


In my POC, I could use EBP to tack the stack frame on IAS32 arch.

But for x64, I tried to use –keepexceptiontable flag to explain 
stack frame from the debug section of image.


I may workson MSFT toolchain, but it did now work well for GCC 
toolchain.


I think Eric could help to verify MSFT for your patch. If it works 
well, that’s will be great!


Say again, I like this feature!!!:-)
Cool! Your help would be really appreciable! If we get this working 
for X64 in both toolchains, that should be easy to port it to IA-32 
as well.

Thank you very much for willing to help on that.
Paulo


Great feature!  You do need some sort of sanity check on the RIP and 
RBP values, though, so if the stack gets corrupted or the RIP is 
nonsense from following a bad pointer, you don't start dereferencing 
garbage addresses and trigger an exception loop.




Brian,

This was a long time ago and my memory might be fuzzy I think we 
talked to some debugger folks about unwinding the stack and they 
mentioned it was common for the C runtime to have a return address or 
frame pointer have a zero value so the unwind logic knows when to stop. 
This is in addition to generic sanity checking.


We got an extra push $0 added to the stack switch to help with stack 
unwind.

https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseLib/X64/SwitchStack.S

If might be a good idea to have a PCD for the max number of stack frames 
to display as a fallback for the error check. For X64 you may also have 
to add a check for a non-cononical address as that will GP fault.




Good idea.

Regarding sanity checks:  I've had good luck validating code locations 
(EIP values) by using a modified PeCoffExtraActionLib to track the top 
and bottom of the range where images have been loaded.  (I've actually 
used two ranges:  one for code executed from firmware space, and one for 
code executed from RAM.)


I'm not sure offhand if there's a platform-independent way to validate 
stack pointer values.  For most PC-like systems, just ensuring that it's 
larger than 1 or 2M (to avoid NULL pointers and the legacy spaces) and 
less than about 3G (or the low memory size, if that's known) may be 
enough to avoid an exception loop.


Brian


Thanks,

Andrew Fish


For at least some versions of Microsoft's IA32 compiler, it's possible 
to compile using EBP as a stack frame base pointer (like gcc) by using 
the "/Oy-" switch.  The proposed unwind code should work in that case. 
The X64 compiler doesn't support this switch, though.


AFAIK the only way to unwind the stack with Microsoft's X64 compilers 
is to parse the unwind info in the .pdata and .xdata sections. 
 Genfw.exe usually strips those sections, but the 
"--keepexceptiontable" flag will preserve them, as Jeff pointed out. 
 I've looked hard for open source code to decode them, but haven't 
found any, even though the format is well documented.  And I haven't 
gotten around to writing it myself.  I'd love it if someone could 
contribute the code!


Another possibility is to use the branch history MSRs available on 
some x86-family processors.  Recent Intel processors can use them as a 
stack, as opposed to a circular list, so they can record a backtrace 
directly. (I'm not familiar with AMD processors' capabilities.)  You 
can enable call stack recording like this:


 #define LBR_ON_FLAG   0x0001
 #define IA32_DEBUGCTL 0x1D9
 #define CALL_STACK_SET_FLAG 0x3C4
 #define CALL_STACK_CLR_FLAG 0xFC7
 #define MSR_LBR_SELECT 0x1C8

 //
 // Enable branch recording
 //
 LbControl = AsmReadMsr64 ((UINT32)IA32_DEBUGCTL);
 LbControl |= LBR_ON_FLAG;
 AsmWriteMsr64 ((UINT32)IA32_DEBUGCTL, LbControl);

 //
 // Configure for call stack
 //
 LbSelect = AsmReadMsr64 ((UINT32)MSR_LBR_SELECT);
 LbSelect &= CALL_STACK_CLR_FLAG;
 LbSelect |= CALL_STACK_SET_FLAG;
 AsmWriteMsr64((UINT32)MSR_LBR_SELECT, LbSelect);

The EIP/RIP values are logged in MSR_SANDY_BRIDGE_LASTBRANCH_n_FROM_IP 
and MSR_SANDY_BRIDGE_LASTBRANCH_n_TO_IP, and the current depth is 
tracked in MSR_LASTBRANCH_TOS.  This works quite well.  Gen10 (Sky 
Lake) processors support 32 LASTBRANCH_n MSR pairs, which is 
sufficient in almost all cases.


Different processor generations have different branch recording 
capabilities, and different numbers of LASTBRANCH_n MSRs; see Intel's 
manuals for details.


Thanks,
Brian



Thanks!

Jeff

*发件人: *Paulo Alcantara 
*发送时间: *2017年11月14日21:23
*收件人: *edk2-devel@lists.01.org  

*抄送: *Rick Bramley ; Laszlo Ersek

Re: [edk2] [PATCH 0/4] BaseTools: Add VS2017 support, including ARM and AARCH64

2017-11-14 Thread Pete Batard 

Hi Kurt,

On 2017.11.14 16:16, Kurt Kennett wrote:

Does this build and boot a platform?


Not yet, but I believe I got pretty close to a full build for ARM.

However, the resources I can devote to finalizing ARM image generation, 
and even more so, to add the missing resources for AARCH64 image 
generation are limited, even more so as I am no ARM specialist.


I was actually hoping that, with the application of these patches, EDK2 
contributors would step in to help with the effort of filling the 
assembly gaps, as well as finalize image generation for ARM and AARCH64.



This should not be committed until a platform is building and booting to UEFI 
shell, IMO.


Well, I see it a bit as a catch 22.

If we don't get the VS2017 ARM toolchains in, I don't think many people 
will be willing to help with the image generation, because there will be 
very little incentive for them to do so. And if we wait for image 
generation to be sorted out before adding the toolchains, it may very 
well be a couple more years before everything is finalized... which 
would penalize the people who simply want the convenience of VS2017 
support to build regular ARM and AARCH64 applications.


IMO, a gradual approach to introducing VS2017 ARM/AARCH64 support might 
be preferable to an "all or nothing" one.


Regards,

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

Re: [edk2] [RFC 0/1] Stack trace support in X64 exception handling

2017-11-14 Thread Andrew Fish 

> On Nov 14, 2017, at 8:33 AM, Brian J. Johnson  wrote:
> 
> On 11/14/2017 09:37 AM, Paulo Alcantara wrote:
>> Hi Fan,
>> On 14/11/2017 12:03, Fan Jeff wrote:
>>> Paul,
>>> 
>>> I like this feature very much. Actually, I did some POC one year ago but I 
>>> did finalize it.
>>> 
>>> In my POC, I could use EBP to tack the stack frame on IAS32 arch.
>>> 
>>> But for x64, I tried to use –keepexceptiontable flag to explain stack frame 
>>> from the debug section of image.
>>> 
>>> I may workson MSFT toolchain, but it did now work well for GCC toolchain.
>>> 
>>> I think Eric could help to verify MSFT for your patch. If it works well, 
>>> that’s will be great!
>>> 
>>> Say again, I like this feature!!!:-)
>> Cool! Your help would be really appreciable! If we get this working for X64 
>> in both toolchains, that should be easy to port it to IA-32 as well.
>> Thank you very much for willing to help on that.
>> Paulo
> 
> Great feature!  You do need some sort of sanity check on the RIP and RBP 
> values, though, so if the stack gets corrupted or the RIP is nonsense from 
> following a bad pointer, you don't start dereferencing garbage addresses and 
> trigger an exception loop.
> 

Brian,

This was a long time ago and my memory might be fuzzy I think we talked to 
some debugger folks about unwinding the stack and they mentioned it was common 
for the C runtime to have a return address or frame pointer have a zero value 
so the unwind logic knows when to stop. This is in addition to generic sanity 
checking. 

We got an extra push $0 added to the stack switch to help with stack unwind. 
https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseLib/X64/SwitchStack.S
 


If might be a good idea to have a PCD for the max number of stack frames to 
display as a fallback for the error check. For X64 you may also have to add a 
check for a non-cononical address as that will GP fault. 

Thanks,

Andrew Fish


> For at least some versions of Microsoft's IA32 compiler, it's possible to 
> compile using EBP as a stack frame base pointer (like gcc) by using the 
> "/Oy-" switch.  The proposed unwind code should work in that case. The X64 
> compiler doesn't support this switch, though.
> 
> AFAIK the only way to unwind the stack with Microsoft's X64 compilers is to 
> parse the unwind info in the .pdata and .xdata sections.  Genfw.exe usually 
> strips those sections, but the "--keepexceptiontable" flag will preserve 
> them, as Jeff pointed out.  I've looked hard for open source code to decode 
> them, but haven't found any, even though the format is well documented.  And 
> I haven't gotten around to writing it myself.  I'd love it if someone could 
> contribute the code!
> 
> Another possibility is to use the branch history MSRs available on some 
> x86-family processors.  Recent Intel processors can use them as a stack, as 
> opposed to a circular list, so they can record a backtrace directly. (I'm not 
> familiar with AMD processors' capabilities.)  You can enable call stack 
> recording like this:
> 
>  #define LBR_ON_FLAG   0x0001
>  #define IA32_DEBUGCTL 0x1D9
>  #define CALL_STACK_SET_FLAG 0x3C4
>  #define CALL_STACK_CLR_FLAG 0xFC7
>  #define MSR_LBR_SELECT 0x1C8
> 
>  //
>  // Enable branch recording
>  //
>  LbControl = AsmReadMsr64 ((UINT32)IA32_DEBUGCTL);
>  LbControl |= LBR_ON_FLAG;
>  AsmWriteMsr64 ((UINT32)IA32_DEBUGCTL, LbControl);
> 
>  //
>  // Configure for call stack
>  //
>  LbSelect = AsmReadMsr64 ((UINT32)MSR_LBR_SELECT);
>  LbSelect &= CALL_STACK_CLR_FLAG;
>  LbSelect |= CALL_STACK_SET_FLAG;
>  AsmWriteMsr64((UINT32)MSR_LBR_SELECT, LbSelect);
> 
> The EIP/RIP values are logged in MSR_SANDY_BRIDGE_LASTBRANCH_n_FROM_IP and 
> MSR_SANDY_BRIDGE_LASTBRANCH_n_TO_IP, and the current depth is tracked in 
> MSR_LASTBRANCH_TOS.  This works quite well.  Gen10 (Sky Lake) processors 
> support 32 LASTBRANCH_n MSR pairs, which is sufficient in almost all cases.
> 
> Different processor generations have different branch recording capabilities, 
> and different numbers of LASTBRANCH_n MSRs; see Intel's manuals for details.
> 
> Thanks,
> Brian
> 
>>> 
>>> Thanks!
>>> 
>>> Jeff
>>> 
>>> *发件人: *Paulo Alcantara 
>>> *发送时间: *2017年11月14日21:23
>>> *收件人: *edk2-devel@lists.01.org 
>>> *抄送: *Rick Bramley ; Laszlo Ersek 
>>> ; Andrew Fish ; Eric Dong 
>>> 
>>> *主题: *Re: [edk2] [RFC 0/1] Stack trace support in X64 exception handling
>>> 
>>> Hi,
>>> 
>>> On 14/11/2017 10:47, Paulo Alcantara wrote:
 Hi,
 
 This series adds stack trace support during a X64 CPU exception.
 
 Informations like back trace, stack contents and image module names
 (that were part of the call stack) will be dumped out.
 
 We

[edk2] [Patch][edk2-platforms/devel-MinnowBoard3-UDK2017] BensonGlacier: Enable generic SPI device

2017-11-14 Thread Teemu Rytkonen 
-Enable generic SPI device for BensonGlacier config to be
used with SenseHat board.
-Enable GPIO config to enable SenseHat board programming

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Teemu Rytkonen 
---
 .../BensonGlacier/BoardInitPostMem/BoardGpios.h| 20 ++--
 .../AcpiTablesPCAT/PlatformSsdt/PlatformSsdt.asl   |  2 ++
 .../PlatformSsdt/Sensors/GenericSpi3.asl   | 38 ++
 3 files changed, 50 insertions(+), 10 deletions(-)
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/Acpi/AcpiTablesPCAT/PlatformSsdt/Sensors/GenericSpi3.asl

diff --git 
a/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardGpios.h 
b/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardGpios.h
index d72cd80c9..db48c4e85 100644
--- 
a/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardGpios.h
+++ 
b/Platform/BroxtonPlatformPkg/Board/BensonGlacier/BoardInitPostMem/BoardGpios.h
@@ -80,13 +80,13 @@ BXT_GPIO_PAD_INIT  mBenson_GpioInitData_N[] =
   BXT_GPIO_PAD_CONF(L"GPIO_14",  M1   ,NA, NA,  NA 
   ,   NA   , Wake_Disabled, P_20K_L,NA   ,NA ,NA   , NA, 
GPIO_PADBAR+0x0070,  NORTH),//Feature: LB
   BXT_GPIO_PAD_CONF(L"GPIO_15",  M1   ,NA, NA,  NA 
   ,   NA   , Wake_Disabled, P_20K_L,NA   ,NA ,NA   , NA, 
GPIO_PADBAR+0x0078,  NORTH),//Feature: LB
   BXT_GPIO_PAD_CONF(L"GPIO_16",  M0   ,GPI   ,  NA   ,  NA 
   ,   Edge , Wake_Disabled, P_20K_H, Inverted,IOAPIC,  HizRx0I ,DisPuPd, 
GPIO_PADBAR+0x0080,  NORTH),//Feature:SIM Card DetectNet in Sch: 
SIM_CON_CD1, falling edge trigger
-  BXT_GPIO_PAD_CONF(L"GPIO_17",  M0   ,GPI   , GPIO_D,  NA 
   ,   Edge , Wake_Disabled, P_NONE ,NA   ,IOAPIC, NA   ,DisPuPd, 
GPIO_PADBAR+0x0088, NORTH), // SOC_LSE_HOST_IRQ_N
+  BXT_GPIO_PAD_CONF(L"GPIO_17",  M0   ,GPI   , GPIO_D,  NA 
   ,   Edge , Wake_Disabled, P_NONE ,NA   ,IOAPIC, NA   ,DisPuPd, 
GPIO_PADBAR+0x0088, NORTH), // SOC_LSE_HOST_IRQ_N
   BXT_GPIO_PAD_CONF(L"GPIO_18",  M1   ,NA, NA,  NA 
   ,   NA   , Wake_Disabled, P_20K_H,NA   ,NA, NA   , NA, 
GPIO_PADBAR+0x0090,  NORTH),//Feature: LB
   BXT_GPIO_PAD_CONF(L"GPIO_19",  M1   ,NA, NA,  NA 
   ,   NA   , Wake_Disabled, P_20K_H,NA   ,NA, NA   , NA, 
GPIO_PADBAR+0x0098,  NORTH),//Feature: LB
   BXT_GPIO_PAD_CONF(L"GPIO_20",  M1   ,NA, NA,  NA 
   ,   NA   , Wake_Disabled, P_20K_L,NA   ,NA, NA   , NA, 
GPIO_PADBAR+0x00A0,  NORTH),//Feature: LB
   BXT_GPIO_PAD_CONF(L"GPIO_21",  M1   ,NA, NA,  NA 
   ,   NA   , Wake_Disabled, P_20K_L,NA   ,NA, NA   , NA, 
GPIO_PADBAR+0x00A8,  NORTH),//Feature: LB
   BXT_GPIO_PAD_CONF(L"GPIO_22",  M0   ,GPIO  ,GPIO_D ,  NA 
   ,   NA   , Wake_Disabled, P_20K_L,NA   ,NA, NA   , NA, 
GPIO_PADBAR+0x00B0,  NORTH),//Feature: LB
-  BXT_GPIO_PAD_CONF(L"GPIO_23",  M0   ,GPO   ,GPIO_D,   LO 
   ,   NA   , Wake_Disabled, P_20K_H,NA   ,NA, NA   ,   EnPu, 
GPIO_PADBAR+0x00B8, NORTH), // SOC_SUE_RST_N
+  BXT_GPIO_PAD_CONF(L"GPIO_23",  M0   ,GPO   ,GPIO_D,   LO 
   ,   NA   , Wake_Disabled, P_20K_H,NA   ,NA, NA   ,   EnPu, 
GPIO_PADBAR+0x00B8, NORTH), // SOC_SUE_RST_N
   BXT_GPIO_PAD_CONF(L"GPIO_24",  M0   ,GPO   ,GPIO_D ,  NA 
   ,   NA   , Wake_Disabled, P_20K_H,   NA,NA, NA   , NA, 
GPIO_PADBAR+0x00C0,  NORTH),//SATA_DEVSLP0
   BXT_GPIO_PAD_CONF(L"GPIO_25",  M0   ,GPO   ,GPIO_D ,  NA 
   ,   Level, Wake_Disabled, P_20K_H, Inverted,   SCI, NA   , NA, 
GPIO_PADBAR+0x00C8,  NORTH),//Feature:ODD MD/DA SCI  Net in Sch: 
SATA_ODD_DA_IN
   BXT_GPIO_PAD_CONF(L"GPIO_26",  M0   ,GPIO  ,GPIO_D ,  NA 
   ,   NA   , Wake_Disabled, P_20K_L,   NA,NA, NA   , NA, 
GPIO_PADBAR+0x00D0,  NORTH),//SATA_LEDN
@@ -216,16 +216,16 @@ BXT_GPIO_PAD_INIT  mBenson_GpioInitData_NW [] =
   BXT_GPIO_PAD_CONF(L"GPIO_106 GP_SSP_0_FS1",M0   ,GPI   , GPIO_D ,  
NA ,  NA  ,  Wake_Disabled, P_20K_H,   NA,NA,HizRx0I   ,  EnPd, 
  GPIO_PADBAR+0x01F8,  NORTHWEST),
   BXT_GPIO_PAD_CONF(L"GPIO_109 GP_SSP_0_RXD",M1   ,NA, NA,  NA 
,  NA  ,  Wake_Disabled, P_20K_H,   NA,NA,HizRx0I   ,  EnPd,   
GPIO_PADBAR+0x0200,  NORTHWEST),
   BXT_GPIO_PAD_CONF(L"GPIO_110 GP_SSP_0_TXD",M1   ,NA, NA,  NA 
,  NA  ,  Wake_Disabled, P_20K_H,   NA,NA,HizRx0I   ,  EnPd,   
GPIO_PADBAR+0x0208,  NORTHWEST),
-  BXT_GPIO_PAD_CONF(L"GPIO_111 GP_SSP_1_CLK",M0   ,GPI   ,GPIO_D,  NA  
   ,   NA

Re: [edk2] [PATCH 0/4] BaseTools: Add VS2017 support, including ARM and AARCH64

2017-11-14 Thread Pete Batard 

Hi Liming,

On 2017.11.14 16:03, Gao, Liming wrote:
  In fact, I have sent one patch serial to add VS2017 tool chain for 
IA32 and X64. https://lists.01.org/pipermail/edk2-devel/2017-October/016175.html


I missed that proposal, else I would have used it as my base. Thanks for 
pointing it out.



  In my patch, I disable warning 4701&4703, because they are also disabled in 
VS2015.


Okay. Then I see no objections to disabling them.

I didn't see the /wd options for those in tools_def.template with 
VS2015, so I thought these needed to be addressed.


But from your looking at your proposal, I realize that these are 
disabled in ProcessorBind.h. Obviously, I'll update my ARM/AARCH64 
proposal so that the warnings for these toolchains are disabled there as 
well, instead of tools_def.template.


In tools_def.template, to align other VS tool chain, I use VS2017_BIN for 32bit, 
VS2017_BINX64 for 64bit. Could you follow this rule to define VS2017_BINARM for

arm, VS2017_BINAARCH64 for AARCH64?


I will do that.

On VS2017, I notice it doesn't set VS150COMNTOOLS env by default. 
So, I use vswhere.exe to detect VS2017 installation path. When we build source

code with VS tool chain, we open normal cmd instead of VS cmd.


I see. I've always been opening a VS command prompt before calling the 
EDK setup script, so I built my proposal around that. But I agree that 
it makes sense to be able to build from regular prompt, so I will try to 
follow your lead.


My only concern is that it may be difficult to locate the relevant ARM 
toolchains without %WindowsSdkVerBinPath% being properly defined.


In your proposal, you seem to be hardcoding WINSDK10_PREFIX to something 
like "%ProgramFiles(x86)%\Windows Kits\10\bin\x86" but that won't work 
with ARM/ARM64 as we will need to get the actual version of the defayult 
SDK installed for VS2017 (e.g. "C:\Program Files (x86)\Windows 
Kits\10\bin\10.0.16299.0\") as the ARM toolchains will not work otherwise.


For instance, on my VS2017 installation the "arm64\" under "Windows 
Kits\10\bin\" does not contain the latest version of the ARM64 tools and 
libraries, which is problematic as proper ARM64 compilation support was 
only enabled with the latest update. Only the one under "Windows 
Kits\10\bin\10.0.16299.0\" does.


All in all, rather than try to guess the SDK path, I think we should try 
to locate and call "%ProgramFiles(x86)%\Microsoft Visual 
Studio\2017\Community\Common7\Tools\vsdevcmd\core\winsdk.bat", to have 
the VS environment provide us with the actual SDK version to use, as 
determined by Microsoft.



In your patch on ARM support,  is included in Base.h.
This is a windows header file. So, VS env must be setup to build source
code with ARM arch. Right?


That is correct. I had a quick look at removing that header, but didn't 
see much harm in keeping it. However, now that I understand our 
constraints better, I will make sure that header is not referenced.


I will also do the same for memset_ms.c and memcpy_ms.c, introduced for 
ARM/ARM64 (In CompilerIntrinsicsLib), as it references  for 
size_t.



Last, I suggest you base on my patch to add VS2017 ARM and AARCH64 arch.

> For your patches, I suggest you separate them per package.

I will follow both these suggestions and send a v2 when ready.

Regards,

/Pete




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

Re: [edk2] [RFC 1/1] UefiCpuPkg/CpuExceptionHandlerLib/X64: Add stack trace support

2017-11-14 Thread Brian J. Johnson 

On 11/14/2017 09:30 AM, Paulo Alcantara wrote:

Hi Andrew,

On 14/11/2017 12:01, Andrew Fish wrote:
C> Paulo,


Cool feature. How does this code deal with VC++ that code does not 
store the frame pointer and requires symbols to unwind.


I haven't tested in with MSVC, so I'd hope to get some help from Intel's 
guys to help supporting and testing it :-)


Regarding the symbols, I performed some tests by writing a userspace 
PE/COFF application [1] and tried to:


(a) handle the RUNTIME_FUNCTION entries in exception table (.pdata 
section) to find the function starting address and then its respective 
symbol name in COFF symbol table.


(b) Walk through UNWIND_INFO entries in .xdata section to figure out 
which CPU register a function used as a frame pointer or if it didn't 
use any at

all.

The problem I had with (a) was that the COFF symbol table is not mapped 
directory into the image's address space -- that is, the 
"PointerToSymbolTable" in File Header *should* be 0 as per PE/COFF 
format specific when handling an executable file rather a object file. 
Additionally, if it's non-zero, it contains a file offset rather than a 
RVA address, so impossible to parse it at runtime.


In (b), I realized that the CodeView format data in debug directory 
should be kept in a separate file (PDB file?) and they aren't mapped 
into image's address space as well.


I don't have so much experience with PE/COFF format, so please correct 
me if I'm mistaken.




You are correct that unfortunately, Microsoft's compilers don't put 
symbolic information in the executable file, they put it in a separate 
PDB file.  And the PDB file format is not documented (although the Wine 
project has reverse engineered parts of it) and changes with different 
compiler versions.  I've struggled with it before, and concluded that 
the only feasible way to parse it is to use the APIs Microsoft provides 
for that purpose, such as dbghelp.dll.  That doesn't work inside a BIOS, 
of course.


It is possible to define a simple, compiler-agnostic symbol table format 
and write a build-time tool to extract symbol data from the PDB files, 
convert it, and insert it into each module.  GenFw is a handy place to 
generate symbol data, since it's reformatting the images already.  I 
actually have code which does this  I'd have to get permission from 
my company's Open Source Review Board to release it, though.  That would 
take time.


Brian


IMHO, there should be exist a function like AsmGetFrameAddress() and/or 
AsmGetStackAddress() which would get implemented for both GCC and MSVC 
toolchains.


Thank you very much for your comments!

Also on the page fault you can print the fault address since it is in 
CR2.


Good point! We should also do that.

It should be possible to post process the text file and make a 
symbolicated backtrace.


Yes.

Thanks!
Paulo



Thanks,

Andrew Fish


On Nov 14, 2017, at 4:47 AM, Paulo Alcantara  wrote:

This patch adds stack trace support during a X64 CPU exception.

It will dump out back trace, stack contents as well as image module
names that were part of the call stack.

Contributed-under: TianoCore Contribution Agreement 1.1
Cc: Eric Dong 
Cc: Laszlo Ersek 
Signed-off-by: Paulo Alcantara 
---
UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c 
| 344 +++-

1 file changed, 342 insertions(+), 2 deletions(-)

diff --git 
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c 


index 65f0cff680..7048247be3 100644
--- 
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
+++ 
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c

@@ -14,6 +14,11 @@

#include "CpuExceptionCommon.h"

+//
+// Unknown PDB file name
+//
+GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 *mUnknownPdbFileName = 
"";

+
/**
   Return address map of exception handler template so that C code 
can generate

   exception tables.
@@ -243,6 +248,325 @@ DumpCpuContext (
}

/**
+  Dump stack contents.
+
+  @param[in]  ImageBase    Base address of PE/COFF image.
+  @param[out] PdbAbsoluteFilePath  Absolute path of PDB file.
+  @param[out] PdbFileName  File name of PDB file.
+**/
+STATIC
+VOID
+GetPdbFileName (
+  IN  UINTN    ImageBase,
+  OUT CHAR8    **PdbAbsoluteFilePath,
+  OUT CHAR8    **PdbFileName
+  )
+{
+  VOID   *PdbPointer;
+  CHAR8  *Str;
+
+  //
+  // Get PDB file name from PE/COFF image
+  //
+  PdbPointer = PeCoffLoaderGetPdbPointer ((VOID *)ImageBase);
+  if (PdbPointer == NULL) {
+    //
+    // No PDB file name found. Set it to an unknown file name.
+    //
+    *PdbFileName = (CHAR8 *)mUnknownPdbFileName;
+    if (PdbAbsoluteFilePath != NULL) {
+  *PdbAbsoluteFilePath = NULL;
+    }
+  } else {
+    //
+    // Get file name portion out of PDB file in PE/COFF image
+    //
+    Str =

Re: [edk2] 答复: [RFC 0/1] Stack trace support in X64 exception handling

2017-11-14 Thread Brian J. Johnson 

On 11/14/2017 09:37 AM, Paulo Alcantara wrote:

Hi Fan,

On 14/11/2017 12:03, Fan Jeff wrote:

Paul,

I like this feature very much. Actually, I did some POC one year ago 
but I did finalize it.


In my POC, I could use EBP to tack the stack frame on IAS32 arch.

But for x64, I tried to use –keepexceptiontable flag to explain stack 
frame from the debug section of image.


I may workson MSFT toolchain, but it did now work well for GCC toolchain.

I think Eric could help to verify MSFT for your patch. If it works 
well, that’s will be great!


Say again, I like this feature!!!:-)


Cool! Your help would be really appreciable! If we get this working for 
X64 in both toolchains, that should be easy to port it to IA-32 as well.


Thank you very much for willing to help on that.

Paulo


Great feature!  You do need some sort of sanity check on the RIP and RBP 
values, though, so if the stack gets corrupted or the RIP is nonsense 
from following a bad pointer, you don't start dereferencing garbage 
addresses and trigger an exception loop.


For at least some versions of Microsoft's IA32 compiler, it's possible 
to compile using EBP as a stack frame base pointer (like gcc) by using 
the "/Oy-" switch.  The proposed unwind code should work in that case. 
The X64 compiler doesn't support this switch, though.


AFAIK the only way to unwind the stack with Microsoft's X64 compilers is 
to parse the unwind info in the .pdata and .xdata sections.  Genfw.exe 
usually strips those sections, but the "--keepexceptiontable" flag will 
preserve them, as Jeff pointed out.  I've looked hard for open source 
code to decode them, but haven't found any, even though the format is 
well documented.  And I haven't gotten around to writing it myself.  I'd 
love it if someone could contribute the code!


Another possibility is to use the branch history MSRs available on some 
x86-family processors.  Recent Intel processors can use them as a stack, 
as opposed to a circular list, so they can record a backtrace directly. 
(I'm not familiar with AMD processors' capabilities.)  You can enable 
call stack recording like this:


  #define LBR_ON_FLAG   0x0001
  #define IA32_DEBUGCTL 0x1D9
  #define CALL_STACK_SET_FLAG 0x3C4
  #define CALL_STACK_CLR_FLAG 0xFC7
  #define MSR_LBR_SELECT 0x1C8

  //
  // Enable branch recording
  //
  LbControl = AsmReadMsr64 ((UINT32)IA32_DEBUGCTL);
  LbControl |= LBR_ON_FLAG;
  AsmWriteMsr64 ((UINT32)IA32_DEBUGCTL, LbControl);

  //
  // Configure for call stack
  //
  LbSelect = AsmReadMsr64 ((UINT32)MSR_LBR_SELECT);
  LbSelect &= CALL_STACK_CLR_FLAG;
  LbSelect |= CALL_STACK_SET_FLAG;
  AsmWriteMsr64((UINT32)MSR_LBR_SELECT, LbSelect);

The EIP/RIP values are logged in MSR_SANDY_BRIDGE_LASTBRANCH_n_FROM_IP 
and MSR_SANDY_BRIDGE_LASTBRANCH_n_TO_IP, and the current depth is 
tracked in MSR_LASTBRANCH_TOS.  This works quite well.  Gen10 (Sky Lake) 
processors support 32 LASTBRANCH_n MSR pairs, which is sufficient in 
almost all cases.


Different processor generations have different branch recording 
capabilities, and different numbers of LASTBRANCH_n MSRs; see Intel's 
manuals for details.


Thanks,
Brian





Thanks!

Jeff

*发件人: *Paulo Alcantara 
*发送时间: *2017年11月14日21:23
*收件人: *edk2-devel@lists.01.org 
*抄送: *Rick Bramley ; Laszlo Ersek 
; Andrew Fish ; Eric 
Dong 
*主题: *Re: [edk2] [RFC 0/1] Stack trace support in X64 exception 
handling


Hi,

On 14/11/2017 10:47, Paulo Alcantara wrote:

Hi,

This series adds stack trace support during a X64 CPU exception.

Informations like back trace, stack contents and image module names
(that were part of the call stack) will be dumped out.

We already have such support in ARM/AArch64 (IIRC) exception handling
(thanks to Ard), and then I thought we'd also deserve it in X64 and
IA-32 platforms.

What do you think guys?

BTW, I've tested this only with OVMF (X64 only), using:
    - gcc-6.3.0, GCC5, NOOPT

Any other tests  would be really appreciable.


I've attached a file to show you how the trace would look like.

Thanks!
Paulo



Thanks!
Paulo

Repo:   https://github.com/pcacjr/edk2.git
Branch: stacktrace_x64

Cc: Rick Bramley 
Cc: Andrew Fish 
Cc: Eric Dong 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara 
---

Paulo Alcantara (1):
    UefiCpuPkg/CpuExceptionHandlerLib/X64: Add stack trace support

   
UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c 
| 344 +++-

   1 file changed, 342 insertions(+), 2 deletions(-)




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



--

Re: [edk2] [PATCH 0/4] BaseTools: Add VS2017 support, including ARM and AARCH64

2017-11-14 Thread Kurt Kennett 
Does this build and boot a platform?

This should not be committed until a platform is building and booting to UEFI 
shell, IMO.

K2

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Pete 
Batard
Sent: Tuesday, November 14, 2017 4:32 AM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH 0/4] BaseTools: Add VS2017 support, including ARM and 
AARCH64

Seeing that Visual Studio Update 4 finally added native ARM64 compilation 
support and this is an open task:
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FTasks=02%7C01%7Ckurt.kennett%40microsoft.com%7C0062ea8181264455563b08d52b5bd042%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636462595702607346=L4JTYL92%2FYzobAaoY5GiTevCKF2f3KhByhmJHFCeVc4%3D=0

This series is broken down into:
- A preliminary patch, to eliminate new warnings that would be produced for
  VS2017/IA32
- IA32 + X64 support
- ARM support
- AARCH64 support

Notes:
- Considering that the multiplication of toolchain flavours is probably not in
  our best interest when it comes to maintenance, I deliberately chose not to
  create extra VS2017 variants (for x64, ASL and EBC). For one thing, I had
  some issues with the x64 tools for ARM compilation (which may very well have
  been my own), and I'd rather see the variants being added after their need
  has been justified by their potential users, rather than preemptively.
- ARM and ARM64 should be considered EXPERIMENTAL at this stage.
  I've had some good success compiling and running moderately complex drivers
  (e.g. ZFS file system driver) using the proposed patches on ARM/ARM64, and I
  am also confident that simple applications should be fine, but more work is
  required to produce QEMU ARM/AARCH64 firmware images, mostly due to missing
  .asm files. Since most of the RVCT .asm files can be reused mostly unchanged
  for MSFT, I was able to cajole the ARM process to go up to the QEMU image
  generation (which failed due to some alignment issue), which gives me some
  confidence that we should eventually be able to use the MSFT toolchain to
  produce working images for ARM and ARM64, but this will require some work.
- While the VS2017 version of IA32 and X64 do not silence any level 4 warnings
  by default, to bring them to par with VS2015, the ARM and ARM64 versions
  have to silence 3 of them, as VS is a bit less leniant than gcc/Clang.
  Again, it should be possible to remove the silencing of these warnings
  eventually, but this will require some work.
- Finally, you'll see that a whole section of build_rule.template had to be
  duplicated just so that we could remove the --convert-hex option for the
  MSFT ARM/ARM64 assemblers. Maybe there is a better way to achieve the
  removal of that option, but I haven't found it.

Regards,

/Pete

Pete Batard (4):
  MdeModulePkg, NetworkPkg: Fix VS2017 IA32 warnings
  BaseTools: Add VS2017 IA32 and X64 support
  BaseTools: Add VS2017 ARM support
  BaseTools: Add VS2017 AARCH64 support

 ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm 
  | 255 
 ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtsrsh.asm
  |  45 
 ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf 
  |  13 +-
 ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c   
  |  30 +++
 ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c   
  |  29 +++
 BaseTools/Conf/build_rule.template 
  |  30 +++
 BaseTools/Conf/tools_def.template  
  | 168 +
 BaseTools/set_vsprefix_envs.bat
  |   9 +
 MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c  
  |   2 +-
 
MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerCustomizedUiSupport.c
 |   2 +-
 MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c  
  |   2 +-
 MdeModulePkg/Universal/Network/MnpDxe/MnpMain.c
  |   2 +-
 MdePkg/Include/Base.h  
  |  15 ++
 MdePkg/Library/BaseLib/AArch64/CpuBreakpoint.asm   
  |  39 +++
 MdePkg/Library/BaseLib/AArch64/DisableInterrupts.asm   
  |  37 +++
 MdePkg/Library/BaseLib/AArch64/EnableInterrupts.asm
  |  37 +++
 MdePkg/Library/BaseLib/AArch64/GetInterruptsState.asm  
  |  49 
 MdePkg/Library/BaseLib/AArch64/MemoryFence.asm 
  |  38 +++

Re: [edk2] [PATCH 0/4] BaseTools: Add VS2017 support, including ARM and AARCH64

2017-11-14 Thread Gao, Liming 
Pete:
 Thanks for your contribution. In fact, I have sent one patch serial to add 
VS2017 tool chain for IA32 and X64. 
https://lists.01.org/pipermail/edk2-devel/2017-October/016175.html 
 In my patch, I disable warning 4701&4703, because they are also disabled in 
VS2015. In tools_def.template, to align other VS tool chain, I use VS2017_BIN 
for 32bit, VS2017_BINX64 for 64bit. Could you follow this rule to define 
VS2017_BINARM for arm, VS2017_BINAARCH64 for AARCH64? On VS2017, I notice it 
doesn't set VS150COMNTOOLS env by default. So, I use vswhere.exe to detect 
VS2017 installation path. When we build source code with VS tool chain, we open 
normal cmd instead of VS cmd. So, we need to consider the case without VS env. 
In your patch on ARM support,  is included in Base.h. This is a 
windows header file. So, VS env must be setup to build source code with ARM 
arch. Right? 
 Last, I suggest you base on my patch to add VS2017 ARM and AARCH64 arch. I 
will still work on VS2017 IA32 and X64. For your patches, I suggest you 
separate them per package. For example, to add ARM arch, you can create one 
patch in BaseTools, one patch in MdePkg, another one is ArmPkg. 

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Pete 
> Batard
> Sent: Tuesday, November 14, 2017 8:32 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 0/4] BaseTools: Add VS2017 support, including ARM and 
> AARCH64
> 
> Seeing that Visual Studio Update 4 finally added native ARM64 compilation
> support and this is an open task:
> https://github.com/tianocore/tianocore.github.io/wiki/Tasks
> 
> This series is broken down into:
> - A preliminary patch, to eliminate new warnings that would be produced for
>   VS2017/IA32
> - IA32 + X64 support
> - ARM support
> - AARCH64 support
> 
> Notes:
> - Considering that the multiplication of toolchain flavours is probably not in
>   our best interest when it comes to maintenance, I deliberately chose not to
>   create extra VS2017 variants (for x64, ASL and EBC). For one thing, I had
>   some issues with the x64 tools for ARM compilation (which may very well have
>   been my own), and I'd rather see the variants being added after their need
>   has been justified by their potential users, rather than preemptively.
> - ARM and ARM64 should be considered EXPERIMENTAL at this stage.
>   I've had some good success compiling and running moderately complex drivers
>   (e.g. ZFS file system driver) using the proposed patches on ARM/ARM64, and I
>   am also confident that simple applications should be fine, but more work is
>   required to produce QEMU ARM/AARCH64 firmware images, mostly due to missing
>   .asm files. Since most of the RVCT .asm files can be reused mostly unchanged
>   for MSFT, I was able to cajole the ARM process to go up to the QEMU image
>   generation (which failed due to some alignment issue), which gives me some
>   confidence that we should eventually be able to use the MSFT toolchain to
>   produce working images for ARM and ARM64, but this will require some work.
> - While the VS2017 version of IA32 and X64 do not silence any level 4 warnings
>   by default, to bring them to par with VS2015, the ARM and ARM64 versions
>   have to silence 3 of them, as VS is a bit less leniant than gcc/Clang.
>   Again, it should be possible to remove the silencing of these warnings
>   eventually, but this will require some work.
> - Finally, you'll see that a whole section of build_rule.template had to be
>   duplicated just so that we could remove the --convert-hex option for the
>   MSFT ARM/ARM64 assemblers. Maybe there is a better way to achieve the
>   removal of that option, but I haven't found it.
> 
> Regards,
> 
> /Pete
> 
> Pete Batard (4):
>   MdeModulePkg, NetworkPkg: Fix VS2017 IA32 warnings
>   BaseTools: Add VS2017 IA32 and X64 support
>   BaseTools: Add VS2017 ARM support
>   BaseTools: Add VS2017 AARCH64 support
> 
>  ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm   
> | 255
> 
>  ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtsrsh.asm  
> |  45 
>  ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf   
> |  13 +-
>  ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c 
> |  30 +++
>  ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c 
> |  29 +++
>  BaseTools/Conf/build_rule.template   
> |  30 +++
>  BaseTools/Conf/tools_def.template
> | 168 +
>  BaseTools/set_vsprefix_envs.bat  
> |   9 +
>  MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
> |   2 +-
>  
>

Re: [edk2] [PATCH 0/2] Refine UDF related codes

2017-11-14 Thread Paulo Alcantara 

Hi Hao,

On 14/11/2017 05:51, Hao Wu wrote:

The series will do the following refinements:
a. Merge the discovery of the El Torito feature on CD/DVD media into the
detect of UDF;
b. Avoid possible loss track of the allocated buffer pointer in UdfDxe.

Cc: Paulo Alcantara 
Cc: Ruiyu Ni 
Cc: Star Zeng 
Cc: Eric Dong 

Hao Wu (2):
   MdeModulePkg/PartitionDxe: Merge the discovery of ElTorito into UDF
   MdeModulePkg/UdfDxe: Avoid possible loss track of allocated buffer

  MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c  |  7 +++
  MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c| 17 
+
  MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 12 +++-
  3 files changed, 27 insertions(+), 9 deletions(-)



Looks good to me. Thanks for fixing and testing it!

Reviewed-by: Paulo Alcantara 

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

Re: [edk2] 答复: [RFC 0/1] Stack trace support in X64 exception handling

2017-11-14 Thread Paulo Alcantara 

Hi Fan,

On 14/11/2017 12:03, Fan Jeff wrote:

Paul,

I like this feature very much. Actually, I did some POC one year ago but 
I did finalize it.


In my POC, I could use EBP to tack the stack frame on IAS32 arch.

But for x64, I tried to use –keepexceptiontable flag to explain stack 
frame from the debug section of image.


I may workson MSFT toolchain, but it did now work well for GCC toolchain.

I think Eric could help to verify MSFT for your patch. If it works well, 
that’s will be great!


Say again, I like this feature!!!:-)


Cool! Your help would be really appreciable! If we get this working for 
X64 in both toolchains, that should be easy to port it to IA-32 as well.


Thank you very much for willing to help on that.

Paulo



Thanks!

Jeff

*发件人: *Paulo Alcantara 
*发送时间: *2017年11月14日21:23
*收件人: *edk2-devel@lists.01.org 
*抄送: *Rick Bramley ; Laszlo Ersek 
; Andrew Fish ; Eric 
Dong 

*主题: *Re: [edk2] [RFC 0/1] Stack trace support in X64 exception handling

Hi,

On 14/11/2017 10:47, Paulo Alcantara wrote:

Hi,

This series adds stack trace support during a X64 CPU exception.

Informations like back trace, stack contents and image module names
(that were part of the call stack) will be dumped out.

We already have such support in ARM/AArch64 (IIRC) exception handling
(thanks to Ard), and then I thought we'd also deserve it in X64 and
IA-32 platforms.

What do you think guys?

BTW, I've tested this only with OVMF (X64 only), using:
    - gcc-6.3.0, GCC5, NOOPT

Any other tests  would be really appreciable.


I've attached a file to show you how the trace would look like.

Thanks!
Paulo



Thanks!
Paulo

Repo:   https://github.com/pcacjr/edk2.git
Branch: stacktrace_x64

Cc: Rick Bramley 
Cc: Andrew Fish 
Cc: Eric Dong 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara 
---

Paulo Alcantara (1):
    UefiCpuPkg/CpuExceptionHandlerLib/X64: Add stack trace support

   UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c | 344 
+++-
   1 file changed, 342 insertions(+), 2 deletions(-)




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

Re: [edk2] [PATCH] CorebootModulePkg/CbSupportDxe: Remove duplicated IO Space addition

2017-11-14 Thread Ma, Maurice 
Looks good to me.
Reviewed-by: Maurice Ma 

Thanks
Maurice

-Original Message-
From: You, Benjamin 
Sent: Tuesday, November 14, 2017 12:41 AM
To: edk2-devel@lists.01.org
Cc: Ma, Maurice ; Agyeman, Prince 

Subject: [PATCH] CorebootModulePkg/CbSupportDxe: Remove duplicated IO Space 
addition

Since UefiCpuPkg's CpuDxe Driver already adds Local Apic's MMIO space to GCD, 
CorebootModulePkg's CbSupportDxe should not do this again. Doing this again 
causes error return status from GCD service, and ASSERT (FALSE) with debug 
build, so the duplicated addition is removed.

Cc: Maurice Ma 
Cc: Prince Agyeman 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Benjamin You 
---
 CorebootModulePkg/CbSupportDxe/CbSupportDxe.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c 
b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
index 24bacf815c..c526c9e871 100755
--- a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
+++ b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
@@ -140,9 +140,6 @@ CbDxeEntryPoint (
   //
   // Report MMIO/IO Resources
   //
-  Status = CbReserveResourceInGcd (TRUE, EfiGcdMemoryTypeMemoryMappedIo, 
0xFEE0, SIZE_1MB, 0, SystemTable); // LAPIC
-  ASSERT_EFI_ERROR (Status);
-
   Status = CbReserveResourceInGcd (TRUE, EfiGcdMemoryTypeMemoryMappedIo, 
0xFEC0, SIZE_4KB, 0, SystemTable); // IOAPIC
   ASSERT_EFI_ERROR (Status);
 
--
2.14.3.windows.1

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

Re: [edk2] [RFC 1/1] UefiCpuPkg/CpuExceptionHandlerLib/X64: Add stack trace support

2017-11-14 Thread Paulo Alcantara 

Hi Andrew,

On 14/11/2017 12:01, Andrew Fish wrote:
C> Paulo,


Cool feature. How does this code deal with VC++ that code does not store the 
frame pointer and requires symbols to unwind.


I haven't tested in with MSVC, so I'd hope to get some help from Intel's 
guys to help supporting and testing it :-)


Regarding the symbols, I performed some tests by writing a userspace 
PE/COFF application [1] and tried to:


(a) handle the RUNTIME_FUNCTION entries in exception table (.pdata 
section) to find the function starting address and then its respective 
symbol name in COFF symbol table.


(b) Walk through UNWIND_INFO entries in .xdata section to figure out 
which CPU register a function used as a frame pointer or if it didn't 
use any at

all.

The problem I had with (a) was that the COFF symbol table is not mapped 
directory into the image's address space -- that is, the 
"PointerToSymbolTable" in File Header *should* be 0 as per PE/COFF 
format specific when handling an executable file rather a object file. 
Additionally, if it's non-zero, it contains a file offset rather than a 
RVA address, so impossible to parse it at runtime.


In (b), I realized that the CodeView format data in debug directory 
should be kept in a separate file (PDB file?) and they aren't mapped 
into image's address space as well.


I don't have so much experience with PE/COFF format, so please correct 
me if I'm mistaken.


IMHO, there should be exist a function like AsmGetFrameAddress() and/or 
AsmGetStackAddress() which would get implemented for both GCC and MSVC 
toolchains.


Thank you very much for your comments!


Also on the page fault you can print the fault address since it is in CR2.


Good point! We should also do that.


It should be possible to post process the text file and make a symbolicated 
backtrace.


Yes.

Thanks!
Paulo



Thanks,

Andrew Fish


On Nov 14, 2017, at 4:47 AM, Paulo Alcantara  wrote:

This patch adds stack trace support during a X64 CPU exception.

It will dump out back trace, stack contents as well as image module
names that were part of the call stack.

Contributed-under: TianoCore Contribution Agreement 1.1
Cc: Eric Dong 
Cc: Laszlo Ersek 
Signed-off-by: Paulo Alcantara 
---
UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c | 344 
+++-
1 file changed, 342 insertions(+), 2 deletions(-)

diff --git 
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c 
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
index 65f0cff680..7048247be3 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
@@ -14,6 +14,11 @@

#include "CpuExceptionCommon.h"

+//
+// Unknown PDB file name
+//
+GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 *mUnknownPdbFileName = "";
+
/**
   Return address map of exception handler template so that C code can generate
   exception tables.
@@ -243,6 +248,325 @@ DumpCpuContext (
}

/**
+  Dump stack contents.
+
+  @param[in]  ImageBaseBase address of PE/COFF image.
+  @param[out] PdbAbsoluteFilePath  Absolute path of PDB file.
+  @param[out] PdbFileName  File name of PDB file.
+**/
+STATIC
+VOID
+GetPdbFileName (
+  IN  UINTNImageBase,
+  OUT CHAR8**PdbAbsoluteFilePath,
+  OUT CHAR8**PdbFileName
+  )
+{
+  VOID   *PdbPointer;
+  CHAR8  *Str;
+
+  //
+  // Get PDB file name from PE/COFF image
+  //
+  PdbPointer = PeCoffLoaderGetPdbPointer ((VOID *)ImageBase);
+  if (PdbPointer == NULL) {
+//
+// No PDB file name found. Set it to an unknown file name.
+//
+*PdbFileName = (CHAR8 *)mUnknownPdbFileName;
+if (PdbAbsoluteFilePath != NULL) {
+  *PdbAbsoluteFilePath = NULL;
+}
+  } else {
+//
+// Get file name portion out of PDB file in PE/COFF image
+//
+Str = (CHAR8 *)((UINTN)PdbPointer +
+AsciiStrLen ((CHAR8 *)PdbPointer) - sizeof *Str);
+for (; *Str != '/' && *Str != '\\'; Str--) {
+  ;
+}
+
+//
+// Set PDB file name (also skip trailing path separator: '/' or '\\')
+//
+*PdbFileName = Str + 1;
+
+if (PdbAbsoluteFilePath != NULL) {
+  //
+  // Set absolute file path of PDB file
+  //
+  *PdbAbsoluteFilePath = PdbPointer;
+}
+  }
+}
+
+/**
+  Dump stack contents.
+
+  @param[in]  CurrentRsp Current stack pointer address.
+  @param[in]  UnwondStacksCount  Count of unwond stack frames.
+**/
+STATIC
+VOID
+DumpStackContents (
+  IN UINT64  CurrentRsp,
+  IN INTNUnwondStacksCount
+  )
+{
+  if (UnwondStacksCount == 0) {
+return;
+  }
+
+  //
+  // Dump out stack contents
+  //
+  InternalPrintMessage ("\nStack dump:\n");
+  while (UnwondStacksCount-- > 0) {
+InternalPrintMessage (
+  "0x%016lx: %016lx %016lx\n",
+  CurrentRsp,
+  *(UINT64 *)CurrentRsp,
+  *(UINT64

Re: [edk2] [RFC 1/1] UefiCpuPkg/CpuExceptionHandlerLib/X64: Add stack trace support

2017-11-14 Thread Andrew Fish 

> On Nov 14, 2017, at 6:26 AM, Fan Jeff  wrote:
> 
> Andrew,
>  
> We could use he EIP offset in Paul’s trace message and work with the 
> generated map file under debug directory for debug trace.

It would also be possible to use gdb. 

Given
0 0x7E510F7F @ 0x7E509000+0x7F7E (0x7F762CB0) in 
PartitionDxe.dll

If you load PartitionDxe.dll into gdb you can then do "l *0x7F7E" to dump the 
source. 

I'm mapping lldb behavior to gdb, but it should be close. 

Thanks,

Andrew Fish

>  
> Jeff
>  
> 发件人: Andrew Fish 
> 发送时间: 2017年11月14日 22:01
> 收件人: Paulo Alcantara 
> 抄送: edk2-devel@lists.01.org ; Laszlo Ersek 
> ; Eric Dong 
> 主题: Re: [edk2] [RFC 1/1] UefiCpuPkg/CpuExceptionHandlerLib/X64: Add stack 
> trace support
>  
> Paulo,
> 
> Cool feature. How does this code deal with VC++ that code does not store the 
> frame pointer and requires symbols to unwind. 
> 
> Also on the page fault you can print the fault address since it is in CR2. 
> 
> It should be possible to post process the text file and make a symbolicated 
> backtrace. 
> 
> Thanks,
> 
> Andrew Fish
> 
> > On Nov 14, 2017, at 4:47 AM, Paulo Alcantara  wrote:
> > 
> > This patch adds stack trace support during a X64 CPU exception.
> > 
> > It will dump out back trace, stack contents as well as image module
> > names that were part of the call stack.
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Cc: Eric Dong 
> > Cc: Laszlo Ersek 
> > Signed-off-by: Paulo Alcantara 
> > ---
> > UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c | 344 
> > +++-
> > 1 file changed, 342 insertions(+), 2 deletions(-)
> > 
> > diff --git 
> > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c 
> > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
> > index 65f0cff680..7048247be3 100644
> > --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
> > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
> > @@ -14,6 +14,11 @@
> > 
> > #include "CpuExceptionCommon.h"
> > 
> > +//
> > +// Unknown PDB file name
> > +//
> > +GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 *mUnknownPdbFileName = "";
> > +
> > /**
> >   Return address map of exception handler template so that C code can 
> > generate
> >   exception tables.
> > @@ -243,6 +248,325 @@ DumpCpuContext (
> > }
> > 
> > /**
> > +  Dump stack contents.
> > +
> > +  @param[in]  ImageBaseBase address of PE/COFF image.
> > +  @param[out] PdbAbsoluteFilePath  Absolute path of PDB file.
> > +  @param[out] PdbFileName  File name of PDB file.
> > +**/
> > +STATIC
> > +VOID
> > +GetPdbFileName (
> > +  IN  UINTNImageBase,
> > +  OUT CHAR8**PdbAbsoluteFilePath,
> > +  OUT CHAR8**PdbFileName
> > +  )
> > +{
> > +  VOID   *PdbPointer;
> > +  CHAR8  *Str;
> > +
> > +  //
> > +  // Get PDB file name from PE/COFF image
> > +  //
> > +  PdbPointer = PeCoffLoaderGetPdbPointer ((VOID *)ImageBase);
> > +  if (PdbPointer == NULL) {
> > +//
> > +// No PDB file name found. Set it to an unknown file name.
> > +//
> > +*PdbFileName = (CHAR8 *)mUnknownPdbFileName;
> > +if (PdbAbsoluteFilePath != NULL) {
> > +  *PdbAbsoluteFilePath = NULL;
> > +}
> > +  } else {
> > +//
> > +// Get file name portion out of PDB file in PE/COFF image
> > +//
> > +Str = (CHAR8 *)((UINTN)PdbPointer +
> > +AsciiStrLen ((CHAR8 *)PdbPointer) - sizeof *Str);
> > +for (; *Str != '/' && *Str != '\\'; Str--) {
> > +  ;
> > +}
> > +
> > +//
> > +// Set PDB file name (also skip trailing path separator: '/' or '\\')
> > +//
> > +*PdbFileName = Str + 1;
> > +
> > +if (PdbAbsoluteFilePath != NULL) {
> > +  //
> > +  // Set absolute file path of PDB file
> > +  //
> > +  *PdbAbsoluteFilePath = PdbPointer;
> > +}
> > +  }
> > +}
> > +
> > +/**
> > +  Dump stack contents.
> > +
> > +  @param[in]  CurrentRsp Current stack pointer address.
> > +  @param[in]  UnwondStacksCount  Count of unwond stack frames.
> > +**/
> > +STATIC
> > +VOID
> > +DumpStackContents (
> > +  IN UINT64  CurrentRsp,
> > +  IN INTNUnwondStacksCount
> > +  )
> > +{
> > +  if (UnwondStacksCount == 0) {
> > +return;
> > +  }
> > +
> > +  //
> > +  // Dump out stack contents
> > +  //
> > +  InternalPrintMessage ("\nStack dump:\n");
> > +  while (UnwondStacksCount-- > 0) {
> > +InternalPrintMessage (
> > +  "0x%016lx: %016lx %016lx\n",
> > +  CurrentRsp,
> > +  *(UINT64 *)CurrentRsp,
> > +  *(UINT64 *)((UINTN)CurrentRsp + 8)
> > +  );
> > +
> > +//
> > +// As per Microsoft x64 ABI, the stack pointer must be aligned on a 16 
> >

Re: [edk2] [PATCH v5] UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in memory map

2017-11-14 Thread Wang, Jian J 
Hi Laszlo,

I did some investigation works on this issue. I think I found the root cause and
tend to believe this is a Fedora kernel issue. Here're proves:

memmap output patterns in which Fedora 26 failed to boot:
1) BS_DataAE20-AE20EFFF 000F 
0002600F
RT_DataAE20F000-AE38EFFF 0180 
8002600F
RT_CodeAE38F000-AE48EFFF 0100 
8002600F
Reserved   AE48F000-AE58EFFF 0100 
0002600F

2) BS_DataAE20-AE20EFFF 000F 
000F
RT_DataAE20F000-AE38EFFF 0180 
800F
RT_CodeAE38F000-AE467FFF 00D9 
800F
RT_CodeAE468000-AE469FFF 0002 
8002600F
RT_CodeAE46A000-AE46EFFF 0005 
800F
RT_CodeAE46F000-AE470FFF 0002 
8002600F
RT_CodeAE471000-AE475FFF 0005 
800F
RT_CodeAE476000-AE479FFF 0004 
8002600F
RT_CodeAE47A000-AE47 0006 
800F
RT_CodeAE48-AE481FFF 0002 
8002600F
RT_CodeAE482000-AE487FFF 0006 
800F
RT_CodeAE488000-AE489FFF 0002 
8002600F
RT_CodeAE48A000-AE48EFFF 0005 
800F
Reserved   AE48F000-AE58EFFF 0100 
000F

3) RT_DataAE20F000-AE38EFFF 0180 
800F
RT_CodeAE38F000-AE418FFF 008A 
800F
RT_CodeAE419000-AE48EFFF 0076 
8002600F
Reserved   AE48F000-AE58EFFF 0100 
000F

4) BS_DataAE20-AE20EFFF 000F 
0002400F
RT_DataAE20F000-AE38EFFF 0180 
8002400F
RT_CodeAE38F000-AE48EFFF 0100 
8002400F
Reserved   AE48F000-AE58EFFF 0100 
0002400F


memmap output pattern in which Fedora 26 booted successfully
5) BS_DataAE20-AE20EFFF 000F 
000F
RT_DataAE20F000-AE38EFFF 0180 
800F
RT_CodeAE38F000-AE48EFFF 0100 
800F
Reserved   AE48F000-AE58EFFF 0100 
000F

6) BS_DataAE20-AE20EFFF 000F 
000F
RT_DataAE20F000-AE38EFFF 0180 
800F
RT_CodeAE38F000-AE418FFF 008A 
800F
RT_CodeAE419000-AE419FFF 0001 
8000400F
RT_CodeAE41A000-AE41BFFF 0002 
8002000F
RT_CodeAE41C000-AE420FFF 0005 
8000400F
RT_CodeAE421000-AE422FFF 0002 
8002000F
RT_CodeAE423000-AE427FFF 0005 
8000400F
RT_CodeAE428000-AE42AFFF 0003 
8002000F
RT_CodeAE42B000-AE42 0005 
8000400F
RT_CodeAE43-AE432FFF 0003 
8002000F
RT_CodeAE433000-AE439FFF 0007 
8000400F
RT_CodeAE43A000-AE43DFFF 0004 
8002000F
RT_CodeAE43E000-AE444FFF 0007 
8000400F
RT_CodeAE445000-AE448FFF 0004 
8002000F
RT_CodeAE449000-AE44EFFF 0006 
8000400F
RT_CodeAE44F000-AE450FFF 0002 
8002000F
RT_CodeAE451000-AE455FFF 0005 
8000400F
RT_CodeAE456000-AE458FFF 0003 
8002000F
RT_CodeAE459000-AE45 0007 
8000400F
RT_CodeAE46-AE461FFF 0002 
8002000F
RT_CodeAE462000-AE467FFF 0006 
8000400F
RT_CodeAE468000-AE469FFF 0002 
8002000F
RT_CodeAE46A000-AE46EFFF 0005 
8000400F
RT_CodeAE46F000-AE470FFF 0002 
8002000F
RT_CodeAE471000-AE475FFF 0005 
8000400F
RT_Code

[edk2] 答复: [RFC 1/1] UefiCpuPkg/CpuExceptionHandlerLib/X64: Add stack trace support

2017-11-14 Thread Fan Jeff 
Andrew,

We could use he EIP offset in Paul’s trace message and work with the generated 
map file under debug directory for debug trace.

Jeff

发件人: Andrew Fish
发送时间: 2017年11月14日 22:01
收件人: Paulo Alcantara
抄送: edk2-devel@lists.01.org; Laszlo 
Ersek; Eric Dong
主题: Re: [edk2] [RFC 1/1] UefiCpuPkg/CpuExceptionHandlerLib/X64: Add stack trace 
support

Paulo,

Cool feature. How does this code deal with VC++ that code does not store the 
frame pointer and requires symbols to unwind.

Also on the page fault you can print the fault address since it is in CR2.

It should be possible to post process the text file and make a symbolicated 
backtrace.

Thanks,

Andrew Fish

> On Nov 14, 2017, at 4:47 AM, Paulo Alcantara  wrote:
>
> This patch adds stack trace support during a X64 CPU exception.
>
> It will dump out back trace, stack contents as well as image module
> names that were part of the call stack.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Signed-off-by: Paulo Alcantara 
> ---
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c | 344 
> +++-
> 1 file changed, 342 insertions(+), 2 deletions(-)
>
> diff --git 
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c 
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
> index 65f0cff680..7048247be3 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
> @@ -14,6 +14,11 @@
>
> #include "CpuExceptionCommon.h"
>
> +//
> +// Unknown PDB file name
> +//
> +GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 *mUnknownPdbFileName = "";
> +
> /**
>   Return address map of exception handler template so that C code can generate
>   exception tables.
> @@ -243,6 +248,325 @@ DumpCpuContext (
> }
>
> /**
> +  Dump stack contents.
> +
> +  @param[in]  ImageBaseBase address of PE/COFF image.
> +  @param[out] PdbAbsoluteFilePath  Absolute path of PDB file.
> +  @param[out] PdbFileName  File name of PDB file.
> +**/
> +STATIC
> +VOID
> +GetPdbFileName (
> +  IN  UINTNImageBase,
> +  OUT CHAR8**PdbAbsoluteFilePath,
> +  OUT CHAR8**PdbFileName
> +  )
> +{
> +  VOID   *PdbPointer;
> +  CHAR8  *Str;
> +
> +  //
> +  // Get PDB file name from PE/COFF image
> +  //
> +  PdbPointer = PeCoffLoaderGetPdbPointer ((VOID *)ImageBase);
> +  if (PdbPointer == NULL) {
> +//
> +// No PDB file name found. Set it to an unknown file name.
> +//
> +*PdbFileName = (CHAR8 *)mUnknownPdbFileName;
> +if (PdbAbsoluteFilePath != NULL) {
> +  *PdbAbsoluteFilePath = NULL;
> +}
> +  } else {
> +//
> +// Get file name portion out of PDB file in PE/COFF image
> +//
> +Str = (CHAR8 *)((UINTN)PdbPointer +
> +AsciiStrLen ((CHAR8 *)PdbPointer) - sizeof *Str);
> +for (; *Str != '/' && *Str != '\\'; Str--) {
> +  ;
> +}
> +
> +//
> +// Set PDB file name (also skip trailing path separator: '/' or '\\')
> +//
> +*PdbFileName = Str + 1;
> +
> +if (PdbAbsoluteFilePath != NULL) {
> +  //
> +  // Set absolute file path of PDB file
> +  //
> +  *PdbAbsoluteFilePath = PdbPointer;
> +}
> +  }
> +}
> +
> +/**
> +  Dump stack contents.
> +
> +  @param[in]  CurrentRsp Current stack pointer address.
> +  @param[in]  UnwondStacksCount  Count of unwond stack frames.
> +**/
> +STATIC
> +VOID
> +DumpStackContents (
> +  IN UINT64  CurrentRsp,
> +  IN INTNUnwondStacksCount
> +  )
> +{
> +  if (UnwondStacksCount == 0) {
> +return;
> +  }
> +
> +  //
> +  // Dump out stack contents
> +  //
> +  InternalPrintMessage ("\nStack dump:\n");
> +  while (UnwondStacksCount-- > 0) {
> +InternalPrintMessage (
> +  "0x%016lx: %016lx %016lx\n",
> +  CurrentRsp,
> +  *(UINT64 *)CurrentRsp,
> +  *(UINT64 *)((UINTN)CurrentRsp + 8)
> +  );
> +
> +//
> +// As per Microsoft x64 ABI, the stack pointer must be aligned on a 16 
> byte
> +// boundary.
> +//
> +CurrentRsp = CurrentRsp + 16;
> +  }
> +}
> +
> +/**
> +  Dump all image module names from call stack.
> +
> +  @param[in]  SystemContext  Pointer to EFI_SYSTEM_CONTEXT.
> +**/
> +STATIC
> +VOID
> +DumpImageModuleNames (
> +  IN EFI_SYSTEM_CONTEXT   SystemContext
> +  )
> +{
> +  EFI_STATUS  Status;
> +  UINT64  Rip;
> +  UINTN   ImageBase;
> +  VOID*EntryPoint;
> +  CHAR8   *PdbAbsoluteFilePath;
> +  CHAR8   *PdbFileName;
> +  UINT64  Rbp;
> +
> +  //
> +  // Set current RIP address
> +  //
> +  Rip = SystemContext.SystemContextX64->Rip;
> +
> +  //
> +  // Set current frame pointer address
> +  //
> +  Rbp =

[edk2] 答复: 答复: [RFC 0/1] Stack trace support in X64 exception handling

2017-11-14 Thread Fan Jeff 
Paul,



Sorry, correct some import words in my last mail. “I did NOT finalize my 
POC”.:-)



Jeff




From: edk2-devel  on behalf of Fan Jeff 

Sent: Tuesday, November 14, 2017 10:03:20 PM
To: Paulo Alcantara; edk2-devel@lists.01.org
Cc: Rick Bramley; Laszlo Ersek; Andrew Fish; Eric Dong
Subject: [edk2] 答复: [RFC 0/1] Stack trace support in X64 exception handling

Paul,

I like this feature very much. Actually, I did some POC one year ago but I did 
finalize it.

In my POC, I could use EBP to tack the stack frame on IAS32 arch.
But for x64, I tried to use �Ckeepexceptiontable flag to explain stack frame 
from the debug section of image.
I may workson MSFT toolchain, but it did now work well for GCC toolchain.

I think Eric could help to verify MSFT for your patch. If it works well, that’s 
will be great!

Say again, I like this feature!!!:-)

Thanks!
Jeff


发件人: Paulo Alcantara
发送时间: 2017年11月14日 21:23
收件人: edk2-devel@lists.01.org
抄送: Rick Bramley; Laszlo 
Ersek; Andrew Fish; Eric 
Dong
主题: Re: [edk2] [RFC 0/1] Stack trace support in X64 exception handling

Hi,

On 14/11/2017 10:47, Paulo Alcantara wrote:
> Hi,
>
> This series adds stack trace support during a X64 CPU exception.
>
> Informations like back trace, stack contents and image module names
> (that were part of the call stack) will be dumped out.
>
> We already have such support in ARM/AArch64 (IIRC) exception handling
> (thanks to Ard), and then I thought we'd also deserve it in X64 and
> IA-32 platforms.
>
> What do you think guys?
>
> BTW, I've tested this only with OVMF (X64 only), using:
>- gcc-6.3.0, GCC5, NOOPT
>
> Any other tests  would be really appreciable.

I've attached a file to show you how the trace would look like.

Thanks!
Paulo

>
> Thanks!
> Paulo
>
> Repo:   https://github.com/pcacjr/edk2.git
> Branch: stacktrace_x64
>
> Cc: Rick Bramley 
> Cc: Andrew Fish 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Paulo Alcantara 
> ---
>
> Paulo Alcantara (1):
>UefiCpuPkg/CpuExceptionHandlerLib/X64: Add stack trace support
>
>   UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c | 344 
> +++-
>   1 file changed, 342 insertions(+), 2 deletions(-)
>

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

[edk2] 答复: [RFC 0/1] Stack trace support in X64 exception handling

2017-11-14 Thread Fan Jeff 
Paul,

I like this feature very much. Actually, I did some POC one year ago but I did 
finalize it.

In my POC, I could use EBP to tack the stack frame on IAS32 arch.
But for x64, I tried to use �Ckeepexceptiontable flag to explain stack frame 
from the debug section of image.
I may workson MSFT toolchain, but it did now work well for GCC toolchain.

I think Eric could help to verify MSFT for your patch. If it works well, that’s 
will be great!

Say again, I like this feature!!!:-)

Thanks!
Jeff


发件人: Paulo Alcantara
发送时间: 2017年11月14日 21:23
收件人: edk2-devel@lists.01.org
抄送: Rick Bramley; Laszlo 
Ersek; Andrew Fish; Eric 
Dong
主题: Re: [edk2] [RFC 0/1] Stack trace support in X64 exception handling

Hi,

On 14/11/2017 10:47, Paulo Alcantara wrote:
> Hi,
>
> This series adds stack trace support during a X64 CPU exception.
>
> Informations like back trace, stack contents and image module names
> (that were part of the call stack) will be dumped out.
>
> We already have such support in ARM/AArch64 (IIRC) exception handling
> (thanks to Ard), and then I thought we'd also deserve it in X64 and
> IA-32 platforms.
>
> What do you think guys?
>
> BTW, I've tested this only with OVMF (X64 only), using:
>- gcc-6.3.0, GCC5, NOOPT
>
> Any other tests  would be really appreciable.

I've attached a file to show you how the trace would look like.

Thanks!
Paulo

>
> Thanks!
> Paulo
>
> Repo:   https://github.com/pcacjr/edk2.git
> Branch: stacktrace_x64
>
> Cc: Rick Bramley 
> Cc: Andrew Fish 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Paulo Alcantara 
> ---
>
> Paulo Alcantara (1):
>UefiCpuPkg/CpuExceptionHandlerLib/X64: Add stack trace support
>
>   UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c | 344 
> +++-
>   1 file changed, 342 insertions(+), 2 deletions(-)
>

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

Re: [edk2] [RFC 1/1] UefiCpuPkg/CpuExceptionHandlerLib/X64: Add stack trace support

2017-11-14 Thread Andrew Fish 
Paulo,

Cool feature. How does this code deal with VC++ that code does not store the 
frame pointer and requires symbols to unwind. 

Also on the page fault you can print the fault address since it is in CR2. 

It should be possible to post process the text file and make a symbolicated 
backtrace. 

Thanks,

Andrew Fish

> On Nov 14, 2017, at 4:47 AM, Paulo Alcantara  wrote:
> 
> This patch adds stack trace support during a X64 CPU exception.
> 
> It will dump out back trace, stack contents as well as image module
> names that were part of the call stack.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Signed-off-by: Paulo Alcantara 
> ---
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c | 344 
> +++-
> 1 file changed, 342 insertions(+), 2 deletions(-)
> 
> diff --git 
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c 
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
> index 65f0cff680..7048247be3 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
> @@ -14,6 +14,11 @@
> 
> #include "CpuExceptionCommon.h"
> 
> +//
> +// Unknown PDB file name
> +//
> +GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 *mUnknownPdbFileName = "";
> +
> /**
>   Return address map of exception handler template so that C code can generate
>   exception tables.
> @@ -243,6 +248,325 @@ DumpCpuContext (
> }
> 
> /**
> +  Dump stack contents.
> +
> +  @param[in]  ImageBaseBase address of PE/COFF image.
> +  @param[out] PdbAbsoluteFilePath  Absolute path of PDB file.
> +  @param[out] PdbFileName  File name of PDB file.
> +**/
> +STATIC
> +VOID
> +GetPdbFileName (
> +  IN  UINTNImageBase,
> +  OUT CHAR8**PdbAbsoluteFilePath,
> +  OUT CHAR8**PdbFileName
> +  )
> +{
> +  VOID   *PdbPointer;
> +  CHAR8  *Str;
> +
> +  //
> +  // Get PDB file name from PE/COFF image
> +  //
> +  PdbPointer = PeCoffLoaderGetPdbPointer ((VOID *)ImageBase);
> +  if (PdbPointer == NULL) {
> +//
> +// No PDB file name found. Set it to an unknown file name.
> +//
> +*PdbFileName = (CHAR8 *)mUnknownPdbFileName;
> +if (PdbAbsoluteFilePath != NULL) {
> +  *PdbAbsoluteFilePath = NULL;
> +}
> +  } else {
> +//
> +// Get file name portion out of PDB file in PE/COFF image
> +//
> +Str = (CHAR8 *)((UINTN)PdbPointer +
> +AsciiStrLen ((CHAR8 *)PdbPointer) - sizeof *Str);
> +for (; *Str != '/' && *Str != '\\'; Str--) {
> +  ;
> +}
> +
> +//
> +// Set PDB file name (also skip trailing path separator: '/' or '\\')
> +//
> +*PdbFileName = Str + 1;
> +
> +if (PdbAbsoluteFilePath != NULL) {
> +  //
> +  // Set absolute file path of PDB file
> +  //
> +  *PdbAbsoluteFilePath = PdbPointer;
> +}
> +  }
> +}
> +
> +/**
> +  Dump stack contents.
> +
> +  @param[in]  CurrentRsp Current stack pointer address.
> +  @param[in]  UnwondStacksCount  Count of unwond stack frames.
> +**/
> +STATIC
> +VOID
> +DumpStackContents (
> +  IN UINT64  CurrentRsp,
> +  IN INTNUnwondStacksCount
> +  )
> +{
> +  if (UnwondStacksCount == 0) {
> +return;
> +  }
> +
> +  //
> +  // Dump out stack contents
> +  //
> +  InternalPrintMessage ("\nStack dump:\n");
> +  while (UnwondStacksCount-- > 0) {
> +InternalPrintMessage (
> +  "0x%016lx: %016lx %016lx\n",
> +  CurrentRsp,
> +  *(UINT64 *)CurrentRsp,
> +  *(UINT64 *)((UINTN)CurrentRsp + 8)
> +  );
> +
> +//
> +// As per Microsoft x64 ABI, the stack pointer must be aligned on a 16 
> byte
> +// boundary.
> +//
> +CurrentRsp = CurrentRsp + 16;
> +  }
> +}
> +
> +/**
> +  Dump all image module names from call stack.
> +
> +  @param[in]  SystemContext  Pointer to EFI_SYSTEM_CONTEXT.
> +**/
> +STATIC
> +VOID
> +DumpImageModuleNames (
> +  IN EFI_SYSTEM_CONTEXT   SystemContext
> +  )
> +{
> +  EFI_STATUS  Status;
> +  UINT64  Rip;
> +  UINTN   ImageBase;
> +  VOID*EntryPoint;
> +  CHAR8   *PdbAbsoluteFilePath;
> +  CHAR8   *PdbFileName;
> +  UINT64  Rbp;
> +
> +  //
> +  // Set current RIP address
> +  //
> +  Rip = SystemContext.SystemContextX64->Rip;
> +
> +  //
> +  // Set current frame pointer address
> +  //
> +  Rbp = SystemContext.SystemContextX64->Rbp;
> +
> +  //
> +  // Get initial PE/COFF image base address from current RIP
> +  //
> +  ImageBase = PeCoffSearchImageBase (Rip);
> +  if (ImageBase == 0) {
> +InternalPrintMessage (" Could not find image module names. ");
> +return;
> +  }
> +
> +  //
> +  // Get initial PE/COFF image's entry point
> +  //
> +  Status = PeCoffLoaderGetEntryPoint ((VOID *)ImageBase, );
> +  if (EFI_ERROR (Status)) {
> +EntryPoint = NULL;
> +  }
> +
> +  //
>

Re: [edk2] [RFC 0/1] Stack trace support in X64 exception handling

2017-11-14 Thread Paulo Alcantara 

Hi,

On 14/11/2017 10:47, Paulo Alcantara wrote:

Hi,

This series adds stack trace support during a X64 CPU exception.

Informations like back trace, stack contents and image module names
(that were part of the call stack) will be dumped out.

We already have such support in ARM/AArch64 (IIRC) exception handling
(thanks to Ard), and then I thought we'd also deserve it in X64 and
IA-32 platforms.

What do you think guys?

BTW, I've tested this only with OVMF (X64 only), using:
   - gcc-6.3.0, GCC5, NOOPT

Any other tests  would be really appreciable.


I've attached a file to show you how the trace would look like.

Thanks!
Paulo



Thanks!
Paulo

Repo:   https://github.com/pcacjr/edk2.git
Branch: stacktrace_x64

Cc: Rick Bramley 
Cc: Andrew Fish 
Cc: Eric Dong 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara 
---

Paulo Alcantara (1):
   UefiCpuPkg/CpuExceptionHandlerLib/X64: Add stack trace support

  UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c | 344 
+++-
  1 file changed, 342 insertions(+), 2 deletions(-)



 X64 Exception Type - 0E(#PF - Page-Fault)  CPU Apic ID -  
ExceptionData - 0002  I:0 R:0 U:0 W:1 P:0 PK:0 S:0
RIP  - 7E510F7F, CS  - 0038, RFLAGS - 00010202
RAX  - , RCX - 7EA01318, RDX - 7F6EE018
RBX  - 00810248, RSP - 7F762C70, RBP - 7F762CB0
RSI  - 0007, RDI - 7EA01418
R8   - 7E513A88, R9  - 7EA01798, R10 - 0036
R11  - 00D7, R12 - , R13 - 
R14  - , R15 - 
DS   - 0030, ES  - 0030, FS  - 0030
GS   - 0030, SS  - 0030
CR0  - 80010033, CR2 - , CR3 - 7F701000
CR4  - 0668, CR8 - 
DR0  - , DR1 - , DR2 - 
DR3  - , DR6 - 0FF0, DR7 - 0400
GDTR - 7F6EEA98 0047, LDTR - 
IDTR - 7EEF2018 0FFF,   TR - 
FXSAVE_STATE - 7F7628D0

Back trace:
0 0x7E510F7F @ 0x7E509000+0x7F7E (0x7F762CB0) in 
PartitionDxe.dll
1 0x7E51135D @ 0x7E509000+0x835C (0x7F762CE0) in 
PartitionDxe.dll
2 0x7E50C116 @ 0x7E509000+0x3115 (0x7F762D20) in 
PartitionDxe.dll
3 0x7F776972 @ 0x7E509000+0x126D971 (0x7F762DB0) in 
PartitionDxe.dll
4 0x7F78EE08 @ 0x7E509000+0x1285E07 (0x7F762E30) in 
PartitionDxe.dll
5 0x7F791343 @ 0x7E509000+0x1288342 (0x7F762F60) in 
PartitionDxe.dll
6 0x7F791AC7 @ 0x7E509000+0x1288AC6 (0x7F762F90) in 
PartitionDxe.dll
7 0x7F767DDB @ 0x7E509000+0x125EDDA (0x7F762FC0) in 
PartitionDxe.dll
8 0x7F7DF75F @ 0x7E509000+0x12D675E (0x7B7DC840) in 
PartitionDxe.dll
9 0x7F7E5546 @ 0x7E509000+0x12DC545 (0x7B7DC8C0) in 
PartitionDxe.dll
10 0x7F7E4312 @ 0x7E509000+0x12DB311 (0x7B7DCA30) in 
PartitionDxe.dll
11 0x7F7F0DB9 @ 0x7E509000+0x12E7DB8 (0x7B7DCF80) in 
PartitionDxe.dll
12 0x008286E9 @ 0x00820140+0x85A8 (0x7B7DD4D0) in 
PeiCore.dll
13 0x0083092F @ 0x00820140+0x107EE (0x00817600) in 
PeiCore.dll
14 0x00831574 @ 0x00820140+0x11433 (0x008176D0) in 
PeiCore.dll
15 0x00828D9B @ 0x00820140+0x8C5A (0x00817C20) in 
PeiCore.dll
16 0x0083238A @ 0x00820140+0x12249 (0x00817C50) in 
PeiCore.dll
17 0x00824312 @ 0x00820140+0x41D1 (0x00817C80) in 
PeiCore.dll
18 0xFFFD4291 @ 0x00820140+0xFF7B4150 (0x00817CE0) in 
PeiCore.dll
19 0xFFFCF578 @ 0x00820140+0xFF7AF437 (0x00817D10) in 
PeiCore.dll
20 0xFFFD422C @ 0x00820140+0xFF7B40EB (0x00817FD0) in 
PeiCore.dll
21 0xFFFD4489 @ 0x00820140+0xFF7B4348 (0xFFFCC000) in 
PeiCore.dll

PartitionDxe.dll (ImageBase=0x7E509000, EntryPoint=0x7E50C01F):
/home/pcacjr/src/edk2.git/Build/OvmfX64/NOOPT_GCC5/X64/MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe/DEBUG/PartitionDxe.dll
PeiCore.dll (ImageBase=0x00820140, EntryPoint=0x008242EC):
/home/pcacjr/src/edk2.git/Build/OvmfX64/NOOPT_GCC5/X64/MdeModulePkg/Core/Pei/PeiMain/DEBUG/PeiCore.dll

Stack dump:
0x7F762C70: 7E5137E0 
0x7F762C80: 7E513A88 0100
0x7F762C90: 7F762CB0 
0x7F762CA0: 7F762CE0

[edk2] [RFC 0/1] Stack trace support in X64 exception handling

2017-11-14 Thread Paulo Alcantara 
Hi,

This series adds stack trace support during a X64 CPU exception.

Informations like back trace, stack contents and image module names
(that were part of the call stack) will be dumped out.

We already have such support in ARM/AArch64 (IIRC) exception handling
(thanks to Ard), and then I thought we'd also deserve it in X64 and
IA-32 platforms.

What do you think guys?

BTW, I've tested this only with OVMF (X64 only), using:
  - gcc-6.3.0, GCC5, NOOPT

Any other tests  would be really appreciable.

Thanks!
Paulo

Repo:   https://github.com/pcacjr/edk2.git
Branch: stacktrace_x64

Cc: Rick Bramley 
Cc: Andrew Fish 
Cc: Eric Dong 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara 
---

Paulo Alcantara (1):
  UefiCpuPkg/CpuExceptionHandlerLib/X64: Add stack trace support

 UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c | 344 
+++-
 1 file changed, 342 insertions(+), 2 deletions(-)

-- 
2.11.0

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

[edk2] [RFC 1/1] UefiCpuPkg/CpuExceptionHandlerLib/X64: Add stack trace support

2017-11-14 Thread Paulo Alcantara 
This patch adds stack trace support during a X64 CPU exception.

It will dump out back trace, stack contents as well as image module
names that were part of the call stack.

Contributed-under: TianoCore Contribution Agreement 1.1
Cc: Eric Dong 
Cc: Laszlo Ersek 
Signed-off-by: Paulo Alcantara 
---
 UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c | 344 
+++-
 1 file changed, 342 insertions(+), 2 deletions(-)

diff --git 
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c 
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
index 65f0cff680..7048247be3 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
@@ -14,6 +14,11 @@
 
 #include "CpuExceptionCommon.h"
 
+//
+// Unknown PDB file name
+//
+GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 *mUnknownPdbFileName = "";
+
 /**
   Return address map of exception handler template so that C code can generate
   exception tables.
@@ -243,6 +248,325 @@ DumpCpuContext (
 }
 
 /**
+  Dump stack contents.
+
+  @param[in]  ImageBaseBase address of PE/COFF image.
+  @param[out] PdbAbsoluteFilePath  Absolute path of PDB file.
+  @param[out] PdbFileName  File name of PDB file.
+**/
+STATIC
+VOID
+GetPdbFileName (
+  IN  UINTNImageBase,
+  OUT CHAR8**PdbAbsoluteFilePath,
+  OUT CHAR8**PdbFileName
+  )
+{
+  VOID   *PdbPointer;
+  CHAR8  *Str;
+
+  //
+  // Get PDB file name from PE/COFF image
+  //
+  PdbPointer = PeCoffLoaderGetPdbPointer ((VOID *)ImageBase);
+  if (PdbPointer == NULL) {
+//
+// No PDB file name found. Set it to an unknown file name.
+//
+*PdbFileName = (CHAR8 *)mUnknownPdbFileName;
+if (PdbAbsoluteFilePath != NULL) {
+  *PdbAbsoluteFilePath = NULL;
+}
+  } else {
+//
+// Get file name portion out of PDB file in PE/COFF image
+//
+Str = (CHAR8 *)((UINTN)PdbPointer +
+AsciiStrLen ((CHAR8 *)PdbPointer) - sizeof *Str);
+for (; *Str != '/' && *Str != '\\'; Str--) {
+  ;
+}
+
+//
+// Set PDB file name (also skip trailing path separator: '/' or '\\')
+//
+*PdbFileName = Str + 1;
+
+if (PdbAbsoluteFilePath != NULL) {
+  //
+  // Set absolute file path of PDB file
+  //
+  *PdbAbsoluteFilePath = PdbPointer;
+}
+  }
+}
+
+/**
+  Dump stack contents.
+
+  @param[in]  CurrentRsp Current stack pointer address.
+  @param[in]  UnwondStacksCount  Count of unwond stack frames.
+**/
+STATIC
+VOID
+DumpStackContents (
+  IN UINT64  CurrentRsp,
+  IN INTNUnwondStacksCount
+  )
+{
+  if (UnwondStacksCount == 0) {
+return;
+  }
+
+  //
+  // Dump out stack contents
+  //
+  InternalPrintMessage ("\nStack dump:\n");
+  while (UnwondStacksCount-- > 0) {
+InternalPrintMessage (
+  "0x%016lx: %016lx %016lx\n",
+  CurrentRsp,
+  *(UINT64 *)CurrentRsp,
+  *(UINT64 *)((UINTN)CurrentRsp + 8)
+  );
+
+//
+// As per Microsoft x64 ABI, the stack pointer must be aligned on a 16 byte
+// boundary.
+//
+CurrentRsp = CurrentRsp + 16;
+  }
+}
+
+/**
+  Dump all image module names from call stack.
+
+  @param[in]  SystemContext  Pointer to EFI_SYSTEM_CONTEXT.
+**/
+STATIC
+VOID
+DumpImageModuleNames (
+  IN EFI_SYSTEM_CONTEXT   SystemContext
+  )
+{
+  EFI_STATUS  Status;
+  UINT64  Rip;
+  UINTN   ImageBase;
+  VOID*EntryPoint;
+  CHAR8   *PdbAbsoluteFilePath;
+  CHAR8   *PdbFileName;
+  UINT64  Rbp;
+
+  //
+  // Set current RIP address
+  //
+  Rip = SystemContext.SystemContextX64->Rip;
+
+  //
+  // Set current frame pointer address
+  //
+  Rbp = SystemContext.SystemContextX64->Rbp;
+
+  //
+  // Get initial PE/COFF image base address from current RIP
+  //
+  ImageBase = PeCoffSearchImageBase (Rip);
+  if (ImageBase == 0) {
+InternalPrintMessage (" Could not find image module names. ");
+return;
+  }
+
+  //
+  // Get initial PE/COFF image's entry point
+  //
+  Status = PeCoffLoaderGetEntryPoint ((VOID *)ImageBase, );
+  if (EFI_ERROR (Status)) {
+EntryPoint = NULL;
+  }
+
+  //
+  // Get file name and absolute path of initial PDB file
+  //
+  GetPdbFileName (ImageBase, , );
+
+  //
+  // Print out initial image module name (if any)
+  //
+  if (PdbAbsoluteFilePath != NULL) {
+InternalPrintMessage (
+  "\n%a (ImageBase=0x%016lx, EntryPoint=0x%016lx):\n",
+  PdbFileName,
+  ImageBase,
+  (UINTN)EntryPoint
+  );
+InternalPrintMessage ("%a\n", PdbAbsoluteFilePath);
+  }
+
+  //
+  // Walk through call stack and find next module names
+  //
+  for (;;) {
+//
+// Set RIP with return address from current stack frame
+//
+Rip = *(UINT64 *)((UINTN)Rbp + 8);
+
+//
+// Check if RIP is within another PE/COFF image base address
+//
+if (Rip < ImageBase) {

[edk2] [PATCH 0/4] BaseTools: Add VS2017 support, including ARM and AARCH64

2017-11-14 Thread Pete Batard 
Seeing that Visual Studio Update 4 finally added native ARM64 compilation
support and this is an open task:
https://github.com/tianocore/tianocore.github.io/wiki/Tasks

This series is broken down into:
- A preliminary patch, to eliminate new warnings that would be produced for
  VS2017/IA32
- IA32 + X64 support
- ARM support
- AARCH64 support

Notes:
- Considering that the multiplication of toolchain flavours is probably not in
  our best interest when it comes to maintenance, I deliberately chose not to
  create extra VS2017 variants (for x64, ASL and EBC). For one thing, I had
  some issues with the x64 tools for ARM compilation (which may very well have
  been my own), and I'd rather see the variants being added after their need
  has been justified by their potential users, rather than preemptively.
- ARM and ARM64 should be considered EXPERIMENTAL at this stage.
  I've had some good success compiling and running moderately complex drivers
  (e.g. ZFS file system driver) using the proposed patches on ARM/ARM64, and I
  am also confident that simple applications should be fine, but more work is
  required to produce QEMU ARM/AARCH64 firmware images, mostly due to missing
  .asm files. Since most of the RVCT .asm files can be reused mostly unchanged
  for MSFT, I was able to cajole the ARM process to go up to the QEMU image
  generation (which failed due to some alignment issue), which gives me some
  confidence that we should eventually be able to use the MSFT toolchain to
  produce working images for ARM and ARM64, but this will require some work.
- While the VS2017 version of IA32 and X64 do not silence any level 4 warnings
  by default, to bring them to par with VS2015, the ARM and ARM64 versions
  have to silence 3 of them, as VS is a bit less leniant than gcc/Clang.
  Again, it should be possible to remove the silencing of these warnings
  eventually, but this will require some work.
- Finally, you'll see that a whole section of build_rule.template had to be
  duplicated just so that we could remove the --convert-hex option for the
  MSFT ARM/ARM64 assemblers. Maybe there is a better way to achieve the
  removal of that option, but I haven't found it.

Regards,

/Pete

Pete Batard (4):
  MdeModulePkg, NetworkPkg: Fix VS2017 IA32 warnings
  BaseTools: Add VS2017 IA32 and X64 support
  BaseTools: Add VS2017 ARM support
  BaseTools: Add VS2017 AARCH64 support

 ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm 
  | 255 
 ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtsrsh.asm
  |  45 
 ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf 
  |  13 +-
 ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c   
  |  30 +++
 ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c   
  |  29 +++
 BaseTools/Conf/build_rule.template 
  |  30 +++
 BaseTools/Conf/tools_def.template  
  | 168 +
 BaseTools/set_vsprefix_envs.bat
  |   9 +
 MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c  
  |   2 +-
 
MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerCustomizedUiSupport.c
 |   2 +-
 MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c  
  |   2 +-
 MdeModulePkg/Universal/Network/MnpDxe/MnpMain.c
  |   2 +-
 MdePkg/Include/Base.h  
  |  15 ++
 MdePkg/Library/BaseLib/AArch64/CpuBreakpoint.asm   
  |  39 +++
 MdePkg/Library/BaseLib/AArch64/DisableInterrupts.asm   
  |  37 +++
 MdePkg/Library/BaseLib/AArch64/EnableInterrupts.asm
  |  37 +++
 MdePkg/Library/BaseLib/AArch64/GetInterruptsState.asm  
  |  49 
 MdePkg/Library/BaseLib/AArch64/MemoryFence.asm 
  |  38 +++
 MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.asm 
  | 101 
 MdePkg/Library/BaseLib/AArch64/SwitchStack.asm 
  |  69 ++
 MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm   
  |   5 +-
 MdePkg/Library/BaseLib/BaseLib.inf 
  |  27 ++-
 MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf 
  |   5 +-
 MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c  
  |  18 ++
 NetworkPkg/HttpBootDxe/HttpBootImpl.c

[edk2] [PATCH 2/4] BaseTools: Add VS2017 IA32 and X64 support

2017-11-14 Thread Pete Batard 
For the sake of reducing toolchain combinations, only one variation
is supported: x86 tools (MS default) with External ASL and no EBC.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Pete Batard 
---
 BaseTools/Conf/tools_def.template | 108 
 BaseTools/set_vsprefix_envs.bat   |   9 ++
 Nt32Pkg/Sec/SecMain.inf   |   2 +
 3 files changed, 119 insertions(+)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index aebd7d558633..0346750886a2 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -74,6 +74,10 @@ DEFINE VS2015x86_BIN= ENV(VS2015_PREFIX)Vc\bin
 DEFINE VS2015x86_DLL= ENV(VS2015_PREFIX)Common7\IDE;DEF(VS2015x86_BIN)
 DEFINE VS2015x86_BINX64 = DEF(VS2015x86_BIN)\x86_amd64
 
+DEFINE VS2017_BIN = ENV(VS2017_PREFIX)bin\Hostx86
+DEFINE VS2017_BIN_IA32= DEF(VS2017_BIN)\x86
+DEFINE VS2017_BIN_X64 = DEF(VS2017_BIN)\x64
+
 DEFINE WINSDK_BIN   = ENV(WINSDK_PREFIX)
 DEFINE WINSDKx86_BIN= ENV(WINSDKx86_PREFIX)
 
@@ -93,6 +97,9 @@ DEFINE WINSDK8x86_BIN= ENV(WINSDK8x86_PREFIX)x64
 DEFINE WINSDK81_BIN   = ENV(WINSDK81_PREFIX)x86\
 DEFINE WINSDK81x86_BIN= ENV(WINSDK81x86_PREFIX)x64
 
+# Microsoft Visual Studio 2017 Community Edition
+DEFINE WINSDK10_BIN   = ENV(WINSDK10_PREFIX)x86
+
 # These defines are needed for certain Microsoft Visual Studio tools that
 # are used by other toolchains.  An example is that ICC on Windows normally
 # uses Microsoft's nmake.exe.
@@ -318,6 +325,11 @@ DEFINE DTC_BIN = ENV(DTC_PREFIX)dtc
 # Required to build platforms or ACPI tables:
 #   Intel(r) ACPI Compiler (iasl.exe) from
 #   https://acpica.org/downloads
+#   VS2017  -win32-  Requires:
+# Microsoft Visual Studio 2017 Community Edition
+# Intel(r) ACPI Compiler (iasl.exe) from 
https://acpica.org/downloads
+#Unsupported:
+# Building of EBC drivers
 #   DDK3790 -win32-  Requires:
 # Microsoft Windows Server 2003 Driver Development 
Kit (Microsoft WINDDK) version 3790.1830
 #Optional:
@@ -4060,6 +4072,102 @@ NOOPT_VS2015x86xASL_X64_DLINK_FLAGS= /NOLOGO 
/NODEFAULTLIB /IGNORE:4001 /OPT
 *_VS2015x86xASL_EBC_SLINK_FLAGS = /lib /NOLOGO /MACHINE:EBC
 *_VS2015x86xASL_EBC_DLINK_FLAGS = "C:\Program Files 
(x86)\Intel\EBC\Lib\EbcLib.lib" /NOLOGO /NODEFAULTLIB /MACHINE:EBC /OPT:REF 
/ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /MAP /ALIGN:32 
/DRIVER
 
+
+#
+# Microsoft Visual Studio 2017
+#
+#   VS2017  - Microsoft Visual Studio 2017 Professional Edition with Intel ASL
+#   ASL - Intel ACPI Source Language Compiler
+
+#   VS2017   - Microsoft Visual Studio 2017 Professional Edition
+*_VS2017_*_*_FAMILY   = MSFT
+
+*_VS2017_*_MAKE_PATH  = DEF(VS2017_BIN_IA32)\nmake.exe
+*_VS2017_*_MAKE_FLAGS = /nologo
+*_VS2017_*_RC_PATH= DEF(WINSDK10_BIN)\rc.exe
+
+*_VS2017_*_SLINK_FLAGS= /NOLOGO /LTCG
+*_VS2017_*_APP_FLAGS  = /nologo /E /TC
+*_VS2017_*_PP_FLAGS   = /nologo /E /TC /FIAutoGen.h
+*_VS2017_*_VFRPP_FLAGS= /nologo /E /TC /DVFRCOMPILE 
/FI$(MODULE_NAME)StrDefs.h
+*_VS2017_*_DLINK2_FLAGS   =
+
+*_VS2017_*_ASM16_PATH = EF(VS2017_BIN_IA32)\ml.exe
+
+##
+# ASL definitions
+##
+*_VS2017_*_ASL_PATH   = DEF(DEFAULT_WIN_ASL_BIN)
+*_VS2017_*_ASL_FLAGS  = DEF(DEFAULT_WIN_ASL_FLAGS)
+*_VS2017_*_ASL_OUTFLAGS   = DEF(DEFAULT_WIN_ASL_OUTFLAGS)
+*_VS2017_*_ASLCC_FLAGS= DEF(MSFT_ASLCC_FLAGS)
+*_VS2017_*_ASLPP_FLAGS= DEF(MSFT_ASLPP_FLAGS)
+*_VS2017_*_ASLDLINK_FLAGS = DEF(MSFT_ASLDLINK_FLAGS)
+
+##
+# IA32 definitions
+##
+*_VS2017_IA32_*_DLL   = DEF(VS2017_BIN_IA32)
+
+*_VS2017_IA32_CC_PATH = DEF(VS2017_BIN_IA32)\cl.exe
+*_VS2017_IA32_VFRPP_PATH  = DEF(VS2017_BIN_IA32)\cl.exe
+*_VS2017_IA32_SLINK_PATH  = DEF(VS2017_BIN_IA32)\lib.exe
+*_VS2017_IA32_DLINK_PATH  = DEF(VS2017_BIN_IA32)\link.exe
+*_VS2017_IA32_APP_PATH= DEF(VS2017_BIN_IA32)\cl.exe
+*_VS2017_IA32_PP_PATH = DEF(VS2017_BIN_IA32)\cl.exe
+*_VS2017_IA32_ASM_PATH= DEF(VS2017_BIN_IA32)\ml.exe
+*_VS2017_IA32_ASLCC_PATH  = DEF(VS2017_BIN_IA32)\cl.exe
+*_VS2017_IA32_ASLPP_PATH  = DEF(VS2017_BIN_IA32)\cl.exe
+*_VS2017_IA32_ASLDLINK_PATH   = DEF(VS2017_BIN_IA32)\link.exe
+
+  *_VS2017_IA32_MAKE_FLAGS=

[edk2] [PATCH 1/4] MdeModulePkg, NetworkPkg: Fix VS2017 IA32 warnings

2017-11-14 Thread Pete Batard 
This is a preliminary patch before the introduction of VS2017 support.
Without this, IA32 compilation will produce the following warnings:
* warning C4701: potentially uninitialized local variable used
* warning C4703: potentially uninitialized local pointer variable used

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Pete Batard 
---
 MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c  
  | 2 +-
 
MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerCustomizedUiSupport.c
 | 2 +-
 MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c  
  | 2 +-
 MdeModulePkg/Universal/Network/MnpDxe/MnpMain.c
  | 2 +-
 NetworkPkg/HttpBootDxe/HttpBootImpl.c  
  | 2 +-
 NetworkPkg/HttpBootDxe/HttpBootSupport.c   
  | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c 
b/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
index 17fc3db507d0..fdab07f9738c 100644
--- a/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
+++ b/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
@@ -512,7 +512,7 @@ RequiredDriver (
   EFI_STATUS  Status;
   UINT8   ClassGuidNum;
   EFI_GUID*ClassGuid;
-  EFI_IFR_FORM_SET*Buffer;
+  EFI_IFR_FORM_SET*Buffer = NULL;
   UINTN   BufferSize;
   UINT8   *Ptr;
   UINTN   TempSize;
diff --git 
a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerCustomizedUiSupport.c
 
b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerCustomizedUiSupport.c
index 6dd4fce13938..4ddfb0926dc2 100644
--- 
a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerCustomizedUiSupport.c
+++ 
b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerCustomizedUiSupport.c
@@ -308,7 +308,7 @@ IsRequiredDriver (
   EFI_STATUS  Status;
   UINT8   ClassGuidNum;
   EFI_GUID*ClassGuid;
-  EFI_IFR_FORM_SET*Buffer;
+  EFI_IFR_FORM_SET*Buffer = NULL;
   UINTN   BufferSize;
   UINT8   *Ptr;
   UINTN   TempSize;
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c 
b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
index 646864f4dfc1..9e92ff5874b7 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
@@ -5307,7 +5307,7 @@ HiiConfigRoutingRouteConfig (
   EFI_STRING  ConfigResp;
   UINTN   Length;
   EFI_STATUS  Status;
-  EFI_DEVICE_PATH_PROTOCOL*DevicePath;
+  EFI_DEVICE_PATH_PROTOCOL*DevicePath = NULL;
   EFI_DEVICE_PATH_PROTOCOL*TempDevicePath;
   LIST_ENTRY  *Link;
   HII_DATABASE_RECORD *Database;
diff --git a/MdeModulePkg/Universal/Network/MnpDxe/MnpMain.c 
b/MdeModulePkg/Universal/Network/MnpDxe/MnpMain.c
index 31c2e3e5b849..04adae62572b 100644
--- a/MdeModulePkg/Universal/Network/MnpDxe/MnpMain.c
+++ b/MdeModulePkg/Universal/Network/MnpDxe/MnpMain.c
@@ -520,7 +520,7 @@ MnpTransmit (
   EFI_STATUSStatus;
   MNP_INSTANCE_DATA *Instance;
   MNP_SERVICE_DATA  *MnpServiceData;
-  UINT8 *PktBuf;
+  UINT8 *PktBuf = NULL;
   UINT32PktLen;
   EFI_TPL   OldTpl;
 
diff --git a/NetworkPkg/HttpBootDxe/HttpBootImpl.c 
b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
index 06a8a6a38615..2687696b0de1 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootImpl.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
@@ -121,7 +121,7 @@ HttpBootStart (
 {
   UINTNIndex;
   EFI_STATUS   Status;
-  CHAR8*Uri;
+  CHAR8*Uri = NULL;
   
 
   if (Private == NULL || FilePath == NULL) {
diff --git a/NetworkPkg/HttpBootDxe/HttpBootSupport.c 
b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
index d508e2c1a979..787dd24e3c71 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootSupport.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
@@ -1199,7 +1199,7 @@ HttpBootCheckImageType (
 {
   EFI_STATUSStatus;
   EFI_HTTP_HEADER   *Header;
-  CHAR8 *FilePath;
+  CHAR8 *FilePath = NULL;
   CHAR8 *FilePost;
 
   if (Uri == NULL || UriParser == NULL || ImageType == NULL) {
-- 
2.14.2

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

[edk2] [PATCH 3/4] BaseTools: Add VS2017 ARM support

2017-11-14 Thread Pete Batard 
Requires the silencing of the following warnings:
* warning C4100: unreferenced formal parameter
* warning C4127: conditional expression is constant
* warning C4214: nonstandard extension used: bit field types other than int
Also requires _ARM_WINAPI_PARTITION_DESKTOP_SDK_AVAILABLE to be defined for
the Windows SDK to allow ARM compilation.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Pete Batard 
---
 ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm | 255 

 ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtsrsh.asm|  45 
 ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf |  13 +-
 ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c   |  30 +++
 ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c   |  29 +++
 BaseTools/Conf/build_rule.template |  30 +++
 BaseTools/Conf/tools_def.template  |  30 +++
 MdePkg/Include/Base.h  |  15 ++
 MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm   |   5 +-
 MdePkg/Library/BaseLib/BaseLib.inf |  19 +-
 MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf |   5 +-
 MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c  |  18 ++
 12 files changed, 486 insertions(+), 8 deletions(-)

diff --git a/ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm 
b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm
new file mode 100644
index ..096dc6317318
--- /dev/null
+++ b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm
@@ -0,0 +1,255 @@
+///** @file
+//
+//  This code provides replacement for MSVC CRT division functions
+//
+//  Copyright (c) 2017, Pete Batard. All rights reserved.
+//  Based on generated assembly of ReactOS' sdk/lib/crt/math/arm/__rt_###div.c,
+//  Copyright (c) Timo Kreuzer. 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.
+//
+//**/
+
+  EXPORT _fltused
+  EXPORT __brkdiv0
+
+  EXPORT __rt_sdiv
+  EXPORT __rt_udiv
+  EXPORT __rt_udiv64
+  EXPORT __rt_sdiv64
+
+  AREA  Math, CODE, READONLY
+
+_fltused
+dcd 0x9875
+
+__brkdiv0
+udf #249
+
+//
+// uint64_t __rt_udiv(uint32_t divisor, uint32_t dividend)
+//
+
+__rt_udiv
+  cmp r0, #0
+  beq __brkdiv0
+  push{r3-r5,lr}
+  mov r5,r0
+  mov r4,r1
+  cmp r5,r4
+  it  hi
+  movhi   r0,#0
+  bhi __rt_udiv_label3
+  clz r2,r5
+  clz r3,r4
+  subsr3,r2,r3
+  movsr1,#1
+  lsl r2,r5,r3
+  lsl r3,r1,r3
+  movsr0,#0
+__rt_udiv_label1
+  cmp r4,r2
+  bcc __rt_udiv_label2
+  orrsr0,r0,r3
+  subsr4,r4,r2
+__rt_udiv_label2
+  lsrsr2,r2,#1
+  lsrsr3,r3,#1
+  bne __rt_udiv_label1
+__rt_udiv_label3
+  mov r1,r4
+  pop {r3-r5,pc}
+
+//
+// uint64_t __rt_sdiv(int32_t divisor, int32_t dividend)
+//
+
+__rt_sdiv
+  cmp r0, #0
+  beq __brkdiv0
+  push{r4-r6,lr}
+  mov r4,r1
+  andsr6,r0,#0x8000
+  it  ne
+  rsbne   r4,r4,#0
+  mov r5,r0
+  rsbsr5,r5,#0
+  cmp r5,r4
+  it  hi
+  movhi   r0,#0
+  bhi __rt_sdiv_label3
+  clz r2,r5
+  clz r3,r4
+  subsr3,r2,r3
+  movsr1,#1
+  lsl r2,r5,r3
+  lsl r3,r1,r3
+  movsr0,#0
+__rt_sdiv_label1
+  cmp r4,r2
+  bcc __rt_sdiv_label2
+  orrsr0,r0,r3
+  subsr4,r4,r2
+__rt_sdiv_label2
+  lsrsr2,r2,#1
+  lsrsr3,r3,#1
+  bne __rt_sdiv_label1
+__rt_sdiv_label3
+  cbz r6,__rt_sdiv_label4
+  rsbsr4,r4,#0
+__rt_sdiv_label4
+  mov r1,r4
+  pop {r4-r6,pc}
+
+//
+// typedef struct {
+//   uint64_t quotient;
+//   uint64_t modulus;
+// } udiv64_result_t;
+//
+// void __rt_udiv64_internal(udiv64_result_t *result, uint64_t divisor, 
uint64_t dividend)
+//
+
+__rt_udiv64_internal
+  orrsr1,r2,r3
+  beq __brkdiv0
+  push{r4-r8,lr}
+  mov r7,r3
+  mov r6,r2
+  mov r4,r0
+  ldrdr0,r5,[sp,#0x18]
+  cmp r7,r5
+  bcc __rt_udiv64_internal_label2
+  bhi __rt_udiv64_internal_label1
+  cmp r6,r0
+  bls __rt_udiv64_internal_label2
+__rt_udiv64_internal_label1
+  movsr3,#0
+  strdr3,r3,[r4]
+  b   __rt_udiv64_internal_label8
+__rt_udiv64_internal_label2
+  clz r2,r7
+  cmp

[edk2] [PATCH 4/4] BaseTools: Add VS2017 AARCH64 support

2017-11-14 Thread Pete Batard 
Requires the silencing of the same warnings as ARM.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Pete Batard 
---
 BaseTools/Conf/build_rule.template|   4 +-
 BaseTools/Conf/tools_def.template |  32 ++-
 MdePkg/Include/Base.h |   2 +-
 MdePkg/Library/BaseLib/AArch64/CpuBreakpoint.asm  |  39 
 MdePkg/Library/BaseLib/AArch64/DisableInterrupts.asm  |  37 +++
 MdePkg/Library/BaseLib/AArch64/EnableInterrupts.asm   |  37 +++
 MdePkg/Library/BaseLib/AArch64/GetInterruptsState.asm |  49 ++
 MdePkg/Library/BaseLib/AArch64/MemoryFence.asm|  38 
 MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.asm| 101 

 MdePkg/Library/BaseLib/AArch64/SwitchStack.asm|  69 +
 MdePkg/Library/BaseLib/BaseLib.inf|   8 ++
 11 files changed, 412 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Conf/build_rule.template 
b/BaseTools/Conf/build_rule.template
index 08c1df14af90..0cf9985cb333 100755
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -207,8 +207,8 @@
 # For RVCTCYGWIN ASM_FLAGS must be first to work around pathing issues
 "$(ASM)" $(ASM_FLAGS) -o ${dst} $(INC) ${d_path}(+)${s_base}.iii
 
-[Assembly-Code-File.COMMON.ARM]
-# Remove --convert-hex for ARM as it breaks MSFT assemblers
+[Assembly-Code-File.COMMON.ARM,Assembly-Code-File.COMMON.AARCH64]
+# Remove --convert-hex for ARM/AARCH64 as it breaks MSFT assemblers
 
 ?.asm, ?.Asm, ?.ASM
 
diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 4bc057f1a568..9c81a06822e9 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -78,6 +78,7 @@ DEFINE VS2017_BIN = ENV(VS2017_PREFIX)bin\Hostx86
 DEFINE VS2017_BIN_IA32= DEF(VS2017_BIN)\x86
 DEFINE VS2017_BIN_X64 = DEF(VS2017_BIN)\x64
 DEFINE VS2017_BIN_ARM = DEF(VS2017_BIN)\arm
+DEFINE VS2017_BIN_AARCH64 = DEF(VS2017_BIN)\arm64
 
 DEFINE WINSDK_BIN   = ENV(WINSDK_PREFIX)
 DEFINE WINSDKx86_BIN= ENV(WINSDKx86_PREFIX)
@@ -327,7 +328,7 @@ DEFINE DTC_BIN = ENV(DTC_PREFIX)dtc
 #   Intel(r) ACPI Compiler (iasl.exe) from
 #   https://acpica.org/downloads
 #   VS2017  -win32-  Requires:
-# Microsoft Visual Studio 2017 Community Edition
+# Microsoft Visual Studio 2017 Community Edition 
(with Update 4 for ARM64 support)
 # Intel(r) ACPI Compiler (iasl.exe) from 
https://acpica.org/downloads
 #Unsupported:
 # Building of EBC drivers
@@ -4199,6 +4200,35 @@ NOOPT_VS2017_ARM_ASM_FLAGS= /nologo
 RELEASE_VS2017_ARM_DLINK_FLAGS= /NOLOGO /NODEFAULTLIB /IGNORE:4001 
/IGNORE:4254 /OPT:REF /OPT:ICF=10 /MAP /SECTION:.xdata,D /SECTION:.pdata,D 
/MACHINE:ARM /LTCG /DLL /ENTRY:$(IMAGE_ENTRY_POINT) 
/SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DRIVER 
/MERGE:.rdata=.data
 NOOPT_VS2017_ARM_DLINK_FLAGS  = /NOLOGO /NODEFAULTLIB /IGNORE:4001 
/OPT:REF /OPT:ICF=10 /MAP /SECTION:.xdata,D /SECTION:.pdata,D /MACHINE:ARM 
/LTCG /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER 
/SAFESEH:NO /BASE:0 /DRIVER /DEBUG
 
+#
+# AARCH64 definitions
+#
+*_VS2017_AARCH64_*_DLL= DEF(VS2017_BIN_AARCH64)
+
+*_VS2017_AARCH64_CC_PATH  = DEF(VS2017_BIN_AARCH64)\cl.exe
+*_VS2017_AARCH64_VFRPP_PATH   = DEF(VS2017_BIN_AARCH64)\cl.exe
+*_VS2017_AARCH64_SLINK_PATH   = DEF(VS2017_BIN_AARCH64)\lib.exe
+*_VS2017_AARCH64_DLINK_PATH   = DEF(VS2017_BIN_AARCH64)\link.exe
+*_VS2017_AARCH64_APP_PATH = DEF(VS2017_BIN_AARCH64)\cl.exe
+*_VS2017_AARCH64_PP_PATH  = DEF(VS2017_BIN_AARCH64)\cl.exe
+*_VS2017_AARCH64_ASM_PATH = DEF(VS2017_BIN_AARCH64)\armasm64.exe
+*_VS2017_AARCH64_ASLCC_PATH   = DEF(VS2017_BIN_AARCH64)\cl.exe
+*_VS2017_AARCH64_ASLPP_PATH   = DEF(VS2017_BIN_AARCH64)\cl.exe
+*_VS2017_AARCH64_ASLDLINK_PATH= DEF(VS2017_BIN_AARCH64)\link.exe
+
+  *_VS2017_AARCH64_MAKE_FLAGS = /nologo
+  DEBUG_VS2017_AARCH64_CC_FLAGS   = /nologo /c /WX /wd4100 /wd4127 /wd4214 
/GS- /W4 /Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi 
/Gm /Gw /Oi-
+RELEASE_VS2017_AARCH64_CC_FLAGS   = /nologo /c /WX /wd4100 /wd4127 /wd4214 
/GS- /W4 /Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gw /Oi-
+NOOPT_VS2017_AARCH64_CC_FLAGS = /nologo /c /WX /wd4100 /wd4127 /wd4214 
/GS- /W4 /Gs32768 /D UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Od /Oi-
+
+  DEBUG_VS2017_AARCH64_ASM_FLAGS  = /nologo /g
+RELEASE_VS2017_AARCH64_ASM_FLAGS  = /nologo
+NOOPT_VS2017_AARCH64_ASM_FLAGS= /nologo
+
+

Re: [edk2] [PATCH edk2-platforms]: resolving Hikey platform build error

2017-11-14 Thread Ard Biesheuvel 
On 14 November 2017 at 11:16, Kalyan Nagabhirava
 wrote:
>
> "Instance of library class [CapsuleLib] is not found" build error is coming
> for Hikey platform, to resolve this issueadded CapsuleLib to
> "LibraryClasses.common" section
>
> diff --git a/Platform/Hisilicon/HiKey/HiKey.dsc
> b/Platform/Hisilicon/HiKey/HiKey.dsc
> index 968e8ac..2e3b1c8 100644
> --- a/Platform/Hisilicon/HiKey/HiKey.dsc
> +++ b/Platform/Hisilicon/HiKey/HiKey.dsc
> @@ -61,6 +61,7 @@
>
> SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
>
>FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> +  CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
>
> UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
>
> PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
>
>

Thanks Kalyan

Could you resend it as a proper patch, please? I.e., with the appropriate

Contributed-under: TianoCore Contribution Agreement 1.1

and Signed-off-by line,
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ShellPkg/Shell: Check the OpenVolume result in OpenRootByHandle()

2017-11-14 Thread Ni, Ruiyu 
Reviewed-by: Ruiyu Ni 

Thanks/Ray

> -Original Message-
> From: Wu, Hao A
> Sent: Tuesday, November 14, 2017 4:41 PM
> To: Ni, Ruiyu ; edk2-devel@lists.01.org
> Cc: Carsey, Jaben 
> Subject: RE: [PATCH] ShellPkg/Shell: Check the OpenVolume result in
> OpenRootByHandle()
> 
> > -Original Message-
> > From: Ni, Ruiyu
> > Sent: Tuesday, November 14, 2017 4:38 PM
> > To: Wu, Hao A; edk2-devel@lists.01.org
> > Cc: Carsey, Jaben
> > Subject: RE: [PATCH] ShellPkg/Shell: Check the OpenVolume result in
> > OpenRootByHandle()
> >
> > How about changing the function header comments from
> > > +  @retval EFI_MEDIA_CHANGED The device has a different medium in it
> > or the medium is no longer supported.
> >
> > To:
> > > +  @retval others  Error status returned from
> > EFI_SIMPLE_FILE_SYSTEM_PROTOCOL->OpenVolume().
> >
> > Because MEDIA_CHANGED is not the only error that could happen.
> 
> Yes, I will update the comments when I push the patch if there's no other
> comments.
> 
> 
> Best Regards,
> Hao Wu
> 
> >
> >
> > Thanks/Ray
> >
> > > -Original Message-
> > > From: Wu, Hao A
> > > Sent: Tuesday, November 14, 2017 3:54 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Wu, Hao A ; Carsey, Jaben
> > > ; Ni, Ruiyu 
> > > Subject: [PATCH] ShellPkg/Shell: Check the OpenVolume result in
> > > OpenRootByHandle()
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=779
> > >
> > > For the API EfiShellOpenRootByHandle():
> > >
> > > The return status of the call to SimpleFileSystem->OpenVolume should
> > > be checked.
> > >
> > > It is possible that there is a media change in the device (like CD/DVD 
> > > ROM).
> > In
> > > such case, the volume root opened and/or the device path opened
> > > previously (also within EfiShellOpenRootByHandle) may be invalid.
> > >
> > > This commit adds a check for the result of OpenVolume before
> > > subsequently calling functions like EfiShellGetMapFromDevicePath() &
> > > ConvertEfiFileProtocolToShellHandle().
> > >
> > > Cc: Jaben Carsey 
> > > Cc: Ruiyu Ni 
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Hao Wu 
> > > ---
> > >  ShellPkg/Application/Shell/ShellProtocol.c | 9 +++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/ShellPkg/Application/Shell/ShellProtocol.c
> > > b/ShellPkg/Application/Shell/ShellProtocol.c
> > > index 5e34b8dad1..0dee267353 100644
> > > --- a/ShellPkg/Application/Shell/ShellProtocol.c
> > > +++ b/ShellPkg/Application/Shell/ShellProtocol.c
> > > @@ -827,7 +827,8 @@ EfiShellGetDeviceName(
> > >@retval EFI_NOT_FOUND EFI_SIMPLE_FILE_SYSTEM could not be
> > found
> > > or the root directory
> > >  could not be opened.
> > >@retval EFI_VOLUME_CORRUPTED  The data structures in the volume
> > were
> > > corrupted.
> > > -  @retval EFI_DEVICE_ERROR  The device had an error
> > > +  @retval EFI_DEVICE_ERROR  The device had an error.
> > > +  @retval EFI_MEDIA_CHANGED The device has a different medium in it
> > or
> > > the medium is no longer supported.
> > >  **/
> > >  EFI_STATUS
> > >  EFIAPI
> > > @@ -867,8 +868,12 @@ EfiShellOpenRootByHandle(
> > >// Open the root volume now...
> > >//
> > >Status = SimpleFileSystem->OpenVolume(SimpleFileSystem,
> > );
> > > +  if (EFI_ERROR(Status)) {
> > > +return Status;
> > > +  }
> > > +
> > >*FileHandle = ConvertEfiFileProtocolToShellHandle(RealFileHandle,
> > > EfiShellGetMapFromDevicePath());
> > > -  return (Status);
> > > +  return (EFI_SUCCESS);
> > >  }
> > >
> > >  /**
> > > --
> > > 2.12.0.windows.1

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

Re: [edk2] [PATCH] ShellPkg/Shell: Check the OpenVolume result in OpenRootByHandle()

2017-11-14 Thread Wu, Hao A 
> -Original Message-
> From: Ni, Ruiyu
> Sent: Tuesday, November 14, 2017 4:38 PM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Carsey, Jaben
> Subject: RE: [PATCH] ShellPkg/Shell: Check the OpenVolume result in
> OpenRootByHandle()
> 
> How about changing the function header comments from
> > +  @retval EFI_MEDIA_CHANGED The device has a different medium in it
> or the medium is no longer supported.
> 
> To:
> > +  @retval others  Error status returned from
> EFI_SIMPLE_FILE_SYSTEM_PROTOCOL->OpenVolume().
> 
> Because MEDIA_CHANGED is not the only error that could happen.

Yes, I will update the comments when I push the patch if there's no other
comments.


Best Regards,
Hao Wu

> 
> 
> Thanks/Ray
> 
> > -Original Message-
> > From: Wu, Hao A
> > Sent: Tuesday, November 14, 2017 3:54 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Hao A ; Carsey, Jaben
> > ; Ni, Ruiyu 
> > Subject: [PATCH] ShellPkg/Shell: Check the OpenVolume result in
> > OpenRootByHandle()
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=779
> >
> > For the API EfiShellOpenRootByHandle():
> >
> > The return status of the call to SimpleFileSystem->OpenVolume should be
> > checked.
> >
> > It is possible that there is a media change in the device (like CD/DVD ROM).
> In
> > such case, the volume root opened and/or the device path opened previously
> > (also within EfiShellOpenRootByHandle) may be invalid.
> >
> > This commit adds a check for the result of OpenVolume before subsequently
> > calling functions like EfiShellGetMapFromDevicePath() &
> > ConvertEfiFileProtocolToShellHandle().
> >
> > Cc: Jaben Carsey 
> > Cc: Ruiyu Ni 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Hao Wu 
> > ---
> >  ShellPkg/Application/Shell/ShellProtocol.c | 9 +++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/ShellPkg/Application/Shell/ShellProtocol.c
> > b/ShellPkg/Application/Shell/ShellProtocol.c
> > index 5e34b8dad1..0dee267353 100644
> > --- a/ShellPkg/Application/Shell/ShellProtocol.c
> > +++ b/ShellPkg/Application/Shell/ShellProtocol.c
> > @@ -827,7 +827,8 @@ EfiShellGetDeviceName(
> >@retval EFI_NOT_FOUND EFI_SIMPLE_FILE_SYSTEM could not be
> found
> > or the root directory
> >  could not be opened.
> >@retval EFI_VOLUME_CORRUPTED  The data structures in the volume
> were
> > corrupted.
> > -  @retval EFI_DEVICE_ERROR  The device had an error
> > +  @retval EFI_DEVICE_ERROR  The device had an error.
> > +  @retval EFI_MEDIA_CHANGED The device has a different medium in it
> or
> > the medium is no longer supported.
> >  **/
> >  EFI_STATUS
> >  EFIAPI
> > @@ -867,8 +868,12 @@ EfiShellOpenRootByHandle(
> >// Open the root volume now...
> >//
> >Status = SimpleFileSystem->OpenVolume(SimpleFileSystem,
> );
> > +  if (EFI_ERROR(Status)) {
> > +return Status;
> > +  }
> > +
> >*FileHandle = ConvertEfiFileProtocolToShellHandle(RealFileHandle,
> > EfiShellGetMapFromDevicePath());
> > -  return (Status);
> > +  return (EFI_SUCCESS);
> >  }
> >
> >  /**
> > --
> > 2.12.0.windows.1

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

[edk2] [Patch 3/3 V2] BaseTools: Update Makefile to support FFS file generation

2017-11-14 Thread Yonghong Zhu 
Update Makefile to support FFS file generation.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
Signed-off-by: Yunhua Feng 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py |  23 ++-
 BaseTools/Source/Python/AutoGen/GenMake.py |  87 +
 BaseTools/Source/Python/GenFds/AprioriSection.py   |  14 +-
 BaseTools/Source/Python/GenFds/CompressSection.py  |  10 +-
 BaseTools/Source/Python/GenFds/DataSection.py  |  37 ++--
 BaseTools/Source/Python/GenFds/DepexSection.py |   6 +-
 BaseTools/Source/Python/GenFds/EfiSection.py   |  78 +---
 BaseTools/Source/Python/GenFds/Fd.py   |  24 ++-
 BaseTools/Source/Python/GenFds/FfsFileStatement.py |   4 +-
 BaseTools/Source/Python/GenFds/FfsInfStatement.py  | 202 -
 BaseTools/Source/Python/GenFds/Fv.py   | 189 ++-
 BaseTools/Source/Python/GenFds/FvImageSection.py   |   8 +-
 BaseTools/Source/Python/GenFds/GenFds.py   |  17 ++
 .../Source/Python/GenFds/GenFdsGlobalVariable.py   | 170 +
 BaseTools/Source/Python/GenFds/GuidSection.py  |  59 +-
 .../Source/Python/GenFds/OptRomFileStatement.py|   4 +-
 .../Source/Python/GenFds/OptRomInfStatement.py |  12 +-
 BaseTools/Source/Python/GenFds/OptionRom.py|  25 +--
 BaseTools/Source/Python/GenFds/Region.py   |  49 +++--
 BaseTools/Source/Python/GenFds/Section.py  |   4 +-
 BaseTools/Source/Python/GenFds/UiSection.py|   5 +-
 BaseTools/Source/Python/GenFds/VerSection.py   |   9 +-
 BaseTools/Source/Python/build/build.py |  45 -
 23 files changed, 692 insertions(+), 389 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 5317921..fe0556d 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -1305,16 +1305,19 @@ class PlatformAutoGen(AutoGen):
 ## Create makefile for the platform and mdoules in it
 #
 #   @param  CreateModuleMakeFileFlag indicating if the makefile for
 #   modules will be created as well
 #
-def CreateMakeFile(self, CreateModuleMakeFile=False):
+def CreateMakeFile(self, CreateModuleMakeFile=False, FfsCommand = {}):
 if CreateModuleMakeFile:
 for ModuleFile in self.Platform.Modules:
 Ma = ModuleAutoGen(self.Workspace, ModuleFile, 
self.BuildTarget,
self.ToolChain, self.Arch, self.MetaFile)
-Ma.CreateMakeFile(True)
+if (ModuleFile.File, self.Arch) in FfsCommand:
+Ma.CreateMakeFile(True, FfsCommand[ModuleFile.File, 
self.Arch])
+else:
+Ma.CreateMakeFile(True)
 #Ma.CreateAsBuiltInf()
 
 # no need to create makefile for the platform more than once
 if self.IsMakeFileCreated:
 return
@@ -2758,10 +2761,11 @@ class ModuleAutoGen(AutoGen):
 self._CustomMakefile  = None
 self._Macro   = None
 
 self._BuildDir= None
 self._OutputDir   = None
+self._FfsOutputDir= None
 self._DebugDir= None
 self._MakeFileDir = None
 
 self._IncludePathList = None
 self._IncludePathLength = 0
@@ -2874,10 +2878,11 @@ class ModuleAutoGen(AutoGen):
 self._Macro["PLATFORM_GUID" ] = self.PlatformInfo.Guid
 self._Macro["PLATFORM_VERSION"  ] = self.PlatformInfo.Version
 self._Macro["PLATFORM_RELATIVE_DIR" ] = self.PlatformInfo.SourceDir
 self._Macro["PLATFORM_DIR"  ] = 
mws.join(self.WorkspaceDir, self.PlatformInfo.SourceDir)
 self._Macro["PLATFORM_OUTPUT_DIR"   ] = self.PlatformInfo.OutputDir
+self._Macro["FFS_OUTPUT_DIR"] = self.FfsOutputDir
 return self._Macro
 
 ## Return the module build data object
 def _GetModule(self):
 if self._Module == None:
@@ -2964,10 +2969,19 @@ class ModuleAutoGen(AutoGen):
 if self._OutputDir == None:
 self._OutputDir = path.join(self.BuildDir, "OUTPUT")
 CreateDirectory(self._OutputDir)
 return self._OutputDir
 
+## Return the directory to store ffs file
+def _GetFfsOutputDir(self):
+if self._FfsOutputDir == None:
+if GlobalData.gFdfParser != None:
+self._FfsOutputDir = path.join(self.PlatformInfo.BuildDir, 
"FV", "Ffs", self.Guid + self.Name)
+else:
+self._FfsOutputDir = ''
+return self._FfsOutputDir
+
 ## Return the directory to store auto-gened source files of the mdoule
 def _GetDebugDir(self):
 if self._DebugDir == None:
 self._DebugDir =

[edk2] [PATCH] ArmVirtPkg/ArmVirtQemu: use non-accelerated CopyMem for VariableRuntimeDxe

2017-11-14 Thread Ard Biesheuvel 
The VariableRuntimeDxe driver may use CopyMem () on NOR flash regions,
assuming such regions always have full memory semantics. Given that
those regions cannot be mapped as ordinary memory on ARM (due to the
fact that the NOR flash requires device semantics while in write mode)
this prevents us from using BaseMemoryLibOptDxe in VariableRuntimeDxe,
since it may use unaligned accesses and/or DC ZVA instructions, both
of which are incompatible with mappings using device semantics.

Note that there is no way we can work around this by changing the
mapping type between 'memory' and 'device' when switching from read to
write mode and back, because the runtime mapping is created by the OS,
and cannot be changed at will.

So let's just switch to the unaccelerated version of BaseMemoryLib which
does not have the same problem.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/ArmVirtQemu.dsc   | 2 ++
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 8a60b61f2aa6..7b220d6e3c31 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -261,6 +261,8 @@ [Components.common]
   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
 
   NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
+  # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
+  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
   }
 !if $(SECURE_BOOT_ENABLE) == TRUE
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 9a31ec93ca06..7c032e1b07e0 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -252,6 +252,8 @@ [Components.common]
   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
 
   NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
+  # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
+  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
   }
 !if $(SECURE_BOOT_ENABLE) == TRUE
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
-- 
2.11.0

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

Re: [edk2] [Patch] BaseTools: Fix the bug to collect source files per build rule family

2017-11-14 Thread Gao, Liming 
Reviewed-by: Liming Gao 

>-Original Message-
>From: Zhu, Yonghong
>Sent: Tuesday, November 14, 2017 1:53 PM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming 
>Subject: [Patch] BaseTools: Fix the bug to collect source files per build rule
>family
>
>when collect source files list we should also consider build rule
>family.
>
>Cc: Liming Gao 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Yonghong Zhu 
>---
> BaseTools/Source/Python/AutoGen/AutoGen.py | 8 
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
>b/BaseTools/Source/Python/AutoGen/AutoGen.py
>index b3e7089..008ad8e 100644
>--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
>+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
>@@ -3265,17 +3265,17 @@ class ModuleAutoGen(AutoGen):
> # match tool chain
> if F.TagName not in ("", "*", self.ToolChain):
> EdkLogger.debug(EdkLogger.DEBUG_9, "The toolchain [%s] for
>processing file [%s] is found, "
> "but [%s] is needed" % (F.TagName, 
> str(F), self.ToolChain))
> continue
>-# match tool chain family
>-if F.ToolChainFamily not in ("", "*", self.ToolChainFamily):
>+# match tool chain family or build rule family
>+if F.ToolChainFamily not in ("", "*", self.ToolChainFamily,
>self.BuildRuleFamily):
> EdkLogger.debug(
> EdkLogger.DEBUG_0,
> "The file [%s] must be built by tools of 
> [%s], " \
>-"but current toolchain family is [%s]" \
>-% (str(F), F.ToolChainFamily, 
>self.ToolChainFamily))
>+"but current toolchain family is [%s], 
>buildrule family is [%s]"
>\
>+% (str(F), F.ToolChainFamily, 
>self.ToolChainFamily,
>self.BuildRuleFamily))
> continue
>
> # add the file path into search path list for file including
> if F.Dir not in self.IncludePathList and self.AutoGenVersion 
> >=
>0x00010005:
> self.IncludePathList.insert(0, F.Dir)
>--
>2.6.1.windows.1

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

[edk2] [PATCH] CorebootModulePkg/CbSupportDxe: Remove duplicated IO Space addition

2017-11-14 Thread Benjamin You 
Since UefiCpuPkg's CpuDxe Driver already adds Local Apic's MMIO space to
GCD, CorebootModulePkg's CbSupportDxe should not do this again. Doing this
again causes error return status from GCD service, and ASSERT (FALSE) with
debug build, so the duplicated addition is removed.

Cc: Maurice Ma 
Cc: Prince Agyeman 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Benjamin You 
---
 CorebootModulePkg/CbSupportDxe/CbSupportDxe.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c 
b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
index 24bacf815c..c526c9e871 100755
--- a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
+++ b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
@@ -140,9 +140,6 @@ CbDxeEntryPoint (
   //
   // Report MMIO/IO Resources
   //
-  Status = CbReserveResourceInGcd (TRUE, EfiGcdMemoryTypeMemoryMappedIo, 
0xFEE0, SIZE_1MB, 0, SystemTable); // LAPIC
-  ASSERT_EFI_ERROR (Status);
-
   Status = CbReserveResourceInGcd (TRUE, EfiGcdMemoryTypeMemoryMappedIo, 
0xFEC0, SIZE_4KB, 0, SystemTable); // IOAPIC
   ASSERT_EFI_ERROR (Status);
 
-- 
2.14.3.windows.1

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

Re: [edk2] [PATCH] ShellPkg/Shell: Check the OpenVolume result in OpenRootByHandle()

2017-11-14 Thread Ni, Ruiyu 
How about changing the function header comments from
> +  @retval EFI_MEDIA_CHANGED The device has a different medium in it or 
> the medium is no longer supported.

To:
> +  @retval others  Error status returned from 
> EFI_SIMPLE_FILE_SYSTEM_PROTOCOL->OpenVolume().

Because MEDIA_CHANGED is not the only error that could happen.


Thanks/Ray

> -Original Message-
> From: Wu, Hao A
> Sent: Tuesday, November 14, 2017 3:54 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Carsey, Jaben
> ; Ni, Ruiyu 
> Subject: [PATCH] ShellPkg/Shell: Check the OpenVolume result in
> OpenRootByHandle()
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=779
> 
> For the API EfiShellOpenRootByHandle():
> 
> The return status of the call to SimpleFileSystem->OpenVolume should be
> checked.
> 
> It is possible that there is a media change in the device (like CD/DVD ROM). 
> In
> such case, the volume root opened and/or the device path opened previously
> (also within EfiShellOpenRootByHandle) may be invalid.
> 
> This commit adds a check for the result of OpenVolume before subsequently
> calling functions like EfiShellGetMapFromDevicePath() &
> ConvertEfiFileProtocolToShellHandle().
> 
> Cc: Jaben Carsey 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu 
> ---
>  ShellPkg/Application/Shell/ShellProtocol.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/ShellPkg/Application/Shell/ShellProtocol.c
> b/ShellPkg/Application/Shell/ShellProtocol.c
> index 5e34b8dad1..0dee267353 100644
> --- a/ShellPkg/Application/Shell/ShellProtocol.c
> +++ b/ShellPkg/Application/Shell/ShellProtocol.c
> @@ -827,7 +827,8 @@ EfiShellGetDeviceName(
>@retval EFI_NOT_FOUND EFI_SIMPLE_FILE_SYSTEM could not be found
> or the root directory
>  could not be opened.
>@retval EFI_VOLUME_CORRUPTED  The data structures in the volume were
> corrupted.
> -  @retval EFI_DEVICE_ERROR  The device had an error
> +  @retval EFI_DEVICE_ERROR  The device had an error.
> +  @retval EFI_MEDIA_CHANGED The device has a different medium in it or
> the medium is no longer supported.
>  **/
>  EFI_STATUS
>  EFIAPI
> @@ -867,8 +868,12 @@ EfiShellOpenRootByHandle(
>// Open the root volume now...
>//
>Status = SimpleFileSystem->OpenVolume(SimpleFileSystem, );
> +  if (EFI_ERROR(Status)) {
> +return Status;
> +  }
> +
>*FileHandle = ConvertEfiFileProtocolToShellHandle(RealFileHandle,
> EfiShellGetMapFromDevicePath());
> -  return (Status);
> +  return (EFI_SUCCESS);
>  }
> 
>  /**
> --
> 2.12.0.windows.1

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

[edk2] [Patch 2/3 V2] BaseTools: Update Trim to generate VfrBinOffset Binary

2017-11-14 Thread Yonghong Zhu 
Its usage is
"Trim --Vfr-Uni-Offset -o $(OUTPUT_DIR)(+)$(MODULE_NAME)VfrOffset.sec
--ModuleName=$(MODULE_NAME) --DebugDir=$(DEBUG_DIR)"

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
---
 BaseTools/Source/Python/Trim/Trim.py | 81 ++--
 1 file changed, 78 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/Python/Trim/Trim.py 
b/BaseTools/Source/Python/Trim/Trim.py
index 9ccc027..d1e40b0 100644
--- a/BaseTools/Source/Python/Trim/Trim.py
+++ b/BaseTools/Source/Python/Trim/Trim.py
@@ -1,9 +1,9 @@
 ## @file
 # Trim files preprocessed by compiler
 #
-# Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2007 - 2017, 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
 #
@@ -15,10 +15,11 @@
 # Import Modules
 #
 import Common.LongFilePathOs as os
 import sys
 import re
+import StringIO
 
 from optparse import OptionParser
 from optparse import make_option
 from Common.BuildToolError import *
 from Common.Misc import *
@@ -27,11 +28,11 @@ import Common.EdkLogger as EdkLogger
 from Common.LongFilePathSupport import OpenLongFilePath as open
 
 # Version and Copyright
 __version_number__ = ("0.10" + " " + gBUILD_VERSION)
 __version__ = "%prog Version " + __version_number__
-__copyright__ = "Copyright (c) 2007-2016, Intel Corporation. All rights 
reserved."
+__copyright__ = "Copyright (c) 2007-2017, Intel Corporation. All rights 
reserved."
 
 ## Regular expression for matching Line Control directive like "#line xxx"
 gLineControlDirective = re.compile('^\s*#(?:line)?\s+([0-9]+)\s+"*([^"]*)"')
 ## Regular expression for matching "typedef struct"
 gTypedefPattern = re.compile("^\s*typedef\s+struct(\s+\w+)?\s*[{]*$", 
re.MULTILINE)
@@ -428,10 +429,72 @@ def TrimAslFile(Source, Target, IncludePathFile):
 EdkLogger.error("Trim", FILE_OPEN_FAILURE, ExtraData=Target)
 
 f.writelines(Lines)
 f.close()
 
+def GenerateVfrBinSec(ModuleName, DebugDir, OutputFile):
+VfrNameList = []
+if os.path.isdir(DebugDir):
+for CurrentDir, Dirs, Files in os.walk(DebugDir):
+for FileName in Files:
+Name, Ext = os.path.splitext(FileName)
+if Ext == '.lst':
+VfrNameList.append (Name + 'Bin')
+
+VfrNameList.append (ModuleName + 'Strings')
+
+EfiFileName = os.path.join(DebugDir, ModuleName + '.efi')
+MapFileName = os.path.join(DebugDir, ModuleName + '.map')
+VfrUniOffsetList = GetVariableOffset(MapFileName, EfiFileName, VfrNameList)
+
+if not VfrUniOffsetList:
+return
+
+try:
+fInputfile = open(OutputFile, "wb+", 0)
+except:
+EdkLogger.error("Trim", FILE_OPEN_FAILURE, "File open failed for %s" 
%OutputFile, None)
+
+# Use a instance of StringIO to cache data
+fStringIO = StringIO.StringIO('')
+
+for Item in VfrUniOffsetList:
+if (Item[0].find("Strings") != -1):
+#
+# UNI offset in image.
+# GUID + Offset
+# { 0x8913c5e0, 0x33f6, 0x4d86, { 0x9b, 0xf1, 0x43, 0xef, 0x89, 
0xfc, 0x6, 0x66 } }
+#
+UniGuid = [0xe0, 0xc5, 0x13, 0x89, 0xf6, 0x33, 0x86, 0x4d, 0x9b, 
0xf1, 0x43, 0xef, 0x89, 0xfc, 0x6, 0x66]
+UniGuid = [chr(ItemGuid) for ItemGuid in UniGuid]
+fStringIO.write(''.join(UniGuid))
+UniValue = pack ('Q', int (Item[1], 16))
+fStringIO.write (UniValue)
+else:
+#
+# VFR binary offset in image.
+# GUID + Offset
+# { 0xd0bc7cb4, 0x6a47, 0x495f, { 0xaa, 0x11, 0x71, 0x7, 0x46, 
0xda, 0x6, 0xa2 } };
+#
+VfrGuid = [0xb4, 0x7c, 0xbc, 0xd0, 0x47, 0x6a, 0x5f, 0x49, 0xaa, 
0x11, 0x71, 0x7, 0x46, 0xda, 0x6, 0xa2]
+VfrGuid = [chr(ItemGuid) for ItemGuid in VfrGuid]
+fStringIO.write(''.join(VfrGuid))
+type (Item[1])
+VfrValue = pack ('Q', int (Item[1], 16))
+fStringIO.write (VfrValue)
+
+#
+# write data into file.
+#
+try :
+fInputfile.write (fStringIO.getvalue())
+except:
+EdkLogger.error("Trim", FILE_WRITE_FAILURE, "Write data to file %s 
failed, please check whether the file been locked or using by other 
applications." %OutputFile, None)
+
+fStringIO.close ()
+fInputfile.close ()
+
 ## Trim EDK source code file(s)
 #
 #
 # @param  SourceFile or directory to be trimmed
 # @param  TargetFile or directory to store the trimmed content
@@ -533,10 +596,12 @@ def Options():
 OptionList = [
 make_option("-s", "--source-code", dest="FileType", 
const="SourceCode", action="store_const",

[edk2] [Patch 0/3 V2] BaseTools: Enable multiple thread to generate FFS file

2017-11-14 Thread Yonghong Zhu 
These patches enable multiple thread to generate ffs file by merge FFS file's
generation into Make phase.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 

Yonghong Zhu (3):
  BaseTools: GenFfs support to get alignment value from SectionFile
  BaseTools: Update Trim to generate VfrBinOffset Binary
  BaseTools: Update Makefile to support FFS file generation

 BaseTools/Source/C/GenFfs/GenFfs.c | 135 +-
 BaseTools/Source/Python/AutoGen/AutoGen.py |  23 ++-
 BaseTools/Source/Python/AutoGen/GenMake.py |  87 +
 BaseTools/Source/Python/GenFds/AprioriSection.py   |  14 +-
 BaseTools/Source/Python/GenFds/CompressSection.py  |  10 +-
 BaseTools/Source/Python/GenFds/DataSection.py  |  37 ++--
 BaseTools/Source/Python/GenFds/DepexSection.py |   6 +-
 BaseTools/Source/Python/GenFds/EfiSection.py   |  78 +---
 BaseTools/Source/Python/GenFds/Fd.py   |  24 ++-
 BaseTools/Source/Python/GenFds/FfsFileStatement.py |   4 +-
 BaseTools/Source/Python/GenFds/FfsInfStatement.py  | 202 -
 BaseTools/Source/Python/GenFds/Fv.py   | 189 ++-
 BaseTools/Source/Python/GenFds/FvImageSection.py   |   8 +-
 BaseTools/Source/Python/GenFds/GenFds.py   |  17 ++
 .../Source/Python/GenFds/GenFdsGlobalVariable.py   | 170 +
 BaseTools/Source/Python/GenFds/GuidSection.py  |  59 +-
 .../Source/Python/GenFds/OptRomFileStatement.py|   4 +-
 .../Source/Python/GenFds/OptRomInfStatement.py |  12 +-
 BaseTools/Source/Python/GenFds/OptionRom.py|  25 +--
 BaseTools/Source/Python/GenFds/Region.py   |  49 +++--
 BaseTools/Source/Python/GenFds/Section.py  |   4 +-
 BaseTools/Source/Python/GenFds/UiSection.py|   5 +-
 BaseTools/Source/Python/GenFds/VerSection.py   |   9 +-
 BaseTools/Source/Python/Trim/Trim.py   |  81 -
 BaseTools/Source/Python/build/build.py |  45 -
 25 files changed, 902 insertions(+), 395 deletions(-)

-- 
2.6.1.windows.1

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

[edk2] [Patch 1/3 V2] BaseTools: GenFfs support to get alignment value from SectionFile

2017-11-14 Thread Yonghong Zhu 
Update GenFfs tool to get alignment value from SectionFile when use
the new option -n 0.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/C/GenFfs/GenFfs.c | 135 -
 1 file changed, 132 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/C/GenFfs/GenFfs.c 
b/BaseTools/Source/C/GenFfs/GenFfs.c
index cb16f38..e2fb3e0 100644
--- a/BaseTools/Source/C/GenFfs/GenFfs.c
+++ b/BaseTools/Source/C/GenFfs/GenFfs.c
@@ -10,10 +10,17 @@ http://opensource.org/licenses/bsd-license.php
 THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,  
   
 WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.  
   
 
 **/
 
+#ifndef __GNUC__
+#include 
+#include 
+#include 
+#include 
+#endif
+
 #include 
 #include 
 #include 
 
 #include 
@@ -22,10 +29,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 
 #include "CommonLib.h"
 #include "ParseInf.h"
 #include "EfiUtilityMsgs.h"
+#include "FvLib.h"
+#include "PeCoffLib.h"
 
 #define UTILITY_NAME"GenFfs"
 #define UTILITY_MAJOR_VERSION   0
 #define UTILITY_MINOR_VERSION   1
 
@@ -151,12 +160,14 @@ Returns:
 128K,256K,512K,1M,2M,4M,8M,16M\n");
   fprintf (stdout, "  -i SectionFile, --sectionfile SectionFile\n\
 Section file will be contained in this FFS file.\n");
   fprintf (stdout, "  -n SectionAlign, --sectionalign SectionAlign\n\
 SectionAlign points to section alignment, which 
support\n\
-the alignment scope 1~16M. It is specified together\n\
-with sectionfile to point its alignment in FFS 
file.\n");
+the alignment scope 0~16M. If SectionAlign is 
specified\n\
+as 0, tool get alignment value from SectionFile. It 
is\n\
+specified together with sectionfile to point its\n\
+alignment in FFS file.\n");
   fprintf (stdout, "  -v, --verbose Turn on verbose output with 
informational messages.\n");
   fprintf (stdout, "  -q, --quiet   Disable all messages except key 
message and fatal error\n");
   fprintf (stdout, "  -d, --debug level Enable debug messages, at input 
debug level.\n");
   fprintf (stdout, "  --version Show program's version number and 
exit.\n");
   fprintf (stdout, "  -h, --helpShow this help message and 
exit.\n");
@@ -455,10 +466,106 @@ Returns:
 *BufferLength = Size;
 return EFI_SUCCESS;
   }
 }
 
+EFI_STATUS
+FfsRebaseImageRead (
+IN  VOID*FileHandle,
+IN  UINTN   FileOffset,
+IN OUT  UINT32  *ReadSize,
+OUT VOID*Buffer
+)
+  /*++
+
+Routine Description:
+
+Support routine for the PE/COFF Loader that reads a buffer from a PE/COFF 
file
+
+Arguments:
+
+   FileHandle - The handle to the PE/COFF file
+
+   FileOffset - The offset, in bytes, into the file to read
+
+   ReadSize   - The number of bytes to read from the file starting at 
FileOffset
+
+   Buffer - A pointer to the buffer to read the data into.
+
+   Returns:
+
+   EFI_SUCCESS - ReadSize bytes of data were read into Buffer from the PE/COFF 
file starting at FileOffset
+
+   --*/
+{
+  CHAR8   *Destination8;
+  CHAR8   *Source8;
+  UINT32  Length;
+
+  Destination8  = Buffer;
+  Source8   = (CHAR8 *) ((UINTN) FileHandle + FileOffset);
+  Length= *ReadSize;
+  while (Length--) {
+*(Destination8++) = *(Source8++);
+  }
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+GetAlignmentFromFile(char *InFile, UINT32 *Alignment)
+  /*++
+InFile is input file for getting alignment
+return the alignment
+--*/
+{
+  FILE   *InFileHandle;
+  UINT8  *PeFileBuffer;
+  UINTN  PeFileSize;
+  UINT32 CurSecHdrSize;
+  PE_COFF_LOADER_IMAGE_CONTEXT   ImageContext;
+  EFI_COMMON_SECTION_HEADER  *CommonHeader;
+  EFI_STATUS Status;
+
+  InFileHandle= NULL;
+  PeFileBuffer= NULL;
+
+  memset (, 0, sizeof (ImageContext));
+
+  InFileHandle = fopen(LongFilePath(InFile), "rb");
+  if (InFileHandle == NULL){
+Error (NULL, 0, 0001, "Error opening file", InFile);
+return EFI_ABORTED;
+  }
+  PeFileSize = _filelength (fileno(InFileHandle));
+  PeFileBuffer = (UINT8 *) malloc (PeFileSize);
+  if (PeFileBuffer == NULL) {
+fclose (InFileHandle);
+Error(NULL, 0, 4001, "Resource", "memory cannot be allocated  of %s", 
InFileHandle);
+return EFI_OUT_OF_RESOURCES;
+  }
+  fread (PeFileBuffer, sizeof (UINT8), PeFileSize, InFileHandle);
+  fclose (InFileHandle);
+  CommonHeader = (EFI_COMMON_SECTION_HEADER *) PeFileBuffer;
+  CurSecHdrSize =