[RFC] arm: Defer lookup of machine_type and vet of atags to setup.c

[RFC] arm: Defer lookup of machine_type and vet of atags to setup.c

[Grant Likely]

Since the debug macros no longer depend on the machine type
information, both the machine type lookup and the atags vetting can be
deferred to setup_arch() in setup.c which simplifies the code
somewhat.

This patch removes both __machine_type_lookup and __vet_atags() from
head.S.  The atags vetting is moved to setup_arch().  machine_type
lookup is already called from setup_machine() in addition to where it
was called from head.S.

I've tried to preserve the existing behaviour in this patch so the
extra atags vetting is only using when CONFIG_MMU is selected.  I may
be being overly cautious, and if so then it is probably possible to
simplify the code further.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

Hi Russell,

I'm not sure if this is a valid change or not, but from what I can
tell it looks like machine and atag processing no longer needs
to be handled as early as head.S.  Please take a look and let me know
what you think.

I've boot tested this on Tegra and versatile qemu, but that's about
it.

Thanks,
g.

 arch/arm/kernel/head-common.S |   35 -----------------------------------
 arch/arm/kernel/head.S        |    5 -----
 arch/arm/kernel/setup.c       |   16 ++++++++++++++++
 3 files changed, 16 insertions(+), 40 deletions(-)

diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S
index bbecaac..7956a48 100644
--- a/arch/arm/kernel/head-common.S
+++ b/arch/arm/kernel/head-common.S
@@ -11,10 +11,6 @@
  *
  */
 
-#define ATAG_CORE 0x54410001
-#define ATAG_CORE_SIZE ((2*4 + 3*4) >> 2)
-#define ATAG_CORE_SIZE_EMPTY ((2*4) >> 2)
-
 /*
  * Exception handling.  Something went wrong and we can't proceed.  We
  * ought to tell the user, but since we don't have any guarantee that
@@ -101,37 +97,6 @@ __lookup_machine_type_data:
 	.long	__arch_info_end
 	.size	__lookup_machine_type_data, . - __lookup_machine_type_data
 
-/* Determine validity of the r2 atags pointer.  The heuristic requires
- * that the pointer be aligned, in the first 16k of physical RAM and
- * that the ATAG_CORE marker is first and present.  Future revisions
- * of this function may be more lenient with the physical address and
- * may also be able to move the ATAGS block if necessary.
- *
- * r8  = machinfo
- *
- * Returns:
- *  r2 either valid atags pointer, or zero
- *  r5, r6 corrupted
- */
-__vet_atags:
-	tst	r2, #0x3			@ aligned?
-	bne	1f
-
-	ldr	r5, [r2, #0]			@ is first tag ATAG_CORE?
-	cmp	r5, #ATAG_CORE_SIZE
-	cmpne	r5, #ATAG_CORE_SIZE_EMPTY
-	bne	1f
-	ldr	r5, [r2, #4]
-	ldr	r6, =ATAG_CORE
-	cmp	r5, r6
-	bne	1f
-
-	mov	pc, lr				@ atag pointer is ok
-
-1:	mov	r2, #0
-	mov	pc, lr
-ENDPROC(__vet_atags)
-
 /*
  * The following fragment of code is executed with the MMU on in MMU mode,
  * and uses absolute addresses; this is not position independent.
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 6bd82d2..9c0e938 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -87,11 +87,6 @@ ENTRY(stext)
 	movs	r10, r5				@ invalid processor (r5=0)?
  THUMB( it	eq )		@ force fixup-able long branch encoding
 	beq	__error_p			@ yes, error 'p'
-	bl	__lookup_machine_type		@ r5=machinfo
-	movs	r8, r5				@ invalid machine (r5=0)?
- THUMB( it	eq )		@ force fixup-able long branch encoding
-	beq	__error_a			@ yes, error 'a'
-	bl	__vet_atags
 #ifdef CONFIG_SMP_ON_UP
 	bl	__fixup_smp
 #endif
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 336f14e..cd28089 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -814,6 +814,22 @@ void __init setup_arch(char **cmdline_p)
 	if (mdesc->soft_reboot)
 		reboot_setup("s");
 
+#if defined(CONFIG_MMU)
+	/*
+	 * Determine validity of the atags pointer.  The heuristic requires
+	 * that the pointer be aligned, and that the ATAG_CORE marker is
+	 * first and present.
+	 */
+	if (__atags_pointer & 0x3)
+		__atags_pointer = 0;
+	if (__atags_pointer) {
+		struct tag *t = phys_to_virt(__atags_pointer);
+		if ((t->hdr.size != tag_size(tag_core)) &&
+		    (t->hdr.size != sizeof(struct tag_header)) &&
+		    (t->hdr.tag != ATAG_CORE))
+			__atags_pointer = 0;
+	}
+#endif
 	if (__atags_pointer)
 		tags = phys_to_virt(__atags_pointer);
 	else if (mdesc->boot_params)

Re: [RFC] arm: Defer lookup of machine_type and vet of atags to setup.c

[Nicolas Pitre]

On Mon, 10 Jan 2011, Grant Likely wrote:

> Since the debug macros no longer depend on the machine type
> information, both the machine type lookup and the atags vetting can be
> deferred to setup_arch() in setup.c which simplifies the code
> somewhat.
> 
> This patch removes both __machine_type_lookup and __vet_atags() from
> head.S.  The atags vetting is moved to setup_arch().  machine_type
> lookup is already called from setup_machine() in addition to where it
> was called from head.S.

While at it, you could also remove the code for __lookup_machine_type, 
lookup_machine_type and __error_a, replacing that with a C 
implementation living in setup.c instead.  If this code is not called 
from head.S anymore, there is no reason to keep it there any longer.


Nicolas

Re: [RFC] arm: Defer lookup of machine_type and vet of atags to setup.c

[Grant Likely]

On Mon, Jan 10, 2011 at 8:19 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Mon, 10 Jan 2011, Grant Likely wrote:
>
>> Since the debug macros no longer depend on the machine type
>> information, both the machine type lookup and the atags vetting can be
>> deferred to setup_arch() in setup.c which simplifies the code
>> somewhat.
>>
>> This patch removes both __machine_type_lookup and __vet_atags() from
>> head.S.  The atags vetting is moved to setup_arch().  machine_type
>> lookup is already called from setup_machine() in addition to where it
>> was called from head.S.
>
> While at it, you could also remove the code for __lookup_machine_type,
> lookup_machine_type and __error_a, replacing that with a C
> implementation living in setup.c instead.  If this code is not called
> from head.S anymore, there is no reason to keep it there any longer.

Certainly.  I'll draft that up as a separate patch and post it tomorrow.

g.

Re: [RFC] arm: Defer lookup of machine_type and vet of atags to setup.c

[Russell King - ARM Linux]

On Mon, Jan 10, 2011 at 07:15:53PM -0700, Grant Likely wrote:
> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
> index 6bd82d2..9c0e938 100644
> --- a/arch/arm/kernel/head.S
> +++ b/arch/arm/kernel/head.S
> @@ -87,11 +87,6 @@ ENTRY(stext)
>  	movs	r10, r5				@ invalid processor (r5=0)?
>   THUMB( it	eq )		@ force fixup-able long branch encoding
>  	beq	__error_p			@ yes, error 'p'
> -	bl	__lookup_machine_type		@ r5=machinfo
> -	movs	r8, r5				@ invalid machine (r5=0)?
> - THUMB( it	eq )		@ force fixup-able long branch encoding
> -	beq	__error_a			@ yes, error 'a'
> -	bl	__vet_atags
>  #ifdef CONFIG_SMP_ON_UP
>  	bl	__fixup_smp
>  #endif

Don't forget to update the comments as well - there's two of them.

Re: [RFC] arm: Defer lookup of machine_type and vet of atags to setup.c

[Grant Likely]

On Tue, Jan 11, 2011 at 10:40:51AM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 10, 2011 at 07:15:53PM -0700, Grant Likely wrote:
> > diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
> > index 6bd82d2..9c0e938 100644
> > --- a/arch/arm/kernel/head.S
> > +++ b/arch/arm/kernel/head.S
> > @@ -87,11 +87,6 @@ ENTRY(stext)
> >  	movs	r10, r5				@ invalid processor (r5=0)?
> >   THUMB( it	eq )		@ force fixup-able long branch encoding
> >  	beq	__error_p			@ yes, error 'p'
> > -	bl	__lookup_machine_type		@ r5=machinfo
> > -	movs	r8, r5				@ invalid machine (r5=0)?
> > - THUMB( it	eq )		@ force fixup-able long branch encoding
> > -	beq	__error_a			@ yes, error 'a'
> > -	bl	__vet_atags
> >  #ifdef CONFIG_SMP_ON_UP
> >  	bl	__fixup_smp
> >  #endif
> 
> Don't forget to update the comments as well - there's two of them.

I'm not entirely clear on what you're referring to here.  Are you
talking about the secondary cpu entry point?  I've fixed up that
comment now as well as a s/__lookup_machine_type/__lookup_processor_type/
typo right before the call to __enable_mmu.

I've also removed the machine type lookup from head-nommu.S and I'll
repost the patch later today.

Thanks for the review,
g.

Re: [RFC] arm: Defer lookup of machine_type and vet of atags to setup.c

[Russell King - ARM Linux]

On Tue, Jan 11, 2011 at 08:36:30AM -0700, Grant Likely wrote:
> On Tue, Jan 11, 2011 at 10:40:51AM +0000, Russell King - ARM Linux wrote:
> > On Mon, Jan 10, 2011 at 07:15:53PM -0700, Grant Likely wrote:
> > > diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
> > > index 6bd82d2..9c0e938 100644
> > > --- a/arch/arm/kernel/head.S
> > > +++ b/arch/arm/kernel/head.S
> > > @@ -87,11 +87,6 @@ ENTRY(stext)
> > >  	movs	r10, r5				@ invalid processor (r5=0)?
> > >   THUMB( it	eq )		@ force fixup-able long branch encoding
> > >  	beq	__error_p			@ yes, error 'p'
> > > -	bl	__lookup_machine_type		@ r5=machinfo
> > > -	movs	r8, r5				@ invalid machine (r5=0)?
> > > - THUMB( it	eq )		@ force fixup-able long branch encoding
> > > -	beq	__error_a			@ yes, error 'a'
> > > -	bl	__vet_atags
> > >  #ifdef CONFIG_SMP_ON_UP
> > >  	bl	__fixup_smp
> > >  #endif
> > 
> > Don't forget to update the comments as well - there's two of them.
> 
> I'm not entirely clear on what you're referring to here.  Are you
> talking about the secondary cpu entry point?  I've fixed up that
> comment now as well as a s/__lookup_machine_type/__lookup_processor_type/
> typo right before the call to __enable_mmu.

In the above code, __lookup_machine_type is called, which to be
consistent with __lookup_processor_type, returns its value in r5.  This
is then copied to r8, which is the only point that r8 is initialized.

The remainder of the code used to make use of r8 to load various fields
from the machine record, and so there's a couple of comments indicating
that this is the case.  These comments are left behind after your patch
and so are misleading.

Shall I assume that you've already searched the head*.S files to make
sure that r8 isn't used before you removed its initialization? ;-)

Re: [RFC] arm: Defer lookup of machine_type and vet of atags to setup.c

[Grant Likely]

On Tue, Jan 11, 2011 at 8:48 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Jan 11, 2011 at 08:36:30AM -0700, Grant Likely wrote:
>> On Tue, Jan 11, 2011 at 10:40:51AM +0000, Russell King - ARM Linux wrote:
>> > On Mon, Jan 10, 2011 at 07:15:53PM -0700, Grant Likely wrote:
>> > > diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
>> > > index 6bd82d2..9c0e938 100644
>> > > --- a/arch/arm/kernel/head.S
>> > > +++ b/arch/arm/kernel/head.S
>> > > @@ -87,11 +87,6 @@ ENTRY(stext)
>> > >   movs    r10, r5                         @ invalid processor (r5=0)?
>> > >   THUMB( it       eq )            @ force fixup-able long branch encoding
>> > >   beq     __error_p                       @ yes, error 'p'
>> > > - bl      __lookup_machine_type           @ r5=machinfo
>> > > - movs    r8, r5                          @ invalid machine (r5=0)?
>> > > - THUMB( it       eq )            @ force fixup-able long branch encoding
>> > > - beq     __error_a                       @ yes, error 'a'
>> > > - bl      __vet_atags
>> > >  #ifdef CONFIG_SMP_ON_UP
>> > >   bl      __fixup_smp
>> > >  #endif
>> >
>> > Don't forget to update the comments as well - there's two of them.
>>
>> I'm not entirely clear on what you're referring to here.  Are you
>> talking about the secondary cpu entry point?  I've fixed up that
>> comment now as well as a s/__lookup_machine_type/__lookup_processor_type/
>> typo right before the call to __enable_mmu.
>
> In the above code, __lookup_machine_type is called, which to be
> consistent with __lookup_processor_type, returns its value in r5.  This
> is then copied to r8, which is the only point that r8 is initialized.
>
> The remainder of the code used to make use of r8 to load various fields
> from the machine record, and so there's a couple of comments indicating
> that this is the case.  These comments are left behind after your patch
> and so are misleading.

Ah, right.  I'll clean those up.

> Shall I assume that you've already searched the head*.S files to make
> sure that r8 isn't used before you removed its initialization? ;-)

Yup!

Looks like I've hit a hiccup though.  When I remove the first call to
__lookup_machine_type, then it is only called after the MMU is turned
on and the kernel is no longer able to output the list of configured
machine ids when it doesn't recognize the value in r1 (tested with
qemu versatile emulation).  I'm still investigating, so I'll defer
reposting the patch until I've got this issue solved.

g.

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

Re: [RFC] arm: Defer lookup of machine_type and vet of atags to setup.c

[Russell King - ARM Linux]

On Wed, Jan 12, 2011 at 08:46:44AM -0700, Grant Likely wrote:
> Looks like I've hit a hiccup though.  When I remove the first call to
> __lookup_machine_type, then it is only called after the MMU is turned
> on and the kernel is no longer able to output the list of configured
> machine ids when it doesn't recognize the value in r1 (tested with
> qemu versatile emulation).  I'm still investigating, so I'll defer
> reposting the patch until I've got this issue solved.

