LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* 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: 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

* 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: [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

* [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

* 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

* 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

* 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: [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

* [stable] [PATCH] powerpc/ptrace: Fix build with gcc 4.6
From: Michael Neuling @ 2012-12-11  3:35 UTC (permalink / raw)
  To: stable; +Cc: linuxppc-dev

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

gcc (rightfully) complains that we are accessing beyond the
end of the fpr array (we do, to access the fpscr).

The only sane thing to do (whether anything in that code can be
called remotely sane is debatable) is to special case fpscr and
handle it as a separate statement.

I initially tried to do it it by making the array access conditional
to index < PT_FPSCR and using a 3rd else leg but for some reason gcc
was unable to understand it and still spewed the warning.

So I ended up with something a tad more intricated but it seems to
build on 32-bit and on 64-bit with and without VSX.

commit e69b742a6793dc5bf16f6eedca534d4bc10d68b2

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Michael Neuling <mikey@neuling.org>
cc: stable@vger.kernel.org (for 3.0 only)

--- 
Mr Stable: This seemed to miss 3.0 stable, please add...

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 05b7dd2..18447c4 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -1497,9 +1497,14 @@ long arch_ptrace(struct task_struct *child, long request,
 		if (index < PT_FPR0) {
 			tmp = ptrace_get_reg(child, (int) index);
 		} else {
+			unsigned int fpidx = index - PT_FPR0;
+
 			flush_fp_to_thread(child);
-			tmp = ((unsigned long *)child->thread.fpr)
-				[TS_FPRWIDTH * (index - PT_FPR0)];
+			if (fpidx < (PT_FPSCR - PT_FPR0))
+				tmp = ((unsigned long *)child->thread.fpr)
+					[fpidx * TS_FPRWIDTH];
+			else
+				tmp = child->thread.fpscr.val;
 		}
 		ret = put_user(tmp, datalp);
 		break;
@@ -1525,9 +1530,14 @@ long arch_ptrace(struct task_struct *child, long request,
 		if (index < PT_FPR0) {
 			ret = ptrace_put_reg(child, index, data);
 		} else {
+			unsigned int fpidx = index - PT_FPR0;
+
 			flush_fp_to_thread(child);
-			((unsigned long *)child->thread.fpr)
-				[TS_FPRWIDTH * (index - PT_FPR0)] = data;
+			if (fpidx < (PT_FPSCR - PT_FPR0))
+				((unsigned long *)child->thread.fpr)
+					[fpidx * TS_FPRWIDTH] = data;
+			else
+				child->thread.fpscr.val = data;
 			ret = 0;
 		}
 		break;

^ permalink raw reply related

* Re: [PATCH 5/6] powerpc: Macros for saving/restore PPR
From: Michael Ellerman @ 2012-12-11  2:41 UTC (permalink / raw)
  To: Haren Myneni; +Cc: paulus, anton, Michael Neuling, linuxppc-dev
In-Reply-To: <50B41C27.4020905@linux.vnet.ibm.com>

On Mon, 2012-11-26 at 17:49 -0800, Haren Myneni wrote:
> On 11/22/2012 07:39 PM, Michael Neuling wrote:
> > Haren Myneni <haren@linux.vnet.ibm.com> wrote:
> > 
> >> [PATCH 5/6] powerpc: Macros for saving/restore PPR
> > 
> > Maybe we should change the names 
> >   HTM_MEDIUM_NO_PPR  => HTM_MEDIUM_PPR_DISCARD   and
> >   HTM_MEDIUM_HAS_PPR => HTM_MEDIUM_PPR_SAVE
> > But now I'm heading into bike shedding territory... plus I think I
> > suggested the names you have currently, so I'm feeling a bit dumb now
> > :-)
> 
> No problem, We can change these macro names if HTM_MEDIUM_PPR_DISCARD/
> HTM_MEDIUM_PPR_SAVE gives better description.

Yes, those names are better.

Even though they're macros they act as functions, so they should
preferably be named using a verb.

cheers

^ permalink raw reply

* Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver.
From: Scott Wood @ 2012-12-11  1:00 UTC (permalink / raw)
  To: Sethi Varun-B16395
  Cc: Wood Scott-B07421, Joerg Roedel, Tabi Timur-B04825,
	linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <C5ECD7A89D1DC44195F34B25E172658D2C09A4@039-SN2MPN1-013.039d.mgd.msft.net>

On 12/10/2012 04:10:06 AM, Sethi Varun-B16395 wrote:
>=20
>=20
> > -----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; =20
> 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; =20
> iommu@lists.linux-
> > > > foundation.org; Wood Scott-B07421; =20
> linuxppc-dev@lists.ozlabs.org;
> > > Tabi
> > > > Timur-B04825
> > > > Subject: Re: [PATCH 3/4 v5] iommu/fsl: Add iommu domain =20
> 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 =20
> [mailto:iommu-
> > > > > > bounces@lists.linux-foundation.org] On Behalf Of Joerg =20
> 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 =20
> 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 =20
> 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 =20
> mechanism?
> > > > > >
> > > > > [Sethi Varun-B16395] A feature mechanism to query the type of
> > > IOMMU?
> > > >
> > > > A feature mechanism to determine whether this subwindow =20
> 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 =20
> windows
> > > would be specified as a part of the geometry and the valid value =20
> 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 =20
> for
> > the "actual number of subwindows"?  How does a user of the =20
> interface find
> > out whether there are any limitations on specifying a value of zero =20
> (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 =20
> this what you get when you read the geometry (iommu_get_attr) after =20
> initializing the domain? In that case the user would know that =20
> implication of setting subwindows to zero with respect to the =20
> aperture size.

So it would default to the maximum aperture size possible with no =20
subwindows?  That might be OK, though is there a way to reset the =20
domain later on to get back to that informational state?

How about finding out the maximum number of subwindows?

> Also, should we introduce an additional API like "iommu_check_attr", =20
> which the user can use to validate the attribute value. For example =20
> in case of geometry, the user can fill up the structure and pass it =20
> to the iommu driver in order to verify the aperture and subwindows =20
> field.

Doesn't the current API raise an error if you do something =20
unsupported?  In any case I don't think making a series of guesses and =20
seeing which ones are accepted is a good substitute for being able to =20
simply read out the relevant parameters.

-Scott=

^ permalink raw reply

* Re: pci and pcie device-tree binding - range No cells
From: Rob Herring @ 2012-12-10 23:24 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: monstr, linux-pci, devicetree-discuss, Thierry Reding,
	Rob Herring, David Laight, linuxppc-dev
In-Reply-To: <20121210181525.3ccd87c6@skate>

On 12/10/2012 11:15 AM, Thomas Petazzoni wrote:
> Dear Michal Simek,
> 
> On Mon, 10 Dec 2012 17:05:13 +0100, Michal Simek wrote:
> 
>> CC: Thomas: I think it will be interesting to see this discussion
>> because you are using size-cell/address-cells equal 1.
>> http://www.spinics.net/lists/arm-kernel/msg211839.html
> 
> Thanks for Cc'ing me.
> 
> The thing is that on Marvell SoCs, we don't need to describe statically
> in the Device Tree the translation between CPU addresses and PCI device
> addresses, because those translations are set up dynamically at run
> time through address decoding windows.
> 
> 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.

> 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.

> Of course, I have no idea if I'm doing a correct usage of the DT, but I
> certainly don't need those 6 values ranges with bits to say if it's I/O
> space, memory space, bus number, device number and so on. The physical
> addresses at which I'm setting up my address decoding windows are
> decided dynamically at runtime, depending on the number of PCIe devices
> that are found in the different PCIe slots (for now, those windows have
> a statically defined size, but I'd ideally would like to size them to
> match exactly the size of the PCIe device memory, in order to avoid
> wasting physical address space, but I haven't found how to get the size
> needed for each PCIe device during the ARM pcibios initialization
> sequence).

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.

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).

You could let the DT settings drive the address window configuration.

Rob

> For those willing to have a look at the PCIe patch set for Armada
> 370/XP, see:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/136455.html.
> The core of the PCIe driver and its DT binding documentation is at
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/136711.html.
> 
> Thanks a lot for your comments,
> 
> Thomas
> 
> [1] that I hope to extend to cover previous Marvell SoCs as well, they
>     work basically the same way
> 

^ permalink raw reply

* Re: pci and pcie device-tree binding - range No cells
From: Mitch Bradley @ 2012-12-10 23:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linux-pci, devicetree-discuss, linuxppc-dev, Rob Herring
In-Reply-To: <1355179097.19932.5.camel@pasglop>

On 12/10/2012 12:38 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2012-12-10 at 21:43 +0000, Grant Likely wrote:
>>> Sorry for my pci ignorance (have never got hw for mb/zynq)
>>> I just want to get better overview how we should we our drivers to
>> be compatible.
>>>
>>> Does it mean that pci is supposed be always 64 bit wide?
>>> And there is no option to have just 32bit values.
>>
>> Yes, PCIe addressing is always 64 bits wide. Even on 32bit PCI systems
>> we use 64 bit PCI addressing in the device tree.
> 
> Right. The size & format of an address cell for PCI is specified in the
> OF PCI bindings and we follow that binding. It's always 3 cells.

.. and the reason why it must be 3 cells, even if the host PCI bus only
supports 32-bit addressing, is because a plug-in PCI card has no way of
knowing what the host supports.


> 
> Cheers,
> Ben.
> 
> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
> 

^ permalink raw reply

* Re: pci and pcie device-tree binding - range No cells
From: Benjamin Herrenschmidt @ 2012-12-10 22:38 UTC (permalink / raw)
  To: Grant Likely
  Cc: monstr, linux-pci, devicetree-discuss, Thierry Reding,
	Rob Herring, Rob Herring, linuxppc-dev
In-Reply-To: <20121210214323.6EA733E0921@localhost>

On Mon, 2012-12-10 at 21:43 +0000, Grant Likely wrote:
> > Sorry for my pci ignorance (have never got hw for mb/zynq)
> > I just want to get better overview how we should we our drivers to
> be compatible.
> > 
> > Does it mean that pci is supposed be always 64 bit wide?
> > And there is no option to have just 32bit values.
> 
> Yes, PCIe addressing is always 64 bits wide. Even on 32bit PCI systems
> we use 64 bit PCI addressing in the device tree.

Right. The size & format of an address cell for PCI is specified in the
OF PCI bindings and we follow that binding. It's always 3 cells.

Cheers,
Ben.

^ permalink raw reply

* Re: pci and pcie device-tree binding - range No cells
From: Grant Likely @ 2012-12-10 21:41 UTC (permalink / raw)
  To: Rob Herring, monstr
  Cc: linux-pci, devicetree-discuss, Thierry Reding, Rob Herring,
	linuxppc-dev
In-Reply-To: <50C5FE0F.3050108@gmail.com>

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!  :-)

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.

g.

^ permalink raw reply

* Re: pci and pcie device-tree binding - range No cells
From: Grant Likely @ 2012-12-10 21:43 UTC (permalink / raw)
  To: monstr, Rob Herring
  Cc: linux-pci, devicetree-discuss, Thierry Reding, Rob Herring,
	linuxppc-dev
In-Reply-To: <50C601B6.2080107@monstr.eu>

On Mon, 10 Dec 2012 16:37:26 +0100, Michal Simek <monstr@monstr.eu> wrote:
> On 12/10/2012 04:21 PM, Rob Herring 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.
> 
> Sorry for my pci ignorance (have never got hw for mb/zynq)
> I just want to get better overview how we should we our drivers to be compatible.
> 
> Does it mean that pci is supposed be always 64 bit wide?
> And there is no option to have just 32bit values.

Yes, PCIe addressing is always 64 bits wide. Even on 32bit PCI systems
we use 64 bit PCI addressing in the device tree.

g.

^ permalink raw reply

* Re: [RFC] Add IBM Blue Gene/Q Platform
From: Jimi Xenidis @ 2012-12-10 21:33 UTC (permalink / raw)
  To: Andrew Tauferner
  Cc: Michael Neuling, Kumar Gala, Jay Bryant, Todd Inglett,
	linuxppc-dev
In-Reply-To: <A2E5C016-9052-49EB-97E4-DB06C7B6DDC1@pobox.com>


On Dec 10, 2012, at 3:32 PM, Jimi Xenidis <jimix@pobox.com> wrote:

>=20
> On Dec 7, 2012, at 8:31 AM, Andrew Tauferner <ataufer@us.ibm.com> =
wrote:
>=20
>> Jimi,
>>=20
>>>> Do you actually want this upstream?  I assume no.
>>>=20
>>> I needed to get these long-term patches out there for the BGQ=20
>>> community for test.
>>=20
>> To which BGQ community are you referring?
>=20
> This work is being done by IBM Research (me) and made possible by the =
US-DOE via:
>  <https://sites.google.com/site/foxxstack/project-updates>
>=20
>> What is the motivation for this work?
>=20
> Maintain a modern kernel that has a "reasonable" set of patches that, =
with the much appreciated feedback from the Linux community, _could_ go =
upstream.
>=20
>>=20
>>> I would very much like to get a version of these upstream.
>>> I expect only the QPX, kexec, and (maybe) the DCR changes to cause=20=

>>> any controversy, but I've been wrong before.
>>=20
>> Hehe.  Ben had a variety of issues with the BG/Q firmware when he=20
>> gave me some feedback a few years ago.
>=20
> Yup, I think I have addressed these issues.
>=20
>>=20
>>> I'll be making those patches soon and hope to get a lot of feedback=20=

>>> from these patches.
>>=20
>> What was the starting point for this work?  On what is it based?
>=20
>  =
<https://repo.anl-external.org/viewvc/bgq-driver/V1R1M2/?sortdir=3Ddown>>

I should add that these patches have been heavily modified to meet =
requirements and suggestions of BenH and others.
-jx


>=20
> -jx
>=20
>=20
>>=20
>>> -jx
>>>=20
>>>=20
>>>>=20
>>>> Mikey
>>>>=20
>>>>>=20
>>>>> Here is a are the summary logs:
>>>>>=20
>>>>> $ git log --reverse linux-stable/linux-3.4.y..
>>>>> commit 5a8edb2bdd914597693eed299119ff4c2e6d31f2
>>>>> Author: Jimi Xenidis <jimix@pobox.com>
>>>>> Date:   Fri Nov 9 09:26:00 2012 -0600
>>>>>=20
>>>>>   powerpc: Fix cputable #ifdef where CONFIG_PPC_A2 is used for=20
>>> CONFIG_PPC_BOOK3E_64
>>>>>=20
>>>>>   Signed-off-by: Jimi Xenidis <jimix@pobox.com>
>>>>>=20
>>>>> commit ea51920d7035c8d23801d6de46261e7d0a537dfd
>>>>> Author: Jimi Xenidis <jimix@pobox.com>
>>>>> Date:   Fri Nov 9 08:58:27 2012 -0600
>>>>>=20
>>>>>   powerpc/book3e: Remove config for PPC_A2_DD2 since there is no
>>> reference to it
>>>>>=20
>>>>>   This must have been leftover from early DD1 days which is not
>>>>>   present in any current kernel code.
>>>>>=20
>>>>>   Signed-off-by: Jimi Xenidis <jimix@pobox.com>
>>>>>=20
>>>>> commit 08151401a5db4ff0d441a1b7bf8ad92bd92b14c5
>>>>> Author: Jimi Xenidis <jimix@pobox.com>
>>>>> Date:   Mon Nov 5 09:38:01 2012 -0600
>>>>>=20
>>>>>   powerpc/dcr: Some native DCR fixes
>>>>>=20
>>>>>   The following fixes have been made:
>>>>>    - dcr_read/write_native() must use the indexed version of the
>>>>>      m[ft]dcrx since the non-indexed version only allows a 10-bit
>>>>>      numerical space, but the C interface allows a full 32-bits.
>>>>>    - C bindings for m[ft]dcrx, and the "table" versions, should =
use
>>>>>      "unsigned long" so that they are 64/32 bit neutral.
>>>>>    - The "table" versions (__m[ft]cdr) should obtain the table =
address
>>>>>      with LOAD_REG_ADDR(), this will also make it 64/32bit =
neutral.
>>>>>=20
>>>>>   Signed-off-by: Jimi Xenidis <jimix@pobox.com>
>>>>>=20
>>>>> commit c8320a5daaceed03992d763302020834ea8e17dd
>>>>> Author: Jimi Xenidis <jimix@pobox.com>
>>>>> Date:   Mon Nov 5 09:12:00 2012 -0600
>>>>>=20
>>>>>   powerpc/dcr: Add 64-bit DCR access methods.
>>>>>=20
>>>>>   This patch adds the ability to make 64-bit Device Control =
Register
>>>>>   (DCR) accesses.
>>>>>=20
>>>>>   Signed-off-by: Jimi Xenidis <jimix@pobox.com>
>>>>>=20
>>>>> commit a763b3f8453b3bd83d7dded8c6644939863af430
>>>>> Author: Jimi Xenidis <jimix@pobox.com>
>>>>> Date:   Thu Nov 29 12:49:24 2012 -0500
>>>>>=20
>>>>>   powerpc/boot: Add a "spin_threads" hook to platform_ops
>>>>>=20
>>>>>   It is useful for the boot program to arrange for all secondary =
cpus
>>>>>   and threads to enter the kernel in a "kexec" fashion.  This hook =
makes
>>>>>   it possible.
>>>>>=20
>>>>>   Signed-off-by: Jimi Xenidis <jimix@pobox.com>
>>>>>=20
>>>>> commit 391e43393380b514d4d02a42d059619542c7597b
>>>>> Author: Jimi Xenidis <jimix@pobox.com>
>>>>> Date:   Thu Nov 29 13:01:23 2012 -0500
>>>>>=20
>>>>>   powerpc/kexec: Add kexec "hold" support for Book3e processors
>>>>>=20
>>>>>   This patch add two items:
>>>>>   1) Book3e requires that GPR4 survive the "hold" process, so we =
make
>>>>>      sure that happens.
>>>>>   2) Book3e has no real mode, and the hold code exploits this.  =
Since
>>>>>      these processors ares always translated, we arrange for the =
kexeced
>>>>>      threads to enter the hold code using the normal kernel=20
>>> linear mapping.
>>>>>=20
>>>>>   Signed-off-by: Jimi Xenidis <jimix@pobox.com>
>>>>>=20
>>>>> commit f6e3c1f706cb6922349d639a74ff6c50acc8b9f8
>>>>> Author: Jimi Xenidis <jimix@pobox.com>
>>>>> Date:   Wed Dec 5 13:41:25 2012 -0500
>>>>>=20
>>>>>   powerpc: Remove unecessary VSX symbols
>>>>>=20
>>>>>   The symbol THREAD_VSR0 is defined to be the same as THREAD_FPR0. =
 Its
>>>>>   presence causes build issues with more complex configurations.
>>>>>=20
>>>>>   Signed-off-by: Jimi Xenidis <jimix@pobox.com>
>>>>>=20
>>>>> commit 4e817bb42ec8e3d3689877528dd97c4286a870eb
>>>>> Author: Jimi Xenidis <jimix@pobox.com>
>>>>> Date:   Tue Nov 20 10:10:52 2012 -0600
>>>>>=20
>>>>>   Blue Gene/Q wicked optimizing compiler does not know the rfdi=20
>>> instruction yet
>>>>>=20
>>>>>   Signed-off-by: Jimi Xenidis <jimix@pobox.com>
>>>>>=20
>>>>> commit 2071aa58b2f3b33d97c94e3a127f7c5d4ffaeb34
>>>>> Author: Jimi Xenidis <jimix@pobox.com>
>>>>> Date:   Tue Nov 20 10:14:22 2012 -0600
>>>>>=20
>>>>>   Blue Gene/Q wicked optimizing compiler does not know the=20
>>> mfdcrx instruction yet
>>>>>=20
>>>>>   Signed-off-by: Jimi Xenidis <jimix@pobox.com>
>>>>>=20
>>>>> commit 6e58088fabedbb2d724637b539ba180c03ed8b68
>>>>> Author: Jimi Xenidis <jimix@pobox.com>
>>>>> Date:   Wed Oct 31 16:33:21 2012 -0500
>>>>>=20
>>>>>   powerpc/book3e: IBM Blue Gene/Q Boot
>>>>>=20
>>>>>   This patch specifically deals with the initial program load
>>>>>   environment so that a boot image (dtbImage.bgq) can be loaded by =
the
>>>>>   BGQ management tools.  The boot code is a little odd because it =
has to
>>>>>   deal with the following issues:
>>>>>    - Linux boot image wrappers are 32-bit programs
>>>>>    - BGQ Tools only load 64bit ELF programs
>>>>>    - BGQ Firmware information is typically loaded at an address > =
4G
>>>>>    - BGQ FW information contains 64-bit ABI function pointers =
(which are
>>>>>      actually function descriptors) to access firmware methods
>>>>>    - BGQ FW methods must be called in 64-bit mode
>>>>>=20
>>>>>   Includes code contributed from:
>>>>>     Andrew Tauferner <ataufer@us.ibm.com>
>>>>>     Todd Inglett <tinglett@us.ibm.com>
>>>>>     Eric Van Hensbergen <ericvh@gmail.com>
>>>>>=20
>>>>>   Signed-off-by: Jimi Xenidis <jimix@pobox.com>
>>>>>=20
>>>>> commit 3bc841935eb4955ce2b2db69bff16f7928464597
>>>>> Author: Jimi Xenidis <jimix@pobox.com>
>>>>> Date:   Wed Oct 31 22:36:54 2012 -0500
>>>>>=20
>>>>>   powerpc/book3e: IBM Blue Gene/Q Platform and SMP
>>>>>=20
>>>>>   This patch introduces BGQ as a platform and adds SMP =
functionality
>>>>>=20
>>>>>   Includes code contributed from:
>>>>>     Andrew Tauferner <ataufer@us.ibm.com>
>>>>>     Todd Inglett <tinglett@us.ibm.com>
>>>>>     Eric Van Hensbergen <ericvh@gmail.com>
>>>>>=20
>>>>>   Signed-off-by: Jimi Xenidis <jimix@pobox.com>
>>>>>=20
>>>>> commit 279c0615917b959a652e81f4ad0d886e2d426d85
>>>>> Author: Jimi Xenidis <jimix@pobox.com>
>>>>> Date:   Wed Dec 5 13:43:22 2012 -0500
>>>>>=20
>>>>>   powerpc/book3e: IBM Blue Gene/Q Quad Processing eXtention (QPX)
>>>>>=20
>>>>>   This enables kernel support for the QPX extention and is =
intended for
>>>>>   processors that support it, usually an IBM Blue Gene processor.
>>>>>   Turning it on does not effect other processors but it does add =
code
>>>>>   and will quadruple the per thread save and restore area for the =
FPU
>>>>>   (hense the name).  If you have enabled VSX it will only double =
the
>>>>>   space.
>>>>>=20
>>>>>   Signed-off-by: Jimi Xenidis <jimix@pobox.com>
>>>>>=20
>>>>> commit 6ff45170ab0463fb34d7011e08c7e47c396f0fd7
>>>>> Author: Jimi Xenidis <jimix@pobox.com>
>>>>> Date:   Fri Nov 23 14:52:14 2012 -0600
>>>>>=20
>>>>>   powerpc/book3e: IBM Blue Gene/Q HVC Based Firmware Console
>>>>>=20
>>>>>   New HVC device that uses the Blue Gene Firmware methods to erad =
and
>>>>>   write to console.
>>>>>=20
>>>>>       Includes code contributed from:
>>>>>         Andrew Tauferner <ataufer@us.ibm.com>
>>>>>         Todd Inglett <tinglett@us.ibm.com>
>>>>>         Eric Van Hensbergen <ericvh@gmail.com>
>>>>>=20
>>>>>   Signed-off-by: Jimi Xenidis <jimix@pobox.com>
>>>>>=20
>>>>> commit e4ddc0c2ad8b3f0260d15d73e153095e95da84ac
>>>>> Author: Jimi Xenidis <jimix@pobox.com>
>>>>> Date:   Thu Nov 29 15:52:04 2012 -0500
>>>>>=20
>>>>>   powerpc/book3e: IBM Blue Gene/Q PCIe and MSI
>>>>>=20
>>>>>   The following patch adds support for the BG/Q PCIe bridge and=20
>>> MSI interrupts.
>>>>>=20
>>>>>   Includes code contributed from:
>>>>>     Jay S. Bryant <jsbryant@us.ibm.com>
>>>>>     Eric Van Hensbergen <ericvh@gmail.com>
>>>>>=20
>>>>>   Signed-off-by: Jimi Xenidis <jimix@pobox.com>
>>>>>=20
>>>>> commit 9fc0b6f729f7bd7e31338283640a718fa4b1693b
>>>>> Author: Jimi Xenidis <jimix@pobox.com>
>>>>> Date:   Wed Dec 5 07:01:49 2012 -0500
>>>>>=20
>>>>>   powerpc/book3e: IBM Blue Gene/Q Character Drivers
>>>>>=20
>>>>>   The following patch adds support for user and administration
>>>>>   applications to access the BG/Q control system.
>>>>>=20
>>>>>   Includes code contributed from:
>>>>>     Jay S. Bryant <jsbryant@us.ibm.com>
>>>>>     Eric Van Hensbergen <ericvh@gmail.com>
>>>>>=20
>>>>>   Signed-off-by: Jimi Xenidis <jimix@pobox.com>
>>>>>=20
>>>>> commit 9df2c4dfde0ac75f8b2bfcf565f78c2b7382b031
>>>>> Author: Jimi Xenidis <jimix@pobox.com>
>>>>> Date:   Thu Dec 6 18:07:16 2012 -0500
>>>>>=20
>>>>>   Linux 3.4.22-BGQ-rc1
>>>>>=20
>>>>>=20
>>>>>=20
>>>>>=20
>>>>> _______________________________________________
>>>>> Linuxppc-dev mailing list
>>>>> Linuxppc-dev@lists.ozlabs.org
>>>>> https://lists.ozlabs.org/listinfo/linuxppc-dev
>>>>>=20
>>>=20
>>=20
>=20

^ permalink raw reply

* Re: [RFC] Add IBM Blue Gene/Q Platform
From: Jimi Xenidis @ 2012-12-10 21:32 UTC (permalink / raw)
  To: Andrew Tauferner
  Cc: Michael Neuling, Kumar Gala, Jay Bryant, Todd Inglett,
	linuxppc-dev
In-Reply-To: <OFD0D3396A.6EC5ADFF-ON86257ACD.004F1662-86257ACD.004FC5B0@us.ibm.com>


On Dec 7, 2012, at 8:31 AM, Andrew Tauferner <ataufer@us.ibm.com> wrote:

> Jimi,
>=20
> > > Do you actually want this upstream?  I assume no.
> >=20
> > I needed to get these long-term patches out there for the BGQ=20
> > community for test.
>=20
> To which BGQ community are you referring?

This work is being done by IBM Research (me) and made possible by the =
US-DOE via:
  <https://sites.google.com/site/foxxstack/project-updates>

>  What is the motivation for this work?

Maintain a modern kernel that has a "reasonable" set of patches that, =
with the much appreciated feedback from the Linux community, _could_ go =
upstream.

>=20
> > I would very much like to get a version of these upstream.
> > I expect only the QPX, kexec, and (maybe) the DCR changes to cause=20=

> > any controversy, but I've been wrong before.
>=20
> Hehe.  Ben had a variety of issues with the BG/Q firmware when he=20
> gave me some feedback a few years ago.

Yup, I think I have addressed these issues.

>=20
> > I'll be making those patches soon and hope to get a lot of feedback=20=

> > from these patches.
>=20
> What was the starting point for this work?  On what is it based?

  =
<https://repo.anl-external.org/viewvc/bgq-driver/V1R1M2/?sortdir=3Ddown>>

-jx


>=20
> > -jx
> >=20
> >=20
> > >=20
> > > Mikey
> > >=20
> > >>=20
> > >> Here is a are the summary logs:
> > >>=20
> > >> $ git log --reverse linux-stable/linux-3.4.y..
> > >> commit 5a8edb2bdd914597693eed299119ff4c2e6d31f2
> > >> Author: Jimi Xenidis <jimix@pobox.com>
> > >> Date:   Fri Nov 9 09:26:00 2012 -0600
> > >>=20
> > >>    powerpc: Fix cputable #ifdef where CONFIG_PPC_A2 is used for=20=

> > CONFIG_PPC_BOOK3E_64
> > >>=20
> > >>    Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> > >>=20
> > >> commit ea51920d7035c8d23801d6de46261e7d0a537dfd
> > >> Author: Jimi Xenidis <jimix@pobox.com>
> > >> Date:   Fri Nov 9 08:58:27 2012 -0600
> > >>=20
> > >>    powerpc/book3e: Remove config for PPC_A2_DD2 since there is no
> > reference to it
> > >>=20
> > >>    This must have been leftover from early DD1 days which is not
> > >>    present in any current kernel code.
> > >>=20
> > >>    Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> > >>=20
> > >> commit 08151401a5db4ff0d441a1b7bf8ad92bd92b14c5
> > >> Author: Jimi Xenidis <jimix@pobox.com>
> > >> Date:   Mon Nov 5 09:38:01 2012 -0600
> > >>=20
> > >>    powerpc/dcr: Some native DCR fixes
> > >>=20
> > >>    The following fixes have been made:
> > >>     - dcr_read/write_native() must use the indexed version of the
> > >>       m[ft]dcrx since the non-indexed version only allows a =
10-bit
> > >>       numerical space, but the C interface allows a full 32-bits.
> > >>     - C bindings for m[ft]dcrx, and the "table" versions, should =
use
> > >>       "unsigned long" so that they are 64/32 bit neutral.
> > >>     - The "table" versions (__m[ft]cdr) should obtain the table =
address
> > >>       with LOAD_REG_ADDR(), this will also make it 64/32bit =
neutral.
> > >>=20
> > >>    Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> > >>=20
> > >> commit c8320a5daaceed03992d763302020834ea8e17dd
> > >> Author: Jimi Xenidis <jimix@pobox.com>
> > >> Date:   Mon Nov 5 09:12:00 2012 -0600
> > >>=20
> > >>    powerpc/dcr: Add 64-bit DCR access methods.
> > >>=20
> > >>    This patch adds the ability to make 64-bit Device Control =
Register
> > >>    (DCR) accesses.
> > >>=20
> > >>    Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> > >>=20
> > >> commit a763b3f8453b3bd83d7dded8c6644939863af430
> > >> Author: Jimi Xenidis <jimix@pobox.com>
> > >> Date:   Thu Nov 29 12:49:24 2012 -0500
> > >>=20
> > >>    powerpc/boot: Add a "spin_threads" hook to platform_ops
> > >>=20
> > >>    It is useful for the boot program to arrange for all secondary =
cpus
> > >>    and threads to enter the kernel in a "kexec" fashion.  This =
hook makes
> > >>    it possible.
> > >>=20
> > >>    Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> > >>=20
> > >> commit 391e43393380b514d4d02a42d059619542c7597b
> > >> Author: Jimi Xenidis <jimix@pobox.com>
> > >> Date:   Thu Nov 29 13:01:23 2012 -0500
> > >>=20
> > >>    powerpc/kexec: Add kexec "hold" support for Book3e processors
> > >>=20
> > >>    This patch add two items:
> > >>    1) Book3e requires that GPR4 survive the "hold" process, so we =
make
> > >>       sure that happens.
> > >>    2) Book3e has no real mode, and the hold code exploits this.  =
Since
> > >>       these processors ares always translated, we arrange for the =
kexeced
> > >>       threads to enter the hold code using the normal kernel=20
> > linear mapping.
> > >>=20
> > >>    Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> > >>=20
> > >> commit f6e3c1f706cb6922349d639a74ff6c50acc8b9f8
> > >> Author: Jimi Xenidis <jimix@pobox.com>
> > >> Date:   Wed Dec 5 13:41:25 2012 -0500
> > >>=20
> > >>    powerpc: Remove unecessary VSX symbols
> > >>=20
> > >>    The symbol THREAD_VSR0 is defined to be the same as =
THREAD_FPR0.  Its
> > >>    presence causes build issues with more complex configurations.
> > >>=20
> > >>    Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> > >>=20
> > >> commit 4e817bb42ec8e3d3689877528dd97c4286a870eb
> > >> Author: Jimi Xenidis <jimix@pobox.com>
> > >> Date:   Tue Nov 20 10:10:52 2012 -0600
> > >>=20
> > >>    Blue Gene/Q wicked optimizing compiler does not know the rfdi=20=

> > instruction yet
> > >>=20
> > >>    Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> > >>=20
> > >> commit 2071aa58b2f3b33d97c94e3a127f7c5d4ffaeb34
> > >> Author: Jimi Xenidis <jimix@pobox.com>
> > >> Date:   Tue Nov 20 10:14:22 2012 -0600
> > >>=20
> > >>    Blue Gene/Q wicked optimizing compiler does not know the=20
> > mfdcrx instruction yet
> > >>=20
> > >>    Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> > >>=20
> > >> commit 6e58088fabedbb2d724637b539ba180c03ed8b68
> > >> Author: Jimi Xenidis <jimix@pobox.com>
> > >> Date:   Wed Oct 31 16:33:21 2012 -0500
> > >>=20
> > >>    powerpc/book3e: IBM Blue Gene/Q Boot
> > >>=20
> > >>    This patch specifically deals with the initial program load
> > >>    environment so that a boot image (dtbImage.bgq) can be loaded =
by the
> > >>    BGQ management tools.  The boot code is a little odd because =
it has to
> > >>    deal with the following issues:
> > >>     - Linux boot image wrappers are 32-bit programs
> > >>     - BGQ Tools only load 64bit ELF programs
> > >>     - BGQ Firmware information is typically loaded at an address =
> 4G
> > >>     - BGQ FW information contains 64-bit ABI function pointers =
(which are
> > >>       actually function descriptors) to access firmware methods
> > >>     - BGQ FW methods must be called in 64-bit mode
> > >>=20
> > >>    Includes code contributed from:
> > >>      Andrew Tauferner <ataufer@us.ibm.com>
> > >>      Todd Inglett <tinglett@us.ibm.com>
> > >>      Eric Van Hensbergen <ericvh@gmail.com>
> > >>=20
> > >>    Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> > >>=20
> > >> commit 3bc841935eb4955ce2b2db69bff16f7928464597
> > >> Author: Jimi Xenidis <jimix@pobox.com>
> > >> Date:   Wed Oct 31 22:36:54 2012 -0500
> > >>=20
> > >>    powerpc/book3e: IBM Blue Gene/Q Platform and SMP
> > >>=20
> > >>    This patch introduces BGQ as a platform and adds SMP =
functionality
> > >>=20
> > >>    Includes code contributed from:
> > >>      Andrew Tauferner <ataufer@us.ibm.com>
> > >>      Todd Inglett <tinglett@us.ibm.com>
> > >>      Eric Van Hensbergen <ericvh@gmail.com>
> > >>=20
> > >>    Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> > >>=20
> > >> commit 279c0615917b959a652e81f4ad0d886e2d426d85
> > >> Author: Jimi Xenidis <jimix@pobox.com>
> > >> Date:   Wed Dec 5 13:43:22 2012 -0500
> > >>=20
> > >>    powerpc/book3e: IBM Blue Gene/Q Quad Processing eXtention =
(QPX)
> > >>=20
> > >>    This enables kernel support for the QPX extention and is =
intended for
> > >>    processors that support it, usually an IBM Blue Gene =
processor.
> > >>    Turning it on does not effect other processors but it does add =
code
> > >>    and will quadruple the per thread save and restore area for =
the FPU
> > >>    (hense the name).  If you have enabled VSX it will only double =
the
> > >>    space.
> > >>=20
> > >>    Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> > >>=20
> > >> commit 6ff45170ab0463fb34d7011e08c7e47c396f0fd7
> > >> Author: Jimi Xenidis <jimix@pobox.com>
> > >> Date:   Fri Nov 23 14:52:14 2012 -0600
> > >>=20
> > >>    powerpc/book3e: IBM Blue Gene/Q HVC Based Firmware Console
> > >>=20
> > >>    New HVC device that uses the Blue Gene Firmware methods to =
erad and
> > >>    write to console.
> > >>=20
> > >>        Includes code contributed from:
> > >>          Andrew Tauferner <ataufer@us.ibm.com>
> > >>          Todd Inglett <tinglett@us.ibm.com>
> > >>          Eric Van Hensbergen <ericvh@gmail.com>
> > >>=20
> > >>    Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> > >>=20
> > >> commit e4ddc0c2ad8b3f0260d15d73e153095e95da84ac
> > >> Author: Jimi Xenidis <jimix@pobox.com>
> > >> Date:   Thu Nov 29 15:52:04 2012 -0500
> > >>=20
> > >>    powerpc/book3e: IBM Blue Gene/Q PCIe and MSI
> > >>=20
> > >>    The following patch adds support for the BG/Q PCIe bridge and=20=

> > MSI interrupts.
> > >>=20
> > >>    Includes code contributed from:
> > >>      Jay S. Bryant <jsbryant@us.ibm.com>
> > >>      Eric Van Hensbergen <ericvh@gmail.com>
> > >>=20
> > >>    Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> > >>=20
> > >> commit 9fc0b6f729f7bd7e31338283640a718fa4b1693b
> > >> Author: Jimi Xenidis <jimix@pobox.com>
> > >> Date:   Wed Dec 5 07:01:49 2012 -0500
> > >>=20
> > >>    powerpc/book3e: IBM Blue Gene/Q Character Drivers
> > >>=20
> > >>    The following patch adds support for user and administration
> > >>    applications to access the BG/Q control system.
> > >>=20
> > >>    Includes code contributed from:
> > >>      Jay S. Bryant <jsbryant@us.ibm.com>
> > >>      Eric Van Hensbergen <ericvh@gmail.com>
> > >>=20
> > >>    Signed-off-by: Jimi Xenidis <jimix@pobox.com>
> > >>=20
> > >> commit 9df2c4dfde0ac75f8b2bfcf565f78c2b7382b031
> > >> Author: Jimi Xenidis <jimix@pobox.com>
> > >> Date:   Thu Dec 6 18:07:16 2012 -0500
> > >>=20
> > >>    Linux 3.4.22-BGQ-rc1
> > >>=20
> > >>=20
> > >>=20
> > >>=20
> > >> _______________________________________________
> > >> Linuxppc-dev mailing list
> > >> Linuxppc-dev@lists.ozlabs.org
> > >> https://lists.ozlabs.org/listinfo/linuxppc-dev
> > >>=20
> >=20
>=20

