Re: [Qemu-devel] [PATCH for-4.0 2/2] target/s390x: Implement STCK et al for CONFIG_USER_ONLY

[David Hildenbrand]

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 <address@hidden>
>>> ---
>>>  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