Re: [edk2-devel] [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction
Tom: I meet with GCC failure on this patch. Can you help check it? If nasm doesn't support the vmmcall instruction in 32-bit mode, you have to use inline assembly to support it. Edk2/Build/IntelFsp2Pkg/DEBUG_GCC5/IA32/MdePkg/Library/BaseLib/BaseLib/OUTPUT/Ia32/VmgExit.iii:33: error: elf32 output format does not support 64-bit code GNUmakefile:741: recipe for target Thanks Liming -Original Message- From: Tom Lendacky Sent: 2020年7月27日 23:26 To: devel@edk2.groups.io Cc: Brijesh Singh ; Ard Biesheuvel ; Dong, Eric ; Justen, Jordan L ; Laszlo Ersek ; Gao, Liming ; Kinney, Michael D ; Ni, Ray Subject: [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction From: Tom Lendacky BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198 VMGEXIT is a new instruction used for Hypervisor/Guest communication when running as an SEV-ES guest. A VMGEXIT will cause an automatic exit (AE) to occur, resulting in a #VMEXIT with an exit code value of 0x403. Provide the necessary support to execute the VMGEXIT instruction, which is "rep; vmmcall". Cc: Michael D Kinney Cc: Liming Gao Signed-off-by: Tom Lendacky --- MdePkg/Library/BaseLib/BaseLib.inf | 2 ++ MdePkg/Include/Library/BaseLib.h | 14 + MdePkg/Library/BaseLib/Ia32/VmgExit.nasm | 37 MdePkg/Library/BaseLib/X64/VmgExit.nasm | 32 4 files changed, 85 insertions(+) create mode 100644 MdePkg/Library/BaseLib/Ia32/VmgExit.nasm create mode 100644 MdePkg/Library/BaseLib/X64/VmgExit.nasm diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf index 3b93b5db8d24..3b85c56c3c03 100644 --- a/MdePkg/Library/BaseLib/BaseLib.inf +++ b/MdePkg/Library/BaseLib/BaseLib.inf @@ -184,6 +184,7 @@ [Sources.Ia32] Ia32/DisableCache.nasm| GCC Ia32/RdRand.nasm Ia32/XGetBv.nasm + Ia32/VmgExit.nasm Ia32/DivS64x64Remainder.c Ia32/InternalSwitchStack.c | MSFT @@ -317,6 +318,7 @@ [Sources.X64] X64/DisablePaging64.nasm X64/RdRand.nasm X64/XGetBv.nasm + X64/VmgExit.nasm ChkStkGcc.c | GCC [Sources.EBC] diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h index 7edf0051a0a0..04fb329eaabb 100644 --- a/MdePkg/Include/Library/BaseLib.h +++ b/MdePkg/Include/Library/BaseLib.h @@ -7848,6 +7848,20 @@ AsmXGetBv ( ); +/** + Executes a VMGEXIT instruction (VMMCALL with a REP prefix) + + Executes a VMGEXIT instruction. This function is only available on + IA-32 and x64. + +**/ +VOID +EFIAPI +AsmVmgExit ( + VOID + ); + + /** Patch the immediate operand of an IA32 or X64 instruction such that the byte, word, dword or qword operand is encoded at the end of the instruction's diff --git a/MdePkg/Library/BaseLib/Ia32/VmgExit.nasm b/MdePkg/Library/BaseLib/Ia32/VmgExit.nasm new file mode 100644 index ..a4b37385cc7a --- /dev/null +++ b/MdePkg/Library/BaseLib/Ia32/VmgExit.nasm @@ -0,0 +1,37 @@ +;-- + +; +; Copyright (C) 2020, Advanced Micro Devices, Inc. All rights +reserved. ; SPDX-License-Identifier: BSD-2-Clause-Patent ; ; Module +Name: +; +; VmgExit.Asm +; +; Abstract: +; +; AsmVmgExit function +; +; Notes: +; +;-- + + +SECTION .text + +;-- + +; VOID +; EFIAPI +; AsmVmgExit ( +; VOID +; ); +;-- + +global ASM_PFX(AsmVmgExit) +ASM_PFX(AsmVmgExit): +; +; NASM doesn't support the vmmcall instruction in 32-bit mode, so work +around ; this by temporarily switching to 64-bit mode. +; +BITS64 +rep vmmcall +BITS32 +ret + diff --git a/MdePkg/Library/BaseLib/X64/VmgExit.nasm b/MdePkg/Library/BaseLib/X64/VmgExit.nasm new file mode 100644 index ..26f034593c67 --- /dev/null +++ b/MdePkg/Library/BaseLib/X64/VmgExit.nasm @@ -0,0 +1,32 @@ +;-- + +; +; Copyright (C) 2020, Advanced Micro Devices, Inc. All rights +reserved. ; SPDX-License-Identifier: BSD-2-Clause-Patent ; ; Module +Name: +; +; VmgExit.Asm +; +; Abstract: +; +; AsmVmgExit function +; +; Notes: +; +;-- + + +DEFAULT REL +SECTION .text + +;-- + +; VOID +; EFIAPI +; AsmVmgExit ( +; VOID +; ); +;-- + +global ASM_PFX(AsmVmgExit) +ASM_PFX(AsmVmgExit): +rep vmmcall +ret + -- 2.27.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#63369): https://edk2.groups.io/g/devel/message/63369 Mute This Topic: https:/
Re: [edk2-devel] [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction
On 7/27/20 8:34 PM, Gao, Liming wrote: Tom: Hi Liming, I meet with GCC failure on this patch. Can you help check it? If nasm doesn't support the vmmcall instruction in 32-bit mode, you have to use inline assembly to support it. What version of GCC are you using. I was able to successfully build the Ia32 version with my GCC level. The Ia32 version uses a trick to do switch to 64-bit just to encode the instruction. Looks like that doesn't work with your version of GCC. I can probably switch to defining the instruction as bytes. Let me look into that and possibly send you a patch to test. Thanks, Tom Edk2/Build/IntelFsp2Pkg/DEBUG_GCC5/IA32/MdePkg/Library/BaseLib/BaseLib/OUTPUT/Ia32/VmgExit.iii:33: error: elf32 output format does not support 64-bit code GNUmakefile:741: recipe for target Thanks Liming -Original Message- From: Tom Lendacky Sent: 2020年7月27日 23:26 To: devel@edk2.groups.io Cc: Brijesh Singh ; Ard Biesheuvel ; Dong, Eric ; Justen, Jordan L ; Laszlo Ersek ; Gao, Liming ; Kinney, Michael D ; Ni, Ray Subject: [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction From: Tom Lendacky BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&data=02%7C01%7Cthomas.lendacky%40amd.com%7C77c8250cd9e14f2929a008d832965726%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637314968570901400&sdata=6zqseI3tVmaw351w9mfEymMnDcjDzjvcBrhARU6r3Ho%3D&reserved=0 VMGEXIT is a new instruction used for Hypervisor/Guest communication when running as an SEV-ES guest. A VMGEXIT will cause an automatic exit (AE) to occur, resulting in a #VMEXIT with an exit code value of 0x403. Provide the necessary support to execute the VMGEXIT instruction, which is "rep; vmmcall". Cc: Michael D Kinney Cc: Liming Gao Signed-off-by: Tom Lendacky --- MdePkg/Library/BaseLib/BaseLib.inf | 2 ++ MdePkg/Include/Library/BaseLib.h | 14 + MdePkg/Library/BaseLib/Ia32/VmgExit.nasm | 37 MdePkg/Library/BaseLib/X64/VmgExit.nasm | 32 4 files changed, 85 insertions(+) create mode 100644 MdePkg/Library/BaseLib/Ia32/VmgExit.nasm create mode 100644 MdePkg/Library/BaseLib/X64/VmgExit.nasm diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf index 3b93b5db8d24..3b85c56c3c03 100644 --- a/MdePkg/Library/BaseLib/BaseLib.inf +++ b/MdePkg/Library/BaseLib/BaseLib.inf @@ -184,6 +184,7 @@ [Sources.Ia32] Ia32/DisableCache.nasm| GCC Ia32/RdRand.nasm Ia32/XGetBv.nasm + Ia32/VmgExit.nasm Ia32/DivS64x64Remainder.c Ia32/InternalSwitchStack.c | MSFT @@ -317,6 +318,7 @@ [Sources.X64] X64/DisablePaging64.nasm X64/RdRand.nasm X64/XGetBv.nasm + X64/VmgExit.nasm ChkStkGcc.c | GCC [Sources.EBC] diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h index 7edf0051a0a0..04fb329eaabb 100644 --- a/MdePkg/Include/Library/BaseLib.h +++ b/MdePkg/Include/Library/BaseLib.h @@ -7848,6 +7848,20 @@ AsmXGetBv ( ); +/** + Executes a VMGEXIT instruction (VMMCALL with a REP prefix) + + Executes a VMGEXIT instruction. This function is only available on + IA-32 and x64. + +**/ +VOID +EFIAPI +AsmVmgExit ( + VOID + ); + + /** Patch the immediate operand of an IA32 or X64 instruction such that the byte, word, dword or qword operand is encoded at the end of the instruction's diff --git a/MdePkg/Library/BaseLib/Ia32/VmgExit.nasm b/MdePkg/Library/BaseLib/Ia32/VmgExit.nasm new file mode 100644 index ..a4b37385cc7a --- /dev/null +++ b/MdePkg/Library/BaseLib/Ia32/VmgExit.nasm @@ -0,0 +1,37 @@ +;-- + +; +; Copyright (C) 2020, Advanced Micro Devices, Inc. All rights +reserved. ; SPDX-License-Identifier: BSD-2-Clause-Patent ; ; Module +Name: +; +; VmgExit.Asm +; +; Abstract: +; +; AsmVmgExit function +; +; Notes: +; +;-- + + +SECTION .text + +;-- + +; VOID +; EFIAPI +; AsmVmgExit ( +; VOID +; ); +;-- + +global ASM_PFX(AsmVmgExit) +ASM_PFX(AsmVmgExit): +; +; NASM doesn't support the vmmcall instruction in 32-bit mode, so work +around ; this by temporarily switching to 64-bit mode. +; +BITS64 +rep vmmcall +BITS32 +ret + diff --git a/MdePkg/Library/BaseLib/X64/VmgExit.nasm b/MdePkg/Library/BaseLib/X64/VmgExit.nasm new file mode 100644 index ..26f034593c67 --- /dev/null +++ b/MdePkg/Library/BaseLib/X64/VmgExit.nasm @@ -0,0 +1,32 @@ +;-- + +; +; Copyright (C) 2020, Advanced Micro Devices, Inc. All rights +reserved. ; SPDX-License-Identifier:
Re: [edk2-devel] [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction
This error is reported from nasm compiler. My nasm compiler version is 2.11.08. It may be a little old. 2.12 should be fine. This change also requires to update edk2\BaseTools\Conf\tools_def.template and mention nasm compiler version. Thanks Liming -Original Message- From: Tom Lendacky Sent: 2020年7月28日 12:08 To: Gao, Liming ; devel@edk2.groups.io Cc: Brijesh Singh ; Ard Biesheuvel ; Dong, Eric ; Justen, Jordan L ; Laszlo Ersek ; Kinney, Michael D ; Ni, Ray Subject: Re: [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction On 7/27/20 8:34 PM, Gao, Liming wrote: > Tom: Hi Liming, >I meet with GCC failure on this patch. Can you help check it? If nasm > doesn't support the vmmcall instruction in 32-bit mode, you have to use > inline assembly to support it. What version of GCC are you using. I was able to successfully build the Ia32 version with my GCC level. The Ia32 version uses a trick to do switch to 64-bit just to encode the instruction. Looks like that doesn't work with your version of GCC. I can probably switch to defining the instruction as bytes. Let me look into that and possibly send you a patch to test. Thanks, Tom > > Edk2/Build/IntelFsp2Pkg/DEBUG_GCC5/IA32/MdePkg/Library/BaseLib/BaseLib > /OUTPUT/Ia32/VmgExit.iii:33: error: elf32 output format does not > support 64-bit code > GNUmakefile:741: recipe for target > > Thanks > Liming > -Original Message- > From: Tom Lendacky > Sent: 2020年7月27日 23:26 > To: devel@edk2.groups.io > Cc: Brijesh Singh ; Ard Biesheuvel > ; Dong, Eric ; Justen, > Jordan L ; Laszlo Ersek > ; Gao, Liming ; Kinney, > Michael D ; Ni, Ray > Subject: [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT > instruction > > From: Tom Lendacky > > BZ: > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz > illa.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&data=02%7C01%7Cthoma > s.lendacky%40amd.com%7C77c8250cd9e14f2929a008d832965726%7C3dd8961fe488 > 4e608e11a82d994e183d%7C0%7C0%7C637314968570901400&sdata=6zqseI3tVm > aw351w9mfEymMnDcjDzjvcBrhARU6r3Ho%3D&reserved=0 > > VMGEXIT is a new instruction used for Hypervisor/Guest communication when > running as an SEV-ES guest. A VMGEXIT will cause an automatic exit (AE) to > occur, resulting in a #VMEXIT with an exit code value of 0x403. > > Provide the necessary support to execute the VMGEXIT instruction, which is > "rep; vmmcall". > > Cc: Michael D Kinney > Cc: Liming Gao > Signed-off-by: Tom Lendacky > --- > MdePkg/Library/BaseLib/BaseLib.inf | 2 ++ > MdePkg/Include/Library/BaseLib.h | 14 + > MdePkg/Library/BaseLib/Ia32/VmgExit.nasm | 37 > MdePkg/Library/BaseLib/X64/VmgExit.nasm | 32 > 4 files changed, 85 insertions(+) > create mode 100644 MdePkg/Library/BaseLib/Ia32/VmgExit.nasm > create mode 100644 MdePkg/Library/BaseLib/X64/VmgExit.nasm > > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf > b/MdePkg/Library/BaseLib/BaseLib.inf > index 3b93b5db8d24..3b85c56c3c03 100644 > --- a/MdePkg/Library/BaseLib/BaseLib.inf > +++ b/MdePkg/Library/BaseLib/BaseLib.inf > @@ -184,6 +184,7 @@ [Sources.Ia32] > Ia32/DisableCache.nasm| GCC > Ia32/RdRand.nasm > Ia32/XGetBv.nasm > + Ia32/VmgExit.nasm > > Ia32/DivS64x64Remainder.c > Ia32/InternalSwitchStack.c | MSFT > @@ -317,6 +318,7 @@ [Sources.X64] > X64/DisablePaging64.nasm > X64/RdRand.nasm > X64/XGetBv.nasm > + X64/VmgExit.nasm > ChkStkGcc.c | GCC > > [Sources.EBC] > diff --git a/MdePkg/Include/Library/BaseLib.h > b/MdePkg/Include/Library/BaseLib.h > index 7edf0051a0a0..04fb329eaabb 100644 > --- a/MdePkg/Include/Library/BaseLib.h > +++ b/MdePkg/Include/Library/BaseLib.h > @@ -7848,6 +7848,20 @@ AsmXGetBv ( > ); > > > +/** > + Executes a VMGEXIT instruction (VMMCALL with a REP prefix) > + > + Executes a VMGEXIT instruction. This function is only available on > + IA-32 and x64. > + > +**/ > +VOID > +EFIAPI > +AsmVmgExit ( > + VOID > + ); > + > + > /** > Patch the immediate operand of an IA32 or X64 instruction such that the > byte, > word, dword or qword operand is encoded at the end of the > instruction's diff --git a/MdePkg/Library/BaseLib/Ia32/VmgExit.nasm > b/MdePkg/Library/BaseLib/Ia32/VmgExit.nasm > new file mode 100644 > index ..a4b37385cc7a > --- /dev/null > +++ b/MdePkg/Library/BaseLib/Ia32/VmgExit.nasm > @@ -0,0 +1,37 @@ > +; > +-- > + > +; > +; Copyright (C) 2020, Advanced Micro Devices, Inc. All rights > +reserved. ; SPDX-License-Identifier: BSD-2-Clause-Patent ; ; > +Module > +Name: > +; > +; VmgExit.Asm > +; > +; Abstract: > +; > +; AsmVmgExit function > +; > +; Notes: > +; > +; > +-- > + > + > +SECTION .text > + > +;---
Re: [edk2-devel] [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction
On 07/28/20 09:39, Gao, Liming wrote: > This error is reported from nasm compiler. My nasm compiler version is > 2.11.08. It may be a little old. 2.12 should be fine. > > This change also requires to update > edk2\BaseTools\Conf\tools_def.template and mention nasm compiler > version. "tools_def.template" says: NASM 2.10 or later for use with the GCC toolchain family Bumping the NASM requirement from 2.10 to 2.12 will rule out: - Debian "jessie" (oldoldstable), - Ubuntu "xenial" (16.04 LTS), - and RHEL7, as build hosts. Debian "jessie" is no longer supported (LTS ended in June 2020), but Ubuntu "xenial" and RHEL7 are still supported by their vendors. I seem to recall that it was me to recommend "BITS 64" in front of "rep vmmcall" in the IA32 NASM source file: https://edk2.groups.io/g/devel/message/48292 http://mid.mail-archive.com/e8a8e21e-4045-1b2b-f959-13fbe00132d9@redhat.com I don't understand why my testing worked back then, and now it doesn't. (IOW, I can also reproduce the error that Liming reported!) It's likely because I didn't specify the elf32 output format back then. Indeed: the following command fails: > "nasm" \ > -I"$WORKSPACE"/MdePkg/Library/BaseLib/Ia32/ \ > -I"$WORKSPACE"/MdePkg/Library/BaseLib/Ia32/ \ > -I"$WORKSPACE"/MdePkg/Library/BaseLib/ \ > > -I"$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/DEBUG/ > \ > -I"$WORKSPACE"/MdePkg/ \ > -I"$WORKSPACE"/MdePkg/Include/ \ > -I"$WORKSPACE"/MdePkg/Test/UnitTest/Include/ \ > -I"$WORKSPACE"/MdePkg/Include/Ia32/ \ > -f elf32 \ > -o > "$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/OUTPUT/Ia32/VmgExit.obj > \ > > "$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/OUTPUT/Ia32/VmgExit.iii but if I remove "-f elf32", it completes fine. :( The AMD manual says about VMGEXIT: > The VMGEXIT opcode is only valid within a guest when run with SEV-ES > mode active. If the guest is not run with SEV-ES mode active, the > VMGEXIT opcode will be treated as a VMMCALL opcode and will behave > exactly like a VMMCALL. VMGEXIT is a SEV-ES-only form of guest-host communication. SEV-ES mode depends on SEV. A SEV guest can only interact with the host (= decrypt its pages for the host to access) if the guest is executing in long mode. So does it even make sense to *attempt* implementing AsmVmgExit() "correctly" for IA32? I don't want to complicate the build dependencies in this series further, so I won't suggest that we simply *not* implement AsmVmgExit() for IA32 at all. (Purely from a BaseLib perspective, this would be a valid approach, but then call sites would have to be *build-time* restricted to X64 too. The call sites *are* already restricted to X64, AIUI, but that happens at runtime (= dynamic checks), not at build time.) So here's what I suggest: implement AsmVmgExit() for IA32 in the C language, namely as a call to CpuBreakpoint(). I wouldn't like to tighten the NASM version requirement for *all* of edk2, for the sake of building a BaseLib primitive for IA32 that we never *call* on IA32. Thanks, Laszlo > > Thanks > Liming > -Original Message- > From: Tom Lendacky > Sent: 2020t728å 12:08 > To: Gao, Liming ; devel@edk2.groups.io > Cc: Brijesh Singh ; Ard Biesheuvel > ; Dong, Eric ; Justen, Jordan L > ; Laszlo Ersek ; Kinney, > Michael D ; Ni, Ray > Subject: Re: [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT > instruction > > On 7/27/20 8:34 PM, Gao, Liming wrote: >> Tom: > > Hi Liming, > >>I meet with GCC failure on this patch. Can you help check it? If nasm >> doesn't support the vmmcall instruction in 32-bit mode, you have to use >> inline assembly to support it. > > What version of GCC are you using. I was able to successfully build the > Ia32 version with my GCC level. The Ia32 version uses a trick to do switch to > 64-bit just to encode the instruction. Looks like that doesn't work with your > version of GCC. > > I can probably switch to defining the instruction as bytes. Let me look into > that and possibly send you a patch to test. > > Thanks, > Tom > >> >> Edk2/Build/IntelFsp2Pkg/DEBUG_GCC5/IA32/MdePkg/Library/BaseLib/BaseLib >> /OUTPUT/Ia32/VmgExit.iii:33: error: elf32 output format does not >> support 64-bit code >> GNUmakefile:741: recipe for target >> >> Thanks >> Liming >> -Original Message- >> From: Tom Lendacky >> Sent: 2020t727å 23:26 >> To: devel@edk2.groups.io >> Cc: Brijesh Singh ; Ard Biesheuvel >> ; Dong, Eric ; Justen, >> Jordan L ; Laszlo Ersek >> ; Gao, Liming ; Kinney, >> Michael D ; Ni, Ray >> Subject: [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT >> instruction >> >> From: Tom Lendacky >> >> BZ: >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz >> illa.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&data=02%7C01%7Cthoma >> s.lendacky%40amd.com%7C77c8250cd9e14f2929a008d832965726%7C3dd8961fe488 >> 4e608e11a82d994e183d%7C0%7C0
Re: [edk2-devel] [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction
Laszlo: > -Original Message- > From: Laszlo Ersek > Sent: Tuesday, July 28, 2020 8:04 PM > To: Gao, Liming ; Tom Lendacky > ; devel@edk2.groups.io > Cc: Brijesh Singh ; Ard Biesheuvel > ; Dong, Eric ; Justen, > Jordan L ; Kinney, Michael D > ; Ni, Ray > Subject: Re: [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT > instruction > > On 07/28/20 09:39, Gao, Liming wrote: > > This error is reported from nasm compiler. My nasm compiler version is > > 2.11.08. It may be a little old. 2.12 should be fine. > > > > This change also requires to update > > edk2\BaseTools\Conf\tools_def.template and mention nasm compiler > > version. > > "tools_def.template" says: > > NASM 2.10 or later for use with the GCC toolchain family > > Bumping the NASM requirement from 2.10 to 2.12 will rule out: > > - Debian "jessie" (oldoldstable), > - Ubuntu "xenial" (16.04 LTS), > - and RHEL7, > > as build hosts. > > Debian "jessie" is no longer supported (LTS ended in June 2020), but > Ubuntu "xenial" and RHEL7 are still supported by their vendors. > Yes. I also realize nasm version impact. It may also impact Mac OS. We should try to avoid new requirement to update nasm version. Thanks Liming > I seem to recall that it was me to recommend "BITS 64" in front of "rep > vmmcall" in the IA32 NASM source file: > > https://edk2.groups.io/g/devel/message/48292 > http://mid.mail-archive.com/e8a8e21e-4045-1b2b-f959-13fbe00132d9@redhat.com > > I don't understand why my testing worked back then, and now it doesn't. > (IOW, I can also reproduce the error that Liming reported!) It's likely > because I didn't specify the elf32 output format back then. > > Indeed: the following command fails: > > > "nasm" \ > > -I"$WORKSPACE"/MdePkg/Library/BaseLib/Ia32/ \ > > -I"$WORKSPACE"/MdePkg/Library/BaseLib/Ia32/ \ > > -I"$WORKSPACE"/MdePkg/Library/BaseLib/ \ > > > > -I"$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/DEBUG/ > > \ > > -I"$WORKSPACE"/MdePkg/ \ > > -I"$WORKSPACE"/MdePkg/Include/ \ > > -I"$WORKSPACE"/MdePkg/Test/UnitTest/Include/ \ > > -I"$WORKSPACE"/MdePkg/Include/Ia32/ \ > > -f elf32 \ > > -o > > "$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/OUTPUT/Ia32/VmgExit.obj > > \ > > > > "$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/OUTPUT/Ia32/VmgExit.iii > > but if I remove "-f elf32", it completes fine. :( > > The AMD manual says about VMGEXIT: > > > The VMGEXIT opcode is only valid within a guest when run with SEV-ES > > mode active. If the guest is not run with SEV-ES mode active, the > > VMGEXIT opcode will be treated as a VMMCALL opcode and will behave > > exactly like a VMMCALL. > > VMGEXIT is a SEV-ES-only form of guest-host communication. SEV-ES mode > depends on SEV. A SEV guest can only interact with the host (= decrypt > its pages for the host to access) if the guest is executing in long > mode. > > So does it even make sense to *attempt* implementing AsmVmgExit() > "correctly" for IA32? > > I don't want to complicate the build dependencies in this series > further, so I won't suggest that we simply *not* implement AsmVmgExit() > for IA32 at all. (Purely from a BaseLib perspective, this would be a > valid approach, but then call sites would have to be *build-time* > restricted to X64 too. The call sites *are* already restricted to X64, > AIUI, but that happens at runtime (= dynamic checks), not at build > time.) > > So here's what I suggest: implement AsmVmgExit() for IA32 in the C > language, namely as a call to CpuBreakpoint(). > > I wouldn't like to tighten the NASM version requirement for *all* of > edk2, for the sake of building a BaseLib primitive for IA32 that we > never *call* on IA32. > > Thanks, > Laszlo > > > > > Thanks > > Liming > > -Original Message- > > From: Tom Lendacky > > Sent: 2020t728å 12:08 > > To: Gao, Liming ; devel@edk2.groups.io > > Cc: Brijesh Singh ; Ard Biesheuvel > > ; Dong, Eric ; Justen, > Jordan L ; Laszlo Ersek ; > Kinney, Michael D ; Ni, Ray > > > Subject: Re: [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT > > instruction > > > > On 7/27/20 8:34 PM, Gao, Liming wrote: > >> Tom: > > > > Hi Liming, > > > >>I meet with GCC failure on this patch. Can you help check it? If nasm > >> doesn't support the vmmcall instruction in 32-bit mode, you > have to use inline assembly to support it. > > > > What version of GCC are you using. I was able to successfully build the > > Ia32 version with my GCC level. The Ia32 version uses a trick to do switch > > to 64-bit just to encode the instruction. Looks like that > doesn't work with your version of GCC. > > > > I can probably switch to defining the instruction as bytes. Let me look > > into that and possibly send you a patch to test. > > > > Thanks, > > Tom > > > >> > >> Edk2/Build/IntelFsp2Pkg/DEBUG_GCC5/IA32/MdePkg/Library/BaseLib/BaseLib > >> /OUTPUT/Ia32/VmgExit.ii
Re: [edk2-devel] [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction
On 7/28/20 7:04 AM, Laszlo Ersek wrote:
> On 07/28/20 09:39, Gao, Liming wrote:
>> This error is reported from nasm compiler. My nasm compiler version is
>> 2.11.08. It may be a little old. 2.12 should be fine.
I have 2.13.02.
>>
>> This change also requires to update
>> edk2\BaseTools\Conf\tools_def.template and mention nasm compiler
>> version.
>
> "tools_def.template" says:
>
> NASM 2.10 or later for use with the GCC toolchain family
>
> Bumping the NASM requirement from 2.10 to 2.12 will rule out:
>
> - Debian "jessie" (oldoldstable),
> - Ubuntu "xenial" (16.04 LTS),
> - and RHEL7,
>
> as build hosts.
>
> Debian "jessie" is no longer supported (LTS ended in June 2020), but
> Ubuntu "xenial" and RHEL7 are still supported by their vendors.
>
> I seem to recall that it was me to recommend "BITS 64" in front of "rep
> vmmcall" in the IA32 NASM source file:
>
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F48292&data=02%7C01%7Cthomas.lendacky%40amd.com%7Ca8b0f66286b745e78b1d08d832ee6f95%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637315346944536242&sdata=4DBhHd2WYgoEx%2F76YaNRalJlSvd0rAIWxrN1qOFR9Tw%3D&reserved=0
>
> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmid.mail-archive.com%2Fe8a8e21e-4045-1b2b-f959-13fbe00132d9%40redhat.com&data=02%7C01%7Cthomas.lendacky%40amd.com%7Ca8b0f66286b745e78b1d08d832ee6f95%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637315346944536242&sdata=AV5lDCYWOm2cTOzlaI7OwMjcK9TknVGBNFAkNJhahuM%3D&reserved=0
>
> I don't understand why my testing worked back then, and now it doesn't.
> (IOW, I can also reproduce the error that Liming reported!) It's likely
> because I didn't specify the elf32 output format back then.
>
> Indeed: the following command fails:
>
>> "nasm" \
>> -I"$WORKSPACE"/MdePkg/Library/BaseLib/Ia32/ \
>> -I"$WORKSPACE"/MdePkg/Library/BaseLib/Ia32/ \
>> -I"$WORKSPACE"/MdePkg/Library/BaseLib/ \
>>
>> -I"$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/DEBUG/
>> \
>> -I"$WORKSPACE"/MdePkg/ \
>> -I"$WORKSPACE"/MdePkg/Include/ \
>> -I"$WORKSPACE"/MdePkg/Test/UnitTest/Include/ \
>> -I"$WORKSPACE"/MdePkg/Include/Ia32/ \
>> -f elf32 \
>> -o
>> "$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/OUTPUT/Ia32/VmgExit.obj
>> \
>>
>> "$WORKSPACE"/Build/OvmfIa32/NOOPT_GCC48/IA32/MdePkg/Library/BaseLib/BaseLib/OUTPUT/Ia32/VmgExit.iii
>
> but if I remove "-f elf32", it completes fine. :(
With version 2.13.02, I can successfully build with '-f elf32', so it
seems to be version related.
>
> The AMD manual says about VMGEXIT:
>
>> The VMGEXIT opcode is only valid within a guest when run with SEV-ES
>> mode active. If the guest is not run with SEV-ES mode active, the
>> VMGEXIT opcode will be treated as a VMMCALL opcode and will behave
>> exactly like a VMMCALL.
>
> VMGEXIT is a SEV-ES-only form of guest-host communication. SEV-ES mode
> depends on SEV. A SEV guest can only interact with the host (= decrypt
> its pages for the host to access) if the guest is executing in long
> mode.
>
> So does it even make sense to *attempt* implementing AsmVmgExit()
> "correctly" for IA32?
We are only enabling SEV-ES support for X64, so this is a valid point. It
would be very hard to enable the Ia32X64 combination (because of how long
it takes to get into long mode) and impossible for the Ia32 alone.
>
> I don't want to complicate the build dependencies in this series
> further, so I won't suggest that we simply *not* implement AsmVmgExit()
> for IA32 at all. (Purely from a BaseLib perspective, this would be a
> valid approach, but then call sites would have to be *build-time*
> restricted to X64 too. The call sites *are* already restricted to X64,
> AIUI, but that happens at runtime (= dynamic checks), not at build
> time.)
>
> So here's what I suggest: implement AsmVmgExit() for IA32 in the C
> language, namely as a call to CpuBreakpoint().
IIUC, create a VmgExit.c file in MdePkg/Library/BaseLib/Ia32/ that doesn't
actually encode the VMGEXIT instruction, just calls CpuBreakpoint(), e.g.:
VOID
EFIAPI
AsmVmgExit (
VOID
)
{
CpuBreakpoint();
}
The other alternative is to use a DB-encoded instruction, though I know
that isn't the most popular approach.
The BITS 64 method to generate the instruction bytes is also used in
OvmfPkg/ResetVector/Ia32/PageTables64.asm, but that file is only included
when ARCH_X64 is defined, so there shouldn't be an issue there, plus the
nasm file format is bin (-f bin).
Thanks,
Tom
>
> I wouldn't like to tighten the NASM version requirement for *all* of
> edk2, for the sake of building a BaseLib primitive for IA32 that we
> never *call* on IA32.
>
> Thanks,
> Laszlo
>
>>
>> Thanks
>> Liming
>> -Original Message-
>> From: Tom Lendacky
>> Sent: 2020t728å 12:08
>> To: Gao, Liming ; devel@edk2.groups.io
>> Cc: Brijesh Singh ; Ard Biesheuvel
>> ; Don
Re: [edk2-devel] [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction
On 07/28/20 16:13, Tom Lendacky wrote:
> IIUC, create a VmgExit.c file in MdePkg/Library/BaseLib/Ia32/ that
> doesn't actually encode the VMGEXIT instruction, just calls
> CpuBreakpoint(), e.g.:
>
> VOID
> EFIAPI
> AsmVmgExit (
> VOID
> )
> {
> CpuBreakpoint();
> }
>
Yes -- either that, or even just open-code (copy) what we have in
MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.nasm
now:
global ASM_PFX(AsmVmgExit)
ASM_PFX(AsmVmgExit):
;
; VMGEXIT makes no sense on IA32, and NASM versions before 2.12 cannot
; translate the 64-bit "rep vmmcall" instruction into elf32 format. So
; provide a stub implementation that is identical to CpuBreakpoint(). In
; practice, AsmVmgExit() should never be called on IA32.
;
int 3
ret
Because this assembly-language implementation might be more "true" to
the name "AsmVmgExit".
Liming, would you be OK with this approach? In other words, the set of
files changed/introduced in this patch would not change, just the
implementation of IA32 AsmVmgExit().
> The other alternative is to use a DB-encoded instruction, though I
> know that isn't the most popular approach.
Right, I've been quite on a quest to eliminate DBs that encode
instructions.
> The BITS 64 method to generate the instruction bytes is also used in
> OvmfPkg/ResetVector/Ia32/PageTables64.asm, but that file is only
> included when ARCH_X64 is defined, so there shouldn't be an issue
> there, plus the nasm file format is bin (-f bin).
I confirm that; the following commands all work on RHEL7, with this
series applied:
$ build -b NOOPT -t GCC48 -m OvmfPkg/ResetVector/ResetVector.inf -a IA32
-p OvmfPkg/OvmfPkgIa32.dsc
$ build -b NOOPT -t GCC48 -m OvmfPkg/ResetVector/ResetVector.inf -a IA32 -a X64
-p OvmfPkg/OvmfPkgIa32X64.dsc
$ build -b NOOPT -t GCC48 -m OvmfPkg/ResetVector/ResetVector.inf -a X64
-p OvmfPkg/OvmfPkgX64.dsc
Thanks!
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#63408): https://edk2.groups.io/g/devel/message/63408
Mute This Topic: https://groups.io/mt/75824947/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction
Laszlo:
Yes. This solution makes sense.
Thanks
Liming
-Original Message-
From: Laszlo Ersek
Sent: 2020年7月28日 23:19
To: Tom Lendacky ; Gao, Liming ;
devel@edk2.groups.io
Cc: Brijesh Singh ; Ard Biesheuvel
; Dong, Eric ; Justen, Jordan L
; Kinney, Michael D ;
Ni, Ray
Subject: Re: [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT
instruction
On 07/28/20 16:13, Tom Lendacky wrote:
> IIUC, create a VmgExit.c file in MdePkg/Library/BaseLib/Ia32/ that
> doesn't actually encode the VMGEXIT instruction, just calls
> CpuBreakpoint(), e.g.:
>
> VOID
> EFIAPI
> AsmVmgExit (
> VOID
> )
> {
> CpuBreakpoint();
> }
>
Yes -- either that, or even just open-code (copy) what we have in
MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.nasm
now:
global ASM_PFX(AsmVmgExit)
ASM_PFX(AsmVmgExit):
;
; VMGEXIT makes no sense on IA32, and NASM versions before 2.12 cannot ;
translate the 64-bit "rep vmmcall" instruction into elf32 format. So ; provide
a stub implementation that is identical to CpuBreakpoint(). In ; practice,
AsmVmgExit() should never be called on IA32.
;
int 3
ret
Because this assembly-language implementation might be more "true" to the name
"AsmVmgExit".
Liming, would you be OK with this approach? In other words, the set of files
changed/introduced in this patch would not change, just the implementation of
IA32 AsmVmgExit().
> The other alternative is to use a DB-encoded instruction, though I
> know that isn't the most popular approach.
Right, I've been quite on a quest to eliminate DBs that encode instructions.
> The BITS 64 method to generate the instruction bytes is also used in
> OvmfPkg/ResetVector/Ia32/PageTables64.asm, but that file is only
> included when ARCH_X64 is defined, so there shouldn't be an issue
> there, plus the nasm file format is bin (-f bin).
I confirm that; the following commands all work on RHEL7, with this series
applied:
$ build -b NOOPT -t GCC48 -m OvmfPkg/ResetVector/ResetVector.inf -a IA32
-p OvmfPkg/OvmfPkgIa32.dsc
$ build -b NOOPT -t GCC48 -m OvmfPkg/ResetVector/ResetVector.inf -a IA32 -a X64
-p OvmfPkg/OvmfPkgIa32X64.dsc
$ build -b NOOPT -t GCC48 -m OvmfPkg/ResetVector/ResetVector.inf -a X64
-p OvmfPkg/OvmfPkgX64.dsc
Thanks!
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#63425): https://edk2.groups.io/g/devel/message/63425
Mute This Topic: https://groups.io/mt/75824947/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
