Re: [Xen-devel] [PATCH] x86/hvm/rtc: preserved guest RTC offset during suspend/resume/migrate

[Julien Grall <julien@xxxxxxx>]

Hi Paul,

On 18/12/2019 15:41, Paul Durrant wrote:
> The emulated RTC is synchronized with the PV wallclock; any write to the
> RTC will update struct domain's 'time_offset_seconds' field and call
> update_domain_wallclock().
>
> However, the value of 'time_offset_seconds' is not preserved in any save
> record and indeed, when the RTC save record is loaded, the CMOS values
> will be updated based on an offset value which may or may not have been
> set by the toolstack [1]. This may result in making bogus values available
> to the guest and messing up any calculations done in the call to
> alarm_timer_update() at the end of rtc_load().
>
> This patch extends the RTC save record to contain an offset value, which
> will be zero filled on load of an older record. The 'time_offset_secoonds'
> field in struct domain is also modified into a 'time_offset' struct,
> containing a 'seconds' field and a boolean 'set' field.
>
> The code in rtc_load() then uses the new value in the save record to
> update the value of struct domain's 'time_offset.seconds' unless
> 'time_offset.set' is true, which will only be the case if the toolstack has
> already performed a XEN_DOMCTL_settimeoffset.
>
> [1] There is currently no way for a toolstack to read the value of
>      'time_offset_seconds' from struct domain. In the past, any hope of
>      preservation of the value across a guest life-cycle operation was based
>      on relying on qemu-dm to write a value into xenstore whenever the RTC
>      was updated, in response to an IOREQ with type IOREQ_TYPE_TIMEOFFSET
>      being sent by Xen; see:
>
>      
> https://xenbits.xen.org/gitweb/?p=qemu-xen-traditional.git;a=blob;f=i386-dm/helper2.c#l457
>
>      but this behaviour was never forward-ported into upstream QEMU, which
>      completely ignores that IOREQ type.
>      In either case, nothing in xl or libxl ever samples the value of
>      RTC offset from xenstore so any offset adjustment to a non-zero value
>      performed by the guest (which in the case of Windows is highly likely
>      as it normally writes RTC in local time, whereas Xen maintains time in
>      UTC) is completely lost with the de-facto toolstack, and always has
>      been. Instead, PV drivers are relied upon to paper over this gaping
>      hole.
>
> Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>
> ---
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien@xxxxxxx>
> Cc: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Cc: Wei Liu <wl@xxxxxxx>
> Cc: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
> ---
>   xen/arch/arm/platform_hypercall.c      |  2 +-
>   xen/arch/arm/time.c                    |  3 ++-
>   xen/arch/arm/vtimer.c                  |  4 ++--
The Arm changes looks mostly mechanical:

Acked-by: Julien Grall <julien@xxxxxxx>

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel