Skip to contentSkip to conversation

Cell: Wrap master run control bit

PATCH In review Geoff Levand 3 participants 9 messages Sep 4, 2007 – Oct 24, 2007

Subsystems (4)
PowerPC Cell broadband engine architecture PS3 platform support SPU file system
lore
Share & tools
lore

Geoff Levand

author
#0 lore raw

PATCH 2/2 Cell: Wrap master run control bit

From: Masato Noguchi <[email protected]>

Add platform specific SPU run control routines.

The current spufs_run_spu() implementation uses the SPU master run control bit (MFC_SR1[S]) to control SPE execution, but the PS3 hypervisor does not support the use of this feature. This change adds run control wrapper routines. The bare metal routines use the master run control bit, and the PS3 specific routines use the priv2 run control register.

Signed-off-by Masato Noguchi <[email protected]>
Signed-off-by Geoff Levand <[email protected]>

---

Arnd, please consider for 2.6.24.

 arch/powerpc/platforms/cell/spu_manage.c        |   15 +++++++++++++++
 arch/powerpc/platforms/cell/spufs/backing_ops.c |    6 ++++++
 arch/powerpc/platforms/cell/spufs/hw_ops.c      |   10 ++++++++++
 arch/powerpc/platforms/cell/spufs/run.c         |    4 ++--
 arch/powerpc/platforms/cell/spufs/spufs.h       |    1 +
 arch/powerpc/platforms/ps3/spu.c                |   14 ++++++++++++++
 include/asm-powerpc/spu_priv1.h                 |   15 +++++++++++++++
 7 files changed, 63 insertions(+), 2 deletions(-)
arch/powerpc/platforms/cell/spu_manage.c +15 -0
@@ -35,6 +35,7 @@
 #include <asm/firmware.h>
 #include <asm/prom.h>
 
+#include "spufs/spufs.h"
 #include "interrupt.h"
 
 struct device_node *spu_devnode(struct spu *spu)

@@ -369,6 +370,18 @@ static int of_destroy_spu(struct spu *sp
 	return 0;
 }
 
+static int enable_spu_by_master_run(struct spu_context *ctx)
+{
+	ctx->ops->master_start(ctx);
+	return 0;
+}
+
+static int disable_spu_by_master_run(struct spu_context *ctx)
+{
+	ctx->ops->master_stop(ctx);
+	return 0;
+}
+
 /* Hardcoded affinity idxs for qs20 */
 #define QS20_SPES_PER_BE 8
 static int qs20_reg_idxs[QS20_SPES_PER_BE] =   { 0, 2, 4, 6, 7, 5, 3, 1 };

@@ -535,5 +548,7 @@ const struct spu_management_ops spu_mana
 	.enumerate_spus = of_enumerate_spus,
 	.create_spu = of_create_spu,
 	.destroy_spu = of_destroy_spu,
+	.enable_spu = enable_spu_by_master_run,
+	.disable_spu = disable_spu_by_master_run,
 	.init_affinity = init_affinity,
 };
arch/powerpc/platforms/cell/spufs/backing_ops.c +6 -0
@@ -285,6 +285,11 @@ static void spu_backing_runcntl_write(st
 	spin_unlock(&ctx->csa.register_lock);
 }
 
