Should wishbone.Decoder generate bus errors for unmapped addresses?

[chenz]

Currently, the Decoder does not catch illegal addresses, meaning that the initiator hangs itself waiting for ack (unless there is a timeout implemented).

When constructed with features = {"err"}, the Decoder only propagates errors from the subordinate busses. Would it be appropriate to also signal an invalid address?

[whitequark]

I would say yes.

[galibert]

It should be an independant feature though.

[whitequark]

@galibert Could you explain your reasoning?

[galibert]

I see three different features flags in there:
a) the bus has the lines to answer a request with an error
b) a given device on the bus may return an error in certain cases (like protected accesses, unmapped zones in a mmu-like, or bridging errors from something else, or timeout on access with something external)
c) a decode may return an error on access to somewhere unmapped

(a) is required for (b) and (c), but I don't see why (a) should force (b) and (c) to be true. In particular you may want to cope with all bad accesses with a global timeout, then specific support in the decoder is just wasted gates.

Also, it goes in general with a capability of the decoder to do something when an access is unmapped. Generating an error is just one possibility, but you also may want to go to a specific device for subtractive decoding, or just trigger a line that can light a led and leave everything frozen for debugging. So the configurability of (c) could go further that just "error or not".

[chenz]

In general, I would expect from a WB interface to follow the protocol and terminate the bus cycle, which means (in case err is part of the interface) asserting either ack or err (ignoring rty for now).

Decoder violates the protocol in these cases:

1. decoder without err, but subordinate bus with err - the decoder will silently ignore this, violating the subordinate's protocol.
2. decoder with err as part of it's interface, but not handling the unmapped address case.

(a) is required for (b) and (c), but I don't see why (a) should force (b) and (c) to be true. In particular you may want to cope with all bad accesses with a global timeout, then specific support in the decoder is just wasted gates.

(a) does not force (b). If a device does not have err as part of it's interface, it is simply expected to always terminate the cycle with ack (if it does not, then this is not the decoder's problem).

I kind of agree that (a) -> (c) should be optional, being able to handle bad addresses separately from bus errors seems like a valid concern. The detection can also be expensive, because it has to be done on top of the decoding (basically addr_bad = !(addr_good_a | addr_good_b | ...), where each addr_good may already be expensive. In fact, when I added this locally to try it out, I had to generate err synchronously in order to make timing (100MHz).

I also think the behaviour outlined in case 1) above should be caught by default, but that is maybe a separate issue.