[Cbe-oss-dev] [RFC] spufs: use master control bit in spu_run

[Arnd Bergmann]

On Thursday 02 November 2006 21:11, Arnd Bergmann wrote:
> This is my first shot at using the SPU master control bit to control
> execution of loaded spu contexts. The basic problem is now that we
> allow user space to access the runcntl bit, we can no longer use that
> to prevent the spu from running outside of spu_run in an idle context.
> 
> Unfortunately, I noticed here that we have a few related problems
> in the new spu_setup_isolated() function. Most importantly, it
> does not serialize against the spu_run function but tries to
> run its own code. I fixed that using the run_sema semaphore (which
> should be a mutex). Since I don't have an easy way to test isolation
> mode, I'd like to have a confirmation that this doesn't break
> recycling isolated spu contexts.
> 
> Moreover, I looked again at the spu status register and found that
> it reports an exit from isolation mode. Now, that got me the idea
> to significantly simplify recycling contexts by just checking the
> isolation exit bit in spu_run(). Whenever we enter spu_run() and
> that bit is already set, we could simply do a new isolated load,
> instead of using the recycle file. Similarly, the initial load
> can be done at spu_run time as well, instead of during the
> spu_create call, so we get only a single code path for the
> isolated load.

I've now hacked something up that should do the right thing
and I've checked it in to CVS so we can get rid of the unfortunate
recycle interface before it's too late. Of course this also
requires respective changes to libspe and needs to be tested.

Kanna and Jeremy, please review the two patches so we can get to
a working version for next week.

	Arnd <><

---
Subject: spufs: replace recycle file with automated method

It seems we don't need to use an explicit 'recycle' file if
we simply rely on the initial secure application to exit secure
mode correctly before loading the new kernel. This makes the
interface much simpler and more consistant.

The code may get even simpler if we move the initial isolated
load into the spu_run instead of doing it behind the back
of spu_create.

Right now, this patch is completely untested, due to lack
of the right simulator version for the isolation feature.

The patch needs spufs-master-control.diff to be applied first.

Please test this and report back on the success with it.

Signed-off-by: Arnd Bergmann <arnd.bergmann at de.ibm.com>

Index: linux-2.6/arch/powerpc/platforms/cell/spufs/file.c
===================================================================
--- linux-2.6.orig/arch/powerpc/platforms/cell/spufs/file.c
+++ linux-2.6/arch/powerpc/platforms/cell/spufs/file.c
@@ -1357,37 +1357,6 @@ static struct file_operations spufs_mfc_
 	.mmap	 = spufs_mfc_mmap,
 };
 
-
-static int spufs_recycle_open(struct inode *inode, struct file *file)
-{
-	file->private_data = SPUFS_I(inode)->i_ctx;
-	return nonseekable_open(inode, file);
-}
-
-static ssize_t spufs_recycle_write(struct file *file,
-		const char __user *buffer, size_t size, loff_t *pos)
-{
-	struct spu_context *ctx = file->private_data;
-	int ret;
-
-	if (!(ctx->flags & SPU_CREATE_ISOLATE))
-		return -EINVAL;
-
-	if (size < 1)
-		return -EINVAL;
-
-	ret = spu_recycle_isolated(ctx);
-
-	if (ret)
-		return ret;
-	return size;
-}
-
-static struct file_operations spufs_recycle_fops = {
-	.open	 = spufs_recycle_open,
-	.write	 = spufs_recycle_write,
-};
-
 static void spufs_npc_set(void *data, u64 val)
 {
 	struct spu_context *ctx = data;
@@ -1788,6 +1757,5 @@ struct tree_descr spufs_dir_nosched_cont
 	{ "psmap", &spufs_psmap_fops, 0666, },
 	{ "phys-id", &spufs_id_ops, 0666, },
 	{ "object-id", &spufs_object_id_ops, 0666, },
-	{ "recycle", &spufs_recycle_fops, 0222, },
 	{},
 };
Index: linux-2.6/arch/powerpc/platforms/cell/spufs/inode.c
===================================================================
--- linux-2.6.orig/arch/powerpc/platforms/cell/spufs/inode.c
+++ linux-2.6/arch/powerpc/platforms/cell/spufs/inode.c
@@ -235,7 +235,7 @@ struct file_operations spufs_context_fop
 	.fsync		= simple_sync_file,
 };
 
-static int spu_setup_isolated(struct spu_context *ctx)
+int spu_setup_isolated(struct spu_context *ctx)
 {
 	int ret;
 	u64 __iomem *mfc_cntl;
@@ -326,11 +326,6 @@ out:
 	return ret;
 }
 
-int spu_recycle_isolated(struct spu_context *ctx)
-{
-	return spu_setup_isolated(ctx);
-}
-
 static int
 spufs_mkdir(struct inode *dir, struct dentry *dentry, unsigned int flags,
 		int mode)
Index: linux-2.6/arch/powerpc/platforms/cell/spufs/run.c
===================================================================
--- linux-2.6.orig/arch/powerpc/platforms/cell/spufs/run.c
+++ linux-2.6/arch/powerpc/platforms/cell/spufs/run.c
@@ -55,16 +55,37 @@ static inline int spu_run_init(struct sp
 	int ret;
 	unsigned long runcntl = SPU_RUNCNTL_RUNNABLE;
 
-	if ((ret = spu_acquire_runnable(ctx)) != 0)
+	ret = spu_acquire_runnable(ctx);
+	if (ret)
 		return ret;
 
-	/* if we're in isolated mode, we would have started the SPU
-	 * earlier, so don't do it again now. */
+	/*
+	 * if we're in isolated mode, we would have started the SPU
+	 * earlier, so don't do it again now.
+	 */
 	if (!(ctx->flags & SPU_CREATE_ISOLATE)) {
 		ctx->ops->npc_write(ctx, *npc);
 		ctx->ops->runcntl_write(ctx, runcntl);
+	} else {
+		/*
+		 * When an isolated program has exited, we have to
+		 * reload the secure kernel in order to continue
+		 * running.
+		 */
+		if (ctx->ops->status_read(ctx) &
+				SPU_STATUS_ISOLATED_EXIT_STATUS) {
+			/*
+			 * Need to release ctx, because
+			 * spu_setup_isolated will acquire it
+			 * exclusively.
+			 */
+			spu_release(ctx);
+			ret = spu_setup_isolated(ctx);
+			if (!ret)
+				ret = spu_acquire_runnable(ctx);
+		}
 	}
-	return 0;
+	return ret;
 }
 
 static inline int spu_run_fini(struct spu_context *ctx, u32 * npc,
Index: linux-2.6/arch/powerpc/platforms/cell/spufs/spufs.h
===================================================================
--- linux-2.6.orig/arch/powerpc/platforms/cell/spufs/spufs.h
+++ linux-2.6/arch/powerpc/platforms/cell/spufs/spufs.h
@@ -182,7 +182,7 @@ void spu_yield(struct spu_context *ctx);
 int __init spu_sched_init(void);
 void __exit spu_sched_exit(void);
 
-int spu_recycle_isolated(struct spu_context *ctx);
+int spu_setup_isolated(struct spu_context *ctx);
 /*
  * spufs_wait
  * 	Same as wait_event_interruptible(), except that here