* RE: eSDHC controller driver on MPC8308rdb
From: Maria Johansen @ 2010-10-19 7:26 UTC (permalink / raw)
To: Bo.Liu; +Cc: linuxppc-dev
In-Reply-To: <4CBC04E8.5030706@windriver.com>
DQoNCj4tLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPkZyb206IFRvbnlsaXUgW21haWx0bzpC
by5MaXVAd2luZHJpdmVyLmNvbV0gDQo+PHNuaXA+DQo+PiBDb3VsZCB0aGlzIGJlIGEgcHJvYmxl
bSByZWxhdGVkIHRoZSBlU0RIQyBjb250cm9sbGVyIChvciB0aGUgZHJpdmVyKSwgDQo+PiBvciBp
cyBpdCB0aGUgbWVtb3J5IGNhcmQ/IChhIDRHQiBTYW5EaXNrIEV4dHJlbWUgU0RIQyBjYXJkLCB3
aGljaCB1bmZvcnR1bmF0ZWx5IGlzIHRoZSBvbmx5IGNhcmQgSSBoYXZlIGF2YWlsYWJsZSBhdCB0
aGUgbW9tZW50LikgSSB3aWxsIGtlZXAgZGlnZ2luZyBpbnRvIGRyaXZlcnMvbW1jL2hvc3Qvc2Ro
Y2kuYyBpbiBzZWFyY2ggb2YgYSBzb2x1dGlvbiwgYW5kIGFueSB0aXBzIHRvIGhvdyBJIHNob3Vs
ZCBwcm9jZWVkIHdvdWxkIGJlIGdyZWF0bHkgYXBwcmVjaWF0ZWQhDQo+PiAgIA0KPkhpLA0KPlRy
eSB0aGUgcGF0Y2ggaW4gYXR0YXRjaG1lbnQuIEJ5IGRlZmF1bHQsIG9uIHNvbWUgZTMwMCBwbGF0
Zm9ybXMgc3VjaCBhcyBtcGM4MzA4X3JkYiwgdGhlIGVudHJ5ICJjbG9jay1mcmVxdWVuY3kiIG9m
IHNlY3Rpb24gc2RjaGkgaW4gRFRCIGlzIG5vdCBmaXhlZCBieSB1LWJvb3QsIHNvIGhhcyB0byA+
Y2FjdWxhdGUgdGhlIGlucHV0IGNsb2NrIGZvciBzZGhjaSBjb250cm9sbGVyIGV4cGxpY2l0bHkg
aW4gZHJpdmVyLg0KPg0KPlRvbnkNCg0KVGhhbmtzLCB0aGUgbWlzc2luZyBjbG9jay1mcmVxdWVu
Y3kgd2FzIHBhcnQgb2YgbXkgcHJvYmxlbS4gVGhlIHJlc3Qgd2FzIHNvbHZlZCBieSBhZGRpbmcg
c29tZSBtb3JlIHF1aXJrcyB0byB0aGUgZXNkaGMtZHJpdmVyIChTREhDSV9RVUlSS19GT1JDRV8x
X0JJVF9EQVRBIGFuZCBTREhDSV9RVUlSS19SRVNFVF9BRlRFUl9SRVFVRVNUKS4gV2h5IGl0IGlz
IG5lY2Vzc2FyeSB0byBmb3JjZSAxLWJpdCBkYXRhIHRyYW5zZmVycyBJIGFtIGEgYml0IHVuc3Vy
ZSBhYm91dCwgc2luY2UgdGhlIG1wYzgzMDggcmVmZXJlbmNlIG1hbnVhbCBjbGVhcmx5IHN0YXRl
cyB0aGF0IDQtYml0IHRyYW5zZmVycyBhcmUgc3VwcG9ydGVkLg0KDQpUaGFua3MgZm9yIHlvdXIg
aGVscCA6KQ0KDQotLQ0KTWFyaWENCg0KDQoNCg0K
^ permalink raw reply
* RE: [PATCH 1/2] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices
From: Zang Roy-R61911 @ 2010-10-19 6:37 UTC (permalink / raw)
To: Wood Scott-B07421, tiejun.chen
Cc: dedekind1, Lan Chunhe-B25806, linuxppc-dev, linux-mtd, akpm,
dwmw2, Gala Kumar-B11780
In-Reply-To: <20101018110631.22d6f081@udp111988uds.am.freescale.net>
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, October 19, 2010 0:07 AM
> To: tiejun.chen
> Cc: Zang Roy-R61911; linux-mtd@lists.infradead.org; Wood Scott-B07421;
> dedekind1@gmail.com; Lan Chunhe-B25806; linuxppc-dev@ozlabs.org;
akpm@linux-
> foundation.org; dwmw2@infradead.org; Gala Kumar-B11780
> Subject: Re: [PATCH 1/2] P4080/eLBC: Make Freescale elbc interrupt
common to
> elbc devices
>=20
> On Mon, 18 Oct 2010 16:55:49 +0800
> "tiejun.chen" <tiejun.chen@windriver.com> wrote:
>=20
> > Looks you always iounmap(fsl_lbc_ctrl_dev->regs) on position 'err'
but here
> > of_iomap() is already failed you should skip iounmap()
fsl_lbc_ctrl_dev-
> >regs
> > again. So you should improve that as the following on 'err', or
layout 'err'
> in
> > gain.
> > ------
> > if(fsl_lbc_ctrl_dev->regs)
> > iounmap(fsl_lbc_ctrl_dev->regs);
> >
>=20
> It looks like iounmap(NULL) is a no-op, just like kfree(NULL).
Thanks for the reminder. I will keep original post, if there is no more
comment.
Roy
^ permalink raw reply
* Re: linux-next: manual merge of the tip tree with the powerpc tree
From: Stephen Rothwell @ 2010-10-19 4:56 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Peter Zijlstra
Cc: linux-next, Paul Mackerras, linux-kernel, linuxppc-dev
In-Reply-To: <20101019154849.126a736a.sfr@canb.auug.org.au>
Hi again,
On Tue, 19 Oct 2010 15:48:49 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next merge of the tip tree got a conflict in
> arch/powerpc/kernel/time.c between commit
> cf9efce0ce3136fa076f53e53154e98455229514 ("powerpc: Account time using
> timebase rather than PURR") from the powerpc tree and commit
> e360adbe29241a0194e10e20595360dd7b98a2b3 ("irq_work: Add generic hardirq
> context callbacks") from the tip tree.
>
> Just context changes. I fixed it up and can carry the fix as necessary.
I forgot the merge diff, sorry:
diff --cc arch/powerpc/kernel/time.c
index 644f918,54888eb..0000000
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@@ -578,9 -585,11 +578,9 @@@ void timer_interrupt(struct pt_regs * r
old_regs = set_irq_regs(regs);
irq_enter();
- if (test_perf_event_pending()) {
- clear_perf_event_pending();
- perf_event_do_pending();
- calculate_steal_time();
-
+ if (test_irq_work_pending()) {
+ clear_irq_work_pending();
+ irq_work_run();
}
#ifdef CONFIG_PPC_ISERIES
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
^ permalink raw reply
* linux-next: manual merge of the tip tree with the powerpc tree
From: Stephen Rothwell @ 2010-10-19 4:48 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Peter Zijlstra
Cc: linux-next, Paul Mackerras, linux-kernel, linuxppc-dev
Hi all,
Today's linux-next merge of the tip tree got a conflict in
arch/powerpc/kernel/time.c between commit
cf9efce0ce3136fa076f53e53154e98455229514 ("powerpc: Account time using
timebase rather than PURR") from the powerpc tree and commit
e360adbe29241a0194e10e20595360dd7b98a2b3 ("irq_work: Add generic hardirq
context callbacks") from the tip tree.
Just context changes. I fixed it up and can carry the fix as necessary.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
^ permalink raw reply
* Re: CONFIG_FEC is not good for mpc8xx ethernet?
From: tiejun.chen @ 2010-10-19 2:03 UTC (permalink / raw)
To: Scott Wood; +Cc: ppcdev
In-Reply-To: <20101018114214.2e72a616@udp111988uds.am.freescale.net>
Scott Wood wrote:
> On Mon, 18 Oct 2010 16:40:42 +0800
> "tiejun.chen" <tiejun.chen@windriver.com> wrote:
>
>> Shawn Jin wrote:
>>> Hi,
>>>
>>> My target is a mpc875 based board and has FEC ethernet. The phy is
>>> AM79C874. I have the following configuration for the network support.
>>>
>>> CONFIG_PHYLIB=y
>>> CONFIG_NET_ETHERNET=y
>>> CONFIG_MII=y
>>> CONFIG_FS_ENET=y
>>> CONFIG_FS_ENET_HAS_FEC=y
>>> CONFIG_FS_ENET_MDIO_FEC=y
>>>
>>> However I found that the phy support (AM79C874) is actually in
>>> drivers/net/fec.c which is compiled only when CONFIG_FEC=y. However
>> The phy driver should not be embedded into the NIC driver in theory.
>
> Right, those are handled by drivers/net/phy/.
>
>> I think you should include the phy driver, mdio-bitbang.c, which should be
>> support AMD79C874.
>
> On MPC8xx you want drivers/net/fs_enet/mii-fec.c. This is just the
> MDIO driver; it doesn't handle any particular PHY. I don't know if
> there is a driver specifically for AM79C874, though the generic PHY
> support may be good enough.
Maybe.
I can found one related patch for supporting PHY AM79C874 on 2.6.15,
------
http://lists.ozlabs.org/pipermail/linuxppc-embedded/2005-November/021043.html
But I don't see that on the latest kernel, and also I don't know the history
completely for that. Maybe its already merged into one generic PHY driver but
I'm not sure.
Tiejun
>
> -Scott
>
>
^ permalink raw reply
* Re: [PATCH 1/2] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices
From: tiejun.chen @ 2010-10-19 1:35 UTC (permalink / raw)
To: Scott Wood
Cc: B07421, dedekind1, B25806, linuxppc-dev, linux-mtd, akpm, dwmw2,
B11780
In-Reply-To: <20101018110631.22d6f081@udp111988uds.am.freescale.net>
Scott Wood wrote:
> On Mon, 18 Oct 2010 16:55:49 +0800
> "tiejun.chen" <tiejun.chen@windriver.com> wrote:
>
>> Looks you always iounmap(fsl_lbc_ctrl_dev->regs) on position 'err' but here
>> of_iomap() is already failed you should skip iounmap() fsl_lbc_ctrl_dev->regs
>> again. So you should improve that as the following on 'err', or layout 'err' in
>> gain.
>> ------
>> if(fsl_lbc_ctrl_dev->regs)
>> iounmap(fsl_lbc_ctrl_dev->regs);
>>
>
> It looks like iounmap(NULL) is a no-op, just like kfree(NULL).
>
Absolutely, I know what you mean :)
But I think we should take care of every line as one normal progress/rule. That
will make us understand easily, and maybe we also would benefit a little good
performance from those codes.
Tiejun
> -Scott
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
^ permalink raw reply
* Re: BUG: dead loop in PowerPC hcall tracepoint (Was: [LTP] [PATCH v2] Add ftrace-stress-test to LTP)
From: Li Zefan @ 2010-10-19 0:49 UTC (permalink / raw)
To: Steven Rostedt
Cc: ltp-list, Peter Zijlstra, linuxppc-dev, LKML, Paul Mackerras,
Anton Blanchard, Ingo Molnar, subrata
In-Reply-To: <1287411903.16971.275.camel@gandalf.stny.rr.com>
Steven Rostedt wrote:
> On Mon, 2010-10-18 at 11:19 +0800, Li Zefan wrote:
>
>> This is a dead loop:
>>
>> trace_hcall_entry() -> trace_clock_global() -> trace_hcall_entry() ..
>>
>> And this is a PPC specific bug. Hope some ppc guys will fix it?
>> Or we kill trace_clock_global() if no one actually uses it..
>
> trace_clock_global() is used by many. I use it (and recommend using it)
> on boxes where the TSC is horribly out of sync, and the trace needs
> synchronization between CPUs.
>
> The trace_hcall_entry and exit has wrappers already. Just add recursion
> protection there.
>
Right, I thought of this. But as I have no machine to test, I'll leave
this to others.
> Perhaps something like this:
>
> (Not compiled nor ran)
>
> +static DEFINE_PER_CPU(hcall_trace_disable);
> +
> void hcall_tracepoint_regfunc(void)
> {
> hcall_tracepoint_refcount++;
> }
>
> void hcall_tracepoint_unregfunc(void)
> {
> hcall_tracepoint_refcount--;
> }
>
> +int __trace_disable_check(void)
> +{
> + if (!hcall_tracepoint_refcount)
> + return 1;
> +
> + if (get_cpu_var(hcall_trace_disable)) {
> + put_cpu_var(hcall_trace_disable);
> + return 1;
> + }
> +
> + __get_cpu_var(hcall_trace_disable)++;
> +
> + return 0;
> +}
> +
> +void __trace_disable_put(void)
> +{
> + __get_cpu_var(hcall_trace_disable)--;
> + put_cpu_var(hcall_trace_disable);
> +}
> +
> void __trace_hcall_entry(unsigned long opcode, unsigned long *args)
> {
> + int trace_disable;
> +
> + if (__trace_disable_check())
> + return;
> +
> trace_hcall_entry(opcode, args);
> + __trace_disable_put();
> }
>
> void __trace_hcall_exit(long opcode, unsigned long retval,
> unsigned long *retbuf)
> {
> + if (__trace_disable_check())
> + return;
> +
> trace_hcall_exit(opcode, retval, retbuf);
> + __trace_disable_put();
> }
>
> -- Steve
>
>
>
>
^ permalink raw reply
* Re: [PATCH 2/2] ppc: lazy flush_tlb_mm for nohash architectures
From: Benjamin Herrenschmidt @ 2010-10-18 23:34 UTC (permalink / raw)
To: Dave Kleikamp; +Cc: linuxppc-dev list
In-Reply-To: <1287439021.3700.22.camel@shaggy-w500>
On Mon, 2010-10-18 at 16:57 -0500, Dave Kleikamp wrote:
>
> I didn't like the implementation of a per-core stale map. The existing
> implementation flushes the core's tlb, but only clears a specific entry
> from the stale map. It's dealing with the stale contexts one at a time,
> where the new function is accumulating many stale contexts, with the
> intent to do a single tlb flush per core.
Right, because I wrote it with A2 in mind which has a TLB invalidate by
PID instruction :-) So I don't flush the whole TLB there... but then
this instruction can take hundreds (or more) of cycles so it might not
necessarily be that great anyways...
> Since I originally intended the new function only to be enabled on the
> 47x, I left the context-stealing code as untouched as possible thinking
> it wouldn't be exercised in 47x-land. This was probably narrow-minded,
> and I should look at either 1) aligning the context-stealing code closer
> to the new lazy flush code, or 2) dropping this code on the floor and
> picking back up the new design that we worked on last year.
In any case, we can probably merge you current stuff upstream (with
appropriate bug fixes if necessary) for now and move from there.
> > At the very least, the "old style" stale map code and "new style" stale
> > TLB code should be more in sync, you may end up flushing the TLB
> > twice...
>
> yeah. if we enable this for 440, it is more likely to be an issue than
> on 476.
Right.
> > > void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
> > > {
> > > unsigned int i, id, cpu = smp_processor_id();
> > > @@ -197,6 +260,8 @@ void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
> > > /* No lockless fast path .. yet */
> > > raw_spin_lock(&context_lock);
> > >
> > > + flush_recycled_contexts(cpu);
> > > +
> >
> > Ok so here's the nasty one I think. You need to make sure that whenever
> > you pick something off the context_available_map, you've done the above
> > first within the same context_lock section right ? At least before you
> > actually -use- said context.
>
> right.
>
> > So far so good ... but steal_context can drop the lock iirc. So you may
> > need to re-flush there. Not sure that can happen in practice but better
> > safe than sorry. I would have preferred seeing that flush near the end
> > of the function to avoid such problem.
>
> I can fix this. For 476, I don't think that even if steal_context()
> could be called, it wouldn't drop the lock. But then again, if we
> enable this for other architectures, it may be a possibility.
Yeah, it's a minor issue but I'd rather get the code right to avoid
surprises later.
.../...
> > Now...
> >
> > > +/*
> > > + * This is called from flush_tlb_mm(). Mark the current context as stale
> > > + * and grab an available one. The tlb will be flushed when no more
> > > + * contexts are available
> > > + */
> > > +void lazy_flush_context(struct mm_struct *mm)
> > > +{
> > > + unsigned int id;
> > > + unsigned long flags;
> > > + unsigned long *map;
> > > +
> > > + raw_spin_lock_irqsave(&context_lock, flags);
> > > +
> > > + id = mm->context.id;
> > > + if (unlikely(id == MMU_NO_CONTEXT))
> > > + goto no_context;
> >
> > First thing is ... you reproduce quite a bit of logic from
> > switch_mmu_context() here. Shouldn't it be abstracted in a separate
> > function ?
>
> I'm sure there's something I can do there.
>
> > The other thing here is that another CPU might have done a
> > recycle_stale_contexts() before you get here. IE. Your TLB may be stale.
> > Shouln't you do a flush here ? Since you are picking up a new PID from
> > the context_available_map, it can potentially be stale if your tlb needs
> > flushing due to another CPU having just done a recycle.
>
> It looks like I missed that. It does seem that there should be a flush
> in here.
Ok, so It wasn't just shit in my eyes :-)
> I think I'm going to dust off the newer implementation based on your and
> Paul's design. I can probably get that in good working order without
> too much more work, and it's something we need to look at eventually
> anyway. If I find anything that really gets in my way, I might fix up
> this patch in the mean time.
As you like. As I said earlier, I'm happy to merge a fixed version of
this one first if you think it's going to take a while to get the other
one right. However, I believe this is too late for the next merge window
anyways so that gives you some time ahead to play and make a decision.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH RFC] PPC: /dev/mem: All RAM is cacheable, not just the kernel's.
From: Kumar Gala @ 2010-10-18 22:41 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20101018223256.GA30946@udp111988uds.am.freescale.net>
On Oct 18, 2010, at 5:32 PM, Scott Wood wrote:
> If mem=3D is used on the kernel command line to create reserved =
regions
> for userspace to map using /dev/mem, let it be mapped cacheable as =
long
> as it is within the memory region described in the device tree.
>=20
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
Just a nit, but subject should be powerpc:.. not PPC:
> This isn't just a performance issue, but it could also be a =
correctness
> issue, if the reserved portion of RAM is mapped cacheable by e.g. a =
KVM
> guest. This patch does not address cases where such regions could =
show up
> as something other than a standard memory node -- such as shared =
regions
> in an AMP configuration. Ideally there would be some means for a =
platform
> to register cacheable regions, without having to completely replace =
the
> entire phys_mem_access_prot function.
>=20
> This patch assumes that there is no region between memstart and memend =
that
> must be non-cacheable. This is already the case with the "for now"
> implementation of page_is_ram on 32-bit, but will this be a problem on
> 64-bit?
>=20
> arch/powerpc/kernel/pci-common.c | 5 ++++-
> arch/powerpc/kernel/prom.c | 3 +++
> arch/powerpc/mm/mem.c | 3 ++-
> arch/powerpc/mm/mmu_decl.h | 1 +
> 4 files changed, 10 insertions(+), 2 deletions(-)
For some time I've been meaning for us to look at the address range =
tracking that x86 does so one can make sure we aren't mapping regions =
with different WIMG settings. However, never enough time for this. So =
I think this is reasonable for now. Hopefully Ben will comment on =
64-bit side of the world.
- k=
^ permalink raw reply
* [PATCH RFC] PPC: /dev/mem: All RAM is cacheable, not just the kernel's.
From: Scott Wood @ 2010-10-18 22:32 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev
If mem= is used on the kernel command line to create reserved regions
for userspace to map using /dev/mem, let it be mapped cacheable as long
as it is within the memory region described in the device tree.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
This isn't just a performance issue, but it could also be a correctness
issue, if the reserved portion of RAM is mapped cacheable by e.g. a KVM
guest. This patch does not address cases where such regions could show up
as something other than a standard memory node -- such as shared regions
in an AMP configuration. Ideally there would be some means for a platform
to register cacheable regions, without having to completely replace the
entire phys_mem_access_prot function.
This patch assumes that there is no region between memstart and memend that
must be non-cacheable. This is already the case with the "for now"
implementation of page_is_ram on 32-bit, but will this be a problem on
64-bit?
arch/powerpc/kernel/pci-common.c | 5 ++++-
arch/powerpc/kernel/prom.c | 3 +++
arch/powerpc/mm/mem.c | 3 ++-
arch/powerpc/mm/mmu_decl.h | 1 +
4 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 10a44e6..4298a56 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -39,6 +39,8 @@
#include <asm/firmware.h>
#include <asm/eeh.h>
+#include <mm/mmu_decl.h>
+
static DEFINE_SPINLOCK(hose_spinlock);
LIST_HEAD(hose_list);
@@ -398,7 +400,8 @@ pgprot_t pci_phys_mem_access_prot(struct file *file,
resource_size_t offset = ((resource_size_t)pfn) << PAGE_SHIFT;
int i;
- if (page_is_ram(pfn))
+ if (pfn >= (memstart_addr >> PAGE_SHIFT) &&
+ pfn <= (memend_addr >> PAGE_SHIFT))
return prot;
prot = pgprot_noncached(prot);
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index fed9bf6..f8f5966 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -62,6 +62,8 @@
#define DBG(fmt...)
#endif
+phys_addr_t memend_addr;
+
#ifdef CONFIG_PPC64
int __initdata iommu_is_off;
int __initdata iommu_force_on;
@@ -504,6 +506,7 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 size)
memblock_add(base, size);
memstart_addr = min((u64)memstart_addr, base);
+ memend_addr = max((u64)memend_addr, base + size - 1);
}
u64 __init early_init_dt_alloc_memory_arch(u64 size, u64 align)
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 1a84a8d..f947a39 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -104,7 +104,8 @@ pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
if (ppc_md.phys_mem_access_prot)
return ppc_md.phys_mem_access_prot(file, pfn, size, vma_prot);
- if (!page_is_ram(pfn))
+ if (pfn < (memstart_addr >> PAGE_SHIFT) ||
+ pfn > (memend_addr >> PAGE_SHIFT))
vma_prot = pgprot_noncached(vma_prot);
return vma_prot;
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index dd0a258..05f1ac6 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -119,6 +119,7 @@ extern phys_addr_t __initial_memory_limit_addr;
extern phys_addr_t total_memory;
extern phys_addr_t total_lowmem;
extern phys_addr_t memstart_addr;
+extern phys_addr_t memend_addr;
extern phys_addr_t lowmem_end_addr;
#ifdef CONFIG_WII
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 2/2] ppc: lazy flush_tlb_mm for nohash architectures
From: Dave Kleikamp @ 2010-10-18 21:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev list
In-Reply-To: <1287017539.2205.48.camel@pasglop>
On Thu, 2010-10-14 at 11:52 +1100, Benjamin Herrenschmidt wrote:
> On Fri, 2010-09-24 at 13:01 -0500, Dave Kleikamp wrote:
> > On PPC_MMU_NOHASH processors that support a large number of contexts,
> > implement a lazy flush_tlb_mm() that switches to a free context, marking
> > the old one stale. The tlb is only flushed when no free contexts are
> > available.
> >
> > The lazy tlb flushing is controlled by the global variable tlb_lazy_flush
> > which is set during init, dependent upon MMU_FTR_TYPE_47x.
>
> Unless I'm mistaken, there are some issues with that patch... sorry for
> the late review, I've been away for a couple of weeks.
>
> > +int tlb_lazy_flush;
> > +static int tlb_needs_flush[NR_CPUS];
> > +static unsigned long *context_available_map;
> > +static unsigned int nr_stale_contexts;
>
> Now I understand what you're doing here, but wouldn't it have been
> possible to re-use the existing stale map concept or do you reckon it
> would have been too messy ?
I didn't like the implementation of a per-core stale map. The existing
implementation flushes the core's tlb, but only clears a specific entry
from the stale map. It's dealing with the stale contexts one at a time,
where the new function is accumulating many stale contexts, with the
intent to do a single tlb flush per core.
Since I originally intended the new function only to be enabled on the
47x, I left the context-stealing code as untouched as possible thinking
it wouldn't be exercised in 47x-land. This was probably narrow-minded,
and I should look at either 1) aligning the context-stealing code closer
to the new lazy flush code, or 2) dropping this code on the floor and
picking back up the new design that we worked on last year.
> At the very least, the "old style" stale map code and "new style" stale
> TLB code should be more in sync, you may end up flushing the TLB
> twice...
yeah. if we enable this for 440, it is more likely to be an issue than
on 476.
> > @@ -189,6 +220,38 @@ static void context_check_map(void)
> > static void context_check_map(void) { }
> > #endif
> >
> > +/*
> > + * On architectures that support a large number of contexts, the tlb
> > + * can be flushed lazily by picking a new context and making the stale
> > + * context unusable until a lazy tlb flush has been issued.
> > + *
> > + * context_available_map keeps track of both active and stale contexts,
> > + * while context_map continues to track only active contexts. When the
> > + * lazy tlb flush is triggered, context_map is copied to
> > + * context_available_map, making the once-stale contexts available again
> > + */
> > +static void recycle_stale_contexts(void)
> > +{
> > + if (nr_free_contexts == 0 && nr_stale_contexts > 0) {
>
> Do an early return and avoid the indentation instead ?
Yeah, that makes sense.
> > + unsigned int cpu = smp_processor_id();
> > + unsigned int i;
> > +
> > + pr_hard("[%d] recycling stale contexts\n", cpu);
> > + /* Time to flush the TLB's */
> > + memcpy(context_available_map, context_map, CTX_MAP_SIZE);
> > + nr_free_contexts = nr_stale_contexts;
> > + nr_stale_contexts = 0;
> > + for_each_online_cpu(i) {
> > + if ((i < cpu_first_thread_in_core(cpu)) ||
> > + (i > cpu_last_thread_in_core(cpu)))
> > + tlb_needs_flush[i] = 1;
> > + else
> > + tlb_needs_flush[i] = 0; /* This core */
> > + }
> > + _tlbil_all();
> > + }
> > +}
> > +
> > void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
> > {
> > unsigned int i, id, cpu = smp_processor_id();
> > @@ -197,6 +260,8 @@ void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
> > /* No lockless fast path .. yet */
> > raw_spin_lock(&context_lock);
> >
> > + flush_recycled_contexts(cpu);
> > +
>
> Ok so here's the nasty one I think. You need to make sure that whenever
> you pick something off the context_available_map, you've done the above
> first within the same context_lock section right ? At least before you
> actually -use- said context.
right.
> So far so good ... but steal_context can drop the lock iirc. So you may
> need to re-flush there. Not sure that can happen in practice but better
> safe than sorry. I would have preferred seeing that flush near the end
> of the function to avoid such problem.
I can fix this. For 476, I don't think that even if steal_context()
could be called, it wouldn't drop the lock. But then again, if we
enable this for other architectures, it may be a possibility.
> Then, you can end up in cases where you flush the TLB, but your context
> is marked stale due to stealing, and flush again. That's one of the
> reason I wonder if we can consolidate a bit the two orthogonal
> "staleness" concepts we have now.
>
> Granted, stealing on 47x is unlikely, but I have reasons to think that
> this lazy flushing will benefit 440 too.
>
> > pr_hard("[%d] activating context for mm @%p, active=%d, id=%d",
> > cpu, next, next->context.active, next->context.id);
> >
> > @@ -227,7 +292,12 @@ void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
> > id = next_context;
> > if (id > last_context)
> > id = first_context;
> > - map = context_map;
> > +
> > + if (tlb_lazy_flush) {
> > + recycle_stale_contexts();
> > + map = context_available_map;
> > + } else
> > + map = context_map;
> >
> > /* No more free contexts, let's try to steal one */
> > if (nr_free_contexts == 0) {
> > @@ -250,6 +320,13 @@ void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
> > if (id > last_context)
> > id = first_context;
> > }
> > + if (tlb_lazy_flush)
> > + /*
> > + * In the while loop above, we set the bit in
> > + * context_available_map, it also needs to be set in
> > + * context_map
> > + */
> > + __set_bit(id, context_map);
> > stolen:
> > next_context = id + 1;
> > context_mm[id] = next;
> > @@ -267,7 +344,7 @@ void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
> > id, cpu_first_thread_in_core(cpu),
> > cpu_last_thread_in_core(cpu));
> >
> > - local_flush_tlb_mm(next);
> > + __local_flush_tlb_mm(next);
> >
> > /* XXX This clear should ultimately be part of local_flush_tlb_mm */
> > for (i = cpu_first_thread_in_core(cpu);
> > @@ -317,11 +394,61 @@ void destroy_context(struct mm_struct *mm)
> > mm->context.active = 0;
> > #endif
> > context_mm[id] = NULL;
> > - nr_free_contexts++;
> > +
> > + if (tlb_lazy_flush)
> > + nr_stale_contexts++;
> > + else
> > + nr_free_contexts++;
> > }
> > raw_spin_unlock_irqrestore(&context_lock, flags);
> > }
>
> Now...
>
> > +/*
> > + * This is called from flush_tlb_mm(). Mark the current context as stale
> > + * and grab an available one. The tlb will be flushed when no more
> > + * contexts are available
> > + */
> > +void lazy_flush_context(struct mm_struct *mm)
> > +{
> > + unsigned int id;
> > + unsigned long flags;
> > + unsigned long *map;
> > +
> > + raw_spin_lock_irqsave(&context_lock, flags);
> > +
> > + id = mm->context.id;
> > + if (unlikely(id == MMU_NO_CONTEXT))
> > + goto no_context;
>
> First thing is ... you reproduce quite a bit of logic from
> switch_mmu_context() here. Shouldn't it be abstracted in a separate
> function ?
I'm sure there's something I can do there.
> The other thing here is that another CPU might have done a
> recycle_stale_contexts() before you get here. IE. Your TLB may be stale.
> Shouln't you do a flush here ? Since you are picking up a new PID from
> the context_available_map, it can potentially be stale if your tlb needs
> flushing due to another CPU having just done a recycle.
It looks like I missed that. It does seem that there should be a flush
in here.
> > + /*
> > + * Make the existing context stale. It remains in
> > + * context_available_map as long as nr_free_contexts remains non-zero
> > + */
> > + __clear_bit(id, context_map);
> > + context_mm[id] = NULL;
> > + nr_stale_contexts++;
> > +
> > + recycle_stale_contexts();
> > + BUG_ON(nr_free_contexts == 0);
> > +
> > + nr_free_contexts--;
> > + id = last_context;
> > + map = context_available_map;
> > + while (__test_and_set_bit(id, map)) {
> > + id = find_next_zero_bit(map, last_context+1, id);
> > + if (id > last_context)
> > + id = first_context;
> > + }
> > + set_bit(id, context_map);
> > + next_context = id + 1;
> > + context_mm[id] = mm;
> > + mm->context.id = id;
> > + if (current->active_mm == mm)
> > + set_context(id, mm->pgd);
> > +no_context:
> > + raw_spin_unlock_irqrestore(&context_lock, flags);
> > +}
> > +
> > #ifdef CONFIG_SMP
> >
> > static int __cpuinit mmu_context_cpu_notify(struct notifier_block *self,
> > @@ -407,6 +534,7 @@ void __init mmu_context_init(void)
> > } else if (mmu_has_feature(MMU_FTR_TYPE_47x)) {
> > first_context = 1;
> > last_context = 65535;
> > + tlb_lazy_flush = 1;
> > } else {
> > first_context = 1;
> > last_context = 255;
>
> Somebody should measure on 440, might actually improve perfs. Something
> like a kernel compile sounds like a good test here.
I think I'm going to dust off the newer implementation based on your and
Paul's design. I can probably get that in good working order without
too much more work, and it's something we need to look at eventually
anyway. If I find anything that really gets in my way, I might fix up
this patch in the mean time.
Thanks,
Shaggy
--
Dave Kleikamp
IBM Linux Technology Center
^ permalink raw reply
* Re: PROBLEM: memory corrupting bug, bisected to 6dda9d55
From: Thomas Gleixner @ 2010-10-18 21:55 UTC (permalink / raw)
To: Andrew Morton
Cc: Helmut Grohne, Mel Gorman, LKML, linux-mm, pacman, linuxppc-dev
In-Reply-To: <20101018123750.ef7d6d48.akpm@linux-foundation.org>
On Mon, 18 Oct 2010, Andrew Morton wrote:
> On Mon, 18 Oct 2010 12:33:31 +0100
> Mel Gorman <mel@csn.ul.ie> wrote:
>
> > A bit but I still don't know why it would cause corruption. Maybe this is still
> > a caching issue but the difference in timing between list_add and list_add_tail
> > is enough to hide the bug. It's also possible there are some registers
> > ioremapped after the memmap array and reading them is causing some
> > problem.
> >
> > Andrew, what is the right thing to do here? We could flail around looking
> > for explanations as to why the bug causes a user buffer corruption but never
> > get an answer or do we go with this patch, preferably before 2.6.36 releases?
>
> Well, you've spotted a bug so I'd say we fix it asap.
>
> It's a bit of a shame that we lose the only known way of reproducing a
> different bug, but presumably that will come back and bite someone else
> one day, and we'll fix it then :(
I might be completely one off as usual, but this thing reminds me of a
bug I stared at yesterday night:
http://permalink.gmane.org/gmane.linux.kernel/1049605
Reporter Cc'ed
Thanks,
tglx
^ permalink raw reply
* Re: PROBLEM: memory corrupting bug, bisected to 6dda9d55
From: pacman @ 2010-10-18 21:33 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Mel Gorman, linux-mm, Andrew Morton, linuxppc-dev, linux-kernel
In-Reply-To: <1287436205.2341.14.camel@pasglop>
Benjamin Herrenschmidt writes:
>
> You can do something fun... like a timer interrupt that peeks at those
> physical addresses from the linear mapping for example, and try to find
> out "when" they get set to the wrong value (you should observe the load
> from disk, then the corruption, unless they end up being loaded
> incorrectly (ie. dma coherency problem ?) ...
I'm headed toward something like that. Maybe not a timer, maybe a "check it
every time the kernel is entered". But first I have to work out exactly when
the disk load completes so I know when to start checking.
>
> >From there, you might be able to close onto the culprit a bit more, for
> example, try using the DABR register to set data access breakpoints
> shortly before the corruption spot. AFAIK, On those old 32-bit CPUs, you
> can set whether you want it to break on a real or a virtual address.
I thought of that, but as far as I can tell, this CPU doesn't have DABR.
/proc/cpuinfo
processor : 0
cpu : 7447/7457
clock : 999.999990MHz
revision : 1.1 (pvr 8002 0101)
bogomips : 66.66
timebase : 33333333
platform : CHRP
model : Pegasos2
machine : CHRP Pegasos2
Memory : 512 MB
My next thought was: right after the correct value appears in memory, unmap
the page from the kernel and let it Oops when it tries to write there. Then I
found out that the kernel is using BATs instead of page tables for its own
view of memory. Booting with "nobats" completely changes the memory usage
pattern (probably because it's allocating a lot of pages to hold PTEs that it
didn't need before)
>
> You can also sprinkle tests for the page content through the code if
> that doesn't work to try to "close in" on the culprit (for example if
> it's a case of stray DMA, like a network driver bug or such).
No network drivers are loaded when this happens.
--
Alan Curry
^ permalink raw reply
* Re: PROBLEM: memory corrupting bug, bisected to 6dda9d55
From: Benjamin Herrenschmidt @ 2010-10-18 21:10 UTC (permalink / raw)
To: pacman; +Cc: Mel Gorman, linux-mm, Andrew Morton, linuxppc-dev, linux-kernel
In-Reply-To: <20101018191034.21165.qmail@kosh.dhis.org>
On Mon, 2010-10-18 at 14:10 -0500, pacman@kosh.dhis.org wrote:
> I've been flailing around quite a bit. Here's my latest result:
>
> Since I can view the corruption with md5sum /sbin/e2fsck, I know it's in a
> clean cached page. So I made an extra copy of /sbin/e2fsck, which won't be
> loaded into memory during boot. So now after the corruption happens, I can
> cmp -l /sbin/e2fsck good-e2fsck
> for a quick look at the changed bytes. Much easier than provoking a segfault
> under gdb.
>
> Then I got really creative and wrote a cmp replacement which mmaps the files
> and reports the physical addresses from /proc/self/pagemap of the pages that
> don't match. And the consistent result is that physical pages 64604 and 64609
> (both in the range of the order=9 64512) have wrong contents. And the
> corruption is always a single word 128 bytes after the start of the page.
> Physical addresses 0x0fc5c080 and 0x0fc61080 are hit every time.
.../...
You can do something fun... like a timer interrupt that peeks at those
physical addresses from the linear mapping for example, and try to find
out "when" they get set to the wrong value (you should observe the load
from disk, then the corruption, unless they end up being loaded
incorrectly (ie. dma coherency problem ?) ...
>From there, you might be able to close onto the culprit a bit more, for
example, try using the DABR register to set data access breakpoints
shortly before the corruption spot. AFAIK, On those old 32-bit CPUs, you
can set whether you want it to break on a real or a virtual address.
You can also sprinkle tests for the page content through the code if
that doesn't work to try to "close in" on the culprit (for example if
it's a case of stray DMA, like a network driver bug or such).
Cheers,
Ben.
> The values of the corrupted words, observed in 5 consecutive boots, were:
> at 0fc5c080 at 0fc61080
> ----------- -----------
> c3540000 92510000
> 565c0000 23590000
> c85b0000 97580000
> d15f0000 9e5c0000
> d95b0000 a8580000
>
> The low 16 bits are all 0 and the upper 16 bits seem randomly distributed.
> But look at the differences:
>
> c3540000 - 92510000 = 31030000
> 565c0000 - 23590000 = 33030000
> c85b0000 - 97580000 = 31030000
> d15f0000 - 9e5c0000 = 33030000
> d95b0000 - a8580000 = 31030000
>
> This means something... but I don't know what.
>
> In a completely different method of investigation, I went back a few stable
> kernels, got 2.6.33.7 and applied 6dda9d55 to it, thinking that if 6dda9d55
> only reveals a pre-existing bug, I could bisect it using 6dda9d55 as a
> bug-revealing assistant. The bug appeared when running 2.6.33.7 with 6dda9d55
> applied. That was discouraging.
>
> >This patch fixes the problem by ensuring we are not reading a possibly
> >invalid location of memory. It's not clear why the read causes
> >corruption but one way or the other it is a buggy read.
>
> At least that part of the explanation is wrong. Where's the buggy read?
> The action taken by the 6dda9d55 version of __free_one_page looks perfectly
> legitimate to me. Page numbers:
>
> [129024 ] [130048 ] order=10
> [129024 129536] [130048 130560] order=9
>
> 130048 is being freed. 130560 is not free. 129024 (the higher_buddy) is
> already free at order=10. So 130048 is being pushed to the tail of the free
> list, on the speculation that 130560 might soon be free and then the whole
> thing will form an order=11 free page, the only problem being that order=11
> is too high so that later merge will never happen. It's not useful, and maybe
> not conceptually valid to say that 129024 is the buddy of 130048, but it is
> an existing page, and the only way it wouldn't be is if the total memory size
> was not a multiple of 1<<(MAX_ORDER-1) pages
>
> --
> Alan Curry
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: PROBLEM: memory corrupting bug, bisected to 6dda9d55
From: Benjamin Herrenschmidt @ 2010-10-18 21:02 UTC (permalink / raw)
To: Andrew Morton; +Cc: Mel Gorman, linux-mm, pacman, linuxppc-dev, linux-kernel
In-Reply-To: <20101018123750.ef7d6d48.akpm@linux-foundation.org>
On Mon, 2010-10-18 at 12:37 -0700, Andrew Morton wrote:
> Well, you've spotted a bug so I'd say we fix it asap.
>
> It's a bit of a shame that we lose the only known way of reproducing a
> different bug, but presumably that will come back and bite someone
> else
> one day, and we'll fix it then :(
Well, I can always revert that and run some experiments here, provided I
can reproduce the problem at all ...
Cheers,
Ben.
^ permalink raw reply
* Re: PROBLEM: memory corrupting bug, bisected to 6dda9d55
From: Benjamin Herrenschmidt @ 2010-10-18 20:59 UTC (permalink / raw)
To: Mel Gorman
Cc: Yinghai Lu, linux-kernel, linux-mm, pacman, KOSAKI Motohiro,
Andrew Morton, linuxppc-dev, Christoph Lameter
In-Reply-To: <20101013144044.GS30667@csn.ul.ie>
On Wed, 2010-10-13 at 15:40 +0100, Mel Gorman wrote:
>
> This is somewhat contrived but I can see how it might happen even on one
> CPU particularly if the L1 cache is virtual and is loose about checking
> physical tags.
>
> > How sensitive/vulnerable is PPC32 to such things?
> >
>
> I can not tell you specifically but if the above scenario is in any way
> plausible, I believe it would depend on what sort of L1 cache the CPU
> has. Maybe this particular version has a virtual cache with no physical
> tagging and is depending on the OS not to make virtual aliasing mistakes.
Nah, ppc doesn't have problems with cache aliases, it all looks
physically tagged to the programmer (tho there's subtleties but none
that explains the reported behaviour).
Looks like real memory corruption to me.
Cheers,
Ben.
^ permalink raw reply
* Re: PROBLEM: memory corrupting bug, bisected to 6dda9d55
From: Andrew Morton @ 2010-10-18 19:37 UTC (permalink / raw)
To: Mel Gorman; +Cc: linux-mm, pacman, linuxppc-dev, linux-kernel
In-Reply-To: <20101018113331.GB30667@csn.ul.ie>
On Mon, 18 Oct 2010 12:33:31 +0100
Mel Gorman <mel@csn.ul.ie> wrote:
> A bit but I still don't know why it would cause corruption. Maybe this is still
> a caching issue but the difference in timing between list_add and list_add_tail
> is enough to hide the bug. It's also possible there are some registers
> ioremapped after the memmap array and reading them is causing some
> problem.
>
> Andrew, what is the right thing to do here? We could flail around looking
> for explanations as to why the bug causes a user buffer corruption but never
> get an answer or do we go with this patch, preferably before 2.6.36 releases?
Well, you've spotted a bug so I'd say we fix it asap.
It's a bit of a shame that we lose the only known way of reproducing a
different bug, but presumably that will come back and bite someone else
one day, and we'll fix it then :(
^ permalink raw reply
* Re: PROBLEM: memory corrupting bug, bisected to 6dda9d55
From: pacman @ 2010-10-18 19:10 UTC (permalink / raw)
To: Mel Gorman; +Cc: linux-mm, Andrew Morton, linuxppc-dev, linux-kernel
In-Reply-To: <20101018113331.GB30667@csn.ul.ie>
Mel Gorman writes:
>
> A bit but I still don't know why it would cause corruption. Maybe this is still
> a caching issue but the difference in timing between list_add and list_add_tail
> is enough to hide the bug. It's also possible there are some registers
> ioremapped after the memmap array and reading them is causing some
> problem.
I've been doing a lot more tests and I'm sure that 6dda9d55 is not really
responsible. It just happens to provoke the bug in my particular setup.
Whatever it is, it's very sensitive to small changes.
At the end of free_all_bootmem, the free list for order 9 has 4 entries.
Which one is at the head of the list depends on whether 6dda9d55 is applied
or not. If page number 130048 is at the head of the list, it gets used fairly
soon, and everything's fine. The alternative is that page number 64512 is at
the head of the list, so it gets used fairly soon, and corruption occurs.
>
> Andrew, what is the right thing to do here? We could flail around looking
> for explanations as to why the bug causes a user buffer corruption but never
> get an answer or do we go with this patch, preferably before 2.6.36 releases?
I've been flailing around quite a bit. Here's my latest result:
Since I can view the corruption with md5sum /sbin/e2fsck, I know it's in a
clean cached page. So I made an extra copy of /sbin/e2fsck, which won't be
loaded into memory during boot. So now after the corruption happens, I can
cmp -l /sbin/e2fsck good-e2fsck
for a quick look at the changed bytes. Much easier than provoking a segfault
under gdb.
Then I got really creative and wrote a cmp replacement which mmaps the files
and reports the physical addresses from /proc/self/pagemap of the pages that
don't match. And the consistent result is that physical pages 64604 and 64609
(both in the range of the order=9 64512) have wrong contents. And the
corruption is always a single word 128 bytes after the start of the page.
Physical addresses 0x0fc5c080 and 0x0fc61080 are hit every time.
The values of the corrupted words, observed in 5 consecutive boots, were:
at 0fc5c080 at 0fc61080
----------- -----------
c3540000 92510000
565c0000 23590000
c85b0000 97580000
d15f0000 9e5c0000
d95b0000 a8580000
The low 16 bits are all 0 and the upper 16 bits seem randomly distributed.
But look at the differences:
c3540000 - 92510000 = 31030000
565c0000 - 23590000 = 33030000
c85b0000 - 97580000 = 31030000
d15f0000 - 9e5c0000 = 33030000
d95b0000 - a8580000 = 31030000
This means something... but I don't know what.
In a completely different method of investigation, I went back a few stable
kernels, got 2.6.33.7 and applied 6dda9d55 to it, thinking that if 6dda9d55
only reveals a pre-existing bug, I could bisect it using 6dda9d55 as a
bug-revealing assistant. The bug appeared when running 2.6.33.7 with 6dda9d55
applied. That was discouraging.
>This patch fixes the problem by ensuring we are not reading a possibly
>invalid location of memory. It's not clear why the read causes
>corruption but one way or the other it is a buggy read.
At least that part of the explanation is wrong. Where's the buggy read?
The action taken by the 6dda9d55 version of __free_one_page looks perfectly
legitimate to me. Page numbers:
[129024 ] [130048 ] order=10
[129024 129536] [130048 130560] order=9
130048 is being freed. 130560 is not free. 129024 (the higher_buddy) is
already free at order=10. So 130048 is being pushed to the tail of the free
list, on the speculation that 130560 might soon be free and then the whole
thing will form an order=11 free page, the only problem being that order=11
is too high so that later merge will never happen. It's not useful, and maybe
not conceptually valid to say that 129024 is the buddy of 130048, but it is
an existing page, and the only way it wouldn't be is if the total memory size
was not a multiple of 1<<(MAX_ORDER-1) pages
--
Alan Curry
^ permalink raw reply
* RE: msi_bitmap.c question
From: Tirumala Marri @ 2010-10-18 18:04 UTC (permalink / raw)
To: michael; +Cc: linuxppc-dev
In-Reply-To: <1287102354.4194.15.camel@concordia>
> >
> > I am trying to resubmit a patch for MSI support for ppc4xx devices.
> > One of the review feedback was not to use the bit map as it is only
> > for the devices which don=92t have hard wired mapping between interrupt
> > controller interrupts and MSI number. For example intr-ctrl0
> interrupt
> > 20 goes to MSI-0, interrupt 21 goes to MSI-1 ..etc. But when I
> checked
> > freescale SoCs and cell SoCs they have interrupts hard wired to MSI
> > interrupts.
> >
> >
> >
> > Why do they have to use the bitmap and create irqhost, even though
> > they are one-to-one mapped between interrupt controller numbers and
> > MSI ?
>
> I'm not quite sure I understand your question.
>
> The MSI bitmap and the irq_host are two different things.
>
> The MSI bitmap is basically an allocator for hardware numbers that can
> be used for MSI. On some interrupt controllers that might be any
> interrupt that's not used, on others there are restrictions on which
> numbers can be used for MSI, it depends. So it's possible you don't
> need to use that code, but I don't know how your hardware works.
>
> The irq_host is the struct that controls mapping hardware irq numbers
> into linux irq numbers. The cell MSI code has no restrictions on what
> the MSI value is, so it just uses the Linux irq number directly using
> irq_create_direct_mapping().
>
Mike, thanks. Could please you clarify your statement
" The cell MSI code has no restrictions on what the MSI value is ".
If MSIs are one to one mapped to system interrupt controller
Interrupts, why do we need to create new irq_host? Isn't passing
Interrupt controllers irq_host instance not enough ?
Also when is cascade is needed?
Regards,
Marri
^ permalink raw reply
* Re: Freescale P2020 CPU Freeze over PCIe abort signal
From: Eran Liberty @ 2010-10-18 18:00 UTC (permalink / raw)
Cc: linuxppc-dev, linux-pci
In-Reply-To: <4CBB4D80.3030007@extricom.com>
Eran Liberty wrote:
> This should probably go to the Freescale support, as it feels like a
> hardware issue yet the end result is a very frozen Linux kernel so I
> post here first...
>
> I have a programmable FPGA PCIe device connected to a Freescale's
> P2020 PCIe port. As part of the bring-up tests, we are testing two
> faulty scenarios:
> 1. The FPGA totally ignores the PCIe transaction.
> 2. The FPGA return a transaction abort.
>
> Both are plausible PCIe behavior and their should be outcome is
> documented in the PCIe spec. The first should be terminated by the
> transaction requestor timeout mechanism and raise an error, the second
> should abort the transaction and raise and error.
>
> In P2020 if I do any of those the CPU is left hung over the transaction.
>
> something like:
> in_le32(addr)
>
> is turned into:
> 7c 00 04 ac sync 7c 00 4c 2c lwbrx r0,0,r9
> 0c 00 00 00 twi 0,r0,0
> 4c 00 01 2c isync
>
> assembly code, where in r9 (in this example) hold an address which is
> physically mapped into the PCIe resource space.
>
> The CPU will hang over the load instruction.
>
> Just for the fun of it, I have wrote my own assembly function omitting
> everything but the load instruction; still freeze.
> Replace "lwbrx" with a simple "lwz"; still freeze.
>
> It looks like the CPU snoozes till the PCIe transaction is done with
> no timeouts, ignoring any abort signal.
>
> I am going to:
> A. Try to reach the Freescale support.
> B. Asked the FPGA designed to give me a new behavior that will stall
> the PCIe transaction replay for 10 sec, but after those return ok.
> C. report back here with either A or B.
>
> If you have any ideas I would love to hear them.
>
> -- Liberty
>
Some more info:
As said the the FPGA designer provided me a PCIe device that will stall
its response to a variable amount of time. The CPU became un-frozen
after this amount of time. More over, we have found that in that period
till it un-froze the PCIe core did a retry to that transaction over and
over every 40 ms. This gave me the bright idea to look for the word
"retry" in the Freescale documentation which rewarded me with these
registers:
------------------------------------------------------- snip
-------------------------------------------------------
16.3.2.3 PCI Express Outbound Completion Timeout Register
(PEX_OTB_CPL_TOR)
The PCI Express outbound completion timeout register, shown in Figure
16-4, contains the maximum wait
time for a response to come back as a result of an outbound non-posted
request before a timeout condition
occurs.
Offset
0x00C
Access: Read/Write
0 1 5 7
8
31
R
TD
— TC
W
Reset 0 0 0 0 0 0 0 0 0 0 0 1 0 0 0 0
1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1
Figure 16-4. PCI Express Outbound Completion Timeout
Register (PEX_OTB_CPL_TOR)
Table 16-6 describes the PCI Express outbound completion timeout
register fields.
Table 16-6. PEX_OTB_CPL_TOR Field
Descriptions
Bits Name
Description
0 TD Timeout disable. This bit controls the
enabling/disabling of the timeout function.
0 Enable completion timeout
1 Disable completion timeout
1–7 — Reserved
8–31 TC Timeout counter. This is the value that is used to
load the response counter of the completion timeout.
One TC unit is 8× the PCI Express controller clock
period; that is, one TC unit is 20 ns at 400 MHz, and 30
ns at 266.66 MHz.
The following are examples of timeout periods based
on different TC settings:
0x00_0000 Reserved
0x10_FFFF 22.28 ms at 400 MHz controller clock; 33.34
ms at 266.66 MHz controller clock
0xFF_FFFF 335.54 ms at 400 MHz controller clock;
503.31 ms at 266.66 MHz controller clock
16.3.2.4 PCI Express Configuration Retry Timeout Register
(PEX_CONF_RTY_TOR)
The PCI Express configuration retry timeout register, shown in Figure
16-5, contains the maximum time
period during which retries of configuration transactions which resulted
in a CRS response occur.
Offset
0x010
Access: Read/Write
0 1 3
4
31
R
RD — TC
W
Reset 0 0 0 0 0 1 0 0 0 0 0 0 0 0 0 0 1 1 1 1
1 1 1 1 1 1 1 1 1 1 1 1
Figure 16-5. PCI Express Configuration Retry Timeout Register
(PEX_CONF_RTY_TOR)
QorIQ P2020 Integrated Processor Reference
Manual, Rev. 0
16-12
Freescale Semiconductor
PCI Express Interface Controller
Table 16-7 describes the PCI Express configuration retry timeout
register fields.
Table 16-7. PEX_CONF_RTY_TOR Field Descriptions
Bits Name Description
0 RD Retry disable. This bit disables the retry of a
configuration transaction that receives a CRS status response
packet.
0 Enable retry of a configuration transaction in response
to receiving a CRS status response until the timeout
counter (defined by the PEX_CONF_RTY_TOR[TC] field)
has expired.
1 Disable retry of a configuration transaction regardless
of receiving a CRS status response.
1–3 — Reserved
4–31 TC Timeout counter. This is the value that is used to load
the CRS response counter.
One TC unit is 8× the PCI Express controller clock
period; that is, one TC unit is 20 ns at 400 MHz and 30 ns
at 266.66 MHz.
Timeout period based on different TC settings:
0x000_0000 Reserved
0x400_FFFF 1.34 s at 400 MHz controller clock,
2.02 s at 266.66 MHz controller clock
0xFFF_FFFF 5.37 s at 400 MHz controller clock,
8.05 s at 266.66 MHz controller clock
------------------------------------------------------- snap
-------------------------------------------------------
Now this is all nice on the paper, but what the P2020 seems to be doing
in reality is
1. never expire
2. do re-tries even in the non configuration access
I am going to try to disable completion timeout and see if I get better
behavior.
-- Liberty
^ permalink raw reply
* [PATCH 1/7] microblaze: pci-common cleanup
From: Nishanth Aravamudan @ 2010-10-18 17:26 UTC (permalink / raw)
To: nacc
Cc: Michal Simek, linux-kernel, miltonm, Jesse Barnes,
microblaze-uclinux, linuxppc-dev, Bjorn Helgaas
In-Reply-To: <1287422825-14999-1-git-send-email-nacc@us.ibm.com>
Use set_dma_ops and remove now used-once oddly named temp pointer sd.
Signed-off-by: Milton Miller <miltonm@bga.com>
Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
Cc: benh@kernel.crashing.org
Cc: linuxppc-dev@lists.ozlabs.org
---
arch/microblaze/pci/pci-common.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 55ef532..9631e1d 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -1075,8 +1075,6 @@ void __devinit pcibios_setup_bus_devices(struct pci_bus *bus)
bus->number, bus->self ? pci_name(bus->self) : "PHB");
list_for_each_entry(dev, &bus->devices, bus_list) {
- struct dev_archdata *sd = &dev->dev.archdata;
-
/* Setup OF node pointer in archdata */
dev->dev.of_node = pci_device_to_OF_node(dev);
@@ -1086,8 +1084,8 @@ void __devinit pcibios_setup_bus_devices(struct pci_bus *bus)
set_dev_node(&dev->dev, pcibus_to_node(dev->bus));
/* Hook up default DMA ops */
- sd->dma_ops = pci_dma_ops;
- sd->dma_data = (void *)PCI_DRAM_OFFSET;
+ set_dma_ops(&dev->dev, pci_dma_ops);
+ dev->dev.archdata.dma_data = (void *)PCI_DRAM_OFFSET;
/* Read default IRQs and fixup if necessary */
pci_read_irq_line(dev);
--
1.7.1
^ permalink raw reply related
* [PATCH 7/7] ppc64 iommu: use coherent_dma_mask for alloc_coherent
From: Nishanth Aravamudan @ 2010-10-18 17:27 UTC (permalink / raw)
To: nacc; +Cc: linux-kernel, miltonm, Paul Mackerras, Brian King, linuxppc-dev
In-Reply-To: <1287422825-14999-1-git-send-email-nacc@us.ibm.com>
The IOMMU code has been passing the dma-mask instead of the
coherent_dma_mask to the iommu allocator. Coherent allocations should
be made using the coherent_dma_mask.
Also update the vio code to ensure the coherent_dma_mask is set. Without
this change drivers, such as ibmvscsi, fail to load with the corrected
dma_iommu_alloc_coherent().
Signed-off-by: Milton Miller <miltonm@bga.com>
Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
---
We currently don't check the mask other than to warn when its being set,
so I don't think this is stable material.
---
arch/powerpc/kernel/dma-iommu.c | 2 +-
arch/powerpc/kernel/vio.c | 4 ++++
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/kernel/dma-iommu.c b/arch/powerpc/kernel/dma-iommu.c
index 6e54a0f..e755415 100644
--- a/arch/powerpc/kernel/dma-iommu.c
+++ b/arch/powerpc/kernel/dma-iommu.c
@@ -19,7 +19,7 @@ static void *dma_iommu_alloc_coherent(struct device *dev, size_t size,
dma_addr_t *dma_handle, gfp_t flag)
{
return iommu_alloc_coherent(dev, get_iommu_table_base(dev), size,
- dma_handle, device_to_mask(dev), flag,
+ dma_handle, dev->coherent_dma_mask, flag,
dev_to_node(dev));
}
diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
index 3c3083f..f15b3df 100644
--- a/arch/powerpc/kernel/vio.c
+++ b/arch/powerpc/kernel/vio.c
@@ -1259,6 +1259,10 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
viodev->dev.parent = &vio_bus_device.dev;
viodev->dev.bus = &vio_bus_type;
viodev->dev.release = vio_dev_release;
+ /* needed to ensure proper operation of coherent allocations
+ * later, in case driver doesn't set it explicitly */
+ dma_set_mask(&viodev->dev, DMA_BIT_MASK(64));
+ dma_set_coherent_mask(&viodev->dev, DMA_BIT_MASK(64));
/* register with generic device framework */
if (device_register(&viodev->dev)) {
--
1.7.1
^ permalink raw reply related
* [PATCH 2/7] ppc/vio: use dma ops helpers
From: Nishanth Aravamudan @ 2010-10-18 17:26 UTC (permalink / raw)
To: nacc; +Cc: linux-kernel, miltonm, Paul Mackerras, Brian King, linuxppc-dev
In-Reply-To: <1287422825-14999-1-git-send-email-nacc@us.ibm.com>
Use the set_dma_ops helper. Instead of modifying vio_dma_mapping_ops,
just create a trivial wrapper for dma_supported.
Signed-off-by: Milton Miller <miltonm@bga.com>
Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
---
arch/powerpc/kernel/vio.c | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
index d692989..3c3083f 100644
--- a/arch/powerpc/kernel/vio.c
+++ b/arch/powerpc/kernel/vio.c
@@ -602,6 +602,11 @@ static void vio_dma_iommu_unmap_sg(struct device *dev,
vio_cmo_dealloc(viodev, alloc_size);
}
+static int vio_dma_iommu_dma_supported(struct device *dev, u64 mask)
+{
+ return dma_iommu_ops.dma_supported(dev, mask);
+}
+
struct dma_map_ops vio_dma_mapping_ops = {
.alloc_coherent = vio_dma_iommu_alloc_coherent,
.free_coherent = vio_dma_iommu_free_coherent,
@@ -609,6 +614,7 @@ struct dma_map_ops vio_dma_mapping_ops = {
.unmap_sg = vio_dma_iommu_unmap_sg,
.map_page = vio_dma_iommu_map_page,
.unmap_page = vio_dma_iommu_unmap_page,
+ .dma_supported = vio_dma_iommu_dma_supported,
};
@@ -860,8 +866,7 @@ static void vio_cmo_bus_remove(struct vio_dev *viodev)
static void vio_cmo_set_dma_ops(struct vio_dev *viodev)
{
- vio_dma_mapping_ops.dma_supported = dma_iommu_ops.dma_supported;
- viodev->dev.archdata.dma_ops = &vio_dma_mapping_ops;
+ set_dma_ops(&viodev->dev, &vio_dma_mapping_ops);
}
/**
@@ -1246,7 +1251,7 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
if (firmware_has_feature(FW_FEATURE_CMO))
vio_cmo_set_dma_ops(viodev);
else
- viodev->dev.archdata.dma_ops = &dma_iommu_ops;
+ set_dma_ops(&viodev->dev, &dma_iommu_ops);
set_iommu_table_base(&viodev->dev, vio_build_iommu_table(viodev));
set_dev_node(&viodev->dev, of_node_to_nid(of_node));
--
1.7.1
^ permalink raw reply related
* [PATCH 5/7] ppc/dart: iommu table cleanup
From: Nishanth Aravamudan @ 2010-10-18 17:27 UTC (permalink / raw)
To: nacc
Cc: linux-kernel, miltonm, André Goddard Rosa, Paul Mackerras,
Tejun Heo, linuxppc-dev
In-Reply-To: <1287422825-14999-1-git-send-email-nacc@us.ibm.com>
No need to set the device tree device_node pci node iommu pointer, its
only used for dlpar remove.
Signed-off-by: Milton Miller <miltonm@bga.com>
Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
---
arch/powerpc/sysdev/dart_iommu.c | 9 +--------
1 files changed, 1 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/sysdev/dart_iommu.c b/arch/powerpc/sysdev/dart_iommu.c
index 17cf15e..8e9e06a 100644
--- a/arch/powerpc/sysdev/dart_iommu.c
+++ b/arch/powerpc/sysdev/dart_iommu.c
@@ -312,17 +312,10 @@ static void pci_dma_dev_setup_dart(struct pci_dev *dev)
static void pci_dma_bus_setup_dart(struct pci_bus *bus)
{
- struct device_node *dn;
-
if (!iommu_table_dart_inited) {
iommu_table_dart_inited = 1;
iommu_table_dart_setup();
}
-
- dn = pci_bus_to_OF_node(bus);
-
- if (dn)
- PCI_DN(dn)->iommu_table = &iommu_table_dart;
}
static bool dart_device_on_pcie(struct device *dev)
@@ -373,7 +366,7 @@ void __init iommu_init_early_dart(void)
if (dn == NULL) {
dn = of_find_compatible_node(NULL, "dart", "u4-dart");
if (dn == NULL)
- goto bail;
+ return; /* use default direct_dma_ops */
dart_is_u4 = 1;
}
--
1.7.1
^ permalink raw reply related
* [PATCH 3/7] ppc/pasemi: clean up pasemi iommu table initializations
From: Nishanth Aravamudan @ 2010-10-18 17:27 UTC (permalink / raw)
To: nacc
Cc: linuxppc-dev, miltonm, linux-kernel, Paul Mackerras,
Olof Johansson, Yinghai Lu
In-Reply-To: <1287422825-14999-1-git-send-email-nacc@us.ibm.com>
No need for empty helpers with iommu off, the ppc_md hooks are optional.
The direct_dma_ops are the default pci_dma_ops, so no need to set in the
them iommu off case.
No need to set the device tree device_node pci node iommu pointer, its
only used for dlpar remove.
Signed-off-by: Milton Miller <miltonm@bga.com>
Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
Acked-by: Olof Johansson <olof@lixom.net>
---
arch/powerpc/platforms/pasemi/iommu.c | 19 +------------------
1 files changed, 1 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/platforms/pasemi/iommu.c b/arch/powerpc/platforms/pasemi/iommu.c
index 1f9fb2c..14943ef 100644
--- a/arch/powerpc/platforms/pasemi/iommu.c
+++ b/arch/powerpc/platforms/pasemi/iommu.c
@@ -156,20 +156,12 @@ static void iommu_table_iobmap_setup(void)
static void pci_dma_bus_setup_pasemi(struct pci_bus *bus)
{
- struct device_node *dn;
-
pr_debug("pci_dma_bus_setup, bus %p, bus->self %p\n", bus, bus->self);
if (!iommu_table_iobmap_inited) {
iommu_table_iobmap_inited = 1;
iommu_table_iobmap_setup();
}
-
- dn = pci_bus_to_OF_node(bus);
-
- if (dn)
- PCI_DN(dn)->iommu_table = &iommu_table_iobmap;
-
}
@@ -192,9 +184,6 @@ static void pci_dma_dev_setup_pasemi(struct pci_dev *dev)
set_iommu_table_base(&dev->dev, &iommu_table_iobmap);
}
-static void pci_dma_bus_setup_null(struct pci_bus *b) { }
-static void pci_dma_dev_setup_null(struct pci_dev *d) { }
-
int __init iob_init(struct device_node *dn)
{
unsigned long tmp;
@@ -251,14 +240,8 @@ void __init iommu_init_early_pasemi(void)
iommu_off = of_chosen &&
of_get_property(of_chosen, "linux,iommu-off", NULL);
#endif
- if (iommu_off) {
- /* Direct I/O, IOMMU off */
- ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_null;
- ppc_md.pci_dma_bus_setup = pci_dma_bus_setup_null;
- set_pci_dma_ops(&dma_direct_ops);
-
+ if (iommu_off)
return;
- }
iob_init(NULL);
--
1.7.1
^ 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