2011-03-31 08:02:39

by Florian Mickler

[permalink] [raw]
Subject: [PATCH][POKE] Skip looking for ioapic overrides when ioapics are not present

From: Eric W. Biederman <[email protected]>

Avinash Kurup <[email protected]> writes:

> Hi Eric,
>     I get the following errors while booting into 2.6.35-rc1. I did not
> get these in 2.6.34 . The computer however boots and works fine, So its not
> serious but the following errors are displayed in dmesg.
>
> [    0.089969] ERROR: Unable to locate IOAPIC for GSI 13
> [    0.090556] ERROR: Unable to locate IOAPIC for GSI 8
> [    0.091104] ERROR: Unable to locate IOAPIC for GSI 12
> [    0.091375] ERROR: Unable to locate IOAPIC for GSI 1
> [    0.093195] ERROR: Unable to locate IOAPIC for GSI 4
> [    0.094342] ERROR: Unable to locate IOAPIC for GSI 10
> [    0.096335] ERROR: Unable to locate IOAPIC for GSI 6

The new warning originates from acpi_get_override_irq, which I changed to
use helper functions that warn when they fail.

When IOAPICs and ACPI are enabled in a kernel and run on ACPI hardware
that doesn't use the ioapics the pnp acpi code calls this function,
looking for ACPI irq overrides. ACPI irq overrides exist only in the
ioapic case so this function will never succeed. So make the function
fail fast so we don't call into help functions that legitimately
complain when they fail.

[I submit this as the corresponding bug report is still not closed, the patch
not merged, distributions applying this patch, it is tested to work and I found
nowhere any resoning as to why this should be out of tree. I just guessed on
the stable tag and number. -Florian]

Reference: https://bugzilla.kernel.org/show_bug.cgi?id=17772
Tested-by: Avinash Kurup <[email protected]>
Tested-by: [email protected]
Signed-off-by: "Eric W. Biederman" <[email protected]>
Signed-off-by: Florian Mickler <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected] [.34+]

---

arch/x86/kernel/apic/io_apic.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 68df09b..3940103 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3789,6 +3789,9 @@ int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
{
int ioapic, pin, idx;

+ if (acpi_irq_model != ACPI_IRQ_MODEL_IOAPIC)
+ return -1;
+
if (skip_ioapic_setup)
return -1;

--
1.7.4.1


2011-03-31 08:44:23

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH][POKE] Skip looking for ioapic overrides when ioapics are not present


* Florian Mickler <[email protected]> wrote:

> From: Eric W. Biederman <[email protected]>
>
> Avinash Kurup <[email protected]> writes:
>
> > Hi Eric,
> >     I get the following errors while booting into 2.6.35-rc1. I did not
> > get these in 2.6.34 . The computer however boots and works fine, So its not
> > serious but the following errors are displayed in dmesg.
> >
> > [    0.089969] ERROR: Unable to locate IOAPIC for GSI 13
> > [    0.090556] ERROR: Unable to locate IOAPIC for GSI 8
> > [    0.091104] ERROR: Unable to locate IOAPIC for GSI 12
> > [    0.091375] ERROR: Unable to locate IOAPIC for GSI 1
> > [    0.093195] ERROR: Unable to locate IOAPIC for GSI 4
> > [    0.094342] ERROR: Unable to locate IOAPIC for GSI 10
> > [    0.096335] ERROR: Unable to locate IOAPIC for GSI 6
>
> The new warning originates from acpi_get_override_irq, which I changed to
> use helper functions that warn when they fail.
>
> When IOAPICs and ACPI are enabled in a kernel and run on ACPI hardware
> that doesn't use the ioapics the pnp acpi code calls this function,
> looking for ACPI irq overrides. ACPI irq overrides exist only in the
> ioapic case so this function will never succeed. So make the function
> fail fast so we don't call into help functions that legitimately
> complain when they fail.
>
> [I submit this as the corresponding bug report is still not closed, the patch
> not merged, distributions applying this patch, it is tested to work and I found
> nowhere any resoning as to why this should be out of tree. I just guessed on
> the stable tag and number. -Florian]

The patch does not build when CONFIG_ACPI is disabled:

arch/x86/kernel/apic/io_apic.c: In function ‘acpi_get_override_irq’:
arch/x86/kernel/apic/io_apic.c:3792:6: error: ‘acpi_irq_model’ undeclared (first use in this function)
arch/x86/kernel/apic/io_apic.c:3792:24: error: ‘ACPI_IRQ_MODEL_IOAPIC’ undeclared (first use in this function)

When you or Eric resubmits the fixed patch please use the improved changelog
below.

Thanks,

Ingo

---------------->
>From 19c4d532952f68dbbcee6f46f22d351496b468a7 Mon Sep 17 00:00:00 2001
From: Eric W. Biederman <[email protected]>
Date: Thu, 31 Mar 2011 10:01:29 +0200
Subject: [PATCH] x86, ioapic: Skip looking for ioapic overrides when ioapics are not present
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Avinash Kurup reported:

|
|     I get the following errors while booting into 2.6.35-rc1. I did not
| get these in 2.6.34. The computer however boots and works fine, so its not
| serious but the following errors are displayed in dmesg:
|
| [    0.089969] ERROR: Unable to locate IOAPIC for GSI 13
| [    0.090556] ERROR: Unable to locate IOAPIC for GSI 8
| [    0.091104] ERROR: Unable to locate IOAPIC for GSI 12
| [    0.091375] ERROR: Unable to locate IOAPIC for GSI 1
| [    0.093195] ERROR: Unable to locate IOAPIC for GSI 4
| [    0.094342] ERROR: Unable to locate IOAPIC for GSI 10
| [    0.096335] ERROR: Unable to locate IOAPIC for GSI 6
|

The new warning originates from acpi_get_override_irq(), which I
recently changed to use helper functions:

9a0a91bb56d2: x86, acpi/irq: Teach acpi_get_override_irq to take a gsi not an isa_irq

These helper functions have the side-effect that they warn when
they fail harmlessly.

When IOAPICs and ACPI are enabled in a kernel and run on ACPI
hardware that doesn't use the ioapics the pnp acpi code calls
this function, looking for ACPI irq overrides. ACPI irq
overrides exist only in the ioapic case so this function will
never succeed.

So make the function fail quickly and silently so we don't call
into help functions that legitimately complain when they fail.

Reference: https://bugzilla.kernel.org/show_bug.cgi?id=17772
Reported-and-Tested-by: Avinash Kurup <[email protected]>
Tested-by: [email protected]
Signed-off-by: "Eric W. Biederman" <[email protected]>
Signed-off-by: Florian Mickler <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: <[email protected]> # .34+
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/apic/io_apic.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 68df09b..3940103 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3789,6 +3789,9 @@ int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
{
int ioapic, pin, idx;

+ if (acpi_irq_model != ACPI_IRQ_MODEL_IOAPIC)
+ return -1;
+
if (skip_ioapic_setup)
return -1;

2011-03-31 08:49:49

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH][POKE] Skip looking for ioapic overrides when ioapics are not present

(Something, somewhere, chocked on that "[.34+]@schatten.dmk.lab" address
that was in the CC list, so I removed it. Invalid address? Invalid TLD?)

On Thu, 2011-03-31 at 10:01 +0200, Florian Mickler wrote:
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 68df09b..3940103 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -3789,6 +3789,9 @@ int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
> {
> int ioapic, pin, idx;
>
> + if (acpi_irq_model != ACPI_IRQ_MODEL_IOAPIC)
> + return -1;
> +
> if (skip_ioapic_setup)
> return -1;
>

Seems to have the same goal as commit
678301ecadec24ff77ab310eebf8a32ccddb1850 ("x86, ioapic: Don't warn about
non-existing IOAPICs if we have none"), which got merged in the v2.6.38
cycle (authored by me, signed off by Ingo Molnar). Maybe Eric's patch is
more correct. I can't say as I was happy with the effect of my patch
(ie, make an uninteresting error disappear) and didn't investigate any
further. I have also no desire to dive into this matter again.


Paul

2011-03-31 10:53:33

by Florian Mickler

[permalink] [raw]
Subject: Re: [PATCH][POKE] Skip looking for ioapic overrides when ioapics are not present

On Thu, 31 Mar 2011 10:48:43 +0200
Paul Bolle <[email protected]> wrote:

> On Thu, 2011-03-31 at 10:01 +0200, Florian Mickler wrote:
> > diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> > index 68df09b..3940103 100644
> > --- a/arch/x86/kernel/apic/io_apic.c
> > +++ b/arch/x86/kernel/apic/io_apic.c
> > @@ -3789,6 +3789,9 @@ int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
> > {
> > int ioapic, pin, idx;
> >
> > + if (acpi_irq_model != ACPI_IRQ_MODEL_IOAPIC)
> > + return -1;
> > +
> > if (skip_ioapic_setup)
> > return -1;
> >
>
> Seems to have the same goal as commit
> 678301ecadec24ff77ab310eebf8a32ccddb1850 ("x86, ioapic: Don't warn about
> non-existing IOAPICs if we have none"), which got merged in the v2.6.38
> cycle (authored by me, signed off by Ingo Molnar). Maybe Eric's patch is
> more correct. I can't say as I was happy with the effect of my patch
> (ie, make an uninteresting error disappear) and didn't investigate any
> further. I have also no desire to dive into this matter again.
>
>
> Paul
>

Thanks for letting me know. Sedat, did you actually test with 2.6.38?

Regards,
Flo

2011-03-31 11:00:11

by Florian Mickler

[permalink] [raw]
Subject: Re: [PATCH][POKE] Skip looking for ioapic overrides when ioapics are not present

On Thu, 31 Mar 2011 10:43:42 +0200
Ingo Molnar <[email protected]> wrote:

>
> * Florian Mickler <[email protected]> wrote:
>
> > From: Eric W. Biederman <[email protected]>
> >
> > [I submit this as the corresponding bug report is still not closed, the patch
> > not merged, distributions applying this patch, it is tested to work and I found
> > nowhere any resoning as to why this should be out of tree. I just guessed on
> > the stable tag and number. -Florian]
>
> The patch does not build when CONFIG_ACPI is disabled:
>
> arch/x86/kernel/apic/io_apic.c: In function ‘acpi_get_override_irq’:
> arch/x86/kernel/apic/io_apic.c:3792:6: error: ‘acpi_irq_model’ undeclared (first use in this function)
> arch/x86/kernel/apic/io_apic.c:3792:24: error: ‘ACPI_IRQ_MODEL_IOAPIC’ undeclared (first use in this function)
>
> When you or Eric resubmits the fixed patch please use the improved changelog
> below.
>
> Thanks,
>
> Ingo

Thank you. Will do if it turns out this is still necessary.
Paul Bolle mentioned in another messgage that there is a commit in
2.6.38 to the same effect:

commit 678301ecadec24ff77ab310eebf8a32ccddb1850
("x86, ioapic: Don't warn about non-existing IOAPICs if we have none")

Regards,
Flo

2011-03-31 11:01:20

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH][POKE] Skip looking for ioapic overrides when ioapics are not present

On Thu, Mar 31, 2011 at 12:53 PM, Florian Mickler <[email protected]> wrote:
> On Thu, 31 Mar 2011 10:48:43 +0200
> Paul Bolle <[email protected]> wrote:
>
>> On Thu, 2011-03-31 at 10:01 +0200, Florian Mickler wrote:
>> > diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
>> > index 68df09b..3940103 100644
>> > --- a/arch/x86/kernel/apic/io_apic.c
>> > +++ b/arch/x86/kernel/apic/io_apic.c
>> > @@ -3789,6 +3789,9 @@ int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
>> >  {
>> >     int ioapic, pin, idx;
>> >
>> > +   if (acpi_irq_model != ACPI_IRQ_MODEL_IOAPIC)
>> > +           return -1;
>> > +
>> >     if (skip_ioapic_setup)
>> >             return -1;
>> >
>>
>> Seems to have the same goal as commit
>> 678301ecadec24ff77ab310eebf8a32ccddb1850 ("x86, ioapic: Don't warn about
>> non-existing IOAPICs if we have none"), which got merged in the v2.6.38
>> cycle (authored by me, signed off by Ingo Molnar). Maybe Eric's patch is
>> more correct. I can't say as I was happy with the effect of my patch
>> (ie, make an uninteresting error disappear) and didn't investigate any
>> further. I have also no desire to dive into this matter again.
>>
>>
>> Paul
>>
>
> Thanks for letting me know. Sedat, did you actually test with 2.6.38?
>
> Regards,
> Flo
>

I have and had this patch in my own patch-series *before* Debian
included it (IIRC right after Eric committed it to LKML).
To answer your question: Yes.
I have the patch also in my current linux-next kernels (next-20110331).

- Sedat -

2011-03-31 12:05:44

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH][POKE] Skip looking for ioapic overrides when ioapics are not present

Sedat Dilek <[email protected]> writes:

> On Thu, Mar 31, 2011 at 12:53 PM, Florian Mickler <[email protected]> wrote:
>> On Thu, 31 Mar 2011 10:48:43 +0200
>> Paul Bolle <[email protected]> wrote:
>>
>>> On Thu, 2011-03-31 at 10:01 +0200, Florian Mickler wrote:
>>> > diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
>>> > index 68df09b..3940103 100644
>>> > --- a/arch/x86/kernel/apic/io_apic.c
>>> > +++ b/arch/x86/kernel/apic/io_apic.c
>>> > @@ -3789,6 +3789,9 @@ int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
>>> >  {
>>> >     int ioapic, pin, idx;
>>> >
>>> > +   if (acpi_irq_model != ACPI_IRQ_MODEL_IOAPIC)
>>> > +           return -1;
>>> > +
>>> >     if (skip_ioapic_setup)
>>> >             return -1;
>>> >
>>>
>>> Seems to have the same goal as commit
>>> 678301ecadec24ff77ab310eebf8a32ccddb1850 ("x86, ioapic: Don't warn about
>>> non-existing IOAPICs if we have none"), which got merged in the v2.6.38
>>> cycle (authored by me, signed off by Ingo Molnar). Maybe Eric's patch is
>>> more correct. I can't say as I was happy with the effect of my patch
>>> (ie, make an uninteresting error disappear) and didn't investigate any
>>> further. I have also no desire to dive into this matter again.

Yes. My patch is more correct. We really do want the warning if we have
0 ioapics and we expect to be using ioapics. It doesn't make sense to
suppress the warning unless we aren't in ioapic mode.

I don't have a clue why my patch got lost, but can we please get it
applied?

>> Thanks for letting me know. Sedat, did you actually test with 2.6.38?
>>
>> Regards,
>> Flo

> I have and had this patch in my own patch-series *before* Debian
> included it (IIRC right after Eric committed it to LKML).
> To answer your question: Yes.
> I have the patch also in my current linux-next kernels
> (next-20110331).

Eric

2011-03-31 12:44:12

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH][POKE] Skip looking for ioapic overrides when ioapics are not present


* Eric W. Biederman <[email protected]> wrote:

> I don't have a clue why my patch got lost, [...]

It was not lost, it broke the build. You were notified but never followed up
with a working patch:

https://lkml.org/lkml/2010/10/2/91

and a different patch was applied to get rid of the warning messages. We can
apply your patch (and that patch can undo the other fix), but it has to build,
obviously.

Thanks,

Ingo

2011-03-31 13:16:57

by Florian Mickler

[permalink] [raw]
Subject: Re: [PATCH][POKE] Skip looking for ioapic overrides when ioapics are not present

On Thu, 31 Mar 2011 13:01:18 +0200
Sedat Dilek <[email protected]> wrote:

> >
> > Thanks for letting me know. Sedat, did you actually test with 2.6.38?
> >
> > Regards,
> > Flo
>
> I have and had this patch in my own patch-series *before* Debian
> included it (IIRC right after Eric committed it to LKML).
> To answer your question: Yes.
> I have the patch also in my current linux-next kernels (next-20110331).
>
> - Sedat -

Sorry, I meant tested _without_ the patch. (i.e. does Paul's patch work
or not for you...) And double sorry, if you did already test
vanilla 2.6.38. Tripple sorry if you already wrote that you did test
_without_ the patch and it is still needed... I guess in that case I
should just ditch my copy of 'speed-reading for dummies' ... :) )

But anyhow, Eric already stepped up and said that the current state is
supoptimal, so it doesn't matter if vanilla 2.6.38 (without Eric's
patch) fixes it for you since it looks like Eric's patch should go in
nevertheless.

Thanks,
Flo

2011-03-31 13:25:28

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH][POKE] Skip looking for ioapic overrides when ioapics are not present

On Thu, Mar 31, 2011 at 3:16 PM, Florian Mickler <[email protected]> wrote:
> On Thu, 31 Mar 2011 13:01:18 +0200
> Sedat Dilek <[email protected]> wrote:
>
>> >
>> > Thanks for letting me know. Sedat, did you actually test with 2.6.38?
>> >
>> > Regards,
>> > Flo
>>
>> I have and had this patch in my own patch-series *before* Debian
>> included it (IIRC right after Eric committed it to LKML).
>> To answer your question: Yes.
>> I have the patch also in my current linux-next kernels (next-20110331).
>>
>> - Sedat -
>
> Sorry, I meant tested _without_ the patch. (i.e. does Paul's patch work
> or not for you...) And double sorry, if you did already test
> vanilla 2.6.38. Tripple sorry if you already wrote that you did test
> _without_ the patch and it is still needed... I guess in that case I
> should just ditch my copy of 'speed-reading for dummies' ... :) )
>

