Add dram ECC tests, dram scrubber disabled at start

hdl/rtl/hydra_core.vhd

@@ -113,6 +113,7 @@ architecture arch of hydra_core is
   signal dm_mem_rdata, dm_wb_rdata : std_logic_vector(31 downto 0);
   signal dm_wb_write, dm_select_wb, dm_select_ram : std_logic;
   signal dm_done, dm_err : std_logic;
+  signal dram_ecc_mask : std_logic_vector(7 downto 0);
 
   signal dwb_out         : t_wishbone_master_out;
 
@@ -263,12 +264,13 @@ begin
     port map (
       clk_i => clk_sys_i,
       rst_n_i => rst_n_i,
-      cpu_rst_n_i => cpu_rst_n_i,
+      scrub_en_i => dram_scrub_en,
       addr_i => reg_dm_addr(g_DRAM_LOG_SIZE - 1 downto 2),
       en_i => reg_dm_en,
       we_i => reg_dm_store,
       sel_i => reg_dm_data_select,
       data_i => reg_dm_data_s,
+      eccm_i => dram_ecc_mask(6 downto 0),
       data_o => dm_mem_rdata,
       done_o => dm_done,
       err_o => dm_err,
@@ -524,6 +526,7 @@ begin
       dram_scrub_de_i => nbr_dram_scrub_uncorr,
       cpu_data_err_i => nbr_cpu_data_err,
       cpu_iaddr_err_i => nbr_cpu_iaddr_err,
+      dram_ecc_mask_o => dram_ecc_mask,
       scrub_cfg_iram_en_i => iram_scrub_en,
       scrub_cfg_iram_en_o => scrub_cfg_iram_val,
       scrub_cfg_dram_en_i => dram_scrub_en,

hdl/rtl/hydra_dram.vhd

@@ -37,7 +37,9 @@ entity hydra_dram is
     --  Note: only writes are allowed during reset.
     clk_i            : in  std_logic;
     rst_n_i          : in  std_logic;
-    cpu_rst_n_i      : in  std_logic;
+
+    --  Enable scrubber
+    scrub_en_i       : in  std_logic;
 
     --  Read port
     --  RDONE_O is a pulse.
@@ -47,6 +49,7 @@ entity hydra_dram is
     sel_i       : in  std_logic_vector(3 downto 0);
     data_i      : in  std_logic_vector(31 downto 0);
     data_o      : out std_logic_vector(31 downto 0);
+    eccm_i      : in  std_logic_vector(6 downto 0);
     done_o      : out std_logic;
     err_o       : out std_logic;
 
@@ -128,7 +131,8 @@ begin
   rerr <= f_ecc_errors(rsyndrome) and ren_d;
   rerr_one <= f_ecc_one_error(rsyndrome) and ren_d;
 
-  err_o <= rerr;
+  --  Only single errors are reported, double errors are fatal.
+  err_o <= rerr_one;
   done_o <= r_done or w_done;
 
   p_scrub: process (clk_i)
@@ -139,11 +143,11 @@ begin
         scrub_addr <= (others => '0');
         scrub_rd <= '0';
       else
-        if scrub_done = '1' and rerr = '0' then
+        if scrub_done = '1' and rerr_one = '0' then
           scrub_counter <= unsigned(scrubber_period_i);
           scrub_addr <= std_logic_vector(unsigned(scrub_addr) + 1);
           scrub_rd <= '0';
-        else
+        elsif scrub_en_i = '1' then
           if scrub_counter = (scrub_counter'range => '0') then
             scrub_rd <= '1';
           else
@@ -158,7 +162,7 @@ begin
                    p_done, r_done, scrub_done, rerr,
                    rerr_one, rdata_ecc, rsyndrome, last_addr, last_we, last_sel, recc,
                    en_i, addr_i, we_i, data_i, sel_i, r_done_d,
-                   scrub_rd, scrub_addr, scrub_done_d, cpu_rst_n_i)
+                   scrub_rd, scrub_addr, scrub_done_d)
   is
     variable d : std_logic_vector(31 downto 0);
   begin
@@ -185,23 +189,22 @@ begin
         if (p_done = '1' or r_done = '1' or scrub_done = '1') and rerr = '1' then
           --  There was an error on the last access.
           --  Write to fix it.
+          addr <= last_addr;
+          n_last_addr <= last_addr;
+          n_last_we <= last_we;
+          n_last_sel <= last_sel;
           if rerr_one = '1' then
             --  Correctable: correct it.
             n_scrub_se <= scrub_done;
             n_ecc_one <= p_done or r_done;
             wdata_ecc <= f_fix_error(rsyndrome, rdata_ecc(38 downto 32), rdata_ecc(31 downto 0));
+            wen <= '1';
+            n_state <= S_REWRITE;
           else
-            --  Uncorrectable.  Just recompute the ECC to be able to continue.
+            --  Uncorrectable.  Do not fix it.
             n_scrub_de <= scrub_done;
             n_ecc_fatal <= p_done or r_done;
-            wdata_ecc <= recc & rdata_ecc(31 downto 0);
           end if;
-          addr <= last_addr;
-          wen <= '1';
-          n_last_addr <= last_addr;
-          n_last_we <= last_we;
-          n_last_sel <= last_sel;
-          n_state <= S_REWRITE;
         elsif last_we = '1' then
           assert last_sel /= "1111";
           --  The partial write;
@@ -230,7 +233,7 @@ begin
             if sel_i = "1111" then
               --  Full write
               wen <= '1';
-              wdata_ecc <= f_calc_ecc (data_i) & data_i;
+              wdata_ecc <= (f_calc_ecc (data_i) xor eccm_i) & data_i;
               n_w_done <= '1';
             else
               --  Partial write: first read the word
@@ -244,8 +247,8 @@ begin
             ren <= '1';
             n_r_done <= '1';
           end if;
-        elsif cpu_rst_n_i = '1' then
-          -- scrub if idle (but not during reset)
+        elsif scrub_en_i = '1' then
+          -- scrub if idle
           addr <= scrub_addr;
           ren <= '1';
           n_scrub_done <= '1';

hdl/rtl/hydra_iram.vhd

@@ -94,7 +94,7 @@ architecture arch of hydra_iram is
   signal last_raddr, n_last_raddr, scrub_addr : std_logic_vector(g_RAM_LOG_SIZE - 1 downto 2);
   
   signal scrub_counter : unsigned(15 downto 0);
-  signal scrub_rd, scrub_done, scrub_done_d, n_scrub_done : std_logic;
+  signal scrub_rd, scrub_done, n_scrub_done : std_logic;
   signal n_scrub_se, n_scrub_de : std_logic;
 
   type state_t is (S_READ, S_REWRITE);
@@ -142,9 +142,10 @@ begin
   rerr <= f_ecc_errors(rsyndrome) and ren_d;
   rerr_one <= f_ecc_one_error(rsyndrome) and ren_d;
 
-  r1_err_o <= rerr;
+  --  Note: only single errors are reported, double errors are fatal.
+  r1_err_o <= rerr_one;
   r1_done_o <= r1_done;
-  r2_err_o <= rerr;
+  r2_err_o <= rerr_one;
   r2_done_o <= r2_done;
   wdone_o <= wdone;
 
@@ -156,7 +157,7 @@ begin
         scrub_addr <= (others => '0');
         scrub_rd <= '0';
       else
-        if scrub_done = '1' and rerr = '0' then
+        if n_scrub_done = '1' then
           scrub_counter <= unsigned(scrubber_period_i);
           scrub_addr <= std_logic_vector(unsigned(scrub_addr) + 1);
           scrub_rd <= '0';
