public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] OvmfPkg/OvmfXen: Fix S3
@ 2023-07-13 10:47 Xenia Ragiadakou
  2023-09-06  9:24 ` [edk2-devel] " Ard Biesheuvel
  2023-09-11 15:06 ` Anthony PERARD via groups.io
  0 siblings, 2 replies; 3+ messages in thread
From: Xenia Ragiadakou @ 2023-07-13 10:47 UTC (permalink / raw)
  To: devel
  Cc: Xenia Ragiadakou, Ard Biesheuvel, Jiewen Yao, Jordan Justen,
	Gerd Hoffmann, Anthony Perard, Julien Grall, xen-devel

Currently, resuming an S3 suspended guest results in the following
assertion failure:
ASSERT MdePkg/Library/PeiResourcePublicationLib/PeiResourcePublicationLib.c(41): MemoryLength > 0
This happens because some parts of the S3 suspend and resume paths
are missing in order for S3 to work. For instance, the variables
mS3AcpiReservedMemoryBase and mS3AcpiReservedMemoryBase are not
initialized, regions that are used on S3 resume are either missing
or not marked as ACPI NVS memory and can be corrupted by the OS.
This patch adds the missing parts based heavily on the existing S3
implementation of other virtual platforms.

For S3 support, the provision of fw_cfg is required in order for
suspend states to be retrieved.

Another issue noticed is that when CalibrateLapicTimer() is called
on S3 resume path, the shared info page is remapped to a different
guest physical address. This remapping happens under guest's feet,
so any subsequent attempt of the guest to access the shared info
page results in nested page faults. This patch removes any local
APIC timer initializion and calibration from S3 resume path.

Signed-off-by: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
---
 OvmfPkg/XenPlatformPei/Fv.c               |  2 +-
 OvmfPkg/XenPlatformPei/MemDetect.c        | 60 ++++++++++++++++++++++-
 OvmfPkg/XenPlatformPei/Platform.c         | 11 ++++-
 OvmfPkg/XenPlatformPei/Platform.h         |  2 +
 OvmfPkg/XenPlatformPei/XenPlatformPei.inf |  7 +++
 5 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/XenPlatformPei/Fv.c b/OvmfPkg/XenPlatformPei/Fv.c
