On 30/03/2022 8:14, Gerd Hoffmann wrote:
> On Tue, Mar 29, 2022 at 03:32:36PM +0300, Dov Murik wrote:
>> Thanks Gerd for reviewing.
>>
>> On 29/03/2022 14:36, Gerd Hoffmann wrote:
>>> On Mon, Mar 28, 2022 at 06:45:29PM +0000, Dov Murik wrote:
>>>> Reorder the pages in the MEMFD section of AmdSevX64.fdf so that it
>>>> matches the same order used in OvmfPkgX64.fdf.
>>>
>>> Makes sense.
>>>
>>> Acked-by: Gerd Hoffmann <kra...@redhat.com>
>>
>> Thanks.
>>
>>>
>>> Maybe also move this to a common include file, so it is less likely that
>>> they run out of sync again?
>>>
>>
>> I was contemplating that, but wasn't sure (I only use OvmfPkgX64 and
>> AmdSevX64 in my testing).
>>
>> Is it common in edk2?
>
> We already have a few:
>
> kraxel@sirius ~/projects/edk2 (gcc12)# ls OvmfPkg/*.fdf.inc
> OvmfPkg/FvmainCompactScratchEnd.fdf.inc OvmfPkg/OvmfTpmDxe.fdf.inc
> OvmfPkg/VarStore.fdf.inc
> OvmfPkg/OvmfPkgDefines.fdf.inc OvmfPkg/OvmfTpmPei.fdf.inc
> OvmfPkg/XenElfHeader.fdf.inc
>
I saw these, but saw no !include directives in MEMFD areas, which are
more sensitive because the addresses and sizes must match the
surrounding definitions (unlike a list of INF directive like in
NetworkPkg/Network.fdf.inc or general settings like in
OvmfPkg/OvmfPkgDefines.fdf.inc).
>> Would it apply to other OvmfPkg targets? I see similar MEMFD in
>> CloudHvX64.fdf .
>
> I'd create one for the confidential computing memory areas,
> that would only affect the builds which support SEV (and soon TDX).
>
Almost all the MEMFD entries are somehow related to confidential
computing, isn't that the case? For example PcdOvmfWorkAreaBase -- I
see it appears in the *.fdf of almost all targets.
I want to reduce duplication (= extract common parts to an .inc file),
but wonder what would be a clear and safe way to do it.
Suggestions:
Option 1:
Extract all the MEMFD entries starting from:
0x000000|0x006000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
up to (including):
0x00E000|0x001000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidSize
into OvmfMemFdPart1.fdf.inc, and !include it in OvmfPkgX64 and AmdSevX64.
Option 2:
Extract entire MEMFD part from OvmfPkgX64.fdf into OvmfMemFd.fdf.inc.
In the middle of it add something like:
!if $(SEV_LAUNCH_SECRET_ENABLE) == TRUE
0x00F000|0x000C00
gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase|gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize
0x00FC00|0x000400
gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableSize
!endif
and set that DEFINE in AmdSevX64.fdf only.
> Not sure about CloudHvX64.fdf, as far I know it does not support
> SEV/TDX, maybe those antries are only there because they have been
> copied over from OvmfPkgX64.fdf
The TDX series ("[PATCH V12 00/47] Enable Intel TDX in OvmfPkg
(Config-A)") modifies CloudHvX64.*, and also the CloudHv/README mentions
TDX. So I assume they intend to support it.
-Dov
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#88253): https://edk2.groups.io/g/devel/message/88253
Mute This Topic: https://groups.io/mt/90092200/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
