sc.w Instruction Returns 0 When Operated on Different Memory Addresses, Indicating Incorrect Success Status

[fly-1011]

Before start

• I have read the RISC-V ISA Manual and this is not a RISC-V ISA question. 我已经阅读过 RISC-V 指令集手册，这不是一个指令集本身的问题。
• I have read the XiangShan Documents. 我已经阅读过香山文档。
• I have searched the previous issues and did not find anything relevant. 我已经搜索过之前的 issue，并没有找到相关的。
• I have reviewed the commit messages from the relevant commit history. 我已经浏览过相关的提交历史和提交信息。

Describe the bug

In xiangshan, when lr.w and sc.w operate on different memory addresses (i.e. lr.w uses address 0x80010024, while sc.w uses address 0x80010028), sc.w still returns 0, indicating that the operation is successful. This behavior is inconsistent with the reference model nemu

Expected behavior

To Reproduce

.section .text
.globl _start
_start:
            
    li      s1, 0x80010024        
    li      a7, 0x80010028
    li      a3, 0x0
    lr.w    a5, (s1)      
    sc.w    a3, a3, (a7)

Environment

• XiangShan branch:
• XiangShan commit id: 7af39ad (HEAD -> master, origin/master, origin/HEAD)
• NEMU commit id: 34ba2259558ed89f5a042179ef0a9131e53ce037 (HEAD, origin/master, origin/HEAD)
• SPIKE commit id:

Additional context

No response

[eastonman]

This is allowed in ISA, maybe reservation set size is different in NEMU.

[fly-1011]

This is allowed in ISA, maybe reservation set size is different in NEMU.

Thank you for your prompt response and clarification! I will now close this issue. Could you please remove the "bug report" label? Thanks again!😄

[eastonman]

I am not very familiar with NEMU. We can discuss whether NEMU should follow this behaviour.
I think this issue can be left here for discussion.

[NewPaulWalker]

This is allowed in ISA, maybe reservation set size is different in NEMU.

In NEMU, only the address is compared, which means the reservation set size in NEMU is only one byte?
And there's a comment here indicating that it should check for overlap, rather than simply comparing if the addresses are equal.
The corresponding source code in NEMU is here.

// should check overlapping instead of equality
int success = (cpu.lr_addr == *src1) && cpu.lr_valid;

Do you know how this is handled in XiangShan?

[eastonman]

Not sure, but very likely to be a cacheline-wide reservation set. Maybe @huxuan0307 can have a look.

[eastonman]

XiangShan/src/main/scala/xiangshan/cache/dcache/mainpipe/MainPipe.scala

Line 578 in 68838bf

val s3_lrsc_addr_match = lrsc_valid_dup(0) && lrsc_addr === get_block_addr(s3_req.addr)

XiangShan/src/main/scala/xiangshan/cache/L1Cache.scala

Line 86 in 68838bf

def get_block_addr(addr: UInt) = (addr >> blockOffBits) << blockOffBits

Seems to be a 64B aligned reservation set.

[NewPaulWalker]

Seems to be a 64B aligned reservation set.

Thanks! Perhaps we should configure NEMU and XiangShan to use the same size for the reservation set. I can submit a PR of NEMU to make this adjustment.

[NewPaulWalker]

I believe this PR can resolve this inconsistency issue.