chipsalliance / rocket-chip

Rocket Chip Generator
Other
3.19k stars 1.12k forks source link

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

Open morecoding2 opened 11 months ago

morecoding2 commented 11 months ago

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.

If the current behavior is a bug, please provide the steps to reproduce the problem: Execute following instructions on Rocket core or 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? Rocket core / Boom does not change the memory content, and sc writes an error bit in the rd register.

Rocket core trace:

C0:       2949 [1] pc=[0000000080001060] W[r13=0000000080000060][1] R[r 0=0000000000000000] R[r 0=0000000000000000] inst=[fffff697] auipc   a3, 0xfffff
C0:       2950 [1] pc=[0000000080001064] W[r29=0000000080010064][1] R[r 0=0000000000000000] R[r 0=0000000000000000] inst=[0000fe97] auipc   t4, 0xf
C0:       2951 [1] pc=[0000000080001068] W[r14=0000000000010000][1] R[r 0=0000000000000000] R[r 0=0000000000000000] inst=[00006741] c.lui   a4, 0x10
C0:       2968 [1] pc=[000000008000106a] W[r28=0030107330529073][1] R[r13=0000000080000060] R[r 0=0000000000000000] inst=[1006be2f] lr.d    t3, (a3)
C0:       2990 [1] pc=[000000008000106e] W[r 0=0000000000000000][0] R[r 8=0000000080022450] R[r14=0000000000010000] inst=[20e43023] sd      a4, 512(s0)
C0:       2991 [1] pc=[0000000080001072] W[r28=0000000000000001][1] R[r13=0000000080000060] R[r14=0000000000010000] inst=[18e6be2f] sc.d    t3, a4, (a3)
C0:       2998 [1] pc=[0000000080001076] W[r29=0030107330529073][1] R[r13=0000000080000060] R[r 0=0000000000000000] inst=[0006be83] ld      t4, 0(a3)
C0:       2999 [1] pc=[000000008000107a] W[r15=0000000000000001][1] R[r28=0000000000000001] R[r 0=0000000000000000] inst=[000e07b3] add     a5, t3, zero

What is the expected behavior? Rocket core / 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: `RocketChip v1.6`
- OS: `CentOS Linux release 7.9.2009 kernel: 5.15.0-78-generic`
- Simulator: `VCS_2021.09-SP1`
- Spike version: `1.1.1-dev`
jerryz123 commented 11 months ago

This is allowed in the specification for LR/SC behavior.

morecoding2 commented 11 months ago

This is allowed in the specification for LR/SC behavior.

Based on specification An implementation can register an arbitrarily large reservation set on each LR, provided the reser- vation set includes all bytes of the addressed data word or doubleword. 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 and 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.
jerryz123 commented 11 months ago

This isn't due to the reservation size, this is due to the limitations on what can be within a constrained LR/SC loop to guarantee eventual success, see section 8.3 of the unprivileged spec (Eventual Success of Store Conditional Instructions).