[rtl] Read only one way on a multi-way icache hit

[kulan-pal]

Without the RTL change in this PR, the new ecc_back_line test fails on most of the tested seeds:

UVM_ERROR @  83410000 ps: (ibex_icache_ram_if.sv:113) [ASSERT FAILED] DataErrChk_A

The icache can hold the same line in two ways when the core branches into a line that is still being fetched. A
lookup that hits both ways ORs their data before the ECC check. A clean copy can then hide corruption in the other
copy. Two corrupted copies can also merge into a valid codeword for a wrong instruction, which is returned without
an error.

This PR reads only the lowest matching way, so the ECC check sees one copy. A data error still invalidates every
matching way. Data in the other ways is not checked, as already happens for ways that miss.

I saw no material area change in a Yosys synthesis of ibex_core with the opentitan config. The added selection
logic was off the critical path in that run, which excludes the register file and the lockstep copy.

The testbench now requires an error when the selected way holds injected data corruption. The new sequence
exercises this on duplicate hits. The VCS coverage config now includes the RAM interface's assertions. A separate
commit drops a redundant always from two valid-bit checks there, so their passes are counted.

Depends on #2515. The first two commits are from that PR.

I ran all icache tests through dvsim on VCS U-2023.03-SP2 with the default testbench parameters and random seeds.
All passed except one stress_all_with_reset run, which hit the reset race fixed in #2516. The new test failed on
17 of 20 seeds without the RTL change and passed on all 20 with it. Lint passed on the six CI configs. I did not
run Xcelium.

AI disclosure (CLA §9): written with help from Claude Code and reviewed with OpenAI Codex; I have reviewed and understood every change and take full responsibility for it.

[bonnie-banks123]

The RTL fix looks correct; restoring the old RTL makes the new test fail as expected. Please also include tb.ram_if in assertion coverage. The default filter excludes DupWayDataErr_C, so the new coverage point is absent from regression reports. Including it records 30 hits at seed 1.

[nasahlpa]

Thanks for the change, @kulan-pal!

I focused on the RTL change, which LGTM! Once the review from @bonnie-banks123 is addressed, we can merge this.

[kulan-pal]

Thanks, the VCS report now includes tb.ram_if and records 30 DupWayDataErr_C matches for ecc_back_line at seed 1. A separate commit drops a redundant always from two valid-bit checks there, so their passes are counted.

[andreaskurth]

Thanks @kulan-pal! Reading a single way is a clean fix, and the new sequence exercises the duplicate-way case well. I only have minor DV suggestions; the RTL looks good to me. Details inline.

[andreaskurth]

With the RTL change, the DUT can no longer flag an error for corrupted data in a matching way that it doesn't read. Should ecc_err_allowed use read_way instead of tag_match, so that NoUnexpectedErrChk_A enforces this? A return to merging the ways would then also fail when the corruption is only in the way that isn't read.

Suggested change

	// The DUT flags an ECC error on a valid lookup that reads a corrupted tag in any way, or corrupted
	// data in the way it reads from: the lowest way that hit. An error for a corrupted copy in another
	// matching way is allowed but not required.
	logic [IC_NUM_WAYS-1:0] tag_err_inj, data_err_inj, read_way;
	logic ecc_err_allowed;
	assign tag_err_inj = (enable_ecc_errors && (& ic_tag_rvalid)) ? tag_sel_line : '0;
	assign data_err_inj = (enable_ecc_errors && (& ic_data_rvalid)) ? data_sel_line : '0;
	assign read_way = tag_match & (~tag_match + 1'b1);
	assign ecc_err_allowed = lookup_valid && ((|tag_err_inj) || (|(data_err_inj & tag_match)));
	// The DUT flags an ECC error on a valid lookup that reads a corrupted tag in any way, or corrupted
	// data in the way it reads from: the lowest way that hit. Corrupted data in another matching way is
	// not read, so it must not cause an error.
	logic [IC_NUM_WAYS-1:0] tag_err_inj, data_err_inj, read_way;
	logic ecc_err_allowed;
	assign tag_err_inj = (enable_ecc_errors && (& ic_tag_rvalid)) ? tag_sel_line : '0;
	assign data_err_inj = (enable_ecc_errors && (& ic_data_rvalid)) ? data_sel_line : '0;
	assign read_way = tag_match & (~tag_match + 1'b1);
	assign ecc_err_allowed = lookup_valid && ((|tag_err_inj) || (|(data_err_inj & read_way)));

[andreaskurth]

If ecc_err_allowed is tightened (see ibex_icache_ram_if.sv), this would change to:

Suggested change

	second way. Check that an error in the way the cache reads from
	is reported. An error for a corrupted copy in another matching
	way is allowed but not required.'''
	second way. Check that an error in the way the cache reads from
	is reported, and that a corrupted copy in another matching way,
	which the cache doesn't read, causes no error.'''

[andreaskurth]

This repeats the common default from vendor/lowrisc_ip/dv/tools/dvsim/common_sim_cfg.hjson. Could you add a note that the first two files must stay in sync with it? Otherwise the bench silently keeps the old list if the vendored default changes.