From 5d2a7b13a31b1e3a09abf9a8ae761d3c8fe472d6 Mon Sep 17 00:00:00 2001
From: Jeff Parsons <Jeff@pcjs.org>
Date: Sun, 3 May 2015 10:11:18 -0700
Subject: [PATCH] Fixed SGDT/SIDT (hopefully)

---
 modules/pcjs/lib/rom.js     |  2 +-
 modules/pcjs/lib/x86func.js | 34 ++++++++++++++++++++++++++++++++--
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/modules/pcjs/lib/rom.js b/modules/pcjs/lib/rom.js
index c75a802fa..d001e5d3e 100644
--- a/modules/pcjs/lib/rom.js
+++ b/modules/pcjs/lib/rom.js
@@ -282,7 +282,7 @@ ROM.prototype.copyROM = function()
             if (this.abROM.length != this.sizeROM) {
                 /*
                  * Note that setError() sets the component's fError flag, which in turn prevents setReady() from
-                 * marking the component ready.  TODO: Revisit this decision.  One the one hand, it sounds like a
+                 * marking the component ready.  TODO: Revisit this decision.  On the one hand, it sounds like a
                  * good idea to stop the machine in its tracks whenever a setError() occurs, but there may also be
                  * times when we'd like to forge ahead anyway.
                  */
diff --git a/modules/pcjs/lib/x86func.js b/modules/pcjs/lib/x86func.js
index f7d898a19..a169f4702 100644
--- a/modules/pcjs/lib/x86func.js
+++ b/modules/pcjs/lib/x86func.js
@@ -2599,6 +2599,7 @@ X86.fnSGDT = function SGDT(dst, src)
          * calls us does that automatically with the value we return (dst).
          */
         dst = this.addrGDTLimit - this.addrGDT;
+        this.assert(!(dst & ~0xffff));
         /*
          * We previously left the 6th byte of the target operand "undefined".  But it turns out we have to set
          * it to *something*, because there's processor detection in PC-DOS 7.0 (at least in the SETUP portion)
@@ -2628,8 +2629,26 @@ X86.fnSGDT = function SGDT(dst, src)
          *      145E:4BC3 CB            RETF
          *
          * This code is expecting SGDT on an 80286 to set the 6th "undefined" byte to 0xFF.
+         *
+         * The 80386 adds an additional wrinkle: the 6th byte must be 0x00 if the OPERAND size is 2, whereas
+         * it must passed through if the OPERAND size is 4.
+         *
+         * In addition, when the OPERAND size is 4, the ModRM group decoder will call setLong(dst) rather than
+         * setShort(dst); we could fix that by forcing the dataSize to 2, but it seems simpler to set the high
+         * bits (16-31) of dst to match the low bits (0-15) of addr, so that the caller will harmlessly rewrite
+         * what we already wrote with the setLong() below.
          */
-        var addr = this.addrGDT | (this.model == X86.MODEL_80286? (0xff000000|0) : 0);
+        var addr = this.addrGDT;
+        if (this.model == X86.MODEL_80286) {
+            addr |= (0xff000000|0);
+        }
+        else if (this.model >= X86.MODEL_80386) {
+            if (this.dataSize == 2) {
+                addr &= 0x00ffffff;
+            } else {
+                dst |= (addr << 16);
+            }
+        }
         this.setLong(this.regEA + 2, addr);
         this.nStepCycles -= 11;
     }
@@ -2977,11 +2996,22 @@ X86.fnSIDT = function SIDT(dst, src)
          * us does that automatically with the value we return (dst).
          */
         dst = this.addrIDTLimit - this.addrIDT;
+        this.assert(!(dst & ~0xffff));
         /*
          * As with SGDT, the 6th byte is technically "undefined" on an 80286, but we now set it to 0xFF, for the
          * same reasons discussed in SGDT (above).
          */
-        var addr = this.addrIDT | (this.model == X86.MODEL_80286? (0xff000000|0) : 0);
+        var addr = this.addrIDT;
+        if (this.model == X86.MODEL_80286) {
+            addr |= (0xff000000|0);
+        }
+        else if (this.model >= X86.MODEL_80386) {
+            if (this.dataSize == 2) {
+                addr &= 0x00ffffff;
+            } else {
+                dst |= (addr << 16);
+            }
+        }
         this.setLong(this.regEA + 2, addr);
         this.nStepCycles -= 12;
     }