Re: [PATCH 3/3] drm/amdkfd: gfx12 context save/restore trap handler fixes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/23/2024 13:37, Lancelot SIX wrote:

@@ -622,8 +638,15 @@ L_SAVE_HWREG:
  #if ASIC_FAMILY >= CHIP_GFX12
      // Ensure no further changes to barrier or LDS state.
+    // STATE_PRIV.BARRIER_COMPLETE may change up to this point.
      s_barrier_signal    -2
      s_barrier_wait    -2
+
+    // Re-read final state of BARRIER_COMPLETE field for save.
+    s_getreg_b32    s_save_tmp, hwreg(S_STATUS_HWREG)
+    s_and_b32    s_save_tmp, s_save_tmp, SQ_WAVE_STATE_PRIV_BARRIER_COMPLETE_MASK +    s_andn2_b32    s_save_status, s_save_status, SQ_WAVE_STATE_PRIV_BARRIER_COMPLETE_MASK

Even if BARRIER_COMPLETE can be asserted while we are in the trap hadler, I do not think it can be cleared.  That being said, it might be easier to just replace the bit, making it clearer.

Yes, I chose to structure it this way to make the intent clearer. We don't gain much from dropping the s_andn2. Most of the time spent in the save handler is stalled on memory instructions.

@@ -1351,7 +1369,17 @@ L_SKIP_BARRIER_RESTORE:
      s_setreg_b32    hwreg(HW_REG_SHADER_XNACK_MASK), s_restore_xnack_mask
  #endif
+#if ASIC_FAMILY < CHIP_GFX12
      s_setreg_b32    hwreg(S_TRAPSTS_HWREG), s_restore_trapsts

Wouldn't other gfx1x architectures have a similar issue when writing TRAPSTS here?  That is if TRAPSTS.SAVECTX is set while we are restoring, wouldn't we loose it?

And for gfx11, there is TRAPSTS.HOST_TRAP that could have the same issue to some degree (not sure if we would loose the host trap completly, or re-enter with trap ID + HT bit set in ttmp1).

Prior to gfx12 context save and host trap exceptions are not delivered to a wave until STATUS.PRIV=0, i.e. it leaves the trap handler.

The changes needed for gfx12 are due to a design change in this area. Exceptions are now flagged immediately and cause re-entry to the trap if any are non-zero.



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux