[edk2-devel] [PATCH 1/3] OvfmPkg/VmgExitLib: Properly decode MMIO MOVZX and MOVSX opcodes
Lendacky, Thomas
thomas.lendacky at amd.com
Thu Apr 22 13:35:06 UTC 2021
On 4/22/21 12:28 AM, Laszlo Ersek wrote:
> On 04/21/21 00:54, Lendacky, Thomas wrote:
>> From: Tom Lendacky <thomas.lendacky at amd.com>
>>
>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3345&data=04%7C01%7Cthomas.lendacky%40amd.com%7C22bf3a3ae9cb4421e93208d9054f79c8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637546661229697941%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=1EmUDf%2FfuCuu%2BkXPZijzatfliplMhKEQH8kiZ9Z8ZF0%3D&reserved=0
>>
>> 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.
Actually, you will see this only once since the code will propagate a GP
and the guest will terminate in this situation.
>
> (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.
Will do.
Thanks,
Tom
>
> 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 (#74365): https://edk2.groups.io/g/devel/message/74365
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