[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 1/1] x86/vtd: Mask DMAR faults for IGD devices
On 17.04.2020 15:36, Brendan Kerrigan wrote: > The Intel graphics device records DMAR faults regardless > of the Fault Process Disable bit being set. I can't seem to be able to find a place where we would set FPD. Why do we need the workaround then, or what am I missing? > When this fault > occurs, enable the Interrupt Mask (IM) bit in the Fault > Event Control (FECTL) register to prevent the continued > recording of the fault. This should mention the underlying erratum in one way or another. > --- a/xen/drivers/passthrough/vtd/iommu.c > +++ b/xen/drivers/passthrough/vtd/iommu.c > @@ -41,6 +41,8 @@ > #include "vtd.h" > #include "../ats.h" > > +#define IS_IGD(seg, id) (0 == seg && 0 == PCI_BUS(id) && 2 == PCI_SLOT(id) > && 0 == PCI_FUNC(id)) > + > struct mapped_rmrr { > struct list_head list; > u64 base, end; > @@ -872,6 +874,13 @@ static int iommu_page_fault_do_one(struct vtd_iommu > *iommu, int type, > printk(XENLOG_G_WARNING VTDPREFIX "%s: reason %02x - %s\n", > kind, fault_reason, reason); > > + if ( DMA_REMAP == fault_type && type && IS_IGD(seg, source_id) ) { Using existing infrastructure would be at least some improvement, in particular to avoid this workaround triggering on unaffected hardware (including such where there's something else at 0000:00:02.0). See e.g. is_igd_drhd(), and note that most uses of it are currently further qualified to limit what hardware quirk workarounds get applied to. In any event you'll want to clarify in the description that this won't ever be done on hardware not having this issue, at least as long as this isn't obvious from the code. Also - why the "type" part of the conditional? The erratum text doesn't talk about only DMA reads being affected. Finally, style-wise the brace belongs on its own line. > + u32 fectl = dmar_readl(iommu->reg, DMAR_FECTL_REG); > + fectl |= DMA_FECTL_IM; While this is the recommended workaround, I don't see you undoing the masking anywhere later, and I'm also unconvinced this should be done upon seeing the first fault. One option might be to do so on the the first fault when FPD is set for the offending device. Or else, following the title, it might want/need doing unconditionally at initialization time. > + dmar_writel(iommu->reg, DMAR_FECTL_REG, fectl); > + printk(XENLOG_G_WARNING VTDPREFIX "Disabling DMAR faults for IGD\n"); If, as suggested above, IM will get cleared again at some point, this printk() should imo not be issued more than once. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |