Re: [PATCH rc3 23/30] hw/core/loader: Let load_elf populate the processor-specific flags

[Philippe Mathieu-Daudé]

On 1/26/20 11:55 PM, Aleksandar Markovic wrote:
> From: Philippe Mathieu-Daudé <address@hidden>
>
> Some platforms (like AVR) need to determine cpu type by reading
> the ELF flags (field e_flags oin ELF header).
>
> This patch enables discovery of the content of that flag while
> using following functions:
>
>    - load_elf()
>    - load_elf_as()
>    - load_elf_ram()
>    - load_elf_ram_sym()
>
> The added argument of these functions is of type uint32_t*. It is
> allowed to pass NULL as that argument, and in such case no lookup
> to the field e_flags will happen, and of course, no information
> will be returned to the caller.
>
> CC: Richard Henderson <address@hidden>
> CC: Peter Maydell <address@hidden>
> CC: Edgar E. Iglesias <address@hidden>
> CC: Michael Walle <address@hidden>
> CC: Thomas Huth <address@hidden>
> CC: Laurent Vivier <address@hidden>
> CC: Philippe Mathieu-Daudé <address@hidden>
> CC: Aleksandar Rikalo <address@hidden>
> CC: Aurelien Jarno <address@hidden>
> CC: Jia Liu <address@hidden>
> CC: David Gibson <address@hidden>
> CC: Mark Cave-Ayland <address@hidden>
> CC: BALATON Zoltan <address@hidden>
> CC: Christian Borntraeger <address@hidden>
> CC: Thomas Huth <address@hidden>
> CC: Artyom Tarasenko <address@hidden>
> CC: Fabien Chouteau <address@hidden>
> CC: KONRAD Frederic <address@hidden>
> CC: Max Filippov <address@hidden>
>
> Signed-off-by: Michael Rolnik <address@hidden>
> Reviewed-by: Aleksandar Markovic <address@hidden>
> [PMD: Extracted from bigger patch,
>        Replaced 'uint32_t *pe_flags' by 'int proc_flags']
> [AM: Replaced 'int proc_flags' with 'uint32_t *pflags',
>       replaced one instance of 'elf_sword' to 'elf_word',

IIRC I used 'int' because load_elf() uses 'int' for all the other ELF 
flags, regardless their type (except the addresses).

>       extended functionality to load_elf()]
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> Signed-off-by: Aleksandar Markovic <address@hidden>
[...]
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 48a96cd..a9eeea3 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -101,6 +101,7 @@ const char *load_elf_strerror(int error);
>    * @pentry: Populated with program entry point. Ignored if NULL.
>    * @lowaddr: Populated with lowest loaded address. Ignored if NULL.
>    * @highaddr: Populated with highest loaded address. Ignored if NULL.
> + * @pflags: Populated with ELF processor-specific flags. Ignore if NULL.
>    * @bigendian: Expected ELF endianness. 0 for LE otherwise BE
>    * @elf_machine: Expected ELF machine type
>    * @clear_lsb: Set to mask off LSB of addresses (Some architectures use
> @@ -131,8 +132,9 @@ int load_elf_ram_sym(const char *filename,
>                        uint64_t (*elf_note_fn)(void *, void *, bool),
>                        uint64_t (*translate_fn)(void *, uint64_t),
>                        void *translate_opaque, uint64_t *pentry,
> -                     uint64_t *lowaddr, uint64_t *highaddr, int big_endian,
> -                     int elf_machine, int clear_lsb, int data_swab,
> +                     uint64_t *lowaddr, uint64_t *highaddr, uint32_t *pflags,
> +                     int big_endian, int elf_machine,
> +                     int clear_lsb, int data_swab,
>                        AddressSpace *as, bool load_rom, symbol_fn_t sym_cb);
>   
>   /** load_elf_ram:
> @@ -143,9 +145,9 @@ int load_elf_ram(const char *filename,
>                    uint64_t (*elf_note_fn)(void *, void *, bool),
>                    uint64_t (*translate_fn)(void *, uint64_t),
>                    void *translate_opaque, uint64_t *pentry, uint64_t *lowaddr,
> -                 uint64_t *highaddr, int big_endian, int elf_machine,
> -                 int clear_lsb, int data_swab, AddressSpace *as,
> -                 bool load_rom);
> +                 uint64_t *highaddr, uint32_t *pflags, int big_endian,
> +                 int elf_machine, int clear_lsb, int data_swab,
> +                 AddressSpace *as, bool load_rom);
>   
>   /** load_elf_as:
>    * Same as load_elf_ram(), but always loads the elf as ROM
> @@ -154,8 +156,9 @@ int load_elf_as(const char *filename,
>                   uint64_t (*elf_note_fn)(void *, void *, bool),
>                   uint64_t (*translate_fn)(void *, uint64_t),
>                   void *translate_opaque, uint64_t *pentry, uint64_t *lowaddr,
> -                uint64_t *highaddr, int big_endian, int elf_machine,
> -                int clear_lsb, int data_swab, AddressSpace *as);
> +                uint64_t *highaddr, uint32_t *pflags, int big_endian,
> +                int elf_machine, int clear_lsb, int data_swab,
> +                AddressSpace *as);
>   
>   /** load_elf:
>    * Same as load_elf_as(), but doesn't allow the caller to specify an
> @@ -165,8 +168,8 @@ int load_elf(const char *filename,
>                uint64_t (*elf_note_fn)(void *, void *, bool),
>                uint64_t (*translate_fn)(void *, uint64_t),
>                void *translate_opaque, uint64_t *pentry, uint64_t *lowaddr,
> -             uint64_t *highaddr, int big_endian, int elf_machine,
> -             int clear_lsb, int data_swab);
> +             uint64_t *highaddr, uint32_t *pflags, int big_endian,
> +             int elf_machine, int clear_lsb, int data_swab);
>   
>   /** load_elf_hdr:
>    * @filename: Path of ELF file