Re: [edk2] Library refinement: OptionRomPkg/BltLib

2015-08-09 Thread Ni, Ruiyu
Just drew a picture to show the current library APIs, V1 and V2 APIs, to help 
you understand my point.

[cid:image001.png@01D0D37C.E78C2030]



-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ni, Ruiyu
Sent: Monday, August 10, 2015 11:10 AM
To: Justen, Jordan L ; Laszlo Ersek 

Cc: edk2-devel@lists.01.org
Subject: [edk2] Library refinement: OptionRomPkg/BltLib



Jordan and Laszlo,

I reviewed the OptionRomPkg/BltLib again and would like to discuss with you 
about the potential API refinement.

I attached two versions of the refined BltLib.h.



The common part of the two versions is:



1.  BltLibGetSizes() is removed.



Because the size information is actually passed from the library consumer to 
the library. Caller do not need to get the information from the library 
interface.



2.  BltLibBufferToVideoEx and BltLibVideoToBltBufferEx are removed and the 
accordingly non-Ex version of APIs will be used for *Ex operation.

Just for reducing the library APIs and avoid the different APIs have overlapped 
functionality.



3.  BltLibGopBlt() is removed



Same reason as the above #2.



BltLib2.h contains another change.



4.  BltLibConfigure() is removed and the frame buffer information will be 
passed through the four BltLib* APIs.



Because it can remove the restriction to allow the library can be re-entry.



What's your thoughts about this?



Regards,

Ray


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


[edk2] error when use ovmf

2015-08-09 Thread ????
when i used ovmf.fd in qemu manager,
then occurcs this tips:
Boot Failed. EFI DVD/CDROM
Boot Failed. EFI Floopy


i don't know what does this mean?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH v2] BaseTools/GenFds: Fix 'NoneType' object is not iterable error.

2015-08-09 Thread Qiu Shumin
When adding section VERSION in FDF file, for example:
FILE FREEFORM = PCD(gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile) {
SECTION RAW = MdeModulePkg/Logo/Logo.bmp
SECTION UI = "Logo"
SECTION VERSION = "0001"
  }

GenFds will report the following error:
Traceback (most recent call last):
  File "GenFds.py", line 276, in main
  File "GenFds.py", line 391, in GenFd
  File "Fd.py", line 93, in GenFd
  File "Region.py", line 106, in AddToBuffer
  File "Fv.py", line 114, in AddToBuffer
  File "FfsFileStatement.py", line 117, in GenFfs
  File "VerSection.py", line 80, in GenSection
  File "GenFdsGlobalVariable.py", line 401, in GenerateSection
TypeError: 'NoneType' object is not iterable.

We found in GenFdsGlobalVariable.py line 401 'list' requires a iteralbe object 
as parameter while the 'Input' is None.

Cc: Liming Gao 
Cc: winggundu...@163.com
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin 
Reviewed-by: Yingke Liu 
---
 BaseTools/Source/Python/GenFds/VerSection.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/VerSection.py 
b/BaseTools/Source/Python/GenFds/VerSection.py
index 657ebd9..7399e7a 100644
--- a/BaseTools/Source/Python/GenFds/VerSection.py
+++ b/BaseTools/Source/Python/GenFds/VerSection.py
@@ -1,7 +1,7 @@
 ## @file
 # process Version section generation
 #
-#  Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.
+#  Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
@@ -76,7 +76,7 @@ class VerSection (VerSectionClassObject):
 else:
 StringData = ''
 
