[patch 1/3] PS3: Add logical performance monitor repository routines

[patch 1/3] PS3: Add logical performance monitor repository routines

[Geoff Levand]

From: Takashi Yamamoto <Takashi02_Yamamoto@hq.scei.sony.co.jp>

Add repository routines for the PS3 Logical Performance Monitor (lpm).

Signed-off-by: Takashi Yamamoto <Takashi02_Yamamoto@hq.scei.sony.co.jp>
Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
---
 arch/powerpc/platforms/ps3/platform.h   |    7 ++++
 arch/powerpc/platforms/ps3/repository.c |   48 ++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)

--- a/arch/powerpc/platforms/ps3/platform.h
+++ b/arch/powerpc/platforms/ps3/platform.h
@@ -189,7 +189,7 @@ int ps3_repository_read_stor_dev_region(
 /* repository pu and memory info */
 
 int ps3_repository_read_num_pu(unsigned int *num_pu);
-int ps3_repository_read_ppe_id(unsigned int *pu_index, unsigned int *ppe_id);
+int ps3_repository_read_pu_id(unsigned int pu_index, u64 *pu_id);
 int ps3_repository_read_rm_base(unsigned int ppe_id, u64 *rm_base);
 int ps3_repository_read_rm_size(unsigned int ppe_id, u64 *rm_size);
 int ps3_repository_read_region_total(u64 *region_total);
@@ -203,6 +203,11 @@ int ps3_repository_read_be_node_id(unsig
 int ps3_repository_read_tb_freq(u64 node_id, u64 *tb_freq);
 int ps3_repository_read_be_tb_freq(unsigned int be_index, u64 *tb_freq);
 
+/* repository performance monitor info */
+
+int ps3_repository_read_lpm_privileges(unsigned int be_index, u64 *lpar,
+	u64 *rights);
+
 /* repository 'Other OS' area */
 
 int ps3_repository_read_boot_dat_addr(u64 *lpar_addr);
--- a/arch/powerpc/platforms/ps3/repository.c
+++ b/arch/powerpc/platforms/ps3/repository.c
@@ -902,6 +902,54 @@ int ps3_repository_read_be_tb_freq(unsig
 		: ps3_repository_read_tb_freq(node_id, tb_freq);
 }
 
+int ps3_repository_read_lpm_privileges(unsigned int be_index, u64 *lpar,
+	u64 *rights)
+{
+	int result;
+	u64 node_id;
+
+	*lpar = 0;
+	*rights = 0;
+	result = ps3_repository_read_be_node_id(be_index, &node_id);
+	return result ? result
+		: read_node(PS3_LPAR_ID_PME,
+			    make_first_field("be", 0),
+			    node_id,
+			    make_field("lpm", 0),
+			    make_field("priv", 0),
+			    lpar, rights);
+}
+
+int ps3_repository_read_num_pu(unsigned int *num_pu)
+{
+	int result;
+	u64 v1;
+
+	v1 = 0;
+	result = read_node(PS3_LPAR_ID_CURRENT,
+			   make_first_field("bi", 0),
+			   make_field("pun", 0),
+			   0, 0,
+			   &v1, NULL);
+	*num_pu = v1;
+	return result;
+}
+
+int ps3_repository_read_pu_id(unsigned int pu_index, u64 *pu_id)
+{
+	int result;
+	u64 v1;
+
+	v1 = 0;
+	result = read_node(PS3_LPAR_ID_CURRENT,
+		make_first_field("bi", 0),
+		make_field("pu", pu_index),
+		0, 0,
+		&v1, NULL);
+	*pu_id = v1;
+	return result;
+}
+
 #if defined(DEBUG)
 
 int ps3_repository_dump_resource_info(const struct ps3_repository_device *repo)

-- 

[patch 2/3] PS3: Add logical performance monitor device support

[Geoff Levand]

Add PS3 logical performance monitor (lpm) device support to the
PS3 system-bus and platform device registration routines.

Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
---
 arch/powerpc/platforms/ps3/device-init.c |   95 ++++++++++++++++++++++++++++++-
 arch/powerpc/platforms/ps3/system-bus.c  |    5 +
 include/asm-powerpc/ps3.h                |    7 ++
 3 files changed, 106 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/platforms/ps3/lpm.c

--- a/arch/powerpc/platforms/ps3/device-init.c
+++ b/arch/powerpc/platforms/ps3/device-init.c
@@ -30,6 +30,97 @@
 
 #include "platform.h"
 
+static int __init ps3_register_lpm_devices(void)
+{
+	int result;
+	unsigned int pu_count;
+	u64 tmp1;
+	u64 tmp2;
+	struct layout {
+		struct ps3_system_bus_device dev;
+	} *p;
+
+	pr_debug(" -> %s:%d\n", __func__, __LINE__);
+
+	p = kzalloc(sizeof(*p), GFP_KERNEL);
+	if (!p)
+		return -ENOMEM;
+
+	p->dev.match_id = PS3_MATCH_ID_LPM;
+	p->dev.dev_type = PS3_DEVICE_TYPE_LPM;
+
+	result = ps3_repository_read_num_pu(&pu_count);
+
+	if (result) {
+		pr_debug("%s:%d: ps3_repository_read_num_pu failed \n",
+			__func__, __LINE__);
+		goto fail_read_repo;
+	}
+
+	/* The current lpm driver only supports a single BE processor. */
+
+	if (pu_count > 1) {
+		pr_info("%s:%d: found %u BE processors, only one supported\n",
+			__func__, __LINE__, pu_count);
+	}
+
+	result = ps3_repository_read_pu_id(0, &p->dev.lpm.pu_id);
+
+	if (result) {
+		pr_debug("%s:%d: ps3_repository_read_pu_id failed \n",
+			__func__, __LINE__);
+		goto fail_read_repo;
+	}
+
+	result = ps3_repository_read_lpm_privileges(0, &tmp1,
+		&p->dev.lpm.rights);
+
+	if (result) {
+		pr_debug("%s:%d: ps3_repository_read_lpm_privleges failed \n",
+			__func__, __LINE__);
+		goto fail_read_repo;
+	}
+
+	lv1_get_logical_partition_id(&tmp2);
+
+	if (tmp1 != tmp2) {
+		pr_debug("%s:%d: wrong lpar\n",
+			__func__, __LINE__);
+		result = -1;
+		goto fail_rights;
+	}
+
+	if (!(p->dev.lpm.rights & PS3_LPM_RIGHTS_USE_LPM)) {
+		pr_debug("%s:%d: don't have rights to use lpm\n",
+			__func__, __LINE__);
+		result = -1;
+		goto fail_rights;
+	}
+
+	pr_debug("%s:%d: pu_id %lu, rights %lu(%lxh)\n",
+		__func__, __LINE__, p->dev.lpm.pu_id, p->dev.lpm.rights,
+		p->dev.lpm.rights);
+
+	result = ps3_system_bus_device_register(&p->dev);
+
+	if (result) {
+		pr_debug("%s:%d ps3_system_bus_device_register failed\n",
+			__func__, __LINE__);
+		goto fail_register;
+	}
+
+	pr_debug(" <- %s:%d\n", __func__, __LINE__);
+	return 0;
+
+
+fail_register:
+fail_rights:
+fail_read_repo:
+	kfree(p);
+	pr_debug(" <- %s:%d: failed\n", __func__, __LINE__);
+	return result;
+}
+
 /**
  * ps3_setup_gelic_device - Setup and register a gelic device instance.
  *
@@ -787,6 +878,8 @@ static int __init ps3_register_devices(v
 
 	ps3_register_sound_devices();
 
+	ps3_register_lpm_devices();
+
 	pr_debug(" <- %s:%d\n", __func__, __LINE__);
 	return 0;
 }
--- a/arch/powerpc/platforms/ps3/system-bus.c
+++ b/arch/powerpc/platforms/ps3/system-bus.c
@@ -715,6 +715,7 @@ int ps3_system_bus_device_register(struc
 	static unsigned int dev_ioc0_count;
 	static unsigned int dev_sb_count;
 	static unsigned int dev_vuart_count;
+	static unsigned int dev_lpm_count;
 
 	if (!dev->core.parent)
 		dev->core.parent = &ps3_system_bus;
@@ -737,6 +738,10 @@ int ps3_system_bus_device_register(struc
 		snprintf(dev->core.bus_id, sizeof(dev->core.bus_id),
 			"vuart_%02x", ++dev_vuart_count);
 		break;
+	case PS3_DEVICE_TYPE_LPM:
+		snprintf(dev->core.bus_id, sizeof(dev->core.bus_id),
+			"lpm_%02x", ++dev_lpm_count);
+		break;
 	default:
 		BUG();
 	};
--- a/include/asm-powerpc/ps3.h
+++ b/include/asm-powerpc/ps3.h
@@ -317,6 +317,7 @@ enum ps3_match_id {
 	PS3_MATCH_ID_STOR_FLASH     = 8,
 	PS3_MATCH_ID_SOUND          = 9,
 	PS3_MATCH_ID_GRAPHICS       = 10,
+	PS3_MATCH_ID_LPM            = 11,
 };
 
 #define PS3_MODULE_ALIAS_EHCI           "ps3:1"
@@ -329,11 +330,13 @@ enum ps3_match_id {
 #define PS3_MODULE_ALIAS_STOR_FLASH     "ps3:8"
 #define PS3_MODULE_ALIAS_SOUND          "ps3:9"
 #define PS3_MODULE_ALIAS_GRAPHICS       "ps3:10"
+#define PS3_MODULE_ALIAS_LPM            "ps3:11"
 
 enum ps3_system_bus_device_type {
 	PS3_DEVICE_TYPE_IOC0 = 1,
 	PS3_DEVICE_TYPE_SB,
 	PS3_DEVICE_TYPE_VUART,
+	PS3_DEVICE_TYPE_LPM,
 };
 
 /**
@@ -350,6 +353,10 @@ struct ps3_system_bus_device {
 	struct ps3_dma_region *d_region;  /* SB, IOC0 */
 	struct ps3_mmio_region *m_region; /* SB, IOC0*/
 	unsigned int port_number;         /* VUART */
+	struct {                          /* LPM */
+		u64 pu_id;
+		u64 rights;
+	} lpm;
 
 /*	struct iommu_table *iommu_table; -- waiting for BenH's cleanups */
 	struct device core;

-- 

[patch 3/3] PS3: Add logical performance monitor driver support

[Geoff Levand]

From: Takashi Yamamoto <Takashi02_Yamamoto@hq.scei.sony.co.jp>

Add PS3 logical performance monitor (lpm) device driver.

The PS3's LV1 hypervisor provides a Logical Performance Monitor that
abstarcts the Cell processor's performance monitor features for use
by guest operating systems.

Signed-off-by: Takashi Yamamoto <Takashi02_Yamamoto@hq.scei.sony.co.jp>
Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
---
 arch/powerpc/platforms/ps3/Kconfig |   14 
 drivers/ps3/Makefile               |    1 
 drivers/ps3/ps3-lpm.c              | 1171 +++++++++++++++++++++++++++++++++++++
 include/asm-powerpc/ps3.h          |   37 +
 4 files changed, 1223 insertions(+)
 create mode 100644 arch/powerpc/platforms/ps3/ps3-lpm.c

--- a/arch/powerpc/platforms/ps3/Kconfig
+++ b/arch/powerpc/platforms/ps3/Kconfig
@@ -138,4 +138,18 @@ config PS3_FLASH
 	  be disabled on the kernel command line using "ps3flash=off", to
 	  not allocate this fixed buffer.
 
+config PS3_LPM
+	tristate "PS3 Logical Performance Monitor support"
+	depends on PPC_PS3
+	default n
+	help
+	  Include support for the PS3 Logical Performance Monitor.
+
+	  This support is required to use the logical performance monitor
+	  of the PS3's LV1 hypervisor.
+
+	  If you intend to use the advanced performance monitoring and
+	  profiling support of the Cell processor with programs like
+	  oprofile and perfmon, then say Y or M, otherwise say N.
+
 endmenu
--- a/drivers/ps3/Makefile
+++ b/drivers/ps3/Makefile
@@ -4,3 +4,4 @@ ps3av_mod-objs		+= ps3av.o ps3av_cmd.o
 obj-$(CONFIG_PPC_PS3) += sys-manager-core.o
 obj-$(CONFIG_PS3_SYS_MANAGER) += ps3-sys-manager.o
 obj-$(CONFIG_PS3_STORAGE) += ps3stor_lib.o
+obj-$(CONFIG_PS3_LPM) += ps3-lpm.o
--- /dev/null
+++ b/drivers/ps3/ps3-lpm.c
@@ -0,0 +1,1171 @@
+/*
+ * PS3 Logical Performance Monitor.
+ *
+ *  Copyright (C) 2007 Sony Computer Entertainment Inc.
+ *  Copyright 2007 Sony Corp.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/uaccess.h>
+#include <asm/ps3.h>
+#include <asm/lv1call.h>
+#include <asm/cell-pmu.h>
+
+#define PS3_SIZE_OF_PM_INTERNAL_TRACE_BUFFER        0x4000
+#define PS3_SIZE_OF_PM_DEFAULT_TRACE_BUFFER_CACHE   0x4000
+
+/**
+ * struct ps3_lpm_priv - private lpm device data.
+ *
+ * @mutex: Open/close mutex.
+ * @rights: The lpm rigths granted by the system policy module.
+ * @pu_id: The lv1 id of the BE prosessor for this lpm instance.
+ * @lpm_id: The lv1 id of this lpm instance.
+ * @outlet_id: The outlet created by lv1 for this lpm instance.
+ * @constructed: A flag indicating the lpm driver has been opened -- can we just use (lpm_id == ???)
+ * @tb_size: The lv1 trace buffer size
+ * @tb_cache: Trace buffer cache
+ * @tb_cache_internal: A flag indicating the trace buffer cache was allocated
+ *                     by the driver.
+ * @tb_cache: Trace buffer cache
+ * @sizeof_traced_data: Traced data size
+ * @sbd: the struct ps3_system_bus_device attached to this driver
+ */
+
+struct ps3_lpm_priv {
+	struct mutex mutex;
+	u64 rights;
+	u64 pu_id;
+	u64 lpm_id;
+	u64 outlet_id;
+	int constructed;
+	u64 tb_size;
+	void *tb_cache;
+	u64 tb_cache_size;
+	int tb_cache_internal;
+	u64 sizeof_traced_data;
+	u64 sizeof_total_copied_data;
+	u64 shadow_pm_control;
+	u64 shadow_pm_start_stop;
+	u64 shadow_pm_interval;
+	u64 shadow_group_control;
+	u64 shadow_debug_bus_control;
+	struct ps3_system_bus_device *sbd;
+};
+
+/**
+ * lpm_priv - Static instance of the lpm data.
+ *
+ * Since the exported routines don't support the notion of a device
+ * instance we need to hold the instance in this static variable
+ * and only allow at most one instance to be created.
+ */
+
+static struct ps3_lpm_priv *lpm_priv;
+
+static struct device *sbd_core(void)
+{
+	BUG_ON(!lpm_priv || !lpm_priv->sbd);
+	return &lpm_priv->sbd->core;
+}
+
+/**
+ * use_start_stop_bookmark - Enable the PPU bookmark trace.
+ *
+ * And it enables PPU bookmark triggers ONLY if the other triggers are not set.
+ * The start/stop bookmarks are inserted at ps3_enable_pm() and ps3_disable_pm()
+ * to start/stop LPM.
+ *
+ * Used to get good quality of the performance counter.
+ */
+
+enum {use_start_stop_bookmark = 1,};
+
+/* BOOKMARK tag macros */
+#define PS3_PM_BOOKMARK_START                    0x8000000000000000ULL
+#define PS3_PM_BOOKMARK_STOP                     0x4000000000000000ULL
+#define PS3_PM_BOOKMARK_TAG_KERNEL               0x1000000000000000ULL
+#define PS3_PM_BOOKMARK_TAG_USER                 0x3000000000000000ULL
+#define PS3_PM_BOOKMARK_TAG_MASK_HI              0xF000000000000000ULL
+#define PS3_PM_BOOKMARK_TAG_MASK_LO              0x0F00000000000000ULL
+
+/* CBE PM CONTROL register macros */
+#define PS3_PM_CONTROL_PPU_TH0_BOOKMARK          0x00001000
+#define PS3_PM_CONTROL_PPU_TH1_BOOKMARK          0x00000800
+#define PS3_PM_CONTROL_PPU_COUNT_MODE_MASK       0x000C0000
+#define PS3_PM_CONTROL_PPU_COUNT_MODE_PROBLEM    0x00080000
+#define PS3_WRITE_PM_MASK                        0xFFFFFFFFFFFFFFFFULL
+
+/* CBE PM START STOP register macros */
+#define PS3_PM_START_STOP_PPU_TH0_BOOKMARK_START 0x02000000
+#define PS3_PM_START_STOP_PPU_TH1_BOOKMARK_START 0x01000000
+#define PS3_PM_START_STOP_PPU_TH0_BOOKMARK_STOP  0x00020000
+#define PS3_PM_START_STOP_PPU_TH1_BOOKMARK_STOP  0x00010000
+#define PS3_PM_START_STOP_START_MASK             0xFF000000
+#define PS3_PM_START_STOP_STOP_MASK              0x00FF0000
+
+/* CBE PM COUNTER register macres */
+#define PS3_PM_COUNTER_MASK_HI                   0xFFFFFFFF00000000ULL
+#define PS3_PM_COUNTER_MASK_LO                   0x00000000FFFFFFFFULL
+
+/* BASE SIGNAL GROUP NUMBER macros */
+#define PM_ISLAND2_BASE_SIGNAL_GROUP_NUMBER  0
+#define PM_ISLAND2_SIGNAL_GROUP_NUMBER1      6
+#define PM_ISLAND2_SIGNAL_GROUP_NUMBER2      7
+#define PM_ISLAND3_BASE_SIGNAL_GROUP_NUMBER  7
+#define PM_ISLAND4_BASE_SIGNAL_GROUP_NUMBER  15
+#define PM_SPU_TRIGGER_SIGNAL_GROUP_NUMBER   17
+#define PM_SPU_EVENT_SIGNAL_GROUP_NUMBER     18
+#define PM_ISLAND5_BASE_SIGNAL_GROUP_NUMBER  18
+#define PM_ISLAND6_BASE_SIGNAL_GROUP_NUMBER  24
+#define PM_ISLAND7_BASE_SIGNAL_GROUP_NUMBER  49
+#define PM_ISLAND8_BASE_SIGNAL_GROUP_NUMBER  52
+#define PM_SIG_GROUP_SPU                     41
+#define PM_SIG_GROUP_SPU_TRIGGER             42
+#define PM_SIG_GROUP_SPU_EVENT               43
+#define PM_SIG_GROUP_MFC_MAX                 60
+
+/* shadow register macros */
+#define PS3_SHADOW_REG_INIT_VALUE          0xFFFFFFFF00000000ULL
+
+/* bookmark spr address */
+#define BOOKMARK_SPR_ADDR 1020
+
+void ps3_set_bookmark(u64 bookmark)
+{
+	/*
+	 * As per PPE book IV, to avoid bookmark lost there
+	 * must not be a traced branch within 10 cycles of
+	 * setting the bookmark spr.
+	 */
+
+	asm volatile("or 29, 29, 29;"); /* db10cyc */
+	mtspr(BOOKMARK_SPR_ADDR, bookmark);
+	asm volatile("or 29, 29, 29;"); /* db10cyc */
+}
+EXPORT_SYMBOL_GPL(ps3_set_bookmark);
+
+void ps3_set_pm_bookmark(u64 tag, u64 incident, u64 th_id)
+{
+	u64 bookmark;
+
+	bookmark = (get_tb() & 0x00000000FFFFFFFFULL) |
+		PS3_PM_BOOKMARK_TAG_KERNEL;
+	bookmark = ((tag << 56) & PS3_PM_BOOKMARK_TAG_MASK_LO) |
+		(incident << 48) | (th_id << 32) | bookmark;
+	ps3_set_bookmark(bookmark);
+}
+EXPORT_SYMBOL_GPL(ps3_set_pm_bookmark);
+
+/**
+ * ps3_read_phys_ctr - Read physical counter registers.
+ *
+ * Each physical counter can act as one 32 bit counter or as two 16 bit
+ * counters.
+ */
+
+u32 ps3_read_phys_ctr(u32 cpu, u32 phys_ctr)
+{
+	int result;
+	u64 counter0415;
+	u64 counter2637;
+
+	if (phys_ctr >= NR_PHYS_CTRS) {
+		dev_dbg(sbd_core(), "%s:%u: phys_ctr too big: %u\n", __func__,
+			__LINE__, phys_ctr);
+		return 0;
+	}
+
+	result = lv1_set_lpm_counter(lpm_priv->lpm_id, 0, 0, 0, 0, &counter0415,
+				     &counter2637);
+	if (result) {
+		dev_err(sbd_core(), "%s:%u: lv1_set_lpm_counter failed: "
+			"phys_ctr %u, %s\n", __func__, __LINE__, phys_ctr,
+			ps3_result(result));
+		return 0;
+	}
+
+	switch (phys_ctr) {
+	case 0:
+		return counter0415 >> 32;
+	case 1:
+		return counter0415 & PS3_PM_COUNTER_MASK_LO;
+	case 2:
+		return counter2637 >> 32;
+	case 3:
+		return counter2637 & PS3_PM_COUNTER_MASK_LO;
+	default:
+		BUG();
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ps3_read_phys_ctr);
+
+/**
+ * ps3_write_phys_ctr - Write physical counter registers.
+ *
+ * Each physical counter can act as one 32 bit counter or as two 16 bit
+ * counters.
+ */
+
+void ps3_write_phys_ctr(u32 cpu, u32 phys_ctr, u32 val)
+{
+	u64 counter0415;
+	u64 counter0415_mask;
+	u64 counter2637;
+	u64 counter2637_mask;
+	int result;
+
+	if (phys_ctr >= NR_PHYS_CTRS) {
+		dev_dbg(sbd_core(), "%s:%u: phys_ctr too big: %u\n", __func__,
+			__LINE__, phys_ctr);
+		return;
+	}
+
+	switch (phys_ctr) {
+	case 0:
+		counter0415 = (u64)val << 32;
+		counter0415_mask = PS3_PM_COUNTER_MASK_HI;
+		counter2637 = 0x0;
+		counter2637_mask = 0x0;
+		break;
+	case 1:
+		counter0415 = (u64)val;
+		counter0415_mask = PS3_PM_COUNTER_MASK_LO;
+		counter2637 = 0x0;
+		counter2637_mask = 0x0;
+		break;
+	case 2:
+		counter0415 = 0x0;
+		counter0415_mask = 0x0;
+		counter2637 = (u64)val << 32;
+		counter2637_mask = PS3_PM_COUNTER_MASK_HI;
+		break;
+	case 3:
+		counter0415 = 0x0;
+		counter0415_mask = 0x0;
+		counter2637 = (u64)val;
+		counter2637_mask = PS3_PM_COUNTER_MASK_LO;
+		break;
+	default:
+		BUG();
+	}
+
+	result = lv1_set_lpm_counter(lpm_priv->lpm_id,
+				     counter0415, counter0415_mask,
+				     counter2637, counter2637_mask,
+				     &counter0415, &counter2637);
+	if (result)
+		dev_err(sbd_core(), "%s:%u: lv1_set_lpm_counter failed: "
+			"phys_ctr %u, val %u, %s\n", __func__, __LINE__,
+			phys_ctr, val, ps3_result(result));
+}
+EXPORT_SYMBOL_GPL(ps3_write_phys_ctr);
+
+/**
+ * ps3_read_ctr - Read counter.
+ *
+ * Read 16 or 32 bits depending on the current size of the counter.
+ * Counters 4, 5, 6 & 7 are always 16 bit.
+ */
+
+u32 ps3_read_ctr(u32 cpu, u32 ctr)
+{
+	u32 val;
+	u32 phys_ctr = ctr & (NR_PHYS_CTRS - 1);
+
+	val = ps3_read_phys_ctr(cpu, phys_ctr);
+
+	if (ps3_get_ctr_size(cpu, phys_ctr) == 16)
+		val = (ctr < NR_PHYS_CTRS) ? (val >> 16) : (val & 0xffff);
+
+	return val;
+}
+EXPORT_SYMBOL_GPL(ps3_read_ctr);
+
+/**
+ * ps3_write_ctr - Write counter.
+ *
+ * Write 16 or 32 bits depending on the current size of the counter.
+ * Counters 4, 5, 6 & 7 are always 16 bit.
+ */
+
+void ps3_write_ctr(u32 cpu, u32 ctr, u32 val)
+{
+	u32 phys_ctr;
+	u32 phys_val;
+
+	phys_ctr = ctr & (NR_PHYS_CTRS - 1);
+
+	if (ps3_get_ctr_size(cpu, phys_ctr) == 16) {
+		phys_val = ps3_read_phys_ctr(cpu, phys_ctr);
+
+		if (ctr < NR_PHYS_CTRS)
+			val = (val << 16) | (phys_val & 0xffff);
+		else
+			val = (val & 0xffff) | (phys_val & 0xffff0000);
+	}
+
+	ps3_write_phys_ctr(cpu, phys_ctr, val);
+}
+EXPORT_SYMBOL_GPL(ps3_write_ctr);
+
+/**
+ * ps3_read_pm07_control - Read counter control registers.
+ *
+ * Each logical counter has a corresponding control register.
+ */
+
+u32 ps3_read_pm07_control(u32 cpu, u32 ctr)
+{
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ps3_read_pm07_control);
+
+/**
+ * ps3_write_pm07_control - Write counter control registers.
+ *
+ * Each logical counter has a corresponding control register.
+ */
+
+void ps3_write_pm07_control(u32 cpu, u32 ctr, u32 val)
+{
+	int result;
+	static const u64 mask = 0xFFFFFFFFFFFFFFFFULL;
+	u64 old_value;
+
+	if (ctr >= NR_CTRS) {
+		dev_dbg(sbd_core(), "%s:%u: ctr too big: %u\n", __func__,
+			__LINE__, ctr);
+		return;
+	}
+
+	result = lv1_set_lpm_counter_control(lpm_priv->lpm_id, ctr, val, mask,
+					     &old_value);
+	if (result)
+		dev_err(sbd_core(), "%s:%u: lv1_set_lpm_counter_control "
+			"failed: ctr %u, %s\n", __func__, __LINE__, ctr,
+			ps3_result(result));
+}
+EXPORT_SYMBOL_GPL(ps3_write_pm07_control);
+
+/**
+ * ps3_read_pm - Read Other LPM control registers.
+ */
+
+u32 ps3_read_pm(u32 cpu, enum pm_reg_name reg)
+{
+	switch (reg) {
+	case pm_control:
+		return lpm_priv->shadow_pm_control;
+	case trace_address:
+		return CBE_PM_TRACE_BUF_EMPTY;
+	case pm_start_stop:
+		return lpm_priv->shadow_pm_start_stop;
+	default:
+		dev_dbg(sbd_core(), "%s:%u: unknown reg: %d\n", __func__,
+			__LINE__, reg);
+		BUG();
+		break;
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ps3_read_pm);
+
+/**
+ * ps3_write_pm - Write Other LPM control registers.
+ */
+
+void ps3_write_pm(u32 cpu, enum pm_reg_name reg, u32 val)
+{
+	int result = 0;
+	u64 dummy;
+
+	switch (reg) {
+	case group_control:
+		if (val != lpm_priv->shadow_group_control)
+			result = lv1_set_lpm_group_control(lpm_priv->lpm_id,
+							   val,
+							   PS3_WRITE_PM_MASK,
+							   &dummy);
+		lpm_priv->shadow_group_control = val;
+		break;
+	case debug_bus_control:
+		if (val != lpm_priv->shadow_debug_bus_control)
+			result = lv1_set_lpm_debug_bus_control(lpm_priv->lpm_id,
+							      val,
+							      PS3_WRITE_PM_MASK,
+							      &dummy);
+		lpm_priv->shadow_debug_bus_control = val;
+		break;
+	case pm_control:
+		if (use_start_stop_bookmark)
+			val |= (PS3_PM_CONTROL_PPU_TH0_BOOKMARK |
+				PS3_PM_CONTROL_PPU_TH1_BOOKMARK);
+		if (val != lpm_priv->shadow_pm_control)
+			result = lv1_set_lpm_general_control(lpm_priv->lpm_id,
+							     val,
+							     PS3_WRITE_PM_MASK,
+							     0, 0, &dummy,
+							     &dummy);
+		lpm_priv->shadow_pm_control = val;
+		break;
+	case pm_interval:
+		if (val != lpm_priv->shadow_pm_interval)
+			result = lv1_set_lpm_interval(lpm_priv->lpm_id, val,
+						   PS3_WRITE_PM_MASK, &dummy);
+		lpm_priv->shadow_pm_interval = val;
+		break;
+	case pm_start_stop:
+		if (val != lpm_priv->shadow_pm_start_stop)
+			result = lv1_set_lpm_trigger_control(lpm_priv->lpm_id,
+							     val,
+							     PS3_WRITE_PM_MASK,
+							     &dummy);
+		lpm_priv->shadow_pm_start_stop = val;
+		break;
+	default:
+		dev_dbg(sbd_core(), "%s:%u: unknown reg: %d\n", __func__,
+			__LINE__, reg);
+		BUG();
+		break;
+	}
+
+	if (result)
+		dev_err(sbd_core(), "%s:%u: lv1 set_control failed: "
+			"reg %u, %s\n", __func__, __LINE__, reg,
+			ps3_result(result));
+}
+EXPORT_SYMBOL_GPL(ps3_write_pm);
+
+/**
+ * ps3_get_ctr_size - Get the size of a physical counter.
+ *
+ * Returns either 16 or 32.
+ */
+
+u32 ps3_get_ctr_size(u32 cpu, u32 phys_ctr)
+{
+	u32 pm_ctrl;
+
+	if (phys_ctr >= NR_PHYS_CTRS) {
+		dev_dbg(sbd_core(), "%s:%u: phys_ctr too big: %u\n", __func__,
+			__LINE__, phys_ctr);
+		return 0;
+	}
+
+	pm_ctrl = ps3_read_pm(cpu, pm_control);
+	return (pm_ctrl & CBE_PM_16BIT_CTR(phys_ctr)) ? 16 : 32;
+}
+EXPORT_SYMBOL_GPL(ps3_get_ctr_size);
+
+/**
+ * ps3_set_ctr_size - Set the size of a physical counter to 16 or 32 bits.
+ */
+
+void ps3_set_ctr_size(u32 cpu, u32 phys_ctr, u32 ctr_size)
+{
+	u32 pm_ctrl;
+
+	if (phys_ctr >= NR_PHYS_CTRS) {
+		dev_dbg(sbd_core(), "%s:%u: phys_ctr too big: %u\n", __func__,
+			__LINE__, phys_ctr);
+		return;
+	}
+
+	pm_ctrl = ps3_read_pm(cpu, pm_control);
+
+	switch (ctr_size) {
+	case 16:
+		pm_ctrl |= CBE_PM_16BIT_CTR(phys_ctr);
+		ps3_write_pm(cpu, pm_control, pm_ctrl);
+		break;
+
+	case 32:
+		pm_ctrl &= ~CBE_PM_16BIT_CTR(phys_ctr);
+		ps3_write_pm(cpu, pm_control, pm_ctrl);
+		break;
+	default:
+		BUG();
+	}
+}
+EXPORT_SYMBOL_GPL(ps3_set_ctr_size);
+
+static u64 pm_translate_signal_group_number_on_island2(u64 subgroup)
+{
+
+	if (subgroup == 2)
+		subgroup = 3;
+
+	if (subgroup <= 6)
+		return PM_ISLAND2_BASE_SIGNAL_GROUP_NUMBER + subgroup;
+	else if (subgroup == 7)
+		return PM_ISLAND2_SIGNAL_GROUP_NUMBER1;
+	else
+		return PM_ISLAND2_SIGNAL_GROUP_NUMBER2;
+}
+
+static u64 pm_translate_signal_group_number_on_island3(u64 subgroup)
+{
+
+	switch (subgroup) {
+	case 2:
+	case 3:
+	case 4:
+		subgroup += 2;
+		break;
+	case 5:
+		subgroup = 8;
+		break;
+	default:
+		break;
+	}
+	return PM_ISLAND3_BASE_SIGNAL_GROUP_NUMBER + subgroup;
+}
+
+static u64 pm_translate_signal_group_number_on_island4(u64 subgroup)
+{
+	return PM_ISLAND4_BASE_SIGNAL_GROUP_NUMBER + subgroup;
+}
+
+static u64 pm_translate_signal_group_number_on_island5(u64 subgroup)
+{
+
+	switch (subgroup) {
+	case 3:
+		subgroup = 4;
+		break;
+	case 4:
+		subgroup = 6;
+		break;
+	default:
+		break;
+	}
+	return PM_ISLAND5_BASE_SIGNAL_GROUP_NUMBER + subgroup;
+}
+
+static u64 pm_translate_signal_group_number_on_island6(u64 subgroup,
+							      u64 subsubgroup)
+{
+	switch (subgroup) {
+	case 3:
+	case 4:
+	case 5:
+		subgroup += 1;
+		break;
+	default:
+		break;
+	}
+
+	switch (subsubgroup) {
+	case 4:
+	case 5:
+	case 6:
+		subsubgroup += 2;
+		break;
+	case 7:
+	case 8:
+	case 9:
+	case 10:
+		subsubgroup += 4;
+		break;
+	case 11:
+	case 12:
+	case 13:
+		subsubgroup += 5;
+		break;
+	default:
+		break;
+	}
+
+	if (subgroup <= 5)
+		return (PM_ISLAND6_BASE_SIGNAL_GROUP_NUMBER + subgroup);
+	else
+		return (PM_ISLAND6_BASE_SIGNAL_GROUP_NUMBER + subgroup
+			+ subsubgroup - 1);
+}
+
+static u64 pm_translate_signal_group_number_on_island7(u64 subgroup)
+{
+	return PM_ISLAND7_BASE_SIGNAL_GROUP_NUMBER + subgroup;
+}
+
+static u64 pm_translate_signal_group_number_on_island8(u64 subgroup)
+{
+	return PM_ISLAND8_BASE_SIGNAL_GROUP_NUMBER + subgroup;
+}
+
+static u64 pm_signal_group_to_ps3_lv1_signal_group(u64 group)
+{
+	u64 island;
+	u64 subgroup;
+	u64 subsubgroup;
+
+	subgroup = 0;
+	subsubgroup = 0;
+	island = 0;
+	if (group < 1000) {
+		if (group < 100) {
+			if (20 <= group && group < 30) {
+				island = 2;
+				subgroup = group - 20;
+			} else if (30 <= group && group < 40) {
+				island = 3;
+				subgroup = group - 30;
+			} else if (40 <= group && group < 50) {
+				island = 4;
+				subgroup = group - 40;
+			} else if (50 <= group && group < 60) {
+				island = 5;
+				subgroup = group - 50;
+			} else if (60 <= group && group < 70) {
+				island = 6;
+				subgroup = group - 60;
+			} else if (70 <= group && group < 80) {
+				island = 7;
+				subgroup = group - 70;
+			} else if (80 <= group && group < 90) {
+				island = 8;
+				subgroup = group - 80;
+			}
+		} else if (200 <= group && group < 300) {
+			island = 2;
+			subgroup = group - 200;
+		} else if (600 <= group && group < 700) {
+			island = 6;
+			subgroup = 5;
+			subsubgroup = group - 650;
+		}
+	} else if (6000 <= group && group < 7000) {
+		island = 6;
+		subgroup = 5;
+		subsubgroup = group - 6500;
+	}
+
+	switch (island) {
+	case 2:
+		return pm_translate_signal_group_number_on_island2(subgroup);
+	case 3:
+		return pm_translate_signal_group_number_on_island3(subgroup);
+	case 4:
+		return pm_translate_signal_group_number_on_island4(subgroup);
+	case 5:
+		return pm_translate_signal_group_number_on_island5(subgroup);
+	case 6:
+		return pm_translate_signal_group_number_on_island6(subgroup,
+								   subsubgroup);
+	case 7:
+		return pm_translate_signal_group_number_on_island7(subgroup);
+	case 8:
+		return pm_translate_signal_group_number_on_island8(subgroup);
+	default:
+		dev_dbg(sbd_core(), "%s:%u: island not found: %lu\n", __func__,
+			__LINE__, group);
+		BUG();
+		break;
+	}
+	return 0;
+}
+
+static u64 pm_bus_word_to_ps3_lv1_bus_word(u8 word)
+{
+
+	switch (word) {
+	case 1:
+		return 0xF000;
+	case 2:
+		return 0x0F00;
+	case 4:
+		return 0x00F0;
+	case 8:
+	default:
+		return 0x000F;
+	}
+}
+
+static int __ps3_set_signal(u64 lv1_signal_group, u64 bus_select,
+			    u64 signal_select, u64 attr1, u64 attr2, u64 attr3)
+{
+	int ret;
+
+	ret = lv1_set_lpm_signal(lpm_priv->lpm_id, lv1_signal_group, bus_select,
+				 signal_select, attr1, attr2, attr3);
+	if (ret)
+		dev_err(sbd_core(),
+			"%s:%u: error:%d 0x%lx 0x%lx 0x%lx 0x%lx 0x%lx 0x%lx\n",
+			__func__, __LINE__, ret, lv1_signal_group, bus_select,
+			signal_select, attr1, attr2, attr3);
+
+	return ret;
+}
+
+int ps3_set_signal(u64 signal_group, u8 signal_bit, u16 sub_unit,
+		   u8 bus_word)
+{
+	int ret;
+	u64 lv1_signal_group;
+	u64 bus_select;
+	u64 signal_select;
+	u64 attr1, attr2, attr3;
+
+	if (signal_group == 0)
+		return __ps3_set_signal(0, 0, 0, 0, 0, 0);
+
+	lv1_signal_group =
+		pm_signal_group_to_ps3_lv1_signal_group(signal_group);
+	bus_select = pm_bus_word_to_ps3_lv1_bus_word(bus_word);
+
+	switch (signal_group) {
+	case PM_SIG_GROUP_SPU_TRIGGER:
+		signal_select = 1;
+		signal_select = signal_select << (63 - signal_bit);
+		break;
+	case PM_SIG_GROUP_SPU_EVENT:
+		signal_select = 1;
+		signal_select = (signal_select << (63 - signal_bit)) | 0x3;
+		break;
+	default:
+		signal_select = 0;
+		break;
+	}
+
+	/*
+	 * 0: physical object.
+	 * 1: logical object.
+	 * This parameter is only used for the PPE and SPE signals.
+	 */
+	attr1 = 1;
+
+	/*
+	 * This parameter is used to specify the target physical/logical
+	 * PPE/SPE object.
+	 */
+	if (PM_SIG_GROUP_SPU <= signal_group &&
+		signal_group < PM_SIG_GROUP_MFC_MAX)
+		attr2 = sub_unit;
+	else
+		attr2 = lpm_priv->pu_id;;
+
+	/*
+	 * This parameter is only used for setting the SPE signal.
+	 */
+	attr3 = 0;
+
+	ret = __ps3_set_signal(lv1_signal_group, bus_select, signal_select,
+			       attr1, attr2, attr3);
+	if (ret)
+		dev_err(sbd_core(), "%s:%u: __ps3_set_signal failed: %d\n",
+			__func__, __LINE__, ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(ps3_set_signal);
+
+inline u32 ps3_get_hw_thread_id(int cpu)
+{
+	return cpu;
+}
+EXPORT_SYMBOL_GPL(ps3_get_hw_thread_id);
+
+/**
+ * ps3_enable_pm - Enable the entire performance monitoring unit.
+ *
+ * When we enable the LPM, all pending writes to counters get committed.
+ */
+
+void ps3_enable_pm(u32 cpu)
+{
+	int result;
+	u64 tmp;
+	int insert_bookmark = 0;
+
+	if (use_start_stop_bookmark) {
+		if (!(lpm_priv->shadow_pm_start_stop &
+			(PS3_PM_START_STOP_START_MASK
+			| PS3_PM_START_STOP_STOP_MASK))) {
+			result = lv1_set_lpm_trigger_control(lpm_priv->lpm_id,
+				(PS3_PM_START_STOP_PPU_TH0_BOOKMARK_START |
+				PS3_PM_START_STOP_PPU_TH1_BOOKMARK_START |
+				PS3_PM_START_STOP_PPU_TH0_BOOKMARK_STOP |
+				PS3_PM_START_STOP_PPU_TH1_BOOKMARK_STOP),
+				0xFFFFFFFFFFFFFFFFULL, &tmp);
+
+			if (result)
+				dev_err(sbd_core(), "%s:%u: "
+					"lv1_set_lpm_trigger_control failed: "
+					"%s\n", __func__, __LINE__,
+					ps3_result(result));
+
+			insert_bookmark = !result;
+		}
+	}
+
+	result = lv1_start_lpm(lpm_priv->lpm_id);
+
+	if (result)
+		dev_err(sbd_core(), "%s:%u: lv1_start_lpm failed: %s\n",
+			__func__, __LINE__, ps3_result(result));
+
+	if (use_start_stop_bookmark && result && insert_bookmark)
+		ps3_set_bookmark(get_tb() | PS3_PM_BOOKMARK_START);
+}
+EXPORT_SYMBOL_GPL(ps3_enable_pm);
+
+/**
+ * ps3_disable_pm - Disable the entire performance monitoring unit.
+ */
+
+void ps3_disable_pm(u32 cpu)
+{
+	int result;
+	u64 param = 0;
+
+	ps3_set_bookmark(get_tb() | PS3_PM_BOOKMARK_STOP);
+
+	result = lv1_stop_lpm(lpm_priv->lpm_id, &param);
+
+	if (result) {
+		dev_err(sbd_core(), "%s:%u: lv1_stop_lpm failed: %s\n",
+			__func__, __LINE__, ps3_result(result));
+		return;
+	}
+
+	lpm_priv->sizeof_traced_data = param;
+	lpm_priv->sizeof_total_copied_data = 0;
+}
+EXPORT_SYMBOL_GPL(ps3_disable_pm);
+
+/**
+ * _ps3_copy_trace_buffer - Copy the trace buffer.
+ */
+
+static u64 _ps3_copy_trace_buffer(u64 offset, u64 size, u64 *to, int to_user)
+{
+	int result;
+	u64 sizeof_copied_data;
+
+	if (offset >= lpm_priv->sizeof_traced_data)
+		return 0;
+
+	result = lv1_copy_lpm_trace_buffer(lpm_priv->lpm_id, offset, size,
+					&sizeof_copied_data);
+	if (result) {
+		dev_err(sbd_core(), "%s:%u: lv1_copy_lpm_trace_buffer failed: "
+			"offset 0x%lx, size 0x%lx: %s\n", __func__, __LINE__,
+			offset, size, ps3_result(result));
+		return 0;
+	}
+
+	if (to_user) {
+		result = copy_to_user((void __user *)to, lpm_priv->tb_cache,
+				      sizeof_copied_data);
+		if (result) {
+			dev_err(sbd_core(), "%s:%u: copy_to_user() error. "
+				"offset:0x%lx size:0x%lx dest:0x%p src:0x%p\n",
+				__func__, __LINE__, offset, sizeof_copied_data,
+				to, lpm_priv->tb_cache);
+			return 0;
+		}
+	} else
+		memcpy(to, lpm_priv->tb_cache, sizeof_copied_data);
+
+	return sizeof_copied_data;
+}
+
+u64 ps3_copy_trace_buffer(u64 offset, u64 size, void *to, int to_user)
+{
+	u64 sz;
+	u64 cp_size;
+	u64 total_cp_size;
+
+	if (!lpm_priv->tb_cache)
+		return 0;
+
+	cp_size = size;
+	if (cp_size > lpm_priv->tb_cache_size)
+		cp_size = lpm_priv->tb_cache_size;
+
+	total_cp_size = 0;
+	while (total_cp_size < size) {
+		sz = _ps3_copy_trace_buffer(offset, cp_size, to, to_user);
+		if (!sz)
+			break;
+
+		total_cp_size += sz;
+		offset += sz;
+		to = ((u8 *)to + sz);
+	}
+	return total_cp_size;
+}
+
+/**
+ * ps3_get_and_clear_pm_interrupts -
+ *
+ * Clearing interrupts for the entire performance monitoring unit.
+ * Reading pm_status clears the interrupt bits.
+ */
+
+u32 ps3_get_and_clear_pm_interrupts(u32 cpu)
+{
+	return ps3_read_pm(cpu, pm_status);
+}
+EXPORT_SYMBOL_GPL(ps3_get_and_clear_pm_interrupts);
+
+/**
+ * ps3_enable_pm_interrupts -
+ *
+ * Enabling interrupts for the entire performance monitoring unit.
+ * Enables the interrupt bits in the pm_status register.
+ */
+
+void ps3_enable_pm_interrupts(u32 cpu, u32 thread, u32 mask)
+{
+	if (mask)
+		ps3_write_pm(cpu, pm_status, mask);
+}
+EXPORT_SYMBOL_GPL(ps3_enable_pm_interrupts);
+
+/**
+ * ps3_enable_pm_interrupts -
+ *
+ * Disabling interrupts for the entire performance monitoring unit.
+ */
+
+void ps3_disable_pm_interrupts(u32 cpu)
+{
+	ps3_get_and_clear_pm_interrupts(cpu);
+	ps3_write_pm(cpu, pm_status, 0);
+}
+EXPORT_SYMBOL_GPL(ps3_disable_pm_interrupts);
+
+/**
+ * ps3_lpm_open - Open the lpm device.
+ * @tb_cache: Optional user supplied buffer for the tb cache.  If NULL,
+ *  the driver will use an internally managed tb cache buffer.
+ * @tb_cache_size: The size in bytes of the user supplied tb cache buffer.
+ *  Unused if @tb_cache is NULL.
+ * @tb_type:
+ *
+ */
+
+int ps3_lpm_open(void *tb_cache, u64 tb_cache_size, u64 tb_type)
+{
+	int result;
+	u64 cbe_node_id;
+	u64 tb_size;
+	u64 ctrl_opt;
+	u64 tb_cache_lpar_addr;
+	u64 lpm_id;
+	u64 outlet_id;
+	u64 used_tb_size;
+
+	if (!lpm_priv) {
+		BUG();
+		return -ENODEV;
+	}
+
+	mutex_lock(&lpm_priv->mutex);
+
+	if (lpm_priv->constructed) {
+		dev_err(sbd_core(), "%s:%u: called twice.\n", __func__,
+			__LINE__);
+		result = -EBUSY;
+		goto unlock;
+	}
+
+	if (tb_cache)
+		lpm_priv->tb_cache_internal = 0;
+	else {
+		dev_dbg(sbd_core(), "%s:%u: Using internal TB cache\n",
+			__func__, __LINE__);
+
+		lpm_priv->tb_cache_internal = 1;
+		tb_cache_size = PS3_SIZE_OF_PM_DEFAULT_TRACE_BUFFER_CACHE;
+		tb_cache = kzalloc(tb_cache_size, GFP_KERNEL);
+
+		if (!tb_cache) {
+			dev_err(sbd_core(), "%s:%u: alloc internal tb_cache "
+				"failed\n", __func__, __LINE__);
+			result = -ENOMEM;
+			goto fail_malloc;
+		}
+	}
+
+	cbe_node_id = 0;
+	if (tb_cache) {
+		if (tb_type == 0) {
+			/* no trace buffer */
+			tb_type = 0;
+			tb_size = 0;
+			ctrl_opt = 0;
+			tb_cache_lpar_addr = 0;
+		} else if (tb_type == 1) {
+			/* internal trace buffer */
+			tb_size = PS3_SIZE_OF_PM_INTERNAL_TRACE_BUFFER;
+			ctrl_opt = 0;
+		} else {
+			dev_err(sbd_core(), "%s:%u: Unkown TB type: 0x%lx\n",
+				__func__, __LINE__, tb_type);
+			result = -EINVAL;
+			goto fail_type;
+		}
+		tb_cache_lpar_addr = (u64)ps3_mm_phys_to_lpar(__pa(tb_cache));
+	} else {
+		/* no trace buffer */
+		tb_type = 0;
+		tb_size = 0;
+		ctrl_opt = 0;
+		tb_cache_lpar_addr = 0;
+	}
+
+	result = lv1_construct_lpm(cbe_node_id, tb_type, tb_size, ctrl_opt,
+				tb_cache_lpar_addr, tb_cache_size,
+				&lpm_id, &outlet_id, &used_tb_size);
+
+	if (result) {
+		dev_err(sbd_core(), "%s:%u: lv1_construct_lpm failed: %s\n",
+			__func__, __LINE__, ps3_result(result));
+		result = -EINVAL;
+		goto fail_construct;
+	}
+
+	lpm_priv->constructed = 1;
+	lpm_priv->tb_cache = tb_cache;
+	lpm_priv->tb_cache_size = tb_cache_size;
+	lpm_priv->lpm_id = lpm_id;
+	lpm_priv->outlet_id = outlet_id;
+	lpm_priv->tb_size = used_tb_size;
+	lpm_priv->shadow_pm_control = PS3_SHADOW_REG_INIT_VALUE;
+	lpm_priv->shadow_pm_start_stop = PS3_SHADOW_REG_INIT_VALUE;
+	lpm_priv->shadow_pm_interval = PS3_SHADOW_REG_INIT_VALUE;
+	lpm_priv->shadow_group_control = PS3_SHADOW_REG_INIT_VALUE;
+	lpm_priv->shadow_debug_bus_control = PS3_SHADOW_REG_INIT_VALUE;
+
+	dev_dbg(sbd_core(), "%s:%u: lpm id 0x%lx, outlet 0x%lx, "
+		"tb_size 0x%lx\n", __func__, __LINE__, lpm_priv->lpm_id,
+		lpm_priv->outlet_id, lpm_priv->tb_size);
+
+	mutex_unlock(&lpm_priv->mutex);
+	return 0;
+
+fail_construct:
+fail_type:
+	if (lpm_priv->tb_cache_internal) {
+		kfree(lpm_priv->tb_cache);
+		lpm_priv->tb_cache = NULL;
+	}
+fail_malloc:
+unlock:
+	mutex_unlock(&lpm_priv->mutex);
+	return result;
+}
+EXPORT_SYMBOL_GPL(ps3_lpm_open);
+
+/**
+ * ps3_lpm_close - Close the lpm device.
+ *
+ */
+
+int ps3_lpm_close(void)
+{
+	dev_dbg(sbd_core(), "%s:%u\n", __func__, __LINE__);
+
+	mutex_lock(&lpm_priv->mutex);
+
+	if (lpm_priv->constructed)
+		lv1_destruct_lpm(lpm_priv->lpm_id);
+
+	lpm_priv->constructed = 0;
+	lpm_priv->lpm_id = 0;
+
+	if (lpm_priv->tb_cache_internal) {
+		kfree(lpm_priv->tb_cache);
+		lpm_priv->tb_cache = NULL;
+	}
+
+	mutex_unlock(&lpm_priv->mutex);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ps3_lpm_close);
+
+static int __devinit ps3_lpm_probe(struct ps3_system_bus_device *dev)
+{
+	dev_dbg(&dev->core, " -> %s:%u\n", __func__, __LINE__);
+
+	if (lpm_priv) {
+		dev_info(&dev->core, "%s:%u: called twice\n",
+			__func__, __LINE__);
+		return -EBUSY;
+	}
+
+	lpm_priv = kzalloc(sizeof(*lpm_priv), GFP_KERNEL);
+
+	if (!lpm_priv)
+		return -ENOMEM;
+
+	lpm_priv->sbd = dev;
+	lpm_priv->pu_id = dev->lpm.pu_id;
+	lpm_priv->rights = dev->lpm.rights;
+	mutex_init(&lpm_priv->mutex);
+
+	dev_info(&dev->core, " <- %s:%u:\n", __func__, __LINE__);
+
+	return 0;
+}
+
+static int ps3_lpm_remove(struct ps3_system_bus_device *dev)
+{
+	dev_dbg(&dev->core, " -> %s:%u:\n", __func__, __LINE__);
+
+	ps3_lpm_close();
+
+	kfree(lpm_priv);
+	lpm_priv = NULL;
+
+	dev_info(&dev->core, " <- %s:%u:\n", __func__, __LINE__);
+	return 0;
+}
+
+static struct ps3_system_bus_driver ps3_lpm_driver = {
+	.match_id = PS3_MATCH_ID_LPM,
+	.core.name	= "ps3-lpm",
+	.core.owner	= THIS_MODULE,
+	.probe		= ps3_lpm_probe,
+	.remove		= ps3_lpm_remove,
+	.shutdown	= ps3_lpm_remove,
+};
+
+static int __init ps3_lpm_init(void)
+{
+	pr_debug("%s:%d:\n", __func__, __LINE__);
+	return ps3_system_bus_driver_register(&ps3_lpm_driver);
+}
+
+static void __exit ps3_lpm_exit(void)
+{
+	pr_debug("%s:%d:\n", __func__, __LINE__);
+	ps3_system_bus_driver_unregister(&ps3_lpm_driver);
+}
+
+module_init(ps3_lpm_init);
+module_exit(ps3_lpm_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("PS3 Logical Performance Monitor Driver");
+MODULE_AUTHOR("Sony Corporation");
+MODULE_ALIAS(PS3_MODULE_ALIAS_LPM);
--- a/include/asm-powerpc/ps3.h
+++ b/include/asm-powerpc/ps3.h
@@ -24,6 +24,7 @@
 #include <linux/init.h>
 #include <linux/types.h>
 #include <linux/device.h>
+#include "cell-pmu.h"
 
 union ps3_firmware_version {
 	u64 raw;
@@ -445,5 +446,41 @@ struct ps3_prealloc {
 extern struct ps3_prealloc ps3fb_videomemory;
 extern struct ps3_prealloc ps3flash_bounce_buffer;
 
+/* logical performance monitor */
+
+enum ps3_lpm_rights {
+	PS3_LPM_RIGHTS_USE_LPM = 1,
+};
+
+int ps3_lpm_open(void *tb_cache, u64 tb_cache_size, u64 tb_type);
+int ps3_lpm_close(void);
+void ps3_set_bookmark(u64 bookmark);
+void ps3_set_pm_bookmark(u64 tag, u64 incident, u64 th_id);
+u64 ps3_copy_trace_buffer(u64 offset, u64 size, void *to, int to_user);
+int ps3_set_signal(u64 rtas_signal_group, u8 signal_bit, u16 sub_unit,
+	u8 bus_word);
+
+u32 ps3_read_phys_ctr(u32 cpu, u32 phys_ctr);
+void ps3_write_phys_ctr(u32 cpu, u32 phys_ctr, u32 val);
+u32 ps3_read_ctr(u32 cpu, u32 ctr);
+void ps3_write_ctr(u32 cpu, u32 ctr, u32 val);
+
+u32 ps3_read_pm07_control(u32 cpu, u32 ctr);
+void ps3_write_pm07_control(u32 cpu, u32 ctr, u32 val);
+u32 ps3_read_pm(u32 cpu, enum pm_reg_name reg);
+void ps3_write_pm(u32 cpu, enum pm_reg_name reg, u32 val);
+
+u32 ps3_get_ctr_size(u32 cpu, u32 phys_ctr);
+void ps3_set_ctr_size(u32 cpu, u32 phys_ctr, u32 ctr_size);
+
+void ps3_enable_pm(u32 cpu);
+void ps3_disable_pm(u32 cpu);
+void ps3_enable_pm_interrupts(u32 cpu, u32 thread, u32 mask);
+void ps3_disable_pm_interrupts(u32 cpu);
+
+u32 ps3_get_and_clear_pm_interrupts(u32 cpu);
+void ps3_sync_irq(int node);
+u32 ps3_get_hw_thread_id(int cpu);
+u64 ps3_get_spe_id(void *arg);
 
 #endif

-- 

Re: [patch 2/3] PS3: Add logical performance monitor device support

[Arnd Bergmann]

T24gU2F0dXJkYXkgMDUgSmFudWFyeSAyMDA4LCBHZW9mZiBMZXZhbmQgd3JvdGU6Cj4gKyAgICAg
ICBzdHJ1Y3QgbGF5b3V0IHsKPiArICAgICAgICAgICAgICAgc3RydWN0IHBzM19zeXN0ZW1fYnVz
X2RldmljZSBkZXY7Cj4gKyAgICAgICB9ICpwOwoKV2hhdCdzIHRoZSBwb2ludCBvZiB0aGlzIGRh
dGEgc3RydWN0dXJlPyBZb3UgZG9uJ3QgdXNlIHRoZQpzdHJ1Y3QgYW55d2hlcmUsIGFuZCBpdCBv
bmx5IGhhcyBvbmUgbWVtYmVyLCBzbyB5b3UgY291bGQKanVzdCBkZWNsYXJlIHRoYXQgZGlyZWN0
bHkuCgo+ICugoKCgoKCgaWYgKHRtcDEgIT0gdG1wMikgewo+ICugoKCgoKCgoKCgoKCgoKBwcl9k
ZWJ1ZygiJXM6JWQ6IHdyb25nIGxwYXJcbiIsCj4gK6CgoKCgoKCgoKCgoKCgoKCgoKCgoKCgX19m
dW5jX18sIF9fTElORV9fKTsKPiAroKCgoKCgoKCgoKCgoKCgcmVzdWx0ID0gLTE7Cj4gK6CgoKCg
oKCgoKCgoKCgoGdvdG8gZmFpbF9yaWdodHM7Cj4gK6CgoKCgoKB9Cj4gKwo+ICugoKCgoKCgaWYg
KCEocC0+ZGV2LmxwbS5yaWdodHMgJiBQUzNfTFBNX1JJR0hUU19VU0VfTFBNKSkgewo+ICugoKCg
oKCgoKCgoKCgoKBwcl9kZWJ1ZygiJXM6JWQ6IGRvbid0IGhhdmUgcmlnaHRzIHRvIHVzZSBscG1c
biIsCj4gK6CgoKCgoKCgoKCgoKCgoKCgoKCgoKCgX19mdW5jX18sIF9fTElORV9fKTsKPiAroKCg
oKCgoKCgoKCgoKCgcmVzdWx0ID0gLTE7Cj4gK6CgoKCgoKCgoKCgoKCgoGdvdG8gZmFpbF9yaWdo
dHM7Cj4gK6CgoKCgoKB9Cj4gKwoKSSB0aGluayBfX2luaXQgZnVuY3Rpb25zIHNob3VsZCByZXR1
cm4gZXJyb3IgY29kZXMgbGlrZSAtRVBFUk0gb3IKLUVJTlZBTCwgbm90IG51bWVyaWMgLTEuCgpB
cGFydCBmcm9tIHRoYXQsIHRoZSBwYXRjaCBsb29rcyBnb29kLgoKCUFybmQgPD48Cg==

Re: [patch 3/3] PS3: Add logical performance monitor driver support

[Arnd Bergmann]

On Saturday 05 January 2008, Geoff Levand wrote:
> From: Takashi Yamamoto <Takashi02_Yamamoto@hq.scei.sony.co.jp>
> 
> Add PS3 logical performance monitor (lpm) device driver.
> 
> The PS3's LV1 hypervisor provides a Logical Performance Monitor that
> abstarcts the Cell processor's performance monitor features for use
> by guest operating systems.
> 
> Signed-off-by: Takashi Yamamoto <Takashi02_Yamamoto@hq.scei.sony.co.jp>
> Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>

Wow, great stuff!

One bit of information I'm missing in the description is what the users
of this code are. Your Kconfig text mentions that can be used for both
perfmon and oprofile. Are these, or one of them, currently under
work as well?

> --- a/arch/powerpc/platforms/ps3/Kconfig
> +++ b/arch/powerpc/platforms/ps3/Kconfig
> @@ -138,4 +138,18 @@ config PS3_FLASH
>  	  be disabled on the kernel command line using "ps3flash=off", to
>  	  not allocate this fixed buffer.
>  
> +config PS3_LPM
> +	tristate "PS3 Logical Performance Monitor support"
> +	depends on PPC_PS3
> +	default n

'default n' is redundant.

> +	help
> +	  Include support for the PS3 Logical Performance Monitor.
> +
> +	  This support is required to use the logical performance monitor
> +	  of the PS3's LV1 hypervisor.
> +
> +	  If you intend to use the advanced performance monitoring and
> +	  profiling support of the Cell processor with programs like
> +	  oprofile and perfmon, then say Y or M, otherwise say N.
> +

Should that be perfmon2 instead of perfmon?

I think once they high-level drivers are merged, it would make sense
to autoselected by these, so the user doesn't have to read through
the help text.

> +/**
> + * struct ps3_lpm_priv - private lpm device data.
> + *
> + * @mutex: Open/close mutex.
> + * @rights: The lpm rigths granted by the system policy module.
> + * @pu_id: The lv1 id of the BE prosessor for this lpm instance.
> + * @lpm_id: The lv1 id of this lpm instance.
> + * @outlet_id: The outlet created by lv1 for this lpm instance.
> + * @constructed: A flag indicating the lpm driver has been opened -- can we just use (lpm_id == ???)
> + * @tb_size: The lv1 trace buffer size
> + * @tb_cache: Trace buffer cache
> + * @tb_cache_internal: A flag indicating the trace buffer cache was allocated
> + *                     by the driver.
> + * @tb_cache: Trace buffer cache
> + * @sizeof_traced_data: Traced data size
> + * @sbd: the struct ps3_system_bus_device attached to this driver
> + */
> +
> +struct ps3_lpm_priv {
> +	struct mutex mutex;
> +	u64 rights;
> +	u64 pu_id;
> +	u64 lpm_id;
> +	u64 outlet_id;
> +	int constructed;
> +	u64 tb_size;
> +	void *tb_cache;
> +	u64 tb_cache_size;
> +	int tb_cache_internal;
> +	u64 sizeof_traced_data;
> +	u64 sizeof_total_copied_data;
> +	u64 shadow_pm_control;
> +	u64 shadow_pm_start_stop;
> +	u64 shadow_pm_interval;
> +	u64 shadow_group_control;
> +	u64 shadow_debug_bus_control;
> +	struct ps3_system_bus_device *sbd;
> +};

Some of the members in this struct are not present in the comment,
which is somewhat confusing.

> +
> +/**
> + * lpm_priv - Static instance of the lpm data.
> + *
> + * Since the exported routines don't support the notion of a device
> + * instance we need to hold the instance in this static variable
> + * and only allow at most one instance to be created.
> + */
> +
> +static struct ps3_lpm_priv *lpm_priv;

Hmmm, my gut feeling about this is that it would be better to actually
pass this around through the exported functions. That would also
provide some arbitration between the possible users, e.g. oprofile
would get -EBUSY when asking for the ps3_lpm while perfmon2 is
already active.

> +/* bookmark spr address */
> +#define BOOKMARK_SPR_ADDR 1020

This one belongs to asm/reg.h and should be named SRPN_BKMK.

> +inline u32 ps3_get_hw_thread_id(int cpu)
> +{
> +	return cpu;
> +}
> +EXPORT_SYMBOL_GPL(ps3_get_hw_thread_id);

The 'inline' is bogus, as that doesn't work across exported functions
and you are not using the function in this file.

Should you perhaps return hard_smp_processor_id(cpu) instead of cpu
here?

> +/**
> + * _ps3_copy_trace_buffer - Copy the trace buffer.
> + */
> +
> +static u64 _ps3_copy_trace_buffer(u64 offset, u64 size, u64 *to, int to_user)
> +{
> +	int result;
> +	u64 sizeof_copied_data;
> +
> +	if (offset >= lpm_priv->sizeof_traced_data)
> +		return 0;
> +
> +	result = lv1_copy_lpm_trace_buffer(lpm_priv->lpm_id, offset, size,
> +					&sizeof_copied_data);
> +	if (result) {
> +		dev_err(sbd_core(), "%s:%u: lv1_copy_lpm_trace_buffer failed: "
> +			"offset 0x%lx, size 0x%lx: %s\n", __func__, __LINE__,
> +			offset, size, ps3_result(result));
> +		return 0;
> +	}
> +
> +	if (to_user) {
> +		result = copy_to_user((void __user *)to, lpm_priv->tb_cache,
> +				      sizeof_copied_data);
> +		if (result) {
> +			dev_err(sbd_core(), "%s:%u: copy_to_user() error. "
> +				"offset:0x%lx size:0x%lx dest:0x%p src:0x%p\n",
> +				__func__, __LINE__, offset, sizeof_copied_data,
> +				to, lpm_priv->tb_cache);
> +			return 0;
> +		}
> +	} else
> +		memcpy(to, lpm_priv->tb_cache, sizeof_copied_data);
> +
> +	return sizeof_copied_data;
> +}

I don't like how this functions uses the same argument for kernel and user
pointers, this will cause sparse warnings in the code using it. How about
making this two separate functions?

> +int ps3_lpm_open(void *tb_cache, u64 tb_cache_size, u64 tb_type)
> +{
> +	int result;
> +	u64 cbe_node_id;
> +	u64 tb_size;
> +	u64 ctrl_opt;
> +	u64 tb_cache_lpar_addr;
> +	u64 lpm_id;
> +	u64 outlet_id;
> +	u64 used_tb_size;
> +
> +	if (!lpm_priv) {
> +		BUG();
> +		return -ENODEV;
> +	}
> +
> +	mutex_lock(&lpm_priv->mutex);

This mutex is documented as the 'open/close' mutex and you use it
to prevent concurrent execution of the open and close functions.
However, it's unclear what data structures are actually protected
by it.

I think this is the result of unusual lifetime rules for the lpm_priv
object, which I already mentioned above.

	Arnd <><

Re: [patch 2/3] PS3: Add logical performance monitor device support

[Geoff Levand]

On 01/05/2008 03:29 AM, Arnd Bergmann wrote:
> On Saturday 05 January 2008, Geoff Levand wrote:
>> +       struct layout {
>> +               struct ps3_system_bus_device dev;
>> +       } *p;
> 
> What's the point of this data structure? You don't use the
> struct anywhere, and it only has one member, so you could
> just declare that directly.

Yes, this code was just cut and pasted from a device that
had more members.

>> +       if (tmp1 != tmp2) {
>> +               pr_debug("%s:%d: wrong lpar\n",
>> +                       __func__, __LINE__);
>> +               result = -1;
>> +               goto fail_rights;
>> +       }
>> +
>> +       if (!(p->dev.lpm.rights & PS3_LPM_RIGHTS_USE_LPM)) {
>> +               pr_debug("%s:%d: don't have rights to use lpm\n",
>> +                       __func__, __LINE__);
>> +               result = -1;
>> +               goto fail_rights;
>> +       }
>> +
> 
> I think __init functions should return error codes like -EPERM or
> -EINVAL, not numeric -1.

I'll fix it in the next post.

-Geoff

Re: [patch 3/3] PS3: Add logical performance monitor driver support

[Geoff Levand]

On 01/05/2008 03:56 AM, Arnd Bergmann wrote:
> On Saturday 05 January 2008, Geoff Levand wrote:
>> From: Takashi Yamamoto <Takashi02_Yamamoto@hq.scei.sony.co.jp>
>> 
>> Add PS3 logical performance monitor (lpm) device driver.
>> 
>> The PS3's LV1 hypervisor provides a Logical Performance Monitor that
>> abstarcts the Cell processor's performance monitor features for use
>> by guest operating systems.
>> 
>> Signed-off-by: Takashi Yamamoto <Takashi02_Yamamoto@hq.scei.sony.co.jp>
>> Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
> 
> Wow, great stuff!
> 
> One bit of information I'm missing in the description is what the users
> of this code are. Your Kconfig text mentions that can be used for both
> perfmon and oprofile. Are these, or one of them, currently under
> work as well?

Yes, perfmon2 support is somewhat working, but still under development.
I added the perfmon2 patches into ps3-linux.git, so the easy way would
be to use those kernel sources.  This info written before I merged
perfmon2 into ps3-linux.git, but does give some info on how to use
it:

  http://www.kernel.org/pub/linux/kernel/people/geoff/cell/CELL-Linux-CL_20071220-ADDON/doc/ApplicationProgrammingEnvironment.html

I just started to work on the kernel oprofile support, but it is not
usable yet.  Any contributions would be welcome.  Currently the patch
is just something hacked up to use for testing:

  http://git.kernel.org/?p=linux/kernel/git/geoff/ps3-linux-patches.git;a=blob;f=ps3-wip/ps3-oprofile.patch;hb=HEAD

>> --- a/arch/powerpc/platforms/ps3/Kconfig
>> +++ b/arch/powerpc/platforms/ps3/Kconfig
>> @@ -138,4 +138,18 @@ config PS3_FLASH
>>  	  be disabled on the kernel command line using "ps3flash=off", to
>>  	  not allocate this fixed buffer.
>>  
>> +config PS3_LPM
>> +	tristate "PS3 Logical Performance Monitor support"
>> +	depends on PPC_PS3
>> +	default n
> 
> 'default n' is redundant.
> 
>> +	help
>> +	  Include support for the PS3 Logical Performance Monitor.
>> +
>> +	  This support is required to use the logical performance monitor
>> +	  of the PS3's LV1 hypervisor.
>> +
>> +	  If you intend to use the advanced performance monitoring and
>> +	  profiling support of the Cell processor with programs like
>> +	  oprofile and perfmon, then say Y or M, otherwise say N.
>> +
> 
> Should that be perfmon2 instead of perfmon?

Yes.

> I think once they high-level drivers are merged, it would make sense
> to autoselected by these, so the user doesn't have to read through
> the help text.

Yes, many things can be improved.

>> +/**
>> + * struct ps3_lpm_priv - private lpm device data.
>> + *
>> + * @mutex: Open/close mutex.
>> + * @rights: The lpm rigths granted by the system policy module.
>> + * @pu_id: The lv1 id of the BE prosessor for this lpm instance.
>> + * @lpm_id: The lv1 id of this lpm instance.
>> + * @outlet_id: The outlet created by lv1 for this lpm instance.
>> + * @constructed: A flag indicating the lpm driver has been opened -- can we just use (lpm_id == ???)
>> + * @tb_size: The lv1 trace buffer size
>> + * @tb_cache: Trace buffer cache
>> + * @tb_cache_internal: A flag indicating the trace buffer cache was allocated
>> + *                     by the driver.
>> + * @tb_cache: Trace buffer cache
>> + * @sizeof_traced_data: Traced data size
>> + * @sbd: the struct ps3_system_bus_device attached to this driver
>> + */
>> +
>> +struct ps3_lpm_priv {
>> +	struct mutex mutex;
>> +	u64 rights;
>> +	u64 pu_id;
>> +	u64 lpm_id;
>> +	u64 outlet_id;
>> +	int constructed;
>> +	u64 tb_size;
>> +	void *tb_cache;
>> +	u64 tb_cache_size;
>> +	int tb_cache_internal;
>> +	u64 sizeof_traced_data;
>> +	u64 sizeof_total_copied_data;
>> +	u64 shadow_pm_control;
>> +	u64 shadow_pm_start_stop;
>> +	u64 shadow_pm_interval;
>> +	u64 shadow_group_control;
>> +	u64 shadow_debug_bus_control;
>> +	struct ps3_system_bus_device *sbd;
>> +};
> 
> Some of the members in this struct are not present in the comment,
> which is somewhat confusing.

I'll add those in for the next post.

>> +
>> +/**
>> + * lpm_priv - Static instance of the lpm data.
>> + *
>> + * Since the exported routines don't support the notion of a device
>> + * instance we need to hold the instance in this static variable
>> + * and only allow at most one instance to be created.
>> + */
>> +
>> +static struct ps3_lpm_priv *lpm_priv;
> 
> Hmmm, my gut feeling about this is that it would be better to actually
> pass this around through the exported functions. 

Yes, I feel the same way.  In general, I think we need to make a
better platform abstraction of the processor's pm functions.  That
would include support for platform specific instance data, RTAS
for blade, lpm for ps3.

> That would also
> provide some arbitration between the possible users, e.g. oprofile
> would get -EBUSY when asking for the ps3_lpm while perfmon2 is
> already active.

That already happens in ps3_lpm_open(), using the lpm_priv.constructed
variable.

>> +inline u32 ps3_get_hw_thread_id(int cpu)
>> +{
>> +	return cpu;
>> +}
>> +EXPORT_SYMBOL_GPL(ps3_get_hw_thread_id);
> 
> The 'inline' is bogus, as that doesn't work across exported functions
> and you are not using the function in this file.
> 
> Should you perhaps return hard_smp_processor_id(cpu) instead of cpu
> here?

Yes, good point.

>> +/**
>> + * _ps3_copy_trace_buffer - Copy the trace buffer.
>> + */
>> +
>> +static u64 _ps3_copy_trace_buffer(u64 offset, u64 size, u64 *to, int to_user)
>> +{
>> +	int result;
>> +	u64 sizeof_copied_data;
>> +
>> +	if (offset >= lpm_priv->sizeof_traced_data)
>> +		return 0;
>> +
>> +	result = lv1_copy_lpm_trace_buffer(lpm_priv->lpm_id, offset, size,
>> +					&sizeof_copied_data);
>> +	if (result) {
>> +		dev_err(sbd_core(), "%s:%u: lv1_copy_lpm_trace_buffer failed: "
>> +			"offset 0x%lx, size 0x%lx: %s\n", __func__, __LINE__,
>> +			offset, size, ps3_result(result));
>> +		return 0;
>> +	}
>> +
>> +	if (to_user) {
>> +		result = copy_to_user((void __user *)to, lpm_priv->tb_cache,
>> +				      sizeof_copied_data);
>> +		if (result) {
>> +			dev_err(sbd_core(), "%s:%u: copy_to_user() error. "
>> +				"offset:0x%lx size:0x%lx dest:0x%p src:0x%p\n",
>> +				__func__, __LINE__, offset, sizeof_copied_data,
>> +				to, lpm_priv->tb_cache);
>> +			return 0;
>> +		}
>> +	} else
>> +		memcpy(to, lpm_priv->tb_cache, sizeof_copied_data);
>> +
>> +	return sizeof_copied_data;
>> +}
> 
> I don't like how this functions uses the same argument for kernel and user
> pointers, this will cause sparse warnings in the code using it. How about
> making this two separate functions?

OK, good point, I'll consider it.

>> +int ps3_lpm_open(void *tb_cache, u64 tb_cache_size, u64 tb_type)
>> +{
>> +	int result;
>> +	u64 cbe_node_id;
>> +	u64 tb_size;
>> +	u64 ctrl_opt;
>> +	u64 tb_cache_lpar_addr;
>> +	u64 lpm_id;
>> +	u64 outlet_id;
>> +	u64 used_tb_size;
>> +
>> +	if (!lpm_priv) {
>> +		BUG();
>> +		return -ENODEV;
>> +	}
>> +
>> +	mutex_lock(&lpm_priv->mutex);
> 
> This mutex is documented as the 'open/close' mutex and you use it
> to prevent concurrent execution of the open and close functions.
> However, it's unclear what data structures are actually protected
> by it.
> 
> I think this is the result of unusual lifetime rules for the lpm_priv
> object, which I already mentioned above.

Yes, we really just need to use the mutex to allow only a single instance,
and so do a mutex_trylock() in ps3_lpm_open() and return -EBUSY on fail,
and then a mutex_unlock() in ps3_lpm_close().

-Geoff

Re: [patch 3/3] PS3: Add logical performance monitor driver support

[Arnd Bergmann]

On Sunday 06 January 2008, Geoff Levand wrote:
> 
> > This mutex is documented as the 'open/close' mutex and you use it
> > to prevent concurrent execution of the open and close functions.
> > However, it's unclear what data structures are actually protected
> > by it.
> > 
> > I think this is the result of unusual lifetime rules for the lpm_priv
> > object, which I already mentioned above.
> 
> Yes, we really just need to use the mutex to allow only a single instance,
> and so do a mutex_trylock() in ps3_lpm_open() and return -EBUSY on fail,
> and then a mutex_unlock() in ps3_lpm_close().

A simple atomic_test_and_set in open() should do the job just as well then.

	Arnd <><