PeCoffExtraActionLibDebug uses the debug registers to pass module load
information to the DebugAgent, then restores the old register values.
However, it was missing code to restore Dr7 in the
DEBUG_LOAD_IMAGE_METHOD_SOFT_INT3 case.  This broke hardware
breakpoints and watchpoints.  It could also lose modifications the
debugger made to Cr4.

Restore the Dr7 and Cr4 values correctly in the
DEBUG_LOAD_IMAGE_METHOD_SOFT_INT3 case, as well as the
DEBUG_LOAD_IMAGE_METHOD_IO_HW_BREAKPOINT case.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brian J. Johnson <[email protected]>
---

This fix has been built with OvmfPkg/OvmfPkgIa32X64.dsc, using the GCC48 
toolchain under Ubuntu.  It has been tested (32- and 64-bit mode) using 
qemu with tcg and a proprietary DebugAgentLib.  We've also used it on 
hardware in a proprietary BIOS with various Intel CPUs, using the 
SourceLevelDebugPkg DebugAgentLib (an older version).  It's been built 
successfully with the WINDDK compilers, but I'm not really in a position 
to build with MSVC.

A note on Dr7 bit 10:  the Intel SDM says that this bit always reads as 
1, and that's the behavior I see on hardware.  (The latest SDM says it 
reads as 0 in 32-bit mode, but this appears to be a typo.  A case is 
open with Intel.)  However, on simulators, including qemu in tcg mode, 
it often reads as 0.  So we force it to 1 unconditionally to simplify 
later code.

Changes in v2:
- Generalized comment wording
- Properly restore Cr4 as well

Changes in v3:
- Set bit 10 when reading Dr7, and simplify the test if Dr7 has changed. 
  This has the side effect of writing Dr7=BIT10 if it was read as 0 (on 
simulators), but this should be harmless.

Thanks,
Brian

   My statements are my own, are not authorized by SGI, and do not
   necessarily represent SGI’s positions.

  .../PeCoffExtraActionLib.c                         |   20 
+++++++++++++-------
  1 file changed, 13 insertions(+), 7 deletions(-)

diff --git 
a/SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/PeCoffExtraActionLib.c 
b/SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/PeCoffExtraActionLib.c
index 9bf76bf..f45e7aa 100644
--- 
a/SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/PeCoffExtraActionLib.c
+++ 
b/SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/PeCoffExtraActionLib.c
@@ -104,7 +104,7 @@ PeCoffLoaderExtraActionCommon (
    Dr1 = AsmReadDr1 ();
    Dr2 = AsmReadDr2 ();
    Dr3 = AsmReadDr3 ();
-  Dr7 = AsmReadDr7 ();
+  Dr7 = AsmReadDr7 () | BIT10; // H/w sets bit 10, some simulators don't
    Cr4 = AsmReadCr4 ();

    //
@@ -144,7 +144,7 @@ PeCoffLoaderExtraActionCommon (
    // E.g.: User halts the target and sets the HW breakpoint while 
target is
    //       in the above exception handler
    //
-  NewDr7 = AsmReadDr7 ();
+  NewDr7 = AsmReadDr7 () | BIT10; // H/w sets bit 10, some simulators don't
    if (!IsDrxEnabled (0, NewDr7) && (AsmReadDr0 () == 0 || AsmReadDr0 
() == Signature)) {
      //
      // If user changed Dr3 (by setting HW bp in the above exception 
handler,
@@ -161,11 +161,17 @@ PeCoffLoaderExtraActionCommon (
    if (!IsDrxEnabled (3, NewDr7) && (AsmReadDr3 () == 
IO_PORT_BREAKPOINT_ADDRESS)) {
      AsmWriteDr3 (Dr3);
    }
-  if (AsmReadCr4 () == (Cr4 | BIT3)) {
-    AsmWriteCr4 (Cr4);
-  }
-  if (NewDr7 == 0x20000480) {
-    AsmWriteDr7 (Dr7);
+  if (LoadImageMethod == DEBUG_LOAD_IMAGE_METHOD_IO_HW_BREAKPOINT) {
+    if (AsmReadCr4 () == (Cr4 | BIT3)) {
+      AsmWriteCr4 (Cr4);
+    }
+    if (NewDr7 == 0x20000480) {
+      AsmWriteDr7 (Dr7);
+    }
+  } else if (LoadImageMethod == DEBUG_LOAD_IMAGE_METHOD_SOFT_INT3) {
+    if (NewDr7 == BIT10) {
+      AsmWriteDr7 (Dr7);
+    }
    }
    //
    // Restore original IDT entry for INT1 if it was hooked.

------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the 
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to