From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by ml01.01.org (Postfix) with ESMTP id 785FE1A1E02 for ; Thu, 18 Aug 2016 19:00:18 -0700 (PDT) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP; 18 Aug 2016 19:00:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,542,1464678000"; d="scan'208";a="1028072637" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga001.fm.intel.com with ESMTP; 18 Aug 2016 19:00:18 -0700 Received: from fmsmsx155.amr.corp.intel.com (10.18.116.71) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 18 Aug 2016 19:00:17 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX155.amr.corp.intel.com (10.18.116.71) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 18 Aug 2016 19:00:17 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.147]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.107]) with mapi id 14.03.0248.002; Fri, 19 Aug 2016 10:00:12 +0800 From: "Fan, Jeff" To: Laszlo Ersek , "edk2-devel@ml01.01.org" CC: "Kinney, Michael D" , "Tian, Feng" Thread-Topic: [edk2] [Patch v5 48/48] UefiCpuPkg/PiSmmCpuDxeSmm: Add gEfiVariableArchProtocolGuid dependency Thread-Index: AQHR7Jy4Gz8Pd1vRVUq8pvPSUJUViKBPEBCAgACPpPA= Date: Fri, 19 Aug 2016 02:00:11 +0000 Message-ID: <542CF652F8836A4AB8DBFAAD40ED192A143D9CFD@shsmsx102.ccr.corp.intel.com> References: <1470128388-17960-1-git-send-email-jeff.fan@intel.com> <1470128388-17960-49-git-send-email-jeff.fan@intel.com> <4f61b2b4-eeb4-8435-412f-20848347c88e@redhat.com> In-Reply-To: <4f61b2b4-eeb4-8435-412f-20848347c88e@redhat.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZjc1NTU1NDQtNTFjNC00MDYxLTg1NDQtMmM4N2Y1NjJjMDdlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IjhSN0IxZ1F2Sk5DUURPemNBV3dOc3E3QnRRTFRpZUkrcnFvZ042QTlrREk9In0= x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [Patch v5 48/48] UefiCpuPkg/PiSmmCpuDxeSmm: Add gEfiVariableArchProtocolGuid dependency X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Aug 2016 02:00:18 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Laszlo, I could revert this patch firstly. Could you please dig out the OVMF to add= ress the potential issue, then I could re-commit it for those platforms req= uired this patch. Thanks! Jeff -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Lasz= lo Ersek Sent: Friday, August 19, 2016 9:19 AM To: Fan, Jeff; edk2-devel@ml01.01.org Cc: Kinney, Michael D; Tian, Feng Subject: Re: [edk2] [Patch v5 48/48] UefiCpuPkg/PiSmmCpuDxeSmm: Add gEfiVar= iableArchProtocolGuid dependency On 08/02/16 10:59, Jeff Fan wrote: > PiSmmCpuDxeSmm driver's entry point will get some PCDs supported dynamic = type. > In case those PCDs are set as DynamicHii type in platform DSC File, it=20 > implies that EFI Variable Arch protocol is required. >=20 > This fix is to add gEfiVariableArchProtocolGuid dependency on=20 > PiSmmCpuDxeSmm driver to make sure those DynamicHii PCDs could be read co= rrectly. >=20 > Cc: Michael Kinney > Cc: Feng Tian > Cc: Giri P Mudusuru > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jeff Fan > Reviewed-by: Giri P Mudusuru > --- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) >=20 > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf=20 > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > index d7e6e07..83e3c55 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > @@ -4,7 +4,7 @@ > # This SMM driver performs SMM initialization, deploy SMM Entry=20 > Vector, # provides CPU specific services in SMM. > # > -# Copyright (c) 2009 - 2015, Intel Corporation. All rights=20 > reserved.
> +# Copyright (c) 2009 - 2016, Intel Corporation. All rights=20 > +reserved.
> # > # This program and the accompanying materials # are licensed and=20 > made available under the terms and conditions of the BSD License @@=20 > -155,7 +155,7 @@ [Pcd] > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode ## CO= NSUMES > =20 > [Depex] > - gEfiMpServiceProtocolGuid > + gEfiMpServiceProtocolGuid AND gEfiVariableArchProtocolGuid > =20 > [UserExtensions.TianoCore."ExtraFiles"] > PiSmmCpuDxeSmmExtra.uni >=20 This patch (commit 7503cd70fb86) breaks OVMF. From the OVMF log: Loading SMM driver at 0x0007FFDA000 EntryPoint=3D0x0007FFDA271 FvbServicesS= mm.efi QEMU Flash: Attempting flash detection at FFE00010 QemuFlashDetected= =3D> FD behaves as ROM QemuFlashDetected =3D> No ASSERT OvmfPkg/QemuFlashF= vbServicesRuntimeDxe/QemuFlash.c(248): !_gPcd_FixedAtBuild_PcdSmmSmramRequi= re This symptom means that the SMM build of the QemuFlashFvbServicesRuntimeDxe= driver is not actually running in SMM when it tries to access the pflash c= hip at startup. Therefore QEMU prevents the access, and then OVMF gives up. Here's the bisection log: git bisect start # good: [de74668f5ea713b7e91e01318f0d15d2bf0effce] MdeModulePkg/PeiCore: Fi= x ConverSinglePpiPointer () typo. git bisect good de74668f5ea713b7e91e01318f0d15d2bf0effce # bad: [7822a1d91d53e80525f571183a24d54488f5437f] NetworkPkg/IpSecDxe: Fix = wrong IKE header "FLAG" update git bisect bad 7822a1d91d53e80525f571183a24d= 54488f5437f # good: [8a2d564b2a811b8dbc85f90e14a67ae4ade21201] UefiCpuPkg/MpInitLib: So= rt processor by ascending order of APIC ID git bisect good 8a2d564b2a811b8d= bc85f90e14a67ae4ade21201 # good: [7fadaacd50d716e8e054a94c20db56cca98e961e] UefiCpuPkg/CpuDxe: Consu= me MpInitLib to produce CPU MP Protocol services git bisect good 7fadaacd50= d716e8e054a94c20db56cca98e961e # bad: [a012df5ec643a0c08c2b723a02919a5c9373ca74] PcAtChipsetPkg AcpiTimerL= ib: Wait 363 ACPI timer counts to get TSC Freq git bisect bad a012df5ec643a= 0c08c2b723a02919a5c9373ca74 # good: [51d4779d7bfb74bfdbb0e196846de78567127348] MdePkg/MpService.h: Fixe= d typo in function header to match PI spec git bisect good 51d4779d7bfb74bf= dbb0e196846de78567127348 # good: [f3b91fa04adea2389c5a6d0fbd9a584d149bae09] UefiCpuPkg/CpuDxe: Fixed= typo in function header to match PI spec git bisect good f3b91fa04adea2389= c5a6d0fbd9a584d149bae09 # bad: [7503cd70fb864a5663edb121c9b2488b4c69e7f5] UefiCpuPkg/PiSmmCpuDxeSmm= : Add gEfiVariableArchProtocolGuid dependency git bisect bad 7503cd70fb864a= 5663edb121c9b2488b4c69e7f5 # first bad commit: [7503cd70fb864a5663edb121c9b2488b4c69e7f5] UefiCpuPkg/P= iSmmCpuDxeSmm: Add gEfiVariableArchProtocolGuid dependency I see that this patch appeared in the final, v5 version of the series, as t= he last patch in the series. I never got around testing v5. I request that = we please revert it for now, and then we investigate the problem. >>From the time we worked on SMM enablement for OVMF, I recall that pulling i= n the SMM variable driver stack was tricky. The three layers (Firmware Volu= me Block (2), Fault Tolerant Write, and Variable) have apparent / implicit = dependencies between them, but these are not actually expressed in DepExes= . I don't know / don't remember why that is the case, but I recall that I h= ad to fiddle with their inclusion order in the FDF files. Following the in= clusion order of the preexistent, SMM-less variable driver stack made it al= l work, if I remember correctly. I don't know why those depexes are not spelled out explicitly in those driv= ers, but at this point I think that the new DepEx on PiSmmCpuDxeSmm.inf ups= ets the dispatch order, and things break. I fully agree that this should ho= pefully be fixed by spelling out all DepExes explicitly everywhere, and I c= an also imagine it is actually a bug in OVMF somewhere, but for now, until = we figure it out, can we please revert the patch? The commit carries Mike's Tested-by -- I wonder what the differences are be= tween Mike's test platform and OVMF. Thank you, Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel