On Thu, Jun 9, 2016 at 10:38 AM, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 25 May 2016 at 19:49, Alistair Francis <alistair.fran...@xilinx.com> wrote:
>> If the caller didn't specify an architecture for the ELF machine
>> the load_elf() function will auto detect it based on the ELF file.
>>
>> Signed-off-by: Alistair Francis <alistair.fran...@xilinx.com>
>> ---
>> V7:
>> - Fix typo
>>
>> hw/core/loader.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/hw/core/loader.c b/hw/core/loader.c
>> index 53e0e41..a8a372d 100644
>> --- a/hw/core/loader.c
>> +++ b/hw/core/loader.c
>> @@ -419,6 +419,7 @@ int load_elf(const char *filename, uint64_t
>> (*translate_fn)(void *, uint64_t),
>> {
>> int fd, data_order, target_data_order, must_swab, ret = ELF_LOAD_FAILED;
>> uint8_t e_ident[EI_NIDENT];
>> + uint16_t e_machine;
>>
>> fd = open(filename, O_RDONLY | O_BINARY);
>> if (fd < 0) {
>> @@ -451,6 +452,15 @@ int load_elf(const char *filename, uint64_t
>> (*translate_fn)(void *, uint64_t),
>> goto fail;
>> }
>>
>> + if (elf_machine < 1) {
>> + /* The caller didn't specify an ARCH, we can figure it out */
>> + lseek(fd, 0x12, SEEK_SET);
>> + if (read(fd, &e_machine, sizeof(e_machine)) != sizeof(e_machine)) {
>> + goto fail;
>> + }
>> + elf_machine = e_machine;
>> + }
>
> Isn't there an endianness problem here, given that e_machine is a 16
> bit field? In load_elf32()/load_elf64() we will byteswap the e_machine
> field we read off the disk if must_swab is true, which will mean it won't
> match the value we've read here and not byteswapped.
>
> I think you're better off pushing the "allow architecture to be
> unspecified" support down into load_elf32()/load_elf64(), where
> it can just become
>
> if (elf_machine < 1) {
> elf_machine = ehdr.e_machine;
> }
>
> once the load_elf code has read and byteswapped the header for you.
Good point, I didn't realise it was the same code for both. I have
moved it into load_elf64/load_elf32.
Thanks,
Alistair
>
> thanks
> -- PMM
>
