LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] powerpc: disable MSI using new interface if possible
From: Nishanth Aravamudan @ 2011-03-04  1:41 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: paulus, linuxppc-dev, miltonm
In-Reply-To: <1299200729.3630.52.camel@concordia>

On 04.03.2011 [12:05:29 +1100], Michael Ellerman wrote:
> On Thu, 2011-03-03 at 11:39 -0800, Nishanth Aravamudan wrote:
> > On upcoming hardware, we have a PCI adapter with two functions, one of
> > which uses MSI and the other uses MSI-X. This adapter, when MSI is
> > disabled using the "old" firmware interface (RTAS_CHANGE_FN), still
> > signals an MSI-X interrupt and triggers an EEH. We are working with the
> > vendor to ensure that the hardware is not at fault, but if we use the
> > "new" interface (RTAS_CHANGE_MSI_FN) to disable MSI, we also
> > automatically disable MSI-X and the adapter does not appear to signal
> > any stray MSI-X interrupt.
> 
> It seems this could also be a firmware bug, have we heard anything from
> them? PAPR explicitly says that RTAS_CHANGE_FN (function=1) should
> disable MSI _and_ MSI-X (R1???7.3.10.5.1???1).

We're tracking that down too. I think the fact that the interrupt is
coming in is a hardware bug in this particular adapter.

I'm looking at PAPR again and I see what might be a contradiction:

7.3.10.5.1: "To removing all MSIs, set the Requested Number of
Interrupts to zero."

Table 71: "Function ... 1: Request to set to a new number of MSI or
MSI-X (platform choice) interrupts (including set to 0)"

It seems like the Table claims that using RTAS_CHANGE_FN with 0, could
change only MSI or MSI-X and still be not a bug?

It does seem like it should disable both, but from what we've seen with
this adapter, it doesn't appear to.

> > Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> > Cc: Milton Miller <miltonm@bga.com>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> 
> Cc: Me  :)

Sorry! I was in a hurry to get this out the door, my fault. Note, you
don't show up per scripts/get_maintainer.pl :)

> > diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
> > index 1164c34..9434576 100644
> > --- a/arch/powerpc/platforms/pseries/msi.c
> > +++ b/arch/powerpc/platforms/pseries/msi.c
> > @@ -93,8 +93,18 @@ static void rtas_disable_msi(struct pci_dev *pdev)
> >     if (!pdn)
> >             return;
> >  
> > -   if (rtas_change_msi(pdn, RTAS_CHANGE_FN, 0) != 0)
> > -           pr_debug("rtas_msi: Setting MSIs to 0 failed!\n");
> > +   /*
> > +    * disabling MSI with the explicit interface also disables MSI-X
> > +    */
> > +   if (rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, 0) != 0) {
> > +           /* 
> > +            * may have failed due to lacking
> > +            * "ibm,change-msix-capable" property
> > +            */
> > +           if (rtas_change_msi(pdn, RTAS_CHANGE_FN, 0) != 0) {
> > +                   pr_debug("rtas_msi: Setting MSIs to 0 failed!\n");
> > +           }
> > +   }
> >  }
> 
> This is probably a pretty safe change anyway, ie. use the newer API if
> it is present. The comment is backward though, the call fails because
> the new interface is not implemented, and that fact is signalled by the
> absence of the property.

Updated the comment, thanks:

On upcoming hardware, we have a PCI adapter with two functions, one of
which uses MSI and the other uses MSI-X. This adapter, when MSI is
disabled using the "old" firmware interface (RTAS_CHANGE_FN), still
signals an MSI-X interrupt and triggers an EEH. We are working with the
vendor to ensure that the hardware is not at fault, but if we use the
"new" interface (RTAS_CHANGE_MSI_FN) to disable MSI, we also
automatically disable MSI-X and the adapter does not appear to signal
any stray MSI-X interrupt.

Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
---

 arch/powerpc/platforms/pseries/msi.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index 1164c34..18ac801 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -93,8 +93,18 @@ static void rtas_disable_msi(struct pci_dev *pdev)
 	if (!pdn)
 		return;
 
-	if (rtas_change_msi(pdn, RTAS_CHANGE_FN, 0) != 0)
-		pr_debug("rtas_msi: Setting MSIs to 0 failed!\n");
+	/*
+	 * disabling MSI with the explicit interface also disables MSI-X
+	 */
+	if (rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, 0) != 0) {
+		/* 
+		 * may have failed because explicit interface is not
+		 * present
+		 */
+		if (rtas_change_msi(pdn, RTAS_CHANGE_FN, 0) != 0) {
+			pr_debug("rtas_msi: Setting MSIs to 0 failed!\n");
+		}
+	}
 }
 
 static int rtas_query_irq_number(struct pci_dn *pdn, int offset)
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH] powerpc: disable MSI using new interface if possible
From: Michael Ellerman @ 2011-03-04  3:01 UTC (permalink / raw)
  To: Nishanth Aravamudan; +Cc: paulus, linuxppc-dev, miltonm
In-Reply-To: <1299202862-10682-1-git-send-email-nacc@us.ibm.com>

[-- Attachment #1: Type: text/plain, Size: 2336 bytes --]

On Thu, 2011-03-03 at 17:41 -0800, Nishanth Aravamudan wrote:
> On 04.03.2011 [12:05:29 +1100], Michael Ellerman wrote:
> > On Thu, 2011-03-03 at 11:39 -0800, Nishanth Aravamudan wrote:
> > > On upcoming hardware, we have a PCI adapter with two functions, one of
> > > which uses MSI and the other uses MSI-X. This adapter, when MSI is
> > > disabled using the "old" firmware interface (RTAS_CHANGE_FN), still
> > > signals an MSI-X interrupt and triggers an EEH. We are working with the
> > > vendor to ensure that the hardware is not at fault, but if we use the
> > > "new" interface (RTAS_CHANGE_MSI_FN) to disable MSI, we also
> > > automatically disable MSI-X and the adapter does not appear to signal
> > > any stray MSI-X interrupt.
> > 
> > It seems this could also be a firmware bug, have we heard anything from
> > them? PAPR explicitly says that RTAS_CHANGE_FN (function=1) should
> > disable MSI _and_ MSI-X (R1???7.3.10.5.1???1).
> 
> We're tracking that down too. I think the fact that the interrupt is
> coming in is a hardware bug in this particular adapter.
> 
> I'm looking at PAPR again and I see what might be a contradiction:
> 
> 7.3.10.5.1: "To removing all MSIs, set the Requested Number of
> Interrupts to zero."
> 
> Table 71: "Function ... 1: Request to set to a new number of MSI or
> MSI-X (platform choice) interrupts (including set to 0)"
> 
> It seems like the Table claims that using RTAS_CHANGE_FN with 0, could
> change only MSI or MSI-X and still be not a bug?

Yeah I guess you could read it that way, though I think that would be a
bug.

The idea is that it chooses for you whether it uses MSI or MSI-X. So the
only sane semantic is that when deconfiguring it deconfigures either,
ie. both, kinds.

Looking closer at your patch, now I don't understand :)

+       /*
+        * disabling MSI with the explicit interface also disables MSI-X
+        */
+       if (rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, 0) != 0) {


So we first disable using function 3, which should:

        3: Request to set to a new number of MSI interrupts (including set to 0)

Which does not mention MSI-X at all, implying it has no effect on them.
Which contradicts what you see, and the comment in the code?

So I think I'm not sure what's going on here :)

cheers

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [PATCH] powerpc: disable MSI using new interface if possible
From: Michael Ellerman @ 2011-03-04  3:06 UTC (permalink / raw)
  To: Nishanth Aravamudan; +Cc: Joe Russell, paulus, linuxppc-dev, miltonm
In-Reply-To: <1299202862-10682-1-git-send-email-nacc@us.ibm.com>

[-- Attachment #1: Type: text/plain, Size: 1357 bytes --]

On Thu, 2011-03-03 at 17:41 -0800, Nishanth Aravamudan wrote:
> On 04.03.2011 [12:05:29 +1100], Michael Ellerman wrote:
> > On Thu, 2011-03-03 at 11:39 -0800, Nishanth Aravamudan wrote:
> > > On upcoming hardware, we have a PCI adapter with two functions, one of
..
> > > Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> > > Cc: Milton Miller <miltonm@bga.com>
> > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Cc: Paul Mackerras <paulus@samba.org>
> > 
> > Cc: Me  :)
> 
> Sorry! I was in a hurry to get this out the door, my fault. Note, you
> don't show up per scripts/get_maintainer.pl :)

No worries, though I will remember never to use get_maintainer.pl, it is
obviously utterly broken:

$ git log --pretty=format:"%an %ae" arch/powerpc/platforms/pseries/msi.c
Andre Detsch adetsch@br.ibm.com
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au
Michael Ellerman michael@ellerman.id.au

And the patch from Andre was also signed off by me.

cheers

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [PATCH] powerpc: disable MSI using new interface if possible
From: Joe Perches @ 2011-03-04  3:29 UTC (permalink / raw)
  To: michael; +Cc: Nishanth Aravamudan, linuxppc-dev, Florian Mickler
In-Reply-To: <1299208016.3630.81.camel@concordia>

