On 11.12.2023 13:23, Julien Grall wrote:
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -2535,6 +2535,17 @@ pages) must also be specified via the tbuf_size 
> parameter.
>  ### tickle_one_idle_cpu
>  > `= <boolean>`
>  
> +### pit-irq-works (x86)
> +> `=<boolean>`
> +
> +> Default: `false`
> +
> +Disables the code which tests for broken timer IRQ sources. Enabling
> +this option will reduce boot time on HW where the timer works properly.
> +
> +If the system is unstable when enabling the option, then it means you
> +may have a broken HW and therefore the testing cannot be be skipped.
> +
>  ### timer_slop
>  > `= <integer>`

With the rename this now needs to move up to retain sorting.

> --- 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.

    if (pin1 != -1) {
        /*
         * Ok, does IRQ0 through the IOAPIC work?
         */
        unmask_IO_APIC_irq(irq_to_desc(0));
        if (pit_irq_works || timer_irq_works()) {
            local_irq_restore(flags);
            return;
        }

Plus this way changes to the various fallback paths can also be done
without needing to consider users who might be making use of the new
option.

Jan

Reply via email to