Re: [edk2] [PATCH 2/5] MdePkg: Include Shell/ShellDynamicCommand/ShellParameters definitions

[Ni, Ruiyu]

I don't want to lose the file history either.
I thought git can auto-detect the copy/move of files. I will investigate 
further.

Thanks/Ray

> -Original Message-
> From: Carsey, Jaben
> Sent: Tuesday, October 18, 2016 2:02 AM
> To: Ni, Ruiyu ; edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Yao, Jiewen
> ; Carsey, Jaben 
> Subject: RE: [edk2] [PATCH 2/5] MdePkg: Include
> Shell/ShellDynamicCommand/ShellParameters definitions
> 
> Can we do a GIT move operation and then merge the content from
> ShellBase.h please?  I do not want to lose the file history information.
> 
> When we move by "delete" then "add" we lose everything for no reason.
> 
> -Jaben
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Ruiyu Ni
> > Sent: Friday, October 14, 2016 2:44 AM
> > To: edk2-devel@lists.01.org
> > Cc: Carsey, Jaben ; Kinney, Michael D
> > ; Yao, Jiewen 
> > Subject: [edk2] [PATCH 2/5] MdePkg: Include
> > Shell/ShellDynamicCommand/ShellParameters definitions
> > Importance: High
> >
> > Copy Shell/ShellDynamicCommand/ShellParameters definitions from
> > ShellPkg to MdePkg.
> > Content of ShellBase.h is moved to Protocol/Shell.h.
> >
> > The following patches will update ShellPkg to reference all protocol
> > definition to MdePkg.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Ruiyu Ni 
> > Cc: Jaben Carsey 
> > Cc: Jiewen Yao 
> > Cc: Michael D Kinney 
> > ---
> >  MdePkg/Include/Protocol/Shell.h   | 1268
> > +
> >  MdePkg/Include/Protocol/ShellDynamicCommand.h |   85 ++
> >  MdePkg/Include/Protocol/ShellParameters.h |   60 ++
> >  MdePkg/MdePkg.dec |   15 +
> >  4 files changed, 1428 insertions(+)
> >  create mode 100644 MdePkg/Include/Protocol/Shell.h
> >  create mode 100644 MdePkg/Include/Protocol/ShellDynamicCommand.h
> >  create mode 100644 MdePkg/Include/Protocol/ShellParameters.h
> >
> > diff --git a/MdePkg/Include/Protocol/Shell.h
> > b/MdePkg/Include/Protocol/Shell.h
> > new file mode 100644
> > index 000..cc1fbdc
> > --- /dev/null
> > +++ b/MdePkg/Include/Protocol/Shell.h
> > @@ -0,0 +1,1268 @@
> > +/** @file
> > +  EFI Shell protocol as defined in the UEFI Shell 2.0 specification 
> > including
> > errata.
> > +
> > +  (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
> > +  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the
> BSD
> > License
> > +  which accompanies this distribution.  The full text of the license may be
> > found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +
> > +  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 __EFI_SHELL_PROTOCOL__
> > +#define __EFI_SHELL_PROTOCOL__
> > +
> > +#include 
> > +
> > +#define EFI_SHELL_PROTOCOL_GUID \
> > +  { \
> > +  0x6302d008, 0x7f9b, 0x4f30, { 0x87, 0xac, 0x60, 0xc9, 0xfe, 0xf5, 0xda,
> 0x4e
> > } \
> > +  }
> > +typedef VOID *SHELL_FILE_HANDLE;
> > +
> > +typedef enum {
> > +  ///
> > +  /// The operation completed successfully.
> > +  ///
> > +  SHELL_SUCCESS   = 0,
> > +
> > +  ///
> > +  /// The image failed to load.
> > +  ///
> > +  SHELL_LOAD_ERROR= 1,
> > +
> > +  ///
> > +  /// The parameter was incorrect.
> > +  ///
> > +  SHELL_INVALID_PARAMETER = 2,
> > +
> > +  ///
> > +  /// The operation is not supported.
> > +  ///
> > +  SHELL_UNSUPPORTED   = 3,
> > +
> > +  ///
> > +  /// The buffer was not the proper size for the request.
> > +  ///
> > +  SHELL_BAD_BUFFER_SIZE   = 4,
> > +
> > +  ///
> > +  /// The buffer was not large enough to hold the requested data.
> > +  /// The required buffer size is returned in the appropriate
> > +  /// parameter when this error occurs.
> > +  ///
> > +  SHELL_BUFFER_TOO_SMALL  = 5,
> > +
> > +  ///
> > +  /// There is no data pending upon return.
> > +  ///
> > +  SHELL_NOT_READY = 6,
> > +
> > +  ///
> > +  /// The physical device reported an error while attempting the
> > +  /// operation.
> > +  ///
> > +  SHELL_DEVICE_ERROR  = 7,
> > +
> > +  ///
> > +  /// The device cannot be written to.
> > +  ///
> > +  SHELL_WRITE_PROTECTED   = 8,
> > +
> > +  ///
> > +  /// The resource has run out.
> > +  ///
> > +  SHELL_OUT_OF_RESOURCES  = 9,
> > +
> > +  ///
> > +  /// An inconsistency was detected on the file system causing the
> > +  /// operation to fail.
> > +  ///
> > +  

[edk2] [patch] MdeModulePkg/Ufs: ensure the DBC field of UTP PRDT is dword-aligned

[Feng Tian]

According to UFS Host Controller Spec(JESD223), the bits 1:0 of this
DataByteCount field shall be 11b to indicate Dword granularity.

But the size of UFS Request Sense Data Response defined in UFS Spec
(JESD220C) is 18 which is not Dword aligned, we would have to round
down to the multiple of 4 to fill the DBC field to avoid bring issue
on some UFS HCs.

Cc: Hao Wu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian 
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c 
b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index 26986cb..a854264 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -399,6 +399,11 @@ UfsInitUtpPrdt (
   UINT32 RemainingLen;
   UINT8  *Remaining;
   UINTN  PrdtNumber;
+  
+  if ((BufferSize & (BIT0 | BIT1)) != 0) {
+BufferSize &= ~(BIT0 | BIT1);
+DEBUG ((EFI_D_WARN, "UfsInitUtpPrdt: The BufferSize [%d] is not 
dword-aligned!\n", BufferSize));
+  }
 
   if (BufferSize == 0) {
 return EFI_SUCCESS;
-- 
2.7.1.windows.2

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

Re: [edk2] [PATCH] IntelFsp2WrapperPkg: Add a PCD to control if signaling first event.

[Yao, Jiewen]

Hi
I am a little concern on this as well.

If today, there is a POC require skipping PciEnumerationComplete.
There might be the other POC tomorrow to skip ReadyToBoot
Then another POC to skip EventExitBootServices.

We will be exhausted to add POC to skip each separately.

Can this POC handle all 3 event together? Or not handle 3 event completely?


Thank you
Yao Jiewen

From: Dong, Guo
Sent: Tuesday, October 18, 2016 12:04 PM
To: Mudusuru, Giri P ; edk2-devel@lists.01.org
Cc: Yao, Jiewen ; Ma, Maurice ; 
Yarlagadda, Satya P 
Subject: RE: [edk2] [PATCH] IntelFsp2WrapperPkg: Add a PCD to control if 
signaling first event.


Hi Giri,

Thank you for the comment.

In our POC implementation, Coreboot will notify FSP EnumerationComplete.  And 
in UEFI payload, the other 2 FSP notifications will be called from 
FspWrapperNotifyDxe.

Thanks,
Guo

-Original Message-
From: Mudusuru, Giri P
Sent: Monday, October 17, 2016 8:32 PM
To: Dong, Guo >; 
edk2-devel@lists.01.org
Cc: Yao, Jiewen >; Ma, 
Maurice >; Yarlagadda, Satya 
P >
Subject: RE: [edk2] [PATCH] IntelFsp2WrapperPkg: Add a PCD to control if 
signaling first event.

Hello Guo,
FSP wrapper and Coreboot are bootloaders consuming FSP binary.

FSP must get called Post PCI bus enumeration to do the required silicon 
initialization. Why do we need a PCD to skip it?

Thanks,
-Giri

> -Original Message-
> From: Dong, Guo
> Sent: Monday, October 17, 2016 3:53 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen >; 
> Mudusuru, Giri P
> >; Dong, Guo 
> >
> Subject: [edk2] [PATCH] IntelFsp2WrapperPkg: Add a PCD to control if
> signaling first event.
>
> PciEnumerationComplete might be signaled to FSP in Coreboot. Add a PCD
> PcdNotifyPciEnumerationComplete so FspWrapperNotifyDxe driver could be
> used in this case.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: gdong1 >
> ---
>  .../FspWrapperNotifyDxe/FspWrapperNotifyDxe.c  | 18 
> ++
>  .../FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf|  3 ++-
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec|  5 +
>  3 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git
> a/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.c
> b/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.c
> index 0797f44..45eae4b 100644
> --- a/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.c
> +++ b/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.c
> @@ -246,14 +246,16 @@ FspWrapperNotifyDxeEntryPoint (
>  return EFI_SUCCESS;
>}
>
> -  ProtocolNotifyEvent = EfiCreateProtocolNotifyEvent (
> -  ,
> -  TPL_CALLBACK,
> -  OnPciEnumerationComplete,
> -  NULL,
> -  
> -  );
> -  ASSERT (ProtocolNotifyEvent != NULL);
> +  if (PcdGetBool(PcdNotifyPciEnumerationComplete)) {
> +ProtocolNotifyEvent = EfiCreateProtocolNotifyEvent (
> +,
> +TPL_CALLBACK,
> +OnPciEnumerationComplete,
> +NULL,
> +
> +);
> +ASSERT (ProtocolNotifyEvent != NULL);  }
>
>Status = EfiCreateEventReadyToBootEx (
>   TPL_CALLBACK,
> diff --git
> a/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf
> b/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf
> index f851f68..f28356d 100644
> --- a/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf
> +++ b/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf
> @@ -61,7 +61,8 @@
>gFspHobGuid   ## CONSUMES ## HOB
>
>  [Pcd]
> -  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress  ## CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress   ##
> CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdNotifyPciEnumerationComplete  ##
> CONSUMES
>
>  [Depex]
>TRUE
> diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> index d9d2d80..52ed9d8 100644
> --- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> +++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> @@ -74,6 +74,11 @@
>## This is the base address of FSP-T/M/S
>
> 

Re: [edk2] [PATCH] IntelFsp2WrapperPkg: Add a PCD to control if signaling first event.

[Mudusuru, Giri P]

Hello Guo,
FSP wrapper and Coreboot are bootloaders consuming FSP binary. 

FSP must get called Post PCI bus enumeration to do the required silicon 
initialization. Why do we need a PCD to skip it?

Thanks,
-Giri

> -Original Message-
> From: Dong, Guo
> Sent: Monday, October 17, 2016 3:53 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Mudusuru, Giri P
> ; Dong, Guo 
> Subject: [edk2] [PATCH] IntelFsp2WrapperPkg: Add a PCD to control if signaling
> first event.
> 
> PciEnumerationComplete might be signaled to FSP in Coreboot. Add a
> PCD PcdNotifyPciEnumerationComplete so FspWrapperNotifyDxe driver
> could be used in this case.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: gdong1 
> ---
>  .../FspWrapperNotifyDxe/FspWrapperNotifyDxe.c  | 18 
> ++
>  .../FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf|  3 ++-
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec|  5 +
>  3 files changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.c
> b/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.c
> index 0797f44..45eae4b 100644
> --- a/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.c
> +++ b/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.c
> @@ -246,14 +246,16 @@ FspWrapperNotifyDxeEntryPoint (
>  return EFI_SUCCESS;
>}
> 
> -  ProtocolNotifyEvent = EfiCreateProtocolNotifyEvent (
> -  ,
> -  TPL_CALLBACK,
> -  OnPciEnumerationComplete,
> -  NULL,
> -  
> -  );
> -  ASSERT (ProtocolNotifyEvent != NULL);
> +  if (PcdGetBool(PcdNotifyPciEnumerationComplete)) {
> +ProtocolNotifyEvent = EfiCreateProtocolNotifyEvent (
> +,
> +TPL_CALLBACK,
> +OnPciEnumerationComplete,
> +NULL,
> +
> +);
> +ASSERT (ProtocolNotifyEvent != NULL);
> +  }
> 
>Status = EfiCreateEventReadyToBootEx (
>   TPL_CALLBACK,
> diff --git
> a/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf
> b/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf
> index f851f68..f28356d 100644
> --- a/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf
> +++ b/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf
> @@ -61,7 +61,8 @@
>gFspHobGuid   ## CONSUMES ## HOB
> 
>  [Pcd]
> -  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress  ## CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress   ##
> CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdNotifyPciEnumerationComplete  ##
> CONSUMES
> 
>  [Depex]
>TRUE
> diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> index d9d2d80..52ed9d8 100644
> --- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> +++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> @@ -74,6 +74,11 @@
>## This is the base address of FSP-T/M/S
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress|0x|UINT32|
> 0x0300
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress|0x|UINT32
> |0x0301
> +  ## Indicates if the FSP wrapper will notify FSP
> PciEnumerationComplete.
> +  #   TRUE  - FSP wrapper will notify PciEnumerationComplete to FSP.
> +  #   FALSE - FSP wrapper will NOT notify PciEnumerationComplete to FSP.
> +  # @Prompt Notify FSP PciEnumerationComplete.
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdNotifyPciEnumerationComplete|TRUE|
> BOOLEAN|0x4009
> 
>  [PcdsFixedAtBuild, PcdsPatchableInModule,PcdsDynamic,PcdsDynamicEx]
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress|0x|UINT32|
> 0x1001
> --
> 2.7.0.windows.1

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

[edk2] [PATCH] IntelSiliconPkg: Fixed bug in IgdOpregion spec

[Giri P Mudusuru]

Spec documents Mailbox3 - RM31 size as 0x45(69) instead of 0x46(70)

Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Giri P Mudusuru 
---
 IntelSiliconPkg/Include/IndustryStandard/IgdOpRegion.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/IntelSiliconPkg/Include/IndustryStandard/IgdOpRegion.h 
b/IntelSiliconPkg/Include/IndustryStandard/IgdOpRegion.h
index c66a452..fd6f813 100644
--- a/IntelSiliconPkg/Include/IndustryStandard/IgdOpRegion.h
+++ b/IntelSiliconPkg/Include/IndustryStandard/IgdOpRegion.h
@@ -4,6 +4,8 @@
 
   https://01.org/sites/default/files/documentation/skl_opregion_rev0p5.pdf
 
+  @note Fixed bug in the spec Mailbox3 - RM31 size from 0x45(69) to 0x46(70)
+
   Copyright (c) 2016, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
@@ -116,7 +118,7 @@ typedef struct {
   UINT64 FDSS;  ///< Offset 0x3AA DSS Buffer address allocated for 
IFFS feature
   UINT32 FDSP;  ///< Offset 0x3B2 Size of DSS buffer
   UINT32 STAT;  ///< Offset 0x3B6 State Indicator
-  UINT8  RM31[0x45];///< Offset 0x3BA - 0x3FF  Reserved Must be zero
+  UINT8  RM31[0x46];///< Offset 0x3BA - 0x3FF  Reserved Must be zero. Bug 
in spec 0x45(69)
 } IGD_OPREGION_MBOX3;
 
 ///
-- 
2.9.0.windows.1

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

Re: [edk2] [PATCH] ArmPkg: Fix the GIC base address variables as 64-bit

[Dennis Chen]

Hello Ard,

On Mon, Oct 17, 2016 at 06:09:00PM +0100, Ard Biesheuvel wrote:
> On 17 October 2016 at 09:54, Dennis Chen  wrote:
> > Since ACPI spec defines the GIC base addresses (CPU interface,
> > Distributor and Redistributor*GICv3 only*) as 64-bit, so we should
> > define these corresponding base address variables as 64-bit instead of
> > 32-bit. This patch redefines them according to the ACPI spec.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Cc: Ard Biesheuvel 
> > Cc: Leif Lindholm 
> > Signed-off-by: Dennis Chen 
> 
> After a closer look, I noticed the following:
> 
> ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  
> GicInterruptInterfaceBase
> ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicDistributorBase,
> ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  
> GicInterruptInterfaceBase
> ArmPkg/Include/Library/ArmGicLib.h:  IN  UINTN GicDistributorBase,
> ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  
> GicInterruptInterfaceBase
> ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  
> GicInterruptInterfaceBase
> ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicDistributorBase
> ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicDistributorBase
> ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicDistributorBase
> ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicDistributorBase,
> ArmPkg/Include/Library/ArmGicLib.h:  IN  UINTN
> GicInterruptInterfaceBase,
> ArmPkg/Include/Library/ArmGicLib.h:  IN  UINTN
> GicInterruptInterfaceBase,
> ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN
> GicInterruptInterfaceBase,
> ArmPkg/Include/Library/ArmGicLib.h:  IN UINTN
> GicDistributorBase,
> ArmPkg/Include/Library/ArmGicLib.h:  IN UINTN
> GicRedistributorBase,
> ArmPkg/Include/Library/ArmGicLib.h:  IN UINTN
> GicDistributorBase,
> ArmPkg/Include/Library/ArmGicLib.h:  IN UINTN
> GicRedistributorBase,
> ArmPkg/Include/Library/ArmGicLib.h:  IN UINTN
> GicDistributorBase,
> ArmPkg/Include/Library/ArmGicLib.h:  IN UINTN
> GicRedistributorBase,
> ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicDistributorBase,
> ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  
> GicInterruptInterfaceBase
> ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  
> GicInterruptInterfaceBase
> ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  
> GicInterruptInterfaceBase
> ArmPkg/Include/Library/ArmGicLib.h:  IN  UINTN
> GicInterruptInterfaceBase
> ArmPkg/Include/Library/ArmGicLib.h:  IN UINTN
> GicInterruptInterfaceBase,
> 
> so I think we need to clean up the use of these values more widely
> than we have done up till now
>
I am not very sure if we still need to support UEFI on 32-bit ARM platform, as 
Leif mentioned
if we use INTN or UINTN that will be more generic to embrace both 32 &64-bit 
platform, at least
in form of. Currently we are only focused on 64-bit platform, let's wait for 
Leif's comment then
I can re-work my patch to adapt it after we have reached a wider agreement.

Thanks,
Dennis 
> 
> Leif: I was wondering if EFI_PHYSICAL_ADDRESS would be more
> appropriate for MMIO base addresses?
> 

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

Re: [edk2] [Patch] MdeModulePkg: Support classless IP for DHCPv4 TransmitReceive()

[Fu, Siyuan]

Hi, Naveen

I checked the code and found the IP4 stack is actually doesn't support 
classless IP address now, the main reason is in the NetLib interface 
NetGetIpClass() and NetIp4IsUnicast(). These 2 interfaces do not consider the 
netmask so it won't recognize a classless IP configuration. Almost all other 
network drivers (IP4, ARP, iSCSI, Mtftp, PXE, TCP, UDP) are using these 2 
interfaces so I guess they may have the same problem.
Please help to submit a ticket in tianocore Bugzilla for this issue, thanks.


BestRegards
Fu Siyuan

From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
Santhapur Naveen
Sent: Friday, October 14, 2016 3:19 PM
To: Wu, Jiaxin ; edk2-devel@lists.01.org
Cc: Ye, Ting ; Fu, Siyuan 
Subject: Re: [edk2] [Patch] MdeModulePkg: Support classless IP for DHCPv4 
TransmitReceive()

Hello Jiaxin,

  We've run into one more problem with PXE boot. The PXE boot is not happening 
when the server tries to assign an IP address whose last octet is zero.

The below is my configuration:

[Server Configuration]
Ipv4 address : 192.168.0.1/16
Netmask: 255.255.0.0

DHCPv4 Scope:
Range: 192.168.0.2 to 192.168.10.10
Netmask: 255.255.0.0

  I've observed that in the given address range, if the server tries to 
allocate any IP address with the last octet as 0 i.e., for instance 192.168.A.0 
where A vary from 1 to 10, then the PXE boot fails saying "PXE-E09: Could not 
allocate I/O buffers."

  I agree that the x.y.z.0 and x.y.z.255 are network address any can't be 
assigned based on the subnet (In this case, 192.168.0.0 and 192.168.255.255). 
But here, the Netmask is different which expects the IP address x.y.a.0 which 
is within the range is valid and can be assigned to any client in the network.

  I captured Wireshark log and as per it, the D.O.R.A process is finished but 
the client is sending a Decline packet. I suspect the function 
NetIp4IsUnicast() has a role to play in this.

  Please provide your comments on this.

Best regards,
Naveen

-Original Message-
From: Santhapur Naveen
Sent: Friday, September 02, 2016 11:46 AM
To: 'Wu, Jiaxin'; 'edk2-devel@lists.01.org'
Cc: 'Ye, Ting'; 'Fu, Siyuan'
Subject: RE: [edk2] [Patch] MdeModulePkg: Support classless IP for DHCPv4 
TransmitReceive()

Hello Jiaxin,

  My sincere apologies for the delayed response.

  I've verified the patch from my side and PXE boot is happening successfully 
even in classless IP network.

  May I know whether this will be included in EDK2? If yes, can you please 
provide any schedule for the same?

Best regards,
Naveen

-Original Message-
From: Santhapur Naveen
Sent: Thursday, August 18, 2016 11:14 AM
To: 'Wu, Jiaxin'; edk2-devel@lists.01.org
Cc: Ye, Ting; Fu, Siyuan; Sivaraman Nainar; Madhan B. Santharam
Subject: RE: [edk2] [Patch] MdeModulePkg: Support classless IP for DHCPv4 
TransmitReceive()

Jiaxin,

We will verify the patch and update you the result.

Thanks,
Naveen

-Original Message-
From: Wu, Jiaxin [mailto:jiaxin...@intel.com]
Sent: Thursday, August 18, 2016 11:12 AM
To: Santhapur Naveen; Wu, Jiaxin; 
edk2-devel@lists.01.org
Cc: Ye, Ting; Fu, Siyuan; Sivaraman Nainar; Madhan B. Santharam
Subject: RE: [edk2] [Patch] MdeModulePkg: Support classless IP for DHCPv4 
TransmitReceive()

Naveen,

Can you help to verify this patch to support the classless IP.

Thanks,
Jiaxin

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Jiaxin Wu
> Sent: Thursday, August 18, 2016 1:39 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting >; Fu, Siyuan 
> >;
> Santhapur Naveen >
> Subject: [edk2] [Patch] MdeModulePkg: Support classless IP for DHCPv4
> TransmitReceive()
>
> The IP address should not be treated as classful one if DHCP options
> contain a classless IP with its true subnet mask. Otherwise, DHCPv4
> TransmitReceive() will failed. This real subnet mask will be parsed
> and recorded in DhcpSb->Netmask. So, we need check it before get the
> IP's corresponding subnet mask.
>
> Cc: Santhapur Naveen >
> Cc: Ye Ting >
> Cc: Fu Siyuan >
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu >
> ---
>  .../Universal/Network/Dhcp4Dxe/Dhcp4Impl.c | 28 +++-
> --
>  1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Impl.c
> b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Impl.c
> index 4f491b4..79f7cde 100644
> --- 

Re: [edk2] [Patch] MdePkg BaseSynchronizationLib: Update InterlockedCompareExchange64.nasm

[Fan, Jeff]

Reviewed-by: Jeff Fan 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Gao, 
Liming
Sent: Tuesday, October 18, 2016 9:36 AM
To: Laszlo Ersek; edk2-de...@ml01.01.org
Subject: Re: [edk2] [Patch] MdePkg BaseSynchronizationLib: Update 
InterlockedCompareExchange64.nasm

Thanks! I will use this subject. 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Laszlo Ersek
> Sent: Friday, October 14, 2016 8:22 PM
> To: Gao, Liming ; edk2-de...@ml01.01.org
> Subject: Re: [edk2] [Patch] MdePkg BaseSynchronizationLib: Update 
> InterlockedCompareExchange64.nasm
> 
> On 10/14/16 08:47, Liming Gao wrote:
> > Remove extra qword in nasm code to make it pass build.
> > This file is only built in INTEL ICC compiler. So, there is missing 
> > build check for it.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Liming Gao 
> > ---
> >  .../BaseSynchronizationLib/Ia32/InterlockedCompareExchange64.nasm
> | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git
> a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchan
> ge64.nasm
> b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchan
> ge64.nasm
> > index ee63ff7..206de40 100644
> > ---
> a/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchan
> ge64.nasm
> > +++
> b/MdePkg/Library/BaseSynchronizationLib/Ia32/InterlockedCompareExchan
> ge64.nasm
> > @@ -41,7 +41,7 @@ ASM_PFX(InternalSyncCompareExchange64):
> >  mov edx, [esp + 20]
> >  mov ebx, [esp + 24]
> >  mov ecx, [esp + 28]
> > -lockcmpxchg8b   qword [esi]
> > +lockcmpxchg8b [esi]
> >  pop ebx
> >  pop esi
> >  ret
> >
> 
> Suggested subject line:
> 
> MdePkg BaseSynchronizationLib InterlockedCompareExchange64: fix ICC 
> build
> 
> (73 characters)
> 
> Thanks,
> Laszlo
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 52/52] BaseTools/VfrCompile/Pccts: Make assignment operator not returning void

[Dong, Eric]

 Hi Hao,

I have below comments:

Case 1: In order to keep consistence, can you also change other cases which 
still initialized with '\0'

  mOptions.VfrFileName[0]= '\0';
  mOptions.RecordListFile= NULL;
  mOptions.CreateRecordListFile  = FALSE;
  mOptions.CreateIfrPkgFile  = FALSE;
  mOptions.PkgOutputFileName = NULL;
  mOptions.COutputFileName   = NULL;
  mOptions.OutputDirectory[0]= '\0';
  mOptions.PreprocessorOutputFileName= NULL;
  mOptions.VfrBaseFileName[0]= '\0';


Case 2: for below case, you can use the actual size for the strncpy instead of 
MAX_PATH - 1. Also the last line code is not needed.

  if (strlen (Argv[Index]) > MAX_PATH - 1) {
DebugError (NULL, 0, 1003, "Invalid option value", "Output directory 
name %s is too long", Argv[Index]);
goto Fail;
  }
  strncpy (mOptions.OutputDirectory, Argv[Index], MAX_PATH - 1);
  mOptions.OutputDirectory[MAX_PATH - 1] = 0;

Thanks,
Eric
> -Original Message-
> From: Wu, Hao A
> Sent: Wednesday, October 12, 2016 8:21 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A; Gao, Liming; Zhu, Yonghong; Dong, Eric; Bi, Dandan
> Subject: [PATCH 52/52] BaseTools/VfrCompile/Pccts: Make assignment operator 
> not returning void
> 
> The assignment operators for class ANTLRTokenPtr return void in current
> code.
> 
> This commit makes them return the reference to the object just like
> primitive types do.
> 
> Cc: Liming Gao 
> Cc: Yonghong Zhu 
> Cc: Eric Dong 
> Cc: Dandan Bi 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu 
> ---
>  BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h | 4 ++--
>  BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h | 6 --
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h 
> b/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h
> index 75b4c86..df89d8f 100644
> --- a/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h
> +++ b/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h
> @@ -58,8 +58,8 @@ public:
>  //  7-Apr-97 133MR1
>  //Fix suggested by Andreas Magnusson
>  //   (andreas.magnus...@mailbox.swipnet.se)
> -void operator = (const ANTLRTokenPtr & lhs); // MR1
> -void operator = (ANTLRAbstractToken *addr);
> +ANTLRTokenPtr& operator = (const ANTLRTokenPtr & lhs);  // MR1
> +ANTLRTokenPtr& operator = (ANTLRAbstractToken *addr);
>  int operator != (const ANTLRTokenPtr ) const   // MR1 
> // MR11 unsigned -> int
>   { return this->ptr_ != q.ptr_; }
>  int operator == (const ANTLRTokenPtr ) const   // MR1 
> // MR11 unsigned -> int
> diff --git a/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h 
> b/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h
> index 9c07cf5..a1efc3b 100644
> --- a/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h
> +++ b/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h
> @@ -71,18 +71,20 @@ ANTLRTokenPtr::~ANTLRTokenPtr()
>  //  8-Apr-97 MR1 Make operator -> a const member function
>  // as weall as some other member functions
>  //
> -void ANTLRTokenPtr::operator = (const ANTLRTokenPtr & lhs)   // MR1
> +ANTLRTokenPtr& ANTLRTokenPtr::operator = (const ANTLRTokenPtr & lhs)// 
> MR1
>  {
>  lhs.ref();   // protect against "xp = xp"; ie same underlying object
>  deref();
>  ptr_ = lhs.ptr_;
> +return *this;
>  }
> 
> -void ANTLRTokenPtr::operator = (ANTLRAbstractToken *addr)
> +ANTLRTokenPtr& ANTLRTokenPtr::operator = (ANTLRAbstractToken *addr)
>  {
>  if (addr != NULL) {
>   addr->ref();
>  }
>  deref();
>  ptr_ = addr;
> +return *this;
>  }
> --
> 1.9.5.msysgit.0

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

Re: [edk2] [Patch 2/3] BaseTools: Update PatchCheck to handle the two [] as prefix

[Zhu, Yonghong]

Thanks for the comment, when I push the patch I will do the update.

Best Regards,
Zhu Yonghong


-Original Message-
From: Justen, Jordan L 
Sent: Tuesday, October 18, 2016 12:49 AM
To: Zhu, Yonghong ; edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: Re: [Patch 2/3] BaseTools: Update PatchCheck to handle the two [] as 
prefix

A small (not too important) suggestion for the patch subject might be:

BaseTools/PatchCheck.py: Update to handle the two [] as prefix

On 2016-10-17 01:28:37, Yonghong Zhu wrote:
> The bug is that only remove the first [] when it does the char count, 
> however sometimes we use [edk2][patch] as prefix, this patch fix this bug.
> 
> Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=113
> 

Remove blank line after 'Fixes:'.

> Cc: Liming Gao 
> Cc: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Scripts/PatchCheck.py | 17 +
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/BaseTools/Scripts/PatchCheck.py 
> b/BaseTools/Scripts/PatchCheck.py index 07fca68..05f8f6e 100755
> --- a/BaseTools/Scripts/PatchCheck.py
> +++ b/BaseTools/Scripts/PatchCheck.py
> @@ -434,10 +434,18 @@ class CheckOnePatch:
> [\s\S\r\n]+
> )
> ''',
> re.IGNORECASE | re.VERBOSE | re.MULTILINE)
>  
> +subject_prefix_re = \
> +re.compile(r'''^
> +   \s* (\[
> +[^\[\]]* # Allow all non-brackets
> +   \])* \s*
> +   ''',
> +   re.VERBOSE)
> +
>  def find_patch_pieces(self):
>  if sys.version_info < (3, 0):
>  patch = self.patch.encode('ascii', 'ignore')
>  else:
>  patch = self.patch
> @@ -470,18 +478,11 @@ class CheckOnePatch:
>  self.stat = mo.group('stat')
>  self.commit_msg = mo.group('commit_message')
>  
>  self.commit_subject = pmail['subject'].replace('\r\n', '')
>  self.commit_subject = self.commit_subject.replace('\n', '')
> -
> -pfx_start = self.commit_subject.find('[')
> -if pfx_start >= 0:
> -pfx_end = self.commit_subject.find(']')
> -if pfx_end > pfx_start:
> -self.commit_prefix = self.commit_subject[pfx_start + 1 : 
> pfx_end]

Since we no longer set self.commit_prefix, and you remove the other references 
to it in the script?

Reviewed-by: Jordan Justen 

> -self.commit_subject = self.commit_subject[pfx_end + 1 
> :].lstrip()
> -
> +self.commit_subject = self.subject_prefix_re.sub('', 
> + self.commit_subject, 1)
>  
>  class CheckGitCommits:
>  """Reads patches from git based on the specified git revision range.
>  
>  The patches are read from git, and then checked.
> --
> 2.6.1.windows.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch 1/3] BaseTools: Update PatchCheck for max length of subject and message line

[Zhu, Yonghong]

I will remove the blank line after 'Fixes:' when I push this patch. thanks for 
the comment.

Best Regards,
Zhu Yonghong

-Original Message-
From: Justen, Jordan L 
Sent: Tuesday, October 18, 2016 12:40 AM
To: Zhu, Yonghong ; edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: Re: [edk2] [Patch 1/3] BaseTools: Update PatchCheck for max length of 
subject and message line

On 2016-10-17 01:28:36, Yonghong Zhu wrote:
> This patch update PatchCheck.py:
> 1. The subject line of the commit message should be < 72 characters.
> 2. The other lines of the commit message should be < 76 characters.
> 
> Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=113
> 

Can you remove the blank line after 'Fixes:'? The end of the commit message has 
various tags, like Fixes, Cc, Contributed-under, Signed-off-by, Reviewed-by, 
etc. They can be one tag per line and grouped together.

Patch 1 Reviewed-by: Jordan Justen 

> Cc: Liming Gao 
> Cc: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Scripts/PatchCheck.py | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/BaseTools/Scripts/PatchCheck.py 
> b/BaseTools/Scripts/PatchCheck.py index 455c130..07fca68 100755
> --- a/BaseTools/Scripts/PatchCheck.py
> +++ b/BaseTools/Scripts/PatchCheck.py
> @@ -1,9 +1,9 @@
>  ## @file
>  #  Check a patch for various format issues  # -#  Copyright (c) 2015, 
> Intel Corporation. All rights reserved.
> +#  Copyright (c) 2015 - 2016, Intel Corporation. All rights 
> +reserved.
>  #
>  #  This program and the accompanying materials are licensed and made  
> #  available under the terms and conditions of the BSD License which  
> #  accompanies this distribution. The full text of the license may be  
> #  found at http://opensource.org/licenses/bsd-license.php
> @@ -14,11 +14,11 @@
>  #
>  
>  from __future__ import print_function
>  
>  VersionNumber = '0.1'
> -__copyright__ = "Copyright (c) 2015, Intel Corporation  All rights reserved."
> +__copyright__ = "Copyright (c) 2015 - 2016, Intel Corporation  All rights 
> reserved."
>  
>  import email
>  import argparse
>  import os
>  import re
> @@ -195,11 +195,11 @@ class CommitMessageCheck:
>  
>  if count <= 0:
>  self.error('Empty commit message!')
>  return
>  
> -if count >= 1 and len(lines[0]) > 76:
> +if count >= 1 and len(lines[0]) >= 72:
>  self.error('First line of commit message (subject line) ' +
> 'is too long.')
>  
>  if count >= 1 and len(lines[0].strip()) == 0:
>  self.error('First line of commit message (subject line) ' 
> + @@ -208,11 +208,11 @@ class CommitMessageCheck:
>  if count >= 2 and lines[1].strip() != '':
>  self.error('Second line of commit message should be ' +
> 'empty.')
>  
>  for i in range(2, count):
> -if (len(lines[i]) > 76 and
> +if (len(lines[i]) >= 76 and
>  len(lines[i].split()) > 1 and
>  not lines[i].startswith('git-svn-id:')):
>  self.error('Line %d of commit message is too long.' % 
> (i + 1))
>  
>  last_sig_line = None
> --
> 2.6.1.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] OvmfPkg: hang in SetInterruptState with git 245cda6641ade1f1013c2d5c9c838f2706636828

[Bruce Cran]

I've just built both OVMF _and_ Qemu from the latest git sources, so I 
don't know which is at fault - but I'm seeing a hang in:


#0  0x7f9dc030 in SetInterruptState (InterruptState=104 'h')
at /home/bcran/workspace/edk2/MdePkg/Library/BaseLib/Cpu.c:60

It's at line 60 when it calls EnableInterrupts().

The entire backtrace is:

#0  0x7f9dc030 in SetInterruptState (InterruptState=104 'h')
at /home/bcran/workspace/edk2/MdePkg/Library/BaseLib/Cpu.c:60
#1  0x7f9d6c57 in UpdateIdtTable (IdtTable=0xd, 
TemplateMap=0x7fadd478, ExceptionHandlerData=0x7f9e2460)
at 
/home/bcran/workspace/edk2/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c:146

#2  0x7f9d5ff0 in InitializeCpuInterruptHandlers (VectorInfo=0xd)
at 
/home/bcran/workspace/edk2/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c:111

#3  0x7f9d65fd in HasErrorCode ()
#4  0x000d in ?? ()
#5  0x7fadd478 in ?? ()
#6  0x7fa7b748 in ?? ()
#7  0x0012 in ?? ()
#8  0x7fadd4a0 in ?? ()
#9  0x in ?? ()


I _have_ tried going back to older revisions of qemu so I'm wondering if 
this could be a problem introduced by OVMF?


--
Bruce

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

Re: [edk2] [PATCH] GPT Shell Application/Library

[Carsey, Jaben]

To the old question about license: I asked our people to check and was told 
that the license is compatible with our BSD and ok by Intel.

To the technical content – I really like this idea of a shared library.  That 
would be a great way to share code and not have as much duplicate.

-Jaben

From: Vladimir Olovyannikov [mailto:vladimir.olovyanni...@broadcom.com]
Sent: Monday, October 17, 2016 10:52 AM
To: Michael Zimmermann 
Cc: Laszlo Ersek ; Carsey, Jaben ; 
Ni, Ruiyu ; edk2-devel@lists.01.org 
Subject: RE: [edk2] [PATCH] GPT Shell Application/Library
Importance: High

Hi Michael,
I am absolutely agree with your proposal.

In the gpt Shell library/application I had to “borrow” some stuff from 
PartitionDxe.c to not reinvent a  wheel.
If the PartitionDxe maintainer agrees to have a separate library available for 
everybody,
I would move all the GPT-related stuff from the GptWorker (and partially from 
the PartitionDxe itself) to that independent library.
This could be a longer-term task.
Right now I just wanted to share the tool which could be useful for anybody who 
would wish to manage
GPT partitions (and/or do a FAT32 format of either a disk or a GPT partition) 
from within the Shell. What do you think?

Thank you,
Vladimir
From: Michael Zimmermann 
[mailto:sigmaepsilo...@gmail.com]
Sent: October-17-16 12:25 AM
To: Vladimir Olovyannikov
Cc: Laszlo Ersek; Jaben Carsey; Ni, Ruiyu; 
edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] GPT Shell Application/Library

Hi,

wouldn't it be better to make a generic gpt parsing library which is 
independent of the shell so both the shell and PartitionDxe can use it?
It may also be useful for other applications which need additional information 
like the gpt partition names.

Thanks
Michael

On Mon, Oct 17, 2016 at 8:49 AM, Vladimir Olovyannikov 
> 
wrote:
Thank you Laszlo,

Sorry, I missed the fields; it is my first contribution, I will add the
required lines, review the source according to your comments and will
resubmit the patch.
So do you think the command should be _gpt instead of gpt? I was following
TFTP and SF commands as a template.

Thank you,
Vladimir

On Oct 16, 2016 1:05 PM, "Laszlo Ersek" 
> wrote:
>
> On 10/16/16 07:23, Vladimir Olovyannikov wrote:
> > This allows managing (create, delete, modify, fat format) of GPT
> > partitions from within UEFI Shell.
> > Syntax:
> > gpt  [device_mapped_name] [parameters...]
> > See usage examples in the .uni file
> > ---
> >  .../Library/UefiShellGptCommandLib/FatFormat.c |  611 +++
> >  .../Library/UefiShellGptCommandLib/FatFormat.h |  111 ++
> >  .../Library/UefiShellGptCommandLib/GptWorker.c | 1902

> >  .../Library/UefiShellGptCommandLib/GptWorker.h |  186 ++
> >  .../UefiShellGptCommandLib.c   | 1135 
> >  .../UefiShellGptCommandLib.inf |   79 +
> >  .../UefiShellGptCommandLib.uni |  117 ++
> >  ShellPkg/ShellPkg.dec  |1 +
> >  ShellPkg/ShellPkg.dsc  |4 +
> >  9 files changed, 4146 insertions(+)
> >  create mode 100644 ShellPkg/Library/UefiShellGptCommandLib/FatFormat.c
> >  create mode 100644 ShellPkg/Library/UefiShellGptCommandLib/FatFormat.h
> >  create mode 100644 ShellPkg/Library/UefiShellGptCommandLib/GptWorker.c
> >  create mode 100644 ShellPkg/Library/UefiShellGptCommandLib/GptWorker.h
> >  create mode 100644
ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.c
> >  create mode 100644
ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.inf
> >  create mode 100644
ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.uni
>
> This looks like a supremely welcome, long-awaited addition (latest
> request:
> ),
> but it really needs your Signed-off-by, and the Contributed-under line
> above it:
>
> ShellPkg/Contributions.txt
>
> I would also suggest (simply based on what I've seen elsewhere in edk2)
> to keep the copyright notices tightly collected in the file headings.
>
> Someone will have to go over all the licenses too -- does the "Marvell
> BSD License Option" for example correspond to the 3-clause BSDL?
>
> On the technical side, I believe that as long as a shell command (or a
> command option) is not standardized (in the shell spec), it usually
> starts with an underscore (_), so as to prevent future name collisions.
> (I could be wrong about this -- I now recall the TFTP command, which is
> also not in the 2.2 spec.)
>
> Just my two cents.
>
> Thanks
> Laszlo
___
edk2-devel mailing list

Re: [edk2] [PATCH] GPT Shell Application/Library

[Vladimir Olovyannikov]

Hi Michael,

I am absolutely agree with your proposal.



In the gpt Shell library/application I had to “borrow” some stuff from
PartitionDxe.c to not reinvent a  wheel.

If the PartitionDxe maintainer agrees to have a separate library available
for everybody,
I would move all the GPT-related stuff from the GptWorker (and partially
from the PartitionDxe itself) to that independent library.
This could be a longer-term task.

Right now I just wanted to share the tool which could be useful for anybody
who would wish to manage
GPT partitions (and/or do a FAT32 format of either a disk or a GPT
partition) from within the Shell. What do you think?



Thank you,

Vladimir

*From:* Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
*Sent:* October-17-16 12:25 AM
*To:* Vladimir Olovyannikov
*Cc:* Laszlo Ersek; Jaben Carsey; Ni, Ruiyu; edk2-devel@lists.01.org
*Subject:* Re: [edk2] [PATCH] GPT Shell Application/Library



Hi,



wouldn't it be better to make a generic gpt parsing library which is
independent of the shell so both the shell and PartitionDxe can use it?

It may also be useful for other applications which need additional
information like the gpt partition names.



Thanks

Michael



On Mon, Oct 17, 2016 at 8:49 AM, Vladimir Olovyannikov <
vladimir.olovyanni...@broadcom.com> wrote:

Thank you Laszlo,

Sorry, I missed the fields; it is my first contribution, I will add the
required lines, review the source according to your comments and will
resubmit the patch.
So do you think the command should be _gpt instead of gpt? I was following
TFTP and SF commands as a template.

Thank you,
Vladimir


On Oct 16, 2016 1:05 PM, "Laszlo Ersek"  wrote:
>
> On 10/16/16 07:23, Vladimir Olovyannikov wrote:
> > This allows managing (create, delete, modify, fat format) of GPT
> > partitions from within UEFI Shell.
> > Syntax:
> > gpt  [device_mapped_name] [parameters...]
> > See usage examples in the .uni file
> > ---
> >  .../Library/UefiShellGptCommandLib/FatFormat.c |  611 +++
> >  .../Library/UefiShellGptCommandLib/FatFormat.h |  111 ++
> >  .../Library/UefiShellGptCommandLib/GptWorker.c | 1902

> >  .../Library/UefiShellGptCommandLib/GptWorker.h |  186 ++
> >  .../UefiShellGptCommandLib.c   | 1135 
> >  .../UefiShellGptCommandLib.inf |   79 +
> >  .../UefiShellGptCommandLib.uni |  117 ++
> >  ShellPkg/ShellPkg.dec  |1 +
> >  ShellPkg/ShellPkg.dsc  |4 +
> >  9 files changed, 4146 insertions(+)
> >  create mode 100644 ShellPkg/Library/UefiShellGptCommandLib/FatFormat.c
> >  create mode 100644 ShellPkg/Library/UefiShellGptCommandLib/FatFormat.h
> >  create mode 100644 ShellPkg/Library/UefiShellGptCommandLib/GptWorker.c
> >  create mode 100644 ShellPkg/Library/UefiShellGptCommandLib/GptWorker.h
> >  create mode 100644
ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.c
> >  create mode 100644
ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.inf
> >  create mode 100644
ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.uni
>
> This looks like a supremely welcome, long-awaited addition (latest
> request:
> ),
> but it really needs your Signed-off-by, and the Contributed-under line
> above it:
>
> ShellPkg/Contributions.txt
>
> I would also suggest (simply based on what I've seen elsewhere in edk2)
> to keep the copyright notices tightly collected in the file headings.
>
> Someone will have to go over all the licenses too -- does the "Marvell
> BSD License Option" for example correspond to the 3-clause BSDL?
>
> On the technical side, I believe that as long as a shell command (or a
> command option) is not standardized (in the shell spec), it usually
> starts with an underscore (_), so as to prevent future name collisions.
> (I could be wrong about this -- I now recall the TFTP command, which is
> also not in the 2.2 spec.)
>
> Just my two cents.
>
> Thanks
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] CorebootPayloadPkg/PciHostBridgeLib: Fix the wrong PCI resource limit

[Maurice Ma]

The current PCI resource limit calculation in CorebootPayloadPkg
PciHostBridgeLib is wrong. Adjusted it to match the PciHostBridge
driver's expectation.

Cc: Prince Agyeman 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Maurice Ma 
---
 CorebootPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/CorebootPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c 
b/CorebootPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
index 0f1c8cb1a210..6d94ff72c956 100644
--- a/CorebootPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
+++ b/CorebootPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
@@ -91,7 +91,7 @@ AdjustRootBridgeResource (
   // Align IO  resource at 4K  boundary
   //
   Mask= 0xFFFULL;
-  Io->Limit   = (Io->Limit + Mask) & ~Mask;
+  Io->Limit   = ((Io->Limit + Mask) & ~Mask) - 1;
   if (Io->Base != MAX_UINT64) {
 Io->Base &= ~Mask;
   }
@@ -100,7 +100,7 @@ AdjustRootBridgeResource (
   // Align MEM resource at 1MB boundary
   //
   Mask= 0xFULL;
-  Mem->Limit  = (Mem->Limit + Mask) & ~Mask;
+  Mem->Limit  = ((Mem->Limit + Mask) & ~Mask) - 1;
   if (Mem->Base != MAX_UINT64) {
 Mem->Base &= ~Mask;
   }
-- 
1.9.5.msysgit.0

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

Re: [edk2] [PATCH] ArmPkg: Fix the GIC base address variables as 64-bit

[Ard Biesheuvel]

On 17 October 2016 at 09:54, Dennis Chen  wrote:
> Since ACPI spec defines the GIC base addresses (CPU interface,
> Distributor and Redistributor*GICv3 only*) as 64-bit, so we should
> define these corresponding base address variables as 64-bit instead of
> 32-bit. This patch redefines them according to the ACPI spec.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Cc: Ard Biesheuvel 
> Cc: Leif Lindholm 
> Signed-off-by: Dennis Chen 

After a closer look, I noticed the following:

ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicInterruptInterfaceBase
ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicDistributorBase,
ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicInterruptInterfaceBase
ArmPkg/Include/Library/ArmGicLib.h:  IN  UINTN GicDistributorBase,
ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicInterruptInterfaceBase
ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicInterruptInterfaceBase
ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicDistributorBase
ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicDistributorBase
ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicDistributorBase
ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicDistributorBase,
ArmPkg/Include/Library/ArmGicLib.h:  IN  UINTN
GicInterruptInterfaceBase,
ArmPkg/Include/Library/ArmGicLib.h:  IN  UINTN
GicInterruptInterfaceBase,
ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN
GicInterruptInterfaceBase,
ArmPkg/Include/Library/ArmGicLib.h:  IN UINTN
GicDistributorBase,
ArmPkg/Include/Library/ArmGicLib.h:  IN UINTN
GicRedistributorBase,
ArmPkg/Include/Library/ArmGicLib.h:  IN UINTN
GicDistributorBase,
ArmPkg/Include/Library/ArmGicLib.h:  IN UINTN
GicRedistributorBase,
ArmPkg/Include/Library/ArmGicLib.h:  IN UINTN
GicDistributorBase,
ArmPkg/Include/Library/ArmGicLib.h:  IN UINTN
GicRedistributorBase,
ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicDistributorBase,
ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicInterruptInterfaceBase
ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicInterruptInterfaceBase
ArmPkg/Include/Library/ArmGicLib.h:  IN  INTN  GicInterruptInterfaceBase
ArmPkg/Include/Library/ArmGicLib.h:  IN  UINTN
GicInterruptInterfaceBase
ArmPkg/Include/Library/ArmGicLib.h:  IN UINTN
GicInterruptInterfaceBase,

so I think we need to clean up the use of these values more widely
than we have done up till now

Leif: I was wondering if EFI_PHYSICAL_ADDRESS would be more
appropriate for MMIO base addresses?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Force reconnect children from within a DXE driver

[Laszlo Ersek]

On 10/17/16 18:05, Marcin Wojtas wrote:
> Hi,
> 
> Short introduction to the problem I'm facing with EDK2: I implemented
> 'ifconfig' command extension allowing to change interface's MAC
> address or reset it to the default value. For that I needed to add a
> couple of helper functions in DxeNetLib, which parse input string and
> one that calls Snp->StationAddress callback from the NIC driver.
> 
> The problem is that the drivers associated to interface device
> comprise a copy of SnpMode, not original pointer. It causes an obvious
> mismatch, because only the latter got updated with new
> Snp->Mode->CurrentAddress. I found out that using 'reconnect' command
> after MAC address change from uefi shell helps and the network can
> operate properly.
> 
> Hence I have a question - is there a way to force reconnecting all
> children drivers from the network controller driver level? I.e. it
> would be great if it was possible not to be forced to call 'ifconfig'
> command and then 'reconnect', but call some function(s) e.g. from
> Snp->StationAddress callback.

I would try this:
- From the shell command (which is running in the shell application),
open the SNP interface in exclusive mode. This should either fail (if
there's already an exclusive open on the interface), or succeed and
force other drivers to disconnect, recursively.
- Call Snp->StationAddress to change the MAC.
- Close the protocol interface.
- Call gBS->ConnectController() to (re)connect all the drivers,
recursively, to the handle with the SNP on it.

Thanks
Laszlo

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

Re: [edk2] [Patch 3/3] BaseTools: Update PatchCheck to report error for EFI_D_*

[Jordan Justen]

On 2016-10-17 01:28:38, Yonghong Zhu wrote:
> In EDK2, DEBUG_* is recommended to be used instead of EFI_D_*. For new
> code, they should use DEBUG_* macro.
> 
> Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=145
>

Remove blank line. I think you meant 143, not 145.

Instead of looking for DEBUG and EFI_D_, I think we should just look
for EFI_D_. It is possible that the user might put the EFI_D_ on a
separate line. What do you think?

Should we add 'warning' support to the script first, and then flag
this as a warning rather than an error? (I don't think this is too
important, because getting an error from the script will not currently
prevent the patch from being merged.)

-Jordan

> Cc: Liming Gao 
> Cc: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Scripts/PatchCheck.py | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
> index 05f8f6e..3ac0769 100755
> --- a/BaseTools/Scripts/PatchCheck.py
> +++ b/BaseTools/Scripts/PatchCheck.py
> @@ -337,10 +337,15 @@ class GitDiffCheck:
>  if self.hunk_filename is not None:
>  lines.append('File: ' + self.hunk_filename)
>  lines.append('Line: ' + line)
>  
>  self.error(*lines)
> +
> +DEBUG_macro_re = re.compile(r'''^
> +\s*DEBUG\s*\(\(
> +''',
> +re.VERBOSE)
>  
>  def check_added_line(self, line):
>  eol = ''
>  for an_eol in self.line_endings:
>  if line.endswith(an_eol):
> @@ -354,10 +359,13 @@ class GitDiffCheck:
>line)
>  if '' in line:
>  self.added_line_error('Tab character used', line)
>  if len(stripped) < len(line):
>  self.added_line_error('Trailing whitespace found', line)
> +if self.DEBUG_macro_re.match(line):
> +if 'EFI_D_' in line:
> +self.added_line_error('EFI_D_* format is used, recommend to 
> use DEBUG_* format', line)
>  
>  split_diff_re = re.compile(r'''
> (?P
> ^ diff \s+ --git \s+ a/.+ \s+ b/.+ $
> )
> -- 
> 2.6.1.windows.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch 2/3] BaseTools: Update PatchCheck to handle the two [] as prefix

[Jordan Justen]

A small (not too important) suggestion for the patch subject might be:

BaseTools/PatchCheck.py: Update to handle the two [] as prefix

On 2016-10-17 01:28:37, Yonghong Zhu wrote:
> The bug is that only remove the first [] when it does the char count,
> however sometimes we use [edk2][patch] as prefix, this patch fix this bug.
> 
> Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=113
> 

Remove blank line after 'Fixes:'.

> Cc: Liming Gao 
> Cc: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Scripts/PatchCheck.py | 17 +
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
> index 07fca68..05f8f6e 100755
> --- a/BaseTools/Scripts/PatchCheck.py
> +++ b/BaseTools/Scripts/PatchCheck.py
> @@ -434,10 +434,18 @@ class CheckOnePatch:
> [\s\S\r\n]+
> )
> ''',
> re.IGNORECASE | re.VERBOSE | re.MULTILINE)
>  
> +subject_prefix_re = \
> +re.compile(r'''^
> +   \s* (\[
> +[^\[\]]* # Allow all non-brackets
> +   \])* \s*
> +   ''',
> +   re.VERBOSE)
> +
>  def find_patch_pieces(self):
>  if sys.version_info < (3, 0):
>  patch = self.patch.encode('ascii', 'ignore')
>  else:
>  patch = self.patch
> @@ -470,18 +478,11 @@ class CheckOnePatch:
>  self.stat = mo.group('stat')
>  self.commit_msg = mo.group('commit_message')
>  
>  self.commit_subject = pmail['subject'].replace('\r\n', '')
>  self.commit_subject = self.commit_subject.replace('\n', '')
> -
> -pfx_start = self.commit_subject.find('[')
> -if pfx_start >= 0:
> -pfx_end = self.commit_subject.find(']')
> -if pfx_end > pfx_start:
> -self.commit_prefix = self.commit_subject[pfx_start + 1 : 
> pfx_end]

Since we no longer set self.commit_prefix, and you remove the other
references to it in the script?

Reviewed-by: Jordan Justen 

> -self.commit_subject = self.commit_subject[pfx_end + 1 
> :].lstrip()
> -
> +self.commit_subject = self.subject_prefix_re.sub('', 
> self.commit_subject, 1)
>  
>  class CheckGitCommits:
>  """Reads patches from git based on the specified git revision range.
>  
>  The patches are read from git, and then checked.
> -- 
> 2.6.1.windows.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch 1/3] BaseTools: Update PatchCheck for max length of subject and message line

[Jordan Justen]

On 2016-10-17 01:28:36, Yonghong Zhu wrote:
> This patch update PatchCheck.py:
> 1. The subject line of the commit message should be < 72 characters.
> 2. The other lines of the commit message should be < 76 characters.
> 
> Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=113
> 

Can you remove the blank line after 'Fixes:'? The end of the commit
message has various tags, like Fixes, Cc, Contributed-under,
Signed-off-by, Reviewed-by, etc. They can be one tag per line and
grouped together.

Patch 1 Reviewed-by: Jordan Justen 

> Cc: Liming Gao 
> Cc: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Scripts/PatchCheck.py | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
> index 455c130..07fca68 100755
> --- a/BaseTools/Scripts/PatchCheck.py
> +++ b/BaseTools/Scripts/PatchCheck.py
> @@ -1,9 +1,9 @@
>  ## @file
>  #  Check a patch for various format issues
>  #
> -#  Copyright (c) 2015, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
>  #
>  #  This program and the accompanying materials are licensed and made
>  #  available under the terms and conditions of the BSD License which
>  #  accompanies this distribution. The full text of the license may be
>  #  found at http://opensource.org/licenses/bsd-license.php
> @@ -14,11 +14,11 @@
>  #
>  
>  from __future__ import print_function
>  
>  VersionNumber = '0.1'
> -__copyright__ = "Copyright (c) 2015, Intel Corporation  All rights reserved."
> +__copyright__ = "Copyright (c) 2015 - 2016, Intel Corporation  All rights 
> reserved."
>  
>  import email
>  import argparse
>  import os
>  import re
> @@ -195,11 +195,11 @@ class CommitMessageCheck:
>  
>  if count <= 0:
>  self.error('Empty commit message!')
>  return
>  
> -if count >= 1 and len(lines[0]) > 76:
> +if count >= 1 and len(lines[0]) >= 72:
>  self.error('First line of commit message (subject line) ' +
> 'is too long.')
>  
>  if count >= 1 and len(lines[0].strip()) == 0:
>  self.error('First line of commit message (subject line) ' +
> @@ -208,11 +208,11 @@ class CommitMessageCheck:
>  if count >= 2 and lines[1].strip() != '':
>  self.error('Second line of commit message should be ' +
> 'empty.')
>  
>  for i in range(2, count):
> -if (len(lines[i]) > 76 and
> +if (len(lines[i]) >= 76 and
>  len(lines[i].split()) > 1 and
>  not lines[i].startswith('git-svn-id:')):
>  self.error('Line %d of commit message is too long.' % (i + 
> 1))
>  
>  last_sig_line = None
> -- 
> 2.6.1.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] GPT Shell Application/Library

[Carsey, Jaben]

Laszlo,

We don't have a hard and fast rule about command names.  The simple thing is 
that we may have a conflict in the future, but I don't see that as likely at 
this point.  

Your comment about _ was when a custom implementation wants to extend an 
existing defined command with a non-standard parameter. For example there is an 
_exit parameter to the shell that tells the shell to auto-close after running 
the initial command line passed into it.

-Jaben

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Monday, October 17, 2016 2:44 AM
> To: Vladimir Olovyannikov 
> Cc: Carsey, Jaben ; Ni, Ruiyu
> ; edk2-devel@lists.01.org 
> Subject: Re: [edk2] [PATCH] GPT Shell Application/Library
> Importance: High
> 
> On 10/17/16 08:49, Vladimir Olovyannikov wrote:
> > Thank you Laszlo,
> >
> > Sorry, I missed the fields; it is my first contribution, I will add the
> > required lines, review the source according to your comments and will
> > resubmit the patch.
> > So do you think the command should be _gpt instead of gpt? I was
> > following TFTP and SF commands as a template.
> 
> Right, at this point I'm confused about TFTP even, in retrospect. It's
> not in the spec, so I guess it should have been _TFTP. I hope Jaben and
> Ray can advise us. :)
> 
> Thanks
> Laszlo
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg

[Carsey, Jaben]

Yao,

How many closed source packages are going to be working against the trunk?  
Don't most of those platforms already have a copy of ShellPkg and therefore 
they wont care?

-Jaben

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Yao, Jiewen
> Sent: Saturday, October 15, 2016 6:30 AM
> To: Tim Lewis ; Ni, Ruiyu ;
> edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg
> Importance: High
> 
> Of course, it is fine to give a good name in MdePkg. :)
> 
> My concern is only about "Compatibility".
> This series patch only updated ArmPkg and EmbeddedPkg.
> But there might be more close source IP protected packages using the
> original header file.
> 
> That is why I propose a compatible way: We give a good file name in MdePkg.
> At same time, we can keep the old file name in ShellPkg to include the good
> name in MdePkg.
> 
> I recommend we give a solution to make close source package unchanged
> after this patch is applied.
> 
> Thank you
> Yao Jiewen
> 
> 
> From: Tim Lewis [mailto:tim.le...@insyde.com]
> Sent: Saturday, October 15, 2016 12:21 AM
> To: Yao, Jiewen ; Ni, Ruiyu ;
> edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg
> 
> I prefer the renamed .h file, even though I have substantial investment in
> the current infrastructure.
> 
> Why? Because engineers don't have time to remember "how does protocol
> X translate to header file Y" It should be a consistent rule. How many times
> have I needed to grep the header file name just because I got a build error
> because the rule wasn't predictable.
> 
> 1) remove the EFI_ prefix and the _PROTOCOL  suffix
> (EFI_BLOCK_IO_PROTOCOL -> BLOCK_IO
> 2) Convert to upper-and-lower case, BLOCK_IO -> Block_Io
> 3) Remove the _ (Block_Io -> BlockIo)
> 4)  add a .h BlockIo -> BlockIo.h
> 
> Most protocol and PPI header files already follow this rule. The current
> header files for shell are among the few exceptions, because they don't
> follow this.
> 
> Tim
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Yao, Jiewen
> Sent: Friday, October 14, 2016 6:14 AM
> To: Yao, Jiewen >;
> Ni, Ruiyu >; edk2-
> de...@lists.01.org
> Subject: Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg
> 
> Or if you really think we should give a better name.
> 
> My recommendation is:
> 
> 1)  We add content in MdePkg.
> 
> 2)  We can keep the old protocol file in ShellPkg, but let .h in shellPkg
> include the .h in MdePkg.
> 
> Then we can avoid duplicated code and make it a compatible solution to
> avoid other module update.
> 
> Thank you
> Yao Jiewen
> 
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Yao, Jiewen
> Sent: Friday, October 14, 2016 9:09 PM
> To: Ni, Ruiyu >; edk2-
> de...@lists.01.org
> Subject: Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg
> 
> Hi
> I think the requests is just to *move*.
> 
> There is no request to *rename*.
> 
> Can we just move to avoid other update?
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Ruiyu Ni
> > Sent: Friday, October 14, 2016 5:44 PM
> > To: edk2-devel@lists.01.org de...@lists.01.org%3cmailto:edk2-devel@lists.01.org>>
> > Subject: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg
> >
> > The patches moves Shell spec defined protocol definitions to MdePkg
> > and updates all references.
> > Content of ShellBase.h is moved to Protocol/Shell.h and ShellBase.h is
> > removed.
> >
> > Ruiyu Ni (5):
> >   ShellPkg: Move SHELL_FREE_NON_NULL from ShellBase.h to ShellLib.h
> >   MdePkg: Include Shell/ShellDynamicCommand/ShellParameters
> > definitions
> >   ArmPkg/LinuxLoader: Reference Shell protocols in MdePkg
> >   EmbeddedPkg/FdtPlatformDxe: Reference Shell protocols in MdePkg
> >   ShellPkg: Remove Shell/ShellDynamicCommand/ShellParameter
> > definitions
> >
> >  ArmPkg/Application/LinuxLoader/LinuxLoader.h   |   4 +-
> >  EmbeddedPkg/Drivers/FdtPlatformDxe/FdtPlatform.h   |   4 +-
> >  .../EfiShell.h => MdePkg/Include/Protocol/Shell.h  | 134
> > +-
> >  .../Include/Protocol/ShellDynamicCommand.h |   7 +-
> >  .../Include/Protocol/ShellParameters.h |   4 +-
> >  MdePkg/MdePkg.dec  |  15 ++
> >  ShellPkg/Application/Shell/Shell.h |   5 +-
> >  ShellPkg/Include/Library/ShellCommandLib.h |   5 +-
> >  ShellPkg/Include/Library/ShellLib.h|  14 +-
> >  

Re: [edk2] [PATCH 1/5] ShellPkg: Move SHELL_FREE_NON_NULL from ShellBase.h to ShellLib.h

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ruiyu Ni
> Sent: Friday, October 14, 2016 2:44 AM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben ; Kinney, Michael D
> ; Yao, Jiewen 
> Subject: [edk2] [PATCH 1/5] ShellPkg: Move SHELL_FREE_NON_NULL from
> ShellBase.h to ShellLib.h
> Importance: High
> 
> The more proper place for macro SHELL_FREE_NON_NULL is ShellLib.h
> instead of ShellBase.h.
> 
> Modify Compress.c to resolve build failure due to this change.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Jaben Carsey 
> Cc: Jiewen Yao 
> Cc: Michael D Kinney 
> ---
>  ShellPkg/Include/Library/ShellLib.h| 10 +-
>  ShellPkg/Include/ShellBase.h   | 10 +-
>  ShellPkg/Library/UefiShellDebug1CommandsLib/Compress.c |  8 
>  3 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/ShellPkg/Include/Library/ShellLib.h
> b/ShellPkg/Include/Library/ShellLib.h
> index fe4b9cf..fafa041 100644
> --- a/ShellPkg/Include/Library/ShellLib.h
> +++ b/ShellPkg/Include/Library/ShellLib.h
> @@ -1,7 +1,7 @@
>  /** @file
>Provides interface to shell functionality for shell commands and
> applications.
> 
> -  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
> +  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
>which accompanies this distribution.  The full text of the license may be
> found at
> @@ -24,6 +24,14 @@
>  #include 
>  #include 
> 
> +#define SHELL_FREE_NON_NULL(Pointer)  \
> +  do {\
> +if ((Pointer) != NULL) {  \
> +  FreePool((Pointer));\
> +  (Pointer) = NULL;   \
> +} \
> +  } while(FALSE)
> +
>  // (20 * (6+5+2))+1) unicode characters from EFI FAT spec (doubled for
> bytes)
>  #define MAX_FILE_NAME_LEN 512
> 
> diff --git a/ShellPkg/Include/ShellBase.h b/ShellPkg/Include/ShellBase.h
> index 09f87b4..4b7a3d1 100644
> --- a/ShellPkg/Include/ShellBase.h
> +++ b/ShellPkg/Include/ShellBase.h
> @@ -1,7 +1,7 @@
>  /** @file
>Root include file for Shell Package modules that utilize the SHELL_RETURN
> type
> 
> -  Copyright (c) 2009 - 2011, Intel Corporation. All rights reserved.
> +  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
>which accompanies this distribution.  The full text of the license may be
> found at
> @@ -17,14 +17,6 @@
> 
>  typedef VOID *SHELL_FILE_HANDLE;
> 
> -#define SHELL_FREE_NON_NULL(Pointer)  \
> -  do {\
> -if ((Pointer) != NULL) {  \
> -  FreePool((Pointer));\
> -  (Pointer) = NULL;   \
> -} \
> -  } while(FALSE)
> -
>  typedef enum {
>  ///
>  /// The operation completed successfully.
> diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Compress.c
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/Compress.c
> index dda2fed..da8e647 100644
> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Compress.c
> +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Compress.c
> @@ -7,7 +7,7 @@
>This sequence is further divided into Blocks and Huffman codings
>are applied to each Block.
> 
> -  Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.
> +  Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
>which accompanies this distribution.  The full text of the license may be
> found at
> @@ -17,12 +17,12 @@
>WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> 
>  **/
> -
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> -#include 
> -#include 
> +#include 
> 
>  //
>  // Macro Definitions
> --
> 2.9.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] Force reconnect children from within a DXE driver

