LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: Need help for USB OTG feature for Canyonlands PPC460EX Board
From: Wolfgang Denk @ 2011-02-02  7:50 UTC (permalink / raw)
  To: sunny bhayani; +Cc: linuxppc-dev
In-Reply-To: <AANLkTikatrqNZydG3Do8UgYeB4c=iteBG==r2dBn6hZV@mail.gmail.com>

Dear sunny bhayani,

In message <AANLkTikatrqNZydG3Do8UgYeB4c=iteBG==r2dBn6hZV@mail.gmail.com> you wrote:
>
> I am trying to enable the USB DWC OTG feature for Canyonlands PPC460EX
> Board, and am using the 2.6.30 kernel from denx.
> 
> Now the issue is I am selecting the "USB Gadget" feature from the kernel
> menuconfig, but the kernel log only shows,

I don't really understand why you ask this again, now on the next
mailing list - after I sent you a reply on the ELDK list.

Here is a link to my reply:
http://thread.gmane.org/gmane.comp.embedded.eldk/1827/focus=1828

Please update, and try again.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
grep me no patterns and I'll tell you no lines.

^ permalink raw reply

* Re: [PATCH V2 4/6] powerpc/44x: don't use tlbivax on AMP systems
From: Dave Kleikamp @ 2011-02-02 13:19 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <85A99D1E-9B32-4816-91FD-355B325D3F8F@kernel.crashing.org>

On Wed, 2011-02-02 at 01:48 -0600, Kumar Gala wrote:
> On Feb 1, 2011, at 12:48 PM, Dave Kleikamp wrote:
> 
> > Since other OS's may be running on the other cores don't use tlbivax
> 
> Are you guys building SMP kernel for use with AMP?  Just wondering why you'd be using tlbivax at all.

Yes, for instance, a 4-core chip could run two 2-way instances.

Shaggy
-- 
Dave Kleikamp
IBM Linux Technology Center

^ permalink raw reply

* Re: [PATCH 4/6] ftrace syscalls: Allow arch specific syscall symbol matching
From: Steven Rostedt @ 2011-02-02 14:04 UTC (permalink / raw)
  To: Ian Munsie
  Cc: Andreas Dilger, Andreas Schwab, Dave Kleikamp,
	open list:DOCUMENTATION, Namhyung Kim, Jiri Kosina, Jason Baron,
	linux-kernel, Alexander Graf, Ingo Molnar, Paul Mackerras,
	KOSAKI Motohiro, Frederic Weisbecker, Scott Wood, Nathan Lynch,
	Andrew Morton, David Gibson, linuxppc-dev, Avantika Mathur
In-Reply-To: <1296630718-17537-5-git-send-email-imunsie@au1.ibm.com>

On Wed, 2011-02-02 at 18:11 +1100, Ian Munsie wrote:

I'll answer your question here.

> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index dcd6a7c..0d0e109 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -527,6 +527,15 @@ extern enum ftrace_dump_mode ftrace_dump_on_oops;
>  #ifdef CONFIG_FTRACE_SYSCALLS
>  
>  unsigned long arch_syscall_addr(int nr);
> +#ifndef arch_syscall_match_sym_name
> +/*
> + * Only compare after the "sys" prefix. Archs that use
> + * syscall wrappers may have syscalls symbols aliases prefixed
> + * with "SyS" instead of "sys", leading to an unwanted
> + * mismatch.
> + */
> +#define arch_syscall_match_sym_name(sym, name) !strcmp(sym + 3, name + 3)

Instead, you could have:

#ifndef ARCH_HAS_SYSCALL_MATCH_SYM_NAME

static inline arch_syscall_match_sym_name(const char *sym, const char *name)
{
	return strcmp(sym + 3, name + 3) != 0;
}


If an arch needs to make its own, then it can simply override it by
creating its own version and defining:

#define ARCH_HAS_SYSCALL_MATCH_SYM_NAME

Just like they do when an arch has its own strcmp.

This just keeps things cleaner. I like to avoid macros as they can have
nasty side effects (never would have guess that if you look at what I've
done in trace/ftrace.h ;-)

For example, if we use your call as:

arch_syscall_match_sym_name(str = sym[5], name);

That str would end up being something unexpected. I'm not condoning such
side-effect code, but it is something to think about when using macros.

-- Steve


> +#endif
>  
>  #endif /* CONFIG_FTRACE_SYSCALLS */
>  
> diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
> index 33360b9..76bffba 100644
> --- a/kernel/trace/trace_syscalls.c
> +++ b/kernel/trace/trace_syscalls.c
> @@ -72,13 +72,7 @@ static struct syscall_metadata *find_syscall_meta(unsigned long syscall)
>  	kallsyms_lookup(syscall, NULL, NULL, NULL, str);
>  
>  	for ( ; start < stop; start++) {
> -		/*
> -		 * Only compare after the "sys" prefix. Archs that use
> -		 * syscall wrappers may have syscalls symbols aliases prefixed
> -		 * with "SyS" instead of "sys", leading to an unwanted
> -		 * mismatch.
> -		 */
> -		if (start->name && !strcmp(start->name + 3, str + 3))
> +		if (start->name && arch_syscall_match_sym_name(str, start->name))
>  			return start;
>  	}
>  	return NULL;

^ permalink raw reply

* RE: [PATCH 4/6] ftrace syscalls: Allow arch specific syscallsymbol matching
From: David Laight @ 2011-02-02 14:15 UTC (permalink / raw)
  To: Steven Rostedt, Ian Munsie
  Cc: Andreas Dilger, David Gibson, Dave Kleikamp,
	open list:DOCUMENTATION, Jiri Kosina, Namhyung Kim, linux-kernel,
	Alexander Graf, Avantika Mathur, Ingo Molnar, Andreas Schwab,
	KOSAKI Motohiro, Frederic Weisbecker, Scott Wood, Paul Mackerras,
	Andrew Morton, Nathan Lynch, linuxppc-dev, Jason Baron
In-Reply-To: <1296655484.10797.47.camel@gandalf.stny.rr.com>

=20
> +#define arch_syscall_match_sym_name(sym, name) !strcmp(sym + 3, name
+ 3)

Whenever you use a #define macro arg, you should enclose it in ().
About the only time you don't need to is when it is being
passed as an argument to another function
(ie when it's use is also ',' separated).

So the above ought to be:
#define arch_syscall_match_sym_name(sym, name) (!strcmp((sym) + 3,
(name) + 3))

Whether an inline function is better or worse is much more subtle!
For instance I've used:
   asm volatile ( "# line " STR(__LINE__) :: )
