[PATCH v2] hv_utils: Allow implicit ICTIMESYNCFLAG_SYNC

[PATCH v2] hv_utils: Allow implicit ICTIMESYNCFLAG_SYNC

[pmartincic]

From: Peter Martincic <pmartincic@microsoft.com>

Hyper-V hosts can omit the _SYNC flag to due a bug on resume from modern
suspend. In such a case, the guest may fail to update its time-of-day to
account for the period when it was suspended, and could proceed with a
significantly wrong time-of-day. In such a case when the guest is
significantly behind, fix it by treating a _SAMPLE the same as if _SYNC
was received so that the guest time-of-day is updated.

This is hidden behind param hv_utils.timesync_implicit.

Signed-off-by: Peter Martincic <pmartincic@microsoft.com>
---
 drivers/hv/hv_util.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 42aec2c5606a..9c97c4065fe7 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -296,6 +296,11 @@ static struct {
 	spinlock_t			lock;
 } host_ts;
 
+static bool timesync_implicit;
+
+module_param(timesync_implicit, bool, 0644);
+MODULE_PARM_DESC(timesync_implicit, "If set treat SAMPLE as SYNC when clock is behind");
+
 static inline u64 reftime_to_ns(u64 reftime)
 {
 	return (reftime - WLTIMEDELTA) * 100;
@@ -344,6 +349,29 @@ static void hv_set_host_time(struct work_struct *work)
 		do_settimeofday64(&ts);
 }
 
+/*
+ * Due to a bug on Hyper-V hosts, the sync flag may not always be sent on resume.
+ * Force a sync if the guest is behind.
+ */
+static inline bool hv_implicit_sync(u64 host_time)
+{
+	struct timespec64 new_ts;
+	struct timespec64 threshold_ts;
+
+	new_ts = ns_to_timespec64(reftime_to_ns(host_time));
+	ktime_get_real_ts64(&threshold_ts);
+
+	threshold_ts.tv_sec += 5;
+
+	/*
+	 * If guest behind the host by 5 or more seconds.
+	 */
+	if (timespec64_compare(&new_ts, &threshold_ts) >= 0)
+		return true;
+
+	return false;
+}
+
 /*
  * Synchronize time with host after reboot, restore, etc.
  *
@@ -384,7 +412,8 @@ static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 adj_flags)
 	spin_unlock_irqrestore(&host_ts.lock, flags);
 
 	/* Schedule work to do do_settimeofday64() */
-	if (adj_flags & ICTIMESYNCFLAG_SYNC)
+	if ((adj_flags & ICTIMESYNCFLAG_SYNC) ||
+	    (timesync_implicit && hv_implicit_sync(host_ts.host_time)))
 		schedule_work(&adj_time_work);
 }
 
-- 
2.34.1

Re: [PATCH v2] hv_utils: Allow implicit ICTIMESYNCFLAG_SYNC

[Boqun Feng]

On Mon, Nov 27, 2023 at 01:35:24PM -0800, pmartincic@linux.microsoft.com wrote:
> From: Peter Martincic <pmartincic@microsoft.com>
> 
> Hyper-V hosts can omit the _SYNC flag to due a bug on resume from modern
> suspend. In such a case, the guest may fail to update its time-of-day to
> account for the period when it was suspended, and could proceed with a
> significantly wrong time-of-day. In such a case when the guest is
> significantly behind, fix it by treating a _SAMPLE the same as if _SYNC
> was received so that the guest time-of-day is updated.
> 
> This is hidden behind param hv_utils.timesync_implicit.
> 
> Signed-off-by: Peter Martincic <pmartincic@microsoft.com>

Looks good to me.

Acked-by: Boqun Feng <boqun.feng@gmail.com>

Regards,
Boqun

> ---
>  drivers/hv/hv_util.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> index 42aec2c5606a..9c97c4065fe7 100644
> --- a/drivers/hv/hv_util.c
> +++ b/drivers/hv/hv_util.c
> @@ -296,6 +296,11 @@ static struct {
>  	spinlock_t			lock;
>  } host_ts;
>  
> +static bool timesync_implicit;
> +
> +module_param(timesync_implicit, bool, 0644);
> +MODULE_PARM_DESC(timesync_implicit, "If set treat SAMPLE as SYNC when clock is behind");
> +
>  static inline u64 reftime_to_ns(u64 reftime)
>  {
>  	return (reftime - WLTIMEDELTA) * 100;
> @@ -344,6 +349,29 @@ static void hv_set_host_time(struct work_struct *work)
>  		do_settimeofday64(&ts);
>  }
>  
> +/*
> + * Due to a bug on Hyper-V hosts, the sync flag may not always be sent on resume.
> + * Force a sync if the guest is behind.
> + */
> +static inline bool hv_implicit_sync(u64 host_time)
> +{
> +	struct timespec64 new_ts;
> +	struct timespec64 threshold_ts;
> +
> +	new_ts = ns_to_timespec64(reftime_to_ns(host_time));
> +	ktime_get_real_ts64(&threshold_ts);
> +
> +	threshold_ts.tv_sec += 5;
> +
> +	/*
> +	 * If guest behind the host by 5 or more seconds.
> +	 */
> +	if (timespec64_compare(&new_ts, &threshold_ts) >= 0)
> +		return true;
> +
> +	return false;
> +}
> +
>  /*
>   * Synchronize time with host after reboot, restore, etc.
>   *
> @@ -384,7 +412,8 @@ static inline void adj_guesttime(u64 hosttime, u64 reftime, u8 adj_flags)
>  	spin_unlock_irqrestore(&host_ts.lock, flags);
>  
>  	/* Schedule work to do do_settimeofday64() */
> -	if (adj_flags & ICTIMESYNCFLAG_SYNC)
> +	if ((adj_flags & ICTIMESYNCFLAG_SYNC) ||
> +	    (timesync_implicit && hv_implicit_sync(host_ts.host_time)))
>  		schedule_work(&adj_time_work);
>  }
>  
> -- 
> 2.34.1
> 

Re: [PATCH v2] hv_utils: Allow implicit ICTIMESYNCFLAG_SYNC

[Wei Liu]

On Thu, Nov 30, 2023 at 11:16:43AM -0800, Boqun Feng wrote:
> On Mon, Nov 27, 2023 at 01:35:24PM -0800, pmartincic@linux.microsoft.com wrote:
> > From: Peter Martincic <pmartincic@microsoft.com>
> > 
> > Hyper-V hosts can omit the _SYNC flag to due a bug on resume from modern
> > suspend. In such a case, the guest may fail to update its time-of-day to
> > account for the period when it was suspended, and could proceed with a
> > significantly wrong time-of-day. In such a case when the guest is
> > significantly behind, fix it by treating a _SAMPLE the same as if _SYNC
> > was received so that the guest time-of-day is updated.
> > 
> > This is hidden behind param hv_utils.timesync_implicit.
> > 
> > Signed-off-by: Peter Martincic <pmartincic@microsoft.com>
> 
> Looks good to me.
> 
> Acked-by: Boqun Feng <boqun.feng@gmail.com>

Applied to hyperv-fixes, thanks!

RE: [PATCH v2] hv_utils: Allow implicit ICTIMESYNCFLAG_SYNC

[David Balazic]

On Mon, Nov 27, 2023 at 01:35:24PM -0800, pmartincic@linux.microsoft.com wrote:

> From: Peter Martincic <pmartincic@microsoft.com>
>
> Hyper-V hosts can omit the _SYNC flag to due a bug on resume from modern
> suspend. In such a case, the guest may fail to update its time-of-day to
> account for the period when it was suspended, and could proceed with a
> significantly wrong time-of-day. In such a case when the guest is significantly
> behind, fix it by treating a _SAMPLE the same as if _SYNC was received so that
> the guest time-of-day is updated.
>
> This is hidden behind param hv_utils.timesync_implicit.
>
> Signed-off-by: Peter Martincic <pmartincic@microsoft.com>

Hi!

As Peters mail does not seem to exists any more, I'll ask here (LKML and LHV), if all right:

Is there any news  about this bug on Hyper-V hosts?
If this is actually a Hyper-V (host) bug, was it ever addressed and fixed?

I encounter this bug when using Windows 11 Enterprise (25H2) host and Oracle Linux 8 and 9 as guests.
They use an older kernel, so I can't use this parameter to fix it.

Windows guests do not seem to be affected by it, in my experience.

Feel free to reply off list, as it is not really on topic.

Regards,
David Balažic



The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection.

Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions.