Error Reporting

[lorenzschmid]

Hi @tgingold-cern! You might remember me from some discussions we had at @cern back when cheby was in its infancy :) These days, I work at ACP and you were already in contact with one of my colleagues, @stefanlippuner. As you know, our goal is to add APB to cheby. This MR is an intermediary step, where a needed feature, error reporting, is added for the existing supporting buses. In a subsequent MR, I would then like to add support for APB interfaces based on this MR. Please let me know what you think about it.

---

AXI4 Lite and Wishbone both support the feature of returning an error when an invalid address (i.e. an address belonging to the memory space of a block but without associated register) is accessed. Previously, such accesses where acknowledged without indication to the user that the address is invalid.

This commit adds an option report-error activating the report error feature for the two bus protocols. Now in case of an addressing error,

• the AXI4 Lite response signal (BRESP for write or RRESP for read requests) is non-zero returning SLVERR (0b10)
• the Wishbone request is not acknowledged (ACK stays deasserted) but instead an error is returned (ERR is asserted). Furthermore, with an active report-error option, the AXI4 Lite interface behaves now differently when put in reset: Its output signal are configured to allow all handshakes while the two response signal indicate an error. This allows to access the interface while being in reset without stalling the bus.

If the report-error is disabled (default), the exact same output is generated as with the previous implementation.

For each of the two modified bus interfaces, the testbench package has been updated to support the error reporting. Two testbenches have been added, verifying the the error reporting behavior.

[tgingold-cern]

Nice to talk you with you again!

The patch looks OK to me, but I was wondering if bus-error is intuitive than report-error.

A few details:

• there could be a little bit of code factorisation (same code in the two parts of an if). Maybe I need to point that as a review.
• cern-be bus has a support for bus errors (just FYI, you don't need to enhance it now)
• I haven't seen any diagnostic mecanism if use try to use a submap with bus errors in a memory map without bus errors. I don't know if you are using submaps and if you have a particular idea of how this should be handled.

To move forward: let's first discuss the attribute name, in the same time I will add comments in the code.

[lorenzschmid]

Thanks a lot for the extensive feedback!

The patch looks OK to me, but I was wondering if bus-error is intuitive than report-error.

That is perfectly fine with me, this is changed in b215e8307554f564cbedb26ee29d641226ee286d.

• there could be a little bit of code factorisation (same code in the two parts of an if). Maybe I need to point that as a review.

I agree. In general, I tried to stick as much as possible to the original code, i.e. probably the old code was already suffering under the same fate. Furthermore, it was important to me that with bus-error: false (i.e. the original implementation), the code would produce the same output as before.

• cern-be bus has a support for bus errors (just FYI, you don't need to enhance it now)

I didn't know that and I gladly add it. I simply couldn't find a documentation for it.

• I haven't seen any diagnostic mecanism if use try to use a submap with bus errors in a memory map without bus errors. I don't know if you are using submaps and if you have a particular idea of how this should be handled.

For a leaf with an AXI Lite interface and bus-error: true, this should not be a problem as the error is routed on a separate signal (BRESP or RRESP) which simply will be ignored by the crossbar. Hence, the error does not propagate but also does not interfere with the remaining bus handshake signals.

This is different for Wishbone: Since it either returns ACK or ERR. Hence, if there is an error but the error signal cannot propagate through the crossbar, the request will never be acknowledged. If you ask me, I would blame the user there but we might want to add some warning or error during generation. What do you think?

I will answer the comments on the code separately. Thanks again for your feedback.

[tgingold-cern]

Thank you for the MR.

All the remaining points could be handled later.

I am not sure what to do for behaviour under reset. I fear we need two levels of reset: a power-on reset which really initialize the design, and a soft-reset which reset the registers and reply err to accesses.

[lorenzschmid]

Awesome, thanks!