On Fri, 2011-03-04 at 14:06 +1100, Michael Ellerman wrote:
> On Thu, 2011-03-03 at 17:41 -0800, Nishanth Aravamudan wrote:
> > On 04.03.2011 [12:05:29 +1100], Michael Ellerman wrote:
> > > Cc: Me  :)
> > Sorry! I was in a hurry to get this out the door, my fault. Note, you
> > don't show up per scripts/get_maintainer.pl :)
> No worries, though I will remember never to use get_maintainer.pl, it is
> obviously utterly broken:

Blah, blah, stupid tool doesn't work exactly as I want,
and it doesn't credit me for my over 2 year old patches,
therefore it's not only stupid, it's broken...

$ ./scripts/get_maintainer.pl -f arch/powerpc/platforms/pseries/msi.c
Benjamin Herrenschmidt <benh@kernel.crashing.org> (supporter:LINUX FOR POWERPC...)
Paul Mackerras <paulus@samba.org> (supporter:LINUX FOR POWERPC...)
Grant Likely <grant.likely@secretlab.ca> (maintainer:OPEN FIRMWARE AND...)
linuxppc-dev@lists.ozlabs.org (open list:LINUX FOR POWERPC...)
linux-kernel@vger.kernel.org (open list)
devicetree-discuss@lists.ozlabs.org (open list:OPEN FIRMWARE AND...)

If you don't like how it currently works, suggest
improvements.

> $ git log --pretty=format:"%an %ae" arch/powerpc/platforms/pseries/msi.c
> Andre Detsch adetsch@br.ibm.com
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> Michael Ellerman michael@ellerman.id.au
> 
> And the patch from Andre was also signed off by me.

How nice for you.  Last patch from you was
2 years ago.

$ git log --format="%an %ae %ad" arch/powerpc/platforms/pseries/msi.c
Andre Detsch adetsch@br.ibm.com Wed Nov 4 13:03:19 2009 -0200
Michael Ellerman michael@ellerman.id.au Thu Mar 5 14:44:26 2009 +0000
Michael Ellerman michael@ellerman.id.au Tue Feb 17 00:21:56 2009 +0000
Michael Ellerman michael@ellerman.id.au Tue Feb 17 00:18:49 2009 +0000
Michael Ellerman michael@ellerman.id.au Thu Jan 22 20:54:33 2009 +0000
Michael Ellerman michael@ellerman.id.au Thu Jan 22 20:54:32 2009 +0000
Michael Ellerman michael@ellerman.id.au Thu Jan 22 20:54:31 2009 +0000
Michael Ellerman michael@ellerman.id.au Thu Jan 22 20:54:31 2009 +0000
Michael Ellerman michael@ellerman.id.au Tue Oct 23 14:23:44 2007 +1000
Michael Ellerman michael@ellerman.id.au Thu Sep 20 16:36:50 2007 +1000
Michael Ellerman michael@ellerman.id.au Thu Sep 20 16:36:48 2007 +1000
Michael Ellerman michael@ellerman.id.au Tue May 8 12:58:35 2007 +1000

CC'ing inactive non named maintainers via git
history also draws complaints btw.

^ permalink raw reply

* Re: [PATCH] powerpc: disable MSI using new interface if possible
From: Nishanth Aravamudan @ 2011-03-04  7:24 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: paulus, linuxppc-dev, miltonm
In-Reply-To: <1299207684.3630.76.camel@concordia>

On 04.03.2011 [14:01:24 +1100], Michael Ellerman wrote:
> On Thu, 2011-03-03 at 17:41 -0800, Nishanth Aravamudan wrote:
> > On 04.03.2011 [12:05:29 +1100], Michael Ellerman wrote:
> > > On Thu, 2011-03-03 at 11:39 -0800, Nishanth Aravamudan wrote:
> > > > On upcoming hardware, we have a PCI adapter with two functions, one of
> > > > which uses MSI and the other uses MSI-X. This adapter, when MSI is
> > > > disabled using the "old" firmware interface (RTAS_CHANGE_FN), still
> > > > signals an MSI-X interrupt and triggers an EEH. We are working with the
> > > > vendor to ensure that the hardware is not at fault, but if we use the
> > > > "new" interface (RTAS_CHANGE_MSI_FN) to disable MSI, we also
> > > > automatically disable MSI-X and the adapter does not appear to signal
> > > > any stray MSI-X interrupt.
> > > 
> > > It seems this could also be a firmware bug, have we heard anything from
> > > them? PAPR explicitly says that RTAS_CHANGE_FN (function=1) should
> > > disable MSI _and_ MSI-X (R1???7.3.10.5.1???1).
> > 
> > We're tracking that down too. I think the fact that the interrupt is
> > coming in is a hardware bug in this particular adapter.
> > 
> > I'm looking at PAPR again and I see what might be a contradiction:
> > 
> > 7.3.10.5.1: "To removing all MSIs, set the Requested Number of
> > Interrupts to zero."
> > 
> > Table 71: "Function ... 1: Request to set to a new number of MSI or
> > MSI-X (platform choice) interrupts (including set to 0)"
> > 
> > It seems like the Table claims that using RTAS_CHANGE_FN with 0, could
> > change only MSI or MSI-X and still be not a bug?
> 
> Yeah I guess you could read it that way, though I think that would be a
> bug.
> 
> The idea is that it chooses for you whether it uses MSI or MSI-X. So the
> only sane semantic is that when deconfiguring it deconfigures either,
> ie. both, kinds.

I agree with you that is how it should be :) I'm asking the firmware
folks to make sure I'm not misunderstanding the underlying issue.

> Looking closer at your patch, now I don't understand :)
> 
> +       /*
> +        * disabling MSI with the explicit interface also disables MSI-X
> +        */
> +       if (rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, 0) != 0) {
> 
> 
> So we first disable using function 3, which should:
> 
>         3: Request to set to a new number of MSI interrupts (including set to 0)
> 
> Which does not mention MSI-X at all, implying it has no effect on them.
> Which contradicts what you see, and the comment in the code?

Thanks for the thorough review!

Per PAPR 2.4 from Power.org, look at the page before that table, page
169:

"Specifying Function 3 (MSI) also disables MSI-X for the specified IOA
function, and likewise specifying Function 4 (MSI-X) disables MSI for
the IOA function....Specifying the Requested Number of Interrupts to
zero for either Function 3 or 4 removes all MSI & MSI-X interrupts from
the IOA function."

So I'm relying on this aspect of PAPR being enforced by the firmware,
which I think it is in my testing.

Thanks,
Nish

^ permalink raw reply

* Re: [PATCH] powerpc: disable MSI using new interface if possible
From: Florian Mickler @ 2011-03-04 11:13 UTC (permalink / raw)
  To: Joe Perches; +Cc: Nishanth Aravamudan, linuxppc-dev
In-Reply-To: <1299209359.4338.220.camel@Joe-Laptop>

On Thu, 03 Mar 2011 19:29:19 -0800
Joe Perches <joe@perches.com> wrote:

> > $ git log --pretty=format:"%an %ae" arch/powerpc/platforms/pseries/msi.c
> > Andre Detsch adetsch@br.ibm.com
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > Michael Ellerman michael@ellerman.id.au
> > 
> > And the patch from Andre was also signed off by me.
> 
> How nice for you.  Last patch from you was
> 2 years ago.
> 

The problem is, that it get's pretty slow if you don't do a time
cut-off... i don't know the complexity but i guess it's linear in
(number of files) * (number of commits) * (avg size of diffs) ... 
(if you compute the diffstat) 

What I'll probably try as soon as I have another few hours to spare
for get_maintainer.pl, is to implement a fall back to older history if
the data basis is not stable enough. 


> 
> CC'ing inactive non named maintainers via git
> history also draws complaints btw.
> 
Yes.  I guess, my current version could use an staggered approach where
it does more digging if the results are too volatile... but first
try to be fast ...

Regards,
Flo

^ permalink raw reply

* Re: [PATCH v4 2/2] add icswx support
From: Tseng-Hui (Frank) Lin @ 2011-03-04 17:29 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: tsenglin, linuxppc-dev
In-Reply-To: <1299200560.8833.869.camel@pasglop>

On Fri, 2011-03-04 at 12:02 +1100, Benjamin Herrenschmidt wrote:
> On Wed, 2011-03-02 at 11:20 -0600, Tseng-Hui (Frank) Lin wrote:
> 
> > +#define CPU_FTR_ICSWX                  LONG_ASM_CONST(0x1000000000000000)
> 
> Do we want a userspace visible feature as well ? Or some other way to
> inform userspace that we support icswx ?
> 
Does a user space program really need to know about icswx? Only
coprocessor drivers need to know about icswx. Shouldn't user space
programs talk to the coprocessor drivers instead? 

> > index acac35d..b0c2549 100644
> > --- a/arch/powerpc/include/asm/mmu-hash64.h
> > +++ b/arch/powerpc/include/asm/mmu-hash64.h
> > @@ -409,6 +409,14 @@ static inline void subpage_prot_init_new_context(struct mm_struct *mm) { }
> >  
> >  typedef unsigned long mm_context_id_t;
> >  
> > +#ifdef CONFIG_ICSWX
> 
> CONFIG_PPC_ICSWX please.
> 
Will change.

> > +/*
> > + * Use forward declearation to avoid including linux/spinlock_types.h
> > + * which breaks kernel build.
> > + */
> > +struct spinlock;
> > +#endif /* CONFIG_ICSWX */
> > +
> 
> Yuck. Instead put all your fields into a structure called something
> like struct copro_data, make that a fwd declaration and stick a pointer
> to it in the mm_context.
> 
> It then only need to be allocated for processes that try to use copros,
> and the definition of that structure can remain local to whatever header
> you have dedicated for that.
> 
Thought about that. However, multiple threads can call use_cop() at the
same time. Without the spinlock being setup in advance, how do I
guarantee allocating struct copro_data and modifying the pointer in the
mm_context to be atomic?

> >  typedef struct {
> >  	mm_context_id_t id;
> >  	u16 user_psize;		/* page size index */
> > @@ -423,6 +431,11 @@ typedef struct {
> >  #ifdef CONFIG_PPC_SUBPAGE_PROT
> >  	struct subpage_prot_table spt;
> >  #endif /* CONFIG_PPC_SUBPAGE_PROT */
> > +#ifdef CONFIG_ICSWX
> > +	struct spinlock *cop_lockp; /* guard acop and cop_pid */
> > +	unsigned long acop;	/* mask of enabled coprocessor types */
> > +	unsigned int cop_pid;	/* pid value used with coprocessors */
> > +#endif /* CONFIG_ICSWX */
> >  } mm_context_t;
> >  
> > 
> > diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> > index 81fb412..fe0a09a 100644
> > --- a/arch/powerpc/include/asm/mmu_context.h
> > +++ b/arch/powerpc/include/asm/mmu_context.h
> > @@ -32,6 +32,12 @@ extern void __destroy_context(unsigned long context_id);
> >  extern void mmu_context_init(void);
> >  #endif
> >  
> > +#ifdef CONFIG_ICSWX
> > +extern void switch_cop(struct mm_struct *next);
> > +extern int use_cop(unsigned long acop, struct mm_struct *mm);
> > +extern void drop_cop(unsigned long acop, struct mm_struct *mm);
> > +#endif /* CONFIG_ICSWX */
> 
> No need to ifdef declarations.
> 
>   .../...
> 
Will change.

> > +
> > +#ifdef CONFIG_ICSWX_LAZY_SWITCH
> > +static DEFINE_PER_CPU(unsigned long, acop_reg);
> > +#define mtspr_acop(val) \
> > +	if (__get_cpu_var(acop_reg) != val) { \
> > +		get_cpu_var(acop_reg) = val; \
> > +		mtspr(SPRN_ACOP, val); \
> > +		put_cpu_var(acop_reg); \
> > +	}
> 
> Why get/put games here since you did a __get in the first place ?
> Doesn't make much sense. This is all inside context switch anyways so
> you just do __ always and don't bother with put.
> 
Will change.

> > +#else
> > +#define mtspr_acop(val) mtspr(SPRN_ACOP, val)
> > +#endif /* CONFIG_ICSWX_LAZY_SWITCH */
> 
> I'm not sure I totally get the point of having an ifdef here. Can't you
> make it unconditional ? Or do you expect distros to turn that off in
> which case what's the point ?
> 
There is only one coprocessor, HFI, using icswx at this moment. The lazy
switching makes sense. However, in the future, if more types of
coprocessors are added, the lazy switching may actually be a bad idea.
This option allows users to turn off the lazy switching.

> > +#define COP_PID_NONE 0
> > +#define COP_PID_MIN (COP_PID_NONE + 1)
> > +#define COP_PID_MAX (0xFFFF)
> > +
> > +static DEFINE_SPINLOCK(mmu_context_acop_lock);
> > +static DEFINE_IDA(cop_ida);
> > +
> > +void switch_cop(struct mm_struct *next)
> > +{
> > +	mtspr(SPRN_PID, next->context.cop_pid);
> > +	mtspr_acop(next->context.acop);
> > +}
> 
> s/mtspr_acop/set_cpu_acop() instead.
> 
Will change.

> > +static int new_cop_pid(struct ida *ida, int min_id, int max_id,
> > +		       spinlock_t *lock)
> > +{
> > +	int index;
> > +	int err;
> > +
> > +again:
> > +	if (!ida_pre_get(ida, GFP_KERNEL))
> > +		return -ENOMEM;
> > +
> > +	spin_lock(lock);
> > +	err = ida_get_new_above(ida, min_id, &index);
> > +	spin_unlock(lock);
> > +
> > +	if (err == -EAGAIN)
> > +		goto again;
> > +	else if (err)
> > +		return err;
> > +
> > +	if (index > max_id) {
> > +		spin_lock(lock);
> > +		ida_remove(ida, index);
> > +		spin_unlock(lock);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	return index;
> > +}
> > +
> > +/*
> > + * Start using a coprocessor.
> > + * @acop: mask of coprocessor to be used.
> > + * @mm: The mm the coprocessor to associate with. Most likely current mm.
> > + *
> > + * Return a positive PID if successful. Negative errno otherwise.
> > + * The returned PID will be fed to the coprocessor to determine if an
> > + * icswx transaction is authenticated.
> > + */
> > +int use_cop(unsigned long acop, struct mm_struct *mm)
> > +{
> > +	int cop_pid;
> > +
> > +	if (!cpu_has_feature(CPU_FTR_ICSWX))
> > +		return -ENODEV;
> > +
> > +	if (!mm || !acop)
> > +		return -EINVAL;
> > +
> > +	spin_lock(mm->context.cop_lockp);
> > +	if (mm->context.cop_pid == COP_PID_NONE) {
> 
> new_cop_pid goes GFP_KERNEL allocs no ? It even goes at great length to
> drop the mmu_context_acop_lock while doing ide_pre_get() ... but you
> call the whole thing with the mm->context.cop_lockp held. So that's all
> wrong. You need to drop the lock, allocate a PID, take the lock again,
> check if somebody came in and gave you a PID already, if yes, release
> the PID you allocated.
> 
> Another option is to use a mutex since none of that is in the context
> switch path right ?
> 
I'll take the first option.

> Also do you ever call use_cop for a non-current mm ?
Hmm, no. OK, remove the second parameter, mm. 

I wonder if I should change drop_cop() to __drop_cop() and make a new
drop_cop() that calls __drop_cop() with current mm.

> > +		cop_pid = new_cop_pid(&cop_ida, COP_PID_MIN, COP_PID_MAX,
> > +				      &mmu_context_acop_lock);
> > +		if (cop_pid < 0) {
> > +			spin_unlock(mm->context.cop_lockp);
> > +			return cop_pid;
> > +		}
> > +		mm->context.cop_pid = cop_pid;
> > +		if (mm == current->active_mm)
> > +			mtspr(SPRN_PID,  mm->context.cop_pid);
> > +	}
> > +	mm->context.acop |= acop;
> > +	if (mm == current->active_mm)
> > +		mtspr_acop(mm->context.acop);
> > +	spin_unlock(mm->context.cop_lockp);
> > +	return mm->context.cop_pid;
> > +}
> > +EXPORT_SYMBOL_GPL(use_cop);
> > +
> > +/*
> > + * Stop using a coprocessor.
> > + * @acop: mask of coprocessor to be stopped.
> > + * @mm: The mm the coprocessor associated with.
> > + */
> > +void drop_cop(unsigned long acop, struct mm_struct *mm)
> > +{
> > +	if (WARN_ON(!mm))
> > +		return;
> > +
> > +	spin_lock(mm->context.cop_lockp);
> > +	mm->context.acop &= ~acop;
> > +	if (mm == current->active_mm)
> > +		mtspr_acop(mm->context.acop);
> > +	if ((!mm->context.acop) && (mm->context.cop_pid != COP_PID_NONE)) {
> > +		spin_lock(&mmu_context_acop_lock);
> > +		ida_remove(&cop_ida, mm->context.cop_pid);
> > +		spin_unlock(&mmu_context_acop_lock);
> > +		mm->context.cop_pid = COP_PID_NONE;
> > +		if (mm == current->active_mm)
> > +			mtspr(SPRN_PID, mm->context.cop_pid);
> > +	}
> > +	spin_unlock(mm->context.cop_lockp);
> > +}
> > +EXPORT_SYMBOL_GPL(drop_cop);
> > +
> > +#endif /* CONFIG_ICSWX */
> > +
> >  static DEFINE_SPINLOCK(mmu_context_lock);
> >  static DEFINE_IDA(mmu_context_ida);
> >  
> > @@ -79,6 +245,12 @@ int init_new_context(struct task_struct *tsk, struct mm_struct *mm)
> >  		slice_set_user_psize(mm, mmu_virtual_psize);
> >  	subpage_prot_init_new_context(mm);
> >  	mm->context.id = index;
> > +#ifdef CONFIG_ICSWX
> > +	mm->context.cop_lockp = kmalloc(sizeof(spinlock_t), GFP_KERNEL);
> > +	if (! mm->context.cop_lockp)
> > +		return -ENOMEM;
> > +	spin_lock_init(mm->context.cop_lockp);
> > +#endif /* CONFIG_ICSWX */
> >  
> >  	return 0;
> >  }
> 
> No, do that on the first time a process tries to use it. No need to add
> overhead to every task in the system.
> 
Same concern as above. I need something initialized in advance to
guarantee allocating memory and updating the pointer are safe when it
happens in use_cop().

> > @@ -93,6 +265,11 @@ EXPORT_SYMBOL_GPL(__destroy_context);
> >  
> >  void destroy_context(struct mm_struct *mm)
> >  {
> > +#ifdef CONFIG_ICSWX
> > +	drop_cop(mm->context.acop, mm);
> > +	kfree(mm->context.cop_lockp);
> > +	mm->context.cop_lockp = NULL;
> > +#endif /* CONFIG_ICSWX */
> >  	__destroy_context(mm->context.id);
> >  	subpage_prot_free(mm);
> >  	mm->context.id = NO_CONTEXT;
> > diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
> > index 111138c..0007b66 100644
> > --- a/arch/powerpc/platforms/Kconfig.cputype
> > +++ b/arch/powerpc/platforms/Kconfig.cputype
> > @@ -226,6 +226,49 @@ config VSX
> >  
> >  	  If in doubt, say Y here.
> >  
> > +config ICSWX
> > +	bool "Support for PowerPC icswx coprocessor instruction"
> > +	depends on POWER4
> > +	default n
> > +	---help---
> > +
> > +	  Enabling this option to turn on the PowerPC Initiate Coprocessor
> > +	  Store Word (icswx) coprocessor instruction support for POWER7
> > +	  or newer processors.
> > +
> > +	  This option is only useful if you have a processor that supports
> > +	  icswx coprocessor instruction. It does not have any effect on
> > +	  processors without icswx coprocessor instruction.
> > +
> > +	  This option slightly increases kernel memory usage.
> > +
> > +	  Say N if you do not have a PowerPC processor supporting icswx
> > +	  instruction and a PowerPC coprocessor.
> > +
> > +config ICSWX_LAZY_SWITCH
> > +	bool "Lazy switching coprocessor type registers at context switching"
> > +	depends on ICSWX
> > +	default y
> > +	---help---
> > +
> > +	  Coprocessor type register is part of process context. It needs
> > +	  to be switched at context switching.
> > +
> > +	  As most machines have a very small number (most likely <= 1)
> > +	  of coprocessors, there is a good chance that the value of the
> > +	  coprocessor type register is the same between many processes.
> > +	  We do not need to change coprocessor type register at context
> > +	  switching unless the task to switch to has a different value.
> > +	
> > +	  Accessing CPU special purpose registers is far more expensive
> > +	  than accessing memory. This option uses a per-CPU variable
> > +	  to track the value of coprocessor type register. The variable
> > +	  is checked before updating coprocessor type register. The saving
> > +	  for one access is small but could turn big with the high
> > +	  frequency of context switching.
> > +	
> > +	  Say Y unless you have multiple coprocessors.
> > +
> >  config SPE
> >  	bool "SPE Support"
> >  	depends on E200 || (E500 && !PPC_E500MC)
> Ben.
> 
Thanks for the comments.
> 

^ permalink raw reply

* Re: [PATCH v4 2/2] add icswx support
From: Benjamin Herrenschmidt @ 2011-03-04 20:26 UTC (permalink / raw)
  To: Tseng-Hui (Frank) Lin; +Cc: tsenglin, linuxppc-dev
In-Reply-To: <1299259794.28840.57.camel@flin.austin.ibm.com>

On Fri, 2011-03-04 at 11:29 -0600, Tseng-Hui (Frank) Lin wrote:
> On Fri, 2011-03-04 at 12:02 +1100, Benjamin Herrenschmidt wrote:
> > On Wed, 2011-03-02 at 11:20 -0600, Tseng-Hui (Frank) Lin wrote:
> > 
> > > +#define CPU_FTR_ICSWX                  LONG_ASM_CONST(0x1000000000000000)
> > 
> > Do we want a userspace visible feature as well ? Or some other way to
> > inform userspace that we support icswx ?
> > 
> Does a user space program really need to know about icswx? Only
> coprocessor drivers need to know about icswx. Shouldn't user space
> programs talk to the coprocessor drivers instead? 

Well, I don't know how you use icswx on P7+, but on Prism it's
definitely issued directly by userspace.

> Thought about that. However, multiple threads can call use_cop() at the
> same time. Without the spinlock being setup in advance, how do I
> guarantee allocating struct copro_data and modifying the pointer in the
> mm_context to be atomic?

You don't need to. You allocate and initialize the structure, and you
compare & swap the pointer. If somebody beat you, you trash your copy. 

> > I'm not sure I totally get the point of having an ifdef here. Can't you
> > make it unconditional ? Or do you expect distros to turn that off in
> > which case what's the point ?
> > 
> There is only one coprocessor, HFI, using icswx at this moment. The lazy
> switching makes sense. However, in the future, if more types of
> coprocessors are added, the lazy switching may actually be a bad idea.
> This option allows users to turn off the lazy switching.

No user in real life plays with kernel config options. Care to explain
why the lazy switching would be a problem ?

> Same concern as above. I need something initialized in advance to
> guarantee allocating memory and updating the pointer are safe when it
> happens in use_cop().

No you don't, see above.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH v4 2/2] add icswx support
From: Tseng-Hui (Frank) Lin @ 2011-03-04 22:22 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: tsenglin, linuxppc-dev
In-Reply-To: <1299270417.8833.897.camel@pasglop>

On Sat, 2011-03-05 at 07:26 +1100, Benjamin Herrenschmidt wrote:
> On Fri, 2011-03-04 at 11:29 -0600, Tseng-Hui (Frank) Lin wrote:
> > On Fri, 2011-03-04 at 12:02 +1100, Benjamin Herrenschmidt wrote:
> > > On Wed, 2011-03-02 at 11:20 -0600, Tseng-Hui (Frank) Lin wrote:
> > > 
> > > > +#define CPU_FTR_ICSWX                  LONG_ASM_CONST(0x1000000000000000)
> > > 
> > > Do we want a userspace visible feature as well ? Or some other way to
> > > inform userspace that we support icswx ?
> > > 
> > Does a user space program really need to know about icswx? Only
> > coprocessor drivers need to know about icswx. Shouldn't user space
> > programs talk to the coprocessor drivers instead? 
> 
> Well, I don't know how you use icswx on P7+, but on Prism it's
> definitely issued directly by userspace.
> 
OK. You've got a point. I wasn't aware of Prism. HFI device driver is
currently the only icswx user on P7. Could you point me to more
information about how Prism uses icswx from user space?

> > Thought about that. However, multiple threads can call use_cop() at the
> > same time. Without the spinlock being setup in advance, how do I
> > guarantee allocating struct copro_data and modifying the pointer in the
> > mm_context to be atomic?
> 
> You don't need to. You allocate and initialize the structure, and you
> compare & swap the pointer. If somebody beat you, you trash your copy. 
> 
Is atomic_cmpxchg() the one to do the trick?

> > > I'm not sure I totally get the point of having an ifdef here. Can't you
> > > make it unconditional ? Or do you expect distros to turn that off in
> > > which case what's the point ?
> > > 
> > There is only one coprocessor, HFI, using icswx at this moment. The lazy
> > switching makes sense. However, in the future, if more types of
> > coprocessors are added, the lazy switching may actually be a bad idea.
> > This option allows users to turn off the lazy switching.
> 
> No user in real life plays with kernel config options. Care to explain
> why the lazy switching would be a problem ?
> 
The lazy switching checks the shadow variable first before setting ACOP
register. This saves mtspr() only if the new value is the same as
current. If there are several coprocessors on the system, the ACOP
register may have to be changed frequently. In that case, the lazy
switching will not save time. In extreme case when the ACOP register
needs to be changed every time, it actually slows down the execution by
the additional shadow variable checking.

> > Same concern as above. I need something initialized in advance to
> > guarantee allocating memory and updating the pointer are safe when it
> > happens in use_cop().
> 
> No you don't, see above.
> 
> Cheers,
> Ben.
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH] Fix masking of interrupts for 52xx GPT IRQ.
From: Henk Stegeman @ 2011-03-04 22:40 UTC (permalink / raw)
  To: Grant Likely, Benjamin Herrenschmidt, linuxppc-dev
In-Reply-To: <AANLkTim8W6np1A1d=SKFNS5ze8FmrbUekoZaY8_a=46A@mail.gmail.com>

What if the unmask/mask functions are dropped and only interrupt
enable/disable functions are provided?
I.e: rename the old unmask/mask functions to enable/disable and
register them as enable/disable? I did try that, and it works too, no
spurious IRQ's and no missing IRQ's.


Cheers,

Henk.

On Wed, Mar 2, 2011 at 10:30 PM, Grant Likely <grant.likely@secretlab.ca> w=
rote:
> [fixed top-posted reply]
>
> On Wed, Feb 9, 2011 at 3:16 AM, Henk Stegeman <henk.stegeman@gmail.com> w=
rote:
>> On Mon, Feb 7, 2011 at 12:05 AM, Benjamin Herrenschmidt
>> <benh@kernel.crashing.org> wrote:
>>> On Sat, 2011-01-15 at 02:28 +0100, Henk Stegeman wrote:
>>>> When using the GPT as interrupt controller interupts were missing.
>>>> It turned out the IrqEn bit of the GPT is not a mask bit, but it also
>>>> disables interrupt generation. This modification masks interrupt one
>>>> level down the cascade. Note that masking one level down the cascade
>>>> is only valid here because the GPT as interrupt ontroller only serves
>>>> one IRQ.
>>>
>>> I'm not too sure here... You shouldn't implemen t both mask/unmask and
>>> enable/disable on the same irq_chip and certainly not cal
>>> enable_irq/disable_irq from a mask or an unmask callback...
>>>
>>> Now, I'm not familiar with the HW here, can you tell me more about what
>>> exactly is happening, how things are layed out and what you are trying
>>> to fix ?
>>>
> [...]
>> Because the old code in the unmask/mask function did enable/disable
>> and I didn't want to just drop that code, I provided it via the
>> enable/disable function.
>> What is wrong by implementing & registering both mask/unmask and
>> enable/disable for the same irq_chip?
>> If it is wrong it would be nice to let the kernel print a big fat
>> warning when this is registered.
>
> After some digging, yes Ben is right. =A0It doesn't make much sense to
> provide an enable/disable function along with the mask/unmask. =A0I
> think you can safely drop the old enable/disable code. =A0I'm going to
> drop this patch from my tree and you can respin and retest.
>
> g.
>
>>
>> Cheers,
>>
>> Henk
>>
>
>>>
>>>> Signed-off-by: Henk Stegeman <henk.stegeman@gmail.com>
>>>> ---
>>>> =A0arch/powerpc/platforms/52xx/mpc52xx_gpt.c | =A0 25 ++++++++++++++++=
++++++---
>>>> =A01 files changed, 22 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c b/arch/powerpc/=
platforms/52xx/mpc52xx_gpt.c
>>>> index 6f8ebe1..9ae2045 100644
>>>> --- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
>>>> +++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
>>>> @@ -91,7 +91,7 @@ struct mpc52xx_gpt_priv {
>>>> =A0 =A0 =A0 struct irq_host *irqhost;
>>>> =A0 =A0 =A0 u32 ipb_freq;
>>>> =A0 =A0 =A0 u8 wdt_mode;
>>>> -
>>>> + =A0 =A0 int cascade_virq;
>>>> =A0#if defined(CONFIG_GPIOLIB)
>>>> =A0 =A0 =A0 struct of_gpio_chip of_gc;
>>>> =A0#endif
>>>> @@ -136,18 +136,35 @@ DEFINE_MUTEX(mpc52xx_gpt_list_mutex);
>>>> =A0static void mpc52xx_gpt_irq_unmask(unsigned int virq)
>>>> =A0{
>>>> =A0 =A0 =A0 struct mpc52xx_gpt_priv *gpt =3D get_irq_chip_data(virq);
>>>> +
>>>> + =A0 =A0 enable_irq(gpt->cascade_virq);
>>>> +
>>>> +}
>>>> +
>>>> +static void mpc52xx_gpt_irq_mask(unsigned int virq)
>>>> +{
>>>> + =A0 =A0 struct mpc52xx_gpt_priv *gpt =3D get_irq_chip_data(virq);
>>>> +
>>>> + =A0 =A0 disable_irq(gpt->cascade_virq);
>>>> +}
>>>> +
>>>> +static void mpc52xx_gpt_irq_enable(unsigned int virq)
>>>> +{
>>>> + =A0 =A0 struct mpc52xx_gpt_priv *gpt =3D get_irq_chip_data(virq);
>>>> =A0 =A0 =A0 unsigned long flags;
>>>>
>>>> + =A0 =A0 dev_dbg(gpt->dev, "%s %d\n", __func__, virq);
>>>> =A0 =A0 =A0 spin_lock_irqsave(&gpt->lock, flags);
>>>> =A0 =A0 =A0 setbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_IRQ_EN);
>>>> =A0 =A0 =A0 spin_unlock_irqrestore(&gpt->lock, flags);
>>>> =A0}
>>>>
>>>> -static void mpc52xx_gpt_irq_mask(unsigned int virq)
>>>> +static void mpc52xx_gpt_irq_disable(unsigned int virq)
>>>> =A0{
>>>> =A0 =A0 =A0 struct mpc52xx_gpt_priv *gpt =3D get_irq_chip_data(virq);
>>>> =A0 =A0 =A0 unsigned long flags;
>>>>
>>>> + =A0 =A0 dev_dbg(gpt->dev, "%s %d\n", __func__, virq);
>>>> =A0 =A0 =A0 spin_lock_irqsave(&gpt->lock, flags);
>>>> =A0 =A0 =A0 clrbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_IRQ_EN);
>>>> =A0 =A0 =A0 spin_unlock_irqrestore(&gpt->lock, flags);
>>>> @@ -184,6 +201,8 @@ static struct irq_chip mpc52xx_gpt_irq_chip =3D {
>>>> =A0 =A0 =A0 .name =3D "MPC52xx GPT",
>>>> =A0 =A0 =A0 .unmask =3D mpc52xx_gpt_irq_unmask,
>>>> =A0 =A0 =A0 .mask =3D mpc52xx_gpt_irq_mask,
>>>> + =A0 =A0 .enable =3D mpc52xx_gpt_irq_enable,
>>>> + =A0 =A0 .disable =3D mpc52xx_gpt_irq_disable,
>>>> =A0 =A0 =A0 .ack =3D mpc52xx_gpt_irq_ack,
>>>> =A0 =A0 =A0 .set_type =3D mpc52xx_gpt_irq_set_type,
>>>> =A0};
>>>> @@ -268,7 +287,7 @@ mpc52xx_gpt_irq_setup(struct mpc52xx_gpt_priv *gpt=
, struct device_node *node)
>>>> =A0 =A0 =A0 if ((mode & MPC52xx_GPT_MODE_MS_MASK) =3D=3D 0)
>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(&gpt->regs->mode, mode | MPC52xx_=
GPT_MODE_MS_IC);
>>>> =A0 =A0 =A0 spin_unlock_irqrestore(&gpt->lock, flags);
>>>> -
>>>> + =A0 =A0 gpt->cascade_virq =3D cascade_virq;
>>>> =A0 =A0 =A0 dev_dbg(gpt->dev, "%s() complete. virq=3D%i\n", __func__, =
cascade_virq);
>>>> =A0}
>>>>
>>>
>>>
>>>
>>
>
>
>
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
>

^ permalink raw reply

* Re: [PATCH v4 2/2] add icswx support
From: Benjamin Herrenschmidt @ 2011-03-04 22:40 UTC (permalink / raw)
  To: Tseng-Hui (Frank) Lin; +Cc: tsenglin, linuxppc-dev
In-Reply-To: <1299277344.28840.79.camel@flin.austin.ibm.com>

On Fri, 2011-03-04 at 16:22 -0600, Tseng-Hui (Frank) Lin wrote:

> > Well, I don't know how you use icswx on P7+, but on Prism it's
> > definitely issued directly by userspace.
> > 
> OK. You've got a point. I wasn't aware of Prism. HFI device driver is
> currently the only icswx user on P7. Could you point me to more
> information about how Prism uses icswx from user space?

Let's ignore that for now, there's a whole infrastructure for it and it
hasn't been published yet, maybe later this year... We can look at
exposing the feature to userspace then.

> > You don't need to. You allocate and initialize the structure, and you
> > compare & swap the pointer. If somebody beat you, you trash your copy. 
> > 
> Is atomic_cmpxchg() the one to do the trick?

No, just cmpxchg()

> The lazy switching checks the shadow variable first before setting ACOP
> register. This saves mtspr() only if the new value is the same as
> current. If there are several coprocessors on the system, the ACOP
> register may have to be changed frequently. In that case, the lazy
> switching will not save time. In extreme case when the ACOP register
> needs to be changed every time, it actually slows down the execution by
> the additional shadow variable checking.

By how much ? Is it even measurable ?

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH] Fix masking of interrupts for 52xx GPT IRQ.
From: Benjamin Herrenschmidt @ 2011-03-04 22:41 UTC (permalink / raw)
  To: Henk Stegeman; +Cc: linuxppc-dev
