[Qemu-devel] [PATCH 0/3] linux-user: select CPU type according ELF header values

[Qemu-devel] [PATCH 0/3] linux-user: select CPU type according ELF header values

[Laurent Vivier]

This idea has been suggested to me before by Philippe
Mathieu-Daudé, and recently YunQiang Su has proposed a
patch to manage the MIPS r6 case.

Based on this, this series tries to clean-up the original
patch, and introduces the use for m68k architecture and
port the patch from YunQiang Su.

Laurent Vivier (1):
  linux-user,m68k: select CPU according to ELF header values

YunQiang Su (2):
  linux-user: introduce functions to detect CPU type
  linux-user: MIPS set cpu to r6 CPU if binary is R6

 include/elf.h        |   4 ++
 linux-user/elfload.c |  37 ++++++++++++++++
 linux-user/main.c    | 121 +++++++++++++++++++++++++++++++--------------------
 linux-user/qemu.h    |   1 +
 4 files changed, 115 insertions(+), 48 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH 1/3] linux-user: introduce functions to detect CPU type

[Laurent Vivier]

From: YunQiang Su <syq@debian.org>

Move CPU type name selection to a function,
and add a function to return ELF e_flags.

[lv: splitted the patch and some cleanup in get_elf_eflags()]
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---

Notes:
    YunQiang Su, please add your Signed-off-by that was
    missing in your original patch.

 linux-user/elfload.c |  37 +++++++++++++++++++
 linux-user/main.c    | 101 +++++++++++++++++++++++++++------------------------
 linux-user/qemu.h    |   1 +
 3 files changed, 91 insertions(+), 48 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 20f3d8c2c3..03244f5e97 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2376,6 +2376,43 @@ give_up:
     g_free(syms);
 }
 
+int get_elf_eflags(int fd, uint32_t *eflags)
+{
+    struct elfhdr ehdr;
+    off_t offset;
+    int ret;
+
+    /* Read ELF header */
+    offset = lseek(fd, 0, SEEK_SET);
+    if (offset == (off_t) -1) {
+        return -1;
+    }
+    ret = read(fd, &ehdr, sizeof(ehdr));
+    if (ret < sizeof(ehdr)) {
+        return -1;
+    }
+    offset = lseek(fd, offset, SEEK_SET);
+    if (offset == (off_t) -1) {
+        return -1;
+    }
+
+    /* Check ELF signature */
+    if (!elf_check_ident(&ehdr)) {
+        return -1;
+    }
+
+    /* check header */
+    bswap_ehdr(&ehdr);
+    if (!elf_check_ehdr(&ehdr)) {
+        return -1;
+    }
+
+    /* return architecture id */
+    *eflags = ehdr.e_flags;
+
+    return 0;
+}
+
 int load_elf_binary(struct linux_binprm *bprm, struct image_info *info)
 {
     struct image_info interp_info;
diff --git a/linux-user/main.c b/linux-user/main.c
index 450eb3ce65..9ce90ae634 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -4254,6 +4254,49 @@ static int parse_args(int argc, char **argv)
     return optind;
 }
 
+static const char *get_cpu_model(int fd)
+{
+#if defined(TARGET_I386)
+#ifdef TARGET_X86_64
+    return "qemu64";
+#else
+    return "qemu32";
+#endif
+#elif defined(TARGET_ARM)
+    return "any";
+#elif defined(TARGET_UNICORE32)
+    return "any";
+#elif defined(TARGET_M68K)
+    return "any";
+#elif defined(TARGET_SPARC)
+#ifdef TARGET_SPARC64
+    return "TI UltraSparc II";
+#else
+    return "Fujitsu MB86904";
+#endif
+#elif defined(TARGET_MIPS)
+#if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64)
+    return "5KEf";
+#else
+    return "24Kf";
+#endif
+#elif defined TARGET_OPENRISC
+    return "or1200";
+#elif defined(TARGET_PPC)
+# ifdef TARGET_PPC64
+    return "POWER8";
+# else
+    return "750";
+# endif
+#elif defined TARGET_SH4
+    return "sh7785";
+#elif defined TARGET_S390X
+    return "qemu";
+#else
+    return "any";
+#endif
+}
+
 int main(int argc, char **argv, char **envp)
 {
     struct target_pt_regs regs1, *regs = &regs1;
@@ -4318,46 +4361,17 @@ int main(int argc, char **argv, char **envp)
 
     init_qemu_uname_release();
 
+    execfd = qemu_getauxval(AT_EXECFD);
+    if (execfd == 0) {
+        execfd = open(filename, O_RDONLY);
+        if (execfd < 0) {
+            printf("Error while loading %s: %s\n", filename, strerror(errno));
+            _exit(EXIT_FAILURE);
+        }
+    }
+
     if (cpu_model == NULL) {
-#if defined(TARGET_I386)
-#ifdef TARGET_X86_64
-        cpu_model = "qemu64";
-#else
-        cpu_model = "qemu32";
-#endif
-#elif defined(TARGET_ARM)
-        cpu_model = "any";
-#elif defined(TARGET_UNICORE32)
-        cpu_model = "any";
-#elif defined(TARGET_M68K)
-        cpu_model = "any";
-#elif defined(TARGET_SPARC)
-#ifdef TARGET_SPARC64
-        cpu_model = "TI UltraSparc II";
-#else
-        cpu_model = "Fujitsu MB86904";
-#endif
-#elif defined(TARGET_MIPS)
-#if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64)
-        cpu_model = "5KEf";
-#else
-        cpu_model = "24Kf";
-#endif
-#elif defined TARGET_OPENRISC
-        cpu_model = "or1200";
-#elif defined(TARGET_PPC)
-# ifdef TARGET_PPC64
-        cpu_model = "POWER8";
-# else
-        cpu_model = "750";
-# endif
-#elif defined TARGET_SH4
-        cpu_model = "sh7785";
-#elif defined TARGET_S390X
-        cpu_model = "qemu";
-#else
-        cpu_model = "any";
-#endif
+        cpu_model = get_cpu_model(execfd);
     }
     tcg_exec_init(0);
     /* NOTE: we need to init the CPU at this stage to get
@@ -4450,15 +4464,6 @@ int main(int argc, char **argv, char **envp)
     cpu->opaque = ts;
     task_settid(ts);
 
-    execfd = qemu_getauxval(AT_EXECFD);
-    if (execfd == 0) {
-        execfd = open(filename, O_RDONLY);
-        if (execfd < 0) {
-            printf("Error while loading %s: %s\n", filename, strerror(errno));
-            _exit(EXIT_FAILURE);
-        }
-    }
-
     ret = loader_exec(execfd, filename, target_argv, target_environ, regs,
         info, &bprm);
     if (ret != 0) {
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 4edd7d0c08..c2a701163c 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -188,6 +188,7 @@ int loader_exec(int fdexec, const char *filename, char **argv, char **envp,
              struct target_pt_regs * regs, struct image_info *infop,
              struct linux_binprm *);
 
+int get_elf_eflags(int fd, uint32_t *eflags);
 int load_elf_binary(struct linux_binprm *bprm, struct image_info *info);
 int load_flt_binary(struct linux_binprm *bprm, struct image_info *info);
 
-- 
2.14.3

Re: [Qemu-devel] [PATCH 1/3] linux-user: introduce functions to detect CPU type

[Richard Henderson]

On 01/13/2018 06:48 AM, Laurent Vivier wrote:
> From: YunQiang Su <syq@debian.org>
> 
> Move CPU type name selection to a function,
> and add a function to return ELF e_flags.
> 
> [lv: splitted the patch and some cleanup in get_elf_eflags()]
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---

This needs to be split.

> +int get_elf_eflags(int fd, uint32_t *eflags)
> +{
> +    struct elfhdr ehdr;
> +    off_t offset;
> +    int ret;
> +
> +    /* Read ELF header */
> +    offset = lseek(fd, 0, SEEK_SET);
> +    if (offset == (off_t) -1) {
> +        return -1;
> +    }
> +    ret = read(fd, &ehdr, sizeof(ehdr));
> +    if (ret < sizeof(ehdr)) {
> +        return -1;
> +    }

There is no reason to read the elf header twice -- e_flags has already been
stored in the struct image_info.

> +static const char *get_cpu_model(int fd)
> +{
> +#if defined(TARGET_I386)
> +#ifdef TARGET_X86_64
> +    return "qemu64";
> +#else
> +    return "qemu32";
> +#endif

This should be our opportunity to split this ifdef chain into small inline
functions within linux-user/*/target_cpu.h.  Pass the e_flags value directly
instead of a file descriptor.


r~

Re: [Qemu-devel] [PATCH 1/3] linux-user: introduce functions to detect CPU type

[Laurent Vivier]

Le 15/01/2018 à 23:04, Richard Henderson a écrit :
> On 01/13/2018 06:48 AM, Laurent Vivier wrote:
>> From: YunQiang Su <syq@debian.org>
>>
>> Move CPU type name selection to a function,
>> and add a function to return ELF e_flags.
>>
>> [lv: splitted the patch and some cleanup in get_elf_eflags()]
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>> ---
> 
> This needs to be split.
> 
>> +int get_elf_eflags(int fd, uint32_t *eflags)
>> +{
>> +    struct elfhdr ehdr;
>> +    off_t offset;
>> +    int ret;
>> +
>> +    /* Read ELF header */
>> +    offset = lseek(fd, 0, SEEK_SET);
>> +    if (offset == (off_t) -1) {
>> +        return -1;
>> +    }
>> +    ret = read(fd, &ehdr, sizeof(ehdr));
>> +    if (ret < sizeof(ehdr)) {
>> +        return -1;
>> +    }
> 
> There is no reason to read the elf header twice -- e_flags has already been
> stored in the struct image_info.

When we set cpu_model, image_info is not initialized.

Do you propose to move cpu_init() after loader_exec()?

>> +static const char *get_cpu_model(int fd)
>> +{
>> +#if defined(TARGET_I386)
>> +#ifdef TARGET_X86_64
>> +    return "qemu64";
>> +#else
>> +    return "qemu32";
>> +#endif
> 
> This should be our opportunity to split this ifdef chain into small inline
> functions within linux-user/*/target_cpu.h.  Pass the e_flags value directly
> instead of a file descriptor.
> 

Good idea.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 1/3] linux-user: introduce functions to detect CPU type

[Richard Henderson]

On 01/16/2018 06:13 AM, Laurent Vivier wrote:
>> There is no reason to read the elf header twice -- e_flags has already been
>> stored in the struct image_info.
> 
> When we set cpu_model, image_info is not initialized.
> Do you propose to move cpu_init() after loader_exec()?

Sure.


r~

Re: [Qemu-devel] [PATCH 1/3] linux-user: introduce functions to detect CPU type

[Laurent Vivier]

Le 16/01/2018 à 16:54, Richard Henderson a écrit :
> On 01/16/2018 06:13 AM, Laurent Vivier wrote:
>>> There is no reason to read the elf header twice -- e_flags has already been
>>> stored in the struct image_info.
>>
>> When we set cpu_model, image_info is not initialized.
>> Do you propose to move cpu_init() after loader_exec()?
> 
> Sure.

After checking, I think we can't move cpu_init() after loader_exec():

load_elf_binary() needs to fill AT_HWCAP and in the case of i386 it
depends on cpu->env.features[FEAT_1_EDX] that comes from the cpu model.

Thanks,
Laurent

[Qemu-devel] [PATCH 2/3] linux-user, m68k: select CPU according to ELF header values

[Laurent Vivier]

M680x0 doesn't support the same set of instructions
as ColdFire, so we can't use "any" CPU type to execute
m68020 instructions.
We select CPU type ("m68020" or "any" for ColdFire)
according to the ELF header. If we can't, we
use by default the value used until now: "any".

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/main.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/linux-user/main.c b/linux-user/main.c
index 9ce90ae634..2fc2267fd4 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -4267,6 +4267,16 @@ static const char *get_cpu_model(int fd)
 #elif defined(TARGET_UNICORE32)
     return "any";
 #elif defined(TARGET_M68K)
+    int ret;
+    uint32_t eflags;
+
+    ret = get_elf_eflags(fd, &eflags);
+    if (ret == 0 && eflags == 0) {
+        /* 680x0 */
+        return "m68020";
+    }
+
+    /* Coldfire */
     return "any";
 #elif defined(TARGET_SPARC)
 #ifdef TARGET_SPARC64
-- 
2.14.3

[Qemu-devel] [PATCH 3/3] linux-user: MIPS set cpu to r6 CPU if binary is R6

[Laurent Vivier]

From: YunQiang Su <syq@debian.org>

So here we need to detect the version of binaries and set
cpu_model for it.

[lv: original patch modified to move code into get_cpu_model()]
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---

Notes:
    YunQiang Su, please add your Signed-off-by that was
    missing in your original patch.

 include/elf.h     |  4 ++++
 linux-user/main.c | 10 ++++++++++
 2 files changed, 14 insertions(+)

diff --git a/include/elf.h b/include/elf.h
index e8a515ce3d..f2104809b1 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -40,6 +40,10 @@ typedef int64_t  Elf64_Sxword;
 #define EF_MIPS_ARCH_5		0x40000000	/* -mips5 code.  */
 #define EF_MIPS_ARCH_32		0x50000000	/* MIPS32 code.  */
 #define EF_MIPS_ARCH_64		0x60000000	/* MIPS64 code.  */
+#define EF_MIPS_ARCH_32R2       0x70000000      /* MIPS32r2 code.  */
+#define EF_MIPS_ARCH_64R2       0x80000000      /* MIPS64r2 code.  */
+#define EF_MIPS_ARCH_32R6       0x90000000      /* MIPS32r6 code.  */
+#define EF_MIPS_ARCH_64R6       0xa0000000      /* MIPS64r6 code.  */
 
 /* The ABI of a file. */
 #define EF_MIPS_ABI_O32		0x00001000	/* O32 ABI.  */
diff --git a/linux-user/main.c b/linux-user/main.c
index 2fc2267fd4..3229ef079e 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -4285,9 +4285,19 @@ static const char *get_cpu_model(int fd)
     return "Fujitsu MB86904";
 #endif
 #elif defined(TARGET_MIPS)
+    int ret;
+    uint32_t eflags;
+
+    ret = get_elf_eflags(fd, &eflags);
 #if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64)
+    if (ret == 0 && (eflags & EF_MIPS_ARCH_64R6) != 0) {
+        return "I6400";
+    }
     return "5KEf";
 #else
+    if (ret == 0 && (eflags & EF_MIPS_ARCH_32R6) != 0) {
+        return "mips32r6-generic";
+    }
     return "24Kf";
 #endif
 #elif defined TARGET_OPENRISC
-- 
2.14.3

Re: [Qemu-devel] [PATCH 3/3] linux-user: MIPS set cpu to r6 CPU if binary is R6

[YunQiang Su]

On Sat, Jan 13, 2018 at 10:48 PM, Laurent Vivier <laurent@vivier.eu> wrote:
> From: YunQiang Su <syq@debian.org>
>
> So here we need to detect the version of binaries and set
> cpu_model for it.
>
> [lv: original patch modified to move code into get_cpu_model()]
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>
> Notes:
>     YunQiang Su, please add your Signed-off-by that was
>     missing in your original patch.

How to add Signed-off-by? Send a v2 for my version of patch?

>
>  include/elf.h     |  4 ++++
>  linux-user/main.c | 10 ++++++++++
>  2 files changed, 14 insertions(+)
>
> diff --git a/include/elf.h b/include/elf.h
> index e8a515ce3d..f2104809b1 100644
> --- a/include/elf.h
> +++ b/include/elf.h
> @@ -40,6 +40,10 @@ typedef int64_t  Elf64_Sxword;
>  #define EF_MIPS_ARCH_5         0x40000000      /* -mips5 code.  */
>  #define EF_MIPS_ARCH_32                0x50000000      /* MIPS32 code.  */
>  #define EF_MIPS_ARCH_64                0x60000000      /* MIPS64 code.  */
> +#define EF_MIPS_ARCH_32R2       0x70000000      /* MIPS32r2 code.  */
> +#define EF_MIPS_ARCH_64R2       0x80000000      /* MIPS64r2 code.  */
> +#define EF_MIPS_ARCH_32R6       0x90000000      /* MIPS32r6 code.  */
> +#define EF_MIPS_ARCH_64R6       0xa0000000      /* MIPS64r6 code.  */
>
>  /* The ABI of a file. */
>  #define EF_MIPS_ABI_O32                0x00001000      /* O32 ABI.  */
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 2fc2267fd4..3229ef079e 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -4285,9 +4285,19 @@ static const char *get_cpu_model(int fd)
>      return "Fujitsu MB86904";
>  #endif
>  #elif defined(TARGET_MIPS)
> +    int ret;
> +    uint32_t eflags;
> +
> +    ret = get_elf_eflags(fd, &eflags);
>  #if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64)
> +    if (ret == 0 && (eflags & EF_MIPS_ARCH_64R6) != 0) {
> +        return "I6400";
> +    }
>      return "5KEf";
>  #else
> +    if (ret == 0 && (eflags & EF_MIPS_ARCH_32R6) != 0) {
> +        return "mips32r6-generic";
> +    }
>      return "24Kf";
>  #endif
>  #elif defined TARGET_OPENRISC
> --
> 2.14.3
>

Re: [Qemu-devel] [PATCH 3/3] linux-user: MIPS set cpu to r6 CPU if binary is R6

[Laurent Vivier]

Le 17/01/2018 à 03:26, YunQiang Su a écrit :
> On Sat, Jan 13, 2018 at 10:48 PM, Laurent Vivier <laurent@vivier.eu> wrote:
>> From: YunQiang Su <syq@debian.org>
>>
>> So here we need to detect the version of binaries and set
>> cpu_model for it.
>>
>> [lv: original patch modified to move code into get_cpu_model()]
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>>
>> Notes:
>>     YunQiang Su, please add your Signed-off-by that was
>>     missing in your original patch.
> 
> How to add Signed-off-by? Send a v2 for my version of patch?

You can reply to your original patch with your "Signed-off-by".
And as I did some changes in the following series, it would be good to
reply with a "Reviewed-by" to the new patches if you agree with the changes.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 3/3] linux-user: MIPS set cpu to r6 CPU if binary is R6

[Laurent Vivier]

Le 17/01/2018 à 03:26, YunQiang Su a écrit :
> On Sat, Jan 13, 2018 at 10:48 PM, Laurent Vivier <laurent@vivier.eu> wrote:
>> From: YunQiang Su <syq@debian.org>
>>
>> So here we need to detect the version of binaries and set
>> cpu_model for it.
>>
>> [lv: original patch modified to move code into get_cpu_model()]
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>>
>> Notes:
>>     YunQiang Su, please add your Signed-off-by that was
>>     missing in your original patch.
> 
> How to add Signed-off-by? Send a v2 for my version of patch?
> 

Could you reply to you original patch with your Signed-off-by?


Thanks,
Laurent