[Cbe-oss-dev] [RFC/PATCH]: User-space access to Cell bookmark registers

[Kevin Corry]

On Wed August 8 2007 4:36 pm, Arnd Bergmann wrote:
> On Wednesday 08 August 2007, Kevin Corry wrote:
> > Actually, smp_call_function_single() and smp_call_function_map() only
> > seem to work if you're trying to run the routine on a different CPU than
> > the one you're currently executing on. So we'd have to add checks to see
> > which CPU we're running on and either call smp_call_function_single() or
> > call the desired routine directly.
>
> No, smp_call_function_single() already contains the code to work around
> the limitation of smp_call_function_map().

Well, smp_call_function_single() will only call smp_call_function_map() if you
are not targetting the current CPU, but it doesn't directly run the desired
routine if you are targetting the current CPU. So we'd need to do something like:

	rc = smp_call_function_single(cpu, set_bookmark_val, &val, 0, 1);
	if (rc == -EBUSY)
		set_bookmark_val(&val);

Would that be preferrable to the set_cpus_allowed() method?

> > I don't know if this register is available on the ps3 or celleb
> > platforms, so I'll use the platform_is(cell) approach for now (actually,
> > it looks like the macro is called "machine_is()"). Do you know if there's
> > documentation available for the ps3 and/or celleb that would have that
> > information?
>
> I'd say you have to try it.

Unfortunately, I don't have access to any ps3 or celleb systems, so I'd have
to get someone else to volunteer to test it. Any takers?

> Of course, since the pmu code checks for "machine_is(cell)" already, it
> makes sense to have an identical check here as well.
>
> Maybe it should even be called from the same initcall. You'll have to
> convert it from an arch_initcall() to a device_initcall in order
> to create the sysfs files, but I think that's the right approach
> anyway.

Sounds good. I've removed the cbe_bookmark_init() routine, renamed
cbe_init_pm_irq() to cbe_init_pmu(), changed it from arch_initcall() to
__initcall(), and added the bookmark initialization to that routine.

> > +static SYSDEV_ATTR(pmu_bookmark, 0200, NULL, cbe_store_pmu_bookmark);
>
> One more thing here: For consistency, we might want to cache the last
> value that was written to the file and make it readable, if that makes
> any sense for the data you write to it.
>
> Is the value per core or per SMT thread? If the former, that would cause
> extra effort to keep the cached value consistent over both threads, which
> is probably not worth it.

The documentation says the register is duplicated for SMT mode, so it should
be safe to use a simple per-cpu variable to hold the last-written value. I've
added a cbe_show_pmu_bookmark() routine to return that value.

Here's take 3.
-- 
Kevin Corry
kevcorry at us.ibm.com
http://www.ibm.com/linux/


[CELL] User-space access to bookmark registers.

Add the files /sys/devices/system/cpu/cpu*/pmu_bookmark to provide user-space
with a method for writing to the bookmark registers in the Cell processor.
Writes to these registers can be used by the performance monitoring unit to
generate trace data and to trigger the starting and stopping of the hardware
performance counters.

The renames the existing cbe_init_pm_irq() routine to cbe_init_pmu(), and adds
the initialization of these new sysfs files to that init routine.

Signed-off-by: Kevin Corry <kevcorry at us.ibm.com>

Index: linux-2.6.22-5.20070724bsc/arch/powerpc/platforms/cell/pmu.c
===================================================================
--- linux-2.6.22-5.20070724bsc.orig/arch/powerpc/platforms/cell/pmu.c
+++ linux-2.6.22-5.20070724bsc/arch/powerpc/platforms/cell/pmu.c
@@ -22,6 +22,7 @@
  * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
+#include <linux/cpu.h>
 #include <linux/interrupt.h>
 #include <linux/types.h>
 #include <asm/io.h>
@@ -376,7 +377,57 @@ static irqreturn_t cbe_pm_irq(int irq, v
 	return IRQ_HANDLED;
 }
 
-static int __init cbe_init_pm_irq(void)
+/*
+ * Provide files in sysfs to allow user-space to access the bookmark registers.
+ * The sysfs files are /sys/devices/system/cpu/cpu?/pmu_bookmark. The registers
+ * are write only, so we'll provide "shadow" values so user-space can read the
+ * values as well.
+ */
+
+static DEFINE_PER_CPU(u64, pmu_bookmark);
+
+static ssize_t cbe_show_pmu_bookmark(struct sys_device *dev, char *buf)
+{
+	cpumask_t old_affinity = current->cpus_allowed;
+	int cpu = dev->id;
+	u64 val;
+
+	if (set_cpus_allowed(current, cpumask_of_cpu(cpu)))
+		return -EINVAL;
+
+	val = __get_cpu_var(pmu_bookmark);
+
+	set_cpus_allowed(current, old_affinity);
+
+	return sprintf(buf, "%lu\n", val);
+}
+
+static ssize_t cbe_store_pmu_bookmark(struct sys_device *dev,
+				      const char *buf, size_t count)
+{
+	cpumask_t old_affinity = current->cpus_allowed;
+	int rc, cpu = dev->id;
+	u64 val;
+
+	rc = sscanf(buf, "%lu", &val);
+	if (rc != 1)
+		return -EINVAL;
+
+	if (set_cpus_allowed(current, cpumask_of_cpu(cpu)))
+		return -EINVAL;
+
+	__get_cpu_var(pmu_bookmark) = val;
+	mtspr(SPRN_BKMK, val);
+
+	set_cpus_allowed(current, old_affinity);
+
+	return count;
+}
+
+static SYSDEV_ATTR(pmu_bookmark, 0600,
+		   cbe_show_pmu_bookmark, cbe_store_pmu_bookmark);
+
+static int __init cbe_init_pmu(void)
 {
 	unsigned int irq;
 	int rc, node;
@@ -402,9 +453,9 @@ static int __init cbe_init_pm_irq(void)
 		}
 	}
 
-	return 0;
+	return cpu_add_sysdev_attr(&attr_pmu_bookmark);
 }
-arch_initcall(cbe_init_pm_irq);
+__initcall(cbe_init_pmu);
 
 void cbe_sync_irq(int node)
 {
Index: linux-2.6.22-5.20070724bsc/include/asm-powerpc/reg.h
===================================================================
--- linux-2.6.22-5.20070724bsc.orig/include/asm-powerpc/reg.h
+++ linux-2.6.22-5.20070724bsc/include/asm-powerpc/reg.h
@@ -427,6 +427,8 @@
 #define SPRN_SCOMC	0x114	/* SCOM Access Control */
 #define SPRN_SCOMD	0x115	/* SCOM Access DATA */
 
+#define SPRN_BKMK	1020	/* Cell Bookmark Register */
+
 /* Performance monitor SPRs */
 #ifdef CONFIG_PPC64
 #define SPRN_MMCR0	795