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.

Do it as part of init_irq_data(), which is called just after x2APIC
BSP init and also makes use of nr_irqs itself.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/io_apic.c | 10 ----------
 xen/arch/x86/irq.c     | 10 ++++++++++
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index c086f40f63..8d4923ba9a 100644
--- 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)
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index de30ee7779..b51e25f696 100644
--- 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;
+    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);
+
     for ( vector = 0; vector < X86_NR_VECTORS; ++vector )
         this_cpu(vector_irq)[vector] = INT_MIN;
 
-- 
2.36.1

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