No worries, no sorries needed for this (I know you fight to get things done).
I can't really say what is w/o Eric's patch.
As I replied to the BR #17772:
I added "lapic" boot-parameter as a result of a perf/NMI patch.
With both things "WorksForMe" I headed to other stuff in kernel development :-).
I was not aware that there was further discussion on the initial patch.

> But anyhow, Eric already stepped up and said that the current state is
> supoptimal, so it doesn't matter if vanilla 2.6.38 (without Eric's
> patch) fixes it for you since it looks like Eric's patch should go in
> nevertheless.
>

I am as always open for testing if there is a new patch around, just
let me know.

- Sedat -

2011-04-01 01:25:00

by Florian Mickler

[permalink] [raw]
Subject: [PATCH v2] x86, ioapic: Skip looking for ioapic overrides when ioapics are not present

From: Eric W. Biederman <[email protected]>

Avinash Kurup reported:

|
|     I get the following errors while booting into 2.6.35-rc1. I did not
| get these in 2.6.34. The computer however boots and works fine, so its not
| serious but the following errors are displayed in dmesg:
|
| [    0.089969] ERROR: Unable to locate IOAPIC for GSI 13
| [    0.090556] ERROR: Unable to locate IOAPIC for GSI 8
| [    0.091104] ERROR: Unable to locate IOAPIC for GSI 12
| [    0.091375] ERROR: Unable to locate IOAPIC for GSI 1
| [    0.093195] ERROR: Unable to locate IOAPIC for GSI 4
| [    0.094342] ERROR: Unable to locate IOAPIC for GSI 10
| [    0.096335] ERROR: Unable to locate IOAPIC for GSI 6
|

The new warning originates from acpi_get_override_irq(), which I
recently changed to use helper functions:

9a0a91bb56d2: x86, acpi/irq: Teach acpi_get_override_irq to take a gsi not an isa_irq

These helper functions have the side-effect that they warn when
they fail harmlessly.

When IOAPICs and ACPI are enabled in a kernel and run on ACPI
hardware that doesn't use the ioapics the pnp acpi code calls
this function, looking for ACPI irq overrides. ACPI irq
overrides exist only in the ioapic case so this function will
never succeed.

So make the function fail quickly and silently so we don't call
into help functions that legitimately complain when they fail.

Reference: https://bugzilla.kernel.org/show_bug.cgi?id=17772
Reported-and-Tested-by: Avinash Kurup <[email protected]>
Tested-by: [email protected]
Signed-off-by: "Eric W. Biederman" <[email protected]>
Signed-off-by: Florian Mickler <[email protected]>
[ #ifdef's added, revert of 678301ecadec squashed -Florian]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: <[email protected]> # .34+
LKML-Reference: <[email protected]>
---
Hi Ingo,

here is the revised patch. I squashed the revert of 678301ecadec into this
and put ifdefs around it to avoid breaking the build...

Regards,
Flo

arch/x86/kernel/apic/io_apic.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 68df09b..edf86ca 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3789,6 +3789,10 @@ int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
{
int ioapic, pin, idx;

+#ifdef CONFIG_ACPI
+ if (acpi_irq_model != ACPI_IRQ_MODEL_IOAPIC)
+ return -1;
+#endif
if (skip_ioapic_setup)
return -1;

@@ -3951,9 +3955,6 @@ int mp_find_ioapic(u32 gsi)
{
int i = 0;

- if (nr_ioapics == 0)
- return -1;
-
/* Find the IOAPIC that manages this GSI. */
for (i = 0; i < nr_ioapics; i++) {
if ((gsi >= mp_gsi_routing[i].gsi_base)
--
1.7.4.1

2011-04-01 06:20:16

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2] x86, ioapic: Skip looking for ioapic overrides when ioapics are not present


* Florian Mickler <[email protected]> wrote:

> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -3789,6 +3789,10 @@ int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
> {
> int ioapic, pin, idx;
>
> +#ifdef CONFIG_ACPI
> + if (acpi_irq_model != ACPI_IRQ_MODEL_IOAPIC)
> + return -1;
> +#endif

That #ifdef is very ugly. Please introduce a suitable helper function in
arch/x86/include/asm/acpi.h - acpi_irq_ioapic_model() or so, which could be
used like this:

if (!acpi_irq_ioapic_model())
return -1;

And would be defined in the !CONFIG_ACPI case as well.

Thanks,

Ingo

2011-04-01 06:43:35

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH v2] x86, ioapic: Skip looking for ioapic overrides when ioapics are not present

Ingo Molnar <[email protected]> writes:

> * Florian Mickler <[email protected]> wrote:
>
>> --- a/arch/x86/kernel/apic/io_apic.c
>> +++ b/arch/x86/kernel/apic/io_apic.c
>> @@ -3789,6 +3789,10 @@ int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
>> {
>> int ioapic, pin, idx;
>>
>> +#ifdef CONFIG_ACPI
>> + if (acpi_irq_model != ACPI_IRQ_MODEL_IOAPIC)
>> + return -1;
>> +#endif
>
> That #ifdef is very ugly. Please introduce a suitable helper function in
> arch/x86/include/asm/acpi.h - acpi_irq_ioapic_model() or so, which could be
> used like this:
>
> if (!acpi_irq_ioapic_model())
> return -1;
>
> And would be defined in the !CONFIG_ACPI case as well.

If you want clean this function needs to be moved out of io_apic.c where
into acpi.c where it belongs. I don't know that going that far to fix
an annoying warning message is warranted. The function should only have
callers when acpi is compiled so it is safe to make the entire function
#ifdef ACPI.

A silly helper in acpi.h is definitely the wrong way to go.

Eric

2011-04-01 06:47:40

by Len Brown

[permalink] [raw]
Subject: Re: [PATCH v2] x86, ioapic: Skip looking for ioapic overrides when ioapics are not present


> > --- a/arch/x86/kernel/apic/io_apic.c
> > +++ b/arch/x86/kernel/apic/io_apic.c
> > @@ -3789,6 +3789,10 @@ int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
> > {
> > int ioapic, pin, idx;
> >
> > +#ifdef CONFIG_ACPI
> > + if (acpi_irq_model != ACPI_IRQ_MODEL_IOAPIC)
> > + return -1;
> > +#endif
>
> That #ifdef is very ugly. Please introduce a suitable helper function in
> arch/x86/include/asm/acpi.h - acpi_irq_ioapic_model() or so, which could be
> used like this:
>
> if (!acpi_irq_ioapic_model())
> return -1;
>
> And would be defined in the !CONFIG_ACPI case as well.

It would be better to compile _none_ of acpi_get_override_irq() for !ACPI.

-Len

2011-04-01 07:50:24

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2] x86, ioapic: Skip looking for ioapic overrides when ioapics are not present


* Len Brown <[email protected]> wrote:

>
> > > --- a/arch/x86/kernel/apic/io_apic.c
> > > +++ b/arch/x86/kernel/apic/io_apic.c
> > > @@ -3789,6 +3789,10 @@ int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
> > > {
> > > int ioapic, pin, idx;
> > >
> > > +#ifdef CONFIG_ACPI
> > > + if (acpi_irq_model != ACPI_IRQ_MODEL_IOAPIC)
> > > + return -1;
> > > +#endif
> >
> > That #ifdef is very ugly. Please introduce a suitable helper function in
> > arch/x86/include/asm/acpi.h - acpi_irq_ioapic_model() or so, which could be
> > used like this:
> >
> > if (!acpi_irq_ioapic_model())
> > return -1;
> >
> > And would be defined in the !CONFIG_ACPI case as well.
>
> It would be better to compile _none_ of acpi_get_override_irq() for !ACPI.

Yeah, agreed - so just move the function into acpi.c and provide an inline stub
in acpi.h for the !ACPI case.

Note: find_irq_entry() would have to be exported from io_apic.c in this case,
renamed to ioapic_find_irq_entry() or so - and a stub provided for the !IO_APIC
case.

Thanks,

Ingo

2011-04-01 15:45:40

by Florian Mickler

[permalink] [raw]
Subject: [PATCH] x86, ioapic: move acpi_get_override_irq to acpi.c

In order to get rid of the ugly CONFIG_ACPI ifdef, we move
the function acpi_get_override_irq into acpi.c and add a new
helper function ioapic_get_irq to io_apic.c.

Signed-off-by: Florian Mickler <[email protected]>
Cc: Len Brown <[email protected]>
Cc: "Eric W. Biederman" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
LKML-Reference: <[email protected]>
---

Hi Ingo,

Instead of exporting find_irq_entry, irq_trigger and irq_polarity, I added a
new helper function 'ioapic_get_irq(..)' and exported that.

Also I did it as a seperate commit, since it is more of a cleanup and the other
commit is already tested... hope this is ok.

Regards,
Flo

arch/x86/include/asm/io_apic.h | 8 +++++++-
arch/x86/kernel/apic/io_apic.c | 40 ++++++++++++----------------------------
arch/x86/pci/acpi.c | 24 ++++++++++++++++++++++++
include/linux/acpi.h | 6 ++++++
4 files changed, 49 insertions(+), 29 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index c4bd267..89d3fc6 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -177,6 +177,7 @@ extern void __init pre_init_apic_IRQ0(void);
extern void mp_save_irq(struct mpc_intsrc *m);

extern void disable_ioapic_support(void);
+extern int ioapic_get_irq(int ioapic, int pin, int *trigger, int *polarity);

#else /* !CONFIG_X86_IO_APIC */

@@ -209,8 +210,13 @@ static inline int restore_IO_APIC_setup(struct IO_APIC_route_entry **ent)
return -ENOMEM;
}

-static inline void mp_save_irq(struct mpc_intsrc *m) { };
+static inline void mp_save_irq(struct mpc_intsrc *m) { }
static inline void disable_ioapic_support(void) { }
+static inline int ioapic_get_irq(int ioapic, int pin, int *trigger,
+ int *polarity)
+{
+ return -1;
+}
#endif

#endif /* _ASM_X86_IO_APIC_H */
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index edf86ca..35eb531 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -938,6 +938,18 @@ static int irq_trigger(int idx)
return trigger;
}

+int ioapic_get_irq(int ioapic, int pin, int *trigger, int *polarity)
+{
+ int idx = find_irq_entry(ioapic, pin, mp_INT);
+
+ if (idx >= 0) {
+ *trigger = irq_trigger(idx);
+ *polarity = irq_polarity(idx);
+ }
+
+ return idx;
+}
+
static int pin_2_irq(int idx, int apic, int pin)
{
int irq;
@@ -3785,34 +3797,6 @@ static int __init io_apic_get_version(int ioapic)
return reg_01.bits.version;
}

-int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
-{
- int ioapic, pin, idx;
-
-#ifdef CONFIG_ACPI
- if (acpi_irq_model != ACPI_IRQ_MODEL_IOAPIC)
- return -1;
-#endif
- if (skip_ioapic_setup)
- return -1;
-
- ioapic = mp_find_ioapic(gsi);
- if (ioapic < 0)
- return -1;
-
- pin = mp_find_ioapic_pin(ioapic, gsi);
- if (pin < 0)
- return -1;
-
- idx = find_irq_entry(ioapic, pin, mp_INT);
- if (idx < 0)
- return -1;
-
- *trigger = irq_trigger(idx);
- *polarity = irq_polarity(idx);
- return 0;
-}
-
/*
* This function currently is only a helper for the i386 smp boot process where
* we need to reprogram the ioredtbls to cater for the cpus which have come online
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index 0972315..941328d 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -70,6 +70,30 @@ void __init pci_acpi_crs_quirks(void)
pci_use_crs ? "nocrs" : "use_crs");
}

+int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
+{
+ int ioapic, pin;
+
+ if (acpi_irq_model != ACPI_IRQ_MODEL_IOAPIC)
+ return -1;
+
+ if (skip_ioapic_setup)
+ return -1;
+
+ ioapic = mp_find_ioapic(gsi);
+ if (ioapic < 0)
+ return -1;
+
+ pin = mp_find_ioapic_pin(ioapic, gsi);
+ if (pin < 0)
+ return -1;
+
+ if (ioapic_get_irq(ioapic, pin, trigger, polarity) < 0)
+ return -1;
+
+ return 0;
+}
+
static acpi_status
resource_to_addr(struct acpi_resource *resource,
struct acpi_resource_address64 *addr)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index a2e910e..82c03db 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -348,6 +348,12 @@ static inline int acpi_table_parse(char *id,
{
return -1;
}
+
+static inline int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
+{
+ return -1;
+}
+
#endif /* !CONFIG_ACPI */

#ifdef CONFIG_ACPI_SLEEP
--
1.7.4.1

2011-04-01 16:27:24

by Florian Mickler

[permalink] [raw]
Subject: Re: [PATCH] x86, ioapic: move acpi_get_override_irq to acpi.c

On Fri, 1 Apr 2011 17:44:46 +0200
Florian Mickler <[email protected]> wrote:


>
> Hi Ingo,
>
> Instead of exporting find_irq_entry, irq_trigger and irq_polarity, I added a
> new helper function 'ioapic_get_irq(..)' and exported that.
>
> Also I did it as a seperate commit, since it is more of a cleanup and the other
> commit is already tested... hope this is ok.
>
> Regards,
> Flo
>

btw, include/linux/acpi.h:
120
121 #ifdef CONFIG_X86_IO_APIC
122 extern int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity);
123 #else
124 #define acpi_get_override_irq(gsi, trigger, polarity) (-1)
125 #endif

this shouldn't be needed anymore, as acpi_get_override_irq is now
independent of CONFIG_X86_IO_APIC (because of ioapic_get_irq having
a placeholder in the !CONFIG_X86_IO_APIC case)

But on the other hand, the code flow for the !CONFIG_X86_IO_APIC would
be kind of silly... what's the rationale for mp_find_ioapic returning
0 in the !CONFIG_X86_IO_APIC case? And how do I check compilation
of !CONFIG_X86_IO_APIC? It looks like it already has problems, as
mp_find_ioapic_pin is not stubbed out, or am I overlooking something?

Regards,
Flo

2011-04-02 00:49:13

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH][POKE] Skip looking for ioapic overrides when ioapics are not present

Ingo Molnar <[email protected]> writes:

> * Eric W. Biederman <[email protected]> wrote:
>
>> I don't have a clue why my patch got lost, [...]
>
> It was not lost, it broke the build. You were notified but never followed up
> with a working patch:
>
> https://lkml.org/lkml/2010/10/2/91
>
> and a different patch was applied to get rid of the warning messages. We can
> apply your patch (and that patch can undo the other fix), but it has to build,
> obviously.

Ingo any clue why I this message didn't make it to me directly but I
only received it through the linux-kernel?

Eric

2011-04-02 15:11:40

by Andi Kleen

[permalink] [raw]
Subject: Re: [stable] [PATCH][POKE] Skip looking for ioapic overrides when ioapics are not present

> Thank you. Will do if it turns out this is still necessary.
> Paul Bolle mentioned in another messgage that there is a commit in
> 2.6.38 to the same effect:
>
> commit 678301ecadec24ff77ab310eebf8a32ccddb1850
> ("x86, ioapic: Don't warn about non-existing IOAPICs if we have none")

Folks, do you want that commit in .35 longterm or should some other
patch be used?

Thanks,
-Andi

2011-04-02 18:35:38

by Ingo Molnar

[permalink] [raw]
Subject: Re: [stable] [PATCH][POKE] Skip looking for ioapic overrides when ioapics are not present


* Andi Kleen <[email protected]> wrote:

> > Thank you. Will do if it turns out this is still necessary.
> > Paul Bolle mentioned in another messgage that there is a commit in
> > 2.6.38 to the same effect:
> >
> > commit 678301ecadec24ff77ab310eebf8a32ccddb1850
> > ("x86, ioapic: Don't warn about non-existing IOAPICs if we have none")
>
> Folks, do you want that commit in .35 longterm or should some other
> patch be used?

Nope, that one's fine for -stable.

Thanks,

Ingo

2011-04-02 18:43:50

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH][POKE] Skip looking for ioapic overrides when ioapics are not present


* Eric W. Biederman <[email protected]> wrote:

> Ingo Molnar <[email protected]> writes:
>
> > * Eric W. Biederman <[email protected]> wrote:
> >
> >> I don't have a clue why my patch got lost, [...]
> >
> > It was not lost, it broke the build. You were notified but never followed up
> > with a working patch:
> >
> > https://lkml.org/lkml/2010/10/2/91
> >
> > and a different patch was applied to get rid of the warning messages. We can
> > apply your patch (and that patch can undo the other fix), but it has to build,
> > obviously.
>
> Ingo any clue why I this message didn't make it to me directly but I
> only received it through the linux-kernel?

Hm, no idea - i did not receive any email bounce message.

Did you get this one directly?

Thanks,

Ingo

2011-04-02 22:21:58

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH][POKE] Skip looking for ioapic overrides when ioapics are not present

Ingo Molnar <[email protected]> writes:

> * Eric W. Biederman <[email protected]> wrote:
>
>> Ingo Molnar <[email protected]> writes:
>>
>> > * Eric W. Biederman <[email protected]> wrote:
>> >
>> >> I don't have a clue why my patch got lost, [...]
>> >
>> > It was not lost, it broke the build. You were notified but never followed up
>> > with a working patch:
>> >
>> > https://lkml.org/lkml/2010/10/2/91
>> >
>> > and a different patch was applied to get rid of the warning messages. We can
>> > apply your patch (and that patch can undo the other fix), but it has to build,
>> > obviously.
>>
>> Ingo any clue why I this message didn't make it to me directly but I
>> only received it through the linux-kernel?
>
> Hm, no idea - i did not receive any email bounce message.
>
> Did you get this one directly?

I did. It looks like the gremlins in the network are at it again.
Thank you for looking.

Eric

2011-04-03 14:35:27

by Florian Mickler

[permalink] [raw]
Subject: Re: [PATCH] x86, ioapic: move acpi_get_override_irq to acpi.c

On Fri, 1 Apr 2011 18:26:53 +0200
Florian Mickler <[email protected]> wrote:

> And how do I check compilation of !CONFIG_X86_IO_APIC?

That was a real question, btw. Not just me mumbling around... make
oldconfig always turns that symbol back on to =y ... and I couldn't find
out why.

> Regards,
> Flo

2011-04-04 15:00:52

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86, ioapic: move acpi_get_override_irq to acpi.c


* Florian Mickler <[email protected]> wrote:

> On Fri, 1 Apr 2011 18:26:53 +0200
> Florian Mickler <[email protected]> wrote:
>
> > And how do I check compilation of !CONFIG_X86_IO_APIC?
>
> That was a real question, btw. Not just me mumbling around... make
> oldconfig always turns that symbol back on to =y ... and I couldn't find
> out why.

'make ARCH=i386 allnoconfig' for example.

Thanks,

Ingo

2011-04-12 07:39:42

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] x86, ioapic: move acpi_get_override_irq to acpi.c


* Florian Mickler <[email protected]> wrote:

> In order to get rid of the ugly CONFIG_ACPI ifdef, we move
> the function acpi_get_override_irq into acpi.c and add a new
> helper function ioapic_get_irq to io_apic.c.
>
> Signed-off-by: Florian Mickler <[email protected]>
> Cc: Len Brown <[email protected]>
> Cc: "Eric W. Biederman" <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> LKML-Reference: <[email protected]>
> ---
>
> Hi Ingo,
>
> Instead of exporting find_irq_entry, irq_trigger and irq_polarity, I added a
> new helper function 'ioapic_get_irq(..)' and exported that.
>
> Also I did it as a seperate commit, since it is more of a cleanup and the other
> commit is already tested... hope this is ok.

Yeah, looks good in principle.

Mind porting to the latest x86 devel tree and resend the patch? I tried to
apply your patch but there's a conflict. You can find the tree at:

http://people.redhat.com/mingo/tip.git/README

Thanks,

Ingo

2011-04-12 19:53:57

by Florian Mickler

[permalink] [raw]
Subject: Re: [PATCH] x86, ioapic: move acpi_get_override_irq to acpi.c

On Tue, 12 Apr 2011 09:39:29 +0200
Ingo Molnar <[email protected]> wrote:

>
> * Florian Mickler <[email protected]> wrote:
>
> > In order to get rid of the ugly CONFIG_ACPI ifdef, we move
> > the function acpi_get_override_irq into acpi.c and add a new
> > helper function ioapic_get_irq to io_apic.c.
> >
> > Signed-off-by: Florian Mickler <[email protected]>
> > Cc: Len Brown <[email protected]>
> > Cc: "Eric W. Biederman" <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > LKML-Reference: <[email protected]>
> > ---
> >
> > Hi Ingo,
> >
> > Instead of exporting find_irq_entry, irq_trigger and irq_polarity, I added a
> > new helper function 'ioapic_get_irq(..)' and exported that.
> >
> > Also I did it as a seperate commit, since it is more of a cleanup and the other
> > commit is already tested... hope this is ok.
>
> Yeah, looks good in principle.
>
> Mind porting to the latest x86 devel tree and resend the patch? I tried to
> apply your patch but there's a conflict. You can find the tree at:
>
> http://people.redhat.com/mingo/tip.git/README
>
> Thanks,
>
> Ingo

It does still apply on top of

[PATCH v2] x86, ioapic: Skip looking for ioapic overrides when ioapics
are not present (<[email protected]>)

I'm going to send both patches as a reply to this message, since I
have updated the $subject patch to also remove the #define
acpi_get_override_irq in acpi.h ..

Regards,
Flo

2011-04-12 20:01:31

by Florian Mickler

[permalink] [raw]
Subject: [PATCH 1/2 v2] x86, ioapic: Skip looking for ioapic overrides when ioapics are not present

From: Eric W. Biederman <[email protected]>

Avinash Kurup reported:

|
|     I get the following errors while booting into 2.6.35-rc1. I did not
| get these in 2.6.34. The computer however boots and works fine, so its not
| serious but the following errors are displayed in dmesg:
|
| [    0.089969] ERROR: Unable to locate IOAPIC for GSI 13
| [    0.090556] ERROR: Unable to locate IOAPIC for GSI 8
| [    0.091104] ERROR: Unable to locate IOAPIC for GSI 12
| [    0.091375] ERROR: Unable to locate IOAPIC for GSI 1
| [    0.093195] ERROR: Unable to locate IOAPIC for GSI 4
| [    0.094342] ERROR: Unable to locate IOAPIC for GSI 10
| [    0.096335] ERROR: Unable to locate IOAPIC for GSI 6
|

The new warning originates from acpi_get_override_irq(), which I
recently changed to use helper functions:

9a0a91bb56d2: x86, acpi/irq: Teach acpi_get_override_irq to take a gsi not an isa_irq

These helper functions have the side-effect that they warn when
they fail harmlessly.

When IOAPICs and ACPI are enabled in a kernel and run on ACPI
hardware that doesn't use the ioapics the pnp acpi code calls
this function, looking for ACPI irq overrides. ACPI irq
overrides exist only in the ioapic case so this function will
never succeed.

So make the function fail quickly and silently so we don't call
into help functions that legitimately complain when they fail.

Reference: https://bugzilla.kernel.org/show_bug.cgi?id=17772
Reported-and-Tested-by: Avinash Kurup <[email protected]>
Tested-by: [email protected]
Signed-off-by: "Eric W. Biederman" <[email protected]>
Signed-off-by: Florian Mickler <[email protected]>
[ #ifdef's added, revert of 678301ecadec squashed -Florian]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: #.34+ <[email protected]>
LKML-Reference: <[email protected]>
---
[v2: added ifdefs and squashed revert of 678301ecadec]

arch/x86/kernel/apic/io_apic.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 68df09b..edf86ca 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3789,6 +3789,10 @@ int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
{
int ioapic, pin, idx;

+#ifdef CONFIG_ACPI
+ if (acpi_irq_model != ACPI_IRQ_MODEL_IOAPIC)
+ return -1;
+#endif
if (skip_ioapic_setup)
return -1;

@@ -3951,9 +3955,6 @@ int mp_find_ioapic(u32 gsi)
{
int i = 0;

- if (nr_ioapics == 0)
- return -1;
-
/* Find the IOAPIC that manages this GSI. */
for (i = 0; i < nr_ioapics; i++) {
if ((gsi >= mp_gsi_routing[i].gsi_base)
--
1.7.4.1

2011-04-12 20:01:45

by Florian Mickler

[permalink] [raw]
Subject: [PATCH 2/2 v2] x86, ioapic: move acpi_get_override_irq to acpi.c

In order to get rid of the ugly CONFIG_ACPI ifdef, we move
the function acpi_get_override_irq into acpi.c and add a new
helper function ioapic_get_irq to io_apic.c.

Signed-off-by: Florian Mickler <[email protected]>
Cc: Len Brown <[email protected]>
Cc: "Eric W. Biederman" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: # .34+ <[email protected]>
LKML-Reference: <[email protected]>
---
[v2: removed the acpi_get_override_irq macro in acpi.h]


arch/x86/include/asm/io_apic.h | 8 +++++++-
arch/x86/kernel/apic/io_apic.c | 40 ++++++++++++----------------------------
arch/x86/pci/acpi.c | 24 ++++++++++++++++++++++++
include/linux/acpi.h | 6 ++++++
4 files changed, 49 insertions(+), 29 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index c4bd267..89d3fc6 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -177,6 +177,7 @@ extern void __init pre_init_apic_IRQ0(void);
extern void mp_save_irq(struct mpc_intsrc *m);

extern void disable_ioapic_support(void);
+extern int ioapic_get_irq(int ioapic, int pin, int *trigger, int *polarity);

#else /* !CONFIG_X86_IO_APIC */

@@ -209,8 +210,13 @@ static inline int restore_IO_APIC_setup(struct IO_APIC_route_entry **ent)
return -ENOMEM;
}

-static inline void mp_save_irq(struct mpc_intsrc *m) { };
+static inline void mp_save_irq(struct mpc_intsrc *m) { }
static inline void disable_ioapic_support(void) { }
+static inline int ioapic_get_irq(int ioapic, int pin, int *trigger,
+ int *polarity)
+{
+ return -1;
+}
#endif

#endif /* _ASM_X86_IO_APIC_H */
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index edf86ca..35eb531 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -938,6 +938,18 @@ static int irq_trigger(int idx)
return trigger;
}

+int ioapic_get_irq(int ioapic, int pin, int *trigger, int *polarity)
+{
+ int idx = find_irq_entry(ioapic, pin, mp_INT);
+
+ if (idx >= 0) {
+ *trigger = irq_trigger(idx);
+ *polarity = irq_polarity(idx);
+ }
+
+ return idx;
+}
+
static int pin_2_irq(int idx, int apic, int pin)
{
int irq;
@@ -3785,34 +3797,6 @@ static int __init io_apic_get_version(int ioapic)
return reg_01.bits.version;
}

-int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
-{
- int ioapic, pin, idx;
-
-#ifdef CONFIG_ACPI
- if (acpi_irq_model != ACPI_IRQ_MODEL_IOAPIC)
- return -1;
-#endif
- if (skip_ioapic_setup)
- return -1;
-
- ioapic = mp_find_ioapic(gsi);
- if (ioapic < 0)
- return -1;
-
- pin = mp_find_ioapic_pin(ioapic, gsi);
- if (pin < 0)
- return -1;
-
- idx = find_irq_entry(ioapic, pin, mp_INT);
- if (idx < 0)
- return -1;
-
- *trigger = irq_trigger(idx);
- *polarity = irq_polarity(idx);
- return 0;
-}
-
/*
* This function currently is only a helper for the i386 smp boot process where
* we need to reprogram the ioredtbls to cater for the cpus which have come online
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index 0972315..941328d 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -70,6 +70,30 @@ void __init pci_acpi_crs_quirks(void)
pci_use_crs ? "nocrs" : "use_crs");
}

+int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
+{
+ int ioapic, pin;
+
+ if (acpi_irq_model != ACPI_IRQ_MODEL_IOAPIC)
+ return -1;
+
+ if (skip_ioapic_setup)
+ return -1;
+
+ ioapic = mp_find_ioapic(gsi);
+ if (ioapic < 0)
+ return -1;
+
+ pin = mp_find_ioapic_pin(ioapic, gsi);
+ if (pin < 0)
+ return -1;
+
+ if (ioapic_get_irq(ioapic, pin, trigger, polarity) < 0)
+ return -1;
+
+ return 0;
+}
+
static acpi_status
resource_to_addr(struct acpi_resource *resource,
struct acpi_resource_address64 *addr)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index a2e910e..2a793b2 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -118,11 +118,8 @@ int acpi_register_gsi (struct device *dev, u32 gsi, int triggering, int polarity
int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 *gsi);

-#ifdef CONFIG_X86_IO_APIC
extern int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity);
-#else
-#define acpi_get_override_irq(gsi, trigger, polarity) (-1)
-#endif
+
/*
* This function undoes the effect of one call to acpi_register_gsi().
* If this matches the last registration, any IRQ resources for gsi
@@ -348,6 +345,12 @@ static inline int acpi_table_parse(char *id,
{
return -1;
}
+
+static inline int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity)
+{
+ return -1;
+}
+
#endif /* !CONFIG_ACPI */

#ifdef CONFIG_ACPI_SLEEP
--
1.7.4.1

2011-04-15 10:19:53

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] x86, ioapic: move acpi_get_override_irq to acpi.c


* Florian Mickler <[email protected]> wrote:

> In order to get rid of the ugly CONFIG_ACPI ifdef, we move
> the function acpi_get_override_irq into acpi.c and add a new
> helper function ioapic_get_irq to io_apic.c.
>
> Signed-off-by: Florian Mickler <[email protected]>
> Cc: Len Brown <[email protected]>
> Cc: "Eric W. Biederman" <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: # .34+ <[email protected]>
> LKML-Reference: <[email protected]>
> ---
> [v2: removed the acpi_get_override_irq macro in acpi.h]
>
>
> arch/x86/include/asm/io_apic.h | 8 +++++++-
> arch/x86/kernel/apic/io_apic.c | 40 ++++++++++++----------------------------
> arch/x86/pci/acpi.c | 24 ++++++++++++++++++++++++
> include/linux/acpi.h | 6 ++++++
> 4 files changed, 49 insertions(+), 29 deletions(-)

Len, is this change fine to you?

Thanks,

Ingo

2011-05-17 14:38:25

by Florian Mickler

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] x86, ioapic: move acpi_get_override_irq to acpi.c

Hi,

On Fri, 15 Apr 2011 12:19:38 +0200
Ingo Molnar <[email protected]> wrote:

> > arch/x86/include/asm/io_apic.h | 8 +++++++-
> > arch/x86/kernel/apic/io_apic.c | 40 ++++++++++++----------------------------
> > arch/x86/pci/acpi.c | 24 ++++++++++++++++++++++++
> > include/linux/acpi.h | 6 ++++++
> > 4 files changed, 49 insertions(+), 29 deletions(-)
>
> Len, is this change fine to you?
>
> Thanks,
>
> Ingo


sorry if I missed something... what's the status of these 2 patches?

(fixing https://bugzilla.kernel.org/show_bug.cgi?id=17772 )

Regards,
Flo