[Cbe-oss-dev] [PATCH 3/3] spufs context switch - fix interrupt routing

[Jeremy Kerr]

Luke,

> 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
> @@ -762,6 +762,7 @@ static inline void enable_interrupts(str
>  	spu_int_mask_set(spu, 0, 0ul);
>  	spu_int_mask_set(spu, 1, class1_mask);
>  	spu_int_mask_set(spu, 2, 0ul);
> +	eieio();
>  	spin_unlock_irq(&spu->register_lock);
>  }

do we need the eieio() here? I would have thought that the spin_unlock 
provides sufficient ordering.

> +static inline void set_int_route(struct spu_state *csa, struct spu
> *spu) +{
> +	/*
> +	 * FIX ME: this is wrong with asynchronous spu scheduling.  If the
> +	 * caller is the time slicer, all interrupts will be steered to
> +	 * the same cpu.  It also needs to be made NUMA aware.
> +	 */
> +	spu_cpu_affinity_set(spu, raw_smp_processor_id());
> +}
> +

Looks good.

>  static inline void restore_other_spu_access(struct spu_state *csa,
>  					    struct spu *spu)
>  {
> @@ -1989,7 +2000,7 @@ static void restore_csa(struct spu_state
>  	wait_suspend_mfc_complete(next, spu);	/* Step 47. */
>  	issue_mfc_tlbie(next, spu);	        /* Step 48. */
>  	clear_interrupts(next, spu);	        /* Step 49. */	/* PENDING */
> -	set_switch_pending(next, spu);	        /* Step 74. */
> +	set_switch_pending(next, spu);	        /* NEW      */
>  	restore_mfc_queues(next, spu);	        /* Step 50. */
>  	restore_ppu_querymask(next, spu);	/* Step 51. */
>  	restore_ppu_querytype(next, spu);	/* Step 52. */
> @@ -2010,11 +2021,12 @@ static void restore_csa(struct spu_state
>  	check_ppuint_mb_stat(next, spu);	/* Step 67. */
>  	spu_invalidate_slbs(spu);		/* Modified Step 68. */
>  	restore_mfc_sr1(next, spu);	        /* Step 69. */
> +	set_int_route(next, spu);		/* NEW	    */
>  	restore_other_spu_access(next, spu);	/* Step 70. */
>  	restore_spu_runcntl(next, spu);	        /* Step 71. */
>  	restore_mfc_cntl(next, spu);	        /* Step 72. */
>  	enable_user_access(next, spu);	        /* Step 73. */
> -	reset_switch_active(next, spu);	        /* Step 74. */
> +	reset_switch_active(next, spu);	        /* NEW 	    */
>  	reenable_interrupts(next, spu);	        /* Step 75. */	/* ~PENDING
> */ }

I think only the actual set_int_route change is required.

Cheers,


Jeremy