In-Reply-To: <AANLkTi=Wq_uF14TXWQ3ooOpbkroJfM_DOWZXJfCWDHcX@mail.gmail.com>

On Fri, 2011-03-04 at 23:40 +0100, Henk Stegeman wrote:
> What if the unmask/mask functions are dropped and only interrupt
> enable/disable functions are provided?

No, that's backward. enable/disable are handled by the core, leave them
there.

> I.e: rename the old unmask/mask functions to enable/disable and
> register them as enable/disable? I did try that, and it works too, no
> spurious IRQ's and no missing IRQ's.
> 

Ben.

^ permalink raw reply

* Re: [PATCH v4 2/2] add icswx support
From: Tseng-Hui (Frank) Lin @ 2011-03-04 23:07 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: tsenglin, linuxppc-dev
In-Reply-To: <1299278441.8833.915.camel@pasglop>

On Sat, 2011-03-05 at 09:40 +1100, Benjamin Herrenschmidt wrote:

> > The lazy switching checks the shadow variable first before setting ACOP
> > register. This saves mtspr() only if the new value is the same as
> > current. If there are several coprocessors on the system, the ACOP
> > register may have to be changed frequently. In that case, the lazy
> > switching will not save time. In extreme case when the ACOP register
> > needs to be changed every time, it actually slows down the execution by
> > the additional shadow variable checking.
> 
> By how much ? Is it even measurable ?
> 
I don't have any measurable numbers. That's why I made it an option in
case people wants to disable it. I do agree that the kernel has so many
options that we should refrain from adding more. If the chance that the
lazy switching slows down the execution is really low, we should just
take out the option. Is there any one has an idea how much the numbers
are?

