Re: [Qemu-devel] [PATCH v3 2/2] docs: Add a generic loader explanation document

[Peter Maydell]

On 19 February 2016 at 20:40, Alistair Francis
<address@hidden> wrote:
> Signed-off-by: Alistair Francis <address@hidden>
> ---
>
>  docs/generic-loader.txt | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 docs/generic-loader.txt
>
> diff --git a/docs/generic-loader.txt b/docs/generic-loader.txt
> new file mode 100644
> index 0000000..69e262d
> --- /dev/null
> +++ b/docs/generic-loader.txt
> @@ -0,0 +1,21 @@
> +Copyright (c) 2016 Xilinx Inc.
> +
> +This work is licensed under the terms of the GNU GPL, version 2 or later.  
> See
> +the COPYING file in the top-level directory.
> +
> +
> +This loader allows the user to load multiple images or values into QEMU at 
> startup.

This is a bit confusing, because it's not entirely clear what
"this" is referring to. Something with a heading and then a
paragraph starting "The 'loader' device allows ..." might be
better.

> +
> +Loading Memory Values
> +---------------------------
> +Memory values can be loaded like this:
> +    -device loader,addr=0xfd1a0104,data=0x8000000e,data-len=4

What values are permitted for data-len ? What endianness is
the data stored as? What address space is the data written to?
Is 'cpu=N' valid here as it is for the image loader syntax below?

> +
> +Loading Images
> +---------------------------
> +Images can be loaded like this:
> +    -device loader,file=./images/boot.elf,cpu=0
> +
> +The limiation for arch is based off settting the ELF_ARCH macro.

I don't understand this sentence (also it has two typos in it :-))


> +
> +At the moment only the ARM arhitectures are supported

"architectures". Why is there an architecture limitation? Missing
full stop.

Straightforward documentation of all possible options, followed
by examples, is probably a better way to structure this than
trying to describe it only by example.

thanks
-- PMM