diff mbox

mx28evk does not boot with linux-next 20130207

Message ID CAOMZO5CbJfpVwqHM1RSYdEQH4n1EL67Kt2_tC9TNPxZGMsPevQ@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Fabio Estevam Feb. 8, 2013, 5:14 p.m. UTC
On Fri, Feb 8, 2013 at 2:48 PM, Will Deacon <will.deacon@arm.com> wrote:

> From dc381a5dff9663901a61fe0dd86c986add382281 Mon Sep 17 00:00:00 2001
> From: Will Deacon <will.deacon@arm.com>
> Date: Fri, 8 Feb 2013 16:41:19 +0000
> Subject: [PATCH] ARM: tlb: fix branch predictor maintenance for ARMv6
>
> The BPIALL operation is not available on all CPUs, so ensure that we
> only execute it on processors implementing the instruction.
>
> Reported-by: Fabio Estevam <festevam@gmail.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Thanks, Will.

This makes the kernel to boot again on my mx28evk, so:

Tested-by: Fabio Estevam <fabio.estevam@freescale.com>

There are still other issues, which seem to be unrelated to this fix though:

I can see the Linux logo and LED activity on my board, but no serial console.

Applying Shawn's patch:

and turning on earlyprintk I see kernel booting and it stops at:

[    5.311281] TCP: cubic registered
[    5.315000] NET: Registered protocol family 17
[    5.320843] Key type dns_resolver registered
[    5.325750] turn off boot console earlycon0

Regards,

Fabio Estevam

Comments

Marek Vasut Feb. 10, 2013, 3:28 a.m. UTC | #1
Dear Fabio Estevam,

> On Fri, Feb 8, 2013 at 2:48 PM, Will Deacon <will.deacon@arm.com> wrote:
> > From dc381a5dff9663901a61fe0dd86c986add382281 Mon Sep 17 00:00:00 2001
> > From: Will Deacon <will.deacon@arm.com>
> > Date: Fri, 8 Feb 2013 16:41:19 +0000
> > Subject: [PATCH] ARM: tlb: fix branch predictor maintenance for ARMv6
> > 
> > The BPIALL operation is not available on all CPUs, so ensure that we
> > only execute it on processors implementing the instruction.
> > 
> > Reported-by: Fabio Estevam <festevam@gmail.com>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> 
> Thanks, Will.
> 
> This makes the kernel to boot again on my mx28evk, so:
> 
> Tested-by: Fabio Estevam <fabio.estevam@freescale.com>
> 
> There are still other issues, which seem to be unrelated to this fix
> though:
> 
> I can see the Linux logo and LED activity on my board, but no serial
> console.
> 
> Applying Shawn's patch:
> 
> diff --git a/arch/arm/boot/compressed/debug.S
> b/arch/arm/boot/compressed/debug.S
> index bdb0e25..6e8382d 100644
> --- a/arch/arm/boot/compressed/debug.S
> +++ b/arch/arm/boot/compressed/debug.S
> @@ -1,4 +1,5 @@
>  #include <linux/linkage.h>
> +#include <asm/assembler.h>
> 
>  #include CONFIG_DEBUG_LL_INCLUDE
> 
> and turning on earlyprintk I see kernel booting and it stops at:
> 
> [    5.311281] TCP: cubic registered
> [    5.315000] NET: Registered protocol family 17
> [    5.320843] Key type dns_resolver registered
> [    5.325750] turn off boot console earlycon0

Is the amba-pl011 driver maybe broken ?

So far, Wills' and Shawns' patches got me further too, thanks!

Best regards,
Marek Vasut
Fabio Estevam Feb. 10, 2013, 4:26 p.m. UTC | #2
On Sun, Feb 10, 2013 at 1:28 AM, Marek Vasut <marex@denx.de> wrote:

> Is the amba-pl011 driver maybe broken ?

At least on mx28evk, ttyAMA0 is not getting registered when running linux-next.

I haven't had a chance to debug it though.
Fabio Estevam Feb. 11, 2013, 1:30 p.m. UTC | #3
On Sun, Feb 10, 2013 at 1:28 AM, Marek Vasut <marex@denx.de> wrote:

> Is the amba-pl011 driver maybe broken ?

Running Linus' git I get:

[    0.189156] Serial: AMBA PL011 UART driver
[    0.191187] 80074000.serial: ttyAMA0 at MMIO 0x80074000 (irq = 221)
is a PL011 rev2
[    0.487625] console [ttyAMA0] enabled

,but when I run linux-next only the first line shows up.
Fabio Estevam Feb. 16, 2013, 5:41 p.m. UTC | #4
Hi Will,

On Fri, Feb 8, 2013 at 3:14 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Fri, Feb 8, 2013 at 2:48 PM, Will Deacon <will.deacon@arm.com> wrote:
>
>> From dc381a5dff9663901a61fe0dd86c986add382281 Mon Sep 17 00:00:00 2001
>> From: Will Deacon <will.deacon@arm.com>
>> Date: Fri, 8 Feb 2013 16:41:19 +0000
>> Subject: [PATCH] ARM: tlb: fix branch predictor maintenance for ARMv6
>>
>> The BPIALL operation is not available on all CPUs, so ensure that we
>> only execute it on processors implementing the instruction.
>>
>> Reported-by: Fabio Estevam <festevam@gmail.com>
>> Signed-off-by: Will Deacon <will.deacon@arm.com>
>
> Thanks, Will.
>
> This makes the kernel to boot again on my mx28evk, so:
>
> Tested-by: Fabio Estevam <fabio.estevam@freescale.com>

Do you plan to submit this patch to Russell's patch system?

Thanks,

Fabio Estevam
Russell King - ARM Linux Feb. 16, 2013, 5:56 p.m. UTC | #5
On Sat, Feb 16, 2013 at 03:41:53PM -0200, Fabio Estevam wrote:
> Hi Will,
> 
> On Fri, Feb 8, 2013 at 3:14 PM, Fabio Estevam <festevam@gmail.com> wrote:
> > On Fri, Feb 8, 2013 at 2:48 PM, Will Deacon <will.deacon@arm.com> wrote:
> >
> >> From dc381a5dff9663901a61fe0dd86c986add382281 Mon Sep 17 00:00:00 2001
> >> From: Will Deacon <will.deacon@arm.com>
> >> Date: Fri, 8 Feb 2013 16:41:19 +0000
> >> Subject: [PATCH] ARM: tlb: fix branch predictor maintenance for ARMv6
> >>
> >> The BPIALL operation is not available on all CPUs, so ensure that we
> >> only execute it on processors implementing the instruction.
> >>
> >> Reported-by: Fabio Estevam <festevam@gmail.com>
> >> Signed-off-by: Will Deacon <will.deacon@arm.com>
> >
> > Thanks, Will.
> >
> > This makes the kernel to boot again on my mx28evk, so:
> >
> > Tested-by: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Do you plan to submit this patch to Russell's patch system?

No, I've suggested to Will that we deal with the branch predictor a
different way, because we actually rarely need to flush it.  Plus,
these patches are architecturally wrong (you're not allowed to branch
between the BP maintanence instruction and the following isb/dsb
instructions because the branches become "unpredictable".

Plus, we _used_ to flush the BP here, and I did an amount of work to
remove the flushing because it just wasn't necessary - all the paths
at that time just didn't require it.

What we have ended up with today is a couple of paths which do, both
of which are arch-specific code, and we should deal with that in an
arch specific way.  Will has been working on a patch to do that...

In the mean time, I'm dropping 5d9e3f9d7fcd7d09feb5d23974768a60bcea4094
which added the BP maintanence (I actually said to Will that I'd already
done this but it appears not.)
Shawn Guo Feb. 17, 2013, 7:44 a.m. UTC | #6
On Sun, Feb 10, 2013 at 02:26:11PM -0200, Fabio Estevam wrote:
> On Sun, Feb 10, 2013 at 1:28 AM, Marek Vasut <marex@denx.de> wrote:
> 
> > Is the amba-pl011 driver maybe broken ?
> 
> At least on mx28evk, ttyAMA0 is not getting registered when running linux-next.
> 
> I haven't had a chance to debug it though.

Try reverting commit aac73f3 (of: use platform_device_add) and see if it
helps.

Shawn
Will Deacon Feb. 17, 2013, 7:18 p.m. UTC | #7
On Sat, Feb 16, 2013 at 05:56:13PM +0000, Russell King - ARM Linux wrote:
> On Sat, Feb 16, 2013 at 03:41:53PM -0200, Fabio Estevam wrote:
> > Hi Will,

Hello,

> > On Fri, Feb 8, 2013 at 3:14 PM, Fabio Estevam <festevam@gmail.com> wrote:
> > Do you plan to submit this patch to Russell's patch system?
> 
> No, I've suggested to Will that we deal with the branch predictor a
> different way, because we actually rarely need to flush it.  Plus,
> these patches are architecturally wrong (you're not allowed to branch
> between the BP maintanence instruction and the following isb/dsb
> instructions because the branches become "unpredictable".

I think the branches are actually ok, because the code in question (that is,
the kernel code dispatching to and performing the flush) is mapped
identically before and after the page table switch.

> Plus, we _used_ to flush the BP here, and I did an amount of work to
> remove the flushing because it just wasn't necessary - all the paths
> at that time just didn't require it.

This is the killer argument. We don't need the maintenance except in a few,
tiny places. I've written some patches for that and I'll post them to the
list in due course, aiming for 3.10.

> What we have ended up with today is a couple of paths which do, both
> of which are arch-specific code, and we should deal with that in an
> arch specific way.  Will has been working on a patch to do that...

Yup. I've also been looking at TLB invalidation as a whole and some
optimisations we can do on ARMv7 where we don't *actually* need to use the
inner-shareable variants. Non-shareable often suffices and is a win in terms
of both performance and power (it makes the deferred flushing on ASID
rollover usable by v7 too). Again, I've been hacking in this area so I'll
post patches for review once I've got a coherent story.

> In the mean time, I'm dropping 5d9e3f9d7fcd7d09feb5d23974768a60bcea4094
> which added the BP maintanence (I actually said to Will that I'd already
> done this but it appears not.)

Thanks Russell.

Will
diff mbox

Patch

diff --git a/arch/arm/boot/compressed/debug.S
b/arch/arm/boot/compressed/debug.S
index bdb0e25..6e8382d 100644
--- a/arch/arm/boot/compressed/debug.S
+++ b/arch/arm/boot/compressed/debug.S
@@ -1,4 +1,5 @@ 
 #include <linux/linkage.h>
+#include <asm/assembler.h>

 #include CONFIG_DEBUG_LL_INCLUDE