diff --git a/docs/ip/spi-verification.md b/docs/ip/spi-verification.md index 2ca6dd5c..006df976 100644 --- a/docs/ip/spi-verification.md +++ b/docs/ip/spi-verification.md @@ -220,6 +220,13 @@ this pre-dates SPI-P3 and is non-blocking inside the regression runner. behavioral defect remains routed to the R2 DV owners; `phase_accepted` stays false. +Icarus defect resolution (2026-10-22, later commit): the routed dv observer +defect is fixed on top of this worktree with semantics preserved; the R2-P7 +Icarus behavioral acceptance passes with `SIM_TEST_PASS Tiny cycles=3859487` +(`build/ihp130-tiny-2026-10-22-12-16-aae2707c2aa5/sim/iverilog/`), matching +the Verilator cycle count exactly. Details are recorded in +[tiny-soc-verification.md](tiny-soc-verification.md#tiny-icarus-behavioral-repair-2026-10-22). + SPI-P3 review-repair round (2026-10-22, same worktree): independent review of the full worktree delta found three P1 defects, all fixed and re-verified. (1) `dma_axi4_master` sampled the origin channel from the live arbiter during @@ -255,11 +262,12 @@ Mini rtl-lint passes; Tiny Yosys synthesis, `netsim-boot` (`SIM_TEST_PASS Tiny cycles=36069`) and OpenSTA all pass under `build/ihp130-tiny-2026-10-22-10-31-8232fa11e7f3/`; `rtl-format-check`, `ruff check .`, `sw-format-check`, `sw-policy-check`, `sw-host-test` and -`git diff --check` all pass. Remaining gaps are unchanged: the pre-existing -Icarus observer/sv2v defect (R2 DV owners), the Mini smoke-synthesis host +`git diff --check` all pass. Remaining gaps are unchanged: the Mini smoke-synthesis host timeout (skipped per user decision), the missing Tiny warning baseline (non-blocking observation), human phase acceptance, and the deferred -SPI-P4/P5 obligations. `phase_accepted` stays false. +SPI-P4/P5 obligations. The pre-existing Icarus observer/sv2v defect listed +above was subsequently fixed (see the resolution note in the Tiny PR matrix +paragraph and `tiny-soc-verification.md`); `phase_accepted` stays false. ## SPI-P1 implementation checkpoint (2026-10-18, historical) diff --git a/docs/ip/tiny-soc-verification.md b/docs/ip/tiny-soc-verification.md index 8f92916c..67fb7823 100644 --- a/docs/ip/tiny-soc-verification.md +++ b/docs/ip/tiny-soc-verification.md @@ -1,5 +1,40 @@ # Tiny MCU Verification Record +## Tiny Icarus behavioral repair (2026-10-22) + +The Tiny Icarus behavioral flow failed elaboration with 29 errors on both +profiles since the committed R2-P7..P10 observer changes (evidence: +`build/ics55-tiny-2026-10-21-16-31-a257f7b03bdb/sim/iverilog/` and +`build/ihp130-tiny-2026-10-21-16-36-76f15177ba7a/sim/iverilog/`; CI runs +39719773195 and 39719773217 failed at the same step). Root cause: the Icarus +flow compiles only the sv2v-lowered `converted_soc.v`, where packed structs +become plain vectors and interface ports dissolve, while the dv observers read +struct fields hierarchically and `tiny_p9_recovery.svh` used a string `case` +that crashes Icarus code generation (`eval_vec4` assertion). + +The repair keeps every observer semantically identical: +`rtl/tiny/dv/tiny_p8_observers.svh` moves a `time` declaration above its +first use; `rtl/tiny/dv/tiny_p9_observers.svh` reads the identical packed bit +ranges through named slice constants (valid in both the Verilator struct view +and the sv2v vector view) and reads the XPI APB wires through the top-level +interface instance `u_xpi_apb4_if` (the same nets); `tiny_p9_recovery.svh` +rewrites the string `case` as an exact-match if/else chain; +`rtl/tiny/dv/retrosoc_tiny_tb.sv` uses the same slice constants. No DUT, +Mini, baseline, or verdict-rule change. + +Evidence: Icarus compile is clean on both profiles; the R2-P7 Icarus +behavioral acceptance now passes with `SIM_TEST_PASS Tiny cycles=3859487`, +flow passed, check passed, no forbidden markers +(`build/ihp130-tiny-2026-10-22-12-16-aae2707c2aa5/sim/iverilog/`), matching +the Verilator cycle count exactly. Verilator results are unchanged after the +rework: SPI-P3 acceptance `cycles=1030088` on both profiles +(`build/ics55-tiny-2026-10-22-12-17-a257f7b03bdb/`, +`build/ihp130-tiny-2026-10-22-12-17-76f15177ba7a/`), R2-P7 Verilator +`cycles=3859487` (`build/ihp130-tiny-2026-10-22-12-17-3f1b609b0b86/`), and +the `+tiny_p9_progress` observer output is byte-identical to the pre-repair +format. Full Pytest after the rework: 2097 passed, 0 failed, 1 skipped +(optional `pypdf`) in 4042.64 s. Formatting gates pass. + ## TINY-R2-P10 quality-gate repair (2026-10-18) The approved repair starts from `dev@5b81dea4fe0a68aabb51f42f01422068280192b2` diff --git a/rtl/tiny/dv/retrosoc_tiny_tb.sv b/rtl/tiny/dv/retrosoc_tiny_tb.sv index b3225e29..f54a4227 100644 --- a/rtl/tiny/dv/retrosoc_tiny_tb.sv +++ b/rtl/tiny/dv/retrosoc_tiny_tb.sv @@ -135,11 +135,12 @@ module retrosoc_tiny_tb; u_dut.u_soc.s_boot_release_q, u_dut.u_soc.s_fabric_rst_n, u_dut.u_soc.s_dma_ready_sys, &u_dut.u_soc.s_apb_cdc_ready, u_dut.u_soc.u_cpu_ahbl_if[0].htrans[1], u_dut.u_soc.u_cpu_ahbl_if[0].hready, u_dut.u_soc.u_cpu_ahbl_if[1].htrans[1], - u_dut.u_soc.u_cpu_ahbl_if[1].hready, u_dut.u_soc.u_cpu_mem.s_req_q[0].state, - u_dut.u_soc.u_cpu_mem.s_slow_q.state, u_dut.u_soc.u_cpu_mem.u_slow_ahbl_if.htrans, - u_dut.u_soc.u_cpu_mem.u_slow_ahbl_if.hready, u_dut.u_soc.u_xpi_mem_axi4_if.arvalid, - u_dut.u_soc.u_xpi_mem_axi4_if.arready, u_dut.u_soc.u_xpi_mem_axi4_if.rvalid, - u_dut.u_soc.u_xpi_mem_axi4_if.rready); + u_dut.u_soc.u_cpu_ahbl_if[1].hready, + u_dut.u_soc.u_cpu_mem.s_req_q[0][P9_REQ_STATE_HI:P9_REQ_STATE_LO], + u_dut.u_soc.u_cpu_mem.s_slow_q[P9_SLOW_STATE_HI:P9_SLOW_STATE_LO], + u_dut.u_soc.u_cpu_mem.u_slow_ahbl_if.htrans, u_dut.u_soc.u_cpu_mem.u_slow_ahbl_if.hready, + u_dut.u_soc.u_xpi_mem_axi4_if.arvalid, u_dut.u_soc.u_xpi_mem_axi4_if.arready, + u_dut.u_soc.u_xpi_mem_axi4_if.rvalid, u_dut.u_soc.u_xpi_mem_axi4_if.rready); end end diff --git a/rtl/tiny/dv/tiny_p8_observers.svh b/rtl/tiny/dv/tiny_p8_observers.svh index 1f0668fe..f200e7cb 100644 --- a/rtl/tiny/dv/tiny_p8_observers.svh +++ b/rtl/tiny/dv/tiny_p8_observers.svh @@ -7,7 +7,8 @@ logic s_p8_finished = 0; longint unsigned s_p8_cycles = 0, s_p8_write_bytes = 0, s_p8_read_bytes = 0; longint unsigned s_p8_stall_run[5], s_p8_stall_max[5]; longint unsigned s_p8_all_writes = 0; -integer s_p8_window = 0; +integer s_p8_window = 0; +time s_p8_cs_start, s_p8_cs_max = 0; wire [4:0] s_p8_stalls = { `_P8_AXI.awvalid && !`_P8_AXI.awready, `_P8_AXI.wvalid && !`_P8_AXI.wready, @@ -55,7 +56,6 @@ always @(posedge u_dut.u_soc.s_backend_mem_clk) begin end `undef _P8_AXI -time s_p8_cs_start, s_p8_cs_max = 0; always @(negedge s_psram_csn) s_p8_cs_start = $time; always @(posedge s_psram_csn) begin if (($test$plusargs("tiny_psram") || $test$plusargs("tiny_p8_psram")) && s_p8_cs_start != 0) begin diff --git a/rtl/tiny/dv/tiny_p9_observers.svh b/rtl/tiny/dv/tiny_p9_observers.svh index 41c0994e..60942efd 100644 --- a/rtl/tiny/dv/tiny_p9_observers.svh +++ b/rtl/tiny/dv/tiny_p9_observers.svh @@ -11,6 +11,24 @@ integer p9_ifetch_completions = 0; integer p9_xpi_completions = 0; integer p9_last_progress = 0; bit p9_reported = 0; +// sv2v lowers packed structs to plain vectors with identical bit layout, so +// the Icarus flow sees only the vector form of these DUT structs. Slice the +// packed views directly; identical bit ranges work in the Verilator view. +localparam int P9_REQ_STATE_LO = 73; // tiny_cpu_mem request_t state[2:0] +localparam int P9_REQ_STATE_HI = 75; +localparam int P9_REQ_ADDR_LO = 41; // request_t addr[31:0] +localparam int P9_REQ_ADDR_HI = 72; +localparam int P9_REQ_DATA_LO = 9; // request_t data[31:0] +localparam int P9_REQ_DATA_HI = 40; +localparam int P9_REQ_WRITE = 5; // request_t write +localparam int P9_SLOW_STATE_LO = 102; // tiny_cpu_mem slow_t state[1:0] +localparam int P9_SLOW_STATE_HI = 103; +localparam int P9_SLOW_OWNER = 101; // slow_t owner +localparam int P9_SLOW_WRITE = 100; // slow_t write +localparam int P9_SLOW_ADDR_LO = 64; // slow_t addr[31:0] +localparam int P9_SLOW_ADDR_HI = 95; +localparam int P9_CDC_STATE_LO = 110; // tiny_apb_async_bridge src_t/dst_t state[1:0] +localparam int P9_CDC_STATE_HI = 111; `include "tiny_p9_monitor.svh" always @(posedge u_dut.u_soc.u_dvp.s_pclk_sel) begin if ($test$plusargs( @@ -138,7 +156,7 @@ always @(posedge u_dut.u_soc.s_backend_mem_clk) begin if (!u_dut.u_soc.s_xpi_rst_n) begin p9_diag_xpi_apb_pending = 0; p9_diag_xpi_apb_wait = 0; - end else if (u_dut.u_soc.u_xpi.apb4.psel && u_dut.u_soc.u_xpi.apb4.penable) begin + end else if (u_dut.u_soc.u_xpi_apb4_if.psel && u_dut.u_soc.u_xpi_apb4_if.penable) begin if (!p9_diag_xpi_apb_pending) begin p9_diag_xpi_apb_pending = 1; p9_diag_xpi_apb_requests++; @@ -150,13 +168,13 @@ always @(posedge u_dut.u_soc.s_backend_mem_clk) begin $realtime, p9_diag_ref_cycles, p9_diag_xpi_apb_requests, - u_dut.u_soc.u_xpi.apb4.paddr, - u_dut.u_soc.u_xpi.apb4.pwrite, - u_dut.u_soc.u_xpi.apb4.pwdata, - u_dut.u_soc.u_xpi.apb4.pstrb + u_dut.u_soc.u_xpi_apb4_if.paddr, + u_dut.u_soc.u_xpi_apb4_if.pwrite, + u_dut.u_soc.u_xpi_apb4_if.pwdata, + u_dut.u_soc.u_xpi_apb4_if.pstrb ); end - if (u_dut.u_soc.u_xpi.apb4.pready) begin + if (u_dut.u_soc.u_xpi_apb4_if.pready) begin p9_diag_xpi_apb_responses++; if (p9_diag_camera_started) $fdisplay( @@ -166,8 +184,8 @@ always @(posedge u_dut.u_soc.s_backend_mem_clk) begin p9_diag_ref_cycles, p9_diag_xpi_apb_requests, p9_diag_xpi_apb_wait, - u_dut.u_soc.u_xpi.apb4.pslverr, - u_dut.u_soc.u_xpi.apb4.prdata + u_dut.u_soc.u_xpi_apb4_if.pslverr, + u_dut.u_soc.u_xpi_apb4_if.prdata ); p9_diag_xpi_apb_pending = 0; p9_diag_xpi_apb_wait = 0; @@ -231,9 +249,10 @@ for (genvar port_index = 0; port_index < 2; port_index++) begin : gen_p9_diag_cp // an address and must not be mislabeled as instruction retirement. if (u_dut.u_soc.u_cpu_mem.s_terminal[port_index]) begin responses++; - last_response_addr = u_dut.u_soc.u_cpu_mem.s_req_q[port_index].addr; - wait_cycles = 0; - end else if (u_dut.u_soc.u_cpu_mem.s_req_q[port_index].state != 0) wait_cycles++; + last_response_addr = u_dut.u_soc.u_cpu_mem.s_req_q[port_index][P9_REQ_ADDR_HI:P9_REQ_ADDR_LO]; + wait_cycles = 0; + end else if (u_dut.u_soc.u_cpu_mem.s_req_q[port_index][P9_REQ_STATE_HI:P9_REQ_STATE_LO] != 0) + wait_cycles++; else wait_cycles = 0; end else wait_cycles = 0; end @@ -246,10 +265,10 @@ for (genvar port_index = 0; port_index < 2; port_index++) begin : gen_p9_diag_cp u_dut.u_soc.u_cpu_ahbl_if[port_index].haddr, u_dut.u_soc.u_cpu_ahbl_if[port_index].hwrite, u_dut.u_soc.u_cpu_ahbl_if[port_index].htrans, u_dut.u_soc.u_cpu_ahbl_if[port_index].hready, u_dut.u_soc.u_cpu_ahbl_if[port_index].hresp, - u_dut.u_soc.u_cpu_mem.s_req_q[port_index].state, - u_dut.u_soc.u_cpu_mem.s_req_q[port_index].addr, - u_dut.u_soc.u_cpu_mem.s_req_q[port_index].data, - u_dut.u_soc.u_cpu_mem.s_req_q[port_index].write, last_response_addr); + u_dut.u_soc.u_cpu_mem.s_req_q[port_index][P9_REQ_STATE_HI:P9_REQ_STATE_LO], + u_dut.u_soc.u_cpu_mem.s_req_q[port_index][P9_REQ_ADDR_HI:P9_REQ_ADDR_LO], + u_dut.u_soc.u_cpu_mem.s_req_q[port_index][P9_REQ_DATA_HI:P9_REQ_DATA_LO], + u_dut.u_soc.u_cpu_mem.s_req_q[port_index][P9_REQ_WRITE], last_response_addr); $fflush(p9_diag_file); end end @@ -337,11 +356,12 @@ always @(negedge s_clk) begin $fdisplay( p9_diag_file, "P9_DIAG_SLOW time_ns=%0.3f ref=%0d owner=%b state=%0d addr=%h write=%b ahb_trans=%b ahb_ready=%b ahb_resp=%b aw=%b/%b w=%b/%b b=%b/%b bresp=%b ar=%b/%b r=%b/%b rresp=%b", - $realtime, p9_diag_ref_cycles, u_dut.u_soc.u_cpu_mem.s_slow_q.owner, - u_dut.u_soc.u_cpu_mem.s_slow_q.state, u_dut.u_soc.u_cpu_mem.s_slow_q.addr, - u_dut.u_soc.u_cpu_mem.s_slow_q.write, u_dut.u_soc.u_cpu_mem.u_slow_ahbl_if.htrans, - u_dut.u_soc.u_cpu_mem.u_slow_ahbl_if.hready, u_dut.u_soc.u_cpu_mem.u_slow_ahbl_if.hresp, - u_dut.u_soc.u_cpu_mem.u_slow_axi4_if.awvalid, + $realtime, p9_diag_ref_cycles, u_dut.u_soc.u_cpu_mem.s_slow_q[P9_SLOW_OWNER], + u_dut.u_soc.u_cpu_mem.s_slow_q[P9_SLOW_STATE_HI:P9_SLOW_STATE_LO], + u_dut.u_soc.u_cpu_mem.s_slow_q[P9_SLOW_ADDR_HI:P9_SLOW_ADDR_LO], + u_dut.u_soc.u_cpu_mem.s_slow_q[P9_SLOW_WRITE], + u_dut.u_soc.u_cpu_mem.u_slow_ahbl_if.htrans, u_dut.u_soc.u_cpu_mem.u_slow_ahbl_if.hready, + u_dut.u_soc.u_cpu_mem.u_slow_ahbl_if.hresp, u_dut.u_soc.u_cpu_mem.u_slow_axi4_if.awvalid, u_dut.u_soc.u_cpu_mem.u_slow_axi4_if.awready, u_dut.u_soc.u_cpu_mem.u_slow_axi4_if.wvalid, u_dut.u_soc.u_cpu_mem.u_slow_axi4_if.wready, u_dut.u_soc.u_cpu_mem.u_slow_axi4_if.bvalid, u_dut.u_soc.u_cpu_mem.u_slow_axi4_if.bready, @@ -351,10 +371,13 @@ always @(negedge s_clk) begin $fdisplay( p9_diag_file, "P9_DIAG_CDC time_ns=%0.3f ref=%0d gpio_src=%0d gpio_dst=%0d gpio_link=%b/%b gpio_ready=%b uart_src=%0d uart_dst=%0d uart_link=%b/%b uart_ready=%b", - $realtime, p9_diag_ref_cycles, u_dut.u_soc.u_gpio_cfg_cdc.s_src_q.state, - u_dut.u_soc.u_gpio_cfg_cdc.s_dst_q.state, u_dut.u_soc.u_gpio_cfg_cdc.s_link_src_rst_n, - u_dut.u_soc.u_gpio_cfg_cdc.s_link_dst_rst_n, u_dut.u_soc.s_apb_cdc_ready[0], - u_dut.u_soc.u_uart0_cfg_cdc.s_src_q.state, u_dut.u_soc.u_uart0_cfg_cdc.s_dst_q.state, + $realtime, p9_diag_ref_cycles, + u_dut.u_soc.u_gpio_cfg_cdc.s_src_q[P9_CDC_STATE_HI:P9_CDC_STATE_LO], + u_dut.u_soc.u_gpio_cfg_cdc.s_dst_q[P9_CDC_STATE_HI:P9_CDC_STATE_LO], + u_dut.u_soc.u_gpio_cfg_cdc.s_link_src_rst_n, u_dut.u_soc.u_gpio_cfg_cdc.s_link_dst_rst_n, + u_dut.u_soc.s_apb_cdc_ready[0], + u_dut.u_soc.u_uart0_cfg_cdc.s_src_q[P9_CDC_STATE_HI:P9_CDC_STATE_LO], + u_dut.u_soc.u_uart0_cfg_cdc.s_dst_q[P9_CDC_STATE_HI:P9_CDC_STATE_LO], u_dut.u_soc.u_uart0_cfg_cdc.s_link_src_rst_n, u_dut.u_soc.u_uart0_cfg_cdc.s_link_dst_rst_n, u_dut.u_soc.s_apb_cdc_ready[1]); $fdisplay( @@ -393,7 +416,7 @@ always @(negedge s_clk) begin u_dut.u_soc.u_sram.gen_group[0].u_group.s_read_bank_q, u_dut.u_soc.u_sram.gen_group[0].u_group.s_bank_select[2], u_dut.u_soc.u_sram.gen_group[0].u_group.s_rsp_valid_q[0], - u_dut.u_soc.u_cpu_mem.s_req_q[0].state == 3, + u_dut.u_soc.u_cpu_mem.s_req_q[0][P9_REQ_STATE_HI:P9_REQ_STATE_LO] == 3, u_dut.u_soc.u_sram.gen_group[0].u_group.s_rsp_data_q[0], u_dut.u_soc.u_sram.gen_group[0].u_group.s_bank_data[2]); end diff --git a/rtl/tiny/dv/tiny_p9_recovery.svh b/rtl/tiny/dv/tiny_p9_recovery.svh index 37f678d7..523f07ca 100644 --- a/rtl/tiny/dv/tiny_p9_recovery.svh +++ b/rtl/tiny/dv/tiny_p9_recovery.svh @@ -17,30 +17,22 @@ always @(posedge u_dut.u_soc.s_uart0_clk) begin ) && u_dut.u_soc.s_uart0_rst_n && u_dut.u_soc.u_uart0.u_uart_reg.s_tx_push && !u_dut.u_soc.u_uart0.s_loopback) begin if (u_dut.u_soc.u_uart0_apb4_if.pwdata[7:0] == 8'd10) begin - case (s_p9_recovery_line) - "P9_CTRL camera": begin - s_p9_camera_population = 1; - s_p9_recovery_controls++; - end - "P9_CTRL audio": begin - s_p9_camera_population = 0; - s_p9_recovery_controls++; - end - "P9_CTRL stop_low": begin - u_dvp_camera.recovery_pause = 1; - s_p9_recovery_controls++; - end - "P9_CTRL stop_high": begin - u_dvp_camera.recovery_pause = 2; - s_p9_recovery_controls++; - end - "P9_CTRL resume": begin - u_dvp_camera.recovery_pause = 0; - s_p9_recovery_controls++; - end - default: begin - end - endcase + if (s_p9_recovery_line == "P9_CTRL camera") begin + s_p9_camera_population = 1; + s_p9_recovery_controls++; + end else if (s_p9_recovery_line == "P9_CTRL audio") begin + s_p9_camera_population = 0; + s_p9_recovery_controls++; + end else if (s_p9_recovery_line == "P9_CTRL stop_low") begin + u_dvp_camera.recovery_pause = 1; + s_p9_recovery_controls++; + end else if (s_p9_recovery_line == "P9_CTRL stop_high") begin + u_dvp_camera.recovery_pause = 2; + s_p9_recovery_controls++; + end else if (s_p9_recovery_line == "P9_CTRL resume") begin + u_dvp_camera.recovery_pause = 0; + s_p9_recovery_controls++; + end s_p9_recovery_line = ""; end else if (u_dut.u_soc.u_uart0_apb4_if.pwdata[7:0] != 8'd13) begin if (s_p9_recovery_line.len() >= 256) $fatal(1, "SIM_TEST_FAIL recovery UART record overflow");