^ permalink raw reply

* Re: [PATCH v2] powerpc: fix wii_memory_fixups() compile error on 3.0.y tree
From: Ben Hutchings @ 2012-12-10 18:55 UTC (permalink / raw)
  To: Shuah Khan; +Cc: Greg KH, stable, paulus, shuahkhan, linuxppc-dev
In-Reply-To: <1355160196.2739.12.camel@lorien2>

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.

Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
                                                              - Albert Camus

^ permalink raw reply

* [PATCH v2] powerpc: fix wii_memory_fixups() compile error on 3.0.y tree
From: Shuah Khan @ 2012-12-10 17:23 UTC (permalink / raw)
  To: Ben Hutchings, Greg KH, paulus, benh; +Cc: linuxppc-dev, shuahkhan, stable
In-Reply-To: <CAKocOOPYv0L5dnVJ-jcKw6u3Axbe-tFRdz+F8FvE6fq-xRQH5A@mail.gmail.com>

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);
 	pr_info("HOLE: <%08lx %08lx>\n", wii_hole_start, wii_hole_size);
-	pr_info("MEM2: <%08llx %08llx>\n", p[1].base, p[1].size);
+	pr_info("MEM2: <%08ulx %08ulx>\n",
+		(phys_addr_t) p[1].base, (phys_addr_t) p[1].size);
 
 	p[0].size += wii_hole_size + p[1].size;
 
-- 
1.7.9.5

^ permalink raw reply related

* Re: pci and pcie device-tree binding - range No cells
From: Thomas Petazzoni @ 2012-12-10 17:15 UTC (permalink / raw)
  To: monstr
  Cc: linux-pci, devicetree-discuss, Thierry Reding, Rob Herring,
	David Laight, Rob Herring, linuxppc-dev
In-Reply-To: <50C60839.70603@monstr.eu>

Dear Michal Simek,

On Mon, 10 Dec 2012 17:05:13 +0100, Michal Simek wrote:

> CC: Thomas: I think it will be interesting to see this discussion
> because you are using size-cell/address-cells equal 1.
> http://www.spinics.net/lists/arm-kernel/msg211839.html

Thanks for Cc'ing me.

The thing is that on Marvell SoCs, we don't need to describe statically
in the Device Tree the translation between CPU addresses and PCI device
addresses, because those translations are set up dynamically at run
time through address decoding windows.

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.

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.

Of course, I have no idea if I'm doing a correct usage of the DT, but I
certainly don't need those 6 values ranges with bits to say if it's I/O
space, memory space, bus number, device number and so on. The physical
addresses at which I'm setting up my address decoding windows are
decided dynamically at runtime, depending on the number of PCIe devices
that are found in the different PCIe slots (for now, those windows have
a statically defined size, but I'd ideally would like to size them to
match exactly the size of the PCIe device memory, in order to avoid
wasting physical address space, but I haven't found how to get the size
needed for each PCIe device during the ARM pcibios initialization
sequence).

For those willing to have a look at the PCIe patch set for Armada
370/XP, see:
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/136455.html.
The core of the PCIe driver and its DT binding documentation is at
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/136711.html.

Thanks a lot for your comments,

Thomas

[1] that I hope to extend to cover previous Marvell SoCs as well, they
    work basically the same way
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* Re: pci and pcie device-tree binding - range No cells
From: Michal Simek @ 2012-12-10 16:11 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-pci, devicetree-discuss, Thierry Reding, Rob Herring,
	linuxppc-dev
In-Reply-To: <50C6078A.8080408@gmail.com>

On 12/10/2012 05:02 PM, Rob Herring wrote:
> On 12/10/2012 09:37 AM, Michal Simek wrote:
>> On 12/10/2012 04:21 PM, Rob Herring 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.
>>
>> Sorry for my pci ignorance (have never got hw for mb/zynq)
>> I just want to get better overview how we should we our drivers to be
>> compatible.
>>
>> Does it mean that pci is supposed be always 64 bit wide?
>> And there is no option to have just 32bit values.
>
> That's what the DT documentation says.
>
> BTW, you can use 2 cells even if you only have a 32-bit bus.

ok - no problem with that. Just want to be sure about it.

>>>> 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.
>>
>> We have done pcie working on zynq and the same pcie host is working on
>> Microblaze too.
>> There are just small differences. That's why I have sent another email
>> ("Sharing PCIE driver between Microblaze and Arm zynq") to find out
>> proper location.
>
> Yes, I've followed the thread. There are lots of areas for consolidation
> with PCIe hosts both across architectures and within ARM. There are
> multiple people using DW PCIe IP although not upstream yet that I'm
> aware of.

