Re: [tip:x86/apic] iommu/vt-d: Move iommu preparatory allocations to irq_remap_ops.prepare

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, 11 Dec 2014, Yinghai Lu wrote:
> On Thu, Dec 11, 2014 at 6:33 AM, Jiang Liu <jiang.liu@linux.intel.com> wrote:
> > On 2014/12/11 15:35, Yinghai Lu wrote:
> >> On Fri, Dec 5, 2014 at 3:26 PM, tip-bot for Thomas Gleixner
> >> <tipbot@zytor.com> wrote:
> >>> Commit-ID:  e9220e591375af6d02604c261999df570fba744f
> >>> Gitweb:     http://git.kernel.org/tip/e9220e591375af6d02604c261999df570fba744f
> >>> Author:     Thomas Gleixner <tglx@linutronix.de>
> >>> AuthorDate: Fri, 5 Dec 2014 08:48:32 +0000
> >>> Committer:  Thomas Gleixner <tglx@linutronix.de>
> >>> CommitDate: Sat, 6 Dec 2014 00:19:25 +0100
> >>>
> >>> iommu/vt-d: Move iommu preparatory allocations to irq_remap_ops.prepare
> >>>
> >>> The whole iommu setup for irq remapping is a convoluted mess. The
> >>> iommu detect function gets called from mem_init() and the prepare
> >>> callback gets called from enable_IR_x2apic() for unknown reasons.
> >>
> >> Got
> >>
> > Hi Yinghai,
> >         From following log messages, it seems that the AHCI controllers
> > allocates 16 MSI/MSI-X interrupt, and triggers NULL pointer reference
> > when enabling interrupts for AHCI.
> >         It doesn't trigger panic with this code path (allocate/enable
> > MSI/MSI-X interrupts with IR enabled) on my test system. So could you
> > please help to get more info with the attached test patch?
> 
> [  113.486917] calling  ahci_pci_driver_init+0x0/0x1b @ 1
> [  113.487299] ahci 0000:00:1f.2: version 3.0
> [  113.507317] ahci 0000:00:1f.2: SSS flag set, parallel bus scan disabled
> [  113.507713] ahci 0000:00:1f.2: AHCI 0001.0200 32 slots 6 ports 3
> Gbps 0x3f impl SATA mode
> [  113.527019] ahci 0000:00:1f.2: flags: 64bit ncq sntf stag pm led
> clo pio slum part ccc ems sxs
> [  113.583977] iommu: chip_data ffff881022b97740, iommu
> ffff88103d80ae00, index 32, subindex 0, ir_table ffff88103d802af0,
> table_base ffff881026c00000, queue ffff88102770c000
> [  113.597261] iommu: chip_data ffff881022b97780, iommu
> (null), index 0, subindex 0, ir_table           (null), table_base
>       (null), queue           (null)

So irq_2_iommu is empty. That's a multi MSI, and that's the second
interrupt which gets enabled.

The patch below should fix it.

Thanks,

	tglx

diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index ff35b0336d2b..46da573a4746 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -1131,7 +1131,7 @@ static int intel_irq_remapping_alloc(struct irq_domain *domain,
 {
 	struct intel_iommu *iommu = domain->host_data;
 	struct irq_alloc_info *info = arg;
-	struct intel_ir_data *data;
+	struct intel_ir_data *data, *ird;
 	struct irq_data *irq_data;
 	struct irq_cfg *irq_cfg;
 	int i, ret, index;
@@ -1176,14 +1176,20 @@ static int intel_irq_remapping_alloc(struct irq_domain *domain,
 		}
 
 		if (i > 0) {
-			data = kzalloc(sizeof(*data), GFP_KERNEL);
-			if (!data)
+			ird = kzalloc(sizeof(*data), GFP_KERNEL);
+			if (!ird)
 				goto out_free_data;
+			/* Initialize the common data */
+			ird->irq_2_iommu = data->irq_2_iommu;
+			ird->irq_2_iommu.sub_handle = i;
+		} else {
+			ird = data;
 		}
+
 		irq_data->hwirq = (index << 16) + i;
-		irq_data->chip_data = data;
+		irq_data->chip_data = ird;
 		irq_data->chip = &intel_ir_chip;
-		intel_irq_remapping_prepare_irte(data, irq_cfg, info, index, i);
+		intel_irq_remapping_prepare_irte(ird, irq_cfg, info, index, i);
 		irq_set_status_flags(virq + i, IRQ_MOVE_PCNTXT);
 	}
 	return 0;