diff --git a/doc/CHANGELOG.md b/doc/CHANGELOG.md index bace4ece..e8c400d7 100644 --- a/doc/CHANGELOG.md +++ b/doc/CHANGELOG.md @@ -95,6 +95,8 @@ The full set of tests is only run for tagged releases. - rtl/w11a - pdp11_mmu: BUGFIX: correct trap and PDR A logic, see [ECO-033](ECO-033-MMU_AFC-1_PDR-A.md) + - pdp11_sequencer: BUGFIX: use I space for all mode=1,2,3 if reg=pc, see + [ECO-034](ECO-034-MMU_d-space-pc.md) - src/librwxxtpp - RtclRw11Cpu.cpp: quit before mem write if asm-11 error seen - tools/asm-11/lib diff --git a/doc/ECO-034-MMU_d-space-pc.md b/doc/ECO-034-MMU_d-space-pc.md new file mode 100644 index 00000000..6f3047e8 --- /dev/null +++ b/doc/ECO-034-MMU_d-space-pc.md @@ -0,0 +1,53 @@ +# ECO-033: D space used instead of I space for some PC specifiers (2022-09-08) + +### Scope +- was in w11a since 2009 +- affects: all w11a systems + +### Symptom summary +Test 072 of `ekbee1` fails with +``` + D-SPACE ENABLE CIRCUITRY HAS FAILED + ERROR AUTOI/D VIRTUAL + REGISTR REGISTR ADDRESS TESTNO PC AT ABORT + 100027 000000 060410 000072 060412 + 100027 000027 060416 000072 060422 +``` +The test does +``` + 060410: tst (pc) + 060416: cmp #240,(pc) +``` +and expects that these accesses are done to I space. +They are done to D space instead. + +### Analysis +The w11 used D space only for `(pc)+` and `@(pc)+` specifiers. +The w11a has 4 specifier flows +- srcr: for source read +- dstr: for destination read (in `CMP`, `BIT`, `TST`, ...) +- dstw: for destination write (also read-modify-write) +- dsta: for destination address (in `JSR`, `JMP`) +A code review showed what was OK and what needed a fix +``` +s_srcr_def OK mov (pc),r0 +s_srcr_inc OK mov #123,r0 +s_dstr_def FIXED cmp,#123,(pc) +s_dstr_inc OK cmp r0,#123 +s_dstw_def FIXED mov r0,(pc) modifies I space code ! +s_dstw_inc FIXED for mode=2 mov r0,#133 modifies I space code ! +s_dstw_inc OK for mode=3 mov r0,@#lbl +s_dsta_inc OK +``` + +Note: the 11/70 uses D space for modes 1-5 in the source specifier flow, +and for mode 1-3 in the destination specifier flow. + +### Fixes +Add `pispace` modifiers for the `do_memread*` and `do_memwrite*` calls in +pdp11_sequencer.vhd. + +### Hindsight +Took 13 years to fix. The fixed address modes are of very little practical +value and are not used in normal code. And certainly used not when running +with kernel D mode enabled. diff --git a/doc/README_known_issues.md b/doc/README_known_issues.md index f5fe5bdf..2c536088 100644 --- a/doc/README_known_issues.md +++ b/doc/README_known_issues.md @@ -3,29 +3,6 @@ Notes - resolved issues are summarized in [resolved issues](README_resolved_issues.md) - the case id indicates the release when the issue was first recognized. -### V0.791-3 {[issue #35](https://github.com/wfjm/w11/issues/35)} -- MMU: D space used instead of I space for PC deferred specifiers - -Test 072 of `ekbee1` fails with -``` - D-SPACE ENABLE CIRCUITRY HAS FAILED - ERROR AUTOI/D VIRTUAL - REGISTR REGISTR ADDRESS TESTNO PC AT ABORT - 100027 000000 060410 000072 060412 - 100027 000027 060416 000072 060422 -``` -The test does -``` - 060410: tst (pc) - 060416: cmp #240,(pc) -``` -and expects that these accesses are done to I space. -They are done to D space instead. - -The w11 uses D space only for `(pc)+` and `@(pc)+` specifiers. -Clearly a bug. -Wasn't detected so far because this access mode has no practical value -and this therefore not used in normal software. - ### V0.50-2 {[issue #28](https://github.com/wfjm/w11/issues/28)} -- RK11: write protect action too slow Some simple RK11 drivers, especially in test codes, don't poll for completion diff --git a/rtl/w11a/pdp11_sequencer.vhd b/rtl/w11a/pdp11_sequencer.vhd index 06e1b3b6..082fe4c7 100644 --- a/rtl/w11a/pdp11_sequencer.vhd +++ b/rtl/w11a/pdp11_sequencer.vhd @@ -1,4 +1,4 @@ --- $Id: pdp11_sequencer.vhd 1279 2022-08-14 08:02:21Z mueller $ +-- $Id: pdp11_sequencer.vhd 1297 2022-09-10 13:04:37Z mueller $ -- SPDX-License-Identifier: GPL-3.0-or-later -- Copyright 2006-2022 by Walter F.J. Mueller -- @@ -13,6 +13,7 @@ -- -- Revision History: -- Date Rev Version Comment +-- 2022-09-08 1296 1.6.14 BUGFIX: use I space for all mode=1,2,3 if reg=pc -- 2022-08-13 1279 1.6.13 ssr->mmr rename -- 2019-08-17 1203 1.6.12 fix for ghdl V0.36 -Whide warnings -- 2018-10-07 1054 1.6.11 drop ITIMER, use DM_STAT_SE.itimer @@ -395,9 +396,9 @@ begin pwstate : in state_type; pbytop : in slbit := '0'; pmacc : in slbit := '0'; - pis_pci : in slbit := '0') is + pispace : in slbit := '0') is begin - pnvmcntl.dspace := not pis_pci; -- ispace if pc immediate modes + pnvmcntl.dspace := not pispace; -- bytop := R_IDSTAT.is_bytop and not is_addr; pnvmcntl.bytop := pbytop; pnvmcntl.macc := pmacc; @@ -433,9 +434,10 @@ begin procedure do_memwrite(pnstate : inout state_type; pnvmcntl : inout vm_cntl_type; pwstate : in state_type; - pmacc : in slbit :='0') is + pmacc : in slbit :='0'; + pispace : in slbit := '0') is begin - pnvmcntl.dspace := '1'; + pnvmcntl.dspace := not pispace; pnvmcntl.bytop := R_IDSTAT.is_bytop; pnvmcntl.wacc := '1'; pnvmcntl.macc := pmacc; @@ -1022,7 +1024,7 @@ begin ndpcntl.vmaddr_sel := c_dpath_vmaddr_dsrc; -- VA = DSRC do_memread_d(nstate, nvmcntl, s_srcr_def_w, pbytop=>R_IDSTAT.is_bytop, - pis_pci=>R_IDSTAT.is_srcpcmode1); + pispace=>R_IDSTAT.is_srcpcmode1); when s_srcr_def_w => -- ----------------------------------- nstate := s_srcr_def_w; @@ -1054,7 +1056,7 @@ begin ndpcntl.vmaddr_sel := c_dpath_vmaddr_dsrc; -- VA = DSRC bytop := R_IDSTAT.is_bytop and not SRCDEF; do_memread_d(nstate, nvmcntl, s_srcr_inc_w, - pbytop=>bytop, pis_pci=>R_IDSTAT.is_srcpc); + pbytop=>bytop, pispace=>R_IDSTAT.is_srcpc); when s_srcr_inc_w => -- ----------------------------------- nstate := s_srcr_inc_w; @@ -1186,7 +1188,8 @@ begin when s_dstr_def => -- ----------------------------------- ndpcntl.vmaddr_sel := c_dpath_vmaddr_ddst; -- VA = DDST do_memread_d(nstate, nvmcntl, s_dstr_def_w, - pbytop=>R_IDSTAT.is_bytop, pmacc=>R_IDSTAT.is_rmwop); + pbytop=>R_IDSTAT.is_bytop, pmacc=>R_IDSTAT.is_rmwop, + pispace=>R_IDSTAT.is_dstpc); when s_dstr_def_w => -- ----------------------------------- nstate := s_dstr_def_w; @@ -1211,7 +1214,7 @@ begin macc := R_IDSTAT.is_rmwop and not DSTDEF; bytop := R_IDSTAT.is_bytop and not DSTDEF; do_memread_d(nstate, nvmcntl, s_dstr_inc_w, - pbytop=>bytop, pmacc=>macc, pis_pci=>R_IDSTAT.is_dstpc); + pbytop=>bytop, pmacc=>macc, pispace=>R_IDSTAT.is_dstpc); when s_dstr_inc_w => -- ----------------------------------- nstate := s_dstr_inc_w; @@ -1335,7 +1338,7 @@ begin ndpcntl.dres_sel := R_IDSTAT.res_sel; -- DRES = choice of idec ndpcntl.vmaddr_sel := c_dpath_vmaddr_ddst; -- VA = DDST nvmcntl.kstack := is_dstkstack1246; - do_memwrite(nstate, nvmcntl, s_dstw_def_w); + do_memwrite(nstate, nvmcntl, s_dstw_def_w, pispace=>R_IDSTAT.is_dstpc); when s_dstw_def_w => -- ----------------------------------- nstate := s_dstw_def_w; @@ -1354,7 +1357,7 @@ begin if DSTDEF = '0' then ndpcntl.dres_sel := R_IDSTAT.res_sel; -- DRES = choice of idec nvmcntl.kstack := is_dstkstack1246; - do_memwrite(nstate, nvmcntl, s_dstw_inc_w); + do_memwrite(nstate, nvmcntl, s_dstw_inc_w, pispace=>R_IDSTAT.is_dstpc); nstatus.do_gprwe := '1'; else ndpcntl.dres_sel := c_dpath_res_ounit; -- DRES = OUNIT @@ -1363,7 +1366,7 @@ begin nmmumoni.regmod := '1'; nmmumoni.isdec := '0'; do_memread_d(nstate, nvmcntl, s_dstw_incdef_w, - pis_pci=>R_IDSTAT.is_dstpc); + pispace=>R_IDSTAT.is_dstpc); end if; when s_dstw_inc_w => -- ----------------------------------- @@ -1495,7 +1498,7 @@ begin do_fork_opa(nstate, R_IDSTAT); else do_memread_d(nstate, nvmcntl, s_dsta_incdef_w, - pis_pci=>R_IDSTAT.is_dstpc); + pispace=>R_IDSTAT.is_dstpc); end if; when s_dsta_incdef_w => -- ----------------------------------- diff --git a/tools/tcode/cpu_mmu.mac b/tools/tcode/cpu_mmu.mac index 643249e7..f90f5bfe 100644 --- a/tools/tcode/cpu_mmu.mac +++ b/tools/tcode/cpu_mmu.mac @@ -1,10 +1,10 @@ -; $Id: cpu_mmu.mac 1295 2022-09-07 16:28:55Z mueller $ +; $Id: cpu_mmu.mac 1297 2022-09-10 13:04:37Z mueller $ ; SPDX-License-Identifier: GPL-3.0-or-later ; Copyright 2022- by Walter F.J. Mueller ; ; Revision History: ; Date Rev Version Comment -; 2022-09-06 1294 1.0 Initial version +; 2022-09-10 1297 1.0 Initial version ; 2022-07-24 1262 0.1 First draft ; ; Test CPU MMU: all aspects of the MMU @@ -13,6 +13,7 @@ ; Section C: mmr1+mmr0 register, aborts ; Section D: mmr2+mmr1+mmr0 register, abort recovery ; Section E: traps and pdr aia and aiw bits +; Section F: miscellaneous ; .include |lib/tcode_std_base.mac| .include |lib/defs_mmu.mac| @@ -32,8 +33,11 @@ sipar7 = sipar+16 kipdr0 = kipdr+ 0 + kdpdr0 = kdpdr+ 0 kipdr6 = kipdr+14 kipar6 = kipar+14 + kdpdr6 = kdpdr+14 + kdpar6 = kdpar+14 kipdr7 = kipdr+16 kipar7 = kipar+16 @@ -1588,10 +1592,80 @@ te0102: mov #vhmmut,v..mmu ; setup MMU trap handler clr v..mmu+2 9999$: iot ; end of test E1.2 ; +; Section F: miscellaneous =================================================== +; +; Test F1: ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +; +; Test F1.1 -- test D-to-I mapping for (PC) address modes ++++++++++++ +; In case of immediate (pc)+ and absolute @(pc)+ addressing the first read +; comes from I space. Same holds for (pc) specifiers. The remaining two modes +; -(pc) and @-(pc) have no practical use and cant even be tested. +; The test runs with D space disabled and the D page 0 set non-resident. +; If any access to D space will cause an MMU abort. Because kernel D page 0 is +; non-resident, the vector fetch will fail and the CPU halts with an F:vecfet. +; +; Summary +; (pc)+ src: mov #nnn,r0 +; (pc)+ dstr: tst #nnn +; (pc)+ dstr: cmp r0,#nnn +; (pc)+ dstw: mov r0,#nnn +; @(pc)+ src: mov @#val,r0 +; @(pc)+ dstr: tst @#val +; @(pc)+ dstr: cmp r0,@#val +; @(pc)+ dstw: mov r0,@#val +; (pc) src: mov (pc),r0 +; (pc) dstr: tst (pc) +; (pc) dstr: cmp r0,(pc) +; (pc) dstw: mov r0,(pc) +; +tf0101: mov #m3.dkm,mmr3 ; enable D space for kernel + clr kdpdr0 ; ensure D space non-resident + mov kipdr6,kdpdr6 ; set up page 6 D space 1-to-0 + mov kipar6,kdpar6 +; + mov #234,100$+2 ; restore target + mov #345,p6base ; restore target + mov #ccc,300$ ; restore target + clr r2 + mov #m0.ena,mmr0 ; enable mmu ;! MMU 18 +; +; (pc)+ + mov #123,r0 ; src immediate + tst #123 ; dstr immediate + cmp r0,#123 ; dstr immediate +100$: mov r0,#234 ; dstw immediate (overwrites I space!) +; +; @(pc)+ + mov @#p6base,r1 ; src absolute + tst @#p6base ; dstr absolute + cmp r1,@#p6base ; dstr absolute + mov r0,@#p6base ; dstw absolute (write #123 to D space) +; +; (pc) + mov (pc),r2 ; src (read next instruction, a ccc) +200$: ccc + tst (pc) ; dstr (read next instruction) + cmp r2,(pc) ; dstr (read next instruction) + mov r2,(pc) ; dstw (overwrites I space!) +300$: scc +; + reset ; mmu off ;! MMU off + hcmpeq 100$+2,#123 ; I space overwritten ? + hcmpeq p6base,#123 ; D space overwritten ? + hcmpeq r2,200$ ; read from I space ? + hcmpeq 300$,#ccc +; + clr kdpdr6 ; reset kdpdr6 + clr kdpar6 ; reset kdpar6 + mov #v..mmu+2,v..mmu ; restore mmu catcher + clr v..mmu+2 +; +9999$: iot ; end of test F1.1 +; ; END OF ALL TESTS - loop closure ============================================ ; mov tstno,r0 ; hack, for easy monitoring ... - hcmpeq tstno,#15. ; all tests done ? + hcmpeq tstno,#16. ; all tests done ? ; jmp loop ;