It only does this when DEBUG_LL is enabled - at which point you have
printascii, printhex8, etc available (although there's no prototype
for them.)

You could use snprintf() to format a message and then use printascii()
(conditional on CONFIG_DEBUG_LL as the existing code does) as well as
printk("%s", buffer).  That means if you have a debugger you can dump
the kernel ring buffer and see the message, or see it via the serial
port/debugging channel if DEBUG_LL is enabled.

Re: [RFC] arm: Defer lookup of machine_type and vet of atags to setup.c

[Grant Likely]

On Wed, Jan 12, 2011 at 8:52 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Jan 12, 2011 at 08:46:44AM -0700, Grant Likely wrote:
>> Looks like I've hit a hiccup though.  When I remove the first call to
>> __lookup_machine_type, then it is only called after the MMU is turned
>> on and the kernel is no longer able to output the list of configured
>> machine ids when it doesn't recognize the value in r1 (tested with
>> qemu versatile emulation).  I'm still investigating, so I'll defer
>> reposting the patch until I've got this issue solved.
>
> It only does this when DEBUG_LL is enabled - at which point you have
> printascii, printhex8, etc available (although there's no prototype
> for them.)
>
> You could use snprintf() to format a message and then use printascii()
> (conditional on CONFIG_DEBUG_LL as the existing code does) as well as
> printk("%s", buffer).  That means if you have a debugger you can dump
> the kernel ring buffer and see the message, or see it via the serial
> port/debugging channel if DEBUG_LL is enabled.

Actually it looks like the real problem is that the mmu has been
turned on, but the virtual mappings for devices have not yet been
established, and so the debug macros aren't using a valid address.
I'm using printk to get output into the ring buffer in my patch to
rework lookup_machine_type() in C, and that does indeed output to the
ring buffer, but the kernel cannot spit stuff out the serial port.

It looks like I'd need to get past paging_init() in order to get
ll_debug working between turning on the mmu and paging_init(), but
paging_init() needs the mdesc pointer, and the whole point of the
error message is that the mdesc pointer is unknown!  I don't see any
code that sets up a debug mapping of the uart before paging_init time.
 I could try to implement something like that, but it is looking to be
more complicated than it is worth when the current code works just
fine.

Let me know if I've missed something, but I think I should drop the
removal of __lookup_machine_type from head.S from my patch.

g.

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

Re: [RFC] arm: Defer lookup of machine_type and vet of atags to setup.c

[Russell King - ARM Linux]

On Wed, Jan 12, 2011 at 09:24:34AM -0700, Grant Likely wrote:
> Actually it looks like the real problem is that the mmu has been
> turned on, but the virtual mappings for devices have not yet been
> established, and so the debug macros aren't using a valid address.

They should be, as they're valid for use from assembly prior to MMU
turn-on, assembly after MMU turn-on and C code.

If CONFIG_DEBUG_LL is enabled, create_page_tables() should be setting
up the necessary initial mappings for printascii() to work.

Maybe the efforts to unify that stuff ended up breaking the printascii
debugging mechanism, and we now require a new debugging mechanism to
debug the printascii debugging mechanism... :-P

I've used it on Versatile Express during the last week and it did work
right from before setup_arch() was called.

> It looks like I'd need to get past paging_init() in order to get
> ll_debug working between turning on the mmu and paging_init(), but
> paging_init() needs the mdesc pointer, and the whole point of the
> error message is that the mdesc pointer is unknown!  I don't see any
> code that sets up a debug mapping of the uart before paging_init time.

See the #ifdef CONFIG_DEBUG_LL section in create_page_tables in head.S

Re: [RFC] arm: Defer lookup of machine_type and vet of atags to setup.c

[Nicolas Pitre]

On Wed, 12 Jan 2011, Grant Likely wrote:

> Actually it looks like the real problem is that the mmu has been
> turned on, but the virtual mappings for devices have not yet been
> established, and so the debug macros aren't using a valid address.

A temporary virtual mapping should be there -- look for addruart in 
head.S.

> I'm using printk to get output into the ring buffer in my patch to
> rework lookup_machine_type() in C, and that does indeed output to the
> ring buffer, but the kernel cannot spit stuff out the serial port.
> 
> It looks like I'd need to get past paging_init() in order to get
> ll_debug working between turning on the mmu and paging_init(), but
> paging_init() needs the mdesc pointer, and the whole point of the
> error message is that the mdesc pointer is unknown!  I don't see any
> code that sets up a debug mapping of the uart before paging_init time.

See above.

>  I could try to implement something like that, but it is looking to be
> more complicated than it is worth when the current code works just
> fine.

My bet is that there is a bug with the current code that you are 
exposing.

> Let me know if I've missed something, but I think I should drop the
> removal of __lookup_machine_type from head.S from my patch.

It's not the location of that code which is a problem.  Even if you 
leave that code in place, you want to call it later and I bet that the 
display would be broken even if __lookup_machine_type doesn't move.


Nicolas

Re: [RFC] arm: Defer lookup of machine_type and vet of atags to setup.c

[Grant Likely]

On Wed, Jan 12, 2011 at 9:53 AM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Wed, 12 Jan 2011, Grant Likely wrote:
>
>> Actually it looks like the real problem is that the mmu has been
>> turned on, but the virtual mappings for devices have not yet been
>> established, and so the debug macros aren't using a valid address.
>
> A temporary virtual mapping should be there -- look for addruart in
> head.S.

Hi Russell and Nicolas,

Oops, yes all the early debug stuff works fine.  Stupid human trick on
my end, but I've sorted it out now.  Thanks for the help.  I'll have
patches to post later today.

g.

>
>> I'm using printk to get output into the ring buffer in my patch to
>> rework lookup_machine_type() in C, and that does indeed output to the
>> ring buffer, but the kernel cannot spit stuff out the serial port.
>>
>> It looks like I'd need to get past paging_init() in order to get
>> ll_debug working between turning on the mmu and paging_init(), but
>> paging_init() needs the mdesc pointer, and the whole point of the
>> error message is that the mdesc pointer is unknown!  I don't see any
>> code that sets up a debug mapping of the uart before paging_init time.
>
> See above.
>
>>  I could try to implement something like that, but it is looking to be
>> more complicated than it is worth when the current code works just
>> fine.
>
> My bet is that there is a bug with the current code that you are
> exposing.
>
>> Let me know if I've missed something, but I think I should drop the
>> removal of __lookup_machine_type from head.S from my patch.
>
> It's not the location of that code which is a problem.  Even if you
> leave that code in place, you want to call it later and I bet that the
> display would be broken even if __lookup_machine_type doesn't move.
>
>
> Nicolas
>



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

Re: [RFC] arm: Defer lookup of machine_type and vet of atags to setup.c

[Russell King - ARM Linux]

