Re: [RFC PATCH v4 27/29] Hexagon (linux-user/hexagon) Linux user emulation

[Laurent Vivier]

Le 01/10/2020 à 17:40, Taylor Simpson a écrit :
> 
> 
>> -Original Message-
>> From: Laurent Vivier 
>> Sent: Thursday, October 1, 2020 1:53 AM
>> To: Taylor Simpson ; qemu-devel@nongnu.org
>> Cc: richard.hender...@linaro.org; phi...@redhat.com; riku.voi...@iki.fi;
>> aleksandar.m.m...@gmail.com; a...@rev.ng
>> Subject: Re: [RFC PATCH v4 27/29] Hexagon (linux-user/hexagon) Linux user
>> emulation
>>
>> Le 30/09/2020 à 22:47, Taylor Simpson a écrit :
>>>
>>>
>>>> -Original Message-
>>>> From: Laurent Vivier 
>>>> Sent: Monday, September 28, 2020 3:03 PM
>>>> To: Taylor Simpson ; qemu-devel@nongnu.org
>>>> Cc: richard.hender...@linaro.org; phi...@redhat.com; riku.voi...@iki.fi;
>>>> aleksandar.m.m...@gmail.com; a...@rev.ng
>>>> Subject: Re: [RFC PATCH v4 27/29] Hexagon (linux-user/hexagon) Linux
>> user
>>>> emulation
>>>>
>>>>> +
>>>>> +syscall_nr_generators += {
>>>>> +  'hexagon': generator(sh,
>>>>> +   arguments: [ 'cat', 'syscall_nr.h', '>', 
>>>>> '@OUTPUT@' ],
>>>>> + output: '@BASENAME@_nr.h')
>>>>
>>>> Why do we need that?
>>>> The syscall_nr_generators is used to generate syscall_nr.h from syscall.tbl
>>>
>>> The other linux-user targets have this in their meson.build files.  I'll 
>>> remove
>> if it's not needed.
>>
>> Not all the targets (only the ones with syscall.tbl) and they call
>> syscallhdr.sh and not cat. So you can remove it.
>>
>>>>
>>>>> +}
>>>>> diff --git a/linux-user/meson.build b/linux-user/meson.build
>>>>> index 2b94e4b..8b1dfc8 100644
>>>>> --- a/linux-user/meson.build
>>>>> +++ b/linux-user/meson.build
>>>>> @@ -22,6 +22,7 @@ syscall_nr_generators = {}
>>>>>
>>>>>  subdir('alpha')
>>>>>  subdir('arm')
>>>>> +subdir('hexagon')
>>>>
>>>> so you don't need that either
>>>
>>> OK
>>>
>>>>> -grep -e "#define __NR_" -e "#define __NR3264"
>>>>> +grep -e "#define __NR_" -e "#define __NR3264" | grep -v
>>>> __NR_syscalls
>>>>
>>>> Why do you remove __NR_syscalls?
>>>
>>> Older kernels have a bunch of
>>> #undef __NR_syscalls
>>> #define __NR_syscalls  X
>>>
>>> The script removes all #undef's but leaves the #define's, so we get compile
>> errors.  That symbol isn't used by qemu so it's safe to remove.
>>>
>>
>> As the syscall_nr.h generated from syscall.tbl don't have this symbol, I
>> think we can remove it. But I'm very surprised it doesn't work with it
>> for you.
>>
>> Could you put this one-line change in a separate patch?
>>
>> The best would be to re-run it once the change is made to update all the
>> generated syscall_nr.h without the __NR_syscalls line.
>>
>> Moreover the syscall_nr.h I generate from linux-5.9 for hexagon is
>> different from the one you put in this series (the 1024 -> 1079 part is
>> missing).
> 
> The syscall_nr.h I submitted is from 4.9, and we are in the process of 
> upgrading the toolchain/kernel/etc.

Do you know why the syscalls between 1024 and 1079 are in your
syscall_nr.h while they are not in linux 4.9?
(They are normally enabled by __ARCH_WANT_SYSCALL_DEPRECATED)

> 
> Which versions of the kernel does qemu expect to support?  I'll investigate 
> whether those versions have multiple __NR_syscalls.  If not, there's no need 
> for that change.

For linux-user, we have all the linux-user targets at the same syscall
API level, and we update them to have the definitions from the latest
kernel.

Thanks,
Laurent

RE: [RFC PATCH v4 27/29] Hexagon (linux-user/hexagon) Linux user emulation

[Taylor Simpson]

> -Original Message-
> From: Laurent Vivier 
> Sent: Thursday, October 1, 2020 1:53 AM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: richard.hender...@linaro.org; phi...@redhat.com; riku.voi...@iki.fi;
> aleksandar.m.m...@gmail.com; a...@rev.ng
> Subject: Re: [RFC PATCH v4 27/29] Hexagon (linux-user/hexagon) Linux user
> emulation
>
> Le 30/09/2020 à 22:47, Taylor Simpson a écrit :
> >
> >
> >> -Original Message-
> >> From: Laurent Vivier 
> >> Sent: Monday, September 28, 2020 3:03 PM
> >> To: Taylor Simpson ; qemu-devel@nongnu.org
> >> Cc: richard.hender...@linaro.org; phi...@redhat.com; riku.voi...@iki.fi;
> >> aleksandar.m.m...@gmail.com; a...@rev.ng
> >> Subject: Re: [RFC PATCH v4 27/29] Hexagon (linux-user/hexagon) Linux
> user
> >> emulation
> >>
> >>> +
> >>> +syscall_nr_generators += {
> >>> +  'hexagon': generator(sh,
> >>> +   arguments: [ 'cat', 'syscall_nr.h', '>', 
> >>> '@OUTPUT@' ],
> >>> + output: '@BASENAME@_nr.h')
> >>
> >> Why do we need that?
> >> The syscall_nr_generators is used to generate syscall_nr.h from syscall.tbl
> >
> > The other linux-user targets have this in their meson.build files.  I'll 
> > remove
> if it's not needed.
>
> Not all the targets (only the ones with syscall.tbl) and they call
> syscallhdr.sh and not cat. So you can remove it.
>
> >>
> >>> +}
> >>> diff --git a/linux-user/meson.build b/linux-user/meson.build
> >>> index 2b94e4b..8b1dfc8 100644
> >>> --- a/linux-user/meson.build
> >>> +++ b/linux-user/meson.build
> >>> @@ -22,6 +22,7 @@ syscall_nr_generators = {}
> >>>
> >>>  subdir('alpha')
> >>>  subdir('arm')
> >>> +subdir('hexagon')
> >>
> >> so you don't need that either
> >
> > OK
> >
> >>> -grep -e "#define __NR_" -e "#define __NR3264"
> >>> +grep -e "#define __NR_" -e "#define __NR3264" | grep -v
> >> __NR_syscalls
> >>
> >> Why do you remove __NR_syscalls?
> >
> > Older kernels have a bunch of
> > #undef __NR_syscalls
> > #define __NR_syscalls  X
> >
> > The script removes all #undef's but leaves the #define's, so we get compile
> errors.  That symbol isn't used by qemu so it's safe to remove.
> >
>
> As the syscall_nr.h generated from syscall.tbl don't have this symbol, I
> think we can remove it. But I'm very surprised it doesn't work with it
> for you.
>
> Could you put this one-line change in a separate patch?
>
> The best would be to re-run it once the change is made to update all the
> generated syscall_nr.h without the __NR_syscalls line.
>
> Moreover the syscall_nr.h I generate from linux-5.9 for hexagon is
> different from the one you put in this series (the 1024 -> 1079 part is
> missing).

The syscall_nr.h I submitted is from 4.9, and we are in the process of 
upgrading the toolchain/kernel/etc.

Which versions of the kernel does qemu expect to support?  I'll investigate 
whether those versions have multiple __NR_syscalls.  If not, there's no need 
for that change.

Taylor

Re: [RFC PATCH v4 27/29] Hexagon (linux-user/hexagon) Linux user emulation

[Laurent Vivier]