@@ -173,7 +174,7 @@ begin
 
   p_ctrl: process (state, r1_done, r2_done, rerr, rerr_one, rdata_ecc, rsyndrome, last_raddr, recc,
                    r1_en_i, r1_addr_i, we_i, waddr_i, wdata_i, wecc_i, r1_done_d, r2_done_d,
-                   r2_en_i, r2_addr_i, scrub_rd, scrub_addr, scrub_done, scrub_done_d, scrub_en_i)
+                   r2_en_i, r2_addr_i, scrub_rd, scrub_addr, scrub_done, scrub_en_i)
   begin
     wen <= '0';
     waddr <= (others => 'X');
@@ -197,21 +198,21 @@ begin
         if (r2_done = '1' or r1_done = '1' or scrub_done = '1') and rerr = '1' then
           --  There was an error on the last access.
           --  Write to fix it.
+          wdata_ecc <= f_fix_error(rsyndrome, rdata_ecc(38 downto 32), rdata_ecc(31 downto 0));
+          waddr <= last_raddr;
+          n_last_raddr <= last_raddr;
           if rerr_one = '1' then
             --  Correctable: correct it.
             n_scrub_se <= scrub_done;
             n_ecc_one <= not scrub_done;
-            wdata_ecc <= f_fix_error(rsyndrome, rdata_ecc(38 downto 32), rdata_ecc(31 downto 0));
+            wen <= '1';
+            n_state <= S_REWRITE;
           else
-            --  Uncorrectable.  Just recompute the ECC to be able to continue.
+            --  Uncorrectable.  Do not fix.  Either it is a fatal error, or it will be when
+            --  accessed by the cpu.  A double-error detected by the scrubber is not fatal.
             n_scrub_de <= scrub_done;
             n_ecc_fatal <= not scrub_done;
-            wdata_ecc <= recc & rdata_ecc(31 downto 0);
           end if;
-          waddr <= last_raddr;
-          wen <= '1';
-          n_last_raddr <= last_raddr;
-          n_state <= S_REWRITE;
         elsif we_i = '1' then
           --  Write
           waddr <= waddr_i;
@@ -251,7 +252,6 @@ begin
         ren <= '1';
         n_r1_done <= r1_done_d;
         n_r2_done <= r2_done_d;
-        n_scrub_done <= scrub_done_d;
         n_last_raddr <= last_raddr;
         n_state <= S_READ;
     end case;
@@ -266,7 +266,6 @@ begin
         r2_done <= '0';
         r2_done_d <= '0';
         scrub_done <= '0';
-        scrub_done_d <= '0';
         state <= S_READ;
         last_raddr <= (others => 'X');
 
@@ -279,7 +278,6 @@ begin
         r1_done <= n_r1_done;
         r2_done_d <= r2_done;
         r2_done <= n_r2_done;
-        scrub_done_d <= scrub_done;
         scrub_done <= n_scrub_done;
 
         state <= n_state;

hdl/rtl/hydra_supervisor_regs.cheby

@@ -107,6 +107,11 @@ memory-map:
       description: Number of CPU errors on instruction bus
       width: 32
       access: ro
+  - reg:
+      name: dram_ecc_mask
+      description: Change ECC bits on dram writes for testing
+      width: 8
+      access: rw
   - reg:
       name: scrub_cfg
       description: configuration of scrubbers

hdl/rtl/hydra_supervisor_regs.vhd

@@ -71,6 +71,9 @@ entity hydra_supervisor_regs is
     -- Number of CPU errors on instruction bus
     cpu_iaddr_err_i      : in    std_logic_vector(31 downto 0);
 
+    -- Change ECC bits on dram writes for testing
+    dram_ecc_mask_o      : out   std_logic_vector(7 downto 0);
+
     -- configuration of scrubbers
     -- Write 1 to enable iram scrubber
     scrub_cfg_iram_en_i  : in    std_logic;
@@ -103,6 +106,9 @@ architecture syn of hydra_supervisor_regs is
   signal wd_key_reg                     : std_logic_vector(31 downto 0);
   signal wd_key_wreq                    : std_logic;
   signal wd_key_wack                    : std_logic;