[Marcin Wojtas]

Hi,

Short introduction to the problem I'm facing with EDK2: I implemented
'ifconfig' command extension allowing to change interface's MAC
address or reset it to the default value. For that I needed to add a
couple of helper functions in DxeNetLib, which parse input string and
one that calls Snp->StationAddress callback from the NIC driver.

The problem is that the drivers associated to interface device
comprise a copy of SnpMode, not original pointer. It causes an obvious
mismatch, because only the latter got updated with new
Snp->Mode->CurrentAddress. I found out that using 'reconnect' command
after MAC address change from uefi shell helps and the network can
operate properly.

Hence I have a question - is there a way to force reconnecting all
children drivers from the network controller driver level? I.e. it
would be great if it was possible not to be forced to call 'ifconfig'
command and then 'reconnect', but call some function(s) e.g. from
Snp->StationAddress callback.

I would appreciate any help.

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

Re: [edk2] [RESEND PATCH 1/1] ArmPlatformPkg/ArmTrustZone: Add support for specifying Subregions to be disabled

[Ard Biesheuvel]

On 17 October 2016 at 14:25, Leif Lindholm  wrote:
> On Mon, Oct 17, 2016 at 10:18:01AM +, Bhupesh Sharma wrote:
>> Hi Ard, Leif,
>>
>> Any comments on this patch ?
>
> You didn't cc me before :)
>
> But more importantly, I don't really have any platform to test this
> on, so I could use a Tested-by: from someone who does. Evan, do you?
>
>> > From: Bhupesh Sharma [mailto:bhupesh.sha...@nxp.com]
>> > Sent: Friday, October 14, 2016 4:40 PM
>> >
>> > ARM TZASC-380 IP provides a mechanism to split memory regions being
>> > protected via it into eight equal-sized sub-regions, with a bit setting
>> > allowing the corresponding subregion to be disabled.
>> >
>> > Several NXP/FSL SoCs support the TZASC-380 IP block and allow the DDR
>> > connected via the TZASC to be partitioned into regions having different
>> > security settings.
>> >
>> > This patch enables this support and can be used for SoCs which support
>> > such partition of DDR regions.
>> >
>> > Details of the 'subregion_disable'
>
> This is not actually what the register is called in the link you're
> providing.
>
>> > register can be viewed here:
>> > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0431c/CJ
>> > ABCFHB.html
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.0
>> > Signed-off-by: Bhupesh Sharma 
>> > Cc: Ard Biesheuvel 
>> > ---
>> >  .../Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c | 21
>> > ++---
>> >  ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c  |  5 +++--
>> >  ArmPlatformPkg/Include/Drivers/ArmTrustzone.h   |  3 ++-
>> >  3 files changed, 19 insertions(+), 10 deletions(-)
>> >
>> > diff --git
>> > a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
>> > ec.c
>> > b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
>> > ec.c
>> > index 6fa0774..d358d65 100644
>> > ---
>> > a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
>> > ec.c
>> > +++
>> > b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9
>> > +++ x4Sec.c
>> > @@ -72,18 +72,21 @@ ArmPlatformSecTrustzoneInit (
>> >// NOR Flash 0 non secure (BootMon)
>> >TZASCSetRegion(ARM_VE_TZASC_BASE,1,TZASC_REGION_ENABLED,
>> >ARM_VE_SMB_NOR0_BASE,0,
>> > -  TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
>> > +  TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW,
>> > +  0);
>> >
>> >// NOR Flash 1. The first half of the NOR Flash1 must be secure for
>> > the secure firmware (sec_uefi.bin)
>> >if (PcdGetBool (PcdTrustzoneSupport) == TRUE) {
>> >  //Note: Your OS Kernel must be aware of the secure regions before
>> > to enable this region
>> >  TZASCSetRegion(ARM_VE_TZASC_BASE,2,TZASC_REGION_ENABLED,
>> >  ARM_VE_SMB_NOR1_BASE + SIZE_32MB,0,
>> > -TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW);
>> > +TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW,
>> > +   0);
>
> TAB used (convert to spaces).
>
>> >} else {
>> >  TZASCSetRegion(ARM_VE_TZASC_BASE,2,TZASC_REGION_ENABLED,
>> >  ARM_VE_SMB_NOR1_BASE,0,
>> > -TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
>> > +TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW,
>> > +   0);
>
> TAB used (convert to spaces).
>
>> >}
>> >
>> >// Base of SRAM. Only half of SRAM in Non Secure world @@ -92,22
>> > +95,26 @@ ArmPlatformSecTrustzoneInit (
>> >  //Note: Your OS Kernel must be aware of the secure regions before
>> > to enable this region
>> >  TZASCSetRegion(ARM_VE_TZASC_BASE,3,TZASC_REGION_ENABLED,
>> >  ARM_VE_SMB_SRAM_BASE,0,
>> > -TZASC_REGION_SIZE_16MB, TZASC_REGION_SECURITY_NSRW);
>> > +TZASC_REGION_SIZE_16MB, TZASC_REGION_SECURITY_NSRW,
>> > +   0);
>
> TAB used (convert to spaces).
>
>> >} else {
>> >  TZASCSetRegion(ARM_VE_TZASC_BASE,3,TZASC_REGION_ENABLED,
>> >  ARM_VE_SMB_SRAM_BASE,0,
>> > -TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW);
>> > +TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW,
>> > +   0);
>
> TAB used (convert to spaces).
>
> (These are all found by BaseTools/Scripts/PatchCheck.py, which also
> points out that the subject line is too long.)
>
>> >}
>> >
>> >// Memory Mapped Peripherals. All in non secure world
>> >TZASCSetRegion(ARM_VE_TZASC_BASE,4,TZASC_REGION_ENABLED,
>> >ARM_VE_SMB_PERIPH_BASE,0,
>> > -  TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
>> > +  TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW,
>> > +  0);
>> >
>> >// MotherBoard Peripherals and On-chip peripherals.
>> >TZASCSetRegion(ARM_VE_TZASC_BASE,5,TZASC_REGION_ENABLED,
>> >ARM_VE_SMB_MB_ON_CHIP_PERIPH_BASE,0,
>> > -  TZASC_REGION_SIZE_256MB, TZASC_REGION_SECURITY_NSRW);
>> > +  TZASC_REGION_SIZE_256MB, TZASC_REGION_SECURITY_NSRW,
>> > +  0);
>> >  }
>> >
>> >  /**
>> > 

Re: [edk2] [PATCH] MdePkg/BaseLib: Remove the unnecessary '_' before library APIs in ASM/NASM

[Gao, Liming]

Reviewed-by: Liming Gao 

-Original Message-
From: Wu, Hao A 
Sent: Monday, October 17, 2016 7:22 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A ; Gao, Liming ; Kinney, 
Michael D 
Subject: [PATCH] MdePkg/BaseLib: Remove the unnecessary '_' before library APIs 
in ASM/NASM

The leading underscore (i.e. '_') before the names of some BaseLib library
API in ASM/NASM files is unnecessary. It will cause link error with GCC
tool chains.

Cc: Liming Gao 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.asm| 6 +++---
 MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.nasm   | 6 +++---
 MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.asm  | 6 +++---
 MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.nasm | 6 +++---
 MdePkg/Library/BaseLib/Ia32/EnableInterrupts.asm | 6 +++---
 MdePkg/Library/BaseLib/Ia32/EnableInterrupts.nasm| 6 +++---
 MdePkg/Library/BaseLib/Ia32/Invd.asm | 6 +++---
 MdePkg/Library/BaseLib/Ia32/Invd.nasm| 6 +++---
 MdePkg/Library/BaseLib/Ia32/WriteLdtr.asm| 6 +++---
 MdePkg/Library/BaseLib/Ia32/WriteLdtr.nasm   | 6 +++---
 10 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.asm 
b/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.asm
index e436405..ab7c2cf 100644
--- a/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.asm
+++ b/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.asm
@@ -1,5 +1,5 @@
 
;-- 
;
-; Copyright (c) 2006, Intel Corporation. All rights reserved.
+; Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
 ; This program and the accompanying materials
 ; are licensed and made available under the terms and conditions of the BSD 
License
 ; which accompanies this distribution.  The full text of the license may be 
found at
@@ -32,9 +32,9 @@
 ;   VOID
 ;   );
 ;--
-_CpuBreakpoint   PROC
+CpuBreakpoint   PROC
 int  3
 ret
-_CpuBreakpoint   ENDP
+CpuBreakpoint   ENDP
 
 END
diff --git a/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.nasm 
b/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.nasm
index fb1dd2e..b8ae0f9 100644
--- a/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.nasm
+++ b/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.nasm
@@ -1,5 +1,5 @@
 
;-- 
;
-; Copyright (c) 2006, Intel Corporation. All rights reserved.
+; Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
 ; This program and the accompanying materials
 ; are licensed and made available under the terms and conditions of the BSD 
License
 ; which accompanies this distribution.  The full text of the license may be 
found at
@@ -29,8 +29,8 @@
 ;   VOID
 ;   );
 ;--
-global ASM_PFX(_CpuBreakpoint)
-ASM_PFX(_CpuBreakpoint):
+global ASM_PFX(CpuBreakpoint)
+ASM_PFX(CpuBreakpoint):
 int  3
 ret
 
diff --git a/MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.asm 
b/MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.asm
index e54f14e..f108864 100644
--- a/MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.asm
+++ b/MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.asm
@@ -1,6 +1,6 @@
 ;--
 ;
-; Copyright (c) 2006, Intel Corporation. All rights reserved.
+; Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
 ; This program and the accompanying materials
 ; are licensed and made available under the terms and conditions of the BSD 
License
 ; which accompanies this distribution.  The full text of the license may be 
found at
@@ -32,10 +32,10 @@
 ;   VOID
 ;   );
 ;--
-_EnableDisableInterruptsPROC
+EnableDisableInterruptsPROC
 sti
 cli
 ret
-_EnableDisableInterruptsENDP
+EnableDisableInterruptsENDP
 
 END
diff --git a/MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.nasm 
b/MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.nasm
index 7b20675..f7a4f62 100644
--- a/MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.nasm
+++ b/MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.nasm
@@ -1,6 +1,6 @@
 ;--
 ;
-; Copyright (c) 2006, Intel Corporation. All rights reserved.
+; Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
 ; This program and the accompanying materials
 ; are licensed and made available under the terms 

Re: [edk2] [RESEND PATCH 1/1] ArmPlatformPkg/ArmTrustZone: Add support for specifying Subregions to be disabled

[Leif Lindholm]

On Mon, Oct 17, 2016 at 10:18:01AM +, Bhupesh Sharma wrote:
> Hi Ard, Leif,
> 
> Any comments on this patch ?

You didn't cc me before :)

But more importantly, I don't really have any platform to test this
on, so I could use a Tested-by: from someone who does. Evan, do you?

> > From: Bhupesh Sharma [mailto:bhupesh.sha...@nxp.com]
> > Sent: Friday, October 14, 2016 4:40 PM
> > 
> > ARM TZASC-380 IP provides a mechanism to split memory regions being
> > protected via it into eight equal-sized sub-regions, with a bit setting
> > allowing the corresponding subregion to be disabled.
> > 
> > Several NXP/FSL SoCs support the TZASC-380 IP block and allow the DDR
> > connected via the TZASC to be partitioned into regions having different
> > security settings.
> > 
> > This patch enables this support and can be used for SoCs which support
> > such partition of DDR regions.
> > 
> > Details of the 'subregion_disable'

This is not actually what the register is called in the link you're
providing.

> > register can be viewed here:
> > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0431c/CJ
> > ABCFHB.html
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Bhupesh Sharma 
> > Cc: Ard Biesheuvel 
> > ---
> >  .../Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c | 21
> > ++---
> >  ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c  |  5 +++--
> >  ArmPlatformPkg/Include/Drivers/ArmTrustzone.h   |  3 ++-
> >  3 files changed, 19 insertions(+), 10 deletions(-)
> > 
> > diff --git
> > a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
> > ec.c
> > b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
> > ec.c
> > index 6fa0774..d358d65 100644
> > ---
> > a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
> > ec.c
> > +++
> > b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9
> > +++ x4Sec.c
> > @@ -72,18 +72,21 @@ ArmPlatformSecTrustzoneInit (
> >// NOR Flash 0 non secure (BootMon)
> >TZASCSetRegion(ARM_VE_TZASC_BASE,1,TZASC_REGION_ENABLED,
> >ARM_VE_SMB_NOR0_BASE,0,
> > -  TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
> > +  TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW,
> > +  0);
> > 
> >// NOR Flash 1. The first half of the NOR Flash1 must be secure for
> > the secure firmware (sec_uefi.bin)
> >if (PcdGetBool (PcdTrustzoneSupport) == TRUE) {
> >  //Note: Your OS Kernel must be aware of the secure regions before
> > to enable this region
> >  TZASCSetRegion(ARM_VE_TZASC_BASE,2,TZASC_REGION_ENABLED,
> >  ARM_VE_SMB_NOR1_BASE + SIZE_32MB,0,
> > -TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW);
> > +TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW,
> > +   0);

TAB used (convert to spaces).

> >} else {
> >  TZASCSetRegion(ARM_VE_TZASC_BASE,2,TZASC_REGION_ENABLED,
> >  ARM_VE_SMB_NOR1_BASE,0,
> > -TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
> > +TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW,
> > +   0);

TAB used (convert to spaces).

> >}
> > 
> >// Base of SRAM. Only half of SRAM in Non Secure world @@ -92,22
> > +95,26 @@ ArmPlatformSecTrustzoneInit (
> >  //Note: Your OS Kernel must be aware of the secure regions before
> > to enable this region
> >  TZASCSetRegion(ARM_VE_TZASC_BASE,3,TZASC_REGION_ENABLED,
> >  ARM_VE_SMB_SRAM_BASE,0,
> > -TZASC_REGION_SIZE_16MB, TZASC_REGION_SECURITY_NSRW);
> > +TZASC_REGION_SIZE_16MB, TZASC_REGION_SECURITY_NSRW,
> > +   0);

TAB used (convert to spaces).

> >} else {
> >  TZASCSetRegion(ARM_VE_TZASC_BASE,3,TZASC_REGION_ENABLED,
> >  ARM_VE_SMB_SRAM_BASE,0,
> > -TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW);
> > +TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW,
> > +   0);

TAB used (convert to spaces).

(These are all found by BaseTools/Scripts/PatchCheck.py, which also
points out that the subject line is too long.)

> >}
> > 
> >// Memory Mapped Peripherals. All in non secure world
> >TZASCSetRegion(ARM_VE_TZASC_BASE,4,TZASC_REGION_ENABLED,
> >ARM_VE_SMB_PERIPH_BASE,0,
> > -  TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
> > +  TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW,
> > +  0);
> > 
> >// MotherBoard Peripherals and On-chip peripherals.
> >TZASCSetRegion(ARM_VE_TZASC_BASE,5,TZASC_REGION_ENABLED,
> >ARM_VE_SMB_MB_ON_CHIP_PERIPH_BASE,0,
> > -  TZASC_REGION_SIZE_256MB, TZASC_REGION_SECURITY_NSRW);
> > +  TZASC_REGION_SIZE_256MB, TZASC_REGION_SECURITY_NSRW,
> > +  0);
> >  }
> > 
> >  /**
> > diff --git a/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
> > b/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
> > index 070c0dc..5cd41ef 100644
> > --- 

[edk2] [PATCH] MdePkg/BaseLib: Remove the unnecessary '_' before library APIs in ASM/NASM

[Hao Wu]

The leading underscore (i.e. '_') before the names of some BaseLib library
API in ASM/NASM files is unnecessary. It will cause link error with GCC
tool chains.

Cc: Liming Gao 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.asm| 6 +++---
 MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.nasm   | 6 +++---
 MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.asm  | 6 +++---
 MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.nasm | 6 +++---
 MdePkg/Library/BaseLib/Ia32/EnableInterrupts.asm | 6 +++---
 MdePkg/Library/BaseLib/Ia32/EnableInterrupts.nasm| 6 +++---
 MdePkg/Library/BaseLib/Ia32/Invd.asm | 6 +++---
 MdePkg/Library/BaseLib/Ia32/Invd.nasm| 6 +++---
 MdePkg/Library/BaseLib/Ia32/WriteLdtr.asm| 6 +++---
 MdePkg/Library/BaseLib/Ia32/WriteLdtr.nasm   | 6 +++---
 10 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.asm 
b/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.asm
index e436405..ab7c2cf 100644
--- a/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.asm
+++ b/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.asm
@@ -1,5 +1,5 @@
 
;-- 
;
-; Copyright (c) 2006, Intel Corporation. All rights reserved.
+; Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
 ; This program and the accompanying materials
 ; are licensed and made available under the terms and conditions of the BSD 
License
 ; which accompanies this distribution.  The full text of the license may be 
found at
@@ -32,9 +32,9 @@
 ;   VOID
 ;   );
 ;--
-_CpuBreakpoint   PROC
+CpuBreakpoint   PROC
 int  3
 ret
-_CpuBreakpoint   ENDP
+CpuBreakpoint   ENDP
 
 END
diff --git a/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.nasm 
b/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.nasm
index fb1dd2e..b8ae0f9 100644
--- a/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.nasm
+++ b/MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.nasm
@@ -1,5 +1,5 @@
 
;-- 
;
-; Copyright (c) 2006, Intel Corporation. All rights reserved.
+; Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
 ; This program and the accompanying materials
 ; are licensed and made available under the terms and conditions of the BSD 
License
 ; which accompanies this distribution.  The full text of the license may be 
found at
@@ -29,8 +29,8 @@
 ;   VOID
 ;   );
 ;--
-global ASM_PFX(_CpuBreakpoint)
-ASM_PFX(_CpuBreakpoint):
+global ASM_PFX(CpuBreakpoint)
+ASM_PFX(CpuBreakpoint):
 int  3
 ret
 
diff --git a/MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.asm 
b/MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.asm
index e54f14e..f108864 100644
--- a/MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.asm
+++ b/MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.asm
@@ -1,6 +1,6 @@
 ;--
 ;
-; Copyright (c) 2006, Intel Corporation. All rights reserved.
+; Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
 ; This program and the accompanying materials
 ; are licensed and made available under the terms and conditions of the BSD 
License
 ; which accompanies this distribution.  The full text of the license may be 
found at
@@ -32,10 +32,10 @@
 ;   VOID
 ;   );
 ;--
-_EnableDisableInterruptsPROC
+EnableDisableInterruptsPROC
 sti
 cli
 ret
-_EnableDisableInterruptsENDP
+EnableDisableInterruptsENDP
 
 END
diff --git a/MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.nasm 
b/MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.nasm
index 7b20675..f7a4f62 100644
--- a/MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.nasm
+++ b/MdePkg/Library/BaseLib/Ia32/EnableDisableInterrupts.nasm
@@ -1,6 +1,6 @@
 ;--
 ;
-; Copyright (c) 2006, Intel Corporation. All rights reserved.
+; Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
 ; This program and the accompanying materials
 ; are licensed and made available under the terms and conditions of the BSD 
License
 ; which accompanies this distribution.  The full text of the license may be 
found at
@@ -30,8 +30,8 @@
 ;   VOID
 ;   );
 ;--
-global ASM_PFX(_EnableDisableInterrupts)
-ASM_PFX(_EnableDisableInterrupts):
+global ASM_PFX(EnableDisableInterrupts)

[edk2] Which CSM supported MotherBoard should be used for development

[Saqib Khan]

Hi, Can any one help my select mother board? currently I am testing on asus
h81-me but it looks like it has bug in its CSM.


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

Re: [edk2] [RESEND PATCH 1/1] ArmPlatformPkg/ArmTrustZone: Add support for specifying Subregions to be disabled

[Bhupesh Sharma]

Hi Ard, Leif,

Any comments on this patch ?

> From: Bhupesh Sharma [mailto:bhupesh.sha...@nxp.com]
> Sent: Friday, October 14, 2016 4:40 PM
> 
> ARM TZASC-380 IP provides a mechanism to split memory regions being
> protected via it into eight equal-sized sub-regions, with a bit setting
> allowing the corresponding subregion to be disabled.
> 
> Several NXP/FSL SoCs support the TZASC-380 IP block and allow the DDR
> connected via the TZASC to be partitioned into regions having different
> security settings.
> 
> This patch enables this support and can be used for SoCs which support
> such partition of DDR regions.
> 
> Details of the 'subregion_disable' register can be viewed here:
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0431c/CJ
> ABCFHB.html
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Bhupesh Sharma 
> Cc: Ard Biesheuvel 
> ---
>  .../Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c | 21
> ++---
>  ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c  |  5 +++--
>  ArmPlatformPkg/Include/Drivers/ArmTrustzone.h   |  3 ++-
>  3 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git
> a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
> ec.c
> b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
> ec.c
> index 6fa0774..d358d65 100644
> ---
> a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
> ec.c
> +++
> b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9
> +++ x4Sec.c
> @@ -72,18 +72,21 @@ ArmPlatformSecTrustzoneInit (
>// NOR Flash 0 non secure (BootMon)
>TZASCSetRegion(ARM_VE_TZASC_BASE,1,TZASC_REGION_ENABLED,
>ARM_VE_SMB_NOR0_BASE,0,
> -  TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
> +  TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW,
> +  0);
> 
>// NOR Flash 1. The first half of the NOR Flash1 must be secure for
> the secure firmware (sec_uefi.bin)
>if (PcdGetBool (PcdTrustzoneSupport) == TRUE) {
>  //Note: Your OS Kernel must be aware of the secure regions before
> to enable this region
>  TZASCSetRegion(ARM_VE_TZASC_BASE,2,TZASC_REGION_ENABLED,
>  ARM_VE_SMB_NOR1_BASE + SIZE_32MB,0,
> -TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW);
> +TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW,
> + 0);
>} else {
>  TZASCSetRegion(ARM_VE_TZASC_BASE,2,TZASC_REGION_ENABLED,
>  ARM_VE_SMB_NOR1_BASE,0,
> -TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
> +TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW,
> + 0);
>}
> 
>// Base of SRAM. Only half of SRAM in Non Secure world @@ -92,22
> +95,26 @@ ArmPlatformSecTrustzoneInit (
>  //Note: Your OS Kernel must be aware of the secure regions before
> to enable this region
>  TZASCSetRegion(ARM_VE_TZASC_BASE,3,TZASC_REGION_ENABLED,
>  ARM_VE_SMB_SRAM_BASE,0,
> -TZASC_REGION_SIZE_16MB, TZASC_REGION_SECURITY_NSRW);
> +TZASC_REGION_SIZE_16MB, TZASC_REGION_SECURITY_NSRW,
> + 0);
>} else {
>  TZASCSetRegion(ARM_VE_TZASC_BASE,3,TZASC_REGION_ENABLED,
>  ARM_VE_SMB_SRAM_BASE,0,
> -TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW);
> +TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW,
> + 0);
>}
> 
>// Memory Mapped Peripherals. All in non secure world
>TZASCSetRegion(ARM_VE_TZASC_BASE,4,TZASC_REGION_ENABLED,
>ARM_VE_SMB_PERIPH_BASE,0,
> -  TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
> +  TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW,
> +  0);
> 
>// MotherBoard Peripherals and On-chip peripherals.
>TZASCSetRegion(ARM_VE_TZASC_BASE,5,TZASC_REGION_ENABLED,
>ARM_VE_SMB_MB_ON_CHIP_PERIPH_BASE,0,
> -  TZASC_REGION_SIZE_256MB, TZASC_REGION_SECURITY_NSRW);
> +  TZASC_REGION_SIZE_256MB, TZASC_REGION_SECURITY_NSRW,
> +  0);
>  }
> 
>  /**
> diff --git a/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
> b/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
> index 070c0dc..5cd41ef 100644
> --- a/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
> +++ b/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
> @@ -87,7 +87,8 @@ TZASCSetRegion (
>IN  UINTN LowAddress,
>IN  UINTN HighAddress,
>IN  UINTN Size,
> -  IN  UINTN Security
> +  IN  UINTN Security,
> +  IN  UINTN SubregionDisableMask
>)
>  {
>UINT32* Region;
> @@ -100,7 +101,7 @@ TZASCSetRegion (
> 
>MmioWrite32((UINTN)(Region), LowAddress&0x8000);
>MmioWrite32((UINTN)(Region+1), HighAddress);
> -  MmioWrite32((UINTN)(Region+2), ((Security & 0xF) <<28) | ((Size &
> 0x3F) << 1) | (Enabled & 0x1));
> +  MmioWrite32((UINTN)(Region+2), ((Security & 0xF) <<28) |
> + ((SubregionDisableMask & 0xFF) << 8) | ((Size & 0x3F) << 1) |
> (Enabled
> + & 0x1));
> 
>return EFI_SUCCESS;
>  }
> diff --git 

Re: [edk2] [PATCH v2] MdeModulePkg/BootMaintenanceUi: Enhance the codes logic

[Laszlo Ersek]

On 10/17/16 11:07, Bi, Dandan wrote:
> Hi Laszlo,
> 
> Thank you very much for  your comments!  
> I have split this patch into 5 independent patches with following subject : 
> [patch 1/5] MdeModulePkg/BMMUI: ...
> [patch 2/5] MdeModulePkg/BMMUI: ...
> [patch 3/5] MdeModulePkg/BMMUI: ...
> [patch 4/5] MdeModulePkg/BMMUI: ...
> [patch 5/5] MdeModulePkg/BMMUI: ...

Thank you very much -- the new subjects look much-much better. Also, I
think it's a nice trick to shorten "Boot Maintenance Manager UI" as
"BMMUI", it's understandable and it saves quite a few characters in the
subjects.

Thanks!
Laszlo

> Hi Liming/Eric
> Please review the new patches and  ignore this one.  Sorry for any 
> inconvenience.
> 
> 
> Regards,
> Dandan
> 
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com] 
> Sent: Friday, October 14, 2016 8:34 PM
> To: Bi, Dandan ; edk2-de...@ml01.01.org
> Cc: Dong, Eric ; Gao, Liming 
> Subject: Re: [edk2] [PATCH v2] MdeModulePkg/BootMaintenanceUi: Enhance the 
> codes logic
> 
> On 10/14/16 08:43, Dandan Bi wrote:
>> This patch is mainly to:
>> 1. Enhance the error handling codes when set variable fail.
>> 2. Enhance the logic to fix some incorrect UI behaviors.
> 
> My apologies, but both the subject line and the commit message are mostly 
> impenetrable.
> 
> This patch should be split up into a series of two patches, minimally 
> (according to the two goals above that it implements), and each change should 
> be described correctly in both the subject line and in the commit message.
> 
> If I got a bug report for OVMF that I managed to bisect back to this patch, 
> I'd be *completely* helpless figuring out what it does.
> 
> What kind of variables are set by the code? What happens now if setting those 
> variables fails? What is the expected behavior instead that the
> (first) patch implements?
> 
> What are those incorrect UI behaviors? When do they happen? What does the 
> second patch do to address those issues?
> 
> Dear Developers, please *stop* writing subject lines like
> 
>   "Enhance the code in DNS driver"
>   "Enhance the codes logic"
> 
> those subject lines are *completely* useless. You could replace all those 
> subject lines, without any loss of information, with the following
> one:
> 
>   "Do Work"
> 
> Please spend time thinking about the granularity, the focus of your patches, 
> as a *standalone activity* during development. Ask yourselves, "Is this patch 
> small enough? Am I doing two or more independent things here? Is the subject 
> line clear enough? If a person sees the code for the first time, will my 
> commit message help them?"
> 
> You don't write the commit message for yourselves only, you write it for 
> other developers who might have absolutely no clue what's going on in your 
> module.
> 
> Thanks
> Laszlo
> 
>> V2: Update the Console/Terminal menu when the related question changed.
>>
>> Cc: Liming Gao 
>> Cc: Eric Dong 
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Dandan Bi 
>> ---
>>  .../BootMaintenanceManagerUiLib/BootMaintenance.c  | 390 
>> -
>>  .../BootMaintenanceManagerUiLib/UpdatePage.c   |  42 ++-
>>  .../Library/BootMaintenanceManagerUiLib/Variable.c |  28 +-
>>  3 files changed, 357 insertions(+), 103 deletions(-)
>>
>> diff --git 
>> a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c 
>> b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c
>> index a190596..924eb49 100644
>> --- 
>> a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c
>> +++ b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance
>> +++ .c
>> @@ -442,10 +442,197 @@ BmmExtractDevicePathFromHiiHandle (
>>return ConvertDevicePathToText(DevicePathFromHandle (DriverHandle), 
>> FALSE, FALSE);
>>  
>>  }
>>  
>>  /**
>> +  Converts the unicode character of the string from uppercase to lowercase.
>> +  This is a internal function.
>> +
>> +  @param ConfigString  String to be converted
>> +
>> +**/
>> +VOID
>> +HiiToLower (
>> +  IN EFI_STRING  ConfigString
>> +  )
>> +{
>> +  EFI_STRING  String;
>> +  BOOLEAN Lower;
>> +
>> +  ASSERT (ConfigString != NULL);
>> +
>> +  //
>> +  // Convert all hex digits in range [A-F] in the configuration 
>> +header to [a-f]
>> +  //
>> +  for (String = ConfigString, Lower = FALSE; *String != L'\0'; String++) {
>> +if (*String == L'=') {
>> +  Lower = TRUE;
>> +} else if (*String == L'&') {
>> +  Lower = FALSE;
>> +} else if (Lower && *String >= L'A' && *String <= L'F') {
>> +  *String = (CHAR16) (*String - L'A' + L'a');
>> +}
>> +  }
>> +}
>> +
>> +/**
>> +  Update the progress string through the offset value.
>> +
>> +  @param Offset   The offset value
>> +  @param ConfigurationPoint to the configuration string.
>> +
>> +**/
>> 

