| Message ID | 20181130192216.26987-3-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/s390x: Implement STCK et al for CONFIG_USER_ONLY | expand |
On 2018-11-30 20:22, Richard Henderson wrote: > This is a non-privileged instruction that was only implemented > for system mode. However, the stck instruction is used by glibc, > so this was causing SIGILL for programs run under debian stretch. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/s390x/helper.h | 2 +- > target/s390x/misc_helper.c | 13 ++++++++++++- > target/s390x/translate.c | 2 ++ > target/s390x/insn-data.def | 11 ++++++----- > 4 files changed, 21 insertions(+), 7 deletions(-) > > diff --git a/target/s390x/helper.h b/target/s390x/helper.h > index 018e9dd414..6260b50496 100644 > --- a/target/s390x/helper.h > +++ b/target/s390x/helper.h > @@ -121,13 +121,13 @@ DEF_HELPER_4(cu41, i32, env, i32, i32, i32) > DEF_HELPER_4(cu42, i32, env, i32, i32, i32) > DEF_HELPER_5(msa, i32, env, i32, i32, i32, i32) > DEF_HELPER_FLAGS_1(stpt, TCG_CALL_NO_RWG, i64, env) > +DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) > > #ifndef CONFIG_USER_ONLY > DEF_HELPER_3(servc, i32, env, i64, i64) > DEF_HELPER_4(diag, void, env, i32, i32, i32) > DEF_HELPER_3(load_psw, noreturn, env, i64, i64) > DEF_HELPER_FLAGS_2(spx, TCG_CALL_NO_RWG, void, env, i64) > -DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) > DEF_HELPER_FLAGS_2(sck, TCG_CALL_NO_RWG, i32, env, i64) > DEF_HELPER_FLAGS_2(sckc, TCG_CALL_NO_RWG, void, env, i64) > DEF_HELPER_FLAGS_2(sckpf, TCG_CALL_NO_RWG, void, env, i64) > diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c > index 3f91579570..c2940afecb 100644 > --- a/target/s390x/misc_helper.c > +++ b/target/s390x/misc_helper.c > @@ -76,8 +76,19 @@ uint64_t HELPER(stpt)(CPUS390XState *env) > #endif > } > > -#ifndef CONFIG_USER_ONLY > +#ifdef CONFIG_USER_ONLY > +/* Store Clock */ > +uint64_t HELPER(stck)(CPUS390XState *env) > +{ > + struct timespec ts; > + uint64_t ms; May I suggest "us" for microseconds? "ms" makes me thing of "milliseconds"... > + clock_gettime(CLOCK_REALTIME, &ts); > + ms = (ts.tv_nsec / 1000) + (ts.tv_sec * 100000ull); > + > + return TOD_UNIX_EPOCH + ms; > +} Don't you need to shift the value around a little bit for getting the right format of the TOD clock? The microseconds are not starting with the lowest bit, the TOD clock has a higher resolution. See also the time2tod() macro in include/hw/s390x/tod.h ... Thomas
On 30.11.18 20:22, Richard Henderson wrote: > This is a non-privileged instruction that was only implemented > for system mode. However, the stck instruction is used by glibc, > so this was causing SIGILL for programs run under debian stretch. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/s390x/helper.h | 2 +- > target/s390x/misc_helper.c | 13 ++++++++++++- > target/s390x/translate.c | 2 ++ > target/s390x/insn-data.def | 11 ++++++----- > 4 files changed, 21 insertions(+), 7 deletions(-) > > diff --git a/target/s390x/helper.h b/target/s390x/helper.h > index 018e9dd414..6260b50496 100644 > --- a/target/s390x/helper.h > +++ b/target/s390x/helper.h > @@ -121,13 +121,13 @@ DEF_HELPER_4(cu41, i32, env, i32, i32, i32) > DEF_HELPER_4(cu42, i32, env, i32, i32, i32) > DEF_HELPER_5(msa, i32, env, i32, i32, i32, i32) > DEF_HELPER_FLAGS_1(stpt, TCG_CALL_NO_RWG, i64, env) > +DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) > > #ifndef CONFIG_USER_ONLY > DEF_HELPER_3(servc, i32, env, i64, i64) > DEF_HELPER_4(diag, void, env, i32, i32, i32) > DEF_HELPER_3(load_psw, noreturn, env, i64, i64) > DEF_HELPER_FLAGS_2(spx, TCG_CALL_NO_RWG, void, env, i64) > -DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) > DEF_HELPER_FLAGS_2(sck, TCG_CALL_NO_RWG, i32, env, i64) > DEF_HELPER_FLAGS_2(sckc, TCG_CALL_NO_RWG, void, env, i64) > DEF_HELPER_FLAGS_2(sckpf, TCG_CALL_NO_RWG, void, env, i64) > diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c > index 3f91579570..c2940afecb 100644 > --- a/target/s390x/misc_helper.c > +++ b/target/s390x/misc_helper.c > @@ -76,8 +76,19 @@ uint64_t HELPER(stpt)(CPUS390XState *env) > #endif > } > > -#ifndef CONFIG_USER_ONLY > +#ifdef CONFIG_USER_ONLY > +/* Store Clock */ > +uint64_t HELPER(stck)(CPUS390XState *env) > +{ > + struct timespec ts; > + uint64_t ms; > > + clock_gettime(CLOCK_REALTIME, &ts); > + ms = (ts.tv_nsec / 1000) + (ts.tv_sec * 100000ull); > + > + return TOD_UNIX_EPOCH + ms; In theory, the TOD can be completely controlled by the operating system (e.g. set the TOD to X). So for user space, there isn't really any guarantee about the values returned via stck. E.g. in the PoP 4-51: "4. A program using the clock value as a time-of-day and calendar indication must be consistent with the programming support under which the pro- gram is to be executed. [...]" Of course, in an ideal world, the system TOD value really corresponds to the reference TOD. So I assume, in theory, we can simply indicate any monotonic values here. E.g. not using TOD_UNIX_EPOCH and only forwarding the result from CLOCK_REALTIME converted to appropriate TOD values. If I am not wrong, under Linux, user space will only use stck for measurement, and use the vdso for getting the real time. -- Thanks, David / dhildenb
On 12/3/18 12:19 AM, Thomas Huth wrote: > On 2018-11-30 20:22, Richard Henderson wrote: >> This is a non-privileged instruction that was only implemented >> for system mode. However, the stck instruction is used by glibc, >> so this was causing SIGILL for programs run under debian stretch. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> target/s390x/helper.h | 2 +- >> target/s390x/misc_helper.c | 13 ++++++++++++- >> target/s390x/translate.c | 2 ++ >> target/s390x/insn-data.def | 11 ++++++----- >> 4 files changed, 21 insertions(+), 7 deletions(-) >> >> diff --git a/target/s390x/helper.h b/target/s390x/helper.h >> index 018e9dd414..6260b50496 100644 >> --- a/target/s390x/helper.h >> +++ b/target/s390x/helper.h >> @@ -121,13 +121,13 @@ DEF_HELPER_4(cu41, i32, env, i32, i32, i32) >> DEF_HELPER_4(cu42, i32, env, i32, i32, i32) >> DEF_HELPER_5(msa, i32, env, i32, i32, i32, i32) >> DEF_HELPER_FLAGS_1(stpt, TCG_CALL_NO_RWG, i64, env) >> +DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) >> >> #ifndef CONFIG_USER_ONLY >> DEF_HELPER_3(servc, i32, env, i64, i64) >> DEF_HELPER_4(diag, void, env, i32, i32, i32) >> DEF_HELPER_3(load_psw, noreturn, env, i64, i64) >> DEF_HELPER_FLAGS_2(spx, TCG_CALL_NO_RWG, void, env, i64) >> -DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) >> DEF_HELPER_FLAGS_2(sck, TCG_CALL_NO_RWG, i32, env, i64) >> DEF_HELPER_FLAGS_2(sckc, TCG_CALL_NO_RWG, void, env, i64) >> DEF_HELPER_FLAGS_2(sckpf, TCG_CALL_NO_RWG, void, env, i64) >> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c >> index 3f91579570..c2940afecb 100644 >> --- a/target/s390x/misc_helper.c >> +++ b/target/s390x/misc_helper.c >> @@ -76,8 +76,19 @@ uint64_t HELPER(stpt)(CPUS390XState *env) >> #endif >> } >> >> -#ifndef CONFIG_USER_ONLY >> +#ifdef CONFIG_USER_ONLY >> +/* Store Clock */ >> +uint64_t HELPER(stck)(CPUS390XState *env) >> +{ >> + struct timespec ts; >> + uint64_t ms; > > May I suggest "us" for microseconds? "ms" makes me thing of > "milliseconds"... Oops, think-o. >> + clock_gettime(CLOCK_REALTIME, &ts); >> + ms = (ts.tv_nsec / 1000) + (ts.tv_sec * 100000ull); >> + >> + return TOD_UNIX_EPOCH + ms; >> +} > > Don't you need to shift the value around a little bit for getting the > right format of the TOD clock? The microseconds are not starting with > the lowest bit, the TOD clock has a higher resolution. See also the > time2tod() macro in include/hw/s390x/tod.h ... That's for STCKE, which does use this routine and shift the result around. r~
On 12/3/18 4:21 AM, David Hildenbrand wrote: > On 30.11.18 20:22, Richard Henderson wrote: >> This is a non-privileged instruction that was only implemented >> for system mode. However, the stck instruction is used by glibc, >> so this was causing SIGILL for programs run under debian stretch. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> target/s390x/helper.h | 2 +- >> target/s390x/misc_helper.c | 13 ++++++++++++- >> target/s390x/translate.c | 2 ++ >> target/s390x/insn-data.def | 11 ++++++----- >> 4 files changed, 21 insertions(+), 7 deletions(-) >> >> diff --git a/target/s390x/helper.h b/target/s390x/helper.h >> index 018e9dd414..6260b50496 100644 >> --- a/target/s390x/helper.h >> +++ b/target/s390x/helper.h >> @@ -121,13 +121,13 @@ DEF_HELPER_4(cu41, i32, env, i32, i32, i32) >> DEF_HELPER_4(cu42, i32, env, i32, i32, i32) >> DEF_HELPER_5(msa, i32, env, i32, i32, i32, i32) >> DEF_HELPER_FLAGS_1(stpt, TCG_CALL_NO_RWG, i64, env) >> +DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) >> >> #ifndef CONFIG_USER_ONLY >> DEF_HELPER_3(servc, i32, env, i64, i64) >> DEF_HELPER_4(diag, void, env, i32, i32, i32) >> DEF_HELPER_3(load_psw, noreturn, env, i64, i64) >> DEF_HELPER_FLAGS_2(spx, TCG_CALL_NO_RWG, void, env, i64) >> -DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) >> DEF_HELPER_FLAGS_2(sck, TCG_CALL_NO_RWG, i32, env, i64) >> DEF_HELPER_FLAGS_2(sckc, TCG_CALL_NO_RWG, void, env, i64) >> DEF_HELPER_FLAGS_2(sckpf, TCG_CALL_NO_RWG, void, env, i64) >> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c >> index 3f91579570..c2940afecb 100644 >> --- a/target/s390x/misc_helper.c >> +++ b/target/s390x/misc_helper.c >> @@ -76,8 +76,19 @@ uint64_t HELPER(stpt)(CPUS390XState *env) >> #endif >> } >> >> -#ifndef CONFIG_USER_ONLY >> +#ifdef CONFIG_USER_ONLY >> +/* Store Clock */ >> +uint64_t HELPER(stck)(CPUS390XState *env) >> +{ >> + struct timespec ts; >> + uint64_t ms; >> >> + clock_gettime(CLOCK_REALTIME, &ts); >> + ms = (ts.tv_nsec / 1000) + (ts.tv_sec * 100000ull); >> + >> + return TOD_UNIX_EPOCH + ms; > > In theory, the TOD can be completely controlled by the operating system > (e.g. set the TOD to X). So for user space, there isn't really any > guarantee about the values returned via stck. > > E.g. in the PoP 4-51: > > "4. A program using the clock value as a time-of-day > and calendar indication must be consistent with > the programming support under which the pro- > gram is to be executed. [...]" Ok, but then there's the matter of the CC result. We currently hard-code this as 0, meaning "clock is set", meaning it does have the real TOD value. We could set CC as 1, meaning "clock is not set", meaning the value is only good for relative computation. Is this perhaps a bug in our system implementation as well? What CC value is provided to userspace on real hardware? r~
On 2018-12-03 14:27, Richard Henderson wrote: > On 12/3/18 12:19 AM, Thomas Huth wrote: >> On 2018-11-30 20:22, Richard Henderson wrote: >>> This is a non-privileged instruction that was only implemented >>> for system mode. However, the stck instruction is used by glibc, >>> so this was causing SIGILL for programs run under debian stretch. >>> >>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>> --- >>> target/s390x/helper.h | 2 +- >>> target/s390x/misc_helper.c | 13 ++++++++++++- >>> target/s390x/translate.c | 2 ++ >>> target/s390x/insn-data.def | 11 ++++++----- >>> 4 files changed, 21 insertions(+), 7 deletions(-) >>> >>> diff --git a/target/s390x/helper.h b/target/s390x/helper.h >>> index 018e9dd414..6260b50496 100644 >>> --- a/target/s390x/helper.h >>> +++ b/target/s390x/helper.h >>> @@ -121,13 +121,13 @@ DEF_HELPER_4(cu41, i32, env, i32, i32, i32) >>> DEF_HELPER_4(cu42, i32, env, i32, i32, i32) >>> DEF_HELPER_5(msa, i32, env, i32, i32, i32, i32) >>> DEF_HELPER_FLAGS_1(stpt, TCG_CALL_NO_RWG, i64, env) >>> +DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) >>> >>> #ifndef CONFIG_USER_ONLY >>> DEF_HELPER_3(servc, i32, env, i64, i64) >>> DEF_HELPER_4(diag, void, env, i32, i32, i32) >>> DEF_HELPER_3(load_psw, noreturn, env, i64, i64) >>> DEF_HELPER_FLAGS_2(spx, TCG_CALL_NO_RWG, void, env, i64) >>> -DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) >>> DEF_HELPER_FLAGS_2(sck, TCG_CALL_NO_RWG, i32, env, i64) >>> DEF_HELPER_FLAGS_2(sckc, TCG_CALL_NO_RWG, void, env, i64) >>> DEF_HELPER_FLAGS_2(sckpf, TCG_CALL_NO_RWG, void, env, i64) >>> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c >>> index 3f91579570..c2940afecb 100644 >>> --- a/target/s390x/misc_helper.c >>> +++ b/target/s390x/misc_helper.c >>> @@ -76,8 +76,19 @@ uint64_t HELPER(stpt)(CPUS390XState *env) >>> #endif >>> } >>> >>> -#ifndef CONFIG_USER_ONLY >>> +#ifdef CONFIG_USER_ONLY >>> +/* Store Clock */ >>> +uint64_t HELPER(stck)(CPUS390XState *env) >>> +{ >>> + struct timespec ts; >>> + uint64_t ms; >> >> May I suggest "us" for microseconds? "ms" makes me thing of >> "milliseconds"... > > Oops, think-o. > >>> + clock_gettime(CLOCK_REALTIME, &ts); >>> + ms = (ts.tv_nsec / 1000) + (ts.tv_sec * 100000ull); >>> + >>> + return TOD_UNIX_EPOCH + ms; >>> +} >> >> Don't you need to shift the value around a little bit for getting the >> right format of the TOD clock? The microseconds are not starting with >> the lowest bit, the TOD clock has a higher resolution. See also the >> time2tod() macro in include/hw/s390x/tod.h ... > > That's for STCKE, which does use this routine and shift the result around. STCKE certainly has even more bits ... but still, according to the PoP: "The TOD clock nominally is incremented by adding a one in bit position 51 every microsecond." So you've got to shift the microseconds by 12 bits to the left? ... but maybe I'm also just reading the PoP or the source code in a wrong way again... Thomas
On 12/3/18 8:02 AM, Thomas Huth wrote: > On 2018-12-03 14:27, Richard Henderson wrote: >> On 12/3/18 12:19 AM, Thomas Huth wrote: >>> On 2018-11-30 20:22, Richard Henderson wrote: >>>> This is a non-privileged instruction that was only implemented >>>> for system mode. However, the stck instruction is used by glibc, >>>> so this was causing SIGILL for programs run under debian stretch. >>>> >>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>>> --- >>>> target/s390x/helper.h | 2 +- >>>> target/s390x/misc_helper.c | 13 ++++++++++++- >>>> target/s390x/translate.c | 2 ++ >>>> target/s390x/insn-data.def | 11 ++++++----- >>>> 4 files changed, 21 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/target/s390x/helper.h b/target/s390x/helper.h >>>> index 018e9dd414..6260b50496 100644 >>>> --- a/target/s390x/helper.h >>>> +++ b/target/s390x/helper.h >>>> @@ -121,13 +121,13 @@ DEF_HELPER_4(cu41, i32, env, i32, i32, i32) >>>> DEF_HELPER_4(cu42, i32, env, i32, i32, i32) >>>> DEF_HELPER_5(msa, i32, env, i32, i32, i32, i32) >>>> DEF_HELPER_FLAGS_1(stpt, TCG_CALL_NO_RWG, i64, env) >>>> +DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) >>>> >>>> #ifndef CONFIG_USER_ONLY >>>> DEF_HELPER_3(servc, i32, env, i64, i64) >>>> DEF_HELPER_4(diag, void, env, i32, i32, i32) >>>> DEF_HELPER_3(load_psw, noreturn, env, i64, i64) >>>> DEF_HELPER_FLAGS_2(spx, TCG_CALL_NO_RWG, void, env, i64) >>>> -DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) >>>> DEF_HELPER_FLAGS_2(sck, TCG_CALL_NO_RWG, i32, env, i64) >>>> DEF_HELPER_FLAGS_2(sckc, TCG_CALL_NO_RWG, void, env, i64) >>>> DEF_HELPER_FLAGS_2(sckpf, TCG_CALL_NO_RWG, void, env, i64) >>>> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c >>>> index 3f91579570..c2940afecb 100644 >>>> --- a/target/s390x/misc_helper.c >>>> +++ b/target/s390x/misc_helper.c >>>> @@ -76,8 +76,19 @@ uint64_t HELPER(stpt)(CPUS390XState *env) >>>> #endif >>>> } >>>> >>>> -#ifndef CONFIG_USER_ONLY >>>> +#ifdef CONFIG_USER_ONLY >>>> +/* Store Clock */ >>>> +uint64_t HELPER(stck)(CPUS390XState *env) >>>> +{ >>>> + struct timespec ts; >>>> + uint64_t ms; >>> >>> May I suggest "us" for microseconds? "ms" makes me thing of >>> "milliseconds"... >> >> Oops, think-o. >> >>>> + clock_gettime(CLOCK_REALTIME, &ts); >>>> + ms = (ts.tv_nsec / 1000) + (ts.tv_sec * 100000ull); >>>> + >>>> + return TOD_UNIX_EPOCH + ms; >>>> +} >>> >>> Don't you need to shift the value around a little bit for getting the >>> right format of the TOD clock? The microseconds are not starting with >>> the lowest bit, the TOD clock has a higher resolution. See also the >>> time2tod() macro in include/hw/s390x/tod.h ... >> >> That's for STCKE, which does use this routine and shift the result around. > > STCKE certainly has even more bits ... but still, according to the PoP: > > "The TOD clock nominally is incremented by adding a > one in bit position 51 every microsecond." I don't see that language within the description of STCK. Perhaps it's elsewhere in the PoO. However you are right because STCK says "bit position 31 of the clock is incremented every 1.048576 seconds", and that amounts to exactly the same thing. Probably I should clean up STCKE at the same time, since it only wants a left shift of 4 bits instead of 12... r~
On 2018-12-03 15:29, Richard Henderson wrote: > On 12/3/18 8:02 AM, Thomas Huth wrote: >> On 2018-12-03 14:27, Richard Henderson wrote: >>> On 12/3/18 12:19 AM, Thomas Huth wrote: >>>> On 2018-11-30 20:22, Richard Henderson wrote: >>>>> This is a non-privileged instruction that was only implemented >>>>> for system mode. However, the stck instruction is used by glibc, >>>>> so this was causing SIGILL for programs run under debian stretch. >>>>> >>>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>>>> --- >>>>> target/s390x/helper.h | 2 +- >>>>> target/s390x/misc_helper.c | 13 ++++++++++++- >>>>> target/s390x/translate.c | 2 ++ >>>>> target/s390x/insn-data.def | 11 ++++++----- >>>>> 4 files changed, 21 insertions(+), 7 deletions(-) >>>>> >>>>> diff --git a/target/s390x/helper.h b/target/s390x/helper.h >>>>> index 018e9dd414..6260b50496 100644 >>>>> --- a/target/s390x/helper.h >>>>> +++ b/target/s390x/helper.h >>>>> @@ -121,13 +121,13 @@ DEF_HELPER_4(cu41, i32, env, i32, i32, i32) >>>>> DEF_HELPER_4(cu42, i32, env, i32, i32, i32) >>>>> DEF_HELPER_5(msa, i32, env, i32, i32, i32, i32) >>>>> DEF_HELPER_FLAGS_1(stpt, TCG_CALL_NO_RWG, i64, env) >>>>> +DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) >>>>> >>>>> #ifndef CONFIG_USER_ONLY >>>>> DEF_HELPER_3(servc, i32, env, i64, i64) >>>>> DEF_HELPER_4(diag, void, env, i32, i32, i32) >>>>> DEF_HELPER_3(load_psw, noreturn, env, i64, i64) >>>>> DEF_HELPER_FLAGS_2(spx, TCG_CALL_NO_RWG, void, env, i64) >>>>> -DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) >>>>> DEF_HELPER_FLAGS_2(sck, TCG_CALL_NO_RWG, i32, env, i64) >>>>> DEF_HELPER_FLAGS_2(sckc, TCG_CALL_NO_RWG, void, env, i64) >>>>> DEF_HELPER_FLAGS_2(sckpf, TCG_CALL_NO_RWG, void, env, i64) >>>>> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c >>>>> index 3f91579570..c2940afecb 100644 >>>>> --- a/target/s390x/misc_helper.c >>>>> +++ b/target/s390x/misc_helper.c >>>>> @@ -76,8 +76,19 @@ uint64_t HELPER(stpt)(CPUS390XState *env) >>>>> #endif >>>>> } >>>>> >>>>> -#ifndef CONFIG_USER_ONLY >>>>> +#ifdef CONFIG_USER_ONLY >>>>> +/* Store Clock */ >>>>> +uint64_t HELPER(stck)(CPUS390XState *env) >>>>> +{ >>>>> + struct timespec ts; >>>>> + uint64_t ms; >>>> >>>> May I suggest "us" for microseconds? "ms" makes me thing of >>>> "milliseconds"... >>> >>> Oops, think-o. >>> >>>>> + clock_gettime(CLOCK_REALTIME, &ts); >>>>> + ms = (ts.tv_nsec / 1000) + (ts.tv_sec * 100000ull); >>>>> + >>>>> + return TOD_UNIX_EPOCH + ms; >>>>> +} >>>> >>>> Don't you need to shift the value around a little bit for getting the >>>> right format of the TOD clock? The microseconds are not starting with >>>> the lowest bit, the TOD clock has a higher resolution. See also the >>>> time2tod() macro in include/hw/s390x/tod.h ... >>> >>> That's for STCKE, which does use this routine and shift the result around. >> >> STCKE certainly has even more bits ... but still, according to the PoP: >> >> "The TOD clock nominally is incremented by adding a >> one in bit position 51 every microsecond." > > I don't see that language within the description of STCK. It's in chapter 4 instead, the sub-section about "Timing". Thomas
On 03.12.18 14:30, Richard Henderson wrote: > On 12/3/18 4:21 AM, David Hildenbrand wrote: >> On 30.11.18 20:22, Richard Henderson wrote: >>> This is a non-privileged instruction that was only implemented >>> for system mode. However, the stck instruction is used by glibc, >>> so this was causing SIGILL for programs run under debian stretch. >>> >>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>> --- >>> target/s390x/helper.h | 2 +- >>> target/s390x/misc_helper.c | 13 ++++++++++++- >>> target/s390x/translate.c | 2 ++ >>> target/s390x/insn-data.def | 11 ++++++----- >>> 4 files changed, 21 insertions(+), 7 deletions(-) >>> >>> diff --git a/target/s390x/helper.h b/target/s390x/helper.h >>> index 018e9dd414..6260b50496 100644 >>> --- a/target/s390x/helper.h >>> +++ b/target/s390x/helper.h >>> @@ -121,13 +121,13 @@ DEF_HELPER_4(cu41, i32, env, i32, i32, i32) >>> DEF_HELPER_4(cu42, i32, env, i32, i32, i32) >>> DEF_HELPER_5(msa, i32, env, i32, i32, i32, i32) >>> DEF_HELPER_FLAGS_1(stpt, TCG_CALL_NO_RWG, i64, env) >>> +DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) >>> >>> #ifndef CONFIG_USER_ONLY >>> DEF_HELPER_3(servc, i32, env, i64, i64) >>> DEF_HELPER_4(diag, void, env, i32, i32, i32) >>> DEF_HELPER_3(load_psw, noreturn, env, i64, i64) >>> DEF_HELPER_FLAGS_2(spx, TCG_CALL_NO_RWG, void, env, i64) >>> -DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) >>> DEF_HELPER_FLAGS_2(sck, TCG_CALL_NO_RWG, i32, env, i64) >>> DEF_HELPER_FLAGS_2(sckc, TCG_CALL_NO_RWG, void, env, i64) >>> DEF_HELPER_FLAGS_2(sckpf, TCG_CALL_NO_RWG, void, env, i64) >>> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c >>> index 3f91579570..c2940afecb 100644 >>> --- a/target/s390x/misc_helper.c >>> +++ b/target/s390x/misc_helper.c >>> @@ -76,8 +76,19 @@ uint64_t HELPER(stpt)(CPUS390XState *env) >>> #endif >>> } >>> >>> -#ifndef CONFIG_USER_ONLY >>> +#ifdef CONFIG_USER_ONLY >>> +/* Store Clock */ >>> +uint64_t HELPER(stck)(CPUS390XState *env) >>> +{ >>> + struct timespec ts; >>> + uint64_t ms; >>> >>> + clock_gettime(CLOCK_REALTIME, &ts); >>> + ms = (ts.tv_nsec / 1000) + (ts.tv_sec * 100000ull); >>> + >>> + return TOD_UNIX_EPOCH + ms; >> >> In theory, the TOD can be completely controlled by the operating system >> (e.g. set the TOD to X). So for user space, there isn't really any >> guarantee about the values returned via stck. >> >> E.g. in the PoP 4-51: >> >> "4. A program using the clock value as a time-of-day >> and calendar indication must be consistent with >> the programming support under which the pro- >> gram is to be executed. [...]" > > Ok, but then there's the matter of the CC result. We currently hard-code this > as 0, meaning "clock is set", meaning it does have the real TOD value. Don't worry about cc/clock state handling. This is handling of old machine generations and will not happen on real machines nowadays (as far as I am aware). Clocks are always operating. CC is always 0. "Not-Set State: When the power for the clock is turned on, the clock is set to zero, and the clock enters the not-set state. The clock is incremented when in the not-set state. When the TOD-clock-steer- ing facility is installed, the TOD clock is never reported to be in the not-set state, as the TOD clock is placed in the set state as part of the initial- machine-loading (IML) process." So this is really only handling for operating systems that want to get the real time from the TOD (because with CC=1, the TOD would not match an actual date). Not for user space! And not for recent machine generations. When the OS changes the tod (set clock), the TOD will still be in "set state". User space has no guarantees here. > > We could set CC as 1, meaning "clock is not set", meaning the value is only > good for relative computation. > > Is this perhaps a bug in our system implementation as well? Don't think so, returning always 0 should be fine. > > What CC value is provided to userspace on real hardware? It will always see CC=0 as far as I know. E.g. in KVM, after migration the guest TOD might differ to the actual system TOD. But we will still get CC=0. CC != 0 is a relict from the past. I guess the important part here is not to confuse "CC==0" with "the TOD is synced to time source X". It is just internal state handling for bringing up the clock source + error handling. > > > r~ > -- Thanks, David / dhildenb
On 03.12.18 15:02, Thomas Huth wrote: > On 2018-12-03 14:27, Richard Henderson wrote: >> On 12/3/18 12:19 AM, Thomas Huth wrote: >>> On 2018-11-30 20:22, Richard Henderson wrote: >>>> This is a non-privileged instruction that was only implemented >>>> for system mode. However, the stck instruction is used by glibc, >>>> so this was causing SIGILL for programs run under debian stretch. >>>> >>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>>> --- >>>> target/s390x/helper.h | 2 +- >>>> target/s390x/misc_helper.c | 13 ++++++++++++- >>>> target/s390x/translate.c | 2 ++ >>>> target/s390x/insn-data.def | 11 ++++++----- >>>> 4 files changed, 21 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/target/s390x/helper.h b/target/s390x/helper.h >>>> index 018e9dd414..6260b50496 100644 >>>> --- a/target/s390x/helper.h >>>> +++ b/target/s390x/helper.h >>>> @@ -121,13 +121,13 @@ DEF_HELPER_4(cu41, i32, env, i32, i32, i32) >>>> DEF_HELPER_4(cu42, i32, env, i32, i32, i32) >>>> DEF_HELPER_5(msa, i32, env, i32, i32, i32, i32) >>>> DEF_HELPER_FLAGS_1(stpt, TCG_CALL_NO_RWG, i64, env) >>>> +DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) >>>> >>>> #ifndef CONFIG_USER_ONLY >>>> DEF_HELPER_3(servc, i32, env, i64, i64) >>>> DEF_HELPER_4(diag, void, env, i32, i32, i32) >>>> DEF_HELPER_3(load_psw, noreturn, env, i64, i64) >>>> DEF_HELPER_FLAGS_2(spx, TCG_CALL_NO_RWG, void, env, i64) >>>> -DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) >>>> DEF_HELPER_FLAGS_2(sck, TCG_CALL_NO_RWG, i32, env, i64) >>>> DEF_HELPER_FLAGS_2(sckc, TCG_CALL_NO_RWG, void, env, i64) >>>> DEF_HELPER_FLAGS_2(sckpf, TCG_CALL_NO_RWG, void, env, i64) >>>> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c >>>> index 3f91579570..c2940afecb 100644 >>>> --- a/target/s390x/misc_helper.c >>>> +++ b/target/s390x/misc_helper.c >>>> @@ -76,8 +76,19 @@ uint64_t HELPER(stpt)(CPUS390XState *env) >>>> #endif >>>> } >>>> >>>> -#ifndef CONFIG_USER_ONLY >>>> +#ifdef CONFIG_USER_ONLY >>>> +/* Store Clock */ >>>> +uint64_t HELPER(stck)(CPUS390XState *env) >>>> +{ >>>> + struct timespec ts; >>>> + uint64_t ms; >>> >>> May I suggest "us" for microseconds? "ms" makes me thing of >>> "milliseconds"... >> >> Oops, think-o. >> >>>> + clock_gettime(CLOCK_REALTIME, &ts); >>>> + ms = (ts.tv_nsec / 1000) + (ts.tv_sec * 100000ull); >>>> + >>>> + return TOD_UNIX_EPOCH + ms; >>>> +} >>> >>> Don't you need to shift the value around a little bit for getting the >>> right format of the TOD clock? The microseconds are not starting with >>> the lowest bit, the TOD clock has a higher resolution. See also the >>> time2tod() macro in include/hw/s390x/tod.h ... I agree, this needs time2tod(). By definition, the TOD is stepped like this. STCK/STCKE just provide different means to that value. But the stepping remains the same. > > Thomas > -- Thanks, David / dhildenb
diff --git a/target/s390x/helper.h b/target/s390x/helper.h index 018e9dd414..6260b50496 100644 --- a/target/s390x/helper.h +++ b/target/s390x/helper.h @@ -121,13 +121,13 @@ DEF_HELPER_4(cu41, i32, env, i32, i32, i32) DEF_HELPER_4(cu42, i32, env, i32, i32, i32) DEF_HELPER_5(msa, i32, env, i32, i32, i32, i32) DEF_HELPER_FLAGS_1(stpt, TCG_CALL_NO_RWG, i64, env) +DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) #ifndef CONFIG_USER_ONLY DEF_HELPER_3(servc, i32, env, i64, i64) DEF_HELPER_4(diag, void, env, i32, i32, i32) DEF_HELPER_3(load_psw, noreturn, env, i64, i64) DEF_HELPER_FLAGS_2(spx, TCG_CALL_NO_RWG, void, env, i64) -DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env) DEF_HELPER_FLAGS_2(sck, TCG_CALL_NO_RWG, i32, env, i64) DEF_HELPER_FLAGS_2(sckc, TCG_CALL_NO_RWG, void, env, i64) DEF_HELPER_FLAGS_2(sckpf, TCG_CALL_NO_RWG, void, env, i64) diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c index 3f91579570..c2940afecb 100644 --- a/target/s390x/misc_helper.c +++ b/target/s390x/misc_helper.c @@ -76,8 +76,19 @@ uint64_t HELPER(stpt)(CPUS390XState *env) #endif } -#ifndef CONFIG_USER_ONLY +#ifdef CONFIG_USER_ONLY +/* Store Clock */ +uint64_t HELPER(stck)(CPUS390XState *env) +{ + struct timespec ts; + uint64_t ms; + clock_gettime(CLOCK_REALTIME, &ts); + ms = (ts.tv_nsec / 1000) + (ts.tv_sec * 100000ull); + + return TOD_UNIX_EPOCH + ms; +} +#else /* SCLP service call */ uint32_t HELPER(servc)(CPUS390XState *env, uint64_t r1, uint64_t r2) { diff --git a/target/s390x/translate.c b/target/s390x/translate.c index b5bd56b7ee..8bb9c0529a 100644 --- a/target/s390x/translate.c +++ b/target/s390x/translate.c @@ -4010,6 +4010,7 @@ static DisasJumpType op_stap(DisasContext *s, DisasOps *o) tcg_gen_ld32u_i64(o->out, cpu_env, offsetof(CPUS390XState, core_id)); return DISAS_NEXT; } +#endif static DisasJumpType op_stck(DisasContext *s, DisasOps *o) { @@ -4046,6 +4047,7 @@ static DisasJumpType op_stcke(DisasContext *s, DisasOps *o) return DISAS_NEXT; } +#ifndef CONFIG_USER_ONLY static DisasJumpType op_sck(DisasContext *s, DisasOps *o) { tcg_gen_qemu_ld_i64(o->in1, o->addr1, get_mem_index(s), MO_TEQ | MO_ALIGN); diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def index 54e39df831..922227bada 100644 --- a/target/s390x/insn-data.def +++ b/target/s390x/insn-data.def @@ -837,6 +837,12 @@ C(0xe33e, STRV, RXY_a, Z, la2, r1_32u, new, m1_32, rev32, 0) C(0xe32f, STRVG, RXY_a, Z, la2, r1_o, new, m1_64, rev64, 0) +/* STORE CLOCK */ + C(0xb205, STCK, S, Z, la2, 0, new, m1_64, stck, 0) + C(0xb27c, STCKF, S, SCF, la2, 0, new, m1_64, stck, 0) +/* STORE CLOCK EXTENDED */ + C(0xb278, STCKE, S, Z, 0, a2, 0, 0, stcke, 0) + /* STORE FACILITY LIST EXTENDED */ C(0xb2b0, STFLE, S, SFLE, 0, a2, 0, 0, stfle, 0) /* STORE FPC */ @@ -1020,11 +1026,6 @@ F(0x8000, SSM, S, Z, 0, m2_8u, 0, 0, ssm, 0, IF_PRIV) /* SIGNAL PROCESSOR */ F(0xae00, SIGP, RS_a, Z, 0, a2, 0, 0, sigp, 0, IF_PRIV) -/* STORE CLOCK */ - C(0xb205, STCK, S, Z, la2, 0, new, m1_64, stck, 0) - C(0xb27c, STCKF, S, SCF, la2, 0, new, m1_64, stck, 0) -/* STORE CLOCK EXTENDED */ - C(0xb278, STCKE, S, Z, 0, a2, 0, 0, stcke, 0) /* STORE CLOCK COMPARATOR */ F(0xb207, STCKC, S, Z, la2, 0, new, m1_64a, stckc, 0, IF_PRIV) /* STORE CONTROL */
This is a non-privileged instruction that was only implemented for system mode. However, the stck instruction is used by glibc, so this was causing SIGILL for programs run under debian stretch. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/s390x/helper.h | 2 +- target/s390x/misc_helper.c | 13 ++++++++++++- target/s390x/translate.c | 2 ++ target/s390x/insn-data.def | 11 ++++++----- 4 files changed, 21 insertions(+), 7 deletions(-) -- 2.17.2