[edk2-devel] [PATCH 1/3] OvfmPkg/VmgExitLib: Properly decode MMIO MOVZX and MOVSX opcodes
Laszlo Ersek
lersek at redhat.com
Thu Apr 22 05:28:25 UTC 2021
On 04/21/21 00:54, Lendacky, Thomas wrote:
> From: Tom Lendacky <thomas.lendacky at amd.com>
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3345
>
> The MOVZX and MOVSX instructions use the ModRM byte in the instruction,
> but the instruction decoding support was not decoding it. This resulted
> in invalid decoding and failing of the MMIO operation. Also, when
> performing the zero-extend or sign-extend operation, the memory operation
> should be using the size, and not the size enumeration value.
>
> Add the ModRM byte decoding for the MOVZX and MOVSX opcodes and use the
> true data size to perform the extend operations. Additionally, add a
> DEBUG statement identifying the MMIO address being flagged as encrypted
> during the MMIO address validation.
>
> Fixes: c45f678a1ea2080344e125dc55b14e4b9f98483d
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Brijesh Singh <brijesh.singh at amd.com>
> Cc: James Bottomley <jejb at linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao at intel.com>
> Cc: Min Xu <min.m.xu at intel.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky at amd.com>
> ---
> OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
> index 24259060fd65..273f36499988 100644
> --- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
> +++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
> @@ -643,6 +643,7 @@ ValidateMmioMemory (
> //
> // Any state other than unencrypted is an error, issue a #GP.
> //
> + DEBUG ((DEBUG_INFO, "MMIO using encrypted memory: %lx\n", MemoryAddress));
> GpEvent.Uint64 = 0;
> GpEvent.Elements.Vector = GP_EXCEPTION;
> GpEvent.Elements.Type = GHCB_EVENT_INJECTION_TYPE_EXCEPTION;
(1) This can potentially generate a large number of debug messages;
please use the DEBUG_VERBOSE log mask.
(2) "MemoryAddress" has type UINTN, but %lx takes UINT64. Given that
this is X64-only code, functionally there is no bug, but it's still
cleaner to pass "(UINT64)MemoryAddress" to %lx.
With that:
Acked-by: Laszlo Ersek <lersek at redhat.com>
Thanks
Laszlo
> @@ -817,6 +818,7 @@ MmioExit (
> // fall through
> //
> case 0xB7:
> + DecodeModRm (Regs, InstructionData);
> Bytes = (Bytes != 0) ? Bytes : 2;
>
> Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData, Bytes);
> @@ -835,7 +837,7 @@ MmioExit (
> }
>
> Register = GetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
> - SetMem (Register, InstructionData->DataSize, 0);
> + SetMem (Register, (UINTN) (1 << InstructionData->DataSize), 0);
> CopyMem (Register, Ghcb->SharedBuffer, Bytes);
> break;
>
> @@ -848,6 +850,7 @@ MmioExit (
> // fall through
> //
> case 0xBF:
> + DecodeModRm (Regs, InstructionData);
> Bytes = (Bytes != 0) ? Bytes : 2;
>
> Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData, Bytes);
> @@ -878,7 +881,7 @@ MmioExit (
> }
>
> Register = GetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
> - SetMem (Register, InstructionData->DataSize, SignByte);
> + SetMem (Register, (UINTN) (1 << InstructionData->DataSize), SignByte);
> CopyMem (Register, Ghcb->SharedBuffer, Bytes);
> break;
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74343): https://edk2.groups.io/g/devel/message/74343
Mute This Topic: https://groups.io/mt/82247952/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