hi Nadav,

On 01/22/2017 12:01 PM, Nadav Har'El wrote:
[..]

    Signed-off-by: Sergiy Kibrik <sergiy.kib...@globallogic.com 
<mailto:sergiy.kib...@globallogic.com>>
    ---
     arch/aarch64/preboot.S | 17 +++++++++++++++++
     1 file changed, 17 insertions(+)

    diff --git a/arch/aarch64/preboot.S b/arch/aarch64/preboot.S
    index 5e265f6..b3dfcc2 100644
    --- a/arch/aarch64/preboot.S
    +++ b/arch/aarch64/preboot.S
    @@ -11,6 +11,23 @@ target = . + 0x10000
     .text
     .align 16

    +_head:
    +       /* Image header expected by Linux boot-loaders */


It would be useful if you could add here a link to some source which defines 
this format. I found https://www.kernel.org/doc/Documentation/arm64/booting.txt 
- is this a good reference? Any better one?

Sure, Documentation/arm64/booting.txt is good enough.



    +       b prestart              // jump to actual prestart


Sorry for the stupid question - do we know that this B(ranch) instruction 
together with its argument is exactly 4 bytes long?

Yes, all aarch32/aarch64 instructions have fixed 32-bit size



    +       .long 0                 // reserved
    +       .long 0x00080000        // image load offset

    +       .long 0x00000000



Again, probably a stupid question, but where does this 0x80000 come from?
in arch/aarch64/loader.ld I see a different number...
Why does this number even matter?


This is fixed TEXT_OFFSET value, unless ARM64_RANDOMIZE_TEXT_OFFSET is defined, 
see arch/arm64/Makefile in Linux tree.
Xen image loader respects this value as offset from RAM base where kernel code 
starts, loads kernel blob there and jumps to it eventually.
The thing is -- qemu image loader ignores this value from the header, and uses 
hard-coded value of 0x80000
(see hw/arm/boot.c in QEMU tree), so to be compatible with qemu I used this 
particular (almost standard) value.

By the way, why use two longs instead of one "quad"? I got confused by this 
initially.

It's just to save some space on screen, but I'll use .long if it's confusing.



    +       .quad 0                 // unused


According to the above link, this is supposed to be the "image size".  I guess 
you set it to 0 because it's not really needed? If so, why does it exist?
Or do you overwrite this later? A comment would be helpful.


We don't use this value because image size (or file size of kernel image) 
already known by the Xen/QEMU image loaders. But it must be present so magic 
number is found at expected offset.
I'll add a comment.

In x64, we really needed the image size (because we need to read it from disk 
and decompress it, and need to know how much we need to read), so we have a 
script imgedit.py to set it later by modifying the executable.


For aarch64 case scripts/imgedit.py only sets cmdline, at least ./Makefile says 
so.


    +       .quad 0                 // unused

    +       .quad 0                 // reserved
    +       .quad 0                 // reserved
    +       .quad 0                 // reserved
    +       .byte 0x41              // magic number, "ARM\x64"
    +       .byte 0x52
    +       .byte 0x4d
    +       .byte 0x64
    +       .word 0                 // reserved


Just curious, what's the difference between ".word" and ".long" you used above? 
If there's no difference, you should probably choose one to reduce confusion.

I will use .long then.

Thank you for thorough review,

--
regards,
Sergiy

--
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to