Re: [edk2-devel] [PATCH v14 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction

["Lendacky, Thomas" <thomas.lendacky@amd.com>]

On 8/10/20 1:47 PM, Laszlo Ersek wrote:
> Hi Tom,

Hi Laszlo,

> 
> On 08/07/20 21:38, Lendacky, Thomas wrote:
>> From: Tom Lendacky <thomas.lendacky@amd.com>
>>
>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&amp;data=02%7C01%7Cthomas.lendacky%40amd.com%7C243b5723f67c4fe89ed008d83d5dd5db%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637326820522631996&amp;sdata=vyh1QG9W%2Fi%2FOHrjG6oiOySzF6B5%2BTi76pZ2UYAH5e3M%3D&amp;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.
>>
>> Since SEV-ES is only supported in X64, provide the necessary X64 support
>> to execute the VMGEXIT instruction, which is coded as "rep vmmcall". For
>> IA32, since "vmmcall" is not supported in NASM 32-bit mode and VMGEXIT
>> should never be called, provide a stub implementation that is identical
>> to CpuBreakpoint().
>>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Reviewed-by: Liming Gao <liming.gao@intel.com>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>>  MdePkg/Library/BaseLib/BaseLib.inf       |  2 ++
>>  MdePkg/Include/Library/BaseLib.h         | 14 +++++++++
>>  MdePkg/Library/BaseLib/Ia32/VmgExit.nasm | 38 ++++++++++++++++++++++++
>>  MdePkg/Library/BaseLib/X64/VmgExit.nasm  | 32 ++++++++++++++++++++
>>  4 files changed, 86 insertions(+)
>>  create mode 100644 MdePkg/Library/BaseLib/Ia32/VmgExit.nasm
>>  create mode 100644 MdePkg/Library/BaseLib/X64/VmgExit.nasm
> 
> this patch is identical to the one in v13, but you forgot to pick up my:
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Yup, sorry about that, I missed it.

Thanks,
Tom

> 
> from:
> 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F63568&amp;data=02%7C01%7Cthomas.lendacky%40amd.com%7C243b5723f67c4fe89ed008d83d5dd5db%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637326820522631996&amp;sdata=jWS0M5i780Nt53YX6CjEdIxP51CNhYhEo3qReHJUQvw%3D&amp;reserved=0
> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmid.mail-archive.com%2F98ff9331-c73b-07f9-de49-f08365c6e003%40redhat.com&amp;data=02%7C01%7Cthomas.lendacky%40amd.com%7C243b5723f67c4fe89ed008d83d5dd5db%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637326820522631996&amp;sdata=ZZHmJPS9MNeEqI08WDDgPwnwsi8TPV7JjyPgziseYm4%3D&amp;reserved=0
> 
> Liming, when you push this series, please add my R-b to the commit
> message on this patch.
> 
> Thanks
> Laszlo
> 
>>
>> 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 000000000000..69f7fbf3506a
>> --- /dev/null
>> +++ b/MdePkg/Library/BaseLib/Ia32/VmgExit.nasm
>> @@ -0,0 +1,38 @@
>> +;------------------------------------------------------------------------------
>> +;
>> +; Copyright (C) 2020, Advanced Micro Devices, Inc. All rights reserved.<BR>
>> +; 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 and NASM versions
>> +; before 2.12 cannot translate the 64-bit "rep vmmcall" instruction into elf32
>> +; format. Given that VMGEXIT does not make sense on IA32, provide a stub
>> +; implementation that is identical to CpuBreakpoint(). In practice, AsmVmgExit()
>> +; should never be called on IA32.
>> +;
>> +    int  3
>> +    ret
>> +
>> diff --git a/MdePkg/Library/BaseLib/X64/VmgExit.nasm b/MdePkg/Library/BaseLib/X64/VmgExit.nasm
>> new file mode 100644
>> index 000000000000..26f034593c67
>> --- /dev/null
>> +++ b/MdePkg/Library/BaseLib/X64/VmgExit.nasm
>> @@ -0,0 +1,32 @@
>> +;------------------------------------------------------------------------------
>> +;
>> +; Copyright (C) 2020, Advanced Micro Devices, Inc. All rights reserved.<BR>
>> +; 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
>> +
>>
>