[Qemu-devel] [ARM/FDPIC 0/4] FDPIC ABI for ARM

[Qemu-devel] [ARM/FDPIC 0/4] FDPIC ABI for ARM

[Christophe Lyon]

Hello,

This patch series implements the QEMU contribution of the FDPIC
ABI for ARM targets.

This ABI enables to run Linux on ARM MMU-less cores and supports
shared libraries to reduce the memory footprint.

Without MMU, text and data segment relative distances are different
from one process to another, hence the need for a dedicated FDPIC
register holding the start address of the data segment. One of the
side effects is that function pointers require two words to be
represented: the address of the code, and the data segment start
address. These two words are designated as "Function Descriptor",
hence the "FD PIC" name.

On ARM, the FDPIC register is r9 [3].

This work was developed some time ago by STMicroelectronics, and was
presented during Linaro Connect SFO15 (September 2015). You can watch
the discussion and read the slides [1].
This presentation was related to the toolchain published on github [2],
which is based on binutils-2.22, gcc-4.7, uclibc-0.9.33.2, gdb-7.5.1
and qemu-2.3.0, and for which pre-built binaries are available [2].

The ABI itself is described in details in [3].

Our Linux kernel patches have been updated and committed by Nicolas
Pitre (Linaro) in July 2017. They are required so that the loader is
able to handle this new file type. Indeed, the ELF files are tagged
with ELFOSABI_ARM_FDPIC. This new tag has been allocated by ARM, as
well as the new relocations involved.

This patch series has been rebased on top of QEMU from 2018-03-28.

I have also rebased the GCC patch series, but it is still WIP as
cleanup is still needed before I can request a review. It can be
useful to build a preview toolchain though, so my WIP branch is
available at [4].
To build such a toolchain, you'd also need to use my uClibc
branch [5].

I am currently working on updating the patches for the other toolchain
components, and will upstream them soon. This includes gcc, uclibc,
and gdb.

This series provides support for ARM v7 and later architectures and
has been used to run the GCC tests on arm-linux-gnueabi without
regression, as well as arm-linux-uclibceabi.

Are the QEMU patches OK for inclusion in master?

Thanks,

Christophe.


[1] http://connect.linaro.org/resource/sfo15/sfo15-406-arm-fdpic-toolset-kernel-libraries-for-cortex-m-cortex-r-mmuless-cores/
[2] https://github.com/mickael-guene/fdpic_manifest
[3] https://github.com/mickael-guene/fdpic_doc/blob/master/abi.txt
[4] https://git.linaro.org/people/christophe.lyon/gcc.git/log/?h=fdpic-upstream
[5] https://git.linaro.org/people/christophe.lyon/uclibc.git/log/?h=uClibc-0.9.33.2-fdpic-upstream

Christophe Lyon (4):
  linux-user: ARM-FDPIC: Add configure option to support loading of
    FDPIC binaries
  linux-user: ARM-FDPIC: Add support of FDPIC for ARM.
  linux-user: ARM-FDPIC: Add support for signals for FDPIC targets
  linux-user: ARM-FDPIC: Add arm get tls syscall support

 configure                       | 10 +++++
 include/elf.h                   |  1 +
 linux-user/arm/target_syscall.h |  1 +
 linux-user/elfload.c            | 35 ++++++++++++++++
 linux-user/main.c               |  8 ++++
 linux-user/qemu.h               |  3 ++
 linux-user/signal.c             | 91 +++++++++++++++++++++++++++++++++++++----
 target/arm/cpu.h                |  6 +++
 8 files changed, 147 insertions(+), 8 deletions(-)

-- 
2.6.3

[Qemu-devel] [ARM/FDPIC 1/4] linux-user: ARM-FDPIC: Add configure option to support loading of FDPIC binaries

[Christophe Lyon]

Adds --enable-fdpic and --disable-fdpic configure options.  This
feature is disabled by default, that's why it is not described in the
"Optional features" help section (which are enabled by default if
possible).

FDPIC ELF objects are identified with e_ident[EI_OSABI] ==
ELFOSABI_ARM_FDPIC.

Co-Authored-By: Mickaël Guêné <mickael.guene@st.com>
Signed-off-by: Christophe Lyon <christophe.lyon@st.com>

diff --git a/configure b/configure
index 4d0e92c..af4c14b 100755
--- a/configure
+++ b/configure
@@ -451,6 +451,7 @@ jemalloc="no"
 replication="yes"
 vxhs=""
 libxml2=""
+fdpic="no"
 
 supported_cpu="no"
 supported_os="no"
@@ -1374,6 +1375,10 @@ for opt do
   ;;
   --disable-git-update) git_update=no
   ;;
+  --disable-fdpic) fdpic="no"
+  ;;
+  --enable-fdpic) fdpic="yes"
+  ;;
   *)
       echo "ERROR: unknown option $opt"
       echo "Try '$0 --help' for more information"
