* [PATCH 4/9] sh: Use clear_tasks_mm_cpumask()
From: Anton Vorontsov @ 2012-04-23 7:08 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: linaro-kernel, Mike Frysinger, Peter Zijlstra,
user-mode-linux-devel, linux-sh, Richard Weinberger, patches,
linux-kernel, uclinux-dist-devel, linux-mm, Paul Mundt,
John Stultz, KOSAKI Motohiro, Russell King, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20120423070641.GA27702@lizard>
Checking for process->mm is not enough because process' main
thread may exit or detach its mm via use_mm(), but other threads
may still have a valid mm.
To fix this we would need to use find_lock_task_mm(), which would
walk up all threads and returns an appropriate task (with task
lock held).
clear_tasks_mm_cpumask() has the issue fixed, so let's use it.
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/sh/kernel/smp.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/arch/sh/kernel/smp.c b/arch/sh/kernel/smp.c
index eaebdf6..4664f76 100644
--- a/arch/sh/kernel/smp.c
+++ b/arch/sh/kernel/smp.c
@@ -123,7 +123,6 @@ void native_play_dead(void)
int __cpu_disable(void)
{
unsigned int cpu = smp_processor_id();
- struct task_struct *p;
int ret;
ret = mp_ops->cpu_disable(cpu);
@@ -153,11 +152,7 @@ int __cpu_disable(void)
flush_cache_all();
local_flush_tlb_all();
- read_lock(&tasklist_lock);
- for_each_process(p)
- if (p->mm)
- cpumask_clear_cpu(cpu, mm_cpumask(p->mm));
- read_unlock(&tasklist_lock);
+ clear_tasks_mm_cpumask(cpu);
return 0;
}
--
1.7.9.2
^ permalink raw reply related
* [PATCH 3/9] powerpc: Use clear_tasks_mm_cpumask()
From: Anton Vorontsov @ 2012-04-23 7:08 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: linaro-kernel, Mike Frysinger, Peter Zijlstra,
user-mode-linux-devel, linux-sh, Richard Weinberger, patches,
linux-kernel, uclinux-dist-devel, linux-mm, Paul Mundt,
John Stultz, KOSAKI Motohiro, Russell King, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20120423070641.GA27702@lizard>
Current CPU hotplug code has some task->mm handling issues:
1. Working with task->mm w/o getting mm or grabing the task lock is
dangerous as ->mm might disappear (exit_mm() assigns NULL under
task_lock(), so tasklist lock is not enough).
We can't use get_task_mm()/mmput() pair as mmput() might sleep,
so we must take the task lock while handle its mm.
2. Checking for process->mm is not enough because process' main
thread may exit or detach its mm via use_mm(), but other threads
may still have a valid mm.
To fix this we would need to use find_lock_task_mm(), which would
walk up all threads and returns an appropriate task (with task
lock held).
clear_tasks_mm_cpumask() has all the issues fixed, so let's use it.
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/powerpc/mm/mmu_context_nohash.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c
index 5b63bd3..e779642 100644
--- a/arch/powerpc/mm/mmu_context_nohash.c
+++ b/arch/powerpc/mm/mmu_context_nohash.c
@@ -333,9 +333,7 @@ static int __cpuinit mmu_context_cpu_notify(struct notifier_block *self,
unsigned long action, void *hcpu)
{
unsigned int cpu = (unsigned int)(long)hcpu;
-#ifdef CONFIG_HOTPLUG_CPU
- struct task_struct *p;
-#endif
+
/* We don't touch CPU 0 map, it's allocated at aboot and kept
* around forever
*/
@@ -358,12 +356,7 @@ static int __cpuinit mmu_context_cpu_notify(struct notifier_block *self,
stale_map[cpu] = NULL;
/* We also clear the cpu_vm_mask bits of CPUs going away */
- read_lock(&tasklist_lock);
- for_each_process(p) {
- if (p->mm)
- cpumask_clear_cpu(cpu, mm_cpumask(p->mm));
- }
- read_unlock(&tasklist_lock);
+ clear_tasks_mm_cpumask(cpu);
break;
#endif /* CONFIG_HOTPLUG_CPU */
}
--
1.7.9.2
^ permalink raw reply related
* [PATCH 2/9] arm: Use clear_tasks_mm_cpumask()
From: Anton Vorontsov @ 2012-04-23 7:08 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: linaro-kernel, Mike Frysinger, Peter Zijlstra,
user-mode-linux-devel, linux-sh, Richard Weinberger, patches,
linux-kernel, uclinux-dist-devel, linux-mm, Paul Mundt,
John Stultz, KOSAKI Motohiro, Russell King, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20120423070641.GA27702@lizard>
Checking for process->mm is not enough because process' main
thread may exit or detach its mm via use_mm(), but other threads
may still have a valid mm.
To fix this we would need to use find_lock_task_mm(), which would
walk up all threads and returns an appropriate task (with task
lock held).
clear_tasks_mm_cpumask() has this issue fixed, so let's use it.
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
arch/arm/kernel/smp.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index addbbe8..e824ee4 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -130,7 +130,6 @@ static void percpu_timer_stop(void);
int __cpu_disable(void)
{
unsigned int cpu = smp_processor_id();
- struct task_struct *p;
int ret;
ret = platform_cpu_disable(cpu);
@@ -160,12 +159,7 @@ int __cpu_disable(void)
flush_cache_all();
local_flush_tlb_all();
- read_lock(&tasklist_lock);
- for_each_process(p) {
- if (p->mm)
- cpumask_clear_cpu(cpu, mm_cpumask(p->mm));
- }
- read_unlock(&tasklist_lock);
+ clear_tasks_mm_cpumask(cpu);
return 0;
}
--
1.7.9.2
^ permalink raw reply related
* [PATCH 1/9] cpu: Introduce clear_tasks_mm_cpumask() helper
From: Anton Vorontsov @ 2012-04-23 7:07 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: linaro-kernel, Mike Frysinger, Peter Zijlstra,
user-mode-linux-devel, linux-sh, Richard Weinberger, patches,
linux-kernel, uclinux-dist-devel, linux-mm, Paul Mundt,
John Stultz, KOSAKI Motohiro, Russell King, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20120423070641.GA27702@lizard>
Many architectures clear tasks' mm_cpumask like this:
read_lock(&tasklist_lock);
for_each_process(p) {
if (p->mm)
cpumask_clear_cpu(cpu, mm_cpumask(p->mm));
}
read_unlock(&tasklist_lock);
Depending on the context, the code above may have several problems,
such as:
1. Working with task->mm w/o getting mm or grabing the task lock is
dangerous as ->mm might disappear (exit_mm() assigns NULL under
task_lock(), so tasklist lock is not enough).
2. Checking for process->mm is not enough because process' main
thread may exit or detach its mm via use_mm(), but other threads
may still have a valid mm.
This patch implements a small helper function that does things
correctly, i.e.:
1. We take the task's lock while whe handle its mm (we can't use
get_task_mm()/mmput() pair as mmput() might sleep);
2. To catch exited main thread case, we use find_lock_task_mm(),
which walks up all threads and returns an appropriate task
(with task lock held).
Also, Per Peter Zijlstra's idea, now we don't grab tasklist_lock in
the new helper, instead we take the rcu read lock. We can do this
because the function is called after the cpu is taken down and marked
offline, so no new tasks will get this cpu set in their mm mask.
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
include/linux/cpu.h | 1 +
kernel/cpu.c | 26 ++++++++++++++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index ee28844..d2ca49f 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -179,6 +179,7 @@ extern void put_online_cpus(void);
#define hotcpu_notifier(fn, pri) cpu_notifier(fn, pri)
#define register_hotcpu_notifier(nb) register_cpu_notifier(nb)
#define unregister_hotcpu_notifier(nb) unregister_cpu_notifier(nb)
+void clear_tasks_mm_cpumask(int cpu);
int cpu_down(unsigned int cpu);
#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 2060c6e..ecdf499 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -10,6 +10,8 @@
#include <linux/sched.h>
#include <linux/unistd.h>
#include <linux/cpu.h>
+#include <linux/oom.h>
+#include <linux/rcupdate.h>
#include <linux/export.h>
#include <linux/kthread.h>
#include <linux/stop_machine.h>
@@ -171,6 +173,30 @@ void __ref unregister_cpu_notifier(struct notifier_block *nb)
}
EXPORT_SYMBOL(unregister_cpu_notifier);
+void clear_tasks_mm_cpumask(int cpu)
+{
+ struct task_struct *p;
+
+ /*
+ * This function is called after the cpu is taken down and marked
+ * offline, so its not like new tasks will ever get this cpu set in
+ * their mm mask. -- Peter Zijlstra
+ * Thus, we may use rcu_read_lock() here, instead of grabbing
+ * full-fledged tasklist_lock.
+ */
+ rcu_read_lock();
+ for_each_process(p) {
+ struct task_struct *t;
+
+ t = find_lock_task_mm(p);
+ if (!t)
+ continue;
+ cpumask_clear_cpu(cpu, mm_cpumask(t->mm));
+ task_unlock(t);
+ }
+ rcu_read_unlock();
+}
+
static inline void check_for_tasks(int cpu)
{
struct task_struct *p;
--
1.7.9.2
^ permalink raw reply related
* [PATCH v3 0/9] Fixes for common mistakes w/ for_each_process and task->mm
From: Anton Vorontsov @ 2012-04-23 7:06 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov
Cc: linaro-kernel, Mike Frysinger, Peter Zijlstra,
user-mode-linux-devel, linux-sh, Richard Weinberger, patches,
linux-kernel, uclinux-dist-devel, linux-mm, Paul Mundt,
John Stultz, KOSAKI Motohiro, Russell King, linuxppc-dev,
linux-arm-kernel
Hi all,
This is another resend of several task->mm fixes, the bugs I found
during LMK code audit. Architectures were traverse the tasklist
in an unsafe manner, plus there are a few cases of unsafe access to
task->mm in general.
There were no objections on the previous resend, and the final words
were somewhere along "the patches are fine" line.
In v3:
- Dropped a controversal 'Make find_lock_task_mm() sparse-aware' patch;
- Reword arm and sh commit messages, per Oleg Nesterov's suggestions;
- Added an optimization trick in clear_tasks_mm_cpumask(): take only
the rcu read lock, no need for the whole tasklist_lock.
Suggested by Peter Zijlstra.
In v2:
- introduced a small helper in cpu.c: most arches duplicate the
same [buggy] code snippet, so it's better to fix it and move the
logic into a common function.
Thanks,
--
Anton Vorontsov
Email: cbouatmailru@gmail.com
^ permalink raw reply
* RE: [RFC] powerpc/fsl-pci: Document the "fsl,has-isa" property for Freescale PCI
From: Jia Hongtao-B38951 @ 2012-04-23 3:34 UTC (permalink / raw)
To: Jia Hongtao-B38951, Kumar Gala, Benjamin Herrenschmidt
Cc: devicetree-discuss@lists.ozlabs.org,
linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <412C8208B4A0464FA894C5F0C278CD5D019DEA96@039-SN1MPN1-002.039d.mgd.msft.net>
> -----Original Message-----
> From: Jia Hongtao-B38951
> Sent: Tuesday, April 10, 2012 5:17 PM
> To: Kumar Gala
> Cc: devicetree-discuss@lists.ozlabs.org; linuxppc-dev@lists.ozlabs.org;
> Li Yang-R58472; Jia Hongtao-B38951
> Subject: RE: [RFC] powerpc/fsl-pci: Document the "fsl,has-isa" property
> for Freescale PCI
>=20
>=20
> > -----Original Message-----
> > From: linuxppc-dev-bounces+b38951=3Dfreescale.com@lists.ozlabs.org
> > [mailto:linuxppc-dev-bounces+b38951=3Dfreescale.com@lists.ozlabs.org] O=
n
> > Behalf Of Jia Hongtao-B38951
> > Sent: Friday, April 06, 2012 11:05 AM
> > To: Kumar Gala
> > Cc: devicetree-discuss@lists.ozlabs.org; linuxppc-dev@lists.ozlabs.org;
> > Li Yang-R58472
> > Subject: RE: [RFC] powerpc/fsl-pci: Document the "fsl,has-isa" property
> > for Freescale PCI
> >
> >
> > > -----Original Message-----
> > > From: Kumar Gala [mailto:galak@kernel.crashing.org]
> > > Sent: Wednesday, April 04, 2012 9:09 PM
> > > To: Jia Hongtao-B38951
> > > Cc: linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; devicetree-
> > > discuss@lists.ozlabs.org
> > > Subject: Re: [RFC] powerpc/fsl-pci: Document the "fsl,has-isa"
> property
> > > for Freescale PCI
> > >
> > >
> > > On Apr 1, 2012, at 1:56 AM, Jia Hongtao wrote:
> > >
> > > > If PCI is primary bus we should set isa_io/mem_base when parsing
> PCI
> > > bridge
> > > > resources from device tree. The previous way to check the primary
> bus
> > > based
> > > > on a hard-coded address named primary_phb_addr. Now we add a
> property
> > > named
> > > > "fsl,has-isa" into device tree. In kernel we use this property to
> > find
> > > out
> > > > the bus is primary or not. This way is more flexible.
> > > >
> > > > Signed-off-by: Jia Hongtao <B38951@freescale.com>
> > > > Signed-off-by: Li Yang <leoli@freescale.com>
> > > > ---
> > > > .../devicetree/bindings/powerpc/fsl/pci.txt | 36
> > > ++++++++++++++++++++
> > > > 1 files changed, 36 insertions(+), 0 deletions(-)
> > > > create mode 100644
> > > Documentation/devicetree/bindings/powerpc/fsl/pci.txt
> > >
> > > This isn't freescale specific, its linux specific. If anything the
> > > property should be linux,has-isa.
> > >
> > > But in general I dont think this is a good idea. In truth one could
> > > search the device tree for:
> > >
> > > device_type =3D "isa";
> > >
> > > to try and set this dynamically.
> > >
> > > - k
> > >
> >
> > Yes, it's not Freescale specific, but it's only used by Freescale now
> in
> > the kernel. This is why I named the property as "fsl,has-isa".
> >
> > To indicate PCI bus is primary we have three ways to go and we now like
> > the 2nd solution:
> >
> > 1. As this patch said, we add a property to device tree manually.
> >
> > 2. Set this property dynamically in uboot when scanning PCI bridge.
> > Actually we have already done this. The problem is users should update
> > uboot and kernel together or it's not work. To support old uboot we
> > decide
> > to add this property into device tree too temporarily. We will remove
> it
> > from device tree at an appropriate future.
> >
> > 3. Just as you said we could search the device tree by device_type =3D
> > "isa".
> > There are two problems here:
> > * There is no OF API for searching just in PCI node now. That means
> > we can't easily find whether there is "isa" bridge or not under
> > this PCI controller while scanning it.
> > * Boards MPC8541CDS and MPC8555CDS has no "isa" node in device tree
> > but has ISA bridge under PCI controller.
> >
> > - Jia Hongtao
> >
>=20
> Hi Kumar,
>=20
> I agree with the name change from "fsl,has-isa" to "linux,has-isa".
> If this is ok I will update this patch and send the patches based on this
> new property.
>=20
> Also, I have a question about "isa" node in dts. Is this node being used
> by kernel? If not we can remove it from all boards and check the primary
> bus by uboot. If "isa" is useful why MPC8541CDS and MPC8555CDS has no
> "isa" node in device tree but has ISA bridge under PCI controller?
>=20
> Thanks.
> - Jia Hongtao
Hi Kumar and Ben,
Do you have any idea on this?
Thanks.
- Jia Hongtao
^ permalink raw reply
* [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2012-04-23 2:04 UTC (permalink / raw)
To: Linus Torvalds
Cc: linuxppc-dev list, Andrew Morton, Thomas Gleixner,
Linux Kernel list
Hi Linus !
Here are a few fixes for powerpc. Note the addition to the generic
irq.h. This is part of a 3-patches regression fix for mpic due to
changes in how IRQ_TYPE_NONE is being handled. Thomas agreed to the
addition of the new IRQ_TYPE_DEFAULT contant, however he hasn't
replied with an Ack to the actual patch yet. I don't to wait much
longer with these patches tho.
Cheers,
Ben.
The following changes since commit 932e9f352b5d685725076f21b237f7c7d804b29c:
Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security (2012-04-18 20:16:02 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge
for you to fetch changes up to 446f6d06fab0b49c61887ecbe8286d6aaa796637:
powerpc/mpic: Properly set default triggers (2012-04-23 11:04:30 +1000)
----------------------------------------------------------------
Baruch Siach (1):
powerpc: fix build when CONFIG_BOOKE_WDT is enabled
Benjamin Herrenschmidt (5):
Merge remote-tracking branch 'kumar/merge' into merge
powerpc/pmac: Don't add_timer() twice
powerpc/mpic: Fix confusion between hw_irq and virq
irq: Add IRQ_TYPE_DEFAULT for use by PIC drivers
powerpc/mpic: Properly set default triggers
Gavin Shan (1):
powerpc/eeh: Fix crash caused by null eeh_dev
Mingkai Hu (4):
powerpc/mpic_msgr: fix compile error when SMP disabled
powerpc/mpic_msgr: add lock for MPIC message global variable
powerpc/mpic_msgr: fix offset error when setting mer register
powerpc/mpc85xx: add MPIC message dts node
Timur Tabi (1):
powerpc/85xx: don't call of_platform_bus_probe() twice
arch/powerpc/boot/dts/fsl/pq3-mpic-message-B.dtsi | 43 ++++++++++++++++
arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi | 10 ++++
arch/powerpc/include/asm/mpic.h | 18 -------
arch/powerpc/include/asm/mpic_msgr.h | 1 +
arch/powerpc/include/asm/reg_booke.h | 5 --
arch/powerpc/kernel/setup_32.c | 3 ++
arch/powerpc/platforms/85xx/common.c | 6 +++
arch/powerpc/platforms/85xx/mpc85xx_mds.c | 11 +----
arch/powerpc/platforms/85xx/p1022_ds.c | 13 +----
arch/powerpc/platforms/powermac/low_i2c.c | 9 ++++
arch/powerpc/platforms/pseries/eeh.c | 2 +-
arch/powerpc/sysdev/mpic.c | 54 +++++++++++++--------
arch/powerpc/sysdev/mpic_msgr.c | 12 ++---
include/linux/irq.h | 7 +++
14 files changed, 122 insertions(+), 72 deletions(-)
create mode 100644 arch/powerpc/boot/dts/fsl/pq3-mpic-message-B.dtsi
^ permalink raw reply
* Re: [PATCH] gianfar: add GRO support
From: David Miller @ 2012-04-21 20:44 UTC (permalink / raw)
To: b06378; +Cc: netdev, linuxppc-dev
In-Reply-To: <1334912075-27073-1-git-send-email-b06378@freescale.com>
From: Jiajun Wu <b06378@freescale.com>
Date: Fri, 20 Apr 2012 16:54:35 +0800
> Replace netif_receive_skb with napi_gro_receive.
>
> Signed-off-by: Jiajun Wu <b06378@freescale.com>
Applied.
^ permalink raw reply
* Re: [PATCH] driver/mtd: IFC NAND: Add support of ONFI NAND flash
From: Artem Bityutskiy @ 2012-04-21 17:39 UTC (permalink / raw)
To: Prabhakar Kushwaha; +Cc: scottwood, linux-mtd, linuxppc-dev
In-Reply-To: <1333949122-10401-1-git-send-email-prabhakar@freescale.com>
On Mon, 2012-04-09 at 10:55 +0530, Prabhakar Kushwaha wrote:
> - Fix NAND_CMD_READID command for ONFI detect.
> - Add NAND_CMD_PARAM command to read the ONFI parameter page.
>
> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
> Acked-by: Scott Wood <scottwood@freescale.com>
> ---
> Based upon git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> Branch master
Pushed to l2-mtd.git, thanks!
--
Best Regards,
Artem Bityutskiy
^ permalink raw reply
* Re: pci node question
From: Benjamin Herrenschmidt @ 2012-04-20 21:24 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4F91CCCA.1050204@freescale.com>
> Between the root complex and whatever's plugged in?
Yes.
> > so that is what the node represents. Its always been a bit confusing to me as its not 100% standardized by PCI sig.
It's absolutely standard. The only case where you have "siblings" to
that RC is when it's some kind of integrated chipset with non-PCI
devices in it (still common in Intel world).
Any real PCIe device -must- have a P2P above it with the PCIe capability
& associated control registers.
> Is it documented anywhere (e.g. in the chip manual)? Is it common (even
> if 100% standardized), or a Freescale-ism?
PCIe spec.
> Is there an actual PCIe-to-PCIe bridge that shows up in the root
> complex's config space
Yes.
> (not talking about the host bridge PCI device
> that has always been there on FSL PCI controllers, which we didn't
> represent in the device tree on non-express PCI)? Why does this bridge
> need to be represented in the device tree (assuming no ISA devices need
> to be represented), when other PCI devices (including bridges) don't?
You don't have to, but we sometimes do it so we can put the
interrupt-map in the right place. But again, since on PCIe the device
underneath should always have dev 0 for non-SRIOV stuff, the swizzling
shall be NOP and so having the interrupt-map all the way at the top
might work. However I'm not sure if that will work with PFs and ARI
where the dev is non-0.
> > Maybe Ben's got some comments to add here from a generic PCIe point of view.
> >
> >>>> Do we really need the error interrupt specified twice?
> >>>
> >>> I put it twice because it has multiple purposes, one has to do with interrupts defined by the PCI spec vs ones defined via FSL controller.
> >>
> >> There are PCI-defined error conditions that cause a host controller
> >> interrupt. What does this have to do with the bridge node?
> >
> > Think of the "error" IRQ as shared between to classes of interrupts. One set are controller errors defined by FSL, the other are errors defined by PCI sig / bus point of view.
> >
> > I'd expect controller errors to be handled by something like EDAC would bind at "fsl,qoriq-pcie-v2.2" level node. The PCI bus code would looks at the virtual bridge down.
>
> This shouldn't be about where a specific piece of Linux code looks.
>
> I don't see why the split of PCI-defined errors versus FSL-defined
> errors maps to bridge node versus controller node. What if we didn't
> have the bridge? We'd still have PCI-defined errors, right?
The linux generic PCIe port driver looks for the interrupt of the bridge
for standardized PCIe AER interrupts.
Cheers,
Ben.
> -Scott
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: pci node question
From: Benjamin Herrenschmidt @ 2012-04-20 21:20 UTC (permalink / raw)
To: Kumar Gala; +Cc: Scott Wood, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <EDA55B32-FD14-4B5C-90AF-7FC5F77898E0@kernel.crashing.org>
On Fri, 2012-04-20 at 15:37 -0500, Kumar Gala wrote:
> > What does this node represent in the first place? Is there really a
> > PCI-to-PCI bridge? Are all other PCI devices underneath it?
>
> PCIe has this concept of a "virtual" bridge between the root-comples,
> so that is what the node represents. Its always been a bit confusing
> to me as its not 100% standardized by PCI sig.
It actually is. The root complex virtual p2p is absolutely part of the
standard.
Cheers,
Ben.
^ permalink raw reply
* Re: pci node question
From: Benjamin Herrenschmidt @ 2012-04-20 21:19 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4F91B335.4010501@freescale.com>
On Fri, 2012-04-20 at 14:04 -0500, Scott Wood wrote:
> That's not supposed to be how device tree bindings are determined.
Ugh ? This is nothing to do with Linux, I think Kumar is confused :-)
This has to do with PCI bindings.
If you put the interrupt-map in the parent, then crossing the virtual
p2p will cause a swizzling which is not what you want. As long as the
device has a device number (in devfn) of 0 that's fine but that may not
always be the case.
> It's causing us problems with virtualization device assignment, because
> if we just assign the parent bus we don't get the interrupt map, but if
> we assign the child we need to deal with what it means to assign an
> individual PCI device (e.g. on our internal KVM stuff we get an error on
> that reg property).
I'm not sure what you are doing with device-assignement in KVM, we are
doing something different for server I suppose since the existing
assignment code in qemu-kvm is a dead end... But you should probably
synthetize an interrupt-map for the guest.
> What does this node represent in the first place? Is there really a
> PCI-to-PCI bridge? Are all other PCI devices underneath it?
Yes, PCIe 101 :-) It's the root complex, it's "virtual" in that it's a
construct of the host bridge, there's no physical "bridge" in the
system, but yes, PCIe always has a virtual P2P at the top to represent
the root complex.
This was done to fix the long standing problem that there wasn't a
proper way to represent host bridges as parent of their devices in PCI
land.
It allows PCIe to guarantee that a device always has a bridge above
itself, with the corresponding link control registers etc... which is
useful for point to point links :-)
That's also why for example PCIe switches look like stacks of bridges,
for example, a 2 fork switch looks like:
|
P2P
/ \
P2P P2P
| |
That way each downstream device gets its own parent P2P with the
corresponding PCIe link control registers etc...
> >> Do we really need the error interrupt specified twice?
> >
> > I put it twice because it has multiple purposes, one has to do with interrupts defined by the PCI spec vs ones defined via FSL controller.
>
> There are PCI-defined error condition s that cause a host controller
> interrupt. What does this have to do with the bridge node?
>
> >> Why is there a zeroed out reg property: reg = <0 0 0 0 0> ??
> >
> > scratching my head, what happens if you remove it?
>
> Sigh.
As I said earlier, this is not some black magic, it's a proper reg value
for the root complex virtual bridge. It has bus 0, devfn 0, so the reg
property for the config space has a value of 0.
Without it, the kernel won't properly match it to the corresponding
pci_dev.
Cheers,
Ben.
> -Scott
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: pci node question
From: Benjamin Herrenschmidt @ 2012-04-20 21:11 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <D04C6167-5286-4675-A6D8-32BA63305424@kernel.crashing.org>
On Fri, 2012-04-20 at 13:53 -0500, Kumar Gala wrote:
> On Apr 20, 2012, at 1:37 PM, Yoder Stuart-B08248 wrote:
>
> > There was refactoring change a while back that moved
> > the interrupt map down into the virtual pci bridge.
> >
> > example:
> > 42 /* controller at 0x200000 */
> > 43 &pci0 {
> > 44 compatible = "fsl,p2041-pcie", "fsl,qoriq-pcie-v2.2";
> > 45 device_type = "pci";
> > 46 #size-cells = <2>;
> > 47 #address-cells = <3>;
> > 48 bus-range = <0x0 0xff>;
> > 49 clock-frequency = <33333333>;
> > 50 interrupts = <16 2 1 15>;
> > 51 pcie@0 {
> > 52 reg = <0 0 0 0 0>;
> > 53 #interrupt-cells = <1>;
> > 54 #size-cells = <2>;
> > 55 #address-cells = <3>;
> > 56 device_type = "pci";
> > 57 interrupts = <16 2 1 15>;
> > 58 interrupt-map-mask = <0xf800 0 0 7>;
> > 59 interrupt-map = <
> > 60 /* IDSEL 0x0 */
> > 61 0000 0 0 1 &mpic 40 1 0 0
> > 62 0000 0 0 2 &mpic 1 1 0 0
> > 63 0000 0 0 3 &mpic 2 1 0 0
> > 64 0000 0 0 4 &mpic 3 1 0 0
> > 65 >;
> > 66 };
> > 67 };
> >
> > Why was the interrupt-map moved here?
>
> Its been a while, but I think i moved it down because of which node is used for interrupt handling in linux.
Yeah, it would swizzle if going accross the virtual P2P bridge. On the
other hand, if it's PCIe, the children of the vP2P should always be at
device 0 and thus the swizzling should be a NOP no ? So we -could- leave
it in the PHB unless I'm mistaken...
On the other hand, we probably want to fix the mapping code to handle
things like SR-IOV PFs with different device numbers... do those get
swizzled ? In that case we might want to keep things down the virtual
bridge.
> > Do we really need the error interrupt specified twice?
>
> I put it twice because it has multiple purposes, one has to do with interrupts defined by the PCI spec vs ones defined via FSL controller.
>
> > Why is there a zeroed out reg property: reg = <0 0 0 0 0> ??
>
> scratching my head, what happens if you remove it?
If you remove it, the kernel will fail to match the vP2P with the
corresponding struct pci_dev.... It's not "zeroed out". It contains a
valid bus/dev/fn ... of 0,0,0 :-)
Cheers,
Ben.
> - k
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: pci node question
From: Scott Wood @ 2012-04-20 20:53 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <EDA55B32-FD14-4B5C-90AF-7FC5F77898E0@kernel.crashing.org>
On 04/20/2012 03:37 PM, Kumar Gala wrote:
>
> On Apr 20, 2012, at 2:04 PM, Scott Wood wrote:
>
>> On 04/20/2012 01:53 PM, Kumar Gala wrote:
>>>
>>> On Apr 20, 2012, at 1:37 PM, Yoder Stuart-B08248 wrote:
>>>
>>>> There was refactoring change a while back that moved
>>>> the interrupt map down into the virtual pci bridge.
>>>>
>>>> example:
>>>> 42 /* controller at 0x200000 */
>>>> 43 &pci0 {
>>>> 44 compatible = "fsl,p2041-pcie", "fsl,qoriq-pcie-v2.2";
>>>> 45 device_type = "pci";
>>>> 46 #size-cells = <2>;
>>>> 47 #address-cells = <3>;
>>>> 48 bus-range = <0x0 0xff>;
>>>> 49 clock-frequency = <33333333>;
>>>> 50 interrupts = <16 2 1 15>;
>>>> 51 pcie@0 {
>>>> 52 reg = <0 0 0 0 0>;
>>>> 53 #interrupt-cells = <1>;
>>>> 54 #size-cells = <2>;
>>>> 55 #address-cells = <3>;
>>>> 56 device_type = "pci";
>>>> 57 interrupts = <16 2 1 15>;
>>>> 58 interrupt-map-mask = <0xf800 0 0 7>;
>>>> 59 interrupt-map = <
>>>> 60 /* IDSEL 0x0 */
>>>> 61 0000 0 0 1 &mpic 40 1 0 0
>>>> 62 0000 0 0 2 &mpic 1 1 0 0
>>>> 63 0000 0 0 3 &mpic 2 1 0 0
>>>> 64 0000 0 0 4 &mpic 3 1 0 0
>>>> 65 >;
>>>> 66 };
>>>> 67 };
>>>>
>>>> Why was the interrupt-map moved here?
>>>
>>> Its been a while, but I think i moved it down because of which node is used for interrupt handling in linux.
>>
>> That's not supposed to be how device tree bindings are determined.
>>
>> It's causing us problems with virtualization device assignment, because
>> if we just assign the parent bus we don't get the interrupt map, but if
>> we assign the child we need to deal with what it means to assign an
>> individual PCI device (e.g. on our internal KVM stuff we get an error on
>> that reg property).
>>
>> What does this node represent in the first place? Is there really a
>> PCI-to-PCI bridge? Are all other PCI devices underneath it?
>
> PCIe has this concept of a "virtual" bridge between the root-comples,
Between the root complex and whatever's plugged in?
> so that is what the node represents. Its always been a bit confusing to me as its not 100% standardized by PCI sig.
Is it documented anywhere (e.g. in the chip manual)? Is it common (even
if 100% standardized), or a Freescale-ism?
Is there an actual PCIe-to-PCIe bridge that shows up in the root
complex's config space (not talking about the host bridge PCI device
that has always been there on FSL PCI controllers, which we didn't
represent in the device tree on non-express PCI)? Why does this bridge
need to be represented in the device tree (assuming no ISA devices need
to be represented), when other PCI devices (including bridges) don't?
> Maybe Ben's got some comments to add here from a generic PCIe point of view.
>
>>>> Do we really need the error interrupt specified twice?
>>>
>>> I put it twice because it has multiple purposes, one has to do with interrupts defined by the PCI spec vs ones defined via FSL controller.
>>
>> There are PCI-defined error conditions that cause a host controller
>> interrupt. What does this have to do with the bridge node?
>
> Think of the "error" IRQ as shared between to classes of interrupts. One set are controller errors defined by FSL, the other are errors defined by PCI sig / bus point of view.
>
> I'd expect controller errors to be handled by something like EDAC would bind at "fsl,qoriq-pcie-v2.2" level node. The PCI bus code would looks at the virtual bridge down.
This shouldn't be about where a specific piece of Linux code looks.
I don't see why the split of PCI-defined errors versus FSL-defined
errors maps to bridge node versus controller node. What if we didn't
have the bridge? We'd still have PCI-defined errors, right?
-Scott
^ permalink raw reply
* Re: pci node question
From: Kumar Gala @ 2012-04-20 20:37 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4F91B335.4010501@freescale.com>
On Apr 20, 2012, at 2:04 PM, Scott Wood wrote:
> On 04/20/2012 01:53 PM, Kumar Gala wrote:
>>=20
>> On Apr 20, 2012, at 1:37 PM, Yoder Stuart-B08248 wrote:
>>=20
>>> There was refactoring change a while back that moved=20
>>> the interrupt map down into the virtual pci bridge.
>>>=20
>>> example:=20
>>> 42 /* controller at 0x200000 */
>>> 43 &pci0 {
>>> 44 compatible =3D "fsl,p2041-pcie", "fsl,qoriq-pcie-v2.2";
>>> 45 device_type =3D "pci";
>>> 46 #size-cells =3D <2>;
>>> 47 #address-cells =3D <3>;
>>> 48 bus-range =3D <0x0 0xff>;
>>> 49 clock-frequency =3D <33333333>;
>>> 50 interrupts =3D <16 2 1 15>;
>>> 51 pcie@0 {
>>> 52 reg =3D <0 0 0 0 0>;
>>> 53 #interrupt-cells =3D <1>;
>>> 54 #size-cells =3D <2>;
>>> 55 #address-cells =3D <3>;
>>> 56 device_type =3D "pci";
>>> 57 interrupts =3D <16 2 1 15>;
>>> 58 interrupt-map-mask =3D <0xf800 0 0 7>;
>>> 59 interrupt-map =3D <
>>> 60 /* IDSEL 0x0 */
>>> 61 0000 0 0 1 &mpic 40 1 0 0
>>> 62 0000 0 0 2 &mpic 1 1 0 0
>>> 63 0000 0 0 3 &mpic 2 1 0 0
>>> 64 0000 0 0 4 &mpic 3 1 0 0
>>> 65 >;
>>> 66 };
>>> 67 };
>>>=20
>>> Why was the interrupt-map moved here?
>>=20
>> Its been a while, but I think i moved it down because of which node =
is used for interrupt handling in linux.
>=20
> That's not supposed to be how device tree bindings are determined.
>=20
> It's causing us problems with virtualization device assignment, =
because
> if we just assign the parent bus we don't get the interrupt map, but =
if
> we assign the child we need to deal with what it means to assign an
> individual PCI device (e.g. on our internal KVM stuff we get an error =
on
> that reg property).
>=20
> What does this node represent in the first place? Is there really a
> PCI-to-PCI bridge? Are all other PCI devices underneath it?
PCIe has this concept of a "virtual" bridge between the root-comples, so =
that is what the node represents. Its always been a bit confusing to me =
as its not 100% standardized by PCI sig.
Maybe Ben's got some comments to add here from a generic PCIe point of =
view.
>>> Do we really need the error interrupt specified twice?
>>=20
>> I put it twice because it has multiple purposes, one has to do with =
interrupts defined by the PCI spec vs ones defined via FSL controller.
>=20
> There are PCI-defined error conditions that cause a host controller
> interrupt. What does this have to do with the bridge node?
Think of the "error" IRQ as shared between to classes of interrupts. =
One set are controller errors defined by FSL, the other are errors =
defined by PCI sig / bus point of view.
I'd expect controller errors to be handled by something like EDAC would =
bind at "fsl,qoriq-pcie-v2.2" level node. The PCI bus code would looks =
at the virtual bridge down.
>=20
>>> Why is there a zeroed out reg property: reg =3D <0 0 0 0 0> ??
>>=20
>> scratching my head, what happens if you remove it?
>=20
> Sigh.
:)
- k=
^ permalink raw reply
* RE: pci node question
From: Yoder Stuart-B08248 @ 2012-04-20 19:50 UTC (permalink / raw)
To: Kumar Gala; +Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <D04C6167-5286-4675-A6D8-32BA63305424@kernel.crashing.org>
> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Friday, April 20, 2012 1:54 PM
> To: Yoder Stuart-B08248
> Cc: linuxppc-dev@lists.ozlabs.org
> Subject: Re: pci node question
>=20
>=20
> On Apr 20, 2012, at 1:37 PM, Yoder Stuart-B08248 wrote:
>=20
> > There was refactoring change a while back that moved
> > the interrupt map down into the virtual pci bridge.
> >
> > example:
> > 42 /* controller at 0x200000 */
> > 43 &pci0 {
> > 44 compatible =3D "fsl,p2041-pcie", "fsl,qoriq-pcie-v2.2";
> > 45 device_type =3D "pci";
> > 46 #size-cells =3D <2>;
> > 47 #address-cells =3D <3>;
> > 48 bus-range =3D <0x0 0xff>;
> > 49 clock-frequency =3D <33333333>;
> > 50 interrupts =3D <16 2 1 15>;
> > 51 pcie@0 {
> > 52 reg =3D <0 0 0 0 0>;
> > 53 #interrupt-cells =3D <1>;
> > 54 #size-cells =3D <2>;
> > 55 #address-cells =3D <3>;
> > 56 device_type =3D "pci";
> > 57 interrupts =3D <16 2 1 15>;
> > 58 interrupt-map-mask =3D <0xf800 0 0 7>;
> > 59 interrupt-map =3D <
> > 60 /* IDSEL 0x0 */
> > 61 0000 0 0 1 &mpic 40 1 0 0
> > 62 0000 0 0 2 &mpic 1 1 0 0
> > 63 0000 0 0 3 &mpic 2 1 0 0
> > 64 0000 0 0 4 &mpic 3 1 0 0
> > 65 >;
> > 66 };
> > 67 };
> >
> > Why was the interrupt-map moved here?
>=20
> Its been a while, but I think i moved it down because of which node is us=
ed for interrupt handling in
> linux.
>=20
> > Do we really need the error interrupt specified twice?
>=20
> I put it twice because it has multiple purposes, one has to do with inter=
rupts defined by the PCI spec
> vs ones defined via FSL controller.
>=20
> > Why is there a zeroed out reg property: reg =3D <0 0 0 0 0> ??
>=20
> scratching my head, what happens if you remove it?
Tried removing the zeroed reg, and get this error when bringing
up an e1000 interface:
[root@kvmtst1 root]# ifconfig eth0 192.168.1.20
e1000e 0000:01:00.0: eth0: Unable to allocate interrupt, Error: -22
SIOCSIFFLAGS: Invalid argument
Stuart
^ permalink raw reply
* Re: pci node question
From: Scott Wood @ 2012-04-20 19:04 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <D04C6167-5286-4675-A6D8-32BA63305424@kernel.crashing.org>
On 04/20/2012 01:53 PM, Kumar Gala wrote:
>
> On Apr 20, 2012, at 1:37 PM, Yoder Stuart-B08248 wrote:
>
>> There was refactoring change a while back that moved
>> the interrupt map down into the virtual pci bridge.
>>
>> example:
>> 42 /* controller at 0x200000 */
>> 43 &pci0 {
>> 44 compatible = "fsl,p2041-pcie", "fsl,qoriq-pcie-v2.2";
>> 45 device_type = "pci";
>> 46 #size-cells = <2>;
>> 47 #address-cells = <3>;
>> 48 bus-range = <0x0 0xff>;
>> 49 clock-frequency = <33333333>;
>> 50 interrupts = <16 2 1 15>;
>> 51 pcie@0 {
>> 52 reg = <0 0 0 0 0>;
>> 53 #interrupt-cells = <1>;
>> 54 #size-cells = <2>;
>> 55 #address-cells = <3>;
>> 56 device_type = "pci";
>> 57 interrupts = <16 2 1 15>;
>> 58 interrupt-map-mask = <0xf800 0 0 7>;
>> 59 interrupt-map = <
>> 60 /* IDSEL 0x0 */
>> 61 0000 0 0 1 &mpic 40 1 0 0
>> 62 0000 0 0 2 &mpic 1 1 0 0
>> 63 0000 0 0 3 &mpic 2 1 0 0
>> 64 0000 0 0 4 &mpic 3 1 0 0
>> 65 >;
>> 66 };
>> 67 };
>>
>> Why was the interrupt-map moved here?
>
> Its been a while, but I think i moved it down because of which node is used for interrupt handling in linux.
That's not supposed to be how device tree bindings are determined.
It's causing us problems with virtualization device assignment, because
if we just assign the parent bus we don't get the interrupt map, but if
we assign the child we need to deal with what it means to assign an
individual PCI device (e.g. on our internal KVM stuff we get an error on
that reg property).
What does this node represent in the first place? Is there really a
PCI-to-PCI bridge? Are all other PCI devices underneath it?
>> Do we really need the error interrupt specified twice?
>
> I put it twice because it has multiple purposes, one has to do with interrupts defined by the PCI spec vs ones defined via FSL controller.
There are PCI-defined error conditions that cause a host controller
interrupt. What does this have to do with the bridge node?
>> Why is there a zeroed out reg property: reg = <0 0 0 0 0> ??
>
> scratching my head, what happens if you remove it?
Sigh.
-Scott
^ permalink raw reply
* Re: pci node question
From: Kumar Gala @ 2012-04-20 18:53 UTC (permalink / raw)
To: Yoder Stuart-B08248; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <9F6FE96B71CF29479FF1CDC8046E1503346013@039-SN1MPN1-002.039d.mgd.msft.net>
On Apr 20, 2012, at 1:37 PM, Yoder Stuart-B08248 wrote:
> There was refactoring change a while back that moved=20
> the interrupt map down into the virtual pci bridge.
>=20
> example:=20
> 42 /* controller at 0x200000 */
> 43 &pci0 {
> 44 compatible =3D "fsl,p2041-pcie", "fsl,qoriq-pcie-v2.2";
> 45 device_type =3D "pci";
> 46 #size-cells =3D <2>;
> 47 #address-cells =3D <3>;
> 48 bus-range =3D <0x0 0xff>;
> 49 clock-frequency =3D <33333333>;
> 50 interrupts =3D <16 2 1 15>;
> 51 pcie@0 {
> 52 reg =3D <0 0 0 0 0>;
> 53 #interrupt-cells =3D <1>;
> 54 #size-cells =3D <2>;
> 55 #address-cells =3D <3>;
> 56 device_type =3D "pci";
> 57 interrupts =3D <16 2 1 15>;
> 58 interrupt-map-mask =3D <0xf800 0 0 7>;
> 59 interrupt-map =3D <
> 60 /* IDSEL 0x0 */
> 61 0000 0 0 1 &mpic 40 1 0 0
> 62 0000 0 0 2 &mpic 1 1 0 0
> 63 0000 0 0 3 &mpic 2 1 0 0
> 64 0000 0 0 4 &mpic 3 1 0 0
> 65 >;
> 66 };
> 67 };
>=20
> Why was the interrupt-map moved here?
Its been a while, but I think i moved it down because of which node is =
used for interrupt handling in linux.
> Do we really need the error interrupt specified twice?
I put it twice because it has multiple purposes, one has to do with =
interrupts defined by the PCI spec vs ones defined via FSL controller.
> Why is there a zeroed out reg property: reg =3D <0 0 0 0 0> ??
scratching my head, what happens if you remove it?
- k
^ permalink raw reply
* pci node question
From: Yoder Stuart-B08248 @ 2012-04-20 18:37 UTC (permalink / raw)
To: Kumar Gala, linuxppc-dev@lists.ozlabs.org
There was refactoring change a while back that moved=20
the interrupt map down into the virtual pci bridge.
example:=20
42 /* controller at 0x200000 */
43 &pci0 {
44 compatible =3D "fsl,p2041-pcie", "fsl,qoriq-pcie-v2.2";
45 device_type =3D "pci";
46 #size-cells =3D <2>;
47 #address-cells =3D <3>;
48 bus-range =3D <0x0 0xff>;
49 clock-frequency =3D <33333333>;
50 interrupts =3D <16 2 1 15>;
51 pcie@0 {
52 reg =3D <0 0 0 0 0>;
53 #interrupt-cells =3D <1>;
54 #size-cells =3D <2>;
55 #address-cells =3D <3>;
56 device_type =3D "pci";
57 interrupts =3D <16 2 1 15>;
58 interrupt-map-mask =3D <0xf800 0 0 7>;
59 interrupt-map =3D <
60 /* IDSEL 0x0 */
61 0000 0 0 1 &mpic 40 1 0 0
62 0000 0 0 2 &mpic 1 1 0 0
63 0000 0 0 3 &mpic 2 1 0 0
64 0000 0 0 4 &mpic 3 1 0 0
65 >;
66 };
67 };
Why was the interrupt-map moved here?
Do we really need the error interrupt specified twice?
Why is there a zeroed out reg property: reg =3D <0 0 0 0 0> ??
Thanks,
Stuart
^ permalink raw reply
* Re: PowerPC radeon KMS - is it possible?
From: Gerhard Pircher @ 2012-04-20 16:14 UTC (permalink / raw)
To: "Michel Dänzer"; +Cc: linuxppc-dev
In-Reply-To: <1334927896.5989.463.camel@thor.local>
-------- Original-Nachricht --------
> Datum: Fri, 20 Apr 2012 15:18:16 +0200
> Von: "Michel Dänzer" <michel@daenzer.net>
> An: Gerhard Pircher <gerhard_pircher@gmx.net>
> CC: linuxppc-dev@lists.ozlabs.org
> Betreff: Re: PowerPC radeon KMS - is it possible?
> On Fre, 2012-04-20 at 13:15 +0200, Gerhard Pircher wrote:
> > > Von: "Michel Dänzer" <michel@daenzer.net>
> > > On Don, 2012-04-19 at 13:48 +0200, Gerhard Pircher wrote:
> > > >
> > > > The "former case" is an explanation, why I see data corruption
> > > > with my< AGPGART driver (more or less a copy of the uninorth
> > > > driver) on my non-coherent platform. There are no cache flushes
> > > > done for writes to already mapped pages.
> > >
> > > As I said, the radeon driver always maps AGP memory uncacheable for
> > > the CPU, so no such CPU cache flushes should be necessary.
> > I know. We also discussed this topic over two years ago. :-)
> >
> > What I didn't understand yet is how this uncacheable memory is
> > allocated (well, I never took the time to look at this again). The
> > functions in ttm_page_alloc.c seem to allocate normal cacheable
> > memory and try to set the page flags with set_pages_array_uc(),
> > which is more or less a no-op on powerpc. ttm_page_alloc_dma.c on
> > the other side is only used with SWIOTLB!?
> [...]
> > Could it be that the memory is finally mapped uncacheable by
> radeon_bo_kmap()/
> > ttm_bo_kmap()/..some other TTM functions../vmap()?
>
> Yeah, AFAICT, basically ttm_io_prot() defines the mapping attributes,
> and vmap() is used to enforce them for kernel mappings.
Okay, that sounds like the approach used by arch/powerpc/mm/dma-
noncoherent.c in my ("green") ears. What about the PCIGART mode?
Is the driver free to use cached memory in this mode?
> > Here is an excerpt of the 2.6.39 kernel log. IIRC the testing code
> > changed in the meantime so I guess it would make sense to repeat it
> > with a newer kernel version.
>
> I was going to suggest that. :)
As expected. :-)
> > [ 5.490569] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c0, expected 0xf1326160 (GTT map 0xf1326000-0xf1426000)
> > [ 5.503397] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c4, expected 0xf1326164 (GTT map 0xf1326000-0xf1426000)
> > [ 5.516202] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c0, expected 0xf1326168 (GTT map 0xf1326000-0xf1426000)
> > [ 5.528993] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c4, expected 0xf132616c (GTT map 0xf1326000-0xf1426000)
> [...]
> > [ 5.878809] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0: Got 0xf1416ec0, expected 0xf1570ec0 (VRAM map 0xf1480000-0xf1580000)
>
> > For the GTT->VRAM copy it looks like the AGPGART driver at least
> > gets the graphics aperture translation table right, as both the
> > returned and expected values are within a page. But the page offset
> > of the returned values (0x8c0, 0x8c4) makes me wonder whether I'm
> > fooled by a hardware bug or a cache coherency problem.
>
> Hard to say... at least it managed to transfer the first 352 bytes
> correctly. ;)
Better than nothing! :-)
> > The VRAM->GTT copy totally puzzles me, as it returns a wrong page
> > address, but the offset is fine!?
>
> Maybe it's still the values from the GTT->VRAM test, i.e. either the GPU
> writes didn't make it to the memory mapped into the AGP GART (some AGP
Good point. Maybe I should explicitly clear the gtt_map before the
VRAM->GTT copy test is executed.
> bridges are known to have issues with that) or the CPU doesn't see it.
What is the workaround for such an AGP bridge? If there is one at all...
> BTW, does your driver set cant_use_aperture, or is the linear aperture
> accessible by the CPU?
The driver sets cant_use_aperture. I couldn't get it working at all
without it.
regards,
Gerhard
--
Empfehlen Sie GMX DSL Ihren Freunden und Bekannten und wir
belohnen Sie mit bis zu 50,- Euro! https://freundschaftswerbung.gmx.de
^ permalink raw reply
* Re: PowerPC radeon KMS - is it possible?
From: Michel Dänzer @ 2012-04-20 13:18 UTC (permalink / raw)
To: Gerhard Pircher; +Cc: linuxppc-dev
In-Reply-To: <20120420111527.190290@gmx.net>
On Fre, 2012-04-20 at 13:15 +0200, Gerhard Pircher wrote:=20
> > Von: "Michel D=C3=A4nzer" <michel@daenzer.net>
> > On Don, 2012-04-19 at 13:48 +0200, Gerhard Pircher wrote:=20
> > >=20
> > > The "former case" is an explanation, why I see data corruption with m=
y
> > > AGPGART driver (more or less a copy of the uninorth driver) on my
> > > non-coherent platform. There are no cache flushes done for writes to
> > > already mapped pages.
> >=20
> > As I said, the radeon driver always maps AGP memory uncacheable for the
> > CPU, so no such CPU cache flushes should be necessary.
> I know. We also discussed this topic over two years ago. :-)
>=20
> What I didn't understand yet is how this uncacheable memory is allocated
> (well, I never took the time to look at this again). The functions in
> ttm_page_alloc.c seem to allocate normal cacheable memory and try to set
> the page flags with set_pages_array_uc(), which is more or less a no-op
> on powerpc.
> ttm_page_alloc_dma.c on the other side is only used with SWIOTLB!?
[...]=20
> Could it be that the memory is finally mapped uncacheable by radeon_bo_km=
ap()/
> ttm_bo_kmap()/..some other TTM functions../vmap()?
Yeah, AFAICT, basically ttm_io_prot() defines the mapping attributes,
and vmap() is used to enforce them for kernel mappings.
> Here is an excerpt of the 2.6.39 kernel log. IIRC the testing code change=
d
> in the meantime so I guess it would make sense to repeat it with a newer
> kernel version.
I was going to suggest that. :)
> [ 5.490569] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0=
: Got 0xf13268c0, expected 0xf1326160 (GTT map 0xf1326000-0xf1426000)
> [ 5.503397] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0=
: Got 0xf13268c4, expected 0xf1326164 (GTT map 0xf1326000-0xf1426000)
> [ 5.516202] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0=
: Got 0xf13268c0, expected 0xf1326168 (GTT map 0xf1326000-0xf1426000)
> [ 5.528993] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0=
: Got 0xf13268c4, expected 0xf132616c (GTT map 0xf1326000-0xf1426000)
[...]=20
> [ 5.878809] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0=
: Got 0xf1416ec0, expected 0xf1570ec0 (VRAM map 0xf1480000-0xf1580000)
> For the GTT->VRAM copy it looks like the AGPGART driver at least
> gets the graphics aperture translation table right, as both the
> returned and expected values are within a page. But the page offset
> of the returned values (0x8c0, 0x8c4) makes me wonder whether I'm
> fooled by a hardware bug or a cache coherency problem.
Hard to say... at least it managed to transfer the first 352 bytes
correctly. ;)
> The VRAM->GTT copy totally puzzles me, as it returns a wrong page
> address, but the offset is fine!?
Maybe it's still the values from the GTT->VRAM test, i.e. either the GPU
writes didn't make it to the memory mapped into the AGP GART (some AGP
bridges are known to have issues with that) or the CPU doesn't see it.
BTW, does your driver set cant_use_aperture, or is the linear aperture
accessible by the CPU?
--=20
Earthling Michel D=C3=A4nzer | http://www.amd.c=
om
Libre software enthusiast | Debian, X and DRI developer
^ permalink raw reply
* Re: PowerPC radeon KMS - is it possible?
From: Gerhard Pircher @ 2012-04-20 11:15 UTC (permalink / raw)
To: "Michel Dänzer"; +Cc: linuxppc-dev
-------- Original-Nachricht --------
> Datum: Thu, 19 Apr 2012 14:41:16 +0200
> Von: "Michel Dänzer" <michel@daenzer.net>
> An: Gerhard Pircher <gerhard_pircher@gmx.net>
> CC: linuxppc-dev@lists.ozlabs.org, schwab@linux-m68k.org, ojordan12345@hotmail.co.uk
> Betreff: Re: PowerPC radeon KMS - is it possible?
> On Don, 2012-04-19 at 13:48 +0200, Gerhard Pircher wrote:
> > > Von: "Michel Dänzer" <michel@daenzer.net>
> > > On Mit, 2012-04-18 at 18:23 +0200, Gerhard Pircher wrote:
> > > > > Von: "Michel Dänzer" <michel@daenzer.net>
> > > > > On Mit, 2012-04-18 at 17:49 +0200, Gerhard Pircher wrote:
> > > > > >
> > > > > > That may be a stupid question, but is it allowed (for a DRM
> > > > > > client or whatever does the mapping) to change the content of
> > > > > > a page mapped into the AGP GART or is it necessary to
> > > > > > explicitly unmap the page, change its content and map it again?
> > > > >
> > > > > The former.
> > > > I know that the uninorth AGPGART driver does a cache flushing for
> > > > newly mapped pages, but is there any code in the driver that
> > > > handles the former case (or isn't this necessary on PPC Macs)?
> > >
> > > If by 'former case' you mean userspace modifying memory mapped into
> > > the AGP GART, then no, this generally doesn't require special
> > > treatment on PowerMacs. (Ignoring the potential issue mentioned by
> > > Ben in this thread)
> > I guess you refer to the ordering issue here.
>
> Yeah.
>
> > The "former case" is an explanation, why I see data corruption with my
> > AGPGART driver (more or less a copy of the uninorth driver) on my
> > non-coherent platform. There are no cache flushes done for writes to
> > already mapped pages.
>
> As I said, the radeon driver always maps AGP memory uncacheable for the
> CPU, so no such CPU cache flushes should be necessary.
I know. We also discussed this topic over two years ago. :-)
What I didn't understand yet is how this uncacheable memory is allocated
(well, I never took the time to look at this again). The functions in
ttm_page_alloc.c seem to allocate normal cacheable memory and try to set
the page flags with set_pages_array_uc(), which is more or less a no-op
on powerpc. ttm_page_alloc_dma.c on the other side is only used with
SWIOTLB!?
> > I tested this with radeon.test=1, but I'm not even sure if this code
> > changes already mapped pages [...]
>
> It does. radeon_bo_pin(..., RADEON_GEM_DOMAIN_GTT, ...) binds the buffer
> memory into the AGP GART, and the test only maps it to the CPU after
> that.
Could it be that the memory is finally mapped uncacheable by radeon_bo_kmap()/
ttm_bo_kmap()/..some other TTM functions../vmap()?
> I take it the test fails for you? How exactly?
I didn't work on the driver for a long time. It looks like I did the last
tests with a 2.6.39 kernel, where I changed the radeon test routine to not
stop on a failed copy. This way I could check for a pattern in the failed
copies.
Here is an excerpt of the 2.6.39 kernel log. IIRC the testing code changed
in the meantime so I guess it would make sense to repeat it with a newer
kernel version.
[ 5.185619] calling agp_init+0x0/0x5c @ 1
[ 5.189816] Linux agpgart interface v0.103
[ 5.193993] initcall agp_init+0x0/0x5c returned 0 after 4076 usecs
[ 5.200359] calling agp_articias_init+0x0/0x58 @ 1
[ 5.205411] agpgart-articias 0000:00:00.0: MAI Logic Articia S chipset
[ 5.213555] agpgart-articias 0000:00:00.0: configuring for size idx: 6
[ 5.220996] agpgart-articias 0000:00:00.0: AGP aperture is 128M @ 0xc0000000
[ 5.228557] initcall agp_articias_init+0x0/0x58 returned 0 after 22629 usecs
[ 5.235791] calling drm_core_init+0x0/0x16c @ 1
[ 5.240839] [drm] Initialized drm 1.1.0 20060810
[ 5.245623] initcall drm_core_init+0x0/0x16c returned 0 after 4937 usecs
[ 5.252510] calling ttm_init+0x0/0x8c @ 1
[ 5.256908] initcall ttm_init+0x0/0x8c returned 0 after 197 usecs
[ 5.263172] calling radeon_init+0x0/0x11c @ 1
[ 5.267731] [drm] radeon kernel modesetting enabled.
[ 5.274683] [drm] initializing kernel modesetting (RV280 0x1002:0x5960).
[ 5.281657] [drm] register mmio base: 0x88000000
[ 5.286397] [drm] register mmio size: 65536
[ 5.327510] [drm] AGP mode requested: 1
[ 5.331485] agpgart-articias 0000:00:00.0: AGP 1.0 bridge
[ 5.337071] agpgart-articias 0000:00:00.0: putting AGP V2 device into 1x mode
[ 5.344440] radeon 0000:01:00.0: putting AGP V2 device into 1x mode
[ 5.350865] radeon 0000:01:00.0: GTT: 128M 0xC0000000 - 0xC7FFFFFF
[ 5.357197] [drm] Generation 2 PCI interface, using max accessible memory
[ 5.364111] radeon 0000:01:00.0: VRAM: 256M 0x0000000080000000 - 0x000000008FFFFFFF (256M used)
[ 5.373060] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
[ 5.379815] [drm] Driver supports precise vblank timestamp query.
[ 5.386068] [drm] radeon: irq initialized.
[ 5.390264] [drm] Detected VRAM RAM=256M, BAR=128M
[ 5.395177] [drm] RAM width 128bits DDR
[ 5.399322] [TTM] Zone kernel: Available graphics memory: 380684 kiB.
[ 5.406047] [TTM] Zone highmem: Available graphics memory: 773900 kiB.
[ 5.412725] [TTM] Initializing pool allocator.
[ 5.417395] [drm] radeon: 256M of VRAM memory ready
[ 5.422442] [drm] radeon: 128M of GTT memory ready.
[ 5.428527] agpgart-articias 0000:00:00.0: TLB flush!
[ 5.433785] radeon 0000:01:00.0: WB disabled
[ 5.438671] [drm] Loading R200 Microcode
[ 5.444292] agpgart-articias 0000:00:00.0: TLB flush!
[ 5.449629] [drm] radeon: ring at 0x00000000C0001000
[ 5.454761] [drm] ring test succeeded in 0 usecs
[ 5.460620] agpgart-articias 0000:00:00.0: TLB flush!
[ 5.465889] [drm] radeon: ib pool ready.
[ 5.470212] [drm] ib test succeeded in 0 usecs
[ 5.475939] agpgart-articias 0000:00:00.0: TLB flush!
[ 5.490569] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c0, expected 0xf1326160 (GTT map 0xf1326000-0xf1426000)
[ 5.503397] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c4, expected 0xf1326164 (GTT map 0xf1326000-0xf1426000)
[ 5.516202] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c0, expected 0xf1326168 (GTT map 0xf1326000-0xf1426000)
[ 5.528993] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c4, expected 0xf132616c (GTT map 0xf1326000-0xf1426000)
[ 5.541777] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c0, expected 0xf1326170 (GTT map 0xf1326000-0xf1426000)
[ 5.554535] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c4, expected 0xf1326174 (GTT map 0xf1326000-0xf1426000)
[ 5.567303] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c0, expected 0xf1326178 (GTT map 0xf1326000-0xf1426000)
[ 5.580049] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c4, expected 0xf132617c (GTT map 0xf1326000-0xf1426000)
[ 5.592843] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c0, expected 0xf1326180 (GTT map 0xf1326000-0xf1426000)
[ 5.605652] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c4, expected 0xf1326184 (GTT map 0xf1326000-0xf1426000)
[ 5.618392] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c0, expected 0xf1326188 (GTT map 0xf1326000-0xf1426000)
[ 5.631178] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c4, expected 0xf132618c (GTT map 0xf1326000-0xf1426000)
[ 5.643970] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c0, expected 0xf1326190 (GTT map 0xf1326000-0xf1426000)
[ 5.656769] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c4, expected 0xf1326194 (GTT map 0xf1326000-0xf1426000)
[ 5.669494] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c0, expected 0xf1326198 (GTT map 0xf1326000-0xf1426000)
[ 5.682293] [drm:radeon_test_moves] *ERROR* Incorrect GTT->VRAM copy 0: Got 0xf13268c4, expected 0xf132619c (GTT map 0xf1326000-0xf1426000)
[ 5.878809] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0: Got 0xf1416ec0, expected 0xf1570ec0 (VRAM map 0xf1480000-0xf1580000)
[ 5.891734] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0: Got 0xf1416ec4, expected 0xf1570ec4 (VRAM map 0xf1480000-0xf1580000)
[ 5.904616] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0: Got 0xf1416ec8, expected 0xf1570ec8 (VRAM map 0xf1480000-0xf1580000)
[ 5.917460] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0: Got 0xf1416ecc, expected 0xf1570ecc (VRAM map 0xf1480000-0xf1580000)
[ 5.930286] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0: Got 0xf1416ed0, expected 0xf1570ed0 (VRAM map 0xf1480000-0xf1580000)
[ 5.943164] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0: Got 0xf1416ed4, expected 0xf1570ed4 (VRAM map 0xf1480000-0xf1580000)
[ 5.956052] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0: Got 0xf1416ed8, expected 0xf1570ed8 (VRAM map 0xf1480000-0xf1580000)
[ 5.968898] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0: Got 0xf1416edc, expected 0xf1570edc (VRAM map 0xf1480000-0xf1580000)
[ 5.981758] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0: Got 0xf1416ee0, expected 0xf1570ee0 (VRAM map 0xf1480000-0xf1580000)
[ 5.994593] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0: Got 0xf1416ee4, expected 0xf1570ee4 (VRAM map 0xf1480000-0xf1580000)
[ 6.007455] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0: Got 0xf1416ee8, expected 0xf1570ee8 (VRAM map 0xf1480000-0xf1580000)
[ 6.020309] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0: Got 0xf1416eec, expected 0xf1570eec (VRAM map 0xf1480000-0xf1580000)
[ 6.033208] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0: Got 0xf1416ef0, expected 0xf1570ef0 (VRAM map 0xf1480000-0xf1580000)
[ 6.046077] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0: Got 0xf1416ef4, expected 0xf1570ef4 (VRAM map 0xf1480000-0xf1580000)
[ 6.058964] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0: Got 0xf1416ef8, expected 0xf1570ef8 (VRAM map 0xf1480000-0xf1580000)
[ 6.071850] [drm:radeon_test_moves] *ERROR* Incorrect VRAM->GTT copy 0: Got 0xf1416efc, expected 0xf1570efc (VRAM map 0xf1480000-0xf1580000)
For the GTT->VRAM copy it looks like the AGPGART driver at least
gets the graphics aperture translation table right, as both the
returned and expected values are within a page. But the page offset
of the returned values (0x8c0, 0x8c4) makes me wonder whether I'm
fooled by a hardware bug or a cache coherency problem.
The VRAM->GTT copy totally puzzles me, as it returns a wrong page
address, but the offset is fine!?
br,
Gerhard
--
Empfehlen Sie GMX DSL Ihren Freunden und Bekannten und wir
belohnen Sie mit bis zu 50,- Euro! https://freundschaftswerbung.gmx.de
^ permalink raw reply
* Re: [PATCH v4] KVM: PPC Use clockevent multiplier and shifter for decrementer
From: Alexander Graf @ 2012-04-20 10:24 UTC (permalink / raw)
To: Bharat Bhushan; +Cc: Bharat Bhushan, linuxppc-dev, kvm, kvm-ppc, bharatb.yadav
In-Reply-To: <1334764879-18020-1-git-send-email-bharat.bhushan@freescale.com>
On 18.04.2012, at 18:01, Bharat Bhushan wrote:
> Time for which the hrtimer is started for decrementer emulation is =
calculated using tb_ticks_per_usec. While hrtimer uses the clockevent =
for DEC reprogramming (if needed) and which calculate timebase ticks =
using the multiplier and shifter mechanism implemented within clockevent =
layer. It was observed that this conversion (timebase->time->timebase) =
are not correct because the mechanism are not consistent. In our setup =
it adds 2% jitter.
>=20
> With this patch clockevent multiplier and shifter mechanism are used =
when starting hrtimer for decrementer emulation. Now the jitter is < =
0.5%.
>=20
> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
Thanks, applied to kvm-ppc-next with fixed commit message and fixed =
trailing whitespace :).
Alex
^ permalink raw reply
* Re: [PATCH 3/3] powerpc/mpic: Properly set default triggers
From: Andreas Schwab @ 2012-04-20 9:17 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Thomas Gleixner, linuxppc-dev
In-Reply-To: <1334892657.31646.2.camel__44318.4292679236$1334892821$gmane$org@pasglop>
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> This gets rid of the unused default senses array, and replaces the
> incorrect use of IRQ_TYPE_NONE with the new IRQ_TYPE_DEFAULT for
> the initial set_trigger() call when mapping an interrupt.
>
> This in turn makes us read the HW state and update the irq desc
> accordingly.
That fixes the interrupt problem for me. The only regression left is
the spurious interrupts.
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
* [PATCH] gianfar: add GRO support
From: Jiajun Wu @ 2012-04-20 8:54 UTC (permalink / raw)
To: netdev, davem; +Cc: Jiajun Wu, linuxppc-dev
Replace netif_receive_skb with napi_gro_receive.
Signed-off-by: Jiajun Wu <b06378@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index e7bed53..1adb024 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -136,7 +136,7 @@ static void gfar_netpoll(struct net_device *dev);
int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit);
static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue);
static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
- int amount_pull);
+ int amount_pull, struct napi_struct *napi);
void gfar_halt(struct net_device *dev);
static void gfar_halt_nodisable(struct net_device *dev);
void gfar_start(struct net_device *dev);
@@ -2675,12 +2675,12 @@ static inline void gfar_rx_checksum(struct sk_buff *skb, struct rxfcb *fcb)
/* gfar_process_frame() -- handle one incoming packet if skb
* isn't NULL. */
static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
- int amount_pull)
+ int amount_pull, struct napi_struct *napi)
{
struct gfar_private *priv = netdev_priv(dev);
struct rxfcb *fcb = NULL;
- int ret;
+ gro_result_t ret;
/* fcb is at the beginning if exists */
fcb = (struct rxfcb *)skb->data;
@@ -2719,9 +2719,9 @@ static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
__vlan_hwaccel_put_tag(skb, fcb->vlctl);
/* Send the packet up the stack */
- ret = netif_receive_skb(skb);
+ ret = napi_gro_receive(napi, skb);
- if (NET_RX_DROP == ret)
+ if (GRO_DROP == ret)
priv->extra_stats.kernel_dropped++;
return 0;
@@ -2783,7 +2783,8 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
skb_put(skb, pkt_len);
rx_queue->stats.rx_bytes += pkt_len;
skb_record_rx_queue(skb, rx_queue->qindex);
- gfar_process_frame(dev, skb, amount_pull);
+ gfar_process_frame(dev, skb, amount_pull,
+ &rx_queue->grp->napi);
} else {
netif_warn(priv, rx_err, dev, "Missing skb!\n");
--
1.5.6.5
^ 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