-GenFdsGlobalVariable.GenerateSection(OutputFile, None, 
'EFI_SECTION_VERSION',
+GenFdsGlobalVariable.GenerateSection(OutputFile, [], 
'EFI_SECTION_VERSION',
  Ver=StringData, 
BuildNumber=self.BuildNum)
 OutputFileList = []
 OutputFileList.append(OutputFile)
-- 
1.9.5.msysgit.1

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


Re: [edk2] [RFC PATCH 2/4] BaseTools GCC: unify warning flags for all GCC versions

2015-08-09 Thread Gao, Liming
Reviewed-by: Liming Gao 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: Friday, August 7, 2015 11:05 PM
To: edk2-devel@lists.01.org; Justen, Jordan L; Liu, Yingke D
Cc: Ard Biesheuvel
Subject: [edk2] [RFC PATCH 2/4] BaseTools GCC: unify warning flags for all GCC 
versions

The warning flags -Wno-address -Wno-unused-but-set-variable are added for 
version 4.6 and up, but since they are happily accepted by version
4.4 and 4.5 as well, add them there as well.

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

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 2f9533b56ec8..15d0ea3985ff 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -3844,7 +3844,7 @@ DEFINE GCC_IPF_RC_FLAGS= -I binary -O 
elf64-ia64-little   -B ia64
 DEFINE GCC_ARM_RC_FLAGS= -I binary -O elf32-littlearm -B arm   
  --rename-section .data=.hii
 DEFINE GCC_AARCH64_RC_FLAGS= -I binary -O elf64-littleaarch64 -B 
aarch64 --rename-section .data=.hii
 
-DEFINE GCC44_ALL_CC_FLAGS= -g -fshort-wchar -fno-strict-aliasing 
-Wall -Werror -Wno-array-bounds -ffunction-sections -fdata-sections -c -include 
AutoGen.h -DSTRING_ARRAY_NAME=$(BASE_NAME)Strings
+DEFINE GCC44_ALL_CC_FLAGS= -g -fshort-wchar -fno-strict-aliasing 
-Wall -Werror -Wno-array-bounds -Wno-address -Wno-unused-but-set-variable 
-ffunction-sections -fdata-sections -c -include AutoGen.h 
-DSTRING_ARRAY_NAME=$(BASE_NAME)Strings
 DEFINE GCC44_IA32_CC_FLAGS   = DEF(GCC44_ALL_CC_FLAGS) -m32 
-malign-double -fno-stack-protector -D EFI32
 DEFINE GCC44_X64_CC_FLAGS= DEF(GCC44_ALL_CC_FLAGS) -m64 
-fno-stack-protector "-DEFIAPI=__attribute__((ms_abi))" -DNO_BUILTIN_VA_FUNCS 
-mno-red-zone -Wno-address -mcmodel=large
 DEFINE GCC44_IA32_X64_DLINK_COMMON   = -nostdlib -n -q --gc-sections -z 
common-page-size=0x20
@@ -3865,8 +3865,8 @@ DEFINE GCC45_X64_DLINK_FLAGS = 
DEF(GCC44_X64_DLINK_FLAGS)
 DEFINE GCC45_X64_DLINK2_FLAGS= DEF(GCC44_X64_DLINK2_FLAGS)
 DEFINE GCC45_ASM_FLAGS   = DEF(GCC44_ASM_FLAGS)
 
-DEFINE GCC46_IA32_CC_FLAGS   = DEF(GCC45_IA32_CC_FLAGS) -Wno-address 
-Wno-unused-but-set-variable
-DEFINE GCC46_X64_CC_FLAGS= DEF(GCC45_X64_CC_FLAGS) -Wno-address 
-Wno-unused-but-set-variable
+DEFINE GCC46_IA32_CC_FLAGS   = DEF(GCC45_IA32_CC_FLAGS)
+DEFINE GCC46_X64_CC_FLAGS= DEF(GCC45_X64_CC_FLAGS)
 DEFINE GCC46_IA32_X64_DLINK_COMMON   = DEF(GCC45_IA32_X64_DLINK_COMMON)
 DEFINE GCC46_IA32_X64_ASLDLINK_FLAGS = DEF(GCC45_IA32_X64_ASLDLINK_FLAGS)
 DEFINE GCC46_IA32_X64_DLINK_FLAGS= DEF(GCC45_IA32_X64_DLINK_FLAGS)
--
1.9.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 v3 6/7] BaseTools/GenFw: allow AArch64 tiny and small code model relocations

2015-08-09 Thread Gao, Liming
Ard:
  I am not familiar with ARM arch. Those changes are only for AArch64. I have 
no comments for them. You can add review-by me. 

Thanks
Liming
-Original Message-
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] 
Sent: Friday, August 7, 2015 10:23 PM
To: edk2-devel@lists.01.org; Liu, Yingke D; leif.lindh...@linaro.org
Cc: Gao, Liming; Ard Biesheuvel
Subject: [PATCH v3 6/7] BaseTools/GenFw: allow AArch64 tiny and small code 
model relocations

The AArch64 small C model makes extensive use of ADRP/ADD and ADRP/{LDR,STR} 
pairs to emit PC-relative symbol references with a +/- 4 GB range. Since the 
relocation pair splits the relative offset into a relative page offset and an 
absolute offset into a 4 KB page, we need to take extra care to ensure that the 
target of the relocation preserves its alignment relative to a 4 KB alignment 
boundary.

Also, due to a problem with the --emit-relocs GNU ld option, where it does not 
recalculate the addends for section relative relocations, the only way to 
guarantee correct code is by requiring the relative section offset to be equal 
in the ELF and PE/COFF versions of the binary. This affects both the 'tiny' and 
'small' GCC code models.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Leif Lindholm 
Tested-by: Leif Lindholm 
---
 BaseTools/Source/C/GenFw/Elf64Convert.c | 92 +++-
 1 file changed, 49 insertions(+), 43 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c 
b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 1c0f4a4dc87c..c758ed9d64a6 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -740,53 +740,60 @@ WriteSections64 (
   }
 } else if (mEhdr->e_machine == EM_AARCH64) {
 
-  // AARCH64 GCC uses RELA relocation, so all relocations have to be 
fixed up.
-  // As opposed to ARM32 using REL.
-
   switch (ELF_R_TYPE(Rel->r_info)) {
 
-  case R_AARCH64_ADR_PREL_LO21:
-if  (Rel->r_addend != 0 ) { /* TODO */
-  Error (NULL, 0, 3000, "Invalid", "AArch64: 
R_AARCH64_ADR_PREL_LO21 Need to fixup with addend!.");
+  case R_AARCH64_ADR_PREL_PG_HI21:
+  case R_AARCH64_ADD_ABS_LO12_NC:
+  case R_AARCH64_LDST8_ABS_LO12_NC:
+  case R_AARCH64_LDST16_ABS_LO12_NC:
+  case R_AARCH64_LDST32_ABS_LO12_NC:
+  case R_AARCH64_LDST64_ABS_LO12_NC:
+  case R_AARCH64_LDST128_ABS_LO12_NC:
+//
+// AArch64 PG_H21 relocations are typically paired with ABS_LO12
+// relocations, where a PC-relative reference with +/- 4 GB range 
is
+// split into a relative high part and an absolute low part. Since
+// the absolute low part represents the offset into a 4 KB page, we
+// have to make sure that the 4 KB relative offsets of both the
+// section containing the reference as well as the section to which
+// it refers have not been changed during PE/COFF conversion (i.e.,
+// in ScanSections64() above).
+//
+if (((SecShdr->sh_addr ^ SecOffset) & 0xfff) != 0 ||
+((SymShdr->sh_addr ^ mCoffSectionsOffset[Sym->st_shndx]) & 
0xfff) != 0 ||
+mCoffAlignment < 0x1000) {
+  Error (NULL, 0, 3000, "Invalid", "WriteSections64(): %s AARCH64 
small code model requires 4 KB section alignment.",
+mInImageName);
+  break;
 }
-break;
+/* fall through */
 
+  case R_AARCH64_ADR_PREL_LO21:
   case R_AARCH64_CONDBR19:
-if  (Rel->r_addend != 0 ) { /* TODO */
-  Error (NULL, 0, 3000, "Invalid", "AArch64: R_AARCH64_CONDBR19 
Need to fixup with addend!.");
-}
-break;
-
   case R_AARCH64_LD_PREL_LO19:
-if  (Rel->r_addend != 0 ) { /* TODO */
-  Error (NULL, 0, 3000, "Invalid", "AArch64: 
R_AARCH64_LD_PREL_LO19 Need to fixup with addend!.");
-}
-break;
-
   case R_AARCH64_CALL26:
   case R_AARCH64_JUMP26:
-if  (Rel->r_addend != 0 ) {
-  // Some references to static functions sometime start at the 
base of .text + addend.
-  // It is safe to ignore these relocations because they patch a 
`BL` instructions that
-  // contains an offset from the instruction itself and there is 
only a single .text section.
-  // So we check if the symbol is a "section symbol"
-  if (ELF64_ST_TYPE (Sym->st_info) == STT_SECTION) {
-break;
-  }
-  Error (NULL, 0, 3000, "Invalid", "AArch64: R_AARCH64_JUMP26 Need 
to fixup with addend!.");
+//
+// The GCC toolchains (i.e., binutils) may corrupt section relative
+// relocations when emitting relocation sections into f

Re: [edk2] [PATCH] BaseTools/GenFds: Fix 'NoneType' object is not iterable error.

2015-08-09 Thread Gao, Liming
Shumin:
  Please highlight what problem resolved by this fix. 

Thanks
Liming
-Original Message-
From: Qiu, Shumin 
Sent: Monday, August 10, 2015 1:43 PM
To: edk2-devel@lists.01.org
Cc: Qiu, Shumin; Gao, Liming; winggundu...@163.com
Subject: [PATCH] BaseTools/GenFds: Fix 'NoneType' object is not iterable error.

In GenFdsGlobalVariable.py line 401 'list' requires a iteralbe object as 
parameter while the 'Input' is None.

Cc: Liming Gao 
Cc: winggundu...@163.com
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin 
Reviewed-by: Yingke Liu 
---
 BaseTools/Source/Python/GenFds/VerSection.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/VerSection.py 
b/BaseTools/Source/Python/GenFds/VerSection.py
index 657ebd9..7399e7a 100644
--- a/BaseTools/Source/Python/GenFds/VerSection.py
+++ b/BaseTools/Source/Python/GenFds/VerSection.py
@@ -1,7 +1,7 @@
 ## @file
 # process Version section generation
 #
-#  Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.
+#  Copyright (c) 2007 - 2015, Intel Corporation. All rights 
+reserved.
 #
 #  This program and the accompanying materials  #  are licensed and made 
available under the terms and conditions of the BSD License @@ -76,7 +76,7 @@ 
class VerSection (VerSectionClassObject):
 else:
 StringData = ''
 
-GenFdsGlobalVariable.GenerateSection(OutputFile, None, 
'EFI_SECTION_VERSION',
+GenFdsGlobalVariable.GenerateSection(OutputFile, [], 
+ 'EFI_SECTION_VERSION',
  Ver=StringData, 
BuildNumber=self.BuildNum)
 OutputFileList = []
 OutputFileList.append(OutputFile)
--
1.9.5.msysgit.1

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


[edk2] [PATCH] BaseTools/GenFds: Fix 'NoneType' object is not iterable error.

2015-08-09 Thread Qiu Shumin
In GenFdsGlobalVariable.py line 401 'list' requires a iteralbe object as 
parameter while the 'Input' is None.

Cc: Liming Gao 
Cc: winggundu...@163.com
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin 
Reviewed-by: Yingke Liu 
---
 BaseTools/Source/Python/GenFds/VerSection.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/VerSection.py 
b/BaseTools/Source/Python/GenFds/VerSection.py
index 657ebd9..7399e7a 100644
--- a/BaseTools/Source/Python/GenFds/VerSection.py
+++ b/BaseTools/Source/Python/GenFds/VerSection.py
@@ -1,7 +1,7 @@
 ## @file
 # process Version section generation
 #
-#  Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.
+#  Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
@@ -76,7 +76,7 @@ class VerSection (VerSectionClassObject):
 else:
 StringData = ''
 
-GenFdsGlobalVariable.GenerateSection(OutputFile, None, 
'EFI_SECTION_VERSION',
+GenFdsGlobalVariable.GenerateSection(OutputFile, [], 
'EFI_SECTION_VERSION',
  Ver=StringData, 
BuildNumber=self.BuildNum)
 OutputFileList = []
 OutputFileList.append(OutputFile)
-- 
1.9.5.msysgit.1

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


[edk2] Library refinement: OptionRomPkg/BltLib

2015-08-09 Thread Ni, Ruiyu
Jordan and Laszlo,
I reviewed the OptionRomPkg/BltLib again and would like to discuss with you 
about the potential API refinement.
I attached two versions of the refined BltLib.h.

The common part of the two versions is:

1.  BltLibGetSizes() is removed.

Because the size information is actually passed from the library consumer to 
the library. Caller do not need to get the information from the library 
interface.

2.  BltLibBufferToVideoEx and BltLibVideoToBltBufferEx are removed and the 
accordingly non-Ex version of APIs will be used for *Ex operation.
Just for reducing the library APIs and avoid the different APIs have overlapped 
functionality.

3.  BltLibGopBlt() is removed

Same reason as the above #2.

BltLib2.h contains another change.

4.  BltLibConfigure() is removed and the frame buffer information will be 
passed through the four BltLib* APIs.

Because it can remove the restriction to allow the library can be re-entry.

What's your thoughts about this?

Regards,
Ray

/** @file
  Library for performing video blt operations

  Copyright (c) 2009 - 2011, 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 __BLT_LIB__
#define __BLT_LIB__

#include 


/**
  Configure the BltLib for a frame-buffer

  @param[in] FrameBuffer  Pointer to the start of the frame buffer
  @param[in] FrameBufferInfo  Describes the frame buffer characteristics

  @retval  EFI_INVALID_PARAMETER - Invalid parameter passed in
  @retval  EFI_SUCCESS - Blt operation success

**/
EFI_STATUS
EFIAPI
BltLibConfigure (
  IN  VOID *FrameBuffer,
  IN  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *FrameBufferInfo
  );

/**
  Performs a UEFI Graphics Output Protocol Blt Video Fill.

  @param[in]  Color Color to fill the region with
  @param[in]  DestinationX  X location to start fill operation
  @param[in]  DestinationY  Y location to start fill operation
  @param[in]  Width Width (in pixels) to fill
  @param[in]  HeightHeight to fill

  @retval  EFI_DEVICE_ERROR - A hardware error occured
  @retval  EFI_INVALID_PARAMETER - Invalid parameter passed in
  @retval  EFI_SUCCESS - Blt operation success

**/
EFI_STATUS
EFIAPI
BltLibVideoFill (
  IN  EFI_GRAPHICS_OUTPUT_BLT_PIXEL *Color,
  IN  UINTN DestinationX,
  IN  UINTN DestinationY,
  IN  UINTN Width,
  IN  UINTN Height
  );

/**
  Performs a UEFI Graphics Output Protocol Blt Video to Buffer operation
  with extended parameters.

  @param[out] BltBuffer Output buffer for pixel color data
  @param[in]  SourceX   X location within video
  @param[in]  SourceY   Y location within video
  @param[in]  DestinationX  X location within BltBuffer
  @param[in]  DestinationY  Y location within BltBuffer
  @param[in]  Width Width (in pixels)
  @param[in]  HeightHeight
  @param[in]  Delta Number of bytes in a row of BltBuffer

  @retval  EFI_DEVICE_ERROR - A hardware error occured
  @retval  EFI_INVALID_PARAMETER - Invalid parameter passed in
  @retval  EFI_SUCCESS - Blt operation success

**/
EFI_STATUS
EFIAPI
BltLibVideoToBltBuffer (
  OUT EFI_GRAPHICS_OUTPUT_BLT_PIXEL *BltBuffer,
  IN  UINTN SourceX,
  IN  UINTN SourceY,
  IN  UINTN DestinationX,
  IN  UINTN DestinationY,
  IN  UINTN Width,
  IN  UINTN Height,
  IN  UINTN Delta
  );

/**
  Performs a UEFI Graphics Output Protocol Blt Buffer to Video operation
  with extended parameters.

  @param[in]  BltBuffer Output buffer for pixel color data
  @param[in]  SourceX   X location within BltBuffer
  @param[in]  SourceY   Y location within BltBuffer
  @param[in]  DestinationX  X location within video
  @param[in]  DestinationY  Y location within video
  @param[in]  Width Width (in pixels)
  @param[in]  HeightHeight
  @param[in]  Delta Number of bytes in a row of BltBuffer

  @retval  EFI_DEVICE_ERROR - A hardware error occured
  @retval  EFI_INVALID_PARAMETER - Invalid parameter passed in
  @retval  EFI_SUCCESS - Blt operation success

**/
EFI_STATUS
EFIAPI
BltLibBufferToVideo (
  IN  EFI_GRAPHICS_OUTPUT_BLT_PIXEL *BltBuffer,
  IN  UINTN SourceX,
  IN  UINTN SourceY,
 

Re: [edk2] [patch] MdeModulePkg: HttpLib BodyParseComplete Event should indicate the end of message body

2015-08-09 Thread Wu, Jiaxin
The patch looks good.

Reviewed-by: Jiaxin Wu 

-Original Message-
From: Zhang, Lubo 
Sent: Monday, August 10, 2015 9:20 AM
To: edk2-devel@lists.01.org
Cc: Ye, Ting; Wu, Jiaxin
Subject: [patch] MdeModulePkg: HttpLib BodyParseComplete Event should indicate 
the end of message body

In HttpLib, the Event BodyParseComplete should return to the callback function 
when the whole message body has been parsed

Cc: Ye Ting
CC: Wu Jiaxin 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
---
 MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 44 ++--
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c 
b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
index 3b0d9c9..3b9048e 100644
--- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
+++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
@@ -816,10 +816,13 @@ typedef enum {
   BodyParserChunkSizeEndCR,
   BodyParserChunkExtStart,
   BodyParserChunkDataStart,
   BodyParserChunkDataEnd,
   BodyParserChunkDataEndCR,
+  BodyParserTrailer,
+  BodyParserLastCRLF,
+  BodyParserLastCRLFEnd,
   BodyParserComplete,
   BodyParserStateMax
 } HTTP_BODY_PARSE_STATE;
 
 typedef struct {
@@ -1210,25 +1213,48 @@ HttpParseMessageBody (
 case BodyParserChunkSizeEndCR:
   if (*Char != '\n') {
 Parser->State = BodyParserStateMax;
 break;
   }
+  Char++;
+  if (Parser->CurrentChunkSize == 0) {
+//
+   // The last chunk has been parsed, then need to decide last message is 
Trailer or CRLF.
+   //  
+ Parser->ContentLengthIsValid = TRUE;
+ Parser->State = BodyParserLastCRLF;
+ break;
+  }
   Parser->State = BodyParserChunkDataStart;
   Parser->CurrentChunkParsedSize = 0;
+  break;
+case BodyParserLastCRLF:
+   if (*Char == '\r'){
+  Char++;
+  Parser->State = BodyParserLastCRLFEnd;
+  break;
+ } else {
+Parser->State = BodyParserTrailer;
+break;
+ }
+case BodyParserLastCRLFEnd:
+  if (*Char == '\n'){
+  Parser->State = BodyParserComplete;
+  break;
+   } else {
+  Parser->State = BodyParserStateMax;
+  break;
+   }
+case BodyParserTrailer:
+  if (*Char == '\r') {
+Parser->State = BodyParserChunkSizeEndCR;
+  }
   Char++;
   break;
+  
 
 case BodyParserChunkDataStart:
-  if (Parser->CurrentChunkSize == 0) {
-//
-// This is the last chunk, the trailer header is unsupported.
-//
-Parser->ContentLengthIsValid = TRUE;
-Parser->State = BodyParserComplete;
-break;
-  }
-  
   //
   // First byte of chunk-data, the chunk data also might be truncated.
   //
   RemainderLengthInThis = BodyLength - (Char - Body);
   LengthForCallback = MIN (Parser->CurrentChunkSize - 
Parser->CurrentChunkParsedSize, RemainderLengthInThis);
--
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: HttpLib BodyParseComplete Event should indicate the end of message body

2015-08-09 Thread Zhang Lubo
In HttpLib, the Event BodyParseComplete should return to the
callback function when the whole message body has been parsed

Cc: Ye Ting
CC: Wu Jiaxin 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
---
 MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 44 ++--
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c 
b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
index 3b0d9c9..3b9048e 100644
--- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
+++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
@@ -816,10 +816,13 @@ typedef enum {
   BodyParserChunkSizeEndCR,
   BodyParserChunkExtStart,
   BodyParserChunkDataStart,
   BodyParserChunkDataEnd,
   BodyParserChunkDataEndCR,
+  BodyParserTrailer,
+  BodyParserLastCRLF,
+  BodyParserLastCRLFEnd,
   BodyParserComplete,
   BodyParserStateMax
 } HTTP_BODY_PARSE_STATE;
 
 typedef struct {
@@ -1210,25 +1213,48 @@ HttpParseMessageBody (
 case BodyParserChunkSizeEndCR:
   if (*Char != '\n') {
 Parser->State = BodyParserStateMax;
 break;
   }
+  Char++;
+  if (Parser->CurrentChunkSize == 0) {
+//
+   // The last chunk has been parsed, then need to decide last message is 
Trailer or CRLF.
+   //  
+ Parser->ContentLengthIsValid = TRUE;
+ Parser->State = BodyParserLastCRLF;
+ break;
+  }
   Parser->State = BodyParserChunkDataStart;
   Parser->CurrentChunkParsedSize = 0;
+  break;
+case BodyParserLastCRLF:
+   if (*Char == '\r'){
+  Char++;
+  Parser->State = BodyParserLastCRLFEnd;
+  break;
+ } else {
+Parser->State = BodyParserTrailer;
+break;
+ }
+case BodyParserLastCRLFEnd:
+  if (*Char == '\n'){
+  Parser->State = BodyParserComplete;
+  break;
+   } else {
+  Parser->State = BodyParserStateMax;
+  break;
+   }
+case BodyParserTrailer:
+  if (*Char == '\r') {
+Parser->State = BodyParserChunkSizeEndCR;
+  }
   Char++;
   break;
+  
 
 case BodyParserChunkDataStart:
-  if (Parser->CurrentChunkSize == 0) {
-//
-// This is the last chunk, the trailer header is unsupported.
-//
-Parser->ContentLengthIsValid = TRUE;
-Parser->State = BodyParserComplete;
-break;
-  }
-  
   //
   // First byte of chunk-data, the chunk data also might be truncated.
   //
   RemainderLengthInThis = BodyLength - (Char - Body);
   LengthForCallback = MIN (Parser->CurrentChunkSize - 
Parser->CurrentChunkParsedSize, RemainderLengthInThis);
-- 
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: Stop and release DHCP4 child after boot info is ready

2015-08-09 Thread Jiaxin Wu
HttpBootDxe need to stop and release the DHCP4 child when it's
not used so the NBP could create new DHCP4 child and use it.

Cc: Ye Ting 
Cc: Zhang Lubo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/HttpBootDxe/HttpBootImpl.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootImpl.c 
b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
index 711cc3c..920761e 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootImpl.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
@@ -242,10 +242,16 @@ HttpBootStop (
   Private->BootFileSize = 0;
   Private->SelectIndex = 0;
   Private->SelectProxyType = HttpOfferTypeMax;
 
   if (!Private->UsingIpv6) {
+//
+// Stop and release the DHCP4 child.
+//
+Private->Dhcp4->Stop (Private->Dhcp4);
+Private->Dhcp4->Configure (Private->Dhcp4, NULL);
+
 for (Index = 0; Index < HTTP_BOOT_OFFER_MAX_NUM; Index++) {
   if (Private->OfferBuffer[Index].Dhcp4.UriParser) {
 HttpUrlFreeParser (Private->OfferBuffer[Index].Dhcp4.UriParser);
   }
 }
@@ -336,10 +342,16 @@ HttpBootDxeLoadFile (
 Status = HttpBootLoadFile (Private, BufferSize, Buffer);
   }
 
   if (Status != EFI_SUCCESS && Status != EFI_BUFFER_TOO_SMALL) {
 HttpBootStop (Private);
+  } else {
+//
+// Stop and release the DHCP4 child.
+//
+Private->Dhcp4->Stop (Private->Dhcp4);
+Private->Dhcp4->Configure (Private->Dhcp4, NULL);
   }
 
   return Status;
 }
 
-- 
1.9.5.msysgit.1

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


Re: [edk2] Core TPL and interrupt state

2015-08-09 Thread Benjamin Herrenschmidt
On Sun, 2015-08-09 at 15:54 -0500, Scott Duplichan wrote:
> 6) TimerInterruptHandler calls RestoreTPL. RestoreTPL executes a loop for
>"Dispatch any pending events". Interrupts are re-enabled if all pending
>event have TPL < HIGH. CoreDispatchEventNotifies calls CoreAcquireEventLock
>which masks interrupts. Before returning CoreDispatchEventNotifies calls
>CoreReleaseEventLock and the interrupt mask is restored. 

Right, this is the point where I am not too happy. Other interrupts
could come in as well leading to quite a bit of stacking.

> 7) Eventually execution returns to the point of interruption when the iret
>instruction executes.
> 
> I am not sure I understand all the EDK2 interrupt logic. But it does seem
> there is potential to re-enter the timer ISR. This is because the 8259 end
> of interrupt command is sent early, and followed by some execution with 
> processor interrupts enabled. I would guess the 150 or so Hz interrupt rate
> of EDK2 is so low that there is little chance of a timer interrupt firing
> before the previous one completes. It might be interesting to experiment
> with higher interrupt rates. 

Ok, so the situation is similar to mine. I can stack additionally up to one
external interrupt, as we can't do early EOI for these and I have them all
set to the same priority.

For now I've hacked my CpuDxe to maintain an interrupt depth and "neutralize"
my CpuEnableInterrupt() callback while it's non-0, this gets me going but I
need to spend a bit more time understanding more precisely what is re-entering
and when. I was essentially hitting an ASSERT somewhere in the lock release
code claiming the lock had already been released, so something went badly wrong.

Ben.


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


Re: [edk2] Core TPL and interrupt state

2015-08-09 Thread Scott Duplichan
Benjamin Herrenschmidt [mailto:b...@kernel.crashing.org] wrote:

]Sent: Sunday, August 09, 2015 06:28 AM
]To: edk2-de...@ml01.01.org
]Subject: [edk2] Core TPL and interrupt state
]
]Hi !
]
]Yet another somewhat newbie question, I apologize if I missed the right
]piece of doc...
]
]I found some kind of disconnect between the core idea of TPL and the
]interrupt enable state of the processor.
]
]The core will enable or disable processor interrupts based on TPL state
]changes.
]
]However the core TPL state will not be affected by *taking* an
]interrupt, at least as far as I can tell from my looking at ARM and x86
]exception handlers.
]
]Now I'm not that fluent in ARM or x86 asm, so I couldn't quite figure
]out easily whether they call the exception/interrupt handlers with core
]interrupts enabled or not or rely on masking by an external controller
]such as the APIC to avoid stacking of interrupts.
]
]In my case, the timer interrupt isn't maskable individually (other than
]by delaying the timer target or masking all core interrupts), and in
]sim, where the timer tend to tick faster than normal, I've observed
]cases where:
]
]  - We enter the interrupt handler, at this point, my code keeps core
]interrupts *disabled*
]
]  - We call the core timer tick
]
]  - The core timer tick code takes and then releases a spinlock, which
]has the side effect of lowering the TPL, and causes the core to
]re-enable interrupts.
]
]  - The core interrupts get re-enable before we return from the actual
]exception
]
]  - Another timer interrupt kicks in before we unwind the stack
]
]This seems quite fragile and prone to exception overstacking, but is
]that on purpose ? Would it make sense to have the interrupt entry keep
]interrupts disabled and inform the core somewhat by raising the TPL,
]so that a spin lock/unlock sequence doesn't re-enable them ?
]
]Or is UEFI purposefully using this as a way to nest interrupts ?
]
]Cheers,
]Ben.

Hello Ben,

I can't answer all your questions, but I know something about the x86
legacy 8259 interrupt system used by x86 EDK2. The nesting and wind-up
avoidance is fairly simple in this case because only a single interrupt
is used (8254 PIT channel 0 for the open source EDK2 code).

When an x86 hardware interrupt is handled by an interrupt gate as is the
case for EDK2, the processor clears its own interrupt enable flag after
pushing the eflags register onto the stack. The interrupt flag in the
pushed eflags is set; otherwise the interrupt wouldn't have been taken.
That is the first wind-up prevention mechanism. But what if interrupts
need to be re-enabled before the handler exits. Another mechanism prevents
interrupt wind-up in this case. The 8259 interrupt controller, at least
with its usual programming, will not allow an interrupt of the same or
lower privilege to execute until the software sends an end-of-interrupt
command. For EDK2 and most other uses, a non-specific end of interrupt
command (0x20) is sent. If processor interrupts are not enabled in the 
eflags register, no new interrupt is possible until interrupts are
re-enabled by the iret pop of the eflags register.

Here is what I find tracing the EDK2 x86 timer ISR:

1) Interrupts start off masked in the eflags register because an
   interrupt gate is used.
2) CommonInterruptEntry executes a cli (clear eflags interrupt enable)
   instruction. This is redundant. The processor has already cleared
   the eflags interrupt enable bit.
