diff mbox

[PULL] boot-wrappe: boot kernel in Hyp mode

Message ID 874nuk7rmf.fsf@rustcorp.com.au
State New
Headers show

Pull-request

git://github.com/rustyrussell/boot-wrapper.git rusty-wip

Commit Message

Rusty Russell Feb. 21, 2012, 4:11 a.m. UTC 
This rolls together all the fixes into a nice single commit.  I could
push directly but prefer to go through Peter.  Sorry this took so long.

The following changes since commit 14dcbbd384abc25d475fd2021db3007a22383e46:

  boot-wrapper: Support reading kernel/initrd via semihosting (2012-02-05 13:07:07 -0500)

are available in the git repository at:
  git://github.com/rustyrussell/boot-wrapper.git rusty-wip

Rusty Russell (1):
      Boot kernel in hyp mode.

 boot.S    |   27 +++++++++++++++++++++++++--
 monitor.S |   23 ++++++++++++++---------
 2 files changed, 39 insertions(+), 11 deletions(-)


commit 998f08afd28156867c7a0020981ccd57f7f3b0ec
Author: Rusty Russell <rusty@rustcorp.com.au>
Date:   Tue Feb 21 14:32:29 2012 +1030

    Boot kernel in hyp mode.
    
    Leave the old monitor stub there for the moment, so we can boot current
    Christoff kernels.
    
    Includes fixes by Dave Martin to make it actually work :)
    
    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Comments

