diff mbox

[v4] enable x2APIC without interrupt remapping under KVM

Message ID 20090630064515.GG20289@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gleb Natapov June 30, 2009, 6:45 a.m. UTC
KVM would like to provide x2APIC interface to a guest without emulating
interrupt remapping device. The reason KVM prefers guest to use x2APIC
is that x2APIC interface is better virtualizable and provides better
performance than mmio xAPIC interface:

- msr exits are faster than mmio (no page table walk, emulation)
- no need to read back ICR to look at the busy bit
- one 64 bit ICR write instead of two 32 bit writes
- shared code with the Hyper-V paravirt interface
    
Included patch changes x2APIC enabling logic to enable it even if IR
initialization failed, but kernel runs under KVM and no apic id is
greater than 255 (if there is one spec requires BIOS to move to x2apic
mode before starting an OS).

Signed-off-by: Gleb Natapov <gleb@redhat.com>
--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Yinghai Lu June 30, 2009, 7:18 a.m. UTC | #1
On Mon, Jun 29, 2009 at 11:45 PM, Gleb Natapov<gleb@redhat.com> wrote:
> KVM would like to provide x2APIC interface to a guest without emulating
> interrupt remapping device. The reason KVM prefers guest to use x2APIC
> is that x2APIC interface is better virtualizable and provides better
> performance than mmio xAPIC interface:
>
> - msr exits are faster than mmio (no page table walk, emulation)
> - no need to read back ICR to look at the busy bit
> - one 64 bit ICR write instead of two 32 bit writes
> - shared code with the Hyper-V paravirt interface
>
> Included patch changes x2APIC enabling logic to enable it even if IR
> initialization failed, but kernel runs under KVM and no apic id is
> greater than 255 (if there is one spec requires BIOS to move to x2apic
> mode before starting an OS).
>
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 8c7c042..351d55a 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -49,6 +49,7 @@
>  #include <asm/mtrr.h>
>  #include <asm/smp.h>
>  #include <asm/mce.h>
> +#include <asm/kvm_para.h>
>
>  unsigned int num_processors;
>
> @@ -1363,52 +1364,75 @@ void enable_x2apic(void)
>  }
>  #endif /* CONFIG_X86_X2APIC */
>
> -void __init enable_IR_x2apic(void)
> +int __init enable_IR(void)
>  {
>  #ifdef CONFIG_INTR_REMAP
>        int ret;
> -       unsigned long flags;
> -       struct IO_APIC_route_entry **ioapic_entries = NULL;
>
>        ret = dmar_table_init();
>        if (ret) {
>                pr_debug("dmar_table_init() failed with %d:\n", ret);
> -               goto ir_failed;
> +               return 0;
>        }
>
>        if (!intr_remapping_supported()) {
>                pr_debug("intr-remapping not supported\n");
> -               goto ir_failed;
> +               return 0;
>        }
>
> -
>        if (!x2apic_preenabled && skip_ioapic_setup) {
>                pr_info("Skipped enabling intr-remap because of skipping "
>                        "io-apic setup\n");
> -               return;
> +               return 0;
>        }
>
> +       if (enable_intr_remapping(x2apic_supported()))
> +               return 0;
> +
> +       pr_info("Enabled Interrupt-remapping\n");
> +
> +       return 1;
> +
> +#endif
> +       return 0;
> +}
> +
> +void __init enable_IR_x2apic(void)
> +{
> +       unsigned long flags;
> +       struct IO_APIC_route_entry **ioapic_entries = NULL;
> +       int ret, x2apic_enabled = 0;
> +
>        ioapic_entries = alloc_ioapic_entries();
>        if (!ioapic_entries) {
> -               pr_info("Allocate ioapic_entries failed: %d\n", ret);
> -               goto end;
> +                pr_info("Allocate ioapic_entries failed\n");
> +                goto out;
>        }
>
>        ret = save_IO_APIC_setup(ioapic_entries);
>        if (ret) {
>                pr_info("Saving IO-APIC state failed: %d\n", ret);
> -               goto end;
> +               goto out;
>        }
>
>        local_irq_save(flags);
> -       mask_IO_APIC_setup(ioapic_entries);
>        mask_8259A();
> +       mask_IO_APIC_setup(ioapic_entries);
>
> -       ret = enable_intr_remapping(x2apic_supported());
> -       if (ret)
> -               goto end_restore;
> +       ret = enable_IR();
> +       if (!ret) {
> +               /* IR is required if x2apic is enabled by BIOS even
> +                * when running in kvm since this indicates present
> +                * of APIC ID > 255 */
> +               if (x2apic_preenabled || !kvm_para_available())
> +                       goto nox2apic;
> +               else
> +                       /* without IR all CPUs can be addressed by IOAPIC/MSI
> +                        * only in physical mode */
> +                       x2apic_phys = 1;
> +       }

how about kexec second kernel in KVM ?

x2apic_preenabled will be set in second kernel.

YH
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov June 30, 2009, 7:54 a.m. UTC | #2
On Tue, Jun 30, 2009 at 12:18:19AM -0700, Yinghai Lu wrote:
> On Mon, Jun 29, 2009 at 11:45 PM, Gleb Natapov<gleb@redhat.com> wrote:
> > KVM would like to provide x2APIC interface to a guest without emulating
> > interrupt remapping device. The reason KVM prefers guest to use x2APIC
> > is that x2APIC interface is better virtualizable and provides better
> > performance than mmio xAPIC interface:
> >
> > - msr exits are faster than mmio (no page table walk, emulation)
> > - no need to read back ICR to look at the busy bit
> > - one 64 bit ICR write instead of two 32 bit writes
> > - shared code with the Hyper-V paravirt interface
> >
> > Included patch changes x2APIC enabling logic to enable it even if IR
> > initialization failed, but kernel runs under KVM and no apic id is
> > greater than 255 (if there is one spec requires BIOS to move to x2apic
> > mode before starting an OS).
> >
> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> > index 8c7c042..351d55a 100644
> > --- a/arch/x86/kernel/apic/apic.c
> > +++ b/arch/x86/kernel/apic/apic.c
> > @@ -49,6 +49,7 @@
> >  #include <asm/mtrr.h>
> >  #include <asm/smp.h>
> >  #include <asm/mce.h>
> > +#include <asm/kvm_para.h>
> >
> >  unsigned int num_processors;
> >
> > @@ -1363,52 +1364,75 @@ void enable_x2apic(void)
> >  }
> >  #endif /* CONFIG_X86_X2APIC */
> >
> > -void __init enable_IR_x2apic(void)
> > +int __init enable_IR(void)
> >  {
> >  #ifdef CONFIG_INTR_REMAP
> >        int ret;
> > -       unsigned long flags;
> > -       struct IO_APIC_route_entry **ioapic_entries = NULL;
> >
> >        ret = dmar_table_init();
> >        if (ret) {
> >                pr_debug("dmar_table_init() failed with %d:\n", ret);
> > -               goto ir_failed;
> > +               return 0;
> >        }
> >
> >        if (!intr_remapping_supported()) {
> >                pr_debug("intr-remapping not supported\n");
> > -               goto ir_failed;
> > +               return 0;
> >        }
> >
> > -
> >        if (!x2apic_preenabled && skip_ioapic_setup) {
> >                pr_info("Skipped enabling intr-remap because of skipping "
> >                        "io-apic setup\n");
> > -               return;
> > +               return 0;
> >        }
> >
> > +       if (enable_intr_remapping(x2apic_supported()))
> > +               return 0;
> > +
> > +       pr_info("Enabled Interrupt-remapping\n");
> > +
> > +       return 1;
> > +
> > +#endif
> > +       return 0;
> > +}
> > +
> > +void __init enable_IR_x2apic(void)
> > +{
> > +       unsigned long flags;
> > +       struct IO_APIC_route_entry **ioapic_entries = NULL;
> > +       int ret, x2apic_enabled = 0;
> > +
> >        ioapic_entries = alloc_ioapic_entries();
> >        if (!ioapic_entries) {
> > -               pr_info("Allocate ioapic_entries failed: %d\n", ret);
> > -               goto end;
> > +                pr_info("Allocate ioapic_entries failed\n");
> > +                goto out;
> >        }
> >
> >        ret = save_IO_APIC_setup(ioapic_entries);
> >        if (ret) {
> >                pr_info("Saving IO-APIC state failed: %d\n", ret);
> > -               goto end;
> > +               goto out;
> >        }
> >
> >        local_irq_save(flags);
> > -       mask_IO_APIC_setup(ioapic_entries);
> >        mask_8259A();
> > +       mask_IO_APIC_setup(ioapic_entries);
> >
> > -       ret = enable_intr_remapping(x2apic_supported());
> > -       if (ret)
> > -               goto end_restore;
> > +       ret = enable_IR();
> > +       if (!ret) {
> > +               /* IR is required if x2apic is enabled by BIOS even
> > +                * when running in kvm since this indicates present
> > +                * of APIC ID > 255 */
> > +               if (x2apic_preenabled || !kvm_para_available())
> > +                       goto nox2apic;
> > +               else
> > +                       /* without IR all CPUs can be addressed by IOAPIC/MSI
> > +                        * only in physical mode */
> > +                       x2apic_phys = 1;
> > +       }
> 
> how about kexec second kernel in KVM ?
> 
> x2apic_preenabled will be set in second kernel.
> 
Yes, bummer. But the similar problem exist now and without KVM. After
running x2apic enabled kernel you can't kexec kernel without x2apic
support. Shouldn't apic be reset to its initial state on reboot?

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov June 30, 2009, 3:58 p.m. UTC | #3
On Tue, Jun 30, 2009 at 12:18:19AM -0700, Yinghai Lu wrote:
> how about kexec second kernel in KVM ?
> 
> x2apic_preenabled will be set in second kernel.
> 
By the way anybody knows why kexec does not use BIOS reset code
(cmos 0xf offset) to jump into new kernel after hard reset?

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric W. Biederman June 30, 2009, 5 p.m. UTC | #4
Gleb Natapov <gleb@redhat.com> writes:

> On Tue, Jun 30, 2009 at 12:18:19AM -0700, Yinghai Lu wrote:
>> how about kexec second kernel in KVM ?
>> 
>> x2apic_preenabled will be set in second kernel.
>> 
> By the way anybody knows why kexec does not use BIOS reset code
> (cmos 0xf offset) to jump into new kernel after hard reset?

After a hard reset.  That simply isn't possible. A hard
reset clears everything even memory.

You might be able to get a full cpu reset but not a reset of the I/O
devices.

The premise of kexec is that we are doing things on our own, and don't
get a 3rd piece of software involved that has not been heavily tested
on the path we want to use.  Occassionally it is a pain to do everything
ourselves but at least when we do and we test it we know it is going
to work.

Cpu designers lately seem fond of adding features that require all
kind of coordination to turn on and off.  We handle the hardware
virtualization mode features now, and if x2apic has similar problems
being turned on and off I am certain we can handle that case in a
similar fashion.

When we can my preference is to keep code like that out of the
kexec on panic path if we can figure out how to write the software
to do something reasonable.

Once we figure out how to work without putting the interrupt controllers
in legacy mode to handle the timer interrupts I expect all kinds of
things will become simpler.

Eric
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity June 30, 2009, 5:14 p.m. UTC | #5
On 06/30/2009 08:00 PM, Eric W. Biederman wrote:
> Gleb Natapov<gleb@redhat.com>  writes:
>
>    
>> On Tue, Jun 30, 2009 at 12:18:19AM -0700, Yinghai Lu wrote:
>>      
>>> how about kexec second kernel in KVM ?
>>>
>>> x2apic_preenabled will be set in second kernel.
>>>
>>>        
>> By the way anybody knows why kexec does not use BIOS reset code
>> (cmos 0xf offset) to jump into new kernel after hard reset?
>>      
>
> After a hard reset.  That simply isn't possible. A hard
> reset clears everything even memory.
>
>    

So the jump-to-vector reset code cannot work on real hardware?  It works 
in qemu, but of course that's easy.

> You might be able to get a full cpu reset but not a reset of the I/O
> devices.
>
> The premise of kexec is that we are doing things on our own, and don't
> get a 3rd piece of software involved that has not been heavily tested
> on the path we want to use.  Occassionally it is a pain to do everything
> ourselves but at least when we do and we test it we know it is going
> to work.
>
> Cpu designers lately seem fond of adding features that require all
> kind of coordination to turn on and off.  We handle the hardware
> virtualization mode features now, and if x2apic has similar problems
> being turned on and off I am certain we can handle that case in a
> similar fashion.
>
> When we can my preference is to keep code like that out of the
> kexec on panic path if we can figure out how to write the software
> to do something reasonable.
>
> Once we figure out how to work without putting the interrupt controllers
> in legacy mode to handle the timer interrupts I expect all kinds of
> things will become simpler.
>    

I think it makes sense to add full reset support as a non-default 
option.  Leaving hardware running and dmaing left and right has its risks.
Eric W. Biederman June 30, 2009, 5:15 p.m. UTC | #6
Gleb Natapov <gleb@redhat.com> writes:

> On Tue, Jun 30, 2009 at 12:18:19AM -0700, Yinghai Lu wrote:
>> how about kexec second kernel in KVM ?
>> 
>> x2apic_preenabled will be set in second kernel.
>> 
> By the way anybody knows why kexec does not use BIOS reset code
> (cmos 0xf offset) to jump into new kernel after hard reset?

What I said before plus the fact that kexec grew up on systems
that don't have a BIOS.  So the option of using that piece of
was not there.

Eric
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yinghai Lu June 30, 2009, 6:43 p.m. UTC | #7
2009/6/30 Gleb Natapov <gleb@redhat.com>:

>>
>> how about kexec second kernel in KVM ?
>>
>> x2apic_preenabled will be set in second kernel.
>>
> Yes, bummer. But the similar problem exist now and without KVM. After
> running x2apic enabled kernel you can't kexec kernel without x2apic
> support.

if the first kernel enable x2apic and intr_remapping, and second
kernel has x2apic and intr_remap support.
we can kexec the second kernel.

restoring to original state is not possible for kdump case.

maybe with minor change to your patch, you could kexec the in kexec
second kernel with x2apic support.

YH
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov June 30, 2009, 6:53 p.m. UTC | #8
On Tue, Jun 30, 2009 at 11:43:41AM -0700, Yinghai Lu wrote:
> 2009/6/30 Gleb Natapov <gleb@redhat.com>:
> 
> >>
> >> how about kexec second kernel in KVM ?
> >>
> >> x2apic_preenabled will be set in second kernel.
> >>
> > Yes, bummer. But the similar problem exist now and without KVM. After
> > running x2apic enabled kernel you can't kexec kernel without x2apic
> > support.
> 
> if the first kernel enable x2apic and intr_remapping, and second
> kernel has x2apic and intr_remap support.
> we can kexec the second kernel.
> 
And if it hasn't we cannot, that is what I am trying to point out.

> restoring to original state is not possible for kdump case.
> 
Shouldn't it be enough to run UP kernel for kdump? So kdump can reset BSP
apic to xAPIC mode before starting a kernel with maxcpus=1.

> maybe with minor change to your patch, you could kexec the in kexec
> second kernel with x2apic support.
> 
My patch can easily avoid the issue by checking max_physical_apicid > 255
instead of x2apic_preenabled.

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov June 30, 2009, 7:15 p.m. UTC | #9
On Tue, Jun 30, 2009 at 10:00:46AM -0700, Eric W. Biederman wrote:
> Gleb Natapov <gleb@redhat.com> writes:
> 
> > On Tue, Jun 30, 2009 at 12:18:19AM -0700, Yinghai Lu wrote:
> >> how about kexec second kernel in KVM ?
> >> 
> >> x2apic_preenabled will be set in second kernel.
> >> 
> > By the way anybody knows why kexec does not use BIOS reset code
> > (cmos 0xf offset) to jump into new kernel after hard reset?
> 
> After a hard reset.  That simply isn't possible. A hard
> reset clears everything even memory.
> 
I mean "soft reset". At least on 286 in order to move from protected
mode to real mode "soft reset" was issued via kbd controller or via
triple fault. BIOS reset code was used to avoid POST. Obviously back
than this kind of reset haven't cleared memory. I'll try to check what
happens on a modern HW.

BTW Linux sets BIOS reset code to "return by long jump" in
smpboot_setup_warm_reset() for some reason.

> You might be able to get a full cpu reset but not a reset of the I/O
> devices.
> 
> The premise of kexec is that we are doing things on our own, and don't
> get a 3rd piece of software involved that has not been heavily tested
> on the path we want to use.  Occassionally it is a pain to do everything
> ourselves but at least when we do and we test it we know it is going
> to work.
> 
> Cpu designers lately seem fond of adding features that require all
> kind of coordination to turn on and off.  We handle the hardware
> virtualization mode features now, and if x2apic has similar problems
> being turned on and off I am certain we can handle that case in a
> similar fashion.
> 
> When we can my preference is to keep code like that out of the
> kexec on panic path if we can figure out how to write the software
> to do something reasonable.
> 
> Once we figure out how to work without putting the interrupt controllers
> in legacy mode to handle the timer interrupts I expect all kinds of
> things will become simpler.
> 
If the "soft reset" method can work it worth to implemented. I am not
suggesting we should make it default (just line --real-mode is not default).

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric W. Biederman June 30, 2009, 7:24 p.m. UTC | #10
Avi Kivity <avi@redhat.com> writes:

> So the jump-to-vector reset code cannot work on real hardware?  It works in
> qemu, but of course that's easy.

In general yes.  Nothing regularly uses that path so there is little
point adding a new users if we can avoid it.

Heck there are machines with unreliable cmos.

>> You might be able to get a full cpu reset but not a reset of the I/O
>> devices.
>>
>> The premise of kexec is that we are doing things on our own, and don't
>> get a 3rd piece of software involved that has not been heavily tested
>> on the path we want to use.  Occassionally it is a pain to do everything
>> ourselves but at least when we do and we test it we know it is going
>> to work.
>>
>> Cpu designers lately seem fond of adding features that require all
>> kind of coordination to turn on and off.  We handle the hardware
>> virtualization mode features now, and if x2apic has similar problems
>> being turned on and off I am certain we can handle that case in a
>> similar fashion.
>>
>> When we can my preference is to keep code like that out of the
>> kexec on panic path if we can figure out how to write the software
>> to do something reasonable.
>>
>> Once we figure out how to work without putting the interrupt controllers
>> in legacy mode to handle the timer interrupts I expect all kinds of
>> things will become simpler.
>>    
>
> I think it makes sense to add full reset support as a non-default option.
> Leaving hardware running and dmaing left and right has its risks.

It is easy enough to do that in user space in /sbin/kexec if that proves
useful for some reason.

As for the leaving hardware running and dmaing left and right.  You
are talking about the kexec on panic code path.  On a normal reboot
we do our darndest to cleanly shut everything down properly.

We avoid the dmaing left and right problem by reserving a chunk of
memory at boot time and we probably should be reserving a chunk
of the iommus as well for that purpose.

Hardware resets in general don't preserve the contents of memory.

In general the kexec on panic path is about teaching drivers to
initialize in the worst possible scenario and using those drivers
that can to get information out of the system.

At the same time 99% of the code that runs is in user space so
it should be easy to jump through whatever hoops make sense in
the user space component.  While keeping the kexec on panic code
path as slim as possible.

Eric
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 8c7c042..351d55a 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -49,6 +49,7 @@ 
 #include <asm/mtrr.h>
 #include <asm/smp.h>
 #include <asm/mce.h>
+#include <asm/kvm_para.h>
 
 unsigned int num_processors;
 
@@ -1363,52 +1364,75 @@  void enable_x2apic(void)
 }
 #endif /* CONFIG_X86_X2APIC */
 
-void __init enable_IR_x2apic(void)
+int __init enable_IR(void)
 {
 #ifdef CONFIG_INTR_REMAP
 	int ret;
-	unsigned long flags;
-	struct IO_APIC_route_entry **ioapic_entries = NULL;
 
 	ret = dmar_table_init();
 	if (ret) {
 		pr_debug("dmar_table_init() failed with %d:\n", ret);
-		goto ir_failed;
+		return 0;
 	}
 
 	if (!intr_remapping_supported()) {
 		pr_debug("intr-remapping not supported\n");
-		goto ir_failed;
+		return 0;
 	}
 
-
 	if (!x2apic_preenabled && skip_ioapic_setup) {
 		pr_info("Skipped enabling intr-remap because of skipping "
 			"io-apic setup\n");
-		return;
+		return 0;
 	}
 
+	if (enable_intr_remapping(x2apic_supported()))
+		return 0;
+
+	pr_info("Enabled Interrupt-remapping\n");
+
+	return 1;
+
+#endif
+	return 0;
+}
+
+void __init enable_IR_x2apic(void)
+{
+	unsigned long flags;
+	struct IO_APIC_route_entry **ioapic_entries = NULL;
+	int ret, x2apic_enabled = 0;
+
 	ioapic_entries = alloc_ioapic_entries();
 	if (!ioapic_entries) {
-		pr_info("Allocate ioapic_entries failed: %d\n", ret);
-		goto end;
+		 pr_info("Allocate ioapic_entries failed\n");
+		 goto out;
 	}
 
 	ret = save_IO_APIC_setup(ioapic_entries);
 	if (ret) {
 		pr_info("Saving IO-APIC state failed: %d\n", ret);
-		goto end;
+		goto out;
 	}
 
 	local_irq_save(flags);
-	mask_IO_APIC_setup(ioapic_entries);
 	mask_8259A();
+	mask_IO_APIC_setup(ioapic_entries);
 
-	ret = enable_intr_remapping(x2apic_supported());
-	if (ret)
-		goto end_restore;
+	ret = enable_IR();
+	if (!ret) {
+		/* IR is required if x2apic is enabled by BIOS even
+		 * when running in kvm since this indicates present
+		 * of APIC ID > 255 */
+	       	if (x2apic_preenabled || !kvm_para_available())
+			goto nox2apic;
+		else
+			/* without IR all CPUs can be addressed by IOAPIC/MSI
+			 * only in physical mode */
+			x2apic_phys = 1;
+	}
 
-	pr_info("Enabled Interrupt-remapping\n");
+	x2apic_enabled = 1;
 
 	if (x2apic_supported() && !x2apic_mode) {
 		x2apic_mode = 1;
@@ -1416,41 +1440,30 @@  void __init enable_IR_x2apic(void)
 		pr_info("Enabled x2apic\n");
 	}
 
-end_restore:
-	if (ret)
-		/*
-		 * IR enabling failed
-		 */
+nox2apic:
+	if (!ret) /* IR enabling failed */
 		restore_IO_APIC_setup(ioapic_entries);
-
 	unmask_8259A();
 	local_irq_restore(flags);
 
-end:
+out:
 	if (ioapic_entries)
 		free_ioapic_entries(ioapic_entries);
 
-	if (!ret)
+	if (x2apic_enabled)
 		return;
 
-ir_failed:
-	if (x2apic_preenabled)
+	if (x2apic_preenabled) {
+#ifdef CONFIG_INTR_REMAP
 		panic("x2apic enabled by bios. But IR enabling failed");
-	else if (cpu_has_x2apic)
-		pr_info("Not enabling x2apic,Intr-remapping\n");
 #else
-	if (!cpu_has_x2apic)
-		return;
-
-	if (x2apic_preenabled)
 		panic("x2apic enabled prior OS handover,"
-		      " enable CONFIG_X86_X2APIC, CONFIG_INTR_REMAP");
+				" enable CONFIG_X86_X2APIC, CONFIG_INTR_REMAP");
 #endif
-
-	return;
+	} else if (cpu_has_x2apic)
+		pr_info("Not enabling x2apic,Intr-remapping\n");
 }
 
-
 #ifdef CONFIG_X86_64
 /*
  * Detect and enable local APICs on non-SMP boards.