Re: [PATCH][4.15] x86/HPET: don't enable legacy replacement mode unconditionally

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

On Wed, Mar 24, 2021 at 11:34:32AM +0100, Jan Beulich wrote:
> Commit e1de4c196a2e ("x86/timer: Fix boot on Intel systems using ITSSPRC
> static PIT clock gating") was reported to cause boot failures on certain
> AMD Ryzen systems. Until we can figure out what the actual issue there
> is, skip this new part of HPET setup by default. Introduce a "hpet"
> command line option to allow enabling this on hardware where it's really
> needed for Xen to boot successfully (i.e. where the PIT doesn't drive
> the timer interrupt).
> 
> Since it makes little sense to introduce just "hpet=legacy-replacement",
> also allow for a boolean argument as well as "broadcast" to replace the
> separate "hpetbroadcast" option.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

I think the commit does what it saying on the commit message, hence:

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

I would like to avoid such RB being seen as me deciding on which
option is best release wise.

Haven't followed the other discussion closely as I'm on PTO today, but
maybe it's an issue worth thinking over during the weekend?

> 
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1274,9 +1274,26 @@ supported. See docs/misc/arm/big.LITTLE.
>  When the hmp-unsafe option is disabled (default), CPUs that are not
>  identical to the boot CPU will be parked and not used by Xen.
>  
> +### hpet (x86)
> +> `= List of [ <boolean> | broadcast | legacy-replacement ]`
> +
> +> Default : `true`, `no-broadcast`, 'no-legacy-replacement`
> +
> +Controls Xen's use of the system's High Precision Event Timer.  The boolean
> +allows to turn off use altogether.
> +
> +`broadcast` forces Xen to keep using the broadcast for CPUs in deep C-states
> +even when an RTC interrupt got enabled.
> +
> +`legacy-replacement` is intended to be used on platforms where the timer
> +interrupt doesn't get raised by the legacy PIT.  This then also affects
> +raising of the RTC interrupt.

I think Andrew rework of the change moved the x86 tag to a field on
the description instead of being in the title of the option and
arranged the options to be in list format, we might want to use that
instead, but can be adjusted later I guess since that would be a
documentation change.

Thanks, Roger.