Re: [edk2] Library refinement: OptionRomPkg/BltLib
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
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.
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
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
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.
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.
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
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
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
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
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
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
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
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
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