Dave Martin Feb. 21, 2012, 9:19 a.m. UTC | #1 
On Tue, Feb 21, 2012 at 02:41:20PM +1030, Rusty Russell wrote:
> This rolls together all the fixes into a nice single commit.  I could
> push directly but prefer to go through Peter.  Sorry this took so long.
> 
> The following changes since commit 14dcbbd384abc25d475fd2021db3007a22383e46:
> 
>   boot-wrapper: Support reading kernel/initrd via semihosting (2012-02-05 13:07:07 -0500)
> 
> are available in the git repository at:
>   git://github.com/rustyrussell/boot-wrapper.git rusty-wip
> 
> Rusty Russell (1):
>       Boot kernel in hyp mode.
> 
>  boot.S    |   27 +++++++++++++++++++++++++--
>  monitor.S |   23 ++++++++++++++---------
>  2 files changed, 39 insertions(+), 11 deletions(-)
> 
> 
> commit 998f08afd28156867c7a0020981ccd57f7f3b0ec
> Author: Rusty Russell <rusty@rustcorp.com.au>
> Date:   Tue Feb 21 14:32:29 2012 +1030
> 
>     Boot kernel in hyp mode.
>     
>     Leave the old monitor stub there for the moment, so we can boot current
>     Christoff kernels.
>     
>     Includes fixes by Dave Martin to make it actually work :)
>     
>     Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> diff --git a/boot.S b/boot.S
> index cf8bdb0..0f61c3c 100644
> --- a/boot.S
> +++ b/boot.S
> @@ -104,12 +104,35 @@ _start:
>  	orr	r0, r0, r1
>  	mcr	p15, 0, r0, c1, c1, 2
>  
> -	@ Change to NS-mode
> +	@ Leave monitor.S trap in place for the transition...
>  	mov	r0, #0xf0000000
>  	mcr	p15, 0, r0, c12, c0, 1		@ Monitor vector base address
> +
> +	@ Set up hvbar so hvc comes back here.
> +	ldr	r0, =vectors
> +	mov	r7, #0xfffffff0
> +	smc	#0				@ Set HVBAR
> +
> +	@ We can't call hvc from secure mode, so drop down first.
>  	mov	r7, #0xffffffff
>  	smc	#0				@ Change to NS-mode 
>  
> +	@ This is how we enter hyp mode, for booting the next stage.
> +	@ This is an hvc instruction (GAS doesn't like it)
> +	.long	0xe1400070

Nothing to do with this patch, but if you pass -march=armv7-a+virt to
the assembler, we should get support for the hvc instruction.

I think everone using the bootwrapper will have new enough binutils
for that.

Cheers
---Dave
Peter Maydell Feb. 21, 2012, 11:27 a.m. UTC | #2 
On 21 February 2012 04:11, Rusty Russell <rusty@rustcorp.com.au> wrote:
> This rolls together all the fixes into a nice single commit.  I could
> push directly but prefer to go through Peter.  Sorry this took so long.

Some nitpicks follow (which are so nitpicky I'm kind of undecided
whether they're worth the effort at all).


> The following changes since commit 14dcbbd384abc25d475fd2021db3007a22383e46:
>
>  boot-wrapper: Support reading kernel/initrd via semihosting (2012-02-05 13:07:07 -0500)
>
> are available in the git repository at:
>  git://github.com/rustyrussell/boot-wrapper.git rusty-wip
>
> Rusty Russell (1):
>      Boot kernel in hyp mode.
>
>  boot.S    |   27 +++++++++++++++++++++++++--
>  monitor.S |   23 ++++++++++++++---------
>  2 files changed, 39 insertions(+), 11 deletions(-)
>
>
> commit 998f08afd28156867c7a0020981ccd57f7f3b0ec
> Author: Rusty Russell <rusty@rustcorp.com.au>
> Date:   Tue Feb 21 14:32:29 2012 +1030
>
>    Boot kernel in hyp mode.
>
>    Leave the old monitor stub there for the moment, so we can boot current
>    Christoff kernels.
>
>    Includes fixes by Dave Martin to make it actually work :)
>
>    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>
> diff --git a/boot.S b/boot.S
> index cf8bdb0..0f61c3c 100644
> --- a/boot.S
> +++ b/boot.S
> @@ -104,12 +104,35 @@ _start:
>        orr     r0, r0, r1
>        mcr     p15, 0, r0, c1, c1, 2
>
> -       @ Change to NS-mode
> +       @ Leave monitor.S trap in place for the transition...
>        mov     r0, #0xf0000000
>        mcr     p15, 0, r0, c12, c0, 1          @ Monitor vector base address
> +
> +       @ Set up hvbar so hvc comes back here.
> +       ldr     r0, =vectors
> +       mov     r7, #0xfffffff0
> +       smc     #0                              @ Set HVBAR
> +
> +       @ We can't call hvc from secure mode, so drop down first.
>        mov     r7, #0xffffffff
>        smc     #0                              @ Change to NS-mode
>
> +       @ This is how we enter hyp mode, for booting the next stage.
> +       @ This is an hvc instruction (GAS doesn't like it)
> +       .long   0xe1400070
> +
> +/* Once we get rid of monitor.S, use these smc vectors too! */
> +vectors:
> +       .word 0 /* reset */
> +       .word 0 /* undef */
> +       .word 0 /* svc */
> +       .word 0 /* pabt */
> +       .word 0 /* dabt */
> +       b       into_hyp_mode /* hvc */
> +       .word 0 /* irq */
> +       .word 0 /* fix */

"fiq", I assume.

> +
> +into_hyp_mode:
>        @ Check CPU nr again
>        mrc     p15, 0, r0, c0, c0, 5           @ MPIDR (ARMv7 only)
>        and     r0, r0, #15                     @ CPU number
> @@ -485,7 +508,7 @@ sh_cmdline:
>        @ Semihosting command line will be written here
>
>  #else /* not SEMIHOSTING */
> -       .org    0x100
> +       .org    0x200
>        @ Static ATAGS for when kernel/etc are compiled into the ELF file
>  atags:
>        @ ATAG_CORE
> diff --git a/monitor.S b/monitor.S
> index 052ab1e..334186e 100644
> --- a/monitor.S
> +++ b/monitor.S
> @@ -25,7 +25,7 @@
>        @
>  1:
>        ldr     sp, =_monitor_stack
> -       push    {r11, r12}
> +       push    {r10-r12,lr}
>
>        cmp     r7, #0xffffffff
>        beq     _non_sec
> @@ -36,16 +36,19 @@
>        movnes  pc, lr
>        and     r12, r7, #0xf
>        cmp     r12, #0x0
> -       popgt   {r11, r12}
> -       movgts  pc, lr
> +       ldmgtfd sp!, {r10-r12,pc}^

I was going to say 'This is more conventionally written "ldmiagt"'
but it looks like we'd need to uncomment the '.syntax unified'
at the top of the file and fix the warnings for that. 'ldmgtia',
then.

>        @ Check the VMID is 0
> +       mrc     p15, 0, r10, c1, c1, 0          @ SCR
> +       orr     r11, r10, #1                    @ SCR.NS = 1
> +       mcr     p15, 0, r11, c1, c1, 0
> +       isb
>        mrrc    p15, 6, r12, r11, c2
> +       mcr     p15, 0, r10, c1, c1, 0          @ Restore SCR
>        lsr     r11, r11, #16
>        and     r11, r11, #0xff
>        cmp     r11, #0
> -       popne   {r11, r12}
> -       movnes  pc, lr
> +       ldmnefd sp!, {r10-r12,pc}^
>
>        @ Jump to the right function
>        and     r12, r7, #0xf
> @@ -68,16 +71,18 @@ _non_sec:
>        ldr     r11, =0x131
>        orr     r12, r12, r11
>        mcr     p15, 0, r12, c1, c1, 0
> -       pop     {r11, r12}
> -       movs    pc, lr
> +       ldmfd   sp!, {r10-r12,pc}^

"ldmia".

>
>        @
>        @ Read/Write HVBAR
>        @
>  _write_hvbar:
> +       orr     r11, r10, #1                    @ SCR.NS = 1 (r10 already = SCR)
> +       mcr     p15, 0, r11, c1, c1, 0
> +       isb
>        mcr     p15, 4, r0, c12, c0, 0
> -       pop     {r11, r12}
> -       movs    pc, lr
> +       mcr     p15, 0, r10, c1, c1, 0          @ Restore SCR
> +       ldmfd   sp!, {r10-r12,pc}^
>
>        .ltorg
>
Dave Martin Feb. 21, 2012, 12:33 p.m. UTC | #3 
On Tue, Feb 21, 2012 at 11:27:05AM +0000, Peter Maydell wrote:
> On 21 February 2012 04:11, Rusty Russell <rusty@rustcorp.com.au> wrote:
> > This rolls together all the fixes into a nice single commit. ??I could
> > push directly but prefer to go through Peter. ??Sorry this took so long.
> 
> Some nitpicks follow (which are so nitpicky I'm kind of undecided
> whether they're worth the effort at all).
> 
> 
> > The following changes since commit 14dcbbd384abc25d475fd2021db3007a22383e46:
> >
> > ??boot-wrapper: Support reading kernel/initrd via semihosting (2012-02-05 13:07:07 -0500)

What user agent did you use to reply?  Something has gone crazy with the
character encoding, but I don't know whose end it happened on...

> >
> > are available in the git repository at:
> > ??git://github.com/rustyrussell/boot-wrapper.git rusty-wip
> >
> > Rusty Russell (1):
> > ?? ?? ??Boot kernel in hyp mode.
> >
> > ??boot.S ?? ??| ?? 27 +++++++++++++++++++++++++--
> > ??monitor.S | ?? 23 ++++++++++++++---------
> > ??2 files changed, 39 insertions(+), 11 deletions(-)
> >
> >
> > commit 998f08afd28156867c7a0020981ccd57f7f3b0ec
> > Author: Rusty Russell <rusty@rustcorp.com.au>
> > Date: ?? Tue Feb 21 14:32:29 2012 +1030
> >
> > ?? ??Boot kernel in hyp mode.
> >
> > ?? ??Leave the old monitor stub there for the moment, so we can boot current
> > ?? ??Christoff kernels.
> >
> > ?? ??Includes fixes by Dave Martin to make it actually work :)
> >
> > ?? ??Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> >
> > diff --git a/boot.S b/boot.S
> > index cf8bdb0..0f61c3c 100644
> > --- a/boot.S
> > +++ b/boot.S
> > @@ -104,12 +104,35 @@ _start:
> > ?? ?? ?? ??orr ?? ?? r0, r0, r1
> > ?? ?? ?? ??mcr ?? ?? p15, 0, r0, c1, c1, 2
> >
> > - ?? ?? ?? @ Change to NS-mode
> > + ?? ?? ?? @ Leave monitor.S trap in place for the transition...
> > ?? ?? ?? ??mov ?? ?? r0, #0xf0000000
> > ?? ?? ?? ??mcr ?? ?? p15, 0, r0, c12, c0, 1 ?? ?? ?? ?? ??@ Monitor vector base address
> > +
> > + ?? ?? ?? @ Set up hvbar so hvc comes back here.
> > + ?? ?? ?? ldr ?? ?? r0, =vectors
> > + ?? ?? ?? mov ?? ?? r7, #0xfffffff0
> > + ?? ?? ?? smc ?? ?? #0 ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??@ Set HVBAR
> > +
> > + ?? ?? ?? @ We can't call hvc from secure mode, so drop down first.
> > ?? ?? ?? ??mov ?? ?? r7, #0xffffffff
> > ?? ?? ?? ??smc ?? ?? #0 ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??@ Change to NS-mode
> >
> > + ?? ?? ?? @ This is how we enter hyp mode, for booting the next stage.
> > + ?? ?? ?? @ This is an hvc instruction (GAS doesn't like it)
> > + ?? ?? ?? .long ?? 0xe1400070
> > +
> > +/* Once we get rid of monitor.S, use these smc vectors too! */
> > +vectors:
> > + ?? ?? ?? .word 0 /* reset */
> > + ?? ?? ?? .word 0 /* undef */
> > + ?? ?? ?? .word 0 /* svc */
> > + ?? ?? ?? .word 0 /* pabt */
> > + ?? ?? ?? .word 0 /* dabt */
> > + ?? ?? ?? b ?? ?? ?? into_hyp_mode /* hvc */
> > + ?? ?? ?? .word 0 /* irq */
> > + ?? ?? ?? .word 0 /* fix */
> 
> "fiq", I assume.
> 
> > +
> > +into_hyp_mode:
> > ?? ?? ?? ??@ Check CPU nr again
> > ?? ?? ?? ??mrc ?? ?? p15, 0, r0, c0, c0, 5 ?? ?? ?? ?? ?? @ MPIDR (ARMv7 only)
> > ?? ?? ?? ??and ?? ?? r0, r0, #15 ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? @ CPU number
> > @@ -485,7 +508,7 @@ sh_cmdline:
> > ?? ?? ?? ??@ Semihosting command line will be written here
> >
> > ??#else /* not SEMIHOSTING */
> > - ?? ?? ?? .org ?? ??0x100
> > + ?? ?? ?? .org ?? ??0x200
> > ?? ?? ?? ??@ Static ATAGS for when kernel/etc are compiled into the ELF file
> > ??atags:
> > ?? ?? ?? ??@ ATAG_CORE
> > diff --git a/monitor.S b/monitor.S
> > index 052ab1e..334186e 100644
> > --- a/monitor.S
> > +++ b/monitor.S
> > @@ -25,7 +25,7 @@
> > ?? ?? ?? ??@
> > ??1:
> > ?? ?? ?? ??ldr ?? ?? sp, =_monitor_stack
> > - ?? ?? ?? push ?? ??{r11, r12}
> > + ?? ?? ?? push ?? ??{r10-r12,lr}
> >
> > ?? ?? ?? ??cmp ?? ?? r7, #0xffffffff
> > ?? ?? ?? ??beq ?? ?? _non_sec
> > @@ -36,16 +36,19 @@
> > ?? ?? ?? ??movnes ??pc, lr
> > ?? ?? ?? ??and ?? ?? r12, r7, #0xf
> > ?? ?? ?? ??cmp ?? ?? r12, #0x0
> > - ?? ?? ?? popgt ?? {r11, r12}
> > - ?? ?? ?? movgts ??pc, lr
> > + ?? ?? ?? ldmgtfd sp!, {r10-r12,pc}^
> 
> I was going to say 'This is more conventionally written "ldmiagt"'
> but it looks like we'd need to uncomment the '.syntax unified'
> at the top of the file and fix the warnings for that. 'ldmgtia',
> then.

I did attempt to write it the other way around, and couldn't figure out
why it didn't work... but I didn't try .syntax unified.

> > ?? ?? ?? ??@ Check the VMID is 0
> > + ?? ?? ?? mrc ?? ?? p15, 0, r10, c1, c1, 0 ?? ?? ?? ?? ??@ SCR
> > + ?? ?? ?? orr ?? ?? r11, r10, #1 ?? ?? ?? ?? ?? ?? ?? ?? ?? ??@ SCR.NS = 1
> > + ?? ?? ?? mcr ?? ?? p15, 0, r11, c1, c1, 0
> > + ?? ?? ?? isb
> > ?? ?? ?? ??mrrc ?? ??p15, 6, r12, r11, c2
> > + ?? ?? ?? mcr ?? ?? p15, 0, r10, c1, c1, 0 ?? ?? ?? ?? ??@ Restore SCR
> > ?? ?? ?? ??lsr ?? ?? r11, r11, #16
> > ?? ?? ?? ??and ?? ?? r11, r11, #0xff
> > ?? ?? ?? ??cmp ?? ?? r11, #0
> > - ?? ?? ?? popne ?? {r11, r12}
> > - ?? ?? ?? movnes ??pc, lr
> > + ?? ?? ?? ldmnefd sp!, {r10-r12,pc}^
> >
> > ?? ?? ?? ??@ Jump to the right function
> > ?? ?? ?? ??and ?? ?? r12, r7, #0xf
> > @@ -68,16 +71,18 @@ _non_sec:
> > ?? ?? ?? ??ldr ?? ?? r11, =0x131
> > ?? ?? ?? ??orr ?? ?? r12, r12, r11
> > ?? ?? ?? ??mcr ?? ?? p15, 0, r12, c1, c1, 0
> > - ?? ?? ?? pop ?? ?? {r11, r12}
> > - ?? ?? ?? movs ?? ??pc, lr
> > + ?? ?? ?? ldmfd ?? sp!, {r10-r12,pc}^
> 
> "ldmia".

That's a valid synonym, but I'm not sure what point you're making
here.

Of course, isince the sp is always reinitialised on entry to the monitor,
the writeback to sp is redundant, so we could equally have written
ldmfd sp, {...}^ or ldmia sp, {...}^.  Without writeback, the access
is less stack-like, and I probably would tend to write "ia" instead of
"fd".

If we care about building in Thumb, we possibly should split those
instructions again, though: stm/ldm ,{...pc}^ is not available in
Thumb, but movs pc, lr is.  Combining this into one instructions was
a possibly-misguided attempt at tidiness on my part while extending the
register lists.


Perhaps we should fix that, but it could be a separate patch.

Cheers
---Dave
Peter Maydell Feb. 21, 2012, 12:55 p.m. UTC | #4 
On 21 February 2012 12:33, Dave Martin <dave.martin@linaro.org> wrote:
> On Tue, Feb 21, 2012 at 11:27:05AM +0000, Peter Maydell wrote:
>> On 21 February 2012 04:11, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> > This rolls together all the fixes into a nice single commit. ??I could
>> > push directly but prefer to go through Peter. ??Sorry this took so long.
>>
>> Some nitpicks follow (which are so nitpicky I'm kind of undecided
>> whether they're worth the effort at all).
>>
>>
>> > The following changes since commit 14dcbbd384abc25d475fd2021db3007a22383e46:
>> >
>> > ??boot-wrapper: Support reading kernel/initrd via semihosting (2012-02-05 13:07:07 -0500)
>
> What user agent did you use to reply?  Something has gone crazy with the
> character encoding, but I don't know whose end it happened on...

GMail. From my end it looks like I sent
  Content-Type: text/plain; charset=UTF-8
  Content-Transfer-Encoding: quoted-printable
and then the body has a lot of "=C2=A0", which is the q-p encoding
of 0xc2 0xa0 which is the UTF-8 of 0xA0 which is NO-BREAK SPACE.
So I think that's something which your end ought to be able to
handle, although admittedly it's a pretty dumb thing for gmail
to have done. (I think I need to fix it since it looks like it
does it consistently; I'm amazed nobody on qemu-devel has bitched
about that...)

>> > +  ldmfd sp!, {r10-r12,pc}^
>>
>> "ldmia".
>
> That's a valid synonym, but I'm not sure what point you're making
> here.

Just that I think ldmia/stmdb is more idomatic assembly than
ldmfd. I said it was a trivial nit.

> Of course, isince the sp is always reinitialised on entry to the monitor,
> the writeback to sp is redundant, so we could equally have written
> ldmfd sp, {...}^ or ldmia sp, {...}^.  Without writeback, the access
> is less stack-like, and I probably would tend to write "ia" instead of
> "fd".
>
> If we care about building in Thumb, we possibly should split those
> instructions again, though: stm/ldm ,{...pc}^ is not available in
> Thumb, but movs pc, lr is.  Combining this into one instructions was
> a possibly-misguided attempt at tidiness on my part while extending the
> register lists.
>
>
> Perhaps we should fix that, but it could be a separate patch.

Yeah. I'm not sure how important building in Thumb is. It would be
nice if we hadn't broken 'build for mps' &c, though of course monitor.c
shouldn't be built at all for M profile cores, and in any case we don't
actually have any way of testing those setups so maybe we shouldn't
worry too hard.

-- PMM
Rusty Russell Feb. 22, 2012, 2:39 a.m. UTC | #5 
On Tue, 21 Feb 2012 09:19:23 +0000, Dave Martin <dave.martin@linaro.org> wrote:
> On Tue, Feb 21, 2012 at 02:41:20PM +1030, Rusty Russell wrote:
> > This rolls together all the fixes into a nice single commit.  I could
> > push directly but prefer to go through Peter.  Sorry this took so long.
> > 
> > The following changes since commit 14dcbbd384abc25d475fd2021db3007a22383e46:
> > 
> >   boot-wrapper: Support reading kernel/initrd via semihosting (2012-02-05 13:07:07 -0500)
> > 
> > are available in the git repository at:
> >   git://github.com/rustyrussell/boot-wrapper.git rusty-wip
> > 
> > Rusty Russell (1):
> >       Boot kernel in hyp mode.
> > 
> >  boot.S    |   27 +++++++++++++++++++++++++--
> >  monitor.S |   23 ++++++++++++++---------
> >  2 files changed, 39 insertions(+), 11 deletions(-)
> > 
> > 
> > commit 998f08afd28156867c7a0020981ccd57f7f3b0ec
> > Author: Rusty Russell <rusty@rustcorp.com.au>
> > Date:   Tue Feb 21 14:32:29 2012 +1030
> > 
> >     Boot kernel in hyp mode.
> >     
> >     Leave the old monitor stub there for the moment, so we can boot current
> >     Christoff kernels.
> >     
> >     Includes fixes by Dave Martin to make it actually work :)
> >     
> >     Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> > 
> > diff --git a/boot.S b/boot.S
> > index cf8bdb0..0f61c3c 100644
> > --- a/boot.S
> > +++ b/boot.S
> > @@ -104,12 +104,35 @@ _start:
> >  	orr	r0, r0, r1
> >  	mcr	p15, 0, r0, c1, c1, 2
> >  
> > -	@ Change to NS-mode
> > +	@ Leave monitor.S trap in place for the transition...
> >  	mov	r0, #0xf0000000
> >  	mcr	p15, 0, r0, c12, c0, 1		@ Monitor vector base address
> > +
> > +	@ Set up hvbar so hvc comes back here.
> > +	ldr	r0, =vectors
> > +	mov	r7, #0xfffffff0
> > +	smc	#0				@ Set HVBAR
> > +
> > +	@ We can't call hvc from secure mode, so drop down first.
> >  	mov	r7, #0xffffffff
> >  	smc	#0				@ Change to NS-mode 
> >  
> > +	@ This is how we enter hyp mode, for booting the next stage.
> > +	@ This is an hvc instruction (GAS doesn't like it)
> > +	.long	0xe1400070
> 
> Nothing to do with this patch, but if you pass -march=armv7-a+virt to
> the assembler, we should get support for the hvc instruction.
> 
> I think everone using the bootwrapper will have new enough binutils
> for that.

Hmm, it's not in current Ubuntu:

cc1: error: bad value (armv7-a+virt) for -march switchcc1: error: bad value (armv7-a+virt) for -march switch

Trying to hand it straight to gas gives:
arm-linux-gnueabi-gcc -DSMP -Wa,-march=armv7-a+virt -marm -DVEXPRESS -DSEMIHOSTING=1 -c -o bootsemi.o boot.S
boot.S: Assembler messages:
boot.S:121: Error: bad instruction `hyp'

$ arm-linux-gnueabi-as --version
GNU assembler (GNU Binutils for Ubuntu) 2.21.53.20110810
Copyright 2011 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or later.
This program has absolutely no warranty.
This assembler was configured for a target of `arm-linux-gnueabi'.
$ arm-linux-gnueabi-gcc --version
arm-linux-gnueabi-gcc (Ubuntu/Linaro 4.6.1-9ubuntu3) 4.6.1
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

So I think this is needed for the moment?

Thanks,
Rusty.
Dave Martin Feb. 22, 2012, 9:14 a.m. UTC | #6 
On Wed, Feb 22, 2012 at 2:39 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> On Tue, 21 Feb 2012 09:19:23 +0000, Dave Martin <dave.martin@linaro.org> wrote:
>> On Tue, Feb 21, 2012 at 02:41:20PM +1030, Rusty Russell wrote:
>> > This rolls together all the fixes into a nice single commit.  I could
>> > push directly but prefer to go through Peter.  Sorry this took so long.
>> >
>> > The following changes since commit 14dcbbd384abc25d475fd2021db3007a22383e46:
>> >
>> >   boot-wrapper: Support reading kernel/initrd via semihosting (2012-02-05 13:07:07 -0500)
>> >
>> > are available in the git repository at:
>> >   git://github.com/rustyrussell/boot-wrapper.git rusty-wip
>> >
>> > Rusty Russell (1):
>> >       Boot kernel in hyp mode.
>> >
>> >  boot.S    |   27 +++++++++++++++++++++++++--
>> >  monitor.S |   23 ++++++++++++++---------
>> >  2 files changed, 39 insertions(+), 11 deletions(-)
>> >
>> >
>> > commit 998f08afd28156867c7a0020981ccd57f7f3b0ec
>> > Author: Rusty Russell <rusty@rustcorp.com.au>
>> > Date:   Tue Feb 21 14:32:29 2012 +1030
>> >
>> >     Boot kernel in hyp mode.
>> >
>> >     Leave the old monitor stub there for the moment, so we can boot current
>> >     Christoff kernels.
>> >
>> >     Includes fixes by Dave Martin to make it actually work :)
>> >
>> >     Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>> >
>> > diff --git a/boot.S b/boot.S
>> > index cf8bdb0..0f61c3c 100644
>> > --- a/boot.S
>> > +++ b/boot.S
>> > @@ -104,12 +104,35 @@ _start:
>> >     orr     r0, r0, r1
>> >     mcr     p15, 0, r0, c1, c1, 2
>> >
>> > -   @ Change to NS-mode
>> > +   @ Leave monitor.S trap in place for the transition...
>> >     mov     r0, #0xf0000000
>> >     mcr     p15, 0, r0, c12, c0, 1          @ Monitor vector base address
>> > +
>> > +   @ Set up hvbar so hvc comes back here.
>> > +   ldr     r0, =vectors
>> > +   mov     r7, #0xfffffff0
>> > +   smc     #0                              @ Set HVBAR
>> > +
>> > +   @ We can't call hvc from secure mode, so drop down first.
>> >     mov     r7, #0xffffffff
>> >     smc     #0                              @ Change to NS-mode
>> >
>> > +   @ This is how we enter hyp mode, for booting the next stage.
>> > +   @ This is an hvc instruction (GAS doesn't like it)
>> > +   .long   0xe1400070
>>
>> Nothing to do with this patch, but if you pass -march=armv7-a+virt to
>> the assembler, we should get support for the hvc instruction.
>>
>> I think everone using the bootwrapper will have new enough binutils
>> for that.
>
> Hmm, it's not in current Ubuntu:
>
> cc1: error: bad value (armv7-a+virt) for -march switchcc1: error: bad value (armv7-a+virt) for -march switch
>
> Trying to hand it straight to gas gives:
> arm-linux-gnueabi-gcc -DSMP -Wa,-march=armv7-a+virt -marm -DVEXPRESS -DSEMIHOSTING=1 -c -o bootsemi.o boot.S
> boot.S: Assembler messages:
> boot.S:121: Error: bad instruction `hyp'
>
> $ arm-linux-gnueabi-as --version
> GNU assembler (GNU Binutils for Ubuntu) 2.21.53.20110810
> Copyright 2011 Free Software Foundation, Inc.
> This program is free software; you may redistribute it under the terms of
> the GNU General Public License version 3 or later.
> This program has absolutely no warranty.
> This assembler was configured for a target of `arm-linux-gnueabi'.
> $ arm-linux-gnueabi-gcc --version
> arm-linux-gnueabi-gcc (Ubuntu/Linaro 4.6.1-9ubuntu3) 4.6.1
> Copyright (C) 2011 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> So I think this is needed for the moment?

I should have clarified -- the compiler doesn't understand that
option.  (For 'hyp', did you mean 'hvc'?)

Cheers
---Dave

echo 'hvc 0' >test.s
arm-linux-gnueabi-gcc -Wa,-march=armv7-a+virt -c test.s
arm-linux-gnueabi-objdump -d test.o

test.o:     file format elf32-littlearm


Disassembly of section .text:

00000000 <.text>:
   0:   e1400070        hvc     0
diff mbox

Patch

diff --git a/boot.S b/boot.S
index cf8bdb0..0f61c3c 100644
--- a/boot.S
+++ b/boot.S
@@ -104,12 +104,35 @@  _start:
 	orr	r0, r0, r1
 	mcr	p15, 0, r0, c1, c1, 2
 
-	@ Change to NS-mode
+	@ Leave monitor.S trap in place for the transition...
 	mov	r0, #0xf0000000
 	mcr	p15, 0, r0, c12, c0, 1		@ Monitor vector base address
+
+	@ Set up hvbar so hvc comes back here.
+	ldr	r0, =vectors
+	mov	r7, #0xfffffff0
+	smc	#0				@ Set HVBAR
+
+	@ We can't call hvc from secure mode, so drop down first.
 	mov	r7, #0xffffffff
 	smc	#0				@ Change to NS-mode 
 
+	@ This is how we enter hyp mode, for booting the next stage.
+	@ This is an hvc instruction (GAS doesn't like it)
+	.long	0xe1400070
+
+/* Once we get rid of monitor.S, use these smc vectors too! */
+vectors:
+	.word 0	/* reset */
+	.word 0	/* undef */
+	.word 0 /* svc */
+	.word 0 /* pabt */
+	.word 0 /* dabt */
+	b	into_hyp_mode /* hvc */
+	.word 0 /* irq */
+	.word 0 /* fix */
+
+into_hyp_mode:
 	@ Check CPU nr again
 	mrc	p15, 0, r0, c0, c0, 5		@ MPIDR (ARMv7 only)
 	and	r0, r0, #15			@ CPU number
@@ -485,7 +508,7 @@  sh_cmdline:
 	@ Semihosting command line will be written here
 
 #else /* not SEMIHOSTING */
-	.org	0x100
+	.org	0x200
 	@ Static ATAGS for when kernel/etc are compiled into the ELF file
 atags:
 	@ ATAG_CORE
diff --git a/monitor.S b/monitor.S
index 052ab1e..334186e 100644
--- a/monitor.S
+++ b/monitor.S
@@ -25,7 +25,7 @@ 
 	@
 1:
 	ldr	sp, =_monitor_stack
-	push	{r11, r12}
+	push	{r10-r12,lr}
 
 	cmp	r7, #0xffffffff
 	beq	_non_sec
@@ -36,16 +36,19 @@ 
 	movnes	pc, lr
 	and	r12, r7, #0xf
 	cmp	r12, #0x0
-	popgt	{r11, r12}
-	movgts	pc, lr
+	ldmgtfd	sp!, {r10-r12,pc}^
 
 	@ Check the VMID is 0
+	mrc	p15, 0, r10, c1, c1, 0		@ SCR
+	orr	r11, r10, #1			@ SCR.NS = 1
+	mcr	p15, 0, r11, c1, c1, 0
+	isb
 	mrrc	p15, 6, r12, r11, c2
+	mcr	p15, 0, r10, c1, c1, 0		@ Restore SCR
 	lsr	r11, r11, #16
 	and	r11, r11, #0xff
 	cmp	r11, #0
-	popne	{r11, r12}
-	movnes	pc, lr
+	ldmnefd	sp!, {r10-r12,pc}^
 
 	@ Jump to the right function
 	and	r12, r7, #0xf
@@ -68,16 +71,18 @@  _non_sec:
 	ldr	r11, =0x131
 	orr	r12, r12, r11
 	mcr	p15, 0, r12, c1, c1, 0
-	pop	{r11, r12}
-	movs	pc, lr
+	ldmfd	sp!, {r10-r12,pc}^
 
 	@
 	@ Read/Write HVBAR
 	@
 _write_hvbar:
+	orr	r11, r10, #1			@ SCR.NS = 1 (r10 already = SCR)
+	mcr	p15, 0, r11, c1, c1, 0
+	isb
 	mcr	p15, 4, r0, c12, c0, 0
-	pop	{r11, r12}
-	movs	pc, lr
+	mcr	p15, 0, r10, c1, c1, 0		@ Restore SCR
+	ldmfd	sp!, {r10-r12,pc}^
 
 	.ltorg