> Cheers,
> Ben.
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH v3 0/9] fsldma: lockup fixes
From: Felix Radensky @ 2011-03-04 22:28 UTC (permalink / raw)
  To: Ira W. Snyder, linuxppc-dev
In-Reply-To: <1299174901-16762-1-git-send-email-iws@ovro.caltech.edu>

Hi Ira,

I've successfully tested this version on P2020RDB,
with 10 threads per channel, 100000 iterations per
thread.

Tested-by: Felix Radensky <felix@embedded-sol.com>

Felix.

^ permalink raw reply

* Re: [PATCH v4 2/2] add icswx support
From: Benjamin Herrenschmidt @ 2011-03-04 23:13 UTC (permalink / raw)
  To: Tseng-Hui (Frank) Lin; +Cc: tsenglin, linuxppc-dev
In-Reply-To: <1299280053.28840.96.camel@flin.austin.ibm.com>

On Fri, 2011-03-04 at 17:07 -0600, Tseng-Hui (Frank) Lin wrote:
> I don't have any measurable numbers. That's why I made it an option in
> case people wants to disable it. I do agree that the kernel has so many
> options that we should refrain from adding more. If the chance that the
> lazy switching slows down the execution is really low, we should just
> take out the option. Is there any one has an idea how much the numbers
> are? 

Well, per-cpu loads are notoriously inefficient due to nasty codegen,
but often cache hot as well, I wouldn't worry too much. Just remove the
option for now. We can always add it back if it ever becomes desirable,
but I very much doubt it.

Cheers,
Ben.

^ permalink raw reply

* [PATCH] powerpc/pseries: Enable Chelsio network and iWARP drivers
From: Anton Blanchard @ 2011-03-05 12:00 UTC (permalink / raw)
  To: benh, linuxppc-dev


Ensure the Chelsio T3/T4 network drivers and iWARP drivers are
enabled in the pseries config.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: linux-2.6/arch/powerpc/configs/pseries_defconfig
===================================================================
--- linux-2.6.orig/arch/powerpc/configs/pseries_defconfig	2011-03-05 22:58:38.886562678 +1100
+++ linux-2.6/arch/powerpc/configs/pseries_defconfig	2011-03-05 22:58:40.346615219 +1100
@@ -189,6 +189,7 @@ CONFIG_TIGON3=y
 CONFIG_BNX2=m
 CONFIG_CHELSIO_T1=m
 CONFIG_CHELSIO_T3=m
+CONFIG_CHELSIO_T4=m
 CONFIG_EHEA=y
 CONFIG_IXGBE=m
 CONFIG_IXGB=m
@@ -255,6 +256,8 @@ CONFIG_INFINIBAND_USER_MAD=m
 CONFIG_INFINIBAND_USER_ACCESS=m
 CONFIG_INFINIBAND_MTHCA=m
 CONFIG_INFINIBAND_EHCA=m
+CONFIG_INFINIBAND_CXGB3=m
+CONFIG_INFINIBAND_CXGB4=m
 CONFIG_MLX4_INFINIBAND=m
 CONFIG_INFINIBAND_IPOIB=m
 CONFIG_INFINIBAND_IPOIB_CM=y

^ permalink raw reply

* [PATCH] powerpc/85xx: Fix writing to spin table 'cpu-release-addr' on ppc64e
From: Kumar Gala @ 2011-03-05 21:05 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Kumar Gala

From: Kumar Gala <kumar.gala@freescale.com>

If the spin table is located in the linear mapping (which can happen if
we have 4G or more of memory) we need to access the spin table via a
cacheable coherent mapping like we do on ppc32 (and do explicit cache
flush).

See the following commit for the ppc32 version of this issue:

commit d1d47ec6e62ab08d2ebb925fd9203abfad3adfbf
Author: Peter Tyser <ptyser@xes-inc.com>
Date:   Fri Dec 18 16:50:37 2009 -0600

    powerpc/85xx: Fix SMP when "cpu-release-addr" is in lowmem

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/platforms/85xx/smp.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c
index 5c91a99..0d00ff9 100644
--- a/arch/powerpc/platforms/85xx/smp.c
+++ b/arch/powerpc/platforms/85xx/smp.c
@@ -91,10 +91,14 @@ smp_85xx_kick_cpu(int nr)
 	while ((__secondary_hold_acknowledge != nr) && (++n < 1000))
 		mdelay(1);
 #else
+	smp_generic_kick_cpu(nr);
+
 	out_be64((u64 *)(bptr_vaddr + BOOT_ENTRY_ADDR_UPPER),
 		__pa((u64)*((unsigned long long *) generic_secondary_smp_init)));
 
-	smp_generic_kick_cpu(nr);
+	if (!ioremappable)
+		flush_dcache_range((ulong)bptr_vaddr,
+				(ulong)(bptr_vaddr + SIZE_BOOT_ENTRY));
 #endif
 
 	local_irq_restore(flags);
-- 
1.7.2.3

^ permalink raw reply related

* Re: [PATCH] fsl_pci: Add support for FSL PCIe controllers v2.x
From: Kumar Gala @ 2011-03-05 21:29 UTC (permalink / raw)
  To: Prabhakar Kushwaha; +Cc: meet2prabhu, linuxppc-dev
In-Reply-To: <1298540104-16545-1-git-send-email-prabhakar@freescale.com>


On Feb 24, 2011, at 3:35 AM, Prabhakar Kushwaha wrote:

> FSL PCIe controller v2.1:
>     - New MSI inbound window
>     - Same Inbound windows address as PCIe controller v1.x
>=20
> Added new pit_t member(pmit) to struct ccsr_pci for MSI inbound window
>=20
> FSL PCIe controller v2.2 and v2.3:
>     - Different addresses for PCIe inbound window 3,2,1
>     - Exposed PCIe inbound window 0
>     - New PCIe interrupt status register
>=20
> Added new config and interrupt Status register to struct ccsr_pci & =
updated
> pit_t array size to reflect the 4 inbound windows.
>=20
> Device tree is used to maintain backward compatibility i.e. update =
inbound
> window 1 index depending upon "compatible" field witin PCIE node.
>=20
> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
> Acked-by: Roy Zang <tie-fei.zang@freescale.com>
> Acked-by: Kumar Gala <kumar.gala@freescale.com>
> ---
>=20
> Based upon =
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git =
(branch master )
>=20
> arch/powerpc/sysdev/fsl_pci.c |   15 +++++++++++----
> arch/powerpc/sysdev/fsl_pci.h |   17 ++++++++++-------
> 2 files changed, 21 insertions(+), 11 deletions(-)

applied to next

- k=

^ permalink raw reply

* [PATCH] powerpc/iseries: Fix early init access to lppaca
From: Benjamin Herrenschmidt @ 2011-03-07  4:02 UTC (permalink / raw)
  To: linuxppc-dev

The combination of commit

8154c5d22d91cd16bd9985b0638c8957e4688d0e and
93c22703efa72c7527dbd586d1951c1f4a85fd70

Broke boot on iSeries.

