* Re: [PATCH] powerpc: Optimise the 64bit optimised __clear_user
From: Segher Boessenkool @ 2012-06-06 16:40 UTC (permalink / raw)
To: Anton Blanchard; +Cc: mikey, michael, paulus, olof, linuxppc-dev
In-Reply-To: <20120605120222.6722a3e3@kryten>
> +err1; dcbz r0,r3
There is no such instruction, you probably meant "dcbz 0,r3"?
Segher
^ permalink raw reply
* Re: [PATCH 2/2] [POWERPC] uprobes: powerpc port
From: Jim Keniston @ 2012-06-06 18:08 UTC (permalink / raw)
To: ananth
Cc: Srikar Dronamraju, Peter Zijlstra, oleg, lkml, Paul Mackerras,
Anton Blanchard, Ingo Molnar, linuxppc-dev
In-Reply-To: <20120606093541.GA29580@in.ibm.com>
On Wed, 2012-06-06 at 15:05 +0530, Ananth N Mavinakayanahalli wrote:
> On Wed, Jun 06, 2012 at 11:27:02AM +0200, Peter Zijlstra wrote:
> > On Wed, 2012-06-06 at 14:51 +0530, Ananth N Mavinakayanahalli wrote:
> > > One TODO in this port compared to x86 is the uprobe abort_xol() logic.
> > > x86 depends on the thread_struct.trap_nr (absent in powerpc) to determine
> > > if a signal was caused when the uprobed instruction was single-stepped/
> > > emulated, in which case, we reset the instruction pointer to the probed
> > > address and retry the probe again.
> >
> > Another curious difference is that x86 uses an instruction decoder and
> > contains massive tables to validate we can probe a particular
> > instruction.
Part of that difference is because the x86 instruction set is a lot more
complex. Another part is due to the lack, back when the x86 code was
created, of robust handling by uprobes of traps by probed instructions.
So we refused to probe instructions that we knew (or strongly suspected)
would generate traps in user mode -- e.g., privileged instructions,
illegal instructions. A couple of times we had to "legalize"
instructions or prefixes that we didn't originally expect to encounter.
> >
> > Can we probe all possible PPC instructions?
>
> For the kernel, the only ones that are off limits are rfi (return from
> interrupt), mtmsr (move to msr). All other instructions can be probed.
>
> Both those instructions are supervisor level, so we won't see them in
> userspace at all; so we should be able to probe all user level
> instructions.
Presumably rfi or mtmsr could show up in the instruction stream via an
erroneous or mischievous asm statement. It'd be good to verify that you
handle that gracefully.
>
> I am not aware of specific caveats for vector/altivec instructions;
> maybe Paul or Ben are more suitable to comment on that.
>
> Ananth
>
Jim
^ permalink raw reply
* Re: [PATCH v5 2/5] powerpc/85xx: add HOTPLUG_CPU support
From: Scott Wood @ 2012-06-06 18:19 UTC (permalink / raw)
To: Zhao Chenhui; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20120606095910.GB23505@localhost.localdomain>
On 06/06/2012 04:59 AM, Zhao Chenhui wrote:
> On Tue, Jun 05, 2012 at 11:15:52AM -0500, Scott Wood wrote:
>> On 06/05/2012 06:18 AM, Zhao Chenhui wrote:
>>> If user does not enable kexec or hotplug, these codes are redundant.
>>> So use CONFIG_KEXEC and CONFIG_HOTPLUG_CPU to gard them.
>>
>> My point is that these lists tend to grow and be a maintenance pain.
>> For small things it's often better to not worry about saving a few
>> bytes. For larger things that need to be conditional, define a new
>> symbol rather than growing ORed lists like this.
>>
>> -Scott
>
> I agree with you in principle. But there are only two config options
> in this patch, and it is unlikely to grow.
That's what everybody says when these things start. :-)
-Scott
^ permalink raw reply
* Re: [PATCH v5 1/5] powerpc/85xx: implement hardware timebase sync
From: Scott Wood @ 2012-06-06 18:26 UTC (permalink / raw)
To: Zhao Chenhui; +Cc: Matthew McClintock, linuxppc-dev, linux-kernel
In-Reply-To: <20120606093142.GA23505@localhost.localdomain>
On 06/06/2012 04:31 AM, Zhao Chenhui wrote:
> On Tue, Jun 05, 2012 at 11:07:41AM -0500, Scott Wood wrote:
>> On 06/05/2012 04:08 AM, Zhao Chenhui wrote:
>>> On Fri, Jun 01, 2012 at 10:40:00AM -0500, Scott Wood wrote:
>>>> I know you say this is for dual-core chips only, but it would be nice if
>>>> you'd write this in a way that doesn't assume that (even if the
>>>> corenet-specific timebase freezing comes later).
>>>
>>> At this point, I have not thought about how to implement the cornet-specific timebase freezing.
>>
>> I wasn't asking you to. I was asking you to not have logic that breaks
>> with more than 2 CPUs.
>
> These routines only called in the dual-core case.
Come on, you know we have chips with more than two cores. Why design
such a limitation into it, just because you're not personally interested
in supporting anything but e500v2?
Is it so hard to make it work for an arbitrary number of cores?
>>> If do not set them, it may make KEXEC fail on other platforms.
>>
>> What platforms?
>
> Such as P4080, P3041, etc.
So we need to wait for corenet timebase sync before we stop causing
problems in virtualization, simulators, etc. if a kernel has kexec or
cpu hotplug enabled (whether used or not)?
Can you at least make sure we're actually in a kexec/hotplug scenario at
runtime?
Or just implement corenet timebase sync -- it's not that different.
-Scott
^ permalink raw reply
* Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source
From: Scott Wood @ 2012-06-06 18:29 UTC (permalink / raw)
To: Li Yang
Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org, Li Yang-R58472,
linux-kernel@vger.kernel.org, Zhao Chenhui-B35336
In-Reply-To: <CADRPPNSzTNPkNExBnNu1iD2hqyRG=Cw+y+PnjTRWxjP_ib1Kww@mail.gmail.com>
On 06/05/2012 11:06 PM, Li Yang wrote:
> On Wed, Jun 6, 2012 at 2:05 AM, Scott Wood <scottwood@freescale.com> wrote:
>> You ignored "what about devices other than ethernet".
>
> No, I haven't. Other devices are so at least for now.
I don't understand that last sentence. Other devices are what?
-Scott
^ permalink raw reply
* Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
From: Scott Wood @ 2012-06-06 21:15 UTC (permalink / raw)
To: Ben Collins; +Cc: linuxppc-dev
In-Reply-To: <4DC27253-67FC-4A55-8C78-7782D9D0CF53@servergy.com>
On 06/05/2012 10:50 PM, Ben Collins wrote:
> The commit introducing pcibios_io_space_offset() was ignoring 32-bit to
> 64-bit sign extention, which is the case on ppc32 with 64-bit resource
> addresses. This only seems to have shown up while running under QEMU for
> e500mc target. It may or may be suboptimal that QEMU has an IO base
> address > 32-bits for the e500-pci implementation, but 1) it's still a
> regression and 2) it's more correct to handle things this way.
Where do you see addresses over 32 bits in QEMU's e500-pci, at least
with current mainline QEMU and the mpc8544ds model?
I/O space should be at 0xe1000000.
I'm also not sure what this has to do with the virtual address returned
by ioremap().
> Signed-off-by: Ben Collins <bcollins@ubuntu.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/powerpc/kernel/pci-common.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 8e78e93..be9ced7 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1477,9 +1477,15 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
> return pci_enable_resources(dev, mask);
> }
>
> +/* Before assuming too much here, take care to realize that we need sign
> + * extension from 32-bit pointers to 64-bit resource addresses to work.
> + */
> resource_size_t pcibios_io_space_offset(struct pci_controller *hose)
> {
> - return (unsigned long) hose->io_base_virt - _IO_BASE;
> + long vbase = (long)hose->io_base_virt;
> + long io_base = _IO_BASE;
> +
> + return (resource_size_t)(vbase - io_base);
Why do we want sign extension here?
If we do want it, there are a lot of other places in this file where the
same calculation is done.
-Scott
^ permalink raw reply
* Re: [PATCH] powerpc: Optimise the 64bit optimised __clear_user
From: Benjamin Herrenschmidt @ 2012-06-06 21:20 UTC (permalink / raw)
To: Segher Boessenkool
Cc: mikey, michael, paulus, Anton Blanchard, olof, linuxppc-dev
In-Reply-To: <178E3BC0-C6E2-4E33-BA66-8144F192A151@kernel.crashing.org>
On Wed, 2012-06-06 at 18:40 +0200, Segher Boessenkool wrote:
> > +err1; dcbz r0,r3
>
> There is no such instruction, you probably meant "dcbz 0,r3"?
This reminds me... what would happen if we changed all our
#define r0 0
#define r1 1
etc... to:
#define r0 %r0
#define r1 %r1
?
I'm thinking it might help catch that sort of nasties (and some of them
can be really nasty, such as inverting mfspr/mtspr arguments, or vs ori,
etc... ). I'm sure we'd have a problem with a few macros & inline
constructs but nothing we can't fix..
(Haven't tested ... still home, officially sick :-)
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
From: Benjamin Herrenschmidt @ 2012-06-06 22:21 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Ben Collins
In-Reply-To: <4FCFC870.40004@freescale.com>
On Wed, 2012-06-06 at 16:15 -0500, Scott Wood wrote:
> On 06/05/2012 10:50 PM, Ben Collins wrote:
> > The commit introducing pcibios_io_space_offset() was ignoring 32-bit to
> > 64-bit sign extention, which is the case on ppc32 with 64-bit resource
> > addresses. This only seems to have shown up while running under QEMU for
> > e500mc target. It may or may be suboptimal that QEMU has an IO base
> > address > 32-bits for the e500-pci implementation, but 1) it's still a
> > regression and 2) it's more correct to handle things this way.
>
> Where do you see addresses over 32 bits in QEMU's e500-pci, at least
> with current mainline QEMU and the mpc8544ds model?
>
> I/O space should be at 0xe1000000.
>
> I'm also not sure what this has to do with the virtual address returned
> by ioremap().
This is due to how we calculate IO offsets on ppc32, see below
> > Signed-off-by: Ben Collins <bcollins@ubuntu.com>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > ---
> > arch/powerpc/kernel/pci-common.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> > index 8e78e93..be9ced7 100644
> > --- a/arch/powerpc/kernel/pci-common.c
> > +++ b/arch/powerpc/kernel/pci-common.c
> > @@ -1477,9 +1477,15 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
> > return pci_enable_resources(dev, mask);
> > }
> >
> > +/* Before assuming too much here, take care to realize that we need sign
> > + * extension from 32-bit pointers to 64-bit resource addresses to work.
> > + */
> > resource_size_t pcibios_io_space_offset(struct pci_controller *hose)
> > {
> > - return (unsigned long) hose->io_base_virt - _IO_BASE;
> > + long vbase = (long)hose->io_base_virt;
> > + long io_base = _IO_BASE;
> > +
> > + return (resource_size_t)(vbase - io_base);
>
> Why do we want sign extension here?
>
> If we do want it, there are a lot of other places in this file where the
> same calculation is done.
We should probably as much as possible factor it, but basically what
happens is that to access IO space, we turn:
oub(port)
into
out_8(_IO_BASE + port)
With _IO_BASE being a global.
Now what happens when you have several PHBs ? Well, we make _IO_BASE be
the result of ioremap'ing the IO space window of the first one, minus
the bus address corresponding to the beginning of that window. Then for
each device, we offset devices with the offset calculated above.
Now that means that we can end up with funky arithmetic in a couple of
cases:
- If the bus address of the IO space is larger than the virtual address
returned by ioremap (it's a bit silly to use large IO addresses but it's
technically possible, normally IO windows start at 0 bus-side though).
In fact I wouldn't be surprised if we have various other bugs if IO
windows don't start at 0 (you may want to double check your dts setup
here).
- If the ioremap'ed address of the IO space of another domain is lower
than the ioremap'ed address of the first domain, in which case the
calculation:
host->io_base_virt - _IO_BASE
results in a negative offset.
Thus we need to make sure that this offset is fully sign extended so
that things work properly when applied to a resource_size_t which can be
64-bit.
On ppc64 we do things differently, we have a single linear region that
has all IO spaces and _IO_BASE is the beginning of it so offsets are
never negative, we can do that because we don't care wasting address
space there.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
From: Ben Collins @ 2012-06-06 23:35 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <4FCFC870.40004@freescale.com>
On Jun 6, 2012, at 5:15 PM, Scott Wood wrote:
> On 06/05/2012 10:50 PM, Ben Collins wrote:
>> The commit introducing pcibios_io_space_offset() was ignoring 32-bit =
to
>> 64-bit sign extention, which is the case on ppc32 with 64-bit =
resource
>> addresses. This only seems to have shown up while running under QEMU =
for
>> e500mc target. It may or may be suboptimal that QEMU has an IO base
>> address > 32-bits for the e500-pci implementation, but 1) it's still =
a
>> regression and 2) it's more correct to handle things this way.
>=20
> Where do you see addresses over 32 bits in QEMU's e500-pci, at least
> with current mainline QEMU and the mpc8544ds model?
>=20
> I/O space should be at 0xe1000000.
The problem is this:
pci_bus 0000:00: root bus resource [io 0xffbed000-0xffbfcfff] (bus =
address [0x100000000-0x10000ffff])
Without the fix that I sent, it ends up looking like:
pci_bus 0000:00: root bus resource [io 0xffbed000-0xffbfcfff] (bus =
address [0x0000-0xffff])
And that's when some devices fail to be assigned valid bar 0's and the =
kernel complains because of it.
> I'm also not sure what this has to do with the virtual address =
returned
> by ioremap().
>=20
>> Signed-off-by: Ben Collins <bcollins@ubuntu.com>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> ---
>> arch/powerpc/kernel/pci-common.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>=20
>> diff --git a/arch/powerpc/kernel/pci-common.c =
b/arch/powerpc/kernel/pci-common.c
>> index 8e78e93..be9ced7 100644
>> --- a/arch/powerpc/kernel/pci-common.c
>> +++ b/arch/powerpc/kernel/pci-common.c
>> @@ -1477,9 +1477,15 @@ int pcibios_enable_device(struct pci_dev *dev, =
int mask)
>> return pci_enable_resources(dev, mask);
>> }
>>=20
>> +/* Before assuming too much here, take care to realize that we need =
sign
>> + * extension from 32-bit pointers to 64-bit resource addresses to =
work.
>> + */
>> resource_size_t pcibios_io_space_offset(struct pci_controller *hose)
>> {
>> - return (unsigned long) hose->io_base_virt - _IO_BASE;
>> + long vbase =3D (long)hose->io_base_virt;
>> + long io_base =3D _IO_BASE;
>> +
>> + return (resource_size_t)(vbase - io_base);
>=20
> Why do we want sign extension here?
>=20
> If we do want it, there are a lot of other places in this file where =
the
> same calculation is done.
>=20
> -Scott
>=20
--
Ben Collins
Servergy, Inc.
(757) 243-7557
CONFIDENTIALITY NOTICE: This communication contains privileged and/or =
confidential information; and should be maintained with the strictest =
confidence. It is intended solely for the use of the person or entity in =
which it is addressed. If you are not the intended recipient, you are =
STRICTLY PROHIBITED from disclosing, copying, distributing or using any =
of this information. If you received this communication in error, please =
contact the sender immediately and destroy the material in its entirety, =
whether electronic or hard copy.
^ permalink raw reply
* Re: [PATCH] powerpc: Optimise the 64bit optimised __clear_user
From: Olof Johansson @ 2012-06-07 0:30 UTC (permalink / raw)
To: Anton Blanchard; +Cc: mikey, michael, paulus, linuxppc-dev
In-Reply-To: <20120605120222.6722a3e3@kryten>
On Mon, Jun 4, 2012 at 7:02 PM, Anton Blanchard <anton@samba.org> wrote:
>
> I blame Mikey for this. He elevated my slightly dubious testcase:
>
> # dd if=/dev/zero of=/dev/null bs=1M count=10000
>
> to benchmark status. And naturally we need to be number 1 at creating
> zeros. So lets improve __clear_user some more.
>
> As Paul suggests we can use dcbz for large lengths. This patch gets
> the destination cacheline aligned then uses dcbz on whole cachelines.
>
> Before:
> 10485760000 bytes (10 GB) copied, 0.414744 s, 25.3 GB/s
>
> After:
> 10485760000 bytes (10 GB) copied, 0.268597 s, 39.0 GB/s
>
> 39 GB/s, a new record.
>
> Signed-off-by: Anton Blanchard <anton@samba.org>
Besides the comments from Segher, feel free to add:
Tested-by: Olof Johansson <olof@lixom.net>
Acked-by: Olof Johansson <olof@lixom.net>
Didn't help performance all that much on pa6t, but it didn't go down.
Too low on cycles to actually analyze why at this time.
-OIof
^ permalink raw reply
* Re: [RFC PATCH] sched/numa: do load balance between remote nodes
From: Alex Shi @ 2012-06-07 0:33 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-mips, linux-ia64, linux-sh, dhowells, paulus, hpa,
sparclinux, mingo, sivanich, x86, greg.pearson, chris.mason,
arjan.van.de.ven, mattst88, pjt, fenghua.yu, seto.hidetoshi,
cmetcalf, ak, ink, anton, tglx, kamezawa.hiroyu, rth, tony.luck,
torvalds, linux-kernel, ralf, lethal, linux-alpha, bob.picco,
akpm, linuxppc-dev, davem
In-Reply-To: <1338973295.2749.81.camel@twins>
On 06/06/2012 05:01 PM, Peter Zijlstra wrote:
> On Wed, 2012-06-06 at 14:52 +0800, Alex Shi wrote:
>> - if (sched_domains_numa_distance[level] > REMOTE_DISTANCE)
>> + if (sched_domains_numa_distance[level] > RECLAIM_DISTANCE)
>
> I actually considered this.. I just felt a little uneasy re-purposing
> the RECLAIM_DISTANCE for this, but I guess its all the same anyway. Both
> mean expensive-away-distance.
>
I understand you, the BIOS guys don't have a good alignment with us on
this.
> So I've taken this.
>
> thanks!
^ permalink raw reply
* Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
From: Scott Wood @ 2012-06-07 0:37 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Ben Collins
In-Reply-To: <1339021310.7150.181.camel@pasglop>
On 06/06/2012 05:21 PM, Benjamin Herrenschmidt wrote:
> Now that means that we can end up with funky arithmetic in a couple of
> cases:
>
> - If the bus address of the IO space is larger than the virtual address
> returned by ioremap (it's a bit silly to use large IO addresses but it's
> technically possible, normally IO windows start at 0 bus-side though).
> In fact I wouldn't be surprised if we have various other bugs if IO
> windows don't start at 0 (you may want to double check your dts setup
> here).
The dts does show the I/O beginning at bus address zero:
ranges = <0x2000000 0x0 0xc0000000 0xc0000000 0x0
0x20000000
0x1000000 0x0 0x0 0xe1000000 0x0 0x10000>;
> - If the ioremap'ed address of the IO space of another domain is lower
> than the ioremap'ed address of the first domain, in which case the
> calculation:
>
> host->io_base_virt - _IO_BASE
>
> results in a negative offset.
There should have been only one PCI domain in the QEMU case.
-Scott
^ permalink raw reply
* [PATCH] mpc85xx_edac: fix error: too few arguments to function 'edac_mc_alloc'
From: Kim Phillips @ 2012-06-07 0:49 UTC (permalink / raw)
To: linux-edac; +Cc: linuxppc-dev, Mauro Carvalho Chehab
commit ca0907b "edac: Remove the legacy EDAC ABI" broke mpc85xx_edac
in the following manner:
mpc85xx_edac.c:983:35: error: too few arguments to function 'edac_mc_alloc'
this patch puts back the missing 'layers' argument.
Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
Signed-off-by: Kim Phillips <kim.phillips@freescale.com>
---
drivers/edac/mpc85xx_edac.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
index 4c40235..0e37462 100644
--- a/drivers/edac/mpc85xx_edac.c
+++ b/drivers/edac/mpc85xx_edac.c
@@ -980,7 +980,8 @@ static int __devinit mpc85xx_mc_err_probe(struct platform_device *op)
layers[1].type = EDAC_MC_LAYER_CHANNEL;
layers[1].size = 1;
layers[1].is_virt_csrow = false;
- mci = edac_mc_alloc(edac_mc_idx, ARRAY_SIZE(layers), sizeof(*pdata));
+ mci = edac_mc_alloc(edac_mc_idx, ARRAY_SIZE(layers), layers,
+ sizeof(*pdata));
if (!mci) {
devres_release_group(&op->dev, mpc85xx_mc_err_probe);
return -ENOMEM;
--
1.7.10.2
^ permalink raw reply related
* Re: [PATCH] powerpc: Optimise the 64bit optimised __clear_user
From: Paul Mackerras @ 2012-06-07 3:04 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: mikey, michael, Anton Blanchard, olof, linuxppc-dev
In-Reply-To: <178E3BC0-C6E2-4E33-BA66-8144F192A151@kernel.crashing.org>
On Wed, Jun 06, 2012 at 06:40:54PM +0200, Segher Boessenkool wrote:
> >+err1; dcbz r0,r3
>
> There is no such instruction, you probably meant "dcbz 0,r3"?
There certainly is such an instruction, though it doesn't do exactly
what a naive reader might expect. Using 0 rather than r0 or %r0
improves readability but makes no difference to the assembler or the
cpu.
Paul.
^ permalink raw reply
* Re: [PATCH v5 1/5] powerpc/85xx: implement hardware timebase sync
From: Zhao Chenhui @ 2012-06-07 4:07 UTC (permalink / raw)
To: Scott Wood; +Cc: Matthew McClintock, linuxppc-dev, linux-kernel
In-Reply-To: <4FCFA0C8.9090800@freescale.com>
On Wed, Jun 06, 2012 at 01:26:16PM -0500, Scott Wood wrote:
> On 06/06/2012 04:31 AM, Zhao Chenhui wrote:
> > On Tue, Jun 05, 2012 at 11:07:41AM -0500, Scott Wood wrote:
> >> On 06/05/2012 04:08 AM, Zhao Chenhui wrote:
> >>> On Fri, Jun 01, 2012 at 10:40:00AM -0500, Scott Wood wrote:
> >>>> I know you say this is for dual-core chips only, but it would be nice if
> >>>> you'd write this in a way that doesn't assume that (even if the
> >>>> corenet-specific timebase freezing comes later).
> >>>
> >>> At this point, I have not thought about how to implement the cornet-specific timebase freezing.
> >>
> >> I wasn't asking you to. I was asking you to not have logic that breaks
> >> with more than 2 CPUs.
> >
> > These routines only called in the dual-core case.
>
> Come on, you know we have chips with more than two cores. Why design
> such a limitation into it, just because you're not personally interested
> in supporting anything but e500v2?
>
> Is it so hard to make it work for an arbitrary number of cores?
>
> >>> If do not set them, it may make KEXEC fail on other platforms.
> >>
> >> What platforms?
> >
> > Such as P4080, P3041, etc.
>
> So we need to wait for corenet timebase sync before we stop causing
> problems in virtualization, simulators, etc. if a kernel has kexec or
> cpu hotplug enabled (whether used or not)?
>
> Can you at least make sure we're actually in a kexec/hotplug scenario at
> runtime?
>
> Or just implement corenet timebase sync -- it's not that different.
>
> -Scott
We also work on the corenet timebase sync. Our plan is first the dual-core case,
then the case of more than 2 cores. We will submit the corenet timebase sync patch soon.
-Chenhui
^ permalink raw reply
* Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source
From: Li Yang @ 2012-06-07 4:10 UTC (permalink / raw)
To: Scott Wood
Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org, Li Yang-R58472,
linux-kernel@vger.kernel.org, Zhao Chenhui-B35336
In-Reply-To: <4FCFA17F.3050207@freescale.com>
On Thu, Jun 7, 2012 at 2:29 AM, Scott Wood <scottwood@freescale.com> wrote:
> On 06/05/2012 11:06 PM, Li Yang wrote:
>> On Wed, Jun 6, 2012 at 2:05 AM, Scott Wood <scottwood@freescale.com> wro=
te:
>>> You ignored "what about devices other than ethernet".
>>
>> No, I haven't. =C2=A0Other devices are so at least for now.
>
> I don't understand that last sentence. =C2=A0Other devices are what?
Probably I misunderstood your question "what about devices other than
ethernet". Did you mean how would other devices other than ethernet
know how to set it?
Other wakeup capable devices can call the API when it is up and
running. It will be the pmc driver's responsibility to find out if
that specific device can be configured as a wakeup source for the SoC.
Leo
^ permalink raw reply
* Re: [PATCH] powerpc: Optimise the 64bit optimised __clear_user
From: Michael Neuling @ 2012-06-07 6:05 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: michael, paulus, Anton Blanchard, olof, linuxppc-dev
In-Reply-To: <1339017602.7150.174.camel@pasglop>
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Wed, 2012-06-06 at 18:40 +0200, Segher Boessenkool wrote:
> > > +err1; dcbz r0,r3
> >
> > There is no such instruction, you probably meant "dcbz 0,r3"?
>
> This reminds me... what would happen if we changed all our
>
> #define r0 0
> #define r1 1
>
> etc... to:
>
> #define r0 %r0
> #define r1 %r1
>
> ?
>
> I'm thinking it might help catch that sort of nasties (and some of them
> can be really nasty, such as inverting mfspr/mtspr arguments, or vs ori,
> etc... ). I'm sure we'd have a problem with a few macros & inline
> constructs but nothing we can't fix..
One problem with this is when we construct the instructions, like using
anything from ppc-opcode.h. eg. using PPC_POPCNTB would need to go from:
PPC_POPCNTB(r3,r3)
to:
PPC_POPCNTB(3,3)
Which is less readable IMHO.
Mikey
^ permalink raw reply
* Re: [PATCH] powerpc: Optimise the 64bit optimised __clear_user
From: Michael Ellerman @ 2012-06-07 6:07 UTC (permalink / raw)
To: Michael Neuling; +Cc: paulus, Anton Blanchard, olof, linuxppc-dev
In-Reply-To: <4725.1339049104@neuling.org>
On Thu, 2012-06-07 at 16:05 +1000, Michael Neuling wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> > On Wed, 2012-06-06 at 18:40 +0200, Segher Boessenkool wrote:
> > > > +err1; dcbz r0,r3
> > >
> > > There is no such instruction, you probably meant "dcbz 0,r3"?
> >
> > This reminds me... what would happen if we changed all our
> >
> > #define r0 0
> > #define r1 1
> >
> > etc... to:
> >
> > #define r0 %r0
> > #define r1 %r1
> >
> > ?
> >
> > I'm thinking it might help catch that sort of nasties (and some of them
> > can be really nasty, such as inverting mfspr/mtspr arguments, or vs ori,
> > etc... ). I'm sure we'd have a problem with a few macros & inline
> > constructs but nothing we can't fix..
>
> One problem with this is when we construct the instructions, like using
> anything from ppc-opcode.h. eg. using PPC_POPCNTB would need to go from:
> PPC_POPCNTB(r3,r3)
> to:
> PPC_POPCNTB(3,3)
#define R(x) x
#define PPC_POPCNTB(R(3), R(3))
??
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: Optimise the 64bit optimised __clear_user
From: Michael Neuling @ 2012-06-07 6:12 UTC (permalink / raw)
To: Michael Ellerman; +Cc: paulus, Anton Blanchard, olof, linuxppc-dev
In-Reply-To: <1339049235.2215.2.camel@concordia>
Michael Ellerman <michael@ellerman.id.au> wrote:
> On Thu, 2012-06-07 at 16:05 +1000, Michael Neuling wrote:
> > Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> >
> > > On Wed, 2012-06-06 at 18:40 +0200, Segher Boessenkool wrote:
> > > > > +err1; dcbz r0,r3
> > > >
> > > > There is no such instruction, you probably meant "dcbz 0,r3"?
> > >
> > > This reminds me... what would happen if we changed all our
> > >
> > > #define r0 0
> > > #define r1 1
> > >
> > > etc... to:
> > >
> > > #define r0 %r0
> > > #define r1 %r1
> > >
> > > ?
> > >
> > > I'm thinking it might help catch that sort of nasties (and some of them
> > > can be really nasty, such as inverting mfspr/mtspr arguments, or vs ori,
> > > etc... ). I'm sure we'd have a problem with a few macros & inline
> > > constructs but nothing we can't fix..
> >
> > One problem with this is when we construct the instructions, like using
> > anything from ppc-opcode.h. eg. using PPC_POPCNTB would need to go from:
> > PPC_POPCNTB(r3,r3)
> > to:
> > PPC_POPCNTB(3,3)
>
> #define R(x) x
#define R(x) (x)
> #define PPC_POPCNTB(R(3), R(3))
Maybe, looks pretty gross but you're the maintainer! :-)
Mikey
^ permalink raw reply
* Re: [PATCH] powerpc: Optimise the 64bit optimised __clear_user
From: Michael Ellerman @ 2012-06-07 6:18 UTC (permalink / raw)
To: Michael Neuling; +Cc: paulus, Anton Blanchard, olof, linuxppc-dev
In-Reply-To: <5626.1339049529@neuling.org>
On Thu, 2012-06-07 at 16:12 +1000, Michael Neuling wrote:
> Michael Ellerman <michael@ellerman.id.au> wrote:
>
> > On Thu, 2012-06-07 at 16:05 +1000, Michael Neuling wrote:
> > > Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> > >
> > > > On Wed, 2012-06-06 at 18:40 +0200, Segher Boessenkool wrote:
> > > > > > +err1; dcbz r0,r3
> > > > >
> > > > > There is no such instruction, you probably meant "dcbz 0,r3"?
> > > >
> > > > This reminds me... what would happen if we changed all our
> > > >
> > > > #define r0 0
> > > > #define r1 1
> > > >
> > > > etc... to:
> > > >
> > > > #define r0 %r0
> > > > #define r1 %r1
> > > >
> > > > ?
> > > >
> > > > I'm thinking it might help catch that sort of nasties (and some of them
> > > > can be really nasty, such as inverting mfspr/mtspr arguments, or vs ori,
> > > > etc... ). I'm sure we'd have a problem with a few macros & inline
> > > > constructs but nothing we can't fix..
> > >
> > > One problem with this is when we construct the instructions, like using
> > > anything from ppc-opcode.h. eg. using PPC_POPCNTB would need to go from:
> > > PPC_POPCNTB(r3,r3)
> > > to:
> > > PPC_POPCNTB(3,3)
> >
> > #define R(x) x
>
> #define R(x) (x)
>
> > #define PPC_POPCNTB(R(3), R(3))
>
> Maybe, looks pretty gross but you're the maintainer! :-)
No I am not!
I agree it's fairly gross. But I'll take gross and correct over ungross
and buggy.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: Optimise the 64bit optimised __clear_user
From: Benjamin Herrenschmidt @ 2012-06-07 6:39 UTC (permalink / raw)
To: Michael Neuling; +Cc: michael, paulus, Anton Blanchard, olof, linuxppc-dev
In-Reply-To: <4725.1339049104@neuling.org>
On Thu, 2012-06-07 at 16:05 +1000, Michael Neuling wrote:
> One problem with this is when we construct the instructions, like using
> anything from ppc-opcode.h. eg. using PPC_POPCNTB would need to go from:
> PPC_POPCNTB(r3,r3)
> to:
> PPC_POPCNTB(3,3)
> Which is less readable IMHO.
Yes, I know. Not much to do about this, but it might still be worth it,
how much time wasted due to mixing up or with ori in asm somewhere ?
One option would be to #define R3 (or _r3) for use in those macros so
we still have something nicer than just "3"... oh well.
Cheers,
Ben.
^ permalink raw reply
* [v2][PATCH 1/1] ppc64: fix missing to check all bits of _TIF_USER_WORK_MASK in preempt
From: Tiejun Chen @ 2012-06-07 6:56 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev
In entry_64.S version of ret_from_except_lite, you'll notice that
in the !preempt case, after we've checked MSR_PR we test for any
TIF flag in _TIF_USER_WORK_MASK to decide whether to go to do_work
or not. However, in the preempt case, we do a convoluted trick to
test SIGPENDING only if PR was set and always test NEED_RESCHED ...
but we forget to test any other bit of _TIF_USER_WORK_MASK !!! So
that means that with preempt, we completely fail to test for things
like single step, syscall tracing, etc...
This should be fixed as the following path:
- Test PR. If not set, go to resume_kernel, else continue.
- If go resume_kernel, to do that original do_work.
- If else, then always test for _TIF_USER_WORK_MASK to decide to do
that original user_work, else restore directly.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
v2:
* reorganize the original do_work/user_work
arch/powerpc/kernel/entry_64.S | 97 ++++++++++++++++-----------------------
1 files changed, 40 insertions(+), 57 deletions(-)
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index ed1718f..5971c85 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -558,27 +558,54 @@ _GLOBAL(ret_from_except_lite)
mtmsrd r10,1 /* Update machine state */
#endif /* CONFIG_PPC_BOOK3E */
-#ifdef CONFIG_PREEMPT
clrrdi r9,r1,THREAD_SHIFT /* current_thread_info() */
- li r0,_TIF_NEED_RESCHED /* bits to check */
ld r3,_MSR(r1)
ld r4,TI_FLAGS(r9)
- /* Move MSR_PR bit in r3 to _TIF_SIGPENDING position in r0 */
- rlwimi r0,r3,32+TIF_SIGPENDING-MSR_PR_LG,_TIF_SIGPENDING
- and. r0,r4,r0 /* check NEED_RESCHED and maybe SIGPENDING */
- bne do_work
-
-#else /* !CONFIG_PREEMPT */
- ld r3,_MSR(r1) /* Returning to user mode? */
andi. r3,r3,MSR_PR
- beq restore /* if not, just restore regs and return */
+ beq resume_kernel
/* Check current_thread_info()->flags */
+ andi. r0,r4,_TIF_USER_WORK_MASK
+ beq restore
+
+ andi. r0,r4,_TIF_NEED_RESCHED
+ beq 1f
+ bl .restore_interrupts
+ bl .schedule
+ b .ret_from_except_lite
+
+1: bl .save_nvgprs
+ bl .restore_interrupts
+ addi r3,r1,STACK_FRAME_OVERHEAD
+ bl .do_notify_resume
+ b .ret_from_except
+
+resume_kernel:
+#ifdef CONFIG_PREEMPT
+ /* Check if we need to preempt */
+ andi. r0,r4,_TIF_NEED_RESCHED
+ beq+ restore
+ /* Check that preempt_count() == 0 and interrupts are enabled */
+ lwz r8,TI_PREEMPT(r9)
+ cmpwi cr1,r8,0
+ ld r0,SOFTE(r1)
+ cmpdi r0,0
+ crandc eq,cr1*4+eq,eq
+ bne restore
+
+ /*
+ * Here we are preempting the current task. We want to make
+ * sure we are soft-disabled first
+ */
+ SOFT_DISABLE_INTS(r3,r4)
+1: bl .preempt_schedule_irq
+
+ /* Re-test flags and eventually loop */
clrrdi r9,r1,THREAD_SHIFT
ld r4,TI_FLAGS(r9)
- andi. r0,r4,_TIF_USER_WORK_MASK
- bne do_work
-#endif /* !CONFIG_PREEMPT */
+ andi. r0,r4,_TIF_NEED_RESCHED
+ bne 1b
+#endif /* CONFIG_PREEMPT */
.globl fast_exc_return_irq
fast_exc_return_irq:
@@ -759,50 +786,6 @@ restore_check_irq_replay:
#endif /* CONFIG_PPC_BOOK3E */
1: b .ret_from_except /* What else to do here ? */
-
-
-3:
-do_work:
-#ifdef CONFIG_PREEMPT
- andi. r0,r3,MSR_PR /* Returning to user mode? */
- bne user_work
- /* Check that preempt_count() == 0 and interrupts are enabled */
- lwz r8,TI_PREEMPT(r9)
- cmpwi cr1,r8,0
- ld r0,SOFTE(r1)
- cmpdi r0,0
- crandc eq,cr1*4+eq,eq
- bne restore
-
- /*
- * Here we are preempting the current task. We want to make
- * sure we are soft-disabled first
- */
- SOFT_DISABLE_INTS(r3,r4)
-1: bl .preempt_schedule_irq
-
- /* Re-test flags and eventually loop */
- clrrdi r9,r1,THREAD_SHIFT
- ld r4,TI_FLAGS(r9)
- andi. r0,r4,_TIF_NEED_RESCHED
- bne 1b
- b restore
-
-user_work:
-#endif /* CONFIG_PREEMPT */
-
- andi. r0,r4,_TIF_NEED_RESCHED
- beq 1f
- bl .restore_interrupts
- bl .schedule
- b .ret_from_except_lite
-
-1: bl .save_nvgprs
- bl .restore_interrupts
- addi r3,r1,STACK_FRAME_OVERHEAD
- bl .do_notify_resume
- b .ret_from_except
-
unrecov_restore:
addi r3,r1,STACK_FRAME_OVERHEAD
bl .unrecoverable_exception
--
1.5.6
^ permalink raw reply related
* RE: [PATCH] powerpc: Optimise the 64bit optimised __clear_user
From: David Laight @ 2012-06-07 8:41 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Michael Neuling
Cc: michael, olof, linuxppc-dev, paulus, Anton Blanchard
In-Reply-To: <1339051173.24838.10.camel@pasglop>
=20
> -----Original Message-----
> From: Linuxppc-dev=20
> [mailto:linuxppc-dev-bounces+david.laight=3Daculab.com@lists.ozl
abs.org] On Behalf Of Benjamin Herrenschmidt
> Sent: 07 June 2012 07:40
> To: Michael Neuling
> Cc: michael@ellerman.id.au; paulus@samba.org; Anton=20
> Blanchard; olof@lixom.net; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH] powerpc: Optimise the 64bit optimised=20
> __clear_user
>=20
> On Thu, 2012-06-07 at 16:05 +1000, Michael Neuling wrote:
>=20
> > One problem with this is when we construct the=20
> instructions, like using
> > anything from ppc-opcode.h. eg. using PPC_POPCNTB would=20
> need to go from:
> > PPC_POPCNTB(r3,r3)=20
> > to:
> > PPC_POPCNTB(3,3)=20
> > Which is less readable IMHO.
>=20
> Yes, I know. Not much to do about this, but it might still be=20
> worth it,
> how much time wasted due to mixing up or with ori in asm somewhere ?
>=20
> One option would be to #define R3 (or _r3) for use in those macros so
> we still have something nicer than just "3"... oh well.
You then need to catch all the other bugs where a register
is being used instead of a constant.
Time to make the assembler require %r0 (etc)
David
^ permalink raw reply
* Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
From: Benjamin Herrenschmidt @ 2012-06-07 9:30 UTC (permalink / raw)
To: Ben Collins; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <16AB27C3-8709-4A7C-AFFE-42A0B658AC1D@servergy.com>
On Wed, 2012-06-06 at 19:35 -0400, Ben Collins wrote:
>
> pci_bus 0000:00: root bus resource [io 0xffbed000-0xffbfcfff] (bus
> address [0x100000000-0x10000ffff])
>
> Without the fix that I sent, it ends up looking like:
>
> pci_bus 0000:00: root bus resource [io 0xffbed000-0xffbfcfff] (bus
> address [0x0000-0xffff])
>
> And that's when some devices fail to be assigned valid bar 0's and the
> kernel complains because of it.
Note that oddly, the second range of bus addresses looks -more- correct
than the first one...
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH v2 0/2] Add pcibios_device_change_notifier
From: Hiroo Matsumoto @ 2012-06-07 12:45 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Bjorn Helgaas, linux-pci, linuxppc-dev, jbarnes, Kenji Kaneshige
In-Reply-To: <1338960416.7150.160.camel@pasglop>
I apologize for my late reply.
> On Wed, 2012-05-23 at 11:33 +0900, Hiroo Matsumoto wrote:
>> This patchset is for PCI hotplug.
>>
>>
>> pcibios_setup_bus_devices which sets DMA and IRQs of PCI device is called
>> only when boot. DMA setting in probe for PCI driver, like dma_set_mask,
>> does not work on powerpc platform. So it is need to set DMA and IRQs of
>> PCI device when hotplug.
>>
>> 1. Moving pcibios_setup_bus_devices code to pcibios_device_change_notifier
>> which is registered to bus notifier in pcibios_init.
>> 2. Removing caller and callee of pcibios_setup_bus_devices bus notifier
>> works instead of pcibios_setup_bus_devices.
>> 3. Using this bus notifier for microblaze because microblaze/PCI is similer
>> with powerpc/PCI.
>
> This makes me a bit nervous (that doesn't mean it's not right, but
> we need some careful auditing & testing here, which I won't be
> able to do until I'm back from leave). Mostly due to the change in when
> we do the work.
>
> pcibios_fixup_bus() used to be called early on in the initial scan pass.
>
> Your code causes the code to be called -much- later when registering the
> device with the device model. Are we 100% certain nothing will happen in
> between that might rely on the stuff being setup already ? It might well
> be ok, but I want us to triple check that.
>
> Now, if we are ok to do the setup that late (basically right before the
> driver probe() routine gets called), would it make sense to simplify
> things even further ... and do it from pcibios_enable_device() ? Thus
> avoiding the notifier business completely or is that pushing it too
> far ?
>
As you said, there are times between pcibios_fixup_bus and device_add.
I'm agree with you.
But it is difficult for me to verify these by myself.
Can I ask for your help?
I will wait that you are back from leave.
> Also you seem to add:
>
> + /* Setup OF node pointer in the device */
> + dev->dev.of_node = pci_device_to_OF_node(dev);
>
> This shouldn't be needed anymore, the device node should be setup by the
> core nowadays. Is this just a remnant of you rebasing an old patch or do
> you have a good reason to add this statement ?
No, I don't have a good reason to add this statement.
I just tried to make this code be same with pcibios_setup_bus_devices.
Thanks for your review and comment.
>
> Cheers,
> Ben.
>
>
>
>
Regards.
Hiroo MATSUMOTO
^ 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