changeset f9e3dac185ba in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=f9e3dac185ba
description:
Packet: Remove NACKs from packet and its use in endpoints
This patch removes the NACK frrom the packet as there is no longer any
module in the system that issues them (the bridge was the only one and
the previous patch removes that).
The handling of NACKs was mostly avoided throughout the code base, by
using e.g. panic or assert false, but in a few locations the NACKs
were actually dealt with (although NACKs never occured in any of the
regressions). Most notably, the DMA port will now never receive a NACK
and the backoff time is thus never changed. As a consequence, the
entire backoff mechanism (similar to a PCI bus) is now removed and the
DMA port entirely relies on the bus performing the arbitration and
issuing a retry when appropriate. This is more in line with e.g. PCIe.
Surprisingly, this patch has no impact on any of the regressions. As
mentioned in the patch that removes the NACK from the bridge, a
follow-up patch should change the request and response buffer size for
at least one regression to also verify that the system behaves as
expected when the bridge fills up.
diffstat:
src/arch/arm/ArmTLB.py | 3 -
src/arch/arm/table_walker.cc | 3 +-
src/arch/arm/table_walker.hh | 5 +-
src/arch/x86/pagetable_walker.cc | 94 +++++++++++++++++----------------------
src/cpu/o3/fetch_impl.hh | 2 -
src/cpu/o3/lsq_unit_impl.hh | 2 -
src/cpu/simple/timing.cc | 62 ++++++++-----------------
src/dev/Device.py | 5 --
src/dev/copy_engine.cc | 3 +-
src/dev/dma_device.cc | 52 ++++-----------------
src/dev/dma_device.hh | 16 +-----
src/mem/cache/cache_impl.hh | 14 -----
src/mem/packet.cc | 2 -
src/mem/packet.hh | 23 ---------
14 files changed, 76 insertions(+), 210 deletions(-)
diffs (truncated from 519 to 300 lines):
diff -r d112473185ea -r f9e3dac185ba src/arch/arm/ArmTLB.py
--- a/src/arch/arm/ArmTLB.py Wed Aug 22 11:39:58 2012 -0400
+++ b/src/arch/arm/ArmTLB.py Wed Aug 22 11:39:59 2012 -0400
@@ -47,9 +47,6 @@
cxx_class = 'ArmISA::TableWalker'
port = MasterPort("Port for TableWalker to do walk the translation with")
sys = Param.System(Parent.any, "system object parameter")
- min_backoff = Param.Tick(0, "Minimum backoff delay after failed send")
- max_backoff = Param.Tick(100000, "Minimum backoff delay after failed send")
-
class ArmTLB(SimObject):
type = 'ArmTLB'
diff -r d112473185ea -r f9e3dac185ba src/arch/arm/table_walker.cc
--- a/src/arch/arm/table_walker.cc Wed Aug 22 11:39:58 2012 -0400
+++ b/src/arch/arm/table_walker.cc Wed Aug 22 11:39:59 2012 -0400
@@ -51,8 +51,7 @@
using namespace ArmISA;
TableWalker::TableWalker(const Params *p)
- : MemObject(p), port(this, params()->sys, params()->min_backoff,
- params()->max_backoff), drainEvent(NULL),
+ : MemObject(p), port(this, params()->sys), drainEvent(NULL),
tlb(NULL), currState(NULL), pending(false),
masterId(p->sys->getMasterId(name())),
doL1DescEvent(this), doL2DescEvent(this), doProcessEvent(this)
diff -r d112473185ea -r f9e3dac185ba src/arch/arm/table_walker.hh
--- a/src/arch/arm/table_walker.hh Wed Aug 22 11:39:58 2012 -0400
+++ b/src/arch/arm/table_walker.hh Wed Aug 22 11:39:59 2012 -0400
@@ -287,9 +287,8 @@
* A snooping DMA port merely calls the construtor of the DMA
* port.
*/
- SnoopingDmaPort(MemObject *dev, System *s, Tick min_backoff,
- Tick max_backoff) :
- DmaPort(dev, s, min_backoff, max_backoff)
+ SnoopingDmaPort(MemObject *dev, System *s) :
+ DmaPort(dev, s)
{ }
};
diff -r d112473185ea -r f9e3dac185ba src/arch/x86/pagetable_walker.cc
--- a/src/arch/x86/pagetable_walker.cc Wed Aug 22 11:39:58 2012 -0400
+++ b/src/arch/x86/pagetable_walker.cc Wed Aug 22 11:39:59 2012 -0400
@@ -570,63 +570,49 @@
Walker::WalkerState::recvPacket(PacketPtr pkt)
{
assert(pkt->isResponse());
- if (!pkt->wasNacked()) {
- assert(inflight);
- assert(state == Waiting);
- assert(!read);
- inflight--;
- if (pkt->isRead()) {
- state = nextState;
- nextState = Ready;
- PacketPtr write = NULL;
- read = pkt;
- timingFault = stepWalk(write);
- state = Waiting;
- assert(timingFault == NoFault || read == NULL);
- if (write) {
- writes.push_back(write);
- }
- sendPackets();
+ assert(inflight);
+ assert(state == Waiting);
+ assert(!read);
+ inflight--;
+ if (pkt->isRead()) {
+ state = nextState;
+ nextState = Ready;
+ PacketPtr write = NULL;
+ read = pkt;
+ timingFault = stepWalk(write);
+ state = Waiting;
+ assert(timingFault == NoFault || read == NULL);
+ if (write) {
+ writes.push_back(write);
+ }
+ sendPackets();
+ } else {
+ sendPackets();
+ }
+ if (inflight == 0 && read == NULL && writes.size() == 0) {
+ state = Ready;
+ nextState = Waiting;
+ if (timingFault == NoFault) {
+ /*
+ * Finish the translation. Now that we now the right entry is
+ * in the TLB, this should work with no memory accesses.
+ * There could be new faults unrelated to the table walk like
+ * permissions violations, so we'll need the return value as
+ * well.
+ */
+ bool delayedResponse;
+ Fault fault = walker->tlb->translate(req, tc, NULL, mode,
+ delayedResponse, true);
+ assert(!delayedResponse);
+ // Let the CPU continue.
+ translation->finish(fault, req, tc, mode);
} else {
- sendPackets();
+ // There was a fault during the walk. Let the CPU know.
+ translation->finish(timingFault, req, tc, mode);
}
- if (inflight == 0 && read == NULL && writes.size() == 0) {
- state = Ready;
- nextState = Waiting;
- if (timingFault == NoFault) {
- /*
- * Finish the translation. Now that we now the right entry is
- * in the TLB, this should work with no memory accesses.
- * There could be new faults unrelated to the table walk like
- * permissions violations, so we'll need the return value as
- * well.
- */
- bool delayedResponse;
- Fault fault = walker->tlb->translate(req, tc, NULL, mode,
- delayedResponse, true);
- assert(!delayedResponse);
- // Let the CPU continue.
- translation->finish(fault, req, tc, mode);
- } else {
- // There was a fault during the walk. Let the CPU know.
- translation->finish(timingFault, req, tc, mode);
- }
- return true;
- }
- } else {
- DPRINTF(PageTableWalker, "Request was nacked. Entering retry state\n");
- pkt->reinitNacked();
- if (!walker->sendTiming(this, pkt)) {
- inflight--;
- retrying = true;
- if (pkt->isWrite()) {
- writes.push_back(pkt);
- } else {
- assert(!read);
- read = pkt;
- }
- }
+ return true;
}
+
return false;
}
diff -r d112473185ea -r f9e3dac185ba src/cpu/o3/fetch_impl.hh
--- a/src/cpu/o3/fetch_impl.hh Wed Aug 22 11:39:58 2012 -0400
+++ b/src/cpu/o3/fetch_impl.hh Wed Aug 22 11:39:59 2012 -0400
@@ -363,8 +363,6 @@
DPRINTF(Fetch, "[tid:%u] Waking up from cache miss.\n", tid);
- assert(!pkt->wasNacked());
-
// Only change the status if it's still waiting on the icache access
// to return.
if (fetchStatus[tid] != IcacheWaitResponse ||
diff -r d112473185ea -r f9e3dac185ba src/cpu/o3/lsq_unit_impl.hh
--- a/src/cpu/o3/lsq_unit_impl.hh Wed Aug 22 11:39:58 2012 -0400
+++ b/src/cpu/o3/lsq_unit_impl.hh Wed Aug 22 11:39:59 2012 -0400
@@ -95,8 +95,6 @@
//iewStage->ldstQueue.removeMSHR(inst->threadNumber,inst->seqNum);
- assert(!pkt->wasNacked());
-
// If this is a split access, wait until all packets are received.
if (TheISA::HasUnalignedMemAcc && !state->complete()) {
delete pkt->req;
diff -r d112473185ea -r f9e3dac185ba src/cpu/simple/timing.cc
--- a/src/cpu/simple/timing.cc Wed Aug 22 11:39:58 2012 -0400
+++ b/src/cpu/simple/timing.cc Wed Aug 22 11:39:59 2012 -0400
@@ -719,25 +719,14 @@
bool
TimingSimpleCPU::IcachePort::recvTimingResp(PacketPtr pkt)
{
- if (!pkt->wasNacked()) {
- DPRINTF(SimpleCPU, "Received timing response %#x\n", pkt->getAddr());
- // delay processing of returned data until next CPU clock edge
- Tick next_tick = cpu->nextCycle(curTick());
+ DPRINTF(SimpleCPU, "Received timing response %#x\n", pkt->getAddr());
+ // delay processing of returned data until next CPU clock edge
+ Tick next_tick = cpu->nextCycle();
- if (next_tick == curTick())
- cpu->completeIfetch(pkt);
- else
- tickEvent.schedule(pkt, next_tick);
-
- return true;
- } else {
- assert(cpu->_status == IcacheWaitResponse);
- pkt->reinitNacked();
- if (!sendTimingReq(pkt)) {
- cpu->_status = IcacheRetry;
- cpu->ifetch_pkt = pkt;
- }
- }
+ if (next_tick == curTick())
+ cpu->completeIfetch(pkt);
+ else
+ tickEvent.schedule(pkt, next_tick);
return true;
}
@@ -839,32 +828,21 @@
bool
TimingSimpleCPU::DcachePort::recvTimingResp(PacketPtr pkt)
{
- if (!pkt->wasNacked()) {
- // delay processing of returned data until next CPU clock edge
- Tick next_tick = cpu->nextCycle(curTick());
+ // delay processing of returned data until next CPU clock edge
+ Tick next_tick = cpu->nextCycle();
- if (next_tick == curTick()) {
- cpu->completeDataAccess(pkt);
+ if (next_tick == curTick()) {
+ cpu->completeDataAccess(pkt);
+ } else {
+ if (!tickEvent.scheduled()) {
+ tickEvent.schedule(pkt, next_tick);
} else {
- if (!tickEvent.scheduled()) {
- tickEvent.schedule(pkt, next_tick);
- } else {
- // In the case of a split transaction and a cache that is
- // faster than a CPU we could get two responses before
- // next_tick expires
- if (!retryEvent.scheduled())
- cpu->schedule(retryEvent, next_tick);
- return false;
- }
- }
-
- return true;
- } else {
- assert(cpu->_status == DcacheWaitResponse);
- pkt->reinitNacked();
- if (!sendTimingReq(pkt)) {
- cpu->_status = DcacheRetry;
- cpu->dcache_pkt = pkt;
+ // In the case of a split transaction and a cache that is
+ // faster than a CPU we could get two responses before
+ // next_tick expires
+ if (!retryEvent.scheduled())
+ cpu->schedule(retryEvent, next_tick);
+ return false;
}
}
diff -r d112473185ea -r f9e3dac185ba src/dev/Device.py
--- a/src/dev/Device.py Wed Aug 22 11:39:58 2012 -0400
+++ b/src/dev/Device.py Wed Aug 22 11:39:59 2012 -0400
@@ -46,11 +46,6 @@
type = 'DmaDevice'
abstract = True
dma = MasterPort("DMA port")
- min_backoff_delay = Param.Latency('4ns',
- "min time between a nack packet being received and the next request made
by the device")
- max_backoff_delay = Param.Latency('10us',
- "max time between a nack packet being received and the next request made
by the device")
-
class IsaFake(BasicPioDevice):
diff -r d112473185ea -r f9e3dac185ba src/dev/copy_engine.cc
--- a/src/dev/copy_engine.cc Wed Aug 22 11:39:58 2012 -0400
+++ b/src/dev/copy_engine.cc Wed Aug 22 11:39:59 2012 -0400
@@ -78,8 +78,7 @@
CopyEngine::CopyEngineChannel::CopyEngineChannel(CopyEngine *_ce, int cid)
- : cePort(_ce, _ce->sys, _ce->params()->min_backoff_delay,
- _ce->params()->max_backoff_delay),
+ : cePort(_ce, _ce->sys),
ce(_ce), channelId(cid), busy(false), underReset(false),
refreshNext(false), latBeforeBegin(ce->params()->latBeforeBegin),
latAfterCompletion(ce->params()->latAfterCompletion),
diff -r d112473185ea -r f9e3dac185ba src/dev/dma_device.cc
--- a/src/dev/dma_device.cc Wed Aug 22 11:39:58 2012 -0400
+++ b/src/dev/dma_device.cc Wed Aug 22 11:39:59 2012 -0400
@@ -47,36 +47,18 @@
#include "dev/dma_device.hh"
#include "sim/system.hh"
-DmaPort::DmaPort(MemObject *dev, System *s, Tick min_backoff, Tick max_backoff)
+DmaPort::DmaPort(MemObject *dev, System *s)
: MasterPort(dev->name() + ".dma", dev), device(dev), sys(s),
masterId(s->getMasterId(dev->name())),
pendingCount(0), drainEvent(NULL),
- backoffTime(0), minBackoffDelay(min_backoff),
- maxBackoffDelay(max_backoff), inRetry(false),
- backoffEvent(this)
+ inRetry(false)
{ }
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
