| Message ID | 20240903130303.71334-2-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86/time: improvements to wallclock logic | expand |
On 03.09.2024 15:02, Roger Pau Monne wrote: > --- a/xen/arch/x86/time.c > +++ b/xen/arch/x86/time.c > @@ -785,6 +785,31 @@ static struct platform_timesource __initdata_cf_clobber plt_xen_timer = > .resume = resume_xen_timer, > .counter_bits = 63, > }; > + > +static unsigned long read_xen_wallclock(void) > +{ > + struct shared_info *sh_info = XEN_shared_info; > + uint32_t wc_version; > + uint64_t wc_sec; > + > + ASSERT(xen_guest); > + > + do { > + wc_version = sh_info->wc_version & ~1; > + smp_rmb(); > + > + wc_sec = sh_info->wc_sec; > + smp_rmb(); > + } while ( wc_version != sh_info->wc_version ); > + > + return wc_sec + read_xen_timer() / 1000000000; > +} > +#else > +static unsigned long read_xen_wallclock(void) > +{ > + ASSERT_UNREACHABLE(); > + return 0; > +} > #endif I understand you try to re-use an existing #ifdef here, but I wonder if I could talk you into not doing so and instead placing the #ifdef inside the (then single) function body. Less redundancy, less room for mistakes / oversights. Jan
On Tue, Sep 03, 2024 at 04:48:41PM +0200, Jan Beulich wrote: > On 03.09.2024 15:02, Roger Pau Monne wrote: > > --- a/xen/arch/x86/time.c > > +++ b/xen/arch/x86/time.c > > @@ -785,6 +785,31 @@ static struct platform_timesource __initdata_cf_clobber plt_xen_timer = > > .resume = resume_xen_timer, > > .counter_bits = 63, > > }; > > + > > +static unsigned long read_xen_wallclock(void) > > +{ > > + struct shared_info *sh_info = XEN_shared_info; > > + uint32_t wc_version; > > + uint64_t wc_sec; > > + > > + ASSERT(xen_guest); > > + > > + do { > > + wc_version = sh_info->wc_version & ~1; > > + smp_rmb(); > > + > > + wc_sec = sh_info->wc_sec; > > + smp_rmb(); > > + } while ( wc_version != sh_info->wc_version ); > > + > > + return wc_sec + read_xen_timer() / 1000000000; > > +} > > +#else > > +static unsigned long read_xen_wallclock(void) > > +{ > > + ASSERT_UNREACHABLE(); > > + return 0; > > +} > > #endif > > I understand you try to re-use an existing #ifdef here, but I wonder if I > could talk you into not doing so and instead placing the #ifdef inside the > (then single) function body. Less redundancy, less room for mistakes / > oversights. I don't mind much, I've assumed reducing the number of #ifdefs blocks was better, but I don't have a strong opinion. Initially I just kept the #ifdef block in get_wallclock_time(). Thanks, Roger.
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c index a97d78484105..d022db4bd4a0 100644 --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -785,6 +785,31 @@ static struct platform_timesource __initdata_cf_clobber plt_xen_timer = .resume = resume_xen_timer, .counter_bits = 63, }; + +static unsigned long read_xen_wallclock(void) +{ + struct shared_info *sh_info = XEN_shared_info; + uint32_t wc_version; + uint64_t wc_sec; + + ASSERT(xen_guest); + + do { + wc_version = sh_info->wc_version & ~1; + smp_rmb(); + + wc_sec = sh_info->wc_sec; + smp_rmb(); + } while ( wc_version != sh_info->wc_version ); + + return wc_sec + read_xen_timer() / 1000000000; +} +#else +static unsigned long read_xen_wallclock(void) +{ + ASSERT_UNREACHABLE(); + return 0; +} #endif #ifdef CONFIG_HYPERV_GUEST @@ -1497,24 +1522,8 @@ void rtc_guest_write(unsigned int port, unsigned int data) static unsigned long get_wallclock_time(void) { -#ifdef CONFIG_XEN_GUEST if ( xen_guest ) - { - struct shared_info *sh_info = XEN_shared_info; - uint32_t wc_version; - uint64_t wc_sec; - - do { - wc_version = sh_info->wc_version & ~1; - smp_rmb(); - - wc_sec = sh_info->wc_sec; - smp_rmb(); - } while ( wc_version != sh_info->wc_version ); - - return wc_sec + read_xen_timer() / 1000000000; - } -#endif + return read_xen_wallclock(); return get_cmos_time(); }
Move the current code in get_wallclock_time() to fetch the Xen wallclock information from the shared page when running as a guest into a separate helper. No functional change intended. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v2: - New in this version. --- xen/arch/x86/time.c | 43 ++++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 17 deletions(-)