Re: [PATCH] x86/time: minor adjustments to init_pit()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.01.2022 17:17, Andrew Cooper wrote:
> On 17/01/2022 10:36, Jan Beulich wrote:
>> For one, "using_pit" shouldn't be set ahead of the function's last
>> (for now: only) error path. Otherwise "clocksource=pit" on the command
>> line can lead to misbehavior when actually taking that error path.
>>
>> And then make an implicit assumption explicit: CALIBRATE_FRAC cannot,
>> for example, simply be changed to 10. The way init_pit() works, the
>> upper bound on the calibration period is about 54ms.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Thanks.

>> ---
>> Really I've noticed this while considering what would happen if someone
>> specified  "clocksource=pit" on the shim's command line. Unlike "hpet"
>> and "acpi", "pit" presently wouldn't be (explicitly) ignored. While,
>> aiui, right now the only error path would be taken (due to port 0x61
>> reads being supposed to get back 0xff), I don't think we can build on
>> that longer term: Seeing what we use port 0x61 for in traps.c, I think
>> sooner or later we will need to have some form of emulation for it. Such
>> emulation is then not unlikely to continuously report 0 in the bit in
>> question. That would leed to an infinite loop here.
> 
> If we're not already doing it, pv shim really ought to set the FADT
> hardware reduced bits.  There should be no need to depend on heuristics
> around ~0.

Before forcing this flag onto "others", I guess we'd better first
start properly honoring this mode ourselves? Outside of ACPICA code
there has been only a single use of this FADT bit so far ...

> I do suspect that the emulation for port 0x61 is obsolete enough for us
> to consider dropping.

Well, as always - I'm hesitant to drop code which we don't know for
sure cannot possibly be of use to anyone anymore, and which also isn't
known to cause (significant) harm.

Jan