LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] mpc52xx-psc-spi: refactor probe and remove to make use of of_register_spi_devices()
From: Wolfram Sang @ 2009-10-31  9:03 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Kári Davíðsson, spi-devel-general, linuxppc-dev
In-Reply-To: <20091031163259.5c651773.sfr@canb.auug.org.au>

[-- Attachment #1: Type: text/plain, Size: 988 bytes --]

Hi Stephen,

> >  static struct of_platform_driver mpc52xx_psc_spi_of_driver = {
> >  	.owner = THIS_MODULE,
> > -	.name = "mpc52xx-psc-spi",
> > +	.name = DRIVER_NAME,
> 
> You no longer need to set either owner or name in the of_platform driver,
> just in the included struct driver (as is done below), so you could just
> remove the above two lines.

Okay, thanks. Will drop it.

> 
> >  	.match_table = mpc52xx_psc_spi_of_match,
> >  	.probe = mpc52xx_psc_spi_of_probe,
> >  	.remove = __exit_p(mpc52xx_psc_spi_of_remove),
> >  	.driver = {
> > -		.name = "mpc52xx-psc-spi",
> > +		.name = DRIVER_NAME,
> >  		.owner = THIS_MODULE,
> >  	},
> >  };
> 
> I am hoping that we can remove the owner and name fields from struct
> of_platform_driver sometime.

Sounds good! :)

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH 0/8]  Fix 8xx MMU/TLB
From: Joakim Tjernlund @ 2009-10-31 10:31 UTC (permalink / raw)
  To: Scott Wood; +Cc: Rex Feany, linuxppc-dev@ozlabs.org
In-Reply-To: <20091030173749.GA855@loki.buserror.net>

Scott Wood <scottwood@freescale.com> wrote on 30/10/2009 18:37:49:
>
> On Fri, Oct 30, 2009 at 12:16:07PM -0500, Scott Wood wrote:
> > On Sat, Oct 17, 2009 at 02:01:38PM +0200, Joakim Tjernlund wrote:
> > > +     mfspr r10, SPRN_SRR0
> > >       DO_8xx_CPU6(0x3780, r3)
> > >       mtspr SPRN_MD_EPN, r10
> > >       mfspr r11, SPRN_M_TWB   /* Get level 1 table entry address */
> > > -     lwz   r11, 0(r11)   /* Get the level 1 entry */
> > > +     cmplwi      cr0, r11, 0x0800
> > > +     blt-  3f      /* Branch if user space */
> > > +     lis   r11, swapper_pg_dir@h
> > > +     ori   r11, r11, swapper_pg_dir@l
> > > +     rlwimi      r11, r11, 0, 2, 19
> >
> > That rlwimi is a no-op -- I think you meant to use a different register
> > here?
> >
> > > +3:   lwz   r11, 0(r11)   /* Get the level 1 entry */
> > >       DO_8xx_CPU6(0x3b80, r3)
> > >       mtspr SPRN_MD_TWC, r11   /* Load pte table base address */
> > >       mfspr r11, SPRN_MD_TWC   /* ....and get the pte address */
> > >       lwz   r11, 0(r11)   /* Get the pte */
> > >       /* concat physical page address(r11) and page offset(r10) */
> > >       rlwimi      r11, r10, 0, 20, 31
> >
> > But r10 here contains SRR0 from above, and this is a data TLB error.
>
> Never mind that last one, forgot that you'd be wanting to load the
> instruction. :-P
>
> But the rlwimi is what's causing the machine checks.  I replaced it with:

Yes, I see now that it is wrong.

> rlwinm   r11, r11, 0, 0x3ffff000
> rlwimi   r11, r10, 22, 0xffc
>
> and things seem to work.  You could probably replace the rlwinm by
> subtracting PAGE_OFFSET from swapper_pg_dir instead.

Would you mind produce a proper path on top of my series? I am blind
here so I can only guess what will work or not.
Then Rex can give it some beating and we can push this to Ben

^ permalink raw reply

* Re: [PATCH 19/27] Export symbols for KVM module
From: Alexander Graf @ 2009-10-31 12:02 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Kevin Wolf, Arnd Bergmann, Hollis Blanchard, Marcelo Tosatti,
	kvm-ppc, linuxppc-dev@ozlabs.org, Avi Kivity, kvm@vger.kernel.org,
	bphilips@suse.de, Olof Johansson
In-Reply-To: <20091031153719.10a4e61b.sfr@canb.auug.org.au>


Am 31.10.2009 um 05:37 schrieb Stephen Rothwell <sfr@canb.auug.org.au>:

> Hi Alexander,
>
> On Fri, 30 Oct 2009 16:47:19 +0100 Alexander Graf <agraf@suse.de>  
> wrote:
>>
>> diff --git a/arch/powerpc/kernel/ppc_ksyms.c b/arch/powerpc/kernel/ 
>> ppc_ksyms.c
>> index c8b27bb..baf778c 100644
>> --- a/arch/powerpc/kernel/ppc_ksyms.c
>> +++ b/arch/powerpc/kernel/ppc_ksyms.c
>> @@ -163,11 +163,12 @@ EXPORT_SYMBOL(screen_info);
>> #ifdef CONFIG_PPC32
>> EXPORT_SYMBOL(timer_interrupt);
>> EXPORT_SYMBOL(irq_desc);
>> -EXPORT_SYMBOL(tb_ticks_per_jiffy);
>> EXPORT_SYMBOL(cacheable_memcpy);
>> EXPORT_SYMBOL(cacheable_memzero);
>> #endif
>>
>> +EXPORT_SYMBOL(tb_ticks_per_jiffy);
>
> Since you are moving this anyway, how about moving it into
> arch/powerpc/kernel/time.c where tb_ticks_per_jiffy is defined.

Well the fun part is that the hrtimer conversion patch actually  
deprecates this change.

I merely forgot to change the export back.

So I suppose I'll leave things here as is and then revert this chunk  
in the hrtimer patch.

Alex

^ permalink raw reply

* Re: mpc512x/clock: fix clk_get logic
From: Wolfram Sang @ 2009-10-31 13:01 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Wolfgang Denk
In-Reply-To: <1256939657.6372.81.camel@pasglop>

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]

Hello Ben,

> Have you considered switching to my proposed device-tree based clock
> reprentation ?

You mean this one?

http://patchwork.ozlabs.org/patch/31551/

Sorry, I have missed it up to now :(

While I think (after one glimpse) this moves things into the right direction,
my priority at the moment is to get the mscan driver working on 512x (and then
to mainline). For that, I needed the patch I posted. I hope I can have another
look at your proposal later on, it looks really worth it.

Until then I'd propose to consider this patch as it fixes a bug in clock
assignment. But I leave the final decision to you maintainers, of course.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* RE: Accessing flash directly from User Space [SOLVED]
From: Joakim Tjernlund @ 2009-10-31 13:26 UTC (permalink / raw)
  To: Jonathan Haws
  Cc: scottwood@freescale.com, Alessandro Rubini, bgat@billgatliff.com,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <BB99A6BA28709744BF22A68E6D7EB51F0330E231BE@midas.usurf.usu.edu>

>
> > On Friday 30 October 2009 16:08:55 Alessandro Rubini wrote:
> > > > asm("eieio; sync");
> > >
> > > Hmm...
> > >    : : : "memory"
> > >
> > > And, doesn't ";" start a comment in assembly? (no, not on powerpc
> > it seems)
> >
> > Yes, I think the barrier is wrong.
> > Please try with
> >
> > #define mb()   __asm__ __volatile__("eieio\n sync\n" : : :
> > "memory")
>
> That definition worked great.  I must have missed the : : : "memory" bit when
> I was digging through code.
>
> Thanks, that gives me about a 2x speedup over the msync() calls.

Exactly when should you use the barrier? At every access,
every read or when changing from write to read?

 Jocke

^ permalink raw reply

* Re: Accessing flash directly from User Space [SOLVED]
From: Michael Buesch @ 2009-10-31 16:42 UTC (permalink / raw)
  To: Joakim Tjernlund
  Cc: scottwood@freescale.com, Alessandro Rubini, bgat@billgatliff.com,
	linuxppc-dev@lists.ozlabs.org, Jonathan Haws
In-Reply-To: <OF8458219C.9E5B54DB-ONC1257660.0049761D-C1257660.0049DD9C@transmode.se>

On Saturday 31 October 2009 14:26:48 Joakim Tjernlund wrote:
> >
> > > On Friday 30 October 2009 16:08:55 Alessandro Rubini wrote:
> > > > > asm("eieio; sync");
> > > >
> > > > Hmm...
> > > >    : : : "memory"
> > > >
> > > > And, doesn't ";" start a comment in assembly? (no, not on powerpc
> > > it seems)
> > >
> > > Yes, I think the barrier is wrong.
> > > Please try with
> > >
> > > #define mb()   __asm__ __volatile__("eieio\n sync\n" : : :
> > > "memory")
> >
> > That definition worked great.  I must have missed the : : : "memory" bit when
> > I was digging through code.
> >
> > Thanks, that gives me about a 2x speedup over the msync() calls.
> 
> Exactly when should you use the barrier? At every access,
> every read or when changing from write to read?

Well, it depends on the device you are accessing. I'll give you a small pseudo example.

mmio[0] = address;
mmio[1] = data;
mb();
mmio[3] |= 0x01; /* This triggers an operation -> address=data */
/* probably also need an mb() here, if the following code
 * depends on the operation to be triggered. */

-- 
Greetings, Michael.

^ permalink raw reply

* Re: Accessing flash directly from User Space [SOLVED]
From: Joakim Tjernlund @ 2009-10-31 20:14 UTC (permalink / raw)
  To: Michael Buesch
  Cc: scottwood@freescale.com, Alessandro Rubini, bgat@billgatliff.com,
	linuxppc-dev@lists.ozlabs.org, Jonathan Haws
In-Reply-To: <200910311742.56280.mb@bu3sch.de>

Michael Buesch <mb@bu3sch.de> wrote on 31/10/2009 17:42:54:
>
> On Saturday 31 October 2009 14:26:48 Joakim Tjernlund wrote:
> > >
> > > > On Friday 30 October 2009 16:08:55 Alessandro Rubini wrote:
> > > > > > asm("eieio; sync");
> > > > >
> > > > > Hmm...
> > > > >    : : : "memory"
> > > > >
> > > > > And, doesn't ";" start a comment in assembly? (no, not on powerpc
> > > > it seems)
> > > >
> > > > Yes, I think the barrier is wrong.
> > > > Please try with
> > > >
> > > > #define mb()   __asm__ __volatile__("eieio\n sync\n" : : :
> > > > "memory")
> > >
> > > That definition worked great.  I must have missed the : : : "memory" bit when
> > > I was digging through code.
> > >
> > > Thanks, that gives me about a 2x speedup over the msync() calls.
> >
> > Exactly when should you use the barrier? At every access,
> > every read or when changing from write to read?
>
> Well, it depends on the device you are accessing. I'll give you a small pseudo example.
>
> mmio[0] = address;
> mmio[1] = data;
> mb();
> mmio[3] |= 0x01; /* This triggers an operation -> address=data */
> /* probably also need an mb() here, if the following code
>  * depends on the operation to be triggered. */

