x86: Delay X86 table walk on receiving walker response

Information
Submitter:	Andreas Hansson
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 10643:30307bfe1e8c
---------------------------
x86: Delay X86 table walk on receiving walker response

This patch fixes a minor issue in the X86 page table walker where it
ended up sending new request packets to the crossbar before the
response processing was finished (recvTimingResp is directly calling
sendTimingReq). Under certain conditions this caused the crossbar to
see illegal combinations of request/response overlap, in turn causing
problems with a slightly modified crossbar implementation.

Testing Done

Issue Summary

Description	From	Last Updated	Status
What clock is this a clock edge of?	Gabe Black	Jan. 7, 2015, 3:52 a.m.	Dropped

src/arch/x86/pagetable_walker.cc (Diff revision 1)

What clock is this a clock edge of?

Show all issues

Andreas Hansson Jan. 5, 2015, 2:52 p.m. (Jan. 5, 2015, 2:52 p.m.)

The clock of the pagetable walker which is a ClockedObject, and as such it is instantiated as a child to the CPU, and will thus be in the CPU clock domain unless someone gets creative with the config scripts.

Can you explain the sequence of events that result in bad behavior? If possible (which is not necessarily the case), it would be best to fix this without adding arbitrary delays.

Andreas Hansson Jan. 5, 2015, 2:52 p.m. (Jan. 5, 2015, 2:52 p.m.)

The crossbar is processing a response, and as part of that forward it to the next object, in this case the pagetable walker, and before the call to sendTimingResp is finished, the pagetable walker sends a new request back to the crossbar.

This change ensures that the sendTimingResp is finished and done with before the request is sent.

If you do not like the idea of sending it on the next clock edge we can also make it curTick().

Gabe Black Jan. 5, 2015, 3 p.m. (Jan. 5, 2015, 3 p.m.)

curTick would be better I think. I feel like I've run into this sort of thing before, where I've wanted to do something inline but there were assumptions/expectations that it be done in a different call stack. I *think* we even had issues where we'd run out of stack when that looped back on itself too much under certain circumstances, but it may just be that that theoretically could happen but hadn't. In any case, it feels like there's a generic problem to be solved here, although not necessarily as part of what you're doing.

Andreas Hansson Jan. 5, 2015, 3:14 p.m. (Jan. 5, 2015, 3:14 p.m.)

The main reason I went for clockEdge rather than curTick is that it aligns the request sending with the CPU clock edges rather than the crossbar. If curTick is aligned with the CPU clock then clockEdge == curTick. Only if the response arrives in the middle of a CPU cycle will the request be delayed to the next edge. Makes sense?

From an realism point of view I do not see how any sensible implementation would have a direct combinational path from receiving a response to sending a request (not that it has stopped us in other places in the code base).

Gabe Black Jan. 6, 2015, 2:42 p.m. (Jan. 6, 2015, 2:42 p.m.)
```
Yep, that makes sense.
```
Andreas Hansson Jan. 12, 2015, 5:06 a.m. (Jan. 12, 2015, 5:06 a.m.)
```
I will consider that a "Ship it" unless someone objects.
```

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Issue Summary

Status: Closed (submitted)