On 09/19/19 21:52, Lendacky, Thomas wrote:
> From: Tom Lendacky <[email protected]>
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
>
> A per-CPU implementation for holding values specific to a CPU when
> running as an SEV-ES guest, specifically to hold the Debug Register
> value. Allocate an extra page immediately after the GHCB page for each
> AP.
>
> Using the page after the GHCB ensures that it is unique per AP. But,
> it also ends up being marked shared/unencrypted when it doesn't need to
> be. It is possible during PEI to mark only the GHCB pages as shared (and
> that is done), but DXE is not as easy. There needs to be a way to change
> the pagetables created for DXE using CreateIdentityMappingPageTables()
> before switching to them.
>
> Cc: Jordan Justen <[email protected]>
> Cc: Laszlo Ersek <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Signed-off-by: Tom Lendacky <[email protected]>
> ---
> OvmfPkg/OvmfPkgX64.fdf | 2 +-
> OvmfPkg/PlatformPei/AmdSev.c | 2 +-
> OvmfPkg/ResetVector/ResetVector.nasmb | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index a567131a0591..84716952052d 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -79,7 +79,7 @@ [FD.MEMFD]
> 0x008000|0x001000
>
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize
>
> -0x009000|0x001000
> +0x009000|0x002000
>
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize
>
> 0x010000|0x010000
> diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
> index 30c0e4af7252..699bb8b11557 100644
> --- a/OvmfPkg/PlatformPei/AmdSev.c
> +++ b/OvmfPkg/PlatformPei/AmdSev.c
> @@ -48,7 +48,7 @@ AmdSevEsInitialize (
> //
> // Allocate GHCB pages.
> //
> - GhcbPageCount = mMaxCpuCount;
> + GhcbPageCount = mMaxCpuCount * 2;
> GhcbBase = AllocatePages (GhcbPageCount);
> ASSERT (GhcbBase);
>
> diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb
> b/OvmfPkg/ResetVector/ResetVector.nasmb
> index 8909fc9313f4..d7c0ab3ada00 100644
> --- a/OvmfPkg/ResetVector/ResetVector.nasmb
> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb
> @@ -57,7 +57,7 @@
> %error "This implementation inherently depends on
> PcdOvmfSecGhcbPageTableSize"
> %endif
>
> - %if (FixedPcdGet32 (PcdOvmfSecGhcbSize) != 0x1000)
> + %if (FixedPcdGet32 (PcdOvmfSecGhcbSize) != 0x2000)
> %error "This implementation inherently depends on PcdOvmfSecGhcbSize"
> %endif
>
>
In connection to my question at [1]:
* Why do we add the extra page to SEC as well?
I thought that, after patch 4 ("OvmfPkg/ResetVector: Add support for a
32-bit SEV check"), we were all set for handling #VC, for the time of
the initial SEV check; furthermore, that only CPUID would cause a #VC.
If that's the case, when exactly would be the new page (at 0x80_a000)
be used?
* Assuming we really need PcdOvmfSecGhcbSize = 0x002000, it is now
theoretically possible that the 8KB area straddles a 2MB page
boundary.
Obviously we don't want to accommodate that corner case, but we should
catch it. I think we should enforce -- with an %if / %error --
something like:
(FixedPcdGet32 (PcdOvmfSecGhcbBase) >> 21) ==
((FixedPcdGet32 (PcdOvmfSecGhcbBase) + FixedPcdGet32 (PcdOvmfSecGhcbSize) -
1) >> 21)
That sanity check is likely best to squash into patch 6 ("OvmfPkg:
Create a GHCB page for use during Sec phase").
[1] [email protected]">http://mid.mail-archive.com/[email protected]
https://edk2.groups.io/g/devel/message/48080
Thanks!
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#48081): https://edk2.groups.io/g/devel/message/48081
Mute This Topic: https://groups.io/mt/34203546/21656
Mute #vc: https://groups.io/mk?hashtag=vc&subid=3846945
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