3) TimerInterruptHandler calls RaiseTPL (TPL_HIGH_LEVEL). Because the new
   level is higher than old level, a cli instruction is executed to mask
   interrupts.
4) TimerInterruptHandler sends the end of interrupt command. It does this
   early, before doing the bulk of its work. This means that EDK2 is not
   using the end of interrupt method of preventing interrupt handler re-
   enter. It is relying on the interrupt mask only.
5) TimerInterruptHandler calls mTimerNotifyFunction. This is where all the
   work is done, and it is done with processor interrupts masked. If the
   amount of work to be done is lengthy, some 8254 timer interrupts could
   be lost.
6) TimerInterruptHandler calls RestoreTPL. RestoreTPL executes a loop for
   "Dispatch any pending events". Interrupts are re-enabled if all pending
   event have TPL < HIGH. CoreDispatchEventNotifies calls CoreAcquireEventLock
   which masks interrupts. Before returning CoreDispatchEventNotifies calls
   CoreReleaseEventLock and the interrupt mask is restored. 

7) Eventually execution returns to the point of interruption when the iret
   instruction executes.

I am not sure I understand all the EDK2 interrupt logic. But it does seem
there is potential to re-enter the timer ISR. This is because the 8259 end
of interrupt command is sent early, and followed by some execution with 
processor interrupts enabled. I would guess the 150 or so Hz interrupt rate
of EDK2 is so low that there is little chance of a timer interrupt firing
before the previous one completes