to stop gcc merging the tails of conditionals.
Useful when the conditional is at the end of a loop (etc),
it might increase code size slightly, but removes a branch.

If I put one of those in an 'inline' function separate copies
of the function end up sharing code.
With a #define __LINE__ differs so they don't.

(I had some code to get below 190 clocks, these changes
were significant!)

	David

^ permalink raw reply

* RE: [PATCH 4/6] ftrace syscalls: Allow arch specific syscallsymbol matching
From: Steven Rostedt @ 2011-02-02 14:28 UTC (permalink / raw)
  To: David Laight
  Cc: Andreas Dilger, David Gibson, Dave Kleikamp,
	open list:DOCUMENTATION, Jiri Kosina, Namhyung Kim, linux-kernel,
	Alexander Graf, Avantika Mathur, Ingo Molnar, Andreas Schwab,
	Ian Munsie, KOSAKI Motohiro, Frederic Weisbecker, Scott Wood,
	Paul Mackerras, Andrew Morton, Nathan Lynch, linuxppc-dev,
	Jason Baron
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6D8AC3A@saturn3.aculab.com>

On Wed, 2011-02-02 at 14:15 +0000, David Laight wrote:
> > +#define arch_syscall_match_sym_name(sym, name) !strcmp(sym + 3, name
> + 3)
> 
> Whenever you use a #define macro arg, you should enclose it in ().
> About the only time you don't need to is when it is being
> passed as an argument to another function
> (ie when it's use is also ',' separated).
> 
> So the above ought to be:
> #define arch_syscall_match_sym_name(sym, name) (!strcmp((sym) + 3,
> (name) + 3))

I would have mentioned this if I wanted it to stay a macro ;)

> 
> Whether an inline function is better or worse is much more subtle!
> For instance I've used:
>    asm volatile ( "# line " STR(__LINE__) :: )
> to stop gcc merging the tails of conditionals.
> Useful when the conditional is at the end of a loop (etc),
> it might increase code size slightly, but removes a branch.
> 
> If I put one of those in an 'inline' function separate copies
> of the function end up sharing code.
> With a #define __LINE__ differs so they don't.
> 
> (I had some code to get below 190 clocks, these changes
> were significant!)

For what you were doing, this may have helped. But the code in question
is the "default" version of the function. I much more prefer it to be a
static inline. The issues you experience could change from gcc to gcc.
But static inlined functions are much cleaner and easier to read than
macros.

Using a macro for this purpose is just too messy.

Again, look at include/trace/ftrace.h. If I'm saying using a macro is
ugly, then don't use it! Listen to me, because I'm Mr. Ugly Macro Man.

-- Steve

^ permalink raw reply

* 4xx next branch
From: Josh Boyer @ 2011-02-02 14:31 UTC (permalink / raw)
  To: linuxppc-dev

Hi All,

I've added the following patches to my next branch:

Dave Kleikamp (2):
      powerpc/476: define specific cpu table entry DD2 core
      powerpc/476: Workaround for PLB6 hang

Rupjyoti Sarmah (1):
      powerpc/44x: PHY fixup for USB on canyonlands board

Tirumala Marri (1):
      powerpc/44x: Add USB DWC DTS entry to Canyonlands board


There are a few more I'd like to review still, including the AMP support
for 476 before I ask Ben to pull.  The above patches will sit in my
branch for a few days.  If there are any others I have missed, please
let me know.

josh

^ permalink raw reply

* Re: BootX
From: kevin diggs @ 2011-02-02 19:36 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <AANLkTik9ejmu2vE13f_nRZZMspU-LW4y2=74iczoXGNt@mail.gmail.com>

Hi,

FYI:

I have narrowed this down to drivers/scsi/mesh.c (the root disk
controller for the PowerMac 8600). I have compiled everything else for
2.6.36 with 4.3.5, including modules. With a 4.1.2 compiled mesh.c the
beast boots. I am using it to post this follow up.

kevin

> On Sat, Jan 22, 2011 at 12:24 PM, kevin diggs <diggskevin38@gmail.com> wrote:
>> Hi,
>>
>> If I enable SMP then I can build a 2.6.28 kernel with gcc 4.3.5 that
>> WILL boot on the PowerMac8600 (single 750GX). The previously mentioned
>> G4 that runs is a dual cpu beast and thus also runs SMP.
>>

^ permalink raw reply

* Re: BootX
From: Benjamin Herrenschmidt @ 2011-02-02 21:40 UTC (permalink / raw)
  To: kevin diggs; +Cc: linuxppc-dev
In-Reply-To: <AANLkTikGK-7JspQMTiqjXcvoP1FVSt2gZJ9SRqGj=7DO@mail.gmail.com>

On Wed, 2011-02-02 at 13:36 -0600, kevin diggs wrote:
> Hi,
> 
> FYI:
> 
> I have narrowed this down to drivers/scsi/mesh.c (the root disk
> controller for the PowerMac 8600). I have compiled everything else for
> 2.6.36 with 4.3.5, including modules. With a 4.1.2 compiled mesh.c the
> beast boots. I am using it to post this follow up.

That's interesting... That driver is really nasty, we probably have a
bug in it that's exposed by optimizations done by more recent compilers
but it's not going to be trivial to figure out I'm afraid. I at least
have very dim memories of mesh and how it operates...

One thing to be careful of with Mesh is that the DMA engine, while
supposedly cache coherent, has shown in the past to have issues when
DMA'ing to unaligned memory locations. This shouldn't be a problem with
normal block transfers but we may have to be careful with things like
inquiry, mode pages, sense requests etc...

Ben.

> kevin
> 
> > On Sat, Jan 22, 2011 at 12:24 PM, kevin diggs <diggskevin38@gmail.com> wrote:
> >> Hi,
> >>
> >> If I enable SMP then I can build a 2.6.28 kernel with gcc 4.3.5 that
> >> WILL boot on the PowerMac8600 (single 750GX). The previously mentioned
> >> G4 that runs is a dual cpu beast and thus also runs SMP.
> >>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: BootX
From: kevin diggs @ 2011-02-02 21:55 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1296682842.2349.730.camel@pasglop>

Ben,

I know you are VERY busy. I appreciate your taking the time to reply.

Since I'm am still using this thing I'll take a stab at trying to
track it down. I just posted the FYI to see if I could trigger some
thoughts (like your post).

With a 4.3.5 compiled mesh, it fails a lot of early stuff like getting
cache info? I don't remember the full list because it fails to find
the root fs and does the reboot in 180 seconds thing (I still have in
a back corner of my brain the serial console xmon boot stuff and will
probably eventually try that).

