[PATCH] spufs-prevent-sdr1-access

[Arnd Bergmann]

On Wednesday 25 January 2006 19:25, Geoff Levand wrote:
> spufs-prevent-sdr1-access.patch:
> 
> SDR1 is a hypervisor resource.  It is not accessible when
> kernel is on LPAR.

Ok, good catch. We need to change it, but I think there is a better
way to do it than this patch.
 
> diff -uprN linux-2.6.15+spufs/arch/powerpc/platforms/cell/spu_base.c linux-2.6.15+spufs.mod/arch/powerpc/platforms/cell/spu_base.c
> --- linux-2.6.15+spufs/arch/powerpc/platforms/cell/spu_base.c	2006-01-18 20:46:38.000000000 +0900
> +++ linux-2.6.15+spufs.mod/arch/powerpc/platforms/cell/spu_base.c	2006-01-20 17:18:41.000000000 +0900
> @@ -626,7 +626,8 @@ static int __init create_spu(struct devi
>  	spu->dsisr = 0UL;
>  	spin_lock_init(&spu->register_lock);
> 
> -	spu_mfc_sdr_set(spu, mfspr(SPRN_SDR1));
> +	if (!platform_is_lpar())
> +		spu_mfc_sdr_set(spu, mfspr(SPRN_SDR1));
>  	spu_mfc_sr1_set(spu, 0x33);
> 
>  	spu->ibox_callback = NULL;

The definition of spu_mfc_sdr_set() already knows about wether it's
running on hypervisor or not. Also, we know that we always pass the
same value. I'd suggest changing this to 'spu_mfc_sdr1_setup();'

which is defined as

void spu_mfc_sdr1_setup(struct spu *spu)
{
	if (!firmware_has_feature(FW_FEATURE_LPAR))
		out_be64(&spu->priv1->mfc_sdr_RW, mfspr(SPRN_SDR1));
	else
		do_whatever_is_necessary(spu);
}

> diff -uprN linux-2.6.15+spufs/arch/powerpc/platforms/cell/spufs/switch.c linux-2.6.15+spufs.mod/arch/powerpc/platforms/cell/spufs/switch.c
> --- linux-2.6.15+spufs/arch/powerpc/platforms/cell/spufs/switch.c	2006-01-18 20:46:38.000000000 +0900
> +++ linux-2.6.15+spufs.mod/arch/powerpc/platforms/cell/spufs/switch.c	2006-01-20 17:17:30.000000000 +0900
> @@ -2136,7 +2136,8 @@ static void init_priv1(struct spu_state
>  	    MFC_STATE1_RELOCATE_MASK | MFC_STATE1_BUS_TLBIE_MASK;
> 
>  	/* Set storage description.  */
> -	csa->priv1.mfc_sdr_RW = mfspr(SPRN_SDR1);
> +	if (!platform_is_lpar())
> +		csa->priv1.mfc_sdr_RW = mfspr(SPRN_SDR1);
> 
>  	/* Enable OS-specific set of interrupts. */
>  	csa->priv1.int_mask_class0_RW = CLASS0_ENABLE_DMA_ALIGNMENT_INTR |
> 

I don't think we need to read SPRN_SDR1 here at all, since the value should
be constant over the lifetime of the kernel. Can you try ripping out
mfc_sdr_RW from csa->priv1 completely?

	Arnd <><