[edk2-devel] [RFC PATCH v2 37/44] OvmfPkg: Add support for SEV-ES AP reset vector re-directing
Lendacky, Thomas
thomas.lendacky at amd.com
Wed Oct 2 17:33:44 UTC 2019
On 10/2/19 9:54 AM, Laszlo Ersek wrote:
> On 09/19/19 21:53, Lendacky, Thomas wrote:
>> From: Tom Lendacky <thomas.lendacky at amd.com>
>>
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
>>
>> A hypervisor is not allowed to update an SEV-ES guests register state,
>> so when booting an SEV-ES guest AP, the hypervisor is not allowed to
>> set the RIP to the guest requested value. Instead an SEV-ES AP must be
>> re-directed from within the guest to the actual requested staring location
>> as specified in the INIT-SIPI-SIPI sequence.
>>
>> Provide reset vector code that contains support to jump to the desired
>> RIP location after having been started. This is required for only the
>> very first AP reset.
>
> (1) In the commit message, can you mention the build mechanism by which
> this file overrides the original in UefiCpuPkg?
>
> Is it due to include path order?
Yes, this is due to the BuildOptions include path order. I'll update the
commit message.
>
>>
>> Cc: Jordan Justen <jordan.l.justen at intel.com>
>> Cc: Laszlo Ersek <lersek at redhat.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel at linaro.org>
>> Signed-off-by: Tom Lendacky <thomas.lendacky at amd.com>
>> ---
>> OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 80 ++++++++++++++++++++
>> 1 file changed, 80 insertions(+)
>> create mode 100644 OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
>>
>> diff --git a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
>> new file mode 100644
>> index 000000000000..1ac8b7ca7e85
>> --- /dev/null
>> +++ b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
>> @@ -0,0 +1,80 @@
>> +;------------------------------------------------------------------------------
>> +; @file
>> +; First code executed by processor after resetting.
>> +; Derived from UefiCpuPkg/ResetVector/Vtf0/Ia16/ResetVectorVtf0.asm
>> +;
>> +; Copyright (c) 2019, AMD Inc. All rights reserved.<BR>
>> +; SPDX-License-Identifier: BSD-2-Clause-Patent
>
> (2) Thanks for the "derived from" hint -- but in that case, you should
> probably keep the original copyright notice too.
Ok, will do.
>
>> +;
>> +;------------------------------------------------------------------------------
>> +
>> +BITS 16
>> +
>> +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
>> +
>> +;
>> +; SEV-ES Processor Reset support
>> +;
>> +; standardProcessorSevEsReset: (0xffffffd0)
>> +; When using the Application Processors entry point, always perform a
>> +; far jump to the RIP/CS value contained at this location. This will
>> +; default to EarlyBspInitReal16 unless specifically overridden.
>> +
>> +standardProcessorSevEsReset:
>> + DW 0x0000
>> + DW 0x0000
>> +
>> +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
>> +
>> +ALIGN 8
>> +
>> + DD 0
>> +
>> +;
>> +; The VTF signature
>> +;
>> +; VTF-0 means that the VTF (Volume Top File) code does not require
>> +; any fixups.
>> +;
>> +vtfSignature:
>> + DB 'V', 'T', 'F', 0
>> +
>> +ALIGN 16
>> +
>> +resetVector:
>> +;
>> +; Reset Vector
>> +;
>> +; This is where the processor will begin execution
>> +;
>> + cmp dword [CS:0xFFD0], 0
>> + je EarlyBspInitReal16
>> + jmp far [CS:0xFFD0]
>> +
>> +ALIGN 16
>> +
>> +fourGigabytes:
>> +
>>
>
> It's worth looking at this patch as a diff against the original:
>
>> --- UefiCpuPkg/ResetVector/Vtf0/Ia16/ResetVectorVtf0.asm 2019-09-25 17:09:42.856850422 +0200
>> +++ OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm 2019-10-02 16:40:55.906630335 +0200
>> @@ -1,8 +1,9 @@
>> ;------------------------------------------------------------------------------
>> ; @file
>> ; First code executed by processor after resetting.
>> +; Derived from UefiCpuPkg/ResetVector/Vtf0/Ia16/ResetVectorVtf0.asm
>> ;
>> -; Copyright (c) 2008 - 2014, Intel Corporation. All rights reserved.<BR>
>> +; Copyright (c) 2019, AMD Inc. All rights reserved.<BR>
>> ; SPDX-License-Identifier: BSD-2-Clause-Patent
>> ;
>> ;------------------------------------------------------------------------------
>> @@ -24,6 +25,20 @@
>> TIMES (0x1000 - ($ - EndOfPageTables) - 0x20) DB 0
>> %endif
>>
>> +;
>> +; SEV-ES Processor Reset support
>> +;
>> +; standardProcessorSevEsReset: (0xffffffd0)
>> +; When using the Application Processors entry point, always perform a
>> +; far jump to the RIP/CS value contained at this location. This will
>> +; default to EarlyBspInitReal16 unless specifically overridden.
>> +
>> +standardProcessorSevEsReset:
>> + DW 0x0000
>> + DW 0x0000
>> +
>> +ALIGN 16
>> +
>> applicationProcessorEntryPoint:
>> ;
>> ; Application Processors entry point
>> @@ -55,9 +70,9 @@
>> ;
>> ; This is where the processor will begin execution
>> ;
>> - nop
>> - nop
>> - jmp EarlyBspInitReal16
>> + cmp dword [CS:0xFFD0], 0
>> + je EarlyBspInitReal16
>> + jmp far [CS:0xFFD0]
>>
>> ALIGN 16
>>
>
> (3) Can't we / shouldn't we implement this change in the original,
> actually? The new code doesn't seem to hurt if it's not activated, and
> it doesn't complicate the code much.
If there are no objections, that can be done. It did concern me that there
are a couple of nop instructions before the jmp (which replaced a WBINVD)
and that there might be a reason for them being there. Based on that, I
just created the new file specific for OVMF.
>
> (4) Can we use "standardProcessorSevEsReset" in place of the constant
> 0xFFD0 somehow?
I'll look into that. I know "CS:standardProcessorSevEsReset" won't work
and I tried a bunch of different things, but there may be another way.
Thanks,
Tom
>
> Thanks
> Laszlo
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#48387): https://edk2.groups.io/g/devel/message/48387
Mute This Topic: https://groups.io/mt/34203584/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