I am hopeful that since it (at least so far) always fails that it
might not be THAT bad to track down. That coupled with some knowledge
of what the compilers are doing differently can hopefully help track
it down.

Thanks!

kevin

On Wed, Feb 2, 2011 at 3:40 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>
> That's interesting... That driver is really nasty, we probably have a
> bug in it that's exposed by optimizations done by more recent compilers
> but it's not going to be trivial to figure out I'm afraid. I at least
> have very dim memories of mesh and how it operates...
>
> One thing to be careful of with Mesh is that the DMA engine, while
> supposedly cache coherent, has shown in the past to have issues when
> DMA'ing to unaligned memory locations. This shouldn't be a problem with
> normal block transfers but we may have to be careful with things like
> inquiry, mode pages, sense requests etc...
>
> Ben.
>

^ permalink raw reply

* Question on supporting multiple HW versions with a single driver (warning: long post)
From: Bruce_Leonard @ 2011-02-02 21:57 UTC (permalink / raw)
  To: linuxppc-dev

So this is sort of a follow on question to one I posted a month ago about 
trying to get a PCI driver to work with OF (which I think I more or less 
understood the answer to).  I'm encountering a different sort of problem 
that I'd like to solve with OF but I'm not sure I can.  Let me lay out a 
little background first.

We build embedded systems, so we never really have hot plug events and our 
addresses (at least for HW interfaces) are pretty much static for any 
given product.  In other words for product "A" the NAND controller will 
always be at address "X", though on product "B" that same NAND controller 
may be at address "Y".  Also, the devices in the product are static, i.e., 
we'll always talk to an LXT971 as the PHY.

Currently I'm working on building a driver for an ethernet MAC we're 
putting in an FPGA.  The MAC is based on the MPC8347 TSEC and the driver 
is based on the gianfar driver.  (My previous question was how to spoof 
the OF gianfar driver into thinking it was a PCI driver because our MAC is 
going to be hanging off a PCI bus.  Ultimately I decided to just 
steal...err...borrow... the guts of the gianfar driver and make it a PCI 
driver that only deals with our MAC.)

Right in the middle of writing this driver, my HW guys came to me and said 
they wanted to use this same MAC in other products.  Great I said.  Local 
bus they said.  Which opens up a whole can of worms and leads to my 
question.  We've got a MAC in a FPGA with a nice generic interface on the 
front of it that can talk to a whole range of different busses, PCI, PCIe, 
local bus (of any variety of any processor), etc.  But the internals of 
the MAC (i.e., the register sets, the buffers, the whole buffer descriptor 
mechanism) all looks the same.  Seems to me that this is exactly the sort 
of situation OF and device trees was developed for.

What I'd like to do, and I'm sure it's possible but I have no idea how, is 
to still have this as an OF driver and have the device tree tell the 
kernel about the HW interface to use.  So on one product (currently all 
products use an MCP83xx variant) I would have a child node under a PCI 
node to describe it's interrupts, addressing (which could also come from a 
PCI probe I expect), compatibility, any attached PHYs etc, and on a second 
product do the same thing under a localbus node.

First question that comes to mind is ordering.  If I put a child node in 
the PCI node of the device tree, what happens when the device tree is 
processed?  Is it immediately going to try and find and install a driver 
for that child node?  Since the device tree is processed very early, the 
PCI bus isn't going to be set up and available yet.  Will trying to 
install a PCI driver via OF even be possible at this point?  Then I'd 
still need a PCI function to claim the device when the PCI bus gets 
probed.  If the driver is already installed via OF, what does the PCI 
function do?

Or am I all backwards.  Does having the child node to the PCI node 
actually do anything when the early OF code runs?  If not would the PCI 
probe function be the first indication to the system that the driver needs 
to be loaded?  In which case I just walk the device tree looking 
for...what?  How would I match up the PCI ID with something in the device 
tree?

Then there's the local bus side of the question?  That should truly be an 
OF driver and use struct of_platform_driver along with that whole 
mechanism.  How do I make that compatible with the version of the MAC that 
runs on PCI?

Or am I making a whole lot of work for myself and I should just make them 
separate drivers?  I'm trying to keep the code base as small and coherent 
as possible.  I don't want to have to maintain multiple copies of a driver 
that are essentially identical.

Thanks.

Bruce

^ permalink raw reply

* Re: BootX
From: kevin diggs @ 2011-02-02 22:09 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <AANLkTi=xM5qt4sic-YBD87Znt0vVOyX2yOyhODhtKNqC@mail.gmail.com>

Hi,

And one more thing:  Why does an SMP kernel (mesh compiled in an SMP
enabled kernel) work?

What all does SMP do? If it matters, I'm voluntary preempt.

Is the DMA hardware in this thing used in any other system (I guess I
mean both other computers and other sub-systems in this computer -
does the 53c94 use it? The audio uses it, right?)?

kevin

^ permalink raw reply

* Re: BootX
From: Benjamin Herrenschmidt @ 2011-02-02 22:29 UTC (permalink / raw)
  To: kevin diggs; +Cc: linuxppc-dev
In-Reply-To: <AANLkTin+1pENJ_CNQGoPA=Hisv4qkJWCE01uyZEXipNu@mail.gmail.com>

On Wed, 2011-02-02 at 16:09 -0600, kevin diggs wrote:
> Hi,
> 
> And one more thing:  Why does an SMP kernel (mesh compiled in an SMP
> enabled kernel) work?

Could be an alignment or timing problem, depending on random things the
alignment of some DMA data structures or timing of access might end up
being subtely different.
 
> What all does SMP do? If it matters, I'm voluntary preempt.
> 
> Is the DMA hardware in this thing used in any other system (I guess I
> mean both other computers and other sub-systems in this computer -
> does the 53c94 use it? The audio uses it, right?)?

The DBDMA engine is used in various Apple chips but with more or less HW
bugs in it :-)

To get some more info about the MESH, I suggest you google for a
document called "MacTech.pdf" (Macintosh Technology in the
Common Hardware Reference Platform).

This describes the "MacIO" chip that was designed by Apple for CHRP
machines, which is a successor of the Bandit chip which I think contains
the MESH on your machine. The basic IO cells like MESH are the same (tho
it's possible that the one you have contains more bugs).

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH V2 1/6] powerpc: Move udbg_early_init() after early_init_devtree()
From: David Gibson @ 2011-02-02 23:06 UTC (permalink / raw)
  To: Dave Kleikamp; +Cc: linuxppc-dev
In-Reply-To: <1296586126-32765-2-git-send-email-shaggy@linux.vnet.ibm.com>

