Re: [Qemu-devel] [PATCH v1 16/17] loader: Add data swap option to load-elf

[Peter Crosthwaite]

On Tue, Jan 19, 2016 at 9:53 AM, Peter Maydell <address@hidden> wrote:
> On 18 January 2016 at 07:12, Peter Crosthwaite
> <address@hidden> wrote:
>> Some CPUs are of an opposite data-endianness to other components in the
>> system. Sometimes elfs have the data sections layed out with this CPU
>> data-endianess accounting for when loaded via the CPU, byte swaps
>> (relative to other system components) will occur.
>>
>> The leading example, is ARM's BE32 mode, which is is basically LE with
>> address manipulation on half-word and byte accesses to access the
>> hw/byte reversed address. This means that word data is invariant
>> accross LE and BE32. This also means that instructions are still LE.
>> The expectation is that the elf will be loaded via the CPU in this
>> endianness scheme, which means the data in the elf is reversed at
>> compile time.
>>
>> As QEMU loads via the system memory directly, rather than the CPU, we
>> need a mechanism to reverse elf data endianness to implement this
>> possibility.
>>
>> Signed-off-by: Peter Crosthwaite <address@hidden>
>
>> diff --git a/include/hw/loader.h b/include/hw/loader.h
>> index 33067f8..e542575 100644
>> --- a/include/hw/loader.h
>> +++ b/include/hw/loader.h
>> @@ -35,7 +35,7 @@ const char *load_elf_strerror(int error);
>>  int load_elf(const char *filename, 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 clear_lsb, int data_swab);
>>  void load_elf_hdr(const char *filename, void *hdr, bool *is64, Error 
>> **errp);
>>  int load_aout(const char *filename, hwaddr addr, int max_sz,
>>                int bswap_needed, hwaddr target_page_size);
>
> Can we have a doc comment so we have something that defines what
> values data_swab accepts? (it's not just a bool).
>

This is difficult to capture without writing to whole documentation
for load_elf. So here goes:

/** load_elf:
 * @filename: Path of ELF file
 * @translate_fn: optional function to translate load addresses
 * @translate_opaque: opaque data passed to @translate_fn
 * @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.
 * @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 arch's use this
               for non-address data)
 * @data_swab: Set to order of byte swapping for data. 0 for no swap, 1
 *             for swapping bytes within halfwords, 2 for bytes within
 *             words and 3 for within doublewords.
 *
 * Load an ELF file's contents to the emulated systems address space.
 * Clients may optionally specify a callback to perform address
 * translations. @pentry, @lowaddr and @highaddr are optional pointers
 * which will be populated with various load information. @bigendian and
 * @elf_machine give the expected endianness and machine for the ELF the
 * load will fail it the target ELF does not match. Some architectures
 * have some arch-specific behaviours that come into effect when their
 * particular values for @elf_machine are set.
 */

Regards,
Peter

> Otherwise
> Reviewed-by: Peter Maydell <address@hidden>
>
> thanks
> -- PMM