* [PATCH v2 1/1] xen: update system time immediately when VCPUOP_register_vcpu_info
@ 2021-10-25 17:35 Dongli Zhang
2021-10-28 21:07 ` Stefano Stabellini
2021-11-02 12:23 ` Jan Beulich
0 siblings, 2 replies; 6+ messages in thread
From: Dongli Zhang @ 2021-10-25 17:35 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, julien, Volodymyr_Babchuk, andrew.cooper3,
george.dunlap, iwj, jbeulich, wl, joe.jin
The guest may access the pv vcpu_time_info immediately after
VCPUOP_register_vcpu_info. This is to borrow the idea of
VCPUOP_register_vcpu_time_memory_area, where the
force_update_vcpu_system_time() is called immediately when the new memory
area is registered.
Otherwise, we may observe clock drift at the VM side if the VM accesses
the clocksource immediately after VCPUOP_register_vcpu_info().
Reference:
https://lists.xenproject.org/archives/html/xen-devel/2021-10/msg00571.html
Cc: Joe Jin <joe.jin@oracle.com>
Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
---
Changed since v1:
- Implement force_update_vcpu_system_time() for ARM
(suggested by Jan Beulich)
While I have tested ARM compilation with aarch64-linux-gnu-gcc, I did
not test on ARM platform.
xen/arch/arm/time.c | 5 +++++
xen/common/domain.c | 2 ++
xen/include/asm-arm/time.h | 2 ++
3 files changed, 9 insertions(+)
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 7dbd363537..dec53b5f7d 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -351,6 +351,11 @@ void update_vcpu_system_time(struct vcpu *v)
/* XXX update shared_info->wc_* */
}
+void force_update_vcpu_system_time(struct vcpu *v)
+{
+ update_vcpu_system_time(v);
+}
+
void domain_set_time_offset(struct domain *d, int64_t time_offset_seconds)
{
d->time_offset.seconds = time_offset_seconds;
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 8b53c49d1e..d71fcab88c 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1704,6 +1704,8 @@ long do_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
rc = map_vcpu_info(v, info.mfn, info.offset);
domain_unlock(d);
+ force_update_vcpu_system_time(v);
+
break;
}
diff --git a/xen/include/asm-arm/time.h b/xen/include/asm-arm/time.h
index 6b8fd839dd..4b401c1110 100644
--- a/xen/include/asm-arm/time.h
+++ b/xen/include/asm-arm/time.h
@@ -105,6 +105,8 @@ extern uint64_t ns_to_ticks(s_time_t ns);
void preinit_xen_time(void);
+void force_update_vcpu_system_time(struct vcpu *v);
+
#endif /* __ARM_TIME_H__ */
/*
* Local variables:
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] xen: update system time immediately when VCPUOP_register_vcpu_info
2021-10-25 17:35 [PATCH v2 1/1] xen: update system time immediately when VCPUOP_register_vcpu_info Dongli Zhang
@ 2021-10-28 21:07 ` Stefano Stabellini
2021-11-02 12:23 ` Jan Beulich
1 sibling, 0 replies; 6+ messages in thread
From: Stefano Stabellini @ 2021-10-28 21:07 UTC (permalink / raw)
To: Dongli Zhang
Cc: xen-devel, sstabellini, julien, Volodymyr_Babchuk,
andrew.cooper3, george.dunlap, iwj, jbeulich, wl, joe.jin
On Mon, 25 Oct 2021, Dongli Zhang wrote:
> The guest may access the pv vcpu_time_info immediately after
> VCPUOP_register_vcpu_info. This is to borrow the idea of
> VCPUOP_register_vcpu_time_memory_area, where the
> force_update_vcpu_system_time() is called immediately when the new memory
> area is registered.
>
> Otherwise, we may observe clock drift at the VM side if the VM accesses
> the clocksource immediately after VCPUOP_register_vcpu_info().
>
> Reference:
> https://lists.xenproject.org/archives/html/xen-devel/2021-10/msg00571.html
> Cc: Joe Jin <joe.jin@oracle.com>
> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
> ---
> Changed since v1:
> - Implement force_update_vcpu_system_time() for ARM
> (suggested by Jan Beulich)
> While I have tested ARM compilation with aarch64-linux-gnu-gcc, I did
> not test on ARM platform.
This is fine. Of course it is not going to do anything on ARM given that
update_vcpu_system_time in unimplemented but it is certainly harmless.
For the ARM part:
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> xen/arch/arm/time.c | 5 +++++
> xen/common/domain.c | 2 ++
> xen/include/asm-arm/time.h | 2 ++
> 3 files changed, 9 insertions(+)
>
> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
> index 7dbd363537..dec53b5f7d 100644
> --- a/xen/arch/arm/time.c
> +++ b/xen/arch/arm/time.c
> @@ -351,6 +351,11 @@ void update_vcpu_system_time(struct vcpu *v)
> /* XXX update shared_info->wc_* */
> }
>
> +void force_update_vcpu_system_time(struct vcpu *v)
> +{
> + update_vcpu_system_time(v);
> +}
> +
> void domain_set_time_offset(struct domain *d, int64_t time_offset_seconds)
> {
> d->time_offset.seconds = time_offset_seconds;
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 8b53c49d1e..d71fcab88c 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1704,6 +1704,8 @@ long do_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
> rc = map_vcpu_info(v, info.mfn, info.offset);
> domain_unlock(d);
>
> + force_update_vcpu_system_time(v);
> +
> break;
> }
>
> diff --git a/xen/include/asm-arm/time.h b/xen/include/asm-arm/time.h
> index 6b8fd839dd..4b401c1110 100644
> --- a/xen/include/asm-arm/time.h
> +++ b/xen/include/asm-arm/time.h
> @@ -105,6 +105,8 @@ extern uint64_t ns_to_ticks(s_time_t ns);
>
> void preinit_xen_time(void);
>
> +void force_update_vcpu_system_time(struct vcpu *v);
> +
> #endif /* __ARM_TIME_H__ */
> /*
> * Local variables:
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] xen: update system time immediately when VCPUOP_register_vcpu_info
2021-10-25 17:35 [PATCH v2 1/1] xen: update system time immediately when VCPUOP_register_vcpu_info Dongli Zhang
2021-10-28 21:07 ` Stefano Stabellini
@ 2021-11-02 12:23 ` Jan Beulich
2021-11-02 16:00 ` Ian Jackson
1 sibling, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2021-11-02 12:23 UTC (permalink / raw)
To: Dongli Zhang, iwj
Cc: sstabellini, julien, Volodymyr_Babchuk, andrew.cooper3,
george.dunlap, wl, joe.jin, xen-devel
On 25.10.2021 19:35, Dongli Zhang wrote:
> The guest may access the pv vcpu_time_info immediately after
> VCPUOP_register_vcpu_info. This is to borrow the idea of
> VCPUOP_register_vcpu_time_memory_area, where the
> force_update_vcpu_system_time() is called immediately when the new memory
> area is registered.
>
> Otherwise, we may observe clock drift at the VM side if the VM accesses
> the clocksource immediately after VCPUOP_register_vcpu_info().
>
> Reference:
> https://lists.xenproject.org/archives/html/xen-devel/2021-10/msg00571.html
> Cc: Joe Jin <joe.jin@oracle.com>
> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Ian - any thoughts towards 4.16 here either way?
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] xen: update system time immediately when VCPUOP_register_vcpu_info
2021-11-02 12:23 ` Jan Beulich
@ 2021-11-02 16:00 ` Ian Jackson
2021-11-02 16:36 ` Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Ian Jackson @ 2021-11-02 16:00 UTC (permalink / raw)
To: Jan Beulich
Cc: Dongli Zhang, sstabellini, julien, Volodymyr_Babchuk,
andrew.cooper3, george.dunlap, wl, joe.jin, xen-devel
Jan Beulich writes ("Re: [PATCH v2 1/1] xen: update system time immediately when VCPUOP_register_vcpu_info"):
> On 25.10.2021 19:35, Dongli Zhang wrote:
> > The guest may access the pv vcpu_time_info immediately after
> > VCPUOP_register_vcpu_info. This is to borrow the idea of
> > VCPUOP_register_vcpu_time_memory_area, where the
> > force_update_vcpu_system_time() is called immediately when the new memory
> > area is registered.
> >
> > Otherwise, we may observe clock drift at the VM side if the VM accesses
> > the clocksource immediately after VCPUOP_register_vcpu_info().
> >
> > Reference:
> > https://lists.xenproject.org/archives/html/xen-devel/2021-10/msg00571.html
> > Cc: Joe Jin <joe.jin@oracle.com>
> > Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
> Ian - any thoughts towards 4.16 here either way?
This looks like a bugfix to me, and the diff is certainly small. I am
positively inclined. I would like to know what the risks are.
Stefano says this does nothing on ARM so the risk would be to x86.
Can you advise ?
Thanks,
Ian.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] xen: update system time immediately when VCPUOP_register_vcpu_info
2021-11-02 16:00 ` Ian Jackson
@ 2021-11-02 16:36 ` Jan Beulich
2021-11-02 16:48 ` Ian Jackson
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2021-11-02 16:36 UTC (permalink / raw)
To: Ian Jackson
Cc: Dongli Zhang, sstabellini, julien, Volodymyr_Babchuk,
andrew.cooper3, george.dunlap, wl, joe.jin, xen-devel
On 02.11.2021 17:00, Ian Jackson wrote:
> Jan Beulich writes ("Re: [PATCH v2 1/1] xen: update system time immediately when VCPUOP_register_vcpu_info"):
>> On 25.10.2021 19:35, Dongli Zhang wrote:
>>> The guest may access the pv vcpu_time_info immediately after
>>> VCPUOP_register_vcpu_info. This is to borrow the idea of
>>> VCPUOP_register_vcpu_time_memory_area, where the
>>> force_update_vcpu_system_time() is called immediately when the new memory
>>> area is registered.
>>>
>>> Otherwise, we may observe clock drift at the VM side if the VM accesses
>>> the clocksource immediately after VCPUOP_register_vcpu_info().
>>>
>>> Reference:
>>> https://lists.xenproject.org/archives/html/xen-devel/2021-10/msg00571.html
>>> Cc: Joe Jin <joe.jin@oracle.com>
>>> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
>>
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>
>> Ian - any thoughts towards 4.16 here either way?
>
> This looks like a bugfix to me, and the diff is certainly small. I am
> positively inclined. I would like to know what the risks are.
> Stefano says this does nothing on ARM so the risk would be to x86.
> Can you advise ?
I don't see any noteworthy risks - a call to a function gets added
in a 2nd place; the function itself has been working fine for years,
and it is fine to be used in this new context.
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] xen: update system time immediately when VCPUOP_register_vcpu_info
2021-11-02 16:36 ` Jan Beulich
@ 2021-11-02 16:48 ` Ian Jackson
0 siblings, 0 replies; 6+ messages in thread
From: Ian Jackson @ 2021-11-02 16:48 UTC (permalink / raw)
To: Jan Beulich
Cc: Dongli Zhang, sstabellini, julien, Volodymyr_Babchuk,
andrew.cooper3, george.dunlap, wl, joe.jin, xen-devel
Jan Beulich writes ("Re: [PATCH v2 1/1] xen: update system time immediately when VCPUOP_register_vcpu_info"):
> I don't see any noteworthy risks - a call to a function gets added
> in a 2nd place; the function itself has been working fine for years,
> and it is fine to be used in this new context.
Thanks.
I think this part:
| it is fine to be used in this new context.
was where I wanted a 2nd opinion for confirmation.
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-02 16:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-25 17:35 [PATCH v2 1/1] xen: update system time immediately when VCPUOP_register_vcpu_info Dongli Zhang
2021-10-28 21:07 ` Stefano Stabellini
2021-11-02 12:23 ` Jan Beulich
2021-11-02 16:00 ` Ian Jackson
2021-11-02 16:36 ` Jan Beulich
2021-11-02 16:48 ` Ian Jackson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.