From 91cd599f058e6891acf93ed5e119c93500a5ecba Mon Sep 17 00:00:00 2001 From: Yu-hsin Wang Date: Wed, 12 Oct 2022 10:39:57 +0800 Subject: [PATCH] systemc: sync the response error between gem5 packet and tlm payload For now we don't return the correct error status to the upstream of the protocol conversion bridge. This prevents from the requestor to distinguish if the response is good or not. This change fixes the issue. Change-Id: Iec2a388b50fb1bd4fd97ece19e9061138b0b0a1f Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/64591 Reviewed-by: Jason Lowe-Power Maintainer: Jason Lowe-Power Reviewed-by: Earl Ou Tested-by: kokoro --- src/systemc/tlm_bridge/gem5_to_tlm.cc | 23 ++++++++++++-- src/systemc/tlm_bridge/tlm_to_gem5.cc | 44 +++++++++++++++------------ src/systemc/tlm_bridge/tlm_to_gem5.hh | 2 -- 3 files changed, 45 insertions(+), 24 deletions(-) diff --git a/src/systemc/tlm_bridge/gem5_to_tlm.cc b/src/systemc/tlm_bridge/gem5_to_tlm.cc index 07e4821179..10f7d1a9c7 100644 --- a/src/systemc/tlm_bridge/gem5_to_tlm.cc +++ b/src/systemc/tlm_bridge/gem5_to_tlm.cc @@ -194,6 +194,24 @@ packet2payload(PacketPtr packet) return trans; } +void +setPacketResponse(PacketPtr pkt, tlm::tlm_generic_payload &trans) +{ + pkt->makeResponse(); + + auto resp = trans.get_response_status(); + switch (resp) { + case tlm::TLM_OK_RESPONSE: + break; + case tlm::TLM_COMMAND_ERROR_RESPONSE: + pkt->setBadCommand(); + break; + default: + pkt->setBadAddress(); + break; + } +} + template void Gem5ToTlmBridge::pec( @@ -225,7 +243,7 @@ Gem5ToTlmBridge::pec( // we make a response packet before sending it back to the initiator // side gem5 module. if (packet->needsResponse()) { - packet->makeResponse(); + setPacketResponse(packet, trans); } if (packet->isResponse()) { need_retry = !bridgeResponsePort.sendTimingResp(packet); @@ -296,7 +314,7 @@ Gem5ToTlmBridge::recvAtomic(PacketPtr packet) } if (packet->needsResponse()) - packet->makeResponse(); + setPacketResponse(packet, *trans); trans->release(); @@ -328,6 +346,7 @@ Gem5ToTlmBridge::recvAtomicBackdoor( backdoor = getBackdoor(*trans); } + // Always set success response in Backdoor case. if (packet->needsResponse()) packet->makeResponse(); diff --git a/src/systemc/tlm_bridge/tlm_to_gem5.cc b/src/systemc/tlm_bridge/tlm_to_gem5.cc index 7aec14fe33..703e118dee 100644 --- a/src/systemc/tlm_bridge/tlm_to_gem5.cc +++ b/src/systemc/tlm_bridge/tlm_to_gem5.cc @@ -178,6 +178,18 @@ payload2packet(RequestorID _id, tlm::tlm_generic_payload &trans) return std::make_pair(pkt, true); } +void +setPayloadResponse(tlm::tlm_generic_payload &trans, PacketPtr pkt) +{ + if (!pkt->isError()) { + trans.set_response_status(tlm::TLM_OK_RESPONSE); + } else if (pkt->isRead() || pkt->isWrite()) { + trans.set_response_status(tlm::TLM_COMMAND_ERROR_RESPONSE); + } else { + trans.set_response_status(tlm::TLM_ADDRESS_ERROR_RESPONSE); + } +} + template void TlmToGem5Bridge::sendEndReq(tlm::tlm_generic_payload &trans) @@ -195,9 +207,15 @@ void TlmToGem5Bridge::sendBeginResp(tlm::tlm_generic_payload &trans, sc_core::sc_time &delay) { - tlm::tlm_phase phase = tlm::BEGIN_RESP; + Gem5SystemC::Gem5Extension *extension = nullptr; + trans.get_extension(extension); + panic_if(extension == nullptr, + "Missing gem5 extension when sending BEGIN_RESP"); + auto pkt = extension->getPacket(); - trans.set_response_status(tlm::TLM_OK_RESPONSE); + setPayloadResponse(trans, pkt); + + tlm::tlm_phase phase = tlm::BEGIN_RESP; auto status = socket->nb_transport_bw(trans, phase, delay); @@ -252,8 +270,6 @@ TlmToGem5Bridge::handleEndResp(tlm::tlm_generic_payload &trans) responseInProgress = false; - checkTransaction(trans); - if (needToSendRetry) { bmp.sendRetryResp(); needToSendRetry = false; @@ -267,18 +283,6 @@ TlmToGem5Bridge::destroyPacket(PacketPtr pkt) delete pkt; } -template -void -TlmToGem5Bridge::checkTransaction(tlm::tlm_generic_payload &trans) -{ - if (trans.is_response_error()) { - std::stringstream ss; - ss << "Transaction returned with error, response status = " - << trans.get_response_string(); - SC_REPORT_ERROR("TLM-2", ss.str().c_str()); - } -} - template void TlmToGem5Bridge::invalidateDmi(const gem5::MemBackdoor &backdoor) @@ -362,10 +366,10 @@ TlmToGem5Bridge::b_transport(tlm::tlm_generic_payload &trans, // clean up delete senderState; + setPayloadResponse(trans, pkt); + if (pkt_created) destroyPacket(pkt); - - trans.set_response_status(tlm::TLM_OK_RESPONSE); } template @@ -437,11 +441,11 @@ TlmToGem5Bridge::get_direct_mem_ptr(tlm::tlm_generic_payload &trans, // clean up delete senderState; + setPayloadResponse(trans, pkt); + if (pkt_created) destroyPacket(pkt); - trans.set_response_status(tlm::TLM_OK_RESPONSE); - return backdoor != nullptr; } diff --git a/src/systemc/tlm_bridge/tlm_to_gem5.hh b/src/systemc/tlm_bridge/tlm_to_gem5.hh index deb332ec8f..ca5f681c9c 100644 --- a/src/systemc/tlm_bridge/tlm_to_gem5.hh +++ b/src/systemc/tlm_bridge/tlm_to_gem5.hh @@ -141,8 +141,6 @@ class TlmToGem5Bridge : public TlmToGem5BridgeBase void destroyPacket(gem5::PacketPtr pkt); - void checkTransaction(tlm::tlm_generic_payload &trans); - void invalidateDmi(const gem5::MemBackdoor &backdoor); protected: