1
0
mirror of https://github.com/wfjm/w11.git synced 2026-02-26 09:03:45 +00:00

BUGFIX: EI_ACK misrouted in rare cases (ECO-030)

- ib_intmap,ib_intmap24: BUGFIX: ensure ACK send to correct device
- ibdr_{maxi,mini}sys: add CLK port to ib_intmap,ib_intmap24
- pdp11_irq: BUGFIX: re-write, ensure ACK send to correct device
- for further details see doc/ECO-030-EI_ACK-misroute.md
This commit is contained in:
wfjm
2019-04-24 11:40:28 +02:00
parent f9f7000a4a
commit 544f1c99d2
8 changed files with 216 additions and 81 deletions

View File

@@ -64,7 +64,7 @@ The full set of tests is only run for tagged releases.
- Rw11Cntl{DEUNA,DL11,RHRP,RK11,RL11,TM11}: call UnitSetupAll() in Start()
- Rw11CntlLP11: remove SetOnline(), use UnitSetup()
- Rw11CntlPC11
- BootCode(): boot loader rewritten
- BootCode(): boot loader rewritten
- remove SetOnline(), use UnitSetup()
- asm-11: .end directive autocreates '...end' label
- ti_w11: for -e use .end start address when available
@@ -72,7 +72,7 @@ The full set of tests is only run for tagged releases.
- rbd_rbmon: more robust ack,err trace when busy
- rbd_tester: use now fifo_simple_dram
- ibd_ibtst: rename dly[rw]->bsy[rw]; datto for write; add datab
- ibdr_dl11: changes for ibdr_dl11_buf compatibility (val in msb, ib_rlim_slv)
- ibdr_dl11: changes for ibdr_dl11_buf compatibility (val in msb, ib_rlim_slv)
- ibdr_lp11: move valid bit to msb of buf (for ibdr_lp11_buf compatibility)
- ibdr_pc11: changes for ibdr_pc11_buf compatibility
- sys_w11a_s3: set BTOWIDTH 7 (was 6, must be > vmbox atowidth (6))
@@ -95,6 +95,8 @@ The full set of tests is only run for tagged releases.
2015-05-12 release w11a_V0.753 which inverted the mask polarity. Had no
practical consequences, went therefore undetected for such a long time.
- RtclRw11Cpu: now proper cpu attn test in the server inactive case
- pdp11_irq,ib_intmap24: now proper EI_ACK routing, for all detail see
[ECO-030](ECO-030-EI_ACK-misroute.md).
### Known issues

View File

@@ -0,0 +1,57 @@
# ECO-030: `EI_ACK` misrouted in rare cases (2019-04-23)
### Scope
- Was in w11a from the very beginning (2007)
- Affects: all w11a systems
### Symptom summary
Tests done with the `pc11copy.mac` code after the buffered version
`ibdr_pc11_buf.vhd` of the `PC11` taper tape reader/puncher had been
implemented showed that sometimes a reader interrupt is lost when
the interrupt rate limiter is enabled.
### Background
The original `UNIBUS` systems used daisy chains for the four priority levels
and special bus requests to determine vector address and PSW information.
The implementation of device interrupts in the w11 is very different.
Each interrupt source has an interrupt request `EI_REQ` output and an
acknowledge `EI_ACK` input port.
The `ib_intmap24` entity evaluates the `EI_REQ` status, determines the
highest priority source, and forwards the resulting priority and vector
information to `pdp11_core`.
When the CPU accepts an interrupt it sends an acknowledge signal `EI_ACKM`
which is send by `ib_intmap24` to the `EI_REQ` line of the winning
interrupt source.
### Analysis
The `EI_ACKM` signal is send in the cycle _after_ the `EI_REQ` pattern was
evaluated. The old implementation of `ib_intmap24` was plain combinatoric
logic, and routed `EI_ACKM` to the `EI_ACK` line based on the status of
the `EI_REQ` pattern of the current cycle. That fails when a higher
priority interrupt source appeared in this cycle. In such cases the
acknowledge is send to the new winner, where it most likely will clear
the interrupt request. The net effect is that the new winner might loose
and interrupt, while the old winner misses an `EI_ACK` and most likely
causes a double interrupt.
The obvious question is: _why did it apparently work the last 12 years ?_
Before the buffered versions of `LP11`, `PC11`, and `DL11` were introduced
all interrupts where caused by `ibus` transactions, the only exception was
the `KW11-L` line clock. So device interrupts were never created in the in
a cycle where a vector decision is taken. Only clock interrupts were
vulnerable, but loosing a clock interrupt in rare cases has little practical
consequences.
### Fixes
Simply ensure that `INT_ACK` is routed based on the status of the previous
clock cycle.
- `pdp11_irq`: add a state bit which ensures that `INT_ACK` is only
forwarded to `EI_ACKM` when an external source won in last cycle
- `ib_intmap24`: add a state register which holds the number of the winning
`EI_REQ` line from last cycle, and use this value to route `EI_ACKM` to the
`EI_ACK` lines.
### Provisos
That 12 year old code worked with no apparent problems doesn't prove that
it is free of fundamental bugs.

