From b52f27f35f4e76ef4adc54350f14972c83755a0a Mon Sep 17 00:00:00 2001 From: Jeff Parsons Date: Sun, 16 Aug 2015 09:38:05 -0700 Subject: [PATCH] Cleaned up fCall handling a bit --- modules/pcjs/lib/x86cpu.js | 7 ++--- modules/pcjs/lib/x86func.js | 12 ++++---- modules/pcjs/lib/x86seg.js | 55 +++++++++++++++++++++++-------------- 3 files changed, 41 insertions(+), 33 deletions(-) diff --git a/modules/pcjs/lib/x86cpu.js b/modules/pcjs/lib/x86cpu.js index d8bbd47ea..1c34b19ca 100644 --- a/modules/pcjs/lib/x86cpu.js +++ b/modules/pcjs/lib/x86cpu.js @@ -2257,12 +2257,11 @@ X86CPU.prototype.setIP = function(off) * @this {X86CPU} * @param {number} off * @param {number} sel - * @param {boolean} [fCall] is true if CALLF in progress, false if RETF/IRET in progress, null/undefined otherwise + * @param {boolean} [fCall] is true if CALLF in progress, false if RETF/IRET in progress, undefined otherwise * @return {boolean|null} true if a stack switch occurred; the only opcode that really needs to pay attention is opRETFn() */ X86CPU.prototype.setCSIP = function(off, sel, fCall) { - this.segCS.fCall = fCall; /* * We break this operation into the following discrete steps (eg, set IP, load CS, and then update IP) so * that segCS.load(sel) has the ability to modify IP when sel refers to a gate (call, interrupt, trap, etc). @@ -2271,9 +2270,8 @@ X86CPU.prototype.setCSIP = function(off, sel, fCall) * internal instruction pointer. Callers that need the real IP must call getIP(). */ this.regEIP = off; - var base = this.segCS.load(sel); + var base = this.segCS.loadCode(sel, fCall); if (base !== X86.ADDR_INVALID) { - /* * TODO: Should this code be factored into a setLIP() function? The other primary client would be fnINT(). */ @@ -2282,7 +2280,6 @@ X86CPU.prototype.setCSIP = function(off, sel, fCall) this.regLIPLimit = (base + this.segCS.limit)|0; this.nCPL = this.segCS.cpl; // cache the current CPL where it's more convenient if (PREFETCH) this.flushPrefetch(this.regLIP); - return this.segCS.fStackSwitch; } return null; diff --git a/modules/pcjs/lib/x86func.js b/modules/pcjs/lib/x86func.js index b8e92d3b6..f541e5b7a 100644 --- a/modules/pcjs/lib/x86func.js +++ b/modules/pcjs/lib/x86func.js @@ -1217,13 +1217,12 @@ X86.fnINCw = function INCw(dst, src) /** * fnINT(nIDT, nError, nCycles) * - * NOTE: We no longer use setCSIP(), because it always loads the new CS using segCS.load(), which - * only knows how to load GDT and LDT descriptors, whereas interrupts must use setCS.loadIDT(), which - * deals exclusively with IDT descriptors. + * NOTE: We no longer use setCSIP(), because it always loads the new CS using segCS.load(), which only knows + * how to load GDT and LDT descriptors, whereas interrupts must use setCS.loadIDT(), which deals exclusively + * with IDT descriptors. * - * This means we must take care to replicate critical features of setCSIP(); ie, setting segCS.fCall - * BEFORE calling loadIDT(), and updating regLIP and regLIPLimit, resetting default operand and address sizes, - * and flushing the prefetch queue AFTER calling loadIDT(). + * This means we must take care to replicate critical features of setCSIP(); ie, updating regLIP and regLIPLimit, + * resetting default operand and address sizes, and flushing the prefetch queue AFTER calling loadIDT(). * * @this {X86CPU} * @param {number} nIDT @@ -1236,7 +1235,6 @@ X86.fnINT = function INT(nIDT, nError, nCycles) * TODO: We assess the cycle cost up front, because otherwise, if loadIDT() fails, no cost may be assessed. */ this.nStepCycles -= this.cycleCounts.nOpCyclesInt + nCycles; - this.segCS.fCall = true; var oldPS = this.getPS(); var oldCS = this.getCS(); var oldIP = this.getIP(); diff --git a/modules/pcjs/lib/x86seg.js b/modules/pcjs/lib/x86seg.js index c7fea6fb5..8061ca10f 100644 --- a/modules/pcjs/lib/x86seg.js +++ b/modules/pcjs/lib/x86seg.js @@ -46,11 +46,11 @@ if (typeof module !== 'undefined') { * with some of the early changes (eg, skipping X86.DESC.EXT.BASE2431 and X86.DESC.EXT.LIMIT1619 * fields unless the processor is an 80386). * - * However, the reality is that I won't always be that strict, either because I'm lazy or because - * any 80286 code you're likely to run probably won't attempt to use descriptor types or other features - * unique to the 80386 anyway, so the extra paranoia may not be worth the effort. - * - * But still, we should all want to live in a perfect world. Someday. + * However, the reality is that I won't always be that strict, either because I'm lazy or I don't + * want to risk a run-time performance hit or (more pragmatically) because any 80286 code you're likely + * to run probably won't attempt to use descriptor types or other features unique to the 80386 anyway, + * so the extra paranoia may not be worth the effort. Ultimately, I would like to see the code tailor + * itself to the current CPU model, generally with model-specific functions, but that's a lot of work. */ /** @@ -64,7 +64,9 @@ if (typeof module !== 'undefined') { * * TODO: Determine what good, if any, these class annotations are for either an IDE like WebStorm or a tool like * the Closure Compiler. More importantly, what good do they do at runtime? Is it better to simply ensure that all - * object properties are explicitly initialized in the constructor, and document them there instead? + * object properties are explicitly initialized in the constructor, and document them there instead? I started by + * listing only what might be considered "public" properties above, in an effort to eliminate WebStorm inspection + * warnings, but it didn't seem to help, so I stopped. */ /** @@ -104,23 +106,18 @@ function X86Seg(cpu, id, sName, fProt) * 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. + * The typical ways of loading a new segment into CS are JMPF, CALLF (or INT), and RETF (or IRET), + * via CPU functions setCSIP() and fnINT(), which use segCS.loadCode() and segCS.loadIDT(), 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 restore and a privilege transition - * to a numerically greater privilege. + * loadCode() requires an fCall value: null means NO privilege level transition may occur, true + * allows a stack switch and a privilege transition to a numerically lower privilege, and false allows + * a stack restore and a privilege transition to a numerically greater privilege. * - * As long as setCSIP() or fnINT() 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 is that such a parameter is meaningless for segments other than segCS. + * loadIDT() sets fCall to true unconditionally in protected-mode (fCall has no meaning in real-mode). */ - this.awParms = (this.id == X86Seg.ID.CODE? new Array(32) : []); this.fCall = null; this.fStackSwitch = false; + this.awParms = (this.id == X86Seg.ID.CODE? new Array(32) : []); this.updateMode(true, fProt); } @@ -135,6 +132,22 @@ X86Seg.ID = { DBG: 7 // "DBG" }; +/** + * loadCode(sel, fCall) + * + * A simple wrapper function that encapsulates setting the fCall property for segCS loads. + * + * @this {X86Seg} + * @param {number} sel + * @param {boolean|undefined} fCall is true if CALLF in progress, false if RETF/IRET in progress, undefined otherwise + * @return {number} base address of selected segment, or ADDR_INVALID if error + */ +X86Seg.prototype.loadCode = function loadCode(sel, fCall) +{ + this.fCall = fCall; + return this.load(sel); +}; + /** * loadReal(sel) * @@ -268,6 +281,7 @@ X86Seg.prototype.loadIDTProt = function loadIDTProt(nIDT) nIDT <<= 3; var addrDesc = (cpu.addrIDT + nIDT)|0; if (((cpu.addrIDTLimit - addrDesc)|0) >= 7) { + this.fCall = true; return this.loadDesc8(addrDesc, nIDT) + cpu.regEIP; } X86.fnFault.call(cpu, X86.EXCEPTION.GP_FAULT, nIDT | X86.ERRCODE.IDT | X86.ERRCODE.EXT, true); @@ -1010,9 +1024,8 @@ X86Seg.prototype.switchTSS = function switchTSS(selNew, fNest) /** * setBase(addr) * - * This is used in unusual situations where the base must be set independently; normally, the base - * is set according to the selector provided to load(), but there are a few cases where setBase() - * is required. + * This is used in unusual situations where the base must be set independently; normally, the base is + * set according to the selector provided to load(), but there are a few cases where setBase() is required. * * For example, in resetRegs(), the real-mode CS selector must be reset to 0xF000 for an 80286 or 80386, * but the CS base must be set to 0x00FF0000 or 0xFFFF0000, respectively. To simplify life for setBase()