On Tue, Feb 01, 2011 at 12:48:41PM -0600, Dave Kleikamp wrote:
> so that it can use information from the device tree.

Hrm.  On the other hand this means that the early_init_devtree() code
can't benefit from hardcoded early debugging.  Since you don't
actually appear to use devtree information in udbg_early_init() in the
latest series, I'd suggest dropping this patch.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH V2 4/6] powerpc/44x: don't use tlbivax on AMP systems
From: David Gibson @ 2011-02-02 23:08 UTC (permalink / raw)
  To: Dave Kleikamp; +Cc: linuxppc-dev
In-Reply-To: <1296586126-32765-5-git-send-email-shaggy@linux.vnet.ibm.com>

On Tue, Feb 01, 2011 at 12:48:44PM -0600, Dave Kleikamp wrote:
> Since other OS's may be running on the other cores don't use tlbivax

[snip]
> +#ifdef CONFIG_44x
> +void __init early_init_mmu_44x(void)
> +{
> +	unsigned long root = of_get_flat_dt_root();
> +	if (of_flat_dt_is_compatible(root, "ibm,47x-AMP"))
> +		amp = 1;
> +}
> +#endif /* CONFIG_44x */

A test against a hardcoded compatible string seems a nasty way to do
this.  Maybe we should define a new boolean property for the root
node.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH V2 4/6] powerpc/44x: don't use tlbivax on AMP systems
From: Dave Kleikamp @ 2011-02-02 23:53 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20110202230844.GG3032@yookeroo>

On Thu, 2011-02-03 at 10:08 +1100, David Gibson wrote:
> On Tue, Feb 01, 2011 at 12:48:44PM -0600, Dave Kleikamp wrote:
> > Since other OS's may be running on the other cores don't use tlbivax
> 
> [snip]
> > +#ifdef CONFIG_44x
> > +void __init early_init_mmu_44x(void)
> > +{
> > +	unsigned long root = of_get_flat_dt_root();
> > +	if (of_flat_dt_is_compatible(root, "ibm,47x-AMP"))
> > +		amp = 1;
> > +}
> > +#endif /* CONFIG_44x */
> 
> A test against a hardcoded compatible string seems a nasty way to do
> this.  Maybe we should define a new boolean property for the root
> node.

I'm not crazy about this string, but I needed something in the device
tree to key off of.  Freescale has something similar (i.e.
MPC8572DS-CAMP), so I chose to follow their example.  I'd be happy to
replace it with a boolean property.  Any objection to just using "amp"?

Thanks,
Shaggy
-- 
Dave Kleikamp
IBM Linux Technology Center

^ permalink raw reply

* Re: [PATCH V2 1/6] powerpc: Move udbg_early_init() after early_init_devtree()
From: Dave Kleikamp @ 2011-02-03  0:00 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20110202230643.GF3032@yookeroo>

On Thu, 2011-02-03 at 10:06 +1100, David Gibson wrote:
> On Tue, Feb 01, 2011 at 12:48:41PM -0600, Dave Kleikamp wrote:
> > so that it can use information from the device tree.
> 
> Hrm.  On the other hand this means that the early_init_devtree() code
> can't benefit from hardcoded early debugging.  Since you don't
> actually appear to use devtree information in udbg_early_init() in the
> latest series, I'd suggest dropping this patch.

Patch 2 depends on early_init_devtree() being run.  Until then, I don't
know of a way to get at the bootargs.
-- 
Dave Kleikamp
IBM Linux Technology Center

^ permalink raw reply

* Re: [PATCH 4/6] ftrace syscalls: Allow arch specific syscall symbol matching
From: Ian Munsie @ 2011-02-03  0:28 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Andreas Dilger, Andreas Schwab, Dave Kleikamp, DOCUMENTATION,
	Namhyung Kim, Jiri Kosina, Jason Baron, linux-kernel,
	Alexander Graf, Ingo Molnar, Paul Mackerras, KOSAKI Motohiro,
	Frederic Weisbecker, Scott Wood, Nathan Lynch, Andrew Morton,
	David Gibson, linuxppc-dev, Avantika Mathur
In-Reply-To: <1296655484.10797.47.camel@gandalf.stny.rr.com>

Excerpts from Steven Rostedt's message of Thu Feb 03 01:04:44 +1100 2011:
> I'll answer your question here.

> > +#define arch_syscall_match_sym_name(sym, name) !strcmp(sym + 3, name + 3)
> 
> Instead, you could have:
> 
> #ifndef ARCH_HAS_SYSCALL_MATCH_SYM_NAME
> 
> static inline arch_syscall_match_sym_name(const char *sym, const char *name)
> {
>     return strcmp(sym + 3, name + 3) != 0;
> }
> 
> 
> If an arch needs to make its own, then it can simply override it by
> creating its own version and defining:
> 
> #define ARCH_HAS_SYSCALL_MATCH_SYM_NAME
> 
> Just like they do when an arch has its own strcmp.

Ok, I've changed it over. Just doing a quick regression test on ppc64 &
x86 then I'll repost.

Cheers,
-Ian

^ permalink raw reply

* [PATCH v2 0/3] powerpc: Open PIC binding and 'no-reset' implementation
From: Meador Inge @ 2011-02-03  1:51 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: devicetree-discuss, Hollis Blanchard

This patch set provides a binding for Open PIC and implements support for
a new property, specified by that binding, called 'no-reset'.  With 'no-reset'
in place the 'protected-sources' property is no longer needed and was removed.

Signed-off-by: Meador Inge <meador_inge@mentor.com>
CC: Hollis Blanchard <hollis_blanchard@mentor.com>

Meador Inge (3):
  powerpc: Removing support for 'protected-sources'
  powerpc: document the Open PIC device tree binding
  powerpc: make MPIC honor the 'no-reset' device tree property

 Documentation/powerpc/dts-bindings/open-pic.txt |  115 +++++++++++++++++++++++
 arch/powerpc/include/asm/mpic.h                 |    7 +-
 arch/powerpc/sysdev/mpic.c                      |   92 ++++++++----------
 3 files changed, 160 insertions(+), 54 deletions(-)
 create mode 100644 Documentation/powerpc/dts-bindings/open-pic.txt

^ permalink raw reply

* [PATCH v2 1/3] powerpc: Removing support for 'protected-sources'
From: Meador Inge @ 2011-02-03  1:51 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: devicetree-discuss, Hollis Blanchard
In-Reply-To: <1296697900-14004-1-git-send-email-meador_inge@mentor.com>

In a recent discussion [1, 2] concerning device trees for AMP systems, the
question of whether we really need 'protected-sources' arose.  The general
consensus was that if you don't want a source to be used, then it should *not*
be mentioned in an 'interrupts' property.  If a source really needs to be
mentioned, then it should be put in a property other than 'interrupts' with
a specific binding for that use case.

[1] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-January/004038.html
[2] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-January/003991.html

Signed-off-by: Meador Inge <meador_inge@mentor.com>
CC: Hollis Blanchard <hollis_blanchard@mentor.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/powerpc/include/asm/mpic.h |    3 ---
 arch/powerpc/sysdev/mpic.c      |   38 --------------------------------------
 2 files changed, 0 insertions(+), 41 deletions(-)

diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h
index e000cce..9b94f18 100644
--- a/arch/powerpc/include/asm/mpic.h
+++ b/arch/powerpc/include/asm/mpic.h
@@ -301,9 +301,6 @@ struct mpic
 	struct mpic_reg_bank	cpuregs[MPIC_MAX_CPUS];
 	struct mpic_reg_bank	isus[MPIC_MAX_ISU];
 
-	/* Protected sources */
-	unsigned long		*protected;
-
 #ifdef CONFIG_MPIC_WEIRD
 	/* Pointer to HW info array */
 	u32			*hw_set;
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 7c13426..a98f41d 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -947,8 +947,6 @@ static int mpic_host_map(struct irq_host *h, unsigned int virq,
 
 	if (hw == mpic->spurious_vec)
 		return -EINVAL;
-	if (mpic->protected && test_bit(hw, mpic->protected))
-		return -EINVAL;
 
 #ifdef CONFIG_SMP
 	else if (hw >= mpic->ipi_vecs[0]) {
@@ -1095,26 +1093,6 @@ struct mpic * __init mpic_alloc(struct device_node *node,
 	if (node && of_get_property(node, "big-endian", NULL) != NULL)
 		mpic->flags |= MPIC_BIG_ENDIAN;
 
-	/* Look for protected sources */
-	if (node) {
-		int psize;
-		unsigned int bits, mapsize;
-		const u32 *psrc =
-			of_get_property(node, "protected-sources", &psize);
-		if (psrc) {
-			psize /= 4;
-			bits = intvec_top + 1;
-			mapsize = BITS_TO_LONGS(bits) * sizeof(unsigned long);
-			mpic->protected = kzalloc(mapsize, GFP_KERNEL);
-			BUG_ON(mpic->protected == NULL);
-			for (i = 0; i < psize; i++) {
-				if (psrc[i] > intvec_top)
-					continue;
-				__set_bit(psrc[i], mpic->protected);
-			}
-		}
-	}
-
 #ifdef CONFIG_MPIC_WEIRD
 	mpic->hw_set = mpic_infos[MPIC_GET_REGSET(flags)];
 #endif
@@ -1321,9 +1299,6 @@ void __init mpic_init(struct mpic *mpic)
 		u32 vecpri = MPIC_VECPRI_MASK | i |
 			(8 << MPIC_VECPRI_PRIORITY_SHIFT);
 		
-		/* check if protected */
-		if (mpic->protected && test_bit(i, mpic->protected))
-			continue;
 		/* init hw */
 		mpic_irq_write(i, MPIC_INFO(IRQ_VECTOR_PRI), vecpri);
 		mpic_irq_write(i, MPIC_INFO(IRQ_DESTINATION), 1 << cpu);
@@ -1492,13 +1467,6 @@ static unsigned int _mpic_get_one_irq(struct mpic *mpic, int reg)
 			mpic_eoi(mpic);
 		return NO_IRQ;
 	}
-	if (unlikely(mpic->protected && test_bit(src, mpic->protected))) {
-		if (printk_ratelimit())
-			printk(KERN_WARNING "%s: Got protected source %d !\n",
-			       mpic->name, (int)src);
-		mpic_eoi(mpic);
-		return NO_IRQ;
-	}
 
 	return irq_linear_revmap(mpic->irqhost, src);
 }
@@ -1532,12 +1500,6 @@ unsigned int mpic_get_coreint_irq(void)
 			mpic_eoi(mpic);
 		return NO_IRQ;
 	}
-	if (unlikely(mpic->protected && test_bit(src, mpic->protected))) {
-		if (printk_ratelimit())
-			printk(KERN_WARNING "%s: Got protected source %d !\n",
-			       mpic->name, (int)src);
-		return NO_IRQ;
-	}
 
 	return irq_linear_revmap(mpic->irqhost, src);
 #else
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH v2 2/3] powerpc: document the Open PIC device tree binding
From: Meador Inge @ 2011-02-03  1:51 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: devicetree-discuss, Hollis Blanchard, Stuart Yoder
In-Reply-To: <1296697900-14004-1-git-send-email-meador_inge@mentor.com>

This binding documents several properties that have been in use for quite
some time, and adds one new property 'no-reset', which controls whether the
Open PIC should be reset during runtime initialization.

The general formatting and interrupt specifier definition is based off of
Stuart Yoder's FSL MPIC binding.

Signed-off-by: Meador Inge <meador_inge@mentor.com>
CC: Hollis Blanchard <hollis_blanchard@mentor.com>
CC: Stuart Yoder <stuart.yoder@freescale.com>
---
 Documentation/powerpc/dts-bindings/open-pic.txt |  115 +++++++++++++++++++++++
 1 files changed, 115 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/powerpc/dts-bindings/open-pic.txt

