Deadlock-prone fixed priority arbitration between prefetches and misses at L1 cache

[colluca]

Prefetch requests have fixed priority over miss requests at the L1 cache. When the cache is bombarded with prefetch requests, miss requests will not be served. This might have performance implications, possibly reducing parallelism across cores, but, more importantly, it's also prone to deadlock as encountered in #36.

[skotti]

@colluca It seems to me that there are two priorities here. First at the exit of L0 where refill is preferred over the prefetch (if there is a valid refill). Second at L1, where it depends on the FETCH_PRIORITY - whether it is set to 0 or not. If yes, then the arbitration is round robin actually. prefetch_req in L1 arbiter combine both types of requests. But if FETCH_PRIORITY is set to 1, then priority is for the refills. Thus I wanted to ask, where exactly in the code you see that the prefetch has a fixed priority? In case there is something I overlooked.

[colluca]

Fair question, the description wasn't really detailed😁

To be more specific, the deadlock condition described in the PR sees the miss request arrive at the icache_handler and also being accepted. At the handler's response side, the cache line is also received (out_rsp_valid_i==1), but never accepted (out_rsp_ready_o==0). This appears to be due to the fact that arb_d.sel is always zero, and in turn this is driven by the following "arbiter", which prioritizes hits over returning refills:

cluster_icache/src/snitch_icache_handler.sv

Lines 302 to 313 in ce0ed94

	if (!arb_q.lock) begin
	if (hit_valid) begin
	arb_d.sel = 0;
	arb_d.lock = 1;
	end else if (out_rsp_valid_i) begin
	arb_d.sel = 1;
	arb_d.lock = 1;
	end else begin
	arb_d.sel = 0;
	arb_d.lock = 0;
	end
	end

It seems to me that this arbitration is the whole cause of the deadlock: so long as there are hits to serve, the response to the previous refill request will not be accepted.