index 871a2c1c5b..37ecb3cfee 100644
--- a/OvmfPkg/XenPlatformPei/Fv.c
+++ b/OvmfPkg/XenPlatformPei/Fv.c
@@ -37,7 +37,7 @@ PeiFvInitialization (
   BuildMemoryAllocationHob (
     PcdGet32 (PcdOvmfPeiMemFvBase),
     PcdGet32 (PcdOvmfPeiMemFvSize),
-    EfiBootServicesData
+    mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
     );
 
   //
diff --git a/OvmfPkg/XenPlatformPei/MemDetect.c b/OvmfPkg/XenPlatformPei/MemDetect.c
index e552e7a55e..1724a4988f 100644
--- a/OvmfPkg/XenPlatformPei/MemDetect.c
+++ b/OvmfPkg/XenPlatformPei/MemDetect.c
@@ -283,6 +283,19 @@ PublishPeiMemory (
 
   LowerMemorySize = GetSystemMemorySizeBelow4gb ();
 
+  //
+  // If S3 is supported, then the S3 permanent PEI memory is placed next,
+  // downwards. Its size is primarily dictated by CpuMpPei. The formula below
+  // is an approximation.
+  //
+  if (mS3Supported) {
+    mS3AcpiReservedMemorySize = SIZE_512KB +
+                                PcdGet32 (PcdCpuMaxLogicalProcessorNumber) *
+                                PcdGet32 (PcdCpuApStackSize);
+    mS3AcpiReservedMemoryBase = LowerMemorySize - mS3AcpiReservedMemorySize;
+    LowerMemorySize           = mS3AcpiReservedMemoryBase;
+  }
+
   if (mBootMode == BOOT_ON_S3_RESUME) {
     MemoryBase = mS3AcpiReservedMemoryBase;
     MemorySize = mS3AcpiReservedMemorySize;
@@ -328,6 +341,51 @@ InitializeRamRegions (
 {
   XenPublishRamRegions ();
 
+  if (mS3Supported && (mBootMode != BOOT_ON_S3_RESUME)) {
+    //
+    // This is the memory range that will be used for PEI on S3 resume
+    //
+    BuildMemoryAllocationHob (
+      mS3AcpiReservedMemoryBase,
+      mS3AcpiReservedMemorySize,
+      EfiACPIMemoryNVS
+      );
+
+    //
+    // Cover the initial RAM area used as stack and temporary PEI heap.
+    //
+    // This is reserved as ACPI NVS so it can be used on S3 resume.
+    //
+    BuildMemoryAllocationHob (
+      PcdGet32 (PcdOvmfSecPeiTempRamBase),
+      PcdGet32 (PcdOvmfSecPeiTempRamSize),
+      EfiACPIMemoryNVS
+      );
+
+    //
+    // SEC stores its table of GUIDed section handlers here.
+    //
+    BuildMemoryAllocationHob (
+      PcdGet64 (PcdGuidedExtractHandlerTableAddress),
+      PcdGet32 (PcdGuidedExtractHandlerTableSize),
+      EfiACPIMemoryNVS
+      );
+
+ #ifdef MDE_CPU_X64
+    //
+    // Reserve the initial page tables built by the reset vector code.
+    //
+    // Since this memory range will be used by the Reset Vector on S3
+    // resume, it must be reserved as ACPI NVS.
+    //
+    BuildMemoryAllocationHob (
+      (EFI_PHYSICAL_ADDRESS)(UINTN)PcdGet32 (PcdOvmfSecPageTablesBase),
+      (UINT64)(UINTN)PcdGet32 (PcdOvmfSecPageTablesSize),
+      EfiACPIMemoryNVS
+      );
+ #endif
+  }
+
   if (mBootMode != BOOT_ON_S3_RESUME) {
     //
     // Reserve the lock box storage area
@@ -346,7 +404,7 @@ InitializeRamRegions (
     BuildMemoryAllocationHob (
       (EFI_PHYSICAL_ADDRESS)(UINTN)PcdGet32 (PcdOvmfLockBoxStorageBase),
       (UINT64)(UINTN)PcdGet32 (PcdOvmfLockBoxStorageSize),
-      EfiBootServicesData
+      mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
       );
   }
 }
diff --git a/OvmfPkg/XenPlatformPei/Platform.c b/OvmfPkg/XenPlatformPei/Platform.c
index c3fdf3d0b8..1b074cff33 100644
--- a/OvmfPkg/XenPlatformPei/Platform.c
+++ b/OvmfPkg/XenPlatformPei/Platform.c
@@ -60,6 +60,8 @@ UINT16  mHostBridgeDevId;
 
 EFI_BOOT_MODE  mBootMode = BOOT_WITH_FULL_CONFIGURATION;
 
+BOOLEAN  mS3Supported = FALSE;
+
 VOID
 AddIoMemoryBaseSizeHob (
   EFI_PHYSICAL_ADDRESS  MemoryBase,
@@ -350,6 +352,11 @@ BootModeInitialization (
 
   if (CmosRead8 (0xF) == 0xFE) {
     mBootMode = BOOT_ON_S3_RESUME;
+    if (!mS3Supported) {
+      DEBUG ((DEBUG_ERROR, "ERROR: S3 not supported\n"));
+      ASSERT (FALSE);
+      CpuDeadLoop ();
+    }
   }
 
   CmosWrite8 (0xF, 0x00);
@@ -463,6 +470,7 @@ InitializeXenPlatform (
   //
   if (QemuFwCfgS3Enabled ()) {
     DEBUG ((DEBUG_INFO, "S3 support was detected on QEMU\n"));
+    mS3Supported = TRUE;
     Status = PcdSetBoolS (PcdAcpiS3Enable, TRUE);
     ASSERT_EFI_ERROR (Status);
   }
@@ -481,9 +489,8 @@ InitializeXenPlatform (
 
   InitializeRamRegions ();
 
-  CalibrateLapicTimer ();
-
   if (mBootMode != BOOT_ON_S3_RESUME) {
+    CalibrateLapicTimer ();
     ReserveEmuVariableNvStore ();
     PeiFvInitialization ();
     MemMapInitialization ();
diff --git a/OvmfPkg/XenPlatformPei/Platform.h b/OvmfPkg/XenPlatformPei/Platform.h
index 7b4de128e7..fda66747cc 100644
--- a/OvmfPkg/XenPlatformPei/Platform.h
+++ b/OvmfPkg/XenPlatformPei/Platform.h
@@ -139,4 +139,6 @@ extern UINT8  mPhysMemAddressWidth;
 
 extern UINT16  mHostBridgeDevId;
 
+extern BOOLEAN  mS3Supported;
+
 #endif // _PLATFORM_PEI_H_INCLUDED_
diff --git a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
index 20c27ff34b..a359cf60ca 100644
--- a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
+++ b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
@@ -69,9 +69,13 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
+  gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
   gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase
   gUefiOvmfPkgTokenSpaceGuid.PcdPciIoSize
@@ -80,6 +84,7 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base
   gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
+  gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
@@ -89,6 +94,8 @@
   gEfiMdePkgTokenSpaceGuid.PcdFSBClock
   gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy
   gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
 
   gUefiOvmfPkgTokenSpaceGuid.PcdXenPvhStartOfDayStructPtr
   gUefiOvmfPkgTokenSpaceGuid.PcdXenPvhStartOfDayStructPtrSize
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [edk2-devel] [PATCH] OvmfPkg/OvmfXen: Fix S3
  2023-07-13 10:47 [PATCH] OvmfPkg/OvmfXen: Fix S3 Xenia Ragiadakou
@ 2023-09-06  9:24 ` Ard Biesheuvel
  2023-09-11 15:06 ` Anthony PERARD via groups.io
  1 sibling, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2023-09-06  9:24 UTC (permalink / raw)
  To: Xenia Ragiadakou
  Cc: devel, Ard Biesheuvel, Jiewen Yao, Jordan Justen, Gerd Hoffmann,
	Anthony Perard, Julien Grall, xen-devel

On Thu, 13 Jul 2023 at 12:48, Xenia Ragiadakou <xenia.ragiadakou@amd.com> wrote:
>
> Currently, resuming an S3 suspended guest results in the following
> assertion failure:
> ASSERT MdePkg/Library/PeiResourcePublicationLib/PeiResourcePublicationLib.c(41): MemoryLength > 0
> This happens because some parts of the S3 suspend and resume paths
> are missing in order for S3 to work. For instance, the variables
> mS3AcpiReservedMemoryBase and mS3AcpiReservedMemoryBase are not
> initialized, regions that are used on S3 resume are either missing
> or not marked as ACPI NVS memory and can be corrupted by the OS.
> This patch adds the missing parts based heavily on the existing S3
> implementation of other virtual platforms.
>
> For S3 support, the provision of fw_cfg is required in order for
> suspend states to be retrieved.
>
> Another issue noticed is that when CalibrateLapicTimer() is called
> on S3 resume path, the shared info page is remapped to a different
> guest physical address. This remapping happens under guest's feet,
> so any subsequent attempt of the guest to access the shared info
> page results in nested page faults. This patch removes any local
> APIC timer initializion and calibration from S3 resume path.
>
> Signed-off-by: Xenia Ragiadakou <xenia.ragiadakou@amd.com>

Anyone care to review this?

> ---
>  OvmfPkg/XenPlatformPei/Fv.c               |  2 +-
>  OvmfPkg/XenPlatformPei/MemDetect.c        | 60 ++++++++++++++++++++++-
>  OvmfPkg/XenPlatformPei/Platform.c         | 11 ++++-
>  OvmfPkg/XenPlatformPei/Platform.h         |  2 +
>  OvmfPkg/XenPlatformPei/XenPlatformPei.inf |  7 +++
>  5 files changed, 78 insertions(+), 4 deletions(-)
>
> diff --git a/OvmfPkg/XenPlatformPei/Fv.c b/OvmfPkg/XenPlatformPei/Fv.c
> index 871a2c1c5b..37ecb3cfee 100644
> --- a/OvmfPkg/XenPlatformPei/Fv.c
> +++ b/OvmfPkg/XenPlatformPei/Fv.c
> @@ -37,7 +37,7 @@ PeiFvInitialization (
>    BuildMemoryAllocationHob (
>      PcdGet32 (PcdOvmfPeiMemFvBase),
>      PcdGet32 (PcdOvmfPeiMemFvSize),
> -    EfiBootServicesData
> +    mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
>      );
>
>    //
> diff --git a/OvmfPkg/XenPlatformPei/MemDetect.c b/OvmfPkg/XenPlatformPei/MemDetect.c
> index e552e7a55e..1724a4988f 100644
> --- a/OvmfPkg/XenPlatformPei/MemDetect.c
> +++ b/OvmfPkg/XenPlatformPei/MemDetect.c
> @@ -283,6 +283,19 @@ PublishPeiMemory (
>
>    LowerMemorySize = GetSystemMemorySizeBelow4gb ();
>
> +  //
> +  // If S3 is supported, then the S3 permanent PEI memory is placed next,
> +  // downwards. Its size is primarily dictated by CpuMpPei. The formula below
> +  // is an approximation.
> +  //
> +  if (mS3Supported) {
> +    mS3AcpiReservedMemorySize = SIZE_512KB +
> +                                PcdGet32 (PcdCpuMaxLogicalProcessorNumber) *
> +                                PcdGet32 (PcdCpuApStackSize);
> +    mS3AcpiReservedMemoryBase = LowerMemorySize - mS3AcpiReservedMemorySize;
> +    LowerMemorySize           = mS3AcpiReservedMemoryBase;
> +  }
> +
>    if (mBootMode == BOOT_ON_S3_RESUME) {
>      MemoryBase = mS3AcpiReservedMemoryBase;
>      MemorySize = mS3AcpiReservedMemorySize;
> @@ -328,6 +341,51 @@ InitializeRamRegions (
>  {
>    XenPublishRamRegions ();
>
> +  if (mS3Supported && (mBootMode != BOOT_ON_S3_RESUME)) {
> +    //
> +    // This is the memory range that will be used for PEI on S3 resume
> +    //
> +    BuildMemoryAllocationHob (
> +      mS3AcpiReservedMemoryBase,
> +      mS3AcpiReservedMemorySize,
> +      EfiACPIMemoryNVS
> +      );
> +
> +    //
> +    // Cover the initial RAM area used as stack and temporary PEI heap.
> +    //
> +    // This is reserved as ACPI NVS so it can be used on S3 resume.
> +    //
> +    BuildMemoryAllocationHob (
> +      PcdGet32 (PcdOvmfSecPeiTempRamBase),
> +      PcdGet32 (PcdOvmfSecPeiTempRamSize),
> +      EfiACPIMemoryNVS
> +      );
> +
> +    //
> +    // SEC stores its table of GUIDed section handlers here.
> +    //
> +    BuildMemoryAllocationHob (
> +      PcdGet64 (PcdGuidedExtractHandlerTableAddress),
> +      PcdGet32 (PcdGuidedExtractHandlerTableSize),
> +      EfiACPIMemoryNVS
> +      );
> +
> + #ifdef MDE_CPU_X64
> +    //
> +    // Reserve the initial page tables built by the reset vector code.
> +    //
> +    // Since this memory range will be used by the Reset Vector on S3
> +    // resume, it must be reserved as ACPI NVS.
> +    //
> +    BuildMemoryAllocationHob (
> +      (EFI_PHYSICAL_ADDRESS)(UINTN)PcdGet32 (PcdOvmfSecPageTablesBase),
> +      (UINT64)(UINTN)PcdGet32 (PcdOvmfSecPageTablesSize),
> +      EfiACPIMemoryNVS
> +      );
> + #endif
> +  }
> +
>    if (mBootMode != BOOT_ON_S3_RESUME) {
>      //
>      // Reserve the lock box storage area
> @@ -346,7 +404,7 @@ InitializeRamRegions (
>      BuildMemoryAllocationHob (
>        (EFI_PHYSICAL_ADDRESS)(UINTN)PcdGet32 (PcdOvmfLockBoxStorageBase),
>        (UINT64)(UINTN)PcdGet32 (PcdOvmfLockBoxStorageSize),
> -      EfiBootServicesData
> +      mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
>        );
>    }
>  }
> diff --git a/OvmfPkg/XenPlatformPei/Platform.c b/OvmfPkg/XenPlatformPei/Platform.c
> index c3fdf3d0b8..1b074cff33 100644
> --- a/OvmfPkg/XenPlatformPei/Platform.c
> +++ b/OvmfPkg/XenPlatformPei/Platform.c
> @@ -60,6 +60,8 @@ UINT16  mHostBridgeDevId;
>
>  EFI_BOOT_MODE  mBootMode = BOOT_WITH_FULL_CONFIGURATION;
>
> +BOOLEAN  mS3Supported = FALSE;
> +
>  VOID
>  AddIoMemoryBaseSizeHob (
>    EFI_PHYSICAL_ADDRESS  MemoryBase,
> @@ -350,6 +352,11 @@ BootModeInitialization (
>
>    if (CmosRead8 (0xF) == 0xFE) {
>      mBootMode = BOOT_ON_S3_RESUME;
> +    if (!mS3Supported) {
> +      DEBUG ((DEBUG_ERROR, "ERROR: S3 not supported\n"));
> +      ASSERT (FALSE);
> +      CpuDeadLoop ();
> +    }
>    }
>
>    CmosWrite8 (0xF, 0x00);
> @@ -463,6 +470,7 @@ InitializeXenPlatform (
>    //
>    if (QemuFwCfgS3Enabled ()) {
>      DEBUG ((DEBUG_INFO, "S3 support was detected on QEMU\n"));
> +    mS3Supported = TRUE;
>      Status = PcdSetBoolS (PcdAcpiS3Enable, TRUE);
>      ASSERT_EFI_ERROR (Status);
>    }
> @@ -481,9 +489,8 @@ InitializeXenPlatform (
>
>    InitializeRamRegions ();
>
> -  CalibrateLapicTimer ();
> -
>    if (mBootMode != BOOT_ON_S3_RESUME) {
> +    CalibrateLapicTimer ();
>      ReserveEmuVariableNvStore ();
>      PeiFvInitialization ();
>      MemMapInitialization ();
> diff --git a/OvmfPkg/XenPlatformPei/Platform.h b/OvmfPkg/XenPlatformPei/Platform.h
> index 7b4de128e7..fda66747cc 100644
> --- a/OvmfPkg/XenPlatformPei/Platform.h
> +++ b/OvmfPkg/XenPlatformPei/Platform.h
> @@ -139,4 +139,6 @@ extern UINT8  mPhysMemAddressWidth;
>
>  extern UINT16  mHostBridgeDevId;
>
> +extern BOOLEAN  mS3Supported;
> +
>  #endif // _PLATFORM_PEI_H_INCLUDED_
> diff --git a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
> index 20c27ff34b..a359cf60ca 100644
> --- a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
> +++ b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
> @@ -69,9 +69,13 @@
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
> +  gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciIoSize
> @@ -80,6 +84,7 @@
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base
>    gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size
>    gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
> +  gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
>    gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
> @@ -89,6 +94,8 @@
>    gEfiMdePkgTokenSpaceGuid.PcdFSBClock
>    gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress
> +  gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize
> +  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
>
>    gUefiOvmfPkgTokenSpaceGuid.PcdXenPvhStartOfDayStructPtr
>    gUefiOvmfPkgTokenSpaceGuid.PcdXenPvhStartOfDayStructPtrSize
> --
> 2.34.1
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108319): https://edk2.groups.io/g/devel/message/108319
Mute This Topic: https://groups.io/mt/100121295/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [edk2-devel] [PATCH] OvmfPkg/OvmfXen: Fix S3
  2023-07-13 10:47 [PATCH] OvmfPkg/OvmfXen: Fix S3 Xenia Ragiadakou
  2023-09-06  9:24 ` [edk2-devel] " Ard Biesheuvel
@ 2023-09-11 15:06 ` Anthony PERARD via groups.io
  1 sibling, 0 replies; 3+ messages in thread
From: Anthony PERARD via groups.io @ 2023-09-11 15:06 UTC (permalink / raw)
  To: Xenia Ragiadakou
  Cc: devel, Ard Biesheuvel, Jiewen Yao, Jordan Justen, Gerd Hoffmann,
	Julien Grall, xen-devel

Hi Xenia,

On Thu, Jul 13, 2023 at 01:47:12PM +0300, Xenia Ragiadakou wrote:
> Currently, resuming an S3 suspended guest results in the following
> assertion failure:
> ASSERT MdePkg/Library/PeiResourcePublicationLib/PeiResourcePublicationLib.c(41): MemoryLength > 0
> This happens because some parts of the S3 suspend and resume paths
> are missing in order for S3 to work. For instance, the variables
> mS3AcpiReservedMemoryBase and mS3AcpiReservedMemoryBase are not
> initialized, regions that are used on S3 resume are either missing
> or not marked as ACPI NVS memory and can be corrupted by the OS.
> This patch adds the missing parts based heavily on the existing S3
> implementation of other virtual platforms.
> 
> For S3 support, the provision of fw_cfg is required in order for
> suspend states to be retrieved.

Is it possible to have S3 work without fw_cfg? We normally disable
fw_cfg in QEMU when using it to start a Xen guest. We only enable fw_cfg
if we want to boot a kernel directly in HVM (like PV guest, called
Direct Kernel Boot in xl.cfg(5)).

But even trying the direct kernel boot method, I've got issue trying to
resume a suspended guest. I ran into:
    ASSERT /root/build/ovmf/UefiCpuPkg/Library/MpInitLib/MpLib.c(2093): (LibPcdGet64(9U) & (0x00000001 | 0x00000002)) == (0x00000001 | 0x00000002)

By the way, I'm using `xl` to boot a guest to test this patch.

So, I've got some question, which version of QEMU do you use, which work
with this patch?
What toolstack do you use to boot a guest?


Thanks,

-- 
Anthony PERARD


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108489): https://edk2.groups.io/g/devel/message/108489
Mute This Topic: https://groups.io/mt/100121295/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-09-11 15:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-13 10:47 [PATCH] OvmfPkg/OvmfXen: Fix S3 Xenia Ragiadakou
2023-09-06  9:24 ` [edk2-devel] " Ard Biesheuvel
2023-09-11 15:06 ` Anthony PERARD via groups.io

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox