qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 10/16] hw/i386/vmport: Add support for CMD_GETTIME


From: Liran Alon
Subject: Re: [PATCH v3 10/16] hw/i386/vmport: Add support for CMD_GETTIME
Date: Sat, 14 Mar 2020 22:05:20 +0200
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:68.0) Gecko/20100101 Thunderbird/68.5.0


On 14/03/2020 21:58, Nikita Leshenko wrote:

On 14 Mar 2020, at 21:26, Michael S. Tsirkin <address@hidden> wrote:

On Sat, Mar 14, 2020 at 09:17:30PM +0200, Liran Alon wrote:
On 14/03/2020 21:14, Michael S. Tsirkin wrote:
On Sat, Mar 14, 2020 at 09:04:30PM +0200, Liran Alon wrote:
On 14/03/2020 20:18, Michael S. Tsirkin wrote:
On Fri, Mar 13, 2020 at 06:26:54PM +0200, Liran Alon wrote:
On 13/03/2020 17:47, Michael S. Tsirkin wrote:
On Fri, Mar 13, 2020 at 05:25:20PM +0200, Liran Alon wrote:
@@ -168,6 +169,20 @@ static uint32_t vmport_cmd_ram_size(void *opaque, uint32_t 
addr)
         return ram_size;
     }
+static uint32_t vmport_cmd_time(void *opaque, uint32_t addr)
+{
+    X86CPU *cpu = X86_CPU(current_cpu);
+    qemu_timeval tv;
+
+    if (qemu_gettimeofday(&tv) < 0) {
+        return UINT32_MAX;
+    }
+
+    cpu->env.regs[R_EBX] = (uint32_t)tv.tv_usec;
+    cpu->env.regs[R_ECX] = port_state->max_time_lag_us;
+    return (uint32_t)tv.tv_sec;
+}
+
     /* vmmouse helpers */
     void vmmouse_get_data(uint32_t *data)
     {
That's a very weird thing to return to the guest.
For example it's not monotonic across migrations.
That's the VMware PV interface... I didn't design it. :P
Regarding how it handles the fact time is not monotonic across migrations,
see big comment at the start of services/plugins/timeSync/timeSync.c in
open-vm-tools regarding the time-sync algorithm used by VMware Tools.
Specifically:
"""
During normal operation this plugin only steps the time forward and only if
the error is greater than one second.
Looks like guest assumes this time only moves forward.
So something needs to be done to avoid it moving
backward across migrations.
Where do you see this assumption in guest code? I don't think this is true.
Guest code seems to handle this by making sure to only step the time
forward.
Exactly. So if host time moved backward e.g. by 100s, then for 100s
time is not correcting. Which possibly vmware has a way to mitigate
against e.g. by synchronising host time using their
management app.

Read carefully services/plugins/timeSync/timeSync.c and point me to what I'm
missing if you think otherwise (i.e. I missed something).
I'm just going by what you write in a patch.

So guest doesn't assume that this time only moves forward...

Can you clarify then which change do you suggest making to this patch in
this regard? It seems correct to me.
i.e. The CMD_GETTIME implementation seems correct to me and it doesn't need
to do anything special to handle host time moving backwards.

-Liran

I think something needs to be done to make sure host time as reported by
this command does not move backwards significantly. Just forwarding
gettimeofday won't cut it IMHO. Look at kvm clock for inspiration of
things to do.

It isn't required by the command interface definition.
It's explicitly specified in timeSync.c that guest code handles the case
host time goes backwards.
According to what you wrote it does not crash but also does not do
updates. That will work well only if the time difference isn't large.
Then with time as guest gets interrupted by host, the time difference
shrinks and eventually you are resyncing again.  And I'm guessing there
are hypervisor management interfaces in place to make sure that is so.
Linux is not guaranteed to have such interfaces so you have to come
up with QEMU specific ones.


We are not inventing the interface, we just implement it according to it's
definition.

-Liran
Host time is a vague enough terminology. I suspect this implementation
is too simplistic to cover all cases.

--
MST
I saw this discussion from the side and I just wanted to point out that as far
as I understand this interface is needed to sync *wallclock time* between the
guest and the host, mostly for user experience. There is no guarantee that it's
monotonic. Unlike TSC, we don't need to take special care with regard to live
migration or drift.

Just as NTP may report inconsistent time, it's OK if this interface is somewhat
inconsistent.

I think that the reason that open-vm-tools doesn't move time backwards is to
help applications that treat wallclock time as if it's monotonic time and break
if the date is moved backwards (which may happen more frequently in virtual
environment so it's handled specifically). But this doesn't change the fact that
this PV interface is for reporting wall time. So I couldn't understand what
source other than gettimeofday() you were suggesting for Liran to use to report
wallclock time.

Nikita

Michael, you can also refer to this VMware time-keeping whitepaper: https://www.vmware.com/pdf/vmware_timekeeping.pdf.
According to section "Initializing and Correcting Wall-Clock Time":
"""
VMware Tools can also optionally be used to correct long‐term drift and errors by periodically resynchronizing the virtual machine’s clock to the host’s clock, but the current version at this writing is limited. In particular, in guest operating systems other than NetWare, it does not correct errors in which the guest clock
is ahead of real time, only those in which the guest clock is behind.

"""

If I understand correctly, this seems to validate my assumption that current implementation for CMD_GETTIME is sufficient.

-Liran






reply via email to

[Prev in Thread] Current Thread [Next in Thread]