From: Gabe Black Date: Sat, 3 Jun 2017 00:03:20 +0000 (-0700) Subject: dev: Avoid arbitrarily deep stack depth in the i8254xGBe model. X-Git-Tag: v19.0.0.0~2755 X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=dd3fc1f996679f4cfd29f980d43a0652542e6d9b;p=gem5.git dev: Avoid arbitrarily deep stack depth in the i8254xGBe model. When it comes time to send a packet with the i8254xGBe model hooked up to EtherTap and while running in KVM mode, the packet will first go to the EtherTap over the network style port between them. EtherTap, because it's not actually a model of anything in the simulation, will immediately pass the packet off to the real network and report that the transmission was successful to the i8254xGBe. The i8254xGBe will notice that it still has stuff it can send (the KVM mode CPU has no trouble keeping it full) and will, without returning and collapsing the stack, immediately call back into EtherTap with the next packet. This loop repeats, continually deepening the stack, until gem5 crashes with a segfault. To break this loop, a few small changes have been made. First, txFifoTick has been repurposed slightly so that it continuously keeps track of whether there's still work to do to flush out the fifo during the current tick. The code in txWire has been adjusted slightly so that it clears that variable at the start (also removing some redundancy), so that other code can set it again if more work needs to be done. Specifically, the ethTxDone function will set that flag. If there's more work to be done flushing the Fifo while in tick(), it will loop until txFifoTick stays cleared, meaning either the Fifo is empty, or the object on the other end hasn't said it's done yet. Finally, a new bool member called inTick has been added which keeps track of whether the tick() function is still active somewhere in the callstack. If it is, then the tick event shouldn't be rescheduled in ethTxDone, since tick will take care of that before it returns. It won't check to see if it needs to, and so without this check there's a panic from scheduling the same event twice. It's not completely clear that the Fifo should send packets over and over as fast as the other side accepts them within the same tick, although it's not clear that it shouldn't either. If not, then probably all that would need to change would be to remove the "while" loop so that the tick event would be rescheduled, and the Fifo would be further emptied the next time around. Change-Id: I653379b43389d0539ecfadb3fc6c40e38a8864c2 Reviewed-on: https://gem5-review.googlesource.com/3642 Reviewed-by: Andreas Hansson Maintainer: Gabe Black --- diff --git a/src/dev/net/i8254xGBe.cc b/src/dev/net/i8254xGBe.cc index 70909549a..74379592a 100644 --- a/src/dev/net/i8254xGBe.cc +++ b/src/dev/net/i8254xGBe.cc @@ -59,9 +59,9 @@ using namespace Net; IGbE::IGbE(const Params *p) : EtherDevice(p), etherInt(NULL), cpa(NULL), - rxFifo(p->rx_fifo_size), txFifo(p->tx_fifo_size), rxTick(false), - txTick(false), txFifoTick(false), rxDmaPacket(false), pktOffset(0), - fetchDelay(p->fetch_delay), wbDelay(p->wb_delay), + rxFifo(p->rx_fifo_size), txFifo(p->tx_fifo_size), inTick(false), + rxTick(false), txTick(false), txFifoTick(false), rxDmaPacket(false), + pktOffset(0), fetchDelay(p->fetch_delay), wbDelay(p->wb_delay), fetchCompDelay(p->fetch_comp_delay), wbCompDelay(p->wb_comp_delay), rxWriteDelay(p->rx_write_delay), txReadDelay(p->tx_read_delay), rdtrEvent(this), radvEvent(this), @@ -2385,9 +2385,10 @@ IGbE::rxStateMachine() void IGbE::txWire() { + txFifoTick = false; + if (txFifo.empty()) { anWe("TXQ", "TX FIFO Q"); - txFifoTick = false; return; } @@ -2411,12 +2412,8 @@ IGbE::txWire() txBytes += txFifo.front()->length; txPackets++; - txFifoTick = false; txFifo.pop(); - } else { - // We'll get woken up when the packet ethTxDone() gets called - txFifoTick = false; } } @@ -2425,18 +2422,27 @@ IGbE::tick() { DPRINTF(EthernetSM, "IGbE: -------------- Cycle --------------\n"); + inTick = true; + if (rxTick) rxStateMachine(); if (txTick) txStateMachine(); - if (txFifoTick) + // If txWire returns and txFifoTick is still set, that means the data we + // sent to the other end was already accepted and we can send another + // frame right away. This is consistent with the previous behavior which + // would send another frame if one was ready in ethTxDone. This version + // avoids growing the stack with each frame sent which can cause stack + // overflow. + while (txFifoTick) txWire(); - if (rxTick || txTick || txFifoTick) schedule(tickEvent, curTick() + clockPeriod()); + + inTick = false; } void @@ -2450,8 +2456,8 @@ IGbE::ethTxDone() if (txDescCache.descLeft() != 0 && drainState() != DrainState::Draining) txTick = true; - restartClock(); - txWire(); + if (!inTick) + restartClock(); DPRINTF(EthernetSM, "TxFIFO: Transmission complete\n"); } diff --git a/src/dev/net/i8254xGBe.hh b/src/dev/net/i8254xGBe.hh index ed0bf4713..80e2104a0 100644 --- a/src/dev/net/i8254xGBe.hh +++ b/src/dev/net/i8254xGBe.hh @@ -75,6 +75,7 @@ class IGbE : public EtherDevice EthPacketPtr txPacket; // Should to Rx/Tx State machine tick? + bool inTick; bool rxTick; bool txTick; bool txFifoTick;