[edk2] Problem with CorebootPayloadPkg startup

2015-08-09 Thread Scott Duplichan
Hello,

Starting with SVN revision 18146, CorebootPayload will no longer boot
on my board. The reason is that the code rearrangement of rev 18146
(MdeModulePkg DxeCore: Move ProcessLibraryConstructorList) causes low
memory to be cleared before a call to FindCbTag scans low memory.
Prior to 18146, low memory was cleared after the last use of FindCbTag.
As a result, the low memory scan of FindCbTag no longer finds the
coreboot tag.

This shouldn't be a problem because there is a backup mechanism where
the needed address stored in low memory is saved to PcdCbHeaderPointer:

  // Set pcd to save the upper coreboot header in case the dxecore will
  // erase 0~4k memory

This mechanism isn't working, for a couple of reasons. First, the DXE
core code that reads the PCD is using a NULL version of the PCD lib.
If this is fixed, it still doesn't work because the PCD lib isn't
loading soon enough. APRIORI doesn't solve the problem:

APRIORI DXE {
  INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
}

The DXE core code that tries to read PcdCbHeaderPointer PCD is:

PcdGet32 called by:
CbParseSerialInfo called by:
SerialPortInitialize called by:
ProcessLibraryConstructorList called by:
DxeMain.

Presumably Dynamic PCD access from ProcessLibraryConstructorList is
just not possible.

