From 42cf78b8e0bde052258b448a6666cfeb5350878a Mon Sep 17 00:00:00 2001 From: Jeff Parsons Date: Tue, 12 May 2015 15:01:03 -0700 Subject: [PATCH] Fix segment ACCESSED tracking --- modules/pcjs/bin/romtests.json | 2 +- modules/pcjs/lib/memory.js | 15 ++++--- modules/pcjs/lib/x86cpu.js | 37 ++++++++++------ modules/pcjs/lib/x86seg.js | 79 +++++++++++++++------------------- tests/pc/80386/tests.nasm | 62 ++++++++++++++++++++++---- 5 files changed, 121 insertions(+), 74 deletions(-) diff --git a/modules/pcjs/bin/romtests.json b/modules/pcjs/bin/romtests.json index a40b43ce4..34b4ee731 100644 --- a/modules/pcjs/bin/romtests.json +++ b/modules/pcjs/bin/romtests.json @@ -78,7 +78,7 @@ "debugger": { "id": "pc386.debugger", "name": "", - "commands": "", + "commands": "bp F000:8271;g", "messages": "" } } diff --git a/modules/pcjs/lib/memory.js b/modules/pcjs/lib/memory.js index 6e1077053..a3a80fccd 100644 --- a/modules/pcjs/lib/memory.js +++ b/modules/pcjs/lib/memory.js @@ -110,6 +110,7 @@ function Memory(addr, used, size, type, controller, cpu) this.controller = null; this.cpu = cpu; this.fDirty = this.fDirtyEver = false; + this.setPhysBlock(); if (BACKTRACK) { if (!size || controller) { @@ -501,11 +502,11 @@ Memory.prototype = { * setPhysBlock(blockPhys, blockPDE, offPDE, blockPTE, offPTE) * * @this {Memory} - * @param {Memory} blockPhys - * @param {Memory} blockPDE - * @param {number} offPDE - * @param {Memory} blockPTE - * @param {number} offPTE + * @param {Memory|null} [blockPhys] + * @param {Memory|null} [blockPDE] + * @param {number} [offPDE] + * @param {Memory|null} [blockPTE] + * @param {number} [offPTE] */ setPhysBlock: function(blockPhys, blockPDE, offPDE, blockPTE, offPTE) { this.blockPhys = blockPhys; @@ -513,8 +514,8 @@ Memory.prototype = { this.iPDE = offPDE >> 2; // convert offPDE into iPDE (an adw index) this.blockPTE = blockPTE; this.iPTE = offPTE >> 2; // convert offPTE into iPTE (an adw index) - this.bitPTEDirty = Memory.adjustEndian(X86.PTE.ACCESSED | X86.PTE.DIRTY); - this.bitPTEAccessed = Memory.adjustEndian(X86.PTE.ACCESSED); + this.bitPTEDirty = blockPhys? Memory.adjustEndian(X86.PTE.ACCESSED | X86.PTE.DIRTY) : 0; + this.bitPTEAccessed = blockPhys? Memory.adjustEndian(X86.PTE.ACCESSED) : 0; }, /** * addBreakpoint(off, fWrite) diff --git a/modules/pcjs/lib/x86cpu.js b/modules/pcjs/lib/x86cpu.js index 70aa738c9..4f0e64e8f 100644 --- a/modules/pcjs/lib/x86cpu.js +++ b/modules/pcjs/lib/x86cpu.js @@ -46,11 +46,21 @@ if (typeof module !== 'undefined') { } if (!I386) { + /* + * These are the original ModRM decoders, which were simpler and faster because they could treat all + * word instructions as 16-bit, assume bits 15-31 of all registers were always zero, and use masking + * constants instead of variables. If I386 is enabled, these decoders are not used, and the compiler + * will eliminate them from the compiled code. + */ if (typeof module !== 'undefined') { var X86ModB = require("./x86modb"); var X86ModW = require("./x86modw"); } } else { + /* + * These are the more general-purpose ModRM decoders, required for I386 suppport. The current addressing + * mode (16-bit or 32-bit) dynamically selects the appropriate byte and word decoders. + */ if (typeof module !== 'undefined') { var X86ModB16 = require("./x86modb16"); var X86ModW16 = require("./x86modw16"); @@ -171,7 +181,6 @@ function X86CPU(parmsCPU) this.aBusBlocks = this.aMemBlocks = []; this.nBusMask = this.nMemMask = 0; this.nBlockShift = this.nBlockSize = this.nBlockLimit = this.nBlockTotal = this.nBlockMask = 0; - this.blockDummy = null; if (SAMPLER) { /* @@ -428,6 +437,10 @@ X86CPU.CYCLES_80286 = { nOpCyclesXLAT: 5 }; +/* + * TODO: Except for the cycle counts at the end of this table (ie, those marked "unique to the 80386"), all these + * values were simply copied from the 80286 table and still need to be modified and verified. + */ X86CPU.CYCLES_80386 = { nWordCyclePenalty: 0, nEACyclesBase: 0, @@ -658,14 +671,14 @@ X86CPU.prototype.initMemory = function(aMemBlocks, nBlockShift) * For 32-bit bus machines (eg, 80386), nBusMask is never changed after the initial call, because A20 * wrap-around is simulated by changing the physical memory map rather than altering the A20 bit in nBusMask. * - * We maintain memMask separate from nBusMask, because when paging is enabled on the 80386, the CPU memory + * We maintain nMemMask separate from nBusMask, because when paging is enabled on the 80386, the CPU memory * functions are now dealing with linear addresses rather than physical addresses, so it would be incorrect - * to apply nBusMask to those addresses; memMask must remain 0xffffffff (-1) for the duration. If we change + * to apply nBusMask to those addresses; nMemMask must remain 0xffffffff (-1) for the duration. If we change * how A20 is simulated on the 80386, then enablePageBlocks() and disablePageBlocks() will need to override - * memMask appropriately. + * nMemMask appropriately. * * TODO: Ideally, we would eliminate masking altogether of 32-bit addresses, but that would require different - * sets of memory access functions for different machines. + * sets of memory access functions for different machines (ie, those with 20-bit or 24-bit buses). * * @this {X86CPU} * @param {number} nBusMask @@ -688,7 +701,7 @@ X86CPU.prototype.setAddressMask = function(nBusMask) * reinitialized with special UNPAGED Memory blocks that know how to perform page directory/page table * lookup and replace themselves with special PAGED Memory blocks that reference memory from the * appropriate block in aBusBlocks. A parallel array, aBlocksPaged, keeps track (by block number) of - * which blocks have been PAGED, so that whenever CR3 is updated, those blocks can be UNPAGED again. + * which blocks have been PAGED, so that whenever CR3 is updated, those blocks can be quickly UNPAGED. * * @this {X86CPU} */ @@ -1181,7 +1194,7 @@ X86CPU.prototype.resetRegs = function() /* * Another internal "register" we occasionally need is an interim copy of bModRM, set inside selected opcode * handlers so that the helper function can have access to the instruction's bModRM without resorting to a - * closure (which, in the Chrome V8 engine, for example, seems to cause constant recompilation). + * closure (which, in the Chrome V8 engine, for example, may cause constant recompilation). */ this.bModRM = 0; @@ -3557,10 +3570,10 @@ X86CPU.prototype.pushWord = function(w) * I/O (ie, even while we're waiting for the remote I/O to finish), so the ChipSet component should avoid * calling setDMA() whenever possible. * - * TODO: While comparing SYMDEB tracing in both PCjs and VMware, I noticed that after single-stepping - * ANY segment-load instruction, SYMDEB would get control immediately after that instruction in VMware, - * whereas I delay acknowledgment of the Trap flag until the *following* instruction, so in PCjs, SYMDEB - * doesn't get control until the following instruction. I think PCjs behavior is correct, at least for SS. + * TODO: While comparing SYMDEB tracing in both PCjs and VMware, I noticed that after single-stepping ANY + * segment-load instruction, SYMDEB would get control immediately after that instruction in VMware, whereas + * I delay acknowledgment of the Trap flag until the *following* instruction, so in PCjs, SYMDEB doesn't get + * control until the following instruction. I think PCjs behavior is correct, at least for SS. * * ERRATA: Early revisions of the 8086/8088 failed to suppress hardware interrupts (and possibly also Trap * acknowledgements) after an SS load, but Intel corrected the problem at some point; however, I don't know when @@ -3588,7 +3601,7 @@ X86CPU.prototype.checkINTR = function() * whereas the 80286 and up give TRAPs higher priority. */ var iPriority = (this.model < X86.MODEL_80286? 0 : 1); - for (var nPriorities = 0; nPriorities < 2; nPriorities++) { + for (var cPriorities = 0; cPriorities < 2; cPriorities++) { switch(iPriority) { case 0: if ((this.intFlags & X86.INTFLAG.INTR) && (this.regPS & X86.PS.IF)) { diff --git a/modules/pcjs/lib/x86seg.js b/modules/pcjs/lib/x86seg.js index 35bf126db..a38d4b4b9 100644 --- a/modules/pcjs/lib/x86seg.js +++ b/modules/pcjs/lib/x86seg.js @@ -309,7 +309,6 @@ X86Seg.prototype.checkReadProt = function checkReadProt(off, cb, fSuppress) * it to an unsigned value using ">>>"; offMax was already converted at segment load time. */ if ((off >>> 0) + cb <= this.offMax) { - this.blockACC.adw[this.iACC] |= this.bitACC; return (this.base + off)|0; } return this.checkReadProtDisallowed(off, cb, fSuppress); @@ -331,7 +330,6 @@ X86Seg.prototype.checkReadProtDown = function checkReadProtDown(off, cb, fSuppre * it to an unsigned value using ">>>"; offMax was already converted at segment load time. */ if ((off >>> 0) + cb > this.offMax) { - this.blockACC.adw[this.iACC] |= this.bitACC; return (this.base + off)|0; } return this.checkReadProtDisallowed(off, cb, fSuppress); @@ -370,7 +368,6 @@ X86Seg.prototype.checkWriteProt = function checkWriteProt(off, cb, fSuppress) * it to an unsigned value using ">>>"; offMax was already converted at segment load time. */ if ((off >>> 0) + cb <= this.offMax) { - this.blockACC.adw[this.iACC] |= this.bitACC; return (this.base + off)|0; } return this.checkWriteProtDisallowed(off, cb, fSuppress); @@ -392,7 +389,6 @@ X86Seg.prototype.checkWriteProtDown = function checkWriteProtDown(off, cb, fSupp * it to an unsigned value using ">>>"; offMax was already converted at segment load time. */ if ((off >>> 0) + cb > this.offMax) { - this.blockACC.adw[this.iACC] |= this.bitACC; return (this.base + off)|0; } return this.checkWriteProtDisallowed(off, cb, fSuppress); @@ -930,13 +926,23 @@ X86Seg.prototype.updateMode = function(fLoad, fProt) this.checkRead = this.checkReadProt; this.checkWrite = this.checkWriteProt; - this.iACC = this.bitACC = 0; - if (this.acc & X86.DESC.ACC.TYPE.SEG) { + /* + * TODO: For null GDT selectors, should we rely on the descriptor being invalid, or should we assume that + * the null descriptor might contain uninitialized (or other) data? I'm assuming the latter, hence the + * following null selector test. However, if we're not going to consult the descriptor, is there anything + * else we should (or should not) be doing for null GDT selectors? + */ + if (!(this.sel & ~X86.SEL.RPL)) { + this.checkRead = this.checkReadProtDisallowed; + this.checkWrite = this.checkWriteProtDisallowed; + + } + else if (this.acc & X86.DESC.ACC.TYPE.SEG) { /* * If the READABLE bit of CODE_READABLE is not set, then disallow reads. */ if ((this.acc & X86.DESC.ACC.TYPE.CODE_READABLE) == X86.DESC.ACC.TYPE.CODE_EXECONLY) { - this.checkWrite = this.checkReadProtDisallowed; + this.checkRead = this.checkReadProtDisallowed; } /* * If the CODE bit is set, or the the WRITABLE bit is not set, then disallow writes. @@ -952,51 +958,34 @@ X86Seg.prototype.updateMode = function(fLoad, fProt) if (this.checkWrite == this.checkWriteProt) this.checkWrite = this.checkWriteProtDown; this.fExpDown = true; } + } + /* + * TODO: For non-SEG descriptors, are there other checks or functions we should establish? + */ + + /* + * Any update to the following properties must occur only on segment loads, not simply when + * we're updating segment registers as part of a mode change. + */ + if (fLoad) { /* - * Here begins the multi-step process of computing block, dword index and bit mask required - * to update the descriptor's ACCESSED bit whenever the segment is accessed. + * We must update the descriptor's ACCESSED bit whenever the segment is "accessed" (ie, + * loaded); unlike the ACCESSED and DIRTY bits in PTEs, a descriptor ACCESSED bit is only + * updated on loads, not on every memory access. * - * Step 1: Compute address of the descriptor byte containing the ACCESSED bit (offset 0x5); + * We compute address of the descriptor byte containing the ACCESSED bit (offset 0x5); * note that it's perfectly normal for addrDesc to occasionally be invalid (eg, when the CPU * is creating protected-mode-only segment registers like LDT and TSS, or when the CPU has * transitioned from real-mode to protected-mode and new selector(s) have not been loaded yet). + * + * TODO: Note I do NOT update the ACCESSED bit for null GDT selectors, because I assume the + * hardware does not update it either. In fact, I've seen code that uses the null GDT descriptor + * for other purposes, on the assumption that that descriptor is completely unused. */ - if (this.addrDesc != X86.ADDR_INVALID) { - var addrAcc = this.addrDesc + X86.DESC.ACC.TYPE.OFFSET; - /* - * Step 2: Compute the logical block number containing that byte, and record the block. - */ - this.blockACC = this.cpu.aMemBlocks[(addrAcc & this.cpu.nMemMask) >>> this.cpu.nBlockShift]; - this.cpu.assert(this.blockACC && this.blockACC.adw); - /* - * It's critical that we check fReadOnly, because ROMs often use GDTs that are also located - * in ROM, in which case the ACCESSED bit cannot be set (ie, we must ensure that blockACC is - * set to a dummy block). - */ - if (this.blockACC && !this.blockACC.fReadOnly && this.blockACC.adw) { - /* - * Step 3: Compute the index of the DWORD (adw entry) containing that byte. - */ - this.iACC = (addrAcc & this.cpu.nBlockLimit) >> 2; - /* - * Step 4: Compute the bit that must be OR'ed into that DWORD in order to set the ACCESSED bit; - * we right-shift the bit into byte 0, and then left-shift it into byte 0, 1, 2 or 3 as appropriate. - */ - this.bitACC = Memory.adjustEndian((X86.DESC.ACC.TYPE.ACCESSED >> 8) << ((addrAcc & 0x3) << 3)); - } + if ((this.sel & ~X86.SEL.RPL) && this.addrDesc != X86.ADDR_INVALID) { + var addrACC = this.addrDesc + X86.DESC.ACC.TYPE.OFFSET; + this.cpu.setByte(addrACC, this.cpu.getByte(addrACC) | (X86.DESC.ACC.TYPE.ACCESSED >> 8)); } - } - - if (!this.bitACC) { - if (!this.cpu.blockDummy) this.cpu.blockDummy = new Memory(0, 0, 4); - this.blockACC = this.cpu.blockDummy; - } - - if (fLoad) { - /* - * Any update to the following properties must occur only on segment loads, not simply when - * we're updating segment registers as part of a mode change. - */ this.cpl = this.sel & X86.SEL.RPL; this.dpl = (this.acc & X86.DESC.ACC.DPL.MASK) >> X86.DESC.ACC.DPL.SHIFT; if (this.cpu.model < X86.MODEL_80386 || !(this.ext & X86.DESC.EXT.BIG)) { diff --git a/tests/pc/80386/tests.nasm b/tests/pc/80386/tests.nasm index e93813bcd..65c9bedc3 100644 --- a/tests/pc/80386/tests.nasm +++ b/tests/pc/80386/tests.nasm @@ -64,6 +64,7 @@ CSEG_REAL equ 0xf000 CSEG_PROT16 equ 0x0008 CSEG_PROT32 equ 0x0010 DSEG_PROT16 equ 0x0018 +DSEG_PROT32 equ 0x0020 ; ; The "defDesc" macro defines a descriptor, given a name (%1), base (%2), limit (%3), type (%4), and ext (%5) @@ -96,9 +97,9 @@ DSEG_PROT16 equ 0x0018 %assign selDesc selDesc+8 %endmacro -start: cli +start: nop ; - ; Test unsigned 32-bit multiplication and division + ; Quick test of unsigned 32-bit multiplication and division ; mov eax,0x44332211 mov ebx,eax @@ -111,7 +112,7 @@ start: cli xor dx,dx mov ds,dx ; DS -> 0x0000 ; - ; Test moving a segment register to a 32-bit register + ; Quick test of moving a segment register to a 32-bit register ; mov eax,ds test eax,eax @@ -153,6 +154,7 @@ myGDT: defDesc NULL,0 ; the first descriptor in any descriptor table is always defDesc CSEG_PROT16,0x000f0000,0x0000ffff,ACC_TYPE_CODE_READABLE,EXT_NONE defDesc CSEG_PROT32,0x000f0000,0x0000ffff,ACC_TYPE_CODE_READABLE,EXT_BIG defDesc DSEG_PROT16,0x00000000,0x000fffff,ACC_TYPE_DATA_WRITABLE,EXT_NONE + defDesc DSEG_PROT32,0x00000000,0x000fffff,ACC_TYPE_DATA_WRITABLE,EXT_BIG myGDTEnd: initGDT: @@ -166,6 +168,7 @@ initGDT: setDesc CSEG_PROT16,eax,0x0000ffff,ACC_TYPE_CODE_READABLE,EXT_NONE setDesc CSEG_PROT32,eax,0x0000ffff,ACC_TYPE_CODE_READABLE,EXT_BIG setDesc DSEG_PROT16,0x0,0x000fffff,ACC_TYPE_DATA_WRITABLE,EXT_NONE + setDesc DSEG_PROT32,0x0,0x000fffff,ACC_TYPE_DATA_WRITABLE,EXT_BIG sub edi,RAM_GDT dec edi mov [RAM_GDTR],di @@ -244,10 +247,10 @@ initPages: mov es,ax xor edi,edi ; - ; Build a page directory at ES:EDI with only 1 valid PDE (the first one) + ; Build a page directory at ES:EDI with only 1 valid PDE (the first one), + ; because we're not going to access any memory outside the first 4Mb. ; cld - cli ; make sure interrupts are still off (in case any DOS calls turned them back on) mov eax,esi add eax,0x1000 ; EAX == page frame address (of the next page) or eax,PTE_USER | PTE_READWRITE | PTE_PRESENT @@ -256,7 +259,7 @@ initPages: sub eax,eax rep stosd ; - ; Build a page table at EDI with only 256 (out of 1024) valid PTEs, which will map the first 1Mb of the + ; Build a page table at EDI with 256 (out of 1024) valid PTEs, mapping the first 1Mb of the ; first 4Mb as linear == physical. ; mov eax,PTE_USER | PTE_READWRITE | PTE_PRESENT @@ -268,7 +271,9 @@ initPT: stosd sub eax,eax rep stosd -goProt: o32 lgdt [cs:addrGDT] +goProt: + cli ; make sure interrupts are off now, since we've not initialized the IDT yet + o32 lgdt [cs:addrGDT] mov cr3,esi mov eax,cr0 %if PAGING @@ -286,6 +291,45 @@ toProt32: mov ds,ax mov es,ax ; + ; Of the 64Kb of scratch memory we allocated, the first 8Kb (0x2000) is being used for a + ; page directory and a single page table, so we have at least another 52Kb, at ESI+0x2000, + ; to play with (I'm rounding down by 4Kb to be safe). + ; + ; Let's use the top of that memory, ESI+0xd000, as the top of our stack. Note, however, that + ; that guarantees ESI will be be greater than 0xffff, so as long as SS contains a 16-bit data + ; segment, pushes will NOT be occurring where you expect. + ; + add esi,0x2000 ; ESI -> bottom of scratch memory + mov ss,ax + lea esp,[esi+0xe000] ; set ESP to bottom of scratch + 52K + lea ebp,[esp-4] + and ebp,0xffff ; EBP now mirrors SP instead of ESP + mov edx,[ebp] + mov eax,0x11223344 + push eax + cmp [ebp],eax ; did the push use SP instead of ESP? + jne near error ; no, error + pop eax + push ax + cmp [ebp+2],ax + jne near error + pop ax + mov [ebp+2],edx ; restore dword trashed by the above pushes + mov ax,DSEG_PROT32 + mov ss,ax + lea esp,[esi+0xe000] ; SS:ESP should now be a valid 32-bit pointer + lea ebp,[esp-4] + mov edx,[ebp] + mov eax,0x11223344 + push eax + cmp [ebp],eax ; did the push use ESP instead of SP? + jne near error ; no, error + pop eax + push ax + cmp [ebp+2],ax + jne near error + pop ax + ; ; Test moving a segment register to a 32-bit memory location ; test1: mov edx,[0x0000] ; save the DWORD at 0x0000:0x0000 in EDX @@ -294,11 +338,11 @@ test1: mov edx,[0x0000] ; save the DWORD at 0x0000:0x0000 in EDX mov [0x0000],ds mov ax,ds cmp eax,[0x0000] - jne error + jne near error mov eax,ds xor eax,0xffff0000 cmp eax,[0x0000] - jne error + jne near error mov [0x0000],edx ; restore the DWORD at 0x0000:0x0000 from EDX jmp test2 ;