Router enhancements #79
Conversation
src/components/scc/router.h
Outdated
| size_t idx = addr_decoder.getEntry(address); | ||
| if(idx == addr_decoder.null_entry) { | ||
| if(default_idx == std::numeric_limits<size_t>::max()) { | ||
| SCCWARN(SCMOD) << "target address=0x" << std::hex << address << " not found for " << (trans.get_command() == tlm::TLM_READ_COMMAND ? "read" : "write") << " transaction."; |
There was a problem hiding this comment.
Hard coding the warning is not a good option. We have use cases where a bus error response is a legal case. This should become configurable.
There was a problem hiding this comment.
Can we use the following approach to configure a warning in this scenario?
void set_warn_on_addr_not_found(bool enable) { warn_on_addr_not_found = enable; }
bool warn_on_addr_not_found{false};
if(warn_on_addr_not_found) {
SCCWARN(SCMOD) << "target address=0x" << std::hex << address << " not found for " << (trans.get_command() == tlm::TLM_READ_COMMAND ? "read" : "write") << " transaction.";
}
There was a problem hiding this comment.
Sounds good. Maybe you call it warn_on_address_error (and the respective setter.
src/components/scc/router.h
Outdated
| size_t idx = addr_decoder.getEntry(address); | ||
| if(idx == addr_decoder.null_entry) { | ||
| if(default_idx == std::numeric_limits<size_t>::max()) { | ||
| SCCWARN(SCMOD) << "target address=0x" << std::hex << address << " not found for " << (trans.get_command() == tlm::TLM_READ_COMMAND ? "read" : "write") << " transaction."; |
There was a problem hiding this comment.
Same here: hard coding the warning is not a good option. We have use cases where a bus error response is a legal case. This should become configurable.
src/components/scc/router.h
Outdated
| size_t idx = addr_decoder.getEntry(address); | ||
| if(idx == addr_decoder.null_entry) { | ||
| if(default_idx == std::numeric_limits<size_t>::max()) { | ||
| SCCWARN(SCMOD) << "target address=0x" << std::hex << address << " not found for " << (trans.get_command() == tlm::TLM_READ_COMMAND ? "read" : "write") << " transaction."; |
There was a problem hiding this comment.
Same here: hard coding the warning is not a good option. We have use cases where a bus error response is a legal case. This should become configurable.
| */ | ||
| template <typename TYPE> void bind_target(TYPE& socket, size_t idx, uint64_t base, uint64_t size, bool remap = true) { | ||
| set_target_range(idx, base, size, remap); | ||
| set_target_name(idx, socket.basename()); |
There was a problem hiding this comment.
Removing this line breaks add_target_range(std::string name, uint64_t base, uint64_t size, bool remap)
There was a problem hiding this comment.
Can we use set_target_name(idx, socket.get_base_export().basename()); instead of set_target_name(idx, socket.basename()); ?
There was a problem hiding this comment.
Is there a reason to use the name of the export?
There was a problem hiding this comment.
Is there a reason to use the name of the export? If the socket to bind is a tlm::tlm_target_socket it maps to the same than tlm::tlm_target_socket::basename. In case of hierachical binding (binding to tlm::tlm_initiator_socket), the export gets a generated name which is the port name with a '_export' appended. And this is not what a user expects.
There was a problem hiding this comment.
We are passing tlm::tlm_base_target_socket_b as the socket type. Based on my understanding, this class does not provide the basename() function because it is an abstract interface and does not inherit from sc_object. This causes a build issue when code tries to call basename() on it.
so maybe we can use socket.get_base_export().basename(), since the export object is derived from sc_object and supports basename().
There was a problem hiding this comment.
As described above this breaks the semantic if doing hierarchical binding to an initiator socket since the basename of the export '<socket_name>_export'.
Lets remove the call to set_target_name() entirely and move the responsibility to the caller.
tlm::tlm_base_target_socket_b does not contain a member named basename, which may be required for virtual platform development.
- Call addr_decoder.validate() after invoking addr_decoder.addEntry() to check for overlaps. A configuration option has been added to enable this validation. - Implement an end_of_elaboration method and call the addr_decoder.validate() function during elaboration.
quic-prateek
force-pushed
the
router-overlap-address-not-found-fix
branch
from
c4eca86 to
052d47a
Compare
2 participants
The following router enhancements have been added:
Add warning statements when an address is not found (b_transport, get_direct_mem_ptr, and transport_dbg).
Call addr_decoder.validate() after invoking addr_decoder.addEntry() to check for overlaps. A configuration option has been added to enable this validation.
Add an end_of_elaboration callback and invoke the addr_decoder.validate() function within it.