Hi Sai, I'd like to fix the VS2017 build failure asap. What would you like done to resolve this?
I would prefer to eliminate the local ResetSystemLib.h file in KabylakeSiliconPkg but I'd be happy to revise the patch series based on your suggestion. Thanks, Michael > -----Original Message----- > From: Kubacki, Michael A > Sent: Wednesday, November 27, 2019 10:42 AM > To: Chaganty, Rangasai V <rangasai.v.chaga...@intel.com>; > devel@edk2.groups.io > Cc: Chiu, Chasel <chasel.c...@intel.com>; Desimone, Nathaniel L > <nathaniel.l.desim...@intel.com>; Gao, Zhichao <zhichao....@intel.com> > Subject: RE: [edk2-platforms][PATCH V1 1/2] KabylakeSiliconPkg: Remove > ResetSystemLib.h override > > This dependency existed prior to this change (and still does exist). It was > obfuscated in such a way that contributed to this problem. > > See the previous library header path: > > \Silicon\Intel\KabylakeSiliconPkg\SampleCode\MdeModulePkg\Include\Libr > ary\ResetSystemLib.h > > The fact KabylakeSiliconPkg implements a MdeModulePkg library API > introduces the dependency on MdeModulePkg. Hiding a redundant > definition of the API locally does not eliminate the dependency in any > meaningful way. > > I think the practice of "freezing" an API with a local copy only works if the > codebase is locked onto a specific stable tag in which the upstream API is not > expected to change. Zhichao rightfully added the new function definition to > the KabylakeSiliconPkg library class implementation because a board package > consumer would expect a ResetSystemLib library class instance to be > compliant with the API defined in MdeModulePkg and link the ResetSystem > () function. The only problem was a set of circumstances that led to the > duplicate symbol definition for ResetSystem () with PchResetLib. > > So I view the task of eliminating the package dependency as a larger separate > effort outside the scope of this change. But I do not agree with maintaining > redundant local copies of edk2 APIs in packages in edk2-platforms. > > Thanks, > Michael > > > -----Original Message----- > > From: Chaganty, Rangasai V <rangasai.v.chaga...@intel.com> > > Sent: Tuesday, November 26, 2019 10:43 PM > > To: Kubacki, Michael A <michael.a.kuba...@intel.com>; > > devel@edk2.groups.io > > Cc: Chiu, Chasel <chasel.c...@intel.com>; Desimone, Nathaniel L > > <nathaniel.l.desim...@intel.com>; Gao, Zhichao > <zhichao....@intel.com> > > Subject: RE: [edk2-platforms][PATCH V1 1/2] KabylakeSiliconPkg: Remove > > ResetSystemLib.h override > > > > This change is introducing SiliconPkg's dependency on MdeModulePkg. > > SiliconPkg dependencies should be limited to a selected few packages > > and this seems to be an unnecessary addition to the dependency list. > > The reset interfaces are providing generic reset services and perhaps > > better suited in packages like MdePkg. > > > > -----Original Message----- > > From: Kubacki, Michael A > > Sent: Tuesday, November 26, 2019 6:57 PM > > To: devel@edk2.groups.io > > Cc: Chaganty, Rangasai V <rangasai.v.chaga...@intel.com>; Chiu, Chasel > > <chasel.c...@intel.com>; Desimone, Nathaniel L > > <nathaniel.l.desim...@intel.com>; Gao, Zhichao > <zhichao....@intel.com> > > Subject: [edk2-platforms][PATCH V1 1/2] KabylakeSiliconPkg: Remove > > ResetSystemLib.h override > > > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2375 > > > > Removes a stale ResetSystemLib.h override in KabylakeSiliconPkg that > > does not contain the prototype for ResetSystem () and > ResetPlatformSpecific (). > > > > The ResetSystemLib.h file from MdeModulePkg will be used. Any INF > > files that did not include the MdeModulePkg.dec under [Packages] were > > updated to do so. > > > > Cc: Sai Chaganty <rangasai.v.chaga...@intel.com> > > Cc: Chasel Chiu <chasel.c...@intel.com> > > Cc: Nate DeSimone <nathaniel.l.desim...@intel.com> > > Cc: Zhichao Gao <zhichao....@intel.com> > > Signed-off-by: Michael Kubacki <michael.a.kuba...@intel.com> > > --- > > > > > Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeResetSystemLib/DxeResetS > > ystemLib.inf | 3 +- > > > > Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeRuntimeResetSystemLib/ > > Dx > > eRuntimeResetSystemLib.inf | 3 +- > > > > > Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PeiPchResetLib. > > inf | 3 +- > > > > > Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetSys > > temLib.inf | 3 +- > > > > > Silicon/Intel/KabylakeSiliconPkg/SampleCode/MdeModulePkg/Include/Libra > > ry/ResetSystemLib.h | 62 -------------------- > > 5 files changed, 8 insertions(+), 66 deletions(-) > > > > diff --git > > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeResetSystemLib/DxeRe > > se > > tSystemLib.inf > > b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeResetSystemLib/DxeRe > > se > > tSystemLib.inf > > index aa8877140a..46313bf35f 100644 > > --- > > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeResetSystemLib/DxeRe > > se > > tSystemLib.inf > > +++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeResetSystemLib/D > > +++ xe > > +++ ResetSystemLib.inf > > @@ -1,7 +1,7 @@ > > ## @file > > # Component description file for Intel Ich7 Reset System Library. > > # > > -# Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> > > +# Copyright (c) 2017 - 2019, Intel Corporation. All rights > > +reserved.<BR> > > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -35,6 +35,7 @@ > > PchCycleDecodingLib > > > > [Packages] > > MdePkg/MdePkg.dec > > +MdeModulePkg/MdeModulePkg.dec > > KabylakeSiliconPkg/SiPkg.dec > > > > > > diff --git > > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeRuntimeResetSystemLi > > b/ > > DxeRuntimeResetSystemLib.inf > > b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeRuntimeResetSystemLi > > b/ > > DxeRuntimeResetSystemLib.inf > > index 6b27661603..c7fad31c71 100644 > > --- > > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeRuntimeResetSystemLi > > b/ > > DxeRuntimeResetSystemLib.inf > > +++ > > b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeRuntimeResetSystem > > +++ Lib/DxeRuntimeResetSystemLib.inf > > @@ -1,7 +1,7 @@ > > ## @file > > # Component description file for Intel Ich7 Reset System Library. > > # > > -# Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> > > +# Copyright (c) 2017 - 2019, Intel Corporation. All rights > > +reserved.<BR> > > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -36,6 +36,7 @@ > > PchCycleDecodingLib > > > > [Packages] > > MdePkg/MdePkg.dec > > +MdeModulePkg/MdeModulePkg.dec > > KabylakeSiliconPkg/SiPkg.dec > > > > > > diff --git > > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PeiPchRe > > setL > > ib.inf > > b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PeiPchRe > > setL > > ib.inf > > index b04f4006ef..29f69078a4 100644 > > --- > > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PeiPchRe > > setL > > ib.inf > > +++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PeiP > > +++ ch > > +++ ResetLib.inf > > @@ -1,7 +1,7 @@ > > ## @file > > # Component description file for PCH Reset Lib Pei Phase # -# > > Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> > > +# Copyright (c) 2017 - 2019, Intel Corporation. All rights > > +reserved.<BR> > > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -28,6 +28,7 @@ > > ResetSystemLib > > > > [Packages] > > MdePkg/MdePkg.dec > > +MdeModulePkg/MdeModulePkg.dec > > KabylakeSiliconPkg/SiPkg.dec > > > > [Sources] > > diff --git > > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiRe > > setS > > ystemLib.inf > > b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiRe > > set > > SystemLib.inf > > index 18a92a6f18..3c6ff78863 100644 > > --- > > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiRe > > setS > > ystemLib.inf > > +++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/P > > +++ ei > > +++ ResetSystemLib.inf > > @@ -1,7 +1,7 @@ > > ## @file > > # Component description file for Intel Ich7 Reset System Library. > > # > > -# Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> > > +# Copyright (c) 2017 - 2019, Intel Corporation. All rights > > +reserved.<BR> > > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -32,6 +32,7 @@ > > PchCycleDecodingLib > > > > [Packages] > > MdePkg/MdePkg.dec > > +MdeModulePkg/MdeModulePkg.dec > > KabylakeSiliconPkg/SiPkg.dec > > > > > > diff --git > > > a/Silicon/Intel/KabylakeSiliconPkg/SampleCode/MdeModulePkg/Include/Lib > > rary/ResetSystemLib.h > > > b/Silicon/Intel/KabylakeSiliconPkg/SampleCode/MdeModulePkg/Include/Lib > > rary/ResetSystemLib.h > > deleted file mode 100644 > > index 75d3e15ed7..0000000000 > > --- > > > a/Silicon/Intel/KabylakeSiliconPkg/SampleCode/MdeModulePkg/Include/Lib > > rary/ResetSystemLib.h > > +++ /dev/null > > @@ -1,62 +0,0 @@ > > -/** @file > > - System reset Library Services. This library class defines a set of > > - methods that reset the whole system. > > - > > -Copyright (c) 2005 - 2010, Intel Corporation. All rights > > reserved.<BR> > > -SPDX-License-Identifier: BSD-2-Clause-Patent > > - > > -**/ > > - > > -#ifndef __RESET_SYSTEM_LIB_H__ > > -#define __RESET_SYSTEM_LIB_H__ > > - > > -/** > > - This function causes a system-wide reset (cold reset), in which > > - all circuitry within the system returns to its initial state. This > > type of reset > > - is asynchronous to system operation and operates without regard to > > - cycle boundaries. > > - > > - If this function returns, it means that the system does not support > > cold reset. > > -**/ > > -VOID > > -EFIAPI > > -ResetCold ( > > - VOID > > - ); > > - > > -/** > > - This function causes a system-wide initialization (warm reset), in > > which all processors > > - are set to their initial state. Pending cycles are not corrupted. > > - > > - If this function returns, it means that the system does not support > > warm reset. > > -**/ > > -VOID > > -EFIAPI > > -ResetWarm ( > > - VOID > > - ); > > - > > -/** > > - This function causes the system to enter a power state equivalent > > - to the ACPI G2/S5 or G3 states. > > - > > - If this function returns, it means that the system does not support > > shutdown reset. > > -**/ > > -VOID > > -EFIAPI > > -ResetShutdown ( > > - VOID > > - ); > > - > > -/** > > - This function causes the system to enter S3 and then wake up > immediately. > > - > > - If this function returns, it means that the system does not support > > S3 feature. > > -**/ > > -VOID > > -EFIAPI > > -EnterS3WithImmediateWake ( > > - VOID > > - ); > > - > > -#endif > > -- > > 2.16.2.windows.1 > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#51382): https://edk2.groups.io/g/devel/message/51382 Mute This Topic: https://groups.io/mt/62194086/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
