Add missing barrier types for Dynamic NoC #18486

yugaoTT · 2025-02-28T14:42:08Z

Llama model hang when in Dynamic noc mode, the bug was found to be the interaction with remote cb and dm noc mode.
Bug fixed in this PR also added missing barriers for dm noc

Checklist

[All post commit] https://github.com/tenstorrent/tt-metal/actions/runs/13593269429
[Blackhole Post commit] https://github.com/tenstorrent/tt-metal/actions/runs/13594151941
model perf https://github.com/tenstorrent/tt-metal/actions/runs/13591271704
T3K nightly https://github.com/tenstorrent/tt-metal/actions/runs/13591283467
T3K model https://github.com/tenstorrent/tt-metal/actions/runs/13591305402
T3k unit test https://github.com/tenstorrent/tt-metal/actions/runs/13594793260
TG unit https://github.com/tenstorrent/tt-metal/actions/runs/13594766139

tt-aho · 2025-02-28T21:43:04Z

tt_metal/hw/firmware/src/active_erisck.cc

+        } else {
+            WAYPOINT("NKFW");
+            // Assert that no noc transactions are outstanding, to ensure that all reads and writes have landed and the
+            // NOC interface is in a known idle state for the next kernel.
+            for (int noc = 0; noc < NUM_NOCS; noc++) {
+                ASSERT(ncrisc_dynamic_noc_reads_flushed(noc));
+                ASSERT(ncrisc_dynamic_noc_nonposted_writes_sent(noc));
+                ASSERT(ncrisc_dynamic_noc_nonposted_writes_flushed(noc));
+                ASSERT(ncrisc_dynamic_noc_nonposted_atomics_flushed(noc));
+                ASSERT(ncrisc_dynamic_noc_posted_writes_sent(noc));
+            }
+            WAYPOINT("NKFD");


I thought we didn't support dynamic noc on ethernet?

tt-aho · 2025-02-28T21:45:28Z

tt_metal/hw/firmware/src/idle_erisck.cc

+        } else {
+            WAYPOINT("NKFW");
+            // Assert that no noc transactions are outstanding, to ensure that all reads and writes have landed and the
+            // NOC interface is in a known idle state for the next kernel.
+            for (int noc = 0; noc < NUM_NOCS; noc++) {
+                ASSERT(ncrisc_dynamic_noc_reads_flushed(noc));
+                ASSERT(ncrisc_dynamic_noc_nonposted_writes_sent(noc));
+                ASSERT(ncrisc_dynamic_noc_nonposted_writes_flushed(noc));
+                ASSERT(ncrisc_dynamic_noc_nonposted_atomics_flushed(noc));
+                ASSERT(ncrisc_dynamic_noc_posted_writes_sent(noc));
+            }
+            WAYPOINT("NKFD");


Same question as above

seems like debug code, will remove

tt-aho · 2025-02-28T21:48:31Z

tt_metal/hw/inc/blackhole/noc_nonblocking_api.h

+    set_noc_counter_val<dm0, NocBarrierType::READS_NUM_ISSUED>(
+        NOC_0, NOC_STATUS_READ_REG(NOC_0, NIU_MST_RD_RESP_RECEIVED));
+    set_noc_counter_val<dm0, NocBarrierType::READS_NUM_ISSUED>(NOC_1, 0);
+    set_noc_counter_val<dm1, NocBarrierType::READS_NUM_ISSUED>(NOC_0, 0);
+    set_noc_counter_val<dm1, NocBarrierType::READS_NUM_ISSUED>(
+        NOC_1, NOC_STATUS_READ_REG(NOC_1, NIU_MST_RD_RESP_RECEIVED));
+
+    set_noc_counter_val<dm0, NocBarrierType::NONPOSTED_WRITES_NUM_ISSUED>(
+        NOC_0, NOC_STATUS_READ_REG(NOC_0, NIU_MST_NONPOSTED_WR_REQ_SENT));
+    set_noc_counter_val<dm0, NocBarrierType::NONPOSTED_WRITES_NUM_ISSUED>(NOC_1, 0);
+    set_noc_counter_val<dm1, NocBarrierType::NONPOSTED_WRITES_NUM_ISSUED>(NOC_0, 0);
+    set_noc_counter_val<dm1, NocBarrierType::NONPOSTED_WRITES_NUM_ISSUED>(
+        NOC_1, NOC_STATUS_READ_REG(NOC_1, NIU_MST_NONPOSTED_WR_REQ_SENT));
+
+    set_noc_counter_val<dm0, NocBarrierType::NONPOSTED_WRITES_ACKED>(
+        NOC_0, NOC_STATUS_READ_REG(NOC_0, NIU_MST_WR_ACK_RECEIVED));
+    set_noc_counter_val<dm0, NocBarrierType::NONPOSTED_WRITES_ACKED>(NOC_1, 0);
+    set_noc_counter_val<dm1, NocBarrierType::NONPOSTED_WRITES_ACKED>(NOC_0, 0);
+    set_noc_counter_val<dm1, NocBarrierType::NONPOSTED_WRITES_ACKED>(
+        NOC_1, NOC_STATUS_READ_REG(NOC_1, NIU_MST_WR_ACK_RECEIVED));
+
+    set_noc_counter_val<dm0, NocBarrierType::NONPOSTED_ATOMICS_ACKED>(
+        NOC_0, NOC_STATUS_READ_REG(NOC_0, NIU_MST_ATOMIC_RESP_RECEIVED));
+    set_noc_counter_val<dm0, NocBarrierType::NONPOSTED_ATOMICS_ACKED>(NOC_1, 0);
+    set_noc_counter_val<dm1, NocBarrierType::NONPOSTED_ATOMICS_ACKED>(NOC_0, 0);
+    set_noc_counter_val<dm1, NocBarrierType::NONPOSTED_ATOMICS_ACKED>(
+        NOC_1, NOC_STATUS_READ_REG(NOC_1, NIU_MST_ATOMIC_RESP_RECEIVED));
+
+    set_noc_counter_val<dm0, NocBarrierType::POSTED_WRITES_NUM_ISSUED>(
+        NOC_0, NOC_STATUS_READ_REG(NOC_0, NIU_MST_POSTED_WR_REQ_SENT));
+    set_noc_counter_val<dm0, NocBarrierType::POSTED_WRITES_NUM_ISSUED>(NOC_1, 0);
+    set_noc_counter_val<dm1, NocBarrierType::POSTED_WRITES_NUM_ISSUED>(NOC_0, 0);
+    set_noc_counter_val<dm1, NocBarrierType::POSTED_WRITES_NUM_ISSUED>(
+        NOC_1, NOC_STATUS_READ_REG(NOC_1, NIU_MST_POSTED_WR_REQ_SENT));


Wondering if we can clean this up in a nice way since it's basically doing the same thing for each barrier type, register pair.

hmm not sure how to make the code shorter

tt-aho · 2025-02-28T21:53:20Z

tt_metal/hw/firmware/src/brisc.cc

+                if (noc_mode == DM_DYNAMIC_NOC) {
+                    // inc dm noc counter for BH as this is non-posted
+                    inc_noc_counter_val<
+                        static_cast<std::underlying_type_t<TensixProcessorTypes>>(TensixProcessorTypes::DM0),
+                        NocBarrierType::NONPOSTED_ATOMICS_ACKED>(noc_index, 1);
+                    // barrier till the atomic response is back
+                    while (!ncrisc_dynamic_noc_nonposted_atomics_flushed(noc_index));
+                    // reset local counters
+                    noc_local_state_init(noc_index);
+                }


Why do we have to do this increment manually, and then reset the dedicated noc counters?

this was some patches that I made to stop BH hang, I did the inc to make the barrier work in dynamic mode.
But it seems we aren't doing any barrier in dedicated mode, this can be removed

tt-aho · 2025-02-28T21:55:08Z

tt_metal/hw/inc/remote_circular_buffer_api.h

+        noc_fast_atomic_increment(
+            noc,
+            write_at_cmd_buf,
+            remote_ack_ptr_addr,
+            NOC_UNICAST_WRITE_VC,
+            aligned_page_adjustment,
+            31 /*wrap*/,
+            false /*linked*/,
+            true /*posted*/,
+            MEM_NOC_ATOMIC_RET_VAL_ADDR);


This is doing a posted atomic. Wasn't this unsafe for BH?
Also, doesn't this assume dedicated noc mode since you aren't setting the noc mode? How does this work for FW?

yeah good point, previously the remote cb setup is before run ncrisc, so it works fine since there's no other txns and we can use dedicated mode.
Now that we moved the ncrisc start before remote cb setup, I think we need to use the proper mode now.

yea for BH we need to do non posted

yugaoTT self-assigned this Feb 28, 2025

#0: Add support for all dynamic noc barriers, fix fw logic

016f839

yugaoTT force-pushed the yugao/dm_noc branch from e6e65eb to 016f839 Compare February 28, 2025 15:22

yugaoTT added 4 commits February 28, 2025 15:26

#0: cleanup

6719c28

#0: cleanup

950fb8f

#0: fix fabric misuse of dm noc

938d140

#0: fixes

73db59b

yugaoTT marked this pull request as ready for review February 28, 2025 21:38

yugaoTT requested review from TT-BrianLiu, bbradelTT, vsureshTT, edwinleeTT, mo-tenstorrent, aliuTT, ubcheema, aagarwalTT, abhullar-tt, pgkeller, tt-aho, tt-dma, tt-asaigal, cfjchu and omilyutin-tt as code owners February 28, 2025 21:38

tt-aho reviewed Feb 28, 2025

View reviewed changes

yugaoTT added 2 commits February 28, 2025 22:32

#0: use posted for BH, use noc mode in remote cb

05d5824

#0: remove erisc debug code

acc146f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add missing barrier types for Dynamic NoC #18486

Add missing barrier types for Dynamic NoC #18486

yugaoTT commented Feb 28, 2025 •

edited

Loading

tt-aho Feb 28, 2025

tt-aho Feb 28, 2025

yugaoTT Feb 28, 2025

tt-aho Feb 28, 2025

yugaoTT Feb 28, 2025

tt-aho Feb 28, 2025

yugaoTT Feb 28, 2025

tt-aho Feb 28, 2025

yugaoTT Feb 28, 2025

yugaoTT Feb 28, 2025

Add missing barrier types for Dynamic NoC #18486

Are you sure you want to change the base?

Add missing barrier types for Dynamic NoC #18486

Conversation

yugaoTT commented Feb 28, 2025 • edited Loading

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yugaoTT commented Feb 28, 2025 •

edited

Loading