On 14.12.2023 11:14, Julien Grall wrote:
> On 14/12/2023 10:10, Jan Beulich wrote:
>> On 11.12.2023 13:23, Julien Grall wrote:
>>> --- a/xen/arch/x86/io_apic.c
>>> +++ b/xen/arch/x86/io_apic.c
>>> @@ -57,6 +57,14 @@ bool __initdata ioapic_ack_forced;
>>> int __read_mostly nr_ioapic_entries[MAX_IO_APICS];
>>> int __read_mostly nr_ioapics;
>>>
>>> +/*
>>> + * The logic to check if the timer is working is expensive. So allow
>>> + * the admin to bypass it if they know their platform doesn't have
>>> + * a buggy timer.
>>> + */
>>> +static bool __initdata pit_irq_works;
>>> +boolean_param("pit-irq-works", pit_irq_works);
>>> +
>>> /*
>>> * Rough estimation of how many shared IRQs there are, can
>>> * be changed anytime.
>>> @@ -1502,6 +1510,9 @@ static int __init timer_irq_works(void)
>>> {
>>> unsigned long t1, flags;
>>>
>>> + if ( pit_irq_works )
>>> + return 1;
>>
>> When the check is placed here, what exactly use of the option means is
>> system dependent. I consider this somewhat risky, so I'd prefer if the
>> check was put on the "normal" path in check_timer(). That way it'll
>> affect only the one case which we can generally consider "known good",
>> but not the cases where the virtual wire setups are being probed. I.e.
>
> I am not against restricting when we allow skipping the timer check. But
> in that case, I wonder why Linux is doing it differently?
Sadly Linux'es git history doesn't go back far enough (begins only at past
2.6.11), so I can't (easily) find the patch (and description) for the x86-64
change. The later i386 change is justified mainly by paravirt needs, so
isn't applicable here. I wouldn't therefore exclude that my point above
wasn't even taken into consideration. Furthermore their command line option
is "no_timer_check", which to me firmly says "don't check" without regard to
whether the source (PIT) is actually okay. That's different with the option
name you (imo validly) chose.
Jan