A temporary solution is to not give the lower 4KB to UEFI:

Index: CbSupportPei.c
===
--- CbSupportPei.c  (revision 18191)
+++ CbSupportPei.c  (working copy)
@@ -197,8 +197,9 @@
 EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
 EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE
 ),
-(EFI_PHYSICAL_ADDRESS)(0),
-(UINT64)(0xA)
+// lower 640KB, except for first 4KB where the coreboot tag resides
+(EFI_PHYSICAL_ADDRESS)(0 + 0x1000),
+(UINT64)(0xA - 0x1000)
 );

This would probably break things if anyone tried to use legacy code
such as CSM or Duet/BiosVideoThunkDxe. Can someone come up with a
better solution?

Thanks,
Scott



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


[edk2] Core TPL and interrupt state

2015-08-09 Thread Benjamin Herrenschmidt
Hi !

Yet another somewhat newbie question, I apologize if I missed the right
piece of doc...

I found some kind of disconnect between the core idea of TPL and the
interrupt enable state of the processor.

The core will enable or disable processor interrupts based on TPL state
changes.

However the core TPL state will not be affected by *taking* an
interrupt, at least as far as I can tell from my looking at ARM and x86
exception handlers.

Now I'm not that fluent in ARM or x86 asm, so I couldn't quite figure
out easily whether they call the exception/interrupt handlers with core
interrupts enabled or not or rely on masking by an external controller
such as the APIC to avoid stacking of interrupts.

In my case, the timer interrupt isn't maskable individually (other than
by delaying the timer target or masking all core interrupts), and in
sim, where the timer tend to tick faster than normal, I've observed
cases where:

  - We enter the interrupt handler, at this point, my code keeps core
interrupts *disabled*

  - We call the core timer tick

  - The core timer tick code takes and then releases a spinlock, which
has the side effect of lowering the TPL, and causes the core to
re-enable interrupts.

  - The core interrupts get re-enable before we return from the actual
exception

  - Another timer interrupt kicks in before we unwind the stack

This seems quite fragile and prone to exception overstacking, but is
that on purpose ? Would it make sense to have the interrupt entry keep
interrupts disabled and inform the core somewhat by raising the TPL,
so that a spin lock/unlock sequence doesn't re-enable them ?

Or is UEFI purposefully using this as a way to nest interrupts ?

Cheers,
Ben.


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