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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Jan 14, 2025 at 12:12:03PM +0100, Oleksii Kurochko wrote:
> 
> On 1/13/25 6:52 PM, Roger Pau Monné wrote:
> > On Mon, Jan 13, 2025 at 05:07:55PM +0100, Marek Marczykowski-Górecki wrote:
> > > 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@xxxxxxxxxx>
> > > Reviewed-by: Marek Marczykowski-Górecki<marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> > Thanks for the review.
> > 
> > Oleksii, can I get your opinion as Release Manager about whether this
> > (and the following patch) would be suitable for committing to staging
> > given the current release state?
> > 
> > It's a workaround for broken EFI implementations that many downstreams
> > carry on their patch queue.
> 
> Based on your commit message, I understand this as addressing a bug ( but not 
> very critical
> as IIUC downstreams have the similar patch on their side ). Therefore, if it 
> has been properly
> reviewed and tested, we should consider including it in the current release.

IIRC at least Qubes, XenServer and XCP-ng have a patch that achieves
the same behavior as proposed here.

> IIUC, setting the wallclock to EFI should align with the behavior Xen had 
> previously.
> It might be preferable to use that argument as the default for a while, 
> allowing others to verify the "auto"
> value over time. After that, we could consider making "auto" the default.
> That said, I am not particularly strict about following this approach.

We cannot really set efi as the default, as it would break when
booting on legacy BIOS systems.

We could take only patch 1 and leave patch 2 after Xen 4.20 has
branched, but at that point I would see little benefit in having just
patch 1.

I don't have a strong opinion, but downstreams have been complaining
about Xen behavior regarding the usage of EFI_GET_TIME, so it might be
good to not ship yet another release with such allegedly broken
behavior.

Let me know what you think, as I would need a formal Release-Ack if
this is to be committed.

Thanks, Roger.

> ~ Oleksii
> 
> 
> > 
> > Thanks, Roger.