From d628eacfc6bd963233ab1a2b3331d03409c7d200 Mon Sep 17 00:00:00 2001 From: Jeff Parsons Date: Mon, 1 Dec 2014 14:50:38 -0800 Subject: [PATCH] Factored common RETF logic into opHelpRETF --- modules/pcjs/lib/x86cpu.js | 8 +++---- modules/pcjs/lib/x86grps.js | 2 +- modules/pcjs/lib/x86help.js | 44 ++++++++++++++++++++++++++++++++++--- modules/pcjs/lib/x86opxx.js | 13 +++-------- modules/pcjs/lib/x86seg.js | 42 +++++++++++++++++++---------------- 5 files changed, 72 insertions(+), 37 deletions(-) diff --git a/modules/pcjs/lib/x86cpu.js b/modules/pcjs/lib/x86cpu.js index 33ccf34df..999e2a868 100644 --- a/modules/pcjs/lib/x86cpu.js +++ b/modules/pcjs/lib/x86cpu.js @@ -518,7 +518,7 @@ X86CPU.prototype.setAddressMask = function(addrMask) * Microprocessor, March 1983" (pp.55-56). "The iAPX 86,88 and iAPX 186,188 User's Manual Programmer's * Reference", p.3-38, apparently contains the same information, but I've not seen that document. * - * Undefined Opcodes: + * Undefined [Invalid] Opcodes: * * When the opcodes 63H, 64H, 65H, 66H, 67H, F1H, FEH/xx111xxxB and FFH/xx111xxxB are executed, * the 80186 will execute an illegal [invalid] instruction exception, interrupt 0x06. @@ -610,9 +610,9 @@ X86CPU.prototype.setAddressMask = function(addrMask) * * 6. Do Not Attempt Undefined 8086/8088 Operations * - * Instructions like POP CS or MOV CS,op will either cause exception 6 (undefined opcode) or perform a protection - * setup operation like LIDT on the 80286. Undefined bit encodings for bits 5-3 of the second byte of POP MEM - * or PUSH MEM will cause exception 13 on the 80286. + * Instructions like POP CS or MOV CS,op will either cause exception 6 (undefined [invalid] opcode) or perform + * a protection setup operation like LIDT on the 80286. Undefined bit encodings for bits 5-3 of the second byte + * of POP MEM or PUSH MEM will cause exception 13 on the 80286. * * 7. Place a Far JMP Instruction at FFFF0H * diff --git a/modules/pcjs/lib/x86grps.js b/modules/pcjs/lib/x86grps.js index 67dd8bfd7..6a6b3d2cf 100644 --- a/modules/pcjs/lib/x86grps.js +++ b/modules/pcjs/lib/x86grps.js @@ -1128,7 +1128,7 @@ var X86Grps = { if (this.regEA < 0) { return X86Grps.opGrpUndefined.call(this, dst, src); } - X86Help.opHelpCallF.call(this, dst, this.getWord(this.regEA + 2)); + X86Help.opHelpCALLF.call(this, dst, this.getWord(this.regEA + 2)); this.nStepCycles -= this.CYCLES.nOpCyclesCallDM; if (EAFUNCS) this.setEAWord = this.setEAWordDisabled; else this.opFlags |= X86.OPFLAG.NOWRITE; return dst; diff --git a/modules/pcjs/lib/x86help.js b/modules/pcjs/lib/x86help.js index e0fe3dee1..bed81af1b 100644 --- a/modules/pcjs/lib/x86help.js +++ b/modules/pcjs/lib/x86help.js @@ -222,7 +222,7 @@ var X86Help = { opHelpBOUND: function(dst, src) { if (this.regEA < 0) { /* - * Generate a #UD fault (INT 0x06: Undefined Opcode) if src is not a memory operand. + * Generate UD_FAULT (INT 0x06: Invalid Opcode) if src is not a memory operand. */ X86OpXX.opInvalid.call(this); return dst; @@ -436,7 +436,7 @@ var X86Help = { }, /** - * opHelpCallF(off, sel) + * opHelpCALLF(off, sel) * * For protected-mode, this function must attempt to load the new code segment first, because if the new segment * requires a change in privilege level, the return address must be pushed on the NEW stack, not the current stack. @@ -445,7 +445,7 @@ var X86Help = { * @param {number} off * @param {number} sel */ - opHelpCallF: function(off, sel) { + opHelpCALLF: function(off, sel) { var regCS = this.segCS.sel; var regIP = this.regIP; if (this.setCSIP(off, sel, true) != null) { @@ -453,6 +453,44 @@ var X86Help = { this.pushWord(regIP); } }, + /** + * opHelpRETF(n) + * + * For protected-mode, this function must be prepared to pop any arguments off the current stack AND + * whatever stack we may have switched to (setCSIP() returns true only when a stack switch has occurred). + * + * @this {X86CPU} + * @param {number} n + */ + opHelpRETF: function(n) { + var regIP = this.popWord(); + var regCS = this.popWord(); + if (n) this.regSP = (this.regSP + n) & 0xffff; + if (this.setCSIP(regIP, regCS, false)) { + if (n) this.regSP = (this.regSP + n) & 0xffff; + /* + * As per Intel documentation: "If any of [the DS or ES] registers refer to segments whose DPL is + * less than the new CPL (excluding conforming code segments), the segment register is loaded with + * the null selector." + * + * TODO: I'm not clear on whether a conforming code segment must also be marked readable, so I'm playing + * it safe and using CODE_CONFORMING instead of CODE_CONFORMING_READABLE. Also, for the record, I've not + * seen this situation occur in OS/2 1.0 yet. + */ + if ((this.segDS.sel & X86.SEL.MASK) && this.segDS.dpl < this.segCS.cpl && (this.segDS.acc & X86.DESC.ACC.TYPE.CODE_CONFORMING) != X86.DESC.ACC.TYPE.CODE_CONFORMING) { + this.assert(false); // I'm not asserting this is bad, I just want to see it in action + this.segDS.load(0); + } + if ((this.segES.sel & X86.SEL.MASK) && this.segES.dpl < this.segCS.cpl && (this.segES.acc & X86.DESC.ACC.TYPE.CODE_CONFORMING) != X86.DESC.ACC.TYPE.CODE_CONFORMING) { + this.assert(false); // I'm not asserting this is bad, I just want to see it in action + this.segES.load(0); + } + } + /* + * We check for possible "INT n" software interrupt returns only in the cases of "IRET" and "RETF 2". + */ + if (n == 2 && this.cIntReturn) this.checkIntReturn(this.regEIP); + }, /** * opHelpINT(nIDT, nError, nCycles) * diff --git a/modules/pcjs/lib/x86opxx.js b/modules/pcjs/lib/x86opxx.js index 78ad90f3b..4d65ecfd8 100644 --- a/modules/pcjs/lib/x86opxx.js +++ b/modules/pcjs/lib/x86opxx.js @@ -1971,7 +1971,7 @@ var X86OpXX = { * op=0x9A (call seg:off) */ opCALLF: function() { - X86Help.opHelpCallF.call(this, this.getIPWord(), this.getIPWord()); + X86Help.opHelpCALLF.call(this, this.getIPWord(), this.getIPWord()); this.nStepCycles -= this.CYCLES.nOpCyclesCallF; }, /** @@ -2784,14 +2784,7 @@ var X86OpXX = { * op=0xCA (retf n) */ opRETFn: function() { - var n = this.getIPWord(); - var regIP = this.popWord(); - var regCS = this.popWord(); - this.regSP = (this.regSP + n) & 0xffff; - if (this.setCSIP(regIP, regCS, false)) { - this.regSP = (this.regSP + n) & 0xffff; - } - if (this.cIntReturn) this.checkIntReturn(this.regEIP); + X86Help.opHelpRETF.call(this, this.getIPWord()); this.nStepCycles -= this.CYCLES.nOpCyclesRetFn; }, /** @@ -2800,7 +2793,7 @@ var X86OpXX = { * op=0xCB (retf) */ opRETF: function() { - this.setCSIP(this.popWord(), this.popWord(), false); + X86Help.opHelpRETF.call(this, 0); this.nStepCycles -= this.CYCLES.nOpCyclesRetF; }, /** diff --git a/modules/pcjs/lib/x86seg.js b/modules/pcjs/lib/x86seg.js index a67950ff8..1b6ccd8ba 100644 --- a/modules/pcjs/lib/x86seg.js +++ b/modules/pcjs/lib/x86seg.js @@ -45,7 +45,7 @@ if (typeof module !== 'undefined') { * @param {X86CPU} cpu * @param {number} id * @param {string} [sName] segment name - * @param {boolean} [fProt] true if segment register used exclusively in protected-mode + * @param {boolean} [fProt] true if segment register used exclusively in protected-mode (eg, segLDT) */ function X86Seg(cpu, id, sName, fProt) { @@ -64,26 +64,25 @@ function X86Seg(cpu, id, sName, fProt) * The following properties are used for CODE segments only (ie, segCS); if the process of loading * CS also requires a stack switch, then fStackSwitch will be set to true; additionally, if the stack * switch was the result of a CALL (ie, fCall is true) and one or more (up to 32) parameters are on - * the old stack, they will be copied to awScratch, and then once the stack is switched, the parameters - * will be pushed from awScratch onto the new stack. + * the old stack, they will be copied to awParms, and then once the stack is switched, the parameters + * will be pushed from awParms onto the new stack. * * The typical ways of loading a new segment into CS are JMPF, CALLF (or INT), and RETF (or IRET); * prior to calling segCS.load(), each of those operations must first set segCS.fCall to one of null, * true, or false, respectively. * - * It's critical that fCall be properly set prior to calling segCS.load(); fCall == null means NO - * privilege level transition may occur, fCall == true allows a stack switch and a privilege transition - * to a numerically lower privilege, and fCall == false allows a stack switch (restore) and a privilege - * transition to a numerically greater privilege. + * It's critical that fCall be properly set prior to calling segCS.load(); fCall === null means NO + * privilege level transition may occur, fCall === true allows a stack switch and a privilege transition + * to a numerically lower privilege, and fCall === false allows a stack restore and a privilege transition + * to a numerically greater privilege. * - * As long as setCSIP() or opHelpINT() are used for all CS changes, the foregoing is automatically - * taken care of. + * As long as setCSIP() or opHelpINT() are used for all CS changes, fCall is set automatically. * * TODO: Consider making fCall a parameter to load(), instead of a property that must be set prior to * calling load(); the downside (and why I didn't do that in the first place) is that such a parameter * is meaningless for segments other than segCS. */ - this.awScratch = (this.id == X86Seg.ID.CODE? new Array(32) : []); + this.awParms = (this.id == X86Seg.ID.CODE? new Array(32) : []); this.fCall = null; this.fStackSwitch = false; this.updateMode(fProt); @@ -136,7 +135,7 @@ X86Seg.loadReal = function loadReal(sel, fSuppress) * * See X86.DESC for offset and bit definitions. * - * IDT descriptor entries are handled separately by loadIDT(). + * IDT descriptor entries are handled separately by loadIDT(), which is mapped to loadRealIDT() or loadProtIDT(). * * @this {X86Seg} * @param {number} sel @@ -379,7 +378,7 @@ X86Seg.checkWriteProtDisallowed = function checkWriteProtDisallowed(off, cb, fSu * 0090:067C EBFD JMP 067B * * but it may not have yet reprogrammed the master PIC to re-vector hardware interrupts to IDT entries 0x50-0x57, - * so when the next timer interrupt (IRQ 0) occurs, it vectors through IDT entry 0x08, which is the double-fault + * so when the next timer interrupt (IRQ 0) occurs, it vectors through IDT entry 0x08, which is the DF_FAULT * vector. A spurious double-fault is generated, and a clean shutdown turns into a messy crash. * * Of course, that all could have been avoided if IBM had heeded Intel's advice and not used Intel-reserved IDT @@ -560,6 +559,10 @@ X86Seg.prototype.loadDesc8 = function(addrDesc, sel, fSuppress) var accCode, selCode, cplPrev, addrTSS, offSP, offSS, regSPPrev, regSSPrev; + /* + * TODO: Consider moving the following chunks of code into worker functions for each X86Seg.ID; + * however, it's not clear that these tests are more costly than making additional function calls. + */ if (this.id == X86Seg.ID.CODE) { this.fStackSwitch = false; var fCall = this.fCall; @@ -582,6 +585,10 @@ X86Seg.prototype.loadDesc8 = function(addrDesc, sel, fSuppress) else if (type == X86.DESC.ACC.TYPE.GATE_CALL) { /* * Since we are X86Seg.ID.CODE, we can use this.cpl instead of the more generic cpu.segCS.cpl + * + * TODO: Consider factoring the GATE_CALL code, and the GATE_INT/GATE_TRAP code below it, into + * something that can be shared; the main differences are privilege level checks, parameter copying, + * and fault generation on error. */ selCode = base & 0xffff; if (rpl < this.cpl) rpl = this.cpl; @@ -608,7 +615,7 @@ X86Seg.prototype.loadDesc8 = function(addrDesc, sel, fSuppress) regSP = cpu.regSP; var i = 0, nWords = (acc & 0x1f); while (nWords--) { - this.awScratch[i++] = cpu.getSOWord(cpu.segSS, regSP); + this.awParms[i++] = cpu.getSOWord(cpu.segSS, regSP); regSP += 2; } addrTSS = cpu.segTSS.base; @@ -620,7 +627,7 @@ X86Seg.prototype.loadDesc8 = function(addrDesc, sel, fSuppress) cpu.segSS.load(cpu.getWord(addrTSS + offSS)); cpu.pushWord(regSSPrev); cpu.pushWord(regSPPrev); - while (i) cpu.pushWord(this.awScratch[--i]); + while (i) cpu.pushWord(this.awParms[--i]); this.fStackSwitch = true; } return this.base; @@ -663,11 +670,8 @@ X86Seg.prototype.loadDesc8 = function(addrDesc, sel, fSuppress) cpu.pushWord(regSPPrev); this.fStackSwitch = true; } - if (type == X86.DESC.ACC.TYPE.GATE_INT) { - cpu.regPS &= ~(X86.PS.NT | X86.PS.TF | X86.PS.IF); - } else { - cpu.regPS &= ~(X86.PS.NT | X86.PS.TF); - } + cpu.regPS &= ~(X86.PS.NT | X86.PS.TF); + if (type == X86.DESC.ACC.TYPE.GATE_INT) cpu.regPS &= ~X86.PS.IF; return this.base; } cpu.assert(false);