+static void spu_backing_runcntl_stop(struct spu_context *ctx)
+{
+	spu_backing_runcntl_write(ctx, SPU_RUNCNTL_STOP);
+}
+
 static void spu_backing_master_start(struct spu_context *ctx)
 {
 	struct spu_state *csa = &ctx->csa;

@@ -381,6 +386,7 @@ struct spu_context_ops spu_backing_ops =
 	.get_ls = spu_backing_get_ls,
 	.runcntl_read = spu_backing_runcntl_read,
 	.runcntl_write = spu_backing_runcntl_write,
+	.runcntl_stop = spu_backing_runcntl_stop,
 	.master_start = spu_backing_master_start,
 	.master_stop = spu_backing_master_stop,
 	.set_mfc_query = spu_backing_set_mfc_query,
arch/powerpc/platforms/cell/spufs/hw_ops.c +10 -0
@@ -220,6 +220,15 @@ static void spu_hw_runcntl_write(struct 
 	spin_unlock_irq(&ctx->spu->register_lock);
 }
 
+static void spu_hw_runcntl_stop(struct spu_context *ctx)
+{
+	spin_lock_irq(&ctx->spu->register_lock);
+	out_be32(&ctx->spu->problem->spu_runcntl_RW, SPU_RUNCNTL_STOP);
+	while(in_be32(&ctx->spu->problem->spu_status_R) & SPU_STATUS_RUNNING)
+		cpu_relax();
+	spin_unlock_irq(&ctx->spu->register_lock);
+}
+
 static void spu_hw_master_start(struct spu_context *ctx)
 {
 	struct spu *spu = ctx->spu;

@@ -321,6 +330,7 @@ struct spu_context_ops spu_hw_ops = {
 	.get_ls = spu_hw_get_ls,
 	.runcntl_read = spu_hw_runcntl_read,
 	.runcntl_write = spu_hw_runcntl_write,
+	.runcntl_stop = spu_hw_runcntl_stop,
 	.master_start = spu_hw_master_start,
 	.master_stop = spu_hw_master_stop,
 	.set_mfc_query = spu_hw_set_mfc_query,
arch/powerpc/platforms/cell/spufs/run.c +2 -2
@@ -302,7 +302,7 @@ long spufs_run_spu(struct spu_context *c
 	if (mutex_lock_interruptible(&ctx->run_mutex))
 		return -ERESTARTSYS;
 
-	ctx->ops->master_start(ctx);
+	spu_enable_spu(ctx);
 	ctx->event_return = 0;
 
 	spu_acquire(ctx);

@@ -376,7 +376,7 @@ long spufs_run_spu(struct spu_context *c
 		ctx->stats.libassist++;
 
 
-	ctx->ops->master_stop(ctx);
+	spu_disable_spu(ctx);
 	ret = spu_run_fini(ctx, npc, &status);
 	spu_yield(ctx);
arch/powerpc/platforms/cell/spufs/spufs.h +1 -0
@@ -170,6 +170,7 @@ struct spu_context_ops {
 	char*(*get_ls) (struct spu_context * ctx);
 	 u32 (*runcntl_read) (struct spu_context * ctx);
 	void (*runcntl_write) (struct spu_context * ctx, u32 data);
+	void (*runcntl_stop) (struct spu_context * ctx);
 	void (*master_start) (struct spu_context * ctx);
 	void (*master_stop) (struct spu_context * ctx);
 	int (*set_mfc_query)(struct spu_context * ctx, u32 mask, u32 mode);
arch/powerpc/platforms/ps3/spu.c +14 -0
@@ -28,6 +28,7 @@
 #include <asm/spu_priv1.h>
 #include <asm/lv1call.h>
 
+#include "../cell/spufs/spufs.h"
 #include "platform.h"
 
 /* spu_management_ops */

@@ -419,10 +420,23 @@ static int ps3_init_affinity(void)
 	return 0;
 }
 
+static int ps3_enable_spu(struct spu_context *ctx)
+{
+	return -ENOSYS;
+}
+
+static int ps3_disable_spu(struct spu_context *ctx)
+{
+	ctx->ops->runcntl_stop(ctx);
+	return -ENOSYS;
+}
+
 const struct spu_management_ops spu_management_ps3_ops = {
 	.enumerate_spus = ps3_enumerate_spus,
 	.create_spu = ps3_create_spu,
 	.destroy_spu = ps3_destroy_spu,
+	.enable_spu = ps3_enable_spu,
+	.disable_spu = ps3_disable_spu,
 	.init_affinity = ps3_init_affinity,
 };
include/asm-powerpc/spu_priv1.h +15 -0
@@ -24,6 +24,7 @@
 #include <linux/types.h>
 
 struct spu;
+struct spu_context;
 
 /* access to priv1 registers */
 

@@ -178,6 +179,8 @@ struct spu_management_ops {
 	int (*enumerate_spus)(int (*fn)(void *data));
 	int (*create_spu)(struct spu *spu, void *data);
 	int (*destroy_spu)(struct spu *spu);
+	int (*enable_spu)(struct spu_context *ctx);
+	int (*disable_spu)(struct spu_context *ctx);
 	int (*init_affinity)(void);
 };
 

@@ -207,6 +210,18 @@ spu_init_affinity (void)
 	return spu_management_ops->init_affinity();
 }
 
+static inline int
+spu_enable_spu (struct spu_context *ctx)
+{
+	return spu_management_ops->enable_spu(ctx);
+}
+
+static inline int
+spu_disable_spu (struct spu_context *ctx)
+{
+	return spu_management_ops->disable_spu(ctx);
+}
+
 /*
  * The declarations folowing are put here for convenience
  * and only intended to be used by the platform setup code.
#1 lore raw
Quoted 12 lines · Geoff Levand

From: Masato Noguchi <[email protected]>

Add platform specific SPU run control routines.

The current spufs_run_spu() implementation uses the SPU master run control bit (MFC_SR1[S]) to control SPE execution, but the PS3 hypervisor does not support the use of this feature. This change adds run control wrapper routines. The bare metal routines use the master run control bit, and the PS3 specific routines use the priv2 run control register.

Quoted 2 lines
Signed-off-by Masato Noguchi <[email protected]>
Signed-off-by Geoff Levand <[email protected]>

Hmm, ok. Since I don't think we'll get to do the right solution (unmapping the ps registers on the ps3) that soon, doing this patch is at least better than the current situation where we end up with an oops every time we leave spu_run.

It would probably be good to mention the remaining problem with the current approach in the changelog and as a comment in the ps3 specific functions.

include/asm-powerpc/spu_priv1.h struct spu_management_ops { · line 185

Quoted 8 lines
include/asm-powerpc/spu_priv1.h +2 -0
@@ -178,6 +179,8 @@ struct spu_management_ops {
 	int (*enumerate_spus)(int (*fn)(void *data));
 	int (*create_spu)(struct spu *spu, void *data);
 	int (*destroy_spu)(struct spu *spu);
+	int (*enable_spu)(struct spu_context *ctx);
+	int (*disable_spu)(struct spu_context *ctx);
 	int (*init_affinity)(void);
 };

Also, I think you should make the return type of the callback 'void' since the result is not used anywhere.

	Arnd <><
#2 lore raw
Quoted 15 lines · Arnd Bergmann
Quoted 12 lines · Geoff Levand

=20
From: Masato Noguchi <[email protected]>
=20
Add platform specific SPU run control routines.
=20
The current spufs_run_spu() implementation uses the SPU master run control bit (MFC_SR1[S]) to control SPE execution, but the PS3 hypervisor does not support the use of this feature. =A0This change adds run control wrapper routines. =A0The =A0bare metal routines use the master run control bit, and the PS3 specific routines use the priv2 run control register.
=20

Quoted 2 lines
Signed-off-by Masato Noguchi <[email protected]>
Signed-off-by Geoff Levand <[email protected]>

One more thing, please send spufs related patches to Jeremy instead of to me. I'm happy to give advice on the patch contents, but since Jeremy has taken over maintainership of spufs, merging a patch is his decision in the end.

	Arnd <><
author
#3 lore raw

PATCH v2 1/1 Cell: Wrap master run control bit

In reply to Arnd Bergmann

Subject: Cell: Wrap master run control bit

From: Masato Noguchi <[email protected]>

Add platform specific SPU run control routines to the spufs. The current spufs implementation uses the SPU master run control bit (MFC_SR1[S]) to control SPE execution, but the PS3 hypervisor does not support the use of this feature.

This change adds the run control wrapper routies spu_enable_spu() and spu_disable_spu(). The bare metal routines use the master run control bit, and the PS3 specific routines use the priv2 run control register.

An outstanding enhancement for the PS3 would be to add a guard to check for incorrect access to the spu problem state when the spu context is disabled. This check could be implemented with a flag added to the spu context that would inhibit mapping problem state pages, and a routine to unmap spu problem state pages. When the spu is enabled with ps3_enable_spu() the flag would be set allowing pages to be mapped, and when the spu is disabled with ps3_disable_spu() the flag would be cleared and mapped problem state pages would be unmapped.

Signed-off-by Masato Noguchi <[email protected]>
Signed-off-by Geoff Levand <[email protected]>

---
v2:
o Added comments about unmapping PS pages when disabled.

 arch/powerpc/platforms/cell/spu_manage.c        |   15 +++++++++++++
 arch/powerpc/platforms/cell/spufs/backing_ops.c |    6 +++++
 arch/powerpc/platforms/cell/spufs/hw_ops.c      |   10 ++++++++
 arch/powerpc/platforms/cell/spufs/run.c         |    4 +--
 arch/powerpc/platforms/cell/spufs/spufs.h       |    1 
 arch/powerpc/platforms/ps3/spu.c                |   27 ++++++++++++++++++++++++
 include/asm-powerpc/spu_priv1.h                 |   15 +++++++++++++
 7 files changed, 76 insertions(+), 2 deletions(-)
arch/powerpc/platforms/cell/spu_manage.c +15 -0
@@ -35,6 +35,7 @@
 #include <asm/firmware.h>
 #include <asm/prom.h>
 
+#include "spufs/spufs.h"
 #include "interrupt.h"
 
 struct device_node *spu_devnode(struct spu *spu)

@@ -369,6 +370,18 @@ static int of_destroy_spu(struct spu *sp
 	return 0;
 }
 
+static int enable_spu_by_master_run(struct spu_context *ctx)
+{
+	ctx->ops->master_start(ctx);
+	return 0;
+}
+
+static int disable_spu_by_master_run(struct spu_context *ctx)
+{
+	ctx->ops->master_stop(ctx);
+	return 0;
+}
+
 /* Hardcoded affinity idxs for qs20 */
 #define QS20_SPES_PER_BE 8
 static int qs20_reg_idxs[QS20_SPES_PER_BE] =   { 0, 2, 4, 6, 7, 5, 3, 1 };

@@ -535,5 +548,7 @@ const struct spu_management_ops spu_mana
 	.enumerate_spus = of_enumerate_spus,
 	.create_spu = of_create_spu,
 	.destroy_spu = of_destroy_spu,
+	.enable_spu = enable_spu_by_master_run,
+	.disable_spu = disable_spu_by_master_run,
 	.init_affinity = init_affinity,
 };
arch/powerpc/platforms/cell/spufs/backing_ops.c +6 -0
@@ -285,6 +285,11 @@ static void spu_backing_runcntl_write(st
 	spin_unlock(&ctx->csa.register_lock);
 }
 
+static void spu_backing_runcntl_stop(struct spu_context *ctx)
+{
+	spu_backing_runcntl_write(ctx, SPU_RUNCNTL_STOP);
+}
+
 static void spu_backing_master_start(struct spu_context *ctx)
 {
 	struct spu_state *csa = &ctx->csa;

@@ -381,6 +386,7 @@ struct spu_context_ops spu_backing_ops =
 	.get_ls = spu_backing_get_ls,
 	.runcntl_read = spu_backing_runcntl_read,
 	.runcntl_write = spu_backing_runcntl_write,
+	.runcntl_stop = spu_backing_runcntl_stop,
 	.master_start = spu_backing_master_start,
 	.master_stop = spu_backing_master_stop,
 	.set_mfc_query = spu_backing_set_mfc_query,
arch/powerpc/platforms/cell/spufs/hw_ops.c +10 -0
@@ -220,6 +220,15 @@ static void spu_hw_runcntl_write(struct 
 	spin_unlock_irq(&ctx->spu->register_lock);
 }
 
+static void spu_hw_runcntl_stop(struct spu_context *ctx)
+{
+	spin_lock_irq(&ctx->spu->register_lock);
+	out_be32(&ctx->spu->problem->spu_runcntl_RW, SPU_RUNCNTL_STOP);
+	while(in_be32(&ctx->spu->problem->spu_status_R) & SPU_STATUS_RUNNING)
+		cpu_relax();
+	spin_unlock_irq(&ctx->spu->register_lock);
+}
+
 static void spu_hw_master_start(struct spu_context *ctx)
 {
 	struct spu *spu = ctx->spu;

@@ -321,6 +330,7 @@ struct spu_context_ops spu_hw_ops = {
 	.get_ls = spu_hw_get_ls,
 	.runcntl_read = spu_hw_runcntl_read,
 	.runcntl_write = spu_hw_runcntl_write,
+	.runcntl_stop = spu_hw_runcntl_stop,
 	.master_start = spu_hw_master_start,
 	.master_stop = spu_hw_master_stop,
 	.set_mfc_query = spu_hw_set_mfc_query,
arch/powerpc/platforms/cell/spufs/run.c +2 -2
@@ -302,7 +302,7 @@ long spufs_run_spu(struct spu_context *c
 	if (mutex_lock_interruptible(&ctx->run_mutex))
 		return -ERESTARTSYS;
 
-	ctx->ops->master_start(ctx);
+	spu_enable_spu(ctx);
 	ctx->event_return = 0;
 
 	spu_acquire(ctx);

@@ -376,7 +376,7 @@ long spufs_run_spu(struct spu_context *c
 		ctx->stats.libassist++;
 
 
-	ctx->ops->master_stop(ctx);
+	spu_disable_spu(ctx);
 	ret = spu_run_fini(ctx, npc, &status);
 	spu_yield(ctx);
arch/powerpc/platforms/cell/spufs/spufs.h +1 -0
@@ -170,6 +170,7 @@ struct spu_context_ops {
 	char*(*get_ls) (struct spu_context * ctx);
 	 u32 (*runcntl_read) (struct spu_context * ctx);
 	void (*runcntl_write) (struct spu_context * ctx, u32 data);
+	void (*runcntl_stop) (struct spu_context * ctx);
 	void (*master_start) (struct spu_context * ctx);
 	void (*master_stop) (struct spu_context * ctx);
 	int (*set_mfc_query)(struct spu_context * ctx, u32 mask, u32 mode);
arch/powerpc/platforms/ps3/spu.c +27 -0
@@ -28,6 +28,7 @@
 #include <asm/spu_priv1.h>
 #include <asm/lv1call.h>
 
+#include "../cell/spufs/spufs.h"
 #include "platform.h"
 
 /* spu_management_ops */

@@ -419,10 +420,36 @@ static int ps3_init_affinity(void)
 	return 0;
 }
 
+/**
+ * ps3_enable_spu - Type of spe to create.
+ *
+ * An outstanding enhancement for the PS3 would be to add a guard to check
+ * for incorrect access to the spu problem state when the spu context is
+ * disabled.  This check could be implemented with a flag added to the spu
+ * context that would inhibit mapping problem state pages, and a routine
+ * to unmap spu problem state pages.  When the spu is enabled with
+ * ps3_enable_spu() the flag would be set allowing pages to be mapped,
+ * and when the spu is disabled with ps3_disable_spu() the flag would be
+ * cleared and the mapped problem state pages would be unmapped.
+ */
+
+static int ps3_enable_spu(struct spu_context *ctx)
+{
+	return -ENOSYS;
+}
+
+static int ps3_disable_spu(struct spu_context *ctx)
+{
+	ctx->ops->runcntl_stop(ctx);
+	return -ENOSYS;
+}
+
 const struct spu_management_ops spu_management_ps3_ops = {
 	.enumerate_spus = ps3_enumerate_spus,
 	.create_spu = ps3_create_spu,
 	.destroy_spu = ps3_destroy_spu,
+	.enable_spu = ps3_enable_spu,
+	.disable_spu = ps3_disable_spu,
 	.init_affinity = ps3_init_affinity,
 };
include/asm-powerpc/spu_priv1.h +15 -0
@@ -24,6 +24,7 @@
 #include <linux/types.h>
 
 struct spu;
+struct spu_context;
 
 /* access to priv1 registers */
 

@@ -178,6 +179,8 @@ struct spu_management_ops {
 	int (*enumerate_spus)(int (*fn)(void *data));
 	int (*create_spu)(struct spu *spu, void *data);
 	int (*destroy_spu)(struct spu *spu);
+	int (*enable_spu)(struct spu_context *ctx);
+	int (*disable_spu)(struct spu_context *ctx);
 	int (*init_affinity)(void);
 };
 

@@ -207,6 +210,18 @@ spu_init_affinity (void)
 	return spu_management_ops->init_affinity();
 }
 
+static inline int
+spu_enable_spu (struct spu_context *ctx)
+{
+	return spu_management_ops->enable_spu(ctx);
+}
+
+static inline int
+spu_disable_spu (struct spu_context *ctx)
+{
+	return spu_management_ops->disable_spu(ctx);
+}
+
 /*
  * The declarations folowing are put here for convenience
  * and only intended to be used by the platform setup code.
#4 lore raw
Quoted 2 lines · Geoff Levand

v2:
o Added comments about unmapping PS pages when disabled.

What about my comment about making the new functions return void?

	Arnd <><
author
#5 lore raw
In reply to Arnd Bergmann
Quoted 16 lines · Arnd Bergmann
Quoted 12 lines · Geoff Levand

From: Masato Noguchi <[email protected]>

Add platform specific SPU run control routines.

The current spufs_run_spu() implementation uses the SPU master run control bit (MFC_SR1[S]) to control SPE execution, but the PS3 hypervisor does not support the use of this feature. This change adds run control wrapper routines. The bare metal routines use the master run control bit, and the PS3 specific routines use the priv2 run control register.

Quoted 2 lines
Signed-off-by Masato Noguchi <[email protected]>
Signed-off-by Geoff Levand <[email protected]>
Quoted 8 lines

Hmm, ok. Since I don't think we'll get to do the right solution (unmapping the ps registers on the ps3) that soon, doing this patch is at least better than the current situation where we end up with an oops every time we leave spu_run.

It would probably be good to mention the remaining problem with the current approach in the changelog and as a comment in the ps3 specific functions.

Ok, I did that.

Quoted 11 lines

include/asm-powerpc/spu_priv1.h struct spu_management_ops { · line 185

Quoted 8 lines
include/asm-powerpc/spu_priv1.h +2 -0
@@ -178,6 +179,8 @@ struct spu_management_ops {
 	int (*enumerate_spus)(int (*fn)(void *data));
 	int (*create_spu)(struct spu *spu, void *data);
 	int (*destroy_spu)(struct spu *spu);
+	int (*enable_spu)(struct spu_context *ctx);
+	int (*disable_spu)(struct spu_context *ctx);
 	int (*init_affinity)(void);
 };

Also, I think you should make the return type of the callback 'void' since the result is not used anywhere.

Noguchi-san was hesitant to do this. I also thought lets leave it as is until we consider the unmapping support, as maybe a return value might make sense. Jeremy, if you think it better to return void, please make the update.

-Geoff

#6 lore raw
Quoted 2 lines · Geoff Levand
Quoted 2 lines
Quoted 2 lines
(quoted diff) +0 -0
@@ -178,6 +179,8 @@ struct spu_management_ops {

=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0int (*enumerate_spus)(int (*fn)(void *da=

ta));

Quoted 1 line
Quoted 1 line
Quoted 1 line

=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0int (*create_spu)(struct spu *spu, void =

*data);

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0int (*destroy_spu)(struct spu *spu);

+=C2=A0=C2=A0=C2=A0=C2=A0int (*enable_spu)(struct spu_context *ctx);
+=C2=A0=C2=A0=C2=A0=C2=A0int (*disable_spu)(struct spu_context *ctx);

=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0int (*init_affinity)(void); =C2=A0};

Quoted 7 lines
Quoted 3 lines

=20
Also, I think you should make the return type of the callback 'void' since the result is not used anywhere.

=20
Noguchi-san was hesitant to do this. =C2=A0I also thought lets leave it as is until we consider the unmapping support, as maybe a return value might make sense.=20

Sorry for having to reply twice on this. I first only got the other mail because of a mail server hickup.

In general, I think we should follow the rule of keeping the interfaces as simple as possible. If nobody is using the return value here, we probably won't need it in the future either. And if we do, we can still change it.

	Arnd <><
author
#7 lore raw

PATCH v3 1/1 Cell: Wrap master run control bit

In reply to Geoff Levand

Subject: Cell: Wrap master run control bit

From: Masato Noguchi <[email protected]>

Add platform specific SPU run control routines to the spufs. The current spufs implementation uses the SPU master run control bit (MFC_SR1[S]) to control SPE execution, but the PS3 hypervisor does not support the use of this feature.

This change adds the run control wrapper routies spu_enable_spu() and spu_disable_spu(). The bare metal routines use the master run control bit, and the PS3 specific routines use the priv2 run control register.

An outstanding enhancement for the PS3 would be to add a guard to check for incorrect access to the spu problem state when the spu context is disabled. This check could be implemented with a flag added to the spu context that would inhibit mapping problem state pages, and a routine to unmap spu problem state pages. When the spu is enabled with ps3_enable_spu() the flag would be set allowing pages to be mapped, and when the spu is disabled with ps3_disable_spu() the flag would be cleared and mapped problem state pages would be unmapped.

Signed-off-by Masato Noguchi <[email protected]>
Signed-off-by Geoff Levand <[email protected]>

---

Jeremy,

Here is an updated version for 2.6.24. Noguchi-san will work on the LS unmapping feature for 2.6.25.

-Geoff

v2:
o Added comments about unmapping PS pages when disabled.
v3:
o Changed routines to return void instead of int.
o Rebased to apply to Jeremy's 2.6.23-rc work-around.

 arch/powerpc/platforms/cell/spu_manage.c        |   13 +++++++++++
 arch/powerpc/platforms/cell/spufs/backing_ops.c |    6 +++++
 arch/powerpc/platforms/cell/spufs/hw_ops.c      |   10 ++++++++
 arch/powerpc/platforms/cell/spufs/run.c         |    4 +--
 arch/powerpc/platforms/cell/spufs/spufs.h       |    1 
 arch/powerpc/platforms/ps3/spu.c                |   27 ++++++++++++++++++++++--
 include/asm-powerpc/spu_priv1.h                 |   15 +++++++++++++
 7 files changed, 72 insertions(+), 4 deletions(-)
arch/powerpc/platforms/cell/spu_manage.c +13 -0
@@ -35,6 +35,7 @@
 #include <asm/firmware.h>
 #include <asm/prom.h>
 
+#include "spufs/spufs.h"
 #include "interrupt.h"
 
 struct device_node *spu_devnode(struct spu *spu)

@@ -369,6 +370,16 @@ static int of_destroy_spu(struct spu *sp
 	return 0;
 }
 
+static void enable_spu_by_master_run(struct spu_context *ctx)
+{
+	ctx->ops->master_start(ctx);
+}
+
+static void disable_spu_by_master_run(struct spu_context *ctx)
+{
+	ctx->ops->master_stop(ctx);
+}
+
 /* Hardcoded affinity idxs for qs20 */
 #define QS20_SPES_PER_BE 8
 static int qs20_reg_idxs[QS20_SPES_PER_BE] =   { 0, 2, 4, 6, 7, 5, 3, 1 };

@@ -535,5 +546,7 @@ const struct spu_management_ops spu_mana
 	.enumerate_spus = of_enumerate_spus,
 	.create_spu = of_create_spu,
 	.destroy_spu = of_destroy_spu,
+	.enable_spu = enable_spu_by_master_run,
+	.disable_spu = disable_spu_by_master_run,
 	.init_affinity = init_affinity,
 };
arch/powerpc/platforms/cell/spufs/backing_ops.c +6 -0
@@ -285,6 +285,11 @@ static void spu_backing_runcntl_write(st
 	spin_unlock(&ctx->csa.register_lock);
 }
 
+static void spu_backing_runcntl_stop(struct spu_context *ctx)
+{
+	spu_backing_runcntl_write(ctx, SPU_RUNCNTL_STOP);
+}
+
 static void spu_backing_master_start(struct spu_context *ctx)
 {
 	struct spu_state *csa = &ctx->csa;

@@ -381,6 +386,7 @@ struct spu_context_ops spu_backing_ops =
 	.get_ls = spu_backing_get_ls,
 	.runcntl_read = spu_backing_runcntl_read,
 	.runcntl_write = spu_backing_runcntl_write,
+	.runcntl_stop = spu_backing_runcntl_stop,
 	.master_start = spu_backing_master_start,
 	.master_stop = spu_backing_master_stop,
 	.set_mfc_query = spu_backing_set_mfc_query,
arch/powerpc/platforms/cell/spufs/hw_ops.c +10 -0
@@ -220,6 +220,15 @@ static void spu_hw_runcntl_write(struct 
 	spin_unlock_irq(&ctx->spu->register_lock);
 }
 
+static void spu_hw_runcntl_stop(struct spu_context *ctx)
+{
+	spin_lock_irq(&ctx->spu->register_lock);
+	out_be32(&ctx->spu->problem->spu_runcntl_RW, SPU_RUNCNTL_STOP);
+	while (in_be32(&ctx->spu->problem->spu_status_R) & SPU_STATUS_RUNNING)
+		cpu_relax();
+	spin_unlock_irq(&ctx->spu->register_lock);
+}
+
 static void spu_hw_master_start(struct spu_context *ctx)
 {
 	struct spu *spu = ctx->spu;

@@ -321,6 +330,7 @@ struct spu_context_ops spu_hw_ops = {
 	.get_ls = spu_hw_get_ls,
 	.runcntl_read = spu_hw_runcntl_read,
 	.runcntl_write = spu_hw_runcntl_write,
+	.runcntl_stop = spu_hw_runcntl_stop,
 	.master_start = spu_hw_master_start,
 	.master_stop = spu_hw_master_stop,
 	.set_mfc_query = spu_hw_set_mfc_query,
arch/powerpc/platforms/cell/spufs/run.c +2 -2
@@ -302,7 +302,7 @@ long spufs_run_spu(struct spu_context *c
 	if (mutex_lock_interruptible(&ctx->run_mutex))
 		return -ERESTARTSYS;
 
-	ctx->ops->master_start(ctx);
+	spu_enable_spu(ctx);
 	ctx->event_return = 0;
 
 	spu_acquire(ctx);

@@ -376,7 +376,7 @@ long spufs_run_spu(struct spu_context *c
 		ctx->stats.libassist++;
 
 
-	ctx->ops->master_stop(ctx);
+	spu_disable_spu(ctx);
 	ret = spu_run_fini(ctx, npc, &status);
 	spu_yield(ctx);
arch/powerpc/platforms/cell/spufs/spufs.h +1 -0
@@ -170,6 +170,7 @@ struct spu_context_ops {
 	char*(*get_ls) (struct spu_context * ctx);
 	 u32 (*runcntl_read) (struct spu_context * ctx);
 	void (*runcntl_write) (struct spu_context * ctx, u32 data);
+	void (*runcntl_stop) (struct spu_context * ctx);
 	void (*master_start) (struct spu_context * ctx);
 	void (*master_stop) (struct spu_context * ctx);
 	int (*set_mfc_query)(struct spu_context * ctx, u32 mask, u32 mode);
arch/powerpc/platforms/ps3/spu.c +25 -2
@@ -28,6 +28,7 @@
 #include <asm/spu_priv1.h>
 #include <asm/lv1call.h>
 
+#include "../cell/spufs/spufs.h"
 #include "platform.h"
 
 /* spu_management_ops */

@@ -419,10 +420,34 @@ static int ps3_init_affinity(void)
 	return 0;
 }
 
+/**
+ * ps3_enable_spu - Enable SPU run control.
+ *
+ * An outstanding enhancement for the PS3 would be to add a guard to check
+ * for incorrect access to the spu problem state when the spu context is
+ * disabled.  This check could be implemented with a flag added to the spu
+ * context that would inhibit mapping problem state pages, and a routine
+ * to unmap spu problem state pages.  When the spu is enabled with
+ * ps3_enable_spu() the flag would be set allowing pages to be mapped,
+ * and when the spu is disabled with ps3_disable_spu() the flag would be
+ * cleared and the mapped problem state pages would be unmapped.
+ */
+
+static void ps3_enable_spu(struct spu_context *ctx)
+{
+}
+
+static void ps3_disable_spu(struct spu_context *ctx)
+{
+	ctx->ops->runcntl_stop(ctx);
+}
+
 const struct spu_management_ops spu_management_ps3_ops = {
 	.enumerate_spus = ps3_enumerate_spus,
 	.create_spu = ps3_create_spu,
 	.destroy_spu = ps3_destroy_spu,
+	.enable_spu = ps3_enable_spu,
+	.disable_spu = ps3_disable_spu,
 	.init_affinity = ps3_init_affinity,
 };
 

@@ -505,8 +530,6 @@ static void mfc_sr1_set(struct spu *spu,
 	static const u64 allowed = ~(MFC_STATE1_LOCAL_STORAGE_DECODE_MASK
 		| MFC_STATE1_PROBLEM_STATE_MASK);
 
-	sr1 |= MFC_STATE1_MASTER_RUN_CONTROL_MASK;
-
 	BUG_ON((sr1 & allowed) != (spu_pdata(spu)->cache.sr1 & allowed));
 
 	spu_pdata(spu)->cache.sr1 = sr1;
include/asm-powerpc/spu_priv1.h +15 -0
@@ -24,6 +24,7 @@
 #include <linux/types.h>
 
 struct spu;
+struct spu_context;
 
 /* access to priv1 registers */
 

@@ -178,6 +179,8 @@ struct spu_management_ops {
 	int (*enumerate_spus)(int (*fn)(void *data));
 	int (*create_spu)(struct spu *spu, void *data);
 	int (*destroy_spu)(struct spu *spu);
+	void (*enable_spu)(struct spu_context *ctx);
+	void (*disable_spu)(struct spu_context *ctx);
 	int (*init_affinity)(void);
 };
 

@@ -207,6 +210,18 @@ spu_init_affinity (void)
 	return spu_management_ops->init_affinity();
 }
 
+static inline void
+spu_enable_spu (struct spu_context *ctx)
+{
+	spu_management_ops->enable_spu(ctx);
+}
+
+static inline void
+spu_disable_spu (struct spu_context *ctx)
+{
+	spu_management_ops->disable_spu(ctx);
+}
+
 /*
  * The declarations folowing are put here for convenience
  * and only intended to be used by the platform setup code.
#8 lore raw
Quoted 22 lines · Geoff Levand

Subject: Cell: Wrap master run control bit

From: Masato Noguchi <[email protected]>

Add platform specific SPU run control routines to the spufs. The current spufs implementation uses the SPU master run control bit (MFC_SR1[S]) to control SPE execution, but the PS3 hypervisor does not support the use of this feature.

This change adds the run control wrapper routies spu_enable_spu() and spu_disable_spu(). The bare metal routines use the master run control bit, and the PS3 specific routines use the priv2 run control register.

An outstanding enhancement for the PS3 would be to add a guard to check for incorrect access to the spu problem state when the spu context is disabled. This check could be implemented with a flag added to the spu context that would inhibit mapping problem state pages, and a routine to unmap spu problem state pages. When the spu is enabled with ps3_enable_spu() the flag would be set allowing pages to be mapped, and when the spu is disabled with ps3_disable_spu() the flag would be cleared and mapped problem state pages would be unmapped.

Quoted 7 lines
Signed-off-by Masato Noguchi <[email protected]>
Signed-off-by Geoff Levand <[email protected]>

---

Jeremy,

Here is an updated version for 2.6.24. Noguchi-san will

As 2.6.24-rc1 is out and Geoff is offline: what's the status of this patch?

Quoted 225 lines

work on the LS unmapping feature for 2.6.25.

-Geoff

v2:
o Added comments about unmapping PS pages when disabled.
v3:
o Changed routines to return void instead of int.
o Rebased to apply to Jeremy's 2.6.23-rc work-around.

 arch/powerpc/platforms/cell/spu_manage.c        |   13 +++++++++++
 arch/powerpc/platforms/cell/spufs/backing_ops.c |    6 +++++
 arch/powerpc/platforms/cell/spufs/hw_ops.c      |   10 ++++++++
 arch/powerpc/platforms/cell/spufs/run.c         |    4 +--
 arch/powerpc/platforms/cell/spufs/spufs.h       |    1 
 arch/powerpc/platforms/ps3/spu.c                |   27 ++++++++++++++++++++++--
 include/asm-powerpc/spu_priv1.h                 |   15 +++++++++++++
 7 files changed, 72 insertions(+), 4 deletions(-)
arch/powerpc/platforms/cell/spu_manage.c +13 -0
@@ -35,6 +35,7 @@
 #include <asm/firmware.h>
 #include <asm/prom.h>
 
+#include "spufs/spufs.h"
 #include "interrupt.h"
 
 struct device_node *spu_devnode(struct spu *spu)

@@ -369,6 +370,16 @@ static int of_destroy_spu(struct spu *sp
 	return 0;
 }
 
+static void enable_spu_by_master_run(struct spu_context *ctx)
+{
+	ctx->ops->master_start(ctx);
+}
+
+static void disable_spu_by_master_run(struct spu_context *ctx)
+{
+	ctx->ops->master_stop(ctx);
+}
+
 /* Hardcoded affinity idxs for qs20 */
 #define QS20_SPES_PER_BE 8
 static int qs20_reg_idxs[QS20_SPES_PER_BE] =   { 0, 2, 4, 6, 7, 5, 3, 1 };

@@ -535,5 +546,7 @@ const struct spu_management_ops spu_mana
 	.enumerate_spus = of_enumerate_spus,
 	.create_spu = of_create_spu,
 	.destroy_spu = of_destroy_spu,
+	.enable_spu = enable_spu_by_master_run,
+	.disable_spu = disable_spu_by_master_run,
 	.init_affinity = init_affinity,
 };
arch/powerpc/platforms/cell/spufs/backing_ops.c +6 -0
@@ -285,6 +285,11 @@ static void spu_backing_runcntl_write(st
 	spin_unlock(&ctx->csa.register_lock);
 }
 
+static void spu_backing_runcntl_stop(struct spu_context *ctx)
+{
+	spu_backing_runcntl_write(ctx, SPU_RUNCNTL_STOP);
+}
+
 static void spu_backing_master_start(struct spu_context *ctx)
 {
 	struct spu_state *csa = &ctx->csa;

@@ -381,6 +386,7 @@ struct spu_context_ops spu_backing_ops =
 	.get_ls = spu_backing_get_ls,
 	.runcntl_read = spu_backing_runcntl_read,
 	.runcntl_write = spu_backing_runcntl_write,
+	.runcntl_stop = spu_backing_runcntl_stop,
 	.master_start = spu_backing_master_start,
 	.master_stop = spu_backing_master_stop,
 	.set_mfc_query = spu_backing_set_mfc_query,
arch/powerpc/platforms/cell/spufs/hw_ops.c +10 -0
@@ -220,6 +220,15 @@ static void spu_hw_runcntl_write(struct 
 	spin_unlock_irq(&ctx->spu->register_lock);
 }
 
+static void spu_hw_runcntl_stop(struct spu_context *ctx)
+{
+	spin_lock_irq(&ctx->spu->register_lock);
+	out_be32(&ctx->spu->problem->spu_runcntl_RW, SPU_RUNCNTL_STOP);
+	while (in_be32(&ctx->spu->problem->spu_status_R) & SPU_STATUS_RUNNING)
+		cpu_relax();
+	spin_unlock_irq(&ctx->spu->register_lock);
+}
+
 static void spu_hw_master_start(struct spu_context *ctx)
 {
 	struct spu *spu = ctx->spu;

@@ -321,6 +330,7 @@ struct spu_context_ops spu_hw_ops = {
 	.get_ls = spu_hw_get_ls,
 	.runcntl_read = spu_hw_runcntl_read,
 	.runcntl_write = spu_hw_runcntl_write,
+	.runcntl_stop = spu_hw_runcntl_stop,
 	.master_start = spu_hw_master_start,
 	.master_stop = spu_hw_master_stop,
 	.set_mfc_query = spu_hw_set_mfc_query,
arch/powerpc/platforms/cell/spufs/run.c +2 -2
@@ -302,7 +302,7 @@ long spufs_run_spu(struct spu_context *c
 	if (mutex_lock_interruptible(&ctx->run_mutex))
 		return -ERESTARTSYS;
 
-	ctx->ops->master_start(ctx);
+	spu_enable_spu(ctx);
 	ctx->event_return = 0;
 
 	spu_acquire(ctx);

@@ -376,7 +376,7 @@ long spufs_run_spu(struct spu_context *c
 		ctx->stats.libassist++;
 
 
-	ctx->ops->master_stop(ctx);
+	spu_disable_spu(ctx);
 	ret = spu_run_fini(ctx, npc, &status);
 	spu_yield(ctx);
arch/powerpc/platforms/cell/spufs/spufs.h +1 -0
@@ -170,6 +170,7 @@ struct spu_context_ops {
 	char*(*get_ls) (struct spu_context * ctx);
 	 u32 (*runcntl_read) (struct spu_context * ctx);
 	void (*runcntl_write) (struct spu_context * ctx, u32 data);
+	void (*runcntl_stop) (struct spu_context * ctx);
 	void (*master_start) (struct spu_context * ctx);
 	void (*master_stop) (struct spu_context * ctx);
 	int (*set_mfc_query)(struct spu_context * ctx, u32 mask, u32 mode);
arch/powerpc/platforms/ps3/spu.c +25 -2
@@ -28,6 +28,7 @@
 #include <asm/spu_priv1.h>
 #include <asm/lv1call.h>
 
+#include "../cell/spufs/spufs.h"
 #include "platform.h"
 
 /* spu_management_ops */

@@ -419,10 +420,34 @@ static int ps3_init_affinity(void)
 	return 0;
 }
 
+/**
+ * ps3_enable_spu - Enable SPU run control.
+ *
+ * An outstanding enhancement for the PS3 would be to add a guard to check
+ * for incorrect access to the spu problem state when the spu context is
+ * disabled.  This check could be implemented with a flag added to the spu
+ * context that would inhibit mapping problem state pages, and a routine
+ * to unmap spu problem state pages.  When the spu is enabled with
+ * ps3_enable_spu() the flag would be set allowing pages to be mapped,
+ * and when the spu is disabled with ps3_disable_spu() the flag would be
+ * cleared and the mapped problem state pages would be unmapped.
+ */
+
+static void ps3_enable_spu(struct spu_context *ctx)
+{
+}
+
+static void ps3_disable_spu(struct spu_context *ctx)
+{
+	ctx->ops->runcntl_stop(ctx);
+}
+
 const struct spu_management_ops spu_management_ps3_ops = {
 	.enumerate_spus = ps3_enumerate_spus,
 	.create_spu = ps3_create_spu,
 	.destroy_spu = ps3_destroy_spu,
+	.enable_spu = ps3_enable_spu,
+	.disable_spu = ps3_disable_spu,
 	.init_affinity = ps3_init_affinity,
 };
 

@@ -505,8 +530,6 @@ static void mfc_sr1_set(struct spu *spu,
 	static const u64 allowed = ~(MFC_STATE1_LOCAL_STORAGE_DECODE_MASK
 		| MFC_STATE1_PROBLEM_STATE_MASK);
 
-	sr1 |= MFC_STATE1_MASTER_RUN_CONTROL_MASK;
-
 	BUG_ON((sr1 & allowed) != (spu_pdata(spu)->cache.sr1 & allowed));
 
 	spu_pdata(spu)->cache.sr1 = sr1;
include/asm-powerpc/spu_priv1.h +15 -0
@@ -24,6 +24,7 @@
 #include <linux/types.h>
 
 struct spu;
+struct spu_context;
 
 /* access to priv1 registers */
 

@@ -178,6 +179,8 @@ struct spu_management_ops {
 	int (*enumerate_spus)(int (*fn)(void *data));
 	int (*create_spu)(struct spu *spu, void *data);
 	int (*destroy_spu)(struct spu *spu);
+	void (*enable_spu)(struct spu_context *ctx);
+	void (*disable_spu)(struct spu_context *ctx);
 	int (*init_affinity)(void);
 };
 

@@ -207,6 +210,18 @@ spu_init_affinity (void)
 	return spu_management_ops->init_affinity();
 }
 
+static inline void
+spu_enable_spu (struct spu_context *ctx)
+{
+	spu_management_ops->enable_spu(ctx);
+}
+
+static inline void
+spu_disable_spu (struct spu_context *ctx)
+{
+	spu_management_ops->disable_spu(ctx);
+}
+
 /*
  * The declarations folowing are put here for convenience
  * and only intended to be used by the platform setup code.
Quoted 5 lines

_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-dev

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   [email protected]	
Internet: http://www.sony-europe.com/

Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V. Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619