+  signal dram_ecc_mask_reg              : std_logic_vector(7 downto 0);
+  signal dram_ecc_mask_wreq             : std_logic;
+  signal dram_ecc_mask_wack             : std_logic;
   signal scrub_cfg_wreq                 : std_logic;
   signal iram_scrub_period_reg          : std_logic_vector(15 downto 0);
   signal iram_scrub_period_wreq         : std_logic;
@@ -213,6 +219,22 @@ begin
 
   -- Register cpu_iaddr_err
 
+  -- Register dram_ecc_mask
+  dram_ecc_mask_o <= dram_ecc_mask_reg;
+  process (clk_i) begin
+    if rising_edge(clk_i) then
+      if rst_n_i = '0' then
+        dram_ecc_mask_reg <= "00000000";
+        dram_ecc_mask_wack <= '0';
+      else
+        if dram_ecc_mask_wreq = '1' then
+          dram_ecc_mask_reg <= wr_dat_d0(7 downto 0);
+        end if;
+        dram_ecc_mask_wack <= dram_ecc_mask_wreq;
+      end if;
+    end if;
+  end process;
+
   -- Register scrub_cfg
   scrub_cfg_iram_en_o <= wr_dat_d0(0);
   scrub_cfg_dram_en_o <= wr_dat_d0(1);
@@ -251,11 +273,12 @@ begin
   end process;
 
   -- Process for write requests.
-  process (wr_adr_d0, wr_req_d0, wd_key_wack, iram_scrub_period_wack,
-           dram_scrub_period_wack) begin
+  process (wr_adr_d0, wr_req_d0, wd_key_wack, dram_ecc_mask_wack,
+           iram_scrub_period_wack, dram_scrub_period_wack) begin
     cpu_wreq <= '0';
     wd_period_wreq <= '0';
     wd_key_wreq <= '0';
+    dram_ecc_mask_wreq <= '0';
     scrub_cfg_wreq <= '0';
     iram_scrub_period_wreq <= '0';
     dram_scrub_period_wreq <= '0';
@@ -306,14 +329,18 @@ begin
       -- Reg cpu_iaddr_err
       wr_ack_int <= wr_req_d0;
     when "01110" =>
+      -- Reg dram_ecc_mask
+      dram_ecc_mask_wreq <= wr_req_d0;
+      wr_ack_int <= dram_ecc_mask_wack;
+    when "01111" =>
       -- Reg scrub_cfg
       scrub_cfg_wreq <= wr_req_d0;
       wr_ack_int <= wr_req_d0;
-    when "01111" =>
+    when "10000" =>
       -- Reg iram_scrub_period
       iram_scrub_period_wreq <= wr_req_d0;
       wr_ack_int <= iram_scrub_period_wack;
-    when "10000" =>
+    when "10001" =>
       -- Reg dram_scrub_period
       dram_scrub_period_wreq <= wr_req_d0;
       wr_ack_int <= dram_scrub_period_wack;
@@ -327,8 +354,9 @@ begin
            reset_cause_watchdog_i, cpu_reset_i, cpu_recovery_i,
            force_divergence_i, wd_period_i, wd_count_i, iram_ecc_se_i,
            iram_scrub_se_i, iram_scrub_de_i, dram_ecc_se_i, dram_scrub_se_i,
-           dram_scrub_de_i, cpu_data_err_i, cpu_iaddr_err_i, scrub_cfg_iram_en_i,
-           scrub_cfg_dram_en_i, iram_scrub_period_reg, dram_scrub_period_reg) begin
+           dram_scrub_de_i, cpu_data_err_i, cpu_iaddr_err_i, dram_ecc_mask_reg,
+           scrub_cfg_iram_en_i, scrub_cfg_dram_en_i, iram_scrub_period_reg,
+           dram_scrub_period_reg) begin
     -- By default ack read requests
     rd_dat_d0 <= (others => 'X');
     force_divergence_rd_o <= '0';
@@ -396,17 +424,22 @@ begin
       rd_ack_d0 <= rd_req_int;
       rd_dat_d0 <= cpu_iaddr_err_i;
     when "01110" =>
+      -- Reg dram_ecc_mask
+      rd_ack_d0 <= rd_req_int;
+      rd_dat_d0(7 downto 0) <= dram_ecc_mask_reg;
+      rd_dat_d0(31 downto 8) <= (others => '0');
+    when "01111" =>
       -- Reg scrub_cfg
       rd_ack_d0 <= rd_req_int;
       rd_dat_d0(0) <= scrub_cfg_iram_en_i;
       rd_dat_d0(1) <= scrub_cfg_dram_en_i;
       rd_dat_d0(31 downto 2) <= (others => '0');
-    when "01111" =>
+    when "10000" =>
       -- Reg iram_scrub_period
       rd_ack_d0 <= rd_req_int;
       rd_dat_d0(15 downto 0) <= iram_scrub_period_reg;
       rd_dat_d0(31 downto 16) <= (others => '0');
-    when "10000" =>
+    when "10001" =>
       -- Reg dram_scrub_period
       rd_ack_d0 <= rd_req_int;
       rd_dat_d0(15 downto 0) <= dram_scrub_period_reg;

sw/include/hydra_supervisor_regs.h

 #ifndef __CHEBY__HYDRA_SUPERVISOR_REGS__H__
 #define __CHEBY__HYDRA_SUPERVISOR_REGS__H__
-#define HYDRA_SUPERVISOR_REGS_SIZE 66 /* 0x42 */
+#define HYDRA_SUPERVISOR_REGS_SIZE 70 /* 0x46 */
 
 /* Cause of a reset */
 #define HYDRA_SUPERVISOR_REGS_RESET_CAUSE 0x0UL
@@ -50,17 +50,20 @@
 /* Number of CPU errors on instruction bus */
 #define HYDRA_SUPERVISOR_REGS_CPU_IADDR_ERR 0x34UL
 
+/* Change ECC bits on dram writes for testing */
+#define HYDRA_SUPERVISOR_REGS_DRAM_ECC_MASK 0x38UL
+
 /* configuration of scrubbers */
-#define HYDRA_SUPERVISOR_REGS_SCRUB_CFG 0x38UL
+#define HYDRA_SUPERVISOR_REGS_SCRUB_CFG 0x3cUL
 #define HYDRA_SUPERVISOR_REGS_SCRUB_CFG_IRAM_EN 0x1UL
 #define HYDRA_SUPERVISOR_REGS_SCRUB_CFG_DRAM_EN 0x2UL
 
 /* Maximum number of cycles between a scrub */
-#define HYDRA_SUPERVISOR_REGS_IRAM_SCRUB_PERIOD 0x3cUL
+#define HYDRA_SUPERVISOR_REGS_IRAM_SCRUB_PERIOD 0x40UL
 #define HYDRA_SUPERVISOR_REGS_IRAM_SCRUB_PERIOD_PRESET 0x800UL
 
 /* Maximum number of cycles between a scrub */
