mips: linked load instruction problem

Information
Submitter:	Deyuan Guo
Repository:	gem5
Branch:
Bugs:
Depends On:
Reviewers
Groups:	Default
People:	ali, gblack, nate, stever

Description

The read/setMiscRegNoEffect are declared in inorder_dyn_inst.hh, but not defined in inorder_dyn_inst.cc.
I hope the definitions can be added for the future use.
----
The problem is caused by a MIPS linked load instruction. It make the CP0 halt then restart. After that, the tick becomes a large number and it says 'because simulate() limit reached'.

Korey Sewell told me:
    If you get "simulate() reached", that means your system likely deadlocked as there are no more events on the main event queue. Run w/the cpu progress interval on to help determine where the CPUs stops committing instructions. Also, post the locked_mem.hh patch if you want comments on what you are trying to do to resolve the problem.

----
I change the locked_mem.hh to an old stable version, to avoid the problem mentioned above. But I know this is not 'fixing the bug', but 'avoiding the bug'.

Testing Done

Only single thread.

src/arch/mips/locked_mem.hh (Diff revision 2)

seems like the correct fix here is to fix the CP0 state handling and not circumvent it.

setMiscReg() by default has some effect that needs to handled/activated the cycle after a misc. reg gets set. 

we should find the condition in the CP0 code that causes the CPU to halt and fix it. 

Any ideas on what lines trigger the code to deadlock (isa.cc maybe?)

Is there an update to this patch?

Change Summary:

We have to change setMiscReg into setMiscRegNoEffect here, in order to ensure the correctness of ll/sc instructions.
While it is unnecessary to use readMiscRegNoEffect instead of readMiscReg.
However, I haven't found the root cause of this problem. May be the misc reg cannot be set more than once in a tick?

Diff:

Revision 3 (+3 -3)

Show changes

src/arch/mips/locked_mem.hh

I'd rather see something that's a little closer to actually solving the problem (even if it doesn't work around it completely).

Looking at arch/mips/isa.cc, the only difference between setMiscReg() and setMiscRegNoEffect() is that the former calls scheduleCP0Update().  I'm not sure what that's all about, but if setting the LL register shouldn't call that, then the right thing to do is to add some code to setMiscReg() to avoid that.  If you look at most ISAs, setMiscReg() typically has a giant switch statement to catch all the idiosyncrasies of misc registers; it's odd that MIPS doesn't.

So we could start with a switch statement that keeps the scheduleCP0Update() in as the default case but does nothing for MISCREG_LLADDR and MISCREG_LLFLAG, unless Korey can help define something even better.  (E.g., if we knew exactly which misc regs require scheduleCP0Update(), it would be vastly preferable to just call it when those regs get written, and have the default be nothing.)

Korey Sewell Jan. 16, 2012, 3:23 a.m. (Jan. 16, 2012, 3:23 a.m.)

I think my review is similar to the original one I had here in that we need to find out why that scheduleCP0Update() isnt working right.

That function is supposed to collect all the writes to the system/CP0 state on a given cycle and then update it simultaneously. It was written because changing the system registers affects the functionality but in a multithreaded core there may be multiple updates to the system state on a given cycle. So instantly changing the system state while these updates could result in incorrect execution.

My theory is that something happens when the LLFLAG/ADDR gets set and somehow doesnt schedule the update event.

Deyuan, if we remove that code, can we identify the cycle that LLFLAG is set and see if it actually gets update (you were saying the system deadlocks instead?)

Deyuan Guo Jan. 17, 2012, 4:05 a.m. (Jan. 17, 2012, 4:05 a.m.)

Hello! I think I find the solution to this problem now.

Firstly, I merged the CP0 initialization code from FSConfig.py into configCP() function in isa.cc, and made ISA::clear() call the configCP() fucntion. Although it is unnecessary to move those codes from FSConfig.py to isa.cc, the other parts of configCP() fucntion, such as setRegMask() for some MiscRegs, are essential to the validity of gem5. After that, I tested a small testcase in MIPS_SE mode, and the errors caused by linked load instruction are as follows:

info: Entering event queue @ 0.  Starting simulation...
info: Increasing stack size by one page.
warn: 300511500: context 0: 100000 consecutive store conditional failures
warn: 600511500: context 0: 200000 consecutive store conditional failures
warn: 900511500: context 0: 300000 consecutive store conditional failures
warn: 1200511500: context 0: 400000 consecutive store conditional failures
...

Then I check the setMiscReg() function. It calls filterCP0Write() just before scheduleCP0Update(), and filterCP0Write() relies on the miscRegFile_WriteMask set by configCP(). At this time, I found that the masks of Random/BADVADDR/LLADDR are wrongly set to 0 in configCP(), which prevented the writing back of the values to these registers. Furthermore, the zero mask of LLADDR caused the problem of linked load instruction, and the zero mask of Random register caused the problem of tlbwr instruction.

By deleting these codes in configCP(), we can get the correct results in both MIPS_SE and MIPS_FS modes, without fixing any other codes such as setMiscReg() or scheduleCP0Update().

Change Summary:

Thanks. Following your suggestions, I implement a switch in setMiscReg, which can deal with LLFLAG and LLADDR specially.
Therefore, it doesn't need to change the locked_mem.cc.

Diff:

Revision 4 (+13 -5)

Show changes

src/arch/mips/isa.cc

Change Summary:

Hello! I think I find the solution to this problem now.

Firstly, I merged the CP0 initialization code from FSConfig.py into configCP() function in isa.cc, and made ISA::clear() call the configCP() fucntion. Although it is unnecessary to move those codes from FSConfig.py to isa.cc, the other parts of configCP() fucntion, such as setRegMask() for some MiscRegs, are essential to the validity of gem5. After that, I tested a small testcase in MIPS_SE mode, and the errors caused by linked load instruction are as follows:

info: Entering event queue @ 0.  Starting simulation...
info: Increasing stack size by one page.
warn: 300511500: context 0: 100000 consecutive store conditional failures
warn: 600511500: context 0: 200000 consecutive store conditional failures
warn: 900511500: context 0: 300000 consecutive store conditional failures
warn: 1200511500: context 0: 400000 consecutive store conditional failures
...

Then I check the setMiscReg() function. It calls filterCP0Write() just before scheduleCP0Update(), and filterCP0Write() relies on the miscRegFile_WriteMask set by configCP(). At this time, I found that the masks of Random/BADVADDR/LLADDR are wrongly set to 0 in configCP(), which prevented the writing back of the values to these registers. Furthermore, the zero mask of LLADDR caused the problem of linked load instruction, and the zero mask of Random register caused the problem of tlbwr instruction.

By deleting these codes in configCP(), we can get the correct results in both MIPS_SE and MIPS_FS modes, without fixing any other codes such as setMiscReg() or scheduleCP0Update().

Diff:

Revision 5 (-5)

Show changes

src/arch/mips/isa.cc

src/arch/mips/isa.cc (Diff revision 5)

Thanks for taking the time to debug this this Deyuan.

I'm going to take another pass through your patches and hopefully start to get them by the end of the week.

You have a pending review.

Review Board 2.0.15

Screenshots

Files

Change Summary:

Diff:

Change Summary:

Diff:

Change Summary:

Diff: