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

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

On 3/30/22 14:31, Dov Murik wrote:
> 
> 
> On 30/03/2022 22:27, Brijesh Singh wrote:
>>
>>
>> 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.
>>
> 
> Gerd actually wants to change SNP_SEC_MEM_BASE_DESC_3 to start from
> PcdOvmfSecPeiTempRamBase, which is 0x010000.
> 
> Supposedly no one uses the single page at 0x00F000 .

Yes, that should be alright as long as the SNP_SEC_MEM_BASE_DESC_3 start 
from PcdOvmfSecPeiTempRamBase. In PEI phase, we validate all the 
unvalidated range. So, as long as SEC phase is not using 800F000 - 
8010000 we should be good. The PEI will validate that page.

thanks