Skip to content

Commit

Permalink
MSHR: fix a dual-core bug when ReleaseData nests the Probe (#177)
Browse files Browse the repository at this point in the history
Before this commit, an error scenario was discovered during dual-core TL-test:
one client L2 requested the T permission for a block, triggering a probe to
another L2. At the same time, the other L2 sent a Release request for the same
block. L3 would then process the ReleaseData first by saving it in dataStorage,
followed by receiving a probeack without data. At this point, L3 would assume
the upper-level cache did not have the block, leading to a AcquireBlock request
for the data block from memory, resulting in stale data being granted to L2.

This commit fixes the bug by adding conditional constraints to some of the
state transitions in L3. Two key signals were added:

* `nestC_save` indicates that an AcquireBlock is nested by a ReleaseData for the
same address, and this ReleaseData has saved into L3.

* `nestC_saveDirty` indicates that nestC_save is satisfied and the ReleaseData is
a dirty block.
  • Loading branch information
sumailyyc committed May 23, 2024
1 parent 6e2322e commit dbba515
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 4 deletions.
6 changes: 6 additions & 0 deletions src/main/scala/huancun/Slice.scala
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,8 @@ class Slice()(implicit p: Parameters) extends HuanCunModule {
mshr.io_c_status.way := c_mshr.io.status.bits.way
mshr.io_c_status.nestedReleaseData :=
c_mshr.io.status.valid && non_inclusive(c_mshr).io_is_nestedReleaseData
mshr.io_c_status.metaValid := non_inclusive(c_mshr).io_metaValid
mshr.io_c_status.reqDirty := non_inclusive(c_mshr).io_reqDirty
mshr.io_b_status.set := bc_mshr.io.status.bits.set
mshr.io_b_status.tag := bc_mshr.io.status.bits.tag
mshr.io_b_status.way := bc_mshr.io.status.bits.way
Expand All @@ -231,6 +233,8 @@ class Slice()(implicit p: Parameters) extends HuanCunModule {
mshr.io_c_status.way := c_mshr.io.status.bits.way
mshr.io_c_status.nestedReleaseData :=
c_mshr.io.status.valid && non_inclusive(c_mshr).io_is_nestedReleaseData
mshr.io_c_status.metaValid := non_inclusive(c_mshr).io_metaValid
mshr.io_c_status.reqDirty := non_inclusive(c_mshr).io_reqDirty
mshr.io_b_status.set := 0.U
mshr.io_b_status.tag := 0.U
mshr.io_b_status.way := 0.U
Expand All @@ -246,6 +250,8 @@ class Slice()(implicit p: Parameters) extends HuanCunModule {
mshr.io_c_status.tag := 0.U
mshr.io_c_status.way := 0.U
mshr.io_c_status.nestedReleaseData := false.B
mshr.io_c_status.metaValid := false.B
mshr.io_c_status.reqDirty := false.B
mshr.io_b_status.set := 0.U
mshr.io_b_status.tag := 0.U
mshr.io_b_status.way := 0.U
Expand Down
39 changes: 35 additions & 4 deletions src/main/scala/huancun/noninclusive/MSHR.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ class C_Status(implicit p: Parameters) extends HuanCunBundle {
val way = Input(UInt(wayBits.W))
val nestedReleaseData = Input(Bool())
val releaseThrough = Output(Bool())
val metaValid = Input(Bool())
val reqDirty = Input(Bool())
}

class B_Status(implicit p: Parameters) extends HuanCunBundle {
Expand Down Expand Up @@ -49,6 +51,8 @@ class MSHR()(implicit p: Parameters) extends BaseMSHR[DirResult, SelfDirWrite, S
val io_is_nestedReleaseData = IO(Output(Bool()))
val io_is_nestedProbeAckData = IO(Output(Bool()))
val io_probeHelperFinish = IO(Output(Bool()))
val io_metaValid = IO(Output(Bool()))
val io_reqDirty = IO(Output(Bool()))

val req = Reg(new MSHRRequest)
val req_valid = RegInit(false.B)
Expand Down Expand Up @@ -558,6 +562,10 @@ class MSHR()(implicit p: Parameters) extends BaseMSHR[DirResult, SelfDirWrite, S
val w_sinkcack = RegInit(true.B)

val acquire_flag = RegInit(false.B)
val nestC_save_flag = RegInit(false.B)
val nestC_saveDirty_flag = RegInit(false.B)
val nestC_save = WireInit(false.B)
val nestC_saveDirty = WireInit(false.B)

def reset_all_flags(): Unit = {
// Default value
Expand Down Expand Up @@ -600,6 +608,8 @@ class MSHR()(implicit p: Parameters) extends BaseMSHR[DirResult, SelfDirWrite, S
acquire_flag := false.B
a_do_release := false.B
a_do_probe := false.B
nestC_save_flag := false.B
nestC_saveDirty_flag := false.B
}
when(!s_acquire) { acquire_flag := acquire_flag | true.B }

Expand Down Expand Up @@ -1134,7 +1144,7 @@ class MSHR()(implicit p: Parameters) extends BaseMSHR[DirResult, SelfDirWrite, S

od.sinkId := io.id
od.useBypass := (!self_meta.hit || self_meta.state === BRANCH && req_needT) &&
(!probe_dirty || acquire_flag && oa.opcode =/= AcquirePerm) &&
(!probe_dirty && !nestC_save || acquire_flag && oa.opcode =/= AcquirePerm) &&
!(meta_reg.self.error || meta_reg.clients.error) && !req_put
od.sourceId := req.source
od.set := req.set
Expand Down Expand Up @@ -1172,8 +1182,8 @@ class MSHR()(implicit p: Parameters) extends BaseMSHR[DirResult, SelfDirWrite, S
req_acquire,
Mux(
self_meta.hit,
Mux(req.param === NtoB && !req_promoteT, false.B, self_meta.dirty || probe_dirty || gotDirty),
gotDirty || probe_dirty
Mux(req.param === NtoB && !req_promoteT, false.B, self_meta.dirty || probe_dirty || gotDirty || nestC_saveDirty),
gotDirty || probe_dirty || nestC_saveDirty
),
false.B
)
Expand Down Expand Up @@ -1351,7 +1361,7 @@ class MSHR()(implicit p: Parameters) extends BaseMSHR[DirResult, SelfDirWrite, S
probe_dirty := probe_dirty || resp.hasData || nested_c_hit
when (
!acquire_flag && req.fromA &&
probeack_last && resp.last && !resp.hasData && !nested_c_hit && !self_meta.hit
probeack_last && resp.last && !resp.hasData && !nested_c_hit && !self_meta.hit && !nestC_save
) {
when(acquireperm_alias && !req_client_meta.hit) {
printf("BUG_REPRODUCE: nested acquire perm alias\n")
Expand Down Expand Up @@ -1506,6 +1516,8 @@ class MSHR()(implicit p: Parameters) extends BaseMSHR[DirResult, SelfDirWrite, S
// B nest A (B -> A)
io_is_nestedProbeAckData := req.fromB /*&& clients_hit*/ && req_valid
io_probeHelperFinish := req.fromB && req.fromProbeHelper && no_schedule && no_wait
io_metaValid := meta_valid || io.dirResult.valid
io_reqDirty := req.dirty && req_valid

// C nest A/B (A/B -> C)
val nest_c_set_match = io_c_status.set === req.set
Expand Down Expand Up @@ -1533,4 +1545,23 @@ class MSHR()(implicit p: Parameters) extends BaseMSHR[DirResult, SelfDirWrite, S
req.fromA && (preferCache_latch || self_meta.hit) && !acquirePermMiss

val read_miss = !self_meta.hit || self_meta.state === INVALID

val nestedReleaseDataSave = req_valid &&
io_c_status.nestedReleaseData &&
nest_c_set_match && nest_c_tag_match &&
(nest_c_way_match && io_c_status.metaValid) &&
~a_c_through

val nestedReleaseDirtyDataSave = nestedReleaseDataSave && io_c_status.reqDirty

when(nestedReleaseDataSave) {
nestC_save_flag := true.B
}

when(nestedReleaseDirtyDataSave) {
nestC_saveDirty_flag := true.B
}

nestC_save := nestC_save_flag || nestedReleaseDataSave
nestC_saveDirty := nestC_saveDirty_flag || nestedReleaseDirtyDataSave
}

0 comments on commit dbba515

Please sign in to comment.