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

Store conditional speculative failures can happen too early #485

Open
Timmmm opened this issue May 22, 2024 · 0 comments
Open

Store conditional speculative failures can happen too early #485

Timmmm opened this issue May 22, 2024 · 0 comments

Comments

@Timmmm
Copy link
Collaborator

Timmmm commented May 22, 2024

Store conditional speculative failures currently can happen as soon as a SC is executed:

function clause execute (STORECON(aq, rl, rs2, rs1, width, rd)) = {
  if speculate_conditional () == false then {
    /* should only happen in rmem
     * rmem: allow SC to fail very early
     */
    X(rd) = zero_extend(0b1); RETIRE_SUCCESS

But that doesn't seem quite right. A real design is always going to check alignment and probably do address translation before the SC has a chance to fail in this way (especially after we change the reservation to use physical addresses - see #360).

After #360 is done I believe the fix is to change this:

          if match_reservation(vaddr) == false then {
            /* cannot happen in rmem */
            X(rd) = zero_extend(0b1); cancel_reservation(); RETIRE_SUCCESS
          } else {

to this

                if not(speculate_conditional()) | not(match_reservation(addr)) then {
                  // Spurious failure (this is allowed), or the reservation
                  // address does not match.
                  X(rd) = zero_extend(0b1); RETIRE_SUCCESS
                } else {

We have been using the code like this to inject spurious SC failures (when it fails when it could have succeeded) into the Sail model for months and it is working perfectly.

I'm not at all familiar with what rmem requires, so we might want to double check with those guys if this is going to break their stuff.

billmcspadden-riscv pushed a commit that referenced this issue May 24, 2024
* Add an exclude list for known failing Hifive1 tests

This commit adds a list of known failing tests based on: riscv-collab/riscv-openocd#869 (comment)

* Fix name of the HiFive1 flash target 

Signed-off-by: Marek Vrbka <[email protected]>

---------

Signed-off-by: Marek Vrbka <[email protected]>
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

1 participant