Le 30/09/2020 à 22:47, Taylor Simpson a écrit :
> 
> 
>> -Original Message-
>> From: Laurent Vivier 
>> Sent: Monday, September 28, 2020 3:03 PM
>> To: Taylor Simpson ; qemu-devel@nongnu.org
>> Cc: richard.hender...@linaro.org; phi...@redhat.com; riku.voi...@iki.fi;
>> aleksandar.m.m...@gmail.com; a...@rev.ng
>> Subject: Re: [RFC PATCH v4 27/29] Hexagon (linux-user/hexagon) Linux user
>> emulation
>>
>>> +
>>> +syscall_nr_generators += {
>>> +  'hexagon': generator(sh,
>>> +   arguments: [ 'cat', 'syscall_nr.h', '>', '@OUTPUT@' 
>>> ],
>>> + output: '@BASENAME@_nr.h')
>>
>> Why do we need that?
>> The syscall_nr_generators is used to generate syscall_nr.h from syscall.tbl
> 
> The other linux-user targets have this in their meson.build files.  I'll 
> remove if it's not needed.

Not all the targets (only the ones with syscall.tbl) and they call
syscallhdr.sh and not cat. So you can remove it.

>>
>>> +}
>>> diff --git a/linux-user/meson.build b/linux-user/meson.build
>>> index 2b94e4b..8b1dfc8 100644
>>> --- a/linux-user/meson.build
>>> +++ b/linux-user/meson.build
>>> @@ -22,6 +22,7 @@ syscall_nr_generators = {}
>>>
>>>  subdir('alpha')
>>>  subdir('arm')
>>> +subdir('hexagon')
>>
>> so you don't need that either
> 
> OK
> 
>>> -grep -e "#define __NR_" -e "#define __NR3264"
>>> +grep -e "#define __NR_" -e "#define __NR3264" | grep -v
>> __NR_syscalls
>>
>> Why do you remove __NR_syscalls?
> 
> Older kernels have a bunch of
> #undef __NR_syscalls
> #define __NR_syscalls  X
> 
> The script removes all #undef's but leaves the #define's, so we get compile 
> errors.  That symbol isn't used by qemu so it's safe to remove.
> 

As the syscall_nr.h generated from syscall.tbl don't have this symbol, I
think we can remove it. But I'm very surprised it doesn't work with it
for you.

Could you put this one-line change in a separate patch?

The best would be to re-run it once the change is made to update all the
generated syscall_nr.h without the __NR_syscalls line.

Moreover the syscall_nr.h I generate from linux-5.9 for hexagon is
different from the one you put in this series (the 1024 -> 1079 part is
missing).

Thanks,
Laurent

RE: [RFC PATCH v4 27/29] Hexagon (linux-user/hexagon) Linux user emulation

[Taylor Simpson]

> -Original Message-
> From: Laurent Vivier 
> Sent: Monday, September 28, 2020 3:03 PM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: richard.hender...@linaro.org; phi...@redhat.com; riku.voi...@iki.fi;
> aleksandar.m.m...@gmail.com; a...@rev.ng
> Subject: Re: [RFC PATCH v4 27/29] Hexagon (linux-user/hexagon) Linux user
> emulation
>
> > +
> > +syscall_nr_generators += {
> > +  'hexagon': generator(sh,
> > +   arguments: [ 'cat', 'syscall_nr.h', '>', '@OUTPUT@' 
> > ],
> > + output: '@BASENAME@_nr.h')
>
> Why do we need that?
> The syscall_nr_generators is used to generate syscall_nr.h from syscall.tbl

The other linux-user targets have this in their meson.build files.  I'll remove 
if it's not needed.

>
> > +}
> > diff --git a/linux-user/meson.build b/linux-user/meson.build
> > index 2b94e4b..8b1dfc8 100644
> > --- a/linux-user/meson.build
> > +++ b/linux-user/meson.build
> > @@ -22,6 +22,7 @@ syscall_nr_generators = {}
> >
> >  subdir('alpha')
> >  subdir('arm')
> > +subdir('hexagon')
>
> so you don't need that either

OK

> > -grep -e "#define __NR_" -e "#define __NR3264"
> > +grep -e "#define __NR_" -e "#define __NR3264" | grep -v
> __NR_syscalls
>
> Why do you remove __NR_syscalls?

Older kernels have a bunch of
#undef __NR_syscalls
#define __NR_syscalls  X

The script removes all #undef's but leaves the #define's, so we get compile 
errors.  That symbol isn't used by qemu so it's safe to remove.

Re: [RFC PATCH v4 27/29] Hexagon (linux-user/hexagon) Linux user emulation

[Laurent Vivier]

Le 28/09/2020 à 19:28, Taylor Simpson a écrit :
> Implementation of Linux user emulation for Hexagon
> Some common files modified in addition to new files in linux-user/hexagon
> 
> Signed-off-by: Taylor Simpson 
> Reviewed-by: Richard Henderson 
> ---
>  linux-user/hexagon/sockbits.h   |  18 ++
>  linux-user/hexagon/syscall_nr.h | 343 
> 
>  linux-user/hexagon/target_cpu.h |  44 +
>  linux-user/hexagon/target_elf.h |  40 +
>  linux-user/hexagon/target_fcntl.h   |  18 ++
>  linux-user/hexagon/target_signal.h  |  34 
>  linux-user/hexagon/target_structs.h |  46 +
>  linux-user/hexagon/target_syscall.h |  36 
>  linux-user/hexagon/termbits.h   |  18 ++
>  linux-user/qemu.h   |   2 +
>  linux-user/syscall_defs.h   |  33 
>  linux-user/elfload.c|  16 ++
>  linux-user/hexagon/cpu_loop.c   |  99 +++
>  linux-user/hexagon/signal.c | 276 +
>  linux-user/hexagon/meson.build  |  22 +++
>  linux-user/meson.build  |   1 +
>  scripts/gensyscalls.sh  |   3 +-
>  17 files changed, 1048 insertions(+), 1 deletion(-)
>  create mode 100644 linux-user/hexagon/sockbits.h
>  create mode 100644 linux-user/hexagon/syscall_nr.h
>  create mode 100644 linux-user/hexagon/target_cpu.h
>  create mode 100644 linux-user/hexagon/target_elf.h
>  create mode 100644 linux-user/hexagon/target_fcntl.h
>  create mode 100644 linux-user/hexagon/target_signal.h
>  create mode 100644 linux-user/hexagon/target_structs.h
>  create mode 100644 linux-user/hexagon/target_syscall.h
>  create mode 100644 linux-user/hexagon/termbits.h
>  create mode 100644 linux-user/hexagon/cpu_loop.c
>  create mode 100644 linux-user/hexagon/signal.c
>  create mode 100644 linux-user/hexagon/meson.build
> 
...
> diff --git a/linux-user/hexagon/meson.build b/linux-user/hexagon/meson.build
> new file mode 100644
> index 000..7158e52
> --- /dev/null
> +++ b/linux-user/hexagon/meson.build
> @@ -0,0 +1,22 @@
> +##
> +##  Copyright(c) 2020 Qualcomm Innovation Center, Inc. All Rights Reserved.
> +##
> +##  This program is free software; you can redistribute it and/or modify
> +##  it under the terms of the GNU General Public License as published by
> +##  the Free Software Foundation; either version 2 of the License, or
> +##  (at your option) any later version.
> +##
> +##  This program is distributed in the hope that it will be useful,
> +##  but WITHOUT ANY WARRANTY; without even the implied warranty of
> +##  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +##  GNU General Public License for more details.
> +##
> +##  You should have received a copy of the GNU General Public License
> +##  along with this program; if not, see .
> +##
> +
> +syscall_nr_generators += {
> +  'hexagon': generator(sh,
> +   arguments: [ 'cat', 'syscall_nr.h', '>', '@OUTPUT@' ],
> + output: '@BASENAME@_nr.h')

Why do we need that?
The syscall_nr_generators is used to generate syscall_nr.h from syscall.tbl

> +}
> diff --git a/linux-user/meson.build b/linux-user/meson.build
> index 2b94e4b..8b1dfc8 100644
> --- a/linux-user/meson.build
> +++ b/linux-user/meson.build
> @@ -22,6 +22,7 @@ syscall_nr_generators = {}
>  
>  subdir('alpha')
>  subdir('arm')
> +subdir('hexagon')

so you don't need that either

>  subdir('hppa')
>  subdir('i386')
>  subdir('m68k')
> diff --git a/scripts/gensyscalls.sh b/scripts/gensyscalls.sh
> index b7b8456..e2ee60a 100755
> --- a/scripts/gensyscalls.sh
> +++ b/scripts/gensyscalls.sh
> @@ -53,7 +53,7 @@ read_includes()
>  
>  filter_defines()
>  {
> -grep -e "#define __NR_" -e "#define __NR3264"
> +grep -e "#define __NR_" -e "#define __NR3264" | grep -v __NR_syscalls

Why do you remove __NR_syscalls?

>  }
>  
>  rename_defines()
> @@ -99,4 +99,5 @@ generate_syscall_nr openrisc 32 
> "$output/linux-user/openrisc/syscall_nr.h"
>  
>  generate_syscall_nr riscv 32 "$output/linux-user/riscv/syscall32_nr.h"
>  generate_syscall_nr riscv 64 "$output/linux-user/riscv/syscall64_nr.h"
> +generate_syscall_nr hexagon 32 "$output/linux-user/hexagon/syscall_nr.h"

Please, move this line to the previous block (that generates
syscall_nr.h and not syscallXX_nr.h)

>  rm -fr "$TMP"
> 

Thanks,
Laurent