Re: [edk2] [PATCH] ArmVirtPkg: Drop the nonsense ASSERT() statement

[Laszlo Ersek]

On 10/17/16 10:54, Dennis Chen wrote:
> Since All the GIC base address variables now are 64-bit, given
> that a UNIT64 var cannot exceed MAX_UNIT64, it doesn't make sense
> to continue keep them in the codes, so this patch just simply drop
> those ASSERT() statements as it should be.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Cc: Ard Biesheuvel 
> Cc: Leif Lindholm 
> Cc: Laszlo Ersek 
> Signed-off-by: Dennis Chen 
> ---
>  ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c 
> b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> index 64afc4d..7a312e5 100644
> --- a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> +++ b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> @@ -79,11 +79,9 @@ ArmVirtGicArchLibConstructor (
>  
>  // RegProp[0..1] == { GICD base, GICD size }
>  DistBase = SwapBytes64 (Reg[0]);
> -ASSERT (DistBase < MAX_UINT32);
>  
>  // RegProp[2..3] == { GICR base, GICR size }
>  RedistBase = SwapBytes64 (Reg[2]);
> -ASSERT (RedistBase < MAX_UINT32);
>  
>  PcdSet64 (PcdGicDistributorBase, DistBase);
>  PcdSet64 (PcdGicRedistributorsBase, RedistBase);
> @@ -117,8 +115,6 @@ ArmVirtGicArchLibConstructor (
>  
>  DistBase = SwapBytes64 (Reg[0]);
>  CpuBase  = SwapBytes64 (Reg[2]);
> -ASSERT (DistBase < MAX_UINT32);
> -ASSERT (CpuBase < MAX_UINT32);
>  
>  PcdSet64 (PcdGicDistributorBase, DistBase);
>  PcdSet64 (PcdGicInterruptInterfaceBase, CpuBase);
> 

Is this a continuation of your

commit 8a1f2378d74390ddfe35c70f68e0c8b03bf84089
Author: Dennis Chen 
Date:   Mon Sep 5 19:38:20 2016 +0800

ArmPkg ArmPlatformPkg ArmVirtPkg: ARM GICv2/v3 Base Address width fix-up

?

I think I'll let Ard review this one. :)

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

Re: [edk2] Urgent help -UefiBootManagerLib and LegacyBootManagerLib issue

[Saqib Khan]

hi,Thank Laszlo and Andrew, here are my findings

Below loop in  LegacyBM.c iterate 16 times out which 2 times it return
success and 14 times it return error code 3























*for (Index = 0; Index < HandleCount; Index++) {  //  // Start the
thunk driver so that the legacy option rom gets dispatched.  // Note:
We don't directly call InstallPciRom because some thunk drivers   //
(e.g. BlockIo thunk driver) depend on the immediate result after
dispatching  //// Print(L"LegacyBmRefreshAllBootOption:
%d\n",Index);  Status = LegacyBios->CheckPciRom
( LegacyBios,
HandleBuffer[Index],
NULL, NULL,
 );  if (!EFI_ERROR (Status)) {
  Print(L"PCI Successfull   [%X]\n",Status);
gBS->ConnectController (HandleBuffer[Index], NULL, NULL, FALSE);  }
  else  {Print(L"PCI fail  [%X]\n",Status);  }}*

should *LegacyBios->CheckPciRom *return always Successful in above loop?
After this LegacyBmUpdateDevOrder () is called and inside this method
system hang in loop below

Ptr = DevOrder;
  NewPtr  = NewDevOrder;
  NewPtr->BbsType = Ptr->BbsType;
  NewPtr->Length  = (UINT16) (sizeof (UINT16) + FDCount * sizeof (UINT16));
  for (Index = 0; Index < Ptr->Length / sizeof (UINT16) - 1; Index++) {
if (!LegacyBmValidBbsEntry ([Ptr->Data[Index] & 0xFF]) ||
LocalBbsTable[Ptr->Data[Index] & 0xFF].DeviceType != BBS_FLOPPY
) {
//   * Print(L"Continure %d\n",Index); it keeps in this loop some
time in loop below*
 continue;
}
Print(L" \n");
NewPtr->Data[FDIndex] = Ptr->Data[Index];
FDIndex++;
  }
  NewFDPtr = NewPtr->Data;



On Thu, Oct 6, 2016 at 1:41 PM, Laszlo Ersek  wrote:

> On 10/06/16 09:28, Saqib Khan wrote:
> > Hi Andrew,
> >
> > I think I did not address my problem well in my previous email.Please
> take
> > a minute again to understand my problem.
> >
> > here is my scenario
> >
> >  have following  lib added to my *.inf file
> > [LibraryClasses]
> >   HiiLib
> >   DebugLib
> >   UefiLib
> >   MemoryAllocationLib
> >   UefiBootServicesTableLib
> >   UefiApplicationEntryPoint
> >
> > *UefiBootManagerLib  LegacyBootManagerLib*
> >   UefiShellLib
> >
> > And here is piece of code I am trying to compile
> >
> > EfiBootManagerConnectAll ();
> > EfiBootManagerRefreshAllBootOption ();
> >
> > BootOption = EfiBootManagerGetLoadOptions (,
> > LoadOptionTypeBoot);
> >
> > Status = gBS->LocateProtocol (, NULL, (VOID
> **)
> > );
> >
> > it Compiles successfully but EFI hangs at
> >
> > *EfiBootManagerRefreshAllBootOption () *
> > When I remove * LegacyBootManagerLib* Libraries it does not hang
> >
> > I think I missing something in it may be i need to add CSM libraries in
> my
> > EFI?
> >
> > I also tried NULL library resolution in DuetPkgx64.dsc like this
> >
> > MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenuApp.inf {
> > 
> >
> > NULL|IntelFrameworkModulePkg/Library/LegacyBootManagerLib/
> LegacyBootManagerLib.inf
> >
> >   }
> >
> >
> > If you still think that is a CSM issue then i will be go for another
> system
> > as i am doing my development on physical system.
> >
> >
> > *Thank you*
>
> Andrew understood your problem just fine, and gave the correct answer.
>
> (1) As I explained to you in one of my previous emails (or, well, I at
> least alluded to it), namely in
>
>   https://lists.01.org/pipermail/edk2-devel/2016-September/001799.html
>
> LegacyBootManagerLib is a *plugin* for UefiBootManagerLib. That is, a
> library to be used with NULL library class resolution in whatever driver
> or application that you already use UefiBootManagerLib in. (If you know
> that your module will always need LegacyBootManagerLib, then you can
> explicitly specify it in your INF file too.)
>
> (2) If you do not add LegacyBootManagerLib to your application like
> explained above, then the following will happen:
>
> - UefiBootManagerLib's EfiBootManagerRegisterLegacyBootSupport()
> function will never be called,
>
> - therefore UefiBootManagerLib's mBmRefreshLegacyBootOption global
> variable will remain NULL,
>
> - therefore EfiBootManagerRefreshAllBootOption() will never call
> (*mBmRefreshLegacyBootOption)().
>
> (3) In comprison, if you *do* add LegacyBootManagerLib to your
> application, then the following will happen:
>
> - LegacyBootManagerLib's constructor function, namely
> LegacyBootManagerLibConstructor(), will call
> EfiBootManagerRegisterLegacyBootSupport(), with the following two
> function pointers:
>   - LegacyBmRefreshAllBootOption
>   - LegacyBmBoot
>
> - In turn, UefiBootManagerLib's mBmRefreshLegacyBootOption will be set
> to LegacyBmRefreshAllBootOption
>
> - In turn, EfiBootManagerRefreshAllBootOption() will call
> LegacyBmRefreshAllBootOption(), through the mBmRefreshLegacyBootOption
> function pointer.
>
> This is exactly what's happening in your case; 

[edk2] [Patch] BaseTools: Enhance tool to generate EFI_HII_IIBT_DUPLICATE image block

[Yonghong Zhu]

When *.IDF file contains multiple definitions of image which point to the
same image, current build tool generates multiple image blocks which
contain the same image content.
This patch enhance tool to generate EFI_HII_IIBT_DUPLICATE image blocks
for non-first images for such case, to save the HII package size.

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

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/AutoGen/GenC.py | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/BaseTools/Source/Python/AutoGen/GenC.py 
b/BaseTools/Source/Python/AutoGen/GenC.py
index 8089e3a..de6eb0e 100644
--- a/BaseTools/Source/Python/AutoGen/GenC.py
+++ b/BaseTools/Source/Python/AutoGen/GenC.py
@@ -1637,10 +1637,11 @@ def CreateIdfFileCode(Info, AutoGenC, StringH, 
IdfGenCFlag, IdfGenBinBuffer):
 PaletteInfoOffset = 0
 ImageBuffer = pack('x')
 PaletteBuffer = pack('x')
 BufferStr = ''
 PaletteStr = ''
+FileDict = {}
 for Idf in ImageFiles.ImageFilesDict:
 if ImageFiles.ImageFilesDict[Idf]:
 for FileObj in ImageFiles.ImageFilesDict[Idf]:
 for sourcefile in Info.SourceFileList:
 if FileObj.FileName == sourcefile.File:
@@ -1661,10 +1662,23 @@ def CreateIdfFileCode(Info, AutoGenC, StringH, 
IdfGenCFlag, IdfGenBinBuffer):
 if (ValueStartPtr - len(DEFINE_STR + ID)) <= 0:
 Line = DEFINE_STR + ' ' + ID + ' ' + 
DecToHexStr(Index, 4) + '\n'
 else:
 Line = DEFINE_STR + ' ' + ID + ' ' * 
(ValueStartPtr - len(DEFINE_STR + ID)) + DecToHexStr(Index, 4) + '\n'
 
+if File not in FileDict:
+FileDict[File] = Index
+else:
+DuplicateBlock = pack('B', 
EFI_HII_IIBT_DUPLICATE)
+DuplicateBlock += pack('H', FileDict[File])
+ImageBuffer += DuplicateBlock
+BufferStr = WriteLine(BufferStr, '// %s: %s: 
%s' % (DecToHexStr(Index, 4), ID, DecToHexStr(Index, 4)))
+TempBufferList = AscToHexList(DuplicateBlock)
+BufferStr = WriteLine(BufferStr, 
CreateArrayItem(TempBufferList, 16) + '\n')
+StringH.Append(Line)
+Index += 1
+continue
+
 TmpFile = open(File.Path, 'rb')
 Buffer = TmpFile.read()
 TmpFile.close()
 if File.Ext.upper() == '.PNG':
 TempBuffer = pack('B', EFI_HII_IIBT_IMAGE_PNG)
-- 
2.6.1.windows.1

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

Re: [edk2] [PATCH] GPT Shell Application/Library

[Laszlo Ersek]

On 10/17/16 08:49, Vladimir Olovyannikov wrote:
> Thank you Laszlo,
> 
> Sorry, I missed the fields; it is my first contribution, I will add the
> required lines, review the source according to your comments and will
> resubmit the patch.
> So do you think the command should be _gpt instead of gpt? I was
> following TFTP and SF commands as a template.

Right, at this point I'm confused about TFTP even, in retrospect. It's
not in the spec, so I guess it should have been _TFTP. I hope Jaben and
Ray can advise us. :)

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

Re: [edk2] [PATCH] ArmPkg ArmVirtPkg: fix the GIC base address variables as 64-bit

[Leif Lindholm]

On Mon, Oct 17, 2016 at 10:20:30AM +0100, Ard Biesheuvel wrote:
> On 17 October 2016 at 09:33, Leif Lindholm  wrote:
> > On Mon, Oct 17, 2016 at 08:28:50AM +0100, Ard Biesheuvel wrote:
> >> > diff --git a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c 
> >> > b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> >> > index 64afc4d..16683ef 100644
> >> > --- a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> >> > +++ b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> >> > @@ -79,11 +79,11 @@ ArmVirtGicArchLibConstructor (
> >> >
> >> >  // RegProp[0..1] == { GICD base, GICD size }
> >> >  DistBase = SwapBytes64 (Reg[0]);
> >> > -ASSERT (DistBase < MAX_UINT32);
> >> > +ASSERT (DistBase < MAX_UINT64);
> >> >
> >>
> >> This becomes equivalent to 'DistBase != MAX_UINT64' given that a
> >> UINT64 cannot exceed MAX_UINT64. That is a nonsensical thing to
> >> assert, so it is better to simply drop it
> >
> > Random thought:
> > Could we keep the assert(s) and change the test to MAX_UINTN, to have
> > a sanity test over whether a 32-bit plaform ends up with a duff
> > address?
> 
> That seems like a useful thing in general, but given that we don't do
> that anywhere else, I'd rather we just remove them.

I won't argue with that.

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

Re: [edk2] [PATCH] ArmPkg ArmVirtPkg: fix the GIC base address variables as 64-bit

[Ard Biesheuvel]

On 17 October 2016 at 09:33, Leif Lindholm  wrote:
> On Mon, Oct 17, 2016 at 08:28:50AM +0100, Ard Biesheuvel wrote:
>> > diff --git a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c 
>> > b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
>> > index 64afc4d..16683ef 100644
>> > --- a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
>> > +++ b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
>> > @@ -79,11 +79,11 @@ ArmVirtGicArchLibConstructor (
>> >
>> >  // RegProp[0..1] == { GICD base, GICD size }
>> >  DistBase = SwapBytes64 (Reg[0]);
>> > -ASSERT (DistBase < MAX_UINT32);
>> > +ASSERT (DistBase < MAX_UINT64);
>> >
>>
>> This becomes equivalent to 'DistBase != MAX_UINT64' given that a
>> UINT64 cannot exceed MAX_UINT64. That is a nonsensical thing to
>> assert, so it is better to simply drop it
>
> Random thought:
> Could we keep the assert(s) and change the test to MAX_UINTN, to have
> a sanity test over whether a 32-bit plaform ends up with a duff
> address?
>

That seems like a useful thing in general, but given that we don't do
that anywhere else, I'd rather we just remove them.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [patch] NetworkPkg: Support bracketed IPv6 address during a redirection in iSCSI

[Fu, Siyuan]

Reviewed-by: Fu Siyuan siyuan...@intel.com


> -Original Message-
> From: Zhang, Lubo
> Sent: Friday, October 14, 2016 2:45 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Wu,
> Jiaxin 
> Subject: [patch] NetworkPkg: Support bracketed IPv6 address during a
> redirection in iSCSI
> 
> According to RFC 3720, the TargetAddress provided in a redirection
> might be a DNS host name, a dotted-decimal IPv4 address, or a
> bracketed IPv6 address. Current ISCSI driver in Networkpkg only
> supports dotted-decimal IPv4 address, so we need add IPv6 address
> support since it is a combo driver supporting dual stack.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Zhang Lubo 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Cc: Wu Jiaxin 
> ---
>  NetworkPkg/IScsiDxe/IScsiProto.c | 51 ++-
> -
>  NetworkPkg/IScsiDxe/IScsiProto.h |  5 +++-
>  2 files changed, 43 insertions(+), 13 deletions(-)
> 
> diff --git a/NetworkPkg/IScsiDxe/IScsiProto.c
> b/NetworkPkg/IScsiDxe/IScsiProto.c
> index 5092365..a9bf491 100644
> --- a/NetworkPkg/IScsiDxe/IScsiProto.c
> +++ b/NetworkPkg/IScsiDxe/IScsiProto.c
> @@ -1087,26 +1087,53 @@ IScsiUpdateTargetAddress (
>  TargetAddress = IScsiGetValueByKeyFromList (KeyValueList,
> ISCSI_KEY_TARGET_ADDRESS);
>  if (TargetAddress == NULL) {
>break;
>  }
> 
> -if (!NET_IS_DIGIT (TargetAddress[0])) {
> +//
> +// RFC 3720 defines format of the
> TargetAddress=domainname[:port][,portal-group-tag]
> +// The domainname can be specified as either a DNS host name,
> adotted-decimal IPv4 address,
> +// or a bracketed IPv6 address as specified in [RFC2732].
> +//
> +if (NET_IS_DIGIT (TargetAddress[0])) {
>//
> -  // The domainname of the target may be presented in three formats:
> a DNS host name,
> -  // a dotted-decimal IPv4 address, or a bracketed IPv6 address. Only
> accept dotted
> -  // IPv4 address.
> +  // The domainname of the target is presented in a dotted-decimal
> IPv4 address format.
>//
> -  continue;
> -}
> -
> -IpStr = TargetAddress;
> +  IpStr = TargetAddress;
> 
> -while ((*TargetAddress != 0) && (*TargetAddress != ':') &&
> (*TargetAddress != ',')) {
> +  while ((*TargetAddress != '\0') && (*TargetAddress != ':') &&
> (*TargetAddress != ',')) {
> +//
> +// NULL, ':', or ',' ends the IPv4 string.
> +//
> +TargetAddress++;
> +  }
> +
> +} else if (*TargetAddress == ISCSI_REDIRECT_ADDR_START_DELIMITER){
>//
> -  // NULL, ':', or ',' ends the IPv4 string.
> +  // The domainname of the target is presented in a bracketed IPv6
> address format.
>//
> -  TargetAddress++;
> +  TargetAddress ++;
> +  IpStr = TargetAddress;
> +  while ((*TargetAddress != '\0') && (*TargetAddress !=
> ISCSI_REDIRECT_ADDR_END_DELIMITER)) {
> +//
> +// ']' ends the IPv6 string.
> +//
> +TargetAddress++;
> +  }
> +
> +  if (*TargetAddress != ISCSI_REDIRECT_ADDR_END_DELIMITER) {
> +continue;
> +  }
> +
> +  *TargetAddress = '\0';
> +  TargetAddress ++;
> +
> +} else {
> +  //
> +  // The domainname of the target is presented in the format of a DNS
> host name.
> +  // Temporary not supported.
> +  continue;
>  }
> 
>  if (*TargetAddress == ',') {
>//
>// Comma and the portal group tag MUST be ommitted if the
> TargetAddress is sent
> @@ -1124,11 +1151,11 @@ IScsiUpdateTargetAddress (
>} else {
>  Session->ConfigData->SessionConfigData.TargetPort = (UINT16)
> Number;
>}
>  } else {
>//
> -  // The string only contains the IPv4 address. Use the well-known
> port.
> +  // The string only contains the Target address. Use the well-known
> port.
>//
>Session->ConfigData->SessionConfigData.TargetPort =
> ISCSI_WELL_KNOWN_PORT;
>  }
>  //
>  // Update the target IP address.
> diff --git a/NetworkPkg/IScsiDxe/IScsiProto.h
> b/NetworkPkg/IScsiDxe/IScsiProto.h
> index 8099f34..367914d 100644
> --- a/NetworkPkg/IScsiDxe/IScsiProto.h
> +++ b/NetworkPkg/IScsiDxe/IScsiProto.h
> @@ -1,9 +1,9 @@
>  /** @file
>The header file of iSCSI Protocol that defines many specific data
> structures.
> 
> -Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.
> +Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be
> found at
>  http://opensource.org/licenses/bsd-license.php
> 
> @@ -38,10 +38,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,

Re: [edk2] [patch] NetworkPkg: Enhance the code in DNS driver.

[Fu, Siyuan]

Reviewed-by: Fu Siyuan siyuan...@intel.com



> -Original Message-
> From: Zhang, Lubo
> Sent: Friday, October 14, 2016 2:54 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Wu,
> Jiaxin 
> Subject: [patch] NetworkPkg: Enhance the code in DNS driver.
> 
> There may be an error happens when we use the
> configure function to set or change the configuration
> data for the DNS6 instance, So we will free the
> DnsServerList without configured to NULL. If we reset
> the instance with the parameter DnsConfigData to NULL, the
> DnsServerList will be freed twice.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Zhang Lubo 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Cc: Wu Jiaxin 
> ---
>  NetworkPkg/DnsDxe/DnsProtocol.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/NetworkPkg/DnsDxe/DnsProtocol.c
> b/NetworkPkg/DnsDxe/DnsProtocol.c
> index 64fca6a..6d117b2 100644
> --- a/NetworkPkg/DnsDxe/DnsProtocol.c
> +++ b/NetworkPkg/DnsDxe/DnsProtocol.c
> @@ -285,10 +285,11 @@ Dns4Configure (
>  //
>  Status = Dns4ConfigUdp (Instance, Instance->UdpIo);
>  if (EFI_ERROR (Status)) {
>if (Instance->Dns4CfgData.DnsServerList != NULL) {
>  FreePool (Instance->Dns4CfgData.DnsServerList);
> +Instance->Dns4CfgData.DnsServerList = NULL;
>}
>goto ON_EXIT;
>  }
> 
>  //
> @@ -296,10 +297,11 @@ Dns4Configure (
>  //
>  Status = AddDns4ServerIp (>Dns4ServerList, Instance-
> >SessionDnsServer.v4);
>  if (EFI_ERROR (Status)) {
>if (Instance->Dns4CfgData.DnsServerList != NULL) {
>  FreePool (Instance->Dns4CfgData.DnsServerList);
> +Instance->Dns4CfgData.DnsServerList = NULL;
>}
>goto ON_EXIT;
>  }
> 
>  Instance->State = DNS_STATE_CONFIGED;
> @@ -1106,10 +1108,11 @@ Dns6Configure (
>  //
>  Status = Dns6ConfigUdp (Instance, Instance->UdpIo);
>  if (EFI_ERROR (Status)) {
>if (Instance->Dns6CfgData.DnsServerList != NULL) {
>  FreePool (Instance->Dns6CfgData.DnsServerList);
> +Instance->Dns6CfgData.DnsServerList = NULL;
>}
>goto ON_EXIT;
>  }
> 
>  //
> @@ -1117,10 +1120,11 @@ Dns6Configure (
>  //
>  Status = AddDns6ServerIp (>Dns6ServerList, Instance-
> >SessionDnsServer.v6);
>  if (EFI_ERROR (Status)) {
>if (Instance->Dns6CfgData.DnsServerList != NULL) {
>  FreePool (Instance->Dns6CfgData.DnsServerList);
> +Instance->Dns6CfgData.DnsServerList = NULL;
>}
>goto ON_EXIT;
>  }
> 
>  Instance->State = DNS_STATE_CONFIGED;
> --
> 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: Add dns support for pxe boot based on IPv6.

[Fu, Siyuan]

Reviewed-by: Fu Siyuan 


> -Original Message-
> From: Zhang, Lubo
> Sent: Friday, October 14, 2016 3:28 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Wu,
> Jiaxin 
> Subject: [patch] NetworkPkg: Add dns support for pxe boot based on IPv6.
> 
> The BootFileURL option (59) in dhcpv6 is used to deliver
> the next server address with bootfile name, as an example
> "tftp://[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]/BOOTFILE_NAME;
> mode=octet", it can also be “tftp://domain_name/BOOTFILE_NAME;
> mode=octet”, this patch is to support this case.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Zhang Lubo 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Cc: Wu Jiaxin 
> ---
>  NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c  |  18 ++-
>  NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c | 269
> +++
>  NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h |   5 +-
>  NetworkPkg/UefiPxeBcDxe/PxeBcImpl.h  |   3 +
>  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf |   4 +-
>  5 files changed, 261 insertions(+), 38 deletions(-)
> 
> diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> index 8eff13c..fc50a82 100644
> --- a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> @@ -619,31 +619,33 @@ PxeBcDhcp6BootInfo (
>}
> 
>ASSERT (Cache6->OptList[PXEBC_DHCP6_IDX_BOOT_FILE_URL] != NULL);
> 
>//
> +  // Set the station address to IP layer.
> +  //
> +  Status = PxeBcSetIp6Address (Private);
> +  if (EFI_ERROR (Status)) {
> +return Status;
> +  }
> +
> +
> +  //
>// Parse (m)tftp server ip address and bootfile name.
>//
>Status = PxeBcExtractBootFileUrl (
> + Private,
>   >BootFileName,
>   >ServerIp.v6,
>   (CHAR8 *) (Cache6->OptList[PXEBC_DHCP6_IDX_BOOT_FILE_URL]-
> >Data),
>   NTOHS (Cache6->OptList[PXEBC_DHCP6_IDX_BOOT_FILE_URL]->OpLen)
>   );
>if (EFI_ERROR (Status)) {
>  return Status;
>}
> 
>//
> -  // Set the station address to IP layer.
> -  //
> -  Status = PxeBcSetIp6Address (Private);
> -  if (EFI_ERROR (Status)) {
> -return Status;
> -  }
> -
> -  //
>// Parse the value of boot file size.
>//
>if (Cache6->OptList[PXEBC_DHCP6_IDX_BOOT_FILE_PARAM] != NULL) {
>  //
>  // Parse it out if have the boot file parameter option.
> diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
> b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
> index 41d3d30..45377e3 100644
> --- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
> +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
> @@ -91,14 +91,15 @@ PxeBcBuildDhcp6Options (
> 
>//
>// Append client option request option
>//
>OptList[Index]->OpCode = HTONS (DHCP6_OPT_ORO);
> -  OptList[Index]->OpLen  = HTONS (4);
> +  OptList[Index]->OpLen  = HTONS (6);
>OptEnt.Oro = (PXEBC_DHCP6_OPTION_ORO *) OptList[Index]-
> >Data;
>OptEnt.Oro->OpCode[0]  = HTONS(DHCP6_OPT_BOOT_FILE_URL);
>OptEnt.Oro->OpCode[1]  = HTONS(DHCP6_OPT_BOOT_FILE_PARAM);
> +  OptEnt.Oro->OpCode[2]  = HTONS(DHCP6_OPT_DNS_SERVERS);
>Index++;
>OptList[Index] = GET_NEXT_DHCP6_OPTION (OptList[Index - 1]);
> 
>//
>// Append client network device interface option
> @@ -214,14 +215,177 @@ PxeBcFreeBootFileOption (
>  RemoveEntryList (Entry);
>  FreePool (Node);
>}
>  }
> 
> +/**
> +  Retrieve the boot server address using the EFI_DNS6_PROTOCOL.
> +
> +  @param[in]  Private Pointer to PxeBc private data.
> +  @param[in]  HostNamePointer to buffer containing hostname.
> +  @param[out] IpAddress   On output, pointer to buffer containing
> IPv6 address.
> +
> +  @retval EFI_SUCCESS Operation succeeded.
> +  @retval EFI_OUT_OF_RESOURCESFailed to allocate needed resources.
> +  @retval EFI_DEVICE_ERRORAn unexpected network error occurred.
> +  @retval Others  Other errors as indicated.
> +
> +**/
> +EFI_STATUS
> +PxeBcDns6 (
> +  IN PXEBC_PRIVATE_DATA   *Private,
> +  IN CHAR16   *HostName,
> + OUT EFI_IPv6_ADDRESS *IpAddress
> +  )
> +{
> +  EFI_STATUS  Status;
> +  EFI_DNS6_PROTOCOL   *Dns6;
> +  EFI_DNS6_CONFIG_DATADns6ConfigData;
> +  EFI_DNS6_COMPLETION_TOKEN   Token;
> +  EFI_HANDLE  Dns6Handle;
> +  EFI_IPv6_ADDRESS*DnsServerList;
> +  BOOLEAN IsDone;
> +
> +  Dns6= NULL;
> +  Dns6Handle  = NULL;
> +  DnsServerList   = Private->DnsServer;
> +  ZeroMem (, sizeof (EFI_DNS6_COMPLETION_TOKEN));
> +
> +  //
> +  // Create a DNSv6 child instance and get the protocol.
> +  //
> +  Status = 

[edk2] [PATCH] ArmVirtPkg: Drop the nonsense ASSERT() statement

[Dennis Chen]

Since All the GIC base address variables now are 64-bit, given
that a UNIT64 var cannot exceed MAX_UNIT64, it doesn't make sense
to continue keep them in the codes, so this patch just simply drop
those ASSERT() statements as it should be.

Contributed-under: TianoCore Contribution Agreement 1.0
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Cc: Laszlo Ersek 
Signed-off-by: Dennis Chen 
---
 ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c 
b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
index 64afc4d..7a312e5 100644
--- a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
+++ b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
@@ -79,11 +79,9 @@ ArmVirtGicArchLibConstructor (
 
 // RegProp[0..1] == { GICD base, GICD size }
 DistBase = SwapBytes64 (Reg[0]);
-ASSERT (DistBase < MAX_UINT32);
 
 // RegProp[2..3] == { GICR base, GICR size }
 RedistBase = SwapBytes64 (Reg[2]);
-ASSERT (RedistBase < MAX_UINT32);
 
 PcdSet64 (PcdGicDistributorBase, DistBase);
 PcdSet64 (PcdGicRedistributorsBase, RedistBase);
@@ -117,8 +115,6 @@ ArmVirtGicArchLibConstructor (
 
 DistBase = SwapBytes64 (Reg[0]);
 CpuBase  = SwapBytes64 (Reg[2]);
-ASSERT (DistBase < MAX_UINT32);
-ASSERT (CpuBase < MAX_UINT32);
 
 PcdSet64 (PcdGicDistributorBase, DistBase);
 PcdSet64 (PcdGicInterruptInterfaceBase, CpuBase);
-- 
2.7.4

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

[edk2] [PATCH] ArmPkg: Fix the GIC base address variables as 64-bit

[Dennis Chen]

Since ACPI spec defines the GIC base addresses (CPU interface,
Distributor and Redistributor*GICv3 only*) as 64-bit, so we should
define these corresponding base address variables as 64-bit instead of
32-bit. This patch redefines them according to the ACPI spec.

Contributed-under: TianoCore Contribution Agreement 1.0
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Signed-off-by: Dennis Chen 
---
 ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c 
b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
index b9ecd55..c7c5af1 100644
--- a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
+++ b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
@@ -30,8 +30,8 @@ Abstract:
 
 extern EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV2Protocol;
 
-STATIC UINT32 mGicInterruptInterfaceBase;
-STATIC UINT32 mGicDistributorBase;
+STATIC UINT64 mGicInterruptInterfaceBase;
+STATIC UINT64 mGicDistributorBase;
 
 /**
   Enable interrupt source Source.
-- 
2.7.4

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

[edk2] [patch 2/5] MdeModulePkg/BMMUI: Remove the incorrect and useless codes

[Dandan Bi]

When updating console page, the "ConsoleCheck" in BmmFakeNvData may maintain
the old uncommitted data, we should not copy it to BmmOldFakeNVData.
And in BootMaintRouteConfig function, when save data successfully,
it will copy the BmmFakeNvData to the BmmOldFakeNVData.
So we can delete the logic here.

Cc: Laszlo Ersek 
Cc: Liming Gao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 .../Library/BootMaintenanceManagerUiLib/UpdatePage.c | 12 
 1 file changed, 12 deletions(-)

diff --git a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c 
b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c
index ee8ff5d..960d0b0 100644
--- a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c
+++ b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c
@@ -457,46 +457,36 @@ UpdateConsolePage (
   BM_TERMINAL_CONTEXT *NewTerminalContext;
   UINT16  Index;
   UINT16  Index2;
   UINT8   CheckFlags;
   UINT8   *ConsoleCheck;
-  UINT8   *OldConsoleCheck;
-  UINTN   ConsoleCheckSize;
   EFI_QUESTION_ID QuestionIdBase;
   UINT16  VariableOffsetBase;
 
   CallbackData->BmmAskSaveOrNot = TRUE;
 
   UpdatePageStart (CallbackData);
 
   ConsoleCheck   = NULL;
-  OldConsoleCheck= NULL;
   QuestionIdBase = 0;
   VariableOffsetBase = 0;
-  ConsoleCheckSize   = 0;
 
   switch (UpdatePageId) {
   case FORM_CON_IN_ID:
 ConsoleCheck   = >BmmFakeNvData.ConsoleInCheck[0];
-OldConsoleCheck= >BmmOldFakeNVData.ConsoleInCheck[0];
-ConsoleCheckSize   = sizeof (CallbackData->BmmFakeNvData.ConsoleInCheck);
 QuestionIdBase = CON_IN_DEVICE_QUESTION_ID;
 VariableOffsetBase = CON_IN_DEVICE_VAR_OFFSET;
 break;
 
   case FORM_CON_OUT_ID:
 ConsoleCheck   = >BmmFakeNvData.ConsoleOutCheck[0];
-OldConsoleCheck= >BmmOldFakeNVData.ConsoleOutCheck[0];
-ConsoleCheckSize   = sizeof (CallbackData->BmmFakeNvData.ConsoleOutCheck);
 QuestionIdBase = CON_OUT_DEVICE_QUESTION_ID;
 VariableOffsetBase = CON_OUT_DEVICE_VAR_OFFSET;
 break;
 
   case FORM_CON_ERR_ID:
 ConsoleCheck   = >BmmFakeNvData.ConsoleErrCheck[0];
-OldConsoleCheck= >BmmOldFakeNVData.ConsoleErrCheck[0];
-ConsoleCheckSize   = sizeof (CallbackData->BmmFakeNvData.ConsoleErrCheck);
 QuestionIdBase = CON_ERR_DEVICE_QUESTION_ID;
 VariableOffsetBase = CON_ERR_DEVICE_VAR_OFFSET;
 break;
   }
   ASSERT (ConsoleCheck != NULL);
@@ -554,12 +544,10 @@ UpdateConsolePage (
   );
 
 Index++;
   }
 
-  CopyMem (OldConsoleCheck, ConsoleCheck, ConsoleCheckSize);
-
   UpdatePageEnd (CallbackData);
 }
 
 /**
   Update the page's NV Map if user has changed the order
-- 
1.9.5.msysgit.1

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

[edk2] [patch 4/5] MdeModulePkg/BMMUI: Show "Change Boot/Driver order" page correctly

[Dandan Bi]

When user enter the "Change Boot Order" page, the BootOptionOrder in
BmmFakeNvData may maintain some uncommitted data which are not saved
in "BootOrder" Variable and BootOptionMenu. So we should not always get
the BootOptionOrder through the function GetBootOrder, it will
result in incorrect UI behaviors. When the BootOptionOrder has not been
saved, we should use the BootOptionOrder in current BmmFakeNvData.

Cc: Laszlo Ersek 
Cc: Liming Gao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 .../Library/BootMaintenanceManagerUiLib/UpdatePage.c | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c 
b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c
index 29d3ac9..8194979 100644
--- a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c
+++ b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c
@@ -583,18 +583,34 @@ UpdateOrderPage (
   QuestionId = 0;
   VarOffset = 0;
   switch (UpdatePageId) { 
   
   case FORM_BOOT_CHG_ID:
-GetBootOrder (CallbackData);
+//
+// If the BootOptionOrder in the BmmFakeNvData are same with the date in 
the BmmOldFakeNVData,
+// means all Boot Options has been save in BootOptionMenu, we can get the 
date from the menu.
+// else means browser maintains some uncommitted date which are not saved 
in BootOptionMenu,
+// so we should not get the data from BootOptionMenu to show it.
+//
+if (CompareMem (CallbackData->BmmFakeNvData.BootOptionOrder, 
CallbackData->BmmOldFakeNVData.BootOptionOrder, sizeof 
(CallbackData->BmmFakeNvData.BootOptionOrder)) == 0) {
+  GetBootOrder (CallbackData);
+}
 OptionOrder = CallbackData->BmmFakeNvData.BootOptionOrder;
 QuestionId = BOOT_OPTION_ORDER_QUESTION_ID;
 VarOffset = BOOT_OPTION_ORDER_VAR_OFFSET;
 break;
 
   case FORM_DRV_CHG_ID:
-GetDriverOrder (CallbackData);
+//
+// If the DriverOptionOrder in the BmmFakeNvData are same with the date in 
the BmmOldFakeNVData,
+// means all Driver Options has been save in DriverOptionMenu, we can get 
the DriverOptionOrder from the menu.
+// else means browser maintains some uncommitted date which are not saved 
in DriverOptionMenu,
+// so we should not get the data from DriverOptionMenu to show it.
+//
+if (CompareMem (CallbackData->BmmFakeNvData.DriverOptionOrder, 
CallbackData->BmmOldFakeNVData.DriverOptionOrder, sizeof 
(CallbackData->BmmFakeNvData.DriverOptionOrder)) == 0) {
+  GetDriverOrder (CallbackData);
+}
 OptionOrder = CallbackData->BmmFakeNvData.DriverOptionOrder;
 QuestionId = DRIVER_OPTION_ORDER_QUESTION_ID;
 VarOffset = DRIVER_OPTION_ORDER_VAR_OFFSET;
 break;
   }  
-- 
1.9.5.msysgit.1

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

[edk2] [patch 3/5] MdeModulePkg/BMMUI: Make the BmmFakeNvData and BmmOldFakeNVData consistent

[Dandan Bi]

In BootMaintRouteConfig function, it will compare the data in BmmFakeNvData
and BmmOldFakeNVData to see whether there are some changes need to save.
In current codes when discarding changes or removing the useless changes,
it will update the related fields in BmmFakeNvData.
But also need to update related fields in BmmOldFakeNVData,
or it will result in incorrect comparison in BootMaintRouteConfig function,
then resulting in incorrect UI behaviors.

Cc: Laszlo Ersek 
Cc: Liming Gao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 .../Library/BootMaintenanceManagerUiLib/BootMaintenance.c  | 14 ++
 .../Library/BootMaintenanceManagerUiLib/UpdatePage.c   |  6 ++
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c 
b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c
index 92c44ea..7475a94 100644
--- a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c
+++ b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c
@@ -923,10 +923,11 @@ BootMaintCallback (
   )
 {
   BMM_CALLBACK_DATA *Private;
   BM_MENU_ENTRY *NewMenuEntry;
   BMM_FAKE_NV_DATA  *CurrentFakeNVMap;
+  BMM_FAKE_NV_DATA  *OldFakeNVMap;
   UINTN Index;
   EFI_DEVICE_PATH_PROTOCOL * File;
 
   if (Action != EFI_BROWSER_ACTION_CHANGING && Action != 
EFI_BROWSER_ACTION_CHANGED && Action != EFI_BROWSER_ACTION_FORM_OPEN) {
 //
@@ -957,10 +958,11 @@ BootMaintCallback (
   }
   //
   // Retrive uncommitted data from Form Browser
   //
   CurrentFakeNVMap = >BmmFakeNvData;
+  OldFakeNVMap = >BmmOldFakeNVData;
   HiiGetBrowserData (, mBootMaintStorageName, sizeof 
(BMM_FAKE_NV_DATA), (UINT8 *) CurrentFakeNVMap);
 
   if (Action == EFI_BROWSER_ACTION_CHANGING) {
 if (Value == NULL) {
   return EFI_INVALID_PARAMETER;
@@ -1059,21 +1061,25 @@ BootMaintCallback (
   *ActionRequest = EFI_BROWSER_ACTION_REQUEST_FORM_SUBMIT_EXIT;
 } else if (QuestionId == KEY_VALUE_NO_SAVE_AND_EXIT_DRIVER) {
   //
   // Discard changes and exit formset
   //
-  CurrentFakeNVMap->DriverOptionalData[0] = 0x;
-  CurrentFakeNVMap->DriverDescriptionData[0]  = 0x;
+  ZeroMem (CurrentFakeNVMap->DriverOptionalData, sizeof 
(CurrentFakeNVMap->DriverOptionalData));
+  ZeroMem (CurrentFakeNVMap->BootDescriptionData, sizeof 
(CurrentFakeNVMap->BootDescriptionData));
+  ZeroMem (OldFakeNVMap->DriverOptionalData, sizeof 
(OldFakeNVMap->DriverOptionalData));
+  ZeroMem (OldFakeNVMap->DriverDescriptionData, sizeof 
(OldFakeNVMap->DriverDescriptionData));
   CurrentFakeNVMap->DriverOptionChanged = FALSE;
   CurrentFakeNVMap->ForceReconnect  = TRUE;
   *ActionRequest = EFI_BROWSER_ACTION_REQUEST_FORM_DISCARD_EXIT;
 } else if (QuestionId == KEY_VALUE_NO_SAVE_AND_EXIT_BOOT) {
   //
   // Discard changes and exit formset
   //
-  CurrentFakeNVMap->BootOptionalData[0] = 0x;
-  CurrentFakeNVMap->BootDescriptionData[0]  = 0x;
+  ZeroMem (CurrentFakeNVMap->BootOptionalData, sizeof 
(CurrentFakeNVMap->BootOptionalData));
+  ZeroMem (CurrentFakeNVMap->BootDescriptionData, sizeof 
(CurrentFakeNVMap->BootDescriptionData));
+  ZeroMem (OldFakeNVMap->BootOptionalData, sizeof 
(OldFakeNVMap->BootOptionalData));
+  ZeroMem (OldFakeNVMap->BootDescriptionData, sizeof 
(OldFakeNVMap->BootDescriptionData));
   CurrentFakeNVMap->BootOptionChanged = FALSE;
   *ActionRequest = EFI_BROWSER_ACTION_REQUEST_FORM_DISCARD_EXIT;
 } else if (QuestionId == KEY_VALUE_BOOT_DESCRIPTION || QuestionId == 
KEY_VALUE_BOOT_OPTION) {
   CurrentFakeNVMap->BootOptionChanged = TRUE;
 } else if (QuestionId == KEY_VALUE_DRIVER_DESCRIPTION || QuestionId == 
KEY_VALUE_DRIVER_OPTION) {
diff --git a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c 
b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c
index 960d0b0..29d3ac9 100644
--- a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c
+++ b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c
@@ -260,10 +260,11 @@ UpdateBootDelPage (
   // CallbackData->BmmFakeNvData.BootOptionDelMark[Index] = FALSE means 
BDS knows the selected boot option has
   // deleted, browser maintains old useless info. So clear this info here, 
and later update this info to browser
   // through HiiSetBrowserData function.
   //
   CallbackData->BmmFakeNvData.BootOptionDel[Index] = FALSE;
+  CallbackData->BmmOldFakeNVData.BootOptionDel[Index] = FALSE;
 }
 
 HiiCreateCheckBoxOpCode (
   mStartOpCodeHandle,
   (EFI_QUESTION_ID) (BOOT_OPTION_DEL_QUESTION_ID + Index),
@@ -346,10 +347,11 @@ UpdateDrvDelPage (
   // CallbackData->BmmFakeNvData.BootOptionDelMark[Index] = FALSE means 
BDS 

[edk2] [patch 5/5] MdeModulePkg/BMMUI: Add error handling codes

[Dandan Bi]

The function which handles the "Boot", "BootOrder" ...
may return failure. This patch adds the error handling codes.
return the failure info to browser.

Cc: Laszlo Ersek 
Cc: Liming Gao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 .../BootMaintenanceManagerUiLib/BootMaintenance.c  | 201 ++---
 .../Library/BootMaintenanceManagerUiLib/Variable.c |  28 ++-
 2 files changed, 202 insertions(+), 27 deletions(-)

diff --git a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c 
b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c
index 7475a94..33c85b7 100644
--- a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c
+++ b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c
@@ -442,10 +442,95 @@ BmmExtractDevicePathFromHiiHandle (
   return ConvertDevicePathToText(DevicePathFromHandle (DriverHandle), FALSE, 
FALSE);
 
 }
 
 /**
+  Converts the unicode character of the string from uppercase to lowercase.
+  This is a internal function.
+
+  @param ConfigString  String to be converted
+
+**/
+VOID
+HiiToLower (
+  IN EFI_STRING  ConfigString
+  )
+{
+  EFI_STRING  String;
+  BOOLEAN Lower;
+
+  ASSERT (ConfigString != NULL);
+
+  //
+  // Convert all hex digits in range [A-F] in the configuration header to [a-f]
+  //
+  for (String = ConfigString, Lower = FALSE; *String != L'\0'; String++) {
+if (*String == L'=') {
+  Lower = TRUE;
+} else if (*String == L'&') {
+  Lower = FALSE;
+} else if (Lower && *String >= L'A' && *String <= L'F') {
+  *String = (CHAR16) (*String - L'A' + L'a');
+}
+  }
+}
+
+/**
+  Update the progress string through the offset value.
+
+  @param Offset   The offset value
+  @param ConfigurationPoint to the configuration string.
+
+**/
+EFI_STRING
+UpdateProgress(
+  IN  UINTN   Offset,
+  IN  EFI_STRING  Configuration
+)
+{
+  UINTN   Length;
+  EFI_STRING  StringPtr;
+  EFI_STRING  ReturnString;
+
+  StringPtr= NULL;
+  ReturnString = NULL;
+
+  //
+  // = followed by a Null-terminator.
+  // Length = StrLen (L"=") + 4 + 1
+  //
+  Length= StrLen (L"=") + 4 + 1;
+
+  StringPtr = AllocateZeroPool (Length * sizeof (CHAR16));
+
+  if (StringPtr == NULL) {
+return  NULL;
+  }
+
+  UnicodeSPrint (
+StringPtr,
+(8 + 4 + 1) * sizeof (CHAR16),
+L"=%04x",
+Offset
+);
+
+  ReturnString = StrStr (Configuration, StringPtr);
+
+  if (ReturnString == NULL) {
+//
+// If doesn't find the string in Configuration, convert the string to 
lower case then search again.
+//
+HiiToLower (StringPtr);
+ReturnString = StrStr (Configuration, StringPtr);
+  }
+
+  FreePool (StringPtr);
+
+  return ReturnString;
+}
+
+/**
   Update the terminal content in TerminalMenu.
 
   @param BmmData   The BMM fake NV data.
 
 **/
@@ -693,11 +778,12 @@ BootMaintRouteConfig (
   BMM_FAKE_NV_DATA*OldBmmData;
   BM_MENU_ENTRY   *NewMenuEntry;
   BM_LOAD_CONTEXT *NewLoadContext;
   UINT16  Index;
   BOOLEAN TerminalAttChange;
-  BMM_CALLBACK_DATA   *Private; 
+  BMM_CALLBACK_DATA   *Private;
+  UINTN   Offset;
 
   if (Progress == NULL) {
 return EFI_INVALID_PARAMETER;
   }
   *Progress = Configuration;
@@ -728,10 +814,11 @@ BootMaintRouteConfig (
   // Get Buffer Storage data from EFI variable
   //
   BufferSize = sizeof (BMM_FAKE_NV_DATA);
   OldBmmData = >BmmOldFakeNVData;
   NewBmmData = >BmmFakeNvData;
+  Offset = 0;
   //
   // Convert  to buffer data by helper function ConfigToBlock()
   //
   Status = ConfigRouting->ConfigToBlock (
 ConfigRouting,
@@ -749,10 +836,14 @@ BootMaintRouteConfig (
   //
   // Check data which located in BMM main page and save the settings if need
   // 
   if (CompareMem (>BootNext, >BootNext, sizeof 
(NewBmmData->BootNext)) != 0) {
 Status = Var_UpdateBootNext (Private);
+if (EFI_ERROR (Status)) {
+  Offset = OFFSET_OF (BMM_FAKE_NV_DATA, BootNext);
+  goto Exit;
+}
   }
 
   //
   // Check data which located in Boot Options Menu and save the settings if 
need
   //  
@@ -765,15 +856,23 @@ BootMaintRouteConfig (
   NewLoadContext->Deleted = NewBmmData->BootOptionDel[Index];
   NewBmmData->BootOptionDel[Index] = FALSE;
   NewBmmData->BootOptionDelMark[Index] = FALSE;
 }
 
-Var_DelBootOption ();
+Status = Var_DelBootOption ();
+if (EFI_ERROR (Status)) {
+  Offset = OFFSET_OF (BMM_FAKE_NV_DATA, BootOptionDel);
+  goto Exit;
+}
   }
 
   if (CompareMem (NewBmmData->BootOptionOrder, OldBmmData->BootOptionOrder, 
sizeof (NewBmmData->BootOptionOrder)) != 0) {
 Status = Var_UpdateBootOrder (Private);
+if (EFI_ERROR 

[edk2] [patch 1/5] MdeModulePkg/BMMUI: Update TerminalMenu and ConsoleMenu in callback

[Dandan Bi]

In current codes, When user does some change related to Console or Terminal,
when saving data, it will update the content in TerminalMenu and ConsoleMenu
in BootMaintRouteConfig function. This patch moves the update action to the
BootMaintCallback function with EFI_BROWSER_ACTION_CHANGED type.
The reason for this change is: in BootMaintRouteConfig function when
Var_UpdateConsoleXXXOption() return failure and user discard the previous
change, we should re_update the content in the TerminalMenu and ConsoleMenu.
So we move the update action to the changed callback.

Cc: Laszlo Ersek 
Cc: Liming Gao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 .../BootMaintenanceManagerUiLib/BootMaintenance.c  | 177 ++---
 .../BootMaintenanceManagerUiLib/UpdatePage.c   |  16 +-
 2 files changed, 126 insertions(+), 67 deletions(-)

diff --git a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c 
b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c
index a190596..92c44ea 100644
--- a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c
+++ b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c
@@ -442,10 +442,112 @@ BmmExtractDevicePathFromHiiHandle (
   return ConvertDevicePathToText(DevicePathFromHandle (DriverHandle), FALSE, 
FALSE);
 
 }
 
 /**
+  Update the terminal content in TerminalMenu.
+
+  @param BmmData   The BMM fake NV data.
+
+**/
+VOID
+UpdateTerminalContent (
+  IN BMM_FAKE_NV_DATA   *BmmData
+  )
+{
+  UINT16  Index;
+  BM_TERMINAL_CONTEXT *NewTerminalContext;
+  BM_MENU_ENTRY   *NewMenuEntry;
+
+  for (Index = 0; Index < TerminalMenu.MenuNumber; Index++) {
+NewMenuEntry = BOpt_GetMenuEntry (, Index);
+ASSERT (NewMenuEntry != NULL);
+NewTerminalContext = (BM_TERMINAL_CONTEXT *) NewMenuEntry->VariableContext;
+NewTerminalContext->BaudRateIndex = BmmData->COMBaudRate[Index];
+ASSERT (BmmData->COMBaudRate[Index] < (sizeof (BaudRateList) / sizeof 
(BaudRateList[0])));
+NewTerminalContext->BaudRate  = 
BaudRateList[BmmData->COMBaudRate[Index]].Value;
+NewTerminalContext->DataBitsIndex = BmmData->COMDataRate[Index];
+ASSERT (BmmData->COMDataRate[Index] < (sizeof (DataBitsList) / sizeof 
(DataBitsList[0])));
+NewTerminalContext->DataBits  = (UINT8) 
DataBitsList[BmmData->COMDataRate[Index]].Value;
+NewTerminalContext->StopBitsIndex = BmmData->COMStopBits[Index];
+ASSERT (BmmData->COMStopBits[Index] < (sizeof (StopBitsList) / sizeof 
(StopBitsList[0])));
+NewTerminalContext->StopBits  = (UINT8) 
StopBitsList[BmmData->COMStopBits[Index]].Value;
+NewTerminalContext->ParityIndex   = BmmData->COMParity[Index];
+ASSERT (BmmData->COMParity[Index] < (sizeof (ParityList) / sizeof 
(ParityList[0])));
+NewTerminalContext->Parity= (UINT8) 
ParityList[BmmData->COMParity[Index]].Value;
+NewTerminalContext->TerminalType  = BmmData->COMTerminalType[Index];
+NewTerminalContext->FlowControl   = BmmData->COMFlowControl[Index];
+ChangeTerminalDevicePath (
+  NewTerminalContext->DevicePath,
+  FALSE
+  );
+  }
+}
+
+/**
+  Update the console content in ConsoleMenu.
+
+  @param BmmData   The BMM fake NV data.
+
+**/
+VOID
+UpdateConsoleContent(
+  IN CHAR16 *ConsoleName,
+  IN BMM_FAKE_NV_DATA   *BmmData
+  )
+{
+  UINT16  Index;
+  BM_CONSOLE_CONTEXT  *NewConsoleContext;
+  BM_TERMINAL_CONTEXT *NewTerminalContext;
+  BM_MENU_ENTRY   *NewMenuEntry;
+
+  if (StrCmp (ConsoleName, L"ConIn") == 0) {
+for (Index = 0; Index < ConsoleInpMenu.MenuNumber; Index++){
+  NewMenuEntry= BOpt_GetMenuEntry(, Index);
+  NewConsoleContext   = (BM_CONSOLE_CONTEXT 
*)NewMenuEntry->VariableContext;
+  ASSERT (Index < MAX_MENU_NUMBER);
+  NewConsoleContext->IsActive = BmmData->ConsoleInCheck[Index];
+}
+for (Index = 0; Index < TerminalMenu.MenuNumber; Index++) {
+  NewMenuEntry= BOpt_GetMenuEntry (, Index);
+  NewTerminalContext  = (BM_TERMINAL_CONTEXT *) 
NewMenuEntry->VariableContext;
+  ASSERT (Index + ConsoleInpMenu.MenuNumber < MAX_MENU_NUMBER);
+  NewTerminalContext->IsConIn = BmmData->ConsoleInCheck[Index + 
ConsoleInpMenu.MenuNumber];
+}
+  }
+
+  if (StrCmp (ConsoleName, L"ConOut") == 0) {
+for (Index = 0; Index < ConsoleOutMenu.MenuNumber; Index++){
+  NewMenuEntry= BOpt_GetMenuEntry(, Index);
+  NewConsoleContext   = (BM_CONSOLE_CONTEXT 
*)NewMenuEntry->VariableContext;
+  ASSERT (Index < MAX_MENU_NUMBER);
+  NewConsoleContext->IsActive = BmmData->ConsoleOutCheck[Index];
+}
+for (Index = 0; Index < TerminalMenu.MenuNumber; Index++) 

Re: [edk2] [PATCH] ArmPkg ArmVirtPkg: fix the GIC base address variables as 64-bit

[Leif Lindholm]

On Mon, Oct 17, 2016 at 08:28:50AM +0100, Ard Biesheuvel wrote:
> > diff --git a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c 
> > b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> > index 64afc4d..16683ef 100644
> > --- a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> > +++ b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> > @@ -79,11 +79,11 @@ ArmVirtGicArchLibConstructor (
> >
> >  // RegProp[0..1] == { GICD base, GICD size }
> >  DistBase = SwapBytes64 (Reg[0]);
> > -ASSERT (DistBase < MAX_UINT32);
> > +ASSERT (DistBase < MAX_UINT64);
> >
> 
> This becomes equivalent to 'DistBase != MAX_UINT64' given that a
> UINT64 cannot exceed MAX_UINT64. That is a nonsensical thing to
> assert, so it is better to simply drop it

Random thought:
Could we keep the assert(s) and change the test to MAX_UINTN, to have
a sanity test over whether a 32-bit plaform ends up with a duff
address?

/
Leif

> >  // RegProp[2..3] == { GICR base, GICR size }
> >  RedistBase = SwapBytes64 (Reg[2]);
> > -ASSERT (RedistBase < MAX_UINT32);
> > +ASSERT (RedistBase < MAX_UINT64);
> >
> 
> Likewise
> 
> >  PcdSet64 (PcdGicDistributorBase, DistBase);
> >  PcdSet64 (PcdGicRedistributorsBase, RedistBase);
> > @@ -117,8 +117,8 @@ ArmVirtGicArchLibConstructor (
> >
> >  DistBase = SwapBytes64 (Reg[0]);
> >  CpuBase  = SwapBytes64 (Reg[2]);
> > -ASSERT (DistBase < MAX_UINT32);
> > -ASSERT (CpuBase < MAX_UINT32);
> > +ASSERT (DistBase < MAX_UINT64);
> > +ASSERT (CpuBase < MAX_UINT64);
> >
> 
> Likewise
> 
> >  PcdSet64 (PcdGicDistributorBase, DistBase);
> >  PcdSet64 (PcdGicInterruptInterfaceBase, CpuBase);
> > --
> > 2.7.4
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch 2/3] BaseTools: Update PatchCheck to handle the two [] as prefix

[Yonghong Zhu]

The bug is that only remove the first [] when it does the char count,
however sometimes we use [edk2][patch] as prefix, this patch fix this bug.

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

Cc: Liming Gao 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Scripts/PatchCheck.py | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
index 07fca68..05f8f6e 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -434,10 +434,18 @@ class CheckOnePatch:
[\s\S\r\n]+
)
''',
re.IGNORECASE | re.VERBOSE | re.MULTILINE)
 
+subject_prefix_re = \
+re.compile(r'''^
+   \s* (\[
+[^\[\]]* # Allow all non-brackets
+   \])* \s*
+   ''',
+   re.VERBOSE)
+
 def find_patch_pieces(self):
 if sys.version_info < (3, 0):
 patch = self.patch.encode('ascii', 'ignore')
 else:
 patch = self.patch
@@ -470,18 +478,11 @@ class CheckOnePatch:
 self.stat = mo.group('stat')
 self.commit_msg = mo.group('commit_message')
 
 self.commit_subject = pmail['subject'].replace('\r\n', '')
 self.commit_subject = self.commit_subject.replace('\n', '')
-
-pfx_start = self.commit_subject.find('[')
-if pfx_start >= 0:
-pfx_end = self.commit_subject.find(']')
-if pfx_end > pfx_start:
-self.commit_prefix = self.commit_subject[pfx_start + 1 : 
pfx_end]
-self.commit_subject = self.commit_subject[pfx_end + 1 
:].lstrip()
-
+self.commit_subject = self.subject_prefix_re.sub('', 
self.commit_subject, 1)
 
 class CheckGitCommits:
 """Reads patches from git based on the specified git revision range.
 
 The patches are read from git, and then checked.
-- 
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] BaseTools: Update PatchCheck for max length of subject and message line

[Yonghong Zhu]

This patch update PatchCheck.py:
1. The subject line of the commit message should be < 72 characters.
2. The other lines of the commit message should be < 76 characters.

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

Cc: Liming Gao 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Scripts/PatchCheck.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
index 455c130..07fca68 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -1,9 +1,9 @@
 ## @file
 #  Check a patch for various format issues
 #
-#  Copyright (c) 2015, Intel Corporation. All rights reserved.
+#  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
 #
 #  This program and the accompanying materials are licensed and made
 #  available under the terms and conditions of the BSD License which
 #  accompanies this distribution. The full text of the license may be
 #  found at http://opensource.org/licenses/bsd-license.php
@@ -14,11 +14,11 @@
 #
 
 from __future__ import print_function
 
 VersionNumber = '0.1'
-__copyright__ = "Copyright (c) 2015, Intel Corporation  All rights reserved."
+__copyright__ = "Copyright (c) 2015 - 2016, Intel Corporation  All rights 
reserved."
 
 import email
 import argparse
 import os
 import re
@@ -195,11 +195,11 @@ class CommitMessageCheck:
 
 if count <= 0:
 self.error('Empty commit message!')
 return
 
-if count >= 1 and len(lines[0]) > 76:
+if count >= 1 and len(lines[0]) >= 72:
 self.error('First line of commit message (subject line) ' +
'is too long.')
 
 if count >= 1 and len(lines[0].strip()) == 0:
 self.error('First line of commit message (subject line) ' +
@@ -208,11 +208,11 @@ class CommitMessageCheck:
 if count >= 2 and lines[1].strip() != '':
 self.error('Second line of commit message should be ' +
'empty.')
 
 for i in range(2, count):
-if (len(lines[i]) > 76 and
+if (len(lines[i]) >= 76 and
 len(lines[i].split()) > 1 and
 not lines[i].startswith('git-svn-id:')):
 self.error('Line %d of commit message is too long.' % (i + 1))
 
 last_sig_line = None
-- 
2.6.1.windows.1

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

[edk2] [Patch 3/3] BaseTools: Update PatchCheck to report error for EFI_D_*

[Yonghong Zhu]

In EDK2, DEBUG_* is recommended to be used instead of EFI_D_*. For new
code, they should use DEBUG_* macro.

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

Cc: Liming Gao 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Scripts/PatchCheck.py | 8 
 1 file changed, 8 insertions(+)

diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
index 05f8f6e..3ac0769 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -337,10 +337,15 @@ class GitDiffCheck:
 if self.hunk_filename is not None:
 lines.append('File: ' + self.hunk_filename)
 lines.append('Line: ' + line)
 
 self.error(*lines)
+
+DEBUG_macro_re = re.compile(r'''^
+\s*DEBUG\s*\(\(
+''',
+re.VERBOSE)
 
 def check_added_line(self, line):
 eol = ''
 for an_eol in self.line_endings:
 if line.endswith(an_eol):
@@ -354,10 +359,13 @@ class GitDiffCheck:
   line)
 if '\t' in line:
 self.added_line_error('Tab character used', line)
 if len(stripped) < len(line):
 self.added_line_error('Trailing whitespace found', line)
+if self.DEBUG_macro_re.match(line):
+if 'EFI_D_' in line:
+self.added_line_error('EFI_D_* format is used, recommend to 
use DEBUG_* format', line)
 
 split_diff_re = re.compile(r'''
(?P
^ diff \s+ --git \s+ a/.+ \s+ b/.+ $
)
-- 
2.6.1.windows.1

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

[edk2] [Patch 0/3] BaseTools: PatchCheck to align with wiki and report error for EFI_D_*

[Yonghong Zhu]

These patches address the following 2 bugs:
https://bugzilla.tianocore.org/show_bug.cgi?id=113
https://bugzilla.tianocore.org/show_bug.cgi?id=145

Cc: Liming Gao 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 

Yonghong Zhu (3):
  BaseTools: Update PatchCheck for max length of subject and message
line
  BaseTools: Update PatchCheck to handle the two [] as prefix
  BaseTools: Update PatchCheck to report error for EFI_D_*

 BaseTools/Scripts/PatchCheck.py | 27 +++
 1 file changed, 15 insertions(+), 12 deletions(-)

-- 
2.6.1.windows.1

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

Re: [edk2] [PATCH 00/52] Resolve issues for C source codes in BaseTools

[Gao, Liming]

Hao, 
  I have some comments for three patches. Others are good to me. 

Patch: BaseTools/TianoCompress: Avoid possible NULL pointer dereference
Comment:  Please free allocated buffer after error happens. 

Patch: BaseTools/C/Common: Fix parameter format mismatch in scanf functions
Comment:  Please add more description to say why INT32 is used in sscanf.

Patch: BaseTools/VolInfo: Use hard-coded format string for calls to sprintf()
Comment: Why the format string may be changed accidentally?

Thanks
Liming
> -Original Message-
> From: Wu, Hao A
> Sent: Wednesday, October 12, 2016 8:20 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Gao, Liming ;
> Zhu, Yonghong ; Dong, Eric
> ; Bi, Dandan 
> Subject: [PATCH 00/52] Resolve issues for C source codes in BaseTools
> 
> The patch series fixes the following types of issues for C source codes in
> BaseTools:
> 
> 1. Avoid possible NULL pointer dereference
> 2. Initialize local variables before use
> 3. Remove unused local variables
> 4. Avoid accessing over array bounds
> 5. Resolve possible memory leak
> 6. Resolve file handles not being closed
> 7. Resolve possible buffer overflow in printf/scanf functions
> 
> The patch series is also available at:
> https://github.com/hwu25/edk2/tree/BaseTools_V1
> 
> Cc: Liming Gao 
> Cc: Yonghong Zhu 
> Cc: Eric Dong 
> Cc: Dandan Bi 
> 
> Hao Wu (52):
>   BaseTools/C/Common: Avoid possible NULL pointer dereference
>   BaseTools/EfiRom: Avoid possible NULL pointer dereference
>   BaseTools/GenFfs: Avoid possible NULL pointer dereference
>   BaseTools/GenFv: Avoid possible NULL pointer dereference
>   BaseTools/GenFw: Avoid possible NULL pointer dereference
>   BaseTools/GenPage: Avoid possible NULL pointer dereference
>   BaseTools/GenSec: Avoid possible NULL pointer dereference
>   BaseTools/GenVtf: Avoid possible NULL pointer dereference
>   BaseTools/TianoCompress: Avoid possible NULL pointer dereference
>   BaseTools/VfrCompile: Avoid possible NULL pointer dereference
>   BaseTools/VolInfo: Avoid possible NULL pointer dereference
>   BaseTools/TianoCompress: Initialize local variables before being used
>   BaseTools/VfrCompile: Initialize local variables before being used
>   BaseTools/GenBootSector: Fix parameter format mismatch in printf
> functions
>   BaseTools/VolInfo: Fix parameter format mismatch in printf functions
>   BaseTools/C/Common: Fix parameter format mismatch in scanf functions
>   BaseTools/GenFv: Fix parameter format mismatch in scanf functions
>   BaseTools/GenFw: Fix parameter format mismatch in scanf functions
>   BaseTools/GenVtf: Fix parameter format mismatch in scanf functions
>   BaseTools/C/Common: Fix potential access over array bounds
>   BaseTools/EfiRom: Fix potential access over array bounds
>   BaseTools/GenFv: Fix potential access over array bounds
>   BaseTools/TianoCompress: Fix potential access over array bounds
>   BaseTools/VfrCompile: Fix potential access over array bounds
>   BaseTools/VfrCompile: Avoid freeing memory with mismatched functions
>   BaseTools/VfrCompile: Add assignment operator definition for some
> classes
>   BaseTools/VfrCompile: Avoid freeing freed memory in classes
>   BaseTools/VfrCompile: Remove unused local variables
>   BaseTools/C/Common: Fix potential memory leak
>   BaseTools/EfiRom: Fix potential memory leak
>   BaseTools/GenFv: Fix potential memory leak
>   BaseTools/GenPage: Fix potential memory leak
>   BaseTools/GenSec: Fix potential memory leak
>   BaseTools/GenVtf: Fix potential memory leak
>   BaseTools/Split: Fix potential memory and resource leak
>   BaseTools/TianoCompress: Fix potential memory leak
>   BaseTools/VfrCompile: Fix potential memory leak
>   BaseTools/VolInfo: Fix potential memory leak
>   BaseTools/EfiRom: Fix file handles not being closed
>   BaseTools/GenBootSector: Fix file handles not being closed
>   BaseTools/GenCrc32: Fix file handles not being closed
>   BaseTools/GenFv: Fix file handles not being closed
>   BaseTools/GenVtf: Fix file handles not being closed
>   BaseTools/LzmaCompress: Fix file handles not being closed
>   BaseTools/TianoCompress: Fix file handles not being closed
>   BaseTools/VolInfo: Fix file handles not being closed
>   BaseTools/GenVtf: Fix potential buffer overflow in scanf functions
>   BaseTools/VolInfo: Fix potential buffer overflow in scanf functions
>   BaseTools/VfrCompile: Explicitly state format string for DebugMsg()
>   BaseTools/VolInfo: Use hard-coded format string for calls to sprintf()
>   BaseTools/VfrCompile/Pccts: Add virtual destructor for class
> DLGInputStream
>   BaseTools/VfrCompile/Pccts: Make assignment operator not returning
> void
> 
>  BaseTools/Source/C/Common/BasePeCoff.c |  12 ++
>  BaseTools/Source/C/Common/CommonLib.c  |   8 

Re: [edk2] [PATCH] ArmPkg ArmVirtPkg: fix the GIC base address variables as 64-bit

[Dennis Chen]

Hello Ard,

Thanks for the comments! I will split this patch into 2 and for ArmVirtPkg 
patch,
we just need to simply drop the original ASSERT() since it's nonsensical any 
more.

Thanks,
Dennis

On Mon, Oct 17, 2016 at 08:28:50AM +0100, Ard Biesheuvel wrote:
> Hi Dennis,
> 
> On 17 October 2016 at 06:03, Dennis Chen  wrote:
> > Since ACPI spec defines the GIC base addresses (CPU interface,
> > Distributor and Redistributor*GICv3 only*) as 64-bit, so we
> > should define these corresponding base address variables as 64-bit
> > instead of 32-bit. This patch redefines them according to the
> > ACPI spec.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Cc: Ard Biesheuvel 
> > Cc: Leif Lindholm 
> > Signed-off-by: Dennis Chen 
> > ---
> >  ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c| 4 ++--
> >  ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c | 8 
> 
> Could you split this patch in 2 please, and put Laszlo Ersek on cc for
> the ArmVirtPkg patch?
> 
> 
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c 
> > b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> > index b9ecd55..a4ba5cf 100644
> > --- a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> > +++ b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> > @@ -30,8 +30,8 @@ Abstract:
> >
> >  extern EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV2Protocol;
> >
> > -STATIC UINT32 mGicInterruptInterfaceBase;
> > -STATIC UINT32 mGicDistributorBase;
> > +STATIC UINTN mGicInterruptInterfaceBase;
> > +STATIC UINTN mGicDistributorBase;
> >
> 
> This should be UINT64 not UINTN
> 
> >  /**
> >Enable interrupt source Source.
> > diff --git a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c 
> > b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> > index 64afc4d..16683ef 100644
> > --- a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> > +++ b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> > @@ -79,11 +79,11 @@ ArmVirtGicArchLibConstructor (
> >
> >  // RegProp[0..1] == { GICD base, GICD size }
> >  DistBase = SwapBytes64 (Reg[0]);
> > -ASSERT (DistBase < MAX_UINT32);
> > +ASSERT (DistBase < MAX_UINT64);
> >
> 
> This becomes equivalent to 'DistBase != MAX_UINT64' given that a
> UINT64 cannot exceed MAX_UINT64. That is a nonsensical thing to
> assert, so it is better to simply drop it
> 
> >  // RegProp[2..3] == { GICR base, GICR size }
> >  RedistBase = SwapBytes64 (Reg[2]);
> > -ASSERT (RedistBase < MAX_UINT32);
> > +ASSERT (RedistBase < MAX_UINT64);
> >
> 
> Likewise
> 
> >  PcdSet64 (PcdGicDistributorBase, DistBase);
> >  PcdSet64 (PcdGicRedistributorsBase, RedistBase);
> > @@ -117,8 +117,8 @@ ArmVirtGicArchLibConstructor (
> >
> >  DistBase = SwapBytes64 (Reg[0]);
> >  CpuBase  = SwapBytes64 (Reg[2]);
> > -ASSERT (DistBase < MAX_UINT32);
> > -ASSERT (CpuBase < MAX_UINT32);
> > +ASSERT (DistBase < MAX_UINT64);
> > +ASSERT (CpuBase < MAX_UINT64);
> >
> 
> Likewise
> 
> >  PcdSet64 (PcdGicDistributorBase, DistBase);
> >  PcdSet64 (PcdGicInterruptInterfaceBase, CpuBase);
> > --
> > 2.7.4
> >
> 

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

Re: [edk2] [PATCH 1/1] MdePkg/IoLib: Add support for big-endian MMIO

[Ard Biesheuvel]

On 17 October 2016 at 05:10, Kinney, Michael D
 wrote:
> Bhupesh,
>
> It is also possible to add an ARM specific PCD to select endianness and update
> MdePkg/Library/BaseIoLibIntrinsic/IoLibArm.c to use that PCD in 
> MmioRead/Write()
> APIs in that file to support both endian types.  You can use the SwapBytesxx()
> functions from BaseLib(as Laszlo suggested) based on the setting of this ARM
> specific PCD.
>
> Modules that link against this lib can select endianness by setting PCD in the
> scope of that module.
>
> The IPF version of IoLib uses an IPF specific PCD to translate I/O port 
> accesses
> to MMIO accesses.  So there is already an example of an arch specific PCD in 
> this
> lib instance.
>

This is not a platform wide thing, it is a per-device property whether
the MMIO occurs in big endian or little endian manner.

So I think Liming's suggestion makes sense: create an IoLib
implementation that performs the byte swapping, and selectively
incorporate it into drivers that require it using

BeMmioDeviceDxe.inf {
  
IoLib|SomePkg/Library/BigEndianIoLib.inf
}
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg ArmVirtPkg: fix the GIC base address variables as 64-bit

[Ard Biesheuvel]

Hi Dennis,

On 17 October 2016 at 06:03, Dennis Chen  wrote:
> Since ACPI spec defines the GIC base addresses (CPU interface,
> Distributor and Redistributor*GICv3 only*) as 64-bit, so we
> should define these corresponding base address variables as 64-bit
> instead of 32-bit. This patch redefines them according to the
> ACPI spec.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Cc: Ard Biesheuvel 
> Cc: Leif Lindholm 
> Signed-off-by: Dennis Chen 
> ---
>  ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c| 4 ++--
>  ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c | 8 

Could you split this patch in 2 please, and put Laszlo Ersek on cc for
the ArmVirtPkg patch?


>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c 
> b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> index b9ecd55..a4ba5cf 100644
> --- a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> +++ b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> @@ -30,8 +30,8 @@ Abstract:
>
>  extern EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV2Protocol;
>
> -STATIC UINT32 mGicInterruptInterfaceBase;
> -STATIC UINT32 mGicDistributorBase;
> +STATIC UINTN mGicInterruptInterfaceBase;
> +STATIC UINTN mGicDistributorBase;
>

This should be UINT64 not UINTN

>  /**
>Enable interrupt source Source.
> diff --git a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c 
> b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> index 64afc4d..16683ef 100644
> --- a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> +++ b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
> @@ -79,11 +79,11 @@ ArmVirtGicArchLibConstructor (
>
>  // RegProp[0..1] == { GICD base, GICD size }
>  DistBase = SwapBytes64 (Reg[0]);
> -ASSERT (DistBase < MAX_UINT32);
> +ASSERT (DistBase < MAX_UINT64);
>

This becomes equivalent to 'DistBase != MAX_UINT64' given that a
UINT64 cannot exceed MAX_UINT64. That is a nonsensical thing to
assert, so it is better to simply drop it

>  // RegProp[2..3] == { GICR base, GICR size }
>  RedistBase = SwapBytes64 (Reg[2]);
> -ASSERT (RedistBase < MAX_UINT32);
> +ASSERT (RedistBase < MAX_UINT64);
>

Likewise

>  PcdSet64 (PcdGicDistributorBase, DistBase);
>  PcdSet64 (PcdGicRedistributorsBase, RedistBase);
> @@ -117,8 +117,8 @@ ArmVirtGicArchLibConstructor (
>
>  DistBase = SwapBytes64 (Reg[0]);
>  CpuBase  = SwapBytes64 (Reg[2]);
> -ASSERT (DistBase < MAX_UINT32);
> -ASSERT (CpuBase < MAX_UINT32);
> +ASSERT (DistBase < MAX_UINT64);
> +ASSERT (CpuBase < MAX_UINT64);
>

Likewise

>  PcdSet64 (PcdGicDistributorBase, DistBase);
>  PcdSet64 (PcdGicInterruptInterfaceBase, CpuBase);
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] GPT Shell Application/Library

[Michael Zimmermann]

Hi,

wouldn't it be better to make a generic gpt parsing library which is
independent of the shell so both the shell and PartitionDxe can use it?
It may also be useful for other applications which need additional
information like the gpt partition names.

Thanks
Michael

On Mon, Oct 17, 2016 at 8:49 AM, Vladimir Olovyannikov <
vladimir.olovyanni...@broadcom.com> wrote:

> Thank you Laszlo,
>
> Sorry, I missed the fields; it is my first contribution, I will add the
> required lines, review the source according to your comments and will
> resubmit the patch.
> So do you think the command should be _gpt instead of gpt? I was following
> TFTP and SF commands as a template.
>
> Thank you,
> Vladimir
>
> On Oct 16, 2016 1:05 PM, "Laszlo Ersek"  wrote:
> >
> > On 10/16/16 07:23, Vladimir Olovyannikov wrote:
> > > This allows managing (create, delete, modify, fat format) of GPT
> > > partitions from within UEFI Shell.
> > > Syntax:
> > > gpt  [device_mapped_name] [parameters...]
> > > See usage examples in the .uni file
> > > ---
> > >  .../Library/UefiShellGptCommandLib/FatFormat.c |  611 +++
> > >  .../Library/UefiShellGptCommandLib/FatFormat.h |  111 ++
> > >  .../Library/UefiShellGptCommandLib/GptWorker.c | 1902
> 
> > >  .../Library/UefiShellGptCommandLib/GptWorker.h |  186 ++
> > >  .../UefiShellGptCommandLib.c   | 1135 
> > >  .../UefiShellGptCommandLib.inf |   79 +
> > >  .../UefiShellGptCommandLib.uni |  117 ++
> > >  ShellPkg/ShellPkg.dec  |1 +
> > >  ShellPkg/ShellPkg.dsc  |4 +
> > >  9 files changed, 4146 insertions(+)
> > >  create mode 100644 ShellPkg/Library/UefiShellGptCommandLib/
> FatFormat.c
> > >  create mode 100644 ShellPkg/Library/UefiShellGptCommandLib/
> FatFormat.h
> > >  create mode 100644 ShellPkg/Library/UefiShellGptCommandLib/
> GptWorker.c
> > >  create mode 100644 ShellPkg/Library/UefiShellGptCommandLib/
> GptWorker.h
> > >  create mode 100644
> ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.c
> > >  create mode 100644
> ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.inf
> > >  create mode 100644
> ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.uni
> >
> > This looks like a supremely welcome, long-awaited addition (latest
> > request:
> > ),
> > but it really needs your Signed-off-by, and the Contributed-under line
> > above it:
> >
> > ShellPkg/Contributions.txt
> >
> > I would also suggest (simply based on what I've seen elsewhere in edk2)
> > to keep the copyright notices tightly collected in the file headings.
> >
> > Someone will have to go over all the licenses too -- does the "Marvell
> > BSD License Option" for example correspond to the 3-clause BSDL?
> >
> > On the technical side, I believe that as long as a shell command (or a
> > command option) is not standardized (in the shell spec), it usually
> > starts with an underscore (_), so as to prevent future name collisions.
> > (I could be wrong about this -- I now recall the TFTP command, which is
> > also not in the 2.2 spec.)
> >
> > Just my two cents.
> >
> > Thanks
> > Laszlo
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] GPT Shell Application/Library

[Vladimir Olovyannikov]

Thank you Laszlo,

Sorry, I missed the fields; it is my first contribution, I will add the
required lines, review the source according to your comments and will
resubmit the patch.
So do you think the command should be _gpt instead of gpt? I was following
TFTP and SF commands as a template.

Thank you,
Vladimir

On Oct 16, 2016 1:05 PM, "Laszlo Ersek"  wrote:
>
> On 10/16/16 07:23, Vladimir Olovyannikov wrote:
> > This allows managing (create, delete, modify, fat format) of GPT
> > partitions from within UEFI Shell.
> > Syntax:
> > gpt  [device_mapped_name] [parameters...]
> > See usage examples in the .uni file
> > ---
> >  .../Library/UefiShellGptCommandLib/FatFormat.c |  611 +++
> >  .../Library/UefiShellGptCommandLib/FatFormat.h |  111 ++
> >  .../Library/UefiShellGptCommandLib/GptWorker.c | 1902

> >  .../Library/UefiShellGptCommandLib/GptWorker.h |  186 ++
> >  .../UefiShellGptCommandLib.c   | 1135 
> >  .../UefiShellGptCommandLib.inf |   79 +
> >  .../UefiShellGptCommandLib.uni |  117 ++
> >  ShellPkg/ShellPkg.dec  |1 +
> >  ShellPkg/ShellPkg.dsc  |4 +
> >  9 files changed, 4146 insertions(+)
> >  create mode 100644 ShellPkg/Library/UefiShellGptCommandLib/FatFormat.c
> >  create mode 100644 ShellPkg/Library/UefiShellGptCommandLib/FatFormat.h
> >  create mode 100644 ShellPkg/Library/UefiShellGptCommandLib/GptWorker.c
> >  create mode 100644 ShellPkg/Library/UefiShellGptCommandLib/GptWorker.h
> >  create mode 100644
ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.c
> >  create mode 100644
ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.inf
> >  create mode 100644
ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.uni
>
> This looks like a supremely welcome, long-awaited addition (latest
> request:
> ),
> but it really needs your Signed-off-by, and the Contributed-under line
> above it:
>
> ShellPkg/Contributions.txt
>
> I would also suggest (simply based on what I've seen elsewhere in edk2)
> to keep the copyright notices tightly collected in the file headings.
>
> Someone will have to go over all the licenses too -- does the "Marvell
> BSD License Option" for example correspond to the 3-clause BSDL?
>
> On the technical side, I believe that as long as a shell command (or a
> command option) is not standardized (in the shell spec), it usually
> starts with an underscore (_), so as to prevent future name collisions.
> (I could be wrong about this -- I now recall the TFTP command, which is
> also not in the 2.2 spec.)
>
> Just my two cents.
>
> Thanks
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_*

[Zhu, Yonghong]

Sure, I will add this info.

Yes, add this suggestion is good.

Best Regards,
Zhu Yonghong

-Original Message-
From: Justen, Jordan L 
Sent: Monday, October 17, 2016 1:56 PM
To: Zhu, Yonghong ; edk2-devel@lists.01.org
Cc: Gao, Liming ; Zhu, Yonghong 
Subject: RE: [edk2] [Patch] BaseTools: PatchCheck to align with wiki and report 
error for EFI_D_*

On 2016-10-16 06:32:32, Zhu, Yonghong wrote:
> Thanks Jordan. I will separate the patches and send out the updated version.
> 

One other suggestion for the commit message on your patch. Can you add this at 
the bottom before the Contributed-under?

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

What do you think?

Maybe we should update the commit message wiki page to suggest this as well?

-Jordan

>
> -Original Message-
> From: Justen, Jordan L
> Sent: Sunday, October 16, 2016 4:14 AM
> To: Zhu, Yonghong ; edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: Re: [edk2] [Patch] BaseTools: PatchCheck to align with wiki 
> and report error for EFI_D_*
> 
> On 2016-10-13 02:29:46, Yonghong Zhu wrote:
> > This patch updates PatchCheck.py：
> 
> Can you split this into 3 separate patches?
> 
> > 1.Update max line length to 70 that describe in wiki
> 
> I think we should update PatchCheck.py and the wiki to say that:
> 
> 1. The subject line of the commit message should be < 72 characters.
> 
> This works well with git log --oneline to still produce output less than 80 
> characters.
> 
> 2. The other lines of the commit message should be < 76 characters.
> 
> When using git log, these lines are indented 4 characters, so once again this 
> will keep the log message readable and under 80 columns.
> 
> > 2.remove the two [] when do the char count calculation
> 
> How about instead we add a new regex to the class:
> 
> subject_prefix_re = \
> re.compile(r'''^
>\s* (\[
>[^\[\]]* # Allow all non-brackets
>\])? \s*
>''',
>re.VERBOSE)
> 
> 
> Then you can use it line this:
> 
>   self.commit_subject = \
>   self.subject_prefix_re.sub('', self.commit_subject, 1)
> 
> This will change:
> 
>   ' a' => 'a'
>   ' [patch 1] a' => 'a'
> 
> But, it will ignore this, because I don't think we should try to match 
> brackets here:
> 
>   ' [a [b]] a'
> 
> > 3.report error for EFI_D_* macro if it is used, recommend to use
> > DEBUG_* format.
> 
> This should definitely be a separate patch since we are starting to look at 
> code.
> 
> -Jordan
> 
> > 
> > Cc: Liming Gao 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Yonghong Zhu 
> > ---
> >  BaseTools/Scripts/PatchCheck.py | 21 -
> >  1 file changed, 16 insertions(+), 5 deletions(-)
> > 
> > diff --git a/BaseTools/Scripts/PatchCheck.py 
> > b/BaseTools/Scripts/PatchCheck.py index 455c130..d423220 100755
> > --- a/BaseTools/Scripts/PatchCheck.py
> > +++ b/BaseTools/Scripts/PatchCheck.py
> > @@ -1,9 +1,9 @@
> >  ## @file
> >  #  Check a patch for various format issues  # -#  Copyright (c) 
> > 2015, Intel Corporation. All rights reserved.
> > +#  Copyright (c) 2015 - 2016, Intel Corporation. All rights 
> > +reserved.
> >  #
> >  #  This program and the accompanying materials are licensed and 
> > made #  available under the terms and conditions of the BSD License 
> > which #  accompanies this distribution. The full text of the license 
> > may be #  found at http://opensource.org/licenses/bsd-license.php
> > @@ -14,11 +14,11 @@
> >  #
> >  
> >  from __future__ import print_function
> >  
> >  VersionNumber = '0.1'
> > -__copyright__ = "Copyright (c) 2015, Intel Corporation  All rights 
> > reserved."
> > +__copyright__ = "Copyright (c) 2015 - 2016, Intel Corporation  All rights 
> > reserved."
> >  
> >  import email
> >  import argparse
> >  import os
> >  import re
> > @@ -195,11 +195,11 @@ class CommitMessageCheck:
> >  
> >  if count <= 0:
> >  self.error('Empty commit message!')
> >  return
> >  
> > -if count >= 1 and len(lines[0]) > 76:
> > +if count >= 1 and len(lines[0]) > 70:
> >  self.error('First line of commit message (subject line) ' +
> > 'is too long.')
> >  
> >  if count >= 1 and len(lines[0].strip()) == 0:
> >  self.error('First line of commit message (subject line) ' 
> > + @@ -208,11 +208,11 @@ class CommitMessageCheck:
> >  if count >= 2 and lines[1].strip() != '':
> >  self.error('Second line of commit message should be ' +
> > 'empty.')
> >  
> >  for i in range(2, count):
> > -if (len(lines[i]) > 76 and
> > +if (len(lines[i]) > 70 and
> 

[edk2] [Patch] ShellPkg: update ping to use timer service instead of timer arch protocol .

[Fu Siyuan]

This patch update the shell ping command to use timer service to calculate the
RTT time, instead of using the timer arch protocol.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
Cc: Ni Ruiyu 
Cc: Ye Ting 
Cc: Zhang Lubo 
---
 .../Library/UefiShellNetwork1CommandsLib/Ping.c| 233 +++--
 .../UefiShellNetwork1CommandsLib.inf   |   3 +-
 .../UefiShellNetwork1CommandsLib.uni   |   4 +-
 3 files changed, 171 insertions(+), 69 deletions(-)

diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c 
b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
index 38625fe..2e1e878 100644
--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
@@ -86,7 +86,7 @@ typedef struct _ICMPX_ECHO_REQUEST_REPLY {
   UINT16  Checksum;
   UINT16  Identifier;
   UINT16  SequenceNum;
-  UINT64  TimeStamp;
+  UINT32  TimeStamp;
   UINT8   Data[1];
 } ICMPX_ECHO_REQUEST_REPLY;
 #pragma pack()
@@ -94,7 +94,7 @@ typedef struct _ICMPX_ECHO_REQUEST_REPLY {
 typedef struct _PING_ICMP_TX_INFO {
   LIST_ENTRYLink;
   UINT16SequenceNum;
-  UINT64TimeStamp;
+  UINT32TimeStamp;
   PING_IPX_COMPLETION_TOKEN *Token;
 } PING_ICMPX_TX_INFO;
 
@@ -109,6 +109,7 @@ typedef struct _PING_ICMP_TX_INFO {
 #define DEFAULT_BUFFER_SIZE   16
 #define ICMP_V4_ECHO_REQUEST  0x8
 #define ICMP_V4_ECHO_REPLY0x0
+#define STALL_1_MILLI_SECOND  1000
 
 #define PING_PRIVATE_DATA_SIGNATURE  SIGNATURE_32 ('P', 'i', 'n', 'g')
 typedef struct _PING_PRIVATE_DATA {
@@ -117,6 +118,10 @@ typedef struct _PING_PRIVATE_DATA {
   EFI_HANDLE  IpChildHandle;
   EFI_EVENT   Timer;
 
+  UINT32  TimerPeriod;
+  UINT32  RttTimerTick;   
+  EFI_EVENT   RttTimer;
+
   EFI_STATUS  Status;
   LIST_ENTRY  TxList;
   UINT16  RxCount;
@@ -221,93 +226,186 @@ STATIC CONST SHELL_PARAM_ITEMPingParamList[] = {
 //
 STATIC CONST CHAR16  *mDstString;
 STATIC CONST CHAR16  *mSrcString;
-STATIC UINT64mFrequency = 0;
 EFI_CPU_ARCH_PROTOCOL*gCpu = NULL;
 
 /**
-  Read the current time.
+  RTT timer tick routine.
+
+  @param[in]EventA EFI_EVENT type event.
+  @param[in]Context  The pointer to Context.
 
-  @retval the current tick value.
 **/
-UINT64
-ReadTime (
+VOID
+EFIAPI
+RttTimerTickRoutine (
+  IN EFI_EVENTEvent,
+  IN VOID *Context
+  )
+{
+  UINT32 *RttTimerTick;
+
+  RttTimerTick = (UINT32*) Context;
+  (*RttTimerTick)++;
+}
+
+/**
+  Get the timer period of the system.
+
+  This function tries to get the system timer period by creating
+  an 1ms period timer.
+
+  @return System timer period in MS, or 0 if operation failed.
+
+**/
+UINT32
+GetTimerPeriod(
   VOID
   )
 {
-  UINT64 TimerPeriod;
-  EFI_STATUS Status;
+  EFI_STATUS Status;
+  UINT32 RttTimerTick;
+  EFI_EVENT  TimerEvent;
+  UINT32 StallCounter;
+  EFI_TPLOldTpl;
 
-  ASSERT (gCpu != NULL);
+  RttTimerTick = 0;
+  StallCounter   = 0;
 
-  Status = gCpu->GetTimerValue (gCpu, 0, , );
+  Status = gBS->CreateEvent (
+  EVT_TIMER | EVT_NOTIFY_SIGNAL,
+  TPL_NOTIFY,
+  RttTimerTickRoutine,
+  ,
+  
+  );
   if (EFI_ERROR (Status)) {
-//
-// The WinntGetTimerValue will return EFI_UNSUPPORTED. Set the
-// TimerPeriod by ourselves.
-//
-mCurrentTick += 100;
+return 0;
+  }
+
+  OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
+  Status = gBS->SetTimer (
+  TimerEvent,
+  TimerPeriodic,
+  TICKS_PER_MS
+  );
+  if (EFI_ERROR (Status)) {
+gBS->CloseEvent (TimerEvent);
+return 0;
+  }
+
+  while (RttTimerTick < 10) {
+gBS->Stall (STALL_1_MILLI_SECOND);
+++StallCounter;
   }
-  
-  return mCurrentTick;
-}
 
+  gBS->RestoreTPL (OldTpl);
+
+  gBS->SetTimer (TimerEvent, TimerCancel, 0);
+  gBS->CloseEvent (TimerEvent);
+
+  return StallCounter / RttTimerTick;
+}
 
 /**
-  Get and calculate the frequency in ticks/ms.
-  The result is saved in the global variable mFrequency
+  Initialize the timer event for RTT (round trip time).
 
-  @retval EFI_SUCCESSCalculated the frequency successfully.
-  @retval Others Failed to calculate the frequency.
+  @param[in]PrivateThe pointer to PING_PRIVATE_DATA.
+
+  @retval EFI_SUCCESS  RTT timer is started.
+  @retval Others   Failed to start the RTT