From: "Laszlo Ersek" <lersek@redhat.com>
To: Tom Lendacky <thomas.lendacky@amd.com>,
devel@edk2.groups.io, jejb@linux.ibm.com
Cc: dovmurik@linux.vnet.ibm.com, Dov.Murik1@il.ibm.com,
ashish.kalra@amd.com, brijesh.singh@amd.com, tobin@ibm.com,
david.kaplan@amd.com, jon.grimm@amd.com, frankeh@us.ibm.com,
"Dr . David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [edk2-devel] [PATCH 3/4] OvmfPkg: create a SEV secret area in the AmdSev memfd
Date: Thu, 19 Nov 2020 08:51:25 +0100 [thread overview]
Message-ID: <7cc1b7f4-a00b-798f-377a-48d3fadc44ad@redhat.com> (raw)
In-Reply-To: <76c21922-aaa5-4ae2-b4b9-055f0720d4ef@amd.com>
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@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
>>
>>
>>
>>
>>
>>
>
next prev parent reply other threads:[~2020-11-19 7:51 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-12 0:13 [PATCH 0/4] SEV Encrypted Boot for Ovmf James Bottomley
2020-11-12 0:13 ` [PATCH 1/4] OvmfPkg/Amdsev: Base commit to build encrypted boot specific OVMF James Bottomley
2020-11-16 19:11 ` [edk2-devel] " Laszlo Ersek
2020-11-16 20:00 ` James Bottomley
2020-11-12 0:13 ` [PATCH 2/4] OvmfPkg/AmdSev: add Grub Firmware Volume Package James Bottomley
2020-11-16 20:42 ` [edk2-devel] " Laszlo Ersek
2020-11-17 0:05 ` Laszlo Ersek
2020-11-18 23:00 ` James Bottomley
2020-11-19 7:59 ` Laszlo Ersek
2020-11-12 0:13 ` [PATCH 3/4] OvmfPkg: create a SEV secret area in the AmdSev memfd James Bottomley
2020-11-16 22:46 ` [edk2-devel] " Laszlo Ersek
2020-11-18 20:23 ` James Bottomley
2020-11-19 7:50 ` Laszlo Ersek
2020-11-19 19:41 ` Brijesh Singh
2020-11-20 6:29 ` jejb
2020-11-20 10:59 ` Laszlo Ersek
2020-11-18 20:39 ` Lendacky, Thomas
2020-11-19 7:51 ` Laszlo Ersek [this message]
2020-11-12 0:13 ` [PATCH 4/4] OvmfPkg/AmdSev: Expose the Sev Secret area using a configuration table James Bottomley
2020-11-17 0:12 ` [edk2-devel] " Laszlo Ersek
2020-11-12 16:21 ` [PATCH 0/4] SEV Encrypted Boot for Ovmf Ashish Kalra
2020-11-12 16:34 ` Dr. David Alan Gilbert
2020-11-12 17:07 ` James Bottomley
2020-11-12 17:22 ` Ashish Kalra
2020-11-12 17:32 ` Brijesh Singh
2020-11-12 19:38 ` Dr. David Alan Gilbert
2020-11-12 21:56 ` Brijesh Singh
2020-11-12 22:50 ` James Bottomley
2020-11-15 14:08 ` Brijesh Singh
2020-11-12 19:44 ` James Bottomley
2020-11-13 2:04 ` [edk2-devel] " James Bottomley
2020-11-13 22:41 ` Laszlo Ersek
2020-11-16 18:50 ` Laszlo Ersek
2020-11-16 18:56 ` Laszlo Ersek
2020-11-16 19:55 ` James Bottomley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7cc1b7f4-a00b-798f-377a-48d3fadc44ad@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox