Re: [PATCH v7 1/2] x86/time: introduce command line option to select wallclock

["Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>]

[-- Attachment #1: Type: text/plain, Size: 4312 bytes --]

On Fri, Sep 13, 2024 at 09:59:06AM +0200, Roger Pau Monne wrote:
> Allow setting the used wallclock from the command line.  When the option is set
> to a value different than `auto` the probing is bypassed and the selected
> implementation is used (as long as it's available).
> 
> The `xen` and `efi` options require being booted as a Xen guest (with Xen guest
> supported built-in) or from UEFI firmware respectively.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

> ---
> Changes since v6:
>  - Clarify documentation regarding repeated using of the wallclock command line
>    option.
> 
> Changes since v5:
>  - Do EFI run-time services checking after command line parsing.
> 
> Changes since v3:
>  - Note xen option is only available if Xen guest support it built.
>  - Fix typo.
> ---
>  docs/misc/xen-command-line.pandoc | 21 ++++++++++++++++
>  xen/arch/x86/time.c               | 41 ++++++++++++++++++++++++++++++-
>  2 files changed, 61 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
> index 959cf45b55d9..2a9b3b9b8975 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -2816,6 +2816,27 @@ vwfi to `native` reduces irq latency significantly. It can also lead to
>  suboptimal scheduling decisions, but only when the system is
>  oversubscribed (i.e., in total there are more vCPUs than pCPUs).
>  
> +### wallclock (x86)
> +> `= auto | xen | cmos | efi`
> +
> +> Default: `auto`
> +
> +Allow forcing the usage of a specific wallclock source.
> +
> + * `auto` let the hypervisor select the clocksource based on internal
> +   heuristics.
> +
> + * `xen` force usage of the Xen shared_info wallclock when booted as a Xen
> +   guest.  This option is only available if the hypervisor was compiled with
> +   `CONFIG_XEN_GUEST` enabled.
> +
> + * `cmos` force usage of the CMOS RTC wallclock.
> +
> + * `efi` force usage of the EFI_GET_TIME run-time method when booted from EFI
> +   firmware.
> +
> +If the selected option is invalid or not available Xen will default to `auto`.
> +
>  ### watchdog (x86)
>  > `= force | <boolean>`
>  
> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> index 29b026735e5d..e4751684951e 100644
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -1552,6 +1552,37 @@ static const char *__init wallclock_type_to_string(void)
>      return "";
>  }
>  
> +static int __init cf_check parse_wallclock(const char *arg)
> +{
> +    wallclock_source = WALLCLOCK_UNSET;
> +
> +    if ( !arg )
> +        return -EINVAL;
> +
> +    if ( !strcmp("auto", arg) )
> +        ASSERT(wallclock_source == WALLCLOCK_UNSET);
> +    else if ( !strcmp("xen", arg) )
> +    {
> +        if ( !xen_guest )
> +            return -EINVAL;
> +
> +        wallclock_source = WALLCLOCK_XEN;
> +    }
> +    else if ( !strcmp("cmos", arg) )
> +        wallclock_source = WALLCLOCK_CMOS;
> +    else if ( !strcmp("efi", arg) )
> +        /*
> +         * Checking if run-time services are available must be done after
> +         * command line parsing.
> +         */
> +        wallclock_source = WALLCLOCK_EFI;
> +    else
> +        return -EINVAL;
> +
> +    return 0;
> +}
> +custom_param("wallclock", parse_wallclock);
> +
>  static void __init probe_wallclock(void)
>  {
>      ASSERT(wallclock_source == WALLCLOCK_UNSET);
> @@ -2527,7 +2558,15 @@ int __init init_xen_time(void)
>  
>      open_softirq(TIME_CALIBRATE_SOFTIRQ, local_time_calibration);
>  
> -    probe_wallclock();
> +    /*
> +     * EFI run time services can be disabled from the command line, hence the
> +     * check for them cannot be done as part of the wallclock option parsing.
> +     */
> +    if ( wallclock_source == WALLCLOCK_EFI && !efi_enabled(EFI_RS) )
> +        wallclock_source = WALLCLOCK_UNSET;
> +
> +    if ( wallclock_source == WALLCLOCK_UNSET )
> +        probe_wallclock();
>  
>      printk(XENLOG_INFO "Wallclock source: %s\n", wallclock_type_to_string());
>  
> -- 
> 2.46.0
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]