diff mbox series

[v4,4/7] iommu: Add quirk for Intel graphic devices in map_sg

Message ID 20200927063437.13988-5-baolu.lu@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series Convert the intel iommu driver to the dma-iommu api | expand

Commit Message

Baolu Lu Sept. 27, 2020, 6:34 a.m. UTC
Combining the sg segments exposes a bug in the Intel i915 driver which
causes visual artifacts and the screen to freeze. This is most likely
because of how the i915 handles the returned list. It probably doesn't
respect the returned value specifying the number of elements in the list
and instead depends on the previous behaviour of the Intel iommu driver
which would return the same number of elements in the output list as in
the input list.

Signed-off-by: Tom Murphy <murphyt7@tcd.ie>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/dma-iommu.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

Robin Murphy Nov. 3, 2020, noon UTC | #1
On 2020-09-27 07:34, Lu Baolu wrote:
> Combining the sg segments exposes a bug in the Intel i915 driver which
> causes visual artifacts and the screen to freeze. This is most likely
> because of how the i915 handles the returned list. It probably doesn't
> respect the returned value specifying the number of elements in the list
> and instead depends on the previous behaviour of the Intel iommu driver
> which would return the same number of elements in the output list as in
> the input list.
> 
> Signed-off-by: Tom Murphy <murphyt7@tcd.ie>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>   drivers/iommu/dma-iommu.c | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 3526db774611..e7e4d758f51a 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -879,6 +879,33 @@ static int __finalise_sg(struct device *dev, struct scatterlist *sg, int nents,
>   	unsigned int cur_len = 0, max_len = dma_get_max_seg_size(dev);
>   	int i, count = 0;
>   
> +	/*
> +	 * The Intel graphic driver is used to assume that the returned
> +	 * sg list is not combound. This blocks the efforts of converting
> +	 * Intel IOMMU driver to dma-iommu api's. Add this quirk to make the
> +	 * device driver work and should be removed once it's fixed in i915
> +	 * driver.
> +	 */
> +	if (IS_ENABLED(CONFIG_DRM_I915) && dev_is_pci(dev) &&
> +	    to_pci_dev(dev)->vendor == PCI_VENDOR_ID_INTEL &&
> +	    (to_pci_dev(dev)->class >> 16) == PCI_BASE_CLASS_DISPLAY) {
> +		for_each_sg(sg, s, nents, i) {
> +			unsigned int s_iova_off = sg_dma_address(s);
> +			unsigned int s_length = sg_dma_len(s);
> +			unsigned int s_iova_len = s->length;
> +
> +			s->offset += s_iova_off;
> +			s->length = s_length;
> +			sg_dma_address(s) = dma_addr + s_iova_off;
> +			sg_dma_len(s) = s_length;
> +			dma_addr += s_iova_len;
> +
> +			pr_info_once("sg combining disabled due to i915 driver\n");
> +		}
> +
> +		return nents;
> +	}

BTW, a much less invasive workaround would be to simply override 
seg_mask to 0. That's enough to make sure that no segment looks eligible 
for merging.

Robin.

> +
>   	for_each_sg(sg, s, nents, i) {
>   		/* Restore this segment's original unaligned fields first */
>   		unsigned int s_iova_off = sg_dma_address(s);
>
diff mbox series

Patch

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 3526db774611..e7e4d758f51a 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -879,6 +879,33 @@  static int __finalise_sg(struct device *dev, struct scatterlist *sg, int nents,
 	unsigned int cur_len = 0, max_len = dma_get_max_seg_size(dev);
 	int i, count = 0;
 
+	/*
+	 * The Intel graphic driver is used to assume that the returned
+	 * sg list is not combound. This blocks the efforts of converting
+	 * Intel IOMMU driver to dma-iommu api's. Add this quirk to make the
+	 * device driver work and should be removed once it's fixed in i915
+	 * driver.
+	 */
+	if (IS_ENABLED(CONFIG_DRM_I915) && dev_is_pci(dev) &&
+	    to_pci_dev(dev)->vendor == PCI_VENDOR_ID_INTEL &&
+	    (to_pci_dev(dev)->class >> 16) == PCI_BASE_CLASS_DISPLAY) {
+		for_each_sg(sg, s, nents, i) {
+			unsigned int s_iova_off = sg_dma_address(s);
+			unsigned int s_length = sg_dma_len(s);
+			unsigned int s_iova_len = s->length;
+
+			s->offset += s_iova_off;
+			s->length = s_length;
+			sg_dma_address(s) = dma_addr + s_iova_off;
+			sg_dma_len(s) = s_length;
+			dma_addr += s_iova_len;
+
+			pr_info_once("sg combining disabled due to i915 driver\n");
+		}
+
+		return nents;
+	}
+
 	for_each_sg(sg, s, nents, i) {
 		/* Restore this segment's original unaligned fields first */
 		unsigned int s_iova_off = sg_dma_address(s);