* Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
[not found] ` <20200903201839.7327-6-murphyt7@tcd.ie>
@ 2020-09-07 7:00 ` Christoph Hellwig
2020-09-07 20:18 ` Tom Murphy
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2020-09-07 7:00 UTC (permalink / raw)
To: Tom Murphy; +Cc: intel-gfx, linux-kernel, iommu, David Woodhouse
On Thu, Sep 03, 2020 at 09:18:37PM +0100, Tom Murphy wrote:
> Disable combining sg segments in the dma-iommu api.
> 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.
So what is the state of addressing this properly in i915? IF we can't
get it done ASAP I wonder if we need a runtime quirk to disable
merging instead of blocking this conversion..
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
2020-09-07 7:00 ` [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu Christoph Hellwig
@ 2020-09-07 20:18 ` Tom Murphy
2020-09-08 5:36 ` Christoph Hellwig
0 siblings, 1 reply; 10+ messages in thread
From: Tom Murphy @ 2020-09-07 20:18 UTC (permalink / raw)
To: Christoph Hellwig
Cc: intel-gfx, Linux Kernel Mailing List, iommu, David Woodhouse
On Mon, 7 Sep 2020 at 08:00, Christoph Hellwig <hch@infradead.org> wrote:
>
> On Thu, Sep 03, 2020 at 09:18:37PM +0100, Tom Murphy wrote:
> > Disable combining sg segments in the dma-iommu api.
> > 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.
>
> So what is the state of addressing this properly in i915? IF we can't
I think this is the latest on addressing this issue:
https://patchwork.kernel.org/cover/11306999/
tl;dr: some people seem to be looking at it but I'm not sure if it's
being actively worked on
> get it done ASAP I wonder if we need a runtime quirk to disable
> merging instead of blocking this conversion..
Yeah we talked about passing an attr to map_sg to disable merging at
the following microconfernce:
https://linuxplumbersconf.org/event/7/contributions/846/
As far as I can remember everyone seemed happy with that solution. I
won't be working on this though as I don't have any more time to
dedicate to this. It seems Lu Baolu will take over this.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
2020-09-07 20:18 ` Tom Murphy
@ 2020-09-08 5:36 ` Christoph Hellwig
2020-09-08 5:55 ` Christoph Hellwig
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2020-09-08 5:36 UTC (permalink / raw)
To: Tom Murphy
Cc: intel-gfx, Linux Kernel Mailing List, Christoph Hellwig, iommu,
David Woodhouse
On Mon, Sep 07, 2020 at 09:18:50PM +0100, Tom Murphy wrote:
> Yeah we talked about passing an attr to map_sg to disable merging at
> the following microconfernce:
> https://linuxplumbersconf.org/event/7/contributions/846/
> As far as I can remember everyone seemed happy with that solution. I
> won't be working on this though as I don't have any more time to
> dedicate to this. It seems Lu Baolu will take over this.
I'm absolutely again passing a flag. Tha just invites further
abuse. We need a PCI ID based quirk or something else that can't
be as easily abused.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
2020-09-08 5:36 ` Christoph Hellwig
@ 2020-09-08 5:55 ` Christoph Hellwig
2020-09-08 6:04 ` Lu Baolu
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2020-09-08 5:55 UTC (permalink / raw)
To: Tom Murphy
Cc: intel-gfx, Linux Kernel Mailing List, Christoph Hellwig, iommu,
David Woodhouse
On Tue, Sep 08, 2020 at 06:36:19AM +0100, Christoph Hellwig wrote:
> On Mon, Sep 07, 2020 at 09:18:50PM +0100, Tom Murphy wrote:
> > Yeah we talked about passing an attr to map_sg to disable merging at
> > the following microconfernce:
> > https://linuxplumbersconf.org/event/7/contributions/846/
> > As far as I can remember everyone seemed happy with that solution. I
> > won't be working on this though as I don't have any more time to
> > dedicate to this. It seems Lu Baolu will take over this.
>
> I'm absolutely again passing a flag. Tha just invites further
> abuse. We need a PCI ID based quirk or something else that can't
> be as easily abused.
Also, I looked at i915 and there are just three dma_map_sg callers.
The two dmabuf related ones are fixed by Marek in his series, leaving
just the one in i915_gem_gtt_prepare_pages, which does indeed look
very fishy. But if that one is so hard to fix it can just be replaced
by an open coded for_each_sg loop that contains manual dma_map_page
calls.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
2020-09-08 5:55 ` Christoph Hellwig
@ 2020-09-08 6:04 ` Lu Baolu
2020-09-08 6:23 ` Christoph Hellwig
0 siblings, 1 reply; 10+ messages in thread
From: Lu Baolu @ 2020-09-08 6:04 UTC (permalink / raw)
To: Christoph Hellwig, Tom Murphy
Cc: intel-gfx, Linux Kernel Mailing List, iommu, David Woodhouse,
baolu.lu
Hi Christoph,
On 9/8/20 1:55 PM, Christoph Hellwig wrote:
> On Tue, Sep 08, 2020 at 06:36:19AM +0100, Christoph Hellwig wrote:
>> On Mon, Sep 07, 2020 at 09:18:50PM +0100, Tom Murphy wrote:
>>> Yeah we talked about passing an attr to map_sg to disable merging at
>>> the following microconfernce:
>>> https://linuxplumbersconf.org/event/7/contributions/846/
>>> As far as I can remember everyone seemed happy with that solution. I
>>> won't be working on this though as I don't have any more time to
>>> dedicate to this. It seems Lu Baolu will take over this.
>>
>> I'm absolutely again passing a flag. Tha just invites further
>> abuse. We need a PCI ID based quirk or something else that can't
>> be as easily abused.
>
> Also, I looked at i915 and there are just three dma_map_sg callers.
> The two dmabuf related ones are fixed by Marek in his series, leaving
Do you mind telling where can I find Marek's series?
Best regards,
baolu
> just the one in i915_gem_gtt_prepare_pages, which does indeed look
> very fishy. But if that one is so hard to fix it can just be replaced
> by an open coded for_each_sg loop that contains manual dma_map_page
> calls.
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
2020-09-08 6:04 ` Lu Baolu
@ 2020-09-08 6:23 ` Christoph Hellwig
2020-09-08 9:07 ` Lu Baolu
2020-09-09 1:43 ` Lu Baolu
0 siblings, 2 replies; 10+ messages in thread
From: Christoph Hellwig @ 2020-09-08 6:23 UTC (permalink / raw)
To: Lu Baolu
Cc: intel-gfx, Linux Kernel Mailing List, Christoph Hellwig, iommu,
Tom Murphy, David Woodhouse
On Tue, Sep 08, 2020 at 02:04:53PM +0800, Lu Baolu wrote:
> Do you mind telling where can I find Marek's series?
[PATCH v10 00/30] DRM: fix struct sg_table nents vs. orig_nents misuse
on various lists including the iommu one.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
2020-09-08 6:23 ` Christoph Hellwig
@ 2020-09-08 9:07 ` Lu Baolu
2020-09-09 1:43 ` Lu Baolu
1 sibling, 0 replies; 10+ messages in thread
From: Lu Baolu @ 2020-09-08 9:07 UTC (permalink / raw)
To: Christoph Hellwig
Cc: intel-gfx, Linux Kernel Mailing List, iommu, Tom Murphy,
David Woodhouse, baolu.lu
On 2020/9/8 14:23, Christoph Hellwig wrote:
> On Tue, Sep 08, 2020 at 02:04:53PM +0800, Lu Baolu wrote:
>> Do you mind telling where can I find Marek's series?
>
> [PATCH v10 00/30] DRM: fix struct sg_table nents vs. orig_nents misuse
>
> on various lists including the iommu one.
>
Get it. Thank you!
Best regards,
baolu
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
2020-09-08 6:23 ` Christoph Hellwig
2020-09-08 9:07 ` Lu Baolu
@ 2020-09-09 1:43 ` Lu Baolu
2020-09-09 7:06 ` Christoph Hellwig
1 sibling, 1 reply; 10+ messages in thread
From: Lu Baolu @ 2020-09-09 1:43 UTC (permalink / raw)
To: Christoph Hellwig
Cc: intel-gfx, Linux Kernel Mailing List, iommu, Tom Murphy,
David Woodhouse, baolu.lu
Hi Christoph,
On 9/8/20 2:23 PM, Christoph Hellwig wrote:
> On Tue, Sep 08, 2020 at 02:04:53PM +0800, Lu Baolu wrote:
>> Do you mind telling where can I find Marek's series?
>
> [PATCH v10 00/30] DRM: fix struct sg_table nents vs. orig_nents misuse
>
> on various lists including the iommu one.
>
It seems that more work is needed in i915 driver. I will added below
quirk as you suggested.
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -851,6 +851,31 @@ 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 device driver is used to assume that the
returned
+ * sg list is not combound. This blocks the efforts of
converting the
+ * 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 (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;
+ }
+
+ return nents;
+ }
+
Best regards,
baolu
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
2020-09-09 1:43 ` Lu Baolu
@ 2020-09-09 7:06 ` Christoph Hellwig
2020-09-12 3:13 ` Lu Baolu
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2020-09-09 7:06 UTC (permalink / raw)
To: Lu Baolu
Cc: intel-gfx, Linux Kernel Mailing List, Christoph Hellwig, iommu,
Tom Murphy, David Woodhouse
On Wed, Sep 09, 2020 at 09:43:09AM +0800, Lu Baolu wrote:
> + /*
> + * The Intel graphic device driver is used to assume that the
> returned
> + * sg list is not combound. This blocks the efforts of converting
> the
This adds pointless overly long lines.
> + * 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 (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;
> + }
> +
> + return nents;
> + }
This wants an IS_ENABLED() check. And probably a pr_once reminding
of the workaround.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
2020-09-09 7:06 ` Christoph Hellwig
@ 2020-09-12 3:13 ` Lu Baolu
0 siblings, 0 replies; 10+ messages in thread
From: Lu Baolu @ 2020-09-12 3:13 UTC (permalink / raw)
To: Christoph Hellwig
Cc: intel-gfx, Linux Kernel Mailing List, iommu, Tom Murphy,
David Woodhouse, baolu.lu
On 2020/9/9 15:06, Christoph Hellwig wrote:
> On Wed, Sep 09, 2020 at 09:43:09AM +0800, Lu Baolu wrote:
>> + /*
>> + * The Intel graphic device driver is used to assume that the
>> returned
>> + * sg list is not combound. This blocks the efforts of converting
>> the
>
> This adds pointless overly long lines.
>
>> + * 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 (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;
>> + }
>> +
>> + return nents;
>> + }
>
> This wants an IS_ENABLED() check. And probably a pr_once reminding
> of the workaround.
>
Will fix in the next version.
Best regards,
baolu
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-09-12 3:14 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20200903201839.7327-1-murphyt7@tcd.ie>
[not found] ` <20200903201839.7327-6-murphyt7@tcd.ie>
2020-09-07 7:00 ` [Intel-gfx] [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu Christoph Hellwig
2020-09-07 20:18 ` Tom Murphy
2020-09-08 5:36 ` Christoph Hellwig
2020-09-08 5:55 ` Christoph Hellwig
2020-09-08 6:04 ` Lu Baolu
2020-09-08 6:23 ` Christoph Hellwig
2020-09-08 9:07 ` Lu Baolu
2020-09-09 1:43 ` Lu Baolu
2020-09-09 7:06 ` Christoph Hellwig
2020-09-12 3:13 ` Lu Baolu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox