[v2] SEV-ES TPM enablement fixes

[edk2-devel] [PATCH v2 1/4] OvfmPkg/VmgExitLib: Properly decode MMIO MOVZX and MOVSX opcodes

Posted by Lendacky, Thomas 4 years, 9 months ago

From: Tom Lendacky <thomas.lendacky@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@redhat.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Min Xu <min.m.xu@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@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..dd117f971134 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_ERROR, "MMIO using encrypted memory: %lx\n", (UINT64) MemoryAddress));
   GpEvent.Uint64 = 0;
   GpEvent.Elements.Vector = GP_EXCEPTION;
   GpEvent.Elements.Type   = GHCB_EVENT_INJECTION_TYPE_EXCEPTION;
@@ -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;
 
-- 
2.31.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74491): https://edk2.groups.io/g/devel/message/74491
Mute This Topic: https://groups.io/mt/82407851/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 1/4] OvfmPkg/VmgExitLib: Properly decode MMIO MOVZX and MOVSX opcodes

Posted by Laszlo Ersek 4 years, 9 months ago

On 04/27/21 18:21, Lendacky, Thomas wrote:
> From: Tom Lendacky <thomas.lendacky@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@redhat.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@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..dd117f971134 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_ERROR, "MMIO using encrypted memory: %lx\n", (UINT64) MemoryAddress));

(1) This line is now too long -- 86 characters. But I'll fix that up on
merge, if I find nothing serious in v2.

Thanks
Laszlo

>    GpEvent.Uint64 = 0;
>    GpEvent.Elements.Vector = GP_EXCEPTION;
>    GpEvent.Elements.Type   = GHCB_EVENT_INJECTION_TYPE_EXCEPTION;
> @@ -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 (#74567): https://edk2.groups.io/g/devel/message/74567
Mute This Topic: https://groups.io/mt/82407851/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 1/4] OvfmPkg/VmgExitLib: Properly decode MMIO MOVZX and MOVSX opcodes
[edk2-devel] [PATCH v2 2/4] OvmfPkg/VmgExitLib: Add support for new MMIO MOV opcodes
[edk2-devel] [PATCH v2 3/4] OvmfPkg: Define a new PPI GUID to signal TPM MMIO accessability
[edk2-devel] [PATCH v2 4/4] OvmfPkg/Tcg2ConfigPei: Mark TPM MMIO range as unencrypted for SEV-ES