MIPS: bug: gettimeofday syscall broken on CI20 board?

MIPS: bug: gettimeofday syscall broken on CI20 board?

[H. Nikolaus Schaller]

Hi,
I am trying to run v5.4-rc6 on the CI20 board (jz4780) and it
is almost ok. Except one strange thing.

If I install a v4.19.81 kernel I can initialize the
ethernet interface and dhclient works.

If I install a v5.4-rc6 kernel on exactly the same
rootfs dhclient fails with

root@letux:~# dhclient
../../../../lib/isc/unix/time.c:200: Operation not permitted
root@letux:~#

I have done some strace and the first significant difference
is that with v5.4-rc6 there is no gettimeofday syscall.

Another symptom pointing in the same direction is that
after manually assigning an IP address I can run ping
but get strange time values.

So it may be that

24640f233b46 mips: Add support for generic vDSO

did break gettimeofday when used with latest Debian Stretch
libraries. I tried to git revert but there are conflicts.

Just a side-note: both kernels work with Debian Jessie,
which likely has an older gettimeofday wrapper that
is not influenced by some subtle change.

BR and thanks,
Nikolaus Schaller

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[H. Nikolaus Schaller]

Hi Vincenzo,

> Am 07.11.2019 um 17:21 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
> Hi,
> I am trying to run v5.4-rc6 on the CI20 board (jz4780) and it
> is almost ok. Except one strange thing.
> 
> If I install a v4.19.81 kernel I can initialize the
> ethernet interface and dhclient works.
> 
> If I install a v5.4-rc6 kernel on exactly the same
> rootfs dhclient fails with
> 
> root@letux:~# dhclient
> ../../../../lib/isc/unix/time.c:200: Operation not permitted
> root@letux:~#
> 
> I have done some strace and the first significant difference
> is that with v5.4-rc6 there is no gettimeofday syscall.
> 
> Another symptom pointing in the same direction is that
> after manually assigning an IP address I can run ping
> but get strange time values.
> 
> So it may be that
> 
> 24640f233b46 mips: Add support for generic vDSO
> 
> did break gettimeofday when used with latest Debian Stretch
> libraries. I tried to git revert but there are conflicts.
> 
> Just a side-note: both kernels work with Debian Jessie,
> which likely has an older gettimeofday wrapper that
> is not influenced by some subtle change.

I finally found time to do a bisect and it confirms:

24640f233b466051ad3a5d2786d2951e43026c9d is the first bad commit
commit 24640f233b466051ad3a5d2786d2951e43026c9d
Author: Vincenzo Frascino <vincenzo.frascino@arm.com>
Date:   Fri Jun 21 10:52:46 2019 +0100

    mips: Add support for generic vDSO
    
    The mips vDSO library requires some adaptations to take advantage of the
    newly introduced generic vDSO library.
    
    Introduce the following changes:
     - Modification of vdso.c to be compliant with the common vdso datapage
     - Use of lib/vdso for gettimeofday
    
    Cc: Ralf Baechle <ralf@linux-mips.org>
    Cc: Paul Burton <paul.burton@mips.com>
    Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
    [paul.burton@mips.com: Prepend $(src) to config-n32-o32-env.c path.]
    Signed-off-by: Paul Burton <paul.burton@mips.com>

 arch/mips/Kconfig                         |   2 +
 arch/mips/include/asm/vdso.h              |  78 +--------------
 arch/mips/include/asm/vdso/gettimeofday.h | 151 ++++++++++++++++++++++++++++++
 arch/mips/include/asm/vdso/vdso.h         |  85 +++++++++++++++++
 arch/mips/include/asm/vdso/vsyscall.h     |  43 +++++++++
 arch/mips/kernel/vdso.c                   |  37 ++------
 arch/mips/vdso/Makefile                   |  33 ++++++-
 arch/mips/vdso/config-n32-o32-env.c       |  17 ++++
 arch/mips/vdso/elf.S                      |   2 +-
 arch/mips/vdso/sigreturn.S                |   2 +-
 arch/mips/vdso/vdso.h                     |  85 -----------------
 arch/mips/vdso/vgettimeofday.c            |  40 ++++++++
 12 files changed, 378 insertions(+), 197 deletions(-)
 create mode 100644 arch/mips/include/asm/vdso/gettimeofday.h
 create mode 100644 arch/mips/include/asm/vdso/vdso.h
 create mode 100644 arch/mips/include/asm/vdso/vsyscall.h
 create mode 100644 arch/mips/vdso/config-n32-o32-env.c
 delete mode 100644 arch/mips/vdso/vdso.h
 create mode 100644 arch/mips/vdso/vgettimeofday.c

So this patch intoruced in v5.4-rc1 breaks compatibility with Debian 9.9
user space assumptions.

One thing seems strange to me:

-/**
- * union mips_vdso_data - Data provided by the kernel for the VDSO.
- * @xtime_sec:         Current real time (seconds part).
- * @xtime_nsec:                Current real time (nanoseconds part, shifted).
- * @wall_to_mono_sec:  Wall-to-monotonic offset (seconds part).
- * @wall_to_mono_nsec: Wall-to-monotonic offset (nanoseconds part).
- * @seq_count:         Counter to synchronise updates (odd = updating).
- * @cs_shift:          Clocksource shift value.
- * @clock_mode:                Clocksource to use for time functions.
- * @cs_mult:           Clocksource multiplier value.
- * @cs_cycle_last:     Clock cycle value at last update.
- * @cs_mask:           Clocksource mask value.
- * @tz_minuteswest:    Minutes west of Greenwich (from timezone).
- * @tz_dsttime:                Type of DST correction (from timezone).
- *
- * This structure contains data needed by functions within the VDSO. It is
- * populated by the kernel and mapped read-only into user memory. The time
- * fields are mirrors of internal data from the timekeeping infrastructure.
- *
- * Note: Care should be taken when modifying as the layout must remain the same
- * for both 64- and 32-bit (for 32-bit userland on 64-bit kernel).
- */
 union mips_vdso_data {
-       struct {
-               u64 xtime_sec;
-               u64 xtime_nsec;
-               u64 wall_to_mono_sec;
-               u64 wall_to_mono_nsec;
-               u32 seq_count;
-               u32 cs_shift;
-               u8 clock_mode;
-               u32 cs_mult;
-               u64 cs_cycle_last;
-               u64 cs_mask;
-               s32 tz_minuteswest;
-               s32 tz_dsttime;
-       };
-
+       struct vdso_data data[CS_BASES];
        u8 page[PAGE_SIZE];
 };

If I look at the definition of vdso_data it *is* significantly differen
from mips_vdso_data.

What I would assume is that the struct mips_vdso_data is embossed in user
space code and therefore using vdso_data instead is breaking API.

Please advise what I should try or check to narrow down further.

BR and thanks,
Nikolaus Schaller

Re:MIPS: bug: gettimeofday syscall broken on CI20 board

[H. Nikolaus Schaller]

Nobody with an idea?

