[Cbe-oss-dev] [PATCH 2/3] spufs context switch - make SPU_CONTEXT_SWITCH_PENDING atomic

Luke Browning lukebr at linux.vnet.ibm.com
Wed Apr 23 11:56:29 EST 2008


Make spu context switch pending flag atomic.

The test_bit operation does not use atomic memory primitives, so put
this flag under the spu register_lock.  SPU exception handlers must
be tightly integrated into spu context switching, so that they don't 
restart DMA while the MFC queue is being saved or restored.

Signed-Off-By: Luke Browning <lukebrowning at us.ibm.com>

---

I left the setting of the CONTEXT_SWITCH_PENDING flag in the restore 
part of the algorithm but I don't mind removing it.   It is not wrong 
but may not be necessary.

Index: public_git/arch/powerpc/platforms/cell/spu_base.c
===================================================================
--- public_git.orig/arch/powerpc/platforms/cell/spu_base.c
+++ public_git/arch/powerpc/platforms/cell/spu_base.c
@@ -139,8 +139,10 @@ static void spu_restart_dma(struct spu *
 {
 	struct spu_priv2 __iomem *priv2 = spu->priv2;
 
+	spin_lock_irq(&spu->register_lock);
 	if (!test_bit(SPU_CONTEXT_SWITCH_PENDING, &spu->flags))
 		out_be64(&priv2->mfc_control_RW, MFC_CNTL_RESTART_DMA_COMMAND);
+	spin_unlock_irq(&spu->register_lock);
 }
 
 static inline void spu_load_slb(struct spu *spu, int slbe, struct spu_slb *slb)
@@ -201,7 +203,9 @@ static int __spu_trap_data_seg(struct sp
 	}
 	slb.vsid |= mmu_psize_defs[psize].sllp;
 
+	spin_lock_irq(&spu->register_lock);
 	spu_load_slb(spu, spu->slb_replace, &slb);
+	spin_unlock_irq(&spu->register_lock);
 
 	spu->slb_replace++;
 	if (spu->slb_replace >= 8)
@@ -346,14 +350,14 @@ spu_irq_class_1(int irq, void *data)
 	if (stat & CLASS1_STORAGE_FAULT_INTR)
 		spu_mfc_dsisr_set(spu, 0ul);
 	spu_int_stat_clear(spu, 1, stat);
-
-	if (stat & CLASS1_SEGMENT_FAULT_INTR)
-		__spu_trap_data_seg(spu, dar);
-
 	spin_unlock(&spu->register_lock);
+
 	pr_debug("%s: %lx %lx %lx %lx\n", __FUNCTION__, mask, stat,
 			dar, dsisr);
 
+	if (stat & CLASS1_SEGMENT_FAULT_INTR)
+		__spu_trap_data_seg(spu, dar);
+
 	if (stat & CLASS1_STORAGE_FAULT_INTR)
 		__spu_trap_data_map(spu, dar, dsisr);
 
Index: public_git/arch/powerpc/platforms/cell/spufs/switch.c
===================================================================
--- public_git.orig/arch/powerpc/platforms/cell/spufs/switch.c
+++ public_git/arch/powerpc/platforms/cell/spufs/switch.c
@@ -130,7 +130,7 @@ static inline void disable_interrupts(st
 	spu_int_mask_set(spu, 0, 0ul);
 	spu_int_mask_set(spu, 1, 0ul);
 	spu_int_mask_set(spu, 2, 0ul);
-	eieio();
+	set_bit(SPU_CONTEXT_SWITCH_PENDING, &spu->flags);
 	spin_unlock_irq(&spu->register_lock);
 	synchronize_irq(spu->irqs[0]);
 	synchronize_irq(spu->irqs[1]);
@@ -166,9 +166,12 @@ static inline void set_switch_pending(st
 	/* Save, Step 7:
 	 * Restore, Step 5:
 	 *     Set a software context switch pending flag.
+	 *
+	 * SPU_CONTEXT_SWITCH_PENDING is set in disable_interrupts() and
+	 * clear_interrupts() under the spu register_lock, so that exception
+	 * handlers can atomically read the flag and not restart dma during
+	 * critical context save and restore windows.
 	 */
-	set_bit(SPU_CONTEXT_SWITCH_PENDING, &spu->flags);
-	mb();
 }
 
 static inline void save_mfc_cntl(struct spu_state *csa, struct spu *spu)
@@ -732,10 +735,9 @@ static inline void set_switch_active(str
 	 *     Change the software context switch pending flag
 	 *     to context switch active.
 	 *
-	 *     This implementation does not uses a switch active flag.
+	 * This step is performed in enable_interrupts so that the flag
+	 * can be read atomically by interrupt handling code.
 	 */
-	clear_bit(SPU_CONTEXT_SWITCH_PENDING, &spu->flags);
-	mb();
 }
 
 static inline void enable_interrupts(struct spu_state *csa, struct spu *spu)
@@ -752,6 +754,7 @@ static inline void enable_interrupts(str
 	 *     (translation) interrupts.
 	 */
 	spin_lock_irq(&spu->register_lock);
+	clear_bit(SPU_CONTEXT_SWITCH_PENDING, &spu->flags);
 	spu_int_stat_clear(spu, 0, CLASS0_INTR_MASK);
 	spu_int_stat_clear(spu, 1, CLASS1_INTR_MASK);
 	spu_int_stat_clear(spu, 2, CLASS2_INTR_MASK);
@@ -1410,6 +1413,7 @@ static inline void clear_interrupts(stru
 	spu_int_stat_clear(spu, 0, CLASS0_INTR_MASK);
 	spu_int_stat_clear(spu, 1, CLASS1_INTR_MASK);
 	spu_int_stat_clear(spu, 2, CLASS2_INTR_MASK);
+	set_bit(SPU_CONTEXT_SWITCH_PENDING, &spu->flags);
 	spin_unlock_irq(&spu->register_lock);
 }
 
@@ -1747,7 +1751,7 @@ static inline void reset_switch_active(s
 {
 	/* Restore, Step 74:
 	 *     Reset the "context switch active" flag.
-	 *     Not performed by this implementation.
+	 * Done in reenable_interrupts/enable_interrupts for locking purposes.
 	 */
 }
 
@@ -1757,6 +1761,7 @@ static inline void reenable_interrupts(s
 	 *     Re-enable SPU interrupts.
 	 */
 	spin_lock_irq(&spu->register_lock);
+	clear_bit(SPU_CONTEXT_SWITCH_PENDING, &spu->flags);
 	spu_int_mask_set(spu, 0, csa->priv1.int_mask_class0_RW);
 	spu_int_mask_set(spu, 1, csa->priv1.int_mask_class1_RW);
 	spu_int_mask_set(spu, 2, csa->priv1.int_mask_class2_RW);
@@ -1778,7 +1783,12 @@ static int quiece_spu(struct spu_state *
 	if (check_spu_isolate(prev, spu)) {	/* Step 2. */
 		return 2;
 	}
-	disable_interrupts(prev, spu);	        /* Step 3. */
+
+	/*
+	 * The section delineated with PENDING below, steps 3-49, is
+	 * protected from dma restart operations from PPE exception handlers.
+	 */
+	disable_interrupts(prev, spu);	        /* Step 3. */	/* PENDING */
 	set_watchdog_timer(prev, spu);	        /* Step 4. */
 	inhibit_user_access(prev, spu);	        /* Step 5. */
 	if (check_spu_isolate(prev, spu)) {	/* Step 6. */
@@ -1847,8 +1857,8 @@ static void save_lscsa(struct spu_state 
 	resume_mfc_queue(prev, spu);	/* Step 46. */
 	/* Step 47. */
 	setup_mfc_slbs(prev, spu, spu_save_code, sizeof(spu_save_code));
-	set_switch_active(prev, spu);	/* Step 48. */
-	enable_interrupts(prev, spu);	/* Step 49. */
+	reset_switch_active(prev, spu);	/* Step 48. */
+	enable_interrupts(prev, spu);	/* Step 49. */	/* ~PENDING */
 	save_ls_16kb(prev, spu);	/* Step 50. */
 	set_spu_npc(prev, spu);	        /* Step 51. */
 	set_signot1(prev, spu);		/* Step 52. */
@@ -1910,9 +1920,11 @@ static void harvest(struct spu_state *pr
 	 * Perform steps 2-25 of SPU context restore sequence,
 	 * which resets an SPU either after a failed save, or
 	 * when using SPU for first time.
+	 *
+	 * The section delineated with PENDING below, steps 2-22, is
+	 * protected from dma restart operations from PPE exception handlers.
 	 */
-
-	disable_interrupts(prev, spu);	        /* Step 2.  */
+	disable_interrupts(prev, spu);	        /* Step 2.  */	/* PENDING */
 	inhibit_user_access(prev, spu);	        /* Step 3.  */
 	terminate_spu_app(prev, spu);	        /* Step 4.  */
 	set_switch_pending(prev, spu);	        /* Step 5.  */
@@ -1933,8 +1945,8 @@ static void harvest(struct spu_state *pr
 	spu_invalidate_slbs(spu);		/* Step 19. */
 	reset_ch_part1(prev, spu);	        /* Step 20. */
 	reset_ch_part2(prev, spu);	        /* Step 21. */
-	enable_interrupts(prev, spu);	        /* Step 22. */
-	set_switch_active(prev, spu);	        /* Step 23. */
+	enable_interrupts(prev, spu);	        /* Step 22. */	/* ~PENDING */
+	reset_switch_active(prev, spu);	        /* Step 23. */
 	set_mfc_tclass_id(prev, spu);	        /* Step 24. */
 	resume_mfc_queue(prev, spu);	        /* Step 25. */
 }
@@ -1980,7 +1992,8 @@ static void restore_csa(struct spu_state
 	suspend_mfc(next, spu);	                /* Step 46. */
 	wait_suspend_mfc_complete(next, spu);	/* Step 47. */
 	issue_mfc_tlbie(next, spu);	        /* Step 48. */
-	clear_interrupts(next, spu);	        /* Step 49. */
+	clear_interrupts(next, spu);	        /* Step 49. */	/* PENDING */
+	set_switch_pending(next, spu);	        /* Step 74. */
 	restore_mfc_queues(next, spu);	        /* Step 50. */
 	restore_ppu_querymask(next, spu);	/* Step 51. */
 	restore_ppu_querytype(next, spu);	/* Step 52. */
@@ -2006,7 +2019,7 @@ static void restore_csa(struct spu_state
 	restore_mfc_cntl(next, spu);	        /* Step 72. */
 	enable_user_access(next, spu);	        /* Step 73. */
 	reset_switch_active(next, spu);	        /* Step 74. */
-	reenable_interrupts(next, spu);	        /* Step 75. */
+	reenable_interrupts(next, spu);	        /* Step 75. */	/* ~PENDING */
 }
 
 static int __do_spu_save(struct spu_state *prev, struct spu *spu)





More information about the cbe-oss-dev mailing list