Re: [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and Cpuid page for the SEV-SNP guest

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

On 04/19/21 23:42, Brijesh Singh wrote:
> 
> On 4/13/21 4:49 AM, Laszlo Ersek wrote:
>> On 04/12/21 16:52, Brijesh Singh wrote:
>>> Hi James and Laszlo,
>>>
>>> I was planning to work to add the support to reserve the Secrets and
>>> CPUID page in E820 map and then create the EFI configuration table entry
>>> for it so that guest OS can reach to it. We have two packages
>>> "SecretDxe" and "SecretPei" in OvmfPkg/AmdSev. Any issues if I use them
>>> in the OvmfPkg.dsc ? Here is what I was thinking:
>>>
>>> 1) Rename the PcdSevLaunchSecretBase -> PcdSevSecretsBase
>>>
>>> 2) When SNP is enabled then VMM use this page as secrets page for the SNP
>>>
>>> 3) When SEV or SEV-ES is enabled then VMM uses this page as a launch
>>> secret page
>>>
>>> This will allow me to drop PcdOvmfSnpSecretsBase. This will not just
>>> save 4-bytes but also minimize the code duplication.
>> I'm pretty unhappy about needing a separate page for each such purpose.
>> We're wasting room in MEMFD. The GUIDed structs that we expose to QEMU
>> seem to be flexible enough to describe non-page-aligned addresses,
>> right? Can we pack larger amounts of cruft into MEMFD pages?
>>
>> I'm not looking forward to the day when we run out of slack in MEMFD and
>> we get to shift PEIFV / DXEFV. (Every time we need to increase the DXEFV
>> size, the same risk exists -- which is why I've been thinking for a
>> while now that OVMF includes too many features already.) This can
>> introduce obscure changes to the UEFI memory map, which has caused
>> compat problems in the past, for example with the "crash" utility.
> 
> 
> What's your take to move all SEV-specific reserved pages at the end of
> PcdOvmfDecompressionScratchEnd ? I have not tried yet, but I can give
> try to make sure the ES works after such moves.

I don't recommend that, without a serious audit anyway. While we use
PCDs in the source code rather than open-coded constants, and so it's
relatively easy to change the value of a PCD, the source code may very
well rely on the *relations* between PCDs. If you change PCDs such that
their relations change (by moving around areas), things can break.

If you absolutely need more room, I'd prefer enlarging the currently
available gap in-place (pushing up everything that's currently above it).

> What is a general rule of thump to what goes in MEMFD ? Is this all the
> data pages accessed during the SEC phase ?

More or less; it lets us lay out ranges, with symbolic names, that are
needed very early (reset vector, SEC), during normal boot or ACPI S3
resume. If something can be allocated manually during PEI or later,
yielding a variable address, then it arguably doesn't beling on MEMFD.

> If so, then we probably can't
> do everything after the PcdOvmfDecompressionScratchEnd. The only thing
> which we can quickly move out is a secret page.

IIRC, PcdOvmfDecompressionScratchEnd stands for the highest RAM address
overwritten during ACPI S3 resume. I wouldn't like to put anything above
it through MEMFD.

Thanks
Laszlo

> 
> 
>> The feature creep in OVMF has gone off the rails in the last few years,
>> really. (Not that I'm not guilty myself.)
>>
>> Thanks,
>> Laszlo
>>
>