Re: [Xen-devel] [PATCH] x86: convert x86_platform_ops to timespec64

2017-10-16 Thread Paolo Bonzini 
On 16/10/2017 14:16, Arnd Bergmann wrote:
> On Mon, Oct 16, 2017 at 2:08 PM, Paolo Bonzini  wrote:
>> On 16/10/2017 10:11, Arnd Bergmann wrote:
>>> Thanks!
>>>
>>> Since you've looked at it overall, do you have an opinion on the question
>>> how to fix the PV interface to deal with the pvclock_wall_clock overflow?
>>
>> It has to be done separately for each hypervisor.
>>
>> In KVM, for example, it is probably best to abandon
>> pvclock_read_wallclock altogether, and instead use the recently
>> introduced KVM_HC_CLOCK_PAIRING hypercall.  drivers/ptp/ptp_kvm.c is
>> already using it and it's y2106 safe.
> 
> Right, makes sense. I see that this interface is currently implemented
> only for 64-bit x86 in kvm_emulate_hypercall(). Could this be extended
> to x86-32 and the non-x86 architectures as well?

Yes, it could be implemented for x86-32 too.  The whole pvclock concept
however is specific to x86.

Paolo

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86: convert x86_platform_ops to timespec64

2017-10-16 Thread Arnd Bergmann 
On Mon, Oct 16, 2017 at 2:08 PM, Paolo Bonzini  wrote:
> On 16/10/2017 10:11, Arnd Bergmann wrote:
>> Thanks!
>>
>> Since you've looked at it overall, do you have an opinion on the question
>> how to fix the PV interface to deal with the pvclock_wall_clock overflow?
>
> It has to be done separately for each hypervisor.
>
> In KVM, for example, it is probably best to abandon
> pvclock_read_wallclock altogether, and instead use the recently
> introduced KVM_HC_CLOCK_PAIRING hypercall.  drivers/ptp/ptp_kvm.c is
> already using it and it's y2106 safe.

Right, makes sense. I see that this interface is currently implemented
only for 64-bit x86 in kvm_emulate_hypercall(). Could this be extended
to x86-32 and the non-x86 architectures as well?

   Arnd

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86: convert x86_platform_ops to timespec64

2017-10-16 Thread Paolo Bonzini 
On 16/10/2017 10:11, Arnd Bergmann wrote:
> Thanks!
> 
> Since you've looked at it overall, do you have an opinion on the question
> how to fix the PV interface to deal with the pvclock_wall_clock overflow?

It has to be done separately for each hypervisor.

In KVM, for example, it is probably best to abandon
pvclock_read_wallclock altogether, and instead use the recently
introduced KVM_HC_CLOCK_PAIRING hypercall.  drivers/ptp/ptp_kvm.c is
already using it and it's y2106 safe.

Paolo

> Should we add a new version now and deprecate the existing one, or
> do you think that y2106 is far enough out that we should just ignore the
> problem?


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86: convert x86_platform_ops to timespec64

2017-10-16 Thread Arnd Bergmann 
On Fri, Oct 13, 2017 at 10:45 PM, Boris Ostrovsky
 wrote:
> On 10/13/2017 02:37 PM, Arnd Bergmann wrote:
>> The x86 platform operations are fairly isolated, so we can
>> change them from using timespec to timespec64. I checked that
>> All the users and callers are safe, and there is only one
>> critical function that is broken beyond 2106:
>>
>> pvclock_read_wallclock() uses a 32-bit number of seconds since
>> the epoch to communicate the boot time between host and guest
>> in a virtual environment. This will work until 2106, but we
>> should ideally find a replacement anyway. I've added a comment
>> about it there.
>>
>> Signed-off-by: Arnd Bergmann 
>> ---
>>  arch/x86/include/asm/intel_mid_vrtc.h|  4 ++--
>>  arch/x86/include/asm/mc146818rtc.h   |  4 ++--
>>  arch/x86/include/asm/pvclock.h   |  2 +-
>>  arch/x86/include/asm/x86_init.h  |  6 +++---
>>  arch/x86/kernel/kvmclock.c   |  4 ++--
>>  arch/x86/kernel/pvclock.c| 12 +---
>>  arch/x86/kernel/rtc.c| 16 
>>  arch/x86/platform/intel-mid/intel_mid_vrtc.c | 10 +-
>>  arch/x86/xen/time.c  | 10 +-
>>  9 files changed, 37 insertions(+), 31 deletions(-)
>
> Xen bits:
> Reviewed-by: Boris Ostrovsky 

Thanks!

Since you've looked at it overall, do you have an opinion on the question
how to fix the PV interface to deal with the pvclock_wall_clock overflow?

Should we add a new version now and deprecate the existing one, or
do you think that y2106 is far enough out that we should just ignore the
problem?

> with a couple of nits:
>
>> @@ -136,11 +136,17 @@ void pvclock_read_wallclock(struct pvclock_wall_clock 
>> *wall_clock,
>>   rmb();  /* fetch time before checking version */
>>   } while ((wall_clock->version & 1) || (version != 
>> wall_clock->version));
>>
>> + /*
>> +  * 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.
>> +  */
>
> I think this comment block should be moved up above 'now.tv_sec  =
> wall_clock->sec;'

right, changed.

>>   delta = pvclock_clocksource_read(vcpu_time);/* time since system 
>> boot */
>>   delta += now.tv_sec * (u64)NSEC_PER_SEC + now.tv_nsec;
>
> Now that tv_sec is a 64-bit quantity the cast can be dropped.

Ok dropped. In the meantime I had noticed two more problems with the
patch that I did not see earlier when I tested with another patch applied
as well. The kbuild test robot reported the exact same problems, and I've
done a few hundred randconfig builds without the other patch now, so
I'm fairly confident that there are no other problems like those.

I'll follow up with a v2 patch soon.

   Arnd

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86: convert x86_platform_ops to timespec64

2017-10-15 Thread kbuild test robot 
Hi Arnd,

[auto build test ERROR on tip/x86/core]
[also build test ERROR on v4.14-rc5 next-20171013]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Arnd-Bergmann/x86-convert-x86_platform_ops-to-timespec64/20171016-091601
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   arch/x86/xen/time.c: In function 'xen_init_time_ops':
>> arch/x86/xen/time.c:418:29: error: assignment from incompatible pointer type 
>> [-Werror=incompatible-pointer-types]
 x86_platform.get_wallclock = xen_get_wallclock;
^
   arch/x86/xen/time.c:421:30: error: assignment from incompatible pointer type 
[-Werror=incompatible-pointer-types]
  x86_platform.set_wallclock = xen_set_wallclock;
 ^
   arch/x86/xen/time.c: In function 'xen_hvm_init_time_ops':
   arch/x86/xen/time.c:458:29: error: assignment from incompatible pointer type 
[-Werror=incompatible-pointer-types]
 x86_platform.get_wallclock = xen_get_wallclock;
^
   arch/x86/xen/time.c:459:29: error: assignment from incompatible pointer type 
[-Werror=incompatible-pointer-types]
 x86_platform.set_wallclock = xen_set_wallclock;
^
   cc1: some warnings being treated as errors
--
   In file included from include/linux/printk.h:6:0,
from include/linux/kernel.h:13,
from arch/x86/platform/intel-mid/intel_mid_vrtc.c:20:
   arch/x86/platform/intel-mid/intel_mid_vrtc.c: In function 'vrtc_set_mmss':
>> include/linux/kern_levels.h:4:18: warning: format '%llx' expects argument of 
>> type 'long long unsigned int', but argument 3 has type '__kernel_time_t {aka 
>> const long int}' [-Wformat=]
#define KERN_SOH "\001"  /* ASCII Start Of Header */
 ^
   include/linux/kern_levels.h:10:18: note: in expansion of macro 'KERN_SOH'
#define KERN_ERR KERN_SOH "3" /* error conditions */
 ^~~~
   include/linux/printk.h:301:9: note: in expansion of macro 'KERN_ERR'
 printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~
>> arch/x86/platform/intel-mid/intel_mid_vrtc.c:113:3: note: in expansion of 
>> macro 'pr_err'
  pr_err("%s: Invalid vRTC value: write of %llx to vRTC failed\n",
  ^~
--
   In file included from include/linux/printk.h:6:0,
from include/linux/kernel.h:13,
from arch/x86//platform/intel-mid/intel_mid_vrtc.c:20:
   arch/x86//platform/intel-mid/intel_mid_vrtc.c: In function 'vrtc_set_mmss':
>> include/linux/kern_levels.h:4:18: warning: format '%llx' expects argument of 
>> type 'long long unsigned int', but argument 3 has type '__kernel_time_t {aka 
>> const long int}' [-Wformat=]
#define KERN_SOH "\001"  /* ASCII Start Of Header */
 ^
   include/linux/kern_levels.h:10:18: note: in expansion of macro 'KERN_SOH'
#define KERN_ERR KERN_SOH "3" /* error conditions */
 ^~~~
   include/linux/printk.h:301:9: note: in expansion of macro 'KERN_ERR'
 printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~
   arch/x86//platform/intel-mid/intel_mid_vrtc.c:113:3: note: in expansion of 
