diff --git a/doc/CHANGELOG.md b/doc/CHANGELOG.md index 9ee55b67..24a143e3 100644 --- a/doc/CHANGELOG.md +++ b/doc/CHANGELOG.md @@ -71,6 +71,8 @@ The full set of tests is only run for tagged releases. - removed designs (drop Atlys) - rtl/sys_gen/tst_rlink_cuff/atlys/sys_tst_rlink_cuff_atlys - rtl/sys_gen/tst_snhumanio/atlys/sys_tst_snhumanio_atlys + - rtl/w11a/pdp11_psr: handle pm protection like cm, for details see + [ECO-031](ECO-031-PSW_protection.md) - general changes - segment -< page rename - DEC used in early documents 'segment', later on 'page' for the MMU object diff --git a/doc/ECO-027-trap_mmu.md b/doc/ECO-027-trap_mmu.md index 6acc904e..6ba47369 100644 --- a/doc/ECO-027-trap_mmu.md +++ b/doc/ECO-027-trap_mmu.md @@ -41,7 +41,7 @@ The later happens due to the logic of state `s_opg_gen_rmw_w`: `pdp11_sequencer` was modified to ensure that `R_STATUS.trap_mmu` is only set in `do_memcheck`. Same for `trap_ysv` (which had the same potential bug) -### Provisos +### Hindsight The issue was found by systematic testing of mmu fault and trap behavior. Because known OS don't use mmu traps the issue should not have any impact on practical usage with OS like rsx or 211bsd. diff --git a/doc/ECO-030-EI_ACK-misroute.md b/doc/ECO-030-EI_ACK-misroute.md index 22aa0742..4b83e7da 100644 --- a/doc/ECO-030-EI_ACK-misroute.md +++ b/doc/ECO-030-EI_ACK-misroute.md @@ -52,6 +52,6 @@ clock cycle. `EI_REQ` line from last cycle, and use this value to route `EI_ACKM` to the `EI_ACK` lines. -### Provisos +### Hindsight That 12 year old code worked with no apparent problems doesn't prove that it is free of fundamental bugs. diff --git a/doc/ECO-031-PSW_protection.md b/doc/ECO-031-PSW_protection.md new file mode 100644 index 00000000..8143c934 --- /dev/null +++ b/doc/ECO-031-PSW_protection.md @@ -0,0 +1,63 @@ +# ECO-031: `PSW` privilege escalation protection overzealous (2022-08-27) + +### Scope +- Was in w11a from the very beginning (2007) +- Affects: all w11a systems + +### Symptom summary +No Symptoms. Was discovered in a code review. + +### Background +The privilege escalation protection for RTT/RTI ensures that a lower +privileged code can't increase the mode. In non-kernel mode, this is done +by or'ing' the new pm,cm,rset values to the existing value. With +``` + kernel 00 + super 01 + user 11 +``` +this fulfills the objective. + +### Analysis +A code review showed a discrepancy between SimH and w11a handling +In SimH this is done in in pdp11_cpu.c in put_PSW() +``` + if (prot) { /* protected? */ + cm = cm | ((val >> PSW_V_CM) & 03); /* or to cm,pm,rs */ + pm = pm | ((val >> PSW_V_PM) & 03); /* can't change ipl */ + rs = rs | ((val >> PSW_V_RS) & 01); + } + else { + cm = (val >> PSW_V_CM) & 03; /* write cm,pm,rs,ipl */ + pm = (val >> PSW_V_PM) & 03; + rs = (val >> PSW_V_RS) & 01; + ipl = (val >> PSW_V_IPL) & 07; + } +``` +In w11a in pdp11_psr.vhd the handling was +``` + R_PSW.cmode <= R_PSW.cmode or DIN(psw_ibf_cmode); + R_PSW.pmode <= R_PSW.pmode or DIN(psw_ibf_pmode) or + R_PSW.cmode or DIN(psw_ibf_cmode); + R_PSW.rset <= R_PSW.rset or DIN(psw_ibf_rset); +``` +Unclear why for `pmode` the `cmode` bits where or'ed in too. +Further analysis +- a scan through documentation did not find a hint +- EK-KB11C-TM-001_1170procMan.pdf page 132 states + _prev mode protected like curr mode_ +- MP0KB11-C0_1170engDrw_Nov75.pdf + - PSW logic in drawing PDRD on page 59. + - pm has the logic to set it from cm in vector pushes + - but in the RTT/RTI update case, pm is handled like cm and reset + +### Fixes +Simply remove the extra term, now +``` + R_PSW.pmode <= R_PSW.pmode or DIN(psw_ibf_pmode); +``` + +### Hindsight +Unclear why it was implemented with this extra term. It is the responsibility +of the software to ensure previous mode is not more privileged than the +current mode when a process is started. diff --git a/rtl/w11a/pdp11_psr.vhd b/rtl/w11a/pdp11_psr.vhd index 926e7086..6287d67f 100644 --- a/rtl/w11a/pdp11_psr.vhd +++ b/rtl/w11a/pdp11_psr.vhd @@ -1,6 +1,6 @@ --- $Id: pdp11_psr.vhd 1181 2019-07-08 17:00:50Z mueller $ +-- $Id: pdp11_psr.vhd 1287 2022-08-27 09:40:43Z mueller $ -- SPDX-License-Identifier: GPL-3.0-or-later --- Copyright 2006-2011 by Walter F.J. Mueller +-- Copyright 2006-2022 by Walter F.J. Mueller -- ------------------------------------------------------------------------------ -- Module Name: pdp11_psr - syn @@ -9,10 +9,11 @@ -- Dependencies: ib_sel -- Test bench: tb/tb_pdp11_core (implicit) -- Target Devices: generic --- Tool versions: ise 8.2-14.7; viv 2014.4; ghdl 0.18-0.31 +-- Tool versions: ise 8.2-14.7; viv 2022.1; ghdl 0.18-2.0.0 -- -- Revision History: -- Date Rev Version Comment +-- 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 -- 2010-10-17 333 1.2 use ibus V2 interface @@ -121,8 +122,7 @@ begin when c_psr_func_wrti => -- wrti (rti/rtt in non-kernel mode) R_PSW.cmode <= R_PSW.cmode or DIN(psw_ibf_cmode); - R_PSW.pmode <= R_PSW.pmode or DIN(psw_ibf_pmode) or - R_PSW.cmode or DIN(psw_ibf_cmode); + R_PSW.pmode <= R_PSW.pmode or DIN(psw_ibf_pmode); R_PSW.rset <= R_PSW.rset or DIN(psw_ibf_rset); R_PSW.tflag <= DIN(psw_ibf_tflag); R_PSW.cc <= DIN(psw_ibf_cc);