[gem5-dev] Change in gem5/gem5[master]: cpu: Beef up stats in branch predictor
Michael LeBeane has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/20188 ) Change subject: cpu: Beef up stats in branch predictor .. cpu: Beef up stats in branch predictor Fix a minor BTB stats bug as well as add stats to classify branches by type (Direct/Indirect and Conditional/Unconditional). Also identify the number of branches that have not been fully classified and are likely being handled incorrectly by the BP. Note that this really should be an assert, since the BP relies on branches being fully classified to function properly. However, most existing ISAs have at least a couple of control flow micro-ops that aren't fully classified, and fixing all of them is beyond the scope of this patch. Change-Id: I5e5deb76b375b22e2bfcba092e13c5b96406486e --- M src/cpu/pred/bpred_unit.cc M src/cpu/pred/bpred_unit.hh 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/cpu/pred/bpred_unit.cc b/src/cpu/pred/bpred_unit.cc index 2b549d3..950615e 100644 --- a/src/cpu/pred/bpred_unit.cc +++ b/src/cpu/pred/bpred_unit.cc @@ -76,7 +76,34 @@ lookups .name(name() + ".lookups") -.desc("Number of BP lookups") +.desc("Number of Total BP lookups") +; + +lookupsDirectUCond +.name(name() + ".lookupsDirectUCond") +.desc("Number of BP lookups for Direct Unconditional Branches") +; + +lookupsIndirectUCond +.name(name() + ".lookupsIndirectUCond") +.desc("Number of BP lookups for Indirect Unconditional Branches") +; + +lookupsDirectCond +.name(name() + ".lookupsDirectCond") +.desc("Number of BP lookups for Direct Conditional Branches") +; + +lookupsIndirectCond +.name(name() + ".lookupsIndirectCond") +.desc("Number of BP lookups for Indirect Conditional Branches") +; + +lookupsUnlabeled +.name(name() + ".lookupsUnlabeled") +.desc("Number of BP lookups for branches that have not been fully " + "labeled as Direct/Indirect and Conditional/Unconditional " + "by the ISA description") ; condPredicted @@ -137,7 +164,7 @@ ; indirectMispredicted -.name(name() + "indirectMispredicted") +.name(name() + ".indirectMispredicted") .desc("Number of mispredicted indirect branches.") ; @@ -176,6 +203,23 @@ // If so, get its target addr either from the BTB or the RAS. // Save off record of branch stuff so the RAS can be fixed // up once it's done. +if (inst->isDirectCtrl()) { +if (inst->isUncondCtrl()) +lookupsDirectUCond++; +else if (inst->isCondCtrl()) +lookupsDirectCond++; +else +lookupsUnlabeled++; +} else if (inst->isIndirectCtrl()) { +if (inst->isUncondCtrl()) +lookupsIndirectUCond++; +else if (inst->isCondCtrl()) +lookupsIndirectCond++; +else +lookupsUnlabeled++; +} else { +lookupsUnlabeled++; +} bool pred_taken = false; TheISA::PCState target = pc; @@ -235,8 +279,6 @@ "RAS predicted target: %s, RAS index: %i\n", tid, seqNum, pc, target, predict_record.RASIndex); } else { -++BTBLookups; - if (inst->isCall()) { RAS[tid].push(pc); predict_record.pushedRAS = true; @@ -253,6 +295,7 @@ if (inst->isDirectCtrl() || !iPred) { // Check BTB on direct branches +++BTBLookups; if (BTB.valid(pc.instAddr(), tid)) { ++BTBHits; // If it's not a return, use the BTB to get target addr. diff --git a/src/cpu/pred/bpred_unit.hh b/src/cpu/pred/bpred_unit.hh index fb2d0b1..5702341 100644 --- a/src/cpu/pred/bpred_unit.hh +++ b/src/cpu/pred/bpred_unit.hh @@ -290,6 +290,11 @@ /** Stat for number of BP lookups. */ Stats::Scalar lookups; +Stats::Scalar lookupsDirectUCond; +Stats::Scalar lookupsDirectCond; +Stats::Scalar lookupsIndirectUCond; +Stats::Scalar lookupsIndirectCond; +Stats::Scalar lookupsUnlabeled; /** Stat for number of conditional branches predicted. */ Stats::Scalar condPredicted; /** Stat for number of conditional branches predicted incorrectly. */ -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/20188 To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings Gerrit-Project: public/gem5 Gerrit-Branch: master Gerrit-Change-Id: I5e5deb76b375
[gem5-dev] Change in gem5/gem5[master]: arch-x86: Fix x86 branch labeling
Michael LeBeane has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/20189 ) Change subject: arch-x86: Fix x86 branch labeling .. arch-x86: Fix x86 branch labeling X86 ISA doesn't fully label branches as conditional/unconditional and direct/indirect, which causes the CPU branch predictor to assume they are all indirect, conditional branches. This patch labels all macro-ops and microcode that performs branches with their correct type. X86 also does not implement the branchTarget function, which is used to resolve direct branches early in the decode stage. I went ahead and added the correct functionality to microcoded 'br' instructions, but adding it to the macroops that use 'wrip' instructions would most likely require defining a new microop specifically for direct branches, and I'm not that interested in messing around with it. Therefore, I added in an option to O3 decode to turn off decode branch resolution until someone feels like doing a proper fix. Change-Id: I1397531edbb83c0a26c483aa742b2b98870d7251 --- M src/arch/x86/isa/insts/general_purpose/control_transfer/call.py M src/arch/x86/isa/insts/general_purpose/control_transfer/conditional_jump.py M src/arch/x86/isa/insts/general_purpose/control_transfer/jump.py M src/arch/x86/isa/insts/general_purpose/control_transfer/loop.py M src/arch/x86/isa/macroop.isa M src/arch/x86/isa/microops/regop.isa M src/arch/x86/isa/microops/seqop.isa M src/cpu/o3/O3CPU.py M src/cpu/o3/decode.hh M src/cpu/o3/decode_impl.hh 10 files changed, 113 insertions(+), 10 deletions(-) diff --git a/src/arch/x86/isa/insts/general_purpose/control_transfer/call.py b/src/arch/x86/isa/insts/general_purpose/control_transfer/call.py index c58152c..02097c6 100644 --- a/src/arch/x86/isa/insts/general_purpose/control_transfer/call.py +++ b/src/arch/x86/isa/insts/general_purpose/control_transfer/call.py @@ -41,6 +41,7 @@ # Make the default data size of calls 64 bits in 64 bit mode .adjust_env oszIn64Override .function_call +.direct_control limm t1, imm rdip t7 @@ -55,6 +56,7 @@ # Make the default data size of calls 64 bits in 64 bit mode .adjust_env oszIn64Override .function_call +.indirect_control rdip t1 # Check target of call @@ -68,6 +70,7 @@ # Make the default data size of calls 64 bits in 64 bit mode .adjust_env oszIn64Override .function_call +.indirect_control rdip t7 ld t1, seg, sib, disp @@ -82,6 +85,7 @@ # Make the default data size of calls 64 bits in 64 bit mode .adjust_env oszIn64Override .function_call +.indirect_control rdip t7 ld t1, seg, riprel, disp diff --git a/src/arch/x86/isa/insts/general_purpose/control_transfer/conditional_jump.py b/src/arch/x86/isa/insts/general_purpose/control_transfer/conditional_jump.py index 87b2e6a..3902499 100644 --- a/src/arch/x86/isa/insts/general_purpose/control_transfer/conditional_jump.py +++ b/src/arch/x86/isa/insts/general_purpose/control_transfer/conditional_jump.py @@ -40,6 +40,7 @@ { # Make the defualt data size of jumps 64 bits in 64 bit mode .adjust_env oszIn64Override +.direct_control rdip t1 limm t2, imm @@ -50,6 +51,7 @@ { # Make the defualt data size of jumps 64 bits in 64 bit mode .adjust_env oszIn64Override +.direct_control rdip t1 limm t2, imm @@ -60,6 +62,7 @@ { # Make the default data size of jumps 64 bits in 64 bit mode .adjust_env oszIn64Override +.direct_control rdip t1 limm t2, imm @@ -70,6 +73,7 @@ { # Make the default data size of jumps 64 bits in 64 bit mode .adjust_env oszIn64Override +.direct_control rdip t1 limm t2, imm @@ -80,6 +84,7 @@ { # Make the default data size of jumps 64 bits in 64 bit mode .adjust_env oszIn64Override +.direct_control rdip t1 limm t2, imm @@ -90,6 +95,7 @@ { # Make the default data size of jumps 64 bits in 64 bit mode .adjust_env oszIn64Override +.direct_control rdip t1 limm t2, imm @@ -100,6 +106,7 @@ { # Make the default data size of jumps 64 bits in 64 bit mode .adjust_env oszIn64Override +.direct_control rdip t1 limm t2, imm @@ -110,6 +117,7 @@ { # Make the default data size of jumps 64 bits in 64 bit mode .adjust_env oszIn64Override +.direct_control rdip t1 limm t2, imm @@ -120,6 +128,7 @@ { # Make the default data size of jumps 64 bits in 64 bit mode .adjust_env oszIn64Override +.direct_control rdip t1 limm t2, imm @@ -130,6 +139,7 @@ { # Make the default data size of jumps 64 bits in 64 bit mode .adjust_env oszIn64Override +.direct_control rdip t1 limm t2, imm @@ -140,6 +150,7 @@ { # Make the default data size of jumps 64 bits i
Re: [gem5-dev] Review Request 3836: x86: remove unnecessary parameter from functions
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3836/#review9476 --- Ship it! Ship It! - Michael LeBeane On Feb. 23, 2017, 10:17 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3836/ > --- > > (Updated Feb. 23, 2017, 10:17 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11892:15f4547c339b > --- > x86: remove unnecessary parameter from functions > > > Diffs > - > > src/arch/x86/process.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/x86/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > > Diff: http://reviews.gem5.org/r/3836/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3835: sim: expand AuxVector class
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3835/#review9475 --- Ship it! Ship It! - Michael LeBeane On Feb. 24, 2017, 5:23 a.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3835/ > --- > > (Updated Feb. 24, 2017, 5:23 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11891:da5745659ea2 > --- > sim: expand AuxVector class > > The AuxVector class is responsible for holding Process data (in SE Mode > of course). The data that it holds is normally setup by an OS kernel in > the process address space. The purpose behind doing this is to pass in > information that the process will need for various reasons. (Check out > the enum in the header file for an idea of what the AuxVector holds.) > > The AuxVector struct was changed into a class and encapsulation methods > were added to protect access to the member variables. > > The host ISA may have a different endianness than the simulated ISA. > Since data is passed between the process address space and the > simulator for auxiliary vectors, we need to worry about maintaining > endianness for the right context. > > > Diffs > - > > src/arch/alpha/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/sim/aux_vector.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/sim/aux_vector.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/sparc/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/x86/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/power/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/riscv/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/arm/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/mips/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > > Diff: http://reviews.gem5.org/r/3835/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3830: syscall_emul: remove unused class and member
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3830/#review9468 --- Ship it! Ship It! - Michael LeBeane On Feb. 23, 2017, 7:33 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3830/ > --- > > (Updated Feb. 23, 2017, 7:33 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11880:770add1a79a8 > --- > syscall_emul: remove unused class and member > > > Diffs > - > > src/sim/process.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > > Diff: http://reviews.gem5.org/r/3830/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3831: syscall_emul: remove memState from process and put into own file
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3831/#review9467 --- Ship it! Ship It! - Michael LeBeane On Feb. 23, 2017, 7:44 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3831/ > --- > > (Updated Feb. 23, 2017, 7:44 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11881:51b650187bb2 > --- > syscall_emul: remove memState from process and put into own file > > > Diffs > - > > src/arch/alpha/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/arm/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/mips/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/power/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/riscv/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/sparc/process.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/sparc/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/x86/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/gpu-compute/shader.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/sim/mem_state.hh PRE-CREATION > src/sim/process.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/sim/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/sim/syscall_emul.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/sim/syscall_emul.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > > Diff: http://reviews.gem5.org/r/3831/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3832: style: local variable name correction
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3832/#review9466 --- Ship it! Ship It! - Michael LeBeane On Feb. 23, 2017, 7:56 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3832/ > --- > > (Updated Feb. 23, 2017, 7:56 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11885:33cd29b5e7c5 > --- > style: local variable name correction > > > Diffs > - > > src/sim/syscall_emul.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > > Diff: http://reviews.gem5.org/r/3832/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3833: style: change NULL to nullptr in syscall files
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3833/#review9465 --- Ship it! Ship It! - Michael LeBeane On Feb. 23, 2017, 7:58 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3833/ > --- > > (Updated Feb. 23, 2017, 7:58 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11890:74ef725af658 > --- > style: change NULL to nullptr in syscall files > > > Diffs > - > > src/sim/syscall_emul.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/sim/syscall_emul.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > > Diff: http://reviews.gem5.org/r/3833/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3696: syscall_emul: [PATCH 15/22] add clone/execve for threading and multiprocess simulations
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3696/#review9464 --- Ship it! Ship It! - Michael LeBeane On Feb. 23, 2017, 7:40 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3696/ > --- > > (Updated Feb. 23, 2017, 7:40 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11879:5e2d3ce3a08c > --- > syscall_emul: [PATCH 15/22] add clone/execve for threading and multiprocess > simulations > > Modifies the clone system call and adds execve system call. Requires allowing > processes to steal thread contexts from other processes in the same system > object and the ability to detach pieces of process state (such as MemState) > to allow dynamic sharing. > > > Diffs > - > > src/arch/alpha/linux/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/alpha/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/arm/linux/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/arm/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/generic/types.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/mips/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/power/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/riscv/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/sparc/linux/syscalls.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/sparc/process.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/sparc/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/x86/linux/process.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/x86/linux/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/x86/process.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/x86/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/arch/x86/types.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/cpu/checker/thread_context.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/cpu/o3/thread_context.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/cpu/simple_thread.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/cpu/thread_context.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/cpu/thread_state.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/mem/page_table.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/mem/page_table.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/mem/se_translating_port_proxy.hh > 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/sim/Process.py 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/sim/fd_array.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/sim/fd_array.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/sim/process.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/sim/process.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/sim/syscall_desc.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/sim/syscall_emul.hh 5ea85692a53ea437c95e5a199884bd3a5266f820 > src/sim/syscall_emul.cc 5ea85692a53ea437c95e5a199884bd3a5266f820 > > Diff: http://reviews.gem5.org/r/3696/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3680: syscall_emul: [patch 13/22] add system call retry capability
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3680/#review9442 --- Ship it! Ship It! - Michael LeBeane On Feb. 17, 2017, 10:03 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3680/ > --- > > (Updated Feb. 17, 2017, 10:03 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11859:2f88471df39a > --- > syscall_emul: [patch 13/22] add system call retry capability > > This changeset adds functionality that allows system calls to retry without > affecting thread context state such as the program counter or register values > for the associated thread context (when system calls return with a retry > fault). > > This functionality is needed to solve problems with blocking system calls > in multi-process or multi-threaded simulations where information is passed > between processes/threads. Blocking system calls can cause deadlock because > the simulator itself is single threaded. There is only a single thread > servicing the event queue which can cause deadlock if the thread hits a > blocking system call instruction. > > To illustrate the problem, consider two processes using the producer/consumer > sharing model. The processes can use file descriptors and the read and write > calls to pass information to one another. If the consumer calls the blocking > read system call before the producer has produced anything, the call will > block the event queue (while executing the system call instruction) and > deadlock the simulation. > > The solution implemented in this changeset is to recognize that the system > calls will block and then generate a special retry fault. The fault will > be sent back up through the function call chain until it is exposed to the > cpu model's pipeline where the fault becomes visible. The fault will trigger > the cpu model to replay the instruction at a future tick where the call has > a chance to succeed without actually going into a blocking state. > > In subsequent patches, we recognize that a syscall will block by calling a > non-blocking poll (from inside the system call implementation) and checking > for events. When events show up during the poll, it signifies that the call > would not have blocked and the syscall is allowed to proceed (calling an > underlying host system call if necessary). If no events are returned from the > poll, we generate the fault and try the instruction for the thread context > at a distant tick. Note that retrying every tick is not efficient. > > As an aside, the simulator has some multi-threading support for the event > queue, but it is not used by default and needs work. Even if the event queue > was completely multi-threaded, meaning that there is a hardware thread on > the host servicing a single simulator thread contexts with a 1:1 mapping > between them, it's still possible to run into deadlock due to the event queue > barriers on quantum boundaries. The solution of replaying at a later tick > is the simplest solution and solves the problem generally. > > > Diffs > - > > src/arch/alpha/isa/decoder.isa f438fcbab00edbb36075e1403da75cb9a9ac7f55 > src/arch/arm/faults.hh f438fcbab00edbb36075e1403da75cb9a9ac7f55 > src/arch/arm/faults.cc f438fcbab00edbb36075e1403da75cb9a9ac7f55 > src/arch/mips/isa/decoder.isa f438fcbab00edbb36075e1403da75cb9a9ac7f55 > src/arch/power/isa/decoder.isa f438fcbab00edbb36075e1403da75cb9a9ac7f55 > src/arch/riscv/faults.cc f438fcbab00edbb36075e1403da75cb9a9ac7f55 > src/arch/sparc/linux/process.hh f438fcbab00edbb36075e1403da75cb9a9ac7f55 > src/arch/sparc/linux/process.cc f438fcbab00edbb36075e1403da75cb9a9ac7f55 > src/arch/x86/isa/decoder/one_byte_opcodes.isa > f438fcbab00edbb36075e1403da75cb9a9ac7f55 > src/arch/x86/isa/decoder/two_byte_opcodes.isa > f438fcbab00edbb36075e1403da75cb9a9ac7f55 > src/arch/x86/process.hh f438fcbab00edbb36075e1403da75cb9a9ac7f55 > src/arch/x86/process.cc f438fcbab00edbb36075e1403da75cb9a9ac7f55 > src/arch/x86/pseudo_inst.cc f438fcbab00edbb36075e1403da75cb9a9ac7f55 > src/cpu/BaseCPU.py f438fcbab00edbb36075e1403da75cb9a9ac7f55 > src/cpu/base.hh f438fcbab00edbb36075e1403da75cb9a9ac7f55 > src/cpu/base.cc f438fcbab00edbb36075e1403da75cb9a9ac7f55 > src/cpu/checker/cpu.hh f438fcbab00edbb36075e1403da75cb9a9ac7f55 > src/cpu/checker/thread_contex
Re: [gem5-dev] Review Request 3676: syscall_emul: [patch 10/22] refactor fdentry and add fdarray class
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3676/#review9278 --- Ship it! Ship It! - Michael LeBeane On Nov. 16, 2016, 5:59 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3676/ > --- > > (Updated Nov. 16, 2016, 5:59 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11717:7f3bf41cce5c > --- > syscall_emul: [patch 10/22] refactor fdentry and add fdarray class > > Several large changes happen in this patch. > > The FDEntry class is rewritten so that file descriptors now correspond to > types: 'Regular' which is normal file-backed file with the file open on the > host machine, 'Pipe' which is a pipe that has been opened on the host machine, > and 'Device' which does not have an open file on the host yet acts as a pseudo > device with which to issue ioctls. Other types which might be added in the > future are directory entries and sockets (off the top of my head). > > The FDArray class was create to hold most of the file descriptor handling > that was stuffed into the Process class. It uses shared pointers and > the std::array type to hold the FDEntries mentioned above. The implementation > could use a review; I feel that there's some room for improvement, but it > seems like a decent first step. > > The changes to these two classes needed to be propagated out to the rest > of the code so there were quite a few changes for that. Also, comments were > added where I thought they were needed to help others and extend our > DOxygen coverage. > > > Diffs > - > > src/sim/syscall_emul.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/syscall_emul.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/process.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/process.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/fd_entry.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/fd_entry.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/fd_array.hh PRE-CREATION > src/sim/fd_array.cc PRE-CREATION > src/sim/SConscript c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/gpu-compute/cl_driver.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/kern/tru64/tru64.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/arch/sparc/process.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > > Diff: http://reviews.gem5.org/r/3676/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3749: config: Refactor the network switch configuration file
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3749/#review9184 --- Ship it! Ship It! - Michael LeBeane On Dec. 6, 2016, 5:46 p.m., Gabor Dozsa wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3749/ > --- > > (Updated Dec. 6, 2016, 5:46 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11758:0699314f0c3e > --- > config: Refactor the network switch configuration file > > This patch prevents the body of the script getting executed when > the script is imported as a module. > > Change-Id: I70a50f6295f1e7a088398017f5fa9d06fe90476a > Reviewed-by: Andreas Sandberg > > > Diffs > - > > configs/dist/sw.py 0d38e56356c7 > > Diff: http://reviews.gem5.org/r/3749/diff/ > > > Testing > --- > > > Thanks, > > Gabor Dozsa > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] changeset in gem5: dev: Fix buffer length when unserializing an ...
changeset e832056deaed in /z/repo/gem5 details: http://repo.gem5.org/gem5?cmd=changeset;node=e832056deaed description: dev: Fix buffer length when unserializing an eth pkt Changeset 11701 only serialized the useful portion of of an ethernet packets' payload. However, the device models expect each ethernet packet to contain a 16KB buffer, even if there is no data in it. This patch adds a 'bufLength' field to EthPacketData so the original size of the packet buffer can always be unserialized. Reported-by: Gabor Dozsa diffstat: src/dev/net/etherpkt.cc | 21 + src/dev/net/etherpkt.hh | 9 +++-- src/dev/net/i8254xGBe.cc | 2 +- src/dev/net/ns_gige.cc | 2 +- src/dev/net/sinic.cc | 2 +- 5 files changed, 27 insertions(+), 9 deletions(-) diffs (104 lines): diff -r 09f8fda798bc -r e832056deaed src/dev/net/etherpkt.cc --- a/src/dev/net/etherpkt.cc Mon Nov 28 12:44:54 2016 -0500 +++ b/src/dev/net/etherpkt.cc Tue Nov 29 13:04:45 2016 -0500 @@ -42,6 +42,7 @@ EthPacketData::serialize(const string &base, CheckpointOut &cp) const { paramOut(cp, base + ".simLength", simLength); +paramOut(cp, base + ".bufLength", bufLength); paramOut(cp, base + ".length", length); arrayParamOut(cp, base + ".data", data, length); } @@ -50,11 +51,23 @@ EthPacketData::unserialize(const string &base, CheckpointIn &cp) { paramIn(cp, base + ".length", length); -if (length) { -assert(data == nullptr); -data = new uint8_t[length]; -arrayParamIn(cp, base + ".data", data, length); +unsigned chkpt_buf_length; +if (optParamIn(cp, base + ".bufLength", chkpt_buf_length)) { +// If bufLength is in the checkpoint, make sure that the current buffer +// is unallocated or that the checkpoint requested size is smaller than +// the current buffer. +assert(!data || chkpt_buf_length <= bufLength); +bufLength = chkpt_buf_length; +} else { +// If bufLength is not in the checkpoint, try to use the existing +// buffer or use length to size the buffer +if (!data) +bufLength = length; } +assert(length <= bufLength); +if (!data) +data = new uint8_t[bufLength]; +arrayParamIn(cp, base + ".data", data, length); if (!optParamIn(cp, base + ".simLength", simLength)) simLength = length; } diff -r 09f8fda798bc -r e832056deaed src/dev/net/etherpkt.hh --- a/src/dev/net/etherpkt.hh Mon Nov 28 12:44:54 2016 -0500 +++ b/src/dev/net/etherpkt.hh Tue Nov 29 13:04:45 2016 -0500 @@ -55,6 +55,11 @@ uint8_t *data; /** + * Total size of the allocated data buffer. + */ +unsigned bufLength; + +/** * Amount of space occupied by the payload in the data buffer */ unsigned length; @@ -69,11 +74,11 @@ unsigned simLength; EthPacketData() -: data(nullptr), length(0), simLength(0) +: data(nullptr), bufLength(0), length(0), simLength(0) { } explicit EthPacketData(unsigned size) -: data(new uint8_t[size]), length(0), simLength(0) +: data(new uint8_t[size]), bufLength(size), length(0), simLength(0) { } ~EthPacketData() { if (data) delete [] data; } diff -r 09f8fda798bc -r e832056deaed src/dev/net/i8254xGBe.cc --- a/src/dev/net/i8254xGBe.cc Mon Nov 28 12:44:54 2016 -0500 +++ b/src/dev/net/i8254xGBe.cc Tue Nov 29 13:04:45 2016 -0500 @@ -2522,7 +2522,7 @@ bool txPktExists; UNSERIALIZE_SCALAR(txPktExists); if (txPktExists) { -txPacket = std::make_shared(); +txPacket = std::make_shared(16384); txPacket->unserialize("txpacket", cp); } diff -r 09f8fda798bc -r e832056deaed src/dev/net/ns_gige.cc --- a/src/dev/net/ns_gige.ccMon Nov 28 12:44:54 2016 -0500 +++ b/src/dev/net/ns_gige.ccTue Nov 29 13:04:45 2016 -0500 @@ -2352,7 +2352,7 @@ bool txPacketExists; UNSERIALIZE_SCALAR(txPacketExists); if (txPacketExists) { -txPacket = make_shared(); +txPacket = make_shared(16384); txPacket->unserialize("txPacket", cp); uint32_t txPktBufPtr; UNSERIALIZE_SCALAR(txPktBufPtr); diff -r 09f8fda798bc -r e832056deaed src/dev/net/sinic.cc --- a/src/dev/net/sinic.cc Mon Nov 28 12:44:54 2016 -0500 +++ b/src/dev/net/sinic.cc Tue Nov 29 13:04:45 2016 -0500 @@ -1496,7 +1496,7 @@ UNSERIALIZE_SCALAR(txPacketExists); txPacket = 0; if (txPacketExists) { -txPacket = make_shared(); +txPacket = make_shared(16384); txPacket->unserialize("txPacket", cp); UNSERIALIZE_SCALAR(txPacketOffset); UNSERIALIZE_SCALAR(txPacketBytes); ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3746: dev: Fix race conditions at terminating dist-gem5 simulations
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3746/#review9175 --- Ship it! Ship It! - Michael LeBeane On Nov. 25, 2016, 5:21 p.m., Gabor Dozsa wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3746/ > --- > > (Updated Nov. 25, 2016, 5:21 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11718:dc064d920160 > --- > dev: Fix race conditions at terminating dist-gem5 simulations > > Two problems may arise when a distributed gem5 simulation terminates: > (i) simulation thread(s) may get stuck in an incomplete synchronisation > event which prohibits processing the simulation exit event; and (ii) a > stale receiver thread may try to access objects that have already been > deleted while exiting gem5. This patch terminates receive threads properly > and aborts the processing of any incomplete synchronisation event. > > Change-Id: I72337aa12c7926cece00309640d478b61e55a429 > Reviewed-by: Andreas Sandberg > > > Diffs > - > > src/dev/net/dist_iface.hh 27622f94fdcc > src/dev/net/dist_iface.cc 27622f94fdcc > > Diff: http://reviews.gem5.org/r/3746/diff/ > > > Testing > --- > > > Thanks, > > Gabor Dozsa > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3705: dev: Fix buffer length when unserializing an eth pkt
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3705/ --- (Updated Nov. 23, 2016, 5:36 p.m.) Review request for Default. Repository: gem5 Description --- Changeset 11705:008f04dcc085 --- dev: Fix buffer length when unserializing an eth pkt Changeset 11701 only serialized the useful portion of of an ethernet packets' payload. However, the device models expect each ethernet packet to contain a 16KB buffer, even if there is no data in it. This patch adds a 'bufLength' field to EthPacketData so the original size of the packet buffer can always be unserialized. Reported-by: Gabor Dozsa Diffs (updated) - src/dev/net/etherpkt.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 src/dev/net/etherpkt.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 src/dev/net/i8254xGBe.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 src/dev/net/ns_gige.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 src/dev/net/sinic.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 Diff: http://reviews.gem5.org/r/3705/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3676: syscall_emul: [patch 10/22] refactor fdentry and add fdarray class
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3676/#review9137 --- src/sim/process.cc (line 228) <http://reviews.gem5.org/r/3676/#comment7863> Please add in a warning. src/sim/process.cc (line 249) <http://reviews.gem5.org/r/3676/#comment7864> Please add in a warning. src/sim/syscall_emul.hh (line 989) <http://reviews.gem5.org/r/3676/#comment7865> I see you added getSimFD() to the base class, but it still looks like you have some unnecessary dynamic casts laying around. - Michael LeBeane On Nov. 16, 2016, 5:59 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3676/ > --- > > (Updated Nov. 16, 2016, 5:59 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11717:7f3bf41cce5c > --- > syscall_emul: [patch 10/22] refactor fdentry and add fdarray class > > Several large changes happen in this patch. > > The FDEntry class is rewritten so that file descriptors now correspond to > types: 'Regular' which is normal file-backed file with the file open on the > host machine, 'Pipe' which is a pipe that has been opened on the host machine, > and 'Device' which does not have an open file on the host yet acts as a pseudo > device with which to issue ioctls. Other types which might be added in the > future are directory entries and sockets (off the top of my head). > > The FDArray class was create to hold most of the file descriptor handling > that was stuffed into the Process class. It uses shared pointers and > the std::array type to hold the FDEntries mentioned above. The implementation > could use a review; I feel that there's some room for improvement, but it > seems like a decent first step. > > The changes to these two classes needed to be propagated out to the rest > of the code so there were quite a few changes for that. Also, comments were > added where I thought they were needed to help others and extend our > DOxygen coverage. > > > Diffs > - > > src/sim/syscall_emul.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/syscall_emul.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/process.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/process.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/fd_entry.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/fd_entry.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/fd_array.hh PRE-CREATION > src/sim/fd_array.cc PRE-CREATION > src/sim/SConscript c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/gpu-compute/cl_driver.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/kern/tru64/tru64.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/arch/sparc/process.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > > Diff: http://reviews.gem5.org/r/3676/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3718: syscall_emul: extend sysinfo system call to include mem_unit
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3718/#review9135 --- Ship it! src/sim/syscall_emul.hh (line 780) <http://reviews.gem5.org/r/3718/#comment7862> Why 1? Comment would be helpful. - Michael LeBeane On Nov. 17, 2016, 8:27 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3718/ > --- > > (Updated Nov. 17, 2016, 8:27 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11711:96f28447a815 > --- > syscall_emul: extend sysinfo system call to include mem_unit > > > Diffs > - > > src/sim/syscall_emul.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > > Diff: http://reviews.gem5.org/r/3718/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3719: syscall_emul: add support for x86 statfs system calls
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3719/#review9134 --- Ship it! Ship It! - Michael LeBeane On Nov. 17, 2016, 8:26 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3719/ > --- > > (Updated Nov. 17, 2016, 8:26 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11712:767b13a252fe > --- > syscall_emul: add support for x86 statfs system calls > > > Diffs > - > > src/arch/x86/linux/linux.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/arch/x86/linux/process.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/syscall_emul.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > > Diff: http://reviews.gem5.org/r/3719/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3720: syscall_emul: implement fallocate
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3720/#review9133 --- Ship it! src/sim/syscall_emul.cc (line 956) <http://reviews.gem5.org/r/3720/#comment7861> Newline here. - Michael LeBeane On Nov. 17, 2016, 8:25 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3720/ > --- > > (Updated Nov. 17, 2016, 8:25 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11713:f2882a43dda0 > --- > syscall_emul: implement fallocate > > > Diffs > - > > src/arch/x86/linux/process.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/syscall_emul.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/syscall_emul.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > > Diff: http://reviews.gem5.org/r/3720/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3705: dev: Fix buffer length when unserializing an eth pkt
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3705/ --- (Updated Nov. 21, 2016, 6:57 p.m.) Review request for Default. Changes --- Fix unserialize to support checkpoints created before changeset 11701. Repository: gem5 Description --- Changeset 11705:008f04dcc085 --- dev: Fix buffer length when unserializing an eth pkt Changeset 11701 only serialized the useful portion of of an ethernet packets' payload. However, the device models expect each ethernet packet to contain a 16KB buffer, even if there is no data in it. This patch adds a 'bufLength' field to EthPacketData so the original size of the packet buffer can always be unserialized. Reported-by: Gabor Dozsa Diffs (updated) - src/dev/net/dist_etherlink.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 src/dev/net/dist_iface.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 src/dev/net/etherlink.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 src/dev/net/etherpkt.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 src/dev/net/etherpkt.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 src/dev/net/i8254xGBe.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 src/dev/net/ns_gige.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 src/dev/net/pktfifo.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 src/dev/net/sinic.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 Diff: http://reviews.gem5.org/r/3705/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request 3705: dev: Fix buffer length when unserializing an eth pkt
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3705/ --- Review request for Default. Repository: gem5 Description --- Changeset 11705:008f04dcc085 --- dev: Fix buffer length when unserializing an eth pkt Changeset 11701 only serialized the useful portion of of an ethernet packets' payload. However, the device models expect each ethernet packet to contain a 16KB buffer, even if there is no data in it. This patch adds a 'bufLength' field to EthPacketData so the original size of the packet buffer can always be unserialized. Reported-by: Gabor Dozsa Diffs - src/dev/net/etherpkt.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 src/dev/net/etherpkt.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 Diff: http://reviews.gem5.org/r/3705/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3701: syscall_emul: [PATCH 20/22] add the tgkill system call
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3701/#review9080 --- Ship it! Ship It! - Michael LeBeane On Nov. 14, 2016, 8:56 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3701/ > --- > > (Updated Nov. 14, 2016, 8:56 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11727:33fae597297e > --- > syscall_emul: [PATCH 20/22] add the tgkill system call > > > Diffs > - > > src/sim/syscall_emul.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/arch/x86/linux/process.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > > Diff: http://reviews.gem5.org/r/3701/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3702: syscall_emul: [PATCH 21/22] rewrite code related to system call exits
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3702/#review9079 --- Ship it! Ship It! - Michael LeBeane On Nov. 14, 2016, 9:30 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3702/ > --- > > (Updated Nov. 14, 2016, 9:30 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11728:9ca77be78b02 > --- > syscall_emul: [PATCH 21/22] rewrite code related to system call exits > > The changeset refactors exit, exit_group, and futex related exit > functionality. > > > Diffs > - > > src/sim/syscall_emul.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/se_signal.hh PRE-CREATION > src/sim/futex_map.hh PRE-CREATION > src/sim/process.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/syscall_emul.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/system.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/se_signal.cc PRE-CREATION > > Diff: http://reviews.gem5.org/r/3702/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3704: style: change NULL to nullptr in syscall files
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3704/#review9078 --- Ship it! Ship It! - Michael LeBeane On Nov. 14, 2016, 9:02 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3704/ > --- > > (Updated Nov. 14, 2016, 9:02 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11730:3b22b3ca4675 > --- > style: change NULL to nullptr in syscall files > > > Diffs > - > > src/sim/syscall_emul.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/syscall_emul.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > > Diff: http://reviews.gem5.org/r/3704/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3696: syscall_emul: [PATCH 15/22] add clone/execve for threading and multiprocess simulations
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3696/#review9076 --- In general, I think this patch could be improved by making better use of copy constructors and overloaded assignment operators. I can review again in more detail once that has been addressed. src/arch/x86/process.cc (line 111) <http://reviews.gem5.org/r/3696/#comment7814> Implementing assigment operator would be nice. src/arch/x86/process.cc (line 1100) <http://reviews.gem5.org/r/3696/#comment7812> Same here. src/arch/x86/process.cc (line 1138) <http://reviews.gem5.org/r/3696/#comment7813> Same here. src/cpu/thread_state.hh (line 113) <http://reviews.gem5.org/r/3696/#comment7830> What is this proxy stuff? Some comments would help. src/mem/page_table.cc (line 107) <http://reviews.gem5.org/r/3696/#comment7816> Name is a bit weird IMO. getMappings make it seem like this is a getter, which it isn't. Maybe something like appendMappings? Also should be const. src/sim/Process.py (line 45) <http://reviews.gem5.org/r/3696/#comment7817> Why change this? src/sim/process.hh (line 190) <http://reviews.gem5.org/r/3696/#comment7825> Seems like this should be an assert() instead of a silent return. src/sim/process.hh (line 264) <http://reviews.gem5.org/r/3696/#comment7818> I prefer shared_pointer here. Also could use some comments explaining what these fields are and that they can be shared accross different processes. src/sim/process.cc (line 161) <http://reviews.gem5.org/r/3696/#comment7819> Unnecessary. src/sim/process.cc (line 166) <http://reviews.gem5.org/r/3696/#comment7821> Looking at how you use this, maybe the right thing to do is to just make getMappings an actual getter: MapVec mappings = pTable->getMappings(); src/sim/process.cc (line 212) <http://reviews.gem5.org/r/3696/#comment7823> np->argv.insert(np->argv.end(), argv.begin(), argv.end()); Seems cleaner. Same for envp. src/sim/process.cc (line 259) <http://reviews.gem5.org/r/3696/#comment7831> Should we panic here? src/sim/process.cc (line 338) <http://reviews.gem5.org/r/3696/#comment7832> What about the other variables you introduce, such as exitGroup, sigchld, maxStackSize, etc? Does checkpointing even work? If not, there should be a warning stating what is not supported. src/sim/syscall_desc.hh (line 110) <http://reviews.gem5.org/r/3696/#comment7824> const src/sim/syscall_emul.hh (line 1183) <http://reviews.gem5.org/r/3696/#comment7826> Seems like there should be a cleaner way to do this with a copy constructor. src/sim/syscall_emul.hh (line 1689) <http://reviews.gem5.org/r/3696/#comment7829> In general, there is too much duplication between this function and cloneFunc. For example, some the ProcessParams stuff seems to be copied. This should be factored out a bit better. - Michael LeBeane On Nov. 14, 2016, 8:37 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3696/ > --- > > (Updated Nov. 14, 2016, 8:37 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11722:6fea3eedd891 > --- > syscall_emul: [PATCH 15/22] add clone/execve for threading and multiprocess > simulations > > Modifies the clone system call and adds execve system call. Requires allowing > processes to steal thread contexts from other processes in the same system > object and the ability to detach pieces of process state (such as MemState) > to allow dynamic sharing. > > > Diffs > - > > src/sim/syscall_emul.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/syscall_desc.hh PRE-CREATION > src/sim/syscall_emul.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/process.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/process.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/mem/page_table.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/mem/se_translating_port_proxy.hh > c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/Process.py c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/cpu/thread_state.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/mem/page_table.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/cpu/thread_context.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/cpu/o3/thread_context.hh c38fcdaa5fe508dbb18cc084e758ad0c
Re: [gem5-dev] Review Request 3702: syscall_emul: [PATCH 21/22] rewrite code related to system call exits
> On Nov. 14, 2016, 7:19 p.m., Michael LeBeane wrote: > > src/sim/futex_map.hh, line 81 > > <http://reviews.gem5.org/r/3702/diff/2/?file=63440#file63440line81> > > > > Why not something simple, like: > > > > return std::hash(addr) ^ (std::hash(tgid) << 1) > > Brandon Potter wrote: > The return value has to be size_t which from the C standard will at least > have a max value of 65536 > (http://stackoverflow.com/questions/918787/whats-sizeofsize-t-on-32-bit-vs-the-various-64-bit-data-models). > The key values are uint64_t. Since the value is cap starts at 65536, I > wanted to make sure that the entire number was being fed into the hash. > That's why I work through with the for loop that breaks uint64_t apart based > on sizeof(size_t). > > I chose the largest prime number less than the max to start as the hash > value. It could've been anything though; the choice was arbitrary. > > So, is this OK? This functor is really just a dumb widget to satisfy > std::hash. I don't expect the futex_map to grow to be so large that the hash > value matters much. Yeah, I don't think the hash matters to much, which is why I prefer something simple. Don't care too much though, do what you prefer. - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3702/#review9052 --- On Nov. 14, 2016, 9:30 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3702/ > --- > > (Updated Nov. 14, 2016, 9:30 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11728:9ca77be78b02 > --- > syscall_emul: [PATCH 21/22] rewrite code related to system call exits > > The changeset refactors exit, exit_group, and futex related exit > functionality. > > > Diffs > - > > src/sim/syscall_emul.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/se_signal.hh PRE-CREATION > src/sim/futex_map.hh PRE-CREATION > src/sim/process.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/syscall_emul.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/system.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/se_signal.cc PRE-CREATION > > Diff: http://reviews.gem5.org/r/3702/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3680: syscall_emul: [patch 13/22] add system call retry capability
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3680/#review9066 --- I think the commit message could be a bit clearer about the problem this patch is attempting to solve. The key issue seems to be that you might be trying to simulate multiple threads on a single host thread. Therefore you can't implement blocking system calls that rely on signaling from other threads by spinning/blocking in the syscall code. So this patch forces the system call to bump down the event queue every X cycles so that other simulated threads can make forward progress. - Michael LeBeane On Nov. 7, 2016, 11:20 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3680/ > --- > > (Updated Nov. 7, 2016, 11:20 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11702:149a5af69d1a > --- > syscall_emul: [patch 13/22] add system call retry capability > > This changeset adds functionality that allows system calls to retry without > affecting system state during on system call returns. > > We need this functionality to solve problems with blocking system calls > in multi-process or multi-threaded simulations where information is passed > between processes/threads. > > For example, consider two processes that are producer/consumer. They can > use file descriptors and read/write to pass the information between one > another. However if the consumer calls the blocking read system call before > the producer has produced anything, the call will block the event queue and > deadlock the simulation. The trick it to recognize that the system calls will > block and then reschedule at some later point when the call would succeed > without blocking. In subsequent patches, we recognize that a syscall will' > block by calling a non-blocking poll and check for events. > > > Diffs > - > > src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscall_desc.hh PRE-CREATION > src/sim/syscall_desc.cc PRE-CREATION > src/cpu/checker/cpu.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/checker/thread_context.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/exec_context.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/minor/exec_context.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/o3/commit.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/o3/commit_impl.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/o3/cpu.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/o3/cpu.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/o3/dyn_inst.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/o3/dyn_inst_impl.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/o3/thread_context.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/o3/thread_state.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/simple/atomic.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/simple/exec_context.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/simple/timing.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/simple_thread.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/thread_context.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/faults.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/faults.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/pseudo_inst.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/BaseCPU.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/base.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/base.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/faults.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/mips/isa/decoder.isa 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/power/isa/decoder.isa 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/isa/decoder/one_byte_opcodes.isa > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/isa/decoder/two_byte_opcodes.isa > 4a86763c0b30cccba0f56c7f48637a46a4663b06
Re: [gem5-dev] Review Request 3681: syscall_emul: [patch 14/22] adds identifier system calls
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3681/#review9064 --- Ship it! Ship It! - Michael LeBeane On Nov. 14, 2016, 7:48 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3681/ > --- > > (Updated Nov. 14, 2016, 7:48 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11721:831409e010d1 > --- > syscall_emul: [patch 14/22] adds identifier system calls > > This changeset add fields to the process object and adds the following > three system calls: setpgid, gettid, getpid. > > > Diffs > - > > src/arch/x86/linux/process.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/Process.py c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/syscall_emul.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/syscall_emul.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/process.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/process.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/system.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > > Diff: http://reviews.gem5.org/r/3681/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3697: style: [PATCH 16/22] correct some style issues
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3697/#review9062 --- Ship it! Ship It! - Michael LeBeane On Nov. 7, 2016, 10:05 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3697/ > --- > > (Updated Nov. 7, 2016, 10:05 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11705:baf743047c38 > --- > style: [PATCH 16/22] correct some style issues > > > Diffs > - > > src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/base/loader/object_file.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/base/loader/elf_object.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscall_emul.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > > Diff: http://reviews.gem5.org/r/3697/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3698: syscall_emul: [PATCH 17/22] refactor and add functionality to open syscall family
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3698/#review9061 --- Ship it! Looks good. Please add a bit more to the commit message saying what functionality you are adding (etc/passwd special file, etc.). - Michael LeBeane On Nov. 7, 2016, 10:07 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3698/ > --- > > (Updated Nov. 7, 2016, 10:07 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11706:693893fbee1f > --- > syscall_emul: [PATCH 17/22] refactor and add functionality to open syscall > family > > > Diffs > - > > src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/linux/linux.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/linux/linux.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > > Diff: http://reviews.gem5.org/r/3698/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3699: syscall_emul: [PATCH 18/22] refactor and add functionality for dup, dup2, and pipe
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3699/#review9060 --- src/sim/syscall_emul.cc (line 222) <http://reviews.gem5.org/r/3699/#comment7797> Seems like an unrelated bug fix, should this be in a seperate changeset? src/sim/syscall_emul.cc (line 236) <http://reviews.gem5.org/r/3699/#comment7796> These style changes seem out-of-scope. - Michael LeBeane On Nov. 7, 2016, 10:09 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3699/ > --- > > (Updated Nov. 7, 2016, 10:09 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11707:01d31dbe35c4 > --- > syscall_emul: [PATCH 18/22] refactor and add functionality for dup, dup2, and > pipe > > > Diffs > - > > src/arch/arm/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/power/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscall_emul.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > > Diff: http://reviews.gem5.org/r/3699/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3700: syscall_emul: [PATCH 19/22] adds basic signaling mechanism to SE mode
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3700/#review9059 --- Where is this used? src/sim/se_signal.hh (line 39) <http://reviews.gem5.org/r/3700/#comment7795> LiveProcess is gone now, right? - Michael LeBeane On Nov. 14, 2016, 6:56 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3700/ > --- > > (Updated Nov. 14, 2016, 6:56 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11726:a99acbcd61b4 > --- > syscall_emul: [PATCH 19/22] adds basic signaling mechanism to SE mode > > > Diffs > - > > src/sim/system.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/se_signal.cc PRE-CREATION > src/sim/se_signal.hh PRE-CREATION > src/sim/SConscript c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > > Diff: http://reviews.gem5.org/r/3700/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3701: syscall_emul: [PATCH 20/22] add the tgkill system call
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3701/#review9058 --- src/sim/syscall_emul.hh (line 2043) <http://reviews.gem5.org/r/3701/#comment7794> I imagine this is no longer needed after your LiveProcess removal patch? - Michael LeBeane On Nov. 7, 2016, 10:10 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3701/ > --- > > (Updated Nov. 7, 2016, 10:10 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11709:4a901f027df3 > --- > syscall_emul: [PATCH 20/22] add the tgkill system call > > > Diffs > - > > src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > > Diff: http://reviews.gem5.org/r/3701/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3703: syscall_emul: [PATCH 22/22] ignore system calls that are unimplemented.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3703/#review9057 --- Ship it! OK with me after you beef up commit message. - Michael LeBeane On Nov. 7, 2016, 10:57 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3703/ > --- > > (Updated Nov. 7, 2016, 10:57 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11711:6fd39a6e6db8 > --- > syscall_emul: [PATCH 22/22] ignore system calls that are unimplemented. > > > Diffs > - > > src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > > Diff: http://reviews.gem5.org/r/3703/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3702: syscall_emul: [PATCH 21/22] rewrite code related to system call exits
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3702/#review9052 --- src/sim/futex_map.hh (line 65) <http://reviews.gem5.org/r/3702/#comment7789> I'm not sure if this is needed. src/sim/futex_map.hh (line 81) <http://reviews.gem5.org/r/3702/#comment7791> Why not something simple, like: return std::hash(addr) ^ (std::hash(tgid) << 1) src/sim/system.hh (line 73) <http://reviews.gem5.org/r/3702/#comment7793> spurious whitespace change - Michael LeBeane On Nov. 14, 2016, 6:49 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3702/ > --- > > (Updated Nov. 14, 2016, 6:49 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11728:be8e9fbd0bec > --- > syscall_emul: [PATCH 21/22] rewrite code related to system call exits > > The changeset refactors exit, exit_group, and futex related exit > functionality. > > > Diffs > - > > src/sim/se_signal.hh PRE-CREATION > src/sim/se_signal.cc PRE-CREATION > src/sim/syscall_emul.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/syscall_emul.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/system.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/sim/futex_map.hh PRE-CREATION > src/sim/process.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > > Diff: http://reviews.gem5.org/r/3702/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] changeset in gem5: ruby: Allow multiple outstanding DMA requests
changeset 0bf388858d1e in /z/repo/gem5 details: http://repo.gem5.org/gem5?cmd=changeset;node=0bf388858d1e description: ruby: Allow multiple outstanding DMA requests DMA sequencers and protocols can currently only issue one DMA access at a time. This patch implements the necessary functionality to support multiple outstanding DMA requests in Ruby. diffstat: src/mem/protocol/MESI_Two_Level-dma.sm | 84 src/mem/protocol/MI_example-dma.sm | 86 +++- src/mem/protocol/MOESI_CMP_directory-dma.sm | 4 +- src/mem/protocol/MOESI_CMP_token-dma.sm | 84 +++- src/mem/protocol/MOESI_hammer-dma.sm| 86 +++- src/mem/protocol/RubySlicc_Types.sm | 4 +- src/mem/ruby/system/DMASequencer.cc | 84 +++- src/mem/ruby/system/DMASequencer.hh | 23 +-- src/mem/ruby/system/Sequencer.py| 1 + 9 files changed, 360 insertions(+), 96 deletions(-) diffs (truncated from 859 to 300 lines): diff -r 5e7599457b97 -r 0bf388858d1e src/mem/protocol/MESI_Two_Level-dma.sm --- a/src/mem/protocol/MESI_Two_Level-dma.smWed Oct 26 22:48:33 2016 -0400 +++ b/src/mem/protocol/MESI_Two_Level-dma.smWed Oct 26 22:48:37 2016 -0400 @@ -50,15 +50,38 @@ Ack, desc="DMA write to memory completed"; } - State cur_state; + structure(TBE, desc="...") { +State TBEState,desc="Transient state"; +DataBlock DataBlk, desc="Data"; + } + + structure(TBETable, external = "yes") { +TBE lookup(Addr); +void allocate(Addr); +void deallocate(Addr); +bool isPresent(Addr); + } + + void set_tbe(TBE b); + void unset_tbe(); + void wakeUpAllBuffers(); + + TBETable TBEs, template="", constructor="m_number_of_TBEs"; + Tick clockEdge(); - State getState(Addr addr) { -return cur_state; + State getState(TBE tbe, Addr addr) { +if (is_valid(tbe)) { +return tbe.TBEState; +} else { +return State:READY; +} } - void setState(Addr addr, State state) { -cur_state := state; + void setState(TBE tbe, Addr addr, State state) { +if (is_valid(tbe)) { +tbe.TBEState := state; +} } AccessPermission getAccessPermission(Addr addr) { @@ -82,9 +105,9 @@ if (dmaRequestQueue_in.isReady(clockEdge())) { peek(dmaRequestQueue_in, SequencerMsg) { if (in_msg.Type == SequencerRequestType:LD ) { - trigger(Event:ReadRequest, in_msg.LineAddress); + trigger(Event:ReadRequest, in_msg.LineAddress, TBEs[in_msg.LineAddress]); } else if (in_msg.Type == SequencerRequestType:ST) { - trigger(Event:WriteRequest, in_msg.LineAddress); + trigger(Event:WriteRequest, in_msg.LineAddress, TBEs[in_msg.LineAddress]); } else { error("Invalid request type"); } @@ -96,9 +119,11 @@ if (dmaResponseQueue_in.isReady(clockEdge())) { peek( dmaResponseQueue_in, ResponseMsg) { if (in_msg.Type == CoherenceResponseType:ACK) { - trigger(Event:Ack, makeLineAddress(in_msg.addr)); + trigger(Event:Ack, makeLineAddress(in_msg.addr), + TBEs[makeLineAddress(in_msg.addr)]); } else if (in_msg.Type == CoherenceResponseType:DATA) { - trigger(Event:Data, makeLineAddress(in_msg.addr)); + trigger(Event:Data, makeLineAddress(in_msg.addr), + TBEs[makeLineAddress(in_msg.addr)]); } else { error("Invalid response type"); } @@ -133,15 +158,30 @@ } action(a_ackCallback, "a", desc="Notify dma controller that write request completed") { -dma_sequencer.ackCallback(); +dma_sequencer.ackCallback(address); } action(d_dataCallback, "d", desc="Write data to dma sequencer") { -peek (dmaResponseQueue_in, ResponseMsg) { - dma_sequencer.dataCallback(in_msg.DataBlk); +dma_sequencer.dataCallback(tbe.DataBlk, address); + } + + action(t_updateTBEData, "t", desc="Update TBE Data") { +assert(is_valid(tbe)); +peek( dmaResponseQueue_in, ResponseMsg) { +tbe.DataBlk := in_msg.DataBlk; } } + action(v_allocateTBE, "v", desc="Allocate TBE entry") { +TBEs.allocate(address); +set_tbe(TBEs[address]); + } + + action(w_deallocateTBE, "w", desc="Deallocate TBE entry") { +TBEs.deallocate(address); +unset_tbe(); + } + action(p_popRequestQueue, "p", desc="Pop request queue") { dmaRequestQueue_in.dequeue(clockEdge()); } @@ -150,23 +190,43 @@ dmaResponseQueue_in.dequeue(clockEdge()); } + action(zz_stallAndWaitRequestQueue, "zz", desc="...") { +stall_and_wait(dmaRequestQueue_in, address); + } + + action(wkad_wakeUpAllDependents, "wkad", desc="wake-up all dependents") { +wakeUpAllBuffers(); + } + transition(READY, ReadRequest, BUSY_RD) { +v_allocateTBE; s_sendReadRequest; p_popRequestQueue
Re: [gem5-dev] Review Request 3671: syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3671/#review8951 --- Ship it! Ship It! - Michael LeBeane On Oct. 19, 2016, 7:27 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3671/ > --- > > (Updated Oct. 19, 2016, 7:27 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11694:c915cc7cff42 > --- > syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead > > The EIOProcess class was removed recently and it was the only other class > which derived from Process. Since every Process invocation is also a > LiveProcess invocation, it makes sense to simplify the who organization > by combining the fields from LiveProcess into Process. > > > Diffs > - > > configs/common/cpu2000.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 > configs/example/apu_se.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 > configs/example/se.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 > configs/learning_gem5/part1/simple.py > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > configs/learning_gem5/part1/two_level.py > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > configs/splash2/cluster.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 > configs/splash2/run.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/alpha/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/alpha/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/alpha/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/alpha/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/alpha/tru64/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/alpha/tru64/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/freebsd/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/freebsd/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/mips/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/mips/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/mips/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/mips/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/power/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/power/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/power/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/power/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/faults.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/linux/syscalls.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/solaris/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/solaris/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/gpu-compute/cl_driver.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/gpu-compute/cl_driver.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/freebsd/freebsd.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/linux/linux.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/linux/linux.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/operatingsystem.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/operatingsystem.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/Process.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/emul_driver.hh 4a86763c0b30cccba0f56
Re: [gem5-dev] Review Request 3676: syscall_emul: [patch 10/22] refactor fdentry and add fdarray class
> On Oct. 17, 2016, 10:17 p.m., Michael LeBeane wrote: > > src/sim/process.cc, line 447 > > <http://reviews.gem5.org/r/3676/diff/1/?file=61684#file61684line447> > > > > Why remove? Does this not work anymore? > > Brandon Potter wrote: > The subscript accessor returns the fdentry directly so it has to discard > the const qualifier for the (un)serialize methods. > > Also, the checkpoints are broken so I'd like to revisit getting them > working a single patch or a subset of patches. I don't think it's > constructive to try to attack the checkpoint problem without having a clear > goal on how to handle the outliers and things that make it general in > difficult. For now, I'm ignoring the feature. > > If you spend enough time looking at this patch and some of the subsequent > ones (look at the fcntl patch and the flags field for an example), it becomes > obvious that several of the fields in these classes are solely meant to hold > the metadata needed for checkpoints. This seems contradictory given that I'm > ignoring the feature. My goal right now is to make it apparent what types > we're dealing with in the system call code, but also to make it possible to > do checkpoints properly in the future by giving the metadata a structure to > reside in. Ok sure. I don't mind that checkpoints are a work in progress, but I would suggest adding some warnings since the code is only partially implemented. - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3676/#review8881 --- On Oct. 17, 2016, 3:41 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3676/ > --- > > (Updated Oct. 17, 2016, 3:41 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11698:502f94aa9638 > --- > syscall_emul: [patch 10/22] refactor fdentry and add fdarray class > > Several large changes happen in this patch. > > The FDEntry class is rewritten so that file descriptors now correspond to > types: 'Regular' which is normal file-backed file with the file open on the > host machine, 'Pipe' which is a pipe that has been opened on the host machine, > and 'Device' which does not have an open file on the host yet acts as a pseudo > device with which to issue ioctls. Other types which might be added in the > future are directory entries and sockets (off the top of my head). > > The FDArray class was create to hold most of the file descriptor handling > that was stuffed into the Process class. It uses shared pointers and > the std::array type to hold the FDEntries mentioned above. The implementation > could use a review; I feel that there's some room for improvement, but it > seems like a decent first step. > > The changes to these two classes needed to be propagated out to the rest > of the code so there were quite a few changes for that. Also, comments were > added where I thought they were needed to help others and extend our > DOxygen coverage. > > > Diffs > - > > src/sim/syscall_emul.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/fd_array.hh PRE-CREATION > src/sim/fd_array.cc PRE-CREATION > src/sim/fd_entry.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/fd_entry.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/SConscript 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > > Diff: http://reviews.gem5.org/r/3676/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3493: dev: Add 'simLength' parameter in EthPacketData
> On Oct. 3, 2016, 4:27 p.m., Andreas Sandberg wrote: > > src/dev/net/etherpkt.cc, line 44 > > <http://reviews.gem5.org/r/3493/diff/2/?file=56881#file56881line44> > > > > I really don't like the way this is breaking checkpoints. > > > > The right thing to do here would be to implement this in a way that > > doesn't break checkpoint. The simplest way would probably be to use length > > as the name for dataLength. If simLength is present (I'm going to push a > > helper function to test for that shortly), you just unserialize the value. > > If not, you use a fallback such as simLength = dataLength. > > > > The other option is to provide a checkpoint updater, which is pretty > > tricky since you need to find all places where packets are serialized. > > Michael LeBeane wrote: > The first option sounds reasonable. Did you push the helper function > that tests if something is present in a checkpoint? I don't think I've seen > it in the commit logs recently. > > Andreas Sandberg wrote: > I committed the helper functions last week > (https://github.com/gem5/gem5/commit/18135ce6abc0ee02e36aef424be183cd7238a55). > I thought a bit more about this and the optParamIn helper probably > implements the functionality you need. I reposted using your suggestions. Does this work for you? Thanks! - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3493/#review8763 --- On Oct. 12, 2016, 3:50 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3493/ > --- > > (Updated Oct. 12, 2016, 3:50 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11637:cd6bb67002fa > --- > dev: Add 'simLength' parameter in EthPacketData > Currently, all the network devices create a 16K buffer for the 'data' field > in EthPacketData, and use 'length' to keep track of the size of the packet > in the buffer. This patch introduces the 'simLength' parameter to > EthPacketData, which is used to hold the effective length of the packet used > for all timing calulations in the simulator. Serialization is performed using > only the useful data in the packet ('length') and not necessarily the entire > original buffer. > > > Diffs > - > > src/dev/net/i8254xGBe.cc e92bf392bf4302e2e88e19907e7fc981f59e777d > src/dev/net/ns_gige.cc e92bf392bf4302e2e88e19907e7fc981f59e777d > src/dev/net/pktfifo.cc e92bf392bf4302e2e88e19907e7fc981f59e777d > src/dev/net/dist_etherlink.cc e92bf392bf4302e2e88e19907e7fc981f59e777d > src/dev/net/dist_iface.cc e92bf392bf4302e2e88e19907e7fc981f59e777d > src/dev/net/dist_packet.hh e92bf392bf4302e2e88e19907e7fc981f59e777d > src/dev/net/etherbus.cc e92bf392bf4302e2e88e19907e7fc981f59e777d > src/dev/net/etherlink.cc e92bf392bf4302e2e88e19907e7fc981f59e777d > src/dev/net/etherpkt.hh e92bf392bf4302e2e88e19907e7fc981f59e777d > src/dev/net/etherpkt.cc e92bf392bf4302e2e88e19907e7fc981f59e777d > src/dev/net/etherswitch.cc e92bf392bf4302e2e88e19907e7fc981f59e777d > src/dev/net/ethertap.cc e92bf392bf4302e2e88e19907e7fc981f59e777d > src/dev/net/tcp_iface.cc e92bf392bf4302e2e88e19907e7fc981f59e777d > src/dev/net/sinic.cc e92bf392bf4302e2e88e19907e7fc981f59e777d > > Diff: http://reviews.gem5.org/r/3493/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3676: syscall_emul: [patch 10/22] refactor fdentry and add fdarray class
> On Oct. 17, 2016, 10:17 p.m., Michael LeBeane wrote: > > src/sim/fd_entry.hh, line 140 > > <http://reviews.gem5.org/r/3676/diff/1/?file=61681#file61681line140> > > > > Can you rename this to something more descriptive, like > > HostMappedFDEntry? > > Brandon Potter wrote: > The only issue is that pipes are "host mapped" too. How about > FileBackedFDEntry? Sure, that's even better. - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3676/#review8881 --- On Oct. 17, 2016, 3:41 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3676/ > --- > > (Updated Oct. 17, 2016, 3:41 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11698:502f94aa9638 > --- > syscall_emul: [patch 10/22] refactor fdentry and add fdarray class > > Several large changes happen in this patch. > > The FDEntry class is rewritten so that file descriptors now correspond to > types: 'Regular' which is normal file-backed file with the file open on the > host machine, 'Pipe' which is a pipe that has been opened on the host machine, > and 'Device' which does not have an open file on the host yet acts as a pseudo > device with which to issue ioctls. Other types which might be added in the > future are directory entries and sockets (off the top of my head). > > The FDArray class was create to hold most of the file descriptor handling > that was stuffed into the Process class. It uses shared pointers and > the std::array type to hold the FDEntries mentioned above. The implementation > could use a review; I feel that there's some room for improvement, but it > seems like a decent first step. > > The changes to these two classes needed to be propagated out to the rest > of the code so there were quite a few changes for that. Also, comments were > added where I thought they were needed to help others and extend our > DOxygen coverage. > > > Diffs > - > > src/sim/syscall_emul.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/fd_array.hh PRE-CREATION > src/sim/fd_array.cc PRE-CREATION > src/sim/fd_entry.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/fd_entry.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/SConscript 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > > Diff: http://reviews.gem5.org/r/3676/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3676: syscall_emul: [patch 10/22] refactor fdentry and add fdarray class
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3676/#review8881 --- src/sim/fd_array.hh (line 109) <http://reviews.gem5.org/r/3676/#comment7642> This seems weird. I'm pretty sure the '=' operator has access to all the private members and that this is not needed. src/sim/fd_array.cc (line 75) <http://reviews.gem5.org/r/3676/#comment7639> Do you need to cast here? src/sim/fd_array.cc (line 88) <http://reviews.gem5.org/r/3676/#comment7641> same here src/sim/fd_array.cc (line 99) <http://reviews.gem5.org/r/3676/#comment7640> same here src/sim/fd_entry.hh (line 67) <http://reviews.gem5.org/r/3676/#comment7638> Maybe mark as TODO? Should also put a warn when trying to (un)serialize if you don't think it will work correctly. src/sim/fd_entry.hh (line 128) <http://reviews.gem5.org/r/3676/#comment7636> Can you rename this to something more descriptive, like HostMappedFDEntry? src/sim/fd_entry.hh (line 156) <http://reviews.gem5.org/r/3676/#comment7637> If you move simFD to the base class you can avoid a lot of your downcasts later. I don't think it's a big deal that DeviceFDEntry doesn't use it. src/sim/fd_entry.cc (line 33) <http://reviews.gem5.org/r/3676/#comment7643> Should probably just add your name here, not delete the others. src/sim/process.cc (line 245) <http://reviews.gem5.org/r/3676/#comment7644> Why remove? Does this not work anymore? src/sim/syscall_emul.hh (line 631) <http://reviews.gem5.org/r/3676/#comment7645> These downcasts are messy, see above for suggestion on getting rid of them. - Michael LeBeane On Oct. 17, 2016, 3:41 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3676/ > --- > > (Updated Oct. 17, 2016, 3:41 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11698:502f94aa9638 > --- > syscall_emul: [patch 10/22] refactor fdentry and add fdarray class > > Several large changes happen in this patch. > > The FDEntry class is rewritten so that file descriptors now correspond to > types: 'Regular' which is normal file-backed file with the file open on the > host machine, 'Pipe' which is a pipe that has been opened on the host machine, > and 'Device' which does not have an open file on the host yet acts as a pseudo > device with which to issue ioctls. Other types which might be added in the > future are directory entries and sockets (off the top of my head). > > The FDArray class was create to hold most of the file descriptor handling > that was stuffed into the Process class. It uses shared pointers and > the std::array type to hold the FDEntries mentioned above. The implementation > could use a review; I feel that there's some room for improvement, but it > seems like a decent first step. > > The changes to these two classes needed to be propagated out to the rest > of the code so there were quite a few changes for that. Also, comments were > added where I thought they were needed to help others and extend our > DOxygen coverage. > > > Diffs > - > > src/sim/syscall_emul.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/fd_array.hh PRE-CREATION > src/sim/fd_array.cc PRE-CREATION > src/sim/fd_entry.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/fd_entry.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/SConscript 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > > Diff: http://reviews.gem5.org/r/3676/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3674: syscall_emul: [patch 8/22] refactor process class
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3674/#review8879 --- Ship it! Ship It! - Michael LeBeane On Oct. 17, 2016, 3:23 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3674/ > --- > > (Updated Oct. 17, 2016, 3:23 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11696:c48d378a972a > --- > syscall_emul: [patch 8/22] refactor process class > > Moves aux_vector into its own .hh and .cc files just to get it out of the > already crowded Process files. Arguably, it could stay there, but it's > probably better just to move it and give it files. > > The changeset looks ugly around the Process header file, but the goal here is > to move methods and members around so that they're not defined randomly > throughout the entire header file. I expect this is likely one of the reasons > why I several unused variables related to this class. So, the methods are > declared first followed by members. I've tried to aggregate them together > so that similar entries reside near one another. > > There are other changes coming to this code so this is by no means the > final product. > > > Diffs > - > > src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/aux_vector.cc PRE-CREATION > src/sim/aux_vector.hh PRE-CREATION > src/sim/SConscript 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/power/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/mips/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/alpha/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > > Diff: http://reviews.gem5.org/r/3674/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3673: syscall_emul: [patch 7/22] remove numCpus method
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3673/#review8878 --- src/sim/process.hh <http://reviews.gem5.org/r/3673/#comment7632> Can you just rename this to numThreads() instead of making the num_threads variable? - Michael LeBeane On Oct. 17, 2016, 3:20 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3673/ > --- > > (Updated Oct. 17, 2016, 3:20 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11695:b71b3bd60a70 > --- > syscall_emul: [patch 7/22] remove numCpus method > > The numCpus method is misleading in that it's not really a measure of > how many CPUs might be executing a process, but how many thread contexts > are assigned to the process at any given point in time. > > It's nice to highlight this distinction because thread contexts are never > reused in the same way that a CPU can be reused for multiple processes. > The reason that there is no reuse is that there is no CPU scheduler for SE. > > The tru64 code intends to use this method and the accompanying contextIDs > field to support SMT and track the number of threads with some system calls. > With the up coming clone and exec patches, this paradigm must change. There > needs to be a 1:1 mapping between the thread contexts and processes so that > the process state between threads is allowed to vary when needed by Linux. > This should not break SMT for tru64 if the Process class is refactored so that > multiple Processes can share state between themselves. The following patches > will do the refactoring incrementally as features are added. > > > Diffs > - > > src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/alpha/tru64/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > > Diff: http://reviews.gem5.org/r/3673/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3679: style: [patch 12/22] fix preliminary style issues for subsequent fault patch
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3679/#review8877 --- Ship it! Ship It! - Michael LeBeane On Oct. 17, 2016, 4:11 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3679/ > --- > > (Updated Oct. 17, 2016, 4:11 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11700:db830c773c2d > --- > style: [patch 12/22] fix preliminary style issues for subsequent fault patch > > This changeset add spaces in a few spots and removes an unnecessary comment. > > > Diffs > - > > src/sim/faults.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > > Diff: http://reviews.gem5.org/r/3679/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3675: syscall_emul: [patch 9/22] remove unused global variable (num_processes)
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3675/#review8867 --- Ship it! Ship It! - Michael LeBeane On Oct. 17, 2016, 3:25 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3675/ > --- > > (Updated Oct. 17, 2016, 3:25 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11697:e818bb00b9ea > --- > syscall_emul: [patch 9/22] remove unused global variable (num_processes) > > > Diffs > - > > src/arch/sparc/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > > Diff: http://reviews.gem5.org/r/3675/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3672: syscall_emul: [patch 6/22] remove unused fields from Process class
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3672/#review8865 --- Ship it! Ship It! - Michael LeBeane On Oct. 17, 2016, 3:19 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3672/ > --- > > (Updated Oct. 17, 2016, 3:19 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11694:ebff480ad584 > --- > syscall_emul: [patch 6/22] remove unused fields from Process class > > It looks like tru64 has some nxm* system calls, but the two fields that > are defined in the Process class are unused by any of the code. There doesn't > appear to be any reference in the tru64 code. > > > Diffs > - > > src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > > Diff: http://reviews.gem5.org/r/3672/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3671: syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3671/#review8864 --- src/arch/arm/process.hh (line 54) <http://reviews.gem5.org/r/3671/#comment7624> I don't think this is needed. src/arch/mips/process.hh (line 41) <http://reviews.gem5.org/r/3671/#comment7625> I don't think this is needed. src/arch/power/process.hh (line 42) <http://reviews.gem5.org/r/3671/#comment7628> I don't think this is needed. src/arch/sparc/faults.cc (line 673) <http://reviews.gem5.org/r/3671/#comment7627> Why remove comment? - Michael LeBeane On Oct. 17, 2016, 3:17 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3671/ > --- > > (Updated Oct. 17, 2016, 3:17 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11693:05fc28ce62a5 > --- > syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead > > The EIOProcess class was removed recently and it was the only other class > which derived from Process. Since every Process invocation is also a > LiveProcess invocation, it makes sense to simplify the who organization > by combining the fields from LiveProcess into Process. > > > Diffs > - > > src/arch/sparc/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/faults.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/power/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/power/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/power/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/power/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/mips/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/mips/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/mips/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/mips/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/freebsd/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/freebsd/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/alpha/tru64/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/alpha/tru64/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/alpha/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/alpha/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/alpha/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/alpha/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > configs/splash2/run.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 > configs/splash2/cluster.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 > configs/learning_gem5/part1/two_level.py > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > configs/learning_gem5/part1/simple.py > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > configs/example/se.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 > configs/example/apu_se.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 > configs/common/cpu2000.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/linux/syscalls.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscall_emul.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscall_desc.cc PRE-CREATION > src/sim/syscall_desc.hh PRE-CREATION > src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/emul_driver.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/Process.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/operatingsystem.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/operatingsystem.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/linux/linux.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/linux/linux.hh 4a86763c0b30
Re: [gem5-dev] Review Request 3643: style: [patch 3/22] reduce include dependencies in some headers
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3643/#review8863 --- Ship it! Ship It! - Michael LeBeane On Oct. 17, 2016, 3:12 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3643/ > --- > > (Updated Oct. 17, 2016, 3:12 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11691:ba6ddbf68d8c > --- > style: [patch 3/22] reduce include dependencies in some headers > > Used cppclean to help identify useless includes and removed them. This > involved erroneously included headers, but also cases where forward > declarations could have been used rather than a full include. > > > Diffs > - > > src/arch/alpha/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/alpha/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/alpha/tru64/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/mips/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/mips/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/power/interrupts.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/power/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/power/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/power/remote_gdb.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/isa_traits.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/pagetable.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/pseudo_inst.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/system.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/system.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/tlb.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/tlb.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/utility.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/utility.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/base/bitfield.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/base/bitunion.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/base/time.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/base/vnc/vncinput.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/minor/buffers.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/testers/directedtest/InvalidateGenerator.cc > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/testers/directedtest/RubyDirectedTester.cc > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/testers/directedtest/SeriesRequestGenerator.cc > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/testers/rubytest/Check.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/testers/rubytest/CheckTable.cc > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/arm/flash_device.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/mc146818.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/net/dist_iface.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/net/etherbus.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/net/etherswitch.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/linux/linux.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/linux/linux.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/mem/cache/prefetch/stride.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/mem/external_master.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/mem/external_slave.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/mem/mem_checker.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/mem/multi_level_page_table.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/mem/multi_level_page_table_impl.hh > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/mem/page_table.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/mem/page_table.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/mem/ruby/network/MessageBuffer.hh > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/mem/ruby/structures/AbstractReplacementPolicy.cc > 4a8676
Re: [gem5-dev] Review Request 3662: syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3662/#review8862 --- Ship it! Ship It! - Michael LeBeane On Oct. 17, 2016, 3:08 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3662/ > --- > > (Updated Oct. 17, 2016, 3:08 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11690:4bd82b7d3c09 > --- > syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc > > The class was crammed into syscall_emul.hh which has tons of forward > declarations and template definitions. To clean it up a bit, moved the > class into separate files and commented the class with doxygen style > comments. Also, provided some encapsulation by adding some accessors and > a mutator. > > The syscallreturn.hh file was renamed syscall_return.hh to make it consistent > with other similarly named files in the src/sim directory. > > The DPRINTF_SYSCALL macro was moved into its own header file with the > include the Base and Verbose flags as well. > > > Diffs > - > > src/arch/alpha/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/freebsd/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/arm/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/mips/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/power/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/linux/syscalls.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/sparc/solaris/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/SConscript 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscall_debug_macros.hh PRE-CREATION > src/sim/syscall_desc.hh PRE-CREATION > src/sim/syscall_desc.cc PRE-CREATION > src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscall_emul.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/sim/syscallreturn.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > > Diff: http://reviews.gem5.org/r/3662/diff/ > > > Testing > --- > > > Thanks, > > Brandon Potter > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3660: style: [patch 1/22] use /r/3648/ to reorganize includes
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3660/#review8861 --- Ship it! Ship It! - Michael LeBeane On Oct. 17, 2016, 3:07 p.m., Brandon Potter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3660/ > --- > > (Updated Oct. 17, 2016, 3:07 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11689:579f5d74b7a6 > --- > style: [patch 1/22] use /r/3648/ to reorganize includes > > > Diffs > - > > src/cpu/static_inst.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/testers/directedtest/DirectedGenerator.cc > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/testers/directedtest/InvalidateGenerator.cc > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/testers/directedtest/RubyDirectedTester.cc > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/testers/directedtest/SeriesRequestGenerator.cc > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/testers/garnet_synthetic_traffic/GarnetSyntheticTraffic.cc > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/testers/memtest/memtest.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/testers/rubytest/Check.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/testers/rubytest/CheckTable.cc > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/testers/rubytest/RubyTester.cc > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/testers/traffic_gen/generators.cc > 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/thread_context.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/thread_state.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/cpu/timing_expr.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/alpha/backdoor.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/alpha/tsunami.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/alpha/tsunami_cchip.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/alpha/tsunami_io.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/arm/a9scu.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/arm/amba_device.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/arm/amba_fake.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/arm/energy_ctrl.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/arm/gic_pl390.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/arm/hdlcd.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/arm/kmi.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/arm/pl011.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/arm/pl111.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/arm/realview.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/arm/rtc_pl031.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/arm/timer_cpulocal.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/arm/timer_sp804.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/arm/vgic.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/baddev.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/intel_8254_timer.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/io_device.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/isa_fake.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/mc146818.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/mips/malta.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/mips/malta_cchip.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/mips/malta_io.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/pci/device.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/pci/host.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/platform.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/ps2.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/ps2.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/sparc/dtod.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/sparc/iob.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/sparc/mm_disk.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/sparc/t1000.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/uart.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/uart8250.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/virtio/base.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 > src/dev/virtio/block.cc 4a86763c0b30c
Re: [gem5-dev] Review Request 3595: dev: Add m5 op to toggle synchronization for dist-gem5.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3595/ --- (Updated Oct. 12, 2016, 3:51 p.m.) Review request for Default. Repository: gem5 Description --- Changeset 11561:1414a40eb1e2 --- dev: Add m5 op to toggle synchronization for dist-gem5. This patch adds the ability for an application to request dist-gem5 to begin/ end synchronization using an m5 op. When toggling on sync, all nodes agree on the next sync point based on the maximum of all nodes' ticks. CPUs are suspended until the sync point to avoid sending network messages until sync has been enabled. Toggling off sync acts like a global execution barrier, where all CPUs are disabled until every node reaches the toggle off point. This avoids tricky situations such as one node hitting a toggle off followed by a toggle on before the other nodes hit the first toggle off. Diffs (updated) - util/m5/m5op_x86.S e92bf392bf4302e2e88e19907e7fc981f59e777d util/m5/m5ops.h e92bf392bf4302e2e88e19907e7fc981f59e777d util/m5/m5op.h e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/dist_iface.hh e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/dist_etherlink.cc e92bf392bf4302e2e88e19907e7fc981f59e777d src/arch/x86/isa/decoder/two_byte_opcodes.isa e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/Ethernet.py e92bf392bf4302e2e88e19907e7fc981f59e777d configs/common/Options.py e92bf392bf4302e2e88e19907e7fc981f59e777d src/sim/pseudo_inst.cc e92bf392bf4302e2e88e19907e7fc981f59e777d src/sim/pseudo_inst.hh e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/tcp_iface.cc e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/tcp_iface.hh e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/dist_packet.hh e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/dist_iface.cc e92bf392bf4302e2e88e19907e7fc981f59e777d Diff: http://reviews.gem5.org/r/3595/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3595: dev: Add m5 op to toggle synchronization for dist-gem5.
> On Sept. 9, 2016, 2:53 p.m., Gabor Dozsa wrote: > > src/dev/net/dist_iface.cc, line 71 > > <http://reviews.gem5.org/r/3595/diff/1/?file=57400#file57400line71> > > > > This change effectively disables the use of the start_tick parameter. > > It is because 'nextAt' is overwritten in SyncEvent::start() by the max tick > > of all participating gem5 - which will be 0 at the first global barrier (if > > we do not start the sync on pseudo op). > > > > However, I think the pseudo op is a much better way to defer the sync > > start. Could you just remove the 'start_tick' option altogether? I did a quick test and the start_tick approach still appears to work with this patch. nextAt is assigned the value of start_tick for all the nodes and the switch in Synch::init(). SyncSwitch::progress() only updates nextAt(which determines the max tick of all paticipating processes) if it is less than send_tick. At time zero this is not true, so the first global barrier will be scheduled at start_tick. - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3595/#review8713 ----------- On Aug. 4, 2016, 4:51 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3595/ > --- > > (Updated Aug. 4, 2016, 4:51 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11561:1414a40eb1e2 > --- > dev: Add m5 op to toggle synchronization for dist-gem5. > This patch adds the ability for an application to request dist-gem5 to begin/ > end synchronization using an m5 op. When toggling on sync, all nodes agree on > the next sync point based on the maximum of all nodes' ticks. CPUs are > suspended until the sync point to avoid sending network messages until sync > has > been enabled. Toggling off sync acts like a global execution barrier, where > all CPUs are disabled until every node reaches the toggle off point. This > avoids tricky situations such as one node hitting a toggle off followed by a > toggle on before the other nodes hit the first toggle off. > > > Diffs > - > > configs/common/Options.py 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/arch/x86/isa/decoder/two_byte_opcodes.isa > 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/Ethernet.py 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_etherlink.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_iface.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_packet.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/tcp_iface.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/tcp_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/sim/pseudo_inst.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/sim/pseudo_inst.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5op.h 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5op_x86.S 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5ops.h 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3595/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3493: dev: Add 'simLength' parameter in EthPacketData
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3493/ --- (Updated Oct. 12, 2016, 3:50 p.m.) Review request for Default. Summary (updated) - dev: Add 'simLength' parameter in EthPacketData Repository: gem5 Description (updated) --- Changeset 11637:cd6bb67002fa --- dev: Add 'simLength' parameter in EthPacketData Currently, all the network devices create a 16K buffer for the 'data' field in EthPacketData, and use 'length' to keep track of the size of the packet in the buffer. This patch introduces the 'simLength' parameter to EthPacketData, which is used to hold the effective length of the packet used for all timing calulations in the simulator. Serialization is performed using only the useful data in the packet ('length') and not necessarily the entire original buffer. Diffs (updated) - src/dev/net/i8254xGBe.cc e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/ns_gige.cc e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/pktfifo.cc e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/dist_etherlink.cc e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/dist_iface.cc e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/dist_packet.hh e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/etherbus.cc e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/etherlink.cc e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/etherpkt.hh e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/etherpkt.cc e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/etherswitch.cc e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/ethertap.cc e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/tcp_iface.cc e92bf392bf4302e2e88e19907e7fc981f59e777d src/dev/net/sinic.cc e92bf392bf4302e2e88e19907e7fc981f59e777d Diff: http://reviews.gem5.org/r/3493/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3591: ruby: Allow multiple outstanding DMA requests
> On Sept. 1, 2016, 3:33 p.m., Jason Lowe-Power wrote: > > What testing did you perform to make sure all of the protocols were > > modified correctly? > > > > Most of these changes seem reasonable to me, but I know from experience > > that even when the SLICC changes seem like they are right, if they aren't > > tested carefully there's almost always bugs. > > Michael LeBeane wrote: > The sequencer changes have been tested pretty thoroughly in a custom > protocol; the public SLICC files only with the regression tester. I'm not > sure how much coverage that provides for DMA other than checking if it > compiles. > > Jason Lowe-Power wrote: > I'm not sure what to do about this. Maybe others in the community will > speak up ;). > > I don't feel comfortable pushing a patch with code that we know hasn't > been tested at all. Specifically with Ruby/SLICC, this has bitten me before. > I've updated gem5 and all of sudden my simluations are broken because someone > changed a protocol without testing it. IMO, code needs to be tested at least > somewhat before it's pushed into the mainline. > > However, I also understand that there isn't any testing infrastructure > for most of the protocols, and we can't ask you to solve that problem before > pushing the patch in. > > Do others have thoughts on this (reoccurring) problem? > > For this specific patch, can you run your workload that use DMA with > protocols other than your internal protocol? > > Michael LeBeane wrote: > It would take some effort, but I do have some DMA intensive networking > benchmarks that should be able to run over the public protocols. I wouldn't > be able to share these or add them to the regression tester (they are > proprietary). > > I know that doesn't help at all with the more general problem of poor > tester coverage, but would that be an acceptable solution for this patch? > > Jason Lowe-Power wrote: > I would appreciate it if you ran those tests. It doesn't bother me nearly > as much that they are proprietary tests. At least we'll know that something > executed correctly with the other protocols! Improving the coverage of the > regression tests is hugely outside of the scope for this patch. > > I'd still like to hear what others think about this problem in the longer > term. > > Andreas Hansson wrote: > Another potential issue: The queue of requests that has been created, > does it officially need to be visible to any functional access? In other > words, has it "happened" yet? For the queued ports we have functions to query > the queued packets on a functional access, but that does not seem to be > present in the patch. > > Brad Beckmann wrote: > I think Michael is a bit unsure how to move this patch forward in a > reasonable way. Here are a couple things to consider: > > - Jason, another way Michael could have approached this patch is simply > provided the changes to the DMASequencer and not touched the public > protocols. Limiting these DMA controllers to only a single outstanding > request is obviously a big bottleneck, but we don't use these protocols at > AMD. Michael really went beyond what was necessary here. If he tests the > basically functionality of the public protocol changes, I think that is > enough. We should minimize the burden for performance fixes to the public > protocols. > - Andreas, in many of your recent Ruby code reviews, you've mentioned > concerns with functional accesses. Perhaps we should arrange a separate > thread on the topic and clarify the current state of Ruby functional > accesses, as well as discuss your goals. I'm pretty sure this patch is > orthogonal to functional accesses. It is simply allowing multiple DMA > requests to be processed by the public DMA controllers. > > Jason Lowe-Power wrote: > I think this change is great, I would like to see it pushed in. I agree > that these changes are needed to get reasonable DMA performance. > > I agree that's enough to just test the basic functionality with the > public protocols. As long as these protocols have all been compiled, and any > workload that exercises DMA is run on the protocol, I'm happy. > > In the past, there have been changes to Ruby protocols that weren't > tested at all. Then, sometimes months later, users (i.e., people down the > hall from me) have noticed that the protocols were broken, in some cases they > didn't even compile! Digging back through hundreds of changes
Re: [gem5-dev] Review Request 3493: dev: Redefine 'length' in EthPacketData
> On Oct. 3, 2016, 4:27 p.m., Andreas Sandberg wrote: > > src/dev/net/etherpkt.cc, line 44 > > <http://reviews.gem5.org/r/3493/diff/2/?file=56881#file56881line44> > > > > I really don't like the way this is breaking checkpoints. > > > > The right thing to do here would be to implement this in a way that > > doesn't break checkpoint. The simplest way would probably be to use length > > as the name for dataLength. If simLength is present (I'm going to push a > > helper function to test for that shortly), you just unserialize the value. > > If not, you use a fallback such as simLength = dataLength. > > > > The other option is to provide a checkpoint updater, which is pretty > > tricky since you need to find all places where packets are serialized. The first option sounds reasonable. Did you push the helper function that tests if something is present in a checkpoint? I don't think I've seen it in the commit logs recently. - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3493/#review8763 --- On June 29, 2016, 6:27 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3493/ > --- > > (Updated June 29, 2016, 6:27 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11549:573a7d349538 > --- > dev: Redefine 'length' in EthPacketData > Currently, all the network devices create a 16K buffer for the 'data' field > in EthPacketData, and use 'length' to keep track of the size of the packet > in the buffer. This patch introduces 'dataLength' and 'simLength' parameters > to EthPacketData. 'dataLength' stores the amount of space taken up by the > packet in the 'data' buffer. 'simLength' is used to hold the effective length > of the packet used for all timing calulations in the simulator. Serialization > is performed using only the useful data in the packet ('dataLength') and not > necessarily the entire original buffer. > > > Diffs > - > > src/dev/net/etherswitch.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/ethertap.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/i8254xGBe.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/ns_gige.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/pktfifo.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/pktfifo.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/sinic.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/tcp_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherbus.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherdump.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherlink.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherpkt.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherpkt.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_etherlink.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_packet.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3493/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3595: dev: Add m5 op to toggle synchronization for dist-gem5.
> On Sept. 9, 2016, 2:53 p.m., Gabor Dozsa wrote: > > I like the idea of starting the sync on a pseudo op very much. I cannot see > > the usefulness of stopping the sync after it started though. Do you have a > > use case in mind? > > > > I have a few comments below (mainly minor nits). > > > > However, I think there is also a basic issue with the CPU suspend approach. > > A CPU can wake up whenever there is an interrupt or even a snoop request. > > That should be taken care of somehow I assume. > > Michael LeBeane wrote: > The use case for switching off would be if you have multiple regions of > interest in the code and no need to sync in between. You could also just run > the simulation a bunch of times and move the start sync point, but I think > it’s much cleaner to run the app once and toggle the sync. > > I suppose a spurious wakeup is possible; I had not really thought about > it. I haven't seen CPUs wakeup in practice, but I don't really have any > devices that would generate an interrupt/snoop when all the cores are asleep. > > One alternative way to do this would be to not suspend any of the cores > and modify the sync to support all the devices being on different ticks when > sync starts. We would need to remember what the start time was for all the > participating nodes and use that time to enforce synchronization instead of > assuming everyone is on the same tick. I avoided doing this since suspending > the CPUs until we reach the max of all nodes ticks' was easier. Before I address your specific review comments, I think it would be good to resolve the high-level concern about suspending CPUs. Do you have any thoughts about the alternative approach I suggested? I'm not a huge fan of it since having all the nodes on different ticks makes correlating traces between nodes very difficult. Did you have a better approach in mind? - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3595/#review8713 --- On Aug. 4, 2016, 4:51 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3595/ > --- > > (Updated Aug. 4, 2016, 4:51 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11561:1414a40eb1e2 > --- > dev: Add m5 op to toggle synchronization for dist-gem5. > This patch adds the ability for an application to request dist-gem5 to begin/ > end synchronization using an m5 op. When toggling on sync, all nodes agree on > the next sync point based on the maximum of all nodes' ticks. CPUs are > suspended until the sync point to avoid sending network messages until sync > has > been enabled. Toggling off sync acts like a global execution barrier, where > all CPUs are disabled until every node reaches the toggle off point. This > avoids tricky situations such as one node hitting a toggle off followed by a > toggle on before the other nodes hit the first toggle off. > > > Diffs > - > > configs/common/Options.py 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/arch/x86/isa/decoder/two_byte_opcodes.isa > 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/Ethernet.py 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_etherlink.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_iface.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_packet.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/tcp_iface.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/tcp_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/sim/pseudo_inst.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/sim/pseudo_inst.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5op.h 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5op_x86.S 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5ops.h 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3595/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3493: dev: Redefine 'length' in EthPacketData
> On Aug. 31, 2016, 5:04 p.m., Michael LeBeane wrote: > > Any more comments on this? We would like to get a few more ship it's since > > this fairly large and will break checkpoints. > > Andreas Hansson wrote: > If it breaks checkpoints there should be an update function added, or am > I missing something? > > Michael LeBeane wrote: > Hmmm, I could add an update function but I don't have any checkpoints to > test it with. Does anyone have any checkpoints out there with serialized > packets? Or are there some easy networking workloads that could be used to > generate some test checkpoints? We plan on shipping this in a week unless someone can provide a workload that create serialized network packets. Thanks! - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3493/#review8696 ------- On June 29, 2016, 6:27 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3493/ > --- > > (Updated June 29, 2016, 6:27 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11549:573a7d349538 > --- > dev: Redefine 'length' in EthPacketData > Currently, all the network devices create a 16K buffer for the 'data' field > in EthPacketData, and use 'length' to keep track of the size of the packet > in the buffer. This patch introduces 'dataLength' and 'simLength' parameters > to EthPacketData. 'dataLength' stores the amount of space taken up by the > packet in the 'data' buffer. 'simLength' is used to hold the effective length > of the packet used for all timing calulations in the simulator. Serialization > is performed using only the useful data in the packet ('dataLength') and not > necessarily the entire original buffer. > > > Diffs > - > > src/dev/net/etherswitch.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/ethertap.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/i8254xGBe.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/ns_gige.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/pktfifo.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/pktfifo.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/sinic.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/tcp_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherbus.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherdump.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherlink.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherpkt.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherpkt.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_etherlink.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_packet.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3493/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3493: dev: Redefine 'length' in EthPacketData
> On Aug. 31, 2016, 5:04 p.m., Michael LeBeane wrote: > > Any more comments on this? We would like to get a few more ship it's since > > this fairly large and will break checkpoints. > > Andreas Hansson wrote: > If it breaks checkpoints there should be an update function added, or am > I missing something? Hmmm, I could add an update function but I don't have any checkpoints to test it with. Does anyone have any checkpoints out there with serialized packets? Or are there some easy networking workloads that could be used to generate some test checkpoints? - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3493/#review8696 --- On June 29, 2016, 6:27 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3493/ > --- > > (Updated June 29, 2016, 6:27 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11549:573a7d349538 > --- > dev: Redefine 'length' in EthPacketData > Currently, all the network devices create a 16K buffer for the 'data' field > in EthPacketData, and use 'length' to keep track of the size of the packet > in the buffer. This patch introduces 'dataLength' and 'simLength' parameters > to EthPacketData. 'dataLength' stores the amount of space taken up by the > packet in the 'data' buffer. 'simLength' is used to hold the effective length > of the packet used for all timing calulations in the simulator. Serialization > is performed using only the useful data in the packet ('dataLength') and not > necessarily the entire original buffer. > > > Diffs > - > > src/dev/net/etherswitch.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/ethertap.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/i8254xGBe.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/ns_gige.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/pktfifo.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/pktfifo.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/sinic.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/tcp_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherbus.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherdump.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherlink.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherpkt.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherpkt.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_etherlink.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_packet.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3493/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] changeset in gem5: dev: Add a DmaCallback class to DmaDevice
changeset 2344c9dcc0d6 in /z/repo/gem5 details: http://repo.gem5.org/gem5?cmd=changeset;node=2344c9dcc0d6 description: dev: Add a DmaCallback class to DmaDevice This patch introduces the DmaCallback helper class, which registers a callback to fire after a sequence of (potentially non-contiguous) DMA transfers on a DmaPort completes. diffstat: src/dev/dma_device.hh | 85 +++ 1 files changed, 85 insertions(+), 0 deletions(-) diffs (95 lines): diff -r 9796e43e751d -r 2344c9dcc0d6 src/dev/dma_device.hh --- a/src/dev/dma_device.hh Tue Sep 13 23:12:46 2016 -0400 +++ b/src/dev/dma_device.hh Tue Sep 13 23:14:24 2016 -0400 @@ -185,6 +185,91 @@ }; /** + * DMA callback class. + * + * Allows one to register for a callback event after a sequence of (potentially + * non-contiguous) DMA transfers on a DmaPort completes. Derived classes must + * implement the process() method and use getChunkEvent() to allocate a + * callback event for each participating DMA. + */ +class DmaCallback : public Drainable +{ + public: +virtual const std::string name() const { return "DmaCallback"; } + +/** + * DmaPort ensures that all oustanding DMA accesses have completed before + * it finishes draining. However, DmaChunkEvents scheduled with a delay + * might still be sitting on the event queue. Therefore, draining is not + * complete until count is 0, which ensures that all outstanding + * DmaChunkEvents associated with this DmaCallback have fired. + */ +DrainState drain() override +{ +return count ? DrainState::Draining : DrainState::Drained; +} + + protected: +int count; + +DmaCallback() +: count(0) +{ } + +virtual ~DmaCallback() { } + +/** + * Callback function invoked on completion of all chunks. + */ +virtual void process() = 0; + + private: +/** + * Called by DMA engine completion event on each chunk completion. + * Since the object may delete itself here, callers should not use + * the object pointer after calling this function. + */ +void chunkComplete() +{ +if (--count == 0) { +process(); +// Need to notify DrainManager that this object is finished +// draining, even though it is immediately deleted. +signalDrainDone(); +delete this; +} +} + +/** + * Event invoked by DmaDevice on completion of each chunk. + */ +class DmaChunkEvent : public Event +{ + private: +DmaCallback *callback; + + public: +DmaChunkEvent(DmaCallback *cb) + : Event(Default_Pri, AutoDelete), callback(cb) +{ } + +void process() { callback->chunkComplete(); } +}; + + public: + +/** + * Request a chunk event. Chunks events should be provided to each DMA + * request that wishes to participate in this DmaCallback. + */ +Event *getChunkEvent() +{ +++count; +return new DmaChunkEvent(this); +} +}; + +/** * Buffered DMA engine helper class * * This class implements a simple DMA engine that feeds a FIFO ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] changeset in gem5: kvm: Support timing accesses for KVM cpu
changeset 22f08c96bf7f in /z/repo/gem5 details: http://repo.gem5.org/gem5?cmd=changeset;node=22f08c96bf7f description: kvm: Support timing accesses for KVM cpu This patch enables timing accesses for KVM cpu. A new state, RunningMMIOPending, is added to indicate that there are outstanding timing requests generated by KVM in the system. KVM's tick() is disabled and the simulation does not enter into KVM until all outstanding timing requests have completed. The main motivation for this is to allow KVM CPU to perform MMIO in Ruby, since Ruby does not support atomic accesses. diffstat: src/cpu/kvm/base.cc| 129 +--- src/cpu/kvm/base.hh| 56 src/cpu/kvm/x86_cpu.cc | 14 ++-- 3 files changed, 159 insertions(+), 40 deletions(-) diffs (truncated from 373 to 300 lines): diff -r 85011e8eaad9 -r 22f08c96bf7f src/cpu/kvm/base.cc --- a/src/cpu/kvm/base.cc Tue Sep 13 23:18:34 2016 -0400 +++ b/src/cpu/kvm/base.cc Tue Sep 13 23:20:03 2016 -0400 @@ -170,6 +170,76 @@ schedule(startupEvent, curTick()); } +BaseKvmCPU::Status +BaseKvmCPU::KVMCpuPort::nextIOState() const +{ +return (activeMMIOReqs || pendingMMIOPkts.size()) +? RunningMMIOPending : RunningServiceCompletion; +} + +Tick +BaseKvmCPU::KVMCpuPort::submitIO(PacketPtr pkt) +{ +if (cpu->system->isAtomicMode()) { +Tick delay = sendAtomic(pkt); +delete pkt->req; +delete pkt; +return delay; +} else { +if (pendingMMIOPkts.empty() && sendTimingReq(pkt)) { +activeMMIOReqs++; +} else { +pendingMMIOPkts.push(pkt); +} +// Return value is irrelevant for timing-mode accesses. +return 0; +} +} + +bool +BaseKvmCPU::KVMCpuPort::recvTimingResp(PacketPtr pkt) +{ +DPRINTF(KvmIO, "KVM: Finished timing request\n"); + +delete pkt->req; +delete pkt; +activeMMIOReqs--; + +// We can switch back into KVM when all pending and in-flight MMIO +// operations have completed. +if (!(activeMMIOReqs || pendingMMIOPkts.size())) { +DPRINTF(KvmIO, "KVM: Finished all outstanding timing requests\n"); +cpu->finishMMIOPending(); +} +return true; +} + +void +BaseKvmCPU::KVMCpuPort::recvReqRetry() +{ +DPRINTF(KvmIO, "KVM: Retry for timing request\n"); + +assert(pendingMMIOPkts.size()); + +// Assuming that we can issue infinite requests this cycle is a bit +// unrealistic, but it's not worth modeling something more complex in +// KVM. +while (pendingMMIOPkts.size() && sendTimingReq(pendingMMIOPkts.front())) { +pendingMMIOPkts.pop(); +activeMMIOReqs++; +} +} + +void +BaseKvmCPU::finishMMIOPending() +{ +assert(_status = RunningMMIOPending); +assert(!tickEvent.scheduled()); + +_status = RunningServiceCompletion; +schedule(tickEvent, nextCycle()); +} + void BaseKvmCPU::startupThread() { @@ -329,6 +399,12 @@ "requesting drain.\n"); return DrainState::Draining; + case RunningMMIOPending: +// We need to drain since there are in-flight timing accesses +DPRINTF(Drain, "KVM CPU is waiting for timing accesses to complete, " +"requesting drain.\n"); +return DrainState::Draining; + case RunningService: // We need to drain since the CPU is waiting for service (e.g., MMIOs) DPRINTF(Drain, "KVM CPU is waiting for service, requesting drain.\n"); @@ -425,9 +501,9 @@ void BaseKvmCPU::verifyMemoryMode() const { -if (!(system->isAtomicMode() && system->bypassCaches())) { +if (!(system->bypassCaches())) { fatal("The KVM-based CPUs requires the memory system to be in the " - "'atomic_noncaching' mode.\n"); + "'noncaching' mode.\n"); } } @@ -536,7 +612,7 @@ BaseKvmCPU::tick() { Tick delay(0); -assert(_status != Idle); +assert(_status != Idle && _status != RunningMMIOPending); switch (_status) { case RunningService: @@ -620,7 +696,7 @@ } // Schedule a new tick if we are still running -if (_status != Idle) +if (_status != Idle && _status != RunningMMIOPending) schedule(tickEvent, clockEdge(ticksToCycles(delay))); } @@ -629,8 +705,9 @@ { // By default, the only thing we need to drain is a pending IO // operation which assumes that we are in the -// RunningServiceCompletion state. -assert(_status == RunningServiceCompletion); +// RunningServiceCompletion or RunningMMIOPending state. +assert(_status == RunningServiceCompletion || + _status == RunningMMIOPending); // Deliver the data from the pending IO operation and immediately // exit. @@ -922,9 +999,12 @@ return handleKvmExitException(); case KVM_EXIT_IO: -_status = RunningServiceCompletion; + { ++n
[gem5-dev] changeset in gem5: sim: Refactor quiesce and remove FS asserts
changeset fe32a5238754 in /z/repo/gem5 details: http://repo.gem5.org/gem5?cmd=changeset;node=fe32a5238754 description: sim: Refactor quiesce and remove FS asserts The quiesce family of magic ops can be simplified by the inclusion of quiesceTick() and quiesce() functions on ThreadContext. This patch also gets rid of the FS guards, since suspending a CPU is also a valid operation for SE mode. diffstat: src/cpu/o3/cpu.cc | 7 +-- src/cpu/simple_thread.cc | 1 + src/cpu/thread_context.cc | 36 + src/cpu/thread_context.hh | 12 +++ src/sim/pseudo_inst.cc| 79 ++ 5 files changed, 56 insertions(+), 79 deletions(-) diffs (230 lines): diff -r c89c72b0e5f5 -r fe32a5238754 src/cpu/o3/cpu.cc --- a/src/cpu/o3/cpu.cc Tue Sep 13 23:16:06 2016 -0400 +++ b/src/cpu/o3/cpu.cc Tue Sep 13 23:17:42 2016 -0400 @@ -380,10 +380,9 @@ assert(o3_tc->cpu); o3_tc->thread = this->thread[tid]; -if (FullSystem) { -// Setup quiesce event. -this->thread[tid]->quiesceEvent = new EndQuiesceEvent(tc); -} +// Setup quiesce event. +this->thread[tid]->quiesceEvent = new EndQuiesceEvent(tc); + // Give the thread the TC. this->thread[tid]->tc = tc; diff -r c89c72b0e5f5 -r fe32a5238754 src/cpu/simple_thread.cc --- a/src/cpu/simple_thread.cc Tue Sep 13 23:16:06 2016 -0400 +++ b/src/cpu/simple_thread.cc Tue Sep 13 23:17:42 2016 -0400 @@ -69,6 +69,7 @@ { clearArchRegs(); tc = new ProxyThreadContext(this); +quiesceEvent = new EndQuiesceEvent(tc); } SimpleThread::SimpleThread(BaseCPU *_cpu, int _thread_num, System *_sys, diff -r c89c72b0e5f5 -r fe32a5238754 src/cpu/thread_context.cc --- a/src/cpu/thread_context.cc Tue Sep 13 23:16:06 2016 -0400 +++ b/src/cpu/thread_context.cc Tue Sep 13 23:17:42 2016 -0400 @@ -41,6 +41,7 @@ * Authors: Kevin Lim */ +#include "arch/kernel_stats.hh" #include "base/misc.hh" #include "base/trace.hh" #include "config/the_isa.hh" @@ -48,6 +49,8 @@ #include "cpu/quiesce_event.hh" #include "cpu/thread_context.hh" #include "debug/Context.hh" +#include "debug/Quiesce.hh" +#include "params/BaseCPU.hh" #include "sim/full_system.hh" void @@ -104,6 +107,39 @@ } void +ThreadContext::quiesce() +{ +if (!getCpuPtr()->params()->do_quiesce) +return; + +DPRINTF(Quiesce, "%s: quiesce()\n", getCpuPtr()->name()); + +suspend(); +if (getKernelStats()) + getKernelStats()->quiesce(); +} + + +void +ThreadContext::quiesceTick(Tick resume) +{ +BaseCPU *cpu = getCpuPtr(); + +if (!cpu->params()->do_quiesce) +return; + +EndQuiesceEvent *quiesceEvent = getQuiesceEvent(); + +cpu->reschedule(quiesceEvent, resume, true); + +DPRINTF(Quiesce, "%s: quiesceTick until %lu\n", cpu->name(), resume); + +suspend(); +if (getKernelStats()) +getKernelStats()->quiesce(); +} + +void serialize(ThreadContext &tc, CheckpointOut &cp) { using namespace TheISA; diff -r c89c72b0e5f5 -r fe32a5238754 src/cpu/thread_context.hh --- a/src/cpu/thread_context.hh Tue Sep 13 23:16:06 2016 -0400 +++ b/src/cpu/thread_context.hh Tue Sep 13 23:17:42 2016 -0400 @@ -174,6 +174,12 @@ /// Set the status to Halted. virtual void halt() = 0; +/// Quiesce thread context +void quiesce(); + +/// Quiesce, suspend, and schedule activate at resume +void quiesceTick(Tick resume); + virtual void dumpFuncProfile() = 0; virtual void takeOverFrom(ThreadContext *old_context) = 0; @@ -367,6 +373,12 @@ /// Set the status to Halted. void halt() { actualTC->halt(); } +/// Quiesce thread context +void quiesce() { actualTC->quiesce(); } + +/// Quiesce, suspend, and schedule activate at resume +void quiesceTick(Tick resume) { actualTC->quiesceTick(resume); } + void dumpFuncProfile() { actualTC->dumpFuncProfile(); } void takeOverFrom(ThreadContext *oldContext) diff -r c89c72b0e5f5 -r fe32a5238754 src/sim/pseudo_inst.cc --- a/src/sim/pseudo_inst.ccTue Sep 13 23:16:06 2016 -0400 +++ b/src/sim/pseudo_inst.ccTue Sep 13 23:17:42 2016 -0400 @@ -234,105 +234,34 @@ quiesce(ThreadContext *tc) { DPRINTF(PseudoInst, "PseudoInst::quiesce()\n"); -if (!FullSystem) -panicFsOnlyPseudoInst("quiesce"); - -if (!tc->getCpuPtr()->params()->do_quiesce) -return; - -DPRINTF(Quiesce, "%s: quiesce()\n", tc->getCpuPtr()->name()); - -tc->suspend(); -if (tc->getKernelStats()) -tc->getKernelStats()->quiesce(); +tc->quiesce(); } void quiesceSkip(ThreadContext *tc) { DPRINTF(PseudoInst, "PseudoInst::quiesceSkip()\n"); -if (!FullSystem) -panicFsOnlyPseudoInst("quiesceSkip"); - -BaseCPU *cpu = tc->getCpuPtr(); - -if (!cpu->params()->do_quiesce) -return; - -EndQuiesceEvent *quiesceEvent = tc->getQuiesceEvent(); - -Tick re
[gem5-dev] changeset in gem5: x86: Force strict ordering for memory mapped ...
changeset 85011e8eaad9 in /z/repo/gem5 details: http://repo.gem5.org/gem5?cmd=changeset;node=85011e8eaad9 description: x86: Force strict ordering for memory mapped m5ops Normal MMAPPED_IPR requests are allowed to execute speculatively under the assumption that they have no side effects. The special case of m5ops that are treated like MMAPPED_IPR should not be allowed to execute speculatively, since they can have side-effects. Adding the STRICT_ORDER flag to these requests blocks execution until the associated instruction hits the ROB head. diffstat: src/arch/x86/tlb.cc | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diffs (13 lines): diff -r fe32a5238754 -r 85011e8eaad9 src/arch/x86/tlb.cc --- a/src/arch/x86/tlb.cc Tue Sep 13 23:17:42 2016 -0400 +++ b/src/arch/x86/tlb.cc Tue Sep 13 23:18:34 2016 -0400 @@ -235,7 +235,8 @@ if (m5opRange.contains(paddr)) { if (m5opRange.contains(paddr)) { -req->setFlags(Request::MMAPPED_IPR | Request::GENERIC_IPR); +req->setFlags(Request::MMAPPED_IPR | Request::GENERIC_IPR | + Request::STRICT_ORDER); req->setPaddr(GenericISA::iprAddressPseudoInst( (paddr >> 8) & 0xFF, paddr & 0xFF)); ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] changeset in gem5: config: move dist-gem5 options to common config
changeset c89c72b0e5f5 in /z/repo/gem5 details: http://repo.gem5.org/gem5?cmd=changeset;node=c89c72b0e5f5 description: config: move dist-gem5 options to common config dist-gem5 should not be restricted to FullSystem mode. diffstat: configs/common/Options.py | 64 -- 1 files changed, 33 insertions(+), 31 deletions(-) diffs (85 lines): diff -r 2344c9dcc0d6 -r c89c72b0e5f5 configs/common/Options.py --- a/configs/common/Options.py Tue Sep 13 23:14:24 2016 -0400 +++ b/configs/common/Options.py Tue Sep 13 23:16:06 2016 -0400 @@ -142,6 +142,39 @@ parser.add_option("--l3_assoc", type="int", default=16) parser.add_option("--cacheline_size", type="int", default=64) +# dist-gem5 options +parser.add_option("--dist", action="store_true", + help="Parallel distributed gem5 simulation.") +parser.add_option("--is-switch", action="store_true", + help="Select the network switch simulator process for a"\ + "distributed gem5 run") +parser.add_option("--dist-rank", default=0, action="store", type="int", + help="Rank of this system within the dist gem5 run.") +parser.add_option("--dist-size", default=0, action="store", type="int", + help="Number of gem5 processes within the dist gem5 run.") +parser.add_option("--dist-server-name", + default="127.0.0.1", + action="store", type="string", + help="Name of the message server host\nDEFAULT: localhost") +parser.add_option("--dist-server-port", + default=2200, + action="store", type="int", + help="Message server listen port\nDEFAULT: 2200") +parser.add_option("--dist-sync-repeat", + default="0us", + action="store", type="string", + help="Repeat interval for synchronisation barriers among dist-gem5 processes\nDEFAULT: --ethernet-linkdelay") +parser.add_option("--dist-sync-start", + default="52000t", + action="store", type="string", + help="Time to schedule the first dist synchronisation barrier\nDEFAULT:52000t") +parser.add_option("--ethernet-linkspeed", default="10Gbps", +action="store", type="string", +help="Link speed in bps\nDEFAULT: 10Gbps") +parser.add_option("--ethernet-linkdelay", default="10us", + action="store", type="string", + help="Link delay in seconds\nDEFAULT: 10us") + # Enable Ruby parser.add_option("--ruby", action="store_true") @@ -297,41 +330,10 @@ # Benchmark options parser.add_option("--dual", action="store_true", help="Simulate two systems attached with an ethernet link") -parser.add_option("--dist", action="store_true", - help="Parallel distributed gem5 simulation.") -parser.add_option("--is-switch", action="store_true", - help="Select the network switch simulator process for a"\ - "distributed gem5 run") -parser.add_option("--dist-rank", default=0, action="store", type="int", - help="Rank of this system within the dist gem5 run.") -parser.add_option("--dist-size", default=0, action="store", type="int", - help="Number of gem5 processes within the dist gem5 run.") -parser.add_option("--dist-server-name", - default="127.0.0.1", - action="store", type="string", - help="Name of the message server host\nDEFAULT: localhost") -parser.add_option("--dist-server-port", - default=2200, - action="store", type="int", - help="Message server listen port\nDEFAULT: 2200") -parser.add_option("--dist-sync-repeat", - default="0us", - action="store", type="string", - help="Repeat interval for synchronisation barriers among dist-gem5 processes\nDEFAULT: --ethernet-linkdelay") -parser.add_option("--dist-sync-start", - default="52000t", - action="store", type="string", - help="Time to schedule the first dist synchronisation barrier\nDEFAULT:52000t") parser.add_option("-b", "--benchmark", action="store", type="string", dest="benchmark", help="Specify the benchmark to run. Available benchmarks: %s"\ % DefinedBenchmarks) -parser.add_option("--ethernet-linkspeed", default="10Gbps", -action="store", type="string", -help="Link speed in bps\nDEFA
[gem5-dev] changeset in gem5: sim, syscall_emul: Add mmap to EmulatedDriver
changeset 9796e43e751d in /z/repo/gem5 details: http://repo.gem5.org/gem5?cmd=changeset;node=9796e43e751d description: sim, syscall_emul: Add mmap to EmulatedDriver Add support for calling mmap on an EmulatedDriver file descriptor. diffstat: src/sim/emul_driver.hh | 15 +-- src/sim/syscall_emul.hh | 12 +++- 2 files changed, 24 insertions(+), 3 deletions(-) diffs (54 lines): diff -r b56cbe6b63a2 -r 9796e43e751d src/sim/emul_driver.hh --- a/src/sim/emul_driver.hhTue Sep 13 23:11:20 2016 -0400 +++ b/src/sim/emul_driver.hhTue Sep 13 23:12:46 2016 -0400 @@ -46,8 +46,8 @@ * hardware inside gem5 can be created by deriving from this class and * overriding the abstract virtual methods. * - * Currently only open() and ioctl() calls are supported, but other calls - * (e.g., read(), write(), mmap()) could be added as needed. + * Currently only open(), ioctl(), and mmap() calls are supported, but other + * calls (e.g., read(), write()) could be added as needed. */ class EmulatedDriver : public SimObject { @@ -85,6 +85,17 @@ * (see the SyscallReturn class). */ virtual int ioctl(LiveProcess *p, ThreadContext *tc, unsigned req) = 0; + +/** + * Virtual method, invoked when the user program calls mmap() on + * the file descriptor returned by a previous open(). The parameters + * are the same as those passed in to mmapFunc() (q.v.). + * @return The return ptr for the mmap, or the negation of the errno + * (see the SyscallReturn class). + */ +virtual Addr mmap(LiveProcess *p, ThreadContext *tc, Addr start, + uint64_t length, int prot, int tgtFlags, int tgtFd, + int offset) { return -EBADF; } }; #endif // __SIM_EMUL_DRIVER_HH diff -r b56cbe6b63a2 -r 9796e43e751d src/sim/syscall_emul.hh --- a/src/sim/syscall_emul.hh Tue Sep 13 23:11:20 2016 -0400 +++ b/src/sim/syscall_emul.hh Tue Sep 13 23:12:46 2016 -0400 @@ -1284,7 +1284,17 @@ int sim_fd = -1; uint8_t *pmap = nullptr; if (!(tgt_flags & OS::TGT_MAP_ANONYMOUS)) { -sim_fd = p->getSimFD(tgt_fd); +// Check for EmulatedDriver mmap +FDEntry *fde = p->getFDEntry(tgt_fd); +if (fde == NULL) +return -EBADF; + +if (fde->driver != NULL) { +return fde->driver->mmap(p, tc, start, length, prot, + tgt_flags, tgt_fd, offset); +} +sim_fd = fde->fd; + if (sim_fd < 0) return -EBADF; ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] changeset in gem5: dev: Exit correctly in dist-gem5
changeset 0b2aaf6f5c78 in /z/repo/gem5 details: http://repo.gem5.org/gem5?cmd=changeset;node=0b2aaf6f5c78 description: dev: Exit correctly in dist-gem5 The receiver thread in dist_iface is allowed to directly exit the simulation. This can cause exit to be called twice if the main thread simultaneously wants to exit the simulation. Therefore, have the receiver thread enqueue a request to exit on the primary event queue for the main simulation thread to handle. diffstat: src/dev/net/dist_iface.cc | 8 src/dev/net/tcp_iface.cc | 5 ++--- 2 files changed, 6 insertions(+), 7 deletions(-) diffs (33 lines): diff -r 3e9314ddf012 -r 0b2aaf6f5c78 src/dev/net/dist_iface.cc --- a/src/dev/net/dist_iface.cc Tue Sep 13 23:06:32 2016 -0400 +++ b/src/dev/net/dist_iface.cc Tue Sep 13 23:08:34 2016 -0400 @@ -610,10 +610,10 @@ // because one of them called m5 exit. So we stop here. // Grab the eventq lock to stop the simulation thread curEventQueue()->lock(); -exit_message("info", - 0, - "Message server closed connection, " - "simulation is exiting"); +exitSimLoop("Message server closed connection, simulator " +"is exiting"); +curEventQueue()->unlock(); +break; } // We got a valid dist header packet, let's process it diff -r 3e9314ddf012 -r 0b2aaf6f5c78 src/dev/net/tcp_iface.cc --- a/src/dev/net/tcp_iface.cc Tue Sep 13 23:06:32 2016 -0400 +++ b/src/dev/net/tcp_iface.cc Tue Sep 13 23:08:34 2016 -0400 @@ -267,9 +267,8 @@ ret = ::send(sock, buf, length, MSG_NOSIGNAL); if (ret < 0) { if (errno == ECONNRESET || errno == EPIPE) { -inform("send(): %s", strerror(errno)); -exit_message("info", 0, "Message server closed connection, " - "simulation is exiting"); +exitSimLoop("Message server closed connection, simulation " +"is exiting"); } else { panic("send() failed: %s", strerror(errno)); } ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] changeset in gem5: gpu-compute: Fix bug with return in cfg
changeset b56cbe6b63a2 in /z/repo/gem5 details: http://repo.gem5.org/gem5?cmd=changeset;node=b56cbe6b63a2 description: gpu-compute: Fix bug with return in cfg Connecting basic blocks would stop too early in kernels where ret was not the last instruction. This patch allows basic blocks after the ret instruction to be properly connected. diffstat: src/gpu-compute/kernel_cfg.cc | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diffs (12 lines): diff -r 0b2aaf6f5c78 -r b56cbe6b63a2 src/gpu-compute/kernel_cfg.cc --- a/src/gpu-compute/kernel_cfg.cc Tue Sep 13 23:08:34 2016 -0400 +++ b/src/gpu-compute/kernel_cfg.cc Tue Sep 13 23:11:20 2016 -0400 @@ -139,7 +139,7 @@ GPUStaticInst* last = lastInstruction(bb.get()); if (last->o_type == Enums::OT_RET) { bb->successorIds.insert(exit_bb->id); -break; +continue; } if (last->o_type == Enums::OT_BRANCH) { const uint32_t target_pc = last->getTargetPc(); ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] changeset in gem5: misc: Remove FullSystem check for networking ...
changeset 3e9314ddf012 in /z/repo/gem5 details: http://repo.gem5.org/gem5?cmd=changeset;node=3e9314ddf012 description: misc: Remove FullSystem check for networking components Ethernet devices are currently only hooked up if running in FS mode. Much of the Ethernet networking code is generic and can be used to build non-Ethernet device models. Some of these device models do not require a complex driver stack and can be built to use an EmulatedDriver in SE mode. This patch enables etherent interfaces to properly connect regardless of whether the simulation is in FS or SE mode. diffstat: src/python/swig/pyobject.cc | 28 +--- 1 files changed, 13 insertions(+), 15 deletions(-) diffs (42 lines): diff -r 57f21c16adde -r 3e9314ddf012 src/python/swig/pyobject.cc --- a/src/python/swig/pyobject.cc Tue Sep 13 23:06:18 2016 -0400 +++ b/src/python/swig/pyobject.cc Tue Sep 13 23:06:32 2016 -0400 @@ -77,25 +77,23 @@ SimObject *o2, const std::string &name2, int i2) { #if THE_ISA != NULL_ISA -if (FullSystem) { -EtherObject *eo1, *eo2; -EtherDevice *ed1, *ed2; -eo1 = dynamic_cast(o1); -ed1 = dynamic_cast(o1); -eo2 = dynamic_cast(o2); -ed2 = dynamic_cast(o2); +EtherObject *eo1, *eo2; +EtherDevice *ed1, *ed2; +eo1 = dynamic_cast(o1); +ed1 = dynamic_cast(o1); +eo2 = dynamic_cast(o2); +ed2 = dynamic_cast(o2); -if ((eo1 || ed1) && (eo2 || ed2)) { -EtherInt *p1 = lookupEthPort(o1, name1, i1); -EtherInt *p2 = lookupEthPort(o2, name2, i2); +if ((eo1 || ed1) && (eo2 || ed2)) { +EtherInt *p1 = lookupEthPort(o1, name1, i1); +EtherInt *p2 = lookupEthPort(o2, name2, i2); -if (p1 != NULL && p2 != NULL) { +if (p1 != NULL && p2 != NULL) { -p1->setPeer(p2); -p2->setPeer(p1); +p1->setPeer(p2); +p2->setPeer(p1); -return 1; -} +return 1; } } #endif ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3595: dev: Add m5 op to toggle synchronization for dist-gem5.
> On Sept. 9, 2016, 2:53 p.m., Gabor Dozsa wrote: > > I like the idea of starting the sync on a pseudo op very much. I cannot see > > the usefulness of stopping the sync after it started though. Do you have a > > use case in mind? > > > > I have a few comments below (mainly minor nits). > > > > However, I think there is also a basic issue with the CPU suspend approach. > > A CPU can wake up whenever there is an interrupt or even a snoop request. > > That should be taken care of somehow I assume. The use case for switching off would be if you have multiple regions of interest in the code and no need to sync in between. You could also just run the simulation a bunch of times and move the start sync point, but I think it’s much cleaner to run the app once and toggle the sync. I suppose a spurious wakeup is possible; I had not really thought about it. I haven't seen CPUs wakeup in practice, but I don't really have any devices that would generate an interrupt/snoop when all the cores are asleep. One alternative way to do this would be to not suspend any of the cores and modify the sync to support all the devices being on different ticks when sync starts. We would need to remember what the start time was for all the participating nodes and use that time to enforce synchronization instead of assuming everyone is on the same tick. I avoided doing this since suspending the CPUs until we reach the max of all nodes ticks' was easier. - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3595/#review8713 ----------- On Aug. 4, 2016, 4:51 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3595/ > --- > > (Updated Aug. 4, 2016, 4:51 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11561:1414a40eb1e2 > --- > dev: Add m5 op to toggle synchronization for dist-gem5. > This patch adds the ability for an application to request dist-gem5 to begin/ > end synchronization using an m5 op. When toggling on sync, all nodes agree on > the next sync point based on the maximum of all nodes' ticks. CPUs are > suspended until the sync point to avoid sending network messages until sync > has > been enabled. Toggling off sync acts like a global execution barrier, where > all CPUs are disabled until every node reaches the toggle off point. This > avoids tricky situations such as one node hitting a toggle off followed by a > toggle on before the other nodes hit the first toggle off. > > > Diffs > - > > configs/common/Options.py 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/arch/x86/isa/decoder/two_byte_opcodes.isa > 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/Ethernet.py 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_etherlink.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_iface.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_packet.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/tcp_iface.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/tcp_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/sim/pseudo_inst.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/sim/pseudo_inst.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5op.h 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5op_x86.S 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5ops.h 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3595/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3595: dev: Add m5 op to toggle synchronization for dist-gem5.
> On Aug. 30, 2016, 3:39 p.m., Michael LeBeane wrote: > > Any comments on this patch? > > Mohammad Alian wrote: > I just have some high level question before goring through the code: > > Can you explain the use-case for this patch? Probably it means to speed > up the simulation but why do we need to switch on/off synchronization at some > points? > > When you suspend CPUs, then how your simulation will gaurantee forward > progress? Doesn't this lead to dead-lock in some situations? > > Thanks. The use case is, like you inferred, to speed up simulation when you don't need to synchronize. There is currently a method to support this using ticks, but I prefer to use source code annotations. The CPUs suspend until the largest tick of all participating CPUs is reached. This is to ensure that we really have started synchronizing after executing the pseudo-op, else we could start sending network messages before we are ready. I don't see a deadlock situation here, can you elaborate on your concern? - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3595/#review8693 ----------- On Aug. 4, 2016, 4:51 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3595/ > --- > > (Updated Aug. 4, 2016, 4:51 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11561:1414a40eb1e2 > --- > dev: Add m5 op to toggle synchronization for dist-gem5. > This patch adds the ability for an application to request dist-gem5 to begin/ > end synchronization using an m5 op. When toggling on sync, all nodes agree on > the next sync point based on the maximum of all nodes' ticks. CPUs are > suspended until the sync point to avoid sending network messages until sync > has > been enabled. Toggling off sync acts like a global execution barrier, where > all CPUs are disabled until every node reaches the toggle off point. This > avoids tricky situations such as one node hitting a toggle off followed by a > toggle on before the other nodes hit the first toggle off. > > > Diffs > - > > configs/common/Options.py 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/arch/x86/isa/decoder/two_byte_opcodes.isa > 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/Ethernet.py 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_etherlink.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_iface.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_packet.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/tcp_iface.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/tcp_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/sim/pseudo_inst.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/sim/pseudo_inst.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5op.h 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5op_x86.S 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5ops.h 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3595/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3591: ruby: Allow multiple outstanding DMA requests
> On Sept. 1, 2016, 3:33 p.m., Jason Lowe-Power wrote: > > What testing did you perform to make sure all of the protocols were > > modified correctly? > > > > Most of these changes seem reasonable to me, but I know from experience > > that even when the SLICC changes seem like they are right, if they aren't > > tested carefully there's almost always bugs. > > Michael LeBeane wrote: > The sequencer changes have been tested pretty thoroughly in a custom > protocol; the public SLICC files only with the regression tester. I'm not > sure how much coverage that provides for DMA other than checking if it > compiles. > > Jason Lowe-Power wrote: > I'm not sure what to do about this. Maybe others in the community will > speak up ;). > > I don't feel comfortable pushing a patch with code that we know hasn't > been tested at all. Specifically with Ruby/SLICC, this has bitten me before. > I've updated gem5 and all of sudden my simluations are broken because someone > changed a protocol without testing it. IMO, code needs to be tested at least > somewhat before it's pushed into the mainline. > > However, I also understand that there isn't any testing infrastructure > for most of the protocols, and we can't ask you to solve that problem before > pushing the patch in. > > Do others have thoughts on this (reoccurring) problem? > > For this specific patch, can you run your workload that use DMA with > protocols other than your internal protocol? It would take some effort, but I do have some DMA intensive networking benchmarks that should be able to run over the public protocols. I wouldn't be able to share these or add them to the regression tester (they are proprietary). I know that doesn't help at all with the more general problem of poor tester coverage, but would that be an acceptable solution for this patch? - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3591/#review8698 --- On Aug. 30, 2016, 3:54 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3591/ > --- > > (Updated Aug. 30, 2016, 3:54 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11557:5d6fcf14c77e > --- > ruby: Allow multiple outstanding DMA requests > DMA sequencers and protocols can currently only issue one DMA access at a > time. > This patch implements the necessary functionality to support multiple > outstanding DMA requests in Ruby. > > > Diffs > - > > src/mem/ruby/system/Sequencer.py 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/ruby/system/DMASequencer.hh > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/ruby/system/DMASequencer.cc > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/MOESI_hammer-dma.sm > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/RubySlicc_Types.sm > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/MOESI_CMP_directory-dma.sm > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/MOESI_CMP_token-dma.sm > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/MESI_Two_Level-dma.sm > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/MI_example-dma.sm 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > > Diff: http://reviews.gem5.org/r/3591/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3591: ruby: Allow multiple outstanding DMA requests
> On Sept. 1, 2016, 3:33 p.m., Jason Lowe-Power wrote: > > What testing did you perform to make sure all of the protocols were > > modified correctly? > > > > Most of these changes seem reasonable to me, but I know from experience > > that even when the SLICC changes seem like they are right, if they aren't > > tested carefully there's almost always bugs. The sequencer changes have been tested pretty thoroughly in a custom protocol; the public SLICC files only with the regression tester. I'm not sure how much coverage that provides for DMA other than checking if it compiles. - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3591/#review8698 ----------- On Aug. 30, 2016, 3:54 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3591/ > --- > > (Updated Aug. 30, 2016, 3:54 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11557:5d6fcf14c77e > --- > ruby: Allow multiple outstanding DMA requests > DMA sequencers and protocols can currently only issue one DMA access at a > time. > This patch implements the necessary functionality to support multiple > outstanding DMA requests in Ruby. > > > Diffs > - > > src/mem/ruby/system/Sequencer.py 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/ruby/system/DMASequencer.hh > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/ruby/system/DMASequencer.cc > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/MOESI_hammer-dma.sm > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/RubySlicc_Types.sm > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/MOESI_CMP_directory-dma.sm > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/MOESI_CMP_token-dma.sm > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/MESI_Two_Level-dma.sm > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/MI_example-dma.sm 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > > Diff: http://reviews.gem5.org/r/3591/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3493: dev: Redefine 'length' in EthPacketData
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3493/#review8696 --- Any more comments on this? We would like to get a few more ship it's since this fairly large and will break checkpoints. - Michael LeBeane On June 29, 2016, 6:27 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3493/ > --- > > (Updated June 29, 2016, 6:27 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11549:573a7d349538 > --- > dev: Redefine 'length' in EthPacketData > Currently, all the network devices create a 16K buffer for the 'data' field > in EthPacketData, and use 'length' to keep track of the size of the packet > in the buffer. This patch introduces 'dataLength' and 'simLength' parameters > to EthPacketData. 'dataLength' stores the amount of space taken up by the > packet in the 'data' buffer. 'simLength' is used to hold the effective length > of the packet used for all timing calulations in the simulator. Serialization > is performed using only the useful data in the packet ('dataLength') and not > necessarily the entire original buffer. > > > Diffs > - > > src/dev/net/etherswitch.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/ethertap.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/i8254xGBe.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/ns_gige.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/pktfifo.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/pktfifo.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/sinic.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/tcp_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherbus.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherdump.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherlink.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherpkt.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/etherpkt.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_etherlink.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_packet.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3493/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3591: ruby: Allow multiple outstanding DMA requests
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3591/ --- (Updated Aug. 30, 2016, 3:54 p.m.) Review request for Default. Repository: gem5 Description --- Changeset 11557:5d6fcf14c77e --- ruby: Allow multiple outstanding DMA requests DMA sequencers and protocols can currently only issue one DMA access at a time. This patch implements the necessary functionality to support multiple outstanding DMA requests in Ruby. Diffs (updated) - src/mem/ruby/system/Sequencer.py 985d9b9a68bf20e22ba65f7398dde0193e6ca076 src/mem/ruby/system/DMASequencer.hh 985d9b9a68bf20e22ba65f7398dde0193e6ca076 src/mem/ruby/system/DMASequencer.cc 985d9b9a68bf20e22ba65f7398dde0193e6ca076 src/mem/protocol/MOESI_hammer-dma.sm 985d9b9a68bf20e22ba65f7398dde0193e6ca076 src/mem/protocol/RubySlicc_Types.sm 985d9b9a68bf20e22ba65f7398dde0193e6ca076 src/mem/protocol/MOESI_CMP_directory-dma.sm 985d9b9a68bf20e22ba65f7398dde0193e6ca076 src/mem/protocol/MOESI_CMP_token-dma.sm 985d9b9a68bf20e22ba65f7398dde0193e6ca076 src/mem/protocol/MESI_Two_Level-dma.sm 985d9b9a68bf20e22ba65f7398dde0193e6ca076 src/mem/protocol/MI_example-dma.sm 985d9b9a68bf20e22ba65f7398dde0193e6ca076 Diff: http://reviews.gem5.org/r/3591/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3619: kvm: Support timing accesses for KVM cpu
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3619/#review8694 --- Andreas H., were your concerns resolved on this patch? Thanks! - Michael LeBeane On Aug. 21, 2016, 3:19 a.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3619/ > --- > > (Updated Aug. 21, 2016, 3:19 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11561:4595cc3848fc > --- > kvm: Support timing accesses for KVM cpu > This patch enables timing accesses for KVM cpu. A new state, > RunningMMIOPending, is added to indicate that there are outstanding timing > requests generated by KVM in the system. KVM's tick() is disabled and the > simulation does not enter into KVM until all outstanding timing requests have > completed. The main motivation for this is to allow KVM CPU to perform MMIO > in Ruby, since Ruby does not support atomic accesses. > > > Diffs > - > > src/cpu/kvm/x86_cpu.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/cpu/kvm/base.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/cpu/kvm/base.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3619/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3589: sim, syscall_emul: Add mmap to EmulatedDriver
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3589/#review8691 --- Any comments on this patch? - Michael LeBeane On Aug. 4, 2016, 4:47 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3589/ > --- > > (Updated Aug. 4, 2016, 4:47 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11555:1b683618d33a > --- > sim, syscall_emul: Add mmap to EmulatedDriver > Add support for calling mmap on an EmulatedDriver file descriptor. > > > Diffs > - > > src/sim/emul_driver.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/sim/syscall_emul.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3589/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3595: dev: Add m5 op to toggle synchronization for dist-gem5.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3595/#review8693 --- Any comments on this patch? - Michael LeBeane On Aug. 4, 2016, 4:51 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3595/ > --- > > (Updated Aug. 4, 2016, 4:51 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11561:1414a40eb1e2 > --- > dev: Add m5 op to toggle synchronization for dist-gem5. > This patch adds the ability for an application to request dist-gem5 to begin/ > end synchronization using an m5 op. When toggling on sync, all nodes agree on > the next sync point based on the maximum of all nodes' ticks. CPUs are > suspended until the sync point to avoid sending network messages until sync > has > been enabled. Toggling off sync acts like a global execution barrier, where > all CPUs are disabled until every node reaches the toggle off point. This > avoids tricky situations such as one node hitting a toggle off followed by a > toggle on before the other nodes hit the first toggle off. > > > Diffs > - > > configs/common/Options.py 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/arch/x86/isa/decoder/two_byte_opcodes.isa > 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/Ethernet.py 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_etherlink.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_iface.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_packet.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/tcp_iface.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/tcp_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/sim/pseudo_inst.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/sim/pseudo_inst.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5op.h 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5op_x86.S 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5ops.h 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3595/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3592: config: move dist-gem5 options to common config
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3592/#review8692 --- Any comments on this patch? - Michael LeBeane On Aug. 4, 2016, 4:47 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3592/ > --- > > (Updated Aug. 4, 2016, 4:47 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11558:935e833b1265 > --- > config: move dist-gem5 options to common config > dist-gem5 should not be restricted to FullSystem mode. > > > Diffs > - > > configs/common/Options.py 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3592/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3619: kvm: Support timing accesses for KVM cpu
> On Aug. 21, 2016, 3:29 a.m., Michael LeBeane wrote: > > Well, having played around with this solution for KVM MMIO + Ruby a bit it > > does work (with the addition of a timing_noncacheable state as Andreas S. > > noted), but not very well. If you put the system in timing mode you get > > events like DRAM refresh that make it so you can't stay in KVM very long, > > which kinda defeats the purpose. Any non-hacky ideas how to get around this? > > Andreas Sandberg wrote: > This is an unfortunate side-effect of using KVM. A display processor > would cause the same type of issues (you'd get events at least once per > refresh, but possibly once per N pixels). There are basically two high-level > solutions: > >1. Don't issue frequent events when running in KVM mode. I have been > considering this for the HDLCD. If running in *_noncacheable, we'd just > reduce simulation fidelity to get events down to something manageable. >2. Run KVM in a separate thread similar to when simulating a > multi-core system using KVM. This allows you to put devices in one event > queue and each of the simulated KVM cores in separate event queues and > control when the queues are synchronised. > > In this particular case, I think 2 sounds like a reasonable solution > since you presumably want good timing fidelity for the GPU. Synchronisation > is going to be "interesting", but the KVM CPU should be able to cope with > being in its own thread. Communication should only really happen when > handling MMIOs and interrupts, which already support synchronisation. I have > something along these lines in my KVM script to map CPUs to threads: > > ```python > root.sim_quantum=m5.ticks.fromSeconds(options.quantum * 1E-3) > > # Assign independent event queues (threads) to the KVM CPUs, > # event queue 0 is reserved for simulated devices. > for idx, cpu in enumerate(system.cpu): > # Child objects usually inherit the parent's event > # queue. Override that and use queue 0 instead. > for obj in cpu.descendants(): > obj.eventq_index = 0 > > cpu.eventq_index = idx + 1 > ``` > > You might want to test the timing changes on their own in a multi-core > system in timing_noncacheable mode to make sure that they synchronise > correctly. I have a sneaking suspicion that they don't at the moment. Thanks for the good suggestions! Yeah, solution 2) seems like the right way to go. I don't exactly need to run KVM and the GPU at the same time (right now I switch CPU models), but I can see how 2) would be very useful for those out there just studying GPU performance. Looks like the dram refresh event is turned off in atomic mode specifically for KVM. Making it do the same in timing with KVM running for solution 1) sounds a bit hacky if not impossible. Having played with multiple event queues a bit for other projects, I know that it sometimes fails to work as intended the first time around :-). I'm a bit too busy with other stuff to get sucked up in this right now, but will hopefully get some free cycles to explore this later. - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3619/#review8668 --- On Aug. 21, 2016, 3:19 a.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3619/ > --- > > (Updated Aug. 21, 2016, 3:19 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11561:4595cc3848fc > --- > kvm: Support timing accesses for KVM cpu > This patch enables timing accesses for KVM cpu. A new state, > RunningMMIOPending, is added to indicate that there are outstanding timing > requests generated by KVM in the system. KVM's tick() is disabled and the > simulation does not enter into KVM until all outstanding timing requests have > completed. The main motivation for this is to allow KVM CPU to perform MMIO > in Ruby, since Ruby does not support atomic accesses. > > > Diffs > - > > src/cpu/kvm/x86_cpu.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/cpu/kvm/base.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/cpu/kvm/base.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3619/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3619: kvm: Support timing accesses for KVM cpu
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3619/#review8668 --- Well, having played around with this solution for KVM MMIO + Ruby a bit it does work (with the addition of a timing_noncacheable state as Andreas S. noted), but not very well. If you put the system in timing mode you get events like DRAM refresh that make it so you can't stay in KVM very long, which kinda defeats the purpose. Any non-hacky ideas how to get around this? - Michael LeBeane On Aug. 21, 2016, 3:19 a.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3619/ > --- > > (Updated Aug. 21, 2016, 3:19 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11561:4595cc3848fc > --- > kvm: Support timing accesses for KVM cpu > This patch enables timing accesses for KVM cpu. A new state, > RunningMMIOPending, is added to indicate that there are outstanding timing > requests generated by KVM in the system. KVM's tick() is disabled and the > simulation does not enter into KVM until all outstanding timing requests have > completed. The main motivation for this is to allow KVM CPU to perform MMIO > in Ruby, since Ruby does not support atomic accesses. > > > Diffs > - > > src/cpu/kvm/x86_cpu.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/cpu/kvm/base.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/cpu/kvm/base.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3619/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3619: kvm: Support timing accesses for KVM cpu
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3619/ --- (Updated Aug. 21, 2016, 3:19 a.m.) Review request for Default. Repository: gem5 Description --- Changeset 11561:4595cc3848fc --- kvm: Support timing accesses for KVM cpu This patch enables timing accesses for KVM cpu. A new state, RunningMMIOPending, is added to indicate that there are outstanding timing requests generated by KVM in the system. KVM's tick() is disabled and the simulation does not enter into KVM until all outstanding timing requests have completed. The main motivation for this is to allow KVM CPU to perform MMIO in Ruby, since Ruby does not support atomic accesses. Diffs (updated) - src/cpu/kvm/x86_cpu.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/cpu/kvm/base.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/cpu/kvm/base.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c Diff: http://reviews.gem5.org/r/3619/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3619: kvm: Support timing accesses for KVM cpu
> On Aug. 18, 2016, 8:50 a.m., Andreas Sandberg wrote: > > src/cpu/kvm/base.cc, line 505 > > <http://reviews.gem5.org/r/3619/diff/2/?file=57574#file57574line505> > > > > I would prefer if we could keep the bypass caches check. Classic memory > > will definitely break if it isn't executing in cache bypass mode. I don't > > know if it is worth fixing, but I also don't like the fact that we can get > > really weird errors if this is removed. > > > > I'm not sure how this would interact with your Ruby patches though. You > > might need to add a timing cache bypass mode. > > Michael LeBeane wrote: > Just so I understand better, what issues are you concerned about if we > don't operate in bypass cache mode? It looks like caches won't be flushed > when entering KVM without being in bypass cache mode, so that's a concern. > Are you also concerned that other devices can write to the cache and KVM > won't see it? KVM itself only issues uncacheable IO requests, which > shouldn't matter whether bypass cache is enabled. > > For Ruby, we definately timing cache bypass mode, if only to make sure > caches are properly flushed when switching into KVM from Ruby-land. But > that's probably a seperate patch. Turns out I was wrong, we don't need to flush Ruby caches because of the backing store. But I will still need to introduce a timing_noncacheable mode if you would like to keep this assertion, else I can't actually use this in Ruby. - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3619/#review8656 --- On Aug. 17, 2016, 8:07 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3619/ > --- > > (Updated Aug. 17, 2016, 8:07 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11561:4595cc3848fc > --- > kvm: Support timing accesses for KVM cpu > This patch enables timing accesses for KVM cpu. A new state, > RunningMMIOPending, is added to indicate that there are outstanding timing > requests generated by KVM in the system. KVM's tick() is disabled and the > simulation does not enter into KVM until all outstanding timing requests have > completed. The main motivation for this is to allow KVM CPU to perform MMIO > in Ruby, since Ruby does not support atomic accesses. > > > Diffs > - > > src/cpu/kvm/base.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/cpu/kvm/base.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/cpu/kvm/x86_cpu.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3619/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3619: kvm: Support timing accesses for KVM cpu
> On Aug. 18, 2016, 8:50 a.m., Andreas Sandberg wrote: > > src/cpu/kvm/base.cc, line 505 > > <http://reviews.gem5.org/r/3619/diff/2/?file=57574#file57574line505> > > > > I would prefer if we could keep the bypass caches check. Classic memory > > will definitely break if it isn't executing in cache bypass mode. I don't > > know if it is worth fixing, but I also don't like the fact that we can get > > really weird errors if this is removed. > > > > I'm not sure how this would interact with your Ruby patches though. You > > might need to add a timing cache bypass mode. Just so I understand better, what issues are you concerned about if we don't operate in bypass cache mode? It looks like caches won't be flushed when entering KVM without being in bypass cache mode, so that's a concern. Are you also concerned that other devices can write to the cache and KVM won't see it? KVM itself only issues uncacheable IO requests, which shouldn't matter whether bypass cache is enabled. For Ruby, we definately timing cache bypass mode, if only to make sure caches are properly flushed when switching into KVM from Ruby-land. But that's probably a seperate patch. - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3619/#review8656 --- On Aug. 17, 2016, 8:07 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3619/ > --- > > (Updated Aug. 17, 2016, 8:07 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11561:4595cc3848fc > --- > kvm: Support timing accesses for KVM cpu > This patch enables timing accesses for KVM cpu. A new state, > RunningMMIOPending, is added to indicate that there are outstanding timing > requests generated by KVM in the system. KVM's tick() is disabled and the > simulation does not enter into KVM until all outstanding timing requests have > completed. The main motivation for this is to allow KVM CPU to perform MMIO > in Ruby, since Ruby does not support atomic accesses. > > > Diffs > - > > src/cpu/kvm/base.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/cpu/kvm/base.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/cpu/kvm/x86_cpu.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3619/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3619: kvm: Support timing accesses for KVM cpu
> On Aug. 17, 2016, 10:05 p.m., Andreas Hansson wrote: > > src/cpu/kvm/base.cc, line 228 > > <http://reviews.gem5.org/r/3619/diff/2/?file=57574#file57574line228> > > > > Conceptually this is not very nice, as it assumes infinite throughput. > > > > I see how we save an even this way, but I am tempted to suggest the > > inclusion of an even rather. I'm not sure it's worth modeling this in KVM, but if you feel strongly about it I can implement a fixed number of requests per cycle. - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3619/#review8652 ----------- On Aug. 17, 2016, 8:07 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3619/ > --- > > (Updated Aug. 17, 2016, 8:07 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11561:4595cc3848fc > --- > kvm: Support timing accesses for KVM cpu > This patch enables timing accesses for KVM cpu. A new state, > RunningMMIOPending, is added to indicate that there are outstanding timing > requests generated by KVM in the system. KVM's tick() is disabled and the > simulation does not enter into KVM until all outstanding timing requests have > completed. The main motivation for this is to allow KVM CPU to perform MMIO > in Ruby, since Ruby does not support atomic accesses. > > > Diffs > - > > src/cpu/kvm/base.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/cpu/kvm/base.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/cpu/kvm/x86_cpu.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3619/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3619: kvm: Support timing accesses for KVM cpu
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3619/ --- (Updated Aug. 17, 2016, 8:07 p.m.) Review request for Default. Changes --- Addresses Andreas S.'s review comments. Repository: gem5 Description --- Changeset 11561:4595cc3848fc --- kvm: Support timing accesses for KVM cpu This patch enables timing accesses for KVM cpu. A new state, RunningMMIOPending, is added to indicate that there are outstanding timing requests generated by KVM in the system. KVM's tick() is disabled and the simulation does not enter into KVM until all outstanding timing requests have completed. The main motivation for this is to allow KVM CPU to perform MMIO in Ruby, since Ruby does not support atomic accesses. Diffs (updated) - src/cpu/kvm/base.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/cpu/kvm/base.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/cpu/kvm/x86_cpu.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c Diff: http://reviews.gem5.org/r/3619/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3586: dev: Exit correctly in dist-gem5
> On Aug. 16, 2016, 9:26 p.m., Mohammad Alian wrote: > > Thanks for this fix. Just one issue that I think still is not resolved for > > the termination of dist-gem5 simulations. Maybe Gabor also has an opinoin > > on this. > > When one of the gem5 processes (nodes) disconnects from message server > > (switch), we need to kill the entire simulation (Please correct me if I'm > > wrong). Then why we continue simulation if we receive an erroneous message > > or connection gets closed in "TCPIface::recvTCP" function? Shoudn't we > > terminate simulation at recvTCP function as well? E.g. simplyfying recvTCP > > function to: > > > > > > bool > > TCPIface::recvTCP(int sock, void *buf, unsigned length) > > { > > ssize_t ret; > > > > ret = ::recv(sock, buf, length, MSG_WAITALL ); > > if (ret <= 0 || ret != length) > > panic("recv() failed"); > > > > return (ret == length); > > } > > Gabor Dozsa wrote: > The problem is that the receive thread may see a "socket closed" event > during a normal exit. That happens if the remote gem5 is faster to do the > exit. We do not want to trigger a panic in that case. > > Mohammad Alian wrote: > Then shouldn't we exit simulation cleanly in that case (instead of panic > or inform)? I believe the current implementation does exit cleanly. When a "socket close" event is generated during a graceful exit, then you get an inform() and TCPIface::recvTCP() returns false. This should eventually trickle down to DistIface::recvThreadFunc(), which will properly exit without a panic(). - Michael ----------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3586/#review8642 --- On Aug. 4, 2016, 4:48 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3586/ > --- > > (Updated Aug. 4, 2016, 4:48 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11552:24441a44d54e > --- > dev: Exit correctly in dist-gem5 > The receiver thread in dist_iface is allowed to directly exit the simulation. > This can cause exit to be called twice if the main thread simultaneously wants > to exit the simulation. Therefore, have the receiver thread enqueue a request > to exit on the primary event queue for the main simulation thread to handle. > > > Diffs > - > > src/dev/net/dist_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/tcp_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3586/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3585: misc: Remove FullSystem check for networking components
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3585/ --- (Updated Aug. 16, 2016, 10:32 p.m.) Review request for Default. Repository: gem5 Description (updated) --- Changeset 11550:c3a28f498d2b --- misc: Remove FullSystem check for networking components Ethernet devices are currently only hooked up if running in FS mode. Much of the Ethernet networking code is generic and can be used to build non-Ethernet device models. Some of these device models do not require a complex driver stack and can be built to use an EmulatedDriver in SE mode. This patch enables etherent interfaces to properly connect regardless of whether the simulation is in FS or SE mode. Diffs (updated) - src/python/swig/pyobject.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c Diff: http://reviews.gem5.org/r/3585/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3590: Add a DmaCallback class to DmaDevice
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3590/ --- (Updated Aug. 16, 2016, 10:31 p.m.) Review request for Default. Summary (updated) - Add a DmaCallback class to DmaDevice Repository: gem5 Description (updated) --- Changeset 11555:e72fa7b7cc13 --- Add a DmaCallback class to DmaDevice This patch introduces the DmaCallback helper class, which registers a callback to fire after a sequence of (potentially non-contiguous) DMA transfers on a DmaPort completes. Diffs (updated) - src/dev/dma_device.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c Diff: http://reviews.gem5.org/r/3590/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3588: ruby: convert atomic accesses to functional in RubyPort
> On Aug. 4, 2016, 4:52 p.m., Andreas Hansson wrote: > > This is _very_ dangerous. > > > > The "functional" accessor functions are really only for debug access, and > > any writes happening on the functional interfaces do not really have a > > well-defined consistency model with respect to the "real" memory accessrs > > using atomic/timing. This is a really dangerous hack... > > Michael LeBeane wrote: > The motivation for this is to make KVM work in ruby (it currently issues > an atomic access to do MMIO, and atomic is not supported in Ruby). We could > add ruby specific code in KVM to do a functional access when using Ruby > instead. Do you have any other suggestions? > > Andreas Hansson wrote: > Who is accessing what, and under what conditions? Why is the atomic > access going via Ruby? > > If all the Ruby blocks are truly idle, is implementing atomic not trivial? > > Andreas Sandberg wrote: > Have you considered adding timing support to KVM instead? > > I don't think it would be that hard, you could just have to add a new > state to BaseKvmCPU to indicate that an MMIO access is in flight (e.g., > RunningMMIOPending). When you receive a response, you transition into the > RunningServiceCompletion state and wake the CPU. You'd end up with these > state transitions Runnning -> RunningService -> RunningMMIOPending -> > RunningServiceCompletion for a typical MMIO. > > Obviously, you'd still have to issue atomic accesses if the system is in > atomic mode. Thanks for the feedback! I have implemented timing accesses in KVM as you suggested in r3619 and will close this patch. - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3588/#review8569 --- On Aug. 4, 2016, 4:49 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3588/ > --- > > (Updated Aug. 4, 2016, 4:49 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11554:57b008eb65c3 > --- > ruby: convert atomic accesses to functional in RubyPort > Instead of panicing when receiving an atomic access in ruby, convert it to a > functional access and warn the user. This makes device models that issue > atomic accesses (such as KVM when performing a MMIO access) work correctly in > Ruby. > > > Diffs > - > > src/mem/ruby/system/RubyPort.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3588/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request 3619: kvm: Support timing accesses for KVM cpu
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3619/ --- Review request for Default. Repository: gem5 Description --- Changeset 11561:4595cc3848fc --- kvm: Support timing accesses for KVM cpu This patch enables timing accesses for KVM cpu. A new state, RunningMMIOPending, is added to indicate that there are outstanding timing requests generated by KVM in the system. KVM's tick() is disabled and the simulation does not enter into KVM until all outstanding timing requests have completed. The main motivation for this is to allow KVM CPU to perform MMIO in Ruby, since Ruby does not support atomic accesses. Diffs - src/cpu/kvm/base.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/cpu/kvm/base.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/cpu/kvm/x86_cpu.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c Diff: http://reviews.gem5.org/r/3619/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3590: dev: Add a DmaCallback class to DmaDevice
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3590/ --- (Updated Aug. 4, 2016, 6:44 p.m.) Review request for Default. Summary (updated) - dev: Add a DmaCallback class to DmaDevice Repository: gem5 Description (updated) --- Changeset 11556:b91232deb828 --- dev: Add a DmaCallback class to DmaDevice DmaDevices are able to trigger events on completion of a DMA transfer by inheriting from this class and implementing the process function. Diffs - src/dev/dma_device.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c Diff: http://reviews.gem5.org/r/3590/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request 3591: ruby: Allow multiple outstanding DMA requests
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3591/ --- Review request for Default. Repository: gem5 Description --- Changeset 11557:5d6fcf14c77e --- ruby: Allow multiple outstanding DMA requests DMA sequencers and protocols can currently only issue one DMA access at a time. This patch implements the necessary functionality to support multiple outstanding DMA requests in Ruby. Diffs - src/mem/protocol/MESI_Two_Level-dma.sm 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/mem/protocol/MI_example-dma.sm 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/mem/protocol/MOESI_CMP_directory-dma.sm 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/mem/protocol/MOESI_CMP_token-dma.sm 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/mem/protocol/MOESI_hammer-dma.sm 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/mem/protocol/RubySlicc_Types.sm 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/mem/ruby/system/DMASequencer.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/mem/ruby/system/DMASequencer.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/mem/ruby/system/Sequencer.py 91f58918a76abf1a1dedcaa70a9b95789da7b88c Diff: http://reviews.gem5.org/r/3591/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3585: misc: Remove FullSystem check for networking components
> On Aug. 4, 2016, 4:48 p.m., Andreas Hansson wrote: > > How on earth would you use an Ethernet device without an OS and a driver > > stack? A lot of the Ethernet networking code is fairly generic and can be used to build non-ethernet device models. Some of these device models (high performance RDMA NICs for example) don't need a complex driver stack and can use a simple EmulatedDriver in SE mode. In the long run, I think it would help to rename a lot of these "Ether" objects to "Net" objects if they don't have ethernet-specific semantics in them. - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3585/#review8568 ------- On Aug. 4, 2016, 4:45 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3585/ > --- > > (Updated Aug. 4, 2016, 4:45 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11550:83b09c2a82ac > --- > misc: Remove FullSystem check for networking components > Ethernet devices are currently only hooked up if running in FS mode. > This patch enables etherent interface to properly connect regardless of > whether the simulation is in FS or SE mode. > > > Diffs > - > > src/python/swig/pyobject.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3585/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 3588: ruby: convert atomic accesses to functional in RubyPort
> On Aug. 4, 2016, 4:52 p.m., Andreas Hansson wrote: > > This is _very_ dangerous. > > > > The "functional" accessor functions are really only for debug access, and > > any writes happening on the functional interfaces do not really have a > > well-defined consistency model with respect to the "real" memory accessrs > > using atomic/timing. This is a really dangerous hack... The motivation for this is to make KVM work in ruby (it currently issues an atomic access to do MMIO, and atomic is not supported in Ruby). We could add ruby specific code in KVM to do a functional access when using Ruby instead. Do you have any other suggestions? - Michael --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3588/#review8569 ----------- On Aug. 4, 2016, 4:49 p.m., Michael LeBeane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3588/ > --- > > (Updated Aug. 4, 2016, 4:49 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > --- > > Changeset 11554:57b008eb65c3 > --- > ruby: convert atomic accesses to functional in RubyPort > Instead of panicing when receiving an atomic access in ruby, convert it to a > functional access and warn the user. This makes device models that issue > atomic accesses (such as KVM when performing a MMIO access) work correctly in > Ruby. > > > Diffs > - > > src/mem/ruby/system/RubyPort.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3588/diff/ > > > Testing > --- > > > Thanks, > > Michael LeBeane > > ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request 3595: dev: Add m5 op to toggle synchronization for dist-gem5.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3595/ --- Review request for Default. Repository: gem5 Description --- Changeset 11561:1414a40eb1e2 --- dev: Add m5 op to toggle synchronization for dist-gem5. This patch adds the ability for an application to request dist-gem5 to begin/ end synchronization using an m5 op. When toggling on sync, all nodes agree on the next sync point based on the maximum of all nodes' ticks. CPUs are suspended until the sync point to avoid sending network messages until sync has been enabled. Toggling off sync acts like a global execution barrier, where all CPUs are disabled until every node reaches the toggle off point. This avoids tricky situations such as one node hitting a toggle off followed by a toggle on before the other nodes hit the first toggle off. Diffs - configs/common/Options.py 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/arch/x86/isa/decoder/two_byte_opcodes.isa 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/dev/net/Ethernet.py 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/dev/net/dist_etherlink.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/dev/net/dist_iface.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/dev/net/dist_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/dev/net/dist_packet.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/dev/net/tcp_iface.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/dev/net/tcp_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/sim/pseudo_inst.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/sim/pseudo_inst.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c util/m5/m5op.h 91f58918a76abf1a1dedcaa70a9b95789da7b88c util/m5/m5op_x86.S 91f58918a76abf1a1dedcaa70a9b95789da7b88c util/m5/m5ops.h 91f58918a76abf1a1dedcaa70a9b95789da7b88c Diff: http://reviews.gem5.org/r/3595/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request 3593: sim: Refactor quiesce and remove FS asserts
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3593/ --- Review request for Default. Repository: gem5 Description --- Changeset 11559:99bab18e1728 --- sim: Refactor quiesce and remove FS asserts The quiesce family of magic ops can be simplified by the inclusion of quiesceTick() and quiesce() functions on ThreadContext. This patch also gets rid of the FS guards, since suspending a CPU is also a valid operation for SE mode. Diffs - src/cpu/o3/cpu.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/cpu/simple_thread.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/cpu/thread_context.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/cpu/thread_context.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/sim/pseudo_inst.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c Diff: http://reviews.gem5.org/r/3593/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request 3588: ruby: convert atomic accesses to functional in RubyPort
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3588/ --- Review request for Default. Repository: gem5 Description --- Changeset 11554:57b008eb65c3 --- ruby: convert atomic accesses to functional in RubyPort Instead of panicing when receiving an atomic access in ruby, convert it to a functional access and warn the user. This makes device models that issue atomic accesses (such as KVM when performing a MMIO access) work correctly in Ruby. Diffs - src/mem/ruby/system/RubyPort.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c Diff: http://reviews.gem5.org/r/3588/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request 3586: dev: Exit correctly in dist-gem5
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3586/ --- Review request for Default. Repository: gem5 Description --- Changeset 11552:24441a44d54e --- dev: Exit correctly in dist-gem5 The receiver thread in dist_iface is allowed to directly exit the simulation. This can cause exit to be called twice if the main thread simultaneously wants to exit the simulation. Therefore, have the receiver thread enqueue a request to exit on the primary event queue for the main simulation thread to handle. Diffs - src/dev/net/dist_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c src/dev/net/tcp_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c Diff: http://reviews.gem5.org/r/3586/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request 3590: Add a DmaCallback class to DmaDevice
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3590/ --- Review request for Default. Repository: gem5 Description --- Changeset 11556:b91232deb828 --- Add a DmaCallback class to DmaDevice DmaDevices are able to trigger events on completion of a DMA transfer by inheriting from this class and implementing the process function. Diffs - src/dev/dma_device.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c Diff: http://reviews.gem5.org/r/3590/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request 3592: config: move dist-gem5 options to common config
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3592/ --- Review request for Default. Repository: gem5 Description --- Changeset 11558:935e833b1265 --- config: move dist-gem5 options to common config dist-gem5 should not be restricted to FullSystem mode. Diffs - configs/common/Options.py 91f58918a76abf1a1dedcaa70a9b95789da7b88c Diff: http://reviews.gem5.org/r/3592/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request 3594: x86: Force strict ordering for memory mapped m5ops
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3594/ --- Review request for Default. Repository: gem5 Description --- Changeset 11560:c9ad5bbea4b2 --- x86: Force strict ordering for memory mapped m5ops Normal MMAPPED_IPR requests are allowed to execute speculatively under the assumption that they have no side effects. The special case of m5ops that are treated like MMAPPED_IPR should not be allowed to execute speculatively, since they can have side-effects. Adding the STRICT_ORDER flag to these requests blocks execution until the associated instruction hits the ROB head. Diffs - src/arch/x86/tlb.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c Diff: http://reviews.gem5.org/r/3594/diff/ Testing --- Thanks, Michael LeBeane ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev