Re: [PATCH 2/2] OvmfPkg/ResetVector: Exclude SEV launch secrets page from pre-validation

["Brijesh Singh" <brijesh.singh@amd.com>]

On 3/30/22 01:04, Dov Murik wrote:
> 
> 
> On 30/03/2022 8:20, Gerd Hoffmann wrote:
>>    Hi,
>>
>>> Check if that page is defined; if it is, skip it in the metadata list.
>>> In such case, VMM should fill the page with the hashes content, or
>>> explicitly update it as a zero page (if kernel hashes are not used).
>>
>> Is it an option to just skip the page unconditionally?
>>
>> I think in the OvmfPkgX64 build the page is not used, so it probably
>> doesn't matter whenever it is included or not, and it would make things
>> a bit less confusing ...
>>
> 
> 
> Brijesh,
> 
> What would happen if we change this:
> 
>      %define SNP_SEC_MEM_BASE_DESC_3 (CPUID_BASE + CPUID_SIZE)
> 
> to:
> 
>      %define SNP_SEC_MEM_BASE_DESC_3 (FixedPcdGet32 (PcdOvmfSecPeiTempRamBase))
> 
> in OvmfPkg/ResetVector/ResetVector.nasmb ?
> 
> It means that the page starting at MEMFD_BASE_ADDRESS+0x00F000 (that is, the page
> that follows the SNP CPUID page) will not be pre-validated by QEMU.
> 

Lets look at the OvmfPkgX64.fdf is

...

0x00E000|0x001000 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidSize

0x010000|0x010000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize

0x020000|0x0E0000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize

...

If you change SNP_SEC_MEM_BASE_DESC_3 to start from PcdOvmfPeiMemFvBase 
then who will validate the range for  PcdOvmfSecPeiTempRamBase - 
PcdOvmfPeiMemFvBase ? The SEC phase (Sec/X64/SecEntry.nasm) uses the 
PcdOvmfSecPeiTempRamBase. If the memory is not validated prior to use 
then it will result in #VC (page-not-validated) and crash the guest BIOS 
boot.