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

From: Thomas Gleixner
Date: Thu Dec 11 2014 - 15:30:40 EST


On Thu, 11 Dec 2014, Yinghai Lu wrote:
> On Thu, Dec 11, 2014 at 6:33 AM, Jiang Liu <jiang.liu@xxxxxxxxxxxxxxx> 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@xxxxxxxxx> wrote:
> >>> Commit-ID: e9220e591375af6d02604c261999df570fba744f
> >>> Gitweb: http://git.kernel.org/tip/e9220e591375af6d02604c261999df570fba744f
> >>> Author: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> >>> AuthorDate: Fri, 5 Dec 2014 08:48:32 +0000
> >>> Committer: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> >>> 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;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/