Retro/pcjs
Watch
1
0
Fork
You've already forked pcjs
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix an inconsistency in Debugger address parsing (problems mixing segment and physical addresses)

Browse source
This commit is contained in:
Jeff Parsons 2014-10-02 10:48:41 -07:00 • committed by jeffpar
commit a474ddfa74
3 changed files with 52 additions and 43 deletions
Download patch file Download diff file Expand all files Collapse all files

65
my_modules/pcjs-client/lib/chipset.js
View file

@ -72,7 +72,7 @@ if (typeof module !== 'undefined') {
* SW2[1-4] (bits 3-0) "NNNNxxxx": number of 32Kb blocks of I/O expansion RAM present * SW2[1-4] (bits 3-0) "NNNNxxxx": number of 32Kb blocks of I/O expansion RAM present
* *
* TODO: There are cryptic references to SW2[5] in the original (5150) TechRef, and apparently the 8255A PPI can * TODO: There are cryptic references to SW2[5] in the original (5150) TechRef, and apparently the 8255A PPI can
* be programmed to return it (which we support), but its purpose is unclear to me (see PPI_B_ENABLE_SW2). * be programmed to return it (which we support), but its purpose is unclear to me (see PPI_B.ENABLE_SW2).
* *
* For example, sw1="01110011" indicates that all SW1 DIP switches are ON, except for SW1[1], SW1[5] and SW1[6], * For example, sw1="01110011" indicates that all SW1 DIP switches are ON, except for SW1[1], SW1[5] and SW1[6],
* which are OFF. Internally, the order of these bits must reversed (to 11001110) and then inverted (to 00110001) * which are OFF. Internally, the order of these bits must reversed (to 11001110) and then inverted (to 00110001)
@ -480,7 +480,7 @@ ChipSet.TIMER_TICKS_PER_SEC = 1193181;
* with the usual 0x99. * with the usual 0x99.
*/ */
ChipSet.PPI_A = {}; // this.bPPIA ChipSet.PPI_A = {}; // this.bPPIA
ChipSet.PPI_A.PORT = 0x60; // INPUT: keyboard scan code (PPI_B_CLEAR_KBD must be clear) ChipSet.PPI_A.PORT = 0x60; // INPUT: keyboard scan code (PPI_B.CLEAR_KBD must be clear)
ChipSet.PPI_B = {}; // this.bPPIB ChipSet.PPI_B = {}; // this.bPPIB
ChipSet.PPI_B.PORT = 0x61; // OUTPUT (although it has to be treated as INPUT, too: the keyboard interrupt handler reads it, OR's PPI_B.CLEAR_KBD, writes it, and then rewrites the original read value) ChipSet.PPI_B.PORT = 0x61; // OUTPUT (although it has to be treated as INPUT, too: the keyboard interrupt handler reads it, OR's PPI_B.CLEAR_KBD, writes it, and then rewrites the original read value)
@ -496,7 +496,7 @@ ChipSet.PPI_B.CLEAR_KBD = 0x80; // ALL: clear to enable keyboard scan co
ChipSet.PPI_C = {}; // this.bPPIC ChipSet.PPI_C = {}; // this.bPPIC
ChipSet.PPI_C.PORT = 0x62; // INPUT (see below) ChipSet.PPI_C.PORT = 0x62; // INPUT (see below)
ChipSet.PPI_C.SW = 0x0F; // MODEL_5150: SW2[1-4] or SW2[5], depending on whether PPI_B_ENABLE_SW2 is set or clear; MODEL_5160: SW1[1-4] or SW1[5-8], depending on whether PPI_B_ENABLE_SW_HI is clear or set ChipSet.PPI_C.SW = 0x0F; // MODEL_5150: SW2[1-4] or SW2[5], depending on whether PPI_B.ENABLE_SW2 is set or clear; MODEL_5160: SW1[1-4] or SW1[5-8], depending on whether PPI_B.ENABLE_SW_HI is clear or set
ChipSet.PPI_C.CASS_DATA_IN = 0x10; ChipSet.PPI_C.CASS_DATA_IN = 0x10;
ChipSet.PPI_C.TIMER2_OUT = 0x20; ChipSet.PPI_C.TIMER2_OUT = 0x20;
ChipSet.PPI_C.IO_CHANNEL_CHK = 0x40; // used by NMI handler to detect I/O channel errors ChipSet.PPI_C.IO_CHANNEL_CHK = 0x40; // used by NMI handler to detect I/O channel errors
@ -514,7 +514,7 @@ ChipSet.PPI_CTRL.A_MODE = 0x60;
/* /*
* On the MODEL_5150, the following PPI_SW bits are exposed through PPI_A. * On the MODEL_5150, the following PPI_SW bits are exposed through PPI_A.
* *
* On the MODEL_5160, either the low or high 4 bits are exposed through PPI_C_SW, if PPI_B_ENABLE_SW_HI is clear or set. * On the MODEL_5160, either the low or high 4 bits are exposed through PPI_C_SW, if PPI_B.ENABLE_SW_HI is clear or set.
*/ */
ChipSet.PPI_SW = {}; ChipSet.PPI_SW = {};
ChipSet.PPI_SW.FDRIVE = {}; ChipSet.PPI_SW.FDRIVE = {};
@ -3288,29 +3288,6 @@ ChipSet.prototype.updateAllTimers = function(fCycleReset)
if (this.model >= ChipSet.MODEL_5170) this.updateRTCDate(); if (this.model >= ChipSet.MODEL_5170) this.updateRTCDate();
}; };
/**
* updateSpeaker(bOut)
*
* @this {ChipSet}
* @param {number} bOut
*/
ChipSet.prototype.updateSpeaker = function(bOut)
{
var fNewSpeaker = !!(bOut & ChipSet.PPI_B.SPK_TIMER2);
var fOldSpeaker = !!(this.bPPIB & ChipSet.PPI_B.SPK_TIMER2);
this.bPPIB = bOut;
if (fNewSpeaker != fOldSpeaker) {
/*
* Originally, this code didn't catch the "ERROR_BEEP" case @F000:EC34, which first turns both PPI_B_CLK_TIMER2 (0x01)
* and PPI_B_SPK_TIMER2 (0x02) off, then turns on only PPI_B_SPK_TIMER2 (0x02), then restores the original port value.
*
* So, when the ROM BIOS keyboard buffer got full, we didn't issue a BEEP alert. I've fixed that by limiting the test
* to PPI_B_SPK_TIMER2 and ignoring PPI_B_CLK_TIMER2.
*/
this.setSpeaker(fNewSpeaker);
}
};
/** /**
* inPPIA(port, addrFrom) * inPPIA(port, addrFrom)
* *
@ -3374,10 +3351,38 @@ ChipSet.prototype.inPPIB = function(port, addrFrom)
ChipSet.prototype.outPPIB = function(port, bOut, addrFrom) ChipSet.prototype.outPPIB = function(port, bOut, addrFrom)
{ {
this.messagePort(port, bOut, addrFrom, "PPI_B", ChipSet.MESSAGE_CHIPSET); this.messagePort(port, bOut, addrFrom, "PPI_B", ChipSet.MESSAGE_CHIPSET);
this.updateSpeaker(bOut); this.updatePPIB(bOut);
if (this.kbd) this.kbd.setEnable((bOut & ChipSet.PPI_B.CLEAR_KBD)? false : true, (bOut & ChipSet.PPI_B.CLK_KBD)? true : false); if (this.kbd) this.kbd.setEnable((bOut & ChipSet.PPI_B.CLEAR_KBD)? false : true, (bOut & ChipSet.PPI_B.CLK_KBD)? true : false);
}; };
/**
* updatePPIB(bOut)
*
* On MODEL_5170 and up, this updates the "simulated" PPI_B. The only common (and well-documented) PPI_B bits
* across all models are PPI_B.CLK_TIMER2 and PPI_B.SPK_TIMER2, so its possible that this function may need to
* limit its updates to just those bits, and move any model-specific requirements back into the appropriate I/O
* handlers (PPIB or 8042RWReg). We'll see.
*
* @this {ChipSet}
* @param {number} bOut
*/
ChipSet.prototype.updatePPIB = function(bOut)
{
var fNewSpeaker = !!(bOut & ChipSet.PPI_B.SPK_TIMER2);
var fOldSpeaker = !!(this.bPPIB & ChipSet.PPI_B.SPK_TIMER2);
this.bPPIB = bOut;
if (fNewSpeaker != fOldSpeaker) {
/*
* Originally, this code didn't catch the "ERROR_BEEP" case @F000:EC34, which first turns both PPI_B.CLK_TIMER2 (0x01)
* and PPI_B.SPK_TIMER2 (0x02) off, then turns on only PPI_B.SPK_TIMER2 (0x02), then restores the original port value.
*
* So, when the ROM BIOS keyboard buffer got full, we didn't issue a BEEP alert. I've fixed that by limiting the test
* to PPI_B.SPK_TIMER2 and ignoring PPI_B.CLK_TIMER2.
*/
this.setSpeaker(fNewSpeaker);
}
};
/** /**
* inPPIC(port, addrFrom) * inPPIC(port, addrFrom)
* *
@ -3523,7 +3528,7 @@ ChipSet.prototype.out8042InBuffData = function(port, bOut, addrFrom)
* Here's some relevant MODEL_5170 ROM BIOS code, "XMIT_8042" (missing from the original MODEL_5170 ROM BIOS listing), * Here's some relevant MODEL_5170 ROM BIOS code, "XMIT_8042" (missing from the original MODEL_5170 ROM BIOS listing),
* which sends a command code in AL to the Keyboard and waits for a response, returning it in AL. Note that * which sends a command code in AL to the Keyboard and waits for a response, returning it in AL. Note that
* the only "success" exit path from this function involves LOOPing 64K times before finally reading the Keyboard's * the only "success" exit path from this function involves LOOPing 64K times before finally reading the Keyboard's
* response; either the hardware and/or this code seems a bit brain-damaged if that's REALLY what you had to do to get * response; either the hardware and/or this code seems a bit brain-damaged if that's REALLY what you had to do to ensure
* a valid response.... * a valid response....
* *
* F000:1B25 86E0 XCHG AH,AL * F000:1B25 86E0 XCHG AH,AL
@ -3624,7 +3629,7 @@ ChipSet.prototype.in8042RWReg = function(port, addrFrom)
ChipSet.prototype.out8042RWReg = function(port, bOut, addrFrom) ChipSet.prototype.out8042RWReg = function(port, bOut, addrFrom)
{ {
this.messagePort(port, bOut, addrFrom, "8042_RWREG", ChipSet.MESSAGE_CHIPSET); this.messagePort(port, bOut, addrFrom, "8042_RWREG", ChipSet.MESSAGE_CHIPSET);
this.updateSpeaker(bOut); this.updatePPIB(bOut);
}; };
/** /**

26
my_modules/pcjs-client/lib/debugger.js
View file

@ -87,10 +87,10 @@ function Debugger(parmsDbg)
* update aAddrNextData and aAddrNextCode, respectively, when they're done. * update aAddrNextData and aAddrNextCode, respectively, when they're done.
* *
* The format of all aAddr variables is [off, seg, addr], where seg:off is the segmented * The format of all aAddr variables is [off, seg, addr], where seg:off is the segmented
* address and addr is the corresponding physical address. For some segmented addresses * address and addr is the corresponding physical address (if known). For certain segmented
* (eg, breakpoint addresses), we pre-compute the physical address and save that in aAddr[2], * addresses (eg, breakpoint addresses), we pre-compute the physical address and save that
* so that the breakpoint will still operate as intended even if the mode changes later * in aAddr[2], so that the breakpoint will still operate as intended even if the mode changes
* (eg, from real-mode to protected-mode). * later (eg, from real-mode to protected-mode).
* *
* Finally, for TEMPORARY breakpoint addresses, we set aAddr[3] to true, so that they can be * Finally, for TEMPORARY breakpoint addresses, we set aAddr[3] to true, so that they can be
* automatically cleared when they're hit. * automatically cleared when they're hit.
@ -2112,7 +2112,9 @@ if (DEBUGGER) {
} }
if (aAddr[1] != null) { if (aAddr[1] != null) {
aAddr[0] += inc; aAddr[0] += inc;
// TODO: Shouldn't we be using the segment (aAddr[1]) limit instead of 0xffff? /*
* TODO: Shouldn't we be using the segment (aAddr[1]) limit instead of 0xffff?
*/
if (aAddr[0] != (aAddr[0] & 0xffff)) { if (aAddr[0] != (aAddr[0] & 0xffff)) {
aAddr[0] = aAddr[0] & 0xffff; aAddr[0] = aAddr[0] & 0xffff;
aAddr[2] = null; aAddr[2] = null;
@ -2318,6 +2320,7 @@ if (DEBUGGER) {
* or history data (see checkInstruction), since we might not actually execute the current instruction. * or history data (see checkInstruction), since we might not actually execute the current instruction.
*/ */
var fBreak = false; var fBreak = false;
/* /*
* Map addresses in the top 64Kb (at the top of the 16Mb range) to the top of the 1Mb range. * Map addresses in the top 64Kb (at the top of the 16Mb range) to the top of the 1Mb range.
* *
@ -2326,6 +2329,7 @@ if (DEBUGGER) {
* in the top 16Mb, yet after the first inter-segment JMP, you will be running in the first 1Mb. * in the top 16Mb, yet after the first inter-segment JMP, you will be running in the first 1Mb.
*/ */
if ((addr & 0xFF0000) == 0xFF0000) addr &= 0x0FFFFF; if ((addr & 0xFF0000) == 0xFF0000) addr &= 0x0FFFFF;
for (var i = 1; i < aBreak.length; i++) { for (var i = 1; i < aBreak.length; i++) {
var aAddrBreak = aBreak[i]; var aAddrBreak = aBreak[i];
if (addr == this.getAddr(aAddrBreak)) { if (addr == this.getAddr(aAddrBreak)) {
@ -2731,9 +2735,8 @@ if (DEBUGGER) {
* addresses. If/when we add support for processors with page tables, we will likely adopt the same * addresses. If/when we add support for processors with page tables, we will likely adopt the same
* convention for linear addresses and provide a different syntax (eg, "%%") physical memory references. * convention for linear addresses and provide a different syntax (eg, "%%") physical memory references.
* *
* Address evaluation and validation (eg, range checks) are no longer performed at this stage, which * Address evaluation and validation (eg, range checks) are no longer performed at this stage. That's
* is why aAddr variables no longer contain an addr element; computing addr is now performed as-needed * done later, by getAddr(), which returns a negative result (-1) for invalid segments, out-of-range offsets,
* by getAddr(), which will return a negative result (-1) for invalid segments, out-of-range offsets,
* etc. The Debugger's low-level get/set memory functions verify all getAddr() results, but even if an * etc. The Debugger's low-level get/set memory functions verify all getAddr() results, but even if an
* invalid address is passed through to the Bus memory interfaces, the address will simply be masked with * invalid address is passed through to the Bus memory interfaces, the address will simply be masked with
* Bus.addrLimit; in the case of -1, that will generally refer to the last byte of physical memory. * Bus.addrLimit; in the case of -1, that will generally refer to the last byte of physical memory.
@ -2762,7 +2765,7 @@ if (DEBUGGER) {
var iColon = sAddr.indexOf(":"); var iColon = sAddr.indexOf(":");
if (iColon < 0) { if (iColon < 0) {
if (addr == null) { if (seg != null) {
off = this.parseValue(sAddr); off = this.parseValue(sAddr);
} else { } else {
addr = this.parseValue(sAddr); addr = this.parseValue(sAddr);
@ -2771,6 +2774,7 @@ if (DEBUGGER) {
else { else {
seg = this.parseValue(sAddr.substring(0, iColon)); seg = this.parseValue(sAddr.substring(0, iColon));
off = this.parseValue(sAddr.substring(iColon + 1)); off = this.parseValue(sAddr.substring(iColon + 1));
addr = null;
} }
} }
return [off, seg, addr]; return [off, seg, addr];
@ -4020,8 +4024,8 @@ if (DEBUGGER) {
this.aAddrNextCode = this.newAddr(this.cpu.regIP, this.cpu.segCS.sel); this.aAddrNextCode = this.newAddr(this.cpu.regIP, this.cpu.segCS.sel);
break; break;
/* /*
* I used to alias "PC" to "IP", until I discovered that early (perhaps even ALL?) versions of DEBUG * I used to alias "PC" to "IP", until I discovered that early (perhaps ALL) versions of
* treat "PC" as an alias for the 16-bit flags register. TODO: Add support for "PC" that matches DEBUG. * DEBUG.COM treat "PC" as an alias for the 16-bit flags register. TODO: Add support for "PC".
*/ */
case "IP": case "IP":
fIns = true; fIns = true;

4
my_modules/pcjs-client/lib/keyboard.js
View file

@ -589,8 +589,8 @@ Keyboard.prototype.resetDevice = function()
/** /**
* setEnable(fEnable, fClock) * setEnable(fEnable, fClock)
* *
* This is the ChipSet's primary interface for controlling "Model F" keyboards (ie, those used * This is the ChipSet's primary interface for controlling "Model F" keyboards (ie, those used with
* with MODEL_5150 and MODEL_5160 machines) * MODEL_5150 and MODEL_5160 machines). This function is called from the ChipSet's PPI_B output handler.
* *
* @this {Keyboard} * @this {Keyboard}
* @param {boolean} fEnable is true if the keyboard interface should be enabled * @param {boolean} fEnable is true if the keyboard interface should be enabled