So anything that depends on the previous accesses needs a mb()
hmm, the mmio[0] and mmio[1] are written in order I hope?

 Jocke

^ permalink raw reply

* Re: Accessing flash directly from User Space [SOLVED]
From: Michael Buesch @ 2009-10-31 20:35 UTC (permalink / raw)
  To: Joakim Tjernlund
  Cc: scottwood@freescale.com, Alessandro Rubini, bgat@billgatliff.com,
	linuxppc-dev@lists.ozlabs.org, Jonathan Haws
In-Reply-To: <OF7FB0F99B.2962ECBA-ONC1257660.006EDDB5-C1257660.006F2808@transmode.se>

On Saturday 31 October 2009 21:14:07 Joakim Tjernlund wrote:
> Michael Buesch <mb@bu3sch.de> wrote on 31/10/2009 17:42:54:
> >
> > On Saturday 31 October 2009 14:26:48 Joakim Tjernlund wrote:
> > > >
> > > > > On Friday 30 October 2009 16:08:55 Alessandro Rubini wrote:
> > > > > > > asm("eieio; sync");
> > > > > >
> > > > > > Hmm...
> > > > > >    : : : "memory"
> > > > > >
> > > > > > And, doesn't ";" start a comment in assembly? (no, not on powerpc
> > > > > it seems)
> > > > >
> > > > > Yes, I think the barrier is wrong.
> > > > > Please try with
> > > > >
> > > > > #define mb()   __asm__ __volatile__("eieio\n sync\n" : : :
> > > > > "memory")
> > > >
> > > > That definition worked great.  I must have missed the : : : "memory" bit when
> > > > I was digging through code.
> > > >
> > > > Thanks, that gives me about a 2x speedup over the msync() calls.
> > >
> > > Exactly when should you use the barrier? At every access,
> > > every read or when changing from write to read?
> >
> > Well, it depends on the device you are accessing. I'll give you a small pseudo example.
> >
> > mmio[0] = address;
> > mmio[1] = data;
> > mb();
> > mmio[3] |= 0x01; /* This triggers an operation -> address=data */
> > /* probably also need an mb() here, if the following code
> >  * depends on the operation to be triggered. */
> 
> So anything that depends on the previous accesses needs a mb()

No, not really. I would always put an mb() where the comment is.
Imagine you have two instances of the above (probably in a loop):

mmio[0] = address;
mmio[1] = data;
mb();
mmio[3] |= 0x01; /* This triggers an operation -> address=data */
mb();

mmio[0] = address;
mmio[1] = data;
mb();
mmio[3] |= 0x01; /* This triggers an operation -> address=data */
mb();

> hmm, the mmio[0] and mmio[1] are written in order I hope?

We do not care in this example, as the write to [3] does trigger
the device operation. We do only care that [0] and [1] are set
when [3] is written. We do not care in what order [0] and [1] are written.

This is just an artificial example, but I think you get my point.
My point just being is that an mb() does only belong where it matters.
And what matters is defined by your device specifications.

-- 
Greetings, Michael.

^ permalink raw reply

* Re: mpc512x/clock: fix clk_get logic
From: Benjamin Herrenschmidt @ 2009-10-31 20:48 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linuxppc-dev, Wolfgang Denk
In-Reply-To: <20091031130131.GA23009@pengutronix.de>

On Sat, 2009-10-31 at 14:01 +0100, Wolfram Sang wrote:
> Hello Ben,
> 
> > Have you considered switching to my proposed device-tree based clock
> > reprentation ?
> 
> You mean this one?
> 
> http://patchwork.ozlabs.org/patch/31551/
> 
> Sorry, I have missed it up to now :(
> 
> While I think (after one glimpse) this moves things into the right direction,
> my priority at the moment is to get the mscan driver working on 512x (and then
> to mainline). For that, I needed the patch I posted. I hope I can have another
> look at your proposal later on, it looks really worth it.
> 
> Until then I'd propose to consider this patch as it fixes a bug in clock
> assignment. But I leave the final decision to you maintainers, of course.

Sure I don't have major objections against your patch (though who is
formally mpc512x maintainer to ack it ?), I just wanted to make sure you
had a look at my proposal since this is almost the only platform to use
the clock API today in arch/powerpc.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH 20/27] Split init_new_context and destroy_context
From: Alexander Graf @ 2009-10-31 21:20 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Kevin Wolf, Arnd Bergmann, Hollis Blanchard, Marcelo Tosatti,
	kvm-ppc, linuxppc-dev, Avi Kivity, kvm, bphilips, Olof Johansson
In-Reply-To: <20091031154059.750d7213.sfr@canb.auug.org.au>

Stephen Rothwell wrote:
> Hi Alexander,
>
> On Fri, 30 Oct 2009 16:47:20 +0100 Alexander Graf <agraf@suse.de> wrote:
>   
>> --- a/arch/powerpc/include/asm/mmu_context.h
>> +++ b/arch/powerpc/include/asm/mmu_context.h
>> @@ -23,6 +23,11 @@ extern void switch_slb(struct task_struct *tsk, struct mm_struct *mm);
>>  extern void set_context(unsigned long id, pgd_t *pgd);
>>  
>>  #ifdef CONFIG_PPC_BOOK3S_64
>> +extern int __init_new_context(void);
>> +extern void __destroy_context(int context_id);
>> +#endif
>> +
>> +#ifdef CONFIG_PPC_BOOK3S_64
>>     
>
> don't add the #endif/#ifdef pair ...

Any other comments? I'd like to wind up a final patch set so I can stop
spamming on all those MLs :-).

Alex

^ permalink raw reply

* Re: [PATCH 20/27] Split init_new_context and destroy_context
From: Benjamin Herrenschmidt @ 2009-10-31 21:37 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Kevin Wolf, Stephen Rothwell, Arnd Bergmann, Hollis Blanchard,
	Marcelo Tosatti, kvm-ppc, linuxppc-dev, Avi Kivity, kvm, bphilips,
	Olof Johansson
In-Reply-To: <4AECAA0A.1080304@suse.de>

On Sat, 2009-10-31 at 22:20 +0100, Alexander Graf wrote:
> Stephen Rothwell wrote:
> > Hi Alexander,
> >
> > On Fri, 30 Oct 2009 16:47:20 +0100 Alexander Graf <agraf@suse.de> wrote:
> >   
> >> --- a/arch/powerpc/include/asm/mmu_context.h
> >> +++ b/arch/powerpc/include/asm/mmu_context.h
> >> @@ -23,6 +23,11 @@ extern void switch_slb(struct task_struct *tsk, struct mm_struct *mm);
> >>  extern void set_context(unsigned long id, pgd_t *pgd);
> >>  
> >>  #ifdef CONFIG_PPC_BOOK3S_64
> >> +extern int __init_new_context(void);
> >> +extern void __destroy_context(int context_id);
> >> +#endif
> >> +
> >> +#ifdef CONFIG_PPC_BOOK3S_64
> >>     
> >
> > don't add the #endif/#ifdef pair ...
> 
> Any other comments? I'd like to wind up a final patch set so I can stop
> spamming on all those MLs :-).

Just send an update for -that- patch to the list.

Ben.

^ permalink raw reply

* Re: Accessing flash directly from User Space [SOLVED]
From: Joakim Tjernlund @ 2009-10-31 22:31 UTC (permalink / raw)
  To: Michael Buesch
  Cc: scottwood@freescale.com, Alessandro Rubini, bgat@billgatliff.com,
	linuxppc-dev@lists.ozlabs.org, Jonathan Haws
In-Reply-To: <200910312135.34196.mb@bu3sch.de>

Michael Buesch <mb@bu3sch.de> wrote on 31/10/2009 21:35:31:
>
> On Saturday 31 October 2009 21:14:07 Joakim Tjernlund wrote:
> > Michael Buesch <mb@bu3sch.de> wrote on 31/10/2009 17:42:54:
> > >
> > > On Saturday 31 October 2009 14:26:48 Joakim Tjernlund wrote:
> > > > >
> > > > > > On Friday 30 October 2009 16:08:55 Alessandro Rubini wrote:
> > > > > > > > asm("eieio; sync");
> > > > > > >
> > > > > > > Hmm...
> > > > > > >    : : : "memory"
> > > > > > >
> > > > > > > And, doesn't ";" start a comment in assembly? (no, not on powerpc
> > > > > > it seems)
> > > > > >
> > > > > > Yes, I think the barrier is wrong.
> > > > > > Please try with
> > > > > >
> > > > > > #define mb()   __asm__ __volatile__("eieio\n sync\n" : : :
> > > > > > "memory")
> > > > >
> > > > > That definition worked great.  I must have missed the : : : "memory" bit when
> > > > > I was digging through code.
> > > > >
> > > > > Thanks, that gives me about a 2x speedup over the msync() calls.
> > > >
> > > > Exactly when should you use the barrier? At every access,
> > > > every read or when changing from write to read?
> > >
> > > Well, it depends on the device you are accessing. I'll give you a small
> pseudo example.
> > >
> > > mmio[0] = address;
> > > mmio[1] = data;
> > > mb();
> > > mmio[3] |= 0x01; /* This triggers an operation -> address=data */
> > > /* probably also need an mb() here, if the following code
> > >  * depends on the operation to be triggered. */
> >
> > So anything that depends on the previous accesses needs a mb()
>
> No, not really. I would always put an mb() where the comment is.
> Imagine you have two instances of the above (probably in a loop):
>
> mmio[0] = address;
> mmio[1] = data;
> mb();
> mmio[3] |= 0x01; /* This triggers an operation -> address=data */
> mb();
>
> mmio[0] = address;
> mmio[1] = data;
> mb();
> mmio[3] |= 0x01; /* This triggers an operation -> address=data */
> mb();
>
> > hmm, the mmio[0] and mmio[1] are written in order I hope?
>
> We do not care in this example, as the write to [3] does trigger
> the device operation. We do only care that [0] and [1] are set
> when [3] is written. We do not care in what order [0] and [1] are written.

In this example yes, I was wondering in general.

>
> This is just an artificial example, but I think you get my point.
> My point just being is that an mb() does only belong where it matters.
> And what matters is defined by your device specifications.

So what does guarded memory mapping on ppc mean really? If I need
that much mb(), guarded does not seem to do much.

 Jocke

^ permalink raw reply

* Re: [PATCH] mpc52xx-psc-spi: refactor probe and remove to make use of of_register_spi_devices()
From: Grant Likely @ 2009-11-01  1:03 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Kári Davíðsson, spi-devel-general, linuxppc-dev
In-Reply-To: <1256931852-13255-1-git-send-email-w.sang@pengutronix.de>

[-- Attachment #1: Type: text/plain, Size: 10450 bytes --]

Hi Wolfram,

Thanks for this work. Comments below.

On Fri, Oct 30, 2009 at 1:44 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> This driver has a generic part for the probe/remove routines which probably
> used to be called from a platform driver and an of_platform driver. Meanwhile,
> the driver is of_platform only, so there is no need for the generic part
> anymore. Unifying probe/remove finally enables us to call
> of_register_spi_devices(), so spi-device nodes from the device tree will be
> parsed. This was also mentioned by:
> http://lists.ozlabs.org/pipermail/linuxppc-dev/2009-June/073273.html

I haven't really been happy with any of the patches that added the
of_register_spi_devices() call, but I never got around to dealing with
it properly, or even replying.  I found Jon's patch too invasive, and
the patch referenced above is a little ugly.  Adding the call should
be really simple.  I've drafted a patch to do only that step and
attached it to this mail.  If this one works for you, then I'll merge
it immediately into -next.

> On the way, some patterns have been changed to current best practices (like
> using '!ptr' and 'struct resource'). Also, an older, yet uncommented, patch
> from me has been incorporated to improve the checks if the selected PSC is
> valid:
>
> http://patchwork.ozlabs.org/patch/36780/

There are at least 3 discrete changes here.  I prefer for each to be
provided as a separate patch so I can cherry pick the ones that are
ready.  I'll go back and comment on the patch you referenced above
patch right now.

Also, I'm resistant to changing the probe layout on this driver at
this time.  With the work being done to generalize the OF support
code, there is a strong possibility that of_platform will be
deprecated in favor of going back to using the platform bus directly
(just like how OF support works for i2c, spi, etc).  I'd rather not
refactor the driver until I'm certain of the direction that things are
going to go.

Cheers,
g.

>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Kári Davíðsson <kari.davidsson@marel.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> ---
>  drivers/spi/mpc52xx_psc_spi.c |  110 +++++++++++++++++++---------------------
>  1 files changed, 52 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/spi/mpc52xx_psc_spi.c b/drivers/spi/mpc52xx_psc_spi.c
> index 1b74d5c..3fa8c78 100644
> --- a/drivers/spi/mpc52xx_psc_spi.c
> +++ b/drivers/spi/mpc52xx_psc_spi.c
> @@ -17,6 +17,7 @@
>  #include <linux/errno.h>
>  #include <linux/interrupt.h>
>  #include <linux/of_platform.h>
> +#include <linux/of_spi.h>
>  #include <linux/workqueue.h>
>  #include <linux/completion.h>
>  #include <linux/io.h>
> @@ -27,6 +28,7 @@
>  #include <asm/mpc52xx.h>
>  #include <asm/mpc52xx_psc.h>
>
> +#define DRIVER_NAME "mpc52xx-psc-spi"
>  #define MCLK 20000000 /* PSC port MClk in hz */
>
>  struct mpc52xx_psc_spi {
> @@ -358,32 +360,54 @@ static irqreturn_t mpc52xx_psc_spi_isr(int irq, void *dev_id)
>        return IRQ_NONE;
>  }
>
> -/* bus_num is used only for the case dev->platform_data == NULL */
> -static int __init mpc52xx_psc_spi_do_probe(struct device *dev, u32 regaddr,
> -                               u32 size, unsigned int irq, s16 bus_num)
> +static int __init mpc52xx_psc_spi_of_probe(struct of_device *op,
> +       const struct of_device_id *match)
>  {
> -       struct fsl_spi_platform_data *pdata = dev->platform_data;
> +       struct fsl_spi_platform_data *pdata = op->dev.platform_data;
>        struct mpc52xx_psc_spi *mps;
>        struct spi_master *master;
> +       struct resource mem;
> +       s16 id = -1;
>        int ret;
>
> -       master = spi_alloc_master(dev, sizeof *mps);
> -       if (master == NULL)
> +       /* get PSC id (only 1,2,3,6 can do SPI) */
> +       if (!pdata) {
> +               const u32 *psc_nump;
> +
> +               psc_nump = of_get_property(op->node, "cell-index", NULL);
> +               if (!psc_nump || (*psc_nump > 2 && *psc_nump != 5)) {
> +                       dev_err(&op->dev, "PSC can't do SPI\n");
> +                       return -EINVAL;
> +               }
> +               id = *psc_nump + 1;
> +       }
> +
> +       ret = of_address_to_resource(op->node, 0, &mem);
> +       if (ret)
> +               return ret;
> +
> +       master = spi_alloc_master(&op->dev, sizeof *mps);
> +       if (!master)
>                return -ENOMEM;
>
> -       dev_set_drvdata(dev, master);
> +       if (!request_mem_region(mem.start, resource_size(&mem), DRIVER_NAME)) {
> +               ret = -ENOMEM;
> +               goto free_master;
> +       }
> +
> +       dev_set_drvdata(&op->dev, master);
>        mps = spi_master_get_devdata(master);
>
>        /* the spi->mode bits understood by this driver: */
>        master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST;
>
> -       mps->irq = irq;
> -       if (pdata == NULL) {
> -               dev_warn(dev, "probe called without platform data, no "
> +       mps->irq = irq_of_parse_and_map(op->node, 0);
> +       if (!pdata) {
> +               dev_info(&op->dev, "probe called without platform data, no "
>                                "cs_control function will be called\n");
>                mps->cs_control = NULL;
>                mps->sysclk = 0;
> -               master->bus_num = bus_num;
> +               master->bus_num = id;
>                master->num_chipselect = 255;
>        } else {
>                mps->cs_control = pdata->cs_control;
> @@ -395,19 +419,18 @@ static int __init mpc52xx_psc_spi_do_probe(struct device *dev, u32 regaddr,
>        master->transfer = mpc52xx_psc_spi_transfer;
>        master->cleanup = mpc52xx_psc_spi_cleanup;
>
> -       mps->psc = ioremap(regaddr, size);
> +       mps->psc = ioremap(mem.start, resource_size(&mem));
>        if (!mps->psc) {
> -               dev_err(dev, "could not ioremap I/O port range\n");
> +               dev_err(&op->dev, "could not ioremap I/O port range\n");
>                ret = -EFAULT;
> -               goto free_master;
> +               goto free_unmap;
>        }
>        /* On the 5200, fifo regs are immediately ajacent to the psc regs */
>        mps->fifo = ((void __iomem *)mps->psc) + sizeof(struct mpc52xx_psc);
>
> -       ret = request_irq(mps->irq, mpc52xx_psc_spi_isr, 0, "mpc52xx-psc-spi",
> -                               mps);
> +       ret = request_irq(mps->irq, mpc52xx_psc_spi_isr, 0, DRIVER_NAME, mps);
>        if (ret)
> -               goto free_master;
> +               goto free_unmap;
>
>        ret = mpc52xx_psc_spi_port_config(master->bus_num, mps);
>        if (ret < 0)
> @@ -429,24 +452,29 @@ static int __init mpc52xx_psc_spi_do_probe(struct device *dev, u32 regaddr,
>        if (ret < 0)
>                goto unreg_master;
>
> +       of_register_spi_devices(master, op->node);
> +
>        return ret;
>
>  unreg_master:
>        destroy_workqueue(mps->workqueue);
>  free_irq:
>        free_irq(mps->irq, mps);
> -free_master:
> +free_unmap:
>        if (mps->psc)
>                iounmap(mps->psc);
> +       release_mem_region(mem.start, resource_size(&mem));
> +free_master:
>        spi_master_put(master);
>
>        return ret;
>  }
>
> -static int __exit mpc52xx_psc_spi_do_remove(struct device *dev)
> +static int __exit mpc52xx_psc_spi_of_remove(struct of_device *op)
>  {
> -       struct spi_master *master = dev_get_drvdata(dev);
> +       struct spi_master *master = dev_get_drvdata(&op->dev);
>        struct mpc52xx_psc_spi *mps = spi_master_get_devdata(master);
> +       struct resource mem;
>
>        flush_workqueue(mps->workqueue);
>        destroy_workqueue(mps->workqueue);
> @@ -454,46 +482,12 @@ static int __exit mpc52xx_psc_spi_do_remove(struct device *dev)
>        free_irq(mps->irq, mps);
>        if (mps->psc)
>                iounmap(mps->psc);
> +       if (of_address_to_resource(op->node, 0, &mem) == 0)
> +               release_mem_region(mem.start, resource_size(&mem));
>
>        return 0;
>  }
>
> -static int __init mpc52xx_psc_spi_of_probe(struct of_device *op,
> -       const struct of_device_id *match)
> -{
> -       const u32 *regaddr_p;
> -       u64 regaddr64, size64;
> -       s16 id = -1;
> -
> -       regaddr_p = of_get_address(op->node, 0, &size64, NULL);
> -       if (!regaddr_p) {
> -               printk(KERN_ERR "Invalid PSC address\n");
> -               return -EINVAL;
> -       }
> -       regaddr64 = of_translate_address(op->node, regaddr_p);
> -
> -       /* get PSC id (1..6, used by port_config) */
> -       if (op->dev.platform_data == NULL) {
> -               const u32 *psc_nump;
> -
> -               psc_nump = of_get_property(op->node, "cell-index", NULL);
> -               if (!psc_nump || *psc_nump > 5) {
> -                       printk(KERN_ERR "mpc52xx_psc_spi: Device node %s has invalid "
> -                                       "cell-index property\n", op->node->full_name);
> -                       return -EINVAL;
> -               }
> -               id = *psc_nump + 1;
> -       }
> -
> -       return mpc52xx_psc_spi_do_probe(&op->dev, (u32)regaddr64, (u32)size64,
> -                                       irq_of_parse_and_map(op->node, 0), id);
> -}
> -
> -static int __exit mpc52xx_psc_spi_of_remove(struct of_device *op)
> -{
> -       return mpc52xx_psc_spi_do_remove(&op->dev);
> -}
> -
>  static struct of_device_id mpc52xx_psc_spi_of_match[] = {
>        { .compatible = "fsl,mpc5200-psc-spi", },
>        { .compatible = "mpc5200-psc-spi", }, /* old */
> @@ -504,12 +498,12 @@ MODULE_DEVICE_TABLE(of, mpc52xx_psc_spi_of_match);
>
>  static struct of_platform_driver mpc52xx_psc_spi_of_driver = {
>        .owner = THIS_MODULE,
> -       .name = "mpc52xx-psc-spi",
> +       .name = DRIVER_NAME,
>        .match_table = mpc52xx_psc_spi_of_match,
>        .probe = mpc52xx_psc_spi_of_probe,
>        .remove = __exit_p(mpc52xx_psc_spi_of_remove),
>        .driver = {
> -               .name = "mpc52xx-psc-spi",
> +               .name = DRIVER_NAME,
>                .owner = THIS_MODULE,
>        },
>  };
> --
> 1.6.3.3
>
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

[-- Attachment #2: 0001-spi-mpc5200-Register-SPI-devices-described-in-device.patch --]
[-- Type: text/x-patch, Size: 1601 bytes --]

From 7629d40dc343ff216b752d5c68654dc9d30f0c91 Mon Sep 17 00:00:00 2001
From: Grant Likely <grant.likely@secretlab.ca>
Date: Sat, 31 Oct 2009 17:49:38 -0600
Subject: [PATCH] spi/mpc5200: Register SPI devices described in device tree

Add call to of_register_spi_devices() to register SPI devices described
in the OF device tree.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/spi/mpc52xx_psc_spi.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/mpc52xx_psc_spi.c b/drivers/spi/mpc52xx_psc_spi.c
index 1b74d5c..b445464 100644
--- a/drivers/spi/mpc52xx_psc_spi.c
+++ b/drivers/spi/mpc52xx_psc_spi.c
@@ -17,6 +17,7 @@
 #include <linux/errno.h>
 #include <linux/interrupt.h>
 #include <linux/of_platform.h>
+#include <linux/of_spi.h>
 #include <linux/workqueue.h>
 #include <linux/completion.h>
 #include <linux/io.h>
@@ -464,6 +465,7 @@ static int __init mpc52xx_psc_spi_of_probe(struct of_device *op,
 	const u32 *regaddr_p;
 	u64 regaddr64, size64;
 	s16 id = -1;
+	int rc;
 
 	regaddr_p = of_get_address(op->node, 0, &size64, NULL);
 	if (!regaddr_p) {
@@ -485,8 +487,12 @@ static int __init mpc52xx_psc_spi_of_probe(struct of_device *op,
 		id = *psc_nump + 1;
 	}
 
-	return mpc52xx_psc_spi_do_probe(&op->dev, (u32)regaddr64, (u32)size64,
+	rc = mpc52xx_psc_spi_do_probe(&op->dev, (u32)regaddr64, (u32)size64,
 					irq_of_parse_and_map(op->node, 0), id);
+	if (!rc)
+		of_register_spi_devices(dev_get_drvdata(&op->dev), op->node);
+
+	return rc;
 }
 
 static int __exit mpc52xx_psc_spi_of_remove(struct of_device *op)
-- 
1.6.3.3


^ permalink raw reply related

* Re: [PATCH] spi/mpc52xx: check for invalid PSC usage
From: Grant Likely @ 2009-11-01  1:22 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: spi-devel-general, David Brownell, linuxppc-dev
In-Reply-To: <1256297157-28246-1-git-send-email-w.sang@pengutronix.de>

On Fri, Oct 23, 2009 at 5:25 AM, Wolfram Sang <w.sang@pengutronix.de> wrote=
:
> Add checks to allow only PSCs capable of SPI.
> Also turn printk to dev_err while we are here.

Hi Wolfram,

I wouldn't even bother.  It's not actively dangerous to try and use
PSC{4,5} in SPI mode.  It just not going to work.  Besides, the
MPC5200 common code already checks for an invalid PSC number when
setting the clock divisor.

Have you seen cases of users trying to do the wrong thing with the
crippled PSCs?

g.

>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: David Brownell <dbrownell@users.sourceforge.net>
> ---
>
> Grant, if the patch is OK, maybe it can also go via your tree?
>
> =A0drivers/spi/mpc52xx_psc_spi.c | =A0 =A09 ++++-----
> =A01 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/spi/mpc52xx_psc_spi.c b/drivers/spi/mpc52xx_psc_spi.=
c
> index 1b74d5c..1d11a6d 100644
> --- a/drivers/spi/mpc52xx_psc_spi.c
> +++ b/drivers/spi/mpc52xx_psc_spi.c
> @@ -467,19 +467,18 @@ static int __init mpc52xx_psc_spi_of_probe(struct o=
f_device *op,
>
> =A0 =A0 =A0 =A0regaddr_p =3D of_get_address(op->node, 0, &size64, NULL);
> =A0 =A0 =A0 =A0if (!regaddr_p) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 printk(KERN_ERR "Invalid PSC address\n");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_err(&op->dev, "Invalid PSC address\n");
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -EINVAL;
> =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0regaddr64 =3D of_translate_address(op->node, regaddr_p);
>
> - =A0 =A0 =A0 /* get PSC id (1..6, used by port_config) */
> + =A0 =A0 =A0 /* get PSC id (only 1,2,3,6 can do SPI) */
> =A0 =A0 =A0 =A0if (op->dev.platform_data =3D=3D NULL) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0const u32 *psc_nump;
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0psc_nump =3D of_get_property(op->node, "ce=
ll-index", NULL);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!psc_nump || *psc_nump > 5) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 printk(KERN_ERR "mpc52xx_ps=
c_spi: Device node %s has invalid "
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 "cell-index property\n", op->node->full_name);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!psc_nump || (*psc_nump > 2 && *psc_num=
p !=3D 5)) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_err(&op->dev, "PSC can'=
t do SPI\n");
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -EINVAL;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0id =3D *psc_nump + 1;
> --
> 1.6.3.3
>
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH/v2] powerpc/5200: make BestComm gen_bd microcode exchangeable
From: Grant Likely @ 2009-11-01  6:47 UTC (permalink / raw)
  To: Albrecht Dreß; +Cc: Linux PPC Development
In-Reply-To: <1254771740.5503.2@antares>

On Mon, Oct 5, 2009 at 1:42 PM, Albrecht Dre=DF <albrecht.dress@arcor.de> w=
rote:
> Am 05.10.09 20:16 schrieb(en) Grant Likely:
>>
>> Hmmm. =A0I've not been comfortable with this change, but it took me a wh=
ile
>> to put my finger on exactly why. =A0In principle, I think it is a good i=
dea.
>> =A0However, I don't want to merge it without any in-tree users.
>
> I could provide my modified microcode (which differs in two words only), =
for
> the "standard" task, just with byte swapping for the LPB, but I wonder if=
 it
> is of much use. =A0No progress with crc32 calculation in BestComm so far.=
..
> :-(
>
>> The other concern is that I don't like that the patch is only applicable
>> to gen_bd tasks.
>
> IMHO the big difference between gen_bd on one hand and ATA and FEC on the
> other is that (afaik) we have *no* other choice than using the implemente=
d
> tasks for the latter two, obviously using the Freescale's example microco=
de.
> =A0Gen_bd can apparently be used for a plenty of purposes, including LPB =
(as
> shown in your driver), maybe PCI (there are examples in Freescale's code)=
,
> PSC, etc. =A0Thus, the FEC and ATA drivers should not have an option as t=
o
> change the microcode, whereas it is just convenient for gen_bd.

agreed.

>> I've been thinking about this for a while, and I'm not exactly thrilled
>> with the bestcomm layout which keeps the bestcomm firmware separate from=
 the
>> only driver that actually uses it (ie FEC firmware & support code is
>> separate from the FEC driver. =A0Same for ATA). =A0I don't like the fact=
 that
>> code which is only ever used by the ATA driver is maintained completely
>> separate from it.
>
> That's a good point! =A0The separate microcode files for ATA and FEC are
> somewhat confusing. =A0I guess it's a result of simply copying the Freesc=
ale
> examples into separate files (and that might be a good reason for keeping
> the microcode separated). =A0Maybe create new sub-folders below
> arch/powerpc/sysdev/bestcomm for fec, ata, and gen_bd? =A0At least some
> documentation would be great.

No, that doesn't really help much.  The ATA and FEC microcode +
support functions really have no business living in the common
bestcomm directory at all.  I should look into moving that stuff right
into the FEC and ATA drivers.  The Bestcomm API is probably designed
to the wrong level, and the base API should expose the same task
loading interface to all uses, FEC, ATA, GenBD, and custom tasks.

>> But I may be splitting hairs here and maybe I shouldn't be too concerned
>> about it. =A0Regardless, I'd think I'd rather see something that isn't t=
otally
>> gen_bd specific. =A0Perhaps *microcode should be part of the bcom_task
>> structure?
>
> See above - I believe FEC and ATA are somewhat special as they need
> *exactly* the provided microcode. =A0And if something cannot be changed
> without breaking the functionality, there should not be a chance to do so=
...
> ;-)

Hence the FEC and ATA microcode should live in (*gasp*) the FEC and
ATA drivers.  ;-)

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 1/1] powerpc/40x: Add new PPC440EPx based board HCU5 of Netstal Maschinen
From: Grant Likely @ 2009-11-01  7:12 UTC (permalink / raw)
  To: Niklaus Giger; +Cc: linuxppc-dev, niklaus.giger
In-Reply-To: <1255075865-4025-2-git-send-email-niklaus.giger@member.fsf.org>

On Fri, Oct 9, 2009 at 2:11 AM, Niklaus Giger
<niklaus.giger@member.fsf.org> wrote:
> Adds support for a HCU5 PPC405EPx based board from Netstal Maschinen AG.
>
> Signed-off-by: Niklaus Giger <niklaus.giger@member.fsf.org>
> ---
> =A0arch/powerpc/boot/dts/hcu5.dts =A0 =A0 =A0 =A0 =A0| =A0254 +++++++
> =A0arch/powerpc/configs/44x/hcu5_defconfig | 1166 +++++++++++++++++++++++=
++++++++

Do you really need your own defconfig?  Can you instead add support
for your board to ppc44x_defconfig?  Josh is the maintainer here, so
it is his decision, but on 5200 stuff I've been pushing back on adding
yet-another-board-specific-defconfig when there isn't a really
compelling reason to do so.  It adds a lot of lines for not a lot of
benefit.  Besides, if you add your board to the ppc44x_defconfig, you
gain the advantage of your code getting compiled by others more
frequently.

g.

^ permalink raw reply

* Fail building 2.6.31.x for CHRP using gcc-4.4.x
From: Giuseppe Coviello @ 2009-11-01  9:17 UTC (permalink / raw)
  To: linuxppc-dev

Hi, we are having trouble while building kernel 2.6.31.x and also
2.6.32rc for CHRP platforms using gcc-4.4.x, the error that give us
the compilation is:

  CC      arch/powerpc/sysdev/indirect_pci.o
  CC      arch/powerpc/sysdev/i8259.o
  LD      arch/powerpc/sysdev/built-in.o
  CC      arch/powerpc/platforms/chrp/setup.o
cc1: warnings being treated as errors
arch/powerpc/platforms/chrp/setup.c: In function 'chrp_event_scan':
arch/powerpc/platforms/chrp/setup.c:378: error: the frame size of 1040
bytes is larger than 1024 bytes
make[2]: *** [arch/powerpc/platforms/chrp/setup.o] Error 1
make[1]: *** [arch/powerpc/platforms/chrp] Error 2
make: *** [arch/powerpc/platforms] Error

The same kernel, with the same configuration builds fine using gcc-4.3.x.

Regards, Giuseppe

^ permalink raw reply

* Re: Accessing flash directly from User Space [SOLVED]
From: Segher Boessenkool @ 2009-11-01 10:07 UTC (permalink / raw)
  To: Joakim Tjernlund
  Cc: bgat@billgatliff.com, Alessandro Rubini, Jonathan Haws,
	scottwood@freescale.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <OFA2803682.B247ED64-ONC1257660.007B44F5-C1257660.007BB531@transmode.se>

>>>> mmio[0] = address;
>>>> mmio[1] = data;
>>>> mb();

eieio is enough here.

>>>> mmio[3] |= 0x01; /* This triggers an operation -> address=data */
>>>> /* probably also need an mb() here, if the following code
>>>>  * depends on the operation to be triggered. */

No, a sync does not guarantee the device has seen the store yet;
you need something specific to the device to guarantee this.
Usually a load (from the same register!) followed by code that
makes sure the load has finished is sufficient (and necessary).

>>> hmm, the mmio[0] and mmio[1] are written in order I hope?
>>
>> We do not care in this example, as the write to [3] does trigger
>> the device operation. We do only care that [0] and [1] are set
>> when [3] is written. We do not care in what order [0] and [1] are  
>> written.
>
> In this example yes, I was wondering in general.

The writes will not be reordered, but they can be combined,
unless you put an eieio (or sync) inbetween.

> So what does guarded memory mapping on ppc mean really? If I need
> that much mb(), guarded does not seem to do much.

Loosely speaking, guarded means no prefetch is done.


Segher

^ permalink raw reply

* Re: Fail building 2.6.31.x for CHRP using gcc-4.4.x
From: Stephen Rothwell @ 2009-11-01 11:41 UTC (permalink / raw)
  To: Giuseppe Coviello; +Cc: linuxppc-dev
In-Reply-To: <a265241c0911010117m65335b80g94457a07d0610cbe@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1068 bytes --]

Hi Giuseppe,

On Sun, 1 Nov 2009 10:17:00 +0100 Giuseppe Coviello <cjg@cruxppc.org> wrote:
>
> Hi, we are having trouble while building kernel 2.6.31.x and also
> 2.6.32rc for CHRP platforms using gcc-4.4.x, the error that give us
> the compilation is:
> 
>   CC      arch/powerpc/sysdev/indirect_pci.o
>   CC      arch/powerpc/sysdev/i8259.o
>   LD      arch/powerpc/sysdev/built-in.o
>   CC      arch/powerpc/platforms/chrp/setup.o
> cc1: warnings being treated as errors
> arch/powerpc/platforms/chrp/setup.c: In function 'chrp_event_scan':
> arch/powerpc/platforms/chrp/setup.c:378: error: the frame size of 1040
> bytes is larger than 1024 bytes
> make[2]: *** [arch/powerpc/platforms/chrp/setup.o] Error 1
> make[1]: *** [arch/powerpc/platforms/chrp] Error 2
> make: *** [arch/powerpc/platforms] Error
> 
> The same kernel, with the same configuration builds fine using gcc-4.3.x.

For now, just enable CONFIG_PPC_DISABLE_WERROR.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [PATCH 08/27] Add SLB switching code for entry/exit
From: Michael Neuling @ 2009-11-01 23:23 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Kevin Wolf, Arnd Bergmann, Hollis Blanchard, Marcelo Tosatti,
	kvm-ppc, linuxppc-dev, Avi Kivity, kvm, bphilips, Olof Johansson
In-Reply-To: <1256917647-6200-9-git-send-email-agraf@suse.de>

> This is the really low level of guest entry/exit code.
> 
> Book3s_64 has an SLB, which stores all ESID -> VSID mappings we're
> currently aware of.
> 
> The segments in the guest differ from the ones on the host, so we need
> to switch the SLB to tell the MMU that we're in a new context.
> 
> So we store a shadow of the guest's SLB in the PACA, switch to that on
> entry and only restore bolted entries on exit, leaving the rest to the
> Linux SLB fault handler.
> 
> That way we get a really clean way of switching the SLB.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/powerpc/kvm/book3s_64_slb.S |  277 ++++++++++++++++++++++++++++++++++++
++
>  1 files changed, 277 insertions(+), 0 deletions(-)
>  create mode 100644 arch/powerpc/kvm/book3s_64_slb.S
> 
> diff --git a/arch/powerpc/kvm/book3s_64_slb.S b/arch/powerpc/kvm/book3s_64_sl
b.S
> new file mode 100644
> index 0000000..00a8367
> --- /dev/null
> +++ b/arch/powerpc/kvm/book3s_64_slb.S
> @@ -0,0 +1,277 @@
> +/*
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
> + *
> + * Copyright SUSE Linux Products GmbH 2009
> + *
> + * Authors: Alexander Graf <agraf@suse.de>
> + */
> +
> +/***************************************************************************
***
> + *                                                                          
  *
> + *                               Entry code                                 
  *
> + *                                                                          
  *
> + ***************************************************************************
**/
> +
> +.global kvmppc_handler_trampoline_enter
> +kvmppc_handler_trampoline_enter:
> +
> +	/* Required state:
> +	 *
> +	 * MSR = ~IR|DR
> +	 * R13 = PACA
> +	 * R9 = guest IP
> +	 * R10 = guest MSR
> +	 * R11 = free
> +	 * R12 = free
> +	 * PACA[PACA_EXMC + EX_R9] = guest R9
> +	 * PACA[PACA_EXMC + EX_R10] = guest R10
> +	 * PACA[PACA_EXMC + EX_R11] = guest R11
> +	 * PACA[PACA_EXMC + EX_R12] = guest R12
> +	 * PACA[PACA_EXMC + EX_R13] = guest R13
> +	 * PACA[PACA_EXMC + EX_CCR] = guest CR
> +	 * PACA[PACA_EXMC + EX_R3] = guest XER
> +	 */
> +
> +	mtsrr0	r9
> +	mtsrr1	r10
> +
> +	mtspr	SPRN_SPRG_SCRATCH0, r0
> +
> +	/* Remove LPAR shadow entries */
> +
> +#if SLB_NUM_BOLTED == 3

You could alternatively check the persistent entry in the slb_shawdow
buffer.  This would give you a run time check.  Not sure what's best
though.  

> +
> +	ld	r12, PACA_SLBSHADOWPTR(r13)
> +	ld	r10, 0x10(r12)
> +	ld	r11, 0x18(r12)

Can you define something in asm-offsets.c for these magic constants 0x10
and 0x18.  Similarly below.

> +	/* Invalid? Skip. */
> +	rldicl. r0, r10, 37, 63
> +	beq	slb_entry_skip_1
> +	xoris	r9, r10, SLB_ESID_V@h
> +	std	r9, 0x10(r12)
> +slb_entry_skip_1:
> +	ld	r9, 0x20(r12)
> +	/* Invalid? Skip. */
> +	rldicl. r0, r9, 37, 63
> +	beq	slb_entry_skip_2
> +	xoris	r9, r9, SLB_ESID_V@h
> +	std	r9, 0x20(r12)
> +slb_entry_skip_2:
> +	ld	r9, 0x30(r12)
> +	/* Invalid? Skip. */
> +	rldicl. r0, r9, 37, 63
> +	beq	slb_entry_skip_3
> +	xoris	r9, r9, SLB_ESID_V@h
> +	std	r9, 0x30(r12)

Can these 3 be made into a macro?

> +slb_entry_skip_3:
> +	
> +#else
> +#error unknown number of bolted entries
> +#endif
> +
> +	/* Flush SLB */
> +
> +	slbia
> +
> +	/* r0 = esid & ESID_MASK */
> +	rldicr  r10, r10, 0, 35
> +	/* r0 |= CLASS_BIT(VSID) */
> +	rldic   r12, r11, 56 - 36, 36
> +	or      r10, r10, r12
> +	slbie	r10
> +
> +	isync
> +
> +	/* Fill SLB with our shadow */
> +
> +	lbz	r12, PACA_KVM_SLB_MAX(r13)
> +	mulli	r12, r12, 16
> +	addi	r12, r12, PACA_KVM_SLB
> +	add	r12, r12, r13
> +
> +	/* for (r11 = kvm_slb; r11 < kvm_slb + kvm_slb_size; r11+=slb_entry) */
> +	li	r11, PACA_KVM_SLB
> +	add	r11, r11, r13
> +
> +slb_loop_enter:
> +
> +	ld	r10, 0(r11)
> +
> +	rldicl. r0, r10, 37, 63
> +	beq	slb_loop_enter_skip
> +
> +	ld	r9, 8(r11)
> +	slbmte	r9, r10

If you're updating the first 3 slbs, you need to make sure the slb
shadow is updated at the same time (BTW dumb question: can we run this
under PHYP?)

> +
> +slb_loop_enter_skip:
> +	addi	r11, r11, 16
> +	cmpd	cr0, r11, r12
> +	blt	slb_loop_enter
> +
> +slb_do_enter:
> +
> +	/* Enter guest */
> +
> +	mfspr	r0, SPRN_SPRG_SCRATCH0
> +
> +	ld	r9, (PACA_EXMC+EX_R9)(r13)
> +	ld	r10, (PACA_EXMC+EX_R10)(r13)
> +	ld	r12, (PACA_EXMC+EX_R12)(r13)
> +
> +	lwz	r11, (PACA_EXMC+EX_CCR)(r13)
> +	mtcr	r11
> +
> +	ld	r11, (PACA_EXMC+EX_R3)(r13)
> +	mtxer	r11
> +
> +	ld	r11, (PACA_EXMC+EX_R11)(r13)
> +	ld	r13, (PACA_EXMC+EX_R13)(r13)
> +
> +	RFI
> +kvmppc_handler_trampoline_enter_end:
> +
> +
> +
> +/***************************************************************************
***
> + *                                                                          
  *
> + *                               Exit code                                  
  *
> + *                                                                          
  *
> + ***************************************************************************
**/
> +
> +.global kvmppc_handler_trampoline_exit
> +kvmppc_handler_trampoline_exit:
> +
> +	/* Register usage at this point:
> +	 *
> +	 * SPRG_SCRATCH0 = guest R13
> +	 * R01           = host R1
> +	 * R02           = host R2
> +	 * R10           = guest PC
> +	 * R11           = guest MSR
> +	 * R12           = exit handler id
> +	 * R13           = PACA
> +	 * PACA.exmc.CCR  = guest CR
> +	 * PACA.exmc.R9  = guest R1
> +	 * PACA.exmc.R10 = guest R10
> +	 * PACA.exmc.R11 = guest R11
> +	 * PACA.exmc.R12 = guest R12
> +	 * PACA.exmc.R13 = guest R2
> +	 *
> +	 */
> +
> +	/* Save registers */
> +
> +	std	r0, (PACA_EXMC+EX_SRR0)(r13)
> +	std	r9, (PACA_EXMC+EX_R3)(r13)
> +	std	r10, (PACA_EXMC+EX_LR)(r13)
> +	std	r11, (PACA_EXMC+EX_DAR)(r13)
> +
> +	/*
> +	 * In order for us to easily get the last instruction,
> +	 * we got the #vmexit at, we exploit the fact that the
> +	 * virtual layout is still the same here, so we can just
> +	 * ld from the guest's PC address
> +	 */
> +
> +	/* We only load the last instruction when it's safe */
> +	cmpwi	r12, BOOK3S_INTERRUPT_DATA_STORAGE
> +	beq	ld_last_inst
> +	cmpwi	r12, BOOK3S_INTERRUPT_PROGRAM
> +	beq	ld_last_inst
> +
> +	b	no_ld_last_inst
> +
> +ld_last_inst:
> +	/* Save off the guest instruction we're at */
> +	/*    1) enable paging for data */
> +	mfmsr	r9
> +	ori	r11, r9, MSR_DR			/* Enable paging for data */
> +	mtmsr	r11
> +	/*    2) fetch the instruction */
> +	lwz	r0, 0(r10)
> +	/*    3) disable paging again */
> +	mtmsr	r9
> +
> +no_ld_last_inst:
> +
> +	/* Restore bolted entries from the shadow and fix it along the way */
> +
> +	/* We don't store anything in entry 0, so we don't need to take care of
 that */
> +	slbia
> +	isync
> +
> +#if SLB_NUM_BOLTED == 3
> +
> +	ld	r11, PACA_SLBSHADOWPTR(r13)
> +
> +	ld	r10, 0x10(r11)
> +	cmpdi	r10, 0
> +	beq	slb_exit_skip_1
> +	oris	r10, r10, SLB_ESID_V@h
> +	ld	r9, 0x18(r11)
> +	slbmte	r9, r10
> +	std	r10, 0x10(r11)
> +slb_exit_skip_1:
> +	
> +	ld	r10, 0x20(r11)
> +	cmpdi	r10, 0
> +	beq	slb_exit_skip_2
> +	oris	r10, r10, SLB_ESID_V@h
> +	ld	r9, 0x28(r11)
> +	slbmte	r9, r10
> +	std	r10, 0x20(r11)
> +slb_exit_skip_2:
> +	
> +	ld	r10, 0x30(r11)
> +	cmpdi	r10, 0
> +	beq	slb_exit_skip_3
> +	oris	r10, r10, SLB_ESID_V@h
> +	ld	r9, 0x38(r11)
> +	slbmte	r9, r10
> +	std	r10, 0x30(r11)
> +slb_exit_skip_3:

Again, a macro here?

> +	
> +#else
> +#error unknown number of bolted entries
> +#endif
> +
> +slb_do_exit:
> +
> +	/* Restore registers */
> +
> +	ld	r11, (PACA_EXMC+EX_DAR)(r13)
> +	ld	r10, (PACA_EXMC+EX_LR)(r13)
> +	ld	r9, (PACA_EXMC+EX_R3)(r13)
> +
> +	/* Save last inst */
> +	stw	r0, (PACA_EXMC+EX_LR)(r13)
> +
> +	/* Save DAR and DSISR before going to paged mode */
> +	mfdar	r0
> +	std	r0, (PACA_EXMC+EX_DAR)(r13)
> +	mfdsisr	r0
> +	stw	r0, (PACA_EXMC+EX_DSISR)(r13)
> +
> +	/* RFI into the highmem handler */
> +	mfmsr	r0
> +	ori	r0, r0, MSR_IR|MSR_DR|MSR_RI	/* Enable paging */
> +	mtsrr1	r0
> +	ld	r0, PACASAVEDMSR(r13)		/* Highmem handler address */
> +	mtsrr0	r0
> +
> +	mfspr	r0, SPRN_SPRG_SCRATCH0
> +
> +	RFI
> +kvmppc_handler_trampoline_exit_end:
> +
> -- 
> 1.6.0.2
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 

^ permalink raw reply

* Re: [PATCH 11/27] Add book3s_64 Host MMU handling
From: Michael Neuling @ 2009-11-01 23:39 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Kevin Wolf, Arnd Bergmann, Hollis Blanchard, Marcelo Tosatti,
	kvm-ppc, linuxppc-dev, Avi Kivity, kvm, bphilips, Olof Johansson
In-Reply-To: <1256917647-6200-12-git-send-email-agraf@suse.de>

<snip>
> +static void invalidate_pte(struct hpte_cache *pte)
> +{
> +	dprintk_mmu("KVM: Flushing SPT %d: 0x%llx (0x%llx) -> 0x%llx\n",
> +		    i, pte->pte.eaddr, pte->pte.vpage, pte->host_va);
> +
> +	ppc_md.hpte_invalidate(pte->slot, pte->host_va,
> +			       MMU_PAGE_4K, MMU_SEGSIZE_256M,
> +			       false);

Are we assuming 256M segments here (and elsewhere)?

<snip>
> +static int kvmppc_mmu_next_segment(struct kvm_vcpu *vcpu, ulong esid)
> +{
> +	int i;
> +	int max_slb_size = 64;
> +	int found_inval = -1;
> +	int r;
> +
> +	if (!get_paca()->kvm_slb_max)
> +		get_paca()->kvm_slb_max = 1;
> +
> +	/* Are we overwriting? */
> +	for (i = 1; i < get_paca()->kvm_slb_max; i++) {
> +		if (!(get_paca()->kvm_slb[i].esid & SLB_ESID_V))
> +			found_inval = i;
> +		else if ((get_paca()->kvm_slb[i].esid & ESID_MASK) == esid)
> +			return i;
> +	}
> +
> +	/* Found a spare entry that was invalidated before */
> +	if (found_inval > 0)
> +		return found_inval;
> +
> +	/* No spare invalid entry, so create one */
> +
> +	if (mmu_slb_size < 64)
> +		max_slb_size = mmu_slb_size;

Can we just use the global mmu_slb_size eliminate max_slb_size?

<snip>

Mikey

^ permalink raw reply

* [PATCH] powerpc: Avoid giving out RTC dates below EPOCH
From: Benjamin Herrenschmidt @ 2009-11-02  5:11 UTC (permalink / raw)
  To: linuxppc-dev

Doing so causes xtime to be negative which crashes the timekeeping
code in funny ways when doing suspend/resume

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 92dc844..6a7ce0e 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -777,7 +777,7 @@ int update_persistent_clock(struct timespec now)
 	return ppc_md.set_rtc_time(&tm);
 }
 
-void read_persistent_clock(struct timespec *ts)
+static void __read_persistent_clock(struct timespec *ts)
 {
 	struct rtc_time tm;
 	static int first = 1;
@@ -800,10 +800,23 @@ void read_persistent_clock(struct timespec *ts)
 		return;
 	}
 	ppc_md.get_rtc_time(&tm);
+
 	ts->tv_sec = mktime(tm.tm_year+1900, tm.tm_mon+1, tm.tm_mday,
 			    tm.tm_hour, tm.tm_min, tm.tm_sec);
 }
 
+void read_persistent_clock(struct timespec *ts)
+{
+	__read_persistent_clock(&ts);
+
+	/* Sanitize it in case real time clock is set below EPOCH */
+	if (ts->tv_sec < 0) {
+		ts->tv_sec = 0;
+		ts->tv_nsec = 0;
+	}
+		
+}
+
 /* clocksource code */
 static cycle_t rtc_read(struct clocksource *cs)
 {

^ permalink raw reply related

* Re: [PATCH] powerpc: Avoid giving out RTC dates below EPOCH
From: Benjamin Herrenschmidt @ 2009-11-02  5:13 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <1257138663.7907.32.camel@pasglop>

On Mon, 2009-11-02 at 16:11 +1100, Benjamin Herrenschmidt wrote:
> Doing so causes xtime to be negative which crashes the timekeeping
> code in funny ways when doing suspend/resume
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---

> +void read_persistent_clock(struct timespec *ts)
> +{
> +	__read_persistent_clock(&ts);

Should read

+	__read_persistent_clock(ts);

Forgot a quilt ref ;-)

Cheers,
Ben.

> +	/* Sanitize it in case real time clock is set below EPOCH */
> +	if (ts->tv_sec < 0) {
> +		ts->tv_sec = 0;
> +		ts->tv_nsec = 0;
> +	}
> +		
> +}
> +
>  /* clocksource code */
>  static cycle_t rtc_read(struct clocksource *cs)
>  {
> 
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* [powerpc] Next tree Nov 2 : kernel BUG at mm/mmap.c:2135!
From: Sachin Sant @ 2009-11-02  9:12 UTC (permalink / raw)
  To: Linux/PPC Development; +Cc: Stephen Rothwell, linux-next
In-Reply-To: <20091102173845.210d1c57.sfr@canb.auug.org.au>

[-- Attachment #1: Type: text/plain, Size: 4210 bytes --]

Stephen Rothwell wrote:
> Hi all,
>
> Changes since 20091030:
>   
Today's next tree failed to boot on a POWER 6 box with :

------------[ cut here ]------------
kernel BUG at mm/mmap.c:2135!
Oops: Exception in kernel mode, sig: 5 [#2]
SMP NR_CPUS=1024 NUMA pSeries
Modules linked in: ibmvscsic scsi_transport_srp scsi_tgt scsi_mod
NIP: c00000000014e30c LR: c00000000014e2f8 CTR: c00000000014db88
REGS: c0000000db703620 TRAP: 0700   Tainted: G      D     (2.6.32-rc5-autotest-next-20091102)
MSR: 8000000000029032 <EE,ME,CE,IR,DR>  CR: 24022442  XER: 2000000c
TASK = c0000000db7f6fe0[76] 'init' THREAD: c0000000db700000 CPU: 1
GPR00: 0000000000000001 c0000000db7038a0 c000000000b19900 0000000000000000
GPR04: c0000000db406a40 000000000000000c c0000000fe10c370 c000000000bb2800
GPR08: 000000000000db40 0000000000000000 c0000000dfdc0e00 000000000000000c
GPR12: 0000000044022442 c000000000bb2800 00000000ffffffff ffffffffffffffff
GPR16: 0000000008430000 00000000003c0000 c0000000db703ea0 c0000000db569108
GPR20: c0000000db568908 0000000000000000 c0000000db703d60 0000000000000000
GPR24: 0000000000000001 0000000000040100 c0000000fe503580 c0000000db1ac180
GPR28: 0000000000000000 c000000000f812d0 c000000000a84f00 0000000000000000
NIP [c00000000014e30c] .exit_mmap+0x190/0x1b8
LR [c00000000014e2f8] .exit_mmap+0x17c/0x1b8
Call Trace:
[c0000000db7038a0] [c00000000014e2f8] .exit_mmap+0x17c/0x1b8 (unreliable)
[c0000000db703950] [c0000000000916cc] .mmput+0x54/0x164
[c0000000db7039e0] [c0000000000968d8] .exit_mm+0x17c/0x1a0
[c0000000db703a90] [c000000000098cb8] .do_exit+0x248/0x784
[c0000000db703b70] [c0000000000992a8] .do_group_exit+0xb4/0xe8
[c0000000db703c00] [c0000000000aca2c] .get_signal_to_deliver+0x3ec/0x478
[c0000000db703cf0] [c0000000000134ac] .do_signal+0x6c/0x31c
[c0000000db703e30] [c000000000008b7c] do_work+0x24/0x28
Instruction dump:
7c8407b4 387d0018 4800ab11 60000000 939d0008 7fe3fb78 4bfffdbd 7c7f1b79
4082fff4 e81b00e8 3120ffff 7c090110 <0b000000> 382100b0 e8010010 eb61ffd8
---[ end trace ec052ac77a8e7cb4 ]---
Fixing recursive fault but reboot is needed!

mm/mmap.c:2135 corresponds to :

BUG_ON(mm->nr_ptes > (FIRST_USER_ADDRESS+PMD_SIZE-1)>>PMD_SHIFT);

3:mon> di 0xc00000000014e300
c00000000014e300  e81b00e8      ld      r0,232(r27)
c00000000014e304  3120ffff      addic   r9,r0,-1
c00000000014e308  7c090110      subfe   r0,r9,r0
c00000000014e30c  0b000000      tdnei   r0,0
c00000000014e310  382100b0      addi    r1,r1,176
c00000000014e314  e8010010      ld      r0,16(r1)
c00000000014e318  eb61ffd8      ld      r27,-40(r1)
c00000000014e31c  7c0803a6      mtlr    r0
c00000000014e320  eb81ffe0      ld      r28,-32(r1)
c00000000014e324  eba1ffe8      ld      r29,-24(r1)
c00000000014e328  ebc1fff0      ld      r30,-16(r1)
c00000000014e32c  ebe1fff8      ld      r31,-8(r1)
c00000000014e330  4e800020      blr
c00000000014e334  fb21ffc8      std     r25,-56(r1)
c00000000014e338  7c0802a6      mflr    r0
c00000000014e33c  fb81ffe0      std     r28,-32(r1)
3:mon> r
R00 = 0000000000000001   R16 = 0000000030daf420
R01 = c0000000fae37a60   R17 = 0000000000000002
R02 = c000000000b19900   R18 = 0000000000000000
R03 = 0000000000000000   R19 = 0000000030d903b0
R04 = c0000000fabd8670   R20 = 0000000000000000
R05 = c000000000a4d600   R21 = 0000000000000000
R06 = 0000000000000003   R22 = 0000000000000000
R07 = c000000000bb2c00   R23 = 0000000030daf340
R08 = 000000000000fabd   R24 = 0000000000000001
R09 = 0000000000000000   R25 = 00000fffc2402158
R10 = c0000000fffb2958   R26 = 00000fffb793df80
R11 = 0000000000000005   R27 = c0000000fa9aa580
R12 = 0000000044000442   R28 = 0000000000000000
R13 = c000000000bb2c00   R29 = c000000000fc12d0
R14 = 00000000ffffffff   R30 = c000000000a84f00
R15 = ffffffffffffffff   R31 = 0000000000000000
pc  = c00000000014e30c .exit_mmap+0x190/0x1b8
lr  = c00000000014e2f8 .exit_mmap+0x17c/0x1b8
msr = 8000000000029032   cr  = 24000442
ctr = c0000000002f0cb0   xer = 000000002000000a   trap =  700
3:mon>

Have attached the boot log. Next tree for 20091030 worked fine.

Thanks
-Sachin

-- 

---------------------------------
Sachin Sant
IBM Linux Technology Center
India Systems and Technology Labs
Bangalore, India
---------------------------------


[-- Attachment #2: boot-log --]
[-- Type: text/plain, Size: 10695 bytes --]

Using 007cf32f bytes for initrd buffer
Please wait, loading kernel...
Allocated 00f00000 bytes for kernel @ 02300000
   Elf64 kernel loaded...
Loading ramdisk...
ramdisk loaded 007cf32f @ 03200000
OF stdout device is: /vdevice/vty@30000000
Preparing to boot Linux version 2.6.32-rc5-autotest-next-20091102 (root@mpower6lp5) (gcc version 4.3.2 [gcc-4_3-branch revision 141291] (SUSE Linux) ) #1 SMP Mon Nov 2 12:29:14 IST 2009
Calling ibm,client-architecture-support... done
command line: root=/dev/sda3 sysrq=8 insmod=sym53c8xx insmod=ipr crashkernel=512M-:256M IDENT=1257146334 
memory layout at init:
  memory_limit : 0000000000000000 (16 MB aligned)
  alloc_bottom : 00000000039d0000
  alloc_top    : 0000000008000000
  alloc_top_hi : 0000000008000000
  rmo_top      : 0000000008000000
  ram_top      : 0000000008000000
instantiating rtas at 0x00000000074e0000... done
boot cpu hw idx 0000000000000000
copying OF device tree...
Building dt strings...
Building dt structure...
Device tree strings 0x00000000039e0000 -> 0x00000000039e15c2
Device tree struct  0x00000000039f0000 -> 0x0000000003a10000
Calling quiesce...
returning from prom_init
Crash kernel location must be 0x2000000
Reserving 256MB of memory at 32MB for crashkernel (System RAM: 4096MB)
Using pSeries machine description
Using 1TB segments
Found initrd at 0xc000000003200000:0xc0000000039cf32f
bootconsole [udbg0] enabled
Partition configured for 2 cpus.
CPU maps initialized for 2 threads per core
Starting Linux PPC64 #1 SMP Mon Nov 2 12:29:14 IST 2009
-----------------------------------------------------
ppc64_pft_size                = 0x1a
physicalMemorySize            = 0x100000000
htab_hash_mask                = 0x7ffff
-----------------------------------------------------
Initializing cgroup subsys cpuset
Initializing cgroup subsys cpu
Linux version 2.6.32-rc5-autotest-next-20091102 (root@mpower6lp5) (gcc version 4.3.2 [gcc-4_3-branch revision 141291] (SUSE Linux) ) #1 SMP Mon Nov 2 12:29:14 IST 2009
[boot]0012 Setup Arch
EEH: No capable adapters found
PPC64 nvram contains 15360 bytes
Zone PFN ranges:
  DMA      0x00000000 -> 0x00010000
  Normal   0x00010000 -> 0x00010000
Movable zone start PFN for each node
early_node_map[2] active PFN ranges
    2: 0x00000000 -> 0x0000e000
    3: 0x0000e000 -> 0x00010000
Could not find start_pfn for node 0
[boot]0015 Setup Done
PERCPU: Embedded 2 pages/cpu @c000000000f00000 s89000 r0 d42072 u524288
pcpu-alloc: s89000 r0 d42072 u524288 alloc=1*1048576
pcpu-alloc: [0] 0 1 
Built 3 zonelists in Node order, mobility grouping on.  Total pages: 65480
Policy zone: DMA
Kernel command line: root=/dev/sda3 sysrq=8 insmod=sym53c8xx insmod=ipr crashkernel=512M-:256M IDENT=1257146334 
PID hash table entries: 4096 (order: -1, 32768 bytes)
freeing bootmem node 2
freeing bootmem node 3
Memory: 3899776k/4194304k available (9216k kernel code, 294528k reserved, 2688k data, 2370k bss, 640k init)
Hierarchical RCU implementation.
RCU-based detection of stalled CPUs is enabled.
NR_IRQS:512 nr_irqs:512
[boot]0020 XICS Init
[boot]0021 XICS Done
clocksource: timebase mult[7d0000] shift[22] registered
Console: colour dummy device 80x25
console [hvc0] enabled, bootconsole disabled
console [hvc0] enabled, bootconsole disabled
allocated 2621440 bytes of page_cgroup
please try 'cgroup_disable=memory' option if you don't want memory cgroups
Security Framework initialized
SELinux:  Disabled at boot.
Dentry cache hash table entries: 524288 (order: 6, 4194304 bytes)
Inode-cache hash table entries: 262144 (order: 5, 2097152 bytes)
Mount-cache hash table entries: 4096
Initializing cgroup subsys ns
Initializing cgroup subsys cpuacct
Initializing cgroup subsys memory
Initializing cgroup subsys devices
Initializing cgroup subsys freezer
Processor 1 found.
Brought up 2 CPUs
NET: Registered protocol family 16
IBM eBus Device Driver
POWER6 performance monitor hardware support registered
PCI: Probing PCI hardware
bio: create slab <bio-0> at 0
vgaarb: loaded
usbcore: registered new interface driver usbfs
usbcore: registered new interface driver hub
usbcore: registered new device driver usb
Switching to clocksource timebase
NET: Registered protocol family 2
IP route cache hash table entries: 32768 (order: 2, 262144 bytes)
TCP established hash table entries: 131072 (order: 5, 2097152 bytes)
TCP bind hash table entries: 65536 (order: 4, 1048576 bytes)
TCP: Hash tables configured (established 131072 bind 65536)
TCP reno registered
UDP hash table entries: 4096 (order: 0, 65536 bytes)
UDP-Lite hash table entries: 4096 (order: 0, 65536 bytes)
NET: Registered protocol family 1
Unpacking initramfs...
IOMMU table initialized, virtual merging enabled
audit: initializing netlink socket (disabled)
type=2000 audit(1257150470.200:1): initialized
rcu-torture:--- Start of test: nreaders=4 nfakewriters=4 stat_interval=0 verbose=0 test_no_idle_hz=0 shuffle_interval=3 stutter=5 irqreader=1
HugeTLB registered 16 MB page size, pre-allocated 0 pages
HugeTLB registered 16 GB page size, pre-allocated 0 pages
VFS: Disk quotas dquot_6.5.2
Dquot-cache hash table entries: 8192 (order 0, 65536 bytes)
msgmni has been set to 7616
alg: No test for stdrng (krng)
Block layer SCSI generic (bsg) driver version 0.4 loaded (major 254)
io scheduler noop registered
io scheduler deadline registered
io scheduler cfq registered (default)
pci_hotplug: PCI Hot Plug PCI Core version: 0.5
pciehp: PCI Express Hot Plug Controller Driver version: 0.4
rpaphp: RPA HOT Plug PCI Controller Driver version: 0.1
Generic RTC Driver v1.07
Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
pmac_zilog: 0.6 (Benjamin Herrenschmidt <benh@kernel.crashing.org>)
input: Macintosh mouse button emulation as /devices/virtual/input/input0
Uniform Multi-Platform E-IDE driver
ide-gd driver 1.18
ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
mice: PS/2 mouse device common for all mice
EDAC MC: Ver: 2.1.0 Nov  2 2009
usbcore: registered new interface driver hiddev
usbcore: registered new interface driver usbhid
usbhid: USB HID core driver
TCP cubic registered
NET: Registered protocol family 15
registered taskstats version 1
Freeing unused kernel memory: 640k freed
doing fast boot
------------[ cut here ]------------
kernel BUG at mm/mmap.c:2135!
Oops: Exception in kernel mode, sig: 5 [#1]
SMP NR_CPUS=1024 NUMA pSeries
Modules linked in:
NIP: c00000000014e30c LR: c00000000014e2f8 CTR: c0000000002f0cb0
REGS: c0000000db7337e0 TRAP: 0700   Not tainted  (2.6.32-rc5-autotest-next-20091102)
MSR: 8000000000029032 <EE,ME,CE,IR,DR>  CR: 24000442  XER: 20000002
TASK = c0000000db618a60[68] 'showconsole' THREAD: c0000000db730000 CPU: 0
GPR00: 0000000000000001 c0000000db733a60 c000000000b19900 0000000000000000 
GPR04: c0000000db408670 c000000000a4d600 0000000000000003 c000000000bb2600 
GPR08: 000000000000db40 0000000000000000 c0000000dfdc0e00 0000000000000005 
GPR12: 0000000044000442 c000000000bb2600 00000000ffffffff ffffffffffffffff 
GPR16: 000000004206e7d0 0000000000000002 0000000000000000 00000000420503b0 
GPR20: 0000000000000000 0000000000000000 0000000000000000 000000004206fd70 
GPR24: 0000000000000001 00000fffc0ac9b08 00000fff99b1df80 c0000000db1aa580 
GPR28: 0000000000000000 c000000000f012d0 c000000000a84f00 0000000000000000 
NIP [c00000000014e30c] .exit_mmap+0x190/0x1b8
LR [c00000000014e2f8] .exit_mmap+0x17c/0x1b8
Call Trace:
[c0000000db733a60] [c00000000014e2f8] .exit_mmap+0x17c/0x1b8 (unreliable)
[c0000000db733b10] [c0000000000916cc] .mmput+0x54/0x164
[c0000000db733ba0] [c0000000000968d8] .exit_mm+0x17c/0x1a0
[c0000000db733c50] [c000000000098cb8] .do_exit+0x248/0x784
[c0000000db733d30] [c0000000000992a8] .do_group_exit+0xb4/0xe8
[c0000000db733dc0] [c0000000000992f0] .SyS_exit_group+0x14/0x28
[c0000000db733e30] [c0000000000085b4] syscall_exit+0x0/0x40
Instruction dump:
7c8407b4 387d0018 4800ab11 60000000 939d0008 7fe3fb78 4bfffdbd 7c7f1b79 
4082fff4 e81b00e8 3120ffff 7c090110 <0b000000> 382100b0 e8010010 eb61ffd8 
SysRq : Changing Loglevel
Loglevel set to 8
---[ end trace ec052ac77a8e7cb3 ]---
Fixing recursive fault but reboot is needed!
SCSI subsystem initialized
vio_register_driver: driver ibmvscsi registering
ibmvscsi 30000007: SRP_VERSION: 16.a
scsi0 : IBM POWER Virtual SCSI Adapter 1.5.8
ibmvscsi 30000007: partner initialization complete
ibmvscsi 30000007: host srp version: 16.a, host partition VIO Server (1), OS 3, max io 1048576
ibmvscsi 30000007: Client reserve enabled
ibmvscsi 30000007: sent SRP login
ibmvscsi 30000007: SRP_LOGIN succeeded
scsi 0:0:1:0: Direct-Access     AIX      VDASD            0001 PQ: 0 ANSI: 3
scsi 0:0:2:0: CD-ROM            AIX      VOPTA                 PQ: 0 ANSI: 4
Creating device nodes with udev
------------[ cut here ]------------
kernel BUG at mm/mmap.c:2135!
Oops: Exception in kernel mode, sig: 5 [#2]
SMP NR_CPUS=1024 NUMA pSeries
Modules linked in: ibmvscsic scsi_transport_srp scsi_tgt scsi_mod
NIP: c00000000014e30c LR: c00000000014e2f8 CTR: c00000000014db88
REGS: c0000000db703620 TRAP: 0700   Tainted: G      D     (2.6.32-rc5-autotest-next-20091102)
MSR: 8000000000029032 <EE,ME,CE,IR,DR>  CR: 24022442  XER: 2000000c
TASK = c0000000db7f6fe0[76] 'init' THREAD: c0000000db700000 CPU: 1
GPR00: 0000000000000001 c0000000db7038a0 c000000000b19900 0000000000000000 
GPR04: c0000000db406a40 000000000000000c c0000000fe10c370 c000000000bb2800 
GPR08: 000000000000db40 0000000000000000 c0000000dfdc0e00 000000000000000c 
GPR12: 0000000044022442 c000000000bb2800 00000000ffffffff ffffffffffffffff 
GPR16: 0000000008430000 00000000003c0000 c0000000db703ea0 c0000000db569108 
GPR20: c0000000db568908 0000000000000000 c0000000db703d60 0000000000000000 
GPR24: 0000000000000001 0000000000040100 c0000000fe503580 c0000000db1ac180 
GPR28: 0000000000000000 c000000000f812d0 c000000000a84f00 0000000000000000 
NIP [c00000000014e30c] .exit_mmap+0x190/0x1b8
LR [c00000000014e2f8] .exit_mmap+0x17c/0x1b8
Call Trace:
[c0000000db7038a0] [c00000000014e2f8] .exit_mmap+0x17c/0x1b8 (unreliable)
[c0000000db703950] [c0000000000916cc] .mmput+0x54/0x164
[c0000000db7039e0] [c0000000000968d8] .exit_mm+0x17c/0x1a0
[c0000000db703a90] [c000000000098cb8] .do_exit+0x248/0x784
[c0000000db703b70] [c0000000000992a8] .do_group_exit+0xb4/0xe8
[c0000000db703c00] [c0000000000aca2c] .get_signal_to_deliver+0x3ec/0x478
[c0000000db703cf0] [c0000000000134ac] .do_signal+0x6c/0x31c
[c0000000db703e30] [c000000000008b7c] do_work+0x24/0x28
Instruction dump:
7c8407b4 387d0018 4800ab11 60000000 939d0008 7fe3fb78 4bfffdbd 7c7f1b79 
4082fff4 e81b00e8 3120ffff 7c090110 <0b000000> 382100b0 e8010010 eb61ffd8 
---[ end trace ec052ac77a8e7cb4 ]---
Fixing recursive fault but reboot is needed!




^ permalink raw reply

* Re: [PATCH 08/27] Add SLB switching code for entry/exit
From: Alexander Graf @ 2009-11-02  9:23 UTC (permalink / raw)
  To: Michael Neuling
  Cc: Kevin Wolf, Arnd Bergmann, Hollis Blanchard, Marcelo Tosatti,
	kvm-ppc, linuxppc-dev@ozlabs.org, Avi Kivity, kvm@vger.kernel.org,
	bphilips@suse.de, Olof Johansson
In-Reply-To: <6695.1257117827@neuling.org>


Am 02.11.2009 um 00:23 schrieb Michael Neuling <mikey@neuling.org>:

>> This is the really low level of guest entry/exit code.
>>
>> Book3s_64 has an SLB, which stores all ESID -> VSID mappings we're
>> currently aware of.
>>
>> The segments in the guest differ from the ones on the host, so we  
>> need
>> to switch the SLB to tell the MMU that we're in a new context.
>>
>> So we store a shadow of the guest's SLB in the PACA, switch to that  
>> on
>> entry and only restore bolted entries on exit, leaving the rest to  
>> the
>> Linux SLB fault handler.
>>
>> That way we get a really clean way of switching the SLB.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> arch/powerpc/kvm/book3s_64_slb.S |  277 ++++++++++++++++++++++++++++ 
>> ++++++++
> ++
>> 1 files changed, 277 insertions(+), 0 deletions(-)
>> create mode 100644 arch/powerpc/kvm/book3s_64_slb.S
>>
>> diff --git a/arch/powerpc/kvm/book3s_64_slb.S b/arch/powerpc/kvm/ 
>> book3s_64_sl
> b.S
>> new file mode 100644
>> index 0000000..00a8367
>> --- /dev/null
>> +++ b/arch/powerpc/kvm/book3s_64_slb.S
>> @@ -0,0 +1,277 @@
>> +/*
>> + * 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.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA   
>> 02110-1301, USA.
>> + *
>> + * Copyright SUSE Linux Products GmbH 2009
>> + *
>> + * Authors: Alexander Graf <agraf@suse.de>
>> + */
>> +
>> +/ 
>> *** 
>> *** 
>> *********************************************************************
> ***
>> + *
>  *
>> + *                               Entry code
>  *
>> + *
>  *
>> +  
>> *** 
>> *** 
>> *********************************************************************
> **/
>> +
>> +.global kvmppc_handler_trampoline_enter
>> +kvmppc_handler_trampoline_enter:
>> +
>> +    /* Required state:
>> +     *
>> +     * MSR = ~IR|DR
>> +     * R13 = PACA
>> +     * R9 = guest IP
>> +     * R10 = guest MSR
>> +     * R11 = free
>> +     * R12 = free
>> +     * PACA[PACA_EXMC + EX_R9] = guest R9
>> +     * PACA[PACA_EXMC + EX_R10] = guest R10
>> +     * PACA[PACA_EXMC + EX_R11] = guest R11
>> +     * PACA[PACA_EXMC + EX_R12] = guest R12
>> +     * PACA[PACA_EXMC + EX_R13] = guest R13
>> +     * PACA[PACA_EXMC + EX_CCR] = guest CR
>> +     * PACA[PACA_EXMC + EX_R3] = guest XER
>> +     */
>> +
>> +    mtsrr0    r9
>> +    mtsrr1    r10
>> +
>> +    mtspr    SPRN_SPRG_SCRATCH0, r0
>> +
>> +    /* Remove LPAR shadow entries */
>> +
>> +#if SLB_NUM_BOLTED == 3
>
> You could alternatively check the persistent entry in the slb_shawdow
> buffer.  This would give you a run time check.  Not sure what's best
> though.

Well we're in the hot path here, so anything using as few registers as  
possible and being simple is the best :-). I'd guess the more we are  
clever at compile time the better.

>
>
>> +
>> +    ld    r12, PACA_SLBSHADOWPTR(r13)
>> +    ld    r10, 0x10(r12)
>> +    ld    r11, 0x18(r12)
>
> Can you define something in asm-offsets.c for these magic constants  
> 0x10
> and 0x18.  Similarly below.
>
>> +    /* Invalid? Skip. */
>> +    rldicl. r0, r10, 37, 63
>> +    beq    slb_entry_skip_1
>> +    xoris    r9, r10, SLB_ESID_V@h
>> +    std    r9, 0x10(r12)
>> +slb_entry_skip_1:
>> +    ld    r9, 0x20(r12)
>> +    /* Invalid? Skip. */
>> +    rldicl. r0, r9, 37, 63
>> +    beq    slb_entry_skip_2
>> +    xoris    r9, r9, SLB_ESID_V@h
>> +    std    r9, 0x20(r12)
>> +slb_entry_skip_2:
>> +    ld    r9, 0x30(r12)
>> +    /* Invalid? Skip. */
>> +    rldicl. r0, r9, 37, 63
>> +    beq    slb_entry_skip_3
>> +    xoris    r9, r9, SLB_ESID_V@h
>> +    std    r9, 0x30(r12)
>
> Can these 3 be made into a macro?

Phew - dynamically generating jump points sounds rather hard. I can  
give it a try...

>
>> +slb_entry_skip_3:
>> +
>> +#else
>> +#error unknown number of bolted entries
>> +#endif
>> +
>> +    /* Flush SLB */
>> +
>> +    slbia
>> +
>> +    /* r0 = esid & ESID_MASK */
>> +    rldicr  r10, r10, 0, 35
>> +    /* r0 |= CLASS_BIT(VSID) */
>> +    rldic   r12, r11, 56 - 36, 36
>> +    or      r10, r10, r12
>> +    slbie    r10
>> +
>> +    isync
>> +
>> +    /* Fill SLB with our shadow */
>> +
>> +    lbz    r12, PACA_KVM_SLB_MAX(r13)
>> +    mulli    r12, r12, 16
>> +    addi    r12, r12, PACA_KVM_SLB
>> +    add    r12, r12, r13
>> +
>> +    /* for (r11 = kvm_slb; r11 < kvm_slb + kvm_slb_size;  
>> r11+=slb_entry) */
>> +    li    r11, PACA_KVM_SLB
>> +    add    r11, r11, r13
>> +
>> +slb_loop_enter:
>> +
>> +    ld    r10, 0(r11)
>> +
>> +    rldicl. r0, r10, 37, 63
>> +    beq    slb_loop_enter_skip
>> +
>> +    ld    r9, 8(r11)
>> +    slbmte    r9, r10
>
> If you're updating the first 3 slbs, you need to make sure the slb
> shadow is updated at the same time

Well - what happens if we don't? We'd get a segment fault when phyp  
stole our entry! So what? Let it fault, see the mapping is already  
there and get back in again :-).

> (BTW dumb question: can we run this
> under PHYP?)

Yes, I tested it on bare metal, phyp and a PS3.


Alex

^ 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