Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
trambu opened this issue Oct 13, 2023 · 3 comments

Comments

@trambu
Copy link

trambu commented Oct 13, 2023

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
Copy link
Contributor

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

@trambu
Copy link
Author

trambu commented Oct 14, 2023

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.

@trambu trambu closed this as completed Oct 14, 2023
@trambu trambu reopened this Oct 14, 2023
@jerryz123
Copy link
Contributor

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants