diff mbox

[v3,3/6] xen: introduce XENPF_settime64

Message ID 1447260696-450-3-git-send-email-stefano.stabellini@eu.citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Stefano Stabellini Nov. 11, 2015, 4:51 p.m. UTC
Rename the current XENPF_settime hypercall and related struct to
XENPF_settime32.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
CC: konrad.wilk@oracle.com
CC: david.vrabel@citrix.com
CC: boris.ostrovsky@oracle.com
---
 arch/x86/xen/time.c              |    8 ++++----
 include/xen/interface/platform.h |   18 ++++++++++++++----
 2 files changed, 18 insertions(+), 8 deletions(-)

Comments

Boris Ostrovsky Nov. 12, 2015, 3:30 p.m. UTC | #1
On 11/11/2015 11:51 AM, Stefano Stabellini wrote:
> Rename the current XENPF_settime hypercall and related struct to
> XENPF_settime32.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> CC: konrad.wilk@oracle.com
> CC: david.vrabel@citrix.com
> CC: boris.ostrovsky@oracle.com
> ---
>   arch/x86/xen/time.c              |    8 ++++----
>   include/xen/interface/platform.h |   18 ++++++++++++++----
>   2 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> index 663c2ea..3bbd377 100644
> --- a/arch/x86/xen/time.c
> +++ b/arch/x86/xen/time.c
> @@ -134,10 +134,10 @@ static int xen_pvclock_gtod_notify(struct notifier_block *nb,
>   	if (!was_set && timespec_compare(&now, &next_sync) < 0)
>   		return NOTIFY_OK;
>   
> -	op.cmd = XENPF_settime;
> -	op.u.settime.secs = now.tv_sec;
> -	op.u.settime.nsecs = now.tv_nsec;
> -	op.u.settime.system_time = xen_clocksource_read();
> +	op.cmd = XENPF_settime32;
> +	op.u.settime32.secs = now.tv_sec;
> +	op.u.settime32.nsecs = now.tv_nsec;
> +	op.u.settime32.system_time = xen_clocksource_read();

Can/should we switch to time64 here? (This would require a couple more 
changes but they would all be local to this routine).

-boris


>   
>   	(void)HYPERVISOR_platform_op(&op);
>   
> diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
> index 8e03587..732efb0 100644
> --- a/include/xen/interface/platform.h
> +++ b/include/xen/interface/platform.h
> @@ -35,14 +35,23 @@
>    * Set clock such that it would read <secs,nsecs> after 00:00:00 UTC,
>    * 1 January, 1970 if the current system time was <system_time>.
>    */
> -#define XENPF_settime             17
> -struct xenpf_settime {
> +#define XENPF_settime32             17
> +struct xenpf_settime32 {
>   	/* IN variables. */
>   	uint32_t secs;
>   	uint32_t nsecs;
>   	uint64_t system_time;
>   };
> -DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime_t);
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime32_t);
> +#define XENPF_settime64           62
> +struct xenpf_settime64 {
> +    /* IN variables. */
> +    uint64_t secs;
> +    uint32_t nsecs;
> +    uint32_t mbz;
> +    uint64_t system_time;
> +};
> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime64_t);
>   
>   /*
>    * Request memory range (@mfn, @mfn+@nr_mfns-1) to have type @type.
> @@ -495,7 +504,8 @@ struct xen_platform_op {
>   	uint32_t cmd;
>   	uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
>   	union {
> -		struct xenpf_settime           settime;
> +		struct xenpf_settime32         settime32;
> +		struct xenpf_settime64         settime64;
>   		struct xenpf_add_memtype       add_memtype;
>   		struct xenpf_del_memtype       del_memtype;
>   		struct xenpf_read_memtype      read_memtype;
Arnd Bergmann Nov. 12, 2015, 4:10 p.m. UTC | #2
On Thursday 12 November 2015 10:30:23 Boris Ostrovsky wrote:
> > diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> > index 663c2ea..3bbd377 100644
> > --- a/arch/x86/xen/time.c
> > +++ b/arch/x86/xen/time.c
> > @@ -134,10 +134,10 @@ static int xen_pvclock_gtod_notify(struct notifier_block *nb,
> >       if (!was_set && timespec_compare(&now, &next_sync) < 0)
> >               return NOTIFY_OK;
> >   
> > -     op.cmd = XENPF_settime;
> > -     op.u.settime.secs = now.tv_sec;
> > -     op.u.settime.nsecs = now.tv_nsec;
> > -     op.u.settime.system_time = xen_clocksource_read();
> > +     op.cmd = XENPF_settime32;
> > +     op.u.settime32.secs = now.tv_sec;
> > +     op.u.settime32.nsecs = now.tv_nsec;
> > +     op.u.settime32.system_time = xen_clocksource_read();
> 
> Can/should we switch to time64 here? (This would require a couple more 
> changes but they would all be local to this routine).

We definitely should. We are in the process of removing all uses of
timespec from the kernel in favor of timespec64, and this requires
changing the Xen code as well if we want to do it right. I suppose that
both Dom0 and DomU will have to support the old and the new interface
for x86, so we have a fallback if the 64-bit interface fails.

	Arnd
Stefano Stabellini Nov. 12, 2015, 4:34 p.m. UTC | #3
On Thu, 12 Nov 2015, Boris Ostrovsky wrote:
> On 11/11/2015 11:51 AM, Stefano Stabellini wrote:
> > Rename the current XENPF_settime hypercall and related struct to
> > XENPF_settime32.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > CC: konrad.wilk@oracle.com
> > CC: david.vrabel@citrix.com
> > CC: boris.ostrovsky@oracle.com
> > ---
> >   arch/x86/xen/time.c              |    8 ++++----
> >   include/xen/interface/platform.h |   18 ++++++++++++++----
> >   2 files changed, 18 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> > index 663c2ea..3bbd377 100644
> > --- a/arch/x86/xen/time.c
> > +++ b/arch/x86/xen/time.c
> > @@ -134,10 +134,10 @@ static int xen_pvclock_gtod_notify(struct
> > notifier_block *nb,
> >   	if (!was_set && timespec_compare(&now, &next_sync) < 0)
> >   		return NOTIFY_OK;
> >   -	op.cmd = XENPF_settime;
> > -	op.u.settime.secs = now.tv_sec;
> > -	op.u.settime.nsecs = now.tv_nsec;
> > -	op.u.settime.system_time = xen_clocksource_read();
> > +	op.cmd = XENPF_settime32;
> > +	op.u.settime32.secs = now.tv_sec;
> > +	op.u.settime32.nsecs = now.tv_nsec;
> > +	op.u.settime32.system_time = xen_clocksource_read();
> 
> Can/should we switch to time64 here? (This would require a couple more changes
> but they would all be local to this routine).

I can do that, but it should be a separate patch. I'll queue it at the
end of the series.


> 
> >     	(void)HYPERVISOR_platform_op(&op);
> >   diff --git a/include/xen/interface/platform.h
> > b/include/xen/interface/platform.h
> > index 8e03587..732efb0 100644
> > --- a/include/xen/interface/platform.h
> > +++ b/include/xen/interface/platform.h
> > @@ -35,14 +35,23 @@
> >    * Set clock such that it would read <secs,nsecs> after 00:00:00 UTC,
> >    * 1 January, 1970 if the current system time was <system_time>.
> >    */
> > -#define XENPF_settime             17
> > -struct xenpf_settime {
> > +#define XENPF_settime32             17
> > +struct xenpf_settime32 {
> >   	/* IN variables. */
> >   	uint32_t secs;
> >   	uint32_t nsecs;
> >   	uint64_t system_time;
> >   };
> > -DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime_t);
> > +DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime32_t);
> > +#define XENPF_settime64           62
> > +struct xenpf_settime64 {
> > +    /* IN variables. */
> > +    uint64_t secs;
> > +    uint32_t nsecs;
> > +    uint32_t mbz;
> > +    uint64_t system_time;
> > +};
> > +DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime64_t);
> >     /*
> >    * Request memory range (@mfn, @mfn+@nr_mfns-1) to have type @type.
> > @@ -495,7 +504,8 @@ struct xen_platform_op {
> >   	uint32_t cmd;
> >   	uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
> >   	union {
> > -		struct xenpf_settime           settime;
> > +		struct xenpf_settime32         settime32;
> > +		struct xenpf_settime64         settime64;
> >   		struct xenpf_add_memtype       add_memtype;
> >   		struct xenpf_del_memtype       del_memtype;
> >   		struct xenpf_read_memtype      read_memtype;
>
Boris Ostrovsky Nov. 12, 2015, 5:16 p.m. UTC | #4
On 11/12/2015 11:34 AM, Stefano Stabellini wrote:
> On Thu, 12 Nov 2015, Boris Ostrovsky wrote:
>> On 11/11/2015 11:51 AM, Stefano Stabellini wrote:
>>> Rename the current XENPF_settime hypercall and related struct to
>>> XENPF_settime32.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>>> CC: konrad.wilk@oracle.com
>>> CC: david.vrabel@citrix.com
>>> CC: boris.ostrovsky@oracle.com
>>> ---
>>>    arch/x86/xen/time.c              |    8 ++++----
>>>    include/xen/interface/platform.h |   18 ++++++++++++++----
>>>    2 files changed, 18 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
>>> index 663c2ea..3bbd377 100644
>>> --- a/arch/x86/xen/time.c
>>> +++ b/arch/x86/xen/time.c
>>> @@ -134,10 +134,10 @@ static int xen_pvclock_gtod_notify(struct
>>> notifier_block *nb,
>>>    	if (!was_set && timespec_compare(&now, &next_sync) < 0)
>>>    		return NOTIFY_OK;
>>>    -	op.cmd = XENPF_settime;
>>> -	op.u.settime.secs = now.tv_sec;
>>> -	op.u.settime.nsecs = now.tv_nsec;
>>> -	op.u.settime.system_time = xen_clocksource_read();
>>> +	op.cmd = XENPF_settime32;
>>> +	op.u.settime32.secs = now.tv_sec;
>>> +	op.u.settime32.nsecs = now.tv_nsec;
>>> +	op.u.settime32.system_time = xen_clocksource_read();
>> Can/should we switch to time64 here? (This would require a couple more changes
>> but they would all be local to this routine).
> I can do that, but it should be a separate patch. I'll queue it at the
> end of the series.

Didn't Arnd just say that something needs to be done in the hypervisor 
for that to work? Or did I misunderstood him?

Anyway, for this patch
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>


>
>
>>>      	(void)HYPERVISOR_platform_op(&op);
>>>    diff --git a/include/xen/interface/platform.h
>>> b/include/xen/interface/platform.h
>>> index 8e03587..732efb0 100644
>>> --- a/include/xen/interface/platform.h
>>> +++ b/include/xen/interface/platform.h
>>> @@ -35,14 +35,23 @@
>>>     * Set clock such that it would read <secs,nsecs> after 00:00:00 UTC,
>>>     * 1 January, 1970 if the current system time was <system_time>.
>>>     */
>>> -#define XENPF_settime             17
>>> -struct xenpf_settime {
>>> +#define XENPF_settime32             17
>>> +struct xenpf_settime32 {
>>>    	/* IN variables. */
>>>    	uint32_t secs;
>>>    	uint32_t nsecs;
>>>    	uint64_t system_time;
>>>    };
>>> -DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime_t);
>>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime32_t);
>>> +#define XENPF_settime64           62
>>> +struct xenpf_settime64 {
>>> +    /* IN variables. */
>>> +    uint64_t secs;
>>> +    uint32_t nsecs;
>>> +    uint32_t mbz;
>>> +    uint64_t system_time;
>>> +};
>>> +DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime64_t);
>>>      /*
>>>     * Request memory range (@mfn, @mfn+@nr_mfns-1) to have type @type.
>>> @@ -495,7 +504,8 @@ struct xen_platform_op {
>>>    	uint32_t cmd;
>>>    	uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
>>>    	union {
>>> -		struct xenpf_settime           settime;
>>> +		struct xenpf_settime32         settime32;
>>> +		struct xenpf_settime64         settime64;
>>>    		struct xenpf_add_memtype       add_memtype;
>>>    		struct xenpf_del_memtype       del_memtype;
>>>    		struct xenpf_read_memtype      read_memtype;
Arnd Bergmann Nov. 12, 2015, 7:27 p.m. UTC | #5
On Thursday 12 November 2015 12:16:47 Boris Ostrovsky wrote:
> >>> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> >>> index 663c2ea..3bbd377 100644
> >>> --- a/arch/x86/xen/time.c
> >>> +++ b/arch/x86/xen/time.c
> >>> @@ -134,10 +134,10 @@ static int xen_pvclock_gtod_notify(struct
> >>> notifier_block *nb,
> >>>     if (!was_set && timespec_compare(&now, &next_sync) < 0)
> >>>             return NOTIFY_OK;
> >>>    -        op.cmd = XENPF_settime;
> >>> -   op.u.settime.secs = now.tv_sec;
> >>> -   op.u.settime.nsecs = now.tv_nsec;
> >>> -   op.u.settime.system_time = xen_clocksource_read();
> >>> +   op.cmd = XENPF_settime32;
> >>> +   op.u.settime32.secs = now.tv_sec;
> >>> +   op.u.settime32.nsecs = now.tv_nsec;
> >>> +   op.u.settime32.system_time = xen_clocksource_read();
> >> Can/should we switch to time64 here? (This would require a couple more changes
> >> but they would all be local to this routine).
> > I can do that, but it should be a separate patch. I'll queue it at the
> > end of the series.
> 
> Didn't Arnd just say that something needs to be done in the hypervisor 
> for that to work? Or did I misunderstood him?

What I meant is that we need to do both sides in order to actually use
64-bit times, but the patches are otherwise independent of one another
because a change to either side is not allowed to break the other.

	Arnd
diff mbox

Patch

diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index 663c2ea..3bbd377 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -134,10 +134,10 @@  static int xen_pvclock_gtod_notify(struct notifier_block *nb,
 	if (!was_set && timespec_compare(&now, &next_sync) < 0)
 		return NOTIFY_OK;
 
-	op.cmd = XENPF_settime;
-	op.u.settime.secs = now.tv_sec;
-	op.u.settime.nsecs = now.tv_nsec;
-	op.u.settime.system_time = xen_clocksource_read();
+	op.cmd = XENPF_settime32;
+	op.u.settime32.secs = now.tv_sec;
+	op.u.settime32.nsecs = now.tv_nsec;
+	op.u.settime32.system_time = xen_clocksource_read();
 
 	(void)HYPERVISOR_platform_op(&op);
 
diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
index 8e03587..732efb0 100644
--- a/include/xen/interface/platform.h
+++ b/include/xen/interface/platform.h
@@ -35,14 +35,23 @@ 
  * Set clock such that it would read <secs,nsecs> after 00:00:00 UTC,
  * 1 January, 1970 if the current system time was <system_time>.
  */
-#define XENPF_settime             17
-struct xenpf_settime {
+#define XENPF_settime32             17
+struct xenpf_settime32 {
 	/* IN variables. */
 	uint32_t secs;
 	uint32_t nsecs;
 	uint64_t system_time;
 };
-DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime_t);
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime32_t);
+#define XENPF_settime64           62
+struct xenpf_settime64 {
+    /* IN variables. */
+    uint64_t secs;
+    uint32_t nsecs;
+    uint32_t mbz;
+    uint64_t system_time;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xenpf_settime64_t);
 
 /*
  * Request memory range (@mfn, @mfn+@nr_mfns-1) to have type @type.
@@ -495,7 +504,8 @@  struct xen_platform_op {
 	uint32_t cmd;
 	uint32_t interface_version; /* XENPF_INTERFACE_VERSION */
 	union {
-		struct xenpf_settime           settime;
+		struct xenpf_settime32         settime32;
+		struct xenpf_settime64         settime64;
 		struct xenpf_add_memtype       add_memtype;
 		struct xenpf_del_memtype       del_memtype;
 		struct xenpf_read_memtype      read_memtype;