Re: [PATCH] [v3] x86: Convert x86_platform_ops to timespec64

[Arnd Bergmann]

On Sat, Apr 28, 2018 at 12:21 AM, Joao Martins
<joao.m.martins@xxxxxxxxxx> wrote:
> On 04/27/2018 09:13 PM, Arnd Bergmann wrote:
>> diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c
>> index 761f6af6efa5..637982efecd8 100644
>> --- a/arch/x86/kernel/pvclock.c
>> +++ b/arch/x86/kernel/pvclock.c
>> @@ -123,28 +123,35 @@ u64 pvclock_clocksource_read(struct pvclock_vcpu_time_info *src)
>>
>>  void pvclock_read_wallclock(struct pvclock_wall_clock *wall_clock,
>>                           struct pvclock_vcpu_time_info *vcpu_time,
>> -                         struct timespec *ts)
>> +                         struct timespec64 *ts)
>>  {
>>       u32 version;
>>       u64 delta;
>> -     struct timespec now;
>> +     struct timespec64 now;
>>
>>       /* get wallclock at system boot */
>>       do {
>>               version = wall_clock->version;
>>               rmb();          /* fetch version before time */
>> +             /*
>> +              * Note: wall_clock->sec is a u32 value, so it can
>> +              * only store dates between 1970 and 2106. To allow
>> +              * times beyond that, we need to create a new hypercall
>> +              * interface with an extended pvclock_wall_clock structure
>> +              * like ARM has.
>> +              */
>>               now.tv_sec  = wall_clock->sec;
>
> IIUC the interface you're probably speaking about is common to both ARM and x86
> on Xen[*] (since Xen 4.6) i.e.
>
>         now.tv_sec  = ((uint64_t)s->wc_sec_hi << 32) | s->wc_sec;
>
> s representing struct shared_info like on ARM (there's a 32-bit hole where
> wc_sec_hi is placed on x86_64/ARM). Except on x86 32-bit guests wc_sec_hi is
> located elsewhere.
>
>         Joao
>
> [*]
> https://xenbits.xen.org/docs/4.6-testing/hypercall/x86_64/include,public,xen.h.html#incontents_startofday_shared

Ah, good. How portable is that? Will it do the right thing (i.e.
guarantee to have
zeroes on the upper half, or the epoch if supported) on all versions of both KVM
and Xen, or do we need an additional check in there?

I'd suggest leaving the implementation of that to a follow-up patch that you
can add once my patch is merged.

        Arnd