Re: [PATCH v1.1 2/2] x86/hpet: Don't enable legacy replacement mode unconditionally

[Tamas K Lengyel <tamas.k.lengyel@xxxxxxxxx>]

On Fri, Mar 26, 2021 at 10:23 AM Marek Marczykowski-Górecki
<marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Fri, Mar 26, 2021 at 01:15:42PM +0000, Ian Jackson wrote:
> > Tamas K Lengyel writes ("Re: [PATCH v1.1 2/2] x86/hpet: Don't enable legacy 
> > replacement mode unconditionally"):
> > > The problem from my perspective is that the end-users have no way to
> > > determine when that boot option is needing to be set. Having an
> > > installation step of "check if things explode when you reboot" is just
> > > plain bad. Many times you don't even have access to a remote serial
> > > console to check why Xen didn't boot. So that's not a realistic route
> > > that can be taken. If Jan's patch is applied then the only thing I'll
> > > be able to do is make all installations always-enable this option even
> > > on systems that would have booted fine otherwise without it. It is
> > > unclear if that has any downsides of its own and could very well just
> > > kick the can down the road and lead to other issues.
> >
> > Thanks for the clear explanation.
> >
> > I think our options are:
> >
> >  1. What is currently in xen.git#staging-4.15: some different set of
> >     machines do not work (reliably? at all?), constituting a
> >     regression on older hardware.
> >
> >  2. Jan's patch, with the consequences you describe.  Constituing a
> >     continued failure to properly support the newer hardware.
> >
> >  3. Andy's patches which are not finished yet and are therefore high
> >     risk.  Ie, delay the release.
>
> I do have several confirmations that the "x86/timer: Fix boot on Intel
> systems using ITSSPRC static PIT clock gating" patch indeed unbreaks
> several Intel systems. And only one report about it causing a regression
> on some AMD (although I may miss some others on the list).
> Reverting to the previous default behavior I would also call a
> regression.
>
> I have tested Andy's patches on several machines and I can confirm they
> fixed the issue - both keep the original issue fixed and fixes the
> regression.
> I see also Frédéric (who originally reported the regression) also
> confirms it fixes it for him.
>
> > Please let me know if you think this characterisation of the situation
> > is inaccurate or misleading.
>
> Both versions (with "x86/timer: Fix boot on Intel systems using ITSSPRC
> static PIT clock gating" and without it) got significant testing and
> results are as you summarize - each of those variants alone is broken on
> some subset of hardware. What Andrew's patches do is to combine both
> versions into one, to choose the right behavior depending on the
> hardware. Specifically, apply the workaround in place of direct panic.
> This isn't some completely new behavior. I think it is reasonably safe
> to have it included in the release, even at such late time.

My preference would also be going with route 3, if possible in 4.15
from the start. If that can't happen without significant delay to the
release then it should be the first patch to be included for 4.15.1.

Thanks,
Tamas