[edk2-devel] [PATCH 3/4] OvmfPkg: create a SEV secret area in the AmdSev memfd
Laszlo Ersek
lersek at redhat.com
Thu Nov 19 07:51:25 UTC 2020
On 11/18/20 21:39, Tom Lendacky wrote:
> On 11/16/20 4:46 PM, Laszlo Ersek via groups.io wrote:
>> On 11/12/20 01:13, 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. The
>>> reset vector scheme allows additions but not removals. If the size of
>>> the reset vector is 22, it only contains the AP reset IP, but if it is
>>> 30 (or greater) it contains the SEV secret page location and size.
>>>
>>> Signed-off-by: James Bottomley <jejb at linux.ibm.com>
>>> ---
>>> OvmfPkg/OvmfPkg.dec | 5 +++++
>>> OvmfPkg/AmdSev/AmdSevX64.fdf | 3 +++
>>> OvmfPkg/ResetVector/ResetVector.inf | 4 ++++
>>> OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 4 ++++
>>> OvmfPkg/ResetVector/ResetVector.nasmb | 2 ++
>>> 5 files changed, 18 insertions(+)
>>>
>
> ...
>
>>> ;
>>> ; SEV-ES Processor Reset support
>>> ;
>>> ; sevEsResetBlock:
>>> ; For the initial boot of an AP under SEV-ES, the "reset" RIP must be
>>> ; programmed to the RAM area defined by SEV_ES_AP_RESET_IP. A known
>>> offset
>>> ; and GUID will be used to locate this block in the firmware and
>>> extract
>>> ; the build time RIP value. The GUID must always be 48 bytes from the
>>> ; end of the firmware.
>>> ;
>>> ; 0xffffffca (-0x36) - IP value
>>> ; 0xffffffcc (-0x34) - CS segment base [31:16]
>>> ; 0xffffffce (-0x32) - Size of the SEV-ES reset block
>>> ; 0xffffffd0 (-0x30) - SEV-ES reset block GUID
>>> ; (00f771de-1a7e-4fcb-890e-68c77e2fb44e)
>>> ;
>>> ; A hypervisor reads the CS segement base and IP value. The CS
>>> segment base
>>> ; value represents the high order 16-bits of the CS segment base,
>>> so the
>>> ; hypervisor must left shift the value of the CS segement base by
>>> 16 bits to
>>> ; form the full CS segment base for the CS segment register. It
>>> would then
>>> ; program the EIP register with the IP value as read.
>>> ;
>>>
>>> TIMES (32 - (sevEsResetBlockEnd - sevEsResetBlockStart)) DB 0
>>>
>>> sevEsResetBlockStart:
>>> DD SEV_ES_AP_RESET_IP
>>> DW sevEsResetBlockEnd - sevEsResetBlockStart
>>> DB 0xDE, 0x71, 0xF7, 0x00, 0x7E, 0x1A, 0xCB, 0x4F
>>> DB 0x89, 0x0E, 0x68, 0xC7, 0x7E, 0x2F, 0xB4, 0x4E
>>> sevEsResetBlockEnd:
>>
>> I'm not exactly sure why we added the padding (TIMES ... DB 0) in edk2
>> commit 30937f2f98c4 ("OvmfPkg: Use the SEV-ES work area for the SEV-ES
>> AP reset vector", 2020-08-17). I can imagine it was *already* for the
>> same purpose -- to deterministically terminate the above-described
>> backwards-traversal of the GUID-ed structures (and at the same time
>> remain aligned to 32 bytes, regarding the cumulative size of all
>> provided structures).
>
> The padding is required to "push" the GUID into the proper location at
> exactly 48 bytes from the end of the file. Without the padding, the GUID
> doesn't line up correctly and can't be located.
OK, thanks! So I think that my proposed movement / reworking of the
prepended padding should be fine.
Laszlo
>
> Thanks,
> Tom
>
>>
>> So, in that vein, I'd propose something like this (relative to master @
>> d448574e7310):
>>
>>> diff --git a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
>>> b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
>>> index 980e0138e7fe..957356ff997e 100644
>>> --- a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
>>> +++ b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
>>> @@ -16,55 +16,83 @@ ALIGN 16
>>> ; Pad the image size to 4k when page tables are in VTF0
>>> ;
>>> ; If the VTF0 image has page tables built in, then we need to make
>>> ; sure the end of VTF0 is 4k above where the page tables end.
>>> ;
>>> ; This is required so the page tables will be 4k aligned when VTF0 is
>>> ; located just below 0x100000000 (4GB) in the firmware device.
>>> ;
>>> %ifdef ALIGN_TOP_TO_4K_FOR_PAGING
>>> TIMES (0x1000 - ($ - EndOfPageTables) - 0x20) DB 0
>>> %endif
>>>
>>> +;
>>> +; pre-pad the sequence of GUIDed structures to a multiple of 32 bytes
>>> +;
>>> +TIMES (31 - (guidedStructuresEnd - guidedStructuresStart + 31) % 32)
>>> DB 0
>>> +
>>> +guidedStructuresStart:
>>> +;
>>> +; Zero GUID to terminate decreasing address order traversal.
>>> +;
>>> +TIMES 16 DB 0
>>> +
>>> +;
>>> +; Expose the location of the SEV Launch Secret area to the hypervisor
>>> +; (necessary when using the remote attestation firmware platform).
>>> +;
>>> +; sevLaunchSecretDescriptor:
>>> +; This GUIDed structure is chained in decreasing address order from
>>> +; sevEsResetBlock. It describes the guest RAM area where the
>>> hypervisor has
>>> +; to securely inject the SEV Launch Secret. The GUID is
>>> +; 78C93F1E-ADBC-4259-B92B-CE81E523FBC4.
>>> +;
>>> +sevLaunchSecretDescriptorStart:
>>> + DD SEV_LAUNCH_SECRET_BASE
>>> + DD SEV_LAUNCH_SECRET_SIZE
>>> + DW sevLaunchSecretDescriptorEnd -
>>> sevLaunchSecretDescriptorStart
>>> + DB 0x1E, 0x3F, 0xC9, 0x78, 0xBC, 0xAD, 0x59, 0x42
>>> + DB 0xB9, 0x2B, 0xCE, 0x81, 0xE5, 0x23, 0xFB, 0xC4
>>> +sevLaunchSecretDescriptorEnd:
>>> +
>>> ;
>>> ; SEV-ES Processor Reset support
>>> ;
>>> ; sevEsResetBlock:
>>> ; For the initial boot of an AP under SEV-ES, the "reset" RIP
>>> must be
>>> ; programmed to the RAM area defined by SEV_ES_AP_RESET_IP. A
>>> known offset
>>> ; and GUID will be used to locate this block in the firmware and
>>> extract
>>> ; the build time RIP value. The GUID must always be 48 bytes from
>>> the
>>> ; end of the firmware.
>>> ;
>>> ; 0xffffffca (-0x36) - IP value
>>> ; 0xffffffcc (-0x34) - CS segment base [31:16]
>>> ; 0xffffffce (-0x32) - Size of the SEV-ES reset block
>>> ; 0xffffffd0 (-0x30) - SEV-ES reset block GUID
>>> ; (00f771de-1a7e-4fcb-890e-68c77e2fb44e)
>>> ;
>>> ; A hypervisor reads the CS segement base and IP value. The CS
>>> segment base
>>> ; value represents the high order 16-bits of the CS segment base,
>>> so the
>>> ; hypervisor must left shift the value of the CS segement base by
>>> 16 bits to
>>> ; form the full CS segment base for the CS segment register. It
>>> would then
>>> ; program the EIP register with the IP value as read.
>>> ;
>>>
>>> -TIMES (32 - (sevEsResetBlockEnd - sevEsResetBlockStart)) DB 0
>>> -
>>> sevEsResetBlockStart:
>>> DD SEV_ES_AP_RESET_IP
>>> DW sevEsResetBlockEnd - sevEsResetBlockStart
>>> DB 0xDE, 0x71, 0xF7, 0x00, 0x7E, 0x1A, 0xCB, 0x4F
>>> DB 0x89, 0x0E, 0x68, 0xC7, 0x7E, 0x2F, 0xB4, 0x4E
>>> sevEsResetBlockEnd:
>>> +guidedStructuresEnd:
>>>
>>> ALIGN 16
>>>
>>> applicationProcessorEntryPoint:
>>> ;
>>> ; Application Processors entry point
>>> ;
>>> ; GenFv generates code aligned on a 4k boundary which will jump to
>>> this
>>> ; location. (0xffffffe0) This allows the Local APIC Startup IPI
>>> to be
>>> ; used to wake up the application processors.
>>> ;
>>> jmp EarlyApInitReal16
>>
>> Back to your patch:
>>
>> On 11/12/20 01:13, James Bottomley wrote:
>>> diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb
>>> b/OvmfPkg/ResetVector/ResetVector.nasmb
>>> index 4913b379a9..c5e0fe93ab 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"
>>>
>>>
>>
>> OK.
>>
>> Thanks,
>> Laszlo
>>
>>
>>
>>
>>
>>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67694): https://edk2.groups.io/g/devel/message/67694
Mute This Topic: https://groups.io/mt/78198620/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list