* Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()
From: Nicolas Pitre @ 2014-02-07 10:48 UTC (permalink / raw)
To: Preeti U Murthy
Cc: Lists linaro-kernel, linux-pm@vger.kernel.org, Peter Zijlstra,
Daniel Lezcano, Rafael J. Wysocki, LKML, Ingo Molnar,
Thomas Gleixner, linuxppc-dev
In-Reply-To: <52F46EB3.5080403@linux.vnet.ibm.com>
On Fri, 7 Feb 2014, Preeti U Murthy wrote:
> Hi Nicolas,
>
> On 02/07/2014 06:47 AM, Nicolas Pitre wrote:
> >
> > What about creating arch_cpu_idle_enter() and arch_cpu_idle_exit() in
> > arch/powerpc/kernel/idle.c and calling ppc64_runlatch_off() and
> > ppc64_runlatch_on() respectively from there instead? Would that work?
> > That would make the idle consolidation much easier afterwards.
>
> I would not suggest doing this. The ppc64_runlatch_*() routines need to
> be called when we are sure that the cpu is about to enter or has exit an
> idle state. Moving the ppc64_runlatch_on() routine to
> arch_cpu_idle_enter() for instance is not a good idea because there are
> places where the cpu can decide not to enter any idle state before the
> call to cpuidle_idle_call() itself. In that case communicating
> prematurely that we are in an idle state would not be a good idea.
>
> So its best to add the ppc64_runlatch_* calls in the powernv cpuidle
> driver IMO. We could however create idle_loop_prologue/epilogue()
> variants inside it so that in addition to the runlatch routines we could
> potentially add more such similar routines that are powernv specific.
> If there are cases where there is work to be done prior to and post an
> entry into an idle state common to both pseries and powernv, we will
> probably put them in arch_cpu_idle_enter/exit(). But the runlatch
> routines are not suitable to be moved there as far as I can see.
OK.
However, one thing we need to do as much as possible is to remove those
loops based on need_resched() from idle backend drivers. A somewhat
common pattern is:
my_idle()
{
/* interrupts disabled on entry */
while (!need_resched()) {
lowpower_wait_for_interrupts();
local_irq_enable();
/* IRQ serviced from here */
local_irq_disable();
}
local_irq_enable();
/* interrupts enabled on exit */
}
To be able to keep statistics on the actual idleness of the CPU we'd
need for all idle backends to always return to generic code on every
interrupt similar to this:
my_idle()
{
/* interrupts disabled on entry */
lowpower_wait_for_interrupts();
local_irq_enable();
/* interrupts enabled on exit */
}
The generic code would be responsible for dealing with need_resched()
and call back into the backend right away if necessary after updating
some stats.
Do you see a problem with the runlatch calls happening around each
interrrupt from such a simplified idle backend?
Nicolas
^ permalink raw reply
* Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()
From: Preeti U Murthy @ 2014-02-07 11:00 UTC (permalink / raw)
To: Deepthi Dharwar
Cc: Nicolas Pitre, Lists linaro-kernel, linux-pm@vger.kernel.org,
Peter Zijlstra, Daniel Lezcano, Rafael J. Wysocki, LKML,
Ingo Molnar, Thomas Gleixner, linuxppc-dev, Linux ARM Kernel ML
In-Reply-To: <52F4AB54.801@linux.vnet.ibm.com>
Hi Deepthi,
On 02/07/2014 03:15 PM, Deepthi Dharwar wrote:
> Hi Preeti,
>
> Thanks for the patch.
>
> On 02/07/2014 12:31 PM, Preeti U Murthy wrote:
>> Hi Nicolas,
>>
>> Find below the patch that will need to be squashed with this one.
>> This patch is based on the mainline.Adding Deepthi, the author of
>> the patch which introduced the powernv cpuidle driver. Deepthi,
>> do you think the below patch looks right? We do not need to do an
>> explicit local_irq_enable() since we are in the call path of
>> cpuidle driver and that explicitly enables irqs on exit from
>> idle states.
>
> Yes, We enable irqs explicitly while entering snooze loop and we always
> have interrupts enabled in the snooze state.
> For NAP state, we exit out of this state with interrupts enabled so we
> do not need an explicit enable of irqs.
>
>> On 02/07/2014 06:47 AM, Nicolas Pitre wrote:
>>> On Thu, 6 Feb 2014, Preeti U Murthy wrote:
>>>
>>>> Hi Daniel,
>>>>
>>>> On 02/06/2014 09:55 PM, Daniel Lezcano wrote:
>>>>> Hi Nico,
>>>>>
>>>>>
>>>>> On 6 February 2014 14:16, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
>>>>>
>>>>>> The core idle loop now takes care of it.
>>>>>>
>>>>>> Signed-off-by: Nicolas Pitre <nico@linaro.org>
>>>>>> ---
>>>>>> arch/powerpc/platforms/powernv/setup.c | 13 +------------
>>>>>> 1 file changed, 1 insertion(+), 12 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/powerpc/platforms/powernv/setup.c
>>>>>> b/arch/powerpc/platforms/powernv/setup.c
>>>>>> index 21166f65c9..a932feb290 100644
>>>>>> --- a/arch/powerpc/platforms/powernv/setup.c
>>>>>> +++ b/arch/powerpc/platforms/powernv/setup.c
>>>>>> @@ -26,7 +26,6 @@
>>>>>> #include <linux/of_fdt.h>
>>>>>> #include <linux/interrupt.h>
>>>>>> #include <linux/bug.h>
>>>>>> -#include <linux/cpuidle.h>
>>>>>>
>>>>>> #include <asm/machdep.h>
>>>>>> #include <asm/firmware.h>
>>>>>> @@ -217,16 +216,6 @@ static int __init pnv_probe(void)
>>>>>> return 1;
>>>>>> }
>>>>>>
>>>>>> -void powernv_idle(void)
>>>>>> -{
>>>>>> - /* Hook to cpuidle framework if available, else
>>>>>> - * call on default platform idle code
>>>>>> - */
>>>>>> - if (cpuidle_idle_call()) {
>>>>>> - power7_idle();
>>>>>> - }
>>>>>>
>>
>> drivers/cpuidle/cpuidle-powernv.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
>> index 78fd174..130f081 100644
>> --- a/drivers/cpuidle/cpuidle-powernv.c
>> +++ b/drivers/cpuidle/cpuidle-powernv.c
>> @@ -31,11 +31,13 @@ static int snooze_loop(struct cpuidle_device *dev,
>> set_thread_flag(TIF_POLLING_NRFLAG);
>>
>> while (!need_resched()) {
>> + ppc64_runlatch_off();
> ^^^^^^^^^^^^^^^
> We could move this before the while() loop.
> It would ideal to turn off latch when we enter snooze and
> turn it on when we are about to exit, rather than doing
> it over and over in the while loop.
You are right, this can be moved out of the loop.
Thanks
Regards
Preeti U Murthy
^ permalink raw reply
* Re: [PATCH v2 6/6] cpu/idle.c: move to sched/idle.c
From: Nicolas Pitre @ 2014-02-07 11:09 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linaro-kernel, Russell King, linux-sh, linux-pm, Daniel Lezcano,
Rafael J. Wysocki, linux-kernel, Paul Mundt, Preeti U Murthy,
Thomas Gleixner, linuxppc-dev, Ingo Molnar, linux-arm-kernel
In-Reply-To: <20140206164336.GU2936@laptop.programming.kicks-ass.net>
On Thu, 6 Feb 2014, Peter Zijlstra wrote:
> On Thu, Feb 06, 2014 at 02:09:59PM +0000, Nicolas Pitre wrote:
> > Hi Peter,
> >
> > Did you merge those patches in your tree?
>
> tree, tree, what's in a word.
Something you may plant on a patch of grass? "Merging" becomes a
strange concept in that context though. :-)
> Its in my patch stack yes.
Quilt I suppose?? (yet another word.)
> I should get some of that into tip I suppose, been side-tracked a bit
> this week. Sorry for the delay.
If you prefer we pile those patches (and future ones after revew)
ourselves just let me know. Future patches are likely to be more
intimate with the scheduler so I just need to know who to upstream them
through afterwards.
Nicolas
^ permalink raw reply
* Re: ppc9a board support
From: Martyn Welch @ 2014-02-07 11:23 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <CAOB9jCJmXTo7NVR4jdV+3AkRsZviR447PKi2_TFyDdjoO0to8g@mail.gmail.com>
Hi Chris,
Support for the PPC9A is already in the Linux kernel, along with VME
drivers.
Martyn
On 04/02/14 14:47, Chris Enrique wrote:
> hello,
>
> sorry if this message doesn't clearly hit the topic of this list but i
> would like to ask if anybody has information about bringing a linux-3.10
> or later kernel to ge fanuc ppc9a powerpc board (existing bsp? vme
> drivers? etc.?) to prevent starting from scratch for this task.
>
> any ideas would be much appreciated.
>
>
> kind regards,
> chris
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
--
--
Martyn Welch (Lead Software Engineer) | Registered in England and Wales
GE Intelligent Platforms | (3828642) at 100 Barbirolli Square
T +44(0)1327322748 | Manchester, M2 3AB
E martyn.welch@ge.com | VAT:GB 927559189
^ permalink raw reply
* Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()
From: Preeti U Murthy @ 2014-02-07 11:41 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Lists linaro-kernel, linux-pm@vger.kernel.org, Peter Zijlstra,
Daniel Lezcano, Rafael J. Wysocki, LKML, Ingo Molnar,
Thomas Gleixner, linuxppc-dev
In-Reply-To: <alpine.LFD.2.11.1402071022490.1906@knanqh.ubzr>
Hi Nicolas,
On 02/07/2014 04:18 PM, Nicolas Pitre wrote:
> On Fri, 7 Feb 2014, Preeti U Murthy wrote:
>
>> Hi Nicolas,
>>
>> On 02/07/2014 06:47 AM, Nicolas Pitre wrote:
>>>
>>> What about creating arch_cpu_idle_enter() and arch_cpu_idle_exit() in
>>> arch/powerpc/kernel/idle.c and calling ppc64_runlatch_off() and
>>> ppc64_runlatch_on() respectively from there instead? Would that work?
>>> That would make the idle consolidation much easier afterwards.
>>
>> I would not suggest doing this. The ppc64_runlatch_*() routines need to
>> be called when we are sure that the cpu is about to enter or has exit an
>> idle state. Moving the ppc64_runlatch_on() routine to
>> arch_cpu_idle_enter() for instance is not a good idea because there are
>> places where the cpu can decide not to enter any idle state before the
>> call to cpuidle_idle_call() itself. In that case communicating
>> prematurely that we are in an idle state would not be a good idea.
>>
>> So its best to add the ppc64_runlatch_* calls in the powernv cpuidle
>> driver IMO. We could however create idle_loop_prologue/epilogue()
>> variants inside it so that in addition to the runlatch routines we could
>> potentially add more such similar routines that are powernv specific.
>> If there are cases where there is work to be done prior to and post an
>> entry into an idle state common to both pseries and powernv, we will
>> probably put them in arch_cpu_idle_enter/exit(). But the runlatch
>> routines are not suitable to be moved there as far as I can see.
>
> OK.
>
> However, one thing we need to do as much as possible is to remove those
> loops based on need_resched() from idle backend drivers. A somewhat
> common pattern is:
>
> my_idle()
> {
> /* interrupts disabled on entry */
> while (!need_resched()) {
> lowpower_wait_for_interrupts();
> local_irq_enable();
> /* IRQ serviced from here */
> local_irq_disable();
> }
> local_irq_enable();
> /* interrupts enabled on exit */
> }
>
> To be able to keep statistics on the actual idleness of the CPU we'd
> need for all idle backends to always return to generic code on every
> interrupt similar to this:
>
> my_idle()
> {
> /* interrupts disabled on entry */
> lowpower_wait_for_interrupts();
You can do this for the idle states which do not have the polling
nature. IOW, these idle states are capable of doing what you describe as
"wait_for_interrupts". They do some kind of spinning at the hardware
level with interrupts enabled. A reschedule IPI or any other interrupt
will wake them up to enter the generic idle loop where they check for
the cause of the interrupt.
But observe the idle state "snooze" on powerpc. The power that this idle
state saves is through the lowering of the thread priority of the CPU.
After it lowers the thread priority, it is done. It cannot
"wait_for_interrupts". It will exit my_idle(). It is now upto the
generic idle loop to increase the thread priority if the need_resched
flag is set. Only an interrupt routine can increase the thread priority.
Else we will need to do it explicitly. And in such states which have a
polling nature, the cpu will not receive a reschedule IPI.
That is why in the snooze_loop() we poll on need_resched. If it is set
we up the priority of the thread using HMT_MEDIUM() and then exit the
my_idle() loop. In case of interrupts, the priority gets automatically
increased.
This might not be required to be done for similar idle routines on other
archs but this is the consequence of applying this idea of simplified
cpuidle backend driver on powerpc.
I would say you could let the backend cpuidle drivers be in this regard,
it could complicate the generic idle loop IMO depending on how the
polling states are implemented in each architecture.
> The generic code would be responsible for dealing with need_resched()
> and call back into the backend right away if necessary after updating
> some stats.
>
> Do you see a problem with the runlatch calls happening around each
> interrrupt from such a simplified idle backend?
The runlatch calls could be moved outside the loop.They do not need to
be called each time.
Thanks
Regards
Preeti U Murthy
>
>
> Nicolas
>
^ permalink raw reply
* Re: [PATCH] Convert powerpc simple spinlocks into ticket locks
From: Torsten Duwe @ 2014-02-07 11:49 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Tom Musta, linux-kernel, Paul Mackerras, Anton Blanchard,
Scott Wood, Paul E. McKenney, linuxppc-dev, Ingo Molnar
In-Reply-To: <20140207104530.GG5126@laptop.programming.kicks-ass.net>
On Fri, Feb 07, 2014 at 11:45:30AM +0100, Peter Zijlstra wrote:
>
> That might need to be lhz too, I'm confused on all the load variants.
;-)
> > unlock:
> > lhz %0, 0, &tail
> > addic %0, %0, 1
No carry with this one, I'd say.
Besides, unlock increments the head.
> > lwsync
> > sth %0, 0, &tail
> >
Given the beauty and simplicity of this, may I ask Ingo:
you signed off 314cdbefd1fd0a7acf3780e9628465b77ea6a836;
can you explain why head and tail must live on the same cache
line? Or is it just a space saver? I just ported it to ppc,
I didn't think about alternatives.
What about
atomic_t tail;
volatile int head; ?
Admittedly, that's usually 8 bytes instead of 4...
Torsten
^ permalink raw reply
* Re: [PATCH V4 3/3] time/cpuidle:Handle failed call to BROADCAST_ENTER on archs with CPUIDLE_FLAG_TIMER_STOP set
From: Rafael J. Wysocki @ 2014-02-07 12:07 UTC (permalink / raw)
To: Preeti U Murthy
Cc: deepthi, linux-pm, peterz, rafael.j.wysocki, linux-kernel, paulus,
srivatsa.bhat, fweisbec, tglx, paulmck, linuxppc-dev, mingo
In-Reply-To: <20140207080652.17187.66344.stgit@preeti.in.ibm.com>
On Friday, February 07, 2014 01:36:52 PM Preeti U Murthy wrote:
> Some archs set the CPUIDLE_FLAG_TIMER_STOP flag for idle states in which the
> local timers stop. The cpuidle_idle_call() currently handles such idle states
> by calling into the broadcast framework so as to wakeup CPUs at their next
> wakeup event. With the hrtimer mode of broadcast, the BROADCAST_ENTER call
> into the broadcast frameowork can fail for archs that do not have an external
> clock device to handle wakeups and the CPU in question has to thus be made
> the stand by CPU. This patch handles such cases by failing the call into
> cpuidle so that the arch can take some default action. The arch will certainly
> not enter a similar idle state because a failed cpuidle call will also implicitly
> indicate that the broadcast framework has not registered this CPU to be woken up.
> Hence we are safe if we fail the cpuidle call.
>
> In the process move the functions that trace idle statistics just before and
> after the entry and exit into idle states respectively. In other
> scenarios where the call to cpuidle fails, we end up not tracing idle
> entry and exit since a decision on an idle state could not be taken. Similarly
> when the call to broadcast framework fails, we skip tracing idle statistics
> because we are in no further position to take a decision on an alternative
> idle state to enter into.
>
> Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>
> drivers/cpuidle/cpuidle.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index a55e68f..8beb0f02 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -140,12 +140,14 @@ int cpuidle_idle_call(void)
> return 0;
> }
>
> - trace_cpu_idle_rcuidle(next_state, dev->cpu);
> -
> broadcast = !!(drv->states[next_state].flags & CPUIDLE_FLAG_TIMER_STOP);
>
> - if (broadcast)
> - clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
> + if (broadcast &&
> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu))
> + return -EBUSY;
> +
> +
> + trace_cpu_idle_rcuidle(next_state, dev->cpu);
>
> if (cpuidle_state_is_coupled(dev, drv, next_state))
> entered_state = cpuidle_enter_state_coupled(dev, drv,
> @@ -153,11 +155,11 @@ int cpuidle_idle_call(void)
> else
> entered_state = cpuidle_enter_state(dev, drv, next_state);
>
> + trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu);
> +
> if (broadcast)
> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
>
> - trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu);
> -
> /* give the governor an opportunity to reflect on the outcome */
> if (cpuidle_curr_governor->reflect)
> cpuidle_curr_governor->reflect(dev, entered_state);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH] Convert powerpc simple spinlocks into ticket locks
From: Peter Zijlstra @ 2014-02-07 12:28 UTC (permalink / raw)
To: Torsten Duwe
Cc: Tom Musta, linux-kernel, Paul Mackerras, Anton Blanchard,
Scott Wood, Paul E. McKenney, linuxppc-dev, Ingo Molnar
In-Reply-To: <20140207114949.GA2107@lst.de>
On Fri, Feb 07, 2014 at 12:49:49PM +0100, Torsten Duwe wrote:
> On Fri, Feb 07, 2014 at 11:45:30AM +0100, Peter Zijlstra wrote:
> >
> > That might need to be lhz too, I'm confused on all the load variants.
>
> ;-)
>
> > > unlock:
> > > lhz %0, 0, &tail
> > > addic %0, %0, 1
>
> No carry with this one, I'd say.
Right you are, add immediate it is.
> Besides, unlock increments the head.
No, unlock increments the tail, lock increments the head and waits until
the tail matches the pre-inc value.
That said, why do the atomic_inc() primitives do an carry add? (that's
where I borrowed it from).
> > > lwsync
> > > sth %0, 0, &tail
> > >
>
> Given the beauty and simplicity of this, may I ask Ingo:
> you signed off 314cdbefd1fd0a7acf3780e9628465b77ea6a836;
> can you explain why head and tail must live on the same cache
> line? Or is it just a space saver? I just ported it to ppc,
> I didn't think about alternatives.
spinlock_t should, ideally, be 32bits.
> What about
>
> atomic_t tail;
> volatile int head; ?
>
> Admittedly, that's usually 8 bytes instead of 4...
That still won't straddle a cacheline unless you do weird alignement
things which will bloat all the various data structures more still.
Anyway, you can do a version with lwarx/stwcx if you're looking get rid
of lharx.
^ permalink raw reply
* Re: [PATCH v2 6/6] cpu/idle.c: move to sched/idle.c
From: Peter Zijlstra @ 2014-02-07 12:32 UTC (permalink / raw)
To: Nicolas Pitre
Cc: linaro-kernel, Russell King, linux-sh, linux-pm, Daniel Lezcano,
Rafael J. Wysocki, linux-kernel, Paul Mundt, Preeti U Murthy,
Thomas Gleixner, linuxppc-dev, Ingo Molnar, linux-arm-kernel
In-Reply-To: <alpine.LFD.2.11.1402071051140.1906@knanqh.ubzr>
On Fri, Feb 07, 2014 at 11:09:23AM +0000, Nicolas Pitre wrote:
> On Thu, 6 Feb 2014, Peter Zijlstra wrote:
> > tree, tree, what's in a word.
>
> Something you may plant on a patch of grass? "Merging" becomes a
> strange concept in that context though. :-)
I do know some farmers who splice trees thought :-)
> > Its in my patch stack yes.
>
> Quilt I suppose?? (yet another word.)
Yes, I'm one of the refugee Quilt users. Comes in handy when cold too
:-)
> > I should get some of that into tip I suppose, been side-tracked a bit
> > this week. Sorry for the delay.
>
> If you prefer we pile those patches (and future ones after revew)
> ourselves just let me know. Future patches are likely to be more
> intimate with the scheduler so I just need to know who to upstream them
> through afterwards.
Normally I get Ingo to pick up the queue 1-2 times a week so latency
shouldn't be too bad. But we just had the merge window and then I got
side-tracked rewriting all atomic implementations.
So usually submit patches against tip/master unless you know there's
other pending bits that conflict, in which case you can grab my queue on
top of tip/master -- but I try to make sure that's usually not needed.
^ permalink raw reply
* Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()
From: Peter Zijlstra @ 2014-02-07 12:41 UTC (permalink / raw)
To: Preeti U Murthy
Cc: Nicolas Pitre, Lists linaro-kernel, linux-pm@vger.kernel.org,
Daniel Lezcano, Rafael J. Wysocki, LKML, Ingo Molnar,
Thomas Gleixner, linuxppc-dev
In-Reply-To: <52F4C666.4050308@linux.vnet.ibm.com>
On Fri, Feb 07, 2014 at 05:11:26PM +0530, Preeti U Murthy wrote:
> But observe the idle state "snooze" on powerpc. The power that this idle
> state saves is through the lowering of the thread priority of the CPU.
> After it lowers the thread priority, it is done. It cannot
> "wait_for_interrupts". It will exit my_idle(). It is now upto the
> generic idle loop to increase the thread priority if the need_resched
> flag is set. Only an interrupt routine can increase the thread priority.
> Else we will need to do it explicitly. And in such states which have a
> polling nature, the cpu will not receive a reschedule IPI.
>
> That is why in the snooze_loop() we poll on need_resched. If it is set
> we up the priority of the thread using HMT_MEDIUM() and then exit the
> my_idle() loop. In case of interrupts, the priority gets automatically
> increased.
You can poll without setting TS_POLLING/TIF_POLLING_NRFLAGS just fine
and get the IPI if that is what you want.
Depending on how horribly unprovisioned the thread gets at the lowest
priority, that might actually be faster than polling and raising the
prio whenever it does get ran.
^ permalink raw reply
* [PATCH V2] powerpc: thp: Fix crash on mremap
From: Aneesh Kumar K.V @ 2014-02-07 13:51 UTC (permalink / raw)
To: benh, paulus, stable; +Cc: linuxppc-dev, Aneesh Kumar K.V
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
This patch fix the below crash
NIP [c00000000004cee4] .__hash_page_thp+0x2a4/0x440
LR [c0000000000439ac] .hash_page+0x18c/0x5e0
...
Call Trace:
[c000000736103c40] [00001ffffb000000] 0x1ffffb000000(unreliable)
[437908.479693] [c000000736103d50] [c0000000000439ac] .hash_page+0x18c/0x5e0
[437908.479699] [c000000736103e30] [c00000000000924c] .do_hash_page+0x4c/0x58
On ppc64 we use the pgtable for storing the hpte slot information and
store address to the pgtable at a constant offset (PTRS_PER_PMD) from
pmd. On mremap, when we switch the pmd, we need to withdraw and deposit
the pgtable again, so that we find the pgtable at PTRS_PER_PMD offset
from new pmd.
We also want to move the withdraw and deposit before the set_pmd so
that, when page fault find the pmd as trans huge we can be sure that
pgtable can be located at the offset.
variant of upstream SHA1: b3084f4db3aeb991c507ca774337c7e7893ed04f
for 3.12 stable series
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
arch/Kconfig | 3 +++
arch/powerpc/platforms/Kconfig.cputype | 1 +
mm/huge_memory.c | 12 ++++++++++++
3 files changed, 16 insertions(+)
diff --git a/arch/Kconfig b/arch/Kconfig
index af2cc6eabcc7..bca9e7a18bd2 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -365,6 +365,9 @@ config HAVE_ARCH_TRANSPARENT_HUGEPAGE
config HAVE_ARCH_SOFT_DIRTY
bool
+config ARCH_THP_MOVE_PMD_ALWAYS_WITHDRAW
+ bool
+
config HAVE_MOD_ARCH_SPECIFIC
bool
help
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 6704e2e20e6b..0225011231ea 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -71,6 +71,7 @@ config PPC_BOOK3S_64
select PPC_FPU
select PPC_HAVE_PMU_SUPPORT
select SYS_SUPPORTS_HUGETLBFS
+ select ARCH_THP_MOVE_PMD_ALWAYS_WITHDRAW
select HAVE_ARCH_TRANSPARENT_HUGEPAGE if PPC_64K_PAGES
config PPC_BOOK3E_64
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 292a266e0d42..89b7a647f1cb 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1474,8 +1474,20 @@ int move_huge_pmd(struct vm_area_struct *vma, struct vm_area_struct *new_vma,
ret = __pmd_trans_huge_lock(old_pmd, vma);
if (ret == 1) {
+#ifdef CONFIG_ARCH_THP_MOVE_PMD_ALWAYS_WITHDRAW
+ pgtable_t pgtable;
+#endif
pmd = pmdp_get_and_clear(mm, old_addr, old_pmd);
VM_BUG_ON(!pmd_none(*new_pmd));
+#ifdef CONFIG_ARCH_THP_MOVE_PMD_ALWAYS_WITHDRAW
+ /*
+ * Archs like ppc64 use pgtable to store per pmd
+ * specific information. So when we switch the pmd,
+ * we should also withdraw and deposit the pgtable
+ */
+ pgtable = pgtable_trans_huge_withdraw(mm, old_pmd);
+ pgtable_trans_huge_deposit(mm, new_pmd, pgtable);
+#endif
set_pmd_at(mm, new_addr, new_pmd, pmd_mksoft_dirty(pmd));
spin_unlock(&mm->page_table_lock);
}
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH] Convert powerpc simple spinlocks into ticket locks
From: Peter Zijlstra @ 2014-02-07 15:18 UTC (permalink / raw)
To: Torsten Duwe
Cc: Tom Musta, linux-kernel, Paul Mackerras, Anton Blanchard,
Scott Wood, Paul E. McKenney, linuxppc-dev, Ingo Molnar
In-Reply-To: <20140207122837.GA3104@twins.programming.kicks-ass.net>
On Fri, Feb 07, 2014 at 01:28:37PM +0100, Peter Zijlstra wrote:
> Anyway, you can do a version with lwarx/stwcx if you're looking get rid
> of lharx.
the below seems to compile into relatively ok asm. It can be done better
if you write the entire thing by hand though.
---
typedef unsigned short ticket_t;
typedef struct {
union {
unsigned int pair;
struct {
/* ensure @head is the MSB */
#ifdef __BIG_ENDIAN__
ticket_t head,tail;
#else
ticket_t tail,head;
#endif
};
};
} tickets_t;
#define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
#define barrier() __asm__ __volatile__ ("" : : :"memory")
#define __lwsync() __asm__ __volatile__ ("lwsync" : : :"memory")
#define smp_store_release(p, v) \
do { \
__lwsync(); \
ACCESS_ONCE(*p) = (v); \
} while (0)
#define smp_load_acquire(p) \
({ \
typeof(*p) ___p1 = ACCESS_ONCE(*p); \
__lwsync(); \
___p1; \
})
#define likely(x) __builtin_expect(!!(x), 1)
#define cpu_relax() barrier();
static inline unsigned int xadd(unsigned int *v, unsigned int i)
{
int t, ret;
__asm__ __volatile__ (
"1: lwarx %0, 0, %4\n"
" mr %1, %0\n"
" add %0, %3, %0\n"
" stwcx. %0, %0, %4\n"
" bne- 1b\n"
: "=&r" (t), "=&r" (ret), "+m" (*v)
: "r" (i), "r" (v)
: "cc");
return ret;
}
void ticket_lock(tickets_t *lock)
{
tickets_t t;
/*
* Because @head is MSB, the direct increment wrap doesn't disturb
* @tail.
*/
t.pair = xadd(&lock->pair, 1<<16);
if (likely(t.head == t.tail)) {
__lwsync(); /* acquire */
return;
}
while (smp_load_acquire(&lock->tail) != t.tail)
cpu_relax();
}
void ticket_unlock(tickets_t *lock)
{
ticket_t tail = lock->tail + 1;
/*
* The store is save against the xadd for it will make the ll/sc fail
* and try again. Aside from that PowerISA guarantees single-copy
* atomicy for half-word writes.
*
* And since only the lock owner will ever write the tail, we're good.
*/
smp_store_release(&lock->tail, tail);
}
^ permalink raw reply
* Re: [PATCH] Convert powerpc simple spinlocks into ticket locks
From: Peter Zijlstra @ 2014-02-07 15:43 UTC (permalink / raw)
To: Torsten Duwe
Cc: Tom Musta, linux-kernel, Paul Mackerras, Anton Blanchard,
Scott Wood, Paul E. McKenney, linuxppc-dev, Ingo Molnar
In-Reply-To: <20140207151847.GB3104@twins.programming.kicks-ass.net>
On Fri, Feb 07, 2014 at 04:18:47PM +0100, Peter Zijlstra wrote:
> void ticket_lock(tickets_t *lock)
> {
> tickets_t t;
>
> /*
> * Because @head is MSB, the direct increment wrap doesn't disturb
> * @tail.
> */
> t.pair = xadd(&lock->pair, 1<<16);
>
> if (likely(t.head == t.tail)) {
> __lwsync(); /* acquire */
> return;
> }
>
> while (smp_load_acquire(&lock->tail) != t.tail)
> cpu_relax();
That should be "!= t.head", for that contains our ticket.
I'm hopelessly scatter brained today it seems :/
> }
>
> void ticket_unlock(tickets_t *lock)
> {
> ticket_t tail = lock->tail + 1;
>
> /*
> * The store is save against the xadd for it will make the ll/sc fail
> * and try again. Aside from that PowerISA guarantees single-copy
> * atomicy for half-word writes.
> *
> * And since only the lock owner will ever write the tail, we're good.
> */
> smp_store_release(&lock->tail, tail);
> }
^ permalink raw reply
* Re: [PATCH] Convert powerpc simple spinlocks into ticket locks
From: Kumar Gala @ 2014-02-07 15:51 UTC (permalink / raw)
To: Torsten Duwe
Cc: Tom Musta, Peter Zijlstra, linux-kernel, Paul Mackerras,
Anton Blanchard, Scott Wood, Paul E. McKenney, linuxppc-dev,
Ingo Molnar
In-Reply-To: <20140207090248.GB26811@lst.de>
On Feb 7, 2014, at 3:02 AM, Torsten Duwe <duwe@lst.de> wrote:
> On Thu, Feb 06, 2014 at 02:19:52PM -0600, Scott Wood wrote:
>> On Thu, 2014-02-06 at 18:37 +0100, Torsten Duwe wrote:
>>> On Thu, Feb 06, 2014 at 05:38:37PM +0100, Peter Zijlstra wrote:
>>=20
>>>> Can you pair lwarx with sthcx ? I couldn't immediately find the =
answer
>>>> in the PowerISA doc. If so I think you can do better by being able =
to
>>>> atomically load both tickets but only storing the head without =
affecting
>>>> the tail.
>=20
> Can I simply write the half word, without a reservation, or will the =
HW caches
> mess up the other half? Will it ruin the cache coherency on some =
(sub)architectures?
The coherency should be fine, I just can=92t remember if you=92ll lose =
the reservation by doing this.
>> Plus, sthcx doesn't exist on all PPC chips.
>=20
> Which ones are lacking it? Do all have at least a simple 16-bit store?
Everything implements a simple 16-bit store, just not everything =
implements the store conditional of 16-bit data.
- k=
^ permalink raw reply
* [RFC PATCH 01/18] powerpc/boot: fix do_div for 64bit wrapper
From: Cédric Le Goater @ 2014-02-07 15:59 UTC (permalink / raw)
To: benh; +Cc: Cédric Le Goater, linuxppc-dev
In-Reply-To: <1391788771-16405-1-git-send-email-clg@fr.ibm.com>
When the boot wrapper is compiled in 64bit, there is no need to
use __div64_32.
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
arch/powerpc/boot/stdio.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/powerpc/boot/stdio.c b/arch/powerpc/boot/stdio.c
index 5b57800bbc67..a701261b1781 100644
--- a/arch/powerpc/boot/stdio.c
+++ b/arch/powerpc/boot/stdio.c
@@ -21,6 +21,18 @@ size_t strnlen(const char * s, size_t count)
return sc - s;
}
+#ifdef __powerpc64__
+
+# define do_div(n, base) ({ \
+ unsigned int __base = (base); \
+ unsigned int __rem; \
+ __rem = ((unsigned long long)(n)) % __base; \
+ (n) = ((unsigned long long)(n)) / __base; \
+ __rem; \
+})
+
+#else
+
extern unsigned int __div64_32(unsigned long long *dividend,
unsigned int divisor);
@@ -39,6 +51,8 @@ extern unsigned int __div64_32(unsigned long long *dividend,
__rem; \
})
+#endif /* __powerpc64__ */
+
static int skip_atoi(const char **s)
{
int i, c;
--
1.7.10.4
^ permalink raw reply related
* [RFC PATCH 03/18] powerpc/boot: use prom_arg_t in oflib
From: Cédric Le Goater @ 2014-02-07 15:59 UTC (permalink / raw)
To: benh; +Cc: Cédric Le Goater, linuxppc-dev
In-Reply-To: <1391788771-16405-1-git-send-email-clg@fr.ibm.com>
This patch updates the wrapper code to converge with the kernel code in
prom_init.
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
arch/powerpc/boot/oflib.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/boot/oflib.c b/arch/powerpc/boot/oflib.c
index c3288a3446b3..3b0c9458504f 100644
--- a/arch/powerpc/boot/oflib.c
+++ b/arch/powerpc/boot/oflib.c
@@ -16,6 +16,8 @@
#include "of.h"
+typedef u32 prom_arg_t;
+
/* The following structure is used to communicate with open firmware.
* All arguments in and out are in big endian format. */
struct prom_args {
@@ -46,7 +48,7 @@ int of_call_prom(const char *service, int nargs, int nret, ...)
va_start(list, nret);
for (i = 0; i < nargs; i++)
- args.args[i] = va_arg(list, unsigned int);
+ args.args[i] = va_arg(list, prom_arg_t);
va_end(list);
for (i = 0; i < nret; i++)
@@ -59,7 +61,7 @@ int of_call_prom(const char *service, int nargs, int nret, ...)
}
static int of_call_prom_ret(const char *service, int nargs, int nret,
- unsigned int *rets, ...)
+ prom_arg_t *rets, ...)
{
int i;
struct prom_args args;
@@ -71,7 +73,7 @@ static int of_call_prom_ret(const char *service, int nargs, int nret,
va_start(list, rets);
for (i = 0; i < nargs; i++)
- args.args[i] = va_arg(list, unsigned int);
+ args.args[i] = va_arg(list, prom_arg_t);
va_end(list);
for (i = 0; i < nret; i++)
@@ -148,7 +150,7 @@ static int check_of_version(void)
void *of_claim(unsigned long virt, unsigned long size, unsigned long align)
{
int ret;
- unsigned int result;
+ prom_arg_t result;
if (need_map < 0)
need_map = check_of_version();
--
1.7.10.4
^ permalink raw reply related
* [RFC PATCH 00/18] powerpc/boot: 64bit little endian wrapper for pseries
From: Cédric Le Goater @ 2014-02-07 15:59 UTC (permalink / raw)
To: benh; +Cc: Cédric Le Goater, linuxppc-dev
Hi,
The following patchset adds support for 64bit little endian boot
wrapper for pseries. It is based on original code from Andrew Tauferner.
The first patches provide fixes for 64bit. I also changed the prom
code to make it converge with the prom_init kernel code. They have
a lot in common and they could probably be merged if we find a way
to do it.
powerpc/boot: fix do_div for 64bit wrapper
powerpc/boot: use a common prom_args struct in oflib
powerpc/boot: use prom_arg_t in oflib
powerpc/boot: add byteswapping routines in oflib
powerpc/boot: add PROM_ERROR define in oflib
powerpc/boot: rework of_claim() to make it 64bit friendly
powerpc/boot: define typedef ihandle as u32
powerpc/boot: fix compile warning in 64bit
These are for little endian only:
powerpc/boot: define byteswapping routines for little endian
powerpc/boot: add 64bit and little endian support to addnote
powerpc/boot: add little endian support to elf utils
and these to support a 64bit boot wrapper in both endian order :
powerpc/boot: define a routine to enter prom
powerpc/boot: modify entry point for 64bit
powerpc/boot: modify how we enter kernel on 64bit
powerpc/boot: add a global entry point for pseries
powerpc/boot: add support for 64bit big endian wrapper
powerpc/boot: add support for 64bit little endian wrapper
This final patch restores the previous configuration for 64bit
big endian kernel, which is to compile in 32bit :
powerpc/boot: add PPC64_BOOT_WRAPPER config option
Here are some initial topics to discuss :
- To compile in 64bit, -m64 is added to the cross32 compiler ...
This is not the most elegant solutions.
- There are still some compile warnings due to 64bit in the
device tree wrapper library.
- The boot wrapper is compiled as a position independent executable.
This might not be an issue though.
This patchset is based on a 3.13 and was tested on qemu with the -kernel
option on little and big endian guests. It was also tested with a
custom yaboot supporting Little Endian kernels.
Yours,
C.
Cédric Le Goater (18):
powerpc/boot: fix do_div for 64bit wrapper
powerpc/boot: use a common prom_args struct in oflib
powerpc/boot: use prom_arg_t in oflib
powerpc/boot: add byteswapping routines in oflib
powerpc/boot: add PROM_ERROR define in oflib
powerpc/boot: rework of_claim() to make it 64bit friendly
powerpc/boot: define typedef ihandle as u32
powerpc/boot: fix compile warning in 64bit
powerpc/boot: define byteswapping routines for little endian
powerpc/boot: add 64bit and little endian support to addnote
powerpc/boot: add little endian support to elf utils
powerpc/boot: define a routine to enter prom
powerpc/boot: modify entry point for 64bit
powerpc/boot: modify how we enter kernel on 64bit
powerpc/boot: add a global entry point for pseries
powerpc/boot: add support for 64bit big endian wrapper
powerpc/boot: add support for 64bit little endian wrapper
powerpc/boot: add PPC64_BOOT_WRAPPER config option
arch/powerpc/boot/Makefile | 18 +++-
arch/powerpc/boot/addnote.c | 114 +++++++++++++-------
arch/powerpc/boot/crt0.S | 180 +++++++++++++++++++++++++++++++-
arch/powerpc/boot/elf_util.c | 4 +
arch/powerpc/boot/main.c | 4 +
arch/powerpc/boot/of.c | 4 +-
arch/powerpc/boot/of.h | 17 ++-
arch/powerpc/boot/ofconsole.c | 6 +-
arch/powerpc/boot/oflib.c | 94 +++++++++--------
arch/powerpc/boot/ppc_asm.h | 12 +++
arch/powerpc/boot/pseries-head.S | 8 ++
arch/powerpc/boot/stdio.c | 14 +++
arch/powerpc/boot/swab.h | 29 +++++
arch/powerpc/boot/wrapper | 17 ++-
arch/powerpc/boot/zImage.lds.S | 25 ++++-
arch/powerpc/platforms/Kconfig.cputype | 5 +
16 files changed, 458 insertions(+), 93 deletions(-)
create mode 100644 arch/powerpc/boot/pseries-head.S
create mode 100644 arch/powerpc/boot/swab.h
--
1.7.10.4
^ permalink raw reply
* [RFC PATCH 02/18] powerpc/boot: use a common prom_args struct in oflib
From: Cédric Le Goater @ 2014-02-07 15:59 UTC (permalink / raw)
To: benh; +Cc: Cédric Le Goater, linuxppc-dev
In-Reply-To: <1391788771-16405-1-git-send-email-clg@fr.ibm.com>
This patch fixes warnings when the wrapper is compiled in 64bit and
updates the boot wrapper code related to prom to converge with the
kernel code in prom_init. This should make the review of changes easier.
The kernel has a different number of possible arguments (10) when
entering prom. There does not seem to be any good reason to have
12 in the wrapper, so the patch changes this value to args[10] in
the prom_args struct.
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
arch/powerpc/boot/of.h | 2 ++
arch/powerpc/boot/oflib.c | 29 +++++++++++++++--------------
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/boot/of.h b/arch/powerpc/boot/of.h
index e4c68f7391c5..5da03d9b9463 100644
--- a/arch/powerpc/boot/of.h
+++ b/arch/powerpc/boot/of.h
@@ -18,4 +18,6 @@ int of_setprop(const void *phandle, const char *name, const void *buf,
/* Console functions */
void of_console_init(void);
+typedef u32 __be32;
+
#endif /* _PPC_BOOT_OF_H_ */
diff --git a/arch/powerpc/boot/oflib.c b/arch/powerpc/boot/oflib.c
index b0ec9cf3eaaf..c3288a3446b3 100644
--- a/arch/powerpc/boot/oflib.c
+++ b/arch/powerpc/boot/oflib.c
@@ -16,6 +16,15 @@
#include "of.h"
+/* The following structure is used to communicate with open firmware.
+ * All arguments in and out are in big endian format. */
+struct prom_args {
+ __be32 service; /* Address of service name string. */
+ __be32 nargs; /* Number of input arguments. */
+ __be32 nret; /* Number of output arguments. */
+ __be32 args[10]; /* Input/output arguments. */
+};
+
static int (*prom) (void *);
void of_init(void *promptr)
@@ -23,18 +32,15 @@ void of_init(void *promptr)
prom = (int (*)(void *))promptr;
}
+#define ADDR(x) (u32)(unsigned long)(x)
+
int of_call_prom(const char *service, int nargs, int nret, ...)
{
int i;
- struct prom_args {
- const char *service;
- int nargs;
- int nret;
- unsigned int args[12];
- } args;
+ struct prom_args args;
va_list list;
- args.service = service;
+ args.service = ADDR(service);
args.nargs = nargs;
args.nret = nret;
@@ -56,15 +62,10 @@ static int of_call_prom_ret(const char *service, int nargs, int nret,
unsigned int *rets, ...)
{
int i;
- struct prom_args {
- const char *service;
- int nargs;
- int nret;
- unsigned int args[12];
- } args;
+ struct prom_args args;
va_list list;
- args.service = service;
+ args.service = ADDR(service);
args.nargs = nargs;
args.nret = nret;
--
1.7.10.4
^ permalink raw reply related
* [RFC PATCH 04/18] powerpc/boot: add byteswapping routines in oflib
From: Cédric Le Goater @ 2014-02-07 15:59 UTC (permalink / raw)
To: benh; +Cc: Cédric Le Goater, linuxppc-dev
In-Reply-To: <1391788771-16405-1-git-send-email-clg@fr.ibm.com>
Values will need to be byte-swapped when calling prom (big endian) from
a little endian boot wrapper.
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
arch/powerpc/boot/of.h | 3 +++
arch/powerpc/boot/ofconsole.c | 6 ++++--
arch/powerpc/boot/oflib.c | 22 +++++++++++-----------
3 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/boot/of.h b/arch/powerpc/boot/of.h
index 5da03d9b9463..40d95bf7402b 100644
--- a/arch/powerpc/boot/of.h
+++ b/arch/powerpc/boot/of.h
@@ -20,4 +20,7 @@ void of_console_init(void);
typedef u32 __be32;
+#define cpu_to_be32(x) (x)
+#define be32_to_cpu(x) (x)
+
#endif /* _PPC_BOOT_OF_H_ */
diff --git a/arch/powerpc/boot/ofconsole.c b/arch/powerpc/boot/ofconsole.c
index ce0e02424453..8b754702460a 100644
--- a/arch/powerpc/boot/ofconsole.c
+++ b/arch/powerpc/boot/ofconsole.c
@@ -18,7 +18,7 @@
#include "of.h"
-static void *of_stdout_handle;
+static unsigned int of_stdout_handle;
static int of_console_open(void)
{
@@ -27,8 +27,10 @@ static int of_console_open(void)
if (((devp = of_finddevice("/chosen")) != NULL)
&& (of_getprop(devp, "stdout", &of_stdout_handle,
sizeof(of_stdout_handle))
- == sizeof(of_stdout_handle)))
+ == sizeof(of_stdout_handle))) {
+ of_stdout_handle = be32_to_cpu(of_stdout_handle);
return 0;
+ }
return -1;
}
diff --git a/arch/powerpc/boot/oflib.c b/arch/powerpc/boot/oflib.c
index 3b0c9458504f..0f72b1a42133 100644
--- a/arch/powerpc/boot/oflib.c
+++ b/arch/powerpc/boot/oflib.c
@@ -42,13 +42,13 @@ int of_call_prom(const char *service, int nargs, int nret, ...)
struct prom_args args;
va_list list;
- args.service = ADDR(service);
- args.nargs = nargs;
- args.nret = nret;
+ args.service = cpu_to_be32(ADDR(service));
+ args.nargs = cpu_to_be32(nargs);
+ args.nret = cpu_to_be32(nret);
va_start(list, nret);
for (i = 0; i < nargs; i++)
- args.args[i] = va_arg(list, prom_arg_t);
+ args.args[i] = cpu_to_be32(va_arg(list, prom_arg_t));
va_end(list);
for (i = 0; i < nret; i++)
@@ -57,7 +57,7 @@ int of_call_prom(const char *service, int nargs, int nret, ...)
if (prom(&args) < 0)
return -1;
- return (nret > 0)? args.args[nargs]: 0;
+ return (nret > 0) ? be32_to_cpu(args.args[nargs]) : 0;
}
static int of_call_prom_ret(const char *service, int nargs, int nret,
@@ -67,13 +67,13 @@ static int of_call_prom_ret(const char *service, int nargs, int nret,
struct prom_args args;
va_list list;
- args.service = ADDR(service);
- args.nargs = nargs;
- args.nret = nret;
+ args.service = cpu_to_be32(ADDR(service));
+ args.nargs = cpu_to_be32(nargs);
+ args.nret = cpu_to_be32(nret);
va_start(list, rets);
for (i = 0; i < nargs; i++)
- args.args[i] = va_arg(list, prom_arg_t);
+ args.args[i] = cpu_to_be32(va_arg(list, prom_arg_t));
va_end(list);
for (i = 0; i < nret; i++)
@@ -84,9 +84,9 @@ static int of_call_prom_ret(const char *service, int nargs, int nret,
if (rets != (void *) 0)
for (i = 1; i < nret; ++i)
- rets[i-1] = args.args[nargs+i];
+ rets[i-1] = be32_to_cpu(args.args[nargs+i]);
- return (nret > 0)? args.args[nargs]: 0;
+ return (nret > 0) ? be32_to_cpu(args.args[nargs]) : 0;
}
/* returns true if s2 is a prefix of s1 */
--
1.7.10.4
^ permalink raw reply related
* [RFC PATCH 05/18] powerpc/boot: add PROM_ERROR define in oflib
From: Cédric Le Goater @ 2014-02-07 15:59 UTC (permalink / raw)
To: benh; +Cc: Cédric Le Goater, linuxppc-dev
In-Reply-To: <1391788771-16405-1-git-send-email-clg@fr.ibm.com>
This is mostly useful to make to the boot wrapper code closer with
the kernel code in prom_init.
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
arch/powerpc/boot/oflib.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/boot/oflib.c b/arch/powerpc/boot/oflib.c
index 0f72b1a42133..96fe4d225abe 100644
--- a/arch/powerpc/boot/oflib.c
+++ b/arch/powerpc/boot/oflib.c
@@ -27,6 +27,8 @@ struct prom_args {
__be32 args[10]; /* Input/output arguments. */
};
+#define PROM_ERROR (-1u)
+
static int (*prom) (void *);
void of_init(void *promptr)
@@ -55,7 +57,7 @@ int of_call_prom(const char *service, int nargs, int nret, ...)
args.args[nargs+i] = 0;
if (prom(&args) < 0)
- return -1;
+ return PROM_ERROR;
return (nret > 0) ? be32_to_cpu(args.args[nargs]) : 0;
}
@@ -80,9 +82,9 @@ static int of_call_prom_ret(const char *service, int nargs, int nret,
args.args[nargs+i] = 0;
if (prom(&args) < 0)
- return -1;
+ return PROM_ERROR;
- if (rets != (void *) 0)
+ if (rets != NULL)
for (i = 1; i < nret; ++i)
rets[i-1] = be32_to_cpu(args.args[nargs+i]);
--
1.7.10.4
^ permalink raw reply related
* [RFC PATCH 07/18] powerpc/boot: define typedef ihandle as u32
From: Cédric Le Goater @ 2014-02-07 15:59 UTC (permalink / raw)
To: benh; +Cc: Cédric Le Goater, linuxppc-dev
In-Reply-To: <1391788771-16405-1-git-send-email-clg@fr.ibm.com>
This makes ihandle 64bit friendly.
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
arch/powerpc/boot/of.h | 2 +-
arch/powerpc/boot/oflib.c | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/boot/of.h b/arch/powerpc/boot/of.h
index 3f35cf0ec432..bf228fea3517 100644
--- a/arch/powerpc/boot/of.h
+++ b/arch/powerpc/boot/of.h
@@ -2,7 +2,7 @@
#define _PPC_BOOT_OF_H_
typedef void *phandle;
-typedef void *ihandle;
+typedef u32 ihandle;
void of_init(void *promptr);
int of_call_prom(const char *service, int nargs, int nret, ...);
diff --git a/arch/powerpc/boot/oflib.c b/arch/powerpc/boot/oflib.c
index cb6a5de0c257..5ab9cb57cd31 100644
--- a/arch/powerpc/boot/oflib.c
+++ b/arch/powerpc/boot/oflib.c
@@ -108,7 +108,7 @@ static int string_match(const char *s1, const char *s2)
*/
static int need_map = -1;
static ihandle chosen_mmu;
-static phandle memory;
+static ihandle memory;
static int check_of_version(void)
{
@@ -137,10 +137,10 @@ static int check_of_version(void)
printf("no mmu\n");
return 0;
}
- memory = (ihandle) of_call_prom("open", 1, 1, "/memory");
- if (memory == (ihandle) -1) {
- memory = (ihandle) of_call_prom("open", 1, 1, "/memory@0");
- if (memory == (ihandle) -1) {
+ memory = of_call_prom("open", 1, 1, "/memory");
+ if (memory == PROM_ERROR) {
+ memory = of_call_prom("open", 1, 1, "/memory@0");
+ if (memory == PROM_ERROR) {
printf("no memory node\n");
return 0;
}
--
1.7.10.4
^ permalink raw reply related
* [RFC PATCH 06/18] powerpc/boot: rework of_claim() to make it 64bit friendly
From: Cédric Le Goater @ 2014-02-07 15:59 UTC (permalink / raw)
To: benh; +Cc: Cédric Le Goater, linuxppc-dev
In-Reply-To: <1391788771-16405-1-git-send-email-clg@fr.ibm.com>
This patch fixes 64bit compile warnings and updates the wrapper code
to converge the kernel code in prom_init.
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
arch/powerpc/boot/of.c | 4 ++--
arch/powerpc/boot/of.h | 3 ++-
arch/powerpc/boot/oflib.c | 15 ++++++++-------
3 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/boot/of.c b/arch/powerpc/boot/of.c
index 62e2f43ec1df..6836a2b2112a 100644
--- a/arch/powerpc/boot/of.c
+++ b/arch/powerpc/boot/of.c
@@ -40,8 +40,8 @@ static void *of_try_claim(unsigned long size)
#ifdef DEBUG
printf(" trying: 0x%08lx\n\r", claim_base);
#endif
- addr = (unsigned long)of_claim(claim_base, size, 0);
- if ((void *)addr != (void *)-1)
+ addr = (unsigned long) of_claim(claim_base, size, 0);
+ if (addr != -1)
break;
}
if (addr == 0)
diff --git a/arch/powerpc/boot/of.h b/arch/powerpc/boot/of.h
index 40d95bf7402b..3f35cf0ec432 100644
--- a/arch/powerpc/boot/of.h
+++ b/arch/powerpc/boot/of.h
@@ -6,7 +6,8 @@ typedef void *ihandle;
void of_init(void *promptr);
int of_call_prom(const char *service, int nargs, int nret, ...);
-void *of_claim(unsigned long virt, unsigned long size, unsigned long align);
+unsigned int of_claim(unsigned long virt, unsigned long size,
+ unsigned long align);
void *of_vmlinux_alloc(unsigned long size);
void of_exit(void);
void *of_finddevice(const char *name);
diff --git a/arch/powerpc/boot/oflib.c b/arch/powerpc/boot/oflib.c
index 96fe4d225abe..cb6a5de0c257 100644
--- a/arch/powerpc/boot/oflib.c
+++ b/arch/powerpc/boot/oflib.c
@@ -149,7 +149,8 @@ static int check_of_version(void)
return 1;
}
-void *of_claim(unsigned long virt, unsigned long size, unsigned long align)
+unsigned int of_claim(unsigned long virt, unsigned long size,
+ unsigned long align)
{
int ret;
prom_arg_t result;
@@ -157,32 +158,32 @@ void *of_claim(unsigned long virt, unsigned long size, unsigned long align)
if (need_map < 0)
need_map = check_of_version();
if (align || !need_map)
- return (void *) of_call_prom("claim", 3, 1, virt, size, align);
+ return of_call_prom("claim", 3, 1, virt, size, align);
ret = of_call_prom_ret("call-method", 5, 2, &result, "claim", memory,
align, size, virt);
if (ret != 0 || result == -1)
- return (void *) -1;
+ return -1;
ret = of_call_prom_ret("call-method", 5, 2, &result, "claim", chosen_mmu,
align, size, virt);
/* 0x12 == coherent + read/write */
ret = of_call_prom("call-method", 6, 1, "map", chosen_mmu,
0x12, size, virt, virt);
- return (void *) virt;
+ return virt;
}
void *of_vmlinux_alloc(unsigned long size)
{
unsigned long start = (unsigned long)_start, end = (unsigned long)_end;
- void *addr;
+ unsigned long addr;
void *p;
/* With some older POWER4 firmware we need to claim the area the kernel
* will reside in. Newer firmwares don't need this so we just ignore
* the return value.
*/
- addr = of_claim(start, end - start, 0);
- printf("Trying to claim from 0x%lx to 0x%lx (0x%lx) got %p\r\n",
+ addr = (unsigned long) of_claim(start, end - start, 0);
+ printf("Trying to claim from 0x%lx to 0x%lx (0x%lx) got %lx\r\n",
start, end, end - start, addr);
p = malloc(size);
--
1.7.10.4
^ permalink raw reply related
* [RFC PATCH 10/18] powerpc/boot: add 64bit and little endian support to addnote
From: Cédric Le Goater @ 2014-02-07 15:59 UTC (permalink / raw)
To: benh; +Cc: Cédric Le Goater, linuxppc-dev
In-Reply-To: <1391788771-16405-1-git-send-email-clg@fr.ibm.com>
It could certainly be improved using Elf macros and byteswapping
routines, but the initial version of the code is organised to be a
single file program with limited dependencies. yaboot is the same.
Please scream if you want a total rewrite.
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
>From the comment in the header :
"This is needed for OF on RS/6000s to load an image correctly"
If so, the data written in the ELF note should be in Big Endian as this
code is doing ?
arch/powerpc/boot/addnote.c | 114 +++++++++++++++++++++++++++++--------------
1 file changed, 78 insertions(+), 36 deletions(-)
diff --git a/arch/powerpc/boot/addnote.c b/arch/powerpc/boot/addnote.c
index 349b5530d2c4..cb7faedf9377 100644
--- a/arch/powerpc/boot/addnote.c
+++ b/arch/powerpc/boot/addnote.c
@@ -6,6 +6,8 @@
*
* Copyright 2000 Paul Mackerras.
*
+ * Adapted for 64 bit little endian images by Andrew Tauferner.
+ *
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version
@@ -56,35 +58,60 @@ unsigned int rpanote[N_RPA_DESCR] = {
#define ROUNDUP(len) (((len) + 3) & ~3)
unsigned char buf[512];
+#define ELFDATA2LSB 1
+#define ELFDATA2MSB 2
+static int e_data = ELFDATA2MSB;
+#define ELFCLASS32 1
+#define ELFCLASS64 2
+static int e_class = ELFCLASS32;
#define GET_16BE(off) ((buf[off] << 8) + (buf[(off)+1]))
-#define GET_32BE(off) ((GET_16BE(off) << 16) + GET_16BE((off)+2))
-
-#define PUT_16BE(off, v) (buf[off] = ((v) >> 8) & 0xff, \
- buf[(off) + 1] = (v) & 0xff)
-#define PUT_32BE(off, v) (PUT_16BE((off), (v) >> 16), \
- PUT_16BE((off) + 2, (v)))
+#define GET_32BE(off) ((GET_16BE(off) << 16U) + GET_16BE((off)+2U))
+#define GET_64BE(off) ((((unsigned long long)GET_32BE(off)) << 32ULL) + \
+ ((unsigned long long)GET_32BE((off)+4ULL)))
+#define PUT_16BE(off, v)(buf[off] = ((v) >> 8) & 0xff, \
+ buf[(off) + 1] = (v) & 0xff)
+#define PUT_32BE(off, v)(PUT_16BE((off), (v) >> 16L), PUT_16BE((off) + 2, (v)))
+#define PUT_64BE(off, v)((PUT_32BE((off), (v) >> 32L), \
+ PUT_32BE((off) + 4, (v))))
+
+#define GET_16LE(off) ((buf[off]) + (buf[(off)+1] << 8))
+#define GET_32LE(off) (GET_16LE(off) + (GET_16LE((off)+2U) << 16U))
+#define GET_64LE(off) ((unsigned long long)GET_32LE(off) + \
+ (((unsigned long long)GET_32LE((off)+4ULL)) << 32ULL))
+#define PUT_16LE(off, v) (buf[off] = (v) & 0xff, \
+ buf[(off) + 1] = ((v) >> 8) & 0xff)
+#define PUT_32LE(off, v) (PUT_16LE((off), (v)), PUT_16LE((off) + 2, (v) >> 16L))
+#define PUT_64LE(off, v) (PUT_32LE((off), (v)), PUT_32LE((off) + 4, (v) >> 32L))
+
+#define GET_16(off) (e_data == ELFDATA2MSB ? GET_16BE(off) : GET_16LE(off))
+#define GET_32(off) (e_data == ELFDATA2MSB ? GET_32BE(off) : GET_32LE(off))
+#define GET_64(off) (e_data == ELFDATA2MSB ? GET_64BE(off) : GET_64LE(off))
+#define PUT_16(off, v) (e_data == ELFDATA2MSB ? PUT_16BE(off, v) : \
+ PUT_16LE(off, v))
+#define PUT_32(off, v) (e_data == ELFDATA2MSB ? PUT_32BE(off, v) : \
+ PUT_32LE(off, v))
+#define PUT_64(off, v) (e_data == ELFDATA2MSB ? PUT_64BE(off, v) : \
+ PUT_64LE(off, v))
/* Structure of an ELF file */
#define E_IDENT 0 /* ELF header */
-#define E_PHOFF 28
-#define E_PHENTSIZE 42
-#define E_PHNUM 44
-#define E_HSIZE 52 /* size of ELF header */
+#define E_PHOFF (e_class == ELFCLASS32 ? 28 : 32)
+#define E_PHENTSIZE (e_class == ELFCLASS32 ? 42 : 54)
+#define E_PHNUM (e_class == ELFCLASS32 ? 44 : 56)
+#define E_HSIZE (e_class == ELFCLASS32 ? 52 : 64)
#define EI_MAGIC 0 /* offsets in E_IDENT area */
#define EI_CLASS 4
#define EI_DATA 5
#define PH_TYPE 0 /* ELF program header */
-#define PH_OFFSET 4
-#define PH_FILESZ 16
-#define PH_HSIZE 32 /* size of program header */
+#define PH_OFFSET (e_class == ELFCLASS32 ? 4 : 8)
+#define PH_FILESZ (e_class == ELFCLASS32 ? 16 : 32)
+#define PH_HSIZE (e_class == ELFCLASS32 ? 32 : 56)
#define PT_NOTE 4 /* Program header type = note */
-#define ELFCLASS32 1
-#define ELFDATA2MSB 2
unsigned char elf_magic[4] = { 0x7f, 'E', 'L', 'F' };
@@ -92,8 +119,8 @@ int
main(int ac, char **av)
{
int fd, n, i;
- int ph, ps, np;
- int nnote, nnote2, ns;
+ unsigned long ph, ps, np;
+ long nnote, nnote2, ns;
if (ac != 2) {
fprintf(stderr, "Usage: %s elf-file\n", av[0]);
@@ -114,26 +141,27 @@ main(int ac, char **av)
exit(1);
}
- if (n < E_HSIZE || memcmp(&buf[E_IDENT+EI_MAGIC], elf_magic, 4) != 0)
+ if (memcmp(&buf[E_IDENT+EI_MAGIC], elf_magic, 4) != 0)
+ goto notelf;
+ e_class = buf[E_IDENT+EI_CLASS];
+ if (e_class != ELFCLASS32 && e_class != ELFCLASS64)
+ goto notelf;
+ e_data = buf[E_IDENT+EI_DATA];
+ if (e_data != ELFDATA2MSB && e_data != ELFDATA2LSB)
+ goto notelf;
+ if (n < E_HSIZE)
goto notelf;
- if (buf[E_IDENT+EI_CLASS] != ELFCLASS32
- || buf[E_IDENT+EI_DATA] != ELFDATA2MSB) {
- fprintf(stderr, "%s is not a big-endian 32-bit ELF image\n",
- av[1]);
- exit(1);
- }
-
- ph = GET_32BE(E_PHOFF);
- ps = GET_16BE(E_PHENTSIZE);
- np = GET_16BE(E_PHNUM);
+ ph = (e_class == ELFCLASS32 ? GET_32(E_PHOFF) : GET_64(E_PHOFF));
+ ps = GET_16(E_PHENTSIZE);
+ np = GET_16(E_PHNUM);
if (ph < E_HSIZE || ps < PH_HSIZE || np < 1)
goto notelf;
if (ph + (np + 2) * ps + nnote + nnote2 > n)
goto nospace;
for (i = 0; i < np; ++i) {
- if (GET_32BE(ph + PH_TYPE) == PT_NOTE) {
+ if (GET_32(ph + PH_TYPE) == PT_NOTE) {
fprintf(stderr, "%s already has a note entry\n",
av[1]);
exit(0);
@@ -148,9 +176,16 @@ main(int ac, char **av)
/* fill in the program header entry */
ns = ph + 2 * ps;
- PUT_32BE(ph + PH_TYPE, PT_NOTE);
- PUT_32BE(ph + PH_OFFSET, ns);
- PUT_32BE(ph + PH_FILESZ, nnote);
+ PUT_32(ph + PH_TYPE, PT_NOTE);
+ if (e_class == ELFCLASS32)
+ PUT_32(ph + PH_OFFSET, ns);
+ else
+ PUT_64(ph + PH_OFFSET, ns);
+
+ if (e_class == ELFCLASS32)
+ PUT_32(ph + PH_FILESZ, nnote);
+ else
+ PUT_64(ph + PH_FILESZ, nnote);
/* fill in the note area we point to */
/* XXX we should probably make this a proper section */
@@ -164,9 +199,16 @@ main(int ac, char **av)
/* fill in the second program header entry and the RPA note area */
ph += ps;
- PUT_32BE(ph + PH_TYPE, PT_NOTE);
- PUT_32BE(ph + PH_OFFSET, ns);
- PUT_32BE(ph + PH_FILESZ, nnote2);
+ PUT_32(ph + PH_TYPE, PT_NOTE);
+ if (e_class == ELFCLASS32)
+ PUT_32(ph + PH_OFFSET, ns);
+ else
+ PUT_64(ph + PH_OFFSET, ns);
+
+ if (e_class == ELFCLASS32)
+ PUT_32(ph + PH_FILESZ, nnote);
+ else
+ PUT_64(ph + PH_FILESZ, nnote2);
/* fill in the note area we point to */
PUT_32BE(ns, strlen(rpaname) + 1);
@@ -178,7 +220,7 @@ main(int ac, char **av)
PUT_32BE(ns, rpanote[i]);
/* Update the number of program headers */
- PUT_16BE(E_PHNUM, np + 2);
+ PUT_16(E_PHNUM, np + 2);
/* write back */
lseek(fd, (long) 0, SEEK_SET);
--
1.7.10.4
^ permalink raw reply related
* [RFC PATCH 08/18] powerpc/boot: fix compile warning in 64bit
From: Cédric Le Goater @ 2014-02-07 15:59 UTC (permalink / raw)
To: benh; +Cc: Cédric Le Goater, linuxppc-dev
In-Reply-To: <1391788771-16405-1-git-send-email-clg@fr.ibm.com>
arch/powerpc/boot/oflib.c:211:9: warning: cast to pointer from integer of \
different size [-Wint-to-pointer-cast]
return (phandle) of_call_prom("finddevice", 1, 1, name);
This is a work around. The definite solution would be to define the
phandle typedef as a u32, as in the kernel, but this would break the
device tree ops API.
Let it be for the moment.
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
arch/powerpc/boot/oflib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/boot/oflib.c b/arch/powerpc/boot/oflib.c
index 5ab9cb57cd31..e8697df2df27 100644
--- a/arch/powerpc/boot/oflib.c
+++ b/arch/powerpc/boot/oflib.c
@@ -203,7 +203,7 @@ void of_exit(void)
*/
void *of_finddevice(const char *name)
{
- return (phandle) of_call_prom("finddevice", 1, 1, name);
+ return (void *) (unsigned long) of_call_prom("finddevice", 1, 1, name);
}
int of_getprop(const void *phandle, const char *name, void *buf,
--
1.7.10.4
^ permalink raw reply related
* [RFC PATCH 11/18] powerpc/boot: add little endian support to elf utils
From: Cédric Le Goater @ 2014-02-07 15:59 UTC (permalink / raw)
To: benh; +Cc: Cédric Le Goater, linuxppc-dev
In-Reply-To: <1391788771-16405-1-git-send-email-clg@fr.ibm.com>
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
arch/powerpc/boot/elf_util.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/boot/elf_util.c b/arch/powerpc/boot/elf_util.c
index 1567a0c0f05c..316552dea4d8 100644
--- a/arch/powerpc/boot/elf_util.c
+++ b/arch/powerpc/boot/elf_util.c
@@ -26,7 +26,11 @@ int parse_elf64(void *hdr, struct elf_info *info)
elf64->e_ident[EI_MAG2] == ELFMAG2 &&
elf64->e_ident[EI_MAG3] == ELFMAG3 &&
elf64->e_ident[EI_CLASS] == ELFCLASS64 &&
+#ifdef __LITTLE_ENDIAN__
+ elf64->e_ident[EI_DATA] == ELFDATA2LSB &&
+#else
elf64->e_ident[EI_DATA] == ELFDATA2MSB &&
+#endif
(elf64->e_type == ET_EXEC ||
elf64->e_type == ET_DYN) &&
elf64->e_machine == EM_PPC64))
--
1.7.10.4
^ permalink raw reply related
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