If we use lsu_unstall, we have to apply the or1200_wb_biu fix I'm
talking in my other post. As a matter of fact, if we use lsu_unstall:

assign icpu_cycstb_o = ~(genpc_freeze | (|pre_branch_op &&
!icpu_rty_i) | lsu_unstall);

the icpu_cycstb_o will go low again at the end of the lsu job *but*
the wishbone cycle signal will stay high because it was waiting for an
ack related to the first icpu_cycstb_o assertion.

In other words, I would stay with a new wait_lsu signal.

Franck.

2012/10/9 Franck Jullien <[email protected]>:
> I think we could use lsu_unstall.
>
> As you can see on the waveforms, lsu_unstall is asserted the cycle
> after lsu_stall goes low.
> IF we use lsu_unstall to control icpu_cycstb_o, it would start a new
> wb cycle with the good icpu_adr_o on the bus.
> However, We would have a false icpu_cycstb_o (at the end of the fetch)
> follwed by a good  icpu_cycstb_o assertion.
>
> assign icpu_cycstb_o = ~(genpc_freeze | (|pre_branch_op &&
> !icpu_rty_i) | lsu_unstall);
>
> I'm not sure what is the best solution.
>
> Franck.
>
> 2012/10/9 Matthew Hicks <[email protected]>:
>> For the data version of this signal we have: assign dcpu_cycstb_o =
>> du_stall | lsu_unstall | except_align ? 1'b0 : |ex_lsu_op;
>>
>> Is there going to a problem here as well?  If not, would it be
>> possible to use the pre-existing lsu_unstall signal instead of added
>> wait_lsu logic?
>>
>> Also, has the bug, trigger code, and proposed fix been added to bugzilla?
>>
>>
>> ---Matthew Hicks
>>
>>
>> On Mon, Oct 8, 2012 at 4:12 PM, Franck Jullien <[email protected]> 
>> wrote:
>>> Hi again,
>>>
>>> I've finally found why I had problem with IC cache enable on my board....
>>>
>>> This is the code causing problem:
>>>
>>> 01fd3cf4 <xmalloc>:
>>>  1fd3cf4:       d7 e1 4f fc     l.sw 0xfffffffc(r1),r9
>>>  1fd3cf8:       07 ff c4 9d     l.jal 1fc4f6c <malloc>
>>>  1fd3cfc:       9c 21 ff fc     l.addi r1,r1,0xfffffffc
>>>  1fd3d00:       bc 2b 00 00     l.sfnei r11,0x0
>>>  1fd3d04:       10 00 00 04     l.bf 1fd3d14 <xmalloc+0x20>
>>>
>>> There is several conditions to see this bug:
>>>
>>> - instruction cache enable,
>>> - 2nd instruction on the cache line trigger an lsu_stall,
>>> - 3rd instruction on the cache line is a branch.
>>>
>>> When the or1200_genpc icpu_cycstb_o signal is triggered at the end of
>>> cache line fetch, the address on icpu_adr_o is still equal to the
>>> instruction after the delay slot (here it is 0x01fd3d00) and the next
>>> data to be fetch will be 0xbc2b0000....
>>> By the time the ack is coming from the wishbone, the lsu_stall has
>>> been removed and the icpu_adr_o is now equal to the branch destination
>>> (0x01fc4f6c). However, the data to be fetched has to changed and
>>> 0xbc2b0000 is fetched from address 0x01fc4f6c which is completely
>>> wrong.....
>>>
>>> This patch fixes this wrong behavior forcing the or1200_genpc
>>> icpu_cycstb_o signal to wait for the lsu_stall to be desaserted before
>>> starting a cycle on the wishbone bus.
>>>
>>> On the attached image you'll see the patched waves on the top and the
>>> wrong ones on the bottom.
>>>
>>> Franck.
>>>
>>> _______________________________________________
>>> OpenRISC mailing list
>>> [email protected]
>>> http://lists.openrisc.net/listinfo/openrisc
>>>

<<attachment: fix_uses_lsu_unstall.png>>

_______________________________________________
OpenRISC mailing list
[email protected]
http://lists.openrisc.net/listinfo/openrisc

Reply via email to