Re: [PATCH -v12 2/2] x86: fix out of order of gsi - full

[ebiederm@xmission.com (Eric W. Biederman)]

Yinghai Lu <yinghai@kernel.org> writes:

> Iranna D Ankad reported that IBM x3950 systems have boot problems
> after this commit:
>
>  |
>  | commit b9c61b70075c87a8612624736faf4a2de5b1ed30
>  |
>  |    x86/pci: update pirq_enable_irq() to setup io apic routing
>  |
>
>     As explained in the previous patch ("x86: Fix out of order gsi - partial)
>     try to remap those gsis
>
>     This patch adds boot_ioapic_idx and gsi_to_irq/irq_to_gsi


A couple of nits, that we will eventually want to fix.

> Index: linux-2.6/arch/x86/kernel/apic/io_apic.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/apic/io_apic.c
> +++ linux-2.6/arch/x86/kernel/apic/io_apic.c
> @@ -97,6 +97,41 @@ int mp_irq_entries;
>  /* GSI interrupts */
>  static int nr_irqs_gsi = NR_IRQS_LEGACY;
>  
> +/* By default isa irqs are identity mapped to gsis */
> +unsigned int isa_irq_to_gsi[NR_IRQS_LEGACY] = {
> +	0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15
> +};
> +
> +static int boot_ioapic_idx;
> +static int gsi_delta;
> +int gsi_to_irq(unsigned int gsi)
> +{
> +	unsigned int irq = gsi;
> +	unsigned int i;
> +
> +	irq += gsi_delta;
> +	for (i = 0; i < NR_IRQS_LEGACY; i++) {
> +		if (isa_irq_to_gsi[i] == gsi) {
> +			irq = i;
> +			break;
> +		}
> +	}
> +
> +	return irq;
> +}
> +
> +unsigned int irq_to_gsi(int irq)
> +{
> +	unsigned int gsi;
> +
> +	if (irq < NR_IRQS_LEGACY)
> +		gsi = isa_irq_to_gsi[irq];
> +	else
> +		gsi = irq - gsi_delta;
> +
> +	return gsi;
> +}

This should really live in arch/x86/kernel/acpi/boot.c or similar as
it has everything to do with acpi and gsis and nothing to do with the
ioapics.

Not this merge window, but ultimately we want a fixed value of 16 for
gsi_delta, and we want to always use it.  This has a greater chance of
breaking things but it will ensure in the long run that we flush out
every place that actually needs translation from irqs to gsis.

> Index: linux-2.6/drivers/pnp/pnpacpi/rsparser.c
> ===================================================================
> --- linux-2.6.orig/drivers/pnp/pnpacpi/rsparser.c
> +++ linux-2.6/drivers/pnp/pnpacpi/rsparser.c
> @@ -123,6 +123,14 @@ static void pnpacpi_parse_allocated_irqr
>  	}
>  
>  	flags = irq_flags(triggering, polarity, shareable);
> +#ifdef CONFIG_X86_IO_APIC
> +	/*
> +	 * looks like IBM x3950 is using irq instead of gsi etc...
> +	 * convert it back at first
> +	 */
> +	if (acpi_irq_model == ACPI_IRQ_MODEL_IOAPIC)
> +		gsi = irq_to_gsi(gsi);
> +#endif

I would like to understand this better.  I suspect the actual problem
is that we are getting passed bus_irq instead of gsi from acpi.  If we
are getting passed the bus_irq than doing this just for x3950 is wrong.

I really dislike having special cases for a specific motherboard.  Both
because they are hard to maintain (as quickly no one has that board) and
because may times they are a symptom of a bug elsewhere in the code
that we are trying to patch over instead of actually fix properly.

>  	irq = acpi_register_gsi(&dev->dev, gsi, triggering, polarity);
>  	if (irq >= 0)
>  		pcibios_penalize_isa_irq(irq, 1);

Eric