On 01/22/2014 06:27 PM, Alexey Kardashevskiy wrote:
> On 01/22/2014 04:20 PM, Alexey Kardashevskiy wrote:
>> The existing load_elf() just returns -1 if it fails to load ELF. However
>> it could be smarter than this and tell more about the failure such as
>> wrong endianness or incompatible platform.
>>
>> This adds additional return codes for wrong architecture, wrong
>> endianness and if the image is not ELF at all.
>>
>> This fixes handling of what load_elf() returns for s390x and moxie
>> architectures, other callers just check the return value for <0 and
>> this remains unchanged.
>>
>> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
>> ---
>> hw/core/loader.c | 12 ++++++------
>> hw/s390x/ipl.c | 4 ++--
>> include/hw/elf_ops.h | 19 ++++++++++++++-----
>> include/hw/loader.h | 5 +++++
>> 4 files changed, 27 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/core/loader.c b/hw/core/loader.c
>> index 0634bee..f510260 100644
>> --- a/hw/core/loader.c
>> +++ b/hw/core/loader.c
>> @@ -289,7 +289,7 @@ 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 fd, data_order, target_data_order, must_swab, ret;
>> + int fd, data_order, target_data_order, must_swab, ret = ELF_LOAD_FAILED;
>> uint8_t e_ident[EI_NIDENT];
>>
>> fd = open(filename, O_RDONLY | O_BINARY);
>> @@ -302,8 +302,10 @@ int load_elf(const char *filename, uint64_t
>> (*translate_fn)(void *, uint64_t),
>> if (e_ident[0] != ELFMAG0 ||
>> e_ident[1] != ELFMAG1 ||
>> e_ident[2] != ELFMAG2 ||
>> - e_ident[3] != ELFMAG3)
>> + e_ident[3] != ELFMAG3) {
>> + ret = ELF_LOAD_NOT_ELF;
>> goto fail;
>> + }
>> #ifdef HOST_WORDS_BIGENDIAN
>> data_order = ELFDATA2MSB;
>> #else
>> @@ -317,6 +319,7 @@ int load_elf(const char *filename, uint64_t
>> (*translate_fn)(void *, uint64_t),
>> }
>>
>> if (target_data_order != e_ident[EI_DATA]) {
>> + ret = ELF_LOAD_WRONG_ENDIAN;
>> goto fail;
>> }
>>
>> @@ -329,12 +332,9 @@ int load_elf(const char *filename, uint64_t
>> (*translate_fn)(void *, uint64_t),
>> pentry, lowaddr, highaddr, elf_machine, clear_lsb);
>> }
>>
>> - close(fd);
>> - return ret;
>> -
>> fail:
>> close(fd);
>> - return -1;
>> + return ret;
>> }
>>
>> static void bswap_uboot_header(uboot_image_header_t *hdr)
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index 1a6397b..cff77ad 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -97,10 +97,10 @@ static int s390_ipl_init(SysBusDevice *dev)
>> } else {
>> kernel_size = load_elf(ipl->kernel, NULL, NULL, NULL, NULL,
>> NULL, 1, ELF_MACHINE, 0);
>> - if (kernel_size == -1) {
>> + if (kernel_size < 0) {
>> kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
>> }
>> - if (kernel_size == -1) {
>> + if (kernel_size < 0) {
>> fprintf(stderr, "could not load kernel '%s'\n", ipl->kernel);
>> return -1;
>> }
>> diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
>> index acc701e..b7e7b36 100644
>> --- a/include/hw/elf_ops.h
>> +++ b/include/hw/elf_ops.h
>> @@ -201,6 +201,7 @@ static int glue(load_elf, SZ)(const char *name, int fd,
>> uint64_t addr, low = (uint64_t)-1, high = 0;
>> uint8_t *data = NULL;
>> char label[128];
>> + int ret = ELF_LOAD_FAILED;
>>
>> if (read(fd, &ehdr, sizeof(ehdr)) != sizeof(ehdr))
>> goto fail;
>> @@ -210,23 +211,31 @@ static int glue(load_elf, SZ)(const char *name, int fd,
>>
>> switch (elf_machine) {
>> case EM_PPC64:
>> - if (EM_PPC64 != ehdr.e_machine)
>> + if (EM_PPC64 != ehdr.e_machine) {
>> if (EM_PPC != ehdr.e_machine)
>
> A stupid bug here, "{" should go one line down :)
>
> I'll repost if someone tells me that the whole idea makes any sense. Thanks.
Ping?
--
Alexey
