diff --git a/doc/CHANGELOG.md b/doc/CHANGELOG.md index a98fe91a..225a6e78 100644 --- a/doc/CHANGELOG.md +++ b/doc/CHANGELOG.md @@ -62,17 +62,22 @@ The full set of tests is only run for tagged releases. - rename vfetch -> vstart ### Bug Fixes - rtl/w11a + - pdp11_psr: + - BUGFIX: inhibit CCWE when PSW being written, see + [ECO-039](ECO-039-cc_and_aborts.md) - pdp11_sequencer: - - BUGFIX: get correct PS after vector push abort, see - [ECO-036](ECO-036-vpush_abort_ps.md) - BUGFIX: use is_kstackdst1246 also in dstr flow, see - [ECO-035](ECO-035-stklim-tbit-fixes.md) + [ECO-035](ECO-035-stklim-tbit-fixes.md) - BUGFIX: correct ysv flow implementation, see - [ECO-035](ECO-035-stklim-tbit-fixes.md) + [ECO-035](ECO-035-stklim-tbit-fixes.md) - BUGFIX: correct mmu trap handing in s_idecode, see - [ECO-035](ECO-035-stklim-tbit-fixes.md) + [ECO-035](ECO-035-stklim-tbit-fixes.md) - BUGFIX: correct mmu trap vs interrupt priority, see - [ECO-035](ECO-035-stklim-tbit-fixes.md) + [ECO-035](ECO-035-stklim-tbit-fixes.md) + - BUGFIX: get correct PS after vector push abort, see + [ECO-036](ECO-036-vpush_abort_ps.md) + - BUGFIX: cc state unchanged after abort, see + [ECO-039](ECO-039-cc_and_aborts.md) - pdp11_vmbox: - BUGFIX: correct red/yellow zone boundary, see [ECO-035](ECO-035-stklim-tbit-fixes.md) diff --git a/doc/ECO-039-cc_and_aborts.md b/doc/ECO-039-cc_and_aborts.md new file mode 100644 index 00000000..781105c8 --- /dev/null +++ b/doc/ECO-039-cc_and_aborts.md @@ -0,0 +1,61 @@ +# ECO-039: fix cc after abort behavior (2022-12-31) + +### Scope +- was in w11a since 2008 +- affects: all w11a systems + +### Background +The PDP-11 architecture requires that +- for direct writes to the `PSW` the cc state of the write must prevail +- the cc state must remain unchanged after an instruction abort + +The first requirement implies that the usual updating of cc's after an +instruction must be suppressed when the `PSW` is written (e.g. with `MOV`) +or modified (e.g. with `BIS`/`BIC`). The second requirement is a natural +consequence of the needs of a recovery from an MMU abort. When the aborted +instruction is re-executed, it must be done in the original environment, +thus with the original condition code status. + +### Symptom summary +The current w11 implementation updates the condition codes in some cases +_before_ the final write, and thus before the last possibility of an +instruction abort. An address error abort on the last write would therefore +leave the CPU in a state with modified condition codes. + +This was initially detected in a code review of the `dstw` flow used by +`MOV(B)`, `CLR(B)` and `SXT` instructions. The condition code update was +done in the entry states `s_dstw_def`, `s_dstw_inc`, `s_dstw_dec`, and +`s_dstw_ind`. This ensured that in direct writes to the `PSW` the cc state +of the write prevailed, simply because this was done several cycles later. + +A closer examination revealed that the `MFP*` and `MTP*` instructions have a +similar problem, but in different flows. + +### Analysis +Simply moving the condition code updates into the `*_w` states that conclude +the last write fixes the abort problem, but will overwrite the condition codes, +which is a violation the first requirement and causes errors in xxdp code +`eqkce1` for tests 036, 043 and 053. + +### Fixes +The correct solution is to _move the condition code updates_ into the `*_w` +states that conclude the last write _and to add an overwrite protection_. +Such an overwrite protection has been added to `pdp11_psr` with a very simple +logic: a `CCWE` request is ignored in the cycle following a `PSW` write access. + +In a first round, the `dstw` flow was fixed. The condition code requests +`ndpcntl.psr_ccwe := '1';` were removed from the flow entry states `s_dstw_def`, +`s_dstw_inc`, `s_dstw_dec`, and `s_dstw_ind` and add to the two exit states +`s_dstw_def_w` and `s_dstw_inc_w`. + +### Hindsight +Further analysis showed that this bug had in practice no consequences +- `MOV` and `CLR` don't depend on the cc state, so a re-execution with a + changed initial cc state will give the same result. Indeed, these two + instructions are often re-executed under 2.11bsd when they trigger a stack + extension. `MFP*` and `MTP*` also don't depend on the cc state and + re-execution will give correct results. Moreover, they are usually used + in kernel mode and therefore never re-executed in 2.11bsd. +- `SXT` depends on the `N` bit, but that bit is not changed by this + instruction, so a re-execution with Z, V, or C changed will give a + correct result. diff --git a/rtl/sys_gen/w11a/cmoda7/sys_w11a_c7.vhd b/rtl/sys_gen/w11a/cmoda7/sys_w11a_c7.vhd index 06575f5d..8cfa5a01 100644 --- a/rtl/sys_gen/w11a/cmoda7/sys_w11a_c7.vhd +++ b/rtl/sys_gen/w11a/cmoda7/sys_w11a_c7.vhd @@ -1,4 +1,4 @@ --- $Id: sys_w11a_c7.vhd 1339 2022-12-27 12:11:34Z mueller $ +-- $Id: sys_w11a_c7.vhd 1340 2023-01-01 08:43:05Z mueller $ -- SPDX-License-Identifier: GPL-3.0-or-later -- Copyright 2017-2022 by Walter F.J. Mueller -- @@ -28,6 +28,7 @@ -- -- Synthesized: -- Date Rev viv Target flop lutl lutm bram slic +-- 2022-12-31 1340 2022.1 xc7a35t-1 3450 6018 279 50.0 1986 -- 2022-12-27 1339 2022.1 xc7a35t-1 3454 6026 279 50.0 2013 -- 2022-12-06 1324 2022.1 xc7a35t-1 3447 5998 278 50.0 1992 -- 2022-07-05 1247 2022.1 xc7a35t-1 3411 6189 279 50.0 2021 diff --git a/rtl/w11a/pdp11_psr.vhd b/rtl/w11a/pdp11_psr.vhd index 6287d67f..6ca579c5 100644 --- a/rtl/w11a/pdp11_psr.vhd +++ b/rtl/w11a/pdp11_psr.vhd @@ -1,4 +1,4 @@ --- $Id: pdp11_psr.vhd 1287 2022-08-27 09:40:43Z mueller $ +-- $Id: pdp11_psr.vhd 1340 2023-01-01 08:43:05Z 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-12-31 1340 1.6.27 BUGFIX: inhibit CCWE when PSW being written -- 2022-08-27 1287 1.2.3 handle pm protection like cm, remove or'ing cm -- 2011-11-18 427 1.2.2 now numeric_std clean -- 2010-10-23 335 1.2.1 use ib_sel @@ -57,6 +58,7 @@ architecture syn of pdp11_psr is signal IBSEL_PSR : slbit := '0'; signal R_PSW : psw_type := psw_init; -- ps register + signal R_WE_1 : slbit := '0'; -- ps written in previous cycle begin @@ -91,12 +93,14 @@ begin if rising_edge(CLK) then + R_WE_1 <= IBSEL_PSR and IB_MREQ.we; -- remember ps write + if CRESET = '1' then R_PSW <= psw_init; else - if CCWE = '1' then + if CCWE='1' and R_WE_1='0' then -- update cc unless ps written R_PSW.cc <= CCIN; end if; diff --git a/rtl/w11a/pdp11_sequencer.vhd b/rtl/w11a/pdp11_sequencer.vhd index 5dcf3335..0a450e59 100644 --- a/rtl/w11a/pdp11_sequencer.vhd +++ b/rtl/w11a/pdp11_sequencer.vhd @@ -1,4 +1,4 @@ --- $Id: pdp11_sequencer.vhd 1338 2022-12-26 18:00:37Z mueller $ +-- $Id: pdp11_sequencer.vhd 1340 2023-01-01 08:43:05Z mueller $ -- SPDX-License-Identifier: GPL-3.0-or-later -- Copyright 2006-2022 by Walter F.J. Mueller -- @@ -13,7 +13,7 @@ -- -- Revision History: -- Date Rev Version Comment --- 2022-12-27 1339 1.6.27 remove PC port +-- 2022-12-31 1340 1.6.27 BUGFIX: cc state unchanged after abort -- 2022-12-26 1337 1.6.26 tbit logic overhaul 2, now fully 11/70 compatible -- 2022-12-12 1330 1.6.25 implement MMR0,MMR2 instruction complete -- 2022-12-10 1329 1.6.24 BUGFIX: get correct PS after vector push abort @@ -1376,7 +1376,6 @@ begin -- -> do_fork_next when s_dstw_def => -- ----------------------------------- - ndpcntl.psr_ccwe := '1'; ndpcntl.dres_sel := R_IDSTAT.res_sel; -- DRES = choice of idec ndpcntl.vmaddr_sel := c_dpath_vmaddr_ddst; -- VA = DDST nvmcntl.kstack := is_kstackdst1246; @@ -1386,12 +1385,12 @@ begin nstate := s_dstw_def_w; do_memcheck(nstate, nstatus, imemok); if imemok then + ndpcntl.psr_ccwe := '1'; idm_idone := '1'; -- instruction done do_fork_next(nstate, nstatus, nmmumoni); -- fetch next end if; when s_dstw_inc => -- ----------------------------------- - ndpcntl.psr_ccwe := '1'; ndpcntl.vmaddr_sel := c_dpath_vmaddr_ddst; -- VA = DDST ndpcntl.ounit_asel := c_ounit_asel_ddst; -- OUNIT A=DDST (for else) do_const_opsize(ndpcntl, R_IDSTAT.is_bytop, DSTDEF, DSTREG); --(...) @@ -1427,6 +1426,7 @@ begin nstatus.do_grwe := '0'; do_memcheck(nstate, nstatus, imemok); if imemok then + ndpcntl.psr_ccwe := '1'; idm_idone := '1'; -- instruction done do_fork_next(nstate, nstatus, nmmumoni); -- fetch next end if; @@ -1442,7 +1442,6 @@ begin end if; when s_dstw_dec => -- ----------------------------------- - ndpcntl.psr_ccwe := '1'; ndpcntl.ounit_asel := c_ounit_asel_ddst; -- OUNIT A=DDST do_const_opsize(ndpcntl, R_IDSTAT.is_bytop, DSTDEF, DSTREG); ndpcntl.ounit_bsel := c_ounit_bsel_const;-- OUNIT B=const @@ -1467,7 +1466,6 @@ begin end if; when s_dstw_ind => -- ----------------------------------- - ndpcntl.psr_ccwe := '1'; do_memread_i(nstate, ndpcntl, nvmcntl, s_dstw_ind_w); when s_dstw_ind_w => -- ----------------------------------- diff --git a/tools/bin/tmuconv b/tools/bin/tmuconv index 88dc6129..39501e61 100755 --- a/tools/bin/tmuconv +++ b/tools/bin/tmuconv @@ -1,10 +1,11 @@ #!/usr/bin/perl -w -# $Id: tmuconv 1324 2022-12-01 11:24:20Z mueller $ +# $Id: tmuconv 1340 2023-01-01 08:43:05Z mueller $ # SPDX-License-Identifier: GPL-3.0-or-later # Copyright 2008-2022 by Walter F.J. Mueller # # Revision History: # Date Rev Version Comment +# 2022-12-29 1340 1.1.12 fix ru header # 2022-12-01 1324 1.1.11 change VFETCH text for MMU(250) and FPP(244) # 2022-11-18 1316 1.1.10 add -t_ru06 and -t_flow # 2022-10-25 1309 1.1.9 rename _gpr -> _gr @@ -486,7 +487,7 @@ if ($nopts == 0) { # if no opts, assume t_id i_vf # write header print "# cycle id pc psw ireg code nc\n" if $opts{t_id}; -print "# cycle ru b sr data\n" +print "# cycle ru b sr data grstat\n" if $opts{t_ru} or $opts{t_ru06}; print "# cycle em d be addr wdat rdat crwh nc\n" if $opts{t_em} or $opts{t_vf}; diff --git a/tools/gwstart/lib/psr.tcl b/tools/gwstart/lib/psr.tcl new file mode 100644 index 00000000..3d71d76f --- /dev/null +++ b/tools/gwstart/lib/psr.tcl @@ -0,0 +1,21 @@ +# $Id: psr.tcl 1340 2023-01-01 08:43:05Z mueller $ +# SPDX-License-Identifier: GPL-3.0-or-later +# Copyright 2022- by Walter F.J. Mueller +# +# pdp11_psr basics +# +gwaddcom "psr" +gwaddsig {**.psr.clk} +gwaddsig -bin {**.psr.r_psw.cmode} +gwaddsig -bin {**.psr.r_psw.pmode} +gwaddsig {**.psr.r_psw.rset} +gwaddsig -oct {**.psr.r_psw.pri} +gwaddsig {**.psr.r_psw.tflag} +gwaddsig -bin {**.psr.r_psw.cc} +gwaddsig -bin {**.psr.ccin} +gwaddsig {**.psr.ccwe} +gwaddsig {**.psr.we} +gwaddsig {**.psr.func} +gwaddsig {**.psr.ibsel_psr} +gwaddsig {**.psr.ib_mreq.we} +gwaddsig {**.psr.r_we_1} diff --git a/tools/tcode/cpu_details.mac b/tools/tcode/cpu_details.mac index 4b484f01..2f3b2cee 100644 --- a/tools/tcode/cpu_details.mac +++ b/tools/tcode/cpu_details.mac @@ -1,4 +1,4 @@ -; $Id: cpu_details.mac 1337 2022-12-26 11:14:21Z mueller $ +; $Id: cpu_details.mac 1340 2023-01-01 08:43:05Z mueller $ ; SPDX-License-Identifier: GPL-3.0-or-later ; Copyright 2022- by Walter F.J. Mueller ; @@ -9,7 +9,7 @@ ; ; Test CPU details ; Section A: CPU registers -; Section B: stress tests +; Section B: stress and flow tests ; Section C: 11/70 specifics ; .include |lib/tcode_std_base.mac| @@ -86,7 +86,7 @@ ; A4.3 RTI/RTT tbit basics ; part 1: tbit after RTI ; part 2: tbit after RTT -; A4.4 Test A4.4 -- tbit trace tests +; A4.4 tbit trace tests ; part 0: traced TRAP that clears tbit ; part 1: simple instruction sequence ; part 2: tracing of trap instructions (EMT tested) @@ -927,6 +927,8 @@ ta0305: cmpb systyp,#sy.sih ; skip in SimH (different stklim logic) ta0401: ; ; part 1: all bits except register set (cp.ars) ---------------------- +; Note: In a direct PSW write the cc of the write prevail. +; This part tests dstw flow via MOV. Part 4 will test dstr flow. ; mov #cp.psw,r0 ; ptr to PSW mov #200$,r1 ; ptr to data @@ -934,14 +936,16 @@ ta0401: 100$: mov (r1)+,r3 ; next value beq 2000$ ; if 0 end of test mov r3,(r0) ; write PSW - mov (r0),r3 ; read PSW - bic r2,r3 ; clear NZVC - cmp (r1)+,r3 ; check value + cmp (r0),(r1)+ ; check PSW beq 100$ clr (r0) ; if error force kernel halt ; and halt ; -200$: .word cp.t,0 ; tbit (not direct writable) +200$: .word cp.c,cp.c ; C cc + .word cp.v,cp.v ; V cc + .word cp.z,cp.z ; Z cc + .word cp.n,cp.n ; N cc + .word cp.t,0 ; tbit (not direct writable) .word cp.pr7,cp.pr7 ; prio bits .word bit10!bit09!bit08,0 ; bit 10:8 unused .word bit12,bit12 ; pm(0) @@ -952,7 +956,8 @@ ta0401: ; ; part 2: PSW(11) - register set ------------------------------------- ; -2000$: mov #1000,r0 ; write to set 0 +2000$: clr cp.psw ; select set 0 + mov #1000,r0 ; write to set 0 mov #1001,r1 mov #1002,r2 mov #1003,r3 @@ -1005,6 +1010,21 @@ ta0401: clr (r0) mov #stack,sp ; +; part 4: verify PSW manipulation via BIS and BIC +; Note: In a direct PSW write the cc of the write prevail. +; + mov #cp.psw,r0 ; ptr to PSW + ccc + bis #cp.n!cp.c,(r0) ; set N+C + hcmpeq #cpn00c,(r0) + ccc + bis #cp.pr7!cp.z!cp.v,(r0) ; set PRI=7,Z+V + hcmpeq #cp.pr7!cp0zv0,(r0) + scc + bic #cp.z!cp.v,(r0) ; clr Z+V + hcmpeq #cp.pr7!cpn00c,(r0) + clr (r0) ; back to normal +; 9999$: iot ; end of test A4.1 ; ; Test A4.2 -- PSW write/read via RTI/RTT ++++++++++++++++++++++++++++ @@ -1514,14 +1534,20 @@ ta0404: mov #vhtbpt,v..bpt ; BPT handler ; 9999$: iot ; end of test A4.4 ; -; Section B: Stress tests ==================================================== +; Section B: Stress and fllow tests ========================================== ; B1 address mode torture tests ; B1.1 src-dst update hazards with (r0)+,(r0) ; B1.2 (pc)+ as destination ; B1.3 pc as destination in clr, mov, and add ; B2 pipeline torture tests -; B2.1 self-modifying code, use (pc), -(pc) -; B2.2 self-modifying code, use (pc) case 2 +; B2.1 self-modifying code, use (pc), -(pc) +; B2.2 self-modifying code, use (pc) case 2 +; B3 specifier flow tests +; B3.1 dstw flow and cc +; part 1: check cc for MOV for all modes +; part 2: check cc for CLR for all modes +; part 3: check cc for SXT for all modes +; part 4: check cc for MOV after abort for all modes ; ; Test B1: address mode torture tests +++++++++++++++++++++++++++++++++++++++ ; This sub-section tests peculiar address node usage @@ -1677,6 +1703,135 @@ tb0202: mov #2,r5 ; 9999$: iot ; end of test B2.2 ; +; Test B3: specifier flow tests +++++++++++++++++++++++++++++++++++++++++++++ +; This sub-section tests flow and cc properties +; +; Test B3.1 -- dstw flow and cc ++++++++++++++++++++++++++++++++++++++ +; +tb0301: mov #123,r0 ; src for MOV + mov #100$,r1 ; dst for (r1),(r1)+,-(r1),0(r1) + mov #200$,r2 ; dst for @(r2)+,@-(r2),@0(r2) + mov #cp.psw,r3 ; ptr to PSW + br 1000$ +; +100$: .word 0 ; dst target +200$: .word 100$ ; ptr to dst +300$: .word 1 ; odd address ptr +; +; part 1: check cc for MOV for all modes +; +1000$: scc + mov r0,r4 ; mode 0 + hcmpeq #cp000c,(r3) ; N=0,Z=0,V=0 and keep C + scc + mov r0,(r1) ; mode 1 + hcmpeq #cp000c,(r3) ; N=0,Z=0,V=0 and keep C + scc + mov r0,(r1)+ ; mode 2 + hcmpeq #cp000c,(r3) ; N=0,Z=0,V=0 and keep C + scc + mov r0,-(r1) ; mode 4 + hcmpeq #cp000c,(r3) ; N=0,Z=0,V=0 and keep C + scc + mov r0,0(r1) ; mode 6 + hcmpeq #cp000c,(r3) ; N=0,Z=0,V=0 and keep C + scc + mov r0,@(r2)+ ; mode 3 + hcmpeq #cp000c,(r3) ; N=0,Z=0,V=0 and keep C + scc + mov r0,@-(r2) ; mode 5 + hcmpeq #cp000c,(r3) ; N=0,Z=0,V=0 and keep C + scc + mov r0,@0(r2) ; mode 7 + hcmpeq #cp000c,(r3) ; N=0,Z=0,V=0 and keep C +; +; part 2: check cc for CLR for all modes +; +2000$: scc + clr r4 ; mode 0 + hcmpeq #cp0z00,(r3) ; N=0,Z=1,V=0,C=0 + scc + clr (r1) ; mode 1 + hcmpeq #cp0z00,(r3) ; N=0,Z=1,V=0,C=0 + scc + clr (r1)+ ; mode 2 + hcmpeq #cp0z00,(r3) ; N=0,Z=1,V=0,C=0 + scc + clr -(r1) ; mode 4 + hcmpeq #cp0z00,(r3) ; N=0,Z=1,V=0,C=0 + scc + clr 0(r1) ; mode 6 + hcmpeq #cp0z00,(r3) ; N=0,Z=1,V=0,C=0 + scc + clr @(r2)+ ; mode 3 + hcmpeq #cp0z00,(r3) ; N=0,Z=1,V=0,C=0 + scc + clr @-(r2) ; mode 5 + hcmpeq #cp0z00,(r3) ; N=0,Z=1,V=0,C=0 + scc + clr @0(r2) ; mode 7 + hcmpeq #cp0z00,(r3) ; N=0,Z=1,V=0,C=0 +; +; part 3: check cc for SXT for all modes +; +3000$: scc + sxt r4 ; mode 0 + hcmpeq #cpn00c,(r3) ; Z=0,V=0 and keep N,C + scc + sxt (r1) ; mode 1 + hcmpeq #cpn00c,(r3) ; Z=0,V=0 and keep N,C + scc + sxt (r1)+ ; mode 2 + hcmpeq #cpn00c,(r3) ; Z=0,V=0 and keep N,C + scc + sxt -(r1) ; mode 4 + hcmpeq #cpn00c,(r3) ; Z=0,V=0 and keep N,C + scc + sxt 0(r1) ; mode 6 + hcmpeq #cpn00c,(r3) ; Z=0,V=0 and keep N,C + scc + sxt @(r2)+ ; mode 3 + hcmpeq #cpn00c,(r3) ; Z=0,V=0 and keep N,C + scc + sxt @-(r2) ; mode 5 + hcmpeq #cpn00c,(r3) ; Z=0,V=0 and keep N,C + scc + sxt @0(r2) ; mode 7 + hcmpeq #cpn00c,(r3) ; Z=0,V=0 and keep N,C +; +; part 4: check cc for MOV after abort for all memory modes +; Notes: +; - w11 updates the cc state after possible address errors +; - SimH updates the cc state before possible address errors +; +4000$: cmpb systyp,#sy.sih ; skip on SimH + beq 9999$ + clr r5 ; abort counter + mov #4100$,v..iit ; set up iit handler + mov #1,r1 ; odd dst for (r1),(r1)+,-(r1),0(r1) + mov #300$,r2 ; odd dst for @(r2)+,@-(r2),@0(r2) + scc +; + mov r0,(r1) ; mode 1 + mov r0,(r1)+ ; mode 2 + mov #3,r1 ; restore r1 + scc + mov r0,-(r1) ; mode 4 + mov r0,0(r1) ; mode 6 + mov r0,@(r2)+ ; mode 3 + mov r0,@-(r2) ; mode 5 + mov r0,@0(r2) ; mode 7 + br 4200$ +; +4100$: inc r5 ; bump counter + hcmpeq #cpnzvc,2(sp) ; PS cc untouched !! + rti ; continue (possible here!) +; +4200$: hcmpeq #7.,r5 ; check that all 7 mov get address error + mov #v..iit+2,v..iit ; restore +; +9999$: iot ; end of test B3.1 +; ; Section C: 11/70 specifics ================================================= ; C1 Implementation differences ; C1.1 Register used as source and changed in dst flow @@ -1735,7 +1890,7 @@ tc0103: mov #vhugen,v..iit ; set iit handler ; END OF ALL TESTS - loop closure ============================================ ; mov tstno,r0 ; hack, for easy monitoring ... - hcmpeq tstno,#29. ; all tests done ? + hcmpeq tstno,#30. ; all tests done ? ; jmp loop ;