Superficially it looks good to me though; thanks for doing this.

I'm a little squeamish about it scheduling the event right at curTick;
there's just something "not right" about that.  However, I definitely prefer
that to the even more meaningless and arbitrary "curTick+1".  Probably the
"right" thing to do is to schedule it on the next CPU clock tick, but I can
imagine that it could be a pain to figure out what that is down in the
bowels of the TLB.  Overall I'm OK with leaving it the way it is, I mostly
wanted to mention that issue to see if anyone else agreed or disagreed or
had a better idea.

Steve

On Sat, May 2, 2009 at 8:55 PM, Gabe Black <[email protected]> wrote:

> Never mind. Something's broken. I'll send this out a new one once I fix it.
>
> Gabe
>
> Gabe Black wrote:
> > This patch deals with the same bug as my last one but uses an event
> > instead of a bool. In the process of implementing this I discovered that
> > any faulting memory operation has the same problem since it doesn't go
> > ahead with the access. That's fixed here by delaying translationFault as
> > well as completeDataAccess.
> >
> > Gabe
> >
> > [email protected] wrote:
> >
> >> # HG changeset patch
> >> # User Gabe Black <[email protected]>
> >> # Date 1241318863 25200
> >> # Node ID 8876c73dde1b40863d98fadb1524536eb333a658
> >> # Parent  af13ed3bea4884454325d5e05d7e9d2ff54e7301
> >> CPU: Defer completing an access until we're no longer running out of
> initiateAcc.
> >>
> >> diff --git a/src/cpu/simple/timing.cc b/src/cpu/simple/timing.cc
> >> --- a/src/cpu/simple/timing.cc
> >> +++ b/src/cpu/simple/timing.cc
> >> @@ -105,7 +105,7 @@
> >>
> >>  TimingSimpleCPU::TimingSimpleCPU(TimingSimpleCPUParams *p)
> >>      : BaseSimpleCPU(p), fetchTranslation(this), icachePort(this,
> p->clock),
> >> -    dcachePort(this, p->clock), fetchEvent(this)
> >> +    dcachePort(this, p->clock), completeAccessEvent(this),
> fetchEvent(this)
> >>  {
> >>      _status = Idle;
> >>
> >> @@ -275,7 +275,7 @@
> >>          delete data;
> >>          delete req;
> >>
> >> -        translationFault(fault);
> >> +        delayAccessCompletion(fault);
> >>          return;
> >>      }
> >>      PacketPtr pkt;
> >> @@ -284,7 +284,7 @@
> >>      if (req->getFlags().isSet(Request::NO_ACCESS)) {
> >>          assert(!dcache_pkt);
> >>          pkt->makeResponse();
> >> -        completeDataAccess(pkt);
> >> +        delayAccessCompletion(pkt);
> >>      } else if (read) {
> >>          handleReadPacket(pkt);
> >>      } else {
> >> @@ -302,7 +302,7 @@
> >>              handleWritePacket();
> >>          } else {
> >>              _status = DcacheWaitResponse;
> >> -            completeDataAccess(pkt);
> >> +            delayAccessCompletion(pkt);
> >>          }
> >>      }
> >>  }
> >> @@ -318,9 +318,9 @@
> >>          delete req1;
> >>          delete req2;
> >>          if (fault1 != NoFault)
> >> -            translationFault(fault1);
> >> +            delayAccessCompletion(fault1);
> >>          else if (fault2 != NoFault)
> >> -            translationFault(fault2);
> >> +            delayAccessCompletion(fault2);
> >>          return;
> >>      }
> >>      PacketPtr pkt1, pkt2;
> >> @@ -328,7 +328,7 @@
> >>      if (req->getFlags().isSet(Request::NO_ACCESS)) {
> >>          assert(!dcache_pkt);
> >>          pkt1->makeResponse();
> >> -        completeDataAccess(pkt1);
> >> +        delayAccessCompletion(pkt1);
> >>      } else if (read) {
> >>          if (handleReadPacket(pkt1)) {
> >>              SplitFragmentSenderState * send_state =
> >> diff --git a/src/cpu/simple/timing.hh b/src/cpu/simple/timing.hh
> >> --- a/src/cpu/simple/timing.hh
> >> +++ b/src/cpu/simple/timing.hh
> >> @@ -309,8 +309,51 @@
> >>
> >>      };
> >>
> >> +    struct CompleteAccessEvent : public Event
> >> +    {
> >> +        PacketPtr pkt;
> >> +        Fault fault;
> >> +        TimingSimpleCPU *cpu;
> >> +
> >> +        CompleteAccessEvent(TimingSimpleCPU *_cpu)
> >> +            : cpu(_cpu) {}
> >> +
> >> +        void
> >> +        process()
> >> +        {
> >> +            if (fault != NoFault) {
> >> +                cpu->translationFault(fault);
> >> +            } else {
> >> +                cpu->completeDataAccess(pkt);
> >> +            }
> >> +        }
> >> +        const char
> >> +        *description() const
> >> +        {
> >> +            return "Delayed data access completion";
> >> +        }
> >> +    };
> >> +
> >>      IcachePort icachePort;
> >>      DcachePort dcachePort;
> >> +
> >> +    CompleteAccessEvent completeAccessEvent;
> >> +
> >> +    void
> >> +    delayAccessCompletion(PacketPtr pkt)
> >> +    {
> >> +        completeAccessEvent.pkt = pkt;
> >> +        completeAccessEvent.fault = NoFault;
> >> +        schedule(&completeAccessEvent, curTick);
> >> +    }
> >> +
> >> +    void
> >> +    delayAccessCompletion(Fault fault)
> >> +    {
> >> +        completeAccessEvent.pkt = NULL;
> >> +        completeAccessEvent.fault = fault;
> >> +        schedule(&completeAccessEvent, curTick);
> >> +    }
> >>
> >>      PacketPtr ifetch_pkt;
> >>      PacketPtr dcache_pkt;
> >> _______________________________________________
> >> m5-dev mailing list
> >> [email protected]
> >> http://m5sim.org/mailman/listinfo/m5-dev
> >>
> >>
> >
> > _______________________________________________
> > m5-dev mailing list
> > [email protected]
> > http://m5sim.org/mailman/listinfo/m5-dev
> >
>
> _______________________________________________
> m5-dev mailing list
> [email protected]
> http://m5sim.org/mailman/listinfo/m5-dev
>
_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev

Reply via email to