View File

@@ -1,6 +1,6 @@
-- $Id: ib_intmap.vhd 984 2018-01-02 20:56:27Z mueller $
-- $Id: ib_intmap.vhd 1136 2019-04-24 09:27:28Z mueller $
--
-- Copyright 2006-2011 by Walter F.J. Mueller <W.F.J.Mueller@gsi.de>
-- Copyright 2006-2019 by Walter F.J. Mueller <W.F.J.Mueller@gsi.de>
--
-- This program is free software; you may redistribute and/or modify it under
-- the terms of the GNU General Public License as published by the Free
@@ -18,7 +18,7 @@
-- Dependencies: -
-- Test bench: tb/tb_pdp11_core (implicit)
-- Target Devices: generic
-- Tool versions: ise 8.2-14.7; viv 2014.4-2016.4; ghdl 0.18-0.33
-- Tool versions: ise 8.2-14.7; viv 2014.4-2017.2; ghdl 0.18-0.35
--
-- Synthesized:
-- Date Rev viv Target flop lutl lutm bram slic MHz
@@ -27,6 +27,7 @@
--
-- Revision History:
-- Date Rev Version Comment
-- 2019-04-23 1136 1.2 BUGFIX: ensure ACK send to correct device
-- 2011-11-18 427 1.2.2 now numeric_std clean
-- 2008-08-22 161 1.2.1 renamed pdp11_ -> ib_; use iblib
-- 2008-01-20 112 1.2 add INTMAP generic to externalize config
@@ -49,6 +50,7 @@ entity ib_intmap is -- external interrupt mapper
generic (
INTMAP : intmap_array_type := intmap_array_init);
port (
CLK : in slbit; -- clock
EI_REQ : in slv16_1; -- interrupt request lines
EI_ACKM : in slbit; -- interrupt acknowledge (from master)
EI_ACK : out slv16_1; -- interrupt acknowledge (to requestor)
@@ -60,6 +62,7 @@ end ib_intmap;
architecture syn of ib_intmap is
signal EI_LINE : slv4 := (others=>'0'); -- external interrupt line
signal R_LINE : slv4 := (others=>'0'); -- line on last cycle
type intp_type is array (15 downto 0) of slv3;
type intv_type is array (15 downto 0) of slv9;
@@ -124,21 +127,35 @@ begin
"0001" when EI_REQ( 1)='1' else
"0000";
proc_intmap : process (EI_LINE, EI_ACKM)
variable iline : integer := 0;
proc_line: process (CLK)
begin
if rising_edge(CLK) then
R_LINE <= EI_LINE;
end if;
end process proc_line;
-- Note: EI_ACKM comes one cycle after vector is latched ! Therefore
-- - use EI_LINE to select vector to send to EI_PRI and EI_VECT
-- - use R_LINE to select EI_ACM line for acknowledge
proc_intmap : process (EI_LINE, EI_ACKM, R_LINE)
variable ilinecur : integer := 0;
variable ilinelst : integer := 0;
variable iei_ack : slv16 := (others=>'0');
begin
iline := to_integer(unsigned(EI_LINE));
ilinecur := to_integer(unsigned(EI_LINE));
ilinelst := to_integer(unsigned(R_LINE));
-- send info of currently highest priority request
EI_PRI <= conf_intp(ilinecur);
EI_VECT <= conf_intv(ilinecur)(8 downto 2);
-- route acknowledge back to winner line of last cycle
iei_ack := (others=>'0');
if EI_ACKM = '1' then
iei_ack(iline) := '1';
iei_ack(ilinelst) := '1';
end if;
EI_ACK <= iei_ack(EI_ACK'range);
EI_PRI <= conf_intp(iline);
EI_VECT <= conf_intv(iline)(8 downto 2);
end process proc_intmap;

View File

@@ -1,6 +1,6 @@
-- $Id: ib_intmap24.vhd 984 2018-01-02 20:56:27Z mueller $
-- $Id: ib_intmap24.vhd 1136 2019-04-24 09:27:28Z mueller $
--
-- Copyright 2017- by Walter F.J. Mueller <W.F.J.Mueller@gsi.de>
-- Copyright 2017-2019 by Walter F.J. Mueller <W.F.J.Mueller@gsi.de>
--
-- This program is free software; you may redistribute and/or modify it under
-- the terms of the GNU General Public License as published by the Free
@@ -18,7 +18,7 @@
-- Dependencies: -
-- Test bench: tb/tb_pdp11_core (implicit)
-- Target Devices: generic
-- Tool versions: ise 14.7; viv 2016.4; ghdl 0.33
-- Tool versions: ise 14.7; viv 2016.4-2017.2; ghdl 0.33-0.35
--
-- Synthesized:
-- Date Rev viv Target flop lutl lutm bram slic MHz
@@ -27,6 +27,7 @@
--
-- Revision History:
-- Date Rev Version Comment
-- 2019-04-23 1136 1.1 BUGFIX: ensure ACK send to correct device
-- 2017-01-28 846 1.0 Initial version (cloned from ib_intmap.vhd)
------------------------------------------------------------------------------
@@ -43,6 +44,7 @@ entity ib_intmap24 is -- external interrupt mapper (23 line)
generic (
INTMAP : intmap24_array_type := intmap24_array_init);
port (
CLK : in slbit; -- clock
EI_REQ : in slv24_1; -- interrupt request lines
EI_ACKM : in slbit; -- interrupt acknowledge (from master)
EI_ACK : out slv24_1; -- interrupt acknowledge (to requestor)
@@ -54,6 +56,7 @@ end ib_intmap24;
architecture syn of ib_intmap24 is
signal EI_LINE : slv5 := (others=>'0'); -- external interrupt line
signal R_LINE : slv5 := (others=>'0'); -- line on last cycle
type intp_type is array (23 downto 0) of slv3;
type intv_type is array (23 downto 0) of slv9;
@@ -143,21 +146,35 @@ begin
"00001" when EI_REQ( 1)='1' else
"00000";
proc_intmap : process (EI_LINE, EI_ACKM)
variable iline : integer := 0;
variable iei_ack : slv24 := (others=>'0');
proc_line: process (CLK)
begin
if rising_edge(CLK) then
R_LINE <= EI_LINE;
end if;
end process proc_line;
-- Note: EI_ACKM comes one cycle after vector is latched ! Therefore
-- - use EI_LINE to select vector to send to EI_PRI and EI_VECT
-- - use R_LINE to select EI_ACM line for acknowledge
proc_intmap : process (EI_LINE, EI_ACKM, R_LINE)
variable ilinecur : integer := 0;
variable ilinelst : integer := 0;
variable iei_ack : slv24 := (others=>'0');
begin
iline := to_integer(unsigned(EI_LINE));
ilinecur := to_integer(unsigned(EI_LINE));
ilinelst := to_integer(unsigned(R_LINE));
-- send info of currently highest priority request
EI_PRI <= conf_intp(ilinecur);
EI_VECT <= conf_intv(ilinecur)(8 downto 2);
-- route acknowledge back to winner line of last cycle
iei_ack := (others=>'0');
if EI_ACKM = '1' then
iei_ack(iline) := '1';
iei_ack(ilinelst) := '1';
end if;
EI_ACK <= iei_ack(EI_ACK'range);
EI_PRI <= conf_intp(iline);
EI_VECT <= conf_intv(iline)(8 downto 2);
end process proc_intmap;

View File

@@ -1,4 +1,4 @@
-- $Id: ibdr_maxisys.vhd 1131 2019-04-14 13:24:25Z mueller $
-- $Id: ibdr_maxisys.vhd 1136 2019-04-24 09:27:28Z mueller $
--
-- Copyright 2009-2019 by Walter F.J. Mueller <W.F.J.Mueller@gsi.de>
--
@@ -51,6 +51,7 @@
--
-- Revision History:
-- Date Rev Version Comment
-- 2019-04-23 1136 1.6.6 add CLK port to ib_intmap24
-- 2019-04-14 1131 1.6.5 ib_rlim_gen has CPUSUSP port; RLIM_CEV now slv8
-- 2019-04-07 1127 1.6.3 ibdr_dl11: use RLIM_CEV, drop CE_USEC
-- 2019-03-17 1123 1.6.2 add ib_rlim_gen, use with ibdr_lp11_buf
@@ -521,6 +522,7 @@ begin
generic map (
INTMAP => conf_intmap24)
port map (
CLK => CLK,
EI_REQ => EI_REQ,
EI_ACKM => EI_ACKM,
EI_ACK => EI_ACK,

View File

@@ -1,4 +1,4 @@
-- $Id: ibdr_minisys.vhd 1131 2019-04-14 13:24:25Z mueller $
-- $Id: ibdr_minisys.vhd 1136 2019-04-24 09:27:28Z mueller $
--
-- Copyright 2008-2019 by Walter F.J. Mueller <W.F.J.Mueller@gsi.de>
--
@@ -33,6 +33,7 @@
--
-- Revision History:
-- Date Rev Version Comment
-- 2019-04-23 1136 1.1.5 add CLK port to ib_intmap
-- 2019-04-14 1131 1.1.4 ib_rlim_gen has CPUSUSP port; RLIM_CEV now slv8
-- 2019-04-07 1129 1.1.3 ibdr_dl11: use RLIM_CEV, drop CE_USEC
-- 2011-11-18 427 1.1.2 now numeric_std clean
@@ -203,6 +204,7 @@ begin
generic map (
INTMAP => conf_intmap)
port map (
CLK => CLK,
EI_REQ => EI_REQ,
EI_ACKM => EI_ACKM,
EI_ACK => EI_ACK,

View File

@@ -1,4 +1,4 @@
-- $Id: iblib.vhd 1131 2019-04-14 13:24:25Z mueller $
-- $Id: iblib.vhd 1136 2019-04-24 09:27:28Z mueller $
--
-- Copyright 2008-2019 by Walter F.J. Mueller <W.F.J.Mueller@gsi.de>
--
@@ -19,6 +19,7 @@
-- Tool versions: ise 8.1-14.7; viv 2014.4-2018.3; ghdl 0.18-0.35
-- Revision History:
-- Date Rev Version Comment
-- 2019-04-23 1136 2.2.4 add CLK port to ib_intmap,ib_intmap24
-- 2019-04-14 1131 2.2.3 ib_rlim_gen: add CPUSUSP port; RLIM_CEV now slv8
-- 2019-03-17 1123 2.2.2 add ib_rlim_gen,ib_rlim_slv
-- 2019-02-10 1111 2.2.1 add ibd_ibtst
@@ -132,6 +133,7 @@ component ib_intmap is -- external interrupt mapper (15 line)
generic (
INTMAP : intmap_array_type := intmap_array_init);
port (
CLK : in slbit; -- clock
EI_REQ : in slv16_1; -- interrupt request lines
EI_ACKM : in slbit; -- interrupt acknowledge (from master)
EI_ACK : out slv16_1; -- interrupt acknowledge (to requestor)
@@ -147,6 +149,7 @@ component ib_intmap24 is -- external interrupt mapper (23 line)
generic (
INTMAP : intmap24_array_type := intmap24_array_init);
port (
CLK : in slbit; -- clock
EI_REQ : in slv24_1; -- interrupt request lines
EI_ACKM : in slbit; -- interrupt acknowledge (from master)
EI_ACK : out slv24_1; -- interrupt acknowledge (to requestor)

View File

@@ -1,6 +1,6 @@
-- $Id: pdp11_irq.vhd 984 2018-01-02 20:56:27Z mueller $
-- $Id: pdp11_irq.vhd 1136 2019-04-24 09:27:28Z mueller $
--
-- Copyright 2007-2011 by Walter F.J. Mueller <W.F.J.Mueller@gsi.de>
-- Copyright 2007-2019 by Walter F.J. Mueller <W.F.J.Mueller@gsi.de>
--
-- This program is free software; you may redistribute and/or modify it under
-- the terms of the GNU General Public License as published by the Free
@@ -18,10 +18,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 2014.4-2017.2; ghdl 0.18-0.35
--
-- Revision History:
-- Date Rev Version Comment
-- 2019-04-23 1136 1.3 BUGFIX: re-write, ensure ACK send to correct device
-- 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
@@ -63,14 +64,27 @@ end pdp11_irq;
architecture syn of pdp11_irq is
constant ibaddr_pirq : slv16 := slv(to_unsigned(8#177772#,16));
constant vect_pirq : slv9 := slv(to_unsigned(8#240#,9));
subtype pirq_ubf_pir is integer range 15 downto 9;
subtype pirq_ubf_pia_h is integer range 7 downto 5;
subtype pirq_ubf_pia_l is integer range 3 downto 1;
subtype pirq_ibf_pir is integer range 15 downto 9;
subtype pirq_ibf_pia_h is integer range 7 downto 5;
subtype pirq_ibf_pia_l is integer range 3 downto 1;
type regs_type is record -- state registers
pirq : slv8_1; -- pirq mask
eilast : slbit; -- ei won in last cycle
end record regs_type;
constant regs_init : regs_type := (
(others=>'0'), -- pirq
'0' -- eilast
);
signal R_REGS : regs_type := regs_init;
signal N_REGS : regs_type := regs_init;
signal IBSEL_PIRQ : slbit := '0';
signal R_PIRQ : slv8_1 := (others => '0'); -- pirq register
signal PI_PRI : slv3 := (others => '0'); -- prog.int. priority
signal PI_PRI : slv3 := (others => '0'); -- prog.int. priority
-- attribute PRIORITY_EXTRACT : string;
-- attribute PRIORITY_EXTRACT of PI_PRI : signal is "force";
@@ -85,56 +99,77 @@ begin
IB_MREQ => IB_MREQ,
SEL => IBSEL_PIRQ
);
proc_ibres : process (IBSEL_PIRQ, IB_MREQ, R_PIRQ, PI_PRI)
variable idout : slv16 := (others=>'0');
begin
idout := (others=>'0');
if IBSEL_PIRQ = '1' then
idout(pirq_ubf_pir) := R_PIRQ;
idout(pirq_ubf_pia_h) := PI_PRI;
idout(pirq_ubf_pia_l) := PI_PRI;
end if;
IB_SRES.dout <= idout;
IB_SRES.ack <= IBSEL_PIRQ and (IB_MREQ.re or IB_MREQ.we); -- ack all
IB_SRES.busy <= '0';
end process proc_ibres;
proc_pirq : process (CLK)
proc_regs: process (CLK)
begin
if rising_edge(CLK) then
if BRESET = '1' then
R_PIRQ <= (others => '0');
elsif IBSEL_PIRQ='1' and IB_MREQ.we='1'and IB_MREQ.be1='1' then
R_PIRQ <= IB_MREQ.din(pirq_ubf_pir);
end if;
R_REGS <= regs_init;
else
R_REGS <= N_REGS;
end if;
end if;
end process proc_pirq;
PI_PRI <= "111" when R_PIRQ(7)='1' else
"110" when R_PIRQ(6)='1' else
"101" when R_PIRQ(5)='1' else
"100" when R_PIRQ(4)='1' else
"011" when R_PIRQ(3)='1' else
"010" when R_PIRQ(2)='1' else
"001" when R_PIRQ(1)='1' else
"000";
proc_irq : process (PI_PRI, EI_PRI, EI_VECT, INT_ACK)
constant vect_default : slv9 := slv(to_unsigned(8#240#,9));
begin
EI_ACKM <= '0';
if unsigned(EI_PRI) > unsigned(PI_PRI) then
PRI <= EI_PRI;
VECT <= EI_VECT;
EI_ACKM <= INT_ACK;
else
PRI <= PI_PRI;
VECT <= vect_default(8 downto 2);
end if;
end process proc_irq;
end process proc_regs;
PI_PRI <= "111" when R_REGS.pirq(7)='1' else
"110" when R_REGS.pirq(6)='1' else
"101" when R_REGS.pirq(5)='1' else
"100" when R_REGS.pirq(4)='1' else
"011" when R_REGS.pirq(3)='1' else
"010" when R_REGS.pirq(2)='1' else
"001" when R_REGS.pirq(1)='1' else
"000";
proc_next : process (R_REGS, IB_MREQ, IBSEL_PIRQ, PI_PRI,
EI_PRI, EI_VECT, INT_ACK)
variable r : regs_type := regs_init;
variable n : regs_type := regs_init;
variable idout : slv16 := (others=>'0');
variable ibreq : slbit := '0';
begin
r := R_REGS;
n := R_REGS;
idout := (others=>'0');
ibreq := IB_MREQ.re or IB_MREQ.we;
-- ibus transactions
if IBSEL_PIRQ = '1' then
idout(pirq_ibf_pir) := r.pirq;
idout(pirq_ibf_pia_h) := PI_PRI;
idout(pirq_ibf_pia_l) := PI_PRI;
if IB_MREQ.we='1'and IB_MREQ.be1='1' then
n.pirq := IB_MREQ.din(pirq_ibf_pir);
end if;
end if;
-- pirq vs ei_vect selection
if unsigned(EI_PRI) > unsigned(PI_PRI) then
n.eilast := '1';
PRI <= EI_PRI;
VECT <= EI_VECT;
else
n.eilast := '0';
PRI <= PI_PRI;
VECT <= vect_pirq(8 downto 2);
end if;
-- Note: INT_ACK comes one cycle after vector is latched !
-- therefore send INT_ACK to EI_ACKM only if EI was winner in last cycle
EI_ACKM <= '0';
if r.eilast = '1' then
EI_ACKM <= INT_ACK;
end if;
N_REGS <= n;
IB_SRES.dout <= idout;
IB_SRES.ack <= IBSEL_PIRQ and ibreq; -- ack all
IB_SRES.busy <= '0';
end process proc_next;
end syn;