Re: [PATCH v12 07/46] MdePkg/BaseLib: Add support for the VMGEXIT instruction

["Laszlo Ersek" <lersek@redhat.com>]

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