* Re: [PATCH] powerpc: Optimise the 64bit optimised __clear_user
From: Andreas Schwab @ 2012-06-08 7:34 UTC (permalink / raw)
To: Scott Wood
Cc: mikey, michael, Paul Mackerras, Anton Blanchard, olof,
linuxppc-dev
In-Reply-To: <4FD0EA08.7040903@freescale.com>
Scott Wood <scottwood@freescale.com> writes:
> and that objdump decodes it as "dcbz r0,r3" rather than "dcbz 0,r3".
<http://permalink.gmane.org/gmane.comp.gnu.binutils/57870>
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH 2/2] [POWERPC] uprobes: powerpc port
From: Michael Ellerman @ 2012-06-08 6:38 UTC (permalink / raw)
To: ananth
Cc: Jim Keniston, Srikar Dronamraju, Peter Zijlstra, lkml, oleg,
Paul Mackerras, Anton Blanchard, Ingo Molnar, linuxppc-dev
In-Reply-To: <20120608061954.GE13409@in.ibm.com>
On Fri, 2012-06-08 at 11:49 +0530, Ananth N Mavinakayanahalli wrote:
> On Fri, Jun 08, 2012 at 04:17:44PM +1000, Michael Ellerman wrote:
> > On Fri, 2012-06-08 at 11:31 +0530, Ananth N Mavinakayanahalli wrote:
> > > On Fri, Jun 08, 2012 at 03:51:54PM +1000, Michael Ellerman wrote:
> > > > On Fri, 2012-06-08 at 10:06 +0530, Ananth N Mavinakayanahalli wrote:
> > >
> > > But MSR_PR=1 and hence emulate_step() will return -1 and hence we will
> > > end up single-stepping using user_enable_single_step(). Same with rfid.
> >
> > Right. But that was exactly Jim's point, you may be asked to emulate
> > those instructions even though you wouldn't expect to see them in
> > userspace code, so you need to handle it.
> >
> > Luckily it looks like emulate_step() will do the right thing for you.
> > It'd be good to test it to make 100% sure.
>
> Sure. Will add that check and send v2.
Sorry I didn't mean add a test in the code, I meant construct a test
case to confirm that it works as expected.
cheers
^ permalink raw reply
* Re: [PATCH 2/2] [POWERPC] uprobes: powerpc port
From: Ananth N Mavinakayanahalli @ 2012-06-08 6:19 UTC (permalink / raw)
To: Michael Ellerman
Cc: Jim Keniston, Srikar Dronamraju, Peter Zijlstra, oleg, lkml,
Paul Mackerras, Anton Blanchard, Ingo Molnar, linuxppc-dev
In-Reply-To: <1339136264.25573.7.camel@concordia>
On Fri, Jun 08, 2012 at 04:17:44PM +1000, Michael Ellerman wrote:
> On Fri, 2012-06-08 at 11:31 +0530, Ananth N Mavinakayanahalli wrote:
> > On Fri, Jun 08, 2012 at 03:51:54PM +1000, Michael Ellerman wrote:
> > > On Fri, 2012-06-08 at 10:06 +0530, Ananth N Mavinakayanahalli wrote:
> > > > On Wed, Jun 06, 2012 at 11:08:04AM -0700, Jim Keniston wrote:
> > > > > 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:
> > > >
> > > > ...
> > > >
> > > > > > 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.
> > > >
> > > > That'd be flagged elsewhere, by the architecture itself -- you'd get a
> > > > privileged instruciton exception if you try execute any instruction not
> > > > part of the UISA. I therefore don't think its a necessary check in the
> > > > uprobes code.
> > >
> > > But you're not executing the instruction, you're passing it to
> > > emulate_step(). Or am I missing something?
> >
> > But MSR_PR=1 and hence emulate_step() will return -1 and hence we will
> > end up single-stepping using user_enable_single_step(). Same with rfid.
>
> Right. But that was exactly Jim's point, you may be asked to emulate
> those instructions even though you wouldn't expect to see them in
> userspace code, so you need to handle it.
>
> Luckily it looks like emulate_step() will do the right thing for you.
> It'd be good to test it to make 100% sure.
Sure. Will add that check and send v2.
Ananth
^ permalink raw reply
* Re: [PATCH 2/2] [POWERPC] uprobes: powerpc port
From: Michael Ellerman @ 2012-06-08 6:17 UTC (permalink / raw)
To: ananth
Cc: Jim Keniston, Srikar Dronamraju, Peter Zijlstra, oleg, lkml,
Paul Mackerras, Anton Blanchard, Ingo Molnar, linuxppc-dev
In-Reply-To: <20120608060104.GD13409@in.ibm.com>
On Fri, 2012-06-08 at 11:31 +0530, Ananth N Mavinakayanahalli wrote:
> On Fri, Jun 08, 2012 at 03:51:54PM +1000, Michael Ellerman wrote:
> > On Fri, 2012-06-08 at 10:06 +0530, Ananth N Mavinakayanahalli wrote:
> > > On Wed, Jun 06, 2012 at 11:08:04AM -0700, Jim Keniston wrote:
> > > > 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:
> > >
> > > ...
> > >
> > > > > 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.
> > >
> > > That'd be flagged elsewhere, by the architecture itself -- you'd get a
> > > privileged instruciton exception if you try execute any instruction not
> > > part of the UISA. I therefore don't think its a necessary check in the
> > > uprobes code.
> >
> > But you're not executing the instruction, you're passing it to
> > emulate_step(). Or am I missing something?
>
> But MSR_PR=1 and hence emulate_step() will return -1 and hence we will
> end up single-stepping using user_enable_single_step(). Same with rfid.
Right. But that was exactly Jim's point, you may be asked to emulate
those instructions even though you wouldn't expect to see them in
userspace code, so you need to handle it.
Luckily it looks like emulate_step() will do the right thing for you.
It'd be good to test it to make 100% sure.
cheers
^ permalink raw reply
* Re: [PATCH 2/2] [POWERPC] uprobes: powerpc port
From: Ananth N Mavinakayanahalli @ 2012-06-08 6:01 UTC (permalink / raw)
To: Michael Ellerman
Cc: Jim Keniston, Srikar Dronamraju, Peter Zijlstra, lkml, oleg,
Paul Mackerras, Anton Blanchard, Ingo Molnar, linuxppc-dev
In-Reply-To: <1339134714.25573.4.camel@concordia>
On Fri, Jun 08, 2012 at 03:51:54PM +1000, Michael Ellerman wrote:
> On Fri, 2012-06-08 at 10:06 +0530, Ananth N Mavinakayanahalli wrote:
> > On Wed, Jun 06, 2012 at 11:08:04AM -0700, Jim Keniston wrote:
> > > 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:
> >
> > ...
> >
> > > > 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.
> >
> > That'd be flagged elsewhere, by the architecture itself -- you'd get a
> > privileged instruciton exception if you try execute any instruction not
> > part of the UISA. I therefore don't think its a necessary check in the
> > uprobes code.
>
> But you're not executing the instruction, you're passing it to
> emulate_step(). Or am I missing something?
But MSR_PR=1 and hence emulate_step() will return -1 and hence we will
end up single-stepping using user_enable_single_step(). Same with rfid.
Ananth
^ permalink raw reply
* Re: [PATCH 2/2] [POWERPC] uprobes: powerpc port
From: Michael Ellerman @ 2012-06-08 5:51 UTC (permalink / raw)
To: ananth
Cc: Jim Keniston, Srikar Dronamraju, Peter Zijlstra, lkml, oleg,
Paul Mackerras, Anton Blanchard, Ingo Molnar, linuxppc-dev
In-Reply-To: <20120608043605.GB13409@in.ibm.com>
On Fri, 2012-06-08 at 10:06 +0530, Ananth N Mavinakayanahalli wrote:
> On Wed, Jun 06, 2012 at 11:08:04AM -0700, Jim Keniston wrote:
> > 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:
>
> ...
>
> > > 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.
>
> That'd be flagged elsewhere, by the architecture itself -- you'd get a
> privileged instruciton exception if you try execute any instruction not
> part of the UISA. I therefore don't think its a necessary check in the
> uprobes code.
But you're not executing the instruction, you're passing it to
emulate_step(). Or am I missing something?
cheers
^ permalink raw reply
* Re: [PATCH 2/2] [POWERPC] uprobes: powerpc port
From: Ananth N Mavinakayanahalli @ 2012-06-08 4:36 UTC (permalink / raw)
To: Jim Keniston
Cc: Srikar Dronamraju, Peter Zijlstra, oleg, lkml, Paul Mackerras,
Anton Blanchard, Ingo Molnar, linuxppc-dev
In-Reply-To: <1339006084.3458.25.camel@localhost>
On Wed, Jun 06, 2012 at 11:08:04AM -0700, Jim Keniston wrote:
> 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:
...
> > 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.
That'd be flagged elsewhere, by the architecture itself -- you'd get a
privileged instruciton exception if you try execute any instruction not
part of the UISA. I therefore don't think its a necessary check in the
uprobes code.
Ananth
^ permalink raw reply
* Re: [PATCH 1/2] uprobes: Pass probed vaddr to arch_uprobe_analyze_insn()
From: Ananth N Mavinakayanahalli @ 2012-06-08 4:33 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Peter Zijlstra, lkml, oleg, Paul Mackerras, Anton Blanchard,
Ingo Molnar, linuxppc-dev, Ingo Molnar
In-Reply-To: <20120606114423.GA32094@linux.vnet.ibm.com>
On Wed, Jun 06, 2012 at 05:14:23PM +0530, Srikar Dronamraju wrote:
> * Ingo Molnar <mingo@kernel.org> [2012-06-06 11:40:15]:
>
> >
> > * Ananth N Mavinakayanahalli <ananth@in.ibm.com> wrote:
> >
> > > On Wed, Jun 06, 2012 at 11:23:52AM +0200, Peter Zijlstra wrote:
> > > > On Wed, 2012-06-06 at 14:49 +0530, Ananth N Mavinakayanahalli wrote:
> > > > > +int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, loff_t vaddr)
> > > >
> > > > Don't we traditionally use unsigned long to pass vaddrs?
> > >
> > > Right. But the vaddr we pass here is vma_info->vaddr which is loff_t.
> > > I guess I should've made that clear in the patch description.
> >
> > Why not fix struct vma_info's vaddr type?
> >
>
> Calculating and comparing vaddr results either uses variables of type loff_t.
> To avoid typecasting and avoid overflow at each of these places, we used
> loff_t.
>
> Ananth, install_breakpoint() already has a variable of type addr of type
> unsigned long. Why dont you use addr instead of vaddr.
Ok, makes sense. I'll change it in v2.
Ananth
^ permalink raw reply
* [PATCH 5/5] powerpc: iommu: Implement IOMMU pools to improve multiqueue adapter performance
From: Anton Blanchard @ 2012-06-08 4:14 UTC (permalink / raw)
To: Michael Ellerman; +Cc: olof, paulus, linuxppc-dev
In-Reply-To: <1339123423.3330.5.camel@concordia>
At the moment all queues in a multiqueue adapter will serialise
against the IOMMU table lock. This is proving to be a big issue,
especially with 10Gbit ethernet.
This patch creates 4 pools and tries to spread the load across
them. If the table is under 1GB in size we revert back to the
original behaviour of 1 pool and 1 largealloc pool.
We create a hash to map CPUs to pools. Since we prefer interrupts to
be affinitised to primary CPUs, without some form of hashing we are
very likely to end up using the same pool. As an example, POWER7
has 4 way SMT and with 4 pools all primary threads will map to the
same pool.
The largealloc pool is reduced from 1/2 to 1/4 of the space to
partially offset the overhead of breaking the table up into pools.
Some performance numbers were obtained with a Chelsio T3 adapter on
two POWER7 boxes, running a 100 session TCP round robin test.
Performance improved 69% with this patch applied.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
All patches combined improve performance by 178%
v2.0: Fix cell build, noticed by mpe.
Index: linux-build/arch/powerpc/kernel/iommu.c
===================================================================
--- linux-build.orig/arch/powerpc/kernel/iommu.c 2012-06-08 14:02:46.229104320 +1000
+++ linux-build/arch/powerpc/kernel/iommu.c 2012-06-08 14:02:46.861113517 +1000
@@ -33,6 +33,7 @@
#include <linux/bitmap.h>
#include <linux/iommu-helper.h>
#include <linux/crash_dump.h>
+#include <linux/hash.h>
#include <asm/io.h>
#include <asm/prom.h>
#include <asm/iommu.h>
@@ -58,6 +59,26 @@ static int __init setup_iommu(char *str)
__setup("iommu=", setup_iommu);
+static DEFINE_PER_CPU(unsigned int, iommu_pool_hash);
+
+/*
+ * We precalculate the hash to avoid doing it on every allocation.
+ *
+ * The hash is important to spread CPUs across all the pools. For example,
+ * on a POWER7 with 4 way SMT we want interrupts on the primary threads and
+ * with 4 pools all primary threads would map to the same pool.
+ */
+static int __init setup_iommu_pool_hash(void)
+{
+ unsigned int i;
+
+ for_each_possible_cpu(i)
+ per_cpu(iommu_pool_hash, i) = hash_32(i, IOMMU_POOL_HASHBITS);
+
+ return 0;
+}
+subsys_initcall(setup_iommu_pool_hash);
+
static unsigned long iommu_range_alloc(struct device *dev,
struct iommu_table *tbl,
unsigned long npages,
@@ -72,6 +93,8 @@ static unsigned long iommu_range_alloc(s
unsigned long align_mask;
unsigned long boundary_size;
unsigned long flags;
+ unsigned int pool_nr;
+ struct iommu_pool *pool;
align_mask = 0xffffffffffffffffl >> (64 - align_order);
@@ -84,38 +107,46 @@ static unsigned long iommu_range_alloc(s
return DMA_ERROR_CODE;
}
- spin_lock_irqsave(&(tbl->it_lock), flags);
+ /*
+ * We don't need to disable preemption here because any CPU can
+ * safely use any IOMMU pool.
+ */
+ pool_nr = __raw_get_cpu_var(iommu_pool_hash) & (tbl->nr_pools - 1);
- if (handle && *handle)
- start = *handle;
+ if (largealloc)
+ pool = &(tbl->large_pool);
else
- start = largealloc ? tbl->it_largehint : tbl->it_hint;
+ pool = &(tbl->pools[pool_nr]);
+
+ spin_lock_irqsave(&(pool->lock), flags);
- /* Use only half of the table for small allocs (15 pages or less) */
- limit = largealloc ? tbl->it_size : tbl->it_halfpoint;
+again:
+ if ((pass == 0) && handle && *handle)
+ start = *handle;
+ else
+ start = pool->hint;
- if (largealloc && start < tbl->it_halfpoint)
- start = tbl->it_halfpoint;
+ limit = pool->end;
/* The case below can happen if we have a small segment appended
* to a large, or when the previous alloc was at the very end of
* the available space. If so, go back to the initial start.
*/
if (start >= limit)
- start = largealloc ? tbl->it_largehint : tbl->it_hint;
-
- again:
+ start = pool->start;
if (limit + tbl->it_offset > mask) {
limit = mask - tbl->it_offset + 1;
/* If we're constrained on address range, first try
* at the masked hint to avoid O(n) search complexity,
- * but on second pass, start at 0.
+ * but on second pass, start at 0 in pool 0.
*/
- if ((start & mask) >= limit || pass > 0)
- start = 0;
- else
+ if ((start & mask) >= limit || pass > 0) {
+ pool = &(tbl->pools[0]);
+ start = pool->start;
+ } else {
start &= mask;
+ }
}
if (dev)
@@ -129,17 +160,25 @@ static unsigned long iommu_range_alloc(s
tbl->it_offset, boundary_size >> IOMMU_PAGE_SHIFT,
align_mask);
if (n == -1) {
- if (likely(pass < 2)) {
- /* First failure, just rescan the half of the table.
- * Second failure, rescan the other half of the table.
- */
- start = (largealloc ^ pass) ? tbl->it_halfpoint : 0;
- limit = pass ? tbl->it_size : limit;
+ if (likely(pass == 0)) {
+ /* First try the pool from the start */
+ pool->hint = pool->start;
pass++;
goto again;
+
+ } else if (pass <= tbl->nr_pools) {
+ /* Now try scanning all the other pools */
+ spin_unlock(&(pool->lock));
+ pool_nr = (pool_nr + 1) & (tbl->nr_pools - 1);
+ pool = &tbl->pools[pool_nr];
+ spin_lock(&(pool->lock));
+ pool->hint = pool->start;
+ pass++;
+ goto again;
+
} else {
- /* Third failure, give up */
- spin_unlock_irqrestore(&(tbl->it_lock), flags);
+ /* Give up */
+ spin_unlock_irqrestore(&(pool->lock), flags);
return DMA_ERROR_CODE;
}
}
@@ -149,10 +188,10 @@ static unsigned long iommu_range_alloc(s
/* Bump the hint to a new block for small allocs. */
if (largealloc) {
/* Don't bump to new block to avoid fragmentation */
- tbl->it_largehint = end;
+ pool->hint = end;
} else {
/* Overflow will be taken care of at the next allocation */
- tbl->it_hint = (end + tbl->it_blocksize - 1) &
+ pool->hint = (end + tbl->it_blocksize - 1) &
~(tbl->it_blocksize - 1);
}
@@ -160,7 +199,8 @@ static unsigned long iommu_range_alloc(s
if (handle)
*handle = end;
- spin_unlock_irqrestore(&(tbl->it_lock), flags);
+ spin_unlock_irqrestore(&(pool->lock), flags);
+
return n;
}
@@ -235,23 +275,45 @@ static bool iommu_free_check(struct iomm
return true;
}
+static struct iommu_pool *get_pool(struct iommu_table *tbl,
+ unsigned long entry)
+{
+ struct iommu_pool *p;
+ unsigned long largepool_start = tbl->large_pool.start;
+
+ /* The large pool is the last pool at the top of the table */
+ if (entry >= largepool_start) {
+ p = &tbl->large_pool;
+ } else {
+ unsigned int pool_nr = entry / tbl->poolsize;
+
+ BUG_ON(pool_nr > tbl->nr_pools);
+ p = &tbl->pools[pool_nr];
+ }
+
+ return p;
+}
+
static void __iommu_free(struct iommu_table *tbl, dma_addr_t dma_addr,
unsigned int npages)
{
unsigned long entry, free_entry;
unsigned long flags;
+ struct iommu_pool *pool;
entry = dma_addr >> IOMMU_PAGE_SHIFT;
free_entry = entry - tbl->it_offset;
+ pool = get_pool(tbl, free_entry);
+
if (!iommu_free_check(tbl, dma_addr, npages))
return;
ppc_md.tce_free(tbl, entry, npages);
- spin_lock_irqsave(&(tbl->it_lock), flags);
+ spin_lock_irqsave(&(pool->lock), flags);
bitmap_clear(tbl->it_map, free_entry, npages);
- spin_unlock_irqrestore(&(tbl->it_lock), flags);
+ spin_unlock_irqrestore(&(pool->lock), flags);
}
static void iommu_free(struct iommu_table *tbl, dma_addr_t dma_addr,
@@ -493,9 +555,8 @@ struct iommu_table *iommu_init_table(str
unsigned long sz;
static int welcomed = 0;
struct page *page;
-
- /* Set aside 1/4 of the table for large allocations. */
- tbl->it_halfpoint = tbl->it_size * 3 / 4;
+ unsigned int i;
+ struct iommu_pool *p;
/* number of bytes needed for the bitmap */
sz = (tbl->it_size + 7) >> 3;
@@ -514,9 +575,28 @@ struct iommu_table *iommu_init_table(str
if (tbl->it_offset == 0)
set_bit(0, tbl->it_map);
- tbl->it_hint = 0;
- tbl->it_largehint = tbl->it_halfpoint;
- spin_lock_init(&tbl->it_lock);
+ /* We only split the IOMMU table if we have 1GB or more of space */
+ if ((tbl->it_size << IOMMU_PAGE_SHIFT) >= (1UL * 1024 * 1024 * 1024))
+ tbl->nr_pools = IOMMU_NR_POOLS;
+ else
+ tbl->nr_pools = 1;
+
+ /* We reserve the top 1/4 of the table for large allocations */
+ tbl->poolsize = (tbl->it_size * 3 / 4) / IOMMU_NR_POOLS;
+
+ for (i = 0; i < IOMMU_NR_POOLS; i++) {
+ p = &tbl->pools[i];
+ spin_lock_init(&(p->lock));
+ p->start = tbl->poolsize * i;
+ p->hint = p->start;
+ p->end = p->start + tbl->poolsize;
+ }
+
+ p = &tbl->large_pool;
+ spin_lock_init(&(p->lock));
+ p->start = tbl->poolsize * i;
+ p->hint = p->start;
+ p->end = tbl->it_size;
iommu_table_clear(tbl);
Index: linux-build/arch/powerpc/include/asm/iommu.h
===================================================================
--- linux-build.orig/arch/powerpc/include/asm/iommu.h 2012-06-06 14:44:13.106458136 +1000
+++ linux-build/arch/powerpc/include/asm/iommu.h 2012-06-08 14:02:46.861113517 +1000
@@ -53,6 +53,16 @@ static __inline__ __attribute_const__ in
*/
#define IOMAP_MAX_ORDER 13
+#define IOMMU_POOL_HASHBITS 2
+#define IOMMU_NR_POOLS (1 << IOMMU_POOL_HASHBITS)
+
+struct iommu_pool {
+ unsigned long start;
+ unsigned long end;
+ unsigned long hint;
+ spinlock_t lock;
+} ____cacheline_aligned_in_smp;
+
struct iommu_table {
unsigned long it_busno; /* Bus number this table belongs to */
unsigned long it_size; /* Size of iommu table in entries */
@@ -61,10 +71,10 @@ struct iommu_table {
unsigned long it_index; /* which iommu table this is */
unsigned long it_type; /* type: PCI or Virtual Bus */
unsigned long it_blocksize; /* Entries in each block (cacheline) */
- unsigned long it_hint; /* Hint for next alloc */
- unsigned long it_largehint; /* Hint for large allocs */
- unsigned long it_halfpoint; /* Breaking point for small/large allocs */
- spinlock_t it_lock; /* Protects it_map */
+ unsigned long poolsize;
+ unsigned long nr_pools;
+ struct iommu_pool large_pool;
+ struct iommu_pool pools[IOMMU_NR_POOLS];
unsigned long *it_map; /* A simple allocation bitmap for now */
};
Index: linux-build/arch/powerpc/platforms/cell/iommu.c
===================================================================
--- linux-build.orig/arch/powerpc/platforms/cell/iommu.c 2012-04-05 13:47:45.715857539 +1000
+++ linux-build/arch/powerpc/platforms/cell/iommu.c 2012-06-08 14:03:00.053305540 +1000
@@ -518,7 +518,6 @@ cell_iommu_setup_window(struct cbe_iommu
__set_bit(0, window->table.it_map);
tce_build_cell(&window->table, window->table.it_offset, 1,
(unsigned long)iommu->pad_page, DMA_TO_DEVICE, NULL);
- window->table.it_hint = window->table.it_blocksize;
return window;
}
^ permalink raw reply
* Re: [PATCH 1/3][RFC] ftrace/ppc: Have PPC skip updating with stop_machine()
From: Michael Ellerman @ 2012-06-08 4:13 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20120426183508.749515835@goodmis.org>
On Thu, 2012-04-26 at 14:31 -0400, Steven Rostedt wrote:
> plain text document attachment
> (0001-ftrace-ppc-Have-PPC-skip-updating-with-stop_machine.patch)
> From: Steven Rostedt <srostedt@redhat.com>
>
> PPC does not have the synchronization issues that x86 has with
> modifying code on one CPU while another CPU is executing it.
> The other CPU will either see the old or new code without any
> issues, unlike x86 which may issue a GPF.
>
> Instead of calling the heavy stop_machine, just update the code.
This looks nice, but it's giving me this:
arch/powerpc/kernel/ftrace.c:497:13: error: static declaration of 'ftrace_replace_code' follows non-static declaration
include/linux/ftrace.h:317:13: note: previous declaration of 'ftrace_replace_code' was here
I think the fix is just to make our ftrace_replace_code() non-static?
cheers
> +static void ftrace_replace_code(int enable)
> +{
> + struct ftrace_rec_iter *iter;
> + struct dyn_ftrace *rec;
> + int ret;
> +
> + for (iter = ftrace_rec_iter_start(); iter;
> + iter = ftrace_rec_iter_next(iter)) {
> + rec = ftrace_rec_iter_record(iter);
> + ret = __ftrace_replace_code(rec, enable);
> + if (ret) {
> + ftrace_bug(ret, rec->ip);
> + return;
> + }
> + }
> +}
> +
^ permalink raw reply
* Re: [PATCH 5/5] powerpc: iommu: Implement IOMMU pools to improve multiqueue adapter performance
From: Michael Ellerman @ 2012-06-08 4:03 UTC (permalink / raw)
To: Anton Blanchard; +Cc: olof, paulus, linuxppc-dev
In-Reply-To: <20120608140204.49f475e8@kryten>
Anton Blanchard <anton@samba.org> wrote:
>
>Hi,
>
>> Breaks the cell code with:
>>
>> arch/powerpc/platforms/cell/iommu.c:521:15: error: 'struct
>> iommu_table' has no member named 'it_hint'
>
>Yuck, I'll spin a fix. There's no need for the code to bump the hint.
OK, I can fix it up.
Cheers
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH 5/5] powerpc: iommu: Implement IOMMU pools to improve multiqueue adapter performance
From: Anton Blanchard @ 2012-06-08 4:02 UTC (permalink / raw)
To: Michael Ellerman; +Cc: olof, paulus, linuxppc-dev
In-Reply-To: <1339123423.3330.5.camel@concordia>
Hi,
> Breaks the cell code with:
>
> arch/powerpc/platforms/cell/iommu.c:521:15: error: 'struct
> iommu_table' has no member named 'it_hint'
Yuck, I'll spin a fix. There's no need for the code to bump the hint.
Anton
^ permalink raw reply
* [PATCH powerpc] fix Section mismatch warnings in prom_init.c
From: Li Zhong @ 2012-06-08 3:44 UTC (permalink / raw)
To: LKML; +Cc: Paul Mackerras, PowerPC email list
This patches tries to fix a couple of Section mismatch warnings like
following one:
WARNING: arch/powerpc/kernel/built-in.o(.text+0x2923c): Section mismatch
in reference from the function .prom_query_opal() to the
function .init.text:.call_prom()
The function .prom_query_opal() references
the function __init .call_prom().
This is often because .prom_query_opal lacks a __init
annotation or the annotation of .call_prom is wrong.
Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
---
arch/powerpc/kernel/prom_init.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/prom_init.c
b/arch/powerpc/kernel/prom_init.c
index 1b488e5..0794a30 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -1312,7 +1312,7 @@ static struct opal_secondary_data {
extern char opal_secondary_entry;
-static void prom_query_opal(void)
+static void __init prom_query_opal(void)
{
long rc;
@@ -1436,7 +1436,7 @@ static void __init prom_opal_hold_cpus(void)
prom_debug("prom_opal_hold_cpus: end...\n");
}
-static void prom_opal_takeover(void)
+static void __init prom_opal_takeover(void)
{
struct opal_secondary_data *data = &RELOC(opal_secondary_data);
struct opal_takeover_args *args = &data->args;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 5/5] powerpc: iommu: Implement IOMMU pools to improve multiqueue adapter performance
From: Michael Ellerman @ 2012-06-08 2:43 UTC (permalink / raw)
To: Anton Blanchard; +Cc: olof, paulus, linuxppc-dev
In-Reply-To: <20120604154519.48ea1aa9@kryten>
On Mon, 2012-06-04 at 15:45 +1000, Anton Blanchard wrote:
> At the moment all queues in a multiqueue adapter will serialise
> against the IOMMU table lock. This is proving to be a big issue,
> especially with 10Gbit ethernet.
..
> +
> struct iommu_table {
> unsigned long it_busno; /* Bus number this table belongs to */
> unsigned long it_size; /* Size of iommu table in entries */
> @@ -61,10 +71,10 @@ struct iommu_table {
> unsigned long it_index; /* which iommu table this is */
> unsigned long it_type; /* type: PCI or Virtual Bus */
> unsigned long it_blocksize; /* Entries in each block (cacheline) */
> - unsigned long it_hint; /* Hint for next alloc */
> - unsigned long it_largehint; /* Hint for large allocs */
> - unsigned long it_halfpoint; /* Breaking point for small/large allocs */
> - spinlock_t it_lock; /* Protects it_map */
> + unsigned long poolsize;
> + unsigned long nr_pools;
> + struct iommu_pool large_pool;
> + struct iommu_pool pools[IOMMU_NR_POOLS];
> unsigned long *it_map; /* A simple allocation bitmap for now */
> };
>
Breaks the cell code with:
arch/powerpc/platforms/cell/iommu.c:521:15: error: 'struct iommu_table' has no member named 'it_hint'
cheers
^ permalink raw reply
* Re: [PATCH] Disable /dev/port interface on powerpc systems
From: Michael Ellerman @ 2012-06-08 2:00 UTC (permalink / raw)
To: Haren Myneni; +Cc: linuxppc-dev, anton
In-Reply-To: <1332577383.19329.5.camel@hbabu-laptop>
On Sat, 2012-03-24 at 01:23 -0700, Haren Myneni wrote:
> Ben, Here it is the updated patch based on your suggestions. Please let
> me know if it has any issues.
>
This breaks the i386 defconfig build with:
drivers/char/mem.c:903:3: error: implicit declaration of function
'arch_has_dev_port' [-Werror=implicit-function-declaration]
I think because that file includes <asm/io.h> directly, not via
include/linux/io.h, so your stub is never seen.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: Optimise the 64bit optimised __clear_user
From: Benjamin Herrenschmidt @ 2012-06-08 0:51 UTC (permalink / raw)
To: Scott Wood
Cc: mikey, michael, Paul Mackerras, Anton Blanchard, olof,
linuxppc-dev
In-Reply-To: <4FD0EA08.7040903@freescale.com>
On Thu, 2012-06-07 at 12:51 -0500, Scott Wood wrote:
>
> The assembler can't tell that you used r0 rather than 0, because
> that's
> handled by the preprocessor, but it seems like a bug (or at least lax
> error checking) that it accepts %r0 there, and that objdump decodes it
> as "dcbz r0,r3" rather than "dcbz 0,r3".
Well, this is the domain of bike shed painting :-) the syntax of the
instruction is:
dcbz RA,RB
Now, of course, like other RA,RB pairs it has the semantic that
if RA = 0 then b <- 0
else b <- (RA)
EA <- b + (RB)
If we were to be pendantic and our assembly could also enforce the use
of % for registers, I suppose it would make sense to require 0 rather
than %0 for those "RA" forms to make it absolutely clear that we are
talking about 0 and not r0 in this specific case, I agree, but in the
current shape of the asm, I don't think that's something to expect.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] powerpc: Fix sldi to take literal not register name
From: Benjamin Herrenschmidt @ 2012-06-08 0:19 UTC (permalink / raw)
To: Michael Neuling
Cc: Michael Ellerman, Paul Mackerras, Anton Blanchard, olof,
linuxppc-dev
In-Reply-To: <9666.1339113149@neuling.org>
On Fri, 2012-06-08 at 09:52 +1000, Michael Neuling wrote:
> Fix sldi to take literal not register name.
>
> Was introduced by:
> commit 19ccb76a1938ab364a412253daec64613acbf3df
> Author: Paul Mackerras <paulus@samba.org>
> Date: Sat Jul 23 17:42:46 2011 +1000
>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---
> I got sick off the bike-shedding in a vacuum and started doing the
> conversion. Found this as a result.
>
> I'll post the full conversion at some point, but this fix is useful
> now.
Euh... except that:
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index a84aafc..969caeb 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -810,7 +810,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_201)
> lwz r3,VCORE_NAPPING_THREADS(r5)
> lwz r4,VCPU_PTID(r9)
> li r0,1
> - sldi r0,r0,r4
> + sldi r0,r0,4
Isn't the right fix s/sldi/sld and keep r4 ?
IE. I think Paul meant to shift 1 by the TID not shift 1 by 4
Cheers,
Ben.
> andc. r3,r3,r0 /* no sense IPI'ing ourselves */
> beq 43f
> mulli r4,r4,PACA_SIZE /* get paca for thread 0 */
^ permalink raw reply
* [PATCH] powerpc: Fix sldi to take literal not register name
From: Michael Neuling @ 2012-06-07 23:52 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Michael Ellerman, Anton Blanchard, olof, linuxppc-dev
In-Reply-To: <1339049882.2215.10.camel@concordia>
Fix sldi to take literal not register name.
Was introduced by:
commit 19ccb76a1938ab364a412253daec64613acbf3df
Author: Paul Mackerras <paulus@samba.org>
Date: Sat Jul 23 17:42:46 2011 +1000
Signed-off-by: Michael Neuling <mikey@neuling.org>
---
I got sick off the bike-shedding in a vacuum and started doing the
conversion. Found this as a result.
I'll post the full conversion at some point, but this fix is useful
now.
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index a84aafc..969caeb 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -810,7 +810,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_201)
lwz r3,VCORE_NAPPING_THREADS(r5)
lwz r4,VCPU_PTID(r9)
li r0,1
- sldi r0,r0,r4
+ sldi r0,r0,4
andc. r3,r3,r0 /* no sense IPI'ing ourselves */
beq 43f
mulli r4,r4,PACA_SIZE /* get paca for thread 0 */
^ permalink raw reply related
* 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 21:32 UTC (permalink / raw)
To: Ben Collins; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <489B8149-6E40-48B3-880F-EF8BE72E94BF@servergy.com>
On Thu, 2012-06-07 at 11:38 -0400, Ben Collins wrote:
> > Note that oddly, the second range of bus addresses looks -more-
> correct
> > than the first one...
>
> Except that the first one is exactly the same bus address as my bare
> metal system:
>
> pci_bus 0000:00: root bus resource [io 0xffbeb000-0xffbfafff] (bus
> address [0x100000000-0x10000ffff])
>
> I only have one PCIe RAID card on the bare metal system. Not
> surprising I never noticed the problem on it directly.
Can you show me the device-tree node for that PCI host bridge ?
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH v2 0/2] Add pcibios_device_change_notifier
From: Benjamin Herrenschmidt @ 2012-06-07 21:31 UTC (permalink / raw)
To: Hiroo Matsumoto
Cc: Bjorn Helgaas, linux-pci, linuxppc-dev, jbarnes, Kenji Kaneshige
In-Reply-To: <4FD0A257.4070506@jp.fujitsu.com>
On Thu, 2012-06-07 at 21:45 +0900, Hiroo Matsumoto wrote:
>
> > 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.
Unless I'm mistaken, this statement was removed from
pcibios_setup_bus_devices() recently so I'd say it's a leftover from
your patch having been originally done on top of an older kernel.
I won't be back for another week or so but I'll see if I can get things
tested in the meantime.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] powerpc: Optimise the 64bit optimised __clear_user
From: Scott Wood @ 2012-06-07 17:51 UTC (permalink / raw)
To: Paul Mackerras; +Cc: mikey, michael, Anton Blanchard, olof, linuxppc-dev
In-Reply-To: <20120607030423.GA30053@drongo>
On 06/06/2012 10:04 PM, Paul Mackerras wrote:
> 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.
The assembler can't tell that you used r0 rather than 0, because that's
handled by the preprocessor, but it seems like a bug (or at least lax
error checking) that it accepts %r0 there, and that objdump decodes it
as "dcbz r0,r3" rather than "dcbz 0,r3".
It's also odd that objdump produces output containing "r3" (not %r3)
when the assembler won't directly accept it.
-Scott
^ permalink raw reply
* unable to use X-fi pci-e sounbdboard on ppc64 ydl_powerstation
From: acrux @ 2012-06-07 17:16 UTC (permalink / raw)
To: linuxppc-dev
Unable to use X-fi pci-e sounbdboard on YDL Powerstation.
here other info:
https://bugtrack.alsa-project.org/alsa-bug/view.php?id=5521
tia,
--nico
^ 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-07 15:38 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <1339061428.24838.24.camel@pasglop>
On Jun 7, 2012, at 5:30 AM, Benjamin Herrenschmidt wrote:
> On Wed, 2012-06-06 at 19:35 -0400, Ben Collins wrote:
>>=20
>> pci_bus 0000:00: root bus resource [io 0xffbed000-0xffbfcfff] (bus
>> address [0x100000000-0x10000ffff])
>>=20
>> Without the fix that I sent, it ends up looking like:
>>=20
>> pci_bus 0000:00: root bus resource [io 0xffbed000-0xffbfcfff] (bus
>> address [0x0000-0xffff])
>>=20
>> And that's when some devices fail to be assigned valid bar 0's and =
the
>> kernel complains because of it.
>=20
> Note that oddly, the second range of bus addresses looks -more- =
correct
> than the first one...
Except that the first one is exactly the same bus address as my bare =
metal system:
pci_bus 0000:00: root bus resource [io 0xffbeb000-0xffbfafff] (bus =
address [0x100000000-0x10000ffff])
I only have one PCIe RAID card on the bare metal system. Not surprising =
I never noticed the problem on it directly.
> Cheers,
> Ben.
>=20
>=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 v3 1/2] powerpc/PCI: move DMA & IRQ init to device_add() notification path
From: Hiroo Matsumoto @ 2012-06-07 12:58 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Michal Simek, microblaze-uclinux, Kenji Kaneshige, Jesse Larrew,
jbarnes, Dominik Brodowski, linux-pci, linuxppc-dev
In-Reply-To: <CAErSpo6W28r4f5pJmZr=ofUAaffiUtOqgXWeabGpr+DmKw+Z+g@mail.gmail.com>
Thanks for your rapid modification about function name and comments.
As Ben said, I want to check that my patch does not cause problems.
I have no complain about my patch being in 3.6 branch (or lately).
I appreciate your time and effort.
Regards.
Hiroo MATSUMOTO
> On Thu, May 24, 2012 at 9:00 PM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
>> On Wed, 2012-05-23 at 16:37 -0600, Bjorn Helgaas wrote:
>>> From: Hiroo Matsumoto <matsumoto.hiroo@jp.fujitsu.com>
>>>
>>> PowerPC initialized DMA and IRQ information in the pci_scan_child_bus()
>>> -> pcibios_fixup_bus() path. Some hotplug drivers use that path, but
>>> others don't, e.g., pciehp, so sometimes hot-added devices are only
>>> partly initialized.
>>>
>>> This patch moves that initialization from pcibios_fixup_bus() to a new
>>> pci_bus_notify() called in the pci_bus_add_device() -> device_add() path.
>>> That means the initialization happens the same way for all PCI devices,
>>> whether they are present at boot or hot-added later.
>>>
>>> Note that powerpc was the only user of pci_fixup_cardbus(), which was
>>> used to do this same setup for cardbus devices. That's no longer
>>> needed because this setup will now be done in the same device_add()
>>> notification path as all other PCI devices.
>> Hrm. That will require a good deal of testing... Unfortunately I'm out
>> for a few weeks getting some surgery and then recovering...
>>
>> Our PCI code has ancient roots and I wouldn't be surprised if that
>> change breaks subtle assumptions made here or there, we'd need to test
>> at least on a good range of macs and ibm hotplug stuff.
>
> OK. Are you worried about cardbus in particular?
>
> This is headed for the 3.6, not 3.5, so we should have plenty of time.
> As soon as everything for the current merge window gets merged and
> -next is ready for the next batch, I'll put this in there.
>
>>> Typical failure of a hot-added e1000e device prior to this change:
>>>
>>> # echo 1 > /sys/bus/pci/slots/1/power
>>> <snip>
>>> e1000e 0000:03:00.0: enabling device (0000 -> 0002)
>>> e1000e 0000:03:00.0: No usable DMA configuration, aborting
>>> e1000e: probe of 0000:03:00.0 failed with error -5
>>>
>>> Successful initialization after this change:
>>>
>>> # echo 1 > /sys/bus/pci/slots/1/power
>>> <snip>
>>> e1000e 0000:03:00.0: enabling device (0000 -> 0002)
>>> irq: irq 6 on host /soc@ffe00000/msi@41600 mapped to virtual irq 27
>>> e1000e 0000:03:00.0: eth0: (PCI Express:2.5GT/s:Width x1) 00:15:17:bf:c0:c9
>>> e1000e 0000:03:00.0: eth0: Intel(R) PRO/1000 Network Connection
>>> e1000e 0000:03:00.0: eth0: MAC: 1, PHY: 4, PBA No: D50861-003
>>>
>>> [bhelgaas: changelog, notifier name, registration can be __init]
>>> CC: Dominik Brodowski <linux@dominikbrodowski.net>
>>> Signed-off-by: Hiroo MATSUMOTO <matsumoto.hiroo@jp.fujitsu.com>
>>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>>> ---
>>> arch/powerpc/include/asm/pci.h | 2 -
>>> arch/powerpc/kernel/pci-common.c | 87 ++++++++++++++--------------
>>> arch/powerpc/kernel/pci_32.c | 2 +
>>> arch/powerpc/kernel/pci_64.c | 2 +
>>> arch/powerpc/kernel/pci_of_scan.c | 1
>>> arch/powerpc/platforms/pseries/pci_dlpar.c | 1
>>> drivers/pci/pci.c | 5 --
>>> drivers/pcmcia/cardbus.c | 3 -
>>> include/linux/pci.h | 3 -
>>> 9 files changed, 49 insertions(+), 57 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
>>> index 6653f27..d6a36a4 100644
>>> --- a/arch/powerpc/include/asm/pci.h
>>> +++ b/arch/powerpc/include/asm/pci.h
>>> @@ -183,10 +183,10 @@ extern void pci_resource_to_user(const struct pci_dev *dev, int bar,
>>> resource_size_t *start, resource_size_t *end);
>>>
>>> extern resource_size_t pcibios_io_space_offset(struct pci_controller *hose);
>>> -extern void pcibios_setup_bus_devices(struct pci_bus *bus);
>>> extern void pcibios_setup_bus_self(struct pci_bus *bus);
>>> extern void pcibios_setup_phb_io_space(struct pci_controller *hose);
>>> extern void pcibios_scan_phb(struct pci_controller *hose);
>>> +extern void pcibios_setup_bus_notifier(void);
>>>
>>> #endif /* __KERNEL__ */
>>> #endif /* __ASM_POWERPC_PCI_H */
>>> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
>>> index 7320f36..41b39ba 100644
>>> --- a/arch/powerpc/kernel/pci-common.c
>>> +++ b/arch/powerpc/kernel/pci-common.c
>>> @@ -1009,40 +1009,6 @@ void __devinit pcibios_setup_bus_self(struct pci_bus *bus)
>>> ppc_md.pci_dma_bus_setup(bus);
>>> }
>>>
>>> -void __devinit pcibios_setup_bus_devices(struct pci_bus *bus)
>>> -{
>>> - struct pci_dev *dev;
>>> -
>>> - pr_debug("PCI: Fixup bus devices %d (%s)\n",
>>> - bus->number, bus->self ? pci_name(bus->self) : "PHB");
>>> -
>>> - list_for_each_entry(dev, &bus->devices, bus_list) {
>>> - /* Cardbus can call us to add new devices to a bus, so ignore
>>> - * those who are already fully discovered
>>> - */
>>> - if (dev->is_added)
>>> - continue;
>>> -
>>> - /* Fixup NUMA node as it may not be setup yet by the generic
>>> - * code and is needed by the DMA init
>>> - */
>>> - set_dev_node(&dev->dev, pcibus_to_node(dev->bus));
>>> -
>>> - /* Hook up default DMA ops */
>>> - set_dma_ops(&dev->dev, pci_dma_ops);
>>> - set_dma_offset(&dev->dev, PCI_DRAM_OFFSET);
>>> -
>>> - /* Additional platform DMA/iommu setup */
>>> - if (ppc_md.pci_dma_dev_setup)
>>> - ppc_md.pci_dma_dev_setup(dev);
>>> -
>>> - /* Read default IRQs and fixup if necessary */
>>> - pci_read_irq_line(dev);
>>> - if (ppc_md.pci_irq_fixup)
>>> - ppc_md.pci_irq_fixup(dev);
>>> - }
>>> -}
>>> -
>>> void pcibios_set_master(struct pci_dev *dev)
>>> {
>>> /* No special bus mastering setup handling */
>>> @@ -1059,19 +1025,9 @@ void __devinit pcibios_fixup_bus(struct pci_bus *bus)
>>>
>>> /* Now fixup the bus bus */
>>> pcibios_setup_bus_self(bus);
>>> -
>>> - /* Now fixup devices on that bus */
>>> - pcibios_setup_bus_devices(bus);
>>> }
>>> EXPORT_SYMBOL(pcibios_fixup_bus);
>>>
>>> -void __devinit pci_fixup_cardbus(struct pci_bus *bus)
>>> -{
>>> - /* Now fixup devices on that bus */
>>> - pcibios_setup_bus_devices(bus);
>>> -}
>>> -
>>> -
>>> static int skip_isa_ioresource_align(struct pci_dev *dev)
>>> {
>>> if (pci_has_flag(PCI_CAN_SKIP_ISA_ALIGN) &&
>>> @@ -1685,6 +1641,49 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
>>> }
>>> }
>>>
>>> +static int pci_bus_notify(struct notifier_block *nb, unsigned long action,
>>> + void *data)
>>> +{
>>> + struct pci_dev *dev = to_pci_dev(data);
>>> +
>>> + switch (action) {
>>> + case BUS_NOTIFY_ADD_DEVICE:
>>> + /* Setup OF node pointer in the device */
>>> + dev->dev.of_node = pci_device_to_OF_node(dev);
>>> +
>>> + /* Fixup NUMA node as it may not be setup yet by the generic
>>> + * code and is needed by the DMA init
>>> + */
>>> + set_dev_node(&dev->dev, pcibus_to_node(dev->bus));
>>> +
>>> + /* Hook up default DMA ops */
>>> + set_dma_ops(&dev->dev, pci_dma_ops);
>>> + set_dma_offset(&dev->dev, PCI_DRAM_OFFSET);
>>> +
>>> + /* Additional platform DMA/iommu setup */
>>> + if (ppc_md.pci_dma_dev_setup)
>>> + ppc_md.pci_dma_dev_setup(dev);
>>> +
>>> + /* Read default IRQs and fixup if necessary */
>>> + pci_read_irq_line(dev);
>>> + if (ppc_md.pci_irq_fixup)
>>> + ppc_md.pci_irq_fixup(dev);
>>> +
>>> + break;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static struct notifier_block device_nb = {
>>> + .notifier_call = pci_bus_notify,
>>> +};
>>> +
>>> +void __init pcibios_setup_bus_notifier(void)
>>> +{
>>> + bus_register_notifier(&pci_bus_type, &device_nb);
>>> +}
>>> +
>>> static void fixup_hide_host_resource_fsl(struct pci_dev *dev)
>>> {
>>> int i, class = dev->class >> 8;
>>> diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
>>> index 4b06ec5..640cc35 100644
>>> --- a/arch/powerpc/kernel/pci_32.c
>>> +++ b/arch/powerpc/kernel/pci_32.c
>>> @@ -231,6 +231,8 @@ static int __init pcibios_init(void)
>>>
>>> printk(KERN_INFO "PCI: Probing PCI hardware\n");
>>>
>>> + pcibios_setup_bus_notifier();
>>> +
>>> if (pci_has_flag(PCI_REASSIGN_ALL_BUS))
>>> pci_assign_all_buses = 1;
>>>
>>> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
>>> index 4ff190f..8b212d3 100644
>>> --- a/arch/powerpc/kernel/pci_64.c
>>> +++ b/arch/powerpc/kernel/pci_64.c
>>> @@ -48,6 +48,8 @@ static int __init pcibios_init(void)
>>>
>>> printk(KERN_INFO "PCI: Probing PCI hardware\n");
>>>
>>> + pcibios_setup_bus_notifier();
>>> +
>>> /* For now, override phys_mem_access_prot. If we need it,g
>>> * later, we may move that initialization to each ppc_md
>>> */
>>> diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
>>> index ae5ea5e..eb09eca 100644
>>> --- a/arch/powerpc/kernel/pci_of_scan.c
>>> +++ b/arch/powerpc/kernel/pci_of_scan.c
>>> @@ -333,7 +333,6 @@ static void __devinit __of_scan_bus(struct device_node *node,
>>> */
>>> if (!rescan_existing)
>>> pcibios_setup_bus_self(bus);
>>> - pcibios_setup_bus_devices(bus);
>>>
>>> /* Now scan child busses */
>>> list_for_each_entry(dev, &bus->devices, bus_list) {
>>> diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c
>>> index 3ccebc8..0b1b6b3 100644
>>> --- a/arch/powerpc/platforms/pseries/pci_dlpar.c
>>> +++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
>>> @@ -120,7 +120,6 @@ void pcibios_add_pci_devices(struct pci_bus * bus)
>>> num = pci_scan_slot(bus, PCI_DEVFN(slotno, 0));
>>> if (!num)
>>> return;
>>> - pcibios_setup_bus_devices(bus);
>>> max = bus->busn_res.start;
>>> for (pass=0; pass < 2; pass++)
>>> list_for_each_entry(dev, &bus->devices, bus_list) {
>>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>>> index 15d442a..43e0a4f 100644
>>> --- a/drivers/pci/pci.c
>>> +++ b/drivers/pci/pci.c
>>> @@ -3789,11 +3789,6 @@ int __attribute__ ((weak)) pci_ext_cfg_avail(struct pci_dev *dev)
>>> return 1;
>>> }
>>>
>>> -void __weak pci_fixup_cardbus(struct pci_bus *bus)
>>> -{
>>> -}
>>> -EXPORT_SYMBOL(pci_fixup_cardbus);
>>> -
>>> static int __init pci_setup(char *str)
>>> {
>>> while (str) {
>>> diff --git a/drivers/pcmcia/cardbus.c b/drivers/pcmcia/cardbus.c
>>> index 24caeaf..a980691 100644
>>> --- a/drivers/pcmcia/cardbus.c
>>> +++ b/drivers/pcmcia/cardbus.c
>>> @@ -71,7 +71,6 @@ int __ref cb_alloc(struct pcmcia_socket *s)
>>> unsigned int max, pass;
>>>
>>> s->functions = pci_scan_slot(bus, PCI_DEVFN(0, 0));
>>> - pci_fixup_cardbus(bus);
>>>
>>> max = bus->busn_res.start;
>>> for (pass = 0; pass < 2; pass++)
>>> @@ -85,7 +84,6 @@ int __ref cb_alloc(struct pcmcia_socket *s)
>>> */
>>> pci_bus_size_bridges(bus);
>>> pci_bus_assign_resources(bus);
>>> - cardbus_config_irq_and_cls(bus, s->pci_irq);
>>>
>>> /* socket specific tune function */
>>> if (s->tune_bridge)
>>> @@ -93,6 +91,7 @@ int __ref cb_alloc(struct pcmcia_socket *s)
>>>
>>> pci_enable_bridges(bus);
>>> pci_bus_add_devices(bus);
>>> + cardbus_config_irq_and_cls(bus, s->pci_irq);
>>>
>>> return 0;
>>> }
>>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>>> index a0e2d7f..3924c02 100644
>>> --- a/include/linux/pci.h
>>> +++ b/include/linux/pci.h
>>> @@ -651,9 +651,6 @@ resource_size_t pcibios_align_resource(void *, const struct resource *,
>>> resource_size_t);
>>> void pcibios_update_irq(struct pci_dev *, int irq);
>>>
>>> -/* Weak but can be overriden by arch */
>>> -void pci_fixup_cardbus(struct pci_bus *);
>>> -
>>> /* Generic PCI functions used internally */
>>>
>>> void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
>>
>
>
^ 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