When interrupt-remapping is enabled, We are relying on setup_IO_APIC_irqs()
to configure remapped entries in the IO-APIC, which comes little bit later
after enabling interrupt-remapping.
Meanwhile, Restore of old io-apic entries after enabling interrupt-remapping
will not make the interrupts through io-apic functional anyway.
So remove unnecessary reinit_intr_remapped_IO_APIC().
Signed-off-by: Suresh Siddha <[email protected]>
Cc: Weidong Han <[email protected]>
---
Index: tip/arch/x86/include/asm/io_apic.h
===================================================================
--- tip.orig/arch/x86/include/asm/io_apic.h
+++ tip/arch/x86/include/asm/io_apic.h
@@ -166,8 +166,6 @@ extern void free_ioapic_entries(struct I
extern int save_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
extern void mask_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
extern int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
-extern void reinit_intr_remapped_IO_APIC(int intr_remapping,
- struct IO_APIC_route_entry **ioapic_entries);
extern void probe_nr_irqs_gsi(void);
Index: tip/arch/x86/kernel/apic/apic.c
===================================================================
--- tip.orig/arch/x86/kernel/apic/apic.c
+++ tip/arch/x86/kernel/apic/apic.c
@@ -1416,8 +1416,6 @@ end_restore:
* IR enabling failed
*/
restore_IO_APIC_setup(ioapic_entries);
- else
- reinit_intr_remapped_IO_APIC(x2apic_preenabled, ioapic_entries);
unmask_8259A();
local_irq_restore(flags);
Index: tip/arch/x86/kernel/apic/io_apic.c
===================================================================
--- tip.orig/arch/x86/kernel/apic/io_apic.c
+++ tip/arch/x86/kernel/apic/io_apic.c
@@ -834,20 +834,6 @@ int restore_IO_APIC_setup(struct IO_APIC
return 0;
}
-void reinit_intr_remapped_IO_APIC(int intr_remapping,
- struct IO_APIC_route_entry **ioapic_entries)
-
-{
- /*
- * for now plain restore of previous settings.
- * TBD: In the case of OS enabling interrupt-remapping,
- * IO-APIC RTE's need to be setup to point to interrupt-remapping
- * table entries. for now, do a plain restore, and wait for
- * the setup_IO_APIC_irqs() to do proper initialization.
- */
- restore_IO_APIC_setup(ioapic_entries);
-}
-
void free_ioapic_entries(struct IO_APIC_route_entry **ioapic_entries)
{
int apic;
--
Siddha, Suresh B wrote:
> When interrupt-remapping is enabled, We are relying on
> setup_IO_APIC_irqs()
> to configure remapped entries in the IO-APIC, which comes little bit
> later after enabling interrupt-remapping.
>
> Meanwhile, Restore of old io-apic entries after enabling
> interrupt-remapping will not make the interrupts through io-apic
> functional anyway.
>
> So remove unnecessary reinit_intr_remapped_IO_APIC().
>
> Signed-off-by: Suresh Siddha <[email protected]>
> Cc: Weidong Han <[email protected]>
> ---
>
> Index: tip/arch/x86/include/asm/io_apic.h
> ===================================================================
> --- tip.orig/arch/x86/include/asm/io_apic.h
> +++ tip/arch/x86/include/asm/io_apic.h
> @@ -166,8 +166,6 @@ extern void free_ioapic_entries(struct I
> extern int save_IO_APIC_setup(struct IO_APIC_route_entry
> **ioapic_entries); extern void mask_IO_APIC_setup(struct
> IO_APIC_route_entry **ioapic_entries); extern int
> restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
> -extern void reinit_intr_remapped_IO_APIC(int intr_remapping,
> - struct IO_APIC_route_entry **ioapic_entries);
>
> extern void probe_nr_irqs_gsi(void);
>
> Index: tip/arch/x86/kernel/apic/apic.c
> ===================================================================
> --- tip.orig/arch/x86/kernel/apic/apic.c
> +++ tip/arch/x86/kernel/apic/apic.c
> @@ -1416,8 +1416,6 @@ end_restore:
> * IR enabling failed
> */
> restore_IO_APIC_setup(ioapic_entries);
> - else
> - reinit_intr_remapped_IO_APIC(x2apic_preenabled, ioapic_entries);
Whether IR enabling succeeds or fails, it always needs to restore old IOAPIC entries. Due to removing reinit_intr_remapped_IO_APIC here, it needs to also remove the "if (ret)" before restore_IO_APIC_setup(ioapic_entries);
Regards,
Weidong
* Han, Weidong <[email protected]> wrote:
> Siddha, Suresh B wrote:
> > When interrupt-remapping is enabled, We are relying on
> > setup_IO_APIC_irqs()
> > to configure remapped entries in the IO-APIC, which comes little bit
> > later after enabling interrupt-remapping.
> >
> > Meanwhile, Restore of old io-apic entries after enabling
> > interrupt-remapping will not make the interrupts through io-apic
> > functional anyway.
> >
> > So remove unnecessary reinit_intr_remapped_IO_APIC().
> >
> > Signed-off-by: Suresh Siddha <[email protected]>
> > Cc: Weidong Han <[email protected]>
> > ---
> >
> > Index: tip/arch/x86/include/asm/io_apic.h
> > ===================================================================
> > --- tip.orig/arch/x86/include/asm/io_apic.h
> > +++ tip/arch/x86/include/asm/io_apic.h
> > @@ -166,8 +166,6 @@ extern void free_ioapic_entries(struct I
> > extern int save_IO_APIC_setup(struct IO_APIC_route_entry
> > **ioapic_entries); extern void mask_IO_APIC_setup(struct
> > IO_APIC_route_entry **ioapic_entries); extern int
> > restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
> > -extern void reinit_intr_remapped_IO_APIC(int intr_remapping,
> > - struct IO_APIC_route_entry **ioapic_entries);
> >
> > extern void probe_nr_irqs_gsi(void);
> >
> > Index: tip/arch/x86/kernel/apic/apic.c
> > ===================================================================
> > --- tip.orig/arch/x86/kernel/apic/apic.c
> > +++ tip/arch/x86/kernel/apic/apic.c
> > @@ -1416,8 +1416,6 @@ end_restore:
> > * IR enabling failed
> > */
> > restore_IO_APIC_setup(ioapic_entries);
> > - else
> > - reinit_intr_remapped_IO_APIC(x2apic_preenabled, ioapic_entries);
>
> Whether IR enabling succeeds or fails, it always needs to restore
> old IOAPIC entries. Due to removing reinit_intr_remapped_IO_APIC
> here, it needs to also remove the "if (ret)" before
> restore_IO_APIC_setup(ioapic_entries);
Ok - i skipped this patch for now.
Ingo
On Tue, 2009-04-21 at 00:01 -0700, Ingo Molnar wrote:
> * Han, Weidong <[email protected]> wrote:
>
> > Siddha, Suresh B wrote:
> > > When interrupt-remapping is enabled, We are relying on
> > > setup_IO_APIC_irqs()
> > > to configure remapped entries in the IO-APIC, which comes little bit
> > > later after enabling interrupt-remapping.
> > >
> > > Meanwhile, Restore of old io-apic entries after enabling
> > > interrupt-remapping will not make the interrupts through io-apic
> > > functional anyway.
> > >
> > > So remove unnecessary reinit_intr_remapped_IO_APIC().
> > >
> > > Signed-off-by: Suresh Siddha <[email protected]>
> > > Cc: Weidong Han <[email protected]>
> > > ---
> > >
> > > Index: tip/arch/x86/include/asm/io_apic.h
> > > ===================================================================
> > > --- tip.orig/arch/x86/include/asm/io_apic.h
> > > +++ tip/arch/x86/include/asm/io_apic.h
> > > @@ -166,8 +166,6 @@ extern void free_ioapic_entries(struct I
> > > extern int save_IO_APIC_setup(struct IO_APIC_route_entry
> > > **ioapic_entries); extern void mask_IO_APIC_setup(struct
> > > IO_APIC_route_entry **ioapic_entries); extern int
> > > restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
> > > -extern void reinit_intr_remapped_IO_APIC(int intr_remapping,
> > > - struct IO_APIC_route_entry **ioapic_entries);
> > >
> > > extern void probe_nr_irqs_gsi(void);
> > >
> > > Index: tip/arch/x86/kernel/apic/apic.c
> > > ===================================================================
> > > --- tip.orig/arch/x86/kernel/apic/apic.c
> > > +++ tip/arch/x86/kernel/apic/apic.c
> > > @@ -1416,8 +1416,6 @@ end_restore:
> > > * IR enabling failed
> > > */
> > > restore_IO_APIC_setup(ioapic_entries);
> > > - else
> > > - reinit_intr_remapped_IO_APIC(x2apic_preenabled, ioapic_entries);
> >
> > Whether IR enabling succeeds or fails, it always needs to restore
> > old IOAPIC entries. Due to removing reinit_intr_remapped_IO_APIC
> > here, it needs to also remove the "if (ret)" before
> > restore_IO_APIC_setup(ioapic_entries);
>
> Ok - i skipped this patch for now.
Let me clarify what I am doing in this patch:
When interrupt-remapping is enabled, IO-APIC entries need to be setup in
the re-mappable format (pointing to interrupt-remapping table entries
setup by the OS). This remapping configuration is happening in the same
place where we traditionally configure IO-APIC (i.e., in
setup_IO_APIC_irqs()).
So when we enable interrupt-remapping successfully, there is no need to
restore old io-apic RTE entries before we actually do a complete
configuration shortly in setup_IO_APIC_irqs(). Old IO-APIC RTE's may be
in traditional format (non re-mappable) or in re-mappable format
pointing to interrupt-remapping table entries setup by BIOS. Restoring
both of these will not make IO-APIC functional. We have to rely on
setup_IO_APIC_irqs() for proper configuration by OS.
So I am removing this unnecessary and broken step.
When enabling interrupt-remapping is not successful, we are doing plain
restore of old RTE's (which will still work as we went back to original
no-remapping state). Complete IO-APIC configuration will be done shortly
in setup_IO_APIC_irqs().
thanks,
suresh
Siddha, Suresh B wrote:
> On Tue, 2009-04-21 at 00:01 -0700, Ingo Molnar wrote:
>> * Han, Weidong <[email protected]> wrote:
>>
>>> Siddha, Suresh B wrote:
>>>> When interrupt-remapping is enabled, We are relying on
>>>> setup_IO_APIC_irqs() to configure remapped entries in the IO-APIC,
>>>> which comes little bit later after enabling interrupt-remapping.
>>>>
>>>> Meanwhile, Restore of old io-apic entries after enabling
>>>> interrupt-remapping will not make the interrupts through io-apic
>>>> functional anyway.
>>>>
>>>> So remove unnecessary reinit_intr_remapped_IO_APIC().
>>>>
>>>> Signed-off-by: Suresh Siddha <[email protected]>
>>>> Cc: Weidong Han <[email protected]>
>>>> ---
>>>>
>>>> Index: tip/arch/x86/include/asm/io_apic.h
>>>> ===================================================================
>>>> --- tip.orig/arch/x86/include/asm/io_apic.h
>>>> +++ tip/arch/x86/include/asm/io_apic.h
>>>> @@ -166,8 +166,6 @@ extern void free_ioapic_entries(struct I
>>>> extern int save_IO_APIC_setup(struct IO_APIC_route_entry
>>>> **ioapic_entries); extern void mask_IO_APIC_setup(struct
>>>> IO_APIC_route_entry **ioapic_entries); extern int
>>>> restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
>>>> -extern void reinit_intr_remapped_IO_APIC(int intr_remapping,
>>>> - struct IO_APIC_route_entry **ioapic_entries);
>>>>
>>>> extern void probe_nr_irqs_gsi(void);
>>>>
>>>> Index: tip/arch/x86/kernel/apic/apic.c
>>>> ===================================================================
>>>> --- tip.orig/arch/x86/kernel/apic/apic.c
>>>> +++ tip/arch/x86/kernel/apic/apic.c
>>>> @@ -1416,8 +1416,6 @@ end_restore:
>>>> * IR enabling failed
>>>> */
>>>> restore_IO_APIC_setup(ioapic_entries);
>>>> - else
>>>> - reinit_intr_remapped_IO_APIC(x2apic_preenabled, ioapic_entries);
>>>
>>> Whether IR enabling succeeds or fails, it always needs to restore
>>> old IOAPIC entries. Due to removing reinit_intr_remapped_IO_APIC
>>> here, it needs to also remove the "if (ret)" before
>>> restore_IO_APIC_setup(ioapic_entries);
>>
>> Ok - i skipped this patch for now.
>
> Let me clarify what I am doing in this patch:
>
> When interrupt-remapping is enabled, IO-APIC entries need to be setup
> in the re-mappable format (pointing to interrupt-remapping table
> entries setup by the OS). This remapping configuration is happening
> in the same place where we traditionally configure IO-APIC (i.e., in
> setup_IO_APIC_irqs()).
>
> So when we enable interrupt-remapping successfully, there is no need
> to restore old io-apic RTE entries before we actually do a complete
> configuration shortly in setup_IO_APIC_irqs(). Old IO-APIC RTE's may
> be in traditional format (non re-mappable) or in re-mappable format
> pointing to interrupt-remapping table entries setup by BIOS. Restoring
> both of these will not make IO-APIC functional. We have to rely on
> setup_IO_APIC_irqs() for proper configuration by OS.
>
> So I am removing this unnecessary and broken step.
>
> When enabling interrupt-remapping is not successful, we are doing
> plain restore of old RTE's (which will still work as we went back to
> original no-remapping state). Complete IO-APIC configuration will be
> done shortly in setup_IO_APIC_irqs().
>
Clear explanation. I misunderstood it. This patch is fine.
Regards,
Weidong
> thanks,
> suresh
* Han, Weidong <[email protected]> wrote:
> Siddha, Suresh B wrote:
> > On Tue, 2009-04-21 at 00:01 -0700, Ingo Molnar wrote:
> >> * Han, Weidong <[email protected]> wrote:
> >>
> >>> Siddha, Suresh B wrote:
> >>>> When interrupt-remapping is enabled, We are relying on
> >>>> setup_IO_APIC_irqs() to configure remapped entries in the IO-APIC,
> >>>> which comes little bit later after enabling interrupt-remapping.
> >>>>
> >>>> Meanwhile, Restore of old io-apic entries after enabling
> >>>> interrupt-remapping will not make the interrupts through io-apic
> >>>> functional anyway.
> >>>>
> >>>> So remove unnecessary reinit_intr_remapped_IO_APIC().
> >>>>
> >>>> Signed-off-by: Suresh Siddha <[email protected]>
> >>>> Cc: Weidong Han <[email protected]>
> >>>> ---
> >>>>
> >>>> Index: tip/arch/x86/include/asm/io_apic.h
> >>>> ===================================================================
> >>>> --- tip.orig/arch/x86/include/asm/io_apic.h
> >>>> +++ tip/arch/x86/include/asm/io_apic.h
> >>>> @@ -166,8 +166,6 @@ extern void free_ioapic_entries(struct I
> >>>> extern int save_IO_APIC_setup(struct IO_APIC_route_entry
> >>>> **ioapic_entries); extern void mask_IO_APIC_setup(struct
> >>>> IO_APIC_route_entry **ioapic_entries); extern int
> >>>> restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
> >>>> -extern void reinit_intr_remapped_IO_APIC(int intr_remapping,
> >>>> - struct IO_APIC_route_entry **ioapic_entries);
> >>>>
> >>>> extern void probe_nr_irqs_gsi(void);
> >>>>
> >>>> Index: tip/arch/x86/kernel/apic/apic.c
> >>>> ===================================================================
> >>>> --- tip.orig/arch/x86/kernel/apic/apic.c
> >>>> +++ tip/arch/x86/kernel/apic/apic.c
> >>>> @@ -1416,8 +1416,6 @@ end_restore:
> >>>> * IR enabling failed
> >>>> */
> >>>> restore_IO_APIC_setup(ioapic_entries);
> >>>> - else
> >>>> - reinit_intr_remapped_IO_APIC(x2apic_preenabled, ioapic_entries);
> >>>
> >>> Whether IR enabling succeeds or fails, it always needs to restore
> >>> old IOAPIC entries. Due to removing reinit_intr_remapped_IO_APIC
> >>> here, it needs to also remove the "if (ret)" before
> >>> restore_IO_APIC_setup(ioapic_entries);
> >>
> >> Ok - i skipped this patch for now.
> >
> > Let me clarify what I am doing in this patch:
> >
> > When interrupt-remapping is enabled, IO-APIC entries need to be setup
> > in the re-mappable format (pointing to interrupt-remapping table
> > entries setup by the OS). This remapping configuration is happening
> > in the same place where we traditionally configure IO-APIC (i.e., in
> > setup_IO_APIC_irqs()).
> >
> > So when we enable interrupt-remapping successfully, there is no need
> > to restore old io-apic RTE entries before we actually do a complete
> > configuration shortly in setup_IO_APIC_irqs(). Old IO-APIC RTE's may
> > be in traditional format (non re-mappable) or in re-mappable format
> > pointing to interrupt-remapping table entries setup by BIOS. Restoring
> > both of these will not make IO-APIC functional. We have to rely on
> > setup_IO_APIC_irqs() for proper configuration by OS.
> >
> > So I am removing this unnecessary and broken step.
> >
> > When enabling interrupt-remapping is not successful, we are doing
> > plain restore of old RTE's (which will still work as we went back to
> > original no-remapping state). Complete IO-APIC configuration will be
> > done shortly in setup_IO_APIC_irqs().
> >
>
> Clear explanation. I misunderstood it. This patch is fine.
Applied to tip:x86/apic - thanks guys,
Ingo
Commit-ID: ff166cb57a17124af75714a9c11f448f56f1a4a3
Gitweb: http://git.kernel.org/tip/ff166cb57a17124af75714a9c11f448f56f1a4a3
Author: Suresh Siddha <[email protected]>
AuthorDate: Mon, 20 Apr 2009 13:02:30 -0700
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 22 Apr 2009 12:03:04 +0200
x86: x2apic, IR: remove reinit_intr_remapped_IO_APIC()
When interrupt-remapping is enabled, we are relying on
setup_IO_APIC_irqs() to configure remapped entries in the
IO-APIC, which comes little bit later after enabling
interrupt-remapping.
Meanwhile, restoration of old io-apic entries after enabling
interrupt-remapping will not make the interrupts through
io-apic functional anyway.
So remove the unnecessary reinit_intr_remapped_IO_APIC() step.
The longer story:
When interrupt-remapping is enabled, IO-APIC entries need to be
setup in the re-mappable format (pointing to
interrupt-remapping table entries setup by the OS). This
remapping configuration is happening in the same place where we
traditionally configure IO-APIC (i.e., in
setup_IO_APIC_irqs()).
So when we enable interrupt-remapping successfully, there is no
need to restore old io-apic RTE entries before we actually do a
complete configuration shortly in setup_IO_APIC_irqs(). Old
IO-APIC RTE's may be in traditional format (non re-mappable) or
in re-mappable format pointing to interrupt-remapping table
entries setup by BIOS. Restoring both of these will not make
IO-APIC functional. We have to rely on setup_IO_APIC_irqs() for
proper configuration by OS.
So I am removing this unnecessary and broken step.
[ Impact: remove unnecessary/broken IO-APIC setup step ]
Signed-off-by: Suresh Siddha <[email protected]>
Acked-by: Weidong Han <[email protected]>
Cc: [email protected]
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/include/asm/io_apic.h | 2 --
arch/x86/kernel/apic/apic.c | 2 --
arch/x86/kernel/apic/io_apic.c | 14 --------------
3 files changed, 0 insertions(+), 18 deletions(-)
diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 34eaa37..1cf1450 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -166,8 +166,6 @@ extern void free_ioapic_entries(struct IO_APIC_route_entry **ioapic_entries);
extern int save_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
extern void mask_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
extern int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
-extern void reinit_intr_remapped_IO_APIC(int intr_remapping,
- struct IO_APIC_route_entry **ioapic_entries);
extern void probe_nr_irqs_gsi(void);
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index d32f558..1386dbe 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1412,8 +1412,6 @@ end_restore:
* IR enabling failed
*/
restore_IO_APIC_setup(ioapic_entries);
- else
- reinit_intr_remapped_IO_APIC(x2apic_preenabled, ioapic_entries);
unmask_8259A();
local_irq_restore(flags);
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 4baa9cb..8aef5f9 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -833,20 +833,6 @@ int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries)
return 0;
}
-void reinit_intr_remapped_IO_APIC(int intr_remapping,
- struct IO_APIC_route_entry **ioapic_entries)
-
-{
- /*
- * for now plain restore of previous settings.
- * TBD: In the case of OS enabling interrupt-remapping,
- * IO-APIC RTE's need to be setup to point to interrupt-remapping
- * table entries. for now, do a plain restore, and wait for
- * the setup_IO_APIC_irqs() to do proper initialization.
- */
- restore_IO_APIC_setup(ioapic_entries);
-}
-
void free_ioapic_entries(struct IO_APIC_route_entry **ioapic_entries)
{
int apic;