[edk2-devel] [PATCH v8 16/46] OvmfPkg/VmgExitLib: Add support for MSR_PROT NAE events

Lendacky, Thomas posted 46 patches 5 years, 8 months ago
There is a newer version of this series
[edk2-devel] [PATCH v8 16/46] OvmfPkg/VmgExitLib: Add support for MSR_PROT NAE events
Posted by Lendacky, Thomas 5 years, 8 months ago
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198

Under SEV-ES, a MSR_PROT intercept generates a #VC exception. VMGEXIT must
be used to allow the hypervisor to handle this intercept.

Add support to construct the required GHCB values to support an MSR_PROT
NAE event. Parse the instruction that generated the #VC exception to
determine whether it is RDMSR or WRMSR, setting the required register
register values in the GHCB and creating the proper SW_EXIT_INFO1 value in
the GHCB.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 .../Library/VmgExitLib/X64/VmgExitVcHandler.c | 63 +++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c
index 2f62795edf61..1c6b472a47c4 100644
--- a/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c
+++ b/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c
@@ -411,6 +411,65 @@ UnsupportedExit (
   return Status;
 }
 
+/**
+  Handle an MSR event.
+
+  Use the VMGEXIT instruction to handle either a RDMSR or WRMSR event.
+
+  @param[in, out] Ghcb             Pointer to the Guest-Hypervisor Communication
+                                   Block
+  @param[in, out] Regs             x64 processor context
+  @param[in]      InstructionData  Instruction parsing context
+
+  @retval 0                        Event handled successfully
+  @retval Others                   New exception value to propagate
+
+**/
+STATIC
+UINT64
+MsrExit (
+  IN OUT GHCB                     *Ghcb,
+  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
+  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
+  )
+{
+  UINT64  ExitInfo1, Status;
+
+  ExitInfo1 = 0;
+
+  switch (*(InstructionData->OpCodes + 1)) {
+  case 0x30: // WRMSR
+    ExitInfo1 = 1;
+    Ghcb->SaveArea.Rax = Regs->Rax;
+    GhcbSetRegValid (Ghcb, GhcbRax);
+    Ghcb->SaveArea.Rdx = Regs->Rdx;
+    GhcbSetRegValid (Ghcb, GhcbRdx);
+    /* Fallthrough */
+  case 0x32: // RDMSR
+    Ghcb->SaveArea.Rcx = Regs->Rcx;
+    GhcbSetRegValid (Ghcb, GhcbRcx);
+    break;
+  default:
+    return UnsupportedExit (Ghcb, Regs, InstructionData);
+  }
+
+  Status = VmgExit (Ghcb, SVM_EXIT_MSR, ExitInfo1, 0);
+  if (Status) {
+    return Status;
+  }
+
+  if (!ExitInfo1) {
+    if (!GhcbIsRegValid (Ghcb, GhcbRax) ||
+        !GhcbIsRegValid (Ghcb, GhcbRdx)) {
+      return UnsupportedExit (Ghcb, Regs, InstructionData);
+    }
+    Regs->Rax = Ghcb->SaveArea.Rax;
+    Regs->Rdx = Ghcb->SaveArea.Rdx;
+  }
+
+  return 0;
+}
+
 #define IOIO_TYPE_STR       (1 << 2)
 #define IOIO_TYPE_IN        1
 #define IOIO_TYPE_INS       (IOIO_TYPE_IN | IOIO_TYPE_STR)
