| Message ID | CAE9FiQV0pnJW2BYdNSH3J-ZJ1JBtQ8qKf8HAfk1A98U3onn92A@mail.gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
* Yinghai Lu <yinghai@kernel.org> wrote: > [PATCH] iommu: parse pci device scope even only intr remap is defined. > > Andrew found when CONFIG_DMAR=N and CONFIG_INTR_REMAP=Y: > > [ 1137.271447] qla2xxx 0000:18:00.0: irq 97 for MSI/MSI-X > > [ 1137.271706] qla2xxx 0000:18:00.0: Configuring PCI space... > > [ 1137.271725] qla2xxx 0000:18:00.0: setting latency timer to 64 > > [ 1137.271732] qla2xxx 0000:18:00.0: enabling Mem-Wr-Inval > > [ 1137.278705] DRHD: handling fault status reg 2 > > [ 1137.278715] INTR-REMAP: Request device [[18:00.0] fault index 20 > > [ 1137.278717] INTR-REMAP:[fault reason 34] Present field in the IRTE entry is clear > > [ 1159.389099] qla2xxx 0000:0c:07.0: Cable is unplugged... > > [ 1167.218478] qla2xxx 0000:18:00.0: Mailbox command timeout occurred. Scheduling ISP abort. eeh_busy: 0x0 > > [ 1167.218490] qla2xxx 0000:18:00.0: Unable to burst-read optrom segment (100/7ff50400/18389b000). > > [ 1167.218496] qla2xxx 0000:18:00.0: Reverting to slow-read. > > [ 1197.174623] qla2xxx 0000:18:00.0: Unable to burst-read optrom segment (100/7ff50000/18389b000). > > [ 1197.174632] qla2xxx 0000:18:00.0: Reverting to slow-read. > > [ 1197.190613] qla2xxx 0000:18:00.0: Configure NVRAM parameters... > > [ 1197.198582] qla2xxx 0000:18:00.0: Verifying loaded RISC code... > > [ 1227.142951] qla2xxx 0000:18:00.0: Failed mailbox send register test > > [ 1227.142959] qla2xxx 0000:18:00.0: Failed to initialize adapter > > It turns out that path, pci devices will not be link to drhd, so later all pci > devices will point to default drhd when using intr-remap. > > Suresh pointed out: > | This issue is caused by this commit: > | > | commit 9d5ce73a64be2be8112147a3e0b551ad9cd1247b > | Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> > | Date: Tue Nov 10 19:46:16 2009 +0900 > | > | x86: intel-iommu: Convert detect_intel_iommu to use iommu_init hook > | > | So this is a regression > > Try to fix the path with calling dmar_dev_scope_init() in intel_iommu_init() > > Reported-and-tested-by: Andrew Vasquez <andrew.vasquez@qlogic.com> > Reviewed-by: Suresh Siddha <suresh.b.siddha@intel.com> > Signed-off-by: Yinghai Lu <yinghai@kernel.org> > > --- > drivers/iommu/dmar.c | 10 ++++++++++ > include/linux/dmar.h | 4 +--- > 2 files changed, 11 insertions(+), 3 deletions(-) > > Index: linux-2.6/include/linux/dmar.h > =================================================================== > --- linux-2.6.orig/include/linux/dmar.h > +++ linux-2.6/include/linux/dmar.h > @@ -232,9 +232,7 @@ struct dmar_atsr_unit { > #define for_each_atsr_unit(atsr) \ > list_for_each_entry(atsr, &dmar_atsr_units, list) > > -extern int intel_iommu_init(void); > -#else /* !CONFIG_DMAR: */ > -static inline int intel_iommu_init(void) { return -ENODEV; } > #endif /* CONFIG_DMAR */ > +extern int intel_iommu_init(void); > > #endif /* __DMAR_H__ */ > Index: linux-2.6/drivers/iommu/dmar.c > =================================================================== > --- linux-2.6.orig/drivers/iommu/dmar.c > +++ linux-2.6/drivers/iommu/dmar.c > @@ -722,6 +722,16 @@ int __init detect_intel_iommu(void) > return ret ? 1 : -ENODEV; > } > > +#ifndef CONFIG_DMAR > +/* When intr remapping is used but dmar remapping is not defined */ > +int __init intel_iommu_init(void) > +{ > + if (dmar_table_init()) > + return -ENODEV; > + > + return dmar_dev_scope_init(); > +} > +#endif So INTR_REMAP functionality really depends on dmar_table_init()? This looks very messy. CONFIG_DMAR has no clear meaning. The DMAR table parsing functionality is intermixed with the DMAR feature itself. The kernel code is littered with a couple of dozen CONFIG_DMAR #ifdefs with no clear structure to the initialization and to the separation of functionality. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 21, 2011 at 1:56 AM, Ingo Molnar <mingo@elte.hu> wrote: > >> >> +#ifndef CONFIG_DMAR >> +/* When intr remapping is used but dmar remapping is not defined */ >> +int __init intel_iommu_init(void) >> +{ >> + if (dmar_table_init()) >> + return -ENODEV; >> + >> + return dmar_dev_scope_init(); >> +} >> +#endif > > So INTR_REMAP functionality really depends on dmar_table_init()? it will need DMAR table and need to parse ioapic and pci device scope in them and dev scope is needed to done later because it will put pci dev pointer into drhd dev list. > > This looks very messy. > > CONFIG_DMAR has no clear meaning. The DMAR table parsing > functionality is intermixed with the DMAR feature itself. The kernel > code is littered with a couple of dozen CONFIG_DMAR #ifdefs with no > clear structure to the initialization and to the separation of > functionality. CONFIG_DMAR is actually DMA_REMAP instead DMAR table. or Do you prefer to clean them up further with following depency? CONFIG_DMAR_TBL for DMAR table CONFIG_DMA_REMAP for DMA remapping CONFIG_INTR_REMAP for Interrupt remapping and XXX_REMAP will select DMAR_TBL Thanks Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Yinghai Lu <yinghai@kernel.org> wrote: > > This looks very messy. > > > > CONFIG_DMAR has no clear meaning. The DMAR table parsing > > functionality is intermixed with the DMAR feature itself. The > > kernel code is littered with a couple of dozen CONFIG_DMAR > > #ifdefs with no clear structure to the initialization and to the > > separation of functionality. > > CONFIG_DMAR is actually DMA_REMAP instead DMAR table. > > or Do you prefer to clean them up further with following depency? > > CONFIG_DMAR_TBL for DMAR table > CONFIG_DMA_REMAP for DMA remapping > CONFIG_INTR_REMAP for Interrupt remapping > and XXX_REMAP will select DMAR_TBL 'DMAR', 'TBL' and 'INTR' are all misnomers! CONFIG_DMA_REMAP_TABLE CONFIG_DMA_REMAP CONFIG_IRQ_REMAP That way we'd get the 'DMAR tables' via CONFIG_DMA_REMAP_TABLE - on top of which enabling CONFIG_DMA_REMAP and CONFIG_IRQ_REMAP would enable and handle various hw remapping features. (Does anyone else have better/other code structure suggestions?) But yes, we should first do this rename/cleanup to clarify what it all means, then fix whatever config-combos don't work perfectly yet. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 25 Jul 2011, Ingo Molnar wrote: > * Yinghai Lu <yinghai@kernel.org> wrote: > > > > This looks very messy. > > > > > > CONFIG_DMAR has no clear meaning. The DMAR table parsing > > > functionality is intermixed with the DMAR feature itself. The > > > kernel code is littered with a couple of dozen CONFIG_DMAR > > > #ifdefs with no clear structure to the initialization and to the > > > separation of functionality. > > > > CONFIG_DMAR is actually DMA_REMAP instead DMAR table. > > > > or Do you prefer to clean them up further with following depency? > > > > CONFIG_DMAR_TBL for DMAR table > > CONFIG_DMA_REMAP for DMA remapping > > CONFIG_INTR_REMAP for Interrupt remapping > > and XXX_REMAP will select DMAR_TBL > > 'DMAR', 'TBL' and 'INTR' are all misnomers! > > CONFIG_DMA_REMAP_TABLE > CONFIG_DMA_REMAP > CONFIG_IRQ_REMAP > > That way we'd get the 'DMAR tables' via CONFIG_DMA_REMAP_TABLE - on > top of which enabling CONFIG_DMA_REMAP and CONFIG_IRQ_REMAP would > enable and handle various hw remapping features. > > (Does anyone else have better/other code structure suggestions?) > > But yes, we should first do this rename/cleanup to clarify what it > all means, then fix whatever config-combos don't work perfectly yet. All, Is this re-work effort going to make it into 3.1? With 3.1.0-rc1, we are seeing the same 'no interupts being routed' issue. Applying Yinghai's patch to 3.1-rc1: http://www.spinics.net/lists/linux-scsi/msg53416.html get's interrupts routing again.... Thanks, AV -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] iommu: parse pci device scope even only intr remap is defined. Andrew found when CONFIG_DMAR=N and CONFIG_INTR_REMAP=Y: > [ 1137.271447] qla2xxx 0000:18:00.0: irq 97 for MSI/MSI-X > [ 1137.271706] qla2xxx 0000:18:00.0: Configuring PCI space... > [ 1137.271725] qla2xxx 0000:18:00.0: setting latency timer to 64 > [ 1137.271732] qla2xxx 0000:18:00.0: enabling Mem-Wr-Inval > [ 1137.278705] DRHD: handling fault status reg 2 > [ 1137.278715] INTR-REMAP: Request device [[18:00.0] fault index 20 > [ 1137.278717] INTR-REMAP:[fault reason 34] Present field in the IRTE entry is clear > [ 1159.389099] qla2xxx 0000:0c:07.0: Cable is unplugged... > [ 1167.218478] qla2xxx 0000:18:00.0: Mailbox command timeout occurred. Scheduling ISP abort. eeh_busy: 0x0 > [ 1167.218490] qla2xxx 0000:18:00.0: Unable to burst-read optrom segment (100/7ff50400/18389b000). > [ 1167.218496] qla2xxx 0000:18:00.0: Reverting to slow-read. > [ 1197.174623] qla2xxx 0000:18:00.0: Unable to burst-read optrom segment (100/7ff50000/18389b000). > [ 1197.174632] qla2xxx 0000:18:00.0: Reverting to slow-read. > [ 1197.190613] qla2xxx 0000:18:00.0: Configure NVRAM parameters... > [ 1197.198582] qla2xxx 0000:18:00.0: Verifying loaded RISC code... > [ 1227.142951] qla2xxx 0000:18:00.0: Failed mailbox send register test > [ 1227.142959] qla2xxx 0000:18:00.0: Failed to initialize adapter It turns out that path, pci devices will not be link to drhd, so later all pci devices will point to default drhd when using intr-remap. Suresh pointed out: | This issue is caused by this commit: | | commit 9d5ce73a64be2be8112147a3e0b551ad9cd1247b | Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> | Date: Tue Nov 10 19:46:16 2009 +0900 | | x86: intel-iommu: Convert detect_intel_iommu to use iommu_init hook | | So this is a regression Try to fix the path with calling dmar_dev_scope_init() in intel_iommu_init() Reported-and-tested-by: Andrew Vasquez <andrew.vasquez@qlogic.com> Reviewed-by: Suresh Siddha <suresh.b.siddha@intel.com> Signed-off-by: Yinghai Lu <yinghai@kernel.org> --- drivers/iommu/dmar.c | 10 ++++++++++ include/linux/dmar.h | 4 +--- 2 files changed, 11 insertions(+), 3 deletions(-) Index: linux-2.6/include/linux/dmar.h =================================================================== --- linux-2.6.orig/include/linux/dmar.h +++ linux-2.6/include/linux/dmar.h @@ -232,9 +232,7 @@ struct dmar_atsr_unit { #define for_each_atsr_unit(atsr) \ list_for_each_entry(atsr, &dmar_atsr_units, list) -extern int intel_iommu_init(void); -#else /* !CONFIG_DMAR: */ -static inline int intel_iommu_init(void) { return -ENODEV; } #endif /* CONFIG_DMAR */ +extern int intel_iommu_init(void); #endif /* __DMAR_H__ */ Index: linux-2.6/drivers/iommu/dmar.c =================================================================== --- linux-2.6.orig/drivers/iommu/dmar.c +++ linux-2.6/drivers/iommu/dmar.c @@ -722,6 +722,16 @@ int __init detect_intel_iommu(void) return ret ? 1 : -ENODEV; } +#ifndef CONFIG_DMAR +/* When intr remapping is used but dmar remapping is not defined */ +int __init intel_iommu_init(void) +{ + if (dmar_table_init()) + return -ENODEV; + + return dmar_dev_scope_init(); +} +#endif int alloc_iommu(struct dmar_drhd_unit *drhd) {