From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.120; helo=mga04.intel.com; envelope-from=star.zeng@intel.com; receiver=edk2-devel@lists.01.org Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 36074202E59C2 for ; Tue, 24 Oct 2017 02:19:24 -0700 (PDT) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 24 Oct 2017 02:23:07 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,427,1503385200"; d="scan'208";a="913101580" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by FMSMGA003.fm.intel.com with ESMTP; 24 Oct 2017 02:23:07 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 24 Oct 2017 02:23:06 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.175]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.159]) with mapi id 14.03.0319.002; Tue, 24 Oct 2017 17:23:04 +0800 From: "Zeng, Star" To: "Dong, Eric" , Laszlo Ersek , "edk2-devel@lists.01.org" CC: "Ni, Ruiyu" , "Yao, Jiewen" , "Zeng, Star" Thread-Topic: [edk2] [Patch] UefiCpuPkg/S3Resume2Pei: Handle Communicate Ppi not exist issue. Thread-Index: AQHTQzV+XJiuTUsI8U2nTP8/M//opaLfcGmAgAADagCAE1iZgA== Date: Tue, 24 Oct 2017 09:23:04 +0000 Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B9AD2F7@shsmsx102.ccr.corp.intel.com> References: <1507797478-13028-1-git-send-email-eric.dong@intel.com> <5842b1ee-80f3-1d66-0b2d-38e392dd5284@redhat.com> In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [Patch] UefiCpuPkg/S3Resume2Pei: Handle Communicate Ppi not exist issue. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Oct 2017 09:19:24 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable // // Attempt to use content from SMRAM first // GuidHob =3D GetFirstGuidHob (&gEfiAcpiVariableGuid); if (GuidHob !=3D NULL) { The code may could use this condition above to skip SignalEndOfS3Resume(). We can see two places already in S3Resume.c. Thanks, Star -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Dong= , Eric Sent: Thursday, October 12, 2017 5:55 PM To: Laszlo Ersek ; edk2-devel@lists.01.org Cc: Ni, Ruiyu ; Yao, Jiewen Subject: Re: [edk2] [Patch] UefiCpuPkg/S3Resume2Pei: Handle Communicate Ppi= not exist issue. Hi Laszlo, > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Thursday, October 12, 2017 5:43 PM > To: Dong, Eric ; edk2-devel@lists.01.org > Cc: Ni, Ruiyu ; Yao, Jiewen > Subject: Re: [edk2] [Patch] UefiCpuPkg/S3Resume2Pei: Handle=20 > Communicate Ppi not exist issue. >=20 > Hi Eric, >=20 > On 10/12/17 10:37, Eric Dong wrote: > > Current code assume Communicate Ppi always existed, so it adds=20 > > ASSERT to confirm it. Ovmf platform happened not has this Ppi, so=20 > > the ASSERT been trig. This patch handle Ppi not existed case. > > > > Cc: Ruiyu Ni > > Cc: Jiewen Yao > > Cc: Laszlo Ersek > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Eric Dong > > --- > > UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 23 > > ++++++++++------------- > > 1 file changed, 10 insertions(+), 13 deletions(-) > > > > diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c > > b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c > > index c2171cb..e0c2d36 100644 > > --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c > > +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c > > @@ -465,7 +465,7 @@ SignalEndOfS3Resume ( > > SMM_COMMUNICATE_HEADER_64 Header64; > > VOID *CommBuffer; > > > > - DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Enter\n")); > > + DEBUG ((DEBUG_INFO, "SignalEndOfS3Resume - Enter\n")); > > > > // > > // This buffer consumed in DXE phase, so base on DXE mode to=20 > > prepare > communicate buffer. > > @@ -484,29 +484,27 @@ SignalEndOfS3Resume ( > > } > > CopyGuid (CommBuffer, &gEdkiiSmmEndOfS3ResumeProtocolGuid); > > > > - // > > - // Get needed resource > > - // > > Status =3D PeiServicesLocatePpi ( > > &gEfiPeiSmmCommunicationPpiGuid, > > 0, > > NULL, > > (VOID **)&SmmCommunicationPpi > > ); > > - ASSERT_EFI_ERROR (Status); > > + if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_ERROR, "Locate Smm Communicate Ppi failed=20 > > + (%r)!\n", > Status)); > > + return Status; > > + } > > > > - // > > - // Send command > > - // > > Status =3D SmmCommunicationPpi->Communicate ( > > SmmCommunicationPpi, > > (VOID *)CommBuffer, > > &CommSize > > ); > > - ASSERT_EFI_ERROR (Status); > > - > > - DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Exit (%r)\n",=20 > > Status)); > > + if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_ERROR, "SmmCommunicationPpi->Communicate > return > > + failure (%r)!\n", Status)); } > > > > + DEBUG ((DEBUG_INFO, "SignalEndOfS3Resume - Exit (%r)\n",=20 > > + Status)); > > return Status; > > } > > > > @@ -587,8 +585,7 @@ S3ResumeBootOs ( > > // > > // Signal EndOfS3Resume event. > > // > > - Status =3D SignalEndOfS3Resume (); > > - ASSERT_EFI_ERROR (Status); > > + SignalEndOfS3Resume (); > > > > // > > // report status code on S3 resume > > >=20 > Thanks for the quick patch! >=20 > Tested-by: Laszlo Ersek > Reviewed-by: Laszlo Ersek >=20 > I used OVMF IA32 and IA32X64 to test the patch (with SMM). >=20 >=20 > I also checked S3 with the OVMF X64 build (without SMM). It works too. > But, interestingly, even in that build, I'm getting a message >=20 > Locate Smm Communicate Ppi failed (Not Found)! >=20 > which seems strange for a firmware that does not include any SMM support. >=20 > This tells me that the original ASSERT (which is removed by this=20 > patch) would have triggered in the OVMF X64 build (*without* SMM) -- I=20 > didn't try that originally, but in retrospect that's what I believe. >=20 > So, my question is; is this intentional? If I remember correctly,=20 > other parts of S3Resume2Pei distinguish explicitly between "no-SMM" and "= SMM". This is an expect behavior. This code is executive right after EndOfPei poi= nt, not in SMM environment. It just tries to use Communicate Ppi to send EndOfS3Resume signal to SmmCor= e. >=20 > If this new facility is meaningful only for SMM, then should we avoid=20 > even the PeiServicesLocatePpi() call in SignalEndOfS3Resume(), in case=20 > the platform lacks SMM support? >=20 > Thanks, > Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel