Re: [edk2-devel] [PATCH v9 43/46] OvmfPkg: Use the SEV-ES work area for the SEV-ES AP reset vector

["Dong, Eric" <eric.dong@intel.com>]

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Lendacky, Thomas
> Sent: Thursday, June 18, 2020 10:51 PM
> To: Dong, Eric <eric.dong@intel.com>; devel@edk2.groups.io
> Cc: Brijesh Singh <brijesh.singh@amd.com>; Ard Biesheuvel
> <ard.biesheuvel@arm.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
> Laszlo Ersek <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: Re: [edk2-devel] [PATCH v9 43/46] OvmfPkg: Use the SEV-ES work
> area for the SEV-ES AP reset vector
> 
> On 6/18/20 2:43 AM, Dong, Eric wrote:
> > Hi Tom,
> 
> Hi Eric,
> 
> >
> > We use GCC5 to build the OVMF platform and report below errors, please
> help to check and fix it.
> 
> That's what I use when I build and have never encountered these errors. I
> also ran the patches through the EDK2 CI and didn't get any errors.
> 
> I've noticed that the dependencies aren't always handle properly for these
> files. Are you doing a clean build or an incremental build for this patch?
> Can you delete your Build directory and rebuild and see if you still get the
> errors?

I trig internal server build with your change and the build server found this issue. 
I think it should use clean build.

> 
> >
> >
> > I/opt/TCAgent/work/f9b29f3e80472c44/Edk2/MdeModulePkg/
> > -I/opt/TCAgent/work/f9b29f3e80472c44/Edk2/MdeModulePkg/Include/
> > -I/opt/TCAgent/work/f9b29f3e80472c44/Edk2/UefiCpuPkg/
> > -I/opt/TCAgent/work/f9b29f3e80472c44/Edk2/UefiCpuPkg/Include/
> > -
> I/opt/TCAgent/work/f9b29f3e80472c44/Edk2/UefiCpuPkg/ResetVector/Vtf0
> /
> > -o
> >
> /opt/TCAgent/work/f9b29f3e80472c44/Edk2/Build/OvmfX64/DEBUG_GCC5/
> X64/O
> > vmfPkg/ResetVector/ResetVector/OUTPUT/ResetVector.bin
> >
> /opt/TCAgent/work/f9b29f3e80472c44/Edk2/Build/OvmfX64/DEBUG_GCC5/
> X64/O
> > vmfPkg/ResetVector/ResetVector/OUTPUT/ResetVector.iii
> >
> /opt/TCAgent/work/f9b29f3e80472c44/Edk2/Build/OvmfX64/DEBUG_GCC5/
> X64/O
> > vmfPkg/ResetVector/ResetVector/OUTPUT/ResetVector.iii:72: error:
> > expression syntax error
> >
> /opt/TCAgent/work/f9b29f3e80472c44/Edk2/Build/OvmfX64/DEBUG_GCC5/
> X64/O
> > vmfPkg/ResetVector/ResetVector/OUTPUT/ResetVector.iii:74: error: label
> > or instruction expected at start of line > Ia32/PageTables64.asm:27:
> > error: label or instruction expected at start
> of line
> > Ia32/PageTables64.asm:29: error: label or instruction expected at
> > start of line
> > Ia32/PageTables64.asm:30: error: label or instruction expected at
> > start of line
> > Ia32/PageTables64.asm:369: error: expression syntax error
> 
> Can you paste the relevant portion of these files in an email or send copies of
> those files to me?

Because the build is in internal sever and I can't copy the build file from it.

If you think you have pass all the test, I can rerun the test and check the result again.

Thanks,
Eric
> 
> Thanks,
> Tom
> 
> > GNUmakefile:319: recipe for target
> >
> '/opt/TCAgent/work/f9b29f3e80472c44/Edk2/Build/OvmfX64/DEBUG_GCC5/
> X64/
> > OvmfPkg/ResetVector/ResetVector/OUTPUT/ResetVector.bin' failed
> > make: ***
> >
> [/opt/TCAgent/work/f9b29f3e80472c44/Edk2/Build/OvmfX64/DEBUG_GCC5
> /X64/
> > OvmfPkg/ResetVector/ResetVector/OUTPUT/ResetVector.bin] Error 1
> >
> >
> > build.py...
> >   : error 7000: Failed to execute command
> >   make tbuild
> >
> [/opt/TCAgent/work/f9b29f3e80472c44/Edk2/Build/OvmfX64/DEBUG_GCC5
> /X64/
> > OvmfPkg/ResetVector/ResetVector]
> >
> >
> > build.py...
> >   : error F002: Failed to build module
> >
> >
> /opt/TCAgent/work/f9b29f3e80472c44/Edk2/OvmfPkg/ResetVector/ResetV
> ecto
> > r.inf [X64, GCC5, DEBUG]
> >
> > ====================================================
> > Related platform build configuration like below:
> >
> > WORKSPACE        = /opt/TCAgent/work/f9b29f3e80472c44/Edk2
> > EDK_TOOLS_PATH   =
> /opt/TCAgent/work/f9b29f3e80472c44/Edk2/BaseTools
> > CONF_PATH        = /opt/TCAgent/work/f9b29f3e80472c44/Edk2/Conf
> > PYTHON_COMMAND   = /usr/bin/python3.5
> >
> > Architecture(s)  = X64
> > Build target     = DEBUG
> > Toolchain        = GCC5
> >
> > Active Platform          =
> /opt/TCAgent/work/f9b29f3e80472c44/Edk2/OvmfPkg/OvmfPkgX64.dsc
> >
> >
> > Thanks,
> > Eric
> >> -----Original Message-----
> >> From: Tom Lendacky <thomas.lendacky@amd.com>
> >> Sent: Friday, June 5, 2020 9:28 PM
> >> To: devel@edk2.groups.io
> >> Cc: Brijesh Singh <brijesh.singh@amd.com>; Ard Biesheuvel
> >> <ard.biesheuvel@arm.com>; Dong, Eric <eric.dong@intel.com>; Justen,
> >> Jordan L <jordan.l.justen@intel.com>; Laszlo Ersek
> >> <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>; Kinney,
> >> Michael D <michael.d.kinney@intel.com>; Ni, Ray <ray.ni@intel.com>
> >> Subject: [PATCH v9 43/46] OvmfPkg: Use the SEV-ES work area for the
> >> SEV- ES AP reset vector
> >>
> >> BZ:
> >>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbug
> >>
> zilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&amp;data=02%7C01%7Ct
> ho
> >>
> mas.lendacky%40amd.com%7C68b736db1acf4493c24308d8135b4a75%7C3dd
> 8961fe
> >>
> 4884e608e11a82d994e183d%7C0%7C0%7C637280630097752935&amp;sdata=
> R8sYtS
> >> a7bQXgeW4tiSAxDdwcoZaldq4%2BoN2Xn378JMc%3D&amp;reserved=0
> >>
> >> A hypervisor is not allowed to update an SEV-ES guest's 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.
> >>
> >> Use the SEV-ES work area for the 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.
> >>
> >> This new OVMF source file, ResetVectorVtf0.asm, is used in place of
> >> the original file through the use of the include path order set in
> >> OvmfPkg/ResetVector/ResetVector.inf under "[BuildOptions]".
> >>
> >> Cc: Jordan Justen <jordan.l.justen@intel.com>
> >> Cc: Laszlo Ersek <lersek@redhat.com>
> >> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> >> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> >> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> >> ---
> >>   OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 100
> >> ++++++++++++++++++++
> >>   OvmfPkg/ResetVector/ResetVector.nasmb        |   1 +
> >>   2 files changed, 101 insertions(+)
> >>
> >> diff --git a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
> >> b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
> >> new file mode 100644
> >> index 000000000000..980e0138e7fe
> >> --- /dev/null
> >> +++ b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
> >> @@ -0,0 +1,100 @@
> >> +;-------------------------------------------------------------------
> >> +-----------
> >>
> >> +; @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>
> >>
> >> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> >>
> >> +;
> >>
> >> +;-------------------------------------------------------------------
> >> +-----------
> >>
> >> +
> >>
> >> +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
> >>
> >> +;
> >>
> >> +; 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:
> >>
> >> +
> >>
> >> +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
> >>
> >> +;
> >>
> >> +    nop
> >>
> >> +    nop
> >>
> >> +    jmp     EarlyBspInitReal16
> >>
> >> +
> >>
> >> +ALIGN   16
> >>
> >> +
> >>
> >> +fourGigabytes:
> >>
> >> +
> >>
> >> diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb
> >> b/OvmfPkg/ResetVector/ResetVector.nasmb
> >> index 762661115d50..4913b379a993 100644
> >> --- a/OvmfPkg/ResetVector/ResetVector.nasmb
> >> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb
> >> @@ -82,5 +82,6 @@
> >>
> >>
> >>   %include "Main.asm"
> >>
> >>
> >>
> >> +  %define SEV_ES_AP_RESET_IP  FixedPcdGet32
> (PcdSevEsWorkAreaBase)
> >>
> >>   %include "Ia16/ResetVectorVtf0.asm"
> >>
> >>
> >>
> >> --
> >> 2.27.0
> >
> 
>