Re: [PATCH v2 4/6] OvmfPkg: create a SEV secret area in the AmdSev memfd

["Laszlo Ersek" <lersek@redhat.com>]

On 11/20/20 19:45, James Bottomley wrote:
> SEV needs an area to place an injected secret where OVMF can find it
> and pass it up as a ConfigurationTable.  This patch implements the
> area itself as an addition to the SEV enhanced reset vector table using
> an additional guid (4c2eb361-7d9b-4cc3-8081-127c90d3d294).
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3077
> Signed-off-by: James Bottomley <jejb@linux.ibm.com>
> 
> ---
> 
> v2: move guid to OVMF token space, separate patches
> ---
>  OvmfPkg/OvmfPkg.dec                          |  6 ++++++
>  OvmfPkg/ResetVector/ResetVector.inf          |  4 ++++
>  OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 14 ++++++++++++++
>  OvmfPkg/ResetVector/ResetVector.nasmb        |  2 ++
>  4 files changed, 26 insertions(+)
> 
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index 3fbf7a0ee1a4..7d27f8e16040 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -304,6 +304,12 @@ [PcdsFixedAtBuild]
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|0|UINT32|0x40
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize|0|UINT32|0x41
>  
> +  ## The base address and size of the SEV Launch Secret Area provisioned
> +  #  after remote attestation.  If this is set in the .fdf, the platform
> +  #  is responsible for protecting the area from DXE phase overwrites.
> +  gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase|0x0|UINT32|0x42
> +  gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize|0x0|UINT32|0x43
> +
>  [PcdsDynamic, PcdsDynamicEx]
>    gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10
> diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/ResetVector.inf
> index a53ae6c194ae..dc38f68919cd 100644
> --- a/OvmfPkg/ResetVector/ResetVector.inf
> +++ b/OvmfPkg/ResetVector/ResetVector.inf
> @@ -43,3 +43,7 @@ [Pcd]
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
> +
> +[FixedPcd]
> +  gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase
> +  gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize
> diff --git a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
> index baf9d09f3625..8d6600f17310 100644
> --- a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
> +++ b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
> @@ -47,6 +47,20 @@ TIMES (32 - ((guidedStructureEnd - guidedStructureStart) % 32)) DB 0
>  ;
>  guidedStructureStart:
>  
> +;
> +; SEV Secret block Guid: 4c2eb361-7d9b-4cc3-8081-127c90d3d294
> +;
> +; This describes the guest ram area where the hypervisor may have
> +; injected the secret

(1) I suggest s/may have injected/should inject/, as this structure gets
built into the uncompressed part of the pflash, and its intended
consumer is the hypervisor.

If you'd like to stick with the wording as posted, that's fine too, however.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo


> +;
> +sevSecretBlockStart:
> +    DD      SEV_LAUNCH_SECRET_BASE
> +    DD      SEV_LAUNCH_SECRET_SIZE
> +    DW      sevSecretBlockEnd - sevSecretBlockStart
> +    DB      0x61, 0xB3, 0x2E, 0x4C, 0x9B, 0x7D, 0xC3, 0x4C
> +    DB      0x80, 0x81, 0x12, 0x7C, 0x90, 0xD3, 0xD2, 0x94
> +sevSecretBlockEnd:
> +
>  ;
>  ; SEV-ES Processor Reset support
>  ;
> diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
> index 4913b379a993..c5e0fe93abf4 100644
> --- a/OvmfPkg/ResetVector/ResetVector.nasmb
> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb
> @@ -83,5 +83,7 @@
>  %include "Main.asm"
>  
>    %define SEV_ES_AP_RESET_IP  FixedPcdGet32 (PcdSevEsWorkAreaBase)
> +  %define SEV_LAUNCH_SECRET_BASE  FixedPcdGet32 (PcdSevLaunchSecretBase)
> +  %define SEV_LAUNCH_SECRET_SIZE  FixedPcdGet32 (PcdSevLaunchSecretSize)
>  %include "Ia16/ResetVectorVtf0.asm"
>  
>