From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by mx.groups.io with SMTP id smtpd.web12.1000.1574880097100111306 for ; Wed, 27 Nov 2019 10:41:37 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=cSMy+oSa; spf=pass (domain: intel.com, ip: 192.55.52.151, mailfrom: michael.a.kubacki@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Nov 2019 10:41:36 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,250,1571727600"; d="scan'208";a="409106323" Received: from orsmsx106.amr.corp.intel.com ([10.22.225.133]) by fmsmga005.fm.intel.com with ESMTP; 27 Nov 2019 10:41:36 -0800 Received: from orsmsx114.amr.corp.intel.com (10.22.240.10) by ORSMSX106.amr.corp.intel.com (10.22.225.133) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 27 Nov 2019 10:41:36 -0800 Received: from ORSEDG001.ED.cps.intel.com (10.7.248.4) by ORSMSX114.amr.corp.intel.com (10.22.240.10) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 27 Nov 2019 10:41:36 -0800 Received: from NAM03-DM3-obe.outbound.protection.outlook.com (104.47.41.53) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 27 Nov 2019 10:41:35 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=S5gdIqHJg8+raMpYNV94uGkL1ehqSllxKfMOxqW15YPUdbR9cZi+5p4uVmMu5wGxvSjrKnLa1K6jPa6u8kTsJtRZg4N08ayjN0uj6zRPZOpA3Rd2B98YeD8IXHHj3JWS4TVuEeG0y79PxTVeDEBpYBOzdrJ/SbAYcXwoiaAQVRAtIZ9TwasvzlipPKlsxWIm1c+QZLR02h5684sT2PXZSCRozlJ4ISUVfGNrYHUm0SaRxkyLle0hIzl5uCLoDU/KU9C1CTZwMYCj2ii1Fftc8bEh6SiCREQJgoQ5N3JscgLRJEhJG2tTlY1aOKw84G15/ckg+uAsDwOKzGpluBN5UQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=KQvAQ1R9LDjVfHQLFJ0w7sjgBLW4A2sUVfgJ5wfmmck=; b=SrAanyQlQpd0HY4591nnrcvMsh66K87BJb4/KSUeQSJkiA8X4e+FxqltyBlhFhVoBNvS/MSGSXdn9UmG1eQz5JJ36yDMA2yRtE664KlMk/bR6dAMrLCxoXxYphqERcd2KOvrWrt4JsD5Ok6WGn1k1eOWjva2zbAUmpQuB/p2yjyzFw0IOYCDWSTKzy+sKpWq5xf+cBM2SRloaqJyDbKh7ebr7DbA9ZnPhbDUvXeRb3K7ysSCGyfrhyqfzPH3IU3L5sl0SArmkpi/eI0kWmmb1lq+6E8orbBMqOpbhK7wJQELWdRiM6X1744XBlOPR2a4HghPIenn5Fghc12Vw9Zsww== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=KQvAQ1R9LDjVfHQLFJ0w7sjgBLW4A2sUVfgJ5wfmmck=; b=cSMy+oSaqb+lD1MiWMFl+0WEqZs+vOy52A2cxz4dJdQBEjVaQqDUS9ny5EwRg98zlVSwJ61o8y8rUZ5WXgW95GDAZXGj+5ffvwtWVKDpwttXyxJiZ3ePSVprOKfw4BfUDaO3/VeheqSc0mRtCsB9+pIXldHrD4mWxVAYfJTPnO8= Received: from BY5PR11MB4484.namprd11.prod.outlook.com (52.132.254.155) by BY5PR11MB4181.namprd11.prod.outlook.com (10.255.162.32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2474.22; Wed, 27 Nov 2019 18:41:34 +0000 Received: from BY5PR11MB4484.namprd11.prod.outlook.com ([fe80::a114:604b:7ca3:5420]) by BY5PR11MB4484.namprd11.prod.outlook.com ([fe80::a114:604b:7ca3:5420%7]) with mapi id 15.20.2474.023; Wed, 27 Nov 2019 18:41:34 +0000 From: "Kubacki, Michael A" To: "Chaganty, Rangasai V" , "devel@edk2.groups.io" CC: "Chiu, Chasel" , "Desimone, Nathaniel L" , "Gao, Zhichao" Subject: Re: [edk2-platforms][PATCH V1 1/2] KabylakeSiliconPkg: Remove ResetSystemLib.h override Thread-Topic: [edk2-platforms][PATCH V1 1/2] KabylakeSiliconPkg: Remove ResetSystemLib.h override Thread-Index: AQHVpO3YpMrzX5i5tkG2OXdegGw2ZKefTdZg Date: Wed, 27 Nov 2019 18:41:33 +0000 Message-ID: References: <20191127025643.32056-1-michael.a.kubacki@intel.com> <20191127025643.32056-2-michael.a.kubacki@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZmQ0NzRhYzYtY2RjYi00ODZkLWJlOWYtNGFmZGVhNWJiM2U3IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiVnVpM293MUNZV29IS0Zld1dcL0I0cnJWS0czSHdcLzliTjkzMmxpSDNpYVBYbzlaSmVEY1JqWm1aYjdCVkIrXC9BVCJ9 dlp-reaction: no-action dlp-version: 11.2.0.6 authentication-results: spf=none (sender IP is ) smtp.mailfrom=michael.a.kubacki@intel.com; x-originating-ip: [134.134.136.217] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 5d9bcb06-7263-49f2-68bb-08d773696d64 x-ms-traffictypediagnostic: BY5PR11MB4181: x-ms-exchange-purlcount: 1 x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-forefront-prvs: 023495660C x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(366004)(136003)(346002)(39860400002)(396003)(376002)(189003)(199004)(85664002)(13464003)(3846002)(6116002)(66066001)(478600001)(76176011)(71200400001)(66946007)(8676002)(66556008)(64756008)(66476007)(76116006)(102836004)(86362001)(2906002)(81166006)(81156014)(53546011)(8936002)(6506007)(107886003)(11346002)(66446008)(4326008)(14444005)(25786009)(229853002)(52536014)(256004)(6306002)(99286004)(110136005)(54906003)(446003)(186003)(6246003)(2501003)(19627235002)(9686003)(14454004)(6436002)(316002)(7736002)(74316002)(7696005)(71190400001)(305945005)(55016002)(5660300002)(33656002)(26005);DIR:OUT;SFP:1102;SCL:1;SRVR:BY5PR11MB4181;H:BY5PR11MB4484.namprd11.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;MX:1;A:1; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 320hWO30dw36o6jPWUwEaiRUpln6j2w8WhLn30ZsAUbmhPBSGXddHqxVvEfJKg0Buc6wdDPMg4wypcnKrDHk593hUJl5wOkG+WxdQeI1It42jaCFDmBDMVoz2HdutkSZpEXR5sSljnJSDCOz/93M7jVmunVp5gQk3KzI0RR/r0cSRJkXYqmGrFt6Eij9JWiS/JVYtV1nPFjlfK3qZ1yHuIj6rNnwQWv1xiAOuqJ+RHt9yeghwqELwvl55jczaoNitoMe4lt4XipxNDgepBOeQs1dwtecC+BLpKN/gLQUZDLQUefsSg6kgwZM2F854Agc13c80+I4YKRhTDAb8jr/6VVhFiDjTxiZ/jQ6oy0IAnt6NOCoS8miOxTWsSmRhzkHk3arkHkBuGxTrmBpqXdAy/fET4s84n8kSbgWSTgfLLmtBw7sW3OfpVcGD59rpRKgRTuUpWOq2URSYe1z44uaARHU30oxvphLkRNJ6T6eAgk= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 5d9bcb06-7263-49f2-68bb-08d773696d64 X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Nov 2019 18:41:33.7743 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: bfbyWdPRkuolSVD6TM//+Lq/0Gr+rIpsVvcZeAT3N1fSqKuRYzropyK958UCEdoRlM9BUHN/jjsm1deAkdKLIan0yBx415ScMBwEWr+559A= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR11MB4181 Return-Path: michael.a.kubacki@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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\Library= \ResetSystemLib.h The fact KabylakeSiliconPkg implements a MdeModulePkg library API introduce= s 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 t= he codebase is locked onto a specific stable tag in which the upstream API = is not expected to change. Zhichao rightfully added the new function defini= tion to the KabylakeSiliconPkg library class implementation because a board= package consumer would expect a ResetSystemLib library class instance to b= e compliant with the API defined in MdeModulePkg and link the ResetSystem (= ) function. The only problem was a set of circumstances that led to the dup= licate symbol definition for ResetSystem () with PchResetLib. So I view the task of eliminating the package dependency as a larger separa= te effort outside the scope of this change. But I do not agree with maintai= ning redundant local copies of edk2 APIs in packages in edk2-platforms. Thanks, Michael > -----Original Message----- > From: Chaganty, Rangasai V > Sent: Tuesday, November 26, 2019 10:43 PM > To: Kubacki, Michael A ; > devel@edk2.groups.io > Cc: Chiu, Chasel ; Desimone, Nathaniel L > ; Gao, Zhichao > Subject: RE: [edk2-platforms][PATCH V1 1/2] KabylakeSiliconPkg: Remove > ResetSystemLib.h override >=20 > 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 bet= ter > suited in packages like MdePkg. >=20 > -----Original Message----- > From: Kubacki, Michael A > Sent: Tuesday, November 26, 2019 6:57 PM > To: devel@edk2.groups.io > Cc: Chaganty, Rangasai V ; Chiu, Chasel > ; Desimone, Nathaniel L > ; Gao, Zhichao > Subject: [edk2-platforms][PATCH V1 1/2] KabylakeSiliconPkg: Remove > ResetSystemLib.h override >=20 > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D2375 >=20 > Removes a stale ResetSystemLib.h override in KabylakeSiliconPkg that does > not contain the prototype for ResetSystem () and ResetPlatformSpecific ()= . >=20 > 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. >=20 > Cc: Sai Chaganty > Cc: Chasel Chiu > Cc: Nate DeSimone > Cc: Zhichao Gao > Signed-off-by: Michael Kubacki > --- >=20 > Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeResetSystemLib/DxeResetS > ystemLib.inf | 3 +- >=20 > Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeRuntimeResetSystemLib/Dx > eRuntimeResetSystemLib.inf | 3 +- >=20 > Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PeiPchResetLi= b. > inf | 3 +- >=20 > Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiResetSy= s > temLib.inf | 3 +- >=20 > Silicon/Intel/KabylakeSiliconPkg/SampleCode/MdeModulePkg/Include/Libra > ry/ResetSystemLib.h | 62 -------------------- > 5 files changed, 8 insertions(+), 66 deletions(-) >=20 > diff --git > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeResetSystemLib/DxeRese > tSystemLib.inf > b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeResetSystemLib/DxeRese > tSystemLib.inf > index aa8877140a..46313bf35f 100644 > --- > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeResetSystemLib/DxeRese > tSystemLib.inf > +++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeResetSystemLib/Dxe > +++ ResetSystemLib.inf > @@ -1,7 +1,7 @@ > ## @file > # Component description file for Intel Ich7 Reset System Library. > # > -# Copyright (c) 2017, Intel Corporation. All rights reserved.
> +# Copyright (c) 2017 - 2019, Intel Corporation. All rights > +reserved.
> # > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -35,6 +35,7 @@ > PchCycleDecodingLib >=20 > [Packages] > MdePkg/MdePkg.dec > +MdeModulePkg/MdeModulePkg.dec > KabylakeSiliconPkg/SiPkg.dec >=20 >=20 > diff --git > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeRuntimeResetSystemLib/ > DxeRuntimeResetSystemLib.inf > b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeRuntimeResetSystemLib/ > DxeRuntimeResetSystemLib.inf > index 6b27661603..c7fad31c71 100644 > --- > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/DxeRuntimeResetSystemLib/ > 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.
> +# Copyright (c) 2017 - 2019, Intel Corporation. All rights > +reserved.
> # > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -36,6 +36,7 @@ > PchCycleDecodingLib >=20 > [Packages] > MdePkg/MdePkg.dec > +MdeModulePkg/MdeModulePkg.dec > KabylakeSiliconPkg/SiPkg.dec >=20 >=20 > diff --git > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PeiPchReset= L > ib.inf > b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PeiPchReset= L > ib.inf > index b04f4006ef..29f69078a4 100644 > --- > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PeiPchReset= L > ib.inf > +++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiPchResetLib/PeiPch > +++ ResetLib.inf > @@ -1,7 +1,7 @@ > ## @file > # Component description file for PCH Reset Lib Pei Phase # -# Copyright= (c) > 2017, Intel Corporation. All rights reserved.
> +# Copyright (c) 2017 - 2019, Intel Corporation. All rights > +reserved.
> # > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -28,6 +28,7 @@ > ResetSystemLib >=20 > [Packages] > MdePkg/MdePkg.dec > +MdeModulePkg/MdeModulePkg.dec > KabylakeSiliconPkg/SiPkg.dec >=20 > [Sources] > diff --git > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiReset= S > ystemLib.inf > b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiReset > SystemLib.inf > index 18a92a6f18..3c6ff78863 100644 > --- > a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/PeiReset= S > ystemLib.inf > +++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiResetSystemLib/Pei > +++ ResetSystemLib.inf > @@ -1,7 +1,7 @@ > ## @file > # Component description file for Intel Ich7 Reset System Library. > # > -# Copyright (c) 2017, Intel Corporation. All rights reserved.
> +# Copyright (c) 2017 - 2019, Intel Corporation. All rights > +reserved.
> # > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -32,6 +32,7 @@ > PchCycleDecodingLib >=20 > [Packages] > MdePkg/MdePkg.dec > +MdeModulePkg/MdeModulePkg.dec > KabylakeSiliconPkg/SiPkg.dec >=20 >=20 > 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.
> -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 typ= e 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 co= ld > reset. > -**/ > -VOID > -EFIAPI > -ResetCold ( > - VOID > - ); > - > -/** > - This function causes a system-wide initialization (warm reset), in whi= ch 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 wa= rm > 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 immediate= ly. > - > - If this function returns, it means that the system does not support S3 > feature. > -**/ > -VOID > -EFIAPI > -EnterS3WithImmediateWake ( > - VOID > - ); > - > -#endif > -- > 2.16.2.windows.1 >=20