macro 'pr_err'
  pr_err("%s: Invalid vRTC value: write of %llx to vRTC failed\n",
  ^~

vim +418 arch/x86/xen/time.c

409771d2 Stefano Stabellini 2010-05-14  408  
d162809f Boris Ostrovsky2017-05-03  409  void __ref xen_init_time_ops(void)
409771d2 Stefano Stabellini 2010-05-14  410  {
409771d2 Stefano Stabellini 2010-05-14  411 pv_time_ops = xen_time_ops;
409771d2 Stefano Stabellini 2010-05-14  412  
409771d2 Stefano Stabellini 2010-05-14  413 x86_init.timers.timer_init = 
xen_time_init;
409771d2 Stefano Stabellini 2010-05-14  414 
x86_init.timers.setup_percpu_clockev = x86_init_noop;
409771d2 Stefano Stabellini 2010-05-14  415 
x86_cpuinit.setup_percpu_clockev = x86_init_noop;
409771d2 Stefano Stabellini 2010-05-14  416  
409771d2 Stefano Stabellini 2010-05-14  417 x86_platform.calibrate_tsc = 
xen_tsc_khz;
409771d2 Stefano Stabellini 2010-05-14 @418 x86_platform.get_wallclock = 
xen_get_wallclock;
47433b8c David Vrabel   2013-06-27  419 /* Dom0 uses the native method 
to set the hardware RTC. */
47433b8c David Vrabel   2013-06-27  420 if (!xen_initial_domain())
409771d2 Stefano Stabellini 2010-05-14  421 
x86_platform.set_wallclock = xen_set_wallclock;
409771d2 Stefano Stabellini 2010-05-14  422  }
409771d2 Stefano Stabellini 2010-05-14  423  

:: The code at line 418 was first introduced by commit
:: 409771d258e9dd71c30f3c9520fd2b796ffc40f0 x86: Use xen_vcpuop_clockevent, 
xen_clocksource and xen

Re: [Xen-devel] [PATCH] x86: convert x86_platform_ops to timespec64

2017-10-13 Thread Boris Ostrovsky 
On 10/13/2017 02:37 PM, Arnd Bergmann wrote:
> The x86 platform operations are fairly isolated, so we can
> change them from using timespec to timespec64. I checked that
> All the users and callers are safe, and there is only one
> critical function that is broken beyond 2106:
>
> pvclock_read_wallclock() uses a 32-bit number of seconds since
> the epoch to communicate the boot time between host and guest
> in a virtual environment. This will work until 2106, but we
> should ideally find a replacement anyway. I've added a comment
> about it there.
>
> Signed-off-by: Arnd Bergmann 
> ---
>  arch/x86/include/asm/intel_mid_vrtc.h|  4 ++--
>  arch/x86/include/asm/mc146818rtc.h   |  4 ++--
>  arch/x86/include/asm/pvclock.h   |  2 +-
>  arch/x86/include/asm/x86_init.h  |  6 +++---
>  arch/x86/kernel/kvmclock.c   |  4 ++--
>  arch/x86/kernel/pvclock.c| 12 +---
>  arch/x86/kernel/rtc.c| 16 
>  arch/x86/platform/intel-mid/intel_mid_vrtc.c | 10 +-
>  arch/x86/xen/time.c  | 10 +-
>  9 files changed, 37 insertions(+), 31 deletions(-)

Xen bits:
Reviewed-by: Boris Ostrovsky 

with a couple of nits:

> @@ -136,11 +136,17 @@ void pvclock_read_wallclock(struct pvclock_wall_clock 
> *wall_clock,
>   rmb();  /* fetch time before checking version */
>   } while ((wall_clock->version & 1) || (version != wall_clock->version));
>  
> + /*
> +  * 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.
> +  */

I think this comment block should be moved up above 'now.tv_sec  =
wall_clock->sec;'


>   delta = pvclock_clocksource_read(vcpu_time);/* time since system 
> boot */
>   delta += now.tv_sec * (u64)NSEC_PER_SEC + now.tv_nsec;

Now that tv_sec is a 64-bit quantity the cast can be dropped.

-boris

>  
>   now.tv_nsec = do_div(delta, NSEC_PER_SEC);
>   now.tv_sec = delta;
>  
> - set_normalized_timespec(ts, now.tv_sec, now.tv_nsec);
> + set_normalized_timespec64(ts, now.tv_sec, now.tv_nsec);
>  }
>


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH] x86: convert x86_platform_ops to timespec64

2017-10-13 Thread Arnd Bergmann 
The x86 platform operations are fairly isolated, so we can
change them from using timespec to timespec64. I checked that
All the users and callers are safe, and there is only one
critical function that is broken beyond 2106:

pvclock_read_wallclock() uses a 32-bit number of seconds since
the epoch to communicate the boot time between host and guest
in a virtual environment. This will work until 2106, but we
should ideally find a replacement anyway. I've added a comment
about it there.

Signed-off-by: Arnd Bergmann 
---
 arch/x86/include/asm/intel_mid_vrtc.h|  4 ++--
 arch/x86/include/asm/mc146818rtc.h   |  4 ++--
 arch/x86/include/asm/pvclock.h   |  2 +-
 arch/x86/include/asm/x86_init.h  |  6 +++---
 arch/x86/kernel/kvmclock.c   |  4 ++--
 arch/x86/kernel/pvclock.c| 12 +---
 arch/x86/kernel/rtc.c| 16 
 arch/x86/platform/intel-mid/intel_mid_vrtc.c | 10 +-
 arch/x86/xen/time.c  | 10 +-
 9 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/arch/x86/include/asm/intel_mid_vrtc.h 
b/arch/x86/include/asm/intel_mid_vrtc.h
index 86ff4685c409..19202320f0be 100644
--- a/arch/x86/include/asm/intel_mid_vrtc.h
+++ b/arch/x86/include/asm/intel_mid_vrtc.h
@@ -3,7 +3,7 @@
 
 extern unsigned char vrtc_cmos_read(unsigned char reg);
 extern void vrtc_cmos_write(unsigned char val, unsigned char reg);
-extern void vrtc_get_time(struct timespec *now);
-extern int vrtc_set_mmss(const struct timespec *now);
+extern void vrtc_get_time(struct timespec64 *now);
+extern int vrtc_set_mmss(const struct timespec64 *now);
 
 #endif
diff --git a/arch/x86/include/asm/mc146818rtc.h 
b/arch/x86/include/asm/mc146818rtc.h
index 24acd9ba7837..1b574e5eb3b2 100644
--- a/arch/x86/include/asm/mc146818rtc.h
+++ b/arch/x86/include/asm/mc146818rtc.h
@@ -94,8 +94,8 @@ static inline unsigned char current_lock_cmos_reg(void)
 unsigned char rtc_cmos_read(unsigned char addr);
 void rtc_cmos_write(unsigned char val, unsigned char addr);
 
-extern int mach_set_rtc_mmss(const struct timespec *now);
-extern void mach_get_cmos_time(struct timespec *now);
+extern int mach_set_rtc_mmss(const struct timespec64 *now);
+extern void mach_get_cmos_time(struct timespec64 *now);
 
 #define RTC_IRQ 8
 
diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h
index 448cfe1b48cf..fc3138fd3aff 100644
--- a/arch/x86/include/asm/pvclock.h
+++ b/arch/x86/include/asm/pvclock.h
@@ -20,7 +20,7 @@ void pvclock_set_flags(u8 flags);
 unsigned long pvclock_tsc_khz(struct pvclock_vcpu_time_info *src);
 void pvclock_read_wallclock(struct pvclock_wall_clock *wall,
struct pvclock_vcpu_time_info *vcpu,
-   struct timespec *ts);
+   struct timespec64 *ts);
 void pvclock_resume(void);
 
 void pvclock_touch_watchdogs(void);
diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index f45acdf45957..0c5007b72916 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -141,7 +141,7 @@ struct x86_cpuinit_ops {
void (*fixup_cpu_id)(struct cpuinfo_x86 *c, int node);
 };
 
-struct timespec;
+struct timespec64;
 
 /**
  * struct x86_legacy_devices - legacy x86 devices
@@ -223,8 +223,8 @@ struct x86_legacy_features {
 struct x86_platform_ops {
unsigned long (*calibrate_cpu)(void);
unsigned long (*calibrate_tsc)(void);
-   void (*get_wallclock)(struct timespec *ts);
-   int (*set_wallclock)(const struct timespec *ts);
+   void (*get_wallclock)(struct timespec64 *ts);
+   int (*set_wallclock)(const struct timespec64 *ts);
void (*iommu_shutdown)(void);
bool (*is_untracked_pat_range)(u64 start, u64 end);
void (*nmi_init)(void);
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index d88967659098..01c76e8cd4be 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -58,7 +58,7 @@ EXPORT_SYMBOL_GPL(pvclock_pvti_cpu0_va);
  * have elapsed since the hypervisor wrote the data. So we try to account for
  * that with system time
  */
-static void kvm_get_wallclock(struct timespec *now)
+static void kvm_get_wallclock(struct timespec64 *now)
 {
struct pvclock_vcpu_time_info *vcpu_time;
int low, high;
@@ -77,7 +77,7 @@ static void kvm_get_wallclock(struct timespec *now)
put_cpu();
 }
 
-static int kvm_set_wallclock(const struct timespec *now)
+static int kvm_set_wallclock(const struct timespec64 *now)
 {
return -1;
 }
diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c
index 5c3f6d6a5078..1a7084ad07b9 100644
--- a/arch/x86/kernel/pvclock.c
+++ b/arch/x86/kernel/pvclock.c
@@ -121,11 +121,11 @@ u64 pvclock_clocksource_read(struct 
pvclock_vcpu_time_info *src)
 
 void pvclock_read_wallclock(struct pvclock_wall_clock *wall_clock,