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

[Kevin Corry]

On Wed August 8 2007 2:13 pm, Arnd Bergmann wrote:
> On Wednesday 08 August 2007, Kevin Corry wrote:
> > The patch adds the new code to the arch/powerpc/platforms/cell/pmu.c
> > file. However, the registers aren't actually part of the PMU.
> > Is pmu.c appropriate for this code or should it go in some other file?
>
> A new file specifically for this might be better, but I can see arguments
> either way, I don't care much. Is there any possibility that this might
> be useful for someone who is not using the pmu.c code?

In the documentation I have available, it says the bookmark register "is for 
hardware-debugging purposes only". The only other places I see it mentioned 
are in the PMU documentation. So I'd guess that it's main use is with the 
PMU, but there may be other uses that don't show up in the docs I have 
available.

> > Add the files /sys/devices/system/cpu/cpu*/bookmark to provide user-space
> > with a method for writing to the bookmark registers in the Cell
> > processor.
>
> The name 'bookmark' might be a little too generic, and not really obvious.
> Maybe something like 'pmu-bookmark' would be more appropriate.

Ok.

> > +#define BOOKMARK_SPR_ADDR 1020
>
> The definition should go into the include/asm-powerpc/reg.h file,
> along with the other SPRN_* macros.

Sure.

> > +static ssize_t cbe_store_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;
> > +
> > +       mtspr(BOOKMARK_SPR_ADDR, val);
> > +
> > +       set_cpus_allowed(current, old_affinity);
> > +
> > +       return count;
> > +}
>
> Looks good. I'd have used the smp_call_function_single() function
> in order to go to the right CPU, but I don't see an important argument
> to change it.

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.

> > +static int __init cbe_bookmark_init(void)
> > +{
> > +       return cpu_add_sysdev_attr(&attr_bookmark);
> > +}
> > +__initcall(cbe_bookmark_init);
>
> This needs to check if it's running on a compatible machine, otherwise
> you might give access to a register that should not be writable.
> The check should probably be either (cur_cpu_spec->cpu_user_features &
> PPC_FEATURE_CELL) if you want it to be available on the ps3 and celleb
> platforms, or 'platform_is(cell)' if it's specific to only the bare
> metal machines.

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've included a new patch below. Thanks for the speedy review.
-- 
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.

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>
@@ -424,3 +425,39 @@ void cbe_sync_irq(int node)
 }
 EXPORT_SYMBOL_GPL(cbe_sync_irq);
 
+/*
+ * Provide files in sysfs to allow user-space to write to the bookmark
+ * registers. The sysfs files are /sys/devices/system/cpu/cpu?/pmu_bookmark.
+ */
+
+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;
+
+	mtspr(SPRN_BKMK, val);
+
+	set_cpus_allowed(current, old_affinity);
+
+	return count;
+}
+
+static SYSDEV_ATTR(pmu_bookmark, 0200, NULL, cbe_store_pmu_bookmark);
+
+static int __init cbe_bookmark_init(void)
+{
+	if (!machine_is(cell))
+		return 0;
+
+	return cpu_add_sysdev_attr(&attr_pmu_bookmark);
+}
+__initcall(cbe_bookmark_init);
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