Re: [edk2] [PATCH v2 00/12] Add WinHost support in EmulatorPkg
Hi, It's a good idea,besides the code,I also think it is bettter to put out some design note to help newbie/user/developer,it may be useful to many people,such as: Emulator_user_guide.pdf, Emulator_developer_guide.pdf, the implementation_of_Emulator.pdf the implementation_of_Emulator's xxx .pdf How does Emulator's something worked.pdf thank you, by krishna. At 2018-09-04 10:32:09, "Ni, Ruiyu" wrote: >Shia, >This is my personal plan. But I need to: >1. make EmulatorPkg/Win be functionality equivalent to Nt32Pkg >2. All existing Nt32Pkg customers are happy to use EmulatorPkg/Win > >Until then, I may remove Nt32Pkg. >Again, this is my personal plan, not an official decision. > >Any comments? > >> -Original Message- >> From: Shia, Cinnamon [mailto:cinnamon.s...@hpe.com] >> Sent: Thursday, August 30, 2018 9:58 AM >> To: Ni, Ruiyu ; edk2-devel@lists.01.org >> Subject: RE: [edk2] [PATCH v2 00/12] Add WinHost support in EmulatorPkg >> >> Hi Ray, >> >> Does this change mean that Nt32Pkg is going to be retired? >> >> Thanks >> Cinnamon Shia >> >> -Original Message- >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu >> Ni >> Sent: Thursday, August 23, 2018 5:56 PM >> To: edk2-devel@lists.01.org >> Subject: [edk2] [PATCH v2 00/12] Add WinHost support in EmulatorPkg >> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1112 >> >> The patch sets add WinHost support (Nt32) in EmulatorPkg. >> 3 EmulatorPkg common issues were found and fixed. >> Other 9 patches are to step-by-step enable the WinHost. >> >> v2 sends to correct mail address. >> >> Ruiyu Ni (12): >> EmulatorPkg/ThunkProtocolList: Fix VS build failure >> EmulatorPkg/Win: Add Windows host support >> EmulatorPkg/Win: Enable source level debugging >> EmulatorPkg/Win: Enable native OS console as firmware console >> EmulatorPkg/Win: Add input/output support >> EmulatorPkg/Win: Add timer and interrupt support >> EmulatorPkg/Win: Add RTC support >> EmulatorPkg/Win: Add SimpleFileSystem support >> EmulatorPkg/Win: Add BlockIo support >> EmulatorPkg/PlatformBds: Signal EndOfDxe in platform BDS >> EmulatorPkg/EmuFileSystem: Fix a bug that causes Close() assertion >> EmulatorPkg/DSC: Remove FS mapping to EDK Shell bin directory >> >> .../EmuSimpleFileSystemDxe/EmuSimpleFileSystem.c | 33 +- >> EmulatorPkg/EmulatorPkg.dsc| 17 +- >> EmulatorPkg/Library/EmuBdsLib/BdsPlatform.c|4 +- >> EmulatorPkg/Library/EmuBdsLib/BdsPlatform.h|4 +- >> EmulatorPkg/Library/EmuBdsLib/EmuBdsLib.inf|5 +- >> .../Library/ThunkProtocolList/ThunkProtocolList.c |4 +- >> EmulatorPkg/Win/Host/WinBlockIo.c | 563 + >> EmulatorPkg/Win/Host/WinFileSystem.c | 2409 >> >> EmulatorPkg/Win/Host/WinGop.h | 204 ++ >> EmulatorPkg/Win/Host/WinGopInput.c | 417 >> EmulatorPkg/Win/Host/WinGopScreen.c| 872 +++ >> EmulatorPkg/Win/Host/WinHost.c | 947 >> EmulatorPkg/Win/Host/WinHost.h | 209 ++ >> EmulatorPkg/Win/Host/WinHost.inf | 107 + >> EmulatorPkg/Win/Host/WinInclude.h | 75 + >> EmulatorPkg/Win/Host/WinMemoryAllocationLib.c | 178 ++ >> EmulatorPkg/Win/Host/WinThunk.c| 577 + >> 17 files changed, 6614 insertions(+), 11 deletions(-) create mode 100644 >> EmulatorPkg/Win/Host/WinBlockIo.c create mode 100644 >> EmulatorPkg/Win/Host/WinFileSystem.c >> create mode 100644 EmulatorPkg/Win/Host/WinGop.h create mode 100644 >> EmulatorPkg/Win/Host/WinGopInput.c >> create mode 100644 EmulatorPkg/Win/Host/WinGopScreen.c >> create mode 100644 EmulatorPkg/Win/Host/WinHost.c create mode 100644 >> EmulatorPkg/Win/Host/WinHost.h create mode 100644 >> EmulatorPkg/Win/Host/WinHost.inf create mode 100644 >> EmulatorPkg/Win/Host/WinInclude.h create mode 100644 >> EmulatorPkg/Win/Host/WinMemoryAllocationLib.c >> create mode 100644 EmulatorPkg/Win/Host/WinThunk.c >> >> -- >> 2.16.1.windows.1 >> >> ___ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel > >___ >edk2-devel mailing list >edk2-devel@lists.01.org >https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch] BaseTools: Extend the keyword "!include"/"!if" to case-insensitive
From: zhijufan Extend the keyword "!include", "!if", etc to case-insensitive. Current DSC parser already support it, while FDF parser only support the lower case, so this patch add the support for FDF parser. Cc: Liming Gao Cc: Yonghong Zhu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Zhiju.Fan --- BaseTools/Source/Python/GenFds/FdfParser.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/BaseTools/Source/Python/GenFds/FdfParser.py b/BaseTools/Source/Python/GenFds/FdfParser.py index 8de0b48..7e1be65 100644 --- a/BaseTools/Source/Python/GenFds/FdfParser.py +++ b/BaseTools/Source/Python/GenFds/FdfParser.py @@ -,10 +,12 @@ class FdfParser: EndPos = self.CurrentOffsetWithinLine if self.CurrentLineNumber != StartLine: EndPos = len(self.Profile.FileLinesList[StartLine-1]) self.__Token = self.Profile.FileLinesList[StartLine-1][StartPos : EndPos] +if self.__Token.lower() in [TAB_IF, TAB_END_IF, TAB_ELSE_IF, TAB_ELSE, TAB_IF_DEF, TAB_IF_N_DEF, TAB_ERROR, TAB_INCLUDE]: +self.__Token = self.__Token.lower() if StartPos != self.CurrentOffsetWithinLine: return True else: return False -- 2.6.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 00/12] Add WinHost support in EmulatorPkg
Shia, This is my personal plan. But I need to: 1. make EmulatorPkg/Win be functionality equivalent to Nt32Pkg 2. All existing Nt32Pkg customers are happy to use EmulatorPkg/Win Until then, I may remove Nt32Pkg. Again, this is my personal plan, not an official decision. Any comments? > -Original Message- > From: Shia, Cinnamon [mailto:cinnamon.s...@hpe.com] > Sent: Thursday, August 30, 2018 9:58 AM > To: Ni, Ruiyu ; edk2-devel@lists.01.org > Subject: RE: [edk2] [PATCH v2 00/12] Add WinHost support in EmulatorPkg > > Hi Ray, > > Does this change mean that Nt32Pkg is going to be retired? > > Thanks > Cinnamon Shia > > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu > Ni > Sent: Thursday, August 23, 2018 5:56 PM > To: edk2-devel@lists.01.org > Subject: [edk2] [PATCH v2 00/12] Add WinHost support in EmulatorPkg > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1112 > > The patch sets add WinHost support (Nt32) in EmulatorPkg. > 3 EmulatorPkg common issues were found and fixed. > Other 9 patches are to step-by-step enable the WinHost. > > v2 sends to correct mail address. > > Ruiyu Ni (12): > EmulatorPkg/ThunkProtocolList: Fix VS build failure > EmulatorPkg/Win: Add Windows host support > EmulatorPkg/Win: Enable source level debugging > EmulatorPkg/Win: Enable native OS console as firmware console > EmulatorPkg/Win: Add input/output support > EmulatorPkg/Win: Add timer and interrupt support > EmulatorPkg/Win: Add RTC support > EmulatorPkg/Win: Add SimpleFileSystem support > EmulatorPkg/Win: Add BlockIo support > EmulatorPkg/PlatformBds: Signal EndOfDxe in platform BDS > EmulatorPkg/EmuFileSystem: Fix a bug that causes Close() assertion > EmulatorPkg/DSC: Remove FS mapping to EDK Shell bin directory > > .../EmuSimpleFileSystemDxe/EmuSimpleFileSystem.c | 33 +- > EmulatorPkg/EmulatorPkg.dsc| 17 +- > EmulatorPkg/Library/EmuBdsLib/BdsPlatform.c|4 +- > EmulatorPkg/Library/EmuBdsLib/BdsPlatform.h|4 +- > EmulatorPkg/Library/EmuBdsLib/EmuBdsLib.inf|5 +- > .../Library/ThunkProtocolList/ThunkProtocolList.c |4 +- > EmulatorPkg/Win/Host/WinBlockIo.c | 563 + > EmulatorPkg/Win/Host/WinFileSystem.c | 2409 > > EmulatorPkg/Win/Host/WinGop.h | 204 ++ > EmulatorPkg/Win/Host/WinGopInput.c | 417 > EmulatorPkg/Win/Host/WinGopScreen.c| 872 +++ > EmulatorPkg/Win/Host/WinHost.c | 947 > EmulatorPkg/Win/Host/WinHost.h | 209 ++ > EmulatorPkg/Win/Host/WinHost.inf | 107 + > EmulatorPkg/Win/Host/WinInclude.h | 75 + > EmulatorPkg/Win/Host/WinMemoryAllocationLib.c | 178 ++ > EmulatorPkg/Win/Host/WinThunk.c| 577 + > 17 files changed, 6614 insertions(+), 11 deletions(-) create mode 100644 > EmulatorPkg/Win/Host/WinBlockIo.c create mode 100644 > EmulatorPkg/Win/Host/WinFileSystem.c > create mode 100644 EmulatorPkg/Win/Host/WinGop.h create mode 100644 > EmulatorPkg/Win/Host/WinGopInput.c > create mode 100644 EmulatorPkg/Win/Host/WinGopScreen.c > create mode 100644 EmulatorPkg/Win/Host/WinHost.c create mode 100644 > EmulatorPkg/Win/Host/WinHost.h create mode 100644 > EmulatorPkg/Win/Host/WinHost.inf create mode 100644 > EmulatorPkg/Win/Host/WinInclude.h create mode 100644 > EmulatorPkg/Win/Host/WinMemoryAllocationLib.c > create mode 100644 EmulatorPkg/Win/Host/WinThunk.c > > -- > 2.16.1.windows.1 > > ___ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] MdeModulePkg/EhciDxe: fix host controller reset condition in BindingStart
Oh, yes. Got the point, the patch is correct indeed. :)
Thanks for the explanation.
Maybe, an abstracted function will make the logic more clear, like
EhcIsDebugPortInUse().
What do you think?
Thanks,
Star
-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com]
Sent: Monday, September 3, 2018 7:46 PM
To: Zeng, Star ; edk2-devel-01
Cc: Gerd Hoffmann ; Wang, Jian J ;
Ni, Ruiyu ; Shi, Steven
Subject: Re: [PATCH] MdeModulePkg/EhciDxe: fix host controller reset condition
in BindingStart
Hi Star,
On 09/03/18 10:35, Zeng, Star wrote:
> Good finding.
>
> There is another place at
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Bus/Pci/Ehc
> iDxe/Ehci.c#L150 which has similar logic, please update it also.
I audited all the "DebugPortNum" checks that were added by commit 09943f5ecc0f,
here:
https://bugzilla.tianocore.org/show_bug.cgi?id=1129#c16
The location that you mention is in the EhcReset() function. It is covered in
my analysis linked above, and it needs no update, because it is correct.
Namely, in EhcReset(), the condition captures when the reset must be *skipped*.
For that, the condition is,
one of the USB ports on the host controller is a debug port, AND
the debug port is in use
When this condition holds, we set Status to EFI_SUCCESS, and jump to ON_EXIT
(that is, we skip the re-set and return success without doing anything). This
is written in C as
> if (Ehc->DebugPortNum != 0) {
> DbgCtrlStatus = EhcReadDbgRegister(Ehc, 0);
> if ((DbgCtrlStatus & (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) ==
> (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) {
> Status = EFI_SUCCESS;
> goto ON_EXIT;
> }
> }
However, at the location that the patch modifies, the condition refers to the
*opposite* case, that is, when the reset must *not* be skipped.
For that, we have to negate the condition seen above.
For the negation, we use De Morgan's Laws, that is:
!(A && B) == (!A || !B)
That is:
*no* USB port on the host controller is a debug port, *OR*
the debug port is *not* in use
And this is written, in C, as
> if (Ehc->DebugPortNum == 0) {
> EhcResetHC (Ehc, EHC_RESET_TIMEOUT);
> } else {
> State = EhcReadDbgRegister(Ehc, 0);
> if ((State & (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) !=
> (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) {
> EhcResetHC (Ehc, EHC_RESET_TIMEOUT);
> }
> }
The original commit (09943f5ecc0f) got the EhcReset() modification right (that
is, the condition is already correct); what it got wrong was the negation of
the condition, for EhcDriverBindingStart(). That's the only thing we have to
correct in this patch.
Thanks,
Laszlo
>
>
> Thanks,
> Star
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Friday, August 31, 2018 10:04 PM
> To: edk2-devel-01
> Cc: Gerd Hoffmann ; Wang, Jian J
> ; Ni, Ruiyu ; Zeng, Star
> ; Shi, Steven
> Subject: [PATCH] MdeModulePkg/EhciDxe: fix host controller reset
> condition in BindingStart
>
> Commit 09943f5ecc0f ("MdeModulePkg: Skip to manage usb debug port in
> EDKII EHCI driver if it's used by usb debug port driver", 2012-04-28)
> made the host controller reset in EhcDriverBindingStart() conditional.
> The intent was to avoid the reset when
> - one of the USB ports on the host controller was a Debug Port, AND
> - the Debug Port was in use.
>
> This translates to the following positive condition: reset the controller
> only if:
> - no USB port on the host controller is a Debug Port, OR
> - the Debug Port is not in use.
>
> Commit 09943f5ecc0f failed to implement the first subcondition, and thus the
> result is too strict now (for resetting the host controller). Relax it to the
> correct condition.
>
> This issue was found by Steven Shi on QEMU. QEMU's EHCI device model does not
> have a Debug Port. In repeated disconnect / connect cycles, the controller is
> never reset in EhcDriverBindingStart(), therefore the devices on the
> controller are never re-enumerated after a disconnect.
>
> Cc: Gerd Hoffmann
> Cc: Jian J Wang
> Cc: Ruiyu Ni
> Cc: Star Zeng
> Cc: Steven Shi
> Reported-by: Steven Shi
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1129
> Fixes: 09943f5ecc0fbc0c98c511c82703a0ba3b2b5819
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek
> ---
>
> Notes:
> Repo: https://github.com/lersek/edk2.git
> Branch: ehci_start_reset_1129
>
> MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> index 30ad2b2ffeef..89ed034b9093 100644
> --- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> +++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> @@ -1916,11 +1916,13 @@ EhcDriverBindingStart (
>//
>if (FeaturePcdGet (PcdTurnOffUsbLegacySupport)) {
> EhcClearLegacySupport (Ehc);
>}
>
> -
[edk2] [PATCH] EmulatorPkg: Update package level Readme.md
Since the emulator under Windows is enabled, the patch changes README to include the information of emulator under Windows. It also changes README to Readme.md for better looking. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Cc: Michael D Kinney Cc: Hao A Wu --- EmulatorPkg/README| 35 -- EmulatorPkg/Readme.md | 68 +++ 2 files changed, 68 insertions(+), 35 deletions(-) delete mode 100644 EmulatorPkg/README create mode 100644 EmulatorPkg/Readme.md diff --git a/EmulatorPkg/README b/EmulatorPkg/README deleted file mode 100644 index fdb26de503..00 --- a/EmulatorPkg/README +++ /dev/null @@ -1,35 +0,0 @@ - -=== EmulatorPkg Overview === - -EmulatorPkg provides an environment where a UEFI environment can be -emulated under an environment where a full UEFI compatible -environment is not possible. (For example, running under an OS -where an OS process hosts the UEFI emulation environment.) - -https://github.com/tianocore/tianocore.github.io/wiki/EmulatorPkg - -=== Status === - -* Builds and runs under a posix-like environment with X windows - - Linux - - OS X - -=== Future Plans === - -* Win32 and Win64 support - -=== Build Scripts === - -On systems with the bash shell you can use EmulatorPkg/build.sh to simplify -building and running EmulatorPkg. - -For example, to build + run: -$ EmulatorPkg/build.sh -$ EmulatorPkg/build.sh run - -The build architecture will match your host machine's architecture. - -On X64 host machines, you can build + run IA32 mode as well: -$ EmulatorPkg/build.sh -a IA32 -$ EmulatorPkg/build.sh -a IA32 run - diff --git a/EmulatorPkg/Readme.md b/EmulatorPkg/Readme.md new file mode 100644 index 00..461975e859 --- /dev/null +++ b/EmulatorPkg/Readme.md @@ -0,0 +1,68 @@ +## Overview + +EmulatorPkg provides an environment where a UEFI environment can be +emulated under an environment where a full UEFI compatible +environment is not possible. (For example, running under an OS +where an OS process hosts the UEFI emulation environment.) + +https://github.com/tianocore/tianocore.github.io/wiki/EmulatorPkg + +## Status + +* Builds and runs under + * a posix-like environment with X windows + - Linux + - OS X + * Windows environment + - Win10 (verified) + - Win8 (not verified) + +## How to Build & Run +**You can use the following command to build.** + * 32bit emulator in Windows: + +`build -p EmulatorPkg\EmulatorPkg.dsc -t VS2017 -D WIN_SEC_BUILD -a IA32` + + * 64bit emulator in Windows: + +`build -p EmulatorPkg\EmulatorPkg.dsc -t VS2017 -D WIN_SEC_BUILD -a X64` + + * 32bit emulator in Linux: + +`build -p EmulatorPkg\EmulatorPkg.dsc -t GCC5 -D UNIX_SEC_BUILD -a IA32` + + * 64bit emulator in Linux: + +`build -p EmulatorPkg\EmulatorPkg.dsc -t GCC5 -D UNIX_SEC_BUILD -a X64` + +**You can start/run the emulator using the following command:** + * 32bit emulator in Windows: + +`cd Build\EmulatorIA32\DEBUG_VS2017\IA32\ && WinHost.exe` + + * 64bit emulator in Windows: + +`cd Build\EmulatorX64\DEBUG_VS2017\X64\ && WinHost.exe` + + * 32bit emulator in Linux: + +`cd Build/EmulatorIA32/DEBUG_GCC5/IA32/ && ./Host` + + * 64bit emulator in Linux: + +`cd Build/EmulatorX64/DEBUG_GCC5/X64/ && ./Host` + +**On posix-like environment with the bash shell you can use EmulatorPkg/build.sh to simplify building and running +emulator.** + +For example, to build + run: + +`$ EmulatorPkg/build.sh` +`$ EmulatorPkg/build.sh run` + +The build architecture will match your host machine's architecture. + +On X64 host machines, you can build + run IA32 mode as well: + +`$ EmulatorPkg/build.sh -a IA32` +`$ EmulatorPkg/build.sh -a IA32 run` -- 2.16.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] MdeModulePkg PeiCore: Fix VS2012 build failure
fwvol.c(1572) : warning C4701: potentially uninitialized local variable 'Status' used The build failure is caused by 0e042d0ad76157ac9bad17bb4e1ff2919ca0d8f4 for https://bugzilla.tianocore.org/show_bug.cgi?id=1131 This patch initializes Status to fix the build failure. Cc: Dandan Bi Cc: Liming Gao Cc: Jian J Wang Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng --- MdeModulePkg/Core/Pei/FwVol/FwVol.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MdeModulePkg/Core/Pei/FwVol/FwVol.c b/MdeModulePkg/Core/Pei/FwVol/FwVol.c index 7ea503a10fb5..5629c9a1ce20 100644 --- a/MdeModulePkg/Core/Pei/FwVol/FwVol.c +++ b/MdeModulePkg/Core/Pei/FwVol/FwVol.c @@ -1412,6 +1412,8 @@ ProcessFvFile ( ParentFvHandle = ParentFvCoreHandle->FvHandle; ParentFvPpi= ParentFvCoreHandle->FvPpi; + Status = EFI_SUCCESS; + // // Find FvImage(s) in FvFile // -- 2.7.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] Emulator/Win: Fix build failure using VS2015x86
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Cc: Michael D Kinney Cc: Hao A Wu --- EmulatorPkg/Win/Host/WinHost.c| 2 +- EmulatorPkg/Win/Host/WinInclude.h | 15 --- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/EmulatorPkg/Win/Host/WinHost.c b/EmulatorPkg/Win/Host/WinHost.c index 9b98d5330f..65e8960eff 100644 --- a/EmulatorPkg/Win/Host/WinHost.c +++ b/EmulatorPkg/Win/Host/WinHost.c @@ -673,7 +673,7 @@ Returns: // Transfer control to the SEC Core // SwitchStack ( -(SWITCH_STACK_ENTRY_POINT)SecCoreEntryPoint, +(SWITCH_STACK_ENTRY_POINT)(UINTN)SecCoreEntryPoint, SecCoreData, GetThunkPpiList (), TopOfStack diff --git a/EmulatorPkg/Win/Host/WinInclude.h b/EmulatorPkg/Win/Host/WinInclude.h index ae90b1ed30..39a5427dae 100644 --- a/EmulatorPkg/Win/Host/WinInclude.h +++ b/EmulatorPkg/Win/Host/WinInclude.h @@ -1,7 +1,7 @@ /**@file Public include file for the WinNt Library -Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved. +Copyright (c) 2006 - 2018, 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 @@ -11,8 +11,8 @@ 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 __WIN_NT_INCLUDE_H__ -#define __WIN_NT_INCLUDE_H__ +#ifndef __WIN_INCLUDE_H__ +#define __WIN_INCLUDE_H__ // // Win32 include files do not compile clean with /W4, so we use the warning @@ -72,4 +72,13 @@ typedef UINT32 size_t ; #pragma warning(default : 4201) +// +// Define the three macros here in-case the WinSDK is too old. +// +#ifndef ENABLE_VIRTUAL_TERMINAL_INPUT +#define ENABLE_VIRTUAL_TERMINAL_INPUT 0x0200 +#define ENABLE_VIRTUAL_TERMINAL_PROCESSING 0x0004 +#define DISABLE_NEWLINE_AUTO_RETURN 0x0008 +#endif + #endif -- 2.16.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch] MdeModulePkg/Setup: Fix incorrect size used in AllocateCopyPool
Reviewed-by: Eric Dong
-Original Message-
From: Bi, Dandan
Sent: Tuesday, August 28, 2018 10:06 AM
To: edk2-devel@lists.01.org
Cc: Dong, Eric
Subject: [patch] MdeModulePkg/Setup: Fix incorrect size used in AllocateCopyPool
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1115
When the type of HiiValue is EFI_IFR_TYPE_BUFFER, its question type is
EFI_IFR_ORDERED_LIST_OP.
And the buffer size allocated for Statement->BufferValue of orderedList is
"Statement->StorageWidth"
in IfrParse.c.
So here when backup the buffer value and copy the size of
"Statement->StorageWidth + sizeof(CHAR16)" is incorrect.
This patch is to fix this issue.
Cc: Eric Dong
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi
---
MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c
b/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c
index ded1c7ad11..58daaab404 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c
@@ -2002,11 +2002,11 @@ ProcessCallBackFunction (
//
// If EFI_BROWSER_ACTION_CHANGING type, back up the new question value.
//
if (Action == EFI_BROWSER_ACTION_CHANGING) {
if (HiiValue->Type == EFI_IFR_TYPE_BUFFER) {
-BackUpBuffer = AllocateCopyPool(Statement->StorageWidth +
sizeof(CHAR16), Statement->BufferValue);
+BackUpBuffer = AllocateCopyPool(Statement->StorageWidth,
+ Statement->BufferValue);
ASSERT (BackUpBuffer != NULL);
} else {
CopyMem (, >Value, sizeof (EFI_IFR_TYPE_VALUE));
}
}
@@ -2128,11 +2128,11 @@ ProcessCallBackFunction (
// then the browser will use the value passed to Callback() and ignore
the
// value returned by Callback().
//
if (Action == EFI_BROWSER_ACTION_CHANGING && Status == EFI_UNSUPPORTED)
{
if (HiiValue->Type == EFI_IFR_TYPE_BUFFER) {
- CopyMem (Statement->BufferValue, BackUpBuffer,
Statement->StorageWidth + sizeof(CHAR16));
+ CopyMem (Statement->BufferValue, BackUpBuffer,
+ Statement->StorageWidth);
} else {
CopyMem (>Value, , sizeof
(EFI_IFR_TYPE_VALUE));
}
//
--
2.14.3.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Missing ‘or’ in CLA
On Mon, Sep 3, 2018 at 4:08 AM Leif Lindholm wrote: > On Fri, Aug 31, 2018 at 09:30:31PM +0200, Laszlo Ersek wrote: > > On 08/31/18 01:58, Henri Yandell wrote: > > > The CLA is missing an ‘or’ in section 3. > > > > > > See https://github.com/tianocore/edk2/pull/133/files for an example > of the > > > specific text. > > > > could you please turn this report into a real patch (if the suggested > > change is valid)? Technically it's easy enough so I could do it as well, > > just so we have something to review on the list, but: > > - I have no idea if the suggested change is valid, > > - I wasn't around when "Contributions.txt" was originally invented. > > Would this require a revision bump to 1.2? > > Whether a valid correction or not, it changes the premises under which > certain organisations have confirmed they are happy to contribute. > Moreso in my eyes than the change that lead to the bump to 1.1. > > / > Leif Just as my tuppence - it relaxes rather than tightens the conditions, so it’s not a harmful change for those who did notice that the meaning of the original Apache ICLA has changed. I postulate that the majority assumed it had the same meaning as the Apache ICLA and agreed to that instead. Hen > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms v5 00/28] Upload for D06 platform
On Fri, Aug 31, 2018 at 09:26:42PM +0800, Ming Huang wrote: > The major features of this patchset include: > 1 D06 source code; > 2 Unify some D0x modules; > > Change since v4: > 1 build on every commit: > Squash "Add PciPlatformLib" to "Add several base file for D06"; > Reorder OemMiscLibD06 before "Add edk2-non-osi components for D06"; > Move some mudules after "Add edk2-non-osi components for D06"; > Move gOemConfigGuid to "Stop watchdog"; > 2 Delete needless SnpDxe; > 3 Reorder "Unify HisiAcpiPlatformDxe"; > 4 Modify Signed-off-by and add Reviewed-by; > 5 Modify other comments in v4; > > Code can also be found in github: > https://github.com/hisilicon/OpenPlatformPkg.git > branch: d06-platform-v5 For the series: Reviewed-by: Leif Lindholm Pushed as 1d331a2eaa..8c3914c90e. > Heyi Guo (3): > Hisilicon/D06: Add Debug Serial Port Init Driver > Hisilicon/Hi1620: Add ACPI PPTT table > Platform/Hisilicon/D06: Enable ACPI PPTT > > Luqi Jiang (1): > Hisilicon/D06: add apei driver > > Ming Huang (19): > Hisilicon/D0x: Modify PcdBootManagerMenuFile for build > Silicon/Hisilicon/Acpi: Unify HisiAcpiPlatformDxe > Hisilicon/D06: Add several base file for D06 > Platform/Hisilicon/D06: Add M41T83RealTimeClockLib > Hisilicon/D06: Add OemMiscLibD06 > Platform/Hisilicon/D06: Add edk2-non-osi components for D06 > Hisilicon/D06: Add some modules > Silicon/Hisilicon/D06: Wait for all disk ready > Hisilicon/D06: Add ACPI Tables for D06 > Silicon/Hisilicon/D06: Stop watchdog > Platform/Hisilicon/D06: Add OemNicLib > Platform/Hisilicon/D06: Add OemNicConfig2P Driver > Platform/Hisilicon/D06: Add EarlyConfigPeim peim > Platform/Hisilicon/D06: Add PciHostBridgeLib > Platform/Hisilicon/D06: Add capsule upgrade support > Silicon/Hisilicon: Add I2C Bus Exception handle function > Silicon/Hisilicon/Setup: Support SPCR table switch > Silicon/Hisilicon/setup: Enable/disable SMMU > Platform/Hisilicon/D0x: Update version string to 18.08 > > Sun Yuanchen (2): > Silicon/Hisilicon/D0x: Move RAS macro to PlatformArch.h > Hisilicon/D0x: Update SMBIOS type9 info > > Yang XinYi (2): > Hisilicon/D06: Add Hi1620OemConfigUiLib > Silicon/Hisilicon/Hi1620/Setup: Add Setup Item "EnableGOP" > > ZhenYao (1): > Silicon/Hisilicon: Modify for disable slave core clock. > > Platform/Hisilicon/D06/D06.dec| 29 + > Silicon/Hisilicon/Hi1620/Hi1620.dec | 23 + > Silicon/Hisilicon/HisiPkg.dec |1 + > Platform/Hisilicon/D03/D03.dsc|4 +- > Platform/Hisilicon/D05/D05.dsc|4 +- > Platform/Hisilicon/D06/D06.dsc| 489 > Platform/Hisilicon/D06/D06.fdf| 441 > .../OemMiscLib2P/OemMiscLib2PHi1610.inf |1 + > .../Library/OemMiscLibD05/OemMiscLibD05.inf |1 + > .../OemNicConfig2PHi1620/OemNicConfig2P.inf | 43 + > .../SystemFirmwareDescriptor.inf | 50 + > .../EarlyConfigPeim/EarlyConfigPeimD06.inf| 50 + > .../Library/OemMiscLibD06/OemMiscLibD06.inf | 50 + > .../D06/Library/OemNicLib/OemNicLib.inf | 35 + > .../PciHostBridgeLib/PciHostBridgeLib.inf | 36 + > .../HisiAcpiPlatformDxe/AcpiPlatformDxe.inf |3 +- > .../Hisilicon/Hi1620/Drivers/Apei/Apei.inf| 59 + > .../Pl011DebugSerialPortInitDxe.inf | 48 + > .../Hi1620AcpiTables/AcpiTablesHi1620.inf | 60 + > .../Hi1620OemConfigUiLib/OemConfigUiLib.inf | 68 + > .../Hi1620PciPlatformLib.inf | 30 + > Silicon/Hisilicon/Hi1620/Pptt/Pptt.inf| 48 + > .../M41T83RealTimeClockLib.inf| 46 + > .../PlatformBootManagerLib.inf|5 + > .../OemNicConfig2PHi1620/OemNicConfig.h | 25 + > .../Hisilicon/D06/Include/Library/CpldD06.h | 39 + > .../Hisilicon/Hi1610/Include/PlatformArch.h | 15 +- > .../Hisilicon/Hi1616/Include/PlatformArch.h | 12 + > Silicon/Hisilicon/Hi1620/Drivers/Apei/Apei.h | 41 + > .../Hisilicon/Hi1620/Drivers/Apei/Bert/Bert.h | 43 + > .../Hisilicon/Hi1620/Drivers/Apei/Einj/Einj.h | 146 ++ > .../Hi1620/Drivers/Apei/ErrorSource/Ghes.h| 110 + > .../Hisilicon/Hi1620/Drivers/Apei/Erst/Erst.h | 140 ++ > .../Hisilicon/Hi1620/Drivers/Apei/Hest/Hest.h | 59 + > .../Hi1620/Drivers/Apei/OemApeiHi1620.h | 43 + > .../Hi1620/Hi1620AcpiTables/Hi1620Platform.h | 27 + > .../Hi1620/Hi1620OemConfigUiLib/OemConfig.h | 142 ++ > .../Hi1620/Hi1620OemConfigUiLib/OemConfigUi.h | 64 + > .../Hi1620/Include/Library/SerdesLib.h| 85 + > .../Hisilicon/Hi1620/Include/PlatformArch.h | 67 + > Silicon/Hisilicon/Hi1620/Pptt/Pptt.h | 68 + > .../Hisilicon/Include/Library/AcpiNextLib.h | 31 +- > .../Hisilicon/Include/Library/IpmiCmdLib.h| 16 + > .../Include/Library/OemAddressMapLib.h|8 + > .../Hisilicon/Include/Library/OemConfigData.h | 84 + >
Re: [edk2] [PATCH edk2-non-osi v4 0/2] Upload D06 binary modules
On Thu, Aug 23, 2018 at 11:59:33PM +0800, Ming Huang wrote: > This patch set include: > 1 Add D06 binary modules; > 2 Add PLATFORM_SAS_NOTIFY API. > > Change since v2: > 1 Add PLATFORM_SAS_NOTIFY API; > 2 Drop Oem Shell libraries; > > Code can also be found in github: > https://github.com/hisilicon/OpenPlatformPkg.git > branch: d06-non-osi-v4 For the series: Reviewed-by: Leif Lindholm Pushed as 3ce657b0..572f1053. > Ming Huang (2): > Hisilicon: Add dec and header file > Hisilicon/D06: Add binary modules > > Silicon/Hisilicon/HisiliconNonOsi.dec | 26 ++ > .../Drivers/GetInfoFromBmc/GetInfoFromBmc.inf | 26 ++ > .../D06/Drivers/IoInitDxe/IoInitDxe.inf | 27 +++ > .../IpmiInterfaceDxe/IpmiInterfaceDxe.inf | 28 +++ > .../IpmiInterfacePei/IpmiInterfacePei.inf | 27 +++ > .../Drivers/IpmiMiscOpDxe/IpmiMiscOpDxe.inf | 27 +++ > .../IpmiWatchdogDxe/IpmiWatchdogDxe.inf | 27 +++ > .../Drivers/Net/SnpHi1620NewDxe/SnpDxe.inf| 27 +++ > .../Drivers/PcieRasInitDxe/PcieRasInitDxe.inf | 26 ++ > .../D06/Drivers/RasInitDxe/RasInitDxe.inf | 25 ++ > .../D06/Drivers/SFC/SfcDxeDriver.inf | 27 +++ > .../D06/Drivers/Sas/SasDxeDriver.inf | 27 +++ > .../D06/Drivers/Sm750Dxe/UefiSmi.inf | 32 + > .../TransferSmbiosInfo/TransSmbiosInfo.inf| 26 ++ > .../OemAddressMapD06/OemAddressMapD06.inf | 40 > .../D06/MemoryInitPei/MemoryInitPeim.inf | 28 +++ > .../Library/Hi1620Serdes/Hi1620SerdesLib.inf | 43 + > .../Hi1620/Library/LpcLibHi1620/LpcLib.inf| 39 +++ > .../PlatformSysCtrlLibHi1620.inf | 45 ++ > .../Include/Protocol/PlatformSasNotify.h | 27 +++ > .../GetInfoFromBmc/GetInfoFromBmc.depex | Bin 0 -> 18 bytes > .../Drivers/GetInfoFromBmc/GetInfoFromBmc.efi | Bin 0 -> 20480 bytes > .../D06/Drivers/IoInitDxe/IoInitDxe.depex | Bin 0 -> 18 bytes > .../D06/Drivers/IoInitDxe/IoInitDxe.efi | Bin 0 -> 229216 bytes > .../IpmiInterfaceDxe/IpmiInterfaceDxe.depex | Bin 0 -> 18 bytes > .../IpmiInterfaceDxe/IpmiInterfaceDxe.efi | Bin 0 -> 29440 bytes > .../IpmiInterfacePei/IpmiInterfacePei.depex | Bin 0 -> 18 bytes > .../IpmiInterfacePei/IpmiInterfacePei.efi | Bin 0 -> 21664 bytes > .../Drivers/IpmiMiscOpDxe/IpmiMiscOp.depex| Bin 0 -> 36 bytes > .../D06/Drivers/IpmiMiscOpDxe/IpmiMiscOp.efi | Bin 0 -> 24736 bytes > .../IpmiWatchdogDxe/IpmiWatchdogDxe.depex | Bin 0 -> 36 bytes > .../IpmiWatchdogDxe/IpmiWatchdogDxe.efi | Bin 0 -> 20768 bytes > .../Net/SnpHi1620NewDxe/SnpPV600Dxe.efi | Bin 0 -> 75040 bytes > .../PcieRasInitDxe/PcieRasInitDxe.depex | Bin 0 -> 36 bytes > .../Drivers/PcieRasInitDxe/PcieRasInitDxe.efi | Bin 0 -> 21248 bytes > .../D06/Drivers/RasInitDxe/RasInitDxe.efi | Bin 0 -> 17984 bytes > .../Hisilicon/D06/Drivers/SFC/SFCDriver.depex | Bin 0 -> 36 bytes > .../Hisilicon/D06/Drivers/SFC/SFCDriver.efi | Bin 0 -> 262144 bytes > .../D06/Drivers/Sas/SasDriverDxe.depex| Bin 0 -> 216 bytes > .../D06/Drivers/Sas/SasDriverDxe.efi | Bin 0 -> 221312 bytes > .../Drivers/Sm750Dxe/SmiGraphicsOutput.efi| Bin 0 -> 38208 bytes > .../TransferSmbiosInfo/TransSmbiosInfo.depex | Bin 0 -> 36 bytes > .../TransferSmbiosInfo/TransSmbiosInfo.efi| Bin 0 -> 20288 bytes > .../OemAddressMapD06/OemAddressMapD06.lib | Bin 0 -> 61892 bytes > .../D06/MemoryInitPei/MemoryInit.depex| Bin 0 -> 18 bytes > .../D06/MemoryInitPei/MemoryInit.efi | Bin 0 -> 297696 bytes > Platform/Hisilicon/D06/Sec/FVMAIN_SEC.Fv | Bin 0 -> 1048576 bytes > Platform/Hisilicon/D06/bl1.bin| Bin 0 -> 12432 bytes > Platform/Hisilicon/D06/fip.bin| Bin 0 -> 113578 bytes > .../Library/Hi1620Serdes/Hi1620SerdesLib.lib | Bin 0 -> 1319320 bytes > .../Hi1620/Library/LpcLibHi1620/LpcLib.lib| Bin 0 -> 15406 bytes > .../PlatformSysCtrlLibHi1620.lib | Bin 0 -> 356032 bytes > 52 files changed, 600 insertions(+) > create mode 100644 Silicon/Hisilicon/HisiliconNonOsi.dec > create mode 100644 > Platform/Hisilicon/D06/Drivers/GetInfoFromBmc/GetInfoFromBmc.inf > create mode 100644 Platform/Hisilicon/D06/Drivers/IoInitDxe/IoInitDxe.inf > create mode 100644 > Platform/Hisilicon/D06/Drivers/Ipmi/IpmiInterfaceDxe/IpmiInterfaceDxe.inf > create mode 100644 > Platform/Hisilicon/D06/Drivers/Ipmi/IpmiInterfacePei/IpmiInterfacePei.inf > create mode 100644 > Platform/Hisilicon/D06/Drivers/IpmiMiscOpDxe/IpmiMiscOpDxe.inf > create mode 100644 > Platform/Hisilicon/D06/Drivers/IpmiWatchdogDxe/IpmiWatchdogDxe.inf > create mode 100644 > Platform/Hisilicon/D06/Drivers/Net/SnpHi1620NewDxe/SnpDxe.inf > create mode 100644 > Platform/Hisilicon/D06/Drivers/PcieRasInitDxe/PcieRasInitDxe.inf > create
Re: [edk2] [PATCH 0/4] Add PEI Stack Guard feature
On 09/03/18 05:15, Jian J Wang wrote: > This patch series try to add PEI Stack Guard feature. Please refer to > following trackers for details. > > The machanism behind this feature is the same as Stack Guard for UEFI > drivers, and similiar implementation is also employed. > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1126 > https://bugzilla.tianocore.org/show_bug.cgi?id=1137 > > Jian J Wang (4): > MdeModulePkg/DxeIpl: disable paging before creating new page table > UefiCpuPkg/CpuExceptionHandlerLib: support stack switch for PEI > exceptions > UefiCpuPkg/MpInitLib: fix register restore issue in AP wakeup > UefiCpuPkg/CpuMpPei: support stack guard feature > > MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c| 10 + > UefiCpuPkg/CpuMpPei/CpuMpPei.c | 269 - > UefiCpuPkg/CpuMpPei/CpuMpPei.h | 14 + > UefiCpuPkg/CpuMpPei/CpuMpPei.inf | 11 +- > UefiCpuPkg/CpuMpPei/CpuPaging.c| 637 > + > .../CpuExceptionHandlerLib/PeiCpuException.c | 27 +- > .../PeiCpuExceptionHandlerLib.inf | 4 + > UefiCpuPkg/Library/MpInitLib/MpLib.c | 8 +- > 8 files changed, 962 insertions(+), 18 deletions(-) > create mode 100644 UefiCpuPkg/CpuMpPei/CpuPaging.c > Regression-tested-by: Laszlo Ersek ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] MdeModulePkg/EhciDxe: fix host controller reset condition in BindingStart
Hi Star,
On 09/03/18 10:35, Zeng, Star wrote:
> Good finding.
>
> There is another place at
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c#L150
> which has similar logic, please update it also.
I audited all the "DebugPortNum" checks that were added by commit
09943f5ecc0f, here:
https://bugzilla.tianocore.org/show_bug.cgi?id=1129#c16
The location that you mention is in the EhcReset() function. It is
covered in my analysis linked above, and it needs no update, because it
is correct.
Namely, in EhcReset(), the condition captures when the reset must be
*skipped*. For that, the condition is,
one of the USB ports on the host controller is a debug port, AND
the debug port is in use
When this condition holds, we set Status to EFI_SUCCESS, and jump to
ON_EXIT (that is, we skip the re-set and return success without doing
anything). This is written in C as
> if (Ehc->DebugPortNum != 0) {
> DbgCtrlStatus = EhcReadDbgRegister(Ehc, 0);
> if ((DbgCtrlStatus & (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) ==
> (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) {
> Status = EFI_SUCCESS;
> goto ON_EXIT;
> }
> }
However, at the location that the patch modifies, the condition refers
to the *opposite* case, that is, when the reset must *not* be skipped.
For that, we have to negate the condition seen above.
For the negation, we use De Morgan's Laws, that is:
!(A && B) == (!A || !B)
That is:
*no* USB port on the host controller is a debug port, *OR*
the debug port is *not* in use
And this is written, in C, as
> if (Ehc->DebugPortNum == 0) {
> EhcResetHC (Ehc, EHC_RESET_TIMEOUT);
> } else {
> State = EhcReadDbgRegister(Ehc, 0);
> if ((State & (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) !=
> (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) {
> EhcResetHC (Ehc, EHC_RESET_TIMEOUT);
> }
> }
The original commit (09943f5ecc0f) got the EhcReset() modification right
(that is, the condition is already correct); what it got wrong was the
negation of the condition, for EhcDriverBindingStart(). That's the only
thing we have to correct in this patch.
Thanks,
Laszlo
>
>
> Thanks,
> Star
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Friday, August 31, 2018 10:04 PM
> To: edk2-devel-01
> Cc: Gerd Hoffmann ; Wang, Jian J ;
> Ni, Ruiyu ; Zeng, Star ; Shi, Steven
>
> Subject: [PATCH] MdeModulePkg/EhciDxe: fix host controller reset condition in
> BindingStart
>
> Commit 09943f5ecc0f ("MdeModulePkg: Skip to manage usb debug port in EDKII
> EHCI driver if it's used by usb debug port driver", 2012-04-28) made the host
> controller reset in EhcDriverBindingStart() conditional. The intent was to
> avoid the reset when
> - one of the USB ports on the host controller was a Debug Port, AND
> - the Debug Port was in use.
>
> This translates to the following positive condition: reset the controller
> only if:
> - no USB port on the host controller is a Debug Port, OR
> - the Debug Port is not in use.
>
> Commit 09943f5ecc0f failed to implement the first subcondition, and thus the
> result is too strict now (for resetting the host controller). Relax it to the
> correct condition.
>
> This issue was found by Steven Shi on QEMU. QEMU's EHCI device model does not
> have a Debug Port. In repeated disconnect / connect cycles, the controller is
> never reset in EhcDriverBindingStart(), therefore the devices on the
> controller are never re-enumerated after a disconnect.
>
> Cc: Gerd Hoffmann
> Cc: Jian J Wang
> Cc: Ruiyu Ni
> Cc: Star Zeng
> Cc: Steven Shi
> Reported-by: Steven Shi
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1129
> Fixes: 09943f5ecc0fbc0c98c511c82703a0ba3b2b5819
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek
> ---
>
> Notes:
> Repo: https://github.com/lersek/edk2.git
> Branch: ehci_start_reset_1129
>
> MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> index 30ad2b2ffeef..89ed034b9093 100644
> --- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> +++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> @@ -1916,11 +1916,13 @@ EhcDriverBindingStart (
>//
>if (FeaturePcdGet (PcdTurnOffUsbLegacySupport)) {
> EhcClearLegacySupport (Ehc);
>}
>
> - if (Ehc->DebugPortNum != 0) {
> + if (Ehc->DebugPortNum == 0) {
> +EhcResetHC (Ehc, EHC_RESET_TIMEOUT); } else {
> State = EhcReadDbgRegister(Ehc, 0);
> if ((State & (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) !=
> (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) {
>EhcResetHC (Ehc, EHC_RESET_TIMEOUT);
> }
>}
> --
> 2.14.1.3.gb7cf6e02401b
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
Re: [edk2] Missing ‘or’ in CLA
On Fri, Aug 31, 2018 at 09:30:31PM +0200, Laszlo Ersek wrote: > On 08/31/18 01:58, Henri Yandell wrote: > > The CLA is missing an ‘or’ in section 3. > > > > See https://github.com/tianocore/edk2/pull/133/files for an example of the > > specific text. > > could you please turn this report into a real patch (if the suggested > change is valid)? Technically it's easy enough so I could do it as well, > just so we have something to review on the list, but: > - I have no idea if the suggested change is valid, > - I wasn't around when "Contributions.txt" was originally invented. Would this require a revision bump to 1.2? Whether a valid correction or not, it changes the premises under which certain organisations have confirmed they are happy to contribute. Moreso in my eyes than the change that lead to the bump to 1.1. / Leif ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] MdeModulePkg/EhciDxe: fix host controller reset condition in BindingStart
Good finding.
There is another place at
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c#L150
which has similar logic, please update it also.
Thanks,
Star
-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com]
Sent: Friday, August 31, 2018 10:04 PM
To: edk2-devel-01
Cc: Gerd Hoffmann ; Wang, Jian J ;
Ni, Ruiyu ; Zeng, Star ; Shi, Steven
Subject: [PATCH] MdeModulePkg/EhciDxe: fix host controller reset condition in
BindingStart
Commit 09943f5ecc0f ("MdeModulePkg: Skip to manage usb debug port in EDKII EHCI
driver if it's used by usb debug port driver", 2012-04-28) made the host
controller reset in EhcDriverBindingStart() conditional. The intent was to
avoid the reset when
- one of the USB ports on the host controller was a Debug Port, AND
- the Debug Port was in use.
This translates to the following positive condition: reset the controller only
if:
- no USB port on the host controller is a Debug Port, OR
- the Debug Port is not in use.
Commit 09943f5ecc0f failed to implement the first subcondition, and thus the
result is too strict now (for resetting the host controller). Relax it to the
correct condition.
This issue was found by Steven Shi on QEMU. QEMU's EHCI device model does not
have a Debug Port. In repeated disconnect / connect cycles, the controller is
never reset in EhcDriverBindingStart(), therefore the devices on the controller
are never re-enumerated after a disconnect.
Cc: Gerd Hoffmann
Cc: Jian J Wang
Cc: Ruiyu Ni
Cc: Star Zeng
Cc: Steven Shi
Reported-by: Steven Shi
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1129
Fixes: 09943f5ecc0fbc0c98c511c82703a0ba3b2b5819
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek
---
Notes:
Repo: https://github.com/lersek/edk2.git
Branch: ehci_start_reset_1129
MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
index 30ad2b2ffeef..89ed034b9093 100644
--- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
@@ -1916,11 +1916,13 @@ EhcDriverBindingStart (
//
if (FeaturePcdGet (PcdTurnOffUsbLegacySupport)) {
EhcClearLegacySupport (Ehc);
}
- if (Ehc->DebugPortNum != 0) {
+ if (Ehc->DebugPortNum == 0) {
+EhcResetHC (Ehc, EHC_RESET_TIMEOUT); } else {
State = EhcReadDbgRegister(Ehc, 0);
if ((State & (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) !=
(USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) {
EhcResetHC (Ehc, EHC_RESET_TIMEOUT);
}
}
--
2.14.1.3.gb7cf6e02401b
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch] FDF Spec: Extend exclamation statement's keyword to case-insensitive
This patch add some description for "!include", "!error", "!if", etc, to extend those statement's keyword to case-insensitive. Cc: Liming Gao Cc: Michael Kinney Cc: Kevin W Shaw Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Yonghong Zhu --- 2_fdf_design_discussion/22_flash_description_file_format.md | 8 +--- 3_edk_ii_fdf_file_format/32_fdf_definition.md | 6 ++ README.md | 1 + 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/2_fdf_design_discussion/22_flash_description_file_format.md b/2_fdf_design_discussion/22_flash_description_file_format.md index 1092f00..3ea818f 100644 --- a/2_fdf_design_discussion/22_flash_description_file_format.md +++ b/2_fdf_design_discussion/22_flash_description_file_format.md @@ -209,11 +209,12 @@ directory this INF file is in or in sub-directories of this directory. ### 2.2.5 !include Statements The `!include` statement may appear within an EDK II FDF file. The included file content must match the content type of the current section definition, -contain complete sections, or combination of both. +contain complete sections, or combination of both. The keyword `!include` +is case-insensitive. The argument of this statement is a filename. The file is relative to the directory that contains this DSC file, and if not found the tool must attempt to find the file relative to paths listed in the system environment variables, `$(WORKSPACE)`, `$(PACKAGES_PATH)`, `$(EFI_SOURCE)`, `$(EDK_SOURCE)`, and @@ -523,11 +524,11 @@ statements may appear anywhere within the FDF file. ** **Note:** A limited number of statements are supported. This specification does not support every conditional statement that C programmers are familiar with. ** -Supported statements are: +Supported following statements and the keyword are case-insensitive: `!ifdef, !ifndef, !if, !elseif, !else and !endif` Refer to the Macro Statement section for information on using Macros in conditional directives. @@ -620,11 +621,12 @@ The following is an example of conditional statements. ### 2.2.9 !error Statement The `!error` statement may appear within any section of EDK II FDF file. The argument of this statement is an error message, it causes build tool to stop at the location where the statement is encountered and error message following -the `!error` statement is output as a message. +the `!error` statement is output as a message. The keyword `!error` is not +case-sensitive. The following example show the valid usage of the `!error` statement. ```ini !if $(FEATURE_ENABLE) == TRUE diff --git a/3_edk_ii_fdf_file_format/32_fdf_definition.md b/3_edk_ii_fdf_file_format/32_fdf_definition.md index 39d211d..4c323c5 100644 --- a/3_edk_ii_fdf_file_format/32_fdf_definition.md +++ b/3_edk_ii_fdf_file_format/32_fdf_definition.md @@ -425,10 +425,12 @@ C pre-processor. * Zero or more `!elseif` statements are permitted; only one `!else` statement is permitted. * Conditional directive blocks may be nested. +* Keyword `!ifdef, !ifndef, !if, !elseif, !else, !endif` are case-insensitive. + * Directives can be used to encapsulate entire sections or elements within a single section, such that they do not break the integrity of the section definitions. * Directives are in-fix expressions that are evaluated left to right; content @@ -658,10 +660,12 @@ to locate the file relaitive to the directory that contains the DSC file. If none of these methods find the file, and a directory separator is in ``, the tools attempt to find the file in a WORKSPACE (or a directory listed in the PACKAGES_PATH) relative path. If the file cannot be found, the build system must exit with an appropriate error message. +The keyword `!include` is case-insensitive. + Prototype ` ::= "!include" ` ## Example (EDK II FDF) @@ -681,10 +685,12 @@ Use of this statement is optional. This section defines the `!error` statement in EDK II FDF files. This statement is used to cause build tool to stop at the location where the statement is encountered and error message following the `!error` statement is output as a message. +The keyword `!error` is case-insensitive. + Prototype ` ::= "!error" ` ` ::= ` diff --git a/README.md b/README.md index cd489ee..5928df8 100644 --- a/README.md +++ b/README.md @@ -213,5 +213,6 @@ Copyright (c) 2006-2017, Intel Corporation. All rights reserved. || Per PI 1.6 to support FV extended header entry contain the used size of FV | | || Add !error statement section | | || clean up the and usage in spec
[edk2] [Patch] Build Spec: Extend exclamation statement's keyword to case-insensitive
This patch add some description for "!include", "!error", "!if", etc,
to extend those statement's keyword to case-insensitive.
Cc: Liming Gao
Cc: Michael Kinney
Cc: Kevin W Shaw
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu
---
.../82_auto-generation_process.md | 22 +-
README.md | 1 +
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/8_pre-build_autogen_stage/82_auto-generation_process.md
b/8_pre-build_autogen_stage/82_auto-generation_process.md
index abfa55c..6ce1710 100644
--- a/8_pre-build_autogen_stage/82_auto-generation_process.md
+++ b/8_pre-build_autogen_stage/82_auto-generation_process.md
@@ -268,11 +268,11 @@ Multiple library class instances for a single library
class must not be
specified in the same `[LibraryClasses]` or `` section in the
DSC file.
8.2.4.1 !include Files
-The DSC (and FDF) file can use `!include` statements to include text files that
+The DSC and FDF file can use `!include` statements to include text files that
contain content that would appear in the DSC file. When gathering the content
from the DSC (or FDF) file, the file pointed to by the !include statement is
read before any other information that appears later in the file.
The build system does not parse the files as the lines are read, but rather the
@@ -283,10 +283,12 @@ If only a filename is provided, the file must be located
in the same directory
as the DSC or FDF file. Use of `$(WORKSPACE)//` is allowed
for include files outside of the directory tree containing the DSC or FDF file,
or `/` if the include file is in the directory tree
containing the DSC or FDF file.
+The keyword `!include` is case-insensitive.
+
8.2.4.2 INF and DEC Parsing
The build tools try to parse the INF file one by one, including the INF file
for library instances. From the INF file, the build tools collect information
such as source file list, library class list, package list, GUID/Protocol/PPI
@@ -594,18 +596,18 @@ enclosed by double quotation marks.
When testing values for PCDs, only the PCD name is required:
`TokenSpaceGuidCname.PcdCname`; enclosing the PCD name in "$(" and ")" is not
permitted.
Supported statements are: `!ifdef`, `!ifndef`, `!if`, `!else`, `!elseif` and
-`!endif`. These control statements are used to either include or exclude lines
-as the parsing tool processes these files. The `!ifdef` and `!ifndef`
-statements test whether a Macro has been defined or not defined (PCDs are
-always defined - the build will break if a PCD is used by a module specified in
-the DSC file that cannot be located in any of the dependent DEC files, from the
-`[Packages]` section of an INF specified in the DSC file). FeatureFlag and
-FixedAtBuild access methods are the only PCDs that can be used in conditional
-directives.
+`!endif`, and those keywords are case-insensitive. These control statements are
+used to either include or exclude lines as the parsing tool processes these
files.
+The `!ifdef` and `!ifndef` statements test whether a Macro has been defined or
+not defined (PCDs are always defined - the build will break if a PCD is used by
+a module specified in the DSC file that cannot be located in any of the
dependent
+DEC files, from the `[Packages]` section of an INF specified in the DSC file).
+FeatureFlag and FixedAtBuild access methods are the only PCDs that can be used
in
+conditional directives.
The build system will process the DSC and FDF files more than once. The first
pass is to pick up all macros and PCD values for macros and PCDs used in
conditional directives, then on the second pass, process the conditional
directive content. This second pass is required as there is no required order
@@ -1064,10 +1066,12 @@ source files and generate the binary files.
The DSC and FDF file can use `!error` statement. The argument of this
statement is an
error message, it causes build tool to stop at the location where the
statement is
encountered and error message following the `!error` statement is output as a
message.
+The keyword `!error` is case-insensitive.
+
### 8.2.5 Post processing
Once all files are parsed, the build tools will do following work for each EDK
II module:
diff --git a/README.md b/README.md
index 9ca8733..8b20643 100644
--- a/README.md
+++ b/README.md
@@ -225,5 +225,6 @@ Copyright (c) 2008-2017, Intel Corporation. All rights
reserved.
|| Update PCD value and SKU, DefaultStore info in build report
| |
|| Clarify structure PCD field value assignment precedence
[edk2] [Patch] DSC Spec: Extend exclamation statement's keyword to case-insensitive
This patch add some description for "!include", "!error", "!if", etc, to extend those statement's keyword to case-insensitive. Cc: Liming Gao Cc: Michael Kinney Cc: Kevin W Shaw Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Yonghong Zhu --- 2_dsc_overview/22_build_description_file_format.md | 8 +--- 3_edk_ii_dsc_file_format/33_platform_dsc_definition.md | 6 ++ README.md | 1 + 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/2_dsc_overview/22_build_description_file_format.md b/2_dsc_overview/22_build_description_file_format.md index 9bf46a6..31e23f4 100644 --- a/2_dsc_overview/22_build_description_file_format.md +++ b/2_dsc_overview/22_build_description_file_format.md @@ -223,11 +223,12 @@ names. ### 2.2.5 !include Statement Processing The `!include` statement may appear within any section of EDK II DSC file. The included file content must match the content type of the current section -definition, contain complete sections, or combination of both. +definition, contain complete sections, or combination of both. And the keyword +`!include` is case-insensitive. The argument of this statement is a filename. The file is relative to the directory that contains this DSC file, and if not found the tool must attempt to find the file relative to the paths listed in the system environment variables `$(WORKSPACE)`, `$(EFI_SOURCE)`, `$(EDK_SOURCE)`, and @@ -502,11 +503,11 @@ statements may appear anywhere within the DSC file. ** **Note:** A limited number of statements are supported. This specification does not support every conditional statement that C programmers are familiar with. ** -Supported statements are: +Supported following statements and the keyword are case-insensitive: `!ifdef, !ifndef, !if, !elseif, !else and !endif` Refer to the Macro Statement section for information on using Macros in conditional directives. @@ -646,11 +647,12 @@ The following are examples of conditional directives. ### 2.2.9 !error Statement The `!error` statement may appear within any section of EDK II DSC file. The argument of this statement is an error message, it causes build tool to stop at the location where the statement is encountered and error message following -the `!error` statement is output as a message. +the `!error` statement is output as a message. The keyword `!error` is not +case-sensitive. The following example show the valid usage of the `!error` statement. ```ini !if $(FEATURE_ENABLE) == TRUE diff --git a/3_edk_ii_dsc_file_format/33_platform_dsc_definition.md b/3_edk_ii_dsc_file_format/33_platform_dsc_definition.md index 3bbcd9d..0ff9d9d 100644 --- a/3_edk_ii_dsc_file_format/33_platform_dsc_definition.md +++ b/3_edk_ii_dsc_file_format/33_platform_dsc_definition.md @@ -474,10 +474,12 @@ directive blocks can be nested. * Zero or more "!elseif" statements are permitted; only one "!else" statement is permitted. * Conditional directive blocks may be nested. +* Keyword `!ifdef, !ifndef, !if, !elseif, !else, !endif` are case-insensitive. + * Directives can be used to encapsulate entire sections or elements within a single section, such that they do not break the integrity of the section definitions. * Directives are in-fix expressions that are evaluated left to right; content @@ -701,10 +703,12 @@ PACKAGES_PATH) relative path. If the file cannot be found, the build system must exit with an appropriate error message. Statements in the include file are permitted to override previous definitions as well as to define new entries. +The keyword `!include` is case-insensitive. + Prototype ` ::= "!include" ` Example (EDK II DSC) @@ -730,10 +734,12 @@ Use of this statement is optional. This section defines the `!error` statement in EDK II Platform (DSC) files. This statement is used to cause build tool to stop at the location where the statement is encountered and error message following the `!error` statement is output as a message. +The keyword `!error` is case-insensitive. + Prototype ` ::= "!error" ` ` ::= ` diff --git a/README.md b/README.md index 5672273..17d4ebf 100644 --- a/README.md +++ b/README.md @@ -191,5 +191,6 @@ Copyright (c) 2006-2017, Intel Corporation. All rights reserved. || Add flexible PCD value format into spec || || Add syntax to support SKU ID inherit from another SKU ID
Re: [edk2] [PATCH] EmulatorPkg/PlatformBmLib: Fix GCC build failure
Reviewed-by: Dandan Bi
Thanks,
Dandan
-Original Message-
From: Ni, Ruiyu
Sent: Monday, September 3, 2018 10:24 AM
To: edk2-devel@lists.01.org
Cc: Bi, Dandan
Subject: [PATCH] EmulatorPkg/PlatformBmLib: Fix GCC build failure
Some local variables are initialized but never used.
GCC complains about that. The patch fixes this issue.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni
Cc: Dandan Bi
---
EmulatorPkg/Library/PlatformBmLib/PlatformBmMemoryTest.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/EmulatorPkg/Library/PlatformBmLib/PlatformBmMemoryTest.c
b/EmulatorPkg/Library/PlatformBmLib/PlatformBmMemoryTest.c
index 5b39776..b07226f 100644
--- a/EmulatorPkg/Library/PlatformBmLib/PlatformBmMemoryTest.c
+++ b/EmulatorPkg/Library/PlatformBmLib/PlatformBmMemoryTest.c
@@ -41,26 +41,15 @@ PlatformBootManagerMemoryTest (
EFI_GENERIC_MEMORY_TEST_PROTOCOL *GenMemoryTest;
UINT64TestedMemorySize;
UINT64TotalMemorySize;
- UINT64PreviousValue;
BOOLEAN ErrorOut;
BOOLEAN TestAbort;
EFI_INPUT_KEY Key;
- CHAR16*StrTotalMemory;
- CHAR16*Pos;
- UINTN StrTotalMemorySize;
ReturnStatus = EFI_SUCCESS;
ZeroMem (, sizeof (EFI_INPUT_KEY));
- StrTotalMemorySize = 128;
- Pos = AllocateZeroPool (StrTotalMemorySize);
- ASSERT (Pos != NULL);
-
- StrTotalMemory= Pos;
-
TestedMemorySize = 0;
TotalMemorySize = 0;
- PreviousValue = 0;
ErrorOut = FALSE;
TestAbort = FALSE;
@@ -72,7 +61,6 @@ PlatformBootManagerMemoryTest (
(VOID **)
);
if (EFI_ERROR (Status)) {
-FreePool (Pos);
return EFI_SUCCESS;
}
@@ -89,7 +77,6 @@ PlatformBootManagerMemoryTest (
// do the test, and then the status of EFI_NO_MEDIA will be returned by
// "MemoryTestInit". So it does not need to test memory again, just return.
//
-FreePool (Pos);
return EFI_SUCCESS;
}
@@ -128,6 +115,5 @@ PlatformBootManagerMemoryTest (
Done:
DEBUG ((DEBUG_INFO, "%d bytes of system memory tested OK\r\n",
TotalMemorySize));
- FreePool (Pos);
return ReturnStatus;
}
--
2.7.4
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API
I prefer S2. Single interface is more easy for consumer to remember how to use. Thanks/Ray > -Original Message- > From: Yao, Jiewen > Sent: Monday, September 3, 2018 2:15 PM > To: Zeng, Star ; Ni, Ruiyu ; > Kinney, Michael D > Cc: edk2-devel@lists.01.org; Younas khan ; > Gao, Liming > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new > EfiFindAcpiTableBySignature() API > > I prefer S1. > I believe that the EfiLocateNextAcpiTable() can also be used in S1. > > > > -Original Message- > > From: Zeng, Star > > Sent: Monday, September 3, 2018 2:12 PM > > To: Ni, Ruiyu ; Yao, Jiewen > > ; Kinney, Michael D > > Cc: edk2-devel@lists.01.org; Younas khan > ; > > Gao, Liming ; Zeng, Star > > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new > > EfiFindAcpiTableBySignature() API > > > > Thanks. > > Ok, please help and we can have good and flexible interface(s) for > > both producer and consumer. > > > > First, there are two cases we need to consider. > > 1. Single table, like FADT, FACS, DSDT, etc. > > 2. Multiple tables, like SSDT, etc. > > > > Then, we have two solutions. > > S1: > > One interface for single table case, consumer only needs input > > Signature, has no need input previous returned Table. Like > GetFirstGuidHob? > > One interface for multiple table case, consumer needs input Signature > > and previous returned Table. Like GetNextGuidHob? Could we add it > > later based on real request? > > S2: > > One interface for both single table and multiple table cases, consumer > > needs input Signature and previous returned Table. Does consumer need > > input the table header stored in configuration table by itself(getting > configuration table)? > > > > > > If we like S2, the interface should be like below? > > > > /** > > This function locates next ACPI table based on Signature and > > previous returned Table. > > If the input previous returned Table is NULL, this function will locate > > next > table > > in gEfiAcpi20TableGuid system configuration table first, and then > > gEfiAcpi10TableGuid > > system configuration table. > > If the input previous returned Table is not NULL and could be located in > > gEfiAcpi20TableGuid system configuration table, this function will > > just locate next > > table in gEfiAcpi20TableGuid system configuration table, otherwise > > gEfiAcpi10TableGuid > > system configuration table. > > > > @param Signature ACPI table signature. > > @param Table Pointer to previous returned Table, or NULL to locate > > first table. > > > > @return Next ACPI table or NULL if not found. > > > > **/ > > EFI_ACPI_COMMON_HEADER * > > EFIAPI > > EfiLocateNextAcpiTable ( > > IN UINT32 Signature, > > IN EFI_ACPI_COMMON_HEADER *Table > > ) > > > > > > > > Thanks, > > Star > > -Original Message- > > From: Ni, Ruiyu > > Sent: Monday, September 3, 2018 1:09 PM > > To: Zeng, Star ; Yao, Jiewen > > ; Kinney, Michael D > > Cc: edk2-devel@lists.01.org; Younas khan > ; > > Gao, Liming > > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new > > EfiFindAcpiTableBySignature() API > > > > That's fine to be in UefiLib. It's already a combo library. > > > > But I do recommend we think about how to handle multiple tables with > > same signature. > > When we are adding new APIs, we not only need to evaluate the existing > > real case, but also we need to generalize the real cases and try to > > think about the more flexible interface. > > > > > > Thanks/Ray > > > > > -Original Message- > > > From: Zeng, Star > > > Sent: Monday, September 3, 2018 11:26 AM > > > To: Yao, Jiewen ; Ni, Ruiyu > > > ; Kinney, Michael D > > > Cc: edk2-devel@lists.01.org; Younas khan > > > ; Gao, Liming ; > > > Zeng, Star > > > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new > > > EfiFindAcpiTableBySignature() API > > > > > > Good idea. > > > > > > I did consider DSDT and multiple SSDTs cases. But I did not find any > > > real case for them. > > > So I made the code simply for current cases, and the code can be > > > easily enhanced later for DSDT, a new API can be added later for > > > multiple > > SSDTs. > > > > > > About adding the new API in UefiLib VS new library class, I did also > > > consider it and even created code for new library class > > > (g...@github.com:lzeng14/edk2.git branch > > > FindAcpiTableBySignature_UefiAcpiTableLib). > > > But I remember I did discuss it with Mike and Jiewen, we recommended > > > UefiLib as it will do not require any platform change. > > > > > > > > > > > > Thanks, > > > Star > > > -Original Message- > > > From: Yao, Jiewen > > > Sent: Saturday, September 1, 2018 7:04 AM > > > To: Ni, Ruiyu > > > Cc: Zeng, Star ; edk2-devel@lists.01.org; > > > Kinney, Michael D ; Younas khan > > > ; Gao, Liming > > > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new > > > EfiFindAcpiTableBySignature() API > > > > > > Good idea on LocateNextAcpiTable(). > > > > > > > > > >
Re: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API
I prefer S1. I believe that the EfiLocateNextAcpiTable() can also be used in S1. > -Original Message- > From: Zeng, Star > Sent: Monday, September 3, 2018 2:12 PM > To: Ni, Ruiyu ; Yao, Jiewen ; > Kinney, > Michael D > Cc: edk2-devel@lists.01.org; Younas khan ; > Gao, Liming ; Zeng, Star > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new > EfiFindAcpiTableBySignature() API > > Thanks. > Ok, please help and we can have good and flexible interface(s) for both > producer > and consumer. > > First, there are two cases we need to consider. > 1. Single table, like FADT, FACS, DSDT, etc. > 2. Multiple tables, like SSDT, etc. > > Then, we have two solutions. > S1: > One interface for single table case, consumer only needs input Signature, has > no > need input previous returned Table. Like GetFirstGuidHob? > One interface for multiple table case, consumer needs input Signature and > previous returned Table. Like GetNextGuidHob? Could we add it later based on > real request? > S2: > One interface for both single table and multiple table cases, consumer needs > input Signature and previous returned Table. Does consumer need input the > table header stored in configuration table by itself(getting configuration > table)? > > > If we like S2, the interface should be like below? > > /** > This function locates next ACPI table based on Signature and previous > returned > Table. > If the input previous returned Table is NULL, this function will locate > next table > in gEfiAcpi20TableGuid system configuration table first, and then > gEfiAcpi10TableGuid > system configuration table. > If the input previous returned Table is not NULL and could be located in > gEfiAcpi20TableGuid system configuration table, this function will just > locate > next > table in gEfiAcpi20TableGuid system configuration table, otherwise > gEfiAcpi10TableGuid > system configuration table. > > @param Signature ACPI table signature. > @param Table Pointer to previous returned Table, or NULL to locate > first table. > > @return Next ACPI table or NULL if not found. > > **/ > EFI_ACPI_COMMON_HEADER * > EFIAPI > EfiLocateNextAcpiTable ( > IN UINT32 Signature, > IN EFI_ACPI_COMMON_HEADER *Table > ) > > > > Thanks, > Star > -Original Message- > From: Ni, Ruiyu > Sent: Monday, September 3, 2018 1:09 PM > To: Zeng, Star ; Yao, Jiewen ; > Kinney, Michael D > Cc: edk2-devel@lists.01.org; Younas khan ; > Gao, Liming > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new > EfiFindAcpiTableBySignature() API > > That's fine to be in UefiLib. It's already a combo library. > > But I do recommend we think about how to handle multiple tables with same > signature. > When we are adding new APIs, we not only need to evaluate the existing real > case, but also we need to generalize the real cases and try to think about the > more flexible interface. > > > Thanks/Ray > > > -Original Message- > > From: Zeng, Star > > Sent: Monday, September 3, 2018 11:26 AM > > To: Yao, Jiewen ; Ni, Ruiyu > > ; Kinney, Michael D > > Cc: edk2-devel@lists.01.org; Younas khan ; > > Gao, Liming ; Zeng, Star > > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new > > EfiFindAcpiTableBySignature() API > > > > Good idea. > > > > I did consider DSDT and multiple SSDTs cases. But I did not find any > > real case for them. > > So I made the code simply for current cases, and the code can be > > easily enhanced later for DSDT, a new API can be added later for multiple > SSDTs. > > > > About adding the new API in UefiLib VS new library class, I did also > > consider it and even created code for new library class > > (g...@github.com:lzeng14/edk2.git branch > > FindAcpiTableBySignature_UefiAcpiTableLib). > > But I remember I did discuss it with Mike and Jiewen, we recommended > > UefiLib as it will do not require any platform change. > > > > > > > > Thanks, > > Star > > -Original Message- > > From: Yao, Jiewen > > Sent: Saturday, September 1, 2018 7:04 AM > > To: Ni, Ruiyu > > Cc: Zeng, Star ; edk2-devel@lists.01.org; Kinney, > > Michael D ; Younas khan > > ; Gao, Liming > > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new > > EfiFindAcpiTableBySignature() API > > > > Good idea on LocateNextAcpiTable(). > > > > > > > -Original Message- > > > From: Ni, Ruiyu > > > Sent: Saturday, September 1, 2018 12:29 AM > > > To: Yao, Jiewen > > > Cc: Zeng, Star ; edk2-devel@lists.01.org; > > > Kinney, Michael D ; Younas khan > > > ; Gao, Liming > > > Subject: Re: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new > > > EfiFindAcpiTableBySignature() API > > > > > > I think LocateNextAcpiTable() is more proper to handle the multiple > > > tables with same signature. It will carry three parameters, one is > > > the table header stored in configuration table, one is the > > > signature, another is > > the previous located table. > > > Can we return a common table header other
Re: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API
Thanks. Ok, please help and we can have good and flexible interface(s) for both producer and consumer. First, there are two cases we need to consider. 1. Single table, like FADT, FACS, DSDT, etc. 2. Multiple tables, like SSDT, etc. Then, we have two solutions. S1: One interface for single table case, consumer only needs input Signature, has no need input previous returned Table. Like GetFirstGuidHob? One interface for multiple table case, consumer needs input Signature and previous returned Table. Like GetNextGuidHob? Could we add it later based on real request? S2: One interface for both single table and multiple table cases, consumer needs input Signature and previous returned Table. Does consumer need input the table header stored in configuration table by itself(getting configuration table)? If we like S2, the interface should be like below? /** This function locates next ACPI table based on Signature and previous returned Table. If the input previous returned Table is NULL, this function will locate next table in gEfiAcpi20TableGuid system configuration table first, and then gEfiAcpi10TableGuid system configuration table. If the input previous returned Table is not NULL and could be located in gEfiAcpi20TableGuid system configuration table, this function will just locate next table in gEfiAcpi20TableGuid system configuration table, otherwise gEfiAcpi10TableGuid system configuration table. @param Signature ACPI table signature. @param Table Pointer to previous returned Table, or NULL to locate first table. @return Next ACPI table or NULL if not found. **/ EFI_ACPI_COMMON_HEADER * EFIAPI EfiLocateNextAcpiTable ( IN UINT32 Signature, IN EFI_ACPI_COMMON_HEADER *Table ) Thanks, Star -Original Message- From: Ni, Ruiyu Sent: Monday, September 3, 2018 1:09 PM To: Zeng, Star ; Yao, Jiewen ; Kinney, Michael D Cc: edk2-devel@lists.01.org; Younas khan ; Gao, Liming Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API That's fine to be in UefiLib. It's already a combo library. But I do recommend we think about how to handle multiple tables with same signature. When we are adding new APIs, we not only need to evaluate the existing real case, but also we need to generalize the real cases and try to think about the more flexible interface. Thanks/Ray > -Original Message- > From: Zeng, Star > Sent: Monday, September 3, 2018 11:26 AM > To: Yao, Jiewen ; Ni, Ruiyu > ; Kinney, Michael D > Cc: edk2-devel@lists.01.org; Younas khan ; > Gao, Liming ; Zeng, Star > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new > EfiFindAcpiTableBySignature() API > > Good idea. > > I did consider DSDT and multiple SSDTs cases. But I did not find any > real case for them. > So I made the code simply for current cases, and the code can be > easily enhanced later for DSDT, a new API can be added later for multiple > SSDTs. > > About adding the new API in UefiLib VS new library class, I did also > consider it and even created code for new library class > (g...@github.com:lzeng14/edk2.git branch > FindAcpiTableBySignature_UefiAcpiTableLib). > But I remember I did discuss it with Mike and Jiewen, we recommended > UefiLib as it will do not require any platform change. > > > > Thanks, > Star > -Original Message- > From: Yao, Jiewen > Sent: Saturday, September 1, 2018 7:04 AM > To: Ni, Ruiyu > Cc: Zeng, Star ; edk2-devel@lists.01.org; Kinney, > Michael D ; Younas khan > ; Gao, Liming > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new > EfiFindAcpiTableBySignature() API > > Good idea on LocateNextAcpiTable(). > > > > -Original Message- > > From: Ni, Ruiyu > > Sent: Saturday, September 1, 2018 12:29 AM > > To: Yao, Jiewen > > Cc: Zeng, Star ; edk2-devel@lists.01.org; > > Kinney, Michael D ; Younas khan > > ; Gao, Liming > > Subject: Re: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new > > EfiFindAcpiTableBySignature() API > > > > I think LocateNextAcpiTable() is more proper to handle the multiple > > tables with same signature. It will carry three parameters, one is > > the table header stored in configuration table, one is the > > signature, another is > the previous located table. > > Can we return a common table header other than void*? > > > > Is there better place other than UefiLib? > > Do we need to add a new library class like AcpiLib? > > > > 发自我的 iPhone > > > > > 在 2018年8月31日，下午8:00，Yao, Jiewen > 写 > > 道： > > > > > > Good enhancement. > > > > > > I have 2 additional thought: > > > > > > 1) How to handle DSDT? > > > We have special code to handle FACS, but no DSDT. > > > > > > 2) How to handle SSDT or other multiple ACPI tables? > > > We may have multiple SSDT. Usually, it is identified as OEMID. > > > Do we want to provide similar function for them? > > > > > > Anyway, just *additional* thought. :-) Current implementation is > > > good enough for