-#define HYDRA_SUPERVISOR_REGS_DRAM_SCRUB_PERIOD 0x40UL
+#define HYDRA_SUPERVISOR_REGS_DRAM_SCRUB_PERIOD 0x44UL
 #define HYDRA_SUPERVISOR_REGS_DRAM_SCRUB_PERIOD_PRESET 0x800UL
 
 struct hydra_supervisor_regs {
@@ -106,16 +109,22 @@ struct hydra_supervisor_regs {
   /* [0x34]: REG (ro) Number of CPU errors on instruction bus */
   uint32_t cpu_iaddr_err;
 
-  /* [0x38]: REG (rw) configuration of scrubbers */
+  /* [0x38]: REG (rw) Change ECC bits on dram writes for testing */
+  uint8_t dram_ecc_mask;
+
+  /* padding to: 60 words */
+  uint8_t __padding_0[3];
+
+  /* [0x3c]: REG (rw) configuration of scrubbers */
   uint32_t scrub_cfg;
 
-  /* [0x3c]: REG (rw) Maximum number of cycles between a scrub */
+  /* [0x40]: REG (rw) Maximum number of cycles between a scrub */
   uint16_t iram_scrub_period;
 
-  /* padding to: 64 words */
-  uint8_t __padding_0[2];
+  /* padding to: 68 words */
+  uint8_t __padding_1[2];
 
-  /* [0x40]: REG (rw) Maximum number of cycles between a scrub */
+  /* [0x44]: REG (rw) Maximum number of cycles between a scrub */
   uint16_t dram_scrub_period;
 };
 

sw/sf2-test/main.c

@@ -18,6 +18,8 @@ extern void clear_bss(void);
 
 static enum t_test
   {
+   TEST_DRAM_DE,
+   TEST_DRAM_SE,
    TEST_IRAM_DE,
    TEST_IRAM_SE,
    TEST_IRAM_SCRUB,
@@ -91,6 +93,8 @@ uart_puts (const char *s)
     uart_putc (*s++);
 }
 
+static volatile unsigned ecc[4];
+
 #define PAD_LEN 8
 static volatile unsigned pad[8];
 
@@ -159,17 +163,39 @@ main (void)
   uart_putc('\n');
 
   switch (next_test) {
+  case TEST_DRAM_DE:
+    SUPERVISOR->dram_ecc_mask = 0x03;
+    ecc[0] = 12;
+    ecc[2] = 23;
+    SUPERVISOR->dram_ecc_mask = 0x01;
+    ecc[1] = 56;
+    ecc[3] = 78;
+    SUPERVISOR->dram_ecc_mask = 0x00;
+    next_test = TEST_DRAM_SE;
+    /* Order is important! */
+    asm volatile ("");
+    v = ecc[0];
+    unreachable();
+    break;
+
+  case TEST_DRAM_SE:
+    if (SUPERVISOR->dram_ecc_se != 0)
+      unreachable();
+    v = ecc[1];
+    if (SUPERVISOR->dram_ecc_se != 1)
+      unreachable();
+    next_test = TEST_IRAM_DE;
+    /* Fallthrough */
+
   case TEST_IRAM_DE:
     next_test = TEST_IRAM_SE;
+    /* Generate a fatal error */
     v = *(volatile unsigned *)0xfffc;
     unreachable();
     break;
 
   case TEST_IRAM_SE:
-    /* Must have been corrected.  */
-    v = *(volatile unsigned *)0xfffc;
-    if (SUPERVISOR->iram_ecc_se != 0)
-      unreachable();
+    /* Generate a single error */
     v = *(volatile unsigned *)0xfff8;
     if (SUPERVISOR->iram_ecc_se != 1)
       unreachable();
@@ -182,13 +208,19 @@ main (void)
       unreachable();
     uart_puts("Start IRAM scrubber");
     SUPERVISOR->scrub_cfg = HYDRA_SUPERVISOR_REGS_SCRUB_CFG_IRAM_EN;
+    next_test = TEST_WD;
     while (SUPERVISOR->iram_scrub_de == 0)
-      uart_putc('.');
+      SUPERVISOR->wd_key = WD_KEY;
     uart_putc('\n');
     if (SUPERVISOR->iram_scrub_se != 1
-        || SUPERVISOR->iram_scrub_de != 1)
+        || SUPERVISOR->iram_scrub_de != 2) {
+      uart_puts ("se:");
+      uart_put_hex8 (SUPERVISOR->iram_scrub_se);
+      uart_puts (", de:");
+      uart_put_hex8 (SUPERVISOR->iram_scrub_de);
+      while (1) ;
       unreachable();
-    next_test = TEST_WD;
+    }
     /* Fallthrough */
 
   case TEST_WD: