Re: [PATCH v5 3/4] x86/time: introduce command line option to select wallclock

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.09.2024 16:24, Roger Pau Monné wrote:
> On Tue, Sep 10, 2024 at 03:49:52PM +0200, Jan Beulich wrote:
>> On 10.09.2024 15:10, Roger Pau Monné wrote:
>>>  Would you be fine with
>>> adding the following in init_xen_time():
>>>
>>>     /*
>>>      * EFI run time services can be disabled form 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();
>>
>> ... this is probably the best we can do (nit: s/form/from/ in the comment;
>> maybe also "..., hence the check done as part of option parsing may not
>> suffice" or some such).
> 
> I didn't put in my previous reply, but I've removed the efi_enabled()
> check from the option parsing and instead added this comment:
> 
>         /*
>          * Checking if run-time services are available must be done after
>          * command line parsing.
>          */
> 
> I don't think there's much point in doing the check in
> parse_wallclock() if it's not reliable, so your reference in the
> comment to "the check done as part of option parsing" is no longer
> valid.

Hmm. Rejecting the option if we can is reasonable imo. "efi=rs" can imo only
sensibly be used to override an earlier "efi=no-rs". Hence what we see while
parsing the wallclock option gives us at least reasonable grounds to reject
the option if EFI_RS is already clear. We then merely fail to reject the
option if the flag is cleared later.

Yet in the end I'd be happy to leave this particular aspect to you and the
EFI maintainers.

Jan