* RE: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver.
From: Sethi Varun-B16395 @ 2012-12-11 4:50 UTC (permalink / raw)
To: Wood Scott-B07421
Cc: Tabi Timur-B04825, Joerg Roedel, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org
In-Reply-To: <1355187644.5334.23@snotra>
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, December 11, 2012 6:31 AM
> To: Sethi Varun-B16395
> Cc: Wood Scott-B07421; Joerg Roedel; linux-kernel@vger.kernel.org;
> iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org; Tabi
> Timur-B04825
> Subject: Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes
> required by fsl PAMU driver.
>=20
> On 12/10/2012 04:10:06 AM, Sethi Varun-B16395 wrote:
> >
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Tuesday, December 04, 2012 11:53 PM
> > > To: Sethi Varun-B16395
> > > Cc: Wood Scott-B07421; Joerg Roedel; linux-kernel@vger.kernel.org;
> > > iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
> > Tabi
> > > Timur-B04825
> > > Subject: Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes
> > > required by fsl PAMU driver.
> > >
> > > On 12/04/2012 05:53:33 AM, Sethi Varun-B16395 wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Wood Scott-B07421
> > > > > Sent: Monday, December 03, 2012 10:34 PM
> > > > > To: Sethi Varun-B16395
> > > > > Cc: Joerg Roedel; linux-kernel@vger.kernel.org;
> > iommu@lists.linux-
> > > > > foundation.org; Wood Scott-B07421;
> > linuxppc-dev@lists.ozlabs.org;
> > > > Tabi
> > > > > Timur-B04825
> > > > > Subject: Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain
> > attributes
> > > > > required by fsl PAMU driver.
> > > > >
> > > > > On 12/03/2012 10:57:29 AM, Sethi Varun-B16395 wrote:
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: iommu-bounces@lists.linux-foundation.org
> > [mailto:iommu-
> > > > > > > bounces@lists.linux-foundation.org] On Behalf Of Joerg
> > Roedel
> > > > > > > Sent: Sunday, December 02, 2012 7:33 PM
> > > > > > > To: Sethi Varun-B16395
> > > > > > > Cc: linux-kernel@vger.kernel.org;
> > > > iommu@lists.linux-foundation.org;
> > > > > > Wood
> > > > > > > Scott-B07421; linuxppc-dev@lists.ozlabs.org; Tabi
> > Timur-B04825
> > > > > > > Subject: Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain
> > > > attributes
> > > > > > > required by fsl PAMU driver.
> > > > > > >
> > > > > > > Hmm, we need to work out a good abstraction for this.
> > > > > > >
> > > > > > > On Tue, Nov 20, 2012 at 07:24:56PM +0530, Varun Sethi wrote:
> > > > > > > > Added the following domain attributes required by FSL PAMU
> > > > driver:
> > > > > > > > 1. Subwindows field added to the iommu domain geometry
> > > > attribute.
> > > > > > >
> > > > > > > Are the Subwindows mapped with full size or do you map only
> > > > parts
> > > > > > of the
> > > > > > > subwindows?
> > > > > > >
> > > > > > [Sethi Varun-B16395] It's possible to map a part of the
> > subwindow
> > > > i.e.
> > > > > > size of the mapping can be less than the sub window size.
> > > > > >
> > > > > > > > + * This attribute indicates number of DMA subwindows
> > > > > supported
> > > > > > by
> > > > > > > > + * the geometry. If there is a single window that maps
> > > > the
> > > > > > entire
> > > > > > > > + * geometry, attribute must be set to "1". A value of
> > > > "0"
> > > > > > implies
> > > > > > > > + * that this mechanism is not used at all(normal paging
> > > > is
> > > > > > used).
> > > > > > > > + * Value other than* "0" or "1" indicates the actual
> > > > number
> > > > > of
> > > > > > > > + * subwindows.
> > > > > > > > + */
> > > > > > >
> > > > > > > This semantic is ugly, how about a feature detection
> > mechanism?
> > > > > > >
> > > > > > [Sethi Varun-B16395] A feature mechanism to query the type of
> > > > IOMMU?
> > > > >
> > > > > A feature mechanism to determine whether this subwindow
> > mechanism is
> > > > > available, and what the limits are.
> > > > >
> > > > So, we use the IOMMU capability interface to find out if IOMMU
> > > > supports sub windows or not, right? But still number of sub
> > windows
> > > > would be specified as a part of the geometry and the valid value
> > for
> > > > sub windows would 0,1 or actual number of sub windows.
> > >
> > > How does a user of the interface find out what values are possible
> > for
> > > the "actual number of subwindows"? How does a user of the
> > interface find
> > > out whether there are any limitations on specifying a value of zero
> > (in
> > > the case of PAMU, that would be a maximum 1 MiB naturally-aligned
> > > aperture to support arbitrary 4KiB mappings)?
> > How about if we say that the default value for subwindows is zero and
> > this what you get when you read the geometry (iommu_get_attr) after
> > initializing the domain? In that case the user would know that
> > implication of setting subwindows to zero with respect to the aperture
> > size.
>=20
> So it would default to the maximum aperture size possible with no
> subwindows? That might be OK, though is there a way to reset the domain
> later on to get back to that informational state?
>=20
[Sethi Varun-B16395] Yes, that can be done via iommu_set_attr API.
> How about finding out the maximum number of subwindows?
[Sethi Varun-B16395] We can introduce an API to determine the permissible r=
ange of values for an attribute, but it may be redundant for other IOMMU im=
plementations. For the IOMMU implementations where geometry is just a read =
only attribute, iommu_get_attr returns the set of permissible values. I thi=
nk it would be better if we can add a read only field (for the users) "max_=
subwindows" to the geometry.
-Varun
^ permalink raw reply
* Re: [PATCH v2 1/4] kprobes/powerpc: Do not disable External interrupts during single step
From: Suzuki K. Poulose @ 2012-12-11 5:48 UTC (permalink / raw)
To: benh, Kumar Gala
Cc: srikar, peterz, bigeasy, oleg, linux-kernel, linuxppc-dev,
Suzuki K. Poulose, anton, mingo
In-Reply-To: <20121203150720.7727.91582.stgit@suzukikp>
On 12/03/2012 08:37 PM, Suzuki K. Poulose wrote:
> From: Suzuki K. Poulose <suzuki@in.ibm.com>
>
> External/Decrement exceptions have lower priority than the Debug Exception.
> So, we don't have to disable the External interrupts before a single step.
> However, on BookE, Critical Input Exception(CE) has higher priority than a
> Debug Exception. Hence we mask them.
>
> Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: Ananth N Mavinakaynahalli <ananth@in.ibm.com>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> Cc: linuxppc-dev@ozlabs.org
> ---
> arch/powerpc/kernel/kprobes.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index e88c643..4901b34 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -104,13 +104,13 @@ void __kprobes arch_remove_kprobe(struct kprobe *p)
>
> static void __kprobes prepare_singlestep(struct kprobe *p, struct pt_regs *regs)
> {
> - /* We turn off async exceptions to ensure that the single step will
> - * be for the instruction we have the kprobe on, if we dont its
> - * possible we'd get the single step reported for an exception handler
> - * like Decrementer or External Interrupt */
> - regs->msr &= ~MSR_EE;
> regs->msr |= MSR_SINGLESTEP;
> #ifdef CONFIG_PPC_ADV_DEBUG_REGS
> + /*
> + * We turn off Critical Input Exception(CE) to ensure that the single
> + * step will be for the instruction we have the probe on; if we don't,
> + * it is possible we'd get the single step reported for CE.
> + */
> regs->msr &= ~MSR_CE;
> mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) | DBCR0_IC | DBCR0_IDM);
> #ifdef CONFIG_PPC_47x
>
Ben, Kumar,
Could you please review this patch ?
Thanks
Suzuki
^ permalink raw reply
* Re: Understanding how kernel updates MMU hash table
From: Pegasus11 @ 2012-12-11 7:27 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <1355087457.28585.61.camel@pasglop>
Hi ben
Now that many things are becoming clear let me sum up my understanding until
this point. Do correct it if there are mistakes.
1. Linux page table structure (PGD, PUD, PMD and PTE) is directly used in
case of architecture that lend themselves to such a tree structure for
maintaining virtual memory information. Otherwise Linux needs to maintain
two seperate constructs like it does in case of PowerPC. Right?
2. PowerPC's hash table as you said is pretty large. However isn't it still
smaller than Linux's VM infrastructure such that the chances of it being
'FULL' are a lot more. It is also possible that there could be two entries
in the table that points to the same Real address. Like a page being shared
by two processes?
My main concern here is to understand if having such an inverted page table
aka the hash table helps us in any way when doing TLB flushes. You mentioned
and I also read in a paper by Paul Mackerras that every Linux PTE (LPTE) in
case of ppc64 contains 4 extra bits that help us to get to the very slot in
the hash table that houses the corresponding hashtable PTE(HPTE). Now this
(at least to me) is smartness on the part of the kernel and I do not think
the architecture per se is doing us any favor by having that hash table
right? Or am I missing something here?
His paper is (or rather was) on how one can optimize the Linux ppc kernel
and time and again he mentions the fact that one can first record the LPTEs
being invalidated and then remove the corresponding HPTEs in a batched
format. In his own words "Alternatively, it would be possible to make a list
of virtual addresses when LPTEs are changed and then use that list in the
TLB flush routines to avoid the search through the Linux page tables". So do
we skip looking for the corresponding LPTEs or perhaps we've already
invalidated them and we remove the corresponding HPTEs in a batch as you
mentioned earlier?? Could you shed some light on how this optimization
actually developed over time? He had results for an "immediate update"
kernel and "batched update" kernel for both ppc32 and ppc64. For ppc32 the
batched update is actually a bit worse than immediate update however for
ppc64, the batched update performs better than immediate update. What
exactly is helping ppc64 perform better with the so called "batched update"?
Is it the encoding of the HPTE address in the LPTE as mentioned above? Or
some aspect of ppc64 that I am unaware of? :thinking:
Also on a generic note, how come we have 4 spare bits in the PTE for 64bit
address space? Large pages perhaps?
--
View this message in context: http://old.nabble.com/Understanding-how-kernel-updates-MMU-hash-table-tp34760537p34782747.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.
^ permalink raw reply
* Please pull 'next' branch of 5xxx tree
From: Anatolij Gustschin @ 2012-12-11 8:23 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
Hi Ben,
please pull mpc5xxx patches for v3.8. There is a dts for a
new a3m071 board, a fix for optional mpc5121 DIU support and
minor changes to mpc5200 lpbfifo driver to simplify module
init/exit code. These patches have already been in linux-next
for a while. Thanks!
Anatolij
The following changes since commit b69f0859dc8e633c5d8c06845811588fe17e68b3:
Linux 3.7-rc8 (2012-12-03 11:22:37 -0800)
are available in the git repository at:
git://git.denx.de/linux-2.6-agust.git next
Srinivas Kandagatla (1):
powerpc/mpc52xx: use module_platform_driver macro
Stefan Roese (1):
powerpc: mpc5200: Add a3m071 board support
Timur Tabi (1):
powerpc/512x: don't compile any platform DIU code if the DIU is not enabled
arch/powerpc/boot/dts/a3m071.dts | 144 +++++++++++++++++++++++++
arch/powerpc/platforms/512x/Kconfig | 1 -
arch/powerpc/platforms/512x/mpc5121_ads.c | 3 +
arch/powerpc/platforms/512x/mpc512x.h | 11 ++-
arch/powerpc/platforms/512x/mpc512x_shared.c | 25 +----
arch/powerpc/platforms/52xx/mpc5200_simple.c | 1 +
arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | 16 +---
7 files changed, 162 insertions(+), 39 deletions(-)
create mode 100644 arch/powerpc/boot/dts/a3m071.dts
^ permalink raw reply
* [PATCH] KVM: PPC: Fix SREGS documentation reference
From: Mihai Caraman @ 2012-12-11 13:38 UTC (permalink / raw)
To: kvm-ppc; +Cc: Mihai Caraman, linuxppc-dev, kvm
Reflect the uapi folder change in SREGS API documentation.
Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
Documentation/virtual/kvm/api.txt | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index a4df553..9cf591d 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -345,7 +345,7 @@ struct kvm_sregs {
__u64 interrupt_bitmap[(KVM_NR_INTERRUPTS + 63) / 64];
};
-/* ppc -- see arch/powerpc/include/asm/kvm.h */
+/* ppc -- see arch/powerpc/include/uapi/asm/kvm.h */
interrupt_bitmap is a bitmap of pending external interrupts. At most
one bit may be set. This interrupt has been acknowledged by the APIC
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH] KVM: PPC: Fix SREGS documentation reference
From: Amos Kong @ 2012-12-11 14:22 UTC (permalink / raw)
To: Mihai Caraman; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1355233103-14587-1-git-send-email-mihai.caraman@freescale.com>
On Tue, Dec 11, 2012 at 9:38 PM, Mihai Caraman
<mihai.caraman@freescale.com> wrote:
>
> Reflect the uapi folder change in SREGS API documentation.
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> Documentation/virtual/kvm/api.txt | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index a4df553..9cf591d 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -345,7 +345,7 @@ struct kvm_sregs {
> __u64 interrupt_bitmap[(KVM_NR_INTERRUPTS + 63) / 64];
> };
>
> -/* ppc -- see arch/powerpc/include/asm/kvm.h */
> +/* ppc -- see arch/powerpc/include/uapi/asm/kvm.h */
Trivial fix.
Reviewed-by: Amos Kong <kongjianjun@gmail.com>
>
> interrupt_bitmap is a bitmap of pending external interrupts. At most
> one bit may be set. This interrupt has been acknowledged by the APIC
> --
> 1.7.4.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* linux-next: some merging notes
From: Stephen Rothwell @ 2012-12-11 22:15 UTC (permalink / raw)
To: Linus
Cc: N, Mugunthan V, Bill Pemberton, Arnd Bergmann, Greg KH,
Tomi Valkeinen, Rusty Russell, LKML, Steven Rostedt, linux-next,
Paul Mackerras, netdev, Olof Johansson, Nathan Fontenot,
linuxppc-dev, David Miller, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 3273 bytes --]
Hi Linus,
Just some notes about the current state of some of the merges in
linux-next.
The powerpc tree
(git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git#next)
contains a commit that breaks the building of
lib/pSeries-reconfig-notifier-error-inject.c. I applied a patch to
linux-next to disable CONFIG_PSERIES_RECONFIG_NOTIFIER_ERROR_INJECT.
The virtio tree
(git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git#virtio-next)
has a conflict with the net-next tree
(git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git#master)
that requires the following extra fix up patch:
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 33d6f6f..8afe32d 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -147,7 +147,7 @@ struct padded_vnet_hdr {
*/
static int vq2txq(struct virtqueue *vq)
{
- return (virtqueue_get_queue_index(vq) - 1) / 2;
+ return (vq->index - 1) / 2;
}
static int txq2vq(int txq)
@@ -157,7 +157,7 @@ static int txq2vq(int txq)
static int vq2rxq(struct virtqueue *vq)
{
- return virtqueue_get_queue_index(vq) / 2;
+ return vq->index / 2;
}
static int rxq2vq(int rxq)
The tty tree
(git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git#tty-next)
contains a new driver (SystemBase Multi-2/PCI) that fails to build for
(at least) the powerpc architecture. I have applied a patch that
disables the driver (I made CONFIG_SB105X in
drivers/staging/sb105x/Kconfig depend on BROKEN).
The arm-soc tree
(git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git#for-next)
(which you will get in pieces) has a merge conflict against the omap_dss2
tree (git://gitorious.org/linux-omap-dss2/linux.git#for-next) that
requires the followin fix up patch:
diff --git a/arch/arm/mach-omap2/common.c b/arch/arm/mach-omap2/common.c
index 5c2fd48..2dabb9e 100644
--- a/arch/arm/mach-omap2/common.c
+++ b/arch/arm/mach-omap2/common.c
@@ -16,8 +16,6 @@
#include <linux/init.h>
#include <linux/platform_data/dsp-omap.h>
-#include <plat/vram.h>
-
#include "common.h"
#include "omap-secure.h"
@@ -32,7 +30,6 @@ int __weak omap_secure_ram_reserve_memblock(void)
void __init omap_reserve(void)
{
- omap_vram_reserve_sdram_memblock();
omap_dsp_reserve_sdram_memblock();
omap_secure_ram_reserve_memblock();
omap_barrier_reserve_memblock();
Part of the arm-soc tree also requires the following fixup patch due to a
conflict with the powerpc tree (patch from N, Mugunthan V
<mugunthanvnm@ti.com>):
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 099e406..5fd5e23 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -192,7 +192,7 @@ static struct device_node * __init omap_get_timer_dt
(struct of_device_id *match, continue;
}
- prom_add_property(np, &device_disabled);
+ of_add_property(np, &device_disabled);
return np;
}
There are also lots of conflicts due to the __dev* annotation removals -
a lot of which are in the driver-core tree
(git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git#driver-core-next).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply related
* Re: linux-next: some merging notes
From: Benjamin Herrenschmidt @ 2012-12-11 23:19 UTC (permalink / raw)
To: Stephen Rothwell
Cc: N, Mugunthan V, Bill Pemberton, Arnd Bergmann, Greg KH,
linuxppc-dev, Tomi Valkeinen, Rusty Russell, LKML, Steven Rostedt,
linux-next, Paul Mackerras, netdev, Olof Johansson,
Nathan Fontenot, Linus, David Miller, linux-arm-kernel
In-Reply-To: <20121212091552.02c72c8926f9f9147b080d68@canb.auug.org.au>
On Wed, 2012-12-12 at 09:15 +1100, Stephen Rothwell wrote:
> The powerpc tree
> (git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git#next)
> contains a commit that breaks the building of
> lib/pSeries-reconfig-notifier-error-inject.c. I applied a patch to
> linux-next to disable CONFIG_PSERIES_RECONFIG_NOTIFIER_ERROR_INJECT.
I will put a fix in before I send the pull request.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH v2] powerpc: fix wii_memory_fixups() compile error on 3.0.y tree
From: Shuah Khan @ 2012-12-12 0:04 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Greg KH, stable, paulus, shuahkhan, linuxppc-dev
In-Reply-To: <20121210185527.GI13292@decadent.org.uk>
On Mon, 2012-12-10 at 18:55 +0000, Ben Hutchings wrote:
> On Mon, Dec 10, 2012 at 10:23:16AM -0700, Shuah Khan wrote:
> > Fix wii_memory_fixups() the following compile error on 3.0.y tree with
> > wii_defconfig on 3.0.y tree.
> >
> > CC arch/powerpc/platforms/embedded6xx/wii.o
> > arch/powerpc/platforms/embedded6xx/wii.c: In function ‘wii_memory_fixups’:
> > arch/powerpc/platforms/embedded6xx/wii.c:88:2: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘phys_addr_t’ [-Werror=format]
> > arch/powerpc/platforms/embedded6xx/wii.c:88:2: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘phys_addr_t’ [-Werror=format]
> > arch/powerpc/platforms/embedded6xx/wii.c:90:2: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘phys_addr_t’ [-Werror=format]
> > arch/powerpc/platforms/embedded6xx/wii.c:90:2: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘phys_addr_t’ [-Werror=format]
> > cc1: all warnings being treated as errors
> > make[2]: *** [arch/powerpc/platforms/embedded6xx/wii.o] Error 1
> > make[1]: *** [arch/powerpc/platforms/embedded6xx] Error 2
> > make: *** [arch/powerpc/platforms] Error 2
> >
> > Signed-off-by: Shuah Khan <shuah.khan@hp.com>
> > CC: stable@vger.kernel.org 3.0.y
> > ---
> > arch/powerpc/platforms/embedded6xx/wii.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/embedded6xx/wii.c b/arch/powerpc/platforms/embedded6xx/wii.c
> > index 1b5dc1a..d8ff38f 100644
> > --- a/arch/powerpc/platforms/embedded6xx/wii.c
> > +++ b/arch/powerpc/platforms/embedded6xx/wii.c
> > @@ -85,9 +85,11 @@ void __init wii_memory_fixups(void)
> > wii_hole_start = p[0].base + p[0].size;
> > wii_hole_size = p[1].base - wii_hole_start;
> >
> > - pr_info("MEM1: <%08llx %08llx>\n", p[0].base, p[0].size);
> > + pr_info("MEM1: <%08ulx %08ulx>\n",
> > + (phys_addr_t) p[0].base, (phys_addr_t) p[0].size);
> [...]
>
> This is incorrect in exactly the same way as the last version,
> but with extra redundant casts.
>
Yes it is. :) That is embarrassing. The lesson is "don't try to work
when not feeling well", at least that is my excuse.
Thanks for catching it. I will really fix it this time and send a new
patch. This bug is in there since Dec 2009, not sure if this is worth
fixing, but might as well.
de32400dd26e743c5d500aa42d8d6818b79edb73
wii: use both mem1 and mem2 as ram
-- Shuah
^ permalink raw reply
* Re: [PATCH] KVM: PPC: Fix SREGS documentation reference
From: Alexander Graf @ 2012-12-12 0:29 UTC (permalink / raw)
To: Amos Kong; +Cc: Mihai Caraman, linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <CAFeW=pb9Afawnp7hi6Muzs7HU=TVeKDX4RD6Tw0bMkkY9-TTBw@mail.gmail.com>
On 11.12.2012, at 15:22, Amos Kong wrote:
> On Tue, Dec 11, 2012 at 9:38 PM, Mihai Caraman
> <mihai.caraman@freescale.com> wrote:
>>=20
>> Reflect the uapi folder change in SREGS API documentation.
>>=20
>> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>> ---
>> Documentation/virtual/kvm/api.txt | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>=20
>> diff --git a/Documentation/virtual/kvm/api.txt =
b/Documentation/virtual/kvm/api.txt
>> index a4df553..9cf591d 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -345,7 +345,7 @@ struct kvm_sregs {
>> __u64 interrupt_bitmap[(KVM_NR_INTERRUPTS + 63) / 64];
>> };
>>=20
>> -/* ppc -- see arch/powerpc/include/asm/kvm.h */
>> +/* ppc -- see arch/powerpc/include/uapi/asm/kvm.h */
>=20
> Trivial fix.
> Reviewed-by: Amos Kong <kongjianjun@gmail.com>
Thanks, applied to kvm-ppc-next.
Alex
^ permalink raw reply
* Re: Understanding how kernel updates MMU hash table
From: Pegasus11 @ 2012-12-12 5:10 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <34760537.post@talk.nabble.com>
I cannot see my post at all on the old nabble system...this is just for
testing purposes//...
My last post is fine..but I cannot see my thread on linuxppc-dev@oldnabble
...?? :confused:
--
View this message in context: http://old.nabble.com/Understanding-how-kernel-updates-MMU-hash-table-tp34760537p34786958.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.
^ permalink raw reply
* Re: [PATCH] vfio powerpc: enabled on powernv platform
From: Alexey Kardashevskiy @ 2012-12-12 6:14 UTC (permalink / raw)
To: Alex Williamson
Cc: kvm, linux-kernel, Paul Mackerras, linuxppc-dev, David Gibson
In-Reply-To: <1354901926.3224.96.camel@bling.home>
On 08/12/12 04:38, Alex Williamson wrote:
>> +static int __init tce_iommu_init(void)
>> +{
>> + struct pci_dev *pdev = NULL;
>> + struct iommu_table *tbl;
>> + struct iommu_group *grp;
>> +
>> + /* Allocate and initialize IOMMU groups */
>> + for_each_pci_dev(pdev) {
>> + tbl = get_iommu_table_base(&pdev->dev);
>> + if (!tbl)
>> + continue;
>> +
>> + /* Skip already initialized */
>> + if (tbl->it_group)
>> + continue;
>> +
>> + grp = iommu_group_alloc();
>> + if (IS_ERR(grp)) {
>> + pr_info("tce_vfio: cannot create new IOMMU group, ret=%ld\n",
>> + PTR_ERR(grp));
>> + return PTR_ERR(grp);
>> + }
>> + tbl->it_group = grp;
>> + iommu_group_set_iommudata(grp, tbl, group_release);
>
> BTW, groups have a name property that shows up in sysfs that can be set
> with iommu_group_set_name(). IIRC, this was a feature David requested
> for PEs. It'd be nice if it was used for PEs... Thanks,
But what would I put there?... IOMMU ID is more than enough at the moment
and struct iommu_table does not have anything what would have made sense to
show in the sysfs...
--
Alexey
^ permalink raw reply
* Re: [PATCH] vfio powerpc: implemented IOMMU driver for VFIO
From: Alexey Kardashevskiy @ 2012-12-12 6:59 UTC (permalink / raw)
To: Alex Williamson
Cc: kvm, linux-kernel, Paul Mackerras, linuxppc-dev, David Gibson
In-Reply-To: <1354899707.3224.86.camel@bling.home>
On 08/12/12 04:01, Alex Williamson wrote:
>> + case VFIO_IOMMU_MAP_DMA: {
>> + vfio_iommu_spapr_tce_dma_map param;
>> + struct iommu_table *tbl = container->tbl;
>> + enum dma_data_direction direction;
>> + unsigned long locked, lock_limit;
>> +
>> + if (WARN_ON(!tbl))
>> + return -ENXIO;
>> +
>> + minsz = offsetofend(vfio_iommu_spapr_tce_dma_map, size);
>> +
>> + if (copy_from_user(¶m, (void __user *)arg, minsz))
>> + return -EFAULT;
>> +
>> + if (param.argsz < minsz)
>> + return -EINVAL;
>> +
>> + if ((param.flags & VFIO_DMA_MAP_FLAG_READ) &&
>> + (param.flags & VFIO_DMA_MAP_FLAG_WRITE))
>> + direction = DMA_BIDIRECTIONAL;
>> + else if (param.flags & VFIO_DMA_MAP_FLAG_READ)
>> + direction = DMA_TO_DEVICE;
>> + else if (param.flags & VFIO_DMA_MAP_FLAG_WRITE)
>> + direction = DMA_FROM_DEVICE;
>> + else
>> + return -EINVAL;
>
> flags needs to be sanitized too. Return EINVAL if any unknown bit is
> set or else sloppy users may make it very difficult to make use of those
> flag bits later.
It already returns -EINVAL on any bit set except READ/WRITE, no?
--
Alexey
^ permalink raw reply
* Re: pci and pcie device-tree binding - range No cells
From: Michal Simek @ 2012-12-12 10:37 UTC (permalink / raw)
To: Grant Likely
Cc: linux-pci, devicetree-discuss, Thierry Reding, Rob Herring,
Rob Herring, linuxppc-dev
In-Reply-To: <20121210214127.D51773E0796@localhost>
On 12/10/2012 10:41 PM, Grant Likely wrote:
> On Mon, 10 Dec 2012 09:21:51 -0600, Rob Herring <robherring2@gmail.com> wrote:
>> On 12/10/2012 09:05 AM, Michal Simek wrote:
>>> On 12/10/2012 03:26 PM, Rob Herring wrote:
>>>> On 12/10/2012 06:20 AM, Michal Simek wrote:
>>>>> Hi Grant and others,
>>>>>
>>>>> I have a question regarding number of cells in ranges property
>>>>> for pci and pcie nodes.
>>>>>
>>>>> Linux pci/pcie powerpc DTSes contain 7 cells (xpedite5370.dts,
>>>>> sequoia.dts, etc)
>>>>> but also 6 cells format too (mpc832x_mds.dts)
>>>>>
>>>>> Here is shown 6 cells ranges format and describe
>>>>> http://devicetree.org/Device_Tree_Usage#PCI_Host_Bridge
>>>>>
>>>>> And also in documentation in the linux
>>>>> Documentation/devicetree/bindings/pci/83xx-512x-pci.txt
>>>>>
>>>>> Both format uses:
>>>>> #size-cells = <2>;
>>>>> #address-cells = <3>;
>>>>>
>>>>> What is valid format?
>>>>
>>>> Both. 7 cells are valid when the host (parent) bus is 64-bit and 6 cells
>>>> are valid when the host bus is 32-bit. The ranges property is <<child
>>>> address> <parent address> <size>>. The parent address #address-cells is
>>>> taken from the parent node.
>>>
>>> Ok. Got it.
>>>
>>> Here is what we use on zynq and microblaze - both 32bit which should be
>>> fine.
>>>
>>> ps7_axi_interconnect_0: axi@0 {
>>> #address-cells = <1>;
>>> #size-cells = <1>;
>>> axi_pcie_0: axi-pcie@50000000 {
>>> #address-cells = <3>;
>>> #size-cells = <2>;
>>> compatible = "xlnx,axi-pcie-1.05.a";
>>> ranges = < 0x02000000 0 0x60000000 0x60000000 0 0x10000000 >;
>>> ...
>>> }
>>> }
>>>
>>> What I am wondering is pci_process_bridge_OF_ranges() at
>>> arch/powerpc/kernel/pci-common.c
>>> where there are used some hardcoded values which should be probably
>>> loaded from device-tree.
>>>
>>> For example:
>>> 683 int np = pna + 5;
>>> ...
>>> 702 pci_addr = of_read_number(ranges + 1, 2);
>>> 703 cpu_addr = of_translate_address(dev, ranges + 3);
>>> 704 size = of_read_number(ranges + pna + 3, 2);
>>
>> These would always be correct whether you have 6 or 7 cells. pna is the
>> parent bus address cells size. The pci address is fixed at 3 cells.
>>
>>>
>>>
>>> Unfortunately we have copied it to microblaze.
>>
>> I look at the PCI DT code in powerpc and see a whole bunch of code that
>> seems like it should be common. The different per arch pci structs
>> complicates that. No one has really gotten to looking at PCI DT on ARM
>> yet except you and Thierry for Tegra. We definitely don't want to create
>> a 3rd copy. Starting the process of moving it to something like
>> drivers/pci/pci-of.c would be great.
>
> A lot of it should be common. The microblaze code is a copy of the
> powerpc version. I'll strongly nack any attempt to add a third! :-)
Yes it. There are some things which we had fixed because that powerpc
port is big endian only and we support PCIe on little endian too.
But changes are really cosmetic.
> drivers/pci/pci-of.c would be good. I'd also accept drivers/of/pci.c
> which might actually be a good idea in the short term so that it gets
> appropriate supervision while being generalized before being moved into
> the pci directory.
Ben: Are you willing to move that ppc code to this location?
It is probably not good idea that I should do it when I even don't have
hardware available for testing (Asking someone else).
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply
* Re: pci and pcie device-tree binding - range No cells
From: Grant Likely @ 2012-12-12 10:49 UTC (permalink / raw)
To: Michal Simek
Cc: linux-pci, devicetree-discuss, Thierry Reding, Rob Herring,
Rob Herring, linuxppc-dev
In-Reply-To: <50C85E7D.5080006@monstr.eu>
On Wed, Dec 12, 2012 at 10:37 AM, Michal Simek <monstr@monstr.eu> wrote:
> On 12/10/2012 10:41 PM, Grant Likely wrote:
>> drivers/pci/pci-of.c would be good. I'd also accept drivers/of/pci.c
>> which might actually be a good idea in the short term so that it gets
>> appropriate supervision while being generalized before being moved into
>> the pci directory.
>
> Ben: Are you willing to move that ppc code to this location?
> It is probably not good idea that I should do it when I even don't have
> hardware available for testing (Asking someone else).
You're a clever guy, you are more than capable of crafting the patch,
even if you can't test on hardware. :-)
I refactored most of the OF support code without having access to most
of the affected hardware. Once I got the changes out there for review
I also asked for spot testing before getting it into linux-next for
even more testing.
g.
^ permalink raw reply
* Re: pci and pcie device-tree binding - range No cells
From: Andrew Murray @ 2012-12-12 12:19 UTC (permalink / raw)
To: Grant Likely
Cc: Michal Simek, linux-pci, devicetree-discuss, Thierry Reding,
Rob Herring, Rob Herring, linuxppc-dev
In-Reply-To: <CAPcvp5EJH-Q6wd7my+V+FUVE1=hzwMN-yOfHiukGvDmkcoRcsQ@mail.gmail.com>
On Wed, Dec 12, 2012 at 10:49 AM, Grant Likely wrote:
> On Wed, Dec 12, 2012 at 10:37 AM, Michal Simek <monstr@monstr.eu<mailto:m=
onstr@monstr.eu>> wrote:
> > On 12/10/2012 10:41 PM, Grant Likely wrote:
> >> drivers/pci/pci-of.c would be good. I'd also accept drivers/of/pci.c
> >> which might actually be a good idea in the short term so that it gets
> >> appropriate supervision while being generalized before being moved int=
o
> >> the pci directory.
> >
> > Ben: Are you willing to move that ppc code to this location?
> > It is probably not good idea that I should do it when I even don't have
> > hardware available for testing (Asking someone else).
>=20
> You're a clever guy, you are more than capable of crafting the patch,
> even if you can't test on hardware. :-)
>=20
> I refactored most of the OF support code without having access to most
> of the affected hardware. Once I got the changes out there for review
> I also asked for spot testing before getting it into linux-next for
> even more testing.
I've been working on a relatively architecture agnostic PCI host bridge dri=
ver
and also wanted to avoid duplicating more generic DT parsing code for PCI
bindings.
I've ended up with a patch which provides an iterator for returning resourc=
es
based on the the typical 'ranges' binding. This has ended up living in
drivers/of/address.c. I originally started out in drivers/of/pci.c and
drivers/pci/pci-of.c but found there were good (and static) implementations=
in
drivers/of/address.c which can be reused (e.g. of_bus_pci_get_flags,
bus->count_cells).
I'm not just ready to post it - but can do before early next week if you ca=
n
wait.
Andrew Murray
^ permalink raw reply
* [PATCH] vfio powerpc: enabled on powernv platform
From: Alexey Kardashevskiy @ 2012-12-12 12:34 UTC (permalink / raw)
To: Alex Williamson
Cc: kvm, Alexey Kardashevskiy, linux-kernel, Paul Mackerras,
linuxppc-dev, David Gibson
In-Reply-To: <1354901926.3224.96.camel@bling.home>
This patch initializes IOMMU groups based on the IOMMU
configuration discovered during the PCI scan on POWERNV
(POWER non virtualized) platform. The IOMMU groups are
to be used later by VFIO driver (PCI pass through).
It also implements an API for mapping/unmapping pages for
guest PCI drivers and providing DMA window properties.
This API is going to be used later by QEMU-VFIO to handle
h_put_tce hypercalls from the KVM guest.
Although this driver has been tested only on the POWERNV
platform, it should work on any platform which supports
TCE tables.
To enable VFIO on POWER, enable SPAPR_TCE_IOMMU config
option and configure VFIO as required.
Cc: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/include/asm/iommu.h | 10 ++
arch/powerpc/kernel/iommu.c | 329 ++++++++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/pci.c | 134 ++++++++++++++
drivers/iommu/Kconfig | 8 +
4 files changed, 481 insertions(+)
diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index cbfe678..3c861ae 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -76,6 +76,9 @@ struct iommu_table {
struct iommu_pool large_pool;
struct iommu_pool pools[IOMMU_NR_POOLS];
unsigned long *it_map; /* A simple allocation bitmap for now */
+#ifdef CONFIG_IOMMU_API
+ struct iommu_group *it_group;
+#endif
};
struct scatterlist;
@@ -147,5 +150,12 @@ static inline void iommu_restore(void)
}
#endif
+extern void iommu_reset_table(struct iommu_table *tbl, bool restore);
+extern long iommu_clear_tces(struct iommu_table *tbl, unsigned long ioba,
+ unsigned long size);
+extern long iommu_put_tces(struct iommu_table *tbl, unsigned long ioba,
+ uint64_t tce, enum dma_data_direction direction,
+ unsigned long size);
+
#endif /* __KERNEL__ */
#endif /* _ASM_IOMMU_H */
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index ff5a6ce..f3bb2e7 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -36,6 +36,7 @@
#include <linux/hash.h>
#include <linux/fault-inject.h>
#include <linux/pci.h>
+#include <linux/uaccess.h>
#include <asm/io.h>
#include <asm/prom.h>
#include <asm/iommu.h>
@@ -44,6 +45,7 @@
#include <asm/kdump.h>
#include <asm/fadump.h>
#include <asm/vio.h>
+#include <asm/tce.h>
#define DBG(...)
@@ -856,3 +858,330 @@ void iommu_free_coherent(struct iommu_table *tbl, size_t size,
free_pages((unsigned long)vaddr, get_order(size));
}
}
+
+#ifdef CONFIG_IOMMU_API
+/*
+ * SPAPR TCE API
+ */
+
+struct vwork {
+ struct mm_struct *mm;
+ long npage;
+ struct work_struct work;
+};
+
+/* delayed decrement/increment for locked_vm */
+static void lock_acct_bg(struct work_struct *work)
+{
+ struct vwork *vwork = container_of(work, struct vwork, work);
+ struct mm_struct *mm;
+
+ mm = vwork->mm;
+ down_write(&mm->mmap_sem);
+ mm->locked_vm += vwork->npage;
+ up_write(&mm->mmap_sem);
+ mmput(mm);
+ kfree(vwork);
+}
+
+static void lock_acct(long npage)
+{
+ struct vwork *vwork;
+ struct mm_struct *mm;
+
+ if (!current->mm)
+ return; /* process exited */
+
+ if (down_write_trylock(¤t->mm->mmap_sem)) {
+ current->mm->locked_vm += npage;
+ up_write(¤t->mm->mmap_sem);
+ return;
+ }
+
+ /*
+ * Couldn't get mmap_sem lock, so must setup to update
+ * mm->locked_vm later. If locked_vm were atomic, we
+ * wouldn't need this silliness
+ */
+ vwork = kmalloc(sizeof(struct vwork), GFP_KERNEL);
+ if (!vwork)
+ return;
+ mm = get_task_mm(current);
+ if (!mm) {
+ kfree(vwork);
+ return;
+ }
+ INIT_WORK(&vwork->work, lock_acct_bg);
+ vwork->mm = mm;
+ vwork->npage = npage;
+ schedule_work(&vwork->work);
+}
+
+/*
+ * iommu_reset_table is called when it started/stopped being used.
+ *
+ * restore==true says to bring the iommu_table into the state as it was
+ * before being used by VFIO.
+ */
+void iommu_reset_table(struct iommu_table *tbl, bool restore)
+{
+ /* Page#0 is marked as used in iommu_init_table, so we clear it... */
+ if (!restore && (tbl->it_offset == 0))
+ clear_bit(0, tbl->it_map);
+
+ iommu_clear_tces(tbl, tbl->it_offset, tbl->it_size);
+
+ /* ... or restore */
+ if (restore && (tbl->it_offset == 0))
+ set_bit(0, tbl->it_map);
+}
+EXPORT_SYMBOL_GPL(iommu_reset_table);
+
+/*
+ * Returns the number of used IOMMU pages (4K) within
+ * the same system page (4K or 64K).
+ *
+ * syspage_weight_zero is optimized for expected case == 0
+ * syspage_weight_one is optimized for expected case > 1
+ * Other case are not used in this file.
+ */
+#if PAGE_SIZE == IOMMU_PAGE_SIZE
+
+#define syspage_weight_zero(map, offset) test_bit((map), (offset))
+#define syspage_weight_one(map, offset) test_bit((map), (offset))
+
+#elif PAGE_SIZE/IOMMU_PAGE_SIZE == 16
+
+static int syspage_weight_zero(unsigned long *map, unsigned long offset)
+{
+ offset &= PAGE_MASK >> IOMMU_PAGE_SHIFT;
+ return 0xffffUL & (map[BIT_WORD(offset)] >>
+ (offset & (BITS_PER_LONG-1)));
+}
+
+static int syspage_weight_one(unsigned long *map, unsigned long offset)
+{
+ int ret = 0, nbits = PAGE_SIZE/IOMMU_PAGE_SIZE;
+
+ /* Aligns TCE entry number to system page boundary */
+ offset &= PAGE_MASK >> IOMMU_PAGE_SHIFT;
+
+ /* Count used 4K pages */
+ while (nbits && (ret < 2)) {
+ if (test_bit(offset, map))
+ ++ret;
+
+ --nbits;
+ ++offset;
+ }
+
+ return ret;
+}
+#else
+#error TODO: support other page size
+#endif
+
+static void tce_flush(struct iommu_table *tbl)
+{
+ /* Flush/invalidate TLB caches if necessary */
+ if (ppc_md.tce_flush)
+ ppc_md.tce_flush(tbl);
+
+ /* Make sure updates are seen by hardware */
+ mb();
+}
+
+/*
+ * iommu_clear_tces clears tces and returned the number of system pages
+ * which it called put_page() on
+ */
+static long clear_tces_nolock(struct iommu_table *tbl, unsigned long entry,
+ unsigned long pages)
+{
+ int i, retpages = 0, clr;
+ unsigned long oldtce, oldweight;
+ struct page *page;
+
+ for (i = 0; i < pages; ++i, ++entry) {
+ if (!test_bit(entry - tbl->it_offset, tbl->it_map))
+ continue;
+
+ oldtce = ppc_md.tce_get(tbl, entry);
+ ppc_md.tce_free(tbl, entry, 1);
+
+ oldweight = syspage_weight_one(tbl->it_map,
+ entry - tbl->it_offset);
+ clr = __test_and_clear_bit(entry - tbl->it_offset,
+ tbl->it_map);
+
+ if (WARN_ON(!(oldtce & (TCE_PCI_WRITE | TCE_PCI_READ))))
+ continue;
+
+ page = pfn_to_page(oldtce >> PAGE_SHIFT);
+
+ if (WARN_ON(!page))
+ continue;
+
+ if (oldtce & TCE_PCI_WRITE)
+ SetPageDirty(page);
+
+ put_page(page);
+
+ /* That was the last IOMMU page within the system page */
+ if ((oldweight == 1) && clr)
+ ++retpages;
+ }
+
+ return retpages;
+}
+
+/*
+ * iommu_clear_tces clears tces and returned the number
+ * of released system pages
+ */
+long iommu_clear_tces(struct iommu_table *tbl, unsigned long ioba,
+ unsigned long size)
+{
+ int ret;
+ unsigned long entry = ioba >> IOMMU_PAGE_SHIFT;
+ unsigned long npages = size >> IOMMU_PAGE_SHIFT;
+ struct iommu_pool *pool = get_pool(tbl, entry);
+
+ if ((size & ~IOMMU_PAGE_MASK) || (ioba & ~IOMMU_PAGE_MASK))
+ return -EINVAL;
+
+ if ((ioba + size) > ((tbl->it_offset + tbl->it_size)
+ << IOMMU_PAGE_SHIFT))
+ return -EINVAL;
+
+ if (ioba < (tbl->it_offset << IOMMU_PAGE_SHIFT))
+ return -EINVAL;
+
+ spin_lock(&(pool->lock));
+ ret = clear_tces_nolock(tbl, entry, npages);
+ tce_flush(tbl);
+ spin_unlock(&(pool->lock));
+
+ if (ret > 0) {
+ lock_acct(-ret);
+ return 0;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_clear_tces);
+
+static int put_tce(struct iommu_table *tbl, unsigned long entry,
+ uint64_t tce, enum dma_data_direction direction)
+{
+ int ret;
+ struct page *page = NULL;
+ unsigned long kva, offset, oldweight;
+
+ /* Map new TCE */
+ offset = tce & IOMMU_PAGE_MASK & ~PAGE_MASK;
+ ret = get_user_pages_fast(tce & PAGE_MASK, 1,
+ direction != DMA_TO_DEVICE, &page);
+ if (ret != 1) {
+ pr_err("tce_vfio: get_user_pages_fast failed tce=%llx ioba=%lx ret=%d\n",
+ tce, entry << IOMMU_PAGE_SHIFT, ret);
+ return -EFAULT;
+ }
+
+ kva = (unsigned long) page_address(page);
+ kva += offset;
+
+ /* tce_build receives a virtual address */
+ ret = ppc_md.tce_build(tbl, entry, 1, kva, direction, NULL);
+
+ /* tce_build() only returns non-zero for transient errors */
+ if (unlikely(ret)) {
+ pr_err("tce_vfio: tce_put failed on tce=%llx ioba=%lx kva=%lx ret=%d\n",
+ tce, entry << IOMMU_PAGE_SHIFT, kva, ret);
+ put_page(page);
+ return -EIO;
+ }
+
+ /* Calculate if new system page has been locked */
+ oldweight = syspage_weight_zero(tbl->it_map, entry - tbl->it_offset);
+ __set_bit(entry - tbl->it_offset, tbl->it_map);
+
+ return (oldweight == 0) ? 1 : 0;
+}
+
+/*
+ * iommu_put_tces builds tces and returned the number of actually
+ * locked system pages
+ */
+long iommu_put_tces(struct iommu_table *tbl, unsigned long ioba,
+ uint64_t tce, enum dma_data_direction direction,
+ unsigned long size)
+{
+ int i, ret = 0, retpages = 0;
+ unsigned long entry = ioba >> IOMMU_PAGE_SHIFT;
+ unsigned long npages = size >> IOMMU_PAGE_SHIFT;
+ struct iommu_pool *pool = get_pool(tbl, entry);
+ unsigned long locked, lock_limit;
+
+ BUILD_BUG_ON(PAGE_SIZE < IOMMU_PAGE_SIZE);
+ BUG_ON(direction == DMA_NONE);
+
+ if ((size & ~IOMMU_PAGE_MASK) ||
+ (ioba & ~IOMMU_PAGE_MASK) ||
+ (tce & ~IOMMU_PAGE_MASK))
+ return -EINVAL;
+
+ if ((ioba + size) > ((tbl->it_offset + tbl->it_size)
+ << IOMMU_PAGE_SHIFT))
+ return -EINVAL;
+
+ if (ioba < (tbl->it_offset << IOMMU_PAGE_SHIFT))
+ return -EINVAL;
+
+ /* Account for locked pages */
+ locked = current->mm->locked_vm +
+ (_ALIGN_UP(size, PAGE_SIZE) >> PAGE_SHIFT);
+ lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
+ if (locked > lock_limit && !capable(CAP_IPC_LOCK)) {
+ pr_warn("RLIMIT_MEMLOCK (%ld) exceeded\n",
+ rlimit(RLIMIT_MEMLOCK));
+ return -ENOMEM;
+ }
+
+ spin_lock(&(pool->lock));
+
+ /* Check if any is in use */
+ for (i = 0; i < npages; ++i) {
+ if (test_bit(entry + i - tbl->it_offset, tbl->it_map)) {
+ spin_unlock(&(pool->lock));
+ return -EBUSY;
+ }
+ }
+
+ /* Put tces to the table */
+ for (i = 0; (i < npages) && (ret >= 0); ++i, tce += IOMMU_PAGE_SIZE) {
+ ret = put_tce(tbl, entry + i, tce, direction);
+ if (ret == 1)
+ ++retpages;
+ }
+
+ /*
+ * If failed, release locked pages, otherwise return the number
+ * of locked system pages
+ */
+ if (ret < 0) {
+ clear_tces_nolock(tbl, entry, i);
+ } else {
+ if (retpages)
+ lock_acct(retpages);
+ ret = 0;
+ }
+
+ tce_flush(tbl);
+ spin_unlock(&(pool->lock));
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_put_tces);
+
+#endif /* CONFIG_IOMMU_API */
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 05205cf..1b970bf 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -20,6 +20,7 @@
#include <linux/irq.h>
#include <linux/io.h>
#include <linux/msi.h>
+#include <linux/iommu.h>
#include <asm/sections.h>
#include <asm/io.h>
@@ -613,3 +614,136 @@ void __init pnv_pci_init(void)
ppc_md.teardown_msi_irqs = pnv_teardown_msi_irqs;
#endif
}
+
+#ifdef CONFIG_IOMMU_API
+/*
+ * IOMMU groups support required by VFIO
+ */
+static int add_device(struct device *dev)
+{
+ struct iommu_table *tbl;
+ int ret = 0;
+
+ if (WARN_ON(dev->iommu_group)) {
+ pr_warn("tce_vfio: device %s is already in iommu group %d, skipping\n",
+ dev_name(dev),
+ iommu_group_id(dev->iommu_group));
+ return -EBUSY;
+ }
+
+ tbl = get_iommu_table_base(dev);
+ if (!tbl) {
+ pr_debug("tce_vfio: skipping device %s with no tbl\n",
+ dev_name(dev));
+ return 0;
+ }
+
+ pr_debug("tce_vfio: adding %s to iommu group %d\n",
+ dev_name(dev), iommu_group_id(tbl->it_group));
+
+ ret = iommu_group_add_device(tbl->it_group, dev);
+ if (ret < 0)
+ pr_err("tce_vfio: %s has not been added, ret=%d\n",
+ dev_name(dev), ret);
+
+ return ret;
+}
+
+static void del_device(struct device *dev)
+{
+ iommu_group_remove_device(dev);
+}
+
+static int iommu_bus_notifier(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct device *dev = data;
+
+ switch (action) {
+ case BUS_NOTIFY_ADD_DEVICE:
+ return add_device(dev);
+ case BUS_NOTIFY_DEL_DEVICE:
+ del_device(dev);
+ return 0;
+ default:
+ return 0;
+ }
+}
+
+static struct notifier_block tce_iommu_bus_nb = {
+ .notifier_call = iommu_bus_notifier,
+};
+
+static void group_release(void *iommu_data)
+{
+ struct iommu_table *tbl = iommu_data;
+ tbl->it_group = NULL;
+}
+
+static int __init tce_iommu_init(void)
+{
+ struct pci_dev *pdev = NULL;
+ struct iommu_table *tbl;
+ struct iommu_group *grp;
+
+ /* Allocate and initialize IOMMU groups */
+ for_each_pci_dev(pdev) {
+ tbl = get_iommu_table_base(&pdev->dev);
+ if (!tbl)
+ continue;
+
+ /* Skip already initialized */
+ if (tbl->it_group)
+ continue;
+
+ grp = iommu_group_alloc();
+ if (IS_ERR(grp)) {
+ pr_info("tce_vfio: cannot create new IOMMU group, ret=%ld\n",
+ PTR_ERR(grp));
+ return PTR_ERR(grp);
+ }
+ tbl->it_group = grp;
+ iommu_group_set_iommudata(grp, tbl, group_release);
+ }
+
+ bus_register_notifier(&pci_bus_type, &tce_iommu_bus_nb);
+
+ /* Add PCI devices to VFIO groups */
+ for_each_pci_dev(pdev)
+ add_device(&pdev->dev);
+
+ return 0;
+}
+
+static void __exit tce_iommu_cleanup(void)
+{
+ struct pci_dev *pdev = NULL;
+ struct iommu_table *tbl;
+ struct iommu_group *grp = NULL;
+
+ bus_unregister_notifier(&pci_bus_type, &tce_iommu_bus_nb);
+
+ /* Delete PCI devices from VFIO groups */
+ for_each_pci_dev(pdev)
+ del_device(&pdev->dev);
+
+ /* Release VFIO groups */
+ for_each_pci_dev(pdev) {
+ tbl = get_iommu_table_base(&pdev->dev);
+ if (!tbl)
+ continue;
+ grp = tbl->it_group;
+
+ /* Skip (already) uninitialized */
+ if (!grp)
+ continue;
+
+ /* Do actual release, group_release() is expected to work */
+ iommu_group_put(grp);
+ BUG_ON(tbl->it_group);
+ }
+}
+
+module_init(tce_iommu_init);
+module_exit(tce_iommu_cleanup);
+#endif /* CONFIG_IOMMU_API */
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 9f69b56..29d11dc 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -187,4 +187,12 @@ config EXYNOS_IOMMU_DEBUG
Say N unless you need kernel log message for IOMMU debugging
+config SPAPR_TCE_IOMMU
+ bool "sPAPR TCE IOMMU Support"
+ depends on PPC_POWERNV
+ select IOMMU_API
+ help
+ Enables bits of IOMMU API required by VFIO. The iommu_ops is
+ still not implemented.
+
endif # IOMMU_SUPPORT
--
1.7.10.4
^ permalink raw reply related
* [PATCH] vfio powerpc: implemented IOMMU driver for VFIO
From: Alexey Kardashevskiy @ 2012-12-12 12:35 UTC (permalink / raw)
To: Alex Williamson
Cc: kvm, Alexey Kardashevskiy, linux-kernel, Paul Mackerras,
linuxppc-dev, David Gibson
In-Reply-To: <1354899707.3224.86.camel@bling.home>
VFIO implements platform independent stuff such as
a PCI driver, BAR access (via read/write on a file descriptor
or direct mapping when possible) and IRQ signaling.
The platform dependent part includes IOMMU initialization
and handling. This patch implements an IOMMU driver for VFIO
which does mapping/unmapping pages for the guest IO and
provides information about DMA window (required by a POWERPC
guest).
The counterpart in QEMU is required to support this functionality.
Cc: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
drivers/vfio/Kconfig | 6 +
drivers/vfio/Makefile | 1 +
drivers/vfio/vfio_iommu_spapr_tce.c | 249 +++++++++++++++++++++++++++++++++++
include/linux/vfio.h | 31 +++++
4 files changed, 287 insertions(+)
create mode 100644 drivers/vfio/vfio_iommu_spapr_tce.c
diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index 7cd5dec..b464687 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -3,10 +3,16 @@ config VFIO_IOMMU_TYPE1
depends on VFIO
default n
+config VFIO_IOMMU_SPAPR_TCE
+ tristate
+ depends on VFIO && SPAPR_TCE_IOMMU
+ default n
+
menuconfig VFIO
tristate "VFIO Non-Privileged userspace driver framework"
depends on IOMMU_API
select VFIO_IOMMU_TYPE1 if X86
+ select VFIO_IOMMU_SPAPR_TCE if PPC_POWERNV
help
VFIO provides a framework for secure userspace device drivers.
See Documentation/vfio.txt for more details.
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index 2398d4a..72bfabc 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_VFIO) += vfio.o
obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
+obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
obj-$(CONFIG_VFIO_PCI) += pci/
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
new file mode 100644
index 0000000..714bf57
--- /dev/null
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -0,0 +1,249 @@
+/*
+ * VFIO: IOMMU DMA mapping support for TCE on POWER
+ *
+ * Copyright (C) 2012 IBM Corp. All rights reserved.
+ * Author: Alexey Kardashevskiy <aik@ozlabs.ru>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Derived from original vfio_iommu_type1.c:
+ * Copyright (C) 2012 Red Hat, Inc. All rights reserved.
+ * Author: Alex Williamson <alex.williamson@redhat.com>
+ */
+
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/err.h>
+#include <linux/vfio.h>
+#include <asm/iommu.h>
+
+#define DRIVER_VERSION "0.1"
+#define DRIVER_AUTHOR "aik@ozlabs.ru"
+#define DRIVER_DESC "VFIO IOMMU SPAPR TCE"
+
+static void tce_iommu_detach_group(void *iommu_data,
+ struct iommu_group *iommu_group);
+
+/*
+ * VFIO IOMMU fd for SPAPR_TCE IOMMU implementation
+ *
+ * This code handles mapping and unmapping of user data buffers
+ * into DMA'ble space using the IOMMU
+ */
+
+/*
+ * The container descriptor supports only a single group per container.
+ * Required by the API as the container is not supplied with the IOMMU group
+ * at the moment of initialization.
+ */
+struct tce_container {
+ struct mutex lock;
+ struct iommu_table *tbl;
+};
+
+static void *tce_iommu_open(unsigned long arg)
+{
+ struct tce_container *container;
+
+ if (arg != VFIO_SPAPR_TCE_IOMMU) {
+ pr_err("tce_vfio: Wrong IOMMU type\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ container = kzalloc(sizeof(*container), GFP_KERNEL);
+ if (!container)
+ return ERR_PTR(-ENOMEM);
+
+ mutex_init(&container->lock);
+
+ return container;
+}
+
+static void tce_iommu_release(void *iommu_data)
+{
+ struct tce_container *container = iommu_data;
+
+ WARN_ON(container->tbl && !container->tbl->it_group);
+ if (container->tbl && container->tbl->it_group)
+ tce_iommu_detach_group(iommu_data, container->tbl->it_group);
+
+ mutex_destroy(&container->lock);
+
+ kfree(container);
+}
+
+static long tce_iommu_ioctl(void *iommu_data,
+ unsigned int cmd, unsigned long arg)
+{
+ struct tce_container *container = iommu_data;
+ unsigned long minsz;
+ long ret;
+
+ switch (cmd) {
+ case VFIO_CHECK_EXTENSION:
+ return (arg == VFIO_SPAPR_TCE_IOMMU) ? 1 : 0;
+
+ case VFIO_IOMMU_SPAPR_TCE_GET_INFO: {
+ struct vfio_iommu_spapr_tce_info info;
+ struct iommu_table *tbl = container->tbl;
+
+ if (WARN_ON(!tbl))
+ return -ENXIO;
+
+ minsz = offsetofend(struct vfio_iommu_spapr_tce_info,
+ dma32_window_size);
+
+ if (copy_from_user(&info, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (info.argsz < minsz)
+ return -EINVAL;
+
+ info.dma32_window_start = tbl->it_offset << IOMMU_PAGE_SHIFT;
+ info.dma32_window_size = tbl->it_size << IOMMU_PAGE_SHIFT;
+ info.flags = 0;
+
+ if (copy_to_user((void __user *)arg, &info, minsz))
+ return -EFAULT;
+
+ return 0;
+ }
+ case VFIO_IOMMU_MAP_DMA: {
+ vfio_iommu_spapr_tce_dma_map param;
+ struct iommu_table *tbl = container->tbl;
+ enum dma_data_direction direction;
+
+ if (WARN_ON(!tbl))
+ return -ENXIO;
+
+ minsz = offsetofend(vfio_iommu_spapr_tce_dma_map, size);
+
+ if (copy_from_user(¶m, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (param.argsz < minsz)
+ return -EINVAL;
+
+ if ((param.flags & VFIO_DMA_MAP_FLAG_READ) &&
+ (param.flags & VFIO_DMA_MAP_FLAG_WRITE))
+ direction = DMA_BIDIRECTIONAL;
+ else if (param.flags & VFIO_DMA_MAP_FLAG_READ)
+ direction = DMA_TO_DEVICE;
+ else if (param.flags & VFIO_DMA_MAP_FLAG_WRITE)
+ direction = DMA_FROM_DEVICE;
+ else
+ return -EINVAL;
+
+ ret = iommu_put_tces(tbl, param.iova, param.vaddr, direction,
+ param.size);
+
+ return ret;
+ }
+ case VFIO_IOMMU_UNMAP_DMA: {
+ vfio_iommu_spapr_tce_dma_unmap param;
+ struct iommu_table *tbl = container->tbl;
+
+ if (WARN_ON(!tbl))
+ return -ENXIO;
+
+ minsz = offsetofend(vfio_iommu_spapr_tce_dma_unmap, size);
+
+ if (copy_from_user(¶m, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (param.argsz < minsz)
+ return -EINVAL;
+
+ /* No flag is supported now */
+ if (param.flags)
+ return -EINVAL;
+
+ ret = iommu_clear_tces(tbl, param.iova, param.size);
+
+ return ret;
+ }
+ }
+
+ return -ENOTTY;
+}
+
+static int tce_iommu_attach_group(void *iommu_data,
+ struct iommu_group *iommu_group)
+{
+ struct tce_container *container = iommu_data;
+ struct iommu_table *tbl = iommu_group_get_iommudata(iommu_group);
+
+ BUG_ON(!tbl);
+ mutex_lock(&container->lock);
+ pr_debug("tce_vfio: Attaching group #%u to iommu %p\n",
+ iommu_group_id(iommu_group), iommu_group);
+ if (container->tbl) {
+ pr_warn("tce_vfio: Only one group per IOMMU container is allowed, existing id=%d, attaching id=%d\n",
+ iommu_group_id(container->tbl->it_group),
+ iommu_group_id(iommu_group));
+ mutex_unlock(&container->lock);
+ return -EBUSY;
+ }
+
+ container->tbl = tbl;
+ iommu_reset_table(tbl, false);
+ mutex_unlock(&container->lock);
+
+ return 0;
+}
+
+static void tce_iommu_detach_group(void *iommu_data,
+ struct iommu_group *iommu_group)
+{
+ struct tce_container *container = iommu_data;
+ struct iommu_table *tbl = iommu_group_get_iommudata(iommu_group);
+
+ BUG_ON(!tbl);
+ mutex_lock(&container->lock);
+ if (tbl != container->tbl) {
+ pr_warn("tce_vfio: detaching group #%u, expected group is #%u\n",
+ iommu_group_id(iommu_group),
+ iommu_group_id(tbl->it_group));
+ } else {
+
+ pr_debug("tce_vfio: detaching group #%u from iommu %p\n",
+ iommu_group_id(iommu_group), iommu_group);
+
+ container->tbl = NULL;
+ iommu_reset_table(tbl, true);
+ }
+ mutex_unlock(&container->lock);
+}
+
+const struct vfio_iommu_driver_ops tce_iommu_driver_ops = {
+ .name = "iommu-vfio-powerpc",
+ .owner = THIS_MODULE,
+ .open = tce_iommu_open,
+ .release = tce_iommu_release,
+ .ioctl = tce_iommu_ioctl,
+ .attach_group = tce_iommu_attach_group,
+ .detach_group = tce_iommu_detach_group,
+};
+
+static int __init tce_iommu_init(void)
+{
+ return vfio_register_iommu_driver(&tce_iommu_driver_ops);
+}
+
+static void __exit tce_iommu_cleanup(void)
+{
+ vfio_unregister_iommu_driver(&tce_iommu_driver_ops);
+}
+
+module_init(tce_iommu_init);
+module_exit(tce_iommu_cleanup);
+
+MODULE_VERSION(DRIVER_VERSION);
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
+
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 0a4f180..b97697d 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -99,6 +99,7 @@ extern void vfio_unregister_iommu_driver(
/* Extensions */
#define VFIO_TYPE1_IOMMU 1
+#define VFIO_SPAPR_TCE_IOMMU 2
/*
* The IOCTL interface is designed for extensibility by embedding the
@@ -442,4 +443,34 @@ struct vfio_iommu_type1_dma_unmap {
#define VFIO_IOMMU_UNMAP_DMA _IO(VFIO_TYPE, VFIO_BASE + 14)
+/* -------- Additional API for SPAPR TCE (Server POWERPC) IOMMU -------- */
+
+/*
+ * The SPAPR TCE info struct provides the information about the PCI bus
+ * address ranges available for DMA, these values are programmed into
+ * the hardware so the guest has to know that information.
+ *
+ * The DMA 32 bit window start is an absolute PCI bus address.
+ * The IOVA address passed via map/unmap ioctls are absolute PCI bus
+ * addresses too so the window works as a filter rather than an offset
+ * for IOVA addresses.
+ *
+ * A flag will need to be added if other page sizes are supported,
+ * so as defined here, it is always 4k.
+ */
+struct vfio_iommu_spapr_tce_info {
+ __u32 argsz;
+ __u32 flags; /* reserved for future use */
+ __u32 dma32_window_start; /* 32 bit window start (bytes) */
+ __u32 dma32_window_size; /* 32 bit window size (bytes) */
+};
+
+#define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
+
+/* Reuse type1 map/unmap structs as they are the same at the moment */
+typedef struct vfio_iommu_type1_dma_map vfio_iommu_spapr_tce_dma_map;
+typedef struct vfio_iommu_type1_dma_unmap vfio_iommu_spapr_tce_dma_unmap;
+
+/* ***************************************************************** */
+
#endif /* VFIO_H */
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] vfio powerpc: enabled on powernv platform
From: Alexey Kardashevskiy @ 2012-12-12 12:38 UTC (permalink / raw)
To: Alex Williamson
Cc: kvm, Alexey Kardashevskiy, linux-kernel, Paul Mackerras,
linuxppc-dev, David Gibson
In-Reply-To: <1355315657-31153-1-git-send-email-aik@ozlabs.ru>
Hi Alex,
I posted other pair of patches. While debugging and testing my stuff I
implemented some rough hack to support IOMMU mappings without passing those
hypercalls to the QEMU, this is why I moved pieces of code around - want to
support both QEMU-VFIO and kernel optimized H_PUT_TCE handler.
On 12/12/12 23:34, Alexey Kardashevskiy wrote:
> This patch initializes IOMMU groups based on the IOMMU
> configuration discovered during the PCI scan on POWERNV
> (POWER non virtualized) platform. The IOMMU groups are
> to be used later by VFIO driver (PCI pass through).
>
> It also implements an API for mapping/unmapping pages for
> guest PCI drivers and providing DMA window properties.
> This API is going to be used later by QEMU-VFIO to handle
> h_put_tce hypercalls from the KVM guest.
>
> Although this driver has been tested only on the POWERNV
> platform, it should work on any platform which supports
> TCE tables.
>
> To enable VFIO on POWER, enable SPAPR_TCE_IOMMU config
> option and configure VFIO as required.
>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
--
Alexey
^ permalink raw reply
* Re: pci and pcie device-tree binding - range No cells
From: Thierry Reding @ 2012-12-12 13:34 UTC (permalink / raw)
To: Andrew Murray
Cc: Michal Simek, linux-pci, devicetree-discuss, Rob Herring,
Rob Herring, linuxppc-dev
In-Reply-To: <20121212121912.GA2776@arm.com>
[-- Attachment #1: Type: text/plain, Size: 2190 bytes --]
On Wed, Dec 12, 2012 at 12:19:12PM +0000, Andrew Murray wrote:
> On Wed, Dec 12, 2012 at 10:49 AM, Grant Likely wrote:
> > On Wed, Dec 12, 2012 at 10:37 AM, Michal Simek <monstr@monstr.eu<mailto:monstr@monstr.eu>> wrote:
> > > On 12/10/2012 10:41 PM, Grant Likely wrote:
> > >> drivers/pci/pci-of.c would be good. I'd also accept drivers/of/pci.c
> > >> which might actually be a good idea in the short term so that it gets
> > >> appropriate supervision while being generalized before being moved into
> > >> the pci directory.
> > >
> > > Ben: Are you willing to move that ppc code to this location?
> > > It is probably not good idea that I should do it when I even don't have
> > > hardware available for testing (Asking someone else).
> >
> > You're a clever guy, you are more than capable of crafting the patch,
> > even if you can't test on hardware. :-)
> >
> > I refactored most of the OF support code without having access to most
> > of the affected hardware. Once I got the changes out there for review
> > I also asked for spot testing before getting it into linux-next for
> > even more testing.
>
> I've been working on a relatively architecture agnostic PCI host bridge driver
> and also wanted to avoid duplicating more generic DT parsing code for PCI
> bindings.
>
> I've ended up with a patch which provides an iterator for returning resources
> based on the the typical 'ranges' binding. This has ended up living in
> drivers/of/address.c. I originally started out in drivers/of/pci.c and
> drivers/pci/pci-of.c but found there were good (and static) implementations in
> drivers/of/address.c which can be reused (e.g. of_bus_pci_get_flags,
> bus->count_cells).
>
> I'm not just ready to post it - but can do before early next week if you can
> wait.
I already posted a similar patch[0] as part of a larger series to bring
DT support to Tegra PCIe back in July. I suppose what you have must be
something pretty close to that. Most of the stuff that had me occupied
since then should be done soon and I was planning on resurrecting the
series one of these days.
Thierry
[0]: https://patchwork.kernel.org/patch/1244451/
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] vfio powerpc: enabled on powernv platform
From: Alex Williamson @ 2012-12-12 14:34 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: kvm, linux-kernel, Paul Mackerras, linuxppc-dev, David Gibson
In-Reply-To: <50C820AF.8050804@ozlabs.ru>
On Wed, 2012-12-12 at 17:14 +1100, Alexey Kardashevskiy wrote:
> On 08/12/12 04:38, Alex Williamson wrote:
> >> +static int __init tce_iommu_init(void)
> >> +{
> >> + struct pci_dev *pdev = NULL;
> >> + struct iommu_table *tbl;
> >> + struct iommu_group *grp;
> >> +
> >> + /* Allocate and initialize IOMMU groups */
> >> + for_each_pci_dev(pdev) {
> >> + tbl = get_iommu_table_base(&pdev->dev);
> >> + if (!tbl)
> >> + continue;
> >> +
> >> + /* Skip already initialized */
> >> + if (tbl->it_group)
> >> + continue;
> >> +
> >> + grp = iommu_group_alloc();
> >> + if (IS_ERR(grp)) {
> >> + pr_info("tce_vfio: cannot create new IOMMU group, ret=%ld\n",
> >> + PTR_ERR(grp));
> >> + return PTR_ERR(grp);
> >> + }
> >> + tbl->it_group = grp;
> >> + iommu_group_set_iommudata(grp, tbl, group_release);
> >
> > BTW, groups have a name property that shows up in sysfs that can be set
> > with iommu_group_set_name(). IIRC, this was a feature David requested
> > for PEs. It'd be nice if it was used for PEs... Thanks,
>
>
>
> But what would I put there?... IOMMU ID is more than enough at the moment
> and struct iommu_table does not have anything what would have made sense to
> show in the sysfs...
I believe David mentioned that PEs had user visible names. Perhaps they
match an enclosure location or something. Group numbers are rather
arbitrary and really have no guarantee of persistence. Thanks,
Alex
^ permalink raw reply
* Re: [PATCH] vfio powerpc: implemented IOMMU driver for VFIO
From: Alex Williamson @ 2012-12-12 14:36 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: kvm, linux-kernel, Paul Mackerras, linuxppc-dev, David Gibson
In-Reply-To: <50C82B3C.1060006@ozlabs.ru>
On Wed, 2012-12-12 at 17:59 +1100, Alexey Kardashevskiy wrote:
> On 08/12/12 04:01, Alex Williamson wrote:
> >> + case VFIO_IOMMU_MAP_DMA: {
> >> + vfio_iommu_spapr_tce_dma_map param;
> >> + struct iommu_table *tbl = container->tbl;
> >> + enum dma_data_direction direction;
> >> + unsigned long locked, lock_limit;
> >> +
> >> + if (WARN_ON(!tbl))
> >> + return -ENXIO;
> >> +
> >> + minsz = offsetofend(vfio_iommu_spapr_tce_dma_map, size);
> >> +
> >> + if (copy_from_user(¶m, (void __user *)arg, minsz))
> >> + return -EFAULT;
> >> +
> >> + if (param.argsz < minsz)
> >> + return -EINVAL;
> >> +
> >> + if ((param.flags & VFIO_DMA_MAP_FLAG_READ) &&
> >> + (param.flags & VFIO_DMA_MAP_FLAG_WRITE))
> >> + direction = DMA_BIDIRECTIONAL;
> >> + else if (param.flags & VFIO_DMA_MAP_FLAG_READ)
> >> + direction = DMA_TO_DEVICE;
> >> + else if (param.flags & VFIO_DMA_MAP_FLAG_WRITE)
> >> + direction = DMA_FROM_DEVICE;
> >> + else
> >> + return -EINVAL;
> >
> > flags needs to be sanitized too. Return EINVAL if any unknown bit is
> > set or else sloppy users may make it very difficult to make use of those
> > flag bits later.
>
>
> It already returns -EINVAL on any bit set except READ/WRITE, no?
No. I could pass flags ~0 through there to get a read/write mapping and
cause you problems if you later want to define another bit. Thanks,
Alex
^ permalink raw reply
* Re: pci and pcie device-tree binding - range No cells
From: Thomas Petazzoni @ 2012-12-12 16:16 UTC (permalink / raw)
To: Rob Herring
Cc: monstr, Arnd Bergmann, linux-pci, devicetree-discuss,
Thierry Reding, Rob Herring, David Laight, linuxppc-dev
In-Reply-To: <50C66F3C.3010204@gmail.com>
Dear Rob Herring,
On Mon, 10 Dec 2012 17:24:44 -0600, Rob Herring wrote:
> > Marvell SoCs have up to 20 configurable address windows, which allow
> > you, at run time, to say "I would like the range from physical
> > address 0xYYYYYYYY to 0xZZZZZZZZ to correspond to the PCIe device
> > in port 1, lane 2, or to the NAND, or to this or that device".
> > Therefore, in the PCIe driver I proposed for the Armada 370/XP SoCs
> > [1], there is no need to encode all those ranges statically in the
> > DT.
>
> That's not a unique feature. I'm not sure if any powerpc systems do
> that though.
Yes, probably not an unique feature.
> > The only "ranges" property I'm using is to allow the DT sub-nodes
> > describing each PCIe port/lane to access the CPU registers that
> > allow to see if the PCIe link is up or down, access the PCI
> > configuration space and so on. So all ranges in my "ranges"
> > property correspond to normal CPU registers, like the one you would
> > put in the "reg" property for any device. The fact that those
> > devices are PCIe is really orthogonal here.
>
> That doesn't really sound right.
Very likely, but I still don't get what is "the right way".
> I don't think deviating from the normal binding is the right approach.
> Perhaps the host driver should fill in the ranges property with the
> addresses it uses. Then any child devices will get the right address
> translation.
I don't really understand what you mean here. If you look at the host
driver code (arch/arm/mach-mvebu/pcie.c), for each PCIe interface
is simply does:
* Create an address decoding window for the memory BAR
* Create an address decoding window for the I/O BAR
* Associate the memory BAR window address and the I/O bar window
address with the PCIe interface
And that's it. See
https://github.com/MISL-EBU-System-SW/mainline-public/blob/marvell-pcie-v1/arch/arm/mach-mvebu/pcie.c#L107.
So this driver is both "deciding" of the physical addresses for each
PCIe interface, and "associating" them with the PCIe interfaces. How is
it useful to feed some addresses back into the Device Tree?
> Also, while the h/w may support practically any config, there are
> practical constraints of what Linux will use like there's no reason to
> support more than 64K i/o space. PCI memory addresses generally start
> at 0x100000. You probably don't need more than 1 memory window per
> root complex (although prefetchable memory may also be needed).
I allocate one 64K I/O window and one memory window per PCIe interface
whose link is up (i.e a PCIe device is connected).
> You could let the DT settings drive the address window configuration.
No, because I don't want to have absolute addresses for the windows: I
have 10 PCIe interfaces, but often, only a few of them are used. So I
don't want in the Device Tree to over-allocate hundreds of MB of
physical address space if it's not useful.
PCIe is dynamic, address window configuration is dynamic. And we should
hardcode all this configuration statically in the DT? Doesn't seem like
the right solution.
Best regards,
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH] pci: Provide support for parsing PCI DT ranges property
From: Andrew Murray @ 2012-12-12 16:37 UTC (permalink / raw)
To: linux-pci
Cc: Michal Simek, devicetree-discuss, Thierry Reding, Liviu Dudau,
Rob Herring, Rob Herring, linuxppc-dev
DT bindings for PCI host bridges often use the ranges property to describe
memory and IO ranges - this binding tends to be the same across architectur=
es
yet several parsing implementations exist, e.g. arch/mips/pci/pci.c,
arch/powerpc/kernel/pci-common.c, arch/sparc/kernel/pci.c and
arch/microblaze/pci/pci-common.c (clone of PPC). Some of these duplicate
functionality provided by drivers/of/address.c.
This patch provides a common iterator-based parser for the ranges property,=
it
is hoped this will reduce DT representation differences between architectur=
es
and that architectures will migrate in part to this new parser.
It is also hoped (and the motativation for the patch) that this patch will
reduce duplication of code when writing host bridge drivers that are suppor=
ted
by multiple architectures.
This patch provides struct resources from a device tree node, e.g.:
=09u32 *last =3D NULL;
=09struct resource res;
=09while ((last =3D of_pci_process_ranges(np, res, last))) {
=09=09//do something with res
=09}
Platforms with quirks can then do what they like with the resource or migra=
te
common quirk handling to the parser. In an ideal world drivers can just req=
uest
the obtained resources and pass them on (e.g. pci_add_resource_offset).
Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
---
drivers/of/address.c | 53 ++++++++++++++++++++++++++++++++++++++++=
+++-
include/linux/of_address.h | 7 +++++
2 files changed, 59 insertions(+), 1 deletions(-)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 7e262a6..03bfe61 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -219,6 +219,57 @@ int of_pci_address_to_resource(struct device_node *dev=
, int bar,
=09return __of_address_to_resource(dev, addrp, size, flags, NULL, r);
}
EXPORT_SYMBOL_GPL(of_pci_address_to_resource);
+
+const __be32 *of_pci_process_ranges(struct device_node *node,
+=09=09=09=09 struct resource *res, const __be32 *from)
+{
+=09const __be32 *start, *end;
+=09int na, ns, np, pna;
+=09int rlen;
+=09struct of_bus *bus;
+=09WARN_ON(!res);
+
+=09bus =3D of_match_bus(node);
+=09bus->count_cells(node, &na, &ns);
+=09if (!OF_CHECK_COUNTS(na, ns)) {
+=09=09pr_err("Bad cell count for %s\n", node->full_name);
+=09=09return NULL;
+=09}
+
+=09pna =3D of_n_addr_cells(node);
+=09np =3D pna + na + ns;
+
+=09start =3D of_get_property(node, "ranges", &rlen);
+=09if (start =3D=3D NULL)
+=09=09return NULL;
+
+=09end =3D start + rlen;
+
+=09if (!from)
+=09=09from =3D start;
+
+=09while (from + np <=3D end) {
+=09=09u64 cpu_addr, size;
+
+=09=09cpu_addr =3D of_translate_address(node, from + na);
+=09=09size =3D of_read_number(from + na + pna, ns);
+=09=09res->flags =3D bus->get_flags(from);
+=09=09from +=3D np;
+
+=09=09if (cpu_addr =3D=3D OF_BAD_ADDR || size =3D=3D 0)
+=09=09=09continue;
+
+=09=09res->name =3D node->full_name;
+=09=09res->start =3D cpu_addr;
+=09=09res->end =3D res->start + size - 1;
+=09=09res->parent =3D res->child =3D res->sibling =3D NULL;
+=09=09return from;
+=09}
+
+=09return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_process_ranges);
+
#endif /* CONFIG_PCI */
=20
/*
@@ -421,7 +472,7 @@ u64 __of_translate_address(struct device_node *dev, con=
st __be32 *in_addr,
=09=09goto bail;
=09bus =3D of_match_bus(parent);
=20
-=09/* Cound address cells & copy address locally */
+=09/* Count address cells & copy address locally */
=09bus->count_cells(dev, &na, &ns);
=09if (!OF_CHECK_COUNTS(na, ns)) {
=09=09printk(KERN_ERR "prom_parse: Bad cell count for %s\n",
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 01b925a..4582b20 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -26,6 +26,8 @@ static inline unsigned long pci_address_to_pio(phys_addr_=
t addr) { return -1; }
#define pci_address_to_pio pci_address_to_pio
#endif
=20
+const __be32 *of_pci_process_ranges(struct device_node *node,
+=09=09=09=09 struct resource *res, const __be32 *from);
#else /* CONFIG_OF_ADDRESS */
static inline int of_address_to_resource(struct device_node *dev, int inde=
x,
=09=09=09=09=09 struct resource *r)
@@ -48,6 +50,11 @@ static inline const u32 *of_get_address(struct device_no=
de *dev, int index,
{
=09return NULL;
}
+const __be32 *of_pci_process_ranges(struct device_node *node,
+=09=09=09=09 struct resource *res, const __be32 *from)
+{
+=09return NULL;
+}
#endif /* CONFIG_OF_ADDRESS */
=20
=20
--=20
1.7.0.4
^ permalink raw reply related
* Re: pci and pcie device-tree binding - range No cells
From: Andrew Murray @ 2012-12-12 16:44 UTC (permalink / raw)
To: Thierry Reding
Cc: Michal Simek, linux-pci@vger.kernel.org, devicetree-discuss,
Liviu.Dudau, rob.herring@calxeda.com, Rob Herring, linuxppc-dev
In-Reply-To: <20121212133424.GA26280@avionic-0098.adnet.avionic-design.de>
On Wed, Dec 12, 2012 at 01:34:24PM +0000, Thierry Reding wrote:
> On Wed, Dec 12, 2012 at 12:19:12PM +0000, Andrew Murray wrote:
> > I've been working on a relatively architecture agnostic PCI host bridge=
driver
> > and also wanted to avoid duplicating more generic DT parsing code for P=
CI
> > bindings.
> >=20
> > I've ended up with a patch which provides an iterator for returning res=
ources
> > based on the the typical 'ranges' binding. This has ended up living in
> > drivers/of/address.c. I originally started out in drivers/of/pci.c and
> > drivers/pci/pci-of.c but found there were good (and static) implementat=
ions in
> > drivers/of/address.c which can be reused (e.g. of_bus_pci_get_flags,
> > bus->count_cells).
> >=20
> > I'm not just ready to post it - but can do before early next week if yo=
u can
> > wait.
>=20
> I already posted a similar patch[0] as part of a larger series to bring
> DT support to Tegra PCIe back in July. I suppose what you have must be
> something pretty close to that. Most of the stuff that had me occupied
> since then should be done soon and I was planning on resurrecting the
> series one of these days.
Thanks for the reference. I've submitted my patch, it's along the lines of =
your
existing patch.
I'm happy to take the best bits from both, drop mine, etc.
Andrew Murray
^ 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