On 6/11/20 4:31 AM, Laszlo Ersek wrote:
On 06/11/20 11:24, Laszlo Ersek wrote:
On 06/05/20 15:27, Tom Lendacky wrote:
BZ: 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&data=02%7C01%7Cthomas.lendacky%40amd.com%7C63ef262427d14f35c62008d80dea363f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637274646869954992&sdata=xoYJQgjiyxcaXK46nNrrbx1qEEVnvispzNgtL1%2BYIxg%3D&reserved=0

Under SEV-ES, a DR7 read or write intercept generates a #VC exception.
The #VC handler must provide special support to the guest for this. On
a DR7 write, the #VC handler must cache the value and issue a VMGEXIT
to notify the hypervisor of the write. However, the #VC handler must
not actually set the value of the DR7 register. On a DR7 read, the #VC
handler must return the cached value of the DR7 register to the guest.
VMGEXIT is not invoked for a DR7 register read.

The caching of the DR7 values will make use of the per-CPU data pages
that are allocated along with the GHCB pages. The per-CPU page for a
vCPU is the page that immediately follows the vCPU's GHCB page. Since
each GHCB page is unique for a vCPU, the page that follows becomes
unique for that vCPU. The SEC phase will reserves an area of memory for
a single GHCB and per-CPU page for use by the BSP. After transitioning
to the PEI phase, new GHCB and per-CPU pages are allocated for the BSP
and all APs.

Cc: Jordan Justen <jordan.l.jus...@intel.com>
Cc: Laszlo Ersek <ler...@redhat.com>
Cc: Ard Biesheuvel <ard.biesheu...@arm.com>
Signed-off-by: Tom Lendacky <thomas.lenda...@amd.com>
---
  OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 114 ++++++++++++++++++++
  1 file changed, 114 insertions(+)

The patch looks good to me:

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

Just one question: in the v8 review, I wrote:

"""
With your explanation above, about platform reset, I think I'm happy
with the current handling of "Dr7Cached". So I'd like to leave the
choice to you: please either add the clearing, or document in the commit
message and/or the code that platform reset will not happen. Whichever
you like more.
"""

So what have you chosen ultimately? I haven't found a comment to the
effect of "An SEV-ES guest can't be rebooted/reset without restarting
Qemu" in this patch, in the commit message or in the code. Did you
implement the clearing, in the end? (Sorry if I should have noticed it
already!)

If I understand correctly, it's the clearGhcbMemoryLoop part (moved to
the new, correct, location) in patch#29. (For SEC.)

Yup, that's the SEC one.


For PEI, we have a ZeroMem() call in patch#31.

Yup, right again.


I'm happy with those. (Hopefully I understand the code enough to be
*justifiedly* happy. :))

Yes, you understand the code very well!

Thanks,
Tom


Thanks!
Laszlo


diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c 
b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
index 9d93e30a8ea4..e8f9d3fa01a8 100644
--- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
@@ -126,6 +126,14 @@ UINT64
    SEV_ES_INSTRUCTION_DATA  *InstructionData
    );
+//
+// Per-CPU data mapping structure
+//
+typedef struct {
+  BOOLEAN  Dr7Cached;
+  UINT64   Dr7;
+} SEV_ES_PER_CPU_DATA;
+
/**
    Checks the GHCB to determine if the specified register has been marked 
valid.
@@ -1478,6 +1486,104 @@ RdtscExit (
    return 0;
  }
+/**
+  Handle a DR7 register write event.
+
+  Use the VMGEXIT instruction to handle a DR7 write 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
+
+  @return 0                        Event handled successfully
+  @return Others                   New exception value to propagate
+
+**/
+STATIC
+UINT64
+Dr7WriteExit (
+  IN OUT GHCB                     *Ghcb,
+  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
+  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
+  )
+{
+  SEV_ES_INSTRUCTION_OPCODE_EXT  *Ext;
+  SEV_ES_PER_CPU_DATA            *SevEsData;
+  UINT64                         *Register;
+  UINT64                         Status;
+
+  Ext = &InstructionData->Ext;
+  SevEsData = (SEV_ES_PER_CPU_DATA *) (Ghcb + 1);
+
+  DecodeModRm (Regs, InstructionData);
+
+  //
+  // MOV DRn always treats MOD == 3 no matter how encoded
+  //
+  Register = GetRegisterPointer (Regs, Ext->ModRm.Rm);
+
+  //
+  // Using a value of 0 for ExitInfo1 means RAX holds the value
+  //
+  Ghcb->SaveArea.Rax = *Register;
+  GhcbSetRegValid (Ghcb, GhcbRax);
+
+  Status = VmgExit (Ghcb, SVM_EXIT_DR7_WRITE, 0, 0);
+  if (Status != 0) {
+    return Status;
+  }
+
+  SevEsData->Dr7 = *Register;
+  SevEsData->Dr7Cached = TRUE;
+
+  return 0;
+}
+
+/**
+  Handle a DR7 register read event.
+
+  Use the VMGEXIT instruction to handle a DR7 read 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
+
+  @return 0                        Event handled successfully
+
+**/
+STATIC
+UINT64
+Dr7ReadExit (
+  IN OUT GHCB                     *Ghcb,
+  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
+  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
+  )
+{
+  SEV_ES_INSTRUCTION_OPCODE_EXT  *Ext;
+  SEV_ES_PER_CPU_DATA            *SevEsData;
+  UINT64                         *Register;
+
+  Ext = &InstructionData->Ext;
+  SevEsData = (SEV_ES_PER_CPU_DATA *) (Ghcb + 1);
+
+  DecodeModRm (Regs, InstructionData);
+
+  //
+  // MOV DRn always treats MOD == 3 no matter how encoded
+  //
+  Register = GetRegisterPointer (Regs, Ext->ModRm.Rm);
+
+  //
+  // If there is a cached valued for DR7, return that. Otherwise return the
+  // DR7 standard reset value of 0x400 (no debug breakpoints set).
+  //
+  *Register = (SevEsData->Dr7Cached) ? SevEsData->Dr7 : 0x400;
+
+  return 0;
+}
+
  /**
    Handle a #VC exception.
@@ -1522,6 +1628,14 @@ VmgExitHandleVc ( ExitCode = Regs->ExceptionData;
    switch (ExitCode) {
+  case SVM_EXIT_DR7_READ:
+    NaeExit = Dr7ReadExit;
+    break;
+
+  case SVM_EXIT_DR7_WRITE:
+    NaeExit = Dr7WriteExit;
+    break;
+
    case SVM_EXIT_RDTSC:
      NaeExit = RdtscExit;
      break;




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

View/Reply Online (#61164): https://edk2.groups.io/g/devel/message/61164
Mute This Topic: https://groups.io/mt/74692440/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to