@@ -1544,6 +1549,8 @@ Advanced options (experts only):
                            xen pv domain builder
   --enable-debug-stack-usage
                            track the maximum stack usage of stacks created by qemu_alloc_stack
+  --disable-fdpic          disable loading of FDPIC binary (default)
+  --enable-fdpic           enable loading of FDPIC binary
 
 Optional features, enabled with --enable-FEATURE and
 disabled with --disable-FEATURE, default is enabled if available:
@@ -7085,6 +7092,9 @@ fi
 
 echo "LDFLAGS+=$ldflags" >> $config_target_mak
 echo "QEMU_CFLAGS+=$cflags" >> $config_target_mak
+if [ "$fdpic" = "yes" ]; then
+    echo "CONFIG_USE_FDPIC=y" >> $config_target_mak
+fi
 
 done # for target in $targets
 
diff --git a/include/elf.h b/include/elf.h
index c0dc9bb..934dbbd 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -1483,6 +1483,7 @@ typedef struct elf64_shdr {
 #define ELFOSABI_TRU64          10      /* Compaq TRU64 UNIX.  */
 #define ELFOSABI_MODESTO        11      /* Novell Modesto.  */
 #define ELFOSABI_OPENBSD        12      /* OpenBSD.  */
+#define ELFOSABI_ARM_FDPIC      65      /* ARM FDPIC */
 #define ELFOSABI_ARM            97      /* ARM */
 #define ELFOSABI_STANDALONE     255     /* Standalone (embedded) application */
 
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 23e3495..7ba3795 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1658,6 +1658,14 @@ static void zero_bss(abi_ulong elf_bss, abi_ulong last_bss, int prot)
 }
 
 #ifdef CONFIG_USE_FDPIC
+
+#ifdef TARGET_ARM
+static int elf_is_fdpic(struct elfhdr *exec)
+{
+    return exec->e_ident[EI_OSABI] == ELFOSABI_ARM_FDPIC;
+}
+#endif
+
 static abi_ulong loader_build_fdpic_loadmap(struct image_info *info, abi_ulong sp)
 {
     uint16_t n;
-- 
2.6.3

Re: [Qemu-devel] [ARM/FDPIC 1/4] linux-user: ARM-FDPIC: Add configure option to support loading of FDPIC binaries

[Peter Maydell]

On 6 April 2018 at 16:17, Christophe Lyon <christophe.lyon@st.com> wrote:
> Adds --enable-fdpic and --disable-fdpic configure options.  This
> feature is disabled by default, that's why it is not described in the
> "Optional features" help section (which are enabled by default if
> possible).
>
> FDPIC ELF objects are identified with e_ident[EI_OSABI] ==
> ELFOSABI_ARM_FDPIC.

If we can identify these ELF objects by looking in the ELF header,
why would we want to make this all a configure option that's
disabled by default?

I think we should just have support for these binaries enabled
always. Otherwise it will bitrot because nobody compiles it.

thanks
-- PMM

Re: [Qemu-devel] [ARM/FDPIC 1/4] linux-user: ARM-FDPIC: Add configure option to support loading of FDPIC binaries

[Christophe Lyon]

On 13/04/2018 17:04, Peter Maydell wrote:
> On 6 April 2018 at 16:17, Christophe Lyon <christophe.lyon@st.com> wrote:
>> Adds --enable-fdpic and --disable-fdpic configure options.  This
>> feature is disabled by default, that's why it is not described in the
>> "Optional features" help section (which are enabled by default if
>> possible).
>>
>> FDPIC ELF objects are identified with e_ident[EI_OSABI] ==
>> ELFOSABI_ARM_FDPIC.
> 
> If we can identify these ELF objects by looking in the ELF header,
> why would we want to make this all a configure option that's
> disabled by default?
> 
> I think we should just have support for these binaries enabled
> always. Otherwise it will bitrot because nobody compiles it.
> 
We are relying on a pre-existing CONFIG_USE_FDPIC, which is currently
never defined. This was introduced by commit 1af02e83 back in 2011,
and indeed if you try to build QEMU with CONFIG_USE_FDPIC, the build fails because elf_is_fdpic() is not defined.

We tried to conform to this existing support, but maybe it's already bit-rotten?
Do you suggest removing CONFIG_USE_FDPIC?

Thanks



> thanks
> -- PMM
> .
> 

Re: [Qemu-devel] [ARM/FDPIC 1/4] linux-user: ARM-FDPIC: Add configure option to support loading of FDPIC binaries

[Peter Maydell]

On 16 April 2018 at 08:57, Christophe Lyon <christophe.lyon@st.com> wrote:
> On 13/04/2018 17:04, Peter Maydell wrote:
>> I think we should just have support for these binaries enabled
>> always. Otherwise it will bitrot because nobody compiles it.
>>
> We are relying on a pre-existing CONFIG_USE_FDPIC, which is currently
> never defined. This was introduced by commit 1af02e83 back in 2011,
> and indeed if you try to build QEMU with CONFIG_USE_FDPIC, the build fails
> because elf_is_fdpic() is not defined.

This looks like an excellent demonstration of why we shouldn't
hide this feature behind a defaults-to-off #define :-)

> Do you suggest removing CONFIG_USE_FDPIC?

Yes. Given how old the existing disabled FDPIC code is, I would also
suggest you carefully review any of it that you're using...

thanks
-- PMM

[Qemu-devel] [ARM/FDPIC 2/4] linux-user: ARM-FDPIC: Add support of FDPIC for ARM.

[Christophe Lyon]

Add FDPIC info into image_info structure since interpreter info is on
stack and needs to be saved to be accessed later on.

Co-Authored-By:  Mickaël Guêné <mickael.guene@st.com>
Signed-off-by: Christophe Lyon <christophe.lyon@st.com>

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 7ba3795..363da67 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -287,6 +287,23 @@ static inline void init_thread(struct target_pt_regs *regs,
     /* For uClinux PIC binaries.  */
     /* XXX: Linux does this only on ARM with no MMU (do we care ?) */
     regs->uregs[10] = infop->start_data;
+#ifdef CONFIG_USE_FDPIC
+    /* Support ARM FDPIC.  */
+    /* As described in the ABI document, r7 points to the loadmap info
+     * prepared by the kernel. If an interpreter is needed, r8 points
+     * to the interpreter loadmap and r9 points to the interpreter
+     * PT_DYNAMIC info. If no interpreter is needed, r8 is zer0, and
+     * r9 points to the main program PT_DYNAMIC info.  */
+    regs->uregs[7] = infop->loadmap_addr;
+    if (infop->interpreter_loadmap_addr) {
+        /* Executable is dynamically loaded.  */
+        regs->uregs[8] = infop->interpreter_loadmap_addr;
+        regs->uregs[9] = infop->interpreter_pt_dynamic_addr;
+    } else {
+        regs->uregs[8] = 0;
+        regs->uregs[9] = infop->pt_dynamic_addr;
+    }
+#endif
 }
 
 #define ELF_NREG    18
@@ -1692,6 +1709,11 @@ static abi_ulong loader_build_fdpic_loadmap(struct image_info *info, abi_ulong s
 }
 #endif
 
+int info_is_fdpic(struct image_info *info)
+{
+    return (info->personality == PER_LINUX_FDPIC);
+}
+
 static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc,
                                    struct elfhdr *exec,
                                    struct image_info *info,
@@ -1719,6 +1741,11 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc,
         if (interp_info) {
             interp_info->other_info = info;
             sp = loader_build_fdpic_loadmap(interp_info, sp);
+            info->interpreter_loadmap_addr = interp_info->loadmap_addr;
+            info->interpreter_pt_dynamic_addr = interp_info->pt_dynamic_addr;
+        } else {
+            info->interpreter_loadmap_addr = 0;
+            info->interpreter_pt_dynamic_addr = 0;
         }
     }
 #endif
diff --git a/linux-user/main.c b/linux-user/main.c
index ba09b7d..00810d6 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -4868,6 +4868,11 @@ int main(int argc, char **argv, char **envp)
             env->cp15.sctlr_el[i] |= SCTLR_EE;
         }
 #endif
