All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] oprofile support for ppc750
@ 2009-01-06 12:55 Octavian Purdila
  2009-01-06 12:55 ` [PATCH 1/2] powerpc: G4 oprofile: variable number of counters Octavian Purdila
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Octavian Purdila @ 2009-01-06 12:55 UTC (permalink / raw)
  To: linuxppc-dev


Adds oprofile support for ppc750. ppc750 performance counters
interface is similar with G4, but it only has 4 performance counters
instead of 6.

I tested this on a PPC750GX and it seems to work fine. 

tavi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/2] powerpc: G4 oprofile: variable number of counters
  2009-01-06 12:55 [PATCH 0/2] oprofile support for ppc750 Octavian Purdila
@ 2009-01-06 12:55 ` Octavian Purdila
  2009-02-04  5:22   ` Benjamin Herrenschmidt
  2009-01-06 12:55 ` [PATCH " Octavian Purdila
  2009-01-07  2:52 ` [PATCH 0/2] oprofile support for ppc750 Benjamin Herrenschmidt
  2 siblings, 1 reply; 14+ messages in thread
From: Octavian Purdila @ 2009-01-06 12:55 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Octavian Purdila

For ppc750 processors which use 4 performance counters instead of the
6 G4 uses but otherwise is compatible with G4.

Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
---
 arch/powerpc/oprofile/op_model_7450.c |   21 +++++++++++----------
 1 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/oprofile/op_model_7450.c b/arch/powerpc/oprofile/op_model_7450.c
index cc599eb..97348f5 100644
--- a/arch/powerpc/oprofile/op_model_7450.c
+++ b/arch/powerpc/oprofile/op_model_7450.c
@@ -29,7 +29,7 @@
 static unsigned long reset_value[OP_MAX_COUNTER];
 
 static int oprofile_running;
-static u32 mmcr0_val, mmcr1_val, mmcr2_val;
+static u32 mmcr0_val, mmcr1_val, mmcr2_val, ctrs;
 
 #define MMCR0_PMC1_SHIFT	6
 #define MMCR0_PMC2_SHIFT	0
@@ -88,13 +88,12 @@ static int fsl7450_cpu_setup(struct op_counter_config *ctr)
 
 	mtspr(SPRN_MMCR0, mmcr0_val);
 	mtspr(SPRN_MMCR1, mmcr1_val);
-	mtspr(SPRN_MMCR2, mmcr2_val);
+	if (ctrs > 4)
+		mtspr(SPRN_MMCR2, mmcr2_val);
 
 	return 0;
 }
 
-#define NUM_CTRS 6
-
 /* Configures the global settings for the countes on all CPUs. */
 static int fsl7450_reg_setup(struct op_counter_config *ctr,
 			     struct op_system_config *sys,
@@ -102,12 +101,13 @@ static int fsl7450_reg_setup(struct op_counter_config *ctr,
 {
 	int i;
 
+	ctrs = num_ctrs;
 	/* Our counters count up, and "count" refers to
 	 * how much before the next interrupt, and we interrupt
 	 * on overflow.  So we calculate the starting value
 	 * which will give us "count" until overflow.
 	 * Then we set the events on the enabled counters */
-	for (i = 0; i < NUM_CTRS; ++i)
+	for (i = 0; i < num_ctrs; ++i)
 		reset_value[i] = 0x80000000UL - ctr[i].count;
 
 	/* Set events for Counters 1 & 2 */
@@ -123,9 +123,10 @@ static int fsl7450_reg_setup(struct op_counter_config *ctr,
 
 	/* Set events for Counters 3-6 */
 	mmcr1_val = mmcr1_event3(ctr[2].event)
-		| mmcr1_event4(ctr[3].event)
-		| mmcr1_event5(ctr[4].event)
-		| mmcr1_event6(ctr[5].event);
+		| mmcr1_event4(ctr[3].event);
+	if (num_ctrs > 4)
+		mmcr1_val |= mmcr1_event5(ctr[4].event)
+			| mmcr1_event6(ctr[5].event);
 
 	mmcr2_val = 0;
 
@@ -139,7 +140,7 @@ static int fsl7450_start(struct op_counter_config *ctr)
 
 	mtmsr(mfmsr() | MSR_PMM);
 
-	for (i = 0; i < NUM_CTRS; ++i) {
+	for (i = 0; i < ctrs; ++i) {
 		if (ctr[i].enabled)
 			classic_ctr_write(i, reset_value[i]);
 		else
@@ -184,7 +185,7 @@ static void fsl7450_handle_interrupt(struct pt_regs *regs,
 	pc = mfspr(SPRN_SIAR);
 	is_kernel = is_kernel_addr(pc);
 
-	for (i = 0; i < NUM_CTRS; ++i) {
+	for (i = 0; i < ctrs; ++i) {
 		val = classic_ctr_read(i);
 		if (val < 0) {
 			if (oprofile_running && ctr[i].enabled) {
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] powerpc: oprofile: enable support for ppc750 processors
  2009-01-06 12:55 [PATCH 0/2] oprofile support for ppc750 Octavian Purdila
  2009-01-06 12:55 ` [PATCH 1/2] powerpc: G4 oprofile: variable number of counters Octavian Purdila
@ 2009-01-06 12:55 ` Octavian Purdila
  2009-03-04  4:47   ` Benjamin Herrenschmidt
  2009-01-07  2:52 ` [PATCH 0/2] oprofile support for ppc750 Benjamin Herrenschmidt
  2 siblings, 1 reply; 14+ messages in thread
From: Octavian Purdila @ 2009-01-06 12:55 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Octavian Purdila

Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
---
 arch/powerpc/kernel/cputable.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 923f87a..4e20cfb 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -726,6 +726,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.cpu_setup		= __setup_cpu_750,
 		.machine_check		= machine_check_generic,
 		.platform		= "ppc750",
+		.oprofile_cpu_type      = "ppc/7450",
+		.oprofile_type		= PPC_OPROFILE_G4,
 	},
 	{	/* 750FX rev 2.0 must disable HID0[DPM] */
 		.pvr_mask		= 0xffffffff,
@@ -741,6 +743,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.cpu_setup		= __setup_cpu_750,
 		.machine_check		= machine_check_generic,
 		.platform		= "ppc750",
+		.oprofile_cpu_type      = "ppc/7450",
+		.oprofile_type		= PPC_OPROFILE_G4,
 	},
 	{	/* 750FX (All revs except 2.0) */
 		.pvr_mask		= 0xffff0000,
@@ -756,6 +760,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.cpu_setup		= __setup_cpu_750fx,
 		.machine_check		= machine_check_generic,
 		.platform		= "ppc750",
+		.oprofile_cpu_type      = "ppc/7450",
+		.oprofile_type		= PPC_OPROFILE_G4,
 	},
 	{	/* 750GX */
 		.pvr_mask		= 0xffff0000,
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] oprofile support for ppc750
  2009-01-06 12:55 [PATCH 0/2] oprofile support for ppc750 Octavian Purdila
  2009-01-06 12:55 ` [PATCH 1/2] powerpc: G4 oprofile: variable number of counters Octavian Purdila
  2009-01-06 12:55 ` [PATCH " Octavian Purdila
@ 2009-01-07  2:52 ` Benjamin Herrenschmidt
  2009-01-07 12:17   ` Octavian Purdila
  2 siblings, 1 reply; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2009-01-07  2:52 UTC (permalink / raw)
  To: Octavian Purdila; +Cc: linuxppc-dev

On Tue, 2009-01-06 at 14:55 +0200, Octavian Purdila wrote:
> Adds oprofile support for ppc750. ppc750 performance counters
> interface is similar with G4, but it only has 4 performance counters
> instead of 6.
> 
> I tested this on a PPC750GX and it seems to work fine. 

I'm not too sure about using the same model as 74xx... I have some
patches that haven't been released yet from Jack Miller to implement the
full OP support for 750 but they appear to cause crashes.

There is a known HW bug in various 750's that basically make the
performance interrupt unuseable. I -think- it may have been fixed in the
very latest revision of the 750CL though but I'm not sure.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] oprofile support for ppc750
  2009-01-07  2:52 ` [PATCH 0/2] oprofile support for ppc750 Benjamin Herrenschmidt
@ 2009-01-07 12:17   ` Octavian Purdila
  0 siblings, 0 replies; 14+ messages in thread
From: Octavian Purdila @ 2009-01-07 12:17 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev

=46rom: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> On Tue, 2009-01-06 at 14:55 +0200, Octavian Purdila wrote:
> > Adds oprofile support for ppc750. ppc750 performance counters
> > interface is similar with G4, but it only has 4 performance counters
> > instead of 6.
> >
> > I tested this on a PPC750GX and it seems to work fine.
>
> I'm not too sure about using the same model as 74xx...=20

I've double checked the bits, and it looks to me that what is implemented i=
n=20
op_model_7450.c maps with my User=E2=80=99s Manual IBM PowerPC 750GX and 75=
0GL RISC=20
Microprocessor copy :)


> I have some
> patches that haven't been released yet from Jack Miller to implement the
> full OP support for 750 but they appear to cause crashes.
>

Were they posted somewhere?

> There is a known HW bug in various 750's that basically make the
> performance interrupt unuseable. I -think- it may have been fixed in the
> very latest revision of the 750CL though but I'm not sure.

Yeah, I've read that somewhere, but I did not notice any issues with 750GX =
I=20
am using - so far.

Thanks,
tavi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] powerpc: G4 oprofile: variable number of counters
  2009-01-06 12:55 ` [PATCH 1/2] powerpc: G4 oprofile: variable number of counters Octavian Purdila
@ 2009-02-04  5:22   ` Benjamin Herrenschmidt
  2009-02-24 12:09     ` Octavian Purdila
                       ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2009-02-04  5:22 UTC (permalink / raw)
  To: Octavian Purdila; +Cc: linuxppc-dev

On Tue, 2009-01-06 at 14:55 +0200, Octavian Purdila wrote:
> For ppc750 processors which use 4 performance counters instead of the
> 6 G4 uses but otherwise is compatible with G4.
> 
> Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
> ---
>  arch/powerpc/oprofile/op_model_7450.c |   21 +++++++++++----------
>  1 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/oprofile/op_model_7450.c b/arch/powerpc/oprofile/op_model_7450.c
> index cc599eb..97348f5 100644
> --- a/arch/powerpc/oprofile/op_model_7450.c
> +++ b/arch/powerpc/oprofile/op_model_7450.c
> @@ -29,7 +29,7 @@
>  static unsigned long reset_value[OP_MAX_COUNTER];
>  
>  static int oprofile_running;
> -static u32 mmcr0_val, mmcr1_val, mmcr2_val;
> +static u32 mmcr0_val, mmcr1_val, mmcr2_val, ctrs;

This may be static but it's still a global scope as far as kernel
symbols are concerned. Care to give it a slightly better name ? num_pmcs
would probably be already more telling.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] powerpc: G4 oprofile: variable number of counters
  2009-02-04  5:22   ` Benjamin Herrenschmidt
@ 2009-02-24 12:09     ` Octavian Purdila
  2009-02-24 12:09     ` [PATCH v2 " Octavian Purdila
  2009-02-24 12:09     ` [PATCH v2 2/2] powerpc: oprofile: enable support for ppc750 processors Octavian Purdila
  2 siblings, 0 replies; 14+ messages in thread
From: Octavian Purdila @ 2009-02-24 12:09 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev

>>  static int oprofile_running;
>> -static u32 mmcr0_val, mmcr1_val, mmcr2_val;
>> +static u32 mmcr0_val, mmcr1_val, mmcr2_val, ctrs;
>
>This may be static but it's still a global scope as far as kernel
>symbols are concerned. Care to give it a slightly better name ? num_pmcs
>would probably be already more telling.

Sure. v2 will follow, with s/ctrs/num_pmcs/.

Thanks,
tavi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/2] powerpc: G4 oprofile: variable number of counters
  2009-02-04  5:22   ` Benjamin Herrenschmidt
  2009-02-24 12:09     ` Octavian Purdila
@ 2009-02-24 12:09     ` Octavian Purdila
  2009-02-24 12:09     ` [PATCH v2 2/2] powerpc: oprofile: enable support for ppc750 processors Octavian Purdila
  2 siblings, 0 replies; 14+ messages in thread
From: Octavian Purdila @ 2009-02-24 12:09 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Octavian Purdila

For ppc750 processors which use 4 performance counters instead of the
6 G4 uses but otherwise is compatible with G4.

Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
---
 arch/powerpc/oprofile/op_model_7450.c |   21 +++++++++++----------
 1 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/oprofile/op_model_7450.c b/arch/powerpc/oprofile/op_model_7450.c
index cc599eb..f8d36f9 100644
--- a/arch/powerpc/oprofile/op_model_7450.c
+++ b/arch/powerpc/oprofile/op_model_7450.c
@@ -29,7 +29,7 @@
 static unsigned long reset_value[OP_MAX_COUNTER];
 
 static int oprofile_running;
-static u32 mmcr0_val, mmcr1_val, mmcr2_val;
+static u32 mmcr0_val, mmcr1_val, mmcr2_val, num_pmcs;
 
 #define MMCR0_PMC1_SHIFT	6
 #define MMCR0_PMC2_SHIFT	0
@@ -88,13 +88,12 @@ static int fsl7450_cpu_setup(struct op_counter_config *ctr)
 
 	mtspr(SPRN_MMCR0, mmcr0_val);
 	mtspr(SPRN_MMCR1, mmcr1_val);
-	mtspr(SPRN_MMCR2, mmcr2_val);
+	if (num_pmcs > 4)
+		mtspr(SPRN_MMCR2, mmcr2_val);
 
 	return 0;
 }
 
-#define NUM_CTRS 6
-
 /* Configures the global settings for the countes on all CPUs. */
 static int fsl7450_reg_setup(struct op_counter_config *ctr,
 			     struct op_system_config *sys,
@@ -102,12 +101,13 @@ static int fsl7450_reg_setup(struct op_counter_config *ctr,
 {
 	int i;
 
+	num_pmcs = num_ctrs;
 	/* Our counters count up, and "count" refers to
 	 * how much before the next interrupt, and we interrupt
 	 * on overflow.  So we calculate the starting value
 	 * which will give us "count" until overflow.
 	 * Then we set the events on the enabled counters */
-	for (i = 0; i < NUM_CTRS; ++i)
+	for (i = 0; i < num_ctrs; ++i)
 		reset_value[i] = 0x80000000UL - ctr[i].count;
 
 	/* Set events for Counters 1 & 2 */
@@ -123,9 +123,10 @@ static int fsl7450_reg_setup(struct op_counter_config *ctr,
 
 	/* Set events for Counters 3-6 */
 	mmcr1_val = mmcr1_event3(ctr[2].event)
-		| mmcr1_event4(ctr[3].event)
-		| mmcr1_event5(ctr[4].event)
-		| mmcr1_event6(ctr[5].event);
+		| mmcr1_event4(ctr[3].event);
+	if (num_ctrs > 4)
+		mmcr1_val |= mmcr1_event5(ctr[4].event)
+			| mmcr1_event6(ctr[5].event);
 
 	mmcr2_val = 0;
 
@@ -139,7 +140,7 @@ static int fsl7450_start(struct op_counter_config *ctr)
 
 	mtmsr(mfmsr() | MSR_PMM);
 
-	for (i = 0; i < NUM_CTRS; ++i) {
+	for (i = 0; i < num_pmcs; ++i) {
 		if (ctr[i].enabled)
 			classic_ctr_write(i, reset_value[i]);
 		else
@@ -184,7 +185,7 @@ static void fsl7450_handle_interrupt(struct pt_regs *regs,
 	pc = mfspr(SPRN_SIAR);
 	is_kernel = is_kernel_addr(pc);
 
-	for (i = 0; i < NUM_CTRS; ++i) {
+	for (i = 0; i < num_pmcs; ++i) {
 		val = classic_ctr_read(i);
 		if (val < 0) {
 			if (oprofile_running && ctr[i].enabled) {
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/2] powerpc: oprofile: enable support for ppc750 processors
  2009-02-04  5:22   ` Benjamin Herrenschmidt
  2009-02-24 12:09     ` Octavian Purdila
  2009-02-24 12:09     ` [PATCH v2 " Octavian Purdila
@ 2009-02-24 12:09     ` Octavian Purdila
  2009-02-26 22:30       ` Andy Fleming
  2 siblings, 1 reply; 14+ messages in thread
From: Octavian Purdila @ 2009-02-24 12:09 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Octavian Purdila

Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
---
 arch/powerpc/kernel/cputable.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 923f87a..4e20cfb 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -726,6 +726,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.cpu_setup		= __setup_cpu_750,
 		.machine_check		= machine_check_generic,
 		.platform		= "ppc750",
+		.oprofile_cpu_type      = "ppc/7450",
+		.oprofile_type		= PPC_OPROFILE_G4,
 	},
 	{	/* 750FX rev 2.0 must disable HID0[DPM] */
 		.pvr_mask		= 0xffffffff,
@@ -741,6 +743,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.cpu_setup		= __setup_cpu_750,
 		.machine_check		= machine_check_generic,
 		.platform		= "ppc750",
+		.oprofile_cpu_type      = "ppc/7450",
+		.oprofile_type		= PPC_OPROFILE_G4,
 	},
 	{	/* 750FX (All revs except 2.0) */
 		.pvr_mask		= 0xffff0000,
@@ -756,6 +760,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.cpu_setup		= __setup_cpu_750fx,
 		.machine_check		= machine_check_generic,
 		.platform		= "ppc750",
+		.oprofile_cpu_type      = "ppc/7450",
+		.oprofile_type		= PPC_OPROFILE_G4,
 	},
 	{	/* 750GX */
 		.pvr_mask		= 0xffff0000,
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/2] powerpc: oprofile: enable support for ppc750 processors
  2009-02-24 12:09     ` [PATCH v2 2/2] powerpc: oprofile: enable support for ppc750 processors Octavian Purdila
@ 2009-02-26 22:30       ` Andy Fleming
  2009-02-26 23:13         ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 14+ messages in thread
From: Andy Fleming @ 2009-02-26 22:30 UTC (permalink / raw)
  To: Octavian Purdila; +Cc: linuxppc-dev

On Tue, Feb 24, 2009 at 6:09 AM, Octavian Purdila <opurdila@ixiacom.com> wr=
ote:
> Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
> ---
> =A0arch/powerpc/kernel/cputable.c | =A0 =A06 ++++++
> =A01 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputabl=
e.c
> index 923f87a..4e20cfb 100644
> --- a/arch/powerpc/kernel/cputable.c
> +++ b/arch/powerpc/kernel/cputable.c
> @@ -726,6 +726,8 @@ static struct cpu_spec __initdata cpu_specs[] =3D {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0.cpu_setup =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D =
__setup_cpu_750,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0.machine_check =A0 =A0 =A0 =A0 =A0=3D mach=
ine_check_generic,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0.platform =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D =
"ppc750",
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 .oprofile_cpu_type =A0 =A0 =A0=3D "ppc/7450=
",
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 .oprofile_type =A0 =A0 =A0 =A0 =A0=3D PPC_O=
PROFILE_G4,
> =A0 =A0 =A0 =A0},


I know this saves you some code, but it seems hacky.  It would be
better to modify oprofile to detect the proper cpu type.  Also, this
will screw things up if you try to use the different event set that
the 750 has.

Also, one more concern is the long-standing errata which makes this
quite dangerous.  All of the versions of the 750 I'm aware of have a
bug where if a Performance Monitor exception occurs within one cycle
of the Decrementer exception, the cpu will lose the ability to return
from the interrupt (SRR0/SRR1 become corrupted).  It's possible the
750s you have modified to support oprofile don't have this errata.
Alternatively, we can decide we don't care, as you have to be root to
use oprofile.  But this is why I didn't add support for anything
before the 7450.

Andy

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/2] powerpc: oprofile: enable support for ppc750 processors
  2009-02-26 22:30       ` Andy Fleming
@ 2009-02-26 23:13         ` Benjamin Herrenschmidt
  2009-02-27 11:57           ` Octavian Purdila
  0 siblings, 1 reply; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2009-02-26 23:13 UTC (permalink / raw)
  To: Andy Fleming; +Cc: Octavian Purdila, John J Miller, linuxppc-dev

On Thu, 2009-02-26 at 16:30 -0600, Andy Fleming wrote:
> 
> I know this saves you some code, but it seems hacky.  It would be
> better to modify oprofile to detect the proper cpu type.  Also, this
> will screw things up if you try to use the different event set that
> the 750 has.

Agreed. Note that Jack Miller (CC) has some patches for the oprofile
side.

> Also, one more concern is the long-standing errata which makes this
> quite dangerous.  All of the versions of the 750 I'm aware of have a
> bug where if a Performance Monitor exception occurs within one cycle
> of the Decrementer exception, the cpu will lose the ability to return
> from the interrupt (SRR0/SRR1 become corrupted).  It's possible the
> 750s you have modified to support oprofile don't have this errata.
> Alternatively, we can decide we don't care, as you have to be root to
> use oprofile.  But this is why I didn't add support for anything
> before the 7450.

I think we need to advertise it as broken in some way... I -think- the
latest batch of IBM 750CL have that bug fixed but I'm not 100% certain.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/2] powerpc: oprofile: enable support for ppc750 processors
  2009-02-26 23:13         ` Benjamin Herrenschmidt
@ 2009-02-27 11:57           ` Octavian Purdila
  0 siblings, 0 replies; 14+ messages in thread
From: Octavian Purdila @ 2009-02-27 11:57 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Andy Fleming, John J Miller

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> On Thu, 2009-02-26 at 16:30 -0600, Andy Fleming wrote:
> > I know this saves you some code, but it seems hacky.  It would be
> > better to modify oprofile to detect the proper cpu type.  Also, this
> > will screw things up if you try to use the different event set that
> > the 750 has.
>
> Agreed. Note that Jack Miller (CC) has some patches for the oprofile
> side.
>

Not sure I understand, but the oprofile userspace  tool sees it as new 
processor (ppc750). I had to patch it to add the events for the new processor.

> > Also, one more concern is the long-standing errata which makes this
> > quite dangerous.  All of the versions of the 750 I'm aware of have a
> > bug where if a Performance Monitor exception occurs within one cycle
> > of the Decrementer exception, the cpu will lose the ability to return
> > from the interrupt (SRR0/SRR1 become corrupted).  It's possible the
> > 750s you have modified to support oprofile don't have this errata.
> > Alternatively, we can decide we don't care, as you have to be root to
> > use oprofile.  But this is why I didn't add support for anything
> > before the 7450.
>

Yes, I understand. We knew about the errata and we used the timer interrupt 
for profiling for some time, but then we run into some issue were we had to use 
advanced counters as L1 cache misses / TLB misses to diagnose some issues and 
we decided to give it a try. And _seems_ to work fine. Although we didn't 
stress it too hard (around 10-20 minutes of continuous run).

So maybe it would be useful for other people in the same situation as ours. 
BTW, this is the out of cat /proc/cpuinfo on our hw:

processor       : 0
cpu             : 750GX
temperature     : 10-12 C (uncalibrated)
revision        : 1.2 (pvr 7002 0102)
bogomips        : 1597.44
vendor          : Ixia
machine         : TCPX [0x6b]

 
> I think we need to advertise it as broken in some way... 

Perhaps set it to use the timer by default, and allow the user to switch to 
the hardware performance counter + printing a message that this mode is 
dangerous?

Thanks,
tavi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] powerpc: oprofile: enable support for ppc750 processors
  2009-01-06 12:55 ` [PATCH " Octavian Purdila
@ 2009-03-04  4:47   ` Benjamin Herrenschmidt
  2009-03-04 12:02     ` Octavian Purdila
  0 siblings, 1 reply; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2009-03-04  4:47 UTC (permalink / raw)
  To: Octavian Purdila; +Cc: linuxppc-dev

On Tue, 2009-01-06 at 14:55 +0200, Octavian Purdila wrote:
> Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>


So I'm going to merge 1/2 but this one should really be changed to
advertise ppc/750 in oprofile_cpu_type (ie. to userspace).

Cheers,
Ben.

>  arch/powerpc/kernel/cputable.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
> index 923f87a..4e20cfb 100644
> --- a/arch/powerpc/kernel/cputable.c
> +++ b/arch/powerpc/kernel/cputable.c
> @@ -726,6 +726,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
>  		.cpu_setup		= __setup_cpu_750,
>  		.machine_check		= machine_check_generic,
>  		.platform		= "ppc750",
> +		.oprofile_cpu_type      = "ppc/7450",
> +		.oprofile_type		= PPC_OPROFILE_G4,
>  	},
>  	{	/* 750FX rev 2.0 must disable HID0[DPM] */
>  		.pvr_mask		= 0xffffffff,
> @@ -741,6 +743,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
>  		.cpu_setup		= __setup_cpu_750,
>  		.machine_check		= machine_check_generic,
>  		.platform		= "ppc750",
> +		.oprofile_cpu_type      = "ppc/7450",
> +		.oprofile_type		= PPC_OPROFILE_G4,
>  	},
>  	{	/* 750FX (All revs except 2.0) */
>  		.pvr_mask		= 0xffff0000,
> @@ -756,6 +760,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
>  		.cpu_setup		= __setup_cpu_750fx,
>  		.machine_check		= machine_check_generic,
>  		.platform		= "ppc750",
> +		.oprofile_cpu_type      = "ppc/7450",
> +		.oprofile_type		= PPC_OPROFILE_G4,
>  	},
>  	{	/* 750GX */
>  		.pvr_mask		= 0xffff0000,

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] powerpc: oprofile: enable support for ppc750 processors
  2009-03-04  4:47   ` Benjamin Herrenschmidt
@ 2009-03-04 12:02     ` Octavian Purdila
  0 siblings, 0 replies; 14+ messages in thread
From: Octavian Purdila @ 2009-03-04 12:02 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> On Tue, 2009-01-06 at 14:55 +0200, Octavian Purdila wrote:
> > Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
>
> So I'm going to merge 1/2 but this one should really be changed to
> advertise ppc/750 in oprofile_cpu_type (ie. to userspace).
>

Sure. Here is the new patch which uses ppc/750. It enables oprofile for all 3 FX variants and GX as well.

Thanks!
tavi


commit 70f4865a614e9b0ff4594ebd52b95f78e998b79f
Author: Octavian Purdila <opurdila@ixiacom.com>
Date:   Tue Jan 6 12:51:43 2009 +0200

    powerpc: oprofile: enable support for ppc750 processors
    
    Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>

diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 923f87a..c3ea72b 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -726,6 +726,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.cpu_setup		= __setup_cpu_750,
 		.machine_check		= machine_check_generic,
 		.platform		= "ppc750",
+		.oprofile_cpu_type      = "ppc/750",
+		.oprofile_type		= PPC_OPROFILE_G4,
 	},
 	{	/* 750FX rev 2.0 must disable HID0[DPM] */
 		.pvr_mask		= 0xffffffff,
@@ -741,6 +743,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.cpu_setup		= __setup_cpu_750,
 		.machine_check		= machine_check_generic,
 		.platform		= "ppc750",
+		.oprofile_cpu_type      = "ppc/750",
+		.oprofile_type		= PPC_OPROFILE_G4,
 	},
 	{	/* 750FX (All revs except 2.0) */
 		.pvr_mask		= 0xffff0000,
@@ -756,6 +760,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.cpu_setup		= __setup_cpu_750fx,
 		.machine_check		= machine_check_generic,
 		.platform		= "ppc750",
+		.oprofile_cpu_type      = "ppc/750",
+		.oprofile_type		= PPC_OPROFILE_G4,
 	},
 	{	/* 750GX */
 		.pvr_mask		= 0xffff0000,
@@ -771,6 +777,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.cpu_setup		= __setup_cpu_750fx,
 		.machine_check		= machine_check_generic,
 		.platform		= "ppc750",
+		.oprofile_cpu_type      = "ppc/750",
+		.oprofile_type		= PPC_OPROFILE_G4,
 	},
 	{	/* 740/750 (L2CR bit need fixup for 740) */
 		.pvr_mask		= 0xffff0000,

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2009-03-04 12:03 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-01-06 12:55 [PATCH 0/2] oprofile support for ppc750 Octavian Purdila
2009-01-06 12:55 ` [PATCH 1/2] powerpc: G4 oprofile: variable number of counters Octavian Purdila
2009-02-04  5:22   ` Benjamin Herrenschmidt
2009-02-24 12:09     ` Octavian Purdila
2009-02-24 12:09     ` [PATCH v2 " Octavian Purdila
2009-02-24 12:09     ` [PATCH v2 2/2] powerpc: oprofile: enable support for ppc750 processors Octavian Purdila
2009-02-26 22:30       ` Andy Fleming
2009-02-26 23:13         ` Benjamin Herrenschmidt
2009-02-27 11:57           ` Octavian Purdila
2009-01-06 12:55 ` [PATCH " Octavian Purdila
2009-03-04  4:47   ` Benjamin Herrenschmidt
2009-03-04 12:02     ` Octavian Purdila
2009-01-07  2:52 ` [PATCH 0/2] oprofile support for ppc750 Benjamin Herrenschmidt
2009-01-07 12:17   ` Octavian Purdila

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.