Re: [edk2] [PATCH 4/6] ArmVirtPkg: use small code model for all UEFI_APPLICATION modules

2016-01-04 Thread Ard Biesheuvel
On 4 January 2016 at 19:27, Laszlo Ersek  wrote:
> On 12/24/15 14:03, Ard Biesheuvel wrote:
>> Unfortunately, compiling the DEBUG shell using the small code model is
>> not sufficient in all cases to get a successful build when the toolchain
>> defaults to the tiny code model. The reason is that not only the Shell binary
>> itself should be built using the small code model, all Shell component
>> libraries that are linked into the Shell binary via NULL library class
>> resolution should use the small code model as well.
>>
>> So override the code model and function alignment for DEBUG builds of
>> UEFI_APPLICATION modules when using GCC 4.9 (which is the only toolchain
>> that uses the tiny model). This should affect all Shell component libraries
>> in addition to the Shell core binary.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  ArmVirtPkg/ArmVirt.dsc.inc | 21 
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
>> index 49e4264ee8a4..fbd710cb870d 100644
>> --- a/ArmVirtPkg/ArmVirt.dsc.inc
>> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
>> @@ -406,3 +406,24 @@ [Components.common]
>>gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
>>gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
>>}
>> +
>> +[BuildOptions.AARCH64.EDKII.UEFI_APPLICATION]
>> +  #
>> +  # The bulk of the Shell functionality is implemented by UEFI_APPLICATION
>> +  # libraries that are linked into the Shell binary via NULL library class
>> +  # resolution. Since the Shell built in DEBUG mode exceeds the 1 MB range 
>> of
>> +  # the AARCH64 tiny code model which we use by default on GCC 4.9 and 
>> later,
>> +  # the .text and .data sections of the remaining base libraries (which are
>> +  # built using the tiny code model regardless of the model we use for
>> +  # UEFI_APPLICATION modules) should be kept as close together as possible.
>> +  #
>> +  # By reverting to 8 byte function alignment for UEFI_APPLICATION modules
>> +  # (which is usually the default, but will be lowered to 4 if we are using 
>> the
>> +  # tiny code model) and letting the linker sort its input by alignment, we 
>> can
>> +  # force all UEFI_APPLICATION small code model .text input sections to 
>> appear
>> +  # first in the binary. The remaining base libraries will end up in close
>> +  # proximity of each other at the end of the image, preventing out of range
>> +  # problems when relocating their tiny model (+/- 1 MB) symbol references.
>> +  #
>> +  DEBUG_GCC49_*_CC_FLAGS = -mcmodel=small -falign-functions=8
>> +  DEBUG_GCC49_*_DLINK_FLAGS = -z common-page-size=0x1000
>>
>
> I don't have technical objections, just that I don't really like the
> complexity of this.
>

You're right, this is awful.

> In ,
> you wrote
>
>> Well, this is primarily caused by the way the Shell is composed of
>> static libraries, and it is unlikely that this should ever affect
>> other UEFI_APPLICATION modules as well. And building everything else
>> with the large code model because of this seems backwards to me too.
>
> I think I'd prefer a "one size fits all" code model where we (as
> developers) get to keep our sanity, even if it costs a tiny bit more
> resources at runtime. Can you please summarize (again?) the benefits of
> the tiny model over the small model?
>
> I can see the ADR / ADRP discussion in patch 3/6, but what does that
> difference mean in practice?
>
> What savings justify the code model proliferation between GCC <= 4.8
> ("large"), CLANG 3.5 ("small"), GCC >= 4.9 ("tiny", except for the DEBUG
> Shell, which would be made "small" by the above)?
>

I failed to cc you for the followup of this series, which no longer
changes anything under ArmVirtPkg
http://thread.gmane.org/gmane.comp.bios.edk2.devel/6307

There are a couple of reasons for supporting all of these code models:
- tiny is preferred for code size, which is a huge deal for bare metal firmware
- small is the only Clang supported model that is compatible with
PE/COFF conversion (Clang has no tiny model, and its large model folds
its absolute symbol references into instruction immediate fields, and
PE/COFF has no runtime relocation for that)
- large is preferred over small when code size is concerned, but
results in lots of relocations (since all symbol references are
absolute)
- EDK2, being a reference implementation for bare metal ARM platforms
in addition to being the upstream for ArmVirtQemu, should support the
toolchains and code models its users are likely to use.


> Would ArmVirtPkg build (with all supported toolchains) using the "small"
> code model?
>

Yes, it builds fine in small and large. The only problematic one is tiny.

Bottom line: I am moving DEBUG_GCC49 to the small model to avoid the
build issues this series 

Re: [edk2] undefined Reference to 'strtof'

2016-01-04 Thread Michael Zimmermann
which tests are you using?
a quick grep on the edk2 tree shows me that strtof isn't really used.

On Sun, Jan 3, 2016 at 11:59 PM, Daryl McDaniel 
wrote:

> Michael,
>
> How would one reproduce the problem you are seeing?
> I don't see any problems when building or running any of the StdLib
> floating point test programs.  This is just on Tile, Ia32 and X64, though.
>
> Sincerely,
> Daryl McDaniel
>
>
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Michael
> > Zimmermann
> > Sent: Wednesday, December 30, 2015 5:21 AM
> > To: edk2-devel@lists.01.org 
> > Subject: [edk2] undefined Reference to 'strtof'
> >
> > this is caused by:
> > #define strtof_strtof
> >
> https://github.com/tianocore/edk2/blob/master/StdLibPrivateInternalFiles/Include/
> > namespace.h#L45
> >
> > we either need to remove this define or fix the function declaration.
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH 2/2] BaseTools AARCH64: build XIP modules with strict alignment

2016-01-04 Thread Ard Biesheuvel
GCC for AARCH64 recognizes byte swapping load and store sequences
and may replace them with wider loads or stores combined with rev
instructions. In some cases (i.e., with GCC version 5 and later)
this may result in unaligned accesses, which are not allowed before
we turn the MMU on.

So build any modules or static libraries that may execute with the MMU
off with -mstrict-align. Other modules don't need this switch, so we
can remove it from the CLANG35/AARCH64 common CC flags.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Conf/tools_def.template | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 0cc85a6f359d..c42a434165c0 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -4324,6 +4324,7 @@ DEFINE GCC_X64_CC_FLAGS= 
DEF(GCC_ALL_CC_FLAGS) -mno-red-zone -Wno-ad
 DEFINE GCC_IPF_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) 
-minline-int-divide-min-latency
 DEFINE GCC_ARM_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-mabi=aapcs -fno-short-enums -save-temps -fsigned-char -ffunction-sections 
-fdata-sections -fomit-frame-pointer -Wno-address -mthumb -mfloat-abi=soft
 DEFINE GCC_AARCH64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-fno-short-enums -save-temps -fverbose-asm -fsigned-char  -ffunction-sections 
-fdata-sections -fomit-frame-pointer -fno-builtin -Wno-address 
-fno-asynchronous-unwind-tables
+DEFINE GCC_AARCH64_CC_XIPFLAGS = -mstrict-align
 DEFINE GCC_DLINK_FLAGS_COMMON  = -nostdlib --pie
 DEFINE GCC_DLINK2_FLAGS_COMMON = 
--script=$(EDK_TOOLS_PATH)/Scripts/GccBase.lds
 DEFINE GCC_IA32_X64_DLINK_COMMON   = DEF(GCC_DLINK_FLAGS_COMMON) --gc-sections
@@ -4398,6 +4399,7 @@ DEFINE GCC47_ARM_ASM_FLAGS   = 
DEF(GCC46_ARM_ASM_FLAGS)
 DEFINE GCC47_AARCH64_ASM_FLAGS   = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC_ASM_FLAGS) -mlittle-endian
 DEFINE GCC47_ARM_CC_FLAGS= DEF(GCC46_ARM_CC_FLAGS) 
-mno-unaligned-access
 DEFINE GCC47_AARCH64_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC44_ALL_CC_FLAGS) -mcmodel=large DEF(GCC_AARCH64_CC_FLAGS)
+DEFINE GCC47_AARCH64_CC_XIPFLAGS = DEF(GCC_AARCH64_CC_XIPFLAGS)
 DEFINE GCC47_ARM_DLINK_FLAGS = DEF(GCC46_ARM_DLINK_FLAGS)
 DEFINE GCC47_ARM_DLINK2_FLAGS= DEF(GCC46_ARM_DLINK2_FLAGS)
 DEFINE GCC47_AARCH64_DLINK_FLAGS = DEF(GCC_AARCH64_DLINK_FLAGS)
@@ -4418,6 +4420,7 @@ DEFINE GCC48_ARM_ASM_FLAGS   = 
DEF(GCC47_ARM_ASM_FLAGS)
 DEFINE GCC48_AARCH64_ASM_FLAGS   = DEF(GCC47_AARCH64_ASM_FLAGS)
 DEFINE GCC48_ARM_CC_FLAGS= DEF(GCC47_ARM_CC_FLAGS)
 DEFINE GCC48_AARCH64_CC_FLAGS= DEF(GCC47_AARCH64_CC_FLAGS)
+DEFINE GCC48_AARCH64_CC_XIPFLAGS = DEF(GCC47_AARCH64_CC_XIPFLAGS)
 DEFINE GCC48_ARM_DLINK_FLAGS = DEF(GCC47_ARM_DLINK_FLAGS)
 DEFINE GCC48_ARM_DLINK2_FLAGS= DEF(GCC47_ARM_DLINK2_FLAGS)
 DEFINE GCC48_AARCH64_DLINK_FLAGS = DEF(GCC47_AARCH64_DLINK_FLAGS)
@@ -4438,6 +4441,7 @@ DEFINE GCC49_ARM_ASM_FLAGS   = 
DEF(GCC48_ARM_ASM_FLAGS)
 DEFINE GCC49_AARCH64_ASM_FLAGS   = DEF(GCC48_AARCH64_ASM_FLAGS)
 DEFINE GCC49_ARM_CC_FLAGS= DEF(GCC48_ARM_CC_FLAGS)
 DEFINE GCC49_AARCH64_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC44_ALL_CC_FLAGS) -mcmodel=tiny DEF(GCC_AARCH64_CC_FLAGS)
+DEFINE GCC49_AARCH64_CC_XIPFLAGS = DEF(GCC48_AARCH64_CC_XIPFLAGS)
 DEFINE GCC49_ARM_DLINK_FLAGS = DEF(GCC48_ARM_DLINK_FLAGS)
 DEFINE GCC49_ARM_DLINK2_FLAGS= DEF(GCC48_ARM_DLINK2_FLAGS)
 DEFINE GCC49_AARCH64_DLINK_FLAGS = DEF(GCC48_AARCH64_DLINK_FLAGS)
@@ -4897,6 +4901,7 @@ RELEASE_GCC47_ARM_CC_FLAGS   = 
DEF(GCC47_ARM_CC_FLAGS) -Wno-unused-but-set-v
 *_GCC47_AARCH64_PP_FLAGS = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC_PP_FLAGS)
 *_GCC47_AARCH64_RC_FLAGS = DEF(GCC_AARCH64_RC_FLAGS)
 *_GCC47_AARCH64_VFRPP_FLAGS  = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC_VFRPP_FLAGS)
+*_GCC47_AARCH64_CC_XIPFLAGS  = DEF(GCC47_AARCH64_CC_XIPFLAGS)
 
   DEBUG_GCC47_AARCH64_CC_FLAGS   = DEF(GCC47_AARCH64_CC_FLAGS) -O0
 RELEASE_GCC47_AARCH64_CC_FLAGS   = DEF(GCC47_AARCH64_CC_FLAGS) 
-Wno-unused-but-set-variable
@@ -5024,6 +5029,7 @@ RELEASE_GCC48_ARM_CC_FLAGS   = 
DEF(GCC48_ARM_CC_FLAGS) -Wno-unused-but-set-v
 *_GCC48_AARCH64_PP_FLAGS = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC_PP_FLAGS)
 *_GCC48_AARCH64_RC_FLAGS = DEF(GCC_AARCH64_RC_FLAGS)
 *_GCC48_AARCH64_VFRPP_FLAGS  = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC_VFRPP_FLAGS)
+*_GCC48_AARCH64_CC_XIPFLAGS  = DEF(GCC48_AARCH64_CC_XIPFLAGS)
 
   DEBUG_GCC48_AARCH64_CC_FLAGS   = DEF(GCC48_AARCH64_CC_FLAGS) -O0
 RELEASE_GCC48_AARCH64_CC_FLAGS   = DEF(GCC48_AARCH64_CC_FLAGS) 