@@ -743,6 +802,10 @@ VmgExitHandleVc (
     NaeExit = IoioExit;
     break;
 
+  case SVM_EXIT_MSR:
+    NaeExit = MsrExit;
+    break;
+
   default:
     NaeExit = UnsupportedExit;
   }
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#59872): https://edk2.groups.io/g/devel/message/59872
Mute This Topic: https://groups.io/mt/74336570/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v8 16/46] OvmfPkg/VmgExitLib: Add support for MSR_PROT NAE events
Posted by Laszlo Ersek 5 years, 8 months ago
On 05/19/20 23:50, Lendacky, Thomas wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
> 
> Under SEV-ES, a MSR_PROT intercept generates a #VC exception. VMGEXIT must
> be used to allow the hypervisor to handle this intercept.
> 
> Add support to construct the required GHCB values to support an MSR_PROT
> NAE event. Parse the instruction that generated the #VC exception to
> determine whether it is RDMSR or WRMSR, setting the required register
> register values in the GHCB and creating the proper SW_EXIT_INFO1 value in
> the GHCB.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  .../Library/VmgExitLib/X64/VmgExitVcHandler.c | 63 +++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c
> index 2f62795edf61..1c6b472a47c4 100644
> --- a/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c
> +++ b/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c
> @@ -411,6 +411,65 @@ UnsupportedExit (
>    return Status;
>  }
>  
> +/**
> +  Handle an MSR event.
> +
> +  Use the VMGEXIT instruction to handle either a RDMSR or WRMSR event.
> +
> +  @param[in, out] Ghcb             Pointer to the Guest-Hypervisor Communication
> +                                   Block
> +  @param[in, out] Regs             x64 processor context
> +  @param[in]      InstructionData  Instruction parsing context
> +
> +  @retval 0                        Event handled successfully
> +  @retval Others                   New exception value to propagate
> +
> +**/
> +STATIC
> +UINT64
> +MsrExit (
> +  IN OUT GHCB                     *Ghcb,
> +  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
> +  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  )
> +{
> +  UINT64  ExitInfo1, Status;
> +
> +  ExitInfo1 = 0;
> +
> +  switch (*(InstructionData->OpCodes + 1)) {
> +  case 0x30: // WRMSR

This comment looks great!

> +    ExitInfo1 = 1;
> +    Ghcb->SaveArea.Rax = Regs->Rax;
> +    GhcbSetRegValid (Ghcb, GhcbRax);
> +    Ghcb->SaveArea.Rdx = Regs->Rdx;
> +    GhcbSetRegValid (Ghcb, GhcbRdx);
> +    /* Fallthrough */

(1) This comment is very appreciated (I vaguely remember that the coding
style actually requires it), but we're supposed to put it like this:

    //
    // fall through
    //

(See: "git grep -B1 -A1 -i 'fall through'".)

> +  case 0x32: // RDMSR
> +    Ghcb->SaveArea.Rcx = Regs->Rcx;
> +    GhcbSetRegValid (Ghcb, GhcbRcx);
> +    break;
> +  default:
> +    return UnsupportedExit (Ghcb, Regs, InstructionData);
> +  }
> +
> +  Status = VmgExit (Ghcb, SVM_EXIT_MSR, ExitInfo1, 0);
> +  if (Status) {

(2) As usual, please check (Status > 0) or (Status != 0) explicitly.

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

> +    return Status;
> +  }
> +
> +  if (!ExitInfo1) {
> +    if (!GhcbIsRegValid (Ghcb, GhcbRax) ||
> +        !GhcbIsRegValid (Ghcb, GhcbRdx)) {
> +      return UnsupportedExit (Ghcb, Regs, InstructionData);
> +    }
> +    Regs->Rax = Ghcb->SaveArea.Rax;
> +    Regs->Rdx = Ghcb->SaveArea.Rdx;
> +  }
> +
> +  return 0;
> +}
> +
>  #define IOIO_TYPE_STR       (1 << 2)
>  #define IOIO_TYPE_IN        1
>  #define IOIO_TYPE_INS       (IOIO_TYPE_IN | IOIO_TYPE_STR)
> @@ -743,6 +802,10 @@ VmgExitHandleVc (
>      NaeExit = IoioExit;
>      break;
>  
> +  case SVM_EXIT_MSR:
> +    NaeExit = MsrExit;
> +    break;
> +
>    default:
>      NaeExit = UnsupportedExit;
>    }
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60111): https://edk2.groups.io/g/devel/message/60111
Mute This Topic: https://groups.io/mt/74336570/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v8 16/46] OvmfPkg/VmgExitLib: Add support for MSR_PROT NAE events
Posted by Lendacky, Thomas 5 years, 8 months ago
On 5/22/20 5:31 AM, Laszlo Ersek wrote:
> On 05/19/20 23:50, Lendacky, Thomas wrote:
>> 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%7C95b407c13bd44ad1b89608d7fe3b450a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637257402837183063&amp;sdata=jSpUywdrvuJ5Iw1lfVb30VUuZc%2FBOGn%2B670DymPBePY%3D&amp;reserved=0
>>
>> Under SEV-ES, a MSR_PROT intercept generates a #VC exception. VMGEXIT must
>> be used to allow the hypervisor to handle this intercept.
>>
>> Add support to construct the required GHCB values to support an MSR_PROT
>> NAE event. Parse the instruction that generated the #VC exception to
>> determine whether it is RDMSR or WRMSR, setting the required register
>> register values in the GHCB and creating the proper SW_EXIT_INFO1 value in
>> the GHCB.
>>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>>   .../Library/VmgExitLib/X64/VmgExitVcHandler.c | 63 +++++++++++++++++++
>>   1 file changed, 63 insertions(+)
>>
>> diff --git a/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c
>> index 2f62795edf61..1c6b472a47c4 100644
>> --- a/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c
>> +++ b/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c
>> @@ -411,6 +411,65 @@ UnsupportedExit (
>>     return Status;
>>   }
>>   
>> +/**
>> +  Handle an MSR event.
>> +
>> +  Use the VMGEXIT instruction to handle either a RDMSR or WRMSR event.
>> +
>> +  @param[in, out] Ghcb             Pointer to the Guest-Hypervisor Communication
>> +                                   Block
>> +  @param[in, out] Regs             x64 processor context
>> +  @param[in]      InstructionData  Instruction parsing context
>> +
>> +  @retval 0                        Event handled successfully
>> +  @retval Others                   New exception value to propagate
>> +
>> +**/
>> +STATIC
>> +UINT64
>> +MsrExit (
>> +  IN OUT GHCB                     *Ghcb,
>> +  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
>> +  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
>> +  )
>> +{
>> +  UINT64  ExitInfo1, Status;
>> +
>> +  ExitInfo1 = 0;
>> +
>> +  switch (*(InstructionData->OpCodes + 1)) {
>> +  case 0x30: // WRMSR
> 
> This comment looks great!
> 
>> +    ExitInfo1 = 1;
>> +    Ghcb->SaveArea.Rax = Regs->Rax;
>> +    GhcbSetRegValid (Ghcb, GhcbRax);
>> +    Ghcb->SaveArea.Rdx = Regs->Rdx;
>> +    GhcbSetRegValid (Ghcb, GhcbRdx);
>> +    /* Fallthrough */
> 
> (1) This comment is very appreciated (I vaguely remember that the coding
> style actually requires it), but we're supposed to put it like this:
> 
>      //
>      // fall through
>      //
> 
> (See: "git grep -B1 -A1 -i 'fall through'".)
> 

Will do.

>> +  case 0x32: // RDMSR
>> +    Ghcb->SaveArea.Rcx = Regs->Rcx;
>> +    GhcbSetRegValid (Ghcb, GhcbRcx);
>> +    break;
>> +  default:
>> +    return UnsupportedExit (Ghcb, Regs, InstructionData);
>> +  }
>> +
>> +  Status = VmgExit (Ghcb, SVM_EXIT_MSR, ExitInfo1, 0);
>> +  if (Status) {
> 
> (2) As usual, please check (Status > 0) or (Status != 0) explicitly.

Yup.

> 
> Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Tom

> 
> Thanks
> Laszlo
> 
>> +    return Status;
>> +  }
>> +
>> +  if (!ExitInfo1) {
>> +    if (!GhcbIsRegValid (Ghcb, GhcbRax) ||
>> +        !GhcbIsRegValid (Ghcb, GhcbRdx)) {
>> +      return UnsupportedExit (Ghcb, Regs, InstructionData);
>> +    }
>> +    Regs->Rax = Ghcb->SaveArea.Rax;
>> +    Regs->Rdx = Ghcb->SaveArea.Rdx;
>> +  }
>> +
>> +  return 0;
>> +}
>> +
>>   #define IOIO_TYPE_STR       (1 << 2)
>>   #define IOIO_TYPE_IN        1
>>   #define IOIO_TYPE_INS       (IOIO_TYPE_IN | IOIO_TYPE_STR)
>> @@ -743,6 +802,10 @@ VmgExitHandleVc (
>>       NaeExit = IoioExit;
>>       break;
>>   
>> +  case SVM_EXIT_MSR:
>> +    NaeExit = MsrExit;
>> +    break;
>> +
>>     default:
>>       NaeExit = UnsupportedExit;
>>     }
>>
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60166): https://edk2.groups.io/g/devel/message/60166
Mute This Topic: https://groups.io/mt/74336570/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-