sail-riscv icon indicating copy to clipboard operation
sail-riscv copied to clipboard

Store conditional speculative failures can happen too early

Open Timmmm opened this issue 1 year ago • 0 comments
trafficstars

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.

Timmmm avatar May 22 '24 13:05 Timmmm