-Wno-unused-but-set-variable
@@ -5151,6 +5157,7 @@ RELEASE_GCC49_ARM_CC_FLAGS   = 
DEF(GCC49_ARM_CC_FLAGS) 

[edk2] [PATCH 0/2] BaseTools AARCH64: build XIP modules with strict alignment

2016-01-04 Thread Ard Biesheuvel
On AARCH64, before we turn on the MMU, unaligned accesses are not allowed.
Since the idiom recognition employed by GCC may turn allowable sequences
into sequences that result in such unaligned accesses (i.e., a sequence of
byte wide loads in reverse order may be turned into a wider load and a rev
instruction of the compiler thinks unaligned accesses are allowed), we have
to make sure that any code that may execute before the MMU is enabled is
built with -mstrict-align.

Since this aligns with the notion of XIP we have in EDK2, let's add a build
rule that allows XIP specific CC option overrides to be set, and use it to
set the -mstrict-align CC flag for BASE, SEC, PEI_CORE and PEIM modules.

(XIPFLAGS has been suggest by Andrew Fish a couple of months ago, when a
similar issue came up)

Ard Biesheuvel (2):
  BaseTools: add separate build rule for modules that may execute in
place
  BaseTools AARCH64: build XIP modules with strict alignment

 BaseTools/Conf/build_rule.template | 26 
 BaseTools/Conf/tools_def.template  |  9 ++-
 2 files changed, 34 insertions(+), 1 deletion(-)

-- 
2.5.0

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


Re: [edk2] [PATCH v2 0/5] AARCH64 code model and toolchain updates

2016-01-04 Thread Ard Biesheuvel
On 31 December 2015 at 13:57, Ard Biesheuvel  wrote:
> This is a followup to the patch 'BaseTools AARCH64: add -mstrict-align to
> all AARCH64 GCC flavors' that I sent out on the 23rd. As it turns out, using
> strict alignment results in a code size increase which breaks the build for
> the DEBUG shell using the tiny code model we use for GCC49 and up.
>
> I have reproduced some patches here that I have already sent out separately
> (#5) or as part of another series (#1, #2) but I included them again since
> they need to be merged in order to prevent breaking bisect due to build
> errors.
>
> Changes since v1:
> - Dropped the patches that rely on function alignment to sort the input 
> objects
>   in a way that results in the tiny model objects to end up in close proximity
>   of each other. This is unmaintainable hack, and since the issue it solves 
> only
>   affects the DEBUG builds, we can simply switch to the small code model for
>   everything and not care about the code size increase.
> - Added some R-b's from Liming and Leif.
>
> Patches #1 and #2 set the CLANG35 target to the GNU flavor of the respective
> architectures, and adds the target to the preprocessor invocations as well.
>
> Patch #3 changes the default code model for all DEBUG_GCC49 generated code
> to the small model, and updates the minimal alignment accordingly. This allows
> us to get rid of module or module type specific overrides, which are difficult
> to maintain. This results in a slight size increase, but code size is not such
> a big concern for DEBUG builds anyway. Below are the compressed/uncompressed
> sizes of ArmVirtQemu.dsc built with the various code models (built without
> support for secure boot). Note that this includes the -mstrict-align switch
> added in patch #5.
>
>  |  tiny  |  small |  large | small / best  |
> -++++---+
>DEBUG | does not build |   1376k/7958k  |   1297k/7360k  |   +6%/+8% |
> -++++---+
>  RELEASE |772k/3897k  |904k/4636k  |791k/4321k  |  +17%/+19%|
> -++++---+
>
> Patch #4 removes the build options for AARCH64 from ShellPkg. They need to be
> overridden anyway or they will only affect the core Shell binary and not all 
> of
> the Shell component libraries that are merged with the core binary at build 
> time.
> Also, we only need this override for GCC49, since GCC 48 and earlier use the
> large code model, and CLANG35 uses the small code model.
>
> Patch #5 moves the -mstrict-align compiler option to the common AARCH64/GCC
> CFLAGS definition so that it applies to all GCC versions in addition to 
> CLANG35.
> This addresses an issue spotted by the Ubuntu/Debian folks (and which they
> kindly never reported upstream, afaict) here:
> https://bugs.launchpad.net/ubuntu/+source/edk2/+bug/1489560
>
> Ard Biesheuvel (5):
>   BaseTools CLANG35: use GNU target triplets explicitly
>   BaseTools CLANG35: use -target in PP flags as well
>   BaseTools AARCH64: move DEBUG GCC49 to the small code model
>   ShellPkg AARCH64: remove DEBUG BuildOptions override
>   BaseTools AARCH64: add -mstrict-align to all AARCH64 GCC flavors
>

Hello all,

I have committed patches #1 and #2 of this series as SVN r19583 and r19584.

For the remaining ones, I will follow up with an alternate approach to
implement the strict alignment, using build rules.

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


[edk2] [PATCH] BaseTools: Fix 'caculate' typos

2016-01-04 Thread Hao Wu
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 BaseTools/Source/C/GenBootSector/GenBootSector.c | 2 +-
 BaseTools/Source/C/GenFv/GenFvInternalLib.c  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/C/GenBootSector/GenBootSector.c 
b/BaseTools/Source/C/GenBootSector/GenBootSector.c
index 40cb19f..c218548 100644
--- a/BaseTools/Source/C/GenBootSector/GenBootSector.c
+++ b/BaseTools/Source/C/GenBootSector/GenBootSector.c
@@ -278,7 +278,7 @@ GetBootSectorOffset (
 Description:
   Get the offset of boot sector.
   For non-MBR disk, offset is just 0
-  for disk with MBR, offset needs to be caculated by parsing MBR
+  for disk with MBR, offset needs to be calculated by parsing MBR
 
   NOTE: if no one is active, we will patch MBR to select first partition as 
active.
 
diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c 
b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
index 10bb88b..6ab8a24 100644
--- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
+++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
@@ -2787,7 +2787,7 @@ Returns:
   }
   
   //
-  // Caculate the required sizes for all FFS files.
+  // Calculate the required sizes for all FFS files.
   //
   CurrentOffset = sizeof (EFI_FIRMWARE_VOLUME_HEADER);
   
@@ -2904,7 +2904,7 @@ Returns:
 }
   }
   CurrentOffset += VtfFileSize;
-  DebugMsg (NULL, 0, 9, "FvImage size", "The caculated fv image size is 0x%x 
and the current set fv image size is 0x%x", (unsigned) CurrentOffset, 
(unsigned) FvInfoPtr->Size);
+  DebugMsg (NULL, 0, 9, "FvImage size", "The calculated fv image size is 0x%x 
and the current set fv image size is 0x%x", (unsigned) CurrentOffset, 
(unsigned) FvInfoPtr->Size);
   
   if (FvInfoPtr->Size == 0) { 
 //
-- 
1.9.5.msysgit.0

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


[edk2] [PATCH v2] MdeModulePkg: Update MNP driver to recycle TX buffer asynchronously.

2016-01-04 Thread Fu Siyuan
This patch updates the MNP driver to recycle TX buffer asynchronously, instead
of using a while loop wait after each transmit command.
This patch also fixes a bug in SNP.GetStatus() interface to support the MNP
update. The UNDI driver may return multiple transmitted buffers in a single
GetStatus command, while SNP.GetStatus could only return one pointer each time,
the rest of them are lost. This patch fixes this issue by store these recycled
pointer in a temporary buffer in SNP driver.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c  | 266 ++---
 MdeModulePkg/Universal/Network/MnpDxe/MnpDriver.h  |   7 +-
 MdeModulePkg/Universal/Network/MnpDxe/MnpImpl.h|  45 +++-
 MdeModulePkg/Universal/Network/MnpDxe/MnpIo.c  | 118 -
 MdeModulePkg/Universal/Network/MnpDxe/MnpMain.c|   7 +-
 MdeModulePkg/Universal/Network/SnpDxe/Get_status.c |  73 --
 MdeModulePkg/Universal/Network/SnpDxe/Snp.c|  16 +-
 MdeModulePkg/Universal/Network/SnpDxe/Snp.h|  18 +-
 8 files changed, 421 insertions(+), 129 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c 
b/MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c
index 046d4df..d1a4cb5 100644
--- a/MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c
+++ b/MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c
@@ -1,7 +1,7 @@
 /** @file
   Implementation of Managed Network Protocol private services.
 
-Copyright (c) 2005 - 2012, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 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
@@ -209,6 +209,208 @@ MnpFreeNbuf (
   gBS->RestoreTPL (OldTpl);
 }
 
+/**
+  Add Count of TX buffers to MnpDeviceData->AllTxBufList and 
MnpDeviceData->FreeTxBufList.
+  The length of the buffer is specified by MnpDeviceData->BufferLength.
+
+  @param[in, out]  MnpDeviceData Pointer to the MNP_DEVICE_DATA.
+  @param[in]   Count Number of TX buffers to add.
+
+  @retval EFI_SUCCESS   The specified amount of TX buffers are 
allocated.
+  @retval EFI_OUT_OF_RESOURCES  Failed to allocate a TX buffer.
+
+**/
+EFI_STATUS
+MnpAddFreeTxBuf (
+  IN OUT MNP_DEVICE_DATA   *MnpDeviceData,
+  IN UINTN Count
+  )
+{
+  EFI_STATUSStatus;
+  UINT32Index;
+  MNP_TX_BUF_WRAP   *TxBufWrap;
+
+  NET_CHECK_SIGNATURE (MnpDeviceData, MNP_DEVICE_DATA_SIGNATURE);
+  ASSERT ((Count > 0) && (MnpDeviceData->BufferLength > 0));
+
+  Status = EFI_SUCCESS;
+  for (Index = 0; Index < Count; Index++) {
+TxBufWrap = (MNP_TX_BUF_WRAP*) AllocatePool (sizeof (MNP_TX_BUF_WRAP) + 
MnpDeviceData->BufferLength - 1);
+if (TxBufWrap == NULL) {
+  DEBUG ((EFI_D_ERROR, "MnpAddFreeTxBuf: TxBuf Alloc failed.\n"));
+
+  Status = EFI_OUT_OF_RESOURCES;
+  break;
+}
+DEBUG ((EFI_D_INFO, "MnpAddFreeTxBuf: Add TxBufWrap %p, TxBuf %p\n", 
TxBufWrap, TxBufWrap->TxBuf));
+TxBufWrap->Signature = MNP_TX_BUF_WRAP_SIGNATURE;
+TxBufWrap->InUse = FALSE;
+InsertTailList (>FreeTxBufList, >WrapEntry);
+InsertTailList (>AllTxBufList, >AllEntry);
+  }
+
+  MnpDeviceData->TxBufCount += Index;
+  return Status;
+}
+
+/**
+  Allocate a free TX buffer from MnpDeviceData->FreeTxBufList. If there is none
+  in the queue, first try to recycle some from SNP, then try to allocate some 
and add 
+  them into the queue, then fetch the NET_BUF from the updated FreeTxBufList.
+
+  @param[in, out]  MnpDeviceDataPointer to the MNP_DEVICE_DATA.
+
+  @return Pointer to the allocated free NET_BUF structure, if NULL the
+  operation is failed.
+
+**/
+UINT8 *
+MnpAllocTxBuf (
+  IN OUT MNP_DEVICE_DATA   *MnpDeviceData
+  )
+{
+  EFI_TPL   OldTpl;
+  UINT8 *TxBuf;
+  EFI_STATUSStatus;
+  LIST_ENTRY*Entry;
+  MNP_TX_BUF_WRAP   *TxBufWrap;
+  
+  NET_CHECK_SIGNATURE (MnpDeviceData, MNP_DEVICE_DATA_SIGNATURE);
+
+  OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
+
+  if (IsListEmpty (>FreeTxBufList)) {
+//
+// First try to recycle some TX buffer from SNP
+//
+Status = MnpRecycleTxBuf (MnpDeviceData);
+if (EFI_ERROR (Status)) {
+  TxBuf = NULL;
+  goto ON_EXIT;
+}
+
+//
+// If still no free TX buffer, allocate more.
+//
+if (IsListEmpty (>FreeTxBufList)) {
+  if ((MnpDeviceData->TxBufCount + MNP_TX_BUFFER_INCREASEMENT) > 
MNP_MAX_TX_BUFFER_NUM) {
+DEBUG (
+  (EFI_D_ERROR,
+  "MnpAllocTxBuf: The maximum TxBuf size is reached for MNP driver 
instance %p.\n",
+  MnpDeviceData)
+  );
+
+TxBuf = NULL;
+goto ON_EXIT;
+  }
+
+  Status = MnpAddFreeTxBuf (MnpDeviceData, MNP_TX_BUFFER_INCREASEMENT);
+  if 

[edk2] [PATCH] ShellPkg: Fix Shell assert when mv a file to a NULL target.

2016-01-04 Thread Qiu Shumin
When run command 'mv file ' the Shell assert. The patch refined the length of 
the buffer to fix this bug.

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

diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Mv.c 
b/ShellPkg/Library/UefiShellLevel2CommandsLib/Mv.c
index eb7287e..29efb1c 100644
--- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Mv.c
+++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Mv.c
@@ -2,7 +2,7 @@
   Main file for mv shell level 2 function.
 
   (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.
-  Copyright (c) 2009 - 2015, 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
@@ -731,7 +731,7 @@ ShellCommandRunMv (
 //
 // ValidateAndMoveFiles will report errors to the screen itself
 //
-CwdSize = StrSize(ShellGetCurrentDir(NULL)) + 1;
+CwdSize = StrSize(ShellGetCurrentDir(NULL)) + sizeof(CHAR16);
 Cwd = AllocateZeroPool(CwdSize);
 ASSERT (Cwd != NULL);
 StrCpyS(Cwd, CwdSize/sizeof(CHAR16), ShellGetCurrentDir(NULL));
-- 
1.9.5.msysgit.1

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


[edk2] [Patch] NetworkPkg: Removing or adding some ASSERT statement

2016-01-04 Thread Jiaxin Wu
Refine the code by removing or adding some ASSERT statement to make 
the code more readable.

Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/DnsDxe/DnsImpl.c | 4 
 NetworkPkg/DnsDxe/DnsProtocol.c | 8 
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c
index 8725670..823bcf3 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -1358,12 +1358,10 @@ ParseDnsResponse (
 
 //
 // Check whether it's the GeneralLookUp querying.
 //
 if (Instance->Service->IpVersion == IP_VERSION_4 && 
Dns4TokenEntry->GeneralLookUp) {
-  ASSERT (Dns4TokenEntry != NULL);
-  
   Dns4RR = Dns4TokenEntry->Token->RspData.GLookupData->RRList;
   AnswerData = (UINT8 *) AnswerSection + sizeof (*AnswerSection);
 
   //
   // Fill the ResourceRecord.
@@ -1385,12 +1383,10 @@ ParseDnsResponse (
   }
   CopyMem (Dns4RR[RRCount].RData, AnswerData, Dns4RR[RRCount].DataLength);
   
   RRCount ++;
 } else if (Instance->Service->IpVersion == IP_VERSION_6 && 
Dns6TokenEntry->GeneralLookUp) {
-  ASSERT (Dns6TokenEntry != NULL);
-  
   Dns6RR = Dns6TokenEntry->Token->RspData.GLookupData->RRList;
   AnswerData = (UINT8 *) AnswerSection + sizeof (*AnswerSection);
 
   //
   // Fill the ResourceRecord.
diff --git a/NetworkPkg/DnsDxe/DnsProtocol.c b/NetworkPkg/DnsDxe/DnsProtocol.c
index e7aa227..a3f3de9 100644
--- a/NetworkPkg/DnsDxe/DnsProtocol.c
+++ b/NetworkPkg/DnsDxe/DnsProtocol.c
@@ -460,10 +460,12 @@ Dns4HostNameToIp (
 }
 
 goto ON_EXIT;
   }
 
+  ASSERT (Packet != NULL);
+
   //
   // Save the token into the Dns4TxTokens map.
   //
   Status = NetMapInsertTail (>Dns4TxTokens, TokenEntry, Packet);
   if (EFI_ERROR (Status)) {
@@ -633,10 +635,12 @@ Dns4GeneralLookUp (
 }
 
 goto ON_EXIT;
   }
 
+  ASSERT (Packet != NULL);
+
   //
   // Save the token into the Dns4TxTokens map.
   //
   Status = NetMapInsertTail (>Dns4TxTokens, TokenEntry, Packet);
   if (EFI_ERROR (Status)) {
@@ -1229,10 +1233,12 @@ Dns6HostNameToIp (
 }
 
 goto ON_EXIT;
   }
 
+  ASSERT (Packet != NULL);
+
   //
   // Save the token into the Dns6TxTokens map.
   //
   Status = NetMapInsertTail (>Dns6TxTokens, TokenEntry, Packet);
   if (EFI_ERROR (Status)) {
@@ -1402,10 +1408,12 @@ Dns6GeneralLookUp (
 }
 
 goto ON_EXIT;
   }
 
+  ASSERT (Packet != NULL);
+
   //
   // Save the token into the Dns6TxTokens map.
   //
   Status = NetMapInsertTail (>Dns6TxTokens, TokenEntry, Packet);
   if (EFI_ERROR (Status)) {
-- 
1.9.5.msysgit.1

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


Re: [edk2] [PATCH] BaseTools: Fix 'caculate' typos

2016-01-04 Thread Gao, Liming
Reviewed-by: Liming Gao 

-Original Message-
From: Wu, Hao A 
Sent: Tuesday, January 05, 2016 9:24 AM
To: edk2-devel@lists.01.org; Zhu, Yonghong; Gao, Liming
Cc: Wu, Hao A
Subject: [PATCH] BaseTools: Fix 'caculate' typos

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 BaseTools/Source/C/GenBootSector/GenBootSector.c | 2 +-
 BaseTools/Source/C/GenFv/GenFvInternalLib.c  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/C/GenBootSector/GenBootSector.c 
b/BaseTools/Source/C/GenBootSector/GenBootSector.c
index 40cb19f..c218548 100644
--- a/BaseTools/Source/C/GenBootSector/GenBootSector.c
+++ b/BaseTools/Source/C/GenBootSector/GenBootSector.c
@@ -278,7 +278,7 @@ GetBootSectorOffset (
 Description:
   Get the offset of boot sector.
   For non-MBR disk, offset is just 0
-  for disk with MBR, offset needs to be caculated by parsing MBR
+  for disk with MBR, offset needs to be calculated by parsing MBR
 
   NOTE: if no one is active, we will patch MBR to select first partition as 
active.
 
diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c 
b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
index 10bb88b..6ab8a24 100644
--- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
+++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
@@ -2787,7 +2787,7 @@ Returns:
   }
   
   //
-  // Caculate the required sizes for all FFS files.
+  // Calculate the required sizes for all FFS files.
   //
   CurrentOffset = sizeof (EFI_FIRMWARE_VOLUME_HEADER);
   
@@ -2904,7 +2904,7 @@ Returns:
 }
   }
   CurrentOffset += VtfFileSize;
-  DebugMsg (NULL, 0, 9, "FvImage size", "The caculated fv image size is 0x%x 
and the current set fv image size is 0x%x", (unsigned) CurrentOffset, 
(unsigned) FvInfoPtr->Size);
+  DebugMsg (NULL, 0, 9, "FvImage size", "The calculated fv image size is 0x%x 
and the current set fv image size is 0x%x", (unsigned) CurrentOffset, 
(unsigned) FvInfoPtr->Size);
   
   if (FvInfoPtr->Size == 0) { 
 //
-- 
1.9.5.msysgit.0

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


Re: [edk2] [Patch] NetworkPkg: Removing or adding some ASSERT statement

2016-01-04 Thread Fu, Siyuan
Reviewed-by: Fu Siyuan 

-Original Message-
From: Wu, Jiaxin 
Sent: Tuesday, January 5, 2016 9:47 AM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Fu, Siyuan 
Subject: [Patch] NetworkPkg: Removing or adding some ASSERT statement

Refine the code by removing or adding some ASSERT statement to make the code 
more readable.

Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/DnsDxe/DnsImpl.c | 4 
 NetworkPkg/DnsDxe/DnsProtocol.c | 8 
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c index 
8725670..823bcf3 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -1358,12 +1358,10 @@ ParseDnsResponse (
 
 //
 // Check whether it's the GeneralLookUp querying.
 //
 if (Instance->Service->IpVersion == IP_VERSION_4 && 
Dns4TokenEntry->GeneralLookUp) {
-  ASSERT (Dns4TokenEntry != NULL);
-  
   Dns4RR = Dns4TokenEntry->Token->RspData.GLookupData->RRList;
   AnswerData = (UINT8 *) AnswerSection + sizeof (*AnswerSection);
 
   //
   // Fill the ResourceRecord.
@@ -1385,12 +1383,10 @@ ParseDnsResponse (
   }
   CopyMem (Dns4RR[RRCount].RData, AnswerData, Dns4RR[RRCount].DataLength);
   
   RRCount ++;
 } else if (Instance->Service->IpVersion == IP_VERSION_6 && 
Dns6TokenEntry->GeneralLookUp) {
-  ASSERT (Dns6TokenEntry != NULL);
-  
   Dns6RR = Dns6TokenEntry->Token->RspData.GLookupData->RRList;
   AnswerData = (UINT8 *) AnswerSection + sizeof (*AnswerSection);
 
   //
   // Fill the ResourceRecord.
diff --git a/NetworkPkg/DnsDxe/DnsProtocol.c b/NetworkPkg/DnsDxe/DnsProtocol.c 
index e7aa227..a3f3de9 100644
--- a/NetworkPkg/DnsDxe/DnsProtocol.c
+++ b/NetworkPkg/DnsDxe/DnsProtocol.c
@@ -460,10 +460,12 @@ Dns4HostNameToIp (
 }
 
 goto ON_EXIT;
   }
 
+  ASSERT (Packet != NULL);
+
   //
   // Save the token into the Dns4TxTokens map.
   //
   Status = NetMapInsertTail (>Dns4TxTokens, TokenEntry, Packet);
   if (EFI_ERROR (Status)) {
@@ -633,10 +635,12 @@ Dns4GeneralLookUp (
 }
 
 goto ON_EXIT;
   }
 
+  ASSERT (Packet != NULL);
+
   //
   // Save the token into the Dns4TxTokens map.
   //
   Status = NetMapInsertTail (>Dns4TxTokens, TokenEntry, Packet);
   if (EFI_ERROR (Status)) {
@@ -1229,10 +1233,12 @@ Dns6HostNameToIp (
 }
 
 goto ON_EXIT;
   }
 
+  ASSERT (Packet != NULL);
+
   //
   // Save the token into the Dns6TxTokens map.
   //
   Status = NetMapInsertTail (>Dns6TxTokens, TokenEntry, Packet);
   if (EFI_ERROR (Status)) {
@@ -1402,10 +1408,12 @@ Dns6GeneralLookUp (
 }
 
 goto ON_EXIT;
   }
 
+  ASSERT (Packet != NULL);
+
   //
   // Save the token into the Dns6TxTokens map.
   //
   Status = NetMapInsertTail (>Dns6TxTokens, TokenEntry, Packet);
   if (EFI_ERROR (Status)) {
--
1.9.5.msysgit.1

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


[edk2] [patch] MdeModulePkg:Avoid ASSERT in HiiConfigRoutingRouteConfig

2016-01-04 Thread Dandan Bi
Add error handling code to enhance the code,the driver may not install
the ConfigAccess protocol,so should not just ASSERT here.

Cc: Liming Gao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c 
b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
index 55ac08c..8f0b968 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
@@ -4344,11 +4344,15 @@ HiiConfigRoutingRouteConfig (
   Status = gBS->HandleProtocol (
   DriverHandle,
   ,
   (VOID **)  
   );
-  ASSERT_EFI_ERROR (Status);
+  if (EFI_ERROR (Status)) {
+*Progress = StringPtr;
+FreePool (ConfigResp);
+return EFI_NOT_FOUND;
+  }
 
   Status = ConfigAccess->RouteConfig (
ConfigAccess,
ConfigResp,

-- 
1.9.5.msysgit.1

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


[edk2] [patch] MdeModulePkg:Fix the potential memory leak issue in Display Engine

2016-01-04 Thread Dandan Bi
The MenuOption insert to gMenuOption allocate memory everytime,but not free.
Now add the code to free it.

Cc: Liming Gao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 .../Universal/DisplayEngineDxe/FormDisplay.c   | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c 
b/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
index a391442..66f7dff 100644
--- a/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
+++ b/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
@@ -3728,10 +3728,39 @@ UiDisplayMenu (
 }
   }
 }
 
 /**
+  Free the UI Menu Option structure data.
+
+  @param   MenuOptionList Point to the menu option list which need to 
be free.
+
+**/
+
+VOID
+FreeMenuOptionData(
+  LIST_ENTRY   *MenuOptionList
+  )
+{
+  LIST_ENTRY   *Link;
+  UI_MENU_OPTION   *Option;
+
+  //
+  // Free menu option list
+  //
+  while (!IsListEmpty (MenuOptionList)) {
+Link = GetFirstNode (MenuOptionList);
+Option = MENU_OPTION_FROM_LINK (Link);
+if (Option->Description != NULL){
+  FreePool(Option->Description);
+}
+RemoveEntryList (>Link);
+FreePool (Option);
+  }
+}
+
+/**
 
   Base on the browser status info to show an pop up message.
 
 **/
 VOID
@@ -3999,10 +4028,15 @@ FormDisplay (
   mIsFirstForm= FALSE;
   gOldFormEntry.HiiHandle = FormData->HiiHandle;
   CopyGuid (, >FormSetGuid);
   gOldFormEntry.FormId= FormData->FormId;
 
+  //
+  //Free the Ui menu option list.
+  //
+  FreeMenuOptionData();
+
   return Status;
 }
 
 /**
   Clear Screen to the initial state.
-- 
1.9.5.msysgit.1

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


[edk2] [PATCH] MdeModulePkg: Fix 'accroding' typos in MdeModulePkg.dec/.uni

2016-01-04 Thread Hao Wu
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/MdeModulePkg.dec | 8 
 MdeModulePkg/MdeModulePkg.uni | 8 
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 68686cf..0d8aede 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1419,7 +1419,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId|"INTEL "|VOID*|0x30001034
 
   ## Default OEM Table ID for ACPI table creation, it is "EDK2".
-  #  Accroding to ACPI specification, this field is particularly useful when
+  #  According to ACPI specification, this field is particularly useful when
   #  defining a definition block to distinguish definition block functions.
   #  The OEM assigns each dissimilar table a new OEM Table ID.
   #  This PCD is ignored for definition block.
@@ -1427,7 +1427,7 @@
   
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId|0x20202020324B4445|UINT64|0x30001035
 
   ## Default OEM Revision for ACPI table creation.
-  #  Accroding to ACPI specification, for LoadTable() opcode, the OS can also
+  #  According to ACPI specification, for LoadTable() opcode, the OS can also
   #  check the OEM Table ID and Revision ID against a database for a newer
   #  revision Definition Block of the same OEM Table ID and load it instead.
   #  This PCD is ignored for definition block.
@@ -1435,14 +1435,14 @@
   
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision|0x0002|UINT32|0x30001036
 
   ## Default Creator ID for ACPI table creation.
-  #  Accroding to ACPI specification, for tables containing Definition Blocks,
+  #  According to ACPI specification, for tables containing Definition Blocks,
   #  this is the ID for the ASL Compiler.
   #  This PCD is ignored for definition block.
   # @Prompt Default Creator ID for ACPI table creation.
   
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId|0x20202020|UINT32|0x30001037
 
   ## Default Creator Revision for ACPI table creation.
-  #  Accroding to ACPI specification, for tables containing Definition Blocks,
+  #  According to ACPI specification, for tables containing Definition Blocks,
   #  this is the revision for the ASL Compiler.
   #  This PCD is ignored for definition block.
   # @Prompt Default Creator Revision for ACPI table creation.
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index e473ec3..3d7f22f 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -226,7 +226,7 @@
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiDefaultOemTableId_PROMPT  
#language en-US "Default OEM Table ID for ACPI table creation"
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiDefaultOemTableId_HELP  
#language en-US "Default OEM Table ID for ACPI table creation.\n"
-   
   "Accroding to ACPI specification, this field is particularly useful 
when\n"
+   
   "According to ACPI specification, this field is particularly useful 
when\n"

   "defining a definition block to distinguish definition block 
functions.\n"

   "The OEM assigns each dissimilar table a new OEM Table ID.\n"

   "This PCD is ignored for definition block."
@@ -234,7 +234,7 @@
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiDefaultOemRevision_PROMPT  
#language en-US "Default OEM Revision for ACPI table creation"
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiDefaultOemRevision_HELP  
#language en-US "Default OEM Revision for ACPI table creation.\n"
-   
"Accroding to ACPI specification, for LoadTable() opcode, the OS 
can also\n"
+   
"According to ACPI specification, for LoadTable() opcode, the OS 
can also\n"

"check the OEM Table ID and Revision ID against a database for a 
newer\n"

"revision Definition Block of the same OEM Table ID and load it 
instead.\n"

"This PCD is ignored for definition block."
@@ -242,14 +242,14 @@
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiDefaultCreatorId_PROMPT  
#language en-US "Default Creator ID for ACPI table creation"
 
 #string 

Re: [edk2] [PATCH] MdeModulePkg: Fix 'accroding' typos in MdeModulePkg.dec/.uni

2016-01-04 Thread Tian, Feng
Reviewed-by: Feng Tian 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Hao Wu
Sent: Tuesday, January 05, 2016 09:21
To: edk2-devel@lists.01.org; Tian, Feng
Cc: Wu, Hao A
Subject: [edk2] [PATCH] MdeModulePkg: Fix 'accroding' typos in 
MdeModulePkg.dec/.uni

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/MdeModulePkg.dec | 8   MdeModulePkg/MdeModulePkg.uni | 8 

 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec 
index 68686cf..0d8aede 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1419,7 +1419,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId|"INTEL "|VOID*|0x30001034
 
   ## Default OEM Table ID for ACPI table creation, it is "EDK2".
-  #  Accroding to ACPI specification, this field is particularly useful when
+  #  According to ACPI specification, this field is particularly useful 
+ when
   #  defining a definition block to distinguish definition block functions.
   #  The OEM assigns each dissimilar table a new OEM Table ID.
   #  This PCD is ignored for definition block.
@@ -1427,7 +1427,7 @@
   
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId|0x20202020324B4445|UINT64|0x30001035
 
   ## Default OEM Revision for ACPI table creation.
-  #  Accroding to ACPI specification, for LoadTable() opcode, the OS can also
+  #  According to ACPI specification, for LoadTable() opcode, the OS 
+ can also
   #  check the OEM Table ID and Revision ID against a database for a newer
   #  revision Definition Block of the same OEM Table ID and load it instead.
   #  This PCD is ignored for definition block.
@@ -1435,14 +1435,14 @@
   
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision|0x0002|UINT32|0x30001036
 
   ## Default Creator ID for ACPI table creation.
-  #  Accroding to ACPI specification, for tables containing Definition Blocks,
+  #  According to ACPI specification, for tables containing Definition 
+ Blocks,
   #  this is the ID for the ASL Compiler.
   #  This PCD is ignored for definition block.
   # @Prompt Default Creator ID for ACPI table creation.
   
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId|0x20202020|UINT32|0x30001037
 
   ## Default Creator Revision for ACPI table creation.
-  #  Accroding to ACPI specification, for tables containing Definition Blocks,
+  #  According to ACPI specification, for tables containing Definition 
+ Blocks,
   #  this is the revision for the ASL Compiler.
   #  This PCD is ignored for definition block.
   # @Prompt Default Creator Revision for ACPI table creation.
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni 
index e473ec3..3d7f22f 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -226,7 +226,7 @@
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiDefaultOemTableId_PROMPT  
#language en-US "Default OEM Table ID for ACPI table creation"
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiDefaultOemTableId_HELP  
#language en-US "Default OEM Table ID for ACPI table creation.\n"
-   
   "Accroding to ACPI specification, this field is particularly useful 
when\n"
+   
   "According to ACPI specification, this field is particularly useful 
when\n"

   "defining a definition block to distinguish definition block 
functions.\n"

   "The OEM assigns each dissimilar table a new OEM Table ID.\n"

   "This PCD is ignored for definition block."
@@ -234,7 +234,7 @@
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiDefaultOemRevision_PROMPT  
#language en-US "Default OEM Revision for ACPI table creation"
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiDefaultOemRevision_HELP  
#language en-US "Default OEM Revision for ACPI table creation.\n"
-   
"Accroding to ACPI specification, for LoadTable() opcode, the OS 
can also\n"
+   
"According to ACPI specification, for LoadTable() opcode, the OS 
can also\n"

"check the OEM Table ID and Revision ID against a database for a 
newer\n"

"revision Definition Block of the same OEM Table ID 

Re: [edk2] [Patch] NetworkPkg: Removing or adding some ASSERT statement

2016-01-04 Thread Ye, Ting
Reviewed-by: Ye Ting   

-Original Message-
From: Wu, Jiaxin 
Sent: Tuesday, January 05, 2016 9:47 AM
To: edk2-devel@lists.01.org
Cc: Ye, Ting; Fu, Siyuan
Subject: [Patch] NetworkPkg: Removing or adding some ASSERT statement

Refine the code by removing or adding some ASSERT statement to make the code 
more readable.

Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/DnsDxe/DnsImpl.c | 4 
 NetworkPkg/DnsDxe/DnsProtocol.c | 8 
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c index 
8725670..823bcf3 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -1358,12 +1358,10 @@ ParseDnsResponse (
 
 //
 // Check whether it's the GeneralLookUp querying.
 //
 if (Instance->Service->IpVersion == IP_VERSION_4 && 
Dns4TokenEntry->GeneralLookUp) {
-  ASSERT (Dns4TokenEntry != NULL);
-  
   Dns4RR = Dns4TokenEntry->Token->RspData.GLookupData->RRList;
   AnswerData = (UINT8 *) AnswerSection + sizeof (*AnswerSection);
 
   //
   // Fill the ResourceRecord.
@@ -1385,12 +1383,10 @@ ParseDnsResponse (
   }
   CopyMem (Dns4RR[RRCount].RData, AnswerData, Dns4RR[RRCount].DataLength);
   
   RRCount ++;
 } else if (Instance->Service->IpVersion == IP_VERSION_6 && 
Dns6TokenEntry->GeneralLookUp) {
-  ASSERT (Dns6TokenEntry != NULL);
-  
   Dns6RR = Dns6TokenEntry->Token->RspData.GLookupData->RRList;
   AnswerData = (UINT8 *) AnswerSection + sizeof (*AnswerSection);
 
   //
   // Fill the ResourceRecord.
diff --git a/NetworkPkg/DnsDxe/DnsProtocol.c b/NetworkPkg/DnsDxe/DnsProtocol.c 
index e7aa227..a3f3de9 100644
--- a/NetworkPkg/DnsDxe/DnsProtocol.c
+++ b/NetworkPkg/DnsDxe/DnsProtocol.c
@@ -460,10 +460,12 @@ Dns4HostNameToIp (
 }
 
 goto ON_EXIT;
   }
 
+  ASSERT (Packet != NULL);
+
   //
   // Save the token into the Dns4TxTokens map.
   //
   Status = NetMapInsertTail (>Dns4TxTokens, TokenEntry, Packet);
   if (EFI_ERROR (Status)) {
@@ -633,10 +635,12 @@ Dns4GeneralLookUp (
 }
 
 goto ON_EXIT;
   }
 
+  ASSERT (Packet != NULL);
+
   //
   // Save the token into the Dns4TxTokens map.
   //
   Status = NetMapInsertTail (>Dns4TxTokens, TokenEntry, Packet);
   if (EFI_ERROR (Status)) {
@@ -1229,10 +1233,12 @@ Dns6HostNameToIp (
 }
 
 goto ON_EXIT;
   }
 
+  ASSERT (Packet != NULL);
+
   //
   // Save the token into the Dns6TxTokens map.
   //
   Status = NetMapInsertTail (>Dns6TxTokens, TokenEntry, Packet);
   if (EFI_ERROR (Status)) {
@@ -1402,10 +1408,12 @@ Dns6GeneralLookUp (
 }
 
 goto ON_EXIT;
   }
 
+  ASSERT (Packet != NULL);
+
   //
   // Save the token into the Dns6TxTokens map.
   //
   Status = NetMapInsertTail (>Dns6TxTokens, TokenEntry, Packet);
   if (EFI_ERROR (Status)) {
--
1.9.5.msysgit.1

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


Re: [edk2] [PATCH v2] MdeModulePkg: Update MNP driver to recycle TX buffer asynchronously.

2016-01-04 Thread Wu, Jiaxin
Siyuan,
Some small comments:
1. Please update MnpBuildTxPacket() function's description.
2. Since Mnp Sent the packet asynchronously, MnpSyncSendPacket() function's 
name is improper, the corresponding description also should be updated.

Other parts good to me.

Reviewed-by: Jiaxin Wu 

Thanks.
Jiaxin


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu Siyuan
Sent: Tuesday, January 5, 2016 11:37 AM
To: edk2-devel@lists.01.org
Cc: Ye, Ting; Wu, Jiaxin
Subject: [edk2] [PATCH v2] MdeModulePkg: Update MNP driver to recycle TX buffer 
asynchronously.

This patch updates the MNP driver to recycle TX buffer asynchronously, instead 
of using a while loop wait after each transmit command.
This patch also fixes a bug in SNP.GetStatus() interface to support the MNP 
update. The UNDI driver may return multiple transmitted buffers in a single 
GetStatus command, while SNP.GetStatus could only return one pointer each time, 
the rest of them are lost. This patch fixes this issue by store these recycled 
pointer in a temporary buffer in SNP driver.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c  | 266 ++---
 MdeModulePkg/Universal/Network/MnpDxe/MnpDriver.h  |   7 +-
 MdeModulePkg/Universal/Network/MnpDxe/MnpImpl.h|  45 +++-
 MdeModulePkg/Universal/Network/MnpDxe/MnpIo.c  | 118 -
 MdeModulePkg/Universal/Network/MnpDxe/MnpMain.c|   7 +-
 MdeModulePkg/Universal/Network/SnpDxe/Get_status.c |  73 --
 MdeModulePkg/Universal/Network/SnpDxe/Snp.c|  16 +-
 MdeModulePkg/Universal/Network/SnpDxe/Snp.h|  18 +-
 8 files changed, 421 insertions(+), 129 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c 
b/MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c
index 046d4df..d1a4cb5 100644
--- a/MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c
+++ b/MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c
@@ -1,7 +1,7 @@
 /** @file
   Implementation of Managed Network Protocol private services.
 
-Copyright (c) 2005 - 2012, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 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 @@ -209,6 +209,208 @@ MnpFreeNbuf (
   gBS->RestoreTPL (OldTpl);
 }
 
+/**
+  Add Count of TX buffers to MnpDeviceData->AllTxBufList and 
MnpDeviceData->FreeTxBufList.
+  The length of the buffer is specified by MnpDeviceData->BufferLength.
+
+  @param[in, out]  MnpDeviceData Pointer to the MNP_DEVICE_DATA.
+  @param[in]   Count Number of TX buffers to add.
+
+  @retval EFI_SUCCESS   The specified amount of TX buffers are 
allocated.
+  @retval EFI_OUT_OF_RESOURCES  Failed to allocate a TX buffer.
+
+**/
+EFI_STATUS
+MnpAddFreeTxBuf (
+  IN OUT MNP_DEVICE_DATA   *MnpDeviceData,
+  IN UINTN Count
+  )
+{
+  EFI_STATUSStatus;
+  UINT32Index;
+  MNP_TX_BUF_WRAP   *TxBufWrap;
+
+  NET_CHECK_SIGNATURE (MnpDeviceData, MNP_DEVICE_DATA_SIGNATURE);  
+ ASSERT ((Count > 0) && (MnpDeviceData->BufferLength > 0));
+
+  Status = EFI_SUCCESS;
+  for (Index = 0; Index < Count; Index++) {
+TxBufWrap = (MNP_TX_BUF_WRAP*) AllocatePool (sizeof (MNP_TX_BUF_WRAP) + 
MnpDeviceData->BufferLength - 1);
+if (TxBufWrap == NULL) {
+  DEBUG ((EFI_D_ERROR, "MnpAddFreeTxBuf: TxBuf Alloc failed.\n"));
+
+  Status = EFI_OUT_OF_RESOURCES;
+  break;
+}
+DEBUG ((EFI_D_INFO, "MnpAddFreeTxBuf: Add TxBufWrap %p, TxBuf %p\n", 
TxBufWrap, TxBufWrap->TxBuf));
+TxBufWrap->Signature = MNP_TX_BUF_WRAP_SIGNATURE;
+TxBufWrap->InUse = FALSE;
+InsertTailList (>FreeTxBufList, >WrapEntry);
+InsertTailList (>AllTxBufList, 
+ >AllEntry);  }
+
+  MnpDeviceData->TxBufCount += Index;
+  return Status;
+}
+
+/**
+  Allocate a free TX buffer from MnpDeviceData->FreeTxBufList. If there 
+is none
+  in the queue, first try to recycle some from SNP, then try to 
+allocate some and add
+  them into the queue, then fetch the NET_BUF from the updated FreeTxBufList.
+
+  @param[in, out]  MnpDeviceDataPointer to the MNP_DEVICE_DATA.
+
+  @return Pointer to the allocated free NET_BUF structure, if NULL the
+  operation is failed.
+
+**/
+UINT8 *
+MnpAllocTxBuf (
+  IN OUT MNP_DEVICE_DATA   *MnpDeviceData
+  )
+{
+  EFI_TPL   OldTpl;
+  UINT8 *TxBuf;
+  EFI_STATUSStatus;
+  LIST_ENTRY*Entry;
+  MNP_TX_BUF_WRAP   *TxBufWrap;
+  
+  NET_CHECK_SIGNATURE (MnpDeviceData, MNP_DEVICE_DATA_SIGNATURE);
+
+  OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
+
+  if (IsListEmpty (>FreeTxBufList)) {
+//
+// First try to recycle some TX buffer from SNP
+//
+Status = MnpRecycleTxBuf 

Re: [edk2] [Patch] NetworkPkg: Fix suspicious dereference of pointer before NULL check

2016-01-04 Thread Ye, Ting
Hi Leif,

I think you are raising a general question "can we trust the consumed API 
already return defined value", not specific to this patch.
I don't have an answer to this. And I also would like to hear more opinions.

For this patch: I would accept the change made to core services such as 
LocateHandleBuffer/HandleProtocol, as I think these are trusted APIs;
For others such as calling Aip->GetSupportedTypes/ GetInformation, I do prefer 
to keep original error handling since the AIP is provided by 3rd party drivers 
and some of them might produce a broken AIP.
Adding assert() to check such returned parameter might not function when the 
system is burned with a release built tip.

Thanks,
Ting


-Original Message-
From: Wu, Jiaxin 
Sent: Monday, January 04, 2016 9:50 AM
To: Leif Lindholm
Cc: Ye, Ting; edk2-devel@lists.01.org; Fu, Siyuan
Subject: RE: [edk2] [Patch] NetworkPkg: Fix suspicious dereference of pointer 
before NULL check

Leif,
As you mentioned you prefer clarifications instead of any testes 
(AipHandleCount and AipHandleBuffer) checking, I don't have strong opinion for 
adding more tests checking for API or adding the clarifications on the 
guaranteed behavior of the code. I think both of them good to me.

If you want to create another patch for the clarifications, please also remove 
the untrusted checks for LocateHandleBuffer() thoroughly.

Thanks.
Jiaxin

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Leif 
Lindholm
Sent: Thursday, December 31, 2015 7:00 PM
To: Wu, Jiaxin
Cc: Ye, Ting; edk2-devel@lists.01.org; Fu, Siyuan
Subject: Re: [edk2] [Patch] NetworkPkg: Fix suspicious dereference of pointer 
before NULL check

Hi Jiaxin,

On Mon, Dec 28, 2015 at 05:41:56AM +, Wu, Jiaxin wrote:
> Hi Lindholm,
> Sorry that misunderstand you, this patch just make the code more 
> readable. We know it's impossible return NULL pointer after 
> LocateHandleBuffer returns EFI_SUCCESS according UEFI Spec or any code 
> logic, but if anyone doesn't have those knowledge(or unaware of 
> checking UEFI Spec/any code logic), he maybe thought AipHandleBuffer 
> is suspicious dereference of pointer especially with below piece
> code:
>
> ...
> Exit:
>   ...
>   if (AipHandleBuffer != NULL) {
> FreePool (AipHandleBuffer);
>   }
>   ...

Thank you.

Ok, this makes me understand why the change was made, and would have been good 
to have covered in the commit message. I realise there is a bit of a language 
issue here as well. With your explanation, I can see what the commit message is 
intended to mean. But just reading the message on its own does not convey this 
meaning to me.
If the message had contained more information, this would have been less of a 
problem.

And I am not saying that the change is bad, but I will make an observation on 
that piece of code in general:
It looks like it does not trust the APIs it is using.
This applies both before and after this specific commit.

We start out with:
---
  if (EFI_ERROR (Status) || AipHandleCount == 0) {
  return EFI_NOT_FOUND;
  }
---

But we already know that LocateHandleBuffer returns EFI_NOT_FOUND if no handles 
were found. So the added test for AipHandleCount adds no added confidence to 
the program.
Similarily, we know that if LocateHandleBuffer returns EFI_SUCCESS, 
AipHandleBuffer contains a valid pointer.

Thus, both of these tests (AipHandleCount and AipHandleBuffer) are checking for 
API compliance rather than execution time success. This makes sense in a test 
suite, but not in core runtime code.

I do agree with you that this behaviour is not obvious unless I read it with 
the UEFI specification next to me, but I would very much prefer clarifications 
on the guaranteed behaviour of the code to be made in the form of comments. So 
the way I would have written this code would have been:
---
  Status = gBS->LocateHandleBuffer (
ByProtocol,
,
NULL,
,

);

  if (EFI_ERROR(Status)) {
return EFI_NOT_FOUND;
  }

  // Guaranteed that AipHandleCount > 0 and AipHandleBuffer != NULL
---

Now, the compiler also does not know about the UEFI specification - and 
especially more recent GCC versions frequently complain in instances like this. 
That was why I was wondering if you were working around a compiler problem.

The explicit initializations
---
  AipHandleCount  = 0;
  AipHandleBuffer = NULL;
---
resolve this problem.

Ting: would you accept a patch that did the above - replacing the additional 
test and the ASSERT() with a comment?

Regards,

Leif

> Thanks.
> Jiaxin
> 
> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: Monday, December 28, 2015 12:48 AM
> To: Wu, Jiaxin
> Cc: edk2-devel@lists.01.org; Ye, Ting; Fu, Siyuan
> Subject: Re: [edk2] [Patch] NetworkPkg: Fix suspicious dereference of 
> pointer before NULL 

Re: [edk2] [patch] MdeModulePkg:Fix the potential memory leak issue in Display Engine

2016-01-04 Thread Gao, Liming
Reviewed-by: Liming Gao 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Dandan Bi
Sent: Tuesday, January 05, 2016 9:59 AM
To: edk2-devel@lists.01.org
Cc: Dong, Eric; Gao, Liming
Subject: [edk2] [patch] MdeModulePkg:Fix the potential memory leak issue in 
Display Engine

The MenuOption insert to gMenuOption allocate memory everytime,but not free.
Now add the code to free it.

Cc: Liming Gao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 .../Universal/DisplayEngineDxe/FormDisplay.c   | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c 
b/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
index a391442..66f7dff 100644
--- a/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
+++ b/MdeModulePkg/Universal/DisplayEngineDxe/FormDisplay.c
@@ -3728,10 +3728,39 @@ UiDisplayMenu (
 }
   }
 }
 
 /**
+  Free the UI Menu Option structure data.
+
+  @param   MenuOptionList Point to the menu option list which need to 
be free.
+
+**/
+
+VOID
+FreeMenuOptionData(
+  LIST_ENTRY   *MenuOptionList
+  )
+{
+  LIST_ENTRY   *Link;
+  UI_MENU_OPTION   *Option;
+
+  //
+  // Free menu option list
+  //
+  while (!IsListEmpty (MenuOptionList)) {
+Link = GetFirstNode (MenuOptionList);
+Option = MENU_OPTION_FROM_LINK (Link);
+if (Option->Description != NULL){
+  FreePool(Option->Description);
+}
+RemoveEntryList (>Link);
+FreePool (Option);
+  }
+}
+
+/**
 
   Base on the browser status info to show an pop up message.
 
 **/
 VOID
@@ -3999,10 +4028,15 @@ FormDisplay (
   mIsFirstForm= FALSE;
   gOldFormEntry.HiiHandle = FormData->HiiHandle;
   CopyGuid (, >FormSetGuid);
   gOldFormEntry.FormId= FormData->FormId;
 
+  //
+  //Free the Ui menu option list.
+  //
+  FreeMenuOptionData();
+
   return Status;
 }
 
 /**
   Clear Screen to the initial state.
-- 
1.9.5.msysgit.1

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


Re: [edk2] [patch] MdeModulePkg:Change the type of BootNext

2016-01-04 Thread Gao, Liming
Reviewed-by: Liming Gao 

-Original Message-
From: Bi, Dandan 
Sent: Tuesday, January 05, 2016 8:34 AM
To: edk2-devel@lists.01.org
Cc: Gao, Liming; Dong, Eric
Subject: [patch] MdeModulePkg:Change the type of BootNext

Currently the invalid boot next set to the number of boot option, when add a 
new boot option,also need update the boot next value, otherwise it will be 
incorrect.Now set the type of BootNext value to UINT32,the number out of the 
range of UINT16 means it is an invalid BootNext Value.

Cc: Liming Gao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 .../BootMaintenanceManagerLib/BootMaintenance.c|  2 +-
 .../BootMaintenanceManagerLib/BootMaintenanceManager.h |  2 ++
 .../Library/BootMaintenanceManagerLib/FormGuid.h   |  2 +-
 .../Library/BootMaintenanceManagerLib/UpdatePage.c | 18 +-
 .../Library/BootMaintenanceManagerLib/Variable.c   |  2 +-
 5 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/MdeModulePkg/Library/BootMaintenanceManagerLib/BootMaintenance.c 
b/MdeModulePkg/Library/BootMaintenanceManagerLib/BootMaintenance.c
index 55c294d..2d52f9b 100644
--- a/MdeModulePkg/Library/BootMaintenanceManagerLib/BootMaintenance.c
+++ b/MdeModulePkg/Library/BootMaintenanceManagerLib/BootMaintenance.c
@@ -1293,11 +1293,11 @@ InitializeBmmConfig (
   InitializeDrivers (CallbackData);
 
   //
   // Initialize data which located in BMM main page
   //
-  CallbackData->BmmFakeNvData.BootNext = (UINT16) (BootOptionMenu.MenuNumber);
+  CallbackData->BmmFakeNvData.BootNext = NONE_BOOTNEXT_VALUE;
   for (Index = 0; Index < BootOptionMenu.MenuNumber; Index++) {
 NewMenuEntry= BOpt_GetMenuEntry (, Index);
 NewLoadContext  = (BM_LOAD_CONTEXT *) NewMenuEntry->VariableContext;
 
 if (NewLoadContext->IsBootNext) {
diff --git 
a/MdeModulePkg/Library/BootMaintenanceManagerLib/BootMaintenanceManager.h 
b/MdeModulePkg/Library/BootMaintenanceManagerLib/BootMaintenanceManager.h
index 6934a69..24526e1 100644
--- a/MdeModulePkg/Library/BootMaintenanceManagerLib/BootMaintenanceManager.h
+++ b/MdeModulePkg/Library/BootMaintenanceManagerLib/BootMaintenanceMana
+++ ger.h
@@ -231,10 +231,12 @@ typedef enum _TYPE_OF_TERMINAL {
 #define COM_TERMINAL_QUESTION_IDQUESTION_ID (COMTerminalType)
 #define COM_FLOWCONTROL_QUESTION_ID QUESTION_ID (COMFlowControl)
 
 #define STRING_DEPOSITORY_NUMBER8
 
+#define NONE_BOOTNEXT_VALUE (0x + 1)
+
 ///
 /// Serial Ports attributes, first one is the value for  /// return from 
callback function, stringtoken is used to  /// display the value properly  /// 
diff --git a/MdeModulePkg/Library/BootMaintenanceManagerLib/FormGuid.h 
b/MdeModulePkg/Library/BootMaintenanceManagerLib/FormGuid.h
index 3e1990d..cf14b40 100644
--- a/MdeModulePkg/Library/BootMaintenanceManagerLib/FormGuid.h
+++ b/MdeModulePkg/Library/BootMaintenanceManagerLib/FormGuid.h
@@ -101,11 +101,11 @@ typedef struct {
   //
   // Three questions displayed at the main page
   // for Timeout, BootNext, Variables respectively
   //
   UINT16  BootTimeOut;
-  UINT16  BootNext;
+  UINT32  BootNext;
 
   //
   // This is the COM1 Attributes value storage
   //
   UINT8   COM1BaudRate;
diff --git a/MdeModulePkg/Library/BootMaintenanceManagerLib/UpdatePage.c 
b/MdeModulePkg/Library/BootMaintenanceManagerLib/UpdatePage.c
index 78ace0c..cd1756a 100644
--- a/MdeModulePkg/Library/BootMaintenanceManagerLib/UpdatePage.c
+++ b/MdeModulePkg/Library/BootMaintenanceManagerLib/UpdatePage.c
@@ -684,51 +684,51 @@ UpdateBootNextPage (
 
   if (NumberOfOptions > 0) {
 OptionsOpCodeHandle = HiiAllocateOpCodeHandle ();
 ASSERT (OptionsOpCodeHandle != NULL);
 
-CallbackData->BmmFakeNvData.BootNext = (UINT16) 
(BootOptionMenu.MenuNumber);
+CallbackData->BmmFakeNvData.BootNext = NONE_BOOTNEXT_VALUE;
 
 for (Index = 0; Index < BootOptionMenu.MenuNumber; Index++) {
   NewMenuEntry= BOpt_GetMenuEntry (, Index);
   NewLoadContext  = (BM_LOAD_CONTEXT *) NewMenuEntry->VariableContext;
 
   if (NewLoadContext->IsBootNext) {
 HiiCreateOneOfOptionOpCode (
   OptionsOpCodeHandle,
   NewMenuEntry->DisplayStringToken,
   EFI_IFR_OPTION_DEFAULT,
-  EFI_IFR_TYPE_NUM_SIZE_16,
+  EFI_IFR_TYPE_NUM_SIZE_32,
   Index
   );
 CallbackData->BmmFakeNvData.BootNext = Index;
   } else {
 HiiCreateOneOfOptionOpCode (
   OptionsOpCodeHandle,
   NewMenuEntry->DisplayStringToken,
   0,
-  EFI_IFR_TYPE_NUM_SIZE_16,
+  EFI_IFR_TYPE_NUM_SIZE_32,
   Index
   );
   }
 }
 
-if (CallbackData->BmmFakeNvData.BootNext == Index) {
+if (CallbackData->BmmFakeNvData.BootNext == NONE_BOOTNEXT_VALUE) {
   HiiCreateOneOfOptionOpCode (
 OptionsOpCodeHandle,
   

Re: [edk2] [PATCH v2] MdeModulePkg: Update MNP driver to recycle TX buffer asynchronously.

2016-01-04 Thread Fu, Siyuan
Thank Jiaxin, I will update the content when commit the patch.

Siyuan

-Original Message-
From: Wu, Jiaxin 
Sent: Tuesday, January 5, 2016 3:34 PM
To: Fu, Siyuan ; edk2-devel@lists.01.org
Cc: Ye, Ting 
Subject: RE: [edk2] [PATCH v2] MdeModulePkg: Update MNP driver to recycle TX 
buffer asynchronously.

Siyuan,
Some small comments:
1. Please update MnpBuildTxPacket() function's description.
2. Since Mnp Sent the packet asynchronously, MnpSyncSendPacket() function's 
name is improper, the corresponding description also should be updated.

Other parts good to me.

Reviewed-by: Jiaxin Wu 

Thanks.
Jiaxin


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu Siyuan
Sent: Tuesday, January 5, 2016 11:37 AM
To: edk2-devel@lists.01.org
Cc: Ye, Ting; Wu, Jiaxin
Subject: [edk2] [PATCH v2] MdeModulePkg: Update MNP driver to recycle TX buffer 
asynchronously.

This patch updates the MNP driver to recycle TX buffer asynchronously, instead 
of using a while loop wait after each transmit command.
This patch also fixes a bug in SNP.GetStatus() interface to support the MNP 
update. The UNDI driver may return multiple transmitted buffers in a single 
GetStatus command, while SNP.GetStatus could only return one pointer each time, 
the rest of them are lost. This patch fixes this issue by store these recycled 
pointer in a temporary buffer in SNP driver.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c  | 266 ++---
 MdeModulePkg/Universal/Network/MnpDxe/MnpDriver.h  |   7 +-
 MdeModulePkg/Universal/Network/MnpDxe/MnpImpl.h|  45 +++-
 MdeModulePkg/Universal/Network/MnpDxe/MnpIo.c  | 118 -
 MdeModulePkg/Universal/Network/MnpDxe/MnpMain.c|   7 +-
 MdeModulePkg/Universal/Network/SnpDxe/Get_status.c |  73 --
 MdeModulePkg/Universal/Network/SnpDxe/Snp.c|  16 +-
 MdeModulePkg/Universal/Network/SnpDxe/Snp.h|  18 +-
 8 files changed, 421 insertions(+), 129 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c 
b/MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c
index 046d4df..d1a4cb5 100644
--- a/MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c
+++ b/MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c
@@ -1,7 +1,7 @@
 /** @file
   Implementation of Managed Network Protocol private services.
 
-Copyright (c) 2005 - 2012, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 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 @@ -209,6 +209,208 @@ MnpFreeNbuf (
   gBS->RestoreTPL (OldTpl);
 }
 
+/**
+  Add Count of TX buffers to MnpDeviceData->AllTxBufList and 
MnpDeviceData->FreeTxBufList.
+  The length of the buffer is specified by MnpDeviceData->BufferLength.
+
+  @param[in, out]  MnpDeviceData Pointer to the MNP_DEVICE_DATA.
+  @param[in]   Count Number of TX buffers to add.
+
+  @retval EFI_SUCCESS   The specified amount of TX buffers are 
allocated.
+  @retval EFI_OUT_OF_RESOURCES  Failed to allocate a TX buffer.
+
+**/
+EFI_STATUS
+MnpAddFreeTxBuf (
+  IN OUT MNP_DEVICE_DATA   *MnpDeviceData,
+  IN UINTN Count
+  )
+{
+  EFI_STATUSStatus;
+  UINT32Index;
+  MNP_TX_BUF_WRAP   *TxBufWrap;
+
+  NET_CHECK_SIGNATURE (MnpDeviceData, MNP_DEVICE_DATA_SIGNATURE); 
+ ASSERT ((Count > 0) && (MnpDeviceData->BufferLength > 0));
+
+  Status = EFI_SUCCESS;
+  for (Index = 0; Index < Count; Index++) {
+TxBufWrap = (MNP_TX_BUF_WRAP*) AllocatePool (sizeof (MNP_TX_BUF_WRAP) + 
MnpDeviceData->BufferLength - 1);
+if (TxBufWrap == NULL) {
+  DEBUG ((EFI_D_ERROR, "MnpAddFreeTxBuf: TxBuf Alloc failed.\n"));
+
+  Status = EFI_OUT_OF_RESOURCES;
+  break;
+}
+DEBUG ((EFI_D_INFO, "MnpAddFreeTxBuf: Add TxBufWrap %p, TxBuf %p\n", 
TxBufWrap, TxBufWrap->TxBuf));
+TxBufWrap->Signature = MNP_TX_BUF_WRAP_SIGNATURE;
+TxBufWrap->InUse = FALSE;
+InsertTailList (>FreeTxBufList, >WrapEntry);
+InsertTailList (>AllTxBufList, 
+ >AllEntry);  }
+
+  MnpDeviceData->TxBufCount += Index;
+  return Status;
+}
+
+/**
+  Allocate a free TX buffer from MnpDeviceData->FreeTxBufList. If there 
+is none
+  in the queue, first try to recycle some from SNP, then try to 
+allocate some and add
+  them into the queue, then fetch the NET_BUF from the updated FreeTxBufList.
+
+  @param[in, out]  MnpDeviceDataPointer to the MNP_DEVICE_DATA.
+
+  @return Pointer to the allocated free NET_BUF structure, if NULL the
+  operation is failed.
+
+**/
+UINT8 *
+MnpAllocTxBuf (
+  IN OUT MNP_DEVICE_DATA   *MnpDeviceData
+  )
+{
+  EFI_TPL   OldTpl;
+  UINT8 

Re: [edk2] [PATCH] StdLib: Implement da_ConFlush() and flush I/O buffers when closing a console device.

2016-01-04 Thread Bjorge, Erik C
Reviewed-by: Erik Bjorge 

> -Original Message-
> From: Daryl McDaniel [mailto:edk2-li...@mc2research.org]
> Sent: Sunday, January 3, 2016 2:45 PM
> To: edk2-devel@lists.01.org; Carsey, Jaben ; Bjorge, 
> Erik C
> 
> Subject: [PATCH] StdLib: Implement da_ConFlush() and flush I/O buffers when 
> closing a
> console device.
> 
> StdLib: Implement da_ConFlush() and flush I/O buffers when closing a console 
> device.
> 
> Add header file Efi/SysEfi.h
> Implement function da_ConFlush()
> Modify da_ConClose() to flush its buffers and clean up better upon close.
> Construct the console instance using the new da_ConFlush() instead of the 
> nullop function.
> Remove da_ConFlush() from the "Not implemented (yet?)" place holder.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Daryl McDaniel 
> 
> ---
> diff U3 a/StdLib/LibC/Uefi/Devices/Console/daConsole.c
> b/StdLib/LibC/Uefi/Devices/Console/daConsole.c
> --- a/StdLib/LibC/Uefi/Devices/Console/daConsole.c  Tue Nov 11 14:56:58 2014
> +++ b/StdLib/LibC/Uefi/Devices/Console/daConsole.c  Sun Jan 03 12:16:39 2016
> @@ -10,6 +10,7 @@
>The devices status as a wide device is indicatd by _S_IWTTY being set in
>f_iflags.
> 
> +  Copyright (c) 2016, Daryl McDaniel. All rights reserved.
>Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.
>This program and the accompanying materials are licensed and made 
> available under
>the terms and conditions of the BSD License that accompanies this 
> distribution.
> @@ -37,6 +38,7 @@
>  #include  
>  #include  
>  #include  
> +#include  
>  #include  
>  #include  
>  #include  
> @@ -104,6 +106,65 @@
>return i;
>  }
> 
> +/** Flush the console's IIO buffers.
> +
> +Flush the IIO Input or Output buffers depending upon the mode
> +of the specified file.
> +
> +If the console is open for output, write any unwritten data in the output
> +buffer to the console.
> +
> +If the console is open for input or output, discard any remaining data
> +in the associated buffers.
> +
> +@param[in]filpPointer to the target file's descriptor structure.
> +
> +@retval 0 Always succeeds
> +**/
> +static
> +int
> +EFIAPI
> +da_ConFlush(
> +  struct __filedes *filp
> +)
> +{
> +  cIIO   *This;
> +  char   *MbcsPtr;
> +  ssize_t NumProc;
> +  void   *OutPtr;
> +
> +  This = filp->devdata;
> +
> +  if (filp->f_iflags & S_ACC_READ)  { // Readable so flush the input 
> buffer
> +This->InBuf->Flush(This->InBuf, UNICODE_STRING_MAX);
> +  }
> +  if (filp->f_iflags & S_ACC_WRITE)  {// Writable so flush the output 
> buffer
> +// At this point, the characters to write are in OutBuf
> +// First, linearize and consume the buffer
> +NumProc = OutBuf->Read(OutBuf, gMD->UString, UNICODE_STRING_MAX-1);
> +if (NumProc > 0) {  // Optimization -- Nothing to do if no characters
> +  gMD->UString[NumProc] = 0;   // Ensure that the buffer is terminated
> +
> +  if(filp->f_iflags & _S_IWTTY) {
> +// Output device expects wide characters, Output what we have
> +OutPtr = gMD->UString;
> +  }
> +  else {
> +// Output device expects narrow characters, convert to MBCS
> +OutPtr = gMD->UString2;
> +// Translate the wide buffer, gMD->UString into MBCS
> +// in the buffer pointed to by OutPtr.
> +// The returned value, NumProc, is the resulting number of bytes.
> +NumProc = wcstombs((char *)OutPtr, (const wchar_t *)gMD->UString, 
> NumProc);
> +((char *)OutPtr)[NumProc] = 0;   // Ensure the buffer is terminated
> +  }
> +  // Do the actual write of the data
> +  (void) filp->f_ops->fo_write(filp, NULL, NumProc, OutPtr);
> +}
> +  }
> +  return 0;
> +}
> +
>  /** Close an open file.
> 
>  @param[in]  filpPointer to the file descriptor structure for this 
> file.
> @@ -127,6 +188,13 @@
>  EFIerrno = RETURN_INVALID_PARAMETER;
>  return -1;// Looks like a bad File Descriptor pointer
>}
> +  // Stream and filp look OK, so continue.
> +  // Flush the I/O buffers
> +  (void) da_ConFlush(filp);
> +
> +  // Break the connection to IIO
> +  filp->devdata = NULL;
> +
>gMD->StdIo[Stream->InstanceNum] = NULL;   // Mark the stream as closed
>return 0;
>  }
> @@ -188,10 +256,10 @@
>  by da_ConWrite are WIDE characters.  It is the responsibility of the
>  higher-level function(s) to perform any necessary conversions.
> 
> -@param[in,out]  BufferSize  Number of characters in Buffer.
> +  @param[in,out]  BufferSize  Number of characters in Buffer.
>@param[in]  Buffer  The WCS string to be displayed
> 
> -@return   The number of Characters written.
> +  @return   The number of Characters written.
>  */
>  static
>  ssize_t
> @@ -525,7 +593,7 @@
>wchar_t

Re: [edk2] [PATCH] StdLib: Temporarily restrict compiler warnings so that sockets can be built using VS2015.

2016-01-04 Thread Bjorge, Erik C
Daryl,

What are the plans for fixing the socket code to pass with VS2015?  I just want 
to know how long this temporary patch will be around.

Thanks,
-Erik

From: Daryl McDaniel [mailto:edk2-li...@mc2research.org]
Sent: Sunday, January 3, 2016 2:21 PM
To: edk2-devel@lists.01.org; Carsey, Jaben ; Bjorge, 
Erik C 
Subject: [PATCH] StdLib: Temporarily restrict compiler warnings so that sockets 
can be built using VS2015.


StdLib: Temporarily restrict compiler warnings so that sockets can be built 
using VS2015.



Contributed-under: TianoCore Contribution Agreement 1.0

Signed-off-by: Daryl McDaniel 
>



---

diff U3 a/StdLib/StdLib.inc b/StdLib/StdLib.inc

--- a/StdLib/StdLib.inc Tue Aug 11 21:25:12 2015

+++ b/StdLib/StdLib.inc Sun Jan 03 12:17:42 2016

@@ -5,6 +5,7 @@

# The including DSC file must DEFINE the EMULATE macro if

# the application is to be run in an emulation environment.

#

+#  Copyright (c) 2016, Daryl McDaniel. All rights reserved.

#  Copyright (c) 2011 - 2014, 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

@@ -133,3 +134,12 @@

 RVCT:*_*_*_CC_FLAGS = --library_interface=none -DUEFI_C_SOURCE 
-J$(WORKSPACE)/StdLib/Include -J$(WORKSPACE)/StdLib/Include/Arm

XCODE:*_*_*_CC_FLAGS = -nostdinc -nostdlib -DUEFI_C_SOURCE 
-Wno-unused-const-variable -Wno-string-compare -Wno-sometimes-uninitialized

!endif

+

+  # Temporarily restrict compiler warnings to those produced by VS2012.

+  # Code that fails when these flags are removed will have to be rewritten

+  # in order to pass.  This may be as simple as renaming an object, but may

+  # require more significant changes.

+MSFT:*_VS2015_*_CC_FLAGS  = /Wv:11

+MSFT:*_VS2015x86_*_CC_FLAGS   = /Wv:11

+MSFT:*_VS2015xASL_*_CC_FLAGS  = /Wv:11

+MSFT:*_VS2015x86xASL_*_CC_FLAGS   = /Wv:11

--

Daryl McDaniel



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


Re: [edk2] [PATCH] StdLib: Fix IIO_Write() to return the number of bytes consumed, not characters output.

2016-01-04 Thread Bjorge, Erik C
I am not sure I like how the W_INSTRUMENT and R_INSTRUMENT look in the code but 
I also do not have a good alternate suggestion.  It just looked odd when I was 
reading the patch.

Reviewed-by: Erik Bjorge 

> -Original Message-
> From: Daryl McDaniel [mailto:edk2-li...@mc2research.org]
> Sent: Sunday, January 3, 2016 2:38 PM
> To: 'Alexey Vegner' ; 
> edk2-devel@lists.01.org; Carsey,
> Jaben ; Bjorge, Erik C 
> Subject: [PATCH] StdLib: Fix IIO_Write() to return the number of bytes 
> consumed, not
> characters output.
> 
> StdLib: Fix IIO_Write() to return the number of bytes consumed, not 
> characters output.
> 
> Depending upon termios settings, writing to a terminal device may result in
> many more characters being output than were in the buffer provided to the
> IIO_Write() function.
> 
> IIO_Write() is supposed to return the number of BYTES written, not characters.
> Since the provided buffer contains MBCS characters, there can be up to three
> bytes per character.  Due to the expansion that may occur, "BYTES written"
> is interpreted to mean the number of BYTES consumed from the MBCS buffer
> provided as a parameter to IIO_Write.
> 
> These changes ensure that the correct number of characters are consumed from
> the internal Output buffer and the correct number of BYTES consumed from the
> buffer parameter are counted and returned.
> 
> Update copyright.
> Add debugging instrumentation to count unconsumed data in the Input and 
> Output buffers.
> Improve comments for IIO_Write().
> Modify IIO_Write() to:
>   * Accurately count input bytes CONSUMED.
>   * Consume only as many expanded (cooked) characters from the output buffer
> as were actually sent to the device.
>   * Purge only the number of CHARACTERS sent from the output buffer.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Daryl McDaniel 
> 
> ---
> diff U3 a/StdLib/LibC/Uefi/InteractiveIO/IIO.c 
> b/StdLib/LibC/Uefi/InteractiveIO/IIO.c
> --- a/StdLib/LibC/Uefi/InteractiveIO/IIO.c  Tue Dec 11 13:19:14 2012
> +++ b/StdLib/LibC/Uefi/InteractiveIO/IIO.c  Sun Jan 03 09:35:14 2016
> @@ -3,6 +3,7 @@
> 
>The functions assume that isatty() is TRUE at the time they are called.
> 
> +  Copyright (c) 2016, Daryl McDaniel. All rights reserved.
>Copyright (c) 2012, 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
> @@ -26,6 +27,20 @@
>  #include  "IIOutilities.h"
>  #include  "IIOechoCtrl.h"
> 
> +// Instrumentation used for debugging
> +#define   IIO_C_DEBUG   0   ///< Set to 1 to enable instrumentation, 0 
> to disable
> +
> +#if IIO_C_DEBUG
> +  static volatile size_t  IIO_C_WRemainder = 0;   ///< Characters in Out 
> buffer after
> IIO_Write
> +  static volatile size_t  IIO_C_RRemainder = 0;   ///< Characters in In 
> buffer after
> IIO_Read
> +
> +  #define W_INSTRUMENT  IIO_C_WRemainder =
> +  #define R_INSTRUMENT  IIO_C_RRemainder =
> +#else // ! IIO_C_DEBUG -- don't instrument code
> +  #define W_INSTRUMENT  (void)
> +  #define R_INSTRUMENT  (void)
> +#endif  // IIO_C_DEBUG
> +
>  /** Read from an Interactive IO device.
> 
>NOTE: If _S_IWTTY is set, the internal buffer contains WIDE characters.
> @@ -82,24 +97,44 @@
>NumRead = wcstombs((char *)Buffer, (const wchar_t *)gMD->UString2, 
> XlateSz);
> 
>// Consume the translated characters
> -  (void)This->InBuf->Flush(This->InBuf, Needed);
> +  (void) This->InBuf->Flush(This->InBuf, Needed);
>  }
>  else {
>// Data in InBuf is narrow characters.  Use verbatim.
>NumRead = This->InBuf->Read(This->InBuf, Buffer, (INT32)BufferSize);
>  }
> +#if IIO_C_DEBUG
> +IIO_C_RRemainder = This->InBuf->Count(This->InBuf, AsElements);
> +#endif // IIO_C_DEBUG
>}
>return NumRead;
>  }
> 
> -/** Process characters from buffer buf and write them to the output device
> +/** Handle write to a Terminal (Interactive) device.
> +
> +Processes characters from buffer buf and writes them to the Terminal 
> device
>  specified by filp.
> 
> +The parameter buf points to a MBCS string to be output. This is processed
> +and buffered one character at a time by IIO_WriteOne() which handles TAB
> +expansion, NEWLINE to CARRIAGE_RETURN + NEWLINE expansion, as well as
> +basic line editing functions. The number of characters actually written 
> to
> +the output device will seldom equal the number of characters consumed 
> from
> +buf.
> +
> +In this implementation, all of the special characters processed by
> +IIO_WriteOne() are single-byte characters with values less than 128.
> +(7-bit ASCII or the single-byte UTF-8 characters)
> +
> +Every byte that is not one of the recognized 

Re: [edk2] [PATCH] Python: Clean up and document how to escape the -# option.

2016-01-04 Thread Bjorge, Erik C
Reviewed-by: Erik Bjorge 

> -Original Message-
> From: Daryl McDaniel [mailto:edk2-li...@mc2research.org]
> Sent: Sunday, January 3, 2016 4:07 PM
> To: edk2-devel@lists.01.org; Carsey, Jaben ; Bjorge, 
> Erik C
> 
> Subject: [PATCH] Python: Clean up and document how to escape the -# option.
> 
> AppPkg/.../Python: Clean up and document how to escape the -# option.
> 
> Depending upon the version of Shell you are using, it may be necessary
> to escape the '#' character, when using the "-#" command-line option, so that
> the Shell doesn't interpret it as the start of a comment.
> The escape character is '^'.
> Example:
> python -^# -V
> 
> * General updating.
> * Re-format so that no line is longer than 80 char.
> * Add note about escaping the "-#" command-line option.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Daryl McDaniel 
> 
> ---
> diff U3 a/AppPkg/Applications/Python/PythonReadMe.txt
> b/AppPkg/Applications/Python/PythonReadMe.txt
> --- a/AppPkg/Applications/Python/PythonReadMe.txt Fri Oct 25 12:09:26 2013
> +++ b/AppPkg/Applications/Python/PythonReadMe.txt Sun Jan 03 15:43:26 2016
> @@ -1,7 +1,8 @@
>  EDK II Python
> -ReadMe
> - Release 1.02
> - 18 Jan. 2013
> +   ReadMe
> +Version 2.7.2
> +Release 1.02
> +18 Jan. 2013
> 
> 
>  1. OVERVIEW
> @@ -25,10 +26,10 @@
>  ==
>3.1 Getting Python
>==
> -  Currently only version 2.7.2 of the CPython distribution is supported.  
> For development
> -  ease, a subset of the Python 2.7.2 distribution has been included in the 
> AppPkg source
> -  tree.  If a full distribution is desired, the Python-2.7.2 directory can 
> be removed or
> -  renamed and the full source code downloaded from
> http://www.python.org/ftp/python/2.7.2/.
> +  For development ease, a subset of the Python 2.7.2 distribution has been
> +  included in the AppPkg source tree.  If a full distribution is desired, the
> +  Python-2.7.2 directory can be removed or renamed and the full source code
> +  downloaded from http://www.python.org/ftp/python/2.7.2/.
> 
>A.  Within your EDK II development tree, extract the Python distribution 
> into
>  AppPkg/Applications/Python.  This should create the
> @@ -70,7 +71,8 @@
> |- \etc  Configuration files used by libraries.
> |- \tmp  Temporary files created by tmpfile(), 
> etc.
> |- \lib  Root of the libraries tree.
> -   |- \python.27Directory containing the Python library 
> modules.
> +   |- \python.27Directory containing the Python library
> +   |modules.
> |- \lib-dynload  Dynamically loadable Python extensions.
> |- \site-packagesSite-specific packages and modules.
> 
> @@ -81,7 +83,7 @@
>  system as follows:
> 
>* \Efi\Tools receives a copy of Build/AppPkg/DEBUG_VS2005/X64/Python.efi.
> -   ^ ^^
> +   ^ ^^
>  Modify the host path to match the your build type and compiler.
> 
>* The \Efi\StdLib\etc directory is populated from the StdLib/Efi/StdLib/etc
> @@ -94,8 +96,8 @@
>  sitetypes copy_reglinecache genericpath
> 
>* Python C Extension Modules built as dynamically loadable extensions go 
> into
> -the \Efi\StdLib\lib\python.27\lib-dynload directory.  This functionality 
> is not
> -yet implemented.
> +the \Efi\StdLib\lib\python.27\lib-dynload directory.  This functionality 
> is
> +not yet implemented.
> 
> 
>  6. Example: Enabling socket support
> @@ -107,11 +109,48 @@
>  functools, types, os, sys, warnings, cStringIO, StringIO, errno
> 
>5.  build -a X64 -p AppPkg\AppPkg.dsc
> -  6.  copy Build\AppPkg\DEBUG_VS2005\X64\Python.efi to \Efi\Tools on your 
> target system.
> - Modify as needed
> +  6.  copy Build\AppPkg\DEBUG_VS2005\X64\Python.efi to \Efi\Tools on your
> +  target system. Replace "DEBUG_VS2005\X64", in the source path, with
> +  values appropriate for your tool chain and processor architecture.
> +
> +
> +7. Running Python
> +=
> +  Python must currently be run from an EFI FAT-32 partition, or volume, under
> +  the UEFI Shell.  At the Shell prompt enter the desired volume name, 
> followed
> +  by a colon ':', then press Enter.  Python can then be executed by typing 
> its
> +  name, followed by any desired options and 

Re: [edk2] [PATCH] StdLib: Clarify and improve comments.

2016-01-04 Thread Bjorge, Erik C
Please also note in the comments for the commits where you fixed indentation 
issues.

Reviewed-by: Erik Bjorge 

> -Original Message-
> From: Daryl McDaniel [mailto:edk2-li...@mc2research.org]
> Sent: Sunday, January 3, 2016 2:51 PM
> To: edk2-devel@lists.01.org; Carsey, Jaben ; Bjorge, 
> Erik C
> 
> Subject: [PATCH] StdLib: Clarify and improve comments.
> 
> StdLib: Clarify and improve comments.
> 
> LibC/Locale/multibyte_Utf8.c
> LibC/Uefi/SysCalls.c
>   Clarify and improve comments.
> 
> Include/sys/termios.h
>   Add parameter names to function prototypes as referenced in the comments.
> 
> StdLibPrivateInternalFiles\Include\kfile.h
>   Add comment for the fo_close fileop.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Daryl McDaniel 
> 
> ---
>   .../StdLib/LibC/Locale/multibyte_Utf8.c |  20 +++-
>   .../StdLib/LibC/Uefi/SysCalls.c | 127 +++--
>   .../StdLib/Include/sys/termios.h|  11 ++-
>   .../StdLibPrivateInternalFiles\Include\kfile.h  |  11 +--
>   4 files changed, 95 insertions(+), 74 deletions(-)
> 
> diff U3 a/StdLib/LibC/Locale/multibyte_Utf8.c 
> b/StdLib/LibC/Locale/multibyte_Utf8.c
> --- a/StdLib/LibC/Locale/multibyte_Utf8.c Tue May 14 17:59:11 2013
> +++ b/StdLib/LibC/Locale/multibyte_Utf8.c Sun Jan 03 12:45:02 2016
> @@ -1,4 +1,5 @@
>  /** @file
> +  Copyright (c) 2016, Daryl McDaniel. All rights reserved.
>Copyright (c) 2012, 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
> @@ -16,7 +17,7 @@
>  #include  
>  #include  
> 
> -typedef  int  ch_UCS4;
> +typedef int  ch_UCS4;
> 
>  static  mbstate_t LocalConvState = {0};
> 
> @@ -79,7 +80,7 @@
>  // We are in an invalid state
>  ps->A = 0;// Initial State
>}
> -  ps->C[ps->A] = ch;  // Save the current character
> +  ps->C[ps->A] = ch;  // Save the current byte
>Mask = utf8_code_length[ch];
> 
>if(ps->A == 0) {// Initial State.  First byte of sequence.
> @@ -89,7 +90,7 @@
>case 0:   // State 0, Code 0
>  errno = EILSEQ;
>  RetVal = -1;
> -ps->E = 1;// Consume this character
> +ps->E = 1;// Consume this byte
>  break;
>case 1:   // State 0, Code 1
>  // ASCII-7 Character
> @@ -116,7 +117,7 @@
>  ps->A = 0;  // Next state is State-0
>  RetVal = 2;
>}
> -  else {// This isn't the last character, get more.  State 1, 
> Code 3 or 4
> +  else {// This isn't the last byte, get more.  State 1, Code 3 
> or 4
>  ps->A = 2;
>  RetVal = -2;
>}
> @@ -158,12 +159,12 @@
>  errno = EILSEQ;
>  ps->A = 0;
>  RetVal = -1;
> -ps->E = 4;  // Can't happen, but consume this character 
> anyway
> +ps->E = 4;  // Can't happen, but consume this byte anyway
>}
>break;
>}
>  }
> -else {// Invalid surrogate character
> +else {// Invalid surrogate byte
>errno = EILSEQ;
>ps->A = 0;  // Next is State-0
>RetVal = -1;
> @@ -287,7 +288,8 @@
> 
>  @param[in]Src   Pointer to a wide character string.
>  @param[in]Limit Maximum number of bytes the converted string may 
> occupy.
> -@param[out]   NumChar   Pointer to where to store the number of wide 
> characters, or
> NULL.
> +@param[out]   NumChar   Pointer to where to store the number of wide 
> characters
> +consumed, or NULL.
> 
>  @return The number of bytes required to convert Src to MBCS,
>  not including the terminating NUL.  If NumChar is not NULL, 
> the number
> @@ -961,8 +963,8 @@
>  @return   If a wide character is encountered that does not correspond to 
> a
>valid multibyte character, the wcstombs function returns
>(size_t)(-1). Otherwise, the wcstombs function returns the 
> number
> -  of bytes modified, not including a terminating null character,
> -  if any.
> +  of bytes in the resulting multibyte character sequence,
> +  not including the terminating null character (if any).
> 
>  Declared in: stdlib.h
>  **/
> diff U3 a/StdLib/LibC/Uefi/SysCalls.c b/StdLib/LibC/Uefi/SysCalls.c
> --- a/StdLib/LibC/Uefi/SysCalls.c Fri Dec 21 10:19:41 2012
> +++ b/StdLib/LibC/Uefi/SysCalls.c Sun Jan 03 12:52:28 2016
> @@ -1,6 +1,7 @@
>  /** @file
>EFI versions of NetBSD system calls.
> 
> +  Copyright (c) 2016, Daryl McDaniel. All rights reserved.
>Copyright (c) 

Re: [edk2] [PATCH 4/6] ArmVirtPkg: use small code model for all UEFI_APPLICATION modules

2016-01-04 Thread Laszlo Ersek
On 12/24/15 14:03, Ard Biesheuvel wrote:
> Unfortunately, compiling the DEBUG shell using the small code model is
> not sufficient in all cases to get a successful build when the toolchain
> defaults to the tiny code model. The reason is that not only the Shell binary
> itself should be built using the small code model, all Shell component
> libraries that are linked into the Shell binary via NULL library class
> resolution should use the small code model as well.
> 
> So override the code model and function alignment for DEBUG builds of
> UEFI_APPLICATION modules when using GCC 4.9 (which is the only toolchain
> that uses the tiny model). This should affect all Shell component libraries
> in addition to the Shell core binary.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/ArmVirt.dsc.inc | 21 
>  1 file changed, 21 insertions(+)
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index 49e4264ee8a4..fbd710cb870d 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -406,3 +406,24 @@ [Components.common]
>gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
>gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
>}
> +
> +[BuildOptions.AARCH64.EDKII.UEFI_APPLICATION]
> +  #
> +  # The bulk of the Shell functionality is implemented by UEFI_APPLICATION
> +  # libraries that are linked into the Shell binary via NULL library class
> +  # resolution. Since the Shell built in DEBUG mode exceeds the 1 MB range of
> +  # the AARCH64 tiny code model which we use by default on GCC 4.9 and later,
> +  # the .text and .data sections of the remaining base libraries (which are
> +  # built using the tiny code model regardless of the model we use for
> +  # UEFI_APPLICATION modules) should be kept as close together as possible.
> +  #
> +  # By reverting to 8 byte function alignment for UEFI_APPLICATION modules
> +  # (which is usually the default, but will be lowered to 4 if we are using 
> the
> +  # tiny code model) and letting the linker sort its input by alignment, we 
> can
> +  # force all UEFI_APPLICATION small code model .text input sections to 
> appear
> +  # first in the binary. The remaining base libraries will end up in close
> +  # proximity of each other at the end of the image, preventing out of range
> +  # problems when relocating their tiny model (+/- 1 MB) symbol references.
> +  #
> +  DEBUG_GCC49_*_CC_FLAGS = -mcmodel=small -falign-functions=8
> +  DEBUG_GCC49_*_DLINK_FLAGS = -z common-page-size=0x1000
> 

I don't have technical objections, just that I don't really like the
complexity of this.

In ,
you wrote

> Well, this is primarily caused by the way the Shell is composed of
> static libraries, and it is unlikely that this should ever affect
> other UEFI_APPLICATION modules as well. And building everything else
> with the large code model because of this seems backwards to me too.

I think I'd prefer a "one size fits all" code model where we (as
developers) get to keep our sanity, even if it costs a tiny bit more
resources at runtime. Can you please summarize (again?) the benefits of
the tiny model over the small model?

I can see the ADR / ADRP discussion in patch 3/6, but what does that
difference mean in practice?

What savings justify the code model proliferation between GCC <= 4.8
("large"), CLANG 3.5 ("small"), GCC >= 4.9 ("tiny", except for the DEBUG
Shell, which would be made "small" by the above)?

Would ArmVirtPkg build (with all supported toolchains) using the "small"
code model?

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