On Wed, Jan 12, 2011 at 10:16:44AM -0700, Grant Likely wrote:
> On Wed, Jan 12, 2011 at 9:53 AM, Nicolas Pitre <nico@fluxnic.net> wrote:
> > On Wed, 12 Jan 2011, Grant Likely wrote:
> >
> >> Actually it looks like the real problem is that the mmu has been
> >> turned on, but the virtual mappings for devices have not yet been
> >> established, and so the debug macros aren't using a valid address.
> >
> > A temporary virtual mapping should be there -- look for addruart in
> > head.S.
> 
> Hi Russell and Nicolas,
> 
> Oops, yes all the early debug stuff works fine.  Stupid human trick on
> my end, but I've sorted it out now.  Thanks for the help.  I'll have
> patches to post later today.

I just hacked this up, and on Versatile (real hardware) it produces
the below for an invalid r1 value - and of course works for a proper r1
value.

Uncompressing Linux... done, booting the kernel.

Error: unrecognized/unsupported machine ID (r1 = 0x00123456).

Available machine support:

ID (hex)        NAME
00000183        ARM-Versatile PB
0000025e        ARM-Versatile AB

Please check your kernel config and/or bootloader.

diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S
index bbecaac..c84b57d 100644
--- a/arch/arm/kernel/head-common.S
+++ b/arch/arm/kernel/head-common.S
@@ -25,81 +25,6 @@
  * machine ID for example).
  */
 	__HEAD
-__error_a:
-#ifdef CONFIG_DEBUG_LL
-	mov	r4, r1				@ preserve machine ID
-	adr	r0, str_a1
-	bl	printascii
-	mov	r0, r4
-	bl	printhex8
-	adr	r0, str_a2
-	bl	printascii
-	adr	r3, __lookup_machine_type_data
-	ldmia	r3, {r4, r5, r6}		@ get machine desc list
-	sub	r4, r3, r4			@ get offset between virt&phys
-	add	r5, r5, r4			@ convert virt addresses to
-	add	r6, r6, r4			@ physical address space
-1:	ldr	r0, [r5, #MACHINFO_TYPE]	@ get machine type
-	bl	printhex8
-	mov	r0, #'\t'
-	bl	printch
-	ldr     r0, [r5, #MACHINFO_NAME]	@ get machine name
-	add	r0, r0, r4
-	bl	printascii
-	mov	r0, #'\n'
-	bl	printch
-	add	r5, r5, #SIZEOF_MACHINE_DESC	@ next machine_desc
-	cmp	r5, r6
-	blo	1b
-	adr	r0, str_a3
-	bl	printascii
-	b	__error
-ENDPROC(__error_a)
-
-str_a1:	.asciz	"\nError: unrecognized/unsupported machine ID (r1 = 0x"
-str_a2:	.asciz	").\n\nAvailable machine support:\n\nID (hex)\tNAME\n"
-str_a3:	.asciz	"\nPlease check your kernel config and/or bootloader.\n"
-	.align
-#endif
-
-/*
- * Lookup machine architecture in the linker-build list of architectures.
- * Note that we can't use the absolute addresses for the __arch_info
- * lists since we aren't running with the MMU on (and therefore, we are
- * not in the correct address space).  We have to calculate the offset.
- *
- *  r1 = machine architecture number
- * Returns:
- *  r3, r4, r6 corrupted
- *  r5 = mach_info pointer in physical address space
- */
-__lookup_machine_type:
-	adr	r3, __lookup_machine_type_data
-	ldmia	r3, {r4, r5, r6}
-	sub	r3, r3, r4			@ get offset between virt&phys
-	add	r5, r5, r3			@ convert virt addresses to
-	add	r6, r6, r3			@ physical address space
-1:	ldr	r3, [r5, #MACHINFO_TYPE]	@ get machine type
-	teq	r3, r1				@ matches loader number?
-	beq	2f				@ found
-	add	r5, r5, #SIZEOF_MACHINE_DESC	@ next machine_desc
-	cmp	r5, r6
-	blo	1b
-	mov	r5, #0				@ unknown machine
-2:	mov	pc, lr
-ENDPROC(__lookup_machine_type)
-
-/*
- * Look in arch/arm/kernel/arch.[ch] for information about the
- * __arch_info structures.
- */
-	.align	2
-	.type	__lookup_machine_type_data, %object
-__lookup_machine_type_data:
-	.long	.
-	.long	__arch_info_begin
-	.long	__arch_info_end
-	.size	__lookup_machine_type_data, . - __lookup_machine_type_data
 
 /* Determine validity of the r2 atags pointer.  The heuristic requires
  * that the pointer be aligned, in the first 16k of physical RAM and
@@ -107,8 +32,6 @@ __lookup_machine_type_data:
  * of this function may be more lenient with the physical address and
  * may also be able to move the ATAGS block if necessary.
  *
- * r8  = machinfo
- *
  * Returns:
  *  r2 either valid atags pointer, or zero
  *  r5, r6 corrupted
@@ -183,17 +106,6 @@ __mmap_switched_data:
 	.size	__mmap_switched_data, . - __mmap_switched_data
 
 /*
- * This provides a C-API version of __lookup_machine_type
- */
-ENTRY(lookup_machine_type)
-	stmfd	sp!, {r4 - r6, lr}
-	mov	r1, r0
-	bl	__lookup_machine_type
-	mov	r0, r5
-	ldmfd	sp!, {r4 - r6, pc}
-ENDPROC(lookup_machine_type)
-
-/*
  * This provides a C-API version of __lookup_processor_type
  */
 ENTRY(lookup_processor_type)
diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
index 814ce1a..6b1e0ad 100644
--- a/arch/arm/kernel/head-nommu.S
+++ b/arch/arm/kernel/head-nommu.S
@@ -44,9 +44,6 @@ ENTRY(stext)
 	bl	__lookup_processor_type		@ r5=procinfo r9=cpuid
 	movs	r10, r5				@ invalid processor (r5=0)?
 	beq	__error_p				@ yes, error 'p'
-	bl	__lookup_machine_type		@ r5=machinfo
-	movs	r8, r5				@ invalid machine (r5=0)?
-	beq	__error_a			@ yes, error 'a'
 
 	adr	lr, BSYM(__after_proc_init)	@ return (PIC) address
  ARM(	add	pc, r10, #PROCINFO_INITFUNC	)
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 06aed19..084db6c 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -87,14 +87,10 @@ ENTRY(stext)
 	movs	r10, r5				@ invalid processor (r5=0)?
  THUMB( it	eq )		@ force fixup-able long branch encoding
 	beq	__error_p			@ yes, error 'p'
-	bl	__lookup_machine_type		@ r5=machinfo
-	movs	r8, r5				@ invalid machine (r5=0)?
- THUMB( it	eq )		@ force fixup-able long branch encoding
-	beq	__error_a			@ yes, error 'a'
 
 	/*
 	 * r1 = machine no, r2 = atags,
-	 * r8 = machinfo, r9 = cpuid, r10 = procinfo
+	 * r9 = cpuid, r10 = procinfo
 	 */
 	bl	__vet_atags
 #ifdef CONFIG_SMP_ON_UP
@@ -108,7 +104,7 @@ ENTRY(stext)
 	/*
 	 * The following calls CPU specific code in a position independent
 	 * manner.  See arch/arm/mm/proc-*.S for details.  r10 = base of
-	 * xxx_proc_info structure selected by __lookup_machine_type
+	 * xxx_proc_info structure selected by __lookup_processor_type
 	 * above.  On return, the CPU will be ready for the MMU to be
 	 * turned on, and r0 will hold the CPU control register value.
 	 */
@@ -127,7 +123,6 @@ ENDPROC(stext)
  * amount which are required to get the kernel running, which
  * generally means mapping in the kernel code.
  *
- * r8  = machinfo
  * r9  = cpuid
  * r10 = procinfo
  *
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 7c5499d..eb952a7 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -308,7 +308,44 @@ static void __init cacheid_init(void)
  * already provide the required functionality.
  */
 extern struct proc_info_list *lookup_processor_type(unsigned int);
-extern struct machine_desc *lookup_machine_type(unsigned int);
+
+static void __init early_print(const char *str, ...)
+{
+	extern void printascii(const char *);
+	char buf[256];
+	va_list ap;
+
+	va_start(ap, str);
+	vsnprintf(buf, sizeof(buf), str, ap);
+	va_end(ap);
+
+#ifdef CONFIG_DEBUG_LL
+	printascii(buf);
+#endif
+	printk("%s", buf);
+}
+
+static struct machine_desc * __init lookup_machine_type(unsigned int type)
+{
+	extern struct machine_desc __arch_info_begin[], __arch_info_end[];
+	struct machine_desc *p;
+
+	for (p = __arch_info_begin; p < __arch_info_end; p++)
+		if (type == p->nr)
+			return p;
+
+	early_print("\n"
+		"Error: unrecognized/unsupported machine ID (r1 = 0x%08x).\n\n"
+		"Available machine support:\n\nID (hex)\tNAME\n", type);
+
+	for (p = __arch_info_begin; p < __arch_info_end; p++)
+		early_print("%08x\t%s\n", p->nr, p->name);
+
+	early_print("\nPlease check your kernel config and/or bootloader.\n");
+
+	while (true)
+		/* can't use cpu_relax() here as it may require MMU setup */;
+}
 
 static void __init feat_v6_fixup(void)
 {

Re: [RFC] arm: Defer lookup of machine_type and vet of atags to setup.c

[Grant Likely]

On Wed, Jan 12, 2011 at 10:25 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Jan 12, 2011 at 10:16:44AM -0700, Grant Likely wrote:
>> On Wed, Jan 12, 2011 at 9:53 AM, Nicolas Pitre <nico@fluxnic.net> wrote:
>> > On Wed, 12 Jan 2011, Grant Likely wrote:
>> >
>> >> Actually it looks like the real problem is that the mmu has been
>> >> turned on, but the virtual mappings for devices have not yet been
>> >> established, and so the debug macros aren't using a valid address.
>> >
>> > A temporary virtual mapping should be there -- look for addruart in
>> > head.S.
>>
>> Hi Russell and Nicolas,
>>
>> Oops, yes all the early debug stuff works fine.  Stupid human trick on
>> my end, but I've sorted it out now.  Thanks for the help.  I'll have
>> patches to post later today.
>
> I just hacked this up, and on Versatile (real hardware) it produces
> the below for an invalid r1 value - and of course works for a proper r1
> value.

Heh, that look pretty close to identical to what I was just about to send.

Acked-by: Grant Likely <grant.likely@secretlab.ca>

I'll send the updated atags patch rebased and retested on top of this one.

g.

>
> Uncompressing Linux... done, booting the kernel.
>
> Error: unrecognized/unsupported machine ID (r1 = 0x00123456).
>
> Available machine support:
>
> ID (hex)        NAME
> 00000183        ARM-Versatile PB
> 0000025e        ARM-Versatile AB
>
> Please check your kernel config and/or bootloader.
>
> diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S
> index bbecaac..c84b57d 100644
> --- a/arch/arm/kernel/head-common.S
> +++ b/arch/arm/kernel/head-common.S
> @@ -25,81 +25,6 @@
>  * machine ID for example).
>  */
>        __HEAD
> -__error_a:
> -#ifdef CONFIG_DEBUG_LL
> -       mov     r4, r1                          @ preserve machine ID
> -       adr     r0, str_a1
> -       bl      printascii
> -       mov     r0, r4
> -       bl      printhex8
> -       adr     r0, str_a2
> -       bl      printascii
> -       adr     r3, __lookup_machine_type_data
> -       ldmia   r3, {r4, r5, r6}                @ get machine desc list
> -       sub     r4, r3, r4                      @ get offset between virt&phys
> -       add     r5, r5, r4                      @ convert virt addresses to
> -       add     r6, r6, r4                      @ physical address space
> -1:     ldr     r0, [r5, #MACHINFO_TYPE]        @ get machine type
> -       bl      printhex8
> -       mov     r0, #'\t'
> -       bl      printch
> -       ldr     r0, [r5, #MACHINFO_NAME]        @ get machine name
> -       add     r0, r0, r4
> -       bl      printascii
> -       mov     r0, #'\n'
> -       bl      printch
> -       add     r5, r5, #SIZEOF_MACHINE_DESC    @ next machine_desc
> -       cmp     r5, r6
> -       blo     1b
> -       adr     r0, str_a3
> -       bl      printascii
> -       b       __error
> -ENDPROC(__error_a)
> -
> -str_a1:        .asciz  "\nError: unrecognized/unsupported machine ID (r1 = 0x"
> -str_a2:        .asciz  ").\n\nAvailable machine support:\n\nID (hex)\tNAME\n"
> -str_a3:        .asciz  "\nPlease check your kernel config and/or bootloader.\n"
> -       .align
> -#endif
> -
> -/*
> - * Lookup machine architecture in the linker-build list of architectures.
> - * Note that we can't use the absolute addresses for the __arch_info
> - * lists since we aren't running with the MMU on (and therefore, we are
> - * not in the correct address space).  We have to calculate the offset.
> - *
> - *  r1 = machine architecture number
> - * Returns:
> - *  r3, r4, r6 corrupted
> - *  r5 = mach_info pointer in physical address space
> - */
> -__lookup_machine_type:
> -       adr     r3, __lookup_machine_type_data
> -       ldmia   r3, {r4, r5, r6}
> -       sub     r3, r3, r4                      @ get offset between virt&phys
> -       add     r5, r5, r3                      @ convert virt addresses to
> -       add     r6, r6, r3                      @ physical address space
> -1:     ldr     r3, [r5, #MACHINFO_TYPE]        @ get machine type
> -       teq     r3, r1                          @ matches loader number?
> -       beq     2f                              @ found
> -       add     r5, r5, #SIZEOF_MACHINE_DESC    @ next machine_desc
> -       cmp     r5, r6
> -       blo     1b
> -       mov     r5, #0                          @ unknown machine
> -2:     mov     pc, lr
> -ENDPROC(__lookup_machine_type)
> -
> -/*
> - * Look in arch/arm/kernel/arch.[ch] for information about the
> - * __arch_info structures.
> - */
> -       .align  2
> -       .type   __lookup_machine_type_data, %object
> -__lookup_machine_type_data:
> -       .long   .
> -       .long   __arch_info_begin
> -       .long   __arch_info_end
> -       .size   __lookup_machine_type_data, . - __lookup_machine_type_data
>
>  /* Determine validity of the r2 atags pointer.  The heuristic requires
>  * that the pointer be aligned, in the first 16k of physical RAM and
> @@ -107,8 +32,6 @@ __lookup_machine_type_data:
>  * of this function may be more lenient with the physical address and
>  * may also be able to move the ATAGS block if necessary.
>  *
> - * r8  = machinfo
> - *
>  * Returns:
>  *  r2 either valid atags pointer, or zero
>  *  r5, r6 corrupted
> @@ -183,17 +106,6 @@ __mmap_switched_data:
>        .size   __mmap_switched_data, . - __mmap_switched_data
>
>  /*
> - * This provides a C-API version of __lookup_machine_type
> - */
> -ENTRY(lookup_machine_type)
> -       stmfd   sp!, {r4 - r6, lr}
> -       mov     r1, r0
> -       bl      __lookup_machine_type
> -       mov     r0, r5
> -       ldmfd   sp!, {r4 - r6, pc}
> -ENDPROC(lookup_machine_type)
> -
> -/*
>  * This provides a C-API version of __lookup_processor_type
>  */
>  ENTRY(lookup_processor_type)
> diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
> index 814ce1a..6b1e0ad 100644
> --- a/arch/arm/kernel/head-nommu.S
> +++ b/arch/arm/kernel/head-nommu.S
> @@ -44,9 +44,6 @@ ENTRY(stext)
>        bl      __lookup_processor_type         @ r5=procinfo r9=cpuid
>        movs    r10, r5                         @ invalid processor (r5=0)?
>        beq     __error_p                               @ yes, error 'p'
> -       bl      __lookup_machine_type           @ r5=machinfo
> -       movs    r8, r5                          @ invalid machine (r5=0)?
> -       beq     __error_a                       @ yes, error 'a'
>
>        adr     lr, BSYM(__after_proc_init)     @ return (PIC) address
>  ARM(  add     pc, r10, #PROCINFO_INITFUNC     )
> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
> index 06aed19..084db6c 100644
> --- a/arch/arm/kernel/head.S
> +++ b/arch/arm/kernel/head.S
> @@ -87,14 +87,10 @@ ENTRY(stext)
>        movs    r10, r5                         @ invalid processor (r5=0)?
>  THUMB( it     eq )            @ force fixup-able long branch encoding
>        beq     __error_p                       @ yes, error 'p'
> -       bl      __lookup_machine_type           @ r5=machinfo
> -       movs    r8, r5                          @ invalid machine (r5=0)?
> - THUMB( it     eq )            @ force fixup-able long branch encoding
> -       beq     __error_a                       @ yes, error 'a'
>
>        /*
>         * r1 = machine no, r2 = atags,
> -        * r8 = machinfo, r9 = cpuid, r10 = procinfo
> +        * r9 = cpuid, r10 = procinfo
>         */
>        bl      __vet_atags
>  #ifdef CONFIG_SMP_ON_UP
> @@ -108,7 +104,7 @@ ENTRY(stext)
>        /*
>         * The following calls CPU specific code in a position independent
>         * manner.  See arch/arm/mm/proc-*.S for details.  r10 = base of
> -        * xxx_proc_info structure selected by __lookup_machine_type
> +        * xxx_proc_info structure selected by __lookup_processor_type
>         * above.  On return, the CPU will be ready for the MMU to be
>         * turned on, and r0 will hold the CPU control register value.
>         */
> @@ -127,7 +123,6 @@ ENDPROC(stext)
>  * amount which are required to get the kernel running, which
>  * generally means mapping in the kernel code.
>  *
> - * r8  = machinfo
>  * r9  = cpuid
>  * r10 = procinfo
>  *
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 7c5499d..eb952a7 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -308,7 +308,44 @@ static void __init cacheid_init(void)
>  * already provide the required functionality.
>  */
>  extern struct proc_info_list *lookup_processor_type(unsigned int);
> -extern struct machine_desc *lookup_machine_type(unsigned int);
> +
> +static void __init early_print(const char *str, ...)
> +{
> +       extern void printascii(const char *);
> +       char buf[256];
> +       va_list ap;
> +
> +       va_start(ap, str);
> +       vsnprintf(buf, sizeof(buf), str, ap);
> +       va_end(ap);
> +
> +#ifdef CONFIG_DEBUG_LL
> +       printascii(buf);
> +#endif
> +       printk("%s", buf);
> +}
> +
> +static struct machine_desc * __init lookup_machine_type(unsigned int type)
> +{
> +       extern struct machine_desc __arch_info_begin[], __arch_info_end[];
> +       struct machine_desc *p;
> +
> +       for (p = __arch_info_begin; p < __arch_info_end; p++)
> +               if (type == p->nr)
> +                       return p;
> +
> +       early_print("\n"
> +               "Error: unrecognized/unsupported machine ID (r1 = 0x%08x).\n\n"
> +               "Available machine support:\n\nID (hex)\tNAME\n", type);
> +
> +       for (p = __arch_info_begin; p < __arch_info_end; p++)
> +               early_print("%08x\t%s\n", p->nr, p->name);
> +
> +       early_print("\nPlease check your kernel config and/or bootloader.\n");
> +
> +       while (true)
> +               /* can't use cpu_relax() here as it may require MMU setup */;
> +}
>
>  static void __init feat_v6_fixup(void)
>  {
>
>



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

Re: [RFC] arm: Defer lookup of machine_type and vet of atags to setup.c

[Tony Lindgren]

* Grant Likely <grant.likely@secretlab.ca> [110112 09:48]:
> On Wed, Jan 12, 2011 at 10:25 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Wed, Jan 12, 2011 at 10:16:44AM -0700, Grant Likely wrote:
> >> On Wed, Jan 12, 2011 at 9:53 AM, Nicolas Pitre <nico@fluxnic.net> wrote:
> >> > On Wed, 12 Jan 2011, Grant Likely wrote:
> >> >
> >> >> Actually it looks like the real problem is that the mmu has been
> >> >> turned on, but the virtual mappings for devices have not yet been
> >> >> established, and so the debug macros aren't using a valid address.
> >> >
> >> > A temporary virtual mapping should be there -- look for addruart in
> >> > head.S.
> >>
> >> Hi Russell and Nicolas,
> >>
> >> Oops, yes all the early debug stuff works fine.  Stupid human trick on
> >> my end, but I've sorted it out now.  Thanks for the help.  I'll have
> >> patches to post later today.
> >
> > I just hacked this up, and on Versatile (real hardware) it produces
> > the below for an invalid r1 value - and of course works for a proper r1
> > value.
> 
> Heh, that look pretty close to identical to what I was just about to send.
> 
> Acked-by: Grant Likely <grant.likely@secretlab.ca>

Tested-by: Tony Lindgren <tony@atomide.com>