Patchwork [6/8] linux-user: Enable NPTL for OpenRISC

login
register
mail settings
Submitter Peter Maydell
Date July 12, 2013, 8:12 p.m.
Message ID <1373659973-23289-7-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/258793/
State New
Headers show

Comments

Peter Maydell - July 12, 2013, 8:12 p.m.
The OpenRISC kernel ignores CLONE_SETTLS in its copy_thread()
implementation, so a cpu_set_tls() implementation is a no-op.
cpu_clone_regs() was setting the syscall return value in the
wrong register -- it is gpr[11], not gpr[2]. With these two
things fixed, we can compile with NPTL enabled.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure                        |    1 -
 linux-user/openrisc/target_cpu.h |    9 +++++++--
 2 files changed, 7 insertions(+), 3 deletions(-)
Jia Liu - July 13, 2013, 7:40 a.m.
Hi Peter,

On Sat, Jul 13, 2013 at 4:12 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> The OpenRISC kernel ignores CLONE_SETTLS in its copy_thread()
> implementation, so a cpu_set_tls() implementation is a no-op.
> cpu_clone_regs() was setting the syscall return value in the
> wrong register -- it is gpr[11], not gpr[2]. With these two
> things fixed, we can compile with NPTL enabled.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  configure                        |    1 -
>  linux-user/openrisc/target_cpu.h |    9 +++++++--
>  2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/configure b/configure
> index 4a241e0..3848c1c 100755
> --- a/configure
> +++ b/configure
> @@ -4229,7 +4229,6 @@ case "$target_name" in
>    or32)
>      TARGET_ARCH=openrisc
>      TARGET_BASE_ARCH=openrisc
> -    target_nptl="no"
>    ;;

I noticed configure have no this line.

>    ppc)
>      gdb_xml_files="power-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
> diff --git a/linux-user/openrisc/target_cpu.h b/linux-user/openrisc/target_cpu.h
> index 501fb81..32a46ac 100644
> --- a/linux-user/openrisc/target_cpu.h
> +++ b/linux-user/openrisc/target_cpu.h
> @@ -25,9 +25,14 @@ static inline void cpu_clone_regs(CPUOpenRISCState *env, target_ulong newsp)
>      if (newsp) {
>          env->gpr[1] = newsp;
>      }
> -    env->gpr[2] = 0;
> +    env->gpr[11] = 0;
>  }
>
> -/* TODO: need to implement cpu_set_tls() */
> +static inline void cpu_set_tls(CPUOpenRISCState *env, target_ulong newtls)
> +{
> +    /* Linux kernel 3.10 does not pay any attention to CLONE_SETTLS
> +     * in copy_thread(), so QEMU need not do so either.
> +     */
> +}

Thanks for fix. It looks good to me.

I need to reply here a Reviewed-by: Jia Liu <proljc@gmail.com> , yes?

>
>  #endif
> --
> 1.7.9.5
>

Regards,
Jia
Peter Maydell - July 13, 2013, 8:04 a.m.
On 13 July 2013 08:40, Jia Liu <proljc@gmail.com> wrote:
> Hi Peter,
>
> On Sat, Jul 13, 2013 at 4:12 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> The OpenRISC kernel ignores CLONE_SETTLS in its copy_thread()
>> implementation, so a cpu_set_tls() implementation is a no-op.
>> cpu_clone_regs() was setting the syscall return value in the
>> wrong register -- it is gpr[11], not gpr[2]. With these two
>> things fixed, we can compile with NPTL enabled.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  configure                        |    1 -
>>  linux-user/openrisc/target_cpu.h |    9 +++++++--
>>  2 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 4a241e0..3848c1c 100755
>> --- a/configure
>> +++ b/configure
>> @@ -4229,7 +4229,6 @@ case "$target_name" in
>>    or32)
>>      TARGET_ARCH=openrisc
>>      TARGET_BASE_ARCH=openrisc
>> -    target_nptl="no"
>>    ;;
>
> I noticed configure have no this line.

An earlier patch in the series rearranges this bit of
configure so targets have to say "no" (and the default
is "yes"), rather than the default being "no" and targets
having to say "yes".

>>    ppc)
>>      gdb_xml_files="power-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
>> diff --git a/linux-user/openrisc/target_cpu.h b/linux-user/openrisc/target_cpu.h
>> index 501fb81..32a46ac 100644
>> --- a/linux-user/openrisc/target_cpu.h
>> +++ b/linux-user/openrisc/target_cpu.h
>> @@ -25,9 +25,14 @@ static inline void cpu_clone_regs(CPUOpenRISCState *env, target_ulong newsp)
>>      if (newsp) {
>>          env->gpr[1] = newsp;
>>      }
>> -    env->gpr[2] = 0;
>> +    env->gpr[11] = 0;
>>  }
>>
>> -/* TODO: need to implement cpu_set_tls() */
>> +static inline void cpu_set_tls(CPUOpenRISCState *env, target_ulong newtls)
>> +{
>> +    /* Linux kernel 3.10 does not pay any attention to CLONE_SETTLS
>> +     * in copy_thread(), so QEMU need not do so either.
>> +     */
>> +}
>
> Thanks for fix. It looks good to me.
>
> I need to reply here a Reviewed-by: Jia Liu <proljc@gmail.com> , yes?

Yes.

thanks
-- PMM

Patch

diff --git a/configure b/configure
index 4a241e0..3848c1c 100755
--- a/configure
+++ b/configure
@@ -4229,7 +4229,6 @@  case "$target_name" in
   or32)
     TARGET_ARCH=openrisc
     TARGET_BASE_ARCH=openrisc
-    target_nptl="no"
   ;;
   ppc)
     gdb_xml_files="power-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
diff --git a/linux-user/openrisc/target_cpu.h b/linux-user/openrisc/target_cpu.h
index 501fb81..32a46ac 100644
--- a/linux-user/openrisc/target_cpu.h
+++ b/linux-user/openrisc/target_cpu.h
@@ -25,9 +25,14 @@  static inline void cpu_clone_regs(CPUOpenRISCState *env, target_ulong newsp)
     if (newsp) {
         env->gpr[1] = newsp;
     }
-    env->gpr[2] = 0;
+    env->gpr[11] = 0;
 }
 
-/* TODO: need to implement cpu_set_tls() */
+static inline void cpu_set_tls(CPUOpenRISCState *env, target_ulong newtls)
+{
+    /* Linux kernel 3.10 does not pay any attention to CLONE_SETTLS
+     * in copy_thread(), so QEMU need not do so either.
+     */
+}
 
 #endif