[Qemu-devel] [PATCH for 2.10 0/2] Move endianness error reporting to the MIPS boards

[Qemu-devel] [PATCH for 2.10 0/2] Move endianness error reporting to the MIPS boards

[Aurelien Jarno]

This patchset improves the MIPS board error reporting when loading a 
kernel with the wrong endianness, using load_elf_strerror.

That way the check added to loader.c can be removed, as it harms the
pseries platform. Indeed it can change its endianness dynamically at
runtime and thus can load both little and big endian kernels.

Alexey Kardashevskiy (1):
  Revert "elf-loader: warn about invalid endianness"

Aurelien Jarno (1):
  hw/mips: load_elf_strerror to report kernel loading failure

 hw/core/loader.c        |  1 -
 hw/mips/mips_fulong2e.c | 15 +++++++++------
 hw/mips/mips_malta.c    | 14 ++++++++------
 hw/mips/mips_mipssim.c  |  5 +++--
 hw/mips/mips_r4k.c      |  6 ++++--
 5 files changed, 24 insertions(+), 17 deletions(-)

-- 
2.11.0

[Qemu-devel] [PATCH for 2.10 1/2] hw/mips: load_elf_strerror to report kernel loading failure

[Aurelien Jarno]

Emulated MIPS boards bail out with a simple "could not load kernel" when
a kernel could not be load, without specifying the underlying reason.
Fix that by calling load_elf_strerror.

At the same time use error_report to report the error instead of
fprintf.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 hw/mips/mips_fulong2e.c | 15 +++++++++------
 hw/mips/mips_malta.c    | 14 ++++++++------
 hw/mips/mips_mipssim.c  |  5 +++--
 hw/mips/mips_r4k.c      |  6 ++++--
 4 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 3f3cb32651..3532399a13 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -110,16 +110,19 @@ static int64_t load_kernel (CPUMIPSState *env)
 {
     int64_t kernel_entry, kernel_low, kernel_high;
     int index = 0;
-    long initrd_size;
+    long kernel_size, initrd_size;
     ram_addr_t initrd_offset;
     uint32_t *prom_buf;
     long prom_size;
 
-    if (load_elf(loaderparams.kernel_filename, cpu_mips_kseg0_to_phys, NULL,
-                 (uint64_t *)&kernel_entry, (uint64_t *)&kernel_low,
-                 (uint64_t *)&kernel_high, 0, EM_MIPS, 1, 0) < 0) {
-        fprintf(stderr, "qemu: could not load kernel '%s'\n",
-                loaderparams.kernel_filename);
+    kernel_size = load_elf(loaderparams.kernel_filename, cpu_mips_kseg0_to_phys,
+                           NULL, (uint64_t *)&kernel_entry,
+                           (uint64_t *)&kernel_low, (uint64_t *)&kernel_high,
+                           0, EM_MIPS, 1, 0);
+    if (kernel_size < 0) {
+        error_report("qemu: could not load kernel '%s': %s",
+                     loaderparams.kernel_filename,
+                     load_elf_strerror(kernel_size));
         exit(1);
     }
 
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 3487d16f61..8ecd544baa 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -794,7 +794,7 @@ static void GCC_FMT_ATTR(3, 4) prom_set(uint32_t* prom_buf, int index,
 static int64_t load_kernel (void)
 {
     int64_t kernel_entry, kernel_high;
-    long initrd_size;
+    long kernel_size, initrd_size;
     ram_addr_t initrd_offset;
     int big_endian;
     uint32_t *prom_buf;
@@ -808,11 +808,13 @@ static int64_t load_kernel (void)
     big_endian = 0;
 #endif
 
-    if (load_elf(loaderparams.kernel_filename, cpu_mips_kseg0_to_phys, NULL,
-                 (uint64_t *)&kernel_entry, NULL, (uint64_t *)&kernel_high,
-                 big_endian, EM_MIPS, 1, 0) < 0) {
-        fprintf(stderr, "qemu: could not load kernel '%s'\n",
-                loaderparams.kernel_filename);
+    kernel_size = load_elf(loaderparams.kernel_filename, cpu_mips_kseg0_to_phys,
+                           NULL, (uint64_t *)&kernel_entry, NULL,
+                           (uint64_t *)&kernel_high, big_endian, EM_MIPS, 1, 0);
+    if (kernel_size < 0) {
+        error_report("qemu: could not load kernel '%s': %s",
+                     loaderparams.kernel_filename,
+                     load_elf_strerror(kernel_size));
         exit(1);
     }
 
diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
index 6990b1b0dd..07fc4c2300 100644
--- a/hw/mips/mips_mipssim.c
+++ b/hw/mips/mips_mipssim.c
@@ -78,8 +78,9 @@ static int64_t load_kernel(void)
         if ((entry & ~0x7fffffffULL) == 0x80000000)
             entry = (int32_t)entry;
     } else {
-        fprintf(stderr, "qemu: could not load kernel '%s'\n",
-                loaderparams.kernel_filename);
+        error_report("qemu: could not load kernel '%s': %s",
+                     loaderparams.kernel_filename,
+                     load_elf_strerror(kernel_size));
         exit(1);
     }
 
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index 690874be2b..2f5ced7409 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -31,6 +31,7 @@
 #include "sysemu/block-backend.h"
 #include "exec/address-spaces.h"
 #include "sysemu/qtest.h"
+#include "qemu/error-report.h"
 
 #define MAX_IDE_BUS 2
 
@@ -96,8 +97,9 @@ static int64_t load_kernel(void)
         if ((entry & ~0x7fffffffULL) == 0x80000000)
             entry = (int32_t)entry;
     } else {
-        fprintf(stderr, "qemu: could not load kernel '%s'\n",
-                loaderparams.kernel_filename);
+        error_report("qemu: could not load kernel '%s': %s",
+                     loaderparams.kernel_filename,
+                     load_elf_strerror(kernel_size));
         exit(1);
     }
 
-- 
2.11.0

Re: [Qemu-devel] [PATCH for 2.10 1/2] hw/mips: load_elf_strerror to report kernel loading failure

[Philippe Mathieu-Daudé]

On 07/26/2017 08:56 PM, Aurelien Jarno wrote:
> Emulated MIPS boards bail out with a simple "could not load kernel" when
> a kernel could not be load, without specifying the underlying reason.
> Fix that by calling load_elf_strerror.
> 
> At the same time use error_report to report the error instead of
> fprintf.
> 
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

Thank you Aurelien :)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>   hw/mips/mips_fulong2e.c | 15 +++++++++------
>   hw/mips/mips_malta.c    | 14 ++++++++------
>   hw/mips/mips_mipssim.c  |  5 +++--
>   hw/mips/mips_r4k.c      |  6 ++++--
>   4 files changed, 24 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
> index 3f3cb32651..3532399a13 100644
> --- a/hw/mips/mips_fulong2e.c
> +++ b/hw/mips/mips_fulong2e.c
> @@ -110,16 +110,19 @@ static int64_t load_kernel (CPUMIPSState *env)
>   {
>       int64_t kernel_entry, kernel_low, kernel_high;
>       int index = 0;
> -    long initrd_size;
> +    long kernel_size, initrd_size;
>       ram_addr_t initrd_offset;
>       uint32_t *prom_buf;
>       long prom_size;
>   
> -    if (load_elf(loaderparams.kernel_filename, cpu_mips_kseg0_to_phys, NULL,
> -                 (uint64_t *)&kernel_entry, (uint64_t *)&kernel_low,
> -                 (uint64_t *)&kernel_high, 0, EM_MIPS, 1, 0) < 0) {
> -        fprintf(stderr, "qemu: could not load kernel '%s'\n",
> -                loaderparams.kernel_filename);
> +    kernel_size = load_elf(loaderparams.kernel_filename, cpu_mips_kseg0_to_phys,
> +                           NULL, (uint64_t *)&kernel_entry,
> +                           (uint64_t *)&kernel_low, (uint64_t *)&kernel_high,
> +                           0, EM_MIPS, 1, 0);
> +    if (kernel_size < 0) {
> +        error_report("qemu: could not load kernel '%s': %s",
> +                     loaderparams.kernel_filename,
> +                     load_elf_strerror(kernel_size));
>           exit(1);
>       }
>   
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 3487d16f61..8ecd544baa 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -794,7 +794,7 @@ static void GCC_FMT_ATTR(3, 4) prom_set(uint32_t* prom_buf, int index,
>   static int64_t load_kernel (void)
>   {
>       int64_t kernel_entry, kernel_high;
> -    long initrd_size;
> +    long kernel_size, initrd_size;
>       ram_addr_t initrd_offset;
>       int big_endian;
>       uint32_t *prom_buf;
> @@ -808,11 +808,13 @@ static int64_t load_kernel (void)
>       big_endian = 0;
>   #endif
>   
> -    if (load_elf(loaderparams.kernel_filename, cpu_mips_kseg0_to_phys, NULL,
> -                 (uint64_t *)&kernel_entry, NULL, (uint64_t *)&kernel_high,
> -                 big_endian, EM_MIPS, 1, 0) < 0) {
> -        fprintf(stderr, "qemu: could not load kernel '%s'\n",
> -                loaderparams.kernel_filename);
> +    kernel_size = load_elf(loaderparams.kernel_filename, cpu_mips_kseg0_to_phys,
> +                           NULL, (uint64_t *)&kernel_entry, NULL,
> +                           (uint64_t *)&kernel_high, big_endian, EM_MIPS, 1, 0);
> +    if (kernel_size < 0) {
> +        error_report("qemu: could not load kernel '%s': %s",
> +                     loaderparams.kernel_filename,
> +                     load_elf_strerror(kernel_size));
>           exit(1);
>       }
>   
> diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
> index 6990b1b0dd..07fc4c2300 100644
> --- a/hw/mips/mips_mipssim.c
> +++ b/hw/mips/mips_mipssim.c
> @@ -78,8 +78,9 @@ static int64_t load_kernel(void)
>           if ((entry & ~0x7fffffffULL) == 0x80000000)
>               entry = (int32_t)entry;
>       } else {
> -        fprintf(stderr, "qemu: could not load kernel '%s'\n",
> -                loaderparams.kernel_filename);
> +        error_report("qemu: could not load kernel '%s': %s",
> +                     loaderparams.kernel_filename,
> +                     load_elf_strerror(kernel_size));
>           exit(1);
>       }
>   
> diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
> index 690874be2b..2f5ced7409 100644
> --- a/hw/mips/mips_r4k.c
> +++ b/hw/mips/mips_r4k.c
> @@ -31,6 +31,7 @@
>   #include "sysemu/block-backend.h"
>   #include "exec/address-spaces.h"
>   #include "sysemu/qtest.h"
> +#include "qemu/error-report.h"
>   
>   #define MAX_IDE_BUS 2
>   
> @@ -96,8 +97,9 @@ static int64_t load_kernel(void)
>           if ((entry & ~0x7fffffffULL) == 0x80000000)
>               entry = (int32_t)entry;
>       } else {
> -        fprintf(stderr, "qemu: could not load kernel '%s'\n",
> -                loaderparams.kernel_filename);
> +        error_report("qemu: could not load kernel '%s': %s",
> +                     loaderparams.kernel_filename,
> +                     load_elf_strerror(kernel_size));
>           exit(1);
>       }
>   
> 

[Qemu-devel] [PATCH for 2.10 2/2] Revert "elf-loader: warn about invalid endianness"

[Aurelien Jarno]

From: Alexey Kardashevskiy <aik@ozlabs.ru>

This reverts c8e1158cf611 "elf-loader: warn about invalid endianness"
as it produces a useless message every time an LE kernel image is
passed via -kernel on a ppc64-pseries machine. The pseries machine
already checks for ELF_LOAD_WRONG_ENDIAN and tries with big_endian=0.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 hw/core/loader.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index c17ace0a2e..e5e8cbb638 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -480,7 +480,6 @@ int load_elf_ram(const char *filename,
     }
 
     if (target_data_order != e_ident[EI_DATA]) {
-        fprintf(stderr, "%s: wrong endianness\n", filename);
         ret = ELF_LOAD_WRONG_ENDIAN;
         goto fail;
     }
-- 
2.11.0

Re: [Qemu-devel] [PATCH for 2.10 2/2] Revert "elf-loader: warn about invalid endianness"

[Philippe Mathieu-Daudé]

On 07/26/2017 08:56 PM, Aurelien Jarno wrote:
> From: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> This reverts c8e1158cf611 "elf-loader: warn about invalid endianness"
> as it produces a useless message every time an LE kernel image is
> passed via -kernel on a ppc64-pseries machine. The pseries machine
> already checks for ELF_LOAD_WRONG_ENDIAN and tries with big_endian=0.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>   hw/core/loader.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index c17ace0a2e..e5e8cbb638 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -480,7 +480,6 @@ int load_elf_ram(const char *filename,
>       }
>   
>       if (target_data_order != e_ident[EI_DATA]) {
> -        fprintf(stderr, "%s: wrong endianness\n", filename);
>           ret = ELF_LOAD_WRONG_ENDIAN;
>           goto fail;
>       }
>