* [Patch 1/3] MdeModulePkg/SmmEndOfS3Resume.h: Add new protocol definition.
2017-10-11 8:22 [Patch v3 0/3] Add SmmEndOfS3Resume event Eric Dong
@ 2017-10-11 8:22 ` Eric Dong
2017-10-11 8:22 ` [Patch v3 2/3] UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore Eric Dong
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Eric Dong @ 2017-10-11 8:22 UTC (permalink / raw)
To: edk2-devel; +Cc: Ruiyu Ni, Jiewen Yao
Add gEdkiiSmmEndOfS3ResumeProtocolGuid which used by SmmCore to
notify smm drives that S3 resume has finished.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h | 26 ++++++++++++++++++++++++
MdeModulePkg/MdeModulePkg.dec | 3 +++
2 files changed, 29 insertions(+)
create mode 100644 MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h
diff --git a/MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h b/MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h
new file mode 100644
index 0000000..9716f6a
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h
@@ -0,0 +1,26 @@
+/** @file
+ This Protocol will be installed at the end of S3 resume phase in SMM environment.
+ It allows for smm drivers to hook this point and do the requried tasks.
+
+ Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __SMM_END_OF_S3_RESUME_H__
+#define __SMM_END_OF_S3_RESUME_H__
+
+#define EDKII_SMM_END_OF_S3_RESUME_PROTOCOL_GUID \
+ { \
+ 0x96f5296d, 0x05f7, 0x4f3c, {0x84, 0x67, 0xe4, 0x56, 0x89, 0x0e, 0x0c, 0xb5 } \
+ }
+
+extern EFI_GUID gEdkiiSmmEndOfS3ResumeProtocolGuid;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index a3c0633..216e4f9 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -556,6 +556,9 @@
## Include/Protocol/IoMmu.h
gEdkiiIoMmuProtocolGuid = { 0x4e939de9, 0xd948, 0x4b0f, { 0x88, 0xed, 0xe6, 0xe1, 0xce, 0x51, 0x7c, 0x1e } }
+ ## Include/Protocol/SmmEndofS3Resume.h
+ gEdkiiSmmEndOfS3ResumeProtocolGuid = { 0x96f5296d, 0x05f7, 0x4f3c, {0x84, 0x67, 0xe4, 0x56, 0x89, 0x0e, 0x0c, 0xb5 } }
+
#
# [Error.gEfiMdeModulePkgTokenSpaceGuid]
# 0x80000001 | Invalid value provided.
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Patch v3 2/3] UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.
2017-10-11 8:22 [Patch v3 0/3] Add SmmEndOfS3Resume event Eric Dong
2017-10-11 8:22 ` [Patch 1/3] MdeModulePkg/SmmEndOfS3Resume.h: Add new protocol definition Eric Dong
@ 2017-10-11 8:22 ` Eric Dong
2017-10-12 8:01 ` Laszlo Ersek
2017-10-11 8:22 ` [Patch v3 3/3] MdeModulePkg/PiSmmCore: Install Protocol when S3 resume finished Eric Dong
2017-10-12 2:04 ` [Patch v3 0/3] Add SmmEndOfS3Resume event Yao, Jiewen
3 siblings, 1 reply; 8+ messages in thread
From: Eric Dong @ 2017-10-11 8:22 UTC (permalink / raw)
To: edk2-devel; +Cc: Ruiyu Ni, Jiewen Yao
Driver will send S3 resume finished event to SmmCore through communicate
buffer after it signals EndOfPei event.
V2 Changes:
1. Change structures name to avoid they start with EFI_.
2. Base on DXE phase bits to provide communication buffer, current implement
check both PEI and DXE phase.
V3 Changes:
1. Change structure name for better understanding.
2. Enhance communication buffer calculate logic to more accurate.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 87 ++++++++++++++++++++++
.../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf | 4 +
2 files changed, 91 insertions(+)
diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
index e53ed21..c2171cb 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
@@ -28,6 +28,9 @@
#include <Ppi/SmmAccess.h>
#include <Ppi/PostBootScriptTable.h>
#include <Ppi/EndOfPeiPhase.h>
+#include <Ppi/SmmCommunication.h>
+
+#include <Protocol/SmmEndOfS3Resume.h>
#include <Library/DebugLib.h>
#include <Library/BaseLib.h>
@@ -151,6 +154,22 @@ typedef union {
UINT64 Uint64;
} PAGE_TABLE_1G_ENTRY;
+//
+// Define two type of smm communicate headers.
+// One for 32 bits PEI + 64 bits DXE, the other for 32 bits PEI + 32 bits DXE case.
+//
+typedef struct {
+ EFI_GUID HeaderGuid;
+ UINT32 MessageLength;
+ UINT8 Data[1];
+} SMM_COMMUNICATE_HEADER_32;
+
+typedef struct {
+ EFI_GUID HeaderGuid;
+ UINT64 MessageLength;
+ UINT8 Data[1];
+} SMM_COMMUNICATE_HEADER_64;
+
#pragma pack()
//
@@ -430,6 +449,68 @@ IsLongModeWakingVector (
}
/**
+ Send EndOfS3Resume event to SmmCore through communication buffer way.
+
+ @retval EFI_SUCCESS Return send the event success.
+**/
+EFI_STATUS
+SignalEndOfS3Resume (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ EFI_PEI_SMM_COMMUNICATION_PPI *SmmCommunicationPpi;
+ UINTN CommSize;
+ SMM_COMMUNICATE_HEADER_32 Header32;
+ SMM_COMMUNICATE_HEADER_64 Header64;
+ VOID *CommBuffer;
+
+ DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Enter\n"));
+
+ //
+ // This buffer consumed in DXE phase, so base on DXE mode to prepare communicate buffer.
+ // Detect whether DXE is 64 bits mode.
+ // if (sizeof(UINTN) == sizeof(UINT64), PEI already 64 bits, assume DXE also 64 bits.
+ // or (FeaturePcdGet (PcdDxeIplSwitchToLongMode)), Dxe will switch to 64 bits.
+ //
+ if ((sizeof(UINTN) == sizeof(UINT64)) || (FeaturePcdGet (PcdDxeIplSwitchToLongMode))) {
+ CommBuffer = &Header64;
+ Header64.MessageLength = 0;
+ CommSize = OFFSET_OF (SMM_COMMUNICATE_HEADER_64, Data);
+ } else {
+ CommBuffer = &Header32;
+ Header32.MessageLength = 0;
+ CommSize = OFFSET_OF (SMM_COMMUNICATE_HEADER_32, Data);
+ }
+ CopyGuid (CommBuffer, &gEdkiiSmmEndOfS3ResumeProtocolGuid);
+
+ //
+ // Get needed resource
+ //
+ Status = PeiServicesLocatePpi (
+ &gEfiPeiSmmCommunicationPpiGuid,
+ 0,
+ NULL,
+ (VOID **)&SmmCommunicationPpi
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ //
+ // Send command
+ //
+ Status = SmmCommunicationPpi->Communicate (
+ SmmCommunicationPpi,
+ (VOID *)CommBuffer,
+ &CommSize
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
+
+ return Status;
+}
+
+/**
Jump to OS waking vector.
The function will install boot script done PPI, report S3 resume status code, and then jump to OS waking vector.
@@ -504,6 +585,12 @@ S3ResumeBootOs (
ASSERT_EFI_ERROR (Status);
//
+ // Signal EndOfS3Resume event.
+ //
+ Status = SignalEndOfS3Resume ();
+ ASSERT_EFI_ERROR (Status);
+
+ //
// report status code on S3 resume
//
REPORT_STATUS_CODE (EFI_PROGRESS_CODE, EFI_SOFTWARE_PEI_MODULE | EFI_SW_PEI_PC_OS_WAKE);
diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
index d514523..943f114 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
@@ -85,6 +85,10 @@
gPeiSmmAccessPpiGuid ## SOMETIMES_CONSUMES
gPeiPostScriptTablePpiGuid ## SOMETIMES_PRODUCES
gEfiEndOfPeiSignalPpiGuid ## SOMETIMES_PRODUCES
+ gEfiPeiSmmCommunicationPpiGuid ## SOMETIMES_CONSUMES
+
+[Protocols]
+ gEdkiiSmmEndOfS3ResumeProtocolGuid ## SOMETIMES_CONSUMES
[FeaturePcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode ## CONSUMES
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [Patch v3 2/3] UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.
2017-10-11 8:22 ` [Patch v3 2/3] UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore Eric Dong
@ 2017-10-12 8:01 ` Laszlo Ersek
2017-10-12 8:14 ` Dong, Eric
0 siblings, 1 reply; 8+ messages in thread
From: Laszlo Ersek @ 2017-10-12 8:01 UTC (permalink / raw)
To: Eric Dong; +Cc: edk2-devel, Ruiyu Ni, Jiewen Yao
Hi,
I'm sorry that I couldn't comment on this patch in time.
However, I must observe that version 1 of the patch,
http://mid.mail-archive.com/1507688554-10264-3-git-send-email-eric.dong@intel.com
was sent out at 04:22 AM on 11 Oct, in my time zone; and version 3 of
the patch was committed at 04:17 AM on 12 Oct in my time zone.
That's not a lot of time for reviewers to comment -- I was very busy
with other stuff yesterday. I even set up an auto-reply about that,
asking for a bit of patience.
Of course the auto-reply didn't help, because I wasn't CC'd on the patch
in the first place; despite my earlier request to be CC'd on all SMM-
and/or S3-related patches.
So, after this introduction:
On 10/11/17 10:22, Eric Dong wrote:
> Driver will send S3 resume finished event to SmmCore through communicate
> buffer after it signals EndOfPei event.
>
> V2 Changes:
> 1. Change structures name to avoid they start with EFI_.
> 2. Base on DXE phase bits to provide communication buffer, current implement
> check both PEI and DXE phase.
>
> V3 Changes:
> 1. Change structure name for better understanding.
> 2. Enhance communication buffer calculate logic to more accurate.
>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
> UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 87 ++++++++++++++++++++++
> .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf | 4 +
> 2 files changed, 91 insertions(+)
>
> diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> index e53ed21..c2171cb 100644
> --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> @@ -28,6 +28,9 @@
> #include <Ppi/SmmAccess.h>
> #include <Ppi/PostBootScriptTable.h>
> #include <Ppi/EndOfPeiPhase.h>
> +#include <Ppi/SmmCommunication.h>
> +
> +#include <Protocol/SmmEndOfS3Resume.h>
>
> #include <Library/DebugLib.h>
> #include <Library/BaseLib.h>
> @@ -151,6 +154,22 @@ typedef union {
> UINT64 Uint64;
> } PAGE_TABLE_1G_ENTRY;
>
> +//
> +// Define two type of smm communicate headers.
> +// One for 32 bits PEI + 64 bits DXE, the other for 32 bits PEI + 32 bits DXE case.
> +//
> +typedef struct {
> + EFI_GUID HeaderGuid;
> + UINT32 MessageLength;
> + UINT8 Data[1];
> +} SMM_COMMUNICATE_HEADER_32;
> +
> +typedef struct {
> + EFI_GUID HeaderGuid;
> + UINT64 MessageLength;
> + UINT8 Data[1];
> +} SMM_COMMUNICATE_HEADER_64;
> +
> #pragma pack()
>
> //
> @@ -430,6 +449,68 @@ IsLongModeWakingVector (
> }
>
> /**
> + Send EndOfS3Resume event to SmmCore through communication buffer way.
> +
> + @retval EFI_SUCCESS Return send the event success.
> +**/
> +EFI_STATUS
> +SignalEndOfS3Resume (
> + VOID
> + )
> +{
> + EFI_STATUS Status;
> + EFI_PEI_SMM_COMMUNICATION_PPI *SmmCommunicationPpi;
> + UINTN CommSize;
> + SMM_COMMUNICATE_HEADER_32 Header32;
> + SMM_COMMUNICATE_HEADER_64 Header64;
> + VOID *CommBuffer;
> +
> + DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Enter\n"));
> +
> + //
> + // This buffer consumed in DXE phase, so base on DXE mode to prepare communicate buffer.
> + // Detect whether DXE is 64 bits mode.
> + // if (sizeof(UINTN) == sizeof(UINT64), PEI already 64 bits, assume DXE also 64 bits.
> + // or (FeaturePcdGet (PcdDxeIplSwitchToLongMode)), Dxe will switch to 64 bits.
> + //
> + if ((sizeof(UINTN) == sizeof(UINT64)) || (FeaturePcdGet (PcdDxeIplSwitchToLongMode))) {
> + CommBuffer = &Header64;
> + Header64.MessageLength = 0;
> + CommSize = OFFSET_OF (SMM_COMMUNICATE_HEADER_64, Data);
> + } else {
> + CommBuffer = &Header32;
> + Header32.MessageLength = 0;
> + CommSize = OFFSET_OF (SMM_COMMUNICATE_HEADER_32, Data);
> + }
> + CopyGuid (CommBuffer, &gEdkiiSmmEndOfS3ResumeProtocolGuid);
> +
> + //
> + // Get needed resource
> + //
> + Status = PeiServicesLocatePpi (
> + &gEfiPeiSmmCommunicationPpiGuid,
> + 0,
> + NULL,
> + (VOID **)&SmmCommunicationPpi
> + );
> + ASSERT_EFI_ERROR (Status);
This patch breaks S3 resume with OVMF+SMM. OVMF does not have or install
an EFI_PEI_SMM_COMMUNICATION_PPI. We discussed this ~2 years ago, when
SMM was being added to KVM, QEMU, and OVMF.
OVMF would have needed EFI_PEI_SMM_COMMUNICATION_PPI only for
SmmLockBoxPeiLib. But we agreed that SmmLockBoxPeiLib would work without
this PPI; exactly the same as if the PPI existed, but Communicate()
returned EFI_NOT_STARTED.
Namely, if RestoreLockBox() failed to locate
EFI_PEI_SMM_COMMUNICATION_PPI, then InternalRestoreLockBoxFromSmram()
would be called instead, which only needed PEI_SMM_ACCESS_PPI. And OVMF
would provide that, so everything would work fine.
Please see the following edk2 commits:
https://github.com/tianocore/edk2/commit/bd3afeb1d62c
https://github.com/tianocore/edk2/commit/9d560947f6d3
Eric, can you please modify SignalEndOfS3Resume() as follows:
- locate EFI_PEI_SMM_COMMUNICATION_PPI as first action,
- if it is not found, return EFI_NOT_FOUND gracefully,
- in S3ResumeBootOs(), simply log the return value, and proceed.
Currently, during S3 resume (with SMM), I get
> SignalEndOfS3Resume - Enter
> ASSERT_EFI_ERROR (Status = Not Found)
> ASSERT UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c(496):
!EFI_ERROR (Status)
Thanks,
Laszlo
> +
> + //
> + // Send command
> + //
> + Status = SmmCommunicationPpi->Communicate (
> + SmmCommunicationPpi,
> + (VOID *)CommBuffer,
> + &CommSize
> + );
> + ASSERT_EFI_ERROR (Status);
> +
> + DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
> +
> + return Status;
> +}
> +
> +/**
> Jump to OS waking vector.
> The function will install boot script done PPI, report S3 resume status code, and then jump to OS waking vector.
>
> @@ -504,6 +585,12 @@ S3ResumeBootOs (
> ASSERT_EFI_ERROR (Status);
>
> //
> + // Signal EndOfS3Resume event.
> + //
> + Status = SignalEndOfS3Resume ();
> + ASSERT_EFI_ERROR (Status);
> +
> + //
> // report status code on S3 resume
> //
> REPORT_STATUS_CODE (EFI_PROGRESS_CODE, EFI_SOFTWARE_PEI_MODULE | EFI_SW_PEI_PC_OS_WAKE);
> diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> index d514523..943f114 100644
> --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> @@ -85,6 +85,10 @@
> gPeiSmmAccessPpiGuid ## SOMETIMES_CONSUMES
> gPeiPostScriptTablePpiGuid ## SOMETIMES_PRODUCES
> gEfiEndOfPeiSignalPpiGuid ## SOMETIMES_PRODUCES
> + gEfiPeiSmmCommunicationPpiGuid ## SOMETIMES_CONSUMES
> +
> +[Protocols]
> + gEdkiiSmmEndOfS3ResumeProtocolGuid ## SOMETIMES_CONSUMES
>
> [FeaturePcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode ## CONSUMES
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Patch v3 2/3] UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.
2017-10-12 8:01 ` Laszlo Ersek
@ 2017-10-12 8:14 ` Dong, Eric
0 siblings, 0 replies; 8+ messages in thread
From: Dong, Eric @ 2017-10-12 8:14 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: Ni, Ruiyu, edk2-devel@lists.01.org, Yao, Jiewen
Hi Laszlo,
Sorry for this patch break OVMF. Platform without this Ppi is not what we expected before. So we just add ASSERT here.
I will send a patch ASAP to fix this ASSERT case. Also we will provide another patch later to handle the platform without this Ppi case.
Thanks,
Eric
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
Sent: Thursday, October 12, 2017 4:01 PM
To: Dong, Eric <eric.dong@intel.com>
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org; Yao, Jiewen <jiewen.yao@intel.com>
Subject: Re: [edk2] [Patch v3 2/3] UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.
Hi,
I'm sorry that I couldn't comment on this patch in time.
However, I must observe that version 1 of the patch,
http://mid.mail-archive.com/1507688554-10264-3-git-send-email-eric.dong@intel.com
was sent out at 04:22 AM on 11 Oct, in my time zone; and version 3 of the patch was committed at 04:17 AM on 12 Oct in my time zone.
That's not a lot of time for reviewers to comment -- I was very busy with other stuff yesterday. I even set up an auto-reply about that, asking for a bit of patience.
Of course the auto-reply didn't help, because I wasn't CC'd on the patch in the first place; despite my earlier request to be CC'd on all SMM- and/or S3-related patches.
So, after this introduction:
On 10/11/17 10:22, Eric Dong wrote:
> Driver will send S3 resume finished event to SmmCore through
> communicate buffer after it signals EndOfPei event.
>
> V2 Changes:
> 1. Change structures name to avoid they start with EFI_.
> 2. Base on DXE phase bits to provide communication buffer, current
> implement check both PEI and DXE phase.
>
> V3 Changes:
> 1. Change structure name for better understanding.
> 2. Enhance communication buffer calculate logic to more accurate.
>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
> UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 87 ++++++++++++++++++++++
> .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf | 4 +
> 2 files changed, 91 insertions(+)
>
> diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> index e53ed21..c2171cb 100644
> --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> @@ -28,6 +28,9 @@
> #include <Ppi/SmmAccess.h>
> #include <Ppi/PostBootScriptTable.h>
> #include <Ppi/EndOfPeiPhase.h>
> +#include <Ppi/SmmCommunication.h>
> +
> +#include <Protocol/SmmEndOfS3Resume.h>
>
> #include <Library/DebugLib.h>
> #include <Library/BaseLib.h>
> @@ -151,6 +154,22 @@ typedef union {
> UINT64 Uint64;
> } PAGE_TABLE_1G_ENTRY;
>
> +//
> +// Define two type of smm communicate headers.
> +// One for 32 bits PEI + 64 bits DXE, the other for 32 bits PEI + 32 bits DXE case.
> +//
> +typedef struct {
> + EFI_GUID HeaderGuid;
> + UINT32 MessageLength;
> + UINT8 Data[1];
> +} SMM_COMMUNICATE_HEADER_32;
> +
> +typedef struct {
> + EFI_GUID HeaderGuid;
> + UINT64 MessageLength;
> + UINT8 Data[1];
> +} SMM_COMMUNICATE_HEADER_64;
> +
> #pragma pack()
>
> //
> @@ -430,6 +449,68 @@ IsLongModeWakingVector ( }
>
> /**
> + Send EndOfS3Resume event to SmmCore through communication buffer way.
> +
> + @retval EFI_SUCCESS Return send the event success.
> +**/
> +EFI_STATUS
> +SignalEndOfS3Resume (
> + VOID
> + )
> +{
> + EFI_STATUS Status;
> + EFI_PEI_SMM_COMMUNICATION_PPI *SmmCommunicationPpi;
> + UINTN CommSize;
> + SMM_COMMUNICATE_HEADER_32 Header32;
> + SMM_COMMUNICATE_HEADER_64 Header64;
> + VOID *CommBuffer;
> +
> + DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Enter\n"));
> +
> + //
> + // This buffer consumed in DXE phase, so base on DXE mode to prepare communicate buffer.
> + // Detect whether DXE is 64 bits mode.
> + // if (sizeof(UINTN) == sizeof(UINT64), PEI already 64 bits, assume DXE also 64 bits.
> + // or (FeaturePcdGet (PcdDxeIplSwitchToLongMode)), Dxe will switch to 64 bits.
> + //
> + if ((sizeof(UINTN) == sizeof(UINT64)) || (FeaturePcdGet (PcdDxeIplSwitchToLongMode))) {
> + CommBuffer = &Header64;
> + Header64.MessageLength = 0;
> + CommSize = OFFSET_OF (SMM_COMMUNICATE_HEADER_64, Data); } else {
> + CommBuffer = &Header32;
> + Header32.MessageLength = 0;
> + CommSize = OFFSET_OF (SMM_COMMUNICATE_HEADER_32, Data); }
> + CopyGuid (CommBuffer, &gEdkiiSmmEndOfS3ResumeProtocolGuid);
> +
> + //
> + // Get needed resource
> + //
> + Status = PeiServicesLocatePpi (
> + &gEfiPeiSmmCommunicationPpiGuid,
> + 0,
> + NULL,
> + (VOID **)&SmmCommunicationPpi
> + );
> + ASSERT_EFI_ERROR (Status);
This patch breaks S3 resume with OVMF+SMM. OVMF does not have or install an EFI_PEI_SMM_COMMUNICATION_PPI. We discussed this ~2 years ago, when SMM was being added to KVM, QEMU, and OVMF.
OVMF would have needed EFI_PEI_SMM_COMMUNICATION_PPI only for SmmLockBoxPeiLib. But we agreed that SmmLockBoxPeiLib would work without this PPI; exactly the same as if the PPI existed, but Communicate() returned EFI_NOT_STARTED.
Namely, if RestoreLockBox() failed to locate EFI_PEI_SMM_COMMUNICATION_PPI, then InternalRestoreLockBoxFromSmram() would be called instead, which only needed PEI_SMM_ACCESS_PPI. And OVMF would provide that, so everything would work fine.
Please see the following edk2 commits:
https://github.com/tianocore/edk2/commit/bd3afeb1d62c
https://github.com/tianocore/edk2/commit/9d560947f6d3
Eric, can you please modify SignalEndOfS3Resume() as follows:
- locate EFI_PEI_SMM_COMMUNICATION_PPI as first action,
- if it is not found, return EFI_NOT_FOUND gracefully,
- in S3ResumeBootOs(), simply log the return value, and proceed.
Currently, during S3 resume (with SMM), I get
> SignalEndOfS3Resume - Enter
> ASSERT_EFI_ERROR (Status = Not Found)
> ASSERT UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c(496):
!EFI_ERROR (Status)
Thanks,
Laszlo
> +
> + //
> + // Send command
> + //
> + Status = SmmCommunicationPpi->Communicate (
> + SmmCommunicationPpi,
> + (VOID *)CommBuffer,
> + &CommSize
> + );
> + ASSERT_EFI_ERROR (Status);
> +
> + DEBUG ((EFI_D_INFO, "SignalEndOfS3Resume - Exit (%r)\n", Status));
> +
> + return Status;
> +}
> +
> +/**
> Jump to OS waking vector.
> The function will install boot script done PPI, report S3 resume status code, and then jump to OS waking vector.
>
> @@ -504,6 +585,12 @@ S3ResumeBootOs (
> ASSERT_EFI_ERROR (Status);
>
> //
> + // Signal EndOfS3Resume event.
> + //
> + Status = SignalEndOfS3Resume ();
> + ASSERT_EFI_ERROR (Status);
> +
> + //
> // report status code on S3 resume
> //
> REPORT_STATUS_CODE (EFI_PROGRESS_CODE, EFI_SOFTWARE_PEI_MODULE |
> EFI_SW_PEI_PC_OS_WAKE); diff --git
> a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> index d514523..943f114 100644
> --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> @@ -85,6 +85,10 @@
> gPeiSmmAccessPpiGuid ## SOMETIMES_CONSUMES
> gPeiPostScriptTablePpiGuid ## SOMETIMES_PRODUCES
> gEfiEndOfPeiSignalPpiGuid ## SOMETIMES_PRODUCES
> + gEfiPeiSmmCommunicationPpiGuid ## SOMETIMES_CONSUMES
> +
> +[Protocols]
> + gEdkiiSmmEndOfS3ResumeProtocolGuid ## SOMETIMES_CONSUMES
>
> [FeaturePcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode ## CONSUMES
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Patch v3 3/3] MdeModulePkg/PiSmmCore: Install Protocol when S3 resume finished.
2017-10-11 8:22 [Patch v3 0/3] Add SmmEndOfS3Resume event Eric Dong
2017-10-11 8:22 ` [Patch 1/3] MdeModulePkg/SmmEndOfS3Resume.h: Add new protocol definition Eric Dong
2017-10-11 8:22 ` [Patch v3 2/3] UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore Eric Dong
@ 2017-10-11 8:22 ` Eric Dong
2017-10-12 2:04 ` [Patch v3 0/3] Add SmmEndOfS3Resume event Yao, Jiewen
3 siblings, 0 replies; 8+ messages in thread
From: Eric Dong @ 2017-10-11 8:22 UTC (permalink / raw)
To: edk2-devel; +Cc: Ruiyu Ni, Jiewen Yao
Install EdkiiSmmEndOfS3ResumeProtocol when S3 resume finished.
S3ResumePei will send S3 resume finished event to SmmCore through
communication buffer.
V2 change:
None.
V3 change:
1. Uninstall the protocol right after install it to avoid run out of memory.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 69 +++++++++++++++++++++++++++----
MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 24 +++++++++++
MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf | 1 +
3 files changed, 87 insertions(+), 7 deletions(-)
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
index 9e4390e..b833763 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
@@ -75,13 +75,14 @@ BOOLEAN mInLegacyBoot = FALSE;
// Table of SMI Handlers that are registered by the SMM Core when it is initialized
//
SMM_CORE_SMI_HANDLERS mSmmCoreSmiHandlers[] = {
- { SmmDriverDispatchHandler, &gEfiEventDxeDispatchGuid, NULL, TRUE },
- { SmmReadyToLockHandler, &gEfiDxeSmmReadyToLockProtocolGuid, NULL, TRUE },
- { SmmLegacyBootHandler, &gEfiEventLegacyBootGuid, NULL, FALSE },
- { SmmExitBootServicesHandler, &gEfiEventExitBootServicesGuid, NULL, FALSE },
- { SmmReadyToBootHandler, &gEfiEventReadyToBootGuid, NULL, FALSE },
- { SmmEndOfDxeHandler, &gEfiEndOfDxeEventGroupGuid, NULL, TRUE },
- { NULL, NULL, NULL, FALSE }
+ { SmmDriverDispatchHandler, &gEfiEventDxeDispatchGuid, NULL, TRUE },
+ { SmmReadyToLockHandler, &gEfiDxeSmmReadyToLockProtocolGuid, NULL, TRUE },
+ { SmmLegacyBootHandler, &gEfiEventLegacyBootGuid, NULL, FALSE },
+ { SmmExitBootServicesHandler, &gEfiEventExitBootServicesGuid, NULL, FALSE },
+ { SmmReadyToBootHandler, &gEfiEventReadyToBootGuid, NULL, FALSE },
+ { SmmEndOfDxeHandler, &gEfiEndOfDxeEventGroupGuid, NULL, TRUE },
+ { SmmEndOfS3ResumeHandler, &gEdkiiSmmEndOfS3ResumeProtocolGuid, NULL, FALSE },
+ { NULL, NULL, NULL, FALSE }
};
UINTN mFullSmramRangeCount;
@@ -383,6 +384,60 @@ SmmEndOfDxeHandler (
}
/**
+ Software SMI handler that is called when the EndOfS3Resume event is trigged.
+ This function installs the SMM EndOfS3Resume Protocol so SMM Drivers are informed that
+ S3 resume has finished.
+
+ @param DispatchHandle The unique handle assigned to this handler by SmiHandlerRegister().
+ @param Context Points to an optional handler context which was specified when the handler was registered.
+ @param CommBuffer A pointer to a collection of data in memory that will
+ be conveyed from a non-SMM environment into an SMM environment.
+ @param CommBufferSize The size of the CommBuffer.
+
+ @return Status Code
+
+**/
+EFI_STATUS
+EFIAPI
+SmmEndOfS3ResumeHandler (
+ IN EFI_HANDLE DispatchHandle,
+ IN CONST VOID *Context, OPTIONAL
+ IN OUT VOID *CommBuffer, OPTIONAL
+ IN OUT UINTN *CommBufferSize OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ EFI_HANDLE SmmHandle;
+
+ DEBUG ((EFI_D_INFO, "SmmEndOfS3ResumeHandler\n"));
+
+ //
+ // Install SMM EndOfS3Resume protocol
+ //
+ SmmHandle = NULL;
+ Status = SmmInstallProtocolInterface (
+ &SmmHandle,
+ &gEdkiiSmmEndOfS3ResumeProtocolGuid,
+ EFI_NATIVE_INTERFACE,
+ NULL
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ //
+ // Uninstall the protocol here because the comsume just hook the
+ // installation event.
+ //
+ Status = SmmUninstallProtocolInterface (
+ SmmHandle,
+ &gEdkiiSmmEndOfS3ResumeProtocolGuid,
+ NULL
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ return Status;
+}
+
+/**
Determine if two buffers overlap in memory.
@param[in] Buff1 Pointer to first buffer
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
index b6f815c..6cc824b 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
@@ -32,6 +32,7 @@
#include <Protocol/SmmExitBootServices.h>
#include <Protocol/SmmLegacyBoot.h>
#include <Protocol/SmmReadyToBoot.h>
+#include <Protocol/SmmEndOfS3Resume.h>
#include <Guid/Apriori.h>
#include <Guid/EventGroup.h>
@@ -802,6 +803,29 @@ SmmReadyToBootHandler (
);
/**
+ Software SMI handler that is called when the EndOfS3Resume event is trigged.
+ This function installs the SMM EndOfS3Resume Protocol so SMM Drivers are informed that
+ S3 resume has finished.
+
+ @param DispatchHandle The unique handle assigned to this handler by SmiHandlerRegister().
+ @param Context Points to an optional handler context which was specified when the handler was registered.
+ @param CommBuffer A pointer to a collection of data in memory that will
+ be conveyed from a non-SMM environment into an SMM environment.
+ @param CommBufferSize The size of the CommBuffer.
+
+ @return Status Code
+
+**/
+EFI_STATUS
+EFIAPI
+SmmEndOfS3ResumeHandler (
+ IN EFI_HANDLE DispatchHandle,
+ IN CONST VOID *Context, OPTIONAL
+ IN OUT VOID *CommBuffer, OPTIONAL
+ IN OUT UINTN *CommBufferSize OPTIONAL
+ );
+
+/**
Place holder function until all the SMM System Table Service are available.
@param Arg1 Undefined
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
index 95e34bd..a724189 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
@@ -79,6 +79,7 @@
gEdkiiSmmExitBootServicesProtocolGuid ## SOMETIMES_PRODUCES
gEdkiiSmmLegacyBootProtocolGuid ## SOMETIMES_PRODUCES
gEdkiiSmmReadyToBootProtocolGuid ## PRODUCES
+ gEdkiiSmmEndOfS3ResumeProtocolGuid ## SOMETIMES_PRODUCES
gEfiSmmSwDispatch2ProtocolGuid ## SOMETIMES_CONSUMES
gEfiSmmSxDispatch2ProtocolGuid ## SOMETIMES_CONSUMES
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [Patch v3 0/3] Add SmmEndOfS3Resume event.
2017-10-11 8:22 [Patch v3 0/3] Add SmmEndOfS3Resume event Eric Dong
` (2 preceding siblings ...)
2017-10-11 8:22 ` [Patch v3 3/3] MdeModulePkg/PiSmmCore: Install Protocol when S3 resume finished Eric Dong
@ 2017-10-12 2:04 ` Yao, Jiewen
3 siblings, 0 replies; 8+ messages in thread
From: Yao, Jiewen @ 2017-10-12 2:04 UTC (permalink / raw)
To: Dong, Eric, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu
Reviewed-by: Jiewen.yao@intel.com
> -----Original Message-----
> From: Dong, Eric
> Sent: Wednesday, October 11, 2017 4:22 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [Patch v3 0/3] Add SmmEndOfS3Resume event.
>
> This patch series add new SmmEndOfS3Resume event which required by some
> SMM drivers.
> It will implmented by SmmCore to install the
> gEdkiiSmmEndOfS3ResumeProtocolGuid
> Protocol. Smm drivers can install this protocol's notification functions to
> hoot this envet.
> It will be trigged right after the EndOfPei event in S3 resume phase.
>
> V2 Changes:
> Only change patch 2/3
> 1. Change structures name to avoid they start with EFI_.
> 2. Base on DXE phase bits to provide communication buffer, current implement
> check both PEI and DXE phase.
>
> V3 Changes:
> for 2/3 patch:UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to
> SmmCore.
> 1. Change structure name for better understanding.
> 2. Enhance communication buffer calculate logic to more accurate.
>
> for 3/3 patch: MdeModulePkg/PiSmmCore: Install Protocol when S3 resume
> finished.
> 1. Uninstall the protocol right after install it to avoid run out of memory.
>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
>
> Eric Dong (3):
> MdeModulePkg/SmmEndOfS3Resume.h: Add new protocol definition.
> UefiCpuPkg/S3Resume2Pei: Send S3 resume finished event to SmmCore.
> MdeModulePkg/PiSmmCore: Install Protocol when S3 resume finished.
>
> MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 55
> ++++++++++++--
> MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 24 ++++++
> MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf | 1 +
> MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h | 31 ++++++++
> MdeModulePkg/MdeModulePkg.dec | 3 +
> UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 85
> ++++++++++++++++++++++
> .../Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf | 4 +
> 7 files changed, 196 insertions(+), 7 deletions(-)
> create mode 100644 MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h
>
> --
> 2.7.0.windows.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Patch 1/3] MdeModulePkg/SmmEndOfS3Resume.h: Add new protocol definition.
2017-10-11 2:22 [Patch " Eric Dong
@ 2017-10-11 2:22 ` Eric Dong
0 siblings, 0 replies; 8+ messages in thread
From: Eric Dong @ 2017-10-11 2:22 UTC (permalink / raw)
To: edk2-devel; +Cc: Ruiyu Ni, Jiewen Yao
Add gEdkiiSmmEndOfS3ResumeProtocolGuid which used by SmmCore to
notify smm drives that S3 resume has finished.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h | 26 ++++++++++++++++++++++++
MdeModulePkg/MdeModulePkg.dec | 3 +++
2 files changed, 29 insertions(+)
create mode 100644 MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h
diff --git a/MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h b/MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h
new file mode 100644
index 0000000..9716f6a
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/SmmEndOfS3Resume.h
@@ -0,0 +1,26 @@
+/** @file
+ This Protocol will be installed at the end of S3 resume phase in SMM environment.
+ It allows for smm drivers to hook this point and do the requried tasks.
+
+ Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __SMM_END_OF_S3_RESUME_H__
+#define __SMM_END_OF_S3_RESUME_H__
+
+#define EDKII_SMM_END_OF_S3_RESUME_PROTOCOL_GUID \
+ { \
+ 0x96f5296d, 0x05f7, 0x4f3c, {0x84, 0x67, 0xe4, 0x56, 0x89, 0x0e, 0x0c, 0xb5 } \
+ }
+
+extern EFI_GUID gEdkiiSmmEndOfS3ResumeProtocolGuid;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index a3c0633..216e4f9 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -556,6 +556,9 @@
## Include/Protocol/IoMmu.h
gEdkiiIoMmuProtocolGuid = { 0x4e939de9, 0xd948, 0x4b0f, { 0x88, 0xed, 0xe6, 0xe1, 0xce, 0x51, 0x7c, 0x1e } }
+ ## Include/Protocol/SmmEndofS3Resume.h
+ gEdkiiSmmEndOfS3ResumeProtocolGuid = { 0x96f5296d, 0x05f7, 0x4f3c, {0x84, 0x67, 0xe4, 0x56, 0x89, 0x0e, 0x0c, 0xb5 } }
+
#
# [Error.gEfiMdeModulePkgTokenSpaceGuid]
# 0x80000001 | Invalid value provided.
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread