On Mon, Apr 15, 2013 at 04:02:56PM -0700, Yinghai Lu wrote: > On Mon, Apr 15, 2013 at 3:41 PM, Neil Horman <nhorman@xxxxxxxxxxxxx> wrote: > > A few years back intel published a spec update: > > http://www.intel.com/content/dam/doc/specification-update/5520-and-5500-chipset-ioh-specification-update.pdf > > ><snip> > > diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c > > index 3755ef4..ef4ac6c 100644 > > --- a/arch/x86/kernel/early-quirks.c > > +++ b/arch/x86/kernel/early-quirks.c > > @@ -18,6 +18,7 @@ > > #include <asm/apic.h> > > #include <asm/iommu.h> > > #include <asm/gart.h> > > +#include "../drivers/iommu/irq_remapping.h" > > looks ugly. > Yes, but I think its acceptible given that it makes sense to me to define the irq_remap_broken flag in the common driver code. We can certainly move the header around, but I'd much rather do that in a separate patch. > > > > static void __init fix_hypertransport_config(int num, int slot, int func) > > { > > @@ -192,6 +193,27 @@ static void __init ati_bugs_contd(int num, int slot, int func) > > } > > #endif > > > > +#ifdef CONFIG_IRQ_REMAP > > +static void __init intel_remapping_check(int num, int slot, int func) > > +{ > > + u8 revision; > > + > > + revision = read_pci_config_byte(num, slot, func, PCI_REVISION_ID); > > + > > + /* > > + * Revision 0x13 of this chipset supports irq remapping > > + * but has an erratum that breaks its behavior, flag it as such > > + */ > > + if (revision == 0x13) > > + irq_remap_broken = 1; > > change to more specific like: > > intel_55xx_rev13_found? > No. This was discussed previously, and the consensus was that we can use a generic name, should other chips have simmilarly broken functionality. ><snip> > > > > int disable_irq_remap; > > +int irq_remap_broken; > > int disable_sourceid_checking; > > int no_x2apic_optout; > > > > diff --git a/drivers/iommu/irq_remapping.h b/drivers/iommu/irq_remapping.h > > index ecb6376..90c4dae 100644 > > --- a/drivers/iommu/irq_remapping.h > > +++ b/drivers/iommu/irq_remapping.h > > @@ -32,6 +32,7 @@ struct pci_dev; > > struct msi_msg; > > > > extern int disable_irq_remap; > > +extern int irq_remap_broken; > > extern int disable_sourceid_checking; > > extern int no_x2apic_optout; > > extern int irq_remapping_enabled; > > @@ -89,6 +90,7 @@ extern struct irq_remap_ops amd_iommu_irq_ops; > > > > #define irq_remapping_enabled 0 > > #define disable_irq_remap 1 > > +#define irq_remap_broken 0 > > this one is needed > Um, yes? I think you mean to say its not needed, since all the users of this check are only in code thats compiled conditionally with CONFIG_IRQ_REMAP. You're correct, but I like to have it there for completness, should that change in the future. Neil -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
