From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.70759.1674729038457342099 for ; Thu, 26 Jan 2023 02:30:38 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=NcOHnWmk; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id ACEAD6178D for ; Thu, 26 Jan 2023 10:30:37 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 94F5AC433A1 for ; Thu, 26 Jan 2023 10:30:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1674729036; bh=TpbZnhufCyMHPr7aSj1gWM+r2qJmpbYYrVATblyFgcQ=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=NcOHnWmkrbbveOsmhis6RxkviMJPX03DXGJPWE6875QMiWFpx75BeYdEFW0iKtyME nsmMD/yU/EjDMu0jz5iD+/Cla+j46bXnUZ670qTt3AP8z21cutpVxvKP0x9gss2fZW jNUr5eaKKLrRwHqRTuyIRxntCX09nshapQwhfYneD/BRlayZyXvmxLHbb3cKDFjjWI cLdBFG326waDZ9Y295kZZO5FuJ21sWmXsa7T+i1oz+qVyXxl/XOsEP21swz7wjD0s9 LwTdpZ22pEWzXBeuU5q0f906RUxRH2RsMUOtwEhYIhBfGmiLdVItsRp69x4p6L0nxW Q1/IDaaov5TQw== Received: by mail-lf1-f54.google.com with SMTP id a11so2452637lfg.0 for ; Thu, 26 Jan 2023 02:30:36 -0800 (PST) X-Gm-Message-State: AFqh2kqXB3e9sM75o+x6PVMKI1THqFZZfE2hw2bf1gqLYMjDRAru0clh uNrJ4P26hFp90rxBzEmxHhpxj7JAWZpNYkvaesU= X-Google-Smtp-Source: AMrXdXu4Fc+aoGMmBnlPAjLM45jbXDaDaRL/MtHL1lNcgUl1q+ZqyV/uwEOAs2zUubwU5WqdAruXtiWsw+RKiuZO+fc= X-Received: by 2002:a05:6512:3b0a:b0:4cc:9438:3b71 with SMTP id f10-20020a0565123b0a00b004cc94383b71mr3188167lfv.239.1674729034582; Thu, 26 Jan 2023 02:30:34 -0800 (PST) MIME-Version: 1.0 References: <20230126005647.3019225-1-dionnaglaze@google.com> <20230126005647.3019225-2-dionnaglaze@google.com> In-Reply-To: <20230126005647.3019225-2-dionnaglaze@google.com> From: "Ard Biesheuvel" Date: Thu, 26 Jan 2023 11:30:23 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe To: Dionna Glaze Cc: devel@edk2.groups.io, Gerd Hoffmann , James Bottomley , Jiewen Yao , Tom Lendacky , "Min M. Xu" , Andrew Fish , "Michael D. Kinney" Content-Type: text/plain; charset="UTF-8" On Thu, 26 Jan 2023 at 01:56, Dionna Glaze wrote: > > The added behavior is to accept all unaccepted memory at > ExitBootServices if the behavior is not disabled. This allows safe > upgrades for OS loaders to affirm their support for the unaccepted > memory type. > > Cc: Gerd Hoffmann > Cc: James Bottomley > Cc: Jiewen Yao > Cc: Tom Lendacky > Cc: Ard Biesheuvel > Cc: "Min M. Xu" > Cc: Andrew Fish > Cc: "Michael D. Kinney" > > Signed-off-by: Dionna Glaze > --- > OvmfPkg/AmdSevDxe/AmdSevDxe.c | 109 ++++++++++++++++++++ > OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 1 + > 2 files changed, 110 insertions(+) > > diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c > index f7600c3c81..5eec76fea2 100644 > --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c > +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include > #include > > STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = { > @@ -34,6 +35,10 @@ STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = { > > STATIC EFI_HANDLE mAmdSevDxeHandle = NULL; > > +STATIC BOOLEAN mAcceptAllMemoryAtEBS = TRUE; > + > +STATIC EFI_EVENT mAcceptAllMemoryEvent = NULL; > + > #define IS_ALIGNED(x, y) ((((x) & ((y) - 1)) == 0)) > > STATIC > @@ -62,6 +67,94 @@ AmdSevMemoryAccept ( > return EFI_SUCCESS; > } > > +STATIC > +EFI_STATUS > +AcceptAllMemory ( > + IN EDKII_MEMORY_ACCEPT_PROTOCOL *AcceptMemory > + ) > +{ > + EFI_GCD_MEMORY_SPACE_DESCRIPTOR *AllDescMap; > + UINTN NumEntries; > + UINTN Index; > + EFI_STATUS Status; > + > + DEBUG ((DEBUG_INFO, "Accepting all memory\n")); > + > + /* > + * Get a copy of the memory space map to iterate over while > + * changing the map. > + */ > + Status = gDS->GetMemorySpaceMap (&NumEntries, &AllDescMap); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + for (Index = 0; Index < NumEntries; Index++) { > + CONST EFI_GCD_MEMORY_SPACE_DESCRIPTOR *Desc; > + > + Desc = &AllDescMap[Index]; > + if (Desc->GcdMemoryType != EFI_GCD_MEMORY_TYPE_UNACCEPTED) { > + continue; > + } > + > + Status = AcceptMemory->AcceptMemory ( > + AcceptMemory, > + Desc->BaseAddress, > + Desc->Length > + ); > + if (EFI_ERROR (Status)) { > + break; > + } > + > + Status = gDS->RemoveMemorySpace (Desc->BaseAddress, Desc->Length); > + if (EFI_ERROR (Status)) { > + break; > + } > + > + Status = gDS->AddMemorySpace ( > + EfiGcdMemoryTypeSystemMemory, > + Desc->BaseAddress, > + Desc->Length, > + EFI_MEMORY_CPU_CRYPTO | EFI_MEMORY_XP | EFI_MEMORY_RO | EFI_MEMORY_RP > + ); > + if (EFI_ERROR (Status)) { > + break; > + } > + } > + > + gBS->FreePool (AllDescMap); > + return Status; > +} > + > +VOID > +EFIAPI > +ResolveUnacceptedMemory ( > + IN EFI_EVENT Event, > + IN VOID *Context > + ) > +{ > + EDKII_MEMORY_ACCEPT_PROTOCOL *AcceptMemory; > + EFI_STATUS Status; > + > + if (!mAcceptAllMemoryAtEBS) { > + return; > + } > + > + Status = gBS->LocateProtocol ( > + &gEdkiiMemoryAcceptProtocolGuid, > + NULL, > + (VOID **)&AcceptMemory > + ); > + if (Status == EFI_NOT_FOUND) { > + return; > + } > + This driver is now both the producer and consumer of gEdkiiMemoryAcceptProtocolGuid. Are there cases where the protocol we locate here could be different from the one installed by this driver? If not, we can simplify this, and just call AmdSevMemoryAccept() directly. > + ASSERT_EFI_ERROR (Status); > + > + Status = AcceptAllMemory (AcceptMemory); > + ASSERT_EFI_ERROR (Status); > +} > + > STATIC EDKII_MEMORY_ACCEPT_PROTOCOL mMemoryAcceptProtocol = { > AmdSevMemoryAccept > }; > @@ -195,6 +288,22 @@ AmdSevDxeEntryPoint ( > ); > ASSERT_EFI_ERROR (Status); > > + // SEV-SNP support does not automatically imply unaccepted memory support, > + // so make ExitBootServices accept all unaccepted memory if support is > + // not communicated. > + Status = gBS->CreateEventEx ( > + EVT_NOTIFY_SIGNAL, > + TPL_CALLBACK, > + ResolveUnacceptedMemory, > + NULL, > + &gEfiEventBeforeExitBootServicesGuid, > + &mAcceptAllMemoryEvent > + ); > + > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "AllowUnacceptedMemory event creation for EventBeforeExitBootServices failed.\n")); > + } > + > // > // If its SEV-SNP active guest then install the CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB. > // It contains the location for both the Secrets and CPUID page. > diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf > index cd1b686c53..5b443d45bc 100644 > --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf > +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf > @@ -52,6 +52,7 @@ > > [Guids] > gConfidentialComputingSevSnpBlobGuid > + gEfiEventBeforeExitBootServicesGuid > > [Pcd] > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId > -- > 2.39.1.456.gfc5497dd1b-goog >