From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 9E06DD80D5A for ; Wed, 6 Sep 2023 09:25:07 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=/SSq6KM90IVm2kSiryxAcGzsrz0lcGAtziGb08gFM1g=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1693992306; v=1; b=qM4Z08KZFzf2nNP0yUB7HDvbaJbTV51UI2UJSvGHFHuBBAVAgEeCIb9FbtyEYlq0F3a+BH8G MEu83XtPNBGxuPoQaXEXkS122FWd0VfDVqzYcqvRd/Qqp3xtlXOzm03veUpiHmoA8DOaGzncnMf FN602CtFiJgy7muiTHpnDTT4= X-Received: by 127.0.0.2 with SMTP id 8iG0YY7687511xVqy6WQN2rQ; Wed, 06 Sep 2023 02:25:06 -0700 X-Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by mx.groups.io with SMTP id smtpd.web10.4594.1693992305386145580 for ; Wed, 06 Sep 2023 02:25:05 -0700 X-Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 06343B81982 for ; Wed, 6 Sep 2023 09:25:03 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id B9B54C433A9 for ; Wed, 6 Sep 2023 09:25:01 +0000 (UTC) X-Received: by mail-lj1-f178.google.com with SMTP id 38308e7fff4ca-2bc63e0d8cdso52643511fa.2 for ; Wed, 06 Sep 2023 02:25:01 -0700 (PDT) X-Gm-Message-State: ybqQnid7W88E0Amjb8aDZPPQx7686176AA= X-Google-Smtp-Source: AGHT+IF9m75EGAUP/FRxHy9FzYPj8AVAnqNyJxlhMAtQ5mOPYoA4I6H2XvJqCnVZ3MZJmkj+1jC0DjCyu17N+9+HlJQ= X-Received: by 2002:a05:6512:3e01:b0:4fd:cae7:2393 with SMTP id i1-20020a0565123e0100b004fdcae72393mr2496977lfv.2.1693992299613; Wed, 06 Sep 2023 02:24:59 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: "Ard Biesheuvel" Date: Wed, 6 Sep 2023 11:24:48 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH] OvmfPkg/OvmfXen: Fix S3 To: Xenia Ragiadakou Cc: devel@edk2.groups.io, Ard Biesheuvel , Jiewen Yao , Jordan Justen , Gerd Hoffmann , Anthony Perard , Julien Grall , xen-devel@lists.xenproject.org Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,ardb@kernel.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=qM4Z08KZ; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=none) On Thu, 13 Jul 2023 at 12:48, 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. > > 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 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] -=-=-=-=-=-=-=-=-=-=-=-