The problem is that iSeries very early boot code, which generates
the device-tree and runs before our normal early initializations
does need access the lppaca's very early, before the PACA array is
initialized, and in fact even before the boot PACA has been
initialized (it contains all 0's at this stage).

However, the first patch above makes that code use the new
llpaca_of(cpu) accessor, which itself is changed by the second patch to
use the PACA array.

We fix that by reverting iSeries to directly dereferencing the array. In
addition, we fix all iterators in the iSeries code to always skip CPU
whose number is above 63 which is the maximum size of that array and
the maximum number of supported CPUs on these machines.

Additionally, we make sure the boot_paca is properly initialized
in our early startup code.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
diff --git a/arch/powerpc/include/asm/lppaca.h b/arch/powerpc/include/asm/lppaca.h
index 380d48b..26b8c80 100644
--- a/arch/powerpc/include/asm/lppaca.h
+++ b/arch/powerpc/include/asm/lppaca.h
@@ -33,9 +33,25 @@
 //
 //----------------------------------------------------------------------------
 #include <linux/cache.h>
+#include <linux/threads.h>
 #include <asm/types.h>
 #include <asm/mmu.h>
 
+/*
+ * We only have to have statically allocated lppaca structs on
+ * legacy iSeries, which supports at most 64 cpus.
+ */
+#ifdef CONFIG_PPC_ISERIES
+#if NR_CPUS < 64
+#define NR_LPPACAS	NR_CPUS
+#else
+#define NR_LPPACAS	64
+#endif
+#else /* not iSeries */
+#define NR_LPPACAS	1
+#endif
+
+
 /* The Hypervisor barfs if the lppaca crosses a page boundary.  A 1k
  * alignment is sufficient to prevent this */
 struct lppaca {
diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index ebf9846..f4adf89 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -27,20 +27,6 @@ extern unsigned long __toc_start;
 #ifdef CONFIG_PPC_BOOK3S
 
 /*
- * We only have to have statically allocated lppaca structs on
- * legacy iSeries, which supports at most 64 cpus.
- */
-#ifdef CONFIG_PPC_ISERIES
-#if NR_CPUS < 64
-#define NR_LPPACAS	NR_CPUS
-#else
-#define NR_LPPACAS	64
-#endif
-#else /* not iSeries */
-#define NR_LPPACAS	1
-#endif
-
-/*
  * The structure which the hypervisor knows about - this structure
  * should not cross a page boundary.  The vpa_init/register_vpa call
  * is now known to fail if the lppaca structure crosses a page
diff --git a/arch/powerpc/platforms/iseries/dt.c b/arch/powerpc/platforms/iseries/dt.c
index fdb7384..f0491cc 100644
--- a/arch/powerpc/platforms/iseries/dt.c
+++ b/arch/powerpc/platforms/iseries/dt.c
@@ -242,8 +242,8 @@ static void __init dt_cpus(struct iseries_flat_dt *dt)
 	pft_size[0] = 0; /* NUMA CEC cookie, 0 for non NUMA  */
 	pft_size[1] = __ilog2(HvCallHpt_getHptPages() * HW_PAGE_SIZE);
 
-	for (i = 0; i < NR_CPUS; i++) {
-		if (lppaca_of(i).dyn_proc_status >= 2)
+	for (i = 0; i < NR_LPPACAS; i++) {
+		if (lppaca[i].dyn_proc_status >= 2)
 			continue;
 
 		snprintf(p, 32 - (p - buf), "@%d", i);
@@ -251,7 +251,7 @@ static void __init dt_cpus(struct iseries_flat_dt *dt)
 
 		dt_prop_str(dt, "device_type", device_type_cpu);
 
-		index = lppaca_of(i).dyn_hv_phys_proc_index;
+		index = lppaca[i].dyn_hv_phys_proc_index;
 		d = &xIoHriProcessorVpd[index];
 
 		dt_prop_u32(dt, "i-cache-size", d->xInstCacheSize * 1024);
diff --git a/arch/powerpc/platforms/iseries/setup.c b/arch/powerpc/platforms/iseries/setup.c
index b086341..2946ae1 100644
--- a/arch/powerpc/platforms/iseries/setup.c
+++ b/arch/powerpc/platforms/iseries/setup.c
@@ -680,6 +680,7 @@ void * __init iSeries_early_setup(void)
 	 * on but calling this function multiple times is fine.
 	 */
 	identify_cpu(0, mfspr(SPRN_PVR));
+	initialise_paca(&boot_paca, 0);
 
 	powerpc_firmware_features |= FW_FEATURE_ISERIES;
 	powerpc_firmware_features |= FW_FEATURE_LPAR;

^ permalink raw reply related

* [PATCH] powerpc/mm: Move the STAB0 location to 0x8000 to make room in low memory
From: Benjamin Herrenschmidt @ 2011-03-07  4:09 UTC (permalink / raw)
  To: linuxppc-dev

Recent linux-next builds with allmodconfig fail due to lack of space
between 0x3000 and 0x6000. We have a hard block at 0x7000 but we can
spare a page by moving the STAB0 from 0x6000 to 0x8000.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

Seems to still boot on legacy iSeries...

diff --git a/arch/powerpc/include/asm/mmu-hash64.h b/arch/powerpc/include/asm/mmu-hash64.h
index acac35d..ae7b3ef 100644
--- a/arch/powerpc/include/asm/mmu-hash64.h
+++ b/arch/powerpc/include/asm/mmu-hash64.h
@@ -27,7 +27,7 @@
 #define STE_VSID_SHIFT	12
 
 /* Location of cpu0's segment table */
-#define STAB0_PAGE	0x6
+#define STAB0_PAGE	0x8
 #define STAB0_OFFSET	(STAB0_PAGE << 12)
 #define STAB0_PHYS_ADDR	(STAB0_OFFSET + PHYSICAL_START)
 
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 8a81799..70f0f99 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -977,20 +977,6 @@ _GLOBAL(do_stab_bolted)
 	rfid
 	b	.	/* prevent speculative execution */
 
-/*
- * Space for CPU0's segment table.
- *
- * On iSeries, the hypervisor must fill in at least one entry before
- * we get control (with relocate on).  The address is given to the hv
- * as a page number (see xLparMap below), so this must be at a
- * fixed address (the linker can't compute (u64)&initial_stab >>
- * PAGE_SHIFT).
- */
-	. = STAB0_OFFSET	/* 0x6000 */
-	.globl initial_stab
-initial_stab:
-	.space	4096
-
 #ifdef CONFIG_PPC_PSERIES
 /*
  * Data area reserved for FWNMI option.
@@ -1027,3 +1013,18 @@ xLparMap:
 #ifdef CONFIG_PPC_PSERIES
         . = 0x8000
 #endif /* CONFIG_PPC_PSERIES */
+
+/*
+ * Space for CPU0's segment table.
+ *
+ * On iSeries, the hypervisor must fill in at least one entry before
+ * we get control (with relocate on).  The address is given to the hv
+ * as a page number (see xLparMap above), so this must be at a
+ * fixed address (the linker can't compute (u64)&initial_stab >>
+ * PAGE_SHIFT).
+ */
+	. = STAB0_OFFSET	/* 0x8000 */
+	.globl initial_stab
+initial_stab:
+	.space	4096
+

^ permalink raw reply related

* [PATCH] driver/FSL SATA:Fix wrong Device Error Register usage
From: Prabhakar Kushwaha @ 2011-03-07  4:30 UTC (permalink / raw)
  To: linux-ide
  Cc: meet2prabhu, Ashish Kalra, jgarzik, linuxppc-dev,
	Prabhakar Kushwaha

When a single device error is detected, the device under the error is indicated
by the error bit set in the DER. There is a one to one mapping between register
bit and devices on Port multiplier(PMP) i.e. bit 0 represents PMP device 0 and
bit 1 represents PMP device 1 etc.

Current implementation treats Device error register value as device number not
set of bits representing multiple device on PMP. It is changed to consider bit
level.
No need to check for each set bit as all command is going to be aborted.

Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
Signed-off-by: Ashish Kalra <B00888@freescale.com>
---
 git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git (branch master)

 This patch is already gone through review of linuxppc-dev mail list.
 Making CC linuxppc-dev@lists.ozlabs.org

 drivers/ata/sata_fsl.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
index b0214d0..895771c 100644
--- a/drivers/ata/sata_fsl.c
+++ b/drivers/ata/sata_fsl.c
@@ -1040,12 +1040,14 @@ static void sata_fsl_error_intr(struct ata_port *ap)
 
 		/* find out the offending link and qc */
 		if (ap->nr_pmp_links) {
+			unsigned int dev_num;
 			dereg = ioread32(hcr_base + DE);
 			iowrite32(dereg, hcr_base + DE);
 			iowrite32(cereg, hcr_base + CE);
 
-			if (dereg < ap->nr_pmp_links) {
-				link = &ap->pmp_link[dereg];
+			dev_num = ffs(dereg)-1;
+			if (dev_num < ap->nr_pmp_links) {
+				link = &ap->pmp_link[dev_num];
 				ehi = &link->eh_info;
 				qc = ata_qc_from_tag(ap, link->active_tag);
 				/*
-- 
1.7.3

^ permalink raw reply related

* [PATCH] driver/FSL SATA: Update RX_WATER_MARK for TRANSCFG
From: Prabhakar Kushwaha @ 2011-03-07  4:29 UTC (permalink / raw)
  To: linux-ide; +Cc: meet2prabhu, jgarzik, linuxppc-dev, Prabhakar Kushwaha

RX_WATER_MARK sets the number of locations in Rx FIFO that can be used before
the transport layer instructs the link layer to transmit HOLDS. Note that it
can take some time for the HOLDs to get to the other end, and that in the
interim there must be enough room in the FIFO to absorb all data that could
arrive.

Update the new recommended value to 16.

Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
---
 git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git (branch master)

 This patch is already gone through review of linuxppc-dev mail list.
 Making CC linuxppc-dev@lists.ozlabs.org

 drivers/ata/sata_fsl.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
index 895771c..29d2f29 100644
--- a/drivers/ata/sata_fsl.c
+++ b/drivers/ata/sata_fsl.c
@@ -186,6 +186,11 @@ enum {
 	COMMANDSTAT = 0x20,
 };
 
+/* TRANSCFG (transport-layer) configuration control */
+enum {
+	TRANSCFG_RX_WATER_MARK = (1 << 4),
+};
+
 /* PHY (link-layer) configuration control */
 enum {
 	PHY_BIST_ENABLE = 0x01,
@@ -1305,6 +1310,7 @@ static int sata_fsl_probe(struct platform_device *ofdev,
 	struct sata_fsl_host_priv *host_priv = NULL;
 	int irq;
 	struct ata_host *host;
+	u32 temp;
 
 	struct ata_port_info pi = sata_fsl_port_info[0];
 	const struct ata_port_info *ppi[] = { &pi, NULL };
@@ -1319,6 +1325,12 @@ static int sata_fsl_probe(struct platform_device *ofdev,
 	ssr_base = hcr_base + 0x100;
 	csr_base = hcr_base + 0x140;
 
+	if (!of_device_is_compatible(ofdev->dev.of_node, "fsl,mpc8315-sata")) {
+		temp = ioread32(csr_base + TRANSCFG);
+		temp = temp & 0xffffffe0;
+		iowrite32(temp | TRANSCFG_RX_WATER_MARK, csr_base + TRANSCFG);
+	}
+
 	DPRINTK("@reset i/o = 0x%x\n", ioread32(csr_base + TRANSCFG));
 	DPRINTK("sizeof(cmd_desc) = %d\n", sizeof(struct command_desc));
 	DPRINTK("sizeof(#define cmd_desc) = %d\n", SATA_FSL_CMD_DESC_SIZE);
-- 
1.7.3

^ permalink raw reply related

* Re: vmsplice bad address
From: Benjamin Herrenschmidt @ 2011-03-07  9:35 UTC (permalink / raw)
  To: Guillaume Dargaud; +Cc: Linuxppc-dev
In-Reply-To: <201103031401.55649.dargaud@lpsc.in2p3.fr>

On Thu, 2011-03-03 at 14:01 +0100, Guillaume Dargaud wrote:
> Hello all,
> I'm trying to use the vmsplice/splice combination to so socket writes in zero-copy mode since it seems to be the big 
> bottle neck in my application.
> 
> Here's a simplified code:
> 
> // This buffer is actually outside kernel space,
> // in reserved contiguous physical memory (custom kernel compilation)
> #define BUFFER_PHY_BASE 0x7000000
> #define BUFFER_SIZE 0x1000000
> memfd=open("/dev/mem", O_RDWR | O_SYNC)
> Base=(unsigned char*)mmap(0, BUFFER_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, memfd, BUFFER_PHY_BASE);
> 
> sockfd = socket(AF_INET, SOCK_STREAM, 0)
> 
> int pfd[2];	pipe(pfd);
> 
> struct iovec iov;
> iov.iov_base=Base;
> iov.iov_len=BUFFER_SIZE;
> printf("Pipe %d/%d, base:0x%x, size:0x%x\n", pfd[0], pfd[1], iov.iov_base, iov.iov_len);
> vmsplice(pfd[1], &iov, 1, SPLICE_F_GIFT)
> splice(pfd[0], NULL, sockfd, NULL, BUFFER_SIZE, SPLICE_F_MOVE)
> 
> I get the following:
> Pipe 6/7, base:0x480cf000, size:0x1000000
> vmsplice: Bad address
> 
> 
> Any idea what is wrong in my use of vmsplice ?

I'm not completely surprised ... I wouldn't expect vmsplice to work
on /dev/mem ... No struct page backing the memory for example.

Ben.

^ permalink raw reply

* Mpc8315erdb openvpn segmentation fault
From: Vasanth Ragavendran @ 2011-03-07 11:09 UTC (permalink / raw)
  To: linuxppc-dev


i'm facing a problem. i've installed openvpn on mpc8315erdb board and when i
give the command

openvpn --mktun --dev tap0 i get the error

 

[root@mpc8315erdb /]# openvpn --mktun --dev tap0
Sat Jan  1 01:04:21 2000 OpenVPNUnable to handle kernel paging request for
data
at address 0x00000000
 2.0.9 ppc-rpm-lFaulting instruction address: 0xc90a208c
inux-gnu [SSL] [Oops: Kernel access of bad area, sig: 11 [#1]
MPC831x RDB
Modules linked in: tun
**bleep**: c90a208c LR: c90a2068 CTR: c0288e58
REGS: c294fd80 TRAP: 0300   Not tainted  (2.6.29.6)
MSR: 00009032 <EE,ME,IR,DR>  CR: 24002424  XER: 20000000
DAR: 00000000, DSISR: 20000000
TASK = c70b6440[1635] 'openvpn' THREAD: c294e000
GPR00: 00000001 c294fe30 c70b6440 c03ebcf4 bfafa0d4 0000001c c294fe58
0fd8dc58
GPR08: 00000000 00000001 ffffffe7 00000000 84002428 100787e0 07ff9000
00000004
GPR16: 00000000 00000000 bfafa930 00010000 00010000 ffffffff 00000005
3b9aca00
GPR24: c7023d20 c294fe3c bfafa0b8 c653e6e0 00000000 c040baf4 bfafa0b8
800454ca
Call Trace:
[c294fe30] [c90a2068] 0xc90a2068 (unreliable)
[c294fe90] [c008054c] 0xc008054c
[c294fea0] [c00809c4] 0xc00809c4
[c294ff10] [c0080c9c] 0xc0080c9c
[c294ff40] [c001141c] 0xc001141c
--- Exception: c01 at 0xfce8268
    LR = 0xfd6bdec
Instruction dump:
4bfffecc 9b81001b 480008ed 3d20c90a 38000001 8129306c 81620308 2f890000
830201a4 83ab0014 817d0124 419e0014 <800b0000> 7c090010 7c000110 7c0000d0
LZO] [EPOLL] bui---[ end trace 54eee2a93ccf53d1 ]---
lt on Mar  2 2011
Sat Jan  1 01:04:21 2000 IMPORTANT: OpenVPN's default port number is now
1194, b
ased on an official port number assignment by IANA.  OpenVPN 2.0-beta16 and
earl
ier used 5000 as the default port.
Sat Jan  1 01:04:21 2000 ******* WARNING *******: all encryption and
authenticat
ion features disabled -- all data will be tunnelled as cleartext
Segmentation fault

 

Some one plz help me out! wat does this mean? Coz with the same kernel image
and the same settings  i'm able to successfully run the above command on
another mpc8315erdb board. but it fails on one of the board! any ideas would
be of great help to me!! plz!
-- 
View this message in context: http://old.nabble.com/Mpc8315erdb-openvpn-segmentation-fault-tp31086738p31086738.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.

^ permalink raw reply

* Re: Mpc8315erdb openvpn segmentation fault
From: Scott Wood @ 2011-03-07 20:22 UTC (permalink / raw)
  To: Vasanth Ragavendran; +Cc: linuxppc-dev
In-Reply-To: <31086738.post@talk.nabble.com>

On Mon, 7 Mar 2011 03:09:27 -0800
Vasanth Ragavendran <ragavendrapec@yahoo.co.in> wrote:

> 
> i'm facing a problem. i've installed openvpn on mpc8315erdb board and when i
> give the command
> 
> openvpn --mktun --dev tap0 i get the error
> 
>  
> 
> [root@mpc8315erdb /]# openvpn --mktun --dev tap0
> Sat Jan  1 01:04:21 2000 OpenVPNUnable to handle kernel paging request for
> data
> at address 0x00000000
>  2.0.9 ppc-rpm-lFaulting instruction address: 0xc90a208c
> inux-gnu [SSL] [Oops: Kernel access of bad area, sig: 11 [#1]
> MPC831x RDB
> Modules linked in: tun
> **bleep**: c90a208c LR: c90a2068 CTR: c0288e58
> REGS: c294fd80 TRAP: 0300   Not tainted  (2.6.29.6)
> MSR: 00009032 <EE,ME,IR,DR>  CR: 24002424  XER: 20000000
> DAR: 00000000, DSISR: 20000000
> TASK = c70b6440[1635] 'openvpn' THREAD: c294e000
> GPR00: 00000001 c294fe30 c70b6440 c03ebcf4 bfafa0d4 0000001c c294fe58
> 0fd8dc58
> GPR08: 00000000 00000001 ffffffe7 00000000 84002428 100787e0 07ff9000
> 00000004
> GPR16: 00000000 00000000 bfafa930 00010000 00010000 ffffffff 00000005
> 3b9aca00
> GPR24: c7023d20 c294fe3c bfafa0b8 c653e6e0 00000000 c040baf4 bfafa0b8
> 800454ca
> Call Trace:
> [c294fe30] [c90a2068] 0xc90a2068 (unreliable)
> [c294fe90] [c008054c] 0xc008054c
> [c294fea0] [c00809c4] 0xc00809c4
> [c294ff10] [c0080c9c] 0xc0080c9c
> [c294ff40] [c001141c] 0xc001141c
> --- Exception: c01 at 0xfce8268
>     LR = 0xfd6bdec
> Instruction dump:
> 4bfffecc 9b81001b 480008ed 3d20c90a 38000001 8129306c 81620308 2f890000
> 830201a4 83ab0014 817d0124 419e0014 <800b0000> 7c090010 7c000110 7c0000d0
> LZO] [EPOLL] bui---[ end trace 54eee2a93ccf53d1 ]---
> lt on Mar  2 2011
> Sat Jan  1 01:04:21 2000 IMPORTANT: OpenVPN's default port number is now
> 1194, b
> ased on an official port number assignment by IANA.  OpenVPN 2.0-beta16 and
> earl
> ier used 5000 as the default port.
> Sat Jan  1 01:04:21 2000 ******* WARNING *******: all encryption and
> authenticat
> ion features disabled -- all data will be tunnelled as cleartext
> Segmentation fault
> 
>  
> 
> Some one plz help me out! wat does this mean? Coz with the same kernel image
> and the same settings  i'm able to successfully run the above command on
> another mpc8315erdb board. but it fails on one of the board! any ideas would
> be of great help to me!! plz!

Please turn on CONFIG_KALLSYMS, so that symbols will be reported in the
crash dump, and let us know which kernel you're using.

Are the other boards where it does not fail running the same kernel?  Same
userspace?  Same U-Boot?  Anything else different?

-Scott

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox