On 30.06.2022 10:54, Roger Pau Monne wrote:
> Logic in ioapic_init() that sets the number of available vectors for
> external interrupts requires knowing the x2APIC Destination Mode. As
> such move the call after x2APIC BSP setup.
"requires" reads as if this was the case already, which I don't think
is true. The dependency likely appears with the next patch (didn't
look there, yet).
> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -2653,16 +2653,6 @@ void __init ioapic_init(void)
> max_gsi_irqs, nr_irqs_gsi);
> nr_irqs_gsi = max_gsi_irqs;
> }
> -
> - if ( nr_irqs == 0 )
> - nr_irqs = cpu_has_apic ?
> - max(0U + num_present_cpus() * NR_DYNAMIC_VECTORS,
> - 8 * nr_irqs_gsi) :
> - nr_irqs_gsi;
> - else if ( nr_irqs < 16 )
> - nr_irqs = 16;
> - printk(XENLOG_INFO "IRQ limits: %u GSI, %u MSI/MSI-X\n",
> - nr_irqs_gsi, nr_irqs - nr_irqs_gsi);
> }
>
> unsigned int arch_hwdom_irqs(domid_t domid)
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -420,6 +420,16 @@ int __init init_irq_data(void)
> struct irq_desc *desc;
> int irq, vector;
>
> + if ( nr_irqs == 0 )
> + nr_irqs = cpu_has_apic ? max(0U + num_present_cpus() *
> + NR_DYNAMIC_VECTORS, 8 * nr_irqs_gsi)
> + : nr_irqs_gsi;
Splitting a function argument across lines and then putting the next
argument on the same line is, well, confusing. May I suggest to either
stick to the original line splitting or to go to
if ( nr_irqs == 0 )
nr_irqs = cpu_has_apic
? max(0U + num_present_cpus() * NR_DYNAMIC_VECTORS,
8 * nr_irqs_gsi)
: nr_irqs_gsi;
?
Jan
