[Cbe-oss-dev] [PATCH 1/3] spufs context switch - spu mfc save

[Kazunori Asayama]

Luke Browning wrote:
> Fix mfc control register save.
> 
> Need to preserve the current contents of the register since there
> are status bits that are used to drive restore.
> 
> Based on comments and patches from Gerhard Stenzel and Jeremy Kerr.
> 
> Signed-off-by: Luke Browning <lukebrowning at us.ibm.com>
> ---
>  arch/powerpc/platforms/cell/spufs/switch.c |   22 ++++++++++++++--------
>  1 files changed, 14 insertions(+), 8 deletions(-)
> 
> 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
> @@ -186,20 +186,25 @@ static inline void save_mfc_cntl(struct 
>  				 MFC_CNTL_SUSPEND_COMPLETE);
>  		/* fall through */
>  	case MFC_CNTL_SUSPEND_COMPLETE:
> -		if (csa) {
> -			csa->priv2.mfc_control_RW =
> -				MFC_CNTL_SUSPEND_MASK |
> -				MFC_CNTL_SUSPEND_DMA_QUEUE;
> -		}
> +		/*
> +		 * This state is seen during a page fault.  The hardware
> +		 * automatically stops the mfc which is reflected in the
> +		 * control register.  If a fault occurs, the spu needs
> +		 * to be retarted with the mfc restart bit, which occurs
> +		 * in restore_mfc_cntl.  The flag MFC_CNTL_SUSPEND_DMA_QUEUE
> +		 * is used as a flag between these two pieces of code.
> +		 */
> +		csa->priv2.mfc_control_RW = in_be64(&priv2->mfc_control_RW) |
> +					MFC_CNTL_SUSPEND_DMA_QUEUE;
>  		break;
>  	case MFC_CNTL_NORMAL_DMA_QUEUE_OPERATION:
>  		out_be64(&priv2->mfc_control_RW, MFC_CNTL_SUSPEND_DMA_QUEUE);
>  		POLL_WHILE_FALSE((in_be64(&priv2->mfc_control_RW) &
>  				  MFC_CNTL_SUSPEND_DMA_STATUS_MASK) ==
>  				 MFC_CNTL_SUSPEND_COMPLETE);
> -		if (csa) {
> -			csa->priv2.mfc_control_RW = 0;
> -		}
> +
> +		csa->priv2.mfc_control_RW = in_be64(&priv2->mfc_control_RW) &
> +					~MFC_CNTL_SUSPEND_DMA_QUEUE;
>  		break;
>  	}
>  }

You must not save some of MFC_CNTL bits at this point (Step 8),
because the SPU is still running and then they can be changed 
until the SPU is stopped at the Step 11 (save_spu_status).

For example, the SPU may issue a new DMA command after the Step 8, 
then the MFC_CNTL[Q] bit can be changed after Step 8 (save_mfc_cntl). 
So MFC_CNTL[Q] bit should be saved after the SPU is stopped. 
See the explanations in:

    http://ozlabs.org/pipermail/cbe-oss-dev/2007-August/003139.html
    http://ozlabs.org/pipermail/cbe-oss-dev/2007-August/003141.html

The MFC_CNTL[Ds] bit in the CSA is updated at the Step 12 
(save_mfc_decr) by or'ing the actual value, however, such update 
doesn't work correctly if the MFC_CNTL[Ds] changes from 1 to 0 
between the Step 8 (save_mfc_cntl) and Step 11 (save_spu_status), 
once your change is applied. So it will be needed that the 
MFC_CNTL[Ds] bit in the CSA is masked at the Step 12 (save_mfc_decr) 
or that the MFC_CNTL[Ds] is excluded at the Step 8 (save_mfc_cntl).

There may be other similar bits in the MFC_CNTL. Please look into 
behavior of each MFC_CNTL bit.


> @@ -1721,12 +1726,8 @@ static inline void restore_mfc_cntl(stru
>  	 */
>  	out_be64(&priv2->mfc_control_RW, csa->priv2.mfc_control_RW);
>  	eieio();
> -	/*
> -	 * FIXME: this is to restart a DMA that we were processing
> -	 *        before the save. better remember the fault information
> -	 *        in the csa instead.
> -	 */
> -	if ((csa->priv2.mfc_control_RW & MFC_CNTL_SUSPEND_DMA_QUEUE_MASK)) {
> +
> +	if (csa->priv2.mfc_control_RW & MFC_CNTL_SUSPEND_DMA_QUEUE) {
>  		out_be64(&priv2->mfc_control_RW, MFC_CNTL_RESTART_DMA_COMMAND);
>  		eieio();
>  	}

-- 
(ASAYAMA Kazunori
  (asayama at sm.sony.co.jp))
t