diff --git a/Documentation/powerpc/dts-bindings/open-pic.txt b/Documentation/powerpc/dts-bindings/open-pic.txt
new file mode 100644
index 0000000..447ef65
--- /dev/null
+++ b/Documentation/powerpc/dts-bindings/open-pic.txt
@@ -0,0 +1,115 @@
+* Open PIC Binding
+
+This binding specifies what properties must be available in the device tree
+representation of an Open PIC compliant interrupt controller.  This binding is
+based on the binding defined for Open PIC in [1] and is a superset of that
+binding.
+
+PROPERTIES
+
+  NOTE: Many of these descriptions were paraphrased here from [1] to aid
+        readability.
+
+  - compatible
+      Usage: required
+      Value type: <string>
+      Definition: Specifies the compatibility list for the PIC.  The
+          property value shall include "open-pic".
+
+  - reg
+      Usage: required
+      Value type: <prop-encoded-array>
+      Definition: Specifies the base physical address(s) and size(s) of this
+          PIC's addressable register space.
+
+  - interrupt-controller
+      Usage: required
+      Value type: <empty>
+      Definition: The presence of this property identifies the node
+          as an Open PIC.  No property value should be defined.
+
+  - #interrupt-cells
+      Usage: required
+      Value type: <u32>
+      Definition: Specifies the number of cells needed to encode an
+          interrupt source.  Shall be 2.
+
+  - #address-cells
+      Usage: required
+      Value type: <u32>
+      Definition: Specifies the number of cells needed to encode an
+          address.  The value of this property shall always be 0.
+          As such, 'interrupt-map' nodes do not have to specify a
+          parent unit address.
+
+  - no-reset
+      Usage: optional
+      Value type: <empty>
+      Definition: The presence of this property indicates that the PIC
+          should not be reset during runtime initialization.  The presence of
+          this property also mandates that any initialization related to
+          interrupt sources shall be limited to sources explicitly referenced
+          in the device tree.
+
+INTERRUPT SPECIFIER DEFINITION
+
+  Interrupt specifiers consists of 2 cells encoded as
+  follows:
+
+   <1st-cell>   interrupt-number
+
+                Identifies the interrupt source.
+
+   <2nd-cell>   level-sense information, encoded as follows:
+                    0 = low-to-high edge triggered
+                    1 = active low level-sensitive
+                    2 = active high level-sensitive
+                    3 = high-to-low edge triggered
+
+EXAMPLE 1
+
+    /*
+     * An Open PIC interrupt controller
+     */
+	mpic: pic@40000 {
+        // This is an interrupt controller node.
+		interrupt-controller;
+
+        // No address cells so that 'interrupt-map' nodes which reference
+        // this Open PIC node do not need a parent address specifier.
+		#address-cells = <0>;
+
+        // Two cells to encode interrupt sources.
+		#interrupt-cells = <2>;
+
+        // Offset address of 0x40000 and size of 0x40000.
+		reg = <0x40000 0x40000>;
+
+        // Compatible with Open PIC.
+		compatible = "open-pic";
+
+        // The PIC should not be reset.
+		no-reset;
+	};
+
+EXAMPLE 2
+
+    /*
+     * An interrupt generating device that is wired to an Open PIC.
+     */
+    serial0: serial@4500 {
+        // Interrupt source '42' that is active high level-sensitive.
+        // Note that there are only two cells as specified in the interrupt
+        // parent's '#interrupt-cells' property.
+        interrupts = <42 2>;
+
+        // The interrupt controller that this device is wired to.
+        interrupt-parent = <&mpic>;
+    };
+
+REFERENCES
+
+[1] Power.org (TM) Standard for Embedded Power Architecture (TM) Platform
+    Requirements (ePAPR), Version 1.0, July 2008.
+    (http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf)
+
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH v2 3/3] powerpc: make MPIC honor the 'no-reset' device tree property
From: Meador Inge @ 2011-02-03  1:51 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: devicetree-discuss, Hollis Blanchard
In-Reply-To: <1296697900-14004-1-git-send-email-meador_inge@mentor.com>

This property, defined in the Open PIC binding, tells the kernel not to use the
reset bit in the global configuration register.  Additionally, its presence
mandates that only sources which are actually used (i.e. appear in the device
tree) should have their VECPRI bits initialized.

Signed-off-by: Meador Inge <meador_inge@mentor.com>
CC: Hollis Blanchard <hollis_blanchard@mentor.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/powerpc/include/asm/mpic.h |    4 ++-
 arch/powerpc/sysdev/mpic.c      |   54 ++++++++++++++++++++++++++++++--------
 2 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h
index 9b94f18..688e3e0 100644
--- a/arch/powerpc/include/asm/mpic.h
+++ b/arch/powerpc/include/asm/mpic.h
@@ -322,6 +322,8 @@ struct mpic
 #ifdef CONFIG_PM
 	struct mpic_irq_save	*save_data;
 #endif
+
+	int cpu;
 };
 
 /*
@@ -333,7 +335,7 @@ struct mpic
  */
 
 /* This is the primary controller, only that one has IPIs and
- * has afinity control. A non-primary MPIC always uses CPU0
+ * has affinity control. A non-primary MPIC always uses CPU0
  * registers only
  */
 #define MPIC_PRIMARY			0x00000001
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index a98f41d..5f17022 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -308,6 +308,15 @@ static inline void mpic_map(struct mpic *mpic, struct device_node *node,
 #define mpic_map(m,n,p,b,o,s)	_mpic_map_mmio(m,p,b,o,s)
 #endif /* !CONFIG_PPC_DCR */
 
+static inline void mpic_init_vector(struct mpic *mpic, int source)
+{
+	/* start with vector = source number, and masked */
+	u32 vecpri = MPIC_VECPRI_MASK | source | (8 << MPIC_VECPRI_PRIORITY_SHIFT);
+		
+	/* init hw */
+	mpic_irq_write(source, MPIC_INFO(IRQ_VECTOR_PRI), vecpri);
+	mpic_irq_write(source, MPIC_INFO(IRQ_DESTINATION), 1 << mpic->cpu);
+}
 
 
 /* Check if we have one of those nice broken MPICs with a flipped endian on
@@ -622,6 +631,14 @@ static unsigned int mpic_is_ipi(struct mpic *mpic, unsigned int irq)
 	return (src >= mpic->ipi_vecs[0] && src <= mpic->ipi_vecs[3]);
 }
 
+/* Determine if the linux irq is a timer interrupt */
+static unsigned int mpic_is_timer_interrupt(struct mpic *mpic, unsigned int irq)
+{
+	unsigned int src = mpic_irq_to_hw(irq);
+
+	return (src >= mpic->timer_vecs[0] && src <= mpic->timer_vecs[3]);
+}
+
 
 /* Convert a cpu mask from logical to physical cpu numbers. */
 static inline u32 mpic_physmask(u32 cpumask)
@@ -963,6 +980,15 @@ static int mpic_host_map(struct irq_host *h, unsigned int virq,
 	if (hw >= mpic->irq_count)
 		return -EINVAL;
 
+	/* If the MPIC was reset, then all vectors have already been
+	 * initialized.  Otherwise, the appropriate vector needs to be
+	 * initialized here to ensure that only used sources are setup with
+	 * a vector.
+	 */
+	if (!(mpic->flags & MPIC_WANTS_RESET))
+		if (!(mpic_is_ipi(mpic, hw) || mpic_is_timer_interrupt(mpic, hw)))
+			mpic_init_vector(mpic, hw);
+
 	mpic_msi_reserve_hwirq(mpic, hw);
 
 	/* Default chip */
@@ -1129,7 +1155,16 @@ struct mpic * __init mpic_alloc(struct device_node *node,
 	mpic_map(mpic, node, paddr, &mpic->tmregs, MPIC_INFO(TIMER_BASE), 0x1000);
 
 	/* Reset */
-	if (flags & MPIC_WANTS_RESET) {
+
+	/* When using a device-node, reset requests are only honored if the MPIC
+	 * is allowed to reset.
+	 */
+	if (node && of_get_property(node, "no-reset", NULL)) {
+		mpic->flags &= ~MPIC_WANTS_RESET;
+	}
+
+	if (mpic->flags & MPIC_WANTS_RESET) {
+		printk(KERN_DEBUG "mpic: Resetting\n");
 		mpic_write(mpic->gregs, MPIC_INFO(GREG_GLOBAL_CONF_0),
 			   mpic_read(mpic->gregs, MPIC_INFO(GREG_GLOBAL_CONF_0))
 			   | MPIC_GREG_GCONF_RESET);
@@ -1246,7 +1281,6 @@ void __init mpic_set_default_senses(struct mpic *mpic, u8 *senses, int count)
 void __init mpic_init(struct mpic *mpic)
 {
 	int i;
-	int cpu;
 
 	BUG_ON(mpic->num_sources == 0);
 
@@ -1290,18 +1324,14 @@ void __init mpic_init(struct mpic *mpic)
 	mpic_pasemi_msi_init(mpic);
 
 	if (mpic->flags & MPIC_PRIMARY)
-		cpu = hard_smp_processor_id();
+		mpic->cpu = hard_smp_processor_id();
 	else
-		cpu = 0;
+		mpic->cpu = 0;
 
-	for (i = 0; i < mpic->num_sources; i++) {
-		/* start with vector = source number, and masked */
-		u32 vecpri = MPIC_VECPRI_MASK | i |
-			(8 << MPIC_VECPRI_PRIORITY_SHIFT);
-		
-		/* init hw */
-		mpic_irq_write(i, MPIC_INFO(IRQ_VECTOR_PRI), vecpri);
-		mpic_irq_write(i, MPIC_INFO(IRQ_DESTINATION), 1 << cpu);
+	if (mpic->flags & MPIC_WANTS_RESET) {
+		for (i = 0; i < mpic->num_sources; i++) {
+			mpic_init_vector(mpic, i);
+		}
 	}
 	
 	/* Init spurious vector */
-- 
1.6.3.3

^ permalink raw reply related

* PowerPC, ftrace: Add PPC raw syscall tracepoints & ftrace fixes (mimimal subset only) v4
From: Ian Munsie @ 2011-02-03  3:27 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andreas Dilger, Andreas Schwab, Dave Kleikamp, Namhyung Kim,
	Jiri Kosina, Jason Baron, Alexander Graf, Steven Rostedt,
	David Gibson, Ingo Molnar, Paul Mackerras, KOSAKI Motohiro,
	Frederic Weisbecker, Scott Wood, Nathan Lynch, Andrew Morton,
	linuxppc-dev, Avantika Mathur

Hi All,

This is a partial version of my 'ftrace syscalls, PowerPC: Various fixes,
Compat Syscall support and PowerPC implementation'. This is updated from
yesterday with arch_syscall_addr changed to a static inline function from
Steven's suggestion.

This subset implements the raw syscall tracepoints on PowerPC which has been
requested recently.  It also fixes ftrace syscalls to ensure that events will
only be created for syscalls that successfully map their metadata to a syscall
number, so that non-working phantom events are not created. Patches #2 and #6
in this series are not strictly necessary for this, they just optimise ftrace
syscalls a bit.

What's missing from this series that was in the full 40 patch v2 series is the
conversion of all the syscalls implemented under /arch/powerpc, Jason Baron's
compat syscall support and the conversion of the remaining native and compat
syscalls to this infrastructure.

Cheers,
-Ian


Changelog:
Subset v4:
- Changed arch_syscall_addr to a static inline function from Steven's
  suggestion. Archs implementing their own function must now define
  ARCH_HAS_SYSCALL_MATCH_SYM_NAME in their asm/ftrace.h

Subset v3:
- Rather than removing the redundant syscall_nr checks completely, I have
  turned them into WARN_ON_ONCE to catch possible future regressions, from
  Steven Rostedt's suggestion.
- From Mike Frysinger's suggestion, arch_syscall_addr is now a macro rather
  than a weak function to minimise the overhead at boot. Archs with special
  requirements (such as ppc64) can define their own macro in asm/ftrace.h.
  Steven Rostedt suggested this be made a static inline function, but I don't
  see how this would be possible (at least without #defines and #ifndefs) given
  that it has to be weak to allow archs to override it (Unless I misunderstood
  something?  Steven?).

Subset v2:
- Minimal unchanged subset from 'ftrace syscalls, PowerPC: Various fixes,
  Compat Syscall support and PowerPC implementation' v2 patch series.

^ permalink raw reply

* [PATCH 1/6] ftrace syscalls: don't add events for unmapped syscalls
From: Ian Munsie @ 2011-02-03  3:27 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andreas Dilger, Andreas Schwab, Dave Kleikamp, Namhyung Kim,
	Jiri Kosina, Jason Baron, Alexander Graf, Steven Rostedt,
	David Gibson, Ingo Molnar, Paul Mackerras, Ian Munsie,
	KOSAKI Motohiro, Frederic Weisbecker, Scott Wood, Nathan Lynch,
	Andrew Morton, linuxppc-dev, Avantika Mathur
In-Reply-To: <1296703645-18718-1-git-send-email-imunsie@au1.ibm.com>

From: Ian Munsie <imunsie@au1.ibm.com>

FTRACE_SYSCALLS would create events for each and every system call, even
if it had failed to map the system call's name with it's number. This
resulted in a number of events being created that would not behave as
expected.

This could happen, for example, on architectures who's symbol names are
unusual and will not match the system call name. It could also happen
with system calls which were mapped to sys_ni_syscall.

This patch changes the default system call number in the metadata to -1.
If the system call name from the metadata is not successfully mapped to
a system call number during boot, than the event initialisation routine
will now return an error, preventing the event from being created.

Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
 include/linux/syscalls.h      |    2 ++
 kernel/trace/trace_syscalls.c |    8 ++++++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 18cd068..2e5a68d 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -160,6 +160,7 @@ extern struct trace_event_functions exit_syscall_print_funcs;
 	  __attribute__((section("__syscalls_metadata")))	\
 	  __syscall_meta_##sname = {				\
 		.name 		= "sys"#sname,			\
+		.syscall_nr	= -1,	/* Filled in at boot */	\
 		.nb_args 	= nb,				\
 		.types		= types_##sname,		\
 		.args		= args_##sname,			\
@@ -176,6 +177,7 @@ extern struct trace_event_functions exit_syscall_print_funcs;
 	  __attribute__((section("__syscalls_metadata")))	\
 	  __syscall_meta__##sname = {				\
 		.name 		= "sys_"#sname,			\
+		.syscall_nr	= -1,	/* Filled in at boot */	\
 		.nb_args 	= 0,				\
 		.enter_event	= &event_enter__##sname,	\
 		.exit_event	= &event_exit__##sname,		\
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index b706529..a66bc13 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -423,6 +423,14 @@ void unreg_event_syscall_exit(struct ftrace_event_call *call)
 int init_syscall_trace(struct ftrace_event_call *call)
 {
 	int id;
+	int num;
+
+	num = ((struct syscall_metadata *)call->data)->syscall_nr;
+	if (num < 0 || num >= NR_syscalls) {
+		pr_debug("syscall %s metadata not mapped, disabling ftrace event\n",
+				((struct syscall_metadata *)call->data)->name);
+		return -ENOSYS;
+	}
 
 	if (set_syscall_print_fmt(call) < 0)
 		return -ENOMEM;
-- 
1.7.2.3

^ permalink raw reply related

* [PATCH 2/6] ftrace syscalls: Convert redundant syscall_nr checks into WARN_ON
From: Ian Munsie @ 2011-02-03  3:27 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andreas Dilger, Andreas Schwab, Dave Kleikamp, Namhyung Kim,
	Jiri Kosina, Jason Baron, Alexander Graf, Steven Rostedt,
	David Gibson, Ingo Molnar, Paul Mackerras, Ian Munsie,
	KOSAKI Motohiro, Frederic Weisbecker, Scott Wood, Nathan Lynch,
	Andrew Morton, linuxppc-dev, Avantika Mathur
In-Reply-To: <1296703645-18718-1-git-send-email-imunsie@au1.ibm.com>

From: Ian Munsie <imunsie@au1.ibm.com>

With the ftrace events now checking if the syscall_nr is valid upon
initialisation it should no longer be possible to register or unregister
a syscall event without a valid syscall_nr since they should not be
created. This adds a WARN_ON_ONCE in the register and unregister
functions to locate potential regressions in the future.

Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
 kernel/trace/trace_syscalls.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index a66bc13..1a6e8dd 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -358,7 +358,7 @@ int reg_event_syscall_enter(struct ftrace_event_call *call)
 	int num;
 
 	num = ((struct syscall_metadata *)call->data)->syscall_nr;
-	if (num < 0 || num >= NR_syscalls)
+	if (WARN_ON_ONCE(num < 0 || num >= NR_syscalls))
 		return -ENOSYS;
 	mutex_lock(&syscall_trace_lock);
 	if (!sys_refcount_enter)
@@ -376,7 +376,7 @@ void unreg_event_syscall_enter(struct ftrace_event_call *call)
 	int num;
 
 	num = ((struct syscall_metadata *)call->data)->syscall_nr;
-	if (num < 0 || num >= NR_syscalls)
+	if (WARN_ON_ONCE(num < 0 || num >= NR_syscalls))
 		return;
 	mutex_lock(&syscall_trace_lock);
 	sys_refcount_enter--;
@@ -392,7 +392,7 @@ int reg_event_syscall_exit(struct ftrace_event_call *call)
 	int num;
 
 	num = ((struct syscall_metadata *)call->data)->syscall_nr;
-	if (num < 0 || num >= NR_syscalls)
+	if (WARN_ON_ONCE(num < 0 || num >= NR_syscalls))
 		return -ENOSYS;
 	mutex_lock(&syscall_trace_lock);
 	if (!sys_refcount_exit)
@@ -410,7 +410,7 @@ void unreg_event_syscall_exit(struct ftrace_event_call *call)
 	int num;
 
 	num = ((struct syscall_metadata *)call->data)->syscall_nr;
-	if (num < 0 || num >= NR_syscalls)
+	if (WARN_ON_ONCE(num < 0 || num >= NR_syscalls))
 		return;
 	mutex_lock(&syscall_trace_lock);
 	sys_refcount_exit--;
-- 
1.7.2.3

^ permalink raw reply related

* [PATCH 3/6] ftrace syscalls: Make arch_syscall_addr weak
From: Ian Munsie @ 2011-02-03  3:27 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andreas Dilger, Andreas Schwab, Dave Kleikamp,
	open list:DOCUMENTATION, Namhyung Kim, Jiri Kosina, Jason Baron,
	Alexander Graf, Steven Rostedt, David Gibson, Ingo Molnar,
	Paul Mackerras, Ian Munsie, KOSAKI Motohiro, Frederic Weisbecker,
	Scott Wood, Nathan Lynch, Andrew Morton, linuxppc-dev,
	Avantika Mathur
In-Reply-To: <1296703645-18718-1-git-send-email-imunsie@au1.ibm.com>

From: Ian Munsie <imunsie@au1.ibm.com>

Some architectures use non-trivial system call tables and will not work
with the generic arch_syscall_addr code. For example, PowerPC64 uses a
table of twin long longs.

This patch makes the generic arch_syscall_addr weak to allow
architectures with non-trivial system call tables to override it.

Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
 Documentation/trace/ftrace-design.txt |    3 +++
 kernel/trace/trace_syscalls.c         |    2 +-
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/Documentation/trace/ftrace-design.txt b/Documentation/trace/ftrace-design.txt
index dc52bd4..6fca17b 100644
--- a/Documentation/trace/ftrace-design.txt
+++ b/Documentation/trace/ftrace-design.txt
@@ -247,6 +247,9 @@ You need very few things to get the syscalls tracing in an arch.
 - Support the TIF_SYSCALL_TRACEPOINT thread flags.
 - Put the trace_sys_enter() and trace_sys_exit() tracepoints calls from ptrace
   in the ptrace syscalls tracing path.
+- If the system call table on this arch is more complicated than a simple array
+  of addresses of the system calls, implement an arch_syscall_addr to return
+  the address of a given system call.
 - Tag this arch as HAVE_SYSCALL_TRACEPOINTS.
 
 
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 1a6e8dd..33360b9 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -445,7 +445,7 @@ int init_syscall_trace(struct ftrace_event_call *call)
 	return id;
 }
 
-unsigned long __init arch_syscall_addr(int nr)
+unsigned long __init __weak arch_syscall_addr(int nr)
 {
 	return (unsigned long)sys_call_table[nr];
 }
-- 
1.7.2.3

^ permalink raw reply related


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