Any store-related instruction between LoadReserved and StoreConditional combination will break the reservation

[trambu]

Type of issue: bug report

Impact: unknown

Development Phase: request

Other information

When we call lr instructions to load memory and reserve the address location, any store-related instruction (atomic or non-atomic) with a totally different address (not even the same cache line) will cause an error for subsequent sc instruction. Note that no PMP is enabled, and based on the device tree and memory map configs, the region is read/writeable. Also, on the Spike golden model, no error happens, and store conditional instruction updates the value of the address. For more information, I attached a sequence of instructions to reproduce.

Execute following instructions on Boom:

auipc   a3, 0xfffff
auipc   t4, 0xf
c.lui   a4, 0x10
lr.d    t3, (a3)
sd      a4, 512(s0)
sc.d    t3, a4, (a3)
ld      t4, 0(a3)
add     a5, t3, zero

What is the current behavior?
Boom does not change the memory content, and sc writes an error bit in the rd register.

Boom core trace:

3 0x0000000080001060 (0xfffff697) x13 0x0000000080000060                                                                                                                                     
3 0x0000000080001064 (0x0000fe97) x29 0x0000000080010064                                                                                                                                     
3 0x0000000080001068 (0x6741) x14 0x0000000000010000                                                                                                                                         
3 0x000000008000106a (0x1006be2f) x28 0x0030107330529073                                                                                                                                     
3 0x000000008000106e (0x20e43023)                                                                                                                                                            
3 0x0000000080001072 (0x18e6be2f) x28 0x0000000000000001                                                                                                                                     
3 0x0000000080001076 (0x0006be83) x29 0x0030107330529073                                                                                                                                     
3 0x000000008000107a (0x000e07b3) x15 0x0000000000000001

What is the expected behavior?
Boom has to change the content of the memory because, between the lr and sc instructions, nothing broke the reservation.
Spike trace:

core   0: 0x0000000080001060 (0xfffff697) auipc   a3, 0xfffff
core   0: 3 0x0000000080001060 (0xfffff697) x13 0x0000000080000060
core   0: 0x0000000080001064 (0x0000fe97) auipc   t4, 0xf
core   0: 3 0x0000000080001064 (0x0000fe97) x29 0x0000000080010064
core   0: 0x0000000080001068 (0x00006741) c.lui   a4, 0x10
core   0: 3 0x0000000080001068 (0x6741) x14 0x0000000000010000
core   0: 0x000000008000106a (0x1006be2f) lr.d    t3, (a3)
core   0: 3 0x000000008000106a (0x1006be2f) x28 0x0030107330529073 mem 0x0000000080000060
core   0: 0x000000008000106e (0x20e43023) sd      a4, 512(s0)
core   0: 3 0x000000008000106e (0x20e43023) mem 0x0000000080022650 0x0000000000010000
core   0: 0x0000000080001072 (0x18e6be2f) sc.d    t3, a4, (a3)
core   0: 3 0x0000000080001072 (0x18e6be2f) x28 0x0000000000000000 mem 0x0000000080000060 0x0000000000010000
core   0: 0x0000000080001076 (0x0006be83) ld      t4, 0(a3)
core   0: 3 0x0000000080001076 (0x0006be83) x29 0x0000000000010000 mem 0x0000000080000060
core   0: 0x000000008000107a (0x000e07b3) add     a5, t3, zero
core   0: 3 0x000000008000107a (0x000e07b3) x15 0x0000000000000000

Please tell us about your environment:

- version: `BOOMv3 (SonicBOOM) (3.0.0)`
- OS: `CentOS Linux release 7.9.2009 kernel: 5.15.0-78-generic`
- Simulator: `VCS_2021.09-SP1`
- Spike version: `1.1.1-dev`

What is the use case for changing the behavior?

[Phantom1003]

Nope, I believe this is a legal implementation difference in the specification. You can find more details in Volume 1 v20191213 Section 8.3 Eventual Success of Store-Conditional Instructions

[trambu]

Nope, I believe this is a legal implementation difference in the specification. You can find more details in Volume 1 v20191213 Section 8.3 Eventual Success of Store-Conditional Instructions

Based on specification Yes, but there are two critical problems:

1. Registering reservation for size of whole physical memory, first of all bring a lots of overhead because any store for any arbitrary address will break the reservation then code(user/kernel/firmware) needs a lots of try in a multi thread system.

2. Furthermore, such behaviour limit the flexibility of (user/kernel/firmware) for writing code between LR and SC because they need to avoid any store between LR and SC. It is makes sense to reserve a page size not whole memory.

[sorear]

Just because the reservation is invalidated by any store on the local hart doesn't mean that arbitrary stores on remote harts will invalidate the reservation. You don't even gain that much flexibility since Rocket uses the cache to track the reservation and if you exceed the associativity of the cache set, you can't keep the reservation.