+
+#if defined(CONFIG_USE_FDPIC)
+        /* Are we running an FDPIC binary?  */
+        env->is_fdpic = info_is_fdpic(info);
+#endif
     }
 #elif defined(TARGET_ARM)
     {
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 192a0d2..7eaf9e9 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -56,6 +56,8 @@ struct image_info {
         uint16_t        nsegs;
         void           *loadsegs;
         abi_ulong       pt_dynamic_addr;
+        abi_ulong       interpreter_loadmap_addr;
+        abi_ulong       interpreter_pt_dynamic_addr;
         struct image_info *other_info;
 #endif
 };
@@ -182,6 +184,7 @@ abi_ulong loader_build_argptr(int envc, int argc, abi_ulong sp,
 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 info_is_fdpic(struct image_info *info);
 
 uint32_t get_elf_eflags(int fd);
 int load_elf_binary(struct linux_binprm *bprm, struct image_info *info);
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 19a0c03..90c8ee1 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -629,6 +629,12 @@ typedef struct CPUARMState {
     const struct arm_boot_info *boot_info;
     /* Store GICv3CPUState to access from this struct */
     void *gicv3state;
+
+#if defined(CONFIG_USER_ONLY) && defined(CONFIG_USE_FDPIC)
+    /* We need to know if we have an FDPIC binary to adapt signal
+     * syscalls.  */
+    int is_fdpic;
+#endif
 } CPUARMState;
 
 /**
-- 
2.6.3

Re: [Qemu-devel] [ARM/FDPIC 2/4] linux-user: ARM-FDPIC: Add support of FDPIC for ARM.

[Peter Maydell]

On 6 April 2018 at 16:17, Christophe Lyon <christophe.lyon@st.com> wrote:
> Add FDPIC info into image_info structure since interpreter info is on
> stack and needs to be saved to be accessed later on.
>
> Co-Authored-By:  Mickaël Guêné <mickael.guene@st.com>
> Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 7ba3795..363da67 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -287,6 +287,23 @@ static inline void init_thread(struct target_pt_regs *regs,
>      /* For uClinux PIC binaries.  */
>      /* XXX: Linux does this only on ARM with no MMU (do we care ?) */
>      regs->uregs[10] = infop->start_data;
> +#ifdef CONFIG_USE_FDPIC
> +    /* Support ARM FDPIC.  */
> +    /* As described in the ABI document, r7 points to the loadmap info
> +     * prepared by the kernel. If an interpreter is needed, r8 points
> +     * to the interpreter loadmap and r9 points to the interpreter
> +     * PT_DYNAMIC info. If no interpreter is needed, r8 is zer0, and
> +     * r9 points to the main program PT_DYNAMIC info.  */
> +    regs->uregs[7] = infop->loadmap_addr;
> +    if (infop->interpreter_loadmap_addr) {
> +        /* Executable is dynamically loaded.  */
> +        regs->uregs[8] = infop->interpreter_loadmap_addr;
> +        regs->uregs[9] = infop->interpreter_pt_dynamic_addr;
> +    } else {
> +        regs->uregs[8] = 0;
> +        regs->uregs[9] = infop->pt_dynamic_addr;
> +    }

Is it really correct to set these registers always, and not only if
this is an FDPIC ELF ?

> +#endif
>  }

>  int load_elf_binary(struct linux_binprm *bprm, struct image_info *info);
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 19a0c03..90c8ee1 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -629,6 +629,12 @@ typedef struct CPUARMState {
>      const struct arm_boot_info *boot_info;
>      /* Store GICv3CPUState to access from this struct */
>      void *gicv3state;
> +
> +#if defined(CONFIG_USER_ONLY) && defined(CONFIG_USE_FDPIC)
> +    /* We need to know if we have an FDPIC binary to adapt signal
> +     * syscalls.  */
> +    int is_fdpic;

linux-user specific information shouldn't live in CPUARMState;
put it in the TaskState struct instead, perhaps.

> +#endif
>  } CPUARMState;
>
>  /**

thanks
-- PMM

Re: [Qemu-devel] [ARM/FDPIC 2/4] linux-user: ARM-FDPIC: Add support of FDPIC for ARM.

[Christophe Lyon]

On 13/04/2018 17:07, Peter Maydell wrote:
> On 6 April 2018 at 16:17, Christophe Lyon <christophe.lyon@st.com> wrote:
>> Add FDPIC info into image_info structure since interpreter info is on
>> stack and needs to be saved to be accessed later on.
>>
>> Co-Authored-By:  Mickaël Guêné <mickael.guene@st.com>
>> Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
>>
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index 7ba3795..363da67 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -287,6 +287,23 @@ static inline void init_thread(struct target_pt_regs *regs,
>>       /* For uClinux PIC binaries.  */
>>       /* XXX: Linux does this only on ARM with no MMU (do we care ?) */
>>       regs->uregs[10] = infop->start_data;
>> +#ifdef CONFIG_USE_FDPIC
>> +    /* Support ARM FDPIC.  */
>> +    /* As described in the ABI document, r7 points to the loadmap info
>> +     * prepared by the kernel. If an interpreter is needed, r8 points
>> +     * to the interpreter loadmap and r9 points to the interpreter
>> +     * PT_DYNAMIC info. If no interpreter is needed, r8 is zer0, and
>> +     * r9 points to the main program PT_DYNAMIC info.  */
>> +    regs->uregs[7] = infop->loadmap_addr;
>> +    if (infop->interpreter_loadmap_addr) {
>> +        /* Executable is dynamically loaded.  */
>> +        regs->uregs[8] = infop->interpreter_loadmap_addr;
>> +        regs->uregs[9] = infop->interpreter_pt_dynamic_addr;
>> +    } else {
>> +        regs->uregs[8] = 0;
>> +        regs->uregs[9] = infop->pt_dynamic_addr;
>> +    }
> 
> Is it really correct to set these registers always, and not only if
> this is an FDPIC ELF ?
> 
I need to check, but I used my FDPIC-enabled QEMU to run non-FDPIC
binaries and saw no problem.

>> +#endif
>>   }
> 
>>   int load_elf_binary(struct linux_binprm *bprm, struct image_info *info);
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index 19a0c03..90c8ee1 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -629,6 +629,12 @@ typedef struct CPUARMState {
>>       const struct arm_boot_info *boot_info;
>>       /* Store GICv3CPUState to access from this struct */
>>       void *gicv3state;
>> +
>> +#if defined(CONFIG_USER_ONLY) && defined(CONFIG_USE_FDPIC)
>> +    /* We need to know if we have an FDPIC binary to adapt signal
>> +     * syscalls.  */
>> +    int is_fdpic;
> 
> linux-user specific information shouldn't live in CPUARMState;
> put it in the TaskState struct instead, perhaps.
> 
OK, I'll have a look at that.

>> +#endif
>>   } CPUARMState;
>>
>>   /**
> 
> thanks
> -- PMM
> .
> 

[Qemu-devel] [ARM/FDPIC 3/4] linux-user: ARM-FDPIC: Add support for signals for FDPIC targets

[Christophe Lyon]

The FDPIC restorer needs to deal with a function descriptor, hence we
have to extend 'retcode' such that it can hold the instructions needed
to perform this.

The restorer sequence uses the same thumbness as the exception
handler (mainly to support Thumb-only architectures).

Co-Authored-By: Mickaël Guêné <mickael.guene@st.com>
Signed-off-by: Christophe Lyon <christophe.lyon@st.com>

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 2ea3e03..75643d7 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -2039,13 +2039,13 @@ struct sigframe_v1
 {
     struct target_sigcontext sc;
     abi_ulong extramask[TARGET_NSIG_WORDS-1];
-    abi_ulong retcode;
+    abi_ulong retcode[4];
 };
 
 struct sigframe_v2
 {
     struct target_ucontext_v2 uc;
-    abi_ulong retcode;
+    abi_ulong retcode[4];
 };
 
 struct rt_sigframe_v1
@@ -2054,14 +2054,14 @@ struct rt_sigframe_v1
     abi_ulong puc;
     struct target_siginfo info;
     struct target_ucontext_v1 uc;
-    abi_ulong retcode;
+    abi_ulong retcode[4];
 };
 
 struct rt_sigframe_v2
 {
     struct target_siginfo info;
     struct target_ucontext_v2 uc;
-    abi_ulong retcode;
+    abi_ulong retcode[4];
 };
 
 #define TARGET_CONFIG_CPU_32 1
@@ -2084,6 +2084,23 @@ static const abi_ulong retcodes[4] = {
 	SWI_SYS_RT_SIGRETURN,	SWI_THUMB_RT_SIGRETURN
 };
 
+#if defined(CONFIG_USE_FDPIC)
+/*
+ * Stub needed to make sure the FD register (r9) contains the right
+ * value.
+ */
+static const unsigned long sigreturn_fdpic_codes[3] = {
+    0xe59fc004, /* ldr r12, [pc, #4] to read function descriptor */
+    0xe59c9004, /* ldr r9, [r12, #4] to setup GOT */
+    0xe59cf000  /* ldr pc, [r12] to jump into restorer */
+};
+
+static const unsigned long sigreturn_fdpic_thumb_codes[3] = {
+    0xc008f8df, /* ldr r12, [pc, #8] to read function descriptor */
+    0x9004f8dc, /* ldr r9, [r12, #4] to setup GOT */
+    0xf000f8dc  /* ldr pc, [r12] to jump into restorer */
+};
+#endif
 
 static inline int valid_user_regs(CPUARMState *regs)
 {
@@ -2143,7 +2160,19 @@ setup_return(CPUARMState *env, struct target_sigaction *ka,
 {
     abi_ulong handler = ka->_sa_handler;
     abi_ulong retcode;
+
+#ifdef CONFIG_USE_FDPIC
+    int thumb;
+
+    if (env->is_fdpic) {
+        thumb = (((abi_ulong *)g2h(ka->_sa_handler))[0]) & 1;
+    } else {
+        thumb = handler & 1;
+    }
+#else
     int thumb = handler & 1;
+#endif
+
     uint32_t cpsr = cpsr_read(env);
 
     cpsr &= ~CPSR_IT;
@@ -2154,8 +2183,37 @@ setup_return(CPUARMState *env, struct target_sigaction *ka,
     }
 
     if (ka->sa_flags & TARGET_SA_RESTORER) {
+#ifdef CONFIG_USE_FDPIC
+        if (env->is_fdpic) {
+            /* For FDPIC we ensure that the restorer is called with a
+             * correct r9 value.  For that we need to write code on
+             * the stack that sets r9 and jumps back to restorer
+             * value.
+             */
+            if (thumb) {
+                __put_user(sigreturn_fdpic_thumb_codes[0], rc);
+                __put_user(sigreturn_fdpic_thumb_codes[1], rc + 1);
+                __put_user(sigreturn_fdpic_thumb_codes[2], rc + 2);
+                __put_user((abi_ulong)ka->sa_restorer, rc + 3);
+            } else {
+                __put_user(sigreturn_fdpic_codes[0], rc);
+                __put_user(sigreturn_fdpic_codes[1], rc + 1);
+                __put_user(sigreturn_fdpic_codes[2], rc + 2);
+                __put_user((abi_ulong)ka->sa_restorer, rc + 3);
+            }
+
+            retcode = rc_addr + thumb;
+        } else
+#endif
         retcode = ka->sa_restorer;
     } else {
+#ifdef CONFIG_USE_FDPIC
+        if (env->is_fdpic) {
+            qemu_log_mask(LOG_UNIMP,
+                          "arm: FDPIC signal return not implemented");
+            abort();
+        } else {
+#endif
         unsigned int idx = thumb;
 
         if (ka->sa_flags & TARGET_SA_SIGINFO) {
@@ -2165,12 +2223,29 @@ setup_return(CPUARMState *env, struct target_sigaction *ka,
         __put_user(retcodes[idx], rc);
 
         retcode = rc_addr + thumb;
+#ifdef CONFIG_USE_FDPIC
+        }
+#endif
     }
 
     env->regs[0] = usig;
+#ifdef CONFIG_USE_FDPIC
+    if (env->is_fdpic) {
+        env->regs[9] = ((abi_ulong *)g2h(ka->_sa_handler))[1];
+    }
+#endif
     env->regs[13] = frame_addr;
     env->regs[14] = retcode;
+#ifdef CONFIG_USE_FDPIC
+    if (env->is_fdpic) {
+        env->regs[15] = ((abi_ulong *)g2h(ka->_sa_handler))[0]
+            & (thumb ? ~1 : ~3);
+    } else {
+        env->regs[15] = handler & (thumb ? ~1 : ~3);
+    }
+#else
     env->regs[15] = handler & (thumb ? ~1 : ~3);
+#endif
     cpsr_write(env, cpsr, CPSR_IT | CPSR_T, CPSRWriteByInstr);
 }
 
@@ -2264,7 +2339,7 @@ static void setup_frame_v1(int usig, struct target_sigaction *ka,
         __put_user(set->sig[i], &frame->extramask[i - 1]);
     }
 
-    setup_return(regs, ka, &frame->retcode, frame_addr, usig,
+    setup_return(regs, ka, frame->retcode, frame_addr, usig,
                  frame_addr + offsetof(struct sigframe_v1, retcode));
 
     unlock_user_struct(frame, frame_addr, 1);
@@ -2286,7 +2361,7 @@ static void setup_frame_v2(int usig, struct target_sigaction *ka,
 
     setup_sigframe_v2(&frame->uc, set, regs);
 
-    setup_return(regs, ka, &frame->retcode, frame_addr, usig,
+    setup_return(regs, ka, frame->retcode, frame_addr, usig,
                  frame_addr + offsetof(struct sigframe_v2, retcode));
 
     unlock_user_struct(frame, frame_addr, 1);
@@ -2341,7 +2416,7 @@ static void setup_rt_frame_v1(int usig, struct target_sigaction *ka,
         __put_user(set->sig[i], &frame->uc.tuc_sigmask.sig[i]);
     }
 
-    setup_return(env, ka, &frame->retcode, frame_addr, usig,
+    setup_return(env, ka, frame->retcode, frame_addr, usig,
                  frame_addr + offsetof(struct rt_sigframe_v1, retcode));
 
     env->regs[1] = info_addr;
@@ -2372,7 +2447,7 @@ static void setup_rt_frame_v2(int usig, struct target_sigaction *ka,
 
     setup_sigframe_v2(&frame->uc, set, env);
 
-    setup_return(env, ka, &frame->retcode, frame_addr, usig,
+    setup_return(env, ka, frame->retcode, frame_addr, usig,
                  frame_addr + offsetof(struct rt_sigframe_v2, retcode));
 
     env->regs[1] = info_addr;
-- 
2.6.3

Re: [Qemu-devel] [ARM/FDPIC 3/4] linux-user: ARM-FDPIC: Add support for signals for FDPIC targets

[Peter Maydell]

On 6 April 2018 at 16:17, Christophe Lyon <christophe.lyon@st.com> wrote:
> The FDPIC restorer needs to deal with a function descriptor, hence we
> have to extend 'retcode' such that it can hold the instructions needed
> to perform this.
>
> The restorer sequence uses the same thumbness as the exception
> handler (mainly to support Thumb-only architectures).
>
> Co-Authored-By: Mickaël Guêné <mickael.guene@st.com>
> Signed-off-by: Christophe Lyon <christophe.lyon@st.com>

> +#if defined(CONFIG_USE_FDPIC)
> +/*
> + * Stub needed to make sure the FD register (r9) contains the right
> + * value.
> + */
> +static const unsigned long sigreturn_fdpic_codes[3] = {
> +    0xe59fc004, /* ldr r12, [pc, #4] to read function descriptor */
> +    0xe59c9004, /* ldr r9, [r12, #4] to setup GOT */
> +    0xe59cf000  /* ldr pc, [r12] to jump into restorer */
> +};
> +
> +static const unsigned long sigreturn_fdpic_thumb_codes[3] = {
> +    0xc008f8df, /* ldr r12, [pc, #8] to read function descriptor */
> +    0x9004f8dc, /* ldr r9, [r12, #4] to setup GOT */
> +    0xf000f8dc  /* ldr pc, [r12] to jump into restorer */
> +};
> +#endif
>
>  static inline int valid_user_regs(CPUARMState *regs)
>  {
> @@ -2143,7 +2160,19 @@ setup_return(CPUARMState *env, struct target_sigaction *ka,
>  {
>      abi_ulong handler = ka->_sa_handler;
>      abi_ulong retcode;
> +
> +#ifdef CONFIG_USE_FDPIC
> +    int thumb;
> +
> +    if (env->is_fdpic) {
> +        thumb = (((abi_ulong *)g2h(ka->_sa_handler))[0]) & 1;
> +    } else {
> +        thumb = handler & 1;
> +    }

Dereferencing a pointer obtained by a raw g2h() is very likely
wrong. You want either to use one of the get_user_*() macros, or
a lock_user/__get_user/unlock_user sequence, so that you can
detect whether the guest actually has read access to the address,
and correctly handle the case where it does not.

thanks
-- PMM

[Qemu-devel] [ARM/FDPIC 4/4] linux-user: ARM-FDPIC: Add arm get tls syscall support

[Christophe Lyon]

Co-Authored-By: Mickaël Guêné <mickael.guene@st.com>
Signed-off-by: Christophe Lyon <christophe.lyon@st.com>

diff --git a/linux-user/arm/target_syscall.h b/linux-user/arm/target_syscall.h
index 94e2a42..afc0772 100644
--- a/linux-user/arm/target_syscall.h
+++ b/linux-user/arm/target_syscall.h
@@ -16,6 +16,7 @@ struct target_pt_regs {
 #define ARM_NR_breakpoint (ARM_NR_BASE + 1)
 #define ARM_NR_cacheflush (ARM_NR_BASE + 2)
 #define ARM_NR_set_tls	  (ARM_NR_BASE + 5)
+#define ARM_NR_get_tls    (ARM_NR_BASE + 6)
 
 #define ARM_NR_semihosting	  0x123456
 #define ARM_NR_thumb_semihosting  0xAB
diff --git a/linux-user/main.c b/linux-user/main.c
index 00810d6..1814578 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -720,6 +720,9 @@ void cpu_loop(CPUARMState *env)
                         case ARM_NR_breakpoint:
                             env->regs[15] -= env->thumb ? 2 : 4;
                             goto excp_debug;
+                        case ARM_NR_get_tls:
+                            env->regs[0] = cpu_get_tls(env);
+                            break;
                         default:
                             gemu_log("qemu: Unsupported ARM syscall: 0x%x\n",
                                      n);
-- 
2.6.3

Re: [Qemu-devel] [ARM/FDPIC 4/4] linux-user: ARM-FDPIC: Add arm get tls syscall support

[Peter Maydell]

On 6 April 2018 at 16:17, Christophe Lyon <christophe.lyon@st.com> wrote:
> Co-Authored-By: Mickaël Guêné <mickael.guene@st.com>
> Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
>
> diff --git a/linux-user/arm/target_syscall.h b/linux-user/arm/target_syscall.h
> index 94e2a42..afc0772 100644
> --- a/linux-user/arm/target_syscall.h
> +++ b/linux-user/arm/target_syscall.h
> @@ -16,6 +16,7 @@ struct target_pt_regs {
>  #define ARM_NR_breakpoint (ARM_NR_BASE + 1)
>  #define ARM_NR_cacheflush (ARM_NR_BASE + 2)
>  #define ARM_NR_set_tls   (ARM_NR_BASE + 5)
> +#define ARM_NR_get_tls    (ARM_NR_BASE + 6)
>
>  #define ARM_NR_semihosting       0x123456
>  #define ARM_NR_thumb_semihosting  0xAB
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 00810d6..1814578 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -720,6 +720,9 @@ void cpu_loop(CPUARMState *env)
>                          case ARM_NR_breakpoint:
>                              env->regs[15] -= env->thumb ? 2 : 4;
>                              goto excp_debug;
> +                        case ARM_NR_get_tls:
> +                            env->regs[0] = cpu_get_tls(env);
> +                            break;
>                          default:
>                              gemu_log("qemu: Unsupported ARM syscall: 0x%x\n",
>                                       n);

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

This is a bug fix unrelated to FDPIC, I think.

thanks
-- PMM

Re: [Qemu-devel] [ARM/FDPIC 4/4] linux-user: ARM-FDPIC: Add arm get tls syscall support

[Christophe Lyon]

On 13/04/2018 17:03, Peter Maydell wrote:
> On 6 April 2018 at 16:17, Christophe Lyon <christophe.lyon@st.com> wrote:
>> Co-Authored-By: Mickaël Guêné <mickael.guene@st.com>
>> Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
>>
>> diff --git a/linux-user/arm/target_syscall.h b/linux-user/arm/target_syscall.h
>> index 94e2a42..afc0772 100644
>> --- a/linux-user/arm/target_syscall.h
>> +++ b/linux-user/arm/target_syscall.h
>> @@ -16,6 +16,7 @@ struct target_pt_regs {
>>   #define ARM_NR_breakpoint (ARM_NR_BASE + 1)
>>   #define ARM_NR_cacheflush (ARM_NR_BASE + 2)
>>   #define ARM_NR_set_tls   (ARM_NR_BASE + 5)
>> +#define ARM_NR_get_tls    (ARM_NR_BASE + 6)
>>
>>   #define ARM_NR_semihosting       0x123456
>>   #define ARM_NR_thumb_semihosting  0xAB
>> diff --git a/linux-user/main.c b/linux-user/main.c
>> index 00810d6..1814578 100644
>> --- a/linux-user/main.c
>> +++ b/linux-user/main.c
>> @@ -720,6 +720,9 @@ void cpu_loop(CPUARMState *env)
>>                           case ARM_NR_breakpoint:
>>                               env->regs[15] -= env->thumb ? 2 : 4;
>>                               goto excp_debug;
>> +                        case ARM_NR_get_tls:
>> +                            env->regs[0] = cpu_get_tls(env);
>> +                            break;
>>                           default:
>>                               gemu_log("qemu: Unsupported ARM syscall: 0x%x\n",
>>                                        n);
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> This is a bug fix unrelated to FDPIC, I think.
> 

Right, I'll resubmit it with an updated commit message.

> thanks
> -- PMM
> .
> 

Re: [Qemu-devel] [ARM/FDPIC 0/4] FDPIC ABI for ARM

[Peter Maydell]

On 6 April 2018 at 16:17, Christophe Lyon <christophe.lyon@st.com> wrote:
> Hello,
>
> This patch series implements the QEMU contribution of the FDPIC
> ABI for ARM targets.

> I am currently working on updating the patches for the other toolchain
> components, and will upstream them soon. This includes gcc, uclibc,
> and gdb.
>
> This series provides support for ARM v7 and later architectures and
> has been used to run the GCC tests on arm-linux-gnueabi without
> regression, as well as arm-linux-uclibceabi.
>
> Are the QEMU patches OK for inclusion in master?

Hi; I've given the patches a quick look over for structural
issues, though I haven't looked too closely at the details.
I think that given that the support is in Linux master I'm
happy with it also going into QEMU.

NB that Laurent has some patchsets outstanding which rework
the linux-user code to split the per-architecture parts of
signal.c and main.c into per-architecture files rather than
having lots of ifdefs. Those will probably land early in the
2.13 cycle. I mention this just as a heads-up that at some point
you'll find yourself with a somewhat painful rebase.

PS: I'd recommend cc'ing the linux-user maintainers (listed in
the MAINTAINERS file) on your next version of this patchset.

thanks
-- PMM

Re: [Qemu-devel] [ARM/FDPIC 0/4] FDPIC ABI for ARM

[Christophe Lyon]

On 13/04/2018 17:18, Peter Maydell wrote:
> On 6 April 2018 at 16:17, Christophe Lyon <christophe.lyon@st.com> wrote:
>> Hello,
>>
>> This patch series implements the QEMU contribution of the FDPIC
>> ABI for ARM targets.
> 
>> I am currently working on updating the patches for the other toolchain
>> components, and will upstream them soon. This includes gcc, uclibc,
>> and gdb.
>>
>> This series provides support for ARM v7 and later architectures and
>> has been used to run the GCC tests on arm-linux-gnueabi without
>> regression, as well as arm-linux-uclibceabi.
>>
>> Are the QEMU patches OK for inclusion in master?
> 
> Hi; I've given the patches a quick look over for structural
> issues, though I haven't looked too closely at the details.
> I think that given that the support is in Linux master I'm
> happy with it also going into QEMU.
> 
> NB that Laurent has some patchsets outstanding which rework
> the linux-user code to split the per-architecture parts of
> signal.c and main.c into per-architecture files rather than
> having lots of ifdefs. Those will probably land early in the
> 2.13 cycle. I mention this just as a heads-up that at some point
> you'll find yourself with a somewhat painful rebase.
> 

Thanks for the heads-up, I was unaware of that, and indeed I'd
rather avoid such a rebase.

> PS: I'd recommend cc'ing the linux-user maintainers (listed in
> the MAINTAINERS file) on your next version of this patchset.
> 
> thanks
> -- PMM
> .
>