> Am 17.11.2019 um 14:14 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
> Hi Vincenzo,
> 
>> Am 07.11.2019 um 17:21 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
>> 
>> Hi,
>> I am trying to run v5.4-rc6 on the CI20 board (jz4780) and it
>> is almost ok. Except one strange thing.
>> 
>> If I install a v4.19.81 kernel I can initialize the
>> ethernet interface and dhclient works.
>> 
>> If I install a v5.4-rc6 kernel on exactly the same
>> rootfs dhclient fails with
>> 
>> root@letux:~# dhclient
>> ../../../../lib/isc/unix/time.c:200: Operation not permitted
>> root@letux:~#
>> 
>> I have done some strace and the first significant difference
>> is that with v5.4-rc6 there is no gettimeofday syscall.
>> 
>> Another symptom pointing in the same direction is that
>> after manually assigning an IP address I can run ping
>> but get strange time values.
>> 
>> So it may be that
>> 
>> 24640f233b46 mips: Add support for generic vDSO
>> 
>> did break gettimeofday when used with latest Debian Stretch
>> libraries. I tried to git revert but there are conflicts.
>> 
>> Just a side-note: both kernels work with Debian Jessie,
>> which likely has an older gettimeofday wrapper that
>> is not influenced by some subtle change.
> 
> I finally found time to do a bisect and it confirms:
> 
> 24640f233b466051ad3a5d2786d2951e43026c9d is the first bad commit
> commit 24640f233b466051ad3a5d2786d2951e43026c9d
> Author: Vincenzo Frascino <vincenzo.frascino@arm.com>
> Date:   Fri Jun 21 10:52:46 2019 +0100
> 
>    mips: Add support for generic vDSO
> 
>    The mips vDSO library requires some adaptations to take advantage of the
>    newly introduced generic vDSO library.
> 
>    Introduce the following changes:
>     - Modification of vdso.c to be compliant with the common vdso datapage
>     - Use of lib/vdso for gettimeofday
> 
>    Cc: Ralf Baechle <ralf@linux-mips.org>
>    Cc: Paul Burton <paul.burton@mips.com>
>    Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>    [paul.burton@mips.com: Prepend $(src) to config-n32-o32-env.c path.]
>    Signed-off-by: Paul Burton <paul.burton@mips.com>
> 
> arch/mips/Kconfig                         |   2 +
> arch/mips/include/asm/vdso.h              |  78 +--------------
> arch/mips/include/asm/vdso/gettimeofday.h | 151 ++++++++++++++++++++++++++++++
> arch/mips/include/asm/vdso/vdso.h         |  85 +++++++++++++++++
> arch/mips/include/asm/vdso/vsyscall.h     |  43 +++++++++
> arch/mips/kernel/vdso.c                   |  37 ++------
> arch/mips/vdso/Makefile                   |  33 ++++++-
> arch/mips/vdso/config-n32-o32-env.c       |  17 ++++
> arch/mips/vdso/elf.S                      |   2 +-
> arch/mips/vdso/sigreturn.S                |   2 +-
> arch/mips/vdso/vdso.h                     |  85 -----------------
> arch/mips/vdso/vgettimeofday.c            |  40 ++++++++
> 12 files changed, 378 insertions(+), 197 deletions(-)
> create mode 100644 arch/mips/include/asm/vdso/gettimeofday.h
> create mode 100644 arch/mips/include/asm/vdso/vdso.h
> create mode 100644 arch/mips/include/asm/vdso/vsyscall.h
> create mode 100644 arch/mips/vdso/config-n32-o32-env.c
> delete mode 100644 arch/mips/vdso/vdso.h
> create mode 100644 arch/mips/vdso/vgettimeofday.c
> 
> So this patch intoruced in v5.4-rc1 breaks compatibility with Debian 9.9
> user space assumptions.
> 
> One thing seems strange to me:
> 
> -/**
> - * union mips_vdso_data - Data provided by the kernel for the VDSO.
> - * @xtime_sec:         Current real time (seconds part).
> - * @xtime_nsec:                Current real time (nanoseconds part, shifted).
> - * @wall_to_mono_sec:  Wall-to-monotonic offset (seconds part).
> - * @wall_to_mono_nsec: Wall-to-monotonic offset (nanoseconds part).
> - * @seq_count:         Counter to synchronise updates (odd = updating).
> - * @cs_shift:          Clocksource shift value.
> - * @clock_mode:                Clocksource to use for time functions.
> - * @cs_mult:           Clocksource multiplier value.
> - * @cs_cycle_last:     Clock cycle value at last update.
> - * @cs_mask:           Clocksource mask value.
> - * @tz_minuteswest:    Minutes west of Greenwich (from timezone).
> - * @tz_dsttime:                Type of DST correction (from timezone).
> - *
> - * This structure contains data needed by functions within the VDSO. It is
> - * populated by the kernel and mapped read-only into user memory. The time
> - * fields are mirrors of internal data from the timekeeping infrastructure.
> - *
> - * Note: Care should be taken when modifying as the layout must remain the same
> - * for both 64- and 32-bit (for 32-bit userland on 64-bit kernel).
> - */
> union mips_vdso_data {
> -       struct {
> -               u64 xtime_sec;
> -               u64 xtime_nsec;
> -               u64 wall_to_mono_sec;
> -               u64 wall_to_mono_nsec;
> -               u32 seq_count;
> -               u32 cs_shift;
> -               u8 clock_mode;
> -               u32 cs_mult;
> -               u64 cs_cycle_last;
> -               u64 cs_mask;
> -               s32 tz_minuteswest;
> -               s32 tz_dsttime;
> -       };
> -
> +       struct vdso_data data[CS_BASES];
>        u8 page[PAGE_SIZE];
> };
> 
> If I look at the definition of vdso_data it *is* significantly differen
> from mips_vdso_data.
> 
> What I would assume is that the struct mips_vdso_data is embossed in user
> space code and therefore using vdso_data instead is breaking API.
> 
> Please advise what I should try or check to narrow down further.
> 
> BR and thanks,
> Nikolaus Schaller
> 
> _______________________________________________
> http://projects.goldelico.com/p/gta04-kernel/
> Letux-kernel mailing list
> Letux-kernel@openphoenux.org
> http://lists.goldelico.com/mailman/listinfo.cgi/letux-kernel

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[Vincenzo Frascino]

[-- Attachment #1: Type: text/plain, Size: 1461 bytes --]

Hi Nikolaus,

sorry for the delay in answering to your email but due to personal reasons I had
to pull off the linux development for few weeks.

On 17/11/2019 13:14, H. Nikolaus Schaller wrote:
> Hi Vincenzo,
> 

[...]

> 
> If I look at the definition of vdso_data it *is* significantly differen
> from mips_vdso_data.
> 
> What I would assume is that the struct mips_vdso_data is embossed in user
> space code and therefore using vdso_data instead is breaking API.
>

vdso_data and mips_vdso_data before are not part of the ABI hence they are not
bind by a contract with the userspace.

This means that they can change at any point and if a userspace software relies
on a specific layout of these data structures is doing something wrong.

> Please advise what I should try or check to narrow down further.
> 

I had a look at [1] line 200 and seems that the error you are seeing is
generated by:
  if (gettimeofday(&tv, NULL) == -1) { ... }

I do not have a CI20 hence I can't do the test myself: could you please write a
small application that invokes gettimeofday() as per above and report the
behavior (I am even interested in the value returned). If we can reproduce the
problem in a smaller environment it is easier to debug and get to the solution.

[1] http://users.isc.org/~each/doxygen/bind9/isc_2unix_2time_8c-source.html

> BR and thanks,
> Nikolaus Schaller
> 

Let me know.

-- 
Regards,
Vincenzo

[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 14291 bytes --]

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[H. Nikolaus Schaller]

Hi Vincenzo,

> Am 26.11.2019 um 11:52 schrieb Vincenzo Frascino <vincenzo.frascino@arm.com>:
> 
> Hi Nikolaus,
> 
> sorry for the delay in answering to your email but due to personal reasons I had
> to pull off the linux development for few weeks.

No problem.

> 
> On 17/11/2019 13:14, H. Nikolaus Schaller wrote:
>> Hi Vincenzo,
>> 
> 
> [...]
> 
>> 
>> If I look at the definition of vdso_data it *is* significantly differen
>> from mips_vdso_data.
>> 
>> What I would assume is that the struct mips_vdso_data is embossed in user
>> space code and therefore using vdso_data instead is breaking API.
>> 
> 
> vdso_data and mips_vdso_data before are not part of the ABI hence they are not
> bind by a contract with the userspace.
> 
> This means that they can change at any point and if a userspace software relies
> on a specific layout of these data structures is doing something wrong.

Maybe the libs are clever enough to find that out dynamically but I have no
idea about how gettimeofday() and user-space VDSO is implemented to handle such
changes.

> 
>> Please advise what I should try or check to narrow down further.
>> 
> 
> I had a look at [1] line 200 and seems that the error you are seeing is
> generated by:
>  if (gettimeofday(&tv, NULL) == -1) { ... }

Yes.

> I do not have a CI20 hence I can't do the test myself: could you please write a
> small application that invokes gettimeofday() as per above and report the
> behavior (I am even interested in the value returned). If we can reproduce the
> problem in a smaller environment it is easier to debug and get to the solution.
> 
> [1] http://users.isc.org/~each/doxygen/bind9/isc_2unix_2time_8c-source.html
> 
>> BR and thanks,
>> Nikolaus Schaller
>> 
> 
> Let me know.

I have done this and it seems as if tv_usec reports something that is beyond 1e6 us
or remains unchanged by the syscall. tv_sec seems to be set correctly. And,
gettimeofday() reports -1.

hwclock isn't set on 45.4 kernel because I have no ethernet connection due to the bug.

BR,
Nikolaus


Here is the log

a) with 5.4 kernel

root@letux:~# cat gettime.c 
#include <stdio.h>
#include <time.h>
#include <sys/time.h>

int main(void)
{
struct timeval tv;
int r = gettimeofday(&tv, NULL);
time_t t;
int rt = time(&t);

printf("r = %d\n", r);
printf("tv.sec = %ld\n", tv.tv_sec);
printf("tv.usec = %d\n", tv.tv_usec);
printf("rt = %d\n", rt);
printf("t = %ld\n", t);
}
root@letux:~# make gettime
cc     gettime.c   -o gettime
root@letux:~# ./gettime 
r = -1
tv.sec = 1431857456
tv.usec = 2012065500
rt = 1478206565
t = 1478206565
root@letux:~# ./gettime 
r = -1
tv.sec = 1431873840
tv.usec = 2012065500
rt = 1478206573
t = 1478206573
root@letux:~# uname -a
Linux letux 5.4.0-letux-l400+ #1485 PREEMPT Wed Nov 27 10:23:16 CET 2019 mips GNU/Linux
root@letux:~# cat /proc/cpuinfo 
system type             : JZ4780
machine                 : img,ci20
processor               : 0
cpu model               : Ingenic JZRISC V4.15  FPU V0.0
BogoMIPS                : 1196.85
wait instruction        : yes
microsecond timers      : no
tlb_entries             : 32
extra interrupt vector  : yes
hardware watchpoint     : yes, count: 1, address/irw mask: [0x0fff]
isa                     : mips1 mips2 mips32r1 mips32r2
ASEs implemented        :
shadow register sets    : 1
kscratch registers      : 0
package                 : 0
core                    : 0
VCED exceptions         : not available
VCEI exceptions         : not available

root@letux:~# dpkg -l | grep libc6
ii  libc6:mipsel                   2.24-11+deb9u4                 mipsel       GNU C Library: Shared libraries
ii  libc6-dev:mipsel               2.24-11+deb9u4                 mipsel       GNU C Library: Development Libraries and Header Files
root@letux:~# cat /etc/debian_version 
9.11
root@letux:~# 

b) same system booted with 4.19 kernel:

root@letux:~# ./gettime 
r = 0
tv.sec = 1574862135
tv.usec = 27974
rt = 1574862135
t = 1574862135
root@letux:~# uname -a
Linux letux 4.19.86-letux-l400+ #1450 PREEMPT Sun Nov 24 17:17:19 CET 2019 mips GNU/Linux
root@letux:~# cat /proc/cpuinfo 
system type             : JZ4780
machine                 : img,ci20
processor               : 0
cpu model               : Ingenic JZRISC V4.15  FPU V0.0
BogoMIPS                : 1196.85
wait instruction        : yes
microsecond timers      : no
tlb_entries             : 32
extra interrupt vector  : yes
hardware watchpoint     : yes, count: 1, address/irw mask: [0x0fff]
isa                     : mips1 mips2 mips32r1 mips32r2
ASEs implemented        :
shadow register sets    : 1
kscratch registers      : 0
package                 : 0
core                    : 0
VCED exceptions         : not available
VCEI exceptions         : not available

root@letux:~# cat /etc/debian_version 
9.11
root@letux:~# 

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[H. Nikolaus Schaller]

Hi Vincenco,

> Am 27.11.2019 um 14:53 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
> Hi Vincenzo,
> 
>> Am 26.11.2019 um 11:52 schrieb Vincenzo Frascino <vincenzo.frascino@arm.com>:
>> 
>> I do not have a CI20 hence I can't do the test myself: could you please write a
>> small application that invokes gettimeofday() as per above and report the
>> behavior (I am even interested in the value returned). If we can reproduce the
>> problem in a smaller environment it is easier to debug and get to the solution.
>> 
>> [1] http://users.isc.org/~each/doxygen/bind9/isc_2unix_2time_8c-source.html
>> 
>> Let me know.
> 
> I have done this and it seems as if tv_usec reports something that is beyond 1e6 us
> or remains unchanged by the syscall. tv_sec seems to be set correctly. And,
> gettimeofday() reports -1.
> 
> hwclock isn't set on 45.4 kernel because I have no ethernet connection due to the bug.
> 
> BR,
> Nikolaus
> 
> 
> Here is the log
> 
> a) with 5.4 kernel
> 
> root@letux:~# cat gettime.c 
> #include <stdio.h>
> #include <time.h>
> #include <sys/time.h>
> 
> int main(void)
> {
> struct timeval tv;
> int r = gettimeofday(&tv, NULL);
> time_t t;
> int rt = time(&t);
> 
> printf("r = %d\n", r);
> printf("tv.sec = %ld\n", tv.tv_sec);
> printf("tv.usec = %d\n", tv.tv_usec);
> printf("rt = %d\n", rt);
> printf("t = %ld\n", t);
> }

I realized that this does not allow to distinguish between
uninitialized struct timeval tv and return values. And it
would be nice to print errno. So I have used a new version:

#include <stdio.h>
#include <time.h>
#include <sys/time.h>
#include <errno.h>

int main(void)
{
	struct timeval tv = { 1, 1 };
	int r;
	time_t t;
	int rt;

	r = gettimeofday(&tv, NULL);
	printf("r = %d errno=%d\n", r, errno);
	printf("tv.sec = %ld\n", tv.tv_sec);
	printf("tv.usec = %d\n", tv.tv_usec);

	rt = time(&t);
	printf("rt = %d errno=%d\n", rt, errno);
	printf("t = %ld\n", t);
}

> root@letux:~# make gettime
> cc     gettime.c   -o gettime
> root@letux:~# ./gettime 
> r = -1
> tv.sec = 1431857456
> tv.usec = 2012065500
> rt = 1478206565
> t = 1478206565

root@letux:~# ./gettime 
r = -1 errno=1
tv.sec = 1
tv.usec = 1
rt = 1478193516 errno=1
t = 1478193516
root@letux:~# 

which means that there is an -EPERM and &tv is not touched.
Well, I did run this as root.

So it may be a completely different reason than changing the
VDSO structure.

> b) same system booted with 4.19 kernel:
> 
> root@letux:~# ./gettime 
> r = 0
> tv.sec = 1574862135
> tv.usec = 27974
> rt = 1574862135
> t = 1574862135

root@letux:~# ./gettime 
r = 0 errno=0
tv.sec = 1574863040
tv.usec = 874858
rt = 1574863040 errno=0
t = 1574863040
root@letux:~#


> root@letux:~# uname -a
> Linux letux 4.19.86-letux-l400+ #1450 PREEMPT Sun Nov 24 17:17:19 CET 2019 mips GNU/Linux

Please let me know what I should try next.

BR and thanks,
Nikolaus

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[Vincenzo Frascino]

[-- Attachment #1: Type: text/plain, Size: 858 bytes --]

Hi Nikolaus,

On 27/11/2019 13:53, H. Nikolaus Schaller wrote:
[...]

>> vdso_data and mips_vdso_data before are not part of the ABI hence they are not
>> bind by a contract with the userspace.
>>
>> This means that they can change at any point and if a userspace software relies
>> on a specific layout of these data structures is doing something wrong.
> 
> Maybe the libs are clever enough to find that out dynamically but I have no
> idea about how gettimeofday() and user-space VDSO is implemented to handle such
> changes.
> 
As I said userspace applications and libraries should not rely on the layout of
vdso_data because this is not part of the ABI.

The only thing that userspace requires is to "know" that gettimeofday() exists,
than it is gettimeofday() that internally accesses the data structure.

-- 
Regards,
Vincenzo

[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 14291 bytes --]

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[Vincenzo Frascino]

[-- Attachment #1: Type: text/plain, Size: 1392 bytes --]

Hi Nikolaus,

On 27/11/2019 14:29, H. Nikolaus Schaller wrote:
> Hi Vincenco,
> 
[...]

>>
>> a) with 5.4 kernel
>>

Could you please report the hash of your top commit?

[...]

> 
> root@letux:~# ./gettime 
> r = -1 errno=1
> tv.sec = 1
> tv.usec = 1
> rt = 1478193516 errno=1
> t = 1478193516
> root@letux:~# 
> 

On arm64 the output of your application is what follows:

r = 0 errno=0
tv.sec = 1574942449
tv.usec = 348684
rt = 1574942449 errno=0
t = 1574942449

Which seems what it is expected.

On a previous email you mentioned what follows:

root@letux:~# cat /proc/cpuinfo
system type             : JZ4780
machine                 : img,ci20
processor               : 0
cpu model               : Ingenic JZRISC V4.15  FPU V0.0
BogoMIPS                : 1196.85
wait instruction        : yes
microsecond timers      : no
tlb_entries             : 32
extra interrupt vector  : yes
hardware watchpoint     : yes, count: 1, address/irw mask: [0x0fff]
isa                     : mips1 mips2 mips32r1 mips32r2
ASEs implemented        :
shadow register sets    : 1
kscratch registers      : 0
package                 : 0
core                    : 0
VCED exceptions         : not available
VCEI exceptions         : not available

I am not familiar with JZ4780, which HW clock is it using? (R4K, GIC or NONE)?

-- 
Regards,
Vincenzo

[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 14291 bytes --]

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[H. Nikolaus Schaller]

> Am 28.11.2019 um 12:51 schrieb Vincenzo Frascino <vincenzo.frascino@arm.com>:
> 
> Hi Nikolaus,
> 
> On 27/11/2019 13:53, H. Nikolaus Schaller wrote:
> [...]
> 
>>> vdso_data and mips_vdso_data before are not part of the ABI hence they are not
>>> bind by a contract with the userspace.
>>> 
>>> This means that they can change at any point and if a userspace software relies
>>> on a specific layout of these data structures is doing something wrong.
>> 
>> Maybe the libs are clever enough to find that out dynamically but I have no
>> idea about how gettimeofday() and user-space VDSO is implemented to handle such
>> changes.
>> 
> As I said userspace applications and libraries should not rely on the layout of
> vdso_data because this is not part of the ABI.
> 
> The only thing that userspace requires is to "know" that gettimeofday() exists,
> than it is gettimeofday() that internally accesses the data structure.

Well, with user-space I include the lib that provides the gettimeofday() syscall
and reads out the memory region where the VDSO data structure is provided by the
kernel. And that part comes from Debian. Somehow it does differently with 4.19
than 5.4. So I summarise all non-kernel code with the term "user-space".

BR,
Nikolaus

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[H. Nikolaus Schaller]

> Am 28.11.2019 um 13:05 schrieb Vincenzo Frascino <vincenzo.frascino@arm.com>:
> 
> Hi Nikolaus,
> 
> On 27/11/2019 14:29, H. Nikolaus Schaller wrote:
>> Hi Vincenco,
>> 
> [...]
> 
>>> 
>>> a) with 5.4 kernel
>>> 
> 
> Could you please report the hash of your top commit?

We have our own config and some compile fixes. But the base
commit is 219d54332a09

> 
> [...]
> 
>> 
>> root@letux:~# ./gettime 
>> r = -1 errno=1
>> tv.sec = 1
>> tv.usec = 1
>> rt = 1478193516 errno=1
>> t = 1478193516
>> root@letux:~# 
>> 
> 
> On arm64 the output of your application is what follows:
> 
> r = 0 errno=0
> tv.sec = 1574942449
> tv.usec = 348684
> rt = 1574942449 errno=0
> t = 1574942449
> 
> Which seems what it is expected.

Indeed. Well, on all other arm architectures I can test (omap3/4/5, imx6, raspi)
it is correct. Only on mips jz4780 I see this.

> 
> On a previous email you mentioned what follows:
> 
> root@letux:~# cat /proc/cpuinfo
> system type             : JZ4780
> machine                 : img,ci20
> processor               : 0
> cpu model               : Ingenic JZRISC V4.15  FPU V0.0
> BogoMIPS                : 1196.85
> wait instruction        : yes
> microsecond timers      : no
> tlb_entries             : 32
> extra interrupt vector  : yes
> hardware watchpoint     : yes, count: 1, address/irw mask: [0x0fff]
> isa                     : mips1 mips2 mips32r1 mips32r2
> ASEs implemented        :
> shadow register sets    : 1
> kscratch registers      : 0
> package                 : 0
> core                    : 0
> VCED exceptions         : not available
> VCEI exceptions         : not available
> 
> I am not familiar with JZ4780, which HW clock is it using? (R4K, GIC or NONE)?

Ah, good question. How do I find out? Checking defconfig did not show any
config with R4K or GIC. So it is likely NONE. CONFIG_RTC_DRV_JZ4740=y if that
helps.

BR and thanks,
Nikolaus

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[Vincenzo Frascino]

[-- Attachment #1: Type: text/plain, Size: 1541 bytes --]

On 28/11/2019 12:11, H. Nikolaus Schaller wrote:
> 
>> Am 28.11.2019 um 12:51 schrieb Vincenzo Frascino <vincenzo.frascino@arm.com>:
>>
>> Hi Nikolaus,
>>
>> On 27/11/2019 13:53, H. Nikolaus Schaller wrote:
>> [...]
>>
>>>> vdso_data and mips_vdso_data before are not part of the ABI hence they are not
>>>> bind by a contract with the userspace.
>>>>
>>>> This means that they can change at any point and if a userspace software relies
>>>> on a specific layout of these data structures is doing something wrong.
>>>
>>> Maybe the libs are clever enough to find that out dynamically but I have no
>>> idea about how gettimeofday() and user-space VDSO is implemented to handle such
>>> changes.
>>>
>> As I said userspace applications and libraries should not rely on the layout of
>> vdso_data because this is not part of the ABI.
>>
>> The only thing that userspace requires is to "know" that gettimeofday() exists,
>> than it is gettimeofday() that internally accesses the data structure.
> 
> Well, with user-space I include the lib that provides the gettimeofday() syscall
> and reads out the memory region where the VDSO data structure is provided by the
> kernel. And that part comes from Debian. Somehow it does differently with 4.19
> than 5.4. So I summarise all non-kernel code with the term "user-space".
> 

The the lib that provides the gettimeofday() changes accordingly with vdso_data.
5.4 and 4.19 have 2 different vdso libraries as well.

> BR,
> Nikolaus
> 

-- 
Regards,
Vincenzo

[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 14291 bytes --]

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[H. Nikolaus Schaller]

Hi Vincenzo,

> Am 28.11.2019 um 13:21 schrieb Vincenzo Frascino <vincenzo.frascino@arm.com>:
> 
> On 28/11/2019 12:11, H. Nikolaus Schaller wrote:
>> 
>>> Am 28.11.2019 um 12:51 schrieb Vincenzo Frascino <vincenzo.frascino@arm.com>:
>>> 
>>> Hi Nikolaus,
>>> 
>>> On 27/11/2019 13:53, H. Nikolaus Schaller wrote:
>>> [...]
>>> 
>>>>> vdso_data and mips_vdso_data before are not part of the ABI hence they are not
>>>>> bind by a contract with the userspace.
>>>>> 
>>>>> This means that they can change at any point and if a userspace software relies
>>>>> on a specific layout of these data structures is doing something wrong.
>>>> 
>>>> Maybe the libs are clever enough to find that out dynamically but I have no
>>>> idea about how gettimeofday() and user-space VDSO is implemented to handle such
>>>> changes.
>>>> 
>>> As I said userspace applications and libraries should not rely on the layout of
>>> vdso_data because this is not part of the ABI.
>>> 
>>> The only thing that userspace requires is to "know" that gettimeofday() exists,
>>> than it is gettimeofday() that internally accesses the data structure.
>> 
>> Well, with user-space I include the lib that provides the gettimeofday() syscall
>> and reads out the memory region where the VDSO data structure is provided by the
>> kernel. And that part comes from Debian. Somehow it does differently with 4.19
>> than 5.4. So I summarise all non-kernel code with the term "user-space".
>> 
> 
> The the lib that provides the gettimeofday() changes accordingly with vdso_data.
> 5.4 and 4.19 have 2 different vdso libraries as well.

Yes, that is what I have assumed what happens. How do these libs go into an existing
and working root-file-system with Debian Stretch?

Is it part of the linux kernel tree so that some make option can build it and we can
install it like kernel modules (sorry for these beginners questions. I never did care
about VDSO before I ran into this problem)?

Is there a mechanism that Debian Stretch knows about the newer library
and can automatically find out which one to install.

This is what I mean with breaking user-space ABI.

Or must the user know about that and do a manual install of the vdso libs from external
sources?

If that is the case there should be at least a CONFIG option to provide the
older vdso_data or the option to completely disable VDSO for gettimeofday()
so that the library falls back to a traditional syscall.

BR and thanks,
Nikolaus

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[Maarten ter Huurne]

On Thursday, 28 November 2019 13:33:17 CET H. Nikolaus Schaller wrote:
> Hi Vincenzo,
> 
> > Am 28.11.2019 um 13:21 schrieb Vincenzo Frascino
> > <vincenzo.frascino@arm.com>:> 
> > [...]
> > The the lib that provides the gettimeofday() changes accordingly
> > with vdso_data. 5.4 and 4.19 have 2 different vdso libraries as
> > well.
> 
> Yes, that is what I have assumed what happens. How do these libs go
> into an existing and working root-file-system with Debian Stretch?

I'm a novice when it comes to vDSO, so someone please correct me if I'm 
wrong.

From what I read vDSO is a library in the sense that it exports ELF 
symbols that applications and other libraries (libc in particular) can 
use, but it is not a file on disk.

As such, which rootfs you use shouldn't matter, since the vDSO is not in 
the rootfs. Instead, it is contained in the kernel image. Searching for 
"linux-vdso.so.1" on packages.debian.org indeed returns no hits.

There is a check in arch/mips/vdso/Makefile that disables vDSO on MIPS 
when building the kernel with binutils < 2.25. I don't know if that is 
in any way related to this issue.

Bye,
		Maarten

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[H. Nikolaus Schaller]

> Am 28.11.2019 um 14:29 schrieb Maarten ter Huurne <maarten@treewalker.org>:
> 
> On Thursday, 28 November 2019 13:33:17 CET H. Nikolaus Schaller wrote:
>> Hi Vincenzo,
>> 
>>> Am 28.11.2019 um 13:21 schrieb Vincenzo Frascino
>>> <vincenzo.frascino@arm.com>:> 
>>> [...]
>>> The the lib that provides the gettimeofday() changes accordingly
>>> with vdso_data. 5.4 and 4.19 have 2 different vdso libraries as
>>> well.
>> 
>> Yes, that is what I have assumed what happens. How do these libs go
>> into an existing and working root-file-system with Debian Stretch?
> 
> I'm a novice when it comes to vDSO, so someone please correct me if I'm 
> wrong.
> 
> From what I read vDSO is a library in the sense that it exports ELF 
> symbols that applications and other libraries (libc in particular) can 
> use, but it is not a file on disk.

Ah, ok. This would mean that the libc providing the gettimeofday()
should be able to find out a modified changed vdso_data format by
inspecting these ELF symbols.

> 
> As such, which rootfs you use shouldn't matter, since the vDSO is not in 
> the rootfs. Instead, it is contained in the kernel image. Searching for 
> "linux-vdso.so.1" on packages.debian.org indeed returns no hits.
> 
> There is a check in arch/mips/vdso/Makefile that disables vDSO on MIPS 
> when building the kernel with binutils < 2.25. I don't know if that is 
> in any way related to this issue.

What still does not fit into the picture is the errno = 1 i.e. EPERM.
Maybe I have to study the libc code that tries to read the ELF symbols
you have mentioned. It may fail for unknown reasons.

BR and thanks,
Nikolaus

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[Vincenzo Frascino]

[-- Attachment #1: Type: text/plain, Size: 2132 bytes --]

On 28/11/2019 13:48, H. Nikolaus Schaller wrote:
> 
>> Am 28.11.2019 um 14:29 schrieb Maarten ter Huurne <maarten@treewalker.org>:
>>
>> On Thursday, 28 November 2019 13:33:17 CET H. Nikolaus Schaller wrote:
>>> Hi Vincenzo,
>>>
>>>> Am 28.11.2019 um 13:21 schrieb Vincenzo Frascino
>>>> <vincenzo.frascino@arm.com>:> 
>>>> [...]
>>>> The the lib that provides the gettimeofday() changes accordingly
>>>> with vdso_data. 5.4 and 4.19 have 2 different vdso libraries as
>>>> well.
>>>
>>> Yes, that is what I have assumed what happens. How do these libs go
>>> into an existing and working root-file-system with Debian Stretch?
>>
>> I'm a novice when it comes to vDSO, so someone please correct me if I'm 
>> wrong.
>>
>> From what I read vDSO is a library in the sense that it exports ELF 
>> symbols that applications and other libraries (libc in particular) can 
>> use, but it is not a file on disk.
> 
> Ah, ok. This would mean that the libc providing the gettimeofday()
> should be able to find out a modified changed vdso_data format by
> inspecting these ELF symbols.
>

I agree with Maarten here. There is a discovery mechanism in the libc based on
AT_SYSINFO_EHDR. This contains the address at which the vdso library is mapped
by the kernel to the userspace process.

>>
>> As such, which rootfs you use shouldn't matter, since the vDSO is not in 
>> the rootfs. Instead, it is contained in the kernel image. Searching for 
>> "linux-vdso.so.1" on packages.debian.org indeed returns no hits.
>>
>> There is a check in arch/mips/vdso/Makefile that disables vDSO on MIPS 
>> when building the kernel with binutils < 2.25. I don't know if that is 
>> in any way related to this issue.
> 
> What still does not fit into the picture is the errno = 1 i.e. EPERM.
> Maybe I have to study the libc code that tries to read the ELF symbols
> you have mentioned. It may fail for unknown reasons.
> 

This is what I was going to suggest next. It might be that something is not
working there.

Let us know your findings.

> BR and thanks,
> Nikolaus
> 

-- 
Regards,
Vincenzo

[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 14291 bytes --]

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[Vincenzo Frascino]

[-- Attachment #1: Type: text/plain, Size: 314 bytes --]

On 28/11/2019 12:16, H. Nikolaus Schaller wrote:
> Ah, good question. How do I find out? Checking defconfig did not show any
> config with R4K or GIC. So it is likely NONE. CONFIG_RTC_DRV_JZ4740=y if that
> helps.

Try to check what do you have in /sys/devices/system/clocksource.

-- 
Regards,
Vincenzo

[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 14291 bytes --]

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[Thomas Bogendoerfer]

On Thu, Nov 28, 2019 at 02:48:46PM +0100, H. Nikolaus Schaller wrote:
> 
> What still does not fit into the picture is the errno = 1 i.e. EPERM.
> Maybe I have to study the libc code that tries to read the ELF symbols
> you have mentioned. It may fail for unknown reasons.

to understand vdso you might look at arch/mips/vdso and lib/vdso
kernel sources.

And if I understand it correctly you neither have a working high resolution
timer usable bei do_hres() in lib/vdso/gettimeofday.c or a working
gettimeofday_fallback(), which is enabled via CONFIG_MIPS_CLOCK_VSYSCALL
and needs either CSRC_R4K or CLKSRC_MIPS_GIC.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[H. Nikolaus Schaller]

> Am 28.11.2019 um 15:04 schrieb Vincenzo Frascino <vincenzo.frascino@arm.com>:
> 
> On 28/11/2019 12:16, H. Nikolaus Schaller wrote:
>> Ah, good question. How do I find out? Checking defconfig did not show any
>> config with R4K or GIC. So it is likely NONE. CONFIG_RTC_DRV_JZ4740=y if that
>> helps.
> 
> Try to check what do you have in /sys/devices/system/clocksource.

Ok, have finished another project and could check:

root@letux:~# ls -l /sys/devices/system/clocksource/clocksource0/                    
total 0
-r--r--r-- 1 root root 4096 Nov  3 17:18 available_clocksource
-rw-r--r-- 1 root root 4096 Nov  3 17:18 current_clocksource
drwxr-xr-x 2 root root    0 Nov  3 17:18 power
lrwxrwxrwx 1 root root    0 Nov  3 17:16 subsystem -> ../../../../bus/clocksource
-rw-r--r-- 1 root root 4096 Nov  3 17:16 uevent
--w------- 1 root root 4096 Nov  3 17:18 unbind_clocksource
root@letux:~# cat /sys/devices/system/clocksource/clocksource0/*
ingenic-timer 
ingenic-timer
cat: /sys/devices/system/clocksource/clocksource0/power: Is a directory
cat: /sys/devices/system/clocksource/clocksource0/subsystem: Is a directory
cat: /sys/devices/system/clocksource/clocksource0/unbind_clocksource: Permission denied
root@letux:~#

Does that help?

BR,
Nikolaus

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[H. Nikolaus Schaller]

> Am 28.11.2019 um 16:07 schrieb Thomas Bogendoerfer <tsbogend@alpha.franken.de>:
> 
> On Thu, Nov 28, 2019 at 02:48:46PM +0100, H. Nikolaus Schaller wrote:
>> 
>> What still does not fit into the picture is the errno = 1 i.e. EPERM.
>> Maybe I have to study the libc code that tries to read the ELF symbols
>> you have mentioned. It may fail for unknown reasons.
> 
> to understand vdso you might look at arch/mips/vdso and lib/vdso
> kernel sources.

Yes that is what I know, but I do not know how glibc can return an EPERM
through VDSO.

> 
> And if I understand it correctly you neither have a working high resolution
> timer usable bei do_hres() in lib/vdso/gettimeofday.c or a working
> gettimeofday_fallback(), which is enabled via CONFIG_MIPS_CLOCK_VSYSCALL
> and needs either CSRC_R4K or CLKSRC_MIPS_GIC.

Well, on kernel v4.19 or using v5.4 with Jessie, gettimeofday() works.

So this may only be part of the reason it starts to fail with commit
24640f233b466051ad3a5d2786d2951e43026c9d.

MIPS_CLOCK_VSYSCALL is neither defined nor undefined in the defconfig.
And removed if I manually add it to my defconfig.
And yes, it depends on either CSRC_R4K || CLKSRC_MIPS_GIC by arch/mips/Kconfig.

Maybe this was just forgotten to properly handle for the ci20_defconfig
or jz4780?

What does CSRC_R4K resp. CLKSRC_MIPS_GIC ecactly mean? Does the jz4780 have this
feature?

BR and thanks,
Nikolaus

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[H. Nikolaus Schaller]

> Am 28.11.2019 um 15:01 schrieb Vincenzo Frascino <vincenzo.frascino@arm.com>:
> 
>>> 
>> 
>> What still does not fit into the picture is the errno = 1 i.e. EPERM.
>> Maybe I have to study the libc code that tries to read the ELF symbols
>> you have mentioned. It may fail for unknown reasons.
>> 
> 
> This is what I was going to suggest next. It might be that something is not
> working there.
> 
> Let us know your findings.

Well, my findings are that the gettimeofday() function in glibc-2.24 (used by
Debian Stretch) is simple:

> #include <errno.h>
> #include <sys/time.h>
> 
> #undef __gettimeofday
> 
> #ifdef HAVE_GETTIMEOFDAY_VSYSCALL
> # define HAVE_VSYSCALL
> #endif
> #include <sysdep-vdso.h>
> 
> /* Get the current time of day and timezone information,
>    putting it into *tv and *tz.  If tz is null, *tz is not filled.
>    Returns 0 on success, -1 on errors.  */
> int
> __gettimeofday (struct timeval *tv, struct timezone *tz)
> {
>   return INLINE_VSYSCALL (gettimeofday, 2, tv, tz);
> }
> libc_hidden_def (__gettimeofday)
> weak_alias (__gettimeofday, gettimeofday)
> libc_hidden_weak (gettimeofday)
> 

but the macro INLINE_VSYSCALL is not immediately understandable.

So I have no idea how to patch the kernel to check why that
sets errno. And patching libc is beyond my range of experience.

It is only clear that your kernel patch did break it for MIPS
(at least for CI20) but for obvious reasons not for ARM.

BTW: kernel v5.4 works with Debian Jessie, i.e. glibc-2.19.
Maybe it triggers a fallback.

BR and thanks,
Nikolaus

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[Vincenzo Frascino]

[-- Attachment #1: Type: text/plain, Size: 4515 bytes --]


On 28/11/2019 15:41, H. Nikolaus Schaller wrote:
> 
>> Am 28.11.2019 um 16:07 schrieb Thomas Bogendoerfer <tsbogend@alpha.franken.de>:
>>
>> On Thu, Nov 28, 2019 at 02:48:46PM +0100, H. Nikolaus Schaller wrote:
>>>
>>> What still does not fit into the picture is the errno = 1 i.e. EPERM.
>>> Maybe I have to study the libc code that tries to read the ELF symbols
>>> you have mentioned. It may fail for unknown reasons.
>>
>> to understand vdso you might look at arch/mips/vdso and lib/vdso
>> kernel sources.
> 
> Yes that is what I know, but I do not know how glibc can return an EPERM
> through VDSO.
> 
>>
>> And if I understand it correctly you neither have a working high resolution
>> timer usable bei do_hres() in lib/vdso/gettimeofday.c or a working
>> gettimeofday_fallback(), which is enabled via CONFIG_MIPS_CLOCK_VSYSCALL
>> and needs either CSRC_R4K or CLKSRC_MIPS_GIC.
> 
> Well, on kernel v4.19 or using v5.4 with Jessie, gettimeofday() works.
> 
> So this may only be part of the reason it starts to fail with commit
> 24640f233b466051ad3a5d2786d2951e43026c9d.
> 
> MIPS_CLOCK_VSYSCALL is neither defined nor undefined in the defconfig.
> And removed if I manually add it to my defconfig.
> And yes, it depends on either CSRC_R4K || CLKSRC_MIPS_GIC by arch/mips/Kconfig.
> 
> Maybe this was just forgotten to properly handle for the ci20_defconfig
> or jz4780?
> 
> What does CSRC_R4K resp. CLKSRC_MIPS_GIC ecactly mean? Does the jz4780 have this
> feature?
>

I think I know what is going on, it is the way on which libc does the fallback.
Could you please try the patch below the scissors and let me know if it works?
If it does I will create a proper one.

> BR and thanks,
> Nikolaus
> 

-- 
Regards,
Vincenzo

--->8---


From e15157402c750298a5c82a08c934c19e71f24fce Mon Sep 17 00:00:00 2001
From: Vincenzo Frascino <vincenzo.frascino@arm.com>
Date: Thu, 28 Nov 2019 15:37:31 +0000
Subject: [PATCH] Mips32 gettimeofday fix

Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 arch/mips/include/asm/vdso/gettimeofday.h | 11 +----------
 arch/mips/vdso/vgettimeofday.c            | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/arch/mips/include/asm/vdso/gettimeofday.h
b/arch/mips/include/asm/vdso/gettimeofday.h
index b08825531e9f..f4fc11a4d324 100644
--- a/arch/mips/include/asm/vdso/gettimeofday.h
+++ b/arch/mips/include/asm/vdso/gettimeofday.h
@@ -48,16 +48,7 @@ static __always_inline long gettimeofday_fallback(
 	return error ? -ret : ret;
 }

-#else
-
-static __always_inline long gettimeofday_fallback(
-				struct __kernel_old_timeval *_tv,
-				struct timezone *_tz)
-{
-	return -1;
-}
-
-#endif
+#endif /* CONFIG_MIPS_CLOCK_VSYSCALL */

 static __always_inline long clock_gettime_fallback(
 					clockid_t _clkid,
diff --git a/arch/mips/vdso/vgettimeofday.c b/arch/mips/vdso/vgettimeofday.c
index 6ebdc37c89fc..6b83b6376a4b 100644
--- a/arch/mips/vdso/vgettimeofday.c
+++ b/arch/mips/vdso/vgettimeofday.c
@@ -17,12 +17,22 @@ int __vdso_clock_gettime(clockid_t clock,
 	return __cvdso_clock_gettime32(clock, ts);
 }

+#ifdef CONFIG_MIPS_CLOCK_VSYSCALL
+
+/*
+ * This is behind the ifdef so that we don't provide the symbol when there's no
+ * possibility of there being a usable clocksource, because there's nothing we
+ * can do without it. When libc fails the symbol lookup it should fall back on
+ * the standard syscall path.
+ */
 int __vdso_gettimeofday(struct __kernel_old_timeval *tv,
 			struct timezone *tz)
 {
 	return __cvdso_gettimeofday(tv, tz);
 }

+#endif /* CONFIG_MIPS_CLOCK_VSYSCALL */
+
 int __vdso_clock_getres(clockid_t clock_id,
 			struct old_timespec32 *res)
 {
@@ -43,12 +53,22 @@ int __vdso_clock_gettime(clockid_t clock,
 	return __cvdso_clock_gettime(clock, ts);
 }

+#ifdef CONFIG_MIPS_CLOCK_VSYSCALL
+
+/*
+ * This is behind the ifdef so that we don't provide the symbol when there's no
+ * possibility of there being a usable clocksource, because there's nothing we
+ * can do without it. When libc fails the symbol lookup it should fall back on
+ * the standard syscall path.
+ */
 int __vdso_gettimeofday(struct __kernel_old_timeval *tv,
 			struct timezone *tz)
 {
 	return __cvdso_gettimeofday(tv, tz);
 }

+#endif /* CONFIG_MIPS_CLOCK_VSYSCALL */
+
 int __vdso_clock_getres(clockid_t clock_id,
 			struct __kernel_timespec *res)
 {
-- 
2.24.0


[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 14291 bytes --]

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[H. Nikolaus Schaller]

Hi Vincenzo,

> Am 28.11.2019 um 16:46 schrieb Vincenzo Frascino <vincenzo.frascino@arm.com>:
> 
> 
> On 28/11/2019 15:41, H. Nikolaus Schaller wrote:
>> 
>>> Am 28.11.2019 um 16:07 schrieb Thomas Bogendoerfer <tsbogend@alpha.franken.de>:
>>> 
>>> On Thu, Nov 28, 2019 at 02:48:46PM +0100, H. Nikolaus Schaller wrote:
>>>> 
>>>> What still does not fit into the picture is the errno = 1 i.e. EPERM.
>>>> Maybe I have to study the libc code that tries to read the ELF symbols
>>>> you have mentioned. It may fail for unknown reasons.
>>> 
>>> to understand vdso you might look at arch/mips/vdso and lib/vdso
>>> kernel sources.
>> 
>> Yes that is what I know, but I do not know how glibc can return an EPERM
>> through VDSO.
>> 
>>> 
>>> And if I understand it correctly you neither have a working high resolution
>>> timer usable bei do_hres() in lib/vdso/gettimeofday.c or a working
>>> gettimeofday_fallback(), which is enabled via CONFIG_MIPS_CLOCK_VSYSCALL
>>> and needs either CSRC_R4K or CLKSRC_MIPS_GIC.
>> 
>> Well, on kernel v4.19 or using v5.4 with Jessie, gettimeofday() works.
>> 
>> So this may only be part of the reason it starts to fail with commit
>> 24640f233b466051ad3a5d2786d2951e43026c9d.
>> 
>> MIPS_CLOCK_VSYSCALL is neither defined nor undefined in the defconfig.
>> And removed if I manually add it to my defconfig.
>> And yes, it depends on either CSRC_R4K || CLKSRC_MIPS_GIC by arch/mips/Kconfig.
>> 
>> Maybe this was just forgotten to properly handle for the ci20_defconfig
>> or jz4780?
>> 
>> What does CSRC_R4K resp. CLKSRC_MIPS_GIC ecactly mean? Does the jz4780 have this
>> feature?
>> 
> 
> I think I know what is going on, it is the way on which libc does the fallback.
> Could you please try the patch below the scissors and let me know if it works?

Well, it does not immediately compile because CONFIG_MIPS_CLOCK_VSYSCALL is not
set and can not be configured by normal means:

Error:

/Volumes/CaseSensitive/master/lib/vdso/gettimeofday.c: In function '__cvdso_gettimeofday':
/Volumes/CaseSensitive/master/lib/vdso/gettimeofday.c:152:4: error: implicit declaration of function 'gettimeofday_fallback' [-Werror=implicit-function-declaration]
    return gettimeofday_fallback(tv, tz);
    ^

So I force-#defined CONFIG_MIPS_CLOCK_VSYSCALL in arch/mips/include/asm/vdso/gettimeofday.h
which is also included by lib/vdso/gettimeofday.c

Yes, it works now! Great!!!

root@letux:~# ./gettime 
r = 0 errno=0
tv.sec = 1574959485
tv.usec = 671306
rt = 1574959485 errno=0
t = 1574959485
root@letux:~# cat /etc/debian_version 
9.11
root@letux:~# uname -a
Linux letux 5.4.0-letux-l400+ #1490 PREEMPT Thu Nov 28 17:23:32 CET 2019 mips GNU/Linux
root@letux:~# dhclient
root@letux:~# 

> If it does I will create a proper one.

It needs some mechanism to set CONFIG_MIPS_CLOCK_VSYSCALL.

I'll test if you have something new.

BR and thanks,
Nikolaus

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[Vincenzo Frascino]

On 11/28/19 4:47 PM, H. Nikolaus Schaller wrote:
> Well, it does not immediately compile because CONFIG_MIPS_CLOCK_VSYSCALL is not
> set and can not be configured by normal means:
> 
> Error:
> 
> /Volumes/CaseSensitive/master/lib/vdso/gettimeofday.c: In function '__cvdso_gettimeofday':
> /Volumes/CaseSensitive/master/lib/vdso/gettimeofday.c:152:4: error: implicit declaration of function 'gettimeofday_fallback' [-Werror=implicit-function-declaration]
>     return gettimeofday_fallback(tv, tz);

Oops, I just realized that I had other changes not committed that's why it does
not build for you directly. Sometimes I get so excited for fixing a problem that
I forget bits and peaces :) Sorry about that.

I am happy to hear that this sorts out the issue though. I will send out a new
series to test.

-- 
Regards,
Vincenzo

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[Vincenzo Frascino]

[-- Attachment #1: Type: text/plain, Size: 995 bytes --]

Hi Nikolaus,

On 11/29/19 8:57 AM, Vincenzo Frascino wrote:
> On 11/28/19 4:47 PM, H. Nikolaus Schaller wrote:
>> Well, it does not immediately compile because CONFIG_MIPS_CLOCK_VSYSCALL is not
>> set and can not be configured by normal means:
>>
>> Error:
>>
>> /Volumes/CaseSensitive/master/lib/vdso/gettimeofday.c: In function '__cvdso_gettimeofday':
>> /Volumes/CaseSensitive/master/lib/vdso/gettimeofday.c:152:4: error: implicit declaration of function 'gettimeofday_fallback' [-Werror=implicit-function-declaration]
>>     return gettimeofday_fallback(tv, tz);
> 
> Oops, I just realized that I had other changes not committed that's why it does
> not build for you directly. Sometimes I get so excited for fixing a problem that
> I forget bits and peaces :) Sorry about that.
> 
> I am happy to hear that this sorts out the issue though. I will send out a new
> series to test.
> 

Could you please try the patch in attachment? It should be all this time.

Thanks!

-- 
Regards,
Vincenzo

[-- Attachment #2: 0001-mips32-gettimeofday-fix.patch --]
[-- Type: text/x-patch, Size: 2876 bytes --]

From 6ee7d583444f7edb1b5b8e834253b34efe4ff257 Mon Sep 17 00:00:00 2001
From: Vincenzo Frascino <vinfra01@e120585-lin.cambridge.arm.com>
Date: Fri, 29 Nov 2019 10:01:18 +0000
Subject: [PATCH] mips32 gettimeofday fix

Signed-off-by: Vincenzo Frascino <vinfra01@e120585-lin.cambridge.arm.com>
---
 arch/mips/include/asm/vdso/gettimeofday.h | 13 -------------
 arch/mips/vdso/vgettimeofday.c            | 20 ++++++++++++++++++++
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/arch/mips/include/asm/vdso/gettimeofday.h b/arch/mips/include/asm/vdso/gettimeofday.h
index b08825531e9f..0ae9b4cbc153 100644
--- a/arch/mips/include/asm/vdso/gettimeofday.h
+++ b/arch/mips/include/asm/vdso/gettimeofday.h
@@ -26,8 +26,6 @@
 
 #define __VDSO_USE_SYSCALL		ULLONG_MAX
 
-#ifdef CONFIG_MIPS_CLOCK_VSYSCALL
-
 static __always_inline long gettimeofday_fallback(
 				struct __kernel_old_timeval *_tv,
 				struct timezone *_tz)
@@ -48,17 +46,6 @@ static __always_inline long gettimeofday_fallback(
 	return error ? -ret : ret;
 }
 
-#else
-
-static __always_inline long gettimeofday_fallback(
-				struct __kernel_old_timeval *_tv,
-				struct timezone *_tz)
-{
-	return -1;
-}
-
-#endif
-
 static __always_inline long clock_gettime_fallback(
 					clockid_t _clkid,
 					struct __kernel_timespec *_ts)
diff --git a/arch/mips/vdso/vgettimeofday.c b/arch/mips/vdso/vgettimeofday.c
index 6ebdc37c89fc..6b83b6376a4b 100644
--- a/arch/mips/vdso/vgettimeofday.c
+++ b/arch/mips/vdso/vgettimeofday.c
@@ -17,12 +17,22 @@ int __vdso_clock_gettime(clockid_t clock,
 	return __cvdso_clock_gettime32(clock, ts);
 }
 
+#ifdef CONFIG_MIPS_CLOCK_VSYSCALL
+
+/*
+ * This is behind the ifdef so that we don't provide the symbol when there's no
+ * possibility of there being a usable clocksource, because there's nothing we
+ * can do without it. When libc fails the symbol lookup it should fall back on
+ * the standard syscall path.
+ */
 int __vdso_gettimeofday(struct __kernel_old_timeval *tv,
 			struct timezone *tz)
 {
 	return __cvdso_gettimeofday(tv, tz);
 }
 
+#endif /* CONFIG_MIPS_CLOCK_VSYSCALL */
+
 int __vdso_clock_getres(clockid_t clock_id,
 			struct old_timespec32 *res)
 {
@@ -43,12 +53,22 @@ int __vdso_clock_gettime(clockid_t clock,
 	return __cvdso_clock_gettime(clock, ts);
 }
 
+#ifdef CONFIG_MIPS_CLOCK_VSYSCALL
+
+/*
+ * This is behind the ifdef so that we don't provide the symbol when there's no
+ * possibility of there being a usable clocksource, because there's nothing we
+ * can do without it. When libc fails the symbol lookup it should fall back on
+ * the standard syscall path.
+ */
 int __vdso_gettimeofday(struct __kernel_old_timeval *tv,
 			struct timezone *tz)
 {
 	return __cvdso_gettimeofday(tv, tz);
 }
 
+#endif /* CONFIG_MIPS_CLOCK_VSYSCALL */
+
 int __vdso_clock_getres(clockid_t clock_id,
 			struct __kernel_timespec *res)
 {
-- 
2.24.0

Re: MIPS: bug: gettimeofday syscall broken on CI20 board

[H. Nikolaus Schaller]

Hi Vincenzo,

> Am 29.11.2019 um 11:53 schrieb Vincenzo Frascino <vincenzo.frascino@arm.com>:
> 
> Hi Nikolaus,
> 
> On 11/29/19 8:57 AM, Vincenzo Frascino wrote:
>> On 11/28/19 4:47 PM, H. Nikolaus Schaller wrote:
>>> Well, it does not immediately compile because CONFIG_MIPS_CLOCK_VSYSCALL is not
>>> set and can not be configured by normal means:
>>> 
>>> Error:
>>> 
>>> /Volumes/CaseSensitive/master/lib/vdso/gettimeofday.c: In function '__cvdso_gettimeofday':
>>> /Volumes/CaseSensitive/master/lib/vdso/gettimeofday.c:152:4: error: implicit declaration of function 'gettimeofday_fallback' [-Werror=implicit-function-declaration]
>>>    return gettimeofday_fallback(tv, tz);
>> 
>> Oops, I just realized that I had other changes not committed that's why it does
>> not build for you directly. Sometimes I get so excited for fixing a problem that
>> I forget bits and peaces :) Sorry about that.
>> 
>> I am happy to hear that this sorts out the issue though. I will send out a new
>> series to test.
>> 
> 
> Could you please try the patch in attachment? It should be all this time.

Yes,
applies well, compiles well and works.

So you can add:

Testes-by: H. Nikolaus Schaller <hns@goldelico.com> # CI20 with JZ4780

> 
> Thanks!

BR and thanks,
Nikolaus

Re: [PATCH] Mips32 gettimeofday fix

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3743 bytes --]

Hi Vincenzo,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.4 next-20191129]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Vincenzo-Frascino/Mips32-gettimeofday-fix/20191129-022306
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a6ed68d6468bd5a3da78a103344ded1435fed57a
config: mips-jmr3927_defconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=mips 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from <command-line>:0:0:
   lib/vdso/gettimeofday.c: In function '__cvdso_gettimeofday':
>> lib/vdso/gettimeofday.c:152:11: error: implicit declaration of function 'gettimeofday_fallback'; did you mean 'clock_gettime32_fallback'? [-Werror=implicit-function-declaration]
       return gettimeofday_fallback(tv, tz);
              ^~~~~~~~~~~~~~~~~~~~~
              clock_gettime32_fallback
   cc1: all warnings being treated as errors

vim +152 lib/vdso/gettimeofday.c

00b26474c2f161 Vincenzo Frascino 2019-06-21  142  
00b26474c2f161 Vincenzo Frascino 2019-06-21  143  static __maybe_unused int
00b26474c2f161 Vincenzo Frascino 2019-06-21  144  __cvdso_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz)
00b26474c2f161 Vincenzo Frascino 2019-06-21  145  {
00b26474c2f161 Vincenzo Frascino 2019-06-21  146  	const struct vdso_data *vd = __arch_get_vdso_data();
00b26474c2f161 Vincenzo Frascino 2019-06-21  147  
00b26474c2f161 Vincenzo Frascino 2019-06-21  148  	if (likely(tv != NULL)) {
00b26474c2f161 Vincenzo Frascino 2019-06-21  149  		struct __kernel_timespec ts;
00b26474c2f161 Vincenzo Frascino 2019-06-21  150  
00b26474c2f161 Vincenzo Frascino 2019-06-21  151  		if (do_hres(&vd[CS_HRES_COARSE], CLOCK_REALTIME, &ts))
00b26474c2f161 Vincenzo Frascino 2019-06-21 @152  			return gettimeofday_fallback(tv, tz);
00b26474c2f161 Vincenzo Frascino 2019-06-21  153  
00b26474c2f161 Vincenzo Frascino 2019-06-21  154  		tv->tv_sec = ts.tv_sec;
00b26474c2f161 Vincenzo Frascino 2019-06-21  155  		tv->tv_usec = (u32)ts.tv_nsec / NSEC_PER_USEC;
00b26474c2f161 Vincenzo Frascino 2019-06-21  156  	}
00b26474c2f161 Vincenzo Frascino 2019-06-21  157  
00b26474c2f161 Vincenzo Frascino 2019-06-21  158  	if (unlikely(tz != NULL)) {
00b26474c2f161 Vincenzo Frascino 2019-06-21  159  		tz->tz_minuteswest = vd[CS_HRES_COARSE].tz_minuteswest;
00b26474c2f161 Vincenzo Frascino 2019-06-21  160  		tz->tz_dsttime = vd[CS_HRES_COARSE].tz_dsttime;
00b26474c2f161 Vincenzo Frascino 2019-06-21  161  	}
00b26474c2f161 Vincenzo Frascino 2019-06-21  162  
00b26474c2f161 Vincenzo Frascino 2019-06-21  163  	return 0;
00b26474c2f161 Vincenzo Frascino 2019-06-21  164  }
00b26474c2f161 Vincenzo Frascino 2019-06-21  165  

:::::: The code at line 152 was first introduced by commit
:::::: 00b26474c2f1613d7ab894c525f775c67c8a9e8f lib/vdso: Provide generic VDSO implementation

:::::: TO: Vincenzo Frascino <vincenzo.frascino@arm.com>
:::::: CC: Thomas Gleixner <tglx@linutronix.de>

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 11751 bytes --]