* Re: MPC5121e, Linux, simple IO ports
From: CF Studelec @ 2013-03-21 13:44 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <514ACAD6.6010909@studelec-sa.com>
Sorry to bump the post, i'm wondering if it was correctly sent.
Le 21/03/2013 09:54, CF Studelec a écrit :
> Hello,
>
> This is a simple board base on mpc5121e MCU.
>
>
> Gpio is detected: kernel is compiled with its support - i got
> gpiochip_find_base: found new base @224 in dmesg - on kernel 3.0.4.
>
>
> But i'm unable to access it through /sys/class/gpio. I can successfully
> export a pin (ie, if i type cat 224 > export, gpio224 is created), but i
> can't successfully control it:
>
>
>
> echo "out" > /sys/class/gpio/gpio224/direction
>
> cat /sys/class/gpio/gpio224/value
>
> 0
>
> echo 1 > /sys/class/gpio/gpio224/value
>
> cat /sys/class/gpio/gpio224/value
>
> 0
>
>
>
> My need is a simple chipselect (well, 3 chipselect exactly ), for a
> custom design. Running linux is mandatory. I suspect the MPC5121e to be
> in the bad function mode, so:
>
> - does anyone knows how to change mode ? What register can i acces and
> how ? does it worths a try ?
>
> - does anyone successfully performed simple IO control ?
>
>
>
> Last thing, i have tryed to access internal registers through /dev/mem,
> but no success. There are very few ressources available for this
> microcontroler, but i'm stick to it. Perhaps anybody knows how to access
> (read) internal registers with /proc or sys-fs ?
>
>
>
> Thank you for your help.
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
^ permalink raw reply
* Re: [PATCH] powerpc/uprobes: teach uprobes to ignore gdb breakpoints
From: Oleg Nesterov @ 2013-03-21 15:58 UTC (permalink / raw)
To: Ananth N Mavinakayanahalli; +Cc: ppcdev, Srikar Dronamraju, stable
In-Reply-To: <20130321071545.GA5271@in.ibm.com>
On 03/21, Ananth N Mavinakayanahalli wrote:
>
> On Wed, Mar 20, 2013 at 05:06:44PM +0100, Oleg Nesterov wrote:
>
> > > > But we did not install UPROBE_SWBP_INSN. Is it fine? I hope yes, just to
> > > > verify. If not, we need 2 definitions. is_uprobe_insn() should still check
> > > > insns == UPROBE_SWBP_INSN, and is_swbp_insn() should check is_trap().
>
> Its fine from gdb's perspective with my patch.
Yes, but this doesn't look right from uprobe's perspective.
> > > So, install_breakpoint()->prepare_uprobe()->is_swbp_insn() will return
> > > ENOTSUPP. In fact, arch_uprobe_analyze_insn() will also do the same.
> >
> > Yes and the check in arch_uprobe_analyze_insn() should go away.
> >
> > But you missed my point. Please forget about prepare_uprobe(), it is
> > wrong anyway. And, prepare_uprobe() inspects the _original_ insn from
> > the file, this has nothing install_breakpoint/etc.
> >
> > I meant verify_opcode() called by install_breakpoint/etc.
>
> For the case where X already exists, verify_opcode() currently returns 0.
> IMO, it should return -EEXIST,
Oh, this is debatable. Currently we assume that uprobe should "win".
See below...
> unless you are proposing that uprobes
> should ride on the existing trap (even if its a variant).
Yes. And this is what the current implementation does.
> If you are proposing that uprobes ride on X if it already exists, that's
> not always possible and is a big can of worms... see below...
Sure. Whatever we do uprobe and gdb can confuse each other. Unless we
rework the vm code completely (not sure this is realistic).
> > OK. So, if I understand correctly, gdb can use some conditional
> > breakpoint, and it is possible that this insn won't generate the
> > trap?
>
> Yes it is possible if the condition is not met. If the condition is
> met, the instruction will generate a trap, and uprobes will do a
> send_sig(SIGTRAP) from handle_swbp().
Unless there is uprobe at the same address. Once again, uprobe wins.
Your patch only fixes the case when the task hits a non-UPROBE_SWBP_INSN
breakpoint and there is no uprobe at the same address.
> > Then this patch is not right, or at least we need another change
> > on top?
> >
> > Once again. Suppose that gdb installs the TRAP_IF_R1_GT_R2.
> >
> > After that uprobe_register() is called, but it won't change this
> > insn because verify_opcode() returns 0.
> >
> > Then the probed task hits this breakoint with "r1 < r2" and we do
> > not report this event.
>
> At this time, the condition for the trap is not satisfied, so no
> exception occurs.
Yes, and thus uprobe->handler() is not called, this was my point.
> If the expectation is that the trap always trigger,
> then all such trap variants need to be replaced with the unconditional
> trap
Yes. that is why I suggested the patch which doesn't affect verify_opcode().
uprobe_register() should replace the conditional trap with the unconditional
UPROBE_SWBP_INSN. uprobes should win.
> and we should either add logic to re-execute the condional trap
> after uprobe handling and send_sig() via handle_swbp() or emulate the
> condition in software and do a send_sig() if needed.
Unlikely this is possible. Or even desirable.
> > So. I still think that we actually need something like below, and
> > powerpc should reimplement is_trap_insn() to use is_trap(insn).
> >
> > No?
>
> I don't see how this will help,
Hmm. This should equally fix this particular problem? handle_swbp()
will send the trap if is_trap() == T?
Again, unless there is uprobe, but this was always true.
> especially since the gdb<->uprobes is
> fraught with races.
They can race anyway, whatever we do.
Unless we rework write_opcode/etc completely.
> With your proposed patch, we refuse to insert a uprobe if the underlying
> instruction is a UPROBE_SWBP_INSTRUCTION;
If "underlying" means the original insn in vma->file, this is already
true. My patch doesn't change this logic.
Otherwise - no, we do not refuse to insert a uprobe if this insn was
already changed by gdb.
> changing is_swbp_at_addr()
> will need changes in handle_swbp() too.
I don't think so. Why?
> But, unlike x86, we cannot
> expect a uprobe with an underlying trap variant (X) to always trigger.
And that is why I think write_opcode() should rewrite the conditional
trap.
> IMHO, its not a good idea to do that for x86 either,
This change has no effect fo x86.
> IMHO, I really think we should not allow uprobe_register() to succeed if
> the underlying instruction is a breakpoint (or a variant thereof).
I disagree.
Just for example. Suppose we change install_breakpoint() so that it fails
if the underlying instruction is int3. (once again, "underlying" does not
mean the original insn from vma->vm_file).
First of all, this is very much nontrivial. I simply do not see how we
can do this. If nothing else, uprobe_register() can race with uprobe_mmap()
and install_breakpoint() can be called twice with the same vaddr. With
this change register or mmap can fail.
But suppose you can do this. Now you can write the trivial application
which mmaps glibc and inserts int3 into, say, getpid()'s vaddr. Voila,
this makes "perf probe -x /lib/libc.so.6" impossible, uprobe_register()
will fail.
Whatever you think about this logic, it was desidned to assume that
install_breakpoint() should be "idempotent", and we ignore the races
with gdb. We should only ensure that the kernel can't crash/etc.
And uprobe can "steal" the trap from gdb if they race, again this is by
design. and your patch can't prevent this but complicates the rules.
I already said this many times, but let me repeat. is_swbp_isn() and
its usage is confusing. Lets forget about prepare_uprobe(). Now,
- verify_opcode()->is_swbp_insn() means:
is this insn fine for uprobe? (we do not care about
gdb, we simply ignore this problem)
- is_swbp_at_addr()->is_swbp_insn() means:
there is no uprobe, should we send SIGTRAP ?
And the patch I sent separates these 2 cases.
Finally. If we want to eliminate the gdb/uprobes races/confusions,
we can not simply use a PageAnon() page, we shuld rewrite this code
completely. I can quote the very old email I sent you:
The proper fix, I think, is to rework the whole idea about uprobe bps,
but this is really "in the long term". install_breakpoint() should
only unmap the page and mark its pte as "owned by kernel, FOLL_WRITE
should not work". Something like migration or PROT_NONE. The task
itself should install bp during the page fault. And we need the
"backing store" for the pages with uprobes. Yes, this all is very
vague and I can be wrong.
IOW, somehow we should ensure that once uprobe changes the page, nobody
else can change it until uprobe_unregister().
Oleg.
^ permalink raw reply
* Re: [PATCH] powerpc/uprobes: teach uprobes to ignore gdb breakpoints
From: Oleg Nesterov @ 2013-03-21 16:00 UTC (permalink / raw)
To: Ananth N Mavinakayanahalli; +Cc: ppcdev, Srikar Dronamraju, stable
In-Reply-To: <20130321071707.GB5271@in.ibm.com>
On 03/21, Ananth N Mavinakayanahalli wrote:
?
> On Wed, Mar 20, 2013 at 05:07:28PM +0100, Oleg Nesterov wrote:
> > On 03/20, Ananth N Mavinakayanahalli wrote:
> > >
> > > On Wed, Mar 20, 2013 at 01:43:01PM +0100, Oleg Nesterov wrote:
> > > > On 03/20, Oleg Nesterov wrote:
> > > > >
> > > > IOW, if I wasn't clear... Lets forget about gdb/etc for the moment.
> > > > Suppose we apply the patch below. Will uprobes on powerpc work?
> > > >
> > > > If yes, then your patch should be fine. If not, we probably need more
> > > > changes.
> > >
> > > Yes, it will work fine.
> >
> > Even if this new insn is conditional?
>
> Yes.
But this can't be true. If it is conditional, it won't always generate a
trap, this means uprobe won't actually work.
Oleg.
^ permalink raw reply
* Re: [PATCH] powerpc/dts: Add qe support for 36bit
From: Timur Tabi @ 2013-03-21 18:42 UTC (permalink / raw)
To: Zhicheng Fan; +Cc: linuxppc-dev
In-Reply-To: <1363857961-23296-1-git-send-email-B32736@freescale.com>
On Thu, Mar 21, 2013 at 4:26 AM, Zhicheng Fan <B32736@freescale.com> wrote:
> + qe: qe@fffe80000 {
> + ranges = <0x0 0xf 0xffe80000 0x40000>;
Are you sure this works? The QE can't handle 36-bit addresses, and I
think the MURAM is located inside the QE address space.
^ permalink raw reply
* Re: [Suggestion] PowerPC: kernel: cross compiling issue with allmodconfig
From: Michael Neuling @ 2013-03-21 22:54 UTC (permalink / raw)
To: Chen Gang
Cc: sfr, matt, linux-kernel@vger.kernel.org, paulus@samba.org,
imunsie, linuxppc-dev
In-Reply-To: <514AA0D9.1090509@asianux.com>
Chen Gang <gang.chen@asianux.com> wrote:
> Hello All:
>=20
> summary:
> the root cause is no enough room in exception area (0x5500 -- 0x7000).
>=20
> it is caused by the patches "for saving/restre PPR":
> they consumed much space of this area (0x5500 -- 0x7000).
> for pseries_defconfig and ppc64_defconfig, it is still ok.
> but for allmodconfig and "some additional config", it will cause issu=
e.
>=20
> the solving patch "Make room in exception vector area" can make room la=
rger.
> it can let "some additional config" ok.
> but for allmodconfig, it is still not enough.
>=20
>=20
> details
> reason:
> it is caused by:
> commit number: 13e7a8e846c2ea38a552b986ea49332f965bbb7a
> commit number: 44e9309f1f357794b7ae93d5f3e3e6f11d2b8a7f
> they are "for saving/restore PPR"
> by Haren Myneni <haren@linux.vnet.ibm.com> Thu, 6 Dec 2012
> compiling result:
> pseries_defconfig: pass (cpu for POWER7)
> ppc64_defconfig: pass (cpu for POWER7)
> allmodconfig: failed (cpu for POWER7)
>=20
> analysing:
> solving patch:
> ------------------------------------------------------------------
> commit number: 61383407677aef05928541a00678591abea2d84c
> Author: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Thu Jan 10 17:44:19 2013 +1100
>=20
> powerpc: Make room in exception vector area
>=20=20=20=20=20
> The FWNMI region is fixed at 0x7000 and the vector are now
> overflowing that with some configurations. Fix that by moving
> some hash management code out of that region as it doesn't need
> to be that close to the call sites (isn't accessed using
> conditional branches).
> ------------------------------------------------------------------
>=20
> but for allmodconfig (not only for "some configurations"):
> it really can reduce much overflow bytes,
> (maybe from hundreds bytes to dozens bytes)
> but still not enough (still content overflow bytes)
>=20
> additional trying:
> after del CONFIG_VSX and CONFIG_PPC_970_NAP in allmodconfig,
> (will reduce dozens bytes in the region .0x5500 -- .0x7000)
> it can pass compiling (not overflow).
>=20
>=20
> next:
> I am sorry:
> I am not quite familiar with the detail features of powerpc.
> it seems I am not the suitable member to continue trying.
>=20
> I prefer Benjamin to continue trying (just like what he has done).
>=20
> if Benjamin will not do it (e.g. maybe no time to do)
> I should continue: "make additional room in exception vector area".
> (if get no reply within a week: before 2013-03-28, I should continu=
e)
>=20
>=20
>=20
> welcome any members' (especially Benjamin) suggestions or completions.
This is great, thanks a lot.=20=20
If you want this to be picked up by the maintainer, you'll need to add
your signed-off-by.
The signed-off-by is to indicate that your happy for it to be included
and that you're legally allowed to do so. See
http://gerrit.googlecode.com/svn/documentation/2.0/user-signedoffby.html
for more info.
Mikey
>=20
> thanks.
>=20
> :-)
>=20
>=20
> On 2013=E5=B9=B403=E6=9C=8815=E6=97=A5 13:14, Chen Gang wrote:
> > =E4=BA=8E 2013=E5=B9=B403=E6=9C=8815=E6=97=A5 12:52, Michael Neuling =
=E5=86=99=E9=81=93:
> >> Yep it's a known problem but no one has bothered to fix it since it
> >> doesn't happen in a config that anyone cares about like
> >> pseries_defconfig and ppc64_defconfig. We've been moving code around =
in
> >> this area a lot recently hence the breakage.
> >>
> >> It should be fixed though. Patches welcome. :-)
> >=20
> > thanks, and I should try, and very glad to try.
> >=20
> > :-) :-)
> >=20
> > excuse me, I try to provide related patch within this month (2013-03-=
31), is it ok ?
> > the reason is:
> > I am not familiar with ppc assembly code, neither ppc kernel,
> > so need additional time resource.
> > (originally, I worked for x86(_64) core dump analysing for kernel=
and user programs)
> >=20
> > thanks.
> >=20
>=20
>=20
> --=20
> Chen Gang
>=20
> Asianux Corporation
>=20
^ permalink raw reply
* Re: [Suggestion] PowerPC: kernel: cross compiling issue with allmodconfig
From: Michael Neuling @ 2013-03-21 23:21 UTC (permalink / raw)
To: Chen Gang F T, Stuart Yoder, Kumar Gala
Cc: sfr, matt, Chen Gang, linux-kernel@vger.kernel.org,
paulus@samba.org, imunsie, linuxppc-dev
In-Reply-To: <514AC418.1070806@gmail.com>
Chen Gang F T <chen.gang.flying.transformer@gmail.com> wrote:
>=20
> it seems:
> only move slb_miss_realmode to the end, can fix this issue without ne=
gative effect.
So this works but uncovers a new bug.
Stuart: if you apply this patch allmodconfig now gets this build error
on linux-next.
arch/powerpc/kernel/built-in.o: In function `restore_pblist_ptr':
ftrace.c:(.toc+0xd78): undefined reference to `epapr_ev_idle_start'
ftrace.c:(.toc+0xd88): undefined reference to `epapr_ev_idle'
make[1]: *** [vmlinux] Error 1
Reverting your patch below fixes this:
commit f070986a07e514e3b4fc4aef6551b8dffcb19287
Author: Stuart Yoder <stuart.yoder@freescale.com>
Date: Fri Feb 8 13:22:56 2013 -0600
powerpc: Add paravirt idle loop for 64-bit Book-E
Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Should we revert your patch or can you send a fix? This is currently
broken in linux-next (at least next-20130321).
Mikey
>=20
>=20
>=20
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/e=
xceptions-64s.S
> index 200afa5..56bd923 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -1066,78 +1066,6 @@ unrecov_user_slb:
> #endif /* __DISABLED__ */
>=20=20
>=20=20
> -/*
> - * r13 points to the PACA, r9 contains the saved CR,
> - * r12 contain the saved SRR1, SRR0 is still ready for return
> - * r3 has the faulting address
> - * r9 - r13 are saved in paca->exslb.
> - * r3 is saved in paca->slb_r3
> - * We assume we aren't going to take any exceptions during this procedur=
e.
> - */
> -_GLOBAL(slb_miss_realmode)
> - mflr r10
> -#ifdef CONFIG_RELOCATABLE
> - mtctr r11
> -#endif
> -
> - stw r9,PACA_EXSLB+EX_CCR(r13) /* save CR in exc. frame */
> - std r10,PACA_EXSLB+EX_LR(r13) /* save LR */
> -
> - bl .slb_allocate_realmode
> -
> - /* All done -- return from exception. */
> -
> - ld r10,PACA_EXSLB+EX_LR(r13)
> - ld r3,PACA_EXSLB+EX_R3(r13)
> - lwz r9,PACA_EXSLB+EX_CCR(r13) /* get saved CR */
> -
> - mtlr r10
> -
> - andi. r10,r12,MSR_RI /* check for unrecoverable exception */
> - beq- 2f
> -
> -.machine push
> -.machine "power4"
> - mtcrf 0x80,r9
> - mtcrf 0x01,r9 /* slb_allocate uses cr0 and cr7 */
> -.machine pop
> -
> - RESTORE_PPR_PACA(PACA_EXSLB, r9)
> - ld r9,PACA_EXSLB+EX_R9(r13)
> - ld r10,PACA_EXSLB+EX_R10(r13)
> - ld r11,PACA_EXSLB+EX_R11(r13)
> - ld r12,PACA_EXSLB+EX_R12(r13)
> - ld r13,PACA_EXSLB+EX_R13(r13)
> - rfid
> - b . /* prevent speculative execution */
> -
> -2: mfspr r11,SPRN_SRR0
> - ld r10,PACAKBASE(r13)
> - LOAD_HANDLER(r10,unrecov_slb)
> - mtspr SPRN_SRR0,r10
> - ld r10,PACAKMSR(r13)
> - mtspr SPRN_SRR1,r10
> - rfid
> - b .
> -
> -unrecov_slb:
> - EXCEPTION_PROLOG_COMMON(0x4100, PACA_EXSLB)
> - DISABLE_INTS
> - bl .save_nvgprs
> -1: addi r3,r1,STACK_FRAME_OVERHEAD
> - bl .unrecoverable_exception
> - b 1b
> -
> -
> -#ifdef CONFIG_PPC_970_NAP
> -power4_fixup_nap:
> - andc r9,r9,r10
> - std r9,TI_LOCAL_FLAGS(r11)
> - ld r10,_LINK(r1) /* make idle task do the */
> - std r10,_NIP(r1) /* equivalent of a blr */
> - blr
> -#endif
> -
> .align 7
> .globl alignment_common
> alignment_common:
> @@ -1336,6 +1264,78 @@ _GLOBAL(opal_mc_secondary_handler)
>=20=20
>=20=20
> /*
> + * r13 points to the PACA, r9 contains the saved CR,
> + * r12 contain the saved SRR1, SRR0 is still ready for return
> + * r3 has the faulting address
> + * r9 - r13 are saved in paca->exslb.
> + * r3 is saved in paca->slb_r3
> + * We assume we aren't going to take any exceptions during this procedur=
e.
> + */
> +_GLOBAL(slb_miss_realmode)
> + mflr r10
> +#ifdef CONFIG_RELOCATABLE
> + mtctr r11
> +#endif
> +
> + stw r9,PACA_EXSLB+EX_CCR(r13) /* save CR in exc. frame */
> + std r10,PACA_EXSLB+EX_LR(r13) /* save LR */
> +
> + bl .slb_allocate_realmode
> +
> + /* All done -- return from exception. */
> +
> + ld r10,PACA_EXSLB+EX_LR(r13)
> + ld r3,PACA_EXSLB+EX_R3(r13)
> + lwz r9,PACA_EXSLB+EX_CCR(r13) /* get saved CR */
> +
> + mtlr r10
> +
> + andi. r10,r12,MSR_RI /* check for unrecoverable exception */
> + beq- 2f
> +
> +.machine push
> +.machine "power4"
> + mtcrf 0x80,r9
> + mtcrf 0x01,r9 /* slb_allocate uses cr0 and cr7 */
> +.machine pop
> +
> + RESTORE_PPR_PACA(PACA_EXSLB, r9)
> + ld r9,PACA_EXSLB+EX_R9(r13)
> + ld r10,PACA_EXSLB+EX_R10(r13)
> + ld r11,PACA_EXSLB+EX_R11(r13)
> + ld r12,PACA_EXSLB+EX_R12(r13)
> + ld r13,PACA_EXSLB+EX_R13(r13)
> + rfid
> + b . /* prevent speculative execution */
> +
> +2: mfspr r11,SPRN_SRR0
> + ld r10,PACAKBASE(r13)
> + LOAD_HANDLER(r10,unrecov_slb)
> + mtspr SPRN_SRR0,r10
> + ld r10,PACAKMSR(r13)
> + mtspr SPRN_SRR1,r10
> + rfid
> + b .
> +
> +unrecov_slb:
> + EXCEPTION_PROLOG_COMMON(0x4100, PACA_EXSLB)
> + DISABLE_INTS
> + bl .save_nvgprs
> +1: addi r3,r1,STACK_FRAME_OVERHEAD
> + bl .unrecoverable_exception
> + b 1b
> +
> +
> +#ifdef CONFIG_PPC_970_NAP
> +power4_fixup_nap:
> + andc r9,r9,r10
> + std r9,TI_LOCAL_FLAGS(r11)
> + ld r10,_LINK(r1) /* make idle task do the */
> + std r10,_NIP(r1) /* equivalent of a blr */
> + blr
> +#endif
> +
> +/*
> * Hash table stuff
> */
> .align 7
>=20
>=20
> On 2013=E5=B9=B403=E6=9C=8821=E6=97=A5 13:55, Chen Gang wrote:
> > Hello All:
> >=20
> > summary:
> > the root cause is no enough room in exception area (0x5500 -- 0x7000).
> >=20
> > it is caused by the patches "for saving/restre PPR":
> > they consumed much space of this area (0x5500 -- 0x7000).
> > for pseries_defconfig and ppc64_defconfig, it is still ok.
> > but for allmodconfig and "some additional config", it will cause is=
sue.
> >=20
> > the solving patch "Make room in exception vector area" can make room =
larger.
> > it can let "some additional config" ok.
> > but for allmodconfig, it is still not enough.
> >=20
> >=20
> > details
> > reason:
> > it is caused by:
> > commit number: 13e7a8e846c2ea38a552b986ea49332f965bbb7a
> > commit number: 44e9309f1f357794b7ae93d5f3e3e6f11d2b8a7f
> > they are "for saving/restore PPR"
> > by Haren Myneni <haren@linux.vnet.ibm.com> Thu, 6 Dec 2012
> > compiling result:
> > pseries_defconfig: pass (cpu for POWER7)
> > ppc64_defconfig: pass (cpu for POWER7)
> > allmodconfig: failed (cpu for POWER7)
> >=20
> > analysing:
> > solving patch:
> > ------------------------------------------------------------------
> > commit number: 61383407677aef05928541a00678591abea2d84c
> > Author: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Date: Thu Jan 10 17:44:19 2013 +1100
> >=20
> > powerpc: Make room in exception vector area
> >=20=20=20=20=20
> > The FWNMI region is fixed at 0x7000 and the vector are now
> > overflowing that with some configurations. Fix that by moving
> > some hash management code out of that region as it doesn't need
> > to be that close to the call sites (isn't accessed using
> > conditional branches).
> > ------------------------------------------------------------------
> >=20
> > but for allmodconfig (not only for "some configurations"):
> > it really can reduce much overflow bytes,
> > (maybe from hundreds bytes to dozens bytes)
> > but still not enough (still content overflow bytes)
> >=20
> > additional trying:
> > after del CONFIG_VSX and CONFIG_PPC_970_NAP in allmodconfig,
> > (will reduce dozens bytes in the region .0x5500 -- .0x7000)
> > it can pass compiling (not overflow).
> >=20
> >=20
> > next:
> > I am sorry:
> > I am not quite familiar with the detail features of powerpc.
> > it seems I am not the suitable member to continue trying.
> >=20
> > I prefer Benjamin to continue trying (just like what he has done).
> >=20
> > if Benjamin will not do it (e.g. maybe no time to do)
> > I should continue: "make additional room in exception vector area".
> > (if get no reply within a week: before 2013-03-28, I should conti=
nue)
> >=20
> >=20
> >=20
> > welcome any members' (especially Benjamin) suggestions or completions.
> >=20
> > thanks.
> >=20
> > :-)
> >=20
> >=20
> > On 2013=E5=B9=B403=E6=9C=8815=E6=97=A5 13:14, Chen Gang wrote:
> >> =E4=BA=8E 2013=E5=B9=B403=E6=9C=8815=E6=97=A5 12:52, Michael Neuling =
=E5=86=99=E9=81=93:
> >>> Yep it's a known problem but no one has bothered to fix it since it
> >>> doesn't happen in a config that anyone cares about like
> >>> pseries_defconfig and ppc64_defconfig. We've been moving code around=
in
> >>> this area a lot recently hence the breakage.
> >>>
> >>> It should be fixed though. Patches welcome. :-)
> >>
> >> thanks, and I should try, and very glad to try.
> >>
> >> :-) :-)
> >>
> >> excuse me, I try to provide related patch within this month (2013-03=
-31), is it ok ?
> >> the reason is:
> >> I am not familiar with ppc assembly code, neither ppc kernel,
> >> so need additional time resource.
> >> (originally, I worked for x86(_64) core dump analysing for kerne=
l and user programs)
> >>
> >> thanks.
> >>
> >=20
> >=20
>=20
>=20
> --=20
> Chen Gang
>=20
> Flying Transformer
>=20
^ permalink raw reply
* [PATCH] kvm: (ppc) stub out more functions in order to permit kernel to build
From: Phil Carmody @ 2013-03-21 23:59 UTC (permalink / raw)
To: sfr; +Cc: Phil Carmody, khilman, Linuxppc-dev
Build failure mentioned by Stephen Rothwell here:
http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg65408.html
Traced to:
commit f445f11eb2cc -"KVM: allow host header to be included even for !CONFIG_KVM"
This patch permits the build to progress by stubbing out the two functions
that it fell over on. I have no idea if that's even vaguely appropriate.
This has been .c->.o compile tested only, as g5_defconfig FTB elsewhere.
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Kevin Hilman <khilman@linaro.org>
Signed-off-by: Phil Carmody <pc+lkml@asdf.org>
---
arch/powerpc/include/asm/kvm_ppc.h | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index 44a657a..262d9b7 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -20,6 +20,8 @@
#ifndef __POWERPC_KVM_PPC_H__
#define __POWERPC_KVM_PPC_H__
+#if IS_ENABLED(CONFIG_KVM)
+
/* This file exists just so we can dereference kvm_vcpu, avoiding nested header
* dependencies. */
@@ -324,4 +326,9 @@ static inline ulong kvmppc_get_ea_indexed(struct kvm_vcpu *vcpu, int ra, int rb)
return ea;
}
+#else
+static inline void kvmppc_set_xics_phys(int cpu, unsigned long addr) { return; }
+static inline void kvm_linear_init(void) { return; }
+#endif /* IS_ENABLED(CONFIG_KVM) */
+
#endif /* __POWERPC_KVM_PPC_H__ */
--
1.7.2.5
^ permalink raw reply related
* [PATCH] KVM: fix powerpc build error for !CONFIG_KVM
From: Stephen Rothwell @ 2013-03-22 3:35 UTC (permalink / raw)
To: Kevin Hilman
Cc: Phil Carmody, Gleb Natapov, Marcelo Tosatti, LKML, linux-next,
Paul Mackerras, linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 2909 bytes --]
Fixes these build error when CONFIG_KVM is not defined:
In file included from arch/powerpc/include/asm/kvm_ppc.h:33:0,
from arch/powerpc/kernel/setup_64.c:67:
arch/powerpc/include/asm/kvm_book3s.h:65:20: error: field 'pte' has incomplete type
arch/powerpc/include/asm/kvm_book3s.h:69:18: error: field 'vcpu' has incomplete type
arch/powerpc/include/asm/kvm_book3s.h:98:34: error: 'HPTEG_HASH_NUM_PTE' undeclared here (not in a function)
arch/powerpc/include/asm/kvm_book3s.h:99:39: error: 'HPTEG_HASH_NUM_PTE_LONG' undeclared here (not in a function)
arch/powerpc/include/asm/kvm_book3s.h:100:35: error: 'HPTEG_HASH_NUM_VPTE' undeclared here (not in a function)
arch/powerpc/include/asm/kvm_book3s.h:101:40: error: 'HPTEG_HASH_NUM_VPTE_LONG' undeclared here (not in a function)
arch/powerpc/include/asm/kvm_book3s.h:129:4: error: 'struct kvm_run' declared inside parameter list [-Werror]
arch/powerpc/include/asm/kvm_book3s.h:129:4: error: its scope is only this definition or declaration, which is probably not what you want [-Werror]
... and so on ...
This was introduced by commit f445f11eb2cc265dd47da5b2e864df46cd6e5a82
"KVM: allow host header to be included even for !CONFIG_KVM"
Cc: Kevin Hilman <khilman@linaro.org>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
include/linux/kvm_host.h | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
Kevin, does this still fix the error that commit
f445f11eb2cc265dd47da5b2e864df46cd6e5a82 was fixing?
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index a942863..90ebec0 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1,8 +1,6 @@
#ifndef __KVM_HOST_H
#define __KVM_HOST_H
-#if IS_ENABLED(CONFIG_KVM)
-
/*
* This work is licensed under the terms of the GNU GPL, version 2. See
* the COPYING file in the top-level directory.
@@ -751,6 +749,7 @@ static inline int kvm_deassign_device(struct kvm *kvm,
}
#endif /* CONFIG_IOMMU_API */
+#if IS_ENABLED(CONFIG_KVM)
static inline void __guest_enter(void)
{
/*
@@ -770,6 +769,10 @@ static inline void __guest_exit(void)
vtime_account_system(current);
current->flags &= ~PF_VCPU;
}
+#else
+static inline void __guest_enter(void) { return; }
+static inline void __guest_exit(void) { return; }
+#endif /* IS_ENABLED(CONFIG_KVM) */
#ifdef CONFIG_CONTEXT_TRACKING
extern void guest_enter(void);
@@ -1057,8 +1060,4 @@ static inline bool kvm_vcpu_eligible_for_directed_yield(struct kvm_vcpu *vcpu)
}
#endif /* CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT */
-#else
-static inline void __guest_enter(void) { return; }
-static inline void __guest_exit(void) { return; }
-#endif /* IS_ENABLED(CONFIG_KVM) */
#endif
--
1.8.1
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply related
* Re: [PATCH 4/11] Add platform_has_feature()
From: Michael Ellerman @ 2013-03-22 3:56 UTC (permalink / raw)
To: Nathan Fontenot; +Cc: linuxppc-dev
In-Reply-To: <5148AB26.4060705@linux.vnet.ibm.com>
On Tue, Mar 19, 2013 at 01:15:02PM -0500, Nathan Fontenot wrote:
> On 03/14/2013 08:42 AM, Michael Ellerman wrote:
> > On Fri, Mar 08, 2013 at 10:02:31PM -0600, Nathan Fontenot wrote:
> >> The firmware_has_feature() function makes it easy to check for supported
> >> features of the hardware. There is not corresponding function to check for
> >> features supported by the client architecture.
> >
> > Actually it doesn't tell you about features of the hardware, it tells
> > you about features of the firmware, or the platform ..
> >
> > So I think you should really just be adding a new firmware feature flag,
> > and adding whatever glue code is required to set it based on what you
> > find in the device tree.
> >
> > Also notice where you end up using it:
> >
> > - if (firmware_has_feature(FW_FEATURE_OPAL))
> > + if (firmware_has_feature(FW_FEATURE_OPAL) ||
> > + platform_has_feature(OV5_TYPE1_AFFINITY)) {
> > + dbg("Using form 1 affinity\n");
> > form1_affinity = 1;
> >
> > Could be:
> >
> > + if (firmware_has_feature(FW_FEATURE_FORM1_AFFINITY) ||
> >
>
> To make sure I understand what you're suggesting...
>
> You think there should be a single firmware_has_feature() for all current
> uses and also for checking items such as FORM1_AFFINITY and PRRN
> features as reported by the device tree for vector 5 portions of the
> client architecture bits. I think this could be done by checking the
> device tree ibm,architecture-vec-5 node for a specified feature and
> setting a bit the appropriate bit in powerpc_firmware_features.
Yes that's right.
So you'd add a new FW_FEATURE_FORM1_AFFINITY, and set it depending on what
you find in "ibm,architecture-vec-5" on IBM pseries machines.
As a cleanup you'd also set it unconditionally on OPAL and then the
check above could be simply for FW_FEATURE_FORM1_AFFINITY.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: Add accounting for Doorbell interrupts
From: Michael Ellerman @ 2013-03-22 4:18 UTC (permalink / raw)
To: Ian Munsie; +Cc: Michael Neuling, linuxppc-dev
In-Reply-To: <1363765332-28461-1-git-send-email-imunsie@au1.ibm.com>
On Wed, Mar 20, 2013 at 06:42:12PM +1100, Ian Munsie wrote:
> From: Ian Munsie <imunsie@au1.ibm.com>
>
> This patch adds a new line to /proc/interrupts to account for the
> doorbell interrupts that each hardware thread has received. The total
> interrupt count in /proc/stat will now also include doorbells.
Should it be inside CONFIG_PPC_DOORBELL ?
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: drop even more unused Kconfig symbols
From: Michael Ellerman @ 2013-03-22 4:21 UTC (permalink / raw)
To: Paul Bolle; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <1363864206.1390.117.camel@x61.thuisdomein>
On Thu, Mar 21, 2013 at 12:10:06PM +0100, Paul Bolle wrote:
> When I submitted commit 6805ab6daa2b589fe3242d05ddc47a9dbb0c4eb1
> ("powerpc: drop unused Kconfig symbols") I apparently failed to notice
> that my patch also made PREP_RESIDUAL and PPC_A2_DD2 unused. Drop these
> now.
>
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> ---
> 0) Untested.
>
> 1) I investigated these Kconfig files a bit and discovered that PPC_PREP
> is marked BROKEN since v2.6.15, see commit
> 5be396b00ca0f2f769c55cf69bbd7c77451c925e ("powerpc: Mark PREP and
> embedded as broken for now"). Though it's not my problem, this does
> suggest PReP support can be removed entirely.
It does, and the best code is deleted code.
Care to send a patch?
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/uprobes: teach uprobes to ignore gdb breakpoints
From: Ananth N Mavinakayanahalli @ 2013-03-22 4:37 UTC (permalink / raw)
To: Oleg Nesterov; +Cc: ppcdev, Srikar Dronamraju, stable
In-Reply-To: <20130321160002.GB20865@redhat.com>
On Thu, Mar 21, 2013 at 05:00:02PM +0100, Oleg Nesterov wrote:
> On 03/21, Ananth N Mavinakayanahalli wrote:
> ?
> > On Wed, Mar 20, 2013 at 05:07:28PM +0100, Oleg Nesterov wrote:
> > > On 03/20, Ananth N Mavinakayanahalli wrote:
> > > >
> > > > On Wed, Mar 20, 2013 at 01:43:01PM +0100, Oleg Nesterov wrote:
> > > > > On 03/20, Oleg Nesterov wrote:
> > > > > >
> > > > > IOW, if I wasn't clear... Lets forget about gdb/etc for the moment.
> > > > > Suppose we apply the patch below. Will uprobes on powerpc work?
> > > > >
> > > > > If yes, then your patch should be fine. If not, we probably need more
> > > > > changes.
> > > >
> > > > Yes, it will work fine.
> > >
> > > Even if this new insn is conditional?
> >
> > Yes.
>
> But this can't be true. If it is conditional, it won't always generate a
> trap, this means uprobe won't actually work.
I meant to say, uprobes if we use a conditional trap instruction as long
as the condition is met.
Ananth
^ permalink raw reply
* Re: [PATCH] powerpc/uprobes: teach uprobes to ignore gdb breakpoints
From: Ananth N Mavinakayanahalli @ 2013-03-22 4:47 UTC (permalink / raw)
To: Oleg Nesterov; +Cc: ppcdev, Srikar Dronamraju, stable
In-Reply-To: <20130321155809.GA20865@redhat.com>
On Thu, Mar 21, 2013 at 04:58:09PM +0100, Oleg Nesterov wrote:
> On 03/21, Ananth N Mavinakayanahalli wrote:
> >
> > On Wed, Mar 20, 2013 at 05:06:44PM +0100, Oleg Nesterov wrote:
> >
> > > > > But we did not install UPROBE_SWBP_INSN. Is it fine? I hope yes, just to
> > > > > verify. If not, we need 2 definitions. is_uprobe_insn() should still check
> > > > > insns == UPROBE_SWBP_INSN, and is_swbp_insn() should check is_trap().
> >
> > Its fine from gdb's perspective with my patch.
>
> Yes, but this doesn't look right from uprobe's perspective.
>
> > > > So, install_breakpoint()->prepare_uprobe()->is_swbp_insn() will return
> > > > ENOTSUPP. In fact, arch_uprobe_analyze_insn() will also do the same.
> > >
> > > Yes and the check in arch_uprobe_analyze_insn() should go away.
> > >
> > > But you missed my point. Please forget about prepare_uprobe(), it is
> > > wrong anyway. And, prepare_uprobe() inspects the _original_ insn from
> > > the file, this has nothing install_breakpoint/etc.
> > >
> > > I meant verify_opcode() called by install_breakpoint/etc.
> >
> > For the case where X already exists, verify_opcode() currently returns 0.
> > IMO, it should return -EEXIST,
>
> Oh, this is debatable. Currently we assume that uprobe should "win".
And there is that one case where gdb also uses an unconditional trap...
but that's besides the point if we don't care about gdb.
> See below...
>
> > unless you are proposing that uprobes
> > should ride on the existing trap (even if its a variant).
>
> Yes. And this is what the current implementation does.
>
> > If you are proposing that uprobes ride on X if it already exists, that's
> > not always possible and is a big can of worms... see below...
>
> Sure. Whatever we do uprobe and gdb can confuse each other. Unless we
> rework the vm code completely (not sure this is realistic).
Agreed.
> > > OK. So, if I understand correctly, gdb can use some conditional
> > > breakpoint, and it is possible that this insn won't generate the
> > > trap?
> >
> > Yes it is possible if the condition is not met. If the condition is
> > met, the instruction will generate a trap, and uprobes will do a
> > send_sig(SIGTRAP) from handle_swbp().
>
> Unless there is uprobe at the same address. Once again, uprobe wins.
In which case, we will need to replace the conditional trap with the
unconditional one when the uprobe register happens. That is doable.
> Your patch only fixes the case when the task hits a non-UPROBE_SWBP_INSN
> breakpoint and there is no uprobe at the same address.
Correct.
> > > Then this patch is not right, or at least we need another change
> > > on top?
> > >
> > > Once again. Suppose that gdb installs the TRAP_IF_R1_GT_R2.
> > >
> > > After that uprobe_register() is called, but it won't change this
> > > insn because verify_opcode() returns 0.
> > >
> > > Then the probed task hits this breakoint with "r1 < r2" and we do
> > > not report this event.
> >
> > At this time, the condition for the trap is not satisfied, so no
> > exception occurs.
>
> Yes, and thus uprobe->handler() is not called, this was my point.
>
> > If the expectation is that the trap always trigger,
> > then all such trap variants need to be replaced with the unconditional
> > trap
>
> Yes. that is why I suggested the patch which doesn't affect verify_opcode().
> uprobe_register() should replace the conditional trap with the unconditional
> UPROBE_SWBP_INSN. uprobes should win.
OK, I see your point.
...
> > But, unlike x86, we cannot
> > expect a uprobe with an underlying trap variant (X) to always trigger.
>
> And that is why I think write_opcode() should rewrite the conditional
> trap.
OK
> > IMHO, its not a good idea to do that for x86 either,
>
> This change has no effect fo x86.
>
> > IMHO, I really think we should not allow uprobe_register() to succeed if
> > the underlying instruction is a breakpoint (or a variant thereof).
>
> I disagree.
>
> Just for example. Suppose we change install_breakpoint() so that it fails
> if the underlying instruction is int3. (once again, "underlying" does not
> mean the original insn from vma->vm_file).
>
> First of all, this is very much nontrivial. I simply do not see how we
> can do this. If nothing else, uprobe_register() can race with uprobe_mmap()
> and install_breakpoint() can be called twice with the same vaddr. With
> this change register or mmap can fail.
>
> But suppose you can do this. Now you can write the trivial application
> which mmaps glibc and inserts int3 into, say, getpid()'s vaddr. Voila,
> this makes "perf probe -x /lib/libc.so.6" impossible, uprobe_register()
> will fail.
>
> Whatever you think about this logic, it was desidned to assume that
> install_breakpoint() should be "idempotent", and we ignore the races
> with gdb. We should only ensure that the kernel can't crash/etc.
>
> And uprobe can "steal" the trap from gdb if they race, again this is by
> design. and your patch can't prevent this but complicates the rules.
>
> I already said this many times, but let me repeat. is_swbp_isn() and
> its usage is confusing. Lets forget about prepare_uprobe(). Now,
>
> - verify_opcode()->is_swbp_insn() means:
>
> is this insn fine for uprobe? (we do not care about
> gdb, we simply ignore this problem)
I will write up a patch for this case.. So, IIUC we do not care to send
gdb a SIGTRAP if we have replaced a conditional trap from gdb with an
unconditional uprobes one, right?
> - is_swbp_at_addr()->is_swbp_insn() means:
>
> there is no uprobe, should we send SIGTRAP ?
This part is handled with my patch now...
Thanks for being patient. I'll write up the patches and send across for
review.
Ananth.
^ permalink raw reply
* Re: [PATCH] powerpc: Add accounting for Doorbell interrupts
From: Ian Munsie @ 2013-03-22 5:13 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Michael Neuling, linuxppc-dev
In-Reply-To: <20130322041803.GB26908@concordia>
Excerpts from Michael Ellerman's message of 2013-03-22 15:18:03 +1100:
> On Wed, Mar 20, 2013 at 06:42:12PM +1100, Ian Munsie wrote:
> > From: Ian Munsie <imunsie@au1.ibm.com>
> >
> > This patch adds a new line to /proc/interrupts to account for the
> > doorbell interrupts that each hardware thread has received. The total
> > interrupt count in /proc/stat will now also include doorbells.
>
> Should it be inside CONFIG_PPC_DOORBELL ?
The doorbell line will only show up in /proc/interrupt if CPU_FTR_DBELL
is set and the total interrupt count in /proc/stat will be unaffected
if we don't have doorbells (+ 0 doorbells). It still builds fine without
CONFIG_PPC_DOORBELL.
Adding #ifdefs would save one per cpu integer in irq_stat. I can throw
together a v2 patch with them in place.
Cheers,
-Ian
^ permalink raw reply
* [PATCH v2] powerpc: Add accounting for Doorbell interrupts
From: Ian Munsie @ 2013-03-22 5:22 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Michael Neuling, linuxppc-dev, Ian Munsie
In-Reply-To: <1363928529-sup-704@delenn.ozlabs.ibm.com>
From: Ian Munsie <imunsie@au1.ibm.com>
This patch adds a new line to /proc/interrupts to account for the
doorbell interrupts that each hardware thread has received. The total
interrupt count in /proc/stat will now also include doorbells.
# cat /proc/interrupts
CPU0 CPU1 CPU2 CPU3
16: 551 1267 281 175 XICS Level IPI
LOC: 2037 1503 1688 1625 Local timer interrupts
SPU: 0 0 0 0 Spurious interrupts
CNT: 0 0 0 0 Performance monitoring interrupts
MCE: 0 0 0 0 Machine check exceptions
DBL: 42 550 20 91 Doorbell interrupts
Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
Changes from v1:
- Added #ifdef CONFIG_PPC_DOORBELL as requested by Michael Ellerman to avoid
the extra per cpu variable in irq_stat when building for a system that does
not support doorbells.
arch/powerpc/include/asm/hardirq.h | 3 +++
arch/powerpc/kernel/dbell.c | 2 ++
arch/powerpc/kernel/irq.c | 12 ++++++++++++
3 files changed, 17 insertions(+)
diff --git a/arch/powerpc/include/asm/hardirq.h b/arch/powerpc/include/asm/hardirq.h
index 3147a29..3bdcfce 100644
--- a/arch/powerpc/include/asm/hardirq.h
+++ b/arch/powerpc/include/asm/hardirq.h
@@ -10,6 +10,9 @@ typedef struct {
unsigned int pmu_irqs;
unsigned int mce_exceptions;
unsigned int spurious_irqs;
+#ifdef CONFIG_PPC_DOORBELL
+ unsigned int doorbell_irqs;
+#endif
} ____cacheline_aligned irq_cpustat_t;
DECLARE_PER_CPU_SHARED_ALIGNED(irq_cpustat_t, irq_stat);
diff --git a/arch/powerpc/kernel/dbell.c b/arch/powerpc/kernel/dbell.c
index 9ebbc24..d55c76c 100644
--- a/arch/powerpc/kernel/dbell.c
+++ b/arch/powerpc/kernel/dbell.c
@@ -41,6 +41,8 @@ void doorbell_exception(struct pt_regs *regs)
may_hard_irq_enable();
+ __get_cpu_var(irq_stat).doorbell_irqs++;
+
smp_ipi_demux();
irq_exit();
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 4f97fe3..5cbcf4d 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -374,6 +374,15 @@ int arch_show_interrupts(struct seq_file *p, int prec)
seq_printf(p, "%10u ", per_cpu(irq_stat, j).mce_exceptions);
seq_printf(p, " Machine check exceptions\n");
+#ifdef CONFIG_PPC_DOORBELL
+ if (cpu_has_feature(CPU_FTR_DBELL)) {
+ seq_printf(p, "%*s: ", prec, "DBL");
+ for_each_online_cpu(j)
+ seq_printf(p, "%10u ", per_cpu(irq_stat, j).doorbell_irqs);
+ seq_printf(p, " Doorbell interrupts\n");
+ }
+#endif
+
return 0;
}
@@ -387,6 +396,9 @@ u64 arch_irq_stat_cpu(unsigned int cpu)
sum += per_cpu(irq_stat, cpu).pmu_irqs;
sum += per_cpu(irq_stat, cpu).mce_exceptions;
sum += per_cpu(irq_stat, cpu).spurious_irqs;
+#ifdef CONFIG_PPC_DOORBELL
+ sum += per_cpu(irq_stat, cpu).doorbell_irqs;
+#endif
return sum;
}
--
1.7.10.4
^ permalink raw reply related
* RE: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
From: Wang Dongsheng-B40534 @ 2013-03-22 5:46 UTC (permalink / raw)
To: Wood Scott-B07421
Cc: Gala Kumar-B11780, linuxppc-dev@lists.ozlabs.org, Li Yang-R58472,
Zhao Chenhui-B35336
In-Reply-To: <1363816138.25034.21@snotra>
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, March 21, 2013 5:49 AM
> To: Wang Dongsheng-B40534
> Cc: Wood Scott-B07421; Gala Kumar-B11780; linuxppc-dev@lists.ozlabs.org;
> Zhao Chenhui-B35336; Li Yang-R58472
> Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
>=20
> On 03/19/2013 10:48:53 PM, Wang Dongsheng-B40534 wrote:
> >
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Wednesday, March 20, 2013 6:55 AM
> > > To: Wang Dongsheng-B40534
> > > Cc: Wood Scott-B07421; Gala Kumar-B11780;
> > linuxppc-dev@lists.ozlabs.org;
> > > Zhao Chenhui-B35336; Li Yang-R58472
> > > Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
> > >
> > > On 03/19/2013 01:25:42 AM, Wang Dongsheng-B40534 wrote:
> > > > > -----Original Message-----
> > > > > From: Wood Scott-B07421
> > > > > Sent: Tuesday, March 19, 2013 8:31 AM
> > > > > To: Wang Dongsheng-B40534
> > > > > Cc: Gala Kumar-B11780; linuxppc-dev@lists.ozlabs.org; Wang
> > > > Dongsheng-
> > > > > B40534; Zhao Chenhui-B35336; Li Yang-R58472
> > > > > Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup
> > support
> > > > >
> > > > > On 03/08/2013 01:38:47 AM, Wang Dongsheng wrote:
> > > > > > +static ssize_t fsl_timer_wakeup_store(struct device *dev,
> > > > > > + struct device_attribute *attr,
> > > > > > + const char *buf,
> > > > > > + size_t count)
> > > > > > +{
> > > > > > + struct timeval interval;
> > > > > > + int ret;
> > > > > > +
> > > > > > + interval.tv_usec =3D 0;
> > > > > > + if (kstrtol(buf, 0, &interval.tv_sec))
> > > > > > + return -EINVAL;
> > > > >
> > > > > I don't think the buffer will NUL-terminated... Ordinarily
> > > > there'll be
> > > > > an LF terminator, but you can't rely on that (many other sysfs
> > > > attributes
> > > > > seem to, though...).
> > > > >
> > > > I think we don't need to care about LF terminator.
> > > > The kstrtol--> _kstrtoull has been done.
> > >
> > > My point is, what happens if userspace passes in a buffer that has
> > no
> > > terminator of any sort? kstrtol will continue reading beyond the
> > end of
> > > the buffer.
> > >
> > Do not care about terminator.
>=20
> kstrtol() obviously *does* because it doesn't take the buffer length as
> a parameter.
>=20
> > kstrtol--> _kstrtoull--> _parse_integer
> >
> > _kstrtoull(...) {
> > ...
> > rv =3D _parse_integer(s, base, &_res);
> > if (rv & KSTRTOX_OVERFLOW)
> > return -ERANGE;
> > rv &=3D ~KSTRTOX_OVERFLOW;
> > if (rv =3D=3D 0)
> > return -EINVAL;
> > s +=3D rv;
> >
> > if (*s =3D=3D '\n')
> > s++;
> > if (*s)
> > return -EINVAL;
> > ...
> > }
> >
> > _parse_integer(...) {
> > ...
> > while (*s) {
> > if ('0' <=3D *s && *s <=3D '9')
> > val =3D *s - '0';
> > else if ('a' <=3D _tolower(*s) && _tolower(*s) <=3D 'f')
> > val =3D _tolower(*s) - 'a' + 10;
> > else
> > break; //this will break out to convert.
>=20
> Really? How do you know that the next byte after the buffer isn't a
> valid hex digit? How do you even know that we won't take a fault
> accessing it?
>=20
Under what case is unsafe, please make sense.
"kstrtol" is used in almost of sysfs interface, I think it should be accept=
ed in defaule :).
^ permalink raw reply
* [PATCH] powerpc/ptrace: Add DAWR debug feature info for userspace
From: Michael Neuling @ 2013-03-22 6:12 UTC (permalink / raw)
To: benh; +Cc: Linux PPC dev, Edjunior Barbosa Machado, David Gibson
This adds new debug feature information so that the DAWR can be
identified by userspace tools like GDB.
Unfortunately the DAWR doesn't sit nicely into the current description
that ptrace provides to userspace via struct ppc_debug_info. It doesn't
allow for specifying that only some ranges are possible or even the end
alignment constraints (DAWR only allows 512 byte wide ranges which can't
cross a 512 byte boundary).
After talking to Edjunior Machado (GDB ppc developer), it was decided
this was the best approach. Just mark it as debug feature DAWR and
tools like GDB can internally decide the constraints.
Signed-off-by: Michael Neuling <mikey@neuling.org>
diff --git a/Documentation/powerpc/ptrace.txt b/Documentation/powerpc/ptrace.txt
index f2a7a39..99c5ce8 100644
--- a/Documentation/powerpc/ptrace.txt
+++ b/Documentation/powerpc/ptrace.txt
@@ -40,6 +40,7 @@ features will have bits indicating whether there is support for:
#define PPC_DEBUG_FEATURE_INSN_BP_MASK 0x2
#define PPC_DEBUG_FEATURE_DATA_BP_RANGE 0x4
#define PPC_DEBUG_FEATURE_DATA_BP_MASK 0x8
+#define PPC_DEBUG_FEATURE_DATA_BP_DAWR 0x10
2. PTRACE_SETHWDEBUG
diff --git a/arch/powerpc/include/uapi/asm/ptrace.h b/arch/powerpc/include/uapi/asm/ptrace.h
index 66b9ca4..77d2ed3 100644
--- a/arch/powerpc/include/uapi/asm/ptrace.h
+++ b/arch/powerpc/include/uapi/asm/ptrace.h
@@ -211,6 +211,7 @@ struct ppc_debug_info {
#define PPC_DEBUG_FEATURE_INSN_BP_MASK 0x0000000000000002
#define PPC_DEBUG_FEATURE_DATA_BP_RANGE 0x0000000000000004
#define PPC_DEBUG_FEATURE_DATA_BP_MASK 0x0000000000000008
+#define PPC_DEBUG_FEATURE_DATA_BP_DAWR 0x0000000000000010
#ifndef __ASSEMBLY__
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index f9b30c6..8240c1a 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -1637,6 +1637,8 @@ long arch_ptrace(struct task_struct *child, long request,
dbginfo.sizeof_condition = 0;
#ifdef CONFIG_HAVE_HW_BREAKPOINT
dbginfo.features = PPC_DEBUG_FEATURE_DATA_BP_RANGE;
+ if (cpu_has_feature(CPU_FTR_DAWR))
+ dbginfo.features |= PPC_DEBUG_FEATURE_DATA_BP_DAWR;
#else
dbginfo.features = 0;
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
^ permalink raw reply related
* RE: [PATCH 2/3] powerpc/mpic: add global timer support
From: Wang Dongsheng-B40534 @ 2013-03-22 6:14 UTC (permalink / raw)
To: Wood Scott-B07421
Cc: Gala Kumar-B11780, linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <1363820378.25034.31@snotra>
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, March 21, 2013 7:00 AM
> To: Wang Dongsheng-B40534
> Cc: Wood Scott-B07421; Gala Kumar-B11780; linuxppc-dev@lists.ozlabs.org;
> Li Yang-R58472
> Subject: Re: [PATCH 2/3] powerpc/mpic: add global timer support
>=20
> On 03/20/2013 01:45:03 AM, Wang Dongsheng-B40534 wrote:
> >
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Wednesday, March 20, 2013 6:59 AM
> > > To: Wang Dongsheng-B40534
> > > Cc: Wood Scott-B07421; Gala Kumar-B11780;
> > linuxppc-dev@lists.ozlabs.org;
> > > Li Yang-R58472
> > > Subject: Re: [PATCH 2/3] powerpc/mpic: add global timer support
> > >
> > > On 03/19/2013 02:55:58 AM, Wang Dongsheng-B40534 wrote:
> > > > > > +static void convert_ticks_to_time(struct timer_group_priv
> > *priv,
> > > > > > + const u64 ticks, struct timeval *time) {
> > > > > > + u64 tmp_sec;
> > > > > > + u32 rem_us;
> > > > > > + u32 div;
> > > > > > +
> > > > > > + if (!(priv->flags & FSL_GLOBAL_TIMER)) {
> > > > > > + time->tv_sec =3D (__kernel_time_t)
> > > > > > + div_u64_rem(ticks, priv->timerfreq,
> > &rem_us);
> > > > > > + tmp_sec =3D (u64)time->tv_sec *
> > (u64)priv->timerfreq;
> > > > > > + time->tv_usec =3D (__kernel_suseconds_t)
> > > > > > + div_u64((ticks - tmp_sec) * 1000000,
> > > > > > priv->timerfreq);
> > > > > > +
> > > > > > + return;
> > > > > > + }
> > > > > > +
> > > > > > + div =3D (1 << (MPIC_TIMER_TCR_CLKDIV_64 >> 8)) * 8;
> > > > > > +
> > > > > > + time->tv_sec =3D (__kernel_time_t)div_u64(ticks, priv-
> > > >timerfreq
> > > > > > / div);
> > > > > > + tmp_sec =3D div_u64((u64)time->tv_sec *
> > (u64)priv->timerfreq,
> > > > > > div);
> > > > > > +
> > > > > > + time->tv_usec =3D (__kernel_suseconds_t)
> > > > > > + div_u64((ticks - tmp_sec) * 1000000,
> > priv->timerfreq /
> > > > > > div);
> > > > > > +
> > > > > > + return;
> > > > >
> > > > > Why don't you just adjust the clock frequency up front for
> > > > CLKDIV_64,
> > > > > rather than introduce alternate (and untested!) code paths
> > > > throughout the
> > > > > driver?
> > > > >
> > > > No, It cannot be integrated. The div cannot be removed.
> > > > Because if do priv->timerfreq /=3D div, that will affect the
> > accuracy.
> > > >
> > > > Like:
> > > > 3 * 5 / 2 =3D 7;
> > > > 3 / 2 * 5 =3D 5;
> > >
> > > I don't follow -- a change in the clock speed is a change in the
> > clock
> > > speed, no matter how you accomplish it.
> > >
> > This is not change hardware clock frequency.
>=20
> Citation needed. It looks like a change in the timer frequency to me:
>=20
> Clock ratio. Specifies the ratio of the timer frequency to the MPIC
> input clock (platform clock/2) . The
> following clock ratios are supported:
> 00
> 01
> 10
> 11
> Default. Divide by 8
> Divide by 16
> Divide by 32
> Divide by 64
>=20
> The end result is that the counter in the timer register changes only
> 1/64 as often as the input clock. There's nothing special about that,
> compared to having an input clock that is 1/64 the speed.
>=20
> > The mpic timer hardware clock is not be changed after initialization.
> > This is just conversion ticks.
> > These calculated ticks will be set to the hardware.
> >
> > > How you round is a different question. You should probably be
> > rounding
> > > up always, based on the final clock frequency -- though it's
> > unlikely to
> > > matter much given the high precision of the timer relative to the
> > input
> > > granularity.
> > >
> > Each ticks are based on the mpic timer hardware clock frequency.
> > The conversion and calculation are in order to make the tick value is
> > more
> > accurate, more close to real time.
> > If echo 40 seconds may be difference is not obvious. But echo
> > 315360000(10 years)
> > difference is obvious.
>=20
> So basically you're taking advantage of the fact that you have what
> appears to be a more precise value of the frequency than is expressible
> in integer Hz -- but I think that's false precision; odds are the
> frequency is not accurate to 1 Hz to begin with. Even if it is, I doubt
> it's worth worrying about. The error as a percentage will still be very
> small with an input frequency of many MHz. Does an error of a few
> minutes really matter if you're delaying for 10 years? That's
> acceptable
> clock drift for something not synced to network time. The main thing is
> to ensure that you round up, not down, so that software doesn't see an
> early wakeup as measured by its own timers.
>=20
> BTW, the input clock frequency has been similarly scaled, yet you don't
> try to scrounge up that information to get further precision...
>=20
Let's go back patch, do you think the code is repeated?
I will remove "if (!(priv->flags & FSL_GLOBAL_TIMER))" branch, there will b=
e no redundant code.
^ permalink raw reply
* Re: [Suggestion] PowerPC: kernel: cross compiling issue with allmodconfig
From: Chen Gang @ 2013-03-22 6:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: sfr, Michael Neuling, matt, linux-kernel@vger.kernel.org,
paulus@samba.org, imunsie, Chen Gang F T, linuxppc-dev
In-Reply-To: <1363869499.17680.25.camel@pasglop>
On 2013年03月21日 20:38, Benjamin Herrenschmidt wrote:
> On Thu, 2013-03-21 at 16:26 +0800, Chen Gang F T wrote:
>> > it seems:
>> > only move slb_miss_realmode to the end, can fix this issue without negative effect.
> Thanks. I'm currently on vacation, I'll have a closer look when I'm back
> unless Stephen or Paulus wants to shoot that to Linus while I'm
> away.
>
ok, I prefer to wait you back to have a closer look.
thanks.
:-)
> Cheers,
> Ben.
>
--
Chen Gang
Asianux Corporation
^ permalink raw reply
* Re: [Suggestion] PowerPC: kernel: cross compiling issue with allmodconfig
From: Chen Gang @ 2013-03-22 6:55 UTC (permalink / raw)
To: Michael Neuling
Cc: sfr, matt, linux-kernel@vger.kernel.org, paulus@samba.org,
imunsie, linuxppc-dev
In-Reply-To: <10123.1363906486@ale.ozlabs.ibm.com>
On 2013年03月22日 06:54, Michael Neuling wrote:
> This is great, thanks a lot.
>
> If you want this to be picked up by the maintainer, you'll need to add
> your signed-off-by.
>
> The signed-off-by is to indicate that your happy for it to be included
> and that you're legally allowed to do so. See
> http://gerrit.googlecode.com/svn/documentation/2.0/user-signedoffby.html
> for more info.
>
> Mikey
thanks, too.
I prefer Benjamin to have a closer look for my fixing.
if pass his checking, I will send a patch.
(he is on vacation, now, and he will check it when he is back).
Regards
--
Chen Gang
Asianux Corporation
^ permalink raw reply
* Re: [PATCH] powerpc/dts: Add qe support for 36bit
From: Zhicheng Fan @ 2013-03-22 7:16 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev
In-Reply-To: <CAOZdJXVs9My50m9KHzdbn6pjvhvLqHrqqMsK02z2kCFx8YB03w@mail.gmail.com>
On 03/22/2013 02:42 AM, Timur Tabi wrote:
> On Thu, Mar 21, 2013 at 4:26 AM, Zhicheng Fan <B32736@freescale.com> wrote:
>> + qe: qe@fffe80000 {
>> + ranges = <0x0 0xf 0xffe80000 0x40000>;
> Are you sure this works? The QE can't handle 36-bit addresses, and I
> think the MURAM is located inside the QE address space.
>
Hi Timur,
you are right ,the QE can not support the 36-bit , I test it on
the p1025 ,the qe can not work
but we need the qe node , becase the dts include the
fsl/p1021si-post.dtsi
that needed, I will send other patch
--
Regards & Thanks
Zhicheng Fan
^ permalink raw reply
* [PATCH] powerpc/dts: Fix the dts for p1025rdb 36bit
From: Zhicheng Fan @ 2013-03-22 6:43 UTC (permalink / raw)
To: galak, linuxppc-dev; +Cc: Zhicheng Fan
fix the following errors:
Error: arch/powerpc/boot/dts/p1025rdb.dtsi:326.2-3 label or path, 'qe', not found
Error: arch/powerpc/boot/dts/fsl/p1021si-post.dtsi:242.2-3 label or path, 'qe', not found
FATAL ERROR: Syntax error parsing input tree
Signed-off-by: Zhicheng Fan <B32736@freescale.com>
---
arch/powerpc/boot/dts/p1025rdb_36b.dts | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/boot/dts/p1025rdb_36b.dts b/arch/powerpc/boot/dts/p1025rdb_36b.dts
index 4ce4bfa..18f52be 100644
--- a/arch/powerpc/boot/dts/p1025rdb_36b.dts
+++ b/arch/powerpc/boot/dts/p1025rdb_36b.dts
@@ -82,6 +82,15 @@
0x0 0x100000>;
};
};
+
+ qe: qe@fffe80000 {
+ ranges = <0x0 0xf 0xffe80000 0x40000>;
+ reg = <0xf 0xffe80000 0 0x480>;
+ brg-frequency = <0>;
+ bus-frequency = <0>;
+ status = "disabled"; /* no firmware loaded */
+ };
+
};
/include/ "p1025rdb.dtsi"
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] KVM: fix powerpc build error for !CONFIG_KVM
From: Gleb Natapov @ 2013-03-22 7:21 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Phil Carmody, Kevin Hilman, Marcelo Tosatti, LKML, linux-next,
Paul Mackerras, linuxppc-dev
In-Reply-To: <20130322143550.c7ebe79c9fd97ec7a385ea16@canb.auug.org.au>
On Fri, Mar 22, 2013 at 02:35:50PM +1100, Stephen Rothwell wrote:
> Fixes these build error when CONFIG_KVM is not defined:
>
> In file included from arch/powerpc/include/asm/kvm_ppc.h:33:0,
> from arch/powerpc/kernel/setup_64.c:67:
> arch/powerpc/include/asm/kvm_book3s.h:65:20: error: field 'pte' has incomplete type
> arch/powerpc/include/asm/kvm_book3s.h:69:18: error: field 'vcpu' has incomplete type
> arch/powerpc/include/asm/kvm_book3s.h:98:34: error: 'HPTEG_HASH_NUM_PTE' undeclared here (not in a function)
> arch/powerpc/include/asm/kvm_book3s.h:99:39: error: 'HPTEG_HASH_NUM_PTE_LONG' undeclared here (not in a function)
> arch/powerpc/include/asm/kvm_book3s.h:100:35: error: 'HPTEG_HASH_NUM_VPTE' undeclared here (not in a function)
> arch/powerpc/include/asm/kvm_book3s.h:101:40: error: 'HPTEG_HASH_NUM_VPTE_LONG' undeclared here (not in a function)
> arch/powerpc/include/asm/kvm_book3s.h:129:4: error: 'struct kvm_run' declared inside parameter list [-Werror]
> arch/powerpc/include/asm/kvm_book3s.h:129:4: error: its scope is only this definition or declaration, which is probably not what you want [-Werror]
>
> ... and so on ...
>
> This was introduced by commit f445f11eb2cc265dd47da5b2e864df46cd6e5a82
> "KVM: allow host header to be included even for !CONFIG_KVM"
>
This will re-introduce the problem that Kevin tried to fix with his
original patch. The problem is that kernel/context_tracking.c includes
linux/kvm_host.h unconditionally and the later includes asm/kvm_host.h
and this breaks on archs without KVM.
I think the correct solution is to not include linux/kvm_host.h in
kernel/context_tracking.c unconditionally. The patch by Kevin is here:
http://lkml.org/lkml/2013/3/21/745
> Cc: Kevin Hilman <khilman@linaro.org>
> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
> include/linux/kvm_host.h | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> Kevin, does this still fix the error that commit
> f445f11eb2cc265dd47da5b2e864df46cd6e5a82 was fixing?
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index a942863..90ebec0 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -1,8 +1,6 @@
> #ifndef __KVM_HOST_H
> #define __KVM_HOST_H
>
> -#if IS_ENABLED(CONFIG_KVM)
> -
> /*
> * This work is licensed under the terms of the GNU GPL, version 2. See
> * the COPYING file in the top-level directory.
> @@ -751,6 +749,7 @@ static inline int kvm_deassign_device(struct kvm *kvm,
> }
> #endif /* CONFIG_IOMMU_API */
>
> +#if IS_ENABLED(CONFIG_KVM)
> static inline void __guest_enter(void)
> {
> /*
> @@ -770,6 +769,10 @@ static inline void __guest_exit(void)
> vtime_account_system(current);
> current->flags &= ~PF_VCPU;
> }
> +#else
> +static inline void __guest_enter(void) { return; }
> +static inline void __guest_exit(void) { return; }
> +#endif /* IS_ENABLED(CONFIG_KVM) */
>
> #ifdef CONFIG_CONTEXT_TRACKING
> extern void guest_enter(void);
> @@ -1057,8 +1060,4 @@ static inline bool kvm_vcpu_eligible_for_directed_yield(struct kvm_vcpu *vcpu)
> }
>
> #endif /* CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT */
> -#else
> -static inline void __guest_enter(void) { return; }
> -static inline void __guest_exit(void) { return; }
> -#endif /* IS_ENABLED(CONFIG_KVM) */
> #endif
> --
> 1.8.1
>
> --
> Cheers,
> Stephen Rothwell sfr@canb.auug.org.au
--
Gleb.
^ permalink raw reply
* Re: [PATCH] powerpc: add Book E support to 64-bit hibernation
From: Johannes Berg @ 2013-03-22 10:58 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Wang Dongsheng
In-Reply-To: <1363731391.16671.32@snotra>
On Tue, 2013-03-19 at 17:16 -0500, Scott Wood wrote:
> > > I wonder about kernel modules, though flushing 32 MiB wouldn't be
> > > adequate there.
> >
> > Good question, but would they be running? You have to have everything
> > built in that you need to load the image? Or maybe not, with the
> > userspace image restoration that became possible at some point...
>
> Is that all that's being restored in this step, or would we be loading
> all modules that were loaded before suspend (as they're normally not
> swappable)? I'm not too familiar with what gets saved where.
Yes, they would be restored since full memory is restored, but the
kernel doing the restore hasn't typically had a chance to load modules.
Although with uswsusp there probably are ways it can already have.
> > Maybe there's a way to completely flush the (i)cache? :-)
>
> There is, but it's platform-dependent, and not pleasant on our chips
> (need a displacement flush for L1, and sometimes errata are involved
> for L2).
:-)
I'll be the last one to tell you what to do, and I really don't remember
for sure why I did the flushing this way. It may very well be wrong for
what I thought I was doing, but it doesn't matter because I didn't need
to be doing it, I can't say any more now, too long ago. Sorry. May you
should ask Rafael what he thinks about flushing.
johannes
^ permalink raw reply
* [PATCH 1/3] uprobes: add trap variant helper
From: Ananth N Mavinakayanahalli @ 2013-03-22 11:44 UTC (permalink / raw)
To: lkml; +Cc: ppcdev, oleg, Srikar Dronamraju
From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Some architectures like powerpc have multiple variants of the trap
instruction. Introduce an additional helper is_trap_insn() for run-time
handling of non-uprobe traps on such architectures.
While there, change is_swbp_at_addr() to is_trap_at_addr() for reading
clarity.
With this change, the uprobe registration path will supercede any trap
instruction inserted at the requested location, while taking care of
delivering the SIGTRAP for cases where the trap notification came in
for an address without a uprobe. See [1] for a more detailed explanation.
[1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-March/104771.html
This change was suggested by Oleg Nesterov.
Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
---
include/linux/uprobes.h | 1 +
kernel/events/uprobes.c | 32 ++++++++++++++++++++++++++++----
2 files changed, 29 insertions(+), 4 deletions(-)
Index: linux-3.9-rc3/include/linux/uprobes.h
===================================================================
--- linux-3.9-rc3.orig/include/linux/uprobes.h
+++ linux-3.9-rc3/include/linux/uprobes.h
@@ -100,6 +100,7 @@ struct uprobes_state {
extern int __weak set_swbp(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
extern int __weak set_orig_insn(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
extern bool __weak is_swbp_insn(uprobe_opcode_t *insn);
+extern bool __weak is_trap_insn(uprobe_opcode_t *insn);
extern int uprobe_register(struct inode *inode, loff_t offset, struct uprobe_consumer *uc);
extern int uprobe_apply(struct inode *inode, loff_t offset, struct uprobe_consumer *uc, bool);
extern void uprobe_unregister(struct inode *inode, loff_t offset, struct uprobe_consumer *uc);
Index: linux-3.9-rc3/kernel/events/uprobes.c
===================================================================
--- linux-3.9-rc3.orig/kernel/events/uprobes.c
+++ linux-3.9-rc3/kernel/events/uprobes.c
@@ -173,6 +173,20 @@ bool __weak is_swbp_insn(uprobe_opcode_t
return *insn == UPROBE_SWBP_INSN;
}
+/**
+ * is_trap_insn - check if instruction is breakpoint instruction.
+ * @insn: instruction to be checked.
+ * Default implementation of is_trap_insn
+ * Returns true if @insn is a breakpoint instruction.
+ *
+ * This function is needed for the case where an architecture has multiple
+ * trap instructions (like powerpc).
+ */
+bool __weak is_trap_insn(uprobe_opcode_t *insn)
+{
+ return is_swbp_insn(insn);
+}
+
static void copy_opcode(struct page *page, unsigned long vaddr, uprobe_opcode_t *opcode)
{
void *kaddr = kmap_atomic(page);
@@ -185,6 +199,15 @@ static int verify_opcode(struct page *pa
uprobe_opcode_t old_opcode;
bool is_swbp;
+ /*
+ * Note: We only check if the old_opcode is UPROBE_SWBP_INSN here.
+ * We do not check if it is any other 'trap variant' which could
+ * be conditional trap instruction such as the one powerpc supports.
+ *
+ * The logic is that we do not care if the underlying instruction
+ * is a trap variant; uprobes always wins over any other (gdb)
+ * breakpoint.
+ */
copy_opcode(page, vaddr, &old_opcode);
is_swbp = is_swbp_insn(&old_opcode);
@@ -204,7 +227,7 @@ static int verify_opcode(struct page *pa
* Expect the breakpoint instruction to be the smallest size instruction for
* the architecture. If an arch has variable length instruction and the
* breakpoint instruction is not of the smallest length instruction
- * supported by that architecture then we need to modify is_swbp_at_addr and
+ * supported by that architecture then we need to modify is_trap_at_addr and
* write_opcode accordingly. This would never be a problem for archs that
* have fixed length instructions.
*/
@@ -1431,7 +1454,7 @@ static void mmf_recalc_uprobes(struct mm
clear_bit(MMF_HAS_UPROBES, &mm->flags);
}
-static int is_swbp_at_addr(struct mm_struct *mm, unsigned long vaddr)
+static int is_trap_at_addr(struct mm_struct *mm, unsigned long vaddr)
{
struct page *page;
uprobe_opcode_t opcode;
@@ -1452,7 +1475,8 @@ static int is_swbp_at_addr(struct mm_str
copy_opcode(page, vaddr, &opcode);
put_page(page);
out:
- return is_swbp_insn(&opcode);
+ /* This needs to return true for any variant of the trap insn */
+ return is_trap_insn(&opcode);
}
static struct uprobe *find_active_uprobe(unsigned long bp_vaddr, int *is_swbp)
@@ -1472,7 +1496,7 @@ static struct uprobe *find_active_uprobe
}
if (!uprobe)
- *is_swbp = is_swbp_at_addr(mm, bp_vaddr);
+ *is_swbp = is_trap_at_addr(mm, bp_vaddr);
} else {
*is_swbp = -EFAULT;
}
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox