* [Patch] UefiCpuPkg/S3Resume2Pei: Handle Communicate Ppi not exist issue.
@ 2017-10-12 8:33 Eric Dong
2017-10-12 8:35 ` Dong, Eric
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dong @ 2017-10-12 8:33 UTC (permalink / raw)
To: edk2-devel; +Cc: Ruiyu Ni, Jiewen Yao, Laszlo Ersek
Current code assume Communicate Ppi always existed, so it adds
ASSERT to confirm it. Ovmf platform happened not has this Ppi, so
the ASSERT been trig. This patch handle Ppi not existed case.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
index c2171cb..975bb6f 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 prepare communicate buffer.
@@ -484,29 +484,27 @@ SignalEndOfS3Resume (
}
CopyGuid (CommBuffer, &gEdkiiSmmEndOfS3ResumeProtocolGuid);
- //
- // Get needed resource
- //
Status = PeiServicesLocatePpi (
&gEfiPeiSmmCommunicationPpiGuid,
0,
NULL,
(VOID **)&SmmCommunicationPpi
);
- ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Locate Smm Communicate Ppi failed (%r)!\n", Status));
+ return Status;
+ }
- //
- // Send command
- //
Status = SmmCommunicationPpi->Communicate (
SmmCommunicationPpi,
(VOID *)CommBuffer,
&CommSize
);
- ASSERT_EFI_ERROR (Status);
-
- DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "SmmCommunicationPpi->Communicate return failure (%r)!\n", Status));
+ }
+ DEBUG ((DEBUG_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
return Status;
}
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Patch] UefiCpuPkg/S3Resume2Pei: Handle Communicate Ppi not exist issue.
2017-10-12 8:33 [Patch] UefiCpuPkg/S3Resume2Pei: Handle Communicate Ppi not exist issue Eric Dong
@ 2017-10-12 8:35 ` Dong, Eric
0 siblings, 0 replies; 6+ messages in thread
From: Dong, Eric @ 2017-10-12 8:35 UTC (permalink / raw)
To: Dong, Eric, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu, Laszlo Ersek, Yao, Jiewen
Hi all,
please ignore this patch, please this is not a full fix. Will send a new one soon.
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Eric Dong
Sent: Thursday, October 12, 2017 4:33 PM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>
Subject: [edk2] [Patch] UefiCpuPkg/S3Resume2Pei: Handle Communicate Ppi not exist issue.
Current code assume Communicate Ppi always existed, so it adds ASSERT to confirm it. Ovmf platform happened not has this Ppi, so the ASSERT been trig. This patch handle Ppi not existed case.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
index c2171cb..975bb6f 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 prepare communicate buffer.
@@ -484,29 +484,27 @@ SignalEndOfS3Resume (
}
CopyGuid (CommBuffer, &gEdkiiSmmEndOfS3ResumeProtocolGuid);
- //
- // Get needed resource
- //
Status = PeiServicesLocatePpi (
&gEfiPeiSmmCommunicationPpiGuid,
0,
NULL,
(VOID **)&SmmCommunicationPpi
);
- ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Locate Smm Communicate Ppi failed (%r)!\n", Status));
+ return Status;
+ }
- //
- // Send command
- //
Status = SmmCommunicationPpi->Communicate (
SmmCommunicationPpi,
(VOID *)CommBuffer,
&CommSize
);
- ASSERT_EFI_ERROR (Status);
-
- DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "SmmCommunicationPpi->Communicate return
+ failure (%r)!\n", Status)); }
+ DEBUG ((DEBUG_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
return Status;
}
--
2.7.0.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Patch] UefiCpuPkg/S3Resume2Pei: Handle Communicate Ppi not exist issue.
@ 2017-10-12 8:37 Eric Dong
2017-10-12 9:42 ` Laszlo Ersek
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dong @ 2017-10-12 8:37 UTC (permalink / raw)
To: edk2-devel; +Cc: Ruiyu Ni, Jiewen Yao, Laszlo Ersek
Current code assume Communicate Ppi always existed, so it adds
ASSERT to confirm it. Ovmf platform happened not has this Ppi, so
the ASSERT been trig. This patch handle Ppi not existed case.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
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 prepare communicate buffer.
@@ -484,29 +484,27 @@ SignalEndOfS3Resume (
}
CopyGuid (CommBuffer, &gEdkiiSmmEndOfS3ResumeProtocolGuid);
- //
- // Get needed resource
- //
Status = PeiServicesLocatePpi (
&gEfiPeiSmmCommunicationPpiGuid,
0,
NULL,
(VOID **)&SmmCommunicationPpi
);
- ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Locate Smm Communicate Ppi failed (%r)!\n", Status));
+ return Status;
+ }
- //
- // Send command
- //
Status = SmmCommunicationPpi->Communicate (
SmmCommunicationPpi,
(VOID *)CommBuffer,
&CommSize
);
- ASSERT_EFI_ERROR (Status);
-
- DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "SmmCommunicationPpi->Communicate return failure (%r)!\n", Status));
+ }
+ DEBUG ((DEBUG_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
return Status;
}
@@ -587,8 +585,7 @@ S3ResumeBootOs (
//
// Signal EndOfS3Resume event.
//
- Status = SignalEndOfS3Resume ();
- ASSERT_EFI_ERROR (Status);
+ SignalEndOfS3Resume ();
//
// report status code on S3 resume
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Patch] UefiCpuPkg/S3Resume2Pei: Handle Communicate Ppi not exist issue.
2017-10-12 8:37 Eric Dong
@ 2017-10-12 9:42 ` Laszlo Ersek
2017-10-12 9:55 ` Dong, Eric
0 siblings, 1 reply; 6+ messages in thread
From: Laszlo Ersek @ 2017-10-12 9:42 UTC (permalink / raw)
To: Eric Dong, edk2-devel; +Cc: Ruiyu Ni, Jiewen Yao
Hi Eric,
On 10/12/17 10:37, Eric Dong wrote:
> Current code assume Communicate Ppi always existed, so it adds
> ASSERT to confirm it. Ovmf platform happened not has this Ppi, so
> the ASSERT been trig. This patch handle Ppi not existed case.
>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
> 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 prepare communicate buffer.
> @@ -484,29 +484,27 @@ SignalEndOfS3Resume (
> }
> CopyGuid (CommBuffer, &gEdkiiSmmEndOfS3ResumeProtocolGuid);
>
> - //
> - // Get needed resource
> - //
> Status = PeiServicesLocatePpi (
> &gEfiPeiSmmCommunicationPpiGuid,
> 0,
> NULL,
> (VOID **)&SmmCommunicationPpi
> );
> - ASSERT_EFI_ERROR (Status);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "Locate Smm Communicate Ppi failed (%r)!\n", Status));
> + return Status;
> + }
>
> - //
> - // Send command
> - //
> Status = SmmCommunicationPpi->Communicate (
> SmmCommunicationPpi,
> (VOID *)CommBuffer,
> &CommSize
> );
> - ASSERT_EFI_ERROR (Status);
> -
> - DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "SmmCommunicationPpi->Communicate return failure (%r)!\n", Status));
> + }
>
> + DEBUG ((DEBUG_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
> return Status;
> }
>
> @@ -587,8 +585,7 @@ S3ResumeBootOs (
> //
> // Signal EndOfS3Resume event.
> //
> - Status = SignalEndOfS3Resume ();
> - ASSERT_EFI_ERROR (Status);
> + SignalEndOfS3Resume ();
>
> //
> // report status code on S3 resume
>
Thanks for the quick patch!
Tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
I used OVMF IA32 and IA32X64 to test the patch (with SMM).
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
Locate Smm Communicate Ppi failed (Not Found)!
which seems strange for a firmware that does not include any SMM support.
This tells me that the original ASSERT (which is removed by this patch)
would have triggered in the OVMF X64 build (*without* SMM) -- I didn't
try that originally, but in retrospect that's what I believe.
So, my question is; is this intentional? If I remember correctly, other
parts of S3Resume2Pei distinguish explicitly between "no-SMM" and "SMM".
If this new facility is meaningful only for SMM, then should we avoid
even the PeiServicesLocatePpi() call in SignalEndOfS3Resume(), in case
the platform lacks SMM support?
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Patch] UefiCpuPkg/S3Resume2Pei: Handle Communicate Ppi not exist issue.
2017-10-12 9:42 ` Laszlo Ersek
@ 2017-10-12 9:55 ` Dong, Eric
2017-10-24 9:23 ` Zeng, Star
0 siblings, 1 reply; 6+ messages in thread
From: Dong, Eric @ 2017-10-12 9:55 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu, Yao, Jiewen
Hi Laszlo,
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, October 12, 2017 5:43 PM
> To: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: Re: [edk2] [Patch] UefiCpuPkg/S3Resume2Pei: Handle
> Communicate Ppi not exist issue.
>
> Hi Eric,
>
> On 10/12/17 10:37, Eric Dong wrote:
> > Current code assume Communicate Ppi always existed, so it adds ASSERT
> > to confirm it. Ovmf platform happened not has this Ppi, so the ASSERT
> > been trig. This patch handle Ppi not existed case.
> >
> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Dong <eric.dong@intel.com>
> > ---
> > 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 prepare
> communicate buffer.
> > @@ -484,29 +484,27 @@ SignalEndOfS3Resume (
> > }
> > CopyGuid (CommBuffer, &gEdkiiSmmEndOfS3ResumeProtocolGuid);
> >
> > - //
> > - // Get needed resource
> > - //
> > Status = PeiServicesLocatePpi (
> > &gEfiPeiSmmCommunicationPpiGuid,
> > 0,
> > NULL,
> > (VOID **)&SmmCommunicationPpi
> > );
> > - ASSERT_EFI_ERROR (Status);
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((DEBUG_ERROR, "Locate Smm Communicate Ppi failed (%r)!\n",
> Status));
> > + return Status;
> > + }
> >
> > - //
> > - // Send command
> > - //
> > Status = SmmCommunicationPpi->Communicate (
> > SmmCommunicationPpi,
> > (VOID *)CommBuffer,
> > &CommSize
> > );
> > - ASSERT_EFI_ERROR (Status);
> > -
> > - DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((DEBUG_ERROR, "SmmCommunicationPpi->Communicate
> return
> > + failure (%r)!\n", Status)); }
> >
> > + DEBUG ((DEBUG_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
> > return Status;
> > }
> >
> > @@ -587,8 +585,7 @@ S3ResumeBootOs (
> > //
> > // Signal EndOfS3Resume event.
> > //
> > - Status = SignalEndOfS3Resume ();
> > - ASSERT_EFI_ERROR (Status);
> > + SignalEndOfS3Resume ();
> >
> > //
> > // report status code on S3 resume
> >
>
> Thanks for the quick patch!
>
> Tested-by: Laszlo Ersek <lersek@redhat.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
> I used OVMF IA32 and IA32X64 to test the patch (with SMM).
>
>
> 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
>
> Locate Smm Communicate Ppi failed (Not Found)!
>
> which seems strange for a firmware that does not include any SMM support.
>
> This tells me that the original ASSERT (which is removed by this patch) would
> have triggered in the OVMF X64 build (*without* SMM) -- I didn't try that
> originally, but in retrospect that's what I believe.
>
> So, my question is; is this intentional? If I remember correctly, other parts of
> S3Resume2Pei distinguish explicitly between "no-SMM" and "SMM".
This is an expect behavior. This code is executive right after EndOfPei point, not in SMM environment.
It just tries to use Communicate Ppi to send EndOfS3Resume signal to SmmCore.
>
> If this new facility is meaningful only for SMM, then should we avoid even
> the PeiServicesLocatePpi() call in SignalEndOfS3Resume(), in case the
> platform lacks SMM support?
>
> Thanks,
> Laszlo
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Patch] UefiCpuPkg/S3Resume2Pei: Handle Communicate Ppi not exist issue.
2017-10-12 9:55 ` Dong, Eric
@ 2017-10-24 9:23 ` Zeng, Star
0 siblings, 0 replies; 6+ messages in thread
From: Zeng, Star @ 2017-10-24 9:23 UTC (permalink / raw)
To: Dong, Eric, Laszlo Ersek, edk2-devel@lists.01.org
Cc: Ni, Ruiyu, Yao, Jiewen, Zeng, Star
//
// Attempt to use content from SMRAM first
//
GuidHob = GetFirstGuidHob (&gEfiAcpiVariableGuid);
if (GuidHob != 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 <lersek@redhat.com>; edk2-devel@lists.01.org
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
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 <eric.dong@intel.com>; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: Re: [edk2] [Patch] UefiCpuPkg/S3Resume2Pei: Handle
> Communicate Ppi not exist issue.
>
> Hi Eric,
>
> On 10/12/17 10:37, Eric Dong wrote:
> > Current code assume Communicate Ppi always existed, so it adds
> > ASSERT to confirm it. Ovmf platform happened not has this Ppi, so
> > the ASSERT been trig. This patch handle Ppi not existed case.
> >
> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Dong <eric.dong@intel.com>
> > ---
> > 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
> > prepare
> communicate buffer.
> > @@ -484,29 +484,27 @@ SignalEndOfS3Resume (
> > }
> > CopyGuid (CommBuffer, &gEdkiiSmmEndOfS3ResumeProtocolGuid);
> >
> > - //
> > - // Get needed resource
> > - //
> > Status = PeiServicesLocatePpi (
> > &gEfiPeiSmmCommunicationPpiGuid,
> > 0,
> > NULL,
> > (VOID **)&SmmCommunicationPpi
> > );
> > - ASSERT_EFI_ERROR (Status);
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((DEBUG_ERROR, "Locate Smm Communicate Ppi failed
> > + (%r)!\n",
> Status));
> > + return Status;
> > + }
> >
> > - //
> > - // Send command
> > - //
> > Status = SmmCommunicationPpi->Communicate (
> > SmmCommunicationPpi,
> > (VOID *)CommBuffer,
> > &CommSize
> > );
> > - ASSERT_EFI_ERROR (Status);
> > -
> > - DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Exit (%r)\n",
> > Status));
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((DEBUG_ERROR, "SmmCommunicationPpi->Communicate
> return
> > + failure (%r)!\n", Status)); }
> >
> > + DEBUG ((DEBUG_INFO, "SignalEndOfS3Resume - Exit (%r)\n",
> > + Status));
> > return Status;
> > }
> >
> > @@ -587,8 +585,7 @@ S3ResumeBootOs (
> > //
> > // Signal EndOfS3Resume event.
> > //
> > - Status = SignalEndOfS3Resume ();
> > - ASSERT_EFI_ERROR (Status);
> > + SignalEndOfS3Resume ();
> >
> > //
> > // report status code on S3 resume
> >
>
> Thanks for the quick patch!
>
> Tested-by: Laszlo Ersek <lersek@redhat.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
> I used OVMF IA32 and IA32X64 to test the patch (with SMM).
>
>
> 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
>
> Locate Smm Communicate Ppi failed (Not Found)!
>
> which seems strange for a firmware that does not include any SMM support.
>
> This tells me that the original ASSERT (which is removed by this
> patch) would have triggered in the OVMF X64 build (*without* SMM) -- I
> didn't try that originally, but in retrospect that's what I believe.
>
> So, my question is; is this intentional? If I remember correctly,
> other parts of S3Resume2Pei distinguish explicitly between "no-SMM" and "SMM".
This is an expect behavior. This code is executive right after EndOfPei point, not in SMM environment.
It just tries to use Communicate Ppi to send EndOfS3Resume signal to SmmCore.
>
> If this new facility is meaningful only for SMM, then should we avoid
> even the PeiServicesLocatePpi() call in SignalEndOfS3Resume(), in case
> the platform lacks SMM support?
>
> Thanks,
> Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-24 9:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-12 8:33 [Patch] UefiCpuPkg/S3Resume2Pei: Handle Communicate Ppi not exist issue Eric Dong
2017-10-12 8:35 ` Dong, Eric
-- strict thread matches above, loose matches on Subject: below --
2017-10-12 8:37 Eric Dong
2017-10-12 9:42 ` Laszlo Ersek
2017-10-12 9:55 ` Dong, Eric
2017-10-24 9:23 ` Zeng, Star
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox