From 30fcb41d0126f20cba8144979c646b71a885aaf1 Mon Sep 17 00:00:00 2001 From: sumailyyc <810583334@qq.com> Date: Wed, 15 May 2024 16:03:33 +0800 Subject: [PATCH] MSHR: fix a dual-core bug when ReleaseData nests the Probe 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. --- src/main/scala/huancun/Slice.scala | 6 +++ .../scala/huancun/noninclusive/MSHR.scala | 39 +++++++++++++++++-- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/main/scala/huancun/Slice.scala b/src/main/scala/huancun/Slice.scala index 8e22b61d..d7edb73c 100644 --- a/src/main/scala/huancun/Slice.scala +++ b/src/main/scala/huancun/Slice.scala @@ -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 @@ -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 @@ -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 diff --git a/src/main/scala/huancun/noninclusive/MSHR.scala b/src/main/scala/huancun/noninclusive/MSHR.scala index 5ee8d70a..cddd9aba 100644 --- a/src/main/scala/huancun/noninclusive/MSHR.scala +++ b/src/main/scala/huancun/noninclusive/MSHR.scala @@ -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 { @@ -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) @@ -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 @@ -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 } @@ -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 @@ -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 ) @@ -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") @@ -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 @@ -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 }