Re: [m5-dev] [PATCH 2 of 3] BaseDynInst: Make the TLB translation timing instead of atomic
On Tue, 01 Dec 2009 20:37:57 -, Steve Reinhardt ste...@gmail.com wrote: It looks like you lost the initialization of isUncacheable... is that safe? Hm, yes I'll fix that. Actually I'm not sure why we need that variable, and don't just have BaseDynInst::uncacheable() call req-isUncacheable() directly (unless there are times we call it when we don't have a req, but then how do we know the right answer?). I'll check this out and fix up if I can. Also I'd suggest just folding this patch in with your last patch that fixes up TimingSimpleCPU too, basically getting there in one step instead of two. Ok, no problem. I'll make them a single patch. Cheers Tim -- The University of Edinburgh is a charitable body, registered in Scotland, with registration number SC005336. ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] [PATCH 2 of 3] BaseDynInst: Make the TLB translation timing instead of atomic
Hi Tim,
It looks like you lost the initialization of isUncacheable... is that safe?
diff --git a/src/cpu/base_dyn_inst.hh b/src/cpu/base_dyn_inst.hh
--- a/src/cpu/base_dyn_inst.hh
+++ b/src/cpu/base_dyn_inst.hh
@@ -861,29 +871,14 @@
Request *req = new Request(asid, addr, sizeof(T), flags, this-PC,
thread-contextId(), threadNumber);
- fault = cpu-dtb-translateAtomic(req, thread-getTC(), BaseTLB::Read);
+ initiateTranslation(req, NULL, BaseTLB::Read);
- if (req-isUncacheable())
- isUncacheable = true;
+ effAddr = req-getVaddr();
+ effAddrValid = true;
+ if (fault == NoFault) {
+ cpu-read(req, data, lqIdx);
+ } else {
Actually I'm not sure why we need that variable, and don't just have
BaseDynInst::uncacheable() call req-isUncacheable() directly (unless
there are times we call it when we don't have a req, but then how do
we know the right answer?).
Also I'd suggest just folding this patch in with your last patch that
fixes up TimingSimpleCPU too, basically getting there in one step
instead of two.
Thanks for all the great work!
Steve
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] [PATCH 2 of 3] BaseDynInst: Make the TLB translation timing instead of atomic
More or less, yes I did. I made some modifications though. I can
definitely try to incorporate this into Timing too though.
Tim
On Mon, 09 Nov 2009 18:26:25 -, nathan binkert n...@binkert.org
wrote:
Did you pull the code in translation.hh out of cpu/simple/timing.hh?
If so, does it need to remain? I'd prefer not to have to copies of
the same code floating around.
Nate
On Mon, Nov 9, 2009 at 5:30 AM, Timothy M. Jones tjon...@inf.ed.ac.uk
wrote:
# HG changeset patch
# User Timothy M. Jones tjon...@inf.ed.ac.uk
# Date 1257772288 0
# Node ID da27e67385cca6cf4dd6d18cdead5cfd54559afb
# Parent 861198113ecaf172b6d1e874cda4d13c92bdb38a
BaseDynInst: Make the TLB translation timing instead of atomic.
This initiates a timing translation and passes the read or write on to
the
processor before waiting for it to finish. Once the translation is
finished,
the instruction's state is updated via the 'finish' function. A new
DataTranslation class is created to handle this. The idea is taken from
the
implementation of timing translations in TimingSimpleCPU by Gabe Black.
diff --git a/src/cpu/base_dyn_inst.hh b/src/cpu/base_dyn_inst.hh
--- a/src/cpu/base_dyn_inst.hh
+++ b/src/cpu/base_dyn_inst.hh
@@ -1,5 +1,6 @@
/*
* Copyright (c) 2004-2006 The Regents of The University of Michigan
+ * Copyright (c) 2009 The University of Edinburgh
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -26,6 +27,7 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*
* Authors: Kevin Lim
+ * Timothy M. Jones
*/
#ifndef __CPU_BASE_DYN_INST_HH__
@@ -45,6 +47,7 @@
#include cpu/inst_seq.hh
#include cpu/op_class.hh
#include cpu/static_inst.hh
+#include cpu/translation.hh
#include mem/packet.hh
#include sim/system.hh
#include sim/tlb.hh
@@ -126,8 +129,15 @@
* @return Returns any fault due to the write.
*/
template class T
-Fault write(T data, Addr addr, unsigned flags,
-uint64_t *res);
+Fault write(T data, Addr addr, unsigned flags, uint64_t *res);
+
+/** Initiate a DTB address translation. */
+void initiateTranslation(RequestPtr req, uint64_t *res,
+ BaseTLB::Mode mode);
+
+/** Finish a DTB address translation. */
+void finishTranslation(Fault translate_fault, RequestPtr req,
+ uint64_t *res, bool read);
void prefetch(Addr addr, unsigned flags);
void writeHint(Addr addr, int size, unsigned flags);
@@ -861,29 +871,14 @@
Request *req = new Request(asid, addr, sizeof(T), flags, this-PC,
thread-contextId(), threadNumber);
-fault = cpu-dtb-translateAtomic(req, thread-getTC(),
BaseTLB::Read);
+initiateTranslation(req, NULL, BaseTLB::Read);
-if (req-isUncacheable())
-isUncacheable = true;
+effAddr = req-getVaddr();
+effAddrValid = true;
+if (fault == NoFault) {
+cpu-read(req, data, lqIdx);
+} else {
-if (fault == NoFault) {
-effAddr = req-getVaddr();
-effAddrValid = true;
-physEffAddr = req-getPaddr();
-memReqFlags = req-getFlags();
-
-#if 0
-if (cpu-system-memctrl-badaddr(physEffAddr)) {
-fault = TheISA::genMachineCheckFault();
-data = (T)-1;
-this-setExecuted();
-} else {
-fault = cpu-read(req, data, lqIdx);
-}
-#else
-fault = cpu-read(req, data, lqIdx);
-#endif
-} else {
// Return a fixed value to keep simulation deterministic even
// along misspeculated paths.
data = (T)-1;
@@ -891,7 +886,6 @@
// Commit will have to clean up whatever happened. Set this
// instruction as executed.
this-setExecuted();
-delete req;
}
if (traceData) {
@@ -916,14 +910,37 @@
Request *req = new Request(asid, addr, sizeof(T), flags, this-PC,
thread-contextId(), threadNumber);
-fault = cpu-dtb-translateAtomic(req, thread-getTC(),
BaseTLB::Write);
+initiateTranslation(req, res, BaseTLB::Write);
+effAddr = req-getVaddr();
+effAddrValid = true;
+if (fault == NoFault) {
+cpu-write(req, data, sqIdx);
+}
+
+return fault;
+}
+
+templateclass Impl
+inline void
+BaseDynInstImpl::initiateTranslation(RequestPtr req, uint64_t *res,
+ BaseTLB::Mode mode)
+{
+DataTranslationImpl *trans =
+new DataTranslationImpl(this, res, mode);
+cpu-dtb-translateTiming(req, thread-getTC(), trans, mode);
+}
+
+templateclass Impl
+inline void
+BaseDynInstImpl::finishTranslation(Fault translate_fault, RequestPtr
req,
+ uint64_t *res, bool read)
+{
+fault = translate_fault;
if (req-isUncacheable())
[m5-dev] [PATCH 2 of 3] BaseDynInst: Make the TLB translation timing instead of atomic
# HG changeset patch
# User Timothy M. Jones tjon...@inf.ed.ac.uk
# Date 1257772288 0
# Node ID da27e67385cca6cf4dd6d18cdead5cfd54559afb
# Parent 861198113ecaf172b6d1e874cda4d13c92bdb38a
BaseDynInst: Make the TLB translation timing instead of atomic.
This initiates a timing translation and passes the read or write on to the
processor before waiting for it to finish. Once the translation is finished,
the instruction's state is updated via the 'finish' function. A new
DataTranslation class is created to handle this. The idea is taken from the
implementation of timing translations in TimingSimpleCPU by Gabe Black.
diff --git a/src/cpu/base_dyn_inst.hh b/src/cpu/base_dyn_inst.hh
--- a/src/cpu/base_dyn_inst.hh
+++ b/src/cpu/base_dyn_inst.hh
@@ -1,5 +1,6 @@
/*
* Copyright (c) 2004-2006 The Regents of The University of Michigan
+ * Copyright (c) 2009 The University of Edinburgh
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -26,6 +27,7 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*
* Authors: Kevin Lim
+ * Timothy M. Jones
*/
#ifndef __CPU_BASE_DYN_INST_HH__
@@ -45,6 +47,7 @@
#include cpu/inst_seq.hh
#include cpu/op_class.hh
#include cpu/static_inst.hh
+#include cpu/translation.hh
#include mem/packet.hh
#include sim/system.hh
#include sim/tlb.hh
@@ -126,8 +129,15 @@
* @return Returns any fault due to the write.
*/
template class T
-Fault write(T data, Addr addr, unsigned flags,
-uint64_t *res);
+Fault write(T data, Addr addr, unsigned flags, uint64_t *res);
+
+/** Initiate a DTB address translation. */
+void initiateTranslation(RequestPtr req, uint64_t *res,
+ BaseTLB::Mode mode);
+
+/** Finish a DTB address translation. */
+void finishTranslation(Fault translate_fault, RequestPtr req,
+ uint64_t *res, bool read);
void prefetch(Addr addr, unsigned flags);
void writeHint(Addr addr, int size, unsigned flags);
@@ -861,29 +871,14 @@
Request *req = new Request(asid, addr, sizeof(T), flags, this-PC,
thread-contextId(), threadNumber);
-fault = cpu-dtb-translateAtomic(req, thread-getTC(), BaseTLB::Read);
+initiateTranslation(req, NULL, BaseTLB::Read);
-if (req-isUncacheable())
-isUncacheable = true;
+effAddr = req-getVaddr();
+effAddrValid = true;
+if (fault == NoFault) {
+cpu-read(req, data, lqIdx);
+} else {
-if (fault == NoFault) {
-effAddr = req-getVaddr();
-effAddrValid = true;
-physEffAddr = req-getPaddr();
-memReqFlags = req-getFlags();
-
-#if 0
-if (cpu-system-memctrl-badaddr(physEffAddr)) {
-fault = TheISA::genMachineCheckFault();
-data = (T)-1;
-this-setExecuted();
-} else {
-fault = cpu-read(req, data, lqIdx);
-}
-#else
-fault = cpu-read(req, data, lqIdx);
-#endif
-} else {
// Return a fixed value to keep simulation deterministic even
// along misspeculated paths.
data = (T)-1;
@@ -891,7 +886,6 @@
// Commit will have to clean up whatever happened. Set this
// instruction as executed.
this-setExecuted();
-delete req;
}
if (traceData) {
@@ -916,14 +910,37 @@
Request *req = new Request(asid, addr, sizeof(T), flags, this-PC,
thread-contextId(), threadNumber);
-fault = cpu-dtb-translateAtomic(req, thread-getTC(), BaseTLB::Write);
+initiateTranslation(req, res, BaseTLB::Write);
+effAddr = req-getVaddr();
+effAddrValid = true;
+if (fault == NoFault) {
+cpu-write(req, data, sqIdx);
+}
+
+return fault;
+}
+
+templateclass Impl
+inline void
+BaseDynInstImpl::initiateTranslation(RequestPtr req, uint64_t *res,
+ BaseTLB::Mode mode)
+{
+DataTranslationImpl *trans =
+new DataTranslationImpl(this, res, mode);
+cpu-dtb-translateTiming(req, thread-getTC(), trans, mode);
+}
+
+templateclass Impl
+inline void
+BaseDynInstImpl::finishTranslation(Fault translate_fault, RequestPtr req,
+ uint64_t *res, bool read)
+{
+fault = translate_fault;
if (req-isUncacheable())
isUncacheable = true;
if (fault == NoFault) {
-effAddr = req-getVaddr();
-effAddrValid = true;
physEffAddr = req-getPaddr();
memReqFlags = req-getFlags();
@@ -931,20 +948,10 @@
assert(res);
req-setExtraData(*res);
}
-#if 0
-if (cpu-system-memctrl-badaddr(physEffAddr)) {
-fault = TheISA::genMachineCheckFault();
-} else {
-fault = cpu-write(req, data, sqIdx);
-}
-#else
-fault = cpu-write(req, data, sqIdx);
-#endif
+
} else {
Re: [m5-dev] [PATCH 2 of 3] BaseDynInst: Make the TLB translation timing instead of atomic
Did you pull the code in translation.hh out of cpu/simple/timing.hh?
If so, does it need to remain? I'd prefer not to have to copies of
the same code floating around.
Nate
On Mon, Nov 9, 2009 at 5:30 AM, Timothy M. Jones tjon...@inf.ed.ac.uk wrote:
# HG changeset patch
# User Timothy M. Jones tjon...@inf.ed.ac.uk
# Date 1257772288 0
# Node ID da27e67385cca6cf4dd6d18cdead5cfd54559afb
# Parent 861198113ecaf172b6d1e874cda4d13c92bdb38a
BaseDynInst: Make the TLB translation timing instead of atomic.
This initiates a timing translation and passes the read or write on to the
processor before waiting for it to finish. Once the translation is finished,
the instruction's state is updated via the 'finish' function. A new
DataTranslation class is created to handle this. The idea is taken from the
implementation of timing translations in TimingSimpleCPU by Gabe Black.
diff --git a/src/cpu/base_dyn_inst.hh b/src/cpu/base_dyn_inst.hh
--- a/src/cpu/base_dyn_inst.hh
+++ b/src/cpu/base_dyn_inst.hh
@@ -1,5 +1,6 @@
/*
* Copyright (c) 2004-2006 The Regents of The University of Michigan
+ * Copyright (c) 2009 The University of Edinburgh
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -26,6 +27,7 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*
* Authors: Kevin Lim
+ * Timothy M. Jones
*/
#ifndef __CPU_BASE_DYN_INST_HH__
@@ -45,6 +47,7 @@
#include cpu/inst_seq.hh
#include cpu/op_class.hh
#include cpu/static_inst.hh
+#include cpu/translation.hh
#include mem/packet.hh
#include sim/system.hh
#include sim/tlb.hh
@@ -126,8 +129,15 @@
* @return Returns any fault due to the write.
*/
template class T
- Fault write(T data, Addr addr, unsigned flags,
- uint64_t *res);
+ Fault write(T data, Addr addr, unsigned flags, uint64_t *res);
+
+ /** Initiate a DTB address translation. */
+ void initiateTranslation(RequestPtr req, uint64_t *res,
+ BaseTLB::Mode mode);
+
+ /** Finish a DTB address translation. */
+ void finishTranslation(Fault translate_fault, RequestPtr req,
+ uint64_t *res, bool read);
void prefetch(Addr addr, unsigned flags);
void writeHint(Addr addr, int size, unsigned flags);
@@ -861,29 +871,14 @@
Request *req = new Request(asid, addr, sizeof(T), flags, this-PC,
thread-contextId(), threadNumber);
- fault = cpu-dtb-translateAtomic(req, thread-getTC(), BaseTLB::Read);
+ initiateTranslation(req, NULL, BaseTLB::Read);
- if (req-isUncacheable())
- isUncacheable = true;
+ effAddr = req-getVaddr();
+ effAddrValid = true;
+ if (fault == NoFault) {
+ cpu-read(req, data, lqIdx);
+ } else {
- if (fault == NoFault) {
- effAddr = req-getVaddr();
- effAddrValid = true;
- physEffAddr = req-getPaddr();
- memReqFlags = req-getFlags();
-
-#if 0
- if (cpu-system-memctrl-badaddr(physEffAddr)) {
- fault = TheISA::genMachineCheckFault();
- data = (T)-1;
- this-setExecuted();
- } else {
- fault = cpu-read(req, data, lqIdx);
- }
-#else
- fault = cpu-read(req, data, lqIdx);
-#endif
- } else {
// Return a fixed value to keep simulation deterministic even
// along misspeculated paths.
data = (T)-1;
@@ -891,7 +886,6 @@
// Commit will have to clean up whatever happened. Set this
// instruction as executed.
this-setExecuted();
- delete req;
}
if (traceData) {
@@ -916,14 +910,37 @@
Request *req = new Request(asid, addr, sizeof(T), flags, this-PC,
thread-contextId(), threadNumber);
- fault = cpu-dtb-translateAtomic(req, thread-getTC(), BaseTLB::Write);
+ initiateTranslation(req, res, BaseTLB::Write);
+ effAddr = req-getVaddr();
+ effAddrValid = true;
+ if (fault == NoFault) {
+ cpu-write(req, data, sqIdx);
+ }
+
+ return fault;
+}
+
+templateclass Impl
+inline void
+BaseDynInstImpl::initiateTranslation(RequestPtr req, uint64_t *res,
+ BaseTLB::Mode mode)
+{
+ DataTranslationImpl *trans =
+ new DataTranslationImpl(this, res, mode);
+ cpu-dtb-translateTiming(req, thread-getTC(), trans, mode);
+}
+
+templateclass Impl
+inline void
+BaseDynInstImpl::finishTranslation(Fault translate_fault, RequestPtr req,
+ uint64_t *res, bool read)
+{
+ fault = translate_fault;
if (req-isUncacheable())
isUncacheable = true;
if (fault == NoFault) {
- effAddr = req-getVaddr();
- effAddrValid = true;
physEffAddr = req-getPaddr();
memReqFlags = req-getFlags();
@@ -931,20 +948,10 @@
