[3/3] xen/x86: ioapic: Simplify ioapic_init()
diff mbox series

Message ID 20200327190546.21580-4-julien@xen.org
State New
Headers show
Series
  • xen/x86: Simplify ioapic_init()
Related show

Commit Message

Julien Grall March 27, 2020, 7:05 p.m. UTC
From: Julien Grall <jgrall@amazon.com>

Since commit 9facd54a45 "x86/ioapic: Add register level checks to detect
bogus io-apic entries", Xen is able to cope with IO APICs not mapped in
the fixmap.

Therefore the whole logic to allocate a fake page for some IO APICs is
unnecessary.

With the logic removed, the code can be simplified a lot as we don't
need to go through all the IO APIC if SMP has not been detected or a
bogus zero IO-APIC address has been detected.

To avoid another level of tabulation, the simplification is now moved in
its own function.

Signed-off-by: Julien Grall <jgrall@amazon.com>
---
 xen/arch/x86/io_apic.c | 63 ++++++++++++++++++++----------------------
 1 file changed, 30 insertions(+), 33 deletions(-)

Comments

Wei Liu March 29, 2020, 2:35 p.m. UTC | #1
On Fri, Mar 27, 2020 at 07:05:46PM +0000, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Since commit 9facd54a45 "x86/ioapic: Add register level checks to detect
> bogus io-apic entries", Xen is able to cope with IO APICs not mapped in
> the fixmap.
> 
> Therefore the whole logic to allocate a fake page for some IO APICs is
> unnecessary.
> 
> With the logic removed, the code can be simplified a lot as we don't
> need to go through all the IO APIC if SMP has not been detected or a
> bogus zero IO-APIC address has been detected.
> 
> To avoid another level of tabulation, the simplification is now moved in
> its own function.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> ---
>  xen/arch/x86/io_apic.c | 63 ++++++++++++++++++++----------------------
>  1 file changed, 30 insertions(+), 33 deletions(-)
> 
> diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
> index 9a11ee8342..3d52e4daf1 100644
> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -2537,34 +2537,25 @@ static __init bool bad_ioapic_register(unsigned int idx)
>      return false;
>  }
>  
> -void __init init_ioapic(void)
> +static void __init init_ioapic_mappings(void)
>  {
> -    unsigned long ioapic_phys;
>      unsigned int i, idx = FIX_IO_APIC_BASE_0;
> -    union IO_APIC_reg_01 reg_01;
>  
> -    if ( smp_found_config )
> -        nr_irqs_gsi = 0;
>      for ( i = 0; i < nr_ioapics; i++ )
>      {
> -        if ( smp_found_config )
> -        {
> -            ioapic_phys = mp_ioapics[i].mpc_apicaddr;
> -            if ( !ioapic_phys )
> -            {
> -                printk(KERN_ERR "WARNING: bogus zero IO-APIC address "
> -                       "found in MPTABLE, disabling IO/APIC support!\n");
> -                smp_found_config = false;
> -                skip_ioapic_setup = true;
> -                goto fake_ioapic_page;
> -            }
> -        }
> -        else
> +        union IO_APIC_reg_01 reg_01;
> +        unsigned long ioapic_phys = mp_ioapics[i].mpc_apicaddr;
> +
> +        ioapic_phys = mp_ioapics[i].mpc_apicaddr;

ioapic_phys is set a second time here. See the line before.

Wei.
Julien Grall March 29, 2020, 3:54 p.m. UTC | #2
Hi Wei,

On 29/03/2020 15:35, Wei Liu wrote:
> On Fri, Mar 27, 2020 at 07:05:46PM +0000, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> Since commit 9facd54a45 "x86/ioapic: Add register level checks to detect
>> bogus io-apic entries", Xen is able to cope with IO APICs not mapped in
>> the fixmap.
>>
>> Therefore the whole logic to allocate a fake page for some IO APICs is
>> unnecessary.
>>
>> With the logic removed, the code can be simplified a lot as we don't
>> need to go through all the IO APIC if SMP has not been detected or a
>> bogus zero IO-APIC address has been detected.
>>
>> To avoid another level of tabulation, the simplification is now moved in
>> its own function.
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>> ---
>>   xen/arch/x86/io_apic.c | 63 ++++++++++++++++++++----------------------
>>   1 file changed, 30 insertions(+), 33 deletions(-)
>>
>> diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
>> index 9a11ee8342..3d52e4daf1 100644
>> --- a/xen/arch/x86/io_apic.c
>> +++ b/xen/arch/x86/io_apic.c
>> @@ -2537,34 +2537,25 @@ static __init bool bad_ioapic_register(unsigned int idx)
>>       return false;
>>   }
>>   
>> -void __init init_ioapic(void)
>> +static void __init init_ioapic_mappings(void)
>>   {
>> -    unsigned long ioapic_phys;
>>       unsigned int i, idx = FIX_IO_APIC_BASE_0;
>> -    union IO_APIC_reg_01 reg_01;
>>   
>> -    if ( smp_found_config )
>> -        nr_irqs_gsi = 0;
>>       for ( i = 0; i < nr_ioapics; i++ )
>>       {
>> -        if ( smp_found_config )
>> -        {
>> -            ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>> -            if ( !ioapic_phys )
>> -            {
>> -                printk(KERN_ERR "WARNING: bogus zero IO-APIC address "
>> -                       "found in MPTABLE, disabling IO/APIC support!\n");
>> -                smp_found_config = false;
>> -                skip_ioapic_setup = true;
>> -                goto fake_ioapic_page;
>> -            }
>> -        }
>> -        else
>> +        union IO_APIC_reg_01 reg_01;
>> +        unsigned long ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>> +
>> +        ioapic_phys = mp_ioapics[i].mpc_apicaddr;
> 
> ioapic_phys is set a second time here. See the line before.

Good spot! I will drop the line.

Cheers,
Roger Pau Monne March 30, 2020, 10:43 a.m. UTC | #3
On Fri, Mar 27, 2020 at 07:05:46PM +0000, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Since commit 9facd54a45 "x86/ioapic: Add register level checks to detect
> bogus io-apic entries", Xen is able to cope with IO APICs not mapped in
> the fixmap.
> 
> Therefore the whole logic to allocate a fake page for some IO APICs is
> unnecessary.
> 
> With the logic removed, the code can be simplified a lot as we don't
> need to go through all the IO APIC if SMP has not been detected or a
> bogus zero IO-APIC address has been detected.
> 
> To avoid another level of tabulation, the simplification is now moved in
> its own function.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> ---
>  xen/arch/x86/io_apic.c | 63 ++++++++++++++++++++----------------------
>  1 file changed, 30 insertions(+), 33 deletions(-)
> 
> diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
> index 9a11ee8342..3d52e4daf1 100644
> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -2537,34 +2537,25 @@ static __init bool bad_ioapic_register(unsigned int idx)
>      return false;
>  }
>  
> -void __init init_ioapic(void)
> +static void __init init_ioapic_mappings(void)

Likewise my comment in 2/3 I would name this ioapic_init_mappings.

>  {
> -    unsigned long ioapic_phys;
>      unsigned int i, idx = FIX_IO_APIC_BASE_0;
> -    union IO_APIC_reg_01 reg_01;
>  
> -    if ( smp_found_config )
> -        nr_irqs_gsi = 0;
>      for ( i = 0; i < nr_ioapics; i++ )
>      {
> -        if ( smp_found_config )
> -        {
> -            ioapic_phys = mp_ioapics[i].mpc_apicaddr;
> -            if ( !ioapic_phys )
> -            {
> -                printk(KERN_ERR "WARNING: bogus zero IO-APIC address "
> -                       "found in MPTABLE, disabling IO/APIC support!\n");
> -                smp_found_config = false;
> -                skip_ioapic_setup = true;
> -                goto fake_ioapic_page;
> -            }
> -        }
> -        else
> +        union IO_APIC_reg_01 reg_01;
> +        unsigned long ioapic_phys = mp_ioapics[i].mpc_apicaddr;

Nit: paddr_t might be better here.

Thanks, Roger.
Julien Grall March 30, 2020, 12:56 p.m. UTC | #4
Hi Roger,

On 30/03/2020 11:43, Roger Pau Monné wrote:
> On Fri, Mar 27, 2020 at 07:05:46PM +0000, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> Since commit 9facd54a45 "x86/ioapic: Add register level checks to detect
>> bogus io-apic entries", Xen is able to cope with IO APICs not mapped in
>> the fixmap.
>>
>> Therefore the whole logic to allocate a fake page for some IO APICs is
>> unnecessary.
>>
>> With the logic removed, the code can be simplified a lot as we don't
>> need to go through all the IO APIC if SMP has not been detected or a
>> bogus zero IO-APIC address has been detected.
>>
>> To avoid another level of tabulation, the simplification is now moved in
>> its own function.
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>> ---
>>   xen/arch/x86/io_apic.c | 63 ++++++++++++++++++++----------------------
>>   1 file changed, 30 insertions(+), 33 deletions(-)
>>
>> diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
>> index 9a11ee8342..3d52e4daf1 100644
>> --- a/xen/arch/x86/io_apic.c
>> +++ b/xen/arch/x86/io_apic.c
>> @@ -2537,34 +2537,25 @@ static __init bool bad_ioapic_register(unsigned int idx)
>>       return false;
>>   }
>>   
>> -void __init init_ioapic(void)
>> +static void __init init_ioapic_mappings(void)
> 
> Likewise my comment in 2/3 I would name this ioapic_init_mappings.

Will do.

> 
>>   {
>> -    unsigned long ioapic_phys;
>>       unsigned int i, idx = FIX_IO_APIC_BASE_0;
>> -    union IO_APIC_reg_01 reg_01;
>>   
>> -    if ( smp_found_config )
>> -        nr_irqs_gsi = 0;
>>       for ( i = 0; i < nr_ioapics; i++ )
>>       {
>> -        if ( smp_found_config )
>> -        {
>> -            ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>> -            if ( !ioapic_phys )
>> -            {
>> -                printk(KERN_ERR "WARNING: bogus zero IO-APIC address "
>> -                       "found in MPTABLE, disabling IO/APIC support!\n");
>> -                smp_found_config = false;
>> -                skip_ioapic_setup = true;
>> -                goto fake_ioapic_page;
>> -            }
>> -        }
>> -        else
>> +        union IO_APIC_reg_01 reg_01;
>> +        unsigned long ioapic_phys = mp_ioapics[i].mpc_apicaddr;
> 
> Nit: paddr_t might be better here.

Sure.

Cheers,
Jan Beulich March 31, 2020, 11:22 a.m. UTC | #5
On 27.03.2020 20:05, Julien Grall wrote:
> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -2537,34 +2537,25 @@ static __init bool bad_ioapic_register(unsigned int idx)
>      return false;
>  }
>  
> -void __init init_ioapic(void)
> +static void __init init_ioapic_mappings(void)
>  {
> -    unsigned long ioapic_phys;
>      unsigned int i, idx = FIX_IO_APIC_BASE_0;
> -    union IO_APIC_reg_01 reg_01;
>  
> -    if ( smp_found_config )
> -        nr_irqs_gsi = 0;
>      for ( i = 0; i < nr_ioapics; i++ )
>      {
> -        if ( smp_found_config )
> -        {
> -            ioapic_phys = mp_ioapics[i].mpc_apicaddr;
> -            if ( !ioapic_phys )
> -            {
> -                printk(KERN_ERR "WARNING: bogus zero IO-APIC address "
> -                       "found in MPTABLE, disabling IO/APIC support!\n");
> -                smp_found_config = false;
> -                skip_ioapic_setup = true;
> -                goto fake_ioapic_page;
> -            }
> -        }
> -        else
> +        union IO_APIC_reg_01 reg_01;
> +        unsigned long ioapic_phys = mp_ioapics[i].mpc_apicaddr;
> +
> +        ioapic_phys = mp_ioapics[i].mpc_apicaddr;
> +        if ( !ioapic_phys )
>          {
> - fake_ioapic_page:
> -            ioapic_phys = __pa(alloc_xenheap_page());
> -            clear_page(__va(ioapic_phys));
> +            printk(KERN_ERR
> +                   "WARNING: bogus zero IO-APIC address found in MPTABLE, disabling IO/APIC support!\n");
> +            smp_found_config = false;
> +            skip_ioapic_setup = true;
> +            break;
>          }
> +
>          set_fixmap_nocache(idx, ioapic_phys);
>          apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08Lx (%08lx)\n",
>                      __fix_to_virt(idx), ioapic_phys);
> @@ -2576,18 +2567,24 @@ void __init init_ioapic(void)
>              continue;
>          }
>  
> -        if ( smp_found_config )
> -        {
> -            /* The number of IO-APIC IRQ registers (== #pins): */
> -            reg_01.raw = io_apic_read(i, 1);
> -            nr_ioapic_entries[i] = reg_01.bits.entries + 1;
> -            nr_irqs_gsi += nr_ioapic_entries[i];
> -
> -            if ( rangeset_add_singleton(mmio_ro_ranges,
> -                                        ioapic_phys >> PAGE_SHIFT) )
> -                printk(KERN_ERR "Failed to mark IO-APIC page %lx read-only\n",
> -                       ioapic_phys);
> -        }
> +        /* The number of IO-APIC IRQ registers (== #pins): */
> +        reg_01.raw = io_apic_read(i, 1);
> +        nr_ioapic_entries[i] = reg_01.bits.entries + 1;
> +        nr_irqs_gsi += nr_ioapic_entries[i];
> +
> +        if ( rangeset_add_singleton(mmio_ro_ranges,
> +                                    ioapic_phys >> PAGE_SHIFT) )
> +            printk(KERN_ERR "Failed to mark IO-APIC page %lx read-only\n",
> +                   ioapic_phys);
> +    }
> +}
> +
> +void __init init_ioapic(void)
> +{
> +    if ( smp_found_config )
> +    {
> +        nr_irqs_gsi = 0;

This would seem to also belong into the function, e.g. as part of
the loop header:

    for ( nr_irqs_gsi = i = 0; i < nr_ioapics; i++ )

Jan
Julien Grall March 31, 2020, 11:51 a.m. UTC | #6
Hi Jan,

On 31/03/2020 12:22, Jan Beulich wrote:
> On 27.03.2020 20:05, Julien Grall wrote:
>> --- a/xen/arch/x86/io_apic.c
>> +++ b/xen/arch/x86/io_apic.c
>> @@ -2537,34 +2537,25 @@ static __init bool bad_ioapic_register(unsigned int idx)
>>       return false;
>>   }
>>   
>> -void __init init_ioapic(void)
>> +static void __init init_ioapic_mappings(void)
>>   {
>> -    unsigned long ioapic_phys;
>>       unsigned int i, idx = FIX_IO_APIC_BASE_0;
>> -    union IO_APIC_reg_01 reg_01;
>>   
>> -    if ( smp_found_config )
>> -        nr_irqs_gsi = 0;
>>       for ( i = 0; i < nr_ioapics; i++ )
>>       {
>> -        if ( smp_found_config )
>> -        {
>> -            ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>> -            if ( !ioapic_phys )
>> -            {
>> -                printk(KERN_ERR "WARNING: bogus zero IO-APIC address "
>> -                       "found in MPTABLE, disabling IO/APIC support!\n");
>> -                smp_found_config = false;
>> -                skip_ioapic_setup = true;
>> -                goto fake_ioapic_page;
>> -            }
>> -        }
>> -        else
>> +        union IO_APIC_reg_01 reg_01;
>> +        unsigned long ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>> +
>> +        ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>> +        if ( !ioapic_phys )
>>           {
>> - fake_ioapic_page:
>> -            ioapic_phys = __pa(alloc_xenheap_page());
>> -            clear_page(__va(ioapic_phys));
>> +            printk(KERN_ERR
>> +                   "WARNING: bogus zero IO-APIC address found in MPTABLE, disabling IO/APIC support!\n");
>> +            smp_found_config = false;
>> +            skip_ioapic_setup = true;
>> +            break;
>>           }
>> +
>>           set_fixmap_nocache(idx, ioapic_phys);
>>           apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08Lx (%08lx)\n",
>>                       __fix_to_virt(idx), ioapic_phys);
>> @@ -2576,18 +2567,24 @@ void __init init_ioapic(void)
>>               continue;
>>           }
>>   
>> -        if ( smp_found_config )
>> -        {
>> -            /* The number of IO-APIC IRQ registers (== #pins): */
>> -            reg_01.raw = io_apic_read(i, 1);
>> -            nr_ioapic_entries[i] = reg_01.bits.entries + 1;
>> -            nr_irqs_gsi += nr_ioapic_entries[i];
>> -
>> -            if ( rangeset_add_singleton(mmio_ro_ranges,
>> -                                        ioapic_phys >> PAGE_SHIFT) )
>> -                printk(KERN_ERR "Failed to mark IO-APIC page %lx read-only\n",
>> -                       ioapic_phys);
>> -        }
>> +        /* The number of IO-APIC IRQ registers (== #pins): */
>> +        reg_01.raw = io_apic_read(i, 1);
>> +        nr_ioapic_entries[i] = reg_01.bits.entries + 1;
>> +        nr_irqs_gsi += nr_ioapic_entries[i];
>> +
>> +        if ( rangeset_add_singleton(mmio_ro_ranges,
>> +                                    ioapic_phys >> PAGE_SHIFT) )
>> +            printk(KERN_ERR "Failed to mark IO-APIC page %lx read-only\n",
>> +                   ioapic_phys);
>> +    }
>> +}
>> +
>> +void __init init_ioapic(void)
>> +{
>> +    if ( smp_found_config )
>> +    {
>> +        nr_irqs_gsi = 0;
> 
> This would seem to also belong into the function, e.g. as part of
> the loop header:
> 
>      for ( nr_irqs_gsi = i = 0; i < nr_ioapics; i++ )

While the initial value of the 'i' and 'nr_irqs_gsi' is the same, the 
variables have very different meaning and may confuse the reader.

So I would rather not follow your suggestion here. Although, I can move 
the zeroing right before the for loop.

Cheers,
Jan Beulich March 31, 2020, 12:12 p.m. UTC | #7
On 31.03.2020 13:51, Julien Grall wrote:
> Hi Jan,
> 
> On 31/03/2020 12:22, Jan Beulich wrote:
>> On 27.03.2020 20:05, Julien Grall wrote:
>>> --- a/xen/arch/x86/io_apic.c
>>> +++ b/xen/arch/x86/io_apic.c
>>> @@ -2537,34 +2537,25 @@ static __init bool bad_ioapic_register(unsigned int idx)
>>>       return false;
>>>   }
>>>   -void __init init_ioapic(void)
>>> +static void __init init_ioapic_mappings(void)
>>>   {
>>> -    unsigned long ioapic_phys;
>>>       unsigned int i, idx = FIX_IO_APIC_BASE_0;
>>> -    union IO_APIC_reg_01 reg_01;
>>>   -    if ( smp_found_config )
>>> -        nr_irqs_gsi = 0;
>>>       for ( i = 0; i < nr_ioapics; i++ )
>>>       {
>>> -        if ( smp_found_config )
>>> -        {
>>> -            ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>>> -            if ( !ioapic_phys )
>>> -            {
>>> -                printk(KERN_ERR "WARNING: bogus zero IO-APIC address "
>>> -                       "found in MPTABLE, disabling IO/APIC support!\n");
>>> -                smp_found_config = false;
>>> -                skip_ioapic_setup = true;
>>> -                goto fake_ioapic_page;
>>> -            }
>>> -        }
>>> -        else
>>> +        union IO_APIC_reg_01 reg_01;
>>> +        unsigned long ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>>> +
>>> +        ioapic_phys = mp_ioapics[i].mpc_apicaddr;
>>> +        if ( !ioapic_phys )
>>>           {
>>> - fake_ioapic_page:
>>> -            ioapic_phys = __pa(alloc_xenheap_page());
>>> -            clear_page(__va(ioapic_phys));
>>> +            printk(KERN_ERR
>>> +                   "WARNING: bogus zero IO-APIC address found in MPTABLE, disabling IO/APIC support!\n");
>>> +            smp_found_config = false;
>>> +            skip_ioapic_setup = true;
>>> +            break;
>>>           }
>>> +
>>>           set_fixmap_nocache(idx, ioapic_phys);
>>>           apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08Lx (%08lx)\n",
>>>                       __fix_to_virt(idx), ioapic_phys);
>>> @@ -2576,18 +2567,24 @@ void __init init_ioapic(void)
>>>               continue;
>>>           }
>>>   -        if ( smp_found_config )
>>> -        {
>>> -            /* The number of IO-APIC IRQ registers (== #pins): */
>>> -            reg_01.raw = io_apic_read(i, 1);
>>> -            nr_ioapic_entries[i] = reg_01.bits.entries + 1;
>>> -            nr_irqs_gsi += nr_ioapic_entries[i];
>>> -
>>> -            if ( rangeset_add_singleton(mmio_ro_ranges,
>>> -                                        ioapic_phys >> PAGE_SHIFT) )
>>> -                printk(KERN_ERR "Failed to mark IO-APIC page %lx read-only\n",
>>> -                       ioapic_phys);
>>> -        }
>>> +        /* The number of IO-APIC IRQ registers (== #pins): */
>>> +        reg_01.raw = io_apic_read(i, 1);
>>> +        nr_ioapic_entries[i] = reg_01.bits.entries + 1;
>>> +        nr_irqs_gsi += nr_ioapic_entries[i];
>>> +
>>> +        if ( rangeset_add_singleton(mmio_ro_ranges,
>>> +                                    ioapic_phys >> PAGE_SHIFT) )
>>> +            printk(KERN_ERR "Failed to mark IO-APIC page %lx read-only\n",
>>> +                   ioapic_phys);
>>> +    }
>>> +}
>>> +
>>> +void __init init_ioapic(void)
>>> +{
>>> +    if ( smp_found_config )
>>> +    {
>>> +        nr_irqs_gsi = 0;
>>
>> This would seem to also belong into the function, e.g. as part of
>> the loop header:
>>
>>      for ( nr_irqs_gsi = i = 0; i < nr_ioapics; i++ )
> 
> While the initial value of the 'i' and 'nr_irqs_gsi' is the same,
> the variables have very different meaning and may confuse the reader.

I expected reservations like this, hence the "e.g."; i.e. ...

> So I would rather not follow your suggestion here. Although, I can
> move the zeroing right before the for loop.

... I'm fine with this as well.

Jan

Patch
diff mbox series

diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index 9a11ee8342..3d52e4daf1 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -2537,34 +2537,25 @@  static __init bool bad_ioapic_register(unsigned int idx)
     return false;
 }
 
-void __init init_ioapic(void)
+static void __init init_ioapic_mappings(void)
 {
-    unsigned long ioapic_phys;
     unsigned int i, idx = FIX_IO_APIC_BASE_0;
-    union IO_APIC_reg_01 reg_01;
 
-    if ( smp_found_config )
-        nr_irqs_gsi = 0;
     for ( i = 0; i < nr_ioapics; i++ )
     {
-        if ( smp_found_config )
-        {
-            ioapic_phys = mp_ioapics[i].mpc_apicaddr;
-            if ( !ioapic_phys )
-            {
-                printk(KERN_ERR "WARNING: bogus zero IO-APIC address "
-                       "found in MPTABLE, disabling IO/APIC support!\n");
-                smp_found_config = false;
-                skip_ioapic_setup = true;
-                goto fake_ioapic_page;
-            }
-        }
-        else
+        union IO_APIC_reg_01 reg_01;
+        unsigned long ioapic_phys = mp_ioapics[i].mpc_apicaddr;
+
+        ioapic_phys = mp_ioapics[i].mpc_apicaddr;
+        if ( !ioapic_phys )
         {
- fake_ioapic_page:
-            ioapic_phys = __pa(alloc_xenheap_page());
-            clear_page(__va(ioapic_phys));
+            printk(KERN_ERR
+                   "WARNING: bogus zero IO-APIC address found in MPTABLE, disabling IO/APIC support!\n");
+            smp_found_config = false;
+            skip_ioapic_setup = true;
+            break;
         }
+
         set_fixmap_nocache(idx, ioapic_phys);
         apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08Lx (%08lx)\n",
                     __fix_to_virt(idx), ioapic_phys);
@@ -2576,18 +2567,24 @@  void __init init_ioapic(void)
             continue;
         }
 
-        if ( smp_found_config )
-        {
-            /* The number of IO-APIC IRQ registers (== #pins): */
-            reg_01.raw = io_apic_read(i, 1);
-            nr_ioapic_entries[i] = reg_01.bits.entries + 1;
-            nr_irqs_gsi += nr_ioapic_entries[i];
-
-            if ( rangeset_add_singleton(mmio_ro_ranges,
-                                        ioapic_phys >> PAGE_SHIFT) )
-                printk(KERN_ERR "Failed to mark IO-APIC page %lx read-only\n",
-                       ioapic_phys);
-        }
+        /* The number of IO-APIC IRQ registers (== #pins): */
+        reg_01.raw = io_apic_read(i, 1);
+        nr_ioapic_entries[i] = reg_01.bits.entries + 1;
+        nr_irqs_gsi += nr_ioapic_entries[i];
+
+        if ( rangeset_add_singleton(mmio_ro_ranges,
+                                    ioapic_phys >> PAGE_SHIFT) )
+            printk(KERN_ERR "Failed to mark IO-APIC page %lx read-only\n",
+                   ioapic_phys);
+    }
+}
+
+void __init init_ioapic(void)
+{
+    if ( smp_found_config )
+    {
+        nr_irqs_gsi = 0;
+        init_ioapic_mappings();
     }
 
     nr_irqs_gsi = max(nr_irqs_gsi, highest_gsi() + 1);