Your suggestions are really appreciate.

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: Michal Simek @ 2012-12-10 16:05 UTC (permalink / raw)
  To: David Laight
  Cc: Thomas Petazzoni, linux-pci, devicetree-discuss, Thierry Reding,
	Rob Herring, Rob Herring, linuxppc-dev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B70E7@saturn3.aculab.com>

On 12/10/2012 04:52 PM, David Laight wrote:
>> Does it mean that pci is supposed be always 64 bit wide?
>> And there is no option to have just 32bit values.
>
> I certainly believe that all PCIe (not PCI) transfers are
> nominally multiples of 64bit data.

And PCI? That powerpc/pci-common code was designed for PCI
and microblaze also used it for PCI.

CC: Thomas: I think it will be interesting to see this discussion
because you are using size-cell/address-cells equal 1.
http://www.spinics.net/lists/arm-kernel/msg211839.html

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: Rob Herring @ 2012-12-10 16:02 UTC (permalink / raw)
  To: monstr
  Cc: linux-pci, devicetree-discuss, Thierry Reding, Rob Herring,
	linuxppc-dev
In-Reply-To: <50C601B6.2080107@monstr.eu>

On 12/10/2012 09:37 AM, Michal Simek wrote:
> On 12/10/2012 04:21 PM, Rob Herring 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.
> 
> Sorry for my pci ignorance (have never got hw for mb/zynq)
> I just want to get better overview how we should we our drivers to be
> compatible.
> 
> Does it mean that pci is supposed be always 64 bit wide?
> And there is no option to have just 32bit values.

That's what the DT documentation says.

BTW, you can use 2 cells even if you only have a 32-bit bus.

> 
>>> 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.
> 
> We have done pcie working on zynq and the same pcie host is working on
> Microblaze too.
> There are just small differences. That's why I have sent another email
> ("Sharing PCIE driver between Microblaze and Arm zynq") to find out
> proper location.

Yes, I've followed the thread. There are lots of areas for consolidation
with PCIe hosts both across architectures and within ARM. There are
multiple people using DW PCIe IP although not upstream yet that I'm
aware of.

Rob

> 
> PCI: Microblaze shares almost the same code with powerpc that's why I am
> definitely open
> to move this code out of architecture.
> 
> Thanks,
> Michal
> 
> 
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox