Re: [XEN PATCH] x86/hpet: Disable legacy replacement mode after IRQ test if not needed

[Simon Gaiser <simon@xxxxxxxxxxxxxxxxxxxxxx>]

Jan Beulich:
> On 18.07.2023 23:51, Simon Gaiser wrote:
>> Roger Pau Monné:
>>> On Tue, Jul 18, 2023 at 02:26:03PM +0200, Simon Gaiser wrote:
>>>> As far as I understand the HPET legacy mode is not required on systems
>>>> with ARAT after the timer IRQ test.
>>>
>>> What's the relation with ARAT here?
>>>
>>> It would seem to me that keeping legacy replacement enabled should
>>> only be done when opt_hpet_legacy_replacement > 0, and the currently
>>> modified block is already in a opt_hpet_legacy_replacement < 0 gated
>>> chunk.
>>
>> I was concerned that on systems without ARAT cpuidle might rely on HPET
>> legacy mode being available. See _disable_pit_irq and lapic_timer_init.
>> But now that I stared at this again, I think that condition isn't
>> actually needed. If we reach that code we know that we have no working
>> PIT, but HPET is working. So _disable_pit_irq which is run after
>> check_timer (__start_xen first calls check_timer via smp_prepare_cpus
>> and only later disable_pit_irq via do_initcalls) will setup HPET
>> broadcast, which should succeed since HPET worked previously.
>>
>> So I guess we can just drop the condition (please double check, that
>> code is quite tangled and I'm not familiar with it).
> 
> What you want to respect instead though is opt_hpet_legacy_replacement.

Can you please explain what behavior you expect? As Roger pointed out
this code only runs with opt_hpet_legacy_replacement < 0 so the user
didn't make an explicit choice. In that case enabling the legacy mode
for the timer IRQ test and then disabling it to allow lower power modes
seems reasonable to me.

Simon

Attachment: OpenPGP_signature
Description: OpenPGP digital signature