fix platform_rename_gsi related ia32 build breakage

Hi Linus,

This is needed to build current BK tree on IA32.

- Arnaldo

===== arch/i386/kernel/io_apic.c 1.116 vs edited =====

@@ -1039,6 +1039,8 @@
 	return MPBIOS_trigger(idx);
 }
 
+extern int (*platform_rename_gsi)(int ioapic, int gsi);
+
 static int pin_2_irq(int idx, int apic, int pin)
 {
 	int irq, i;

arch/i386/kernel/built-in.o(.text+0xeade): In function `pin_2_irq':
: undefined reference to `platform_rename_gsi'
make: ** [.tmp_vmlinux1] Erro 1

Sorry, the patch is not enough...

- Arnaldo

This one compiles and links OK.

- Arnaldo

===== arch/i386/kernel/io_apic.c 1.116 vs edited =====

@@ -1069,12 +1069,14 @@
 			while (i < apic)
 				irq += nr_ioapic_registers[i++];
 			irq += pin;
+#ifdef CONFIG_ACPI_BOOT
 			/*
 			 * For MPS mode, so far only used by ES7000 platform
 			 */
 			if (platform_rename_gsi)
 				irq = platform_rename_gsi(apic, irq);
 			break;
+#endif
 		}
 		default:
 		{

OK, last try, of course the patch is busted because the #endif should be _before_ the break statement.

/me goes looking for a brown paper bag :-(

- Arnaldo

@@ -1069,11 +1069,13 @@
 			while (i < apic)
 				irq += nr_ioapic_registers[i++];
 			irq += pin;
+#ifdef CONFIG_ACPI_BOOT
 			/*
 			 * For MPS mode, so far only used by ES7000 platform
 			 */
 			if (platform_rename_gsi)
 				irq = platform_rename_gsi(apic, irq);
+#endif
 			break;
 		}
 		default:

Including when CONFIG_ACPI_BOOT is set? Does it see the prototype then? Looks like <asm/acpi.h> should be included, but I assume it gets included some other way?

Quite frankly, I think the whole thing is broken. #ifdef's inside code is _evil_, and "platform_rename_gsi()" doesn't make sense as a name. I'll apply your patch, but quite frankly, I think the ACPI layer is doing crap.

Len, can you please use a more descriptive name, and have it be always defined (make it an inline function that just becomes a no-op or something).

Linus

Agreed, I leave this now to the ACPI guys, it may well be that the whole case is to be ifdefed or something different, who knows. Just please apply the one in the message I was looking for a brown paper bag (i.e. the one with the #endif _before_ the break statement).

Ouch, time to make a hole for the nose, I need some air :o)

This is already fixed in the latest ACPI patch, included in the current -mm tree. So I've gone ahead and excluded the two csets you applied adding the #ifdefs.

I used a function pointer here because the same kernel binary must be able to run on an ES7000 or a non-ES7000, so the compile-time inline idiom doesn't work. Originally we called it es7000-something, but since the mechanism is general it could be used just as well by any non-ES7000 system, so we settled on a more generic name -- platform_rename_gsi().

What this routine does is accomodate exotic hardware. On the ES7000 the hardware designers decided to connect their legacy devices to interrupt pins > 15. Getting Linux to handle this was goodness because it forced me to go through and clean up some old interrupt-source-override code that basically worked by accident. But on the ES7000, the problem remained for what to do about the PCI devices connected to interrupt pins < 16, including pin0. Without platform_rename_gsi, these pins are given identity-mapped IRQ#'s below 16, and Linux lives with lots of legacy code that assumes that IRQ's below 16 are special. So platform_rename_gsi() on the ES7000 simply gives these pins new IRQ#'s starting above what would normally be the highest GSI in the system. These number are not subject to any legacy special cases and MSI etc. work fine.

Finally, there were two places this technicaue is used. ACPI-mode has been in for a while. The one that broke the !ACPI MPS+IOAPIC build yesterday was new -- it replaced a hack in the MPS code that that was downright scary because it could potentially break any non-ES7000 system.

If you read this far and have suggestions for a more descriptive name than platform_rename_gsi(), just let me know.

thanks,
-Len

Sure it does. Do something like this in a header file

	static inline int translate_irq_number(...)
	{
		#ifdef CONFIG_ACPI_BOOT
			return fn_ptr_xxx();
		#else
			return irq;
		#endif
	}

which means that yes, it uses the function pointer when it is meaningful, but if there is no point, the code just goes away.

At _least_ write out what the hell "gsi" is.

TLA's are bad. "gsi" apparently isn't the Geological Survey of Ireland, but that's all I can tell from google.

Linus

sure, we could add a wrapper for the wrapper, but the ifdefs are already gone without doing this -- platform_rename_gsi is present with or without ACPI.

I suppose I could shrink the kernel by 4-bytes by compiling out the function pointer when IO_APIC is not defined. I'll be happy to optimize for that case if you think it justifies the code; though if I were optimizing for that case, this probably isn't where I'd start.

The _gsi in platform_rename_gsi was consistent with the surrounding use in the ACPI case. I decided to re-use the same funtion for the MPS case for simplicity, even though io_apic.c uses _irq. If you like, I can add a synonym using an inline for _irq, but I thought we were moving away from using _irq, not towards it.

GSI = Global System Interrupt
IRQ = overused so much it means nothing at all

While this fact may not win you over as a fan, GSI is actually consistent with the language in the ACPI spec. I don't know if that is where Bjorn came up with the name, but I do think it was a positive change. The reason is that "global" actually means something -- and it still has the same semantics no matter if your machine passes around cpu interrupt vectors or if it passes around pin numbers.

cheers,
-Len

We _definitely_ prefer "irq" over something else that means the same thing.

If GSI means some _specific_ interrupt, and thus has additional meaning over "irq", then by all means, use it, but spell it out. "Global System Interrupt" means _nothing_ to me. What makes it "global"? What makes it "system"?

The _only_ thing that uses "gsi" is the MP table stuff, and that's apparently just from the documentation.

In other words, if it's a normal interrupt, it's "irq" or "interrupt". The same way a "disk" is a "disk" - it's not a DASD.

Stupid acronyms that don't actually mean anything more than the standard name are nothing but stupid.

Interrupts are interrupts. We call them something else only if they are _specific_ interrupts (ie a "NMI" clearly has a very _specific_ meaning, as has "SCI", although the latter is already obscure enough that it's probably a good idea to spell it out a bit if it is ever used outside of a context where use is obvious).

Linus

Hopeless, I know, to debate the "Royal We" on points of style, especially when underscores have been invoked;-)

But humor me -- offer a concise definition of the term IRQ, and then examine usage of the term to see if actually fits within the definition. The term started off to mean the interrupt pin numbers on the 8259A PIC, then it started to mean PCI interrupts (PIRQ) pins too, and sort of IO-APIC pin numbers, except of course when the pin numbers don't actually linearly map to the IRQ numbers, and it can also be an interrupt vector, you see some system never had 8259's, and it can also be the name of the routine that handles an interrupt, and there are softirqs, which are different from hardirqs, and there are fixup-irqs etc. etc.

It isn't possible to offer a term that means the same thing as IRQ, because IRQ is so over-used that it no longer means anything at all.

The term GSI is simple, it is an interrupt number that applies to the entire system, as if the system had a completely flat interrupt model. They even drew a little picture to make this clear in the ACPI spec at
http://www.acpi.info Who would think such a simple thing would merit a diagram?

Don't those silly tech-writers know that software can be written directly from the silicon, and that source-code and the terms used in it is all the documentation that anybody could ever need?:-)

GSI actually did not appear in the MP spec, that referred only to 8259A ISA/EISA IRQ's, PCI PIRQs, and IOAPIC INTINs. If they were not PCI interrupts, the "source bus IRQs" are the same as 8259A Interrupt Request pins.

I agree 100%, and submit that the term "IRQ" fits this definition of a "stupid acronym".

It is true that a GSI and 8259 ISA IRQ are synonyms when their values are < 16., but the term IRQ is being mis-used whenever it is not in that context.

-Len

ps. note that x86 built with MSI or other architectures such as ia64 translate the simple GSI number into a vector and pass it around in that form. I don't know if this is a bug or a feature, but doing so means that all the interrupt code didn't have to be re-written to accomodate these varied architectures, and the semantics of a GSI as a unique global integer identifying an interrupt source remain intact.

There's a huge difference between an acronyn that is well-established, and one that is _totally_ made up, has no history, and is only used on one platform.

"irq" is a very traditional shorthand for "interrupt request", and anybody who has _ever_ worked with any OS on _any_ platform knows _exactly_ what it is.

In contrast, gsi has _zero_ meaning outside some small ACPI group.

Trust me. Do a poll.

The same way we don't call disks DASD devices do we not call interrupts gsi's.

And that "we" is not a "royal we". It's a f*cking established _fact_. Type "irq" into google, and what's the first hit? In fact, EVERY SINGLE hit on the first page is relevant.

In contrast, type "gsi" into google, and NOT A SINGLE ONE has _anything_ to do with interrupts. Not the first one, not the tenth one.

So stop being idiotic.

Linus

Can you please put it into some sane header file, so that if the definition of this thing ever changes, we'll get an error instead of a wrong type silently being used.

In fact, it _is_ in a hader file: <asm/acpi.h>.

Why not just include it?

Linus

Look at the other messages in this brown paper bag saga... It is already in asm/acpi.h, but depends on some config options, etc.

- Arnaldo